[PATCH 00/10] arm64: dts: imx8mp-skov: flesh out device trees

[PATCH 00/10] arm64: dts: imx8mp-skov: flesh out device trees

[Ahmad Fatoum]

When the Skov device trees were added a year ago, there were no
upstream bindings yet for many parts of the display pipeline.

This has changed now, so this series fleshes the display pipeline parts
out and adds some more fixes/enhancements that have accrued in the
meantime.

---
Ahmad Fatoum (5):
      arm64: dts: imx8mp-skov: correct PMIC board limits
      arm64: dts: imx8mp-skov: operate CPU at 850 mV by default
      arm64: dts: imx8mp-skov: use I2C5 for DDC
      dt-bindings: display/lvds-codec: add ti,sn65lvds822
      arm64: dts: imx8mp-skov: configure uart1 for RS485

Oleksij Rempel (5):
      arm64: dts: imx8mp-skov: describe HDMI display pipeline
      arm64: dts: imx8mp-skov: describe LVDS display pipeline
      arm64: dts: imx8mp-skov: describe mains fail detection
      arm64: dts: imx8mp-skov: fix phy-mode
      arm64: dts: imx8mp-skov: increase I2C clock frequency for RTC

 .../bindings/display/bridge/lvds-codec.yaml        |  1 +
 .../arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi | 63 ++++++++++++++++------
 .../boot/dts/freescale/imx8mp-skov-revb-hdmi.dts   | 45 +++++++++++++++-
 .../boot/dts/freescale/imx8mp-skov-revb-lt6.dts    | 60 +++++++++++++++++++++
 4 files changed, 152 insertions(+), 17 deletions(-)
---
base-commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
change-id: 20241218-skov-dt-updates-eb8b73c693d1

Best regards,
-- 
Ahmad Fatoum <a.fatoum@pengutronix.de>

[PATCH 01/10] arm64: dts: imx8mp-skov: correct PMIC board limits

[Ahmad Fatoum]

The PMIC voltage constraints in the device tree currently describe the
permissible range of the PMIC. This is unnecessary as this information
already exists in the driver and wrong as it doesn't account for
board-specific constraints, e.g. a 2.1V on VDD_SOC would fry the SoC and
a maximum voltage of 3.4V on the VDD_3V3 rail may be unexpected across
the board.

Fix this by adjusting constraints to reflect the board limits.

Fixes: 6d382d51d979 ("arm64: dts: freescale: Add SKOV IMX8MP CPU revB board")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 .../arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi | 24 +++++++++++-----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
index 59813ef8e2bb..ae82166b5c26 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
@@ -247,8 +247,8 @@ reg_vdd_soc: BUCK1 {
 
 			reg_vdd_arm: BUCK2 {
 				regulator-name = "VDD_ARM";
-				regulator-min-microvolt = <600000>;
-				regulator-max-microvolt = <2187500>;
+				regulator-min-microvolt = <850000>;
+				regulator-max-microvolt = <1000000>;
 				vin-supply = <&reg_5v_p>;
 				regulator-boot-on;
 				regulator-always-on;
@@ -259,8 +259,8 @@ reg_vdd_arm: BUCK2 {
 
 			reg_vdd_3v3: BUCK4 {
 				regulator-name = "VDD_3V3";
-				regulator-min-microvolt = <600000>;
-				regulator-max-microvolt = <3400000>;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
 				vin-supply = <&reg_5v_p>;
 				regulator-boot-on;
 				regulator-always-on;
@@ -268,8 +268,8 @@ reg_vdd_3v3: BUCK4 {
 
 			reg_vdd_1v8: BUCK5 {
 				regulator-name = "VDD_1V8";
-				regulator-min-microvolt = <600000>;
-				regulator-max-microvolt = <3400000>;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
 				vin-supply = <&reg_5v_p>;
 				regulator-boot-on;
 				regulator-always-on;
@@ -277,8 +277,8 @@ reg_vdd_1v8: BUCK5 {
 
 			reg_nvcc_dram_1v1: BUCK6 {
 				regulator-name = "NVCC_DRAM_1V1";
-				regulator-min-microvolt = <600000>;
-				regulator-max-microvolt = <3400000>;
+				regulator-min-microvolt = <1100000>;
+				regulator-max-microvolt = <1100000>;
 				vin-supply = <&reg_5v_p>;
 				regulator-boot-on;
 				regulator-always-on;
@@ -286,8 +286,8 @@ reg_nvcc_dram_1v1: BUCK6 {
 
 			reg_nvcc_snvs_1v8: LDO1 {
 				regulator-name = "NVCC_SNVS_1V8";
-				regulator-min-microvolt = <1600000>;
-				regulator-max-microvolt = <3300000>;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
 				vin-supply = <&reg_5v_p>;
 				regulator-boot-on;
 				regulator-always-on;
@@ -295,8 +295,8 @@ reg_nvcc_snvs_1v8: LDO1 {
 
 			reg_vdda_1v8: LDO3 {
 				regulator-name = "VDDA_1V8";
-				regulator-min-microvolt = <800000>;
-				regulator-max-microvolt = <3300000>;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
 				vin-supply = <&reg_5v_p>;
 				regulator-boot-on;
 				regulator-always-on;

-- 
2.39.5

[PATCH 02/10] arm64: dts: imx8mp-skov: operate CPU at 850 mV by default

[Ahmad Fatoum]

The Skov i.MX8MP boards are passively cooled and heatsink is specced for
continuous operation at 1.2 GHz only. Short bouts of 1.6 GHz are ok,
but these should be invoked intentionally, not as part of
suspend/resume cycles.

Therefore, configure RUN frequency as 850 mV and remove the higher
voltage operating points from those permissible for suspend.

Fixes: 6d382d51d979 ("arm64: dts: freescale: Add SKOV IMX8MP CPU revB board")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
index ae82166b5c26..7ae686d37dda 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
@@ -163,6 +163,19 @@ reg_vsd_3v3: regulator-vsd-3v3 {
 	};
 };
 
+/*
+ * Board is passively cooled and heatsink is specced for continuous operation
+ * at 1.2 GHz only. Short bouts of 1.6 GHz are ok, but these should be done
+ * intentionally, not as part of suspend/resume cycles.
+ */
+&{/opp-table/opp-1600000000} {
+	/delete-property/ opp-suspend;
+};
+
+&{/opp-table/opp-1800000000} {
+	/delete-property/ opp-suspend;
+};
+
 &A53_0 {
 	cpu-supply = <&reg_vdd_arm>;
 };
@@ -253,7 +266,7 @@ reg_vdd_arm: BUCK2 {
 				regulator-boot-on;
 				regulator-always-on;
 				regulator-ramp-delay = <3125>;
-				nxp,dvs-run-voltage = <950000>;
+				nxp,dvs-run-voltage = <850000>;
 				nxp,dvs-standby-voltage = <850000>;
 			};
 

-- 
2.39.5

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

[Ahmad Fatoum]

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

[PATCH 04/10] arm64: dts: imx8mp-skov: describe HDMI display pipeline

[Ahmad Fatoum]

From: Oleksij Rempel <o.rempel@pengutronix.de>

Back when this device tree was first added, there were no i.MX8MP
HDMI bindings upstream yet. This has changed now, so let's use them
to describe the HDMI on the imx8mp-skov-revb-hdmi board.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 .../boot/dts/freescale/imx8mp-skov-revb-hdmi.dts      | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

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 206116be8166..32a429437cbd 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
@@ -9,6 +9,21 @@ / {
 	compatible = "skov,imx8mp-skov-revb-hdmi", "fsl,imx8mp";
 };
 
+&hdmi_pvi {
+	status = "okay";
+};
+
+&hdmi_tx {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_hdmi>;
+	ddc-i2c-bus = <&i2c5>;
+	status = "okay";
+};
+
+&hdmi_tx_phy {
+	status = "okay";
+};
+
 &i2c5 {
 	pinctrl-names = "default", "gpio";
 	pinctrl-0 = <&pinctrl_i2c5>;
@@ -19,6 +34,10 @@ &i2c5 {
 	status = "okay";
 };
 
+&lcdif3 {
+	status = "okay";
+};
+
 &iomuxc {
 	pinctrl_hdmi: hdmigrp {
 		fsl,pins = <

-- 
2.39.5

[PATCH 05/10] dt-bindings: display/lvds-codec: add ti,sn65lvds822

[Ahmad Fatoum]

Add compatible strings for TI SN65LVDS822, a FlatLink LVDS receiver.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
To: Andrzej Hajda <andrzej.hajda@intel.com> (maintainer:DRM DRIVERS FOR BRIDGE CHIPS)
To: Neil Armstrong <neil.armstrong@linaro.org> (maintainer:DRM DRIVERS FOR BRIDGE CHIPS)
To: Robert Foss <rfoss@kernel.org> (maintainer:DRM DRIVERS FOR BRIDGE CHIPS)
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> (reviewer:DRM DRIVERS FOR BRIDGE CHIPS,in file)
Cc: Jonas Karlman <jonas@kwiboo.se> (reviewer:DRM DRIVERS FOR BRIDGE CHIPS)
Cc: Jernej Skrabec <jernej.skrabec@gmail.com> (reviewer:DRM DRIVERS FOR BRIDGE CHIPS)
Cc: David Airlie <airlied@gmail.com> (maintainer:DRM DRIVERS)
Cc: Simona Vetter <simona@ffwll.ch> (maintainer:DRM DRIVERS)
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (maintainer:DRM DRIVERS AND MISC GPU PATCHES)
Cc: Maxime Ripard <mripard@kernel.org> (maintainer:DRM DRIVERS AND MISC GPU PATCHES)
Cc: Thomas Zimmermann <tzimmermann@suse.de> (maintainer:DRM DRIVERS AND MISC GPU PATCHES)
Cc: dri-devel@lists.freedesktop.org (open list:DRM DRIVERS)
---
 Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
index 6ceeed76e88e..0487bbffd7f7 100644
--- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
@@ -41,6 +41,7 @@ properties:
           - enum:
               - ti,ds90cf364a # For the DS90CF364A FPD-Link LVDS Receiver
               - ti,ds90cf384a # For the DS90CF384A FPD-Link LVDS Receiver
+              - ti,sn65lvds822  # For the SN65LVDS822 FlatLink LVDS Receiver
               - ti,sn65lvds94 # For the SN65DS94 LVDS serdes
           - const: lvds-decoder # Generic LVDS decoders compatible fallback
       - enum:

-- 
2.39.5

[PATCH 06/10] arm64: dts: imx8mp-skov: describe LVDS display pipeline

[Ahmad Fatoum]

From: Oleksij Rempel <o.rempel@pengutronix.de>

The Skov i.MX8MP LT6 device tree so far described the touch screen, but
didn't describe the screen itself. Fill out these missing pieces.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 .../boot/dts/freescale/imx8mp-skov-revb-lt6.dts    | 60 ++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts
index ccbd3abedd69..baecf768a2ee 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts
@@ -8,6 +8,45 @@ / {
 	model = "SKOV IMX8MP CPU revB - LT6";
 	compatible = "skov,imx8mp-skov-revb-lt6", "fsl,imx8mp";
 
+	lvds-decoder {
+		compatible = "ti,sn65lvds822", "lvds-decoder";
+		power-supply = <&reg_3v3>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+
+				in_lvds1: endpoint {
+					data-mapping = "vesa-24";
+					remote-endpoint = <&ldb_lvds_ch1>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+
+				lvds_decoder_out: endpoint {
+					remote-endpoint = <&panel_in>;
+				};
+			};
+		};
+	};
+
+	panel {
+		compatible = "logictechno,lttd800480070-l6wh-rt";
+		backlight = <&backlight>;
+		power-supply = <&reg_tft_vcom>;
+
+		port {
+			panel_in: endpoint {
+				remote-endpoint = <&lvds_decoder_out>;
+			};
+		};
+	};
+
 	touchscreen {
 		compatible = "resistive-adc-touch";
 		io-channels = <&adc_ts 1>, <&adc_ts 3>, <&adc_ts 4>, <&adc_ts 5>;
@@ -78,6 +117,27 @@ channel@5 {
 	};
 };
 
+&lcdif2 {
+	status = "okay";
+};
+
+&lvds_bridge {
+	assigned-clocks = <&clk IMX8MP_CLK_MEDIA_LDB>,
+				 <&clk IMX8MP_VIDEO_PLL1>;
+	assigned-clock-parents = <&clk IMX8MP_VIDEO_PLL1_OUT>;
+	/* IMX8MP_VIDEO_PLL1 = IMX8MP_CLK_MEDIA_DISP2_PIX * 2 * 7 */
+	assigned-clock-rates = <0>, <462000000>;
+	status = "okay";
+
+	ports {
+		port@2 {
+			ldb_lvds_ch1: endpoint {
+				remote-endpoint = <&in_lvds1>;
+			};
+		};
+	};
+};
+
 &pwm1 {
 	status = "okay";
 };

-- 
2.39.5

[PATCH 07/10] arm64: dts: imx8mp-skov: configure uart1 for RS485

[Ahmad Fatoum]

uart1 TX, RX and RTS signals go off the base board and to a RS485
transceiver. Describe this in the device tree.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
index 7ae686d37dda..bce23d62cd27 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
@@ -404,6 +404,13 @@ &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1>;
 	status = "okay";
+	/*
+	 * While there is no CTS line, the property "uart-has-rtscts" is still
+	 * the right thing to do to enable the UART to do RS485. In RS485-Mode
+	 * CTS isn't used anyhow and there is no dedicated property
+	 * "uart-has-rts-but-no-cts".
+	 */
+	uart-has-rtscts;
 };
 
 &uart2 {
@@ -618,6 +625,8 @@ pinctrl_uart1: uart1grp {
 			MX8MP_IOMUXC_UART1_RXD__UART1_DCE_RX			0x140
 			MX8MP_IOMUXC_UART1_TXD__UART1_DCE_TX			0x140
 			MX8MP_IOMUXC_UART3_RXD__UART1_DTE_RTS			0x140
+			/* CTS pin is not connected, but needed as workaround */
+			MX8MP_IOMUXC_UART3_TXD__UART1_DTE_CTS			0x140
 		>;
 	};
 

-- 
2.39.5

[PATCH 08/10] arm64: dts: imx8mp-skov: describe mains fail detection

[Ahmad Fatoum]

From: Oleksij Rempel <o.rempel@pengutronix.de>

The main 24V regulator will toggle GPIO4_IO23 to signal imminent voltage
loss. Reflect this in the DT, so the OS can take appropriate action when
this happens.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
index bce23d62cd27..80d212acc0c3 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
@@ -116,6 +116,11 @@ reg_24v: regulator-24v {
 		regulator-name = "24V";
 		regulator-min-microvolt = <24000000>;
 		regulator-max-microvolt = <24000000>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_reg24v>;
+		interrupts-extended = <&gpio4 23 IRQ_TYPE_EDGE_FALLING>;
+		system-critical-regulator;
+		regulator-uv-less-critical-window-ms = <50>;
 	};
 
 	reg_can2rs: regulator-can2rs {
@@ -591,6 +596,12 @@ MX8MP_IOMUXC_SAI3_MCLK__PWM4_OUT			0x116
 		>;
 	};
 
+	pinctrl_reg24v: reg24vgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SAI2_RXD0__GPIO4_IO23			0x154
+		>;
+	};
+
 	pinctrl_reg_vsd_3v3: regvsd3v3grp {
 		fsl,pins = <
 			MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19	0x40

-- 
2.39.5

[PATCH 09/10] arm64: dts: imx8mp-skov: fix phy-mode

[Ahmad Fatoum]

From: Oleksij Rempel <o.rempel@pengutronix.de>

Use rgmii-rxid instead of rgmii-txid. It should have no effect here,
but rxid is the correct value in our case.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
index 80d212acc0c3..a683f46fcbab 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
@@ -215,7 +215,7 @@ adc: adc@0 {
 &eqos {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_eqos>;
-	phy-mode = "rgmii-txid";
+	phy-mode = "rgmii-rxid";
 	status = "okay";
 
 	fixed-link {

-- 
2.39.5

[PATCH 10/10] arm64: dts: imx8mp-skov: increase I2C clock frequency for RTC

[Ahmad Fatoum]

From: Oleksij Rempel <o.rempel@pengutronix.de>

The NXP PCF85063TP RTC we use is capable of up to 400 kHz of SCL clock
frequency, so let's make use of this instead of the 100 kHz bus frequency
we are currently using.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
index a683f46fcbab..ec7857db7bf0 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
@@ -333,7 +333,7 @@ reg_nvcc_sd2: LDO5 {
 };
 
 &i2c3 {
-	clock-frequency = <100000>;
+	clock-frequency = <400000>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_i2c3>;
 	status = "okay";

-- 
2.39.5

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

[Frank Li]

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 04/10] arm64: dts: imx8mp-skov: describe HDMI display pipeline

[Frank Li]

On Thu, Dec 19, 2024 at 08:25:28AM +0100, Ahmad Fatoum wrote:
> From: Oleksij Rempel <o.rempel@pengutronix.de>
>
> Back when this device tree was first added, there were no i.MX8MP
> HDMI bindings upstream yet. This has changed now, so let's use them
> to describe the HDMI on the imx8mp-skov-revb-hdmi board.

Skip your story.
Just said

Add HDMI display pipeline for imx8mp-skov-revb-hdmi board.

Frank
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  .../boot/dts/freescale/imx8mp-skov-revb-hdmi.dts      | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> 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 206116be8166..32a429437cbd 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
> @@ -9,6 +9,21 @@ / {
>  	compatible = "skov,imx8mp-skov-revb-hdmi", "fsl,imx8mp";
>  };
>
> +&hdmi_pvi {
> +	status = "okay";
> +};
> +
> +&hdmi_tx {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_hdmi>;
> +	ddc-i2c-bus = <&i2c5>;
> +	status = "okay";
> +};
> +
> +&hdmi_tx_phy {
> +	status = "okay";
> +};
> +
>  &i2c5 {
>  	pinctrl-names = "default", "gpio";
>  	pinctrl-0 = <&pinctrl_i2c5>;
> @@ -19,6 +34,10 @@ &i2c5 {
>  	status = "okay";
>  };
>
> +&lcdif3 {
> +	status = "okay";
> +};
> +
>  &iomuxc {
>  	pinctrl_hdmi: hdmigrp {
>  		fsl,pins = <
>
> --
> 2.39.5
>

Re: [PATCH 06/10] arm64: dts: imx8mp-skov: describe LVDS display pipeline

[Frank Li]

On Thu, Dec 19, 2024 at 08:25:30AM +0100, Ahmad Fatoum wrote:
> From: Oleksij Rempel <o.rempel@pengutronix.de>
>
> The Skov i.MX8MP LT6 device tree so far described the touch screen, but
> didn't describe the screen itself. Fill out these missing pieces.

Add "ti,sn65lvds822" touch screen description for i.MX8MP LT6 board.

Frank

> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  .../boot/dts/freescale/imx8mp-skov-revb-lt6.dts    | 60 ++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts
> index ccbd3abedd69..baecf768a2ee 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts
> @@ -8,6 +8,45 @@ / {
>  	model = "SKOV IMX8MP CPU revB - LT6";
>  	compatible = "skov,imx8mp-skov-revb-lt6", "fsl,imx8mp";
>
> +	lvds-decoder {
> +		compatible = "ti,sn65lvds822", "lvds-decoder";
> +		power-supply = <&reg_3v3>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +
> +				in_lvds1: endpoint {
> +					data-mapping = "vesa-24";
> +					remote-endpoint = <&ldb_lvds_ch1>;
> +				};
> +			};
> +
> +			port@1 {
> +				reg = <1>;
> +
> +				lvds_decoder_out: endpoint {
> +					remote-endpoint = <&panel_in>;
> +				};
> +			};
> +		};
> +	};
> +
> +	panel {
> +		compatible = "logictechno,lttd800480070-l6wh-rt";
> +		backlight = <&backlight>;
> +		power-supply = <&reg_tft_vcom>;
> +
> +		port {
> +			panel_in: endpoint {
> +				remote-endpoint = <&lvds_decoder_out>;
> +			};
> +		};
> +	};
> +
>  	touchscreen {
>  		compatible = "resistive-adc-touch";
>  		io-channels = <&adc_ts 1>, <&adc_ts 3>, <&adc_ts 4>, <&adc_ts 5>;
> @@ -78,6 +117,27 @@ channel@5 {
>  	};
>  };
>
> +&lcdif2 {
> +	status = "okay";
> +};
> +
> +&lvds_bridge {
> +	assigned-clocks = <&clk IMX8MP_CLK_MEDIA_LDB>,
> +				 <&clk IMX8MP_VIDEO_PLL1>;
> +	assigned-clock-parents = <&clk IMX8MP_VIDEO_PLL1_OUT>;
> +	/* IMX8MP_VIDEO_PLL1 = IMX8MP_CLK_MEDIA_DISP2_PIX * 2 * 7 */
> +	assigned-clock-rates = <0>, <462000000>;
> +	status = "okay";
> +
> +	ports {
> +		port@2 {
> +			ldb_lvds_ch1: endpoint {
> +				remote-endpoint = <&in_lvds1>;
> +			};
> +		};
> +	};
> +};
> +
>  &pwm1 {
>  	status = "okay";
>  };
>
> --
> 2.39.5
>

Re: [PATCH 04/10] arm64: dts: imx8mp-skov: describe HDMI display pipeline

[Ahmad Fatoum]

Hello Frank,

On 19.12.24 18:35, Frank Li wrote:
> On Thu, Dec 19, 2024 at 08:25:28AM +0100, Ahmad Fatoum wrote:
>> From: Oleksij Rempel <o.rempel@pengutronix.de>
>>
>> Back when this device tree was first added, there were no i.MX8MP
>> HDMI bindings upstream yet. This has changed now, so let's use them
>> to describe the HDMI on the imx8mp-skov-revb-hdmi board.
> 
> Skip your story.

I am sure you don't mean it this way, but the way you phrase it
sounds a bit rude.

> Just said
> 
> Add HDMI display pipeline for imx8mp-skov-revb-hdmi board.

That's already what my commit message title says.
I am not a fan of just repeating the commit message title
in the commit message body.

Unless you have a concrete problem with my commit message,
I'd prefer to keep it the way it is.

Thanks,
Ahmad

> 
> Frank
>>
>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  .../boot/dts/freescale/imx8mp-skov-revb-hdmi.dts      | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> 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 206116be8166..32a429437cbd 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-hdmi.dts
>> @@ -9,6 +9,21 @@ / {
>>  	compatible = "skov,imx8mp-skov-revb-hdmi", "fsl,imx8mp";
>>  };
>>
>> +&hdmi_pvi {
>> +	status = "okay";
>> +};
>> +
>> +&hdmi_tx {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_hdmi>;
>> +	ddc-i2c-bus = <&i2c5>;
>> +	status = "okay";
>> +};
>> +
>> +&hdmi_tx_phy {
>> +	status = "okay";
>> +};
>> +
>>  &i2c5 {
>>  	pinctrl-names = "default", "gpio";
>>  	pinctrl-0 = <&pinctrl_i2c5>;
>> @@ -19,6 +34,10 @@ &i2c5 {
>>  	status = "okay";
>>  };
>>
>> +&lcdif3 {
>> +	status = "okay";
>> +};
>> +
>>  &iomuxc {
>>  	pinctrl_hdmi: hdmigrp {
>>  		fsl,pins = <
>>
>> --
>> 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 07/10] arm64: dts: imx8mp-skov: configure uart1 for RS485

[Frank Li]

On Thu, Dec 19, 2024 at 08:25:31AM +0100, Ahmad Fatoum wrote:
> uart1 TX, RX and RTS signals go off the base board and to a RS485
> transceiver. Describe this in the device tree.

Look like it is not related what your did in this patch.

Add uart-has-rtscts in uart1 to ... (because ....)

Frank
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
> index 7ae686d37dda..bce23d62cd27 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
> @@ -404,6 +404,13 @@ &uart1 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_uart1>;
>  	status = "okay";
> +	/*
> +	 * While there is no CTS line, the property "uart-has-rtscts" is still
> +	 * the right thing to do to enable the UART to do RS485. In RS485-Mode
> +	 * CTS isn't used anyhow and there is no dedicated property
> +	 * "uart-has-rts-but-no-cts".
> +	 */
> +	uart-has-rtscts;
>  };
>
>  &uart2 {
> @@ -618,6 +625,8 @@ pinctrl_uart1: uart1grp {
>  			MX8MP_IOMUXC_UART1_RXD__UART1_DCE_RX			0x140
>  			MX8MP_IOMUXC_UART1_TXD__UART1_DCE_TX			0x140
>  			MX8MP_IOMUXC_UART3_RXD__UART1_DTE_RTS			0x140
> +			/* CTS pin is not connected, but needed as workaround */

what' means here! what's problem to be workaround?

> +			MX8MP_IOMUXC_UART3_TXD__UART1_DTE_CTS			0x140
>  		>;
>  	};
>
>
> --
> 2.39.5
>

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

[Ahmad Fatoum]

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 06/10] arm64: dts: imx8mp-skov: describe LVDS display pipeline

[Ahmad Fatoum]

On 19.12.24 18:37, Frank Li wrote:
> On Thu, Dec 19, 2024 at 08:25:30AM +0100, Ahmad Fatoum wrote:
>> From: Oleksij Rempel <o.rempel@pengutronix.de>
>>
>> The Skov i.MX8MP LT6 device tree so far described the touch screen, but
>> didn't describe the screen itself. Fill out these missing pieces.
> 
> Add "ti,sn65lvds822" touch screen description for i.MX8MP LT6 board.

This is not what this patch does. Touch screen was already described
and now the remainder is being added, just like what my message said.

Thanks,
Ahmad

> 
> Frank
> 
>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  .../boot/dts/freescale/imx8mp-skov-revb-lt6.dts    | 60 ++++++++++++++++++++++
>>  1 file changed, 60 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts
>> index ccbd3abedd69..baecf768a2ee 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts
>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts
>> @@ -8,6 +8,45 @@ / {
>>  	model = "SKOV IMX8MP CPU revB - LT6";
>>  	compatible = "skov,imx8mp-skov-revb-lt6", "fsl,imx8mp";
>>
>> +	lvds-decoder {
>> +		compatible = "ti,sn65lvds822", "lvds-decoder";
>> +		power-supply = <&reg_3v3>;
>> +
>> +		ports {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			port@0 {
>> +				reg = <0>;
>> +
>> +				in_lvds1: endpoint {
>> +					data-mapping = "vesa-24";
>> +					remote-endpoint = <&ldb_lvds_ch1>;
>> +				};
>> +			};
>> +
>> +			port@1 {
>> +				reg = <1>;
>> +
>> +				lvds_decoder_out: endpoint {
>> +					remote-endpoint = <&panel_in>;
>> +				};
>> +			};
>> +		};
>> +	};
>> +
>> +	panel {
>> +		compatible = "logictechno,lttd800480070-l6wh-rt";
>> +		backlight = <&backlight>;
>> +		power-supply = <&reg_tft_vcom>;
>> +
>> +		port {
>> +			panel_in: endpoint {
>> +				remote-endpoint = <&lvds_decoder_out>;
>> +			};
>> +		};
>> +	};
>> +
>>  	touchscreen {
>>  		compatible = "resistive-adc-touch";
>>  		io-channels = <&adc_ts 1>, <&adc_ts 3>, <&adc_ts 4>, <&adc_ts 5>;
>> @@ -78,6 +117,27 @@ channel@5 {
>>  	};
>>  };
>>
>> +&lcdif2 {
>> +	status = "okay";
>> +};
>> +
>> +&lvds_bridge {
>> +	assigned-clocks = <&clk IMX8MP_CLK_MEDIA_LDB>,
>> +				 <&clk IMX8MP_VIDEO_PLL1>;
>> +	assigned-clock-parents = <&clk IMX8MP_VIDEO_PLL1_OUT>;
>> +	/* IMX8MP_VIDEO_PLL1 = IMX8MP_CLK_MEDIA_DISP2_PIX * 2 * 7 */
>> +	assigned-clock-rates = <0>, <462000000>;
>> +	status = "okay";
>> +
>> +	ports {
>> +		port@2 {
>> +			ldb_lvds_ch1: endpoint {
>> +				remote-endpoint = <&in_lvds1>;
>> +			};
>> +		};
>> +	};
>> +};
>> +
>>  &pwm1 {
>>  	status = "okay";
>>  };
>>
>> --
>> 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 09/10] arm64: dts: imx8mp-skov: fix phy-mode

[Frank Li]

On Thu, Dec 19, 2024 at 08:25:33AM +0100, Ahmad Fatoum wrote:
> From: Oleksij Rempel <o.rempel@pengutronix.de>
>
> Use rgmii-rxid instead of rgmii-txid. It should have no effect here,
> but rxid is the correct value in our case.

Use rgmii-rxid instead of rgmii-txid because ...

Frank

>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
> index 80d212acc0c3..a683f46fcbab 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
> @@ -215,7 +215,7 @@ adc: adc@0 {
>  &eqos {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_eqos>;
> -	phy-mode = "rgmii-txid";
> +	phy-mode = "rgmii-rxid";
>  	status = "okay";
>
>  	fixed-link {
>
> --
> 2.39.5
>

Re: [PATCH 10/10] arm64: dts: imx8mp-skov: increase I2C clock frequency for RTC

[Frank Li]

On Thu, Dec 19, 2024 at 08:25:34AM +0100, Ahmad Fatoum wrote:
> From: Oleksij Rempel <o.rempel@pengutronix.de>
>
> The NXP PCF85063TP RTC we use is capable of up to 400 kHz of SCL clock
> frequency, so let's make use of this instead of the 100 kHz bus frequency
> we are currently using.

Increase I2C frequency to 400khz from 100kHz because NXP PCF85063TP RTC
support it.

Frank

>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
> index a683f46fcbab..ec7857db7bf0 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
> @@ -333,7 +333,7 @@ reg_nvcc_sd2: LDO5 {
>  };
>
>  &i2c3 {
> -	clock-frequency = <100000>;
> +	clock-frequency = <400000>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_i2c3>;
>  	status = "okay";
>
> --
> 2.39.5
>

Re: [PATCH 07/10] arm64: dts: imx8mp-skov: configure uart1 for RS485

[Ahmad Fatoum]

On 19.12.24 18:42, Frank Li wrote:
> On Thu, Dec 19, 2024 at 08:25:31AM +0100, Ahmad Fatoum wrote:
>> uart1 TX, RX and RTS signals go off the base board and to a RS485
>> transceiver. Describe this in the device tree.
> 
> Look like it is not related what your did in this patch.
> 
> Add uart-has-rtscts in uart1 to ... (because ....)

because they go off to the base board to a RS485 transceiver?

>> +	/*
>> +	 * While there is no CTS line, the property "uart-has-rtscts" is still
>> +	 * the right thing to do to enable the UART to do RS485. In RS485-Mode
>> +	 * CTS isn't used anyhow and there is no dedicated property
>> +	 * "uart-has-rts-but-no-cts".
>> +	 */
>> +	uart-has-rtscts;
>>  };
>>
>>  &uart2 {
>> @@ -618,6 +625,8 @@ pinctrl_uart1: uart1grp {
>>  			MX8MP_IOMUXC_UART1_RXD__UART1_DCE_RX			0x140
>>  			MX8MP_IOMUXC_UART1_TXD__UART1_DCE_TX			0x140
>>  			MX8MP_IOMUXC_UART3_RXD__UART1_DTE_RTS			0x140
>> +			/* CTS pin is not connected, but needed as workaround */
> 
> what' means here! what's problem to be workaround?

See the more descriptive comment above. We don't actually have CTS connected in hardware,
but we mux it anyway, so the i.MX UART is cleared to send always.

Thanks,
Ahmad

> 
>> +			MX8MP_IOMUXC_UART3_TXD__UART1_DTE_CTS			0x140
>>  		>;
>>  	};
>>
>>
>> --
>> 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 09/10] arm64: dts: imx8mp-skov: fix phy-mode

[Ahmad Fatoum]

On 19.12.24 18:45, Frank Li wrote:
> On Thu, Dec 19, 2024 at 08:25:33AM +0100, Ahmad Fatoum wrote:
>> From: Oleksij Rempel <o.rempel@pengutronix.de>
>>
>> Use rgmii-rxid instead of rgmii-txid. It should have no effect here,
>> but rxid is the correct value in our case.
> 
> Use rgmii-rxid instead of rgmii-txid because ...

I consider it useful information to note that it should introduce no
functional change.

Thanks,
Ahmad

> 
> Frank
> 
>>
>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
>> index 80d212acc0c3..a683f46fcbab 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
>> @@ -215,7 +215,7 @@ adc: adc@0 {
>>  &eqos {
>>  	pinctrl-names = "default";
>>  	pinctrl-0 = <&pinctrl_eqos>;
>> -	phy-mode = "rgmii-txid";
>> +	phy-mode = "rgmii-rxid";
>>  	status = "okay";
>>
>>  	fixed-link {
>>
>> --
>> 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 10/10] arm64: dts: imx8mp-skov: increase I2C clock frequency for RTC

[Ahmad Fatoum]

On 19.12.24 18:47, Frank Li wrote:
> On Thu, Dec 19, 2024 at 08:25:34AM +0100, Ahmad Fatoum wrote:
>> From: Oleksij Rempel <o.rempel@pengutronix.de>
>>
>> The NXP PCF85063TP RTC we use is capable of up to 400 kHz of SCL clock
>> frequency, so let's make use of this instead of the 100 kHz bus frequency
>> we are currently using.
> 
> Increase I2C frequency to 400khz from 100kHz because NXP PCF85063TP RTC
> support it.

Unlike your other suggestions, these is no information lost by rewriting
the commit message as you suggest. I don't mind, but must admit it feels
like bikeshedding. What is your concrete objection to my commit message?

Thanks,
Ahmad


> 
> Frank
> 
>>
>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
>> index a683f46fcbab..ec7857db7bf0 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
>> @@ -333,7 +333,7 @@ reg_nvcc_sd2: LDO5 {
>>  };
>>
>>  &i2c3 {
>> -	clock-frequency = <100000>;
>> +	clock-frequency = <400000>;
>>  	pinctrl-names = "default";
>>  	pinctrl-0 = <&pinctrl_i2c3>;
>>  	status = "okay";
>>
>> --
>> 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 10/10] arm64: dts: imx8mp-skov: increase I2C clock frequency for RTC

[Frank Li]

On Thu, Dec 19, 2024 at 06:51:35PM +0100, Ahmad Fatoum wrote:
> On 19.12.24 18:47, Frank Li wrote:
> > On Thu, Dec 19, 2024 at 08:25:34AM +0100, Ahmad Fatoum wrote:
> >> From: Oleksij Rempel <o.rempel@pengutronix.de>
> >>
> >> The NXP PCF85063TP RTC we use is capable of up to 400 kHz of SCL clock
> >> frequency, so let's make use of this instead of the 100 kHz bus frequency
> >> we are currently using.
> >
> > Increase I2C frequency to 400khz from 100kHz because NXP PCF85063TP RTC
> > support it.
>
> Unlike your other suggestions, these is no information lost by rewriting
> the commit message as you suggest. I don't mind, but must admit it feels
> like bikeshedding. What is your concrete objection to my commit message?

According to
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.

Generally, avoid use

"this patch ..."
"let's ...."
"we do ... for ... "

Just simple said

Do ... to ...
Do ... because ...

Frank

>
> Thanks,
> Ahmad
>
>
> >
> > Frank
> >
> >>
> >> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >>  arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
> >> index a683f46fcbab..ec7857db7bf0 100644
> >> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
> >> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
> >> @@ -333,7 +333,7 @@ reg_nvcc_sd2: LDO5 {
> >>  };
> >>
> >>  &i2c3 {
> >> -	clock-frequency = <100000>;
> >> +	clock-frequency = <400000>;
> >>  	pinctrl-names = "default";
> >>  	pinctrl-0 = <&pinctrl_i2c3>;
> >>  	status = "okay";
> >>
> >> --
> >> 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 06/10] arm64: dts: imx8mp-skov: describe LVDS display pipeline

[Frank Li]

On Thu, Dec 19, 2024 at 06:45:24PM +0100, Ahmad Fatoum wrote:
> On 19.12.24 18:37, Frank Li wrote:
> > On Thu, Dec 19, 2024 at 08:25:30AM +0100, Ahmad Fatoum wrote:
> >> From: Oleksij Rempel <o.rempel@pengutronix.de>
> >>
> >> The Skov i.MX8MP LT6 device tree so far described the touch screen, but
> >> didn't describe the screen itself. Fill out these missing pieces.
> >
> > Add "ti,sn65lvds822" touch screen description for i.MX8MP LT6 board.
>
> This is not what this patch does. Touch screen was already described
> and now the remainder is being added, just like what my message said.

My main means, use below order to descript commit message

Add ... for ...

Frank

>
> Thanks,
> Ahmad
>
> >
> > Frank
> >
> >> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >>  .../boot/dts/freescale/imx8mp-skov-revb-lt6.dts    | 60 ++++++++++++++++++++++
> >>  1 file changed, 60 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts
> >> index ccbd3abedd69..baecf768a2ee 100644
> >> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts
> >> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts
> >> @@ -8,6 +8,45 @@ / {
> >>  	model = "SKOV IMX8MP CPU revB - LT6";
> >>  	compatible = "skov,imx8mp-skov-revb-lt6", "fsl,imx8mp";
> >>
> >> +	lvds-decoder {
> >> +		compatible = "ti,sn65lvds822", "lvds-decoder";
> >> +		power-supply = <&reg_3v3>;
> >> +
> >> +		ports {
> >> +			#address-cells = <1>;
> >> +			#size-cells = <0>;
> >> +
> >> +			port@0 {
> >> +				reg = <0>;
> >> +
> >> +				in_lvds1: endpoint {
> >> +					data-mapping = "vesa-24";
> >> +					remote-endpoint = <&ldb_lvds_ch1>;
> >> +				};
> >> +			};
> >> +
> >> +			port@1 {
> >> +				reg = <1>;
> >> +
> >> +				lvds_decoder_out: endpoint {
> >> +					remote-endpoint = <&panel_in>;
> >> +				};
> >> +			};
> >> +		};
> >> +	};
> >> +
> >> +	panel {
> >> +		compatible = "logictechno,lttd800480070-l6wh-rt";
> >> +		backlight = <&backlight>;
> >> +		power-supply = <&reg_tft_vcom>;
> >> +
> >> +		port {
> >> +			panel_in: endpoint {
> >> +				remote-endpoint = <&lvds_decoder_out>;
> >> +			};
> >> +		};
> >> +	};
> >> +
> >>  	touchscreen {
> >>  		compatible = "resistive-adc-touch";
> >>  		io-channels = <&adc_ts 1>, <&adc_ts 3>, <&adc_ts 4>, <&adc_ts 5>;
> >> @@ -78,6 +117,27 @@ channel@5 {
> >>  	};
> >>  };
> >>
> >> +&lcdif2 {
> >> +	status = "okay";
> >> +};
> >> +
> >> +&lvds_bridge {
> >> +	assigned-clocks = <&clk IMX8MP_CLK_MEDIA_LDB>,
> >> +				 <&clk IMX8MP_VIDEO_PLL1>;
> >> +	assigned-clock-parents = <&clk IMX8MP_VIDEO_PLL1_OUT>;
> >> +	/* IMX8MP_VIDEO_PLL1 = IMX8MP_CLK_MEDIA_DISP2_PIX * 2 * 7 */
> >> +	assigned-clock-rates = <0>, <462000000>;
> >> +	status = "okay";
> >> +
> >> +	ports {
> >> +		port@2 {
> >> +			ldb_lvds_ch1: endpoint {
> >> +				remote-endpoint = <&in_lvds1>;
> >> +			};
> >> +		};
> >> +	};
> >> +};
> >> +
> >>  &pwm1 {
> >>  	status = "okay";
> >>  };
> >>
> >> --
> >> 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

[Frank Li]

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 10/10] arm64: dts: imx8mp-skov: increase I2C clock frequency for RTC

[Ahmad Fatoum]

Hello Frank,

On 19.12.24 20:16, Frank Li wrote:
> On Thu, Dec 19, 2024 at 06:51:35PM +0100, Ahmad Fatoum wrote:
>> On 19.12.24 18:47, Frank Li wrote:
>>> On Thu, Dec 19, 2024 at 08:25:34AM +0100, Ahmad Fatoum wrote:
>>>> From: Oleksij Rempel <o.rempel@pengutronix.de>
>>>>
>>>> The NXP PCF85063TP RTC we use is capable of up to 400 kHz of SCL clock
>>>> frequency, so let's make use of this instead of the 100 kHz bus frequency
>>>> we are currently using.
>>>
>>> Increase I2C frequency to 400khz from 100kHz because NXP PCF85063TP RTC
>>> support it.
>>
>> Unlike your other suggestions, these is no information lost by rewriting
>> the commit message as you suggest. I don't mind, but must admit it feels
>> like bikeshedding. What is your concrete objection to my commit message?
> 
> According to
> 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.

I have to disagree with your interpretation. The imperative mood is primarily
expected for commit message titles, but if you take a look at normal commit
message _bodies_ that make it into the kernel, you'll find that a whole lot
of them start off like I do: Describe the situation and then what's done about
it.

I personally find that this is often easier to follow than just having two
imperatives back-to-back.

I have just Cc'd you on an RFC patch to amend the documentation you linked
to reflect this. I am happy to continue the discussion over there:

https://lore.kernel.org/workflows/20241220-submitting-patches-imperative-v1-0-ee874c1859b3@pengutronix.de/T/#t

Thanks,
Ahmad

> 
> Generally, avoid use
> 
> "this patch ..."
> "let's ...."
> "we do ... for ... "
> 
> Just simple said
> 
> Do ... to ...
> Do ... because ...
> 
> Frank
> 
>>
>> Thanks,
>> Ahmad
>>
>>
>>>
>>> Frank
>>>
>>>>
>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>>> ---
>>>>  arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
>>>> index a683f46fcbab..ec7857db7bf0 100644
>>>> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
>>>> @@ -333,7 +333,7 @@ reg_nvcc_sd2: LDO5 {
>>>>  };
>>>>
>>>>  &i2c3 {
>>>> -	clock-frequency = <100000>;
>>>> +	clock-frequency = <400000>;
>>>>  	pinctrl-names = "default";
>>>>  	pinctrl-0 = <&pinctrl_i2c3>;
>>>>  	status = "okay";
>>>>
>>>> --
>>>> 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 |

Re: [PATCH 10/10] arm64: dts: imx8mp-skov: increase I2C clock frequency for RTC

[Frank Li]

On Fri, Dec 20, 2024 at 10:12:49AM +0100, Ahmad Fatoum wrote:
> Hello Frank,
>
> On 19.12.24 20:16, Frank Li wrote:
> > On Thu, Dec 19, 2024 at 06:51:35PM +0100, Ahmad Fatoum wrote:
> >> On 19.12.24 18:47, Frank Li wrote:
> >>> On Thu, Dec 19, 2024 at 08:25:34AM +0100, Ahmad Fatoum wrote:
> >>>> From: Oleksij Rempel <o.rempel@pengutronix.de>
> >>>>
> >>>> The NXP PCF85063TP RTC we use is capable of up to 400 kHz of SCL clock
> >>>> frequency, so let's make use of this instead of the 100 kHz bus frequency
> >>>> we are currently using.
> >>>
> >>> Increase I2C frequency to 400khz from 100kHz because NXP PCF85063TP RTC
> >>> support it.
> >>
> >> Unlike your other suggestions, these is no information lost by rewriting
> >> the commit message as you suggest. I don't mind, but must admit it feels
> >> like bikeshedding. What is your concrete objection to my commit message?
> >
> > According to
> > 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.
>
> I have to disagree with your interpretation. The imperative mood is primarily
> expected for commit message titles, but if you take a look at normal commit
> message _bodies_ that make it into the kernel, you'll find that a whole lot
> of them start off like I do: Describe the situation and then what's done about
> it.

I was got many similar feedback from difference maintainer to be required
change my commit message _bodies_ in past years.

https://lore.kernel.org/imx/3c9fe85a-5f86-4df6-92fb-e4ceb033f161@kernel.org/
https://lore.kernel.org/imx/117dd6f3-8829-4000-a05b-6cb421ca7de6@kernel.org/
https://lore.kernel.org/linux-pci/20240807025423.GF3412@thinkpad/
https://lore.kernel.org/linux-pci/YnvyrSTAxJmGxful@lpieralisi/

At beginning, I can't understand this. But after follow these expertor
suggestion, I found some beanfit.
- sentense will be shorter and easy to capture most important part. for
example:

  "The NXP PCF85063TP RTC we use is capable of up to 400 kHz of SCL clock
  frequency, so let's make use of this instead of the 100 kHz bus frequency
  we are currently using."

  34 words

  "Increase I2C frequency to 400khz from 100kHz because NXP PCF85063TP RTC
   support it"

  13 words

  Linux is big community, even reduce 1 minutes to read it, multi by
totall reviewer/reader will be very big numbers.

- empahse the important change first to help understand whole patch
quickly.

You can choose what you want if maintainer don't reject it. Generally I
just send such kind comments for v1 patch, because it is less possible to
be accepted by by maintainer to accept (except some critial fixes).

>
> I personally find that this is often easier to follow than just having two
> imperatives back-to-back.

Basic principle is clear, simple, and straightforward.

Frank

>
> I have just Cc'd you on an RFC patch to amend the documentation you linked
> to reflect this. I am happy to continue the discussion over there:
>
> https://lore.kernel.org/workflows/20241220-submitting-patches-imperative-v1-0-ee874c1859b3@pengutronix.de/T/#t
>
> Thanks,
> Ahmad
>
> >
> > Generally, avoid use
> >
> > "this patch ..."
> > "let's ...."
> > "we do ... for ... "
> >
> > Just simple said
> >
> > Do ... to ...
> > Do ... because ...
> >
> > Frank
> >
> >>
> >> Thanks,
> >> Ahmad
> >>
> >>
> >>>
> >>> Frank
> >>>
> >>>>
> >>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> >>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >>>> ---
> >>>>  arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
> >>>> index a683f46fcbab..ec7857db7bf0 100644
> >>>> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
> >>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
> >>>> @@ -333,7 +333,7 @@ reg_nvcc_sd2: LDO5 {
> >>>>  };
> >>>>
> >>>>  &i2c3 {
> >>>> -	clock-frequency = <100000>;
> >>>> +	clock-frequency = <400000>;
> >>>>  	pinctrl-names = "default";
> >>>>  	pinctrl-0 = <&pinctrl_i2c3>;
> >>>>  	status = "okay";
> >>>>
> >>>> --
> >>>> 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 |

Re: [PATCH 05/10] dt-bindings: display/lvds-codec: add ti,sn65lvds822

[Rob Herring (Arm)]

On Thu, 19 Dec 2024 08:25:29 +0100, Ahmad Fatoum wrote:
> Add compatible strings for TI SN65LVDS822, a FlatLink LVDS receiver.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> To: Andrzej Hajda <andrzej.hajda@intel.com> (maintainer:DRM DRIVERS FOR BRIDGE CHIPS)
> To: Neil Armstrong <neil.armstrong@linaro.org> (maintainer:DRM DRIVERS FOR BRIDGE CHIPS)
> To: Robert Foss <rfoss@kernel.org> (maintainer:DRM DRIVERS FOR BRIDGE CHIPS)
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> (reviewer:DRM DRIVERS FOR BRIDGE CHIPS,in file)
> Cc: Jonas Karlman <jonas@kwiboo.se> (reviewer:DRM DRIVERS FOR BRIDGE CHIPS)
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com> (reviewer:DRM DRIVERS FOR BRIDGE CHIPS)
> Cc: David Airlie <airlied@gmail.com> (maintainer:DRM DRIVERS)
> Cc: Simona Vetter <simona@ffwll.ch> (maintainer:DRM DRIVERS)
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (maintainer:DRM DRIVERS AND MISC GPU PATCHES)
> Cc: Maxime Ripard <mripard@kernel.org> (maintainer:DRM DRIVERS AND MISC GPU PATCHES)
> Cc: Thomas Zimmermann <tzimmermann@suse.de> (maintainer:DRM DRIVERS AND MISC GPU PATCHES)
> Cc: dri-devel@lists.freedesktop.org (open list:DRM DRIVERS)
> ---
>  Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

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

[Ahmad Fatoum]

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 |

Re: [PATCH 06/10] arm64: dts: imx8mp-skov: describe LVDS display pipeline

[Ahmad Fatoum]

Hello Frank,

On 19.12.24 20:24, Frank Li wrote:
> On Thu, Dec 19, 2024 at 06:45:24PM +0100, Ahmad Fatoum wrote:
>> On 19.12.24 18:37, Frank Li wrote:
>>> On Thu, Dec 19, 2024 at 08:25:30AM +0100, Ahmad Fatoum wrote:
>>>> From: Oleksij Rempel <o.rempel@pengutronix.de>
>>>>
>>>> The Skov i.MX8MP LT6 device tree so far described the touch screen, but
>>>> didn't describe the screen itself. Fill out these missing pieces.
>>>
>>> Add "ti,sn65lvds822" touch screen description for i.MX8MP LT6 board.
>>
>> This is not what this patch does. Touch screen was already described
>> and now the remainder is being added, just like what my message said.
> 
> My main means, use below order to descript commit message
> 
> Add ... for ...

Sorry, I won't start every sentence with a verb. The commit message
is fine IMO.

Thanks,
Ahmad

> 
> Frank
> 
>>
>> Thanks,
>> Ahmad
>>
>>>
>>> Frank
>>>
>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>>> ---
>>>>  .../boot/dts/freescale/imx8mp-skov-revb-lt6.dts    | 60 ++++++++++++++++++++++
>>>>  1 file changed, 60 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts
>>>> index ccbd3abedd69..baecf768a2ee 100644
>>>> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts
>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-lt6.dts
>>>> @@ -8,6 +8,45 @@ / {
>>>>  	model = "SKOV IMX8MP CPU revB - LT6";
>>>>  	compatible = "skov,imx8mp-skov-revb-lt6", "fsl,imx8mp";
>>>>
>>>> +	lvds-decoder {
>>>> +		compatible = "ti,sn65lvds822", "lvds-decoder";
>>>> +		power-supply = <&reg_3v3>;
>>>> +
>>>> +		ports {
>>>> +			#address-cells = <1>;
>>>> +			#size-cells = <0>;
>>>> +
>>>> +			port@0 {
>>>> +				reg = <0>;
>>>> +
>>>> +				in_lvds1: endpoint {
>>>> +					data-mapping = "vesa-24";
>>>> +					remote-endpoint = <&ldb_lvds_ch1>;
>>>> +				};
>>>> +			};
>>>> +
>>>> +			port@1 {
>>>> +				reg = <1>;
>>>> +
>>>> +				lvds_decoder_out: endpoint {
>>>> +					remote-endpoint = <&panel_in>;
>>>> +				};
>>>> +			};
>>>> +		};
>>>> +	};
>>>> +
>>>> +	panel {
>>>> +		compatible = "logictechno,lttd800480070-l6wh-rt";
>>>> +		backlight = <&backlight>;
>>>> +		power-supply = <&reg_tft_vcom>;
>>>> +
>>>> +		port {
>>>> +			panel_in: endpoint {
>>>> +				remote-endpoint = <&lvds_decoder_out>;
>>>> +			};
>>>> +		};
>>>> +	};
>>>> +
>>>>  	touchscreen {
>>>>  		compatible = "resistive-adc-touch";
>>>>  		io-channels = <&adc_ts 1>, <&adc_ts 3>, <&adc_ts 4>, <&adc_ts 5>;
>>>> @@ -78,6 +117,27 @@ channel@5 {
>>>>  	};
>>>>  };
>>>>
>>>> +&lcdif2 {
>>>> +	status = "okay";
>>>> +};
>>>> +
>>>> +&lvds_bridge {
>>>> +	assigned-clocks = <&clk IMX8MP_CLK_MEDIA_LDB>,
>>>> +				 <&clk IMX8MP_VIDEO_PLL1>;
>>>> +	assigned-clock-parents = <&clk IMX8MP_VIDEO_PLL1_OUT>;
>>>> +	/* IMX8MP_VIDEO_PLL1 = IMX8MP_CLK_MEDIA_DISP2_PIX * 2 * 7 */
>>>> +	assigned-clock-rates = <0>, <462000000>;
>>>> +	status = "okay";
>>>> +
>>>> +	ports {
>>>> +		port@2 {
>>>> +			ldb_lvds_ch1: endpoint {
>>>> +				remote-endpoint = <&in_lvds1>;
>>>> +			};
>>>> +		};
>>>> +	};
>>>> +};
>>>> +
>>>>  &pwm1 {
>>>>  	status = "okay";
>>>>  };
>>>>
>>>> --
>>>> 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 |

Re: [PATCH 10/10] arm64: dts: imx8mp-skov: increase I2C clock frequency for RTC

[Ahmad Fatoum]

Hello Frank,

On 20.12.24 16:38, Frank Li wrote:
> On Fri, Dec 20, 2024 at 10:12:49AM +0100, Ahmad Fatoum wrote:
>> On 19.12.24 20:16, Frank Li wrote:
>>> On Thu, Dec 19, 2024 at 06:51:35PM +0100, Ahmad Fatoum wrote:
>>>> On 19.12.24 18:47, Frank Li wrote:
>>>>> On Thu, Dec 19, 2024 at 08:25:34AM +0100, Ahmad Fatoum wrote:
>>>>>> From: Oleksij Rempel <o.rempel@pengutronix.de>
>>> According to
>>> 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.
>>
>> I have to disagree with your interpretation. The imperative mood is primarily
>> expected for commit message titles, but if you take a look at normal commit
>> message _bodies_ that make it into the kernel, you'll find that a whole lot
>> of them start off like I do: Describe the situation and then what's done about
>> it.
> 
> I was got many similar feedback from difference maintainer to be required
> change my commit message _bodies_ in past years.
> 
> https://lore.kernel.org/imx/3c9fe85a-5f86-4df6-92fb-e4ceb033f161@kernel.org/

Surely, you can see a difference between marketing fluff and the commit
messages in my series?

> https://lore.kernel.org/imx/117dd6f3-8829-4000-a05b-6cb421ca7de6@kernel.org/

Agreed, I don't see where I talked about my patches in the third person though?

> https://lore.kernel.org/linux-pci/20240807025423.GF3412@thinkpad/

Did you check out how the final version of that commit log looked
like? Here it is:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=64706ba771f5e8f

You may notice that the maintainer has edited it for readability instead
of just having a few imperatives follow each other. :-)

> https://lore.kernel.org/linux-pci/YnvyrSTAxJmGxful@lpieralisi/
> 
> At beginning, I can't understand this. But after follow these expertor
> suggestion, I found some beanfit.

This commit message is indeed very vague. I agree that patch 09/10 in my
series is vague too, so this will be fixed for v2. I don't see this applicable
to other patches though.

> - sentense will be shorter and easy to capture most important part. for
> example:
> 
>   "The NXP PCF85063TP RTC we use is capable of up to 400 kHz of SCL clock
>   frequency, so let's make use of this instead of the 100 kHz bus frequency
>   we are currently using."
> 
>   34 words
> 
>   "Increase I2C frequency to 400khz from 100kHz because NXP PCF85063TP RTC
>    support it"
> 
>   13 words

The commit message will be shortened a bit for v2.

>   Linux is big community, even reduce 1 minutes to read it, multi by
> totall reviewer/reader will be very big numbers.
> 
> - empahse the important change first to help understand whole patch
> quickly.
> 
> You can choose what you want if maintainer don't reject it. Generally I
> just send such kind comments for v1 patch, because it is less possible to
> be accepted by by maintainer to accept (except some critial fixes).> 
>>
>> I personally find that this is often easier to follow than just having two
>> imperatives back-to-back.
> 
> Basic principle is clear, simple, and straightforward.

I still believe you are misunderstanding the guidelines in submitting-patches.rst
and that asking for all commit message bodies to be rewritten in imperative mood
doesn't help anyone.

I would have liked for both my patches against submitting-patches.rst to be accepted
to clarify this, but you can check out Jon's answer and compare with commit messages
not authored by you and perhaps see that one needn't golf the commit messages
at the expense of readability.

Thanks,
Ahmad

> 
> Frank
> 
>>
>> I have just Cc'd you on an RFC patch to amend the documentation you linked
>> to reflect this. I am happy to continue the discussion over there:
>>
>> https://lore.kernel.org/workflows/20241220-submitting-patches-imperative-v1-0-ee874c1859b3@pengutronix.de/T/#t
>>
>> Thanks,
>> Ahmad
>>
>>>
>>> Generally, avoid use
>>>
>>> "this patch ..."
>>> "let's ...."
>>> "we do ... for ... "
>>>
>>> Just simple said
>>>
>>> Do ... to ...
>>> Do ... because ...
>>>
>>> Frank
>>>
>>>>
>>>> Thanks,
>>>> Ahmad
>>>>
>>>>
>>>>>
>>>>> Frank
>>>>>
>>>>>>
>>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>>>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>>>>> ---
>>>>>>  arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
>>>>>> index a683f46fcbab..ec7857db7bf0 100644
>>>>>> --- a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
>>>>>> @@ -333,7 +333,7 @@ reg_nvcc_sd2: LDO5 {
>>>>>>  };
>>>>>>
>>>>>>  &i2c3 {
>>>>>> -	clock-frequency = <100000>;
>>>>>> +	clock-frequency = <400000>;
>>>>>>  	pinctrl-names = "default";
>>>>>>  	pinctrl-0 = <&pinctrl_i2c3>;
>>>>>>  	status = "okay";
>>>>>>
>>>>>> --
>>>>>> 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 |

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