[U-Boot] [PATCH 0/7] *** Qualcomm Snapdraon serial fixes***
In my ongoing effort to get rid of LK (Qualcomm's bootloader) that runs before U-boot, I found that there were some problems with the uart driver of Snapdragon. It appears that the uart only worked because the initialization was previously done by LK.
While fixing, I also introduced a new pinctrl driver for snapdragon (currently only enabled for APQ8016). the pinctrl driver is used for muxing the uart pins and configure the pins IO properties.
Ramon Fried (7): db820c: set clk node to be probed before relocation serial: serial_msm: fail probe if settings clocks fails serial: serial_msm: initialize uart only before relocation mach-snapdragon: Fix UART clock flow mach-snapdragon: Introduce pinctrl driver db410: added pinctrl node and serial bindings serial: serial_msm: added pinmux & config
arch/arm/dts/dragonboard410c.dts | 14 ++ arch/arm/dts/dragonboard820c-uboot.dtsi | 14 ++ arch/arm/mach-snapdragon/Makefile | 2 + arch/arm/mach-snapdragon/clock-apq8016.c | 23 ++- arch/arm/mach-snapdragon/clock-apq8096.c | 4 +- arch/arm/mach-snapdragon/clock-snapdragon.c | 17 ++- arch/arm/mach-snapdragon/clock-snapdragon.h | 9 +- .../mach-snapdragon/include/mach/sysmap-apq8016.h | 1 + arch/arm/mach-snapdragon/pinctrl-apq8016.c | 162 +++++++++++++++++++++ arch/arm/mach-snapdragon/pinctrl-snapdragon.c | 118 +++++++++++++++ arch/arm/mach-snapdragon/pinctrl-snapdragon.h | 21 +++ configs/dragonboard410c_defconfig | 5 + drivers/serial/serial_msm.c | 30 +++- include/dt-bindings/pinctrl/pinctrl-snapdragon.h | 22 +++ 14 files changed, 423 insertions(+), 19 deletions(-) create mode 100644 arch/arm/mach-snapdragon/pinctrl-apq8016.c create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.c create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.h create mode 100644 include/dt-bindings/pinctrl/pinctrl-snapdragon.h
The clock and serial nodes are needed before relocation. This patch ensures that the msm-serial driver will probe and provide uart output before relocation.
Signed-off-by: Ramon Fried ramon.fried@gmail.com --- arch/arm/dts/dragonboard820c-uboot.dtsi | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/arch/arm/dts/dragonboard820c-uboot.dtsi b/arch/arm/dts/dragonboard820c-uboot.dtsi index 88312b3fa1..81df788fca 100644 --- a/arch/arm/dts/dragonboard820c-uboot.dtsi +++ b/arch/arm/dts/dragonboard820c-uboot.dtsi @@ -5,6 +5,20 @@ * (C) Copyright 2017 Jorge Ramirez-Ortiz jorge.ramirez-ortiz@linaro.org */
+/ { + soc { + u-boot,dm-pre-reloc; + + clock-controller@300000 { + u-boot,dm-pre-reloc; + }; + + serial@75b0000 { + u-boot,dm-pre-reloc; + }; + }; +}; + &pm8994_pon { key_vol_down { gpios = <&pm8994_pon 1 0>;
Hi Ramon,
On 12 May 2018 at 20:15, Ramon Fried ramon.fried@gmail.com wrote:
The clock and serial nodes are needed before relocation. This patch ensures that the msm-serial driver will probe and provide uart output before relocation.
Signed-off-by: Ramon Fried ramon.fried@gmail.com
arch/arm/dts/dragonboard820c-uboot.dtsi | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/arch/arm/dts/dragonboard820c-uboot.dtsi b/arch/arm/dts/dragonboard820c-uboot.dtsi index 88312b3fa1..81df788fca 100644 --- a/arch/arm/dts/dragonboard820c-uboot.dtsi +++ b/arch/arm/dts/dragonboard820c-uboot.dtsi @@ -5,6 +5,20 @@
- (C) Copyright 2017 Jorge Ramirez-Ortiz jorge.ramirez-ortiz@linaro.org
*/
+/ {
soc {u-boot,dm-pre-reloc;clock-controller@300000 {
This line and a few below it should be indented one more stop.
u-boot,dm-pre-reloc;};serial@75b0000 {u-boot,dm-pre-reloc;};};+};
&pm8994_pon { key_vol_down { gpios = <&pm8994_pon 1 0>; -- 2.14.1
Regards, Simon
On Mon, May 14, 2018 at 1:00 AM, Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 12 May 2018 at 20:15, Ramon Fried ramon.fried@gmail.com wrote:
The clock and serial nodes are needed before relocation. This patch ensures that the msm-serial driver will probe and provide uart output before relocation.
Signed-off-by: Ramon Fried ramon.fried@gmail.com
arch/arm/dts/dragonboard820c-uboot.dtsi | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/arch/arm/dts/dragonboard820c-uboot.dtsi b/arch/arm/dts/dragonboard820c-uboot.dtsi index 88312b3fa1..81df788fca 100644 --- a/arch/arm/dts/dragonboard820c-uboot.dtsi +++ b/arch/arm/dts/dragonboard820c-uboot.dtsi @@ -5,6 +5,20 @@
- (C) Copyright 2017 Jorge Ramirez-Ortiz jorge.ramirez-ortiz@linaro.org
*/
+/ {
soc {u-boot,dm-pre-reloc;clock-controller@300000 {This line and a few below it should be indented one more stop.
Sure, will do.
u-boot,dm-pre-reloc;};serial@75b0000 {u-boot,dm-pre-reloc;};};+};
&pm8994_pon { key_vol_down { gpios = <&pm8994_pon 1 0>; -- 2.14.1
Regards, Simon
Thanks !
Failure to set the clocks will causes data abort exception when trying to write to AHB uart registers. This patch ensures that we don't touch these registers if clock setting failed.
Signed-off-by: Ramon Fried ramon.fried@gmail.com --- drivers/serial/serial_msm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c index 119e6b9846..250e48c996 100644 --- a/drivers/serial/serial_msm.c +++ b/drivers/serial/serial_msm.c @@ -183,8 +183,8 @@ static int msm_serial_probe(struct udevice *dev) { struct msm_serial_data *priv = dev_get_priv(dev);
- msm_uart_clk_init(dev); /* Ignore return value and hope clock was - properly initialized by earlier loaders */ + if (msm_uart_clk_init(dev)) + return -EINVAL;
if (readl(priv->base + UARTDM_SR) & UARTDM_SR_UART_OVERRUN) writel(UARTDM_CR_CMD_RESET_ERR, priv->base + UARTDM_CR);
Hi Ramon,
On 12 May 2018 at 20:15, Ramon Fried ramon.fried@gmail.com wrote:
Failure to set the clocks will causes data abort exception when trying to write to AHB uart registers. This patch ensures that we don't touch these registers if clock setting failed.
Signed-off-by: Ramon Fried ramon.fried@gmail.com
drivers/serial/serial_msm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c index 119e6b9846..250e48c996 100644 --- a/drivers/serial/serial_msm.c +++ b/drivers/serial/serial_msm.c @@ -183,8 +183,8 @@ static int msm_serial_probe(struct udevice *dev) { struct msm_serial_data *priv = dev_get_priv(dev);
msm_uart_clk_init(dev); /* Ignore return value and hope clock wasproperly initialized by earlier loaders */
if (msm_uart_clk_init(dev))return -EINVAL;
Would it not be better to return the error that msm_uart_clk_init() returns, rather than -EINVAL?
if (readl(priv->base + UARTDM_SR) & UARTDM_SR_UART_OVERRUN) writel(UARTDM_CR_CMD_RESET_ERR, priv->base + UARTDM_CR);-- 2.14.1
Regards, Simon
On Mon, May 14, 2018 at 1:00 AM, Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 12 May 2018 at 20:15, Ramon Fried ramon.fried@gmail.com wrote:
Failure to set the clocks will causes data abort exception when trying to write to AHB uart registers. This patch ensures that we don't touch these registers if clock setting failed.
Signed-off-by: Ramon Fried ramon.fried@gmail.com
drivers/serial/serial_msm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c index 119e6b9846..250e48c996 100644 --- a/drivers/serial/serial_msm.c +++ b/drivers/serial/serial_msm.c @@ -183,8 +183,8 @@ static int msm_serial_probe(struct udevice *dev) { struct msm_serial_data *priv = dev_get_priv(dev);
msm_uart_clk_init(dev); /* Ignore return value and hope clock wasproperly initialized by earlier loaders */
if (msm_uart_clk_init(dev))return -EINVAL;Would it not be better to return the error that msm_uart_clk_init() returns, rather than -EINVAL?
Yes. you're right. I will fix.
if (readl(priv->base + UARTDM_SR) & UARTDM_SR_UART_OVERRUN) writel(UARTDM_CR_CMD_RESET_ERR, priv->base + UARTDM_CR);-- 2.14.1
Regards, Simon
The uart is already initialized prior to relocation, reinitialization after relocation is unnecessary.
Signed-off-by: Ramon Fried ramon.fried@gmail.com --- drivers/serial/serial_msm.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c index 250e48c996..22301c0e37 100644 --- a/drivers/serial/serial_msm.c +++ b/drivers/serial/serial_msm.c @@ -183,6 +183,10 @@ static int msm_serial_probe(struct udevice *dev) { struct msm_serial_data *priv = dev_get_priv(dev);
+ /* No need to reinitialize the UART after relocation */ + if ((gd->flags & GD_FLG_RELOC)) + return 0; + if (msm_uart_clk_init(dev)) return -EINVAL;
On 12 May 2018 at 20:15, Ramon Fried ramon.fried@gmail.com wrote:
The uart is already initialized prior to relocation, reinitialization after relocation is unnecessary.
Signed-off-by: Ramon Fried ramon.fried@gmail.com
drivers/serial/serial_msm.c | 4 ++++ 1 file changed, 4 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
But please just use one set of brackets.
diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c index 250e48c996..22301c0e37 100644 --- a/drivers/serial/serial_msm.c +++ b/drivers/serial/serial_msm.c @@ -183,6 +183,10 @@ static int msm_serial_probe(struct udevice *dev) { struct msm_serial_data *priv = dev_get_priv(dev);
/* No need to reinitialize the UART after relocation */if ((gd->flags & GD_FLG_RELOC))return 0;if (msm_uart_clk_init(dev)) return -EINVAL;-- 2.14.1
UART clock enabling flow was wrong. Changed the flow according to downstream implementation in LK.
Signed-off-by: Ramon Fried ramon.fried@gmail.com --- arch/arm/mach-snapdragon/clock-apq8016.c | 23 +++++++++++++++------- arch/arm/mach-snapdragon/clock-apq8096.c | 4 ++-- arch/arm/mach-snapdragon/clock-snapdragon.c | 17 +++++++++++++++- arch/arm/mach-snapdragon/clock-snapdragon.h | 9 +++++++-- .../mach-snapdragon/include/mach/sysmap-apq8016.h | 1 + 5 files changed, 42 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mach-snapdragon/clock-apq8016.c b/arch/arm/mach-snapdragon/clock-apq8016.c index 9c0cc1c22c..6e4a0ccb90 100644 --- a/arch/arm/mach-snapdragon/clock-apq8016.c +++ b/arch/arm/mach-snapdragon/clock-apq8016.c @@ -17,7 +17,6 @@
/* GPLL0 clock control registers */ #define GPLL0_STATUS_ACTIVE BIT(17) -#define APCS_GPLL_ENA_VOTE_GPLL0 BIT(0)
static const struct bcr_regs sdc_regs[] = { { @@ -36,11 +35,17 @@ static const struct bcr_regs sdc_regs[] = { } };
-static struct gpll0_ctrl gpll0_ctrl = { +static struct pll_vote_clk gpll0_vote_clk = { .status = GPLL0_STATUS, .status_bit = GPLL0_STATUS_ACTIVE, .ena_vote = APCS_GPLL_ENA_VOTE, - .vote_bit = APCS_GPLL_ENA_VOTE_GPLL0, + .vote_bit = BIT(0), +}; + +static struct vote_clk gcc_blsp1_ahb_clk = { + .cbcr_reg = BLSP1_AHB_CBCR, + .ena_vote = APCS_CLOCK_BRANCH_ENA_VOTE, + .vote_bit = BIT(10), };
/* SDHCI */ @@ -55,7 +60,7 @@ static int clk_init_sdc(struct msm_clk_priv *priv, int slot, uint rate) /* 800Mhz/div, gpll0 */ clk_rcg_set_rate_mnd(priv->base, &sdc_regs[slot], div, 0, 0, CFG_CLK_SRC_GPLL0); - clk_enable_gpll0(priv->base, &gpll0_ctrl); + clk_enable_gpll0(priv->base, &gpll0_vote_clk); clk_enable_cbc(priv->base + SDCC_APPS_CBCR(slot));
return rate; @@ -72,12 +77,16 @@ static const struct bcr_regs uart2_regs = { /* UART: 115200 */ static int clk_init_uart(struct msm_clk_priv *priv) { - /* Enable iface clk */ - clk_enable_cbc(priv->base + BLSP1_AHB_CBCR); + /* Enable AHB clock */ + clk_enable_vote_clk(priv->base, &gcc_blsp1_ahb_clk); + /* 7372800 uart block clock @ GPLL0 */ clk_rcg_set_rate_mnd(priv->base, &uart2_regs, 1, 144, 15625, CFG_CLK_SRC_GPLL0); - clk_enable_gpll0(priv->base, &gpll0_ctrl); + + /* Vote for gpll0 clock */ + clk_enable_gpll0(priv->base, &gpll0_vote_clk); + /* Enable core clk */ clk_enable_cbc(priv->base + BLSP1_UART2_APPS_CBCR);
diff --git a/arch/arm/mach-snapdragon/clock-apq8096.c b/arch/arm/mach-snapdragon/clock-apq8096.c index 008649a4c6..628c38785b 100644 --- a/arch/arm/mach-snapdragon/clock-apq8096.c +++ b/arch/arm/mach-snapdragon/clock-apq8096.c @@ -27,7 +27,7 @@ static const struct bcr_regs sdc_regs = { .D = SDCC2_D, };
-static const struct gpll0_ctrl gpll0_ctrl = { +static const struct pll_vote_clk gpll0_vote_clk = { .status = GPLL0_STATUS, .status_bit = GPLL0_STATUS_ACTIVE, .ena_vote = APCS_GPLL_ENA_VOTE, @@ -41,7 +41,7 @@ static int clk_init_sdc(struct msm_clk_priv *priv, uint rate) clk_enable_cbc(priv->base + SDCC2_AHB_CBCR); clk_rcg_set_rate_mnd(priv->base, &sdc_regs, div, 0, 0, CFG_CLK_SRC_GPLL0); - clk_enable_gpll0(priv->base, &gpll0_ctrl); + clk_enable_gpll0(priv->base, &gpll0_vote_clk); clk_enable_cbc(priv->base + SDCC2_APPS_CBCR);
return rate; diff --git a/arch/arm/mach-snapdragon/clock-snapdragon.c b/arch/arm/mach-snapdragon/clock-snapdragon.c index f738f57043..85526186c6 100644 --- a/arch/arm/mach-snapdragon/clock-snapdragon.c +++ b/arch/arm/mach-snapdragon/clock-snapdragon.c @@ -30,7 +30,7 @@ void clk_enable_cbc(phys_addr_t cbcr) ; }
-void clk_enable_gpll0(phys_addr_t base, const struct gpll0_ctrl *gpll0) +void clk_enable_gpll0(phys_addr_t base, const struct pll_vote_clk *gpll0) { if (readl(base + gpll0->status) & gpll0->status_bit) return; /* clock already enabled */ @@ -41,6 +41,21 @@ void clk_enable_gpll0(phys_addr_t base, const struct gpll0_ctrl *gpll0) ; }
+#define BRANCH_ON_VAL (0) +#define BRANCH_NOC_FSM_ON_VAL BIT(29) +#define BRANCH_CHECK_MASK GENMASK(31, 28) + +void clk_enable_vote_clk(phys_addr_t base, const struct vote_clk *vclk) +{ + u32 val; + + setbits_le32(base + vclk->ena_vote, vclk->vote_bit); + do { + val = readl(base + vclk->cbcr_reg); + val &= BRANCH_CHECK_MASK; + } while ((val != BRANCH_ON_VAL) && (val != BRANCH_NOC_FSM_ON_VAL)); +} + #define APPS_CMD_RGCR_UPDATE BIT(0)
/* Update clock command via CMD_RGCR */ diff --git a/arch/arm/mach-snapdragon/clock-snapdragon.h b/arch/arm/mach-snapdragon/clock-snapdragon.h index 2cff4f8a06..3ae21099c2 100644 --- a/arch/arm/mach-snapdragon/clock-snapdragon.h +++ b/arch/arm/mach-snapdragon/clock-snapdragon.h @@ -11,13 +11,18 @@ #define CFG_CLK_SRC_GPLL0 (1 << 8) #define CFG_CLK_SRC_MASK (7 << 8)
-struct gpll0_ctrl { +struct pll_vote_clk { uintptr_t status; int status_bit; uintptr_t ena_vote; int vote_bit; };
+struct vote_clk { + uintptr_t cbcr_reg; + uintptr_t ena_vote; + int vote_bit; +}; struct bcr_regs { uintptr_t cfg_rcgr; uintptr_t cmd_rcgr; @@ -30,7 +35,7 @@ struct msm_clk_priv { phys_addr_t base; };
-void clk_enable_gpll0(phys_addr_t base, const struct gpll0_ctrl *pll0); +void clk_enable_gpll0(phys_addr_t base, const struct pll_vote_clk *gpll0); void clk_bcr_update(phys_addr_t apps_cmd_rgcr); void clk_enable_cbc(phys_addr_t cbcr); void clk_rcg_set_rate_mnd(phys_addr_t base, const struct bcr_regs *regs, diff --git a/arch/arm/mach-snapdragon/include/mach/sysmap-apq8016.h b/arch/arm/mach-snapdragon/include/mach/sysmap-apq8016.h index ae784387fa..520e2e6bd7 100644 --- a/arch/arm/mach-snapdragon/include/mach/sysmap-apq8016.h +++ b/arch/arm/mach-snapdragon/include/mach/sysmap-apq8016.h @@ -13,6 +13,7 @@ /* Clocks: (from CLK_CTL_BASE) */ #define GPLL0_STATUS (0x2101C) #define APCS_GPLL_ENA_VOTE (0x45000) +#define APCS_CLOCK_BRANCH_ENA_VOTE (0x45004)
#define SDCC_BCR(n) ((n * 0x1000) + 0x41000) #define SDCC_CMD_RCGR(n) ((n * 0x1000) + 0x41004)
This patch adds pinmux and pinctrl driver for TLMM subsystem in snapdragon chipsets. Currently, supporting only 8016, but implementation is generic and 8096 can be added easily.
Driver is using the generic dt-bindings and doesn't introduce any new bindings (yet).
Signed-off-by: Ramon Fried ramon.fried@gmail.com --- arch/arm/mach-snapdragon/Makefile | 2 + arch/arm/mach-snapdragon/pinctrl-apq8016.c | 162 +++++++++++++++++++++++ arch/arm/mach-snapdragon/pinctrl-snapdragon.c | 118 +++++++++++++++++ arch/arm/mach-snapdragon/pinctrl-snapdragon.h | 21 +++ configs/dragonboard410c_defconfig | 5 + include/dt-bindings/pinctrl/pinctrl-snapdragon.h | 22 +++ 6 files changed, 330 insertions(+) create mode 100644 arch/arm/mach-snapdragon/pinctrl-apq8016.c create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.c create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.h create mode 100644 include/dt-bindings/pinctrl/pinctrl-snapdragon.h
diff --git a/arch/arm/mach-snapdragon/Makefile b/arch/arm/mach-snapdragon/Makefile index 1c23dc52cf..1d35fea912 100644 --- a/arch/arm/mach-snapdragon/Makefile +++ b/arch/arm/mach-snapdragon/Makefile @@ -6,4 +6,6 @@ obj-$(CONFIG_TARGET_DRAGONBOARD820C) += clock-apq8096.o obj-$(CONFIG_TARGET_DRAGONBOARD820C) += sysmap-apq8096.o obj-$(CONFIG_TARGET_DRAGONBOARD410C) += clock-apq8016.o obj-$(CONFIG_TARGET_DRAGONBOARD410C) += sysmap-apq8016.o +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-apq8016.o +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-snapdragon.o obj-y += clock-snapdragon.o diff --git a/arch/arm/mach-snapdragon/pinctrl-apq8016.c b/arch/arm/mach-snapdragon/pinctrl-apq8016.c new file mode 100644 index 0000000000..8e57e2338c --- /dev/null +++ b/arch/arm/mach-snapdragon/pinctrl-apq8016.c @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Qualcomm APQ8016 pinctrl + * + * (C) Copyright 2018 Ramon Fried ramon.fried@gmail.com + * + */ + +#include "pinctrl-snapdragon.h" +#include <common.h> + +const char * const msm_pinctrl_pins[] = { + "GPIO_0", + "GPIO_1", + "GPIO_2", + "GPIO_3", + "GPIO_4", + "GPIO_5", + "GPIO_6", + "GPIO_7", + "GPIO_8", + "GPIO_9", + "GPIO_10", + "GPIO_11", + "GPIO_12", + "GPIO_13", + "GPIO_14", + "GPIO_15", + "GPIO_16", + "GPIO_17", + "GPIO_18", + "GPIO_19", + "GPIO_20", + "GPIO_21", + "GPIO_22", + "GPIO_23", + "GPIO_24", + "GPIO_25", + "GPIO_26", + "GPIO_27", + "GPIO_28", + "GPIO_29", + "GPIO_30", + "GPIO_31", + "GPIO_32", + "GPIO_33", + "GPIO_34", + "GPIO_35", + "GPIO_36", + "GPIO_37", + "GPIO_38", + "GPIO_39", + "GPIO_40", + "GPIO_41", + "GPIO_42", + "GPIO_43", + "GPIO_44", + "GPIO_45", + "GPIO_46", + "GPIO_47", + "GPIO_48", + "GPIO_49", + "GPIO_50", + "GPIO_51", + "GPIO_52", + "GPIO_53", + "GPIO_54", + "GPIO_55", + "GPIO_56", + "GPIO_57", + "GPIO_58", + "GPIO_59", + "GPIO_60", + "GPIO_61", + "GPIO_62", + "GPIO_63", + "GPIO_64", + "GPIO_65", + "GPIO_66", + "GPIO_67", + "GPIO_68", + "GPIO_69", + "GPIO_70", + "GPIO_71", + "GPIO_72", + "GPIO_73", + "GPIO_74", + "GPIO_75", + "GPIO_76", + "GPIO_77", + "GPIO_78", + "GPIO_79", + "GPIO_80", + "GPIO_81", + "GPIO_82", + "GPIO_83", + "GPIO_84", + "GPIO_85", + "GPIO_86", + "GPIO_87", + "GPIO_88", + "GPIO_89", + "GPIO_90", + "GPIO_91", + "GPIO_92", + "GPIO_93", + "GPIO_94", + "GPIO_95", + "GPIO_96", + "GPIO_97", + "GPIO_98", + "GPIO_99", + "GPIO_100", + "GPIO_101", + "GPIO_102", + "GPIO_103", + "GPIO_104", + "GPIO_105", + "GPIO_106", + "GPIO_107", + "GPIO_108", + "GPIO_109", + "GPIO_110", + "GPIO_111", + "GPIO_112", + "GPIO_113", + "GPIO_114", + "GPIO_115", + "GPIO_116", + "GPIO_117", + "GPIO_118", + "GPIO_119", + "GPIO_120", + "GPIO_121", + "GPIO_122", + "GPIO_123", + "GPIO_124", + "GPIO_125", + "GPIO_126", + "GPIO_127", + "GPIO_128", + "GPIO_129", + "SDC1_CLK", + "SDC1_CMD", + "SDC1_DATA", + "SDC2_CLK", + "SDC2_CMD", + "SDC2_DATA", + "QDSD_CLK", + "QDSD_CMD", + "QDSD_DATA0", + "QDSD_DATA1", + "QDSD_DATA2", + "QDSD_DATA3", +}; + +const struct pinctrl_function msm_pinctrl_functions[] = { + {"blsp1_uart", 2}, +}; + +const int msm_functions_count = ARRAY_SIZE(msm_pinctrl_functions); +const int msm_pins_count = ARRAY_SIZE(msm_pinctrl_pins); diff --git a/arch/arm/mach-snapdragon/pinctrl-snapdragon.c b/arch/arm/mach-snapdragon/pinctrl-snapdragon.c new file mode 100644 index 0000000000..8aa6f9a7bd --- /dev/null +++ b/arch/arm/mach-snapdragon/pinctrl-snapdragon.c @@ -0,0 +1,118 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * TLMM driver for Qualcomm APQ8016, APQ8096 + * + * (C) Copyright 2018 Ramon Fried ramon.fried@linaro.org + * + */ + +#include <common.h> +#include <dm.h> +#include <errno.h> +#include <asm/io.h> +#include <dm/pinctrl.h> +#include "pinctrl-snapdragon.h" + +struct msm_pinctrl_priv { + phys_addr_t base; +}; + +#define GPIO_CONFIG_OFFSET(x) ((x) * 0x1000) +#define TLMM_GPIO_PULL_MASK GENMASK(1, 0) +#define TLMM_FUNC_SEL_MASK GENMASK(5, 2) +#define TLMM_DRV_STRENGTH_MASK GENMASK(8, 6) +#define TLMM_GPIO_ENABLE BIT(9) + +static const struct pinconf_param msm_conf_params[] = { + { "drive-strength", PIN_CONFIG_DRIVE_STRENGTH, 3 }, + { "bias-disable", PIN_CONFIG_BIAS_DISABLE, 0 }, +}; + +static int msm_get_functions_count(struct udevice *dev) +{ + return msm_functions_count; +} + +static int msm_get_pins_count(struct udevice *dev) +{ + return msm_pins_count; +} + +static const char *msm_get_function_name(struct udevice *dev, + unsigned int selector) +{ + return msm_pinctrl_functions[selector].name; +} + +static int msm_pinctrl_probe(struct udevice *dev) +{ + struct msm_pinctrl_priv *priv = dev_get_priv(dev); + + priv->base = devfdt_get_addr(dev); + + return priv->base == FDT_ADDR_T_NONE ? -EINVAL : 0; +} + +static const char *msm_get_pin_name(struct udevice *dev, unsigned int selector) +{ + return msm_pinctrl_pins[selector]; +} + +static int msm_pinmux_set(struct udevice *dev, unsigned int pin_selector, + unsigned int func_selector) +{ + struct msm_pinctrl_priv *priv = dev_get_priv(dev); + + clrsetbits_le32(priv->base + GPIO_CONFIG_OFFSET(pin_selector), + TLMM_FUNC_SEL_MASK | TLMM_GPIO_ENABLE, + msm_pinctrl_functions[func_selector].val << 2); + return 0; +} + +static int msm_pinconf_set(struct udevice *dev, unsigned int pin_selector, + unsigned int param, unsigned int argument) +{ + struct msm_pinctrl_priv *priv = dev_get_priv(dev); + + switch (param) { + case PIN_CONFIG_DRIVE_STRENGTH: + clrsetbits_le32(priv->base + GPIO_CONFIG_OFFSET(pin_selector), + TLMM_DRV_STRENGTH_MASK, argument << 6); + break; + case PIN_CONFIG_BIAS_DISABLE: + clrbits_le32(priv->base + GPIO_CONFIG_OFFSET(pin_selector), + TLMM_GPIO_PULL_MASK); + break; + default: + return 0; + } + + return 0; +} + +static struct pinctrl_ops msm_pinctrl_ops = { + .get_pins_count = msm_get_pins_count, + .get_pin_name = msm_get_pin_name, + .set_state = pinctrl_generic_set_state, + .pinmux_set = msm_pinmux_set, + .pinconf_num_params = ARRAY_SIZE(msm_conf_params), + .pinconf_params = msm_conf_params, + .pinconf_set = msm_pinconf_set, + .get_functions_count = msm_get_functions_count, + .get_function_name = msm_get_function_name, +}; + +static const struct udevice_id msm_pinctrl_ids[] = { + { .compatible = "qcom,tlmm-msm8916" }, + { .compatible = "qcom,tlmm-apq8016" }, + { } +}; + +U_BOOT_DRIVER(pinctrl_snapdraon) = { + .name = "pinctrl_msm", + .id = UCLASS_PINCTRL, + .of_match = msm_pinctrl_ids, + .priv_auto_alloc_size = sizeof(struct msm_pinctrl_priv), + .ops = &msm_pinctrl_ops, + .probe = msm_pinctrl_probe, +}; diff --git a/arch/arm/mach-snapdragon/pinctrl-snapdragon.h b/arch/arm/mach-snapdragon/pinctrl-snapdragon.h new file mode 100644 index 0000000000..3d0527148a --- /dev/null +++ b/arch/arm/mach-snapdragon/pinctrl-snapdragon.h @@ -0,0 +1,21 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Qualcomm Pin control + * + * (C) Copyright 2018 Ramon Fried ramon.fried@gmail.com + * + */ +#ifndef _PINCTRL_SNAPDRAGON_H +#define _PINCTRL_SNAPDRAGON_H + +struct pinctrl_function { + const char *name; + int val; +}; + +extern const char * const msm_pinctrl_pins[]; +extern const struct pinctrl_function msm_pinctrl_functions[]; +extern const int msm_functions_count; +extern const int msm_pins_count; + +#endif diff --git a/configs/dragonboard410c_defconfig b/configs/dragonboard410c_defconfig index e6114db2ce..4b3de64dd5 100644 --- a/configs/dragonboard410c_defconfig +++ b/configs/dragonboard410c_defconfig @@ -45,3 +45,8 @@ CONFIG_USB_ETHER_ASIX88179=y CONFIG_USB_ETHER_MCS7830=y CONFIG_USB_ETHER_SMSC95XX=y CONFIG_OF_LIBFDT_OVERLAY=y +CONFIG_PINCTRL=y +CONFIG_PINCTRL_FULL=y +CONFIG_PINCTRL_GENERIC=y +CONFIG_PINMUX=y +CONFIG_PINCONF=y diff --git a/include/dt-bindings/pinctrl/pinctrl-snapdragon.h b/include/dt-bindings/pinctrl/pinctrl-snapdragon.h new file mode 100644 index 0000000000..615affb6f2 --- /dev/null +++ b/include/dt-bindings/pinctrl/pinctrl-snapdragon.h @@ -0,0 +1,22 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * This header provides constants for Qualcomm Snapdragon pinctrl bindings. + * + * (C) Copyright 2018 Ramon Fried ramon.fried@gmail.com + * + */ + +#ifndef _DT_BINDINGS_PINCTRL_SNAPDRAGON_H +#define _DT_BINDINGS_PINCTRL_SNAPDRAGON_H + +/* GPIO Drive Strength */ +#define DRIVE_STRENGTH_2MA 0 +#define DRIVE_STRENGTH_4MA 1 +#define DRIVE_STRENGTH_6MA 2 +#define DRIVE_STRENGTH_8MA 3 +#define DRIVE_STRENGTH_10MA 4 +#define DRIVE_STRENGTH_12MA 5 +#define DRIVE_STRENGTH_14MA 6 +#define DRIVE_STRENGTH_16MA 7 + +#endif
Hi Ramon,
On 12 May 2018 at 20:15, Ramon Fried ramon.fried@gmail.com wrote:
This patch adds pinmux and pinctrl driver for TLMM subsystem in snapdragon chipsets. Currently, supporting only 8016, but implementation is generic and 8096 can be added easily.
Driver is using the generic dt-bindings and doesn't introduce any new bindings (yet).
Signed-off-by: Ramon Fried ramon.fried@gmail.com
arch/arm/mach-snapdragon/Makefile | 2 + arch/arm/mach-snapdragon/pinctrl-apq8016.c | 162 +++++++++++++++++++++++ arch/arm/mach-snapdragon/pinctrl-snapdragon.c | 118 +++++++++++++++++ arch/arm/mach-snapdragon/pinctrl-snapdragon.h | 21 +++ configs/dragonboard410c_defconfig | 5 + include/dt-bindings/pinctrl/pinctrl-snapdragon.h | 22 +++ 6 files changed, 330 insertions(+) create mode 100644 arch/arm/mach-snapdragon/pinctrl-apq8016.c create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.c create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.h create mode 100644 include/dt-bindings/pinctrl/pinctrl-snapdragon.h
diff --git a/arch/arm/mach-snapdragon/Makefile b/arch/arm/mach-snapdragon/Makefile index 1c23dc52cf..1d35fea912 100644 --- a/arch/arm/mach-snapdragon/Makefile +++ b/arch/arm/mach-snapdragon/Makefile @@ -6,4 +6,6 @@ obj-$(CONFIG_TARGET_DRAGONBOARD820C) += clock-apq8096.o obj-$(CONFIG_TARGET_DRAGONBOARD820C) += sysmap-apq8096.o obj-$(CONFIG_TARGET_DRAGONBOARD410C) += clock-apq8016.o obj-$(CONFIG_TARGET_DRAGONBOARD410C) += sysmap-apq8016.o +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-apq8016.o +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-snapdragon.o obj-y += clock-snapdragon.o diff --git a/arch/arm/mach-snapdragon/pinctrl-apq8016.c b/arch/arm/mach-snapdragon/pinctrl-apq8016.c new file mode 100644 index 0000000000..8e57e2338c --- /dev/null +++ b/arch/arm/mach-snapdragon/pinctrl-apq8016.c @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Qualcomm APQ8016 pinctrl
- (C) Copyright 2018 Ramon Fried ramon.fried@gmail.com
- */
+#include "pinctrl-snapdragon.h" +#include <common.h>
+const char * const msm_pinctrl_pins[] = {
"GPIO_0","GPIO_1","GPIO_2","GPIO_3","GPIO_4","GPIO_5","GPIO_6","GPIO_7",
This seems inefficient. Could you not sprintf() the name for most of these values?
Regards, Simon
On Mon, May 14, 2018 at 1:00 AM, Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 12 May 2018 at 20:15, Ramon Fried ramon.fried@gmail.com wrote:
This patch adds pinmux and pinctrl driver for TLMM subsystem in snapdragon chipsets. Currently, supporting only 8016, but implementation is generic and 8096 can be added easily.
Driver is using the generic dt-bindings and doesn't introduce any new bindings (yet).
Signed-off-by: Ramon Fried ramon.fried@gmail.com
arch/arm/mach-snapdragon/Makefile | 2 + arch/arm/mach-snapdragon/pinctrl-apq8016.c | 162 +++++++++++++++++++++++ arch/arm/mach-snapdragon/pinctrl-snapdragon.c | 118 +++++++++++++++++ arch/arm/mach-snapdragon/pinctrl-snapdragon.h | 21 +++ configs/dragonboard410c_defconfig | 5 + include/dt-bindings/pinctrl/pinctrl-snapdragon.h | 22 +++ 6 files changed, 330 insertions(+) create mode 100644 arch/arm/mach-snapdragon/pinctrl-apq8016.c create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.c create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.h create mode 100644 include/dt-bindings/pinctrl/pinctrl-snapdragon.h
diff --git a/arch/arm/mach-snapdragon/Makefile b/arch/arm/mach-snapdragon/Makefile index 1c23dc52cf..1d35fea912 100644 --- a/arch/arm/mach-snapdragon/Makefile +++ b/arch/arm/mach-snapdragon/Makefile @@ -6,4 +6,6 @@ obj-$(CONFIG_TARGET_DRAGONBOARD820C) += clock-apq8096.o obj-$(CONFIG_TARGET_DRAGONBOARD820C) += sysmap-apq8096.o obj-$(CONFIG_TARGET_DRAGONBOARD410C) += clock-apq8016.o obj-$(CONFIG_TARGET_DRAGONBOARD410C) += sysmap-apq8016.o +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-apq8016.o +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-snapdragon.o obj-y += clock-snapdragon.o diff --git a/arch/arm/mach-snapdragon/pinctrl-apq8016.c b/arch/arm/mach-snapdragon/pinctrl-apq8016.c new file mode 100644 index 0000000000..8e57e2338c --- /dev/null +++ b/arch/arm/mach-snapdragon/pinctrl-apq8016.c @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Qualcomm APQ8016 pinctrl
- (C) Copyright 2018 Ramon Fried ramon.fried@gmail.com
- */
+#include "pinctrl-snapdragon.h" +#include <common.h>
+const char * const msm_pinctrl_pins[] = {
"GPIO_0","GPIO_1","GPIO_2","GPIO_3","GPIO_4","GPIO_5","GPIO_6","GPIO_7",This seems inefficient. Could you not sprintf() the name for most of these values?
The origin of this table is from the Linux kernel driver. I'm not sure I understand how sprintf will more efficient, do you want to fill up this table on runtime ?
Regards, Simon
Hi Ramon,
On 14 May 2018 at 01:10, Ramon Fried ramon.fried@gmail.com wrote:
On Mon, May 14, 2018 at 1:00 AM, Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 12 May 2018 at 20:15, Ramon Fried ramon.fried@gmail.com wrote:
This patch adds pinmux and pinctrl driver for TLMM subsystem in snapdragon chipsets. Currently, supporting only 8016, but implementation is generic and 8096 can be added easily.
Driver is using the generic dt-bindings and doesn't introduce any new bindings (yet).
Signed-off-by: Ramon Fried ramon.fried@gmail.com
arch/arm/mach-snapdragon/Makefile | 2 + arch/arm/mach-snapdragon/pinctrl-apq8016.c | 162 +++++++++++++++++++++++ arch/arm/mach-snapdragon/pinctrl-snapdragon.c | 118 +++++++++++++++++ arch/arm/mach-snapdragon/pinctrl-snapdragon.h | 21 +++ configs/dragonboard410c_defconfig | 5 + include/dt-bindings/pinctrl/pinctrl-snapdragon.h | 22 +++ 6 files changed, 330 insertions(+) create mode 100644 arch/arm/mach-snapdragon/pinctrl-apq8016.c create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.c create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.h create mode 100644 include/dt-bindings/pinctrl/pinctrl-snapdragon.h
diff --git a/arch/arm/mach-snapdragon/Makefile b/arch/arm/mach-snapdragon/Makefile index 1c23dc52cf..1d35fea912 100644 --- a/arch/arm/mach-snapdragon/Makefile +++ b/arch/arm/mach-snapdragon/Makefile @@ -6,4 +6,6 @@ obj-$(CONFIG_TARGET_DRAGONBOARD820C) += clock-apq8096.o obj-$(CONFIG_TARGET_DRAGONBOARD820C) += sysmap-apq8096.o obj-$(CONFIG_TARGET_DRAGONBOARD410C) += clock-apq8016.o obj-$(CONFIG_TARGET_DRAGONBOARD410C) += sysmap-apq8016.o +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-apq8016.o +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-snapdragon.o obj-y += clock-snapdragon.o diff --git a/arch/arm/mach-snapdragon/pinctrl-apq8016.c b/arch/arm/mach-snapdragon/pinctrl-apq8016.c new file mode 100644 index 0000000000..8e57e2338c --- /dev/null +++ b/arch/arm/mach-snapdragon/pinctrl-apq8016.c @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Qualcomm APQ8016 pinctrl
- (C) Copyright 2018 Ramon Fried ramon.fried@gmail.com
- */
+#include "pinctrl-snapdragon.h" +#include <common.h>
+const char * const msm_pinctrl_pins[] = {
"GPIO_0","GPIO_1","GPIO_2","GPIO_3","GPIO_4","GPIO_5","GPIO_6","GPIO_7",This seems inefficient. Could you not sprintf() the name for most of these values?
The origin of this table is from the Linux kernel driver. I'm not sure I understand how sprintf will more efficient, do you want to fill up this table on runtime ?
I think this table is only used in one function, so you could create the string there perhaps?
Regards, Simon
On Mon, May 14, 2018 at 10:51 PM, Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 14 May 2018 at 01:10, Ramon Fried ramon.fried@gmail.com wrote:
On Mon, May 14, 2018 at 1:00 AM, Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 12 May 2018 at 20:15, Ramon Fried ramon.fried@gmail.com wrote:
This patch adds pinmux and pinctrl driver for TLMM subsystem in snapdragon chipsets. Currently, supporting only 8016, but implementation is generic and 8096 can be added easily.
Driver is using the generic dt-bindings and doesn't introduce any new bindings (yet).
Signed-off-by: Ramon Fried ramon.fried@gmail.com
arch/arm/mach-snapdragon/Makefile | 2 + arch/arm/mach-snapdragon/pinctrl-apq8016.c | 162 +++++++++++++++++++++++ arch/arm/mach-snapdragon/pinctrl-snapdragon.c | 118 +++++++++++++++++ arch/arm/mach-snapdragon/pinctrl-snapdragon.h | 21 +++ configs/dragonboard410c_defconfig | 5 + include/dt-bindings/pinctrl/pinctrl-snapdragon.h | 22 +++ 6 files changed, 330 insertions(+) create mode 100644 arch/arm/mach-snapdragon/pinctrl-apq8016.c create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.c create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.h create mode 100644 include/dt-bindings/pinctrl/pinctrl-snapdragon.h
diff --git a/arch/arm/mach-snapdragon/Makefile b/arch/arm/mach-snapdragon/Makefile index 1c23dc52cf..1d35fea912 100644 --- a/arch/arm/mach-snapdragon/Makefile +++ b/arch/arm/mach-snapdragon/Makefile @@ -6,4 +6,6 @@ obj-$(CONFIG_TARGET_DRAGONBOARD820C) += clock-apq8096.o obj-$(CONFIG_TARGET_DRAGONBOARD820C) += sysmap-apq8096.o obj-$(CONFIG_TARGET_DRAGONBOARD410C) += clock-apq8016.o obj-$(CONFIG_TARGET_DRAGONBOARD410C) += sysmap-apq8016.o +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-apq8016.o +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-snapdragon.o obj-y += clock-snapdragon.o diff --git a/arch/arm/mach-snapdragon/pinctrl-apq8016.c b/arch/arm/mach-snapdragon/pinctrl-apq8016.c new file mode 100644 index 0000000000..8e57e2338c --- /dev/null +++ b/arch/arm/mach-snapdragon/pinctrl-apq8016.c @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Qualcomm APQ8016 pinctrl
- (C) Copyright 2018 Ramon Fried ramon.fried@gmail.com
- */
+#include "pinctrl-snapdragon.h" +#include <common.h>
+const char * const msm_pinctrl_pins[] = {
"GPIO_0","GPIO_1","GPIO_2","GPIO_3","GPIO_4","GPIO_5","GPIO_6","GPIO_7",This seems inefficient. Could you not sprintf() the name for most of these values?
The origin of this table is from the Linux kernel driver. I'm not sure I understand how sprintf will more efficient, do you want to fill up this table on runtime ?
I think this table is only used in one function, so you could create the string there perhaps?
Actually, it works the other way around, the generic-pinctrl needs a function to translate string to index. Basically, it reads strings from the FDT and then go over all indexes until it matches that string. this is inefficient IMHO as I think it will be easier just to be able to provide an index instead of a string in the FDT.
Regards, Simon
Hi Ramon,
On 15 May 2018 at 07:23, Ramon Fried ramon.fried@gmail.com wrote:
On Mon, May 14, 2018 at 10:51 PM, Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 14 May 2018 at 01:10, Ramon Fried ramon.fried@gmail.com wrote:
On Mon, May 14, 2018 at 1:00 AM, Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 12 May 2018 at 20:15, Ramon Fried ramon.fried@gmail.com wrote:
This patch adds pinmux and pinctrl driver for TLMM subsystem in snapdragon chipsets. Currently, supporting only 8016, but implementation is generic and 8096 can be added easily.
Driver is using the generic dt-bindings and doesn't introduce any new bindings (yet).
Signed-off-by: Ramon Fried ramon.fried@gmail.com
arch/arm/mach-snapdragon/Makefile | 2 + arch/arm/mach-snapdragon/pinctrl-apq8016.c | 162 +++++++++++++++++++++++ arch/arm/mach-snapdragon/pinctrl-snapdragon.c | 118 +++++++++++++++++ arch/arm/mach-snapdragon/pinctrl-snapdragon.h | 21 +++ configs/dragonboard410c_defconfig | 5 + include/dt-bindings/pinctrl/pinctrl-snapdragon.h | 22 +++ 6 files changed, 330 insertions(+) create mode 100644 arch/arm/mach-snapdragon/pinctrl-apq8016.c create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.c create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.h create mode 100644 include/dt-bindings/pinctrl/pinctrl-snapdragon.h
diff --git a/arch/arm/mach-snapdragon/Makefile b/arch/arm/mach-snapdragon/Makefile index 1c23dc52cf..1d35fea912 100644 --- a/arch/arm/mach-snapdragon/Makefile +++ b/arch/arm/mach-snapdragon/Makefile @@ -6,4 +6,6 @@ obj-$(CONFIG_TARGET_DRAGONBOARD820C) += clock-apq8096.o obj-$(CONFIG_TARGET_DRAGONBOARD820C) += sysmap-apq8096.o obj-$(CONFIG_TARGET_DRAGONBOARD410C) += clock-apq8016.o obj-$(CONFIG_TARGET_DRAGONBOARD410C) += sysmap-apq8016.o +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-apq8016.o +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-snapdragon.o obj-y += clock-snapdragon.o diff --git a/arch/arm/mach-snapdragon/pinctrl-apq8016.c b/arch/arm/mach-snapdragon/pinctrl-apq8016.c new file mode 100644 index 0000000000..8e57e2338c --- /dev/null +++ b/arch/arm/mach-snapdragon/pinctrl-apq8016.c @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Qualcomm APQ8016 pinctrl
- (C) Copyright 2018 Ramon Fried ramon.fried@gmail.com
- */
+#include "pinctrl-snapdragon.h" +#include <common.h>
+const char * const msm_pinctrl_pins[] = {
"GPIO_0","GPIO_1","GPIO_2","GPIO_3","GPIO_4","GPIO_5","GPIO_6","GPIO_7",This seems inefficient. Could you not sprintf() the name for most of these values?
The origin of this table is from the Linux kernel driver. I'm not sure I understand how sprintf will more efficient, do you want to fill up this table on runtime ?
I think this table is only used in one function, so you could create the string there perhaps?
Actually, it works the other way around, the generic-pinctrl needs a function to translate string to index. Basically, it reads strings from the FDT and then go over all indexes until it matches that string. this is inefficient IMHO as I think it will be easier just to be able to provide an index instead of a string in the FDT.
OK, so long as the index is actually a known value. If someone changes the DT, won't that fail?
Regards, Simon
On Tue, May 15, 2018, 7:03 PM Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 15 May 2018 at 07:23, Ramon Fried ramon.fried@gmail.com wrote:
On Mon, May 14, 2018 at 10:51 PM, Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 14 May 2018 at 01:10, Ramon Fried ramon.fried@gmail.com wrote:
On Mon, May 14, 2018 at 1:00 AM, Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 12 May 2018 at 20:15, Ramon Fried ramon.fried@gmail.com wrote:
This patch adds pinmux and pinctrl driver for TLMM subsystem in snapdragon chipsets. Currently, supporting only 8016, but implementation is generic and 8096 can be added easily.
Driver is using the generic dt-bindings and doesn't introduce any new bindings (yet).
Signed-off-by: Ramon Fried ramon.fried@gmail.com
arch/arm/mach-snapdragon/Makefile | 2 + arch/arm/mach-snapdragon/pinctrl-apq8016.c | 162
+++++++++++++++++++++++
arch/arm/mach-snapdragon/pinctrl-snapdragon.c | 118
+++++++++++++++++
arch/arm/mach-snapdragon/pinctrl-snapdragon.h | 21 +++ configs/dragonboard410c_defconfig | 5 + include/dt-bindings/pinctrl/pinctrl-snapdragon.h | 22 +++ 6 files changed, 330 insertions(+) create mode 100644 arch/arm/mach-snapdragon/pinctrl-apq8016.c create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.c create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.h create mode 100644 include/dt-bindings/pinctrl/pinctrl-snapdragon.h
diff --git a/arch/arm/mach-snapdragon/Makefile
b/arch/arm/mach-snapdragon/Makefile
index 1c23dc52cf..1d35fea912 100644 --- a/arch/arm/mach-snapdragon/Makefile +++ b/arch/arm/mach-snapdragon/Makefile @@ -6,4 +6,6 @@ obj-$(CONFIG_TARGET_DRAGONBOARD820C) +=
clock-apq8096.o
obj-$(CONFIG_TARGET_DRAGONBOARD820C) += sysmap-apq8096.o obj-$(CONFIG_TARGET_DRAGONBOARD410C) += clock-apq8016.o obj-$(CONFIG_TARGET_DRAGONBOARD410C) += sysmap-apq8016.o +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-apq8016.o +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-snapdragon.o obj-y += clock-snapdragon.o diff --git a/arch/arm/mach-snapdragon/pinctrl-apq8016.c
b/arch/arm/mach-snapdragon/pinctrl-apq8016.c
new file mode 100644 index 0000000000..8e57e2338c --- /dev/null +++ b/arch/arm/mach-snapdragon/pinctrl-apq8016.c @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Qualcomm APQ8016 pinctrl
- (C) Copyright 2018 Ramon Fried ramon.fried@gmail.com
- */
+#include "pinctrl-snapdragon.h" +#include <common.h>
+const char * const msm_pinctrl_pins[] = {
"GPIO_0","GPIO_1","GPIO_2","GPIO_3","GPIO_4","GPIO_5","GPIO_6","GPIO_7",This seems inefficient. Could you not sprintf() the name for most of these values?
The origin of this table is from the Linux kernel driver. I'm not sure I understand how sprintf will more efficient, do you want to fill up this table on runtime ?
I think this table is only used in one function, so you could create the string there perhaps?
Actually, it works the other way around, the generic-pinctrl needs a function to translate string to index. Basically, it reads strings from the FDT and then go over all indexes until it matches that string. this is inefficient IMHO as I think it will be easier just to be able to provide an index instead of a string in the FDT.
OK, so long as the index is actually a known value. If someone changes the DT, won't that fail?
Regards, Simon
I'm not sure I understand the question. If someone changes the DT to an invalid string? Which is not in the table. Then yes. it will fail.
Hi Ramon,
On 16 May 2018 at 02:09, Ramon Fried ramon.fried@gmail.com wrote:
On Tue, May 15, 2018, 7:03 PM Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 15 May 2018 at 07:23, Ramon Fried ramon.fried@gmail.com wrote:
On Mon, May 14, 2018 at 10:51 PM, Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 14 May 2018 at 01:10, Ramon Fried ramon.fried@gmail.com wrote:
On Mon, May 14, 2018 at 1:00 AM, Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 12 May 2018 at 20:15, Ramon Fried ramon.fried@gmail.com wrote: > This patch adds pinmux and pinctrl driver for TLMM > subsystem in snapdragon chipsets. > Currently, supporting only 8016, but implementation is > generic and 8096 can be added easily. > > Driver is using the generic dt-bindings and doesn't > introduce any new bindings (yet). > > Signed-off-by: Ramon Fried ramon.fried@gmail.com > --- > arch/arm/mach-snapdragon/Makefile | 2 + > arch/arm/mach-snapdragon/pinctrl-apq8016.c | 162 > +++++++++++++++++++++++ > arch/arm/mach-snapdragon/pinctrl-snapdragon.c | 118 > +++++++++++++++++ > arch/arm/mach-snapdragon/pinctrl-snapdragon.h | 21 +++ > configs/dragonboard410c_defconfig | 5 + > include/dt-bindings/pinctrl/pinctrl-snapdragon.h | 22 +++ > 6 files changed, 330 insertions(+) > create mode 100644 arch/arm/mach-snapdragon/pinctrl-apq8016.c > create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.c > create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.h > create mode 100644 include/dt-bindings/pinctrl/pinctrl-snapdragon.h > > diff --git a/arch/arm/mach-snapdragon/Makefile > b/arch/arm/mach-snapdragon/Makefile > index 1c23dc52cf..1d35fea912 100644 > --- a/arch/arm/mach-snapdragon/Makefile > +++ b/arch/arm/mach-snapdragon/Makefile > @@ -6,4 +6,6 @@ obj-$(CONFIG_TARGET_DRAGONBOARD820C) += > clock-apq8096.o > obj-$(CONFIG_TARGET_DRAGONBOARD820C) += sysmap-apq8096.o > obj-$(CONFIG_TARGET_DRAGONBOARD410C) += clock-apq8016.o > obj-$(CONFIG_TARGET_DRAGONBOARD410C) += sysmap-apq8016.o > +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-apq8016.o > +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-snapdragon.o > obj-y += clock-snapdragon.o > diff --git a/arch/arm/mach-snapdragon/pinctrl-apq8016.c > b/arch/arm/mach-snapdragon/pinctrl-apq8016.c > new file mode 100644 > index 0000000000..8e57e2338c > --- /dev/null > +++ b/arch/arm/mach-snapdragon/pinctrl-apq8016.c > @@ -0,0 +1,162 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Qualcomm APQ8016 pinctrl > + * > + * (C) Copyright 2018 Ramon Fried ramon.fried@gmail.com > + * > + */ > + > +#include "pinctrl-snapdragon.h" > +#include <common.h> > + > +const char * const msm_pinctrl_pins[] = { > + "GPIO_0", > + "GPIO_1", > + "GPIO_2", > + "GPIO_3", > + "GPIO_4", > + "GPIO_5", > + "GPIO_6", > + "GPIO_7",
This seems inefficient. Could you not sprintf() the name for most of these values?
The origin of this table is from the Linux kernel driver. I'm not sure I understand how sprintf will more efficient, do you want to fill up this table on runtime ?
I think this table is only used in one function, so you could create the string there perhaps?
Actually, it works the other way around, the generic-pinctrl needs a function to translate string to index. Basically, it reads strings from the FDT and then go over all indexes until it matches that string. this is inefficient IMHO as I think it will be easier just to be able to provide an index instead of a string in the FDT.
OK, so long as the index is actually a known value. If someone changes the DT, won't that fail?
Regards, Simon
I'm not sure I understand the question. If someone changes the DT to an invalid string? Which is not in the table. Then yes. it will fail.
I am suggesting that the table could be reduced in size since most of the entries can be generated with sprintf().
Regards, Simon
On Tue, May 15, 2018 at 7:15 PM, Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 16 May 2018 at 02:09, Ramon Fried ramon.fried@gmail.com wrote:
On Tue, May 15, 2018, 7:03 PM Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 15 May 2018 at 07:23, Ramon Fried ramon.fried@gmail.com wrote:
On Mon, May 14, 2018 at 10:51 PM, Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 14 May 2018 at 01:10, Ramon Fried ramon.fried@gmail.com wrote:
On Mon, May 14, 2018 at 1:00 AM, Simon Glass sjg@chromium.org wrote: > Hi Ramon, > > On 12 May 2018 at 20:15, Ramon Fried ramon.fried@gmail.com wrote: >> This patch adds pinmux and pinctrl driver for TLMM >> subsystem in snapdragon chipsets. >> Currently, supporting only 8016, but implementation is >> generic and 8096 can be added easily. >> >> Driver is using the generic dt-bindings and doesn't >> introduce any new bindings (yet). >> >> Signed-off-by: Ramon Fried ramon.fried@gmail.com >> --- >> arch/arm/mach-snapdragon/Makefile | 2 + >> arch/arm/mach-snapdragon/pinctrl-apq8016.c | 162 >> +++++++++++++++++++++++ >> arch/arm/mach-snapdragon/pinctrl-snapdragon.c | 118 >> +++++++++++++++++ >> arch/arm/mach-snapdragon/pinctrl-snapdragon.h | 21 +++ >> configs/dragonboard410c_defconfig | 5 + >> include/dt-bindings/pinctrl/pinctrl-snapdragon.h | 22 +++ >> 6 files changed, 330 insertions(+) >> create mode 100644 arch/arm/mach-snapdragon/pinctrl-apq8016.c >> create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.c >> create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.h >> create mode 100644 include/dt-bindings/pinctrl/pinctrl-snapdragon.h >> >> diff --git a/arch/arm/mach-snapdragon/Makefile >> b/arch/arm/mach-snapdragon/Makefile >> index 1c23dc52cf..1d35fea912 100644 >> --- a/arch/arm/mach-snapdragon/Makefile >> +++ b/arch/arm/mach-snapdragon/Makefile >> @@ -6,4 +6,6 @@ obj-$(CONFIG_TARGET_DRAGONBOARD820C) += >> clock-apq8096.o >> obj-$(CONFIG_TARGET_DRAGONBOARD820C) += sysmap-apq8096.o >> obj-$(CONFIG_TARGET_DRAGONBOARD410C) += clock-apq8016.o >> obj-$(CONFIG_TARGET_DRAGONBOARD410C) += sysmap-apq8016.o >> +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-apq8016.o >> +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-snapdragon.o >> obj-y += clock-snapdragon.o >> diff --git a/arch/arm/mach-snapdragon/pinctrl-apq8016.c >> b/arch/arm/mach-snapdragon/pinctrl-apq8016.c >> new file mode 100644 >> index 0000000000..8e57e2338c >> --- /dev/null >> +++ b/arch/arm/mach-snapdragon/pinctrl-apq8016.c >> @@ -0,0 +1,162 @@ >> +// SPDX-License-Identifier: GPL-2.0+ >> +/* >> + * Qualcomm APQ8016 pinctrl >> + * >> + * (C) Copyright 2018 Ramon Fried ramon.fried@gmail.com >> + * >> + */ >> + >> +#include "pinctrl-snapdragon.h" >> +#include <common.h> >> + >> +const char * const msm_pinctrl_pins[] = { >> + "GPIO_0", >> + "GPIO_1", >> + "GPIO_2", >> + "GPIO_3", >> + "GPIO_4", >> + "GPIO_5", >> + "GPIO_6", >> + "GPIO_7", > > This seems inefficient. Could you not sprintf() the name for most of > these values? The origin of this table is from the Linux kernel driver. I'm not sure I understand how sprintf will more efficient, do you want to fill up this table on runtime ?
I think this table is only used in one function, so you could create the string there perhaps?
Actually, it works the other way around, the generic-pinctrl needs a function to translate string to index. Basically, it reads strings from the FDT and then go over all indexes until it matches that string. this is inefficient IMHO as I think it will be easier just to be able to provide an index instead of a string in the FDT.
OK, so long as the index is actually a known value. If someone changes the DT, won't that fail?
Regards, Simon
I'm not sure I understand the question. If someone changes the DT to an invalid string? Which is not in the table. Then yes. it will fail.
I am suggesting that the table could be reduced in size since most of the entries can be generated with sprintf().
OK. I see. I will try to fix it.
Regards, Simon
Added TLMM pinctrl node for pin muxing & config. Additionally, added a serial node for uart.
Signed-off-by: Ramon Fried ramon.fried@gmail.com --- arch/arm/dts/dragonboard410c.dts | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/arch/arm/dts/dragonboard410c.dts b/arch/arm/dts/dragonboard410c.dts index d9d5831f4f..182a865b0a 100644 --- a/arch/arm/dts/dragonboard410c.dts +++ b/arch/arm/dts/dragonboard410c.dts @@ -8,6 +8,7 @@ /dts-v1/;
#include "skeleton64.dtsi" +#include <dt-bindings/pinctrl/pinctrl-snapdragon.h>
/ { model = "Qualcomm Technologies, Inc. Dragonboard 410c"; @@ -38,6 +39,17 @@ ranges = <0x0 0x0 0x0 0xffffffff>; compatible = "simple-bus";
+ pinctrl: qcom,tlmm@1000000 { + compatible = "qcom,tlmm-apq8016"; + reg = <0x1000000 0x400000>; + + blsp1_uart: uart { + function = "blsp1_uart"; + pins = "GPIO_4", "GPIO_5"; + drive-strength = <DRIVE_STRENGTH_8MA>; + bias-disable; + }; + }; clkc: qcom,gcc@1800000 { compatible = "qcom,gcc-apq8016"; reg = <0x1800000 0x80000>; @@ -49,6 +61,8 @@ compatible = "qcom,msm-uartdm-v1.4"; reg = <0x78b0000 0x200>; clock = <&clkc 4>; + pinctrl-names = "uart"; + pinctrl-0 = <&blsp1_uart>; };
soc_gpios: pinctrl@1000000 {
On 12 May 2018 at 20:15, Ramon Fried ramon.fried@gmail.com wrote:
Added TLMM pinctrl node for pin muxing & config. Additionally, added a serial node for uart.
Signed-off-by: Ramon Fried ramon.fried@gmail.com
arch/arm/dts/dragonboard410c.dts | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Serial port configuration was missing from previous implementation. It only worked because it was preconfigured by LK. This patch configures the uart for 115200 8N1. It also configures the pin mux for uart pins using DT bindings.
Signed-off-by: Ramon Fried ramon.fried@gmail.com --- drivers/serial/serial_msm.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c index 22301c0e37..4a0a2cc450 100644 --- a/drivers/serial/serial_msm.c +++ b/drivers/serial/serial_msm.c @@ -16,6 +16,7 @@ #include <watchdog.h> #include <asm/io.h> #include <linux/compiler.h> +#include <dm/pinctrl.h>
/* Serial registers - this driver works in uartdm mode*/
@@ -25,6 +26,9 @@ #define UARTDM_RXFS 0x50 /* RX channel status register */ #define UARTDM_RXFS_BUF_SHIFT 0x7 /* Number of bytes in the packing buffer */ #define UARTDM_RXFS_BUF_MASK 0x7 +#define UARTDM_MR1 0x00 +#define UARTDM_MR2 0x04 +#define UARTDM_CSR 0xA0
#define UARTDM_SR 0xA4 /* Status register */ #define UARTDM_SR_RX_READY (1 << 0) /* Word is the receiver FIFO */ @@ -45,6 +49,10 @@ #define UARTDM_TF 0x100 /* UART Transmit FIFO register */ #define UARTDM_RF 0x140 /* UART Receive FIFO register */
+#define UART_DM_CLK_RX_TX_BIT_RATE 0xCC +#define MSM_BOOT_UART_DM_8_N_1_MODE 0x34 +#define MSM_BOOT_UART_DM_CMD_RESET_RX 0x10 +#define MSM_BOOT_UART_DM_CMD_RESET_TX 0x20
DECLARE_GLOBAL_DATA_PTR;
@@ -179,6 +187,14 @@ static int msm_uart_clk_init(struct udevice *dev) return 0; }
+static void uart_dm_init(struct msm_serial_data *priv) +{ + writel(UART_DM_CLK_RX_TX_BIT_RATE, priv->base + UARTDM_CSR); + writel(0x0, priv->base + UARTDM_MR1); + writel(MSM_BOOT_UART_DM_8_N_1_MODE, priv->base + UARTDM_MR2); + writel(MSM_BOOT_UART_DM_CMD_RESET_RX, priv->base + UARTDM_CR); + writel(MSM_BOOT_UART_DM_CMD_RESET_TX, priv->base + UARTDM_CR); +} static int msm_serial_probe(struct udevice *dev) { struct msm_serial_data *priv = dev_get_priv(dev); @@ -190,12 +206,8 @@ static int msm_serial_probe(struct udevice *dev) if (msm_uart_clk_init(dev)) return -EINVAL;
- if (readl(priv->base + UARTDM_SR) & UARTDM_SR_UART_OVERRUN) - writel(UARTDM_CR_CMD_RESET_ERR, priv->base + UARTDM_CR); - - writel(0, priv->base + UARTDM_IMR); - writel(UARTDM_CR_CMD_STALE_EVENT_DISABLE, priv->base + UARTDM_CR); - msm_serial_fetch(dev); + pinctrl_select_state(dev, "uart"); + uart_dm_init(priv);
return 0; }
On 12 May 2018 at 20:15, Ramon Fried ramon.fried@gmail.com wrote:
Serial port configuration was missing from previous implementation. It only worked because it was preconfigured by LK. This patch configures the uart for 115200 8N1. It also configures the pin mux for uart pins using DT bindings.
Signed-off-by: Ramon Fried ramon.fried@gmail.com
drivers/serial/serial_msm.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
participants (2)
-
Ramon Fried -
Simon Glass