2024-04-04 16:19:47

by Frank Li

[permalink] [raw]
Subject: [PATCH 1/1] arm64: dts: imx8qxp-mek: add cm40_i2c, wm8960/wm8962 and sai[0,1,4,5]

imx8qxp-mek use two kind audio codec, wm8960 and wm8962. Using dummy gpio
i2c bus mux to connect both i2c devices. One will probe failure and other
will probe success when devices driver check whoami. So one dtb can cover
both board configuration.

Signed-off-by: Frank Li <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 210 ++++++++++++++++++
1 file changed, 210 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
index 8360bb851ac03..adff87c7cf305 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
+++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
@@ -30,6 +30,13 @@ reg_usdhc2_vmmc: usdhc2-vmmc {
enable-active-high;
};

+ reg_audio: regulator-wm8962 {
+ compatible = "regulator-fixed";
+ regulator-name = "3v3_aud";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ };
+
gpio-sbu-mux {
compatible = "nxp,cbdtu02043", "gpio-sbu-mux";
pinctrl-names = "default";
@@ -44,6 +51,105 @@ usb3_data_ss: endpoint {
};
};
};
+
+ sound-wm8960 {
+ compatible = "fsl,imx-audio-wm8960";
+ model = "wm8960-audio";
+ audio-cpu = <&sai1>;
+ audio-codec = <&wm8960>;
+ hp-det-gpio = <&lsio_gpio1 0 GPIO_ACTIVE_HIGH>;
+ audio-routing =
+ "Headphone Jack", "HP_L",
+ "Headphone Jack", "HP_R",
+ "Ext Spk", "SPK_LP",
+ "Ext Spk", "SPK_LN",
+ "Ext Spk", "SPK_RP",
+ "Ext Spk", "SPK_RN",
+ "LINPUT1", "Mic Jack",
+ "Mic Jack", "MICB";
+ };
+
+ sound-wm8962 {
+ compatible = "fsl,imx-audio-wm8962";
+ model = "wm8962-audio";
+ audio-cpu = <&sai1>;
+ audio-codec = <&wm8962>;
+ hp-det-gpio = <&lsio_gpio1 0 GPIO_ACTIVE_HIGH>;
+ audio-routing =
+ "Headphone Jack", "HPOUTL",
+ "Headphone Jack", "HPOUTR",
+ "Ext Spk", "SPKOUTL",
+ "Ext Spk", "SPKOUTR",
+ "AMIC", "MICBIAS",
+ "IN3R", "AMIC",
+ "IN1R", "AMIC";
+ };
+
+ /*
+ * This dummy i2c mux. GPIO actually will not impact selection. At actual boards, only 1
+ * device connectted. I2C client driver will check ID when probe. Only matched ID's driver
+ * probe successfully.
+ */
+ i2cvmux: i2cmux {
+ compatible = "i2c-mux-gpio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ mux-gpios = <&lsio_gpio5 0 GPIO_ACTIVE_HIGH>; /* use an unused gpio */
+ i2c-parent = <&cm40_i2c>;
+
+ i2c@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ /* WCPU boards SCH-54536 */
+ wm8962: wm8962@1a {
+ compatible = "wlf,wm8962";
+ reg = <0x1a>;
+ clocks = <&mclkout0_lpcg IMX_LPCG_CLK_0>;
+ assigned-clocks = <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_PLL>,
+ <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_SLV_BUS>,
+ <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_MST_BUS>,
+ <&mclkout0_lpcg IMX_LPCG_CLK_0>;
+ assigned-clock-rates = <786432000>,
+ <49152000>,
+ <12288000>,
+ <12288000>;
+ DCVDD-supply = <&reg_audio>;
+ DBVDD-supply = <&reg_audio>;
+ AVDD-supply = <&reg_audio>;
+ CPVDD-supply = <&reg_audio>;
+ MICVDD-supply = <&reg_audio>;
+ PLLVDD-supply = <&reg_audio>;
+ SPKVDD1-supply = <&reg_audio>;
+ SPKVDD2-supply = <&reg_audio>;
+ };
+ };
+
+ i2c@1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ wm8960: wm8960@1a {
+ compatible = "wlf,wm8960";
+ reg = <0x1a>;
+ clocks = <&mclkout0_lpcg IMX_LPCG_CLK_0>;
+ clock-names = "mclk";
+ wlf,shared-lrclk;
+ wlf,hp-cfg = <2 2 3>;
+ wlf,gpio-cfg = <1 3>;
+ assigned-clocks = <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_PLL>,
+ <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_SLV_BUS>,
+ <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_MST_BUS>,
+ <&mclkout0_lpcg IMX_LPCG_CLK_0>;
+ assigned-clock-rates = <786432000>,
+ <49152000>,
+ <12288000>,
+ <12288000>;
+ };
+ };
+ };
};

