diff mbox

[net-next,1/2] net: dsa: Add support for a switch reset gpio

Message ID 1447889365-25256-2-git-send-email-andrew@lunn.ch
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Andrew Lunn Nov. 18, 2015, 11:29 p.m. UTC
Some boards have a gpio line tied to the switch reset pin. Allow this
gpio to be retrieved from the device tree, and take the switch out of
reset before performing the probe.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 Documentation/devicetree/bindings/net/dsa/dsa.txt |  3 +++
 include/net/dsa.h                                 |  9 +++++++++
 net/dsa/dsa.c                                     | 19 +++++++++++++++++++
 3 files changed, 31 insertions(+)

Comments

Andrew Lunn Nov. 19, 2015, 3:06 a.m. UTC | #1
> index 1eba07feb34a..39cd19eaaf4e 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -21,6 +21,8 @@
>  #include <linux/of_mdio.h>
>  #include <linux/of_platform.h>
>  #include <linux/of_net.h>
> +#include <linux/nvmem-consumer.h>

Comment to self:

That include should not be there.

I will post a v2 once the discussion has stopped.

  Andrew
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Neil Armstrong Nov. 19, 2015, 8:39 a.m. UTC | #2
Hi Andrew,

On 11/19/2015 12:29 AM, Andrew Lunn wrote:
> +		gpio = of_get_named_gpio_flags(child, "reset-gpios", 0,
> +					       &flags);
> +		if (gpio_is_valid(gpio)) {
> +			ret = devm_gpio_request_one(dev, gpio, flags,
> +						    "switch_reset");
> +			if (ret)
> +				goto out_free_chip;
> +
> +			cd->reset = gpio;
> +			cd->reset_flags = flags;
> +			off = (flags && OF_GPIO_ACTIVE_LOW ? 1 : 0);
> +			gpio_direction_output(cd->reset, off);
> +		}
> +
>  		for_each_available_child_of_node(child, port) {
>  			port_reg = of_get_property(port, "reg", NULL);
>  			if (!port_reg)
> 

You could also use :
gpio = of_get_named_gpio(child, "reset-gpios", 0)
devm_gpio_request(dev, gpio, "switch_reset")

and :
cd->reset = gpio_to_desc(gpio);

and cd->switch reset to struct gpio_desc *reset

to use the gpiod calls afterward. The flags are no more needed.

Neil
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrew Lunn Nov. 19, 2015, 2:55 p.m. UTC | #3
On Thu, Nov 19, 2015 at 09:39:11AM +0100, Neil Armstrong wrote:
> Hi Andrew,
> 
> On 11/19/2015 12:29 AM, Andrew Lunn wrote:
> > +		gpio = of_get_named_gpio_flags(child, "reset-gpios", 0,
> > +					       &flags);
> > +		if (gpio_is_valid(gpio)) {
> > +			ret = devm_gpio_request_one(dev, gpio, flags,
> > +						    "switch_reset");
> > +			if (ret)
> > +				goto out_free_chip;
> > +
> > +			cd->reset = gpio;
> > +			cd->reset_flags = flags;
> > +			off = (flags && OF_GPIO_ACTIVE_LOW ? 1 : 0);
> > +			gpio_direction_output(cd->reset, off);
> > +		}
> > +
> >  		for_each_available_child_of_node(child, port) {
> >  			port_reg = of_get_property(port, "reg", NULL);
> >  			if (!port_reg)
> > 
> 
> You could also use :
> gpio = of_get_named_gpio(child, "reset-gpios", 0)
> devm_gpio_request(dev, gpio, "switch_reset")
> 
> and :
> cd->reset = gpio_to_desc(gpio);
> 
> and cd->switch reset to struct gpio_desc *reset
> 
> to use the gpiod calls afterward. The flags are no more needed.

Hi Neil

I really wanted to use gpiod, since as you said, it takes care of
active low/active high. I tried all sorts of combinations. The problem
with this one is that desc->flags is set in gpiod_parse_flags(), which
is only called from gpiod_get_index(), which is limited to gpios in
the root of the devices subtree. I was 'lucky' in that my reset is
active low, so i noticed the problem when my switch failed to probe,
being held in reset, because the ACTIVE_LOW flag in DT is being ignored.

It seems like gpiod is backwards compatible to gpio, but gpio is not
forward compatible to gpiod.

	Andrew
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.txt b/Documentation/devicetree/bindings/net/dsa/dsa.txt
index 04e6bef3ac3f..5fdbbcdf8c4b 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa.txt
+++ b/Documentation/devicetree/bindings/net/dsa/dsa.txt
@@ -31,6 +31,8 @@  A switch child node has the following optional property:
 			  switch. Must be set if the switch can not detect
 			  the presence and/or size of a connected EEPROM,
 			  otherwise optional.
+- reset-gpios		: phandle and specifier to a gpio line connected to
+			  reset pin of the switch chip.
 
 A switch may have multiple "port" children nodes
 
@@ -114,6 +116,7 @@  Example:
 			#size-cells = <0>;
 			reg = <17 1>;	/* MDIO address 17, switch 1 in tree */
 			mii-bus = <&mii_bus1>;
+			reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
 
 			switch1port0: port@0 {
 				reg = <0>;
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 82a4c6011173..913a61ea81d3 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -16,6 +16,7 @@ 
 #include <linux/timer.h>
 #include <linux/workqueue.h>
 #include <linux/of.h>
+#include <linux/of_gpio.h>
 #include <linux/phy.h>
 #include <linux/phy_fixed.h>
 #include <linux/ethtool.h>
@@ -64,6 +65,14 @@  struct dsa_chip_data {
 	 * NULL if there is only one switch chip.
 	 */
 	s8		*rtable;
+
+	/*
+	 * A switch may have a GPIO line tied to its reset pin. Parse
+	 * this from the device tree, and use it before performing
+	 * switch soft reset.
+	 */
+	int reset;
+	enum of_gpio_flags reset_flags;
 };
 
 struct dsa_platform_data {
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 1eba07feb34a..39cd19eaaf4e 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -21,6 +21,8 @@ 
 #include <linux/of_mdio.h>
 #include <linux/of_platform.h>
 #include <linux/of_net.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of_gpio.h>
 #include <linux/sysfs.h>
 #include <linux/phy_fixed.h>
 #include "dsa_priv.h"
@@ -688,6 +690,9 @@  static int dsa_of_probe(struct device *dev)
 	const char *port_name;
 	int chip_index, port_index;
 	const unsigned int *sw_addr, *port_reg;
+	int gpio;
+	enum of_gpio_flags flags;
+	int off;
 	u32 eeprom_len;
 	int ret;
 
@@ -767,6 +772,20 @@  static int dsa_of_probe(struct device *dev)
 			cd->host_dev = &mdio_bus_switch->dev;
 		}
 
+		gpio = of_get_named_gpio_flags(child, "reset-gpios", 0,
+					       &flags);
+		if (gpio_is_valid(gpio)) {
+			ret = devm_gpio_request_one(dev, gpio, flags,
+						    "switch_reset");
+			if (ret)
+				goto out_free_chip;
+
+			cd->reset = gpio;
+			cd->reset_flags = flags;
+			off = (flags && OF_GPIO_ACTIVE_LOW ? 1 : 0);
+			gpio_direction_output(cd->reset, off);
+		}
+
 		for_each_available_child_of_node(child, port) {
 			port_reg = of_get_property(port, "reg", NULL);
 			if (!port_reg)