usb: phy: tegra: HSIC adjustments

[PATCH v1 2/4] usb: phy: tegra: return error value from utmi_wait_register

Posted by Svyatoslav Ryhel 6 days, 15 hours ago

Return exact error value from utmi_wait_register during HSIC power on.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/usb/phy/phy-tegra-usb.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
index 3a7a74f01d1c..6173b240c3ea 100644
--- a/drivers/usb/phy/phy-tegra-usb.c
+++ b/drivers/usb/phy/phy-tegra-usb.c
@@ -891,6 +891,7 @@ static int uhsic_phy_power_on(struct tegra_usb_phy *phy)
 	struct tegra_utmip_config *config = phy->config;
 	void __iomem *base = phy->regs;
 	u32 val;
+	int err = 0;
 
 	val = tegra_hsic_readl(phy, UHSIC_PADS_CFG1);
 	val &= ~(UHSIC_PD_BG | UHSIC_PD_TX | UHSIC_PD_TRK | UHSIC_PD_RX |
@@ -984,12 +985,14 @@ static int uhsic_phy_power_on(struct tegra_usb_phy *phy)
 	val |= UHSIC_TX_RTUNE(phy->soc_config->uhsic_tx_rtune);
 	tegra_hsic_writel(phy, UHSIC_PADS_CFG0, val);
 
-	if (utmi_wait_register(base + USB_SUSP_CTRL, USB_PHY_CLK_VALID,
-			       USB_PHY_CLK_VALID))
+	err = utmi_wait_register(base + USB_SUSP_CTRL, USB_PHY_CLK_VALID,
+				 USB_PHY_CLK_VALID);
+
+	if (err)
 		dev_err(phy->u_phy.dev,
 			"Timeout waiting for PHY to stabilize on enable (HSIC)\n");
 
-	return 0;
+	return err;
 }
 
 static int uhsic_phy_power_off(struct tegra_usb_phy *phy)
-- 
2.51.0

Re: [PATCH v1 2/4] usb: phy: tegra: return error value from utmi_wait_register

Posted by Sergey Shtylyov 6 days, 11 hours ago

On 2/2/26 11:05 AM, Svyatoslav Ryhel wrote:

> Return exact error value from utmi_wait_register during HSIC power on.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/usb/phy/phy-tegra-usb.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
> index 3a7a74f01d1c..6173b240c3ea 100644
> --- a/drivers/usb/phy/phy-tegra-usb.c
> +++ b/drivers/usb/phy/phy-tegra-usb.c
> @@ -891,6 +891,7 @@ static int uhsic_phy_power_on(struct tegra_usb_phy *phy)
>  	struct tegra_utmip_config *config = phy->config;
>  	void __iomem *base = phy->regs;
>  	u32 val;
> +	int err = 0;

   This initialization seems pointless -- the newly added variable gets overwritten
by you later...

[...]
> @@ -984,12 +985,14 @@ static int uhsic_phy_power_on(struct tegra_usb_phy *phy)
>  	val |= UHSIC_TX_RTUNE(phy->soc_config->uhsic_tx_rtune);
>  	tegra_hsic_writel(phy, UHSIC_PADS_CFG0, val);
>  
> -	if (utmi_wait_register(base + USB_SUSP_CTRL, USB_PHY_CLK_VALID,
> -			       USB_PHY_CLK_VALID))
> +	err = utmi_wait_register(base + USB_SUSP_CTRL, USB_PHY_CLK_VALID,
> +				 USB_PHY_CLK_VALID);
> +
> +	if (err)
>  		dev_err(phy->u_phy.dev,
>  			"Timeout waiting for PHY to stabilize on enable (HSIC)\n");
>  
> -	return 0;
> +	return err;
>  }
>  
>  static int uhsic_phy_power_off(struct tegra_usb_phy *phy)

MBR, Sergey

Re: [PATCH v1 2/4] usb: phy: tegra: return error value from utmi_wait_register

Posted by Svyatoslav Ryhel 6 days, 10 hours ago

пн, 2 лют. 2026 р. о 14:05 Sergey Shtylyov <sergei.shtylyov@gmail.com> пише:
>
> On 2/2/26 11:05 AM, Svyatoslav Ryhel wrote:
>
> > Return exact error value from utmi_wait_register during HSIC power on.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> >  drivers/usb/phy/phy-tegra-usb.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
> > index 3a7a74f01d1c..6173b240c3ea 100644
> > --- a/drivers/usb/phy/phy-tegra-usb.c
> > +++ b/drivers/usb/phy/phy-tegra-usb.c
> > @@ -891,6 +891,7 @@ static int uhsic_phy_power_on(struct tegra_usb_phy *phy)
> >       struct tegra_utmip_config *config = phy->config;
> >       void __iomem *base = phy->regs;
> >       u32 val;
> > +     int err = 0;
>
>    This initialization seems pointless -- the newly added variable gets overwritten
> by you later...
>

