[PATCH 3/3] mx51: Move OTG initialisation for all boards to a single file
u.kleine-koenig at pengutronix.de
Thu Oct 7 08:22:30 EDT 2010
On Thu, Oct 07, 2010 at 03:04:59PM +0300, Amit Kucheria wrote:
> On 10 Oct 07, Uwe Kleine-König wrote:
> > On Thu, Oct 07, 2010 at 03:58:48AM +0300, Amit Kucheria wrote:
> > > + /* Set the PHY clock to 24 MHz */
> > > + v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> > > + v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
> > I wonder why a field in a register called MXC_USB_PHY_CTR_FUNC2 is named
> > MX5_USB_UTMI_PHYCTRL1_PLLDIV. Usually the register name is a prefix for
> > the field name?!
> I have a patch renaming the various bit-fields and registers to conform to
> the reference manual. I'll post it soon.
> > > + v |= MX51_USB_PLL_DIV_24_MHZ;
> > hmm, babbage used
> > > - v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
> > > - v |= MX51_USB_PLL_DIV_19_2_MHZ;
> > ?
> Good catch :)
> But if you look at the reference manual, table 60-6, 19.2 MHz would use a
> value of 0x0, but the actual code uses a value of 0x1 that corresponds to
> 24MHz. 19.2MHz (0x0) doesn't even work for me.
Then maybe note it in the commit log?!
> > Maybe you want to reorder your patches such that your patch 2 comes
> > after this one, as currently this patch removes ~80% of the code patch 2
> > introduces.
> I did it to make bisection easier. I didn't want to add a new board feature
> and do consolidation in a single patch.
Yep, that is ok. Currently you have:
1/3: add board support for iforgotthename
2/3: add usb support for iforgotthename
3/3: consolidate usb support
I suggested to do 1, 3, 2 instead. Thinking again 3, 1+2 might even be
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
More information about the linux-arm-kernel