| 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
| ||
|
Message-ID: <dd9da345-e056-4f34-8e39-6901bf9c1636@microchip.com>
Date: Mon, 29 Apr 2024 06:09:09 +0000
From: <Parthiban.Veerasooran@...rochip.com>
To: <ramon.nordin.rodriguez@...roamp.se>, <andrew@...n.ch>
CC: <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
<pabeni@...hat.com>, <horms@...nel.org>, <saeedm@...dia.com>,
<anthony.l.nguyen@...el.com>, <netdev@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <andrew@...n.ch>, <corbet@....net>,
<linux-doc@...r.kernel.org>, <robh+dt@...nel.org>,
<krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
<devicetree@...r.kernel.org>, <Horatiu.Vultur@...rochip.com>,
<ruanjinjie@...wei.com>, <Steen.Hegelund@...rochip.com>,
<vladimir.oltean@....com>, <UNGLinuxDriver@...rochip.com>,
<Thorsten.Kummermehr@...rochip.com>, <Pier.Beruto@...emi.com>,
<Selvamani.Rajagopal@...emi.com>, <Nicolas.Ferre@...rochip.com>,
<benjamin.bigler@...nformulastudent.ch>
Subject: Re: [PATCH net-next v4 13/12] net: lan865x: optional hardware reset
Hi Ramon,
On 29/04/24 2:46 am, Ramón Nordin Rodriguez wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> From c65e42982684d5fd8b2294eb6acf755aa0fcab83 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Ram=C3=B3n=20Nordin=20Rodriguez?=
> <ramon.nordin.rodriguez@...roamp.se>
> Date: Sun, 28 Apr 2024 22:25:12 +0200
> Subject: [PATCH net-next v4 13/12] net: lan865x: optional hardware reset
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> This commit optionally enables a hardware reset of the lan8650/1
> mac-phy. These chips have a software reset that is discourage from use
> in the manual since it only resets the internal phy.
The software reset done by the current driver is not only resetting the
internal PHY, it resets the entire MAC-PHY including the integrated PHY.
The reset bit of the Clause 22 basic control register only will reset
the internal PHY alone. But oa_tc6_sw_reset_macphy() function is writing
software reset bit in the Reset Control and Status register which resets
the entire MAC-PHY including the internal PHY.
OPEN Alliance spec says the following which is done in the
oa_tc6_sw_reset_macphy().
9.2.4
Reset Control and Status Register (0x0003)
9.2.4.1 RSVD
Reserved for future use. Writing to these bits shall have no effect on
the MAC-PHY.
9.2.4.2 SWRESET
MAC-PHY Software Reset. The action of writing a ‘1’ to this bit shall
fully reset the MAC-PHY, including the integrated PHY, to an initial
state including but not limited to resetting all state machines and
registers to their default value. When this bit is set, the reset shall
not occur until CSn is deasserted to allow for the control command write
to complete. This bit is self-clearing.
LAN8650 spec says the following,
4.1.1.3 Software Reset
A software reset of the LAN8650/1 is available via the Soft Reset
(SW_RESET) bit in the standard OA_CONFIG0 register.
Note: The SW_RESET bit of the Clause 22 Basic Control register will
reset only the internal PHY, not the entire device. This PHY only reset
is not recommended for use. If such a reset is detected, by reading the
RESETC bit of the STS2 register, reset the entire device.
The above note is given in the lan8650 datasheet to let the user to know
that clause 22 software reset will reset only internal PHY but I don't
think they mean it for the MAC-PHY software reset done from Reset
Control and Status register.
So in my opinion, I don't see the need of external pin reset as the
existing oa_tc6_sw_reset_macphy() function does the software reset of
the entire MAC-PHY.
Still if you see a need to have this external pin reset as an optional
function then it may be needed for all the vendor specific MAC drivers.
In that case, reset-gpios parameter value alone can be taken from the
chip specific device tree and the remaining code for operating the reset
gpio can be moved to oa_tc6.c and the function name can be
oa_tc6_hw_reset_macphy().
Best regards,
Parthiban V
>
> Signed-off-by: Ramón Nordin Rodriguez <ramon.nordin.rodriguez@...roamp.se>
> ---
> .../bindings/net/microchip,lan865x.yaml | 4 +++
> .../net/ethernet/microchip/lan865x/lan865x.c | 28 +++++++++++++++++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/microchip,lan865x.yaml b/Documentation/devicetree/bindings/net/microchip,lan865x.yaml
> index 4fdec0ba3532..0f11f431df06 100644
> --- a/Documentation/devicetree/bindings/net/microchip,lan865x.yaml
> +++ b/Documentation/devicetree/bindings/net/microchip,lan865x.yaml
> @@ -44,6 +44,9 @@ properties:
> minimum: 15000000
> maximum: 25000000
>
> + reset-gpios:
> + maxItems: 1
> +
> "#address-cells":
> const: 1
>
> @@ -76,5 +79,6 @@ examples:
> interrupts = <6 IRQ_TYPE_EDGE_FALLING>;
> local-mac-address = [04 05 06 01 02 03];
> spi-max-frequency = <15000000>;
> + reset-gpios = <&gpio2 8 GPIO_ACTIVE_HIGH>;
> };
> };
> diff --git a/drivers/net/ethernet/microchip/lan865x/lan865x.c b/drivers/net/ethernet/microchip/lan865x/lan865x.c
> index 9abefa8b9d9f..bed9033574b2 100644
> --- a/drivers/net/ethernet/microchip/lan865x/lan865x.c
> +++ b/drivers/net/ethernet/microchip/lan865x/lan865x.c
> @@ -9,6 +9,7 @@
> #include <linux/kernel.h>
> #include <linux/phy.h>
> #include <linux/oa_tc6.h>
> +#include <linux/gpio/driver.h>
>
> #define DRV_NAME "lan865x"
>
> @@ -33,6 +34,7 @@
>
> struct lan865x_priv {
> struct work_struct multicast_work;
> + struct gpio_desc *reset_gpio;
> struct net_device *netdev;
> struct spi_device *spi;
> struct oa_tc6 *tc6;
> @@ -283,6 +285,24 @@ static int lan865x_set_zarfe(struct lan865x_priv *priv)
> return oa_tc6_write_register(priv->tc6, OA_TC6_REG_CONFIG0, regval);
> }
>
> +static int lan865x_probe_reset_gpio(struct lan865x_priv *priv)
> +{
> + priv->reset_gpio = devm_gpiod_get_optional(&priv->spi->dev, "reset",
> + GPIOD_OUT_HIGH);
> + if (IS_ERR(priv->reset_gpio))
> + return PTR_ERR(priv->reset_gpio);
> +
> + return 0;
> +}
> +
> +static void lan865x_hw_reset(struct lan865x_priv *priv)
> +{
> + gpiod_set_value_cansleep(priv->reset_gpio, 1);
> + // section 9.6.3 RESET_N Timing specifies a minimum hold of 5us
> + usleep_range(5, 10);
> + gpiod_set_value_cansleep(priv->reset_gpio, 0);
> +}
> +
> static int lan865x_probe(struct spi_device *spi)
> {
> struct net_device *netdev;
> @@ -297,6 +317,14 @@ static int lan865x_probe(struct spi_device *spi)
> priv->netdev = netdev;
> priv->spi = spi;
> spi_set_drvdata(spi, priv);
> + if (lan865x_probe_reset_gpio(priv)) {
> + dev_err(&spi->dev, "failed to probe reset pin");
> + ret = -ENODEV;
> + goto free_netdev;
> + }
> +
> + if (priv->reset_gpio)
> + lan865x_hw_reset(priv);
> INIT_WORK(&priv->multicast_work, lan865x_multicast_work_handler);
>
> priv->tc6 = oa_tc6_init(spi, netdev);
> --
> 2.43.0
>
>
Powered by blists - more mailing lists