Hi Stephen, On Fri, Dec 2, 2011 at 12:40 PM, Stephen Warren <swar...@nvidia.com> wrote: > On 12/02/2011 10:00 AM, Simon Glass wrote: >> On Fri, Dec 2, 2011 at 8:10 AM, Stephen Warren <swar...@nvidia.com> wrote: >>> On 12/01/2011 06:51 PM, Simon Glass wrote: >>>> On Mon, Nov 28, 2011 at 11:21 AM, Stephen Warren <swar...@nvidia.com> >>>> wrote: >>>>> On 11/23/2011 08:54 PM, Simon Glass wrote: >>>>>> This adds basic support for the Tegra2 USB controller. Board files should >>>>>> call board_usb_init() to set things up. > ... >>>>>> + config->enabled = fdtdec_get_is_enabled(blob, node); >>>>>> + config->periph_id = fdtdec_get_int(blob, node, "periph-id", -1); >>>>> >>>>> periph-id is a U-Boot specific concept, not HW description. The DT >>>>> shouldn't contain that value. >>>> >>>> It is actually the bit position of the peripheral in the clock >>>> registers, so arguably a hardware description. U-Boot uses this to >>>> efficiently manage peripheral clocks, reset, pinmux, etc. >>>> >>>> How does the kernel figure out the clock register (etc.) to use with a >>>> particular peripheral? Also bear in mind that the intent with U-Boot >>>> is to be a lot more lightweight with these things. >>> >>> The DT binding has to be identical though; U-Boot implementation details >>> aren't supposed to affect the content of the DT. >>> >>> Clock bindings are an area of active development. I haven't been >>> following the progress, but I imagine that the clock controller will >>> define a node per clock, and the devices that consume the clock will >>> refer to that node using a phandle. It's then up to the clock controller >>> driver to extract whatever information it needs from the clock node and >>> map that to an internal periph-id. It's plausible that a legitimate part >>> of the clock binding itself is such a periph-id field, but that should >>> be defined by the clock controller binding, not the peripheral binding. >> >> OK, well this is an example of where I would like to run with what we >> have, and adjust it later when things are finalized in the kernel. >> >> I'm not sure about your analysis here actually. The peripherals have a >> selectable source clock and their own divider from that clock, plus >> they have bits for enabling their internal clock and reset. The >> registers for all of these can sort-of be indexed through the >> peripheral ID. I think with this model you would need to have a >> separate clock node for every peripheral, with the peripheral node >> pointing back to that. Perhaps that is what you mean. It means that >> every peripheral has its own node and then a clock node. It probably >> won't be too slow to decode. > > re: the last-but-one sentence: Yes, I think that's how it'll work. > >>>>>> +int board_usb_init(const void *blob) >>>>>> +{ >>>>>> +#ifdef CONFIG_OF_CONTROL >>>>>> + struct fdt_usb config; >>>>>> + int clk_done = 0; >>>>>> + int node, upto = 0; >>>>>> + unsigned osc_freq = clock_get_rate(CLOCK_ID_OSC); >>>>>> + >>>>>> + do { >>>>>> + node = fdtdec_next_alias(blob, "usb", >>>>>> + COMPAT_NVIDIA_TEGRA20_USB, >>>>>> &upto); >>>>> >>>>> Why only initialize USB controllers with aliases? Surely this should >>>>> enumerate all nodes with a specific compatible flag? >>>> >>>> The aliases are (I thought) the official way that device trees specify >>>> device ordering. No we do not enumerate things in U-Boot - there is no >>>> device model as such. We can do this on Tegra, but still need to know >>>> the order to use (i.e. which is port 0). >>> >>> I don't believe the kernel uses the alias for anything at all right now. >>> Instead, it enumerates all nodes that match a certain compatible flag, >>> and instantiates a device for each one it has a driver for. I believe >>> this mode of operation is pretty implicit in DT itself; it's something >>> U-Boot should do too. >> >> It does this at present with USB. But we want to enumerate the ports >> and know which is port 0, which is port 1, etc. How does the kernel do >> that? > > I don't think it cares; it just hosts a number of USB ports, and > peripherals show up on those USB ports. The numbering of the ports is > entirely arbitrary AFAIK.
OK. For the moment in U-Boot we do care, so I will leave the alias solution in there for now. I may look later at the patch to suppose a virtual hub on Tegra. Regards, Simon > > -- > nvpublic > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot