diff mbox series

[03/10] arm64: dts: imx8mp-skov: use I2C5 for DDC

Message ID 20241219-skov-dt-updates-v1-3-38bf80dc22df@pengutronix.de (mailing list archive)
State New
Headers show
Series arm64: dts: imx8mp-skov: flesh out device trees | expand

Commit Message

Ahmad Fatoum Dec. 19, 2024, 7:25 a.m. UTC
The HDMI DDC pads can be muxed either to an i.MX I2C controller or
to a limited I2C controller within the Designware HDMI bridge.

So far we muxed the pads to the HDMI bridge, but the i.MX I2C controller
is the better choice:

  - We use DDC/CI commands for display brightness configuration, but the
    Linux driver refuses[1] transfers to/from address 0x37, because the
    controller doesn't support multi-byte requests.

  - The driver doesn't support I2C bus recovery, but we need that,
    because some HDMI panels used with the board can be flaky.

As the i.MX I2C controller doesn't have either of these limitations,
let's make use of it instead.

[1]: https://lore.kernel.org/all/20190722181945.244395-1-mka@chromium.org/

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 .../boot/dts/freescale/imx8mp-skov-revb-hdmi.dts   | 26 ++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

Comments

Frank Li Dec. 19, 2024, 5:34 p.m. UTC | #1
On Thu, Dec 19, 2024 at 08:25:27AM +0100, Ahmad Fatoum wrote:
> The HDMI DDC pads can be muxed either to an i.MX I2C controller or
> to a limited I2C controller within the Designware HDMI bridge.
>
> So far we muxed the pads to the HDMI bridge, but the i.MX I2C controller
> is the better choice:

Switch to the i.MX I2C controller because

>
>   - We use DDC/CI commands for display brightness configuration, but the
>     Linux driver refuses[1] transfers to/from address 0x37, because the
>     controller doesn't support multi-byte requests.

      Designware HDMI Limited I2C controller doesn't support multi-byte
requests and display brightness configuration by DDC/CI command need it.

>
>   - The driver doesn't support I2C bus recovery, but we need that,
>     because some HDMI panels used with the board can be flaky.

      Designware HDMI Limited I2C controller doesn't support I2C bus
recovery.

>
> As the i.MX I2C controller doesn't have either of these limitations,
> let's make use of it instead.

Reduntant. can be removed.

Frank
>
> [1]: https://lore.kernel.org/all/20190722181945.244395-1-mka@chromium.org/
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  .../boot/dts/freescale/imx8mp-skov-revb-hdmi.dts   | 26 ++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
> index c1ca69da3cb8..206116be8166 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
> @@ -9,12 +9,34 @@ / {
>  	compatible = "skov,imx8mp-skov-revb-hdmi", "fsl,imx8mp";
>  };
>
> +&i2c5 {
> +	pinctrl-names = "default", "gpio";
> +	pinctrl-0 = <&pinctrl_i2c5>;
> +	pinctrl-1 = <&pinctrl_i2c5_gpio>;
> +	scl-gpios = <&gpio3 26 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +	sda-gpios = <&gpio3 27 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +	clock-frequency = <100000>;
> +	status = "okay";
> +};
> +
>  &iomuxc {
>  	pinctrl_hdmi: hdmigrp {
>  		fsl,pins = <
> -			MX8MP_IOMUXC_HDMI_DDC_SCL__HDMIMIX_HDMI_SCL		0x1c3
> -			MX8MP_IOMUXC_HDMI_DDC_SDA__HDMIMIX_HDMI_SDA		0x1c3
>  			MX8MP_IOMUXC_HDMI_HPD__HDMIMIX_HDMI_HPD			0x19
>  		>;
>  	};
> +
> +	pinctrl_i2c5: i2c5grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_HDMI_DDC_SCL__I2C5_SCL			0x400001c2
> +			MX8MP_IOMUXC_HDMI_DDC_SDA__I2C5_SDA			0x400001c2
> +		>;
> +	};
> +
> +	pinctrl_i2c5_gpio: i2c5gpiogrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_HDMI_DDC_SCL__GPIO3_IO26			0x400001c2
> +			MX8MP_IOMUXC_HDMI_DDC_SDA__GPIO3_IO27			0x400001c2
> +		>;
> +	};
>  };
>
> --
> 2.39.5
>
Ahmad Fatoum Dec. 19, 2024, 5:43 p.m. UTC | #2
Hello Frank,

