From mboxrd@z Thu Jan  1 00:00:00 1970
From: Chunfeng Yun <chunfeng.yun@mediatek.com>
Date: Wed, 11 Mar 2020 16:17:12 +0800
Subject: [PATCH 5/8] xhci: mediatek: Add support for MTK xHCI host
 controller
In-Reply-To: <2a096d71-d4a3-a693-8822-791448f64f99@denx.de>
References: <1583909445-27444-1-git-send-email-chunfeng.yun@mediatek.com>
 <1583909445-27444-6-git-send-email-chunfeng.yun@mediatek.com>
 <2a096d71-d4a3-a693-8822-791448f64f99@denx.de>
Message-ID: <1583914632.25913.1.camel@mhfsdcap03>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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
> [...]
>