On 16/06/2023 15:32, Josua Mayer wrote: > HI Krzysztof, > > Am 16.06.23 um 14:36 schrieb Krzysztof Kozlowski: >> On 16/06/2023 13:06, Josua Mayer wrote: >>> Add DT compatible for SolidRun LX2162A SoM and Clearfog board. >>> >>> Signed-off-by: Josua Mayer <josua@xxxxxxxxxxxxx> >>> --- >>> Documentation/devicetree/bindings/arm/fsl.yaml | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml >>> index 15d411084065..438a4ece8157 100644 >>> --- a/Documentation/devicetree/bindings/arm/fsl.yaml >>> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml >>> @@ -1373,9 +1373,11 @@ properties: >>> - description: SolidRun LX2160A based Boards >>> items: >>> - enum: >>> + - solidrun,clearfog >>> - solidrun,clearfog-cx >>> - solidrun,honeycomb >>> - const: solidrun,lx2160a-cex7 >>> + - const: solidrun,lx2162a-som >>> - const: fsl,lx2160a >> You change existing entries, breaking boards and changing the meaning, >> without any explanation in commit msg. That's not how it is done. Please >> provide rationale in commit msg. > > I'm sorry. Given your comment I think I did not understand how these > entries are supposed to work. > So perhaps you can provide some guidance based on my explanation?: > > - NXP LX2162 is a smaller physical package of the same LX2160 SoC, with > reduced IOs and some silicon blocks disabled. > - SolidRun LX2162 SoM is essentially a different form factor of LX2160 CEX > - SolidRun LX2162 Clearfog is the reference platform for the SoM. > Despite it's naming similarity to clearfog-cx, it has a different > feature set more similar to SolidRun Armada 388 Clearfog Pro > > So I believed I could just add to the existing entry "SolidRun LX2160A > based Boards" also the new LX2162 Board & SoM. But you added much more, didn't you? > I see now that adding a fourth const messes upthe existing 3-part > compatible for those already existing boards. > > Please can you confirm if it would have been more correct to replace > "const: solidrun,lx2160a-cex7" with an enum?: > enum: > - solidrun,lx2160a-cex7 > - solidrun,lx2162a-som > > Finally, is it okay to add a "solidrun,clearfog" given my explanation > above, or should it be more specific "solidrun,lx2162a-clearfog"? > Test the binding and test DTS against it: Please run `make dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ It might point you to answer. Why do you make solidrun,honeycomb compatible with cex7 and som? Best regards, Krzysztof
