Hi,
On Wed, Nov 21, 2012 at 02:36:48PM +0200, Roger Quadros wrote:
> >> break;
> >> default:
> >> - dev_err(dev, "TLL version failed\n");
> >> - ret = -ENODEV;
> >> - goto err_ioremap;
> >> + tll->nch = DEFAULT_TLL_CHANNEL_COUNT;
> >> + dev_info(dev,
> >> + "USB TLL Rev : 0x%x not recognized, assuming %d channels\n",
> >> + ver, tll->nch);
> >
> > this hsould be dev_dbg().
> >
>
> I think it should be more of a warning because it signals a problem.
> There is another pr_info in the success path which could probably be a
> dev_dbg.
it's not a problem at all. It just means that a newer OMAP has come to
market and perhaps we don't need extra code to support it. A revision
detection should *never* be grounds to failure. When we can't understand
the revision, we default to the highest possible value we know.
that's not an error.
> >> + struct clk *fck;
> >> +
> >> + sprintf(clk_name, "usb_tll_hs_usb_ch%d_clk", i);
> >
> > this will overflow if 'i' (for whatever reason) goes over 9.
>
> OK i'll add an extra character. Highly unlikely to go above 99 :)
I'd stick to snprintf() though, or something safer.
> >> + fck = clk_get(dev, clk_name);
> >
> > please use devm_clk_get().
sidenote, it would be amazing to a patch at the top of this series
converting to devm_* api ;-)
> >> @@ -373,11 +385,17 @@ static int usbtll_runtime_resume(struct device *dev)
> >>
> >> spin_lock_irqsave(&tll->lock, flags);
> >>
> >> - if (is_ehci_tll_mode(pdata->port_mode[0]))
> >> - clk_enable(tll->usbtll_p1_fck);
> >> -
> >> - if (is_ehci_tll_mode(pdata->port_mode[1]))
> >> - clk_enable(tll->usbtll_p2_fck);
> >> + for (i = 0; i < tll->nch; i++) {
> >> + if (is_ehci_tll_mode(pdata->port_mode[i])) {
> >> + int r;
> >> + r = clk_enable(tll->ch_clk[i]);
> >> + if (r) {
> >> + dev_err(dev,
> >> + "%s : Error enabling ch %d clock: %d\n",
> >> + __func__, i, r);
> >
> > you don't need __func__.
> >
>
> Thought it would be useful to point out where the message is coming
> from. But it should be easy to grep too so I'll remove it.
correct, if messages are unique, you don't need __func__ anyway ;-)
--
balbi
Attachment:
signature.asc
Description: Digital signature
