diff mbox series

[V6,01/15] ARM: dts: imx6q-dhcom: Add the parallel system bus

Message ID 20210714210713.9015-1-cniedermaier@dh-electronics.com (mailing list archive)
State New, archived
Headers show
Series [V6,01/15] ARM: dts: imx6q-dhcom: Add the parallel system bus | expand

Commit Message

Christoph Niedermaier July 14, 2021, 9:06 p.m. UTC
Add the parallel system bus provided by the i.MX6 WEIM interface via an
address latch. The OE pin of the latch is controlled by a fixed regulator.
The pin is low active. This is ensured by omitting the regulators property
enable-active-high. The flags value of the gpio property (3rd value), which
is also use to define active high/low, is set to 0 because it is ignored
by gpiolib-of.c.

Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
Reviewed-by: Marek Vasut <marex@denx.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Fabio Estevam <festevam@denx.de>
Cc: Marek Vasut <marex@denx.de>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: kernel@dh-electronics.com
To: linux-arm-kernel@lists.infradead.org
---
V2: - Rework of the commit message
    - Add a comment to the regulator
    - Replace GPIO_ACTIVE_HIGH with 0 at the gpio property of the regulator
    - Remove spaces from the ranges property of the weim node
    - Rebase on Shawn Guos branch for-next
V3: - Add Reviewed-by tag
V4: - No changes
V5: - No changes
V6: - Rebase on 5.14-rc1
---
 arch/arm/boot/dts/imx6q-dhcom-som.dtsi | 57 ++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

Comments

Fabio Estevam July 18, 2021, 3:31 p.m. UTC | #1
Hi Christoph,

On 14/07/2021 18:06, Christoph Niedermaier wrote:

> +	/* OE pin of the latch is low active */
> +	reg_latch_oe_on: regulator-latch-oe-on {
> +		compatible = "regulator-fixed";
> +		gpio = <&gpio3 22 0>;

I understand that the GPIO polarity is ignored, but it would be better 
to
just describe the real polarity in the devicetree:

gpio = <&gpio3 22 GPIO_ACTIVE_LOW>;
Marek Vasut July 18, 2021, 4:21 p.m. UTC | #2
On 7/18/21 5:31 PM, Fabio Estevam wrote:
> Hi Christoph,
> 
> On 14/07/2021 18:06, Christoph Niedermaier wrote:
> 
>> +    /* OE pin of the latch is low active */
>> +    reg_latch_oe_on: regulator-latch-oe-on {
>> +        compatible = "regulator-fixed";
>> +        gpio = <&gpio3 22 0>;
> 
> I understand that the GPIO polarity is ignored, but it would be better to
> just describe the real polarity in the devicetree:
> 
> gpio = <&gpio3 22 GPIO_ACTIVE_LOW>;

The 0 is correct as the field is ignored, please keep it.

Any other variants (like GPIO_ACTIVE_LOW = 1) interact badly with
drivers/gpio/gpiolib-of.c of_gpio_flags_quirks()
which is already a total compatibility attempt mess and lead to odd 
misbehavior of the regulator where the polarity of the GPIO is randomly 
interpreted as low or high dependent on the kernel version.
Shawn Guo July 23, 2021, 3:56 a.m. UTC | #3
On Wed, Jul 14, 2021 at 11:06:59PM +0200, Christoph Niedermaier wrote:
> Add the parallel system bus provided by the i.MX6 WEIM interface via an
> address latch. The OE pin of the latch is controlled by a fixed regulator.
> The pin is low active. This is ensured by omitting the regulators property
> enable-active-high. The flags value of the gpio property (3rd value), which
> is also use to define active high/low, is set to 0 because it is ignored
> by gpiolib-of.c.
> 
> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
> Reviewed-by: Marek Vasut <marex@denx.de>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Fabio Estevam <festevam@denx.de>
> Cc: Marek Vasut <marex@denx.de>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: kernel@dh-electronics.com
> To: linux-arm-kernel@lists.infradead.org

Applied, thanks!
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/imx6q-dhcom-som.dtsi b/arch/arm/boot/dts/imx6q-dhcom-som.dtsi
index 4bf51f3ce003..921a695b79fb 100644
--- a/arch/arm/boot/dts/imx6q-dhcom-som.dtsi
+++ b/arch/arm/boot/dts/imx6q-dhcom-som.dtsi
@@ -46,6 +46,14 @@ 
 		vin-supply = <&sw2_reg>;
 	};
 
+	/* OE pin of the latch is low active */
+	reg_latch_oe_on: regulator-latch-oe-on {
+		compatible = "regulator-fixed";
+		gpio = <&gpio3 22 0>;
+		regulator-always-on;
+		regulator-name = "latch_oe_on";
+	};
+
 	reg_usb_otg_vbus: regulator-usb-otg-vbus {
 		compatible = "regulator-fixed";
 		regulator-name = "usb_otg_vbus";
@@ -455,6 +463,43 @@ 
 			MX6QDL_PAD_SD4_DAT7__SD4_DATA7		0x17059
 		>;
 	};
+
+	pinctrl_weim: weim-grp {
+		fsl,pins = <
+			MX6QDL_PAD_EIM_OE__EIM_OE_B		0xb0a6
+			MX6QDL_PAD_EIM_RW__EIM_RW		0xb0a6 /* WE */
+			MX6QDL_PAD_EIM_LBA__EIM_LBA_B		0xb060 /* LE */
+			MX6QDL_PAD_EIM_D22__GPIO3_IO22		0x130b0
+			MX6QDL_PAD_EIM_DA15__EIM_AD15		0xb0a6
+			MX6QDL_PAD_EIM_DA14__EIM_AD14		0xb0a6
+			MX6QDL_PAD_EIM_DA13__EIM_AD13		0xb0a6
+			MX6QDL_PAD_EIM_DA12__EIM_AD12		0xb0a6
+			MX6QDL_PAD_EIM_DA11__EIM_AD11		0xb0a6
+			MX6QDL_PAD_EIM_DA10__EIM_AD10		0xb0a6
+			MX6QDL_PAD_EIM_DA9__EIM_AD09		0xb0a6
+			MX6QDL_PAD_EIM_DA8__EIM_AD08		0xb0a6
+			MX6QDL_PAD_EIM_DA7__EIM_AD07		0xb0a6
+			MX6QDL_PAD_EIM_DA6__EIM_AD06		0xb0a6
+			MX6QDL_PAD_EIM_DA5__EIM_AD05		0xb0a6
+			MX6QDL_PAD_EIM_DA4__EIM_AD04		0xb0a6
+			MX6QDL_PAD_EIM_DA3__EIM_AD03		0xb0a6
+			MX6QDL_PAD_EIM_DA2__EIM_AD02		0xb0a6
+			MX6QDL_PAD_EIM_DA1__EIM_AD01		0xb0a6
+			MX6QDL_PAD_EIM_DA0__EIM_AD00		0xb0a6
+		>;
+	};
+
+	pinctrl_weim_cs0: weim-cs0-grp {
+		fsl,pins = <
+			MX6QDL_PAD_EIM_CS0__EIM_CS0_B		0xb0b1
+		>;
+	};
+
+	pinctrl_weim_cs1: weim-cs1-grp {
+		fsl,pins = <
+			MX6QDL_PAD_EIM_CS1__EIM_CS1_B		0xb0b1
+		>;
+	};
 };
 
 &reg_arm {
@@ -544,3 +589,15 @@ 
 	keep-power-in-suspend;
 	status = "okay";
 };
+
+&weim {
+	#address-cells = <2>;
+	#size-cells = <1>;
+	fsl,weim-cs-gpr = <&gpr>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_weim &pinctrl_weim_cs0 &pinctrl_weim_cs1>;
+	/* It is necessary to setup 2x 64MB otherwise setting gpr fails */
+	ranges = <0 0 0x08000000 0x04000000>, /* CS0 */
+		 <1 0 0x0c000000 0x04000000>; /* CS1 */
+	status = "disabled";
+};