[v3] Add optional properties to MAX17040

[PATCH v3 4/4] ARM: configs: s5pv210_defconfig: enable IIO required by MAX17040

Posted by Svyatoslav Ryhel 2 years, 1 month ago

After adding support for passing temperature data from thermal sensor
to MAX17040 it got dependency on CONFIG_IIO. From all defconfigs
using MAX17040 only s5pv210_defconfig did not have IIO already enabled
so let's enable it to avoid regression.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 arch/arm/configs/s5pv210_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/s5pv210_defconfig b/arch/arm/configs/s5pv210_defconfig
index 4c1e480b5bbd..24070ee3d43e 100644
--- a/arch/arm/configs/s5pv210_defconfig
+++ b/arch/arm/configs/s5pv210_defconfig
@@ -97,6 +97,7 @@ CONFIG_MMC_SDHCI_S3C_DMA=y
 CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_MAX8998=m
 CONFIG_DMADEVICES=y
+CONFIG_IIO=y
 CONFIG_PWM=y
 CONFIG_PWM_SAMSUNG=y
 CONFIG_PHY_SAMSUNG_USB2=m
-- 
2.39.2

Re: [PATCH v3 4/4] ARM: configs: s5pv210_defconfig: enable IIO required by MAX17040

Posted by Andi Shyti 2 years, 1 month ago

Hi Svyatoslav,

On Mon, Jul 31, 2023 at 10:36:13AM +0300, Svyatoslav Ryhel wrote:
> After adding support for passing temperature data from thermal sensor
> to MAX17040 it got dependency on CONFIG_IIO. From all defconfigs
> using MAX17040 only s5pv210_defconfig did not have IIO already enabled
> so let's enable it to avoid regression.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  arch/arm/configs/s5pv210_defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/configs/s5pv210_defconfig b/arch/arm/configs/s5pv210_defconfig
> index 4c1e480b5bbd..24070ee3d43e 100644
> --- a/arch/arm/configs/s5pv210_defconfig
> +++ b/arch/arm/configs/s5pv210_defconfig
> @@ -97,6 +97,7 @@ CONFIG_MMC_SDHCI_S3C_DMA=y
>  CONFIG_RTC_CLASS=y
>  CONFIG_RTC_DRV_MAX8998=m
>  CONFIG_DMADEVICES=y
> +CONFIG_IIO=y
>  CONFIG_PWM=y
>  CONFIG_PWM_SAMSUNG=y
>  CONFIG_PHY_SAMSUNG_USB2=m

Should this patch be squashed to the previous patch? I think you
break bisectability for this board if you enable iio only here.

Andi

Re: [PATCH v3 4/4] ARM: configs: s5pv210_defconfig: enable IIO required by MAX17040

Posted by Krzysztof Kozlowski 2 years, 1 month ago

On 06/08/2023 00:42, Andi Shyti wrote:
> Hi Svyatoslav,
> 
> On Mon, Jul 31, 2023 at 10:36:13AM +0300, Svyatoslav Ryhel wrote:
>> After adding support for passing temperature data from thermal sensor
>> to MAX17040 it got dependency on CONFIG_IIO. From all defconfigs
>> using MAX17040 only s5pv210_defconfig did not have IIO already enabled
>> so let's enable it to avoid regression.
>>
>> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
>> ---
>>  arch/arm/configs/s5pv210_defconfig | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/configs/s5pv210_defconfig b/arch/arm/configs/s5pv210_defconfig
>> index 4c1e480b5bbd..24070ee3d43e 100644
>> --- a/arch/arm/configs/s5pv210_defconfig
>> +++ b/arch/arm/configs/s5pv210_defconfig
>> @@ -97,6 +97,7 @@ CONFIG_MMC_SDHCI_S3C_DMA=y
>>  CONFIG_RTC_CLASS=y
>>  CONFIG_RTC_DRV_MAX8998=m
>>  CONFIG_DMADEVICES=y
>> +CONFIG_IIO=y
>>  CONFIG_PWM=y
>>  CONFIG_PWM_SAMSUNG=y
>>  CONFIG_PHY_SAMSUNG_USB2=m
> 
> Should this patch be squashed to the previous patch? I think you
> break bisectability for this board if you enable iio only here.

The defconfig change matters less - distros don't use them - so this
points to the fact that patchset affected the users. All existing users
of max17040 drivers, who do not enable IIO, will have their setups broken.

Best regards,
Krzysztof

Re: [PATCH v3 4/4] ARM: configs: s5pv210_defconfig: enable IIO required by MAX17040

Posted by Andi Shyti 2 years, 1 month ago

Hi,

