fpga: mgr: altera-ps-spi: enable usage on non-dt platforms
PATCH v2 1/1 fpga: mgr: altera-ps-spi: enable usage on non-dt platforms
Driver probing fails on non-dt platforms since of_match_device() always returns NULL here. Add device names and matching driver data to the spi_device_id table. This allows driver binding to dynamically added PS-SPI devices (e.g. added via spi_new_device() after hot-plugging).
---
Changes in v2:
- removed not necessary braces {}
- dropped not needed !id check
drivers/fpga/altera-ps-spi.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
drivers/fpga/altera-ps-spi.c +13 -5
@@ -238,17 +238,23 @@ static int altera_ps_probe(struct spi_device *spi) {struct altera_ps_conf *conf;const struct of_device_id *of_id; +const struct spi_device_id *id;struct fpga_manager *mgr; conf =devm_kzalloc (&spi->dev,sizeof (*conf),GFP_KERNEL );if (!conf)return -ENOMEM ; - of_id =of_match_device (of_ef_match, &spi->dev); -if (!of_id) -return -ENODEV ; +if (spi->dev.of_node) { + of_id =of_match_device (of_ef_match, &spi->dev); +if (!of_id) +return -ENODEV ; + conf->data = of_id->data; + }else { + id =spi_get_device_id (spi); + conf->data = (struct altera_ps_data *)id->driver_data; + } - conf->data = of_id->data; conf->spi = spi; conf->config =devm_gpiod_get (&spi->dev,"nconfig" ,GPIOD_OUT_LOW );if (IS_ERR (conf->config)) {
@@ -294,7 +300,9 @@ static int altera_ps_remove(struct spi_device *spi) }static const struct spi_device_id altera_ps_spi_ids[] = { - {"cyclone-ps-spi" ,0 }, + {"cyclone-ps-spi" , (kernel_ulong_t)&c5_data}, + {"fpga-passive-serial" , (kernel_ulong_t)&c5_data}, + {"fpga-arria10-passive-serial" , (kernel_ulong_t)&a10_data}, {} };MODULE_DEVICE_TABLE (spi, altera_ps_spi_ids);
Signature · 1 line
2.17.1
On Mon, Oct 29, 2018 at 9:17 AM Anatolij Gustschin <[email protected]> wrote:
Hi Anatolij,
Quoted 49 lines
Driver probing fails on non-dt platforms since
of_match_device()always returns NULL here. Add device names and matching driver data to thespi_device_idtable. This allows driver binding to dynamically added PS-SPI devices (e.g. added viaspi_new_device()after hot-plugging).Signed-off-by Anatolij Gustschin <[email protected]>---
Changes in v2:
- removed not necessary braces {}
- dropped not needed !id checkdrivers/fpga/altera-ps-spi.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)drivers/fpga/altera-ps-spi.c +10
-4@@ -238,17 +238,23 @@ static int altera_ps_probe(struct spi_device *spi) {struct altera_ps_conf *conf;const struct of_device_id *of_id; +const struct spi_device_id *id;struct fpga_manager *mgr; conf =devm_kzalloc (&spi->dev,sizeof (*conf),GFP_KERNEL );if (!conf)return -ENOMEM ; - of_id =of_match_device (of_ef_match, &spi->dev); -if (!of_id) -return -ENODEV ; +if (spi->dev.of_node) { + of_id =of_match_device (of_ef_match, &spi->dev); +if (!of_id) +return -ENODEV ; + conf->data = of_id->data; + }else { + id =spi_get_device_id (spi); + conf->data = (struct altera_ps_data *)id->driver_data; + } - conf->data = of_id->data; conf->spi = spi; conf->config =devm_gpiod_get (&spi->dev,"nconfig" ,GPIOD_OUT_LOW );if (IS_ERR (conf->config)) {
drivers/fpga/altera-ps-spi.c static int altera_ps_remove(struct spi_device *spi) · line 305
Quoted 7 lines
drivers/fpga/altera-ps-spi.c +3
-1@@ -294,7 +300,9 @@ static int altera_ps_remove(struct spi_device *spi) }static const struct spi_device_id altera_ps_spi_ids[] = { - {"cyclone-ps-spi" ,0 }, + {"cyclone-ps-spi" , (kernel_ulong_t)&c5_data}, + {"fpga-passive-serial" , (kernel_ulong_t)&c5_data}, + {"fpga-arria10-passive-serial" , (kernel_ulong_t)&a10_data},
I don't think this should be implemented as a pointer. This would be easy if driver_data were void* but it's a value that's not a pointer.
I suggest using driver_data as a index to an array of pointers to the
structs instead.
Thanks,
Alan
Quoted 6 lines
drivers/fpga/altera-ps-spi.c +0
-1{} };MODULE_DEVICE_TABLE (spi, altera_ps_spi_ids); --2.17.1
Hi Alan,
On Thu, 1 Nov 2018 14:37:02 -0500
Alan Tull [email protected] wrote:
...
Quoted 10 lines
drivers/fpga/altera-ps-spi.c static int altera_ps_remove(struct spi_device *spi) · line 305
Quoted 5 lines
static const struct
spi_device_idaltera_ps_spi_ids[] = {drivers/fpga/altera-ps-spi.c +3
-1- {"cyclone-ps-spi", 0}, + {"cyclone-ps-spi", (kernel_ulong_t)&c5_data}, + {"fpga-passive-serial", (kernel_ulong_t)&c5_data}, + {"fpga-arria10-passive-serial", (kernel_ulong_t)&a10_data},I don't think this should be implemented as a pointer. This would be easy if
driver_datawere void* but it's a value that's not a pointer. I suggest usingdriver_dataas a index to an array of pointers to the structs instead.
Thanks for review. I've sent v3 using array of pointers. It uses the FPGA type in driver_data. It could be used as an index to the array of pointers, but I'd prefer checking for this type explicitly, so it will work even if the array is wrongly extended or reordered by future changes.
Thanks,
Anatolij