diff mbox series

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

Message ID 206f0f25-8a83-4e53-89fd-cbe025e5798d@gmail.com
State New
Headers show
Series Add and use new helper acpi_use_parent_companion | expand

Commit Message

Heiner Kallweit Oct. 15, 2023, 9:36 p.m. UTC
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(-)

Comments

Michal Wilczynski Oct. 16, 2023, 4:09 p.m. UTC | #1
Hi,

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.


>  	priv->adapter.retries = 3;
>  
>  	priv->pci_dev = dev;
Rafael J. Wysocki Oct. 16, 2023, 5:32 p.m. UTC | #2
On Mon, Oct 16, 2023 at 6:10 PM Wilczynski, Michal
<michal.wilczynski@intel.com> wrote:
>
> Hi,
>
> 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.

If this pattern is repeated in multiple places, the helper makes sense IMO.

>
> >       priv->adapter.retries = 3;
> >
> >       priv->pci_dev = dev;
>
Heiner Kallweit Oct. 16, 2023, 8:05 p.m. UTC | #3
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:
>>
>> Hi,
>>
>> 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.
> 
> 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.

drivers/usb/common/ulpi.c:      ACPI_COMPANION_SET(&ulpi->dev, ACPI_COMPANION(dev));
drivers/usb/dwc3/dwc3-pci.c:    ACPI_COMPANION_SET(&dwc->dwc3->dev, ACPI_COMPANION(dev));
drivers/usb/core/message.c:             ACPI_COMPANION_SET(&intf->dev, ACPI_COMPANION(&dev->dev));
drivers/tty/serial/8250/8250_exar.c:    ACPI_COMPANION_SET(&pdev->dev, ACPI_COMPANION(&pcidev->dev));
drivers/i2c/busses/i2c-imx.c:   ACPI_COMPANION_SET(&i2c_imx->adapter.dev, ACPI_COMPANION(&pdev->dev));
drivers/i2c/busses/i2c-kempld.c:        ACPI_COMPANION_SET(&i2c->adap.dev, ACPI_COMPANION(&pdev->dev));
drivers/i2c/busses/i2c-virtio.c:        ACPI_COMPANION_SET(&vi->adap.dev, ACPI_COMPANION(vdev->dev.parent));
drivers/i2c/busses/i2c-dln2.c:  ACPI_COMPANION_SET(&dln2->adapter.dev, ACPI_COMPANION(&pdev->dev));
drivers/i2c/busses/i2c-i801.c:     ACPI_COMPANION_SET(&priv->adapter.dev, ACPI_COMPANION(&dev->dev));
drivers/i2c/busses/i2c-ismt.c:  ACPI_COMPANION_SET(&priv->adapter.dev, ACPI_COMPANION(&pdev->dev));
drivers/i2c/busses/i2c-cros-ec-tunnel.c:        ACPI_COMPANION_SET(&bus->adap.dev, ACPI_COMPANION(&pdev->dev));
drivers/i2c/busses/i2c-synquacer.c:     ACPI_COMPANION_SET(&i2c->adapter.dev, ACPI_COMPANION(&pdev->dev));
drivers/i2c/busses/i2c-designware-pcidrv.c:     ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev));
drivers/i2c/busses/i2c-qup.c:           ACPI_COMPANION_SET(&qup->adap.dev, ACPI_COMPANION(qup->dev));
drivers/i2c/busses/i2c-designware-platdrv.c:    ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev));
drivers/i2c/busses/i2c-qcom-geni.c:             ACPI_COMPANION_SET(&gi2c->adap.dev, ACPI_COMPANION(dev));
drivers/i2c/busses/i2c-amd-mp2-plat.c:  ACPI_COMPANION_SET(&i2c_dev->adap.dev, ACPI_COMPANION(&pdev->dev));
drivers/i2c/busses/i2c-xgene-slimpro.c: ACPI_COMPANION_SET(&adapter->dev, ACPI_COMPANION(&pdev->dev));
drivers/i2c/busses/i2c-tegra.c: ACPI_COMPANION_SET(&i2c_dev->adapter.dev, ACPI_COMPANION(&pdev->dev));
drivers/i2c/busses/i2c-xlp9xx.c:        ACPI_COMPANION_SET(&priv->adapter.dev, ACPI_COMPANION(&pdev->dev));


>>
>>>       priv->adapter.retries = 3;
>>>
>>>       priv->pci_dev = dev;
>>
Jean Delvare Oct. 24, 2023, 1:09 p.m. UTC | #4
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).
Andi Shyti Oct. 24, 2023, 5:34 p.m. UTC | #5
Hi Heiner,

On Sun, Oct 15, 2023 at 11:36:17PM +0200, 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 find this neater.

Reviewed-by: Andi Shyti <andi.shyti@kernel.org>

Andi
Wolfram Sang Oct. 28, 2023, 1:29 p.m. UTC | #6
> > >> 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.

Because Rafael likes it, I will pick it up.

> > > 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).

I guess this can still be done seperately?
Wolfram Sang Oct. 28, 2023, 1:31 p.m. UTC | #7
On Sun, Oct 15, 2023 at 11:36:17PM +0200, Heiner Kallweit wrote:
> Use new helper acpi_use_parent_companion to simplify the code.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Applied to for-next, thanks!
diff mbox series

Patch

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);
 	priv->adapter.retries = 3;
 
 	priv->pci_dev = dev;