fpga: mgr: altera-ps-spi: enable usage on non-dt platforms
PATCH v3 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 v3:
- don't use pointers to device data struct in driver_data
Changes in v2:
- removed not necessary braces {}
- dropped not needed !id check
drivers/fpga/altera-ps-spi.c | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-)
drivers/fpga/altera-ps-spi.c +31 -5
@@ -75,6 +75,11 @@ static struct altera_ps_data a10_data = { .t_st2ck_us = 10, /* min(t_ST2CK) */ }; +static const struct altera_ps_data *altera_ps_data_map[] = { + &c5_data, + &a10_data, +}; +static const struct of_device_id of_ef_match[] = { { .compatible ="altr,fpga-passive-serial" , .data = &c5_data }, { .compatible ="altr,fpga-arria10-passive-serial" , .data = &a10_data },
@@ -234,6 +239,19 @@ static const struct fpga_manager_ops altera_ps_ops = { .write_complete = altera_ps_write_complete, }; +static const struct altera_ps_data *id_to_data (const struct spi_device_id *id) +{ +const struct altera_ps_data *data; +unsigned int i; + +for (i =0 ; i <ARRAY_SIZE (altera_ps_data_map); i++) { + data = altera_ps_data_map[i]; +if (data && data->devtype == id->driver_data) +return data; + } +return NULL ; +} +static int altera_ps_probe (struct spi_device *spi) {struct altera_ps_conf *conf;
@@ -244,11 +262,17 @@ static int altera_ps_probe(struct spi_device *spi)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 { + conf->data =id_to_data (spi_get_device_id (spi)); +if (!conf->data) +return -ENODEV ; + } - 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 +318,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" ,CYCLONE5 }, + {"fpga-passive-serial" ,CYCLONE5 }, + {"fpga-arria10-passive-serial" ,ARRIA10 }, {} };MODULE_DEVICE_TABLE (spi, altera_ps_spi_ids);
Signature · 1 line
2.17.1
Hi Alan,
Quoted 10 lines · On Thu, 1 Nov 2018 22:22:31 +0100 Anatolij Gustschin [email protected] wrote:
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 v3:
- don't use pointers to device data struct indriver_data
Ping. Could this be queued for -next?
Thanks,
Anatolij
On Thu, Nov 1, 2018 at 4:22 PM Anatolij Gustschin <[email protected]> wrote:
Hi Anatolij,
Quoted 6 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).
Need to update the header to explain what has been added since v2 (added spi ids and an array and code to map spi ids to data).
Quoted 18 lines
Signed-off-by Anatolij Gustschin <[email protected]>---
Changes in v3:
- don't use pointers to device data struct indriver_dataChanges in v2:
- removed not necessary braces {}
- dropped not needed !id checkdrivers/fpga/altera-ps-spi.c | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-)drivers/fpga/altera-ps-spi.c +0
-0
drivers/fpga/altera-ps-spi.c static struct altera_ps_data a10_data = { · line 78
Quoted 4 lines
drivers/fpga/altera-ps-spi.c +1
-0@@ -75,6 +75,11 @@ static struct altera_ps_data a10_data = { .t_st2ck_us = 10, /* min(t_ST2CK) */ }; +static const struct altera_ps_data *altera_ps_data_map[] = {
Please add a comment that array index is enum altera_ps_devtype or something like that. Make it easy for future generations to do the right thing. :)
Quoted 8 lines
drivers/fpga/altera-ps-spi.c +4
-0+ &c5_data, + &a10_data, +}; +static const struct of_device_id of_ef_match[] = { { .compatible ="altr,fpga-passive-serial" , .data = &c5_data }, { .compatible = "altr,fpga-arria10-passive-serial", .data = &a10_data },
drivers/fpga/altera-ps-spi.c static const struct fpga_manager_ops altera_ps_ops = { · line 247
Quoted 9 lines
drivers/fpga/altera-ps-spi.c +6
-0@@ -234,6 +239,19 @@ static const struct fpga_manager_ops altera_ps_ops = { .write_complete = altera_ps_write_complete, }; +static const struct altera_ps_data *id_to_data (const struct spi_device_id *id) +{ +const struct altera_ps_data *data; +unsigned int i; + +for (i =0 ; i <ARRAY_SIZE (altera_ps_data_map); i++) {
At this point, id->driver_data == CYCLONE5 or ARRIA10 (more added in the future) so we don't need a loop to search the mapping array for a
matching id. Would something like the following work instead of
this for loop? I understand you wanted to do some checking, this accomplishes some checking but will exit NULL if someone extends the array wrongly.
kernel_ulong_t devtype = id->driver_data;
/* someone added a altera_ps_devtype without adding to the map array */ if (devtype >= ARRAY_SIZE(altera_ps_data_map))
return NULL;
data = altera_ps_data_map[devtype];
if (!data || data->devtype != devtype)
return NULL;
return data;
Quoted 44 lines
drivers/fpga/altera-ps-spi.c +20
-6+ data = altera_ps_data_map[i]; +if (data && data->devtype == id->driver_data) +return data; + } +return NULL ; +} +static int altera_ps_probe (struct spi_device *spi) {struct altera_ps_conf *conf;@@ -244,11 +262,17 @@ static int altera_ps_probe(struct spi_device *spi)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 { + conf->data =id_to_data (spi_get_device_id (spi)); +if (!conf->data) +return -ENODEV ; + } - 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 +318,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" ,CYCLONE5 }, + {"fpga-passive-serial" ,CYCLONE5 }, + {"fpga-arria10-passive-serial" ,ARRIA10 }, {} };MODULE_DEVICE_TABLE (spi, altera_ps_spi_ids); --
Quoted 1 line
2.17.1
Thanks,
Alan
Quoted 1 line
Hi Alan,
On Thu, 15 Nov 2018 16:24:21 -0600
Alan Tull [email protected] wrote:
...
Quoted 2 lines
Need to update the header to explain what has been added since v2 (added spi ids and an array and code to map spi ids to data).
Done in v4.
...
Quoted 5 lines
Quoted 1 line
+static const struct
altera_ps_data*altera_ps_data_map[] = {Please add a comment that array index is enum
altera_ps_devtypeor something like that. Make it easy for future generations to do the right thing. :)
Done in v4.
...
Quoted 5 lines
Quoted 5 lines
drivers/fpga/altera-ps-spi.c +5
-0+static const struct altera_ps_data *id_to_data (const struct spi_device_id *id) +{ +const struct altera_ps_data *data; +unsigned int i; +
Quoted 8 lines
Quoted 1 line
+ for (i = 0; i <
ARRAY_SIZE(altera_ps_data_map); i++) {At this point,
id->driver_data== CYCLONE5 or ARRIA10 (more added in the future) so we don't need a loop to search the mapping array for a
matching id. Would something like the following work instead of
this for loop? I understand you wanted to do some checking, this accomplishes some checking but will exit NULL if someone extends the array wrongly.
Yes, this works.
Quoted 11 lines
kernel_ulong_tdevtype =id->driver_data;/* someone added a
altera_ps_devtypewithout adding to the map array */ if (devtype >=ARRAY_SIZE(altera_ps_data_map))
return NULL;data = altera_ps_data_map[devtype];
if (!data ||data->devtype!= devtype)
return NULL;return data;
Okay, v4 uses this code.
Thanks,
Anatolij