Re: [PATCH 3/3] phy: amlogic: meson8b-usb2: fix shared reset control use

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Martin,

Thank you for the review !

On 14/11/2020 20:11, Martin Blumenstingl wrote:

Hi Amjad,

On Fri, Nov 13, 2020 at 1:07 AM Amjad Ouled-Ameur
<aouledameur@xxxxxxxxxxxx> wrote:
[...]
         ret = clk_prepare_enable(priv->clk_usb);
         if (ret) {
                 dev_err(&phy->dev, "Failed to enable USB DDR clock\n");
+               reset_control_rearm(priv->reset);
this should come after reset_control_rearm so we're cleaning up in
reverse order of initializing things
(in this case it probably makes no difference since
reset_control_rearm is not touching any registers, but I'd still have
it in the correct order to not confuse future developers)

Agreed, it works in this current order since the two lines do not
interfere with each other, but it is cleaner to do it in the reverse
order of initialization. Will fix it in next change.

                 clk_disable_unprepare(priv->clk_usb_general);
                 return ret;
         }
@@ -197,6 +199,7 @@ static int phy_meson8b_usb2_power_on(struct phy *phy)
                         regmap_read(priv->regmap, REG_ADP_BC, &reg);
                         if (reg & REG_ADP_BC_ACA_PIN_FLOAT) {
                                 dev_warn(&phy->dev, "USB ID detect failed!\n");
+                               reset_control_rearm(priv->reset);
same here, reset_control_rearm should be after clk_disable_unprepare

Ditto, will fix it in next change.


                                 clk_disable_unprepare(priv->clk_usb);
                                 clk_disable_unprepare(priv->clk_usb_general);
                                 return -EINVAL;
@@ -216,6 +219,7 @@ static int phy_meson8b_usb2_power_off(struct phy *phy)
                                    REG_DBG_UART_SET_IDDQ,
                                    REG_DBG_UART_SET_IDDQ);

+       reset_control_rearm(priv->reset);
same here, reset_control_rearm should be after clk_disable_unprepare

Ditto, will fix it in next change.


         clk_disable_unprepare(priv->clk_usb);
         clk_disable_unprepare(priv->clk_usb_general);

Best regards,
Martin


Sincerely,
Amjad




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux