Re: [PATCH] i2c: pca954x: Fix reset GPIO property name

2014-06-03 Thread Laurent Pinchart
Hi Alexandre,

On Tuesday 03 June 2014 10:38:53 Alexandre Courbot wrote:
 On 06/03/2014 09:22 AM, Laurent Pinchart wrote:
  The DT bindings for the pca954x document that the reset GPIO is
  specified through the reset-gpios property. However, the driver
  erroneously uses a property name of reset-gpio.
  
  The GPIO DT bindings documentation mentions that
  
  GPIO properties should be named [name-]gpios. The exact meaning of
  each gpios property must be documented in the device tree binding for
  each device.
  
  The correct reset GPIO property name is thus reset-gpios. Fix the
  driver accordingly.
 
 The plural form should be preferred indeed.
 
  Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
  ---
  
drivers/i2c/muxes/i2c-mux-pca954x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
  
  While commit dd34c37aa3e81715a1ed8da61fa438072428e188
  
  Author: Thierry Reding tred...@nvidia.com
  Date:   Wed Apr 23 17:28:09 2014 +0200
  
   gpio: of: Allow -gpio suffix for property names
   
   Many bindings use the -gpio suffix in property names. Support this in
   addition to the -gpios suffix when requesting GPIOs using the new
   descriptor-based API.
  
  adds support for the singular form of the property, the GPIO DT bindings
  document the plural form only. I've thus decided to fix the driver instead
  of the bindings, even though it could be argued that the singular form
  makes more sense in this case. I've CC'ed the people involved with the
  above commit for comments on that, and have no issue fixing the bindings
  instead of the driver if the singular form is preferred.
  
  diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c
  b/drivers/i2c/muxes/i2c-mux-pca954x.c index 550bd36..73491ca 100644
  --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
  +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
  @@ -205,7 +205,7 @@ static int pca954x_probe(struct i2c_client *client,
  
  int gpio;
  
  /* Get the mux out of reset if a reset GPIO is specified. */
  
  -   gpio = of_get_named_gpio_flags(np, reset-gpio, 0, flags);
  +   gpio = of_get_named_gpio_flags(np, reset-gpios, 0, flags);
 
 Won't this break DT compatibility for potential users of this driver? (I
 cannot find any in mainline though)

Only if those users don't conform to the DT bindings documentation that 
mandates the plural form, and are thus broken already ;-)

 Considering that this GPIO is only acquired and set once and for all
 during probe (a very simple use-case), how about switching this to the
 gpiod interface instead? That way, you can take advantage of Thierry's
 patch and the plural form will work while also maintaining the singular
 form for backward compatibility.

Regardless of the singular vs. plural debate I think that's a good idea. I'll 
thus rework this patch.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] i2c: pca954x: Fix reset GPIO property name

2014-06-02 Thread Alexandre Courbot

On 06/03/2014 09:22 AM, Laurent Pinchart wrote:

The DT bindings for the pca954x document that the reset GPIO is
specified through the reset-gpios property. However, the driver
erroneously uses a property name of reset-gpio.

The GPIO DT bindings documentation mentions that

GPIO properties should be named [name-]gpios. The exact meaning of
each gpios property must be documented in the device tree binding for
each device.

The correct reset GPIO property name is thus reset-gpios. Fix the
driver accordingly.


The plural form should be preferred indeed.



Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
  drivers/i2c/muxes/i2c-mux-pca954x.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

While commit dd34c37aa3e81715a1ed8da61fa438072428e188

Author: Thierry Reding tred...@nvidia.com
Date:   Wed Apr 23 17:28:09 2014 +0200

 gpio: of: Allow -gpio suffix for property names

 Many bindings use the -gpio suffix in property names. Support this in
 addition to the -gpios suffix when requesting GPIOs using the new
 descriptor-based API.

adds support for the singular form of the property, the GPIO DT bindings
document the plural form only. I've thus decided to fix the driver instead of
the bindings, even though it could be argued that the singular form makes more
sense in this case. I've CC'ed the people involved with the above commit for
comments on that, and have no issue fixing the bindings instead of the driver
if the singular form is preferred.

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c 
b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 550bd36..73491ca 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -205,7 +205,7 @@ static int pca954x_probe(struct i2c_client *client,
int gpio;

/* Get the mux out of reset if a reset GPIO is specified. */
-   gpio = of_get_named_gpio_flags(np, reset-gpio, 0, flags);
+   gpio = of_get_named_gpio_flags(np, reset-gpios, 0, flags);


Won't this break DT compatibility for potential users of this driver? (I 
cannot find any in mainline though)


Considering that this GPIO is only acquired and set once and for all 
during probe (a very simple use-case), how about switching this to the 
gpiod interface instead? That way, you can take advantage of Thierry's 
patch and the plural form will work while also maintaining the singular 
form for backward compatibility.


Alex.

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html