Re: [PATCH v2] i2c: i801: Fix missing Kconfig dependency

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06.04.2024 02:29, Andi Shyti wrote:
> Hi Heiner,
> 
> On Fri, Apr 05, 2024 at 07:57:24AM +0200, Heiner Kallweit wrote:
>> On 05.04.2024 02:54, Andi Shyti wrote:
>>> Hi Heiner,
>>>
>>> first of all, thanks for the fast reaction!
>>>
>>> On Thu, Apr 04, 2024 at 10:09:50PM +0200, Heiner Kallweit wrote:
>>>> The original change adds usage of i2c_root_adapter(), which is
>>>> implemented in i2c-mux.c. Therefore we can't use the multiplexing
>>>> if I2C_I801=y and I2C_MUX=m.
>>>
>>> What is wrong with select I2C_MUX?
>>>
>> It would solve the issue, but:
>> We would uselessly enable I2C_MUX also if I2C_MUX_GPIO or DMI are disabled.
>> W/o them the mux part in i801 is a no-op. The call to i2c_root_adapter()
>> is in a conditionally compiled code part, controlled by:
>>
>> #if IS_ENABLED(CONFIG_I2C_MUX_GPIO) && defined CONFIG_DMI
>>
>>> And is this covering all the cases?
>>>
>> yes
>>
>>> Last thing, how have you tested and reproduced the issue?
>>>
>> The CI bot report included a link to the kernel config. So it was easy
>> to understand the root cause of the issue. I could reproduce it by setting:
>> I2C_I801=y
>> I2C_MUX=m
>> I2C_MUX_GPIO=m
>> This config was also used to test the fix.
>>
>> Underlying reason for the issue is that i801 has a code dependency on i2c_mux,
>> but not on i2c_mux_gpio.
> 
> Even though it might look trivial, I need to reproduce the issue
> first and check the various cases.
> 
> Because I'm not at home and, anyway, the report has come late in
> the week, I can queue this up in the next week's pull request.
> 
> I'm not extremely happy with the new _CONFIG but I understand the
> reason behind it.
> 
Another simple solution would be to move the implementation of
i2c_root_adapter() from i2c mux to i2c core. It just uses
i2c_parent_is_i2c_adapter() which is an inline function of
i2c core. What do you think?

> Thanks,
> Andi
> 
Heiner

>>> Thanks,
>>> Andi
>>>
>> Heiner
>>
>>>> Handling the dependencies in the code would become unnecessarily
>>>> complex, therefore create a new config symbol.
>>>>
>>>> Fixes: 71b494e043d2 ("i2c: i801: Call i2c_register_spd for muxed child segments")
>>>> Reported-by: kernel test robot <lkp@xxxxxxxxx>
>>>> Closes: https://lore.kernel.org/oe-kbuild-all/202404042206.MjAQC32x-lkp@xxxxxxxxx/
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
>>





[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux