Re: [RFC 05/10] i2c: s3c2410: Remove support for Exynos5440

2018-04-25 Thread Sylwester Nawrocki 
On 04/24/2018 10:32 PM, Krzysztof Kozlowski wrote:
> The Exynos5440 is not actively developed, there are no development
> boards available and probably there are no real products with it.
> Remove wide-tree support for Exynos5440.
> 
> Signed-off-by: Krzysztof Kozlowski <k...@kernel.org>

Reviewed-by: Sylwester Nawrocki <s.nawro...@samsung.com>

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

Re: [RFC 08/10] spi: s3c64xx: samsung: Remove support for Exynos5440

2018-04-25 Thread Sylwester Nawrocki 
On 04/24/2018 10:32 PM, Krzysztof Kozlowski wrote:
> The Exynos5440 is not actively developed, there are no development
> boards available and probably there are no real products with it.
> Remove wide-tree support for Exynos5440.
> 
> Signed-off-by: Krzysztof Kozlowski <k...@kernel.org>

It's good you didn't remove S3C64XX_SPI_QUIRK_POLL entirely as it will
be needed for Exynos5433 ISP SPI controllers.

Reviewed-by: Sylwester Nawrocki <s.nawro...@samsung.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 04/10] clk: samsung: Remove support for Exynos5440

2018-04-25 Thread Sylwester Nawrocki 
Cc: Jingoo Han

On 04/24/2018 10:32 PM, Krzysztof Kozlowski wrote:
> The Exynos5440 is not actively developed, there are no development
> boards available and probably there are no real products with it.
> Remove wide-tree support for Exynos5440.
> 
> Signed-off-by: Krzysztof Kozlowski 

Thanks Krzysztof, patch applied, assuming there is no issues with applying 
patches from this series individually through the subsystem trees.

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

Re: [PATCHv3 2/2] phy: exynos5-usbdrd: Calibrate LOS levels for exynos5420/5800

2017-10-05 Thread Sylwester Nawrocki 
On 10/05/2017 02:11 PM, Andrzej Pietrasiewicz wrote:
> +static int crport_handshake(struct exynos5_usbdrd_phy *phy_drd,
> + u32 val, u32 cmd)
> +{
> + u32 usec = 100;
> + unsigned int result;
> +
> + writel(val | cmd, phy_drd->reg_phy + EXYNOS5_DRD_PHYREG0);
> +
> + do {
> + result = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYREG1);
> + if (result & PHYREG1_CR_ACK)
> + break;
> +
> + udelay(1);
> + } while (usec-- > 0);

It looks like you could use readl_poll_timeout_atomic() macro 
instead of these polling loops.

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

Re: USB 3.0 is broken on Odroid Xu4 on latest kernel

2017-07-21 Thread Sylwester Nawrocki 

Hi,

On 07/21/2017 10:10 AM, Felipe Balbi wrote:

Jochen Sprickerhof  writes:



I've send a patch for this some time ago here:
http://marc.info/?l=linux-usb=149945465112440=2

This goes along with the patch in this thread:
http://marc.info/?l=linux-usb=149983203023058=2

Would be great if you could give it a try and report back.

@Felipe can I do anything more to get it accepted upstream?

you got rid of*all*  context. I have no idea what you're replying
to. Looking at the patch, though, I think this may be caused by the
regression on the order of when to get the PHY. That was fixed by TI,
patch is already on next and greg's queue for next -rc. Please check if
commit 541768b08a400d9d292cfd9c898401b8178856ac helps you guys.


I tested with that commit on top of v4.13-rc1 and USB seems to
be working properly on Odroid XU4, the mouse is working.

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

Re: [PATCHv3 1/4] phy: phy-core: Make GENERIC_PHY an invisible option

2015-05-29 Thread Sylwester Nawrocki 
On 29/05/15 14:37, Kishon Vijay Abraham I wrote:
 Tejun, Maxime, Sylwester, Kyungmin
 
 On Thursday 23 April 2015 04:34 AM, Arun Ramamurthy wrote:
 Most of the phy providers use select to enable GENERIC_PHY. Since select
 is only recommended when the config is not visible, GENERIC_PHY is changed
 an invisible option. To maintain consistency, all phy providers are changed
 to select GENERIC_PHY and all non-phy drivers use depends on when the
 phy framework is explicity required. USB_MUSB_OMAP2PLUS has a cyclic
 dependency, so it is left as select.

 Signed-off-by: Arun Ramamurthy arun.ramamur...@broadcom.com
 
 Need your ACK for this patch.

For
drivers/media/platform/exynos4-is/Kconfig
drivers/video/fbdev/exynos/Kconfig

Acked-by: Sylwester Nawrocki s.nawro...@samsung.com

--
Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/11] Exynos7: Adding USB 3.0 support

2014-12-22 Thread Sylwester Nawrocki 
Hi Vivek,

On 25/11/14 12:48, Vivek Gautam wrote:
 On Sat, Nov 22, 2014 at 8:42 PM, Kukjin Kim kg...@kernel.org wrote:
  On 11/22/14 17:40, Kishon Vijay Abraham I wrote:
  On Friday 21 November 2014 08:41 PM, Felipe Balbi wrote:
...
  I took dwc3 driver patches.
 
  I took the phy patches.
 
  I'll take DT changes once exynos7 is landing into samsung tree :)
 
 You too may want to pick the sole clock driver patch in this series
 for 3.19 ? :-)
 clk: exynos7: Add required clock tree for USB
 
 Please let me know if the merge window is still open on your side so
 that you can pick this patch.

My apologies, it seems I have missed this patch. :/
Feel free to ping me off the list in future if there is no response
for more than week.
I queued this patch now for 3.20.  I'm putting all the exynos7 clk
patches on a separate branch which could then be pulled into samsung
arm tree for the dependant dts changes.

-- 
Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] usb: Remove references to non-existent PLAT_S5P symbol

2014-11-05 Thread Sylwester Nawrocki 
On 04/11/14 20:52, Paul Bolle wrote:
 On Tue, 2014-11-04 at 11:42 -0800, Greg KH wrote:
  As it's something that no one seemed to ever need before (i.e. it's not
  a regression fix), but it would be a new feature, I don't think it's
  really a stable fix.
  
  But feel free to convince me otherwise :)

 Sylwester, was I right in thinking that users of PLAT_S5P, who could set
 USB_EHCI_EXYNOS or USB_OHCI_EXYNOS pre v3.17, got, well, transferred to
 ARCH_S5PV210 and lost the ability to set one of those symbols in v3.17?

Yes, after commit d78c16ccde96 (ARM: SAMSUNG: Remove remaining legacy code)
we lost the ability to enable USB OHCI and EHCI on S5PV210 SoC.
Thus for those who use the mainline kernel (might be rare) with S5PV210 SoC
there is obviously a regression in USB subsystem in v3.17.


-- 
Regards,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] usb: Remove references to non-existent PLAT_S5P symbol

2014-11-04 Thread Sylwester Nawrocki 
On 04/11/14 00:24, Greg KH wrote:
 On Tue, Oct 07, 2014 at 11:12:07AM +0200, Sylwester Nawrocki wrote:
  The PLAT_S5P Kconfig symbol was removed in commit d78c16ccde96
  (ARM: SAMSUNG: Remove remaining legacy code). There are still
  some references left, fix that by replacing them with ARCH_S5PV210.
  
  Fixes: d78c16ccde96 (ARM: SAMSUNG: Remove remaining legacy code)
  Reported-by: Paul Bolle pebo...@tiscali.nl
  Acked-by: Jingoo Han jg1@samsung.com
  Cc: sta...@vger.kernel.org   [3.17+]

 This isn't a stable issue...

Sorry for disturbing then, let me go and read the documentation again.

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

[PATCH v3] usb: Remove references to non-existent PLAT_S5P symbol

2014-10-07 Thread Sylwester Nawrocki 
The PLAT_S5P Kconfig symbol was removed in commit d78c16ccde96
(ARM: SAMSUNG: Remove remaining legacy code). There are still
some references left, fix that by replacing them with ARCH_S5PV210.

Fixes: d78c16ccde96 (ARM: SAMSUNG: Remove remaining legacy code)
Reported-by: Paul Bolle pebo...@tiscali.nl
Acked-by: Jingoo Han jg1@samsung.com
Cc: sta...@vger.kernel.org[3.17+]
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
---
Changes since v2:
 - updated the commit description.

 drivers/usb/host/Kconfig |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 82800a7..6f1d48e 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -220,7 +220,7 @@ config USB_EHCI_SH

 config USB_EHCI_EXYNOS
tristate EHCI support for Samsung S5P/EXYNOS SoC Series
-   depends on PLAT_S5P || ARCH_EXYNOS
+   depends on ARCH_S5PV210 || ARCH_EXYNOS
help
Enable support for the Samsung Exynos SOC's on-chip EHCI controller.

@@ -527,7 +527,7 @@ config USB_OHCI_SH

 config USB_OHCI_EXYNOS
tristate OHCI support for Samsung S5P/EXYNOS SoC Series
-   depends on PLAT_S5P || ARCH_EXYNOS
+   depends on ARCH_S5PV210 || ARCH_EXYNOS
help
 Enable support for the Samsung Exynos SOC's on-chip OHCI controller.

--
1.7.9.5

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

[PATCH] usb: Remove references to non-existent PLAT_S5P symbol

2014-10-06 Thread Sylwester Nawrocki 
The PLAT_S5P Kconfig symbol was removed in commit d78c16ccde96
(ARM: SAMSUNG: Remove remaining legacy code). There are still
some references left, fix that by replacing them with ARCH_S5PV210.

Reported-by: Paul Bolle pebo...@tiscali.nl
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
---
 drivers/usb/host/Kconfig |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 82800a7..6f1d48e 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -220,7 +220,7 @@ config USB_EHCI_SH
 
 config USB_EHCI_EXYNOS
tristate EHCI support for Samsung S5P/EXYNOS SoC Series
-   depends on PLAT_S5P || ARCH_EXYNOS
+   depends on ARCH_S5PV210 || ARCH_EXYNOS
help
Enable support for the Samsung Exynos SOC's on-chip EHCI controller.
 
@@ -527,7 +527,7 @@ config USB_OHCI_SH
 
 config USB_OHCI_EXYNOS
tristate OHCI support for Samsung S5P/EXYNOS SoC Series
-   depends on PLAT_S5P || ARCH_EXYNOS
+   depends on ARCH_S5PV210 || ARCH_EXYNOS
help
 Enable support for the Samsung Exynos SOC's on-chip OHCI controller.
 
-- 
1.7.9.5

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

Re: [PATCH v7 1/2] phy: Add new Exynos5 USB 3.0 PHY driver

2014-05-09 Thread Sylwester Nawrocki 
Hi Vivek,

On 08/05/14 11:03, Vivek Gautam wrote:
 diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt 
 b/Documentation/devicetree/bindings/phy/samsung-phy.txt
  index b422e38..51efe4c 100644
  --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
  +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
  @@ -114,3 +114,43 @@ Example:
compatible = samsung,exynos-sataphy-i2c;
reg = 0x38;
};
  +
  +Samsung Exynos5 SoC series USB DRD PHY controller
  +--
  +
  +Required properties:
  +- compatible : Should be set to one of the following supported values:
  + - samsung,exynos5250-usbdrd-phy - for exynos5250 SoC,
  + - samsung,exynos5420-usbdrd-phy - for exynos5420 SoC.
  +- reg : Register offset and length of USB DRD PHY register set;
  +- clocks: Clock IDs array as required by the controller
  +- clock-names: names of clocks correseponding to IDs in the clock 
  property;
  +Required clocks:
  + - phy: main PHY clock (same as USB DRD controller i.e. DWC3 IP 
  clock),
  +used for register access.
  + - ref: PHY's reference clock (usually crystal clock), used for
  +PHY operations, associated by phy name. It is used to
  +determine bit values for clock settings register.
  +For Exynos5420 this is given as 'sclk_usbphy30' in CMU.
  +- samsung,pmu-syscon: phandle for PMU system controller interface, 
  used to
  +   control pmu registers for power isolation.
  +- samsung,pmu-offset: phy power control register offset to 
  pmu-system-controller
  +   base.
 
  It doesn't seem right to have register offset encoded in the device tree
  like this. I think it'd be more appropriate to associate such an offset
  with the compatible string's value in the driver.
 
  Ok, it makes more sense.
  Just out of curiosity, what difference would this make ?

 Moreover, in case of Exynos5420 (and may be in future SoCs), where we
 have 2 USB DRD PHY controllers,
 we will need to have a way around to deal with two separate offsets in
 the driver for one compatible string.

It could be handled by creating a list of offsets per compatible string and
then adding some way to identify the PHY device instance. So I believe that's
not a big issue. Now you'd be encoding a list of registers offsets in the
device tree, without encoding bit layout of each register. It's unlikely that
each instance would have different bits layout, but describing individual 
registers in DT I thought is something that we're not supposed to do.
 
 Getting the offsets from DT seems a cleaner way to handle this case of
 multi controllers.

I think it's easier from the driver POV, but isn't it violating the device
tree rules ?
Anyway. I'm just pointing this minor issue in the binding as it appears
to me. The final word of course belongs to a DT binding maintainer.

Regards,
-- 
Sylwester Nawrocki
Samsung RD Institute Poland
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 1/2] phy: Add new Exynos5 USB 3.0 PHY driver

2014-05-06 Thread Sylwester Nawrocki 
On 28/04/14 08:17, Vivek Gautam wrote:
 Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs.
 The new driver uses the generic PHY framework and will interact
 with DWC3 controller present on Exynos5 series of SoCs.
 Thereby, removing old phy-samsung-usb3 driver and related code
 used untill now which was based on usb/phy framework.
 
 Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
 ---
 
 Changes from v6:
  - Addressed review comments:
-- Sorted config entries in Kconfig and Makefile
-- Made #define to_usbdrd_phy(inst) to a static inline routine.
-- Restructured exynos5_rate_to_clk() as suggested.
-- Amended 'val' field for regmap_update_bits() in the routine
   exynos5_usbdrd_phy_isol().
-- Removed sentinel entry from exynos5_usbdrd_phy_cfg[] struct.
-- Removed check for 'match' entry in probe().
 
  .../devicetree/bindings/phy/samsung-phy.txt|   40 ++
  drivers/phy/Kconfig|   11 +
  drivers/phy/Makefile   |1 +
  drivers/phy/phy-exynos5-usbdrd.c   |  627 
 
  4 files changed, 679 insertions(+)
  create mode 100644 drivers/phy/phy-exynos5-usbdrd.c
 
 diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt 
 b/Documentation/devicetree/bindings/phy/samsung-phy.txt
 index b422e38..51efe4c 100644
 --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
 +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
 @@ -114,3 +114,43 @@ Example:
   compatible = samsung,exynos-sataphy-i2c;
   reg = 0x38;
   };
 +
 +Samsung Exynos5 SoC series USB DRD PHY controller
 +--
 +
 +Required properties:
 +- compatible : Should be set to one of the following supported values:
 + - samsung,exynos5250-usbdrd-phy - for exynos5250 SoC,
 + - samsung,exynos5420-usbdrd-phy - for exynos5420 SoC.
 +- reg : Register offset and length of USB DRD PHY register set;
 +- clocks: Clock IDs array as required by the controller
 +- clock-names: names of clocks correseponding to IDs in the clock property;
 +Required clocks:
 + - phy: main PHY clock (same as USB DRD controller i.e. DWC3 IP clock),
 +used for register access.
 + - ref: PHY's reference clock (usually crystal clock), used for
 +PHY operations, associated by phy name. It is used to
 +determine bit values for clock settings register.
 +For Exynos5420 this is given as 'sclk_usbphy30' in CMU.
 +- samsung,pmu-syscon: phandle for PMU system controller interface, used to
 +   control pmu registers for power isolation.
 +- samsung,pmu-offset: phy power control register offset to 
 pmu-system-controller
 +   base.

It doesn't seem right to have register offset encoded in the device tree
like this. I think it'd be more appropriate to associate such an offset
with the compatible string's value in the driver.

Also it might be sensible to create a new header file in include/linux/mfd/
syscon/ for Exynos5 SoCs and put any required PMU offset definitions there. 
Instead having them scattered and possibly duplicated in various drivers.

 +- #phy-cells : from the generic PHY bindings, must be 1;
 +
 +For samsung,exynos5250-usbdrd-phy and samsung,exynos5420-usbdrd-phy
 +compatible PHYs, the second cell in the PHY specifier identifies the
 +PHY id, which is interpreted as follows:
 +  0 - UTMI+ type phy,
 +  1 - PIPE3 type phy,
 +
 +Example:
 + usb3_phy: usbphy@1210 {
 + compatible = samsung,exynos5250-usbdrd-phy;
 + reg = 0x1210 0x100;
 + clocks = clock 286, clock 1;
 + clock-names = phy, ref;
 + samsung,pmu-syscon = pmu_system_controller;
 + samsung,pmu-offset = 0x704;
 + #phy-cells = 1;
 + };
[...]
 +struct usbdrd_phy_config {

Isn't name of this data structure too generic ? 
Perhaps rename it to exynos_usbdrd_phy_config ?

 + u32 id;
 + void (*phy_isol)(struct phy_usb_instance *inst, u32 on);
 + void (*phy_init)(struct exynos5_usbdrd_phy *phy_drd);
 + unsigned int (*set_refclk)(struct phy_usb_instance *inst);
 +};
 +
 +struct exynos5_usbdrd_phy_drvdata {
 + const struct usbdrd_phy_config *phy_cfg;
 +};
 +
[...]
 +const struct exynos5_usbdrd_phy_drvdata exynos5420_usbdrd_phy = {
 + .phy_cfg= exynos5_usbdrd_phy_cfg,
 +};
 +
 +const struct exynos5_usbdrd_phy_drvdata exynos5250_usbdrd_phy = {
 + .phy_cfg= exynos5_usbdrd_phy_cfg,
 +};
 +
 +static const struct of_device_id exynos5_usbdrd_phy_of_match[] = {
 + {
 + .compatible = samsung,exynos5250-usbdrd-phy,
 + .data = exynos5250_usbdrd_phy
 + }, {
 + .compatible = samsung,exynos5420-usbdrd-phy,
 + .data = exynos5420_usbdrd_phy
 + },
 + { },
 +};
 +
 +static int

Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

2014-04-14 Thread Sylwester Nawrocki 
On 08/04/14 16:36, Vivek Gautam wrote:
 diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt 
 b/Documentation/devicetree/bindings/phy/samsung-phy.txt
 index 28f9edb..6d99ba9 100644
 --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
 +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
 @@ -74,3 +74,45 @@ phy-consumer@1234 {
  
  Refer to DT bindings documentation of particular PHY consumer devices for 
 more
  information about required PHYs and the way of specification.
 +
 +Samsung Exynos5 SoC series USB DRD PHY controller
 +--
 +
 +Required properties:
 +- compatible : Should be set to one of the following supported values:
 + - samsung,exynos5250-usbdrd-phy - for exynos5250 SoC,
 + - samsung,exynos5420-usbdrd-phy - for exynos5420 SoC.
 +- reg : Register offset and length of USB DRD PHY register set;
 +- clocks: Clock IDs array as required by the controller
 +- clock-names: names of clocks correseponding to IDs in the clock property;
 +Required clocks:
 + - phy: main PHY clock (same as USB DRD controller i.e. DWC3 IP clock),
 +used for register access.
 + - ref: PHY's reference clock (usually crystal clock), associated by
 +phy name, used to determine bit values for clock settings
 +register.
 + Additional clock required for Exynos5420:
 + - usb30_sclk_100m: Additional special clock used for PHY operation
 +depicted as 'sclk_usbphy30' in CMU of Exynos5420.
 +- samsung,syscon-phandle: phandle for syscon interface, which is used to
 +   control pmu registers for power isolation.

Why to append -phandle to the property's name ? If this is for PMU
perhaps make it more explicit and name it: samsung,pmu-syscon or
samsung,pmureg ?

 +- samsung,pmu-offset: phy power control register offset to 
 pmu-system-controller
 +   base.
 +- #phy-cells : from the generic PHY bindings, must be 1;
 +
 +For samsung,exynos5250-usbdrd-phy and samsung,exynos5420-usbdrd-phy
 +compatible PHYs, the second cell in the PHY specifier identifies the
 +PHY id, which is interpreted as follows:
 +  0 - UTMI+ type phy,
 +  1 - PIPE3 type phy,

-- 
Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

2014-04-09 Thread Sylwester Nawrocki 
Hi Vivek,

On 09/04/14 13:54, Vivek Gautam wrote:
 Adding support to enable/disable VBUS hooked to a gpio
 to enable vbus supply on the port.

Does the GPIO control a fixed voltage regulator ? If so, shouldn't
it be modelled by the regulator API instead ?

 Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
[...]
 + /* Get required GPIO for vbus */
 + phy_drd-gpio = of_get_named_gpio(dev-of_node,
 +   samsung,vbus-gpio, 0);
 + if (!gpio_is_valid(phy_drd-gpio))
 + dev_dbg(dev, no usbdrd-phy vbus gpio defined\n);
 +
 + if (devm_gpio_request(dev, phy_drd-gpio, phydrd_vbus_gpio))
 + dev_dbg(dev, can't request usbdrd-phy vbus gpio %d\n,
 + phy_drd-gpio);

--
Regards,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

2014-04-09 Thread Sylwester Nawrocki 
On 09/04/14 14:24, Vivek Gautam wrote:
 On Wed, Apr 9, 2014 at 5:41 PM, Sylwester Nawrocki
 s.nawro...@samsung.com wrote:
  On 09/04/14 13:54, Vivek Gautam wrote:
  Adding support to enable/disable VBUS hooked to a gpio
  to enable vbus supply on the port.
 
  Does the GPIO control a fixed voltage regulator ? If so, shouldn't
  it be modelled by the regulator API instead ?

 No, this GPIO controls a 'current limiting power distribution switch',
 which gives the output vbus to usb controller.
 Should i model this as a fixed regulator ?

OK, it's just a power switch then. I suspect using the regulator API
would be more universal, as such a GPIO is somewhat a board design
detail. I'm not going to object to your patch, just might be better
to use the gpiod API instead.

-- 
Regards,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

2013-08-13 Thread Sylwester Nawrocki 

W dniu 2013-08-13 14:05, Kishon Vijay Abraham I pisze:

On Tuesday 13 August 2013 05:07 PM, Tomasz Figa wrote:

On Tuesday 13 of August 2013 16:14:44 Kishon Vijay Abraham I wrote:

On Wednesday 31 July 2013 11:45 AM, Felipe Balbi wrote:

On Wed, Jul 31, 2013 at 11:14:32AM +0530, Kishon Vijay Abraham I wrote:

IMHO we need a lookup method for PHYs, just like for clocks,
regulators, PWMs or even i2c busses because there are complex
cases
when passing just a name using platform data will not work. I
would
second what Stephen said [1] and define a structure doing things
in a
DT-like way.

Example;

[platform code]

static const struct phy_lookup my_phy_lookup[] = {

PHY_LOOKUP(s3c-hsotg.0, otg, samsung-usbphy.1, phy.2),


The only problem here is that if *PLATFORM_DEVID_AUTO* is used
while
creating the device, the ids in the device name would change and
PHY_LOOKUP wont be useful.


I don't think this is a problem. All the existing lookup methods
already
use ID to identify devices (see regulators, clkdev, PWMs, i2c,
...). You
can simply add a requirement that the ID must be assigned manually,
without using PLATFORM_DEVID_AUTO to use PHY lookup.


And I'm saying that this idea, of using a specific name and id, is
frought with fragility and will break in the future in various ways
when
devices get added to systems, making these strings constantly have
to be
kept up to date with different board configurations.

People, NEVER, hardcode something like an id.  The fact that this
happens today with the clock code, doesn't make it right, it makes
the
clock code wrong.  Others have already said that this is wrong there
as
well, as systems change and dynamic ids get used more and more.

Let's not repeat the same mistakes of the past just because we
refuse to
learn from them...

So again, the find a phy by a string functions should be removed,
the
device id should be automatically created by the phy core just to
make
things unique in sysfs, and no driver code should _ever_ be reliant
on
the number that is being created, and the pointer to the phy
structure
should be used everywhere instead.

With those types of changes, I will consider merging this subsystem,
but
without them, sorry, I will not.


I'll agree with Greg here, the very fact that we see people trying to
add a requirement of *NOT* using PLATFORM_DEVID_AUTO already points
to a big problem in the framework.

The fact is that if we don't allow PLATFORM_DEVID_AUTO we will end up
adding similar infrastructure to the driver themselves to make sure
we
don't end up with duplicate names in sysfs in case we have multiple
instances of the same IP in the SoC (or several of the same PCIe
card).
I really don't want to go back to that.


If we are using PLATFORM_DEVID_AUTO, then I dont see any way we can
give the correct binding information to the PHY framework. I think we
can drop having this non-dt support in PHY framework? I see only one
platform (OMAP3) going to be needing this non-dt support and we can
use the USB PHY library for it.

you shouldn't drop support for non-DT platform, in any case we lived
without DT (and still do) for years. Gotta find a better way ;-)


hmm..

how about passing the device names of PHY in platform data of the
controller? It should be deterministic as the PHY framework assigns its
own id and we *don't* want to add any requirement that the ID must be
assigned manually without using PLATFORM_DEVID_AUTO. We can get rid of
*phy_init_data* in the v10 patch series.


OK, so the PHY device name would have a fixed part, passed as
platform data of the controller and a variable part appended
by the PHY core, depending on the number of registered PHYs ?

Then same PHY names would be passed as the PHY provider driver's
platform data ?

Then if there are 2 instances of the above (same names in platform
data) how would be determined which PHY controller is linked to
which PHY supplier ?

I guess you want each device instance to have different PHY device
names already in platform data ? That might work. We probably will
be focused mostly on DT anyway. It seem without DT we are trying
to find some layer that would allow us to couple relevant devices
and overcome driver core inconvenience that it provides to means
to identify specific devices in advance. :) Your proposal sounds
reasonable, however I might be missing some details or corner cases.


What about slightly altering the concept of v9 to pass a pointer to struct
device instead of device name inside phy_init_data?


As Felipe said, we don't want to pass pointers in platform_data
to/from random subsystems. We pass data, passing pointers would
be a total mess IMHO.


The problem is device might be created very late. (For example in omap4, usb2
phy device gets created when ocp2scp bus is probed). And we have to pass the
init data in board file.


Regards,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More

Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling

2013-08-07 Thread Sylwester Nawrocki 
On 08/07/2013 07:06 PM, Julius Werner wrote:
 This breaks compatibility, both for an old kernel and a new dt and a new
 kernel with an old dt. Is anyone using these bindings?
 
 They only affect Samsung SoCs and have only been upstream for half a
 year, so I doubt it's heavily used.

It probably wouldn't be of much concern, but I can't tell for sure.

 Why are we describing fewer registers now? Are they described elsewhere?

 The dt should describe the device, not only the portion of it Linux
 wants to use right now.
 
 This only ever described a small section of the huge set of PMU
 registers anyway. 

The PMU registers are already scattered across various drivers, like
Power Domain, various PHYs (USB/HSIC/MIPI CSI-2/DSI, ADC, ...), Reset
and more. And it happens currently there is no central driver for
the Power Management Unit, that would, for example expose the regmap
interface that the interested drivers could use. The downside of this
would be that each, e.g. USB PHY driver would need to know SoC specific
offsets to its registers in the PMU.

Before device tree started to be used those PMU registers were controlled
by respective drivers, mostly through platform_data callbacks. As they
could be considered parts of given device.

   Before it described up to three registers
 controlling different PHYs (using hardcoded offsets in the code to
 later find the right one)... with my patch every PHY's DT entry only
 describes the one register concerning itself, which makes more sense
 in my opinion. It will also prevent the register descriptions in
 different DT entries from overlapping.

Yes, the patch looks sensible. Since currently each USB PHY has its
own device tree node it looks wrong to have the reg property in the
usbphy-sys subnodes defining register region for all possible PHYs.
And we indeed and up with multiple reg properties pointing to same
register region.

However the biggest drawback is breaking backwards compatibility.
I'm not sure if those changes are worth it, especially that all those
USB PHY drivers are supposed to be rewritten to use the generic PHY API.


Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

2013-07-25 Thread Sylwester Nawrocki 
On 07/24/2013 08:32 PM, Arnd Bergmann wrote:
 On Tuesday 23 July 2013, Tomasz Figa wrote:
 On Tuesday 23 of July 2013 17:14:20 Alan Stern wrote:
 On Tue, 23 Jul 2013, Tomasz Figa wrote:
 Where would you want to have those phy_address arrays stored? There
 are no board files when booting with DT. Not even saying that you
 don't need to use any hacky schemes like this when you have DT that
 nicely specifies relations between devices.

 If everybody agrees DT has a nice scheme for specifying relations
 between devices, why not use that same scheme in the PHY core?

 It is already used, for cases when consumer device has a DT node attached. 
 In non-DT case this kind lookup translates loosely to something that is 
 being done in regulator framework - you can't bind devices by pointers, 
 because you don't have those pointers, so you need to use device names.

 
 Sorry for jumping in to the middle of the discussion, but why does a *new*
 framework even bother defining an interface for board files?
 
 Can't we just drop any interfaces for platform data passing in the phy
 framework and put the burden of adding those to anyone who actually needs
 them? All the platforms we are concerned with here (exynos and omap,
 plus new platforms) can be booted using DT anyway.

Indeed, I was also a bit surprised we still need non-dt support, since
migration to this generic PHY framework in case of exynos was solely
part of migration of the whole platform to DT.

Two of the drivers that are being converted are also used on s5pv210,
but there is currently no boards in mainline that would use devices
covered by those drivers and s5pv210 will very likely get DT support
in v3.13 anyway.

But it seems omap still needs non-dt support in the PHY framework.

---
Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

2013-07-21 Thread Sylwester Nawrocki 

On 07/21/2013 05:48 PM, Greg KH wrote:

On Sun, Jul 21, 2013 at 12:22:48PM +0200, Sascha Hauer wrote:

On Sat, Jul 20, 2013 at 07:59:10PM -0700, Greg KH wrote:

On Sat, Jul 20, 2013 at 10:32:26PM -0400, Alan Stern wrote:

On Sat, 20 Jul 2013, Greg KH wrote:


That should be passed using platform data.


Ick, don't pass strings around, pass pointers.  If you have platform
data you can get to, then put the pointer there, don't use a name.


I don't think I understood you here :-s We wont have phy pointer
when we create the device for the controller no?(it'll be done in
board file). Probably I'm missing something.


Why will you not have that pointer?  You can't rely on the name as the
device id will not match up, so you should be able to rely on the
pointer being in the structure that the board sets up, right?

Don't use names, especially as ids can, and will, change, that is going
to cause big problems.  Use pointers, this is C, we are supposed to be
doing that :)


Kishon, I think what Greg means is this:  The name you are using must
be stored somewhere in a data structure constructed by the board file,
right?  Or at least, associated with some data structure somehow.
Otherwise the platform code wouldn't know which PHY hardware
corresponded to a particular name.

Greg's suggestion is that you store the address of that data structure
in the platform data instead of storing the name string.  Have the
consumer pass the data structure's address when it calls phy_create,
instead of passing the name.  Then you don't have to worry about two
PHYs accidentally ending up with the same name or any other similar
problems.


Close, but the issue is that whatever returns from phy_create() should
then be used, no need to call any find functions, as you can just use
the pointer that phy_create() returns.  Much like all other class api
functions in the kernel work.


I think the problem here is to connect two from the bus structure
completely independent devices. Several frameworks (ASoC, soc-camera)
had this problem and this wasn't solved until the advent of devicetrees
and their phandles.
phy_create might be called from the probe function of some i2c device
(the phy device) and the resulting pointer is then needed in some other
platform devices (the user of the phy) probe function.
The best solution we have right now is implemented in the clk framework
which uses a string matching of the device names in clk_get() (at least
in the non-dt case).


I would argue that clocks are wrong here as well, as others have already
pointed out.

What's wrong with the platform_data structure, why can't that be used
for this?


At the point the platform data of some driver is initialized, e.g. in
board setup code the PHY pointer is not known, since the PHY supplier
driver has not initialized yet.  Even though we wanted to pass pointer
to a PHY through some notifier call, it would have been not clear
which PHY user driver should match on such notifier.  A single PHY
supplier driver can create M PHY objects and this needs to be mapped
to N PHY user drivers.  This mapping needs to be defined somewhere by
the system integrator.  It works well with device tree, but except that
there seems to be no other reliable infrastructure in the kernel to
define links/dependencies between devices, since device identifiers are
not known in advance in all cases.

What Tomasz proposed seems currently most reasonable to me for non-dt.


Or, if not, we can always add pointers to the platform device structure,
or even the main 'struct device' as well, that's what it is there for.


Still we would need to solve a problem which platform device structure
gets which PHY pointer.

--
Regards,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 0/9] Generic PHY Framework

2013-06-24 Thread Sylwester Nawrocki 
Hi,

On 06/18/2013 11:49 AM, Felipe Balbi wrote:
 On Mon, Jun 17, 2013 at 12:16:35PM +0200, Sylwester Nawrocki wrote:
 I have already used this API for our MIPI CSI-2/DSIM DPHYs driver,
 the RFC patch series can be found at [1].

 Thanks,
 Sylwester

 [1] http://www.spinics.net/lists/arm-kernel/msg251666.html
 
 one comment to that series:
 
 let's make the phy names standardized, how about phy-exynos-video-mipi.c
 or phy-mipi-csi-dsim.c ?

Yes, I have been thinking about that, wasn't sure exactly what pattern to
chose. I would make it phy-exynos-mipi-video.c, phy-exynos-dsim-csis.c feels
a bit odd. phy-mipi-csis-dsim.c might be to generic as MIPI CSIS stands for
MIPI CSI Slave and MIPI DSIM - MIPI DSI Master. And there might be (actually
are) IP blocks in an SoC that use the other MIPI Aliance standards.
For the HDMI PHY I guess just phy-exynos-hdmi.c could be used.

Thanks,
Sylwester

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

Re: [PATCH v7 1/9] drivers: phy: add generic PHY framework

2013-06-19 Thread Sylwester Nawrocki 
Hi,

On 06/13/2013 10:43 AM, Kishon Vijay Abraham I wrote:
 +/**
 + * phy_create() - create a new phy
 + * @dev: device that is creating the new phy
 + * @id: id of the phy
 + * @ops: function pointers for performing phy operations
 + * @label: label given to the phy
 + * @priv: private data from phy driver
 + *
 + * Called to create a phy using phy framework.
 + */
 +struct phy *phy_create(struct device *dev, u8 id, const struct phy_ops *ops,
 + const char *label, void *priv)
 +{
 + int ret;
 + struct phy *phy;
 +
 + if (!dev) {
 + dev_WARN(dev, no device provided for PHY\n);
 + ret = -EINVAL;
 + goto err0;
 + }
 +
 + phy = kzalloc(sizeof(*phy), GFP_KERNEL);
 + if (!phy) {
 + ret = -ENOMEM;
 + goto err0;
 + }
 +
 + device_initialize(phy-dev);
 +
 + phy-dev.class = phy_class;
 + phy-dev.parent = dev;
 + phy-dev.of_node = dev-of_node;
 + phy-id = id;
 + phy-ops = ops;
 + phy-label = label;

Perhaps we could make it:

phy-label = kstrdup(label, GFP_KERNEL);

and free the label in phy_destroy() ?

That way the users would't need to keep the label around. It might
be useful when PHY provider generates the labels dynamically. I guess
it is OK for PHY providers to hard code the labels and having the PHY
user drivers passed labels in platform data ?

 + dev_set_drvdata(phy-dev, priv);
 +
 + ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id);
 + if (ret)
 + goto err1;
 +
 + ret = device_add(phy-dev);
 + if (ret)
 + goto err1;
 +
 + if (pm_runtime_enabled(dev))
 + pm_runtime_enable(phy-dev);
 +
 + return phy;
 +
 +err1:
 + put_device(phy-dev);
 + kfree(phy);
 +
 +err0:
 + return ERR_PTR(ret);
 +}
 +EXPORT_SYMBOL_GPL(phy_create);

 +/**
 + * phy_destroy() - destroy the phy
 + * @phy: the phy to be destroyed
 + *
 + * Called to destroy the phy.
 + */
 +void phy_destroy(struct phy *phy)
 +{
 + pm_runtime_disable(phy-dev);
 + device_unregister(phy-dev);

Isn't kfree(phy); missing here ?

 +}
 +EXPORT_SYMBOL_GPL(phy_destroy);

Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 1/9] drivers: phy: add generic PHY framework

2013-06-18 Thread Sylwester Nawrocki 

Hi Kishon,

I've noticed there is a little inconsistency between the code and 
documentation.


On 06/13/2013 10:43 AM, Kishon Vijay Abraham I wrote:

+3. Creating the PHY
+
+The PHY driver should create the PHY in order for other peripheral controllers
+to make use of it. The PHY framework provides 2 APIs to create the PHY.
+
+struct phy *phy_create(struct device *dev, int id, const struct phy_ops *ops,
+   void *priv);
+struct phy *devm_phy_create(struct device *dev, int id,
+   const struct phy_ops *ops, void *priv);


The 'label' argument is missing in those function prototypes. It seems the
labels must be unique, I guess this needs to be documented. And do we allow
NULL labels ? If so, then there is probably a check for NULL label needed
in phy_lookup() and if not, then phy_create() should fail when NULL label
is passed ?


+The PHY drivers can use one of the above 2 APIs to create the PHY by passing
+the device pointer, id, phy ops and a driver data.
+phy_ops is a set of function pointers for performing PHY operations such as
+init, exit, power_on and power_off.



+/**
+ * phy_create() - create a new phy
+ * @dev: device that is creating the new phy
+ * @id: id of the phy
+ * @ops: function pointers for performing phy operations
+ * @label: label given to the phy
+ * @priv: private data from phy driver
+ *
+ * Called to create a phy using phy framework.
+ */
+struct phy *phy_create(struct device *dev, u8 id, const struct phy_ops *ops,
+   const char *label, void *priv)
+{
+   int ret;
+   struct phy *phy;
+
+   if (!dev) {
+   dev_WARN(dev, no device provided for PHY\n);
+   ret = -EINVAL;
+   goto err0;
+   }
+
+   phy = kzalloc(sizeof(*phy), GFP_KERNEL);
+   if (!phy) {
+   ret = -ENOMEM;
+   goto err0;
+   }
+
+   device_initialize(phy-dev);
+
+   phy-dev.class = phy_class;
+   phy-dev.parent = dev;
+   phy-dev.of_node = dev-of_node;
+   phy-id = id;
+   phy-ops = ops;
+   phy-label = label;
+
+   dev_set_drvdata(phy-dev, priv);
+
+   ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id);
+   if (ret)
+   goto err1;
+
+   ret = device_add(phy-dev);
+   if (ret)
+   goto err1;
+
+   if (pm_runtime_enabled(dev))
+   pm_runtime_enable(phy-dev);
+
+   return phy;
+
+err1:
+   put_device(phy-dev);
+   kfree(phy);
+
+err0:
+   return ERR_PTR(ret);
+}


Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 0/9] Generic PHY Framework

2013-06-17 Thread Sylwester Nawrocki 
Hi,

On 06/13/2013 10:43 AM, Kishon Vijay Abraham I wrote:
 Added a generic PHY framework that provides a set of APIs for the PHY drivers
 to create/destroy a PHY and APIs for the PHY users to obtain a reference to
 the PHY with or without using phandle.
 
 This framework will be of use only to devices that uses external PHY (PHY
 functionality is not embedded within the controller).
 
 The intention of creating this framework is to bring the phy drivers spread
 all over the Linux kernel to drivers/phy to increase code re-use and to
 increase code maintainability.
 
 Comments to make PHY as bus wasn't done because PHY devices can be part of
 other bus and making a same device attached to multiple bus leads to bad
 design.
 
 If the PHY driver has to send notification on connect/disconnect, the PHY
 driver should make use of the extcon framework. Using this susbsystem 
 to use extcon framwork will have to be analysed.
 
 Making omap-usb2 and twl4030 to use this framework is provided as a sample.
 
 This patch series is developed on linux mainline tree.
 
 Changes from v6
 * corrected few typos in Documentation
 * Changed PHY Subsystem to *bool* in Kconfig (to avoid compilation errors when
   PHY Subsystem is kept as module and the dependent modules are built-in)
 * Added if pm_runtime_enabled check before runtime pm calls.

Looks good, feel free to add:

Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com for patches
1...3/9, 5...7/9

and Tested-by: Sylwester Nawrocki s.nawro...@samsung.com for patch 1/9

Hopefully we can gather more Acks and merge it for 3.11.

I have already used this API for our MIPI CSI-2/DSIM DPHYs driver,
the RFC patch series can be found at [1].

Thanks,
Sylwester

[1] http://www.spinics.net/lists/arm-kernel/msg251666.html
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 1/9] drivers: phy: add generic PHY framework

2013-06-04 Thread Sylwester Nawrocki 
Hi,

On 05/29/2013 07:38 AM, Kishon Vijay Abraham I wrote:
 On Wednesday 29 May 2013 04:07 AM, Sylwester Nawrocki wrote:
  On 04/29/2013 12:03 PM, Kishon Vijay Abraham I wrote:
  The PHY framework provides a set of APIs for the PHY drivers to
  create/destroy a PHY and APIs for the PHY users to obtain a reference
  to the
  PHY with or without using phandle. For dt-boot, the PHY drivers should
  also register *PHY provider* with the framework.
 
  PHY drivers should create the PHY by passing id and ops like init, exit,
  power_on and power_off. This framework is also pm runtime enabled.
 
  The documentation for the generic PHY framework is added in
  Documentation/phy.txt and the documentation for dt binding can be
  found at
  Documentation/devicetree/bindings/phy/phy-bindings.txt
 
  Signed-off-by: Kishon Vijay Abraham Ikis...@ti.com
 
  Thanks for working on this. For the record, I plan to give this a try
  in the end of this week, with my simple MIPI CSI/DSI PHY driver. I might
  have some more comments then. For now just couple of remarks after
  reading the documentation.

 Thanks for reviewing. I'll wait for your comments before posting the 
 next version.

So I've used this API for the Exynos SoC MIPI CSI-2 and MIPI DSI DPHYs.
I could remove all the local modifications, comparing to your v5, since 
this iteration already have what's needed, thanks! In my case the PHY 
provider was a platform device and so were the PHY consumer devices. Those 
PHYs are very simple, there is less than one register per PHY (some bits 
are shared across the MIPI CSI-2 receiver and MIPI DSI transmitter DPHYs), 
but having this generic PHY driver means, among others, there is a proper 
DT support. I could finally get rid of the the platform callback at 
arch/arm/plat-samsung/setup-mipiphy.c.

I found this code useful as is, except previous minor comments I don't 
really have more remarks now. This API looks quite good, and it seems much 
lighter comparing to the original version.

I assume, the way it is designed now, allows it to be used also with PHYs that 
hang off other buses, e.g. I2C.

It would be nice to get this in 3.11.

Hmm, actually I have some doubts, let me comment in other e-mail..


