Re: [PATCH v2 1/5] phy: Add new Exynos USB PHY driver
Hi, On Monday 28 October 2013 07:22 PM, Kamil Debski wrote: Hi Kishon, Thank you for your review! I will answer your comments below. From: Kishon Vijay Abraham I [mailto:kis...@ti.com] Sent: Friday, October 25, 2013 5:40 PM Hi, On Friday 25 October 2013 07:45 PM, Kamil Debski wrote: Add a new driver for the Exynos USB PHY. The new driver uses the generic PHY framework. The driver includes support for the Exynos 4x10 and 4x12 SoC families. Signed-off-by: Kamil Debski k.deb...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- .../devicetree/bindings/phy/samsung-usbphy.txt | 51 +++ drivers/phy/Kconfig| 21 ++ drivers/phy/Makefile |3 + drivers/phy/phy-exynos-usb.c | 245 ++ drivers/phy/phy-exynos-usb.h | 94 ++ drivers/phy/phy-exynos4210-usb.c | 295 + drivers/phy/phy-exynos4212-usb.c | 343 7 files changed, 1052 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt create mode 100644 drivers/phy/phy-exynos-usb.c create mode 100644 drivers/phy/phy-exynos-usb.h create mode 100644 drivers/phy/phy-exynos4210-usb.c create mode 100644 drivers/phy/phy-exynos4212-usb.c diff --git a/Documentation/devicetree/bindings/phy/samsung-usbphy.txt b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt new file mode 100644 index 000..f112b37 --- /dev/null +++ b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt @@ -0,0 +1,51 @@ +Samsung S5P/EXYNOS SoC series USB PHY +- + +Required properties: +- compatible : should be one of the listed compatibles: + - samsung,exynos4210-usbphy + - samsung,exynos4212-usbphy +- reg : a list of registers used by phy driver + - first and obligatory is the location of phy modules registers + - second and also required is the location of isolation registers + (isolation registers control the physical connection between the in + SoC modules and outside of the SoC, this also can be called enable + control in the documentation of the SoC) + - third is the location of the mode switch register, this only applies + to SoCs that have such a feature; mode switching enables to have + both host and device used the same SoC pins and is commonly used + when OTG is supported +- #phy-cells : from the generic phy bindings, must be 1; + +The second cell in the PHY specifier identifies the PHY its meaning +is SoC dependent. For the currently supported SoCs (Exynos 4210 and +Exynos 4212) it is as follows: + 0 - USB device, + 1 - USB host, + 2 - HSIC0, + 3 - HSIC1, HSIC is supposedly to be transceiver less no? You have to program something in the digital side? You have a single IP that have all these functionalities? There is a single USB PHY controller for all the above functionalities (i.e. host, device, hsic 0 and 1). Ok. + +Example: + +For Exynos 4412 (compatible with Exynos 4212): + +exynos_usbphy: exynos-usbphy@125B { + compatible = samsung,exynos4212-usbphy; + reg = 0x125B 0x100 0x10020704 0x0c 0x1001021c 0x4; + ranges; + #address-cells = 1; + #size-cells = 1; The above 3 properties aren't documented? Are they needed here? My bad. I was working on two branches and corrected it in only one of them. + clocks = clock 305, clock 2, clock 2, clock 2, + clock 2; + clock-names = phy, device, host, hsic0, hsic1; + status = okay; + #phy-cells = 1; +}; + +Then the PHY can be used in other nodes such as: + +ehci@1258 { + status = okay; + phys = exynos_usbphy 2; + phy-names = hsic0; +}; diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index 349bef2..2f7ac0a 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -15,4 +15,25 @@ config GENERIC_PHY phy users can obtain reference to the PHY. All the users of this framework should select this config. +config PHY_EXYNOS_USB + tristate Samsung USB PHY driver (using the Generic PHY Framework) Mentioning *Generic PHY Framework* is not necessary. *select GENERIC_PHY* here This was added to distinguish this driver from the ols USB PHY driver. I agree that in the final version it should be removed. I understand that the correct way to do this is by removing the old driver when the new gets merged. Yes? right. + help + Enable this to support Samsung USB phy helper driver for Samsung SoCs. + This driver provides common interface to interact, for Samsung + USB 2.0 PHY driver. If it's going to be used only for usb2, name it PHY_EXYNOS_USB2. I agree. + +config PHY_EXYNOS4210_USB + bool Support for Exynos 4210 + depends on
RE: [PATCH v2 1/5] phy: Add new Exynos USB PHY driver
Hi Kumar Gala, From: Kumar Gala [mailto:ga...@codeaurora.org] Sent: Friday, October 25, 2013 11:36 PM On Oct 25, 2013, at 9:15 AM, Kamil Debski wrote: Add a new driver for the Exynos USB PHY. The new driver uses the generic PHY framework. The driver includes support for the Exynos 4x10 and 4x12 SoC families. Signed-off-by: Kamil Debski k.deb...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- .../devicetree/bindings/phy/samsung-usbphy.txt | 51 +++ drivers/phy/Kconfig| 21 ++ drivers/phy/Makefile |3 + drivers/phy/phy-exynos-usb.c | 245 ++ drivers/phy/phy-exynos-usb.h | 94 ++ drivers/phy/phy-exynos4210-usb.c | 295 + drivers/phy/phy-exynos4212-usb.c | 343 7 files changed, 1052 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt create mode 100644 drivers/phy/phy-exynos-usb.c create mode 100644 drivers/phy/phy-exynos-usb.h create mode 100644 drivers/phy/phy-exynos4210-usb.c create mode 100644 drivers/phy/phy-exynos4212-usb.c diff --git a/Documentation/devicetree/bindings/phy/samsung-usbphy.txt b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt new file mode 100644 index 000..f112b37 --- /dev/null +++ b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt @@ -0,0 +1,51 @@ +Samsung S5P/EXYNOS SoC series USB PHY +- + +Required properties: +- compatible : should be one of the listed compatibles: + - samsung,exynos4210-usbphy + - samsung,exynos4212-usbphy +- reg : a list of registers used by phy driver + - first and obligatory is the location of phy modules registers + - second and also required is the location of isolation registers + (isolation registers control the physical connection between the in + SoC modules and outside of the SoC, this also can be called enable + control in the documentation of the SoC) + - third is the location of the mode switch register, this only applies + to SoCs that have such a feature; mode switching enables to have + both host and device used the same SoC pins and is commonly used + when OTG is supported +- #phy-cells : from the generic phy bindings, must be 1; Please add if clock clock-names are required properties. Ok, thanks for pointing this out. + +The second cell in the PHY specifier identifies the PHY its meaning +is SoC dependent. For the currently supported SoCs (Exynos 4210 and +Exynos 4212) it is as follows: Can we say instead of 'its meaning is SoC dependent...' something like 'its meaning is compatible dependent? Ok, this sounds better in deed. + 0 - USB device, + 1 - USB host, + 2 - HSIC0, + 3 - HSIC1, + +Example: + +For Exynos 4412 (compatible with Exynos 4212): + +exynos_usbphy: exynos-usbphy@125B { + compatible = samsung,exynos4212-usbphy; + reg = 0x125B 0x100 0x10020704 0x0c 0x1001021c 0x4; + ranges; + #address-cells = 1; + #size-cells = 1; Why do you have ranges, and #address-cells #size-cells here? As, I mentioned in my reply to Kishon. I worked on two branches and I forgot to remove this in the one used to generate patches. + clocks = clock 305, clock 2, clock 2, clock 2, + clock 2; + clock-names = phy, device, host, hsic0, hsic1; + status = okay; + #phy-cells = 1; +}; + +Then the PHY can be used in other nodes such as: + +ehci@1258 { + status = okay; + phys = exynos_usbphy 2; + phy-names = hsic0; +}; Best wishes, -- Kamil Debski Samsung RD Institute Poland -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc 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 1/5] phy: Add new Exynos USB PHY driver
Hi Kishon, Thank you for your review! I will answer your comments below. From: Kishon Vijay Abraham I [mailto:kis...@ti.com] Sent: Friday, October 25, 2013 5:40 PM Hi, On Friday 25 October 2013 07:45 PM, Kamil Debski wrote: Add a new driver for the Exynos USB PHY. The new driver uses the generic PHY framework. The driver includes support for the Exynos 4x10 and 4x12 SoC families. Signed-off-by: Kamil Debski k.deb...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- .../devicetree/bindings/phy/samsung-usbphy.txt | 51 +++ drivers/phy/Kconfig| 21 ++ drivers/phy/Makefile |3 + drivers/phy/phy-exynos-usb.c | 245 ++ drivers/phy/phy-exynos-usb.h | 94 ++ drivers/phy/phy-exynos4210-usb.c | 295 + drivers/phy/phy-exynos4212-usb.c | 343 7 files changed, 1052 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt create mode 100644 drivers/phy/phy-exynos-usb.c create mode 100644 drivers/phy/phy-exynos-usb.h create mode 100644 drivers/phy/phy-exynos4210-usb.c create mode 100644 drivers/phy/phy-exynos4212-usb.c diff --git a/Documentation/devicetree/bindings/phy/samsung-usbphy.txt b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt new file mode 100644 index 000..f112b37 --- /dev/null +++ b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt @@ -0,0 +1,51 @@ +Samsung S5P/EXYNOS SoC series USB PHY +- + +Required properties: +- compatible : should be one of the listed compatibles: + - samsung,exynos4210-usbphy + - samsung,exynos4212-usbphy +- reg : a list of registers used by phy driver + - first and obligatory is the location of phy modules registers + - second and also required is the location of isolation registers + (isolation registers control the physical connection between the in + SoC modules and outside of the SoC, this also can be called enable + control in the documentation of the SoC) + - third is the location of the mode switch register, this only applies + to SoCs that have such a feature; mode switching enables to have + both host and device used the same SoC pins and is commonly used + when OTG is supported +- #phy-cells : from the generic phy bindings, must be 1; + +The second cell in the PHY specifier identifies the PHY its meaning +is SoC dependent. For the currently supported SoCs (Exynos 4210 and +Exynos 4212) it is as follows: + 0 - USB device, + 1 - USB host, + 2 - HSIC0, + 3 - HSIC1, HSIC is supposedly to be transceiver less no? You have to program something in the digital side? You have a single IP that have all these functionalities? There is a single USB PHY controller for all the above functionalities (i.e. host, device, hsic 0 and 1). + +Example: + +For Exynos 4412 (compatible with Exynos 4212): + +exynos_usbphy: exynos-usbphy@125B { + compatible = samsung,exynos4212-usbphy; + reg = 0x125B 0x100 0x10020704 0x0c 0x1001021c 0x4; + ranges; + #address-cells = 1; + #size-cells = 1; The above 3 properties aren't documented? Are they needed here? My bad. I was working on two branches and corrected it in only one of them. + clocks = clock 305, clock 2, clock 2, clock 2, + clock 2; + clock-names = phy, device, host, hsic0, hsic1; + status = okay; + #phy-cells = 1; +}; + +Then the PHY can be used in other nodes such as: + +ehci@1258 { + status = okay; + phys = exynos_usbphy 2; + phy-names = hsic0; +}; diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index 349bef2..2f7ac0a 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -15,4 +15,25 @@ config GENERIC_PHY phy users can obtain reference to the PHY. All the users of this framework should select this config. +config PHY_EXYNOS_USB + tristate Samsung USB PHY driver (using the Generic PHY Framework) Mentioning *Generic PHY Framework* is not necessary. *select GENERIC_PHY* here This was added to distinguish this driver from the ols USB PHY driver. I agree that in the final version it should be removed. I understand that the correct way to do this is by removing the old driver when the new gets merged. Yes? + help + Enable this to support Samsung USB phy helper driver for Samsung SoCs. + This driver provides common interface to interact, for Samsung + USB 2.0 PHY driver. If it's going to be used only for usb2, name it PHY_EXYNOS_USB2. I agree. + +config PHY_EXYNOS4210_USB + bool Support for Exynos 4210 + depends on
Re: [PATCH v2 1/5] phy: Add new Exynos USB PHY driver
Hi Kamil, On Monday 28 of October 2013 14:52:19 Kamil Debski wrote: Hi Kishon, Thank you for your review! I will answer your comments below. [snip] + + switch (drv-cfg-cpu) { + case TYPE_EXYNOS4210: + case TYPE_EXYNOS4212: Lets not add such cpu checks inside driver. Some SoC have a special register, which switches the OTG lines between device and host modes. I understand that it might not be the prettiest code. I see this as a good compromise between having a single huge driver for all Exynos SoCs and having a multiple drivers for each SoC version. PHY IPs in these chips very are similar but have to be handled differently. Any other ideas to solve this issue? Maybe adding a flag in drv-cfg called, for example, .has_mode_switch could solve this problem without having to check the SoC type explicitly? [snip] +#ifdef CONFIG_PHY_EXYNOS4210_USB Do we really need this? No we don't. The driver can always support all Exynos SoC versions. These config options were added for flexibility. +extern const struct uphy_config exynos4210_uphy_config; #endif + +#ifdef CONFIG_PHY_EXYNOS4212_USB Same here. +extern const struct uphy_config exynos4212_uphy_config; #endif + +static const struct of_device_id exynos_uphy_of_match[] = { #ifdef +CONFIG_PHY_EXYNOS4210_USB #if not needed here. If the #ifdef CONFIG_PHY_EXYNOS4210_USB is removed then yes. Otherwise it is necessary - exynos4210_uphy_config may be undefined. I believe this and other ifdefs below are needed, otherwise, with support for one of the SoCs disabled, the driver could still bind to its compatible value. Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc 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 1/5] phy: Add new Exynos USB PHY driver
Hi, On Friday 25 October 2013 07:45 PM, Kamil Debski wrote: Add a new driver for the Exynos USB PHY. The new driver uses the generic PHY framework. The driver includes support for the Exynos 4x10 and 4x12 SoC families. Signed-off-by: Kamil Debski k.deb...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- .../devicetree/bindings/phy/samsung-usbphy.txt | 51 +++ drivers/phy/Kconfig| 21 ++ drivers/phy/Makefile |3 + drivers/phy/phy-exynos-usb.c | 245 ++ drivers/phy/phy-exynos-usb.h | 94 ++ drivers/phy/phy-exynos4210-usb.c | 295 + drivers/phy/phy-exynos4212-usb.c | 343 7 files changed, 1052 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt create mode 100644 drivers/phy/phy-exynos-usb.c create mode 100644 drivers/phy/phy-exynos-usb.h create mode 100644 drivers/phy/phy-exynos4210-usb.c create mode 100644 drivers/phy/phy-exynos4212-usb.c diff --git a/Documentation/devicetree/bindings/phy/samsung-usbphy.txt b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt new file mode 100644 index 000..f112b37 --- /dev/null +++ b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt @@ -0,0 +1,51 @@ +Samsung S5P/EXYNOS SoC series USB PHY +- + +Required properties: +- compatible : should be one of the listed compatibles: + - samsung,exynos4210-usbphy + - samsung,exynos4212-usbphy +- reg : a list of registers used by phy driver + - first and obligatory is the location of phy modules registers + - second and also required is the location of isolation registers + (isolation registers control the physical connection between the in + SoC modules and outside of the SoC, this also can be called enable + control in the documentation of the SoC) + - third is the location of the mode switch register, this only applies + to SoCs that have such a feature; mode switching enables to have + both host and device used the same SoC pins and is commonly used + when OTG is supported +- #phy-cells : from the generic phy bindings, must be 1; + +The second cell in the PHY specifier identifies the PHY its meaning is SoC +dependent. For the currently supported SoCs (Exynos 4210 and Exynos 4212) it +is as follows: + 0 - USB device, + 1 - USB host, + 2 - HSIC0, + 3 - HSIC1, HSIC is supposedly to be transceiver less no? You have to program something in the digital side? You have a single IP that have all these functionalities? + +Example: + +For Exynos 4412 (compatible with Exynos 4212): + +exynos_usbphy: exynos-usbphy@125B { + compatible = samsung,exynos4212-usbphy; + reg = 0x125B 0x100 0x10020704 0x0c 0x1001021c 0x4; + ranges; + #address-cells = 1; + #size-cells = 1; The above 3 properties aren't documented? Are they needed here? + clocks = clock 305, clock 2, clock 2, clock 2, + clock 2; + clock-names = phy, device, host, hsic0, hsic1; + status = okay; + #phy-cells = 1; +}; + +Then the PHY can be used in other nodes such as: + +ehci@1258 { + status = okay; + phys = exynos_usbphy 2; + phy-names = hsic0; +}; diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index 349bef2..2f7ac0a 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -15,4 +15,25 @@ config GENERIC_PHY phy users can obtain reference to the PHY. All the users of this framework should select this config. +config PHY_EXYNOS_USB + tristate Samsung USB PHY driver (using the Generic PHY Framework) Mentioning *Generic PHY Framework* is not necessary. *select GENERIC_PHY* here + help + Enable this to support Samsung USB phy helper driver for Samsung SoCs. + This driver provides common interface to interact, for Samsung + USB 2.0 PHY driver. If it's going to be used only for usb2, name it PHY_EXYNOS_USB2. + +config PHY_EXYNOS4210_USB + bool Support for Exynos 4210 + depends on PHY_EXYNOS_USB + depends on CPU_EXYNOS4210 + help + Enable USB PHY support for Exynos 4210 + +config PHY_EXYNOS4212_USB + bool Support for Exynos 4212 + depends on PHY_EXYNOS_USB + depends on (SOC_EXYNOS4212 || SOC_EXYNOS4412) + help + Enable USB PHY support for Exynos 4212 How difference is USB PHY in Exynos 4212 from Exynos 4210? If there isn't much, I would suggest to use a single driver. + endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index 9e9560f..ca3dc82 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -3,3 +3,6 @@ # obj-$(CONFIG_GENERIC_PHY)
Re: [PATCH v2 1/5] phy: Add new Exynos USB PHY driver
On Oct 25, 2013, at 9:15 AM, Kamil Debski wrote: Add a new driver for the Exynos USB PHY. The new driver uses the generic PHY framework. The driver includes support for the Exynos 4x10 and 4x12 SoC families. Signed-off-by: Kamil Debski k.deb...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- .../devicetree/bindings/phy/samsung-usbphy.txt | 51 +++ drivers/phy/Kconfig| 21 ++ drivers/phy/Makefile |3 + drivers/phy/phy-exynos-usb.c | 245 ++ drivers/phy/phy-exynos-usb.h | 94 ++ drivers/phy/phy-exynos4210-usb.c | 295 + drivers/phy/phy-exynos4212-usb.c | 343 7 files changed, 1052 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt create mode 100644 drivers/phy/phy-exynos-usb.c create mode 100644 drivers/phy/phy-exynos-usb.h create mode 100644 drivers/phy/phy-exynos4210-usb.c create mode 100644 drivers/phy/phy-exynos4212-usb.c diff --git a/Documentation/devicetree/bindings/phy/samsung-usbphy.txt b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt new file mode 100644 index 000..f112b37 --- /dev/null +++ b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt @@ -0,0 +1,51 @@ +Samsung S5P/EXYNOS SoC series USB PHY +- + +Required properties: +- compatible : should be one of the listed compatibles: + - samsung,exynos4210-usbphy + - samsung,exynos4212-usbphy +- reg : a list of registers used by phy driver + - first and obligatory is the location of phy modules registers + - second and also required is the location of isolation registers + (isolation registers control the physical connection between the in + SoC modules and outside of the SoC, this also can be called enable + control in the documentation of the SoC) + - third is the location of the mode switch register, this only applies + to SoCs that have such a feature; mode switching enables to have + both host and device used the same SoC pins and is commonly used + when OTG is supported +- #phy-cells : from the generic phy bindings, must be 1; Please add if clock clock-names are required properties. + +The second cell in the PHY specifier identifies the PHY its meaning is SoC +dependent. For the currently supported SoCs (Exynos 4210 and Exynos 4212) it +is as follows: Can we say instead of 'its meaning is SoC dependent...' something like 'its meaning is compatible dependent? + 0 - USB device, + 1 - USB host, + 2 - HSIC0, + 3 - HSIC1, + +Example: + +For Exynos 4412 (compatible with Exynos 4212): + +exynos_usbphy: exynos-usbphy@125B { + compatible = samsung,exynos4212-usbphy; + reg = 0x125B 0x100 0x10020704 0x0c 0x1001021c 0x4; + ranges; + #address-cells = 1; + #size-cells = 1; Why do you have ranges, and #address-cells #size-cells here? + clocks = clock 305, clock 2, clock 2, clock 2, + clock 2; + clock-names = phy, device, host, hsic0, hsic1; + status = okay; + #phy-cells = 1; +}; + +Then the PHY can be used in other nodes such as: + +ehci@1258 { + status = okay; + phys = exynos_usbphy 2; + phy-names = hsic0; +}; -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html