Hello, On Mon, Jun 03, 2024 at 05:42:08PM +0900, Hironori KIKUCHI wrote: > 2024年6月3日(月) 9:10 Andre Przywara <andre.przywara@xxxxxxx>: > > > > On Sun, 2 Jun 2024 15:15:13 +0900 > > Hironori KIKUCHI <kikuchan98@xxxxxxxxx> wrote: > > > > > > On 31/05/2024 19:57, Hironori KIKUCHI wrote: > > > > >>> --- > > > > >>> .../bindings/pwm/allwinner,sun20i-pwm.yaml | 19 +++++++++++++++++++ > > > > >>> 1 file changed, 19 insertions(+) > > > > >>> > > > > >>> diff --git a/Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml b/Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml > > > > >>> index b9b6d7e7c87..436a1d344ab 100644 > > > > >>> --- a/Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml > > > > >>> +++ b/Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml > > > > >>> @@ -45,6 +45,25 @@ properties: > > > > >>> description: The number of PWM channels configured for this instance > > > > >>> enum: [6, 9] > > > > >>> > > > > >>> + allwinner,pwm-pair-clock-sources: > > > > >>> + description: The clock source names for each PWM pair > > > > >>> + items: > > > > >>> + enum: [hosc, apb] > > > > >>> + minItems: 1 > > > > >>> + maxItems: 8 > > > > >> > > > > >> Missing type... and add 8 of such items to your example to make it complete. > > > > > > > > > > Thank you. I'll fix it. > > > > > > > > > >> > > > > >>> + > > > > >>> + allwinner,pwm-pair-clock-prescales: > > > > >>> + description: The prescale (DIV_M register) values for each PWM pair > > > > >>> + $ref: /schemas/types.yaml#/definitions/uint32-matrix > > > > >>> + items: > > > > >>> + items: > > > > >>> + minimum: 0 > > > > >>> + maximum: 8 > > > > >>> + minItems: 1 > > > > >>> + maxItems: 1 > > > > >>> + minItems: 1 > > > > >>> + maxItems: 8 > > > > >> > > > > >> This does not look like matrix but array. > > > > > > > > > > I wanted to specify values like this: > > > > > > > > > > allwinner,pwm-pair-clock-prescales = <0>, <1>, <3>; > > > > > allwinner,pwm-pair-clock-sources = "hosc", "apb", "hosc": > > > > > > > > > > These should correspond to each PWM pair. > > > > > This way, I thought we might be able to visually understand the relationship > > > > > between prescalers and sources, like clock-names and clocks. > > > > > > > > > > Is this notation uncommon, perhaps? > > > > > > > > It's still an array. > > > > > > Oh I understood and clear. Thank you. For clocks there is already a binding to assign a working configuration. assigned-clocks, assigned-clock-rates and assigned-clock-parents are the relevant properties. If you create a clk from the parent clock selector and mdiv, you can stick to the existing bindings. > > > [...] > > > > So I understand the problem, but I don't think expressing this in the > > devicetree is the right solution. It seems like a tempting pragmatical > > approach, but it sounds like the wrong way: this is not a hardware > > *description* of any kind, but rather a way to describe a certain user > > intention or a configuration. So this looks like a rather embedded > > approach to me, where you have a certain fixed register setup in mind, > > and want to somehow force this to the hardware. > > Another problem with this approach is that it doesn't really cover the > > sysfs interface, which is very dynamic by nature. > > ... Indeed. You're right. > Now I've realized it was a bad idea. > It should be done in sysfs or sysctl perhaps. I don't think a driver specific knob somewhere is a practical solution. Best regards Uwe
Attachment:
signature.asc
Description: PGP signature
