From: Alan Stern <stern@rowland.harvard.edu> To: Kamil Debski <k.debski@samsung.com> Cc: linux-kernel@vger.kernel.org, <linux-samsung-soc@vger.kernel.org>, <linux-usb@vger.kernel.org>, <devicetree@vger.kernel.org>, <kyungmin.park@samsung.com>, <kishon@ti.com>, <t.figa@samsung.com>, <s.nawrocki@samsung.com>, <m.szyprowski@samsung.com>, <gautam.vivek@samsung.com>, <mat.krawczuk@gmail.com>, <yulgon.kim@samsung.com>, <p.paneri@samsung.com>, <av.tikhomirov@samsung.com>, <jg1.han@samsung.com>, <galak@codeaurora.org>, <matt.porter@linaro.org>, <tjakobi@math.uni-bielefeld.de> Subject: Re: [PATCH v6 8/8] usb: ehci-exynos: Change to use phy provided by the generic phy framework Date: Wed, 29 Jan 2014 15:42:49 -0500 (EST) [thread overview] Message-ID: <Pine.LNX.4.44L0.1401291530001.23560-100000@netrider.rowland.org> (raw) In-Reply-To: <1391016574-25237-9-git-send-email-k.debski@samsung.com> 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.debski@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? > @@ -42,10 +42,10 @@ > 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; You have removed all the OTG stuff from the driver. This wasn't mentioned in the patch description, and it has no connection with the PHY work. > + struct phy *phy[PHY_NUMBER]; > }; > > #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv) > @@ -69,13 +69,43 @@ static void exynos_setup_vbus_gpio(struct platform_device *pdev) > dev_err(dev, "can't request ehci vbus gpio %d", gpio); > } > > +static int exynos_phys_on(struct phy *p[]) > +{ > + int i; > + int ret = 0; > + > + for (i = 0; ret == 0 && i < PHY_NUMBER; i++) > + if (p[i]) > + ret = phy_power_on(p[i]); > + if (ret) > + for (i--; i > 0; i--) > + if (p[i]) > + phy_power_off(p[i]); This loop runs while i > 0. Therefore you will never turn off the power to p[0]. > @@ -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); You omitted this line from the new error returns below. > - 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; Here, for example. Wouldn't it be better to goto fail_clk? Alan Stern
WARNING: multiple messages have this Message-ID (diff)
From: Alan Stern <stern@rowland.harvard.edu> To: Kamil Debski <k.debski@samsung.com> Cc: linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org, linux-usb@vger.kernel.org, devicetree@vger.kernel.org, kyungmin.park@samsung.com, kishon@ti.com, t.figa@samsung.com, s.nawrocki@samsung.com, m.szyprowski@samsung.com, gautam.vivek@samsung.com, mat.krawczuk@gmail.com, yulgon.kim@samsung.com, p.paneri@samsung.com, av.tikhomirov@samsung.com, jg1.han@samsung.com, galak@codeaurora.org, matt.porter@linaro.org, tjakobi@math.uni-bielefeld.de Subject: Re: [PATCH v6 8/8] usb: ehci-exynos: Change to use phy provided by the generic phy framework Date: Wed, 29 Jan 2014 15:42:49 -0500 (EST) [thread overview] Message-ID: <Pine.LNX.4.44L0.1401291530001.23560-100000@netrider.rowland.org> (raw) In-Reply-To: <1391016574-25237-9-git-send-email-k.debski@samsung.com> 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.debski@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? > @@ -42,10 +42,10 @@ > 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; You have removed all the OTG stuff from the driver. This wasn't mentioned in the patch description, and it has no connection with the PHY work. > + struct phy *phy[PHY_NUMBER]; > }; > > #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv) > @@ -69,13 +69,43 @@ static void exynos_setup_vbus_gpio(struct platform_device *pdev) > dev_err(dev, "can't request ehci vbus gpio %d", gpio); > } > > +static int exynos_phys_on(struct phy *p[]) > +{ > + int i; > + int ret = 0; > + > + for (i = 0; ret == 0 && i < PHY_NUMBER; i++) > + if (p[i]) > + ret = phy_power_on(p[i]); > + if (ret) > + for (i--; i > 0; i--) > + if (p[i]) > + phy_power_off(p[i]); This loop runs while i > 0. Therefore you will never turn off the power to p[0]. > @@ -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); You omitted this line from the new error returns below. > - 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; Here, for example. Wouldn't it be better to goto fail_clk? Alan Stern
next prev parent reply other threads:[~2014-01-29 20:42 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-01-29 17:29 [PATCH v6 0/8] phy: Add new Exynos USB 2.0 PHY driver Kamil Debski 2014-01-29 17:29 ` [PATCH v6 1/8] phy: core: Add an exported of_phy_get function Kamil Debski 2014-01-29 17:29 ` [PATCH v6 2/8] phy: core: Add devm_of_phy_get to phy-core Kamil Debski 2014-01-29 17:29 ` [PATCH v6 3/8] dts: Add usb2phy to Exynos 4 Kamil Debski 2014-01-29 20:50 ` Olof Johansson 2014-01-29 20:50 ` Olof Johansson 2014-02-05 15:57 ` Kamil Debski 2014-01-29 17:29 ` [PATCH v6 4/8] dts: Add usb2phy to Exynos 5250 Kamil Debski 2014-01-29 17:29 ` [PATCH v6 5/8] phy: Add new Exynos USB PHY driver Kamil Debski 2014-03-03 14:27 ` Kishon Vijay Abraham I 2014-03-03 14:27 ` Kishon Vijay Abraham I 2014-03-03 15:07 ` Kamil Debski 2014-03-03 15:07 ` Kamil Debski 2014-01-29 17:29 ` [PATCH v6 6/8] phy: Add support for S5PV210 to the " Kamil Debski 2014-01-29 17:29 ` Kamil Debski 2014-01-29 17:29 ` [PATCH v6 7/8] phy: Add Exynos 5250 support to the Exynos USB 2.0 " Kamil Debski 2014-01-29 17:29 ` [PATCH v6 8/8] usb: ehci-exynos: Change to use phy provided by the generic phy framework Kamil Debski 2014-01-29 20:42 ` Alan Stern [this message] 2014-01-29 20:42 ` Alan Stern 2014-02-04 15:06 ` Tomasz Figa 2014-02-05 15:57 ` Kamil Debski 2014-02-05 19:55 ` Alan Stern 2014-02-05 19:55 ` Alan Stern 2014-01-29 20:55 ` Olof Johansson 2014-01-29 20:55 ` Olof Johansson 2014-02-05 15:57 ` Kamil Debski 2014-02-05 17:30 ` Olof Johansson 2014-02-05 17:30 ` Olof Johansson 2014-03-03 14:37 ` Tomasz Figa 2014-03-03 14:37 ` Tomasz Figa
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=Pine.LNX.4.44L0.1401291530001.23560-100000@netrider.rowland.org \ --to=stern@rowland.harvard.edu \ --cc=av.tikhomirov@samsung.com \ --cc=devicetree@vger.kernel.org \ --cc=galak@codeaurora.org \ --cc=gautam.vivek@samsung.com \ --cc=jg1.han@samsung.com \ --cc=k.debski@samsung.com \ --cc=kishon@ti.com \ --cc=kyungmin.park@samsung.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=linux-usb@vger.kernel.org \ --cc=m.szyprowski@samsung.com \ --cc=mat.krawczuk@gmail.com \ --cc=matt.porter@linaro.org \ --cc=p.paneri@samsung.com \ --cc=s.nawrocki@samsung.com \ --cc=t.figa@samsung.com \ --cc=tjakobi@math.uni-bielefeld.de \ --cc=yulgon.kim@samsung.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.