Re: [PATCH v4] phy: Renesas R-Car Gen2 PHY driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hello.

On 06/04/2014 03:40 PM, Kishon Vijay Abraham I wrote:

This PHY, though formally being a part of Renesas USBHS controller, contains the
UGCTRL2 register that controls multiplexing of the USB ports (Renesas calls them
channels) to the different USB controllers: channel 0 can be connected to either
PCI EHCI/OHCI or USBHS controllers, channel 2 can be connected to PCI EHCI/OHCI
or xHCI controllers.

This is a new driver for this USB PHY currently already supported under drivers/
usb/phy/. The reason for writing the new driver was the requirement that the
multiplexing of USB channels to the controller be dynamic, depending on what
USB drivers are loaded,  rather than static as provided by the old driver.
The infrastructure provided by drivers/phy/phy-core.c seems to fit that purpose
ideally. The new driver only supports device tree probing for now.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>

[...]

Index: linux-phy/Documentation/devicetree/bindings/phy/rcar-gen2-phy.txt
===================================================================
--- /dev/null
+++ linux-phy/Documentation/devicetree/bindings/phy/rcar-gen2-phy.txt
@@ -0,0 +1,60 @@
+* Renesas R-Car generation 2 USB PHY
+
+This file provides information on what the device node for the R-Car generation
+2 USB PHY contains.
+
+Required properties:
+- compatible: "renesas,usb-phy-r8a7790" if the device is a part of R8A7790 SoC.
+	      "renesas,usb-phy-r8a7791" if the device is a part of R8A7791 SoC.
+- reg: offset and length of the register block.
+- #phy-cells: see phy-bindings.txt in the same directory, must be 2.
+- clocks: clock phandle and specifier pair.
+- clock-names: string, clock input name, must be "usbhs".
+
+The phandle's first argument in the PHY specifier identifies the USB channel,
+the second one is the USB controller selector and depends on the first:
+
++-----------+---------------+---------------+
+|\ Selector |               |               |
++ --------- +       0       |       1       |
+| Channel  \|               |               |
++-----------+---------------+---------------+
+| 0         | PCI EHCI/OHCI | HS-USB        |
+| 1         | PCI EHCI/OHCI | xHCI          |
++-----------+---------------+---------------+
+
+The USB PHY device tree node should be the subnodes corresponding to the USB
+channels and the controllers connected to them. These subnodes must contain the
+following properties:
+- renesas,phy-select: the first cell identifies the USB channel, the second cell
+  is the USB controller selector; see the table above for the values.
+- renesas,ugctrl2-masks: the first cell is the UGCTRL2 mask corresponding to
+  the USB channel, the second cell is the UGCTRL2 value corresponding to the
+  USB controller selected for that channel.
+
+Example (Lager board):
+
+	usb-phy@e6590100 {
+		compatible = "renesas,usb-phy-r8a7790";
+		reg = <0 0xe6590100 0 0x100>;
+		#phy-cells = <2>;
+		clocks = <&mstp7_clks R8A7790_CLK_HSUSB>;
+		clock-names = "usbhs";
+
+		usb-phy@0,0 {
+			renesas,phy-select = <0 0>;
+			renesas,ugctrl2-masks = <0x00000030 0x00000010>;
+		};
+		usb-phy@0,1 {
+			renesas,phy-select = <0 1>;
+			renesas,ugctrl2-masks = <0x00000030 0x00000030>;
+		};
+		usb-phy@1,0 {
+			renesas,phy-select = <1 0>;
+			renesas,ugctrl2-masks = <0x80000000 0x00000000>;
+		};
+		usb-phy@1,1 {
+			renesas,phy-select = <1 1>;
+			renesas,ugctrl2-masks = <0x80000000 0x80000000>;
+		};
+	};

IIUC, channel 0 can be configured for either EHCI/OHCI or HS-USB but can't be
used for both.  And channel 1 can be configured for either PCI EHCI/OHCI or
xHCI. right?

Yes. However that depends on the driver load order: if e.g. xHCI driver is loaded later than PCI USB drivers,
it will override the channel routing.

So ideally only two sub-nodes should be created for channel '0' and channel
'1'.

Hm, but I need to perform a special PHY power up sequence for the USBHS PHY itself (corresponding to channel #0, selector #1).

You can configure a channel to a particular mode by passing the mode in
PHY specifier

   I already have "#phy-cells" prop equal to 2.

(The channel should be configured to a particualr mode in xlate).

I have even considered using the of_xlate() method at first but then abandoned that idea for the phy_init() method...

Btw I'm not sure if it is recommended to pass register mask values from dt.

Neither am I. I did that because you requested the device tree representation for each of the sub-PHYs under that part of the driver which initialized the register masks, so that I thought that you wanted those masks to be represented in the device tree as well...

[...]
Index: linux-phy/drivers/phy/phy-rcar-gen2.c
===================================================================
--- /dev/null
+++ linux-phy/drivers/phy/phy-rcar-gen2.c
@@ -0,0 +1,288 @@
[...]
+#define USBHS_LPSTS			0x02
+#define USBHS_UGCTRL			0x80
+#define USBHS_UGCTRL2			0x84
+#define USBHS_UGSTS			0x88
+
+/* Low Power Status register (LPSTS) */
+#define USBHS_LPSTS_SUSPM		0x4000
+
+/* USB General control register (UGCTRL) */
+#define USBHS_UGCTRL_CONNECT		0x00000004
+#define USBHS_UGCTRL_PLLRESET		0x00000001
+
+/* USB General control register 2 (UGCTRL2) */
+#define USBHS_UGCTRL2_USB2SEL		0x80000000
+#define USBHS_UGCTRL2_USB2SEL_PCI	0x00000000
+#define USBHS_UGCTRL2_USB2SEL_USB30	0x80000000
+#define USBHS_UGCTRL2_USB0SEL		0x00000030
+#define USBHS_UGCTRL2_USB0SEL_PCI	0x00000010
+#define USBHS_UGCTRL2_USB0SEL_HS_USB	0x00000030
+
+/* USB General status register (UGSTS) */
+#define USBHS_UGSTS_LOCK		0x00000300
+
+#define	NUM_USB_CHANNELS	2
+
+struct rcar_gen2_phy {
+	struct phy *phy;
+	struct rcar_gen2_phy_driver *drv;
+	u32 select_mask;
+	u32 select_value;
+};
+
+struct rcar_gen2_phy_driver {
+	void __iomem *base;
+	struct clk *clk;
+	spinlock_t lock;
+	struct rcar_gen2_phy phys[NUM_USB_CHANNELS][2];

This can be created dynamically based on the number of sub-nodes. In this case

Hm, that sounds to me like another complication of the driver with no apparent win...

it'll be only rcar_gen2_phy phys[2], one for each channel.
By this we need not hard code NUM_USB_CHANNELS.

I don't quite understand what's up with hard-coding it -- this constant is dictated by the UGCTRL2 register layout anyway.

Thanks
Kishon

WBR, Sergei

--
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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux