linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC v2 0/8] Pinctrl support for Zynq
@ 2014-10-16 17:11 Soren Brinkmann
  2014-10-16 17:11 ` [PATCH RFC v2 1/8] pinctrl: Add driver " Soren Brinkmann
                   ` (7 more replies)
  0 siblings, 8 replies; 30+ messages in thread
From: Soren Brinkmann @ 2014-10-16 17:11 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Sören Brinkmann, Michal Simek, linux-kernel,
	linux-arm-kernel, Steffen Trumtrar

Hi,

this is the second iteration of pinctrl for Zynq (first submission:
https://lkml.org/lkml/2014/9/24/1140).

The high level changes:
 - rebase to 3.18, i.e. usage of 'groups' property in DT
 - added pinconf functionality
 - revised the DT descriptions

In more detail:
pinctrl: pinconf-generic: Add flag to print arguments
  There are some generic properties that take an argument but don't have
  a unit attached to it. Furthermore, printing the argument depended on
  the argument being non-zero, which is also not always useful. To add a
  little more flexibility, this patch adds a dedicated flag to indicate
  whether a property takes an argument that should be printed. The first
  example is the slew-rate property, that according to bindings takes a
  driver specific argument, but does not have a unit. So, printing it
  including a zero argument does make sense.

pinctrl: pinconf-generic: Add parameter 'IO standard':
  I guess this is mostly seld-explanatory. On Zynq we can select between
  different IO-standards.

pinctrl: pinconf-generic: Infer map type from DT property:
  This is the important one. Before this drivers could choose between
  pinconf_generic_dt_node_to_map_group() and
  pinconf_generic_dt_node_to_map_pin() to create mapping from DT.
  Choosing groups would make the core interpret the 'pins' property as
  group, and you lost the ability to address by pin. And vice versa,
  using the pin function would force using pins and you lose the ability
  to address groups.
  With the new 'groups' DT property, this is no longer needed and both
  options can coexist by inferring the map type by testing whether the
  pins or groups property was used.
  To make this change backwards-compatible with current usage, this
  inferring is only done when PIN_MAP_TYPE_INVALID is passed to the
  mapping function. For that reason pinconf_generic_dt_node_to_map_all()
  is introduced, which does exactly that.
  With this change it is possible to have nodes in DT that work on
  pingroups using the 'grops' property as well has nodes using 'pins'
  that address specific pins.

ARM: zynq: DT: Add pinctrl information:
  I completely rewrote this taking the v1 input into account. The common
  dtsi file does only feature the pin-controller without any
  configuration nodes. The board files define the used configurations.
  To configure all the muxes and configurations the new core features
  are used.

	Thanks,
	Sören

Soren Brinkmann (8):
  pinctrl: Add driver for Zynq
  pinctrl: pinconf-generic: Add flag to print arguments
  pinctrl: pinconf-generic: Add parameter 'IO standard'
  pinctrl: zynq: Support IO standard property
  pinctrl: zynq: Support low power mode property
  pinctrl: pinconf-generic: Infer map type from DT property
  pinctrl: zynq: Use generic map_all function
  ARM: zynq: DT: Add pinctrl information

 .../bindings/pinctrl/pinctrl-bindings.txt          |    3 +
 arch/arm/boot/dts/zynq-7000.dtsi                   |    8 +-
 arch/arm/boot/dts/zynq-zc702.dts                   |  147 +++
 arch/arm/boot/dts/zynq-zc706.dts                   |  126 +++
 arch/arm/mach-zynq/Kconfig                         |    1 +
 drivers/pinctrl/Kconfig                            |    8 +
 drivers/pinctrl/Makefile                           |    1 +
 drivers/pinctrl/pinconf-generic.c                  |   84 +-
 drivers/pinctrl/pinctrl-zynq.c                     | 1139 ++++++++++++++++++++
 include/linux/pinctrl/pinconf-generic.h            |   11 +
 10 files changed, 1496 insertions(+), 32 deletions(-)
 create mode 100644 drivers/pinctrl/pinctrl-zynq.c

-- 
2.1.2.1.g5e69ed6


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

* [PATCH RFC v2 1/8] pinctrl: Add driver for Zynq
  2014-10-16 17:11 [PATCH RFC v2 0/8] Pinctrl support for Zynq Soren Brinkmann
@ 2014-10-16 17:11 ` Soren Brinkmann
  2014-10-28 14:53   ` Linus Walleij
  2014-10-28 15:16   ` Lothar Waßmann
  2014-10-16 17:11 ` [PATCH RFC v2 2/8] pinctrl: pinconf-generic: Add flag to print arguments Soren Brinkmann
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 30+ messages in thread
From: Soren Brinkmann @ 2014-10-16 17:11 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Sören Brinkmann, Michal Simek, linux-kernel,
	linux-arm-kernel, Steffen Trumtrar

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
changes since RFC:
 - use syscon/regmap to access registers in SLCR space
 - add pinctrl to zc702 DT
 - rebase to 3.18: rename enable -> set_mux
 - add kernel-doc
 - support pinconf
   - supported attributes
     - pin-bias: pull up, tristate, disable
     - slew-rate: 0 == slow, 1 == fast; generic pinconf does not display
       argument

---
 arch/arm/mach-zynq/Kconfig     |    1 +
 drivers/pinctrl/Kconfig        |    8 +
 drivers/pinctrl/Makefile       |    1 +
 drivers/pinctrl/pinctrl-zynq.c | 1091 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 1101 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-zynq.c

diff --git a/arch/arm/mach-zynq/Kconfig b/arch/arm/mach-zynq/Kconfig
index aaa5162c1509..13b8524354ee 100644
--- a/arch/arm/mach-zynq/Kconfig
+++ b/arch/arm/mach-zynq/Kconfig
@@ -9,6 +9,7 @@ config ARCH_ZYNQ
 	select HAVE_ARM_TWD if SMP
 	select ICST
 	select MFD_SYSCON
+	select PINCTRL
 	select SOC_BUS
 	help
 	  Support for Xilinx Zynq ARM Cortex A9 Platform
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index c6a66de6ed72..33cbae0a7f6f 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -203,6 +203,14 @@ config PINCTRL_PALMAS
 	  open drain configuration for the Palmas series devices like
 	  TPS65913, TPS80036 etc.
 
+config PINCTRL_ZYNQ
+	bool "Pinctrl driver for Xilinx Zynq"
+	depends on ARCH_ZYNQ
+	select PINMUX
+	select GENERIC_PINCONF
+	help
+	  This selectes the pinctrl driver for Xilinx Zynq.
+
 source "drivers/pinctrl/berlin/Kconfig"
 source "drivers/pinctrl/freescale/Kconfig"
 source "drivers/pinctrl/mvebu/Kconfig"
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 51f52d32859e..aa999cf57f04 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_PINCTRL_XWAY)	+= pinctrl-xway.o
 obj-$(CONFIG_PINCTRL_LANTIQ)	+= pinctrl-lantiq.o
 obj-$(CONFIG_PINCTRL_TB10X)	+= pinctrl-tb10x.o
 obj-$(CONFIG_PINCTRL_ST) 	+= pinctrl-st.o
+obj-$(CONFIG_PINCTRL_ZYNQ)	+= pinctrl-zynq.o
 
 obj-$(CONFIG_ARCH_BERLIN)	+= berlin/
 obj-y				+= freescale/
