| Message ID | 1403072180-4944-6-git-send-email-abrestic@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/18/2014 12:16 AM, Andrew Bresticker wrote: > Add device-tree binding documentation for the XHCI controller present > on Tegra124 and later SoCs. > diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt > +Required properties: > +-------------------- > + - clock-names: Must include the following entries: > + - xusb_host > + - xusb_falcon_src > + - xusb_ss > + - xusb_ss_src > + - xusb_hs_src > + - xusb_fs_src > + - pll_u_480m > + - clk_m > + - pll_e > + - reset-names: Must include the following entries: > + - xusb_host > + - xusb_ss Usually the CAR has a reset control for each clock. So, I would expect as many entries in reset-names as in clock-names. Even if the SW doesn't currently touch all the reset lines, we should make sure the binding requires them to be present so that any DT will contain the entries if they're ever needed in the future. In the CAR documentation, I see "XUSB_DEV" as a clock/reset bit. Is that missing from the list above? > + - nvidia,xusb-mbox: Handle to the Tegra XUSB mailbox node. As mentioned earlier, I think that's an internal implementation detail. Shouldn't the two nodes be squashed together? > +Optional properties: > + - s1p05v-supply: 1.05V supply regulator. > + - s1p8v-supply: 1.8V supply regulator. > + - s3p3v-supply: 3.3V supply regulator. What are those supplies for? I would have expected any input to the SoC to have a name that described its purpose, and the pins and DT properties would be named to match.
On 06/18/2014 12:16 AM, Andrew Bresticker wrote: > Add device-tree binding documentation for the XHCI controller present > on Tegra124 and later SoCs. > diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt > + - clock-names: Must include the following entries: > + - xusb_host > + - xusb_falcon_src > + - xusb_ss > + - xusb_ss_src > + - xusb_hs_src > + - xusb_fs_src Looking at include/dt-bindings/clock/tegra124-car.h I see a few entries potentially missing here: #define TEGRA124_CLK_XUSB_HOST_SRC 252 #define TEGRA124_CLK_XUSB_DEV_SRC 256 #define TEGRA124_CLK_XUSB_DEV 257 #define TEGRA124_CLK_XUSB_SS_DIV2 312 > + - pll_u_480m Not just pll_u?
On Wed, Jun 25, 2014 at 2:52 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 06/18/2014 12:16 AM, Andrew Bresticker wrote: >> Add device-tree binding documentation for the XHCI controller present >> on Tegra124 and later SoCs. > >> diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt > >> +Required properties: >> +-------------------- > >> + - clock-names: Must include the following entries: >> + - xusb_host >> + - xusb_falcon_src >> + - xusb_ss >> + - xusb_ss_src >> + - xusb_hs_src >> + - xusb_fs_src >> + - pll_u_480m >> + - clk_m >> + - pll_e > >> + - reset-names: Must include the following entries: >> + - xusb_host >> + - xusb_ss > > Usually the CAR has a reset control for each clock. So, I would expect > as many entries in reset-names as in clock-names. Even if the SW doesn't > currently touch all the reset lines, we should make sure the binding > requires them to be present so that any DT will contain the entries if > they're ever needed in the future. The xusb_{falcon,host,hs,fs,ss}_src clocks all share the same reset bit (143), so I can add a single entry for those. > In the CAR documentation, I see "XUSB_DEV" as a clock/reset bit. Is that > missing from the list above? This is used when XUSB is in device mode, which the driver does not support. I can add those clocks here though if you want. >> +Optional properties: > >> + - s1p05v-supply: 1.05V supply regulator. >> + - s1p8v-supply: 1.8V supply regulator. >> + - s3p3v-supply: 3.3V supply regulator. > > What are those supplies for? I would have expected any input to the SoC > to have a name that described its purpose, and the pins and DT > properties would be named to match. I *think* this what they are from looking at the schematic, but I'll have to ask around: - s1p05v: avddio_pex, dvddio_pex, and maybe avdd_pll_erefe - s1p8v: avdd_pll_utmip - s3p3v: avdd_usb, hvdd_pex, hvdd_pex_pll_e Should these be separated out as they are for PCIe?
On Wed, Jun 25, 2014 at 2:54 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 06/18/2014 12:16 AM, Andrew Bresticker wrote: >> Add device-tree binding documentation for the XHCI controller present >> on Tegra124 and later SoCs. > >> diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt > >> + - clock-names: Must include the following entries: >> + - xusb_host >> + - xusb_falcon_src >> + - xusb_ss >> + - xusb_ss_src >> + - xusb_hs_src >> + - xusb_fs_src > > Looking at include/dt-bindings/clock/tegra124-car.h I see a few entries > potentially missing here: > > #define TEGRA124_CLK_XUSB_HOST_SRC 252 > #define TEGRA124_CLK_XUSB_DEV_SRC 256 > #define TEGRA124_CLK_XUSB_DEV 257 > #define TEGRA124_CLK_XUSB_SS_DIV2 312 The driver doesn't use them, so I didn't put them in the binding. >> + - pll_u_480m > > Not just pll_u? We specifically want pll_u_480M as that's what we use as the parent of xusb_ss_src when scaling it to 120Mhz.
On 06/25/2014 05:01 PM, Andrew Bresticker wrote: > On Wed, Jun 25, 2014 at 2:52 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> On 06/18/2014 12:16 AM, Andrew Bresticker wrote: >>> Add device-tree binding documentation for the XHCI controller present >>> on Tegra124 and later SoCs. >> >>> diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt >> >>> +Required properties: >>> +-------------------- >> >>> + - clock-names: Must include the following entries: >>> + - xusb_host >>> + - xusb_falcon_src >>> + - xusb_ss >>> + - xusb_ss_src >>> + - xusb_hs_src >>> + - xusb_fs_src >>> + - pll_u_480m >>> + - clk_m >>> + - pll_e >> >>> + - reset-names: Must include the following entries: >>> + - xusb_host >>> + - xusb_ss >> >> Usually the CAR has a reset control for each clock. So, I would expect >> as many entries in reset-names as in clock-names. Even if the SW doesn't >> currently touch all the reset lines, we should make sure the binding >> requires them to be present so that any DT will contain the entries if >> they're ever needed in the future. > > The xusb_{falcon,host,hs,fs,ss}_src clocks all share the same reset > bit (143), so I can add a single entry for those. > >> In the CAR documentation, I see "XUSB_DEV" as a clock/reset bit. Is that >> missing from the list above? > > This is used when XUSB is in device mode, which the driver does not > support. I can add those clocks here though if you want. That'd be a good idea. That way, the DT doesn't have to change later. >>> +Optional properties: >> >>> + - s1p05v-supply: 1.05V supply regulator. >>> + - s1p8v-supply: 1.8V supply regulator. >>> + - s3p3v-supply: 3.3V supply regulator. >> >> What are those supplies for? I would have expected any input to the SoC >> to have a name that described its purpose, and the pins and DT >> properties would be named to match. > > I *think* this what they are from looking at the schematic, but I'll > have to ask around: > - s1p05v: avddio_pex, dvddio_pex, and maybe avdd_pll_erefe > - s1p8v: avdd_pll_utmip > - s3p3v: avdd_usb, hvdd_pex, hvdd_pex_pll_e > Should these be separated out as they are for PCIe? Yes, I think they should be separated. I wonder if the supplies for PCIe shouldn't have been added to the XUSB padctrl rather than PCIe node though? It probably doesn't matter much either way.
On 06/25/2014 05:02 PM, Andrew Bresticker wrote: > On Wed, Jun 25, 2014 at 2:54 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> On 06/18/2014 12:16 AM, Andrew Bresticker wrote: >>> Add device-tree binding documentation for the XHCI controller present >>> on Tegra124 and later SoCs. >> >>> diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt >> >>> + - clock-names: Must include the following entries: >>> + - xusb_host >>> + - xusb_falcon_src >>> + - xusb_ss >>> + - xusb_ss_src >>> + - xusb_hs_src >>> + - xusb_fs_src >> >> Looking at include/dt-bindings/clock/tegra124-car.h I see a few entries >> potentially missing here: >> >> #define TEGRA124_CLK_XUSB_HOST_SRC 252 >> #define TEGRA124_CLK_XUSB_DEV_SRC 256 >> #define TEGRA124_CLK_XUSB_DEV 257 >> #define TEGRA124_CLK_XUSB_SS_DIV2 312 > > The driver doesn't use them, so I didn't put them in the binding. I think we should add them in case we need them later. Best to fully describe the HW rather than the parts of the HW that SW currently uses. >>> + - pll_u_480m >> >> Not just pll_u? > > We specifically want pll_u_480M as that's what we use as the parent of > xusb_ss_src when scaling it to 120Mhz. OK. I recall text in the TRM implying that SW should just leave PLL_U alone and not fiddle with the separate output clocks. Still, if we have a clock ID for each output, and it's the correct parent for the clock, then it does make sense to use that ID.
diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt new file mode 100644 index 0000000..fdb8624 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt @@ -0,0 +1,76 @@ +NVIDIA Tegra XHCI controller +============================ + +The Tegra XHCI controller supports both USB2 and USB3 interfaces exposed +by the Tegra XUSB pad controller. + +Required properties: +-------------------- + - compatible: Should be "nvidia,tegra124-xhci". + - reg: Address and length of the register sets. There should be three + entries in the following order: XHCI host registers, FPCI registers, and + IPFS registers. + - interrupts: XHCI host interrupt. + - clocks: Must contain an entry for each entry in clock-names. + See ../clock/clock-bindings.txt for details. + - clock-names: Must include the following entries: + - xusb_host + - xusb_falcon_src + - xusb_ss + - xusb_ss_src + - xusb_hs_src + - xusb_fs_src + - pll_u_480m + - clk_m + - pll_e + - resets: Must contain an entry for each entry in reset-names. + See ../reset/reset.txt for details. + - reset-names: Must include the following entries: + - xusb_host + - xusb_ss + - nvidia,xusb-mbox: Handle to the Tegra XUSB mailbox node. + +Optional properties: +-------------------- + - phys: Must contain an entry for each entry in phy-names. + See ../phy/phy-bindings.txt for details. + - phy-names: Should include an entry for each PHY used by the controller. + May be a subset of the following: + - utmi-{0,1,2} + - hsic-{0,1} + - usb3-{0,1} + - s1p05v-supply: 1.05V supply regulator. + - s1p8v-supply: 1.8V supply regulator. + - s3p3v-supply: 3.3V supply regulator. + +Example: +-------- + usb@0,70090000 { + compatible = "nvidia,tegra124-xhci"; + reg = <0x0 0x70090000 0x0 0x8000>, + <0x0 0x70098000 0x0 0x1000>, + <0x0 0x70099000 0x0 0x1000>; + interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_XUSB_HOST>, + <&tegra_car TEGRA124_CLK_XUSB_FALCON_SRC>, + <&tegra_car TEGRA124_CLK_XUSB_SS>, + <&tegra_car TEGRA124_CLK_XUSB_SS_SRC>, + <&tegra_car TEGRA124_CLK_XUSB_HS_SRC>, + <&tegra_car TEGRA124_CLK_XUSB_FS_SRC>, + <&tegra_car TEGRA124_CLK_PLL_U_480M>, + <&tegra_car TEGRA124_CLK_CLK_M>, + <&tegra_car TEGRA124_CLK_PLL_E>; + clock-names = "xusb_host", "xusb_falcon_src", "xusb_ss", + "xusb_ss_src", "xusb_hs_src", "xusb_fs_src", + "pll_u_480m", "clk_m", "pll_e"; + resets = <&tegra_car 89>, <&tegra_car 156>; + reset-names = "xusb_host", "xusb_ss"; + nvidia,xusb-mbox = <&mbox>; + phys = <&padctl TEGRA_XUSB_PADCTL_UTMI_P1>, /* mini-PCIe USB */ + <&padctl TEGRA_XUSB_PADCTL_UTMI_P2>, /* USB A */ + <&padctl TEGRA_XUSB_PADCTL_USB3_P0>; /* USB A */ + phy-names = "utmi-1", "utmi-2", "usb3-0"; + s1p05v-supply = <&vdd_1v05_run>; + s3p3v-supply = <&vdd_3v3_lp0>; + s1p8v-supply = <&vddio_1v8>; + };
Add device-tree binding documentation for the XHCI controller present on Tegra124 and later SoCs. Signed-off-by: Andrew Bresticker <abrestic@chromium.org> --- .../bindings/usb/nvidia,tegra124-xhci.txt | 76 ++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt