On 23/12/2024 16:31, André Draszik wrote: > On Mon, 2024-12-23 at 15:14 +0100, Krzysztof Kozlowski wrote: >> On 23/12/2024 08:45, André Draszik wrote: >>> Hi Krzysztof, >>> >>> On Sun, 2024-12-22 at 12:38 +0100, Krzysztof Kozlowski wrote: >>>> On 20/12/2024 12:27, André Draszik wrote: >>>>> Raven is Google's code name for Pixel 6 Pro. Since there are >>>>> differences compared to Pixel 6 (Oriole), we need to add a separate >>>>> compatible for it. >>>>> >>>>> We also want to support a generic DT, which can work on any type of >>>> >>>> There are no such generic DT devices upstream, so we cannot add bindings >>>> for them. >>> >>> Do you have a better suggestion for the wording? >>> How about 'gs101-based Pixel base board'? >> >> It's not exactly about the wording but the concept. We don't have >> generic devices, thus no generic DT (DTS). Period. Thus you cannot have >> such schema. > > There is a Pixel base board, with different additions to it, e.g. > different displays. The boot loader can pick the right one. > > Let's discuss that in the other thread to have things in one place :-) >> > >>>>> gs101-based Pixel device, e.g. Pixel 6, or Pixel 6 Pro, or Pixel 6a (as >>>>> a future addition). Such a DT will have certain nodes disabled / not >>>>> added. To facilitate such a generic gs101-based Pixel device, also add >>>>> a more generic gs101-pixel compatible. We can not just use the existing >>>>> google,gs101 for that, as it refers to the SoC, not a board. >>>>> >>>>> Signed-off-by: André Draszik <andre.draszik@xxxxxxxxxx> >>>>> --- >>>>> Documentation/devicetree/bindings/arm/google.yaml | 18 ++++++++++++++---- >>>>> 1 file changed, 14 insertions(+), 4 deletions(-) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/arm/google.yaml b/Documentation/devicetree/bindings/arm/google.yaml >>>>> index e20b5c9b16bc..a8faf2256242 100644 >>>>> --- a/Documentation/devicetree/bindings/arm/google.yaml >>>>> +++ b/Documentation/devicetree/bindings/arm/google.yaml >>>>> @@ -34,11 +34,21 @@ properties: >>>>> const: '/' >>>>> compatible: >>>>> oneOf: >>>>> - - description: Google Pixel 6 / Oriole >>>>> + - description: Google GS101 Pixel devices, as generic Pixel, or Pixel 6 >>>>> + (Oriole), or 6 Pro (Raven) >>>>> + minItems: 2 >>>>> + maxItems: 3 >>>>> items: >>>>> - - enum: >>>>> - - google,gs101-oriole >>>>> - - const: google,gs101 >>>>> + enum: >>>>> + - google,gs101-oriole >>>>> + - google,gs101-raven >>>>> + - google,gs101-pixel >>>>> + - google,gs101 >>>> >>>> SoC cannot be a board in the same time. >>> >>> Can you please expand? google,gs101 is the SoC, the other ones are boards. >>> Is the commit message unclear? >> >> You now say that these are valid boards: >> >> compatible = "google,gs101", "google,gs101"; > > Sorry, I don't see how (apart from the fact that dtbs_check flags > non-unique elements anyway). The result of the patch is: > > minItems: 2 > maxItems: 3 > items: > enum: > - google,gs101-oriole > - google,gs101-raven > - google,gs101-pixel > - google,gs101 The problem is this line. Although entire concept of flexible list is neither need nor ever recommended. > allOf: > - contains: > const: google,gs101-pixel > - contains: > const: google,gs101 > > So one can not have 'google,gs101' twice. And if I only add Still can be, but indeed not with my example but: "google,gs101", "google,gs101", "google,gs101-pixel"; > compatible = "google,gs101", "google,gs101"; > to the .dts, then dtbs_check complains indeed. > >> (although compatibles >> >> compatible = "google,gs101", "google,gs101-pixel"; > > OK, the schema doesn't flag incorrect ordering indeed. > >> Both are wrong. SoC should not be before the board and SoC cannot be >> used alone. Your schema allows that and that's not good. >> >> I did not get what you want to achieve here, so tricky to advice. > > The intention is to enforce either of the following three: > > compatible = "google,gs101-raven", "google,gs101-pixel", "google,gs101"; > compatible = "google,gs101-oriole", "google,gs101-pixel", "google,gs101"; These two are standard - list of three: enum + const + const > compatible = "google,gs101-pixel", "google,gs101"; And that's a separate entry. > > I think this works (but I was aiming for something shorter, > possibly involving minItems): > > compatible: > oneOf: > - description: Google GS101 Pixel base board What is a base board in terms of phone? This can be only final product, you cannot use/have a baseboard. This is not an evalkit. > items: > - const: google,gs101-pixel > - const: google,gs101 Best regards, Krzysztof
