[PATCH V2] net: dsa: ksz: Add reset GPIO handling

[PATCH V2] net: dsa: ksz: Add reset GPIO handling

[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.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Cc: Woojung Huh <woojung.huh@microchip.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
---
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(+)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 9705808c3af7a..3b12e2dcff31b 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -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);
 
diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
index a38ff0841ed4e..60b49010904bf 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -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;
-- 
2.18.0

Re: [PATCH V2] net: dsa: ksz: Add reset GPIO handling

[Andrew Lunn]

On Fri, Dec 07, 2018 at 10:51:36PM +0100, Marek Vasut wrote:
> 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

Re: [PATCH V2] net: dsa: ksz: Add reset GPIO handling

[Marek Vasut]

On 12/07/2018 11:24 PM, Andrew Lunn wrote:
> On Fri, Dec 07, 2018 at 10:51:36PM +0100, Marek Vasut wrote:
>> 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,

> 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 ?

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


-- 
Best regards,
Marek Vasut

Re: [PATCH V2] net: dsa: ksz: Add reset GPIO handling

[David Miller]

From: Marek Vasut <marex@denx.de>
Date: Fri, 7 Dec 2018 23:59:58 +0100

> On 12/07/2018 11:24 PM, Andrew Lunn wrote:
>> On Fri, Dec 07, 2018 at 10:51:36PM +0100, Marek Vasut wrote:
>>> 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,
> 
>> 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.

Re: [PATCH V2] net: dsa: ksz: Add reset GPIO handling

[Marek Vasut]

On 12/08/2018 12:46 AM, David Miller wrote:
> From: Marek Vasut <marex@denx.de>
> Date: Fri, 7 Dec 2018 23:59:58 +0100
> 
>> On 12/07/2018 11:24 PM, Andrew Lunn wrote:
>>> On Fri, Dec 07, 2018 at 10:51:36PM +0100, Marek Vasut wrote:
>>>> 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,
>>
>>> 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.

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

Sure

-- 
Best regards,
Marek Vasut

Re: [PATCH V2] net: dsa: ksz: Add reset GPIO handling

[Andrew Lunn]

> 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

Re: [PATCH V2] net: dsa: ksz: Add reset GPIO handling

[Marek Vasut]

On 12/08/2018 12:11 PM, Andrew Lunn wrote:
>> 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

> 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 ?

>, 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
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH V2] net: dsa: ksz: Add reset GPIO handling

[Andrew Lunn]

On Mon, Dec 10, 2018 at 02:26:51PM +0100, Marek Vasut wrote:
> On 12/08/2018 12:11 PM, Andrew Lunn wrote:
> >> 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
> 
> > 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 ?

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

Re: [PATCH V2] net: dsa: ksz: Add reset GPIO handling

[David Miller]

From: Marek Vasut <marex@denx.de>
Date: Mon, 10 Dec 2018 14:26:51 +0100

> 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.

Re: [PATCH V2] net: dsa: ksz: Add reset GPIO handling

[Marek Vasut]

On 12/10/2018 03:37 PM, Andrew Lunn wrote:
> On Mon, Dec 10, 2018 at 02:26:51PM +0100, Marek Vasut wrote:
>> On 12/08/2018 12:11 PM, Andrew Lunn wrote:
>>>> 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
>>
>>> 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 ?
> 
> Hi Marek

Hello Andrew,

> 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.

> 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.

> 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.

-- 
Best regards,
Marek Vasut