[U-Boot] [PATCH 02/10] mx5: Use explicit clock gate names

[Benoît Thébaudeau]

Dear Marek Vasut,

> Dear Benoît Thébaudeau,
> 
> > Use clock gate definitions having names showing clearly the gated
> > clock
> > instead of names giving only a register field index.
> > 
> > This change reveals that the USB PHY clock functions were broken on
> > i.MX51,
> > so this patch fixes those too.
> 
> Expanding CC /wrt this mx51 usb biz.
> 
> > Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau at advansee.com>
> > Cc: Stefano Babic <sbabic at denx.de>
> > Cc: Marek Vasut <marex at denx.de>
> > ---
> >  .../arch/arm/cpu/armv7/mx5/clock.c                 |   39 ++++-
> >  .../arch/arm/include/asm/arch-mx5/clock.h          |    7 +
> >  .../arch/arm/include/asm/arch-mx5/crm_regs.h       |  158
> > +++++++++++++++++++- .../drivers/usb/host/ehci-mx5.c
> >                    |
> >   5 +
> >  .../drivers/video/ipu_common.c                     |    2 +-
> >  5 files changed, 200 insertions(+), 11 deletions(-)
> > 
> > diff --git u-boot-4d3c95f.orig/arch/arm/cpu/armv7/mx5/clock.c
> > u-boot-4d3c95f/arch/arm/cpu/armv7/mx5/clock.c index
> > c67c3cf..9b083c0
> > 100644
> > --- u-boot-4d3c95f.orig/arch/arm/cpu/armv7/mx5/clock.c
> > +++ u-boot-4d3c95f/arch/arm/cpu/armv7/mx5/clock.c
> > @@ -110,10 +110,11 @@ void enable_usboh3_clk(unsigned char enable)
> >  	unsigned int reg;
> > 
> >  	reg = readl(&mxc_ccm->CCGR2);
> > +	reg &= ~(MXC_CCM_CCGR_CG_MASK <<
> > MXC_CCM_CCGR2_USBOH3_60M_OFFSET);
> 
> What's this addition?

It's to clear this 2-bit bit-field before setting it.

> >  	if (enable)
> > -		reg |= 1 << MXC_CCM_CCGR2_CG14_OFFSET;
> > +		reg |= MXC_CCM_CCGR_CG_ON << MXC_CCM_CCGR2_USBOH3_60M_OFFSET;
> 
> Can't you make it like ... MXC_CCM_CCGR2_USBOH3_60M(<on/off>) ...
> which would
> emit the correct bit? So you'd wrap the bitshift into it as well ...
> also, using
> clrsetbits_le32() won't hurt here.

It's only to be consistent with the existing code. A cosmetic patch could be
applied after that for the whole file.

> >  	else
> > -		reg &= ~(1 << MXC_CCM_CCGR2_CG14_OFFSET);
> > +		reg |= MXC_CCM_CCGR_CG_OFF << MXC_CCM_CCGR2_USBOH3_60M_OFFSET;
> >  	writel(reg, &mxc_ccm->CCGR2);
> >  }
> > 
> > @@ -137,6 +138,29 @@ int enable_i2c_clk(unsigned char enable,
> > unsigned
> > i2c_num) }
> >  #endif
> > 
> > +#if defined(CONFIG_MX51)
> > +void set_usb_phy_clk(void)
> > +{
> > +	unsigned int reg;
> > +
> > +	reg = readl(&mxc_ccm->cscmr1);
> > +	reg &= ~MXC_CCM_CSCMR1_USB_PHY_CLK_SEL;
> > +	writel(reg, &mxc_ccm->cscmr1);
> 
> clrbits_le32() etc., please fix globally.

Ditto.

> This doesn't even fit into this patch, so please split away.
> [...]

These cosmetic changes are off topic for my series. I don't have much time to
work on that. Perhaps this can be done by someone else later.

Best regards,
Benoît