| Message ID | 20240411090628.2436389-3-ckeepax@opensource.cirrus.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add bridged amplifiers to cs42l43 | expand |
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.
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
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 --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; }
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(-)