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@c5000000 { >>>>>>>> compatible = "nvidia,tegra20-ehci", "usb-ehci"; >>>>>>>> reg = <0xc5000000 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 really don't see the difference between changing the code later and changing the FDT later. The full clock bindings could be ages away. We have already given it a u-boot prefix. We might even decided to keep it for efficiency reasons. > >> 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. > > Sure, phandles won't work until the complete clock binding is implemnted. No I mean they won't work if someone isn't using CONFIG_OF_CONTROL and thus doesn't have a device tree at all. This option is optional in U-Boot, and in fact has only been in mainline for 5 minutes, and there are no users up until this patch. FDT still needs to prove its worth in U-Boot. > >> 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?). > > It's a HW feature of the clock/reset controller, not the USB controller. I think I was responding to your request to have tables in U-Boot like: <device_type>, <peripheral_address> -> clock ID My objection is that <device_type> is already encoded in the compatible string, <peripheral_address> is in the reg property, and we can avoid the same altogether by putting the peripheral ID in the FDT too. In fact I thought we already agreed on u-boot,periph-id. > >> Plus I still don't have an answer to my question about how we can >> ensure that instance 0 is a particular device. > > As I said before, in the context of USB (where IIRC the question was > asked), you can enable just a single USB controller. The code only > supports a single controller anyway. Yes but it must be the correct controller. In other words, on Seaboard the side controller must be first for USB to work. > > For SD/MMC, it does make sense to statically name some/all devices. That > is what /aliases is for. It's just that as I said, /aliases is meant to > control naming of devices that have been enumerated, not control the > enumeration itself. And UARTs I think. So I think this means we are happy with the aliases approach, and we are now just discussing the algorithm. Regards, SImon > > -- > nvpublic _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot