[U-Boot] [PATCH 10/14] tegra: usb: Add support for USB peripheral

[Stephen Warren]

On 12/02/2011 10:00 AM, Simon Glass wrote:
> On Fri, Dec 2, 2011 at 8:10 AM, Stephen Warren <swarren at nvidia.com> wrote:
>> On 12/01/2011 06:51 PM, Simon Glass wrote:
>>> On Mon, Nov 28, 2011 at 11:21 AM, Stephen Warren <swarren at 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.

-- 
nvpublic