net: dsa: ksz: Add reset GPIO handling

PATCH Superseded Marek Vasut 3 participants 10 messages Dec 7, 2018 – Dec 12, 2018

Subsystems (4)

Networking Network drivers Networking DSA Microchip ksz series Ethernet switch driver

lore

Share & tools

lore

Marek Vasut

author

2018-12-07 21:51 UTC #0 lore raw

PATCH v2 1/1 net: dsa: ksz: Add reset GPIO handling

Add code to handle optional reset GPIO in the KSZ switch driver. The switch has a reset GPIO line which can be controlled by the CPU, so make sure it is configured correctly in such setups.

Signed-off-by Marek Vasut <[email protected]>

Cc Vivien Didelot <[email protected]>

Cc Woojung Huh <[email protected]>

Cc David S. Miller <[email protected]>

Cc Tristram Ha <[email protected]>

---
V2: Switch to devm_gpiod_get_optional()

---
 drivers/net/dsa/microchip/ksz_common.c | 17 +++++++++++++++++
 drivers/net/dsa/microchip/ksz_priv.h   |  2 ++
 2 files changed, 19 insertions(+)

drivers/net/dsa/microchip/ksz_common.c +17 -0

index 9705808c3af7a..3b12e2dcff31b 100644
@@ -8,12 +8,14 @@
 #include <linux/delay.h>
 #include <linux/export.h>
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/platform_data/microchip-ksz.h>
 #include <linux/phy.h>
 #include <linux/etherdevice.h>
 #include <linux/if_bridge.h>
+#include <linux/of_gpio.h>
 #include <linux/of_net.h>
 #include <net/dsa.h>
 #include <net/switchdev.h>

