[U-Boot] [PATCH] dm: spi: Read default speed, mode values from DT

In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node. This will make sure that boards with multiple SPI/QSPI controllers can be probed at different bus frequencies and modes.
Signed-off-by: Vignesh R vigneshr@ti.com --- cmd/sf.c | 2 ++ drivers/spi/spi-uclass.c | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c index 42862d9d921a..4c40d5104422 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -88,6 +88,8 @@ static int do_spi_flash_probe(int argc, char * const argv[]) #ifdef CONFIG_DM_SPI_FLASH struct udevice *new, *bus_dev; int ret; + /* In DM mode defaults wiil be taken from DT */ + speed = -1, mode = -1; #else struct spi_flash *new; #endif diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c index 5561f36762f9..55ebb5ef19b7 100644 --- a/drivers/spi/spi-uclass.c +++ b/drivers/spi/spi-uclass.c @@ -264,6 +264,7 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, struct udevice **busp, struct spi_slave **devp) { struct udevice *bus, *dev; + struct dm_spi_slave_platdata *plat; bool created = false; int ret;
@@ -280,8 +281,6 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, * SPI flash chip - we will bind to the correct driver. */ if (ret == -ENODEV && drv_name) { - struct dm_spi_slave_platdata *plat; - debug("%s: Binding new device '%s', busnum=%d, cs=%d, driver=%s\n", __func__, dev_name, busnum, cs, drv_name); ret = device_bind_driver(bus, drv_name, dev_name, &dev); @@ -308,6 +307,11 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, slave->dev = dev; }
+ plat = dev_get_parent_platdata(dev); + if (speed == -1) + speed = plat->max_hz; + if (mode == -1) + mode = plat->mode; ret = spi_set_speed_mode(bus, speed, mode); if (ret) goto err;

On Thu, Apr 07, 2016 at 09:02:26PM +0530, Vignesh R wrote:
In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node. This will make sure that boards with multiple SPI/QSPI controllers can be probed at different bus frequencies and modes.
Signed-off-by: Vignesh R vigneshr@ti.com
cmd/sf.c | 2 ++ drivers/spi/spi-uclass.c | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c index 42862d9d921a..4c40d5104422 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -88,6 +88,8 @@ static int do_spi_flash_probe(int argc, char * const argv[]) #ifdef CONFIG_DM_SPI_FLASH struct udevice *new, *bus_dev; int ret;
- /* In DM mode defaults wiil be taken from DT */
- speed = -1, mode = -1;
#else struct spi_flash *new; #endif diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c index 5561f36762f9..55ebb5ef19b7 100644 --- a/drivers/spi/spi-uclass.c +++ b/drivers/spi/spi-uclass.c @@ -264,6 +264,7 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, struct udevice **busp, struct spi_slave **devp) { struct udevice *bus, *dev;
- struct dm_spi_slave_platdata *plat; bool created = false; int ret;
@@ -280,8 +281,6 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, * SPI flash chip - we will bind to the correct driver. */ if (ret == -ENODEV && drv_name) {
struct dm_spi_slave_platdata *plat;
- debug("%s: Binding new device '%s', busnum=%d, cs=%d, driver=%s\n", __func__, dev_name, busnum, cs, drv_name); ret = device_bind_driver(bus, drv_name, dev_name, &dev);
@@ -308,6 +307,11 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, slave->dev = dev; }
- plat = dev_get_parent_platdata(dev);
- if (speed == -1)
speed = plat->max_hz;
- if (mode == -1)
ret = spi_set_speed_mode(bus, speed, mode); if (ret) goto err;mode = plat->mode;
I like the concept but if it can fail, passing -1 for speed/mode seems like a bad idea and we should just fail.

