Hi, > -----Original Message----- > From: Paul Spooren [mailto:m...@aparcar.org] > Sent: Montag, 13. Juli 2020 11:46 > To: m...@adrianschmutzler.de; 'Rafał Miłecki' <zaj...@gmail.com>; > openwrt-devel@lists.openwrt.org > Cc: 'Rafał Miłecki' <ra...@milecki.pl>; 'Petr Štetiar' <yn...@true.cz>; > 'Moritz > Warning' <moritzwarn...@web.de> > Subject: Re: [PATCH] build: put DT "compatible" value as "board_name" in > profiles.json > > > On 10.07.20 04:23, m...@adrianschmutzler.de wrote: > >> -----Original Message----- > >> From: openwrt-devel [mailto:openwrt-devel- > boun...@lists.openwrt.org] > >> On Behalf Of Rafal Milecki > >> Sent: Mittwoch, 8. Juli 2020 17:10 > >> To: openwrt-devel@lists.openwrt.org > >> Cc: Rafał Miłecki <ra...@milecki.pl>; Adrian Schmutzler > >> <freif...@adrianschmutzler.de>; Petr Štetiar <yn...@true.cz>; Moritz > >> Warning <moritzwarn...@web.de>; Paul Spooren <m...@aparcar.org> > >> Subject: [PATCH] build: put DT "compatible" value as "board_name" in > >> profiles.json > >> > >> From: Rafał Miłecki <ra...@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 <ra...@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-m...@aparcar.org/ > > [2]: https://lists.infradead.org/pipermail/openwrt-adm/2020- > June/001589.html
openpgp-digital-signature.asc
Description: PGP signature
_______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
