Re: [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jerome,

On 20/11/2023 10:18, Jerome Brunet wrote:

On Mon 20 Nov 2023 at 09:27, Neil Armstrong <neil.armstrong@xxxxxxxxxx> wrote:

Hi Rob,

On 19/11/2023 17:05, Rob Herring wrote:
On Fri, 17 Nov 2023 13:59:12 +0100, Jerome Brunet wrote:
Add a new compatible for the pwm found in the meson8 to sm1 Amlogic SoCs.

The previous clock bindings for these SoCs described the driver and not the
HW itself. The clock provided was used to set the parent of the input clock
mux among the possible parents hard-coded in the driver.

The new bindings allows to describe the actual clock inputs of the PWM in
DT, like most bindings do, instead of relying of hard-coded data.

The new bindings make the old one deprecated.

There is enough experience on this HW to know that the PWM is exactly the
same all the supported SoCs. There is no need for a per-SoC compatible.

Signed-off-by: Jerome Brunet <jbrunet@xxxxxxxxxxxx>
---
   .../devicetree/bindings/pwm/pwm-amlogic.yaml  | 36 +++++++++++++++++--
   1 file changed, 34 insertions(+), 2 deletions(-)

Reviewed-by: Rob Herring <robh@xxxxxxxxxx>


I'm puzzled, isn't it recommended to have a per-soc compatible now ?

I have specifically addressed this matter in the description,
haven't I ? What good would it do in this case ?

Yes you did but I was asked for the last year+ that all new compatible
should be soc specific (while imprecise, in our care soc family should be ok),
with a possible semi-generic callback with an IP version or a first soc
implementing the IP.


Plus the definition of a SoC is very vague. One could argue that
the content of the list bellow are vaguely defined families. Should we
add meson8b, gxl, gxm, sm1 ? ... or even the actual SoC reference ?
This list gets huge for no reason.

I think in our case soc family is reasonable since they share same silicon
design.


We know all existing PWM of this type are the same. We have been using
them for years. It is not a new support we know nothing about.


I thought something like:
- items:
     - enum:
         - amlogic,gxbb-pwm
         - amlogic,axg-pwm
         - amlogic,g12a-pwm
     - const: amlogic,pwm-v1

I'm not sure I understand what you are suggesting here.
Adding a "amlogic,pwm-v1" for the obsolete compatible ? No amlogic DT
has that and I'm working to remove this type, so I don't get the point.


should be preferred instead of a single amlogic,meson8-pwm-v2 ?

This is named after the first SoC supporting the type.

Naming it amlogic,pwm-v2 would feel weird with the s4 coming after.
Plus the doc specifically advise against this type of names.

The -v2 refers to a pure software/dt implementation versioning and not
an HW version, so I'm puzzled and I requires DT maintainers advice here.

Yes meson8b is the first "known" platform, even if I'm pretty sure meson6 has
the same pwm architecture, this is why "amlogic,pwm-v1" as fallback seems more
reasonable and s4 and later pwm could use the "amlogic,pwm-v2" fallback.

Neil


Neil






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux