fpga: xilinx-spi: Use unique FPGA manager names

PATCH In review Anatolij Gustschin 4 participants 8 messages Oct 29, 2018 – Nov 2, 2018

Subsystems (2)

Arm/Zynq architecture FPGA manager framework

lore

Share & tools

lore

Anatolij Gustschin

author

2018-10-29 23:31 UTC #0 lore raw

PATCH 1/1 fpga: xilinx-spi: Use unique FPGA manager names

Currently we have the same FPGA manager name for all registered xlnx-slave-spi managers, so it is not clear which fpga manager index belongs to which configuration interface (SPI slave device). Use unique fpga manager name for each registered manager. With this change we have names with SPI slave device name encoded in the manager name string, e.g. like "xlnx-slave-spi spi1.2".

Signed-off-by Anatolij Gustschin <[email protected]>

---
 drivers/fpga/xilinx-spi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

drivers/fpga/xilinx-spi.c +6 -2

index 469486be20c4..34b13d1c0f88 100644
@@ -27,6 +27,7 @@ struct xilinx_spi_conf {
 	struct spi_device *spi;
 	struct gpio_desc *prog_b;
 	struct gpio_desc *done;
+	char mgr_name[64];
 };
 
 static enum fpga_mgr_states xilinx_spi_state(struct fpga_manager *mgr)

@@ -166,8 +167,11 @@ static int xilinx_spi_probe(struct spi_device *spi)
 		return PTR_ERR(conf->done);
 	}
 
-	mgr = devm_fpga_mgr_create(&spi->dev,
-				   "Xilinx Slave Serial FPGA Manager",
+	/* Register manager with unique name */
+	snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s %s",
+		 dev_driver_string(&spi->dev), dev_name(&spi->dev));
+
+	mgr = devm_fpga_mgr_create(&spi->dev, conf->mgr_name,
 				   &xilinx_spi_ops, conf);
 	if (!mgr)
 		return -ENOMEM;

Signature · 1 line

2.17.1

Moritz Fischer

maintainer

2018-10-29 16:48 UTC #1 lore raw

Hi Anatolij,

Quoted 6 lines · Anatolij Gustschin

Currently we have the same FPGA manager name for all registered xlnx-slave-spi managers, so it is not clear which fpga manager index belongs to which configuration interface (SPI slave device). Use unique fpga manager name for each registered manager. With this change we have names with SPI slave device name encoded in the manager name string, e.g. like "xlnx-slave-spi spi1.2".

Sounds like a udev problem that can be solved by looking at the parent device?

Cheers,

Moritz

Anatolij Gustschin

author

2018-10-30 11:19 UTC #2 lore raw

Hi Moritz,

On Mon, 29 Oct 2018 09:48:57 -0700
Moritz Fischer [email protected] wrote:
...

Quoted 2 lines

Sounds like a udev problem that can be solved by looking at the parent device?

Yes, but there are still all kinds of small embedded systems without udev/systemd support.

Thanks,

Anatolij

Alan Tull

2018-11-01 18:21 UTC #3 lore raw

Quoted 11 lines · Anatolij Gustschin

Hi Moritz,
On Mon, 29 Oct 2018 09:48:57 -0700
Moritz Fischer [email protected] wrote:
...
Quoted 2 lines
Sounds like a udev problem that can be solved by looking at the parent device?
Yes, but there are still all kinds of small embedded systems without udev/systemd support.

What do you see if you look at the following for your fpga manager (may be other than fpga0)?

/sys/class/fpga_manager/fpga0/device

/sys/class/fpga_manager/fpga0/of_node

Alan

Quoted 4 lines

Thanks,
Anatolij

Anatolij Gustschin

author

2018-11-01 18:33 UTC #4 lore raw

Quoted 13 lines · On Thu, 1 Nov 2018 13:21:00 -0500 Alan Tull [email protected] wrote:

Quoted 11 lines · Anatolij Gustschin
Hi Moritz,
On Mon, 29 Oct 2018 09:48:57 -0700
Moritz Fischer [email protected] wrote:
...
Quoted 2 lines
Sounds like a udev problem that can be solved by looking at the parent device?
Yes, but there are still all kinds of small embedded systems without udev/systemd support.

Quoted 8 lines

What do you see if you look at the following for your fpga manager (may be other than fpga0)?
/sys/class/fpga_manager/fpga0/device
or
/sys/class/fpga_manager/fpga0/of_node

of_node isn't there, it is a non-dt platform.

# ls -l /sys/class/fpga_manager/fpga0/device
lrwxrwxrwx 1 root root 0 Nov  1 19:26 /sys/class/fpga_manager/fpga0/device -> ../../../spi1.0

# ls -l /sys/class/fpga_manager/fpga0/device/
total 0
lrwxrwxrwx 1 root root    0 Nov  1 16:48 driver -> ../../../../../../../../../../../../bus/spi/drivers/altera-ps-spi
-rw-r--r-- 1 root root 4096 Nov  1 19:26 driver_override
drwxr-xr-x 3 root root    0 Nov  1 16:48 fpga_manager
-r--r--r-- 1 root root 4096 Nov  1 19:26 modalias
drwxr-xr-x 2 root root    0 Nov  1 19:26 power
drwxr-xr-x 2 root root    0 Nov  1 19:26 statistics
lrwxrwxrwx 1 root root    0 Nov  1 16:48 subsystem -> ../../../../../../../../../../../../bus/spi
-rw-r--r-- 1 root root 4096 Nov  1 16:48 uevent

Thanks,

Anatolij

Alan Tull

2018-11-01 18:46 UTC #5 lore raw

Quoted 20 lines · Anatolij Gustschin

Quoted 16 lines · On Thu, 1 Nov 2018 13:21:00 -0500 Alan Tull [email protected] wrote:
Quoted 11 lines · Anatolij Gustschin
Hi Moritz,
On Mon, 29 Oct 2018 09:48:57 -0700
Moritz Fischer [email protected] wrote:
...
Quoted 2 lines
Sounds like a udev problem that can be solved by looking at the parent device?
Yes, but there are still all kinds of small embedded systems without udev/systemd support.
What do you see if you look at the following for your fpga manager (may be other than fpga0)?

Quoted 7 lines

Quoted 5 lines
/sys/class/fpga_manager/fpga0/device
or
/sys/class/fpga_manager/fpga0/of_node
of_node isn't there, it is a non-dt platform.

Oh yeah.

Quoted 3 lines

# ls -l /sys/class/fpga_manager/fpga0/device
lrwxrwxrwx 1 root root 0 Nov  1 19:26 /sys/class/fpga_manager/fpga0/device -> ../../../spi1.0

Is 'device' giving the info this patch is adding to 'name'?

Quoted 15 lines

# ls -l /sys/class/fpga_manager/fpga0/device/
total 0
lrwxrwxrwx 1 root root    0 Nov  1 16:48 driver -> ../../../../../../../../../../../../bus/spi/drivers/altera-ps-spi
-rw-r--r-- 1 root root 4096 Nov  1 19:26 driver_override
drwxr-xr-x 3 root root    0 Nov  1 16:48 fpga_manager
-r--r--r-- 1 root root 4096 Nov  1 19:26 modalias
drwxr-xr-x 2 root root    0 Nov  1 19:26 power
drwxr-xr-x 2 root root    0 Nov  1 19:26 statistics
lrwxrwxrwx 1 root root    0 Nov  1 16:48 subsystem -> ../../../../../../../../../../../../bus/spi
-rw-r--r-- 1 root root 4096 Nov  1 16:48 uevent

Thanks,

Anatolij

Anatolij Gustschin

author

2018-11-01 19:23 UTC #6 lore raw

On Thu, 1 Nov 2018 13:46:27 -0500
Alan Tull [email protected] wrote:
...

Quoted 4 lines

Quoted 2 lines
# ls -l /sys/class/fpga_manager/fpga0/device
lrwxrwxrwx 1 root root 0 Nov  1 19:26 /sys/class/fpga_manager/fpga0/device -> ../../../spi1.0  
Is 'device' giving the info this patch is adding to 'name'?

One can extract this info from 'device' somehow, but it is much more easier to get it from the name

# cat /sys/class/fpga_manager/fpga0/name 
xlnx-slave-spi spi1.0

Thanks,

Anatolij

Moritz Fischer

2018-11-02 04:48 UTC #7 lore raw

Hi Anatolij,

Quoted 7 lines · Anatolij Gustschin

On Thu, 1 Nov 2018 13:46:27 -0500
Alan Tull [email protected] wrote:
...
Quoted 3 lines
Quoted 2 lines
# ls -l /sys/class/fpga_manager/fpga0/device
lrwxrwxrwx 1 root root 0 Nov  1 19:26 /sys/class/fpga_manager/fpga0/device -> ../../../spi1.0

Quoted 7 lines

Quoted 1 line
Is 'device' giving the info this patch is adding to 'name'?
One can extract this info from 'device' somehow, but it is much more easier to get it from the name
# cat /sys/class/fpga_manager/fpga0/name
xlnx-slave-spi spi1.0

I see your use case, but I don't think the FPGA Manager framework is the right place to fix this,
especially not encoding bus information in the FPGA Manager name. The info is already available
to userland (not conveniently I agree, but nevertheless).

From my end NAK.

Thanks,

Moritz