Re: [PATCH 2/2] dt-bindings: pwm: mediatek: Add compatible string for MT7986

[Daniel Golle]

Hi Krzysztof,

On Sat, Oct 22, 2022 at 12:35:25PM -0400, Krzysztof Kozlowski wrote:
> On 21/10/2022 18:58, Daniel Golle wrote:
> > On Fri, Oct 21, 2022 at 05:23:38PM -0500, Rob Herring wrote:
> >> On Fri, Oct 21, 2022 at 04:25:18PM +0100, Daniel Golle wrote:
> >>> Add new compatible string for MT7986 PWM.
> >>>
> >>> Signed-off-by: Daniel Golle <daniel@xxxxxxxxxxxxxx>
> >>> ---
> >>>  Documentation/devicetree/bindings/pwm/pwm-mediatek.txt | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
> >>> index 554c96b6d0c3e0..6f4e60c9e18b81 100644
> >>> --- a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
> >>> +++ b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
> >>> @@ -8,6 +8,7 @@ Required properties:
> >>>     - "mediatek,mt7623-pwm": found on mt7623 SoC.
> >>>     - "mediatek,mt7628-pwm": found on mt7628 SoC.
> >>>     - "mediatek,mt7629-pwm": found on mt7629 SoC.
> >>> +   - "mediatek,mt7986-pwm": found on mt7986 SoC.
> >>
> >> This version of the PWM h/w is not compatible with any of the existing 
> >> chips? If it is, it should have a fallback compatible.
> > 
> > No, it is unique because it comes with just 2 PWM channels.
> > Otherwise the driver behaves just like for MT8183 (4 channels) or
> > MT8365 (3 channels) which also got distinct compatible strings.
> 
> Then something would be here compatible. E.g. If you bound MT8183 with
> mt7986-pwm compatible, would you get working device with two channels?

Yes, but I'd see another 2 channels which do not work, accessing them
may even cause problems (I haven't tried that) as it means accessing
an undocumented memory range of the SoC which we in general we
shouldn't be messing around with.

Also note that this case is the same as MT8183 vs. MT8365, they got
distinct compatible strings and also for those two the only difference
is the number of channels.

> 
> If so, they are compatible.

By that definition you should remove the additional compatible for
MT8365 or rather, it should have been rejected for the same argument.

I'm talking about
commit fe00faee8060402a3d85aed95775e16838a6dad2
commit 394b517585da9fbb2eea2f2103ff47d37321e976


Cheers


Daniel