[PATCH v2 0/6] of, of_gpio, of_spi: Bugfix and improve of_parse_phandle_with_args, of_gpio_named_count and of_spi_register_master

[PATCH v2 0/6] of, of_gpio, of_spi: Bugfix and improve of_parse_phandle_with_args, of_gpio_named_count and of_spi_register_master

[Andreas Larsson]

This patch series fixes a bug where of_gpio_named count relied upon a return
value that was no longer returned from of_parse_phandle_with_args and adds the
possibility for of_gpio_named_count to return error values.

In addition, for of_spi_register_master it fixes a bug, adds documentation,
adds fetching of gpio flags and initializes gpio values to be consistent with
return values from of_parse_phandle_with_args.

Tested on sparc (excluding the changes to drivers gpio-fan, i2c-mux-gpio,
matrix_keypad, mdio-mux-gpio, spi-mpc52xx, spi-oc-tiny, spi-ppc4xx, selftest). 
Compile tested on x86, arm and ppc (all changed source files, when appropriate
for platform)

Changes since v1:
- PATCH 2/6: Handle error return values from calls to of_gpio_count

Andreas Larsson (6):
  of: Return -EEXIST from of_parse_phandle_with_args for holes in
    phandle list
  of: Return -ENXIO from of_parse_phandle_with_args for too large index
    and return errors from of_gpio_named_count
  of_spi: Initialize cs_gpios properly
  of_spi: Document cs_gpios and cs_gpio in kernel-doc
  of_spi: Add fetching of of_gpio flags to of_spi_register_master
  of_spi: Initialize cs_gpios and cs_gpio with -EEXIST

 Documentation/devicetree/bindings/spi/spi-bus.txt |    3 +-
 drivers/gpio/gpiolib-of.c                         |    8 +++--
 drivers/hwmon/gpio-fan.c                          |    6 ++--
 drivers/i2c/muxes/i2c-mux-gpio.c                  |    3 +-
 drivers/input/keyboard/matrix_keypad.c            |    2 +-
 drivers/net/phy/mdio-mux-gpio.c                   |    2 +-
 drivers/of/base.c                                 |    9 +++--
 drivers/of/selftest.c                             |    2 +-
 drivers/spi/spi-fsl-spi.c                         |    4 ++-
 drivers/spi/spi-mpc52xx.c                         |    5 +++
 drivers/spi/spi-oc-tiny.c                         |    4 ++-
 drivers/spi/spi-ppc4xx.c                          |    6 +++-
 drivers/spi/spi.c                                 |   40 ++++++++++++++++-----
 include/linux/spi/spi.h                           |   10 +++++
 14 files changed, 78 insertions(+), 26 deletions(-)


------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnnow-d2d

[PATCH v2 1/6] of: Return -EEXIST from of_parse_phandle_with_args for holes in phandle list

[Andreas Larsson]

Return value for an empty phandle was -EEXIST before commit 15c9a0ac, that
changed the return value in this case to -ENOENT. However, of_gpio_named_count
relies upon the return value to be -EEXIST and relies upon being able to
distinguish this case from the case of no list at all which also returns
-ENOENT.

Also change the of selftest to expect -EEXIST in this case.

Signed-off-by: Andreas Larsson <andreas-FkzTOoA/JUlBDgjK7y7TUQ@public.gmane.org>
---

I have only compile tested the selftest, not having appropriate hardware around for running it.

 drivers/of/base.c     |    4 ++--
 drivers/of/selftest.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 2390ddb..986afd7 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1083,11 +1083,11 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
 		 * All of the error cases above bail out of the loop, so at
 		 * this point, the parsing is successful. If the requested
 		 * index matches, then fill the out_args structure and return,
