spi: efm32: Clean up non-DT paths

PATCH Superseded Axel Lin 2 participants 4 messages Mar 10, 2014 – Mar 11, 2014 2 messages not in the archive SPI

lore

Share & tools

lore

Axel Lin

author

2014-03-10 00:01 UTC #0 lore raw

PATCH 1/1 spi: efm32: Clean up non-DT paths

This is a DT-only driver, so remove all non-DT paths.

Signed-off-by Axel Lin <[email protected]>

---
 drivers/spi/spi-efm32.c | 28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

drivers/spi/spi-efm32.c +7 ~~-21~~

index f53bbea..26e0362 100644
@@ -294,9 +294,6 @@ static int efm32_spi_probe_dt(struct platform_device *pdev,
 	u32 location;
 	int ret;
 
-	if (!np)
-		return 1;
-
 	ret = of_property_read_u32(np, "location", &location);
 	if (!ret) {
 		dev_dbg(&pdev->dev, "using location %u\n", location);

@@ -318,9 +315,14 @@ static int efm32_spi_probe(struct platform_device *pdev)
 	int ret;
 	struct spi_master *master;
 	struct device_node *np = pdev->dev.of_node;
-	unsigned int num_cs, i;
+	int num_cs, i;
+
+	if (!np)
+		return -EINVAL;
 
 	num_cs = of_gpio_named_count(np, "cs-gpios");
+	if (num_cs < 0)
+		return num_cs;
 
 	master = spi_alloc_master(&pdev->dev,
 			sizeof(*ddata) + num_cs * sizeof(unsigned));

@@ -412,23 +414,7 @@ static int efm32_spi_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	ret = efm32_spi_probe_dt(pdev, master, ddata);
-	if (ret > 0) {
-		/* not created by device tree */
-		const struct efm32_spi_pdata *pdata =
-			dev_get_platdata(&pdev->dev);
-
-		if (pdata)
-			ddata->pdata = *pdata;
-		else
-			ddata->pdata.location =
-				efm32_spi_get_configured_location(ddata);
-
-		master->bus_num = pdev->id;
-
-	} else if (ret < 0) {
-		goto err_disable_clk;
-	}
+	efm32_spi_probe_dt(pdev, master, ddata);
 
 	efm32_spi_write32(ddata, 0, REG_IEN);
 	efm32_spi_write32(ddata, REG_ROUTE_TXPEN | REG_ROUTE_RXPEN |

Signature · 8 lines

1.8.1.2



--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Uwe Kleine-König

2014-03-11 09:09 UTC #1 lore raw

Hi Axel,

Quoted 1 line · Axel Lin

This is a DT-only driver, so remove all non-DT paths.

ok in general, but I have a few comments, see below.

Quoted 22 lines

Signed-off-by Axel Lin <[email protected]>

---
 drivers/spi/spi-efm32.c | 28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

drivers/spi/spi-efm32.c +0 -3

index f53bbea..26e0362 100644
@@ -294,9 +294,6 @@ static int efm32_spi_probe_dt(struct platform_device *pdev,
 	u32 location;
 	int ret;
 
-	if (!np)
-		return 1;
-
 	ret = of_property_read_u32(np, "location", &location);
 	if (!ret) {
 		dev_dbg(&pdev->dev, "using location %u\n", location);

@@ -318,9 +315,14 @@ static int efm32_spi_probe(struct platform_device *pdev)
 	int ret;
 	struct spi_master *master;

drivers/spi/spi-efm32.c static int efm32_spi_probe(struct platform_device *pdev) · line 325

Quoted 10 lines

drivers/spi/spi-efm32.c +6 -1

 	struct device_node *np = pdev->dev.of_node;
-	unsigned int num_cs, i;
+	int num_cs, i;
+
+	if (!np)
+		return -EINVAL;
 
 	num_cs = of_gpio_named_count(np, "cs-gpios");
+	if (num_cs < 0)
+		return num_cs;

This wasn't checked before and doesn't fit into the "cleanup non-DT paths"-category. Maybe note that in the commit log?
Also did you change the type of i on purpose?

Quoted 14 lines

drivers/spi/spi-efm32.c +0 -8

 	master = spi_alloc_master(&pdev->dev,
 			sizeof(*ddata) + num_cs * sizeof(unsigned));

@@ -412,23 +414,7 @@ static int efm32_spi_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	ret = efm32_spi_probe_dt(pdev, master, ddata);
-	if (ret > 0) {
-		/* not created by device tree */
-		const struct efm32_spi_pdata *pdata =
-			dev_get_platdata(&pdev->dev);
-
-		if (pdata)
-			ddata->pdata = *pdata;

drivers/spi/spi-efm32.c static int efm32_spi_probe(struct platform_device *pdev) · line 417

Quoted 10 lines

drivers/spi/spi-efm32.c +1 -9

-		else
-			ddata->pdata.location =
-				efm32_spi_get_configured_location(ddata);
-
-		master->bus_num = pdev->id;
-
-	} else if (ret < 0) {
-		goto err_disable_clk;
-	}
+	efm32_spi_probe_dt(pdev, master, ddata);

This must still be:

	ret = efm32_spi_probe_dt(pdev, master, ddata);
	if (ret < 0)
		goto err_disable_clk;

Best regards
Uwe

Signature · 6 lines

Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Axel Lin

author

2014-03-11 10:33 UTC #3 lore raw

In reply to a message not in the archive

2014-03-11 17:09 GMT+08:00 Uwe Kleine-König <[email protected]>:

Quoted 35 lines

Hi Axel,

Quoted 1 line · Axel Lin

This is a DT-only driver, so remove all non-DT paths.

ok in general, but I have a few comments, see below.

Quoted 29 lines

Signed-off-by Axel Lin <[email protected]>

---
 drivers/spi/spi-efm32.c | 28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

drivers/spi/spi-efm32.c +4 -4

index f53bbea..26e0362 100644
@@ -294,9 +294,6 @@ static int efm32_spi_probe_dt(struct platform_device *pdev,
      u32 location;
      int ret;
 
-     if (!np)
-             return 1;
-
      ret = of_property_read_u32(np, "location", &location);
      if (!ret) {
              dev_dbg(&pdev->dev, "using location %u\n", location);

@@ -318,9 +315,14 @@ static int efm32_spi_probe(struct platform_device *pdev)
      int ret;
      struct spi_master *master;
      struct device_node *np = pdev->dev.of_node;
-     unsigned int num_cs, i;
+     int num_cs, i;
+
+     if (!np)
+             return -EINVAL;

Quoted 5 lines

drivers/spi/spi-efm32.c static int efm32_spi_probe(struct platform_device *pdev) · line 325
Quoted 3 lines
drivers/spi/spi-efm32.c +2 -0
      num_cs = of_gpio_named_count(np, "cs-gpios");
+     if (num_cs < 0)
+             return num_cs;
This wasn't checked before and doesn't fit into the "cleanup non-DT paths"-category. Maybe note that in the commit log?

Ok, will update commit log in v2.

Quoted 1 line

Also did you change the type of i on purpose?

It's a "doesn't matter" case to me.
If you really like to keep i as unsigned int, I can leave it as unsigned int in v2.

Quoted 18 lines

drivers/spi/spi-efm32.c +0 ~~-11~~

      master = spi_alloc_master(&pdev->dev,
                      sizeof(*ddata) + num_cs * sizeof(unsigned));

@@ -412,23 +414,7 @@ static int efm32_spi_probe(struct platform_device *pdev)
              goto err;
      }
 
-     ret = efm32_spi_probe_dt(pdev, master, ddata);
-     if (ret > 0) {
-             /* not created by device tree */
-             const struct efm32_spi_pdata *pdata =
-                     dev_get_platdata(&pdev->dev);
-
-             if (pdata)
-                     ddata->pdata = *pdata;
-             else
-                     ddata->pdata.location =
-                             efm32_spi_get_configured_location(ddata);

Quoted 8 lines

drivers/spi/spi-efm32.c static int efm32_spi_probe(struct platform_device *pdev) · line 417
Quoted 7 lines
drivers/spi/spi-efm32.c +1 -6
-
-             master->bus_num = pdev->id;
-
-     } else if (ret < 0) {
-             goto err_disable_clk;
-     }
+     efm32_spi_probe_dt(pdev, master, ddata);
This must still be:

efm32_spi_probe_dt() always return 0, why *must* check the return value here?

Quoted 11 lines

        ret = efm32_spi_probe_dt(pdev, master, ddata);
        if (ret < 0)
                goto err_disable_clk;
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Uwe Kleine-König

2014-03-11 18:26 UTC #5 lore raw

In reply to a message not in the archive

Hello Axel,

Quoted 21 lines · Axel Lin

2014-03-11 17:09 GMT+08:00 Uwe Kleine-König <[email protected]>:
Quoted 15 lines · Axel Lin
Quoted 14 lines
drivers/spi/spi-efm32.c +4 -1
index f53bbea..26e0362 100644
@@ -318,9 +315,14 @@ static int efm32_spi_probe(struct platform_device *pdev)
      int ret;
      struct spi_master *master;
      struct device_node *np = pdev->dev.of_node;
-     unsigned int num_cs, i;
+     int num_cs, i;
+
+     if (!np)
+             return -EINVAL;
 
Quoted 5 lines
drivers/spi/spi-efm32.c static int efm32_spi_probe(struct platform_device *pdev) · line 325
Quoted 3 lines
drivers/spi/spi-efm32.c +2 -0
      num_cs = of_gpio_named_count(np, "cs-gpios");
+     if (num_cs < 0)
+             return num_cs;
This wasn't checked before and doesn't fit into the "cleanup non-DT paths"-category. Maybe note that in the commit log?

Quoted 6 lines

Ok, will update commit log in v2.
Quoted 1 line
Also did you change the type of i on purpose?
It's a "doesn't matter" case to me.
If you really like to keep i as unsigned int, I can leave it as unsigned int in v2.

I'd always use an unsigned type for it, but I don't care much.

Quoted 25 lines

Quoted 17 lines

drivers/spi/spi-efm32.c +0 ~~-11~~

      master = spi_alloc_master(&pdev->dev,
                      sizeof(*ddata) + num_cs * sizeof(unsigned));

@@ -412,23 +414,7 @@ static int efm32_spi_probe(struct platform_device *pdev)
              goto err;
      }
 
-     ret = efm32_spi_probe_dt(pdev, master, ddata);
-     if (ret > 0) {
-             /* not created by device tree */
-             const struct efm32_spi_pdata *pdata =
-                     dev_get_platdata(&pdev->dev);
-
-             if (pdata)
-                     ddata->pdata = *pdata;
-             else
-                     ddata->pdata.location =
-                             efm32_spi_get_configured_location(ddata);

Quoted 8 lines

drivers/spi/spi-efm32.c static int efm32_spi_probe(struct platform_device *pdev) · line 417
Quoted 7 lines
drivers/spi/spi-efm32.c +1 -6
-
-             master->bus_num = pdev->id;
-
-     } else if (ret < 0) {
-             goto err_disable_clk;
-     }
+     efm32_spi_probe_dt(pdev, master, ddata);
This must still be:

Quoted 1 line

efm32_spi_probe_dt() always return 0, why *must* check the return value here?

Then also change efm32_spi_probe_dt to return void, or now that platform device support is gone, inline it into efm32_spi_probe.

Best regards
Uwe

Signature · 6 lines

Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html