&dsp {
@@ -188,6 +294,29 @@ typec_con_ss: endpoint {

};

+&cm40_i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ clock-frequency = <100000>;
+ pinctrl-names = "default", "gpio";
+ pinctrl-0 = <&pinctrl_cm40_i2c>;
+ pinctrl-1 = <&pinctrl_cm40_i2c_gpio>;
+ scl-gpios = <&lsio_gpio1 10 GPIO_ACTIVE_HIGH>;
+ sda-gpios = <&lsio_gpio1 9 GPIO_ACTIVE_HIGH>;
+ status = "okay";
+
+ pca6416: gpio@20 {
+ compatible = "ti,tca6416";
+ reg = <0x20>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+};
+
+&cm40_intmux {
+ status = "okay";
+};
+
&lpuart0 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_lpuart0>;
@@ -218,6 +347,53 @@ &scu_key {
status = "okay";
};

+&sai0 {
+ #sound-dai-cells = <0>;
+ assigned-clocks = <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_PLL>,
+ <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_SLV_BUS>,
+ <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_MST_BUS>,
+ <&sai0_lpcg IMX_LPCG_CLK_0>;
+ assigned-clock-rates = <786432000>, <49152000>, <12288000>, <49152000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_sai0>;
+ status = "okay";
+};
+
+&sai1 {
+ assigned-clocks = <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_PLL>,
+ <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_SLV_BUS>,
+ <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_MST_BUS>,
+ <&sai1_lpcg IMX_LPCG_CLK_0>;
+ assigned-clock-rates = <786432000>, <49152000>, <12288000>, <49152000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_sai1>;
+ status = "okay";
+};
+
+&sai4 {
+ assigned-clocks = <&acm IMX_ADMA_ACM_SAI4_MCLK_SEL>,
+ <&clk IMX_SC_R_AUDIO_PLL_1 IMX_SC_PM_CLK_PLL>,
+ <&clk IMX_SC_R_AUDIO_PLL_1 IMX_SC_PM_CLK_SLV_BUS>,
+ <&clk IMX_SC_R_AUDIO_PLL_1 IMX_SC_PM_CLK_MST_BUS>,
+ <&sai4_lpcg IMX_LPCG_CLK_0>;
+ assigned-clock-parents = <&aud_pll_div1_lpcg IMX_LPCG_CLK_0>;
+ assigned-clock-rates = <0>, <786432000>, <98304000>, <12288000>, <98304000>;
+ fsl,sai-asynchronous;
+ status = "okay";
+};
+
+&sai5 {
+ assigned-clocks = <&acm IMX_ADMA_ACM_SAI5_MCLK_SEL>,
+ <&clk IMX_SC_R_AUDIO_PLL_1 IMX_SC_PM_CLK_PLL>,
+ <&clk IMX_SC_R_AUDIO_PLL_1 IMX_SC_PM_CLK_SLV_BUS>,
+ <&clk IMX_SC_R_AUDIO_PLL_1 IMX_SC_PM_CLK_MST_BUS>,
+ <&sai5_lpcg IMX_LPCG_CLK_0>;
+ assigned-clock-parents = <&aud_pll_div1_lpcg IMX_LPCG_CLK_0>;
+ assigned-clock-rates = <0>, <786432000>, <98304000>, <12288000>, <98304000>;
+ fsl,sai-asynchronous;
+ status = "okay";
+};
+
&thermal_zones {
pmic-thermal {
polling-delay-passive = <250>;
@@ -314,6 +490,21 @@ &vpu_core1 {
};

&iomuxc {
+
+ pinctrl_cm40_i2c: cm40i2cgrp {
+ fsl,pins = <
+ IMX8QXP_ADC_IN1_M40_I2C0_SDA 0x0600004c
+ IMX8QXP_ADC_IN0_M40_I2C0_SCL 0x0600004c
+ >;
+ };
+
+ pinctrl_cm40_i2c_gpio: cm40i2cgpio-grp {
+ fsl,pins = <
+ IMX8QXP_ADC_IN1_LSIO_GPIO1_IO09 0xc600004c
+ IMX8QXP_ADC_IN0_LSIO_GPIO1_IO10 0xc600004c
+ >;
+ };
+
pinctrl_fec1: fec1grp {
fsl,pins = <
IMX8QXP_ENET0_MDC_CONN_ENET0_MDC 0x06000020
@@ -385,6 +576,25 @@ IMX8QXP_ENET0_REFCLK_125M_25M_LSIO_GPIO5_IO09 0x60
>;
};

+ pinctrl_sai0: sai0grp {
+ fsl,pins = <
+ IMX8QXP_SAI0_TXD_ADMA_SAI0_TXD 0x06000060
+ IMX8QXP_SAI0_RXD_ADMA_SAI0_RXD 0x06000040
+ IMX8QXP_SAI0_TXC_ADMA_SAI0_TXC 0x06000040
+ IMX8QXP_SAI0_TXFS_ADMA_SAI0_TXFS 0x06000040
+ >;
+ };
+
+ pinctrl_sai1: sai1grp {
+ fsl,pins = <
+ IMX8QXP_SAI1_RXD_ADMA_SAI1_RXD 0x06000040
+ IMX8QXP_SAI1_RXC_ADMA_SAI1_TXC 0x06000040
+ IMX8QXP_SAI1_RXFS_ADMA_SAI1_TXFS 0x06000040
+ IMX8QXP_SPI0_CS1_ADMA_SAI1_TXD 0x06000060
+ IMX8QXP_SPI2_CS0_LSIO_GPIO1_IO00 0x06000040
+ >;
+ };
+
pinctrl_usdhc1: usdhc1grp {
fsl,pins = <
IMX8QXP_EMMC0_CLK_CONN_EMMC0_CLK 0x06000041
--
2.34.1



2024-04-05 06:37:21

by Francesco Dolcini

[permalink] [raw]
Subject: Re: [PATCH 1/1] arm64: dts: imx8qxp-mek: add cm40_i2c, wm8960/wm8962 and sai[0,1,4,5]

Hello Frank,

On Thu, Apr 04, 2024 at 12:19:13PM -0400, Frank Li wrote:
> imx8qxp-mek use two kind audio codec, wm8960 and wm8962. Using dummy gpio
> i2c bus mux to connect both i2c devices. One will probe failure and other
> will probe success when devices driver check whoami. So one dtb can cover
> both board configuration.
>
> Signed-off-by: Frank Li <[email protected]>
> ---
> arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 210 ++++++++++++++++++
> 1 file changed, 210 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> index 8360bb851ac03..adff87c7cf305 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> @@ -44,6 +51,105 @@ usb3_data_ss: endpoint {

[...]

> + /*
> + * This dummy i2c mux. GPIO actually will not impact selection. At actual boards, only 1
> + * device connectted. I2C client driver will check ID when probe. Only matched ID's driver
> + * probe successfully.
> + */
> + i2cvmux: i2cmux {
> + compatible = "i2c-mux-gpio";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + mux-gpios = <&lsio_gpio5 0 GPIO_ACTIVE_HIGH>; /* use an unused gpio */

There is for sure people that have more experience and competency that
me and it would be interesting to hear their feedback, but this
looks like a bad hack, and you are just playing with the driver
behavior to ensure that you get what you need.

Francesco



2024-04-05 06:45:49

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/1] arm64: dts: imx8qxp-mek: add cm40_i2c, wm8960/wm8962 and sai[0,1,4,5]

On 04/04/2024 18:19, Frank Li wrote:
> imx8qxp-mek use two kind audio codec, wm8960 and wm8962. Using dummy gpio
> i2c bus mux to connect both i2c devices. One will probe failure and other
> will probe success when devices driver check whoami. So one dtb can cover
> both board configuration.

I don't understand it. Either you add real device or not. If one board
has two devices, then why do you need to check for failures?

Anyway, don't add fake stuff to DTS.

NAK.
>
> Signed-off-by: Frank Li <[email protected]>
> ---
> arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 210 ++++++++++++++++++
> 1 file changed, 210 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> index 8360bb851ac03..adff87c7cf305 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> @@ -30,6 +30,13 @@ reg_usdhc2_vmmc: usdhc2-vmmc {
> enable-active-high;
> };
>
> + reg_audio: regulator-wm8962 {
> + compatible = "regulator-fixed";
> + regulator-name = "3v3_aud";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + };
> +
> gpio-sbu-mux {
> compatible = "nxp,cbdtu02043", "gpio-sbu-mux";
> pinctrl-names = "default";
> @@ -44,6 +51,105 @@ usb3_data_ss: endpoint {
> };
> };
> };
> +
> + sound-wm8960 {
> + compatible = "fsl,imx-audio-wm8960";
> + model = "wm8960-audio";
> + audio-cpu = <&sai1>;
> + audio-codec = <&wm8960>;
> + hp-det-gpio = <&lsio_gpio1 0 GPIO_ACTIVE_HIGH>;
> + audio-routing =
> + "Headphone Jack", "HP_L",
> + "Headphone Jack", "HP_R",
> + "Ext Spk", "SPK_LP",
> + "Ext Spk", "SPK_LN",
> + "Ext Spk", "SPK_RP",
> + "Ext Spk", "SPK_RN",
> + "LINPUT1", "Mic Jack",
> + "Mic Jack", "MICB";
> + };
> +
> + sound-wm8962 {
> + compatible = "fsl,imx-audio-wm8962";
> + model = "wm8962-audio";
> + audio-cpu = <&sai1>;
> + audio-codec = <&wm8962>;
> + hp-det-gpio = <&lsio_gpio1 0 GPIO_ACTIVE_HIGH>;
> + audio-routing =
> + "Headphone Jack", "HPOUTL",
> + "Headphone Jack", "HPOUTR",
> + "Ext Spk", "SPKOUTL",
> + "Ext Spk", "SPKOUTR",
> + "AMIC", "MICBIAS",
> + "IN3R", "AMIC",
> + "IN1R", "AMIC";
> + };
> +
> + /*
> + * This dummy i2c mux. GPIO actually will not impact selection. At actual boards, only 1
> + * device connectted. I2C client driver will check ID when probe. Only matched ID's driver
> + * probe successfully.

NAK


Best regards,
Krzysztof


2024-04-05 14:47:31

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH 1/1] arm64: dts: imx8qxp-mek: add cm40_i2c, wm8960/wm8962 and sai[0,1,4,5]

On Fri, Apr 05, 2024 at 08:41:59AM +0200, Krzysztof Kozlowski wrote:
> On 04/04/2024 18:19, Frank Li wrote:
> > imx8qxp-mek use two kind audio codec, wm8960 and wm8962. Using dummy gpio
> > i2c bus mux to connect both i2c devices. One will probe failure and other
> > will probe success when devices driver check whoami. So one dtb can cover
> > both board configuration.
>
> I don't understand it. Either you add real device or not. If one board
> has two devices, then why do you need to check for failures?
>
> Anyway, don't add fake stuff to DTS.

NAK can't resolve the problem. It should be common problem for long time
cycle boards. Some chipes will be out life cycle. such as some sensor. So
chips on boards have been replace by some pin to pin compatible sensor. For
example:
old boards: use sensor A with address 0x1a
new bench: use sensor B with address 0x1b.

You can treat it as two kind boards, RevA or RevB. But most user want to
use one dtb to handle such small differences. For this case, it should be
simple. Just add a super set.
i2c
{
sensorA@1a
{
}
sensorB@1b
{
}
}

It also depend on whoami check by i2c devices. Only A or B will probe.

wm8960 and wm8962 are more complex example. wm8960 is out of life. But
wm8962 and wm8960 have the same i2c address. The current i2c frame can't
allow the same i2c address in one i2c bus.

You are feel to NAK my method, but I hope you also provide constructive
solution to help resolve the problem.

Frank

>
> NAK.
> >
> > Signed-off-by: Frank Li <[email protected]>
> > ---
> > arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 210 ++++++++++++++++++
> > 1 file changed, 210 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> > index 8360bb851ac03..adff87c7cf305 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> > @@ -30,6 +30,13 @@ reg_usdhc2_vmmc: usdhc2-vmmc {
> > enable-active-high;
> > };
> >
> > + reg_audio: regulator-wm8962 {
> > + compatible = "regulator-fixed";
> > + regulator-name = "3v3_aud";
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + };
> > +
> > gpio-sbu-mux {
> > compatible = "nxp,cbdtu02043", "gpio-sbu-mux";
> > pinctrl-names = "default";
> > @@ -44,6 +51,105 @@ usb3_data_ss: endpoint {
> > };
> > };
> > };
> > +
> > + sound-wm8960 {
> > + compatible = "fsl,imx-audio-wm8960";
> > + model = "wm8960-audio";
> > + audio-cpu = <&sai1>;
> > + audio-codec = <&wm8960>;
> > + hp-det-gpio = <&lsio_gpio1 0 GPIO_ACTIVE_HIGH>;
> > + audio-routing =
> > + "Headphone Jack", "HP_L",
> > + "Headphone Jack", "HP_R",
> > + "Ext Spk", "SPK_LP",
> > + "Ext Spk", "SPK_LN",
> > + "Ext Spk", "SPK_RP",
> > + "Ext Spk", "SPK_RN",
> > + "LINPUT1", "Mic Jack",
> > + "Mic Jack", "MICB";
> > + };
> > +
> > + sound-wm8962 {
> > + compatible = "fsl,imx-audio-wm8962";
> > + model = "wm8962-audio";
> > + audio-cpu = <&sai1>;
> > + audio-codec = <&wm8962>;
> > + hp-det-gpio = <&lsio_gpio1 0 GPIO_ACTIVE_HIGH>;
> > + audio-routing =
> > + "Headphone Jack", "HPOUTL",
> > + "Headphone Jack", "HPOUTR",
> > + "Ext Spk", "SPKOUTL",
> > + "Ext Spk", "SPKOUTR",
> > + "AMIC", "MICBIAS",
> > + "IN3R", "AMIC",
> > + "IN1R", "AMIC";
> > + };
> > +
> > + /*
> > + * This dummy i2c mux. GPIO actually will not impact selection. At actual boards, only 1
> > + * device connectted. I2C client driver will check ID when probe. Only matched ID's driver
> > + * probe successfully.
>
> NAK
>
>
> Best regards,
> Krzysztof
>

2024-04-05 14:56:56

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH 1/1] arm64: dts: imx8qxp-mek: add cm40_i2c, wm8960/wm8962 and sai[0,1,4,5]

On Fri, Apr 05, 2024 at 08:36:48AM +0200, Francesco Dolcini wrote:
> Hello Frank,
>
> On Thu, Apr 04, 2024 at 12:19:13PM -0400, Frank Li wrote:
> > imx8qxp-mek use two kind audio codec, wm8960 and wm8962. Using dummy gpio
> > i2c bus mux to connect both i2c devices. One will probe failure and other
> > will probe success when devices driver check whoami. So one dtb can cover
> > both board configuration.
> >
> > Signed-off-by: Frank Li <[email protected]>
> > ---
> > arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 210 ++++++++++++++++++
> > 1 file changed, 210 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> > index 8360bb851ac03..adff87c7cf305 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> > @@ -44,6 +51,105 @@ usb3_data_ss: endpoint {
>
> [...]
>
> > + /*
> > + * This dummy i2c mux. GPIO actually will not impact selection. At actual boards, only 1
> > + * device connectted. I2C client driver will check ID when probe. Only matched ID's driver
> > + * probe successfully.
> > + */
> > + i2cvmux: i2cmux {
> > + compatible = "i2c-mux-gpio";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + mux-gpios = <&lsio_gpio5 0 GPIO_ACTIVE_HIGH>; /* use an unused gpio */
>
> There is for sure people that have more experience and competency that
> me and it would be interesting to hear their feedback, but this
> looks like a bad hack, and you are just playing with the driver
> behavior to ensure that you get what you need.

We want to use one dtb to handle differecne bench boards because some chips
are out of life-cycle. I don't think it is 'hack' although not
straightforward.

check woiam is quite common at i2c drivers. All used method is quite
common, I just change it from difference point.

>
> Francesco
>
>

2024-04-05 18:21:32

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/1] arm64: dts: imx8qxp-mek: add cm40_i2c, wm8960/wm8962 and sai[0,1,4,5]

On 05/04/2024 16:46, Frank Li wrote:
> On Fri, Apr 05, 2024 at 08:41:59AM +0200, Krzysztof Kozlowski wrote:
>> On 04/04/2024 18:19, Frank Li wrote:
>>> imx8qxp-mek use two kind audio codec, wm8960 and wm8962. Using dummy gpio
>>> i2c bus mux to connect both i2c devices. One will probe failure and other
>>> will probe success when devices driver check whoami. So one dtb can cover
>>> both board configuration.
>>
>> I don't understand it. Either you add real device or not. If one board
>> has two devices, then why do you need to check for failures?
>>
>> Anyway, don't add fake stuff to DTS.
>
> NAK can't resolve the problem. It should be common problem for long time
> cycle boards. Some chipes will be out life cycle. such as some sensor. So
> chips on boards have been replace by some pin to pin compatible sensor. For
> example:
> old boards: use sensor A with address 0x1a
> new bench: use sensor B with address 0x1b.
>
> You can treat it as two kind boards, RevA or RevB. But most user want to
> use one dtb to handle such small differences. For this case, it should be
> simple. Just add a super set.
> i2c
> {
> sensorA@1a
> {
> }
> sensorB@1b
> {
> }
> }
>
> It also depend on whoami check by i2c devices. Only A or B will probe.
>
> wm8960 and wm8962 are more complex example. wm8960 is out of life. But
> wm8962 and wm8960 have the same i2c address. The current i2c frame can't
> allow the same i2c address in one i2c bus.
>
> You are feel to NAK my method, but I hope you also provide constructive
> solution to help resolve the problem.

Yes, we resolved it long time ago. Your bootloader can (usually easily)
detect revision of the board and load appropriate DTS or DTS+DTSO.

Best regards,
Krzysztof


2024-04-08 15:45:04

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH 1/1] arm64: dts: imx8qxp-mek: add cm40_i2c, wm8960/wm8962 and sai[0,1,4,5]

On Fri, Apr 05, 2024 at 08:21:18PM +0200, Krzysztof Kozlowski wrote:
> On 05/04/2024 16:46, Frank Li wrote:
> > On Fri, Apr 05, 2024 at 08:41:59AM +0200, Krzysztof Kozlowski wrote:
> >> On 04/04/2024 18:19, Frank Li wrote:
> >>> imx8qxp-mek use two kind audio codec, wm8960 and wm8962. Using dummy gpio
> >>> i2c bus mux to connect both i2c devices. One will probe failure and other
> >>> will probe success when devices driver check whoami. So one dtb can cover
> >>> both board configuration.
> >>
> >> I don't understand it. Either you add real device or not. If one board
> >> has two devices, then why do you need to check for failures?
> >>
> >> Anyway, don't add fake stuff to DTS.
> >
> > NAK can't resolve the problem. It should be common problem for long time
> > cycle boards. Some chipes will be out life cycle. such as some sensor. So
> > chips on boards have been replace by some pin to pin compatible sensor. For
> > example:
> > old boards: use sensor A with address 0x1a
> > new bench: use sensor B with address 0x1b.
> >
> > You can treat it as two kind boards, RevA or RevB. But most user want to
> > use one dtb to handle such small differences. For this case, it should be
> > simple. Just add a super set.
> > i2c
> > {
> > sensorA@1a
> > {
> > }
> > sensorB@1b
> > {
> > }
> > }
> >
> > It also depend on whoami check by i2c devices. Only A or B will probe.
> >
> > wm8960 and wm8962 are more complex example. wm8960 is out of life. But
> > wm8962 and wm8960 have the same i2c address. The current i2c frame can't
> > allow the same i2c address in one i2c bus.
> >
> > You are feel to NAK my method, but I hope you also provide constructive
> > solution to help resolve the problem.
>
> Yes, we resolved it long time ago. Your bootloader can (usually easily)
> detect revision of the board and load appropriate DTS or DTS+DTSO.

I knewn it. But the problem is one development boards A have many options,
so create many child dts for files, A1, A2, ... An which base on A

If there are difference happen at A, create new B. then create all child
dtb, B1, B2, ... Bn. DTB number will increase exponent.

If change is quite bit, we have to do that. But if change is quite small,
One dtb can cover it by driver auto detect, which will work like some
adaptor card have not plug into boards, or some sensor or NOR-flash have
not installed because reduce cost.

Although boot loader can update dts or choose difference dts, It also cause
many confusition, such as layerscape, uboot update many kernel dtb's
information, which actually increase dependence between uboot and kernel.
Also it confuse people, for example, when try to debug kernel dtb, why
change have not token affect when change dts because not realized uboot
over write it.

What's I dide is that trying to reduce unnecessary dts.

Frank

>
> Best regards,
> Krzysztof
>

2024-04-09 06:33:40

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/1] arm64: dts: imx8qxp-mek: add cm40_i2c, wm8960/wm8962 and sai[0,1,4,5]

On 08/04/2024 17:36, Frank Li wrote:
> On Fri, Apr 05, 2024 at 08:21:18PM +0200, Krzysztof Kozlowski wrote:
>> On 05/04/2024 16:46, Frank Li wrote:
>>> On Fri, Apr 05, 2024 at 08:41:59AM +0200, Krzysztof Kozlowski wrote:
>>>> On 04/04/2024 18:19, Frank Li wrote:
>>>>> imx8qxp-mek use two kind audio codec, wm8960 and wm8962. Using dummy gpio
>>>>> i2c bus mux to connect both i2c devices. One will probe failure and other
>>>>> will probe success when devices driver check whoami. So one dtb can cover
>>>>> both board configuration.
>>>>
>>>> I don't understand it. Either you add real device or not. If one board
>>>> has two devices, then why do you need to check for failures?
>>>>
>>>> Anyway, don't add fake stuff to DTS.
>>>
>>> NAK can't resolve the problem. It should be common problem for long time
>>> cycle boards. Some chipes will be out life cycle. such as some sensor. So
>>> chips on boards have been replace by some pin to pin compatible sensor. For
>>> example:
>>> old boards: use sensor A with address 0x1a
>>> new bench: use sensor B with address 0x1b.
>>>
>>> You can treat it as two kind boards, RevA or RevB. But most user want to
>>> use one dtb to handle such small differences. For this case, it should be
>>> simple. Just add a super set.
>>> i2c
>>> {
>>> sensorA@1a
>>> {
>>> }
>>> sensorB@1b
>>> {
>>> }
>>> }
>>>
>>> It also depend on whoami check by i2c devices. Only A or B will probe.
>>>
>>> wm8960 and wm8962 are more complex example. wm8960 is out of life. But
>>> wm8962 and wm8960 have the same i2c address. The current i2c frame can't
>>> allow the same i2c address in one i2c bus.
>>>
>>> You are feel to NAK my method, but I hope you also provide constructive
>>> solution to help resolve the problem.
>>
>> Yes, we resolved it long time ago. Your bootloader can (usually easily)
>> detect revision of the board and load appropriate DTS or DTS+DTSO.
>
> I knewn it. But the problem is one development boards A have many options,
> so create many child dts for files, A1, A2, ... An which base on A

So use DTSO, what's the problem? Other vendors, liek Rpi does not have
problem with it and it works well. No confusion.

>
> If there are difference happen at A, create new B. then create all child
> dtb, B1, B2, ... Bn. DTB number will increase exponent.
>
> If change is quite bit, we have to do that. But if change is quite small,
> One dtb can cover it by driver auto detect, which will work like some
> adaptor card have not plug into boards, or some sensor or NOR-flash have
> not installed because reduce cost.

You have two boards, not 20 here!

>
> Although boot loader can update dts or choose difference dts, It also cause
> many confusition, such as layerscape, uboot update many kernel dtb's
> information, which actually increase dependence between uboot and kernel.
> Also it confuse people, for example, when try to debug kernel dtb, why
> change have not token affect when change dts because not realized uboot
> over write it.
>
> What's I dide is that trying to reduce unnecessary dts.

There is no confusion. That's normal process, so if someone is confused
by having variants of board, this someone will be even more confused by
seeing non-existing hardware in his DTS.

This problem was solved long time ago and you do not bring any
reasonable new arguments. All vendors solved it (just look at ongoing
discussions on board id) but only you have problem with their solution.

NAK

Best regards,
Krzysztof


2024-04-09 16:04:12

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH 1/1] arm64: dts: imx8qxp-mek: add cm40_i2c, wm8960/wm8962 and sai[0,1,4,5]

On Tue, Apr 09, 2024 at 08:33:22AM +0200, Krzysztof Kozlowski wrote:
> On 08/04/2024 17:36, Frank Li wrote:
> > On Fri, Apr 05, 2024 at 08:21:18PM +0200, Krzysztof Kozlowski wrote:
> >> On 05/04/2024 16:46, Frank Li wrote:
> >>> On Fri, Apr 05, 2024 at 08:41:59AM +0200, Krzysztof Kozlowski wrote:
> >>>> On 04/04/2024 18:19, Frank Li wrote:
> >>>>> imx8qxp-mek use two kind audio codec, wm8960 and wm8962. Using dummy gpio
> >>>>> i2c bus mux to connect both i2c devices. One will probe failure and other
> >>>>> will probe success when devices driver check whoami. So one dtb can cover
> >>>>> both board configuration.
> >>>>
> >>>> I don't understand it. Either you add real device or not. If one board
> >>>> has two devices, then why do you need to check for failures?
> >>>>
> >>>> Anyway, don't add fake stuff to DTS.
> >>>
> >>> NAK can't resolve the problem. It should be common problem for long time
> >>> cycle boards. Some chipes will be out life cycle. such as some sensor. So
> >>> chips on boards have been replace by some pin to pin compatible sensor. For
> >>> example:
> >>> old boards: use sensor A with address 0x1a
> >>> new bench: use sensor B with address 0x1b.
> >>>
> >>> You can treat it as two kind boards, RevA or RevB. But most user want to
> >>> use one dtb to handle such small differences. For this case, it should be
> >>> simple. Just add a super set.
> >>> i2c
> >>> {
> >>> sensorA@1a
> >>> {
> >>> }
> >>> sensorB@1b
> >>> {
> >>> }
> >>> }
> >>>
> >>> It also depend on whoami check by i2c devices. Only A or B will probe.
> >>>
> >>> wm8960 and wm8962 are more complex example. wm8960 is out of life. But
> >>> wm8962 and wm8960 have the same i2c address. The current i2c frame can't
> >>> allow the same i2c address in one i2c bus.
> >>>
> >>> You are feel to NAK my method, but I hope you also provide constructive
> >>> solution to help resolve the problem.
> >>
> >> Yes, we resolved it long time ago. Your bootloader can (usually easily)
> >> detect revision of the board and load appropriate DTS or DTS+DTSO.
> >
> > I knewn it. But the problem is one development boards A have many options,
> > so create many child dts for files, A1, A2, ... An which base on A
>
> So use DTSO, what's the problem? Other vendors, liek Rpi does not have
> problem with it and it works well. No confusion.
>
> >
> > If there are difference happen at A, create new B. then create all child
> > dtb, B1, B2, ... Bn. DTB number will increase exponent.
> >
> > If change is quite bit, we have to do that. But if change is quite small,
> > One dtb can cover it by driver auto detect, which will work like some
> > adaptor card have not plug into boards, or some sensor or NOR-flash have
> > not installed because reduce cost.
>
> You have two boards, not 20 here!

Actually, it is around ~20 derived boards. It is not upstream just because
we have not time to do that yet. After some clean up, I estimate about 7 -
- 10.

>
> >
> > Although boot loader can update dts or choose difference dts, It also cause
> > many confusition, such as layerscape, uboot update many kernel dtb's
> > information, which actually increase dependence between uboot and kernel.
> > Also it confuse people, for example, when try to debug kernel dtb, why
> > change have not token affect when change dts because not realized uboot
> > over write it.
> >
> > What's I dide is that trying to reduce unnecessary dts.
>
> There is no confusion. That's normal process, so if someone is confused
> by having variants of board, this someone will be even more confused by
> seeing non-existing hardware in his DTS.

How about existed dummy_clk and dummy regulator in dts?

>
> This problem was solved long time ago and you do not bring any
> reasonable new arguments. All vendors solved it (just look at ongoing
> discussions on board id) but only you have problem with their solution.

I never said solution was not work. Just not friend for user enough for
this case. It likes USB TypeC vs USB A port. Both works, just TypeC can't
care direction.

>
> NAK
>
> Best regards,
> Krzysztof
>