All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v3 0/5] DM: Convert i.MX28 gpio, pinmux, spi and eth drivers to DM/DTS
@ 2019-06-15 22:34 Lukasz Majewski
  2019-06-15 22:34 ` [U-Boot] [PATCH v3 1/5] ARM: dts: imx: Copy imx28 device tree related files from Linux kernel (v5.1.9) Lukasz Majewski
                   ` (4 more replies)
  0 siblings, 5 replies; 32+ messages in thread
From: Lukasz Majewski @ 2019-06-15 22:34 UTC (permalink / raw)
  To: u-boot

This patch series converts some i.MX28 drivers to use DM/DTS.

Converted drivers:
- mxs_gpio.c - gpio driver
- pinctrl-mxs.c - pinmux driver
- Adjust fec_eth.c to support also i.MX28 SoC.
- mxs_spi.c - SPI driver

After applying this series it is possible to use ETH on i.MX28
board with following DTS description:

        &mac0 {
    	phy-mode = "rmii";
    	pinctrl-names = "default";
    	pinctrl-0 = <&mac0_pins_a>;
    	phy-supply = <&reg_fec_3v3>;
    	phy-reset-gpios = <&gpio2 13 GPIO_ACTIVE_LOW>;
    	phy-reset-duration = <1>;
    	phy-reset-post-delay = <1>;
    	status = "okay";

    	fixed-link {
    	      speed = <100>;
    	      full-duplex;
    	};
};

And store the received binaries on SPI-NOR memory.

Travis-CI:
https://travis-ci.org/lmajewski/u-boot-dfu/builds/545476866

Applied on top of u-boot/master branch
SHA1: c2ea87883ef309570c8903e6de4b8b78685d73d0


Changes in v3:
- Update tag to 5.1.9
- Set more apropriate tags
- Set more apropriate tags
- Set more apropriate tags
- Set more apropriate tags
- Set more apropriate tags

Changes in v2:
- Use #if !CONFIG_IS_ENABLED(DM_GPIO) instead of plain #ifdef CONFIG_DM_GPIO
- New patch (conversion of mxs_spi.c to DM_SPI)

Lukasz Majewski (5):
  ARM: dts: imx: Copy imx28 device tree related files from Linux kernel
    (v5.1.9)
  ARM: imx: net: Enable support for i.MX28 DM_ETH in the fec_mxc.c
    driver
  ARM: dm: mxs: gpio: Add support for DM/DTS in the mxs_gpio.c driver
    (DM_GPIO)
  ARM: imx: pinctrl: Add support for i.MX28 mxs pinctrl driver
  ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI
    conversion)

 arch/arm/dts/imx28-pinfunc.h         |  506 +++++++++++++
 arch/arm/dts/imx28.dtsi              | 1330 ++++++++++++++++++++++++++++++++++
 arch/arm/dts/mxs-pinfunc.h           |   31 +
 arch/arm/include/asm/arch-mxs/gpio.h |   28 +
 drivers/gpio/mxs_gpio.c              |  149 ++++
 drivers/net/Kconfig                  |    2 +-
 drivers/net/fec_mxc.c                |    1 +
 drivers/pinctrl/nxp/Kconfig          |   10 +
 drivers/pinctrl/nxp/Makefile         |    1 +
 drivers/pinctrl/nxp/pinctrl-mxs.c    |  164 +++++
 drivers/spi/mxs_spi.c                |  393 +++++++---
 11 files changed, 2531 insertions(+), 84 deletions(-)
 create mode 100644 arch/arm/dts/imx28-pinfunc.h
 create mode 100644 arch/arm/dts/imx28.dtsi
 create mode 100644 arch/arm/dts/mxs-pinfunc.h
 create mode 100644 drivers/pinctrl/nxp/pinctrl-mxs.c

-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 1/5] ARM: dts: imx: Copy imx28 device tree related files from Linux kernel (v5.1.9)
  2019-06-15 22:34 [U-Boot] [PATCH v3 0/5] DM: Convert i.MX28 gpio, pinmux, spi and eth drivers to DM/DTS Lukasz Majewski
@ 2019-06-15 22:34 ` Lukasz Majewski
  2019-06-15 22:43   ` Marek Vasut
  2019-06-15 22:34 ` [U-Boot] [PATCH v3 2/5] ARM: imx: net: Enable support for i.MX28 DM_ETH in the fec_mxc.c driver Lukasz Majewski
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 32+ messages in thread
From: Lukasz Majewski @ 2019-06-15 22:34 UTC (permalink / raw)
  To: u-boot

This commit copies from the Linux kernel (tag v5.1.9) i.MX28 related
device tree files.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---

Changes in v3:
- Update tag to 5.1.9
- Set more apropriate tags

Changes in v2: None

 arch/arm/dts/imx28-pinfunc.h |  506 ++++++++++++++++
 arch/arm/dts/imx28.dtsi      | 1330 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm/dts/mxs-pinfunc.h   |   31 +
 3 files changed, 1867 insertions(+)
 create mode 100644 arch/arm/dts/imx28-pinfunc.h
 create mode 100644 arch/arm/dts/imx28.dtsi
 create mode 100644 arch/arm/dts/mxs-pinfunc.h

diff --git a/arch/arm/dts/imx28-pinfunc.h b/arch/arm/dts/imx28-pinfunc.h
new file mode 100644
index 0000000000..e11f69ba0f
--- /dev/null
+++ b/arch/arm/dts/imx28-pinfunc.h
@@ -0,0 +1,506 @@
+/*
+ * Header providing constants for i.MX28 pinctrl bindings.
+ *
+ * Copyright (C) 2013 Lothar Waßmann <LW@KARO-electronics.de>
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#ifndef __DT_BINDINGS_MX28_PINCTRL_H__
+#define __DT_BINDINGS_MX28_PINCTRL_H__
+
+#include "mxs-pinfunc.h"
+
+#define MX28_PAD_GPMI_D00__GPMI_D0			0x0000
+#define MX28_PAD_GPMI_D01__GPMI_D1			0x0010
+#define MX28_PAD_GPMI_D02__GPMI_D2			0x0020
+#define MX28_PAD_GPMI_D03__GPMI_D3			0x0030
+#define MX28_PAD_GPMI_D04__GPMI_D4			0x0040
+#define MX28_PAD_GPMI_D05__GPMI_D5			0x0050
+#define MX28_PAD_GPMI_D06__GPMI_D6			0x0060
+#define MX28_PAD_GPMI_D07__GPMI_D7			0x0070
+#define MX28_PAD_GPMI_CE0N__GPMI_CE0N			0x0100
+#define MX28_PAD_GPMI_CE1N__GPMI_CE1N			0x0110
+#define MX28_PAD_GPMI_CE2N__GPMI_CE2N			0x0120
+#define MX28_PAD_GPMI_CE3N__GPMI_CE3N			0x0130
+#define MX28_PAD_GPMI_RDY0__GPMI_READY0			0x0140
+#define MX28_PAD_GPMI_RDY1__GPMI_READY1			0x0150
+#define MX28_PAD_GPMI_RDY2__GPMI_READY2			0x0160
+#define MX28_PAD_GPMI_RDY3__GPMI_READY3			0x0170
+#define MX28_PAD_GPMI_RDN__GPMI_RDN			0x0180
+#define MX28_PAD_GPMI_WRN__GPMI_WRN			0x0190
+#define MX28_PAD_GPMI_ALE__GPMI_ALE			0x01a0
+#define MX28_PAD_GPMI_CLE__GPMI_CLE			0x01b0
+#define MX28_PAD_GPMI_RESETN__GPMI_RESETN		0x01c0
+#define MX28_PAD_LCD_D00__LCD_D0			0x1000
+#define MX28_PAD_LCD_D01__LCD_D1			0x1010
+#define MX28_PAD_LCD_D02__LCD_D2			0x1020
+#define MX28_PAD_LCD_D03__LCD_D3			0x1030
+#define MX28_PAD_LCD_D04__LCD_D4			0x1040
+#define MX28_PAD_LCD_D05__LCD_D5			0x1050
+#define MX28_PAD_LCD_D06__LCD_D6			0x1060
+#define MX28_PAD_LCD_D07__LCD_D7			0x1070
+#define MX28_PAD_LCD_D08__LCD_D8			0x1080
+#define MX28_PAD_LCD_D09__LCD_D9			0x1090
+#define MX28_PAD_LCD_D10__LCD_D10			0x10a0
+#define MX28_PAD_LCD_D11__LCD_D11			0x10b0
+#define MX28_PAD_LCD_D12__LCD_D12			0x10c0
+#define MX28_PAD_LCD_D13__LCD_D13			0x10d0
+#define MX28_PAD_LCD_D14__LCD_D14			0x10e0
+#define MX28_PAD_LCD_D15__LCD_D15			0x10f0
+#define MX28_PAD_LCD_D16__LCD_D16			0x1100
+#define MX28_PAD_LCD_D17__LCD_D17			0x1110
+#define MX28_PAD_LCD_D18__LCD_D18			0x1120
+#define MX28_PAD_LCD_D19__LCD_D19			0x1130
+#define MX28_PAD_LCD_D20__LCD_D20			0x1140
+#define MX28_PAD_LCD_D21__LCD_D21			0x1150
+#define MX28_PAD_LCD_D22__LCD_D22			0x1160
+#define MX28_PAD_LCD_D23__LCD_D23			0x1170
+#define MX28_PAD_LCD_RD_E__LCD_RD_E			0x1180
+#define MX28_PAD_LCD_WR_RWN__LCD_WR_RWN			0x1190
+#define MX28_PAD_LCD_RS__LCD_RS				0x11a0
+#define MX28_PAD_LCD_CS__LCD_CS				0x11b0
+#define MX28_PAD_LCD_VSYNC__LCD_VSYNC			0x11c0
+#define MX28_PAD_LCD_HSYNC__LCD_HSYNC			0x11d0
+#define MX28_PAD_LCD_DOTCLK__LCD_DOTCLK			0x11e0
+#define MX28_PAD_LCD_ENABLE__LCD_ENABLE			0x11f0
+#define MX28_PAD_SSP0_DATA0__SSP0_D0			0x2000
+#define MX28_PAD_SSP0_DATA1__SSP0_D1			0x2010
+#define MX28_PAD_SSP0_DATA2__SSP0_D2			0x2020
+#define MX28_PAD_SSP0_DATA3__SSP0_D3			0x2030
+#define MX28_PAD_SSP0_DATA4__SSP0_D4			0x2040
+#define MX28_PAD_SSP0_DATA5__SSP0_D5			0x2050
+#define MX28_PAD_SSP0_DATA6__SSP0_D6			0x2060
+#define MX28_PAD_SSP0_DATA7__SSP0_D7			0x2070
+#define MX28_PAD_SSP0_CMD__SSP0_CMD			0x2080
+#define MX28_PAD_SSP0_DETECT__SSP0_CARD_DETECT		0x2090
+#define MX28_PAD_SSP0_SCK__SSP0_SCK			0x20a0
+#define MX28_PAD_SSP1_SCK__SSP1_SCK			0x20c0
+#define MX28_PAD_SSP1_CMD__SSP1_CMD			0x20d0
+#define MX28_PAD_SSP1_DATA0__SSP1_D0			0x20e0
+#define MX28_PAD_SSP1_DATA3__SSP1_D3			0x20f0
+#define MX28_PAD_SSP2_SCK__SSP2_SCK			0x2100
+#define MX28_PAD_SSP2_MOSI__SSP2_CMD			0x2110
+#define MX28_PAD_SSP2_MISO__SSP2_D0			0x2120
+#define MX28_PAD_SSP2_SS0__SSP2_D3			0x2130
+#define MX28_PAD_SSP2_SS1__SSP2_D4			0x2140
+#define MX28_PAD_SSP2_SS2__SSP2_D5			0x2150
+#define MX28_PAD_SSP3_SCK__SSP3_SCK			0x2180
+#define MX28_PAD_SSP3_MOSI__SSP3_CMD			0x2190
+#define MX28_PAD_SSP3_MISO__SSP3_D0			0x21a0
+#define MX28_PAD_SSP3_SS0__SSP3_D3			0x21b0
+#define MX28_PAD_AUART0_RX__AUART0_RX			0x3000
+#define MX28_PAD_AUART0_TX__AUART0_TX			0x3010
+#define MX28_PAD_AUART0_CTS__AUART0_CTS			0x3020
+#define MX28_PAD_AUART0_RTS__AUART0_RTS			0x3030
+#define MX28_PAD_AUART1_RX__AUART1_RX			0x3040
+#define MX28_PAD_AUART1_TX__AUART1_TX			0x3050
+#define MX28_PAD_AUART1_CTS__AUART1_CTS			0x3060
+#define MX28_PAD_AUART1_RTS__AUART1_RTS			0x3070
+#define MX28_PAD_AUART2_RX__AUART2_RX			0x3080
+#define MX28_PAD_AUART2_TX__AUART2_TX			0x3090
+#define MX28_PAD_AUART2_CTS__AUART2_CTS			0x30a0
+#define MX28_PAD_AUART2_RTS__AUART2_RTS			0x30b0
+#define MX28_PAD_AUART3_RX__AUART3_RX			0x30c0
+#define MX28_PAD_AUART3_TX__AUART3_TX			0x30d0
+#define MX28_PAD_AUART3_CTS__AUART3_CTS			0x30e0
+#define MX28_PAD_AUART3_RTS__AUART3_RTS			0x30f0
+#define MX28_PAD_PWM0__PWM_0				0x3100
+#define MX28_PAD_PWM1__PWM_1				0x3110
+#define MX28_PAD_PWM2__PWM_2				0x3120
+#define MX28_PAD_SAIF0_MCLK__SAIF0_MCLK			0x3140
+#define MX28_PAD_SAIF0_LRCLK__SAIF0_LRCLK		0x3150
+#define MX28_PAD_SAIF0_BITCLK__SAIF0_BITCLK		0x3160
+#define MX28_PAD_SAIF0_SDATA0__SAIF0_SDATA0		0x3170
+#define MX28_PAD_I2C0_SCL__I2C0_SCL			0x3180
+#define MX28_PAD_I2C0_SDA__I2C0_SDA			0x3190
+#define MX28_PAD_SAIF1_SDATA0__SAIF1_SDATA0		0x31a0
+#define MX28_PAD_SPDIF__SPDIF_TX			0x31b0
+#define MX28_PAD_PWM3__PWM_3				0x31c0
+#define MX28_PAD_PWM4__PWM_4				0x31d0
+#define MX28_PAD_LCD_RESET__LCD_RESET			0x31e0
+#define MX28_PAD_ENET0_MDC__ENET0_MDC			0x4000
+#define MX28_PAD_ENET0_MDIO__ENET0_MDIO			0x4010
+#define MX28_PAD_ENET0_RX_EN__ENET0_RX_EN		0x4020
+#define MX28_PAD_ENET0_RXD0__ENET0_RXD0			0x4030
+#define MX28_PAD_ENET0_RXD1__ENET0_RXD1			0x4040
+#define MX28_PAD_ENET0_TX_CLK__ENET0_TX_CLK		0x4050
+#define MX28_PAD_ENET0_TX_EN__ENET0_TX_EN		0x4060
+#define MX28_PAD_ENET0_TXD0__ENET0_TXD0			0x4070
+#define MX28_PAD_ENET0_TXD1__ENET0_TXD1			0x4080
+#define MX28_PAD_ENET0_RXD2__ENET0_RXD2			0x4090
+#define MX28_PAD_ENET0_RXD3__ENET0_RXD3			0x40a0
+#define MX28_PAD_ENET0_TXD2__ENET0_TXD2			0x40b0
+#define MX28_PAD_ENET0_TXD3__ENET0_TXD3			0x40c0
+#define MX28_PAD_ENET0_RX_CLK__ENET0_RX_CLK		0x40d0
+#define MX28_PAD_ENET0_COL__ENET0_COL			0x40e0
+#define MX28_PAD_ENET0_CRS__ENET0_CRS			0x40f0
+#define MX28_PAD_ENET_CLK__CLKCTRL_ENET			0x4100
+#define MX28_PAD_JTAG_RTCK__JTAG_RTCK			0x4140
+#define MX28_PAD_EMI_D00__EMI_DATA0			0x5000
+#define MX28_PAD_EMI_D01__EMI_DATA1			0x5010
+#define MX28_PAD_EMI_D02__EMI_DATA2			0x5020
+#define MX28_PAD_EMI_D03__EMI_DATA3			0x5030
+#define MX28_PAD_EMI_D04__EMI_DATA4			0x5040
+#define MX28_PAD_EMI_D05__EMI_DATA5			0x5050
+#define MX28_PAD_EMI_D06__EMI_DATA6			0x5060
+#define MX28_PAD_EMI_D07__EMI_DATA7			0x5070
+#define MX28_PAD_EMI_D08__EMI_DATA8			0x5080
+#define MX28_PAD_EMI_D09__EMI_DATA9			0x5090
+#define MX28_PAD_EMI_D10__EMI_DATA10			0x50a0
+#define MX28_PAD_EMI_D11__EMI_DATA11			0x50b0
+#define MX28_PAD_EMI_D12__EMI_DATA12			0x50c0
+#define MX28_PAD_EMI_D13__EMI_DATA13			0x50d0
+#define MX28_PAD_EMI_D14__EMI_DATA14			0x50e0
+#define MX28_PAD_EMI_D15__EMI_DATA15			0x50f0
+#define MX28_PAD_EMI_ODT0__EMI_ODT0			0x5100
+#define MX28_PAD_EMI_DQM0__EMI_DQM0			0x5110
+#define MX28_PAD_EMI_ODT1__EMI_ODT1			0x5120
+#define MX28_PAD_EMI_DQM1__EMI_DQM1			0x5130
+#define MX28_PAD_EMI_DDR_OPEN_FB__EMI_DDR_OPEN_FEEDBACK	0x5140
+#define MX28_PAD_EMI_CLK__EMI_CLK			0x5150
+#define MX28_PAD_EMI_DQS0__EMI_DQS0			0x5160
+#define MX28_PAD_EMI_DQS1__EMI_DQS1			0x5170
+#define MX28_PAD_EMI_DDR_OPEN__EMI_DDR_OPEN		0x51a0
+#define MX28_PAD_EMI_A00__EMI_ADDR0			0x6000
+#define MX28_PAD_EMI_A01__EMI_ADDR1			0x6010
+#define MX28_PAD_EMI_A02__EMI_ADDR2			0x6020
+#define MX28_PAD_EMI_A03__EMI_ADDR3			0x6030
+#define MX28_PAD_EMI_A04__EMI_ADDR4			0x6040
+#define MX28_PAD_EMI_A05__EMI_ADDR5			0x6050
+#define MX28_PAD_EMI_A06__EMI_ADDR6			0x6060
+#define MX28_PAD_EMI_A07__EMI_ADDR7			0x6070
+#define MX28_PAD_EMI_A08__EMI_ADDR8			0x6080
+#define MX28_PAD_EMI_A09__EMI_ADDR9			0x6090
+#define MX28_PAD_EMI_A10__EMI_ADDR10			0x60a0
+#define MX28_PAD_EMI_A11__EMI_ADDR11			0x60b0
+#define MX28_PAD_EMI_A12__EMI_ADDR12			0x60c0
+#define MX28_PAD_EMI_A13__EMI_ADDR13			0x60d0
+#define MX28_PAD_EMI_A14__EMI_ADDR14			0x60e0
+#define MX28_PAD_EMI_BA0__EMI_BA0			0x6100
+#define MX28_PAD_EMI_BA1__EMI_BA1			0x6110
+#define MX28_PAD_EMI_BA2__EMI_BA2			0x6120
+#define MX28_PAD_EMI_CASN__EMI_CASN			0x6130
+#define MX28_PAD_EMI_RASN__EMI_RASN			0x6140
+#define MX28_PAD_EMI_WEN__EMI_WEN			0x6150
+#define MX28_PAD_EMI_CE0N__EMI_CE0N			0x6160
+#define MX28_PAD_EMI_CE1N__EMI_CE1N			0x6170
+#define MX28_PAD_EMI_CKE__EMI_CKE			0x6180
+#define MX28_PAD_GPMI_D00__SSP1_D0			0x0001
+#define MX28_PAD_GPMI_D01__SSP1_D1			0x0011
+#define MX28_PAD_GPMI_D02__SSP1_D2			0x0021
+#define MX28_PAD_GPMI_D03__SSP1_D3			0x0031
+#define MX28_PAD_GPMI_D04__SSP1_D4			0x0041
+#define MX28_PAD_GPMI_D05__SSP1_D5			0x0051
+#define MX28_PAD_GPMI_D06__SSP1_D6			0x0061
+#define MX28_PAD_GPMI_D07__SSP1_D7			0x0071
+#define MX28_PAD_GPMI_CE0N__SSP3_D0			0x0101
+#define MX28_PAD_GPMI_CE1N__SSP3_D3			0x0111
+#define MX28_PAD_GPMI_CE2N__CAN1_TX			0x0121
+#define MX28_PAD_GPMI_CE3N__CAN1_RX			0x0131
+#define MX28_PAD_GPMI_RDY0__SSP1_CARD_DETECT		0x0141
+#define MX28_PAD_GPMI_RDY1__SSP1_CMD			0x0151
+#define MX28_PAD_GPMI_RDY2__CAN0_TX			0x0161
+#define MX28_PAD_GPMI_RDY3__CAN0_RX			0x0171
+#define MX28_PAD_GPMI_RDN__SSP3_SCK			0x0181
+#define MX28_PAD_GPMI_WRN__SSP1_SCK			0x0191
+#define MX28_PAD_GPMI_ALE__SSP3_D1			0x01a1
+#define MX28_PAD_GPMI_CLE__SSP3_D2			0x01b1
+#define MX28_PAD_GPMI_RESETN__SSP3_CMD			0x01c1
+#define MX28_PAD_LCD_D03__ETM_DA8			0x1031
+#define MX28_PAD_LCD_D04__ETM_DA9			0x1041
+#define MX28_PAD_LCD_D08__ETM_DA3			0x1081
+#define MX28_PAD_LCD_D09__ETM_DA4			0x1091
+#define MX28_PAD_LCD_D20__ENET1_1588_EVENT2_OUT		0x1141
+#define MX28_PAD_LCD_D21__ENET1_1588_EVENT2_IN		0x1151
+#define MX28_PAD_LCD_D22__ENET1_1588_EVENT3_OUT		0x1161
+#define MX28_PAD_LCD_D23__ENET1_1588_EVENT3_IN		0x1171
+#define MX28_PAD_LCD_RD_E__LCD_VSYNC			0x1181
+#define MX28_PAD_LCD_WR_RWN__LCD_HSYNC			0x1191
+#define MX28_PAD_LCD_RS__LCD_DOTCLK			0x11a1
+#define MX28_PAD_LCD_CS__LCD_ENABLE			0x11b1
+#define MX28_PAD_LCD_VSYNC__SAIF1_SDATA0		0x11c1
+#define MX28_PAD_LCD_HSYNC__SAIF1_SDATA1		0x11d1
+#define MX28_PAD_LCD_DOTCLK__SAIF1_MCLK			0x11e1
+#define MX28_PAD_SSP0_DATA4__SSP2_D0			0x2041
+#define MX28_PAD_SSP0_DATA5__SSP2_D3			0x2051
+#define MX28_PAD_SSP0_DATA6__SSP2_CMD			0x2061
+#define MX28_PAD_SSP0_DATA7__SSP2_SCK			0x2071
+#define MX28_PAD_SSP1_SCK__SSP2_D1			0x20c1
+#define MX28_PAD_SSP1_CMD__SSP2_D2			0x20d1
+#define MX28_PAD_SSP1_DATA0__SSP2_D6			0x20e1
+#define MX28_PAD_SSP1_DATA3__SSP2_D7			0x20f1
+#define MX28_PAD_SSP2_SCK__AUART2_RX			0x2101
+#define MX28_PAD_SSP2_MOSI__AUART2_TX			0x2111
+#define MX28_PAD_SSP2_MISO__AUART3_RX			0x2121
+#define MX28_PAD_SSP2_SS0__AUART3_TX			0x2131
+#define MX28_PAD_SSP2_SS1__SSP2_D1			0x2141
+#define MX28_PAD_SSP2_SS2__SSP2_D2			0x2151
+#define MX28_PAD_SSP3_SCK__AUART4_TX			0x2181
+#define MX28_PAD_SSP3_MOSI__AUART4_RX			0x2191
+#define MX28_PAD_SSP3_MISO__AUART4_RTS			0x21a1
+#define MX28_PAD_SSP3_SS0__AUART4_CTS			0x21b1
+#define MX28_PAD_AUART0_RX__I2C0_SCL			0x3001
+#define MX28_PAD_AUART0_TX__I2C0_SDA			0x3011
+#define MX28_PAD_AUART0_CTS__AUART4_RX			0x3021
+#define MX28_PAD_AUART0_RTS__AUART4_TX			0x3031
+#define MX28_PAD_AUART1_RX__SSP2_CARD_DETECT		0x3041
+#define MX28_PAD_AUART1_TX__SSP3_CARD_DETECT		0x3051
+#define MX28_PAD_AUART1_CTS__USB0_OVERCURRENT		0x3061
+#define MX28_PAD_AUART1_RTS__USB0_ID			0x3071
+#define MX28_PAD_AUART2_RX__SSP3_D1			0x3081
+#define MX28_PAD_AUART2_TX__SSP3_D2			0x3091
+#define MX28_PAD_AUART2_CTS__I2C1_SCL			0x30a1
+#define MX28_PAD_AUART2_RTS__I2C1_SDA			0x30b1
+#define MX28_PAD_AUART3_RX__CAN0_TX			0x30c1
+#define MX28_PAD_AUART3_TX__CAN0_RX			0x30d1
+#define MX28_PAD_AUART3_CTS__CAN1_TX			0x30e1
+#define MX28_PAD_AUART3_RTS__CAN1_RX			0x30f1
+#define MX28_PAD_PWM0__I2C1_SCL				0x3101
+#define MX28_PAD_PWM1__I2C1_SDA				0x3111
+#define MX28_PAD_PWM2__USB0_ID				0x3121
+#define MX28_PAD_SAIF0_MCLK__PWM_3			0x3141
+#define MX28_PAD_SAIF0_LRCLK__PWM_4			0x3151
+#define MX28_PAD_SAIF0_BITCLK__PWM_5			0x3161
+#define MX28_PAD_SAIF0_SDATA0__PWM_6			0x3171
+#define MX28_PAD_I2C0_SCL__TIMROT_ROTARYA		0x3181
+#define MX28_PAD_I2C0_SDA__TIMROT_ROTARYB		0x3191
+#define MX28_PAD_SAIF1_SDATA0__PWM_7			0x31a1
+#define MX28_PAD_LCD_RESET__LCD_VSYNC			0x31e1
+#define MX28_PAD_ENET0_MDC__GPMI_CE4N			0x4001
+#define MX28_PAD_ENET0_MDIO__GPMI_CE5N			0x4011
+#define MX28_PAD_ENET0_RX_EN__GPMI_CE6N			0x4021
+#define MX28_PAD_ENET0_RXD0__GPMI_CE7N			0x4031
+#define MX28_PAD_ENET0_RXD1__GPMI_READY4		0x4041
+#define MX28_PAD_ENET0_TX_CLK__HSADC_TRIGGER		0x4051
+#define MX28_PAD_ENET0_TX_EN__GPMI_READY5		0x4061
+#define MX28_PAD_ENET0_TXD0__GPMI_READY6		0x4071
+#define MX28_PAD_ENET0_TXD1__GPMI_READY7		0x4081
+#define MX28_PAD_ENET0_RXD2__ENET1_RXD0			0x4091
+#define MX28_PAD_ENET0_RXD3__ENET1_RXD1			0x40a1
+#define MX28_PAD_ENET0_TXD2__ENET1_TXD0			0x40b1
+#define MX28_PAD_ENET0_TXD3__ENET1_TXD1			0x40c1
+#define MX28_PAD_ENET0_RX_CLK__ENET0_RX_ER		0x40d1
+#define MX28_PAD_ENET0_COL__ENET1_TX_EN			0x40e1
+#define MX28_PAD_ENET0_CRS__ENET1_RX_EN			0x40f1
+#define MX28_PAD_GPMI_CE2N__ENET0_RX_ER			0x0122
+#define MX28_PAD_GPMI_CE3N__SAIF1_MCLK			0x0132
+#define MX28_PAD_GPMI_RDY0__USB0_ID			0x0142
+#define MX28_PAD_GPMI_RDY2__ENET0_TX_ER			0x0162
+#define MX28_PAD_GPMI_RDY3__HSADC_TRIGGER		0x0172
+#define MX28_PAD_GPMI_ALE__SSP3_D4			0x01a2
+#define MX28_PAD_GPMI_CLE__SSP3_D5			0x01b2
+#define MX28_PAD_LCD_D00__ETM_DA0			0x1002
+#define MX28_PAD_LCD_D01__ETM_DA1			0x1012
+#define MX28_PAD_LCD_D02__ETM_DA2			0x1022
+#define MX28_PAD_LCD_D03__ETM_DA3			0x1032
+#define MX28_PAD_LCD_D04__ETM_DA4			0x1042
+#define MX28_PAD_LCD_D05__ETM_DA5			0x1052
+#define MX28_PAD_LCD_D06__ETM_DA6			0x1062
+#define MX28_PAD_LCD_D07__ETM_DA7			0x1072
+#define MX28_PAD_LCD_D08__ETM_DA8			0x1082
+#define MX28_PAD_LCD_D09__ETM_DA9			0x1092
+#define MX28_PAD_LCD_D10__ETM_DA10			0x10a2
+#define MX28_PAD_LCD_D11__ETM_DA11			0x10b2
+#define MX28_PAD_LCD_D12__ETM_DA12			0x10c2
+#define MX28_PAD_LCD_D13__ETM_DA13			0x10d2
+#define MX28_PAD_LCD_D14__ETM_DA14			0x10e2
+#define MX28_PAD_LCD_D15__ETM_DA15			0x10f2
+#define MX28_PAD_LCD_D16__ETM_DA7			0x1102
+#define MX28_PAD_LCD_D17__ETM_DA6			0x1112
+#define MX28_PAD_LCD_D18__ETM_DA5			0x1122
+#define MX28_PAD_LCD_D19__ETM_DA4			0x1132
+#define MX28_PAD_LCD_D20__ETM_DA3			0x1142
+#define MX28_PAD_LCD_D21__ETM_DA2			0x1152
+#define MX28_PAD_LCD_D22__ETM_DA1			0x1162
+#define MX28_PAD_LCD_D23__ETM_DA0			0x1172
+#define MX28_PAD_LCD_RD_E__ETM_TCTL			0x1182
+#define MX28_PAD_LCD_WR_RWN__ETM_TCLK			0x1192
+#define MX28_PAD_LCD_HSYNC__ETM_TCTL			0x11d2
+#define MX28_PAD_LCD_DOTCLK__ETM_TCLK			0x11e2
+#define MX28_PAD_SSP1_SCK__ENET0_1588_EVENT2_OUT	0x20c2
+#define MX28_PAD_SSP1_CMD__ENET0_1588_EVENT2_IN		0x20d2
+#define MX28_PAD_SSP1_DATA0__ENET0_1588_EVENT3_OUT	0x20e2
+#define MX28_PAD_SSP1_DATA3__ENET0_1588_EVENT3_IN	0x20f2
+#define MX28_PAD_SSP2_SCK__SAIF0_SDATA1			0x2102
+#define MX28_PAD_SSP2_MOSI__SAIF0_SDATA2		0x2112
+#define MX28_PAD_SSP2_MISO__SAIF1_SDATA1		0x2122
+#define MX28_PAD_SSP2_SS0__SAIF1_SDATA2			0x2132
+#define MX28_PAD_SSP2_SS1__USB1_OVERCURRENT		0x2142
+#define MX28_PAD_SSP2_SS2__USB0_OVERCURRENT		0x2152
+#define MX28_PAD_SSP3_SCK__ENET1_1588_EVENT0_OUT	0x2182
+#define MX28_PAD_SSP3_MOSI__ENET1_1588_EVENT0_IN	0x2192
+#define MX28_PAD_SSP3_MISO__ENET1_1588_EVENT1_OUT	0x21a2
+#define MX28_PAD_SSP3_SS0__ENET1_1588_EVENT1_IN		0x21b2
+#define MX28_PAD_AUART0_RX__DUART_CTS			0x3002
+#define MX28_PAD_AUART0_TX__DUART_RTS			0x3012
+#define MX28_PAD_AUART0_CTS__DUART_RX			0x3022
+#define MX28_PAD_AUART0_RTS__DUART_TX			0x3032
+#define MX28_PAD_AUART1_RX__PWM_0			0x3042
+#define MX28_PAD_AUART1_TX__PWM_1			0x3052
+#define MX28_PAD_AUART1_CTS__TIMROT_ROTARYA		0x3062
+#define MX28_PAD_AUART1_RTS__TIMROT_ROTARYB		0x3072
+#define MX28_PAD_AUART2_RX__SSP3_D4			0x3082
+#define MX28_PAD_AUART2_TX__SSP3_D5			0x3092
+#define MX28_PAD_AUART2_CTS__SAIF1_BITCLK		0x30a2
+#define MX28_PAD_AUART2_RTS__SAIF1_LRCLK		0x30b2
+#define MX28_PAD_AUART3_RX__ENET0_1588_EVENT0_OUT	0x30c2
+#define MX28_PAD_AUART3_TX__ENET0_1588_EVENT0_IN	0x30d2
+#define MX28_PAD_AUART3_CTS__ENET0_1588_EVENT1_OUT	0x30e2
+#define MX28_PAD_AUART3_RTS__ENET0_1588_EVENT1_IN	0x30f2
+#define MX28_PAD_PWM0__DUART_RX				0x3102
+#define MX28_PAD_PWM1__DUART_TX				0x3112
+#define MX28_PAD_PWM2__USB1_OVERCURRENT			0x3122
+#define MX28_PAD_SAIF0_MCLK__AUART4_CTS			0x3142
+#define MX28_PAD_SAIF0_LRCLK__AUART4_RTS		0x3152
+#define MX28_PAD_SAIF0_BITCLK__AUART4_RX		0x3162
+#define MX28_PAD_SAIF0_SDATA0__AUART4_TX		0x3172
+#define MX28_PAD_I2C0_SCL__DUART_RX			0x3182
+#define MX28_PAD_I2C0_SDA__DUART_TX			0x3192
+#define MX28_PAD_SAIF1_SDATA0__SAIF0_SDATA1		0x31a2
+#define MX28_PAD_SPDIF__ENET1_RX_ER			0x31b2
+#define MX28_PAD_ENET0_MDC__SAIF0_SDATA1		0x4002
+#define MX28_PAD_ENET0_MDIO__SAIF0_SDATA2		0x4012
+#define MX28_PAD_ENET0_RX_EN__SAIF1_SDATA1		0x4022
+#define MX28_PAD_ENET0_RXD0__SAIF1_SDATA2		0x4032
+#define MX28_PAD_ENET0_TX_CLK__ENET0_1588_EVENT2_OUT	0x4052
+#define MX28_PAD_ENET0_RXD2__ENET0_1588_EVENT0_OUT	0x4092
+#define MX28_PAD_ENET0_RXD3__ENET0_1588_EVENT0_IN	0x40a2
+#define MX28_PAD_ENET0_TXD2__ENET0_1588_EVENT1_OUT	0x40b2
+#define MX28_PAD_ENET0_TXD3__ENET0_1588_EVENT1_IN	0x40c2
+#define MX28_PAD_ENET0_RX_CLK__ENET0_1588_EVENT2_IN	0x40d2
+#define MX28_PAD_ENET0_COL__ENET0_1588_EVENT3_OUT	0x40e2
+#define MX28_PAD_ENET0_CRS__ENET0_1588_EVENT3_IN	0x40f2
+#define MX28_PAD_GPMI_D00__GPIO_0_0			0x0003
+#define MX28_PAD_GPMI_D01__GPIO_0_1			0x0013
+#define MX28_PAD_GPMI_D02__GPIO_0_2			0x0023
+#define MX28_PAD_GPMI_D03__GPIO_0_3			0x0033
+#define MX28_PAD_GPMI_D04__GPIO_0_4			0x0043
+#define MX28_PAD_GPMI_D05__GPIO_0_5			0x0053
+#define MX28_PAD_GPMI_D06__GPIO_0_6			0x0063
+#define MX28_PAD_GPMI_D07__GPIO_0_7			0x0073
+#define MX28_PAD_GPMI_CE0N__GPIO_0_16			0x0103
+#define MX28_PAD_GPMI_CE1N__GPIO_0_17			0x0113
+#define MX28_PAD_GPMI_CE2N__GPIO_0_18			0x0123
+#define MX28_PAD_GPMI_CE3N__GPIO_0_19			0x0133
+#define MX28_PAD_GPMI_RDY0__GPIO_0_20			0x0143
+#define MX28_PAD_GPMI_RDY1__GPIO_0_21			0x0153
+#define MX28_PAD_GPMI_RDY2__GPIO_0_22			0x0163
+#define MX28_PAD_GPMI_RDY3__GPIO_0_23			0x0173
+#define MX28_PAD_GPMI_RDN__GPIO_0_24			0x0183
+#define MX28_PAD_GPMI_WRN__GPIO_0_25			0x0193
+#define MX28_PAD_GPMI_ALE__GPIO_0_26			0x01a3
+#define MX28_PAD_GPMI_CLE__GPIO_0_27			0x01b3
+#define MX28_PAD_GPMI_RESETN__GPIO_0_28			0x01c3
+#define MX28_PAD_LCD_D00__GPIO_1_0			0x1003
+#define MX28_PAD_LCD_D01__GPIO_1_1			0x1013
+#define MX28_PAD_LCD_D02__GPIO_1_2			0x1023
+#define MX28_PAD_LCD_D03__GPIO_1_3			0x1033
+#define MX28_PAD_LCD_D04__GPIO_1_4			0x1043
+#define MX28_PAD_LCD_D05__GPIO_1_5			0x1053
+#define MX28_PAD_LCD_D06__GPIO_1_6			0x1063
+#define MX28_PAD_LCD_D07__GPIO_1_7			0x1073
+#define MX28_PAD_LCD_D08__GPIO_1_8			0x1083
+#define MX28_PAD_LCD_D09__GPIO_1_9			0x1093
+#define MX28_PAD_LCD_D10__GPIO_1_10			0x10a3
+#define MX28_PAD_LCD_D11__GPIO_1_11			0x10b3
+#define MX28_PAD_LCD_D12__GPIO_1_12			0x10c3
+#define MX28_PAD_LCD_D13__GPIO_1_13			0x10d3
+#define MX28_PAD_LCD_D14__GPIO_1_14			0x10e3
+#define MX28_PAD_LCD_D15__GPIO_1_15			0x10f3
+#define MX28_PAD_LCD_D16__GPIO_1_16			0x1103
+#define MX28_PAD_LCD_D17__GPIO_1_17			0x1113
+#define MX28_PAD_LCD_D18__GPIO_1_18			0x1123
+#define MX28_PAD_LCD_D19__GPIO_1_19			0x1133
+#define MX28_PAD_LCD_D20__GPIO_1_20			0x1143
+#define MX28_PAD_LCD_D21__GPIO_1_21			0x1153
+#define MX28_PAD_LCD_D22__GPIO_1_22			0x1163
+#define MX28_PAD_LCD_D23__GPIO_1_23			0x1173
+#define MX28_PAD_LCD_RD_E__GPIO_1_24			0x1183
+#define MX28_PAD_LCD_WR_RWN__GPIO_1_25			0x1193
+#define MX28_PAD_LCD_RS__GPIO_1_26			0x11a3
+#define MX28_PAD_LCD_CS__GPIO_1_27			0x11b3
+#define MX28_PAD_LCD_VSYNC__GPIO_1_28			0x11c3
+#define MX28_PAD_LCD_HSYNC__GPIO_1_29			0x11d3
+#define MX28_PAD_LCD_DOTCLK__GPIO_1_30			0x11e3
+#define MX28_PAD_LCD_ENABLE__GPIO_1_31			0x11f3
+#define MX28_PAD_SSP0_DATA0__GPIO_2_0			0x2003
+#define MX28_PAD_SSP0_DATA1__GPIO_2_1			0x2013
+#define MX28_PAD_SSP0_DATA2__GPIO_2_2			0x2023
+#define MX28_PAD_SSP0_DATA3__GPIO_2_3			0x2033
+#define MX28_PAD_SSP0_DATA4__GPIO_2_4			0x2043
+#define MX28_PAD_SSP0_DATA5__GPIO_2_5			0x2053
+#define MX28_PAD_SSP0_DATA6__GPIO_2_6			0x2063
+#define MX28_PAD_SSP0_DATA7__GPIO_2_7			0x2073
+#define MX28_PAD_SSP0_CMD__GPIO_2_8			0x2083
+#define MX28_PAD_SSP0_DETECT__GPIO_2_9			0x2093
+#define MX28_PAD_SSP0_SCK__GPIO_2_10			0x20a3
+#define MX28_PAD_SSP1_SCK__GPIO_2_12			0x20c3
+#define MX28_PAD_SSP1_CMD__GPIO_2_13			0x20d3
+#define MX28_PAD_SSP1_DATA0__GPIO_2_14			0x20e3
+#define MX28_PAD_SSP1_DATA3__GPIO_2_15			0x20f3
+#define MX28_PAD_SSP2_SCK__GPIO_2_16			0x2103
+#define MX28_PAD_SSP2_MOSI__GPIO_2_17			0x2113
+#define MX28_PAD_SSP2_MISO__GPIO_2_18			0x2123
+#define MX28_PAD_SSP2_SS0__GPIO_2_19			0x2133
+#define MX28_PAD_SSP2_SS1__GPIO_2_20			0x2143
+#define MX28_PAD_SSP2_SS2__GPIO_2_21			0x2153
+#define MX28_PAD_SSP3_SCK__GPIO_2_24			0x2183
+#define MX28_PAD_SSP3_MOSI__GPIO_2_25			0x2193
+#define MX28_PAD_SSP3_MISO__GPIO_2_26			0x21a3
+#define MX28_PAD_SSP3_SS0__GPIO_2_27			0x21b3
+#define MX28_PAD_AUART0_RX__GPIO_3_0			0x3003
+#define MX28_PAD_AUART0_TX__GPIO_3_1			0x3013
+#define MX28_PAD_AUART0_CTS__GPIO_3_2			0x3023
+#define MX28_PAD_AUART0_RTS__GPIO_3_3			0x3033
+#define MX28_PAD_AUART1_RX__GPIO_3_4			0x3043
+#define MX28_PAD_AUART1_TX__GPIO_3_5			0x3053
+#define MX28_PAD_AUART1_CTS__GPIO_3_6			0x3063
+#define MX28_PAD_AUART1_RTS__GPIO_3_7			0x3073
+#define MX28_PAD_AUART2_RX__GPIO_3_8			0x3083
+#define MX28_PAD_AUART2_TX__GPIO_3_9			0x3093
+#define MX28_PAD_AUART2_CTS__GPIO_3_10			0x30a3
+#define MX28_PAD_AUART2_RTS__GPIO_3_11			0x30b3
+#define MX28_PAD_AUART3_RX__GPIO_3_12			0x30c3
+#define MX28_PAD_AUART3_TX__GPIO_3_13			0x30d3
+#define MX28_PAD_AUART3_CTS__GPIO_3_14			0x30e3
+#define MX28_PAD_AUART3_RTS__GPIO_3_15			0x30f3
+#define MX28_PAD_PWM0__GPIO_3_16			0x3103
+#define MX28_PAD_PWM1__GPIO_3_17			0x3113
+#define MX28_PAD_PWM2__GPIO_3_18			0x3123
+#define MX28_PAD_SAIF0_MCLK__GPIO_3_20			0x3143
+#define MX28_PAD_SAIF0_LRCLK__GPIO_3_21			0x3153
+#define MX28_PAD_SAIF0_BITCLK__GPIO_3_22		0x3163
+#define MX28_PAD_SAIF0_SDATA0__GPIO_3_23		0x3173
+#define MX28_PAD_I2C0_SCL__GPIO_3_24			0x3183
+#define MX28_PAD_I2C0_SDA__GPIO_3_25			0x3193
+#define MX28_PAD_SAIF1_SDATA0__GPIO_3_26		0x31a3
+#define MX28_PAD_SPDIF__GPIO_3_27			0x31b3
+#define MX28_PAD_PWM3__GPIO_3_28			0x31c3
+#define MX28_PAD_PWM4__GPIO_3_29			0x31d3
+#define MX28_PAD_LCD_RESET__GPIO_3_30			0x31e3
+#define MX28_PAD_ENET0_MDC__GPIO_4_0			0x4003
+#define MX28_PAD_ENET0_MDIO__GPIO_4_1			0x4013
+#define MX28_PAD_ENET0_RX_EN__GPIO_4_2			0x4023
+#define MX28_PAD_ENET0_RXD0__GPIO_4_3			0x4033
+#define MX28_PAD_ENET0_RXD1__GPIO_4_4			0x4043
+#define MX28_PAD_ENET0_TX_CLK__GPIO_4_5			0x4053
+#define MX28_PAD_ENET0_TX_EN__GPIO_4_6			0x4063
+#define MX28_PAD_ENET0_TXD0__GPIO_4_7			0x4073
+#define MX28_PAD_ENET0_TXD1__GPIO_4_8			0x4083
+#define MX28_PAD_ENET0_RXD2__GPIO_4_9			0x4093
+#define MX28_PAD_ENET0_RXD3__GPIO_4_10			0x40a3
+#define MX28_PAD_ENET0_TXD2__GPIO_4_11			0x40b3
+#define MX28_PAD_ENET0_TXD3__GPIO_4_12			0x40c3
+#define MX28_PAD_ENET0_RX_CLK__GPIO_4_13		0x40d3
+#define MX28_PAD_ENET0_COL__GPIO_4_14			0x40e3
+#define MX28_PAD_ENET0_CRS__GPIO_4_15			0x40f3
+#define MX28_PAD_ENET_CLK__GPIO_4_16			0x4103
+#define MX28_PAD_JTAG_RTCK__GPIO_4_20			0x4143
+
+#endif /* __DT_BINDINGS_MX28_PINCTRL_H__ */
diff --git a/arch/arm/dts/imx28.dtsi b/arch/arm/dts/imx28.dtsi
new file mode 100644
index 0000000000..e14d8ef015
--- /dev/null
+++ b/arch/arm/dts/imx28.dtsi
@@ -0,0 +1,1330 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Copyright 2012 Freescale Semiconductor, Inc.
+
+#include <dt-bindings/gpio/gpio.h>
+#include "imx28-pinfunc.h"
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	interrupt-parent = <&icoll>;
+	/*
+	 * The decompressor and also some bootloaders rely on a
+	 * pre-existing /chosen node to be available to insert the
+	 * command line and merge other ATAGS info.
+	 */
+	chosen {};
+
+	aliases {
+		ethernet0 = &mac0;
+		ethernet1 = &mac1;
+		gpio0 = &gpio0;
+		gpio1 = &gpio1;
+		gpio2 = &gpio2;
+		gpio3 = &gpio3;
+		gpio4 = &gpio4;
+		saif0 = &saif0;
+		saif1 = &saif1;
+		serial0 = &auart0;
+		serial1 = &auart1;
+		serial2 = &auart2;
+		serial3 = &auart3;
+		serial4 = &auart4;
+		spi0 = &ssp1;
+		spi1 = &ssp2;
+		usbphy0 = &usbphy0;
+		usbphy1 = &usbphy1;
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu at 0 {
+			compatible = "arm,arm926ej-s";
+			device_type = "cpu";
+			reg = <0>;
+		};
+	};
+
+	apb at 80000000 {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x80000000 0x80000>;
+		ranges;
+
+		apbh at 80000000 {
+			compatible = "simple-bus";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			reg = <0x80000000 0x3c900>;
+			ranges;
+
+			icoll: interrupt-controller at 80000000 {
+				compatible = "fsl,imx28-icoll", "fsl,icoll";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+				reg = <0x80000000 0x2000>;
+			};
+
+			hsadc: hsadc at 80002000 {
+				reg = <0x80002000 0x2000>;
+				interrupts = <13>;
+				dmas = <&dma_apbh 12>;
+				dma-names = "rx";
+				status = "disabled";
+			};
+
+			dma_apbh: dma-apbh at 80004000 {
+				compatible = "fsl,imx28-dma-apbh";
+				reg = <0x80004000 0x2000>;
+				interrupts = <82 83 84 85
+					      88 88 88 88
+					      88 88 88 88
+					      87 86 0 0>;
+				interrupt-names = "ssp0", "ssp1", "ssp2", "ssp3",
+						  "gpmi0", "gmpi1", "gpmi2", "gmpi3",
+						  "gpmi4", "gmpi5", "gpmi6", "gmpi7",
+						  "hsadc", "lcdif", "empty", "empty";
+				#dma-cells = <1>;
+				dma-channels = <16>;
+				clocks = <&clks 25>;
+			};
+
+			perfmon: perfmon at 80006000 {
+				reg = <0x80006000 0x800>;
+				interrupts = <27>;
+				status = "disabled";
+			};
+
+			gpmi: gpmi-nand at 8000c000 {
+				compatible = "fsl,imx28-gpmi-nand";
+				#address-cells = <1>;
+				#size-cells = <1>;
+				reg = <0x8000c000 0x2000>, <0x8000a000 0x2000>;
+				reg-names = "gpmi-nand", "bch";
+				interrupts = <41>;
+				interrupt-names = "bch";
+				clocks = <&clks 50>;
+				clock-names = "gpmi_io";
+				dmas = <&dma_apbh 4>;
+				dma-names = "rx-tx";
+				status = "disabled";
+			};
+
+			ssp0: spi at 80010000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0x80010000 0x2000>;
+				interrupts = <96>;
+				clocks = <&clks 46>;
+				dmas = <&dma_apbh 0>;
+				dma-names = "rx-tx";
+				status = "disabled";
+			};
+
+			ssp1: spi at 80012000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0x80012000 0x2000>;
+				interrupts = <97>;
+				clocks = <&clks 47>;
+				dmas = <&dma_apbh 1>;
+				dma-names = "rx-tx";
+				status = "disabled";
+			};
+
+			ssp2: spi at 80014000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0x80014000 0x2000>;
+				interrupts = <98>;
+				clocks = <&clks 48>;
+				dmas = <&dma_apbh 2>;
+				dma-names = "rx-tx";
+				status = "disabled";
+			};
+
+			ssp3: spi at 80016000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0x80016000 0x2000>;
+				interrupts = <99>;
+				clocks = <&clks 49>;
+				dmas = <&dma_apbh 3>;
+				dma-names = "rx-tx";
+				status = "disabled";
+			};
+
+			pinctrl: pinctrl at 80018000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "fsl,imx28-pinctrl", "simple-bus";
+				reg = <0x80018000 0x2000>;
+
+				gpio0: gpio at 0 {
+					compatible = "fsl,imx28-gpio", "fsl,mxs-gpio";
+					reg = <0>;
+					interrupts = <127>;
+					gpio-controller;
+					#gpio-cells = <2>;
+					interrupt-controller;
+					#interrupt-cells = <2>;
+				};
+
+				gpio1: gpio at 1 {
+					compatible = "fsl,imx28-gpio", "fsl,mxs-gpio";
+					reg = <1>;
+					interrupts = <126>;
+					gpio-controller;
+					#gpio-cells = <2>;
+					interrupt-controller;
+					#interrupt-cells = <2>;
+				};
+
+				gpio2: gpio at 2 {
+					compatible = "fsl,imx28-gpio", "fsl,mxs-gpio";
+					reg = <2>;
+					interrupts = <125>;
+					gpio-controller;
+					#gpio-cells = <2>;
+					interrupt-controller;
+					#interrupt-cells = <2>;
+				};
+
+				gpio3: gpio at 3 {
+					compatible = "fsl,imx28-gpio", "fsl,mxs-gpio";
+					reg = <3>;
+					interrupts = <124>;
+					gpio-controller;
+					#gpio-cells = <2>;
+					interrupt-controller;
+					#interrupt-cells = <2>;
+				};
+
+				gpio4: gpio at 4 {
+					compatible = "fsl,imx28-gpio", "fsl,mxs-gpio";
+					reg = <4>;
+					interrupts = <123>;
+					gpio-controller;
+					#gpio-cells = <2>;
+					interrupt-controller;
+					#interrupt-cells = <2>;
+				};
+
+				duart_pins_a: duart at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_PWM0__DUART_RX
+						MX28_PAD_PWM1__DUART_TX
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				duart_pins_b: duart at 1 {
+					reg = <1>;
+					fsl,pinmux-ids = <
+						MX28_PAD_AUART0_CTS__DUART_RX
+						MX28_PAD_AUART0_RTS__DUART_TX
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				duart_4pins_a: duart-4pins at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_AUART0_CTS__DUART_RX
+						MX28_PAD_AUART0_RTS__DUART_TX
+						MX28_PAD_AUART0_RX__DUART_CTS
+						MX28_PAD_AUART0_TX__DUART_RTS
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				gpmi_pins_a: gpmi-nand at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_GPMI_D00__GPMI_D0
+						MX28_PAD_GPMI_D01__GPMI_D1
+						MX28_PAD_GPMI_D02__GPMI_D2
+						MX28_PAD_GPMI_D03__GPMI_D3
+						MX28_PAD_GPMI_D04__GPMI_D4
+						MX28_PAD_GPMI_D05__GPMI_D5
+						MX28_PAD_GPMI_D06__GPMI_D6
+						MX28_PAD_GPMI_D07__GPMI_D7
+						MX28_PAD_GPMI_CE0N__GPMI_CE0N
+						MX28_PAD_GPMI_RDY0__GPMI_READY0
+						MX28_PAD_GPMI_RDN__GPMI_RDN
+						MX28_PAD_GPMI_WRN__GPMI_WRN
+						MX28_PAD_GPMI_ALE__GPMI_ALE
+						MX28_PAD_GPMI_CLE__GPMI_CLE
+						MX28_PAD_GPMI_RESETN__GPMI_RESETN
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				gpmi_status_cfg: gpmi-status-cfg at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_GPMI_RDN__GPMI_RDN
+						MX28_PAD_GPMI_WRN__GPMI_WRN
+						MX28_PAD_GPMI_RESETN__GPMI_RESETN
+					>;
+					fsl,drive-strength = <MXS_DRIVE_12mA>;
+				};
+
+				auart0_pins_a: auart0 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_AUART0_RX__AUART0_RX
+						MX28_PAD_AUART0_TX__AUART0_TX
+						MX28_PAD_AUART0_CTS__AUART0_CTS
+						MX28_PAD_AUART0_RTS__AUART0_RTS
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				auart0_2pins_a: auart0-2pins at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_AUART0_RX__AUART0_RX
+						MX28_PAD_AUART0_TX__AUART0_TX
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				auart1_pins_a: auart1 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_AUART1_RX__AUART1_RX
+						MX28_PAD_AUART1_TX__AUART1_TX
+						MX28_PAD_AUART1_CTS__AUART1_CTS
+						MX28_PAD_AUART1_RTS__AUART1_RTS
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				auart1_2pins_a: auart1-2pins at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_AUART1_RX__AUART1_RX
+						MX28_PAD_AUART1_TX__AUART1_TX
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				auart2_2pins_a: auart2-2pins at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SSP2_SCK__AUART2_RX
+						MX28_PAD_SSP2_MOSI__AUART2_TX
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				auart2_2pins_b: auart2-2pins at 1 {
+					reg = <1>;
+					fsl,pinmux-ids = <
+						MX28_PAD_AUART2_RX__AUART2_RX
+						MX28_PAD_AUART2_TX__AUART2_TX
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				auart2_pins_a: auart2-pins at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_AUART2_RX__AUART2_RX
+						MX28_PAD_AUART2_TX__AUART2_TX
+						MX28_PAD_AUART2_CTS__AUART2_CTS
+						MX28_PAD_AUART2_RTS__AUART2_RTS
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				auart3_pins_a: auart3 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_AUART3_RX__AUART3_RX
+						MX28_PAD_AUART3_TX__AUART3_TX
+						MX28_PAD_AUART3_CTS__AUART3_CTS
+						MX28_PAD_AUART3_RTS__AUART3_RTS
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				auart3_2pins_a: auart3-2pins at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SSP2_MISO__AUART3_RX
+						MX28_PAD_SSP2_SS0__AUART3_TX
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				auart3_2pins_b: auart3-2pins at 1 {
+					reg = <1>;
+					fsl,pinmux-ids = <
+						MX28_PAD_AUART3_RX__AUART3_RX
+						MX28_PAD_AUART3_TX__AUART3_TX
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				auart4_2pins_a: auart4 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SSP3_SCK__AUART4_TX
+						MX28_PAD_SSP3_MOSI__AUART4_RX
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				auart4_2pins_b: auart4 at 1 {
+					reg = <1>;
+					fsl,pinmux-ids = <
+						MX28_PAD_AUART0_CTS__AUART4_RX
+						MX28_PAD_AUART0_RTS__AUART4_TX
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				mac0_pins_a: mac0 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_ENET0_MDC__ENET0_MDC
+						MX28_PAD_ENET0_MDIO__ENET0_MDIO
+						MX28_PAD_ENET0_RX_EN__ENET0_RX_EN
+						MX28_PAD_ENET0_RXD0__ENET0_RXD0
+						MX28_PAD_ENET0_RXD1__ENET0_RXD1
+						MX28_PAD_ENET0_TX_EN__ENET0_TX_EN
+						MX28_PAD_ENET0_TXD0__ENET0_TXD0
+						MX28_PAD_ENET0_TXD1__ENET0_TXD1
+						MX28_PAD_ENET_CLK__CLKCTRL_ENET
+					>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				mac0_pins_b: mac0 at 1 {
+					reg = <1>;
+					fsl,pinmux-ids = <
+						MX28_PAD_ENET0_MDC__ENET0_MDC
+						MX28_PAD_ENET0_MDIO__ENET0_MDIO
+						MX28_PAD_ENET0_RX_EN__ENET0_RX_EN
+						MX28_PAD_ENET0_RXD0__ENET0_RXD0
+						MX28_PAD_ENET0_RXD1__ENET0_RXD1
+						MX28_PAD_ENET0_RXD2__ENET0_RXD2
+						MX28_PAD_ENET0_RXD3__ENET0_RXD3
+						MX28_PAD_ENET0_TX_EN__ENET0_TX_EN
+						MX28_PAD_ENET0_TXD0__ENET0_TXD0
+						MX28_PAD_ENET0_TXD1__ENET0_TXD1
+						MX28_PAD_ENET0_TXD2__ENET0_TXD2
+						MX28_PAD_ENET0_TXD3__ENET0_TXD3
+						MX28_PAD_ENET_CLK__CLKCTRL_ENET
+						MX28_PAD_ENET0_COL__ENET0_COL
+						MX28_PAD_ENET0_CRS__ENET0_CRS
+						MX28_PAD_ENET0_TX_CLK__ENET0_TX_CLK
+						MX28_PAD_ENET0_RX_CLK__ENET0_RX_CLK
+						>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				mac1_pins_a: mac1 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_ENET0_CRS__ENET1_RX_EN
+						MX28_PAD_ENET0_RXD2__ENET1_RXD0
+						MX28_PAD_ENET0_RXD3__ENET1_RXD1
+						MX28_PAD_ENET0_COL__ENET1_TX_EN
+						MX28_PAD_ENET0_TXD2__ENET1_TXD0
+						MX28_PAD_ENET0_TXD3__ENET1_TXD1
+					>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				mmc0_8bit_pins_a: mmc0-8bit at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SSP0_DATA0__SSP0_D0
+						MX28_PAD_SSP0_DATA1__SSP0_D1
+						MX28_PAD_SSP0_DATA2__SSP0_D2
+						MX28_PAD_SSP0_DATA3__SSP0_D3
+						MX28_PAD_SSP0_DATA4__SSP0_D4
+						MX28_PAD_SSP0_DATA5__SSP0_D5
+						MX28_PAD_SSP0_DATA6__SSP0_D6
+						MX28_PAD_SSP0_DATA7__SSP0_D7
+						MX28_PAD_SSP0_CMD__SSP0_CMD
+						MX28_PAD_SSP0_DETECT__SSP0_CARD_DETECT
+						MX28_PAD_SSP0_SCK__SSP0_SCK
+					>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				mmc0_4bit_pins_a: mmc0-4bit at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SSP0_DATA0__SSP0_D0
+						MX28_PAD_SSP0_DATA1__SSP0_D1
+						MX28_PAD_SSP0_DATA2__SSP0_D2
+						MX28_PAD_SSP0_DATA3__SSP0_D3
+						MX28_PAD_SSP0_CMD__SSP0_CMD
+						MX28_PAD_SSP0_DETECT__SSP0_CARD_DETECT
+						MX28_PAD_SSP0_SCK__SSP0_SCK
+					>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				mmc0_cd_cfg: mmc0-cd-cfg at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SSP0_DETECT__SSP0_CARD_DETECT
+					>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				mmc0_sck_cfg: mmc0-sck-cfg at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SSP0_SCK__SSP0_SCK
+					>;
+					fsl,drive-strength = <MXS_DRIVE_12mA>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				mmc1_4bit_pins_a: mmc1-4bit at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_GPMI_D00__SSP1_D0
+						MX28_PAD_GPMI_D01__SSP1_D1
+						MX28_PAD_GPMI_D02__SSP1_D2
+						MX28_PAD_GPMI_D03__SSP1_D3
+						MX28_PAD_GPMI_RDY1__SSP1_CMD
+						MX28_PAD_GPMI_RDY0__SSP1_CARD_DETECT
+						MX28_PAD_GPMI_WRN__SSP1_SCK
+					>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				mmc1_cd_cfg: mmc1-cd-cfg at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_GPMI_RDY0__SSP1_CARD_DETECT
+					>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				mmc1_sck_cfg: mmc1-sck-cfg at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_GPMI_WRN__SSP1_SCK
+					>;
+					fsl,drive-strength = <MXS_DRIVE_12mA>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+
+				mmc2_4bit_pins_a: mmc2-4bit at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SSP0_DATA4__SSP2_D0
+						MX28_PAD_SSP1_SCK__SSP2_D1
+						MX28_PAD_SSP1_CMD__SSP2_D2
+						MX28_PAD_SSP0_DATA5__SSP2_D3
+						MX28_PAD_SSP0_DATA6__SSP2_CMD
+						MX28_PAD_AUART1_RX__SSP2_CARD_DETECT
+						MX28_PAD_SSP0_DATA7__SSP2_SCK
+					>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				mmc2_4bit_pins_b: mmc2-4bit at 1 {
+					reg = <1>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SSP2_SCK__SSP2_SCK
+						MX28_PAD_SSP2_MOSI__SSP2_CMD
+						MX28_PAD_SSP2_MISO__SSP2_D0
+						MX28_PAD_SSP2_SS0__SSP2_D3
+						MX28_PAD_SSP2_SS1__SSP2_D1
+						MX28_PAD_SSP2_SS2__SSP2_D2
+						MX28_PAD_AUART1_RX__SSP2_CARD_DETECT
+					>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				mmc2_cd_cfg: mmc2-cd-cfg at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_AUART1_RX__SSP2_CARD_DETECT
+					>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				mmc2_sck_cfg_a: mmc2-sck-cfg at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SSP0_DATA7__SSP2_SCK
+					>;
+					fsl,drive-strength = <MXS_DRIVE_12mA>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				mmc2_sck_cfg_b: mmc2-sck-cfg at 1 {
+					reg = <1>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SSP2_SCK__SSP2_SCK
+					>;
+					fsl,drive-strength = <MXS_DRIVE_12mA>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				i2c0_pins_a: i2c0 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_I2C0_SCL__I2C0_SCL
+						MX28_PAD_I2C0_SDA__I2C0_SDA
+					>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				i2c0_pins_b: i2c0 at 1 {
+					reg = <1>;
+					fsl,pinmux-ids = <
+						MX28_PAD_AUART0_RX__I2C0_SCL
+						MX28_PAD_AUART0_TX__I2C0_SDA
+					>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				i2c1_pins_a: i2c1 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_PWM0__I2C1_SCL
+						MX28_PAD_PWM1__I2C1_SDA
+					>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				i2c1_pins_b: i2c1 at 1 {
+					reg = <1>;
+					fsl,pinmux-ids = <
+						MX28_PAD_AUART2_CTS__I2C1_SCL
+						MX28_PAD_AUART2_RTS__I2C1_SDA
+					>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				saif0_pins_a: saif0 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SAIF0_MCLK__SAIF0_MCLK
+						MX28_PAD_SAIF0_LRCLK__SAIF0_LRCLK
+						MX28_PAD_SAIF0_BITCLK__SAIF0_BITCLK
+						MX28_PAD_SAIF0_SDATA0__SAIF0_SDATA0
+					>;
+					fsl,drive-strength = <MXS_DRIVE_12mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				saif0_pins_b: saif0 at 1 {
+					reg = <1>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SAIF0_LRCLK__SAIF0_LRCLK
+						MX28_PAD_SAIF0_BITCLK__SAIF0_BITCLK
+						MX28_PAD_SAIF0_SDATA0__SAIF0_SDATA0
+					>;
+					fsl,drive-strength = <MXS_DRIVE_12mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				saif1_pins_a: saif1 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SAIF1_SDATA0__SAIF1_SDATA0
+					>;
+					fsl,drive-strength = <MXS_DRIVE_12mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				pwm0_pins_a: pwm0 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_PWM0__PWM_0
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				pwm2_pins_a: pwm2 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_PWM2__PWM_2
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				pwm3_pins_a: pwm3 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_PWM3__PWM_3
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				pwm3_pins_b: pwm3 at 1 {
+					reg = <1>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SAIF0_MCLK__PWM_3
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				pwm4_pins_a: pwm4 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_PWM4__PWM_4
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				lcdif_24bit_pins_a: lcdif-24bit at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_LCD_D00__LCD_D0
+						MX28_PAD_LCD_D01__LCD_D1
+						MX28_PAD_LCD_D02__LCD_D2
+						MX28_PAD_LCD_D03__LCD_D3
+						MX28_PAD_LCD_D04__LCD_D4
+						MX28_PAD_LCD_D05__LCD_D5
+						MX28_PAD_LCD_D06__LCD_D6
+						MX28_PAD_LCD_D07__LCD_D7
+						MX28_PAD_LCD_D08__LCD_D8
+						MX28_PAD_LCD_D09__LCD_D9
+						MX28_PAD_LCD_D10__LCD_D10
+						MX28_PAD_LCD_D11__LCD_D11
+						MX28_PAD_LCD_D12__LCD_D12
+						MX28_PAD_LCD_D13__LCD_D13
+						MX28_PAD_LCD_D14__LCD_D14
+						MX28_PAD_LCD_D15__LCD_D15
+						MX28_PAD_LCD_D16__LCD_D16
+						MX28_PAD_LCD_D17__LCD_D17
+						MX28_PAD_LCD_D18__LCD_D18
+						MX28_PAD_LCD_D19__LCD_D19
+						MX28_PAD_LCD_D20__LCD_D20
+						MX28_PAD_LCD_D21__LCD_D21
+						MX28_PAD_LCD_D22__LCD_D22
+						MX28_PAD_LCD_D23__LCD_D23
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				lcdif_18bit_pins_a: lcdif-18bit at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_LCD_D00__LCD_D0
+						MX28_PAD_LCD_D01__LCD_D1
+						MX28_PAD_LCD_D02__LCD_D2
+						MX28_PAD_LCD_D03__LCD_D3
+						MX28_PAD_LCD_D04__LCD_D4
+						MX28_PAD_LCD_D05__LCD_D5
+						MX28_PAD_LCD_D06__LCD_D6
+						MX28_PAD_LCD_D07__LCD_D7
+						MX28_PAD_LCD_D08__LCD_D8
+						MX28_PAD_LCD_D09__LCD_D9
+						MX28_PAD_LCD_D10__LCD_D10
+						MX28_PAD_LCD_D11__LCD_D11
+						MX28_PAD_LCD_D12__LCD_D12
+						MX28_PAD_LCD_D13__LCD_D13
+						MX28_PAD_LCD_D14__LCD_D14
+						MX28_PAD_LCD_D15__LCD_D15
+						MX28_PAD_LCD_D16__LCD_D16
+						MX28_PAD_LCD_D17__LCD_D17
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				lcdif_16bit_pins_a: lcdif-16bit at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_LCD_D00__LCD_D0
+						MX28_PAD_LCD_D01__LCD_D1
+						MX28_PAD_LCD_D02__LCD_D2
+						MX28_PAD_LCD_D03__LCD_D3
+						MX28_PAD_LCD_D04__LCD_D4
+						MX28_PAD_LCD_D05__LCD_D5
+						MX28_PAD_LCD_D06__LCD_D6
+						MX28_PAD_LCD_D07__LCD_D7
+						MX28_PAD_LCD_D08__LCD_D8
+						MX28_PAD_LCD_D09__LCD_D9
+						MX28_PAD_LCD_D10__LCD_D10
+						MX28_PAD_LCD_D11__LCD_D11
+						MX28_PAD_LCD_D12__LCD_D12
+						MX28_PAD_LCD_D13__LCD_D13
+						MX28_PAD_LCD_D14__LCD_D14
+						MX28_PAD_LCD_D15__LCD_D15
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				lcdif_sync_pins_a: lcdif-sync at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_LCD_RS__LCD_DOTCLK
+						MX28_PAD_LCD_CS__LCD_ENABLE
+						MX28_PAD_LCD_RD_E__LCD_VSYNC
+						MX28_PAD_LCD_WR_RWN__LCD_HSYNC
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				can0_pins_a: can0 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_GPMI_RDY2__CAN0_TX
+						MX28_PAD_GPMI_RDY3__CAN0_RX
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				can1_pins_a: can1 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_GPMI_CE2N__CAN1_TX
+						MX28_PAD_GPMI_CE3N__CAN1_RX
+					>;
+					fsl,drive-strength = <MXS_DRIVE_4mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				spi2_pins_a: spi2 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SSP2_SCK__SSP2_SCK
+						MX28_PAD_SSP2_MOSI__SSP2_CMD
+						MX28_PAD_SSP2_MISO__SSP2_D0
+						MX28_PAD_SSP2_SS0__SSP2_D3
+					>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				spi3_pins_a: spi3 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_AUART2_RX__SSP3_D4
+						MX28_PAD_AUART2_TX__SSP3_D5
+						MX28_PAD_SSP3_SCK__SSP3_SCK
+						MX28_PAD_SSP3_MOSI__SSP3_CMD
+						MX28_PAD_SSP3_MISO__SSP3_D0
+						MX28_PAD_SSP3_SS0__SSP3_D3
+					>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				spi3_pins_b: spi3 at 1 {
+					reg = <1>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SSP3_SCK__SSP3_SCK
+						MX28_PAD_SSP3_MOSI__SSP3_CMD
+						MX28_PAD_SSP3_MISO__SSP3_D0
+						MX28_PAD_SSP3_SS0__SSP3_D3
+					>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				usb0_pins_a: usb0 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SSP2_SS2__USB0_OVERCURRENT
+					>;
+					fsl,drive-strength = <MXS_DRIVE_12mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				usb0_pins_b: usb0 at 1 {
+					reg = <1>;
+					fsl,pinmux-ids = <
+						MX28_PAD_AUART1_CTS__USB0_OVERCURRENT
+					>;
+					fsl,drive-strength = <MXS_DRIVE_12mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				usb1_pins_a: usb1 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_SSP2_SS1__USB1_OVERCURRENT
+					>;
+					fsl,drive-strength = <MXS_DRIVE_12mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_DISABLE>;
+				};
+
+				usb0_id_pins_a: usb0id at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_AUART1_RTS__USB0_ID
+					>;
+					fsl,drive-strength = <MXS_DRIVE_12mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				usb0_id_pins_b: usb0id1 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX28_PAD_PWM2__USB0_ID
+					>;
+					fsl,drive-strength = <MXS_DRIVE_12mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+			};
+
+			digctl: digctl at 8001c000 {
+				compatible = "fsl,imx28-digctl", "fsl,imx23-digctl";
+				reg = <0x8001c000 0x2000>;
+				interrupts = <89>;
+				status = "disabled";
+			};
+
+			etm: etm at 80022000 {
+				reg = <0x80022000 0x2000>;
+				status = "disabled";
+			};
+
+			dma_apbx: dma-apbx at 80024000 {
+				compatible = "fsl,imx28-dma-apbx";
+				reg = <0x80024000 0x2000>;
+				interrupts = <78 79 66 0
+					      80 81 68 69
+					      70 71 72 73
+					      74 75 76 77>;
+				interrupt-names = "auart4-rx", "auart4-tx", "spdif-tx", "empty",
+						  "saif0", "saif1", "i2c0", "i2c1",
+						  "auart0-rx", "auart0-tx", "auart1-rx", "auart1-tx",
+						  "auart2-rx", "auart2-tx", "auart3-rx", "auart3-tx";
+				#dma-cells = <1>;
+				dma-channels = <16>;
+				clocks = <&clks 26>;
+			};
+
+			dcp: dcp at 80028000 {
+				compatible = "fsl,imx28-dcp", "fsl,imx23-dcp";
+				reg = <0x80028000 0x2000>;
+				interrupts = <52 53 54>;
+				status = "okay";
+			};
+
+			pxp: pxp at 8002a000 {
+				reg = <0x8002a000 0x2000>;
+				interrupts = <39>;
+				status = "disabled";
+			};
+
+			ocotp: ocotp at 8002c000 {
+				compatible = "fsl,imx28-ocotp", "fsl,ocotp";
+				#address-cells = <1>;
+				#size-cells = <1>;
+				reg = <0x8002c000 0x2000>;
+				clocks = <&clks 25>;
+			};
+
+			axi-ahb at 8002e000 {
+				reg = <0x8002e000 0x2000>;
+				status = "disabled";
+			};
+
+			lcdif: lcdif at 80030000 {
+				compatible = "fsl,imx28-lcdif";
+				reg = <0x80030000 0x2000>;
+				interrupts = <38>;
+				clocks = <&clks 55>;
+				dmas = <&dma_apbh 13>;
+				dma-names = "rx";
+				status = "disabled";
+			};
+
+			can0: can at 80032000 {
+				compatible = "fsl,imx28-flexcan";
+				reg = <0x80032000 0x2000>;
+				interrupts = <8>;
+				clocks = <&clks 58>, <&clks 58>;
+				clock-names = "ipg", "per";
+				status = "disabled";
+			};
+
+			can1: can at 80034000 {
+				compatible = "fsl,imx28-flexcan";
+				reg = <0x80034000 0x2000>;
+				interrupts = <9>;
+				clocks = <&clks 59>, <&clks 59>;
+				clock-names = "ipg", "per";
+				status = "disabled";
+			};
+
+			simdbg: simdbg at 8003c000 {
+				reg = <0x8003c000 0x200>;
+				status = "disabled";
+			};
+
+			simgpmisel: simgpmisel at 8003c200 {
+				reg = <0x8003c200 0x100>;
+				status = "disabled";
+			};
+
+			simsspsel: simsspsel at 8003c300 {
+				reg = <0x8003c300 0x100>;
+				status = "disabled";
+			};
+
+			simmemsel: simmemsel at 8003c400 {
+				reg = <0x8003c400 0x100>;
+				status = "disabled";
+			};
+
+			gpiomon: gpiomon at 8003c500 {
+				reg = <0x8003c500 0x100>;
+				status = "disabled";
+			};
+
+			simenet: simenet at 8003c700 {
+				reg = <0x8003c700 0x100>;
+				status = "disabled";
+			};
+
+			armjtag: armjtag at 8003c800 {
+				reg = <0x8003c800 0x100>;
+				status = "disabled";
+			};
+		};
+
+		apbx at 80040000 {
+			compatible = "simple-bus";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			reg = <0x80040000 0x40000>;
+			ranges;
+
+			clks: clkctrl at 80040000 {
+				compatible = "fsl,imx28-clkctrl", "fsl,clkctrl";
+				reg = <0x80040000 0x2000>;
+				#clock-cells = <1>;
+			};
+
+			saif0: saif at 80042000 {
+				#sound-dai-cells = <0>;
+				compatible = "fsl,imx28-saif";
+				reg = <0x80042000 0x2000>;
+				interrupts = <59>;
+				#clock-cells = <0>;
+				clocks = <&clks 53>;
+				dmas = <&dma_apbx 4>;
+				dma-names = "rx-tx";
+				status = "disabled";
+			};
+
+			power: power at 80044000 {
+				reg = <0x80044000 0x2000>;
+				status = "disabled";
+			};
+
+			saif1: saif at 80046000 {
+				#sound-dai-cells = <0>;
+				compatible = "fsl,imx28-saif";
+				reg = <0x80046000 0x2000>;
+				interrupts = <58>;
+				clocks = <&clks 54>;
+				dmas = <&dma_apbx 5>;
+				dma-names = "rx-tx";
+				status = "disabled";
+			};
+
+			lradc: lradc at 80050000 {
+				compatible = "fsl,imx28-lradc";
+				reg = <0x80050000 0x2000>;
+				interrupts = <10 14 15 16 17 18 19
+						20 21 22 23 24 25>;
+				status = "disabled";
+				clocks = <&clks 41>;
+				#io-channel-cells = <1>;
+			};
+
+			spdif: spdif at 80054000 {
+				reg = <0x80054000 0x2000>;
+				interrupts = <45>;
+				dmas = <&dma_apbx 2>;
+				dma-names = "tx";
+				status = "disabled";
+			};
+
+			mxs_rtc: rtc at 80056000 {
+				compatible = "fsl,imx28-rtc", "fsl,stmp3xxx-rtc";
+				reg = <0x80056000 0x2000>;
+				interrupts = <29>;
+			};
+
+			i2c0: i2c at 80058000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "fsl,imx28-i2c";
+				reg = <0x80058000 0x2000>;
+				interrupts = <111>;
+				clock-frequency = <100000>;
+				dmas = <&dma_apbx 6>;
+				dma-names = "rx-tx";
+				status = "disabled";
+			};
+
+			i2c1: i2c at 8005a000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "fsl,imx28-i2c";
+				reg = <0x8005a000 0x2000>;
+				interrupts = <110>;
+				clock-frequency = <100000>;
+				dmas = <&dma_apbx 7>;
+				dma-names = "rx-tx";
+				status = "disabled";
+			};
+
+			pwm: pwm at 80064000 {
+				compatible = "fsl,imx28-pwm", "fsl,imx23-pwm";
+				reg = <0x80064000 0x2000>;
+				clocks = <&clks 44>;
+				#pwm-cells = <2>;
+				fsl,pwm-number = <8>;
+				status = "disabled";
+			};
+
+			timer: timrot at 80068000 {
+				compatible = "fsl,imx28-timrot", "fsl,timrot";
+				reg = <0x80068000 0x2000>;
+				interrupts = <48 49 50 51>;
+				clocks = <&clks 26>;
+			};
+
+			auart0: serial at 8006a000 {
+				compatible = "fsl,imx28-auart", "fsl,imx23-auart";
+				reg = <0x8006a000 0x2000>;
+				interrupts = <112>;
+				dmas = <&dma_apbx 8>, <&dma_apbx 9>;
+				dma-names = "rx", "tx";
+				clocks = <&clks 45>;
+				status = "disabled";
+			};
+
+			auart1: serial at 8006c000 {
+				compatible = "fsl,imx28-auart", "fsl,imx23-auart";
+				reg = <0x8006c000 0x2000>;
+				interrupts = <113>;
+				dmas = <&dma_apbx 10>, <&dma_apbx 11>;
+				dma-names = "rx", "tx";
+				clocks = <&clks 45>;
+				status = "disabled";
+			};
+
+			auart2: serial at 8006e000 {
+				compatible = "fsl,imx28-auart", "fsl,imx23-auart";
+				reg = <0x8006e000 0x2000>;
+				interrupts = <114>;
+				dmas = <&dma_apbx 12>, <&dma_apbx 13>;
+				dma-names = "rx", "tx";
+				clocks = <&clks 45>;
+				status = "disabled";
+			};
+
+			auart3: serial at 80070000 {
+				compatible = "fsl,imx28-auart", "fsl,imx23-auart";
+				reg = <0x80070000 0x2000>;
+				interrupts = <115>;
+				dmas = <&dma_apbx 14>, <&dma_apbx 15>;
+				dma-names = "rx", "tx";
+				clocks = <&clks 45>;
+				status = "disabled";
+			};
+
+			auart4: serial at 80072000 {
+				compatible = "fsl,imx28-auart", "fsl,imx23-auart";
+				reg = <0x80072000 0x2000>;
+				interrupts = <116>;
+				dmas = <&dma_apbx 0>, <&dma_apbx 1>;
+				dma-names = "rx", "tx";
+				clocks = <&clks 45>;
+				status = "disabled";
+			};
+
+			duart: serial at 80074000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x80074000 0x1000>;
+				interrupts = <47>;
+				clocks = <&clks 45>, <&clks 26>;
+				clock-names = "uart", "apb_pclk";
+				status = "disabled";
+			};
+
+			usbphy0: usbphy at 8007c000 {
+				compatible = "fsl,imx28-usbphy", "fsl,imx23-usbphy";
+				reg = <0x8007c000 0x2000>;
+				clocks = <&clks 62>;
+				status = "disabled";
+			};
+
+			usbphy1: usbphy at 8007e000 {
+				compatible = "fsl,imx28-usbphy", "fsl,imx23-usbphy";
+				reg = <0x8007e000 0x2000>;
+				clocks = <&clks 63>;
+				status = "disabled";
+			};
+		};
+	};
+
+	ahb at 80080000 {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x80080000 0x80000>;
+		ranges;
+
+		usb0: usb at 80080000 {
+			compatible = "fsl,imx28-usb", "fsl,imx27-usb";
+			reg = <0x80080000 0x10000>;
+			interrupts = <93>;
+			clocks = <&clks 60>;
+			fsl,usbphy = <&usbphy0>;
+			status = "disabled";
+		};
+
+		usb1: usb at 80090000 {
+			compatible = "fsl,imx28-usb", "fsl,imx27-usb";
+			reg = <0x80090000 0x10000>;
+			interrupts = <92>;
+			clocks = <&clks 61>;
+			fsl,usbphy = <&usbphy1>;
+			dr_mode = "host";
+			status = "disabled";
+		};
+
+		dflpt: dflpt at 800c0000 {
+			reg = <0x800c0000 0x10000>;
+			status = "disabled";
+		};
+
+		mac0: ethernet at 800f0000 {
+			compatible = "fsl,imx28-fec";
+			reg = <0x800f0000 0x4000>;
+			interrupts = <101>;
+			clocks = <&clks 57>, <&clks 57>, <&clks 64>;
+			clock-names = "ipg", "ahb", "enet_out";
+			status = "disabled";
+		};
+
+		mac1: ethernet at 800f4000 {
+			compatible = "fsl,imx28-fec";
+			reg = <0x800f4000 0x4000>;
+			interrupts = <102>;
+			clocks = <&clks 57>, <&clks 57>;
+			clock-names = "ipg", "ahb";
+			status = "disabled";
+		};
+
+		etn_switch: switch at 800f8000 {
+			reg = <0x800f8000 0x8000>;
+			status = "disabled";
+		};
+	};
+
+	iio-hwmon {
+		compatible = "iio-hwmon";
+		io-channels = <&lradc 8>;
+	};
+};
diff --git a/arch/arm/dts/mxs-pinfunc.h b/arch/arm/dts/mxs-pinfunc.h
new file mode 100644
index 0000000000..c6da987b20
--- /dev/null
+++ b/arch/arm/dts/mxs-pinfunc.h
@@ -0,0 +1,31 @@
+/*
+ * Header providing constants for i.MX28 pinctrl bindings.
+ *
+ * Copyright (C) 2013 Lothar Waßmann <LW@KARO-electronics.de>
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#ifndef __DT_BINDINGS_MXS_PINCTRL_H__
+#define __DT_BINDINGS_MXS_PINCTRL_H__
+
+/* fsl,drive-strength property */
+#define MXS_DRIVE_4mA		0
+#define MXS_DRIVE_8mA		1
+#define MXS_DRIVE_12mA		2
+#define MXS_DRIVE_16mA		3
+
+/* fsl,voltage property */
+#define MXS_VOLTAGE_LOW		0
+#define MXS_VOLTAGE_HIGH	1
+
+/* fsl,pull-up property */
+#define MXS_PULL_DISABLE	0
+#define MXS_PULL_ENABLE		1
+
+#endif /* __DT_BINDINGS_MXS_PINCTRL_H__ */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 2/5] ARM: imx: net: Enable support for i.MX28 DM_ETH in the fec_mxc.c driver
  2019-06-15 22:34 [U-Boot] [PATCH v3 0/5] DM: Convert i.MX28 gpio, pinmux, spi and eth drivers to DM/DTS Lukasz Majewski
  2019-06-15 22:34 ` [U-Boot] [PATCH v3 1/5] ARM: dts: imx: Copy imx28 device tree related files from Linux kernel (v5.1.9) Lukasz Majewski
@ 2019-06-15 22:34 ` Lukasz Majewski
  2019-06-15 22:43   ` Marek Vasut
  2019-06-15 22:34 ` [U-Boot] [PATCH v3 3/5] ARM: dm: mxs: gpio: Add support for DM/DTS in the mxs_gpio.c driver (DM_GPIO) Lukasz Majewski
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 32+ messages in thread
From: Lukasz Majewski @ 2019-06-15 22:34 UTC (permalink / raw)
  To: u-boot

The fec_mxc.c driver can be reused by i.MX28 when DM_ETH is enabled.
One only needs to add proper compatible and dependency on FEC_MXC in the
Kconfig.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---

Changes in v3:
- Set more apropriate tags

Changes in v2: None

 drivers/net/Kconfig   | 2 +-
 drivers/net/fec_mxc.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index e6a4fdf30e..04f886fe86 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -190,7 +190,7 @@ config FEC_MXC_MDIO_BASE
 
 config FEC_MXC
 	bool "FEC Ethernet controller"
-	depends on MX5 || MX6 || MX7 || IMX8 || VF610
+	depends on MX5 || MX6 || MX7 || IMX8 || VF610 || MX28
 	help
 	  This driver supports the 10/100 Fast Ethernet controller for
 	  NXP i.MX processors.
diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index d7c080943a..4042dcf9ca 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -1492,6 +1492,7 @@ static const struct udevice_id fecmxc_ids[] = {
 	{ .compatible = "fsl,imx53-fec" },
 	{ .compatible = "fsl,imx7d-fec" },
 	{ .compatible = "fsl,mvf600-fec" },
+	{ .compatible = "fsl,imx28-fec" },
 	{ }
 };
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 3/5] ARM: dm: mxs: gpio: Add support for DM/DTS in the mxs_gpio.c driver (DM_GPIO)
  2019-06-15 22:34 [U-Boot] [PATCH v3 0/5] DM: Convert i.MX28 gpio, pinmux, spi and eth drivers to DM/DTS Lukasz Majewski
  2019-06-15 22:34 ` [U-Boot] [PATCH v3 1/5] ARM: dts: imx: Copy imx28 device tree related files from Linux kernel (v5.1.9) Lukasz Majewski
  2019-06-15 22:34 ` [U-Boot] [PATCH v3 2/5] ARM: imx: net: Enable support for i.MX28 DM_ETH in the fec_mxc.c driver Lukasz Majewski
@ 2019-06-15 22:34 ` Lukasz Majewski
  2019-06-15 22:53   ` Marek Vasut
  2019-06-15 22:34 ` [U-Boot] [PATCH v3 4/5] ARM: imx: pinctrl: Add support for i.MX28 mxs pinctrl driver Lukasz Majewski
  2019-06-15 22:34 ` [U-Boot] [PATCH v3 5/5] ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI conversion) Lukasz Majewski
  4 siblings, 1 reply; 32+ messages in thread
From: Lukasz Majewski @ 2019-06-15 22:34 UTC (permalink / raw)
  To: u-boot

This commit adds support for DM in the mxs_gpio.c driver when DM_GPIO
is enabled in Kconfig.

Signed-off-by: Lukasz Majewski <lukma@denx.de>

---

Changes in v3:
- Set more apropriate tags

Changes in v2:
- Use #if !CONFIG_IS_ENABLED(DM_GPIO) instead of plain #ifdef CONFIG_DM_GPIO

 arch/arm/include/asm/arch-mxs/gpio.h |  28 +++++++
 drivers/gpio/mxs_gpio.c              | 149 +++++++++++++++++++++++++++++++++++
 2 files changed, 177 insertions(+)

diff --git a/arch/arm/include/asm/arch-mxs/gpio.h b/arch/arm/include/asm/arch-mxs/gpio.h
index 34fa421945..0c152e25e2 100644
--- a/arch/arm/include/asm/arch-mxs/gpio.h
+++ b/arch/arm/include/asm/arch-mxs/gpio.h
@@ -15,4 +15,32 @@ void mxs_gpio_init(void);
 inline void mxs_gpio_init(void) {}
 #endif
 
+#if defined(CONFIG_MX28) && CONFIG_IS_ENABLED(DM_GPIO)
+/*
+ * According to i.MX28 Reference Manual:
+ * 'i.MX28 Applications Processor Reference Manual, Rev. 1, 2010'
+ * The i.MX28 has following number of GPIOs available:
+ * Bank 0: 0-28 -> 29 PINS
+ * Bank 1: 0-31 -> 32 PINS
+ * Bank 2: 0-27 -> 28 PINS
+ * Bank 3: 0-30 -> 31 PINS
+ * Bank 4: 0-20 -> 21 PINS
+ */
+#define IMX28_GPIO_BANK0_PIN_NR 29
+#define IMX28_GPIO_BANK1_PIN_NR 32
+#define IMX28_GPIO_BANK2_PIN_NR 28
+#define IMX28_GPIO_BANK3_PIN_NR 31
+#define IMX28_GPIO_BANK4_PIN_NR 21
+#define IMX28_GPIO_BANK_NR 5
+
+struct mxs_gpio_platdata {
+	u32 gpio_nr_of_bank_pins[IMX28_GPIO_BANK_NR];
+	u32 gpio_base_nr[IMX28_GPIO_BANK_NR];
+	u32 ngpio;
+};
+
+extern const struct mxs_gpio_platdata mxs_gpio_def;
+#define IMX_GPIO_NR(port, index) (mxs_gpio_def.gpio_base_nr[(port)] + (index))
+#endif
+
 #endif	/* __MX28_GPIO_H__ */
diff --git a/drivers/gpio/mxs_gpio.c b/drivers/gpio/mxs_gpio.c
index c2c8a25886..f386235ff1 100644
--- a/drivers/gpio/mxs_gpio.c
+++ b/drivers/gpio/mxs_gpio.c
@@ -51,6 +51,7 @@ void mxs_gpio_init(void)
 	}
 }
 
+#if !CONFIG_IS_ENABLED(DM_GPIO)
 int gpio_get_value(unsigned gpio)
 {
 	uint32_t bank = PAD_BANK(gpio);
@@ -127,3 +128,151 @@ int name_to_gpio(const char *name)
 
 	return (bank << MXS_PAD_BANK_SHIFT) | (pin << MXS_PAD_PIN_SHIFT);
 }
+#else /* CONFIG_DM_GPIO */
+#include <dm.h>
+#include <asm/gpio.h>
+#include <asm/arch/gpio.h>
+#ifdef CONFIG_MX28
+const struct mxs_gpio_platdata mxs_gpio_def = {
+	.gpio_nr_of_bank_pins[0] = IMX28_GPIO_BANK0_PIN_NR,
+	.gpio_nr_of_bank_pins[1] = IMX28_GPIO_BANK1_PIN_NR,
+	.gpio_nr_of_bank_pins[2] = IMX28_GPIO_BANK2_PIN_NR,
+	.gpio_nr_of_bank_pins[3] = IMX28_GPIO_BANK3_PIN_NR,
+	.gpio_nr_of_bank_pins[4] = IMX28_GPIO_BANK4_PIN_NR,
+	.gpio_base_nr[0] = 0,
+	.gpio_base_nr[1] = IMX28_GPIO_BANK0_PIN_NR,
+	.gpio_base_nr[2] = (IMX28_GPIO_BANK0_PIN_NR + \
+			   IMX28_GPIO_BANK1_PIN_NR),
+	.gpio_base_nr[3] = (IMX28_GPIO_BANK0_PIN_NR + \
+			   IMX28_GPIO_BANK1_PIN_NR + \
+			   IMX28_GPIO_BANK2_PIN_NR),
+	.gpio_base_nr[4] = (IMX28_GPIO_BANK0_PIN_NR + \
+			   IMX28_GPIO_BANK1_PIN_NR + \
+			   IMX28_GPIO_BANK2_PIN_NR + \
+			   IMX28_GPIO_BANK3_PIN_NR),
+	.ngpio = (IMX28_GPIO_BANK0_PIN_NR + \
+		  IMX28_GPIO_BANK1_PIN_NR + \
+		  IMX28_GPIO_BANK2_PIN_NR + \
+		  IMX28_GPIO_BANK3_PIN_NR + \
+		  IMX28_GPIO_BANK4_PIN_NR),
+};
+
+#else
+#error "Only i.MX28 supported with DM_GPIO"
+#endif
+
+struct mxs_gpio_priv {
+	unsigned int bank;
+};
+
+static int mxs_gpio_get_value(struct udevice *dev, unsigned offset)
+{
+	struct mxs_gpio_priv *priv = dev_get_priv(dev);
+	struct mxs_register_32 *reg =
+		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
+					   PINCTRL_DIN(priv->bank));
+
+	return (readl(&reg->reg) >> offset) & 1;
+}
+
+static int mxs_gpio_set_value(struct udevice *dev, unsigned offset,
+			      int value)
+{
+	struct mxs_gpio_priv *priv = dev_get_priv(dev);
+	struct mxs_register_32 *reg =
+		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
+					   PINCTRL_DOUT(priv->bank));
+	if (value)
+		writel(1 << offset, &reg->reg_set);
+	else
+		writel(1 << offset, &reg->reg_clr);
+
+	return 0;
+}
+
+static int mxs_gpio_direction_input(struct udevice *dev, unsigned offset)
+{
+	struct mxs_gpio_priv *priv = dev_get_priv(dev);
+	struct mxs_register_32 *reg =
+		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
+					   PINCTRL_DOE(priv->bank));
+
+	writel(1 << offset, &reg->reg_clr);
+
+	return 0;
+}
+
+static int mxs_gpio_direction_output(struct udevice *dev, unsigned offset,
+				     int value)
+{
+	struct mxs_gpio_priv *priv = dev_get_priv(dev);
+	struct mxs_register_32 *reg =
+		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
+					   PINCTRL_DOE(priv->bank));
+
+	mxs_gpio_set_value(dev, offset, value);
+
+	writel(1 << offset, &reg->reg_set);
+
+	return 0;
+}
+
+static int mxs_gpio_get_function(struct udevice *dev, unsigned offset)
+{
+	struct mxs_gpio_priv *priv = dev_get_priv(dev);
+	struct mxs_register_32 *reg =
+		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
+					   PINCTRL_DOE(priv->bank));
+	bool is_output = !!(readl(&reg->reg) >> offset);
+
+	return is_output ? GPIOF_OUTPUT : GPIOF_INPUT;
+}
+
+static const struct dm_gpio_ops gpio_mxs_ops = {
+	.direction_input	= mxs_gpio_direction_input,
+	.direction_output	= mxs_gpio_direction_output,
+	.get_value		= mxs_gpio_get_value,
+	.set_value		= mxs_gpio_set_value,
+	.get_function		= mxs_gpio_get_function,
+};
+
+static int mxs_gpio_probe(struct udevice *dev)
+{
+	struct mxs_gpio_priv *priv = dev_get_priv(dev);
+	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+	struct mxs_gpio_platdata *pdata =
+		(struct mxs_gpio_platdata *)dev_get_driver_data(dev);
+	char name[18], *str;
+	int ret;
+
+	ret = dev_read_u32(dev, "reg", &priv->bank);
+	if (ret) {
+		printf("%s: No 'reg' property defined!\n", __func__);
+		return ret;
+	}
+
+	sprintf(name, "GPIO%d_", priv->bank);
+	str = strdup(name);
+	if (!str)
+		return -ENOMEM;
+
+	uc_priv->bank_name = str;
+	uc_priv->gpio_count = pdata->gpio_nr_of_bank_pins[priv->bank];
+
+	return 0;
+}
+
+static const struct udevice_id mxs_gpio_ids[] = {
+	{ .compatible = "fsl,imx28-gpio", .data = (ulong)&mxs_gpio_def },
+	{ }
+};
+
+U_BOOT_DRIVER(gpio_mxs) = {
+	.name	= "gpio_mxs",
+	.id	= UCLASS_GPIO,
+	.ops	= &gpio_mxs_ops,
+	.probe	= mxs_gpio_probe,
+	.priv_auto_alloc_size = sizeof(struct mxs_gpio_priv),
+	.of_match = mxs_gpio_ids,
+};
+#endif /* CONFIG_DM_GPIO */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 4/5] ARM: imx: pinctrl: Add support for i.MX28 mxs pinctrl driver
  2019-06-15 22:34 [U-Boot] [PATCH v3 0/5] DM: Convert i.MX28 gpio, pinmux, spi and eth drivers to DM/DTS Lukasz Majewski
                   ` (2 preceding siblings ...)
  2019-06-15 22:34 ` [U-Boot] [PATCH v3 3/5] ARM: dm: mxs: gpio: Add support for DM/DTS in the mxs_gpio.c driver (DM_GPIO) Lukasz Majewski
@ 2019-06-15 22:34 ` Lukasz Majewski
  2019-06-15 23:00   ` Marek Vasut
  2019-06-15 22:34 ` [U-Boot] [PATCH v3 5/5] ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI conversion) Lukasz Majewski
  4 siblings, 1 reply; 32+ messages in thread
From: Lukasz Majewski @ 2019-06-15 22:34 UTC (permalink / raw)
  To: u-boot

The code responsible for setting proper values in the MUX registers
(in the mxs_pinctrl_set_state()) has been ported from Barebox project
(branch: master, SHA1: eb3b0f7414cd8102844dd16b1c789e445e8947f8,
file: drivers/pinctrl/pinctrl-mxs.c).

As the pinctrl node in the imx28.dtsi file has gpio pins nodes as subnodes,
it was necessary to use 'dm_scan_fdt_dev()' (as a .bind method) to also
make them 'visible' by the DM's "gpio_mxs" driver.

Signed-off-by: Lukasz Majewski <lukma@denx.de>

---

Changes in v3:
- Set more apropriate tags

Changes in v2: None

 drivers/pinctrl/nxp/Kconfig       |  10 +++
 drivers/pinctrl/nxp/Makefile      |   1 +
 drivers/pinctrl/nxp/pinctrl-mxs.c | 164 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 175 insertions(+)
 create mode 100644 drivers/pinctrl/nxp/pinctrl-mxs.c

diff --git a/drivers/pinctrl/nxp/Kconfig b/drivers/pinctrl/nxp/Kconfig
index 61f93be42d..f2e67ca231 100644
--- a/drivers/pinctrl/nxp/Kconfig
+++ b/drivers/pinctrl/nxp/Kconfig
@@ -89,6 +89,16 @@ config PINCTRL_IMX8M
 	  only parses the 'fsl,pins' property and configure related
 	  registers.
 
+config PINCTRL_MXS
+	bool "NXP MXS pinctrl driver"
+	depends on ARCH_MX28 && PINCTRL_FULL
+	help
+	  Say Y here to enable the i.MX mxs pinctrl driver
+
+	  This option provides a simple pinctrl driver for i.MX mxs SoC
+	  familiy, e.g. i.MX28. This feature depends on device tree
+	  configuration.
+
 config PINCTRL_VYBRID
 	bool "Vybrid (vf610) pinctrl driver"
 	depends on ARCH_VF610 && PINCTRL_FULL
diff --git a/drivers/pinctrl/nxp/Makefile b/drivers/pinctrl/nxp/Makefile
index b340d9448a..b86448aac9 100644
--- a/drivers/pinctrl/nxp/Makefile
+++ b/drivers/pinctrl/nxp/Makefile
@@ -6,4 +6,5 @@ obj-$(CONFIG_PINCTRL_IMX7ULP)		+= pinctrl-imx7ulp.o
 obj-$(CONFIG_PINCTRL_IMX_SCU)		+= pinctrl-scu.o
 obj-$(CONFIG_PINCTRL_IMX8)		+= pinctrl-imx8.o
 obj-$(CONFIG_PINCTRL_IMX8M)		+= pinctrl-imx8m.o
+obj-$(CONFIG_PINCTRL_MXS)		+= pinctrl-mxs.o
 obj-$(CONFIG_PINCTRL_VYBRID)		+= pinctrl-vf610.o
diff --git a/drivers/pinctrl/nxp/pinctrl-mxs.c b/drivers/pinctrl/nxp/pinctrl-mxs.c
new file mode 100644
index 0000000000..42b1b7998b
--- /dev/null
+++ b/drivers/pinctrl/nxp/pinctrl-mxs.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 DENX Software Engineering
+ * Lukasz Majewski, DENX Software Engineering, lukma at denx.de
+ *
+ * This work is based on drivers/pinctrl/pinctrl-mxs.c
+ * SHA1: eb3b0f7414cd8102844dd16b1c789e445e8947f8
+ * from Barebox project.
+ *
+ * Author of original Barebox pinctrl-mxs.c:
+ * Copyright (c) 2015 Sascha Hauer <s.hauer@pengutronix.de>
+ */
+
+#include <common.h>
+#include <linux/io.h>
+#include <linux/err.h>
+#include <dm.h>
+#include <dm/pinctrl.h>
+#include <dm/read.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define PINID(bank, pin)	((bank) * 32 + (pin))
+#define MUXID_TO_PINID(m)	PINID((m) >> 12 & 0xf, (m) >> 4 & 0xff)
+#define MUXID_TO_MUXSEL(m)	((m) & 0xf)
+#define PINID_TO_BANK(p)	((p) >> 5)
+#define PINID_TO_PIN(p)	((p) % 32)
+
+#define STMP_OFFSET_REG_SET     0x4
+#define STMP_OFFSET_REG_CLR     0x8
+#define STMP_OFFSET_REG_TOG     0xc
+
+struct mxs_pinctrl_priv {
+	void __iomem *base;
+};
+
+static int mxs_pinctrl_set_state(struct udevice *dev, struct udevice *config)
+{
+	int ma_present = 0, vol_present = 0, pull_present = 0;
+	struct mxs_pinctrl_priv *iomux = dev_get_priv(dev);
+	u32 *pin_data, val, ma, vol, pull;
+	int npins, size, i, ret;
+
+	debug("\n%s: set state: %s\n", __func__, config->name);
+
+	size = dev_read_size(config, "fsl,pinmux-ids");
+	if (size < 0)
+		return size;
+
+	if (!size || size % 4) {
+		dev_err(dev, "Invalid fsl,pinmux-ids property in %s\n",
+			config->name);
+		return -EINVAL;
+	}
+
+	npins = size / 4;
+
+	pin_data = devm_kzalloc(dev, size, 0);
+	if (!pin_data)
+		return -ENOMEM;
+
+	ret = dev_read_u32_array(config, "fsl,pinmux-ids", pin_data, npins);
+	if (ret) {
+		dev_err(dev, "Error reading pin data.\n");
+		devm_kfree(dev, pin_data);
+		return -EINVAL;
+	}
+
+	ret = dev_read_u32(config, "fsl,drive-strength", &ma);
+	if (!ret)
+		ma_present = 1;
+
+	ret = dev_read_u32(config, "fsl,voltage", &vol);
+	if (!ret)
+		vol_present = 1;
+
+	ret = dev_read_u32(config, "fsl,pull-up", &pull);
+	if (!ret)
+		pull_present = 1;
+
+	for (i = 0; i < npins; i++) {
+		int muxsel, pinid, bank, pin, shift;
+		void __iomem *reg;
+
+		val = *pin_data++;
+
+		muxsel = MUXID_TO_MUXSEL(val);
+		pinid = MUXID_TO_PINID(val);
+
+		bank = PINID_TO_BANK(pinid);
+		pin = PINID_TO_PIN(pinid);
+		reg = iomux->base + 0x100;
+		reg += bank * 0x20 + pin / 16 * 0x10;
+		shift = pin % 16 * 2;
+
+		writel(0x3 << shift, reg + STMP_OFFSET_REG_CLR);
+		writel(muxsel << shift, reg + STMP_OFFSET_REG_SET);
+
+		debug("(val: 0x%x) pin %d, mux %d, ma: %d, vol: %d, pull: %d\n",
+		      val, pinid, muxsel, ma, vol, pull);
+
+		/* drive */
+		reg = iomux->base + 0x300;
+		reg += bank * 0x40 + pin / 8 * 0x10;
+
+		/* mA */
+		if (ma_present) {
+			shift = pin % 8 * 4;
+			writel(0x3 << shift, reg + STMP_OFFSET_REG_CLR);
+			writel(ma << shift, reg + STMP_OFFSET_REG_SET);
+		}
+
+		/* vol */
+		if (vol_present) {
+			shift = pin % 8 * 4 + 2;
+			if (vol)
+				writel(1 << shift, reg + STMP_OFFSET_REG_SET);
+			else
+				writel(1 << shift, reg + STMP_OFFSET_REG_CLR);
+		}
+
+		/* pull */
+		if (pull_present) {
+			reg = iomux->base + 0x600;
+			reg += bank * 0x10;
+			shift = pin;
+			if (pull)
+				writel(1 << shift, reg + STMP_OFFSET_REG_SET);
+			else
+				writel(1 << shift, reg + STMP_OFFSET_REG_CLR);
+		}
+	}
+
+	return 0;
+}
+
+static struct pinctrl_ops mxs_pinctrl_ops = {
+	.set_state = mxs_pinctrl_set_state,
+};
+
+static int mxs_pinctrl_probe(struct udevice *dev)
+{
+	struct mxs_pinctrl_priv *iomux = dev_get_priv(dev);
+	iomux->base = dev_read_addr_ptr(dev);
+
+	return 0;
+}
+
+static const struct udevice_id mxs_pinctrl_match[] = {
+	{ .compatible = "fsl,imx28-pinctrl" },
+	{ /* sentinel */ }
+};
+
+U_BOOT_DRIVER(mxs_pinctrl) = {
+	.name = "mxs-pinctrl",
+	.id = UCLASS_PINCTRL,
+	.of_match = of_match_ptr(mxs_pinctrl_match),
+	.probe = mxs_pinctrl_probe,
+#if !CONFIG_IS_ENABLED(OF_PLATDATA)
+	.bind		= dm_scan_fdt_dev,
+#endif
+	.priv_auto_alloc_size = sizeof(struct mxs_pinctrl_priv),
+	.ops = &mxs_pinctrl_ops,
+};
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 5/5] ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI conversion)
  2019-06-15 22:34 [U-Boot] [PATCH v3 0/5] DM: Convert i.MX28 gpio, pinmux, spi and eth drivers to DM/DTS Lukasz Majewski
                   ` (3 preceding siblings ...)
  2019-06-15 22:34 ` [U-Boot] [PATCH v3 4/5] ARM: imx: pinctrl: Add support for i.MX28 mxs pinctrl driver Lukasz Majewski
@ 2019-06-15 22:34 ` Lukasz Majewski
  2019-06-15 23:02   ` Marek Vasut
  4 siblings, 1 reply; 32+ messages in thread
From: Lukasz Majewski @ 2019-06-15 22:34 UTC (permalink / raw)
  To: u-boot

This commit converts mxs_spi driver to support DM/DTS.

Signed-off-by: Lukasz Majewski <lukma@denx.de>

---

Changes in v3:
- Set more apropriate tags

Changes in v2:
- New patch (conversion of mxs_spi.c to DM_SPI)

 drivers/spi/mxs_spi.c | 393 +++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 310 insertions(+), 83 deletions(-)

diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
index 5065e407f8..6ae76309d7 100644
--- a/drivers/spi/mxs_spi.c
+++ b/drivers/spi/mxs_spi.c
@@ -2,6 +2,9 @@
 /*
  * Freescale i.MX28 SPI driver
  *
+ * Copyright (C) 2019 DENX Software Engineering
+ * Lukasz Majewski, DENX Software Engineering, lukma at denx.de
+ *
  * Copyright (C) 2011 Marek Vasut <marek.vasut@gmail.com>
  * on behalf of DENX Software Engineering GmbH
  *
@@ -27,6 +30,19 @@
 
 #define MXSSSP_SMALL_TRANSFER	512
 
+static void mxs_spi_start_xfer(struct mxs_ssp_regs *ssp_regs)
+{
+	writel(SSP_CTRL0_LOCK_CS, &ssp_regs->hw_ssp_ctrl0_set);
+	writel(SSP_CTRL0_IGNORE_CRC, &ssp_regs->hw_ssp_ctrl0_clr);
+}
+
+static void mxs_spi_end_xfer(struct mxs_ssp_regs *ssp_regs)
+{
+	writel(SSP_CTRL0_LOCK_CS, &ssp_regs->hw_ssp_ctrl0_clr);
+	writel(SSP_CTRL0_IGNORE_CRC, &ssp_regs->hw_ssp_ctrl0_set);
+}
+
+#if !CONFIG_IS_ENABLED(DM_SPI)
 struct mxs_spi_slave {
 	struct spi_slave	slave;
 	uint32_t		max_khz;
@@ -38,94 +54,38 @@ static inline struct mxs_spi_slave *to_mxs_slave(struct spi_slave *slave)
 {
 	return container_of(slave, struct mxs_spi_slave, slave);
 }
+#else
+#include <dm.h>
+#include <errno.h>
+struct mxs_spi_platdata {
+	s32 frequency;		/* Default clock frequency, -1 for none */
+	fdt_addr_t base;        /* SPI IP block base address */
+	int num_cs;             /* Number of CSes supported */
+	int dma_id;             /* ID of the DMA channel */
+	int clk_id;             /* ID of the SSP clock */
+};
 
-int spi_cs_is_valid(unsigned int bus, unsigned int cs)
-{
-	/* MXS SPI: 4 ports and 3 chip selects maximum */
-	if (!mxs_ssp_bus_id_valid(bus) || cs > 2)
-		return 0;
-	else
-		return 1;
-}
-
-struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
-				  unsigned int max_hz, unsigned int mode)
-{
-	struct mxs_spi_slave *mxs_slave;
-
-	if (!spi_cs_is_valid(bus, cs)) {
-		printf("mxs_spi: invalid bus %d / chip select %d\n", bus, cs);
-		return NULL;
-	}
-
-	mxs_slave = spi_alloc_slave(struct mxs_spi_slave, bus, cs);
-	if (!mxs_slave)
-		return NULL;
-
-	if (mxs_dma_init_channel(MXS_DMA_CHANNEL_AHB_APBH_SSP0 + bus))
-		goto err_init;
-
-	mxs_slave->max_khz = max_hz / 1000;
-	mxs_slave->mode = mode;
-	mxs_slave->regs = mxs_ssp_regs_by_bus(bus);
-
-	return &mxs_slave->slave;
-
-err_init:
-	free(mxs_slave);
-	return NULL;
-}
-
-void spi_free_slave(struct spi_slave *slave)
-{
-	struct mxs_spi_slave *mxs_slave = to_mxs_slave(slave);
-	free(mxs_slave);
-}
-
-int spi_claim_bus(struct spi_slave *slave)
-{
-	struct mxs_spi_slave *mxs_slave = to_mxs_slave(slave);
-	struct mxs_ssp_regs *ssp_regs = mxs_slave->regs;
-	uint32_t reg = 0;
-
-	mxs_reset_block(&ssp_regs->hw_ssp_ctrl0_reg);
-
-	writel((slave->cs << MXS_SSP_CHIPSELECT_SHIFT) |
-	       SSP_CTRL0_BUS_WIDTH_ONE_BIT,
-	       &ssp_regs->hw_ssp_ctrl0);
-
-	reg = SSP_CTRL1_SSP_MODE_SPI | SSP_CTRL1_WORD_LENGTH_EIGHT_BITS;
-	reg |= (mxs_slave->mode & SPI_CPOL) ? SSP_CTRL1_POLARITY : 0;
-	reg |= (mxs_slave->mode & SPI_CPHA) ? SSP_CTRL1_PHASE : 0;
-	writel(reg, &ssp_regs->hw_ssp_ctrl1);
-
-	writel(0, &ssp_regs->hw_ssp_cmd0);
-
-	mxs_set_ssp_busclock(slave->bus, mxs_slave->max_khz);
-
-	return 0;
-}
-
-void spi_release_bus(struct spi_slave *slave)
-{
-}
-
-static void mxs_spi_start_xfer(struct mxs_ssp_regs *ssp_regs)
-{
-	writel(SSP_CTRL0_LOCK_CS, &ssp_regs->hw_ssp_ctrl0_set);
-	writel(SSP_CTRL0_IGNORE_CRC, &ssp_regs->hw_ssp_ctrl0_clr);
-}
-
-static void mxs_spi_end_xfer(struct mxs_ssp_regs *ssp_regs)
-{
-	writel(SSP_CTRL0_LOCK_CS, &ssp_regs->hw_ssp_ctrl0_clr);
-	writel(SSP_CTRL0_IGNORE_CRC, &ssp_regs->hw_ssp_ctrl0_set);
-}
+struct mxs_spi_priv {
+	struct mxs_ssp_regs *regs;
+	unsigned int dma_channel;
+	unsigned int max_freq;
+	unsigned int clk_id;
+	unsigned int mode;
+};
+#endif
 
+#if !CONFIG_IS_ENABLED(DM_SPI)
 static int mxs_spi_xfer_pio(struct mxs_spi_slave *slave,
 			char *data, int length, int write, unsigned long flags)
 {
 	struct mxs_ssp_regs *ssp_regs = slave->regs;
+#else
+static int mxs_spi_xfer_pio(struct mxs_spi_priv *priv,
+			    char *data, int length, int write,
+			    unsigned long flags)
+{
+	struct mxs_ssp_regs *ssp_regs = priv->regs;
+#endif
 
 	if (flags & SPI_XFER_BEGIN)
 		mxs_spi_start_xfer(ssp_regs);
@@ -181,12 +141,18 @@ static int mxs_spi_xfer_pio(struct mxs_spi_slave *slave,
 	return 0;
 }
 
+#if !CONFIG_IS_ENABLED(DM_SPI)
 static int mxs_spi_xfer_dma(struct mxs_spi_slave *slave,
 			char *data, int length, int write, unsigned long flags)
 {
+	struct mxs_ssp_regs *ssp_regs = slave->regs;
+#else
+static int mxs_spi_xfer_dma(struct mxs_spi_priv *priv,
+			char *data, int length, int write, unsigned long flags)
+{	struct mxs_ssp_regs *ssp_regs = priv->regs;
+#endif
 	const int xfer_max_sz = 0xff00;
 	const int desc_count = DIV_ROUND_UP(length, xfer_max_sz) + 1;
-	struct mxs_ssp_regs *ssp_regs = slave->regs;
 	struct mxs_dma_desc *dp;
 	uint32_t ctrl0;
 	uint32_t cache_data_count;
@@ -225,7 +191,11 @@ static int mxs_spi_xfer_dma(struct mxs_spi_slave *slave,
 	/* Invalidate the area, so no writeback into the RAM races with DMA */
 	invalidate_dcache_range(dstart, dstart + cache_data_count);
 
+#if !CONFIG_IS_ENABLED(DM_SPI)
 	dmach = MXS_DMA_CHANNEL_AHB_APBH_SSP0 + slave->slave.bus;
+#else
+	dmach = priv->dma_channel;
+#endif
 
 	dp = desc;
 	while (length) {
@@ -302,11 +272,20 @@ static int mxs_spi_xfer_dma(struct mxs_spi_slave *slave,
 	return ret;
 }
 
+#if !CONFIG_IS_ENABLED(DM_SPI)
 int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
 		const void *dout, void *din, unsigned long flags)
 {
 	struct mxs_spi_slave *mxs_slave = to_mxs_slave(slave);
 	struct mxs_ssp_regs *ssp_regs = mxs_slave->regs;
+#else
+int mxs_spi_xfer(struct udevice *dev, unsigned int bitlen,
+		const void *dout, void *din, unsigned long flags)
+{
+	struct udevice *bus = dev_get_parent(dev);
+	struct mxs_spi_priv *priv = dev_get_priv(bus);
+	struct mxs_ssp_regs *ssp_regs = priv->regs;
+#endif
 	int len = bitlen / 8;
 	char dummy;
 	int write = 0;
@@ -350,9 +329,257 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
 
 	if (!dma || (len < MXSSSP_SMALL_TRANSFER)) {
 		writel(SSP_CTRL1_DMA_ENABLE, &ssp_regs->hw_ssp_ctrl1_clr);
+#if !CONFIG_IS_ENABLED(DM_SPI)
 		return mxs_spi_xfer_pio(mxs_slave, data, len, write, flags);
+#else
+		return mxs_spi_xfer_pio(priv, data, len, write, flags);
+#endif
 	} else {
 		writel(SSP_CTRL1_DMA_ENABLE, &ssp_regs->hw_ssp_ctrl1_set);
+#if !CONFIG_IS_ENABLED(DM_SPI)
 		return mxs_spi_xfer_dma(mxs_slave, data, len, write, flags);
+#else
+		return mxs_spi_xfer_dma(priv, data, len, write, flags);
+#endif
 	}
 }
+
+#if !CONFIG_IS_ENABLED(DM_SPI)
+int spi_cs_is_valid(unsigned int bus, unsigned int cs)
+{
+	/* MXS SPI: 4 ports and 3 chip selects maximum */
+	if (!mxs_ssp_bus_id_valid(bus) || cs > 2)
+		return 0;
+	else
+		return 1;
+}
+
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
+				  unsigned int max_hz, unsigned int mode)
+{
+	struct mxs_spi_slave *mxs_slave;
+
+	if (!spi_cs_is_valid(bus, cs)) {
+		printf("mxs_spi: invalid bus %d / chip select %d\n", bus, cs);
+		return NULL;
+	}
+
+	mxs_slave = spi_alloc_slave(struct mxs_spi_slave, bus, cs);
+	if (!mxs_slave)
+		return NULL;
+
+	if (mxs_dma_init_channel(MXS_DMA_CHANNEL_AHB_APBH_SSP0 + bus))
+		goto err_init;
+
+	mxs_slave->max_khz = max_hz / 1000;
+	mxs_slave->mode = mode;
+	mxs_slave->regs = mxs_ssp_regs_by_bus(bus);
+
+	return &mxs_slave->slave;
+
+err_init:
+	free(mxs_slave);
+	return NULL;
+}
+
+void spi_free_slave(struct spi_slave *slave)
+{
+	struct mxs_spi_slave *mxs_slave = to_mxs_slave(slave);
+	free(mxs_slave);
+}
+
+int spi_claim_bus(struct spi_slave *slave)
+{
+	struct mxs_spi_slave *mxs_slave = to_mxs_slave(slave);
+	struct mxs_ssp_regs *ssp_regs = mxs_slave->regs;
+	uint32_t reg = 0;
+
+	mxs_reset_block(&ssp_regs->hw_ssp_ctrl0_reg);
+
+	writel((slave->cs << MXS_SSP_CHIPSELECT_SHIFT) |
+	       SSP_CTRL0_BUS_WIDTH_ONE_BIT,
+	       &ssp_regs->hw_ssp_ctrl0);
+
+	reg = SSP_CTRL1_SSP_MODE_SPI | SSP_CTRL1_WORD_LENGTH_EIGHT_BITS;
+	reg |= (mxs_slave->mode & SPI_CPOL) ? SSP_CTRL1_POLARITY : 0;
+	reg |= (mxs_slave->mode & SPI_CPHA) ? SSP_CTRL1_PHASE : 0;
+	writel(reg, &ssp_regs->hw_ssp_ctrl1);
+
+	writel(0, &ssp_regs->hw_ssp_cmd0);
+
+	mxs_set_ssp_busclock(slave->bus, mxs_slave->max_khz);
+
+	return 0;
+}
+
+void spi_release_bus(struct spi_slave *slave)
+{
+}
+
+#else /* CONFIG_DM_SPI */
+/* Base number of i.MX28 clk for ssp0 IP block */
+#define MXS_SSP_IMX28_CLKID_SSP0 46
+
+static int mxs_spi_probe(struct udevice *bus)
+{
+	struct mxs_spi_platdata *plat = dev_get_platdata(bus);
+	struct mxs_spi_priv *priv = dev_get_priv(bus);
+	int ret;
+
+	debug("%s: probe\n", __func__);
+	priv->regs = (struct mxs_ssp_regs *)plat->base;
+	priv->max_freq = plat->frequency;
+
+	priv->dma_channel = plat->dma_id;
+	priv->clk_id = plat->clk_id;
+
+	ret = mxs_dma_init_channel(priv->dma_channel);
+	if (ret) {
+		printf("%s: DMA init channel error %d\n", __func__, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int mxs_spi_claim_bus(struct udevice *dev)
+{
+	struct udevice *bus = dev_get_parent(dev);
+	struct mxs_spi_priv *priv = dev_get_priv(bus);
+	struct mxs_ssp_regs *ssp_regs = priv->regs;
+	int cs = spi_chip_select(dev);
+
+	/*
+	 * i.MX28 supports up to 3 CS (SSn0, SSn1, SSn2)
+	 * To set them it uses following tuple (WAIT_FOR_IRQ,WAIT_FOR_CMD),
+	 * where:
+	 *
+	 * WAIT_FOR_IRQ is bit 21 of HW_SSP_CTRL0
+	 * WAIT_FOR_CMD is bit 20 (#defined as MXS_SSP_CHIPSELECT_SHIFT here) of
+	 *                        HW_SSP_CTRL0
+	 * SSn0 b00
+	 * SSn1 b01
+	 * SSn2 b10 (which require setting WAIT_FOR_IRQ)
+	 *
+	 * However, for now i.MX28 SPI driver will support up till 2 CSes
+	 * (SSn0, and SSn1).
+	 */
+
+	/* Ungate SSP clock and set active CS */
+	clrsetbits_le32(&ssp_regs->hw_ssp_ctrl0,
+			(1 << MXS_SSP_CHIPSELECT_SHIFT) |
+			SSP_CTRL0_CLKGATE, (cs << MXS_SSP_CHIPSELECT_SHIFT));
+
+	return 0;
+}
+
+static int mxs_spi_release_bus(struct udevice *dev)
+{
+	struct udevice *bus = dev_get_parent(dev);
+	struct mxs_spi_priv *priv = dev_get_priv(bus);
+	struct mxs_ssp_regs *ssp_regs = priv->regs;
+
+	/* Gate SSP clock */
+	setbits_le32(&ssp_regs->hw_ssp_ctrl0, SSP_CTRL0_CLKGATE);
+
+	return 0;
+}
+
+static int mxs_spi_set_speed(struct udevice *bus, uint speed)
+{
+	struct mxs_spi_priv *priv = dev_get_priv(bus);
+	int clkid = priv->clk_id - MXS_SSP_IMX28_CLKID_SSP0;
+
+	if (speed > priv->max_freq)
+		speed = priv->max_freq;
+
+	debug("%s speed: %u [Hz] clkid: %d\n", __func__, speed, clkid);
+	mxs_set_ssp_busclock(clkid, speed / 1000);
+
+	return 0;
+}
+
+static int mxs_spi_set_mode(struct udevice *bus, uint mode)
+{
+	struct mxs_spi_priv *priv = dev_get_priv(bus);
+	struct mxs_ssp_regs *ssp_regs = priv->regs;
+	u32 reg;
+
+	priv->mode = mode;
+	debug("%s: mode 0x%x\n", __func__, mode);
+
+	reg = SSP_CTRL1_SSP_MODE_SPI | SSP_CTRL1_WORD_LENGTH_EIGHT_BITS;
+	reg |= (priv->mode & SPI_CPOL) ? SSP_CTRL1_POLARITY : 0;
+	reg |= (priv->mode & SPI_CPHA) ? SSP_CTRL1_PHASE : 0;
+	writel(reg, &ssp_regs->hw_ssp_ctrl1);
+
+	/* Single bit SPI support */
+	writel(SSP_CTRL0_BUS_WIDTH_ONE_BIT, &ssp_regs->hw_ssp_ctrl0);
+
+	return 0;
+}
+
+static const struct dm_spi_ops mxs_spi_ops = {
+	.claim_bus	= mxs_spi_claim_bus,
+	.release_bus    = mxs_spi_release_bus,
+	.xfer		= mxs_spi_xfer,
+	.set_speed	= mxs_spi_set_speed,
+	.set_mode	= mxs_spi_set_mode,
+	/*
+	 * cs_info is not needed, since we require all chip selects to be
+	 * in the device tree explicitly
+	 */
+};
+
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
+static int mxs_ofdata_to_platdata(struct udevice *bus)
+{
+	struct mxs_spi_platdata *plat = bus->platdata;
+	u32 prop[2];
+	int ret;
+
+	plat->base = dev_read_addr(bus);
+	plat->frequency =
+		dev_read_u32_default(bus, "spi-max-frequency", 40000000);
+	plat->num_cs = dev_read_u32_default(bus, "num-cs", 2);
+
+	ret = dev_read_u32_array(bus, "dmas", prop, ARRAY_SIZE(prop));
+	if (ret) {
+		printf("%s: Reading 'dmas' property failed!\n", __func__);
+		return ret;
+	}
+	plat->dma_id = prop[1];
+
+	ret = dev_read_u32_array(bus, "clocks", prop, ARRAY_SIZE(prop));
+	if (ret) {
+		printf("%s: Reading 'clocks' property failed!\n", __func__);
+		return ret;
+	}
+	plat->clk_id = prop[1];
+
+	debug("%s: base=0x%x, max-frequency=%d num-cs=%d dma_id=%d clk_id=%d\n",
+	      __func__, (uint)plat->base, plat->frequency, plat->num_cs,
+	      plat->dma_id, plat->clk_id);
+
+	return 0;
+}
+#endif
+
+static const struct udevice_id mxs_spi_ids[] = {
+	{ .compatible = "fsl,imx28-spi" },
+	{ }
+};
+
+U_BOOT_DRIVER(mxs_spi) = {
+	.name	= "mxs_spi",
+	.id	= UCLASS_SPI,
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
+	.of_match = mxs_spi_ids,
+	.ofdata_to_platdata = mxs_ofdata_to_platdata,
+#endif
+	.priv_auto_alloc_size = sizeof(struct mxs_spi_platdata),
+	.ops	= &mxs_spi_ops,
+	.priv_auto_alloc_size = sizeof(struct mxs_spi_priv),
+	.probe	= mxs_spi_probe,
+};
+#endif
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 1/5] ARM: dts: imx: Copy imx28 device tree related files from Linux kernel (v5.1.9)
  2019-06-15 22:34 ` [U-Boot] [PATCH v3 1/5] ARM: dts: imx: Copy imx28 device tree related files from Linux kernel (v5.1.9) Lukasz Majewski
@ 2019-06-15 22:43   ` Marek Vasut
  2019-06-17  6:57     ` Lukasz Majewski
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2019-06-15 22:43 UTC (permalink / raw)
  To: u-boot

On 6/16/19 12:34 AM, Lukasz Majewski wrote:
> This commit copies from the Linux kernel (tag v5.1.9) i.MX28 related
> device tree files.

Please use commit hash (first 12 characters of the SHA-1 ID, see [1]),
that's a unique identifier. Tag name is not.

[1] https://www.kernel.org/doc/html/v4.12/process/submitting-patches.html

> Signed-off-by: Lukasz Majewski <lukma@denx.de>

[...]

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 2/5] ARM: imx: net: Enable support for i.MX28 DM_ETH in the fec_mxc.c driver
  2019-06-15 22:34 ` [U-Boot] [PATCH v3 2/5] ARM: imx: net: Enable support for i.MX28 DM_ETH in the fec_mxc.c driver Lukasz Majewski
@ 2019-06-15 22:43   ` Marek Vasut
  2019-06-17  6:57     ` Lukasz Majewski
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2019-06-15 22:43 UTC (permalink / raw)
  To: u-boot

On 6/16/19 12:34 AM, Lukasz Majewski wrote:
> The fec_mxc.c driver can be reused by i.MX28 when DM_ETH is enabled.
> One only needs to add proper compatible and dependency on FEC_MXC in the
> Kconfig.
> 
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> ---
> 
> Changes in v3:
> - Set more apropriate tags
> 
> Changes in v2: None
> 
>  drivers/net/Kconfig   | 2 +-
>  drivers/net/fec_mxc.c | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index e6a4fdf30e..04f886fe86 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -190,7 +190,7 @@ config FEC_MXC_MDIO_BASE
>  
>  config FEC_MXC
>  	bool "FEC Ethernet controller"
> -	depends on MX5 || MX6 || MX7 || IMX8 || VF610
> +	depends on MX5 || MX6 || MX7 || IMX8 || VF610 || MX28

Keep the list sorted -- IMX28 goes at the beginning

>  	help
>  	  This driver supports the 10/100 Fast Ethernet controller for
>  	  NXP i.MX processors.
> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
> index d7c080943a..4042dcf9ca 100644
> --- a/drivers/net/fec_mxc.c
> +++ b/drivers/net/fec_mxc.c
> @@ -1492,6 +1492,7 @@ static const struct udevice_id fecmxc_ids[] = {
>  	{ .compatible = "fsl,imx53-fec" },
>  	{ .compatible = "fsl,imx7d-fec" },
>  	{ .compatible = "fsl,mvf600-fec" },
> +	{ .compatible = "fsl,imx28-fec" },

Keep this list sorted too.

>  	{ }
>  };
>  
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 3/5] ARM: dm: mxs: gpio: Add support for DM/DTS in the mxs_gpio.c driver (DM_GPIO)
  2019-06-15 22:34 ` [U-Boot] [PATCH v3 3/5] ARM: dm: mxs: gpio: Add support for DM/DTS in the mxs_gpio.c driver (DM_GPIO) Lukasz Majewski
@ 2019-06-15 22:53   ` Marek Vasut
  2019-06-17  8:37     ` Lukasz Majewski
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2019-06-15 22:53 UTC (permalink / raw)
  To: u-boot

On 6/16/19 12:34 AM, Lukasz Majewski wrote:
> This commit

This is not a commit, it's a change. It only becomes a commit when applied.

> adds support for DM in the mxs_gpio.c driver when DM_GPIO
> is enabled in Kconfig.

But this also adds support for DT probing, which is orthogonal to DM
support, yet it's not documented in the commit message.

> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> 
> ---
> 
> Changes in v3:
> - Set more apropriate tags
> 
> Changes in v2:
> - Use #if !CONFIG_IS_ENABLED(DM_GPIO) instead of plain #ifdef CONFIG_DM_GPIO
> 
>  arch/arm/include/asm/arch-mxs/gpio.h |  28 +++++++
>  drivers/gpio/mxs_gpio.c              | 149 +++++++++++++++++++++++++++++++++++
>  2 files changed, 177 insertions(+)
> 
> diff --git a/arch/arm/include/asm/arch-mxs/gpio.h b/arch/arm/include/asm/arch-mxs/gpio.h
> index 34fa421945..0c152e25e2 100644
> --- a/arch/arm/include/asm/arch-mxs/gpio.h
> +++ b/arch/arm/include/asm/arch-mxs/gpio.h
> @@ -15,4 +15,32 @@ void mxs_gpio_init(void);
>  inline void mxs_gpio_init(void) {}
>  #endif
>  
> +#if defined(CONFIG_MX28) && CONFIG_IS_ENABLED(DM_GPIO)
> +/*
> + * According to i.MX28 Reference Manual:
> + * 'i.MX28 Applications Processor Reference Manual, Rev. 1, 2010'
> + * The i.MX28 has following number of GPIOs available:
> + * Bank 0: 0-28 -> 29 PINS
> + * Bank 1: 0-31 -> 32 PINS
> + * Bank 2: 0-27 -> 28 PINS
> + * Bank 3: 0-30 -> 31 PINS
> + * Bank 4: 0-20 -> 21 PINS
> + */
> +#define IMX28_GPIO_BANK0_PIN_NR 29
> +#define IMX28_GPIO_BANK1_PIN_NR 32
> +#define IMX28_GPIO_BANK2_PIN_NR 28
> +#define IMX28_GPIO_BANK3_PIN_NR 31
> +#define IMX28_GPIO_BANK4_PIN_NR 21
> +#define IMX28_GPIO_BANK_NR 5

Please make a complete conversion, not partial one.
You want to use gpio-ranges in DT and then parse the size of each GPIO
bank from those gpio-ranges , not hard-code it into the driver.

> +struct mxs_gpio_platdata {
> +	u32 gpio_nr_of_bank_pins[IMX28_GPIO_BANK_NR];
> +	u32 gpio_base_nr[IMX28_GPIO_BANK_NR];
> +	u32 ngpio;
> +};
> +
> +extern const struct mxs_gpio_platdata mxs_gpio_def;
> +#define IMX_GPIO_NR(port, index) (mxs_gpio_def.gpio_base_nr[(port)] + (index))

So this should be completely unnecessary when using the DM GPIO, this
was only needed for non-DM-GPIO .

> +#endif
> +
>  #endif	/* __MX28_GPIO_H__ */
> diff --git a/drivers/gpio/mxs_gpio.c b/drivers/gpio/mxs_gpio.c
> index c2c8a25886..f386235ff1 100644
> --- a/drivers/gpio/mxs_gpio.c
> +++ b/drivers/gpio/mxs_gpio.c
> @@ -51,6 +51,7 @@ void mxs_gpio_init(void)
>  	}
>  }
>  
> +#if !CONFIG_IS_ENABLED(DM_GPIO)
>  int gpio_get_value(unsigned gpio)
>  {
>  	uint32_t bank = PAD_BANK(gpio);
> @@ -127,3 +128,151 @@ int name_to_gpio(const char *name)
>  
>  	return (bank << MXS_PAD_BANK_SHIFT) | (pin << MXS_PAD_PIN_SHIFT);
>  }
> +#else /* CONFIG_DM_GPIO */
> +#include <dm.h>
> +#include <asm/gpio.h>
> +#include <asm/arch/gpio.h>
> +#ifdef CONFIG_MX28
> +const struct mxs_gpio_platdata mxs_gpio_def = {
> +	.gpio_nr_of_bank_pins[0] = IMX28_GPIO_BANK0_PIN_NR,
> +	.gpio_nr_of_bank_pins[1] = IMX28_GPIO_BANK1_PIN_NR,
> +	.gpio_nr_of_bank_pins[2] = IMX28_GPIO_BANK2_PIN_NR,
> +	.gpio_nr_of_bank_pins[3] = IMX28_GPIO_BANK3_PIN_NR,
> +	.gpio_nr_of_bank_pins[4] = IMX28_GPIO_BANK4_PIN_NR,
> +	.gpio_base_nr[0] = 0,
> +	.gpio_base_nr[1] = IMX28_GPIO_BANK0_PIN_NR,
> +	.gpio_base_nr[2] = (IMX28_GPIO_BANK0_PIN_NR + \
> +			   IMX28_GPIO_BANK1_PIN_NR),
> +	.gpio_base_nr[3] = (IMX28_GPIO_BANK0_PIN_NR + \
> +			   IMX28_GPIO_BANK1_PIN_NR + \
> +			   IMX28_GPIO_BANK2_PIN_NR),
> +	.gpio_base_nr[4] = (IMX28_GPIO_BANK0_PIN_NR + \
> +			   IMX28_GPIO_BANK1_PIN_NR + \
> +			   IMX28_GPIO_BANK2_PIN_NR + \
> +			   IMX28_GPIO_BANK3_PIN_NR),
> +	.ngpio = (IMX28_GPIO_BANK0_PIN_NR + \
> +		  IMX28_GPIO_BANK1_PIN_NR + \
> +		  IMX28_GPIO_BANK2_PIN_NR + \
> +		  IMX28_GPIO_BANK3_PIN_NR + \
> +		  IMX28_GPIO_BANK4_PIN_NR),
> +};

So please use gpio-ranges in DT.

> +#else
> +#error "Only i.MX28 supported with DM_GPIO"
> +#endif
> +
> +struct mxs_gpio_priv {
> +	unsigned int bank;
> +};
> +
> +static int mxs_gpio_get_value(struct udevice *dev, unsigned offset)
> +{
> +	struct mxs_gpio_priv *priv = dev_get_priv(dev);
> +	struct mxs_register_32 *reg =
> +		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
> +					   PINCTRL_DIN(priv->bank));
> +
> +	return (readl(&reg->reg) >> offset) & 1;
> +}
> +
> +static int mxs_gpio_set_value(struct udevice *dev, unsigned offset,
> +			      int value)
> +{
> +	struct mxs_gpio_priv *priv = dev_get_priv(dev);
> +	struct mxs_register_32 *reg =
> +		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
> +					   PINCTRL_DOUT(priv->bank));
> +	if (value)
> +		writel(1 << offset, &reg->reg_set);

BIT(), fix globally.

> +	else
> +		writel(1 << offset, &reg->reg_clr);
> +
> +	return 0;
> +}
> +
> +static int mxs_gpio_direction_input(struct udevice *dev, unsigned offset)
> +{
> +	struct mxs_gpio_priv *priv = dev_get_priv(dev);
> +	struct mxs_register_32 *reg =
> +		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
> +					   PINCTRL_DOE(priv->bank));
> +
> +	writel(1 << offset, &reg->reg_clr);
> +
> +	return 0;
> +}
> +
> +static int mxs_gpio_direction_output(struct udevice *dev, unsigned offset,
> +				     int value)
> +{
> +	struct mxs_gpio_priv *priv = dev_get_priv(dev);
> +	struct mxs_register_32 *reg =
> +		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
> +					   PINCTRL_DOE(priv->bank));
> +
> +	mxs_gpio_set_value(dev, offset, value);
> +
> +	writel(1 << offset, &reg->reg_set);
> +
> +	return 0;
> +}
> +
> +static int mxs_gpio_get_function(struct udevice *dev, unsigned offset)
> +{
> +	struct mxs_gpio_priv *priv = dev_get_priv(dev);
> +	struct mxs_register_32 *reg =
> +		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
> +					   PINCTRL_DOE(priv->bank));
> +	bool is_output = !!(readl(&reg->reg) >> offset);
> +
> +	return is_output ? GPIOF_OUTPUT : GPIOF_INPUT;
> +}
> +
> +static const struct dm_gpio_ops gpio_mxs_ops = {
> +	.direction_input	= mxs_gpio_direction_input,
> +	.direction_output	= mxs_gpio_direction_output,
> +	.get_value		= mxs_gpio_get_value,
> +	.set_value		= mxs_gpio_set_value,
> +	.get_function		= mxs_gpio_get_function,
> +};
> +
> +static int mxs_gpio_probe(struct udevice *dev)
> +{
> +	struct mxs_gpio_priv *priv = dev_get_priv(dev);
> +	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> +	struct mxs_gpio_platdata *pdata =
> +		(struct mxs_gpio_platdata *)dev_get_driver_data(dev);
> +	char name[18], *str;
> +	int ret;
> +
> +	ret = dev_read_u32(dev, "reg", &priv->bank);

devfdt_get_addr()

> +	if (ret) {
> +		printf("%s: No 'reg' property defined!\n", __func__);
> +		return ret;
> +	}
> +
> +	sprintf(name, "GPIO%d_", priv->bank);

Name cannot conceivably have more than 16 characters, why is the array
18 characters large ? Also, snprintf() would likely be better here.

> +	str = strdup(name);
> +	if (!str)
> +		return -ENOMEM;
> +
> +	uc_priv->bank_name = str;
> +	uc_priv->gpio_count = pdata->gpio_nr_of_bank_pins[priv->bank];
> +
> +	return 0;
> +}
> +
> +static const struct udevice_id mxs_gpio_ids[] = {
> +	{ .compatible = "fsl,imx28-gpio", .data = (ulong)&mxs_gpio_def },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(gpio_mxs) = {
> +	.name	= "gpio_mxs",
> +	.id	= UCLASS_GPIO,
> +	.ops	= &gpio_mxs_ops,
> +	.probe	= mxs_gpio_probe,
> +	.priv_auto_alloc_size = sizeof(struct mxs_gpio_priv),
> +	.of_match = mxs_gpio_ids,
> +};
> +#endif /* CONFIG_DM_GPIO */
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 4/5] ARM: imx: pinctrl: Add support for i.MX28 mxs pinctrl driver
  2019-06-15 22:34 ` [U-Boot] [PATCH v3 4/5] ARM: imx: pinctrl: Add support for i.MX28 mxs pinctrl driver Lukasz Majewski
@ 2019-06-15 23:00   ` Marek Vasut
  2019-06-17  7:43     ` Lukasz Majewski
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2019-06-15 23:00 UTC (permalink / raw)
  To: u-boot

On 6/16/19 12:34 AM, Lukasz Majewski wrote:
> The code responsible for setting proper values in the MUX registers
> (in the mxs_pinctrl_set_state()) has been ported from Barebox project
> (branch: master, SHA1: eb3b0f7414cd8102844dd16b1c789e445e8947f8,
> file: drivers/pinctrl/pinctrl-mxs.c).

The format of a commit, when referenced, is documented here:
https://www.kernel.org/doc/html/v4.12/process/submitting-patches.html
e.g. e21d2170f366 ("video: remove unnecessary platform_set_drvdata()")

Although. maybe you should port this from Linux instead ?

> As the pinctrl node in the imx28.dtsi file has gpio pins nodes as subnodes,
> it was necessary to use 'dm_scan_fdt_dev()' (as a .bind method) to also
> make them 'visible' by the DM's "gpio_mxs" driver.

Look at drivers/pinctrl/renesas/pfc-r7s72100.c r7s72100_pfc_probe() , I
think that one deals with the exact same problem .

> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> 
> ---
> 
> Changes in v3:
> - Set more apropriate tags
> 
> Changes in v2: None
> 
>  drivers/pinctrl/nxp/Kconfig       |  10 +++
>  drivers/pinctrl/nxp/Makefile      |   1 +
>  drivers/pinctrl/nxp/pinctrl-mxs.c | 164 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 175 insertions(+)
>  create mode 100644 drivers/pinctrl/nxp/pinctrl-mxs.c
> 
> diff --git a/drivers/pinctrl/nxp/Kconfig b/drivers/pinctrl/nxp/Kconfig
> index 61f93be42d..f2e67ca231 100644
> --- a/drivers/pinctrl/nxp/Kconfig
> +++ b/drivers/pinctrl/nxp/Kconfig
> @@ -89,6 +89,16 @@ config PINCTRL_IMX8M
>  	  only parses the 'fsl,pins' property and configure related
>  	  registers.
>  
> +config PINCTRL_MXS
> +	bool "NXP MXS pinctrl driver"
> +	depends on ARCH_MX28 && PINCTRL_FULL
> +	help
> +	  Say Y here to enable the i.MX mxs pinctrl driver
> +
> +	  This option provides a simple pinctrl driver for i.MX mxs SoC
> +	  familiy, e.g. i.MX28. This feature depends on device tree
> +	  configuration.
> +
>  config PINCTRL_VYBRID
>  	bool "Vybrid (vf610) pinctrl driver"
>  	depends on ARCH_VF610 && PINCTRL_FULL
> diff --git a/drivers/pinctrl/nxp/Makefile b/drivers/pinctrl/nxp/Makefile
> index b340d9448a..b86448aac9 100644
> --- a/drivers/pinctrl/nxp/Makefile
> +++ b/drivers/pinctrl/nxp/Makefile
> @@ -6,4 +6,5 @@ obj-$(CONFIG_PINCTRL_IMX7ULP)		+= pinctrl-imx7ulp.o
>  obj-$(CONFIG_PINCTRL_IMX_SCU)		+= pinctrl-scu.o
>  obj-$(CONFIG_PINCTRL_IMX8)		+= pinctrl-imx8.o
>  obj-$(CONFIG_PINCTRL_IMX8M)		+= pinctrl-imx8m.o
> +obj-$(CONFIG_PINCTRL_MXS)		+= pinctrl-mxs.o
>  obj-$(CONFIG_PINCTRL_VYBRID)		+= pinctrl-vf610.o
> diff --git a/drivers/pinctrl/nxp/pinctrl-mxs.c b/drivers/pinctrl/nxp/pinctrl-mxs.c
> new file mode 100644
> index 0000000000..42b1b7998b
> --- /dev/null
> +++ b/drivers/pinctrl/nxp/pinctrl-mxs.c
> @@ -0,0 +1,164 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 DENX Software Engineering
> + * Lukasz Majewski, DENX Software Engineering, lukma at denx.de
> + *
> + * This work is based on drivers/pinctrl/pinctrl-mxs.c
> + * SHA1: eb3b0f7414cd8102844dd16b1c789e445e8947f8
> + * from Barebox project.
> + *
> + * Author of original Barebox pinctrl-mxs.c:
> + * Copyright (c) 2015 Sascha Hauer <s.hauer@pengutronix.de>
> + */
> +
> +#include <common.h>
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <dm.h>
> +#include <dm/pinctrl.h>
> +#include <dm/read.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define PINID(bank, pin)	((bank) * 32 + (pin))
> +#define MUXID_TO_PINID(m)	PINID((m) >> 12 & 0xf, (m) >> 4 & 0xff)
> +#define MUXID_TO_MUXSEL(m)	((m) & 0xf)
> +#define PINID_TO_BANK(p)	((p) >> 5)
> +#define PINID_TO_PIN(p)	((p) % 32)
> +
> +#define STMP_OFFSET_REG_SET     0x4
> +#define STMP_OFFSET_REG_CLR     0x8
> +#define STMP_OFFSET_REG_TOG     0xc
> +
> +struct mxs_pinctrl_priv {
> +	void __iomem *base;
> +};
> +
> +static int mxs_pinctrl_set_state(struct udevice *dev, struct udevice *config)
> +{
> +	int ma_present = 0, vol_present = 0, pull_present = 0;
> +	struct mxs_pinctrl_priv *iomux = dev_get_priv(dev);
> +	u32 *pin_data, val, ma, vol, pull;
> +	int npins, size, i, ret;
> +
> +	debug("\n%s: set state: %s\n", __func__, config->name);
> +
> +	size = dev_read_size(config, "fsl,pinmux-ids");
> +	if (size < 0)
> +		return size;
> +
> +	if (!size || size % 4) {
> +		dev_err(dev, "Invalid fsl,pinmux-ids property in %s\n",
> +			config->name);
> +		return -EINVAL;
> +	}
> +
> +	npins = size / 4;
> +
> +	pin_data = devm_kzalloc(dev, size, 0);

Is this ever free'd() ?

[...]

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 5/5] ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI conversion)
  2019-06-15 22:34 ` [U-Boot] [PATCH v3 5/5] ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI conversion) Lukasz Majewski
@ 2019-06-15 23:02   ` Marek Vasut
  2019-06-17  6:49     ` Lukasz Majewski
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2019-06-15 23:02 UTC (permalink / raw)
  To: u-boot

On 6/16/19 12:34 AM, Lukasz Majewski wrote:
> This commit converts mxs_spi driver to support DM/DTS.
> 
> Signed-off-by: Lukasz Majewski <lukma@denx.de>

Is the non-DM part needed for anything ? I recall the SPL jumps back to
BootROM when loading the U-Boot proper. So if not, drop it.

Also, please don't do partial conversion for iMX28 only, do the iMX23
part as well, it cannot be hard.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 5/5] ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI conversion)
  2019-06-15 23:02   ` Marek Vasut
@ 2019-06-17  6:49     ` Lukasz Majewski
  2019-06-17 10:06       ` Marek Vasut
  0 siblings, 1 reply; 32+ messages in thread
From: Lukasz Majewski @ 2019-06-17  6:49 UTC (permalink / raw)
  To: u-boot

Hi Marek,

> On 6/16/19 12:34 AM, Lukasz Majewski wrote:
> > This commit converts mxs_spi driver to support DM/DTS.
> > 
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>  
> 
> Is the non-DM part needed for anything ? 

Do you mean the non-DM part of the mxs_gpio driver? Yes, it is used by
not converted boards.

> I recall the SPL jumps back
> to BootROM when loading the U-Boot proper. So if not, drop it.
> 
> Also, please don't do partial conversion for iMX28 only, do the iMX23
> part as well, it cannot be hard.

Maybe it is not hard, but I cannot test it properly as I don't have
i.MX23 device. If you are offering your help with testing (i.e. you do
have the access to i.MX23 device and you will test those changes) I can
add support for it.

Otherwise, NO, I will not add ANY new untested code.

> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190617/54589b03/attachment.sig>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 1/5] ARM: dts: imx: Copy imx28 device tree related files from Linux kernel (v5.1.9)
  2019-06-15 22:43   ` Marek Vasut
@ 2019-06-17  6:57     ` Lukasz Majewski
  2019-06-17 10:13       ` Marek Vasut
  0 siblings, 1 reply; 32+ messages in thread
From: Lukasz Majewski @ 2019-06-17  6:57 UTC (permalink / raw)
  To: u-boot

Hi Marek,

> On 6/16/19 12:34 AM, Lukasz Majewski wrote:
> > This commit copies from the Linux kernel (tag v5.1.9) i.MX28 related
> > device tree files.  
> 
> Please use commit hash (first 12 characters of the SHA-1 ID, see [1]),
> that's a unique identifier. Tag name is not.

Could you show me any example of tag: v5.1.9 in Linux kernel being not
unique (or any other tag) ? 

Ok, I will add 12 digit SHA1 ID number.

> 
> [1]
> https://www.kernel.org/doc/html/v4.12/process/submitting-patches.html
> 
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>  
> 
> [...]
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190617/309458ba/attachment.sig>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 2/5] ARM: imx: net: Enable support for i.MX28 DM_ETH in the fec_mxc.c driver
  2019-06-15 22:43   ` Marek Vasut
@ 2019-06-17  6:57     ` Lukasz Majewski
  0 siblings, 0 replies; 32+ messages in thread
From: Lukasz Majewski @ 2019-06-17  6:57 UTC (permalink / raw)
  To: u-boot

Hi Marek,

> On 6/16/19 12:34 AM, Lukasz Majewski wrote:
> > The fec_mxc.c driver can be reused by i.MX28 when DM_ETH is enabled.
> > One only needs to add proper compatible and dependency on FEC_MXC
> > in the Kconfig.
> > 
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > ---
> > 
> > Changes in v3:
> > - Set more apropriate tags
> > 
> > Changes in v2: None
> > 
> >  drivers/net/Kconfig   | 2 +-
> >  drivers/net/fec_mxc.c | 1 +
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> > index e6a4fdf30e..04f886fe86 100644
> > --- a/drivers/net/Kconfig
> > +++ b/drivers/net/Kconfig
> > @@ -190,7 +190,7 @@ config FEC_MXC_MDIO_BASE
> >  
> >  config FEC_MXC
> >  	bool "FEC Ethernet controller"
> > -	depends on MX5 || MX6 || MX7 || IMX8 || VF610
> > +	depends on MX5 || MX6 || MX7 || IMX8 || VF610 || MX28  
> 
> Keep the list sorted -- IMX28 goes at the beginning

Ok.

> 
> >  	help
> >  	  This driver supports the 10/100 Fast Ethernet controller
> > for NXP i.MX processors.
> > diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
> > index d7c080943a..4042dcf9ca 100644
> > --- a/drivers/net/fec_mxc.c
> > +++ b/drivers/net/fec_mxc.c
> > @@ -1492,6 +1492,7 @@ static const struct udevice_id fecmxc_ids[] =
> > { { .compatible = "fsl,imx53-fec" },
> >  	{ .compatible = "fsl,imx7d-fec" },
> >  	{ .compatible = "fsl,mvf600-fec" },
> > +	{ .compatible = "fsl,imx28-fec" },  
> 
> Keep this list sorted too.

Ok.

> 
> >  	{ }
> >  };
> >  
> >   
> 
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190617/0b3ccd89/attachment.sig>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 4/5] ARM: imx: pinctrl: Add support for i.MX28 mxs pinctrl driver
  2019-06-15 23:00   ` Marek Vasut
@ 2019-06-17  7:43     ` Lukasz Majewski
  2019-06-17 10:23       ` Marek Vasut
  0 siblings, 1 reply; 32+ messages in thread
From: Lukasz Majewski @ 2019-06-17  7:43 UTC (permalink / raw)
  To: u-boot

Hi Marek,

> On 6/16/19 12:34 AM, Lukasz Majewski wrote:
> > The code responsible for setting proper values in the MUX registers
> > (in the mxs_pinctrl_set_state()) has been ported from Barebox
> > project (branch: master, SHA1:
> > eb3b0f7414cd8102844dd16b1c789e445e8947f8, file:
> > drivers/pinctrl/pinctrl-mxs.c).  
> 
> The format of a commit, when referenced, is documented here:
> https://www.kernel.org/doc/html/v4.12/process/submitting-patches.html
> e.g. e21d2170f366 ("video: remove unnecessary platform_set_drvdata()")
> 
> Although. maybe you should port this from Linux instead ?

The code from linux is a bit more convoluted and verbose. I may re-look
into the kernel version.

> 
> > As the pinctrl node in the imx28.dtsi file has gpio pins nodes as
> > subnodes, it was necessary to use 'dm_scan_fdt_dev()' (as a .bind
> > method) to also make them 'visible' by the DM's "gpio_mxs" driver.  
> 
> Look at drivers/pinctrl/renesas/pfc-r7s72100.c r7s72100_pfc_probe() ,
> I think that one deals with the exact same problem .

It looks like this is the same problem.

However, the dm_scan_fdt_dev() is exactly for that, when combined
with .bind method from DM.

> 
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > 
> > ---
> > 
> > Changes in v3:
> > - Set more apropriate tags
> > 
> > Changes in v2: None
> > 
> >  drivers/pinctrl/nxp/Kconfig       |  10 +++
> >  drivers/pinctrl/nxp/Makefile      |   1 +
> >  drivers/pinctrl/nxp/pinctrl-mxs.c | 164
> > ++++++++++++++++++++++++++++++++++++++ 3 files changed, 175
> > insertions(+) create mode 100644 drivers/pinctrl/nxp/pinctrl-mxs.c
> > 
> > diff --git a/drivers/pinctrl/nxp/Kconfig
> > b/drivers/pinctrl/nxp/Kconfig index 61f93be42d..f2e67ca231 100644
> > --- a/drivers/pinctrl/nxp/Kconfig
> > +++ b/drivers/pinctrl/nxp/Kconfig
> > @@ -89,6 +89,16 @@ config PINCTRL_IMX8M
> >  	  only parses the 'fsl,pins' property and configure related
> >  	  registers.
> >  
> > +config PINCTRL_MXS
> > +	bool "NXP MXS pinctrl driver"
> > +	depends on ARCH_MX28 && PINCTRL_FULL
> > +	help
> > +	  Say Y here to enable the i.MX mxs pinctrl driver
> > +
> > +	  This option provides a simple pinctrl driver for i.MX
> > mxs SoC
> > +	  familiy, e.g. i.MX28. This feature depends on device tree
> > +	  configuration.
> > +
> >  config PINCTRL_VYBRID
> >  	bool "Vybrid (vf610) pinctrl driver"
> >  	depends on ARCH_VF610 && PINCTRL_FULL
> > diff --git a/drivers/pinctrl/nxp/Makefile
> > b/drivers/pinctrl/nxp/Makefile index b340d9448a..b86448aac9 100644
> > --- a/drivers/pinctrl/nxp/Makefile
> > +++ b/drivers/pinctrl/nxp/Makefile
> > @@ -6,4 +6,5 @@ obj-$(CONFIG_PINCTRL_IMX7ULP)		+=
> > pinctrl-imx7ulp.o obj-$(CONFIG_PINCTRL_IMX_SCU)		+=
> > pinctrl-scu.o obj-$(CONFIG_PINCTRL_IMX8)		+=
> > pinctrl-imx8.o obj-$(CONFIG_PINCTRL_IMX8M)		+=
> > pinctrl-imx8m.o +obj-$(CONFIG_PINCTRL_MXS)		+=
> > pinctrl-mxs.o obj-$(CONFIG_PINCTRL_VYBRID)		+=
> > pinctrl-vf610.o diff --git a/drivers/pinctrl/nxp/pinctrl-mxs.c
> > b/drivers/pinctrl/nxp/pinctrl-mxs.c new file mode 100644
> > index 0000000000..42b1b7998b
> > --- /dev/null
> > +++ b/drivers/pinctrl/nxp/pinctrl-mxs.c
> > @@ -0,0 +1,164 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2019 DENX Software Engineering
> > + * Lukasz Majewski, DENX Software Engineering, lukma at denx.de
> > + *
> > + * This work is based on drivers/pinctrl/pinctrl-mxs.c
> > + * SHA1: eb3b0f7414cd8102844dd16b1c789e445e8947f8
> > + * from Barebox project.
> > + *
> > + * Author of original Barebox pinctrl-mxs.c:
> > + * Copyright (c) 2015 Sascha Hauer <s.hauer@pengutronix.de>
> > + */
> > +
> > +#include <common.h>
> > +#include <linux/io.h>
> > +#include <linux/err.h>
> > +#include <dm.h>
> > +#include <dm/pinctrl.h>
> > +#include <dm/read.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +#define PINID(bank, pin)	((bank) * 32 + (pin))
> > +#define MUXID_TO_PINID(m)	PINID((m) >> 12 & 0xf, (m) >> 4 &
> > 0xff) +#define MUXID_TO_MUXSEL(m)	((m) & 0xf)
> > +#define PINID_TO_BANK(p)	((p) >> 5)
> > +#define PINID_TO_PIN(p)	((p) % 32)
> > +
> > +#define STMP_OFFSET_REG_SET     0x4
> > +#define STMP_OFFSET_REG_CLR     0x8
> > +#define STMP_OFFSET_REG_TOG     0xc
> > +
> > +struct mxs_pinctrl_priv {
> > +	void __iomem *base;
> > +};
> > +
> > +static int mxs_pinctrl_set_state(struct udevice *dev, struct
> > udevice *config) +{
> > +	int ma_present = 0, vol_present = 0, pull_present = 0;
> > +	struct mxs_pinctrl_priv *iomux = dev_get_priv(dev);
> > +	u32 *pin_data, val, ma, vol, pull;
> > +	int npins, size, i, ret;
> > +
> > +	debug("\n%s: set state: %s\n", __func__, config->name);
> > +
> > +	size = dev_read_size(config, "fsl,pinmux-ids");
> > +	if (size < 0)
> > +		return size;
> > +
> > +	if (!size || size % 4) {
> > +		dev_err(dev, "Invalid fsl,pinmux-ids property in
> > %s\n",
> > +			config->name);
> > +		return -EINVAL;
> > +	}
> > +
> > +	npins = size / 4;
> > +
> > +	pin_data = devm_kzalloc(dev, size, 0);  
> 
> Is this ever free'd() ?
> 

Ach... right the devm_kfree() shall be used on the exit from the
function.

> [...]
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190617/91817d89/attachment.sig>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 3/5] ARM: dm: mxs: gpio: Add support for DM/DTS in the mxs_gpio.c driver (DM_GPIO)
  2019-06-15 22:53   ` Marek Vasut
@ 2019-06-17  8:37     ` Lukasz Majewski
  2019-06-17 10:20       ` Marek Vasut
  0 siblings, 1 reply; 32+ messages in thread
From: Lukasz Majewski @ 2019-06-17  8:37 UTC (permalink / raw)
  To: u-boot

Hi Marek,

> On 6/16/19 12:34 AM, Lukasz Majewski wrote:
> > This commit  
> 
> This is not a commit, it's a change. It only becomes a commit when
> applied.
> 
> > adds support for DM in the mxs_gpio.c driver when DM_GPIO
> > is enabled in Kconfig.  
> 
> But this also adds support for DT probing, which is orthogonal to DM
> support, yet it's not documented in the commit message.

Ok. Will rewrite the commit message to reflect the changes in the
commit.

> 
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > 
> > ---
> > 
> > Changes in v3:
> > - Set more apropriate tags
> > 
> > Changes in v2:
> > - Use #if !CONFIG_IS_ENABLED(DM_GPIO) instead of plain #ifdef
> > CONFIG_DM_GPIO
> > 
> >  arch/arm/include/asm/arch-mxs/gpio.h |  28 +++++++
> >  drivers/gpio/mxs_gpio.c              | 149
> > +++++++++++++++++++++++++++++++++++ 2 files changed, 177
> > insertions(+)
> > 
> > diff --git a/arch/arm/include/asm/arch-mxs/gpio.h
> > b/arch/arm/include/asm/arch-mxs/gpio.h index 34fa421945..0c152e25e2
> > 100644 --- a/arch/arm/include/asm/arch-mxs/gpio.h
> > +++ b/arch/arm/include/asm/arch-mxs/gpio.h
> > @@ -15,4 +15,32 @@ void mxs_gpio_init(void);
> >  inline void mxs_gpio_init(void) {}
> >  #endif
> >  
> > +#if defined(CONFIG_MX28) && CONFIG_IS_ENABLED(DM_GPIO)
> > +/*
> > + * According to i.MX28 Reference Manual:
> > + * 'i.MX28 Applications Processor Reference Manual, Rev. 1, 2010'
> > + * The i.MX28 has following number of GPIOs available:
> > + * Bank 0: 0-28 -> 29 PINS
> > + * Bank 1: 0-31 -> 32 PINS
> > + * Bank 2: 0-27 -> 28 PINS
> > + * Bank 3: 0-30 -> 31 PINS
> > + * Bank 4: 0-20 -> 21 PINS
> > + */
> > +#define IMX28_GPIO_BANK0_PIN_NR 29
> > +#define IMX28_GPIO_BANK1_PIN_NR 32
> > +#define IMX28_GPIO_BANK2_PIN_NR 28
> > +#define IMX28_GPIO_BANK3_PIN_NR 31
> > +#define IMX28_GPIO_BANK4_PIN_NR 21
> > +#define IMX28_GPIO_BANK_NR 5  
> 
> Please make a complete conversion, not partial one.
> You want to use gpio-ranges in DT and then parse the size of each GPIO
> bank from those gpio-ranges , not hard-code it into the driver.

I would have used the gpio-ranges, but the original Linux code [1]
(imx28.dtsi) doesn't define them.

Also, the dts files which include [1] don't extend original gpio nodes
to have one.

As it is not available in the Linux kernel, I don't see any good reason
to add the gpio-ranges to U-Boot. The same approach, as presented here,
is used in zynq_gpio.c file (which also uses DM/DTS).

Adding per u-boot property - like 'u-boot,mx28-gpio-ranges' is also less
appealing than 24 lines of code, which can be then easily re-used with
OF_PLATDATA (without DM/DTS suport) in SPL (u-boot.sb to be precise).


> 
> > +struct mxs_gpio_platdata {
> > +	u32 gpio_nr_of_bank_pins[IMX28_GPIO_BANK_NR];
> > +	u32 gpio_base_nr[IMX28_GPIO_BANK_NR];
> > +	u32 ngpio;
> > +};
> > +
> > +extern const struct mxs_gpio_platdata mxs_gpio_def;
> > +#define IMX_GPIO_NR(port, index)
> > (mxs_gpio_def.gpio_base_nr[(port)] + (index))  
> 
> So this should be completely unnecessary when using the DM GPIO, this
> was only needed for non-DM-GPIO .

This is a compatibility layer - for some reason ALL iMX ports define it
- i.e. imx8, imx7 - which shall use DM/DTS for gpio from the outset.

With the in-board code the dm_gpio_* set of functions shall (and
will) be used (as done in opos6ul.c file).

> 
> > +#endif
> > +
> >  #endif	/* __MX28_GPIO_H__ */
> > diff --git a/drivers/gpio/mxs_gpio.c b/drivers/gpio/mxs_gpio.c
> > index c2c8a25886..f386235ff1 100644
> > --- a/drivers/gpio/mxs_gpio.c
> > +++ b/drivers/gpio/mxs_gpio.c
> > @@ -51,6 +51,7 @@ void mxs_gpio_init(void)
> >  	}
> >  }
> >  
> > +#if !CONFIG_IS_ENABLED(DM_GPIO)
> >  int gpio_get_value(unsigned gpio)
> >  {
> >  	uint32_t bank = PAD_BANK(gpio);
> > @@ -127,3 +128,151 @@ int name_to_gpio(const char *name)
> >  
> >  	return (bank << MXS_PAD_BANK_SHIFT) | (pin <<
> > MXS_PAD_PIN_SHIFT); }
> > +#else /* CONFIG_DM_GPIO */
> > +#include <dm.h>
> > +#include <asm/gpio.h>
> > +#include <asm/arch/gpio.h>
> > +#ifdef CONFIG_MX28
> > +const struct mxs_gpio_platdata mxs_gpio_def = {
> > +	.gpio_nr_of_bank_pins[0] = IMX28_GPIO_BANK0_PIN_NR,
> > +	.gpio_nr_of_bank_pins[1] = IMX28_GPIO_BANK1_PIN_NR,
> > +	.gpio_nr_of_bank_pins[2] = IMX28_GPIO_BANK2_PIN_NR,
> > +	.gpio_nr_of_bank_pins[3] = IMX28_GPIO_BANK3_PIN_NR,
> > +	.gpio_nr_of_bank_pins[4] = IMX28_GPIO_BANK4_PIN_NR,
> > +	.gpio_base_nr[0] = 0,
> > +	.gpio_base_nr[1] = IMX28_GPIO_BANK0_PIN_NR,
> > +	.gpio_base_nr[2] = (IMX28_GPIO_BANK0_PIN_NR + \
> > +			   IMX28_GPIO_BANK1_PIN_NR),
> > +	.gpio_base_nr[3] = (IMX28_GPIO_BANK0_PIN_NR + \
> > +			   IMX28_GPIO_BANK1_PIN_NR + \
> > +			   IMX28_GPIO_BANK2_PIN_NR),
> > +	.gpio_base_nr[4] = (IMX28_GPIO_BANK0_PIN_NR + \
> > +			   IMX28_GPIO_BANK1_PIN_NR + \
> > +			   IMX28_GPIO_BANK2_PIN_NR + \
> > +			   IMX28_GPIO_BANK3_PIN_NR),
> > +	.ngpio = (IMX28_GPIO_BANK0_PIN_NR + \
> > +		  IMX28_GPIO_BANK1_PIN_NR + \
> > +		  IMX28_GPIO_BANK2_PIN_NR + \
> > +		  IMX28_GPIO_BANK3_PIN_NR + \
> > +		  IMX28_GPIO_BANK4_PIN_NR),
> > +};  
> 
> So please use gpio-ranges in DT.

Please see the above comment regarding gpio-ranges.

> 
> > +#else
> > +#error "Only i.MX28 supported with DM_GPIO"
> > +#endif
> > +
> > +struct mxs_gpio_priv {
> > +	unsigned int bank;
> > +};
> > +
> > +static int mxs_gpio_get_value(struct udevice *dev, unsigned offset)
> > +{
> > +	struct mxs_gpio_priv *priv = dev_get_priv(dev);
> > +	struct mxs_register_32 *reg =
> > +		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
> > +
> > PINCTRL_DIN(priv->bank)); +
> > +	return (readl(&reg->reg) >> offset) & 1;
> > +}
> > +
> > +static int mxs_gpio_set_value(struct udevice *dev, unsigned offset,
> > +			      int value)
> > +{
> > +	struct mxs_gpio_priv *priv = dev_get_priv(dev);
> > +	struct mxs_register_32 *reg =
> > +		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
> > +
> > PINCTRL_DOUT(priv->bank));
> > +	if (value)
> > +		writel(1 << offset, &reg->reg_set);  
> 
> BIT(), fix globally.

I took it from the original implementation :-).

Ok. I will fix it and replace 1 << offset with BIT().

> 
> > +	else
> > +		writel(1 << offset, &reg->reg_clr);
> > +
> > +	return 0;
> > +}
> > +
> > +static int mxs_gpio_direction_input(struct udevice *dev, unsigned
> > offset) +{
> > +	struct mxs_gpio_priv *priv = dev_get_priv(dev);
> > +	struct mxs_register_32 *reg =
> > +		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
> > +
> > PINCTRL_DOE(priv->bank)); +
> > +	writel(1 << offset, &reg->reg_clr);
> > +
> > +	return 0;
> > +}
> > +
> > +static int mxs_gpio_direction_output(struct udevice *dev, unsigned
> > offset,
> > +				     int value)
> > +{
> > +	struct mxs_gpio_priv *priv = dev_get_priv(dev);
> > +	struct mxs_register_32 *reg =
> > +		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
> > +
> > PINCTRL_DOE(priv->bank)); +
> > +	mxs_gpio_set_value(dev, offset, value);
> > +
> > +	writel(1 << offset, &reg->reg_set);
> > +
> > +	return 0;
> > +}
> > +
> > +static int mxs_gpio_get_function(struct udevice *dev, unsigned
> > offset) +{
> > +	struct mxs_gpio_priv *priv = dev_get_priv(dev);
> > +	struct mxs_register_32 *reg =
> > +		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
> > +
> > PINCTRL_DOE(priv->bank));
> > +	bool is_output = !!(readl(&reg->reg) >> offset);
> > +
> > +	return is_output ? GPIOF_OUTPUT : GPIOF_INPUT;
> > +}
> > +
> > +static const struct dm_gpio_ops gpio_mxs_ops = {
> > +	.direction_input	= mxs_gpio_direction_input,
> > +	.direction_output	= mxs_gpio_direction_output,
> > +	.get_value		= mxs_gpio_get_value,
> > +	.set_value		= mxs_gpio_set_value,
> > +	.get_function		= mxs_gpio_get_function,
> > +};
> > +
> > +static int mxs_gpio_probe(struct udevice *dev)
> > +{
> > +	struct mxs_gpio_priv *priv = dev_get_priv(dev);
> > +	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> > +	struct mxs_gpio_platdata *pdata =
> > +		(struct mxs_gpio_platdata
> > *)dev_get_driver_data(dev);
> > +	char name[18], *str;
> > +	int ret;
> > +
> > +	ret = dev_read_u32(dev, "reg", &priv->bank);  
> 
> devfdt_get_addr()

Good point - thanks.

> 
> > +	if (ret) {
> > +		printf("%s: No 'reg' property defined!\n",
> > __func__);
> > +		return ret;
> > +	}
> > +
> > +	sprintf(name, "GPIO%d_", priv->bank);  
> 
> Name cannot conceivably have more than 16 characters, why is the array
> 18 characters large ? 

It shall be 16. 

> Also, snprintf() would likely be better here.

Ok, I will replace it.

> 
> > +	str = strdup(name);
> > +	if (!str)
> > +		return -ENOMEM;
> > +
> > +	uc_priv->bank_name = str;
> > +	uc_priv->gpio_count =
> > pdata->gpio_nr_of_bank_pins[priv->bank]; +
> > +	return 0;
> > +}
> > +
> > +static const struct udevice_id mxs_gpio_ids[] = {
> > +	{ .compatible = "fsl,imx28-gpio", .data =
> > (ulong)&mxs_gpio_def },
> > +	{ }
> > +};
> > +
> > +U_BOOT_DRIVER(gpio_mxs) = {
> > +	.name	= "gpio_mxs",
> > +	.id	= UCLASS_GPIO,
> > +	.ops	= &gpio_mxs_ops,
> > +	.probe	= mxs_gpio_probe,
> > +	.priv_auto_alloc_size = sizeof(struct mxs_gpio_priv),
> > +	.of_match = mxs_gpio_ids,
> > +};
> > +#endif /* CONFIG_DM_GPIO */
> >   
> 
> 

Note:

[1] -
https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/imx28.dtsi


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190617/fc4b1d64/attachment.sig>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 5/5] ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI conversion)
  2019-06-17  6:49     ` Lukasz Majewski
@ 2019-06-17 10:06       ` Marek Vasut
  2019-06-17 12:27         ` Lukasz Majewski
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2019-06-17 10:06 UTC (permalink / raw)
  To: u-boot

On 6/17/19 8:49 AM, Lukasz Majewski wrote:
> Hi Marek,
> 
>> On 6/16/19 12:34 AM, Lukasz Majewski wrote:
>>> This commit converts mxs_spi driver to support DM/DTS.
>>>
>>> Signed-off-by: Lukasz Majewski <lukma@denx.de>  
>>
>> Is the non-DM part needed for anything ? 
> 
> Do you mean the non-DM part of the mxs_gpio driver? Yes, it is used by
> not converted boards.

This is a SPI driver though.

>> I recall the SPL jumps back
>> to BootROM when loading the U-Boot proper. So if not, drop it.
>>
>> Also, please don't do partial conversion for iMX28 only, do the iMX23
>> part as well, it cannot be hard.
> 
> Maybe it is not hard, but I cannot test it properly as I don't have
> i.MX23 device. If you are offering your help with testing (i.e. you do
> have the access to i.MX23 device and you will test those changes) I can
> add support for it.
> 
> Otherwise, NO, I will not add ANY new untested code.

In general, you don't have to add any code, the iMX23/28 SPI IP is very
much the same hardware, DTTO for most of the other blocks. If there are
any differences between iMX23/28, they are already handled in the
existing driver(s).

Half-way converted drivers in fact increase maintenance burden, because
then we have to deal with two different variants of the code, instead of
only one. That's why I would like to see this practice go away wherever
possible, and in this case it is possible.

If you need someone to test your changes, CC the board maintainers,
that's standard practice. If they don't respond, that cannot be helped.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 1/5] ARM: dts: imx: Copy imx28 device tree related files from Linux kernel (v5.1.9)
  2019-06-17  6:57     ` Lukasz Majewski
@ 2019-06-17 10:13       ` Marek Vasut
  0 siblings, 0 replies; 32+ messages in thread
From: Marek Vasut @ 2019-06-17 10:13 UTC (permalink / raw)
  To: u-boot

On 6/17/19 8:57 AM, Lukasz Majewski wrote:
> Hi Marek,

Hi,

>> On 6/16/19 12:34 AM, Lukasz Majewski wrote:
>>> This commit copies from the Linux kernel (tag v5.1.9) i.MX28 related
>>> device tree files.  
>>
>> Please use commit hash (first 12 characters of the SHA-1 ID, see [1]),
>> that's a unique identifier. Tag name is not.
> 
> Could you show me any example of tag: v5.1.9 in Linux kernel being not
> unique (or any other tag) ? 

I hope not, since that would bring it's own bag of problems.
But keep in mind that the tag is just a pointer to an object, it's not
part of the git history.

> Ok, I will add 12 digit SHA1 ID number.

Thanks, that is part of the history.

[...]

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 3/5] ARM: dm: mxs: gpio: Add support for DM/DTS in the mxs_gpio.c driver (DM_GPIO)
  2019-06-17  8:37     ` Lukasz Majewski
@ 2019-06-17 10:20       ` Marek Vasut
  2019-06-17 13:01         ` Lukasz Majewski
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2019-06-17 10:20 UTC (permalink / raw)
  To: u-boot

On 6/17/19 10:37 AM, Lukasz Majewski wrote:
> Hi Marek,
> 
>> On 6/16/19 12:34 AM, Lukasz Majewski wrote:
>>> This commit  
>>
>> This is not a commit, it's a change. It only becomes a commit when
>> applied.
>>
>>> adds support for DM in the mxs_gpio.c driver when DM_GPIO
>>> is enabled in Kconfig.  
>>
>> But this also adds support for DT probing, which is orthogonal to DM
>> support, yet it's not documented in the commit message.
> 
> Ok. Will rewrite the commit message to reflect the changes in the
> commit.
> 
>>
>>> Signed-off-by: Lukasz Majewski <lukma@denx.de>
>>>
>>> ---
>>>
>>> Changes in v3:
>>> - Set more apropriate tags
>>>
>>> Changes in v2:
>>> - Use #if !CONFIG_IS_ENABLED(DM_GPIO) instead of plain #ifdef
>>> CONFIG_DM_GPIO
>>>
>>>  arch/arm/include/asm/arch-mxs/gpio.h |  28 +++++++
>>>  drivers/gpio/mxs_gpio.c              | 149
>>> +++++++++++++++++++++++++++++++++++ 2 files changed, 177
>>> insertions(+)
>>>
>>> diff --git a/arch/arm/include/asm/arch-mxs/gpio.h
>>> b/arch/arm/include/asm/arch-mxs/gpio.h index 34fa421945..0c152e25e2
>>> 100644 --- a/arch/arm/include/asm/arch-mxs/gpio.h
>>> +++ b/arch/arm/include/asm/arch-mxs/gpio.h
>>> @@ -15,4 +15,32 @@ void mxs_gpio_init(void);
>>>  inline void mxs_gpio_init(void) {}
>>>  #endif
>>>  
>>> +#if defined(CONFIG_MX28) && CONFIG_IS_ENABLED(DM_GPIO)
>>> +/*
>>> + * According to i.MX28 Reference Manual:
>>> + * 'i.MX28 Applications Processor Reference Manual, Rev. 1, 2010'
>>> + * The i.MX28 has following number of GPIOs available:
>>> + * Bank 0: 0-28 -> 29 PINS
>>> + * Bank 1: 0-31 -> 32 PINS
>>> + * Bank 2: 0-27 -> 28 PINS
>>> + * Bank 3: 0-30 -> 31 PINS
>>> + * Bank 4: 0-20 -> 21 PINS
>>> + */
>>> +#define IMX28_GPIO_BANK0_PIN_NR 29
>>> +#define IMX28_GPIO_BANK1_PIN_NR 32
>>> +#define IMX28_GPIO_BANK2_PIN_NR 28
>>> +#define IMX28_GPIO_BANK3_PIN_NR 31
>>> +#define IMX28_GPIO_BANK4_PIN_NR 21
>>> +#define IMX28_GPIO_BANK_NR 5  
>>
>> Please make a complete conversion, not partial one.
>> You want to use gpio-ranges in DT and then parse the size of each GPIO
>> bank from those gpio-ranges , not hard-code it into the driver.
> 
> I would have used the gpio-ranges, but the original Linux code [1]
> (imx28.dtsi) doesn't define them.

You can add them to imx28-u-boot.dtsi , and submit patch for mainline
Linux, it's easy.

> Also, the dts files which include [1] don't extend original gpio nodes
> to have one.
> 
> As it is not available in the Linux kernel, I don't see any good reason
> to add the gpio-ranges to U-Boot. The same approach, as presented here,
> is used in zynq_gpio.c file (which also uses DM/DTS).
> 
> Adding per u-boot property - like 'u-boot,mx28-gpio-ranges' is also less
> appealing than 24 lines of code, which can be then easily re-used with
> OF_PLATDATA (without DM/DTS suport) in SPL (u-boot.sb to be precise).

It is well possible the zynq DTs predate the gpio-ranges . Read the
documentation for it at [2] . It even explicitly states it's used for
interaction between pincontrol and gpio controllers.

[2]
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/gpio/gpio.txt#L239

>>> +struct mxs_gpio_platdata {
>>> +	u32 gpio_nr_of_bank_pins[IMX28_GPIO_BANK_NR];
>>> +	u32 gpio_base_nr[IMX28_GPIO_BANK_NR];
>>> +	u32 ngpio;
>>> +};
>>> +
>>> +extern const struct mxs_gpio_platdata mxs_gpio_def;
>>> +#define IMX_GPIO_NR(port, index)
>>> (mxs_gpio_def.gpio_base_nr[(port)] + (index))  
>>
>> So this should be completely unnecessary when using the DM GPIO, this
>> was only needed for non-DM-GPIO .
> 
> This is a compatibility layer - for some reason ALL iMX ports define it
> - i.e. imx8, imx7 - which shall use DM/DTS for gpio from the outset.
> 
> With the in-board code the dm_gpio_* set of functions shall (and
> will) be used (as done in opos6ul.c file).

Then use them and drop this.

>>> +#endif
>>> +
>>>  #endif	/* __MX28_GPIO_H__ */
>>> diff --git a/drivers/gpio/mxs_gpio.c b/drivers/gpio/mxs_gpio.c
>>> index c2c8a25886..f386235ff1 100644
>>> --- a/drivers/gpio/mxs_gpio.c
>>> +++ b/drivers/gpio/mxs_gpio.c
>>> @@ -51,6 +51,7 @@ void mxs_gpio_init(void)
>>>  	}
>>>  }
>>>  
>>> +#if !CONFIG_IS_ENABLED(DM_GPIO)
>>>  int gpio_get_value(unsigned gpio)
>>>  {
>>>  	uint32_t bank = PAD_BANK(gpio);
>>> @@ -127,3 +128,151 @@ int name_to_gpio(const char *name)
>>>  
>>>  	return (bank << MXS_PAD_BANK_SHIFT) | (pin <<
>>> MXS_PAD_PIN_SHIFT); }
>>> +#else /* CONFIG_DM_GPIO */
>>> +#include <dm.h>
>>> +#include <asm/gpio.h>
>>> +#include <asm/arch/gpio.h>
>>> +#ifdef CONFIG_MX28
>>> +const struct mxs_gpio_platdata mxs_gpio_def = {
>>> +	.gpio_nr_of_bank_pins[0] = IMX28_GPIO_BANK0_PIN_NR,
>>> +	.gpio_nr_of_bank_pins[1] = IMX28_GPIO_BANK1_PIN_NR,
>>> +	.gpio_nr_of_bank_pins[2] = IMX28_GPIO_BANK2_PIN_NR,
>>> +	.gpio_nr_of_bank_pins[3] = IMX28_GPIO_BANK3_PIN_NR,
>>> +	.gpio_nr_of_bank_pins[4] = IMX28_GPIO_BANK4_PIN_NR,
>>> +	.gpio_base_nr[0] = 0,
>>> +	.gpio_base_nr[1] = IMX28_GPIO_BANK0_PIN_NR,
>>> +	.gpio_base_nr[2] = (IMX28_GPIO_BANK0_PIN_NR + \
>>> +			   IMX28_GPIO_BANK1_PIN_NR),
>>> +	.gpio_base_nr[3] = (IMX28_GPIO_BANK0_PIN_NR + \
>>> +			   IMX28_GPIO_BANK1_PIN_NR + \
>>> +			   IMX28_GPIO_BANK2_PIN_NR),
>>> +	.gpio_base_nr[4] = (IMX28_GPIO_BANK0_PIN_NR + \
>>> +			   IMX28_GPIO_BANK1_PIN_NR + \
>>> +			   IMX28_GPIO_BANK2_PIN_NR + \
>>> +			   IMX28_GPIO_BANK3_PIN_NR),
>>> +	.ngpio = (IMX28_GPIO_BANK0_PIN_NR + \
>>> +		  IMX28_GPIO_BANK1_PIN_NR + \
>>> +		  IMX28_GPIO_BANK2_PIN_NR + \
>>> +		  IMX28_GPIO_BANK3_PIN_NR + \
>>> +		  IMX28_GPIO_BANK4_PIN_NR),
>>> +};  
>>
>> So please use gpio-ranges in DT.
> 
> Please see the above comment regarding gpio-ranges.

DTTO

>>> +#else
>>> +#error "Only i.MX28 supported with DM_GPIO"
>>> +#endif
>>> +
>>> +struct mxs_gpio_priv {
>>> +	unsigned int bank;
>>> +};
>>> +
>>> +static int mxs_gpio_get_value(struct udevice *dev, unsigned offset)
>>> +{
>>> +	struct mxs_gpio_priv *priv = dev_get_priv(dev);
>>> +	struct mxs_register_32 *reg =
>>> +		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
>>> +
>>> PINCTRL_DIN(priv->bank)); +
>>> +	return (readl(&reg->reg) >> offset) & 1;
>>> +}
>>> +
>>> +static int mxs_gpio_set_value(struct udevice *dev, unsigned offset,
>>> +			      int value)
>>> +{
>>> +	struct mxs_gpio_priv *priv = dev_get_priv(dev);
>>> +	struct mxs_register_32 *reg =
>>> +		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
>>> +
>>> PINCTRL_DOUT(priv->bank));
>>> +	if (value)
>>> +		writel(1 << offset, &reg->reg_set);  
>>
>> BIT(), fix globally.
> 
> I took it from the original implementation :-).

Original implementation is very old code.

[...]

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 4/5] ARM: imx: pinctrl: Add support for i.MX28 mxs pinctrl driver
  2019-06-17  7:43     ` Lukasz Majewski
@ 2019-06-17 10:23       ` Marek Vasut
  0 siblings, 0 replies; 32+ messages in thread
From: Marek Vasut @ 2019-06-17 10:23 UTC (permalink / raw)
  To: u-boot

On 6/17/19 9:43 AM, Lukasz Majewski wrote:
> Hi Marek,
> 
>> On 6/16/19 12:34 AM, Lukasz Majewski wrote:
>>> The code responsible for setting proper values in the MUX registers
>>> (in the mxs_pinctrl_set_state()) has been ported from Barebox
>>> project (branch: master, SHA1:
>>> eb3b0f7414cd8102844dd16b1c789e445e8947f8, file:
>>> drivers/pinctrl/pinctrl-mxs.c).  
>>
>> The format of a commit, when referenced, is documented here:
>> https://www.kernel.org/doc/html/v4.12/process/submitting-patches.html
>> e.g. e21d2170f366 ("video: remove unnecessary platform_set_drvdata()")
>>
>> Although. maybe you should port this from Linux instead ?
> 
> The code from linux is a bit more convoluted and verbose. I may re-look
> into the kernel version.

Please do

>>> As the pinctrl node in the imx28.dtsi file has gpio pins nodes as
>>> subnodes, it was necessary to use 'dm_scan_fdt_dev()' (as a .bind
>>> method) to also make them 'visible' by the DM's "gpio_mxs" driver.  
>>
>> Look at drivers/pinctrl/renesas/pfc-r7s72100.c r7s72100_pfc_probe() ,
>> I think that one deals with the exact same problem .
> 
> It looks like this is the same problem.
> 
> However, the dm_scan_fdt_dev() is exactly for that, when combined
> with .bind method from DM.

Ah, because you have a compat string for the GPIO controllers, right.
Then please ignore this comment, this is better solution.

>>> Signed-off-by: Lukasz Majewski <lukma@denx.de>
>>>
>>> ---
>>>
>>> Changes in v3:
>>> - Set more apropriate tags
>>>
>>> Changes in v2: None
>>>
>>>  drivers/pinctrl/nxp/Kconfig       |  10 +++
>>>  drivers/pinctrl/nxp/Makefile      |   1 +
>>>  drivers/pinctrl/nxp/pinctrl-mxs.c | 164
>>> ++++++++++++++++++++++++++++++++++++++ 3 files changed, 175
>>> insertions(+) create mode 100644 drivers/pinctrl/nxp/pinctrl-mxs.c
>>>
>>> diff --git a/drivers/pinctrl/nxp/Kconfig
>>> b/drivers/pinctrl/nxp/Kconfig index 61f93be42d..f2e67ca231 100644
>>> --- a/drivers/pinctrl/nxp/Kconfig
>>> +++ b/drivers/pinctrl/nxp/Kconfig
>>> @@ -89,6 +89,16 @@ config PINCTRL_IMX8M
>>>  	  only parses the 'fsl,pins' property and configure related
>>>  	  registers.
>>>  
>>> +config PINCTRL_MXS
>>> +	bool "NXP MXS pinctrl driver"
>>> +	depends on ARCH_MX28 && PINCTRL_FULL
>>> +	help
>>> +	  Say Y here to enable the i.MX mxs pinctrl driver
>>> +
>>> +	  This option provides a simple pinctrl driver for i.MX
>>> mxs SoC
>>> +	  familiy, e.g. i.MX28. This feature depends on device tree
>>> +	  configuration.
>>> +
>>>  config PINCTRL_VYBRID
>>>  	bool "Vybrid (vf610) pinctrl driver"
>>>  	depends on ARCH_VF610 && PINCTRL_FULL
>>> diff --git a/drivers/pinctrl/nxp/Makefile
>>> b/drivers/pinctrl/nxp/Makefile index b340d9448a..b86448aac9 100644
>>> --- a/drivers/pinctrl/nxp/Makefile
>>> +++ b/drivers/pinctrl/nxp/Makefile
>>> @@ -6,4 +6,5 @@ obj-$(CONFIG_PINCTRL_IMX7ULP)		+=
>>> pinctrl-imx7ulp.o obj-$(CONFIG_PINCTRL_IMX_SCU)		+=
>>> pinctrl-scu.o obj-$(CONFIG_PINCTRL_IMX8)		+=
>>> pinctrl-imx8.o obj-$(CONFIG_PINCTRL_IMX8M)		+=
>>> pinctrl-imx8m.o +obj-$(CONFIG_PINCTRL_MXS)		+=
>>> pinctrl-mxs.o obj-$(CONFIG_PINCTRL_VYBRID)		+=
>>> pinctrl-vf610.o diff --git a/drivers/pinctrl/nxp/pinctrl-mxs.c
>>> b/drivers/pinctrl/nxp/pinctrl-mxs.c new file mode 100644
>>> index 0000000000..42b1b7998b
>>> --- /dev/null
>>> +++ b/drivers/pinctrl/nxp/pinctrl-mxs.c
>>> @@ -0,0 +1,164 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (C) 2019 DENX Software Engineering
>>> + * Lukasz Majewski, DENX Software Engineering, lukma at denx.de
>>> + *
>>> + * This work is based on drivers/pinctrl/pinctrl-mxs.c
>>> + * SHA1: eb3b0f7414cd8102844dd16b1c789e445e8947f8
>>> + * from Barebox project.
>>> + *
>>> + * Author of original Barebox pinctrl-mxs.c:
>>> + * Copyright (c) 2015 Sascha Hauer <s.hauer@pengutronix.de>
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <linux/io.h>
>>> +#include <linux/err.h>
>>> +#include <dm.h>
>>> +#include <dm/pinctrl.h>
>>> +#include <dm/read.h>
>>> +
>>> +DECLARE_GLOBAL_DATA_PTR;
>>> +
>>> +#define PINID(bank, pin)	((bank) * 32 + (pin))
>>> +#define MUXID_TO_PINID(m)	PINID((m) >> 12 & 0xf, (m) >> 4 &
>>> 0xff) +#define MUXID_TO_MUXSEL(m)	((m) & 0xf)
>>> +#define PINID_TO_BANK(p)	((p) >> 5)
>>> +#define PINID_TO_PIN(p)	((p) % 32)
>>> +
>>> +#define STMP_OFFSET_REG_SET     0x4
>>> +#define STMP_OFFSET_REG_CLR     0x8
>>> +#define STMP_OFFSET_REG_TOG     0xc
>>> +
>>> +struct mxs_pinctrl_priv {
>>> +	void __iomem *base;
>>> +};
>>> +
>>> +static int mxs_pinctrl_set_state(struct udevice *dev, struct
>>> udevice *config) +{
>>> +	int ma_present = 0, vol_present = 0, pull_present = 0;
>>> +	struct mxs_pinctrl_priv *iomux = dev_get_priv(dev);
>>> +	u32 *pin_data, val, ma, vol, pull;
>>> +	int npins, size, i, ret;
>>> +
>>> +	debug("\n%s: set state: %s\n", __func__, config->name);
>>> +
>>> +	size = dev_read_size(config, "fsl,pinmux-ids");
>>> +	if (size < 0)
>>> +		return size;
>>> +
>>> +	if (!size || size % 4) {
>>> +		dev_err(dev, "Invalid fsl,pinmux-ids property in
>>> %s\n",
>>> +			config->name);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	npins = size / 4;
>>> +
>>> +	pin_data = devm_kzalloc(dev, size, 0);  
>>
>> Is this ever free'd() ?
>>
> 
> Ach... right the devm_kfree() shall be used on the exit from the
> function.
> 
>> [...]
>>
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 5/5] ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI conversion)
  2019-06-17 10:06       ` Marek Vasut
@ 2019-06-17 12:27         ` Lukasz Majewski
  2019-06-17 13:23           ` Marek Vasut
  0 siblings, 1 reply; 32+ messages in thread
From: Lukasz Majewski @ 2019-06-17 12:27 UTC (permalink / raw)
  To: u-boot

Hi Marek,

> On 6/17/19 8:49 AM, Lukasz Majewski wrote:
> > Hi Marek,
> >   
> >> On 6/16/19 12:34 AM, Lukasz Majewski wrote:  
> >>> This commit converts mxs_spi driver to support DM/DTS.
> >>>
> >>> Signed-off-by: Lukasz Majewski <lukma@denx.de>    
> >>
> >> Is the non-DM part needed for anything ?   
> > 
> > Do you mean the non-DM part of the mxs_gpio driver? Yes, it is used
> > by not converted boards.  
> 
> This is a SPI driver though.
> 
> >> I recall the SPL jumps back
> >> to BootROM when loading the U-Boot proper. So if not, drop it.
> >>
> >> Also, please don't do partial conversion for iMX28 only, do the
> >> iMX23 part as well, it cannot be hard.  
> > 
> > Maybe it is not hard, but I cannot test it properly as I don't have
> > i.MX23 device. If you are offering your help with testing (i.e. you
> > do have the access to i.MX23 device and you will test those
> > changes) I can add support for it.
> > 
> > Otherwise, NO, I will not add ANY new untested code.  
> 
> In general, you don't have to add any code, the iMX23/28 SPI IP is
> very much the same hardware, DTTO for most of the other blocks. If
> there are any differences between iMX23/28, they are already handled
> in the existing driver(s).
> 
> Half-way converted drivers in fact increase maintenance burden,
> because then we have to deal with two different variants of the code,
> instead of only one.

I cannot agree with this sentence. The conversion would be done for
i.MX28, which is then tested and validated (and clearly stated in the
cover letter/commit message that only supported was i.MX28).

If I don't need to adjust common, reused code (which already supports
both variants as it is the case with mxs_spi.c), then I don't mind.

