* Qualcomm Snapdraon serial fixes*

PATCH Superseded Ramon Fried 2 participants 23 messages May 12, 2018 – May 16, 2018

Subsystems (4)

Device tree bindings Arm (32-bit) Arm sub-architectures Pin control

lore

Share & tools

lore

Ramon Fried

author

2018-05-12 10:15 UTC #0 lore raw

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

Signature · 1 line

2.14.1

Ramon Fried

author

2018-05-12 10:15 UTC #1 lore raw

PATCH 1/7 db820c: set clk node to be probed before relocation

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 <[email protected]>

---
 arch/arm/dts/dragonboard820c-uboot.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

arch/arm/dts/dragonboard820c-uboot.dtsi +14 -0

index 88312b3fa1..81df788fca 100644
@@ -5,6 +5,20 @@
  * (C) Copyright 2017 Jorge Ramirez-Ortiz <[email protected]>
  */
 
+/ {
+	soc {
+		u-boot,dm-pre-reloc;
+
+	clock-controller at 300000 {
+		u-boot,dm-pre-reloc;
+	};
+
+	serial at 75b0000 {
+		u-boot,dm-pre-reloc;
+		};
+	};
+};
+
 &pm8994_pon {
 	key_vol_down {
 		gpios = <&pm8994_pon 1 0>;

Signature · 1 line

2.14.1

Simon Glass

2018-05-13 22:00 UTC #2 lore raw

PATCH 1/7 db820c: set clk node to be probed before relocation

Hi Ramon,

Quoted 14 lines · Ramon Fried

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 <[email protected]>
---
 arch/arm/dts/dragonboard820c-uboot.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
arch/arm/dts/dragonboard820c-uboot.dtsi +0 -0
index 88312b3fa1..81df788fca 100644

arch/arm/dts/dragonboard820c-uboot.dtsi line 12

Quoted 8 lines

arch/arm/dts/dragonboard820c-uboot.dtsi +5 -0

@@ -5,6 +5,20 @@
  * (C) Copyright 2017 Jorge Ramirez-Ortiz <[email protected]>
  */
 
+/ {
+       soc {
+               u-boot,dm-pre-reloc;
+
+       clock-controller at 300000 {

This line and a few below it should be indented one more stop.

Quoted 13 lines

arch/arm/dts/dragonboard820c-uboot.dtsi +9 -1

+               u-boot,dm-pre-reloc;
+       };
+
+       serial at 75b0000 {
+               u-boot,dm-pre-reloc;
+               };
+       };
+};
+
 &pm8994_pon {
        key_vol_down {
                gpios = <&pm8994_pon 1 0>;
--

Quoted 2 lines

2.14.1

Regards,
Simon

Ramon Fried

author

2018-05-14 07:06 UTC #3 lore raw

PATCH 1/7 db820c: set clk node to be probed before relocation

Quoted 20 lines · Simon Glass

Hi Ramon,
Quoted 17 lines · Ramon Fried
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 <[email protected]>
---
 arch/arm/dts/dragonboard820c-uboot.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
arch/arm/dts/dragonboard820c-uboot.dtsi +0 -0
index 88312b3fa1..81df788fca 100644
@@ -5,6 +5,20 @@
  * (C) Copyright 2017 Jorge Ramirez-Ortiz <[email protected]>
  */
 

Quoted 7 lines

arch/arm/dts/dragonboard820c-uboot.dtsi line 12
Quoted 5 lines
arch/arm/dts/dragonboard820c-uboot.dtsi +5 -0
+/ {
+       soc {
+               u-boot,dm-pre-reloc;
+
+       clock-controller at 300000 {
This line and a few below it should be indented one more stop.

Sure, will do.

Quoted 11 lines

arch/arm/dts/dragonboard820c-uboot.dtsi +9 -0

+               u-boot,dm-pre-reloc;
+       };
+
+       serial at 75b0000 {
+               u-boot,dm-pre-reloc;
+               };
+       };
+};
+
 &pm8994_pon {

Quoted 8 lines

Quoted 5 lines
arch/arm/dts/dragonboard820c-uboot.dtsi +0 -1
        key_vol_down {
                gpios = <&pm8994_pon 1 0>;
--
2.14.1
Regards,
Simon

Thanks !

Ramon Fried

author

2018-05-12 10:15 UTC #4 lore raw

PATCH 2/7 serial: serial_msm: fail probe if settings clocks fails

In reply to Ramon Fried

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 <[email protected]>

---
 drivers/serial/serial_msm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/serial/serial_msm.c +2 -2

index 119e6b9846..250e48c996 100644
@@ -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);

Signature · 1 line

2.14.1

Simon Glass

2018-05-13 22:00 UTC #5 lore raw

PATCH 2/7 serial: serial_msm: fail probe if settings clocks fails

Hi Ramon,

Quoted 15 lines · Ramon Fried

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 <[email protected]>
---
 drivers/serial/serial_msm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/serial/serial_msm.c +0 -0
index 119e6b9846..250e48c996 100644

drivers/serial/serial_msm.c static int msm_serial_probe(struct udevice *dev) · line 187

Quoted 7 lines

drivers/serial/serial_msm.c +2 -2

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

Would it not be better to return the error that msm_uart_clk_init() returns, rather than -EINVAL?

Quoted 6 lines

drivers/serial/serial_msm.c +0 -1

        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

Ramon Fried

author

2018-05-14 07:07 UTC #6 lore raw

PATCH 2/7 serial: serial_msm: fail probe if settings clocks fails

Quoted 21 lines · Simon Glass

Hi Ramon,
Quoted 18 lines · Ramon Fried
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 <[email protected]>
---
 drivers/serial/serial_msm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/serial/serial_msm.c +0 -0
index 119e6b9846..250e48c996 100644
@@ -183,8 +183,8 @@ static int msm_serial_probe(struct udevice *dev)
 {
        struct msm_serial_data *priv = dev_get_priv(dev);
 

Quoted 7 lines

drivers/serial/serial_msm.c static int msm_serial_probe(struct udevice *dev) · line 187
Quoted 4 lines
drivers/serial/serial_msm.c +2 -2
-       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;
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.

Quoted 9 lines

Quoted 6 lines

drivers/serial/serial_msm.c +0 -1

        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

Ramon Fried

author

2018-05-12 10:15 UTC #7 lore raw

PATCH 3/7 serial: serial_msm: initialize uart only before relocation

In reply to Ramon Fried

The uart is already initialized prior to relocation, reinitialization after relocation is unnecessary.

Signed-off-by Ramon Fried <[email protected]>

---
 drivers/serial/serial_msm.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/serial/serial_msm.c +4 -0

index 250e48c996..22301c0e37 100644
@@ -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;

Signature · 1 line

2.14.1

Simon Glass

2018-05-13 22:00 UTC #8 lore raw

PATCH 3/7 serial: serial_msm: initialize uart only before relocation

Quoted 7 lines · Ramon Fried

The uart is already initialized prior to relocation, reinitialization after relocation is unnecessary.
Signed-off-by Ramon Fried <[email protected]>
---
 drivers/serial/serial_msm.c | 4 ++++
 1 file changed, 4 insertions(+)

Reviewed-by Simon Glass <[email protected]>

But please just use one set of brackets.

Quoted 19 lines

drivers/serial/serial_msm.c +4 -1

index 250e48c996..22301c0e37 100644
@@ -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

Ramon Fried

author

2018-05-12 10:15 UTC #9 lore raw

PATCH 4/7 mach-snapdragon: Fix UART clock flow

In reply to Ramon Fried

UART clock enabling flow was wrong.
Changed the flow according to downstream implementation in LK.

Signed-off-by Ramon Fried <[email protected]>

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

arch/arm/mach-snapdragon/clock-apq8016.c +16 -7

index 9c0cc1c22c..6e4a0ccb90 100644
@@ -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);

arch/arm/mach-snapdragon/clock-apq8096.c +2 -2

index 008649a4c6..628c38785b 100644
@@ -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;

arch/arm/mach-snapdragon/clock-snapdragon.c +16 -1

index f738f57043..85526186c6 100644
@@ -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 */

arch/arm/mach-snapdragon/clock-snapdragon.h +7 -2

index 2cff4f8a06..3ae21099c2 100644
@@ -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,

arch/arm/mach-snapdragon/include/mach/sysmap-apq8016.h +1 -0

index ae784387fa..520e2e6bd7 100644
@@ -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)

Signature · 1 line

2.14.1

Ramon Fried

author

2018-05-12 10:15 UTC #10 lore raw

PATCH 5/7 mach-snapdragon: Introduce pinctrl driver

In reply to Ramon Fried

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 <[email protected]>

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

arch/arm/mach-snapdragon/Makefile +2 -0

index 1c23dc52cf..1d35fea912 100644
@@ -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

arch/arm/mach-snapdragon/pinctrl-apq8016.c new file +162 -0

162 lines not on this page · raw message

arch/arm/mach-snapdragon/pinctrl-snapdragon.c new file +118 -0

118 lines not on this page · raw message

arch/arm/mach-snapdragon/pinctrl-snapdragon.h new file +21 -0

new file mode 100644
index 0000000000..3d0527148a
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Qualcomm Pin control
+ *
+ * (C) Copyright 2018 Ramon Fried <[email protected]>
+ *
+ */
+#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

configs/dragonboard410c_defconfig +5 -0

index e6114db2ce..4b3de64dd5 100644
@@ -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

include/dt-bindings/pinctrl/pinctrl-snapdragon.h new file +22 -0

new file mode 100644
index 0000000000..615affb6f2
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * This header provides constants for Qualcomm Snapdragon pinctrl bindings.
+ *
+ * (C) Copyright 2018 Ramon Fried <[email protected]>
+ *
+ */
+
+#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

Signature · 1 line

2.14.1

Simon Glass

2018-05-13 22:00 UTC #11 lore raw

PATCH 5/7 mach-snapdragon: Introduce pinctrl driver

Hi Ramon,

Quoted 49 lines · Ramon Fried

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 <[email protected]>

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

arch/arm/mach-snapdragon/Makefile +2 -0

index 1c23dc52cf..1d35fea912 100644
@@ -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

arch/arm/mach-snapdragon/pinctrl-apq8016.c new file +10 -0

new file mode 100644
index 0000000000..8e57e2338c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Qualcomm APQ8016 pinctrl
+ *
+ * (C) Copyright 2018 Ramon Fried <[email protected]>
+ *
+ */
+
+#include "pinctrl-snapdragon.h"
+#include <common.h>

arch/arm/mach-snapdragon/pinctrl-apq8016.c line 20

Quoted 10 lines

arch/arm/mach-snapdragon/pinctrl-apq8016.c new file +10 -0

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

Ramon Fried

author

2018-05-14 07:10 UTC #12 lore raw

PATCH 5/7 mach-snapdragon: Introduce pinctrl driver

Quoted 57 lines · Simon Glass

Hi Ramon,

Quoted 54 lines · Ramon Fried

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 <[email protected]>

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

arch/arm/mach-snapdragon/Makefile +2 -0

index 1c23dc52cf..1d35fea912 100644
@@ -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

arch/arm/mach-snapdragon/pinctrl-apq8016.c new file +15 -0

new file mode 100644
index 0000000000..8e57e2338c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Qualcomm APQ8016 pinctrl
+ *
+ * (C) Copyright 2018 Ramon Fried <[email protected]>
+ *
+ */
+
+#include "pinctrl-snapdragon.h"
+#include <common.h>
+
+const char * const msm_pinctrl_pins[] = {
+       "GPIO_0",
+       "GPIO_1",
+       "GPIO_2",

Quoted 8 lines

arch/arm/mach-snapdragon/pinctrl-apq8016.c line 20
Quoted 5 lines
arch/arm/mach-snapdragon/pinctrl-apq8016.c new file +5 -0
+       "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 ?

Quoted 3 lines

Regards,
Simon

Simon Glass

2018-05-14 19:51 UTC #13 lore raw

PATCH 5/7 mach-snapdragon: Introduce pinctrl driver

Hi Ramon,

Quoted 66 lines · Ramon Fried

Quoted 57 lines · Simon Glass

Hi Ramon,

Quoted 54 lines · Ramon Fried

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 <[email protected]>

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

arch/arm/mach-snapdragon/Makefile +2 -0

index 1c23dc52cf..1d35fea912 100644
@@ -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

arch/arm/mach-snapdragon/pinctrl-apq8016.c new file +15 -0

new file mode 100644
index 0000000000..8e57e2338c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Qualcomm APQ8016 pinctrl
+ *
+ * (C) Copyright 2018 Ramon Fried <[email protected]>
+ *
+ */
+
+#include "pinctrl-snapdragon.h"
+#include <common.h>
+
+const char * const msm_pinctrl_pins[] = {
+       "GPIO_0",
+       "GPIO_1",
+       "GPIO_2",

Quoted 8 lines

arch/arm/mach-snapdragon/pinctrl-apq8016.c line 20
Quoted 5 lines
arch/arm/mach-snapdragon/pinctrl-apq8016.c new file +5 -0
+       "GPIO_3",
+       "GPIO_4",
+       "GPIO_5",
+       "GPIO_6",
+       "GPIO_7",
This seems inefficient. Could you not sprintf() the name for most of these values?

Quoted 3 lines

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

Ramon Fried

author

2018-05-14 21:23 UTC #14 lore raw

PATCH 5/7 mach-snapdragon: Introduce pinctrl driver

Quoted 69 lines · Simon Glass

Hi Ramon,

Quoted 66 lines · Ramon Fried

Quoted 65 lines · Simon Glass

Hi Ramon,

Quoted 49 lines · Ramon Fried

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 <[email protected]>

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

arch/arm/mach-snapdragon/Makefile +2 -0

index 1c23dc52cf..1d35fea912 100644
@@ -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

arch/arm/mach-snapdragon/pinctrl-apq8016.c new file +10 -0

new file mode 100644
index 0000000000..8e57e2338c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Qualcomm APQ8016 pinctrl
+ *
+ * (C) Copyright 2018 Ramon Fried <[email protected]>
+ *
+ */
+
+#include "pinctrl-snapdragon.h"
+#include <common.h>

arch/arm/mach-snapdragon/pinctrl-apq8016.c line 20

Quoted 10 lines

arch/arm/mach-snapdragon/pinctrl-apq8016.c new file +10 -0

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

Quoted 7 lines

Quoted 3 lines
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.

Quoted 2 lines

Regards,
Simon

Simon Glass

2018-05-15 16:02 UTC #15 lore raw

PATCH 5/7 mach-snapdragon: Introduce pinctrl driver

Hi Ramon,

Quoted 77 lines · Ramon Fried

Quoted 69 lines · Simon Glass
Hi Ramon,
Quoted 66 lines · Ramon Fried
Quoted 65 lines · Simon Glass
Hi Ramon,
Quoted 49 lines · Ramon Fried
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 <[email protected]>
---
 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
arch/arm/mach-snapdragon/Makefile +2 -0
index 1c23dc52cf..1d35fea912 100644
@@ -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
arch/arm/mach-snapdragon/pinctrl-apq8016.c new file +10 -0
new file mode 100644
index 0000000000..8e57e2338c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Qualcomm APQ8016 pinctrl
+ *
+ * (C) Copyright 2018 Ramon Fried <[email protected]>
+ *
+ */
+
+#include "pinctrl-snapdragon.h"
+#include <common.h>
arch/arm/mach-snapdragon/pinctrl-apq8016.c line 20
Quoted 10 lines
arch/arm/mach-snapdragon/pinctrl-apq8016.c new file +10 -0
+
+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?
Quoted 7 lines
Quoted 3 lines
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?

Quoted 6 lines

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

Ramon Fried

author

2018-05-15 16:09 UTC #16 lore raw

PATCH 5/7 mach-snapdragon: Introduce pinctrl driver

Quoted 92 lines · Simon Glass

Hi Ramon,
Quoted 13 lines · Ramon Fried
Quoted 12 lines · Simon Glass
Hi Ramon,
Quoted 9 lines · Ramon Fried
Quoted 8 lines · Simon Glass
Hi Ramon,
Quoted 5 lines · Ramon Fried
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.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Driver is using the generic dt-bindings and doesn't introduce any new bindings (yet).
Signed-off-by Ramon Fried <[email protected]>
---
 arch/arm/mach-snapdragon/Makefile                |   2 +
 arch/arm/mach-snapdragon/pinctrl-apq8016.c       | 162
+++++++++++++++++++++++
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
 arch/arm/mach-snapdragon/pinctrl-snapdragon.c    | 118
+++++++++++++++++
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Review
Quoted 10 lines
 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
(quoted diff) +0 -0
b/arch/arm/mach-snapdragon/Makefile
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
arch/arm/mach-snapdragon/Makefile obj-$(CONFIG_TARGET_DRAGONBOARD820C) +=
Quoted 4 lines
index 1c23dc52cf..1d35fea912 100644
arch/arm/mach-snapdragon/Makefile +0 -0
@@ -6,4 +6,6 @@ obj-$(CONFIG_TARGET_DRAGONBOARD820C) +=
clock-apq8096.o
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Review
Quoted 7 lines
arch/arm/mach-snapdragon/Makefile +2 -0
 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
(quoted diff) +0 -0
b/arch/arm/mach-snapdragon/pinctrl-apq8016.c
Quoted 35 lines
Quoted 28 lines
Quoted 28 lines
Quoted 28 lines
Quoted 15 lines
new file mode 100644
index 0000000000..8e57e2338c
arch/arm/mach-snapdragon/pinctrl-apq8016.c new file +10 -0
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Qualcomm APQ8016 pinctrl
+ *
+ * (C) Copyright 2018 Ramon Fried <[email protected]>
+ *
+ */
+
+#include "pinctrl-snapdragon.h"
+#include <common.h>
arch/arm/mach-snapdragon/pinctrl-apq8016.c line 20
Quoted 10 lines
arch/arm/mach-snapdragon/pinctrl-apq8016.c new file +10 -0
+
+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?
Quoted 7 lines
Quoted 3 lines
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?
Quoted 6 lines
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.

Quoted 7 lines

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.

Quoted 1 line

Simon Glass

2018-05-15 16:15 UTC #17 lore raw

PATCH 5/7 mach-snapdragon: Introduce pinctrl driver

Hi Ramon,

Quoted 100 lines · Ramon Fried

Quoted 97 lines · Simon Glass
Hi Ramon,
Quoted 82 lines · Ramon Fried
Quoted 74 lines · Simon Glass
Hi Ramon,
Quoted 71 lines · Ramon Fried
Quoted 70 lines · Simon Glass
Hi Ramon,
Quoted 54 lines · Ramon Fried
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 <[email protected]>
---
 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
arch/arm/mach-snapdragon/Makefile +0 -0
index 1c23dc52cf..1d35fea912 100644
@@ -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
arch/arm/mach-snapdragon/Makefile +2 -0
+obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-apq8016.o
+obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-snapdragon.o
 obj-y += clock-snapdragon.o
arch/arm/mach-snapdragon/pinctrl-apq8016.c new file +10 -0
new file mode 100644
index 0000000000..8e57e2338c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Qualcomm APQ8016 pinctrl
+ *
+ * (C) Copyright 2018 Ramon Fried <[email protected]>
+ *
+ */
+
+#include "pinctrl-snapdragon.h"
+#include <common.h>
arch/arm/mach-snapdragon/pinctrl-apq8016.c line 20
Quoted 10 lines
arch/arm/mach-snapdragon/pinctrl-apq8016.c new file +10 -0
+
+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?
Quoted 7 lines
Quoted 3 lines
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?
Quoted 6 lines
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?

Quoted 5 lines

Quoted 2 lines
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

Ramon Fried

author

2018-05-16 05:37 UTC #18 lore raw

PATCH 5/7 mach-snapdragon: Introduce pinctrl driver

Quoted 103 lines · Simon Glass

Hi Ramon,
Quoted 100 lines · Ramon Fried
Quoted 97 lines · Simon Glass
Hi Ramon,
Quoted 82 lines · Ramon Fried
Quoted 74 lines · Simon Glass
Hi Ramon,
Quoted 71 lines · Ramon Fried
Quoted 70 lines · Simon Glass
Hi Ramon,
Quoted 54 lines · Ramon Fried
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 <[email protected]>
---
 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
arch/arm/mach-snapdragon/Makefile +0 -0
index 1c23dc52cf..1d35fea912 100644
@@ -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
arch/arm/mach-snapdragon/Makefile +2 -0
+obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-apq8016.o
+obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-snapdragon.o
 obj-y += clock-snapdragon.o
arch/arm/mach-snapdragon/pinctrl-apq8016.c new file +10 -0
new file mode 100644
index 0000000000..8e57e2338c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Qualcomm APQ8016 pinctrl
+ *
+ * (C) Copyright 2018 Ramon Fried <[email protected]>
+ *
+ */
+
+#include "pinctrl-snapdragon.h"
+#include <common.h>
arch/arm/mach-snapdragon/pinctrl-apq8016.c line 20
Quoted 10 lines
arch/arm/mach-snapdragon/pinctrl-apq8016.c new file +10 -0
+
+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?
Quoted 7 lines
Quoted 3 lines
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?
Quoted 6 lines
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?

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
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.

Quoted 3 lines

Regards,
Simon

Ramon Fried

author

2018-05-12 10:15 UTC #19 lore raw

PATCH 6/7 db410: added pinctrl node and serial bindings

In reply to Ramon Fried

Added TLMM pinctrl node for pin muxing & config.
Additionally, added a serial node for uart.

Signed-off-by Ramon Fried <[email protected]>

---
 arch/arm/dts/dragonboard410c.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

arch/arm/dts/dragonboard410c.dts +14 -0

index d9d5831f4f..182a865b0a 100644
@@ -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 at 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 at 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 at 1000000 {

Signature · 1 line

2.14.1

Simon Glass

2018-05-13 22:00 UTC #20 lore raw

PATCH 6/7 db410: added pinctrl node and serial bindings

Quoted 8 lines · Ramon Fried

Added TLMM pinctrl node for pin muxing & config.
Additionally, added a serial node for uart.
Signed-off-by Ramon Fried <[email protected]>
---
 arch/arm/dts/dragonboard410c.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Reviewed-by Simon Glass <[email protected]>

Ramon Fried

author

2018-05-12 10:15 UTC #21 lore raw

PATCH 7/7 serial: serial_msm: added pinmux & config

In reply to Ramon Fried

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 <[email protected]>

---
 drivers/serial/serial_msm.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

drivers/serial/serial_msm.c +18 -6

index 22301c0e37..4a0a2cc450 100644
@@ -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;
 }

Signature · 1 line

2.14.1

Simon Glass

2018-05-13 22:00 UTC #22 lore raw

PATCH 7/7 serial: serial_msm: added pinmux & config

Quoted 9 lines · Ramon Fried

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 <[email protected]>
---
 drivers/serial/serial_msm.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

Reviewed-by Simon Glass <[email protected]>