Regards,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 1/9] drivers: phy: add generic PHY framework

2013-06-04 Thread Sylwester Nawrocki 
On 04/29/2013 12:03 PM, Kishon Vijay Abraham I wrote:
 The PHY framework provides a set of APIs for the PHY drivers to
 create/destroy a PHY and APIs for the PHY users to obtain a reference to the
 PHY with or without using phandle. For dt-boot, the PHY drivers should
 also register *PHY provider* with the framework.
 
 PHY drivers should create the PHY by passing id and ops like init, exit,
 power_on and power_off. This framework is also pm runtime enabled.
 
 The documentation for the generic PHY framework is added in
 Documentation/phy.txt and the documentation for dt binding can be found at
 Documentation/devicetree/bindings/phy/phy-bindings.txt
 
 Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
 ---
  .../devicetree/bindings/phy/phy-bindings.txt   |   66 +++
  Documentation/phy.txt  |  123 +
  MAINTAINERS|7 +
  drivers/Kconfig|2 +
  drivers/Makefile   |2 +
  drivers/phy/Kconfig|   13 +
  drivers/phy/Makefile   |5 +
  drivers/phy/phy-core.c |  539 
 
  include/linux/phy/phy.h|  248 +
  9 files changed, 1005 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/phy/phy-bindings.txt
  create mode 100644 Documentation/phy.txt
  create mode 100644 drivers/phy/Kconfig
  create mode 100644 drivers/phy/Makefile
  create mode 100644 drivers/phy/phy-core.c
  create mode 100644 include/linux/phy/phy.h
 
 +static inline int phy_init(struct phy *phy)
 +{
 + pm_runtime_get_sync(phy-dev);

Hmm, no need to check return value here ? Also it looks a bit unexpected to 
possibly have runtime_resume callback of a PHY device called before ops-init()
call ? It seems a bit unclear what the purpose of init() callback is.

 + if (phy-ops-init)
 + return phy-ops-init(phy);
 +
 + return -EINVAL;
 +}
 +
 +static inline int phy_exit(struct phy *phy)
 +{
 + int ret = -EINVAL;
 +
 + if (phy-ops-exit)
 + ret = phy-ops-exit(phy);
 +
 + pm_runtime_put_sync(phy-dev);
 +
 + return ret;
 +}

Do phy_init/phy_exit need to be mandatory ? What if there is really 
nothing to do in those callbacks ? Perhaps -ENOIOCTLCMD should be 
returned if a callback is not implemented, so PHY users can recognize 
such situation and proceed ?

 +static inline int phy_power_on(struct phy *phy)
 +{
 + if (phy-ops-power_on)
 + return phy-ops-power_on(phy);
 +
 + return -EINVAL;
 +}
 +
 +static inline int phy_power_off(struct phy *phy)
 +{
 + if (phy-ops-power_off)
 + return phy-ops-power_off(phy);
 +
 + return -EINVAL;
 +}
 +
 +static inline int phy_pm_runtime_get(struct phy *phy)
 +{
 + if (WARN(IS_ERR(phy), Invalid PHY reference\n))
 + return -EINVAL;
 +
 + return pm_runtime_get(phy-dev);
 +}
 +
 +static inline int phy_pm_runtime_get_sync(struct phy *phy)
 +{
 + if (WARN(IS_ERR(phy), Invalid PHY reference\n))
 + return -EINVAL;
 +
 + return pm_runtime_get_sync(phy-dev);
 +}
 +
 +static inline int phy_pm_runtime_put(struct phy *phy)
 +{
 + if (WARN(IS_ERR(phy), Invalid PHY reference\n))
 + return -EINVAL;
 +
 + return pm_runtime_put(phy-dev);
 +}
 +
 +static inline int phy_pm_runtime_put_sync(struct phy *phy)
 +{
 + if (WARN(IS_ERR(phy), Invalid PHY reference\n))
 + return -EINVAL;
 +
 + return pm_runtime_put_sync(phy-dev);
 +}
 +
 +static inline void phy_pm_runtime_allow(struct phy *phy)
 +{
 + if (WARN(IS_ERR(phy), Invalid PHY reference\n))
 + return;
 +
 + pm_runtime_allow(phy-dev);
 +}
 +
 +static inline void phy_pm_runtime_forbid(struct phy *phy)
 +{
 + if (WARN(IS_ERR(phy), Invalid PHY reference\n))
 + return;
 +
 + pm_runtime_forbid(phy-dev);
 +}

Do we need to have all these runtime PM wrappers ? I guess you 
intended to avoid referencing phy-dev from the PHY consumers ?


Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 1/9] drivers: phy: add generic PHY framework

2013-06-04 Thread Sylwester Nawrocki 
Hi,

On 06/04/2013 02:26 PM, Kishon Vijay Abraham I wrote:
 +static inline int phy_init(struct phy *phy)
 +{
 +   pm_runtime_get_sync(phy-dev);

 Hmm, no need to check return value here ? Also it looks a bit unexpected to
 
 I purposely dint check the return values in order to support platforms 
 that don’t enable pm_runtime.

Then I guess this should be called conditionally and any errors returned
if runtime PM is enabled ? Not sure if pm_runtime_enabled() would be
helpful such situation.

 possibly have runtime_resume callback of a PHY device called before 
 ops-init()
 call ? It seems a bit unclear what the purpose of init() callback is.
 
 Not really. Anything that is used to initialize the PHY (internal 
 configuration) can go in phy_init. Usually in runtime_resume callback, 
 optional functional clocks are enabled and also in some cases context 
 restore is done. So it really makes sense to enable clocks/module 
 (pm_runtime_get_sync) before doing a PHY configuration (phy_init).

OK, that makes sense. All PHY device resources must be prepared anyway
before a PHY object is registered with the PHY core.

 +   if (phy-ops-init)
 +   return phy-ops-init(phy);
 +
 +   return -EINVAL;
 +}
 +
 +static inline int phy_exit(struct phy *phy)
 +{
 +   int ret = -EINVAL;
 +
 +   if (phy-ops-exit)
 +   ret = phy-ops-exit(phy);
 +
 +   pm_runtime_put_sync(phy-dev);
 +
 +   return ret;
 +}

 Do phy_init/phy_exit need to be mandatory ? What if there is really
 
 No. phy_init/phy_exit is not mandatory at all.
 nothing to do in those callbacks ? Perhaps -ENOIOCTLCMD should be
 returned if a callback is not implemented, so PHY users can recognize
 such situation and proceed ?
 
 So currently these APIs return -EINVAL if these callbacks are not 
 populated which is good enough IMHO.

But -EINVAL could be well returned from the callback function. Perhaps
ENOTSUPP could be used instead ?


Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 1/9] drivers: phy: add generic PHY framework

2013-05-28 Thread Sylwester Nawrocki 

Hi Kishon,

On 04/29/2013 12:03 PM, Kishon Vijay Abraham I wrote:

The PHY framework provides a set of APIs for the PHY drivers to
create/destroy a PHY and APIs for the PHY users to obtain a reference to the
PHY with or without using phandle. For dt-boot, the PHY drivers should
also register *PHY provider* with the framework.

PHY drivers should create the PHY by passing id and ops like init, exit,
power_on and power_off. This framework is also pm runtime enabled.

The documentation for the generic PHY framework is added in
Documentation/phy.txt and the documentation for dt binding can be found at
Documentation/devicetree/bindings/phy/phy-bindings.txt

Signed-off-by: Kishon Vijay Abraham Ikis...@ti.com


Thanks for working on this. For the record, I plan to give this a try
in the end of this week, with my simple MIPI CSI/DSI PHY driver. I might
have some more comments then. For now just couple of remarks after
reading the documentation.


---
  .../devicetree/bindings/phy/phy-bindings.txt   |   66 +++
  Documentation/phy.txt  |  123 +
  MAINTAINERS|7 +
  drivers/Kconfig|2 +
  drivers/Makefile   |2 +
  drivers/phy/Kconfig|   13 +
  drivers/phy/Makefile   |5 +
  drivers/phy/phy-core.c |  539 
  include/linux/phy/phy.h|  248 +
  9 files changed, 1005 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/phy/phy-bindings.txt
  create mode 100644 Documentation/phy.txt
  create mode 100644 drivers/phy/Kconfig
  create mode 100644 drivers/phy/Makefile
  create mode 100644 drivers/phy/phy-core.c
  create mode 100644 include/linux/phy/phy.h

diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt 
b/Documentation/devicetree/bindings/phy/phy-bindings.txt
new file mode 100644
index 000..8ae844f
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
@@ -0,0 +1,66 @@
+This document explains only the device tree data binding. For general
+information about PHY subsystem refer to Documentation/phy.txt
+
+PHY device node
+===
+
+Required Properties:
+#phy-cells:Number of cells in a PHY specifier;  The meaning of all those
+   cells is defined by the binding for the phy node. The PHY
+   provider can use the values in cells to find the appropriate
+   PHY.
+
+For example:
+
+phys: phy {
+compatible = xxx;
+reg =...;
+.
+.
+#phy-cells =1;
+.
+.
+};
+
+That node describes an IP block (PHY provider) that implements 2 different 
PHYs.
+In order to differentiate between these 2 PHYs, an additonal specifier should 
be
+given while trying to get a reference to it.
+
+PHY user node
+=
+
+Required Properties:
+phys : the phandle for the PHY device (used by the PHY subsystem)
+phy-names : the names of the PHY corresponding to the PHYs present in the
+   *phys* phandle
+
+Example 1:
+usb1: usb_otg_ss@xxx {
+compatible = xxx;
+reg =xxx;
+.
+.
+phys =usb2_phy,usb3_phy;
+phy-names = usb2phy, usb3phy;
+.
+.
+};
+
+This node represents a controller that uses two PHYs, one for usb2 and one for
+usb3.
+
+Example 2:
+usb2: usb_otg_ss@xxx {
+compatible = xxx;
+reg =xxx;
+.
+.
+phys =phys 1;
+phy-names = usbphy;
+.
+.
+};
+
+This node represents a controller that uses one of the PHYs of the PHY provider
+device defined previously. Note that the phy handle has an additional specifier
+1 to differentiate between the two PHYs.
diff --git a/Documentation/phy.txt b/Documentation/phy.txt
new file mode 100644
index 000..408d25f
--- /dev/null
+++ b/Documentation/phy.txt
@@ -0,0 +1,123 @@
+   PHY SUBSYSTEM
+ Kishon Vijay Abraham Ikis...@ti.com
+
+This document explains the Generic PHY Framework along with the APIs provided,
+and how-to-use.
+
+1. Introduction
+
+*PHY* is the abbreviation for physical layer. It is used to connect a device
+to the physical medium e.g., the USB controller has a PHY to provide functions
+such as serialization, de-serialization, encoding, decoding and is responsible
+for obtaining the required data transmission rate. Note that some USB
+controller has PHY functionality embedded into it and others use an external


Note that some USB
controllers have PHY functionality embedded into them... ?


+PHY. Other peripherals that uses a PHY include Wireless LAN, Ethernet,


s/uses/use ?


+SATA etc.
+
+The intention of creating this framework is to bring the phy drivers spread


s/phy/PHY ?


+all over the Linux kernel to drivers/phy to increase code re-use and for
+better code maintainability.
+
+This framework will be of use only to devices that uses external PHY (PHY


s/that

Re: [PATCH v3 0/6] Generic PHY Framework

2013-04-15 Thread Sylwester Nawrocki 
On 04/15/2013 12:36 PM, Kishon Vijay Abraham I wrote:
 On Monday 15 April 2013 03:50 PM, Grant Likely wrote:
 On Wed, 20 Mar 2013 14:41:59 +0530, Kishon Vijay Abraham I kis...@ti.com
 wrote:
 Added a generic PHY framework that provides a set of APIs for the PHY 
 drivers
 to create/destroy a PHY and APIs for the PHY users to obtain a reference to
 the PHY with or without using phandle. To obtain a reference to the PHY
 without using phandle, the platform specfic intialization code (say from 
 board
 file) should have already called phy_bind with the binding information. The
 binding information consists of phy's device name, phy user device name and 
 an
 index. The index is used when the same phy user binds to mulitple phys.

 This framework will be of use only to devices that uses external PHY (PHY
 functionality is not embedded within the controller).

 The intention of creating this framework is to bring the phy drivers spread
 all over the Linux kernel to drivers/phy to increase code re-use and to
 increase code maintainability.

 Comments to make PHY as bus wasn't done because PHY devices can be part of
 other bus and making a same device attached to multiple bus leads to bad
 design.

 Making omap-usb2 and twl4030 to use this framework is provided as a sample.

 This patch series is developed on 3.9-rc3. Once the patch series gets 
 finalised
 I'll resend omap-usb2 and twl4030 part based on Felipe's tree.


 [...]

   drivers/Kconfig|2 +
   drivers/Makefile   |2 +
   drivers/phy/Kconfig|   13 +
   drivers/phy/Makefile   |5 +
   drivers/phy/phy-core.c |  574
 

 This looks to be very specific for USB PHYs. Are you intending it to be
 used for other types of PHYs, like Ethernet PHYs? If not, then this
 
 Not really. This can be used by USB, SATA and Sylwester was planning to use it
 for video PHY's.

Yes, I have already some RFC patches to handle the display and the camera
interface DPHYs (MIPI DSI, MIPI CSI-2) with this API. I didn't post it, since
this framework is not settled yet. Those DPHYs need very few operations, like
disable/enable only and there was really not suitable API in the kernel until
now to handle them. We had plans in the past to write something like this 
generic PHY framework for the Samsung SoCs.

Some SoCs have plenty different PHYs: USB, SATA, MIPI CSI-2, MIPI DSI, HDMI... 
And some of them are simple enough to be covered by this generic PHY API.

Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/6] drivers: phy: add generic PHY framework

2013-04-04 Thread Sylwester Nawrocki 
Hi,

On 04/04/2013 11:21 AM, Kishon Vijay Abraham I wrote:
 On Thursday 04 April 2013 03:16 AM, Sylwester Nawrocki wrote:
 On 04/03/2013 02:53 PM, Kishon Vijay Abraham I wrote:

 +4. Getting a reference to the PHY
 +
 +Before the controller can make use of the PHY, it has to get a
 reference to
 +it. This framework provides 6 APIs to get a reference to the PHY.
 +
 +struct phy *phy_get(struct device *dev, int index);
 +struct phy *devm_phy_get(struct device *dev, int index);
 +struct phy *of_phy_get(struct device *dev, const char *phandle, int
 index);
 +struct phy *devm_of_phy_get(struct device *dev, const char *phandle,
 int index);

 Why do we need separate functions for dt and non-dt ? Couldn't it be
 handled
 internally by the framework ? So the PHY users can use same calls for dt
 and non-dt, like in case of e.g. the regulators API ?
 
 yeah. Indeed it makes sense to do it that way.

 Also signatures of some functions are different now:

 extern struct phy *phy_get(struct device *dev, int index);
 extern struct phy *devm_phy_get(struct device *dev, int index);
 extern struct phy *of_phy_get(struct device *dev, int index);
 extern struct phy *devm_of_phy_get(struct device *dev, int index);
 
 My bad :-(
 
 BTW, I think extern could be dropped, does it have any significance in
 function declarations in header files ?
 
 it shouldn't have any effect I guess. It's harmless nevertheless.

Yup.

 +struct phy *of_phy_get_byname(struct device *dev, const char *string);
 +struct phy *devm_of_phy_get_byname(struct device *dev, const char
 *string);

 --- /dev/null
 +++ b/drivers/phy/phy-core.c
 @@ -0,0 +1,616 @@

 +static struct phy *of_phy_lookup(struct device_node *node)
 +{
 +struct phy *phy;
 +struct device *dev;
 +struct class_dev_iter iter;
 +
 +class_dev_iter_init(iter, phy_class, NULL, NULL);

 There is currently nothing preventing a call to this function before
 phy_class is initialized. It happened during my tests, and the nice
 stack dump showed that it was the PHY user attempting to get the PHY
 before the framework got initialized. So either phy_core_init should
 be a subsys_initcall or we need a better protection against phy_class
 being NULL or ERR_PTR in more places.
 
 Whats the initcall used in your PHY user? Looks like more people prefer having

It happened in a regular platform driver probe() callback.

 module_init and any issue because of it should be fixed in PHY providers and
 PHY users.

OK. In fact this uncovered some issues in the MIPI DSI interface driver
(some unexpected interrupts). But this should just be fixed in those drivers
anyway. Now the DSI interface driver needs to wait for the PHY to be
registered and ready, so the initialization will likely be changed regardless
the framework initializes in module_init or earlier.

 +while ((dev = class_dev_iter_next(iter))) {
 +phy = container_of(dev, struct phy, dev);
 +if (node != phy-of_node)
 +continue;
 +
 +class_dev_iter_exit(iter);
 +return phy;
 +}
 +
 +class_dev_iter_exit(iter);
 +return ERR_PTR(-EPROBE_DEFER);
 +}

 +/**
 + * of_phy_get() - lookup and obtain a reference to a phy by phandle
 + * @dev: device that requests this phy
 + * @index: the index of the phy
 + *
 + * Returns the phy associated with the given phandle value,
 + * after getting a refcount to it or -ENODEV if there is no such phy or
 + * -EPROBE_DEFER if there is a phandle to the phy, but the device is
 + * not yet loaded.
 + */
 +struct phy *of_phy_get(struct device *dev, int index)
 +{
 +int ret;
 +struct phy *phy = NULL;
 +struct phy_bind *phy_map = NULL;
 +struct of_phandle_args args;
 +struct device_node *node;
 +
 +if (!dev-of_node) {
 +dev_dbg(dev, device does not have a device node entry\n);
 +return ERR_PTR(-EINVAL);
 +}
 +
 +ret = of_parse_phandle_with_args(dev-of_node, phys, #phy-cells,
 +index,args);
 +if (ret) {
 +dev_dbg(dev, failed to get phy in %s node\n,
 +dev-of_node-full_name);
 +return ERR_PTR(-ENODEV);
 +}
 +
 +phy = of_phy_lookup(args.np);
 +if (IS_ERR(phy) || !try_module_get(phy-ops-owner)) {
 +phy = ERR_PTR(-EPROBE_DEFER);
 +goto err0;
 +}
 +
 +phy = phy-ops-of_xlate(phy,args);
 +if (IS_ERR(phy))
 +goto err1;
 +
 +phy_map = phy_bind(dev_name(dev), index, dev_name(phy-dev));
 +if (!IS_ERR(phy_map)) {
 +phy_map-phy = phy;
 +phy_map-auto_bind = true;

 Shouldn't it be done with the phy_bind_mutex held ? I guess an unlocked
 version of the phy_bind functions is needed, so it can be used internally.
 
 The locking is done inside phy_bind function. I'm not sure but I vaguely
 remember getting a dump stack (incorrect mutex ordering or something) when
 trying to have phy_bind_mutex here. I can check it again.

Yes, IIUC the locking needs to be reworked a bit so phy_map is not modified
without the mutex held

Re: [PATCH v5 1/6] drivers: phy: add generic PHY framework

2013-04-03 Thread Sylwester Nawrocki 

On 04/03/2013 02:53 PM, Kishon Vijay Abraham I wrote:

--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
@@ -0,0 +1,67 @@
+This document explains only the dt data binding. For general information about


s/dt data/device tree ?


+PHY subsystem refer Documentation/phy.txt


s/refer/refer to ?


+
+PHY device node
+===
+
+Required Properties:
+#phy-cells:Number of cells in a PHY specifier;  The meaning of all those
+   cells is defined by the binding for the phy node. The PHY
+   provider can use the values in cells to find the appropriate
+   PHY.
+
+For example:
+
+phys: phy {
+compatible = xxx;
+reg =...;
+.
+.
+#phy-cells =1;
+.
+.
+};
+
+That node describes an IP block that implements 2 different PHYs. In order to
+differentiate between these 2 PHYs, an additonal specifier should be given
+while trying to get a reference to it.
+
+PHY user node
+=
+
+Required Properties:
+phys : the phandle for the PHY device (used by the PHY subsystem)
+
+Optional properties:
+phy-names : the names of the PHY corresponding to the PHYs present in the
+   *phys* phandle
+
+Example 1:
+usb1: usb_otg_ss@xxx {
+compatible = xxx;
+reg =xxx;
+.
+.
+phys =usb2_phy,usb3_phy;
+phy-names = usb2phy, usb3phy;
+.
+.
+};
+
+This node represents a controller that uses two PHYs one for usb2 and one for


s/PHYs/PHYs,  ?


+usb3.
+
+Example 2:
+usb2: usb_otg_ss@xxx {
+compatible = xxx;
+reg =xxx;
+.
+.
+phys =phys 1;
+.
+.
+};
+
+This node represents a controller that uses one of the PHYs which is defined
+previously. Note that the phy handle has an additional specifier 1 to


I find it a bit difficult to parse. Perhaps

This node represents a controller that uses one of the PHYs of the PHY 
provider

device defined previously. ...

or something similar ?


+differentiate between the two PHYs.


s/the two/the two available  ?


diff --git a/Documentation/phy.txt b/Documentation/phy.txt
new file mode 100644
index 000..7785ec0
--- /dev/null
+++ b/Documentation/phy.txt
@@ -0,0 +1,113 @@
+   PHY SUBSYSTEM
+ Kishon Vijay Abraham Ikis...@ti.com
+
+This document explains the Generic PHY Framework along with the APIs provided,
+and how-to-use.
+
+1. Introduction
+
+*PHY* is the abbreviation for physical layer. It is used to connect a device
+to the physical medium e.g., the USB controller has a PHY to provide functions


Shouldn't it be ...medium, e.g. the USB... ?


+such as serialization, de-serialization, encoding, decoding and is responsible
+for obtaining the required data transmission rate. Note that some USB
+controller has PHY functionality embedded into it and others use an external


controllers have PHY functionality embedded into them ?


+PHY. Other peripherals that uses a PHY include Wireless LAN, Ethernet,


s/uses/use ?


+SATA etc.
+
+The intention of creating this framework is to bring the phy drivers spread
+all over the Linux kernel to drivers/phy to increase code re-use and to
+increase code maintainability.
+
+This framework will be of use only to devices that uses external PHY (PHY


s/uses/use ?


+functionality is not embedded within the controller).
+
+2. Creating the PHY
+
+The PHY driver should create the PHY in order for other peripheral controllers
+to make use of it. The PHY framework provides 2 APIs to create the PHY.
+
+struct phy *phy_create(struct device *dev, const char *label,
+   struct device_node *of_node, int type, struct phy_ops *ops,
+   void *priv);
+struct phy *devm_phy_create(struct device *dev, const char *label,
+   struct device_node *of_node, int type, struct phy_ops *ops,
+   void *priv);
+
+The PHY drivers can use one of the above 2 APIs to create the PHY by passing
+the device pointer, label, device node, type, phy ops and a driver data.
+phy_ops is a set of function pointers for performing PHY operations such as
+init, exit, suspend, resume, power_on and power_off.
+
+3. Binding the PHY to the controller
+
+The framework provides an API for binding the controller to the PHY in the
+case of non dt boot.
+
+struct phy_bind *phy_bind(const char *dev_name, int index,
+   const char *phy_dev_name);
+
+The API fills the phy_bind structure with the dev_name (device name of the
+controller), index and phy_dev_name (device name of the PHY). This will
+be used when the controller requests this phy. This API should be used by
+platform specific initialization code (board file).
+
+In the case of dt boot, the binding information should be added in the dt
+data of the controller.


s/in the dt data of/in the device tree node of ?


+4. Getting a reference to the PHY
+
+Before the controller can make use of the PHY, it has to get a reference to
+it. This framework provides 6 APIs to get a reference to the PHY.
+
+struct phy

Re: [PATCH v4 1/6] drivers: phy: add generic PHY framework

2013-04-02 Thread Sylwester Nawrocki 

On 04/02/2013 12:38 AM, Stephen Warren wrote:

On 04/01/2013 04:27 PM, Sylwester Nawrocki wrote:

On 03/28/2013 06:43 AM, Kishon Vijay Abraham I wrote:

diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt



+example2:
+phys: phy {
+compatible = xxx;
+reg =...;
+.
+.
+phys =phys 1;
+.
+.
+};
+
+This node represents a controller that uses one of the PHYs which is defined
+previously. Note that the phy handle has an additional specifier 1 to
+differentiate between the three PHYs. For this case, the controller driver
+should use of_phy_get_with_args/devm_of_phy_get_with_args.


This means a PHY user needs to know indexes at the PHY driver ?


The client node's DT has to specify which of the provider's PHYs it
references, yes. Presumably the device driver for the client node knows
absolutely nothing about this.


Ah, right. The device driver for the client node not necessarily need to be
aware about this. I think I got misled by the 'index' argument of
of_phy_get_with_args() which the PHY consumer need to supply. However it is
only an index pointing to a PHY specifier in its 'phys' property, hence it
would be normally 0 if the client needs only one PHY. Hopefully I got it
right this time.


I have been thinking of using this for an IP which has 4 video PHYs: 2 MIPI
CSI-2 and 2 MIPI DSI. The IP has just 2 registers, each of which is shared
between one MIPI CSI-2 DPHY and one MIPI DSI DPHY. So I thought about creating
a single device node for this IP and using 4 indexes for the PHYs, e.g. 0...3.


That sounds right.


Then users of each PHY type would use only indexes 0..1 (to select their
corresponding port).


I don't follow that. If the provider exports PHYs 0..3, then the client
nodes would refer to PHYS 0..3 not 0..1.


Indeed, it seems I just wanted to preserve the CSI/DSI channel information
(0, 1), but that is not really needed anywhere.


However I fail to see how this could now be represented in the bindings.


Exactly like the example you gave below, I would expect.


I assume struct phy::type could be used to differentiate between CSI-2 and DSI.
And struct phy::port could be used to select specific CSI-2 or DSI channel
(0, 1). Ideally the phy users should not care about index of a PHY at the PHY
device tree node. E.g. there are 2 MIPI CSI-2 receivers and each has only
one PHY assigned to it. I'm just wondering how the binding should look like,
so a PHY could be associated with a receiver automatically by the phy-core,
e.g.


Details such as phy::type and phy::port sounds like internal driver
implementation details which would have no effect on the bindings.


Yes, I seem to have mixed the phy-core implementation and the bindings a 
bit.
My intention was to have the PHYs registered with phy::port that would 
reflect
data channel id, as specified in the SoC's datasheet. However I can't 
think of

any use of it at the moment, except for debugging purpose.


/* DPHY IP node */
video-phy {
   reg =0x1000 8;
};

/* MIPI DSI nodes */
dsi_0 {
  phys =video-phy 0;
};

dsi_1 {
  phys =video-phy 1;
};

/* MIPI CSI-2 nodes */
csi_0 {
  phys =video-phy 2;
};

csi_1 {
  phys =video-phy 3;
};


That looks correct to me.


I'm not sure if it is not an overkill to use this the PHY framework with
a device which has only 2 registers. Perhaps something less heavy could
be designed for it. However, if the PHY framework is commonly used there
should be no issue wrt enabling the whole big infrastructure for a simple
device like this.


I don't think the number of registers should really makes any
difference; the whole point of the PHY framework is to decouple to
providers and consumers, so doing anything custom for special cases
would completely destroy the ability of the PHY framework to do that.


Ok, that's a very good argument. Something I have not been focused on
that much, given the architecture of hardware I used to work with.

I'll git it a try and I'll see if any any further questions jump out.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/6] drivers: phy: add generic PHY framework

2013-04-01 Thread Sylwester Nawrocki 

On 03/28/2013 06:43 AM, Kishon Vijay Abraham I wrote:

diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt
b/Documentation/devicetree/bindings/phy/phy-bindings.txt
new file mode 100644
index 000..35696b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
@@ -0,0 +1,76 @@
+This document explains only the dt data binding. For general information about
+PHY subsystem refer Documentation/phy.txt
+
+PHY device node
+===
+
+Optional Properties:
+#phy-cells:Number of cells in a PHY specifier;  The meaning of all those
+   cells is defined by the binding for the phy node. However
+   in-order to return the correct PHY, the PHY susbsystem
+   requires the first cell always refers to the port.
+
+This property is optional because it is needed only for the case where a
+single IP implements multiple PHYs.
+
+For example:
+
+phys: phy {
+compatible = xxx;
+reg1 =...;
+reg2 =...;
+reg3 =...;
+.
+.
+#phy-cells =1;
+.
+.
+};
+
+That node describes an IP block that implements 3 different PHYs. In order to
+differentiate between these 3 PHYs, an additonal specifier should be given
+while trying to get a reference to it. (The PHY subsystem assumes the
+specifier is port id).
+
+PHY user node
+=
+
+Required Properties:
+phys : the phandle for the PHY device (used by the PHY subsystem)
+
+Optional properties:
+phy-names : the names of the PHY corresponding to the PHYs present in the
+   *phys* phandle
+
+example1:
+phys: phy {
+compatible = xxx;
+reg =...;
+.
+.
+phys =usb2_phy,usb3_phy;
+phy-names = usb2phy, usb3phy;
+.
+.
+};
+This node represents a controller that uses two PHYs one for usb2 and one for
+usb3. The controller driver can get the appropriate PHY either by using
+devm_of_phy_get/of_phy_get by passing the correct index or by using
+of_phy_get_byname/devm_of_phy_get_byname by passing the names given in
+*phy-names*.
+
+example2:
+phys: phy {
+compatible = xxx;
+reg =...;
+.
+.
+phys =phys 1;
+.
+.
+};
+
+This node represents a controller that uses one of the PHYs which is defined
+previously. Note that the phy handle has an additional specifier 1 to
+differentiate between the three PHYs. For this case, the controller driver
+should use of_phy_get_with_args/devm_of_phy_get_with_args.


This means a PHY user needs to know indexes at the PHY driver ?

I have been thinking of using this for an IP which has 4 video PHYs: 2 MIPI
CSI-2 and 2 MIPI DSI. The IP has just 2 registers, each of which is shared
between one MIPI CSI-2 DPHY and one MIPI DSI DPHY. So I thought about 
creating
a single device node for this IP and using 4 indexes for the PHYs, e.g. 
0...3.

Then users of each PHY type would use only indexes 0..1 (to select their
corresponding port).

However I fail to see how this could now be represented in the bindings.

I assume struct phy::type could be used to differentiate between CSI-2 
and DSI.

And struct phy::port could be used to select specific CSI-2 or DSI channel
(0, 1). Ideally the phy users should not care about index of a PHY at 
the PHY

device tree node. E.g. there are 2 MIPI CSI-2 receivers and each has only
one PHY assigned to it. I'm just wondering how the binding should look like,
so a PHY could be associated with a receiver automatically by the phy-core,
e.g.

/* DPHY IP node */
video-phy {
  reg = 0x1000 8;
};

/* MIPI DSI nodes */
dsi_0 {
 phys = video-phy 0;
};

dsi_1 {
 phys = video-phy 1;
};

/* MIPI CSI-2 nodes */
csi_0 {
 phys = video-phy 2;
};

csi_1 {
 phys = video-phy 3;
};

I'm not sure if it is not an overkill to use this the PHY framework with
a device which has only 2 registers. Perhaps something less heavy could
be designed for it. However, if the PHY framework is commonly used there
should be no issue wrt enabling the whole big infrastructure for a simple
device like this.


Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/6] drivers: phy: add generic PHY framework

2013-03-20 Thread Sylwester Nawrocki 

Hi Kishon,

On 03/20/2013 10:12 AM, Kishon Vijay Abraham I wrote:

The PHY framework provides a set of APIs for the PHY drivers to
create/destroy a PHY and APIs for the PHY users to obtain a reference to the
PHY with or without using phandle. To obtain a reference to the PHY without
using phandle, the platform specfic intialization code (say from board file)
should have already called phy_bind with the binding information. The binding
information consists of phy's device name, phy user device name and an index.
The index is used when the same phy user binds to mulitple phys.

PHY drivers should create the PHY by passing phy_descriptor that has
describes the PHY (label, type etc..) and ops like init, exit, suspend, resume,
poweron, shutdown.

The documentation for the generic PHY framework is added in
Documentation/phy.txt and the documentation for the sysfs entry is added
in Documentation/ABI/testing/sysfs-class-phy.

Signed-off-by: Kishon Vijay Abraham Ikis...@ti.com
---

[...]

+static inline struct phy *phy_get(struct device *dev, int index)
+{
+   return ERR_PTR(-EOPNOTSUPP);


Shouldn't these be -ENOSYS ? EOPNOTSUPP is defined by POSIX as
Operation not supported on socket. And EOPNOTSUPP appears to be
mostly used in the network code in the kernel.


+}
+
+static inline struct phy *devm_phy_get(struct device *dev, int index)
+{
+   return ERR_PTR(-EOPNOTSUPP);
+}
+
+static inline struct phy *of_phy_get(struct device *dev, int index)
+{
+   return ERR_PTR(-EOPNOTSUPP);
+}
+
+static inline struct phy *devm_of_phy_get(struct device *dev, int index)
+{
+   return ERR_PTR(-EOPNOTSUPP);
+}
+
+static inline struct phy *of_phy_get_byname(struct device *dev,
+   const char *string)
+{
+   return ERR_PTR(-EOPNOTSUPP);
+}
+
+static inline struct phy *devm_of_phy_get_byname(struct device *dev,
+   const char *string)
+{
+   return ERR_PTR(-EOPNOTSUPP);
+}


Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] usb: exynos: Fix compatible strings used for device

