| Message ID | 20211205215846.153703-2-aouledameur@baylibre.com |
|---|---|
| State | New |
| Headers | show |
| Series | usb: meson: fix shared reset control use | expand |
Hi Amjad, On Sun, Dec 5, 2021 at 10:59 PM Amjad Ouled-Ameur <aouledameur@baylibre.com> wrote: > > Use reset_control_rearm() call if an error occurs in case > phy_meson_gxl_usb2_init() fails after reset() has been called ; or in case > phy_meson_gxl_usb2_exit() is called i.e the resource is no longer used > and the reset line may be triggered again by other devices. > > reset_control_rearm() keeps use of triggered_count sane in the reset > framework. Therefore, use of reset_control_reset() on shared reset line > should be balanced with reset_control_rearm(). > > Signed-off-by: Amjad Ouled-Ameur <aouledameur@baylibre.com> > Reported-by: Jerome Brunet <jbrunet@baylibre.com> > --- > changes since v3: > - Remove unnecessary reset_control_rearm() after reset_control_reset() > failure. I double-checked your patch in v3 and Philipp was right: reset_control_rearm() should not be right after reset_control_reset(). However, I think reset_control_rearm() is still needed phy_meson_gxl_usb2_init() whenever clk_prepare_enable() fails. So my suggestion is to add reset_control_rearm() in phy_meson_gxl_usb2_init() if clk_prepare_enable() fails so we are resetting the ref-count for the reset line (just like phy_meson_gxl_usb2_exit() does). The difference compared to the previous version is that the reset_control_rearm() call needs to be placed a few lines down. Thank you! Martin
Hi Martin, Thank you for the thorough review. On 06/12/2021 22:19, Martin Blumenstingl wrote: > Hi Amjad, > > On Sun, Dec 5, 2021 at 10:59 PM Amjad Ouled-Ameur > <aouledameur@baylibre.com> wrote: >> Use reset_control_rearm() call if an error occurs in case >> phy_meson_gxl_usb2_init() fails after reset() has been called ; or in case >> phy_meson_gxl_usb2_exit() is called i.e the resource is no longer used >> and the reset line may be triggered again by other devices. >> >> reset_control_rearm() keeps use of triggered_count sane in the reset >> framework. Therefore, use of reset_control_reset() on shared reset line >> should be balanced with reset_control_rearm(). >> >> Signed-off-by: Amjad Ouled-Ameur <aouledameur@baylibre.com> >> Reported-by: Jerome Brunet <jbrunet@baylibre.com> >> --- >> changes since v3: >> - Remove unnecessary reset_control_rearm() after reset_control_reset() >> failure. > I double-checked your patch in v3 and Philipp was right: > reset_control_rearm() should not be right after reset_control_reset(). > However, I think reset_control_rearm() is still needed > phy_meson_gxl_usb2_init() whenever clk_prepare_enable() fails. Well seen, reset_control_rearm() should actually be called after clk_prepare_enable() fails. I will wait for any other potential reviews before sending next version with this change you suggested. Thank you Martin. Amjad > So my suggestion is to add reset_control_rearm() in > phy_meson_gxl_usb2_init() if clk_prepare_enable() fails so we are > resetting the ref-count for the reset line (just like > phy_meson_gxl_usb2_exit() does). > The difference compared to the previous version is that the > reset_control_rearm() call needs to be placed a few lines down. > > > Thank you! > Martin
diff --git a/drivers/phy/amlogic/phy-meson-gxl-usb2.c b/drivers/phy/amlogic/phy-meson-gxl-usb2.c index 2b3c0d730f20..d2f56075dc57 100644 --- a/drivers/phy/amlogic/phy-meson-gxl-usb2.c +++ b/drivers/phy/amlogic/phy-meson-gxl-usb2.c @@ -125,6 +125,7 @@ static int phy_meson_gxl_usb2_exit(struct phy *phy) struct phy_meson_gxl_usb2_priv *priv = phy_get_drvdata(phy); clk_disable_unprepare(priv->clk); + reset_control_rearm(priv->reset); return 0; }
Use reset_control_rearm() call if an error occurs in case phy_meson_gxl_usb2_init() fails after reset() has been called ; or in case phy_meson_gxl_usb2_exit() is called i.e the resource is no longer used and the reset line may be triggered again by other devices. reset_control_rearm() keeps use of triggered_count sane in the reset framework. Therefore, use of reset_control_reset() on shared reset line should be balanced with reset_control_rearm(). Signed-off-by: Amjad Ouled-Ameur <aouledameur@baylibre.com> Reported-by: Jerome Brunet <jbrunet@baylibre.com> --- changes since v3: - Remove unnecessary reset_control_rearm() after reset_control_reset() failure. drivers/phy/amlogic/phy-meson-gxl-usb2.c | 1 + 1 file changed, 1 insertion(+)