* [PATCH 0/2] Allow xhci-plat using a second clock
@ 2018-02-14 16:16 Gregory CLEMENT
2018-02-14 16:16 ` [PATCH 1/2] usb: host: xhci-plat: Remove useless test before clk_disable_unprepare Gregory CLEMENT
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Gregory CLEMENT @ 2018-02-14 16:16 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
The purpose of this series is to allow xhci-plat using a second
clock. It is needed on the Armada 7K/8K but could be used by other
SoCs.
The first patch is just a fix found while I was working on this
feature.
Thanks,
Gregory
Gregory CLEMENT (2):
usb: host: xhci-plat: Remove useless test before clk_disable_unprepare
usb: host: xhci-plat: Fix clock resource by adding a register clock
Documentation/devicetree/bindings/usb/usb-xhci.txt | 5 ++-
drivers/usb/host/xhci-plat.c | 39 ++++++++++++++++------
drivers/usb/host/xhci.h | 3 +-
3 files changed, 35 insertions(+), 12 deletions(-)
--
2.15.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/2] usb: host: xhci-plat: Remove useless test before clk_disable_unprepare 2018-02-14 16:16 [PATCH 0/2] Allow xhci-plat using a second clock Gregory CLEMENT @ 2018-02-14 16:16 ` Gregory CLEMENT 2018-02-14 16:16 ` [PATCH 2/2] usb: host: xhci-plat: Fix clock resource by adding a register clock Gregory CLEMENT ` (2 subsequent siblings) 3 siblings, 0 replies; 12+ messages in thread From: Gregory CLEMENT @ 2018-02-14 16:16 UTC (permalink / raw) To: linux-arm-kernel clk_disable_unprepare() already checks that the clock pointer is valid. No need to test it before calling it. Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> --- drivers/usb/host/xhci-plat.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index 6f038306c14d..79afaac57ef6 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -319,8 +319,7 @@ static int xhci_plat_probe(struct platform_device *pdev) usb_put_hcd(xhci->shared_hcd); disable_clk: - if (!IS_ERR(clk)) - clk_disable_unprepare(clk); + clk_disable_unprepare(clk); put_hcd: usb_put_hcd(hcd); @@ -346,8 +345,7 @@ static int xhci_plat_remove(struct platform_device *dev) usb_remove_hcd(hcd); usb_put_hcd(xhci->shared_hcd); - if (!IS_ERR(clk)) - clk_disable_unprepare(clk); + clk_disable_unprepare(clk); usb_put_hcd(hcd); pm_runtime_set_suspended(&dev->dev); -- 2.15.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] usb: host: xhci-plat: Fix clock resource by adding a register clock 2018-02-14 16:16 [PATCH 0/2] Allow xhci-plat using a second clock Gregory CLEMENT 2018-02-14 16:16 ` [PATCH 1/2] usb: host: xhci-plat: Remove useless test before clk_disable_unprepare Gregory CLEMENT @ 2018-02-14 16:16 ` Gregory CLEMENT 2018-02-28 16:05 ` Mathias Nyman 2018-02-28 16:09 ` Manu Gautam 2018-02-28 14:02 ` [PATCH 0/2] Allow xhci-plat using a second clock Gregory CLEMENT 2018-04-18 14:20 ` Gregory CLEMENT 3 siblings, 2 replies; 12+ messages in thread From: Gregory CLEMENT @ 2018-02-14 16:16 UTC (permalink / raw) To: linux-arm-kernel On Armada 7K/8K we need to explicitly enable the register clock. This clock is optional because not all the SoCs using this IP need it but at least for Armada 7K/8K it is actually mandatory. The change was done at xhci-plat level and not at a xhci-mvebu.c because, it is expected that other SoC would have this kind of constraint. The binding documentation is updating accordingly. Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> --- Documentation/devicetree/bindings/usb/usb-xhci.txt | 5 +++- drivers/usb/host/xhci-plat.c | 33 ++++++++++++++++++---- drivers/usb/host/xhci.h | 3 +- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt index e2ea59bbca93..e4b14511f4f8 100644 --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt @@ -27,7 +27,10 @@ Required properties: - interrupts: one XHCI interrupt should be described here. Optional properties: - - clocks: reference to a clock + - clocks: reference to the clocks + - clock-names: mandatory if there is a second clock, in this case + the name must be "core" for the first clock and "reg" for the + second one - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM - usb3-lpm-capable: determines if platform is USB3 LPM capable - quirk-broken-port-ped: set if the controller has broken port disable mechanism diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index 79afaac57ef6..fd0c399013a2 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -157,6 +157,7 @@ static int xhci_plat_probe(struct platform_device *pdev) struct resource *res; struct usb_hcd *hcd; struct clk *clk; + struct clk *reg_clk; int ret; int irq; @@ -226,17 +227,27 @@ static int xhci_plat_probe(struct platform_device *pdev) hcd->rsrc_len = resource_size(res); /* - * Not all platforms have a clk so it is not an error if the - * clock does not exists. + * Not all platforms have clks so it is not an error if the + * clock do not exist. */ + reg_clk = devm_clk_get(&pdev->dev, "reg"); + if (!IS_ERR(reg_clk)) { + ret = clk_prepare_enable(reg_clk); + if (ret) + goto put_hcd; + } else if (PTR_ERR(reg_clk) == -EPROBE_DEFER) { + ret = -EPROBE_DEFER; + goto put_hcd; + } + clk = devm_clk_get(&pdev->dev, NULL); if (!IS_ERR(clk)) { ret = clk_prepare_enable(clk); if (ret) - goto put_hcd; + goto disable_reg_clk; } else if (PTR_ERR(clk) == -EPROBE_DEFER) { ret = -EPROBE_DEFER; - goto put_hcd; + goto disable_reg_clk; } xhci = hcd_to_xhci(hcd); @@ -252,6 +263,7 @@ static int xhci_plat_probe(struct platform_device *pdev) device_wakeup_enable(hcd->self.controller); xhci->clk = clk; + xhci->reg_clk = reg_clk; xhci->main_hcd = hcd; xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev, dev_name(&pdev->dev), hcd); @@ -321,6 +333,9 @@ static int xhci_plat_probe(struct platform_device *pdev) disable_clk: clk_disable_unprepare(clk); +disable_reg_clk: + clk_disable_unprepare(reg_clk); + put_hcd: usb_put_hcd(hcd); @@ -336,6 +351,7 @@ static int xhci_plat_remove(struct platform_device *dev) struct usb_hcd *hcd = platform_get_drvdata(dev); struct xhci_hcd *xhci = hcd_to_xhci(hcd); struct clk *clk = xhci->clk; + struct clk *reg_clk = xhci->reg_clk; xhci->xhc_state |= XHCI_STATE_REMOVING; @@ -346,6 +362,7 @@ static int xhci_plat_remove(struct platform_device *dev) usb_put_hcd(xhci->shared_hcd); clk_disable_unprepare(clk); + clk_disable_unprepare(reg_clk); usb_put_hcd(hcd); pm_runtime_set_suspended(&dev->dev); @@ -370,8 +387,10 @@ static int __maybe_unused xhci_plat_suspend(struct device *dev) */ ret = xhci_suspend(xhci, device_may_wakeup(dev)); - if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk)) + if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk)) { clk_disable_unprepare(xhci->clk); + clk_disable_unprepare(xhci->reg_clk); + } return ret; } @@ -382,8 +401,10 @@ static int __maybe_unused xhci_plat_resume(struct device *dev) struct xhci_hcd *xhci = hcd_to_xhci(hcd); int ret; - if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk)) + if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk)) { + clk_prepare_enable(xhci->reg_clk); clk_prepare_enable(xhci->clk); + } ret = xhci_priv_resume_quirk(hcd); if (ret) diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 96099a245c69..9884aaba1b40 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1726,8 +1726,9 @@ struct xhci_hcd { int page_shift; /* msi-x vectors */ int msix_count; - /* optional clock */ + /* optional clocks */ struct clk *clk; + struct clk *reg_clk; /* data structures */ struct xhci_device_context_array *dcbaa; struct xhci_ring *cmd_ring; -- 2.15.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] usb: host: xhci-plat: Fix clock resource by adding a register clock 2018-02-14 16:16 ` [PATCH 2/2] usb: host: xhci-plat: Fix clock resource by adding a register clock Gregory CLEMENT @ 2018-02-28 16:05 ` Mathias Nyman 2018-03-13 10:58 ` Gregory CLEMENT 2018-03-14 15:56 ` Gregory CLEMENT 2018-02-28 16:09 ` Manu Gautam 1 sibling, 2 replies; 12+ messages in thread From: Mathias Nyman @ 2018-02-28 16:05 UTC (permalink / raw) To: linux-arm-kernel On 14.02.2018 18:16, Gregory CLEMENT wrote: > On Armada 7K/8K we need to explicitly enable the register clock. This > clock is optional because not all the SoCs using this IP need it but at > least for Armada 7K/8K it is actually mandatory. > > The change was done at xhci-plat level and not at a xhci-mvebu.c because, > it is expected that other SoC would have this kind of constraint. > > The binding documentation is updating accordingly. > > Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> > --- > Documentation/devicetree/bindings/usb/usb-xhci.txt | 5 +++- > drivers/usb/host/xhci-plat.c | 33 ++++++++++++++++++---- > drivers/usb/host/xhci.h | 3 +- > 3 files changed, 33 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt > index e2ea59bbca93..e4b14511f4f8 100644 > --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt > +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt > @@ -27,7 +27,10 @@ Required properties: > - interrupts: one XHCI interrupt should be described here. > > Optional properties: > - - clocks: reference to a clock > + - clocks: reference to the clocks > + - clock-names: mandatory if there is a second clock, in this case > + the name must be "core" for the first clock and "reg" for the > + second one > - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM > - usb3-lpm-capable: determines if platform is USB3 LPM capable > - quirk-broken-port-ped: set if the controller has broken port disable mechanism Would be good to get a Ack or review by Rob Herring for the above > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c > index 79afaac57ef6..fd0c399013a2 100644 > --- a/drivers/usb/host/xhci-plat.c > +++ b/drivers/usb/host/xhci-plat.c > @@ -157,6 +157,7 @@ static int xhci_plat_probe(struct platform_device *pdev) > struct resource *res; > struct usb_hcd *hcd; > struct clk *clk; > + struct clk *reg_clk; > int ret; > int irq; > > @@ -226,17 +227,27 @@ static int xhci_plat_probe(struct platform_device *pdev) > hcd->rsrc_len = resource_size(res); > > /* > - * Not all platforms have a clk so it is not an error if the > - * clock does not exists. > + * Not all platforms have clks so it is not an error if the > + * clock do not exist. > */ > + reg_clk = devm_clk_get(&pdev->dev, "reg"); > + if (!IS_ERR(reg_clk)) { > + ret = clk_prepare_enable(reg_clk); > + if (ret) > + goto put_hcd; > + } else if (PTR_ERR(reg_clk) == -EPROBE_DEFER) { > + ret = -EPROBE_DEFER; > + goto put_hcd; > + } > + > clk = devm_clk_get(&pdev->dev, NULL); > if (!IS_ERR(clk)) { > ret = clk_prepare_enable(clk); > if (ret) > - goto put_hcd; > + goto disable_reg_clk; > } else if (PTR_ERR(clk) == -EPROBE_DEFER) { > ret = -EPROBE_DEFER; > - goto put_hcd; > + goto disable_reg_clk; > } > > xhci = hcd_to_xhci(hcd); > @@ -252,6 +263,7 @@ static int xhci_plat_probe(struct platform_device *pdev) > device_wakeup_enable(hcd->self.controller); > > xhci->clk = clk; > + xhci->reg_clk = reg_clk; > xhci->main_hcd = hcd; > xhci->shared_hcd = __usb_create_ hcd(driver, sysdev, &pdev->dev, > dev_name(&pdev->dev), hcd); > @@ -321,6 +333,9 @@ static int xhci_plat_probe(struct platform_device *pdev) > disable_clk: > clk_disable_unprepare(clk); > > +disable_reg_clk: > + clk_disable_unprepare(reg_clk); > + > put_hcd: > usb_put_hcd(hcd); > > @@ -336,6 +351,7 @@ static int xhci_plat_remove(struct platform_device *dev) > struct usb_hcd *hcd = platform_get_drvdata(dev); > struct xhci_hcd *xhci = hcd_to_xhci(hcd); > struct clk *clk = xhci->clk; > + struct clk *reg_clk = xhci->reg_clk; > > xhci->xhc_state |= XHCI_STATE_REMOVING; > > @@ -346,6 +362,7 @@ static int xhci_plat_remove(struct platform_device *dev) > usb_put_hcd(xhci->shared_hcd); > > clk_disable_unprepare(clk); > + clk_disable_unprepare(reg_clk); > usb_put_hcd(hcd); > > pm_runtime_set_suspended(&dev->dev); > @@ -370,8 +387,10 @@ static int __maybe_unused xhci_plat_suspend(struct device *dev) > */ > ret = xhci_suspend(xhci, device_may_wakeup(dev)); > > - if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk)) > + if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk)) { > clk_disable_unprepare(xhci->clk); > + clk_disable_unprepare(xhci->reg_clk); > + } > > return ret; > } > @@ -382,8 +401,10 @@ static int __maybe_unused xhci_plat_resume(struct device *dev) > struct xhci_hcd *xhci = hcd_to_xhci(hcd); > int ret; > > - if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk)) > + if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk)) { > + clk_prepare_enable(xhci->reg_clk); > clk_prepare_enable(xhci->clk); > + } > > ret = xhci_priv_resume_quirk(hcd); > if (ret) > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h > index 96099a245c69..9884aaba1b40 100644 > --- a/drivers/usb/host/xhci.h > +++ b/drivers/usb/host/xhci.h > @@ -1726,8 +1726,9 @@ struct xhci_hcd { > int page_shift; > /* msi-x vectors */ > int msix_count; > - /* optional clock */ > + /* optional clocks */ > struct clk *clk; > + struct clk *reg_clk; > /* data structures */ > struct xhci_device_context_array *dcbaa; > struct xhci_ring *cmd_ring; > Looks good to me, I'll queue it up if the devicetree bindings looks ok for Rob Herring -Mathias ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] usb: host: xhci-plat: Fix clock resource by adding a register clock 2018-02-28 16:05 ` Mathias Nyman @ 2018-03-13 10:58 ` Gregory CLEMENT 2018-03-14 15:56 ` Gregory CLEMENT 1 sibling, 0 replies; 12+ messages in thread From: Gregory CLEMENT @ 2018-03-13 10:58 UTC (permalink / raw) To: linux-arm-kernel Hi Rob, On mer., f?vr. 28 2018, Mathias Nyman <mathias.nyman@intel.com> wrote: > On 14.02.2018 18:16, Gregory CLEMENT wrote: >> On Armada 7K/8K we need to explicitly enable the register clock. This >> clock is optional because not all the SoCs using this IP need it but at >> least for Armada 7K/8K it is actually mandatory. >> >> The change was done at xhci-plat level and not at a xhci-mvebu.c because, >> it is expected that other SoC would have this kind of constraint. >> >> The binding documentation is updating accordingly. >> >> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> >> --- >> Documentation/devicetree/bindings/usb/usb-xhci.txt | 5 +++- >> drivers/usb/host/xhci-plat.c | 33 ++++++++++++++++++---- >> drivers/usb/host/xhci.h | 3 +- >> 3 files changed, 33 insertions(+), 8 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt >> index e2ea59bbca93..e4b14511f4f8 100644 >> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt >> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt >> @@ -27,7 +27,10 @@ Required properties: >> - interrupts: one XHCI interrupt should be described here. >> Optional properties: >> - - clocks: reference to a clock >> + - clocks: reference to the clocks >> + - clock-names: mandatory if there is a second clock, in this case >> + the name must be "core" for the first clock and "reg" for the >> + second one >> - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM >> - usb3-lpm-capable: determines if platform is USB3 LPM capable >> - quirk-broken-port-ped: set if the controller has broken port disable mechanism > > Would be good to get a Ack or review by Rob Herring for the above Would you mind to have a look on this binding update ? Initially I didn't copy you trying to not overflow you because this change is only about adding a new clock, which is pretty common. But I think Mathias would be more confident with a Ack or review by you. Thanks, Gregory > >> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c >> index 79afaac57ef6..fd0c399013a2 100644 >> --- a/drivers/usb/host/xhci-plat.c >> +++ b/drivers/usb/host/xhci-plat.c >> @@ -157,6 +157,7 @@ static int xhci_plat_probe(struct platform_device *pdev) >> struct resource *res; >> struct usb_hcd *hcd; >> struct clk *clk; >> + struct clk *reg_clk; >> int ret; >> int irq; >> @@ -226,17 +227,27 @@ static int xhci_plat_probe(struct >> platform_device *pdev) >> hcd->rsrc_len = resource_size(res); >> /* >> - * Not all platforms have a clk so it is not an error if the >> - * clock does not exists. >> + * Not all platforms have clks so it is not an error if the >> + * clock do not exist. >> */ >> + reg_clk = devm_clk_get(&pdev->dev, "reg"); >> + if (!IS_ERR(reg_clk)) { >> + ret = clk_prepare_enable(reg_clk); >> + if (ret) >> + goto put_hcd; >> + } else if (PTR_ERR(reg_clk) == -EPROBE_DEFER) { >> + ret = -EPROBE_DEFER; >> + goto put_hcd; >> + } >> + >> clk = devm_clk_get(&pdev->dev, NULL); >> if (!IS_ERR(clk)) { >> ret = clk_prepare_enable(clk); >> if (ret) >> - goto put_hcd; >> + goto disable_reg_clk; >> } else if (PTR_ERR(clk) == -EPROBE_DEFER) { >> ret = -EPROBE_DEFER; >> - goto put_hcd; >> + goto disable_reg_clk; >> } >> xhci = hcd_to_xhci(hcd); >> @@ -252,6 +263,7 @@ static int xhci_plat_probe(struct platform_device *pdev) >> device_wakeup_enable(hcd->self.controller); >> xhci->clk = clk; >> + xhci->reg_clk = reg_clk; >> xhci->main_hcd = hcd; >> xhci->shared_hcd = __usb_create_ > > hcd(driver, sysdev, &pdev->dev, >> dev_name(&pdev->dev), hcd); >> @@ -321,6 +333,9 @@ static int xhci_plat_probe(struct platform_device *pdev) >> disable_clk: >> clk_disable_unprepare(clk); >> +disable_reg_clk: >> + clk_disable_unprepare(reg_clk); >> + >> put_hcd: >> usb_put_hcd(hcd); >> @@ -336,6 +351,7 @@ static int xhci_plat_remove(struct >> platform_device *dev) >> struct usb_hcd *hcd = platform_get_drvdata(dev); >> struct xhci_hcd *xhci = hcd_to_xhci(hcd); >> struct clk *clk = xhci->clk; >> + struct clk *reg_clk = xhci->reg_clk; >> xhci->xhc_state |= XHCI_STATE_REMOVING; >> @@ -346,6 +362,7 @@ static int xhci_plat_remove(struct >> platform_device *dev) >> usb_put_hcd(xhci->shared_hcd); >> clk_disable_unprepare(clk); >> + clk_disable_unprepare(reg_clk); >> usb_put_hcd(hcd); >> pm_runtime_set_suspended(&dev->dev); >> @@ -370,8 +387,10 @@ static int __maybe_unused xhci_plat_suspend(struct device *dev) >> */ >> ret = xhci_suspend(xhci, device_may_wakeup(dev)); >> - if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk)) >> + if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk)) { >> clk_disable_unprepare(xhci->clk); >> + clk_disable_unprepare(xhci->reg_clk); >> + } >> return ret; >> } >> @@ -382,8 +401,10 @@ static int __maybe_unused xhci_plat_resume(struct device *dev) >> struct xhci_hcd *xhci = hcd_to_xhci(hcd); >> int ret; >> - if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk)) >> + if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk)) { >> + clk_prepare_enable(xhci->reg_clk); >> clk_prepare_enable(xhci->clk); >> + } >> ret = xhci_priv_resume_quirk(hcd); >> if (ret) >> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h >> index 96099a245c69..9884aaba1b40 100644 >> --- a/drivers/usb/host/xhci.h >> +++ b/drivers/usb/host/xhci.h >> @@ -1726,8 +1726,9 @@ struct xhci_hcd { >> int page_shift; >> /* msi-x vectors */ >> int msix_count; >> - /* optional clock */ >> + /* optional clocks */ >> struct clk *clk; >> + struct clk *reg_clk; >> /* data structures */ >> struct xhci_device_context_array *dcbaa; >> struct xhci_ring *cmd_ring; >> > > Looks good to me, I'll queue it up if the devicetree bindings looks ok for Rob Herring > > -Mathias > -- Gregory Clement, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] usb: host: xhci-plat: Fix clock resource by adding a register clock 2018-02-28 16:05 ` Mathias Nyman 2018-03-13 10:58 ` Gregory CLEMENT @ 2018-03-14 15:56 ` Gregory CLEMENT 1 sibling, 0 replies; 12+ messages in thread From: Gregory CLEMENT @ 2018-03-14 15:56 UTC (permalink / raw) To: linux-arm-kernel Hi Rob, (resent because of malformed cc list) On mer., f?vr. 28 2018, Mathias Nyman <mathias.nyman@intel.com> wrote: > On 14.02.2018 18:16, Gregory CLEMENT wrote: >> On Armada 7K/8K we need to explicitly enable the register clock. This >> clock is optional because not all the SoCs using this IP need it but at >> least for Armada 7K/8K it is actually mandatory. >> >> The change was done at xhci-plat level and not at a xhci-mvebu.c because, >> it is expected that other SoC would have this kind of constraint. >> >> The binding documentation is updating accordingly. >> >> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> >> --- >> Documentation/devicetree/bindings/usb/usb-xhci.txt | 5 +++- >> drivers/usb/host/xhci-plat.c | 33 ++++++++++++++++++---- >> drivers/usb/host/xhci.h | 3 +- >> 3 files changed, 33 insertions(+), 8 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt >> index e2ea59bbca93..e4b14511f4f8 100644 >> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt >> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt >> @@ -27,7 +27,10 @@ Required properties: >> - interrupts: one XHCI interrupt should be described here. >> Optional properties: >> - - clocks: reference to a clock >> + - clocks: reference to the clocks >> + - clock-names: mandatory if there is a second clock, in this case >> + the name must be "core" for the first clock and "reg" for the >> + second one >> - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM >> - usb3-lpm-capable: determines if platform is USB3 LPM capable >> - quirk-broken-port-ped: set if the controller has broken port disable mechanism > > Would be good to get a Ack or review by Rob Herring for the above Would you mind to have a look on this binding update ? Initially I didn't copy you trying to not overflow you because this change is only about adding a new clock, which is pretty common. But I think Mathias would be more confident with a Ack or review by you. Thanks, Gregory -- Gregory Clement, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] usb: host: xhci-plat: Fix clock resource by adding a register clock 2018-02-14 16:16 ` [PATCH 2/2] usb: host: xhci-plat: Fix clock resource by adding a register clock Gregory CLEMENT 2018-02-28 16:05 ` Mathias Nyman @ 2018-02-28 16:09 ` Manu Gautam 2018-02-28 16:24 ` Gregory CLEMENT 1 sibling, 1 reply; 12+ messages in thread From: Manu Gautam @ 2018-02-28 16:09 UTC (permalink / raw) To: linux-arm-kernel Hi, On 2/14/2018 9:46 PM, Gregory CLEMENT wrote: > On Armada 7K/8K we need to explicitly enable the register clock. This > clock is optional because not all the SoCs using this IP need it but at > least for Armada 7K/8K it is actually mandatory. > > The change was done at xhci-plat level and not at a xhci-mvebu.c because, > it is expected that other SoC would have this kind of constraint. > > The binding documentation is updating accordingly. > > Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> > --- > Documentation/devicetree/bindings/usb/usb-xhci.txt | 5 +++- > drivers/usb/host/xhci-plat.c | 33 ++++++++++++++++++---- > drivers/usb/host/xhci.h | 3 +- > 3 files changed, 33 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt > index e2ea59bbca93..e4b14511f4f8 100644 > --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt > +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt > @@ -27,7 +27,10 @@ Required properties: > - interrupts: one XHCI interrupt should be described here. > > Optional properties: > - - clocks: reference to a clock > + - clocks: reference to the clocks > + - clock-names: mandatory if there is a second clock, in this case > + the name must be "core" for the first clock and "reg" for the > + second one > - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM > - usb3-lpm-capable: determines if platform is USB3 LPM capable > - quirk-broken-port-ped: set if the controller has broken port disable mechanism > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c > index 79afaac57ef6..fd0c399013a2 100644 > --- a/drivers/usb/host/xhci-plat.c > +++ b/drivers/usb/host/xhci-plat.c > @@ -157,6 +157,7 @@ static int xhci_plat_probe(struct platform_device *pdev) > struct resource *res; > struct usb_hcd *hcd; > struct clk *clk; > + struct clk *reg_clk; > int ret; > int irq; > > @@ -226,17 +227,27 @@ static int xhci_plat_probe(struct platform_device *pdev) > hcd->rsrc_len = resource_size(res); > > /* > - * Not all platforms have a clk so it is not an error if the > - * clock does not exists. > + * Not all platforms have clks so it is not an error if the > + * clock do not exist. > */ > + reg_clk = devm_clk_get(&pdev->dev, "reg"); > + if (!IS_ERR(reg_clk)) { > + ret = clk_prepare_enable(reg_clk); > + if (ret) > + goto put_hcd; > + } else if (PTR_ERR(reg_clk) == -EPROBE_DEFER) { > + ret = -EPROBE_DEFER; > + goto put_hcd; > + } > + How about using clk_bulk_ APIs? > clk = devm_clk_get(&pdev->dev, NULL); > if (!IS_ERR(clk)) { > ret = clk_prepare_enable(clk); > -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] usb: host: xhci-plat: Fix clock resource by adding a register clock 2018-02-28 16:09 ` Manu Gautam @ 2018-02-28 16:24 ` Gregory CLEMENT 0 siblings, 0 replies; 12+ messages in thread From: Gregory CLEMENT @ 2018-02-28 16:24 UTC (permalink / raw) To: linux-arm-kernel Hi Manu, On mer., f?vr. 28 2018, Manu Gautam <mgautam@codeaurora.org> wrote: > Hi, > > > On 2/14/2018 9:46 PM, Gregory CLEMENT wrote: >> On Armada 7K/8K we need to explicitly enable the register clock. This >> clock is optional because not all the SoCs using this IP need it but at >> least for Armada 7K/8K it is actually mandatory. >> >> The change was done at xhci-plat level and not at a xhci-mvebu.c because, >> it is expected that other SoC would have this kind of constraint. >> >> The binding documentation is updating accordingly. >> >> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> >> --- >> Documentation/devicetree/bindings/usb/usb-xhci.txt | 5 +++- >> drivers/usb/host/xhci-plat.c | 33 ++++++++++++++++++---- >> drivers/usb/host/xhci.h | 3 +- >> 3 files changed, 33 insertions(+), 8 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt >> index e2ea59bbca93..e4b14511f4f8 100644 >> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt >> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt >> @@ -27,7 +27,10 @@ Required properties: >> - interrupts: one XHCI interrupt should be described here. >> >> Optional properties: >> - - clocks: reference to a clock >> + - clocks: reference to the clocks >> + - clock-names: mandatory if there is a second clock, in this case >> + the name must be "core" for the first clock and "reg" for the >> + second one >> - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM >> - usb3-lpm-capable: determines if platform is USB3 LPM capable >> - quirk-broken-port-ped: set if the controller has broken port disable mechanism >> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c >> index 79afaac57ef6..fd0c399013a2 100644 >> --- a/drivers/usb/host/xhci-plat.c >> +++ b/drivers/usb/host/xhci-plat.c >> @@ -157,6 +157,7 @@ static int xhci_plat_probe(struct platform_device *pdev) >> struct resource *res; >> struct usb_hcd *hcd; >> struct clk *clk; >> + struct clk *reg_clk; >> int ret; >> int irq; >> >> @@ -226,17 +227,27 @@ static int xhci_plat_probe(struct platform_device *pdev) >> hcd->rsrc_len = resource_size(res); >> >> /* >> - * Not all platforms have a clk so it is not an error if the >> - * clock does not exists. >> + * Not all platforms have clks so it is not an error if the >> + * clock do not exist. >> */ >> + reg_clk = devm_clk_get(&pdev->dev, "reg"); >> + if (!IS_ERR(reg_clk)) { >> + ret = clk_prepare_enable(reg_clk); >> + if (ret) >> + goto put_hcd; >> + } else if (PTR_ERR(reg_clk) == -EPROBE_DEFER) { >> + ret = -EPROBE_DEFER; >> + goto put_hcd; >> + } >> + > > How about using clk_bulk_ APIs? I didn't know this API, but after having a look on it, it didn't match what I need. Indeed the second clock is "optional" to handle the backward compatibility. With the clk_bulk_ APIS all the clocks are mandatory. Thanks, Gregory > >> clk = devm_clk_get(&pdev->dev, NULL); >> if (!IS_ERR(clk)) { >> ret = clk_prepare_enable(clk); >> > > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Gregory Clement, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 0/2] Allow xhci-plat using a second clock 2018-02-14 16:16 [PATCH 0/2] Allow xhci-plat using a second clock Gregory CLEMENT 2018-02-14 16:16 ` [PATCH 1/2] usb: host: xhci-plat: Remove useless test before clk_disable_unprepare Gregory CLEMENT 2018-02-14 16:16 ` [PATCH 2/2] usb: host: xhci-plat: Fix clock resource by adding a register clock Gregory CLEMENT @ 2018-02-28 14:02 ` Gregory CLEMENT 2018-04-18 14:20 ` Gregory CLEMENT 3 siblings, 0 replies; 12+ messages in thread From: Gregory CLEMENT @ 2018-02-28 14:02 UTC (permalink / raw) To: linux-arm-kernel Hi Mathias, On mer., f?vr. 14 2018, Gregory CLEMENT <gregory.clement@bootlin.com> wrote: > Hello, > > The purpose of this series is to allow xhci-plat using a second > clock. It is needed on the Armada 7K/8K but could be used by other > SoCs. Do you have some comments on this series and especially on the second patch? Thanks, Gregory > > The first patch is just a fix found while I was working on this > feature. > > Thanks, > > Gregory > > > Gregory CLEMENT (2): > usb: host: xhci-plat: Remove useless test before clk_disable_unprepare > usb: host: xhci-plat: Fix clock resource by adding a register clock > > Documentation/devicetree/bindings/usb/usb-xhci.txt | 5 ++- > drivers/usb/host/xhci-plat.c | 39 ++++++++++++++++------ > drivers/usb/host/xhci.h | 3 +- > 3 files changed, 35 insertions(+), 12 deletions(-) > > -- > 2.15.1 > -- Gregory Clement, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 0/2] Allow xhci-plat using a second clock 2018-02-14 16:16 [PATCH 0/2] Allow xhci-plat using a second clock Gregory CLEMENT ` (2 preceding siblings ...) 2018-02-28 14:02 ` [PATCH 0/2] Allow xhci-plat using a second clock Gregory CLEMENT @ 2018-04-18 14:20 ` Gregory CLEMENT 2018-04-19 5:59 ` Mathias Nyman 3 siblings, 1 reply; 12+ messages in thread From: Gregory CLEMENT @ 2018-04-18 14:20 UTC (permalink / raw) To: linux-arm-kernel Hi Mathias, On mer., f?vr. 14 2018, Gregory CLEMENT <gregory.clement@bootlin.com> wrote: > Hello, > > The purpose of this series is to allow xhci-plat using a second > clock. It is needed on the Armada 7K/8K but could be used by other > SoCs. > > The first patch is just a fix found while I was working on this > feature. I've just realized that this series sent 2 months ago was not merged in v4.17. The issue is that now the USB support on the Armada 7K/8K is broken, because the clock support part was already merged. You already had a look on this series one month ago. The issue you had was about getting an approval for the extension of the binding [1]. I pinged Rob about it [2] one month ago but we didn't get any feedback. However, Rob already approved the similar changes I introduced in an other for mv_xor_v2: [3]. So would it be possible to apply this series on v4.17-rc ? Thanks, Gregory [1]: http://lists.infradead.org/pipermail/linux-arm-kernel/2018-February/562739.html [2]: http://lists.infradead.org/pipermail/linux-arm-kernel/2018-March/566076.html [3]: http://lists.infradead.org/pipermail/linux-arm-kernel/2018-March/564594.html > > Thanks, > > Gregory > > > Gregory CLEMENT (2): > usb: host: xhci-plat: Remove useless test before clk_disable_unprepare > usb: host: xhci-plat: Fix clock resource by adding a register clock > > Documentation/devicetree/bindings/usb/usb-xhci.txt | 5 ++- > drivers/usb/host/xhci-plat.c | 39 ++++++++++++++++------ > drivers/usb/host/xhci.h | 3 +- > 3 files changed, 35 insertions(+), 12 deletions(-) > > -- > 2.15.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Gregory Clement, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 0/2] Allow xhci-plat using a second clock 2018-04-18 14:20 ` Gregory CLEMENT @ 2018-04-19 5:59 ` Mathias Nyman 2018-04-19 13:43 ` Gregory CLEMENT 0 siblings, 1 reply; 12+ messages in thread From: Mathias Nyman @ 2018-04-19 5:59 UTC (permalink / raw) To: linux-arm-kernel On 18.04.2018 17:20, Gregory CLEMENT wrote: > Hi Mathias, > > On mer., f?vr. 14 2018, Gregory CLEMENT <gregory.clement@bootlin.com> wrote: > >> Hello, >> >> The purpose of this series is to allow xhci-plat using a second >> clock. It is needed on the Armada 7K/8K but could be used by other >> SoCs. >> >> The first patch is just a fix found while I was working on this >> feature. > > I've just realized that this series sent 2 months ago was not merged in > v4.17. The issue is that now the USB support on the Armada 7K/8K is > broken, because the clock support part was already merged. > > You already had a look on this series one month ago. The issue you had > was about getting an approval for the extension of the binding [1]. I > pinged Rob about it [2] one month ago but we didn't get any > feedback. However, Rob already approved the similar changes I introduced > in an other for mv_xor_v2: [3]. > > So would it be possible to apply this series on v4.17-rc ? Patch 2/2 no longer applies due to clk suspend/resume changes in: commit d56e57ca030c8b4296944a2ae61ac167bf979c07 usb: host: xhci-plat: revert "usb: host: xhci-plat: enable clk in resume timing" Not sure how that change affects Armada 7K/8K Can you rebase and resend the series? -Mathias ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 0/2] Allow xhci-plat using a second clock 2018-04-19 5:59 ` Mathias Nyman @ 2018-04-19 13:43 ` Gregory CLEMENT 0 siblings, 0 replies; 12+ messages in thread From: Gregory CLEMENT @ 2018-04-19 13:43 UTC (permalink / raw) To: linux-arm-kernel Hi Mathias, On jeu., avril 19 2018, Mathias Nyman <mathias.nyman@linux.intel.com> wrote: > On 18.04.2018 17:20, Gregory CLEMENT wrote: >> Hi Mathias, >> On mer., f?vr. 14 2018, Gregory CLEMENT >> <gregory.clement@bootlin.com> wrote: >> >>> Hello, >>> >>> The purpose of this series is to allow xhci-plat using a second >>> clock. It is needed on the Armada 7K/8K but could be used by other >>> SoCs. >>> >>> The first patch is just a fix found while I was working on this >>> feature. >> >> I've just realized that this series sent 2 months ago was not merged in >> v4.17. The issue is that now the USB support on the Armada 7K/8K is >> broken, because the clock support part was already merged. >> >> You already had a look on this series one month ago. The issue you had >> was about getting an approval for the extension of the binding [1]. I >> pinged Rob about it [2] one month ago but we didn't get any >> feedback. However, Rob already approved the similar changes I introduced >> in an other for mv_xor_v2: [3]. >> >> So would it be possible to apply this series on v4.17-rc ? > > Patch 2/2 no longer applies due to clk suspend/resume changes in: > > commit d56e57ca030c8b4296944a2ae61ac167bf979c07 > usb: host: xhci-plat: revert "usb: host: xhci-plat: enable clk in resume timing" > > Not sure how that change affects Armada 7K/8K > Can you rebase and resend the series? It's done! About the suspend/resume part it does not affect Armada 7K/8K because we don't support it yet. However I am a bit surprised by the commit log because reverting a common part because of a specific implementation looks wrong. I expect issues when we will add PM support but we will see that later. Gregory > > -Mathias > -- Gregory Clement, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-04-19 13:43 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-14 16:16 [PATCH 0/2] Allow xhci-plat using a second clock Gregory CLEMENT 2018-02-14 16:16 ` [PATCH 1/2] usb: host: xhci-plat: Remove useless test before clk_disable_unprepare Gregory CLEMENT 2018-02-14 16:16 ` [PATCH 2/2] usb: host: xhci-plat: Fix clock resource by adding a register clock Gregory CLEMENT 2018-02-28 16:05 ` Mathias Nyman 2018-03-13 10:58 ` Gregory CLEMENT 2018-03-14 15:56 ` Gregory CLEMENT 2018-02-28 16:09 ` Manu Gautam 2018-02-28 16:24 ` Gregory CLEMENT 2018-02-28 14:02 ` [PATCH 0/2] Allow xhci-plat using a second clock Gregory CLEMENT 2018-04-18 14:20 ` Gregory CLEMENT 2018-04-19 5:59 ` Mathias Nyman 2018-04-19 13:43 ` Gregory CLEMENT
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).