2013-01-23 Thread Sylwester Nawrocki 
Hi,

On 01/23/2013 01:20 PM, Vivek Gautam wrote:
 - { .compatible = samsung,exynos-dwc3 },
 + { .compatible = samsung,synopsis-dwc3 }

 You're both missing a point here. The synopsys IP driver is called
 dwc3.ko and that's compatible with synopsys,dwc3. Your glue layer driver
 (dwc3-exynos.ko) is compatible with your platform, so
 samsung,exynos-dwc3 sounds correct to me.

 Hmm...yeah, you're right and agreed.

 However, we need to use more clear name there like samsung,exynos-dwusb3
 in compatible, because you know there are lots of other IPs in Synopsis
 Design Ware brand. So we have to include usb in compatible for that.

 fair enough.

 
 Thanks for your suggestions. This definitely make things clear.
 I shall then keep samsung,exynos-dwusb3 as the compatible string or,
 should i be including '5250' string as well, something like
 samsung,exynos5250-dwusb3
 as pointed out by Grant earlier ?  :-O

IMHO this needs to be samsung,exynos5250-dwusb3, rather than 
samsung,exynos-dwusb3. :)

--

Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] usb: exynos: Fix compatible strings used for device

2013-01-22 Thread Sylwester Nawrocki 
On 01/22/2013 06:35 AM, Kukjin Kim wrote:
 - { .compatible = samsung,exynos-dwc3 },
 + { .compatible = samsung,synopsis-dwc3 },

 Or if any version or something, how about following?

 + { .compatible = samsung,dwc-v3 },

 Well, yes the newer SoCs with same IP using the chip name can cause some
 confusion, but won't it be fine that -
 Newer parts using the same core can claim compatibility by
 including the older string in the compatible list - as quoted by Grant
 Likely

 Or, can we try another option, using multiple compatible strings for
 SoC specific
 in of_match_table, so that we don't create any confusion by using same
 compatible for newer SoCs also. Like,

 - { .compatible = samsung,exynos-dwc3 },
 + { .compatible = samsung,exynos5250-dwc3 },
 + { .compatible =new SoC using same IP  },

 Yes, why not just use an SoC name where given IP first appeared ? I
 believe
 IP revision numbers are not always well documented. Also when an IP is
 instantiated multiple times in specific SoC, its revision number might not
 be sufficient to determine the system integration details for each
 instance.
 I think having version for some devices and SoC name for others just adds
 to the confusion. Thus using specific chip name in the compatible property
 seems more clear to me.

 Well, I don't think so. Let's see the DMAC PL330. Its compatible is
 arm,pl330 and arm,primecell not SoC/Chip name. I think DWC is a same
 case or at least similar.
 
 You know, the DWC is a IP from Synopsis and I _Believe_ it has a kind of
 version and it can be used for identify.

That's a good point, but isn't DesignWare just a name of a family of IP
cores from Synopsys [1] ? And what would DWC be supposed to signify ?
DesignWare Controller ? Wouldn't that be too generic ? Synopsys seems
to offer multiple different controllers and any of them could eventually
end up in a specific SoC [2].

Maybe the compatible property should be something like:

compatible = samsung,exynos5250-dwc-3, synopsys,dw-usb-3;
or
compatible = samsung,exynos5250-usb3, synopsys,dw-ss-usb3;

? Or anything more specific in the synopsys part to indicate which
exactly USB controller IP is used ?

[1] http://www.synopsys.com/IP/InterfaceIP/USB/Pages/default.aspx
[2] http://www.synopsys.com/IP/InterfaceIP/Pages/default.aspx

--

Regards,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: phy: samsung: Add support to set pmu isolation

2013-01-11 Thread Sylwester Nawrocki 
On 01/11/2013 09:08 AM, Vivek Gautam wrote:
 Adding support to parse device node data in order to get
 required properties to set pmu isolation for usb-phy.
 
 Signed-off-by: Vivek Gautam gautam.vi...@samsung.com

Thanks for addressing my all comments,

Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com

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

Re: [PATCH v5] usb: phy: samsung: Add support to set pmu isolation

2013-01-09 Thread Sylwester Nawrocki 

Hi,

On 12/28/2012 10:13 AM, Vivek Gautam wrote:

Adding support to parse device node data in order to get
required properties to set pmu isolation for usb-phy.

Signed-off-by: Vivek Gautamgautam.vi...@samsung.com

...

--- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
+++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
@@ -9,3 +9,38 @@ Required properties:
  - compatible : should be samsung,exynos4210-usbphy
  - reg : base physical address of the phy registers and length of memory mapped
  region.
+
+Optional properties:
+- #address-cells: should be '1' when usbphy node has a child node with 'reg'
+  property.
+- #size-cells: should be '1' when usbphy node has a child node with 'reg'
+   property.
+- ranges: allows valid translation between child's address space and parent's
+  address space.
+
+- The child node 'usbphy-sys' to the node 'usbphy' is for the system controller
+  interface for usb-phy. It should provide the following information required 
by
+  usb-phy controller to control phy.
+   - reg : base physical address of PHY control register in PMU which
+   enables/disables the phy controller.


On some SoCs USB_PHY_CONTROL registers don't belong to PMU. So maybe you 
should
drop references to PMU, or list all SoC entities where USB_PHY_CONTROL 
appears:

PMU, MISC REGISTER, etc.

I would just rephrase this to:

- reg : base physical address of PHY_CONTROL registers

phy controller might be confusing, since PHY CONTROLLER is an entity 
separate

from PHY 0 and PHY 1 ?


+   The size of this register is the total sum of size of all 
phy-control


And what about using PHY_CONTROL name as per the User Manuals ? That would
perhaps make it a bit easier to follow.


+   registers that the SoC has. For example, the size will be
+   '0x4' in case we have only one phy-control register (like in 
S3C64XX) or
+   '0x8' in case we have two phy-control registers (like in Exynos4210)
+   and so on.
+
+Example:
+ - Exynos4210
+
+ usbphy@125B {
+ #address-cells =1;
+ #size-cells =1;
+ compatible = samsung,exynos4210-usbphy;
+ reg =0x125B 0x100;
+ ranges;
+
+ usbphy-sys {
+ /* USB device and host PHY_CONTROL registers */
+ reg =0x10020704 0x8;
+ };
+ };

