Re: [PATCH RESEND] libata: Assign OF node to the SCSI device

[Guenter Roeck <linux@roeck-us.net>]

On Thu, Feb 20, 2020 at 10:29:35PM +0100, Linus Walleij wrote:
> When we spawn a SCSI device from an ATA device in libata-scsi
> the SCSI device had no relation to the device tree.
> 
> The DT binding allows us to define port nodes under a
> PATA (IDE) or SATA host controller, so we can have proper device
> nodes for these devices.
> 
> If OF is enabled, walk the children of the host controller node
> to see if there is a valid device tree node to assign. The reg
> is used to match to ID 0 for the master device and ID 1 for the
> slave device.
> 
> The corresponding device tree bindings have been accepted by
> the device tree maintainers.
> 
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Bart Van Assche <bvanassche@acm.org>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/ata/libata-scsi.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index eb2eb599e602..0610a3a86de3 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -36,6 +36,7 @@
>  #include <linux/suspend.h>
>  #include <asm/unaligned.h>
>  #include <linux/ioprio.h>
> +#include <linux/of.h>
>  
>  #include "libata.h"
>  #include "libata-transport.h"
> @@ -4582,6 +4583,33 @@ int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
>  	return rc;
>  }
>  
> +#ifdef CONFIG_OF
> +static void ata_scsi_assign_ofnode(struct ata_device *dev, struct ata_port *ap)
> +{
> +	struct scsi_device *sdev = dev->sdev;
> +	struct device *d = ap->host->dev;
> +	struct device_node *np = d->of_node;
> +	struct device_node *child;
> +
> +	for_each_available_child_of_node(np, child) {
> +		int ret;
> +		u32 val;
> +
> +		ret = of_property_read_u32(child, "reg", &val);
> +		if (ret)
> +			continue;
> +		if (val == dev->devno) {
> +			dev_info(d, "found matching device node\n");

Does that add any value or should it be dev_dbg (if anything) ?

> +			sdev->sdev_gendev.of_node = child;

			of_node_put(child);
			break/return;

or is it on purpose to assign the last child if there are multiple matches ?

> +		}
> +	}
> +}
> +#else
> +static void ata_scsi_assign_ofnode(struct ata_device *dev, struct ata_port *ap)
> +{
> +}
> +#endif
> +
>  void ata_scsi_scan_host(struct ata_port *ap, int sync)
>  {
>  	int tries = 5;
> @@ -4607,6 +4635,7 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
>  						 NULL);
>  			if (!IS_ERR(sdev)) {
>  				dev->sdev = sdev;
> +				ata_scsi_assign_ofnode(dev, ap);
>  				scsi_device_put(sdev);
>  			} else {
>  				dev->sdev = NULL;
> -- 
> 2.21.1
>