[U-Boot] [PATCH 0/7] *** Qualcomm Snapdraon serial fixes***

[U-Boot] [PATCH 0/7] *** Qualcomm Snapdraon serial fixes***

[Ramon Fried]

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

-- 
2.14.1

[U-Boot] [PATCH 1/7] db820c: set clk node to be probed before relocation

[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 <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 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>;
-- 
2.14.1

[U-Boot] [PATCH 2/7] serial: serial_msm: fail probe if settings clocks fails

[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 <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);
-- 
2.14.1

[U-Boot] [PATCH 3/7] serial: serial_msm: initialize uart only before relocation

[Ramon Fried]

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

[U-Boot] [PATCH 4/7] mach-snapdragon: Fix UART clock flow

[Ramon Fried]

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

[U-Boot] [PATCH 5/7] mach-snapdragon: Introduce pinctrl driver

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

[U-Boot] [PATCH 6/7] db410: added pinctrl node and serial bindings

[Ramon Fried]

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

[U-Boot] [PATCH 7/7] serial: serial_msm: added pinmux & config

[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 <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;
 }
-- 
2.14.1

[U-Boot] [PATCH 2/7] serial: serial_msm: fail probe if settings clocks fails

[Simon Glass]

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

[U-Boot] [PATCH 5/7] mach-snapdragon: Introduce pinctrl driver

[Simon Glass]

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

[U-Boot] [PATCH 1/7] db820c: set clk node to be probed before relocation

[Simon Glass]

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 at 300000 {

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

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

Regards,
Simon

[U-Boot] [PATCH 3/7] serial: serial_msm: initialize uart only before relocation

[Simon Glass]

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
>

[U-Boot] [PATCH 6/7] db410: added pinctrl node and serial bindings

[Simon Glass]

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>

[U-Boot] [PATCH 7/7] serial: serial_msm: added pinmux & config

[Simon Glass]

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>

[U-Boot] [PATCH 1/7] db820c: set clk node to be probed before relocation

[Ramon Fried]

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 at 300000 {
>
> This line and a few below it should be indented one more stop.
Sure, will do.
>
>> +               u-boot,dm-pre-reloc;
>> +       };
>> +
>> +       serial at 75b0000 {
>> +               u-boot,dm-pre-reloc;
>> +               };
>> +       };
>> +};
>> +
>>  &pm8994_pon {
>>         key_vol_down {
>>                 gpios = <&pm8994_pon 1 0>;
>> --
>> 2.14.1
>>
>
> Regards,
> Simon
Thanks !

[U-Boot] [PATCH 2/7] serial: serial_msm: fail probe if settings clocks fails

[Ramon Fried]

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

[U-Boot] [PATCH 5/7] mach-snapdragon: Introduce pinctrl driver

[Ramon Fried]

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

[U-Boot] [PATCH 5/7] mach-snapdragon: Introduce pinctrl driver

[Simon Glass]

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

[U-Boot] [PATCH 5/7] mach-snapdragon: Introduce pinctrl driver

[Ramon Fried]

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

[U-Boot] [PATCH 5/7] mach-snapdragon: Introduce pinctrl driver

[Simon Glass]

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

[U-Boot] [PATCH 5/7] mach-snapdragon: Introduce pinctrl driver

[Ramon Fried]

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.

>

[U-Boot] [PATCH 5/7] mach-snapdragon: Introduce pinctrl driver

[Simon Glass]

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

[U-Boot] [PATCH 5/7] mach-snapdragon: Introduce pinctrl driver

[Ramon Fried]

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