lkml.org 
[lkml]   [2024]   [Jan]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 1/6] dt-bindings: pwm: amlogic: fix s4 bindings


On 2024/1/17 18:30, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On Wed 17 Jan 2024 at 11:03, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
>
>> [[PGP Signed Part:Undecided]]
>> On Fri, Dec 22, 2023 at 12:16:49PM +0100, Jerome Brunet wrote:
>>> s4 has been added to the compatible list while converting the Amlogic PWM
>>> binding documentation from txt to yaml.
>>>
>>> However, on the s4, the clock bindings have different meaning compared to
>>> the previous SoCs.
>>>
>>> On the previous SoCs the clock bindings used to describe which input the
>>> PWM channel multiplexer should pick among its possible parents.
>>>
>>> This is very much tied to the driver implementation, instead of describing
>>> the HW for what it is. When support for the Amlogic PWM was first added,
>>> how to deal with clocks through DT was not as clear as it nowadays.
>>> The Linux driver now ignores this DT setting, but still relies on the
>>> hard-coded list of clock sources.
>>>
>>> On the s4, the input multiplexer is gone. The clock bindings actually
>>> describe the clock as it exists, not a setting. The property has a
>>> different meaning, even if it is still 2 clocks and it would pass the check
>>> when support is actually added.
>>>
>>> Also the s4 cannot work if the clocks are not provided, so the property no
>>> longer optional.
>>
>> s/no/is no/
>>
>>> Finally, for once it makes sense to see the input as being numbered
>>> somehow. No need to bother with clock-names on the s4 type of PWM.
>>>
>>> Fixes: 43a1c4ff3977 ("dt-bindings: pwm: Convert Amlogic Meson PWM binding")
>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>>> ---
>>> .../devicetree/bindings/pwm/pwm-amlogic.yaml | 67 ++++++++++++++++---
>>> 1 file changed, 58 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml b/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
>>> index 527864a4d855..a1d382aacb82 100644
>>> --- a/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
>>> +++ b/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
>>> @@ -9,9 +9,6 @@ title: Amlogic PWM
>>> maintainers:
>>> - Heiner Kallweit <hkallweit1@gmail.com>
>>>
>>> -allOf:
>>> - - $ref: pwm.yaml#
>>> -
>>> properties:
>>> compatible:
>>> oneOf:
>>> @@ -43,12 +40,8 @@ properties:
>>> maxItems: 2
>>>
>>> clock-names:
>>> - oneOf:
>>> - - items:
>>> - - enum: [clkin0, clkin1]
>>> - - items:
>>> - - const: clkin0
>>> - - const: clkin1
>>> + minItems: 1
>>> + maxItems: 2
>>>
>>> "#pwm-cells":
>>> const: 3
>>> @@ -57,6 +50,55 @@ required:
>>> - compatible
>>> - reg
>>>
>>> +allOf:
>>> + - $ref: pwm.yaml#
>>> +
>>> + - if:
>>> + properties:
>>> + compatible:
>>> + contains:
>>> + enum:
>>> + - amlogic,meson8-pwm
>>> + - amlogic,meson8b-pwm
>>> + - amlogic,meson-gxbb-pwm
>>> + - amlogic,meson-gxbb-ao-pwm
>>> + - amlogic,meson-axg-ee-pwm
>>> + - amlogic,meson-axg-ao-pwm
>>> + - amlogic,meson-g12a-ee-pwm
>>> + - amlogic,meson-g12a-ao-pwm-ab
>>> + - amlogic,meson-g12a-ao-pwm-cd
>>> + then:
>>> + # Historic bindings tied to the driver implementation
>>> + # The clocks provided here are meant to be matched with the input
>>> + # known (hard-coded) in the driver and used to select pwm clock
>>> + # source. Currently, the linux driver ignores this.
>>
>> I admit I didn't understand the relevant difference between the old and
>> the new binding yet.
>
> Let's try to explain differently then:
> * So far each AML PWM IP has 2 pwm.
> * Up to G12, 4 input PLL/clocks are wired to the HW IP and there
> mux/div/gate to select which input to take.
> - The historic bindings just described how to setup each of the 2
> internal muxes - 2 optionnal clocks.
> The actual 4 inputs names from CCF are hard coded in
> the driver. This is a pretty bad description. The driver has been
> updated since then to use CCF to figure out the best parent
> according to pwm rate so this setting is ignored now.
> - The 'new' bindings (introduced in patch #2) fixes the problem above
> but the meaning of the clock binding is different. It describes the
> actual HW parents - 4 clocks, optionnal since some are not wired on
> some PWM blocks. To avoid breaking the ABI, a new compatible for
> these SoC is introduced.
>
>> (The driver currently doesn't support the s4, right?)
>
> Indeed. I know Amlogic is preparing the support. I could do it in this
> series but I prefer to encourage them to contribute. It should come
> shortly after this series is merged.
Yes. Amlogic is waiting these patches of Jerome. after these merge, we
will start to rebase and submit s4 code.
-
Best Regards Junyi

\
 
 \ /
  Last update: 2024-05-27 14:29    [W:0.052 / U:1.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site