Re: [PATCH v3 2/3] spi: Add a mechanism to use the fwnode name for the SPI device

[Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx>]

On Wed, Apr 03, 2024 at 11:29:27PM +0300, Andy Shevchenko wrote:
> Fri, Mar 29, 2024 at 11:47:29AM +0000, Charles Keepax kirjoitti:
> >  	struct acpi_device *adev = ACPI_COMPANION(&spi->dev);
> > +	struct fwnode_handle *fwnode = dev_fwnode(&spi->dev);
> > +
> > +	if (spi->use_fwnode_name && fwnode) {
> > +		dev_set_name(&spi->dev, "spi-%s", fwnode_get_name(fwnode));
> > +		return;
> > +	}
> >  
> >  	if (adev) {
> >  		dev_set_name(&spi->dev, "spi-%s", acpi_dev_name(adev));
> 
> This should be something like this
> 
> 	struct device *dev = &spi->dev;
> 	struct fwnode_handle *fwnode = dev_fwnode(dev);
> 
> 	if (is_acpi_device_node(fwnode)) {
> 		dev_set_name(dev, "spi-%s", acpi_dev_name(to_acpi_device_node(fwnode)));
> 		return;
> 	}
> 
> 	if (is_software_node(fwnode)) {
> 		dev_set_name(dev, "spi-%s", fwnode_get_name(fwnode));
> 		return;
> 	}
> 
> i.o.w. we don't need to have two ways of checking fwnode type and you may get
> rid of unneeded variable, and always use fwnode name for swnode.
> 
> ...
> 
> > +	proxy->use_fwnode_name = chip->use_fwnode_name;
> 
> Unneeded variable. See above.
> 

Hmm... I guess I was viewing this feature more as something that
users would opt into. So other firmware mechanisms could use it
if required, and so most swnode based controllers would still get
caught by the standard naming at the bottom of the function.

>From my perspective it will do what I need either way, so happy
to update it to always use this for software nodes if consensus
goes that way.

Thanks,
Charles