On Wed, Jul 13, 2022 at 11:37:18AM -0700, William Zhang wrote: > Hi Rafal, > > On 7/13/22 03:58, Rafał Miłecki wrote: > > On 2022-07-13 12:50, Rafał Miłecki wrote: > > > On 2022-07-13 02:57, William Zhang wrote: > > > > On 7/12/22 11:18, Krzysztof Kozlowski wrote: > > > > > On 12/07/2022 19:37, William Zhang wrote: > > > > > > > > + - description: BCM4908 Family based boards > > > > > > > > + items: > > > > > > > > + - enum: > > > > > > > > + # BCM4908 SoC based boards > > > > > > > > + - brcm,bcm94908 > > > > > > > > + - asus,gt-ac5300 > > > > > > > > + - netgear,raxe500 > > > > > > > > + # BCM4906 SoC based boards > > > > > > > > + - brcm,bcm94906 > > > > > > > > + - netgear,r8000p > > > > > > > > + - tplink,archer-c2300-v1 > > > > > > > > + - enum: > > > > > > > > + - brcm,bcm4908 > > > > > > > > + - brcm,bcm4906 > > > > > > > > + - brcm,bcm49408 > > > > > > > > > > > > > > This is wrong. brcm,bcm94908 followed by brcm,bcm4906 does not look > > > > > > > like valid list of compatibles. > > > > > > > > > > > > > For 4908 board variant, it will need to be followed by > > > > > > 4908 chip. Sorry > > > > > > for the basic question but is there any requirement to > > > > > > enforce this kind > > > > > > of rule? I would assume dts writer know what he/she is > > > > > > doing and select > > > > > > the right combination. > > > > > > > > > > The entire point of DT schema is to validate DTS. > > > > > Combination like above > > > > > prevents that goal. > > > > > > > > > > Best regards, > > > > > Krzysztof > > > > Understand the DT schema purpose. But items property allows multiple > > > > enums in the list which gives a lot of flexibility but make it hard to > > > > validate. I am not familiar with DT schema, is there any directive to > > > > specify one enum value depending on another so dts validation tool can > > > > report error if combination is wrong? > > > > > > > > This is our preferred format of all bcmbca compatible string > > > > especially when we could have more than 10 chip variants for the same > > > > chip family and we really want to work on the chip family id. We will > > > > make sure they are in the right combination in our own patch and patch > > > > from other contributors. Would this work? If not, I will probably have > > > > to revert the change of 4908(maybe append brcm,bcmbca as this chip > > > > belongs to the same bca group) and use "enum board variant", "const > > > > main chip id", "brcm,bca" for all other chips as our secondary choice. > > > > > > I'm not sure why I didn't even receive 1/3 and half of discussion > > > e-mails. > > > > > > You can't just put all strings into a single bag and allow mixing them > > > in any combos. Please check how it's properly handled in the current > > > existing binding: > > > Documentation/devicetree/bindings/arm/bcm/brcm,bcm4908.yaml > > > > > > Above binding enforces that non-matching compatible strings are not used > > > together. > > > > I just noticed you're actually removing brcm,bcm4908.yaml in the 2/3 so > > you must be aware of that file. > > > > So you see a cleanly working binding in the brcm,bcm4908.yaml but > > instead copying it you decided to wrote your own one from scratch. > > Incorrectly. > > > > This smells of NIH (not invented here). Please just use that binding I > > wrote and move if it needed. > > Not mean to discredit any of your work and I did copy over your binding and > combine them into one SoC entry to the new bcmbca.yaml and add you as one of > the maintainer to this file. As this change would certainly concern you, > that's why I sent RFC first. As I explained in the cover letter, the > purpose of the change is to reduce the number of compatible strings and keep > one entry for one chip family due to possible large number of chip variants. > But since there is no way to validate the combination, I will copy the > existing 4908 bindings as they are now but I would propose to append "brcm, > bcmbca" as it is part of bcmbca chip. And for the other chips, we would just > use enum "board variant", const "main chip id", const "brcm,bca". Does that > sound good to you? If you want fewer combinations of compatibles, adding a genericish "brcm,bcmbca" is not going to help. Is there much value to adding it? What can you do with that information (and nothing else) is the question to ask. Rob
