lkml.org 
[lkml]   [2013]   [Jan]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v5 2/4] usb: phy: samsung: Add host phy support to samsung-phy driver
From
Hi Felipe,


On Fri, Jan 11, 2013 at 6:28 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Fri, Jan 11, 2013 at 06:10:48PM +0530, Vivek Gautam wrote:
>> Hi Doug,
>>
>> Sorry!! for the delayed response.
>>
>>
>> On Thu, Dec 20, 2012 at 4:31 AM, Doug Anderson <dianders@chromium.org> wrote:
>> > Vivek,
>> >
>> > I don't really have a good 10000 foot view about how all of the USB
>> > bits fit together, but a few detail-oriented comments below.
>> >
>> >
>> > On Tue, Dec 18, 2012 at 6:43 AM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
>> >> This patch adds host phy support to samsung-usbphy.c and
>> >> further adds support for samsung's exynos5250 usb-phy.
>> >>
>> >> Signed-off-by: Praveen Paneri <p.paneri@samsung.com>
>> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> >> ---
>> >> .../devicetree/bindings/usb/samsung-usbphy.txt | 25 +-
>> >> drivers/usb/phy/Kconfig | 2 +-
>> >> drivers/usb/phy/samsung-usbphy.c | 465 ++++++++++++++++++--
>> >> include/linux/usb/samsung_usb_phy.h | 13 +
>> >> 4 files changed, 454 insertions(+), 51 deletions(-)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
>> >> index a7b28b2..2ec5400 100644
>> >> --- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
>> >> +++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
>> >> @@ -1,23 +1,38 @@
>> >> * Samsung's usb phy transceiver
>> >>
>> >> -The Samsung's phy transceiver is used for controlling usb otg phy for
>> >> -s3c-hsotg usb device controller.
>> >> +The Samsung's phy transceiver is used for controlling usb phy for
>> >> +s3c-hsotg as well as ehci-s5p and ohci-exynos usb controllers
>> >> +across Samsung SOCs.
>> >> TODO: Adding the PHY binding with controller(s) according to the under
>> >> developement generic PHY driver.
>> >>
>> >> Required properties:
>> >> +
>> >> +Exynos4210:
>> >> - compatible : should be "samsung,exynos4210-usbphy"
>> >> - reg : base physical address of the phy registers and length of memory mapped
>> >> region.
>> >>
>> >> +Exynos5250:
>> >> +- compatible : should be "samsung,exynos5250-usbphy"
>> >> +- reg : base physical address of the phy registers and length of memory mapped
>> >> + region.
>> >> +
>> >> Optional properties:
>> >> - samsung,usb-phyhandle : should point to usb-phyhandle sub-node which provides
>> >> binding data to enable/disable device PHY handled by
>> >> - PMU register.
>> >> + PMU register; or to configure usb2.0 phy handled by
>> >> + SYSREG.
>> >>
>> >> Required properties:
>> >> - compatible : should be "samsung,usbdev-phyctrl" for
>> >> - DEVICE type phy.
>> >> + DEVICE type phy; or
>> >> + should be "samsung,usbhost-phyctrl" for
>> >> + HOST type phy; or
>> >> + should be "samsung,usb-phycfg" for
>> >> + USB2.0 PHY_CFG.
>> >> - samsung,phyhandle-reg: base physical address of
>> >> - PHY_CONTROL register in PMU.
>> >> + PHY_CONTROL register in PMU;
>> >> + or USB2.0 PHY_CFG register
>> >> + in SYSREG.
>> >> - samsung,enable-mask : should be '1'
>> >> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
>> >> index 17ad743..13c0eaf 100644
>> >> --- a/drivers/usb/phy/Kconfig
>> >> +++ b/drivers/usb/phy/Kconfig
>> >> @@ -47,7 +47,7 @@ config USB_RCAR_PHY
>> >>
>> >> config SAMSUNG_USBPHY
>> >> bool "Samsung USB PHY controller Driver"
>> >> - depends on USB_S3C_HSOTG
>> >> + depends on USB_S3C_HSOTG || USB_EHCI_S5P || USB_OHCI_EXYNOS
>> >> select USB_OTG_UTILS
>> >> help
>> >> Enable this to support Samsung USB phy controller for samsung
>> >> diff --git a/drivers/usb/phy/samsung-usbphy.c b/drivers/usb/phy/samsung-usbphy.c
>> >> index 4ceabe3..621348a 100644
>> >> --- a/drivers/usb/phy/samsung-usbphy.c
>> >> +++ b/drivers/usb/phy/samsung-usbphy.c
>> >> @@ -5,7 +5,8 @@
>> >> *
>> >> * Author: Praveen Paneri <p.paneri@samsung.com>
>> >> *
>> >> - * Samsung USB2.0 High-speed OTG transceiver, talks to S3C HS OTG controller
>> >> + * Samsung USB-PHY transceiver; talks to S3C HS OTG controller, EHCI-S5P and
>> >> + * OHCI-EXYNOS controllers.
>> >> *
>> >> * This program is free software; you can redistribute it and/or modify
>> >> * it under the terms of the GNU General Public License version 2 as
>> >> @@ -24,7 +25,7 @@
>> >> #include <linux/err.h>
>> >> #include <linux/io.h>
>> >> #include <linux/of.h>
>> >> -#include <linux/usb/otg.h>
>> >> +#include <linux/usb/samsung_usb_phy.h>
>> >> #include <linux/platform_data/samsung-usbphy.h>
>> >>
>> >> /* Register definitions */
>> >> @@ -56,6 +57,103 @@
>> >> #define RSTCON_HLINK_SWRST (0x1 << 1)
>> >> #define RSTCON_SWRST (0x1 << 0)
>> >>
>> >> +/* EXYNOS5 */
>> >> +#define EXYNOS5_PHY_HOST_CTRL0 (0x00)
>> >> +
>> >> +#define HOST_CTRL0_PHYSWRSTALL (0x1 << 31)
>> >> +
>> >> +#define HOST_CTRL0_REFCLKSEL_MASK (0x3)
>> >> +#define HOST_CTRL0_REFCLKSEL_XTAL (0x0 << 19)
>> >> +#define HOST_CTRL0_REFCLKSEL_EXTL (0x1 << 19)
>> >> +#define HOST_CTRL0_REFCLKSEL_CLKCORE (0x2 << 19)
>> >> +
>> >> +#define HOST_CTRL0_FSEL_MASK (0x7 << 16)
>> >> +#define HOST_CTRL0_FSEL(_x) ((_x) << 16)
>> >> +#define HOST_CTRL0_FSEL_CLKSEL_50M (0x7)
>> >> +#define HOST_CTRL0_FSEL_CLKSEL_24M (0x5)
>> >> +#define HOST_CTRL0_FSEL_CLKSEL_20M (0x4)
>> >> +#define HOST_CTRL0_FSEL_CLKSEL_19200K (0x3)
>> >> +#define HOST_CTRL0_FSEL_CLKSEL_12M (0x2)
>> >> +#define HOST_CTRL0_FSEL_CLKSEL_10M (0x1)
>> >> +#define HOST_CTRL0_FSEL_CLKSEL_9600K (0x0)
>> >
>> > Add the shifts to the #defines and remove HOST_CTRL0_FSEL(_x). That
>> > makes it match the older phy more closely.
>> >
>> Wouldn't it hamper the readability when shifts are used ??
>> I mean if we have something like this -
>>
>> phyhost |= HOST_CTRL0_FSEL(phyclk)
>
> this one looks better to my eyes.
>

Ok, better we keep this.

>> >> + refclk_freq |= HOST_CTRL0_FSEL_CLKSEL_9600K;
>> >
>> > Why |= with 0?
>> >
>> kept this just to keep things look similar :-). will remove this line,
>
> I would rather keep it. Compiler will optimize it out anyway and it
> makes things logical, I mean, we can see that you're telling IP that
> reference clock is 9600KHz.
>

ditto !


--
Thanks & Regards
Vivek


\
 
 \ /
  Last update: 2013-01-11 15:21    [W:0.145 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site