Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 28 Sep 2022 13:59:25 +0200
From:   Thierry Reding <thierry.reding@gmail.com>
To:     Johan Jonker <jbx6244@gmail.com>
Cc:     Rob Herring <robh@kernel.org>, u.kleine-koenig@pengutronix.de,
        linux-rockchip@lists.infradead.org, philipp.tomsich@vrull.eu,
        linux-arm-kernel@lists.infradead.org,
        krzysztof.kozlowski+dt@linaro.org, devicetree@vger.kernel.org,
        linux-pwm@vger.kernel.org, kever.yang@rock-chips.com,
        zhangqing@rock-chips.com, linux-kernel@vger.kernel.org,
        heiko@sntech.de
Subject: Re: [PATCH v1 03/11] dt-bindings: pwm: rockchip: add
 rockchip,rk3128-pwm
Message-ID: <YzQ3He2wyD2bgxz1@orome>
References: <20220909212543.17428-1-jbx6244@gmail.com>
 <f5dd0ee4-d97e-d878-ffde-c06e9b233e38@gmail.com>
 <1662821635.180247.34700.nullmailer@robh.at.kernel.org>
 <1c13181b-8421-69d8-21ee-9742dd5f55dd@gmail.com>
 <20220912162159.GA1397560-robh@kernel.org>
 <37fd8d4b-3a66-bc51-c2dc-76c9e756fed8@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="znR/AMl6swXutvbU"
Content-Disposition: inline
In-Reply-To: <37fd8d4b-3a66-bc51-c2dc-76c9e756fed8@gmail.com>
User-Agent: Mutt/2.2.7 (2022-08-07)
Precedence: bulk


--znR/AMl6swXutvbU
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Sep 13, 2022 at 04:38:32PM +0200, Johan Jonker wrote:
>=20
>=20
> On 9/12/22 18:21, Rob Herring wrote:
> > On Sat, Sep 10, 2022 at 09:48:04PM +0200, Johan Jonker wrote:
> >> Reduced CC.
> >>
> >> Hi Rob,
> >>
> >=20
> > Seemed like a simple enough warning to fix...
>=20
> Some examples for comment.
> Let us know what would be the better solution?
>=20
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D
>=20
> option1:
>=20
> 	combpwm0: combpwm0 {
> 		compatible =3D "rockchip,rv1108-combpwm";
> 		interrupts =3D <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
> 		#address-cells =3D <2>;
> 		#size-cells =3D <2>;
>=20
> 		pwm0: pwm@20040000 {
> 			compatible =3D "rockchip,rv1108-pwm";
> 			reg =3D <0x20040000 0x10>;
> 		};
>=20
> 		pwm1: pwm@20040010 {
> 			compatible =3D "rockchip,rv1108-pwm";
> 			reg =3D <0x20040010 0x10>;
> 		};
>=20
> 		pwm2: pwm@20040020 {
> 			compatible =3D "rockchip,rv1108-pwm";
> 			reg =3D <0x20040020 0x10>;
> 		};
>=20
> 		pwm3: pwm@20040030 {
> 			compatible =3D "rockchip,rv1108-pwm";
> 			reg =3D <0x20040030 0x10>;
> 		};
> 	};
>=20
> PRO:
> - Existing driver might still work.
> CON:
> - New compatible needed to service the combined interrupts.
> - Driver change needed.
>=20
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D
> option 2:
>=20
> 	combpwm0: pwm@10280000 {
> 		compatible =3D "rockchip,rv1108-pwm";
> 		reg =3D <0x10280000 0x40>;
> 		interrupts =3D <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
> 		#address-cells =3D <1>;
> 		#size-cells =3D <0>;
>=20
> 		pwm4: pwm-4@0 {
> 			reg =3D <0x0>;
> 		};
>=20
> 		pwm5: pwm-5@10 {
> 			reg =3D <0x10>;
> 		};
>=20
> 		pwm6: pwm-6@20 {
> 			reg =3D <0x20>;
> 		};
>=20
> 		pwm7: pwm-7@30 {
> 			reg =3D <0x30>;
> 		};
> 	};
>=20
> CON:
> - Driver change needed.
> - Not compatible with current drivers.
>=20
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D
>=20
> Current situation:
>=20
> 	pwm0: pwm@20040000 {
> 		compatible =3D "rockchip,rv1108-pwm", "rockchip,rk3288-pwm";
> 		reg =3D <0x20040000 0x10>;
> 		interrupts =3D <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
> 	};
>=20
> 	pwm1: pwm@20040010 {
> 		compatible =3D "rockchip,rv1108-pwm", "rockchip,rk3288-pwm";
> 		reg =3D <0x20040010 0x10>;
> 		interrupts =3D <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
> 	};
>=20
> 	pwm2: pwm@20040020 {
> 		compatible =3D "rockchip,rv1108-pwm", "rockchip,rk3288-pwm";
> 		reg =3D <0x20040020 0x10>;
> 		interrupts =3D <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
> 	};
>=20
> 	pwm3: pwm@20040030 {
> 		compatible =3D "rockchip,rv1108-pwm", "rockchip,rk3288-pwm";
> 		reg =3D <0x20040030 0x10>;
> 		interrupts =3D <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
> 	};
>=20
> CON:
> - The property "interrupts 39" can only be claimed ones by one probe func=
tion at the time.
> - Has a fall-back string for rk3288, but unknown identical behavior for i=
nterrupts ???

