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

Ahmad Fatoum posted 10 patches 12 months ago
There is a newer version of this series
[PATCH 03/10] arm64: dts: imx8mp-skov: use I2C5 for DDC
Posted by Ahmad Fatoum 12 months ago
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(-)

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
Re: [PATCH 03/10] arm64: dts: imx8mp-skov: use I2C5 for DDC
Posted by Frank Li 12 months ago
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
>
Re: [PATCH 03/10] arm64: dts: imx8mp-skov: use I2C5 for DDC
Posted by Ahmad Fatoum 12 months ago
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
>>
> 


-- 
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 |
Re: [PATCH 03/10] arm64: dts: imx8mp-skov: use I2C5 for DDC
Posted by Frank Li 12 months ago
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 |
Re: [PATCH 03/10] arm64: dts: imx8mp-skov: use I2C5 for DDC
Posted by Ahmad Fatoum 11 months, 2 weeks ago
On 19.12.24 20:27, Frank Li wrote:
> 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) ..."

We is replaced by passive voice in v2.

Cheers,
Ahmad

> 
> 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 |
> 


-- 
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 |