* re: usb: phy: tegra: Program new PHY parameters
@ 2014-09-04 14:10 Dan Carpenter
2014-09-04 14:20 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2014-09-04 14:10 UTC (permalink / raw)
To: ttynkkynen-DDmLM1+adcrQT0dZR+AlfA; +Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA
Hello Tuomas Tynkkynen,
The patch e497a24d8e18: "usb: phy: tegra: Program new PHY parameters"
from Aug 12, 2013, leads to the following static checker warning:
drivers/usb/phy/phy-tegra-usb.c:460 utmi_phy_power_on()
warn: 0x1fc is larger than 8 bits
drivers/usb/phy/phy-tegra-usb.c
101 #define UTMIP_XCVR_HSSLEW(x) (((x) & 0x3) << 4)
102 #define UTMIP_XCVR_HSSLEW_MSB(x) ((((x) & 0x1fc) >> 2) << 25)
[snip]
455 val |= UTMIP_XCVR_LSRSLEW(config->xcvr_lsrslew);
456
457 if (phy->soc_config->requires_extra_tuning_parameters) {
458 val &= ~(UTMIP_XCVR_HSSLEW(~0) | UTMIP_XCVR_HSSLEW_MSB(~0));
459 val |= UTMIP_XCVR_HSSLEW(config->xcvr_hsslew);
460 val |= UTMIP_XCVR_HSSLEW_MSB(config->xcvr_hsslew);
^^^^^^^^^^^^^^^^^^^
This is a u8 so it can only go up to 0xff and not 0x1fc. Also 0xfc and
0x03 are symetric so I think 0xfc was probably intended?
But I don't know the hardware at all, this is pure guess work.
461 }
462 writel(val, base + UTMIP_XCVR_CFG0);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: usb: phy: tegra: Program new PHY parameters
2014-09-04 14:10 usb: phy: tegra: Program new PHY parameters Dan Carpenter
@ 2014-09-04 14:20 ` Dan Carpenter
2014-09-04 15:19 ` Stephen Warren
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2014-09-04 14:20 UTC (permalink / raw)
To: ttynkkynen-DDmLM1+adcrQT0dZR+AlfA; +Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA
On Thu, Sep 04, 2014 at 05:10:02PM +0300, Dan Carpenter wrote:
> Hello Tuomas Tynkkynen,
>
> The patch e497a24d8e18: "usb: phy: tegra: Program new PHY parameters"
> from Aug 12, 2013, leads to the following static checker warning:
>
> drivers/usb/phy/phy-tegra-usb.c:460 utmi_phy_power_on()
> warn: 0x1fc is larger than 8 bits
>
> drivers/usb/phy/phy-tegra-usb.c
> 101 #define UTMIP_XCVR_HSSLEW(x) (((x) & 0x3) << 4)
> 102 #define UTMIP_XCVR_HSSLEW_MSB(x) ((((x) & 0x1fc) >> 2) << 25)
>
> [snip]
>
> 455 val |= UTMIP_XCVR_LSRSLEW(config->xcvr_lsrslew);
> 456
> 457 if (phy->soc_config->requires_extra_tuning_parameters) {
> 458 val &= ~(UTMIP_XCVR_HSSLEW(~0) | UTMIP_XCVR_HSSLEW_MSB(~0));
Btw, the only place where this matters is right here. We end up masking
out a bit which we had not intended. The extra '1' doesn't make a
difference to the line below which generated the static checker warning.
regards,
dan carpenter
> 459 val |= UTMIP_XCVR_HSSLEW(config->xcvr_hsslew);
> 460 val |= UTMIP_XCVR_HSSLEW_MSB(config->xcvr_hsslew);
> ^^^^^^^^^^^^^^^^^^^
> This is a u8 so it can only go up to 0xff and not 0x1fc. Also 0xfc and
> 0x03 are symetric so I think 0xfc was probably intended?
>
> But I don't know the hardware at all, this is pure guess work.
>
> 461 }
> 462 writel(val, base + UTMIP_XCVR_CFG0);
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: usb: phy: tegra: Program new PHY parameters
2014-09-04 14:20 ` Dan Carpenter
@ 2014-09-04 15:19 ` Stephen Warren
0 siblings, 0 replies; 3+ messages in thread
From: Stephen Warren @ 2014-09-04 15:19 UTC (permalink / raw)
To: Dan Carpenter, ttynkkynen-DDmLM1+adcrQT0dZR+AlfA
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA
On 09/04/2014 08:20 AM, Dan Carpenter wrote:
> On Thu, Sep 04, 2014 at 05:10:02PM +0300, Dan Carpenter wrote:
>> Hello Tuomas Tynkkynen,
>>
>> The patch e497a24d8e18: "usb: phy: tegra: Program new PHY parameters"
>> from Aug 12, 2013, leads to the following static checker warning:
>>
>> drivers/usb/phy/phy-tegra-usb.c:460 utmi_phy_power_on()
>> warn: 0x1fc is larger than 8 bits
>>
>> drivers/usb/phy/phy-tegra-usb.c
>> 101 #define UTMIP_XCVR_HSSLEW(x) (((x) & 0x3) << 4)
>> 102 #define UTMIP_XCVR_HSSLEW_MSB(x) ((((x) & 0x1fc) >> 2) << 25)
I believe HSSLEW_MSB should use a mask of 0x3fc, since it's an 8-bit
field. I believe the correct fix for the problem you reported is to
change struct tegra_utmip_config's xcvr_hsslew field to a u16.
(Sigh, this means changing the prototype of read_utmi_param() too, or
adding separate u8/u16 versions of it)
Would you like me to send a patch for this?
>>
>> [snip]
>>
>> 455 val |= UTMIP_XCVR_LSRSLEW(config->xcvr_lsrslew);
>> 456
>> 457 if (phy->soc_config->requires_extra_tuning_parameters) {
>> 458 val &= ~(UTMIP_XCVR_HSSLEW(~0) | UTMIP_XCVR_HSSLEW_MSB(~0));
>
> Btw, the only place where this matters is right here. We end up masking
> out a bit which we had not intended. The extra '1' doesn't make a
> difference to the line below which generated the static checker warning.
>
> regards,
> dan carpenter
>
>> 459 val |= UTMIP_XCVR_HSSLEW(config->xcvr_hsslew);
>> 460 val |= UTMIP_XCVR_HSSLEW_MSB(config->xcvr_hsslew);
>> ^^^^^^^^^^^^^^^^^^^
>> This is a u8 so it can only go up to 0xff and not 0x1fc. Also 0xfc and
>> 0x03 are symetric so I think 0xfc was probably intended?
>>
>> But I don't know the hardware at all, this is pure guess work.
>>
>> 461 }
>> 462 writel(val, base + UTMIP_XCVR_CFG0);
>>
>> regards,
>> dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-09-04 15:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-04 14:10 usb: phy: tegra: Program new PHY parameters Dan Carpenter
2014-09-04 14:20 ` Dan Carpenter
2014-09-04 15:19 ` Stephen Warren
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).