Re: [PATCH v2 2/3] arm64: dts: exynos: gs101-pixel: add generic gs101-based Pixel support

[Krzysztof Kozlowski <krzk@kernel.org>]

On 23/12/2024 08:59, André Draszik wrote:
> Hi Krzysztof,
> 
> On Sun, 2024-12-22 at 12:42 +0100, Krzysztof Kozlowski wrote:
>> On 20/12/2024 12:27, André Draszik wrote:
>>> In order to support Pixel 6 (Oriole), Pixel 6 Pro (Raven), and Pixel 6a
>>> (Bluejay) correctly, we have to be able to distinguish them properly as
>>> we add support for more features.
>>>
>>> For example, Raven has a larger display. There are other differences,
>>> like battery design capacity, etc.
>>>
>>> To facilitate this, we create a generic gs101-based Pixel DT that can
>>> work on any such gs101-based device. At the same time, we move the
>>
>> No, whatever insanity Android has there, please don't populate it to
>> upstream.
>>
>> There is no such thing as "generic board" thus cannot be a
>> "generic DTS".
> 
> I'll rephrase to gs101-based Pixel base board. Unless you have a better
> suggestion.
> 
>>
>>> Oriole specific parts that we have at the moment (display) into an
>>> overlay, making it easy to add support for Raven and Bluejay in a
>>> similar way.
>>>
>>> Note1:
>>> Despite being an overlay, we instruct kbuild to create a merged
>>> gs101-oriole.dtb and a gs101-oriole.dtbo. This way existing scripts can
>>> keep working, but it also gives the option to just apply the overlay
>>> before boot (e.g. by the bootloader).
>>>
>>> Note2:
>>> I've changed the simple-framebuffer node to specify the memory via
>>> memory-region instead of reg, as that avoids unnecessary duplication
>>> (of the size), and it avoids having to specify #address-cells
>>> and #size-cells in the chosen node (and duplicating this in the DTSO),
>>> which is otherwise necessary to keep dt_binding_check happy and DT
>>> validation working in general.
>>>
>>> Signed-off-by: André Draszik <andre.draszik@linaro.org>
>>> ---
>>> Note: MAINTAINERS doesn't need updating, it covers this whole directory
>>> ---
>>>  arch/arm64/boot/dts/exynos/google/Makefile         |  6 ++--
>>>  .../arm64/boot/dts/exynos/google/gs101-oriole.dtso | 33 ++++++++++++++++++++++
>>>  .../{gs101-oriole.dts => gs101-pixel-generic.dts}  | 24 +++++++---------
>>>  3 files changed, 47 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/exynos/google/Makefile b/arch/arm64/boot/dts/exynos/google/Makefile
>>> index 0a6d5e1fe4ee..6e6b5319212a 100644
>>> --- a/arch/arm64/boot/dts/exynos/google/Makefile
>>> +++ b/arch/arm64/boot/dts/exynos/google/Makefile
>>> @@ -1,4 +1,6 @@
>>>  # SPDX-License-Identifier: GPL-2.0
>>>  
>>> -dtb-$(CONFIG_ARCH_EXYNOS) += \
>>> -	gs101-oriole.dtb \
>>> +dtb-$(CONFIG_ARCH_EXYNOS) += gs101-pixel-generic.dtb
>>> +
>>> +gs101-oriole-dtbs := gs101-pixel-generic.dtb gs101-oriole.dtbo
>>> +dtb-$(CONFIG_ARCH_EXYNOS) += gs101-oriole.dtb
>>> diff --git a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dtso b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dtso
>>> new file mode 100644
>>> index 000000000000..43572039cd07
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dtso
>>> @@ -0,0 +1,33 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * Oriole Device Tree
>>> + *
>>> + * Copyright 2021-2023 Google LLC
>>> + * Copyright 2023-2024 Linaro Ltd
>>> + */
>>> +
>>> +/dts-v1/;
>>> +/plugin/;
>>> +
>>> +&{/} {
>>> +	model = "Oriole";
>>> +	compatible = "google,gs101-oriole", "google,gs101-pixel", "google,gs101";
>>
>> Boards are not overlays. Board equals DTB.
> 
> You're saying this should move into a dts instead of dtso?
> There are numerous boards upstream which use this same dtso
> approach.

Numerous? My quick look found zero. There are for specific
configurations, but not for boards. Look at something which could
support your case: imx8mm-kontron-dl.dtso
In first glance this is a board... but no! This is only about board with
display panel, because panel is detachable.

> 
> There is a base board, and also different versions of it,
> oriole being one of them.

Well, I did not see here baseboard - you renamed it.



Best regards,
Krzysztof