For more intrusive changes - the driver needs to be tested and
validated (by somebody who has the HW for testing).

> That's why I would like to see this practice go
> away wherever possible, and in this case it is possible.

In this particular case it is possible to add support for both as SoC
specific changes (i.MX23 vs i.MX28) is performed in common code (e.g.
mxs_spi_xfer_dma).


> 
> If you need someone to test your changes, CC the board maintainers,
> that's standard practice.

As fair as I remember only Angelo and Michael had also interest in
testing converted code for i.MX28 based board.

There was NO reply from other people when this (and few others) driver
was marked as DEPRECATED.

> If they don't respond, that cannot be
> helped.
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190617/d771ce19/attachment.sig>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 3/5] ARM: dm: mxs: gpio: Add support for DM/DTS in the mxs_gpio.c driver (DM_GPIO)
  2019-06-17 10:20       ` Marek Vasut
@ 2019-06-17 13:01         ` Lukasz Majewski
  2019-06-17 13:34           ` Marek Vasut
  0 siblings, 1 reply; 32+ messages in thread
From: Lukasz Majewski @ 2019-06-17 13:01 UTC (permalink / raw)
  To: u-boot

Hi Marek,

> On 6/17/19 10:37 AM, Lukasz Majewski wrote:
> > Hi Marek,
> >   
> >> On 6/16/19 12:34 AM, Lukasz Majewski wrote:  
> >>> This commit    
> >>
> >> This is not a commit, it's a change. It only becomes a commit when
> >> applied.
> >>  
> >>> adds support for DM in the mxs_gpio.c driver when DM_GPIO
> >>> is enabled in Kconfig.    
> >>
> >> But this also adds support for DT probing, which is orthogonal to
> >> DM support, yet it's not documented in the commit message.  
> > 
> > Ok. Will rewrite the commit message to reflect the changes in the
> > commit.
> >   
> >>  
> >>> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> >>>
> >>> ---
> >>>
> >>> Changes in v3:
> >>> - Set more apropriate tags
> >>>
> >>> Changes in v2:
> >>> - Use #if !CONFIG_IS_ENABLED(DM_GPIO) instead of plain #ifdef
> >>> CONFIG_DM_GPIO
> >>>
> >>>  arch/arm/include/asm/arch-mxs/gpio.h |  28 +++++++
> >>>  drivers/gpio/mxs_gpio.c              | 149
> >>> +++++++++++++++++++++++++++++++++++ 2 files changed, 177
> >>> insertions(+)
> >>>
> >>> diff --git a/arch/arm/include/asm/arch-mxs/gpio.h
> >>> b/arch/arm/include/asm/arch-mxs/gpio.h index
> >>> 34fa421945..0c152e25e2 100644 ---
> >>> a/arch/arm/include/asm/arch-mxs/gpio.h +++
> >>> b/arch/arm/include/asm/arch-mxs/gpio.h @@ -15,4 +15,32 @@ void
> >>> mxs_gpio_init(void); inline void mxs_gpio_init(void) {}
> >>>  #endif
> >>>  
> >>> +#if defined(CONFIG_MX28) && CONFIG_IS_ENABLED(DM_GPIO)
> >>> +/*
> >>> + * According to i.MX28 Reference Manual:
> >>> + * 'i.MX28 Applications Processor Reference Manual, Rev. 1, 2010'
> >>> + * The i.MX28 has following number of GPIOs available:
> >>> + * Bank 0: 0-28 -> 29 PINS
> >>> + * Bank 1: 0-31 -> 32 PINS
> >>> + * Bank 2: 0-27 -> 28 PINS
> >>> + * Bank 3: 0-30 -> 31 PINS
> >>> + * Bank 4: 0-20 -> 21 PINS
> >>> + */
> >>> +#define IMX28_GPIO_BANK0_PIN_NR 29
> >>> +#define IMX28_GPIO_BANK1_PIN_NR 32
> >>> +#define IMX28_GPIO_BANK2_PIN_NR 28
> >>> +#define IMX28_GPIO_BANK3_PIN_NR 31
> >>> +#define IMX28_GPIO_BANK4_PIN_NR 21
> >>> +#define IMX28_GPIO_BANK_NR 5    
> >>
> >> Please make a complete conversion, not partial one.
> >> You want to use gpio-ranges in DT and then parse the size of each
> >> GPIO bank from those gpio-ranges , not hard-code it into the
> >> driver.  
> > 
> > I would have used the gpio-ranges, but the original Linux code [1]
> > (imx28.dtsi) doesn't define them.  
> 
> You can add them to imx28-u-boot.dtsi , 

No, IMHO this is not the best solution. My point is:

1. i.MX28 driver in Linux is stable and it works (without gpio-ranges).
I do not have any interest in fixing code which is already working. If
that were new driver then no issue to use gpio-ranges from the outset.
Also if Linux kernel driver would support it - then also no problems
with adding it.


2. The proposed code is small - only 24 LOC and doesn't require any
extra parsing of DTS (including pinctrl driver's properties).

Why shall I make the driver more verbose if I can avoid it?

3. It is easily reusable in SPL.

> and submit patch for mainline
> Linux, it's easy.

Submitting patches to Linux is easy - but to have them already accepted
and pulled is not :-).

> 
> > Also, the dts files which include [1] don't extend original gpio
> > nodes to have one.
> > 
> > As it is not available in the Linux kernel, I don't see any good
> > reason to add the gpio-ranges to U-Boot. The same approach, as
> > presented here, is used in zynq_gpio.c file (which also uses
> > DM/DTS).
> > 
> > Adding per u-boot property - like 'u-boot,mx28-gpio-ranges' is also
> > less appealing than 24 lines of code, which can be then easily
> > re-used with OF_PLATDATA (without DM/DTS suport) in SPL (u-boot.sb
> > to be precise).  
> 
> It is well possible the zynq DTs predate the gpio-ranges .

No, it is not.

> Read the
> documentation for it at [2] . It even explicitly states it's used for
> interaction between pincontrol and gpio controllers.

In those cases where both support it. The i.MX28 Linux GPIO driver is
NOT supporting gpio-ranges.

> 
> [2]
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/gpio/gpio.txt#L239
> 
> >>> +struct mxs_gpio_platdata {
> >>> +	u32 gpio_nr_of_bank_pins[IMX28_GPIO_BANK_NR];
> >>> +	u32 gpio_base_nr[IMX28_GPIO_BANK_NR];
> >>> +	u32 ngpio;
> >>> +};
> >>> +
> >>> +extern const struct mxs_gpio_platdata mxs_gpio_def;
> >>> +#define IMX_GPIO_NR(port, index)
> >>> (mxs_gpio_def.gpio_base_nr[(port)] + (index))    
> >>
> >> So this should be completely unnecessary when using the DM GPIO,
> >> this was only needed for non-DM-GPIO .  
> > 
> > This is a compatibility layer - for some reason ALL iMX ports
> > define it
> > - i.e. imx8, imx7 - which shall use DM/DTS for gpio from the outset.
> > 
> > With the in-board code the dm_gpio_* set of functions shall (and
> > will) be used (as done in opos6ul.c file).  
> 
> Then use them and drop this.

I will use the new dm_gpio_* API where applicable. However, to be in
sync with other iMX ports the IMX_GPIO_NR() shall be also defined.

> 
> >>> +#endif
> >>> +
> >>>  #endif	/* __MX28_GPIO_H__ */
> >>> diff --git a/drivers/gpio/mxs_gpio.c b/drivers/gpio/mxs_gpio.c
> >>> index c2c8a25886..f386235ff1 100644
> >>> --- a/drivers/gpio/mxs_gpio.c
> >>> +++ b/drivers/gpio/mxs_gpio.c
> >>> @@ -51,6 +51,7 @@ void mxs_gpio_init(void)
> >>>  	}
> >>>  }
> >>>  
> >>> +#if !CONFIG_IS_ENABLED(DM_GPIO)
> >>>  int gpio_get_value(unsigned gpio)
> >>>  {
> >>>  	uint32_t bank = PAD_BANK(gpio);
> >>> @@ -127,3 +128,151 @@ int name_to_gpio(const char *name)
> >>>  
> >>>  	return (bank << MXS_PAD_BANK_SHIFT) | (pin <<
> >>> MXS_PAD_PIN_SHIFT); }
> >>> +#else /* CONFIG_DM_GPIO */
> >>> +#include <dm.h>
> >>> +#include <asm/gpio.h>
> >>> +#include <asm/arch/gpio.h>
> >>> +#ifdef CONFIG_MX28
> >>> +const struct mxs_gpio_platdata mxs_gpio_def = {
> >>> +	.gpio_nr_of_bank_pins[0] = IMX28_GPIO_BANK0_PIN_NR,
> >>> +	.gpio_nr_of_bank_pins[1] = IMX28_GPIO_BANK1_PIN_NR,
> >>> +	.gpio_nr_of_bank_pins[2] = IMX28_GPIO_BANK2_PIN_NR,
> >>> +	.gpio_nr_of_bank_pins[3] = IMX28_GPIO_BANK3_PIN_NR,
> >>> +	.gpio_nr_of_bank_pins[4] = IMX28_GPIO_BANK4_PIN_NR,
> >>> +	.gpio_base_nr[0] = 0,
> >>> +	.gpio_base_nr[1] = IMX28_GPIO_BANK0_PIN_NR,
> >>> +	.gpio_base_nr[2] = (IMX28_GPIO_BANK0_PIN_NR + \
> >>> +			   IMX28_GPIO_BANK1_PIN_NR),
> >>> +	.gpio_base_nr[3] = (IMX28_GPIO_BANK0_PIN_NR + \
> >>> +			   IMX28_GPIO_BANK1_PIN_NR + \
> >>> +			   IMX28_GPIO_BANK2_PIN_NR),
> >>> +	.gpio_base_nr[4] = (IMX28_GPIO_BANK0_PIN_NR + \
> >>> +			   IMX28_GPIO_BANK1_PIN_NR + \
> >>> +			   IMX28_GPIO_BANK2_PIN_NR + \
> >>> +			   IMX28_GPIO_BANK3_PIN_NR),
> >>> +	.ngpio = (IMX28_GPIO_BANK0_PIN_NR + \
> >>> +		  IMX28_GPIO_BANK1_PIN_NR + \
> >>> +		  IMX28_GPIO_BANK2_PIN_NR + \
> >>> +		  IMX28_GPIO_BANK3_PIN_NR + \
> >>> +		  IMX28_GPIO_BANK4_PIN_NR),
> >>> +};    
> >>
> >> So please use gpio-ranges in DT.  
> > 
> > Please see the above comment regarding gpio-ranges.  
> 
> DTTO
> 
> >>> +#else
> >>> +#error "Only i.MX28 supported with DM_GPIO"
> >>> +#endif
> >>> +
> >>> +struct mxs_gpio_priv {
> >>> +	unsigned int bank;
> >>> +};
> >>> +
> >>> +static int mxs_gpio_get_value(struct udevice *dev, unsigned
> >>> offset) +{
> >>> +	struct mxs_gpio_priv *priv = dev_get_priv(dev);
> >>> +	struct mxs_register_32 *reg =
> >>> +		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
> >>> +
> >>> PINCTRL_DIN(priv->bank)); +
> >>> +	return (readl(&reg->reg) >> offset) & 1;
> >>> +}
> >>> +
> >>> +static int mxs_gpio_set_value(struct udevice *dev, unsigned
> >>> offset,
> >>> +			      int value)
> >>> +{
> >>> +	struct mxs_gpio_priv *priv = dev_get_priv(dev);
> >>> +	struct mxs_register_32 *reg =
> >>> +		(struct mxs_register_32 *)(MXS_PINCTRL_BASE +
> >>> +
> >>> PINCTRL_DOUT(priv->bank));
> >>> +	if (value)
> >>> +		writel(1 << offset, &reg->reg_set);    
> >>
> >> BIT(), fix globally.  
> > 
> > I took it from the original implementation :-).  
> 
> Original implementation is very old code.

:-)

> 
> [...]
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190617/a02231d2/attachment.sig>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 5/5] ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI conversion)
  2019-06-17 12:27         ` Lukasz Majewski
@ 2019-06-17 13:23           ` Marek Vasut
  2019-06-17 13:41             ` Lukasz Majewski
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2019-06-17 13:23 UTC (permalink / raw)
  To: u-boot

On 6/17/19 2:27 PM, Lukasz Majewski wrote:
> Hi Marek,
> 
>> On 6/17/19 8:49 AM, Lukasz Majewski wrote:
>>> Hi Marek,
>>>   
>>>> On 6/16/19 12:34 AM, Lukasz Majewski wrote:  
>>>>> This commit converts mxs_spi driver to support DM/DTS.
>>>>>
>>>>> Signed-off-by: Lukasz Majewski <lukma@denx.de>    
>>>>
>>>> Is the non-DM part needed for anything ?   
>>>
>>> Do you mean the non-DM part of the mxs_gpio driver? Yes, it is used
>>> by not converted boards.  
>>
>> This is a SPI driver though.
>>
>>>> I recall the SPL jumps back
>>>> to BootROM when loading the U-Boot proper. So if not, drop it.
>>>>
>>>> Also, please don't do partial conversion for iMX28 only, do the
>>>> iMX23 part as well, it cannot be hard.  
>>>
>>> Maybe it is not hard, but I cannot test it properly as I don't have
>>> i.MX23 device. If you are offering your help with testing (i.e. you
>>> do have the access to i.MX23 device and you will test those
>>> changes) I can add support for it.
>>>
>>> Otherwise, NO, I will not add ANY new untested code.  
>>
>> In general, you don't have to add any code, the iMX23/28 SPI IP is
>> very much the same hardware, DTTO for most of the other blocks. If
>> there are any differences between iMX23/28, they are already handled
>> in the existing driver(s).
>>
>> Half-way converted drivers in fact increase maintenance burden,
>> because then we have to deal with two different variants of the code,
>> instead of only one.
> 
> I cannot agree with this sentence.

Do you think maintaining - one DM driver which supports both iMX23 and
iMX28 - is more burden than maintaining - one driver which supports DM,
but only for iMX28 and non-DM for iMX23 and iMX28 ? I don't think so.

> The conversion would be done for
> i.MX28, which is then tested and validated (and clearly stated in the
> cover letter/commit message that only supported was i.MX28).>
> If I don't need to adjust common, reused code (which already supports
> both variants as it is the case with mxs_spi.c), then I don't mind.

Well, that is what I said above, you don't.

> For more intrusive changes - the driver needs to be tested and
> validated (by somebody who has the HW for testing).

That's up to board maintainers.

>> That's why I would like to see this practice go
>> away wherever possible, and in this case it is possible.
> 
> In this particular case it is possible to add support for both as SoC
> specific changes (i.MX23 vs i.MX28) is performed in common code (e.g.
> mxs_spi_xfer_dma).

Both SPI and DMA blocks are basically the same on iMX23 and iMX28.

>> If you need someone to test your changes, CC the board maintainers,
>> that's standard practice.
> 
> As fair as I remember only Angelo and Michael had also interest in
> testing converted code for i.MX28 based board.
> 
> There was NO reply from other people when this (and few others) driver
> was marked as DEPRECATED.

Well, too bad, clearly the interest in this platform is low.
That does not mean we should do sub-par upstream work, does it ?

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 3/5] ARM: dm: mxs: gpio: Add support for DM/DTS in the mxs_gpio.c driver (DM_GPIO)
  2019-06-17 13:01         ` Lukasz Majewski
@ 2019-06-17 13:34           ` Marek Vasut
  2019-06-18  6:57             ` Lukasz Majewski
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2019-06-17 13:34 UTC (permalink / raw)
  To: u-boot

On 6/17/19 3:01 PM, Lukasz Majewski wrote:
> Hi Marek,
> 
>> On 6/17/19 10:37 AM, Lukasz Majewski wrote:
>>> Hi Marek,
>>>   
>>>> On 6/16/19 12:34 AM, Lukasz Majewski wrote:  
>>>>> This commit    
>>>>
>>>> This is not a commit, it's a change. It only becomes a commit when
>>>> applied.
>>>>  
>>>>> adds support for DM in the mxs_gpio.c driver when DM_GPIO
>>>>> is enabled in Kconfig.    
>>>>
>>>> But this also adds support for DT probing, which is orthogonal to
>>>> DM support, yet it's not documented in the commit message.  
>>>
>>> Ok. Will rewrite the commit message to reflect the changes in the
>>> commit.
>>>   
>>>>  
>>>>> Signed-off-by: Lukasz Majewski <lukma@denx.de>
>>>>>
>>>>> ---
>>>>>
>>>>> Changes in v3:
>>>>> - Set more apropriate tags
>>>>>
>>>>> Changes in v2:
>>>>> - Use #if !CONFIG_IS_ENABLED(DM_GPIO) instead of plain #ifdef
>>>>> CONFIG_DM_GPIO
>>>>>
>>>>>  arch/arm/include/asm/arch-mxs/gpio.h |  28 +++++++
>>>>>  drivers/gpio/mxs_gpio.c              | 149
>>>>> +++++++++++++++++++++++++++++++++++ 2 files changed, 177
>>>>> insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/include/asm/arch-mxs/gpio.h
>>>>> b/arch/arm/include/asm/arch-mxs/gpio.h index
>>>>> 34fa421945..0c152e25e2 100644 ---
>>>>> a/arch/arm/include/asm/arch-mxs/gpio.h +++
>>>>> b/arch/arm/include/asm/arch-mxs/gpio.h @@ -15,4 +15,32 @@ void
>>>>> mxs_gpio_init(void); inline void mxs_gpio_init(void) {}
>>>>>  #endif
>>>>>  
>>>>> +#if defined(CONFIG_MX28) && CONFIG_IS_ENABLED(DM_GPIO)
>>>>> +/*
>>>>> + * According to i.MX28 Reference Manual:
>>>>> + * 'i.MX28 Applications Processor Reference Manual, Rev. 1, 2010'
>>>>> + * The i.MX28 has following number of GPIOs available:
>>>>> + * Bank 0: 0-28 -> 29 PINS
>>>>> + * Bank 1: 0-31 -> 32 PINS
>>>>> + * Bank 2: 0-27 -> 28 PINS
>>>>> + * Bank 3: 0-30 -> 31 PINS
>>>>> + * Bank 4: 0-20 -> 21 PINS
>>>>> + */
>>>>> +#define IMX28_GPIO_BANK0_PIN_NR 29
>>>>> +#define IMX28_GPIO_BANK1_PIN_NR 32
>>>>> +#define IMX28_GPIO_BANK2_PIN_NR 28
>>>>> +#define IMX28_GPIO_BANK3_PIN_NR 31
>>>>> +#define IMX28_GPIO_BANK4_PIN_NR 21
>>>>> +#define IMX28_GPIO_BANK_NR 5    
>>>>
>>>> Please make a complete conversion, not partial one.
>>>> You want to use gpio-ranges in DT and then parse the size of each
>>>> GPIO bank from those gpio-ranges , not hard-code it into the
>>>> driver.  
>>>
>>> I would have used the gpio-ranges, but the original Linux code [1]
>>> (imx28.dtsi) doesn't define them.  
>>
>> You can add them to imx28-u-boot.dtsi , 
> 
> No, IMHO this is not the best solution. My point is:
> 
> 1. i.MX28 driver in Linux is stable and it works (without gpio-ranges).
> I do not have any interest in fixing code which is already working. If
> that were new driver then no issue to use gpio-ranges from the outset.
> Also if Linux kernel driver would support it - then also no problems
> with adding it.

Linux is continuously improving, so is U-Boot, code is being constantly
rewritten and improved. So this isn't really a convincing argument.

> 2. The proposed code is small - only 24 LOC and doesn't require any
> extra parsing of DTS (including pinctrl driver's properties).
> 
> Why shall I make the driver more verbose if I can avoid it?

It also adds churn into the driver which does not have to be there. In
fact, DT is a hardware description, it should describe hardware and it
has facilities to do so -- gpio-ranges . Will you keep adding more and
more new macros into the code with every new/old iteration of this GPIO
block ?

If you were to put that information into DT, where it belongs, the
driver would be much simple(r) and wouldn't grow unnecessarily.

> 3. It is easily reusable in SPL.

And with gpio-ranges it's not ?

>> and submit patch for mainline
>> Linux, it's easy.
> 
> Submitting patches to Linux is easy - but to have them already accepted
> and pulled is not :-).

Linux GPIO maintainer is real friendly.

>>> Also, the dts files which include [1] don't extend original gpio
>>> nodes to have one.
>>>
>>> As it is not available in the Linux kernel, I don't see any good
>>> reason to add the gpio-ranges to U-Boot. The same approach, as
>>> presented here, is used in zynq_gpio.c file (which also uses
>>> DM/DTS).
>>>
>>> Adding per u-boot property - like 'u-boot,mx28-gpio-ranges' is also
>>> less appealing than 24 lines of code, which can be then easily
>>> re-used with OF_PLATDATA (without DM/DTS suport) in SPL (u-boot.sb
>>> to be precise).  
>>
>> It is well possible the zynq DTs predate the gpio-ranges .
> 
> No, it is not.
> 
>> Read the
>> documentation for it at [2] . It even explicitly states it's used for
>> interaction between pincontrol and gpio controllers.
> 
> In those cases where both support it. The i.MX28 Linux GPIO driver is
> NOT supporting gpio-ranges.

See above -- add support for it and it'd even simplify the driver.

>>
>> [2]
>> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/gpio/gpio.txt#L239
>>
>>>>> +struct mxs_gpio_platdata {
>>>>> +	u32 gpio_nr_of_bank_pins[IMX28_GPIO_BANK_NR];
>>>>> +	u32 gpio_base_nr[IMX28_GPIO_BANK_NR];
>>>>> +	u32 ngpio;
>>>>> +};
>>>>> +
>>>>> +extern const struct mxs_gpio_platdata mxs_gpio_def;
>>>>> +#define IMX_GPIO_NR(port, index)
>>>>> (mxs_gpio_def.gpio_base_nr[(port)] + (index))    
>>>>
>>>> So this should be completely unnecessary when using the DM GPIO,
>>>> this was only needed for non-DM-GPIO .  
>>>
>>> This is a compatibility layer - for some reason ALL iMX ports
>>> define it
>>> - i.e. imx8, imx7 - which shall use DM/DTS for gpio from the outset.
>>>
>>> With the in-board code the dm_gpio_* set of functions shall (and
>>> will) be used (as done in opos6ul.c file).  
>>
>> Then use them and drop this.
> 
> I will use the new dm_gpio_* API where applicable. However, to be in
> sync with other iMX ports the IMX_GPIO_NR() shall be also defined.
Are there any users which will actually have to use the old stuff ? If
not, just don't add it.

[...]

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 5/5] ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI conversion)
  2019-06-17 13:23           ` Marek Vasut
@ 2019-06-17 13:41             ` Lukasz Majewski
  2019-06-17 13:46               ` Marek Vasut
  0 siblings, 1 reply; 32+ messages in thread
From: Lukasz Majewski @ 2019-06-17 13:41 UTC (permalink / raw)
  To: u-boot

On Mon, 17 Jun 2019 15:23:55 +0200
Marek Vasut <marex@denx.de> wrote:

