| Message ID | eaf050fbd62803784856a4f37d4b332f7778e22c.1384798508.git.jason@lakedaemon.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/18/2013 11:38 AM, Jason Cooper wrote: > Unlike other build products in the Linux kernel, the devicetree blobs > are simply the name of their source file, s/dts/dtb/. There is also no > 'make *install' mechanism to put them in a standard place with a > standard naming structure. > > Unfortunately, users have begun scripting pulling the needed dtbs from > within the kernel tree, thus hardcoding the dtbs names. In turn, this > means any changes to the dts filenames breaks these scripts. > > This patch is an attempt to fix this problem. Akin to 'make install', > this creates a new make target, dtbs_install. The script that gets > called defers to a vendor or distribution supplied installdtbs binary, > if found in the system. Otherwise, the default action is to install a > given dtb into > > /lib/modules/${kernel_version}/devicetree/${board_compat}.dtb Is co-mingling the DTs in the same (top-level) directory as modules a good idea. I guess there is an explicit devicetree/ sub-directory so they're easy to pull out of the source tree, but even so, they're certainly not modules. I would assume that distros would put them into e.g. /boot/dtbs/${kernelversion} or something more like that. Would it make sense for the install target to use a path more like that, or perhaps at least /dtbs/${kernelversion} to keep it separate from modules? Sorry for the bikeshedding. > diff --git a/arch/arm/Makefile b/arch/arm/Makefile > +dtbs_install: dtbs > + $(CONFIG_SHELL) $(srctree)/$(boot)/installdtbs.sh $(KERNELRELEASE) \ > + "$(MODLIB)/devicetree" "$(srctree)/$(boot)/dts" Architectures besides ARM use device trees. Shouldn't "make dtbs_install" work for them too? > diff --git a/arch/arm/boot/installdtbs.sh b/arch/arm/boot/installdtbs.sh > +for dtb in `find "$3" -name "*.dtb"`; do > + # we use dtc to parse the dtb, get the board compatible string, > + # and then copy the dtb to $2/${board_compatible}.dtb > + compat="`$DTC -I dtb -O compat "$dtb"`" > + > + if [ -e "$2/${compat}.dtb" ]; then > + echo "Install $dtb: $2/${compat}.dtb already exists!" 1>&2 > + exit 1 > + else > + cp "$dtb" "$2/${compat}.dtb" > + fi > +done This only appears to create ${compat}.dtb, and not ${dtb} too. So, it doesn't seem to address part of the cover letter, "In addition, some vendors have done a diligent job naming their devicetree source files appropriately and we don't want to break their setups." Was that deliberate? If so, I guess I need to send some patches to U-Boot.
On Mon, Nov 18, 2013 at 12:09:19PM -0700, Stephen Warren wrote: > On 11/18/2013 11:38 AM, Jason Cooper wrote: > > Unlike other build products in the Linux kernel, the devicetree blobs > > are simply the name of their source file, s/dts/dtb/. There is also no > > 'make *install' mechanism to put them in a standard place with a > > standard naming structure. > > > > Unfortunately, users have begun scripting pulling the needed dtbs from > > within the kernel tree, thus hardcoding the dtbs names. In turn, this > > means any changes to the dts filenames breaks these scripts. > > > > This patch is an attempt to fix this problem. Akin to 'make install', > > this creates a new make target, dtbs_install. The script that gets > > called defers to a vendor or distribution supplied installdtbs binary, > > if found in the system. Otherwise, the default action is to install a > > given dtb into > > > > /lib/modules/${kernel_version}/devicetree/${board_compat}.dtb > > Is co-mingling the DTs in the same (top-level) directory as modules a > good idea. I guess there is an explicit devicetree/ sub-directory so > they're easy to pull out of the source tree, but even so, they're > certainly not modules. I would assume that distros would put them into > e.g. /boot/dtbs/${kernelversion} or something more like that. Would it > make sense for the install target to use a path more like that, or > perhaps at least /dtbs/${kernelversion} to keep it separate from modules? I thought about the proposed /boot/dtbs/${kernelversion} and it didn't sit well with me. /boot is for the files needed to boot the machine, not all the files you *might* need to boot the machine. I'm speaking generally, I understand your situation is different. I tried to follow the analogy of the kernel modules, All of them are installed in /lib/modules/$kernelversion}, then, the initramfs builder selects a few modules needed to find the rootfs, etc and packs just those into the initramfs. > > diff --git a/arch/arm/Makefile b/arch/arm/Makefile > > > +dtbs_install: dtbs This answers below ^^^^ > > + $(CONFIG_SHELL) $(srctree)/$(boot)/installdtbs.sh $(KERNELRELEASE) \ > > + "$(MODLIB)/devicetree" "$(srctree)/$(boot)/dts" > > Architectures besides ARM use device trees. Shouldn't "make > dtbs_install" work for them too? Yes, once the idea is hammered out, I'll add powerpc/mips/etc. > > diff --git a/arch/arm/boot/installdtbs.sh b/arch/arm/boot/installdtbs.sh > > > +for dtb in `find "$3" -name "*.dtb"`; do > > + # we use dtc to parse the dtb, get the board compatible string, > > + # and then copy the dtb to $2/${board_compatible}.dtb > > + compat="`$DTC -I dtb -O compat "$dtb"`" > > + > > + if [ -e "$2/${compat}.dtb" ]; then > > + echo "Install $dtb: $2/${compat}.dtb already exists!" 1>&2 > > + exit 1 > > + else > > + cp "$dtb" "$2/${compat}.dtb" > > + fi > > +done > > This only appears to create ${compat}.dtb, and not ${dtb} too. See my comment, above. > So, it doesn't seem to address part of the cover letter, "In addition, > some vendors have done a diligent job naming their devicetree source > files appropriately and we don't want to break their setups." Was that > deliberate? If so, I guess I need to send some patches to U-Boot. I assume you are still pulling from arch/arm/boot/dts/*.dtb, right? Those build products don't go away with this patch, so you should be fine. Unless I'm mis-understanding your workflow... thx, Jason.
On 11/18/2013 12:19 PM, Jason Cooper wrote: > On Mon, Nov 18, 2013 at 12:09:19PM -0700, Stephen Warren wrote: >> On 11/18/2013 11:38 AM, Jason Cooper wrote: >>> Unlike other build products in the Linux kernel, the devicetree blobs >>> are simply the name of their source file, s/dts/dtb/. There is also no >>> 'make *install' mechanism to put them in a standard place with a >>> standard naming structure. >>> >>> Unfortunately, users have begun scripting pulling the needed dtbs from >>> within the kernel tree, thus hardcoding the dtbs names. In turn, this >>> means any changes to the dts filenames breaks these scripts. >>> >>> This patch is an attempt to fix this problem. Akin to 'make install', >>> this creates a new make target, dtbs_install. The script that gets >>> called defers to a vendor or distribution supplied installdtbs binary, >>> if found in the system. Otherwise, the default action is to install a >>> given dtb into ... >> This only appears to create ${compat}.dtb, and not ${dtb} too. > > See my comment, above. > >> So, it doesn't seem to address part of the cover letter, "In addition, >> some vendors have done a diligent job naming their devicetree source >> files appropriately and we don't want to break their setups." Was that >> deliberate? If so, I guess I need to send some patches to U-Boot. > > I assume you are still pulling from arch/arm/boot/dts/*.dtb, right? > Those build products don't go away with this patch, so you should be > fine. Unless I'm mis-understanding your workflow... Yes, but I thought the whole point of this was that everyone would/could switch to running "make dtbs_install", then pulling out the files from the install tree, instead of any use-case requiring obtaining files directly from the source/build tree?
On Mon, Nov 18, 2013 at 12:23:48PM -0700, Stephen Warren wrote: > On 11/18/2013 12:19 PM, Jason Cooper wrote: > > On Mon, Nov 18, 2013 at 12:09:19PM -0700, Stephen Warren wrote: > >> On 11/18/2013 11:38 AM, Jason Cooper wrote: > >>> Unlike other build products in the Linux kernel, the devicetree blobs > >>> are simply the name of their source file, s/dts/dtb/. There is also no > >>> 'make *install' mechanism to put them in a standard place with a > >>> standard naming structure. > >>> > >>> Unfortunately, users have begun scripting pulling the needed dtbs from > >>> within the kernel tree, thus hardcoding the dtbs names. In turn, this > >>> means any changes to the dts filenames breaks these scripts. > >>> > >>> This patch is an attempt to fix this problem. Akin to 'make install', > >>> this creates a new make target, dtbs_install. The script that gets > >>> called defers to a vendor or distribution supplied installdtbs binary, > >>> if found in the system. Otherwise, the default action is to install a > >>> given dtb into > ... > >> This only appears to create ${compat}.dtb, and not ${dtb} too. > > > > See my comment, above. > > > >> So, it doesn't seem to address part of the cover letter, "In addition, > >> some vendors have done a diligent job naming their devicetree source > >> files appropriately and we don't want to break their setups." Was that > >> deliberate? If so, I guess I need to send some patches to U-Boot. > > > > I assume you are still pulling from arch/arm/boot/dts/*.dtb, right? > > Those build products don't go away with this patch, so you should be > > fine. Unless I'm mis-understanding your workflow... > > Yes, but I thought the whole point of this was that everyone would/could > switch to running "make dtbs_install", then pulling out the files from > the install tree, instead of any use-case requiring obtaining files > directly from the source/build tree? Yes, in the script, it checks for the existence of /sbin/installdtbs. If it's there, it will override the default behavior and execute that script. So yours can be: #!/bin/sh [ -d "/boot/dtbs/$1" ] && rm -rf "/boot/dtbs/$1" mkdir -p "/boot/dtbs/$1" for dtb in `find "$3" -name "*.dtb"`; do cp "$dtb" "/boot/dtbs/$1/${dtb##*/}" done
On 11/18/2013 12:28 PM, Jason Cooper wrote: > On Mon, Nov 18, 2013 at 12:23:48PM -0700, Stephen Warren wrote: >> On 11/18/2013 12:19 PM, Jason Cooper wrote: >>> On Mon, Nov 18, 2013 at 12:09:19PM -0700, Stephen Warren wrote: >>>> On 11/18/2013 11:38 AM, Jason Cooper wrote: >>>>> Unlike other build products in the Linux kernel, the devicetree blobs >>>>> are simply the name of their source file, s/dts/dtb/. There is also no >>>>> 'make *install' mechanism to put them in a standard place with a >>>>> standard naming structure. >>>>> >>>>> Unfortunately, users have begun scripting pulling the needed dtbs from >>>>> within the kernel tree, thus hardcoding the dtbs names. In turn, this >>>>> means any changes to the dts filenames breaks these scripts. >>>>> >>>>> This patch is an attempt to fix this problem. Akin to 'make install', >>>>> this creates a new make target, dtbs_install. The script that gets >>>>> called defers to a vendor or distribution supplied installdtbs binary, >>>>> if found in the system. Otherwise, the default action is to install a >>>>> given dtb into >> ... >>>> This only appears to create ${compat}.dtb, and not ${dtb} too. >>> >>> See my comment, above. >>> >>>> So, it doesn't seem to address part of the cover letter, "In addition, >>>> some vendors have done a diligent job naming their devicetree source >>>> files appropriately and we don't want to break their setups." Was that >>>> deliberate? If so, I guess I need to send some patches to U-Boot. >>> >>> I assume you are still pulling from arch/arm/boot/dts/*.dtb, right? >>> Those build products don't go away with this patch, so you should be >>> fine. Unless I'm mis-understanding your workflow... >> >> Yes, but I thought the whole point of this was that everyone would/could >> switch to running "make dtbs_install", then pulling out the files from >> the install tree, instead of any use-case requiring obtaining files >> directly from the source/build tree? > > Yes, in the script, it checks for the existence of /sbin/installdtbs. > If it's there, it will override the default behavior and execute that > script. But that means everyone (a lot of people, and all distros) has to write, or at least download and install, such a script. Equally, that script would incorrectly affect the naming of non-Tegra DTBs that follow the new convention. The naming of the Tegra DTBs is based more on the fact that they're the Tegra DTBs than the fact that I'm using them.
On Mon, Nov 18, 2013 at 12:38:11PM -0700, Stephen Warren wrote: > On 11/18/2013 12:28 PM, Jason Cooper wrote: > > On Mon, Nov 18, 2013 at 12:23:48PM -0700, Stephen Warren wrote: > >> On 11/18/2013 12:19 PM, Jason Cooper wrote: > >>> On Mon, Nov 18, 2013 at 12:09:19PM -0700, Stephen Warren wrote: > >>>> On 11/18/2013 11:38 AM, Jason Cooper wrote: > >>>>> Unlike other build products in the Linux kernel, the devicetree blobs > >>>>> are simply the name of their source file, s/dts/dtb/. There is also no > >>>>> 'make *install' mechanism to put them in a standard place with a > >>>>> standard naming structure. > >>>>> > >>>>> Unfortunately, users have begun scripting pulling the needed dtbs from > >>>>> within the kernel tree, thus hardcoding the dtbs names. In turn, this > >>>>> means any changes to the dts filenames breaks these scripts. > >>>>> > >>>>> This patch is an attempt to fix this problem. Akin to 'make install', > >>>>> this creates a new make target, dtbs_install. The script that gets > >>>>> called defers to a vendor or distribution supplied installdtbs binary, > >>>>> if found in the system. Otherwise, the default action is to install a > >>>>> given dtb into > >> ... > >>>> This only appears to create ${compat}.dtb, and not ${dtb} too. > >>> > >>> See my comment, above. > >>> > >>>> So, it doesn't seem to address part of the cover letter, "In addition, > >>>> some vendors have done a diligent job naming their devicetree source > >>>> files appropriately and we don't want to break their setups." Was that > >>>> deliberate? If so, I guess I need to send some patches to U-Boot. > >>> > >>> I assume you are still pulling from arch/arm/boot/dts/*.dtb, right? > >>> Those build products don't go away with this patch, so you should be > >>> fine. Unless I'm mis-understanding your workflow... > >> > >> Yes, but I thought the whole point of this was that everyone would/could > >> switch to running "make dtbs_install", then pulling out the files from > >> the install tree, instead of any use-case requiring obtaining files > >> directly from the source/build tree? > > > > Yes, in the script, it checks for the existence of /sbin/installdtbs. > > If it's there, it will override the default behavior and execute that > > script. > > But that means everyone (a lot of people, and all distros) has to write, > or at least download and install, such a script. They would already be modifying their build environment to make use of 'make dtbs_install', and few extra lines of shell script to integrate with existing systems certainly isn't unreasonable? > Equally, that script would incorrectly affect the naming of non-Tegra > DTBs that follow the new convention. The naming of the Tegra DTBs is > based more on the fact that they're the Tegra DTBs than the fact that > I'm using them. Umm, I wrote that script in 2 minutes. I'm sure you could come up with something Tegra-specific (perhaps using fdtget?) that places only the Tegra dtbs in the location that works with Tegra's u-boot. The point is, we're trying to set a reasonable default, _and_ respect the pre-existing workflows. I don't think 15 lines of script is too much to ask. Especially since it would be written to adopt a *new* workflow, not fix a workflow broken by callous kernel maintainers. ;-) thx, Jason.
On 11/18/2013 12:52 PM, Jason Cooper wrote: > On Mon, Nov 18, 2013 at 12:38:11PM -0700, Stephen Warren wrote: >> On 11/18/2013 12:28 PM, Jason Cooper wrote: >>> On Mon, Nov 18, 2013 at 12:23:48PM -0700, Stephen Warren wrote: >>>> On 11/18/2013 12:19 PM, Jason Cooper wrote: >>>>> On Mon, Nov 18, 2013 at 12:09:19PM -0700, Stephen Warren wrote: >>>>>> On 11/18/2013 11:38 AM, Jason Cooper wrote: >>>>>>> Unlike other build products in the Linux kernel, the devicetree blobs >>>>>>> are simply the name of their source file, s/dts/dtb/. There is also no >>>>>>> 'make *install' mechanism to put them in a standard place with a >>>>>>> standard naming structure. >>>>>>> >>>>>>> Unfortunately, users have begun scripting pulling the needed dtbs from >>>>>>> within the kernel tree, thus hardcoding the dtbs names. In turn, this >>>>>>> means any changes to the dts filenames breaks these scripts. >>>>>>> >>>>>>> This patch is an attempt to fix this problem. Akin to 'make install', >>>>>>> this creates a new make target, dtbs_install. The script that gets >>>>>>> called defers to a vendor or distribution supplied installdtbs binary, >>>>>>> if found in the system. Otherwise, the default action is to install a >>>>>>> given dtb into >>>> ... >>>>>> This only appears to create ${compat}.dtb, and not ${dtb} too. >>>>> >>>>> See my comment, above. >>>>> >>>>>> So, it doesn't seem to address part of the cover letter, "In addition, >>>>>> some vendors have done a diligent job naming their devicetree source >>>>>> files appropriately and we don't want to break their setups." Was that >>>>>> deliberate? If so, I guess I need to send some patches to U-Boot. >>>>> >>>>> I assume you are still pulling from arch/arm/boot/dts/*.dtb, right? >>>>> Those build products don't go away with this patch, so you should be >>>>> fine. Unless I'm mis-understanding your workflow... >>>> >>>> Yes, but I thought the whole point of this was that everyone would/could >>>> switch to running "make dtbs_install", then pulling out the files from >>>> the install tree, instead of any use-case requiring obtaining files >>>> directly from the source/build tree? >>> >>> Yes, in the script, it checks for the existence of /sbin/installdtbs. >>> If it's there, it will override the default behavior and execute that >>> script. >> >> But that means everyone (a lot of people, and all distros) has to write, >> or at least download and install, such a script. > > They would already be modifying their build environment to make use of > 'make dtbs_install', and few extra lines of shell script to integrate > with existing systems certainly isn't unreasonable? > >> Equally, that script would incorrectly affect the naming of non-Tegra >> DTBs that follow the new convention. The naming of the Tegra DTBs is >> based more on the fact that they're the Tegra DTBs than the fact that >> I'm using them. > > Umm, I wrote that script in 2 minutes. I'm sure you could come up with > something Tegra-specific (perhaps using fdtget?) that places only the > Tegra dtbs in the location that works with Tegra's u-boot. Again, the point is not that it's hard to write the script. As you demonstrated, it's easy. The problem is that then, everybody has to do something different for Tegra, forever. No matter how small the actual cost of doing it is, it's still non-scalable to require that everyone know about this special case. I'm not convinced the issue would be isolated to Tegra either. The whole point of "make dtbs_install" is that people can run it, and get the results they're expected to use. If this isn't the case, what exactly is the point of any of the renaming that "make dtbs_install" is doing? Certainly, it makes sense to have a "make dtbs_install" target, to isolate DTB consumers from the source locations of the DTBs. However, if "make dtbs_install" is doing anything other than just copying files (i.e. if it does any kind of renaming at all), then it had better do *everything* that's required to solve *all* cases, and not just solve the one case you care about.
On Mon, Nov 18, 2013 at 03:46:32PM -0700, Stephen Warren wrote: > Again, the point is not that it's hard to write the script. As you > demonstrated, it's easy. The problem is that then, everybody has to do > something different for Tegra, forever. No matter how small the actual > cost of doing it is, it's still non-scalable to require that everyone > know about this special case. I'm not convinced the issue would be > isolated to Tegra either. That's why there's the facility to allow an override to the script, just like there's the facility to override the default script when running "make install". The script which the kernel uses is just a template for what kernel developers expect to be the common case. That's not necessarily what most distros do. In fact, most distros today provide their own installation script which gets run automatically when you do "make install" which copies the kernel and then rebuilds an initramfs with everything appropriate for that kernel inside. Even the distro script can be overridden with a script in ~/bin by the user running "make install" (which therefore doesn't even have to be root if installing it into a location accessible by the user.) This is really no different. If the as-supplied-by-the-kernel script doesn't fit your needs, you are free to override it. What this gives us is the facility for distros to _sanely_ hook into the kernel build process to obtain the DT files if they so wish. As for renaming the files from what we have in the kernel, I'd say... why bother. What if we have two DT blobs with the same compatible string (which is not unlikely when you have two configurations for a platform)? I'd recommend keeping the names in the kernel tree when installing. If a distro needs to find the compatible string, then that's a bit of parsing they should do. Otherwise, we lose the ability to tell users "you need this DT blob file called X" because X will depend whether it's been installed or not. And in some cases where getting the wrong DT blob could end up quite literally destroying your hardware (eg, by setting an LDO regulator to bypass mode), it's extremely important that anything here is _simple_ and doesn't involve lots of indirection.
Russell, On Tue, Nov 19, 2013 at 12:28:01PM +0000, Russell King - ARM Linux wrote: > As for renaming the files from what we have in the kernel, I'd say... > why bother. When I first read your email, I agreed, and started down the path of dropping the rename, then... > What if we have two DT blobs with the same compatible string (which is > not unlikely when you have two configurations for a platform)? If a board has two different, especially destructive, configurations, it should have two different compatible strings. Especially since 90% (I'm swagging) of a distro's use of dtbs is to upgrade the dtb on an already installed system. They'll most likely pull the board compatible string from /proc/device-tree/compatible, and then go hunt for the dtb. That's why I prefer to rename the dtbs, it removes the need to parse the dtb (fdtget *isn't* currently built/installed by default in dtc), and it allows distros, for the most common use case, to easily find a matching dtb on an existing system. In the event that an already deployed dtb is found to have an alternate rev (with or without destructive changes), on our side, we add a more specific compatible string and a new dts for the new rev. The distro dtb upgrader detects it's on a 'globalscale,mirabox' and sees that the options are now globalscale,mirabox-rev1.dtb and globalscale,mirabox-rev2.dtb. It throws up a user prompt asking the user to determine which board rev it's on. Or, if safe to do so, it programatically determines which rev it is on and updates the dtb. > I'd recommend keeping the names in the kernel tree when installing. > If a distro needs to find the compatible string, then that's a bit of > parsing they should do. Otherwise, we lose the ability to tell users > "you need this DT blob file called X" because X will depend whether > it's been installed or not. All I'm suggesting doing is extending the same standard we have for compatible strings at the node level to the board level. If we don't change the filename, then we *have* to extend the consistent naming to the dts files as well. I think it's just as easy to say "you need globalscale,mirabox-rev1 now" as anything else. > And in some cases where getting the wrong DT blob could end up quite > literally destroying your hardware (eg, by setting an LDO regulator to > bypass mode), it's extremely important that anything here is _simple_ > and doesn't involve lots of indirection. kept for context, above. thx, Jason.
On Tue, Nov 19, 2013 at 09:23:36AM -0500, Jason Cooper wrote: > All I'm suggesting doing is extending the same standard we have for > compatible strings at the node level to the board level. If we don't > change the filename, then we *have* to extend the consistent naming to > the dts files as well. That, in itself, is not a bad thing to do, and totally solves the problem.
Russell, On Tue, Nov 19, 2013 at 03:02:12PM +0000, Russell King - ARM Linux wrote: > On Tue, Nov 19, 2013 at 09:23:36AM -0500, Jason Cooper wrote: > > All I'm suggesting doing is extending the same standard we have for > > compatible strings at the node level to the board level. If we don't > > change the filename, To be clear, by this I meant dtbs_install changing armada-370-mirabox.dtb to globalscale,mirabox.dtb. > > then we *have* to extend the consistent naming to the dts files as > > well. > > That, in itself, is not a bad thing to do, and totally solves the problem. By "That" are you referring to extending the standard for compatible strings to the board level, or forcing consistent naming on the dts files? thx, Jason.
On Tue, Nov 19, 2013 at 10:20:47AM -0500, Jason Cooper wrote: > By "That" are you referring to extending the standard for compatible > strings to the board level, or forcing consistent naming on the dts > files? A consistent file naming on both the dtb and dts files.
On 11/19/2013 07:23 AM, Jason Cooper wrote: ... > That's why I prefer to rename the dtbs, it removes the need to parse the > dtb (fdtget *isn't* currently built/installed by default in dtc) fdtget is in fact both built and installed by default. At the very least in the latest dtc source, and it's certainly part of at least the Ubuntu packages for older versions too, so I assume it's been the default for a while.
On 11/19/2013 05:28 AM, Russell King - ARM Linux wrote: > On Mon, Nov 18, 2013 at 03:46:32PM -0700, Stephen Warren wrote: >> Again, the point is not that it's hard to write the script. As you >> demonstrated, it's easy. The problem is that then, everybody has to do >> something different for Tegra, forever. No matter how small the actual >> cost of doing it is, it's still non-scalable to require that everyone >> know about this special case. I'm not convinced the issue would be >> isolated to Tegra either. > > That's why there's the facility to allow an override to the script, > just like there's the facility to override the default script when > running "make install". Again, you are completely missing the point about that not scaling at all. But I will go and investigate what it takes to support renaming the DTBs. Everyone (using Tegra) will have to update their bootloader, but perhaps that can be dealt with.
On Tue, Nov 19, 2013 at 11:40:36AM -0700, Stephen Warren wrote: > On 11/19/2013 07:23 AM, Jason Cooper wrote: > ... > > That's why I prefer to rename the dtbs, it removes the need to parse the > > dtb (fdtget *isn't* currently built/installed by default in dtc) > > fdtget is in fact both built and installed by default. At the very least > in the latest dtc source, and it's certainly part of at least the Ubuntu > packages for older versions too, so I assume it's been the default for a > while. Ah. Gentoo must be the weird one. :( thx, Jason.
On Tue, Nov 19, 2013 at 11:42:17AM -0700, Stephen Warren wrote: > On 11/19/2013 05:28 AM, Russell King - ARM Linux wrote: > > On Mon, Nov 18, 2013 at 03:46:32PM -0700, Stephen Warren wrote: > >> Again, the point is not that it's hard to write the script. As you > >> demonstrated, it's easy. The problem is that then, everybody has to do > >> something different for Tegra, forever. No matter how small the actual > >> cost of doing it is, it's still non-scalable to require that everyone > >> know about this special case. I'm not convinced the issue would be > >> isolated to Tegra either. > > > > That's why there's the facility to allow an override to the script, > > just like there's the facility to override the default script when > > running "make install". > > Again, you are completely missing the point about that not scaling at all. No I'm not. What you're looking for is perfection. Perfection is something we can't have here - we can have "good enough". Look, the problem is very simple: we have a bunch of DT descriptions in the kernel, which we build to a blob. We don't want distributions poking around in the kernel source tree to retrieve these blobs, so we provide a way to install them somewhere. The installation should just copy the blobs to a path in the filesystem _or_ allow a distro to hook into the installation process, just like we provide for the installation of the zImage. That is our job done. We should do *nothing* more. If we start doing more than that, we're getting into the realms of distribution policy, and that's something we should not be involved in. This is precisely the reason why we don't get involved with working out what steps are needed to install a kernel zImage onto a target: that is _not_ the kernel's job to work out that you may need to package it up and maybe copy it over to a TFTP server, or maybe copy it onto a SD card, or maybe place it in /boot and do a certain number of other steps. All that the kernel build's job here is to provide the results of the kernel build. It is not about working out which DT blob is right for any particular platform.
On Tue, Nov 19, 2013 at 11:42:17AM -0700, Stephen Warren wrote: > On 11/19/2013 05:28 AM, Russell King - ARM Linux wrote: > > On Mon, Nov 18, 2013 at 03:46:32PM -0700, Stephen Warren wrote: > >> Again, the point is not that it's hard to write the script. As you > >> demonstrated, it's easy. The problem is that then, everybody has to do > >> something different for Tegra, forever. No matter how small the actual > >> cost of doing it is, it's still non-scalable to require that everyone > >> know about this special case. I'm not convinced the issue would be > >> isolated to Tegra either. > > > > That's why there's the facility to allow an override to the script, > > just like there's the facility to override the default script when > > running "make install". > > Again, you are completely missing the point about that not scaling at all. > > But I will go and investigate what it takes to support renaming the > DTBs. Everyone (using Tegra) will have to update their bootloader, but > perhaps that can be dealt with. Setting aside the idea of hard-coding filenames into any bootloader binary, did you catch that the proposed solution would allow you to continue using the dts filenames as they currently are? The find command in my example /sbin/installdtbs script spits out the in-tree dtb filename as it currently stands, and then you do what you want with it in your script. Including, copying it, filename intact, into the location of your choice. How could that possibly require you to upgrade your bootloader? I must admit I'm a bit mystified as to what Tegra is doing that this breaks it so horribly... thx, Jason.
On Tue, Nov 19, 2013 at 03:21:46PM +0000, Russell King - ARM Linux wrote: > On Tue, Nov 19, 2013 at 10:20:47AM -0500, Jason Cooper wrote: > > By "That" are you referring to extending the standard for compatible > > strings to the board level, or forcing consistent naming on the dts > > files? > > A consistent file naming on both the dtb and dts files. It's time to knock the idea of naming the DT blobs after the compatible string on its head as well. That's really not going to work. Just leave the filenames as is, and move the problem into userspace to solve. Why? arch/arm/boot/dts/imx28-cfa10036.dts- compatible = "crystalfontz,cfa10036", "fsl,imx28"; arch/arm/boot/dts/imx28-cfa10037.dts- compatible = "crystalfontz,cfa10037", "crystalfontz,cfa10036", "fsl,imx28"; arch/arm/boot/dts/imx28-cfa10049.dts- compatible = "crystalfontz,cfa10049", "crystalfontz,cfa10036", "fsl,imx28"; arch/arm/boot/dts/imx28-cfa10055.dts- compatible = "crystalfontz,cfa10055", "crystalfontz,cfa10037", "crystalfontz,cfa10036", "fsl,imx28"; arch/arm/boot/dts/imx28-cfa10057.dts- compatible = "crystalfontz,cfa10057", "crystalfontz,cfa10036", "fsl,imx28"; We already have DT blobs where the compatible string(s) for a DT blob conflict with other DT blobs. Using the compatible string as a filename means that you have to choose one of the above to name the DT blob, whereas the above compatible strings allow any of those to be used with "crystalfontz,cfa10036". So, given a platform with "crystalfontz,cfa10036" you may wish to offer to the user to select one of those five. How do you do that if you've named the files after the compatible string (maybe the first compatible string.) See - by doing this, you are building policy into the kernel build system and potentially taking away choice from userspace. Just use the filenames we already have in the kernel tree. Let userspace parse the DT blobs for the compatible strings - it can then provide the user with the choice of all and _also_ show that certain of DT blobs may be compatible with a range of boards as well.
On 11/19/2013 11:52 AM, Russell King - ARM Linux wrote: > On Tue, Nov 19, 2013 at 11:42:17AM -0700, Stephen Warren wrote: >> On 11/19/2013 05:28 AM, Russell King - ARM Linux wrote: >>> On Mon, Nov 18, 2013 at 03:46:32PM -0700, Stephen Warren wrote: >>>> Again, the point is not that it's hard to write the script. As you >>>> demonstrated, it's easy. The problem is that then, everybody has to do >>>> something different for Tegra, forever. No matter how small the actual >>>> cost of doing it is, it's still non-scalable to require that everyone >>>> know about this special case. I'm not convinced the issue would be >>>> isolated to Tegra either. >>> >>> That's why there's the facility to allow an override to the script, >>> just like there's the facility to override the default script when >>> running "make install". >> >> Again, you are completely missing the point about that not scaling at all. > > No I'm not. What you're looking for is perfection. Perfection is something > we can't have here - we can have "good enough". For the issue I care about, it's not hard to have perfection. Why wouldn't I want that. Reading your comments below, I think we're talking about slightly different aspects of the problem though. > Look, the problem is very simple: we have a bunch of DT descriptions in > the kernel, which we build to a blob. We don't want distributions poking > around in the kernel source tree to retrieve these blobs, so we provide > a way to install them somewhere. Sure. I have no problem at all if the kernel implemnts a "make dtbs_install" target. It certainly should. > The installation should just copy the blobs to a path in the filesystem > _or_ allow a distro to hook into the installation process, just like we > provide for the installation of the zImage. That is our job done. We > should do *nothing* more. OK, if you're arguing that there should be absolutely zero renaming at all under any circumstances, I'm fine with that. My argument was that the proposed renaming step (renaming the DTBs based on the top-level compatible) is not appropriate for Tegra, even if it is what some other platforms might want. My issue could be addressed by any of: a) Never renaming anything. I believe this is what you're arguing for. b) Having some kind of in-kernel list of DTs that do get renamed, and those that don't. Tegra DTs would be in the latter don't-rename list. c) Simply renaming any DT source files in the kernel that are perceived to have the wrong name currently, rather than renaming during each "make dtbs_install". Tegra DTs currently have the correct name, so wouldn't be renamed. I'm fine with any of those solutions. (a) or (c) seem simplest to me. > If we start doing more than that, we're getting into the realms of > distribution policy, and that's something we should not be involved in. > > This is precisely the reason why we don't get involved with working out > what steps are needed to install a kernel zImage onto a target: that > is _not_ the kernel's job to work out that you may need to package it > up and maybe copy it over to a TFTP server, or maybe copy it onto a SD > card, or maybe place it in /boot and do a certain number of other > steps. > > All that the kernel build's job here is to provide the results of the > kernel build. It is not about working out which DT blob is right for > any particular platform. For the record, I was never talking about the kernel selecting a particular DTB to install, or a particular DTB for a target. I assume that "make dtbs_install" always installs all DTBs that the kernel built. The discussion in this thread has been about (re-)naming not selecting DTBs. It's up to some out-of-kernel process to pick which (or all) of those to actually use.
On 11/19/2013 11:57 AM, Jason Cooper wrote: > On Tue, Nov 19, 2013 at 11:42:17AM -0700, Stephen Warren wrote: >> On 11/19/2013 05:28 AM, Russell King - ARM Linux wrote: >>> On Mon, Nov 18, 2013 at 03:46:32PM -0700, Stephen Warren wrote: >>>> Again, the point is not that it's hard to write the script. As you >>>> demonstrated, it's easy. The problem is that then, everybody has to do >>>> something different for Tegra, forever. No matter how small the actual >>>> cost of doing it is, it's still non-scalable to require that everyone >>>> know about this special case. I'm not convinced the issue would be >>>> isolated to Tegra either. >>> >>> That's why there's the facility to allow an override to the script, >>> just like there's the facility to override the default script when >>> running "make install". >> >> Again, you are completely missing the point about that not scaling at all. >> >> But I will go and investigate what it takes to support renaming the >> DTBs. Everyone (using Tegra) will have to update their bootloader, but >> perhaps that can be dealt with. > > Setting aside the idea of hard-coding filenames into any bootloader > binary, did you catch that the proposed solution would allow you to > continue using the dts filenames as they currently are? I don't consider requiring people to install a custom /sbin/installdtbs in order to override renaming done by the default in-kernel scripts/installdtbs a solution. Think of a distro that wants to support 5 different SoCs. If they install a custom /sbin/installdtbs that does zero renaming, then they'll get the desired DTB names for Tegra, but perhaps not for other SoCs. If the don't install a custom /sbin/installdtbs that does zero renaming, then they'll perhaps get the desired DTB names for SoCs other than Tegra, but won't for Tegra. Assuming we ever rename DTBs at all, there are a couple ways to solve this: a) Require everyone who runs "make dtbs_install" (and who cares about multiple SoCs, i.e. all distros at least) to install (and perhaps write) their own custom /sbin/installdtbs that only renames some DTBs. b) Just put the correct logic in the in-kernel installdtbs script, so nobody has to do anything; it "just works" out-of-the-box. > The find command in my example /sbin/installdtbs script spits out the > in-tree dtb filename as it currently stands, and then you do what you > want with it in your script. Including, copying it, filename intact, > into the location of your choice. How could that possibly require you > to upgrade your bootloader? A "solution" that requires people to install custom scripts to undo renaming that shouldn't be happening in the first place just isn't workable; there's too much communication and support cost associated with it. So, if the renaming during "make dtbs_install" absolutely has to happen, then I guess I'd just have to live with it, and force everyone to upgrade their bootloader instead. > I must admit I'm a bit mystified as to what Tegra is doing that this > breaks it so horribly... The idea is that a disto boot process runs as follow: * U-Boot runs and initializes. * U-Boot searches the boot disk (or any disk that could be a boot disk) for e.g. /boot/boot.scr, and executes it. * boot.scr loads the kernel, initrd, DTB, sets up the command-line, and boots the kernel. A few points to make here: a) There must be a boot.scr or similar on disk, to provide a way for distros to parameterize the boot process. At the very least they need to configure the kernel command-line, specify whether an initrd is used at all, etc. b) boot.scr is responsible for loading the zImage etc. from disk, rather than the default U-Boot environment doing this. This allows distros complete flexibility re: where to put the zImage etc. on disk. On distro might store a single copy in /boot/zImage, another might use /boot/zImage-${kernelversion}, another might use /boot/${kernelversion}/zImage, etc. By putting the onus on boot.scr to do the loading, the distro has complete flexibility in file naming here. c) There are a number of system-specific parameters required to implement zImage/DTB loading. For example, the SDRAM address that they should be loaded to. Distros shouldn't have to detect which board they're running on and calculate the values of all those parameters. Instead, U-Boot should provide the values to boot.scr, by setting certain standard variables in the environment. For example, the zImage gets loaded to $kernel_addr_r, the DTB to $fdt_addr_r, etc. d) Finally, boot.scr needs to load the DTB. This requires knowing which DTB to load. Again, the distro shouldn't have to detect which board they're running on, and either install the correct DTB to a static filename, or make a decision on the DTB filename to load. Instead, distros should simply install all DTBs generated by the kernel build, and use some run-time information to calculate the DTB filename using a completely HW-agnostic and generic algorithm. To support this, U-Boot can be configured to add certain standard environment variables to the default environment. These define which SoC and board the code is running on. These are ${soc} and ${board}. If you then calculate the DTB filename as ${soc}-${board}.dtb, that should work anywhere. This is why keeping the current in-kernel DTB filenames is important, so they match the assumption that this algorithm works. Note: if the firmware contains an embedded DTB, boot.scr could detect this e.g. by the standard variable $fdt_addr being set, and hence skiip loading a DTB. IIRC, this is how Calxeda boards are set up. So finally, if the kernel "make dtbs_install" starts renaming DTB files, then I need to adjust the boot.scr we're using to use some other algorithm to calculate the DTB filename. Given the filename isn't then purely derived from ${soc} and ${board}, I probably need to set up some additional (or replacement) variables in the standard U-Boot environment that contain the extra fields required to calculate the DTB filename. The existing ${vendor} might work. However, it'd be simplest if I just had U-Boot export something like ${dtb_filename}, which contained the (or a list of) filenames to try. You can take a look at my boot.scr generation tool at: https://github.com/NVIDIA/tegra-uboot-scripts That probably works on the Raspberry Pi too (since I set up the Pi's U-Boot port to work the same way) and likely would require minimal changes to work on at least some TI and Calxeda boards. The idea is that eventually, and board that wants to support a generic distro booting on it would export the standard variables I mentioned above, have a default U-Boot environment that simply searched for and executed boot.scr, and distros would use a generic boot.scr as generated by the tool I linked to above.
On Tue, Nov 19, 2013 at 12:27:01PM -0700, Stephen Warren wrote: > On 11/19/2013 11:52 AM, Russell King - ARM Linux wrote: > > The installation should just copy the blobs to a path in the filesystem > > _or_ allow a distro to hook into the installation process, just like we > > provide for the installation of the zImage. That is our job done. We > > should do *nothing* more. > > OK, if you're arguing that there should be absolutely zero renaming at > all under any circumstances, I'm fine with that. That's what I'm arguing for. Consider what happens when we need to identify a specific DT blob to be used for a board: "For the Zynteknology Amazingboard, you need to use the DT file imx6dl-amazingboard.dtb". What happens if we have an installation system which renames the files? "For the Zynteknology Amazingboard, you need to use the DT file imx6dl-amazingboard.dtb, but this may also be named zynteknology,amazingboard.dtb on installation". Now, the user has to work out which of two filenames instead of just one, and it becomes a lot harder - it's not difficult for _us_ developers, but from the user point of view, it is, because there is now two things they have to deal with, and a greater chance of making an error. Now, as for using the compatible strings, if you look at the email I just sent previously, I've shown why naming DT blobs by their compatible string is a stupid thing to do. I'll go further: it's an idiotic thing which can't work. We already have DT blobs which contain a _range_ of compatible strings. Does that mean we always choose the first compatible string? If you're using that to match up a DT blob, you've just failed because what if your platform needs to match against the second compatible string? Use symlinks for all compatible strings? Well, what if you have two DT blobs which contain the same compatible string? Yes, we do have lots which fall into this case already. Hence, naming files by compatible string _can't_ ever work sanely. It assumes that compatible strings are unique. They plainly aren't. So, I think we keep the existing naming scheme and just copy the files as is to the installation location. Anything more than that is a "distro" choice - if a distro wants to offer the user a list of DT images to use, the distro needs a tool which parses the compatible strings, works out which of the files match the target, and present the user with that choice - because we already have the case where there is no 1:1 relationship between files and board.
On Tue, Nov 19, 2013 at 07:22:46PM +0000, Russell King - ARM Linux wrote: > On Tue, Nov 19, 2013 at 03:21:46PM +0000, Russell King - ARM Linux wrote: > > On Tue, Nov 19, 2013 at 10:20:47AM -0500, Jason Cooper wrote: > > > By "That" are you referring to extending the standard for compatible > > > strings to the board level, or forcing consistent naming on the dts > > > files? > > > > A consistent file naming on both the dtb and dts files. > > It's time to knock the idea of naming the DT blobs after the compatible > string on its head as well. That's really not going to work. Just > leave the filenames as is, and move the problem into userspace to solve. Ack, I was working my way towards that anyway. > See - by doing this, you are building policy into the kernel build > system and potentially taking away choice from userspace. And this is my reason for dropping the idea. Thanks for clarifying it for me. I'll submit a non-RFC version that just copies the files without the name change. thx, Jason.
On Tue, Nov 19, 2013 at 12:53:07PM -0700, Stephen Warren wrote: > On 11/19/2013 11:57 AM, Jason Cooper wrote: > > On Tue, Nov 19, 2013 at 11:42:17AM -0700, Stephen Warren wrote: > >> On 11/19/2013 05:28 AM, Russell King - ARM Linux wrote: > >>> On Mon, Nov 18, 2013 at 03:46:32PM -0700, Stephen Warren wrote: > >>>> Again, the point is not that it's hard to write the script. As you > >>>> demonstrated, it's easy. The problem is that then, everybody has to do > >>>> something different for Tegra, forever. No matter how small the actual > >>>> cost of doing it is, it's still non-scalable to require that everyone > >>>> know about this special case. I'm not convinced the issue would be > >>>> isolated to Tegra either. > >>> > >>> That's why there's the facility to allow an override to the script, > >>> just like there's the facility to override the default script when > >>> running "make install". > >> > >> Again, you are completely missing the point about that not scaling at all. > >> > >> But I will go and investigate what it takes to support renaming the > >> DTBs. Everyone (using Tegra) will have to update their bootloader, but > >> perhaps that can be dealt with. > > > > Setting aside the idea of hard-coding filenames into any bootloader > > binary, did you catch that the proposed solution would allow you to > > continue using the dts filenames as they currently are? > > I don't consider requiring people to install a custom /sbin/installdtbs > in order to override renaming done by the default in-kernel > scripts/installdtbs a solution. Russell's point about not creating policy for userspace is on the mark. *I* think the most-specific compatible string should be unique per board, and thus would be a sane naming scheme. But that is exactly as Russell said making policy for userspace. If you said as much, I missed it. Sorry about that. Now, off on a tangent... > > I must admit I'm a bit mystified as to what Tegra is doing that this > > breaks it so horribly... > > The idea is that a disto boot process runs as follow: > > * U-Boot runs and initializes. > > * U-Boot searches the boot disk (or any disk that could be a boot disk) > for e.g. /boot/boot.scr, and executes it. > > * boot.scr loads the kernel, initrd, DTB, sets up the command-line, and > boots the kernel. > > A few points to make here: > > a) There must be a boot.scr or similar on disk, to provide a way for > distros to parameterize the boot process. At the very least they need to > configure the kernel command-line, specify whether an initrd is used at > all, etc. > > b) boot.scr is responsible for loading the zImage etc. from disk, rather > than the default U-Boot environment doing this. This allows distros > complete flexibility re: where to put the zImage etc. on disk. On distro > might store a single copy in /boot/zImage, another might use > /boot/zImage-${kernelversion}, another might use > /boot/${kernelversion}/zImage, etc. By putting the onus on boot.scr to > do the loading, the distro has complete flexibility in file naming here. > > c) There are a number of system-specific parameters required to > implement zImage/DTB loading. For example, the SDRAM address that they > should be loaded to. Distros shouldn't have to detect which board > they're running on and calculate the values of all those parameters. > Instead, U-Boot should provide the values to boot.scr, by setting > certain standard variables in the environment. > > For example, the zImage gets loaded to $kernel_addr_r, the DTB to > $fdt_addr_r, etc. > > d) Finally, boot.scr needs to load the DTB. This requires knowing which > DTB to load. Again, the distro shouldn't have to detect which board > they're running on, and either install the correct DTB to a static > filename, or make a decision on the DTB filename to load. Instead, > distros should simply install all DTBs generated by the kernel build, > and use some run-time information to calculate the DTB filename using a > completely HW-agnostic and generic algorithm. > > To support this, U-Boot can be configured to add certain standard > environment variables to the default environment. These define which SoC > and board the code is running on. These are ${soc} and ${board}. If you > then calculate the DTB filename as ${soc}-${board}.dtb, that should work > anywhere. This is why keeping the current in-kernel DTB filenames is > important, so they match the assumption that this algorithm works. > > Note: if the firmware contains an embedded DTB, boot.scr could detect > this e.g. by the standard variable $fdt_addr being set, and hence skiip > loading a DTB. IIRC, this is how Calxeda boards are set up. Ok, so the ${soc}-${board} is a work-around until all bootloaders are providing a dtb, or until all bootloaders are providing a stable dtb? I ask because in tinkering with pxa-impedance-matcher [1], I've setup selecting from many dtbs at boot (linked in, no fs support) by specifying the desired board compatible string on the kernel commandline (patches still a wip). The impedance-matcher then scans through the provided dtbs looking for a match. It's a hack, and only useful until bootloaders are providing dtbs. If the dtb is upgradable, then it goes away earlier. If the dtb isn't, then it'll be around longer. It's goal is to keep silly code out of the kernel (non-standard atag-to-fdt translation, other bootloader workarounds). Perhaps it'll be used to translate ACPI into DT for a bit... :) > So finally, if the kernel "make dtbs_install" starts renaming DTB files, > then I need to adjust the boot.scr we're using to use some other > algorithm to calculate the DTB filename. Given the filename isn't then > purely derived from ${soc} and ${board}, I probably need to set up some > additional (or replacement) variables in the standard U-Boot environment > that contain the extra fields required to calculate the DTB filename. > The existing ${vendor} might work. However, it'd be simplest if I just > had U-Boot export something like ${dtb_filename}, which contained the > (or a list of) filenames to try. > > You can take a look at my boot.scr generation tool at: > https://github.com/NVIDIA/tegra-uboot-scripts > > That probably works on the Raspberry Pi too (since I set up the Pi's > U-Boot port to work the same way) and likely would require minimal > changes to work on at least some TI and Calxeda boards. > > The idea is that eventually, and board that wants to support a generic > distro booting on it would export the standard variables I mentioned > above, have a default U-Boot environment that simply searched for and > executed boot.scr, and distros would use a generic boot.scr as generated > by the tool I linked to above. Thanks for the detailed explanation. I'll post a non-RFC version in the next few days. thx, Jason. [1] https://github.com/zonque/pxa-impedance-matcher.git
On 11/19/2013 01:39 PM, Jason Cooper wrote: > On Tue, Nov 19, 2013 at 12:53:07PM -0700, Stephen Warren wrote: ... >> d) Finally, boot.scr needs to load the DTB. This requires knowing which >> DTB to load. Again, the distro shouldn't have to detect which board >> they're running on, and either install the correct DTB to a static >> filename, or make a decision on the DTB filename to load. Instead, >> distros should simply install all DTBs generated by the kernel build, >> and use some run-time information to calculate the DTB filename using a >> completely HW-agnostic and generic algorithm. >> >> To support this, U-Boot can be configured to add certain standard >> environment variables to the default environment. These define which SoC >> and board the code is running on. These are ${soc} and ${board}. If you >> then calculate the DTB filename as ${soc}-${board}.dtb, that should work >> anywhere. This is why keeping the current in-kernel DTB filenames is >> important, so they match the assumption that this algorithm works. >> >> Note: if the firmware contains an embedded DTB, boot.scr could detect >> this e.g. by the standard variable $fdt_addr being set, and hence skiip >> loading a DTB. IIRC, this is how Calxeda boards are set up. > > Ok, so the ${soc}-${board} is a work-around until all bootloaders are > providing a dtb, or until all bootloaders are providing a stable dtb? I suppose so yes. However, I'm not really convinced we'll see that many products with a DT embedded into them that's stable, complete[1], and based on upstream bindings, in the particularly near future. I think distros will likely need to deal with DTs-in-filesystems for a while yet. But anyway, either way should work out just fine. [1] For example, we aren't that far off some reasonably stable basic bindings for Tegra now . However, there are still devices we haven't added to DT at all, so while the DT ABI stability issue is fading, the need-to-upgrade-your-DTB case still exists to get new features, and upgrading the DTB via the filesystem is a lot easier and more convenient than doing so via firmware updates or firmware flashing commands.
On Tue, 19 Nov 2013 19:22:46 +0000, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Tue, Nov 19, 2013 at 03:21:46PM +0000, Russell King - ARM Linux wrote: > > On Tue, Nov 19, 2013 at 10:20:47AM -0500, Jason Cooper wrote: > > > By "That" are you referring to extending the standard for compatible > > > strings to the board level, or forcing consistent naming on the dts > > > files? > > > > A consistent file naming on both the dtb and dts files. > > It's time to knock the idea of naming the DT blobs after the compatible > string on its head as well. That's really not going to work. Just > leave the filenames as is, and move the problem into userspace to solve. > [...] > > Just use the filenames we already have in the kernel tree. Let userspace > parse the DT blobs for the compatible strings - it can then provide the > user with the choice of all and _also_ show that certain of DT blobs > may be compatible with a range of boards as well. Yes, the consistent filename format is a non-problem. The files that exist already have names. Nothing else is going to use them. Done. It makes total sense to require that new files follow some sane naming convention, but it is a mistake to project that back and say all existing files must also be renamed. Sure it may look untidy, particularly to those of us with obsessive-compulsive tendencies, but it isn't actually a problem. g.
On Tue, 19 Nov 2013 14:06:05 -0700, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 11/19/2013 01:39 PM, Jason Cooper wrote: > > On Tue, Nov 19, 2013 at 12:53:07PM -0700, Stephen Warren wrote: > ... > >> d) Finally, boot.scr needs to load the DTB. This requires knowing which > >> DTB to load. Again, the distro shouldn't have to detect which board > >> they're running on, and either install the correct DTB to a static > >> filename, or make a decision on the DTB filename to load. Instead, > >> distros should simply install all DTBs generated by the kernel build, > >> and use some run-time information to calculate the DTB filename using a > >> completely HW-agnostic and generic algorithm. > >> > >> To support this, U-Boot can be configured to add certain standard > >> environment variables to the default environment. These define which SoC > >> and board the code is running on. These are ${soc} and ${board}. If you > >> then calculate the DTB filename as ${soc}-${board}.dtb, that should work > >> anywhere. This is why keeping the current in-kernel DTB filenames is > >> important, so they match the assumption that this algorithm works. > >> > >> Note: if the firmware contains an embedded DTB, boot.scr could detect > >> this e.g. by the standard variable $fdt_addr being set, and hence skiip > >> loading a DTB. IIRC, this is how Calxeda boards are set up. > > > > Ok, so the ${soc}-${board} is a work-around until all bootloaders are > > providing a dtb, or until all bootloaders are providing a stable dtb? > > I suppose so yes. > > However, I'm not really convinced we'll see that many products with a DT > embedded into them that's stable, complete[1], and based on upstream > bindings, in the particularly near future. I think distros will likely > need to deal with DTs-in-filesystems for a while yet. But anyway, either > way should work out just fine. > > [1] For example, we aren't that far off some reasonably stable basic > bindings for Tegra now . However, there are still devices we haven't > added to DT at all, so while the DT ABI stability issue is fading, the > need-to-upgrade-your-DTB case still exists to get new features, and > upgrading the DTB via the filesystem is a lot easier and more convenient > than doing so via firmware updates or firmware flashing commands. I'm completely fine with requiring a DT upgrade to enable new features, so long as keeping the same DT doesn't cause regress when running on mainline. Keeping that support enabled in firmware is important. g.
On Wed, Nov 20, 2013 at 01:10:04PM +0000, Grant Likely wrote: > On Tue, 19 Nov 2013 19:22:46 +0000, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > On Tue, Nov 19, 2013 at 03:21:46PM +0000, Russell King - ARM Linux wrote: > > > On Tue, Nov 19, 2013 at 10:20:47AM -0500, Jason Cooper wrote: > > > > By "That" are you referring to extending the standard for compatible > > > > strings to the board level, or forcing consistent naming on the dts > > > > files? > > > > > > A consistent file naming on both the dtb and dts files. > > > > It's time to knock the idea of naming the DT blobs after the compatible > > string on its head as well. That's really not going to work. Just > > leave the filenames as is, and move the problem into userspace to solve. > > > [...] > > > > Just use the filenames we already have in the kernel tree. Let userspace > > parse the DT blobs for the compatible strings - it can then provide the > > user with the choice of all and _also_ show that certain of DT blobs > > may be compatible with a range of boards as well. > > Yes, the consistent filename format is a non-problem. The files that > exist already have names. Nothing else is going to use them. Done. Grant, you have the wrong end of the stick. This discussion is about what happens when files from $(objtree)/arch/arm/boot/dts get installed into /lib/modules/$(kernelversion)/... and their renaming from the filenames we already have in arch/arm/boot/dts to a filename generated from the compatible string.
On Wed, Nov 20, 2013 at 01:10:04PM +0000, Grant Likely wrote: > Sure it may look untidy, particularly to those of us with > obsessive-compulsive tendencies, but it isn't actually a problem. Hey! I resemble that remark! :) thx, Jason.
On Wed, 20 Nov 2013 13:56:18 +0000, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Wed, Nov 20, 2013 at 01:10:04PM +0000, Grant Likely wrote: > > On Tue, 19 Nov 2013 19:22:46 +0000, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > > On Tue, Nov 19, 2013 at 03:21:46PM +0000, Russell King - ARM Linux wrote: > > > > On Tue, Nov 19, 2013 at 10:20:47AM -0500, Jason Cooper wrote: > > > > > By "That" are you referring to extending the standard for compatible > > > > > strings to the board level, or forcing consistent naming on the dts > > > > > files? > > > > > > > > A consistent file naming on both the dtb and dts files. > > > > > > It's time to knock the idea of naming the DT blobs after the compatible > > > string on its head as well. That's really not going to work. Just > > > leave the filenames as is, and move the problem into userspace to solve. > > > > > [...] > > > > > > Just use the filenames we already have in the kernel tree. Let userspace > > > parse the DT blobs for the compatible strings - it can then provide the > > > user with the choice of all and _also_ show that certain of DT blobs > > > may be compatible with a range of boards as well. > > > > Yes, the consistent filename format is a non-problem. The files that > > exist already have names. Nothing else is going to use them. Done. > > Grant, you have the wrong end of the stick. This discussion is about > what happens when files from > > $(objtree)/arch/arm/boot/dts > > get installed into > > /lib/modules/$(kernelversion)/... > > and their renaming from the filenames we already have in arch/arm/boot/dts > to a filename generated from the compatible string. Umm, I don't follow where we're having a disconnect. That is what I'm talking about. I'm arguing that renaming the files with installing them is a bad idea. Are you not arguing the same here? g.
diff --git a/Makefile b/Makefile index 920ad07180c9..29d609e972d6 100644 --- a/Makefile +++ b/Makefile @@ -339,6 +339,7 @@ OBJDUMP = $(CROSS_COMPILE)objdump AWK = awk GENKSYMS = scripts/genksyms/genksyms INSTALLKERNEL := installkernel +INSTALLDTBS := installdtbs DEPMOD = /sbin/depmod PERL = perl CHECK = sparse @@ -391,7 +392,7 @@ KERNELVERSION = $(VERSION)$(if $(PATCHLEVEL),.$(PATCHLEVEL)$(if $(SUBLEVEL),.$(S export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION export ARCH SRCARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC export CPP AR NM STRIP OBJCOPY OBJDUMP -export MAKE AWK GENKSYMS INSTALLKERNEL PERL UTS_MACHINE +export MAKE AWK GENKSYMS INSTALLKERNEL INSTALLDTBS PERL UTS_MACHINE export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS diff --git a/arch/arm/Makefile b/arch/arm/Makefile index c99b1086d83d..c16ebb1e74b0 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -314,6 +314,10 @@ PHONY += dtbs dtbs: scripts $(Q)$(MAKE) $(build)=$(boot)/dts MACHINE=$(MACHINE) dtbs +dtbs_install: dtbs + $(CONFIG_SHELL) $(srctree)/$(boot)/installdtbs.sh $(KERNELRELEASE) \ + "$(MODLIB)/devicetree" "$(srctree)/$(boot)/dts" + # We use MRPROPER_FILES and CLEAN_FILES now archclean: $(Q)$(MAKE) $(clean)=$(boot) diff --git a/arch/arm/boot/installdtbs.sh b/arch/arm/boot/installdtbs.sh new file mode 100644 index 000000000000..c46c109363d2 --- /dev/null +++ b/arch/arm/boot/installdtbs.sh @@ -0,0 +1,44 @@ +#!/bin/sh +# +# This file is subject to the terms and conditions of the GNU General Public +# License. See the file "COPYING" in the main directory of this archive +# for more details. +# +# Copyright (C) 1995 by Linus Torvalds +# +# Adapted from code in arch/i386/boot/Makefile by H. Peter Anvin +# +# Further adapted from arch/x86/boot/install.sh by Jason Cooper +# +# "make dtbs_install" script +# +# Arguments: +# $1 - kernel version +# $2 - default install path (blank if root directory) +# $3 - directory containing dtbs +# + +# User may have a custom install script + +if [ -x ~/bin/${INSTALLDTBS} ]; then exec ~/bin/${INSTALLDTBS} "$@"; fi +if [ -x /sbin/${INSTALLDTBS} ]; then exec /sbin/${INSTALLDTBS} "$@"; fi + +DTC=./scripts/dtc/dtc + +# Default install +[ -d "$2" ] && rm -rf "$2" + +mkdir -p "$2" + +for dtb in `find "$3" -name "*.dtb"`; do + # we use dtc to parse the dtb, get the board compatible string, + # and then copy the dtb to $2/${board_compatible}.dtb + compat="`$DTC -I dtb -O compat "$dtb"`" + + if [ -e "$2/${compat}.dtb" ]; then + echo "Install $dtb: $2/${compat}.dtb already exists!" 1>&2 + exit 1 + else + cp "$dtb" "$2/${compat}.dtb" + fi +done
Unlike other build products in the Linux kernel, the devicetree blobs are simply the name of their source file, s/dts/dtb/. There is also no 'make *install' mechanism to put them in a standard place with a standard naming structure. Unfortunately, users have begun scripting pulling the needed dtbs from within the kernel tree, thus hardcoding the dtbs names. In turn, this means any changes to the dts filenames breaks these scripts. This patch is an attempt to fix this problem. Akin to 'make install', this creates a new make target, dtbs_install. The script that gets called defers to a vendor or distribution supplied installdtbs binary, if found in the system. Otherwise, the default action is to install a given dtb into /lib/modules/${kernel_version}/devicetree/${board_compat}.dtb This solves several problems for us. The board compatible string should be unique, this enforces/highlights that. While devicetrees are stablilizing, the fact is, devicetrees aren't (yet) independent of kernel version. This install target facilitates keeping track of that. Once the devicetree files are moved to their own repository, this install target can be removed as users will be modifying their scripts anyway. Signed-off-by: Jason Cooper <jason@lakedaemon.net> --- changes since v1: - added this patch Makefile | 3 ++- arch/arm/Makefile | 4 ++++ arch/arm/boot/installdtbs.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 arch/arm/boot/installdtbs.sh