[PATCH 0/8] kernel: mtdsplit_uimage: use device tree properties for non-standard uimage parsing

[Adrian Schmutzler]

Hi,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces at lists.openwrt.org]
> On Behalf Of Bjørn Mork
> Sent: Mittwoch, 25. November 2020 12:45
> To: openwrt-devel at lists.openwrt.org
> Cc: Bjørn Mork <bjorn at mork.no>
> Subject: [PATCH 0/8] kernel: mtdsplit_uimage: use device tree properties for
> non-standard uimage parsing
> 
> This ended up a bit more invasive than I imagined, but here goes..
> 
> I wanted to add a couple of new rtl83xx devices.  They both use standard
> uimages, but with device specific magic values.  Like most devices in this
> target, it seems.
> 
> The easy route would be to just add another magic to one of the existing
> parsers in mtdsplit_uimage, or maybe even a new parser grouping devices
> "belonging together" on some scale.  But this seems so unnecessary
> complicated to add something which is basically a device specific
> configuration and nothing more.  Not to mention that it does have some side
> effects.  New parsers make the code grow.
> Adding new magic numbers to existing parsers make them less fool proof,
> allowing any of the magic values to match while there is only one correct
> value most of the time.
> 
> So I thought I'd rather put these magic values in the device tree, for the cases
> where the parser is otherwise identical to the generic "denx,uimage" parser.
> I believe this makes sense.
> 
> No so sure about the rest of the series.  But the snow ball rolled, and I
> thought "why not do the same for the other device specific parameters?".
> So I added device tree properties for those as well.
> And ended up removing all the parsers except for the generic one.

I like the general idea, it feels elegant.

However, I wonder whether - strictly - these parameter belong into device tree, i.e. "describe the hardware".
(this is meant as an open question, I do not want to express any rejection with that)

And probably we would need to involve upstream (devicetree-spec) for the naming of the properties?
Or is this irrelevant as mtdsplit_uimage is just "our stuff" anyway?

Best

Adrian

> 
> Still not sure that was a good idea.  In particular because it's hard to test this
> without having the hardware, which I don't have.  And console is required if
> something breaks.
> 
> Anyway, here it is.  Please be gentle :-)
> 
> 
> 
> Bjørn
> ---
> 
> Bjørn Mork (8):
>   kernel: mtdsplit_uimage: read extralen from device tree
>   kernel: mtdsplit_uimage: replace "fonfxc" and "sge" parsers
>   kernel: mtdsplit_uimage: add "ih-magic" device-tree property
>   kernel: mtdsplit_uimage: replace "openwrt,okli" parser
>   kernel: mtdsplit_uimage: replace "allnet,uimage" parser
>   kernel: mtdsplit_uimage: add "ih-type" device-tree property
>   kernel: mtdsplit_uimage: replace "netgear,uimage" parser
>   kernel: mtdsplit_uimage: replace "edimax,uimage" parser
> 
>  .../ath79/dts/ar7161_netgear_wndr3700-v2.dts  |   4 +-
>  .../ath79/dts/ar7161_netgear_wndr3700.dts     |   4 +-
>  .../ath79/dts/ar7161_netgear_wndr3800.dts     |   4 +-
>  .../ath79/dts/ar7161_netgear_wndr3800ch.dts   |   4 +-
>  .../ath79/dts/ar7161_netgear_wndrmac-v1.dts   |   4 +-
>  .../ath79/dts/ar7161_netgear_wndrmac-v2.dts   |   4 +-
>  .../ath79/dts/ar7240_engenius_enh202-v1.dts   |   3 +-
>  .../ath79/dts/ar7240_netgear_wnr1000-v2.dts   |   4 +-
>  .../ath79/dts/ar7240_netgear_wnr612-v2.dtsi   |   4 +-
>  .../ath79/dts/ar7241_netgear_wnr2000-v3.dts   |   4 +-
>  .../ath79/dts/ar7241_netgear_wnr2200-16m.dts  |   4 +-
>  .../ath79/dts/ar7241_netgear_wnr2200-8m.dts   |   4 +-
>  .../dts/ar9341_engenius_ens202ext-v1.dts      |   3 +-
>  .../linux/ath79/dts/ar9341_pisen_wmb001n.dts  |   3 +-
>  .../linux/ath79/dts/ar9344_netgear_wndr.dtsi  |   4 +-
>  .../drivers/mtd/mtdsplit/mtdsplit_uimage.c    | 418 +++---------------
>  .../dts/mt7620a_edimax_br-6478ac-v2.dts       |   4 +-
>  .../ramips/dts/mt7620a_edimax_ew-7478apc.dts  |   4 +-
>  .../linux/ramips/dts/mt7620a_fon_fon2601.dts  |   3 +-
>  .../ramips/dts/mt7620n_sunvalley_filehub.dtsi |   3 +-
>  .../ramips/dts/mt7621_dlink_dir-8xx-a1.dtsi   |   3 +-
>  .../ramips/dts/mt7621_dlink_dir-xx60-a1.dtsi  |   6 +-
>  .../linux/ramips/dts/mt7621_edimax_re23s.dts  |   4 +-
>  .../ramips/dts/rt3050_edimax_3g-6200n.dts     |   4 +-
>  .../ramips/dts/rt3050_edimax_3g-6200nl.dts    |   4 +-
>  .../ramips/dts/rt3662_edimax_br-6475nd.dts    |   4 +-
>  .../dts/rtl8382_allnet_all-sg8208m.dts        |   3 +-
>  27 files changed, 140 insertions(+), 377 deletions(-)
> 
> --
> 2.20.1
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel at lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
-------------- 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/20201125/65336640/attachment.sig>