Hi,
On Wed, Nov 21, 2012 at 04:45:27PM +0200, Roger Quadros wrote:
> >> + switch (omap->usbhs_rev) {
> >> + case OMAP_USBHS_REV1:
> >> + omap->nports = 3;
> >> + break;
> >> + case OMAP_USBHS_REV2:
> >> + omap->nports = 2;
> >> + break;
> >> + default:
> >> + omap->nports = MAX_HS_USB_PORTS;
> >> + dev_info(dev,
> >> + "USB HOST Rev : 0x%d not recognized, assuming %d ports\n",
> >> + omap->usbhs_rev, omap->nports);
> >
> > please make this dev_dbg().
> >
>
> IMHO, I think this should be displayed on the console as the driver
> doesn't really support that revision and might need to be upgraded. It
> won't be displayed for existing hardware that we know about till date.
Ok, there are two ways to see this:
a) (my preferred) you don't treat it as an error, you assume that newer
HW is backwards compatible until proven otherwise. If that's the case,
you don't need this message because driver will just work.
b) you treat it as an error, you assume that newer HW is *not* backwards
compatible until prove otherwise. If that's the case, you don't need
this message because driver won't probe.
In both situations the message is pretty much meaningless. I prefer to
assume driver will work with newer HW and if it doesn't, it just means
(NOTICE, THIS IS MY OWN OPINION, NOT MY EMPLOYER'S OPINION BY ANY MEANS)
we're not fast enough delivering code to mainline kernel. We are the
first ones to have access to the HW afterall ;-)
> >> + break;
> >> + }
> >> +
> >> need_logic_fck = false;
> >> - for (i = 0; i < MAX_HS_USB_PORTS; i++) {
> >> + for (i = 0; i < omap->nports; i++) {
> >> if (is_ehci_phy_mode(i) || is_ehci_tll_mode(i) ||
> >> is_ehci_hsic_mode(i))
> >> need_logic_fck |= true;
> >> @@ -538,7 +561,7 @@ static int __devinit usbhs_omap_probe(struct platform_device *pdev)
> >> goto err_init60m;
> >> }
> >>
> >> - for (i = 0; i < MAX_HS_USB_PORTS; i++) {
> >> + for (i = 0; i < omap->nports; i++) {
> >> struct clk *pclk;
> >> char utmi_clk[] = "usb_host_hs_utmi_px_clk";
> >>
> >> @@ -588,8 +611,6 @@ static int __devinit usbhs_omap_probe(struct platform_device *pdev)
> >> }
> >>
> >>
> >> - pm_runtime_enable(dev);
> >
> > moving this part around isn't part of $SUBJECT aparently.
>
> pm_runtime_enable is moved earlier so that we can read the REVISION
> register, so it is part of $SUBJECT.
fair enough, but then it needs to be mentioned on commit log.
--
balbi
Attachment:
signature.asc
Description: Digital signature
