diff mbox series

[v5,2/4] spi: Switch to using is_acpi_device_node() in spi_dev_set_name()

Message ID 20240411090628.2436389-3-ckeepax@opensource.cirrus.com (mailing list archive)
State Superseded
Headers show
Series Add bridged amplifiers to cs42l43 | expand

Commit Message

Charles Keepax April 11, 2024, 9:06 a.m. UTC
Use the more modern is_acpi_device_node() rather than checking
ACPI_COMPANION().

Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---

New since v4 of the series.

Thanks,
Charles

 drivers/spi/spi.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Andy Shevchenko April 11, 2024, 1:30 p.m. UTC | #1
On Thu, Apr 11, 2024 at 12:06 PM Charles Keepax
<ckeepax@opensource.cirrus.com> wrote:
>
> Use the more modern is_acpi_device_node() rather than checking
> ACPI_COMPANION().

I don't think it's valuable on its own. There is no clear motivation
why to do that, I suggested it exactly in the conjunction of not
introducing two ways of fwnode type check. That said, you probably
want to elaborate the motivation in the commit message if you want to
keep it separate.

...

> +#include <linux/fwnode.h>

This header is not supposed to be included by the end users. property.h is.
Charles Keepax April 11, 2024, 4:56 p.m. UTC | #2
On Thu, Apr 11, 2024 at 04:30:13PM +0300, Andy Shevchenko wrote:
> On Thu, Apr 11, 2024 at 12:06 PM Charles Keepax
> <ckeepax@opensource.cirrus.com> wrote:
> >
> > Use the more modern is_acpi_device_node() rather than checking
> > ACPI_COMPANION().
> 
> I don't think it's valuable on its own. There is no clear motivation
> why to do that, I suggested it exactly in the conjunction of not
> introducing two ways of fwnode type check. That said, you probably
> want to elaborate the motivation in the commit message if you want to
> keep it separate.
> 

I am really tempted to just drop this, its not necessary for my
changes and changes something that is unrelated to them. At the
least it belongs in a separate patch.

> ...
> 
> > +#include <linux/fwnode.h>
> 
> This header is not supposed to be included by the end users. property.h is.
> 

Fair enough will update, although I really feel these headers
could use some annotation if they are not supposed to be directly
included. Either include everything you use or just include a top
level header makes sense but this weird mixture we seem to use is
very confusing and I don't have a big enough brain to remember
every header.

Thanks,
Charles
Andy Shevchenko April 11, 2024, 6:09 p.m. UTC | #3
On Thu, Apr 11, 2024 at 7:56 PM Charles Keepax
<ckeepax@opensource.cirrus.com> wrote:
> On Thu, Apr 11, 2024 at 04:30:13PM +0300, Andy Shevchenko wrote:
> > On Thu, Apr 11, 2024 at 12:06 PM Charles Keepax
> > <ckeepax@opensource.cirrus.com> wrote:
> > >
> > > Use the more modern is_acpi_device_node() rather than checking
> > > ACPI_COMPANION().
> >
> > I don't think it's valuable on its own. There is no clear motivation
> > why to do that, I suggested it exactly in the conjunction of not
> > introducing two ways of fwnode type check. That said, you probably
> > want to elaborate the motivation in the commit message if you want to
> > keep it separate.
>
> I am really tempted to just drop this, its not necessary for my
> changes and changes something that is unrelated to them. At the
> least it belongs in a separate patch.

Okay, just elaborate in the commit message that this is done to make
new comer not to invent its own way for fwnode type check,

...

> > > +#include <linux/fwnode.h>
> >
> > This header is not supposed to be included by the end users. property.h is.
>
> Fair enough will update, although I really feel these headers
> could use some annotation if they are not supposed to be directly
> included. Either include everything you use or just include a top
> level header makes sense but this weird mixture we seem to use is
> very confusing and I don't have a big enough brain to remember
> every header.

Rough hint is to look into the contents. But I agree, that is a complete mess.
diff mbox series

Patch

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index a2f01116ba09..05b33901eaa9 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -12,6 +12,7 @@ 
 #include <linux/dmaengine.h>
 #include <linux/dma-mapping.h>
 #include <linux/export.h>
+#include <linux/fwnode.h>
 #include <linux/gpio/consumer.h>
 #include <linux/highmem.h>
 #include <linux/idr.h>
@@ -597,10 +598,11 @@  EXPORT_SYMBOL_GPL(spi_alloc_device);
 
 static void spi_dev_set_name(struct spi_device *spi)
 {
-	struct acpi_device *adev = ACPI_COMPANION(&spi->dev);
+	struct device *dev = &spi->dev;
+	struct fwnode_handle *fwnode = dev_fwnode(dev);
 
-	if (adev) {
-		dev_set_name(&spi->dev, "spi-%s", acpi_dev_name(adev));
+	if (is_acpi_device_node(fwnode)) {
+		dev_set_name(dev, "spi-%s", acpi_dev_name(to_acpi_device_node(fwnode)));
 		return;
 	}