On 19.12.24 18:34, Frank Li wrote:
> On Thu, Dec 19, 2024 at 08:25:27AM +0100, Ahmad Fatoum wrote:
>> The HDMI DDC pads can be muxed either to an i.MX I2C controller or
>> to a limited I2C controller within the Designware HDMI bridge.
>>
>> So far we muxed the pads to the HDMI bridge, but the i.MX I2C controller
>> is the better choice:
> 
> Switch to the i.MX I2C controller because
> 
>>
>>   - We use DDC/CI commands for display brightness configuration, but the
>>     Linux driver refuses[1] transfers to/from address 0x37, because the
>>     controller doesn't support multi-byte requests.
> 
>       Designware HDMI Limited I2C controller doesn't support multi-byte
> requests and display brightness configuration by DDC/CI command need it.
> 
>>
>>   - The driver doesn't support I2C bus recovery, but we need that,
>>     because some HDMI panels used with the board can be flaky.
> 
>       Designware HDMI Limited I2C controller doesn't support I2C bus
> recovery.
> 
>>
>> As the i.MX I2C controller doesn't have either of these limitations,
>> let's make use of it instead.
> 
> Reduntant. can be removed.

Sorry, I am very much open to review feedback, but I fail to see how
your subjective take improves things.

Thanks,
Ahmad




> 
> Frank
>>
>> [1]: https://lore.kernel.org/all/20190722181945.244395-1-mka@chromium.org/
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  .../boot/dts/freescale/imx8mp-skov-revb-hdmi.dts   | 26 ++++++++++++++++++++--
>>  1 file changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
>> index c1ca69da3cb8..206116be8166 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
>> @@ -9,12 +9,34 @@ / {
>>  	compatible = "skov,imx8mp-skov-revb-hdmi", "fsl,imx8mp";
>>  };
>>
>> +&i2c5 {
>> +	pinctrl-names = "default", "gpio";
>> +	pinctrl-0 = <&pinctrl_i2c5>;
>> +	pinctrl-1 = <&pinctrl_i2c5_gpio>;
>> +	scl-gpios = <&gpio3 26 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> +	sda-gpios = <&gpio3 27 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> +	clock-frequency = <100000>;
>> +	status = "okay";
>> +};
>> +
>>  &iomuxc {
>>  	pinctrl_hdmi: hdmigrp {
>>  		fsl,pins = <
>> -			MX8MP_IOMUXC_HDMI_DDC_SCL__HDMIMIX_HDMI_SCL		0x1c3
>> -			MX8MP_IOMUXC_HDMI_DDC_SDA__HDMIMIX_HDMI_SDA		0x1c3
>>  			MX8MP_IOMUXC_HDMI_HPD__HDMIMIX_HDMI_HPD			0x19
>>  		>;
>>  	};
>> +
>> +	pinctrl_i2c5: i2c5grp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_HDMI_DDC_SCL__I2C5_SCL			0x400001c2
>> +			MX8MP_IOMUXC_HDMI_DDC_SDA__I2C5_SDA			0x400001c2
>> +		>;
>> +	};
>> +
>> +	pinctrl_i2c5_gpio: i2c5gpiogrp {
>> +		fsl,pins = <
>> +			MX8MP_IOMUXC_HDMI_DDC_SCL__GPIO3_IO26			0x400001c2
>> +			MX8MP_IOMUXC_HDMI_DDC_SDA__GPIO3_IO27			0x400001c2
>> +		>;
>> +	};
>>  };
>>
>> --
>> 2.39.5
>>
>
Frank Li Dec. 19, 2024, 7:27 p.m. UTC | #3
On Thu, Dec 19, 2024 at 06:43:56PM +0100, Ahmad Fatoum wrote:
> Hello Frank,
>
> On 19.12.24 18:34, Frank Li wrote:
> > On Thu, Dec 19, 2024 at 08:25:27AM +0100, Ahmad Fatoum wrote:
> >> The HDMI DDC pads can be muxed either to an i.MX I2C controller or
> >> to a limited I2C controller within the Designware HDMI bridge.
> >>
> >> So far we muxed the pads to the HDMI bridge, but the i.MX I2C controller
> >> is the better choice:
> >
> > Switch to the i.MX I2C controller because
> >
> >>
> >>   - We use DDC/CI commands for display brightness configuration, but the
> >>     Linux driver refuses[1] transfers to/from address 0x37, because the
> >>     controller doesn't support multi-byte requests.
> >
> >       Designware HDMI Limited I2C controller doesn't support multi-byte
> > requests and display brightness configuration by DDC/CI command need it.
> >
> >>
> >>   - The driver doesn't support I2C bus recovery, but we need that,
> >>     because some HDMI panels used with the board can be flaky.
> >
> >       Designware HDMI Limited I2C controller doesn't support I2C bus
> > recovery.
> >
> >>
> >> As the i.MX I2C controller doesn't have either of these limitations,
> >> let's make use of it instead.
> >
> > Reduntant. can be removed.
>
> Sorry, I am very much open to review feedback, but I fail to see how
> your subjective take improves things.

