Hi > -----Original Message----- > From: [email protected] [mailto:[email protected]] > On Behalf Of Alan Stern > Sent: Wednesday, January 15, 2014 3:18 AM > To: Hans de Goede > Cc: Tony Prisk; [email protected]; devicetree; linux-usb; > [email protected] > Subject: [linux-sunxi] Re: [PATCH v5 2/2] ehci-platform: Add support for clks > and phy passed through devicetree > > On Mon, 13 Jan 2014, Hans de Goede wrote: > > > Currently ehci-platform is only used in combination with devicetree > > when used with some Via socs. By extending it to (optionally) get clks > > and a phy from devicetree, and enabling / disabling those on power_on > > / off, it can be used more generically. Specifically after this commit > > it can be used for the ehci controller on Allwinner sunxi SoCs. > > > > Since ehci-platform is intended to handle any generic enough non pci > > ehci device, add a "usb-ehci" compatibility string. > > > > There already is a usb-ehci device-tree bindings document, update this > > with clks and phy bindings info. > > > > Although actually quite generic so far the via,vt8500 compatibilty > > string had its own bindings document. Somehow we even ended up with 2 > > of them. Since these provide no extra information over the generic > > usb-ehci documentation, this patch removes them. > > > > The ehci-ppc-of.c driver also claims the usb-ehci compatibility > > string, even though it mostly is ibm,usb-ehci-440epx specific. > > ehci-platform.c is not needed on ppc platforms, so add a !PPC_OF > > dependency to it to avoid > > 2 drivers claiming the same compatibility string getting build on ppc. > > > > Signed-off-by: Hans de Goede <[email protected]> > > Same comment as the ohci-platform patch. > > Oh, and one other thing I almost missed. (This applies to both > patches.) > > > +static int ehci_platform_power_on(struct platform_device *dev) { > > + struct usb_hcd *hcd = platform_get_drvdata(dev); > > + struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd); > > + int clk, ret; > > + > > + for (clk = 0; priv->clks[clk] && clk < EHCI_MAX_CLKS; clk++) { > > Here and in ehci_platform_remove, you need to check clk < EHCI_MAX_CLKS > _before_ dereferencing priv->clks[clk]. Yes, it probably won't make any > difference in real life, but it's still wrong in principle. In fact, this is the sort of > thing a static checker might find. >
AFAIK, clk subsystem can deal with NULL pointer properly, so shall we remove this kind priv->clks[clk] check? I am hesitating about this myself, since it may confuse people does not know the fact. Regards, Ma > Alan Stern > > -- > You received this message because you are subscribed to the Google Groups > "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/groups/opt_out. -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/groups/opt_out.
