On Friday 21 February 2014 11:48:03 Mark Rutland wrote: > > + > > + np = of_find_compatible_node(NULL, NULL, "ibm,usb-ohci-440epx"); > > + if (np != NULL) { > > + /* claim we really affected by usb23 erratum */ > > + if (!of_address_to_resource(np, 0, &res)) > > + ehci->ohci_hcctrl_reg = > > + devm_ioremap(&pdev->dev, > > + res.start + OHCI_HCCTRL_OFFSET, > > + OHCI_HCCTRL_LEN); > > + else > > + ehci_dbg(ehci, "%s: no ohci offset in fdt\n", __FILE__); > > + if (!ehci->ohci_hcctrl_reg) { > > + ehci_dbg(ehci, "%s: ioremap for ohci hcctrl failed\n", > > + __FILE__); > > + } else { > > + ehci->has_amcc_usb23 = 1; > > + } > > + } > > As this is being dropped into a generic driver, it would be nice to have > a comment explaining why we need to do this -- not everyone using this > will know the powerpc history. It certainly seems odd to look for > another node in the tree that this driver isn't necessarily handling, > and that should probably be explained. > > As this bit of fixup is only needed for powerpc, it would be nice to not > have to do it elsewhere. Perhaps these could be factored out into a > ppc_platform_reset function that could be empty stub for other > architectures. How about using the .data field of the of_device_id array to point to a structure of functions? That way you don't even have to call of_device_is_compatible() here. Note that using of_find_compatible_node() is the wrong approach anyway, you want to check the current device for compatibility, not just any device I assume. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html