On 04/09/2016 01:15 AM, Tom Rini wrote:
On Thu, Apr 07, 2016 at 09:02:26PM +0530, Vignesh R wrote:
In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node. This will make sure that boards with multiple SPI/QSPI controllers can be probed at different bus frequencies and modes.
Signed-off-by: Vignesh R vigneshr@ti.com
cmd/sf.c | 2 ++ drivers/spi/spi-uclass.c | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c index 42862d9d921a..4c40d5104422 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -88,6 +88,8 @@ static int do_spi_flash_probe(int argc, char * const argv[]) #ifdef CONFIG_DM_SPI_FLASH struct udevice *new, *bus_dev; int ret;
- /* In DM mode defaults wiil be taken from DT */
- speed = -1, mode = -1;
#else struct spi_flash *new; #endif diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c index 5561f36762f9..55ebb5ef19b7 100644 --- a/drivers/spi/spi-uclass.c +++ b/drivers/spi/spi-uclass.c @@ -264,6 +264,7 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, struct udevice **busp, struct spi_slave **devp) { struct udevice *bus, *dev;
- struct dm_spi_slave_platdata *plat; bool created = false; int ret;
@@ -280,8 +281,6 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, * SPI flash chip - we will bind to the correct driver. */ if (ret == -ENODEV && drv_name) {
struct dm_spi_slave_platdata *plat;
- debug("%s: Binding new device '%s', busnum=%d, cs=%d, driver=%s\n", __func__, dev_name, busnum, cs, drv_name); ret = device_bind_driver(bus, drv_name, dev_name, &dev);
@@ -308,6 +307,11 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, slave->dev = dev; }
- plat = dev_get_parent_platdata(dev);
- if (speed == -1)
speed = plat->max_hz;
- if (mode == -1)
ret = spi_set_speed_mode(bus, speed, mode); if (ret) goto err;mode = plat->mode;
I like the concept but if it can fail, passing -1 for speed/mode seems like a bad idea and we should just fail.
I didn't quite understand your comment. Could you explain what can fail? I don't think mode/speed will be -1 when calling spi_set_speed_mode() after above change. speed/mode will either be command line arg passed to sf probe cmd or CONFIG_SF_DEFAULT_* in case of non DM driver or mode/speed specified in DT node in case of DM.

On Monday 11 April 2016 10:12 AM, Vignesh R wrote:
On 04/09/2016 01:15 AM, Tom Rini wrote:
On Thu, Apr 07, 2016 at 09:02:26PM +0530, Vignesh R wrote:
In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node. This will make sure that boards with multiple SPI/QSPI controllers can be probed at different bus frequencies and modes.
Signed-off-by: Vignesh R vigneshr@ti.com
cmd/sf.c | 2 ++ drivers/spi/spi-uclass.c | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c index 42862d9d921a..4c40d5104422 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -88,6 +88,8 @@ static int do_spi_flash_probe(int argc, char * const argv[]) #ifdef CONFIG_DM_SPI_FLASH struct udevice *new, *bus_dev; int ret;
- /* In DM mode defaults wiil be taken from DT */
- speed = -1, mode = -1;
speed and mode are uint and it is assigned with -1 which is not making sense. Assigning 0 will be a better option.
Regards Mugunthan V N
#else struct spi_flash *new; #endif diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c index 5561f36762f9..55ebb5ef19b7 100644 --- a/drivers/spi/spi-uclass.c +++ b/drivers/spi/spi-uclass.c @@ -264,6 +264,7 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, struct udevice **busp, struct spi_slave **devp) { struct udevice *bus, *dev;
- struct dm_spi_slave_platdata *plat; bool created = false; int ret;
@@ -280,8 +281,6 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, * SPI flash chip - we will bind to the correct driver. */ if (ret == -ENODEV && drv_name) {
struct dm_spi_slave_platdata *plat;
- debug("%s: Binding new device '%s', busnum=%d, cs=%d, driver=%s\n", __func__, dev_name, busnum, cs, drv_name); ret = device_bind_driver(bus, drv_name, dev_name, &dev);
@@ -308,6 +307,11 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, slave->dev = dev; }
- plat = dev_get_parent_platdata(dev);
- if (speed == -1)
speed = plat->max_hz;
- if (mode == -1)
ret = spi_set_speed_mode(bus, speed, mode); if (ret) goto err;mode = plat->mode;
I like the concept but if it can fail, passing -1 for speed/mode seems like a bad idea and we should just fail.
I didn't quite understand your comment. Could you explain what can fail? I don't think mode/speed will be -1 when calling spi_set_speed_mode() after above change. speed/mode will either be command line arg passed to sf probe cmd or CONFIG_SF_DEFAULT_* in case of non DM driver or mode/speed specified in DT node in case of DM.

On 04/11/2016 11:18 AM, Mugunthan V N wrote:
On Monday 11 April 2016 10:12 AM, Vignesh R wrote:
On 04/09/2016 01:15 AM, Tom Rini wrote:
On Thu, Apr 07, 2016 at 09:02:26PM +0530, Vignesh R wrote:
In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node. This will make sure that boards with multiple SPI/QSPI controllers can be probed at different bus frequencies and modes.
Signed-off-by: Vignesh R vigneshr@ti.com
cmd/sf.c | 2 ++ drivers/spi/spi-uclass.c | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c index 42862d9d921a..4c40d5104422 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -88,6 +88,8 @@ static int do_spi_flash_probe(int argc, char * const argv[]) #ifdef CONFIG_DM_SPI_FLASH struct udevice *new, *bus_dev; int ret;
- /* In DM mode defaults wiil be taken from DT */
- speed = -1, mode = -1;
speed and mode are uint and it is assigned with -1 which is not making sense. Assigning 0 will be a better option.
Ah, thanks! I will fix this up in the next version.
participants (3)
-
Mugunthan V N
-
Tom Rini
-
Vignesh R