Hi Kishon: On 2014/12/11 14:02, Kishon Vijay Abraham I wrote: > Hi, > > On Wednesday 10 December 2014 04:16 PM, Yunzhi Li wrote: >> This patch to add a generic PHY driver for ROCKCHIP usb PHYs, >> currently this driver can support RK3288. The RK3288 SoC have >> three independent USB PHY IPs which are all configured through a >> set of registers located in the GRF (general register files) >> module. >> >> Signed-off-by: Yunzhi Li <lyz at rock-chips.com> >> >> + >> +#define ROCKCHIP_RK3288_UOC(n) (0x320 + n * 0x14) >> + >> +/* >> + * The higher 16-bit of this register is used for write protection >> + * only if BIT(13 + 16) set to 1 the BIT(13) can be written. >> + */ >> +#define SIDDQ_MSK BIT(13 + 16) > I think here the "MSK" is misleading. it should be something that refers write > protection? So, #define SIDDQ_WRITE_ENA BIT(29) , could be ok ? >> +#define SIDDQ_ON BIT(13) >> +#define SIDDQ_OFF (0 << 13) >> + >> +struct rockchip_usb_phy { >> + struct regmap *reg_base; >> + unsigned int reg_offset; >> + struct clk *clk; >> + struct phy *phy; >> + unsigned index; >> +}; >> + >> +struct rockchip_usb_phy_priv { >> + struct rockchip_usb_phy *phys; >> + unsigned nphys; >> +}; >> + >> +static int rockchip_usb_phy_power(struct rockchip_usb_phy *phy, >> + bool siddq) >> +{ >> + return regmap_write(phy->reg_base, phy->reg_offset, >> + SIDDQ_MSK | (siddq ? SIDDQ_ON : SIDDQ_OFF)); > Shouldn't we actually reset the bit for power off? Sorry, which bit you refer to here and why should it be reset? could you give more infomation. --- Yunzhi Li @ rockchip