Re: [PATCH 1/2] USB: OHCI: Eliminate platform-specific test in ohci.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 11 Oct 2014, Kevin Cernekee wrote:

> On Sat, Oct 11, 2014 at 7:58 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> > On Fri, 10 Oct 2014, Kevin Cernekee wrote:
> >
> >> OHCI_QUIRK_FRAME_NO is currently set under either of the following
> >> conditions:
> >>
> >> 1) If a ppc-of-ohci DT node indicates a compatible string of
> >> "fsl,mpc5200-ohci" or "mpc5200-ohci"
> >>
> >> 2) If usb_ohci_pdata->no_big_frame_no is set
> >>
> >> For #1, the affected platforms already enable CONFIG_PPC_MPC52xx.
> >> For #2, there are currently no in-tree users.
> >>
> >> So we can safely remove the #ifdef, and thereby allow OHCI_QUIRK_FRAME_NO
> >> to be used by other (non-PPC) platforms that have the same property.
> >> bcm63xx and bcm3384 are two such users.
> >
> > Sorry, but I can't understand this patch description.  What #ifdef does
> > it refer to?
> 
> It refers to "#ifdef CONFIG_PPC_MPC52xx".  This check appears to be
> redundant: from what I can tell, it isn't ever necessary to gate the
> workaround logic based on CONFIG_PPC_MPC52xx, because currently
> OHCI_QUIRK_FRAME_NO is only getting set for (some) mpc52xx chips.

That's not the reason for the #ifdef.  It's there to eliminate a
runtime test on systems where we know at build-time that the test can't
succeed.  It isn't necessary; it's just a small optimization.

> So, removing "#ifdef CONFIG_PPC_MPC52xx" both eliminates PPC-specific
> code from the generic driver, and allows bcm63xx/bcm3384 to make use
> of OHCI_QUIRK_FRAME_NO.
> 
> > By comparing the description with the patch, it looks like you _wanted_
> > to say something along these lines:
> >
> >         The bcm63xx and bcm3384 platforms need to set
> >         OHCI_QUIRK_FRAME_NO, but they are non-PPC platforms and
> >         don't enable CONFIG_PPC_MPC52xx.  Therefore this patch changes
> >         the code that uses OHCI_QUIRK_FRAME_NO, making it not depend
> >         on CONFIG_PPC_MPC52xx.
> >
> > Does that properly describe what you are doing?
> 
> Yes.

All right, then please resubmit the patch with a more suitable
description.  (You might also mention that the patch rephrases some
comments.)  I have no objection to the patch itself.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux