Re: [U-Boot] [PATCH v2 09/17] tegra: usb: fdt: Add additional device tree definitions for USB ports

2011-12-12 Thread Stephen Warren
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

2011-12-12 Thread Simon Glass
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

2011-12-08 Thread Simon Glass
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

2011-12-07 Thread Stephen Warren
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

2011-12-06 Thread Stephen Warren
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

2011-12-06 Thread Simon Glass
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

2011-12-05 Thread Stephen Warren
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

2011-12-05 Thread Simon Glass
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

2011-12-02 Thread Simon Glass
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