| Message ID | 206f0f25-8a83-4e53-89fd-cbe025e5798d@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Add and use new helper acpi_use_parent_companion | expand |
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;
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; >
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; >>
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).
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
> > >> 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?
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 --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;
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(-)