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

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

 



Hello.

On 04/14/2014 09:53 AM, 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/drivers/phy/phy-rcar-gen2.c
===================================================================
--- /dev/null
+++ linux-phy/drivers/phy/phy-rcar-gen2.c
@@ -0,0 +1,283 @@

[...]

+static int rcar_gen2_phy_init(struct phy *p)
+{
+	struct rcar_gen2_phy *phy = phy_get_drvdata(p);
+	struct rcar_gen2_phy_driver *drv = phy->drv;
+	unsigned long flags;
+	u32 ugctrl2;

We can just add
	if (!phy->select_mask)
		return 0;

   Yes, we can, if you'd prefer that.

+
+	if (phy->select_mask) {
+		clk_prepare_enable(drv->clk);
+
+		spin_lock_irqsave(&drv->lock, flags);
+		ugctrl2 = readl(drv->base + USBHS_UGCTRL2);
+		ugctrl2 &= ~phy->select_mask;
+		ugctrl2 |= phy->select_value;
+		writel(ugctrl2, drv->base + USBHS_UGCTRL2);
+		spin_unlock_irqrestore(&drv->lock, flags);
+	}
+
+	return 0;
+}

[...]

+static int rcar_gen2_phy_probe(struct platform_device *pdev)
+{

[...]

+	drv->phys[0][0].select_mask  = USBHS_UGCTRL2_USB0SEL;
+	drv->phys[0][0].select_value = USBHS_UGCTRL2_USB0SEL_PCI;
+	drv->phys[0][1].select_mask  = USBHS_UGCTRL2_USB0SEL;
+	drv->phys[0][1].select_value = USBHS_UGCTRL2_USB0SEL_HS_USB;
+	drv->phys[2][0].select_mask  = USBHS_UGCTRL2_USB2SEL;
+	drv->phys[2][0].select_value = USBHS_UGCTRL2_USB2SEL_PCI;
+	drv->phys[2][1].select_mask  = USBHS_UGCTRL2_USB2SEL;
+	drv->phys[2][1].select_value = USBHS_UGCTRL2_USB2SEL_USB30;
+
+	for (i = 0; i < NUM_USB_CHANNELS; i++) {

Instead of hard coding the number of channels,

It's hard coded in the hardware. We can even decrease that number to 2 as for the channel #1 we have nothing to do, regardless of whether it's present or not...

we can model the channels (PHYs) as sub-nodes of the main PHY

Hm, I don't think such representation would be adequate: the channels themselves do not usually correspond to any particular PHY, that's why I used #phy-cells = <2>.

in dt and use it to create individual PHYs.

   Well, we probably can... however, I fail to see any immediate gain from
it here...
I have to ask why you've selected this particular driver for such DT representation experiments, despite it not being the first one supporting multiple PHYs?

Thanks
Kishon

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux