Re: [PATCH] fpga: mgr: Use devicetree /alias to assign FPGA IDs

[Michal Simek <monstr@monstr.eu>]

[-- Attachment #1.1: Type: text/plain, Size: 2366 bytes --]

On 12. 04. 19 0:11, Brandon Maier wrote:
> The fpga-mgr assigns device IDs based on the order each driver calls
> fpga_mgr_register(). This is fine for systems with one FPGA, but if
> multiple FPGAs are in use, the device ID can change as probe order
> changes across reboots or configuration changes.
> 
> This makes it difficult to use the sysfs attributes, as the device path
> may change. Instead allow a static ID to be set by adding fpgaX props
> to the devicetree /aliases. Falling back to incrementing IDs if none are
> provided.
> 
> This is based on the function rtc_device_get_id() in drivers/rtc/class.c
> (9d2b7e532da8 rtc: honor device tree /alias entries when assigning IDs).
> 
> Signed-off-by: Brandon Maier <brandon.maier@rockwellcollins.com>
> ---
>  drivers/fpga/fpga-mgr.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index c3866816456a..d8b7e7a61117 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -551,6 +551,26 @@ void fpga_mgr_unlock(struct fpga_manager *mgr)
>  }
>  EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
>  
> +static int fpga_mgr_get_id(struct device *dev)
> +{
> +	int of_id = -1, id = -1;
> +
> +	if (dev->of_node)
> +		of_id = of_alias_get_id(dev->of_node, "fpga");
> +
> +	if (of_id >= 0) {
> +		id = ida_simple_get(&fpga_mgr_ida, of_id, of_id + 1,
> +				    GFP_KERNEL);
> +		if (id < 0)
> +			dev_warn(dev, "/aliases ID %d not available\n", of_id);
> +	}
> +
> +	if (id < 0)
> +		id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL);
> +
> +	return id;
> +}

From the first look this is not proper implementation. It is not
handling cases where you have mix of devices with and without aliases.
If first mgr has no alias it gets 0, then second devices with fpga0
alias can't take it even it is assign to it.

For exactly this reason I have introduced of_alias_get_alias_list()
and take a look at cdns_get_id() in serial driver how I am using it.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]