[PATCH] build: put DT "compatible" value as "board_name" in profiles.json
mail at adrianschmutzler.de
mail at adrianschmutzler.de
Mon Jul 13 19:07:55 EDT 2020
Hi,
> -----Original Message-----
> From: Paul Spooren [mailto:mail at aparcar.org]
> Sent: Montag, 13. Juli 2020 11:46
> To: mail at adrianschmutzler.de; 'Rafał Miłecki' <zajec5 at gmail.com>;
> openwrt-devel at lists.openwrt.org
> Cc: 'Rafał Miłecki' <rafal at milecki.pl>; 'Petr Štetiar' <ynezz at true.cz>; 'Moritz
> Warning' <moritzwarning at web.de>
> Subject: Re: [PATCH] build: put DT "compatible" value as "board_name" in
> profiles.json
>
>
> On 10.07.20 04:23, mail at adrianschmutzler.de wrote:
> >> -----Original Message-----
> >> From: openwrt-devel [mailto:openwrt-devel-
> bounces at lists.openwrt.org]
> >> On Behalf Of Rafal Milecki
> >> Sent: Mittwoch, 8. Juli 2020 17:10
> >> To: openwrt-devel at lists.openwrt.org
> >> Cc: Rafał Miłecki <rafal at milecki.pl>; Adrian Schmutzler
> >> <freifunk at adrianschmutzler.de>; Petr Štetiar <ynezz at true.cz>; Moritz
> >> Warning <moritzwarning at web.de>; Paul Spooren <mail at aparcar.org>
> >> Subject: [PATCH] build: put DT "compatible" value as "board_name" in
> >> profiles.json
> >>
> >> From: Rafał Miłecki <rafal at milecki.pl>
> >>
> >> The purpose of "board_name" in JSON is matchine OpenWrt running
> >> device with JSON profile entry. Right now it gets filled for devices using
> DT.
> >> Other targets will require custom solutions or just speciyfing that
> >> value manually.
> >>
> >> Signed-off-by: Rafał Miłecki <rafal at milecki.pl>
> >> ---
> >> include/image.mk | 3 +++
> >> scripts/json_add_image_info.py | 19 +++++++++++++++++++
> >> 2 files changed, 22 insertions(+)
> >>
> >> diff --git a/include/image.mk b/include/image.mk index
> >> 15f4fe9d3b..b33c1032f8 100644
> >> --- a/include/image.mk
> >> +++ b/include/image.mk
> >> @@ -532,10 +532,13 @@ define Device/Build/image
> >> @mkdir -p $$(shell dirname $$@)
> >> DEVICE_ID="$(DEVICE_NAME)" \
> >> BIN_DIR="$(BIN_DIR)" \
> >> + LINUX_DIR="$(LINUX_DIR)" \
> >> + KDIR="$(KDIR)" \
> >> IMAGE_NAME="$(IMAGE_NAME)" \
> >> IMAGE_TYPE=$(word 1,$(subst ., ,$(2))) \
> >> IMAGE_PREFIX="$(IMAGE_PREFIX)" \
> >> DEVICE_TITLE="$(DEVICE_TITLE)" \
> >> + DEVICE_DTS="$(DEVICE_DTS)" \
> >> TARGET="$(BOARD)" \
> >> SUBTARGET="$(if $(SUBTARGET),$(SUBTARGET),generic)" \
> >> VERSION_NUMBER="$(VERSION_NUMBER)" \ diff --git
> >> a/scripts/json_add_image_info.py b/scripts/json_add_image_info.py
> >> index b4d2dd8d71..5df4bf2a2a 100755
> >> --- a/scripts/json_add_image_info.py
> >> +++ b/scripts/json_add_image_info.py
> >> @@ -5,6 +5,8 @@ from pathlib import Path from sys import argv
> >> import hashlib import json
> >> +import re
> >> +import subprocess
> >>
> >> if len(argv) != 2:
> >> print("ERROR: JSON info script requires output arg") @@ -22,6
> >> +24,20 @@ if not image_file.is_file():
> >> def get_titles():
> >> return [{"title": getenv("DEVICE_TITLE")}]
> >>
> >> +def get_board_name():
> >> + device_dts = getenv("DEVICE_DTS")
> >> + if device_dts is not None:
> >> + dtc = getenv("LINUX_DIR") + "/scripts/dtc/dtc"
> >> + dtb_file = getenv("KDIR") + "/image-" + device_dts + ".dtb"
> >> + dts = subprocess.run([dtc, "-q", "-I", "dtb", "-O", "dts",
> >> +"-o", "-",
> >> dtb_file], capture_output=True, text=True)
> >> + if dts.returncode != 0:
> >> + return None
> >> + match = re.search("compatible = \"([^\"]*)", dts.stdout)
> >> + if match is None:
> >> + return None
> >> + return match[1]
> >> + return None
> >> +
> >>
> >> device_id = getenv("DEVICE_ID")
> >> image_hash = hashlib.sha256(image_file.read_bytes()).hexdigest()
> >> @@ -46,5 +62,8 @@ image_info = {
> >> }
> >> },
> >> }
> >> +board_name = get_board_name()
> >> +if board_name is not None:
> >> + image_info["profiles"][device_id]["board_name"] = board_name
> > Hi,
> >
> > coming back to the initial subject of your patch:
> >
> > As stated earlier in the discussion (I think), we have
> "SUPPORTED_DEVICES" on many devices that will essentially give us the
> board name.
> > So, one could say, the first entry of SUPPORTED_DEVICES just happens to
> be the board name (as that one is designed to match the current compatible
> ...).
> >
> > So, instead of establishing another mechanism to retrieve the board name
> in this patch (which might have several special cases etc.), I'd vote for just
> providing SUPPORTED_DEVICES for the remaining devices, even if it's not
> required for the upgrade mechanism itself.
> I'm also in favor for using the first entry of SUPPORTED_DEVICES.
> > This would allow us to benefit from what's set up already, and would allow
> to make target-specific solutions for the remaining cases (in some cases it
> might be just a one-liner in Device/Default).
>
> I guess the only way which doesn't require per image root filesystems is to
> use a file like Adrian mentioned before[0] which ideally uses the very same
> schema as DT does (vendor_model).
>
> [0]:
> https://github.com/openwrt/openwrt/blob/openwrt-
> 19.07/target/linux/ramips/base-files/lib/ramips.sh
Just to state that explicitly: That's a lot of work, and not something you quickly put together in one hour.
>
> Adrian/Rafal do you have an overview which targets would be affected?
> Would it be okay to update them within the dev branch like previously done
> for some Linksys mvebu/cortexa9 devies?
Shouldn't be too many, and I think I updated most of the DT targets to have consistent compatible/board name already.
But one would have to check each of them (=target) and compile a list.
>
> Lastly I'd like to know your opinion on device codenames. As written some
> Linksys devices were renamed recently from codenames to model names
> (e.g. linksys,rango to linksys,wrt3200acm). This makes very much sense as
> long as only one device exists with a particular codename, however becomes
> messy if multiple models use the same board (e.g. boards with indoor or
> outdoor cases). Imre, presumably working together with Linksys and having
> some insight, responded with the following concern when I initially send the
> renaming patches to upstream:
>
> > The Linksys Viper has been sold as E4200v2 and EA4500. The Linksys Focus
> as EA6100 and EA5800. The LeMans is the EA6300 and the EA6200. The Macan
> is both EA7500 and EA7400 - on the other hand, the EA7500v2 and the
> EA7400v2 are the Savannah.
> So for e.g. the device Linksys E4200v2 / EA4500 (kirkwood) with codename
> Viper I see no sane way to reflect both models within the build system.
It's a matter of taste after all. However, using code names means that _everybody_ needs to do research to find the proper device. In contrast, if we just "arbitrarily" decided for one of the model names, 50 % of the user could just take the image and only the other half would have to learn a new name for their device. So, I'm still inclined to using model names and then just decide for one of them if necessary (or even add ALTx_ stuff for make menuconfig). (And if we had a very unpleasant situation where names are really misleading, we could still build the device twice.)
>
> Renaming the current profile called `linksys_viper` to
> `linksys_e4200v2_and_ea4500` to have "user friendly" image names doesn't
No, please not.
> seem to cut it neither. I'm therefore wondering if we should stick in such
> cases to the codename and improve our documentation and user interfaces
> for such cases. Developers are capable to keep track of such codenames
> (yes, not super intuitive) and so are end users. It's maybe not a matter of less
> confusing filenames (else we would remove targets in filenames[1], right?)
> but of better documentation for the end user.
>
> The very same works for the LineageOS project where downloaded firmware
> images always contains the codename only.
I only used it once for my old Samsung Galaxy S3, and back then I found it under this name.
Best
Adrian
>
> I might be entirely wrong here, some month ago I promoted the exact
> opposite approach by sending patches to the Kernel.
>
> [1]:
> https://patchwork.ozlabs.org/project/openwrt/patch/20200614093330.1751
> 6-1-mail at aparcar.org/
>
> [2]: https://lists.infradead.org/pipermail/openwrt-adm/2020-
> June/001589.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: openpgp-digital-signature.asc
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.openwrt.org/pipermail/openwrt-devel/attachments/20200714/1bf6db94/attachment.sig>
More information about the openwrt-devel
mailing list