From patchwork Thu Aug 2 05:44:13 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: p.paneri@samsung.com X-Patchwork-Id: 1266221 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork1.kernel.org (Postfix) with ESMTP id AA09E3FC33 for ; Thu, 2 Aug 2012 05:47:05 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1SwoCp-0008Vw-JT; Thu, 02 Aug 2012 05:44:19 +0000 Received: from mail-yw0-f49.google.com ([209.85.213.49]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1SwoCl-0008Vi-Rm for linux-arm-kernel@lists.infradead.org; Thu, 02 Aug 2012 05:44:17 +0000 Received: by yhjj52 with SMTP id j52so8571517yhj.36 for ; Wed, 01 Aug 2012 22:44:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; bh=atepjYzJ0SpJpuVP2jKUJ5B8qP5AEaL6Lehu/frjdvo=; b=hTwneoL141XuIUE6kCn8tSv4GwuFAwwCXVAloF2+4Ryg9zARIk4giXrWyTIeW/dvkT GphAUevyMhUYl0SKpwrhajgtmiDjg9LyWc7jsF0gsXqzGMe0rpYyv6GO+yBRscdmOMqR JGy8strdHEoN4oyJSaU9JKxNz9/zIiTdZ+9goXcYajEeQqBw37Q7XffC/h6UEgLcedAF vQZmW5crWMRrnKIoryuRCtu4/fvfPITPaYIZbQtvJTFTrbe9ZsLY3ygxb7tov+sjNuIY VQl/ed4ADMbhUKztKs1qNmUa3WHz1WHhtRwQcECE3G0ZtoEdU5GP0wzE1czCsfzgHxse pJsw== MIME-Version: 1.0 Received: by 10.50.163.70 with SMTP id yg6mr1353981igb.30.1343886253318; Wed, 01 Aug 2012 22:44:13 -0700 (PDT) Received: by 10.50.89.98 with HTTP; Wed, 1 Aug 2012 22:44:13 -0700 (PDT) In-Reply-To: <201208011520.02353.arnd@arndb.de> References: <1343826351-8756-1-git-send-email-p.paneri@samsung.com> <201208011520.02353.arnd@arndb.de> Date: Thu, 2 Aug 2012 11:14:13 +0530 X-Google-Sender-Auth: w1ZkvamMkbLoa7mpto1vwlqOw5A Message-ID: Subject: Re: [PATCH 0/5] usb: phy: samsung: Introducing usb phy driver for samsung SoCs From: Praveen Paneri To: Arnd Bergmann X-Spam-Note: CRM114 invocation failed X-Spam-Score: -2.6 (--) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-2.6 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [209.85.213.49 listed in list.dnswl.org] 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (paneri[at]gmail.com) -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature Cc: l.majewski@samsung.com, kgene.kim@samsung.com, gregkh@linuxfoundation.org, devicetree-discuss@lists.ozlabs.org, linux-usb@vger.kernel.org, balbi@ti.com, grant.likely@secretlab.ca, kyungmin.park@samsung.com, linux-samsung-soc@vger.kernel.org, thomas.abraham@linaro.org, ben-linux@fluff.org, broonie@opensource.wolfsonmicro.com, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org Hi Arnd, On Wed, Aug 1, 2012 at 8:50 PM, Arnd Bergmann 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. 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Praveen 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 #include #include +#include #include -#include #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;