Hi Stephen,
On Fri, Dec 2, 2011 at 8:10 AM, Stephen Warren swarren@nvidia.com wrote:
On 12/01/2011 06:51 PM, Simon Glass wrote:
Hi Stephen,
On Mon, Nov 28, 2011 at 11:21 AM, Stephen Warren swarren@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.
+#ifdef CONFIG_OF_CONTROL +int fdt_decode_usb(const void *blob, int node, unsigned osc_frequency_mhz,
- struct fdt_usb *config)
+{
- int clk_node = 0, rate;
- /* Find the parameters for our oscillator frequency */
- do {
- clk_node = fdt_node_offset_by_compatible(blob, clk_node,
- "nvidia,tegra20-usbparams");
- if (clk_node < 0) {
- debug("Cannot find USB params for clock %u",
- osc_frequency_mhz);
- return -FDT_ERR_NOTFOUND;
- }
- rate = fdtdec_get_int(blob, clk_node, "osc-frequency", 0);
- } while (rate != osc_frequency_mhz);
- config->reg = (struct usb_ctlr *)fdtdec_get_addr(blob, node, "reg");
- config->host_mode = fdtdec_get_bool(blob, node, "support-host-mode");
Property "support-host-mode" isn't something that's supported by the kernel binding, and needs discussion/ack there.
Do you mean device-tree list? I did send the patch there.
linux-tegra and perhaps linux-usb too.
OK
Does the kernel have another way of knowing that this port is special?
When booted without DT, the platform data defines mode: device, host, OTG.
The current DT bindings for Tegra USB are the minimum required to get host mode working, and don't include any representation of this, and so host mode is assumed.
OK, I will take a closer look, but may keep this in anticipation of the kernel catching up.
- config->utmi = fdtdec_lookup_phandle(blob, node, "utmi") >= 0;
In the kernel DT binding, this is property "phy_type" with legal values "utmi" or "ulpi."
- 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.
- if (config->periph_id == -1)
- return -FDT_ERR_NOTFOUND;
- return fdtdec_get_int_array(blob, clk_node, "params", config->params,
- PARAM_COUNT);
Property "params" (which should probably be named something better anyway) isn't something that's supported by the kernel binding, and needs discussion/ack there. Instead, I suggest following the kernel's approach for now - don't specify these PHY parameters in the binding, but rather just apply the defaults, which are apparently suitable for all the boards supported by the mainline kernel at least.
My actual intent was that the device tree would provide options for each oscillator frequency and the board would simply select which it is using. This does not fit well with out the device tree works though - we end up having everything in there and available at run-time, even useless data. Anyway, the oscillator frequency is detected at run-time, so I decided to put these into the SOC description.
Since you say these values are fixed for all boards we might as well put them back into the code.
The values aren't necessarily fixed for /all/ boards, they just /happen/ to be identical for all boards currently supported in the mainline kernel. I imagine that as more boards are supported, we'll see different sets of values in use.
OK so perhaps I should keep my bindings here...
+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?
If U-boot were to operate solely based on the aliases node, it wouldn't work at all for the .dts files currently in the kernel since there are no aliases, and if there were aliases, might end up instantiating a subset of devices if some devices weren't mentioned in aliases.
I believe what aliases is meant for is that once you've enumerated all the devices, then check the aliases node in order to override the default naming (or set up alternative names). Still, you'd have to check with a DT expert here, since I've never done anything with the aliases node.
I did go around this loop some time ago and my understanding was that aliases were the correct way to enumerate ports (I originally just had an ID in the port which is easier to decode).
Regards, Simon
-- nvpublic