| Message ID | 20181029144222.26927-1-agust@denx.de (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | fpga: xilinx-spi: Use unique FPGA manager names | expand |
Hi Anatolij, On Mon, Oct 29, 2018 at 03:42:22PM +0100, Anatolij Gustschin wrote: > 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
Hi Moritz, On Mon, 29 Oct 2018 09:48:57 -0700 Moritz Fischer mdf@kernel.org wrote: ... >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
On Tue, Oct 30, 2018 at 6:19 AM Anatolij Gustschin <agust@denx.de> wrote: > > Hi Moritz, > > On Mon, 29 Oct 2018 09:48:57 -0700 > Moritz Fischer mdf@kernel.org wrote: > ... > >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 or /sys/class/fpga_manager/fpga0/of_node Alan > > Thanks, > > Anatolij
On Thu, 1 Nov 2018 13:21:00 -0500 Alan Tull atull@kernel.org wrote: >On Tue, Oct 30, 2018 at 6:19 AM Anatolij Gustschin <agust@denx.de> wrote: >> >> Hi Moritz, >> >> On Mon, 29 Oct 2018 09:48:57 -0700 >> Moritz Fischer mdf@kernel.org wrote: >> ... >> >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 > >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
On Thu, Nov 1, 2018 at 1:33 PM Anatolij Gustschin <agust@denx.de> wrote: > > On Thu, 1 Nov 2018 13:21:00 -0500 > Alan Tull atull@kernel.org wrote: > > >On Tue, Oct 30, 2018 at 6:19 AM Anatolij Gustschin <agust@denx.de> wrote: > >> > >> Hi Moritz, > >> > >> On Mon, 29 Oct 2018 09:48:57 -0700 > >> Moritz Fischer mdf@kernel.org wrote: > >> ... > >> >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 > > > >or > > > > /sys/class/fpga_manager/fpga0/of_node > > of_node isn't there, it is a non-dt platform. Oh yeah. > > # 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'? > > # 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
On Thu, 1 Nov 2018 13:46:27 -0500 Alan Tull atull@kernel.org wrote: ... >> # 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
Hi Anatolij, On Thu, Nov 1, 2018 at 12:23 PM Anatolij Gustschin <agust@denx.de> wrote: > > On Thu, 1 Nov 2018 13:46:27 -0500 > Alan Tull atull@kernel.org wrote: > ... > >> # 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 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
diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c index 469486be20c4..34b13d1c0f88 100644 --- a/drivers/fpga/xilinx-spi.c +++ b/drivers/fpga/xilinx-spi.c @@ -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;
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 <agust@denx.de> --- drivers/fpga/xilinx-spi.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)