On 10.05.2023 10:06, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > 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 > ? Not w/o syscon. This is valid: compatible = "atmel,at91sam9g20-pmc", "atmel,at91sam9260-pmc", "syscon"; available in arch/arm/boot/dts/at91sam9g20.dtsi +45 > >> >> 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 >
