Hello.
On 11/01/2012 05:03 AM, Kuninori Morimoto wrote:
Sorry for the late feedback but I'm studying this driver just now
(in order to port it to R-Car M1A).
diff --git a/drivers/usb/phy/rcar-phy.c b/drivers/usb/phy/rcar-phy.c
new file mode 100644
index 0000000..792f505
--- /dev/null
+++ b/drivers/usb/phy/rcar-phy.c
@@ -0,0 +1,220 @@
+/*
+ * Renesas R-Car USB phy driver
+ *
+ * Copyright (C) 2012 Renesas Solutions Corp.
+ * Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
[...]
+/*
+ * USB initial/install operation.
+ *
+ * This function setup USB phy.
+ * The used value and setting order came from
+ * [USB :: Initial setting] on datasheet.
+ */
+static int rcar_usb_phy_init(struct usb_phy *phy)
+{
[...]
+ /* set platform specific port settings */
+ iowrite32(0x00000000, (reg0 + USBPCTRL0));
This register contains completely board specific setting,
hard-coding it to 0 is wrong.
Shouldn't you have passed its value as platform data instead?
+
+ /*
+ * EHCI IP internal buffer setting
+ * EHCI IP internal buffer enable
+ *
+ * These are recommended value of a datasheet
+ * see [USB :: EHCI internal buffer setting]
+ */
+ iowrite32(0x00ff0040, (reg0 + EIIBC1));
+ iowrite32(0x00ff0040, (reg1 + EIIBC1));
+
+ iowrite32(0x00000001, (reg0 + EIIBC2));
+ iowrite32(0x00000001, (reg1 + EIIBC2));
Why write each of these register twice, at different bases? The USB
section of
the R-Car H1 manual doesn't seem to mention that they are dual mapped...
[...]
+static void rcar_usb_phy_shutdown(struct usb_phy *phy)
+{
+ struct rcar_usb_phy_priv *priv = usb_phy_to_priv(phy);
+ void __iomem *reg0 = priv->reg0;
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->lock, flags);
+
+ if (priv->counter-- == 1) { /* last user */
+ iowrite32(0x00000000, (reg0 + USBPCTRL0));
Why change this register at all at shutdown?
+ iowrite32(0x00000000, (reg0 + USBPCTRL1));
+ }
WBR, Sergei
--
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
