On Wed, 2020-03-11 at 08:11 +0100, Marek Vasut wrote:
> On 3/11/20 7:50 AM, Chunfeng Yun wrote:
> [...]
> > + * @u3_ctrl_p[x]: ip usb3 port x control register, only low 4bytes are used
> > + * @u2_ctrl_p[x]: ip usb2 port x control register, only low 4bytes are used
> > + * @u2_phy_pll: usb2 phy pll control register
> > + */
> > +struct mtk_ippc_regs {
> > +   __le32 ip_pw_ctr0;
> > +   __le32 ip_pw_ctr1;
> > +   __le32 ip_pw_ctr2;
> 
> Please define the registers with #define macros , this struct-based
> approach doesn't scale.
Ok
> 
> [..]
> 
> > +static int xhci_mtk_host_enable(struct mtk_xhci *mtk)
> > +{
> > +   struct mtk_ippc_regs *ippc = mtk->ippc;
> > +   u32 value, check_val;
> > +   int ret;
> > +   int i;
> > +
> > +   /* power on host ip */
> > +   value = readl(&ippc->ip_pw_ctr1);
> > +   value &= ~CTRL1_IP_HOST_PDN;
> > +   writel(value, &ippc->ip_pw_ctr1);
> > +
> > +   /* power on and enable all u3 ports */
> > +   for (i = 0; i < mtk->num_u3_ports; i++) {
> > +           value = readl(&ippc->u3_ctrl_p[i]);
> > +           value &= ~(CTRL_U3_PORT_PDN | CTRL_U3_PORT_DIS);
> > +           value |= CTRL_U3_PORT_HOST_SEL;
> > +           writel(value, &ippc->u3_ctrl_p[i]);
> > +   }
> 
> Use clrsetbits_le32() above and below and where applicable.
Ok
> 
> > +   /* power on and enable all u2 ports */
> > +   for (i = 0; i < mtk->num_u2_ports; i++) {
> > +           value = readl(&ippc->u2_ctrl_p[i]);
> > +           value &= ~(CTRL_U2_PORT_PDN | CTRL_U2_PORT_DIS);
> > +           value |= CTRL_U2_PORT_HOST_SEL;
> > +           writel(value, &ippc->u2_ctrl_p[i]);
> > +   }
> 
> [...]
> 
> > +static int xhci_mtk_clks_enable(struct mtk_xhci *mtk)
> > +{
> > +   int ret;
> > +
> > +   ret = clk_enable(mtk->sys_clk);
> > +   if (ret) {
> > +           dev_err(mtk->dev, "failed to enable sys_clk\n");
> > +           goto sys_clk_err;
> > +   }
> 
> Can you use clk_enable_bulk() and other clk.*bulk functions ?
Will try it, thank you
> [...]
> 

Reply via email to