On 10/05/2023 09:00, Claudiu.Beznea@xxxxxxxxxxxxx wrote: > On 09.05.2023 09:25, Krzysztof Kozlowski wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> On 09/05/2023 07:27, Claudiu Beznea wrote: >>> Convert Atmel PMC documentation to yaml. Along with it clock names >>> were adapted according to the current available device trees as >>> different controller versions accept different clocks (some of them >>> have 3 clocks as input, some has 2 clocks as inputs and some with 2 >>> input clocks uses different clock names). >>> >> >> Thank you for your patch. There is something to discuss/improve. >> >>> +title: Atmel Power Management Controller (PMC) >>> + >>> +maintainers: >>> + - Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx> >>> + >>> +description: >>> + The power management controller optimizes power consumption by controlling all >>> + system and user peripheral clocks. The PMC enables/disables the clock inputs >>> + to many of the peripherals and to the processor. >>> + >>> +properties: >>> + compatible: >>> + oneOf: >>> + - items: >>> + - enum: >>> + - atmel,at91sam9g15-pmc >>> + - atmel,at91sam9g20-pmc >>> + - atmel,at91sam9g25-pmc >>> + - atmel,at91sam9g35-pmc >>> + - atmel,at91sam9x25-pmc >>> + - atmel,at91sam9x35-pmc >>> + - enum: >>> + - atmel,at91sam9260-pmc >>> + - atmel,at91sam9x5-pmc >> >> I missed it last time - why you have two enums? We never talked about >> this. It's usually wrong... are you sure this is real hardware: >> atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc >> ? > > I have 2 enums because there are some hardware covered by: > "vendor-name,hardware-v1-pmc", "syscon" and some covered by: > "vendor-name,hardware-v2-pmc", "vendor-name,hardware-v1-pmc", "syscon". The enum does not say this. At all. So again, answer, do not ignore: is this valid setup: atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc ? > > Many AT91 device trees compatibles were written in this way. Thus when new > versions of the same IP has been introduced the drivers were not > necessarily updated but the compatibles in device trees were updated e.g. > with "vendor-name,hardware-v2-pmc" (the full compatible becoming > "vendor-name,hardware-v2-pmc", "vendor-name,hardware-v1-pmc", "syscon") and > let the drivers fall back to already in driver supported compatible > "vendor-name,hardware-v1-pmc", "syscon". In general v2 comes with new > features in addition to v1. > > That way they AT91 ensures the ABI properties of DT and thus when the > drivers were finally updated with the new features of the > "vendor-name,hardware-v2-pmc" DT remained in place. > > Please let me know if these could be handled better in YAML. enum + const + syscon, like every binding that type does in all bindings. Don't invent some new syntax. Best regards, Krzysztof