To be honest, all three descriptions look wrong to me. From the above it
looks like this is simply one PWM controller with four channels, so it
should really be described as such, i.e.:

	pwm@20040030 {
		compatible =3D "rockchip,rv1108-pwm";
		reg =3D <0x20040030 0x40>;
		interrupts =3D <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
	};

Looking through existing Rockchip SoC DTSI files, though, it looks like
this has been done the wrong way since the beginning, so not sure if you
still want to fix it up.

This whole problem of dealing with a shared interrupt wouldn't be a
problem if this was described properly.

Thierry

--znR/AMl6swXutvbU
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAmM0NxoACgkQ3SOs138+
s6F2DQ//SNItqjUD13hctOIu27Mgc1QlCOleIYcIh+Tvc1QSG838jZzQPNzG2+Ft
ZuWfVO3UfsF3Ny3OA+GvfTEJTCj5bZAFKb/HkoDlmAXDeYqteLREoO/jo8eVDPYx
Yyqs1l1UxpnJJIurdJtYWwyMVlzxXkVHH6+ZCidaf3oUvWCsrfFMbo/6V7YZmasU
22bjdfqqWexG86xbVsML6y2WNzMLiBlWxGwRPBEkOXwyJ7AcEk0KV7RiENxOERs7
zwI6rePM0V9CDUStIcO1mlhljgk/6h64v+yNFlTiDwgG3GjBSQGw5M0JyuvzYzWF
dj+62s4HnAwYWWldLJpR0RooYiW3GMXUHizY+KnevRcaLhMRwe/D4PFmZyuxumzN
ywnSDS6yIGN3DGKmz8/+YjvLLitmCvCzffNNlAMXnz29pz7RlirRhfCJylsPMs4R
VxCZFX6P7saeLzJn0jNBbZck2KcMWFK+uZJQ7hezYt81Z1vdRahymS6SelDlbQEA
fAkU1tudRbu062RNVnI9okSaLafrXVUdnhWdxuD+hCJWLv/2DYCKuEHmTXMTkCo5
GLtjPULz1Cz/GyHYBAAcKPXp/VcTVMm3sUXwHYlaJ67YyxFpZgQGfLej5y1+oJfU
cSFQu91Ek+ZogUFWckR/+oA0tq6VrZqG52O0tj8iMsS7JoB/Cz8=
=s0oH
-----END PGP SIGNATURE-----

--znR/AMl6swXutvbU--