-		 * or return -ENOENT for an empty entry.
+		 * or return -EEXIST for an empty entry.
 		 */
 		if (cur_index == index) {
 			if (!phandle)
-				return -ENOENT;
+				return -EEXIST;
 
 			if (out_args) {
 				int i;
diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
index f24ffd7..b1c2ae9 100644
--- a/drivers/of/selftest.c
+++ b/drivers/of/selftest.c
@@ -54,7 +54,7 @@ static void __init of_selftest_parse_phandle_with_args(void)
 			passed &= (args.args[1] == 0);
 			break;
 		case 2:
-			passed &= (rc == -ENOENT);
+			passed &= (rc == -EEXIST);
 			break;
 		case 3:
 			passed &= !rc;
-- 
1.7.0.4


------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnnow-d2d

[PATCH v2 2/6] of: Return -ENXIO from of_parse_phandle_with_args for too large index and return errors from of_gpio_named_count

[Andreas Larsson]

This lets of_gpio_named_count return an errno on errors by being able to
distinguish between reaching the end of the phandle list and getting some other
error from of_parse_phandle_with_args.

Return error from of_spi_register_master when there is an "cs-gpios" list for
which gp_gpio_named_count fails.

Adjust various drivers cope with error return from of_gpio_named_count,
including via of_gpio_count.

Signed-off-by: Andreas Larsson <andreas-FkzTOoA/JUlBDgjK7y7TUQ@public.gmane.org>
---
Changes since v1:
- Handle error return values from calls to of_gpio_count

 drivers/gpio/gpiolib-of.c              |    8 +++++---
 drivers/hwmon/gpio-fan.c               |    6 +++---
 drivers/i2c/muxes/i2c-mux-gpio.c       |    3 ++-
 drivers/input/keyboard/matrix_keypad.c |    2 +-
 drivers/net/phy/mdio-mux-gpio.c        |    2 +-
 drivers/of/base.c                      |    5 ++++-
 drivers/spi/spi-fsl-spi.c              |    4 +++-
 drivers/spi/spi-mpc52xx.c              |    5 +++++
 drivers/spi/spi-oc-tiny.c              |    4 +++-
 drivers/spi/spi-ppc4xx.c               |    6 +++++-
 drivers/spi/spi.c                      |   11 +++++++----
 11 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index d542a14..28f24a6 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -107,11 +107,10 @@ EXPORT_SYMBOL(of_get_named_gpio_flags);
  */
 unsigned int of_gpio_named_count(struct device_node *np, const char* propname)
 {
+	int ret;
 	unsigned int cnt = 0;
 
 	do {
-		int ret;
-
 		ret = of_parse_phandle_with_args(np, propname, "#gpio-cells",
 						 cnt, NULL);
 		/* A hole in the gpios = <> counts anyway. */
@@ -119,7 +118,10 @@ unsigned int of_gpio_named_count(struct device_node *np, const char* propname)
 			break;
 	} while (++cnt);
 
-	return cnt;
+	if (ret == -ENXIO)
+		return cnt;
+	else
+		return ret;
 }
 EXPORT_SYMBOL(of_gpio_named_count);
 
diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
index 4e04c12..9b92d34 100644
--- a/drivers/hwmon/gpio-fan.c
+++ b/drivers/hwmon/gpio-fan.c
@@ -422,8 +422,8 @@ static int gpio_fan_get_of_pdata(struct device *dev,
 
 	/* Fill GPIO pin array */
 	pdata->num_ctrl = of_gpio_count(node);
-	if (!pdata->num_ctrl) {
-		dev_err(dev, "gpios DT property empty / missing");
+	if (pdata->num_ctrl <= 0) {
+		dev_err(dev, "gpios DT property broken / empty / missing");
 		return -ENODEV;
 	}
 	ctrl = devm_kzalloc(dev, pdata->num_ctrl * sizeof(unsigned),
@@ -477,7 +477,7 @@ static int gpio_fan_get_of_pdata(struct device *dev,
 	pdata->speed = speed;
 
 	/* Alarm GPIO if one exists */
-	if (of_gpio_named_count(node, "alarm-gpios")) {
+	if (of_gpio_named_count(node, "alarm-gpios") > 0) {
 		struct gpio_fan_alarm *alarm;
 		int val;
 		enum of_gpio_flags flags;
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 9272743..a3ddb36 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -106,7 +106,8 @@ static int i2c_mux_gpio_probe_dt(struct gpiomux *mux,
 
 	mux->data.n_gpios = of_gpio_named_count(np, "mux-gpios");
 	if (mux->data.n_gpios < 0) {
-		dev_err(&pdev->dev, "Missing mux-gpios property in the DT.\n");
+		dev_err(&pdev->dev,
+			"Missing or broken mux-gpios property in the DT.\n");
 		return -EINVAL;
 	}
 
diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
index f4ff0dd..bc7cec5 100644
--- a/drivers/input/keyboard/matrix_keypad.c
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -418,7 +418,7 @@ matrix_keypad_parse_dt(struct device *dev)
 
 	pdata->num_row_gpios = of_gpio_named_count(np, "row-gpios");
 	pdata->num_col_gpios = of_gpio_named_count(np, "col-gpios");
-	if (!pdata->num_row_gpios || !pdata->num_col_gpios) {
+	if (pdata->num_row_gpios <= 0 || !pdata->num_col_gpios <= 0) {
 		dev_err(dev, "number of keypad rows/columns not specified\n");
 		return ERR_PTR(-EINVAL);
 	}
diff --git a/drivers/net/phy/mdio-mux-gpio.c b/drivers/net/phy/mdio-mux-gpio.c
index 0c9accb..3e1d285 100644
--- a/drivers/net/phy/mdio-mux-gpio.c
+++ b/drivers/net/phy/mdio-mux-gpio.c
@@ -61,7 +61,7 @@ static int mdio_mux_gpio_probe(struct platform_device *pdev)
 		return -ENODEV;
 
 	num_gpios = of_gpio_count(pdev->dev.of_node);
-	if (num_gpios == 0 || num_gpios > MDIO_MUX_GPIO_MAX_BITS)
+	if (num_gpios <= 0 || num_gpios > MDIO_MUX_GPIO_MAX_BITS)
 		return -ENODEV;
 
 	s = devm_kzalloc(&pdev->dev, sizeof(*s), GFP_KERNEL);
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 986afd7..1f16629 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1110,7 +1110,10 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
 	/* Loop exited without finding a valid entry; return an error */
 	if (node)
 		of_node_put(node);
-	return -EINVAL;
+	if (list == list_end)
+		return -ENXIO; /* Index beyond end of list */
+	else
+		return -EINVAL;
 }
 EXPORT_SYMBOL(of_parse_phandle_with_args);
 
diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c
index 1a7f635..9dcfeed 100644
--- a/drivers/spi/spi-fsl-spi.c
+++ b/drivers/spi/spi-fsl-spi.c
@@ -952,13 +952,15 @@ static int of_fsl_spi_get_chipselects(struct device *dev)
 	int ret;
 
 	ngpios = of_gpio_count(np);
-	if (!ngpios) {
+	if (!ngpios || ngpios == -ENOENT) {
 		/*
 		 * SPI w/o chip-select line. One SPI device is still permitted
 		 * though.
 		 */
 		pdata->max_chipselect = 1;
 		return 0;
+	} else if (ngpios < 0) {
+		return ngpios;
 	}
 
 	pinfo->gpios = kmalloc(ngpios * sizeof(*pinfo->gpios), GFP_KERNEL);
diff --git a/drivers/spi/spi-mpc52xx.c b/drivers/spi/spi-mpc52xx.c
index 29f7705..e945195 100644
--- a/drivers/spi/spi-mpc52xx.c
+++ b/drivers/spi/spi-mpc52xx.c
@@ -478,6 +478,11 @@ static int mpc52xx_spi_probe(struct platform_device *op)
 			gpio_direction_output(gpio_cs, 1);
 			ms->gpio_cs[i] = gpio_cs;
 		}
+	} else if (ms->gpio_cs_count < 0 && ms->gpio_cs_count != -ENOENT) {
+		dev_err(&op->dev,
+			"could not count the gpio field entries in oftree\n");
+		rc = ms->gpio_cs_count;
+		goto err_alloc_gpio;
 	}
 
 	spin_lock_init(&ms->lock);
diff --git a/drivers/spi/spi-oc-tiny.c b/drivers/spi/spi-oc-tiny.c
index 432e66e..d1a7151 100644
--- a/drivers/spi/spi-oc-tiny.c
+++ b/drivers/spi/spi-oc-tiny.c
@@ -254,12 +254,14 @@ static int tiny_spi_of_probe(struct platform_device *pdev)
 	if (!np)
 		return 0;
 	hw->gpio_cs_count = of_gpio_count(np);
-	if (hw->gpio_cs_count) {
+	if (hw->gpio_cs_count > 0) {
 		hw->gpio_cs = devm_kzalloc(&pdev->dev,
 				hw->gpio_cs_count * sizeof(unsigned int),
 				GFP_KERNEL);
 		if (!hw->gpio_cs)
 			return -ENOMEM;
+	} else if (hw->gpio_cs_count < 0 && hw->gpio_cs_count != -ENOENT) {
+		return hw->gpio_cs_count;
 	}
 	for (i = 0; i < hw->gpio_cs_count; i++) {
 		hw->gpio_cs[i] = of_get_gpio_flags(np, i, NULL);
diff --git a/drivers/spi/spi-ppc4xx.c b/drivers/spi/spi-ppc4xx.c
index 7a85f22..b5243c5 100644
--- a/drivers/spi/spi-ppc4xx.c
+++ b/drivers/spi/spi-ppc4xx.c
@@ -419,7 +419,7 @@ static int __init spi_ppc4xx_of_probe(struct platform_device *op)
 	 * This includes both "null" gpio's and real ones.
 	 */
 	num_gpios = of_gpio_count(np);
-	if (num_gpios) {
+	if (num_gpios > 0) {
 		int i;
 
 		hw->gpios = kzalloc(sizeof(int) * num_gpios, GFP_KERNEL);
@@ -454,6 +454,10 @@ static int __init spi_ppc4xx_of_probe(struct platform_device *op)
 				goto free_gpios;
 			}
 		}
+	} else if (num_gpios < 0 && num_gpios != -ENOENT) {
+		dev_err(dev, "could not count gpio field entries\n");
+		ret = num_gpios;
+		goto free_master;
 	}
 
 	/* Setup the state for the bitbang driver */
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 19ee901..9c2acf1 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1059,7 +1059,7 @@ EXPORT_SYMBOL_GPL(spi_alloc_master);
 #ifdef CONFIG_OF
 static int of_spi_register_master(struct spi_master *master)
 {
-	u16 nb;
+	int nb;
 	int i, *cs;
 	struct device_node *np = master->dev.of_node;
 
@@ -1067,10 +1067,13 @@ static int of_spi_register_master(struct spi_master *master)
 		return 0;
 
 	nb = of_gpio_named_count(np, "cs-gpios");
-	master->num_chipselect = max(nb, master->num_chipselect);
-
-	if (nb < 1)
+	if (nb == 0 || nb == -ENOENT) /* No error if cs-gpios does not exist */
 		return 0;
+	else if (nb < 0)
+		return nb;
+
+	if (nb > master->num_chipselect)
+		master->num_chipselect = (u16)nb;
 
 	cs = devm_kzalloc(&master->dev,
 			  sizeof(int) * master->num_chipselect,
-- 
1.7.0.4


------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnnow-d2d

Re: [PATCH v2 2/6] of: Return -ENXIO from of_parse_phandle_with_args for too large index and return errors from of_gpio_named_count

[Linus Walleij]

On Tue, Jan 29, 2013 at 3:53 PM, Andreas Larsson <andreas-FkzTOoA/JUlBDgjK7y7TUQ@public.gmane.org> wrote:

> This lets of_gpio_named_count return an errno on errors by being able to
> distinguish between reaching the end of the phandle list and getting some other
> error from of_parse_phandle_with_args.
>
> Return error from of_spi_register_master when there is an "cs-gpios" list for
> which gp_gpio_named_count fails.
>
> Adjust various drivers cope with error return from of_gpio_named_count,
> including via of_gpio_count.
>
> Signed-off-by: Andreas Larsson <andreas-FkzTOoA/JUlBDgjK7y7TUQ@public.gmane.org>
> ---
> Changes since v1:
> - Handle error return values from calls to of_gpio_count

Looks correct to me, but I'm no DT-ninja.

For the GPIO portions:
Acked-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Yours,
Linus Walleij

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

Re: [PATCH v2 2/6] of: Return -ENXIO from of_parse_phandle_with_args for too large index and return errors from of_gpio_named_count

[Grant Likely]

On Tue, 29 Jan 2013 15:53:39 +0100, Andreas Larsson <andreas@gaisler.com> wrote:
> This lets of_gpio_named_count return an errno on errors by being able to
> distinguish between reaching the end of the phandle list and getting some other
> error from of_parse_phandle_with_args.
> 
> Return error from of_spi_register_master when there is an "cs-gpios" list for
> which gp_gpio_named_count fails.
> 
> Adjust various drivers cope with error return from of_gpio_named_count,
> including via of_gpio_count.
> 
> Signed-off-by: Andreas Larsson <andreas@gaisler.com>

Actually, I'd been meaning to fix of_gpio_count for quite a while now.
The current code uses a stupid algorithm to parse each specifier one at
a time and count how many iterations it takes before getting an error.
BLECH! A pox on me for being so lazy when I wrote it.

Anyway, your patch here spurred me on to fix it. I've just done so and
will post the patch in a few minutes. Please test and let me know if it
works for you.

g.

[PATCH v2 3/6] of_spi: Initialize cs_gpios properly

[Andreas Larsson]

Using memset does not set an array of integers properly

Signed-off-by: Andreas Larsson <andreas-FkzTOoA/JUlBDgjK7y7TUQ@public.gmane.org>
---
 drivers/spi/spi.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 9c2acf1..a4baa0a 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1083,7 +1083,8 @@ static int of_spi_register_master(struct spi_master *master)
 	if (!master->cs_gpios)
 		return -ENOMEM;
 
-	memset(cs, -EINVAL, master->num_chipselect);
+	for (i = 0; i < master->num_chipselect; i++)
+		cs[i] = -EINVAL;
 
 	for (i = 0; i < nb; i++)
 		cs[i] = of_get_named_gpio(np, "cs-gpios", i);
-- 
1.7.0.4


------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnnow-d2d

Re: [PATCH v2 3/6] of_spi: Initialize cs_gpios properly

[Grant Likely]

On Tue, 29 Jan 2013 15:53:40 +0100, Andreas Larsson <andreas@gaisler.com> wrote:
> Using memset does not set an array of integers properly
> 
> Signed-off-by: Andreas Larsson <andreas@gaisler.com>

Applied, thanks.

g.

> ---
>  drivers/spi/spi.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 9c2acf1..a4baa0a 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -1083,7 +1083,8 @@ static int of_spi_register_master(struct spi_master *master)
>  	if (!master->cs_gpios)
>  		return -ENOMEM;
>  
> -	memset(cs, -EINVAL, master->num_chipselect);
> +	for (i = 0; i < master->num_chipselect; i++)
> +		cs[i] = -EINVAL;
>  
>  	for (i = 0; i < nb; i++)
>  		cs[i] = of_get_named_gpio(np, "cs-gpios", i);
> -- 
> 1.7.0.4
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

[PATCH v2 4/6] of_spi: Document cs_gpios and cs_gpio in kernel-doc

[Andreas Larsson]

This adds missing kernel-doc entries for cs_gpios in struct spi_master and
cs_gpio in struct spi_device.

Signed-off-by: Andreas Larsson <andreas-FkzTOoA/JUlBDgjK7y7TUQ@public.gmane.org>
---
 include/linux/spi/spi.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index f629189..88a669c 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -57,6 +57,7 @@ extern struct bus_type spi_bus_type;
  * @modalias: Name of the driver to use with this device, or an alias
  *	for that name.  This appears in the sysfs "modalias" attribute
  *	for driver coldplugging, and in uevents used for hotplugging
+ * @cs_gpio: Negative or gpio for chip select.
  *
  * A @spi_device is used to interchange data between an SPI slave
  * (usually a discrete chip) and CPU memory.
@@ -258,6 +259,7 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
  * @unprepare_transfer_hardware: there are currently no more messages on the
  *	queue so the subsystem notifies the driver that it may relax the
  *	hardware by issuing this call
+ * @cs_gpios: possible array of negative values or gpios for chip selects
  *
  * Each SPI master controller can communicate with one or more @spi_device
  * children.  These make a small bus, sharing MOSI, MISO and SCK signals
-- 
1.7.0.4


------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnnow-d2d

Re: [PATCH v2 4/6] of_spi: Document cs_gpios and cs_gpio in kernel-doc

[Grant Likely]

On Tue, 29 Jan 2013 15:53:41 +0100, Andreas Larsson <andreas-FkzTOoA/JUlBDgjK7y7TUQ@public.gmane.org> wrote:
> This adds missing kernel-doc entries for cs_gpios in struct spi_master and
> cs_gpio in struct spi_device.
> 
> Signed-off-by: Andreas Larsson <andreas-FkzTOoA/JUlBDgjK7y7TUQ@public.gmane.org>

Applied, thanks. I did tweak the language a bit.

g.

> ---
>  include/linux/spi/spi.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index f629189..88a669c 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -57,6 +57,7 @@ extern struct bus_type spi_bus_type;
>   * @modalias: Name of the driver to use with this device, or an alias
>   *	for that name.  This appears in the sysfs "modalias" attribute
>   *	for driver coldplugging, and in uevents used for hotplugging
> + * @cs_gpio: Negative or gpio for chip select.
>   *
>   * A @spi_device is used to interchange data between an SPI slave
>   * (usually a discrete chip) and CPU memory.
> @@ -258,6 +259,7 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
>   * @unprepare_transfer_hardware: there are currently no more messages on the
>   *	queue so the subsystem notifies the driver that it may relax the
>   *	hardware by issuing this call
> + * @cs_gpios: possible array of negative values or gpios for chip selects
>   *
>   * Each SPI master controller can communicate with one or more @spi_device
>   * children.  These make a small bus, sharing MOSI, MISO and SCK signals
> -- 
> 1.7.0.4
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

[PATCH v2 5/6] of_spi: Add fetching of of_gpio flags to of_spi_register_master

[Andreas Larsson]

When using a gpio chip select with a OF_GPIO_ACTIVE_LOW flag, this needs to be
known to the controller driver.

Signed-off-by: Andreas Larsson <andreas@gaisler.com>
---
 Documentation/devicetree/bindings/spi/spi-bus.txt |    3 +-
 drivers/spi/spi.c                                 |   24 ++++++++++++++++++--
 include/linux/spi/spi.h                           |    5 ++++
 3 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
index 296015e..a4950a6 100644
--- a/Documentation/devicetree/bindings/spi/spi-bus.txt
+++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
@@ -57,7 +57,8 @@ contain the following properties.
     		    3-wire mode.
 
 If a gpio chipselect is used for the SPI slave the gpio number will be passed
-via the cs_gpio
+via the cs_gpio and the corresponding of_gpio_flags will be passed via
+cs_gpio_flags.
 
 SPI example for an MPC5200 SPI bus:
 	spi@f00 {
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index a4baa0a..6f1b717 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -385,8 +385,10 @@ int spi_add_device(struct spi_device *spi)
 		goto done;
 	}
 
-	if (master->cs_gpios)
+	if (master->cs_gpios) {
 		spi->cs_gpio = master->cs_gpios[spi->chip_select];
+		spi->cs_gpio_flags = master->cs_gpio_flags[spi->chip_select];
+	}
 
 	/* Drivers may modify this initial i/o setup, but will
 	 * normally rely on the device being setup.  Devices
@@ -1060,7 +1062,8 @@ EXPORT_SYMBOL_GPL(spi_alloc_master);
 static int of_spi_register_master(struct spi_master *master)
 {
 	int nb;
-	int i, *cs;
+	int ret, i, *cs;
+	enum of_gpio_flags *flags;
 	struct device_node *np = master->dev.of_node;
 
 	if (!np)
@@ -1083,13 +1086,28 @@ static int of_spi_register_master(struct spi_master *master)
 	if (!master->cs_gpios)
 		return -ENOMEM;
 
+	flags = devm_kzalloc(&master->dev,
+			     sizeof(*flags) * master->num_chipselect,
+			     GFP_KERNEL);
+	master->cs_gpio_flags = flags;
+
+	if (!master->cs_gpio_flags) {
+		ret = -ENOMEM;
+		goto err_alloc_flags;
+	}
+
 	for (i = 0; i < master->num_chipselect; i++)
 		cs[i] = -EINVAL;
 
 	for (i = 0; i < nb; i++)
-		cs[i] = of_get_named_gpio(np, "cs-gpios", i);
+		cs[i] = of_get_named_gpio_flags(np, "cs-gpios", i, &flags[i]);
 
 	return 0;
+
+err_alloc_flags:
+	devm_kfree(&master->dev, master->cs_gpios);
+	master->cs_gpios = NULL;
+	return ret;
 }
 #else
 static int of_spi_register_master(struct spi_master *master)
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 88a669c..96b1055 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -23,6 +23,7 @@
 #include <linux/mod_devicetable.h>
 #include <linux/slab.h>
 #include <linux/kthread.h>
+#include <linux/of_gpio.h>
 
 /*
  * INTERFACES between SPI master-side drivers and SPI infrastructure.
@@ -58,6 +59,7 @@ extern struct bus_type spi_bus_type;
  *	for that name.  This appears in the sysfs "modalias" attribute
  *	for driver coldplugging, and in uevents used for hotplugging
  * @cs_gpio: Negative or gpio for chip select.
+ * @cs_gpio_flags: of_gpio_flags corresponding to cs_gpio
  *
  * A @spi_device is used to interchange data between an SPI slave
  * (usually a discrete chip) and CPU memory.
@@ -92,6 +94,7 @@ struct spi_device {
 	void			*controller_data;
 	char			modalias[SPI_NAME_SIZE];
 	int			cs_gpio;	/* chip select gpio */
+	enum of_gpio_flags	cs_gpio_flags;	/* chip select of_gpio_flags */
 
 	/*
 	 * likely need more hooks for more protocol options affecting how
@@ -260,6 +263,7 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
  *	queue so the subsystem notifies the driver that it may relax the
  *	hardware by issuing this call
  * @cs_gpios: possible array of negative values or gpios for chip selects
+ * @cs_gpio_flags: possible array of of_gpio_flags corresponding to cs_gpios
  *
  * Each SPI master controller can communicate with one or more @spi_device
  * children.  These make a small bus, sharing MOSI, MISO and SCK signals
@@ -367,6 +371,7 @@ struct spi_master {
 	int (*unprepare_transfer_hardware)(struct spi_master *master);
 	/* gpio chip select */
 	int			*cs_gpios;
+	enum of_gpio_flags	*cs_gpio_flags;
 };
 
 static inline void *spi_master_get_devdata(struct spi_master *master)
-- 
1.7.0.4

Re: [PATCH v2 5/6] of_spi: Add fetching of of_gpio flags to of_spi_register_master

[Grant Likely]

On Tue, 29 Jan 2013 15:53:42 +0100, Andreas Larsson <andreas@gaisler.com> wrote:
> When using a gpio chip select with a OF_GPIO_ACTIVE_LOW flag, this needs to be
> known to the controller driver.
> 
> Signed-off-by: Andreas Larsson <andreas@gaisler.com>

Out of curiosity, what do you need the flags for? Polarity of the CS
signal? I think it is long past time to revisit baking polarity control
into core of gpiolib. Would you mind investigating doing that instead of
having to manage it manually in each and every driver?

g.

Re: [PATCH v2 5/6] of_spi: Add fetching of of_gpio flags to of_spi_register_master

[Andreas Larsson]

On 2013-02-11 01:22, Grant Likely wrote:
> On Tue, 29 Jan 2013 15:53:42 +0100, Andreas Larsson <andreas-FkzTOoA/JUlBDgjK7y7TUQ@public.gmane.org> wrote:
>> When using a gpio chip select with a OF_GPIO_ACTIVE_LOW flag, this needs to be
>> known to the controller driver.
>>
>> Signed-off-by: Andreas Larsson <andreas-FkzTOoA/JUlBDgjK7y7TUQ@public.gmane.org>
>
> Out of curiosity, what do you need the flags for? Polarity of the CS
> signal? I think it is long past time to revisit baking polarity control
> into core of gpiolib. Would you mind investigating doing that instead of
> having to manage it manually in each and every driver?

I don't even need it on my hardware. It seemed like a good thing to have 
via of_spi_register_master. I am OK with just dropping this.

Cheers,
Andreas


------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

Re: [PATCH v2 5/6] of_spi: Add fetching of of_gpio flags to of_spi_register_master

[Grant Likely]

On Tue, 12 Feb 2013 10:39:02 +0100, Andreas Larsson <andreas@gaisler.com> wrote:
> On 2013-02-11 01:22, Grant Likely wrote:
> > On Tue, 29 Jan 2013 15:53:42 +0100, Andreas Larsson <andreas@gaisler.com> wrote:
> >> When using a gpio chip select with a OF_GPIO_ACTIVE_LOW flag, this needs to be
> >> known to the controller driver.
> >>
> >> Signed-off-by: Andreas Larsson <andreas@gaisler.com>
> >
> > Out of curiosity, what do you need the flags for? Polarity of the CS
> > signal? I think it is long past time to revisit baking polarity control
> > into core of gpiolib. Would you mind investigating doing that instead of
> > having to manage it manually in each and every driver?
> 
> I don't even need it on my hardware. It seemed like a good thing to have 
> via of_spi_register_master. I am OK with just dropping this.

Hahaha. Okay, let's leave it for now until someone does need it. I can
however see the need for the polarity flag, so the request still stands
for someone to put that feature into the core.  :-)

g.

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

[PATCH v2 6/6] of_spi: Initialize cs_gpios and cs_gpio with -EEXIST

[Andreas Larsson]

Holes in the cs-gpios DT phandle list is supposed to mark that native
chipselects is to be used. The value returned from of_get_named_gpio_flags in
this case is -EEXIST. By initializing cs_gpios and cs_gpio with -EEXIST, this
and only this errno will indicate to a spi controller driver that a native
chipselect is to be used.

Signed-off-by: Andreas Larsson <andreas@gaisler.com>
---
 drivers/spi/spi.c       |    4 ++--
 include/linux/spi/spi.h |    7 +++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 6f1b717..7494bad 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -334,7 +334,7 @@ struct spi_device *spi_alloc_device(struct spi_master *master)
 	spi->dev.parent = &master->dev;
 	spi->dev.bus = &spi_bus_type;
 	spi->dev.release = spidev_release;
-	spi->cs_gpio = -EINVAL;
+	spi->cs_gpio = -EEXIST;
 	device_initialize(&spi->dev);
 	return spi;
 }
@@ -1097,7 +1097,7 @@ static int of_spi_register_master(struct spi_master *master)
 	}
 
 	for (i = 0; i < master->num_chipselect; i++)
-		cs[i] = -EINVAL;
+		cs[i] = -EEXIST;
 
 	for (i = 0; i < nb; i++)
 		cs[i] = of_get_named_gpio_flags(np, "cs-gpios", i, &flags[i]);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 96b1055..0701882 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -58,7 +58,8 @@ extern struct bus_type spi_bus_type;
  * @modalias: Name of the driver to use with this device, or an alias
  *	for that name.  This appears in the sysfs "modalias" attribute
  *	for driver coldplugging, and in uevents used for hotplugging
- * @cs_gpio: Negative or gpio for chip select.
+ * @cs_gpio: Negative or gpio for chip select.  Set to -EEXIST when chipselect
+ *	should be handled natively by the controller driver
  * @cs_gpio_flags: of_gpio_flags corresponding to cs_gpio
  *
  * A @spi_device is used to interchange data between an SPI slave
@@ -262,7 +263,9 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
  * @unprepare_transfer_hardware: there are currently no more messages on the
  *	queue so the subsystem notifies the driver that it may relax the
  *	hardware by issuing this call
- * @cs_gpios: possible array of negative values or gpios for chip selects
+ * @cs_gpios: possible array of negative values or gpios for chip selects.  A
+ *	chipselect that should be handled natively by the controller driver is
+ *	set to -EEXIST.
  * @cs_gpio_flags: possible array of of_gpio_flags corresponding to cs_gpios
  *
  * Each SPI master controller can communicate with one or more @spi_device
-- 
1.7.0.4

Re: [PATCH v2 6/6] of_spi: Initialize cs_gpios and cs_gpio with -EEXIST

[Grant Likely]

On Tue, 29 Jan 2013 15:53:43 +0100, Andreas Larsson <andreas@gaisler.com> wrote:
> Holes in the cs-gpios DT phandle list is supposed to mark that native
> chipselects is to be used. The value returned from of_get_named_gpio_flags in
> this case is -EEXIST. By initializing cs_gpios and cs_gpio with -EEXIST, this
> and only this errno will indicate to a spi controller driver that a native
> chipselect is to be used.
> 
> Signed-off-by: Andreas Larsson <andreas@gaisler.com>

I've left this one off for now. Take a look at the patch I posted and
let me know if you think this one should still be applied.

g.

> ---
>  drivers/spi/spi.c       |    4 ++--
>  include/linux/spi/spi.h |    7 +++++--
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 6f1b717..7494bad 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -334,7 +334,7 @@ struct spi_device *spi_alloc_device(struct spi_master *master)
>  	spi->dev.parent = &master->dev;
>  	spi->dev.bus = &spi_bus_type;
>  	spi->dev.release = spidev_release;
> -	spi->cs_gpio = -EINVAL;
> +	spi->cs_gpio = -EEXIST;
>  	device_initialize(&spi->dev);
>  	return spi;
>  }
> @@ -1097,7 +1097,7 @@ static int of_spi_register_master(struct spi_master *master)
>  	}
>  
>  	for (i = 0; i < master->num_chipselect; i++)
> -		cs[i] = -EINVAL;
> +		cs[i] = -EEXIST;
>  
>  	for (i = 0; i < nb; i++)
>  		cs[i] = of_get_named_gpio_flags(np, "cs-gpios", i, &flags[i]);
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index 96b1055..0701882 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -58,7 +58,8 @@ extern struct bus_type spi_bus_type;
>   * @modalias: Name of the driver to use with this device, or an alias
>   *	for that name.  This appears in the sysfs "modalias" attribute
>   *	for driver coldplugging, and in uevents used for hotplugging
> - * @cs_gpio: Negative or gpio for chip select.
> + * @cs_gpio: Negative or gpio for chip select.  Set to -EEXIST when chipselect
> + *	should be handled natively by the controller driver
>   * @cs_gpio_flags: of_gpio_flags corresponding to cs_gpio
>   *
>   * A @spi_device is used to interchange data between an SPI slave
> @@ -262,7 +263,9 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
>   * @unprepare_transfer_hardware: there are currently no more messages on the
>   *	queue so the subsystem notifies the driver that it may relax the
>   *	hardware by issuing this call
> - * @cs_gpios: possible array of negative values or gpios for chip selects
> + * @cs_gpios: possible array of negative values or gpios for chip selects.  A
> + *	chipselect that should be handled natively by the controller driver is
> + *	set to -EEXIST.
>   * @cs_gpio_flags: possible array of of_gpio_flags corresponding to cs_gpios
>   *
>   * Each SPI master controller can communicate with one or more @spi_device
> -- 
> 1.7.0.4
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

Re: [PATCH v2 6/6] of_spi: Initialize cs_gpios and cs_gpio with -EEXIST

[Andreas Larsson]

On 2013-02-11 01:23, Grant Likely wrote:
> On Tue, 29 Jan 2013 15:53:43 +0100, Andreas Larsson <andreas-FkzTOoA/JUlBDgjK7y7TUQ@public.gmane.org> wrote:
>> Holes in the cs-gpios DT phandle list is supposed to mark that native
>> chipselects is to be used. The value returned from of_get_named_gpio_flags in
>> this case is -EEXIST. By initializing cs_gpios and cs_gpio with -EEXIST, this
>> and only this errno will indicate to a spi controller driver that a native
>> chipselect is to be used.
>>
>> Signed-off-by: Andreas Larsson <andreas-FkzTOoA/JUlBDgjK7y7TUQ@public.gmane.org>
>
> I've left this one off for now. Take a look at the patch I posted and
> let me know if you think this one should still be applied.

I think that of_spi_register_master should return an error when the 
cs-gpios property is broken (part of patch 2) and that cs_gpios and 
cs_gpio should be initialized to the same value as a hole in the plist.

This patch does not work without 1 and 2, so I'll submit a modified 
patch once "of: Create function for counting number of phandles in a 
property" has stabilized. Would it have to go through the gpio branch then?

Cheers,
Andreas


------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

Re: [PATCH v2 6/6] of_spi: Initialize cs_gpios and cs_gpio with -EEXIST

[Grant Likely]

On Tue, 12 Feb 2013 10:46:27 +0100, Andreas Larsson <andreas@gaisler.com> wrote:
> On 2013-02-11 01:23, Grant Likely wrote:
> > On Tue, 29 Jan 2013 15:53:43 +0100, Andreas Larsson <andreas@gaisler.com> wrote:
> >> Holes in the cs-gpios DT phandle list is supposed to mark that native
> >> chipselects is to be used. The value returned from of_get_named_gpio_flags in
> >> this case is -EEXIST. By initializing cs_gpios and cs_gpio with -EEXIST, this
> >> and only this errno will indicate to a spi controller driver that a native
> >> chipselect is to be used.
> >>
> >> Signed-off-by: Andreas Larsson <andreas@gaisler.com>
> >
> > I've left this one off for now. Take a look at the patch I posted and
> > let me know if you think this one should still be applied.
> 
> I think that of_spi_register_master should return an error when the 
> cs-gpios property is broken (part of patch 2) and that cs_gpios and 
> cs_gpio should be initialized to the same value as a hole in the plist.
> 
> This patch does not work without 1 and 2, so I'll submit a modified 
> patch once "of: Create function for counting number of phandles in a 
> property" has stabilized. Would it have to go through the gpio branch then?

I'll put it through the DT branch since that is where I'll put the
of_count_phandle patch (assuming it is ready for the 3.9 merge window).

g.