Re: [PATCH 2/2] i2c: i801: Use new helper acpi_use_parent_companion

[Jean Delvare <jdelvare@suse.de>]

Hi Heiner and all,

On Mon, 16 Oct 2023 22:05:51 +0200, Heiner Kallweit wrote:
> On 16.10.2023 19:32, Rafael J. Wysocki wrote:
> > On Mon, Oct 16, 2023 at 6:10 PM Wilczynski, Michal
> > <michal.wilczynski@intel.com> wrote:  
> >> On 10/15/2023 11:36 PM, Heiner Kallweit wrote:  
> >>> Use new helper acpi_use_parent_companion to simplify the code.
> >>>
> >>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> >>> ---
> >>>  drivers/i2c/busses/i2c-i801.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> >>> index a41f5349a..ac223146c 100644
> >>> --- a/drivers/i2c/busses/i2c-i801.c
> >>> +++ b/drivers/i2c/busses/i2c-i801.c
> >>> @@ -1620,7 +1620,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
> >>>       priv->adapter.class = I2C_CLASS_HWMON;
> >>>       priv->adapter.algo = &smbus_algorithm;
> >>>       priv->adapter.dev.parent = &dev->dev;
> >>> -     ACPI_COMPANION_SET(&priv->adapter.dev, ACPI_COMPANION(&dev->dev));
> >>> +     acpi_use_parent_companion(&priv->adapter.dev);  
> >>
> >> I think this case is a bit too trivial for a helper, it's one line before, and
> >> one line after, so it doesn't really save much.  

I must say I share Michal's skepticism.

> > If this pattern is repeated in multiple places, the helper makes sense IMO.
> 
> I didn't check each usage in detail, but this should be the places where the new
> helper can be used.
> Another advantage IMO is that the helper, being a function instead of a macro,
> is type-safe.

If type safety is a concern then I'd rather turn ACPI_COMPANION_SET to
an inline function (which would make more sense than a macro anyway
IMHO, as it has an intended side effect).

-- 
Jean Delvare
SUSE L3 Support