diff --git a/drivers/pinctrl/pinctrl-zynq.c b/drivers/pinctrl/pinctrl-zynq.c
new file mode 100644
index 000000000000..a32fac61cba0
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-zynq.c
@@ -0,0 +1,1091 @@
+/*
+ * Zynq pin controller
+ *
+ *  Copyright (C) 2014 Xilinx
+ *
+ *  Sören Brinkmann <soren.brinkmann@xilinx.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/regmap.h>
+#include "pinctrl-utils.h"
+#include "core.h"
+
+#define ZYNQ_PCTRL_MIO_MST_TRI0	0x10c
+#define ZYNQ_PCTRL_MIO_MST_TRI1	0x110
+
+#define ZYNQ_PINMUX_MUX_SHIFT	1
+#define ZYNQ_PINMUX_MUX_MASK	(0x7f << ZYNQ_PINMUX_MUX_SHIFT)
+
+/**
+ * struct zynq_pinctrl - driver data
+ * @pctrl:		Pinctrl device
+ * @syscon:		Syscon regmap
+ * @pctrl_offset:	Offset for pinctrl into the @syscon space
+ * @groups:		Pingroups
+ * @ngroupos:		Number of @groups
+ * @funcs:		Pinmux functions
+ * @nfuncs:		Number of @funcs
+ */
+struct zynq_pinctrl {
+	struct pinctrl_dev *pctrl;
+	struct regmap *syscon;
+	u32 pctrl_offset;
+	const struct zynq_pctrl_group *groups;
+	unsigned int ngroups;
+	const struct zynq_pinmux_function *funcs;
+	unsigned int nfuncs;
+};
+
+struct zynq_pctrl_group {
+	const char *name;
+	const unsigned int *pins;
+	const unsigned npins;
+};
+
+/**
+ * struct zynq_pinmux_function - a pinmux function
+ * @name:	Name of the pinmux function.
+ * @groups:	List of pingroups for this function.
+ * @ngroups:	Number of entries in @groups.
+ * @mux_val:	Selector for this function
+ * @mux:	Offset of function specific mux
+ * @mux_mask:	Mask for function specific selector
+ * @mux_shift:	Shift for function specific selector
+ */
+struct zynq_pinmux_function {
+	const char *name;
+	const char * const *groups;
+	unsigned int ngroups;
+	unsigned int mux_val;
+	u32 mux;
+	u32 mux_mask;
+	u8 mux_shift;
+};
+
+enum zynq_pinmux_functions {
+	ZYNQ_PMUX_ethernet0,
+	ZYNQ_PMUX_ethernet1,
+	ZYNQ_PMUX_mdio0,
+	ZYNQ_PMUX_mdio1,
+	ZYNQ_PMUX_qspi0,
+	ZYNQ_PMUX_qspi1,
+	ZYNQ_PMUX_qspi_fbclk,
+	ZYNQ_PMUX_qspi_cs1,
+	ZYNQ_PMUX_spi0,
+	ZYNQ_PMUX_spi1,
+	ZYNQ_PMUX_sdio0,
+	ZYNQ_PMUX_sdio0_pc,
+	ZYNQ_PMUX_sdio0_cd,
+	ZYNQ_PMUX_sdio0_wp,
+	ZYNQ_PMUX_sdio1,
+	ZYNQ_PMUX_sdio1_pc,
+	ZYNQ_PMUX_sdio1_cd,
+	ZYNQ_PMUX_sdio1_wp,
+	ZYNQ_PMUX_smc0_nor,
+	ZYNQ_PMUX_smc0_nor_cs1,
+	ZYNQ_PMUX_smc0_nor_addr25,
+	ZYNQ_PMUX_smc0_nand,
+	ZYNQ_PMUX_can0,
+	ZYNQ_PMUX_can1,
+	ZYNQ_PMUX_uart0,
+	ZYNQ_PMUX_uart1,
+	ZYNQ_PMUX_i2c0,
+	ZYNQ_PMUX_i2c1,
+	ZYNQ_PMUX_ttc0,
+	ZYNQ_PMUX_ttc1,
+	ZYNQ_PMUX_swdt0,
+	ZYNQ_PMUX_gpio0,
+	ZYNQ_PMUX_MAX_FUNC
+};
+
+const struct pinctrl_pin_desc zynq_pins[] = {
+	PINCTRL_PIN(0,  "MIO0"),
+	PINCTRL_PIN(1,  "MIO1"),
+	PINCTRL_PIN(2,  "MIO2"),
+	PINCTRL_PIN(3,  "MIO3"),
+	PINCTRL_PIN(4,  "MIO4"),
+	PINCTRL_PIN(5,  "MIO5"),
+	PINCTRL_PIN(6,  "MIO6"),
+	PINCTRL_PIN(7,  "MIO7"),
+	PINCTRL_PIN(8,  "MIO8"),
+	PINCTRL_PIN(9,  "MIO9"),
+	PINCTRL_PIN(10, "MIO10"),
+	PINCTRL_PIN(11, "MIO11"),
+	PINCTRL_PIN(12, "MIO12"),
+	PINCTRL_PIN(13, "MIO13"),
+	PINCTRL_PIN(14, "MIO14"),
+	PINCTRL_PIN(15, "MIO15"),
+	PINCTRL_PIN(16, "MIO16"),
+	PINCTRL_PIN(17, "MIO17"),
+	PINCTRL_PIN(18, "MIO18"),
+	PINCTRL_PIN(19, "MIO19"),
+	PINCTRL_PIN(20, "MIO20"),
+	PINCTRL_PIN(21, "MIO21"),
+	PINCTRL_PIN(22, "MIO22"),
+	PINCTRL_PIN(23, "MIO23"),
+	PINCTRL_PIN(24, "MIO24"),
+	PINCTRL_PIN(25, "MIO25"),
+	PINCTRL_PIN(26, "MIO26"),
+	PINCTRL_PIN(27, "MIO27"),
+	PINCTRL_PIN(28, "MIO28"),
+	PINCTRL_PIN(29, "MIO29"),
+	PINCTRL_PIN(30, "MIO30"),
+	PINCTRL_PIN(31, "MIO31"),
+	PINCTRL_PIN(32, "MIO32"),
+	PINCTRL_PIN(33, "MIO33"),
+	PINCTRL_PIN(34, "MIO34"),
+	PINCTRL_PIN(35, "MIO35"),
+	PINCTRL_PIN(36, "MIO36"),
+	PINCTRL_PIN(37, "MIO37"),
+	PINCTRL_PIN(38, "MIO38"),
+	PINCTRL_PIN(39, "MIO39"),
+	PINCTRL_PIN(40, "MIO40"),
+	PINCTRL_PIN(41, "MIO41"),
+	PINCTRL_PIN(42, "MIO42"),
+	PINCTRL_PIN(43, "MIO43"),
+	PINCTRL_PIN(44, "MIO44"),
+	PINCTRL_PIN(45, "MIO45"),
+	PINCTRL_PIN(46, "MIO46"),
+	PINCTRL_PIN(47, "MIO47"),
+	PINCTRL_PIN(48, "MIO48"),
+	PINCTRL_PIN(49, "MIO49"),
+	PINCTRL_PIN(50, "MIO50"),
+	PINCTRL_PIN(51, "MIO51"),
+	PINCTRL_PIN(52, "MIO52"),
+	PINCTRL_PIN(53, "MIO53"),
+	PINCTRL_PIN(54, "EMIO_SD0_WP"),
+	PINCTRL_PIN(55, "EMIO_SD0_CD"),
+	PINCTRL_PIN(56, "EMIO_SD1_WP"),
+	PINCTRL_PIN(57, "EMIO_SD0_CD"),
+};
+
+/* pin groups */
+static const unsigned int ethernet0_0_pins[] = {16, 17, 18, 19, 20, 21, 22, 23,
+						24, 25, 26, 27};
+static const unsigned int ethernet1_0_pins[] = {28, 29, 30, 31, 32, 33, 34, 35,
+						36, 37, 38, 39};
+static const unsigned int mdio0_0_pins[] = {52, 53};
+static const unsigned int mdio1_0_pins[] = {52, 53};
+static const unsigned int qspi0_0_pins[] = {1, 2, 3, 4, 5, 6};
+
+static const unsigned int qspi1_0_pins[] = {9, 10, 11, 12, 13};
+static const unsigned int qspi_cs1_pins[] = {0};
+static const unsigned int qspi_fbclk_pins[] = {8};
+static const unsigned int spi0_0_pins[] = {16, 17, 18, 19, 20, 21};
+static const unsigned int spi0_1_pins[] = {28, 29, 30, 31, 32, 33};
+static const unsigned int spi0_2_pins[] = {40, 41, 42, 43, 44, 45};
+static const unsigned int spi1_0_pins[] = {10, 11, 12, 13, 14, 15};
+static const unsigned int spi1_1_pins[] = {22, 23, 24, 25, 26, 27};
+static const unsigned int spi1_2_pins[] = {34, 35, 36, 37, 38, 39};
+static const unsigned int spi1_3_pins[] = {46, 47, 48, 49, 40, 51};
+static const unsigned int sdio0_0_pins[] = {16, 17, 18, 19, 20, 21};
+static const unsigned int sdio0_1_pins[] = {28, 29, 30, 31, 32, 33};
+static const unsigned int sdio0_2_pins[] = {40, 41, 42, 43, 44, 45};
+static const unsigned int sdio1_0_pins[] = {10, 11, 12, 13, 14, 15};
+static const unsigned int sdio1_1_pins[] = {22, 23, 24, 25, 26, 27};
+static const unsigned int sdio1_2_pins[] = {34, 35, 36, 37, 38, 39};
+static const unsigned int sdio1_3_pins[] = {46, 47, 48, 49, 40, 51};
+static const unsigned int sdio0_emio_wp_pins[] = {54};
+static const unsigned int sdio0_emio_cd_pins[] = {55};
+static const unsigned int sdio1_emio_wp_pins[] = {56};
+static const unsigned int sdio1_emio_cd_pins[] = {57};
+static const unsigned int smc0_nor_pins[] = {0, 3, 4, 5, 6, 7, 8, 9, 10, 11, 13,
+					     15, 16, 17, 18, 19, 20, 21, 22, 23,
+					     24, 25, 26, 27, 28, 29, 30, 31, 32,
+					     33, 34, 35, 36, 37, 38, 39};
+static const unsigned int smc0_nor_cs1_pins[] = {1};
+static const unsigned int smc0_nor_addr25_pins[] = {1};
+static const unsigned int smc0_nand_pins[] = {0, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11,
+					      12, 13, 14, 16, 17, 18, 19, 20,
+					      21, 22, 23};
+/* Note: CAN MIO clock inputs are modeled in the clock framework */
+static const unsigned int can0_0_pins[] = {10, 11};
+static const unsigned int can0_1_pins[] = {14, 15};
+static const unsigned int can0_2_pins[] = {18, 19};
+static const unsigned int can0_3_pins[] = {22, 23};
+static const unsigned int can0_4_pins[] = {26, 27};
+static const unsigned int can0_5_pins[] = {30, 31};
+static const unsigned int can0_6_pins[] = {34, 35};
+static const unsigned int can0_7_pins[] = {38, 39};
+static const unsigned int can0_8_pins[] = {42, 43};
+static const unsigned int can0_9_pins[] = {46, 47};
+static const unsigned int can0_10_pins[] = {50, 51};
+static const unsigned int can1_0_pins[] = {8, 9};
+static const unsigned int can1_1_pins[] = {12, 13};
+static const unsigned int can1_2_pins[] = {16, 17};
+static const unsigned int can1_3_pins[] = {20, 21};
+static const unsigned int can1_4_pins[] = {24, 25};
+static const unsigned int can1_5_pins[] = {28, 29};
+static const unsigned int can1_6_pins[] = {32, 33};
+static const unsigned int can1_7_pins[] = {36, 37};
+static const unsigned int can1_8_pins[] = {40, 41};
+static const unsigned int can1_9_pins[] = {44, 45};
+static const unsigned int can1_10_pins[] = {48, 49};
+static const unsigned int can1_11_pins[] = {52, 53};
+static const unsigned int uart0_0_pins[] = {10, 11};
+static const unsigned int uart0_1_pins[] = {14, 15};
+static const unsigned int uart0_2_pins[] = {18, 19};
+static const unsigned int uart0_3_pins[] = {22, 23};
+static const unsigned int uart0_4_pins[] = {26, 27};
+static const unsigned int uart0_5_pins[] = {30, 31};
+static const unsigned int uart0_6_pins[] = {34, 35};
+static const unsigned int uart0_7_pins[] = {38, 39};
+static const unsigned int uart0_8_pins[] = {42, 43};
+static const unsigned int uart0_9_pins[] = {46, 47};
+static const unsigned int uart0_10_pins[] = {50, 51};
+static const unsigned int uart1_0_pins[] = {8, 9};
+static const unsigned int uart1_1_pins[] = {12, 13};
+static const unsigned int uart1_2_pins[] = {16, 17};
+static const unsigned int uart1_3_pins[] = {20, 21};
+static const unsigned int uart1_4_pins[] = {24, 25};
+static const unsigned int uart1_5_pins[] = {28, 29};
+static const unsigned int uart1_6_pins[] = {32, 33};
+static const unsigned int uart1_7_pins[] = {36, 37};
+static const unsigned int uart1_8_pins[] = {40, 41};
+static const unsigned int uart1_9_pins[] = {44, 45};
+static const unsigned int uart1_10_pins[] = {48, 49};
+static const unsigned int uart1_11_pins[] = {52, 53};
+static const unsigned int i2c0_0_pins[] = {10, 11};
+static const unsigned int i2c0_1_pins[] = {14, 15};
+static const unsigned int i2c0_2_pins[] = {18, 19};
+static const unsigned int i2c0_3_pins[] = {22, 23};
+static const unsigned int i2c0_4_pins[] = {26, 27};
+static const unsigned int i2c0_5_pins[] = {30, 31};
+static const unsigned int i2c0_6_pins[] = {34, 35};
+static const unsigned int i2c0_7_pins[] = {38, 39};
+static const unsigned int i2c0_8_pins[] = {42, 43};
+static const unsigned int i2c0_9_pins[] = {46, 47};
+static const unsigned int i2c0_10_pins[] = {50, 51};
+static const unsigned int i2c1_0_pins[] = {12, 13};
+static const unsigned int i2c1_1_pins[] = {16, 17};
+static const unsigned int i2c1_2_pins[] = {20, 21};
+static const unsigned int i2c1_3_pins[] = {24, 25};
+static const unsigned int i2c1_4_pins[] = {28, 29};
+static const unsigned int i2c1_5_pins[] = {32, 33};
+static const unsigned int i2c1_6_pins[] = {36, 37};
+static const unsigned int i2c1_7_pins[] = {40, 41};
+static const unsigned int i2c1_8_pins[] = {44, 45};
+static const unsigned int i2c1_9_pins[] = {48, 49};
+static const unsigned int i2c1_10_pins[] = {52, 53};
+static const unsigned int ttc0_0_pins[] = {18, 19};
+static const unsigned int ttc0_1_pins[] = {30, 31};
+static const unsigned int ttc0_2_pins[] = {42, 43};
+static const unsigned int ttc1_0_pins[] = {16, 17};
+static const unsigned int ttc1_1_pins[] = {28, 29};
+static const unsigned int ttc1_2_pins[] = {40, 41};
+static const unsigned int swdt0_0_pins[] = {14, 15};
+static const unsigned int swdt0_1_pins[] = {26, 27};
+static const unsigned int swdt0_2_pins[] = {38, 39};
+static const unsigned int swdt0_3_pins[] = {50, 51};
+static const unsigned int swdt0_4_pins[] = {52, 53};
+static const unsigned int gpio0_0_pins[] = {0};
+static const unsigned int gpio0_1_pins[] = {1};
+static const unsigned int gpio0_2_pins[] = {2};
+static const unsigned int gpio0_3_pins[] = {3};
+static const unsigned int gpio0_4_pins[] = {4};
+static const unsigned int gpio0_5_pins[] = {5};
+static const unsigned int gpio0_6_pins[] = {6};
+static const unsigned int gpio0_7_pins[] = {7};
+static const unsigned int gpio0_8_pins[] = {8};
+static const unsigned int gpio0_9_pins[] = {9};
+static const unsigned int gpio0_10_pins[] = {10};
+static const unsigned int gpio0_11_pins[] = {11};
+static const unsigned int gpio0_12_pins[] = {12};
+static const unsigned int gpio0_13_pins[] = {13};
+static const unsigned int gpio0_14_pins[] = {14};
+static const unsigned int gpio0_15_pins[] = {15};
+static const unsigned int gpio0_16_pins[] = {16};
+static const unsigned int gpio0_17_pins[] = {17};
+static const unsigned int gpio0_18_pins[] = {18};
+static const unsigned int gpio0_19_pins[] = {19};
+static const unsigned int gpio0_20_pins[] = {20};
+static const unsigned int gpio0_21_pins[] = {21};
+static const unsigned int gpio0_22_pins[] = {22};
+static const unsigned int gpio0_23_pins[] = {23};
+static const unsigned int gpio0_24_pins[] = {24};
+static const unsigned int gpio0_25_pins[] = {25};
+static const unsigned int gpio0_26_pins[] = {26};
+static const unsigned int gpio0_27_pins[] = {27};
+static const unsigned int gpio0_28_pins[] = {28};
+static const unsigned int gpio0_29_pins[] = {29};
+static const unsigned int gpio0_30_pins[] = {30};
+static const unsigned int gpio0_31_pins[] = {31};
+static const unsigned int gpio0_32_pins[] = {32};
+static const unsigned int gpio0_33_pins[] = {33};
+static const unsigned int gpio0_34_pins[] = {34};
+static const unsigned int gpio0_35_pins[] = {35};
+static const unsigned int gpio0_36_pins[] = {36};
+static const unsigned int gpio0_37_pins[] = {37};
+static const unsigned int gpio0_38_pins[] = {38};
+static const unsigned int gpio0_39_pins[] = {39};
+static const unsigned int gpio0_40_pins[] = {40};
+static const unsigned int gpio0_41_pins[] = {41};
+static const unsigned int gpio0_42_pins[] = {42};
+static const unsigned int gpio0_43_pins[] = {43};
+static const unsigned int gpio0_44_pins[] = {44};
+static const unsigned int gpio0_45_pins[] = {45};
+static const unsigned int gpio0_46_pins[] = {46};
+static const unsigned int gpio0_47_pins[] = {47};
+static const unsigned int gpio0_48_pins[] = {48};
+static const unsigned int gpio0_49_pins[] = {49};
+static const unsigned int gpio0_50_pins[] = {50};
+static const unsigned int gpio0_51_pins[] = {51};
+static const unsigned int gpio0_52_pins[] = {52};
+static const unsigned int gpio0_53_pins[] = {53};
+
+#define DEFINE_ZYNQ_PINCTRL_GRP(nm) \
+	{ \
+		.name = #nm "_grp", \
+		.pins = nm ## _pins, \
+		.npins = ARRAY_SIZE(nm ## _pins), \
+	}
+
+struct zynq_pctrl_group zynq_pctrl_groups[] = {
+	DEFINE_ZYNQ_PINCTRL_GRP(ethernet0_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(ethernet1_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(mdio0_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(mdio1_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(qspi0_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(qspi1_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(qspi_fbclk),
+	DEFINE_ZYNQ_PINCTRL_GRP(qspi_cs1),
+	DEFINE_ZYNQ_PINCTRL_GRP(spi0_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(spi0_1),
+	DEFINE_ZYNQ_PINCTRL_GRP(spi0_2),
+	DEFINE_ZYNQ_PINCTRL_GRP(spi1_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(spi1_1),
+	DEFINE_ZYNQ_PINCTRL_GRP(spi1_2),
+	DEFINE_ZYNQ_PINCTRL_GRP(spi1_3),
+	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_1),
+	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_2),
+	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_1),
+	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_2),
+	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_3),
+	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_emio_wp),
+	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_emio_cd),
+	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_emio_wp),
+	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_emio_cd),
+	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor),
+	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor_cs1),
+	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor_addr25),
+	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nand),
+	DEFINE_ZYNQ_PINCTRL_GRP(can0_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(can0_1),
+	DEFINE_ZYNQ_PINCTRL_GRP(can0_2),
+	DEFINE_ZYNQ_PINCTRL_GRP(can0_3),
+	DEFINE_ZYNQ_PINCTRL_GRP(can0_4),
+	DEFINE_ZYNQ_PINCTRL_GRP(can0_5),
+	DEFINE_ZYNQ_PINCTRL_GRP(can0_6),
+	DEFINE_ZYNQ_PINCTRL_GRP(can0_7),
+	DEFINE_ZYNQ_PINCTRL_GRP(can0_8),
+	DEFINE_ZYNQ_PINCTRL_GRP(can0_9),
+	DEFINE_ZYNQ_PINCTRL_GRP(can0_10),
+	DEFINE_ZYNQ_PINCTRL_GRP(can1_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(can1_1),
+	DEFINE_ZYNQ_PINCTRL_GRP(can1_2),
+	DEFINE_ZYNQ_PINCTRL_GRP(can1_3),
+	DEFINE_ZYNQ_PINCTRL_GRP(can1_4),
+	DEFINE_ZYNQ_PINCTRL_GRP(can1_5),
+	DEFINE_ZYNQ_PINCTRL_GRP(can1_6),
+	DEFINE_ZYNQ_PINCTRL_GRP(can1_7),
+	DEFINE_ZYNQ_PINCTRL_GRP(can1_8),
+	DEFINE_ZYNQ_PINCTRL_GRP(can1_9),
+	DEFINE_ZYNQ_PINCTRL_GRP(can1_10),
+	DEFINE_ZYNQ_PINCTRL_GRP(can1_11),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart0_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart0_1),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart0_2),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart0_3),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart0_4),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart0_5),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart0_6),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart0_7),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart0_8),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart0_9),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart0_10),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart1_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart1_1),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart1_2),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart1_3),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart1_4),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart1_5),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart1_6),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart1_7),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart1_8),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart1_9),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart1_10),
+	DEFINE_ZYNQ_PINCTRL_GRP(uart1_11),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_1),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_2),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_3),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_4),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_5),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_6),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_7),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_8),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_9),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_10),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_1),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_2),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_3),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_4),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_5),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_6),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_7),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_8),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_9),
+	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_10),
+	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_1),
+	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_2),
+	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_1),
+	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_2),
+	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_1),
+	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_2),
+	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_3),
+	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_4),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_0),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_1),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_2),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_3),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_4),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_5),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_6),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_7),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_8),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_9),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_10),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_11),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_12),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_13),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_14),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_15),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_16),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_17),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_18),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_19),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_20),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_21),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_22),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_23),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_24),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_25),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_26),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_27),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_28),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_29),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_30),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_31),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_32),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_33),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_34),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_35),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_36),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_37),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_38),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_39),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_40),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_41),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_42),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_43),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_44),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_45),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_46),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_47),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_48),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_49),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_50),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_51),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_52),
+	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_53)
+};
+
+/* function groups */
+static const char * const ethernet0_groups[] = {"ethernet0_0_grp"};
+static const char * const ethernet1_groups[] = {"ethernet1_0_grp"};
+static const char * const mdio0_groups[] = {"mdio0_0_grp"};
+static const char * const mdio1_groups[] = {"mdio1_0_grp"};
+static const char * const qspi0_groups[] = {"qspi0_0_grp"};
+static const char * const qspi1_groups[] = {"qspi0_1_grp"};
+static const char * const qspi_fbclk_groups[] = {"qspi_fbclk_grp"};
+static const char * const qspi_cs1_groups[] = {"qspi_cs1_grp"};
+static const char * const spi0_groups[] = {"spi0_0_grp", "spi0_1_grp",
+					   "spi0_2_grp"};
+static const char * const spi1_groups[] = {"spi1_0_grp", "spi1_1_grp",
+					   "spi1_2_grp", "spi1_3_grp"};
+static const char * const sdio0_groups[] = {"sdio0_0_grp", "sdio0_1_grp",
+					    "sdio0_2_grp"};
+static const char * const sdio1_groups[] = {"sdio1_0_grp", "sdio1_1_grp",
+					    "sdio1_2_grp", "sdio1_3_grp"};
+static const char * const sdio0_pc_groups[] = {"gpio0_0_grp",
+		"gpio0_2_grp", "gpio0_4_grp", "gpio0_6_grp",
+		"gpio0_8_grp", "gpio0_10_grp", "gpio0_12_grp",
+		"gpio0_14_grp", "gpio0_16_grp", "gpio0_18_grp",
+		"gpio0_20_grp", "gpio0_22_grp", "gpio0_24_grp",
+		"gpio0_26_grp", "gpio0_28_grp", "gpio0_30_grp",
+		"gpio0_32_grp", "gpio0_34_grp", "gpio0_36_grp",
+		"gpio0_38_grp", "gpio0_40_grp", "gpio0_42_grp",
+		"gpio0_44_grp", "gpio0_46_grp", "gpio0_48_grp",
+		"gpio0_50_grp", "gpio0_52_grp"};
+static const char * const sdio1_pc_groups[] = {"gpio0_1_grp",
+		"gpio0_3_grp", "gpio0_5_grp", "gpio0_7_grp",
+		"gpio0_9_grp", "gpio0_11_grp", "gpio0_13_grp",
+		"gpio0_15_grp", "gpio0_17_grp", "gpio0_19_grp",
+		"gpio0_21_grp", "gpio0_23_grp", "gpio0_25_grp",
+		"gpio0_27_grp", "gpio0_29_grp", "gpio0_31_grp",
+		"gpio0_33_grp", "gpio0_35_grp", "gpio0_37_grp",
+		"gpio0_39_grp", "gpio0_41_grp", "gpio0_43_grp",
+		"gpio0_45_grp", "gpio0_47_grp", "gpio0_49_grp",
+		"gpio0_51_grp", "gpio0_53_grp"};
+static const char * const sdio0_cd_groups[] = {"gpio0_0_grp",
+		"gpio0_2_grp", "gpio0_4_grp", "gpio0_6_grp",
+		"gpio0_10_grp", "gpio0_12_grp",
+		"gpio0_14_grp", "gpio0_16_grp", "gpio0_18_grp",
+		"gpio0_20_grp", "gpio0_22_grp", "gpio0_24_grp",
+		"gpio0_26_grp", "gpio0_28_grp", "gpio0_30_grp",
+		"gpio0_32_grp", "gpio0_34_grp", "gpio0_36_grp",
+		"gpio0_38_grp", "gpio0_40_grp", "gpio0_42_grp",
+		"gpio0_44_grp", "gpio0_46_grp", "gpio0_48_grp",
+		"gpio0_50_grp", "gpio0_52_grp", "gpio0_1_grp",
+		"gpio0_3_grp", "gpio0_5_grp",
+		"gpio0_9_grp", "gpio0_11_grp", "gpio0_13_grp",
+		"gpio0_15_grp", "gpio0_17_grp", "gpio0_19_grp",
+		"gpio0_21_grp", "gpio0_23_grp", "gpio0_25_grp",
+		"gpio0_27_grp", "gpio0_29_grp", "gpio0_31_grp",
+		"gpio0_33_grp", "gpio0_35_grp", "gpio0_37_grp",
+		"gpio0_39_grp", "gpio0_41_grp", "gpio0_43_grp",
+		"gpio0_45_grp", "gpio0_47_grp", "gpio0_49_grp",
+		"gpio0_51_grp", "gpio0_53_grp", "sdio0_emio_cd_grp"};
+static const char * const sdio0_wp_groups[] = {"gpio0_0_grp",
+		"gpio0_2_grp", "gpio0_4_grp", "gpio0_6_grp",
+		"gpio0_10_grp", "gpio0_12_grp",
+		"gpio0_14_grp", "gpio0_16_grp", "gpio0_18_grp",
+		"gpio0_20_grp", "gpio0_22_grp", "gpio0_24_grp",
+		"gpio0_26_grp", "gpio0_28_grp", "gpio0_30_grp",
+		"gpio0_32_grp", "gpio0_34_grp", "gpio0_36_grp",
+		"gpio0_38_grp", "gpio0_40_grp", "gpio0_42_grp",
+		"gpio0_44_grp", "gpio0_46_grp", "gpio0_48_grp",
+		"gpio0_50_grp", "gpio0_52_grp", "gpio0_1_grp",
+		"gpio0_3_grp", "gpio0_5_grp",
+		"gpio0_9_grp", "gpio0_11_grp", "gpio0_13_grp",
+		"gpio0_15_grp", "gpio0_17_grp", "gpio0_19_grp",
+		"gpio0_21_grp", "gpio0_23_grp", "gpio0_25_grp",
+		"gpio0_27_grp", "gpio0_29_grp", "gpio0_31_grp",
+		"gpio0_33_grp", "gpio0_35_grp", "gpio0_37_grp",
+		"gpio0_39_grp", "gpio0_41_grp", "gpio0_43_grp",
+		"gpio0_45_grp", "gpio0_47_grp", "gpio0_49_grp",
+		"gpio0_51_grp", "gpio0_53_grp", "sdio0_emio_wp_grp"};
+static const char * const sdio1_cd_groups[] = {"gpio0_0_grp",
+		"gpio0_2_grp", "gpio0_4_grp", "gpio0_6_grp",
+		"gpio0_10_grp", "gpio0_12_grp",
+		"gpio0_14_grp", "gpio0_16_grp", "gpio0_18_grp",
+		"gpio0_20_grp", "gpio0_22_grp", "gpio0_24_grp",
+		"gpio0_26_grp", "gpio0_28_grp", "gpio0_30_grp",
+		"gpio0_32_grp", "gpio0_34_grp", "gpio0_36_grp",
+		"gpio0_38_grp", "gpio0_40_grp", "gpio0_42_grp",
+		"gpio0_44_grp", "gpio0_46_grp", "gpio0_48_grp",
+		"gpio0_50_grp", "gpio0_52_grp", "gpio0_1_grp",
+		"gpio0_3_grp", "gpio0_5_grp",
+		"gpio0_9_grp", "gpio0_11_grp", "gpio0_13_grp",
+		"gpio0_15_grp", "gpio0_17_grp", "gpio0_19_grp",
+		"gpio0_21_grp", "gpio0_23_grp", "gpio0_25_grp",
+		"gpio0_27_grp", "gpio0_29_grp", "gpio0_31_grp",
+		"gpio0_33_grp", "gpio0_35_grp", "gpio0_37_grp",
+		"gpio0_39_grp", "gpio0_41_grp", "gpio0_43_grp",
+		"gpio0_45_grp", "gpio0_47_grp", "gpio0_49_grp",
+		"gpio0_51_grp", "gpio0_53_grp", "sdio1_emio_cd_grp"};
+static const char * const sdio1_wp_groups[] = {"gpio0_0_grp",
+		"gpio0_2_grp", "gpio0_4_grp", "gpio0_6_grp",
+		"gpio0_10_grp", "gpio0_12_grp",
+		"gpio0_14_grp", "gpio0_16_grp", "gpio0_18_grp",
+		"gpio0_20_grp", "gpio0_22_grp", "gpio0_24_grp",
+		"gpio0_26_grp", "gpio0_28_grp", "gpio0_30_grp",
+		"gpio0_32_grp", "gpio0_34_grp", "gpio0_36_grp",
+		"gpio0_38_grp", "gpio0_40_grp", "gpio0_42_grp",
+		"gpio0_44_grp", "gpio0_46_grp", "gpio0_48_grp",
+		"gpio0_50_grp", "gpio0_52_grp", "gpio0_1_grp",
+		"gpio0_3_grp", "gpio0_5_grp",
+		"gpio0_9_grp", "gpio0_11_grp", "gpio0_13_grp",
+		"gpio0_15_grp", "gpio0_17_grp", "gpio0_19_grp",
+		"gpio0_21_grp", "gpio0_23_grp", "gpio0_25_grp",
+		"gpio0_27_grp", "gpio0_29_grp", "gpio0_31_grp",
+		"gpio0_33_grp", "gpio0_35_grp", "gpio0_37_grp",
+		"gpio0_39_grp", "gpio0_41_grp", "gpio0_43_grp",
+		"gpio0_45_grp", "gpio0_47_grp", "gpio0_49_grp",
+		"gpio0_51_grp", "gpio0_53_grp", "sdio1_emio_wp_grp"};
+static const char * const smc0_nor_groups[] = {"smc0_nor"};
+static const char * const smc0_nor_cs1_groups[] = {"smc0_nor_cs1_grp"};
+static const char * const smc0_nor_addr25_groups[] = {"smc0_nor_addr25_grp"};
+static const char * const smc0_nand_groups[] = {"smc0_nand"};
+static const char * const can0_groups[] = {"can0_0_grp", "can0_1_grp",
+		"can0_2_grp", "can0_3_grp", "can0_4_grp", "can0_5_grp",
+		"can0_6_grp", "can0_7_grp", "can0_8_grp", "can0_9_grp",
+		"can0_10_grp"};
+static const char * const can1_groups[] = {"can1_0_grp", "can1_1_grp",
+		"can1_2_grp", "can1_3_grp", "can1_4_grp", "can1_5_grp",
+		"can1_6_grp", "can1_7_grp", "can1_8_grp", "can1_9_grp",
+		"can1_10_grp", "can1_11_grp"};
+static const char * const uart0_groups[] = {"uart0_0_grp", "uart0_1_grp",
+		"uart0_2_grp", "uart0_3_grp", "uart0_4_grp", "uart0_5_grp",
+		"uart0_6_grp", "uart0_7_grp", "uart0_8_grp", "uart0_9_grp",
+		"uart0_10_grp"};
+static const char * const uart1_groups[] = {"uart1_0_grp", "uart1_1_grp",
+		"uart1_2_grp", "uart1_3_grp", "uart1_4_grp", "uart1_5_grp",
+		"uart1_6_grp", "uart1_7_grp", "uart1_8_grp", "uart1_9_grp",
+		"uart1_10_grp", "uart1_11_grp"};
+static const char * const i2c0_groups[] = {"i2c0_0_grp", "i2c0_1_grp",
+		"i2c0_2_grp", "i2c0_3_grp", "i2c0_4_grp", "i2c0_5_grp",
+		"i2c0_6_grp", "i2c0_7_grp", "i2c0_8_grp", "i2c0_9_grp",
+		"i2c0_10_grp"};
+static const char * const i2c1_groups[] = {"i2c1_0_grp", "i2c1_1_grp",
+		"i2c1_2_grp", "i2c1_3_grp", "i2c1_4_grp", "i2c1_5_grp",
+		"i2c1_6_grp", "i2c1_7_grp", "i2c1_8_grp", "i2c1_9_grp",
+		"i2c1_10_grp"};
+static const char * const ttc0_groups[] = {"ttc0_0_grp", "ttc0_1_grp",
+					   "ttc0_2_grp"};
+static const char * const ttc1_groups[] = {"ttc1_0_grp", "ttc1_1_grp",
+					   "ttc1_2_grp"};
+static const char * const swdt0_groups[] = {"swdt0_0_grp", "swdt0_1_grp",
+		"swdt0_2_grp", "swdt0_3_grp", "swdt0_4_grp"};
+static const char * const gpio0_groups[] = {"gpio0_0_grp",
+		"gpio0_2_grp", "gpio0_4_grp", "gpio0_6_grp",
+		"gpio0_8_grp", "gpio0_10_grp", "gpio0_12_grp",
+		"gpio0_14_grp", "gpio0_16_grp", "gpio0_18_grp",
+		"gpio0_20_grp", "gpio0_22_grp", "gpio0_24_grp",
+		"gpio0_26_grp", "gpio0_28_grp", "gpio0_30_grp",
+		"gpio0_32_grp", "gpio0_34_grp", "gpio0_36_grp",
+		"gpio0_38_grp", "gpio0_40_grp", "gpio0_42_grp",
+		"gpio0_44_grp", "gpio0_46_grp", "gpio0_48_grp",
+		"gpio0_50_grp", "gpio0_52_grp", "gpio0_1_grp",
+		"gpio0_3_grp", "gpio0_5_grp", "gpio0_7_grp",
+		"gpio0_9_grp", "gpio0_11_grp", "gpio0_13_grp",
+		"gpio0_15_grp", "gpio0_17_grp", "gpio0_19_grp",
+		"gpio0_21_grp", "gpio0_23_grp", "gpio0_25_grp",
+		"gpio0_27_grp", "gpio0_29_grp", "gpio0_31_grp",
+		"gpio0_33_grp", "gpio0_35_grp", "gpio0_37_grp",
+		"gpio0_39_grp", "gpio0_41_grp", "gpio0_43_grp",
+		"gpio0_45_grp", "gpio0_47_grp", "gpio0_49_grp",
+		"gpio0_51_grp", "gpio0_53_grp"};
+
+#define DEFINE_ZYNQ_PINMUX_FUNCTION(fname, mval)	\
+	[ZYNQ_PMUX_##fname] = {				\
+		.name = #fname,				\
+		.groups = fname##_groups,		\
+		.ngroups = ARRAY_SIZE(fname##_groups),	\
+		.mux_val = mval				\
+	}
+
+#define DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(fname, mval, mux, mask, shift)	\
+	[ZYNQ_PMUX_##fname] = {				\
+		.name = #fname,				\
+		.groups = fname##_groups,		\
+		.ngroups = ARRAY_SIZE(fname##_groups),	\
+		.mux_val = mval,			\
+		.mux_mask = mask,			\
+		.mux_shift = shift			\
+	}
+
+#define ZYNQ_SDIO_WP_SHIFT	0
+#define ZYNQ_SDIO_WP_MASK	(0x3f << ZYNQ_SDIO_WP_SHIFT)
+#define ZYNQ_SDIO_CD_SHIFT	16
+#define ZYNQ_SDIO_CD_MASK	(0x3f << ZYNQ_SDIO_CD_SHIFT)
+
+static const struct zynq_pinmux_function zynq_pmux_functions[] = {
+	DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet0, 1),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet1, 1),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(mdio0, 0x40),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(mdio1, 0x50),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi0, 1),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi1, 1),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi_fbclk, 1),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi_cs1, 1),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(spi0, 0x50),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(spi1, 0x50),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio0, 0x40),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio0_pc, 0xc),
+	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio0_wp, 0, 130, ZYNQ_SDIO_WP_MASK,
+					ZYNQ_SDIO_WP_SHIFT),
+	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio0_cd, 0, 130, ZYNQ_SDIO_CD_MASK,
+					ZYNQ_SDIO_CD_SHIFT),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio1, 0x40),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio1_pc, 0xc),
+	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio1_wp, 0, 134, ZYNQ_SDIO_WP_MASK,
+					ZYNQ_SDIO_WP_SHIFT),
+	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio1_cd, 0, 134, ZYNQ_SDIO_CD_MASK,
+					ZYNQ_SDIO_CD_SHIFT),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor, 4),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor_cs1, 8),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor_addr25, 4),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nand, 8),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(can0, 0x10),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(can1, 0x10),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(uart0, 0x70),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(uart1, 0x70),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(i2c0, 0x20),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(i2c1, 0x20),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(ttc0, 0x60),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(ttc1, 0x60),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(swdt0, 0x30),
+	DEFINE_ZYNQ_PINMUX_FUNCTION(gpio0, 0)
+};
+
+
+/* pinctrl */
+static int zynq_pctrl_get_groups_count(struct pinctrl_dev *pctldev)
+{
+	struct zynq_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	return pctrl->ngroups;
+}
+
+static const char *zynq_pctrl_get_group_name(struct pinctrl_dev *pctldev,
+					     unsigned selector)
+{
+	struct zynq_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	return pctrl->groups[selector].name;
+}
+
+static int zynq_pctrl_get_group_pins(struct pinctrl_dev *pctldev,
+				     unsigned selector,
+				     const unsigned **pins,
+				     unsigned *num_pins)
+{
+	struct zynq_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	*pins = pctrl->groups[selector].pins;
+	*num_pins = pctrl->groups[selector].npins;
+
+	return 0;
+}
+
+static const struct pinctrl_ops zynq_pctrl_ops = {
+	.get_groups_count = zynq_pctrl_get_groups_count,
+	.get_group_name = zynq_pctrl_get_group_name,
+	.get_group_pins = zynq_pctrl_get_group_pins,
+	.dt_node_to_map = pinconf_generic_dt_node_to_map_group,
+	.dt_free_map = pinctrl_utils_dt_free_map
+};
+
+/* pinmux */
+static int zynq_pmux_get_functions_count(struct pinctrl_dev *pctldev)
+{
+	struct zynq_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	return pctrl->nfuncs;
+}
+
+static const char *zynq_pmux_get_function_name(struct pinctrl_dev *pctldev,
+					       unsigned selector)
+{
+	struct zynq_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	return pctrl->funcs[selector].name;
+}
+
+static int zynq_pmux_get_function_groups(struct pinctrl_dev *pctldev,
+					 unsigned selector,
+					 const char * const **groups,
+					 unsigned * const num_groups)
+{
+	struct zynq_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	*groups = pctrl->funcs[selector].groups;
+	*num_groups = pctrl->funcs[selector].ngroups;
+	return 0;
+}
+
+static int zynq_pinmux_set_mux(struct pinctrl_dev *pctldev,
+			       unsigned function,
+			       unsigned group)
+{
+	int i, ret;
+	struct zynq_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+	const struct zynq_pctrl_group *pgrp = &pctrl->groups[group];
+	const struct zynq_pinmux_function *func = &pctrl->funcs[function];
+
+	/*
+	 * SD WP & CD are special. They have dedicated registers
+	 * to mux them in
+	 */
+	if (function == ZYNQ_PMUX_sdio0_cd || function == ZYNQ_PMUX_sdio0_wp ||
+			function == ZYNQ_PMUX_sdio1_cd ||
+			function == ZYNQ_PMUX_sdio1_wp) {
+		u32 reg;
+
+		ret = regmap_read(pctrl->syscon,
+				  pctrl->pctrl_offset + func->mux, &reg);
+		if (ret)
+			return ret;
+
+		reg &= ~func->mux_mask;
+		reg |= pgrp->pins[0] << func->mux_shift;
+		ret = regmap_write(pctrl->syscon,
+				   pctrl->pctrl_offset + func->mux, reg);
+		if (ret)
+			return ret;
+	} else {
+		for (i = 0; i < pgrp->npins; i++) {
+			unsigned int pin = pgrp->pins[i];
+			u32 reg, addr = pctrl->pctrl_offset + (4 * pin);
+
+			ret = regmap_read(pctrl->syscon, addr, &reg);
+			if (ret)
+				return ret;
+
+			reg &= ~ZYNQ_PINMUX_MUX_MASK;
+			reg |= func->mux_val << ZYNQ_PINMUX_MUX_SHIFT;
+			ret = regmap_write(pctrl->syscon, addr, reg);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+static const struct pinmux_ops zynq_pinmux_ops = {
+	.get_functions_count = zynq_pmux_get_functions_count,
+	.get_function_name = zynq_pmux_get_function_name,
+	.get_function_groups = zynq_pmux_get_function_groups,
+	.set_mux = zynq_pinmux_set_mux,
+};
+
+/* pinconfig */
+#define ZYNQ_PINCONF_TRISTATE	BIT(0)
+#define ZYNQ_PINCONF_SPEED	BIT(8)
+#define ZYNQ_PINCONF_PULLUP	BIT(12)
+
+static int zynq_pinconf_cfg_get(struct pinctrl_dev *pctldev,
+				unsigned pin,
+				unsigned long *config)
+{
+	u32 reg;
+	int ret;
+	unsigned int arg = 0;
+	unsigned int param = pinconf_to_config_param(*config);
+	struct zynq_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	if (pin > 53)
+		return -ENOTSUPP;
+
+	ret = regmap_read(pctrl->syscon, pctrl->pctrl_offset + (4 * pin), &reg);
+	if (ret)
+		return -EIO;
+
+	switch (param) {
+	case PIN_CONFIG_BIAS_PULL_UP:
+		if (!(reg & ZYNQ_PINCONF_PULLUP))
+			return -EINVAL;
+		arg = 1;
+		break;
+	case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
+		if (!(reg & ZYNQ_PINCONF_TRISTATE))
+			return -EINVAL;
+		arg = 1;
+		break;
+	case PIN_CONFIG_BIAS_DISABLE:
+		if (reg & ZYNQ_PINCONF_PULLUP || reg & ZYNQ_PINCONF_TRISTATE)
+			return -EINVAL;
+		break;
+	case PIN_CONFIG_SLEW_RATE:
+		arg = !!(reg & ZYNQ_PINCONF_SPEED);
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+
+	*config = pinconf_to_config_packed(param, arg);
+	return 0;
+}
+
+static int zynq_pinconf_cfg_set(struct pinctrl_dev *pctldev,
+				unsigned pin,
+				unsigned long *configs,
+				unsigned num_configs)
+{
+	int i, ret;
+	u32 reg;
+	struct zynq_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	if (pin > 53)
+		return -ENOTSUPP;
+
+	ret = regmap_read(pctrl->syscon, pctrl->pctrl_offset + (4 * pin), &reg);
+	if (ret)
+		return -EIO;
+
+	for (i = 0; i < num_configs; i++) {
+		unsigned int param = pinconf_to_config_param(configs[i]);
+		unsigned int arg = pinconf_to_config_argument(configs[i]);
+
+		switch (param) {
+		case PIN_CONFIG_BIAS_PULL_UP:
+			if (arg)
+				reg |= ZYNQ_PINCONF_PULLUP;
+			else
+				reg &= ~ZYNQ_PINCONF_PULLUP;
+			break;
+		case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
+			if (arg)
+				reg |= ZYNQ_PINCONF_TRISTATE;
+			else
+				reg &= ~ZYNQ_PINCONF_TRISTATE;
+			break;
+		case PIN_CONFIG_BIAS_DISABLE:
+			reg &= ~(ZYNQ_PINCONF_PULLUP || ZYNQ_PINCONF_TRISTATE);
+			break;
+		case PIN_CONFIG_SLEW_RATE:
+			if (arg)
+				reg |= ZYNQ_PINCONF_SPEED;
+			else
+				reg &= ~ZYNQ_PINCONF_SPEED;
+
+			break;
+		default:
+			dev_warn(pctldev->dev,
+				 "unsupported configuration parameter '%u'\n",
+				 param);
+			continue;
+		}
+	}
+
+	ret = regmap_write(pctrl->syscon, pctrl->pctrl_offset + (4 * pin), reg);
+	if (ret)
+		return -EIO;
+
+	return 0;
+}
+
+static int zynq_pinconf_group_set(struct pinctrl_dev *pctldev,
+				  unsigned selector,
+				  unsigned long *configs,
+				  unsigned num_configs)
+{
+	int i, ret;
+	struct zynq_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+	const struct zynq_pctrl_group *pgrp = &pctrl->groups[selector];
+
+	for (i = 0; i < pgrp->npins; i++) {
+		ret = zynq_pinconf_cfg_set(pctldev, pgrp->pins[i], configs,
+					   num_configs);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static const struct pinconf_ops zynq_pinconf_ops = {
+	.is_generic = true,
+	.pin_config_get = zynq_pinconf_cfg_get,
+	.pin_config_set = zynq_pinconf_cfg_set,
+	.pin_config_group_set = zynq_pinconf_group_set,
+};
+
+static struct pinctrl_desc zynq_desc = {
+	.name = "zynq_pinctrl",
+	.pins = zynq_pins,
+	.npins = ARRAY_SIZE(zynq_pins),
+	.pctlops = &zynq_pctrl_ops,
+	.pmxops = &zynq_pinmux_ops,
+	.confops = &zynq_pinconf_ops,
+	.owner = THIS_MODULE,
+};
+
+static int zynq_pinctrl_probe(struct platform_device *pdev)
+
+{
+	struct resource *res;
+	struct zynq_pinctrl *pctrl;
+
+	pctrl = devm_kzalloc(&pdev->dev, sizeof(*pctrl), GFP_KERNEL);
+	if (!pctrl)
+		return -ENOMEM;
+
+	pctrl->syscon = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+							"syscon");
+	if (IS_ERR(pctrl->syscon)) {
+		dev_err(&pdev->dev, "unable to get syscon\n");
+		return PTR_ERR(pctrl->syscon);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "missing IO resource\n");
+		return -ENODEV;
+	}
+	pctrl->pctrl_offset = res->start;
+
+	pctrl->groups = zynq_pctrl_groups;
+	pctrl->ngroups = ARRAY_SIZE(zynq_pctrl_groups);
+	pctrl->funcs = zynq_pmux_functions;
+	pctrl->nfuncs = ARRAY_SIZE(zynq_pmux_functions);
+
+	pctrl->pctrl = pinctrl_register(&zynq_desc, &pdev->dev, pctrl);
+	if (!pctrl->pctrl)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, pctrl);
+
+	dev_info(&pdev->dev, "zynq pinctrl initialized\n");
+
+	return 0;
+}
+
+int zynq_pinctrl_remove(struct platform_device *pdev)
+{
+	struct zynq_pinctrl *pctrl = platform_get_drvdata(pdev);
+
+	pinctrl_unregister(pctrl->pctrl);
+
+	return 0;
+}
+
+static const struct of_device_id zynq_pinctrl_of_match[] = {
+	{ .compatible = "xlnx,pinctrl-zynq" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, zynq_pinctrl_of_match);
+
+static struct platform_driver zynq_pinctrl_driver = {
+	.driver = {
+		.name = "zynq-pinctrl",
+		.of_match_table = zynq_pinctrl_of_match,
+	},
+	.probe = zynq_pinctrl_probe,
+	.remove = zynq_pinctrl_remove,
+};
+
+module_platform_driver(zynq_pinctrl_driver);
+
+MODULE_AUTHOR("Sören Brinkmann <soren.brinkmann@xilinx.com>");
+MODULE_DESCRIPTION("Xilinx Zynq pinctrl driver");
+MODULE_LICENSE("GPL");
-- 
2.1.2.1.g5e69ed6


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

* [PATCH RFC v2 2/8] pinctrl: pinconf-generic: Add flag to print arguments
  2014-10-16 17:11 [PATCH RFC v2 0/8] Pinctrl support for Zynq Soren Brinkmann
  2014-10-16 17:11 ` [PATCH RFC v2 1/8] pinctrl: Add driver " Soren Brinkmann
@ 2014-10-16 17:11 ` Soren Brinkmann
  2014-10-28 14:55   ` Linus Walleij
  2014-10-16 17:11 ` [PATCH RFC v2 3/8] pinctrl: pinconf-generic: Add parameter 'IO standard' Soren Brinkmann
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 30+ messages in thread
From: Soren Brinkmann @ 2014-10-16 17:11 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Sören Brinkmann, Michal Simek, linux-kernel,
	linux-arm-kernel, Steffen Trumtrar

When dumping pinconf information in debugfs, config arguments are only
printed when a unit is present and the argument is != 0. For parameters
like the slew rate, this does not work. The slew rate uses a driver
specific format for the argument, i.e. 0 can be a valid argument and a
unit is not provided for it.
For that reason, add a flag to enable printing the argument instead of
inferring it from the presence of a unit and the value of the argument.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/pinctrl/pinconf-generic.c | 65 ++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 28 deletions(-)

diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index 29ff77f90fcb..e28ef957ca2d 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -32,30 +32,32 @@ struct pin_config_item {
 	const enum pin_config_param param;
 	const char * const display;
 	const char * const format;
+	bool has_arg;
 };
 
-#define PCONFDUMP(a, b, c) { .param = a, .display = b, .format = c }
+#define PCONFDUMP(a, b, c, d) { .param = a, .display = b, .format = c, \
+				.has_arg = d }
 
 static struct pin_config_item conf_items[] = {
-	PCONFDUMP(PIN_CONFIG_BIAS_DISABLE, "input bias disabled", NULL),
-	PCONFDUMP(PIN_CONFIG_BIAS_HIGH_IMPEDANCE, "input bias high impedance", NULL),
-	PCONFDUMP(PIN_CONFIG_BIAS_BUS_HOLD, "input bias bus hold", NULL),
-	PCONFDUMP(PIN_CONFIG_BIAS_PULL_UP, "input bias pull up", NULL),
-	PCONFDUMP(PIN_CONFIG_BIAS_PULL_DOWN, "input bias pull down", NULL),
+	PCONFDUMP(PIN_CONFIG_BIAS_DISABLE, "input bias disabled", NULL, false),
+	PCONFDUMP(PIN_CONFIG_BIAS_HIGH_IMPEDANCE, "input bias high impedance", NULL, false),
+	PCONFDUMP(PIN_CONFIG_BIAS_BUS_HOLD, "input bias bus hold", NULL, false),
+	PCONFDUMP(PIN_CONFIG_BIAS_PULL_UP, "input bias pull up", NULL, false),
+	PCONFDUMP(PIN_CONFIG_BIAS_PULL_DOWN, "input bias pull down", NULL, false),
 	PCONFDUMP(PIN_CONFIG_BIAS_PULL_PIN_DEFAULT,
-				"input bias pull to pin specific state", NULL),
-	PCONFDUMP(PIN_CONFIG_DRIVE_PUSH_PULL, "output drive push pull", NULL),
-	PCONFDUMP(PIN_CONFIG_DRIVE_OPEN_DRAIN, "output drive open drain", NULL),
-	PCONFDUMP(PIN_CONFIG_DRIVE_OPEN_SOURCE, "output drive open source", NULL),
-	PCONFDUMP(PIN_CONFIG_DRIVE_STRENGTH, "output drive strength", "mA"),
-	PCONFDUMP(PIN_CONFIG_INPUT_ENABLE, "input enabled", NULL),
-	PCONFDUMP(PIN_CONFIG_INPUT_SCHMITT_ENABLE, "input schmitt enabled", NULL),
-	PCONFDUMP(PIN_CONFIG_INPUT_SCHMITT, "input schmitt trigger", NULL),
-	PCONFDUMP(PIN_CONFIG_INPUT_DEBOUNCE, "input debounce", "usec"),
-	PCONFDUMP(PIN_CONFIG_POWER_SOURCE, "pin power source", "selector"),
-	PCONFDUMP(PIN_CONFIG_SLEW_RATE, "slew rate", NULL),
-	PCONFDUMP(PIN_CONFIG_LOW_POWER_MODE, "pin low power", "mode"),
-	PCONFDUMP(PIN_CONFIG_OUTPUT, "pin output", "level"),
+				"input bias pull to pin specific state", NULL, false),
+	PCONFDUMP(PIN_CONFIG_DRIVE_PUSH_PULL, "output drive push pull", NULL, false),
+	PCONFDUMP(PIN_CONFIG_DRIVE_OPEN_DRAIN, "output drive open drain", NULL, false),
+	PCONFDUMP(PIN_CONFIG_DRIVE_OPEN_SOURCE, "output drive open source", NULL, false),
+	PCONFDUMP(PIN_CONFIG_DRIVE_STRENGTH, "output drive strength", "mA", true),
+	PCONFDUMP(PIN_CONFIG_INPUT_ENABLE, "input enabled", NULL, false),
+	PCONFDUMP(PIN_CONFIG_INPUT_SCHMITT_ENABLE, "input schmitt enabled", NULL, false),
+	PCONFDUMP(PIN_CONFIG_INPUT_SCHMITT, "input schmitt trigger", NULL, false),
+	PCONFDUMP(PIN_CONFIG_INPUT_DEBOUNCE, "input debounce", "usec", true),
+	PCONFDUMP(PIN_CONFIG_POWER_SOURCE, "pin power source", "selector", true),
+	PCONFDUMP(PIN_CONFIG_SLEW_RATE, "slew rate", NULL, true),
+	PCONFDUMP(PIN_CONFIG_LOW_POWER_MODE, "pin low power", "mode", true),
+	PCONFDUMP(PIN_CONFIG_OUTPUT, "pin output", "level", true),
 };
 
 void pinconf_generic_dump_pin(struct pinctrl_dev *pctldev,
@@ -85,11 +87,14 @@ void pinconf_generic_dump_pin(struct pinctrl_dev *pctldev,
 		seq_puts(s, " ");
 		seq_puts(s, conf_items[i].display);
 		/* Print unit if available */
-		if (conf_items[i].format &&
-		    pinconf_to_config_argument(config) != 0)
-			seq_printf(s, " (%u %s)",
-				   pinconf_to_config_argument(config),
-				   conf_items[i].format);
+		if (conf_items[i].has_arg) {
+			seq_printf(s, " (%u",
+				   pinconf_to_config_argument(config));
+			if (conf_items[i].format)
+				seq_printf(s, " %s)", conf_items[i].format);
+			else
+				seq_puts(s, ")");
+		}
 	}
 }
 
@@ -121,10 +126,14 @@ void pinconf_generic_dump_group(struct pinctrl_dev *pctldev,
 		seq_puts(s, " ");
 		seq_puts(s, conf_items[i].display);
 		/* Print unit if available */
-		if (conf_items[i].format && config != 0)
-			seq_printf(s, " (%u %s)",
-				   pinconf_to_config_argument(config),
-				   conf_items[i].format);
+		if (conf_items[i].has_arg) {
+			seq_printf(s, " (%u",
+				   pinconf_to_config_argument(config));
+			if (conf_items[i].format)
+				seq_printf(s, " %s)", conf_items[i].format);
+			else
+				seq_puts(s, ")");
+		}
 	}
 }
 
-- 
2.1.2.1.g5e69ed6


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

* [PATCH RFC v2 3/8] pinctrl: pinconf-generic: Add parameter 'IO standard'
  2014-10-16 17:11 [PATCH RFC v2 0/8] Pinctrl support for Zynq Soren Brinkmann
  2014-10-16 17:11 ` [PATCH RFC v2 1/8] pinctrl: Add driver " Soren Brinkmann
  2014-10-16 17:11 ` [PATCH RFC v2 2/8] pinctrl: pinconf-generic: Add flag to print arguments Soren Brinkmann
@ 2014-10-16 17:11 ` Soren Brinkmann
  2014-10-28 14:59   ` Linus Walleij
  2014-10-16 17:11 ` [PATCH RFC v2 4/8] pinctrl: zynq: Support IO standard property Soren Brinkmann
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 30+ messages in thread
From: Soren Brinkmann @ 2014-10-16 17:11 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Sören Brinkmann, Michal Simek, linux-kernel,
	linux-arm-kernel, Steffen Trumtrar

For HW that can select the IO standard for pins, add the infrastructure
in pinconf generic to parse this parameter.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt | 3 +++
 drivers/pinctrl/pinconf-generic.c                              | 2 ++
 include/linux/pinctrl/pinconf-generic.h                        | 4 ++++
 3 files changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
index 98eb94d91a1c..862c4fe17d04 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
@@ -184,6 +184,7 @@ low-power-disable	- disable low power mode
 output-low		- set the pin to output mode with low level
 output-high		- set the pin to output mode with high level
 slew-rate		- set the slew rate
+io-standard		- set the IO standard
 
 For example:
 
@@ -215,5 +216,7 @@ arguments are described below.
 - input-debounce takes the debounce time in usec as argument
   or 0 to disable debouncing
 
+- io-standard takes a driver specific argument to select an IO standard
+
 More in-depth documentation on these parameters can be found in
 <include/linux/pinctrl/pinconfig-generic.h>
diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index e28ef957ca2d..17ac8f00e16b 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -58,6 +58,7 @@ static struct pin_config_item conf_items[] = {
 	PCONFDUMP(PIN_CONFIG_SLEW_RATE, "slew rate", NULL, true),
 	PCONFDUMP(PIN_CONFIG_LOW_POWER_MODE, "pin low power", "mode", true),
 	PCONFDUMP(PIN_CONFIG_OUTPUT, "pin output", "level", true),
+	PCONFDUMP(PIN_CONFIG_IOSTANDARD, "io standard", NULL, true),
 };
 
 void pinconf_generic_dump_pin(struct pinctrl_dev *pctldev,
@@ -181,6 +182,7 @@ static struct pinconf_generic_dt_params dt_params[] = {
 	{ "output-low", PIN_CONFIG_OUTPUT, 0, },
 	{ "output-high", PIN_CONFIG_OUTPUT, 1, },
 	{ "slew-rate", PIN_CONFIG_SLEW_RATE, 0},
+	{ "io-standard", PIN_CONFIG_IOSTANDARD, 0},
 };
 
 /**
diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
index d578a60eff23..dd1d3251fb93 100644
--- a/include/linux/pinctrl/pinconf-generic.h
+++ b/include/linux/pinctrl/pinconf-generic.h
@@ -92,6 +92,9 @@
  * @PIN_CONFIG_END: this is the last enumerator for pin configurations, if
  *	you need to pass in custom configurations to the pin controller, use
  *	PIN_CONFIG_END+1 as the base offset.
+ * @PIN_CONFIG_IOSTANDARD: if the pin can select an IO standard, the argument to
+ *	this parameter (on a custom format) tells the driver which alternative
+ *	IO standard to use.
  */
 enum pin_config_param {
 	PIN_CONFIG_BIAS_DISABLE,
@@ -112,6 +115,7 @@ enum pin_config_param {
 	PIN_CONFIG_SLEW_RATE,
 	PIN_CONFIG_LOW_POWER_MODE,
 	PIN_CONFIG_OUTPUT,
+	PIN_CONFIG_IOSTANDARD,
 	PIN_CONFIG_END = 0x7FFF,
 };
 
-- 
2.1.2.1.g5e69ed6


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

* [PATCH RFC v2 4/8] pinctrl: zynq: Support IO standard property
  2014-10-16 17:11 [PATCH RFC v2 0/8] Pinctrl support for Zynq Soren Brinkmann
                   ` (2 preceding siblings ...)
  2014-10-16 17:11 ` [PATCH RFC v2 3/8] pinctrl: pinconf-generic: Add parameter 'IO standard' Soren Brinkmann
@ 2014-10-16 17:11 ` Soren Brinkmann
  2014-10-16 17:11 ` [PATCH RFC v2 5/8] pinctrl: zynq: Support low power mode property Soren Brinkmann
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Soren Brinkmann @ 2014-10-16 17:11 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Sören Brinkmann, Michal Simek, linux-kernel,
	linux-arm-kernel, Steffen Trumtrar

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/pinctrl/pinctrl-zynq.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-zynq.c b/drivers/pinctrl/pinctrl-zynq.c
index a32fac61cba0..f3fce76c8390 100644
--- a/drivers/pinctrl/pinctrl-zynq.c
+++ b/drivers/pinctrl/pinctrl-zynq.c
@@ -883,6 +883,18 @@ static const struct pinmux_ops zynq_pinmux_ops = {
 #define ZYNQ_PINCONF_SPEED	BIT(8)
 #define ZYNQ_PINCONF_PULLUP	BIT(12)
 
+#define ZYNQ_PINCONF_IOTYPE_SHIFT	9
+#define ZYNQ_PINCONF_IOTYPE_MASK	(7 << ZYNQ_PINCONF_IOTYPE_SHIFT)
+
+enum zynq_io_standards {
+	zynq_iostd_min,
+	zynq_iostd_lvcmos18,
+	zynq_iostd_lvcmos25,
+	zynq_iostd_lvcmos33,
+	zynq_iostd_hstl,
+	zynq_iostd_max
+};
+
 static int zynq_pinconf_cfg_get(struct pinctrl_dev *pctldev,
 				unsigned pin,
 				unsigned long *config)
@@ -918,6 +930,10 @@ static int zynq_pinconf_cfg_get(struct pinctrl_dev *pctldev,
 	case PIN_CONFIG_SLEW_RATE:
 		arg = !!(reg & ZYNQ_PINCONF_SPEED);
 		break;
+	case PIN_CONFIG_IOSTANDARD:
+		arg = reg & ZYNQ_PINCONF_IOTYPE_MASK;
+		arg >>= ZYNQ_PINCONF_IOTYPE_SHIFT;
+		break;
 	default:
 		return -ENOTSUPP;
 	}
@@ -969,6 +985,15 @@ static int zynq_pinconf_cfg_set(struct pinctrl_dev *pctldev,
 				reg &= ~ZYNQ_PINCONF_SPEED;
 
 			break;
+		case PIN_CONFIG_IOSTANDARD:
+			if (arg <= zynq_iostd_min || arg >= zynq_iostd_max) {
+				dev_warn(pctldev->dev,
+					 "unsupported IO standard '%u'\n",
+					 param);
+				break;
+			}
+			reg |= arg << ZYNQ_PINCONF_IOTYPE_SHIFT;
+			break;
 		default:
 			dev_warn(pctldev->dev,
 				 "unsupported configuration parameter '%u'\n",
-- 
2.1.2.1.g5e69ed6


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

* [PATCH RFC v2 5/8] pinctrl: zynq: Support low power mode property
  2014-10-16 17:11 [PATCH RFC v2 0/8] Pinctrl support for Zynq Soren Brinkmann
                   ` (3 preceding siblings ...)
  2014-10-16 17:11 ` [PATCH RFC v2 4/8] pinctrl: zynq: Support IO standard property Soren Brinkmann
@ 2014-10-16 17:11 ` Soren Brinkmann
  2014-10-16 17:11 ` [PATCH RFC v2 6/8] pinctrl: pinconf-generic: Infer map type from DT property Soren Brinkmann
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Soren Brinkmann @ 2014-10-16 17:11 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Sören Brinkmann, Michal Simek, linux-kernel,
	linux-arm-kernel, Steffen Trumtrar

 - for HSTL type pins, allow setting the low power mode property

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/pinctrl/pinctrl-zynq.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-zynq.c b/drivers/pinctrl/pinctrl-zynq.c
index f3fce76c8390..0136bddbb4be 100644
--- a/drivers/pinctrl/pinctrl-zynq.c
+++ b/drivers/pinctrl/pinctrl-zynq.c
@@ -879,9 +879,10 @@ static const struct pinmux_ops zynq_pinmux_ops = {
 };
 
 /* pinconfig */
-#define ZYNQ_PINCONF_TRISTATE	BIT(0)
-#define ZYNQ_PINCONF_SPEED	BIT(8)
-#define ZYNQ_PINCONF_PULLUP	BIT(12)
+#define ZYNQ_PINCONF_TRISTATE		BIT(0)
+#define ZYNQ_PINCONF_SPEED		BIT(8)
+#define ZYNQ_PINCONF_PULLUP		BIT(12)
+#define ZYNQ_PINCONF_DISABLE_RECVR	BIT(13)
 
 #define ZYNQ_PINCONF_IOTYPE_SHIFT	9
 #define ZYNQ_PINCONF_IOTYPE_MASK	(7 << ZYNQ_PINCONF_IOTYPE_SHIFT)
@@ -895,6 +896,11 @@ enum zynq_io_standards {
 	zynq_iostd_max
 };
 
+static unsigned int zynq_pinconf_iostd_get(u32 reg)
+{
+	return (reg & ZYNQ_PINCONF_IOTYPE_MASK) >> ZYNQ_PINCONF_IOTYPE_SHIFT;
+}
+
 static int zynq_pinconf_cfg_get(struct pinctrl_dev *pctldev,
 				unsigned pin,
 				unsigned long *config)
@@ -930,9 +936,19 @@ static int zynq_pinconf_cfg_get(struct pinctrl_dev *pctldev,
 	case PIN_CONFIG_SLEW_RATE:
 		arg = !!(reg & ZYNQ_PINCONF_SPEED);
 		break;
+	case PIN_CONFIG_LOW_POWER_MODE:
+	{
+		enum zynq_io_standards iostd = zynq_pinconf_iostd_get(reg);
+
+		if (iostd != zynq_iostd_hstl)
+			return -EINVAL;
+		if (!(reg & ZYNQ_PINCONF_DISABLE_RECVR))
+			return -EINVAL;
+		arg = !!(reg & ZYNQ_PINCONF_DISABLE_RECVR);
+		break;
+	}
 	case PIN_CONFIG_IOSTANDARD:
-		arg = reg & ZYNQ_PINCONF_IOTYPE_MASK;
-		arg >>= ZYNQ_PINCONF_IOTYPE_SHIFT;
+		arg = zynq_pinconf_iostd_get(reg);
 		break;
 	default:
 		return -ENOTSUPP;
@@ -994,6 +1010,13 @@ static int zynq_pinconf_cfg_set(struct pinctrl_dev *pctldev,
 			}
 			reg |= arg << ZYNQ_PINCONF_IOTYPE_SHIFT;
 			break;
+		case PIN_CONFIG_LOW_POWER_MODE:
+			if (arg)
+				reg |= ZYNQ_PINCONF_DISABLE_RECVR;
+			else
+				reg &= ~ZYNQ_PINCONF_DISABLE_RECVR;
+
+			break;
 		default:
 			dev_warn(pctldev->dev,
 				 "unsupported configuration parameter '%u'\n",
-- 
2.1.2.1.g5e69ed6


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

* [PATCH RFC v2 6/8] pinctrl: pinconf-generic: Infer map type from DT property
  2014-10-16 17:11 [PATCH RFC v2 0/8] Pinctrl support for Zynq Soren Brinkmann
                   ` (4 preceding siblings ...)
  2014-10-16 17:11 ` [PATCH RFC v2 5/8] pinctrl: zynq: Support low power mode property Soren Brinkmann
@ 2014-10-16 17:11 ` Soren Brinkmann
  2014-10-16 17:11 ` [PATCH RFC v2 7/8] pinctrl: zynq: Use generic map_all function Soren Brinkmann
  2014-10-16 17:11 ` [PATCH RFC v2 8/8] ARM: zynq: DT: Add pinctrl information Soren Brinkmann
  7 siblings, 0 replies; 30+ messages in thread
From: Soren Brinkmann @ 2014-10-16 17:11 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Sören Brinkmann, Michal Simek, linux-kernel,
	linux-arm-kernel, Steffen Trumtrar

With the new 'groups' property, the DT parser can infer the map type
from the fact whether 'pins' or 'groups' is used to specify the pin
group to work on.
To maintain backwards compatibitliy with current usage of the DT
binding, this is only done when an invalid map type is passed to the
parsing function.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/pinctrl/pinconf-generic.c       | 17 ++++++++++++++---
 include/linux/pinctrl/pinconf-generic.h |  7 +++++++
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index 17ac8f00e16b..38b21e6a36a8 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -266,6 +266,7 @@ int pinconf_generic_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 	unsigned reserve;
 	struct property *prop;
 	const char *group;
+	const char *dt_pin_specifier = "pins";
 
 	ret = of_property_read_string(np, "function", &function);
 	if (ret < 0) {
@@ -286,10 +287,20 @@ int pinconf_generic_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 		reserve++;
 	if (num_configs)
 		reserve++;
+
 	ret = of_property_count_strings(np, "pins");
 	if (ret < 0) {
-		dev_err(dev, "could not parse property pins\n");
-		goto exit;
+		ret = of_property_count_strings(np, "groups");
+		if (ret < 0) {
+			dev_err(dev, "could not parse property pins/groups\n");
+			goto exit;
+		}
+		if (type == PIN_MAP_TYPE_INVALID)
+			type = PIN_MAP_TYPE_CONFIGS_GROUP;
+		dt_pin_specifier = "groups";
+	} else {
+		if (type == PIN_MAP_TYPE_INVALID)
+			type = PIN_MAP_TYPE_CONFIGS_PIN;
 	}
 	reserve *= ret;
 
@@ -298,7 +309,7 @@ int pinconf_generic_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 	if (ret < 0)
 		goto exit;
 
-	of_property_for_each_string(np, "pins", prop, group) {
+	of_property_for_each_string(np, dt_pin_specifier, prop, group) {
 		if (function) {
 			ret = pinctrl_utils_add_map_mux(pctldev, map,
 					reserved_maps, num_maps, group,
diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
index dd1d3251fb93..5a13bdf9dfc7 100644
--- a/include/linux/pinctrl/pinconf-generic.h
+++ b/include/linux/pinctrl/pinconf-generic.h
@@ -178,6 +178,13 @@ static inline int pinconf_generic_dt_node_to_map_pin(
 			PIN_MAP_TYPE_CONFIGS_PIN);
 }
 
+static inline int pinconf_generic_dt_node_to_map_all(
+		struct pinctrl_dev *pctldev, struct device_node *np_config,
+		struct pinctrl_map **map, unsigned *num_maps)
+{
+	return pinconf_generic_dt_node_to_map(pctldev, np_config, map, num_maps,
+			PIN_MAP_TYPE_INVALID);
+}
 #endif
 
 #endif /* CONFIG_GENERIC_PINCONF */
-- 
2.1.2.1.g5e69ed6


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

* [PATCH RFC v2 7/8] pinctrl: zynq: Use generic map_all function
  2014-10-16 17:11 [PATCH RFC v2 0/8] Pinctrl support for Zynq Soren Brinkmann
                   ` (5 preceding siblings ...)
  2014-10-16 17:11 ` [PATCH RFC v2 6/8] pinctrl: pinconf-generic: Infer map type from DT property Soren Brinkmann
@ 2014-10-16 17:11 ` Soren Brinkmann
  2014-10-16 17:11 ` [PATCH RFC v2 8/8] ARM: zynq: DT: Add pinctrl information Soren Brinkmann
  7 siblings, 0 replies; 30+ messages in thread
From: Soren Brinkmann @ 2014-10-16 17:11 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Sören Brinkmann, Michal Simek, linux-kernel,
	linux-arm-kernel, Steffen Trumtrar

Use the generic pinconf_generic_dt_node_to_map_all() function
to parse config and mux settings from DT.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/pinctrl/pinctrl-zynq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinctrl-zynq.c b/drivers/pinctrl/pinctrl-zynq.c
index 0136bddbb4be..743a6f563c1e 100644
--- a/drivers/pinctrl/pinctrl-zynq.c
+++ b/drivers/pinctrl/pinctrl-zynq.c
@@ -790,7 +790,7 @@ static const struct pinctrl_ops zynq_pctrl_ops = {
 	.get_groups_count = zynq_pctrl_get_groups_count,
 	.get_group_name = zynq_pctrl_get_group_name,
 	.get_group_pins = zynq_pctrl_get_group_pins,
-	.dt_node_to_map = pinconf_generic_dt_node_to_map_group,
+	.dt_node_to_map = pinconf_generic_dt_node_to_map_all,
 	.dt_free_map = pinctrl_utils_dt_free_map
 };
 
-- 
2.1.2.1.g5e69ed6


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

* [PATCH RFC v2 8/8] ARM: zynq: DT: Add pinctrl information
  2014-10-16 17:11 [PATCH RFC v2 0/8] Pinctrl support for Zynq Soren Brinkmann
                   ` (6 preceding siblings ...)
  2014-10-16 17:11 ` [PATCH RFC v2 7/8] pinctrl: zynq: Use generic map_all function Soren Brinkmann
@ 2014-10-16 17:11 ` Soren Brinkmann
  2014-10-21  5:54   ` Michal Simek
  2014-10-28 15:05   ` Linus Walleij
  7 siblings, 2 replies; 30+ messages in thread
From: Soren Brinkmann @ 2014-10-16 17:11 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Sören Brinkmann, Michal Simek, linux-kernel,
	linux-arm-kernel, Steffen Trumtrar

Add pinctrl descriptions to the zc702 and zc706 device trees.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 arch/arm/boot/dts/zynq-7000.dtsi |   8 ++-
 arch/arm/boot/dts/zynq-zc702.dts | 147 +++++++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/zynq-zc706.dts | 126 +++++++++++++++++++++++++++++++++
 3 files changed, 280 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index 24036c440440..37d7fe36a129 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -238,7 +238,7 @@
 		slcr: slcr@f8000000 {
 			#address-cells = <1>;
 			#size-cells = <1>;
-			compatible = "xlnx,zynq-slcr", "syscon";
+			compatible = "xlnx,zynq-slcr", "syscon", "simple-bus";
 			reg = <0xF8000000 0x1000>;
 			ranges;
 			clkc: clkc@100 {
@@ -259,6 +259,12 @@
 						"dbg_trc", "dbg_apb";
 				reg = <0x100 0x100>;
 			};
+
+			pinctrl0: pinctrl@700 {
+				compatible = "xlnx,pinctrl-zynq";
+				reg = <0x700 0x200>;
+				syscon = <&slcr>;
+			};
 		};
 
 		dmac_s: dmac@f8003000 {
diff --git a/arch/arm/boot/dts/zynq-zc702.dts b/arch/arm/boot/dts/zynq-zc702.dts
index 94e2cda6f9b6..b3ec4d26a9b3 100644
--- a/arch/arm/boot/dts/zynq-zc702.dts
+++ b/arch/arm/boot/dts/zynq-zc702.dts
@@ -40,21 +40,32 @@
 
 &can0 {
 	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_can0_default>;
 };
 
 &gem0 {
 	status = "okay";
 	phy-mode = "rgmii-id";
 	phy-handle = <&ethernet_phy>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_gem0_default>;
 
 	ethernet_phy: ethernet-phy@7 {
 		reg = <7>;
 	};
 };
 
+&gpio0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_gpio0_default>;
+};
+
 &i2c0 {
 	status = "okay";
 	clock-frequency = <400000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c0_default>;
 
 	i2cswitch@74 {
 		compatible = "nxp,pca9548";
@@ -128,10 +139,146 @@
 	};
 };
 
+&pinctrl0 {
+	pinctrl_can0_default: pinctrl-can0-default {
+		common {
+			function = "can0";
+			groups = "can0_9_grp";
+			bias-pull-up = <0>;
+			slew-rate = <0>;
+			io-standard = <1>;
+		};
+
+		rx {
+			pins = "MIO46";
+			bias-high-impedance = <1>;
+		};
+
+		tx {
+			pins = "MIO47";
+			bias-high-impedance = <0>;
+		};
+	};
+
+	pinctrl_gem0_default: pinctrl-gem0-default {
+		common {
+			function = "ethernet0";
+			groups = "ethernet0_0_grp";
+			bias-pull-up = <0>;
+			slew-rate = <0>;
+			io-standard = <4>;
+		};
+
+		rx {
+			pins = "MIO22", "MIO23", "MIO24", "MIO25", "MIO26", "MIO27";
+			bias-high-impedance = <1>;
+			low-power-disable;
+		};
+
+		tx {
+			pins = "MIO16", "MIO17", "MIO18", "MIO19", "MIO20", "MIO21";
+			bias-high-impedance = <0>;
+			low-power-enable;
+		};
+
+		mdio {
+			function = "mdio0";
+			groups = "mdio0_0_grp";
+		};
+	};
+
+	pinctrl_gpio0_default: pinctrl-gpio0-default {
+		common {
+			function = "gpio0";
+			groups = "gpio0_7_grp", "gpio0_8_grp", "gpio0_9_grp",
+				 "gpio0_10_grp", "gpio0_11_grp", "gpio0_12_grp",
+				 "gpio0_13_grp", "gpio0_14_grp";
+			bias-high-impedance = <0>;
+			slew-rate = <0>;
+			io-standard = <1>;
+		};
+
+		pull-up {
+			pins = "MIO9", "MIO10", "MIO11", "MIO12", "MIO13", "MIO14";
+			bias-pull-up = <1>;
+		};
+
+		pull-none {
+			pins = "MIO7", "MIO8";
+			bias-pull-up = <0>;
+		};
+
+	};
+
+	pinctrl_i2c0_default: pinctrl-i2c0-default {
+		common {
+			groups = "i2c0_10_grp";
+			function = "i2c0";
+			bias-pull-up = <1>;
+			bias-high-impedance = <0>;
+			slew-rate = <0>;
+			io-standard = <1>;
+		};
+	};
+
+	pinctrl_sdhci0_default: pinctrl-sdhci0-default {
+		common {
+			groups = "sdio0_2_grp";
+			function = "sdio0";
+			bias-high-impedance = <0>;
+			bias-pull-up = <0>;
+			slew-rate = <0>;
+			io-standard = <1>;
+		};
+
+		cd {
+			groups = "gpio0_0_grp";
+			function = "sdio0_cd";
+			bias-high-impedance = <1>;
+			bias-pull-up = <1>;
+			slew-rate = <0>;
+			io-standard = <1>;
+		};
+
+		wp {
+			groups = "gpio0_15_grp";
+			function = "sdio0_wp";
+			bias-high-impedance = <1>;
+			bias-pull-up = <1>;
+			slew-rate = <0>;
+			io-standard = <1>;
+		};
+	};
+
+	pinctrl_uart1_default: pinctrl-uart1-default {
+		common {
+			groups = "uart1_10_grp";
+			function = "uart1";
+			bias-pull-up = <0>;
+			slew-rate = <0>;
+			io-standard = <1>;
+		};
+
+		rx {
+			pins = "MIO49";
+			bias-high-impedance = <1>;
+		};
+
+		tx {
+			pins = "MIO48";
+			bias-high-impedance = <0>;
+		};
+	};
+};
+
 &sdhci0 {
 	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_sdhci0_default>;
 };
 
 &uart1 {
 	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart1_default>;
 };
diff --git a/arch/arm/boot/dts/zynq-zc706.dts b/arch/arm/boot/dts/zynq-zc706.dts
index a8bbdfbc7093..42a335431613 100644
--- a/arch/arm/boot/dts/zynq-zc706.dts
+++ b/arch/arm/boot/dts/zynq-zc706.dts
@@ -33,15 +33,24 @@
 	status = "okay";
 	phy-mode = "rgmii-id";
 	phy-handle = <&ethernet_phy>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_gem0_default>;
 
 	ethernet_phy: ethernet-phy@7 {
 		reg = <7>;
 	};
 };
 
+&gpio0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_gpio0_default>;
+};
+
 &i2c0 {
 	status = "okay";
 	clock-frequency = <400000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c0_default>;
 
 	i2cswitch@74 {
 		compatible = "nxp,pca9548";
@@ -107,10 +116,127 @@
 	};
 };
 
+&pinctrl0 {
+	pinctrl_gem0_default: pinctrl-gem0-default {
+		common {
+			function = "ethernet0";
+			groups = "ethernet0_0_grp";
+			bias-pull-up = <0>;
+			slew-rate = <0>;
+			io-standard = <4>;
+		};
+
+		rx {
+			pins = "MIO22", "MIO23", "MIO24", "MIO25", "MIO26", "MIO27";
+			bias-high-impedance = <1>;
+			low-power-disable;
+		};
+
+		tx {
+			pins = "MIO16", "MIO17", "MIO18", "MIO19", "MIO20", "MIO21";
+			bias-high-impedance = <0>;
+			low-power-enable;
+		};
+
+		mdio {
+			function = "mdio0";
+			groups = "mdio0_0_grp";
+			bias-high-impedance = <0>;
+			bias-pull-up = <0>;
+			slew-rate = <0>;
+			io-standard = <1>;
+		};
+	};
+
+	pinctrl_gpio0_default: pinctrl-gpio0-default {
+		common {
+			function = "gpio0";
+			groups = "gpio0_7_grp", "gpio0_46_grp", "gpio0_47_grp";
+			bias-high-impedance = <0>;
+			slew-rate = <0>;
+			io-standard = <1>;
+		};
+
+		pull-up {
+			pins = "MIO46", "MIO47";
+			bias-pull-up = <1>;
+		};
+
+		pull-none {
+			pins = "MIO7";
+			bias-pull-up = <0>;
+		};
+	};
+
+	pinctrl_i2c0_default: pinctrl-i2c0-default {
+		common {
+			groups = "i2c0_10_grp";
+			function = "i2c0";
+			bias-high-impedance = <0>;
+			bias-pull-up = <1>;
+			slew-rate = <0>;
+			io-standard = <1>;
+		};
+	};
+
+	pinctrl_sdhci0_default: pinctrl-sdhci0-default {
+		common {
+			groups = "sdio0_2_grp";
+			function = "sdio0";
+			bias-high-impedance = <0>;
+			bias-pull-up = <0>;
+			slew-rate = <0>;
+			io-standard = <1>;
+		};
+
+		cd {
+			groups = "gpio0_14_grp";
+			function = "sdio0_cd";
+			bias-high-impedance = <1>;
+			bias-pull-up = <1>;
+			slew-rate = <0>;
+			io-standard = <1>;
+		};
+
+		wp {
+			groups = "gpio0_15_grp";
+			function = "sdio0_wp";
+			bias-high-impedance = <1>;
+			bias-pull-up = <1>;
+			slew-rate = <0>;
+			io-standard = <1>;
+		};
+	};
+
+	pinctrl_uart1_default: pinctrl-uart1-default {
+		common {
+			groups = "uart1_10_grp";
+			function = "uart1";
+			bias-pull-up = <0>;
+			slew-rate = <0>;
+			io-standard = <1>;
+		};
+
+		rx {
+			pins = "MIO49";
+			bias-high-impedance = <1>;
+		};
+
+		tx {
+			pins = "MIO48";
+			bias-high-impedance = <0>;
+		};
+	};
+};
+
 &sdhci0 {
 	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_sdhci0_default>;
 };
 
 &uart1 {
 	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart1_default>;
 };
-- 
2.1.2.1.g5e69ed6


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

* Re: [PATCH RFC v2 8/8] ARM: zynq: DT: Add pinctrl information
  2014-10-16 17:11 ` [PATCH RFC v2 8/8] ARM: zynq: DT: Add pinctrl information Soren Brinkmann
@ 2014-10-21  5:54   ` Michal Simek
  2014-10-28 15:05   ` Linus Walleij
  1 sibling, 0 replies; 30+ messages in thread
From: Michal Simek @ 2014-10-21  5:54 UTC (permalink / raw)
  To: Soren Brinkmann, Linus Walleij
  Cc: Michal Simek, linux-kernel, linux-arm-kernel, Steffen Trumtrar

On 10/16/2014 07:11 PM, Soren Brinkmann wrote:
> Add pinctrl descriptions to the zc702 and zc706 device trees.
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> ---
>  arch/arm/boot/dts/zynq-7000.dtsi |   8 ++-
>  arch/arm/boot/dts/zynq-zc702.dts | 147 +++++++++++++++++++++++++++++++++++++++
>  arch/arm/boot/dts/zynq-zc706.dts | 126 +++++++++++++++++++++++++++++++++
>  3 files changed, 280 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> index 24036c440440..37d7fe36a129 100644
> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> @@ -238,7 +238,7 @@
>  		slcr: slcr@f8000000 {
>  			#address-cells = <1>;
>  			#size-cells = <1>;
> -			compatible = "xlnx,zynq-slcr", "syscon";
> +			compatible = "xlnx,zynq-slcr", "syscon", "simple-bus";

I expect that you have this here to be able to probe the driver.
slcr is not the bus.
You should be able just to probe child nodes.

Thanks,
Michal

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

* Re: [PATCH RFC v2 1/8] pinctrl: Add driver for Zynq
  2014-10-16 17:11 ` [PATCH RFC v2 1/8] pinctrl: Add driver " Soren Brinkmann
@ 2014-10-28 14:53   ` Linus Walleij
  2014-10-28 16:14     ` Sören Brinkmann
  2014-10-28 15:16   ` Lothar Waßmann
  1 sibling, 1 reply; 30+ messages in thread
From: Linus Walleij @ 2014-10-28 14:53 UTC (permalink / raw)
  To: Soren Brinkmann
  Cc: Michal Simek, linux-kernel, linux-arm-kernel, Steffen Trumtrar

On Thu, Oct 16, 2014 at 7:11 PM, Soren Brinkmann
<soren.brinkmann@xilinx.com> wrote:

> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> ---
> changes since RFC:
>  - use syscon/regmap to access registers in SLCR space
>  - add pinctrl to zc702 DT
>  - rebase to 3.18: rename enable -> set_mux
>  - add kernel-doc
>  - support pinconf
>    - supported attributes
>      - pin-bias: pull up, tristate, disable
>      - slew-rate: 0 == slow, 1 == fast; generic pinconf does not display
>        argument

Great progress!!

(...)
> +++ b/arch/arm/mach-zynq/Kconfig
> @@ -9,6 +9,7 @@ config ARCH_ZYNQ
>         select HAVE_ARM_TWD if SMP
>         select ICST
>         select MFD_SYSCON
> +       select PINCTRL

Don't you also want to select PINCTRL_ZYNQ or is it
really optional?

>         select SOC_BUS
>         help
>           Support for Xilinx Zynq ARM Cortex A9 Platform

Please split these machine changes into a separate patch. It is hitting
a totally different subsystem.

(...)
> +++ b/drivers/pinctrl/pinctrl-zynq.c
(...)
> +static const struct pinctrl_ops zynq_pctrl_ops = {
> +       .get_groups_count = zynq_pctrl_get_groups_count,
> +       .get_group_name = zynq_pctrl_get_group_name,
> +       .get_group_pins = zynq_pctrl_get_group_pins,
> +       .dt_node_to_map = pinconf_generic_dt_node_to_map_group,
> +       .dt_free_map = pinctrl_utils_dt_free_map
> +};

Nice use of generic functions!

> +static int zynq_pinconf_cfg_get(struct pinctrl_dev *pctldev,
> +                               unsigned pin,
> +                               unsigned long *config)
> +{
> +       u32 reg;
> +       int ret;
> +       unsigned int arg = 0;
> +       unsigned int param = pinconf_to_config_param(*config);
> +       struct zynq_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> +
> +       if (pin > 53)
> +               return -ENOTSUPP;

53 looks a bit magical? #define or comment here to explain what's
going on?

Apart from these small things this looks like merge material.

Yours,
Linus Walleij

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

* Re: [PATCH RFC v2 2/8] pinctrl: pinconf-generic: Add flag to print arguments
  2014-10-16 17:11 ` [PATCH RFC v2 2/8] pinctrl: pinconf-generic: Add flag to print arguments Soren Brinkmann
@ 2014-10-28 14:55   ` Linus Walleij
  0 siblings, 0 replies; 30+ messages in thread
From: Linus Walleij @ 2014-10-28 14:55 UTC (permalink / raw)
  To: Soren Brinkmann
  Cc: Michal Simek, linux-kernel, linux-arm-kernel, Steffen Trumtrar

On Thu, Oct 16, 2014 at 7:11 PM, Soren Brinkmann
<soren.brinkmann@xilinx.com> wrote:

> When dumping pinconf information in debugfs, config arguments are only
> printed when a unit is present and the argument is != 0. For parameters
> like the slew rate, this does not work. The slew rate uses a driver
> specific format for the argument, i.e. 0 can be a valid argument and a
> unit is not provided for it.
> For that reason, add a flag to enable printing the argument instead of
> inferring it from the presence of a unit and the value of the argument.
>
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>

Patch applied.

Yours,
Linus Walleij

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

* Re: [PATCH RFC v2 3/8] pinctrl: pinconf-generic: Add parameter 'IO standard'
  2014-10-16 17:11 ` [PATCH RFC v2 3/8] pinctrl: pinconf-generic: Add parameter 'IO standard' Soren Brinkmann
@ 2014-10-28 14:59   ` Linus Walleij
  2014-10-28 16:07     ` Sören Brinkmann
  0 siblings, 1 reply; 30+ messages in thread
From: Linus Walleij @ 2014-10-28 14:59 UTC (permalink / raw)
  To: Soren Brinkmann
  Cc: Michal Simek, linux-kernel, linux-arm-kernel, Steffen Trumtrar

On Thu, Oct 16, 2014 at 7:11 PM, Soren Brinkmann
<soren.brinkmann@xilinx.com> wrote:

> For HW that can select the IO standard for pins, add the infrastructure
> in pinconf generic to parse this parameter.
>
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>

Nothing in this patch actually explain what an "IO standard" is,
which is paramount if it should be a generic property.

And if it should be generic, I really want to know why this is a
thing not overlapping with existing generic configs, and exactly
what it is and that it is probable other silicon will have it too.
And we need to discuss if it's the right name.

Looking at what you actually set up, it looks very much like this
is actually PIN_CONFIG_DRIVE_STRENGTH.

Yours,
Linus Walleij

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

* Re: [PATCH RFC v2 8/8] ARM: zynq: DT: Add pinctrl information
  2014-10-16 17:11 ` [PATCH RFC v2 8/8] ARM: zynq: DT: Add pinctrl information Soren Brinkmann
  2014-10-21  5:54   ` Michal Simek
@ 2014-10-28 15:05   ` Linus Walleij
  2014-10-28 16:03     ` Sören Brinkmann
  1 sibling, 1 reply; 30+ messages in thread
From: Linus Walleij @ 2014-10-28 15:05 UTC (permalink / raw)
  To: Soren Brinkmann
  Cc: Michal Simek, linux-kernel, linux-arm-kernel, Steffen Trumtrar

On Thu, Oct 16, 2014 at 7:11 PM, Soren Brinkmann
<soren.brinkmann@xilinx.com> wrote:

> Add pinctrl descriptions to the zc702 and zc706 device trees.
>
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>

(...)
> +&pinctrl0 {
> +       pinctrl_can0_default: pinctrl-can0-default {
> +               common {
> +                       function = "can0";
> +                       groups = "can0_9_grp";
> +                       bias-pull-up = <0>;

No. If you want pull-up, just use
bias-pull-up;

If you want to disable pull-up, use
bias-disable;

> +                       slew-rate = <0>;

If this measure is any kind of time unit, this is against the laws of nature.

> +                       io-standard = <1>;
> +               };
> +
> +               rx {
> +                       pins = "MIO46";
> +                       bias-high-impedance = <1>;

Just
bias-high-impedance;

> +               };
> +
> +               tx {
> +                       pins = "MIO47";
> +                       bias-high-impedance = <0>;

Just
bias-disable;

Look all these over closely.

Yours,
Linus Walleij

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

* Re: [PATCH RFC v2 1/8] pinctrl: Add driver for Zynq
  2014-10-16 17:11 ` [PATCH RFC v2 1/8] pinctrl: Add driver " Soren Brinkmann
  2014-10-28 14:53   ` Linus Walleij
@ 2014-10-28 15:16   ` Lothar Waßmann
  2014-10-28 16:37     ` Sören Brinkmann
  1 sibling, 1 reply; 30+ messages in thread
From: Lothar Waßmann @ 2014-10-28 15:16 UTC (permalink / raw)
  To: Soren Brinkmann
  Cc: Linus Walleij, linux-arm-kernel, Steffen Trumtrar, Michal Simek,
	linux-kernel

Hi,

Soren Brinkmann wrote:
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> ---
> changes since RFC:
>  - use syscon/regmap to access registers in SLCR space
>  - add pinctrl to zc702 DT
>  - rebase to 3.18: rename enable -> set_mux
>  - add kernel-doc
>  - support pinconf
>    - supported attributes
>      - pin-bias: pull up, tristate, disable
>      - slew-rate: 0 == slow, 1 == fast; generic pinconf does not display
>        argument
> 
[...]
> +struct zynq_pctrl_group zynq_pctrl_groups[] = {
> +	DEFINE_ZYNQ_PINCTRL_GRP(ethernet0_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(ethernet1_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(mdio0_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(mdio1_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(qspi0_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(qspi1_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(qspi_fbclk),
> +	DEFINE_ZYNQ_PINCTRL_GRP(qspi_cs1),
> +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_1),
> +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_2),
> +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_1),
> +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_2),
> +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_3),
> +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_1),
> +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_2),
> +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_1),
> +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_2),
> +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_3),
> +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_emio_wp),
> +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_emio_cd),
> +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_emio_wp),
> +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_emio_cd),
> +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor),
> +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor_cs1),
> +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor_addr25),
> +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nand),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can0_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can0_1),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can0_2),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can0_3),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can0_4),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can0_5),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can0_6),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can0_7),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can0_8),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can0_9),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can0_10),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can1_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can1_1),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can1_2),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can1_3),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can1_4),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can1_5),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can1_6),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can1_7),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can1_8),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can1_9),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can1_10),
> +	DEFINE_ZYNQ_PINCTRL_GRP(can1_11),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_1),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_2),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_3),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_4),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_5),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_6),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_7),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_8),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_9),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_10),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_1),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_2),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_3),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_4),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_5),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_6),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_7),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_8),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_9),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_10),
> +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_11),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_1),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_2),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_3),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_4),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_5),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_6),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_7),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_8),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_9),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_10),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_1),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_2),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_3),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_4),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_5),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_6),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_7),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_8),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_9),
> +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_10),
> +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_1),
> +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_2),
> +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_1),
> +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_2),
> +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_1),
> +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_2),
> +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_3),
> +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_4),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_0),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_1),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_2),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_3),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_4),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_5),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_6),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_7),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_8),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_9),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_10),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_11),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_12),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_13),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_14),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_15),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_16),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_17),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_18),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_19),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_20),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_21),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_22),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_23),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_24),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_25),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_26),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_27),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_28),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_29),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_30),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_31),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_32),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_33),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_34),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_35),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_36),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_37),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_38),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_39),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_40),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_41),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_42),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_43),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_44),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_45),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_46),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_47),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_48),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_49),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_50),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_51),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_52),
> +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_53)
dto.

[...]
> +#define DEFINE_ZYNQ_PINMUX_FUNCTION(fname, mval)	\
> +	[ZYNQ_PMUX_##fname] = {				\
> +		.name = #fname,				\
> +		.groups = fname##_groups,		\
> +		.ngroups = ARRAY_SIZE(fname##_groups),	\
> +		.mux_val = mval				\
> +	}
> +
dto.

[...]
> +#define DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(fname, mval, mux, mask, shift)	\
> +	[ZYNQ_PMUX_##fname] = {				\
> +		.name = #fname,				\
> +		.groups = fname##_groups,		\
> +		.ngroups = ARRAY_SIZE(fname##_groups),	\
> +		.mux_val = mval,			\
> +		.mux_mask = mask,			\
> +		.mux_shift = shift			\
dto.

[...]
> +static const struct zynq_pinmux_function zynq_pmux_functions[] = {
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet0, 1),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet1, 1),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(mdio0, 0x40),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(mdio1, 0x50),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi0, 1),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi1, 1),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi_fbclk, 1),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi_cs1, 1),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(spi0, 0x50),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(spi1, 0x50),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio0, 0x40),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio0_pc, 0xc),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio0_wp, 0, 130, ZYNQ_SDIO_WP_MASK,
> +					ZYNQ_SDIO_WP_SHIFT),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio0_cd, 0, 130, ZYNQ_SDIO_CD_MASK,
> +					ZYNQ_SDIO_CD_SHIFT),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio1, 0x40),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio1_pc, 0xc),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio1_wp, 0, 134, ZYNQ_SDIO_WP_MASK,
> +					ZYNQ_SDIO_WP_SHIFT),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio1_cd, 0, 134, ZYNQ_SDIO_CD_MASK,
> +					ZYNQ_SDIO_CD_SHIFT),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor, 4),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor_cs1, 8),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor_addr25, 4),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nand, 8),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(can0, 0x10),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(can1, 0x10),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(uart0, 0x70),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(uart1, 0x70),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(i2c0, 0x20),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(i2c1, 0x20),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(ttc0, 0x60),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(ttc1, 0x60),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(swdt0, 0x30),
> +	DEFINE_ZYNQ_PINMUX_FUNCTION(gpio0, 0)
dto.

[...]
> +static const struct pinctrl_ops zynq_pctrl_ops = {
> +	.get_groups_count = zynq_pctrl_get_groups_count,
> +	.get_group_name = zynq_pctrl_get_group_name,
> +	.get_group_pins = zynq_pctrl_get_group_pins,
> +	.dt_node_to_map = pinconf_generic_dt_node_to_map_group,
> +	.dt_free_map = pinctrl_utils_dt_free_map
dto.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

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

* Re: [PATCH RFC v2 8/8] ARM: zynq: DT: Add pinctrl information
  2014-10-28 15:05   ` Linus Walleij
@ 2014-10-28 16:03     ` Sören Brinkmann
  2014-10-31  8:17       ` Linus Walleij
  0 siblings, 1 reply; 30+ messages in thread
From: Sören Brinkmann @ 2014-10-28 16:03 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Michal Simek, linux-kernel, linux-arm-kernel, Steffen Trumtrar

On Tue, 2014-10-28 at 04:05PM +0100, Linus Walleij wrote:
> On Thu, Oct 16, 2014 at 7:11 PM, Soren Brinkmann
> <soren.brinkmann@xilinx.com> wrote:
> 
> > Add pinctrl descriptions to the zc702 and zc706 device trees.
> >
> > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> 
> (...)
> > +&pinctrl0 {
> > +       pinctrl_can0_default: pinctrl-can0-default {
> > +               common {
> > +                       function = "can0";
> > +                       groups = "can0_9_grp";
> > +                       bias-pull-up = <0>;
> 
> No. If you want pull-up, just use
> bias-pull-up;
> 
> If you want to disable pull-up, use
> bias-disable;

But bias-disable also disables high-impedance. That doesn't work for me,
I think.

> 
> > +                       slew-rate = <0>;
> 
> If this measure is any kind of time unit, this is against the laws of nature.

It's not. As the bindings say, the argument is driver specific. In the
Zynq case you can simply choose fast or slow - whatever that means -
which maps to 0 or 1 in the DT. This though raises the question where
that documentation lives. Since this is a driver specific
interpretation, it should not be in the binding docs, IMHO. So, some
other place might need to be found to document the implementation
specifics.

> 
> > +                       io-standard = <1>;
> > +               };
> > +
> > +               rx {
> > +                       pins = "MIO46";
> > +                       bias-high-impedance = <1>;
> 
> Just
> bias-high-impedance;

Same problem as I have above. To allow all permutations of pull-up and
tri-state I can't just have a single disable-bias property.

> 
> > +               };
> > +
> > +               tx {
> > +                       pins = "MIO47";
> > +                       bias-high-impedance = <0>;
> 
> Just
> bias-disable;

That would also disable the pull-up.

	Sören

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

* Re: [PATCH RFC v2 3/8] pinctrl: pinconf-generic: Add parameter 'IO standard'
  2014-10-28 14:59   ` Linus Walleij
@ 2014-10-28 16:07     ` Sören Brinkmann
  2014-10-31  8:19       ` Linus Walleij
  0 siblings, 1 reply; 30+ messages in thread
From: Sören Brinkmann @ 2014-10-28 16:07 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Michal Simek, linux-kernel, linux-arm-kernel, Steffen Trumtrar

On Tue, 2014-10-28 at 03:59PM +0100, Linus Walleij wrote:
> On Thu, Oct 16, 2014 at 7:11 PM, Soren Brinkmann
> <soren.brinkmann@xilinx.com> wrote:
> 
> > For HW that can select the IO standard for pins, add the infrastructure
> > in pinconf generic to parse this parameter.
> >
> > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> 
> Nothing in this patch actually explain what an "IO standard" is,
> which is paramount if it should be a generic property.
> 
> And if it should be generic, I really want to know why this is a
> thing not overlapping with existing generic configs, and exactly
> what it is and that it is probable other silicon will have it too.
> And we need to discuss if it's the right name.
> 
> Looking at what you actually set up, it looks very much like this
> is actually PIN_CONFIG_DRIVE_STRENGTH.

I was thinking about that, but:
drive strength is well defined saying you choose the drive strength in
mA (or some other variation of Amperes). That is not (directly) what you
can set in Zynq. In Zynq the config allows selecting IO-standards like
LVCMOS(1.8|2.5|3.3) or HSTL.
So, this might also map to a certain drive-strength, but is not really
mapping well to Zynq pin configuration options.
Hence, I thought introducing this new property which allows a more
flexible interpretation of the argument would be a better way and might
also help other pinctrl cases (though this is speculative).

	Thanks,
	Sören

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

* Re: [PATCH RFC v2 1/8] pinctrl: Add driver for Zynq
  2014-10-28 14:53   ` Linus Walleij
@ 2014-10-28 16:14     ` Sören Brinkmann
  0 siblings, 0 replies; 30+ messages in thread
From: Sören Brinkmann @ 2014-10-28 16:14 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Michal Simek, linux-kernel, linux-arm-kernel, Steffen Trumtrar

On Tue, 2014-10-28 at 03:53PM +0100, Linus Walleij wrote:
> On Thu, Oct 16, 2014 at 7:11 PM, Soren Brinkmann
> <soren.brinkmann@xilinx.com> wrote:
> 
> > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> > ---
> > changes since RFC:
> >  - use syscon/regmap to access registers in SLCR space
> >  - add pinctrl to zc702 DT
> >  - rebase to 3.18: rename enable -> set_mux
> >  - add kernel-doc
> >  - support pinconf
> >    - supported attributes
> >      - pin-bias: pull up, tristate, disable
> >      - slew-rate: 0 == slow, 1 == fast; generic pinconf does not display
> >        argument
> 
> Great progress!!

Thans.

> 
> (...)
> > +++ b/arch/arm/mach-zynq/Kconfig
> > @@ -9,6 +9,7 @@ config ARCH_ZYNQ
> >         select HAVE_ARM_TWD if SMP
> >         select ICST
> >         select MFD_SYSCON
> > +       select PINCTRL
> 
> Don't you also want to select PINCTRL_ZYNQ or is it
> really optional?

Yep, you're right. I did some testing and it is required. I already have
that included in my work tree.

> 
> >         select SOC_BUS
> >         help
> >           Support for Xilinx Zynq ARM Cortex A9 Platform
> 
> Please split these machine changes into a separate patch. It is hitting
> a totally different subsystem.

OK

> 
> (...)
> > +++ b/drivers/pinctrl/pinctrl-zynq.c
> (...)
> > +static const struct pinctrl_ops zynq_pctrl_ops = {
> > +       .get_groups_count = zynq_pctrl_get_groups_count,
> > +       .get_group_name = zynq_pctrl_get_group_name,
> > +       .get_group_pins = zynq_pctrl_get_group_pins,
> > +       .dt_node_to_map = pinconf_generic_dt_node_to_map_group,
> > +       .dt_free_map = pinctrl_utils_dt_free_map
> > +};
> 
> Nice use of generic functions!

Yeah, but for it to really work I need the changes I made to
pinconf-generic. Otherwise things just don't work well since I would
only be able to either select pin groups or pins in DT.

> 
> > +static int zynq_pinconf_cfg_get(struct pinctrl_dev *pctldev,
> > +                               unsigned pin,
> > +                               unsigned long *config)
> > +{
> > +       u32 reg;
> > +       int ret;
> > +       unsigned int arg = 0;
> > +       unsigned int param = pinconf_to_config_param(*config);
> > +       struct zynq_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> > +
> > +       if (pin > 53)
> > +               return -ENOTSUPP;
> 
> 53 looks a bit magical? #define or comment here to explain what's
> going on?

Yep, I'll add something like ZYNQ_NUM_MIOS

> 
> Apart from these small things this looks like merge material.

That is good to hear, thank you. But before merging this the
pinconf-generic changes need to go through. Even though I didn't order
things that way in this series (I wanted to show how my work proceeded),
proper operation depends of the driver depends on those pinconf-generic
changes.

	Thanks,
	Sören

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

* Re: [PATCH RFC v2 1/8] pinctrl: Add driver for Zynq
  2014-10-28 15:16   ` Lothar Waßmann
@ 2014-10-28 16:37     ` Sören Brinkmann
  2014-10-29  4:49       ` Lothar Waßmann
  0 siblings, 1 reply; 30+ messages in thread
From: Sören Brinkmann @ 2014-10-28 16:37 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: Linus Walleij, linux-arm-kernel, Steffen Trumtrar, Michal Simek,
	linux-kernel

On Tue, 2014-10-28 at 04:16PM +0100, Lothar Waßmann wrote:
> Hi,
> 
> Soren Brinkmann wrote:
> > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> > ---
> > changes since RFC:
> >  - use syscon/regmap to access registers in SLCR space
> >  - add pinctrl to zc702 DT
> >  - rebase to 3.18: rename enable -> set_mux
> >  - add kernel-doc
> >  - support pinconf
> >    - supported attributes
> >      - pin-bias: pull up, tristate, disable
> >      - slew-rate: 0 == slow, 1 == fast; generic pinconf does not display
> >        argument
> > 
> [...]
> > +struct zynq_pctrl_group zynq_pctrl_groups[] = {
> > +	DEFINE_ZYNQ_PINCTRL_GRP(ethernet0_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(ethernet1_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(mdio0_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(mdio1_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi0_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi1_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi_fbclk),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi_cs1),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_1),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_2),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_1),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_2),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_3),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_1),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_2),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_1),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_2),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_3),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_emio_wp),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_emio_cd),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_emio_wp),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_emio_cd),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor_cs1),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor_addr25),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nand),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_1),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_2),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_3),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_4),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_5),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_6),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_7),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_8),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_9),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_10),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_1),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_2),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_3),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_4),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_5),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_6),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_7),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_8),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_9),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_10),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_11),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_1),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_2),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_3),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_4),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_5),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_6),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_7),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_8),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_9),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_10),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_1),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_2),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_3),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_4),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_5),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_6),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_7),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_8),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_9),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_10),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_11),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_1),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_2),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_3),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_4),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_5),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_6),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_7),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_8),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_9),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_10),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_1),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_2),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_3),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_4),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_5),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_6),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_7),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_8),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_9),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_10),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_1),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_2),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_1),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_2),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_1),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_2),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_3),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_4),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_0),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_1),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_2),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_3),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_4),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_5),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_6),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_7),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_8),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_9),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_10),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_11),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_12),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_13),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_14),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_15),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_16),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_17),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_18),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_19),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_20),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_21),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_22),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_23),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_24),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_25),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_26),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_27),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_28),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_29),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_30),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_31),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_32),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_33),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_34),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_35),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_36),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_37),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_38),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_39),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_40),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_41),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_42),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_43),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_44),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_45),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_46),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_47),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_48),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_49),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_50),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_51),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_52),
> > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_53)
> dto.
> 
> [...]
> > +#define DEFINE_ZYNQ_PINMUX_FUNCTION(fname, mval)	\
> > +	[ZYNQ_PMUX_##fname] = {				\
> > +		.name = #fname,				\
> > +		.groups = fname##_groups,		\
> > +		.ngroups = ARRAY_SIZE(fname##_groups),	\
> > +		.mux_val = mval				\
> > +	}
> > +
> dto.
> 
> [...]
> > +#define DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(fname, mval, mux, mask, shift)	\
> > +	[ZYNQ_PMUX_##fname] = {				\
> > +		.name = #fname,				\
> > +		.groups = fname##_groups,		\
> > +		.ngroups = ARRAY_SIZE(fname##_groups),	\
> > +		.mux_val = mval,			\
> > +		.mux_mask = mask,			\
> > +		.mux_shift = shift			\
> dto.
> 
> [...]
> > +static const struct zynq_pinmux_function zynq_pmux_functions[] = {
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet0, 1),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet1, 1),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(mdio0, 0x40),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(mdio1, 0x50),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi0, 1),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi1, 1),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi_fbclk, 1),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi_cs1, 1),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(spi0, 0x50),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(spi1, 0x50),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio0, 0x40),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio0_pc, 0xc),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio0_wp, 0, 130, ZYNQ_SDIO_WP_MASK,
> > +					ZYNQ_SDIO_WP_SHIFT),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio0_cd, 0, 130, ZYNQ_SDIO_CD_MASK,
> > +					ZYNQ_SDIO_CD_SHIFT),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio1, 0x40),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio1_pc, 0xc),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio1_wp, 0, 134, ZYNQ_SDIO_WP_MASK,
> > +					ZYNQ_SDIO_WP_SHIFT),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio1_cd, 0, 134, ZYNQ_SDIO_CD_MASK,
> > +					ZYNQ_SDIO_CD_SHIFT),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor, 4),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor_cs1, 8),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor_addr25, 4),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nand, 8),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(can0, 0x10),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(can1, 0x10),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(uart0, 0x70),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(uart1, 0x70),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(i2c0, 0x20),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(i2c1, 0x20),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ttc0, 0x60),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ttc1, 0x60),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(swdt0, 0x30),
> > +	DEFINE_ZYNQ_PINMUX_FUNCTION(gpio0, 0)
> dto.
> 
> [...]
> > +static const struct pinctrl_ops zynq_pctrl_ops = {
> > +	.get_groups_count = zynq_pctrl_get_groups_count,
> > +	.get_group_name = zynq_pctrl_get_group_name,
> > +	.get_group_pins = zynq_pctrl_get_group_pins,
> > +	.dt_node_to_map = pinconf_generic_dt_node_to_map_group,
> > +	.dt_free_map = pinctrl_utils_dt_free_map
> dto.

Can we please use full sentences? But at least real words? I have no
idea what you're trying to tell me.

	Thanks,
	Sören

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

* Re: [PATCH RFC v2 1/8] pinctrl: Add driver for Zynq
  2014-10-28 16:37     ` Sören Brinkmann
@ 2014-10-29  4:49       ` Lothar Waßmann
  2014-10-29 14:12         ` Sören Brinkmann
  0 siblings, 1 reply; 30+ messages in thread
From: Lothar Waßmann @ 2014-10-29  4:49 UTC (permalink / raw)
  To: Sören Brinkmann
  Cc: Linus Walleij, linux-arm-kernel, Steffen Trumtrar, Michal Simek,
	linux-kernel

Hi,

Sören Brinkmann wrote:
> On Tue, 2014-10-28 at 04:16PM +0100, Lothar Waßmann wrote:
> > Hi,
> > 
> > Soren Brinkmann wrote:
> > > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> > > ---
> > > changes since RFC:
> > >  - use syscon/regmap to access registers in SLCR space
> > >  - add pinctrl to zc702 DT
> > >  - rebase to 3.18: rename enable -> set_mux
> > >  - add kernel-doc
> > >  - support pinconf
> > >    - supported attributes
> > >      - pin-bias: pull up, tristate, disable
> > >      - slew-rate: 0 == slow, 1 == fast; generic pinconf does not display
> > >        argument
> > > 
> > [...]
> > > +struct zynq_pctrl_group zynq_pctrl_groups[] = {
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(ethernet0_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(ethernet1_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(mdio0_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(mdio1_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi0_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi1_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi_fbclk),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi_cs1),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_1),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_2),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_1),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_2),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_3),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_1),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_2),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_1),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_2),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_3),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_emio_wp),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_emio_cd),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_emio_wp),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_emio_cd),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor_cs1),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor_addr25),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nand),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_1),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_2),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_3),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_4),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_5),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_6),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_7),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_8),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_9),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_10),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_1),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_2),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_3),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_4),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_5),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_6),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_7),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_8),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_9),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_10),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_11),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_1),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_2),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_3),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_4),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_5),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_6),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_7),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_8),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_9),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_10),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_1),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_2),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_3),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_4),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_5),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_6),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_7),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_8),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_9),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_10),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_11),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_1),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_2),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_3),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_4),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_5),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_6),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_7),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_8),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_9),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_10),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_1),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_2),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_3),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_4),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_5),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_6),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_7),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_8),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_9),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_10),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_1),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_2),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_1),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_2),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_1),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_2),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_3),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_4),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_0),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_1),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_2),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_3),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_4),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_5),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_6),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_7),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_8),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_9),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_10),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_11),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_12),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_13),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_14),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_15),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_16),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_17),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_18),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_19),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_20),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_21),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_22),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_23),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_24),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_25),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_26),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_27),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_28),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_29),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_30),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_31),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_32),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_33),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_34),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_35),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_36),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_37),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_38),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_39),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_40),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_41),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_42),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_43),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_44),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_45),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_46),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_47),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_48),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_49),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_50),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_51),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_52),
> > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_53)
> > dto.
> > 
> > [...]
> > > +#define DEFINE_ZYNQ_PINMUX_FUNCTION(fname, mval)	\
> > > +	[ZYNQ_PMUX_##fname] = {				\
> > > +		.name = #fname,				\
> > > +		.groups = fname##_groups,		\
> > > +		.ngroups = ARRAY_SIZE(fname##_groups),	\
> > > +		.mux_val = mval				\
> > > +	}
> > > +
> > dto.
> > 
> > [...]
> > > +#define DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(fname, mval, mux, mask, shift)	\
> > > +	[ZYNQ_PMUX_##fname] = {				\
> > > +		.name = #fname,				\
> > > +		.groups = fname##_groups,		\
> > > +		.ngroups = ARRAY_SIZE(fname##_groups),	\
> > > +		.mux_val = mval,			\
> > > +		.mux_mask = mask,			\
> > > +		.mux_shift = shift			\
> > dto.
> > 
> > [...]
> > > +static const struct zynq_pinmux_function zynq_pmux_functions[] = {
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet0, 1),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet1, 1),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(mdio0, 0x40),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(mdio1, 0x50),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi0, 1),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi1, 1),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi_fbclk, 1),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi_cs1, 1),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(spi0, 0x50),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(spi1, 0x50),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio0, 0x40),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio0_pc, 0xc),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio0_wp, 0, 130, ZYNQ_SDIO_WP_MASK,
> > > +					ZYNQ_SDIO_WP_SHIFT),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio0_cd, 0, 130, ZYNQ_SDIO_CD_MASK,
> > > +					ZYNQ_SDIO_CD_SHIFT),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio1, 0x40),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio1_pc, 0xc),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio1_wp, 0, 134, ZYNQ_SDIO_WP_MASK,
> > > +					ZYNQ_SDIO_WP_SHIFT),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio1_cd, 0, 134, ZYNQ_SDIO_CD_MASK,
> > > +					ZYNQ_SDIO_CD_SHIFT),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor, 4),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor_cs1, 8),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor_addr25, 4),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nand, 8),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(can0, 0x10),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(can1, 0x10),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(uart0, 0x70),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(uart1, 0x70),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(i2c0, 0x20),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(i2c1, 0x20),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ttc0, 0x60),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ttc1, 0x60),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(swdt0, 0x30),
> > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(gpio0, 0)
> > dto.
> > 
> > [...]
> > > +static const struct pinctrl_ops zynq_pctrl_ops = {
> > > +	.get_groups_count = zynq_pctrl_get_groups_count,
> > > +	.get_group_name = zynq_pctrl_get_group_name,
> > > +	.get_group_pins = zynq_pctrl_get_group_pins,
> > > +	.dt_node_to_map = pinconf_generic_dt_node_to_map_group,
> > > +	.dt_free_map = pinctrl_utils_dt_free_map
> > dto.
> 
> Can we please use full sentences? But at least real words? I have no
> idea what you're trying to tell me.
> 
Sorry, I must have accidentally removed my first comment which was:
'missing comma at end of initializer.'


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

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

* Re: [PATCH RFC v2 1/8] pinctrl: Add driver for Zynq
  2014-10-29  4:49       ` Lothar Waßmann
@ 2014-10-29 14:12         ` Sören Brinkmann
  2014-10-30  8:16           ` Lothar Waßmann
  0 siblings, 1 reply; 30+ messages in thread
From: Sören Brinkmann @ 2014-10-29 14:12 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: Linus Walleij, linux-arm-kernel, Steffen Trumtrar, Michal Simek,
	linux-kernel

On Wed, 2014-10-29 at 05:49AM +0100, Lothar Waßmann wrote:
> Hi,
> 
> Sören Brinkmann wrote:
> > On Tue, 2014-10-28 at 04:16PM +0100, Lothar Waßmann wrote:
> > > Hi,
> > > 
> > > Soren Brinkmann wrote:
> > > > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> > > > ---
> > > > changes since RFC:
> > > >  - use syscon/regmap to access registers in SLCR space
> > > >  - add pinctrl to zc702 DT
> > > >  - rebase to 3.18: rename enable -> set_mux
> > > >  - add kernel-doc
> > > >  - support pinconf
> > > >    - supported attributes
> > > >      - pin-bias: pull up, tristate, disable
> > > >      - slew-rate: 0 == slow, 1 == fast; generic pinconf does not display
> > > >        argument
> > > > 
> > > [...]
> > > > +struct zynq_pctrl_group zynq_pctrl_groups[] = {
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ethernet0_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ethernet1_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(mdio0_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(mdio1_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi0_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi1_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi_fbclk),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi_cs1),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_1),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_2),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_1),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_2),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_3),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_1),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_2),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_1),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_2),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_3),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_emio_wp),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_emio_cd),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_emio_wp),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_emio_cd),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor_cs1),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor_addr25),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nand),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_1),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_2),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_3),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_4),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_5),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_6),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_7),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_8),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_9),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_10),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_1),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_2),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_3),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_4),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_5),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_6),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_7),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_8),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_9),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_10),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_11),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_1),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_2),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_3),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_4),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_5),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_6),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_7),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_8),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_9),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_10),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_1),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_2),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_3),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_4),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_5),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_6),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_7),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_8),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_9),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_10),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_11),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_1),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_2),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_3),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_4),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_5),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_6),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_7),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_8),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_9),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_10),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_1),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_2),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_3),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_4),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_5),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_6),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_7),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_8),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_9),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_10),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_1),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_2),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_1),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_2),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_1),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_2),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_3),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_4),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_0),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_1),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_2),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_3),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_4),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_5),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_6),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_7),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_8),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_9),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_10),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_11),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_12),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_13),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_14),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_15),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_16),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_17),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_18),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_19),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_20),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_21),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_22),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_23),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_24),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_25),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_26),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_27),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_28),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_29),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_30),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_31),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_32),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_33),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_34),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_35),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_36),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_37),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_38),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_39),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_40),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_41),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_42),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_43),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_44),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_45),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_46),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_47),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_48),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_49),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_50),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_51),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_52),
> > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_53)
> > > dto.
> > > 
> > > [...]
> > > > +#define DEFINE_ZYNQ_PINMUX_FUNCTION(fname, mval)	\
> > > > +	[ZYNQ_PMUX_##fname] = {				\
> > > > +		.name = #fname,				\
> > > > +		.groups = fname##_groups,		\
> > > > +		.ngroups = ARRAY_SIZE(fname##_groups),	\
> > > > +		.mux_val = mval				\
> > > > +	}
> > > > +
> > > dto.
> > > 
> > > [...]
> > > > +#define DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(fname, mval, mux, mask, shift)	\
> > > > +	[ZYNQ_PMUX_##fname] = {				\
> > > > +		.name = #fname,				\
> > > > +		.groups = fname##_groups,		\
> > > > +		.ngroups = ARRAY_SIZE(fname##_groups),	\
> > > > +		.mux_val = mval,			\
> > > > +		.mux_mask = mask,			\
> > > > +		.mux_shift = shift			\
> > > dto.
> > > 
> > > [...]
> > > > +static const struct zynq_pinmux_function zynq_pmux_functions[] = {
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet0, 1),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet1, 1),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(mdio0, 0x40),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(mdio1, 0x50),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi0, 1),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi1, 1),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi_fbclk, 1),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi_cs1, 1),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(spi0, 0x50),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(spi1, 0x50),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio0, 0x40),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio0_pc, 0xc),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio0_wp, 0, 130, ZYNQ_SDIO_WP_MASK,
> > > > +					ZYNQ_SDIO_WP_SHIFT),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio0_cd, 0, 130, ZYNQ_SDIO_CD_MASK,
> > > > +					ZYNQ_SDIO_CD_SHIFT),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio1, 0x40),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio1_pc, 0xc),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio1_wp, 0, 134, ZYNQ_SDIO_WP_MASK,
> > > > +					ZYNQ_SDIO_WP_SHIFT),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio1_cd, 0, 134, ZYNQ_SDIO_CD_MASK,
> > > > +					ZYNQ_SDIO_CD_SHIFT),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor, 4),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor_cs1, 8),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor_addr25, 4),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nand, 8),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(can0, 0x10),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(can1, 0x10),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(uart0, 0x70),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(uart1, 0x70),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(i2c0, 0x20),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(i2c1, 0x20),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ttc0, 0x60),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ttc1, 0x60),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(swdt0, 0x30),
> > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(gpio0, 0)
> > > dto.
> > > 
> > > [...]
> > > > +static const struct pinctrl_ops zynq_pctrl_ops = {
> > > > +	.get_groups_count = zynq_pctrl_get_groups_count,
> > > > +	.get_group_name = zynq_pctrl_get_group_name,
> > > > +	.get_group_pins = zynq_pctrl_get_group_pins,
> > > > +	.dt_node_to_map = pinconf_generic_dt_node_to_map_group,
> > > > +	.dt_free_map = pinctrl_utils_dt_free_map
> > > dto.
> > 
> > Can we please use full sentences? But at least real words? I have no
> > idea what you're trying to tell me.
> > 
> Sorry, I must have accidentally removed my first comment which was:
> 'missing comma at end of initializer.'

Why should there be one? Neither gcc nor checkpatch seem to "miss" one.

	Sören

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

* Re: [PATCH RFC v2 1/8] pinctrl: Add driver for Zynq
  2014-10-29 14:12         ` Sören Brinkmann
@ 2014-10-30  8:16           ` Lothar Waßmann
  2014-11-02 17:22             ` Sören Brinkmann
  0 siblings, 1 reply; 30+ messages in thread
From: Lothar Waßmann @ 2014-10-30  8:16 UTC (permalink / raw)
  To: Sören Brinkmann
  Cc: Linus Walleij, linux-arm-kernel, Steffen Trumtrar, Michal Simek,
	linux-kernel

Hi,

Sören Brinkmann wrote:
> On Wed, 2014-10-29 at 05:49AM +0100, Lothar Waßmann wrote:
> > Hi,
> > 
> > Sören Brinkmann wrote:
> > > On Tue, 2014-10-28 at 04:16PM +0100, Lothar Waßmann wrote:
> > > > Hi,
> > > > 
> > > > Soren Brinkmann wrote:
> > > > > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> > > > > ---
> > > > > changes since RFC:
> > > > >  - use syscon/regmap to access registers in SLCR space
> > > > >  - add pinctrl to zc702 DT
> > > > >  - rebase to 3.18: rename enable -> set_mux
> > > > >  - add kernel-doc
> > > > >  - support pinconf
> > > > >    - supported attributes
> > > > >      - pin-bias: pull up, tristate, disable
> > > > >      - slew-rate: 0 == slow, 1 == fast; generic pinconf does not display
> > > > >        argument
> > > > > 
> > > > [...]
> > > > > +struct zynq_pctrl_group zynq_pctrl_groups[] = {
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ethernet0_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ethernet1_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(mdio0_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(mdio1_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi0_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi1_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi_fbclk),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi_cs1),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_1),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_2),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_1),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_2),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_3),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_1),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_2),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_1),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_2),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_3),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_emio_wp),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_emio_cd),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_emio_wp),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_emio_cd),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor_cs1),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor_addr25),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nand),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_1),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_2),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_3),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_4),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_5),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_6),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_7),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_8),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_9),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_10),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_1),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_2),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_3),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_4),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_5),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_6),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_7),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_8),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_9),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_10),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_11),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_1),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_2),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_3),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_4),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_5),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_6),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_7),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_8),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_9),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_10),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_1),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_2),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_3),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_4),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_5),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_6),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_7),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_8),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_9),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_10),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_11),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_1),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_2),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_3),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_4),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_5),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_6),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_7),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_8),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_9),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_10),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_1),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_2),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_3),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_4),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_5),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_6),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_7),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_8),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_9),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_10),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_1),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_2),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_1),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_2),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_1),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_2),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_3),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_4),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_0),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_1),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_2),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_3),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_4),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_5),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_6),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_7),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_8),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_9),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_10),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_11),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_12),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_13),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_14),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_15),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_16),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_17),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_18),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_19),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_20),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_21),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_22),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_23),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_24),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_25),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_26),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_27),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_28),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_29),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_30),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_31),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_32),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_33),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_34),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_35),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_36),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_37),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_38),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_39),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_40),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_41),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_42),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_43),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_44),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_45),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_46),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_47),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_48),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_49),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_50),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_51),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_52),
> > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_53)
> > > > dto.
> > > > 
> > > > [...]
> > > > > +#define DEFINE_ZYNQ_PINMUX_FUNCTION(fname, mval)	\
> > > > > +	[ZYNQ_PMUX_##fname] = {				\
> > > > > +		.name = #fname,				\
> > > > > +		.groups = fname##_groups,		\
> > > > > +		.ngroups = ARRAY_SIZE(fname##_groups),	\
> > > > > +		.mux_val = mval				\
> > > > > +	}
> > > > > +
> > > > dto.
> > > > 
> > > > [...]
> > > > > +#define DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(fname, mval, mux, mask, shift)	\
> > > > > +	[ZYNQ_PMUX_##fname] = {				\
> > > > > +		.name = #fname,				\
> > > > > +		.groups = fname##_groups,		\
> > > > > +		.ngroups = ARRAY_SIZE(fname##_groups),	\
> > > > > +		.mux_val = mval,			\
> > > > > +		.mux_mask = mask,			\
> > > > > +		.mux_shift = shift			\
> > > > dto.
> > > > 
> > > > [...]
> > > > > +static const struct zynq_pinmux_function zynq_pmux_functions[] = {
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet0, 1),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet1, 1),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(mdio0, 0x40),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(mdio1, 0x50),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi0, 1),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi1, 1),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi_fbclk, 1),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi_cs1, 1),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(spi0, 0x50),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(spi1, 0x50),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio0, 0x40),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio0_pc, 0xc),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio0_wp, 0, 130, ZYNQ_SDIO_WP_MASK,
> > > > > +					ZYNQ_SDIO_WP_SHIFT),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio0_cd, 0, 130, ZYNQ_SDIO_CD_MASK,
> > > > > +					ZYNQ_SDIO_CD_SHIFT),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio1, 0x40),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio1_pc, 0xc),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio1_wp, 0, 134, ZYNQ_SDIO_WP_MASK,
> > > > > +					ZYNQ_SDIO_WP_SHIFT),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio1_cd, 0, 134, ZYNQ_SDIO_CD_MASK,
> > > > > +					ZYNQ_SDIO_CD_SHIFT),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor, 4),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor_cs1, 8),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor_addr25, 4),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nand, 8),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(can0, 0x10),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(can1, 0x10),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(uart0, 0x70),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(uart1, 0x70),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(i2c0, 0x20),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(i2c1, 0x20),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ttc0, 0x60),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ttc1, 0x60),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(swdt0, 0x30),
> > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(gpio0, 0)
> > > > dto.
> > > > 
> > > > [...]
> > > > > +static const struct pinctrl_ops zynq_pctrl_ops = {
> > > > > +	.get_groups_count = zynq_pctrl_get_groups_count,
> > > > > +	.get_group_name = zynq_pctrl_get_group_name,
> > > > > +	.get_group_pins = zynq_pctrl_get_group_pins,
> > > > > +	.dt_node_to_map = pinconf_generic_dt_node_to_map_group,
> > > > > +	.dt_free_map = pinctrl_utils_dt_free_map
> > > > dto.
> > > 
> > > Can we please use full sentences? But at least real words? I have no
> > > idea what you're trying to tell me.
> > > 
> > Sorry, I must have accidentally removed my first comment which was:
> > 'missing comma at end of initializer.'
> 
> Why should there be one? Neither gcc nor checkpatch seem to "miss" one.
> 
It helps further patching and avoiding possible conflicts because you
don't need to modify an existing line when adding a new one at the end.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

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

* Re: [PATCH RFC v2 8/8] ARM: zynq: DT: Add pinctrl information
  2014-10-28 16:03     ` Sören Brinkmann
@ 2014-10-31  8:17       ` Linus Walleij
  2014-10-31 16:57         ` Sören Brinkmann
  0 siblings, 1 reply; 30+ messages in thread
From: Linus Walleij @ 2014-10-31  8:17 UTC (permalink / raw)
  To: Sören Brinkmann
  Cc: Michal Simek, linux-kernel, linux-arm-kernel, Steffen Trumtrar

On Tue, Oct 28, 2014 at 5:03 PM, Sören Brinkmann
<soren.brinkmann@xilinx.com> wrote:
> On Tue, 2014-10-28 at 04:05PM +0100, Linus Walleij wrote:
>> On Thu, Oct 16, 2014 at 7:11 PM, Soren Brinkmann
>> <soren.brinkmann@xilinx.com> wrote:
>>
>> > Add pinctrl descriptions to the zc702 and zc706 device trees.
>> >
>> > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
>>
>> (...)
>> > +&pinctrl0 {
>> > +       pinctrl_can0_default: pinctrl-can0-default {
>> > +               common {
>> > +                       function = "can0";
>> > +                       groups = "can0_9_grp";
>> > +                       bias-pull-up = <0>;
>>
>> No. If you want pull-up, just use
>> bias-pull-up;
>>
>> If you want to disable pull-up, use
>> bias-disable;
>
> But bias-disable also disables high-impedance. That doesn't work for me,
> I think.

Hm. Some sequencing problem right? Like you count on
bias-high-impdedance being set in some other state?

I think each state should be self-contained, so you set
all the stuff you need in a state, do not rely on things coming
in pre-set from another state.

So in this case just set bias-high-impedance; then and
if the state does not have bias-pull-up, *always* disable it
in the driver.

>>
>> > +                       slew-rate = <0>;
>>
>> If this measure is any kind of time unit, this is against the laws of nature.
>
> It's not. As the bindings say, the argument is driver specific.

Okay then.

>> > +               rx {
>> > +                       pins = "MIO46";
>> > +                       bias-high-impedance = <1>;
>>
>> Just
>> bias-high-impedance;
>
> Same problem as I have above. To allow all permutations of pull-up and
> tri-state I can't just have a single disable-bias property.

Again it seems to be a sequencing problem. And device tree is
not good at sequences, therefore all states should be self-contained.

Yours,
Linus Walleij

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

* Re: [PATCH RFC v2 3/8] pinctrl: pinconf-generic: Add parameter 'IO standard'
  2014-10-28 16:07     ` Sören Brinkmann
@ 2014-10-31  8:19       ` Linus Walleij
  0 siblings, 0 replies; 30+ messages in thread
From: Linus Walleij @ 2014-10-31  8:19 UTC (permalink / raw)
  To: Sören Brinkmann
  Cc: Michal Simek, linux-kernel, linux-arm-kernel, Steffen Trumtrar

On Tue, Oct 28, 2014 at 5:07 PM, Sören Brinkmann
<soren.brinkmann@xilinx.com> wrote:

> So, this might also map to a certain drive-strength, but is not really
> mapping well to Zynq pin configuration options.
> Hence, I thought introducing this new property which allows a more
> flexible interpretation of the argument would be a better way and might
> also help other pinctrl cases (though this is speculative).

Maybe a new property is needed, but it doesn't seem generic enough.

It may be that you need to have a combination of generic and custom
config options (which is fine).

It may need some tweaking of the generic DT config parser to allow
custom attributes but that's needed one of these days anyway.

Yours,
Linus Walleij

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

* Re: [PATCH RFC v2 8/8] ARM: zynq: DT: Add pinctrl information
  2014-10-31  8:17       ` Linus Walleij
@ 2014-10-31 16:57         ` Sören Brinkmann
  2014-10-31 17:36           ` Linus Walleij
  0 siblings, 1 reply; 30+ messages in thread
From: Sören Brinkmann @ 2014-10-31 16:57 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Michal Simek, linux-kernel, linux-arm-kernel, Steffen Trumtrar

On Fri, 2014-10-31 at 09:17AM +0100, Linus Walleij wrote:
> On Tue, Oct 28, 2014 at 5:03 PM, Sören Brinkmann
> <soren.brinkmann@xilinx.com> wrote:
> > On Tue, 2014-10-28 at 04:05PM +0100, Linus Walleij wrote:
> >> On Thu, Oct 16, 2014 at 7:11 PM, Soren Brinkmann
> >> <soren.brinkmann@xilinx.com> wrote:
> >>
> >> > Add pinctrl descriptions to the zc702 and zc706 device trees.
> >> >
> >> > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> >>
> >> (...)
> >> > +&pinctrl0 {
> >> > +       pinctrl_can0_default: pinctrl-can0-default {
> >> > +               common {
> >> > +                       function = "can0";
> >> > +                       groups = "can0_9_grp";
> >> > +                       bias-pull-up = <0>;
> >>
> >> No. If you want pull-up, just use
> >> bias-pull-up;
> >>
> >> If you want to disable pull-up, use
> >> bias-disable;
> >
> > But bias-disable also disables high-impedance. That doesn't work for me,
> > I think.
> 
> Hm. Some sequencing problem right? Like you count on
> bias-high-impdedance being set in some other state?
> 
> I think each state should be self-contained, so you set
> all the stuff you need in a state, do not rely on things coming
> in pre-set from another state.
> 
> So in this case just set bias-high-impedance; then and
> if the state does not have bias-pull-up, *always* disable it
> in the driver.
> 
> >>
> >> > +                       slew-rate = <0>;
> >>
> >> If this measure is any kind of time unit, this is against the laws of nature.
> >
> > It's not. As the bindings say, the argument is driver specific.
> 
> Okay then.
> 
> >> > +               rx {
> >> > +                       pins = "MIO46";
> >> > +                       bias-high-impedance = <1>;
> >>
> >> Just
> >> bias-high-impedance;
> >
> > Same problem as I have above. To allow all permutations of pull-up and
> > tri-state I can't just have a single disable-bias property.
> 
> Again it seems to be a sequencing problem. And device tree is
> not good at sequences, therefore all states should be self-contained.

I agree, but how would I define a pin with pull-up enabled and
tri-state disabled - assume the pin is currently in a random state that
can have those things set/not set arbitrarily.

I don't see how that can be handled without using arguments the way I
proposed.

I can't put bias-disable in DT since it would potentially disable both
and the pull-up enable would have only a transient effect.
I can't do the sequencing in the driver either. If I see pull-up enable,
I can't imply an effect on tri-state since I can't know whether there
was/will be a tri-state property that sets it as well.

	Sören

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

* Re: [PATCH RFC v2 8/8] ARM: zynq: DT: Add pinctrl information
  2014-10-31 16:57         ` Sören Brinkmann
@ 2014-10-31 17:36           ` Linus Walleij
  2014-10-31 17:40             ` Sören Brinkmann
  0 siblings, 1 reply; 30+ messages in thread
From: Linus Walleij @ 2014-10-31 17:36 UTC (permalink / raw)
  To: Sören Brinkmann
  Cc: Michal Simek, linux-kernel, linux-arm-kernel, Steffen Trumtrar

On Fri, Oct 31, 2014 at 5:57 PM, Sören Brinkmann
<soren.brinkmann@xilinx.com> wrote:
> On Fri, 2014-10-31 at 09:17AM +0100, Linus Walleij wrote:

>> Again it seems to be a sequencing problem. And device tree is
>> not good at sequences, therefore all states should be self-contained.
>
> I agree, but how would I define a pin with pull-up enabled and
> tri-state disabled - assume the pin is currently in a random state that
> can have those things set/not set arbitrarily.

I was more thinking as everything you don't enable explicitly
in a state is per definition disabled.

So if you are in state A and tri-state is enabled there and you
move to state B where pull-up is enabled, then tri-state should
be disabled, since it is not explicitly enabled.

> I can't put bias-disable in DT since it would potentially disable both
> and the pull-up enable would have only a transient effect.

Well look at the callback from the core:

        int (*pin_config_set) (struct pinctrl_dev *pctldev,
                               unsigned pin,
                               unsigned long *configs,
                               unsigned num_configs);

You get all configs in an array. The driver can walk over the list and
make informed decisions on what to do *BEFORE* poking any registers.

Avoiding transients as you describe is part of why the callback
looks as it does. This is why every driver has its own for-loop.

> I can't do the sequencing in the driver either. If I see pull-up enable,
> I can't imply an effect on tri-state since I can't know whether there
> was/will be a tri-state property that sets it as well.

If you define that each state is self-contained you can.

Yours,
Linus Walleij

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

* Re: [PATCH RFC v2 8/8] ARM: zynq: DT: Add pinctrl information
  2014-10-31 17:36           ` Linus Walleij
@ 2014-10-31 17:40             ` Sören Brinkmann
  2014-11-02 20:20               ` Sören Brinkmann
  0 siblings, 1 reply; 30+ messages in thread
From: Sören Brinkmann @ 2014-10-31 17:40 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Michal Simek, linux-kernel, linux-arm-kernel, Steffen Trumtrar

On Fri, 2014-10-31 at 06:36PM +0100, Linus Walleij wrote:
> On Fri, Oct 31, 2014 at 5:57 PM, Sören Brinkmann
> <soren.brinkmann@xilinx.com> wrote:
> > On Fri, 2014-10-31 at 09:17AM +0100, Linus Walleij wrote:
> 
> >> Again it seems to be a sequencing problem. And device tree is
> >> not good at sequences, therefore all states should be self-contained.
> >
> > I agree, but how would I define a pin with pull-up enabled and
> > tri-state disabled - assume the pin is currently in a random state that
> > can have those things set/not set arbitrarily.
> 
> I was more thinking as everything you don't enable explicitly
> in a state is per definition disabled.
> 
> So if you are in state A and tri-state is enabled there and you
> move to state B where pull-up is enabled, then tri-state should
> be disabled, since it is not explicitly enabled.
> 
> > I can't put bias-disable in DT since it would potentially disable both
> > and the pull-up enable would have only a transient effect.
> 
> Well look at the callback from the core:
> 
>         int (*pin_config_set) (struct pinctrl_dev *pctldev,
>                                unsigned pin,
>                                unsigned long *configs,
>                                unsigned num_configs);
> 
> You get all configs in an array. The driver can walk over the list and
> make informed decisions on what to do *BEFORE* poking any registers.
> 
> Avoiding transients as you describe is part of why the callback
> looks as it does. This is why every driver has its own for-loop.

Okay, I guess that is possible. I find usage of the arguments more
elegant since it is more explicit and reduces code in the driver, but I
suspect it should work.

	Thanks,
	Sören

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

* Re: [PATCH RFC v2 1/8] pinctrl: Add driver for Zynq
  2014-10-30  8:16           ` Lothar Waßmann
@ 2014-11-02 17:22             ` Sören Brinkmann
  0 siblings, 0 replies; 30+ messages in thread
From: Sören Brinkmann @ 2014-11-02 17:22 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: Linus Walleij, linux-arm-kernel, Steffen Trumtrar, Michal Simek,
	linux-kernel

On Thu, 2014-10-30 at 09:16AM +0100, Lothar Waßmann wrote:
> Hi,
> 
> Sören Brinkmann wrote:
> > On Wed, 2014-10-29 at 05:49AM +0100, Lothar Waßmann wrote:
> > > Hi,
> > > 
> > > Sören Brinkmann wrote:
> > > > On Tue, 2014-10-28 at 04:16PM +0100, Lothar Waßmann wrote:
> > > > > Hi,
> > > > > 
> > > > > Soren Brinkmann wrote:
> > > > > > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> > > > > > ---
> > > > > > changes since RFC:
> > > > > >  - use syscon/regmap to access registers in SLCR space
> > > > > >  - add pinctrl to zc702 DT
> > > > > >  - rebase to 3.18: rename enable -> set_mux
> > > > > >  - add kernel-doc
> > > > > >  - support pinconf
> > > > > >    - supported attributes
> > > > > >      - pin-bias: pull up, tristate, disable
> > > > > >      - slew-rate: 0 == slow, 1 == fast; generic pinconf does not display
> > > > > >        argument
> > > > > > 
> > > > > [...]
> > > > > > +struct zynq_pctrl_group zynq_pctrl_groups[] = {
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ethernet0_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ethernet1_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(mdio0_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(mdio1_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi0_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi1_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi_fbclk),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(qspi_cs1),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_1),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi0_2),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_1),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_2),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(spi1_3),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_1),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_2),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_1),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_2),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_3),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_emio_wp),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio0_emio_cd),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_emio_wp),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(sdio1_emio_cd),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor_cs1),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nor_addr25),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(smc0_nand),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_1),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_2),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_3),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_4),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_5),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_6),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_7),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_8),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_9),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can0_10),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_1),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_2),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_3),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_4),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_5),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_6),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_7),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_8),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_9),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_10),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(can1_11),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_1),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_2),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_3),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_4),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_5),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_6),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_7),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_8),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_9),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart0_10),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_1),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_2),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_3),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_4),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_5),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_6),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_7),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_8),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_9),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_10),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(uart1_11),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_1),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_2),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_3),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_4),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_5),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_6),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_7),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_8),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_9),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c0_10),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_1),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_2),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_3),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_4),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_5),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_6),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_7),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_8),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_9),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(i2c1_10),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_1),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc0_2),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_1),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(ttc1_2),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_1),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_2),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_3),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(swdt0_4),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_0),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_1),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_2),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_3),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_4),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_5),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_6),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_7),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_8),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_9),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_10),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_11),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_12),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_13),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_14),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_15),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_16),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_17),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_18),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_19),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_20),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_21),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_22),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_23),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_24),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_25),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_26),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_27),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_28),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_29),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_30),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_31),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_32),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_33),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_34),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_35),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_36),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_37),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_38),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_39),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_40),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_41),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_42),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_43),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_44),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_45),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_46),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_47),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_48),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_49),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_50),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_51),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_52),
> > > > > > +	DEFINE_ZYNQ_PINCTRL_GRP(gpio0_53)
> > > > > dto.
> > > > > 
> > > > > [...]
> > > > > > +#define DEFINE_ZYNQ_PINMUX_FUNCTION(fname, mval)	\
> > > > > > +	[ZYNQ_PMUX_##fname] = {				\
> > > > > > +		.name = #fname,				\
> > > > > > +		.groups = fname##_groups,		\
> > > > > > +		.ngroups = ARRAY_SIZE(fname##_groups),	\
> > > > > > +		.mux_val = mval				\
> > > > > > +	}
> > > > > > +
> > > > > dto.
> > > > > 
> > > > > [...]
> > > > > > +#define DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(fname, mval, mux, mask, shift)	\
> > > > > > +	[ZYNQ_PMUX_##fname] = {				\
> > > > > > +		.name = #fname,				\
> > > > > > +		.groups = fname##_groups,		\
> > > > > > +		.ngroups = ARRAY_SIZE(fname##_groups),	\
> > > > > > +		.mux_val = mval,			\
> > > > > > +		.mux_mask = mask,			\
> > > > > > +		.mux_shift = shift			\
> > > > > dto.
> > > > > 
> > > > > [...]
> > > > > > +static const struct zynq_pinmux_function zynq_pmux_functions[] = {
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet0, 1),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet1, 1),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(mdio0, 0x40),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(mdio1, 0x50),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi0, 1),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi1, 1),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi_fbclk, 1),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(qspi_cs1, 1),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(spi0, 0x50),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(spi1, 0x50),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio0, 0x40),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio0_pc, 0xc),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio0_wp, 0, 130, ZYNQ_SDIO_WP_MASK,
> > > > > > +					ZYNQ_SDIO_WP_SHIFT),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio0_cd, 0, 130, ZYNQ_SDIO_CD_MASK,
> > > > > > +					ZYNQ_SDIO_CD_SHIFT),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio1, 0x40),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(sdio1_pc, 0xc),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio1_wp, 0, 134, ZYNQ_SDIO_WP_MASK,
> > > > > > +					ZYNQ_SDIO_WP_SHIFT),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION_MUX(sdio1_cd, 0, 134, ZYNQ_SDIO_CD_MASK,
> > > > > > +					ZYNQ_SDIO_CD_SHIFT),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor, 4),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor_cs1, 8),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nor_addr25, 4),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(smc0_nand, 8),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(can0, 0x10),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(can1, 0x10),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(uart0, 0x70),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(uart1, 0x70),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(i2c0, 0x20),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(i2c1, 0x20),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ttc0, 0x60),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(ttc1, 0x60),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(swdt0, 0x30),
> > > > > > +	DEFINE_ZYNQ_PINMUX_FUNCTION(gpio0, 0)
> > > > > dto.
> > > > > 
> > > > > [...]
> > > > > > +static const struct pinctrl_ops zynq_pctrl_ops = {
> > > > > > +	.get_groups_count = zynq_pctrl_get_groups_count,
> > > > > > +	.get_group_name = zynq_pctrl_get_group_name,
> > > > > > +	.get_group_pins = zynq_pctrl_get_group_pins,
> > > > > > +	.dt_node_to_map = pinconf_generic_dt_node_to_map_group,
> > > > > > +	.dt_free_map = pinctrl_utils_dt_free_map
> > > > > dto.
> > > > 
> > > > Can we please use full sentences? But at least real words? I have no
> > > > idea what you're trying to tell me.
> > > > 
> > > Sorry, I must have accidentally removed my first comment which was:
> > > 'missing comma at end of initializer.'
> > 
> > Why should there be one? Neither gcc nor checkpatch seem to "miss" one.
> > 
> It helps further patching and avoiding possible conflicts because you
> don't need to modify an existing line when adding a new one at the end.

Guess that makes sense. I add some commas.

	Thanks,
	Sören

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

* Re: [PATCH RFC v2 8/8] ARM: zynq: DT: Add pinctrl information
  2014-10-31 17:40             ` Sören Brinkmann
@ 2014-11-02 20:20               ` Sören Brinkmann
  2014-11-03 14:40                 ` Linus Walleij
  0 siblings, 1 reply; 30+ messages in thread
From: Sören Brinkmann @ 2014-11-02 20:20 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Michal Simek, linux-kernel, linux-arm-kernel, Steffen Trumtrar

On Fri, 2014-10-31 at 10:40AM -0700, Sören Brinkmann wrote:
> On Fri, 2014-10-31 at 06:36PM +0100, Linus Walleij wrote:
> > On Fri, Oct 31, 2014 at 5:57 PM, Sören Brinkmann
> > <soren.brinkmann@xilinx.com> wrote:
> > > On Fri, 2014-10-31 at 09:17AM +0100, Linus Walleij wrote:
> > 
> > >> Again it seems to be a sequencing problem. And device tree is
> > >> not good at sequences, therefore all states should be self-contained.
> > >
> > > I agree, but how would I define a pin with pull-up enabled and
> > > tri-state disabled - assume the pin is currently in a random state that
> > > can have those things set/not set arbitrarily.
> > 
> > I was more thinking as everything you don't enable explicitly
> > in a state is per definition disabled.
> > 
> > So if you are in state A and tri-state is enabled there and you
> > move to state B where pull-up is enabled, then tri-state should
> > be disabled, since it is not explicitly enabled.
> > 
> > > I can't put bias-disable in DT since it would potentially disable both
> > > and the pull-up enable would have only a transient effect.
> > 
> > Well look at the callback from the core:
> > 
> >         int (*pin_config_set) (struct pinctrl_dev *pctldev,
> >                                unsigned pin,
> >                                unsigned long *configs,
> >                                unsigned num_configs);
> > 
> > You get all configs in an array. The driver can walk over the list and
> > make informed decisions on what to do *BEFORE* poking any registers.
> > 
> > Avoiding transients as you describe is part of why the callback
> > looks as it does. This is why every driver has its own for-loop.
> 
> Okay, I guess that is possible. I find usage of the arguments more
> elegant since it is more explicit and reduces code in the driver, but I
> suspect it should work.

It does work with some limitation though.
This was how I originally described a state in DT:
	pinctrl_uart1_default: pinctrl-uart1-default {
		common {
			groups = "uart1_10_grp";
			function = "uart1";
			bias-pull-up = <0>;
			slew-rate = <0>;
			io-standard = <1>;
		};
	
		rx {
			pins = "MIO49";
			bias-high-impedance = <1>;
		};
	
		tx {
			pins = "MIO48";
			bias-high-impedance = <0>;
		};
	};

Now, I removed the arguments for tri-state and pull-up. The problem,
this state is handled per-sub-node. I.e. one call to the cfg-set
callback per node. I.e. I cannot split things in common, rx and tx, but
I need to duplicate the pinconf props in rx and tx, resulting in:
	pinctrl_uart1_default: pinctrl-uart1-default {
		common {
			groups = "uart1_10_grp";
			function = "uart1";
		};

		rx {
			pins = "MIO49";
			slew-rate = <0>;
			io-standard = <1>;
			bias-high-impedance;
		};

		tx {
			pins = "MIO48";
			slew-rate = <0>;
			io-standard = <1>;
		};
	};

In a nutshell, yes, it's possible to work without the arguments for
pull-up or tri-state. But it adds complexity in the driver and the DT
description.

	Sören

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

* Re: [PATCH RFC v2 8/8] ARM: zynq: DT: Add pinctrl information
  2014-11-02 20:20               ` Sören Brinkmann
@ 2014-11-03 14:40                 ` Linus Walleij
  0 siblings, 0 replies; 30+ messages in thread
From: Linus Walleij @ 2014-11-03 14:40 UTC (permalink / raw)
  To: Sören Brinkmann
  Cc: Michal Simek, linux-kernel, linux-arm-kernel, Steffen Trumtrar

On Sun, Nov 2, 2014 at 9:20 PM, Sören Brinkmann
<soren.brinkmann@xilinx.com> wrote:
(...)
>                 common {
>                         groups = "uart1_10_grp";
>                         function = "uart1";
>                         bias-pull-up = <0>;
>                         slew-rate = <0>;
>                         io-standard = <1>;
>                 };
>
>                 rx {
>                         pins = "MIO49";
>                         bias-high-impedance = <1>;
>                 };
>
>                 tx {
>                         pins = "MIO48";
>                         bias-high-impedance = <0>;
>                 };
(...)
>                 common {
>                         groups = "uart1_10_grp";
>                         function = "uart1";
>                 };
>
>                 rx {
>                         pins = "MIO49";
>                         slew-rate = <0>;
>                         io-standard = <1>;
>                         bias-high-impedance;
>                 };
>
>                 tx {
>                         pins = "MIO48";
>                         slew-rate = <0>;
>                         io-standard = <1>;
>                 };
>         };
>
> In a nutshell, yes, it's possible to work without the arguments for
> pull-up or tri-state. But it adds complexity in the driver and the DT
> description.

But isn't it obvious that the latter is easier for a human to read
and understand (and not misunderstand) and remember readability
of config is one of the design goals of DT...

Yours,
Linus Walleij

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

end of thread, other threads:[~2014-11-03 14:40 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-16 17:11 [PATCH RFC v2 0/8] Pinctrl support for Zynq Soren Brinkmann
2014-10-16 17:11 ` [PATCH RFC v2 1/8] pinctrl: Add driver " Soren Brinkmann
2014-10-28 14:53   ` Linus Walleij
2014-10-28 16:14     ` Sören Brinkmann
2014-10-28 15:16   ` Lothar Waßmann
2014-10-28 16:37     ` Sören Brinkmann
2014-10-29  4:49       ` Lothar Waßmann
2014-10-29 14:12         ` Sören Brinkmann
2014-10-30  8:16           ` Lothar Waßmann
2014-11-02 17:22             ` Sören Brinkmann
2014-10-16 17:11 ` [PATCH RFC v2 2/8] pinctrl: pinconf-generic: Add flag to print arguments Soren Brinkmann
2014-10-28 14:55   ` Linus Walleij
2014-10-16 17:11 ` [PATCH RFC v2 3/8] pinctrl: pinconf-generic: Add parameter 'IO standard' Soren Brinkmann
2014-10-28 14:59   ` Linus Walleij
2014-10-28 16:07     ` Sören Brinkmann
2014-10-31  8:19       ` Linus Walleij
2014-10-16 17:11 ` [PATCH RFC v2 4/8] pinctrl: zynq: Support IO standard property Soren Brinkmann
2014-10-16 17:11 ` [PATCH RFC v2 5/8] pinctrl: zynq: Support low power mode property Soren Brinkmann
2014-10-16 17:11 ` [PATCH RFC v2 6/8] pinctrl: pinconf-generic: Infer map type from DT property Soren Brinkmann
2014-10-16 17:11 ` [PATCH RFC v2 7/8] pinctrl: zynq: Use generic map_all function Soren Brinkmann
2014-10-16 17:11 ` [PATCH RFC v2 8/8] ARM: zynq: DT: Add pinctrl information Soren Brinkmann
2014-10-21  5:54   ` Michal Simek
2014-10-28 15:05   ` Linus Walleij
2014-10-28 16:03     ` Sören Brinkmann
2014-10-31  8:17       ` Linus Walleij
2014-10-31 16:57         ` Sören Brinkmann
2014-10-31 17:36           ` Linus Walleij
2014-10-31 17:40             ` Sören Brinkmann
2014-11-02 20:20               ` Sören Brinkmann
2014-11-03 14:40                 ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).