On Mon, Feb 19, 2024 at 08:44:42PM -0300, Luiz Angelo Daros de Luca wrote:
> +void rtl83xx_reset_assert(struct realtek_priv *priv)
> +{
> + int ret;
> +
> + ret = reset_control_assert(priv->reset_ctl);
> + if (!ret)
> + return;
If priv->reset_ctl is NULL - i.e. if no DT property is specified - then
this will always return early and the GPIO will not be asserted.
> +
> + dev_warn(priv->dev,
> + "Failed to assert the switch reset control: %pe\n",
> + ERR_PTR(ret));
You only log an error if the reset controller assert fails, but not if
the GPIO assert fails. Why the unequal treatment?
I suggest keeping it simple:
void rtl83xx_reset_assert(struct realtek_priv *priv)
{
int ret;
ret = reset_control_assert(priv->reset_ctl);
if (ret)
dev_warn(priv->dev, "failed to assert reset control: %d\n", ret);
ret = gpiod_set_value(priv->reset, false);
if (ret)
dev_warn(priv->dev, "failed to assert reset GPIO: %d\n", ret);
}
or even drop the warnings altogether.
> +
> + gpiod_set_value(priv->reset, true);
> +}
> +
> +void rtl83xx_reset_deassert(struct realtek_priv *priv)
> +{
> + int ret;
> +
> + ret = reset_control_deassert(priv->reset_ctl);
> + if (!ret)
> + return;
> +
> + dev_warn(priv->dev,
> + "Failed to deassert the switch reset control: %pe\n",
> + ERR_PTR(ret));
> +
> + gpiod_set_value(priv->reset, false);
> +}
Same comments apply to this function. Just deassert both.
> +
> MODULE_AUTHOR("Luiz Angelo Daros de Luca <luizluca@xxxxxxxxx>");
> MODULE_AUTHOR("Linus Walleij <linus.walleij@xxxxxxxxxx>");
> MODULE_DESCRIPTION("Realtek DSA switches common module");
> diff --git a/drivers/net/dsa/realtek/rtl83xx.h b/drivers/net/dsa/realtek/rtl83xx.h
> index 0ddff33df6b0..c8a0ff8fd75e 100644
> --- a/drivers/net/dsa/realtek/rtl83xx.h
> +++ b/drivers/net/dsa/realtek/rtl83xx.h
> @@ -18,5 +18,7 @@ int rtl83xx_register_switch(struct realtek_priv *priv);
> void rtl83xx_unregister_switch(struct realtek_priv *priv);
> void rtl83xx_shutdown(struct realtek_priv *priv);
> void rtl83xx_remove(struct realtek_priv *priv);
> +void rtl83xx_reset_assert(struct realtek_priv *priv);
> +void rtl83xx_reset_deassert(struct realtek_priv *priv);
>
> #endif /* _RTL83XX_H */
>
> --
> 2.43.0
>
