usb: phy: tegra: Program new PHY parameters
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
Quoted 12 lines · Dan Carpenter
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.c101 #define UTMIP_XCVR_HSSLEW(x) (((x) & 0x3) << 4) 102 #define UTMIP_XCVR_HSSLEW_MSB(x) ((((x) & 0x1fc) >> 2) << 25)
Quoted 6 lines
[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
Quoted 13 lines
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
0xffand not0x1fc. Also0xfcand0x03are symetric so I think0xfcwas 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
Quoted 6 lines · Dan Carpenter
Quoted 5 lines · Dan Carpenter
Hello Tuomas Tynkkynen,
The patch
e497a24d8e18: "usb: phy: tegra: Program new PHY parameters" from Aug 12, 2013, leads to the following static checker warning:
Quoted 6 lines
Quoted 6 lines
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.c101 #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?
Quoted 28 lines
Quoted 7 lines
[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 carpenterQuoted 13 lines
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
0xffand not0x1fc. Also0xfcand0x03are symetric so I think0xfcwas 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