Ref https://docs.kernel.org/process/submitting-patches.html

"Describe your changes in imperative mood, e.g. “make xyzzy do frotz”
instead of “[This patch] makes xyzzy do frotz” or “[I] changed xyzzy to do
frotz”, as if you are giving orders to the codebase to change its behaviour."

Avoid use "we(same as "I" in above example) ..."

Frank

>
> Thanks,
> Ahmad
>
>
>
>
> >
> > Frank
> >>
> >> [1]: https://lore.kernel.org/all/20190722181945.244395-1-mka@chromium.org/
> >>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >>  .../boot/dts/freescale/imx8mp-skov-revb-hdmi.dts   | 26 ++++++++++++++++++++--
> >>  1 file changed, 24 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
> >> index c1ca69da3cb8..206116be8166 100644
> >> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
> >> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
> >> @@ -9,12 +9,34 @@ / {
> >>  	compatible = "skov,imx8mp-skov-revb-hdmi", "fsl,imx8mp";
> >>  };
> >>
> >> +&i2c5 {
> >> +	pinctrl-names = "default", "gpio";
> >> +	pinctrl-0 = <&pinctrl_i2c5>;
> >> +	pinctrl-1 = <&pinctrl_i2c5_gpio>;
> >> +	scl-gpios = <&gpio3 26 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> >> +	sda-gpios = <&gpio3 27 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> >> +	clock-frequency = <100000>;
> >> +	status = "okay";
> >> +};
> >> +
> >>  &iomuxc {
> >>  	pinctrl_hdmi: hdmigrp {
> >>  		fsl,pins = <
> >> -			MX8MP_IOMUXC_HDMI_DDC_SCL__HDMIMIX_HDMI_SCL		0x1c3
> >> -			MX8MP_IOMUXC_HDMI_DDC_SDA__HDMIMIX_HDMI_SDA		0x1c3
> >>  			MX8MP_IOMUXC_HDMI_HPD__HDMIMIX_HDMI_HPD			0x19
> >>  		>;
> >>  	};
> >> +
> >> +	pinctrl_i2c5: i2c5grp {
> >> +		fsl,pins = <
> >> +			MX8MP_IOMUXC_HDMI_DDC_SCL__I2C5_SCL			0x400001c2
> >> +			MX8MP_IOMUXC_HDMI_DDC_SDA__I2C5_SDA			0x400001c2
> >> +		>;
> >> +	};
> >> +
> >> +	pinctrl_i2c5_gpio: i2c5gpiogrp {
> >> +		fsl,pins = <
> >> +			MX8MP_IOMUXC_HDMI_DDC_SCL__GPIO3_IO26			0x400001c2
> >> +			MX8MP_IOMUXC_HDMI_DDC_SDA__GPIO3_IO27			0x400001c2
> >> +		>;
> >> +	};
> >>  };
> >>
> >> --
> >> 2.39.5
> >>
> >
>
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
index c1ca69da3cb8..206116be8166 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
@@ -9,12 +9,34 @@  / {
 	compatible = "skov,imx8mp-skov-revb-hdmi", "fsl,imx8mp";
 };
 
+&i2c5 {
+	pinctrl-names = "default", "gpio";
+	pinctrl-0 = <&pinctrl_i2c5>;
+	pinctrl-1 = <&pinctrl_i2c5_gpio>;
+	scl-gpios = <&gpio3 26 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+	sda-gpios = <&gpio3 27 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+	clock-frequency = <100000>;
+	status = "okay";
+};
+
 &iomuxc {
 	pinctrl_hdmi: hdmigrp {
 		fsl,pins = <
-			MX8MP_IOMUXC_HDMI_DDC_SCL__HDMIMIX_HDMI_SCL		0x1c3
-			MX8MP_IOMUXC_HDMI_DDC_SDA__HDMIMIX_HDMI_SDA		0x1c3
 			MX8MP_IOMUXC_HDMI_HPD__HDMIMIX_HDMI_HPD			0x19
 		>;
 	};
+
+	pinctrl_i2c5: i2c5grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_HDMI_DDC_SCL__I2C5_SCL			0x400001c2
+			MX8MP_IOMUXC_HDMI_DDC_SDA__I2C5_SDA			0x400001c2
+		>;
+	};
+
+	pinctrl_i2c5_gpio: i2c5gpiogrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_HDMI_DDC_SCL__GPIO3_IO26			0x400001c2
+			MX8MP_IOMUXC_HDMI_DDC_SDA__GPIO3_IO27			0x400001c2
+		>;
+	};
 };