@@ -294,6 +296,17 @@ int ksz_switch_register(struct ksz_device *dev,
 	if (dev->pdata)
 		dev->chip_id = dev->pdata->chip_id;
 
+	dev->reset_gpio = devm_gpiod_get_optional(dev->dev, "reset",
+						  GPIOD_OUT_LOW);
+	if (IS_ERR(dev->reset_gpio))
+		return PTR_ERR(dev->reset_gpio);
+
+	if (dev->reset_gpio) {
+		gpiod_set_value(dev->reset_gpio, 1);
+		mdelay(10);
+		gpiod_set_value(dev->reset_gpio, 0);
+	}
+
 	mutex_init(&dev->reg_mutex);
 	mutex_init(&dev->stats_mutex);
 	mutex_init(&dev->alu_mutex);

@@ -329,6 +342,10 @@ void ksz_switch_remove(struct ksz_device *dev)
 {
 	dev->dev_ops->exit(dev);
 	dsa_unregister_switch(dev->ds);
+
+	if (dev->reset_gpio)
+		gpiod_set_value(dev->reset_gpio, 1);
+
 }
 EXPORT_SYMBOL(ksz_switch_remove);

drivers/net/dsa/microchip/ksz_priv.h +2 -0

index a38ff0841ed4e..60b49010904bf 100644
@@ -59,6 +59,8 @@ struct ksz_device {
 
 	void *priv;
 
+	struct gpio_desc *reset_gpio;	/* Optional reset GPIO */
+
 	/* chip specific data */
 	u32 chip_id;
 	int num_vlans;

Signature · 1 line

2.18.0

Andrew Lunn

maintainer

2018-12-07 22:24 UTC #1 lore raw

Quoted 3 lines · Marek Vasut

Add code to handle optional reset GPIO in the KSZ switch driver. The switch has a reset GPIO line which can be controlled by the CPU, so make sure it is configured correctly in such setups.

Hi Marek

Please make this a patch series, not two individual patches.

And as David has already said, include a cover letter.

Otherwise, this looks O.K.

Thanks
Andrew

Marek Vasut

author

2018-12-07 23:00 UTC #2 lore raw

Quoted 6 lines · Andrew Lunn

Quoted 3 lines · Marek Vasut
Add code to handle optional reset GPIO in the KSZ switch driver. The switch has a reset GPIO line which can be controlled by the CPU, so make sure it is configured correctly in such setups.
Hi Marek

Hi Andrew,

Quoted 1 line

Please make this a patch series, not two individual patches.

This actually is an individual patch, it doesn't depend on anything. Or do you mean a series with the DT documentation change ?

Quoted 7 lines

And as David has already said, include a cover letter.
Otherwise, this looks O.K.
Thanks
Andrew

Signature · 2 lines

Best regards,
Marek Vasut

David Miller

maintainer

2018-12-07 23:46 UTC #3 lore raw

From: Marek Vasut <[email protected]>
Date: Fri, 7 Dec 2018 23:59:58 +0100

Quoted 6 lines · Andrew Lunn

Quoted 5 lines · Marek Vasut
Quoted 3 lines
Add code to handle optional reset GPIO in the KSZ switch driver. The switch has a reset GPIO line which can be controlled by the CPU, so make sure it is configured correctly in such setups.

Quoted 8 lines

Quoted 1 line
Hi Marek
Hi Andrew,
Quoted 1 line
Please make this a patch series, not two individual patches.
This actually is an individual patch, it doesn't depend on anything. Or do you mean a series with the DT documentation change ?

Yes, but all of this stuff is building up for one single purpose, and that is to support a new mode of operation with DSA or whatever.

So please group them together in a series with an appropriate header posting.

Marek Vasut

author

2018-12-08 04:27 UTC #4 lore raw

Quoted 13 lines · David Miller

From: Marek Vasut <[email protected]>
Date: Fri, 7 Dec 2018 23:59:58 +0100
Quoted 10 lines · Andrew Lunn
Quoted 6 lines · Marek Vasut
Quoted 3 lines
Add code to handle optional reset GPIO in the KSZ switch driver. The switch has a reset GPIO line which can be controlled by the CPU, so make sure it is configured correctly in such setups.
Hi Marek
Hi Andrew,

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
Please make this a patch series, not two individual patches.
This actually is an individual patch, it doesn't depend on anything. Or do you mean a series with the DT documentation change ?
Yes, but all of this stuff is building up for one single purpose, and that is to support a new mode of operation with DSA or whatever.

I'll group together the ones which make sense to group together and are not orthogonal if that's OK with you. The reset handling really is orthogonal from the rest and can go in independently of the rest.

Quoted 2 lines

So please group them together in a series with an appropriate header posting.

Sure

Signature · 2 lines

Best regards,
Marek Vasut

Andrew Lunn

maintainer

2018-12-08 11:11 UTC #5 lore raw

In reply to Marek Vasut

Quoted 2 lines

This actually is an individual patch, it doesn't depend on anything. Or do you mean a series with the DT documentation change ?

Yes, i mean together with the DT documentation change. Those two belong together, they are one functional change.

Part of this is also to do with scalability. It takes less effort to merge one patchset of two patches, as two individual patches. The truth is, developer time is cheap, maintainer time is expensive, so the process is optimized towards making the maintainers life easy.

So sometimes you do combine orthogonal changes together into one patchset, if there is a high purpose, eg. adding support for a new device on a new board. However, given the situation of two overlapping patchsets, it might be better to submit smaller patchsets.

Andrew

Marek Vasut

author

2018-12-10 13:44 UTC #6 lore raw

Quoted 5 lines · Andrew Lunn

Quoted 2 lines
This actually is an individual patch, it doesn't depend on anything. Or do you mean a series with the DT documentation change ?
Yes, i mean together with the DT documentation change. Those two belong together, they are one functional change.

Fine

Quoted 3 lines

Part of this is also to do with scalability. It takes less effort to merge one patchset of two patches, as two individual patches. The truth is, developer time is cheap, maintainer time is expensive

This is _not_ fine and I am actually offended by this statement. The way I read this is that maintainer time has more value than developer time, which justifies spending the developer time by maintainers without having any appreciation for it. I hope I am misreading your statement ?

Quoted 10 lines

, so
the process is optimized towards making the maintainers life easy.
So sometimes you do combine orthogonal changes together into one patchset, if there is a high purpose, eg. adding support for a new device on a new board. However, given the situation of two overlapping patchsets, it might be better to submit smaller patchsets.
Andrew

Signature · 2 lines

Best regards,
Marek Vasut

Andrew Lunn

maintainer

2018-12-10 14:37 UTC #7 lore raw

Quoted 13 lines · Marek Vasut

Quoted 5 lines · Andrew Lunn
Quoted 2 lines
This actually is an individual patch, it doesn't depend on anything. Or do you mean a series with the DT documentation change ?
Yes, i mean together with the DT documentation change. Those two belong together, they are one functional change.
Fine
Quoted 3 lines
Part of this is also to do with scalability. It takes less effort to merge one patchset of two patches, as two individual patches. The truth is, developer time is cheap, maintainer time is expensive

Quoted 5 lines

This is _not_ fine and I am actually offended by this statement. The way I read this is that maintainer time has more value than developer time, which justifies spending the developer time by maintainers without having any appreciation for it. I hope I am misreading your statement ?

Hi Marek

Sorry, i was not meaning to be offence. But it is part of the economics of the Linux Kernel. There are many more developers than maintainers. There are lots of studies over the years which suggests there are not enough maintainers, and those maintainers we have are overloaded. So the development process is based towards making the maintainer role easier, by asking the developers to do a bit more work. Writing easy to review patchsets, adding reviewed by tags to new versions of patches, including a summary of changes between each version, meaningful patchset, etc. Much of that is to make the reviewers job, who are mostly maintainers, easier. And to make the job of people like David, who does the actually merge, easier, scalable to the number of patches he needs to work on every day.

Andrew

Marek Vasut

author

2018-12-12 02:21 UTC #8 lore raw

Quoted 14 lines · Andrew Lunn

Quoted 13 lines · Marek Vasut
Quoted 5 lines · Andrew Lunn
Quoted 2 lines
This actually is an individual patch, it doesn't depend on anything. Or do you mean a series with the DT documentation change ?
Yes, i mean together with the DT documentation change. Those two belong together, they are one functional change.
Fine
Quoted 3 lines
Part of this is also to do with scalability. It takes less effort to merge one patchset of two patches, as two individual patches. The truth is, developer time is cheap, maintainer time is expensive

Quoted 7 lines

Quoted 5 lines
This is _not_ fine and I am actually offended by this statement. The way I read this is that maintainer time has more value than developer time, which justifies spending the developer time by maintainers without having any appreciation for it. I hope I am misreading your statement ?
Hi Marek

Hello Andrew,

Quoted 5 lines

Sorry, i was not meaning to be offence. But it is part of the economics of the Linux Kernel. There are many more developers than maintainers. There are lots of studies over the years which suggests there are not enough maintainers, and those maintainers we have are overloaded.

That's understandable, it's not just Linux kernel that's suffering from lack of maintainers.

Quoted 5 lines

So the development process is based towards making the maintainer role easier, by asking the developers to do a bit more work. Writing easy to review patchsets, adding reviewed by tags to new versions of patches, including a summary of changes between each version, meaningful patchset, etc.

And this is all fine and normal.

Quoted 4 lines

Much of that is to make the
reviewers job, who are mostly maintainers, easier. And to make the job of people like David, who does the actually merge, easier, scalable to the number of patches he needs to work on every day.

That's also fine by me. Although, a single maintainer cannot scale indefinitely, but that's another discussion altogether.

Notice how none of the above implied that someone's time is more important than someone else's. I'll just assume the above is what you wanted to express all along.

Signature · 2 lines

Best regards,
Marek Vasut

David Miller

maintainer

2018-12-10 18:01 UTC #9 lore raw

In reply to Marek Vasut

From: Marek Vasut <[email protected]>
Date: Mon, 10 Dec 2018 14:26:51 +0100

Quoted 5 lines

This is _not_ fine and I am actually offended by this statement. The way I read this is that maintainer time has more value than developer time, which justifies spending the developer time by maintainers without having any appreciation for it. I hope I am misreading your statement ?

Absolutely maintainer time, which is devoted to handling the work done by _EVERYONE_, is more important than individual developer time.

Therefore as much work as possible is pushed down to the end nodes, the developers, to make the top level maintainer's job easier.

You may be offended, but this is exactly how things are and have been for a very long time.