Hi Mathias,
On 01.12.2017 16:12, Mathias Kresin wrote:
Hey Piotr ,
thanks a lot for thinking about my proposal.
Sorry for a very late reply.
2017-12-01 14:30 GMT+01:00 Piotr Dymacz <pep...@gmail.com>:
On 29.11.2017 09:33, Mathias Kresin wrote:
1. fix the compatible strings in the DTS files
I think we should start at the same time upstreaming vendor prefixes to [1],
to avoid possible incompatibility/inconsistency later.
I really doubt that upstream is going to accept vendor prefixes if
they aren't used anywhere. I would prefer to use what already exists,
add our own prefixes where required and upstream them at the time the
dts is send upstream.
I don't know but have a look at "opalkelly" [1] (recent example).
The assumption about the underscore character was my suggestion but it seems
that it's not (fully) valid. At least, after researching this more, I was
able to find only one reference in kernel patchwork: [2].
I suppose it's only a general convention rather than a documented
requirement (I'm might be wrong here, anyone? Maybe we should just ask
upstream about this) as there are already some examples in upstream which
contain underscores inside compatible strings: [3], [4].
Assuming above and the fact that <manufacture> and <model> parts used in
compatible string, based on dt specification: [5], might contain any
printable characters (which might not be filename and Makefile safe), we
shouldn't make any assumptions about that.
I share your understanding of the devicetree spec.
So far I have only seen a handful of distortions in compat strings,
like underlines. Never noticed any (special) charter beside [a-z],
[0-9] and [-_].
I was able to find also examples of: '+' in [2], '.' in [3] and '/' in
[4]. What's more, we have '+', '@' and '/' in our tree: [5], [6], [7].
Due to the sync of runtime boardname and the boardname used in the image
filename, it should be possible to guess the used image filename more
reliably as requested.
I would like to propose here slightly extended version of compatible string
-> boardname part in image filename (and our building code) conversion
function:
1. Convert all letters to lowercase.
2. Replace characters different than [a-z], [0-9] and comma with dash.
3. Replace comma with underscore.
3. is already done. I would prefer to add code for 1. and 2. at the
time it is required.
Agree.
Just be aware of above findings.
Beside of that, ACK to the filename specs.
I think that we could even include above function in userspace and expose
the value somewhere, maybe in /tmp/sysinfo?
Same as above. Fine with me, but I prefer to add such code at the time
it is required.
So far, all dts compat strings I've touched only consists of [a-z],
[0-9] and [-], which allows an easy conversion of dts compat string to
boardname.
Agree here as well.
Any feedback on this approach (and help in migrating exists boards of
course) is highly welcome.
I still have here some concerns/thoughts:
1. I don't know how to deal in above approach with region variant images but
I'm sure we should _somehow_ separate region code from boardname and other
parts of the image filename. IMHO, dash character won't work here.
It would be illusionary to assume that the compat string to image
filename matching will work in all cases. I'm fine if it works in most
cases. For edge cases we can use some kind of mapping file.
Maybe that mapping file isn't a bad idea. Could be useful also for some
external projects already mentioned before.
2. We have some boards versions like "v2.1" which with my above approach
would end up as "v2-1". Maybe we should keep dot as a valid character in
version part?
Yeah why not. As the devicetree spec doesn't prohibit any chars, it
should be fine. Is anyone aware that such a compatible string was
rejected upstream?
There are some compatible strings with dots in upstream so I would say
it's allowed or at least it was at some point before.
3. Some of devices come in different RAM/flash configurations and in case
where common image can't support all of them, we include information about
RAM/flash variant sometimes only in image name and sometimes in both the
image name and boardname/compatible string. A common and defined approach
would be required for this as well.
Shouldn't different devicetree source files be used in that case? I
mean, they need a different image for a reason. Maybe because the
memory and/or the partition node in the dts is different.
Yes, I just wanted to point out that most of the manufacturers don't
care about properly naming different variants of their boards/devices
and thus we have to deal with that.
Anyway, we have other examples for the mentioned issue in the lantiq
tree: dgn3500/dgn3500b and wbmrA/wbmrB. The only difference between
the boards is the supported ADSL Annex and therefore the bundled adsl
firmware.
For the dgn3500 two dts with (only) different compat strings and two
images exists. For the wbmr the same devicetree source file is used
for two images. An easy fix would be to bundle both adsl firmware
versions.
Another example: TP-Link TL-MR6400 v1 and v2. The basic hardware is
exactly the same, only the bundled miniPCIe 4G modem is different and
requires different set of packages.
For now I tend to leave it as it is. Of course, it doesn't improve the
situation for the mentioned boards, but it doesn't make it worse
either. Otherwise I would be busy with fixing all the corner cases
instead of improving the situation for the (majority) of the boards
where it is already easily possible. Yeah, call me lazy ;-).
You are the last person I would call lazy ;)
Thanks for your hard work on this!
[1] https://patchwork.ozlabs.org/patch/824516/
[2]
http://elixir.free-electrons.com/linux/v4.15-rc3/source/arch/arm/boot/dts/ste-hrefv60plus.dtsi#L18
[3]
http://elixir.free-electrons.com/linux/v4.15-rc3/source/arch/arm/boot/dts/xenvm-4.2.dts#L14
[4]
http://elixir.free-electrons.com/linux/v4.15-rc3/source/arch/arm/boot/dts/imx6dl-hummingboard.dts#L49
[5]
https://github.com/lede-project/source/blob/master/target/linux/brcm63xx/dts/cpva502plus.dts#L9
[6]
https://github.com/lede-project/source/blob/master/target/linux/brcm63xx/dts/fast2704v2.dts#L9
[7]
https://github.com/lede-project/source/blob/master/target/linux/brcm63xx/dts/dva-g3810bn_tl.dts#L9
--
Cheers,
Piotr
_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
