On 1/16/23 11:51, Frieder Schrempf wrote:
On 16.01.23 11:39, Marek Vasut wrote:
On 1/11/23 08:38, Ahmad Fatoum wrote:
Hello Marek,
Hi,
[...]
Could you share your get_maintainers.pl invocation? I'd like to
adjust the reviewer entry in MAINTAINERS, so such patches get
into our kernel@xxxxxxxxxxxxxx inbox as well.
Plain get_maintainer -f path/to/dts
Some more comments below.
+ pmic: pmic@25 {
+ compatible = "nxp,pca9450c";
+ reg = <0x25>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pmic>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+ sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
Here you assume GPIO1_IO04 is muxed as GPIO.
+ pinctrl_usdhc2: usdhc2-grp {
+ fsl,pins = <
+ MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT 0xc1
Here you mux it for USDHC2_VSELECT though. Is this intended?
That's a good question, other DTs do the same thing, I suspect the
sd-vsel-gpios could be dropped,
The sd-vsel-gpios is only needed when the hardware has an arbitrary GPIO
connected to the SD_VSEL signal of the PCA9450. IMHO, if SD_VSEL is
connected to the VSELECT signal it would be better to drop sd-vsel-gpios.
Current practice does not match this statement however, notice how they
are all gpio 1 4 :
$ git grep sd-vsel-gpios
Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml:
sd-vsel-gpios:
arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi:
sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
arch/arm64/boot/dts/freescale/imx8mp-data-modul-edm-sbc.dts:
sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
arch/arm64/boot/dts/freescale/imx8mp-dhcom-som.dtsi:
sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi:
sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
but as long as it is not outright
harmful, it can be used for backward compatibility to support less
complete OSes which may not handle the eSDHC VSELECT bit , so I figured
it is good to keep both options.
Would be interesting what OSes you have in mind? The eSDHC
driver/hardware should always handle the VSELECT signal, no?
Not necessarily, but the ones I am aware of (U-Boot, Linux) do handle
VSELECT.