mbox series

[net-next,v3,0/3] net: dsa: realtek: support reset controller and update docs

Message ID 20240213-realtek-reset-v3-0-37837e574713@gmail.com
Headers
Series net: dsa: realtek: support reset controller and update docs |

Message

Luiz Angelo Daros de Luca Feb. 14, 2024, 12:54 a.m. UTC 
  The driver previously supported reset pins using GPIO, but it lacked
support for reset controllers. Although a reset method is generally not
required, the driver fails to detect the switch if the reset was kept
asserted by a previous driver.

This series adds support to reset a Realtek switch using a reset
controller. It also updates the binding documentation to remove the
requirement of a reset method and to add the new reset controller
property.

It was tested on a TL-WR1043ND v1 router (rtl8366rb via SMI).

Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---

Changes in v3:
- Rebased on the Realtek DSA driver refactoring (08f627164126)
- Dropped the reset controller example in bindings
- Used %pe in error printing
- Linked to v2: https://lore.kernel.org/r/20231027190910.27044-1-luizluca@gmail.com/

Changes in v2:
- Introduced a dedicated commit for removing the reset-gpios requirement
- Placed binding patches before code changes
- Removed the 'reset-names' property
- Moved the example from the commit message to realtek.yaml
- Split the reset function into _assert/_deassert variants
- Modified reset functions to return a warning instead of a value
- Utilized devm_reset_control_get_optional to prevent failure when the
  reset control is missing
- Used 'true' and 'false' for boolean values
- Removed the CONFIG_RESET_CONTROLLER check as stub methods are
  sufficient when undefined
- Linked to v1: https://lore.kernel.org/r/20231024205805.19314-1-luizluca@gmail.com/

---
Luiz Angelo Daros de Luca (3):
      dt-bindings: net: dsa: realtek: reset-gpios is not required
      dt-bindings: net: dsa: realtek: add reset controller
      net: dsa: realtek: support reset controller

 .../devicetree/bindings/net/dsa/realtek.yaml       |  4 +-
 drivers/net/dsa/realtek/realtek.h                  |  2 +
 drivers/net/dsa/realtek/rtl83xx.c                  | 52 +++++++++++++++++++---
 drivers/net/dsa/realtek/rtl83xx.h                  |  2 +
 4 files changed, 54 insertions(+), 6 deletions(-)
---
base-commit: 0f37666d87d2dea42ec21776c3d562b7cbd71612
change-id: 20240212-realtek-reset-88a0bf25bb22

Best regards,

Comments

Linus Walleij Feb. 14, 2024, 8:04 a.m. UTC | #1 
Hi Luiz,

thanks for your patch!

On Wed, Feb 14, 2024 at 1:54 AM Luiz Angelo Daros de Luca
<luizluca@gmail.com> wrote:

> The 'reset-gpios' will not work when the switch reset is controlled by a
> reset controller.
>
> Although the reset is optional and the driver performs a soft reset
> during setup, if the initial reset state was asserted, the driver will
> not detect it.
>
> The reset controller will take precedence over the reset GPIO.
>
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
(...)
> +void rtl83xx_reset_assert(struct realtek_priv *priv)
> +{
> +       int ret;
> +
> +       if (priv->reset_ctl) {
> +               ret = reset_control_assert(priv->reset_ctl);

Actually, reset_control_assert() is NULL-tolerand (as well as the
stubs) so you can just issue this unconditionally. If priv->reset_ctl
is NULL it will just return 0.

> +               if (!ret)
> +                       return;
> +
> +               dev_warn(priv->dev,
> +                        "Failed to assert the switch reset control: %pe\n",
> +                        ERR_PTR(ret));
> +       }
> +
> +       if (priv->reset)
> +               gpiod_set_value(priv->reset, true);

Same here! Also NULL-tolerant. You do not need to check priv->reset.
Just issue it.

> +void rtl83xx_reset_deassert(struct realtek_priv *priv)

Same comments for deassert.

With this fixed (the rest looks just fine):
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij