linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Removal of qcom,board-id and qcom,msm-id
@ 2022-05-19 10:44 Krzysztof Kozlowski
  2022-05-19 11:39 ` Dmitry Baryshkov
  2022-05-22 19:51 ` Konrad Dybcio
  0 siblings, 2 replies; 25+ messages in thread
From: Krzysztof Kozlowski @ 2022-05-19 10:44 UTC (permalink / raw)
  To: Bjorn Andersson, Rob Herring, Stephen Boyd, Arnd Bergmann,
	Andy Gross, Olof Johansson
  Cc: devicetree, linux-arm-msm, Linux Kernel Mailing List

Hi,

There was an old effort of removal of qcom,board-id and qcom,msm-id
properties from Qualcomm SoC-based boards like [1].

First approach was to document them, which (obviously) was not well
received [2] [3] [4].

The solution from Stephen was to encode these in the board compatible,
so bootloader can extract that information. That seemed to receive
positive comments, at least from Rob. [5]

It was 2015... ~7 years later we are still things doing the same way,
still with undocumented properties: qcom,board-id and qcom,msm-id.


I would like to revive that topic, but before I start doing something
pointless - any guidance on last patch from Stephen [5]? Was it ok? Some
early NAKs?


[1]
https://elixir.bootlin.com/linux/v5.18-rc7/source/arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dts#L14

[2] https://lore.kernel.org/all/7229476.C4So9noUlf@wuerfel/
[3]
https://lore.kernel.org/all/1450371534-10923-20-git-send-email-mtitinger+renesas@baylibre.com/
[4] https://lore.kernel.org/all/20151119153640.GC893@linaro.org/
[5]
https://lore.kernel.org/all/1448062280-15406-1-git-send-email-sboyd@codeaurora.org/

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-19 10:44 Removal of qcom,board-id and qcom,msm-id Krzysztof Kozlowski
@ 2022-05-19 11:39 ` Dmitry Baryshkov
  2022-05-19 11:53   ` Krzysztof Kozlowski
  2022-05-22 19:51 ` Konrad Dybcio
  1 sibling, 1 reply; 25+ messages in thread
From: Dmitry Baryshkov @ 2022-05-19 11:39 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Bjorn Andersson, Rob Herring, Stephen Boyd,
	Arnd Bergmann, Andy Gross, Olof Johansson
  Cc: devicetree, linux-arm-msm, Linux Kernel Mailing List

On 19/05/2022 13:44, Krzysztof Kozlowski wrote:
> Hi,
> 
> There was an old effort of removal of qcom,board-id and qcom,msm-id
> properties from Qualcomm SoC-based boards like [1].
> 
> First approach was to document them, which (obviously) was not well
> received [2] [3] [4].
> 
> The solution from Stephen was to encode these in the board compatible,
> so bootloader can extract that information. That seemed to receive
> positive comments, at least from Rob. [5]
> 
> It was 2015... ~7 years later we are still things doing the same way,
> still with undocumented properties: qcom,board-id and qcom,msm-id.
> 
> 
> I would like to revive that topic, but before I start doing something
> pointless - any guidance on last patch from Stephen [5]? Was it ok? Some
> early NAKs?

I do not quite fancy the idea of using extra tools to process dtb files. 
At this moment it is possible to concatenate several kernel-generated 
dtb files together. AOSP developers use this to have an image that boots 
on both RB3 and RB5 boards.

I think that changing compat strings only makes sense if Qualcomm would 
use such compat strings in future. Otherwise we end up in a position 
where we have custom bootloaders for the RB3/RB5/etc, but the majority 
of the board requires extra processing steps.

So, I think, we should drop the unspecified usid aliases, document the 
board-id/msm-id/pmic-id properties and stick with them. They might be 
ugly, but they are expected/processed by the majority of devices present 
in the wild.

> [1]
> https://elixir.bootlin.com/linux/v5.18-rc7/source/arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dts#L14
> 
> [2] https://lore.kernel.org/all/7229476.C4So9noUlf@wuerfel/
> [3]
> https://lore.kernel.org/all/1450371534-10923-20-git-send-email-mtitinger+renesas@baylibre.com/
> [4] https://lore.kernel.org/all/20151119153640.GC893@linaro.org/
> [5]
> https://lore.kernel.org/all/1448062280-15406-1-git-send-email-sboyd@codeaurora.org/
> 
> Best regards,
> Krzysztof


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-19 11:39 ` Dmitry Baryshkov
@ 2022-05-19 11:53   ` Krzysztof Kozlowski
  2022-05-19 12:46     ` Dmitry Baryshkov
  0 siblings, 1 reply; 25+ messages in thread
From: Krzysztof Kozlowski @ 2022-05-19 11:53 UTC (permalink / raw)
  To: Dmitry Baryshkov, Bjorn Andersson, Rob Herring, Stephen Boyd,
	Arnd Bergmann, Andy Gross, Olof Johansson
  Cc: devicetree, linux-arm-msm, Linux Kernel Mailing List

On 19/05/2022 13:39, Dmitry Baryshkov wrote:
> On 19/05/2022 13:44, Krzysztof Kozlowski wrote:
>> Hi,
>>
>> There was an old effort of removal of qcom,board-id and qcom,msm-id
>> properties from Qualcomm SoC-based boards like [1].
>>
>> First approach was to document them, which (obviously) was not well
>> received [2] [3] [4].
>>
>> The solution from Stephen was to encode these in the board compatible,
>> so bootloader can extract that information. That seemed to receive
>> positive comments, at least from Rob. [5]
>>
>> It was 2015... ~7 years later we are still things doing the same way,
>> still with undocumented properties: qcom,board-id and qcom,msm-id.
>>
>>
>> I would like to revive that topic, but before I start doing something
>> pointless - any guidance on last patch from Stephen [5]? Was it ok? Some
>> early NAKs?
> 
> I do not quite fancy the idea of using extra tools to process dtb files. 
> At this moment it is possible to concatenate several kernel-generated 
> dtb files together. AOSP developers use this to have an image that boots 
> on both RB3 and RB5 boards.
> 
> I think that changing compat strings only makes sense if Qualcomm would 
> use such compat strings in future. Otherwise we end up in a position 
> where we have custom bootloaders for the RB3/RB5/etc, but the majority 
> of the board requires extra processing steps.

This was discussed in [2] [3] and [4] (previous links) and did not pass.

Do you have any new arguments for above objections from Arnd, Olof and
Rob? I don't think patch will get accepted if previous concerns during
review are not addressed...

> 
> So, I think, we should drop the unspecified usid aliases, document the 
> board-id/msm-id/pmic-id properties and stick with them. 

The existing properties need anyway documenting, probably as deprecated
so the schema can pass, because we cannot fix the bootloaders easly.

> They might be 
> ugly, but they are expected/processed by the majority of devices present 
> in the wild.

Any change in DTS affects only future devices, so not in the wild...

> 
>> [1]
>> https://elixir.bootlin.com/linux/v5.18-rc7/source/arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dts#L14
>>
>> [2] https://lore.kernel.org/all/7229476.C4So9noUlf@wuerfel/
>> [3]
>> https://lore.kernel.org/all/1450371534-10923-20-git-send-email-mtitinger+renesas@baylibre.com/
>> [4] https://lore.kernel.org/all/20151119153640.GC893@linaro.org/
>> [5]
>> https://lore.kernel.org/all/1448062280-15406-1-git-send-email-sboyd@codeaurora.org/
>>
>> Best regards,
>> Krzysztof
> 
> 


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-19 11:53   ` Krzysztof Kozlowski
@ 2022-05-19 12:46     ` Dmitry Baryshkov
       [not found]       ` <20220519221227.B66D3C385AA@smtp.kernel.org>
  0 siblings, 1 reply; 25+ messages in thread
From: Dmitry Baryshkov @ 2022-05-19 12:46 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Bjorn Andersson, Rob Herring, Stephen Boyd,
	Arnd Bergmann, Andy Gross, Olof Johansson
  Cc: devicetree, linux-arm-msm, Linux Kernel Mailing List

On 19/05/2022 14:53, Krzysztof Kozlowski wrote:
> On 19/05/2022 13:39, Dmitry Baryshkov wrote:
>> On 19/05/2022 13:44, Krzysztof Kozlowski wrote:
>>> Hi,
>>>
>>> There was an old effort of removal of qcom,board-id and qcom,msm-id
>>> properties from Qualcomm SoC-based boards like [1].
>>>
>>> First approach was to document them, which (obviously) was not well
>>> received [2] [3] [4].
>>>
>>> The solution from Stephen was to encode these in the board compatible,
>>> so bootloader can extract that information. That seemed to receive
>>> positive comments, at least from Rob. [5]
>>>
>>> It was 2015... ~7 years later we are still things doing the same way,
>>> still with undocumented properties: qcom,board-id and qcom,msm-id.
>>>
>>>
>>> I would like to revive that topic, but before I start doing something
>>> pointless - any guidance on last patch from Stephen [5]? Was it ok? Some
>>> early NAKs?
>>
>> I do not quite fancy the idea of using extra tools to process dtb files.
>> At this moment it is possible to concatenate several kernel-generated
>> dtb files together. AOSP developers use this to have an image that boots
>> on both RB3 and RB5 boards.
>>
>> I think that changing compat strings only makes sense if Qualcomm would
>> use such compat strings in future. Otherwise we end up in a position
>> where we have custom bootloaders for the RB3/RB5/etc, but the majority
>> of the board requires extra processing steps.
> 
> This was discussed in [2] [3] and [4] (previous links) and did not pass.
> 
> Do you have any new arguments for above objections from Arnd, Olof and
> Rob? I don't think patch will get accepted if previous concerns during
> review are not addressed...

I'm not sure if the patches to the dtbTool have landed or not.
Anyway, as I said, I don't think post-processing the dtb is good way to 
go. It makes extremely hard to check that the dtb, used by the kernel or 
being a part of the boot.img, corresponds to this or that compiled dtb.

> 
>>
>> So, I think, we should drop the unspecified usid aliases, document the
>> board-id/msm-id/pmic-id properties and stick with them.
> 
> The existing properties need anyway documenting, probably as deprecated
> so the schema can pass, because we cannot fix the bootloaders easly.
> 
>> They might be
>> ugly, but they are expected/processed by the majority of devices present
>> in the wild.
> 
> Any change in DTS affects only future devices, so not in the wild...

It affects existing devices that have deployed bootloaders. So far we 
could workaround thus by enforcing a single dtb attached to the kernel 
image. However this doesn't play well for the distro (or AOSP) kernels, 
where we'd like to have multiple dtb image attached.

> 
>>
>>> [1]
>>> https://elixir.bootlin.com/linux/v5.18-rc7/source/arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dts#L14
>>>
>>> [2] https://lore.kernel.org/all/7229476.C4So9noUlf@wuerfel/
>>> [3]
>>> https://lore.kernel.org/all/1450371534-10923-20-git-send-email-mtitinger+renesas@baylibre.com/
>>> [4] https://lore.kernel.org/all/20151119153640.GC893@linaro.org/
>>> [5]
>>> https://lore.kernel.org/all/1448062280-15406-1-git-send-email-sboyd@codeaurora.org/


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
       [not found]       ` <20220519221227.B66D3C385AA@smtp.kernel.org>
@ 2022-05-20  1:39         ` Dmitry Baryshkov
  2022-05-20  7:00           ` Krzysztof Kozlowski
  2022-05-22 10:26           ` Krzysztof Kozlowski
  0 siblings, 2 replies; 25+ messages in thread
From: Dmitry Baryshkov @ 2022-05-20  1:39 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Andy Gross, Arnd Bergmann, Bjorn Andersson, Krzysztof Kozlowski,
	Olof Johansson, Rob Herring, devicetree, linux-arm-msm,
	Linux Kernel Mailing List

On Fri, 20 May 2022 at 01:12, Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Dmitry Baryshkov (2022-05-19 05:46:53)
> > On 19/05/2022 14:53, Krzysztof Kozlowski wrote:
> > > On 19/05/2022 13:39, Dmitry Baryshkov wrote:
> > >> On 19/05/2022 13:44, Krzysztof Kozlowski wrote:
> > >>> Hi,
> > >>>
> > >>> There was an old effort of removal of qcom,board-id and qcom,msm-id
> > >>> properties from Qualcomm SoC-based boards like [1].
> > >>>
> > >>> First approach was to document them, which (obviously) was not well
> > >>> received [2] [3] [4].
> > >>>
> > >>> The solution from Stephen was to encode these in the board compatible,
> > >>> so bootloader can extract that information. That seemed to receive
> > >>> positive comments, at least from Rob. [5]
> > >>>
> > >>> It was 2015... ~7 years later we are still things doing the same way,
> > >>> still with undocumented properties: qcom,board-id and qcom,msm-id.
> > >>>
> > >>>
> > >>> I would like to revive that topic, but before I start doing something
> > >>> pointless - any guidance on last patch from Stephen [5]? Was it ok? Some
> > >>> early NAKs?
> > >>
> > >> I do not quite fancy the idea of using extra tools to process dtb files.
> > >> At this moment it is possible to concatenate several kernel-generated
> > >> dtb files together. AOSP developers use this to have an image that boots
> > >> on both RB3 and RB5 boards.
> > >>
> > >> I think that changing compat strings only makes sense if Qualcomm would
> > >> use such compat strings in future. Otherwise we end up in a position
> > >> where we have custom bootloaders for the RB3/RB5/etc, but the majority
> > >> of the board requires extra processing steps.
> > >
> > > This was discussed in [2] [3] and [4] (previous links) and did not pass.
> > >
> > > Do you have any new arguments for above objections from Arnd, Olof and
> > > Rob? I don't think patch will get accepted if previous concerns during
> > > review are not addressed...
> >
> > I'm not sure if the patches to the dtbTool have landed or not.
> > Anyway, as I said, I don't think post-processing the dtb is good way to
> > go. It makes extremely hard to check that the dtb, used by the kernel or
> > being a part of the boot.img, corresponds to this or that compiled dtb.
>
> One option would be to work it into upstream DTC :P Then the properties
> could be injected during the compilation stage. Note that DTC already
> injects properties like linux,phandle so I'm not really following how
> dtbTool mucking with the DTB is any different.

See, after running make dtbs you get a set of artifacts, Image.gz and
set of dtbs (yes, extended with phandle/linux,phandle).
Then one generates a boot.img. Now without the dtbTool step you can
dissect the boot.img and verify that dtbs match what you've
generated/provided/etc.
If one employs dtbTool, there no simple way to match dtbs from
boot.img with dtbs from kernel build process. This might be required
e.g. to check what the user has been testing, etc.

> Can you elaborate on what
> is extremely hard? Would working dtbTool into the kernel build process
> alleviate any issues?

From my point of view, yet, it will. So will e.g. using any
preprocessor magic to generate such properties during the dts->dtb
process.

>
> I vaguely recall that the properties had to be extracted during the
> boot.img creation process to create a table of contents header. But
> after some time the bootloader started scanning the DTBs directly for
> the vendor properties and thus the header was deprecated/removed. If the
> bootloader is doing the scanning then I'm not sure what is preventing
> the properties from being documented and allowed. I think the main
> rejection was that the properties were added purely to be extracted
> during post processing and placed into the table of contents header,
> i.e. they weren't actually used by the kernel or the bootloader. If they
> are now used by the bootloader it sounds OK to me if they're kept
> around.

Yes, as far as I understand, they are used by the bootloader directly.

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-20  1:39         ` Dmitry Baryshkov
@ 2022-05-20  7:00           ` Krzysztof Kozlowski
  2022-05-22 10:26           ` Krzysztof Kozlowski
  1 sibling, 0 replies; 25+ messages in thread
From: Krzysztof Kozlowski @ 2022-05-20  7:00 UTC (permalink / raw)
  To: Dmitry Baryshkov, Stephen Boyd
  Cc: Andy Gross, Arnd Bergmann, Bjorn Andersson, Olof Johansson,
	Rob Herring, devicetree, linux-arm-msm,
	Linux Kernel Mailing List

On 20/05/2022 03:39, Dmitry Baryshkov wrote:
>> I vaguely recall that the properties had to be extracted during the
>> boot.img creation process to create a table of contents header. But
>> after some time the bootloader started scanning the DTBs directly for
>> the vendor properties and thus the header was deprecated/removed. If the
>> bootloader is doing the scanning then I'm not sure what is preventing
>> the properties from being documented and allowed. I think the main
>> rejection was that the properties were added purely to be extracted
>> during post processing and placed into the table of contents header,
>> i.e. they weren't actually used by the kernel or the bootloader. If they
>> are now used by the bootloader it sounds OK to me if they're kept
>> around.
> 
> Yes, as far as I understand, they are used by the bootloader directly.

This solves only one part of concern form previous discussions - having
entries not used by anything. Kernel still don't use them but some
vendor bootloader (not U-boot) does.

The second problem with these is still not solved - these duplicate what
is already described by compatible. Basically, they do not add any new
hardware description, because board or SoC revision should be encoded
into the compatible. Imagine now adding such "vendor,device-id"
properties to every device node in DTS!

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-20  1:39         ` Dmitry Baryshkov
  2022-05-20  7:00           ` Krzysztof Kozlowski
@ 2022-05-22 10:26           ` Krzysztof Kozlowski
  2022-05-22 10:50             ` Dmitry Baryshkov
  1 sibling, 1 reply; 25+ messages in thread
From: Krzysztof Kozlowski @ 2022-05-22 10:26 UTC (permalink / raw)
  To: Dmitry Baryshkov, Stephen Boyd
  Cc: Andy Gross, Arnd Bergmann, Bjorn Andersson, Olof Johansson,
	Rob Herring, devicetree, linux-arm-msm,
	Linux Kernel Mailing List

On 20/05/2022 03:39, Dmitry Baryshkov wrote:
> 
>>
>> I vaguely recall that the properties had to be extracted during the
>> boot.img creation process to create a table of contents header. But
>> after some time the bootloader started scanning the DTBs directly for
>> the vendor properties and thus the header was deprecated/removed. If the
>> bootloader is doing the scanning then I'm not sure what is preventing
>> the properties from being documented and allowed. I think the main
>> rejection was that the properties were added purely to be extracted
>> during post processing and placed into the table of contents header,
>> i.e. they weren't actually used by the kernel or the bootloader. If they
>> are now used by the bootloader it sounds OK to me if they're kept
>> around.
> 
> Yes, as far as I understand, they are used by the bootloader directly.
> 

I entirely missed one part - Stephen's patches from 2015 were actually
applied and since 2015 we expect all boards to follow convention:

compatible =
"qcom,<SoC>[-<soc_version>][-<foundry_id>]-<board>[/<subtype>][-<board_version>]"

The patchset was accepted, although in the thread I do not see "Applied"
message.

Stephen,
can you or anyone else confirm that the dtbTool Qualcomm uses (and/or
bootloader) are adjusted as well to these new compatibles?

If yes, we can simply remove board-id and msm-id properties from new
boards, because 7 years was enough to switch to these new tools...

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-22 10:26           ` Krzysztof Kozlowski
@ 2022-05-22 10:50             ` Dmitry Baryshkov
  0 siblings, 0 replies; 25+ messages in thread
From: Dmitry Baryshkov @ 2022-05-22 10:50 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Amit Pundir
  Cc: Stephen Boyd, Andy Gross, Arnd Bergmann, Bjorn Andersson,
	Olof Johansson, Rob Herring, devicetree, linux-arm-msm,
	Linux Kernel Mailing List

Adding Amit to the CC list.

On Sun, 22 May 2022 at 13:27, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 20/05/2022 03:39, Dmitry Baryshkov wrote:
> >
> >>
> >> I vaguely recall that the properties had to be extracted during the
> >> boot.img creation process to create a table of contents header. But
> >> after some time the bootloader started scanning the DTBs directly for
> >> the vendor properties and thus the header was deprecated/removed. If the
> >> bootloader is doing the scanning then I'm not sure what is preventing
> >> the properties from being documented and allowed. I think the main
> >> rejection was that the properties were added purely to be extracted
> >> during post processing and placed into the table of contents header,
> >> i.e. they weren't actually used by the kernel or the bootloader. If they
> >> are now used by the bootloader it sounds OK to me if they're kept
> >> around.
> >
> > Yes, as far as I understand, they are used by the bootloader directly.
> >
>
> I entirely missed one part - Stephen's patches from 2015 were actually
> applied and since 2015 we expect all boards to follow convention:
>
> compatible =
> "qcom,<SoC>[-<soc_version>][-<foundry_id>]-<board>[/<subtype>][-<board_version>]"
>
> The patchset was accepted, although in the thread I do not see "Applied"
> message.
>
> Stephen,
> can you or anyone else confirm that the dtbTool Qualcomm uses (and/or
> bootloader) are adjusted as well to these new compatibles?
>
> If yes, we can simply remove board-id and msm-id properties from new
> boards, because 7 years was enough to switch to these new tools...

Amit, can you please comment on the AOSP image build process and the
possibility to drop the board-id/msm-id from the dts files in favour
of using the dtbTool.


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-19 10:44 Removal of qcom,board-id and qcom,msm-id Krzysztof Kozlowski
  2022-05-19 11:39 ` Dmitry Baryshkov
@ 2022-05-22 19:51 ` Konrad Dybcio
  2022-05-23  5:41   ` Amit Pundir
                     ` (2 more replies)
  1 sibling, 3 replies; 25+ messages in thread
From: Konrad Dybcio @ 2022-05-22 19:51 UTC (permalink / raw)
  To: krzysztof.kozlowski
  Cc: agross, arnd, bjorn.andersson, devicetree, linux-arm-msm,
	linux-kernel, olof, robh, sboyd, Konrad Dybcio

Hi,

removing these properties will not bring almost any benefit (other than making
some checks happy any saving some <200 LoC) and will make the lives of almost
all people doing independent development for linux-on-msm harder. There are
almost unironically like 3 people outside Linaro and QUIC who have
non-vendor-fused development boards AND the sources to rebuild the
bootloader on their own. Making it harder to boot is only going to
discourage people from developing on these devices, which is already not
that pleasant, especially with newer platforms where you have to fight with
the oh-so-bright ideas of Android boot chain..

This only concerns devices released before sm8350, as the new ones will not
even boot with these properties present (or at least SONY Sagami, but I
doubt it's an isolated case), so other than completing support for older
devices, it won't be an issue going forward, anyway. But there are give
or take 50 locked down devices in mainline right now, and many more waiting
to be upstreamed in various downstream close-to-mainline trees that should
not be disregarded just because Qualcomm is far from the best at making
their BSP software stack clean.

One solution is to chainload another, (n+1)-stage bootloader, but this is
not ideal, as:

1) the stock bootloader can boot Linux just fine on most devices (except
for single exceptions, where beloved OEMs didn't implement arm64 booting or
something)

2) the boot chain on MSM is already 3- or 4- stage and adding to that will
only create an unnecessary mess

3) the job of kernel people is not to break userspace. If the
device can not even exit bootloader after a kernel upgrade, it's a big
failure.

If you *really really really* want these either gone or documented, we can
for example use them in the SOCID driver, read the values from DTB and
compare against what SMEM has to say and for example print a warning when
there are inconsistencies or use it as a fallback when it fails for any
reason, such as using a newer SoC on an older kernel, without updates
for SOCID read (which are sometimes necessary, which was the case for 8450
recently, iirc).

My stance is to just leave them as is, as moving them anywhere, or removing
them at all will cause unnecessary mess and waste time that could have been
spent on more glaring issues..

Konrad

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-22 19:51 ` Konrad Dybcio
@ 2022-05-23  5:41   ` Amit Pundir
  2022-05-23  7:21   ` Krzysztof Kozlowski
  2022-06-22  8:21   ` Dmitry Baryshkov
  2 siblings, 0 replies; 25+ messages in thread
From: Amit Pundir @ 2022-05-23  5:41 UTC (permalink / raw)
  To: krzysztof.kozlowski, Dmitry Baryshkov
  Cc: agross, arnd, bjorn.andersson, devicetree, linux-arm-msm,
	linux-kernel, olof, robh, sboyd, Konrad Dybcio

On Mon, 23 May 2022 at 01:22, Konrad Dybcio
<konrad.dybcio@somainline.org> wrote:
>
> Hi,
>
> removing these properties will not bring almost any benefit (other than making
> some checks happy any saving some <200 LoC) and will make the lives of almost
> all people doing independent development for linux-on-msm harder. There are
> almost unironically like 3 people outside Linaro and QUIC who have
> non-vendor-fused development boards AND the sources to rebuild the
> bootloader on their own. Making it harder to boot is only going to
> discourage people from developing on these devices, which is already not
> that pleasant, especially with newer platforms where you have to fight with
> the oh-so-bright ideas of Android boot chain..
>
> This only concerns devices released before sm8350, as the new ones will not
> even boot with these properties present (or at least SONY Sagami, but I
> doubt it's an isolated case), so other than completing support for older
> devices, it won't be an issue going forward, anyway. But there are give
> or take 50 locked down devices in mainline right now, and many more waiting
> to be upstreamed in various downstream close-to-mainline trees that should
> not be disregarded just because Qualcomm is far from the best at making
> their BSP software stack clean.
>
> One solution is to chainload another, (n+1)-stage bootloader, but this is
> not ideal, as:
>
> 1) the stock bootloader can boot Linux just fine on most devices (except
> for single exceptions, where beloved OEMs didn't implement arm64 booting or
> something)
>
> 2) the boot chain on MSM is already 3- or 4- stage and adding to that will
> only create an unnecessary mess
>
> 3) the job of kernel people is not to break userspace. If the
> device can not even exit bootloader after a kernel upgrade, it's a big
> failure.
>
> If you *really really really* want these either gone or documented, we can
> for example use them in the SOCID driver, read the values from DTB and
> compare against what SMEM has to say and for example print a warning when
> there are inconsistencies or use it as a fallback when it fails for any
> reason, such as using a newer SoC on an older kernel, without updates
> for SOCID read (which are sometimes necessary, which was the case for 8450
> recently, iirc).
>
> My stance is to just leave them as is, as moving them anywhere, or removing
> them at all will cause unnecessary mess and waste time that could have been
> spent on more glaring issues..

I couldn't have put it better myself. I suggest we document these
properties, if that is the blocker, and keep them. A lot has changed
in the last 7 years, we now have dozens of devices booting upstream
kernel using these properties.

And fwiw, I have not used dtbTool before, if anything I'd rather
explore dtb overlays.

Regards,
Amit Pundir

>
> Konrad

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-22 19:51 ` Konrad Dybcio
  2022-05-23  5:41   ` Amit Pundir
@ 2022-05-23  7:21   ` Krzysztof Kozlowski
  2022-05-23 12:02     ` Konrad Dybcio
                       ` (2 more replies)
  2022-06-22  8:21   ` Dmitry Baryshkov
  2 siblings, 3 replies; 25+ messages in thread
From: Krzysztof Kozlowski @ 2022-05-23  7:21 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: agross, arnd, bjorn.andersson, devicetree, linux-arm-msm,
	linux-kernel, olof, robh, sboyd

On 22/05/2022 21:51, Konrad Dybcio wrote:
> Hi,
> 
> removing these properties will not bring almost any benefit (other than making
> some checks happy any saving some <200 LoC) and will make the lives of almost
> all people doing independent development for linux-on-msm harder. There are
> almost unironically like 3 people outside Linaro and QUIC who have
> non-vendor-fused development boards AND the sources to rebuild the
> bootloader on their own. Making it harder to boot is only going to
> discourage people from developing on these devices, which is already not
> that pleasant, especially with newer platforms where you have to fight with
> the oh-so-bright ideas of Android boot chain..
> 
> This only concerns devices released before sm8350, as the new ones will not
> even boot with these properties present (or at least SONY Sagami, but I
> doubt it's an isolated case), so other than completing support for older
> devices, it won't be an issue going forward, anyway. But there are give
> or take 50 locked down devices in mainline right now, and many more waiting
> to be upstreamed in various downstream close-to-mainline trees that should
> not be disregarded just because Qualcomm is far from the best at making
> their BSP software stack clean.

I actually wonder why do you need these properties for community work on
such boards? You ship kernel with one concatenated DTB and the
bootloader does not need the board-id/msm-id fields, doesn't it?

Not mentioning that in the past bootloader was actually not using these
properties at all, because it was the dtbTool who was parsing them. So
in any case either your device works fine without these properties or
you have to use dtbTool, right?

> 
> One solution is to chainload another, (n+1)-stage bootloader, but this is
> not ideal, as:
> 
> 1) the stock bootloader can boot Linux just fine on most devices (except
> for single exceptions, where beloved OEMs didn't implement arm64 booting or
> something)
> 
> 2) the boot chain on MSM is already 3- or 4- stage and adding to that will
> only create an unnecessary mess
> 
> 3) the job of kernel people is not to break userspace. If the
> device can not even exit bootloader after a kernel upgrade, it's a big
> failure.

The job of kernel people is to follow bindings and since they were
introduced 7 years ago, I would say there was plenty of time for that.

If the dtbTool support for the bindings is there, then there is no
breakage, because you had to use dtbTool before so you have to use now.

> 
> If you *really really really* want these either gone or documented, we can
> for example use them in the SOCID driver, read the values from DTB and
> compare against what SMEM has to say and for example print a warning when
> there are inconsistencies or use it as a fallback when it fails for any
> reason, such as using a newer SoC on an older kernel, without updates
> for SOCID read (which are sometimes necessary, which was the case for 8450
> recently, iirc).
> 
> My stance is to just leave them as is, as moving them anywhere, or removing
> them at all will cause unnecessary mess and waste time that could have been
> spent on more glaring issues..
> 
> Konrad


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-23  7:21   ` Krzysztof Kozlowski
@ 2022-05-23 12:02     ` Konrad Dybcio
  2022-05-23 12:14       ` Krzysztof Kozlowski
  2022-05-23 21:29     ` Rob Clark
  2022-05-23 21:50     ` Bjorn Andersson
  2 siblings, 1 reply; 25+ messages in thread
From: Konrad Dybcio @ 2022-05-23 12:02 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: agross, arnd, bjorn.andersson, devicetree, linux-arm-msm,
	linux-kernel, olof, robh, sboyd


On 23/05/2022 09:21, Krzysztof Kozlowski wrote:
> On 22/05/2022 21:51, Konrad Dybcio wrote:
>> Hi,
>>
>> removing these properties will not bring almost any benefit (other than making
>> some checks happy any saving some <200 LoC) and will make the lives of almost
>> all people doing independent development for linux-on-msm harder. There are
>> almost unironically like 3 people outside Linaro and QUIC who have
>> non-vendor-fused development boards AND the sources to rebuild the
>> bootloader on their own. Making it harder to boot is only going to
>> discourage people from developing on these devices, which is already not
>> that pleasant, especially with newer platforms where you have to fight with
>> the oh-so-bright ideas of Android boot chain..
>>
>> This only concerns devices released before sm8350, as the new ones will not
>> even boot with these properties present (or at least SONY Sagami, but I
>> doubt it's an isolated case), so other than completing support for older
>> devices, it won't be an issue going forward, anyway. But there are give
>> or take 50 locked down devices in mainline right now, and many more waiting
>> to be upstreamed in various downstream close-to-mainline trees that should
>> not be disregarded just because Qualcomm is far from the best at making
>> their BSP software stack clean.
> I actually wonder why do you need these properties for community work on
> such boards? You ship kernel with one concatenated DTB and the
> bootloader does not need the board-id/msm-id fields, doesn't it?

If that were the case, I would have never complained about this! It's 
the bootloader itself that needs it, you can see it in a "Best match 
[blah blah] 258/0x1000/...." log line, where it walks through the 
appended (or otherwise compiled into the boot.img) DTBs and looks for 
matches for the burnt-in msm-, board- and (on newer-older platforms) 
pmic-id. If it cannot find these, it refuses to boot with an Android 
Verified Boot red state and you get a not-so-nice "Your device has been 
unlocked and the boot image is not working" or something like this on 
your screen.


>
> Not mentioning that in the past bootloader was actually not using these
> properties at all, because it was the dtbTool who was parsing them.

Not sure when that was the case, maybe with very old arm32 bootloaders 
in the times before I did development on Qualcomm devices.


>   So
> in any case either your device works fine without these properties or
> you have to use dtbTool, right?

To the best of my idea, wrong :( Unless the vendor modified the LK/XBL 
code on their own, it looks for a "best match" (but if it's not a 
precise match, it won't even bother trying to boot, just fyi..), meaning 
it tries to go through a list of SoC ID and revision pairs (msm-id), 
board IDs (board-id) and PMIC id+rev pairs (pmic-id) and if no match is 
found, it doesn't even exit the bootloader and says something like "no 
dtbs found".


And hence, they are absolutely necessary one way or another.


Konrad

>
>> One solution is to chainload another, (n+1)-stage bootloader, but this is
>> not ideal, as:
>>
>> 1) the stock bootloader can boot Linux just fine on most devices (except
>> for single exceptions, where beloved OEMs didn't implement arm64 booting or
>> something)
>>
>> 2) the boot chain on MSM is already 3- or 4- stage and adding to that will
>> only create an unnecessary mess
>>
>> 3) the job of kernel people is not to break userspace. If the
>> device can not even exit bootloader after a kernel upgrade, it's a big
>> failure.
> The job of kernel people is to follow bindings and since they were
> introduced 7 years ago, I would say there was plenty of time for that.
>
> If the dtbTool support for the bindings is there, then there is no
> breakage, because you had to use dtbTool before so you have to use now.
>
>> If you *really really really* want these either gone or documented, we can
>> for example use them in the SOCID driver, read the values from DTB and
>> compare against what SMEM has to say and for example print a warning when
>> there are inconsistencies or use it as a fallback when it fails for any
>> reason, such as using a newer SoC on an older kernel, without updates
>> for SOCID read (which are sometimes necessary, which was the case for 8450
>> recently, iirc).
>>
>> My stance is to just leave them as is, as moving them anywhere, or removing
>> them at all will cause unnecessary mess and waste time that could have been
>> spent on more glaring issues..
>>
>> Konrad
>
> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-23 12:02     ` Konrad Dybcio
@ 2022-05-23 12:14       ` Krzysztof Kozlowski
  2022-05-23 15:29         ` Konrad Dybcio
  2022-05-23 16:41         ` Trilok Soni
  0 siblings, 2 replies; 25+ messages in thread
From: Krzysztof Kozlowski @ 2022-05-23 12:14 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: agross, arnd, bjorn.andersson, devicetree, linux-arm-msm,
	linux-kernel, olof, robh, sboyd

On 23/05/2022 14:02, Konrad Dybcio wrote:
> 
> On 23/05/2022 09:21, Krzysztof Kozlowski wrote:
>> On 22/05/2022 21:51, Konrad Dybcio wrote:
>>> Hi,
>>>
>>> removing these properties will not bring almost any benefit (other than making
>>> some checks happy any saving some <200 LoC) and will make the lives of almost
>>> all people doing independent development for linux-on-msm harder. There are
>>> almost unironically like 3 people outside Linaro and QUIC who have
>>> non-vendor-fused development boards AND the sources to rebuild the
>>> bootloader on their own. Making it harder to boot is only going to
>>> discourage people from developing on these devices, which is already not
>>> that pleasant, especially with newer platforms where you have to fight with
>>> the oh-so-bright ideas of Android boot chain..
>>>
>>> This only concerns devices released before sm8350, as the new ones will not
>>> even boot with these properties present (or at least SONY Sagami, but I
>>> doubt it's an isolated case), so other than completing support for older
>>> devices, it won't be an issue going forward, anyway. But there are give
>>> or take 50 locked down devices in mainline right now, and many more waiting
>>> to be upstreamed in various downstream close-to-mainline trees that should
>>> not be disregarded just because Qualcomm is far from the best at making
>>> their BSP software stack clean.
>> I actually wonder why do you need these properties for community work on
>> such boards? You ship kernel with one concatenated DTB and the
>> bootloader does not need the board-id/msm-id fields, doesn't it?
> 
> If that were the case, I would have never complained about this! It's 
> the bootloader itself that needs it, you can see it in a "Best match 
> [blah blah] 258/0x1000/...." log line, where it walks through the 
> appended (or otherwise compiled into the boot.img) DTBs and looks for 
> matches for the burnt-in msm-, board- and (on newer-older platforms) 
> pmic-id. If it cannot find these, it refuses to boot with an Android 
> Verified Boot red state and you get a not-so-nice "Your device has been 
> unlocked and the boot image is not working" or something like this on 
> your screen.
> 
> 
>>
>> Not mentioning that in the past bootloader was actually not using these
>> properties at all, because it was the dtbTool who was parsing them.
> 
> Not sure when that was the case, maybe with very old arm32 bootloaders 
> in the times before I did development on Qualcomm devices.
> 
> 
>>   So
>> in any case either your device works fine without these properties or
>> you have to use dtbTool, right?
> 
> To the best of my idea, wrong :( Unless the vendor modified the LK/XBL 
> code on their own, it looks for a "best match" (but if it's not a 
> precise match, it won't even bother trying to boot, just fyi..), meaning 
> it tries to go through a list of SoC ID and revision pairs (msm-id), 
> board IDs (board-id) and PMIC id+rev pairs (pmic-id) and if no match is 
> found, it doesn't even exit the bootloader and says something like "no 
> dtbs found".

This would mean that dtbTool as described in the actual patch [1] is not
used and bootloader ignores the table. If that's the case, the commit
and requirement of such complex board-foundry-pmic-compatibles should be
dropped. So I am getting now to what Dmitry said...

[1]
https://lore.kernel.org/all/1448062280-15406-2-git-send-email-sboyd@codeaurora.org/


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-23 12:14       ` Krzysztof Kozlowski
@ 2022-05-23 15:29         ` Konrad Dybcio
  2022-05-23 16:41         ` Trilok Soni
  1 sibling, 0 replies; 25+ messages in thread
From: Konrad Dybcio @ 2022-05-23 15:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: agross, arnd, bjorn.andersson, devicetree, linux-arm-msm,
	linux-kernel, olof, robh, sboyd


On 23/05/2022 14:14, Krzysztof Kozlowski wrote:
> On 23/05/2022 14:02, Konrad Dybcio wrote:
>> On 23/05/2022 09:21, Krzysztof Kozlowski wrote:
>>> On 22/05/2022 21:51, Konrad Dybcio wrote:
>>>> Hi,
>>>>
>>>> removing these properties will not bring almost any benefit (other than making
>>>> some checks happy any saving some <200 LoC) and will make the lives of almost
>>>> all people doing independent development for linux-on-msm harder. There are
>>>> almost unironically like 3 people outside Linaro and QUIC who have
>>>> non-vendor-fused development boards AND the sources to rebuild the
>>>> bootloader on their own. Making it harder to boot is only going to
>>>> discourage people from developing on these devices, which is already not
>>>> that pleasant, especially with newer platforms where you have to fight with
>>>> the oh-so-bright ideas of Android boot chain..
>>>>
>>>> This only concerns devices released before sm8350, as the new ones will not
>>>> even boot with these properties present (or at least SONY Sagami, but I
>>>> doubt it's an isolated case), so other than completing support for older
>>>> devices, it won't be an issue going forward, anyway. But there are give
>>>> or take 50 locked down devices in mainline right now, and many more waiting
>>>> to be upstreamed in various downstream close-to-mainline trees that should
>>>> not be disregarded just because Qualcomm is far from the best at making
>>>> their BSP software stack clean.
>>> I actually wonder why do you need these properties for community work on
>>> such boards? You ship kernel with one concatenated DTB and the
>>> bootloader does not need the board-id/msm-id fields, doesn't it?
>> If that were the case, I would have never complained about this! It's
>> the bootloader itself that needs it, you can see it in a "Best match
>> [blah blah] 258/0x1000/...." log line, where it walks through the
>> appended (or otherwise compiled into the boot.img) DTBs and looks for
>> matches for the burnt-in msm-, board- and (on newer-older platforms)
>> pmic-id. If it cannot find these, it refuses to boot with an Android
>> Verified Boot red state and you get a not-so-nice "Your device has been
>> unlocked and the boot image is not working" or something like this on
>> your screen.
>>
>>
>>> Not mentioning that in the past bootloader was actually not using these
>>> properties at all, because it was the dtbTool who was parsing them.
>> Not sure when that was the case, maybe with very old arm32 bootloaders
>> in the times before I did development on Qualcomm devices.
>>
>>
>>>    So
>>> in any case either your device works fine without these properties or
>>> you have to use dtbTool, right?
>> To the best of my idea, wrong :( Unless the vendor modified the LK/XBL
>> code on their own, it looks for a "best match" (but if it's not a
>> precise match, it won't even bother trying to boot, just fyi..), meaning
>> it tries to go through a list of SoC ID and revision pairs (msm-id),
>> board IDs (board-id) and PMIC id+rev pairs (pmic-id) and if no match is
>> found, it doesn't even exit the bootloader and says something like "no
>> dtbs found".
> This would mean that dtbTool as described in the actual patch [1] is not
> used and bootloader ignores the table. If that's the case, the commit
> and requirement of such complex board-foundry-pmic-compatibles should be
> dropped. So I am getting now to what Dmitry said...
>
> [1]
> https://lore.kernel.org/all/1448062280-15406-2-git-send-email-sboyd@codeaurora.org/

This solution assumes everybody is using the so-called QCDT images, 
which is not necessarily the case, as not all bootloaders (even if they 
should, as their base BSP tags sometimes imply) support that. Others, in 
turn, require that and will not recognize appended DTBs properly for 
reasons unknown..


I once went as far as writing up solutions to getting a boot on almost 
all combinations of these.. I may even still have it stashed somewhere.. 
things get crazy when you factor in DTBO and GKI..


Konrad

>
>
> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-23 12:14       ` Krzysztof Kozlowski
  2022-05-23 15:29         ` Konrad Dybcio
@ 2022-05-23 16:41         ` Trilok Soni
  2022-05-23 21:34           ` Bjorn Andersson
  1 sibling, 1 reply; 25+ messages in thread
From: Trilok Soni @ 2022-05-23 16:41 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Konrad Dybcio
  Cc: agross, arnd, bjorn.andersson, devicetree, linux-arm-msm,
	linux-kernel, olof, robh, sboyd

Hello Krzysztof,

On 5/23/2022 5:14 AM, Krzysztof Kozlowski wrote:
> On 23/05/2022 14:02, Konrad Dybcio wrote:
>>
>> On 23/05/2022 09:21, Krzysztof Kozlowski wrote:
>>> On 22/05/2022 21:51, Konrad Dybcio wrote:
>>>> Hi,
>>>>
>>>> removing these properties will not bring almost any benefit (other than making
>>>> some checks happy any saving some <200 LoC) and will make the lives of almost
>>>> all people doing independent development for linux-on-msm harder. There are
>>>> almost unironically like 3 people outside Linaro and QUIC who have
>>>> non-vendor-fused development boards AND the sources to rebuild the
>>>> bootloader on their own. Making it harder to boot is only going to
>>>> discourage people from developing on these devices, which is already not
>>>> that pleasant, especially with newer platforms where you have to fight with
>>>> the oh-so-bright ideas of Android boot chain..
>>>>
>>>> This only concerns devices released before sm8350, as the new ones will not
>>>> even boot with these properties present (or at least SONY Sagami, but I
>>>> doubt it's an isolated case), so other than completing support for older
>>>> devices, it won't be an issue going forward, anyway. But there are give
>>>> or take 50 locked down devices in mainline right now, and many more waiting
>>>> to be upstreamed in various downstream close-to-mainline trees that should
>>>> not be disregarded just because Qualcomm is far from the best at making
>>>> their BSP software stack clean.
>>> I actually wonder why do you need these properties for community work on
>>> such boards? You ship kernel with one concatenated DTB and the
>>> bootloader does not need the board-id/msm-id fields, doesn't it?
>>
>> If that were the case, I would have never complained about this! It's
>> the bootloader itself that needs it, you can see it in a "Best match
>> [blah blah] 258/0x1000/...." log line, where it walks through the
>> appended (or otherwise compiled into the boot.img) DTBs and looks for
>> matches for the burnt-in msm-, board- and (on newer-older platforms)
>> pmic-id. If it cannot find these, it refuses to boot with an Android
>> Verified Boot red state and you get a not-so-nice "Your device has been
>> unlocked and the boot image is not working" or something like this on
>> your screen.
>>
>>
>>>
>>> Not mentioning that in the past bootloader was actually not using these
>>> properties at all, because it was the dtbTool who was parsing them.
>>
>> Not sure when that was the case, maybe with very old arm32 bootloaders
>> in the times before I did development on Qualcomm devices.
>>
>>
>>>    So
>>> in any case either your device works fine without these properties or
>>> you have to use dtbTool, right?
>>
>> To the best of my idea, wrong :( Unless the vendor modified the LK/XBL
>> code on their own, it looks for a "best match" (but if it's not a
>> precise match, it won't even bother trying to boot, just fyi..), meaning
>> it tries to go through a list of SoC ID and revision pairs (msm-id),
>> board IDs (board-id) and PMIC id+rev pairs (pmic-id) and if no match is
>> found, it doesn't even exit the bootloader and says something like "no
>> dtbs found".
> 
> This would mean that dtbTool as described in the actual patch [1] is not
> used and bootloader ignores the table. If that's the case, the commit
> and requirement of such complex board-foundry-pmic-compatibles should be
> dropped. So I am getting now to what Dmitry said...
> 
> [1]
> https://lore.kernel.org/all/1448062280-15406-2-git-send-email-sboyd@codeaurora.org/


The link above is from 2015. Lot has changed downstream. Most of what 
was mentioned by Konrad is right. Application bootloader acts on picking 
on platform DTBO based on the platform ID plus some combinations like 
PMIC etc; These platform DTBOs gets overlay on top of SOC DTB by the 
Application bootloader.

We have moved to DTBO for all the latest targets, but I can understand 
that some old targets at upstream could be using the very old approaches.

Downstream all of the platforms including the DTBO files will need 
board-id and msm-id since we also do the compile time stitching of dtb + 
dtbo and dtbo + dtbo to generate the proper SOC DTB and PLATFORM DTBOs 
which gets flashed in the DTBO partition and follows the Android boot 
requirements. Application bootloader then picks the right Platform DTBO 
as mentioned above w/ the right SOC DTB. It gets more complicated w/ GKI 
new requirements every year (better for GKI, may not be better for 
upstream kernel + downstream bootloader combination).

---Trilok Soni



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-23  7:21   ` Krzysztof Kozlowski
  2022-05-23 12:02     ` Konrad Dybcio
@ 2022-05-23 21:29     ` Rob Clark
  2022-05-23 21:50     ` Bjorn Andersson
  2 siblings, 0 replies; 25+ messages in thread
From: Rob Clark @ 2022-05-23 21:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Konrad Dybcio, Andy Gross, Arnd Bergmann, Bjorn Andersson,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, Linux Kernel Mailing List, Olof Johansson,
	Rob Herring, Stephen Boyd

On Mon, May 23, 2022 at 1:21 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 22/05/2022 21:51, Konrad Dybcio wrote:
> > Hi,
> >
> > removing these properties will not bring almost any benefit (other than making
> > some checks happy any saving some <200 LoC) and will make the lives of almost
> > all people doing independent development for linux-on-msm harder. There are
> > almost unironically like 3 people outside Linaro and QUIC who have
> > non-vendor-fused development boards AND the sources to rebuild the
> > bootloader on their own. Making it harder to boot is only going to
> > discourage people from developing on these devices, which is already not
> > that pleasant, especially with newer platforms where you have to fight with
> > the oh-so-bright ideas of Android boot chain..
> >
> > This only concerns devices released before sm8350, as the new ones will not
> > even boot with these properties present (or at least SONY Sagami, but I
> > doubt it's an isolated case), so other than completing support for older
> > devices, it won't be an issue going forward, anyway. But there are give
> > or take 50 locked down devices in mainline right now, and many more waiting
> > to be upstreamed in various downstream close-to-mainline trees that should
> > not be disregarded just because Qualcomm is far from the best at making
> > their BSP software stack clean.
>
> I actually wonder why do you need these properties for community work on
> such boards? You ship kernel with one concatenated DTB and the
> bootloader does not need the board-id/msm-id fields, doesn't it?
>
> Not mentioning that in the past bootloader was actually not using these
> properties at all, because it was the dtbTool who was parsing them. So
> in any case either your device works fine without these properties or
> you have to use dtbTool, right?
>
> >
> > One solution is to chainload another, (n+1)-stage bootloader, but this is
> > not ideal, as:
> >
> > 1) the stock bootloader can boot Linux just fine on most devices (except
> > for single exceptions, where beloved OEMs didn't implement arm64 booting or
> > something)
> >
> > 2) the boot chain on MSM is already 3- or 4- stage and adding to that will
> > only create an unnecessary mess
> >
> > 3) the job of kernel people is not to break userspace. If the
> > device can not even exit bootloader after a kernel upgrade, it's a big
> > failure.
>
> The job of kernel people is to follow bindings and since they were
> introduced 7 years ago, I would say there was plenty of time for that.

Then we should document these fields to reflect reality, rather than
remove them.  The kernel isn't the only consumer of dtb ;-)

> If the dtbTool support for the bindings is there, then there is no
> breakage, because you had to use dtbTool before so you have to use now.

I don't believe this was the case?  At any rate, why are we trying so
hard to make our lives harder?  Let's just acknowledge reality
(bootloader uses these fields), document it, and move on with life

BR,
-R

> >
> > If you *really really really* want these either gone or documented, we can
> > for example use them in the SOCID driver, read the values from DTB and
> > compare against what SMEM has to say and for example print a warning when
> > there are inconsistencies or use it as a fallback when it fails for any
> > reason, such as using a newer SoC on an older kernel, without updates
> > for SOCID read (which are sometimes necessary, which was the case for 8450
> > recently, iirc).
> >
> > My stance is to just leave them as is, as moving them anywhere, or removing
> > them at all will cause unnecessary mess and waste time that could have been
> > spent on more glaring issues..
> >
> > Konrad
>
>
> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-23 16:41         ` Trilok Soni
@ 2022-05-23 21:34           ` Bjorn Andersson
  2022-05-23 22:13             ` Trilok Soni
  0 siblings, 1 reply; 25+ messages in thread
From: Bjorn Andersson @ 2022-05-23 21:34 UTC (permalink / raw)
  To: Trilok Soni
  Cc: Krzysztof Kozlowski, Konrad Dybcio, agross, arnd, devicetree,
	linux-arm-msm, linux-kernel, olof, robh, sboyd

On Mon 23 May 11:41 CDT 2022, Trilok Soni wrote:

> Hello Krzysztof,
> 
> On 5/23/2022 5:14 AM, Krzysztof Kozlowski wrote:
> > On 23/05/2022 14:02, Konrad Dybcio wrote:
> > > 
> > > On 23/05/2022 09:21, Krzysztof Kozlowski wrote:
> > > > On 22/05/2022 21:51, Konrad Dybcio wrote:
> > > > > Hi,
> > > > > 
> > > > > removing these properties will not bring almost any benefit (other than making
> > > > > some checks happy any saving some <200 LoC) and will make the lives of almost
> > > > > all people doing independent development for linux-on-msm harder. There are
> > > > > almost unironically like 3 people outside Linaro and QUIC who have
> > > > > non-vendor-fused development boards AND the sources to rebuild the
> > > > > bootloader on their own. Making it harder to boot is only going to
> > > > > discourage people from developing on these devices, which is already not
> > > > > that pleasant, especially with newer platforms where you have to fight with
> > > > > the oh-so-bright ideas of Android boot chain..
> > > > > 
> > > > > This only concerns devices released before sm8350, as the new ones will not
> > > > > even boot with these properties present (or at least SONY Sagami, but I
> > > > > doubt it's an isolated case), so other than completing support for older
> > > > > devices, it won't be an issue going forward, anyway. But there are give
> > > > > or take 50 locked down devices in mainline right now, and many more waiting
> > > > > to be upstreamed in various downstream close-to-mainline trees that should
> > > > > not be disregarded just because Qualcomm is far from the best at making
> > > > > their BSP software stack clean.
> > > > I actually wonder why do you need these properties for community work on
> > > > such boards? You ship kernel with one concatenated DTB and the
> > > > bootloader does not need the board-id/msm-id fields, doesn't it?
> > > 
> > > If that were the case, I would have never complained about this! It's
> > > the bootloader itself that needs it, you can see it in a "Best match
> > > [blah blah] 258/0x1000/...." log line, where it walks through the
> > > appended (or otherwise compiled into the boot.img) DTBs and looks for
> > > matches for the burnt-in msm-, board- and (on newer-older platforms)
> > > pmic-id. If it cannot find these, it refuses to boot with an Android
> > > Verified Boot red state and you get a not-so-nice "Your device has been
> > > unlocked and the boot image is not working" or something like this on
> > > your screen.
> > > 
> > > 
> > > > 
> > > > Not mentioning that in the past bootloader was actually not using these
> > > > properties at all, because it was the dtbTool who was parsing them.
> > > 
> > > Not sure when that was the case, maybe with very old arm32 bootloaders
> > > in the times before I did development on Qualcomm devices.
> > > 
> > > 
> > > >    So
> > > > in any case either your device works fine without these properties or
> > > > you have to use dtbTool, right?
> > > 
> > > To the best of my idea, wrong :( Unless the vendor modified the LK/XBL
> > > code on their own, it looks for a "best match" (but if it's not a
> > > precise match, it won't even bother trying to boot, just fyi..), meaning
> > > it tries to go through a list of SoC ID and revision pairs (msm-id),
> > > board IDs (board-id) and PMIC id+rev pairs (pmic-id) and if no match is
> > > found, it doesn't even exit the bootloader and says something like "no
> > > dtbs found".
> > 
> > This would mean that dtbTool as described in the actual patch [1] is not
> > used and bootloader ignores the table. If that's the case, the commit
> > and requirement of such complex board-foundry-pmic-compatibles should be
> > dropped. So I am getting now to what Dmitry said...
> > 
> > [1]
> > https://lore.kernel.org/all/1448062280-15406-2-git-send-email-sboyd@codeaurora.org/
> 
> 
> The link above is from 2015. Lot has changed downstream. Most of what was
> mentioned by Konrad is right. Application bootloader acts on picking on
> platform DTBO based on the platform ID plus some combinations like PMIC etc;
> These platform DTBOs gets overlay on top of SOC DTB by the Application
> bootloader.
> 
> We have moved to DTBO for all the latest targets, but I can understand that
> some old targets at upstream could be using the very old approaches.
> 
> Downstream all of the platforms including the DTBO files will need board-id
> and msm-id since we also do the compile time stitching of dtb + dtbo and
> dtbo + dtbo to generate the proper SOC DTB and PLATFORM DTBOs which gets
> flashed in the DTBO partition and follows the Android boot requirements.
> Application bootloader then picks the right Platform DTBO as mentioned above
> w/ the right SOC DTB. It gets more complicated w/ GKI new requirements every
> year (better for GKI, may not be better for upstream kernel + downstream
> bootloader combination).
> 

FWIW, this doesn't fit with the upstream model at all. In particular the
DTBO that comes with the devices are not compatible with any upstream
DTB.

As such, the first step to run an upstream DTB+kernel is to zero out the
dtbo partitions.


With the DTBO cleared, most devices (all Qualcomm reference devices) can
be booted with the dtb appended to the Image.gz, without the
qcom,{board,msm}-id. As such I would say things are working okay
currently.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-23  7:21   ` Krzysztof Kozlowski
  2022-05-23 12:02     ` Konrad Dybcio
  2022-05-23 21:29     ` Rob Clark
@ 2022-05-23 21:50     ` Bjorn Andersson
  2022-05-23 22:18       ` Dmitry Baryshkov
  2022-05-26  7:16       ` Krzysztof Kozlowski
  2 siblings, 2 replies; 25+ messages in thread
From: Bjorn Andersson @ 2022-05-23 21:50 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Konrad Dybcio, agross, arnd, devicetree, linux-arm-msm,
	linux-kernel, olof, robh, sboyd

On Mon 23 May 02:21 CDT 2022, Krzysztof Kozlowski wrote:

> On 22/05/2022 21:51, Konrad Dybcio wrote:
> > Hi,
> > 
> > removing these properties will not bring almost any benefit (other than making
> > some checks happy any saving some <200 LoC) and will make the lives of almost
> > all people doing independent development for linux-on-msm harder. There are
> > almost unironically like 3 people outside Linaro and QUIC who have
> > non-vendor-fused development boards AND the sources to rebuild the
> > bootloader on their own. Making it harder to boot is only going to
> > discourage people from developing on these devices, which is already not
> > that pleasant, especially with newer platforms where you have to fight with
> > the oh-so-bright ideas of Android boot chain..
> > 
> > This only concerns devices released before sm8350, as the new ones will not
> > even boot with these properties present (or at least SONY Sagami, but I
> > doubt it's an isolated case), so other than completing support for older
> > devices, it won't be an issue going forward, anyway. But there are give
> > or take 50 locked down devices in mainline right now, and many more waiting
> > to be upstreamed in various downstream close-to-mainline trees that should
> > not be disregarded just because Qualcomm is far from the best at making
> > their BSP software stack clean.
> 
> I actually wonder why do you need these properties for community work on
> such boards? You ship kernel with one concatenated DTB and the
> bootloader does not need the board-id/msm-id fields, doesn't it?
> 

During the last years all reference devices that I know of has allowed
us to boot Image.gz+dtb concatenated kernels without
qcom,{board-msm}-id.

There's however been several end-user devices that for some reason
refuse to accept the concatenated dtb unless these values matches.

> Not mentioning that in the past bootloader was actually not using these
> properties at all, because it was the dtbTool who was parsing them. So
> in any case either your device works fine without these properties or
> you have to use dtbTool, right?
> 

Unfortunately not. There are the devices which accepts a single appended
dtb without these properties, but beyond that it's been a large mix.

I've seen cases where dtbTool packs up a number of dtbs, but the loaded
one still need to have these properties, and there are devices out there
that supports multiple appended dtbs etc.


Last but not least, forcing everyone to use dtbTool adds a
non-standardized tool to everyone's workflow, a tool that has to be kept
up to date with the compatible to msm/board-id mapping.

> > 
> > One solution is to chainload another, (n+1)-stage bootloader, but this is
> > not ideal, as:
> > 
> > 1) the stock bootloader can boot Linux just fine on most devices (except
> > for single exceptions, where beloved OEMs didn't implement arm64 booting or
> > something)
> > 
> > 2) the boot chain on MSM is already 3- or 4- stage and adding to that will
> > only create an unnecessary mess
> > 
> > 3) the job of kernel people is not to break userspace. If the
> > device can not even exit bootloader after a kernel upgrade, it's a big
> > failure.
> 
> The job of kernel people is to follow bindings and since they were
> introduced 7 years ago, I would say there was plenty of time for that.
> 

We're following the bindings and don't pick board-id or msm-id unless
there's a particular reason for it - which typically is that the
downstream bootloader requires it - we don't use the properties on the
kernel side.

> If the dtbTool support for the bindings is there, then there is no
> breakage, because you had to use dtbTool before so you have to use now.
> 

Among all the platforms I maintain, MSM8916 (db410c) is the only one
where I use dtbTool - because it refuses to accept the concatenated
dtb.

Regards,
Bjorn

> > 
> > If you *really really really* want these either gone or documented, we can
> > for example use them in the SOCID driver, read the values from DTB and
> > compare against what SMEM has to say and for example print a warning when
> > there are inconsistencies or use it as a fallback when it fails for any
> > reason, such as using a newer SoC on an older kernel, without updates
> > for SOCID read (which are sometimes necessary, which was the case for 8450
> > recently, iirc).
> > 
> > My stance is to just leave them as is, as moving them anywhere, or removing
> > them at all will cause unnecessary mess and waste time that could have been
> > spent on more glaring issues..
> > 
> > Konrad
> 
> 
> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-23 21:34           ` Bjorn Andersson
@ 2022-05-23 22:13             ` Trilok Soni
  0 siblings, 0 replies; 25+ messages in thread
From: Trilok Soni @ 2022-05-23 22:13 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Krzysztof Kozlowski, Konrad Dybcio, agross, arnd, devicetree,
	linux-arm-msm, linux-kernel, olof, robh, sboyd

Hi Bjorn,

On 5/23/2022 2:34 PM, Bjorn Andersson wrote:
> On Mon 23 May 11:41 CDT 2022, Trilok Soni wrote:
> 
>> Hello Krzysztof,
>>
>> On 5/23/2022 5:14 AM, Krzysztof Kozlowski wrote:
>>> On 23/05/2022 14:02, Konrad Dybcio wrote:
>>>>
>>>> On 23/05/2022 09:21, Krzysztof Kozlowski wrote:
>>>>> On 22/05/2022 21:51, Konrad Dybcio wrote:
>>>>>> Hi,
>>>>>>
>>>>>> removing these properties will not bring almost any benefit (other than making
>>>>>> some checks happy any saving some <200 LoC) and will make the lives of almost
>>>>>> all people doing independent development for linux-on-msm harder. There are
>>>>>> almost unironically like 3 people outside Linaro and QUIC who have
>>>>>> non-vendor-fused development boards AND the sources to rebuild the
>>>>>> bootloader on their own. Making it harder to boot is only going to
>>>>>> discourage people from developing on these devices, which is already not
>>>>>> that pleasant, especially with newer platforms where you have to fight with
>>>>>> the oh-so-bright ideas of Android boot chain..
>>>>>>
>>>>>> This only concerns devices released before sm8350, as the new ones will not
>>>>>> even boot with these properties present (or at least SONY Sagami, but I
>>>>>> doubt it's an isolated case), so other than completing support for older
>>>>>> devices, it won't be an issue going forward, anyway. But there are give
>>>>>> or take 50 locked down devices in mainline right now, and many more waiting
>>>>>> to be upstreamed in various downstream close-to-mainline trees that should
>>>>>> not be disregarded just because Qualcomm is far from the best at making
>>>>>> their BSP software stack clean.
>>>>> I actually wonder why do you need these properties for community work on
>>>>> such boards? You ship kernel with one concatenated DTB and the
>>>>> bootloader does not need the board-id/msm-id fields, doesn't it?
>>>>
>>>> If that were the case, I would have never complained about this! It's
>>>> the bootloader itself that needs it, you can see it in a "Best match
>>>> [blah blah] 258/0x1000/...." log line, where it walks through the
>>>> appended (or otherwise compiled into the boot.img) DTBs and looks for
>>>> matches for the burnt-in msm-, board- and (on newer-older platforms)
>>>> pmic-id. If it cannot find these, it refuses to boot with an Android
>>>> Verified Boot red state and you get a not-so-nice "Your device has been
>>>> unlocked and the boot image is not working" or something like this on
>>>> your screen.
>>>>
>>>>
>>>>>
>>>>> Not mentioning that in the past bootloader was actually not using these
>>>>> properties at all, because it was the dtbTool who was parsing them.
>>>>
>>>> Not sure when that was the case, maybe with very old arm32 bootloaders
>>>> in the times before I did development on Qualcomm devices.
>>>>
>>>>
>>>>>     So
>>>>> in any case either your device works fine without these properties or
>>>>> you have to use dtbTool, right?
>>>>
>>>> To the best of my idea, wrong :( Unless the vendor modified the LK/XBL
>>>> code on their own, it looks for a "best match" (but if it's not a
>>>> precise match, it won't even bother trying to boot, just fyi..), meaning
>>>> it tries to go through a list of SoC ID and revision pairs (msm-id),
>>>> board IDs (board-id) and PMIC id+rev pairs (pmic-id) and if no match is
>>>> found, it doesn't even exit the bootloader and says something like "no
>>>> dtbs found".
>>>
>>> This would mean that dtbTool as described in the actual patch [1] is not
>>> used and bootloader ignores the table. If that's the case, the commit
>>> and requirement of such complex board-foundry-pmic-compatibles should be
>>> dropped. So I am getting now to what Dmitry said...
>>>
>>> [1]
>>> https://lore.kernel.org/all/1448062280-15406-2-git-send-email-sboyd@codeaurora.org/
>>
>>
>> The link above is from 2015. Lot has changed downstream. Most of what was
>> mentioned by Konrad is right. Application bootloader acts on picking on
>> platform DTBO based on the platform ID plus some combinations like PMIC etc;
>> These platform DTBOs gets overlay on top of SOC DTB by the Application
>> bootloader.
>>
>> We have moved to DTBO for all the latest targets, but I can understand that
>> some old targets at upstream could be using the very old approaches.
>>
>> Downstream all of the platforms including the DTBO files will need board-id
>> and msm-id since we also do the compile time stitching of dtb + dtbo and
>> dtbo + dtbo to generate the proper SOC DTB and PLATFORM DTBOs which gets
>> flashed in the DTBO partition and follows the Android boot requirements.
>> Application bootloader then picks the right Platform DTBO as mentioned above
>> w/ the right SOC DTB. It gets more complicated w/ GKI new requirements every
>> year (better for GKI, may not be better for upstream kernel + downstream
>> bootloader combination).
>>
> 
> FWIW, this doesn't fit with the upstream model at all. In particular the
> DTBO that comes with the devices are not compatible with any upstream
> DTB.
> 
> As such, the first step to run an upstream DTB+kernel is to zero out the
> dtbo partitions.
> 
> 
> With the DTBO cleared, most devices (all Qualcomm reference devices) can
> be booted with the dtb appended to the Image.gz, without the
> qcom,{board,msm}-id. As such I would say things are working okay
> currently.
> 

Thanks. Yup, I know things are working fine right now. May be we can 
look at changing the downstream bootloader so that you don't need to 
erase the DTBO partition for reference/unlocked devices. No promise, but 
it will make easy for anyone do the upstream development on the 
reference devices.

---Trilok Soni

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-23 21:50     ` Bjorn Andersson
@ 2022-05-23 22:18       ` Dmitry Baryshkov
  2022-05-25 17:36         ` Stephan Gerhold
  2022-05-26  7:16       ` Krzysztof Kozlowski
  1 sibling, 1 reply; 25+ messages in thread
From: Dmitry Baryshkov @ 2022-05-23 22:18 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Krzysztof Kozlowski, Konrad Dybcio, agross, arnd, devicetree,
	linux-arm-msm, linux-kernel, olof, robh, sboyd

On Tue, 24 May 2022 at 00:50, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Mon 23 May 02:21 CDT 2022, Krzysztof Kozlowski wrote:
>
> > On 22/05/2022 21:51, Konrad Dybcio wrote:
> > I actually wonder why do you need these properties for community work on
> > such boards? You ship kernel with one concatenated DTB and the
> > bootloader does not need the board-id/msm-id fields, doesn't it?

You know, this reminds me of an old argument dating 2005-7: why do we
need to support multi-platform kernels, while we can boot a good plain
single-mach (or a single-board) kernel on a particular board.
Supporting msm-id/board-id/pmic-id gives us flexibility. Dropping them
would remove flexibility.

> > Not mentioning that in the past bootloader was actually not using these
> > properties at all, because it was the dtbTool who was parsing them. So
> > in any case either your device works fine without these properties or
> > you have to use dtbTool, right?

I think it was supposed to be done in an opposite way: to let dtbTool
process compat strings and generate the properties in question.

> > >
> > > One solution is to chainload another, (n+1)-stage bootloader, but this is
> > > not ideal, as:
> > >
> > > 1) the stock bootloader can boot Linux just fine on most devices (except
> > > for single exceptions, where beloved OEMs didn't implement arm64 booting or
> > > something)
> > >
> > > 2) the boot chain on MSM is already 3- or 4- stage and adding to that will
> > > only create an unnecessary mess
> > >
> > > 3) the job of kernel people is not to break userspace. If the
> > > device can not even exit bootloader after a kernel upgrade, it's a big
> > > failure.
> >
> > The job of kernel people is to follow bindings and since they were
> > introduced 7 years ago, I would say there was plenty of time for that.
> >
>
> We're following the bindings and don't pick board-id or msm-id unless
> there's a particular reason for it - which typically is that the
> downstream bootloader requires it - we don't use the properties on the
> kernel side.

Or unless we have another reason (like handling a single RB3+RB5 image).
I suspect PmOS might also like shipping a single image for some/all of
the supported devices. Or we might use that for the qcom-armv8a OE
machine.

>
> > If the dtbTool support for the bindings is there, then there is no
> > breakage, because you had to use dtbTool before so you have to use now.
> >
>
> Among all the platforms I maintain, MSM8916 (db410c) is the only one
> where I use dtbTool - because it refuses to accept the concatenated
> dtb.

It's strange, I have been using concatenated dtb with db410c for ages.


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-23 22:18       ` Dmitry Baryshkov
@ 2022-05-25 17:36         ` Stephan Gerhold
  0 siblings, 0 replies; 25+ messages in thread
From: Stephan Gerhold @ 2022-05-25 17:36 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Krzysztof Kozlowski, Konrad Dybcio, agross,
	arnd, devicetree, linux-arm-msm, linux-kernel, olof, robh, sboyd

On Tue, May 24, 2022 at 01:18:53AM +0300, Dmitry Baryshkov wrote:
> > We're following the bindings and don't pick board-id or msm-id unless
> > there's a particular reason for it - which typically is that the
> > downstream bootloader requires it - we don't use the properties on the
> > kernel side.
> 
> Or unless we have another reason (like handling a single RB3+RB5 image).
> I suspect PmOS might also like shipping a single image for some/all of
> the supported devices. Or we might use that for the qcom-armv8a OE
> machine.
> 

On a larger scale the qcom,msm-id/board-id properties are not very
useful for automatic DTB selection. This is simply because they are not
unique when you look beyond just the Qualcomm reference boards. I know
at least 3 totally different smartphones (from different vendors) where
the bootloader picks "msm8916-mtp", even though they have little in
common with Qualcomm's original MTP board.

There are also vendors who made up their own broken numbering scheme,
broken bootloaders that pick seemingly random DTBs or start modifying
random properties etc etc. Perhaps it has improved on more recent
devices but somehow I doubt it...

This means that the qcom,msm-id/board-id properties are really just
useful for making the bootloader happy, or if you have a number of
devices in full control. It's not the consistently implemented standard
that would actually be worth promoting for automatic DTB selection.

> >
> > > If the dtbTool support for the bindings is there, then there is no
> > > breakage, because you had to use dtbTool before so you have to use now.
> > >
> >
> > Among all the platforms I maintain, MSM8916 (db410c) is the only one
> > where I use dtbTool - because it refuses to accept the concatenated
> > dtb.
> 
> It's strange, I have been using concatenated dtb with db410c for ages.
> 

There is a patch in Linaro's LK fork for DB410c that selects the
appended DTB even if the qcom,msm-id/board-id properties are missing:
https://git.linaro.org/landing-teams/working/qualcomm/lk.git/commit/?id=3be1d459a546a24f2bf10b9551663a3e69a8214e

If you don't have this commit or something equivalent, appended DTBs
must have the qcom,msm-id/board-id properties for LK to accept them.

Stephan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-23 21:50     ` Bjorn Andersson
  2022-05-23 22:18       ` Dmitry Baryshkov
@ 2022-05-26  7:16       ` Krzysztof Kozlowski
  1 sibling, 0 replies; 25+ messages in thread
From: Krzysztof Kozlowski @ 2022-05-26  7:16 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Konrad Dybcio, agross, arnd, devicetree, linux-arm-msm,
	linux-kernel, olof, robh, sboyd, Rob Clark, Trilok Soni,
	Amit Pundir, Dmitry Baryshkov

Thank you all (not only Bjorn here) for comments. These are nice
arguments to prepare a proper patch.

If I got correctly all your feedback, the preference is to document
board-id/msm-id boards. I can prepare a patch for this, unless someone
wants to revive old effort (from few years ago).

On 23/05/2022 23:50, Bjorn Andersson wrote:
> On Mon 23 May 02:21 CDT 2022, Krzysztof Kozlowski wrote:
> 
>> On 22/05/2022 21:51, Konrad Dybcio wrote:
>>> Hi,
>>>
>>> removing these properties will not bring almost any benefit (other than making
>>> some checks happy any saving some <200 LoC) and will make the lives of almost
>>> all people doing independent development for linux-on-msm harder. There are
>>> almost unironically like 3 people outside Linaro and QUIC who have
>>> non-vendor-fused development boards AND the sources to rebuild the
>>> bootloader on their own. Making it harder to boot is only going to
>>> discourage people from developing on these devices, which is already not
>>> that pleasant, especially with newer platforms where you have to fight with
>>> the oh-so-bright ideas of Android boot chain..
>>>
>>> This only concerns devices released before sm8350, as the new ones will not
>>> even boot with these properties present (or at least SONY Sagami, but I
>>> doubt it's an isolated case), so other than completing support for older
>>> devices, it won't be an issue going forward, anyway. But there are give
>>> or take 50 locked down devices in mainline right now, and many more waiting
>>> to be upstreamed in various downstream close-to-mainline trees that should
>>> not be disregarded just because Qualcomm is far from the best at making
>>> their BSP software stack clean.
>>
>> I actually wonder why do you need these properties for community work on
>> such boards? You ship kernel with one concatenated DTB and the
>> bootloader does not need the board-id/msm-id fields, doesn't it?
>>
> 
> During the last years all reference devices that I know of has allowed
> us to boot Image.gz+dtb concatenated kernels without
> qcom,{board-msm}-id.
> 
> There's however been several end-user devices that for some reason
> refuse to accept the concatenated dtb unless these values matches.

I think several recently upstreamed boards have board-id/msm-id
properties, so I wonder how can we judge whether these are needed or not.

> 
>> Not mentioning that in the past bootloader was actually not using these
>> properties at all, because it was the dtbTool who was parsing them. So
>> in any case either your device works fine without these properties or
>> you have to use dtbTool, right?
>>
> 
> Unfortunately not. There are the devices which accepts a single appended
> dtb without these properties, but beyond that it's been a large mix.
> 
> I've seen cases where dtbTool packs up a number of dtbs, but the loaded
> one still need to have these properties, and there are devices out there
> that supports multiple appended dtbs etc.
> 
> 
> Last but not least, forcing everyone to use dtbTool adds a
> non-standardized tool to everyone's workflow, a tool that has to be kept
> up to date with the compatible to msm/board-id mapping.

OK

> 
>>>
>>> One solution is to chainload another, (n+1)-stage bootloader, but this is
>>> not ideal, as:
>>>
>>> 1) the stock bootloader can boot Linux just fine on most devices (except
>>> for single exceptions, where beloved OEMs didn't implement arm64 booting or
>>> something)
>>>
>>> 2) the boot chain on MSM is already 3- or 4- stage and adding to that will
>>> only create an unnecessary mess
>>>
>>> 3) the job of kernel people is not to break userspace. If the
>>> device can not even exit bootloader after a kernel upgrade, it's a big
>>> failure.
>>
>> The job of kernel people is to follow bindings and since they were
>> introduced 7 years ago, I would say there was plenty of time for that.
>>
> 
> We're following the bindings and don't pick board-id or msm-id unless
> there's a particular reason for it - which typically is that the
> downstream bootloader requires it - we don't use the properties on the
> kernel side.

I meant to follow existing bindings for compatible:
"qcom,<SoC>[-<soc_version>][-<foundry_id>]-<board>[/<subtype>][-<board_version>]"

I think none of handsets (google, fairphone, lenovo, microsoft, sony,
xiaomi) follow it.

It seems only Qcom boards (with 'qcom' as vendor prefix) use such
pattern, but the bindings say "Each board must specify", not "Board for
such bootloader must specify"...

> 
>> If the dtbTool support for the bindings is there, then there is no
>> breakage, because you had to use dtbTool before so you have to use now.
>>
> 
> Among all the platforms I maintain, MSM8916 (db410c) is the only one
> where I use dtbTool - because it refuses to accept the concatenated
> dtb.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-05-22 19:51 ` Konrad Dybcio
  2022-05-23  5:41   ` Amit Pundir
  2022-05-23  7:21   ` Krzysztof Kozlowski
@ 2022-06-22  8:21   ` Dmitry Baryshkov
  2022-06-22 11:53     ` Konrad Dybcio
  2 siblings, 1 reply; 25+ messages in thread
From: Dmitry Baryshkov @ 2022-06-22  8:21 UTC (permalink / raw)
  To: Konrad Dybcio, krzysztof.kozlowski
  Cc: agross, arnd, bjorn.andersson, devicetree, linux-arm-msm,
	linux-kernel, olof, robh, sboyd

On 22/05/2022 22:51, Konrad Dybcio wrote:
> Hi,
> 
> removing these properties will not bring almost any benefit (other than making
> some checks happy any saving some <200 LoC) and will make the lives of almost
> all people doing independent development for linux-on-msm harder. There are
> almost unironically like 3 people outside Linaro and QUIC who have
> non-vendor-fused development boards AND the sources to rebuild the
> bootloader on their own. Making it harder to boot is only going to
> discourage people from developing on these devices, which is already not
> that pleasant, especially with newer platforms where you have to fight with
> the oh-so-bright ideas of Android boot chain..
> 
> This only concerns devices released before sm8350, as the new ones will not
> even boot with these properties present (or at least SONY Sagami, but I
> doubt it's an isolated case), so other than completing support for older
> devices, it won't be an issue going forward, anyway.

I almost missed this part of the discussion (and Krzysztof had to point 
me to it in discussion of his patches).

I think this is a Sony peculiarity. At least the distributed SM8350 
(lahaina) and SM8450 (waipio) Qualcomm device trees use these properties:

https://github.com/MiCode/kernel_devicetree/blob/zeus-s-oss/qcom/lahaina-hdk.dts
https://github.com/MiCode/kernel_devicetree/blob/zeus-s-oss/qcom/lahaina-v2.1.dtsi
https://github.com/MiCode/kernel_devicetree/blob/zeus-s-oss/qcom/waipio-qrd-pm8010.dts
https://github.com/MiCode/kernel_devicetree/blob/zeus-s-oss/qcom/waipio-v2.dtsi


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-06-22  8:21   ` Dmitry Baryshkov
@ 2022-06-22 11:53     ` Konrad Dybcio
  2022-06-23  9:55       ` Dmitry Baryshkov
  0 siblings, 1 reply; 25+ messages in thread
From: Konrad Dybcio @ 2022-06-22 11:53 UTC (permalink / raw)
  To: Dmitry Baryshkov, krzysztof.kozlowski
  Cc: agross, arnd, bjorn.andersson, devicetree, linux-arm-msm,
	linux-kernel, olof, robh, sboyd



On 22.06.2022 10:21, Dmitry Baryshkov wrote:
> On 22/05/2022 22:51, Konrad Dybcio wrote:
>> Hi,
>>
>> removing these properties will not bring almost any benefit (other than making
>> some checks happy any saving some <200 LoC) and will make the lives of almost
>> all people doing independent development for linux-on-msm harder. There are
>> almost unironically like 3 people outside Linaro and QUIC who have
>> non-vendor-fused development boards AND the sources to rebuild the
>> bootloader on their own. Making it harder to boot is only going to
>> discourage people from developing on these devices, which is already not
>> that pleasant, especially with newer platforms where you have to fight with
>> the oh-so-bright ideas of Android boot chain..
>>
>> This only concerns devices released before sm8350, as the new ones will not
>> even boot with these properties present (or at least SONY Sagami, but I
>> doubt it's an isolated case), so other than completing support for older
>> devices, it won't be an issue going forward, anyway.
> 
> I almost missed this part of the discussion (and Krzysztof had to point me to it in discussion of his patches).
> 
> I think this is a Sony peculiarity. At least the distributed SM8350 (lahaina) and SM8450 (waipio) Qualcomm device trees use these properties:
> 
> https://github.com/MiCode/kernel_devicetree/blob/zeus-s-oss/qcom/lahaina-hdk.dts
> https://github.com/MiCode/kernel_devicetree/blob/zeus-s-oss/qcom/lahaina-v2.1.dtsi
> https://github.com/MiCode/kernel_devicetree/blob/zeus-s-oss/qcom/waipio-qrd-pm8010.dts
> https://github.com/MiCode/kernel_devicetree/blob/zeus-s-oss/qcom/waipio-v2.dtsi
> 
> 
Hi, 

I was puzzled on this back when I first tried to get mainline booting on 8350 too. What I think happened, is that msm-id is used in some code paths, but not others (remember there are plenty of combinations including various Google's inventions from all over the years: QCDT, DTBO, vendor_boot, AVB signage, A/B presence/absence of recovery partition, virtual partitions etc etc).

Frankly, I have no idea why they are still here, but for booting just the kernel (no vendor_boot / GKI / dtbo mess), they need to be absent, at least on Sagami devices. This may be a bug in the Qualcomm bootloader, but they officially have to go with the GKI path to pass Google's compatibility tests, so this may not have been thouroughly tested (if at all), though I highly doubt this is going to change, as vendors are generally reluctant to update their bootloaders and Qualcomm is probably not interested in messing with a useless-to-the-main-purpose feature.

Konrad

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Removal of qcom,board-id and qcom,msm-id
  2022-06-22 11:53     ` Konrad Dybcio
@ 2022-06-23  9:55       ` Dmitry Baryshkov
  0 siblings, 0 replies; 25+ messages in thread
From: Dmitry Baryshkov @ 2022-06-23  9:55 UTC (permalink / raw)
  To: Konrad Dybcio, krzysztof.kozlowski
  Cc: agross, arnd, bjorn.andersson, devicetree, linux-arm-msm,
	linux-kernel, olof, robh, sboyd

On 22/06/2022 14:53, Konrad Dybcio wrote:
> 
> 
> On 22.06.2022 10:21, Dmitry Baryshkov wrote:
>> On 22/05/2022 22:51, Konrad Dybcio wrote:
>>> Hi,
>>>
>>> removing these properties will not bring almost any benefit (other than making
>>> some checks happy any saving some <200 LoC) and will make the lives of almost
>>> all people doing independent development for linux-on-msm harder. There are
>>> almost unironically like 3 people outside Linaro and QUIC who have
>>> non-vendor-fused development boards AND the sources to rebuild the
>>> bootloader on their own. Making it harder to boot is only going to
>>> discourage people from developing on these devices, which is already not
>>> that pleasant, especially with newer platforms where you have to fight with
>>> the oh-so-bright ideas of Android boot chain..
>>>
>>> This only concerns devices released before sm8350, as the new ones will not
>>> even boot with these properties present (or at least SONY Sagami, but I
>>> doubt it's an isolated case), so other than completing support for older
>>> devices, it won't be an issue going forward, anyway.
>>
>> I almost missed this part of the discussion (and Krzysztof had to point me to it in discussion of his patches).
>>
>> I think this is a Sony peculiarity. At least the distributed SM8350 (lahaina) and SM8450 (waipio) Qualcomm device trees use these properties:
>>
>> https://github.com/MiCode/kernel_devicetree/blob/zeus-s-oss/qcom/lahaina-hdk.dts
>> https://github.com/MiCode/kernel_devicetree/blob/zeus-s-oss/qcom/lahaina-v2.1.dtsi
>> https://github.com/MiCode/kernel_devicetree/blob/zeus-s-oss/qcom/waipio-qrd-pm8010.dts
>> https://github.com/MiCode/kernel_devicetree/blob/zeus-s-oss/qcom/waipio-v2.dtsi
>>
>>
> Hi,
> 
> I was puzzled on this back when I first tried to get mainline booting on 8350 too. What I think happened, is that msm-id is used in some code paths, but not others (remember there are plenty of combinations including various Google's inventions from all over the years: QCDT, DTBO, vendor_boot, AVB signage, A/B presence/absence of recovery partition, virtual partitions etc etc).
> 
> Frankly, I have no idea why they are still here, but for booting just the kernel (no vendor_boot / GKI / dtbo mess), they need to be absent, at least on Sagami devices. This may be a bug in the Qualcomm bootloader, but they officially have to go with the GKI path to pass Google's compatibility tests, so this may not have been thouroughly tested (if at all), though I highly doubt this is going to change, as vendors are generally reluctant to update their bootloaders and Qualcomm is probably not interested in messing with a useless-to-the-main-purpose feature.

I remember that on early sm8450 boards/bootloaders we had the issues 
with the bootloader (I don't remember exact details). However I just 
checked the SM4850-HDK + the downstream kernel + appended dtb (which 
contains qcom,msm-id and qcom,board-id properties) and the kernel boots 
fine. So, I can suppose, there was some kind of an issue, which got 
fixed with later ABL releases.

Anyway the latest Krzysztof's text seems fine to me. And if anybody adds 
these properties to the DT, he knows what he is doing and why.

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2022-06-23  9:56 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-19 10:44 Removal of qcom,board-id and qcom,msm-id Krzysztof Kozlowski
2022-05-19 11:39 ` Dmitry Baryshkov
2022-05-19 11:53   ` Krzysztof Kozlowski
2022-05-19 12:46     ` Dmitry Baryshkov
     [not found]       ` <20220519221227.B66D3C385AA@smtp.kernel.org>
2022-05-20  1:39         ` Dmitry Baryshkov
2022-05-20  7:00           ` Krzysztof Kozlowski
2022-05-22 10:26           ` Krzysztof Kozlowski
2022-05-22 10:50             ` Dmitry Baryshkov
2022-05-22 19:51 ` Konrad Dybcio
2022-05-23  5:41   ` Amit Pundir
2022-05-23  7:21   ` Krzysztof Kozlowski
2022-05-23 12:02     ` Konrad Dybcio
2022-05-23 12:14       ` Krzysztof Kozlowski
2022-05-23 15:29         ` Konrad Dybcio
2022-05-23 16:41         ` Trilok Soni
2022-05-23 21:34           ` Bjorn Andersson
2022-05-23 22:13             ` Trilok Soni
2022-05-23 21:29     ` Rob Clark
2022-05-23 21:50     ` Bjorn Andersson
2022-05-23 22:18       ` Dmitry Baryshkov
2022-05-25 17:36         ` Stephan Gerhold
2022-05-26  7:16       ` Krzysztof Kozlowski
2022-06-22  8:21   ` Dmitry Baryshkov
2022-06-22 11:53     ` Konrad Dybcio
2022-06-23  9:55       ` Dmitry Baryshkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).