Skip to contentSkip to conversation

dm: spi: Read default speed, mode values from DT

PATCH New Vignesh R 3 participants 5 messages Apr 7, 2016 – Apr 11, 2016 SPI

lore
Share & tools
lore

Vignesh R

author
#0 lore raw

PATCH 1/1 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.

---
 cmd/sf.c                 | 2 ++
 drivers/spi/spi-uclass.c | 8 ++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)
cmd/sf.c +2 -0
index 42862d9d921a..4c40d5104422 100644
@@ -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
drivers/spi/spi-uclass.c +6 -2
index 5561f36762f9..55ebb5ef19b7 100644
@@ -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;
Signature · 1 line
2.8.1
#1 lore raw

PATCH 1/1 dm: spi: Read default speed, mode values from DT

Quoted 47 lines · Vignesh R

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.

---
 cmd/sf.c                 | 2 ++
 drivers/spi/spi-uclass.c | 8 ++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)
cmd/sf.c +2 -0
index 42862d9d921a..4c40d5104422 100644
@@ -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
drivers/spi/spi-uclass.c +1 -2
index 5561f36762f9..55ebb5ef19b7 100644
@@ -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;

drivers/spi/spi-uclass.c int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, · line 317

Quoted 10 lines
drivers/spi/spi-uclass.c +5 -0
 	}
 
+	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;

I like the concept but if it can fail, passing -1 for speed/mode seems like a bad idea and we should just fail.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160408/c0d38710/attachment.sig>
author
#2 lore raw

PATCH 1/1 dm: spi: Read default speed, mode values from DT

Quoted 51 lines · Tom Rini
Quoted 49 lines · Vignesh R

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.

---
 cmd/sf.c                 | 2 ++
 drivers/spi/spi-uclass.c | 8 ++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)
cmd/sf.c +2 -0
index 42862d9d921a..4c40d5104422 100644
@@ -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
drivers/spi/spi-uclass.c +1 -2
index 5561f36762f9..55ebb5ef19b7 100644
@@ -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;
 	}
 
Quoted 12 lines

drivers/spi/spi-uclass.c int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, · line 317

Quoted 8 lines
drivers/spi/spi-uclass.c +5 -0
+	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;

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.

Signature · 2 lines
Regards
Vignesh
#3 lore raw

PATCH 1/1 dm: spi: Read default speed, mode values from DT

Quoted 16 lines · Vignesh R
Quoted 13 lines · Tom Rini
Quoted 11 lines · Vignesh R

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.

---
 cmd/sf.c                 | 2 ++
 drivers/spi/spi-uclass.c | 8 ++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)
Quoted 10 lines
Quoted 10 lines

cmd/sf.c static int do_spi_flash_probe(int argc, char * const argv[]) · line 92

Quoted 10 lines
cmd/sf.c +2 -0
index 42862d9d921a..4c40d5104422 100644
@@ -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

Quoted 48 lines
Quoted 28 lines
Quoted 28 lines
cmd/sf.c +0 -0
 #else
 	struct spi_flash *new;
 #endif
drivers/spi/spi-uclass.c +1 -2
index 5561f36762f9..55ebb5ef19b7 100644
@@ -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;
 	}
 
Quoted 12 lines

drivers/spi/spi-uclass.c int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, · line 317

Quoted 8 lines
drivers/spi/spi-uclass.c +5 -0
+	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;

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.

author
#4 lore raw

PATCH 1/1 dm: spi: Read default speed, mode values from DT

Quoted 17 lines · Mugunthan V N
Quoted 16 lines · Vignesh R
Quoted 13 lines · Tom Rini
Quoted 11 lines · Vignesh R

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.

---
 cmd/sf.c                 | 2 ++
 drivers/spi/spi-uclass.c | 8 ++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)
Quoted 13 lines
Quoted 10 lines
Quoted 10 lines

cmd/sf.c static int do_spi_flash_probe(int argc, char * const argv[]) · line 92

Quoted 10 lines
cmd/sf.c +2 -0
index 42862d9d921a..4c40d5104422 100644
@@ -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.

Signature · 2 lines
Regards
Vignesh