Re: [PATCH V3 2/2] pinctrl: exynos: Add BUS1 pin controller for exynos7
On 10.12.2014 17:39, Vivek Gautam wrote: USB and Power regulator on Exynos7 require gpios available in BUS1 pin controller block. So adding the BUS1 pinctrl support. Signed-off-by: Naveen Krishna Ch naveenkrishna...@gmail.com Signed-off-by: Vivek Gautam gautam.vi...@samsung.com Cc: Tomasz Figa tomasz.f...@gmail.com Cc: Linus Walleij linus.wall...@linaro.org --- Changes since V2: - Added documentation on alias for BUS1 pin controller block. Changes since V1: - Added support for all pin banks which are part of BUS1 pin controller. .../devicetree/bindings/pinctrl/samsung-pinctrl.txt |1 + drivers/pinctrl/samsung/pinctrl-exynos.c| 19 +++ 2 files changed, 20 insertions(+) Acked-by: Tomasz Figa tomasz.f...@gmail.com Best regards, Tomasz -- 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 1/2] pinctrl: exynos: Add BUS1 pin controller for exynos7
Hi Vivek, Please see my comments below. 2014-11-24 22:02 GMT+09:00 Vivek Gautam gautam.vi...@samsung.com: USB and Power regulator on Exynos7 require gpios available in BUS1 pin controller block. So adding the BUS1 pinctrl support. Signed-off-by: Naveen Krishna Ch naveenkrishna...@gmail.com Signed-off-by: Vivek Gautam gautam.vi...@samsung.com Cc: Linus Walleij linus.wall...@linaro.org --- This patch was part of series: [PATCH 00/11] Exynos7: Adding USB 3.0 support https://lkml.org/lkml/2014/11/21/247 Changes since V1: - Added support for all pin banks which are part of BUS1 pin controller. drivers/pinctrl/samsung/pinctrl-exynos.c | 19 +++ 1 file changed, 19 insertions(+) I have missed this with previous patch, but DT bindings documentation should list all aliases for all supported compatible strings, because they are required for correct operation. There is a small section about aliases in [1] already, so please add there information about aliases for Exynos7 pin controllers along with their names, e.g. Aliases for controllers compatible with samsung,exynos7-pinctrl: - pinctrl0: pin controller of ALIVE block, - pinctrl1: pin controller of BUS0 block, [...] - pinctrl8: pin controller of BUS1 block. [1] Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt I guess you can do this in separate patch or respin this one with this added. Best regards, Tomasz -- 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 1/4] dwc3: exynos: Add support for SCLK present on Exynos7
Hi Anton, On 13.10.2014 06:54, Anton Tikhomirov wrote: Hi Vivek, Exynos7 also has a separate special gate clock going to the IP apart from the usual AHB clock. So add support for the same. As we discussed before, Exynos7 SoCs have 7 clocks to be controlled by the driver. Adding only sclk is not enough. I'm quite interested in this discussion. Has it happened on mailing lists? In general, previous SoCs also gave the possibility of controlling all the bus clocks separately, in addition to bulk gates, but there was no real advantage in using those, while burdening the clock tree with numerous clocks. Isn't Exynos7 similar in this aspect? Best regards, Tomasz -- 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 3/5] usb: phy: samsung: remove old USB 2.0 PHY driver
On 18.08.2014 13:02, Bartlomiej Zolnierkiewicz wrote: On Thursday, August 14, 2014 08:07:40 PM Vivek Gautam wrote: On Thursday, August 14, 2014 7:55 PM, Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com wrote There's one thing that I would want to comment here, since we don't have any new usb-phy driver for S3C64XX, so we can't simply remove this entire driver. I have posted my patch-series [1], which does cleanup while keeping the support for S3C64XX. AFAIK S3C64XX code from drivers/usb/phy/phy-samsung-usb2.c has never been used as this platform still uses its own code from arch/arm/mach-s3c64xx/setup-usb-phy.c (there are no users in the kernel tree of either s3c64xx-usb2phy platform device or samsung,s3c64xx-usb2phy DT compatible) . Therefore I think that the entire drivers/usb/phy/phy-samsung-usb2.c driver should be removed (somebody with the hardware can as well add S3C64XX support to the new drivers/phy/phy-samsung-usb2.c driver and port the platform to use it). I agree with removal of this driver. As Bart said, it is not used for S3C64xx at all. The platform was supposed to be moved to this driver, but that never happened. In fact, I already have a patch adding support for S3C64xx to the new driver. Best regards, Tomasz -- 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] drivers: phy: exynos-usb2: add support for Exynos 3250
On 07.07.2014 11:39, Marek Szyprowski wrote: This patch adds support for Exynos3250 SoC to Exynos2USB PHY driver. Although Exynos3250 has only one device phy interface, the register layout and all operations that are required to get it enabled are almost same as on Exynos4x12. The only different is one more register (REFCLKSEL) which need to be set and lack of MODE SWITCH register. Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com --- Changelog: v2: - added new binding documentation - removed superfluous defines - removed mode switch for 3250, because it is not really needed --- Documentation/devicetree/bindings/phy/samsung-phy.txt | 2 ++ drivers/phy/Kconfig | 12 ++-- drivers/phy/phy-exynos4x12-usb2.c | 17 +++-- drivers/phy/phy-samsung-usb2.c| 6 ++ drivers/phy/phy-samsung-usb2.h| 2 ++ 5 files changed, 31 insertions(+), 8 deletions(-) Reviewed-by: Tomasz Figa t.f...@samsung.com -- Best regards, Tomasz -- 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] drivers: phy: exynos-usb2: add support for Exynos 3250
Hi Marek, Please see my comments inline. On 04.07.2014 14:13, Marek Szyprowski wrote: This patch adds support for Exynos3250 SoC to Exynos2USB PHY driver. Although Exynos3250 has only one device phy interface, the register layout and all operations that are required to get it enabled are almost same as on Exynos4x12. The only different is one more register (REFCLKSEL) which need to be set. Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com --- drivers/phy/Kconfig | 12 ++-- drivers/phy/phy-exynos4x12-usb2.c | 18 ++ drivers/phy/phy-samsung-usb2.c| 6 ++ drivers/phy/phy-samsung-usb2.h| 2 ++ This patch adds new compatible string, but there is no respective change to the documentation. Please add it. 4 files changed, 32 insertions(+), 6 deletions(-) [snip] diff --git a/drivers/phy/phy-exynos4x12-usb2.c b/drivers/phy/phy-exynos4x12-usb2.c index 59d8dd3ff390..b29bbfacd4b5 100644 --- a/drivers/phy/phy-exynos4x12-usb2.c +++ b/drivers/phy/phy-exynos4x12-usb2.c @@ -67,6 +67,12 @@ #define EXYNOS_4x12_UPHYCLK_PHYFSEL_24MHZ(0x5 0) #define EXYNOS_4x12_UPHYCLK_PHYFSEL_50MHZ(0x7 0) +#define EXYNOS_4212_UPHYCLK_PHY0_ID_PULLUP (0x1 3) +#define EXYNOS_4212_UPHYCLK_PHY0_COMMON_ON (0x1 4) +#define EXYNOS_4212_UPHYCLK_PHY1_COMMON_ON (0x1 7) The 3 macros above don't seem to be used anywhere in this patch. Best regards, Tomasz -- 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 3/4] usb: ohci-exynos: Add facility to use phy provided by the generic phy framework
Hi Vivek, On 05.05.2014 07:02, Vivek Gautam wrote: Add support to consume phy provided by Generic phy framework. Keeping the support for older usb-phy intact right now, in order to prevent any functionality break in absence of relevant device tree side change for ohci-exynos. Once we move to new phy in the device nodes for ohci, we can remove the support for older phys. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com Cc: Jingoo Han jg1@samsung.com Acked-by: Alan Stern st...@rowland.harvard.edu Acked-by: Kukjin Kim kgene@samsung.com --- Changes from v5: - Removed setting phy explicitly to error pointer. - Changed error check to '-ENOSYS' instead of '-ENXIO' in failure case of devm_of_phy_get(). Changes from v4: - Removed 'phy-names' property from the bindings since we don't need it. - Restructured exynos_ohci_get_phy() function to handle error codes as well as return relevant error codes effectively. - Added IS_ERR() check for PHYs in exynos_ohci_phy_enable()/disable(). Changes from v3: - Calling usb_phy_shutdown() when exynos_ohci_phy_enable() is failing. - Made exynos_ohci_phy_disable() return void, since its return value did not serve any purpose. - Calling clk_disable_unprepare() in exynos_ohci_resume() when exynos_ohci_phy_enable() is failed. .../devicetree/bindings/usb/exynos-usb.txt | 16 +++ drivers/usb/host/ohci-exynos.c | 118 +--- 2 files changed, 118 insertions(+), 16 deletions(-) Reviewed-by: Tomasz Figa t.f...@samsung.com -- Best regards, Tomasz -- 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 v12 4/4] usb: ehci-exynos: Change to use phy provided by the generic phy framework
Hi Vivek, On 05.05.2014 07:02, Vivek Gautam wrote: From: Kamil Debski k.deb...@samsung.com Add the phy provider, supplied by new Exynos-usb2phy using Generic phy framework. Keeping the support for older USB phy intact right now, in order to prevent any functionality break in absence of relevant device tree side change for ehci-exynos. Once we move to new phy in the device nodes for ehci, we can remove the support for older phys. Signed-off-by: Kamil Debski k.deb...@samsung.com [gautam.vi...@samsung.com: Addressed review comments from mailing list] [gautam.vi...@samsung.com: Kept the code for old usb-phy, and just added support for new exynos5-usb2phy in generic phy framework] [gautam.vi...@samsung.com: Edited the commit message] Signed-off-by: Vivek Gautam gautam.vi...@samsung.com Cc: Jingoo Han jg1@samsung.com Acked-by: Alan Stern st...@rowland.harvard.edu Acked-by: Kukjin Kim kgene@samsung.com --- Changes from v11: - Removed setting phy explicitly to error pointer. - Changed error check to '-ENOSYS' instead of '-ENXIO' in failure case of devm_of_phy_get(). Changes from v10: - Removed 'phy-names' property from the bindings since we don't need it. - Restructured exynos_ehci_get_phy() function to handle error codes as well as return relevant error codes effectively. - Added IS_ERR() check for PHYs in exynos_ehci_phy_enable()/disable(). Changes from v9: - Calling usb_phy_shutdown() when exynos_ehci_phy_enable() is failing. - Made exynos_ehci_phy_disable() return void, since its return value did not serve any purpose. - Calling clk_disable_unprepare() in exynos_ehci_resume() when exynos_ehci_phy_enable() is failed. .../devicetree/bindings/usb/exynos-usb.txt | 15 +++ drivers/usb/host/ehci-exynos.c | 129 +--- 2 files changed, 124 insertions(+), 20 deletions(-) Reviewed-by: Tomasz Figa t.f...@samsung.com -- Best regards, Tomasz -- 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
[CCing DT maintainers] On 08.05.2014 11:03, Vivek Gautam wrote: On Thu, May 8, 2014 at 11:35 AM, Vivek Gautam gautam.vi...@samsung.com wrote: Hi Sylwester, On Tue, May 6, 2014 at 7:57 PM, Sylwester Nawrocki s.nawro...@samsung.com wrote: 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. 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. Getting the offsets from DT seems a cleaner way to handle this case of multi controllers. Mark, Rob, what is your opinion on this? Best regards, Tomasz -- 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] ARM: dts: Enable USB 3503 hub on exynos5250-snow
Hi Vivek, On 05.05.2014 14:30, Vivek Gautam wrote: The exynos5250-snow has a SMSC USB3503 connected in hardware only mode like a PHY. Enable support for it, and add necessary 'reset-gpio' for it. This is in correspondance to similar patch by Mark Brown 7c1b0ec ARM: dts: Enable USB hub on Arndale Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- Based on 'for-next' branch of kgene's linux-samsung tree. arch/arm/boot/dts/exynos5250-pinctrl.dtsi |7 +++ arch/arm/boot/dts/exynos5250-snow.dts | 14 ++ 2 files changed, 21 insertions(+) diff --git a/arch/arm/boot/dts/exynos5250-pinctrl.dtsi b/arch/arm/boot/dts/exynos5250-pinctrl.dtsi index 9a49e68..bd8c8f1 100644 --- a/arch/arm/boot/dts/exynos5250-pinctrl.dtsi +++ b/arch/arm/boot/dts/exynos5250-pinctrl.dtsi @@ -706,6 +706,13 @@ samsung,pin-pud = 0; samsung,pin-drv = 0; }; + + usb_hub_reset: usb-hub-reset { + samsung,pins = gpe1-0; + samsung,pin-function = 1; + samsung,pin-pud = 0; + samsung,pin-drv = 0; + }; This is not a generic pin config group and should not be added to this file. Btw. it just sets the pin to output, which is what the consumer driver can do as well by calling gpio_direction_output. Do you really need this? }; pinctrl@10d1 { diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts index 1ce1088..2e48f27 100644 --- a/arch/arm/boot/dts/exynos5250-snow.dts +++ b/arch/arm/boot/dts/exynos5250-snow.dts @@ -200,6 +200,20 @@ samsung,vbus-gpio = gpx1 1 0; }; + usb_hub_bus { + compatible = simple-bus; + #address-cells = 1; + #size-cells = 0; Why do you need this bus? + + // SMSC USB3503 connected in hardware only mode as a PHY Wrong comment style. Also basically the comment says exactly the same as could be inferred from the node below. + usb_hub { + compatible = smsc,usb3503a; + reset-gpios = gpe1 0 1; + pinctrl-0 = usb_hub_reset; + pinctrl-names = default; As above, do you really need to use pinctrl here? I believe the same comments apply for patch 2/2 as well. Best regards, Tomasz -- 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] ARM: dts: Enable USB 3503 hub on exynos5250-snow
On 08.05.2014 17:59, Andreas Färber wrote: Hello, Am 05.05.2014 14:30, schrieb Vivek Gautam: The exynos5250-snow has a SMSC USB3503 connected in hardware only mode like a PHY. Enable support for it, and add necessary 'reset-gpio' for it. This is in correspondance to similar patch by Mark Brown 7c1b0ec ARM: dts: Enable USB hub on Arndale Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- Based on 'for-next' branch of kgene's linux-samsung tree. arch/arm/boot/dts/exynos5250-pinctrl.dtsi |7 +++ arch/arm/boot/dts/exynos5250-snow.dts | 14 ++ 2 files changed, 21 insertions(+) The same snippet as for snow also fixed my USB issues on spring. Should it go into exynos5250-cros-common.dtsi instead? The GPIO pins used are different, so I don't think the hub node added by these patches could be shared. Also, I believe all you need to add is a simple node as follows usb_hub { compatible = smsc,usb3503a; reset-gpios = gpe1 0 1; }; at root level (see my comments to this patch in another reply), which is affordable to be duplicated for every platform that requires it, especially if the reset-gpios property differs between boards. Best regards, Tomasz -- 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 3/4] usb: ohci-exynos: Add facility to use phy provided by the generic phy framework
Hi Vivek, This looks much better, but there still are some issues. Please see my comments inline. On 02.05.2014 14:47, Vivek Gautam wrote: Add support to consume phy provided by Generic phy framework. Keeping the support for older usb-phy intact right now, in order to prevent any functionality break in absence of relevant device tree side change for ohci-exynos. Once we move to new phy in the device nodes for ohci, we can remove the support for older phys. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com Cc: Jingoo Han jg1@samsung.com Acked-by: Alan Stern st...@rowland.harvard.edu --- Changes from v4: - Removed 'phy-names' property from the bindings since we don't need it. - Restructured exynos_ohci_get_phy() function to handle error codes as well as return relevant error codes effectively. - Added IS_ERR() check for PHYs in exynos_ohci_phy_enable()/disable(). Changes from v3: - Calling usb_phy_shutdown() when exynos_ohci_phy_enable() is failing. - Made exynos_ohci_phy_disable() return void, since its return value did not serve any purpose. - Calling clk_disable_unprepare() in exynos_ohci_resume() when exynos_ohci_phy_enable() is failed. .../devicetree/bindings/usb/exynos-usb.txt | 16 +++ drivers/usb/host/ohci-exynos.c | 120 +--- 2 files changed, 120 insertions(+), 16 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt b/Documentation/devicetree/bindings/usb/exynos-usb.txt index d967ba1..49a9c6f 100644 --- a/Documentation/devicetree/bindings/usb/exynos-usb.txt +++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt @@ -38,6 +38,13 @@ Required properties: - interrupts: interrupt number to the cpu. - clocks: from common clock binding: handle to usb clock. - clock-names: from common clock binding: Shall be usbhost. + - port: if in the SoC there are OHCI phys, they should be listed here. + One phy per port. Each port should have following entries: + - reg: port number on OHCI controller, e.g + On Exynos5250, port 0 is USB2.0 otg phy + port 1 is HSIC phy0 + port 2 is HSIC phy1 + - phys: from the *Generic PHY* bindings, specifying phy used by port. Example: usb@1212 { @@ -47,6 +54,15 @@ Example: clocks = clock 285; clock-names = usbhost; + + #address-cells = 1; + #size-cells = 0; + port@0 { + reg = 0; + phys = usb2phy 1; + status = disabled; + }; + }; DWC3 diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c index 05f00e3..e66d391 100644 --- a/drivers/usb/host/ohci-exynos.c +++ b/drivers/usb/host/ohci-exynos.c @@ -18,6 +18,7 @@ #include linux/module.h #include linux/of.h #include linux/platform_device.h +#include linux/phy/phy.h #include linux/usb/phy.h #include linux/usb/samsung_usb_phy.h #include linux/usb.h @@ -33,28 +34,112 @@ static struct hc_driver __read_mostly exynos_ohci_hc_driver; #define to_exynos_ohci(hcd) (struct exynos_ohci_hcd *)(hcd_to_ohci(hcd)-priv) +#define PHY_NUMBER 3 + struct exynos_ohci_hcd { struct clk *clk; struct usb_phy *phy; struct usb_otg *otg; + struct phy *phy_g[PHY_NUMBER]; }; -static void exynos_ohci_phy_enable(struct device *dev) +static int exynos_ohci_get_phy(struct device *dev, + struct exynos_ohci_hcd *exynos_ohci) +{ + struct device_node *child; + struct phy *phy; + int phy_number; + int ret = 0; + + exynos_ohci-phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); + if (IS_ERR(exynos_ohci-phy)) { + ret = PTR_ERR(exynos_ohci-phy); + if (ret != -ENXIO ret != -ENODEV) { + dev_err(dev, no usb2 phy configured\n); + return ret; + } + dev_dbg(dev, Failed to get usb2 phy\n); + exynos_ohci-phy = ERR_PTR(-ENODEV); Here you already have exynos_ohci-phy set to an ERR_PTR() value, which was returned by devm_usb_get_phy(). There is no need to overwrite it. + } else { + exynos_ohci-otg = exynos_ohci-phy-otg; + } + + /* +* Getting generic phy: +* We are keeping both types of phys as a part of transiting OHCI +* to generic phy framework, so as to maintain backward compatibilty +* with old DTB. +* If there are existing devices using DTB files built from them, +* to remove the support for old bindings in this driver, +* we need to make sure that such devices have their DTBs +* updated to ones built from new DTS. +*/ + for_each_available_child_of_node(dev-of_node, child) { + ret = of_property_read_u32(child, reg, phy_number); +
Re: [PATCH v10 4/4] usb: ehci-exynos: Change to use phy provided by the generic phy framework
Hi Vivek, I believe the same comments as for the patch for ohci-exynos apply for this patch as well. Best regards, Tomasz On 30.04.2014 07:19, Vivek Gautam wrote: From: Kamil Debski k.deb...@samsung.com Add the phy provider, supplied by new Exynos-usb2phy using Generic phy framework. Keeping the support for older USB phy intact right now, in order to prevent any functionality break in absence of relevant device tree side change for ehci-exynos. Once we move to new phy in the device nodes for ehci, we can remove the support for older phys. Signed-off-by: Kamil Debski k.deb...@samsung.com [gautam.vi...@samsung.com: Addressed review comments from mailing list] [gautam.vi...@samsung.com: Kept the code for old usb-phy, and just added support for new exynos5-usb2phy in generic phy framework] [gautam.vi...@samsung.com: Edited the commit message] Signed-off-by: Vivek Gautam gautam.vi...@samsung.com Cc: Jingoo Han jg1@samsung.com Cc: Alan Stern st...@rowland.harvard.edu --- Changes from v9: - Calling usb_phy_shutdown() when exynos_ehci_phy_enable() is failing. - Made exynos_ehci_phy_disable() return void, since its return value did not serve any purpose. - Calling clk_disable_unprepare() in exynos_ehci_resume() when exynos_ehci_phy_enable() is failed. .../devicetree/bindings/usb/exynos-usb.txt | 18 +++ drivers/usb/host/ehci-exynos.c | 144 +--- 2 files changed, 142 insertions(+), 20 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt b/Documentation/devicetree/bindings/usb/exynos-usb.txt index a90c973..126a7a9 100644 --- a/Documentation/devicetree/bindings/usb/exynos-usb.txt +++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt @@ -12,6 +12,15 @@ Required properties: - interrupts: interrupt number to the cpu. - clocks: from common clock binding: handle to usb clock. - clock-names: from common clock binding: Shall be usbhost. + - port: if in the SoC there are EHCI phys, they should be listed here. + One phy per port. Each port should have following entries: + - reg: port number on EHCI controller, e.g + On Exynos5250, port 0 is USB2.0 otg phy + port 1 is HSIC phy0 + port 2 is HSIC phy1 + - phys: from the *Generic PHY* bindings; specifying phy used by port. + - phy-names: from the *Generic PHY* bindings; specifying name of phy +used by the port. Optional properties: - samsung,vbus-gpio: if present, specifies the GPIO that @@ -27,6 +36,15 @@ Example: clocks = clock 285; clock-names = usbhost; + + #address-cells = 1; + #size-cells = 0; + port@0 { + reg = 0; + phys = usb2phy 1; + phy-names = host; + status = disabled; + }; }; OHCI diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c index 4d763dc..931cfc8 100644 --- a/drivers/usb/host/ehci-exynos.c +++ b/drivers/usb/host/ehci-exynos.c @@ -19,6 +19,7 @@ #include linux/module.h #include linux/of.h #include linux/of_gpio.h +#include linux/phy/phy.h #include linux/platform_device.h #include linux/usb/phy.h #include linux/usb/samsung_usb_phy.h @@ -42,14 +43,119 @@ static const char hcd_name[] = ehci-exynos; static struct hc_driver __read_mostly exynos_ehci_hc_driver; +#define PHY_NUMBER 3 struct exynos_ehci_hcd { struct clk *clk; struct usb_phy *phy; struct usb_otg *otg; + struct phy *phy_g[PHY_NUMBER]; }; #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)-priv) +static int exynos_ehci_get_phy(struct device *dev, + struct exynos_ehci_hcd *exynos_ehci) +{ + struct device_node *child; + struct phy *phy; + int phy_number; + int ret = 0; + + exynos_ehci-phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); + if (IS_ERR(exynos_ehci-phy)) { + ret = PTR_ERR(exynos_ehci-phy); + /* This is the case when PHY config is disabled */ + if (ret == -ENXIO || ret == -ENODEV) { + dev_dbg(dev, Failed to get usb2 phy\n); + exynos_ehci-phy = NULL; + ret = 0; + } else if (ret == -EPROBE_DEFER) { + goto fail_phy; + } else { + dev_err(dev, no usb2 phy configured\n); + goto fail_phy; + } + } else { + exynos_ehci-otg = exynos_ehci-phy-otg; + } + + for_each_available_child_of_node(dev-of_node, child) { + ret = of_property_read_u32(child, reg, phy_number); + if (ret) { + dev_err(dev, Failed to parse device tree\n); +
Re: [PATCH v5 1/2] phy: Add new Exynos5 USB 3.0 PHY driver
Hi Vivek, On 22.04.2014 10:03, 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 --- .../devicetree/bindings/phy/samsung-phy.txt| 40 ++ drivers/phy/Kconfig| 11 + drivers/phy/Makefile |1 + drivers/phy/phy-exynos5-usbdrd.c | 629 4 files changed, 681 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. +- #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; + }; diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index 3bb05f1..8a5d2b4 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -166,4 +166,15 @@ config PHY_XGENE help This option enables support for APM X-Gene SoC multi-purpose PHY. +config PHY_EXYNOS5_USBDRD + tristate Exynos5 SoC series USB DRD PHY driver + depends on ARCH_EXYNOS5 OF + depends on HAS_IOMEM + select GENERIC_PHY + select MFD_SYSCON + help + Enable USB DRD PHY support for Exynos 5 SoC series. + This driver provides PHY interface for USB 3.0 DRD controller + present on Exynos5 SoC series. + I think you should probably keep the entries sorted, so this one should be somewhere around other EXYNOS PHYs. endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index 2faf78e..31baa0c 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -18,3 +18,4 @@ obj-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o obj-$(CONFIG_PHY_EXYNOS4X12_USB2) += phy-exynos4x12-usb2.o obj-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o obj-$(CONFIG_PHY_XGENE) += phy-xgene.o +obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o Ditto. diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c new file mode 100644 index 000..89d7ae8 --- /dev/null +++ b/drivers/phy/phy-exynos5-usbdrd.c @@ -0,0 +1,629 @@ +/* + * Samsung EXYNOS5 SoC series USB DRD PHY driver + * + * Phy provider for USB 3.0 DRD controller on Exynos5 SoC series + * + * Copyright (C) 2014 Samsung Electronics Co., Ltd. + * Author: Vivek Gautam gautam.vi...@samsung.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include linux/clk.h +#include linux/delay.h
Re: [PATCH v6 1/2] phy: Add new Exynos5 USB 3.0 PHY driver
Hi Vivek, I have reviewed v5 without noticing this one, but I think most of the comments still apply. Best regards, Tomasz On 22.04.2014 13:24, 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 --- Mistakenly sent a WIP patchset in v5 version of this patch, that gives build errors. So fixed those. Changes from v5: - Removed any mention about sclk_usbphy30* in the driver. .../devicetree/bindings/phy/samsung-phy.txt| 40 ++ drivers/phy/Kconfig| 11 + drivers/phy/Makefile |1 + drivers/phy/phy-exynos5-usbdrd.c | 626 4 files changed, 678 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. +- #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; + }; diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index 3bb05f1..8a5d2b4 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -166,4 +166,15 @@ config PHY_XGENE help This option enables support for APM X-Gene SoC multi-purpose PHY. +config PHY_EXYNOS5_USBDRD + tristate Exynos5 SoC series USB DRD PHY driver + depends on ARCH_EXYNOS5 OF + depends on HAS_IOMEM + select GENERIC_PHY + select MFD_SYSCON + help + Enable USB DRD PHY support for Exynos 5 SoC series. + This driver provides PHY interface for USB 3.0 DRD controller + present on Exynos5 SoC series. + endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index 2faf78e..31baa0c 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -18,3 +18,4 @@ obj-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o obj-$(CONFIG_PHY_EXYNOS4X12_USB2) += phy-exynos4x12-usb2.o obj-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o obj-$(CONFIG_PHY_XGENE) += phy-xgene.o +obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c new file mode 100644 index 000..ca1f6ab --- /dev/null +++ b/drivers/phy/phy-exynos5-usbdrd.c @@ -0,0 +1,626 @@ +/* + * Samsung EXYNOS5 SoC series USB DRD PHY driver + * + * Phy provider for USB 3.0 DRD controller on Exynos5 SoC series + * + * Copyright (C) 2014 Samsung Electronics Co., Ltd. + * Author: Vivek Gautam gautam.vi...@samsung.com + * + * This program is free software; you can redistribute it and/or modify + * it
Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver
Hi Vivek, On 15.04.2014 08:09, Vivek Gautam wrote: Hi Tomasz, On Thu, Apr 10, 2014 at 5:09 PM, Vivek Gautam gautamvivek1...@gmail.com wrote: On Wed, Apr 9, 2014 at 7:03 PM, Tomasz Figa t.f...@samsung.com wrote: On 09.04.2014 13:49, Vivek Gautam wrote: Hi, On Wed, Apr 9, 2014 at 4:36 PM, Tomasz Figa t.f...@samsung.com wrote: Hi Vivek, Please see my comments inline. On 08.04.2014 16:36, 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 --- .../devicetree/bindings/phy/samsung-phy.txt| 42 ++ drivers/phy/Kconfig| 11 + drivers/phy/Makefile |1 + drivers/phy/phy-exynos5-usbdrd.c | 668 4 files changed, 722 insertions(+) create mode 100644 drivers/phy/phy-exynos5-usbdrd.c [snip] + Additional clock required for Exynos5420: + - usb30_sclk_100m: Additional special clock used for PHY operation + depicted as 'sclk_usbphy30' in CMU of Exynos5420. Are you sure this isn't simply a gate for the ref clock, as it can be found on another SoC that is not upstream yet? I don't have documentation for Exynos 5420 so I can't tell, but I'd like to ask you to recheck this. From what i can see in the manual : sclk_usbphy30 is derived from OSCCLK. It is coming from a MUX (default input line to this is OSCCLK) and then through a DIV there's this gate. {OSCCLK + other sources} ---[MUX] --- [DIV] -- [GATE for sclk_usbphy30] the {rate of sclk_usbphy30} == OSCCLK However the 'ref' clock that we have been using is the actual oscillator clock. And on SoC Exynos5250, we don't have any such gate (sclk_usbphy30). So should this mean that ref clock and sclk_usbphy30 are still be controlled by two different gates ? Is there maybe a diagram of PHY input clocks in the datasheet, like for USB 2.0 PHY in Exynos4210/4412/5250 datasheets in the chapter about USB2.0 Device? Something like: || | ___| XusbXTI | Phy_fsel[2:0]| ___ | ___[X]___|| __|_|___|\__|_| | | _v___ | _ ^ | |/ | | _ | | || | | | | ___ | | ___| | || | | | | | |_|_| |___| | | X 0 ||_| PLL |__|_|_|CLK|_|_| _ | | | | || |DIV|_|_| |___[X] | |_| 12 |_|480 | |___| | | | MHz MHz |Digital| | XusbXTO | USB PHY|___| | || Below is the block diagram given for DRD controller. ___ || | | | | PHY | | | | controller |-|--- | |__ | | | || | | USB 3.0 | V | DRD | --- |Controller | | | ||USB30_SCLK_100M| USB 3.0 DRD | | | --- | PHY | | | Link cont. | | | | | - | | | |___| |_| Does this help ? So, USB30_SCLK_100M is the SCLK that we are talking in the driver. I don't see any reference to XXTI in the USB 3.0 DRD controller chapter (in both Exynos5250 and 5420) In addition to this there's one more point to be noticed here. On Exynos5420 system, the sclk_usbphy300 (which is the sclk_usbphy30 for USB DRD channel 0), is also the PICO phy clock, i.e. USB 2.0 phy clock. So we should add a similar clk_get() for this clock in the phy-exynos5250-usb2 driver too, to support Exynos5420. Is something clear from the above block diagram ? (although the diagram looks weird - space and tabs problem :-( ) Basically there's the clock USB30_SCLK_100M which is going into the USB 3.0 DRD PHY controller. And this is the only sclk mentioned in the block diagram for USB 3.0 DRD controller in Exynos5420. Same
Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver
On 14.04.2014 15:05, Kishon Vijay Abraham I wrote: On Monday 14 April 2014 05:35 PM, Vivek Gautam wrote: Hi Kishon, On Mon, Apr 14, 2014 at 5:24 PM, Kishon Vijay Abraham I kis...@ti.com wrote: Hi, On Wednesday 09 April 2014 04:36 PM, Tomasz Figa wrote: Hi Vivek, Please see my comments inline. On 08.04.2014 16:36, 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 --- .../devicetree/bindings/phy/samsung-phy.txt| 42 ++ drivers/phy/Kconfig| 11 + drivers/phy/Makefile |1 + drivers/phy/phy-exynos5-usbdrd.c | 668 4 files changed, 722 insertions(+) create mode 100644 drivers/phy/phy-exynos5-usbdrd.c [snip] +Additional clock required for Exynos5420: +- usb30_sclk_100m: Additional special clock used for PHY operation + depicted as 'sclk_usbphy30' in CMU of Exynos5420. Are you sure this isn't simply a gate for the ref clock, as it can be found on another SoC that is not upstream yet? I don't have documentation for Exynos 5420 so I can't tell, but I'd like to ask you to recheck this. +- samsung,syscon-phandle: phandle for syscon interface, which is used to + control pmu registers for power isolation. +- 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, + +Example: +usb3_phy: usbphy@1210 { +compatible = samsung,exynos5250-usbdrd-phy; +reg = 0x1210 0x100; +clocks = clock 286, clock 1; +clock-names = phy, usb3phy_refclk; Binding description above doesn't mention usb3phy_refclk entry. +samsung,syscon-phandle = pmu_syscon; +samsung,pmu-offset = 0x704; +#phy-cells = 1; +}; [snip] diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c new file mode 100644 index 000..ff54a7c --- /dev/null +++ b/drivers/phy/phy-exynos5-usbdrd.c [snip] +static int exynos5_usbdrd_phy_probe(struct platform_device *pdev) +{ +struct device *dev = pdev-dev; +struct device_node *node = dev-of_node; +struct exynos5_usbdrd_phy *phy_drd; +struct phy_provider *phy_provider; +struct resource *res; +const struct of_device_id *match; +const struct exynos5_usbdrd_phy_drvdata *drv_data; +struct regmap *reg_pmu; +u32 pmu_offset; +int i; + +/* + * Exynos systems are completely DT enabled, + * so lets not have any platform data support for this driver. + */ +if (!node) { +dev_err(dev, no device node found\n); This error message is not very meaningful. I'd rather use something like This driver can be only instantiated using Device Tree. how about just adding depend_on OF in Kconfig? Already added a depend on 'OF'. Copying below the part of Kconfig in this patch. Alright.. Do we need the check then? If config_OF is enabled devices will be created using device tree no? Not necessarily. Enabling support for OF doesn't mean that it is the only boot method that can be used. Legacy board files may be still available. I'm not sure why someone would try to instantiate this driver from them, though. Best regards, Tomasz -- 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/5] phy: Add new Exynos5 USB 3.0 PHY driver
Hi Vivek, Please see my comments inline. On 08.04.2014 16:36, 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 --- .../devicetree/bindings/phy/samsung-phy.txt| 42 ++ drivers/phy/Kconfig| 11 + drivers/phy/Makefile |1 + drivers/phy/phy-exynos5-usbdrd.c | 668 4 files changed, 722 insertions(+) create mode 100644 drivers/phy/phy-exynos5-usbdrd.c [snip] + Additional clock required for Exynos5420: + - usb30_sclk_100m: Additional special clock used for PHY operation + depicted as 'sclk_usbphy30' in CMU of Exynos5420. Are you sure this isn't simply a gate for the ref clock, as it can be found on another SoC that is not upstream yet? I don't have documentation for Exynos 5420 so I can't tell, but I'd like to ask you to recheck this. +- samsung,syscon-phandle: phandle for syscon interface, which is used to + control pmu registers for power isolation. +- 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, + +Example: + usb3_phy: usbphy@1210 { + compatible = samsung,exynos5250-usbdrd-phy; + reg = 0x1210 0x100; + clocks = clock 286, clock 1; + clock-names = phy, usb3phy_refclk; Binding description above doesn't mention usb3phy_refclk entry. + samsung,syscon-phandle = pmu_syscon; + samsung,pmu-offset = 0x704; + #phy-cells = 1; + }; [snip] diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c new file mode 100644 index 000..ff54a7c --- /dev/null +++ b/drivers/phy/phy-exynos5-usbdrd.c [snip] +static int exynos5_usbdrd_phy_probe(struct platform_device *pdev) +{ + struct device *dev = pdev-dev; + struct device_node *node = dev-of_node; + struct exynos5_usbdrd_phy *phy_drd; + struct phy_provider *phy_provider; + struct resource *res; + const struct of_device_id *match; + const struct exynos5_usbdrd_phy_drvdata *drv_data; + struct regmap *reg_pmu; + u32 pmu_offset; + int i; + + /* +* Exynos systems are completely DT enabled, +* so lets not have any platform data support for this driver. +*/ + if (!node) { + dev_err(dev, no device node found\n); This error message is not very meaningful. I'd rather use something like This driver can be only instantiated using Device Tree. + return -ENODEV; + } + + match = of_match_node(exynos5_usbdrd_phy_of_match, pdev-dev.of_node); + if (!match) { + dev_err(dev, of_match_node() failed\n); + return -EINVAL; + } + drv_data = match-data; + + phy_drd = devm_kzalloc(dev, sizeof(*phy_drd), GFP_KERNEL); + if (!phy_drd) + return -ENOMEM; + + dev_set_drvdata(dev, phy_drd); + phy_drd-dev = dev; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + phy_drd-reg_phy = devm_ioremap_resource(dev, res); + if (IS_ERR(phy_drd-reg_phy)) { + dev_err(dev, Failed to map register memory (phy)\n); devm_ioremap_resource() already prints an error message. Best regards, Tomasz -- 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 2/5] dt: exynos5420: Enable support for USB 3.0 PHY controller
On 08.04.2014 16:36, Vivek Gautam wrote: Add device tree nodes for USB 3.0 PHY present alongwith USB 3.0 controller Exynos 5420 SoC. This phy driver is based on generic phy framework. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- arch/arm/boot/dts/exynos5420.dtsi | 20 1 file changed, 20 insertions(+) diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi index 8db792b..a6efb52 100644 --- a/arch/arm/boot/dts/exynos5420.dtsi +++ b/arch/arm/boot/dts/exynos5420.dtsi @@ -652,4 +652,24 @@ clocks = clock 319, clock 318; clock-names = tmu_apbif, tmu_triminfo_apbif; }; + + usbdrd_phy0: phy@1210 { + compatible = samsung,exynos5420-usbdrd-phy; + reg = 0x1210 0x100; + clocks = clock 366, clock 1, clock 152; + clock-names = phy, ref, usb30_sclk_100m; As I mentioned in another reply, please make sure that usb30_sclk_100m isn't simply a gate clock for ref clock. Otherwise, Reviewed-by: Tomasz Figa t.f...@samsung.com -- Best regards, Tomasz -- 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 3/5] dt: exynos5420: Enable support for DWC3 controller
On 08.04.2014 16:36, Vivek Gautam wrote: Add device tree nodes for DWC3 controller present on Exynos 5420 SoC, to enable support for USB 3.0. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- arch/arm/boot/dts/exynos5420.dtsi | 34 ++ 1 file changed, 34 insertions(+) diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi index a6efb52..20c2d0b 100644 --- a/arch/arm/boot/dts/exynos5420.dtsi +++ b/arch/arm/boot/dts/exynos5420.dtsi @@ -653,6 +653,23 @@ clock-names = tmu_apbif, tmu_triminfo_apbif; }; + usb@1200 { + compatible = samsung,exynos5250-dwusb3; + clocks = clock 366; + clock-names = usbdrd30; + #address-cells = 1; + #size-cells = 1; + ranges; + + dwc3 { + compatible = synopsys,dwc3; + reg = 0x1200 0x1; + interrupts = 0 72 0; + phys = usbdrd_phy0 0, usbdrd_phy0 1; + phy-names = usb2-phy, usb3-phy; + }; + }; + usbdrd_phy0: phy@1210 { compatible = samsung,exynos5420-usbdrd-phy; reg = 0x1210 0x100; @@ -663,6 +680,23 @@ #phy-cells = 1; }; + usb@1240 { + compatible = samsung,exynos5250-dwusb3; + clocks = clock 367; + clock-names = usbdrd30; + #address-cells = 1; + #size-cells = 1; + ranges; + + dwc3 { + compatible = synopsys,dwc3; + reg = 0x1240 0x1; + interrupts = 0 73 0; + phys = usbdrd_phy1 0, usbdrd_phy1 1; + phy-names = usb2-phy, usb3-phy; + }; + }; + usbdrd_phy1: phy@1250 { compatible = samsung,exynos5420-usbdrd-phy; reg = 0x1250 0x100; Reviewed-by: Tomasz Figa t.f...@samsung.com -- Best regards, Tomasz -- 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 4/5] dt: exynos5250: Enable support for generic USB DRD phy
On 08.04.2014 16:36, Vivek Gautam wrote: Add device tree node for new usbdrd-phy driver, which is based on generic phy framework. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- arch/arm/boot/dts/exynos5250.dtsi | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi index b7dec41..92c6fcd 100644 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi @@ -530,6 +530,16 @@ }; }; + usbdrd_phy: phy@1210 { + compatible = samsung,exynos5250-usbdrd-phy; + reg = 0x1210 0x100; + clocks = clock 286, clock 1; + clock-names = phy, ref; + samsung,syscon-phandle = pmu_system_controller; + samsung,pmu-offset = 0x704; + #phy-cells = 1; + }; + usb@1211 { compatible = samsung,exynos4210-ehci; reg = 0x1211 0x100; Reviewed-by: Tomasz Figa t.f...@samsung.com -- Best regards, Tomasz -- 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 5/5] usb-phy: samsung-usb3: Remove older phy-samsung-usb3 driver
Hi Vivek, On 08.04.2014 16:36, Vivek Gautam wrote: Removing this older USB 3.0 DRD controller PHY driver, since a new driver based on generic phy framework is now available. Also removing the dt node for older driver from Exynos5250 device tree and updating the dt node for DWC3 controller. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- NOTE: This patch should be merged only when the new USB 3.0 DRD phy controller driver is available in the tree from the patches: phy: Add new Exynos5 USB 3.0 PHY driver; and dt: exynos5250: Enable support for generic USB DRD phy arch/arm/boot/dts/exynos5250.dtsi | 17 +- drivers/usb/phy/phy-samsung-usb.h | 83 - drivers/usb/phy/phy-samsung-usb3.c | 350 3 files changed, 2 insertions(+), 448 deletions(-) delete mode 100644 drivers/usb/phy/phy-samsung-usb3.c diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi index 92c6fcd..1cb1e91 100644 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi IMHO driver and dts changes should be separated into two patches, first updating device tree to use the new driver and second removing the driver. After fixing this issue, Reviewed-by: Tomasz Figa t.f...@samsung.com -- Best regards, Tomasz -- 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
Hi, On 09.04.2014 14:24, Vivek Gautam wrote: Hi Sylwester, On Wed, Apr 9, 2014 at 5:41 PM, Sylwester Nawrocki s.nawro...@samsung.com wrote: 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 ? 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 ? If I understand this correctly, this is just a switch that lets you control whether vbus is provided to the USB connector or not. If so, this doesn't look like an Exynos-specific thing at all and should rather be modeled on higher level. Best regards, Tomasz -- 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/5] phy: Add new Exynos5 USB 3.0 PHY driver
On 09.04.2014 13:49, Vivek Gautam wrote: Hi, On Wed, Apr 9, 2014 at 4:36 PM, Tomasz Figa t.f...@samsung.com wrote: Hi Vivek, Please see my comments inline. On 08.04.2014 16:36, 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 --- .../devicetree/bindings/phy/samsung-phy.txt| 42 ++ drivers/phy/Kconfig| 11 + drivers/phy/Makefile |1 + drivers/phy/phy-exynos5-usbdrd.c | 668 4 files changed, 722 insertions(+) create mode 100644 drivers/phy/phy-exynos5-usbdrd.c [snip] + Additional clock required for Exynos5420: + - usb30_sclk_100m: Additional special clock used for PHY operation + depicted as 'sclk_usbphy30' in CMU of Exynos5420. Are you sure this isn't simply a gate for the ref clock, as it can be found on another SoC that is not upstream yet? I don't have documentation for Exynos 5420 so I can't tell, but I'd like to ask you to recheck this. From what i can see in the manual : sclk_usbphy30 is derived from OSCCLK. It is coming from a MUX (default input line to this is OSCCLK) and then through a DIV there's this gate. {OSCCLK + other sources} ---[MUX] --- [DIV] -- [GATE for sclk_usbphy30] the {rate of sclk_usbphy30} == OSCCLK However the 'ref' clock that we have been using is the actual oscillator clock. And on SoC Exynos5250, we don't have any such gate (sclk_usbphy30). So should this mean that ref clock and sclk_usbphy30 are still be controlled by two different gates ? Is there maybe a diagram of PHY input clocks in the datasheet, like for USB 2.0 PHY in Exynos4210/4412/5250 datasheets in the chapter about USB2.0 Device? Something like: || | ___| XusbXTI | Phy_fsel[2:0]| ___ | ___[X]___|| __|_|___|\__|_| | | _v___ | _ ^ | |/ | | _ | | || | | | | ___ | | ___| | || | | | | | |_|_| |___| | | X 0 ||_| PLL |__|_|_|CLK|_|_| _ | | | | || |DIV|_|_| |___[X] | |_| 12 |_|480 | |___| | | | MHz MHz |Digital| | XusbXTO | USB PHY|___| | || Best regards, Tomasz -- 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 v9 3/4] phy: Add new Exynos USB 2.0 PHY driver
On 05.03.2014 16:28, Kamil Debski wrote: Add a new driver for the Exynos USB 2.0 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 --- .../devicetree/bindings/phy/samsung-phy.txt| 53 Documentation/phy/samsung-usb2.txt | 134 drivers/phy/Kconfig| 29 ++ drivers/phy/Makefile |3 + drivers/phy/phy-exynos4210-usb2.c | 261 drivers/phy/phy-exynos4x12-usb2.c | 328 drivers/phy/phy-samsung-usb2.c | 222 + drivers/phy/phy-samsung-usb2.h | 66 8 files changed, 1096 insertions(+) create mode 100644 Documentation/phy/samsung-usb2.txt create mode 100644 drivers/phy/phy-exynos4210-usb2.c create mode 100644 drivers/phy/phy-exynos4x12-usb2.c create mode 100644 drivers/phy/phy-samsung-usb2.c create mode 100644 drivers/phy/phy-samsung-usb2.h Reviewed-by: Tomasz Figa t.f...@samsung.com Best regards, Tomasz -- 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 v9 2/4] phy: core: Add devm_of_phy_get to phy-core
On 05.03.2014 16:28, Kamil Debski wrote: Adding devm_of_phy_get will allow to get phys by supplying a pointer to the struct device_node instead of struct device. Signed-off-by: Kamil Debski k.deb...@samsung.com --- drivers/phy/phy-core.c | 31 +++ include/linux/phy/phy.h |8 2 files changed, 39 insertions(+) Reviewed-by: Tomasz Figa t.f...@samsung.com Best regards, Tomasz -- 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 8/8] usb: ehci-exynos: Change to use phy provided by the generic phy framework
On 05.02.2014 18:30, Olof Johansson wrote: On Wed, Feb 5, 2014 at 7:57 AM, Kamil Debski k.deb...@samsung.com wrote: Hi Olof, Thank you for your review. From: Olof Johansson [mailto:o...@lixom.net] Sent: Wednesday, January 29, 2014 9:55 PM Hi, On Wed, Jan 29, 2014 at 9:29 AM, Kamil Debski k.deb...@samsung.com wrote: Change the phy provider used from the old one using the USB phy framework to a new one using the Generic phy framework. Signed-off-by: Kamil Debski k.deb...@samsung.com --- .../devicetree/bindings/usb/exynos-usb.txt | 13 +++ drivers/usb/host/ehci-exynos.c | 97 +--- 2 files changed, 76 insertions(+), 34 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt b/Documentation/devicetree/bindings/usb/exynos-usb.txt index d967ba1..25e199a 100644 --- a/Documentation/devicetree/bindings/usb/exynos-usb.txt +++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt @@ -12,6 +12,10 @@ Required properties: - interrupts: interrupt number to the cpu. - clocks: from common clock binding: handle to usb clock. - clock-names: from common clock binding: Shall be usbhost. + - port: if in the SoC there are EHCI phys, they should be listed here. +One phy per port. Each port should have its reg entry with a +consecutive number. Also it should contain phys and phy-names entries +specifying the phy used by the port. Optional properties: - samsung,vbus-gpio: if present, specifies the GPIO that @@ -27,6 +31,15 @@ Example: clocks = clock 285; clock-names = usbhost; + + #address-cells = 1; + #size-cells = 0; + port@0 { + reg = 0; + phys = usb2phy 1; + phy-names = host; + status = disabled; + }; }; OHCI [...] @@ -102,14 +132,26 @@ static int exynos_ehci_probe(struct platform_device *pdev) samsung,exynos5440-ehci)) goto skip_phy; - phy = devm_usb_get_phy(pdev-dev, USB_PHY_TYPE_USB2); - if (IS_ERR(phy)) { - usb_put_hcd(hcd); - dev_warn(pdev-dev, no platform data or transceiver defined\n); - return -EPROBE_DEFER; - } else { - exynos_ehci-phy = phy; - exynos_ehci-otg = phy-otg; + for_each_available_child_of_node(pdev-dev.of_node, child) { + err = of_property_read_u32(child, reg, phy_number); + if (err) { + dev_err(pdev-dev, Failed to parse device tree\n); + of_node_put(child); + return err; + } + if (phy_number = PHY_NUMBER) { + dev_err(pdev-dev, Failed to parse device tree - number out of range\n); + of_node_put(child); + return -EINVAL; + } + phy = devm_of_phy_get(pdev-dev, child, 0); + of_node_put(child); + if (IS_ERR(phy)) { + dev_err(pdev-dev, Failed to get phy number %d, + phy_number); + return PTR_ERR(phy); + } + exynos_ehci-phy[phy_number] = phy; this looks like it is now breaking older device trees, where ports might not be described. Since device tree interfaces need to be backwards compatible, you still need to handle the old case of not having ports described. There are two ways of doing this: 1. Fall back to the old behavior if there are no ports 2. Use a new compatible value for the new model with port subnodes, and if the old compatible value is used, then fall back to the old behavior. I'm guessing (1) might be easiest since you can check for the presence of #address-cells to tell if this is just an old style node, or if it's a new-style node without any ports below it. The ultimate goal is to remove the old phy driver. Unfortunately this has to be synced with the new USB3 phy driver by Vivek Gautam. I think he is also close to completion. What about this case? In the end the old driver will be removed and no longer be supported. Having backward compatibility in mind, it is possible to have the old and the new phy driver together in one kernel release. But do we want to have two drivers doing the same thing at the same time? It is mostly irrelevant if there is a new driver or not -- the old device tree has to keep working. In this case it would mean that the new driver needs to work with older device trees as well, or people will see functionality regressing. The device tree is a description of the hardware, not an extension of the driver. The problem with this case is that when the original driver was added there was no way to bind PHY providers and consumers together. Basically there was no generic PHY subsystem. Instead the hacky USB
Re: [PATCH v3] phy: Add new Exynos5 USB 3.0 PHY driver
Hi Vivek, On 14.02.2014 14:53, Vivek Gautam wrote: Changes from v2: 1) Added support for multiple PHYs (UTMI+ and PIPE3) and related changes in the driver structuring. I'm a bit skeptical about this separation. Can the PHY operate with just the UTMI+ or PIPE3 part enabled alone without the other? Can any PHY consumer operate this way? Yes :-) As also pointed by Kishon the PHY consumer (which is DWC3 in case of Exynos5 SoC series) should theoretically be able use either UTMI+ phy for High speed operations or both (UTMI+ and PIPE3) for Super Speed operations. OK, that's fine then. This is the explanation I needed, thanks. I believe the right thing to do here is to do all the initialization in .power_on() and let the driver simply call phy_power_on() when it needs the PHY and phy_power_off() otherwise. If this is what we should be doing then what will be the purpose of two separate APIs : phy_power_on() and phy_init(). Am i missing while understanding the things. I don't understand this separation as well. Operations that should be done together shouldn't be separated. Is there any case when you can call one of phy_power_on() and phy_init() without calling another one right before/after it? Best regards, Tomasz -- 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] phy: Add new Exynos5 USB 3.0 PHY driver
Hi Vivek, This patch is just adding the PHY driver. I would also like to look at some users of it, to see how this works when put together. For now, please see my comments inline. On 20.01.2014 14:42, 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 v2: 1) Added support for multiple PHYs (UTMI+ and PIPE3) and related changes in the driver structuring. I'm a bit skeptical about this separation. Can the PHY operate with just the UTMI+ or PIPE3 part enabled alone without the other? Can any PHY consumer operate this way? Introducing separation of something that can't exist alone doesn't add any value, but instead makes things more difficult to work with. Of course, it's fine if the answer to my questions above if yes, but better safe than sorry. 2) Added a xlate function to get the required phy out of number of PHYs in mutiple PHY scenerio. 3) Changed the names of few structures and variables to have a clearer meaning. 4) Added 'usb3phy_config' structure to take care of mutiple phys for a SoC having 'exynos5_usb3phy_drv_data' driver data. 5) Not deleting support for old driver 'phy-samsung-usb3' until required support for generic phy is added to DWC3. [snip] + +- aliases: For SoCs like Exynos5420 having multiple USB PHY controllers, + 'usb3_phy' nodes should have numbered alias in the aliases node, + in the form of usb3phyN, N = 0, 1... (depending on number of + controllers). +Example: + aliases { + usb3phy0 = usb3_phy0; + usb3phy1 = usb3_phy1; + }; What is the reason to have these aliases? diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index 330ef2d..32f9f38 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -51,4 +51,12 @@ config PHY_EXYNOS_DP_VIDEO help Support for Display Port PHY found on Samsung EXYNOS SoCs. +config PHY_EXYNOS5_USB3 + tristate Exynos5 SoC series USB 3.0 PHY driver + depends on ARCH_EXYNOS5 + select GENERIC_PHY + select MFD_SYSCON + help + Enable USB 3.0 PHY support for Exynos 5 SoC series + endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index d0caae9..9c06a61 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -7,3 +7,4 @@ obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o +obj-$(CONFIG_PHY_EXYNOS5_USB3) += phy-exynos5-usb3.o diff --git a/drivers/phy/phy-exynos5-usb3.c b/drivers/phy/phy-exynos5-usb3.c new file mode 100644 index 000..24efed0 --- /dev/null +++ b/drivers/phy/phy-exynos5-usb3.c @@ -0,0 +1,621 @@ +/* + * Samsung EXYNOS5 SoC series USB 3.0 PHY driver + * + * Copyright (C) 2013 Samsung Electronics Co., Ltd. + * Author: Vivek Gautam gautam.vi...@samsung.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include linux/clk.h +#include linux/delay.h +#include linux/io.h +#include linux/kernel.h +#include linux/module.h +#include linux/of.h +#include linux/of_address.h +#include linux/phy/phy.h +#include linux/platform_device.h +#include linux/mutex.h +#include linux/mfd/syscon.h +#include linux/regmap.h + +/* Exynos USB PHY registers */ +#define EXYNOS5_FSEL_9MHZ6 0x0 +#define EXYNOS5_FSEL_10MHZ 0x1 +#define EXYNOS5_FSEL_12MHZ 0x2 +#define EXYNOS5_FSEL_19MHZ20x3 +#define EXYNOS5_FSEL_20MHZ 0x4 +#define EXYNOS5_FSEL_24MHZ 0x5 +#define EXYNOS5_FSEL_50MHZ 0x7 + +/* EXYNOS5: USB 3.0 DRD PHY registers */ +#define EXYNOS5_DRD_LINKSYSTEM (0x04) nit: No need for parentheses around simple literal. (+ more occurrences below) + +#define LINKSYSTEM_FLADJ_MASK (0x3f 1) +#define LINKSYSTEM_FLADJ(_x) ((_x) 1) +#define LINKSYSTEM_XHCI_VERSION_CONTROL(0x1 27) nit: BIT() macro could be used for single bits. (+ more occurrences below) + +#define EXYNOS5_DRD_PHYUTMI(0x08) + +#define PHYUTMI_OTGDISABLE (0x1 6) +#define PHYUTMI_FORCESUSPEND (0x1 1) +#define PHYUTMI_FORCESLEEP (0x1 0) + +#define EXYNOS5_DRD_PHYPIPE(0x0c) + +#define EXYNOS5_DRD_PHYCLKRST (0x10) + +#define PHYCLKRST_EN_UTMISUSPEND
Re: [PATCH v6 8/8] usb: ehci-exynos: Change to use phy provided by the generic phy framework
Hi Alan, On 29.01.2014 21:42, Alan Stern wrote: On Wed, 29 Jan 2014, Kamil Debski wrote: Change the phy provider used from the old one using the USB phy framework to a new one using the Generic phy framework. Signed-off-by: Kamil Debski k.deb...@samsung.com --- .../devicetree/bindings/usb/exynos-usb.txt | 13 +++ drivers/usb/host/ehci-exynos.c | 97 +--- 2 files changed, 76 insertions(+), 34 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt b/Documentation/devicetree/bindings/usb/exynos-usb.txt index d967ba1..25e199a 100644 --- a/Documentation/devicetree/bindings/usb/exynos-usb.txt +++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt @@ -12,6 +12,10 @@ Required properties: - interrupts: interrupt number to the cpu. - clocks: from common clock binding: handle to usb clock. - clock-names: from common clock binding: Shall be usbhost. + - port: if in the SoC there are EHCI phys, they should be listed here. +One phy per port. Each port should have its reg entry with a consecutive +number. Also it should contain phys and phy-names entries specifying the +phy used by the port. What is the reg entry number used for? As far as I can see, it isn't used for anything. In which case, why have it at all? The reg property is here to identify which EHCI port the node is describing. This should be mentioned in the documentation, though, as well as the whole description of port nodes should be written in a more structured manner, just as other properties. Best regards, Tomasz -- 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 0/2] ohci and ehci-platform clks, phy and dt support
Hi, [Cc'ing DT maintainers directly] On 12.01.2014 04:04, Tony Prisk wrote: On 12/01/14 11:30, Alan Stern wrote: On Fri, 10 Jan 2014, Hans de Goede wrote: Hi, On 01/11/2014 12:50 AM, Sergei Shtylyov wrote: Hello. On 01/11/2014 01:46 AM, Hans de Goede wrote: Here is v4 of my ohci and ehci-platform clks, phy and dt support patch-set, this version should be 100% ready for merging upstream. I see you've decided to completely ignore my opinion. NAK, FWIW. I'm sorry but the whole prefix thing has become a thing of -ETOOMUCHBIKESHEDDING, everyone except you seems to be fine with mmio and and one point in time we need to make a decision and move forward. If this isn't beating a dead horse... Maybe everyone can agree on a name like ohci-generic or generic-ohci. That seems like a pretty good description of the hardware that the platform driver can handle. Alan Stern I prefer the -generic option, although generic- is equally fine - Having said that, I don't really care if it's called mmio either (although this does seem less 'descriptive'). Grepping over existing dts files, I can find several occurrences of usb-ehci compatible string: at91sam9g45.dtsi: compatible = atmel,at91sam9g45-ehci, usb-ehci; at91sam9x5.dtsi:compatible = atmel,at91sam9g45-ehci, usb-ehci; omap3.dtsi: compatible = ti,ehci-omap, usb-ehci; omap4.dtsi: compatible = ti,ehci-omap, usb-ehci; omap5.dtsi: compatible = ti,ehci-omap, usb-ehci; sama5d3.dtsi: compatible = atmel,at91sam9g45-ehci, usb-ehci; spear13xx.dtsi: compatible = st,spear600-ehci, usb-ehci; spear13xx.dtsi: compatible = st,spear600-ehci, usb-ehci; spear3xx.dtsi: compatible = st,spear600-ehci, usb-ehci; spear600.dtsi: compatible = st,spear600-ehci, usb-ehci; spear600.dtsi: compatible = st,spear600-ehci, usb-ehci; tegra114.dtsi: compatible = nvidia,tegra30-ehci, usb-ehci; tegra114.dtsi: compatible = nvidia,tegra30-ehci, usb-ehci; tegra20.dtsi: compatible = nvidia,tegra20-ehci, usb-ehci; tegra20.dtsi: compatible = nvidia,tegra20-ehci, usb-ehci; tegra20.dtsi: compatible = nvidia,tegra20-ehci, usb-ehci; tegra30.dtsi: compatible = nvidia,tegra30-ehci, usb-ehci; tegra30.dtsi: compatible = nvidia,tegra30-ehci, usb-ehci; tegra30.dtsi: compatible = nvidia,tegra30-ehci, usb-ehci; Same for usb-ohci: arch/arm/boot/dts/at91rm9200.dtsi: compatible = atmel,at91rm9200-ohci, usb-ohci; arch/arm/boot/dts/at91sam9260.dtsi: compatible = atmel,at91rm9200-ohci, usb-ohci; arch/arm/boot/dts/at91sam9263.dtsi: compatible = atmel,at91rm9200-ohci, usb-ohci; arch/arm/boot/dts/at91sam9g45.dtsi: compatible = atmel,at91rm9200-ohci, usb-ohci; arch/arm/boot/dts/at91sam9n12.dtsi: compatible = atmel,at91rm9200-ohci, usb-ohci; arch/arm/boot/dts/at91sam9x5.dtsi: compatible = atmel,at91rm9200-ohci, usb-ohci; arch/arm/boot/dts/lpc32xx.dtsi: compatible = nxp,ohci-nxp, usb-ohci; arch/arm/boot/dts/omap3.dtsi: compatible = ti,ohci-omap3, usb-ohci; arch/arm/boot/dts/omap4.dtsi: compatible = ti,ohci-omap3, usb-ohci; arch/arm/boot/dts/omap5.dtsi: compatible = ti,ohci-omap3, usb-ohci; arch/arm/boot/dts/sama5d3.dtsi: compatible = atmel,at91rm9200-ohci, usb-ohci; arch/arm/boot/dts/spear13xx.dtsi: compatible = st,spear600-ohci, usb-ohci; arch/arm/boot/dts/spear13xx.dtsi: compatible = st,spear600-ohci, usb-ohci; arch/arm/boot/dts/spear3xx.dtsi:compatible = st,spear600-ohci, usb-ohci; arch/arm/boot/dts/spear3xx.dtsi:compatible = st,spear600-ohci, usb-ohci; arch/arm/boot/dts/spear600.dtsi:compatible = st,spear600-ohci, usb-ohci; arch/arm/boot/dts/spear600.dtsi:compatible = st,spear600-ohci, usb-ohci; For usb-ehci there is even a documentation file [1], while usb-ohci seems to be undocumented. [1] Documentation/devicetree/bindings/usb/usb-ehci.txt Aren't they both something that should be accounted for in this series? Best regards, Tomasz -- 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 v6 6/9] phy: Add support for S5PV210 to the Exynos USB 2.0 PHY driver
From: Mateusz Krawczuk mat.krawc...@gmail.com Add support for the Samsung's S5PV210 SoC to the Exynos USB 2.0 PHY driver. Signed-off-by: Mateusz Krawczuk m.krawc...@partner.samsung.com [k.deb...@samsung.com: cleanup and commit description] [k.deb...@samsung.com: make changes accordingly to the mailing list comments] Signed-off-by: Kamil Debski k.deb...@samsung.com [tomasz.f...@gmail.com: fix compilation issues and UPHYCLK register setup] Signed-off-by: Tomasz Figa tomasz.f...@gmail.com --- .../devicetree/bindings/phy/samsung-phy.txt| 1 + drivers/phy/Kconfig| 10 ++ drivers/phy/Makefile | 1 + drivers/phy/phy-s5pv210-usb2.c | 188 + drivers/phy/phy-samsung-usb2.c | 6 + drivers/phy/phy-samsung-usb2.h | 1 + 6 files changed, 207 insertions(+) create mode 100644 drivers/phy/phy-s5pv210-usb2.c Changes since v5: - Fixed compilation issues. - Fixed incorrect value written to UPHYCLK register. diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt index 39d52cc..eb40460 100644 --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt @@ -26,6 +26,7 @@ Samsung S5P/EXYNOS SoC series USB PHY Required properties: - compatible : should be one of the listed compatibles: + - samsung,s5pv210-usb2-phy - samsung,exynos4210-usb2-phy - samsung,exynos4212-usb2-phy - reg : a list of registers used by phy driver diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index 55b49d1..8298d7c 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -61,6 +61,16 @@ config PHY_SAMSUNG_USB2 particular SoCs has to be enabled in addition to this driver. Number and type of supported phys depends on the SoC. +config PHY_S5PV210_USB2 + bool Support for S5PV210 + depends on PHY_SAMSUNG_USB2 + depends on ARCH_S5PV210 + help + Enable USB PHY support for S5PV210. This option requires that Samsung + USB 2.0 PHY driver is enabled and means that support for this + particular SoC is compiled in the driver. In case of S5PV210 two phys + are available - device and host. + config PHY_EXYNOS4210_USB2 bool Support for Exynos 4210 depends on PHY_SAMSUNG_USB2 diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index 9f4befd..fefc6c2 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -8,5 +8,6 @@ obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o obj-$(CONFIG_OMAP_USB2)+= phy-omap-usb2.o obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o obj-$(CONFIG_PHY_SAMSUNG_USB2) += phy-samsung-usb2.o +obj-$(CONFIG_PHY_S5PV210_USB2) += phy-s5pv210-usb2.o obj-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o obj-$(CONFIG_PHY_EXYNOS4212_USB2) += phy-exynos4212-usb2.o diff --git a/drivers/phy/phy-s5pv210-usb2.c b/drivers/phy/phy-s5pv210-usb2.c new file mode 100644 index 000..f96764c --- /dev/null +++ b/drivers/phy/phy-s5pv210-usb2.c @@ -0,0 +1,188 @@ +/* + * Samsung SoC USB 1.1/2.0 PHY driver - S5PV210 support + * + * Copyright (C) 2013 Samsung Electronics Co., Ltd. + * Authors: Kamil Debski k.deb...@samsung.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include linux/delay.h +#include linux/io.h +#include linux/phy/phy.h +#include phy-samsung-usb2.h + +/* Exynos USB PHY registers */ + +/* PHY power control */ +#define S5PV210_UPHYPWR0x0 + +#define S5PV210_UPHYPWR_PHY0_SUSPEND BIT(0) +#define S5PV210_UPHYPWR_PHY0_PWR BIT(3) +#define S5PV210_UPHYPWR_PHY0_OTG_PWR BIT(4) +#define S5PV210_UPHYPWR_PHY0 ( \ + S5PV210_UPHYPWR_PHY0_SUSPEND | \ + S5PV210_UPHYPWR_PHY0_PWR | \ + S5PV210_UPHYPWR_PHY0_OTG_PWR) + +#define S5PV210_UPHYPWR_PHY1_SUSPEND BIT(6) +#define S5PV210_UPHYPWR_PHY1_PWR BIT(7) +#define S5PV210_UPHYPWR_PHY1 ( \ + S5PV210_UPHYPWR_PHY1_SUSPEND | \ + S5PV210_UPHYPWR_PHY1_PWR) + +/* PHY clock control */ +#define S5PV210_UPHYCLK0x4 + +#define S5PV210_UPHYCLK_PHYFSEL_MASK (0x3 0) +#define S5PV210_UPHYCLK_PHYFSEL_48MHZ (0x0 0) +#define S5PV210_UPHYCLK_PHYFSEL_24MHZ (0x3 0) +#define S5PV210_UPHYCLK_PHYFSEL_12MHZ (0x2 0) + +#define S5PV210_UPHYCLK_PHY0_ID_PULLUP BIT(2) +#define S5PV210_UPHYCLK_PHY0_COMMON_ON BIT(4) +#define S5PV210_UPHYCLK_PHY1_COMMON_ON BIT(7) + +/* PHY reset control */ +#define S5PV210_UPHYRST0x8 + +#define S5PV210_URSTCON_PHY0 BIT(0) +#define S5PV210_URSTCON_OTG_HLINK BIT(1) +#define S5PV210_URSTCON_OTG_PHYLINKBIT(2
Re: [PATCH v8 3/9] usb: gadget: s3c-hsotg: enable build for other platforms
On Monday 23 of December 2013 10:25:57 Felipe Balbi wrote: On Thu, Dec 19, 2013 at 09:23:04AM -0500, Matt Porter wrote: Remove unused Samsung-specific machine include and Kconfig dependency on S3C. Signed-off-by: Matt Porter mpor...@linaro.org Reviewed-by: Markus Mayer markus.ma...@linaro.org Reviewed-by: Tim Kryger tim.kry...@linaro.org --- drivers/usb/gadget/Kconfig | 7 +++ drivers/usb/gadget/s3c-hsotg.c | 2 -- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index a91e642..181e760 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -294,11 +294,10 @@ config USB_PXA27X gadget drivers to also be dynamically linked. config USB_S3C_HSOTG - tristate S3C HS/OtG USB Device controller - depends on S3C_DEV_USB_HSOTG + tristate Designware/S3C HS/OtG USB Device controller causes build failure in x86. Sorry dropping from my queue. Maybe depends on ARM would be a good enough stepping stone? Best regards, Tomasz -- 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: ohci-at91: fix irq and iomem resource retrieval
Hi Boris, On Thursday 05 of December 2013 10:50:13 Boris BREZILLON wrote: When using dt resources retrieval (interrupts and reg properties) there is no predefined order for these resources in the platform dev resources table. Retrieve resources using the platform_get_resource function instead of direct resource table entries to avoid resource type mismatch. Signed-off-by: Boris BREZILLON b.brezil...@overkiz.com Acked-by: Nicolas Ferre nicolas.fe...@atmel.com Signed-off-by: Alan Stern st...@rowland.harvard.edu --- Changes since v2: - split the patch series to isolate the urgent fix provided by this patch Changes since v1: - none drivers/usb/host/ohci-at91.c | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c index 418444e..7aec6ca 100644 --- a/drivers/usb/host/ohci-at91.c +++ b/drivers/usb/host/ohci-at91.c @@ -136,23 +136,26 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver, struct ohci_hcd *ohci; int retval; struct usb_hcd *hcd = NULL; + struct device *dev = pdev-dev; + struct resource *mem_r, *irq_r; - if (pdev-num_resources != 2) { - pr_debug(hcd probe: invalid num_resources); + mem_r = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!mem_r) { + dev_dbg(dev, hcd probe: missing memory resource\n); return -ENODEV; } - if ((pdev-resource[0].flags != IORESOURCE_MEM) - || (pdev-resource[1].flags != IORESOURCE_IRQ)) { - pr_debug(hcd probe: invalid resource type\n); + irq_r = platform_get_resource(pdev, IORESOURCE_IRQ, 0); You could have simply used platform_get_irq() here, but I guess it's just a matter of preference, so: Reviewed-by: Tomasz Figa tomasz.f...@gmail.com Best regards, Tomasz -- 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 3/3] usb: ohci-at91: use device managed clk retrieval
2013/12/4 Douglas Gilbert dgilb...@interlog.com: On 13-12-04 04:21 PM, Alan Stern wrote: On Wed, 4 Dec 2013, boris brezillon wrote: The patches look fine to me. But only the 1/3 patch fixes a bug; the others merely change the resource management. Do you want me to split this series ? 1) the 1st patch that should be merged in 3.13 2) patches 2 to 4 that might be applied later That probably would make Greg happier. Putting my initial reporter hat on, USB OHCI is completely broken in lk 3.13.0 rc1 and rc2 for AT91 family members. So it is difficult to make the situation worse. IMO the whole 4 patches should go in, since it only impacts that family. Also the kernel is more than a month away from release. Perhaps the naysayers should be looking around for whatever else the of/irq: Pass trigger type in IRQ resource flags patch has broken. ...or rather whatever brokenness it has uncovered. Best regards, Tomasz -- 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 RFC 1/4] phy: Add new Exynos5 USB 3.0 PHY driver
Hi Vivek, On Thursday 31 of October 2013 13:15:41 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. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- .../devicetree/bindings/phy/samsung-phy.txt| 20 + drivers/phy/Kconfig|7 + drivers/phy/Makefile |1 + drivers/phy/phy-exynos5-usb3.c | 562 4 files changed, 590 insertions(+), 0 deletions(-) create mode 100644 drivers/phy/phy-exynos5-usb3.c diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt index c0fccaa..9b5c111 100644 --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt @@ -20,3 +20,23 @@ Required properties: - compatible : should be samsung,exynos5250-dp-video-phy; - reg : offset and length of the Display Port PHY register set; - #phy-cells : from the generic PHY bindings, must be 0; + +Samsung Exynos5 SoC seiries USB 3.0 PHY controller typo: s/seiries/series/ +-- + +Required properties: +- compatible : + should be samsung,exynos5250-usb3phy for exynos5250 SoC + should be samsung,exynos5420-usb3phy for exynos5420 SoC I'd slightly change this into something like this: - compatible: Should be set to one of following supported values: - samsung,exynos5250-usb3phy - for Exynos5250 SoC, - samsung,exynos5420-usb3phy - for Exynos5420 SoC. +- reg : Register offset and length array + - first field corresponds to USB 3.0 PHY register set; + - second field corresponds to PHY power isolation register + present in PMU; For consistency and to make things more future-proof, you should consider using the PMU indirectly, through the syscon interface, as done in Leela Krishna Amudala's series[1] in case of watchdog driver. I will tell Kamil to do the same for USB 2.0 PHY as well. [1] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/24652 +- clocks: Clock IDs array as required by the controller +- clock-names: names of clocks correseponding to IDs in the clock property; + Required clocks: + - first clock is main PHY clock (same as USB 3.0 controller IP clock) + - second clock is reference clock (usually crystal clock) + optional clock: + - third clock is special clock used by PHY for operation Is this clock really optional? It looks like it's required for Exynos5420. If so, you should instead change this to: Additional clocks required for Exynos5420: Also you have not specified names of the clocks, just what they are. Please remember that those are input names, so you can imagine them as names of clock input pins of the IP block, not SoC-level clock names. +- #phy-cells : from the generic PHY bindings, must be 0; I'd also add an example of correct USB 3.0 PHY device tree node here. diff --git a/drivers/phy/phy-exynos5-usb3.c b/drivers/phy/phy-exynos5-usb3.c new file mode 100644 index 000..b9a2674 --- /dev/null +++ b/drivers/phy/phy-exynos5-usb3.c @@ -0,0 +1,562 @@ [snip] +#define EXYNOS5_DRD_PHYRESUME(0x34) +#define EXYNOS5_DRD_LINKPORT (0x44) + + nit: Duplicate blank line. +/* Isolation, configured in the power management unit */ +#define EXYNOS5_USB_ISOL_DRD (1 0) + +#define CLKSEL_ERROR -1 What's this? + +#ifndef KHZ +#define KHZ 1000 +#endif + +#ifndef MHZ +#define MHZ (KHZ * KHZ) +#endif Do you really need the #ifndef's above? + +enum samsung_cpu_type { + TYPE_EXYNOS5250, + TYPE_EXYNOS5420, +}; Instead of using this kind of enumeration, I'd rather introduce a struct that describes the differences between all supported types. + +enum usb3phy_state { + STATE_OFF, + STATE_ON, +}; Hmm, isn't it a simple boolean value - false and true? + +struct usb3phy_config { + enum samsung_cpu_type cpu; + bool has_sclk_usbphy30; +}; Oh, you already have such struct, so there is even a bigger reason to drop the samsung_cpu_type enum above. + +struct usb3phy_instance { + char *label; + struct usb3phy_driver *drv; + struct phy *phy; + enum usb3phy_state state; + u32 clk; + unsigned long rate; +}; You seem to have just one instance in this driver. Do you really need this struct? + +struct usb3phy_driver { + struct device *dev; + void __iomem *reg_phy; + void __iomem *reg_isol; + struct clk *clk; + struct clk *sclk_usbphy30; + struct usb3phy_instance instance; Fields from that struct could be simply moved here. +}; + +/* + * exynos5_rate_to_clk() converts the supplied clock rate to
Re: [PATCH 2/4] dt: exynos5250: Enable support for generic USB 3.0 phy
Hi Vivek, On Thursday 31 of October 2013 13:15:42 Vivek Gautam wrote: Update device tree bindings for DWC3 controller and USB 3.0 phy present on Exynos 5250 SoC, to start using the phy driver based on generic phy framework. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- arch/arm/boot/dts/exynos5250.dtsi | 17 ++--- 1 files changed, 6 insertions(+), 11 deletions(-) diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi index bbac42a..31a6595 100644 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi @@ -472,22 +472,17 @@ compatible = synopsys,dwc3; reg = 0x1200 0x1; interrupts = 0 72 0; - usb-phy = usb2_phy usb3_phy; + phys = usb3_phy; + phy-names = usb3-phy; Does the driver already support generic PHY framework? Also it looks like originally it required two PHYs, while your patch changes it to just one. }; }; usb3_phy: usbphy@1210 { compatible = samsung,exynos5250-usb3phy; Hmm, this is not fully right. The new bindings should have new compatible value. This is also a comment to patch 1/1. This is because a device tree binding associated with specific compatible value is an ABI and should not be changed in a way that breaks backwards compatibility. Best regards, Tomasz -- 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/3] phy: Add new Exynos USB PHY driver
Hi Kamil, Please see my comments inline. On Tuesday 05 of November 2013 17:13:19 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 | 52 drivers/phy/Kconfig| 23 +- drivers/phy/Makefile |4 + drivers/phy/phy-exynos-usb2.c | 234 ++ drivers/phy/phy-exynos-usb2.h | 87 ++ drivers/phy/phy-exynos4210-usb2.c | 272 drivers/phy/phy-exynos4212-usb2.c | 324 7 files changed, 995 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt create mode 100644 drivers/phy/phy-exynos-usb2.c create mode 100644 drivers/phy/phy-exynos-usb2.h create mode 100644 drivers/phy/phy-exynos4210-usb2.c create mode 100644 drivers/phy/phy-exynos4212-usb2.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..c8fbc70 --- /dev/null +++ b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt @@ -0,0 +1,52 @@ +Samsung S5P/EXYNOS SoC series USB PHY I would not limit this only to S5P and newer series, especially that I'm planning to add support for S3C64xx using this framework. Instead I would call it Samsung SoC USB 1.1/2.0 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 You should consider using the PMU registers indirectly, as done in Leela Krisha Amudala's series[1] adding PMU register handling to the watchdog driver. [1] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/24652 +- #phy-cells : from the generic phy bindings, must be 1; +- clocks and clock-names: + - the phy clocks is required by the phy module + - other clocks are associated by name with their respective phys and + are used to determine the value of the clock settings register Those names should be explicitly listed. + +The second cell in the PHY specifier identifies the PHY, its meaning is It should say The first cell, I think? +compatible 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, + +Example: + +For Exynos 4412 (compatible with Exynos 4212): + +exynos_usbphy: exynos-usbphy@125B { nit: Node names should be generic and the label is slightly too long, so a better example would be: usbphy: phy@125B { + compatible = samsung,exynos4212-usbphy; + reg = 0x125B 0x100 0x10020704 0x0c 0x1001021c 0x4; + 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 a344f3d..bdf0fab 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -14,7 +14,7 @@ config GENERIC_PHY API by which phy drivers can create PHY using the phy framework and phy users can obtain reference to the PHY. All the users of this framework should select this config. - + Stray white space added. config PHY_EXYNOS_MIPI_VIDEO tristate S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver help @@ -51,4 +51,25 @@ config PHY_EXYNOS_DP_VIDEO help Support for Display Port PHY found on Samsung EXYNOS SoCs. +config PHY_EXYNOS_USB2 Wouldn't PHY_SAMSUNG_USB2 be better here? + tristate Samsung USB 2.0 PHY driver + help + Enable this to support Samsung USB phy helper driver for
Re: [PATCH v3 2/3] usb: ehci-s5p: Change to use phy provided by the generic phy framework
Hi Kamil, On Tuesday 05 of November 2013 17:13:20 Kamil Debski wrote: Change the phy provider used from the old usb phy specific to a new one using the generic phy framework. I believe that until Exynos5250 also gets converted to the new PHY driver, support for the old USB PHY API should remain in this driver. As for the patch itself, please see my comments inline. Signed-off-by: Kamil Debski k.deb...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/usb/host/ehci-exynos.c | 34 +++--- 1 file changed, 11 insertions(+), 23 deletions(-) This patch is changing a DT binding, but there is no update to the documentation. diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c index 8898c01..974001b 100644 --- a/drivers/usb/host/ehci-exynos.c +++ b/drivers/usb/host/ehci-exynos.c @@ -19,12 +19,12 @@ #include linux/module.h #include linux/of.h #include linux/of_gpio.h +#include linux/phy/phy.h #include linux/platform_device.h #include linux/usb/phy.h #include linux/usb/samsung_usb_phy.h #include linux/usb.h #include linux/usb/hcd.h -#include linux/usb/otg.h #include ehci.h @@ -44,8 +44,7 @@ static struct hc_driver __read_mostly exynos_ehci_hc_driver; struct exynos_ehci_hcd { struct clk *clk; - struct usb_phy *phy; - struct usb_otg *otg; + struct phy *phy; }; #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)-priv) @@ -75,7 +74,8 @@ static int exynos_ehci_probe(struct platform_device *pdev) struct usb_hcd *hcd; struct ehci_hcd *ehci; struct resource *res; - struct usb_phy *phy; + struct phy *phy; + const char *phy_name; int irq; int err; @@ -98,12 +98,12 @@ static int exynos_ehci_probe(struct platform_device *pdev) return -ENOMEM; } exynos_ehci = to_exynos_ehci(hcd); - if (of_device_is_compatible(pdev-dev.of_node, samsung,exynos5440-ehci)) goto skip_phy; - phy = devm_usb_get_phy(pdev-dev, USB_PHY_TYPE_USB2); + phy_name = of_get_property(pdev-dev.of_node, phy-names, NULL); + phy = devm_phy_get(pdev-dev, phy_name); This is definitely not the way you should parse PHY DT bindings. PHY names are supposed to be fixed in the binding for each compatible value, which means that you should call here devm_phy_get() with a static name. Moreover, this driver needs one PHY per port, not just one PHY, so the design needs to be completely changed and this patch is not really enough to correctly support USB 2.0 on Exynos. Best regards, Tomasz -- 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 3/3] usb: s3c-hsotg: Use the new Exynos USB phy driver with the generic phy framework
Hi Kamil, This patch is changing a DT binding, but you haven't updated relevant documentation. Please remember about it in next version. On Tuesday 05 of November 2013 17:13:21 Kamil Debski wrote: Change the used phy driver to the new Exynos USB phy driver that uses the generic phy framework. Signed-off-by: Kamil Debski k.deb...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/usb/gadget/s3c-hsotg.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c index bb31262..dc7f20c 100644 --- a/drivers/usb/gadget/s3c-hsotg.c +++ b/drivers/usb/gadget/s3c-hsotg.c @@ -31,6 +31,7 @@ #include linux/regulator/consumer.h #include linux/of.h #include linux/of_platform.h +#include linux/phy/phy.h #include linux/usb/ch9.h #include linux/usb/gadget.h @@ -162,7 +163,7 @@ struct s3c_hsotg_ep { struct s3c_hsotg { struct device*dev; struct usb_gadget_driver *driver; - struct usb_phy *phy; + struct phy *phy; struct s3c_hsotg_plat*plat; spinlock_t lock; @@ -2905,9 +2906,10 @@ static void s3c_hsotg_phy_enable(struct s3c_hsotg *hsotg) dev_dbg(hsotg-dev, pdev 0x%p\n, pdev); if (hsotg-phy) - usb_phy_init(hsotg-phy); + phy_power_on(hsotg-phy); else if (hsotg-plat-phy_init) hsotg-plat-phy_init(pdev, hsotg-plat-phy_type); + nit: Stray blank line. Best regards, Tomasz -- 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/3] phy: Add new Exynos USB PHY driver
Hi Kishon On Wednesday 06 of November 2013 13:48:13 Kishon Vijay Abraham I wrote: Hi, On Tuesday 05 November 2013 09:43 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 | 52 drivers/phy/Kconfig| 23 +- drivers/phy/Makefile |4 + drivers/phy/phy-exynos-usb2.c | 234 ++ drivers/phy/phy-exynos-usb2.h | 87 ++ drivers/phy/phy-exynos4210-usb2.c | 272 drivers/phy/phy-exynos4212-usb2.c | 324 7 files changed, 995 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt create mode 100644 drivers/phy/phy-exynos-usb2.c create mode 100644 drivers/phy/phy-exynos-usb2.h create mode 100644 drivers/phy/phy-exynos4210-usb2.c create mode 100644 drivers/phy/phy-exynos4212-usb2.c [snip] diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index a344f3d..bdf0fab 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig [snip] @@ -51,4 +51,25 @@ config PHY_EXYNOS_DP_VIDEO help Support for Display Port PHY found on Samsung EXYNOS SoCs. +config PHY_EXYNOS_USB2 + tristate Samsung USB 2.0 PHY driver + 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. I still think we can get rid of this helper driver and have a single driver for both PHY_EXYNOS4210_USB2 and PHY_EXYNOS4212_USB2. This helper driver is a really nice way to avoid code duplication, while still leaving the code clean and readable. All the Samsung USB 2.0 PHYs require exactly the same semantics (isolation, reference rate configuration, power up, power on), but each one has completely different layout of registers and bits inside registers. Making a big single driver would end up being identical to the old Exynos USB2PHY driver with a lot of if and switch statements inside most of functions, which is not only ugly but makes any further extension hard. In addition, this approach makes it possible to disable support for SoCs that are not needed in particular use cases, allowing smaller kernel images. + +config PHY_EXYNOS4210_USB2 + bool Support for Exynos 4210 + depends on PHY_EXYNOS_USB2 + depends on CPU_EXYNOS4210 + help + Enable USB PHY support for Exynos 4210 + +config PHY_EXYNOS4212_USB2 + bool Support for Exynos 4212 + depends on PHY_EXYNOS_USB2 + depends on (SOC_EXYNOS4212 || SOC_EXYNOS4412) + help + Enable USB PHY support for Exynos 4212 + endmenu [snip] +extern const struct usb2_phy_config exynos4210_usb2_phy_config; +extern const struct usb2_phy_config exynos4212_usb2_phy_config; + +static const struct of_device_id exynos_usb2_phy_of_match[] = { +#ifdef CONFIG_PHY_EXYNOS4210_USB2 I don't think you'll need #ifdef here. Anyways the driver data can be obtained using the appropriate compatible value in dt data no? Huh? This is not about driver data, but rather about the ability to match the driver only to devices that are actually supported with selected Kconfig options. Best regards, Tomasz -- 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/3] phy: Add new Exynos USB PHY driver
On Wednesday 06 of November 2013 18:20:36 Kishon Vijay Abraham I wrote: Hi, On Wednesday 06 November 2013 05:08 PM, Tomasz Figa wrote: Hi Kishon On Wednesday 06 of November 2013 13:48:13 Kishon Vijay Abraham I wrote: Hi, On Tuesday 05 November 2013 09:43 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 | 52 drivers/phy/Kconfig| 23 +- drivers/phy/Makefile |4 + drivers/phy/phy-exynos-usb2.c | 234 ++ drivers/phy/phy-exynos-usb2.h | 87 ++ drivers/phy/phy-exynos4210-usb2.c | 272 drivers/phy/phy-exynos4212-usb2.c | 324 7 files changed, 995 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt create mode 100644 drivers/phy/phy-exynos-usb2.c create mode 100644 drivers/phy/phy-exynos-usb2.h create mode 100644 drivers/phy/phy-exynos4210-usb2.c create mode 100644 drivers/phy/phy-exynos4212-usb2.c [snip] diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index a344f3d..bdf0fab 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig [snip] @@ -51,4 +51,25 @@ config PHY_EXYNOS_DP_VIDEO help Support for Display Port PHY found on Samsung EXYNOS SoCs. +config PHY_EXYNOS_USB2 + tristate Samsung USB 2.0 PHY driver + 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. I still think we can get rid of this helper driver and have a single driver for both PHY_EXYNOS4210_USB2 and PHY_EXYNOS4212_USB2. This helper driver is a really nice way to avoid code duplication, while still leaving the code clean and readable. All the Samsung USB 2.0 PHYs require exactly the same semantics (isolation, reference rate configuration, power up, power on), but each one has completely different layout of registers and bits inside registers. I just did a diff of registers in exynos 4210 and 4212 PHY drivers [1] and couldn't find that big a difference in register layout. Of course there are a few changes in HSIC bit fields and PHYFSEL but that's only minimal and could well be handled in a single driver. [1] - http://diffchecker.com/py3tp68m This is quite a lot of differences, especially including shifted bitfields... In addition there is another set of available PHYs and inter-dependencies between them. Making a big single driver would end up being identical to the old Exynos USB2PHY driver with a lot of if and switch statements inside most of functions, which is not only ugly but makes any further extension hard. Probably we shouldn't try to over design and just convert the old exynos usb2 phy driver to the generic phy framework to begin with? The old exynos USB2 PHY driver is incomplete and has very limited functionality. It needs a complete redesign to support remaining features and this is the reason we decided to develop a new driver from scratch. I believe the way Kamil's driver is designed is definitely the way to go with Samsung's USB2 PHY drivers, especially considering that support for more SoCs using the same framework will be added - S3C64xx, S5PV210, and Exynos5250. Best regards. Tomasz -- 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/3] phy: Add new Exynos USB PHY driver
Hi David, On Wednesday 06 of November 2013 13:03:45 David Laight wrote: I just did a diff of registers in exynos 4210 and 4212 PHY drivers [1] and couldn't find that big a difference in register layout. Of course there are a few changes in HSIC bit fields and PHYFSEL but that's only minimal and could well be handled in a single driver. [1] - http://diffchecker.com/py3tp68m This is quite a lot of differences, especially including shifted bitfields... In addition there is another set of available PHYs and inter-dependencies between them. Might be worth feeding both files through sed -e 's/_421[02]_/_421x_/' prior to doing the diff. And maybe changing the order of some definitions so they match. Then the actual differences will be more obvious. I have fed it already through my built-in sed when reading. Still despite many similarities, I think there is enough difference to justify having different callback functions for both, especially based on my experience with the old Exynos USB2 PHY driver in drivers/usb/phy, when trying to make it more complete. Best regards, Tomasz -- 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 RFC 1/4] phy: Add new Exynos5 USB 3.0 PHY driver
On Tuesday 05 of November 2013 12:50:18 Vivek Gautam wrote: Hi Kishon, On Mon, Nov 4, 2013 at 6:42 PM, Kishon Vijay Abraham I kis...@ti.com wrote: Hi, On Monday 04 November 2013 03:45 PM, Kamil Debski wrote: Hi Kishon, From: Kishon Vijay Abraham I [mailto:kis...@ti.com] Sent: Monday, November 04, 2013 7:55 AM Hi Vivek, On Thursday 31 October 2013 01:15 PM, 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. In Exynos, you have a single IP that supports both USB3 and USB2 PHY right? I think that needs to be mentioned here. As far as I know the IP is different. Ok. Sometime back Vivek was mentioning about a single IP for both USB3 and USB2. Thought it should be this driver. Anyway thanks for the clarification. Right Kishon, I had mentioned that Exynos5's dwc3 controller have a single IP for USB2 and USB3 phy. From what i see, on exynos5 systems the dwc3 controller uses a combo of usb 2 (utmi+) and usb 3 (pipe 3) phy (with base address starting 0x1210). I meant there is a single PHY used with the USB 3.0 controller (dwc3) and it is different from the PHY used with the USB 2.0 controller (s3c-hsotg aka dwc2). The USB 3.0 PHY and controller blocks also support USB 2.0 operation, though. So we were both right. ;) Best regards, Tomasz -- 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 RFC 1/4] phy: Add new Exynos5 USB 3.0 PHY driver
Hi Kishon, On Monday 04 of November 2013 12:24:42 Kishon Vijay Abraham I wrote: Hi Vivek, On Thursday 31 October 2013 01:15 PM, 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. In Exynos, you have a single IP that supports both USB3 and USB2 PHY right? I think that needs to be mentioned here. Nope. There are two separate, different IPs. Do you have separate registers that should be used for initializing/powerin_on/powering_off etc.. for usb2 phy and usb3 phy? If so, then you should model this driver as a single driver that supports two PHYs similar to what Sylwester has done before? Sylwester's MIPI PHY uses such model because it has a single register that controls both PHYs. Best regards, Tomasz -- 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 5/9] usb: gadget: s3c-hsotg: enable generic phy support
Hi Matt. On Friday 01 of November 2013 15:45:54 Matt Porter wrote: Adds support for the generic PHY subsystem. Generic PHY support is probed and then the driver falls back to checking for an old style USB PHY and pdata if not found. Signed-off-by: Matt Porter matt.por...@linaro.org --- drivers/usb/gadget/s3c-hsotg.c | 54 -- 1 file changed, 36 insertions(+), 18 deletions(-) Patches that convert the driver to generic PHY have been already posted by Kamil Debski, as a part of a series[1] adding generic PHY drivers for S5P and Exynos SoCs. After that series, there will be no need to support the usb_phy subsystem in this driver anymore. [1] http://www.mail-archive.com/linux-usb@vger.kernel.org/msg31189.html Best regards, Tomasz -- 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 1/9] phy: add phy_get_bus_width()/phy_set_bus_width() calls
Hi Matt, On Friday 01 of November 2013 15:45:50 Matt Porter wrote: This adds a pair of APIs that allows the generic PHY subsystem to provide information on the PHY bus width. The PHY provider driver may use phy_set_bus_width() to set the bus width that the PHY supports. The controller driver may then use phy_get_bus_width() to fetch the PHY bus width in order to properly configure the controller. I somehow does not like this. If we take this path for any further properties that we may need, we will end up with a lot of consumer specific properties stored in a PHY object having their own accessor functions. Since this is just an integration detail, what about simply adding this as a property in device tree node of the OTG controller (and pdata if considering non-DT support)? Another option would be some framework for retrieving arbitrary properties from the PHY, but I'm not really sure there is a need for such. Best regards, Tomasz -- 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 1/9] phy: add phy_get_bus_width()/phy_set_bus_width() calls
On Saturday 02 of November 2013 13:47:09 Matt Porter wrote: On Sat, Nov 02, 2013 at 10:46:55PM +0530, Kishon Vijay Abraham I wrote: Hi Tomasz, On Saturday 02 November 2013 06:44 PM, Tomasz Figa wrote: Hi Matt, On Friday 01 of November 2013 15:45:50 Matt Porter wrote: This adds a pair of APIs that allows the generic PHY subsystem to provide information on the PHY bus width. The PHY provider driver may use phy_set_bus_width() to set the bus width that the PHY supports. The controller driver may then use phy_get_bus_width() to fetch the PHY bus width in order to properly configure the controller. I somehow does not like this. If we take this path for any further properties that we may need, we will end up with a lot of consumer specific properties stored in a PHY object having their own accessor functions. Only after all of us feel that a property is *generic* enough, we allow it to be added in the PHY object. I also want to note that this was discussed over in another thread [2] where you did consider my rough stab at a more generic attribute accessor. It was definitely my first reaction as the way to do it like Tomasz has said. The specific accessors are more readable to me besides the justification you mention above. [2] http://lkml.indiana.edu/hypermail/linux/kernel/1310.3/00673.html Personally I like that version much better, but still it would need to be polished a bit. How I imagine such interface to be implemented: phy.h: struct phy { // ... const struct phy_attrs *attrs; // ... }; static inline const struct phy_attrs *phy_get_attrs(struct phy *phy) { return phy-attrs; }; phy driver: static const struct phy_attrs my_phy_attrs = { // ... }; static int my_phy_probe(...) { // ... phy = devm_phy_create_attrs(dev, ops, my_phy_attrs, NULL); // ... } phy consumer: // ... const struct phy_attrs *phy_attrs; phy_attrs = phy_get_attrs(phy); // ... Why I think it is better than what I've seen in this and previous instance of this thread? (in random order) a) Only the PHY driver can set the attrs. b) PHY consumer has access only to a const pointer. c) PHY attributes can be placed in a static struct inside a driver file, without the need to call any functions to set particular attributes. d) Can be extended with more attributes easily. Best regards, Tomasz -- 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 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-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: OHCI: Check the overrides pointer for NULL in ohci_init_driver()
A series of commit starting at 50a97e059b USB: OHCI: make ohci-exynos a separate driver and ending at b8ad5c3706 USB: OHCI: make ohci-pxa27x a separate driver introduced the concept of separate OHCI drivers for particular controllers. Respective drivers need to call ohci_init_driver() to initialize hc_driver struct with generic data and to certain extent with platform specific overrides through ohci_driver_overrides struct passed as second argument to this function. However the code does not check if the ohci_driver_overrides struct pointer is non-NULL, which leads for a NULL pointer dereference for drivers that do not need any overrides. This patch fixes the problem by dereferencing the passed pointer to ohci_driver_overrides struct only if it is non-NULL. Signed-off-by: Tomasz Figa tomasz.f...@gmail.com --- drivers/usb/host/ohci-hcd.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index 21d937a..8ada13f 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -1161,10 +1161,12 @@ void ohci_init_driver(struct hc_driver *drv, /* Copy the generic table to drv and then apply the overrides */ *drv = ohci_hc_driver; - drv-product_desc = over-product_desc; - drv-hcd_priv_size += over-extra_priv_size; - if (over-reset) - drv-reset = over-reset; + if (over) { + drv-product_desc = over-product_desc; + drv-hcd_priv_size += over-extra_priv_size; + if (over-reset) + drv-reset = over-reset; + } } EXPORT_SYMBOL_GPL(ohci_init_driver); -- 1.8.3.2 -- 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/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
Hi Felipe, On Tuesday 17 of September 2013 10:36:16 Felipe Balbi wrote: Hi, On Tue, Aug 27, 2013 at 01:27:48PM -0700, Julius Werner wrote: *Ping!* Are there still unanswered concerns left with this patch? I hope my prior mails could clear up why I think that the PMU register description in the device tree for a specific PHY is represents the hardware more accurately after my patch, and my analysis of the Exynos4 situation currently not being implemented (and therefore not needing to be addressed by this patch) was correct. Please let me know if you have further objections... and if not, could we agree to have this picked up somewhere? I need acks for DTS part... This patch breaks Exynos 4, so it's a NAK from me. Anyway, a new, completely redesigned PHY driver supporting most of the PHY features (as opposed to only the basic subset currently) developed by Kamil Debski should show up soon, so I don't think there is any reason to apply any patches to this old driver. Best regards, Tomasz -- 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] clk: samsung: pll: Use new registration method for PLL6552 and PLL6553
This patch modifies PLL6552 and PLL6553 clock drivers to use recently added common Samsung PLL registration method. Signed-off-by: Tomasz Figa tomasz.f...@gmail.com --- drivers/clk/samsung/clk-pll.c | 105 +- drivers/clk/samsung/clk-pll.h | 6 +-- 2 files changed, 13 insertions(+), 98 deletions(-) diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c index 0775554..7572d1d 100644 --- a/drivers/clk/samsung/clk-pll.c +++ b/drivers/clk/samsung/clk-pll.c @@ -441,9 +441,6 @@ struct clk * __init samsung_clk_register_pll46xx(const char *name, * PLL6552 Clock Type */ -#define PLL6552_LOCK_REG 0x00 -#define PLL6552_CON_REG0x0c - #define PLL6552_MDIV_MASK 0x3ff #define PLL6552_PDIV_MASK 0x3f #define PLL6552_SDIV_MASK 0x7 @@ -451,21 +448,14 @@ struct clk * __init samsung_clk_register_pll46xx(const char *name, #define PLL6552_PDIV_SHIFT 8 #define PLL6552_SDIV_SHIFT 0 -struct samsung_clk_pll6552 { - struct clk_hw hw; - void __iomem *reg_base; -}; - -#define to_clk_pll6552(_hw) container_of(_hw, struct samsung_clk_pll6552, hw) - static unsigned long samsung_pll6552_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) { - struct samsung_clk_pll6552 *pll = to_clk_pll6552(hw); + struct samsung_clk_pll *pll = to_clk_pll(hw); u32 mdiv, pdiv, sdiv, pll_con; u64 fvco = parent_rate; - pll_con = __raw_readl(pll-reg_base + PLL6552_CON_REG); + pll_con = __raw_readl(pll-con_reg); mdiv = (pll_con PLL6552_MDIV_SHIFT) PLL6552_MDIV_MASK; pdiv = (pll_con PLL6552_PDIV_SHIFT) PLL6552_PDIV_MASK; sdiv = (pll_con PLL6552_SDIV_SHIFT) PLL6552_SDIV_MASK; @@ -480,48 +470,10 @@ static const struct clk_ops samsung_pll6552_clk_ops = { .recalc_rate = samsung_pll6552_recalc_rate, }; -struct clk * __init samsung_clk_register_pll6552(const char *name, - const char *pname, void __iomem *base) -{ - struct samsung_clk_pll6552 *pll; - struct clk *clk; - struct clk_init_data init; - - pll = kzalloc(sizeof(*pll), GFP_KERNEL); - if (!pll) { - pr_err(%s: could not allocate pll clk %s\n, __func__, name); - return NULL; - } - - init.name = name; - init.ops = samsung_pll6552_clk_ops; - init.parent_names = pname; - init.num_parents = 1; - - pll-hw.init = init; - pll-reg_base = base; - - clk = clk_register(NULL, pll-hw); - if (IS_ERR(clk)) { - pr_err(%s: failed to register pll clock %s\n, __func__, - name); - kfree(pll); - } - - if (clk_register_clkdev(clk, name, NULL)) - pr_err(%s: failed to register lookup for %s, __func__, name); - - return clk; -} - /* * PLL6553 Clock Type */ -#define PLL6553_LOCK_REG 0x00 -#define PLL6553_CON0_REG 0x0c -#define PLL6553_CON1_REG 0x10 - #define PLL6553_MDIV_MASK 0xff #define PLL6553_PDIV_MASK 0x3f #define PLL6553_SDIV_MASK 0x7 @@ -531,22 +483,15 @@ struct clk * __init samsung_clk_register_pll6552(const char *name, #define PLL6553_SDIV_SHIFT 0 #define PLL6553_KDIV_SHIFT 0 -struct samsung_clk_pll6553 { - struct clk_hw hw; - void __iomem *reg_base; -}; - -#define to_clk_pll6553(_hw) container_of(_hw, struct samsung_clk_pll6553, hw) - static unsigned long samsung_pll6553_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) { - struct samsung_clk_pll6553 *pll = to_clk_pll6553(hw); + struct samsung_clk_pll *pll = to_clk_pll(hw); u32 mdiv, pdiv, sdiv, kdiv, pll_con0, pll_con1; u64 fvco = parent_rate; - pll_con0 = __raw_readl(pll-reg_base + PLL6553_CON0_REG); - pll_con1 = __raw_readl(pll-reg_base + PLL6553_CON1_REG); + pll_con0 = __raw_readl(pll-con_reg); + pll_con1 = __raw_readl(pll-con_reg + 0x4); mdiv = (pll_con0 PLL6553_MDIV_SHIFT) PLL6553_MDIV_MASK; pdiv = (pll_con0 PLL6553_PDIV_SHIFT) PLL6553_PDIV_MASK; sdiv = (pll_con0 PLL6553_SDIV_SHIFT) PLL6553_SDIV_MASK; @@ -563,40 +508,6 @@ static const struct clk_ops samsung_pll6553_clk_ops = { .recalc_rate = samsung_pll6553_recalc_rate, }; -struct clk * __init samsung_clk_register_pll6553(const char *name, - const char *pname, void __iomem *base) -{ - struct samsung_clk_pll6553 *pll; - struct clk *clk; - struct clk_init_data init; - - pll = kzalloc(sizeof(*pll), GFP_KERNEL); - if (!pll) { - pr_err(%s: could not allocate pll clk %s\n, __func__, name); - return NULL; - } - - init.name = name; - init.ops = samsung_pll6553_clk_ops; - init.parent_names
Re: [PATCH V5 6/6] USB: OHCI: make ohci-s3c2410 a separate driver
); +} +module_init(ohci_s3c2410_init); + +static void __exit ohci_s3c2410_cleanup(void) +{ + platform_driver_unregister(ohci_hcd_s3c2410_driver); +} +module_exit(ohci_s3c2410_cleanup); + +MODULE_DESCRIPTION(DRIVER_DESC); +MODULE_LICENSE(GPL); MODULE_ALIAS(platform:s3c2410-ohci); Looks good. Reviewed-by: Tomasz Figa t.f...@samsung.com Best regards, Tomasz -- 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 0/8] Common Clock Framework support for Samsung S3C64xx
Hi Mike, On Monday 05 of August 2013 11:06:25 Mike Turquette wrote: Quoting Kukjin Kim (2013-08-05 10:01:36) On 07/23/13 08:49, Tomasz Figa wrote: This series is an attempt to move clock support on Samsung S3C64xx SoCs to Common Clock Framework. First, support for PLL types present on S3C64xx SoCs is added to Samsung Common Clock Framework driver. Then the main clock driver for mentioned SoCs is introduced. Further patches contain fixes for drivers to make them compliant with CCF semantics, migration of platform code to use the new clock driver and removal of old clock management code. Depends on: - [PATCH v4 00/20] Samsung PWM support cleanup http://thread.gmane.org/gmane.linux.kernel.samsung-soc/20856 On S3C6410-based Tiny6410 board (Mini6410-compatible): Tested-by: Tomasz Figatomasz.f...@gmail.com For v1: Acked-by: Mike Turquettemturque...@linaro.org Changes since v1: - added patch for read-only muxes, - exported configurable muxes and dividers, - defined mout_syncmux as read-only mux, - in DT-enabled case fixed-clock binding is used to define external clocks. Tomasz Figa (8): clk: mux: Add support for read-only muxes. clk: samsung: pll: Add support for PLL6552 and PLL6553 clk: samsung: Add clock driver for S3C64xx SoCs ARM: SAMSUNG: Add soc_is_s3c6400/s3c6410 macros ARM: s3c64xx: dma: Use clk_prepare_enable/clk_disable_unprepare usb: host: ohci-s3c2410 Use clk_prepare_enable/clk_disable_unprepare ARM: s3c64xx: Migrate clock handling to Common Clock Framework ARM: s3c64xx: Remove old clock management code .../bindings/clock/samsung,s3c64xx-clock.txt | 77 ++ arch/arm/Kconfig |2 +- arch/arm/mach-s3c64xx/Makefile |2 +- arch/arm/mach-s3c64xx/clock.c | 1007 arch/arm/mach-s3c64xx/common.c | 21 +- arch/arm/mach-s3c64xx/common.h | 12 +- arch/arm/mach-s3c64xx/dma.c|4 +- arch/arm/mach-s3c64xx/include/mach/regs-clock.h| 132 +-- arch/arm/mach-s3c64xx/mach-anw6410.c |2 +- arch/arm/mach-s3c64xx/mach-crag6410.c |2 +- arch/arm/mach-s3c64xx/mach-hmt.c |2 +- arch/arm/mach-s3c64xx/mach-mini6410.c |2 +- arch/arm/mach-s3c64xx/mach-ncp.c |2 +- arch/arm/mach-s3c64xx/mach-smartq.c| 11 +- arch/arm/mach-s3c64xx/mach-smdk6400.c |2 +- arch/arm/mach-s3c64xx/mach-smdk6410.c |2 +- arch/arm/mach-s3c64xx/pm.c | 21 - arch/arm/mach-s3c64xx/s3c6400.c|6 - arch/arm/mach-s3c64xx/s3c6410.c|7 - arch/arm/plat-samsung/include/plat/cpu.h |4 + drivers/clk/clk-mux.c | 10 +- drivers/clk/samsung/Makefile |1 + drivers/clk/samsung/clk-pll.c | 160 drivers/clk/samsung/clk-pll.h |4 + drivers/clk/samsung/clk-s3c64xx.c | 465 + drivers/usb/host/ohci-s3c2410.c|8 +- include/dt-bindings/clock/samsung,s3c64xx-clock.h | 178 include/linux/clk-provider.h |2 + 28 files changed, 943 insertions(+), 1205 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/samsung,s3c64xx-clock.txt delete mode 100644 arch/arm/mach-s3c64xx/clock.c create mode 100644 drivers/clk/samsung/clk-s3c64xx.c create mode 100644 include/dt-bindings/clock/samsung,s3c64xx-clock.h Basically, this series looks good to me, but I'm not sure how this should be handled because of dependency with PWM cleanup and clk stuff in clk tree now... Patches 1-3 can go into the clk tree. 4-6 should go through their respective trees. It looks like version 2 of patch 2/8 has been applied by mistake, breaking compilation (and operation) of the clock driver added in patch 3/8. Could you please fix this up? Thanks in advance. Best regards, Tomasz If you want I can take 7 8 through the clk tree. Alternatively I can provide patches 1-3 in a separate stable topic branch for you to pull in as a dependency. We'll both merge that stable topic branch into our trees and you can make a note of it for the arm-soc folks. Regards, Mike - Kukjin -- 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 signature.asc Description: This is a digitally signed message part.
Re: [PATCH v2 0/8] Common Clock Framework support for Samsung S3C64xx
On Friday 16 of August 2013 14:02:03 Mike Turquette wrote: Quoting Tomasz Figa (2013-08-16 03:44:44) Hi Mike, On Monday 05 of August 2013 11:06:25 Mike Turquette wrote: Quoting Kukjin Kim (2013-08-05 10:01:36) On 07/23/13 08:49, Tomasz Figa wrote: This series is an attempt to move clock support on Samsung S3C64xx SoCs to Common Clock Framework. First, support for PLL types present on S3C64xx SoCs is added to Samsung Common Clock Framework driver. Then the main clock driver for mentioned SoCs is introduced. Further patches contain fixes for drivers to make them compliant with CCF semantics, migration of platform code to use the new clock driver and removal of old clock management code. Depends on: - [PATCH v4 00/20] Samsung PWM support cleanup http://thread.gmane.org/gmane.linux.kernel.samsung-soc/20856 On S3C6410-based Tiny6410 board (Mini6410-compatible): Tested-by: Tomasz Figatomasz.f...@gmail.com For v1: Acked-by: Mike Turquettemturque...@linaro.org Changes since v1: - added patch for read-only muxes, - exported configurable muxes and dividers, - defined mout_syncmux as read-only mux, - in DT-enabled case fixed-clock binding is used to define external clocks. Tomasz Figa (8): clk: mux: Add support for read-only muxes. clk: samsung: pll: Add support for PLL6552 and PLL6553 clk: samsung: Add clock driver for S3C64xx SoCs ARM: SAMSUNG: Add soc_is_s3c6400/s3c6410 macros ARM: s3c64xx: dma: Use clk_prepare_enable/clk_disable_unprepare usb: host: ohci-s3c2410 Use clk_prepare_enable/clk_disable_unprepare ARM: s3c64xx: Migrate clock handling to Common Clock Framework ARM: s3c64xx: Remove old clock management code .../bindings/clock/samsung,s3c64xx-clock.txt | 77 ++ arch/arm/Kconfig |2 +- arch/arm/mach-s3c64xx/Makefile |2 +- arch/arm/mach-s3c64xx/clock.c | 1007 arch/arm/mach-s3c64xx/common.c | 21 +- arch/arm/mach-s3c64xx/common.h | 12 +- arch/arm/mach-s3c64xx/dma.c|4 +- arch/arm/mach-s3c64xx/include/mach/regs-clock.h| 132 +-- arch/arm/mach-s3c64xx/mach-anw6410.c |2 +- arch/arm/mach-s3c64xx/mach-crag6410.c |2 +- arch/arm/mach-s3c64xx/mach-hmt.c |2 +- arch/arm/mach-s3c64xx/mach-mini6410.c |2 +- arch/arm/mach-s3c64xx/mach-ncp.c |2 +- arch/arm/mach-s3c64xx/mach-smartq.c| 11 +- arch/arm/mach-s3c64xx/mach-smdk6400.c |2 +- arch/arm/mach-s3c64xx/mach-smdk6410.c |2 +- arch/arm/mach-s3c64xx/pm.c | 21 - arch/arm/mach-s3c64xx/s3c6400.c|6 - arch/arm/mach-s3c64xx/s3c6410.c|7 - arch/arm/plat-samsung/include/plat/cpu.h |4 + drivers/clk/clk-mux.c | 10 +- drivers/clk/samsung/Makefile |1 + drivers/clk/samsung/clk-pll.c | 160 drivers/clk/samsung/clk-pll.h |4 + drivers/clk/samsung/clk-s3c64xx.c | 465 + drivers/usb/host/ohci-s3c2410.c|8 +- include/dt-bindings/clock/samsung,s3c64xx-clock.h | 178 include/linux/clk-provider.h |2 + 28 files changed, 943 insertions(+), 1205 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/samsung,s3c64xx-clock. txt delete mode 100644 arch/arm/mach-s3c64xx/clock.c create mode 100644 drivers/clk/samsung/clk-s3c64xx.c create mode 100644 include/dt-bindings/clock/samsung,s3c64xx-clock.h Basically, this series looks good to me, but I'm not sure how this should be handled because of dependency with PWM cleanup and clk stuff in clk tree now... Patches 1-3 can go into the clk tree. 4-6 should go through their respective trees. It looks like version 2 of patch 2/8 has been applied by mistake, breaking compilation (and operation) of the clock driver added in patch 3/8. Ugh. My mistake. Happens. Thanks for fast response. Could you please fix this up? Thanks in advance. This is a little tricky since I published the clk-next-s3c64xx branch as a stable branch for Samsung which I think has been merged to the Samsung tree already. Right, this somewhat
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 13 of August 2013 16:14:44 Kishon Vijay Abraham I wrote: Hi, On Wednesday 31 July 2013 11:45 AM, Felipe Balbi wrote: Hi, 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. What about slightly altering the concept of v9 to pass a pointer to struct device instead of device name inside phy_init_data? Best regards, Tomasz -- 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
On Wednesday 14 of August 2013 00:19:28 Sylwester Nawrocki wrote: 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. Well
Re: [PATCH V4 6/6] USB: OHCI: make ohci-s3c2410 a separate driver
Hi Manjunath, On Saturday 10 of August 2013 13:07:36 Manjunath Goudar wrote: Separate the Samsung OHCI S3C host controller driver from ohci-hcd host code so that it can be built as a separate driver module. This work is part of enabling multi-platform kernels on ARM; it would be nice to have in 3.12. Signed-off-by: Manjunath Goudar manjunath.gou...@linaro.org Signed-off-by: Deepak Saxena dsax...@linaro.org Acked-by: Alan Stern st...@rowland.harvard.edu Cc: Arnd Bergmann a...@arndb.de Cc: Greg KH g...@kroah.com Cc: linux-usb@vger.kernel.org V2: -Set non-standard fields in ohci_s3c2410_hc_driver manually, rather than relying on an expanded struct ohci_driver_overrides. -Save orig_ohci_hub_control and orig_ohci_hub_status_data rather than relying on ohci_hub_control and hub_status_data being exported. V3: -Kconfig wrong parentheses discription fixed. -ohci_setup() has been removed because it is called in .reset member of the ohci_hc_driver structure. V4: - Removed extra space before the '='. - Moved /* forward definitions */ line before the declarations of functions. --- drivers/usb/host/Kconfig|8 +++ drivers/usb/host/Makefile |1 + drivers/usb/host/ohci-hcd.c | 18 -- drivers/usb/host/ohci-s3c2410.c | 128 +-- 4 files changed, 66 insertions(+), 89 deletions(-) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 693560a..795d14d 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -390,6 +390,14 @@ config USB_OHCI_HCD_SPEAR Enables support for the on-chip OHCI controller on ST SPEAr chips. +config USB_OHCI_HCD_S3C +tristate Support for S3C on-chip OHCI USB controller As far as I remember, you were supposed to keep the original name of this driver, as I suggested in previous iteration of this patch and you agreed. S3C is totally confusing when it's about Samsung SoC naming. So here the config entry would be USB_OHCI_HCD_S3C2410 and I would call it OHCI support for Samsung S3C24xx/S3C64xx SoC series. +depends on USB_OHCI_HCD (ARCH_S3C24XX || ARCH_S3C64XX) +default y +---help--- + Enables support for the on-chip OHCI controller on + S3C chips. S3C24xx/S3C64xx chips + config USB_OHCI_HCD_AT91 tristate Support for Atmel on-chip OHCI USB controller depends on USB_OHCI_HCD ARCH_AT91 diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile index a0ac663..d3e9e66 100644 --- a/drivers/usb/host/Makefile +++ b/drivers/usb/host/Makefile @@ -49,6 +49,7 @@ obj-$(CONFIG_USB_OHCI_HCD_OMAP1)+= ohci-omap.o obj-$(CONFIG_USB_OHCI_HCD_OMAP3) += ohci-omap3.o obj-$(CONFIG_USB_OHCI_HCD_SPEAR) += ohci-spear.o obj-$(CONFIG_USB_OHCI_HCD_AT91) += ohci-at91.o +obj-$(CONFIG_USB_OHCI_HCD_S3C) += ohci-s3c2410.o CONFIG_USB_OHCI_HCD_S3C2410 obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o obj-$(CONFIG_USB_FHCI_HCD) += fhci.o diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index b48c892..b69a49e 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -1177,11 +1177,6 @@ MODULE_LICENSE (GPL); #define SA_DRIVERohci_hcd_sa_driver #endif -#if defined(CONFIG_ARCH_S3C24XX) || defined(CONFIG_ARCH_S3C64XX) -#include ohci-s3c2410.c -#define S3C2410_PLATFORM_DRIVER ohci_hcd_s3c2410_driver -#endif - #if defined(CONFIG_PXA27x) || defined(CONFIG_PXA3xx) #include ohci-pxa27x.c #define PLATFORM_DRIVER ohci_hcd_pxa27x_driver @@ -1293,12 +1288,6 @@ static int __init ohci_hcd_mod_init(void) goto error_tmio; #endif -#ifdef S3C2410_PLATFORM_DRIVER - retval = platform_driver_register(S3C2410_PLATFORM_DRIVER); - if (retval 0) - goto error_s3c2410; -#endif - #ifdef EP93XX_PLATFORM_DRIVER retval = platform_driver_register(EP93XX_PLATFORM_DRIVER); if (retval 0) @@ -1332,10 +1321,6 @@ static int __init ohci_hcd_mod_init(void) platform_driver_unregister(EP93XX_PLATFORM_DRIVER); error_ep93xx: #endif -#ifdef S3C2410_PLATFORM_DRIVER - platform_driver_unregister(S3C2410_PLATFORM_DRIVER); - error_s3c2410: -#endif #ifdef TMIO_OHCI_DRIVER platform_driver_unregister(TMIO_OHCI_DRIVER); error_tmio: @@ -1382,9 +1367,6 @@ static void __exit ohci_hcd_mod_exit(void) #ifdef EP93XX_PLATFORM_DRIVER platform_driver_unregister(EP93XX_PLATFORM_DRIVER); #endif -#ifdef S3C2410_PLATFORM_DRIVER - platform_driver_unregister(S3C2410_PLATFORM_DRIVER); -#endif #ifdef TMIO_OHCI_DRIVER platform_driver_unregister(TMIO_OHCI_DRIVER); #endif diff --git a/drivers/usb/host/ohci-s3c2410.c b/drivers/usb/host/ohci-s3c2410.c index e125770..48b5948 100644 --- a/drivers/usb/host/ohci-s3c2410.c +++ b/drivers/usb/host/ohci-s3c2410.c
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
Hi Julius, On Thursday 08 of August 2013 11:06:54 Julius Werner wrote: I'm not sure I understand. The old documentation referred to the USBDEVICE_PHY_CONTROL and USBHOST_PHY_CONTROL registers for a phy, and your new version only refers to (usb device) PHY_CONTROL. Regardless of multiple phys, you're suggesting that we describe less of each phy. That seems like taking away usable information. Unless I've misunderstood? Well that's just the thing that's confusing right now, and which I am trying to fix: every PHY is either DEVICE or HOST and thus has only one PMU register. The current code describes the PMU register space for all PHYs on the system in the DT entry of every PHY and then calculates which register to use with hardcoded offsets. I think it makes much more sense if every PHY only describes its own register and doesn't need to do address arithmetic later on. As Vivek said there is one exception in an old Exynos4, Not that old yet. :) but that is currently not implemented in the upstream kernel anyway Sorry, I don't understand what is not implemented. Without your patch, the PHY driver handles both PMU registers of Exynos4. Best regards, Tomasz , and if it ever will be it's still much easier to special case one weird chip than to have a super complicated and confusing mechanism for all of them. -- To unsubscribe from this list: send the line unsubscribe devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.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 v2 0/8] Common Clock Framework support for Samsung S3C64xx
On Tuesday 06 of August 2013 12:47:51 Mike Turquette wrote: Quoting Tomasz Figa (2013-08-05 16:42:16) On Monday 05 of August 2013 12:02:16 Mike Turquette wrote: Quoting Kukjin Kim (2013-08-05 11:13:55) On 08/06/13 03:06, Mike Turquette wrote: Quoting Kukjin Kim (2013-08-05 10:01:36) On 07/23/13 08:49, Tomasz Figa wrote: This series is an attempt to move clock support on Samsung S3C64xx SoCs to Common Clock Framework. First, support for PLL types present on S3C64xx SoCs is added to Samsung Common Clock Framework driver. Then the main clock driver for mentioned SoCs is introduced. Further patches contain fixes for drivers to make them compliant with CCF semantics, migration of platform code to use the new clock driver and removal of old clock management code. Depends on: - [PATCH v4 00/20] Samsung PWM support cleanup http://thread.gmane.org/gmane.linux.kernel.samsung-soc/20 856 On S3C6410-based Tiny6410 board (Mini6410-compatible): Tested-by: Tomasz Figatomasz.f...@gmail.com For v1: Acked-by: Mike Turquettemturque...@linaro.org Changes since v1: - added patch for read-only muxes, - exported configurable muxes and dividers, - defined mout_syncmux as read-only mux, - in DT-enabled case fixed-clock binding is used to define external clocks. Tomasz Figa (8): clk: mux: Add support for read-only muxes. clk: samsung: pll: Add support for PLL6552 and PLL6553 clk: samsung: Add clock driver for S3C64xx SoCs ARM: SAMSUNG: Add soc_is_s3c6400/s3c6410 macros ARM: s3c64xx: dma: Use clk_prepare_enable/clk_disable_unprepare usb: host: ohci-s3c2410 Use clk_prepare_enable/clk_disable_unprepare ARM: s3c64xx: Migrate clock handling to Common Clock Framework ARM: s3c64xx: Remove old clock management code .../bindings/clock/samsung,s3c64xx-clock.txt | 77 ++ arch/arm/Kconfig |2 +- arch/arm/mach-s3c64xx/Makefile |2 +- arch/arm/mach-s3c64xx/clock.c | 1007 arch/arm/mach-s3c64xx/common.c | 21 +- arch/arm/mach-s3c64xx/common.h | 12 +- arch/arm/mach-s3c64xx/dma.c|4 +- arch/arm/mach-s3c64xx/include/mach/regs-clock.h| 132 +-- arch/arm/mach-s3c64xx/mach-anw6410.c |2 +- arch/arm/mach-s3c64xx/mach-crag6410.c |2 +- arch/arm/mach-s3c64xx/mach-hmt.c |2 +- arch/arm/mach-s3c64xx/mach-mini6410.c |2 +- arch/arm/mach-s3c64xx/mach-ncp.c |2 +- arch/arm/mach-s3c64xx/mach-smartq.c| 11 +- arch/arm/mach-s3c64xx/mach-smdk6400.c |2 +- arch/arm/mach-s3c64xx/mach-smdk6410.c |2 +- arch/arm/mach-s3c64xx/pm.c | 21 - arch/arm/mach-s3c64xx/s3c6400.c|6 - arch/arm/mach-s3c64xx/s3c6410.c|7 - arch/arm/plat-samsung/include/plat/cpu.h |4 + drivers/clk/clk-mux.c | 10 +- drivers/clk/samsung/Makefile |1 + drivers/clk/samsung/clk-pll.c | 160 drivers/clk/samsung/clk-pll.h |4 + drivers/clk/samsung/clk-s3c64xx.c | 465 + drivers/usb/host/ohci-s3c2410.c|8 +- include/dt-bindings/clock/samsung,s3c64xx-clock.h | 178 include/linux/clk-provider.h |2 + 28 files changed, 943 insertions(+), 1205 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/samsung,s3c64xx-clo ck. txt delete mode 100644 arch/arm/mach-s3c64xx/clock.c create mode 100644 drivers/clk/samsung/clk-s3c64xx.c create mode 100644 include/dt-bindings/clock/samsung,s3c64xx-clock.h Basically, this series looks good to me, but I'm not sure how this should be handled because of dependency with PWM cleanup and clk stuff in clk tree now... Patches 1-3 can go into the clk tree. 4-6 should go through their respective trees. If you want I can take 7 8 through the clk tree. Alternatively I can provide patches 1-3
Re: [PATCH v2 0/8] Common Clock Framework support for Samsung S3C64xx
On Wednesday 07 of August 2013 07:11:40 Kukjin Kim wrote: On 08/07/13 07:06, Tomasz Figa wrote: On Tuesday 06 of August 2013 12:47:51 Mike Turquette wrote: Quoting Tomasz Figa (2013-08-05 16:42:16) On Monday 05 of August 2013 12:02:16 Mike Turquette wrote: Quoting Kukjin Kim (2013-08-05 11:13:55) On 08/06/13 03:06, Mike Turquette wrote: Quoting Kukjin Kim (2013-08-05 10:01:36) On 07/23/13 08:49, Tomasz Figa wrote: This series is an attempt to move clock support on Samsung S3C64xx SoCs to Common Clock Framework. First, support for PLL types present on S3C64xx SoCs is added to Samsung Common Clock Framework driver. Then the main clock driver for mentioned SoCs is introduced. Further patches contain fixes for drivers to make them compliant with CCF semantics, migration of platform code to use the new clock driver and removal of old clock management code. Depends on: - [PATCH v4 00/20] Samsung PWM support cleanup http://thread.gmane.org/gmane.linux.kernel.samsung-soc/20 856 On S3C6410-based Tiny6410 board (Mini6410-compatible): Tested-by: Tomasz Figatomasz.f...@gmail.com For v1: Acked-by: Mike Turquettemturque...@linaro.org Changes since v1: - added patch for read-only muxes, - exported configurable muxes and dividers, - defined mout_syncmux as read-only mux, - in DT-enabled case fixed-clock binding is used to define external clocks. Tomasz Figa (8): clk: mux: Add support for read-only muxes. clk: samsung: pll: Add support for PLL6552 and PLL6553 clk: samsung: Add clock driver for S3C64xx SoCs ARM: SAMSUNG: Add soc_is_s3c6400/s3c6410 macros ARM: s3c64xx: dma: Use clk_prepare_enable/clk_disable_unprepare usb: host: ohci-s3c2410 Use clk_prepare_enable/clk_disable_unprepare ARM: s3c64xx: Migrate clock handling to Common Clock Framework ARM: s3c64xx: Remove old clock management code .../bindings/clock/samsung,s3c64xx-clock.txt | 77 ++ arch/arm/Kconfig |2 +- arch/arm/mach-s3c64xx/Makefile |2 +- arch/arm/mach-s3c64xx/clock.c | 1007 arch/arm/mach-s3c64xx/common.c | 21 +- arch/arm/mach-s3c64xx/common.h | 12 +- arch/arm/mach-s3c64xx/dma.c|4 +- arch/arm/mach-s3c64xx/include/mach/regs-clock.h| 132 +-- arch/arm/mach-s3c64xx/mach-anw6410.c |2 +- arch/arm/mach-s3c64xx/mach-crag6410.c |2 +- arch/arm/mach-s3c64xx/mach-hmt.c |2 +- arch/arm/mach-s3c64xx/mach-mini6410.c |2 +- arch/arm/mach-s3c64xx/mach-ncp.c |2 +- arch/arm/mach-s3c64xx/mach-smartq.c| 11 +- arch/arm/mach-s3c64xx/mach-smdk6400.c |2 +- arch/arm/mach-s3c64xx/mach-smdk6410.c |2 +- arch/arm/mach-s3c64xx/pm.c | 21 - arch/arm/mach-s3c64xx/s3c6400.c|6 - arch/arm/mach-s3c64xx/s3c6410.c|7 - arch/arm/plat-samsung/include/plat/cpu.h |4 + drivers/clk/clk-mux.c | 10 +- drivers/clk/samsung/Makefile |1 + drivers/clk/samsung/clk-pll.c | 160 drivers/clk/samsung/clk-pll.h |4 + drivers/clk/samsung/clk-s3c64xx.c | 465 + drivers/usb/host/ohci-s3c2410.c|8 +- include/dt-bindings/clock/samsung,s3c64xx-clock.h | 178 include/linux/clk-provider.h |2 + 28 files changed, 943 insertions(+), 1205 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/samsung,s3c64xx-clo ck. txt delete mode 100644 arch/arm/mach-s3c64xx/clock.c create mode 100644 drivers/clk/samsung/clk-s3c64xx.c create mode 100644 include/dt-bindings/clock/samsung,s3c64xx-clock.h Basically, this series looks good to me, but I'm not sure how this should be handled because of dependency with PWM cleanup and clk stuff in clk tree now... Patches 1-3 can go into the clk tree. 4-6 should go through their respective trees. If you want I can take 7 8 through the clk tree. Alternatively I can provide patches 1-3 in a separate stable topic branch for you to pull in as a dependency. We'll both merge that stable topic branch into our trees and you can make a note of it for the arm-soc folks. Thanks
Re: [PATCH v2 0/8] Common Clock Framework support for Samsung S3C64xx
On Monday 05 of August 2013 12:02:16 Mike Turquette wrote: Quoting Kukjin Kim (2013-08-05 11:13:55) On 08/06/13 03:06, Mike Turquette wrote: Quoting Kukjin Kim (2013-08-05 10:01:36) On 07/23/13 08:49, Tomasz Figa wrote: This series is an attempt to move clock support on Samsung S3C64xx SoCs to Common Clock Framework. First, support for PLL types present on S3C64xx SoCs is added to Samsung Common Clock Framework driver. Then the main clock driver for mentioned SoCs is introduced. Further patches contain fixes for drivers to make them compliant with CCF semantics, migration of platform code to use the new clock driver and removal of old clock management code. Depends on: - [PATCH v4 00/20] Samsung PWM support cleanup http://thread.gmane.org/gmane.linux.kernel.samsung-soc/20856 On S3C6410-based Tiny6410 board (Mini6410-compatible): Tested-by: Tomasz Figatomasz.f...@gmail.com For v1: Acked-by: Mike Turquettemturque...@linaro.org Changes since v1: - added patch for read-only muxes, - exported configurable muxes and dividers, - defined mout_syncmux as read-only mux, - in DT-enabled case fixed-clock binding is used to define external clocks. Tomasz Figa (8): clk: mux: Add support for read-only muxes. clk: samsung: pll: Add support for PLL6552 and PLL6553 clk: samsung: Add clock driver for S3C64xx SoCs ARM: SAMSUNG: Add soc_is_s3c6400/s3c6410 macros ARM: s3c64xx: dma: Use clk_prepare_enable/clk_disable_unprepare usb: host: ohci-s3c2410 Use clk_prepare_enable/clk_disable_unprepare ARM: s3c64xx: Migrate clock handling to Common Clock Framework ARM: s3c64xx: Remove old clock management code .../bindings/clock/samsung,s3c64xx-clock.txt | 77 ++ arch/arm/Kconfig |2 +- arch/arm/mach-s3c64xx/Makefile |2 +- arch/arm/mach-s3c64xx/clock.c | 1007 arch/arm/mach-s3c64xx/common.c | 21 +- arch/arm/mach-s3c64xx/common.h | 12 +- arch/arm/mach-s3c64xx/dma.c|4 +- arch/arm/mach-s3c64xx/include/mach/regs-clock.h| 132 +-- arch/arm/mach-s3c64xx/mach-anw6410.c |2 +- arch/arm/mach-s3c64xx/mach-crag6410.c |2 +- arch/arm/mach-s3c64xx/mach-hmt.c |2 +- arch/arm/mach-s3c64xx/mach-mini6410.c |2 +- arch/arm/mach-s3c64xx/mach-ncp.c |2 +- arch/arm/mach-s3c64xx/mach-smartq.c| 11 +- arch/arm/mach-s3c64xx/mach-smdk6400.c |2 +- arch/arm/mach-s3c64xx/mach-smdk6410.c |2 +- arch/arm/mach-s3c64xx/pm.c | 21 - arch/arm/mach-s3c64xx/s3c6400.c|6 - arch/arm/mach-s3c64xx/s3c6410.c|7 - arch/arm/plat-samsung/include/plat/cpu.h |4 + drivers/clk/clk-mux.c | 10 +- drivers/clk/samsung/Makefile |1 + drivers/clk/samsung/clk-pll.c | 160 drivers/clk/samsung/clk-pll.h |4 + drivers/clk/samsung/clk-s3c64xx.c | 465 + drivers/usb/host/ohci-s3c2410.c|8 +- include/dt-bindings/clock/samsung,s3c64xx-clock.h | 178 include/linux/clk-provider.h |2 + 28 files changed, 943 insertions(+), 1205 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/samsung,s3c64xx-clock. txt delete mode 100644 arch/arm/mach-s3c64xx/clock.c create mode 100644 drivers/clk/samsung/clk-s3c64xx.c create mode 100644 include/dt-bindings/clock/samsung,s3c64xx-clock.h Basically, this series looks good to me, but I'm not sure how this should be handled because of dependency with PWM cleanup and clk stuff in clk tree now... Patches 1-3 can go into the clk tree. 4-6 should go through their respective trees. If you want I can take 7 8 through the clk tree. Alternatively I can provide patches 1-3 in a separate stable topic branch for you to pull in as a dependency. We'll both merge that stable topic branch into our trees and you can make a note of it for the arm-soc folks. Thanks for your quick response. Would be helpful to samsung tree if you could provide a separate stable topic branch what you suggested for clk stuff in this series. git://git.linaro.org/people/mturquette/linux.git clk-next-s3c64xx Topic branch contains patches 1-3 of this series on top of v3.11-rc2. I have already
Re: [PATCH v2 6/8] usb: host: ohci-s3c2410 Use clk_prepare_enable/clk_disable_unprepare
Alan, Greg, On Tuesday 23 of July 2013 01:49:23 Tomasz Figa wrote: This patch modifies the ohci-s3c2410 driver to prepare and unprepare clocks in addition to enabling and disabling, since it is required by common clock framework. Signed-off-by: Tomasz Figa tomasz.f...@gmail.com --- drivers/usb/host/ohci-s3c2410.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/ohci-s3c2410.c b/drivers/usb/host/ohci-s3c2410.c index e125770..db096bf 100644 --- a/drivers/usb/host/ohci-s3c2410.c +++ b/drivers/usb/host/ohci-s3c2410.c @@ -47,10 +47,10 @@ static void s3c2410_start_hc(struct platform_device *dev, struct usb_hcd *hcd) dev_dbg(dev-dev, s3c2410_start_hc:\n); - clk_enable(usb_clk); + clk_prepare_enable(usb_clk); mdelay(2); /* let the bus clock stabilise */ - clk_enable(clk); + clk_prepare_enable(clk); if (info != NULL) { info-hcd = hcd; @@ -75,8 +75,8 @@ static void s3c2410_stop_hc(struct platform_device *dev) (info-enable_oc)(info, 0); } - clk_disable(clk); - clk_disable(usb_clk); + clk_disable_unprepare(clk); + clk_disable_unprepare(usb_clk); } /* ohci_s3c2410_hub_status_data Any chance to get your ack on this? Best regards, Tomasz -- 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 2/8] clk: samsung: pll: Add support for PLL6552 and PLL6553
On Sunday 28 of July 2013 13:30:51 Mark Brown wrote: On Wed, Jul 24, 2013 at 01:52:19AM +0200, Tomasz Figa wrote: Changes since v2: - Reworked to use new PLL registration method introduced by Yadwinder Singh Brar's patch series: ( http://thread.gmane.org/gmane.linux.kernel.samsung-soc/20041 ) I'm not able to test this series since that lot isn't in -next and I didn't get a copy. Hmm, I wonder what happened with that series, as Mike was supposed to try applying it to clk tree [1]. Mike, could you shed some light on this? Best regards, Tomasz [1] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/20041/focus=21003 -- 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 1/8] clk: mux: Add support for read-only muxes.
Hi Mike, On Tuesday 23 of July 2013 01:49:18 Tomasz Figa wrote: Some platforms have read-only clock muxes that are preconfigured at reset and cannot be changed at runtime. This patch extends mux clock driver to allow handling such read-only muxes by adding new CLK_MUX_READ_ONLY mux flag. Signed-off-by: Tomasz Figa tomasz.f...@gmail.com --- drivers/clk/clk-mux.c| 10 +- include/linux/clk-provider.h | 2 ++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c index 61c..92f1a1b 100644 --- a/drivers/clk/clk-mux.c +++ b/drivers/clk/clk-mux.c @@ -107,6 +107,11 @@ const struct clk_ops clk_mux_ops = { }; EXPORT_SYMBOL_GPL(clk_mux_ops); +const struct clk_ops clk_mux_ro_ops = { + .get_parent = clk_mux_get_parent, +}; +EXPORT_SYMBOL_GPL(clk_mux_ro_ops); + struct clk *clk_register_mux_table(struct device *dev, const char *name, const char **parent_names, u8 num_parents, unsigned long flags, void __iomem *reg, u8 shift, u32 mask, @@ -133,7 +138,10 @@ struct clk *clk_register_mux_table(struct device *dev, const char *name, } init.name = name; - init.ops = clk_mux_ops; + if (clk_mux_flags CLK_MUX_READ_ONLY) + init.ops = clk_mux_ro_ops; + else + init.ops = clk_mux_ops; init.flags = flags | CLK_IS_BASIC; init.parent_names = parent_names; init.num_parents = num_parents; diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 1ec14a7..9487b96 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -327,8 +327,10 @@ struct clk_mux { #define CLK_MUX_INDEX_ONEBIT(0) #define CLK_MUX_INDEX_BITBIT(1) #define CLK_MUX_HIWORD_MASK BIT(2) +#define CLK_MUX_READ_ONLYBIT(3) /* mux setting cannot be changed */ extern const struct clk_ops clk_mux_ops; +extern const struct clk_ops clk_mux_ro_ops; struct clk *clk_register_mux(struct device *dev, const char *name, const char **parent_names, u8 num_parents, unsigned long flags, What do you think about this? Best regards, Tomasz -- 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
Hi Alan, On Monday 22 of July 2013 10:44:39 Alan Stern wrote: On Mon, 22 Jul 2013, Kishon Vijay Abraham I wrote: The PHY and the controller it is attached to are both physical devices. The connection between them is hardwired by the system manufacturer and cannot be changed by software. PHYs are generally described by fixed system-specific board files or by Device Tree information. Are they ever discovered dynamically? No. They are created just like any other platform devices are created. Okay. Are PHYs _always_ platform devices? They can be i2c, spi or any other device types as well. Is the same true for the controllers attached to the PHYs? If not -- if both a PHY and a controller are discovered dynamically -- how does the kernel know whether they are connected to each other? No differences here. Both PHY and controller will have dt information or hwmod data using which platform devices will be created. The kernel needs to know which controller is attached to which PHY. Currently this information is represented by name or ID strings embedded in platform data. right. It's embedded in the platform data of the controller. It must also be embedded in the PHY's platform data somehow. Otherwise, how would the kernel know which PHY to use? By using a PHY lookup as Stephen and I suggested in our previous replies. Without any extra data in platform data. (I have even posted a code example.) The PHY's driver (the supplier) uses the platform data to construct a platform_device structure that represents the PHY. Currently the driver assigns static labels (corresponding to the label used in the platform data of the controller). Until this is done, the controller's driver (the client) cannot use the PHY. right. Since there is no parent-child relation between the PHY and the controller, there is no guarantee that the PHY's driver will be ready when the controller's driver wants to use it. A deferred probe may be needed. right. The issue (or one of the issues) in this discussion is that Greg does not like the idea of using names or IDs to associate PHYs with controllers, because they are too prone to duplications or other errors. Pointers are more reliable. But pointers to what? Since the only data known to be available to both the PHY driver and controller driver is the platform data, the obvious answer is a pointer to platform data (either for the PHY or for the controller, or maybe both). hmm.. it's not going to be simple though as the platform device for the PHY and controller can be created in entirely different places. e.g., in some cases the PHY device is a child of some mfd core device (the device will be created in drivers/mfd) and the controller driver (usually) is created in board file. I guess then we have to come up with something to share a pointer in two different files. The ability for two different source files to share a pointer to a data item defined in a third source file has been around since long before the C language was invented. :-) In this case, it doesn't matter where the platform_device structures are created or where the driver source code is. Let's take a simple example. Suppose the system design includes a PHY named foo. Then the board file could contain: struct phy_info { ... } phy_foo; EXPORT_SYMBOL_GPL(phy_foo); and a header file would contain: extern struct phy_info phy_foo; The PHY supplier could then call phy_create(phy_foo), and the PHY client could call phy_find(phy_foo). Or something like that; make up your own structure tags and function names. It's still possible to have conflicts, but now two PHYs with the same name (or a misspelled name somewhere) will cause an error at link time. This is incorrect, sorry. First of all it's a layering violation - you export random driver-specific symbols from one driver to another. Then imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2 and there are two types of consumer drivers (e.g. USB host controllers). Now consider following mapping: SoC PHY consumer A PHY1HOST1 B PHY1HOST2 C PHY2HOST1 D PHY2HOST2 So we have to be able to use any of the PHYs with any of the host drivers. This means you would have to export symbol with the same name from both PHY drivers, which obviously would not work in this case, because having both drivers enabled (in a multiplatform aware configuration) would lead to linking conflict. Best regards, Tomasz -- 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
[Fixed address of devicetree mailing list and added more people on CC.] For reference, full thread can be found under following link: http://thread.gmane.org/gmane.linux.ports.arm.kernel/252813 Best regards, Tomasz On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote: Hi Alan, On Monday 22 of July 2013 10:44:39 Alan Stern wrote: On Mon, 22 Jul 2013, Kishon Vijay Abraham I wrote: The PHY and the controller it is attached to are both physical devices. The connection between them is hardwired by the system manufacturer and cannot be changed by software. PHYs are generally described by fixed system-specific board files or by Device Tree information. Are they ever discovered dynamically? No. They are created just like any other platform devices are created. Okay. Are PHYs _always_ platform devices? They can be i2c, spi or any other device types as well. Is the same true for the controllers attached to the PHYs? If not -- if both a PHY and a controller are discovered dynamically -- how does the kernel know whether they are connected to each other? No differences here. Both PHY and controller will have dt information or hwmod data using which platform devices will be created. The kernel needs to know which controller is attached to which PHY. Currently this information is represented by name or ID strings embedded in platform data. right. It's embedded in the platform data of the controller. It must also be embedded in the PHY's platform data somehow. Otherwise, how would the kernel know which PHY to use? By using a PHY lookup as Stephen and I suggested in our previous replies. Without any extra data in platform data. (I have even posted a code example.) The PHY's driver (the supplier) uses the platform data to construct a platform_device structure that represents the PHY. Currently the driver assigns static labels (corresponding to the label used in the platform data of the controller). Until this is done, the controller's driver (the client) cannot use the PHY. right. Since there is no parent-child relation between the PHY and the controller, there is no guarantee that the PHY's driver will be ready when the controller's driver wants to use it. A deferred probe may be needed. right. The issue (or one of the issues) in this discussion is that Greg does not like the idea of using names or IDs to associate PHYs with controllers, because they are too prone to duplications or other errors. Pointers are more reliable. But pointers to what? Since the only data known to be available to both the PHY driver and controller driver is the platform data, the obvious answer is a pointer to platform data (either for the PHY or for the controller, or maybe both). hmm.. it's not going to be simple though as the platform device for the PHY and controller can be created in entirely different places. e.g., in some cases the PHY device is a child of some mfd core device (the device will be created in drivers/mfd) and the controller driver (usually) is created in board file. I guess then we have to come up with something to share a pointer in two different files. The ability for two different source files to share a pointer to a data item defined in a third source file has been around since long before the C language was invented. :-) In this case, it doesn't matter where the platform_device structures are created or where the driver source code is. Let's take a simple example. Suppose the system design includes a PHY named foo. Then the board file could contain: struct phy_info { ... } phy_foo; EXPORT_SYMBOL_GPL(phy_foo); and a header file would contain: extern struct phy_info phy_foo; The PHY supplier could then call phy_create(phy_foo), and the PHY client could call phy_find(phy_foo). Or something like that; make up your own structure tags and function names. It's still possible to have conflicts, but now two PHYs with the same name (or a misspelled name somewhere) will cause an error at link time. This is incorrect, sorry. First of all it's a layering violation - you export random driver-specific symbols from one driver to another. Then imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2 and there are two types of consumer drivers (e.g. USB host controllers). Now consider following mapping: SoC PHY consumer A PHY1HOST1 B PHY1HOST2 C PHY2HOST1 D PHY2HOST2
Re: [PATCH 6/6] USB: OHCI: make ohci-s3c2410 a separate driver
Hi Manjunath, Please see some comments inline. On Monday 22 of July 2013 14:49:30 Manjunath Goudar wrote: Separate the Samsung OHCI S3C host controller driver from ohci-hcd host code so that it can be built as a separate driver module. This work is part of enabling multi-platform kernels on ARM; it would be nice to have in 3.12. Signed-off-by: Manjunath Goudar manjunath.gou...@linaro.org Acked-by: Alan Stern st...@rowland.harvard.edu Cc: Arnd Bergmann a...@arndb.de Cc: Greg KH g...@kroah.com Cc: linux-usb@vger.kernel.org V2: -Set non-standard fields in ohci_s3c2410_hc_driver manually, rather than relying on an expanded struct ohci_driver_overrides. -Save orig_ohci_hub_control and orig_ohci_hub_status_data rather than relying on ohci_hub_control and hub_status_data being exported. V3: -Kconfig wrong parentheses discription fixed. -ohci_setup() has been removed because it is called in .reset member of the ohci_hc_driver structure. V4: - Removed extra space before the '='. - Moved /* forward definitions */ line before the declarations of functions. --- drivers/usb/host/Kconfig|8 +++ drivers/usb/host/Makefile |1 + drivers/usb/host/ohci-hcd.c | 18 -- drivers/usb/host/ohci-s3c2410.c | 128 +-- 4 files changed, 66 insertions(+), 89 deletions(-) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 693560a..795d14d 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -390,6 +390,14 @@ config USB_OHCI_HCD_SPEAR Enables support for the on-chip OHCI controller on ST SPEAr chips. +config USB_OHCI_HCD_S3C +tristate Support for S3C on-chip OHCI USB controller +depends on USB_OHCI_HCD (ARCH_S3C24XX || ARCH_S3C64XX) +default y +---help--- + Enables support for the on-chip OHCI controller on + S3C chips. + Please keep the name s3c2410 for consistency. Having all the names come from the first SoC with this hardware is somewhat deterministic as opposed to some other naming methods, e.g. ohci-exynos2, while later on a different SoC from Exynos 2 shows up that needs a different driver. config USB_OHCI_HCD_AT91 tristate Support for Atmel on-chip OHCI USB controller depends on USB_OHCI_HCD ARCH_AT91 diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile index a70b044..9dffe81 100644 --- a/drivers/usb/host/Makefile +++ b/drivers/usb/host/Makefile @@ -51,6 +51,7 @@ obj-$(CONFIG_USB_OHCI_HCD_OMAP1)+= ohci-omap.o obj-$(CONFIG_USB_OHCI_HCD_OMAP3) += ohci-omap3.o obj-$(CONFIG_USB_OHCI_HCD_SPEAR) += ohci-spear.o obj-$(CONFIG_USB_OHCI_HCD_AT91) += ohci-at91.o +obj-$(CONFIG_USB_OHCI_HCD_S3C) += ohci-s3c2410.o obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o obj-$(CONFIG_USB_FHCI_HCD) += fhci.o diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index b48c892..b69a49e 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -1177,11 +1177,6 @@ MODULE_LICENSE (GPL); #define SA_DRIVERohci_hcd_sa_driver #endif -#if defined(CONFIG_ARCH_S3C24XX) || defined(CONFIG_ARCH_S3C64XX) -#include ohci-s3c2410.c -#define S3C2410_PLATFORM_DRIVER ohci_hcd_s3c2410_driver -#endif - #if defined(CONFIG_PXA27x) || defined(CONFIG_PXA3xx) #include ohci-pxa27x.c #define PLATFORM_DRIVER ohci_hcd_pxa27x_driver @@ -1293,12 +1288,6 @@ static int __init ohci_hcd_mod_init(void) goto error_tmio; #endif -#ifdef S3C2410_PLATFORM_DRIVER - retval = platform_driver_register(S3C2410_PLATFORM_DRIVER); - if (retval 0) - goto error_s3c2410; -#endif - #ifdef EP93XX_PLATFORM_DRIVER retval = platform_driver_register(EP93XX_PLATFORM_DRIVER); if (retval 0) @@ -1332,10 +1321,6 @@ static int __init ohci_hcd_mod_init(void) platform_driver_unregister(EP93XX_PLATFORM_DRIVER); error_ep93xx: #endif -#ifdef S3C2410_PLATFORM_DRIVER - platform_driver_unregister(S3C2410_PLATFORM_DRIVER); - error_s3c2410: -#endif #ifdef TMIO_OHCI_DRIVER platform_driver_unregister(TMIO_OHCI_DRIVER); error_tmio: @@ -1382,9 +1367,6 @@ static void __exit ohci_hcd_mod_exit(void) #ifdef EP93XX_PLATFORM_DRIVER platform_driver_unregister(EP93XX_PLATFORM_DRIVER); #endif -#ifdef S3C2410_PLATFORM_DRIVER - platform_driver_unregister(S3C2410_PLATFORM_DRIVER); -#endif #ifdef TMIO_OHCI_DRIVER platform_driver_unregister(TMIO_OHCI_DRIVER); #endif diff --git a/drivers/usb/host/ohci-s3c2410.c b/drivers/usb/host/ohci-s3c2410.c index e125770..48b5948 100644 --- a/drivers/usb/host/ohci-s3c2410.c +++ b/drivers/usb/host/ohci-s3c2410.c @@ -19,19 +19,36 @@ * This file is licenced under the GPL. */ -#include linux/platform_device.h #include linux/clk.h
Re: [PATCH v2 1/8] clk: mux: Add support for read-only muxes.
Hi Sergei, On Tuesday 23 of July 2013 15:22:44 Sergei Shtylyov wrote: Hello. On 23-07-2013 3:49, Tomasz Figa wrote: Some platforms have read-only clock muxes that are preconfigured at reset and cannot be changed at runtime. This patch extends mux clock driver to allow handling such read-only muxes by adding new CLK_MUX_READ_ONLY mux flag. Signed-off-by: Tomasz Figa tomasz.f...@gmail.com [...] diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 1ec14a7..9487b96 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -327,8 +327,10 @@ struct clk_mux { #define CLK_MUX_INDEX_ONE BIT(0) #define CLK_MUX_INDEX_BIT BIT(1) #define CLK_MUX_HIWORD_MASK BIT(2) +#define CLK_MUX_READ_ONLY BIT(3) /* mux setting cannot be changed */ Please align BIT(3) with the above BIT() invocations. Different indentation was intended here to fit the comment, like in case of generic flags. IMHO remaining flags should be changed to this way as well, but this is probably material for another patch. Best regards, Tomasz -- 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
On Tuesday 23 of July 2013 10:37:05 Alan Stern wrote: On Tue, 23 Jul 2013, Tomasz Figa wrote: On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote: Hi Alan, Thanks for helping to clarify the issues here. Okay. Are PHYs _always_ platform devices? They can be i2c, spi or any other device types as well. In those other cases, presumably there is no platform data associated with the PHY since it isn't a platform device. Then how does the kernel know which controller is attached to the PHY? Is this spelled out in platform data associated with the PHY's i2c/spi/whatever parent? PHY. Currently this information is represented by name or ID strings embedded in platform data. right. It's embedded in the platform data of the controller. It must also be embedded in the PHY's platform data somehow. Otherwise, how would the kernel know which PHY to use? By using a PHY lookup as Stephen and I suggested in our previous replies. Without any extra data in platform data. (I have even posted a code example.) I don't understand, because I don't know what a PHY lookup does. I have provided a code example in [1]. Feel free to ask questions about those code snippets. [1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/252813/focus=20889 In this case, it doesn't matter where the platform_device structures are created or where the driver source code is. Let's take a simple example. Suppose the system design includes a PHY named foo. Then the board file could contain: struct phy_info { ... } phy_foo; EXPORT_SYMBOL_GPL(phy_foo); and a header file would contain: extern struct phy_info phy_foo; The PHY supplier could then call phy_create(phy_foo), and the PHY client could call phy_find(phy_foo). Or something like that; make up your own structure tags and function names. It's still possible to have conflicts, but now two PHYs with the same name (or a misspelled name somewhere) will cause an error at link time. This is incorrect, sorry. First of all it's a layering violation - you export random driver-specific symbols from one driver to another. Then No, that's not what I said. Neither the PHY driver nor the controller driver exports anything to the other. Instead, both drivers use data exported by the board file. It's still a random, driver-specific global symbol exported from board file to drivers. imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2 and there are two types of consumer drivers (e.g. USB host controllers). Now consider following mapping: SoC PHY consumer A PHY1HOST1 B PHY1HOST2 C PHY2HOST1 D PHY2HOST2 So we have to be able to use any of the PHYs with any of the host drivers. This means you would have to export symbol with the same name from both PHY drivers, which obviously would not work in this case, because having both drivers enabled (in a multiplatform aware configuration) would lead to linking conflict. You're right; the scheme was too simple. Instead, the board file must export two types of data structures, one for PHYs and one for controllers. Like this: struct phy_info { /* Info for the controller attached to this PHY */ struct controller_info *hinfo; }; struct controller_info { /* Info for the PHY which this controller is attached to */ struct phy_info *pinfo; }; The board file for SoC A would contain: struct phy_info phy1 = {host1); EXPORT_SYMBOL(phy1); struct controller_info host1 = {phy1}; EXPORT_SYMBOL(host1); The board file for SoC B would contain: struct phy_info phy1 = {host2); EXPORT_SYMBOL(phy1); struct controller_info host2 = {phy1}; EXPORT_SYMBOL(host2); And so on. This explicitly gives the connection between PHYs and controllers. The PHY providers would use phy1 or phy2, and the PHY consumers would use host1 or host2. This could work assuming that only one SoC and one board is supported in single kernel image. However it's not the case. We've used to support multiple boards since a long time already and now for selected platforms we even support multiplatform, i.e. multiple SoCs in single zImage. Such solution will not work. Best regards, Tomasz -- 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
On Tuesday 23 of July 2013 09:18:46 Greg KH wrote: On Tue, Jul 23, 2013 at 08:48:24PM +0530, Kishon Vijay Abraham I wrote: Hi, On Tuesday 23 July 2013 08:07 PM, Alan Stern wrote: On Tue, 23 Jul 2013, Tomasz Figa wrote: On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote: Hi Alan, Thanks for helping to clarify the issues here. Okay. Are PHYs _always_ platform devices? They can be i2c, spi or any other device types as well. In those other cases, presumably there is no platform data associated with the PHY since it isn't a platform device. Then how does the kernel know which controller is attached to the PHY? Is this spelled out in platform data associated with the PHY's i2c/spi/whatever parent? Yes. I think we could use i2c_board_info for passing platform data. PHY. Currently this information is represented by name or ID strings embedded in platform data. right. It's embedded in the platform data of the controller. It must also be embedded in the PHY's platform data somehow. Otherwise, how would the kernel know which PHY to use? By using a PHY lookup as Stephen and I suggested in our previous replies. Without any extra data in platform data. (I have even posted a code example.) I don't understand, because I don't know what a PHY lookup does. It is how the PHY framework finds a PHY, when the controller (say USB)requests a PHY from the PHY framework. In this case, it doesn't matter where the platform_device structures are created or where the driver source code is. Let's take a simple example. Suppose the system design includes a PHY named foo. Then the board file could contain: struct phy_info { ... } phy_foo; EXPORT_SYMBOL_GPL(phy_foo); and a header file would contain: extern struct phy_info phy_foo; The PHY supplier could then call phy_create(phy_foo), and the PHY client could call phy_find(phy_foo). Or something like that; make up your own structure tags and function names. It's still possible to have conflicts, but now two PHYs with the same name (or a misspelled name somewhere) will cause an error at link time. This is incorrect, sorry. First of all it's a layering violation - you export random driver-specific symbols from one driver to another. Then No, that's not what I said. Neither the PHY driver nor the controller driver exports anything to the other. Instead, both drivers use data exported by the board file. I think instead we can use the same data while creating the platform data of the controller and the PHY. The PHY driver while creating the PHY (using PHY framework) will also pass the *data* it actually got from the platform data to the framework. The PHY user driver (USB), while requesting for the PHY (from the PHY framework) will pass the *data* it got from its platform data. The PHY framework can do a comparison of the *data* pointers it has and return the appropriate PHY to the controller. imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2 and there are two types of consumer drivers (e.g. USB host controllers). Now consider following mapping: SoC PHY consumer A PHY1HOST1 B PHY1HOST2 C PHY2HOST1 D PHY2HOST2 So we have to be able to use any of the PHYs with any of the host drivers. This means you would have to export symbol with the same name from both PHY drivers, which obviously would not work in this case, because having both drivers enabled (in a multiplatform aware configuration) would lead to linking conflict. You're right; the scheme was too simple. Instead, the board file must export two types of data structures, one for PHYs and one for controllers. Like this: struct phy_info { /* Info for the controller attached to this PHY */ struct controller_info *hinfo; }; struct controller_info { /* Info for the PHY which this controller is attached to */ struct phy_info *pinfo; }; The board file for SoC A would contain: struct phy_info phy1 = {host1); EXPORT_SYMBOL(phy1); struct controller_info host1 = {phy1}; EXPORT_SYMBOL(host1); The board file for SoC B would contain: struct phy_info phy1 = {host2); EXPORT_SYMBOL(phy1); struct controller_info host2 = {phy1}; EXPORT_SYMBOL(host2); I meant something like this struct phy_info { const char *name; }; struct phy_platform_data { . . struct phy_info *info; }; struct usb_controller_platform_data { . . struct phy_info *info; }; struct phy_info phy_info; While creating the phy device struct phy_platform_data phy_data
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 10:37:11 Greg KH wrote: On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote: Ick, no. Why can't you just pass the pointer to the phy itself? If you had a priv pointer to search from, then you could have just passed the original phy pointer in the first place, right? IMHO it would be better if you provided some code example, but let's try to check if I understood you correctly. It's not my code that I want to have added, so I don't have to write examples, I just get to complain about the existing stuff :) Still, I think that some small code snippets illustrating the idea are really helpful. 8 [Board file] static struct phy my_phy; static struct platform_device phy_pdev = { /* ... */ .platform_data = my_phy; /* ... */ }; static struct platform_device phy_pdev = { /* ... */ .platform_data = my_phy; /* ... */ }; [Provider driver] struct phy *phy = pdev-dev.platform_data; ret = phy_create(phy); [Consumer driver] struct phy *phy = pdev-dev.platform_data; ret = phy_get(pdev-dev, phy); --- -8 Is this what you mean? No. Well, kind of. What's wrong with using the platform data structure unique to the board to have the pointer? For example (just randomly picking one), the ata-pxa driver would change include/linux/platform_data/ata-pxa.h to have a phy pointer in it: struct phy; struct pata_pxa_pdata { /* PXA DMA DREQ0:2 pin */ uint32_tdma_dreq; /* Register shift */ uint32_treg_shift; /* IRQ flags */ uint32_tirq_flags; /* PHY */ struct phy *phy; }; Then, when you create the platform, set the phy* pointer with a call to phy_create(). Then you can use that pointer wherever that plaform data is available (i.e. whereever platform_data is at). Hmm? So, do you suggest to call phy_create() from board file? What phy_ops struct and other hardware parameters would it take? The issue is that a string name is not going to scale at all, as it requires hard-coded information that will change over time (as the existing clock interface is already showing.) I fully agree that a simple, single string will not scale even in some, not so uncommon cases, but there is already a lot of existing lookup solutions over the kernel and so there is no point in introducing another one. I'm trying to get _rid_ of lookup solutions and just use a real pointer, as you should. I'll go tackle those other ones after this one is taken care of, to show how the others should be handled as well. There was a reason for introducing lookup solutions. The reason was that in board file there is no way to get a pointer to something that is going to be created much later in time. We don't do time travel ;-). Please just pass the real phy pointer around, that's what it is there for. Your board binding logic/code should be able to handle this, as it somehow was going to do the same thing with a name. It's technically correct, but quality of this solution isn't really nice, because it's a layering violation (at least if I understood what you mean). This is because you need to have full definition of struct phy in board file and a structure that is used as private data in PHY core comes from platform code. No, just a pointer, you don't need the full structure until you get to some .c code that actually manipulates the phy itself, for all other places, you are just dealing with a pointer and a structure you never reference. Does that make more sense? Well, to the point that I think I now understood your suggestion. Unfortunately the suggestion alone isn't really something that can be done, considering how driver core and generic frameworks work. Best regards, Tomasz -- 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
On Tuesday 23 of July 2013 12:44:23 Greg KH wrote: On Tue, Jul 23, 2013 at 08:31:05PM +0100, Mark Brown wrote: You don't know the id of the device you are looking up, due to multiple devices being in the system (dynamic ids, look back earlier in this thread for details about that.) I got copied in very late so don't have most of the thread I'm afraid, I did try looking at web archives but didn't see a clear problem statement. In any case this is why the APIs doing lookups do the lookups in the context of the requesting device - devices ask for whatever name they use locally. What do you mean by locally? The problem with the api was that the phy core wanted a id and a name to create a phy, and then later other code was doing a lookup based on the name and id (mushed together), because it knew that this device was the one it wanted. Just like the clock api, which, for multiple devices, has proven to cause problems. I don't want to see us accept an api that we know has issues in it now, I'd rather us fix it up properly. Subsystems should be able to create ids how ever they want to, and not rely on the code calling them to specify the names of the devices that way, otherwise the api is just too fragile. I think, that if you create a device, then just carry around the pointer to that device (in this case a phy) and pass it to whatever other code needs it. No need to do lookups on known names or anything else, just normal pointers, with no problems for multiple devices, busses, or naming issues. PHY object is not a device, it is something that a device driver creates (one or more instances of) when it is being probed. You don't have a clean way to export this PHY object to other driver, other than keeping this PHY on a list inside PHY core with some well-known ID (e.g. device name + consumer port name/index, like in regulator core) and then to use this well-known ID inside consumer driver as a lookup key passed to phy_get(); Actually I think for PHY case, exactly the same way as used for regulators might be completely fine: 1. Each PHY would have some kind of platform, non-unique name, that is just used to print some messages (like the platform/board name of a regulator). 2. Each PHY would have an array of consumers. Consumer specifier would consist of consumer device name and consumer port name - just like in regulator subsystem. 3. PHY driver receives an array of, let's say, phy_init_data inside its platform data that it would use to register its PHYs. 4. Consumer drivers would have constant consumer port names and wouldn't receive any information about PHYs from platform code. Code example: [Board file] static const struct phy_consumer_data usb_20_phy0_consumers[] = { { .devname = foo-ehci, .port = usbphy, }, }; static const struct phy_consumer_data usb_20_phy1_consumers[] = { { .devname = foo-otg, .port = otgphy, }, }; static const struct phy_init_data my_phys[] = { { .name = USB 2.0 PHY 0, .consumers = usb_20_phy0_consumers, .num_consumers = ARRAY_SIZE(usb_20_phy0_consumers), }, { .name = USB 2.0 PHY 1, .consumers = usb_20_phy1_consumers, .num_consumers = ARRAY_SIZE(usb_20_phy1_consumers), }, { } }; static const struct platform_device usb_phy_pdev = { .name = foo-usbphy, .id = -1, .dev = { .platform_data = my_phys, }, }; [PHY driver] static int foo_usbphy_probe(pdev) { struct foo_usbphy *foo; struct phy_init_data *init_data = pdev-dev.platform_data; /* ... */ // for each PHY in init_data { phy_register(foo-phy[i], init_data[i]); // } /* ... */ } [EHCI driver] static int foo_ehci_probe(pdev) { struct phy *phy; /* ... */ phy = phy_get(pdev-dev, usbphy); /* ... */ } [OTG driver] static int foo_otg_probe(pdev) { struct phy *phy; /* ... */ phy = phy_get(pdev-dev, otgphy); /* ... */ } Having to write platform data for everything gets old fast and the code duplication is pretty tedious... Adding a single pointer is tedious? Where is the name that you are going to lookup going to come from? That code doesn't write itself... It's adding platform data in the first place that gets tedious - and of course there's also DT and ACPI to worry about, it's not just a case of platform data and then you're done. Pushing the lookup into library code means that drivers don't have to worry about any of this stuff. I agree, so just pass around the pointer to the phy and all is good. No need to worry about DT or ACPI or anything else. With Device Tree we don't have board files anymore. How would
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 15:36:00 Alan Stern wrote: On Tue, 23 Jul 2013, Tomasz Figa wrote: IMHO it would be better if you provided some code example, but let's try to check if I understood you correctly. 8--- - [Board file] static struct phy my_phy; static struct platform_device phy_pdev = { /* ... */ .platform_data = my_phy; /* ... */ }; static struct platform_device phy_pdev = { This should be controller_pdev, not phy_pdev, yes? Right. A copy-pasto. /* ... */ .platform_data = my_phy; /* ... */ }; [Provider driver] struct phy *phy = pdev-dev.platform_data; ret = phy_create(phy); [Consumer driver] struct phy *phy = pdev-dev.platform_data; ret = phy_get(pdev-dev, phy); Or even just phy_get(pdev-dev), because phy_get() could be smart enough to to set phy = dev-platform_data. Unless you need more than one PHY in this driver... -- --8 Is this what you mean? That's what I was going to suggest too. The struct phy is defined in the board file, which already knows about all the PHYs that exist in the system. (Or perhaps it is allocated dynamically, so that when many board files are present in the same kernel, only the entries listed in the board file for the current system get created.) Well, such dynamic allocation is a must. We don't accept non-multiplatform aware code anymore, not even saying about multiboard. Then the structure's address is stored in the platform data and made available to both the provider and the consumer. Yes, technically this can work. You would still have to perform some kind of synchronization to make sure that the PHY bound to this structure is actually present. This is again technically doable (e.g. a list of registered struct phys inside PHY core). Even though the struct phy is defined (or allocated) in the board file, its contents don't get filled in until the PHY driver provides the details. You can't assure this. Board file is free to do whatever it wants with this struct. A clean solution would prevent this. It's technically correct, but quality of this solution isn't really nice, because it's a layering violation (at least if I understood what you mean). This is because you need to have full definition of struct phy in board file and a structure that is used as private data in PHY core comes from platform code. You don't have to have a full definition in the board file. Just a partial definition -- most of the contents can be filled in later, when the PHY driver is ready to store the private data. It's not a layering violation for one region of the kernel to store private data in a structure defined by another part of the kernel. This happens all the time (e.g., dev_set_drvdata). Not really. The phy struct is something that _is_ private data of PHY subsystem, not something that can store private data of PHY subsystem (sure it can store private data of particular PHY driver, but that's another story) and only PHY subsystem should have access to its contents. By the way, we need to consider other cases here as well, for example it would be nice to have a single phy_get() function that works for both non- DT and DT cases to make the consumer driver not have to worry whether it's being probed from DT or not. I'd suggest simply reusing the lookup method of regulator framework, just as I suggested here: http://thread.gmane.org/gmane.linux.ports.arm.kernel/252813/focus=101661 Best regards, Tomasz -- 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
On Tuesday 23 of July 2013 11:04:14 Greg KH wrote: On Tue, Jul 23, 2013 at 07:48:11PM +0200, Tomasz Figa wrote: On Tuesday 23 of July 2013 10:37:11 Greg KH wrote: On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote: Ick, no. Why can't you just pass the pointer to the phy itself? If you had a priv pointer to search from, then you could have just passed the original phy pointer in the first place, right? IMHO it would be better if you provided some code example, but let's try to check if I understood you correctly. It's not my code that I want to have added, so I don't have to write examples, I just get to complain about the existing stuff :) Still, I think that some small code snippets illustrating the idea are really helpful. 8--- - [Board file] static struct phy my_phy; static struct platform_device phy_pdev = { /* ... */ .platform_data = my_phy; /* ... */ }; static struct platform_device phy_pdev = { /* ... */ .platform_data = my_phy; /* ... */ }; [Provider driver] struct phy *phy = pdev-dev.platform_data; ret = phy_create(phy); [Consumer driver] struct phy *phy = pdev-dev.platform_data; ret = phy_get(pdev-dev, phy); -- - -8 Is this what you mean? No. Well, kind of. What's wrong with using the platform data structure unique to the board to have the pointer? For example (just randomly picking one), the ata-pxa driver would change include/linux/platform_data/ata-pxa.h to have a phy pointer in it: struct phy; struct pata_pxa_pdata { /* PXA DMA DREQ0:2 pin */ uint32_tdma_dreq; /* Register shift */ uint32_treg_shift; /* IRQ flags */ uint32_tirq_flags; /* PHY */ struct phy *phy; }; Then, when you create the platform, set the phy* pointer with a call to phy_create(). Then you can use that pointer wherever that plaform data is available (i.e. whereever platform_data is at). Hmm? So, do you suggest to call phy_create() from board file? What phy_ops struct and other hardware parameters would it take? The issue is that a string name is not going to scale at all, as it requires hard-coded information that will change over time (as the existing clock interface is already showing.) I fully agree that a simple, single string will not scale even in some, not so uncommon cases, but there is already a lot of existing lookup solutions over the kernel and so there is no point in introducing another one. I'm trying to get _rid_ of lookup solutions and just use a real pointer, as you should. I'll go tackle those other ones after this one is taken care of, to show how the others should be handled as well. There was a reason for introducing lookup solutions. The reason was that in board file there is no way to get a pointer to something that is going to be created much later in time. We don't do time travel ;-). Please just pass the real phy pointer around, that's what it is there for. Your board binding logic/code should be able to handle this, as it somehow was going to do the same thing with a name. It's technically correct, but quality of this solution isn't really nice, because it's a layering violation (at least if I understood what you mean). This is because you need to have full definition of struct phy in board file and a structure that is used as private data in PHY core comes from platform code. No, just a pointer, you don't need the full structure until you get to some .c code that actually manipulates the phy itself, for all other places, you are just dealing with a pointer and a structure you never reference. Does that make more sense? Well, to the point that I think I now understood your suggestion. Unfortunately the suggestion alone isn't really something that can be done, considering how driver core and generic frameworks work. Ok, given that I seem to be totally confused as to exactly how the board-specific frameworks work, I'll take your word for it. Well, they are working in a way that keeps separation of layers, making things clean. Platform code should not (well, there might exist some in tree hacks, but this should not be propagated) used to exchange data between drivers, but rather to specify board specific parameters for generic drivers. If drivers need to cooperate, there must be a dedicated interface
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 16:53:55 Alan Stern wrote: On Tue, 23 Jul 2013, Tomasz Figa wrote: That's what I was going to suggest too. The struct phy is defined in the board file, which already knows about all the PHYs that exist in the system. (Or perhaps it is allocated dynamically, so that when many board files are present in the same kernel, only the entries listed in the board file for the current system get created.) Well, such dynamic allocation is a must. We don't accept non-multiplatform aware code anymore, not even saying about multiboard. Then the structure's address is stored in the platform data and made available to both the provider and the consumer. Yes, technically this can work. You would still have to perform some kind of synchronization to make sure that the PHY bound to this structure is actually present. This is again technically doable (e.g. a list of registered struct phys inside PHY core). The synchronization takes place inside phy_get. If phy_create hasn't been called for this structure by the time phy_get runs, phy_get will return an error. Yes, this is the solution that I had in mind when saying that this is doable. Even though the struct phy is defined (or allocated) in the board file, its contents don't get filled in until the PHY driver provides the details. You can't assure this. Board file is free to do whatever it wants with this struct. A clean solution would prevent this. I'm not sure what you mean here. Of course I can't prevent a board file from messing up a data structure. I can't prevent it from causing memory access violations either; in fact, I can't prevent any bugs in other people's code. Besides, why do you say the board file is free to do whatever it wants with the struct phy? Currently the struct phy is created by the PHY provider and the PHY core, right? It's not even mentioned in the board file. I mean, if you have a struct type of which full declaration is available for some code, this code can access any memeber of it without any hacks, which is not something that we want to have in board files. The phy struct should be opaque for them. It's technically correct, but quality of this solution isn't really nice, because it's a layering violation (at least if I understood what you mean). This is because you need to have full definition of struct phy in board file and a structure that is used as private data in PHY core comes from platform code. You don't have to have a full definition in the board file. Just a partial definition -- most of the contents can be filled in later, when the PHY driver is ready to store the private data. It's not a layering violation for one region of the kernel to store private data in a structure defined by another part of the kernel. This happens all the time (e.g., dev_set_drvdata). Not really. The phy struct is something that _is_ private data of PHY subsystem, not something that can store private data of PHY subsystem (sure it can store private data of particular PHY driver, but that's another story) and only PHY subsystem should have access to its contents. If you want to keep the phy struct completely separate from the board file, there's an easy way to do it. Let's say the board file knows about N different PHYs in the system. Then you define an array of N pointers to phys: struct phy *(phy_address[N]); In the platform data for both PHY j and its controller, store phy_address[j]. The PHY provider passes this cookie to phy_create: cookie = pdev-dev.platform_data; ret = phy_create(phy, cookie); and phy_create simply stores: *cookie = phy. The PHY consumer does much the same the same thing: cookie = pdev-dev.platform_data; phy = phy_get(cookie); phy_get returns *cookie if it isn't NULL, or an ERR_PTR otherwise. OK, this can work. Again, just technically, because it's rather ugly. By the way, we need to consider other cases here as well, for example it would be nice to have a single phy_get() function that works for both non- DT and DT cases to make the consumer driver not have to worry whether it's being probed from DT or not. You ought to be able to adapt this scheme to work with DT. Maybe by having multiple phy_address arrays. 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. Anyway, board file should not be considered as a method to exchange data between drivers. It should be used only to pass data from it to drivers, not the other way. Ideally all data in a board file should be marked as const and __init and dropped after system initialization. Best regards, Tomasz
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 13:50:07 Greg KH wrote: On Tue, Jul 23, 2013 at 10:07:52PM +0200, Tomasz Figa wrote: On Tuesday 23 of July 2013 12:44:23 Greg KH wrote: On Tue, Jul 23, 2013 at 08:31:05PM +0100, Mark Brown wrote: You don't know the id of the device you are looking up, due to multiple devices being in the system (dynamic ids, look back earlier in this thread for details about that.) I got copied in very late so don't have most of the thread I'm afraid, I did try looking at web archives but didn't see a clear problem statement. In any case this is why the APIs doing lookups do the lookups in the context of the requesting device - devices ask for whatever name they use locally. What do you mean by locally? The problem with the api was that the phy core wanted a id and a name to create a phy, and then later other code was doing a lookup based on the name and id (mushed together), because it knew that this device was the one it wanted. Just like the clock api, which, for multiple devices, has proven to cause problems. I don't want to see us accept an api that we know has issues in it now, I'd rather us fix it up properly. Subsystems should be able to create ids how ever they want to, and not rely on the code calling them to specify the names of the devices that way, otherwise the api is just too fragile. I think, that if you create a device, then just carry around the pointer to that device (in this case a phy) and pass it to whatever other code needs it. No need to do lookups on known names or anything else, just normal pointers, with no problems for multiple devices, busses, or naming issues. PHY object is not a device, it is something that a device driver creates (one or more instances of) when it is being probed. But you created a 'struct device' for it, so I think of it as a device be it virtual or real :) Keep in mind that those virtual devices are created by PHY driver bound to a real device and one real device can have multiple virtual devices behind it. You don't have a clean way to export this PHY object to other driver, other than keeping this PHY on a list inside PHY core with some well-known ID (e.g. device name + consumer port name/index, like in regulator core) and then to use this well-known ID inside consumer driver as a lookup key passed to phy_get(); Actually I think for PHY case, exactly the same way as used for regulators might be completely fine: 1. Each PHY would have some kind of platform, non-unique name, that is just used to print some messages (like the platform/board name of a regulator). 2. Each PHY would have an array of consumers. Consumer specifier would consist of consumer device name and consumer port name - just like in regulator subsystem. 3. PHY driver receives an array of, let's say, phy_init_data inside its platform data that it would use to register its PHYs. 4. Consumer drivers would have constant consumer port names and wouldn't receive any information about PHYs from platform code. Code example: [Board file] static const struct phy_consumer_data usb_20_phy0_consumers[] = { { .devname = foo-ehci, .port = usbphy, }, }; static const struct phy_consumer_data usb_20_phy1_consumers[] = { { .devname = foo-otg, .port = otgphy, }, }; static const struct phy_init_data my_phys[] = { { .name = USB 2.0 PHY 0, .consumers = usb_20_phy0_consumers, .num_consumers = ARRAY_SIZE(usb_20_phy0_consumers), }, { .name = USB 2.0 PHY 1, .consumers = usb_20_phy1_consumers, .num_consumers = ARRAY_SIZE(usb_20_phy1_consumers), }, { } }; static const struct platform_device usb_phy_pdev = { .name = foo-usbphy, .id = -1, .dev = { .platform_data = my_phys, }, }; [PHY driver] static int foo_usbphy_probe(pdev) { struct foo_usbphy *foo; struct phy_init_data *init_data = pdev-dev.platform_data; /* ... */ // for each PHY in init_data { phy_register(foo-phy[i], init_data[i]); // } /* ... */ } [EHCI driver] static int foo_ehci_probe(pdev) { struct phy *phy; /* ... */ phy = phy_get(pdev-dev, usbphy); /* ... */ } [OTG driver] static int foo_otg_probe(pdev) { struct phy *phy; /* ... */ phy = phy_get(pdev-dev, otgphy); /* ... */ } That's not so bad, as long as you let the phy core use whatever name it wants for the device when it registers it with sysfs. Yes, in regulator core
[PATCH v3 2/8] clk: samsung: pll: Add support for PLL6552 and PLL6553
This patch adds support for PLL6552 and PLL6553 PLLs present on Samsung S3C64xx SoCs. Signed-off-by: Tomasz Figa tomasz.f...@gmail.com Acked-by: Mike Turquette mturque...@linaro.org --- drivers/clk/samsung/clk-pll.c | 77 +++ drivers/clk/samsung/clk-pll.h | 2 ++ 2 files changed, 79 insertions(+) Changes since v2: - Reworked to use new PLL registration method introduced by Yadwinder Singh Brar's patch series: ( http://thread.gmane.org/gmane.linux.kernel.samsung-soc/20041 ) diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c index f80efb6..7572d1d 100644 --- a/drivers/clk/samsung/clk-pll.c +++ b/drivers/clk/samsung/clk-pll.c @@ -438,6 +438,77 @@ struct clk * __init samsung_clk_register_pll46xx(const char *name, } /* + * PLL6552 Clock Type + */ + +#define PLL6552_MDIV_MASK 0x3ff +#define PLL6552_PDIV_MASK 0x3f +#define PLL6552_SDIV_MASK 0x7 +#define PLL6552_MDIV_SHIFT 16 +#define PLL6552_PDIV_SHIFT 8 +#define PLL6552_SDIV_SHIFT 0 + +static unsigned long samsung_pll6552_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct samsung_clk_pll *pll = to_clk_pll(hw); + u32 mdiv, pdiv, sdiv, pll_con; + u64 fvco = parent_rate; + + pll_con = __raw_readl(pll-con_reg); + mdiv = (pll_con PLL6552_MDIV_SHIFT) PLL6552_MDIV_MASK; + pdiv = (pll_con PLL6552_PDIV_SHIFT) PLL6552_PDIV_MASK; + sdiv = (pll_con PLL6552_SDIV_SHIFT) PLL6552_SDIV_MASK; + + fvco *= mdiv; + do_div(fvco, (pdiv sdiv)); + + return (unsigned long)fvco; +} + +static const struct clk_ops samsung_pll6552_clk_ops = { + .recalc_rate = samsung_pll6552_recalc_rate, +}; + +/* + * PLL6553 Clock Type + */ + +#define PLL6553_MDIV_MASK 0xff +#define PLL6553_PDIV_MASK 0x3f +#define PLL6553_SDIV_MASK 0x7 +#define PLL6553_KDIV_MASK 0x +#define PLL6553_MDIV_SHIFT 16 +#define PLL6553_PDIV_SHIFT 8 +#define PLL6553_SDIV_SHIFT 0 +#define PLL6553_KDIV_SHIFT 0 + +static unsigned long samsung_pll6553_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct samsung_clk_pll *pll = to_clk_pll(hw); + u32 mdiv, pdiv, sdiv, kdiv, pll_con0, pll_con1; + u64 fvco = parent_rate; + + pll_con0 = __raw_readl(pll-con_reg); + pll_con1 = __raw_readl(pll-con_reg + 0x4); + mdiv = (pll_con0 PLL6553_MDIV_SHIFT) PLL6553_MDIV_MASK; + pdiv = (pll_con0 PLL6553_PDIV_SHIFT) PLL6553_PDIV_MASK; + sdiv = (pll_con0 PLL6553_SDIV_SHIFT) PLL6553_SDIV_MASK; + kdiv = (pll_con1 PLL6553_KDIV_SHIFT) PLL6553_KDIV_MASK; + + fvco *= (mdiv 16) + kdiv; + do_div(fvco, (pdiv sdiv)); + fvco = 16; + + return (unsigned long)fvco; +} + +static const struct clk_ops samsung_pll6553_clk_ops = { + .recalc_rate = samsung_pll6553_recalc_rate, +}; + +/* * PLL2550x Clock Type */ @@ -572,6 +643,12 @@ static void __init _samsung_clk_register_pll(struct samsung_pll_clock *pll_clk, else init.ops = samsung_pll36xx_clk_ops; break; + case pll_6552: + init.ops = samsung_pll6552_clk_ops; + break; + case pll_6553: + init.ops = samsung_pll6553_clk_ops; + break; default: pr_warn(%s: Unknown pll type for pll clk %s\n, __func__, pll_clk-name); diff --git a/drivers/clk/samsung/clk-pll.h b/drivers/clk/samsung/clk-pll.h index 95ae23d..cd11037 100644 --- a/drivers/clk/samsung/clk-pll.h +++ b/drivers/clk/samsung/clk-pll.h @@ -17,6 +17,8 @@ enum samsung_pll_type { pll_36xx, pll_2550, pll_2650, + pll_6552, + pll_6553, }; #define PLL_35XX_RATE(_rate, _m, _p, _s) \ -- 1.8.3.2 -- 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 3/8] clk: samsung: Add clock driver for S3C64xx SoCs
This patch adds new, Common Clock Framework-based clock driver for Samsung S3C64xx SoCs. The driver is just added, without actually letting the platforms use it yet, since this requires more intermediate steps. Signed-off-by: Tomasz Figa tomasz.f...@gmail.com Acked-by: Mike Turquette mturque...@linaro.org --- .../bindings/clock/samsung,s3c64xx-clock.txt | 77 drivers/clk/samsung/Makefile | 3 + drivers/clk/samsung/clk-s3c64xx.c | 473 + include/dt-bindings/clock/samsung,s3c64xx-clock.h | 178 4 files changed, 731 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/samsung,s3c64xx-clock.txt create mode 100644 drivers/clk/samsung/clk-s3c64xx.c create mode 100644 include/dt-bindings/clock/samsung,s3c64xx-clock.h Changes since v2: - Reworked to use new PLL registration method introduced by Yadwinder Singh Brar's patch series: ( http://thread.gmane.org/gmane.linux.kernel.samsung-soc/20041 ) diff --git a/Documentation/devicetree/bindings/clock/samsung,s3c64xx-clock.txt b/Documentation/devicetree/bindings/clock/samsung,s3c64xx-clock.txt new file mode 100644 index 000..fa171dc --- /dev/null +++ b/Documentation/devicetree/bindings/clock/samsung,s3c64xx-clock.txt @@ -0,0 +1,77 @@ +* Samsung S3C64xx Clock Controller + +The S3C64xx clock controller generates and supplies clock to various controllers +within the SoC. The clock binding described here is applicable to all SoCs in +the S3C64xx family. + +Required Properties: + +- compatible: should be one of the following. + - samsung,s3c6400-clock - controller compatible with S3C6400 SoC. + - samsung,s3c6410-clock - controller compatible with S3C6410 SoC. + +- reg: physical base address of the controller and length of memory mapped + region. + +- #clock-cells: should be 1. + +Each clock is assigned an identifier and client nodes can use this identifier +to specify the clock which they consume. Some of the clocks are available only +on a particular S3C64xx SoC and this is specified where applicable. + +All available clocks are defined as preprocessor macros in +dt-bindings/clock/samsung,s3c64xx-clock.h header and can be used in device +tree sources. + +External clocks: + +There are several clocks that are generated outside the SoC. It is expected +that they are defined using standard clock bindings with following +clock-output-names: + - fin_pll - PLL input clock (xtal/extclk) - required, + - xusbxti - USB xtal - required, + - iiscdclk0 - I2S0 codec clock - optional, + - iiscdclk1 - I2S1 codec clock - optional, + - iiscdclk2 - I2S2 codec clock - optional, + - pcmcdclk0 - PCM0 codec clock - optional, + - pcmcdclk1 - PCM1 codec clock - optional, only S3C6410. + +Example: Clock controller node: + + clock: clock-controller@7e00f000 { + compatible = samsung,s3c6410-clock; + reg = 0x7e00f000 0x1000; + #clock-cells = 1; + }; + +Example: Required external clocks: + + fin_pll: clock-fin-pll { + compatible = fixed-clock; + clock-output-names = fin_pll; + clock-frequency = 1200; + #clock-cells = 0; + }; + + xusbxti: clock-xusbxti { + compatible = fixed-clock; + clock-output-names = xusbxti; + clock-frequency = 4800; + #clock-cells = 0; + }; + +Example: UART controller node that consumes the clock generated by the clock + controller (refer to the standard clock bindings for information about + clocks and clock-names properties): + + uart0: serial@7f005000 { + compatible = samsung,s3c6400-uart; + reg = 0x7f005000 0x100; + interrupt-parent = vic1; + interrupts = 5; + clock-names = uart, clk_uart_baud2, + clk_uart_baud3; + clocks = clock PCLK_UART0, clocks PCLK_UART0, + clock SCLK_UART; + status = disabled; + }; diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile index 5d4d432..3413380 100644 --- a/drivers/clk/samsung/Makefile +++ b/drivers/clk/samsung/Makefile @@ -8,3 +8,6 @@ obj-$(CONFIG_SOC_EXYNOS5250)+= clk-exynos5250.o obj-$(CONFIG_SOC_EXYNOS5420) += clk-exynos5420.o obj-$(CONFIG_SOC_EXYNOS5440) += clk-exynos5440.o obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos-audss.o +ifdef CONFIG_COMMON_CLK +obj-$(CONFIG_ARCH_S3C64XX) += clk-s3c64xx.o +endif diff --git a/drivers/clk/samsung/clk-s3c64xx.c b/drivers/clk/samsung/clk-s3c64xx.c new file mode 100644 index 000..eeda567 --- /dev/null +++ b/drivers/clk/samsung/clk-s3c64xx.c @@ -0,0 +1,473 @@ +/* + * Copyright (c) 2013 Tomasz Figa tomasz.figa at gmail.com + * + * This program is free software; you can redistribute
[PATCH v2 0/8] Common Clock Framework support for Samsung S3C64xx
This series is an attempt to move clock support on Samsung S3C64xx SoCs to Common Clock Framework. First, support for PLL types present on S3C64xx SoCs is added to Samsung Common Clock Framework driver. Then the main clock driver for mentioned SoCs is introduced. Further patches contain fixes for drivers to make them compliant with CCF semantics, migration of platform code to use the new clock driver and removal of old clock management code. Depends on: - [PATCH v4 00/20] Samsung PWM support cleanup http://thread.gmane.org/gmane.linux.kernel.samsung-soc/20856 On S3C6410-based Tiny6410 board (Mini6410-compatible): Tested-by: Tomasz Figa tomasz.f...@gmail.com For v1: Acked-by: Mike Turquette mturque...@linaro.org Changes since v1: - added patch for read-only muxes, - exported configurable muxes and dividers, - defined mout_syncmux as read-only mux, - in DT-enabled case fixed-clock binding is used to define external clocks. Tomasz Figa (8): clk: mux: Add support for read-only muxes. clk: samsung: pll: Add support for PLL6552 and PLL6553 clk: samsung: Add clock driver for S3C64xx SoCs ARM: SAMSUNG: Add soc_is_s3c6400/s3c6410 macros ARM: s3c64xx: dma: Use clk_prepare_enable/clk_disable_unprepare usb: host: ohci-s3c2410 Use clk_prepare_enable/clk_disable_unprepare ARM: s3c64xx: Migrate clock handling to Common Clock Framework ARM: s3c64xx: Remove old clock management code .../bindings/clock/samsung,s3c64xx-clock.txt | 77 ++ arch/arm/Kconfig |2 +- arch/arm/mach-s3c64xx/Makefile |2 +- arch/arm/mach-s3c64xx/clock.c | 1007 arch/arm/mach-s3c64xx/common.c | 21 +- arch/arm/mach-s3c64xx/common.h | 12 +- arch/arm/mach-s3c64xx/dma.c|4 +- arch/arm/mach-s3c64xx/include/mach/regs-clock.h| 132 +-- arch/arm/mach-s3c64xx/mach-anw6410.c |2 +- arch/arm/mach-s3c64xx/mach-crag6410.c |2 +- arch/arm/mach-s3c64xx/mach-hmt.c |2 +- arch/arm/mach-s3c64xx/mach-mini6410.c |2 +- arch/arm/mach-s3c64xx/mach-ncp.c |2 +- arch/arm/mach-s3c64xx/mach-smartq.c| 11 +- arch/arm/mach-s3c64xx/mach-smdk6400.c |2 +- arch/arm/mach-s3c64xx/mach-smdk6410.c |2 +- arch/arm/mach-s3c64xx/pm.c | 21 - arch/arm/mach-s3c64xx/s3c6400.c|6 - arch/arm/mach-s3c64xx/s3c6410.c|7 - arch/arm/plat-samsung/include/plat/cpu.h |4 + drivers/clk/clk-mux.c | 10 +- drivers/clk/samsung/Makefile |1 + drivers/clk/samsung/clk-pll.c | 160 drivers/clk/samsung/clk-pll.h |4 + drivers/clk/samsung/clk-s3c64xx.c | 465 + drivers/usb/host/ohci-s3c2410.c|8 +- include/dt-bindings/clock/samsung,s3c64xx-clock.h | 178 include/linux/clk-provider.h |2 + 28 files changed, 943 insertions(+), 1205 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/samsung,s3c64xx-clock.txt delete mode 100644 arch/arm/mach-s3c64xx/clock.c create mode 100644 drivers/clk/samsung/clk-s3c64xx.c create mode 100644 include/dt-bindings/clock/samsung,s3c64xx-clock.h -- 1.8.3.2 -- 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 v2 2/8] clk: samsung: pll: Add support for PLL6552 and PLL6553
This patch adds support for PLL6552 and PLL6553 PLLs present on Samsung S3C64xx SoCs. Signed-off-by: Tomasz Figa tomasz.f...@gmail.com Acked-by: Mike Turquette mturque...@linaro.org --- drivers/clk/samsung/clk-pll.c | 160 ++ drivers/clk/samsung/clk-pll.h | 4 ++ 2 files changed, 164 insertions(+) diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c index 362f12d..698e562 100644 --- a/drivers/clk/samsung/clk-pll.c +++ b/drivers/clk/samsung/clk-pll.c @@ -337,6 +337,166 @@ struct clk * __init samsung_clk_register_pll46xx(const char *name, } /* + * PLL6552 Clock Type + */ + +#define PLL6552_LOCK_REG 0x00 +#define PLL6552_CON_REG0x0c + +#define PLL6552_MDIV_MASK 0x3ff +#define PLL6552_PDIV_MASK 0x3f +#define PLL6552_SDIV_MASK 0x7 +#define PLL6552_MDIV_SHIFT 16 +#define PLL6552_PDIV_SHIFT 8 +#define PLL6552_SDIV_SHIFT 0 + +struct samsung_clk_pll6552 { + struct clk_hw hw; + void __iomem *reg_base; +}; + +#define to_clk_pll6552(_hw) container_of(_hw, struct samsung_clk_pll6552, hw) + +static unsigned long samsung_pll6552_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct samsung_clk_pll6552 *pll = to_clk_pll6552(hw); + u32 mdiv, pdiv, sdiv, pll_con; + u64 fvco = parent_rate; + + pll_con = __raw_readl(pll-reg_base + PLL6552_CON_REG); + mdiv = (pll_con PLL6552_MDIV_SHIFT) PLL6552_MDIV_MASK; + pdiv = (pll_con PLL6552_PDIV_SHIFT) PLL6552_PDIV_MASK; + sdiv = (pll_con PLL6552_SDIV_SHIFT) PLL6552_SDIV_MASK; + + fvco *= mdiv; + do_div(fvco, (pdiv sdiv)); + + return (unsigned long)fvco; +} + +static const struct clk_ops samsung_pll6552_clk_ops = { + .recalc_rate = samsung_pll6552_recalc_rate, +}; + +struct clk * __init samsung_clk_register_pll6552(const char *name, + const char *pname, void __iomem *base) +{ + struct samsung_clk_pll6552 *pll; + struct clk *clk; + struct clk_init_data init; + + pll = kzalloc(sizeof(*pll), GFP_KERNEL); + if (!pll) { + pr_err(%s: could not allocate pll clk %s\n, __func__, name); + return NULL; + } + + init.name = name; + init.ops = samsung_pll6552_clk_ops; + init.parent_names = pname; + init.num_parents = 1; + + pll-hw.init = init; + pll-reg_base = base; + + clk = clk_register(NULL, pll-hw); + if (IS_ERR(clk)) { + pr_err(%s: failed to register pll clock %s\n, __func__, + name); + kfree(pll); + } + + if (clk_register_clkdev(clk, name, NULL)) + pr_err(%s: failed to register lookup for %s, __func__, name); + + return clk; +} + +/* + * PLL6553 Clock Type + */ + +#define PLL6553_LOCK_REG 0x00 +#define PLL6553_CON0_REG 0x0c +#define PLL6553_CON1_REG 0x10 + +#define PLL6553_MDIV_MASK 0xff +#define PLL6553_PDIV_MASK 0x3f +#define PLL6553_SDIV_MASK 0x7 +#define PLL6553_KDIV_MASK 0x +#define PLL6553_MDIV_SHIFT 16 +#define PLL6553_PDIV_SHIFT 8 +#define PLL6553_SDIV_SHIFT 0 +#define PLL6553_KDIV_SHIFT 0 + +struct samsung_clk_pll6553 { + struct clk_hw hw; + void __iomem *reg_base; +}; + +#define to_clk_pll6553(_hw) container_of(_hw, struct samsung_clk_pll6553, hw) + +static unsigned long samsung_pll6553_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct samsung_clk_pll6553 *pll = to_clk_pll6553(hw); + u32 mdiv, pdiv, sdiv, kdiv, pll_con0, pll_con1; + u64 fvco = parent_rate; + + pll_con0 = __raw_readl(pll-reg_base + PLL6553_CON0_REG); + pll_con1 = __raw_readl(pll-reg_base + PLL6553_CON1_REG); + mdiv = (pll_con0 PLL6553_MDIV_SHIFT) PLL6553_MDIV_MASK; + pdiv = (pll_con0 PLL6553_PDIV_SHIFT) PLL6553_PDIV_MASK; + sdiv = (pll_con0 PLL6553_SDIV_SHIFT) PLL6553_SDIV_MASK; + kdiv = (pll_con1 PLL6553_KDIV_SHIFT) PLL6553_KDIV_MASK; + + fvco *= (mdiv 16) + kdiv; + do_div(fvco, (pdiv sdiv)); + fvco = 16; + + return (unsigned long)fvco; +} + +static const struct clk_ops samsung_pll6553_clk_ops = { + .recalc_rate = samsung_pll6553_recalc_rate, +}; + +struct clk * __init samsung_clk_register_pll6553(const char *name, + const char *pname, void __iomem *base) +{ + struct samsung_clk_pll6553 *pll; + struct clk *clk; + struct clk_init_data init; + + pll = kzalloc(sizeof(*pll), GFP_KERNEL); + if (!pll) { + pr_err(%s: could not allocate pll clk %s\n, __func__, name); + return NULL; + } + + init.name = name; + init.ops = samsung_pll6553_clk_ops; + init.parent_names = pname
[PATCH v2 4/8] ARM: SAMSUNG: Add soc_is_s3c6400/s3c6410 macros
This patch adds soc_is_s3c6400() and soc_is_s3c6410() macros that allow to distinguish between specific SoCs from s3c64xx series that is needed to handle differences between them. Signed-off-by: Tomasz Figa tomasz.f...@gmail.com --- arch/arm/plat-samsung/include/plat/cpu.h | 4 1 file changed, 4 insertions(+) diff --git a/arch/arm/plat-samsung/include/plat/cpu.h b/arch/arm/plat-samsung/include/plat/cpu.h index 4fb1f03..335beb3 100644 --- a/arch/arm/plat-samsung/include/plat/cpu.h +++ b/arch/arm/plat-samsung/include/plat/cpu.h @@ -87,8 +87,12 @@ IS_SAMSUNG_CPU(exynos5440, EXYNOS5440_SOC_ID, EXYNOS5_SOC_MASK) #endif #if defined(CONFIG_CPU_S3C6400) || defined(CONFIG_CPU_S3C6410) +# define soc_is_s3c6400() is_samsung_s3c6400() +# define soc_is_s3c6410() is_samsung_s3c6410() # define soc_is_s3c64xx() (is_samsung_s3c6400() || is_samsung_s3c6410()) #else +# define soc_is_s3c6400() 0 +# define soc_is_s3c6410() 0 # define soc_is_s3c64xx() 0 #endif -- 1.8.3.2 -- 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 v2 3/8] clk: samsung: Add clock driver for S3C64xx SoCs
This patch adds new, Common Clock Framework-based clock driver for Samsung S3C64xx SoCs. The driver is just added, without actually letting the platforms use it yet, since this requires more intermediate steps. Signed-off-by: Tomasz Figa tomasz.f...@gmail.com Acked-by: Mike Turquette mturque...@linaro.org --- .../bindings/clock/samsung,s3c64xx-clock.txt | 77 drivers/clk/samsung/Makefile | 3 + drivers/clk/samsung/clk-s3c64xx.c | 465 + include/dt-bindings/clock/samsung,s3c64xx-clock.h | 178 4 files changed, 723 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/samsung,s3c64xx-clock.txt create mode 100644 drivers/clk/samsung/clk-s3c64xx.c create mode 100644 include/dt-bindings/clock/samsung,s3c64xx-clock.h diff --git a/Documentation/devicetree/bindings/clock/samsung,s3c64xx-clock.txt b/Documentation/devicetree/bindings/clock/samsung,s3c64xx-clock.txt new file mode 100644 index 000..fa171dc --- /dev/null +++ b/Documentation/devicetree/bindings/clock/samsung,s3c64xx-clock.txt @@ -0,0 +1,77 @@ +* Samsung S3C64xx Clock Controller + +The S3C64xx clock controller generates and supplies clock to various controllers +within the SoC. The clock binding described here is applicable to all SoCs in +the S3C64xx family. + +Required Properties: + +- compatible: should be one of the following. + - samsung,s3c6400-clock - controller compatible with S3C6400 SoC. + - samsung,s3c6410-clock - controller compatible with S3C6410 SoC. + +- reg: physical base address of the controller and length of memory mapped + region. + +- #clock-cells: should be 1. + +Each clock is assigned an identifier and client nodes can use this identifier +to specify the clock which they consume. Some of the clocks are available only +on a particular S3C64xx SoC and this is specified where applicable. + +All available clocks are defined as preprocessor macros in +dt-bindings/clock/samsung,s3c64xx-clock.h header and can be used in device +tree sources. + +External clocks: + +There are several clocks that are generated outside the SoC. It is expected +that they are defined using standard clock bindings with following +clock-output-names: + - fin_pll - PLL input clock (xtal/extclk) - required, + - xusbxti - USB xtal - required, + - iiscdclk0 - I2S0 codec clock - optional, + - iiscdclk1 - I2S1 codec clock - optional, + - iiscdclk2 - I2S2 codec clock - optional, + - pcmcdclk0 - PCM0 codec clock - optional, + - pcmcdclk1 - PCM1 codec clock - optional, only S3C6410. + +Example: Clock controller node: + + clock: clock-controller@7e00f000 { + compatible = samsung,s3c6410-clock; + reg = 0x7e00f000 0x1000; + #clock-cells = 1; + }; + +Example: Required external clocks: + + fin_pll: clock-fin-pll { + compatible = fixed-clock; + clock-output-names = fin_pll; + clock-frequency = 1200; + #clock-cells = 0; + }; + + xusbxti: clock-xusbxti { + compatible = fixed-clock; + clock-output-names = xusbxti; + clock-frequency = 4800; + #clock-cells = 0; + }; + +Example: UART controller node that consumes the clock generated by the clock + controller (refer to the standard clock bindings for information about + clocks and clock-names properties): + + uart0: serial@7f005000 { + compatible = samsung,s3c6400-uart; + reg = 0x7f005000 0x100; + interrupt-parent = vic1; + interrupts = 5; + clock-names = uart, clk_uart_baud2, + clk_uart_baud3; + clocks = clock PCLK_UART0, clocks PCLK_UART0, + clock SCLK_UART; + status = disabled; + }; diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile index 5d4d432..3413380 100644 --- a/drivers/clk/samsung/Makefile +++ b/drivers/clk/samsung/Makefile @@ -8,3 +8,6 @@ obj-$(CONFIG_SOC_EXYNOS5250)+= clk-exynos5250.o obj-$(CONFIG_SOC_EXYNOS5420) += clk-exynos5420.o obj-$(CONFIG_SOC_EXYNOS5440) += clk-exynos5440.o obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos-audss.o +ifdef CONFIG_COMMON_CLK +obj-$(CONFIG_ARCH_S3C64XX) += clk-s3c64xx.o +endif diff --git a/drivers/clk/samsung/clk-s3c64xx.c b/drivers/clk/samsung/clk-s3c64xx.c new file mode 100644 index 000..6511f78 --- /dev/null +++ b/drivers/clk/samsung/clk-s3c64xx.c @@ -0,0 +1,465 @@ +/* + * Copyright (c) 2013 Tomasz Figa tomasz.figa at gmail.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * Common Clock Framework support for all S3C64xx SoCs
[PATCH v2 7/8] ARM: s3c64xx: Migrate clock handling to Common Clock Framework
This patch migrates the s3c64xx platform to use the new clock driver using Common Clock Framework. Signed-off-by: Tomasz Figa tomasz.f...@gmail.com --- arch/arm/Kconfig | 2 +- arch/arm/mach-s3c64xx/Makefile| 2 +- arch/arm/mach-s3c64xx/common.c| 21 + arch/arm/mach-s3c64xx/common.h| 10 +- arch/arm/mach-s3c64xx/mach-anw6410.c | 2 +- arch/arm/mach-s3c64xx/mach-crag6410.c | 2 +- arch/arm/mach-s3c64xx/mach-hmt.c | 2 +- arch/arm/mach-s3c64xx/mach-mini6410.c | 2 +- arch/arm/mach-s3c64xx/mach-ncp.c | 2 +- arch/arm/mach-s3c64xx/mach-smartq.c | 11 ++- arch/arm/mach-s3c64xx/mach-smdk6400.c | 2 +- arch/arm/mach-s3c64xx/mach-smdk6410.c | 2 +- arch/arm/mach-s3c64xx/s3c6400.c | 6 -- arch/arm/mach-s3c64xx/s3c6410.c | 7 --- drivers/clk/samsung/Makefile | 2 -- 15 files changed, 33 insertions(+), 42 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index fa7ffca..cfa7e59 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -724,6 +724,7 @@ config ARCH_S3C64XX select ARM_VIC select CLKDEV_LOOKUP select CLKSRC_SAMSUNG_PWM + select COMMON_CLK select CPU_V6 select GENERIC_CLOCKEVENTS select GPIO_SAMSUNG @@ -737,7 +738,6 @@ config ARCH_S3C64XX select S3C_DEV_NAND select S3C_GPIO_TRACK select SAMSUNG_ATAGS - select SAMSUNG_CLKSRC select SAMSUNG_GPIOLIB_4BIT select SAMSUNG_WDT_RESET select USB_ARCH_HAS_OHCI diff --git a/arch/arm/mach-s3c64xx/Makefile b/arch/arm/mach-s3c64xx/Makefile index 31d0c91..645a8fe 100644 --- a/arch/arm/mach-s3c64xx/Makefile +++ b/arch/arm/mach-s3c64xx/Makefile @@ -12,7 +12,7 @@ obj- := # Core -obj-y += common.o clock.o +obj-y += common.o # Core support diff --git a/arch/arm/mach-s3c64xx/common.c b/arch/arm/mach-s3c64xx/common.c index 73d79cf..7d3cb58 100644 --- a/arch/arm/mach-s3c64xx/common.c +++ b/arch/arm/mach-s3c64xx/common.c @@ -17,6 +17,7 @@ #include linux/kernel.h #include linux/init.h #include linux/module.h +#include linux/clk-provider.h #include linux/interrupt.h #include linux/ioport.h #include linux/serial_core.h @@ -38,7 +39,6 @@ #include mach/regs-gpio.h #include plat/cpu.h -#include plat/clock.h #include plat/devs.h #include plat/pm.h #include plat/gpio-cfg.h @@ -50,6 +50,19 @@ #include common.h +/* External clock frequency */ +static unsigned long xtal_f = 1200, xusbxti_f = 4800; + +void __init s3c64xx_set_xtal_freq(unsigned long freq) +{ + xtal_f = freq; +} + +void __init s3c64xx_set_xusbxti_freq(unsigned long freq) +{ + xusbxti_f = freq; +} + /* uart registration process */ static void __init s3c64xx_init_uarts(struct s3c2410_uartcfg *cfg, int no) @@ -67,7 +80,6 @@ static struct cpu_table cpu_ids[] __initdata = { .idcode = S3C6400_CPU_ID, .idmask = S3C64XX_CPU_MASK, .map_io = s3c6400_map_io, - .init_clocks= s3c6400_init_clocks, .init_uarts = s3c64xx_init_uarts, .init = s3c6400_init, .name = name_s3c6400, @@ -75,7 +87,6 @@ static struct cpu_table cpu_ids[] __initdata = { .idcode = S3C6410_CPU_ID, .idmask = S3C64XX_CPU_MASK, .map_io = s3c6410_map_io, - .init_clocks= s3c6410_init_clocks, .init_uarts = s3c64xx_init_uarts, .init = s3c6410_init, .name = name_s3c6410, @@ -213,8 +224,10 @@ void __init s3c64xx_init_irq(u32 vic0_valid, u32 vic1_valid) { /* * FIXME: there is no better place to put this at the moment -* (samsung_wdt_reset_init needs clocks) +* (s3c64xx_clk_init needs ioremap and must happen before init_time +* samsung_wdt_reset_init needs clocks) */ + s3c64xx_clk_init(NULL, xtal_f, xusbxti_f, soc_is_s3c6400(), S3C_VA_SYS); samsung_wdt_reset_init(S3C_VA_WATCHDOG); printk(KERN_DEBUG %s: initialising interrupts\n, __func__); diff --git a/arch/arm/mach-s3c64xx/common.h b/arch/arm/mach-s3c64xx/common.h index e8f990b..a2af0e1 100644 --- a/arch/arm/mach-s3c64xx/common.h +++ b/arch/arm/mach-s3c64xx/common.h @@ -22,18 +22,19 @@ void s3c64xx_init_irq(u32 vic0, u32 vic1); void s3c64xx_init_io(struct map_desc *mach_desc, int size); -void s3c64xx_register_clocks(unsigned long xtal, unsigned armclk_limit); -void s3c64xx_setup_clocks(void); - void s3c64xx_restart(enum reboot_mode mode, const char *cmd); void s3c64xx_init_late(void); +void s3c64xx_clk_init(struct device_node *np, unsigned long xtal_f, + unsigned long xusbxti_f, bool is_s3c6400, void __iomem *reg_base); +void s3c64xx_set_xtal_freq
[PATCH v2 5/8] ARM: s3c64xx: dma: Use clk_prepare_enable/clk_disable_unprepare
This patch modifies s3c64xx DMA driver to prepare and unprepare clocks in addition to enableind and disabling, since it is required by common clock framework. Signed-off-by: Tomasz Figa tomasz.f...@gmail.com --- arch/arm/mach-s3c64xx/dma.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-s3c64xx/dma.c b/arch/arm/mach-s3c64xx/dma.c index 759846c..c511dfa 100644 --- a/arch/arm/mach-s3c64xx/dma.c +++ b/arch/arm/mach-s3c64xx/dma.c @@ -677,7 +677,7 @@ static int s3c64xx_dma_init1(int chno, enum dma_ch chbase, goto err_map; } - clk_enable(dmac-clk); + clk_prepare_enable(dmac-clk); dmac-regs = regs; dmac-chanbase = chbase; @@ -711,7 +711,7 @@ static int s3c64xx_dma_init1(int chno, enum dma_ch chbase, return 0; err_clk: - clk_disable(dmac-clk); + clk_disable_unprepare(dmac-clk); clk_put(dmac-clk); err_map: iounmap(regs); -- 1.8.3.2 -- 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 v2 8/8] ARM: s3c64xx: Remove old clock management code
This patch removes old clock management code of S3C64xx, since the platform has been already moved to the new clock driver using Common Clock Framework. Signed-off-by: Tomasz Figa tomasz.f...@gmail.com --- arch/arm/mach-s3c64xx/clock.c | 1007 --- arch/arm/mach-s3c64xx/common.h |2 - arch/arm/mach-s3c64xx/include/mach/regs-clock.h | 132 +-- arch/arm/mach-s3c64xx/pm.c | 21 - 4 files changed, 4 insertions(+), 1158 deletions(-) delete mode 100644 arch/arm/mach-s3c64xx/clock.c diff --git a/arch/arm/mach-s3c64xx/clock.c b/arch/arm/mach-s3c64xx/clock.c deleted file mode 100644 index c1bcc4a..000 --- a/arch/arm/mach-s3c64xx/clock.c +++ /dev/null @@ -1,1007 +0,0 @@ -/* linux/arch/arm/plat-s3c64xx/clock.c - * - * Copyright 2008 Openmoko, Inc. - * Copyright 2008 Simtec Electronics - * Ben Dooks b...@simtec.co.uk - * http://armlinux.simtec.co.uk/ - * - * S3C64XX Base clock support - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. -*/ - -#include linux/init.h -#include linux/module.h -#include linux/interrupt.h -#include linux/ioport.h -#include linux/clk.h -#include linux/err.h -#include linux/io.h - -#include mach/hardware.h -#include mach/map.h - -#include mach/regs-clock.h - -#include plat/cpu.h -#include plat/devs.h -#include plat/cpu-freq.h -#include plat/clock.h -#include plat/clock-clksrc.h -#include plat/pll.h - -#include regs-sys.h - -/* fin_apll, fin_mpll and fin_epll are all the same clock, which we call - * ext_xtal_mux for want of an actual name from the manual. -*/ - -static struct clk clk_ext_xtal_mux = { - .name = ext_xtal, -}; - -#define clk_fin_apll clk_ext_xtal_mux -#define clk_fin_mpll clk_ext_xtal_mux -#define clk_fin_epll clk_ext_xtal_mux - -#define clk_fout_mpll clk_mpll -#define clk_fout_epll clk_epll - -struct clk clk_h2 = { - .name = hclk2, - .rate = 0, -}; - -struct clk clk_27m = { - .name = clk_27m, - .rate = 2700, -}; - -static int clk_48m_ctrl(struct clk *clk, int enable) -{ - unsigned long flags; - u32 val; - - /* can't rely on clock lock, this register has other usages */ - local_irq_save(flags); - - val = __raw_readl(S3C64XX_OTHERS); - if (enable) - val |= S3C64XX_OTHERS_USBMASK; - else - val = ~S3C64XX_OTHERS_USBMASK; - - __raw_writel(val, S3C64XX_OTHERS); - local_irq_restore(flags); - - return 0; -} - -struct clk clk_48m = { - .name = clk_48m, - .rate = 4800, - .enable = clk_48m_ctrl, -}; - -struct clk clk_xusbxti = { - .name = xusbxti, - .rate = 4800, -}; - -static int inline s3c64xx_gate(void __iomem *reg, - struct clk *clk, - int enable) -{ - unsigned int ctrlbit = clk-ctrlbit; - u32 con; - - con = __raw_readl(reg); - - if (enable) - con |= ctrlbit; - else - con = ~ctrlbit; - - __raw_writel(con, reg); - return 0; -} - -static int s3c64xx_pclk_ctrl(struct clk *clk, int enable) -{ - return s3c64xx_gate(S3C_PCLK_GATE, clk, enable); -} - -static int s3c64xx_hclk_ctrl(struct clk *clk, int enable) -{ - return s3c64xx_gate(S3C_HCLK_GATE, clk, enable); -} - -int s3c64xx_sclk_ctrl(struct clk *clk, int enable) -{ - return s3c64xx_gate(S3C_SCLK_GATE, clk, enable); -} - -static struct clk init_clocks_off[] = { - { - .name = nand, - .parent = clk_h, - }, { - .name = rtc, - .parent = clk_p, - .enable = s3c64xx_pclk_ctrl, - .ctrlbit= S3C_CLKCON_PCLK_RTC, - }, { - .name = adc, - .parent = clk_p, - .enable = s3c64xx_pclk_ctrl, - .ctrlbit= S3C_CLKCON_PCLK_TSADC, - }, { - .name = i2c, - .devname= s3c2440-i2c.0, - .parent = clk_p, - .enable = s3c64xx_pclk_ctrl, - .ctrlbit= S3C_CLKCON_PCLK_IIC, - }, { - .name = i2c, - .devname= s3c2440-i2c.1, - .parent = clk_p, - .enable = s3c64xx_pclk_ctrl, - .ctrlbit= S3C6410_CLKCON_PCLK_I2C1, - }, { - .name = keypad, - .parent = clk_p, - .enable = s3c64xx_pclk_ctrl, - .ctrlbit= S3C_CLKCON_PCLK_KEYPAD, - }, { - .name = spi
Re: [PATCH v2 6/8] usb: host: ohci-s3c2410 Use clk_prepare_enable/clk_disable_unprepare
On Monday 22 of July 2013 21:15:12 Fabio Estevam wrote: On Mon, Jul 22, 2013 at 8:49 PM, Tomasz Figa tomasz.f...@gmail.com wrote: dev_dbg(dev-dev, s3c2410_start_hc:\n); - clk_enable(usb_clk); + clk_prepare_enable(usb_clk); clk_prepare_enable may fail, so you would better check its return value. Well, ideally yes, but since this driver doesn't have any error path here anyway and on Samsung platforms clk_prepare_enable() simply can't fail, I'd keep it this way until somebody fixes this driver, as it has more issues than just this one. Best regards, Tomasz -- 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
Hi, On Saturday 20 of July 2013 19:59:10 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 there is a confusion here about who registers the PHYs. All platform code does is registering a platform/i2c/whatever device, which causes a driver (located in drivers/phy/) to be instantiated. Such drivers call phy_create(), usually in their probe() callbacks, so platform_code has no way (and should have no way, for the sake of layering) to get what phy_create() returns. 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), /* etc... */ }; static void my_machine_init(void) { /* ... */ phy_register_lookup(my_phy_lookup, ARRAY_SIZE(my_phy_lookup)); /* ... */ } /* Notice nothing stuffed in platform data. */ [provider code - samsung-usbphy driver] static int samsung_usbphy_probe(...) { /* ... */ for (i = 0; i PHY_COUNT; ++i) { usbphy-phy[i].name = phy; usbphy-phy[i].id = i; /* ... */ ret = phy_create(usbphy-phy); /* err handling */ } /* ... */ } [consumer code - s3c-hsotg driver] static int s3c_hsotg_probe(...) { /* ... */ phy = phy_get(pdev-dev, otg); /* ... */ } [1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/252813 Best regards, Tomasz -- 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
On Sunday 21 of July 2013 16:37:33 Kishon Vijay Abraham I wrote: Hi, On Sunday 21 July 2013 04:01 PM, Tomasz Figa wrote: Hi, On Saturday 20 of July 2013 19:59:10 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 there is a confusion here about who registers the PHYs. All platform code does is registering a platform/i2c/whatever device, which causes a driver (located in drivers/phy/) to be instantiated. Such drivers call phy_create(), usually in their probe() callbacks, so platform_code has no way (and should have no way, for the sake of layering) to get what phy_create() returns. right. 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. Best regards, Tomasz -- 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-usb2: Toggle HSIC GPIO from device tree
Hi, On Friday 12 of July 2013 09:48:54 Jingoo Han wrote: On Friday, July 12, 2013 6:46 AM, Julius Werner wrote: Hi Jingoo, Yeah, I followed that discussion back then, but it seems to have stalled a little (at least the HSIC patches haven't been picked up in any kernel.org repo yet to my knowledge). The problem is that I think these approaches cannot work reliably. I agree that it would be nice to control the HSIC device from its own driver, and have spent quite some time playing around with the usb/misc/usb3503.c driver to try to make this work... but there's a timing dependency here that you just can't model correctly with independent drivers. If the HSIC device is already active during boot (e.g. because it was used by firmware), there's always a chance that the USB stack will come up before the driver that resets it does. The device will be enumerated as normal, and when the other driver later pulls the reset signal the USB stack will not notice because there is no real disconnect detection on HSIC. Only when you eventually try to send another transfer to the device will you start to get timeouts, and the newly reset device will not be able to reenumerate because the host never asks it to. I really don't see how you could solve this without putting some kind of synchronization mechanism in the USB drivers. So this leaves ehci-s5p and phy-samsung-usb2 as the only possible places, and I chose the latter since all the host-side HSIC initialization is also there already. I think if you think of it as reset whatever is on the other side of this PHY, it's okay to put it as an optional feature into the PHY driver. CC'ed Tomasz Figa, Dongjin Kim, Yulgon Kim Hi Tomasz, Dongjin, Julius Werner wants to put 'SMSC 3503 hub reset on Arndale board' to 'phy-samsung-usb*.c' files, because there is a timing dependency above mentioned. The following is the original patch sent by Julius Werner two day ago. (http://www.spinics.net/lists/linux-samsung-soc/msg20250.html) Well, I think this is simply broken. Following are the reasons why I think so: a) you can use the smsc3503 chip on any USB/HSIC controller and with any USB/HSIC PHY, so you would have to add such reset GPIO to _every_ PHY or controller driver, b) you might want to use other USB/HSIC hubs like this one that requires some initialization sequence, other than just toggling a GPIO, so you would have to add cases for all of those hubs or other chips in _every_ PHY or controller driver. Previously, Olof mentioned that 'drivers/platform/arm/' would be used. (http://patches.linaro.org/16856/) Also, another way was mentioned by Fabio Estevam, using 'drivers/reset/gpio-reset.c' which is not merged yet. (http://permalink.gmane.org/gmane.linux.drivers.devicetree/36830) I think that 'phy-samsung-usb*.c' files are not a good place. However, Julius Werner's comment looks reasonable enough. I can see this problem almost equivalent to the problem with on-board WLAN adapters connected using SDIO. Those adapters require some kind of power on sequence before they can be enumerated using standard MMC/SDIO mechanisms and so does this USB/HSIC hub. So basically this is a thing that we should consider on a more generic level, not just for this particular single chip. CCing some extra people to increase chance of getting more opinions on this. Best regards, Tomasz -- 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 1/4] usb: gadget: s3c-hsotg: Allow driver instantiation using device tree
This patch adds OF match table to the driver to allow instantiating it using device tree. Signed-off-by: Tomasz Figa t.f...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- .../devicetree/bindings/usb/samsung-hsotg.txt | 40 ++ drivers/usb/gadget/s3c-hsotg.c | 10 ++ 2 files changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/samsung-hsotg.txt diff --git a/Documentation/devicetree/bindings/usb/samsung-hsotg.txt b/Documentation/devicetree/bindings/usb/samsung-hsotg.txt new file mode 100644 index 000..b83d428 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/samsung-hsotg.txt @@ -0,0 +1,40 @@ +Samsung High Speed USB OTG controller +- + +The Samsung HSOTG IP can be found on Samsung SoCs, from S3C6400 onwards. +It gives functionality of OTG-compliant USB 2.0 host and device with +support for USB 2.0 high-speed (480Mbps) and full-speed (12 Mbps) +operation. + +Currently only device mode is supported. + +Binding details +- + +Required properties: +- compatible: samsung,s3c6400-hsotg should be used for all currently +supported SoC, +- interrupt-parent: phandle for the interrupt controller to which the +interrupt signal of the HSOTG block is routed, +- interrupts: specifier of interrupt signal of interrupt controller, +according to bindings of interrupt controller, +- clocks: contains an array of clock specifiers: +- first entry: OTG clock +- clock-names: contains array of clock names: +- first entry: must be otg +- vusb_d-supply: phandle to voltage regulator of digital section, +- vusb_a-supply: phandle to voltage regulator of analog section. + +Example +- + + hsotg@1248 { + compatible = samsung,s3c6400-hsotg; + reg = 0x1248 0x2; + interrupts = 0 71 0; + clocks = clock 305; + clock-names = otg; + vusb_d-supply = vusb_reg; + vusb_a-supply = vusbdac_reg; + }; + diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c index af22f24..616ed51 100644 --- a/drivers/usb/gadget/s3c-hsotg.c +++ b/drivers/usb/gadget/s3c-hsotg.c @@ -29,6 +29,7 @@ #include linux/slab.h #include linux/clk.h #include linux/regulator/consumer.h +#include linux/of_platform.h #include linux/usb/ch9.h #include linux/usb/gadget.h @@ -3648,10 +3649,19 @@ static int s3c_hsotg_remove(struct platform_device *pdev) #define s3c_hsotg_resume NULL #endif +#ifdef CONFIG_OF +static const struct of_device_id s3c_hsotg_of_ids[] = { + { .compatible = samsung,s3c6400-hsotg, }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, s3c_hsotg_of_ids); +#endif + static struct platform_driver s3c_hsotg_driver = { .driver = { .name = s3c-hsotg, .owner = THIS_MODULE, + .of_match_table = of_match_ptr(s3c_hsotg_of_ids), }, .probe = s3c_hsotg_probe, .remove = s3c_hsotg_remove, -- 1.8.2.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
[PATCH 3/4] ARM: dts: exynos4: Add nodes for USB PHY block
This patch adds device tree nodes for USB OTG PHYs of Exynos4210 and Exynos4x12 SoCs. Signed-off-by: Tomasz Figa t.f...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- arch/arm/boot/dts/exynos4210.dtsi | 15 +++ arch/arm/boot/dts/exynos4x12.dtsi | 15 +++ 2 files changed, 30 insertions(+) diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi index 54710de..3c71ea5 100644 --- a/arch/arm/boot/dts/exynos4210.dtsi +++ b/arch/arm/boot/dts/exynos4210.dtsi @@ -114,6 +114,21 @@ interrupts = 2 4; }; + usbphy@125B { + compatible = samsung,exynos4210-usb2phy; + reg = 0x125B 0x100; + clocks = clock 305; + clock-names = otg; + status = disabled; + ranges; + #address-cells = 1; + #size-cells = 1; + + usbphy-sys { + reg = 0x10020704 0x8; + }; + }; + g2d@1280 { compatible = samsung,s5pv210-g2d; reg = 0x1280 0x1000; diff --git a/arch/arm/boot/dts/exynos4x12.dtsi b/arch/arm/boot/dts/exynos4x12.dtsi index e3380a7..267ed95 100644 --- a/arch/arm/boot/dts/exynos4x12.dtsi +++ b/arch/arm/boot/dts/exynos4x12.dtsi @@ -79,4 +79,19 @@ interrupts = 0 89 0; status = disabled; }; + + usbphy@125B { + compatible = samsung,exynos4x12-usb2phy; + reg = 0x125B 0x100; + clocks = clock 305; + clock-names = otg; + status = disabled; + ranges; + #address-cells = 1; + #size-cells = 1; + + usbphy-sys { + reg = 0x10020704 0x8; + }; + }; }; -- 1.8.2.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
[PATCH 0/4] Add support for Samsung HSOTG on DT-enabled platforms
This series enables platforms booting with Device Tree to use the Samsung HSOTG IP. Since USB PHY support has been already added, it's just a matter of adding an OF match table to the driver and respective device tree nodes to dts files. [On Samsung Trats board based on Exynos 4210] Tested-by: Tomasz Figa t.f...@samsung.com Tomasz Figa (4): usb: gadget: s3c-hsotg: Allow driver instantiation using device tree ARM: dts: exynos4: Add node for hsotg ARM: dts: exynos4: Add nodes for USB PHY block ARM: dts: exynos4210-trats: Enable USB gadget functionality .../devicetree/bindings/usb/samsung-hsotg.txt | 40 ++ arch/arm/boot/dts/exynos4.dtsi | 9 + arch/arm/boot/dts/exynos4210-trats.dts | 10 ++ arch/arm/boot/dts/exynos4210.dtsi | 15 arch/arm/boot/dts/exynos4x12.dtsi | 15 drivers/usb/gadget/s3c-hsotg.c | 10 ++ 6 files changed, 99 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/samsung-hsotg.txt -- 1.8.2.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
[PATCH 4/4] ARM: dts: exynos4210-trats: Enable USB gadget functionality
This patch adds device tree nodes necessary to enable USB gadget functionality on Exynos4210-based Trats board. Signed-off-by: Tomasz Figa t.f...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- arch/arm/boot/dts/exynos4210-trats.dts | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/arm/boot/dts/exynos4210-trats.dts b/arch/arm/boot/dts/exynos4210-trats.dts index 9a14484..d2ce1cf 100644 --- a/arch/arm/boot/dts/exynos4210-trats.dts +++ b/arch/arm/boot/dts/exynos4210-trats.dts @@ -39,6 +39,12 @@ enable-active-high; }; + hsotg@1248 { + vusb_d-supply = vusb_reg; + vusb_a-supply = vusbdac_reg; + status = okay; + }; + sdhci_emmc: sdhci@1251 { bus-width = 8; non-removable; @@ -48,6 +54,10 @@ status = okay; }; + usbphy@125B { + status = okay; + }; + serial@1380 { status = okay; }; -- 1.8.2.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
[PATCH 2/4] ARM: dts: exynos4: Add node for hsotg
This patch adds device tree node for USB HSOTG controller present on Exynos4 SoCs. Signed-off-by: Tomasz Figa t.f...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- arch/arm/boot/dts/exynos4.dtsi | 9 + 1 file changed, 9 insertions(+) diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi index 359694c..d53b9da 100644 --- a/arch/arm/boot/dts/exynos4.dtsi +++ b/arch/arm/boot/dts/exynos4.dtsi @@ -119,6 +119,15 @@ status = disabled; }; + hsotg@1248 { + compatible = samsung,s3c6400-hsotg; + reg = 0x1248 0x2; + interrupts = 0 71 0; + clocks = clock 305; + clock-names = otg; + status = disabled; + }; + sdhci@1251 { compatible = samsung,exynos4210-sdhci; reg = 0x1251 0x100; -- 1.8.2.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 1/4] usb: gadget: s3c-hsotg: Allow driver instantiation using device tree
On Tuesday 25 of June 2013 23:46:32 Felipe Balbi wrote: On Tue, Jun 25, 2013 at 05:38:23PM +0200, Tomasz Figa wrote: This patch adds OF match table to the driver to allow instantiating it using device tree. Signed-off-by: Tomasz Figa t.f...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com when will this driver get merged into dwc2 ? What's dwc2? I can only see dwc3 in mainline kernel tree and it doesn't seem to be compatible with s3c-hsotg. Best regards, Tomasz -- 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/4] usb: gadget: s3c-hsotg: Allow driver instantiation using device tree
On Wednesday 26 of June 2013 00:00:44 Felipe Balbi wrote: On Tue, Jun 25, 2013 at 10:57:06PM +0200, Tomasz Figa wrote: On Tuesday 25 of June 2013 23:46:32 Felipe Balbi wrote: On Tue, Jun 25, 2013 at 05:38:23PM +0200, Tomasz Figa wrote: This patch adds OF match table to the driver to allow instantiating it using device tree. Signed-off-by: Tomasz Figa t.f...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com when will this driver get merged into dwc2 ? What's dwc2? I can only see dwc3 in mainline kernel tree and it doesn't seem to be compatible with s3c-hsotg. drivers/staging/dwc2/ Oh right, staging, I forgot to check there. That's the same IP you're using Paul Zimmerman (now in Cc) is working towards getting that out of staging. It currently has only host mode, so plugging your code in there shouldn't be too difficult. OTOH, it might be better to make your code a device-only version of dwc2 (drivers/usb/dwc2) and have Paul move his host side to that instead. Hmm, I believe Lukasz Majewski and Marek Szyprowski (both on CC) should be able to say something more on this topic as I'm not really into USB drivers and just doing Device Tree enablement here. Best regards, Tomasz -- 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/7] Common Clock Framework support for Samsung S3C64xx
On Wednesday 19 of June 2013 23:17:06 Kukjin Kim wrote: On 06/06/13 08:57, Tomasz Figa wrote: This series is an attempt to move clock support on Samsung S3C64xx SoCs to Common Clock Framework. Looks good :) Thanks. First, support for PLL types present on S3C64xx SoCs is added to Samsung Common Clock Framework driver. Then the main clock driver for mentioned SoCs is introduced. Further patches contain fixes for drivers to make them compliant with CCF semantics, migration of platform code to use the new clock driver and removal of old clock management code. Depends on: - [PATCH 0/6] Samsung watchdog support clean-up http://thread.gmane.org/gmane.linux.kernel.samsung-soc/18736/focus= 18989 - [PATCH 00/15] Final Samsung PWM support cleanup http://www.spinics.net/lists/arm-kernel/msg248725.html BTW, Tomasz, so how was going on above PWM patches? I have them ready now, but the PWM maintainer has some objections, which will hopefully be resolved soon. Best regards, Tomasz On S3C6410-based Tiny6410 board (Mini6410-compatible): Tested-by: Tomasz Figatomasz.f...@gmail.com Tomasz Figa (7): clk: samsung: pll: Add support for PLL6552 and PLL6553 clk: samsung: Add clock driver for S3C64xx SoCs ARM: SAMSUNG: Add soc_is_s3c6400/s3c6410 macros ARM: s3c64xx: dma: Use clk_prepare_enable/clk_disable_unprepare usb: host: ohci-s3c2410 Use clk_prepare_enable/clk_disable_unprepare ARM: s3c64xx: Migrate clock handling to Common Clock Framework ARM: s3c64xx: Remove old clock management code -- 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 -- 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 05/10] USB: OHCI: Properly handle ohci-exynos suspend
Hi Manjunath, On Thursday 13 of June 2013 14:46:24 Manjunath Goudar wrote: Suspend scenario in case of ohci-exynos glue was not properly handled as it was not suspending generic part of ohci controller.Calling explicitly the ohci_suspend() routine in exynos_ohci_suspend() will ensure proper handling of suspend scenario. V2: -Incase ohci_suspend() fails, return right away without executing further. Signed-off-by: Manjunath Goudar manjunath.gou...@linaro.org Cc: Arnd Bergmann a...@arndb.de Cc: Alan Stern st...@rowland.harvard.edu Cc: Greg KH g...@kroah.com Cc: linux-usb@vger.kernel.org --- drivers/usb/host/ohci-exynos.c |8 1 file changed, 8 insertions(+) diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c index 6ff830c..8fecb6a 100644 --- a/drivers/usb/host/ohci-exynos.c +++ b/drivers/usb/host/ohci-exynos.c @@ -203,9 +203,17 @@ static int exynos_ohci_suspend(struct device *dev) struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd); struct ohci_hcd *ohci = hcd_to_ohci(hcd); struct platform_device *pdev = to_platform_device(dev); + bool do_wakeup = device_may_wakeup(dev); unsigned long flags; int rc = 0; + rc = ohci_suspend(hcd, do_wakeup); + if (rc == 0 do_wakeup HCD_WAKEUP_PENDING(hcd)) { + ohci_resume(hcd, false); + rc = -EBUSY; + } I'm not into USB host subsystem, so I might just ask a stupid question. Can't we make ohci_suspend check this for us, so the drivers would just check for error code? It seems like in all your patches this part of code is duplicated, looking as a good candidate to be generic. Best regards, Tomasz + if (rc) + return rc; /* * Root hub was already suspended. Disable irq emission and * mark HW unaccessible, bail out if RH has been resumed. Use -- 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/7] clk: samsung: Add clock driver for S3C64xx SoCs
Hi Mike, On Tuesday 11 of June 2013 19:54:51 Mike Turquette wrote: Quoting Tomasz Figa (2013-06-05 16:57:26) This patch adds new, Common Clock Framework-based clock driver for Samsung S3C64xx SoCs. The driver is just added, without actually letting the platforms use it yet, since this requires more intermediate steps. It seems like there is an awful lot of clock data here that exists alongside the stuff in DT. Is this how you plan to keep things going forward or is this conversion just an intermediate step? Current S3C64xx support contains a lot of boards, for which I don't see any chance for DT conversion in any time soon, so the driver must cover both DT and non-DT cases. (Not even saying that DT support for S3C64xx is not yet submitted, as I want to get all the dependencies merged, or at least acked, first.) Also, personally, I don't see anything wrong with having those clocks defined in the driver. The binding specifies the exact mapping between clock IDs inside the clock provider and hardware clocks and not all clocks need to be exported (most of muxes and divs don't need to), so I find it more reasonable to define them in the driver instead. Another thing is that it's unlikely for any new SoC from S3C64xx series to show up, so basically the clock list is fixed. Best regards, Tomasz Regards, Mike Signed-off-by: Tomasz Figa tomasz.f...@gmail.com --- .../bindings/clock/samsung,s3c64xx-clock.txt | 48 ++ drivers/clk/samsung/Makefile | 3 + drivers/clk/samsung/clk-s3c64xx.c | 503 + include/dt-bindings/clock/samsung,s3c64xx-clock.h | 144 ++ 4 files changed, 698 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/samsung,s3c64xx-clock.txt create mode 100644 drivers/clk/samsung/clk-s3c64xx.c create mode 100644 include/dt-bindings/clock/samsung,s3c64xx-clock.h diff --git a/Documentation/devicetree/bindings/clock/samsung,s3c64xx-clock.txt b/Documentation/devicetree/bindings/clock/samsung,s3c64xx-clock.txt new file mode 100644 index 000..278ab6e --- /dev/null +++ b/Documentation/devicetree/bindings/clock/samsung,s3c64xx-clock.txt @@ -0,0 +1,48 @@ +* Samsung S3C64xx Clock Controller + +The S3C64xx clock controller generates and supplies clock to various controllers +within the SoC. The clock binding described here is applicable to all SoCs in +the S3C64xx family. + +Required Properties: + +- comptible: should be one of the following. + - samsung,s3c6400-clock - controller compatible with S3C6400 SoC. + - samsung,s3c6410-clock - controller compatible with S3C6410 SoC. + +- reg: physical base address of the controller and length of memory mapped + region. + +- #clock-cells: should be 1. + +Each clock is assigned an identifier and client nodes can use this identifier +to specify the clock which they consume. Some of the clocks are available only +on a particular S3C64xx SoC and this is specified where applicable. + +All available clocks are defined as preprocessor macros in +dt-bindings/clock/samsung,s3c64xx-clock.h header and can be used in device +tree sources. + +Example: Clock controller node: + + clocks: clock-controller@7e00f000 { + compatible = samsung,s3c6410-clock; + reg = 0x7e00f000 0x1000; + #clock-cells = 1; + }; + +Example: UART controller node that consumes the clock generated by the clock + controller (refer to the standard clock bindings for information about + clocks and clock-names properties): + + uart0: serial@7f005000 { + compatible = samsung,s3c6400-uart; + reg = 0x7f005000 0x100; + interrupt-parent = vic1; + interrupts = 5; + clock-names = uart, clk_uart_baud2, + clk_uart_baud3; + clocks = clocks PCLK_UART0, clocks PCLK_UART0, + clocks SCLK_UART; + status = disabled; + }; diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile index b7c232e..c023474 100644 --- a/drivers/clk/samsung/Makefile +++ b/drivers/clk/samsung/Makefile @@ -6,3 +6,6 @@ obj-$(CONFIG_COMMON_CLK)+= clk.o clk-pll.o obj-$(CONFIG_ARCH_EXYNOS4) += clk-exynos4.o obj-$(CONFIG_SOC_EXYNOS5250) += clk-exynos5250.o obj-$(CONFIG_SOC_EXYNOS5440) += clk-exynos5440.o +ifdef CONFIG_COMMON_CLK +obj-$(CONFIG_ARCH_S3C64XX) += clk-s3c64xx.o +endif diff --git a/drivers/clk/samsung/clk-s3c64xx.c b/drivers/clk/samsung/clk-s3c64xx.c new file mode 100644 index 000..253a972 --- /dev/null +++ b/drivers/clk/samsung/clk-s3c64xx.c @@ -0,0 +1,503 @@ +/* + * Copyright (c