> On 18/11/2022 10:18, Thierry Reding wrote: > > On Fri, Nov 18, 2022 at 09:38:52AM +0000, Jon Hunter wrote: > >> > >> On 17/11/2022 10:04, Akhil R wrote: > >>> Set ACPI node as the primary fwnode of I2C adapter to allow > >>> enumeration of child devices from the ACPI table > >>> > >>> Signed-off-by: Zubair Waheed <zwaheed@xxxxxxxxxx> > >>> Signed-off-by: Akhil R <akhilrajeev@xxxxxxxxxx> > >>> --- > >>> drivers/i2c/busses/i2c-tegra.c | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c > >>> index 954022c04cc4..69c9ae161bbe 100644 > >>> --- a/drivers/i2c/busses/i2c-tegra.c > >>> +++ b/drivers/i2c/busses/i2c-tegra.c > >>> @@ -1826,6 +1826,7 @@ static int tegra_i2c_probe(struct platform_device > *pdev) > >>> i2c_dev->adapter.class = I2C_CLASS_DEPRECATED; > >>> i2c_dev->adapter.algo = &tegra_i2c_algo; > >>> i2c_dev->adapter.nr = pdev->id; > >>> + ACPI_COMPANION_SET(&i2c_dev->adapter.dev, > ACPI_COMPANION(&pdev->dev)); > >>> if (i2c_dev->hw->supports_bus_clear) > >>> i2c_dev->adapter.bus_recovery_info = > &tegra_i2c_recovery_info; > >> > >> > >> Do we always want to set as the primary fwnode even when booting with > >> device-tree? I some other drivers do, but I also see some others ... > >> > >> if (has_acpi_companion(dev)) > >> ACPI_COMPANION_SET(&i2c_dev->adapter.dev, > >> ACPI_COMPANION(&pdev->dev)); > >> > >> It would be nice to know why it is OK to always do this even for device-tree > >> because it is not clear to me. > > > > ACPI_COMPANION() returns NULL if there is no ACPI companion, which will > > cause ACPI_COMPANION_SET() to set the primary fwnode to NULL. If I read > > the code for set_primary_fwnode() correctly, that's essentially a no-op > > for DT devices. > > Yes it does, but doesn't it is not clear to me if it is a good idea to > pass NULL to set_primary_fwnode(). It does seem to handle this but my > biggest gripe is the lack of explanation in the commit message why this > is OK. I saw ACPI_COMPANION_SET() as an empty function if CONFIG_ACPI is not set. Yes, I agree that I should have mentioned this in the commit message. Shall I send a v2 with the details added in the commit description? Regards, Akhil