Hi Neil, On Thu, Mar 7, 2019 at 9:44 AM Neil Armstrong <narmstrong@xxxxxxxxxxxx> wrote: > > On 06/03/2019 22:04, Martin Blumenstingl wrote: > > Hi Neil, > > > > On Mon, Mar 4, 2019 at 11:40 AM Neil Armstrong <narmstrong@xxxxxxxxxxxx> wrote: > > [...] > >> +static int phy_g12a_usb3_init(struct phy *phy) > >> +{ > >> + struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy); > >> + int data, ret; > >> + > >> + /* Switch PHY to USB3 */ > >> + regmap_update_bits(priv->regmap, PHY_R0, > >> + PHY_R0_PCIE_USB3_SWITCH, > >> + PHY_R0_PCIE_USB3_SWITCH); > > does this automatically clear PHY_R0_PCIE_POWER_STATE (in case the > > bootloader incorrectly set that)? > > Don't forget it's a static configuration, on the board, only USB3 XOR PCIE > will be available, if the bootloader sets this and the kernel uses USB3, > or the reverse, one of them is wrong. I'm specifically asking is because we've seen various GXL/GXM boards with a bootloader which is configured for the wrong PHY interface (RGMII != RMII) > > > > [...] > >> +static int phy_g12a_usb3_pcie_init(struct phy *phy) > >> +{ > >> + struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy); > >> + int ret; > >> + > >> + ret = reset_control_reset(priv->reset); > >> + if (ret) > >> + return ret; > >> + > >> + if (priv->mode == PHY_TYPE_USB3) > >> + return phy_g12a_usb3_init(phy); > >> + > >> + /* Power UP PCIE */ > >> + regmap_update_bits(priv->regmap, PHY_R0, > >> + PHY_R0_PCIE_POWER_STATE, > >> + FIELD_PREP(PHY_R0_PCIE_POWER_STATE, 0x1c)); > > similar to my question above: does this automatically clear > > PHY_R0_PCIE_USB3_SWITCH (in case the bootloader incorrectly set that)? > > Same answer, but I'll investigate to have more details on this register. > > It's more an implementation issue, we can change it when PCIe is enabled > on this platform. I'm fine with postponing this until we bring up PCIe support on this platform if you add a comment (preferably marked with "TODO") in the driver. that makes it obvious to everyone who comes across this in the PHY driver with a TODO-comment you can add my: Reviewed-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> Regards Martin