Re: [U-Boot] [PATCH v2 09/17] tegra: usb: fdt: Add additional device tree definitions for USB ports
On 12/08/2011 02:10 PM, Simon Glass wrote: Hi Stephen, On Wed, Dec 7, 2011 at 3:36 PM, Stephen Warren swar...@nvidia.com wrote: On 12/06/2011 02:09 PM, Simon Glass wrote: On Tue, Dec 6, 2011 at 12:28 PM, Stephen Warren swar...@nvidia.com wrote: On 12/05/2011 05:55 PM, Simon Glass wrote: On Mon, Dec 5, 2011 at 3:25 PM, Stephen Warren swar...@nvidia.com wrote: On 12/02/2011 07:11 PM, Simon Glass wrote: This adds peripheral IDs and timing information to the USB part of the device tree for U-Boot. The peripheral IDs provide easy access to clock registers. We will likely remove this in favor of a full clock tree when it is available in the kernel (but probably still retain the peripheral ID, just move it into a clock node). The USB timing information does apparently vary between boards sometimes, so is include in the fdt for convenience. usb@c500 { compatible = nvidia,tegra20-ehci, usb-ehci; reg = 0xc500 0x4000; interrupts = 52 ; phy_type = utmi; + periph-id = 22; // PERIPH_ID_USBD Given this is a temporary U-Boot-specific solution, can the property be named u-boot,periph-id so it's obvious that when writing a .dts for the kernel only, you don't care about this value. ok. I suggest the kernel does something similar. The kernel will use the standardized clock bindings once they're ready and we convert Tegra over to use them. The kernel is extremely unlikely to ever use periph-id or u-boot,periph-id. What is the time frame on this working be completed and merged? Sorry, I have no idea. I've been focusing on other subsystems (pinmux, audio) and haven't been following the clock stuff at all. Hopefully someone will start driving Tegra kernel towards common clock soon, but I don't think exactly who and when has been nailed down yet. Right now, the kernel's clock driver contains a mapping table from device name (e.g. tegra-ehci.2) to clock name (e.g. usb3). This allows the kernel USB driver to work without any explicit periph-id or similar DT property. Where does tegra-ehci.2 come from? I don't see that in the fdt. Pre-DT, everything was instantiated from platform devices. Each one had a name (tegra-ehci) and an instance number (2), which concatenate to tegra-ehci.2. All the clocks (and I think other resources like regulators) in the kernel were marked as being for use by a particular device name. For example in arch/arm/mach-tegra/tegra2_clocks.c: static struct clk tegra_list_clks[] = { ... PERIPH_CLK(usb3, tegra-ehci.2, ...), With DT, the device names typically don't follow this format (in this case, it'd be something more like /usb@c5008000). However, this prevented the clock lookups by device name from working, so a temporary scheme was put in place to keep the same device names. This is driven by AUXDATA, for example in arch/arm/mach-tegra/board-dt.c: struct of_dev_auxdata tegra20_auxdata_lookup[] __initdata = { ... // compatible, unit address, device name OF_DEV_AUXDATA(nvidia,tegra20-ehci, TEGRA_USB3_BASE, tegra-ehci.2, This means that any device with the given compatible property value, the given unit address will be named accordingly. This allows the existing clock/regulator lookups to work unmodified. Once DT bindings are in place for clocks, regulators, etc., the clock tables can be derived from DT, phandles will be used to match clocks and devices rather than device names, and the AUXDATA table can go away. The equivalent in U-Boot would be a table that maps from driver type (e.g. COMPAT_NVIDIA_TEGRA20_USB or perhaps NVIDIA_TEGRA20_USB?) and address to periph id. Again, once the clock bindings are complete and the nodes present in the .dts file, that mapping table can be removed and everything will work based on phandles. I'd like to point out here that everything is in a pretty big state of flux/development, since DT support for ARM is new. Temporary workarounds like AUXDATA allow us to make as much work as possible using device tree, but without having to put temporary nodes/properties into the .dts files themselves. That way, the DT bindings will only ever get added to in a compatible fashion, rather than going through multiple incompatible sets of requirements. Gosh. I have to say that I feel that peripheral IDs are the best solution for Tegra U-Boot until everything is worked out in the kernel. The problem here is that it requires the DT to change incompatibly later; it adds a property to the DT now that will be at best meaningless/unused in the future. If we simply don't add anything to the DT now, there's nothing to remove from the DT later. Newer U-Boots might require additional information in the DT (i.e. perhaps rely on full clock bindings) but won't deprecate any existing fields. We can't rely on phandles since they don't exist without an fdt, unless we mandate that everyone
Re: [U-Boot] [PATCH v2 09/17] tegra: usb: fdt: Add additional device tree definitions for USB ports
Hi Stephen, On Mon, Dec 12, 2011 at 10:13 AM, Stephen Warren swar...@nvidia.com wrote: On 12/08/2011 02:10 PM, Simon Glass wrote: Hi Stephen, On Wed, Dec 7, 2011 at 3:36 PM, Stephen Warren swar...@nvidia.com wrote: On 12/06/2011 02:09 PM, Simon Glass wrote: On Tue, Dec 6, 2011 at 12:28 PM, Stephen Warren swar...@nvidia.com wrote: On 12/05/2011 05:55 PM, Simon Glass wrote: On Mon, Dec 5, 2011 at 3:25 PM, Stephen Warren swar...@nvidia.com wrote: On 12/02/2011 07:11 PM, Simon Glass wrote: This adds peripheral IDs and timing information to the USB part of the device tree for U-Boot. The peripheral IDs provide easy access to clock registers. We will likely remove this in favor of a full clock tree when it is available in the kernel (but probably still retain the peripheral ID, just move it into a clock node). The USB timing information does apparently vary between boards sometimes, so is include in the fdt for convenience. usb@c500 { compatible = nvidia,tegra20-ehci, usb-ehci; reg = 0xc500 0x4000; interrupts = 52 ; phy_type = utmi; + periph-id = 22; // PERIPH_ID_USBD Given this is a temporary U-Boot-specific solution, can the property be named u-boot,periph-id so it's obvious that when writing a .dts for the kernel only, you don't care about this value. ok. I suggest the kernel does something similar. The kernel will use the standardized clock bindings once they're ready and we convert Tegra over to use them. The kernel is extremely unlikely to ever use periph-id or u-boot,periph-id. What is the time frame on this working be completed and merged? Sorry, I have no idea. I've been focusing on other subsystems (pinmux, audio) and haven't been following the clock stuff at all. Hopefully someone will start driving Tegra kernel towards common clock soon, but I don't think exactly who and when has been nailed down yet. Right now, the kernel's clock driver contains a mapping table from device name (e.g. tegra-ehci.2) to clock name (e.g. usb3). This allows the kernel USB driver to work without any explicit periph-id or similar DT property. Where does tegra-ehci.2 come from? I don't see that in the fdt. Pre-DT, everything was instantiated from platform devices. Each one had a name (tegra-ehci) and an instance number (2), which concatenate to tegra-ehci.2. All the clocks (and I think other resources like regulators) in the kernel were marked as being for use by a particular device name. For example in arch/arm/mach-tegra/tegra2_clocks.c: static struct clk tegra_list_clks[] = { ... PERIPH_CLK(usb3, tegra-ehci.2, ...), With DT, the device names typically don't follow this format (in this case, it'd be something more like /usb@c5008000). However, this prevented the clock lookups by device name from working, so a temporary scheme was put in place to keep the same device names. This is driven by AUXDATA, for example in arch/arm/mach-tegra/board-dt.c: struct of_dev_auxdata tegra20_auxdata_lookup[] __initdata = { ... // compatible, unit address, device name OF_DEV_AUXDATA(nvidia,tegra20-ehci, TEGRA_USB3_BASE, tegra-ehci.2, This means that any device with the given compatible property value, the given unit address will be named accordingly. This allows the existing clock/regulator lookups to work unmodified. Once DT bindings are in place for clocks, regulators, etc., the clock tables can be derived from DT, phandles will be used to match clocks and devices rather than device names, and the AUXDATA table can go away. The equivalent in U-Boot would be a table that maps from driver type (e.g. COMPAT_NVIDIA_TEGRA20_USB or perhaps NVIDIA_TEGRA20_USB?) and address to periph id. Again, once the clock bindings are complete and the nodes present in the .dts file, that mapping table can be removed and everything will work based on phandles. I'd like to point out here that everything is in a pretty big state of flux/development, since DT support for ARM is new. Temporary workarounds like AUXDATA allow us to make as much work as possible using device tree, but without having to put temporary nodes/properties into the .dts files themselves. That way, the DT bindings will only ever get added to in a compatible fashion, rather than going through multiple incompatible sets of requirements. Gosh. I have to say that I feel that peripheral IDs are the best solution for Tegra U-Boot until everything is worked out in the kernel. The problem here is that it requires the DT to change incompatibly later; it adds a property to the DT now that will be at best meaningless/unused in the future. If we simply don't add anything to the DT now, there's nothing to remove from the DT later. Newer U-Boots might require additional information in the DT (i.e. perhaps rely on full clock bindings) but won't deprecate any existing fields. I
Re: [U-Boot] [PATCH v2 09/17] tegra: usb: fdt: Add additional device tree definitions for USB ports
Hi Stephen, On Wed, Dec 7, 2011 at 3:36 PM, Stephen Warren swar...@nvidia.com wrote: On 12/06/2011 02:09 PM, Simon Glass wrote: On Tue, Dec 6, 2011 at 12:28 PM, Stephen Warren swar...@nvidia.com wrote: On 12/05/2011 05:55 PM, Simon Glass wrote: On Mon, Dec 5, 2011 at 3:25 PM, Stephen Warren swar...@nvidia.com wrote: On 12/02/2011 07:11 PM, Simon Glass wrote: This adds peripheral IDs and timing information to the USB part of the device tree for U-Boot. The peripheral IDs provide easy access to clock registers. We will likely remove this in favor of a full clock tree when it is available in the kernel (but probably still retain the peripheral ID, just move it into a clock node). The USB timing information does apparently vary between boards sometimes, so is include in the fdt for convenience. usb@c500 { compatible = nvidia,tegra20-ehci, usb-ehci; reg = 0xc500 0x4000; interrupts = 52 ; phy_type = utmi; + periph-id = 22; // PERIPH_ID_USBD Given this is a temporary U-Boot-specific solution, can the property be named u-boot,periph-id so it's obvious that when writing a .dts for the kernel only, you don't care about this value. ok. I suggest the kernel does something similar. The kernel will use the standardized clock bindings once they're ready and we convert Tegra over to use them. The kernel is extremely unlikely to ever use periph-id or u-boot,periph-id. What is the time frame on this working be completed and merged? Sorry, I have no idea. I've been focusing on other subsystems (pinmux, audio) and haven't been following the clock stuff at all. Hopefully someone will start driving Tegra kernel towards common clock soon, but I don't think exactly who and when has been nailed down yet. Right now, the kernel's clock driver contains a mapping table from device name (e.g. tegra-ehci.2) to clock name (e.g. usb3). This allows the kernel USB driver to work without any explicit periph-id or similar DT property. Where does tegra-ehci.2 come from? I don't see that in the fdt. Pre-DT, everything was instantiated from platform devices. Each one had a name (tegra-ehci) and an instance number (2), which concatenate to tegra-ehci.2. All the clocks (and I think other resources like regulators) in the kernel were marked as being for use by a particular device name. For example in arch/arm/mach-tegra/tegra2_clocks.c: static struct clk tegra_list_clks[] = { ... PERIPH_CLK(usb3, tegra-ehci.2, ...), With DT, the device names typically don't follow this format (in this case, it'd be something more like /usb@c5008000). However, this prevented the clock lookups by device name from working, so a temporary scheme was put in place to keep the same device names. This is driven by AUXDATA, for example in arch/arm/mach-tegra/board-dt.c: struct of_dev_auxdata tegra20_auxdata_lookup[] __initdata = { ... // compatible, unit address, device name OF_DEV_AUXDATA(nvidia,tegra20-ehci, TEGRA_USB3_BASE, tegra-ehci.2, This means that any device with the given compatible property value, the given unit address will be named accordingly. This allows the existing clock/regulator lookups to work unmodified. Once DT bindings are in place for clocks, regulators, etc., the clock tables can be derived from DT, phandles will be used to match clocks and devices rather than device names, and the AUXDATA table can go away. The equivalent in U-Boot would be a table that maps from driver type (e.g. COMPAT_NVIDIA_TEGRA20_USB or perhaps NVIDIA_TEGRA20_USB?) and address to periph id. Again, once the clock bindings are complete and the nodes present in the .dts file, that mapping table can be removed and everything will work based on phandles. I'd like to point out here that everything is in a pretty big state of flux/development, since DT support for ARM is new. Temporary workarounds like AUXDATA allow us to make as much work as possible using device tree, but without having to put temporary nodes/properties into the .dts files themselves. That way, the DT bindings will only ever get added to in a compatible fashion, rather than going through multiple incompatible sets of requirements. Gosh. I have to say that I feel that peripheral IDs are the best solution for Tegra U-Boot until everything is worked out in the kernel. We can't rely on phandles since they don't exist without an fdt, unless we mandate that everyone must use an fdt. I don't feel comfortable doing that until things are a bit more stable with all the things you are working on. I really can't see why we want to put a table in U-Boot which does a mapping that is clear a hardware feature and IMO belongs in the fdt (why repeat peripheral addresses in the code and the fdt?). Plus I still don't have an answer to my question about how we can ensure that instance 0 is a particular device. The fact that
Re: [U-Boot] [PATCH v2 09/17] tegra: usb: fdt: Add additional device tree definitions for USB ports
On 12/06/2011 02:09 PM, Simon Glass wrote: On Tue, Dec 6, 2011 at 12:28 PM, Stephen Warren swar...@nvidia.com wrote: On 12/05/2011 05:55 PM, Simon Glass wrote: On Mon, Dec 5, 2011 at 3:25 PM, Stephen Warren swar...@nvidia.com wrote: On 12/02/2011 07:11 PM, Simon Glass wrote: This adds peripheral IDs and timing information to the USB part of the device tree for U-Boot. The peripheral IDs provide easy access to clock registers. We will likely remove this in favor of a full clock tree when it is available in the kernel (but probably still retain the peripheral ID, just move it into a clock node). The USB timing information does apparently vary between boards sometimes, so is include in the fdt for convenience. usb@c500 { compatible = nvidia,tegra20-ehci, usb-ehci; reg = 0xc500 0x4000; interrupts = 52 ; phy_type = utmi; + periph-id = 22; // PERIPH_ID_USBD Given this is a temporary U-Boot-specific solution, can the property be named u-boot,periph-id so it's obvious that when writing a .dts for the kernel only, you don't care about this value. ok. I suggest the kernel does something similar. The kernel will use the standardized clock bindings once they're ready and we convert Tegra over to use them. The kernel is extremely unlikely to ever use periph-id or u-boot,periph-id. What is the time frame on this working be completed and merged? Sorry, I have no idea. I've been focusing on other subsystems (pinmux, audio) and haven't been following the clock stuff at all. Hopefully someone will start driving Tegra kernel towards common clock soon, but I don't think exactly who and when has been nailed down yet. Right now, the kernel's clock driver contains a mapping table from device name (e.g. tegra-ehci.2) to clock name (e.g. usb3). This allows the kernel USB driver to work without any explicit periph-id or similar DT property. Where does tegra-ehci.2 come from? I don't see that in the fdt. Pre-DT, everything was instantiated from platform devices. Each one had a name (tegra-ehci) and an instance number (2), which concatenate to tegra-ehci.2. All the clocks (and I think other resources like regulators) in the kernel were marked as being for use by a particular device name. For example in arch/arm/mach-tegra/tegra2_clocks.c: static struct clk tegra_list_clks[] = { ... PERIPH_CLK(usb3, tegra-ehci.2, ...), With DT, the device names typically don't follow this format (in this case, it'd be something more like /usb@c5008000). However, this prevented the clock lookups by device name from working, so a temporary scheme was put in place to keep the same device names. This is driven by AUXDATA, for example in arch/arm/mach-tegra/board-dt.c: struct of_dev_auxdata tegra20_auxdata_lookup[] __initdata = { ... // compatible, unit address, device name OF_DEV_AUXDATA(nvidia,tegra20-ehci, TEGRA_USB3_BASE, tegra-ehci.2, This means that any device with the given compatible property value, the given unit address will be named accordingly. This allows the existing clock/regulator lookups to work unmodified. Once DT bindings are in place for clocks, regulators, etc., the clock tables can be derived from DT, phandles will be used to match clocks and devices rather than device names, and the AUXDATA table can go away. The equivalent in U-Boot would be a table that maps from driver type (e.g. COMPAT_NVIDIA_TEGRA20_USB or perhaps NVIDIA_TEGRA20_USB?) and address to periph id. Again, once the clock bindings are complete and the nodes present in the .dts file, that mapping table can be removed and everything will work based on phandles. I'd like to point out here that everything is in a pretty big state of flux/development, since DT support for ARM is new. Temporary workarounds like AUXDATA allow us to make as much work as possible using device tree, but without having to put temporary nodes/properties into the .dts files themselves. That way, the DT bindings will only ever get added to in a compatible fashion, rather than going through multiple incompatible sets of requirements. -- nvpublic ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 09/17] tegra: usb: fdt: Add additional device tree definitions for USB ports
On 12/05/2011 05:55 PM, Simon Glass wrote: On Mon, Dec 5, 2011 at 3:25 PM, Stephen Warren swar...@nvidia.com wrote: On 12/02/2011 07:11 PM, Simon Glass wrote: This adds peripheral IDs and timing information to the USB part of the device tree for U-Boot. The peripheral IDs provide easy access to clock registers. We will likely remove this in favor of a full clock tree when it is available in the kernel (but probably still retain the peripheral ID, just move it into a clock node). The USB timing information does apparently vary between boards sometimes, so is include in the fdt for convenience. Which parts vary? Most of it is PLL configuration, and it seems basically impossible for that to vary yet still create the correct output frequency. See here for T30 differences, for example. https://gerrit.chromium.org/gerrit/#patch,sidebyside,12440,1,board/nvidia/cardhu/tegra30.dtsi OK, so that varies by SoC not board. +/* This table has USB timing parameters for each Oscillator frequency we + * support. There are four sets of values: + * + * 1. PLLU configuration information (reference clock is osc/clk_m and + * PLLU-FOs are fixed at 12MHz/60MHz/480MHz). + * + * Reference frequency 13.0MHz 19.2MHz 12.0MHz 26.0MHz + * -- + * DIVN960 (0x3c0) 200 (0c8)960 (3c0h) 960 (3c0) + * DIVM13 (0d) 4 (04) 12 (0c) 26 (1a) + * Filter frequency (MHz) 14.8 62 + * CPCON1100b0011b1100b1100b + * LFCON0 0000 + * + * 2. PLL CONFIGURATION PARAMETERS for different clock generators: + * + * Reference frequency 13.0MHz 19.2MHz 12.0MHz 26.0MHz + * --- + * PLLU_ENABLE_DLY_COUNT 02 (0x02) 03 (03) 02 (02) 04 (04) + * PLLU_STABLE_COUNT 51 (33) 75 (4B) 47 (2F)102 (66) + * PLL_ACTIVE_DLY_COUNT05 (05) 06 (06) 04 (04) 09 (09) + * XTAL_FREQ_COUNT127 (7F)187 (BB)118 (76)254 (FE) Those two sets of data seem like they should simply be part of the clock driver. Some part of the system boot or USB init process needs to say enable the USB clocks, and that code needs to write those hard-coded values into the relevant clock registers (or calculate them at run-time; whatever). This stuff can't vary by board. We don't have a USB clock driver. This driver is in CPU code, so I am not suggesting that it varies by board, only by SOC. This stuff is in the SOC dts. + * 3. Debounce values IdDig, Avalid, Bvalid, VbusValid, VbusWakeUp, and + * SessEnd. Each of these signals have their own debouncer and for each of + * those one out of two debouncing times can be chosen (BIAS_DEBOUNCE_A or + * BIAS_DEBOUNCE_B). + * + * The values of DEBOUNCE_A and DEBOUNCE_B are calculated as follows: + *0x - No debouncing at all + *n ms = n *1000 / (1/19.2MHz) / 4 + * + * So to program a 1 ms debounce for BIAS_DEBOUNCE_A, we have: + * BIAS_DEBOUNCE_A[15:0] = 1000 * 19.2 / 4 = 4800 = 0x12c0 + * + * We need to use only DebounceA for BOOTROM. We don't need the DebounceB + * values, so we can keep those to default. + * + * a4. The 20 microsecond delay after bias cell operation. ^ typo + * Reference frequency 13.0MHz 19.2MHz 12.0MHz 26.0MHz If this data varies at all, then those two sets of data seem port-specific to me, and hence should be moved into a per-port property. Having a single global set of parameters that gets applied to all ports at once doesn't make sense to me, if the data varies. The hardware doesn't support this does it? It looks like it doesn't; there are two debounce count registers and each port (or something) can select which of those two debounce values is used. I imagine the values are board specific too, and hence shouldn't be in tegra20.dtsi but rather in tegra-seaboard.dts. I believe they vary by SOC, not board. The values in these fields should be specific in a way that's agnostic to the reference clock, e.g. as a number of mS/nS. That way, you'd just specify the single set of values to use, and the driver would do the calculations to map the time domain into the reference-clock-specific values to put into the registers. Yes I agree. I don't have time to write this. Shall we go forward with what we have, and Nvidia can tidy up later? I definitely oppose that; the current bindings are simply not conceptually correct. I'm opposed to merging something that's known to be broken even if there's a clear intent/timeframe to fix it; it'd be better to just defer merging it. (Well, even better to fix it to be right an merge it!) As a blanket
Re: [U-Boot] [PATCH v2 09/17] tegra: usb: fdt: Add additional device tree definitions for USB ports
Hi Stephen, On Tue, Dec 6, 2011 at 12:28 PM, Stephen Warren swar...@nvidia.com wrote: On 12/05/2011 05:55 PM, Simon Glass wrote: On Mon, Dec 5, 2011 at 3:25 PM, Stephen Warren swar...@nvidia.com wrote: On 12/02/2011 07:11 PM, Simon Glass wrote: This adds peripheral IDs and timing information to the USB part of the device tree for U-Boot. The peripheral IDs provide easy access to clock registers. We will likely remove this in favor of a full clock tree when it is available in the kernel (but probably still retain the peripheral ID, just move it into a clock node). The USB timing information does apparently vary between boards sometimes, so is include in the fdt for convenience. Which parts vary? Most of it is PLL configuration, and it seems basically impossible for that to vary yet still create the correct output frequency. See here for T30 differences, for example. https://gerrit.chromium.org/gerrit/#patch,sidebyside,12440,1,board/nvidia/cardhu/tegra30.dtsi OK, so that varies by SoC not board. Yes that's why I put it in tegra20.dtsi. But I have removed it now. +/* This table has USB timing parameters for each Oscillator frequency we + * support. There are four sets of values: + * + * 1. PLLU configuration information (reference clock is osc/clk_m and + * PLLU-FOs are fixed at 12MHz/60MHz/480MHz). + * + * Reference frequency 13.0MHz 19.2MHz 12.0MHz 26.0MHz + * -- + * DIVN 960 (0x3c0) 200 (0c8) 960 (3c0h) 960 (3c0) + * DIVM 13 (0d) 4 (04) 12 (0c) 26 (1a) + * Filter frequency (MHz) 1 4.8 6 2 + * CPCON 1100b 0011b 1100b 1100b + * LFCON0 0 0 0 0 + * + * 2. PLL CONFIGURATION PARAMETERS for different clock generators: + * + * Reference frequency 13.0MHz 19.2MHz 12.0MHz 26.0MHz + * --- + * PLLU_ENABLE_DLY_COUNT 02 (0x02) 03 (03) 02 (02) 04 (04) + * PLLU_STABLE_COUNT 51 (33) 75 (4B) 47 (2F) 102 (66) + * PLL_ACTIVE_DLY_COUNT 05 (05) 06 (06) 04 (04) 09 (09) + * XTAL_FREQ_COUNT 127 (7F) 187 (BB) 118 (76) 254 (FE) Those two sets of data seem like they should simply be part of the clock driver. Some part of the system boot or USB init process needs to say enable the USB clocks, and that code needs to write those hard-coded values into the relevant clock registers (or calculate them at run-time; whatever). This stuff can't vary by board. We don't have a USB clock driver. This driver is in CPU code, so I am not suggesting that it varies by board, only by SOC. This stuff is in the SOC dts. + * 3. Debounce values IdDig, Avalid, Bvalid, VbusValid, VbusWakeUp, and + * SessEnd. Each of these signals have their own debouncer and for each of + * those one out of two debouncing times can be chosen (BIAS_DEBOUNCE_A or + * BIAS_DEBOUNCE_B). + * + * The values of DEBOUNCE_A and DEBOUNCE_B are calculated as follows: + * 0x - No debouncing at all + * n ms = n *1000 / (1/19.2MHz) / 4 + * + * So to program a 1 ms debounce for BIAS_DEBOUNCE_A, we have: + * BIAS_DEBOUNCE_A[15:0] = 1000 * 19.2 / 4 = 4800 = 0x12c0 + * + * We need to use only DebounceA for BOOTROM. We don't need the DebounceB + * values, so we can keep those to default. + * + * a4. The 20 microsecond delay after bias cell operation. ^ typo + * Reference frequency 13.0MHz 19.2MHz 12.0MHz 26.0MHz If this data varies at all, then those two sets of data seem port-specific to me, and hence should be moved into a per-port property. Having a single global set of parameters that gets applied to all ports at once doesn't make sense to me, if the data varies. The hardware doesn't support this does it? It looks like it doesn't; there are two debounce count registers and each port (or something) can select which of those two debounce values is used. I imagine the values are board specific too, and hence shouldn't be in tegra20.dtsi but rather in tegra-seaboard.dts. I believe they vary by SOC, not board. The values in these fields should be specific in a way that's agnostic to the reference clock, e.g. as a number of mS/nS. That way, you'd just specify the single set of values to use, and the driver would do the calculations to map the time domain into the reference-clock-specific values to put into the registers. Yes I agree. I don't have time to write this. Shall we go forward with what we have, and Nvidia can tidy up later? I definitely oppose that; the current bindings are simply not conceptually correct. I'm opposed to merging something that's known to be broken even if
Re: [U-Boot] [PATCH v2 09/17] tegra: usb: fdt: Add additional device tree definitions for USB ports
On 12/02/2011 07:11 PM, Simon Glass wrote: This adds peripheral IDs and timing information to the USB part of the device tree for U-Boot. The peripheral IDs provide easy access to clock registers. We will likely remove this in favor of a full clock tree when it is available in the kernel (but probably still retain the peripheral ID, just move it into a clock node). The USB timing information does apparently vary between boards sometimes, so is include in the fdt for convenience. Which parts vary? Most of it is PLL configuration, and it seems basically impossible for that to vary yet still create the correct output frequency. +/* This table has USB timing parameters for each Oscillator frequency we This comment block should be indented to match the nodes. + * support. There are four sets of values: + * + * 1. PLLU configuration information (reference clock is osc/clk_m and + * PLLU-FOs are fixed at 12MHz/60MHz/480MHz). + * + * Reference frequency 13.0MHz 19.2MHz 12.0MHz 26.0MHz + * -- + * DIVN960 (0x3c0) 200 (0c8)960 (3c0h) 960 (3c0) + * DIVM13 (0d) 4 (04) 12 (0c) 26 (1a) + * Filter frequency (MHz) 14.8 62 + * CPCON1100b0011b1100b1100b + * LFCON0 0000 + * + * 2. PLL CONFIGURATION PARAMETERS for different clock generators: + * + * Reference frequency 13.0MHz 19.2MHz 12.0MHz 26.0MHz + * --- + * PLLU_ENABLE_DLY_COUNT 02 (0x02) 03 (03) 02 (02) 04 (04) + * PLLU_STABLE_COUNT 51 (33) 75 (4B) 47 (2F)102 (66) + * PLL_ACTIVE_DLY_COUNT05 (05) 06 (06) 04 (04) 09 (09) + * XTAL_FREQ_COUNT127 (7F)187 (BB)118 (76)254 (FE) Those two sets of data seem like they should simply be part of the clock driver. Some part of the system boot or USB init process needs to say enable the USB clocks, and that code needs to write those hard-coded values into the relevant clock registers (or calculate them at run-time; whatever). This stuff can't vary by board. + * 3. Debounce values IdDig, Avalid, Bvalid, VbusValid, VbusWakeUp, and + * SessEnd. Each of these signals have their own debouncer and for each of + * those one out of two debouncing times can be chosen (BIAS_DEBOUNCE_A or + * BIAS_DEBOUNCE_B). + * + * The values of DEBOUNCE_A and DEBOUNCE_B are calculated as follows: + *0x - No debouncing at all + *n ms = n *1000 / (1/19.2MHz) / 4 + * + * So to program a 1 ms debounce for BIAS_DEBOUNCE_A, we have: + * BIAS_DEBOUNCE_A[15:0] = 1000 * 19.2 / 4 = 4800 = 0x12c0 + * + * We need to use only DebounceA for BOOTROM. We don't need the DebounceB + * values, so we can keep those to default. + * + * a4. The 20 microsecond delay after bias cell operation. ^ typo + * Reference frequency 13.0MHz 19.2MHz 12.0MHz 26.0MHz If this data varies at all, then those two sets of data seem port-specific to me, and hence should be moved into a per-port property. Having a single global set of parameters that gets applied to all ports at once doesn't make sense to me, if the data varies. I imagine the values are board specific too, and hence shouldn't be in tegra20.dtsi but rather in tegra-seaboard.dts. The values in these fields should be specific in a way that's agnostic to the reference clock, e.g. as a number of mS/nS. That way, you'd just specify the single set of values to use, and the driver would do the calculations to map the time domain into the reference-clock-specific values to put into the registers. + */ + + usbtiming@0 { + compatible = nvidia,tegra20-usbtiming; + osc-frequency = 1300; + /* DivN, DivM, DivP, CPCON, LFCON, Delays Debounce, Bias */ + timing = 0x3c0 0x0d 0x00 0xc 0 0x02 0x33 0x05 0x7f 0x7ef4 5; + }; + + usbtiming@1 { + compatible = nvidia,tegra20-usbtiming; + osc-frequency = 1920; + timing = 0x0c8 0x04 0x00 0x3 0 0x03 0x4b 0x06 0xbb 0xbb80 7; + }; + + usbtiming@2 { + compatible = nvidia,tegra20-usbtiming; + osc-frequency = 1200; + timing = 0x3c0 0x0c 0x00 0xc 0 0x02 0x2f 0x04 0x76 0x7530 5; + }; + + usbtiming@3 { + compatible = nvidia,tegra20-usbtiming; + osc-frequency = 2600; + timing = 0x3c0 0x1a 0x00 0xc 0 0x04 0x66 0x09 0xfe 0xfde8 9; + }; You can't include those timing nodes right there, because the unit addresses (@0, ... @3) aren't physical addresses in the same scheme as all the other nodes. +
Re: [U-Boot] [PATCH v2 09/17] tegra: usb: fdt: Add additional device tree definitions for USB ports
Hi Stephen, On Mon, Dec 5, 2011 at 3:25 PM, Stephen Warren swar...@nvidia.com wrote: On 12/02/2011 07:11 PM, Simon Glass wrote: This adds peripheral IDs and timing information to the USB part of the device tree for U-Boot. The peripheral IDs provide easy access to clock registers. We will likely remove this in favor of a full clock tree when it is available in the kernel (but probably still retain the peripheral ID, just move it into a clock node). The USB timing information does apparently vary between boards sometimes, so is include in the fdt for convenience. Which parts vary? Most of it is PLL configuration, and it seems basically impossible for that to vary yet still create the correct output frequency. See here for T30 differences, for example. https://gerrit.chromium.org/gerrit/#patch,sidebyside,12440,1,board/nvidia/cardhu/tegra30.dtsi +/* This table has USB timing parameters for each Oscillator frequency we This comment block should be indented to match the nodes. + * support. There are four sets of values: + * + * 1. PLLU configuration information (reference clock is osc/clk_m and + * PLLU-FOs are fixed at 12MHz/60MHz/480MHz). + * + * Reference frequency 13.0MHz 19.2MHz 12.0MHz 26.0MHz + * -- + * DIVN 960 (0x3c0) 200 (0c8) 960 (3c0h) 960 (3c0) + * DIVM 13 (0d) 4 (04) 12 (0c) 26 (1a) + * Filter frequency (MHz) 1 4.8 6 2 + * CPCON 1100b 0011b 1100b 1100b + * LFCON0 0 0 0 0 + * + * 2. PLL CONFIGURATION PARAMETERS for different clock generators: + * + * Reference frequency 13.0MHz 19.2MHz 12.0MHz 26.0MHz + * --- + * PLLU_ENABLE_DLY_COUNT 02 (0x02) 03 (03) 02 (02) 04 (04) + * PLLU_STABLE_COUNT 51 (33) 75 (4B) 47 (2F) 102 (66) + * PLL_ACTIVE_DLY_COUNT 05 (05) 06 (06) 04 (04) 09 (09) + * XTAL_FREQ_COUNT 127 (7F) 187 (BB) 118 (76) 254 (FE) Those two sets of data seem like they should simply be part of the clock driver. Some part of the system boot or USB init process needs to say enable the USB clocks, and that code needs to write those hard-coded values into the relevant clock registers (or calculate them at run-time; whatever). This stuff can't vary by board. We don't have a USB clock driver. This driver is in CPU code, so I am not suggesting that it varies by board, only by SOC. This stuff is in the SOC dts. + * 3. Debounce values IdDig, Avalid, Bvalid, VbusValid, VbusWakeUp, and + * SessEnd. Each of these signals have their own debouncer and for each of + * those one out of two debouncing times can be chosen (BIAS_DEBOUNCE_A or + * BIAS_DEBOUNCE_B). + * + * The values of DEBOUNCE_A and DEBOUNCE_B are calculated as follows: + * 0x - No debouncing at all + * n ms = n *1000 / (1/19.2MHz) / 4 + * + * So to program a 1 ms debounce for BIAS_DEBOUNCE_A, we have: + * BIAS_DEBOUNCE_A[15:0] = 1000 * 19.2 / 4 = 4800 = 0x12c0 + * + * We need to use only DebounceA for BOOTROM. We don't need the DebounceB + * values, so we can keep those to default. + * + * a4. The 20 microsecond delay after bias cell operation. ^ typo + * Reference frequency 13.0MHz 19.2MHz 12.0MHz 26.0MHz If this data varies at all, then those two sets of data seem port-specific to me, and hence should be moved into a per-port property. Having a single global set of parameters that gets applied to all ports at once doesn't make sense to me, if the data varies. The hardware doesn't support this does it? I imagine the values are board specific too, and hence shouldn't be in tegra20.dtsi but rather in tegra-seaboard.dts. I believe they vary by SOC, not board. The values in these fields should be specific in a way that's agnostic to the reference clock, e.g. as a number of mS/nS. That way, you'd just specify the single set of values to use, and the driver would do the calculations to map the time domain into the reference-clock-specific values to put into the registers. Yes I agree. I don't have time to write this. Shall we go forward with what we have, and Nvidia can tidy up later? + */ + + usbtiming@0 { + compatible = nvidia,tegra20-usbtiming; + osc-frequency = 1300; + /* DivN, DivM, DivP, CPCON, LFCON, Delays Debounce, Bias */ + timing = 0x3c0 0x0d 0x00 0xc 0 0x02 0x33 0x05 0x7f 0x7ef4 5; + }; + + usbtiming@1 { + compatible = nvidia,tegra20-usbtiming; + osc-frequency = 1920; + timing = 0x0c8 0x04 0x00 0x3 0 0x03 0x4b 0x06 0xbb
[U-Boot] [PATCH v2 09/17] tegra: usb: fdt: Add additional device tree definitions for USB ports
This adds peripheral IDs and timing information to the USB part of the device tree for U-Boot. The peripheral IDs provide easy access to clock registers. We will likely remove this in favor of a full clock tree when it is available in the kernel (but probably still retain the peripheral ID, just move it into a clock node). The USB timing information does apparently vary between boards sometimes, so is include in the fdt for convenience. Signed-off-by: Simon Glass s...@chromium.org --- Changes in v2: - Change fdt property usbparmas to usbtiming - Add details for port USB2 arch/arm/dts/tegra20.dtsi | 69 + 1 files changed, 69 insertions(+), 0 deletions(-) diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi index 6146d24..d7ab843 100644 --- a/arch/arm/dts/tegra20.dtsi +++ b/arch/arm/dts/tegra20.dtsi @@ -144,11 +144,78 @@ interrupts = 63 ; }; +/* This table has USB timing parameters for each Oscillator frequency we + * support. There are four sets of values: + * + * 1. PLLU configuration information (reference clock is osc/clk_m and + * PLLU-FOs are fixed at 12MHz/60MHz/480MHz). + * + * Reference frequency 13.0MHz 19.2MHz 12.0MHz 26.0MHz + * -- + * DIVN960 (0x3c0) 200 (0c8)960 (3c0h) 960 (3c0) + * DIVM13 (0d) 4 (04) 12 (0c) 26 (1a) + * Filter frequency (MHz) 14.8 62 + * CPCON1100b0011b1100b1100b + * LFCON0 0000 + * + * 2. PLL CONFIGURATION PARAMETERS for different clock generators: + * + * Reference frequency 13.0MHz 19.2MHz 12.0MHz 26.0MHz + * --- + * PLLU_ENABLE_DLY_COUNT 02 (0x02) 03 (03) 02 (02) 04 (04) + * PLLU_STABLE_COUNT 51 (33) 75 (4B) 47 (2F)102 (66) + * PLL_ACTIVE_DLY_COUNT05 (05) 06 (06) 04 (04) 09 (09) + * XTAL_FREQ_COUNT127 (7F)187 (BB)118 (76)254 (FE) + * + * 3. Debounce values IdDig, Avalid, Bvalid, VbusValid, VbusWakeUp, and + * SessEnd. Each of these signals have their own debouncer and for each of + * those one out of two debouncing times can be chosen (BIAS_DEBOUNCE_A or + * BIAS_DEBOUNCE_B). + * + * The values of DEBOUNCE_A and DEBOUNCE_B are calculated as follows: + *0x - No debouncing at all + *n ms = n *1000 / (1/19.2MHz) / 4 + * + * So to program a 1 ms debounce for BIAS_DEBOUNCE_A, we have: + * BIAS_DEBOUNCE_A[15:0] = 1000 * 19.2 / 4 = 4800 = 0x12c0 + * + * We need to use only DebounceA for BOOTROM. We don't need the DebounceB + * values, so we can keep those to default. + * + * a4. The 20 microsecond delay after bias cell operation. + * Reference frequency 13.0MHz 19.2MHz 12.0MHz 26.0MHz + */ + usbtiming@0 { + compatible = nvidia,tegra20-usbtiming; + osc-frequency = 1300; + /* DivN, DivM, DivP, CPCON, LFCON, Delays Debounce, Bias */ + timing = 0x3c0 0x0d 0x00 0xc 0 0x02 0x33 0x05 0x7f 0x7ef4 5; + }; + + usbtiming@1 { + compatible = nvidia,tegra20-usbtiming; + osc-frequency = 1920; + timing = 0x0c8 0x04 0x00 0x3 0 0x03 0x4b 0x06 0xbb 0xbb80 7; + }; + + usbtiming@2 { + compatible = nvidia,tegra20-usbtiming; + osc-frequency = 1200; + timing = 0x3c0 0x0c 0x00 0xc 0 0x02 0x2f 0x04 0x76 0x7530 5; + }; + + usbtiming@3 { + compatible = nvidia,tegra20-usbtiming; + osc-frequency = 2600; + timing = 0x3c0 0x1a 0x00 0xc 0 0x04 0x66 0x09 0xfe 0xfde8 9; + }; + usb@c500 { compatible = nvidia,tegra20-ehci, usb-ehci; reg = 0xc500 0x4000; interrupts = 52 ; phy_type = utmi; + periph-id = 22; // PERIPH_ID_USBD }; usb@c5004000 { @@ -156,6 +223,7 @@ reg = 0xc5004000 0x4000; interrupts = 53 ; phy_type = ulpi; + periph-id = 58; // PERIPH_ID_USB2 }; usb@c5008000 { @@ -163,6 +231,7 @@ reg = 0xc5008000 0x4000; interrupts = 129 ; phy_type = utmi; + periph-id = 59; // PERIPH_ID_USB3 }; }; -- 1.7.3.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot