[PATCH 0/5] usb: phy: samsung: Introducing usb phy driver for samsung SoCs

[Praveen Paneri]

Hi Arnd,

On Wed, Aug 1, 2012 at 8:50 PM, Arnd Bergmann <arnd at arndb.de> wrote:
> On Wednesday 01 August 2012, Praveen Paneri wrote:
>> This patch set introduces a phy driver for samsung SoCs. It uses the existing
>> transceiver infrastructure to provide phy control functions. Use of this driver
>> can be extended for usb host phy as well. Over the period of time all the phy
>> related code for most of the samsung SoCs can be integrated here.
>> Removing the existing phy code from mach-s3c64xx but not from other machine
>> code.This driver is tested with smdk6410 and Exynos4210(with DT).
>
> This looks very nice overall, great work!
Thank you!
>
> We will still have to take care of the pmu isolation callback in the
> long run, when we get to the point of removing all auxdata. Do you
> have a plan on how to do that? If yes, it would be good to mention
> that in the changelog.
Yes! I understand this problem and this is the reason these patches
were sitting in my system for couple of weeks. In a  discussion with
Thomas an idea of using the existing regulator framework to
enable/disable numerous PHYs came up. For example the PMU unit
of Exynos4210 has a register set dedicated just to control USBD_PHY,
HDMI_PHY, MIPI_PHY, DAC_PHY and more. If a regulator with
each phy control register as LDO is written, the phy driver becomes a
static consumer and will modify as below.

diff --git a/drivers/usb/phy/sec_usbphy.c b/drivers/usb/phy/sec_usbphy.c
index 33119eb..4f69675 100644
--- a/drivers/usb/phy/sec_usbphy.c
+++ b/drivers/usb/phy/sec_usbphy.c
@@ -28,8 +28,8 @@
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/of.h>
+#include <linux/regulator/consumer.h>
 #include <linux/usb/otg.h>
-#include <linux/platform_data/s3c-hsotg.h>

 #include "sec_usbphy.h"

@@ -41,7 +41,7 @@ enum sec_cpu_type {
 /*
  * struct sec_usbphy - transceiver driver state
  * @phy: transceiver structure
- * @plat: platform data
+ * @vusbphy: PMU regulator for usb phy
  * @dev: The parent device supplied to the probe function
  * @clk: usb phy clock
  * @regs: usb phy register memory base
@@ -49,7 +49,7 @@ enum sec_cpu_type {
  */
 struct sec_usbphy {
 	struct usb_phy	phy;
-	struct s3c_usbphy_plat *plat;
+	struct regulator *vusbphy;
 	struct device	*dev;
 	struct clk	*clk;
 	void __iomem	*regs;
@@ -187,8 +187,11 @@ static int sec_usbphy_init(struct usb_phy *phy)
 	}

 	/* Disable phy isolation */
-	if (sec->plat && sec->plat->pmu_isolation)
-		sec->plat->pmu_isolation(false);
+	ret = regulator_enable(sec->vusbphy);
+	if (ret) {
+		dev_err(sec->dev, "Failed to enable regulator for USBPHY\n");
+		return ret;
+	}

 	/* Initialize usb phy registers */
 	sec_usbphy_enable(sec);
@@ -208,8 +211,8 @@ static void sec_usbphy_shutdown(struct usb_phy *phy)
 	sec_usbphy_disable(sec);

 	/* Enable phy isolation */
-	if (sec->plat && sec->plat->pmu_isolation)
-		sec->plat->pmu_isolation(true);
+	if (regulator_disable(sec->vusbphy))
+		dev_err(sec->dev, "Failed to disable regulator for USBPHY\n");

 	/* Disable the phy clock */
 	sec_usbphy_clk_control(sec, false);
@@ -263,7 +266,6 @@ static int __devinit sec_usbphy_probe(struct
platform_device *pdev)
 		return -ENOMEM;

 	sec->dev		= &pdev->dev;
-	sec->plat		= pdata;
 	sec->regs		= phy_base;
 	sec->phy.dev		= sec->dev;
 	sec->phy.label		= "sec-usbphy";
@@ -271,6 +273,14 @@ static int __devinit sec_usbphy_probe(struct
platform_device *pdev)
 	sec->phy.shutdown	= sec_usbphy_shutdown;
 	sec->cpu_type		= sec_usbphy_get_driver_data(pdev);

+	/* acquire regulator */
+	sec->vusbphy = regulator_get(sec->dev, "usbdphy");
+	if (IS_ERR_OR_NULL(sec->vusbphy)) {
+		dev_err(dev, "failed to get regulator 'usbdphy'\n");
+		ret = -ENXIO;
+		goto err;
+	}
+
 	ret = usb_add_phy(&sec->phy, USB_PHY_TYPE_USB2);
 	if (ret)
 		goto err;
@@ -292,6 +302,7 @@ static int __exit sec_usbphy_remove(struct
platform_device *pdev)
 		clk_put(sec->clk);
 		sec->clk = NULL;
 	}
+	regulator_put(sec->vusbphy);

 	return 0;
 }

This kind of regulator, if generalised can be useful.
Please comment.
>
> My guess is that the PMU code should be moved into a higher-level
> abstraction. I don't know if you would use one of those we already
Could you please point out the location of the code.
> have or whether you'd introduce a new subsystem for those. Apparently.
Havent thought about it. Trying to work it out with the existing infra of the
kernel.
> other platforms have similar issues, so I'd suggest you leave the
> callback for now, but we should at some point discuss what to to
> about it.
>
>         Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Praveen