Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 23 Jan 2024 16:28:34 +0800 | From | Junyi Zhao <> | Subject | Re: [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
| ![\](/images/icornerr.gif) |