netdev - Re: [PATCH net-next 2/2] dsa: mv88e6xxx.c: Hardware reset the chip if available

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <564D2F16.1020701@electromag.com.au>
Date:	Thu, 19 Nov 2015 10:08:22 +0800
From:	Phil Reid <preid@...ctromag.com.au>
To:	Andrew Lunn <andrew@...n.ch>, David Miller <davem@...emloft.net>
Cc:	netdev <netdev@...r.kernel.org>,
	Florian Fainelli <f.fainelli@...il.com>,
	Vivien Didelot <vivien.didelot@...oirfairelinux.com>,
	Neil Armstrong <narmstrong@...libre.com>
Subject: Re: [PATCH net-next 2/2] dsa: mv88e6xxx.c: Hardware reset the chip if
 available

On 19/11/2015 7:29 AM, Andrew Lunn wrote:
> The device tree binding now allows a gpio to be specified which is
> attached to the switch chips reset line. If it is defined, perform
> a hardware reset on the switch during setup.
>
> Signed-off-by: Andrew Lunn <andrew@...n.ch>
> ---
>   drivers/net/dsa/mv88e6xxx.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
>
> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
> index b06dba05594a..c0bbbe7713c5 100644
> --- a/drivers/net/dsa/mv88e6xxx.c
> +++ b/drivers/net/dsa/mv88e6xxx.c
> @@ -19,6 +19,7 @@
>   #include <linux/list.h>
>   #include <linux/module.h>
>   #include <linux/netdevice.h>
> +#include <linux/gpio/consumer.h>
>   #include <linux/phy.h>
>   #include <net/dsa.h>
>   #include <net/switchdev.h>
> @@ -2323,7 +2324,10 @@ int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active)
>   {
>   	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
>   	u16 is_reset = (ppu_active ? 0x8800 : 0xc800);
> +	int gpio = ds->pd->reset;
> +	int flags = ds->pd->reset_flags;
>   	unsigned long timeout;
> +	int on = 1;
>   	int ret;
>   	int i;
>
> @@ -2336,6 +2340,16 @@ int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active)
>   	/* Wait for transmit queues to drain. */
>   	usleep_range(2000, 4000);
>
> +	/* If there is a gpio connected to the reset pin, toggle it */
> +	if (gpio_is_valid(gpio)) {
> +		if (flags && OF_GPIO_ACTIVE_LOW)
> +			on = !on;
> +		gpio_set_value_cansleep(gpio, on);
> +		usleep_range(10000, 20000);
> +		gpio_set_value_cansleep(gpio, !on);
> +		usleep_range(10000, 20000);
> +	}
> +
This is a general query about what is the preferred method of allocating gpios.
The gpiod* family of functions provided similar functionality and automatically
deal with active low / high outputs, direction, inital value  etc...
I raise this more for knowledge on what method I should use for my patches.


>   	/* Reset the switch. Keep the PPU active if requested. The PPU
>   	 * needs to be active to support indirect phy register access
>   	 * through global registers 0x18 and 0x19.
>

Other than that the concept looks good and something I has been looking at adding.

Would it be worth considering placing the chip in reset on driver remove?
I have an battery powered hardware platform using one of this marvell devices and
for certain configurations we don't need the switch active. So unloading the
module to place the device in reset and would save power.
Reloading would reinitialise the port.





-- 
Regards
Phil Reid

ElectroMagnetic Imaging Technology Pty Ltd
Development of Geophysical Instrumentation & Software
www.electromag.com.au

3 The Avenue, Midland WA 6056, AUSTRALIA
Ph: +61 8 9250 8100
Fax: +61 8 9250 7100
Email: preid@...ctromag.com.au
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html