Messages in this thread | | | Date | Mon, 30 Nov 2020 16:00:23 +0530 | From | Vinod Koul <> | Subject | Re: [PATCH 1/8] phy: phy-hi3670-usb3: move driver from staging into phy |
| |
On 16-11-20, 13:59, Mauro Carvalho Chehab wrote:
> +#define CTRL7_USB2_REFCLKSEL_MASK (3 << 3) > +#define CTRL7_USB2_REFCLKSEL_ABB (3 << 3) > +#define CTRL7_USB2_REFCLKSEL_PAD (2 << 3)
This should use GENMASK() > + > +#define CFG50_USB3_PHY_TEST_POWERDOWN BIT(23) > + > +#define CFG54_USB31PHY_CR_ADDR_MASK (0xFFFF) > +#define CFG54_USB31PHY_CR_ADDR_SHIFT (16)
We can skip this by using FIELD_GET/FIELD_SET macros and only define register fields.
> +static int hi3670_phy_cr_start(struct regmap *usb31misc, int direction) > +{ > + int ret; > + > + if (direction) > + ret = regmap_update_bits(usb31misc, USB_MISC_CFG54, > + CFG54_USB31PHY_CR_WR_EN, > + CFG54_USB31PHY_CR_WR_EN); > + else > + ret = regmap_update_bits(usb31misc, USB_MISC_CFG54, > + CFG54_USB31PHY_CR_RD_EN, > + CFG54_USB31PHY_CR_RD_EN);
how about: if direction reg = CFG54_USB31PHY_CR_WR_EN; else reg = CFG54_USB31PHY_CR_RD_EN;
regmap_update_bits(usb31misc, USB_MISC_CFG54, reg, reg);
> + > + if (ret) > + return ret; > + > + ret = hi3670_phy_cr_clk(usb31misc); > + if (ret) > + return ret; > + > + ret = regmap_update_bits(usb31misc, USB_MISC_CFG54, > + CFG54_USB31PHY_CR_RD_EN | CFG54_USB31PHY_CR_WR_EN, 0); > + > + return ret;
return regmap_update_bits()
> +static int hi3670_phy_cr_wait_ack(struct regmap *usb31misc) > +{ > + u32 reg; > + int retry = 100000; > + int ret; > + > + while (retry-- > 0) { > + ret = regmap_read(usb31misc, USB_MISC_CFG54, ®); > + if (ret) > + return ret; > + if ((reg & CFG54_USB31PHY_CR_ACK) == CFG54_USB31PHY_CR_ACK) > + return 0; > + > + ret = hi3670_phy_cr_clk(usb31misc); > + if (ret) > + return ret;
No delay in between reads..? maybe add a small delay and reduce the retries?
> +static int hi3670_phy_cr_set_addr(struct regmap *usb31misc, u32 addr) > +{ > + u32 reg; > + int ret; > + > + ret = regmap_read(usb31misc, USB_MISC_CFG54, ®); > + if (ret) > + return ret; > + > + reg &= ~(CFG54_USB31PHY_CR_ADDR_MASK << CFG54_USB31PHY_CR_ADDR_SHIFT); > + reg |= ((addr & CFG54_USB31PHY_CR_ADDR_MASK) << CFG54_USB31PHY_CR_ADDR_SHIFT); > + ret = regmap_write(usb31misc, USB_MISC_CFG54, reg);
regmap_update_bits() ?
> +static int hi3670_is_abbclk_seleted(struct hi3670_priv *priv) > +{ > + u32 reg; > + > + if (!priv->sctrl) { > + dev_err(priv->dev, "priv->sctrl is null!\n"); > + return 1; > + } > + > + if (regmap_read(priv->sctrl, SCTRL_SCDEEPSLEEPED, ®)) { > + dev_err(priv->dev, "SCTRL_SCDEEPSLEEPED read failed!\n"); > + return 1;
Not a -ve error code? -- ~Vinod
| |