| Message ID | 1438183144-11471-1-git-send-email-sven@open-mesh.com |
|---|---|
| State | Superseded |
| Headers | show |
On 29/07/2015 17:19, Sven Eckelmann wrote: > The pinctrl-rt2880 code doesn't support multiple functions with the same > name. This will result in incorrect pinmux configuration. > > The MT7621 uses 2 bit wide configuration of the sdhci, spi, mdio, pcie, > wdt, uart2 and uart3. But the code only changed a single bit for uart3, > uart2 and mdio. Also the order of uart2 and uart3 group was exchanged. > > The spi "nand" settings was also reserved only 7 PINs instead of 8 as the > spi "spi" setting. > i stumbled across this problem on mt7628/88 last few days. the pinctrl code was written for a core with 1 bit / function and then grew with new socs. i'll try to fix this in the pinctrl driver the next few days rather than have redundant function names. John > Signed-off-by: Sven Eckelmann <sven@open-mesh.com> > --- > target/linux/ramips/dts/mt7621.dtsi | 14 +++---- > .../0012-MIPS-ralink-add-MT7621-support.patch | 45 +++++++++++++++------- > 2 files changed, 38 insertions(+), 21 deletions(-) > > diff --git a/target/linux/ramips/dts/mt7621.dtsi b/target/linux/ramips/dts/mt7621.dtsi > index 53b215f40f10..f09ec3e5b694 100644 > --- a/target/linux/ramips/dts/mt7621.dtsi > +++ b/target/linux/ramips/dts/mt7621.dtsi > @@ -130,31 +130,31 @@ > uart1_pins: uart1 { > uart1 { > ralink,group = "uart1"; > - ralink,function = "uart"; > + ralink,function = "uart1"; > }; > }; > uart2_pins: uart2 { > uart2 { > ralink,group = "uart2"; > - ralink,function = "uart"; > + ralink,function = "uart2"; > }; > }; > uart3_pins: uart3 { > uart3 { > ralink,group = "uart3"; > - ralink,function = "uart"; > + ralink,function = "uart3"; > }; > }; > rgmii1_pins: rgmii1 { > rgmii1 { > ralink,group = "rgmii1"; > - ralink,function = "rgmii"; > + ralink,function = "rgmii1"; > }; > }; > rgmii2_pins: rgmii2 { > rgmii2 { > ralink,group = "rgmii2"; > - ralink,function = "rgmii"; > + ralink,function = "rgmii2"; > }; > }; > mdio_pins: mdio { > @@ -172,11 +172,11 @@ > nand_pins: nand { > spi-nand { > ralink,group = "spi"; > - ralink,function = "nand"; > + ralink,function = "nand1"; > }; > sdhci-nand { > ralink,group = "sdhci"; > - ralink,function = "nand"; > + ralink,function = "nand2"; > }; > }; > sdhci_pins: sdhci { > diff --git a/target/linux/ramips/patches-3.18/0012-MIPS-ralink-add-MT7621-support.patch b/target/linux/ramips/patches-3.18/0012-MIPS-ralink-add-MT7621-support.patch > index 771de12f171d..23d32681bf77 100644 > --- a/target/linux/ramips/patches-3.18/0012-MIPS-ralink-add-MT7621-support.patch > +++ b/target/linux/ramips/patches-3.18/0012-MIPS-ralink-add-MT7621-support.patch > @@ -520,7 +520,7 @@ Signed-off-by: John Crispin <blogic@openwrt.org> > +} > --- /dev/null > +++ b/arch/mips/ralink/mt7621.c > -@@ -0,0 +1,192 @@ > +@@ -0,0 +1,209 @@ > +/* > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as published > @@ -555,8 +555,12 @@ Signed-off-by: John Crispin <blogic@openwrt.org> > + > +#define MT7621_GPIO_MODE_UART1 1 > +#define MT7621_GPIO_MODE_I2C 2 > -+#define MT7621_GPIO_MODE_UART2 3 > -+#define MT7621_GPIO_MODE_UART3 5 > ++#define MT7621_GPIO_MODE_UART3_MASK 0x3 > ++#define MT7621_GPIO_MODE_UART3_SHIFT 3 > ++#define MT7621_GPIO_MODE_UART3_GPIO 1 > ++#define MT7621_GPIO_MODE_UART2_MASK 0x3 > ++#define MT7621_GPIO_MODE_UART2_SHIFT 5 > ++#define MT7621_GPIO_MODE_UART2_GPIO 1 > +#define MT7621_GPIO_MODE_JTAG 7 > +#define MT7621_GPIO_MODE_WDT_MASK 0x3 > +#define MT7621_GPIO_MODE_WDT_SHIFT 8 > @@ -566,7 +570,9 @@ Signed-off-by: John Crispin <blogic@openwrt.org> > +#define MT7621_GPIO_MODE_PCIE_MASK 0x3 > +#define MT7621_GPIO_MODE_PCIE_SHIFT 10 > +#define MT7621_GPIO_MODE_PCIE_GPIO 1 > -+#define MT7621_GPIO_MODE_MDIO 12 > ++#define MT7621_GPIO_MODE_MDIO_MASK 0x3 > ++#define MT7621_GPIO_MODE_MDIO_SHIFT 12 > ++#define MT7621_GPIO_MODE_MDIO_GPIO 1 > +#define MT7621_GPIO_MODE_RGMII1 14 > +#define MT7621_GPIO_MODE_RGMII2 15 > +#define MT7621_GPIO_MODE_SPI_MASK 0x3 > @@ -576,10 +582,18 @@ Signed-off-by: John Crispin <blogic@openwrt.org> > +#define MT7621_GPIO_MODE_SDHCI_SHIFT 18 > +#define MT7621_GPIO_MODE_SDHCI_GPIO 1 > + > -+static struct rt2880_pmx_func uart1_grp[] = { FUNC("uart", 0, 1, 2) }; > ++static struct rt2880_pmx_func uart1_grp[] = { FUNC("uart1", 0, 1, 2) }; > +static struct rt2880_pmx_func i2c_grp[] = { FUNC("i2c", 0, 3, 2) }; > -+static struct rt2880_pmx_func uart3_grp[] = { FUNC("uart", 0, 5, 4) }; > -+static struct rt2880_pmx_func uart2_grp[] = { FUNC("uart", 0, 9, 4) }; > ++static struct rt2880_pmx_func uart3_grp[] = { > ++ FUNC("uart3", 0, 5, 4), > ++ FUNC("i2s", 2, 5, 4), > ++ FUNC("spdif3", 3, 5, 4), > ++}; > ++static struct rt2880_pmx_func uart2_grp[] = { > ++ FUNC("uart2", 0, 9, 4), > ++ FUNC("pcm", 2, 9, 4), > ++ FUNC("spdif2", 3, 9, 4), > ++}; > +static struct rt2880_pmx_func jtag_grp[] = { FUNC("jtag", 0, 13, 5) }; > +static struct rt2880_pmx_func wdt_grp[] = { > + FUNC("wdt rst", 0, 18, 1), > @@ -590,28 +604,31 @@ Signed-off-by: John Crispin <blogic@openwrt.org> > + FUNC("pcie refclk", MT7621_GPIO_MODE_PCIE_REF, 19, 1) > +}; > +static struct rt2880_pmx_func mdio_grp[] = { FUNC("mdio", 0, 20, 2) }; > -+static struct rt2880_pmx_func rgmii2_grp[] = { FUNC("rgmii", 0, 22, 12) }; > ++static struct rt2880_pmx_func rgmii2_grp[] = { FUNC("rgmii2", 0, 22, 12) }; > +static struct rt2880_pmx_func spi_grp[] = { > + FUNC("spi", 0, 34, 7), > -+ FUNC("nand", 2, 34, 8), > ++ FUNC("nand1", 2, 34, 7), > +}; > +static struct rt2880_pmx_func sdhci_grp[] = { > + FUNC("sdhci", 0, 41, 8), > -+ FUNC("nand", 2, 41, 8), > ++ FUNC("nand2", 2, 41, 8), > +}; > -+static struct rt2880_pmx_func rgmii1_grp[] = { FUNC("rgmii", 0, 49, 12) }; > ++static struct rt2880_pmx_func rgmii1_grp[] = { FUNC("rgmii1", 0, 49, 12) }; > + > +static struct rt2880_pmx_group mt7621_pinmux_data[] = { > + GRP("uart1", uart1_grp, 1, MT7621_GPIO_MODE_UART1), > + GRP("i2c", i2c_grp, 1, MT7621_GPIO_MODE_I2C), > -+ GRP("uart3", uart2_grp, 1, MT7621_GPIO_MODE_UART2), > -+ GRP("uart2", uart3_grp, 1, MT7621_GPIO_MODE_UART3), > ++ GRP_G("uart3", uart3_grp, MT7621_GPIO_MODE_UART3_MASK, > ++ MT7621_GPIO_MODE_UART3_GPIO, MT7621_GPIO_MODE_UART3_SHIFT), > ++ GRP_G("uart2", uart2_grp, MT7621_GPIO_MODE_UART2_MASK, > ++ MT7621_GPIO_MODE_UART2_GPIO, MT7621_GPIO_MODE_UART2_SHIFT), > + GRP("jtag", jtag_grp, 1, MT7621_GPIO_MODE_JTAG), > + GRP_G("wdt", wdt_grp, MT7621_GPIO_MODE_WDT_MASK, > + MT7621_GPIO_MODE_WDT_GPIO, MT7621_GPIO_MODE_WDT_SHIFT), > + GRP_G("pcie", pcie_rst_grp, MT7621_GPIO_MODE_PCIE_MASK, > + MT7621_GPIO_MODE_PCIE_GPIO, MT7621_GPIO_MODE_PCIE_SHIFT), > -+ GRP("mdio", mdio_grp, 1, MT7621_GPIO_MODE_MDIO), > ++ GRP_G("mdio", mdio_grp, MT7621_GPIO_MODE_MDIO_MASK, > ++ MT7621_GPIO_MODE_MDIO_GPIO, MT7621_GPIO_MODE_MDIO_SHIFT), > + GRP("rgmii2", rgmii2_grp, 1, MT7621_GPIO_MODE_RGMII2), > + GRP_G("spi", spi_grp, MT7621_GPIO_MODE_SPI_MASK, > + MT7621_GPIO_MODE_SPI_GPIO, MT7621_GPIO_MODE_SPI_SHIFT), >
On Thursday 30 July 2015 14:23:06 John Crispin wrote: > > On 29/07/2015 17:19, Sven Eckelmann wrote: > > The pinctrl-rt2880 code doesn't support multiple functions with the same > > name. This will result in incorrect pinmux configuration. > > > > The MT7621 uses 2 bit wide configuration of the sdhci, spi, mdio, pcie, > > wdt, uart2 and uart3. But the code only changed a single bit for uart3, > > uart2 and mdio. Also the order of uart2 and uart3 group was exchanged. > > > > The spi "nand" settings was also reserved only 7 PINs instead of 8 as the > > spi "spi" setting. > > > > i stumbled across this problem on mt7628/88 last few days. the pinctrl > code was written for a core with 1 bit / function and then grew with new > socs. i'll try to fix this in the pinctrl driver the next few days > rather than have redundant function names. What about the other problems like wrong settings width, wrong settings position and missing settings? Should I resend them? Btw. keep in mind that the functions have conflicting settings (different PINs). Kind regards, Sven
On Friday 31 July 2015 10:33:59 John Crispin wrote: [...] > > What about the other problems like wrong settings width, wrong settings > > position and missing settings? Should I resend them? > > which patch exactly ? I have now submitted the patch but this time split into separate patches [1,2,3] (these are the ones with the "other problems"). The last one [4] is the patch which you want to drop and instead modify the pinctrl-rt2880. Kind regards, Sven [1] http://patchwork.ozlabs.org/patch/502439/ [2] http://patchwork.ozlabs.org/patch/502440/ [3] http://patchwork.ozlabs.org/patch/502442/ [4] http://patchwork.ozlabs.org/patch/502444/
On 31/07/2015 11:23, Sven Eckelmann wrote: > On Friday 31 July 2015 10:33:59 John Crispin wrote: > [...] >>> What about the other problems like wrong settings width, wrong settings >>> position and missing settings? Should I resend them? >> >> which patch exactly ? > > I have now submitted the patch but this time split into separate > patches [1,2,3] (these are the ones with the "other problems"). The > last one [4] is the patch which you want to drop and instead modify > the pinctrl-rt2880. > thanks, i'll add them to my tree in a sec and push them later on with a pile of other mt7628 patches which i am testing just now. > Kind regards, > Sven > > [1] http://patchwork.ozlabs.org/patch/502439/ > [2] http://patchwork.ozlabs.org/patch/502440/ > [3] http://patchwork.ozlabs.org/patch/502442/ > [4] http://patchwork.ozlabs.org/patch/502444/ > > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel >
diff --git a/target/linux/ramips/dts/mt7621.dtsi b/target/linux/ramips/dts/mt7621.dtsi index 53b215f40f10..f09ec3e5b694 100644 --- a/target/linux/ramips/dts/mt7621.dtsi +++ b/target/linux/ramips/dts/mt7621.dtsi @@ -130,31 +130,31 @@ uart1_pins: uart1 { uart1 { ralink,group = "uart1"; - ralink,function = "uart"; + ralink,function = "uart1"; }; }; uart2_pins: uart2 { uart2 { ralink,group = "uart2"; - ralink,function = "uart"; + ralink,function = "uart2"; }; }; uart3_pins: uart3 { uart3 { ralink,group = "uart3"; - ralink,function = "uart"; + ralink,function = "uart3"; }; }; rgmii1_pins: rgmii1 { rgmii1 { ralink,group = "rgmii1"; - ralink,function = "rgmii"; + ralink,function = "rgmii1"; }; }; rgmii2_pins: rgmii2 { rgmii2 { ralink,group = "rgmii2"; - ralink,function = "rgmii"; + ralink,function = "rgmii2"; }; }; mdio_pins: mdio { @@ -172,11 +172,11 @@ nand_pins: nand { spi-nand { ralink,group = "spi"; - ralink,function = "nand"; + ralink,function = "nand1"; }; sdhci-nand { ralink,group = "sdhci"; - ralink,function = "nand"; + ralink,function = "nand2"; }; }; sdhci_pins: sdhci { diff --git a/target/linux/ramips/patches-3.18/0012-MIPS-ralink-add-MT7621-support.patch b/target/linux/ramips/patches-3.18/0012-MIPS-ralink-add-MT7621-support.patch index 771de12f171d..23d32681bf77 100644 --- a/target/linux/ramips/patches-3.18/0012-MIPS-ralink-add-MT7621-support.patch +++ b/target/linux/ramips/patches-3.18/0012-MIPS-ralink-add-MT7621-support.patch @@ -520,7 +520,7 @@ Signed-off-by: John Crispin <blogic@openwrt.org> +} --- /dev/null +++ b/arch/mips/ralink/mt7621.c -@@ -0,0 +1,192 @@ +@@ -0,0 +1,209 @@ +/* + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published @@ -555,8 +555,12 @@ Signed-off-by: John Crispin <blogic@openwrt.org> + +#define MT7621_GPIO_MODE_UART1 1 +#define MT7621_GPIO_MODE_I2C 2 -+#define MT7621_GPIO_MODE_UART2 3 -+#define MT7621_GPIO_MODE_UART3 5 ++#define MT7621_GPIO_MODE_UART3_MASK 0x3 ++#define MT7621_GPIO_MODE_UART3_SHIFT 3 ++#define MT7621_GPIO_MODE_UART3_GPIO 1 ++#define MT7621_GPIO_MODE_UART2_MASK 0x3 ++#define MT7621_GPIO_MODE_UART2_SHIFT 5 ++#define MT7621_GPIO_MODE_UART2_GPIO 1 +#define MT7621_GPIO_MODE_JTAG 7 +#define MT7621_GPIO_MODE_WDT_MASK 0x3 +#define MT7621_GPIO_MODE_WDT_SHIFT 8 @@ -566,7 +570,9 @@ Signed-off-by: John Crispin <blogic@openwrt.org> +#define MT7621_GPIO_MODE_PCIE_MASK 0x3 +#define MT7621_GPIO_MODE_PCIE_SHIFT 10 +#define MT7621_GPIO_MODE_PCIE_GPIO 1 -+#define MT7621_GPIO_MODE_MDIO 12 ++#define MT7621_GPIO_MODE_MDIO_MASK 0x3 ++#define MT7621_GPIO_MODE_MDIO_SHIFT 12 ++#define MT7621_GPIO_MODE_MDIO_GPIO 1 +#define MT7621_GPIO_MODE_RGMII1 14 +#define MT7621_GPIO_MODE_RGMII2 15 +#define MT7621_GPIO_MODE_SPI_MASK 0x3 @@ -576,10 +582,18 @@ Signed-off-by: John Crispin <blogic@openwrt.org> +#define MT7621_GPIO_MODE_SDHCI_SHIFT 18 +#define MT7621_GPIO_MODE_SDHCI_GPIO 1 + -+static struct rt2880_pmx_func uart1_grp[] = { FUNC("uart", 0, 1, 2) }; ++static struct rt2880_pmx_func uart1_grp[] = { FUNC("uart1", 0, 1, 2) }; +static struct rt2880_pmx_func i2c_grp[] = { FUNC("i2c", 0, 3, 2) }; -+static struct rt2880_pmx_func uart3_grp[] = { FUNC("uart", 0, 5, 4) }; -+static struct rt2880_pmx_func uart2_grp[] = { FUNC("uart", 0, 9, 4) }; ++static struct rt2880_pmx_func uart3_grp[] = { ++ FUNC("uart3", 0, 5, 4), ++ FUNC("i2s", 2, 5, 4), ++ FUNC("spdif3", 3, 5, 4), ++}; ++static struct rt2880_pmx_func uart2_grp[] = { ++ FUNC("uart2", 0, 9, 4), ++ FUNC("pcm", 2, 9, 4), ++ FUNC("spdif2", 3, 9, 4), ++}; +static struct rt2880_pmx_func jtag_grp[] = { FUNC("jtag", 0, 13, 5) }; +static struct rt2880_pmx_func wdt_grp[] = { + FUNC("wdt rst", 0, 18, 1), @@ -590,28 +604,31 @@ Signed-off-by: John Crispin <blogic@openwrt.org> + FUNC("pcie refclk", MT7621_GPIO_MODE_PCIE_REF, 19, 1) +}; +static struct rt2880_pmx_func mdio_grp[] = { FUNC("mdio", 0, 20, 2) }; -+static struct rt2880_pmx_func rgmii2_grp[] = { FUNC("rgmii", 0, 22, 12) }; ++static struct rt2880_pmx_func rgmii2_grp[] = { FUNC("rgmii2", 0, 22, 12) }; +static struct rt2880_pmx_func spi_grp[] = { + FUNC("spi", 0, 34, 7), -+ FUNC("nand", 2, 34, 8), ++ FUNC("nand1", 2, 34, 7), +}; +static struct rt2880_pmx_func sdhci_grp[] = { + FUNC("sdhci", 0, 41, 8), -+ FUNC("nand", 2, 41, 8), ++ FUNC("nand2", 2, 41, 8), +}; -+static struct rt2880_pmx_func rgmii1_grp[] = { FUNC("rgmii", 0, 49, 12) }; ++static struct rt2880_pmx_func rgmii1_grp[] = { FUNC("rgmii1", 0, 49, 12) }; + +static struct rt2880_pmx_group mt7621_pinmux_data[] = { + GRP("uart1", uart1_grp, 1, MT7621_GPIO_MODE_UART1), + GRP("i2c", i2c_grp, 1, MT7621_GPIO_MODE_I2C), -+ GRP("uart3", uart2_grp, 1, MT7621_GPIO_MODE_UART2), -+ GRP("uart2", uart3_grp, 1, MT7621_GPIO_MODE_UART3), ++ GRP_G("uart3", uart3_grp, MT7621_GPIO_MODE_UART3_MASK, ++ MT7621_GPIO_MODE_UART3_GPIO, MT7621_GPIO_MODE_UART3_SHIFT), ++ GRP_G("uart2", uart2_grp, MT7621_GPIO_MODE_UART2_MASK, ++ MT7621_GPIO_MODE_UART2_GPIO, MT7621_GPIO_MODE_UART2_SHIFT), + GRP("jtag", jtag_grp, 1, MT7621_GPIO_MODE_JTAG), + GRP_G("wdt", wdt_grp, MT7621_GPIO_MODE_WDT_MASK, + MT7621_GPIO_MODE_WDT_GPIO, MT7621_GPIO_MODE_WDT_SHIFT), + GRP_G("pcie", pcie_rst_grp, MT7621_GPIO_MODE_PCIE_MASK, + MT7621_GPIO_MODE_PCIE_GPIO, MT7621_GPIO_MODE_PCIE_SHIFT), -+ GRP("mdio", mdio_grp, 1, MT7621_GPIO_MODE_MDIO), ++ GRP_G("mdio", mdio_grp, MT7621_GPIO_MODE_MDIO_MASK, ++ MT7621_GPIO_MODE_MDIO_GPIO, MT7621_GPIO_MODE_MDIO_SHIFT), + GRP("rgmii2", rgmii2_grp, 1, MT7621_GPIO_MODE_RGMII2), + GRP_G("spi", spi_grp, MT7621_GPIO_MODE_SPI_MASK, + MT7621_GPIO_MODE_SPI_GPIO, MT7621_GPIO_MODE_SPI_SHIFT),
The pinctrl-rt2880 code doesn't support multiple functions with the same name. This will result in incorrect pinmux configuration. The MT7621 uses 2 bit wide configuration of the sdhci, spi, mdio, pcie, wdt, uart2 and uart3. But the code only changed a single bit for uart3, uart2 and mdio. Also the order of uart2 and uart3 group was exchanged. The spi "nand" settings was also reserved only 7 PINs instead of 8 as the spi "spi" setting. Signed-off-by: Sven Eckelmann <sven@open-mesh.com> --- target/linux/ramips/dts/mt7621.dtsi | 14 +++---- .../0012-MIPS-ralink-add-MT7621-support.patch | 45 +++++++++++++++------- 2 files changed, 38 insertions(+), 21 deletions(-)