So? let it better be initialized and rewritten then later on catch errors.

> [...]
> > @@ -984,12 +985,14 @@ static int uhsic_phy_power_on(struct tegra_usb_phy *phy)
> >       val |= UHSIC_TX_RTUNE(phy->soc_config->uhsic_tx_rtune);
> >       tegra_hsic_writel(phy, UHSIC_PADS_CFG0, val);
> >
> > -     if (utmi_wait_register(base + USB_SUSP_CTRL, USB_PHY_CLK_VALID,
> > -                            USB_PHY_CLK_VALID))
> > +     err = utmi_wait_register(base + USB_SUSP_CTRL, USB_PHY_CLK_VALID,
> > +                              USB_PHY_CLK_VALID);
> > +
> > +     if (err)
> >               dev_err(phy->u_phy.dev,
> >                       "Timeout waiting for PHY to stabilize on enable (HSIC)\n");
> >
> > -     return 0;
> > +     return err;
> >  }
> >
> >  static int uhsic_phy_power_off(struct tegra_usb_phy *phy)
>
> MBR, Sergey
>

Re: [PATCH v1 2/4] usb: phy: tegra: return error value from utmi_wait_register

Posted by Sergey Shtylyov 6 days, 5 hours ago

On 2/2/26 3:14 PM, Svyatoslav Ryhel wrote:
> пн, 2 лют. 2026 р. о 14:05 Sergey Shtylyov <sergei.shtylyov@gmail.com> пише:
>>
>> On 2/2/26 11:05 AM, Svyatoslav Ryhel wrote:
>>
>>> Return exact error value from utmi_wait_register during HSIC power on.
>>>
>>> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
>>> ---
>>>  drivers/usb/phy/phy-tegra-usb.c | 9 ++++++---
>>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
>>> index 3a7a74f01d1c..6173b240c3ea 100644
>>> --- a/drivers/usb/phy/phy-tegra-usb.c
>>> +++ b/drivers/usb/phy/phy-tegra-usb.c
>>> @@ -891,6 +891,7 @@ static int uhsic_phy_power_on(struct tegra_usb_phy *phy)
>>>       struct tegra_utmip_config *config = phy->config;
>>>       void __iomem *base = phy->regs;
>>>       u32 val;
>>> +     int err = 0;
>>
>>    This initialization seems pointless -- the newly added variable gets overwritten
>> by you later...
>>
> 
> So? let it better be initialized and rewritten then later on catch errors.

   I'm not sure what errors you mean here. To me, it (contrariwise) seems to
mask the possible errors when you forget to set err to e.g. -ENOMEM before
returning (when adding a call to kmalloc() or any other function that doesn't
return an error code itself)...
   I'm pretty sure gcc will drop this initialization when generating the object
code and (what's worse) the static analyzers will trip on this code telling you
that the value 0 is unused...

[...]

MBR, Sergey

Re: [PATCH v1 2/4] usb: phy: tegra: return error value from utmi_wait_register

Posted by Sergey Shtylyov 6 days, 3 hours ago

On 2/2/26 9:01 PM, Sergey Shtylyov wrote:
[...]

>>>> Return exact error value from utmi_wait_register during HSIC power on.
>>>>
>>>> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
>>>> ---
>>>>  drivers/usb/phy/phy-tegra-usb.c | 9 ++++++---
>>>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
>>>> index 3a7a74f01d1c..6173b240c3ea 100644
>>>> --- a/drivers/usb/phy/phy-tegra-usb.c
>>>> +++ b/drivers/usb/phy/phy-tegra-usb.c
>>>> @@ -891,6 +891,7 @@ static int uhsic_phy_power_on(struct tegra_usb_phy *phy)
>>>>       struct tegra_utmip_config *config = phy->config;
>>>>       void __iomem *base = phy->regs;
>>>>       u32 val;
>>>> +     int err = 0;
>>>
>>>    This initialization seems pointless -- the newly added variable gets overwritten
>>> by you later...
>>>

[...]

>    I'm pretty sure gcc will drop this initialization when generating the object
> code and (what's worse)

   Well, that's actually the good news. I've tried to feed an analogous code to
gcc and clang -- and both seemed to drop the initialization.  I was not able to
make them complain using C=1 and C=2 with make...

> the static analyzers will trip on this code telling you
> that the value 0 is unused...

   Svace (that we have to use here) surely bitches about that. :-)

[...]

MBR, Sergey