Hi,
On 01/16/2014 01:58 AM, Ma Haijun wrote:
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.
I believe it is better to explicitly check for clk == NULL as that makes it much
more clear when reading the code that clk maybe NULL.
Regards,
Hans
--
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.
