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