| Message ID | 1442368183-8103-2-git-send-email-peter.chen@freescale.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Wed, Sep 16, 2015 at 12:23:26AM -0300, Fabio Estevam wrote: > On Tue, Sep 15, 2015 at 10:49 PM, Peter Chen <peter.chen@freescale.com> wrote: > > Some SoCs needs three clock to let controller work, but others only > > need one, add one property to differentiate this. > > > > Signed-off-by: Peter Chen <peter.chen@freescale.com> > > --- > > Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > index f15a317..4900092 100644 > > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > @@ -54,6 +54,9 @@ i.mx specific properties > > argument that indicate usb controller index > > - disable-over-current: disable over current detect > > - external-vbus-divider: enables off-chip resistor divider for Vbus > > +- need-three-clocks: the SoC before imx6 series (except for imx23/imx28) > > + needs three clcoks for controller, others only need one. Without this > > + property, the driver will consider this controller only need one clock. > > Looking at mx31, mx35, mx25 reference manuals we see that they don't > really need three usb clocks. > > Also, if we look at the old drivers/usb/host/ehci-mxc.c we only > require 'ipg' and 'ahb' clocks. > but drivers/usb/gadget/udc/fsl_mxc_udc.c has three clocks > So it would be better to just require these two clocks and if they are > not found on dts, then we fall back to requesting clk_get(NULL), > without the need of an extra property. I considered before, but if it has clk_ipg, but without clk_ahb, do we consider it is an error or not ? Using extra property can make things cleaner.
On Tue, Sep 15, 2015 at 10:49 PM, Peter Chen <peter.chen@freescale.com> wrote: > Some SoCs needs three clock to let controller work, but others only > need one, add one property to differentiate this. > > Signed-off-by: Peter Chen <peter.chen@freescale.com> > --- > Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > index f15a317..4900092 100644 > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > @@ -54,6 +54,9 @@ i.mx specific properties > argument that indicate usb controller index > - disable-over-current: disable over current detect > - external-vbus-divider: enables off-chip resistor divider for Vbus > +- need-three-clocks: the SoC before imx6 series (except for imx23/imx28) > + needs three clcoks for controller, others only need one. Without this > + property, the driver will consider this controller only need one clock. Looking at mx31, mx35, mx25 reference manuals we see that they don't really need three usb clocks. Also, if we look at the old drivers/usb/host/ehci-mxc.c we only require 'ipg' and 'ahb' clocks. So it would be better to just require these two clocks and if they are not found on dts, then we fall back to requesting clk_get(NULL), without the need of an extra property. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Sep 15, 2015 at 11:24 PM, Peter Chen <peter.chen@freescale.com> wrote:
> but drivers/usb/gadget/udc/fsl_mxc_udc.c has three clocks
Ok, looking at the clock driver I see that they used to associate
three clocks with mxc-ehci:
clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.0");
clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.0");
clk_register_clkdev(clk[usbotg_gate], "ahb", "mxc-ehci.0");
So your approach looks good.
I only think that mx25 and mx35 dtsi should be also updated to have
the 'three-clock' property.
Thanks
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/15/2015 08:49 PM, Peter Chen wrote: > Some SoCs needs three clock to let controller work, but others only > need one, add one property to differentiate this. A given licensed IP block is going to have the same number of clock inputs from SOC to SOC. So different numbers of clocks is a bit suspect. I guess there can be variations in bus clocks or other outside logic. > > Signed-off-by: Peter Chen <peter.chen@freescale.com> > --- > Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > index f15a317..4900092 100644 > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > @@ -54,6 +54,9 @@ i.mx specific properties > argument that indicate usb controller index > - disable-over-current: disable over current detect > - external-vbus-divider: enables off-chip resistor divider for Vbus > +- need-three-clocks: the SoC before imx6 series (except for imx23/imx28) > + needs three clcoks for controller, others only need one. Without this > + property, the driver will consider this controller only need one clock. That's pretty ugly and unnecessary. Either use the compatible string to determine if you have 3 clocks or just always try to retrieve the 3 clocks in the driver and fall back to 1. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 16, 2015 at 08:54:25AM -0500, Rob Herring wrote: > On 09/15/2015 08:49 PM, Peter Chen wrote: > > Some SoCs needs three clock to let controller work, but others only > > need one, add one property to differentiate this. > > A given licensed IP block is going to have the same number of clock > inputs from SOC to SOC. Yes, I agree with you. > So different numbers of clocks is a bit suspect. > I guess there can be variations in bus clocks or other outside logic. There are legacy platforms, no one knows exactly what the other clocks are used for, and the former i.mx usb driver (see drivers/usb/gadget/udc/fsl_mxc_udc.c) uses like this way. So, I just follow it. > > > > > Signed-off-by: Peter Chen <peter.chen@freescale.com> > > --- > > Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > index f15a317..4900092 100644 > > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > @@ -54,6 +54,9 @@ i.mx specific properties > > argument that indicate usb controller index > > - disable-over-current: disable over current detect > > - external-vbus-divider: enables off-chip resistor divider for Vbus > > +- need-three-clocks: the SoC before imx6 series (except for imx23/imx28) > > + needs three clcoks for controller, others only need one. Without this > > + property, the driver will consider this controller only need one clock. > > That's pretty ugly and unnecessary. Either use the compatible string to > determine Since there are too many SoCs to use it, I just didn't want to add judgement for platforms, and thought using feature property is simpler. If you doesn't agree, I will use other ways. > if you have 3 clocks or just always try to retrieve the 3 > clocks in the driver and fall back to 1. > It is not easy to use this way, one-clock platforms have no dev_id for clock, but three-clock platforms have.
On Thu, Sep 17, 2015 at 9:35 PM, Peter Chen <peter.chen@freescale.com> wrote: > On Wed, Sep 16, 2015 at 08:54:25AM -0500, Rob Herring wrote: >> On 09/15/2015 08:49 PM, Peter Chen wrote: >> > Some SoCs needs three clock to let controller work, but others only >> > need one, add one property to differentiate this. [...] >> > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt >> > index f15a317..4900092 100644 >> > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt >> > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt >> > @@ -54,6 +54,9 @@ i.mx specific properties >> > argument that indicate usb controller index >> > - disable-over-current: disable over current detect >> > - external-vbus-divider: enables off-chip resistor divider for Vbus >> > +- need-three-clocks: the SoC before imx6 series (except for imx23/imx28) >> > + needs three clcoks for controller, others only need one. Without this >> > + property, the driver will consider this controller only need one clock. >> >> That's pretty ugly and unnecessary. Either use the compatible string to >> determine > > Since there are too many SoCs to use it, I just didn't want to add > judgement for platforms, and thought using feature property is simpler. > If you doesn't agree, I will use other ways. > >> if you have 3 clocks or just always try to retrieve the 3 >> clocks in the driver and fall back to 1. >> > > It is not easy to use this way, one-clock platforms have no dev_id for > clock, but three-clock platforms have. You just need to try to retrieve the 3rd clock by name. If that succeeds, retrieve clocks 1 and 2. If you can't retrieve the 3rd clock, the just get the 1st clock with no name. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt index f15a317..4900092 100644 --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt @@ -54,6 +54,9 @@ i.mx specific properties argument that indicate usb controller index - disable-over-current: disable over current detect - external-vbus-divider: enables off-chip resistor divider for Vbus +- need-three-clocks: the SoC before imx6 series (except for imx23/imx28) + needs three clcoks for controller, others only need one. Without this + property, the driver will consider this controller only need one clock. Example:
Some SoCs needs three clock to let controller work, but others only need one, add one property to differentiate this. Signed-off-by: Peter Chen <peter.chen@freescale.com> --- Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 3 +++ 1 file changed, 3 insertions(+)