...

  /*
+ * struct samsung_usbphy_drvdata - driver data for various SoC variants
+ * @cpu_type: machine identifier
+ * @devphy_en_mask: device phy enable mask for PHY CONTROL register
+ * @pmureg_devphy_offset: offset to DEVICE PHY CONTROL register from
+ *  mapped address of system controller.
+ *
+ * Here we have a separate mask for device type phy.
+ * Having different masks for host and device type phy helps
+ * in setting independent masks in case of SoCs like S5PV210,
+ * in which PHY0 and PHY1 enable bits belong to same register
+ * placed at position 0 and 1 respectively.
+ * Although for newer SoCs like exynos these bits belong to
+ * different registers altogether placed at position 0.
+ */
+struct samsung_usbphy_drvdata {
+ int cpu_type;
+ int devphy_en_mask;
+ u32 pmureg_devphy_offset;


Perhaps just devphy_reg_offset would do ?


+};
+
+/*
   * struct samsung_usbphy - transceiver driver state
   * @phy: transceiver structure
   * @plat: platform data
   * @dev: The parent device supplied to the probe function
   * @clk: usb phy clock
   * @regs: usb phy register memory base


Is this more precisely:

  * @regs: usb phy controller registers memory base
?

+ * @pmureg: usb device phy-control pmu register memory base


Maybe something like this would be more clear:

@pmureg: USB device PHY_CONTROL registers memory region base

Note, that not on all SoCs USB_PHY_CONTROL registers belong to PMU.
Haven't you considered changing pmureg to ctrl_regs or something
else more generic ?


   * @ref_clk_freq: reference clock frequency selection
- * @cpu_type: machine identifier
+ * @drv_data: driver data available for different SoCs
   */
  struct samsung_usbphy {
  struct usb_phy phy;
@@ -81,12 +107,63 @@ struct samsung_usbphy {
  struct device *dev;
  struct clk *clk;
  void __iomem *regs;
+ void __iomem *pmureg;
  int ref_clk_freq;
- int cpu_type;
+ const struct samsung_usbphy_drvdata *drv_data;
  };

...

+/*
+ * Set isolation here for phy.
+ * SOCs control this by controlling corresponding PMU registers


Hmm, it's not always PMU registers. I would remove this sentence and
instead explain what's the meaning of 'on' argument, so it is clear
the PHY is deactivated when on != 0.


+ */
+static void samsung_usbphy_set_isolation(struct samsung_usbphy *sphy, int on)
+{
+ static DEFINE_SPINLOCK(lock);


You probably don't need a global spinlock. Couldn't the spinlock be added
as struct samsung_usbhy field instead ?


+ unsigned long flags;
+ void __iomem *reg;
+ u32 reg_val;
+ u32 en_mask;
+
+ if (!sphy-pmureg) {
+ dev_warn(sphy-dev, Can't set pmu isolation\n);
+ return;
+ }
+
+ reg = sphy-pmureg + sphy-drv_data-pmureg_devphy_offset;
+ en_mask =

Re: [PATCH v4] usb: phy: samsung: Add support to set pmu isolation

2012-12-26 Thread Sylwester Nawrocki 

On 12/26/2012 01:28 PM, Vivek Gautam wrote:

Adding support to parse device node data in order to get
required properties to set pmu isolation for usb-phy.

Signed-off-by: Vivek Gautamgautam.vi...@samsung.com
---
  .../devicetree/bindings/usb/samsung-usbphy.txt |   31 
  drivers/usb/phy/samsung-usbphy.c   |  145 +---
  2 files changed, 155 insertions(+), 21 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt 
b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
index 7b26e2d..6b873fd 100644
--- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
+++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
@@ -9,3 +9,34 @@ Required properties:
  - compatible : should be samsung,exynos4210-usbphy
  - reg : base physical address of the phy registers and length of memory mapped
region.
+
+Optional properties:
+- #address-cells: should be '1' when usbphy node has a child node with 'reg'
+ property.
+- #size-cells: should be '1' when usbphy node has a child node with 'reg'
+  property.
+
+- The child node 'usbphy-pmu' to the node usbphy should provide the following
+  information required by usb-phy controller to enable/disable the phy.
+   - reg : base physical address of PHY control register in PMU which
+   enables/disables the phy controller.
+   The size of this register is the total sum of size of all 
phy-control
+   registers that the SoC has. For example, the size will be
+   '0x4' in case we have only one phy-control register (like in 
S3C64XX) or
+   '0x8' in case we have two phy-control registers (like in Exynos4210)
+   and so on.
+
+Example:
+ - Exysno4210


s/Exysno/Exynos


+
+   usbphy@125B {
+   #address-cells =1;
+   #size-cells =1;
+   compatible = samsung,exynos4210-usbphy;
+   reg =0x125B 0x100;
+
+   usbphy-pmu {
+   /* USB device and host PHY_CONTROL registers */
+   reg =0x10020704 0x8;
+   };
+   };
diff --git a/drivers/usb/phy/samsung-usbphy.c b/drivers/usb/phy/samsung-usbphy.c
index 5c5e1bb5..b9f4f42 100644
--- a/drivers/usb/phy/samsung-usbphy.c
+++ b/drivers/usb/phy/samsung-usbphy.c
@@ -60,20 +60,42 @@
  #define MHZ (1000*1000)
  #endif

+#define S3C64XX_USBPHY_ENABLE  (0x1  16)
+#define EXYNOS_USBPHY_ENABLE   (0x1  0)
+
  enum samsung_cpu_type {
TYPE_S3C64XX,
TYPE_EXYNOS4210,
  };

  /*
+ * struct samsung_usbphy_drvdata - driver data for various SoC variants
+ * @cpu_type: machine identifier
+ * @devphy_en_mask: device phy enable mask for PHY CONTROL register
+ *
+ * Here we have a separate mask for device type phy.
+ * Having different masks for host and device type phy helps
+ * in setting independent masks in case of SoCs like S5PV210,
+ * in which PHY0 and PHY1 enable bits belong to same register
+ * placed at [0] and [1] respectively.


and are placed at positions 0 and 1 respectively ?


+ * Although for newer SoCs like exynos these bits belong to
+ * different registers altogether placed at [0].
+ */
+struct samsung_usbphy_drvdata {
+   int cpu_type;
+   int devphy_en_mask;
+};
+
+/*
   * struct samsung_usbphy - transceiver driver state
   * @phy: transceiver structure
   * @plat: platform data
   * @dev: The parent device supplied to the probe function
   * @clk: usb phy clock
   * @regs: usb phy register memory base
+ * @phyctrl_pmureg: usb device phy-control pmu register memory base


nit: Perhaps we could just call it pmureg' ?


   * @ref_clk_freq: reference clock frequency selection
- * @cpu_type: machine identifier
+ * @drv_data: driver data available for different cpu types


Actually it's for different SoCs, not CPUs, right ?


   */
  struct samsung_usbphy {
struct usb_phy  phy;
@@ -81,12 +103,67 @@ struct samsung_usbphy {
struct device   *dev;
struct clk  *clk;
void __iomem*regs;
+   void __iomem*phyctrl_pmureg;
int ref_clk_freq;
-   int cpu_type;
+   const struct samsung_usbphy_drvdata *drv_data;
  };

  #define phy_to_sphy(x)container_of((x), struct 
samsung_usbphy, phy)

+static int samsung_usbphy_parse_dt_param(struct samsung_usbphy *sphy)


nit: s/samsung_usbphy_parse_dt_param/samsung_usbphy_parse_dt ?


+{
+   struct device_node *usbphy_pmu;
+   u32 reg[2];
+   int ret;
+
+   if (!sphy-dev-of_node) {
+   dev_err(sphy-dev, Can't get usb-phy node\n);
+   return -ENODEV;


The function is called only when dev-of_node is not NULL, so this path
is never executed, AFAICS. I would just drop this redundant check.


+   }
+
+   usbphy_pmu = of_get_child_by_name(sphy-dev-of_node, usbphy-pmu);
+   if (!usbphy_pmu)
+

Re: [PATCH 0/2] usb: exynos: Fix compatible strings used for device

2012-12-26 Thread Sylwester Nawrocki 

On 12/24/2012 09:13 AM, Vivek Gautam wrote:

These two changes look good to me.  For both of them:

Reviewed-by: Doug Andersondiand...@chromium.org


Well, I have another idea. Yes, I know, specific chip name should be used.

But

you know the specific chip name in compatible can cause another confusion
on other SoC which has same IP. So I think, we need to consider to use
common name or any specific name not chip in compatible for IP/driver like
following?

- { .compatible = samsung,exynos-dwc3 },
+ { .compatible = samsung,synopsis-dwc3 },

Or if any version or something, how about following?

+ { .compatible = samsung,dwc-v3 },


Well, yes the newer SoCs with same IP using the chip name can cause some
confusion, but won't it be fine that -
Newer parts using the same core can claim compatibility by
including the older string in the compatible list - as quoted by Grant Likely

Or, can we try another option, using multiple compatible strings for
SoC specific
in of_match_table, so that we don't create any confusion by using same
compatible for newer SoCs also. Like,

- { .compatible = samsung,exynos-dwc3 },
+ { .compatible = samsung,exynos5250-dwc3 },
+ { .compatible =new SoC using same IP  },


Yes, why not just use an SoC name where given IP first appeared ? I believe
IP revision numbers are not always well documented. Also when an IP is
instantiated multiple times in specific SoC, its revision number might not
be sufficient to determine the system integration details for each instance.
I think having version for some devices and SoC name for others just adds
to the confusion. Thus using specific chip name in the compatible property
seems more clear to me.

--

Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: phy: samsung: Add support to set pmu isolation

2012-12-26 Thread Sylwester Nawrocki 

Hi,

On 12/26/2012 02:56 PM, Vivek Gautam wrote:

On Wed, Dec 26, 2012 at 5:58 PM, Vivek Gautamgautam.vi...@samsung.com  wrote:

Adding support to parse device node data in order to get
required properties to set pmu isolation for usb-phy.

Signed-off-by: Vivek Gautamgautam.vi...@samsung.com
---


Hope these changes align with what architectural changes you had suggested ?


It looks much better now, thanks! I had a few additional comments, please
see my other reply.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] usb: phy: samsung: Add support to set pmu isolation

2012-12-19 Thread Sylwester Nawrocki 

Hi,

On 12/19/2012 02:44 PM, Vivek Gautam wrote:

--- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
+++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
@@ -9,3 +9,15 @@ Required properties:
   - compatible : should be samsung,exynos4210-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.
+
+   Required properties:
+   - compatible : should be samsung,usbdev-phyctrl
for
+   DEVICE type phy.
+   - samsung,phyhandle-reg: base physical address of
+   PHY_CONTROL register in
PMU.
+- samsung,enable-mask : should be '1'



This should only be 1 for Exynos4210+ SoCs, right ?


Yes that's true Exynso4210+ SoCs have only single PHY for both host and device
which gets enabled by single bit.


S5PV210 uses bit 0 for OTG and bit1 for USB host, doesn't it ? And for
s3c64xx
it seems to be bit 16.


True, S5PV210 uses two bits separately for OTG and HOST, so in that
case we would
require to set both these bits. Thanks for pointing out !!

I couldn't see device tree support for S5PV210 and S3C64xx so thought
of going for
4210+ SoCs. Better we make this more generic so that once these SoCs
are moved to
device tree we don't face any issue. Right ??


Fair enough. Yes, let's not make any future addition of the device tree
support for the older Samsung platforms unnecessarily difficult. It doesn't
take much effort, and there is many drivers that are shared by multiple 
SoCs

already, starting from s3c64xx to exynos4 series.


How about deriving this information from 'compatible' property instead ?


It will definitely be good to use the compatible property to derive
such information,
Need to amend the current approach.


Maybe you could just encode the USB PMU registers (I assume those aren't
touched by anything but the usb drivers) in a regular 'reg' property in
an usbphy subnode. Then the driver could interpret it also with help
of 'compatible' property. And you could just use of_iomap(). E.g.

  usbphy@1213 {
 compatible = samsung,exynos5250-usbphy;
 reg =0x1213 0x100,0x1210 0x100;
 usbphy-pmu {
 /* USB device and host PHY_CONTROL registers */
 reg =0x10040704 8;
 };
  };



This approach seems nice.


Your samsung,usb-phyhandle approach seems over-engineered to me.
I might be missing something though.



The idea behind using phandles for usb-phy was to get the multiple
registers (2 in PMU
and 1 in SYSREG) and program them separately as required.


You could still specify multiple address, size pairs in 'reg' property
or perhaps use separate node for SYSREG. And if on some SoCs PHY_CONTROL
registers do not immediately follow each other in memory it might make
sense to define the bindings so that each register is specified separately,
e.g.

reg = 0x10040704 4, 0x10040708 4, 0x10050230 4;

However AFAICS single region for the PHY_CONTROL registers should be
sufficient for all existing SoCs.

--

Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] usb: phy: samsung: Add support to set pmu isolation

2012-12-18 Thread Sylwester Nawrocki 

Hi Vivek,

On 12/18/2012 02:56 PM, Vivek Gautam wrote:

Adding support to parse device node data in order to get
required properties to set pmu isolation for usb-phy.

Signed-off-by: Vivek Gautamgautam.vi...@samsung.com
---

Changes from v1:
  - Changed the name of property for phy handler from'samsung,usb-phyctrl'
to 'samsung,usb-phyhandle' to make it look more generic.
  - Similarly 'samsung,phyctrl-reg' is changed to 'samsung,phyhandle-reg'
  - Added a check for 'samsung,usb-phyhandle' before getting node from
phandle.
  - Putting the node using 'of_node_put()' which had been missed.
  - Adding necessary check for the pointer in 'samsung_usbphy_set_isolation()'
to avoid any NULL pointer dereferencing.
  - Unmapping the register ioremapped in 'samsung_usbphy_parse_dt_param()'.


  .../devicetree/bindings/usb/samsung-usbphy.txt |   12 +++
  drivers/usb/phy/samsung-usbphy.c   |   94 ++--
  2 files changed, 98 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt 
b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
index 7b26e2d..a7b28b2 100644
--- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
+++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
@@ -9,3 +9,15 @@ Required properties:
  - compatible : should be samsung,exynos4210-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.
+
+   Required properties:
+   - compatible : should be samsung,usbdev-phyctrl for
+   DEVICE type phy.
+   - samsung,phyhandle-reg: base physical address of
+   PHY_CONTROL register in PMU.
+- samsung,enable-mask : should be '1'


This should only be 1 for Exynos4210+ SoCs, right ?
S5PV210 uses bit 0 for OTG and bit1 for USB host, doesn't it ? And for 
s3c64xx

it seems to be bit 16.

How about deriving this information from 'compatible' property instead ?

Maybe you could just encode the USB PMU registers (I assume those aren't
touched by anything but the usb drivers) in a regular 'reg' property in
an usbphy subnode. Then the driver could interpret it also with help
of 'compatible' property. And you could just use of_iomap(). E.g.

 usbphy@1213 {
compatible = samsung,exynos5250-usbphy;
reg = 0x1213 0x100, 0x1210 0x100;
usbphy-pmu {
/* USB device and host PHY_CONTROL registers */
reg = 0x10040704 8;
};
 };

Your samsung,usb-phyhandle approach seems over-engineered to me.
I might be missing something though.


diff --git a/drivers/usb/phy/samsung-usbphy.c b/drivers/usb/phy/samsung-usbphy.c
index 5c5e1bb5..4ceabe3 100644
--- a/drivers/usb/phy/samsung-usbphy.c
+++ b/drivers/usb/phy/samsung-usbphy.c
@@ -72,6 +72,8 @@ enum samsung_cpu_type {
   * @dev: The parent device supplied to the probe function
   * @clk: usb phy clock
   * @regs: usb phy register memory base
+ * @devctrl_reg: usb device phy-control pmu register memory base


hum, this name is not really helpful in understanding what's going
on here.

Looking at arch/arm/mach-s5pv210/setup-usb-phy.c, there is only one
PHY_CONTROL (Power Management Unit) register for both OTG and USB host
PHYs. So are you really taking care of that case as well ?


+ * @en_mask: enable mask
   * @ref_clk_freq: reference clock frequency selection
   * @cpu_type: machine identifier
   */
@@ -81,12 +83,73 @@ struct samsung_usbphy {
struct device   *dev;
struct clk  *clk;
void __iomem*regs;
+   void __iomem*devctrl_reg;
+   u32 en_mask;
int ref_clk_freq;
int cpu_type;
  };

  #define phy_to_sphy(x)container_of((x), struct 
samsung_usbphy, phy)

+static int samsung_usbphy_parse_dt_param(struct samsung_usbphy *sphy)
+{
+   struct device_node *usb_phyctrl;
+   u32 reg;
+   int lenp;
+
+   if (!sphy-dev-of_node) {
+   sphy-devctrl_reg = NULL;
+   return -ENODEV;
+   }
+
+   if (of_get_property(sphy-dev-of_node, samsung,usb-phyhandle,lenp)) 
{
+   usb_phyctrl = of_parse_phandle(sphy-dev-of_node,
+   samsung,usb-phyhandle, 0);
+   if (!usb_phyctrl) {
+   dev_warn(sphy-dev, Can't get usb-phy handle\n);
+   sphy-devctrl_reg = NULL;
+   }
+
+   of_property_read_u32(usb_phyctrl, samsung,phyhandle-reg,reg);
+
+   sphy-devctrl_reg = ioremap(reg, SZ_4);


What happens if invalid address value is assigned to

Re: [PATCH 2/2] ARM: Exynos5250: Enabling USB 3.0 phy for samsung-usbphy driver

2012-12-18 Thread Sylwester Nawrocki 

On 12/18/2012 04:39 PM, Vivek Gautam wrote:

Adding base address information required for enabling
USB 3.0 DRD phy on exynos5250 SOC.

Signed-off-by: Vivek Gautamgautam.vi...@samsung.com
---
  arch/arm/boot/dts/exynos5250.dtsi |3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
b/arch/arm/boot/dts/exynos5250.dtsi
index bbdb2c2..07b7477 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -316,7 +316,8 @@

usbphy@1213 {
compatible = samsung,exynos5250-usbphy;
-   reg =0x1213 0x100;
+   reg =0x1213 0x100,
+   0x1210 0x100;


Doesn't this second memory region mean distinct PHY controller device ?
Why separate usbphy node can't/shouldn't be created for it ?


samsung,usb-phyhandle =phy_h0phy_h1phy_h2;
samsung,enable-mask =1;
};

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

Re: [PATCH 2/2] ARM: Exynos5250: Enabling USB 3.0 phy for samsung-usbphy driver

2012-11-07 Thread Sylwester Nawrocki 

Hi Vivek,

On 11/06/2012 04:36 PM, Vivek Gautam wrote:

Adding base address information and required platform data
support for enabling USB DRD phy on exynos5250 SOC.

Signed-off-by: Vivek Gautamgautam.vi...@samsung.com
---
  arch/arm/boot/dts/exynos5250.dtsi|3 ++-
  arch/arm/mach-exynos/include/mach/regs-pmu.h |4 
  arch/arm/mach-exynos/setup-usb-phy.c |9 +
  3 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
b/arch/arm/boot/dts/exynos5250.dtsi
index 82bf042..51693af 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -220,7 +220,8 @@

usbphy {
compatible = samsung,exynos5250-usbphy;
-   reg =0x1213 0x100;
+   reg =0x1213 0x100,
+   0x1210 0x100;
};

amba {
diff --git a/arch/arm/mach-exynos/include/mach/regs-pmu.h 
b/arch/arm/mach-exynos/include/mach/regs-pmu.h
index d4e392b..67132b4 100644
--- a/arch/arm/mach-exynos/include/mach/regs-pmu.h
+++ b/arch/arm/mach-exynos/include/mach/regs-pmu.h
@@ -39,6 +39,10 @@
  #define S5P_HDMI_PHY_CONTROL  S5P_PMUREG(0x0700)
  #define S5P_HDMI_PHY_ENABLE   (1  0)

+/* only for EXYNOS5250*/
+#define S5P_USBDRD_PHY_CONTROL S5P_PMUREG(0x0704)
+#define S5P_USBDRD_PHY_ENABLE  (1  0)


Hmm, couldn't it be added to your usbphy node above and then this register
left for the usb phy driver to do ioremap and control it directly ? Rather
than relying on the platform data callback ? I hoped this static mapping
can be dropped once there is a proper usb phy driver in place. AFAIU
arch/arm/mach-exynos/setup-usb-phy.c is supposed to be a non-dt only thing.


+
  #define S5P_DAC_PHY_CONTROL   S5P_PMUREG(0x070C)
  #define S5P_DAC_PHY_ENABLE(1  0)

diff --git a/arch/arm/mach-exynos/setup-usb-phy.c 
b/arch/arm/mach-exynos/setup-usb-phy.c
index 6c768e0..5e46fdd 100644
--- a/arch/arm/mach-exynos/setup-usb-phy.c
+++ b/arch/arm/mach-exynos/setup-usb-phy.c
@@ -238,6 +238,15 @@ void s5p_usb_phy_pmu_isolation(int on, int type)
writel(readl(S5P_USBHOST_PHY_CONTROL)
| S5P_USBHOST_PHY_ENABLE,
S5P_USBHOST_PHY_CONTROL);
+   } else if (type == USB_PHY_TYPE_DRD) {
+   if (on)
+   writel(readl(S5P_USBDRD_PHY_CONTROL)
+ ~S5P_USBDRD_PHY_ENABLE,
+   S5P_USBDRD_PHY_CONTROL);


This is horrible coding style IMHO BTW. Why not just do

u32 reg = readl(S5P_USBDRD_PHY_CONTROL);
if (on)
reg = ~S5P_USBDRD_PHY_ENABLE;
else
reg |= ~S5P_USBDRD_PHY_ENABLE;
writel(reg, S5P_USBDRD_PHY_CONTROL);

Or to create some read/modify/write helper ? Anyway, I suppose this whole
setup-usb-phy.c file is going to be removed, once exynos is completely dt
only.


+   else
+   writel(readl(S5P_USBDRD_PHY_CONTROL)
+   | S5P_USBDRD_PHY_ENABLE,
+   S5P_USBDRD_PHY_CONTROL);
} else {
if (on)
writel(readl(S5P_USBDEVICE_PHY_CONTROL)


--

Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] USB: PHY: Add support for USB 3.0 phy for exynos5250

2012-11-07 Thread Sylwester Nawrocki 

On 11/07/2012 02:35 PM, Vivek Gautam wrote:

@@ -180,10 +273,12 @@ enum samsung_cpu_type {
   /*
* struct samsung_usbphy - transceiver driver state
* @phy: transceiver structure
+ * @phy3: transceiver structure for USB 3.0
* @plat: platform data
* @dev: The parent device supplied to the probe function
* @clk: usb phy clock
* @regs: usb phy register memory base
+ * @regs_phy3: usb 3.0 phy register memory base
* @ref_clk_freq: reference clock frequency selection
* @cpu_type: machine identifier
* @phy_type: It keeps track of the PHY type.
@@ -191,10 +286,12 @@ enum samsung_cpu_type {
*/
   struct samsung_usbphy {
 struct usb_phy  phy;
+   struct usb_phy  phy3;
 struct samsung_usbphy_data *plat;
 struct device   *dev;
 struct clk  *clk;
 void __iomem*regs;
+   void __iomem*regs_phy3;



Wouldn't it be better to create a new data structure for USB 3.0
and embedding it here, rather than adding multiple fields with 3
suffix ? E.g.

 struct {
 void __iomem*phy_regs;
 struct usb_phy  phy;
 } usb3;
?


Yes, thanks for suggesting. This way things will look clearer.
Will update this.


And why do you need to duplicate those fields in first place ?
I guess phy and phy3 are dependant and you can't register 2 PHYs
separately ?


Controllers like DWC3 needs two different PHYs. One of USB2 type and
one of USB3 type. So we needed to register two separate PHYs.


OK, I was just wondering if there is any dependency between those two phys
so you need to aggregate them in one struct samsung_usbphy, rather than
creating two separate struct samsung_usbphy objects for them.


+/*
+ * The function passed to the usb driver for phy initialization
+ */
   static int samsung_usbphy_init(struct usb_phy *phy)
   {
 struct samsung_usbphy *sphy;
@@ -570,6 +872,8 @@ static int __devinit samsung_usbphy_probe(struct 
platform_device *pdev)
 struct device *dev =pdev-dev;

 struct resource *phy_mem;
 void __iomem*phy_base;
+   struct resource *phy3_mem;
+   void __iomem*phy3_base = NULL;
 struct clk *clk;
 int ret = 0;

@@ -618,7 +922,38 @@ static int __devinit samsung_usbphy_probe(struct 
platform_device *pdev)

 sphy-clk = clk;

+   if (sphy-cpu_type == TYPE_EXYNOS5250) {
+   phy3_mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+   if (!phy3_mem) {
+   dev_err(dev, %s: missing mem resource\n, __func__);
+   return -ENODEV;
+   }
+
+   phy3_base = devm_request_and_ioremap(dev, phy3_mem);
+   if (!phy3_base) {
+   dev_err(dev, %s: register mapping failed\n, __func__);
+   return -ENXIO;
+   }
+   }
+
+   sphy-regs_phy3 = phy3_base;
+   sphy-phy3.dev  = sphy-dev;
+   sphy-phy3.label= samsung-usbphy3;
+   sphy-phy3.init = samsung_usbphy3_init;
+   sphy-phy3.shutdown = samsung_usbphy3_shutdown;
+
 ret = usb_add_phy(sphy-phy, USB_PHY_TYPE_USB2);
+   if (ret)
+   return ret;
+
+   if (sphy-cpu_type != TYPE_EXYNOS5250) {
+   dev_warn(dev, Not a valid cpu_type for USB 3.0\n);
+   } else {
+   ret = usb_add_phy(sphy-phy3, USB_PHY_TYPE_USB3);
+   if (ret)
+   return ret;



2 redundant lines here.


Will two returns under if return not error codes ? The last return
actually returns success.
If still it needs modification, will do that.


It's up to you how you structure it. The last return returns whatever
value ret has. I can't see what is an advantage of doing something like:

if (ret)
return ret;

return ret;
--

Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] USB: PHY: Add support for USB 3.0 phy for exynos5250

2012-11-06 Thread Sylwester Nawrocki 

Hi,

I have a few comments. Please see below...

On 11/06/2012 04:36 PM, Vivek Gautam wrote:

Adding support for USB3.0 phy for dwc3 controller on
exynso5250 SOC.


exynso - exynos



Signed-off-by: Vivek Gautamgautam.vi...@samsung.com
---
  drivers/usb/phy/samsung-usbphy.c|  337 +++
  include/linux/usb/samsung_usb_phy.h |1 +
  2 files changed, 338 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/phy/samsung-usbphy.c b/drivers/usb/phy/samsung-usbphy.c
index 3b4863d..e3b5fb1 100644
--- a/drivers/usb/phy/samsung-usbphy.c
+++ b/drivers/usb/phy/samsung-usbphy.c
@@ -167,6 +167,99 @@

  #define EXYNOS5_PHY_OTG_TUNE  (0x40)

+/* USB 3.0: DRD */
+#define EXYNOS5_DRD_LINKSYSTEM (0x04)
+
+#define LINKSYSTEM_FLADJ_MASK  (0x3f  1)
+#define LINKSYSTEM_FLADJ(_x)   ((_x)  1)
+#define LINKSYSTEM_XHCI_VERSION_CONTROL(1  27)
+
+#define EXYNOS5_DRD_PHYUTMI(0x08)
+
+#define PHYUTMI_OTGDISABLE (1  6)
+#define PHYUTMI_FORCESUSPEND   (1  1)
+#define PHYUTMI_FORCESLEEP (1  0)
+
+#define EXYNOS5_DRD_PHYPIPE(0x0C)


Would be nice to put all hex numbers in lower case.


+
+#define EXYNOS5_DRD_PHYCLKRST  (0x10)
+
+#define PHYCLKRST_SSC_REFCLKSEL_MASK   (0xff  23)
+#define PHYCLKRST_SSC_REFCLKSEL(_x)((_x)  23)
+
+#define PHYCLKRST_SSC_RANGE_MASK   (0x03  21)
+#define PHYCLKRST_SSC_RANGE(_x)((_x)  21)
+
+#define PHYCLKRST_SSC_EN   (1  20)
+#define PHYCLKRST_REF_SSP_EN   (1  19)
+#define PHYCLKRST_REF_CLKDIV2  (1  18)
+
+#define PHYCLKRST_MPLL_MULTIPLIER_MASK (0x7f  11)
+#define PHYCLKRST_MPLL_MULTIPLIER(_x)  ((_x)  11)


Is this macro-definition going to be used anywhere else except the
5 definitions below ? Is this really helpful ? In anything else than
forcing you to use questionable line breaking below ?


+#define PHYCLKRST_MPLL_MULTIPLIER_100MHZ_REF   \


How about simply defining it as

#define PHYCLKRST_MPLL_MULTIPLIER_100MHZ_REF(0x19  11)

?

+   PHYCLKRST_MPLL_MULTIPLIER(0x19)
+#define PHYCLKRST_MPLL_MULTIPLIER_50M_REF  \
+   PHYCLKRST_MPLL_MULTIPLIER(0x02)
+#define PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF\
+   PHYCLKRST_MPLL_MULTIPLIER(0x68)
+#define PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF\
+   PHYCLKRST_MPLL_MULTIPLIER(0x7d)
+#define PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF \
+   PHYCLKRST_MPLL_MULTIPLIER(0x02)
+
+#define PHYCLKRST_FSEL_MASK(0x3f  5)
+#define PHYCLKRST_FSEL(_x) ((_x)  5)


Ditto.


+#define PHYCLKRST_FSEL_PAD_100MHZ  \
+   PHYCLKRST_FSEL(0x27)
+#define PHYCLKRST_FSEL_PAD_24MHZ   \
+   PHYCLKRST_FSEL(0x2a)
+#define PHYCLKRST_FSEL_PAD_20MHZ   \
+   PHYCLKRST_FSEL(0x31)
+#define PHYCLKRST_FSEL_PAD_19_2MHZ \
+   PHYCLKRST_FSEL(0x38)
+
+#define PHYCLKRST_RETENABLEN   (1  4)
+
+#define PHYCLKRST_REFCLKSEL_MASK   (0x03  2)
+#define PHYCLKRST_REFCLKSEL(_x)((_x)  2)


Ditto.


+#define PHYCLKRST_REFCLKSEL_PAD_REFCLK \
+   PHYCLKRST_REFCLKSEL(2)
+#define PHYCLKRST_REFCLKSEL_EXT_REFCLK \
+   PHYCLKRST_REFCLKSEL(3)
+
+#define PHYCLKRST_PORTRESET(1  1)
+#define PHYCLKRST_COMMONONN(1  0)
+
+#define EXYNOS5_DRD_PHYREG0(0x14)
+#define EXYNOS5_DRD_PHYREG1(0x18)
+
+#define EXYNOS5_DRD_PHYPARAM0  (0x1C)
+
+#define PHYPARAM0_REF_USE_PAD  (0x1  31)
+#define PHYPARAM0_REF_LOSLEVEL_MASK(0x1f  26)
+#define PHYPARAM0_REF_LOSLEVEL (0x9  26)
+
+#define EXYNOS5_DRD_PHYPARAM1  (0x20)
+
+#define PHYPARAM1_PCS_TXDEEMPH_MASK(0x1f  0)
+#define PHYPARAM1_PCS_TXDEEMPH (0x1C)
+
+#define EXYNOS5_DRD_PHYTERM(0x24)
+
+#define EXYNOS5_DRD_PHYTEST(0x28)
+
+#define PHYTEST_POWERDOWN_SSP  (1  3)
+#define PHYTEST_POWERDOWN_HSP  (1  2)
+
+#define EXYNOS5_DRD_PHYADP (0x2C)
+
+#define EXYNOS5_DRD_PHYBATCHG  (0x30)
+
+#define PHYBATCHG_UTMI_CLKSEL  (0x1  2)
+
+#define EXYNOS5_DRD_PHYRESUME  (0x34)
+#define EXYNOS5_DRD_LINKPORT   (0x44)
+
  #ifndef MHZ
  #define MHZ (1000*1000)
  #endif
@@ -180,10 +273,12 @@ enum samsung_cpu_type {
  /*
   * struct samsung_usbphy - transceiver driver state
   * @phy: transceiver structure
+ * @phy3: transceiver structure for USB 3.0
   * @plat: platform data
   * @dev: The parent device supplied to the probe function
   * @clk: usb phy