On Sun, Aug 06, 2023 at 10:29:04AM +0200, Krzysztof Kozlowski wrote:
> On 06/08/2023 00:42, Andi Shyti wrote:
> > Hi Svyatoslav,
> > 
> > On Mon, Jul 31, 2023 at 10:36:13AM +0300, Svyatoslav Ryhel wrote:
> >> After adding support for passing temperature data from thermal sensor
> >> to MAX17040 it got dependency on CONFIG_IIO. From all defconfigs
> >> using MAX17040 only s5pv210_defconfig did not have IIO already enabled
> >> so let's enable it to avoid regression.
> >>
> >> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> >> ---
> >>  arch/arm/configs/s5pv210_defconfig | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/arch/arm/configs/s5pv210_defconfig b/arch/arm/configs/s5pv210_defconfig
> >> index 4c1e480b5bbd..24070ee3d43e 100644
> >> --- a/arch/arm/configs/s5pv210_defconfig
> >> +++ b/arch/arm/configs/s5pv210_defconfig
> >> @@ -97,6 +97,7 @@ CONFIG_MMC_SDHCI_S3C_DMA=y
> >>  CONFIG_RTC_CLASS=y
> >>  CONFIG_RTC_DRV_MAX8998=m
> >>  CONFIG_DMADEVICES=y
> >> +CONFIG_IIO=y
> >>  CONFIG_PWM=y
> >>  CONFIG_PWM_SAMSUNG=y
> >>  CONFIG_PHY_SAMSUNG_USB2=m
> > 
> > Should this patch be squashed to the previous patch? I think you
> > break bisectability for this board if you enable iio only here.
> 
> The defconfig change matters less - distros don't use them - so this
> points to the fact that patchset affected the users. All existing users
> of max17040 drivers, who do not enable IIO, will have their setups broken.

That's why I'm suggesting to squash this patch with the previous.

Anyway, up to you... except of this note everything looks fine in
the series.

Andi

Re: [PATCH v3 4/4] ARM: configs: s5pv210_defconfig: enable IIO required by MAX17040

Posted by Krzysztof Kozlowski 2 years, 1 month ago

On 07/08/2023 10:55, Andi Shyti wrote:
> Hi,
> 
> On Sun, Aug 06, 2023 at 10:29:04AM +0200, Krzysztof Kozlowski wrote:
>> On 06/08/2023 00:42, Andi Shyti wrote:
>>> Hi Svyatoslav,
>>>
>>> On Mon, Jul 31, 2023 at 10:36:13AM +0300, Svyatoslav Ryhel wrote:
>>>> After adding support for passing temperature data from thermal sensor
>>>> to MAX17040 it got dependency on CONFIG_IIO. From all defconfigs
>>>> using MAX17040 only s5pv210_defconfig did not have IIO already enabled
>>>> so let's enable it to avoid regression.
>>>>
>>>> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
>>>> ---
>>>>  arch/arm/configs/s5pv210_defconfig | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/arch/arm/configs/s5pv210_defconfig b/arch/arm/configs/s5pv210_defconfig
>>>> index 4c1e480b5bbd..24070ee3d43e 100644
>>>> --- a/arch/arm/configs/s5pv210_defconfig
>>>> +++ b/arch/arm/configs/s5pv210_defconfig
>>>> @@ -97,6 +97,7 @@ CONFIG_MMC_SDHCI_S3C_DMA=y
>>>>  CONFIG_RTC_CLASS=y
>>>>  CONFIG_RTC_DRV_MAX8998=m
>>>>  CONFIG_DMADEVICES=y
>>>> +CONFIG_IIO=y
>>>>  CONFIG_PWM=y
>>>>  CONFIG_PWM_SAMSUNG=y
>>>>  CONFIG_PHY_SAMSUNG_USB2=m
>>>
>>> Should this patch be squashed to the previous patch? I think you
>>> break bisectability for this board if you enable iio only here.
>>
>> The defconfig change matters less - distros don't use them - so this
>> points to the fact that patchset affected the users. All existing users
>> of max17040 drivers, who do not enable IIO, will have their setups broken.
> 
> That's why I'm suggesting to squash this patch with the previous.

It would not solve much. All existing users will be still broken.

> 
> Anyway, up to you... except of this note everything looks fine in
> the series.

I would actually prefer not to depend on IIO, but this would require
stubs for missing IIO functions.

Best regards,
Krzysztof

Re: (subset) [PATCH v3 4/4] ARM: configs: s5pv210_defconfig: enable IIO required by MAX17040

Posted by Krzysztof Kozlowski 1 year, 12 months ago

On Mon, 31 Jul 2023 10:36:13 +0300, Svyatoslav Ryhel wrote:
> After adding support for passing temperature data from thermal sensor
> to MAX17040 it got dependency on CONFIG_IIO. From all defconfigs
> using MAX17040 only s5pv210_defconfig did not have IIO already enabled
> so let's enable it to avoid regression.
> 
> 

Applied, thanks!

[4/4] ARM: configs: s5pv210_defconfig: enable IIO required by MAX17040
      https://git.kernel.org/krzk/linux/c/dc836afd2be7618d8c849fd93bd3e15513289b70

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>