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 ? 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. 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. > > Neil