> On 6/17/19 2:27 PM, Lukasz Majewski wrote:
> > Hi Marek,
> >   
> >> On 6/17/19 8:49 AM, Lukasz Majewski wrote:  
> >>> Hi Marek,
> >>>     
> >>>> On 6/16/19 12:34 AM, Lukasz Majewski wrote:    
> >>>>> This commit converts mxs_spi driver to support DM/DTS.
> >>>>>
> >>>>> Signed-off-by: Lukasz Majewski <lukma@denx.de>      
> >>>>
> >>>> Is the non-DM part needed for anything ?     
> >>>
> >>> Do you mean the non-DM part of the mxs_gpio driver? Yes, it is
> >>> used by not converted boards.    
> >>
> >> This is a SPI driver though.
> >>  
> >>>> I recall the SPL jumps back
> >>>> to BootROM when loading the U-Boot proper. So if not, drop it.
> >>>>
> >>>> Also, please don't do partial conversion for iMX28 only, do the
> >>>> iMX23 part as well, it cannot be hard.    
> >>>
> >>> Maybe it is not hard, but I cannot test it properly as I don't
> >>> have i.MX23 device. If you are offering your help with testing
> >>> (i.e. you do have the access to i.MX23 device and you will test
> >>> those changes) I can add support for it.
> >>>
> >>> Otherwise, NO, I will not add ANY new untested code.    
> >>
> >> In general, you don't have to add any code, the iMX23/28 SPI IP is
> >> very much the same hardware, DTTO for most of the other blocks. If
> >> there are any differences between iMX23/28, they are already
> >> handled in the existing driver(s).
> >>
> >> Half-way converted drivers in fact increase maintenance burden,
> >> because then we have to deal with two different variants of the
> >> code, instead of only one.  
> > 
> > I cannot agree with this sentence.  
> 
> Do you think maintaining - one DM driver which supports both iMX23 and
> iMX28 - is more burden than maintaining - one driver which supports
> DM, but only for iMX28 and non-DM for iMX23 and iMX28 ? I don't think
> so.
> 
> > The conversion would be done for
> > i.MX28, which is then tested and validated (and clearly stated in
> > the cover letter/commit message that only supported was i.MX28).>
> > If I don't need to adjust common, reused code (which already
> > supports both variants as it is the case with mxs_spi.c), then I
> > don't mind.  
> 
> Well, that is what I said above, you don't.

To make myself clear - If I can reuse the common code (which supports
both imx23 and 28) for DM/DTS conversion then I'm OK with doing so.

If you require me to add untested code specific to i.MX23 - then NO.

> 
> > For more intrusive changes - the driver needs to be tested and
> > validated (by somebody who has the HW for testing).  
> 
> That's up to board maintainers.
> 
> >> That's why I would like to see this practice go
> >> away wherever possible, and in this case it is possible.  
> > 
> > In this particular case it is possible to add support for both as
> > SoC specific changes (i.MX23 vs i.MX28) is performed in common code
> > (e.g. mxs_spi_xfer_dma).  
> 
> Both SPI and DMA blocks are basically the same on iMX23 and iMX28.

If I can reuse the common code, then I'm fine to do it.

> 
> >> If you need someone to test your changes, CC the board maintainers,
> >> that's standard practice.  
> > 
> > As fair as I remember only Angelo and Michael had also interest in
> > testing converted code for i.MX28 based board.
> > 
> > There was NO reply from other people when this (and few others)
> > driver was marked as DEPRECATED.  
> 
> Well, too bad, clearly the interest in this platform is low.

This means that people are using either some old U-Boot version, or
there are a few people who want to refurbish the old HW with new code
(e.g. Michael, Angelo).

> That does not mean we should do sub-par upstream work, does it ?
> 

We shall not add untested code.



Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190617/95dbcd99/attachment.sig>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 5/5] ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI conversion)
  2019-06-17 13:41             ` Lukasz Majewski
@ 2019-06-17 13:46               ` Marek Vasut
  2019-06-17 14:57                 ` Lukasz Majewski
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2019-06-17 13:46 UTC (permalink / raw)
  To: u-boot

On 6/17/19 3:41 PM, Lukasz Majewski wrote:
> On Mon, 17 Jun 2019 15:23:55 +0200
> Marek Vasut <marex@denx.de> wrote:
> 
>> On 6/17/19 2:27 PM, Lukasz Majewski wrote:
>>> Hi Marek,
>>>   
>>>> On 6/17/19 8:49 AM, Lukasz Majewski wrote:  
>>>>> Hi Marek,
>>>>>     
>>>>>> On 6/16/19 12:34 AM, Lukasz Majewski wrote:    
>>>>>>> This commit converts mxs_spi driver to support DM/DTS.
>>>>>>>
>>>>>>> Signed-off-by: Lukasz Majewski <lukma@denx.de>      
>>>>>>
>>>>>> Is the non-DM part needed for anything ?     
>>>>>
>>>>> Do you mean the non-DM part of the mxs_gpio driver? Yes, it is
>>>>> used by not converted boards.    
>>>>
>>>> This is a SPI driver though.
>>>>  
>>>>>> I recall the SPL jumps back
>>>>>> to BootROM when loading the U-Boot proper. So if not, drop it.
>>>>>>
>>>>>> Also, please don't do partial conversion for iMX28 only, do the
>>>>>> iMX23 part as well, it cannot be hard.    
>>>>>
>>>>> Maybe it is not hard, but I cannot test it properly as I don't
>>>>> have i.MX23 device. If you are offering your help with testing
>>>>> (i.e. you do have the access to i.MX23 device and you will test
>>>>> those changes) I can add support for it.
>>>>>
>>>>> Otherwise, NO, I will not add ANY new untested code.    
>>>>
>>>> In general, you don't have to add any code, the iMX23/28 SPI IP is
>>>> very much the same hardware, DTTO for most of the other blocks. If
>>>> there are any differences between iMX23/28, they are already
>>>> handled in the existing driver(s).
>>>>
>>>> Half-way converted drivers in fact increase maintenance burden,
>>>> because then we have to deal with two different variants of the
>>>> code, instead of only one.  
>>>
>>> I cannot agree with this sentence.  
>>
>> Do you think maintaining - one DM driver which supports both iMX23 and
>> iMX28 - is more burden than maintaining - one driver which supports
>> DM, but only for iMX28 and non-DM for iMX23 and iMX28 ? I don't think
>> so.
>>
>>> The conversion would be done for
>>> i.MX28, which is then tested and validated (and clearly stated in
>>> the cover letter/commit message that only supported was i.MX28).>
>>> If I don't need to adjust common, reused code (which already
>>> supports both variants as it is the case with mxs_spi.c), then I
>>> don't mind.  
>>
>> Well, that is what I said above, you don't.
> 
> To make myself clear - If I can reuse the common code (which supports
> both imx23 and 28) for DM/DTS conversion then I'm OK with doing so.
> 
> If you require me to add untested code specific to i.MX23 - then NO.

Yes, you can.

>>
>>> For more intrusive changes - the driver needs to be tested and
>>> validated (by somebody who has the HW for testing).  
>>
>> That's up to board maintainers.
>>
>>>> That's why I would like to see this practice go
>>>> away wherever possible, and in this case it is possible.  
>>>
>>> In this particular case it is possible to add support for both as
>>> SoC specific changes (i.MX23 vs i.MX28) is performed in common code
>>> (e.g. mxs_spi_xfer_dma).  
>>
>> Both SPI and DMA blocks are basically the same on iMX23 and iMX28.
> 
> If I can reuse the common code, then I'm fine to do it.
> 
>>
>>>> If you need someone to test your changes, CC the board maintainers,
>>>> that's standard practice.  
>>>
>>> As fair as I remember only Angelo and Michael had also interest in
>>> testing converted code for i.MX28 based board.
>>>
>>> There was NO reply from other people when this (and few others)
>>> driver was marked as DEPRECATED.  
>>
>> Well, too bad, clearly the interest in this platform is low.
> 
> This means that people are using either some old U-Boot version, or
> there are a few people who want to refurbish the old HW with new code
> (e.g. Michael, Angelo).

Yes

>> That does not mean we should do sub-par upstream work, does it ?
>>
> 
> We shall not add untested code.

You cannot test every single platform in existence. Post patches and let
board maintainers test them ; if they won't, too bad, their platform
might become broken. There's no other way to move forward without
dragging behind a tremendous amount of ancient baggage, and that in turn
is not viable.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 5/5] ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI conversion)
  2019-06-17 13:46               ` Marek Vasut
@ 2019-06-17 14:57                 ` Lukasz Majewski
  2019-06-17 15:00                   ` Marek Vasut
  0 siblings, 1 reply; 32+ messages in thread
From: Lukasz Majewski @ 2019-06-17 14:57 UTC (permalink / raw)
  To: u-boot

Hi Marek,

> On 6/17/19 3:41 PM, Lukasz Majewski wrote:
> > On Mon, 17 Jun 2019 15:23:55 +0200
> > Marek Vasut <marex@denx.de> wrote:
> >   
> >> On 6/17/19 2:27 PM, Lukasz Majewski wrote:  
> >>> Hi Marek,
> >>>     
> >>>> On 6/17/19 8:49 AM, Lukasz Majewski wrote:    
> >>>>> Hi Marek,
> >>>>>       
> >>>>>> On 6/16/19 12:34 AM, Lukasz Majewski wrote:      
> >>>>>>> This commit converts mxs_spi driver to support DM/DTS.
> >>>>>>>
> >>>>>>> Signed-off-by: Lukasz Majewski <lukma@denx.de>        
> >>>>>>
> >>>>>> Is the non-DM part needed for anything ?       
> >>>>>
> >>>>> Do you mean the non-DM part of the mxs_gpio driver? Yes, it is
> >>>>> used by not converted boards.      
> >>>>
> >>>> This is a SPI driver though.
> >>>>    
> >>>>>> I recall the SPL jumps back
> >>>>>> to BootROM when loading the U-Boot proper. So if not, drop it.
> >>>>>>
> >>>>>> Also, please don't do partial conversion for iMX28 only, do the
> >>>>>> iMX23 part as well, it cannot be hard.      
> >>>>>
> >>>>> Maybe it is not hard, but I cannot test it properly as I don't
> >>>>> have i.MX23 device. If you are offering your help with testing
> >>>>> (i.e. you do have the access to i.MX23 device and you will test
> >>>>> those changes) I can add support for it.
> >>>>>
> >>>>> Otherwise, NO, I will not add ANY new untested code.      
> >>>>
> >>>> In general, you don't have to add any code, the iMX23/28 SPI IP
> >>>> is very much the same hardware, DTTO for most of the other
> >>>> blocks. If there are any differences between iMX23/28, they are
> >>>> already handled in the existing driver(s).
> >>>>
> >>>> Half-way converted drivers in fact increase maintenance burden,
> >>>> because then we have to deal with two different variants of the
> >>>> code, instead of only one.    
> >>>
> >>> I cannot agree with this sentence.    
> >>
> >> Do you think maintaining - one DM driver which supports both iMX23
> >> and iMX28 - is more burden than maintaining - one driver which
> >> supports DM, but only for iMX28 and non-DM for iMX23 and iMX28 ? I
> >> don't think so.
> >>  
> >>> The conversion would be done for
> >>> i.MX28, which is then tested and validated (and clearly stated in
> >>> the cover letter/commit message that only supported was i.MX28).>
> >>> If I don't need to adjust common, reused code (which already
> >>> supports both variants as it is the case with mxs_spi.c), then I
> >>> don't mind.    
> >>
> >> Well, that is what I said above, you don't.  
> > 
> > To make myself clear - If I can reuse the common code (which
> > supports both imx23 and 28) for DM/DTS conversion then I'm OK with
> > doing so.
> > 
> > If you require me to add untested code specific to i.MX23 - then
> > NO.  
> 
> Yes, you can.

If possible, by reusing the old, common working code, I can add this to
the converted driver.

But I will not any new untested code.

> 
> >>  
> >>> For more intrusive changes - the driver needs to be tested and
> >>> validated (by somebody who has the HW for testing).    
> >>
> >> That's up to board maintainers.
> >>  
> >>>> That's why I would like to see this practice go
> >>>> away wherever possible, and in this case it is possible.    
> >>>
> >>> In this particular case it is possible to add support for both as
> >>> SoC specific changes (i.MX23 vs i.MX28) is performed in common
> >>> code (e.g. mxs_spi_xfer_dma).    
> >>
> >> Both SPI and DMA blocks are basically the same on iMX23 and
> >> iMX28.  
> > 
> > If I can reuse the common code, then I'm fine to do it.
> >   
> >>  
> >>>> If you need someone to test your changes, CC the board
> >>>> maintainers, that's standard practice.    
> >>>
> >>> As fair as I remember only Angelo and Michael had also interest in
> >>> testing converted code for i.MX28 based board.
> >>>
> >>> There was NO reply from other people when this (and few others)
> >>> driver was marked as DEPRECATED.    
> >>
> >> Well, too bad, clearly the interest in this platform is low.  
> > 
> > This means that people are using either some old U-Boot version, or
> > there are a few people who want to refurbish the old HW with new
> > code (e.g. Michael, Angelo).  
> 
> Yes
> 
> >> That does not mean we should do sub-par upstream work, does it ?
> >>  
> > 
> > We shall not add untested code.  
> 
> You cannot test every single platform in existence. Post patches and
> let board maintainers test them ; if they won't, too bad, their
> platform might become broken. There's no other way to move forward
> without dragging behind a tremendous amount of ancient baggage, and
> that in turn is not viable.
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190617/08f590ec/attachment.sig>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 5/5] ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI conversion)
  2019-06-17 14:57                 ` Lukasz Majewski
@ 2019-06-17 15:00                   ` Marek Vasut
  0 siblings, 0 replies; 32+ messages in thread
From: Marek Vasut @ 2019-06-17 15:00 UTC (permalink / raw)
  To: u-boot

On 6/17/19 4:57 PM, Lukasz Majewski wrote:
> Hi Marek,
> 
>> On 6/17/19 3:41 PM, Lukasz Majewski wrote:
>>> On Mon, 17 Jun 2019 15:23:55 +0200
>>> Marek Vasut <marex@denx.de> wrote:
>>>   
>>>> On 6/17/19 2:27 PM, Lukasz Majewski wrote:  
>>>>> Hi Marek,
>>>>>     
>>>>>> On 6/17/19 8:49 AM, Lukasz Majewski wrote:    
>>>>>>> Hi Marek,
>>>>>>>       
>>>>>>>> On 6/16/19 12:34 AM, Lukasz Majewski wrote:      
>>>>>>>>> This commit converts mxs_spi driver to support DM/DTS.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Lukasz Majewski <lukma@denx.de>        
>>>>>>>>
>>>>>>>> Is the non-DM part needed for anything ?       
>>>>>>>
>>>>>>> Do you mean the non-DM part of the mxs_gpio driver? Yes, it is
>>>>>>> used by not converted boards.      
>>>>>>
>>>>>> This is a SPI driver though.
>>>>>>    
>>>>>>>> I recall the SPL jumps back
>>>>>>>> to BootROM when loading the U-Boot proper. So if not, drop it.
>>>>>>>>
>>>>>>>> Also, please don't do partial conversion for iMX28 only, do the
>>>>>>>> iMX23 part as well, it cannot be hard.      
>>>>>>>
>>>>>>> Maybe it is not hard, but I cannot test it properly as I don't
>>>>>>> have i.MX23 device. If you are offering your help with testing
>>>>>>> (i.e. you do have the access to i.MX23 device and you will test
>>>>>>> those changes) I can add support for it.
>>>>>>>
>>>>>>> Otherwise, NO, I will not add ANY new untested code.      
>>>>>>
>>>>>> In general, you don't have to add any code, the iMX23/28 SPI IP
>>>>>> is very much the same hardware, DTTO for most of the other
>>>>>> blocks. If there are any differences between iMX23/28, they are
>>>>>> already handled in the existing driver(s).
>>>>>>
>>>>>> Half-way converted drivers in fact increase maintenance burden,
>>>>>> because then we have to deal with two different variants of the
>>>>>> code, instead of only one.    
>>>>>
>>>>> I cannot agree with this sentence.    
>>>>
>>>> Do you think maintaining - one DM driver which supports both iMX23
>>>> and iMX28 - is more burden than maintaining - one driver which
>>>> supports DM, but only for iMX28 and non-DM for iMX23 and iMX28 ? I
>>>> don't think so.
>>>>  
>>>>> The conversion would be done for
>>>>> i.MX28, which is then tested and validated (and clearly stated in
>>>>> the cover letter/commit message that only supported was i.MX28).>
>>>>> If I don't need to adjust common, reused code (which already
>>>>> supports both variants as it is the case with mxs_spi.c), then I
>>>>> don't mind.    
>>>>
>>>> Well, that is what I said above, you don't.  
>>>
>>> To make myself clear - If I can reuse the common code (which
>>> supports both imx23 and 28) for DM/DTS conversion then I'm OK with
>>> doing so.
>>>
>>> If you require me to add untested code specific to i.MX23 - then
>>> NO.  
>>
>> Yes, you can.
> 
> If possible, by reusing the old, common working code, I can add this to
> the converted driver.

Again, yes, you can. All the code is already in the driver.

[...]

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 3/5] ARM: dm: mxs: gpio: Add support for DM/DTS in the mxs_gpio.c driver (DM_GPIO)
  2019-06-17 13:34           ` Marek Vasut
@ 2019-06-18  6:57             ` Lukasz Majewski
  2019-06-18 10:33               ` Marek Vasut
  0 siblings, 1 reply; 32+ messages in thread
From: Lukasz Majewski @ 2019-06-18  6:57 UTC (permalink / raw)
  To: u-boot

Hi Marek,

> On 6/17/19 3:01 PM, Lukasz Majewski wrote:
> > Hi Marek,
> >   
> >> On 6/17/19 10:37 AM, Lukasz Majewski wrote:  
> >>> Hi Marek,
> >>>     
> >>>> On 6/16/19 12:34 AM, Lukasz Majewski wrote:    
> >>>>> This commit      
> >>>>
> >>>> This is not a commit, it's a change. It only becomes a commit
> >>>> when applied.
> >>>>    
> >>>>> adds support for DM in the mxs_gpio.c driver when DM_GPIO
> >>>>> is enabled in Kconfig.      
> >>>>
> >>>> But this also adds support for DT probing, which is orthogonal to
> >>>> DM support, yet it's not documented in the commit message.    
> >>>
> >>> Ok. Will rewrite the commit message to reflect the changes in the
> >>> commit.
> >>>     
> >>>>    
> >>>>> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> >>>>>
> >>>>> ---
> >>>>>
> >>>>> Changes in v3:
> >>>>> - Set more apropriate tags
> >>>>>
> >>>>> Changes in v2:
> >>>>> - Use #if !CONFIG_IS_ENABLED(DM_GPIO) instead of plain #ifdef
> >>>>> CONFIG_DM_GPIO
> >>>>>
> >>>>>  arch/arm/include/asm/arch-mxs/gpio.h |  28 +++++++
> >>>>>  drivers/gpio/mxs_gpio.c              | 149
> >>>>> +++++++++++++++++++++++++++++++++++ 2 files changed, 177
> >>>>> insertions(+)
> >>>>>
> >>>>> diff --git a/arch/arm/include/asm/arch-mxs/gpio.h
> >>>>> b/arch/arm/include/asm/arch-mxs/gpio.h index
> >>>>> 34fa421945..0c152e25e2 100644 ---
> >>>>> a/arch/arm/include/asm/arch-mxs/gpio.h +++
> >>>>> b/arch/arm/include/asm/arch-mxs/gpio.h @@ -15,4 +15,32 @@ void
> >>>>> mxs_gpio_init(void); inline void mxs_gpio_init(void) {}
> >>>>>  #endif
> >>>>>  
> >>>>> +#if defined(CONFIG_MX28) && CONFIG_IS_ENABLED(DM_GPIO)
> >>>>> +/*
> >>>>> + * According to i.MX28 Reference Manual:
> >>>>> + * 'i.MX28 Applications Processor Reference Manual, Rev. 1,
> >>>>> 2010'
> >>>>> + * The i.MX28 has following number of GPIOs available:
> >>>>> + * Bank 0: 0-28 -> 29 PINS
> >>>>> + * Bank 1: 0-31 -> 32 PINS
> >>>>> + * Bank 2: 0-27 -> 28 PINS
> >>>>> + * Bank 3: 0-30 -> 31 PINS
> >>>>> + * Bank 4: 0-20 -> 21 PINS
> >>>>> + */
> >>>>> +#define IMX28_GPIO_BANK0_PIN_NR 29
> >>>>> +#define IMX28_GPIO_BANK1_PIN_NR 32
> >>>>> +#define IMX28_GPIO_BANK2_PIN_NR 28
> >>>>> +#define IMX28_GPIO_BANK3_PIN_NR 31
> >>>>> +#define IMX28_GPIO_BANK4_PIN_NR 21
> >>>>> +#define IMX28_GPIO_BANK_NR 5      
> >>>>
> >>>> Please make a complete conversion, not partial one.
> >>>> You want to use gpio-ranges in DT and then parse the size of each
> >>>> GPIO bank from those gpio-ranges , not hard-code it into the
> >>>> driver.    
> >>>
> >>> I would have used the gpio-ranges, but the original Linux code [1]
> >>> (imx28.dtsi) doesn't define them.    
> >>
> >> You can add them to imx28-u-boot.dtsi ,   
> > 
> > No, IMHO this is not the best solution. My point is:
> > 
> > 1. i.MX28 driver in Linux is stable and it works (without
> > gpio-ranges). I do not have any interest in fixing code which is
> > already working. If that were new driver then no issue to use
> > gpio-ranges from the outset. Also if Linux kernel driver would
> > support it - then also no problems with adding it.  
> 
> Linux is continuously improving, so is U-Boot, code is being
> constantly rewritten and improved. So this isn't really a convincing
> argument.
> 
> > 2. The proposed code is small - only 24 LOC and doesn't require any
> > extra parsing of DTS (including pinctrl driver's properties).
> > 
> > Why shall I make the driver more verbose if I can avoid it?  
> 
> It also adds churn into the driver which does not have to be there. In
> fact, DT is a hardware description, it should describe hardware and it
> has facilities to do so -- gpio-ranges. 
> Will you keep adding more and
> more new macros into the code with every new/old iteration of this
> GPIO block ?
> 
> If you were to put that information into DT, where it belongs, the
> driver would be much simple(r) and wouldn't grow unnecessarily.
> 
> > 3. It is easily reusable in SPL.  
> 
> And with gpio-ranges it's not ?
> 
> >> and submit patch for mainline
> >> Linux, it's easy.  
> > 
> > Submitting patches to Linux is easy - but to have them already
> > accepted and pulled is not :-).  
> 
> Linux GPIO maintainer is real friendly.
 
> >>> Also, the dts files which include [1] don't extend original gpio
> >>> nodes to have one.
> >>>
> >>> As it is not available in the Linux kernel, I don't see any good
> >>> reason to add the gpio-ranges to U-Boot. The same approach, as
> >>> presented here, is used in zynq_gpio.c file (which also uses
> >>> DM/DTS).
> >>>
> >>> Adding per u-boot property - like 'u-boot,mx28-gpio-ranges' is
> >>> also less appealing than 24 lines of code, which can be then
> >>> easily re-used with OF_PLATDATA (without DM/DTS suport) in SPL
> >>> (u-boot.sb to be precise).    
> >>
> >> It is well possible the zynq DTs predate the gpio-ranges .  
> > 
> > No, it is not.
> >   
> >> Read the
> >> documentation for it at [2] . It even explicitly states it's used
> >> for interaction between pincontrol and gpio controllers.  
> > 
> > In those cases where both support it. The i.MX28 Linux GPIO driver
> > is NOT supporting gpio-ranges.  
> 
> See above -- add support for it and it'd even simplify the driver.
> 
> >>
> >> [2]
> >> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/gpio/gpio.txt#L239
> >>  
> >>>>> +struct mxs_gpio_platdata {
> >>>>> +	u32 gpio_nr_of_bank_pins[IMX28_GPIO_BANK_NR];
> >>>>> +	u32 gpio_base_nr[IMX28_GPIO_BANK_NR];
> >>>>> +	u32 ngpio;
> >>>>> +};
> >>>>> +
> >>>>> +extern const struct mxs_gpio_platdata mxs_gpio_def;
> >>>>> +#define IMX_GPIO_NR(port, index)
> >>>>> (mxs_gpio_def.gpio_base_nr[(port)] + (index))      
> >>>>
> >>>> So this should be completely unnecessary when using the DM GPIO,
> >>>> this was only needed for non-DM-GPIO .    
> >>>
> >>> This is a compatibility layer - for some reason ALL iMX ports
> >>> define it
> >>> - i.e. imx8, imx7 - which shall use DM/DTS for gpio from the
> >>> outset.
> >>>
> >>> With the in-board code the dm_gpio_* set of functions shall (and
> >>> will) be used (as done in opos6ul.c file).    
> >>
> >> Then use them and drop this.  
> > 
> > I will use the new dm_gpio_* API where applicable. However, to be in
> > sync with other iMX ports the IMX_GPIO_NR() shall be also defined.  
> Are there any users which will actually have to use the old stuff ? If
> not, just don't add it.
> 
> [...]
> 

To have a consensus regarding the gpio-ranges and mxs_gpio - here is
my proposition:

1. I can provide "gpio-ranges" properties to imx28-u-boot.dtsi

With support similar to one from drivers/gpio/gpio-rcar.c, which only
extracts the info regarding pin mapping, but omits the phandle for
pinmux (pinmux will not be supported).

2. I will not upstream those properties to Linux (and adjust the
working mxs gpio driver in any way) - they would be only U-Boot specific

3. The IMX_GPIO_NR() macro will not be defined for i.MX28 GPIO.

4. Considering point 3 - all converted boards will have to use dm_gpio*
API.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190618/1ede9e67/attachment.sig>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 3/5] ARM: dm: mxs: gpio: Add support for DM/DTS in the mxs_gpio.c driver (DM_GPIO)
  2019-06-18  6:57             ` Lukasz Majewski
@ 2019-06-18 10:33               ` Marek Vasut
  2019-06-18 10:56                 ` Lukasz Majewski
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2019-06-18 10:33 UTC (permalink / raw)
  To: u-boot

On 6/18/19 8:57 AM, Lukasz Majewski wrote:

[...]

> To have a consensus regarding the gpio-ranges and mxs_gpio - here is
> my proposition:
> 
> 1. I can provide "gpio-ranges" properties to imx28-u-boot.dtsi
> 
> With support similar to one from drivers/gpio/gpio-rcar.c, which only
> extracts the info regarding pin mapping, but omits the phandle for
> pinmux (pinmux will not be supported).
> 
> 2. I will not upstream those properties to Linux (and adjust the
> working mxs gpio driver in any way) - they would be only U-Boot specific

Why not ?

> 3. The IMX_GPIO_NR() macro will not be defined for i.MX28 GPIO.
> 
> 4. Considering point 3 - all converted boards will have to use dm_gpio*
> API.

That's good.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 3/5] ARM: dm: mxs: gpio: Add support for DM/DTS in the mxs_gpio.c driver (DM_GPIO)
  2019-06-18 10:33               ` Marek Vasut
@ 2019-06-18 10:56                 ` Lukasz Majewski
  2019-06-18 11:04                   ` Marek Vasut
  0 siblings, 1 reply; 32+ messages in thread
From: Lukasz Majewski @ 2019-06-18 10:56 UTC (permalink / raw)
  To: u-boot

Hi Marek,

> On 6/18/19 8:57 AM, Lukasz Majewski wrote:
> 
> [...]
> 
> > To have a consensus regarding the gpio-ranges and mxs_gpio - here is
> > my proposition:
> > 
> > 1. I can provide "gpio-ranges" properties to imx28-u-boot.dtsi
> > 
> > With support similar to one from drivers/gpio/gpio-rcar.c, which
> > only extracts the info regarding pin mapping, but omits the phandle
> > for pinmux (pinmux will not be supported).
> > 
> > 2. I will not upstream those properties to Linux (and adjust the
> > working mxs gpio driver in any way) - they would be only U-Boot
> > specific  
> 
> Why not ?

Because I do not have any interest in doing so.

Do we have agreement?

> 
> > 3. The IMX_GPIO_NR() macro will not be defined for i.MX28 GPIO.
> > 
> > 4. Considering point 3 - all converted boards will have to use
> > dm_gpio* API.  
> 
> That's good.
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190618/1f03a729/attachment.sig>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH v3 3/5] ARM: dm: mxs: gpio: Add support for DM/DTS in the mxs_gpio.c driver (DM_GPIO)
  2019-06-18 10:56                 ` Lukasz Majewski
@ 2019-06-18 11:04                   ` Marek Vasut
  0 siblings, 0 replies; 32+ messages in thread
From: Marek Vasut @ 2019-06-18 11:04 UTC (permalink / raw)
  To: u-boot

On 6/18/19 12:56 PM, Lukasz Majewski wrote:
> Hi Marek,
> 
>> On 6/18/19 8:57 AM, Lukasz Majewski wrote:
>>
>> [...]
>>
>>> To have a consensus regarding the gpio-ranges and mxs_gpio - here is
>>> my proposition:
>>>
>>> 1. I can provide "gpio-ranges" properties to imx28-u-boot.dtsi
>>>
>>> With support similar to one from drivers/gpio/gpio-rcar.c, which
>>> only extracts the info regarding pin mapping, but omits the phandle
>>> for pinmux (pinmux will not be supported).
>>>
>>> 2. I will not upstream those properties to Linux (and adjust the
>>> working mxs gpio driver in any way) - they would be only U-Boot
>>> specific  
>>
>> Why not ?
> 
> Because I do not have any interest in doing so.

That's unfortunate.

> Do we have agreement?
I think so, let's see how this pans out in v4.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2019-06-18 11:04 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-15 22:34 [U-Boot] [PATCH v3 0/5] DM: Convert i.MX28 gpio, pinmux, spi and eth drivers to DM/DTS Lukasz Majewski
2019-06-15 22:34 ` [U-Boot] [PATCH v3 1/5] ARM: dts: imx: Copy imx28 device tree related files from Linux kernel (v5.1.9) Lukasz Majewski
2019-06-15 22:43   ` Marek Vasut
2019-06-17  6:57     ` Lukasz Majewski
2019-06-17 10:13       ` Marek Vasut
2019-06-15 22:34 ` [U-Boot] [PATCH v3 2/5] ARM: imx: net: Enable support for i.MX28 DM_ETH in the fec_mxc.c driver Lukasz Majewski
2019-06-15 22:43   ` Marek Vasut
2019-06-17  6:57     ` Lukasz Majewski
2019-06-15 22:34 ` [U-Boot] [PATCH v3 3/5] ARM: dm: mxs: gpio: Add support for DM/DTS in the mxs_gpio.c driver (DM_GPIO) Lukasz Majewski
2019-06-15 22:53   ` Marek Vasut
2019-06-17  8:37     ` Lukasz Majewski
2019-06-17 10:20       ` Marek Vasut
2019-06-17 13:01         ` Lukasz Majewski
2019-06-17 13:34           ` Marek Vasut
2019-06-18  6:57             ` Lukasz Majewski
2019-06-18 10:33               ` Marek Vasut
2019-06-18 10:56                 ` Lukasz Majewski
2019-06-18 11:04                   ` Marek Vasut
2019-06-15 22:34 ` [U-Boot] [PATCH v3 4/5] ARM: imx: pinctrl: Add support for i.MX28 mxs pinctrl driver Lukasz Majewski
2019-06-15 23:00   ` Marek Vasut
2019-06-17  7:43     ` Lukasz Majewski
2019-06-17 10:23       ` Marek Vasut
2019-06-15 22:34 ` [U-Boot] [PATCH v3 5/5] ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI conversion) Lukasz Majewski
2019-06-15 23:02   ` Marek Vasut
2019-06-17  6:49     ` Lukasz Majewski
2019-06-17 10:06       ` Marek Vasut
2019-06-17 12:27         ` Lukasz Majewski
2019-06-17 13:23           ` Marek Vasut
2019-06-17 13:41             ` Lukasz Majewski
2019-06-17 13:46               ` Marek Vasut
2019-06-17 14:57                 ` Lukasz Majewski
2019-06-17 15:00                   ` Marek Vasut

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.