* [PATCH RESEND 0/5] ARM: berlin: add pinctrl support @ 2014-04-10 13:07 Antoine Ténart 2014-04-10 13:07 ` [PATCH RESEND 1/5] pinctrl: allows not to define the get_group_pins operation Antoine Ténart ` (4 more replies) 0 siblings, 5 replies; 30+ messages in thread From: Antoine Ténart @ 2014-04-10 13:07 UTC (permalink / raw) To: sebastian.hesselbarth, linus.walleij Cc: Antoine Ténart, alexandre.belloni, zmxu, jszhang, linux-arm-kernel, devicetree, linux-kernel I made a typo in the LAKML address, so resend it. This series add support for the Marvell Berlin pinctrl, allowing to configure the pinmuxing from the device tree. This is a group based pinmux and only group informations are defined. For this to work a modification in the pinctrl framework is needed so that get_group_pin could not be defined. Please have a look on patches 1/5 and 2/5 commit logs. This serie applies on top of patches introducing the Marvell Berlin BG2Q you can find on Sebastian's for-next branch[1] and Alexandre's BG2Q symbol introduction[2]. Tested on the Berlin BG2Q. [1] https://github.com/shesselba/linux-berlin/commits/for-next [2] https://patchwork.kernel.org/patch/3876141/ Antoine Ténart (5): pinctrl: allows not to define the get_group_pins operation pinctrl: berlin: add a pinctrl driver for Marvell Berlin SoCs ARM: berlin: add the pinctrl dependency for the Marvell Berlin SoCs Documentation: add the Marvell Berlin pinctrl documentation ARM: dts: berlin: add the pinctrl node and muxing setup for uarts .../bindings/pinctrl/marvell,berlin-pinctrl.txt | 43 ++ arch/arm/boot/dts/berlin2.dtsi | 20 + arch/arm/boot/dts/berlin2cd.dtsi | 13 + arch/arm/boot/dts/berlin2q.dtsi | 20 + arch/arm/mach-berlin/Kconfig | 2 + drivers/pinctrl/Kconfig | 4 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/core.c | 17 +- drivers/pinctrl/pinctrl-berlin.c | 498 +++++++++++++++++++++ drivers/pinctrl/pinctrl-berlin.h | 72 +++ drivers/pinctrl/pinmux.c | 23 +- 11 files changed, 696 insertions(+), 17 deletions(-) create mode 100644 Documentation/devicetree/bindings/pinctrl/marvell,berlin-pinctrl.txt create mode 100644 drivers/pinctrl/pinctrl-berlin.c create mode 100644 drivers/pinctrl/pinctrl-berlin.h -- 1.8.3.2 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH RESEND 1/5] pinctrl: allows not to define the get_group_pins operation 2014-04-10 13:07 [PATCH RESEND 0/5] ARM: berlin: add pinctrl support Antoine Ténart @ 2014-04-10 13:07 ` Antoine Ténart 2014-04-11 9:21 ` Sebastian Hesselbarth 2014-04-22 12:48 ` Linus Walleij 2014-04-10 13:07 ` [PATCH RESEND 2/5] pinctrl: berlin: add a pinctrl driver for Marvell Berlin SoCs Antoine Ténart ` (3 subsequent siblings) 4 siblings, 2 replies; 30+ messages in thread From: Antoine Ténart @ 2014-04-10 13:07 UTC (permalink / raw) To: sebastian.hesselbarth, linus.walleij Cc: Antoine Ténart, alexandre.belloni, zmxu, jszhang, linux-arm-kernel, devicetree, linux-kernel When using a group only pinctrl driver, which does not have any information on the pins it is useless to define a get_group_pins always returning an empty list of pins. When not using get_group_pin[1], a driver must implement it so pins = NULL and num_pins = 0. This patch makes it the default behaviour if not defined in the pinctrl driver when used in pinmux enable and disable funtions and in pinctrl_groups_show. It also adds a check in pinctrl_get_group_pins and return -EINVAL if not defined. This function is called in the gpiolib when adding when pingroup range. It cannot be used if no group is defined, so this seams reasonable. [1] get_group_pin(struct pinctrl_dev *pctldev, unsigned selector, const unsigned **pins, unsigned *num_pins); Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> --- drivers/pinctrl/core.c | 17 ++++++++++------- drivers/pinctrl/pinmux.c | 23 +++++++++++++---------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c index c0fe6091566a..e09474ecde23 100644 --- a/drivers/pinctrl/core.c +++ b/drivers/pinctrl/core.c @@ -468,6 +468,9 @@ int pinctrl_get_group_pins(struct pinctrl_dev *pctldev, const char *pin_group, const struct pinctrl_ops *pctlops = pctldev->desc->pctlops; int gs; + if (!pctlops->get_group_pins) + return -EINVAL; + gs = pinctrl_get_group_selector(pctldev, pin_group); if (gs < 0) return gs; @@ -1362,15 +1365,16 @@ static int pinctrl_groups_show(struct seq_file *s, void *what) seq_puts(s, "registered pin groups:\n"); while (selector < ngroups) { - const unsigned *pins; - unsigned num_pins; + const unsigned *pins = NULL; + unsigned num_pins = 0; const char *gname = ops->get_group_name(pctldev, selector); const char *pname; - int ret; + int ret = 0; int i; - ret = ops->get_group_pins(pctldev, selector, - &pins, &num_pins); + if (ops->get_group_pins) + ret = ops->get_group_pins(pctldev, selector, + &pins, &num_pins); if (ret) seq_printf(s, "%s [ERROR GETTING PINS]\n", gname); @@ -1694,8 +1698,7 @@ static int pinctrl_check_ops(struct pinctrl_dev *pctldev) if (!ops || !ops->get_groups_count || - !ops->get_group_name || - !ops->get_group_pins) + !ops->get_group_name) return -EINVAL; if (ops->dt_node_to_map && !ops->dt_free_map) diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c index 9248ce4efed4..051e8592990e 100644 --- a/drivers/pinctrl/pinmux.c +++ b/drivers/pinctrl/pinmux.c @@ -391,14 +391,16 @@ int pinmux_enable_setting(struct pinctrl_setting const *setting) struct pinctrl_dev *pctldev = setting->pctldev; const struct pinctrl_ops *pctlops = pctldev->desc->pctlops; const struct pinmux_ops *ops = pctldev->desc->pmxops; - int ret; - const unsigned *pins; - unsigned num_pins; + int ret = 0; + const unsigned *pins = NULL; + unsigned num_pins = 0; int i; struct pin_desc *desc; - ret = pctlops->get_group_pins(pctldev, setting->data.mux.group, - &pins, &num_pins); + if (pctlops->get_group_pins) + ret = pctlops->get_group_pins(pctldev, setting->data.mux.group, + &pins, &num_pins); + if (ret) { const char *gname; @@ -470,14 +472,15 @@ void pinmux_disable_setting(struct pinctrl_setting const *setting) struct pinctrl_dev *pctldev = setting->pctldev; const struct pinctrl_ops *pctlops = pctldev->desc->pctlops; const struct pinmux_ops *ops = pctldev->desc->pmxops; - int ret; - const unsigned *pins; - unsigned num_pins; + int ret = 0; + const unsigned *pins = NULL; + unsigned num_pins = 0; int i; struct pin_desc *desc; - ret = pctlops->get_group_pins(pctldev, setting->data.mux.group, - &pins, &num_pins); + if (pctlops->get_group_pins) + ret = pctlops->get_group_pins(pctldev, setting->data.mux.group, + &pins, &num_pins); if (ret) { const char *gname; -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 1/5] pinctrl: allows not to define the get_group_pins operation 2014-04-10 13:07 ` [PATCH RESEND 1/5] pinctrl: allows not to define the get_group_pins operation Antoine Ténart @ 2014-04-11 9:21 ` Sebastian Hesselbarth 2014-04-22 12:48 ` Linus Walleij 1 sibling, 0 replies; 30+ messages in thread From: Sebastian Hesselbarth @ 2014-04-11 9:21 UTC (permalink / raw) To: Antoine Ténart, linus.walleij Cc: alexandre.belloni, zmxu, jszhang, linux-arm-kernel, devicetree, linux-kernel On 04/10/2014 03:07 PM, Antoine Ténart wrote: > When using a group only pinctrl driver, which does not have any > information on the pins it is useless to define a get_group_pins > always returning an empty list of pins. > > When not using get_group_pin[1], a driver must implement it so > pins = NULL and num_pins = 0. This patch makes it the default > behaviour if not defined in the pinctrl driver when used in > pinmux enable and disable funtions and in pinctrl_groups_show. > > It also adds a check in pinctrl_get_group_pins and return -EINVAL if > not defined. This function is called in the gpiolib when adding when > pingroup range. It cannot be used if no group is defined, so this seams > reasonable. I don't have a strong opinion about making pins optional for pinctrl drivers. I'll leave it for Linus to decide on it. But have you checked all the other places pins are referred to? One place that jumps into my mind would be the DEBUGFS files... Sebastian > [1] get_group_pin(struct pinctrl_dev *pctldev, > unsigned selector, > const unsigned **pins, > unsigned *num_pins); > > Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> > --- > drivers/pinctrl/core.c | 17 ++++++++++------- > drivers/pinctrl/pinmux.c | 23 +++++++++++++---------- > 2 files changed, 23 insertions(+), 17 deletions(-) > > diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c > index c0fe6091566a..e09474ecde23 100644 > --- a/drivers/pinctrl/core.c > +++ b/drivers/pinctrl/core.c > @@ -468,6 +468,9 @@ int pinctrl_get_group_pins(struct pinctrl_dev *pctldev, const char *pin_group, > const struct pinctrl_ops *pctlops = pctldev->desc->pctlops; > int gs; > > + if (!pctlops->get_group_pins) > + return -EINVAL; > + > gs = pinctrl_get_group_selector(pctldev, pin_group); > if (gs < 0) > return gs; > @@ -1362,15 +1365,16 @@ static int pinctrl_groups_show(struct seq_file *s, void *what) > > seq_puts(s, "registered pin groups:\n"); > while (selector < ngroups) { > - const unsigned *pins; > - unsigned num_pins; > + const unsigned *pins = NULL; > + unsigned num_pins = 0; > const char *gname = ops->get_group_name(pctldev, selector); > const char *pname; > - int ret; > + int ret = 0; > int i; > > - ret = ops->get_group_pins(pctldev, selector, > - &pins, &num_pins); > + if (ops->get_group_pins) > + ret = ops->get_group_pins(pctldev, selector, > + &pins, &num_pins); > if (ret) > seq_printf(s, "%s [ERROR GETTING PINS]\n", > gname); > @@ -1694,8 +1698,7 @@ static int pinctrl_check_ops(struct pinctrl_dev *pctldev) > > if (!ops || > !ops->get_groups_count || > - !ops->get_group_name || > - !ops->get_group_pins) > + !ops->get_group_name) > return -EINVAL; > > if (ops->dt_node_to_map && !ops->dt_free_map) > diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c > index 9248ce4efed4..051e8592990e 100644 > --- a/drivers/pinctrl/pinmux.c > +++ b/drivers/pinctrl/pinmux.c > @@ -391,14 +391,16 @@ int pinmux_enable_setting(struct pinctrl_setting const *setting) > struct pinctrl_dev *pctldev = setting->pctldev; > const struct pinctrl_ops *pctlops = pctldev->desc->pctlops; > const struct pinmux_ops *ops = pctldev->desc->pmxops; > - int ret; > - const unsigned *pins; > - unsigned num_pins; > + int ret = 0; > + const unsigned *pins = NULL; > + unsigned num_pins = 0; > int i; > struct pin_desc *desc; > > - ret = pctlops->get_group_pins(pctldev, setting->data.mux.group, > - &pins, &num_pins); > + if (pctlops->get_group_pins) > + ret = pctlops->get_group_pins(pctldev, setting->data.mux.group, > + &pins, &num_pins); > + > if (ret) { > const char *gname; > > @@ -470,14 +472,15 @@ void pinmux_disable_setting(struct pinctrl_setting const *setting) > struct pinctrl_dev *pctldev = setting->pctldev; > const struct pinctrl_ops *pctlops = pctldev->desc->pctlops; > const struct pinmux_ops *ops = pctldev->desc->pmxops; > - int ret; > - const unsigned *pins; > - unsigned num_pins; > + int ret = 0; > + const unsigned *pins = NULL; > + unsigned num_pins = 0; > int i; > struct pin_desc *desc; > > - ret = pctlops->get_group_pins(pctldev, setting->data.mux.group, > - &pins, &num_pins); > + if (pctlops->get_group_pins) > + ret = pctlops->get_group_pins(pctldev, setting->data.mux.group, > + &pins, &num_pins); > if (ret) { > const char *gname; > > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 1/5] pinctrl: allows not to define the get_group_pins operation 2014-04-10 13:07 ` [PATCH RESEND 1/5] pinctrl: allows not to define the get_group_pins operation Antoine Ténart 2014-04-11 9:21 ` Sebastian Hesselbarth @ 2014-04-22 12:48 ` Linus Walleij 2014-04-22 15:58 ` Antoine Ténart 1 sibling, 1 reply; 30+ messages in thread From: Linus Walleij @ 2014-04-22 12:48 UTC (permalink / raw) To: Antoine Ténart Cc: Sebastian Hesselbarth, Alexandre Belloni, zmxu, Jisheng Zhang, linux-arm-kernel, devicetree, linux-kernel On Thu, Apr 10, 2014 at 3:07 PM, Antoine Ténart <antoine.tenart@free-electrons.com> wrote: > When using a group only pinctrl driver, which does not have any > information on the pins it is useless to define a get_group_pins > always returning an empty list of pins. > > When not using get_group_pin[1], a driver must implement it so > pins = NULL and num_pins = 0. This patch makes it the default > behaviour if not defined in the pinctrl driver when used in > pinmux enable and disable funtions and in pinctrl_groups_show. > > It also adds a check in pinctrl_get_group_pins and return -EINVAL if > not defined. This function is called in the gpiolib when adding when > pingroup range. It cannot be used if no group is defined, so this seams > reasonable. > > [1] get_group_pin(struct pinctrl_dev *pctldev, > unsigned selector, > const unsigned **pins, > unsigned *num_pins); > > Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> OK makes perfect sense. Patch applied. Can you please check the Documentation/pinctrl.txt to see if anything needs to be updated due to this? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 1/5] pinctrl: allows not to define the get_group_pins operation 2014-04-22 12:48 ` Linus Walleij @ 2014-04-22 15:58 ` Antoine Ténart 0 siblings, 0 replies; 30+ messages in thread From: Antoine Ténart @ 2014-04-22 15:58 UTC (permalink / raw) To: Linus Walleij Cc: Sebastian Hesselbarth, Alexandre Belloni, zmxu, Jisheng Zhang, linux-arm-kernel, devicetree, linux-kernel Linus, On Tue, Apr 22, 2014 at 02:48:04PM +0200, Linus Walleij wrote: > On Thu, Apr 10, 2014 at 3:07 PM, Antoine Ténart > <antoine.tenart@free-electrons.com> wrote: > > > When using a group only pinctrl driver, which does not have any > > information on the pins it is useless to define a get_group_pins > > always returning an empty list of pins. > > > > When not using get_group_pin[1], a driver must implement it so > > pins = NULL and num_pins = 0. This patch makes it the default > > behaviour if not defined in the pinctrl driver when used in > > pinmux enable and disable funtions and in pinctrl_groups_show. > > > > It also adds a check in pinctrl_get_group_pins and return -EINVAL if > > not defined. This function is called in the gpiolib when adding when > > pingroup range. It cannot be used if no group is defined, so this seams > > reasonable. > > > > [1] get_group_pin(struct pinctrl_dev *pctldev, > > unsigned selector, > > const unsigned **pins, > > unsigned *num_pins); > > > > Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> > > OK makes perfect sense. > > Patch applied. Thanks! > Can you please check the Documentation/pinctrl.txt to see > if anything needs to be updated due to this? I just checked and did not see needed updates due to this patch. Antoine -- Antoine Ténart, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH RESEND 2/5] pinctrl: berlin: add a pinctrl driver for Marvell Berlin SoCs 2014-04-10 13:07 [PATCH RESEND 0/5] ARM: berlin: add pinctrl support Antoine Ténart 2014-04-10 13:07 ` [PATCH RESEND 1/5] pinctrl: allows not to define the get_group_pins operation Antoine Ténart @ 2014-04-10 13:07 ` Antoine Ténart 2014-04-11 6:44 ` Jisheng Zhang 2014-04-11 9:03 ` Sebastian Hesselbarth 2014-04-10 13:07 ` [PATCH RESEND 3/5] ARM: berlin: add the pinctrl dependency for the " Antoine Ténart ` (2 subsequent siblings) 4 siblings, 2 replies; 30+ messages in thread From: Antoine Ténart @ 2014-04-10 13:07 UTC (permalink / raw) To: sebastian.hesselbarth, linus.walleij Cc: Antoine Ténart, alexandre.belloni, zmxu, jszhang, linux-arm-kernel, devicetree, linux-kernel The Marvell Berlin boards have a group based pinmuxing mechanism. This driver adds the support for the BG2CD, BG2 and BG2Q. We actually do not need any information about the pins here and only have the definition of the groups. Let's take the example of the uart0 pinmuxing on the BG2Q. Balls BK4 and BH6 are muxed to respectively UART0 RX and TX if the group GSM12 is set to mode 0: Group Modes Offset Base Offset LSB Bit Width GSM12 3 sm_base 0x40 0x10 0x2 Ball Group Mode 0 Mode 1 Mode 2 BK4 GSM12 UART0_RX IrDA0_RX GPIO9 BH6 GSM12 UART0_TX IrDA0_TX GPIO10 So in order to configure BK4 -> UART0_TX and BH6 -> UART0_RX, we need to set (sm_base + 0x40 + 0x10) &= ff3fffff. Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> --- drivers/pinctrl/Kconfig | 4 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-berlin.c | 498 +++++++++++++++++++++++++++++++++++++++ drivers/pinctrl/pinctrl-berlin.h | 72 ++++++ 4 files changed, 575 insertions(+) create mode 100644 drivers/pinctrl/pinctrl-berlin.c create mode 100644 drivers/pinctrl/pinctrl-berlin.h diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index e49324032611..2d9339a7bd05 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -104,6 +104,10 @@ config PINCTRL_BCM2835 select PINMUX select PINCONF +config PINCTRL_BERLIN + bool + select PINMUX + config PINCTRL_CAPRI bool "Broadcom Capri pinctrl driver" depends on OF diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile index 4b835880cf80..fd5a01d4475f 100644 --- a/drivers/pinctrl/Makefile +++ b/drivers/pinctrl/Makefile @@ -21,6 +21,7 @@ obj-$(CONFIG_PINCTRL_BF60x) += pinctrl-adi2-bf60x.o obj-$(CONFIG_PINCTRL_AT91) += pinctrl-at91.o obj-$(CONFIG_PINCTRL_BCM2835) += pinctrl-bcm2835.o obj-$(CONFIG_PINCTRL_BAYTRAIL) += pinctrl-baytrail.o +obj-$(CONFIG_PINCTRL_BERLIN) += pinctrl-berlin.o obj-$(CONFIG_PINCTRL_CAPRI) += pinctrl-capri.o obj-$(CONFIG_PINCTRL_IMX) += pinctrl-imx.o obj-$(CONFIG_PINCTRL_IMX1_CORE) += pinctrl-imx1-core.o diff --git a/drivers/pinctrl/pinctrl-berlin.c b/drivers/pinctrl/pinctrl-berlin.c new file mode 100644 index 000000000000..a377d6fbb127 --- /dev/null +++ b/drivers/pinctrl/pinctrl-berlin.c @@ -0,0 +1,498 @@ +/* + * Marvell Berlin SoC pinctrl driver. + * + * Copyright (C) 2014 Marvell Technology Group Ltd. + * + * Antoine Ténart <antoine.tenart@free-electrons.com> + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#include <linux/io.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/of_device.h> +#include <linux/pinctrl/pinctrl.h> +#include <linux/pinctrl/pinmux.h> +#include <linux/platform_device.h> +#include <linux/slab.h> + +#include "core.h" +#include "pinctrl-utils.h" +#include "pinctrl-berlin.h" + +#define NFCTS(bit_width) (1 << ((bit_width) + 1)) + +static const struct berlin_desc_group berlin2cd_pinctrl_groups[] = { + /* GSM */ + BERLIN_PINCTRL_GROUP("GSM0", "apb_base", 0x40, 0x2, 0x00), + BERLIN_PINCTRL_GROUP("GSM1", "apb_base", 0x40, 0x2, 0x02), + BERLIN_PINCTRL_GROUP("GSM2", "apb_base", 0x40, 0x2, 0x04), + BERLIN_PINCTRL_GROUP("GSM3", "apb_base", 0x40, 0x2, 0x06), + BERLIN_PINCTRL_GROUP("GSM4", "apb_base", 0x40, 0x2, 0x08), + BERLIN_PINCTRL_GROUP("GSM5", "apb_base", 0x40, 0x2, 0x0a), + BERLIN_PINCTRL_GROUP("GSM6", "apb_base", 0x40, 0x2, 0x0c), + BERLIN_PINCTRL_GROUP("GSM7", "apb_base", 0x40, 0x1, 0x0e), + BERLIN_PINCTRL_GROUP("GSM8", "apb_base", 0x40, 0x1, 0x0f), + BERLIN_PINCTRL_GROUP("GSM9", "apb_base", 0x40, 0x1, 0x10), + BERLIN_PINCTRL_GROUP("GSM10", "apb_base", 0x40, 0x1, 0x11), + BERLIN_PINCTRL_GROUP("GSM11", "apb_base", 0x40, 0x1, 0x12), + /* G */ + BERLIN_PINCTRL_GROUP("G0", "global_base", 0x00, 0x1, 0x00), + BERLIN_PINCTRL_GROUP("G1", "global_base", 0x00, 0x2, 0x01), + BERLIN_PINCTRL_GROUP("G2", "global_base", 0x00, 0x2, 0x02), + BERLIN_PINCTRL_GROUP("G3", "global_base", 0x00, 0x2, 0x04), + BERLIN_PINCTRL_GROUP("G4", "global_base", 0x00, 0x2, 0x06), + BERLIN_PINCTRL_GROUP("G5", "global_base", 0x00, 0x3, 0x08), + BERLIN_PINCTRL_GROUP("G6", "global_base", 0x00, 0x2, 0x0b), + BERLIN_PINCTRL_GROUP("G7", "global_base", 0x00, 0x3, 0x0d), + BERLIN_PINCTRL_GROUP("G8", "global_base", 0x00, 0x3, 0x10), + BERLIN_PINCTRL_GROUP("G9", "global_base", 0x00, 0x3, 0x13), + BERLIN_PINCTRL_GROUP("G10", "global_base", 0x00, 0x2, 0x16), + BERLIN_PINCTRL_GROUP("G11", "global_base", 0x00, 0x2, 0x18), + BERLIN_PINCTRL_GROUP("G12", "global_base", 0x00, 0x3, 0x1a), + BERLIN_PINCTRL_GROUP("G13", "global_base", 0x04, 0x3, 0x00), + BERLIN_PINCTRL_GROUP("G14", "global_base", 0x04, 0x1, 0x03), + BERLIN_PINCTRL_GROUP("G15", "global_base", 0x04, 0x2, 0x04), + BERLIN_PINCTRL_GROUP("G16", "global_base", 0x04, 0x3, 0x06), + BERLIN_PINCTRL_GROUP("G17", "global_base", 0x04, 0x3, 0x09), + BERLIN_PINCTRL_GROUP("G18", "global_base", 0x04, 0x1, 0x0c), + BERLIN_PINCTRL_GROUP("G19", "global_base", 0x04, 0x1, 0x0d), + BERLIN_PINCTRL_GROUP("G20", "global_base", 0x04, 0x1, 0x0e), + BERLIN_PINCTRL_GROUP("G21", "global_base", 0x04, 0x3, 0x0f), + BERLIN_PINCTRL_GROUP("G22", "global_base", 0x04, 0x3, 0x12), + BERLIN_PINCTRL_GROUP("G23", "global_base", 0x04, 0x3, 0x15), + BERLIN_PINCTRL_GROUP("G24", "global_base", 0x04, 0x2, 0x18), + BERLIN_PINCTRL_GROUP("G25", "global_base", 0x04, 0x2, 0x1a), + BERLIN_PINCTRL_GROUP("G26", "global_base", 0x04, 0x1, 0x1c), + BERLIN_PINCTRL_GROUP("G27", "global_base", 0x04, 0x1, 0x1d), + BERLIN_PINCTRL_GROUP("G28", "global_base", 0x04, 0x2, 0x1e), +}; + +static const struct berlin_desc_group berlin2q_pinctrl_groups[] = { + /* GSM */ + BERLIN_PINCTRL_GROUP("GSM0", "sm_base", 0x40, 0x2, 0x00), + BERLIN_PINCTRL_GROUP("GSM1", "sm_base", 0x40, 0x2, 0x02), + BERLIN_PINCTRL_GROUP("GSM2", "sm_base", 0x40, 0x2, 0x04), + BERLIN_PINCTRL_GROUP("GSM3", "sm_base", 0x40, 0x2, 0x06), + BERLIN_PINCTRL_GROUP("GSM4", "sm_base", 0x40, 0x1, 0x08), + BERLIN_PINCTRL_GROUP("GSM5", "sm_base", 0x40, 0x1, 0x09), + BERLIN_PINCTRL_GROUP("GSM6", "sm_base", 0x40, 0x1, 0x0a), + BERLIN_PINCTRL_GROUP("GSM7", "sm_base", 0x40, 0x1, 0x0b), + BERLIN_PINCTRL_GROUP("GSM8", "sm_base", 0x40, 0x1, 0x0c), + BERLIN_PINCTRL_GROUP("GSM9", "sm_base", 0x40, 0x1, 0x0d), + BERLIN_PINCTRL_GROUP("GSM10", "sm_base", 0x40, 0x1, 0x0e), + BERLIN_PINCTRL_GROUP("GSM11", "sm_base", 0x40, 0x1, 0x0f), + BERLIN_PINCTRL_GROUP("GSM12", "sm_base", 0x40, 0x2, 0x10), + BERLIN_PINCTRL_GROUP("GSM13", "sm_base", 0x40, 0x2, 0x12), + BERLIN_PINCTRL_GROUP("GSM14", "sm_base", 0x40, 0x2, 0x14), + BERLIN_PINCTRL_GROUP("GSM15", "sm_base", 0x40, 0x2, 0x16), + BERLIN_PINCTRL_GROUP("GSM16", "sm_base", 0x40, 0x1, 0x18), + BERLIN_PINCTRL_GROUP("GSM17", "sm_base", 0x40, 0x1, 0x19), + BERLIN_PINCTRL_GROUP("GSM18", "sm_base", 0x40, 0x1, 0x1a), + /* G */ + BERLIN_PINCTRL_GROUP("G0", "global_base", 0x18, 0x3, 0x00), + BERLIN_PINCTRL_GROUP("G1", "global_base", 0x18, 0x3, 0x03), + BERLIN_PINCTRL_GROUP("G2", "global_base", 0x18, 0x3, 0x06), + BERLIN_PINCTRL_GROUP("G3", "global_base", 0x18, 0x3, 0x09), + BERLIN_PINCTRL_GROUP("G4", "global_base", 0x18, 0x3, 0x0c), + BERLIN_PINCTRL_GROUP("G5", "global_base", 0x18, 0x3, 0x0f), + BERLIN_PINCTRL_GROUP("G6", "global_base", 0x18, 0x3, 0x12), + BERLIN_PINCTRL_GROUP("G7", "global_base", 0x18, 0x3, 0x15), + BERLIN_PINCTRL_GROUP("G8", "global_base", 0x18, 0x3, 0x18), + BERLIN_PINCTRL_GROUP("G9", "global_base", 0x18, 0x3, 0x1b), + BERLIN_PINCTRL_GROUP("G10", "global_base", 0x1c, 0x3, 0x00), + BERLIN_PINCTRL_GROUP("G11", "global_base", 0x1c, 0x3, 0x03), + BERLIN_PINCTRL_GROUP("G12", "global_base", 0x1c, 0x3, 0x06), + BERLIN_PINCTRL_GROUP("G13", "global_base", 0x1c, 0x3, 0x09), + BERLIN_PINCTRL_GROUP("G14", "global_base", 0x1c, 0x3, 0x0c), + BERLIN_PINCTRL_GROUP("G15", "global_base", 0x1c, 0x3, 0x0f), + BERLIN_PINCTRL_GROUP("G16", "global_base", 0x1c, 0x3, 0x12), + BERLIN_PINCTRL_GROUP("G17", "global_base", 0x1c, 0x3, 0x15), + BERLIN_PINCTRL_GROUP("G18", "global_base", 0x1c, 0x3, 0x18), + BERLIN_PINCTRL_GROUP("G19", "global_base", 0x1c, 0x3, 0x1b), + BERLIN_PINCTRL_GROUP("G20", "global_base", 0x20, 0x3, 0x00), + BERLIN_PINCTRL_GROUP("G21", "global_base", 0x20, 0x3, 0x03), + BERLIN_PINCTRL_GROUP("G22", "global_base", 0x20, 0x3, 0x06), + BERLIN_PINCTRL_GROUP("G23", "global_base", 0x20, 0x3, 0x09), + BERLIN_PINCTRL_GROUP("G24", "global_base", 0x20, 0x3, 0x0c), + BERLIN_PINCTRL_GROUP("G25", "global_base", 0x20, 0x3, 0x0f), + BERLIN_PINCTRL_GROUP("G26", "global_base", 0x20, 0x3, 0x12), + BERLIN_PINCTRL_GROUP("G27", "global_base", 0x20, 0x3, 0x15), + BERLIN_PINCTRL_GROUP("G28", "global_base", 0x20, 0x3, 0x18), + BERLIN_PINCTRL_GROUP("G29", "global_base", 0x20, 0x3, 0x1b), + BERLIN_PINCTRL_GROUP("G30", "global_base", 0x24, 0x3, 0x00), + BERLIN_PINCTRL_GROUP("G31", "global_base", 0x24, 0x3, 0x03), + BERLIN_PINCTRL_GROUP("G32", "global_base", 0x24, 0x3, 0x06), + /* GAV */ + BERLIN_PINCTRL_GROUP("GAV0", "global_base", 0x24, 0x3, 0x09), + BERLIN_PINCTRL_GROUP("GAV1", "global_base", 0x24, 0x3, 0x0c), + BERLIN_PINCTRL_GROUP("GAV2", "global_base", 0x24, 0x3, 0x0f), + BERLIN_PINCTRL_GROUP("GAV3", "global_base", 0x24, 0x3, 0x12), + BERLIN_PINCTRL_GROUP("GAV4", "global_base", 0x24, 0x3, 0x15), + BERLIN_PINCTRL_GROUP("GAV5", "global_base", 0x24, 0x3, 0x18), + BERLIN_PINCTRL_GROUP("GAV6", "global_base", 0x24, 0x3, 0x1b), + BERLIN_PINCTRL_GROUP("GAV7", "global_base", 0x28, 0x3, 0x00), + BERLIN_PINCTRL_GROUP("GAV8", "global_base", 0x28, 0x3, 0x03), + BERLIN_PINCTRL_GROUP("GAV9", "global_base", 0x28, 0x3, 0x06), + BERLIN_PINCTRL_GROUP("GAV10", "global_base", 0x28, 0x3, 0x09), + BERLIN_PINCTRL_GROUP("GAV11", "global_base", 0x28, 0x3, 0x0c), + BERLIN_PINCTRL_GROUP("GAV12", "global_base", 0x28, 0x3, 0x0f), + BERLIN_PINCTRL_GROUP("GAV13", "global_base", 0x28, 0x3, 0x12), + BERLIN_PINCTRL_GROUP("GAV14", "global_base", 0x28, 0x3, 0x15), + BERLIN_PINCTRL_GROUP("GAV15", "global_base", 0x28, 0x3, 0x18), + BERLIN_PINCTRL_GROUP("GAV16", "global_base", 0x28, 0x3, 0x1b), + BERLIN_PINCTRL_GROUP("GAV17", "global_base", 0x2c, 0x3, 0x00), + BERLIN_PINCTRL_GROUP("GAV18", "global_base", 0x2c, 0x3, 0x03), + BERLIN_PINCTRL_GROUP("GAV19", "global_base", 0x2c, 0x3, 0x06), +}; + +static int berlin_pinctrl_get_group_count(struct pinctrl_dev *pctrl_dev) +{ + struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); + + return pctrl->ngroups; +} + +static const char *berlin_pinctrl_get_group_name(struct pinctrl_dev *pctrl_dev, + unsigned group) +{ + struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); + + return pctrl->groups[group].name; +} + +static int berlin_pinctrl_dt_node_to_map(struct pinctrl_dev *pctrl_dev, + struct device_node *node, + struct pinctrl_map **map, + unsigned *num_maps) +{ + struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); + const char *group_name; + char *function_name; + unsigned reserved_maps = 0; + u32 function; + int ret; + + *map = NULL; + *num_maps = 0; + + ret = pinctrl_utils_reserve_map(pctrl_dev, map, &reserved_maps, + num_maps, 1); + if (ret) { + dev_err(pctrl->dev, "can't reserve map: %d\n", ret); + return ret; + } + + ret = of_property_read_u32(node, "berlin,function", &function); + if (ret) { + dev_err(pctrl->dev, + "missing 'berlin,function' property in node %s\n", + node->name); + return -EINVAL; + } + + ret = of_property_read_string(node, "berlin,group", &group_name); + if (ret) { + dev_err(pctrl->dev, + "missing 'berlin,group' property in node %s\n", + node->name); + return -EINVAL; + } + + function_name = kzalloc(strlen(group_name) + 7, GFP_KERNEL); + if (!function_name) + return -ENOMEM; + snprintf(function_name, strlen(group_name) + 7, "%s_mode%d", group_name, + function); + + ret = pinctrl_utils_add_map_mux(pctrl_dev, map, &reserved_maps, + num_maps, group_name, function_name); + if (ret) { + dev_err(pctrl->dev, "can't add map: %d\n", ret); + return ret; + } + + return 0; +} + +static void berlin_pinctrl_dt_free_map(struct pinctrl_dev *pctrl_dev, + struct pinctrl_map *map, + unsigned nmaps) +{ + int i; + for (i = 0; i < nmaps; i++) { + if (map[i].type == PIN_MAP_TYPE_MUX_GROUP) { + kfree(map[i].data.mux.group); + kfree(map[i].data.mux.function); + } + } + + kfree(map); +} + +static const struct pinctrl_ops berlin_pinctrl_ops = { + .get_groups_count = &berlin_pinctrl_get_group_count, + .get_group_name = &berlin_pinctrl_get_group_name, + .dt_node_to_map = &berlin_pinctrl_dt_node_to_map, + .dt_free_map = &berlin_pinctrl_dt_free_map, +}; + +static int berlin_pinmux_get_functions_count(struct pinctrl_dev *pctrl_dev) +{ + struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); + + return pctrl->nfunctions; +} + +static const char *berlin_pinmux_get_function_name(struct pinctrl_dev *pctrl_dev, + unsigned function) +{ + struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); + + return pctrl->functions[function].name; +} + +static int berlin_pinmux_get_function_groups(struct pinctrl_dev *pctrl_dev, + unsigned function, + const char * const **groups, + unsigned * const num_groups) +{ + struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); + + *groups = &pctrl->functions[function].group; + *num_groups = 1; + + return 0; +} + +static int berlin_pinmux_enable(struct pinctrl_dev *pctrl_dev, + unsigned function, + unsigned group) +{ + struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); + struct berlin_pinctrl_group *group_desc = pctrl->groups + group; + struct berlin_pinctrl_function *function_desc = + pctrl->functions + function; + unsigned long flags; + u32 regval; + + spin_lock_irqsave(&pctrl->lock, flags); + + regval = readl(group_desc->reg); + regval &= group_desc->mask; + regval |= function_desc->muxval << group_desc->lsb; + writel(regval, group_desc->reg); + + spin_unlock_irqrestore(&pctrl->lock, flags); + + return 0; +} + +static const struct pinmux_ops berlin_pinmux_ops = { + .get_functions_count = &berlin_pinmux_get_functions_count, + .get_function_name = &berlin_pinmux_get_function_name, + .get_function_groups = &berlin_pinmux_get_function_groups, + .enable = &berlin_pinmux_enable, +}; + +static int berlin_pinctrl_build_state(struct platform_device *pdev, + struct berlin_pinctrl_reg_base *bases, + unsigned nbases) +{ + struct berlin_pinctrl *pctrl = platform_get_drvdata(pdev); + int i, j, cur_fct = 0; + + pctrl->ngroups = pctrl->desc->ngroups; + pctrl->nfunctions = 0; + + for (i = 0; i < pctrl->ngroups; i++) { + struct berlin_desc_group const *desc = pctrl->desc->groups + i; + pctrl->nfunctions += NFCTS(desc->bit_width); + } + + pctrl->groups = devm_kzalloc(&pdev->dev, + pctrl->ngroups * sizeof(struct berlin_pinctrl_group), + GFP_KERNEL); + if (!pctrl->groups) + return -ENOMEM; + + pctrl->functions = devm_kzalloc(&pdev->dev, + pctrl->ngroups * pctrl->nfunctions * + sizeof(struct berlin_pinctrl_function), + GFP_KERNEL); + if (!pctrl->functions) + return -ENOMEM; + + for (i = 0; i < pctrl->ngroups; i++) { + struct berlin_desc_group const *desc = pctrl->desc->groups + i; + struct berlin_pinctrl_group *group = pctrl->groups + i; + + group->name = desc->name; + group->mask = GENMASK(desc->lsb + desc->bit_width - 1, + desc->lsb); + group->lsb = desc->lsb; + + for (j = 0; j < nbases; j++) { + if (!strcmp(desc->base_name, bases[j].name)) { + group->reg = bases[j].base + desc->offset; + break; + } + } + + if (j == nbases) { + dev_err(pctrl->dev, "cannot find base address for %s\n", + desc->base_name); + return -EINVAL; + } + + for (j = 0; j < NFCTS(desc->bit_width); j++) { + struct berlin_pinctrl_function *function = + pctrl->functions + cur_fct; + char *function_name = devm_kzalloc(&pdev->dev, + (strlen(desc->name) + 7) * sizeof(char), + GFP_KERNEL); + if (!function_name) + return -ENOMEM; + + snprintf(function_name, strlen(desc->name) + 7, + "%s_mode%d", desc->name, j); + + function->name = function_name; + function->group = desc->name; + function->muxval = j; + + cur_fct++; + } + } + + return 0; +} + +static struct pinctrl_desc berlin_pctrl_desc = { + .name = "berlin-pinctrl", + .pctlops = &berlin_pinctrl_ops, + .pmxops = &berlin_pinmux_ops, + .owner = THIS_MODULE, +}; + +static const struct berlin_pinctrl_desc berlin2cd_pinctrl_data = { + .groups = berlin2cd_pinctrl_groups, + .ngroups = ARRAY_SIZE(berlin2cd_pinctrl_groups), +}; + +static const struct berlin_pinctrl_desc berlin2q_pinctrl_data = { + .groups = berlin2q_pinctrl_groups, + .ngroups = ARRAY_SIZE(berlin2q_pinctrl_groups), +}; + +static struct of_device_id berlin_pinctrl_match[] = { + { + .compatible = "marvell,berlin2cd-pinctrl", + .data = &berlin2cd_pinctrl_data + }, + { + .compatible = "marvell,berlin2-pinctrl", + .data = &berlin2cd_pinctrl_data + }, + { + .compatible = "marvell,berlin2q-pinctrl", + .data = &berlin2q_pinctrl_data + }, + {} +}; +MODULE_DEVICE_TABLE(of, berlin_pinctrl_match); + +static int berlin_pinctrl_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct device_node *node; + struct property *prop; + struct berlin_pinctrl *pctrl; + struct berlin_pinctrl_reg_base *bases; + const struct of_device_id *device; + const char *reg_name; + int ret, i = 0; + unsigned nbases; + + if (!dev->of_node) { + dev_err(dev, "device node not found\n"); + return -ENODEV; + } + node = dev->of_node; + + pctrl = devm_kzalloc(dev, sizeof(struct berlin_pinctrl), GFP_KERNEL); + if (!pctrl) + return -ENOMEM; + + platform_set_drvdata(pdev, pctrl); + + spin_lock_init(&pctrl->lock); + + device = of_match_device(berlin_pinctrl_match, dev); + if (!device) { + dev_err(dev, "pinctrl didn't match\n"); + return -ENODEV; + } + + nbases = of_property_count_strings(node, "reg-names"); + if (nbases < 0) { + dev_err(dev, "missing 'reg-names' property in node %s\n", + node->name); + return -EINVAL; + } + + bases = devm_kzalloc(dev, + nbases * sizeof(struct berlin_pinctrl_reg_base), + GFP_KERNEL); + if (!bases) + return -ENOMEM; + + of_property_for_each_string(node, "reg-names", prop, reg_name) { + struct resource *r = + platform_get_resource_byname(pdev, IORESOURCE_MEM, + reg_name); + bases[i].base = devm_ioremap_resource(&pdev->dev, r); + if (IS_ERR(bases[i].base)) + return PTR_ERR(bases[i].base); + + bases[i].name = reg_name; + i++; + } + + pctrl->desc = (struct berlin_pinctrl_desc *)device->data; + + ret = berlin_pinctrl_build_state(pdev, bases, nbases); + kfree(bases); + if (ret) { + dev_err(dev, "cannot build driver state: %d\n", ret); + return ret; + } + + pctrl->dev = &pdev->dev; + + pctrl->pctrl_dev = pinctrl_register(&berlin_pctrl_desc, dev, pctrl); + if (!pctrl->pctrl_dev) { + dev_err(dev, "failed to register pinctrl driver\n"); + return -EINVAL; + } + + return 0; +} + +static struct platform_driver berlin_pinctrl_driver = { + .probe = berlin_pinctrl_probe, + .driver = { + .name = "berlin-pinctrl", + .owner = THIS_MODULE, + .of_match_table = berlin_pinctrl_match, + }, +}; +module_platform_driver(berlin_pinctrl_driver); + +MODULE_AUTHOR("Antoine Ténart <antoine.tenart@free-electrons.com>"); +MODULE_DESCRIPTION("Marvell Berlin pinctrl driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/pinctrl/pinctrl-berlin.h b/drivers/pinctrl/pinctrl-berlin.h new file mode 100644 index 000000000000..db3e8a379e84 --- /dev/null +++ b/drivers/pinctrl/pinctrl-berlin.h @@ -0,0 +1,72 @@ +/* + * Marvell Berlin SoC pinctrl driver. + * + * Copyright (C) 2014 Marvell Technology Group Ltd. + * + * Antoine Ténart <antoine.tenart@free-electrons.com> + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#ifndef __PINCTRL_BERLIN_H +#define __PINCTRL_BERLIN_H + +struct berlin_desc_group { + const char *name; + const char *base_name; + u32 offset; + u32 bit_width; + u32 lsb; +}; + +struct berlin_pinctrl_base_reg { + void __iomem *base; + const char *name; +}; + +struct berlin_pinctrl_desc { + const struct berlin_desc_group *groups; + unsigned ngroups; +}; + +struct berlin_pinctrl_group { + const char *name; + void __iomem *reg; + u32 mask; + u32 lsb; +}; + +struct berlin_pinctrl_function { + const char *name; + const char *group; + u32 muxval; +}; + +struct berlin_pinctrl { + spinlock_t lock; + struct device *dev; + struct berlin_pinctrl_desc *desc; + struct berlin_pinctrl_group *groups; + unsigned ngroups; + struct berlin_pinctrl_function *functions; + unsigned nfunctions; + struct pinctrl_dev *pctrl_dev; +}; + +struct berlin_pinctrl_reg_base { + const char *name; + void __iomem *base; +}; + +#define BERLIN_PINCTRL_GROUP(_name, _base_name, _offset, _width, _lsb) \ + { \ + .name = _name, \ + .base_name = _base_name, \ + .offset = _offset, \ + .bit_width = _width, \ + .lsb = _lsb, \ + } + +#endif /* __PINCTRL_BERLIN_H */ -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 2/5] pinctrl: berlin: add a pinctrl driver for Marvell Berlin SoCs 2014-04-10 13:07 ` [PATCH RESEND 2/5] pinctrl: berlin: add a pinctrl driver for Marvell Berlin SoCs Antoine Ténart @ 2014-04-11 6:44 ` Jisheng Zhang 2014-04-11 8:18 ` Antoine Ténart 2014-04-11 9:03 ` Sebastian Hesselbarth 1 sibling, 1 reply; 30+ messages in thread From: Jisheng Zhang @ 2014-04-11 6:44 UTC (permalink / raw) To: Antoine Ténart, sebastian.hesselbarth, linus.walleij Cc: alexandre.belloni, Jimmy Xu, linux-arm-kernel, devicetree, linux-kernel Hi Antoine, On Thu, 10 Apr 2014 06:07:51 -0700 Antoine Ténart <antoine.tenart@free-electrons.com> wrote: ... > +static int berlin_pinmux_enable(struct pinctrl_dev *pctrl_dev, > + unsigned function, > + unsigned group) > +{ > + struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); > + struct berlin_pinctrl_group *group_desc = pctrl->groups + group; > + struct berlin_pinctrl_function *function_desc = > + pctrl->functions + function; > + unsigned long flags; > + u32 regval; > + > + spin_lock_irqsave(&pctrl->lock, flags); > + > + regval = readl(group_desc->reg); > + regval &= group_desc->mask; > + regval |= function_desc->muxval << group_desc->lsb; > + writel(regval, group_desc->reg); Could we use relaxed version instead? Thanks, Jisheng ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 2/5] pinctrl: berlin: add a pinctrl driver for Marvell Berlin SoCs 2014-04-11 6:44 ` Jisheng Zhang @ 2014-04-11 8:18 ` Antoine Ténart 2014-04-11 8:27 ` Jisheng Zhang 0 siblings, 1 reply; 30+ messages in thread From: Antoine Ténart @ 2014-04-11 8:18 UTC (permalink / raw) To: Jisheng Zhang Cc: sebastian.hesselbarth, linus.walleij, alexandre.belloni, Jimmy Xu, linux-arm-kernel, devicetree, linux-kernel Hi Jisheng, On Fri, Apr 11, 2014 at 02:44:31PM +0800, Jisheng Zhang wrote: > On Thu, 10 Apr 2014 06:07:51 -0700 > Antoine Ténart <antoine.tenart@free-electrons.com> wrote: > > ... > > +static int berlin_pinmux_enable(struct pinctrl_dev *pctrl_dev, > > + unsigned function, > > + unsigned group) > > +{ > > + struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); > > + struct berlin_pinctrl_group *group_desc = pctrl->groups + group; > > + struct berlin_pinctrl_function *function_desc = > > + pctrl->functions + function; > > + unsigned long flags; > > + u32 regval; > > + > > + spin_lock_irqsave(&pctrl->lock, flags); > > + > > + regval = readl(group_desc->reg); > > + regval &= group_desc->mask; > > + regval |= function_desc->muxval << group_desc->lsb; > > + writel(regval, group_desc->reg); > > Could we use relaxed version instead? We could, but this is not a performance issue here at all, so I guess we can keep writel(). Thanks for the feedback ! Antoine -- Antoine Ténart, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 2/5] pinctrl: berlin: add a pinctrl driver for Marvell Berlin SoCs 2014-04-11 8:18 ` Antoine Ténart @ 2014-04-11 8:27 ` Jisheng Zhang 2014-04-11 9:21 ` Antoine Ténart 0 siblings, 1 reply; 30+ messages in thread From: Jisheng Zhang @ 2014-04-11 8:27 UTC (permalink / raw) To: Antoine Ténart Cc: sebastian.hesselbarth, linus.walleij, alexandre.belloni, Jimmy Xu, linux-arm-kernel, devicetree, linux-kernel Hi Antoine, On Fri, 11 Apr 2014 01:18:39 -0700 Antoine Ténart <antoine.tenart@free-electrons.com> wrote: > Hi Jisheng, > > On Fri, Apr 11, 2014 at 02:44:31PM +0800, Jisheng Zhang wrote: > > On Thu, 10 Apr 2014 06:07:51 -0700 > > Antoine Ténart <antoine.tenart@free-electrons.com> wrote: > > > > ... > > > +static int berlin_pinmux_enable(struct pinctrl_dev *pctrl_dev, > > > + unsigned function, > > > + unsigned group) > > > +{ > > > + struct berlin_pinctrl *pctrl = > > > pinctrl_dev_get_drvdata(pctrl_dev); > > > + struct berlin_pinctrl_group *group_desc = pctrl->groups + group; > > > + struct berlin_pinctrl_function *function_desc = > > > + pctrl->functions + function; > > > + unsigned long flags; > > > + u32 regval; > > > + > > > + spin_lock_irqsave(&pctrl->lock, flags); > > > + > > > + regval = readl(group_desc->reg); > > > + regval &= group_desc->mask; > > > + regval |= function_desc->muxval << group_desc->lsb; > > > + writel(regval, group_desc->reg); > > > > Could we use relaxed version instead? > > We could, but this is not a performance issue here at all, so I guess we can > keep writel(). Yes it's not a performance issue here but an issue for the system which is doing PL310 L2 cache maintenance. If pinmux operation hold the l2x0_lock due to writel() the important video/audio process which is cleaning PL310 cache must wait, thus cause jitter. So I'd like relaxed version if we can. Then I don't need to add this patch to mainline kernel when we upgrade internal tree. Thanks, Jisheng ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 2/5] pinctrl: berlin: add a pinctrl driver for Marvell Berlin SoCs 2014-04-11 8:27 ` Jisheng Zhang @ 2014-04-11 9:21 ` Antoine Ténart 0 siblings, 0 replies; 30+ messages in thread From: Antoine Ténart @ 2014-04-11 9:21 UTC (permalink / raw) To: Jisheng Zhang Cc: sebastian.hesselbarth, linus.walleij, alexandre.belloni, Jimmy Xu, linux-arm-kernel, devicetree, linux-kernel Jisheng, On Fri, Apr 11, 2014 at 04:27:16PM +0800, Jisheng Zhang wrote: > Hi Antoine, > > On Fri, 11 Apr 2014 01:18:39 -0700 > Antoine Ténart <antoine.tenart@free-electrons.com> wrote: > > > Hi Jisheng, > > > > On Fri, Apr 11, 2014 at 02:44:31PM +0800, Jisheng Zhang wrote: > > > On Thu, 10 Apr 2014 06:07:51 -0700 > > > Antoine Ténart <antoine.tenart@free-electrons.com> wrote: > > > > > > ... > > > > +static int berlin_pinmux_enable(struct pinctrl_dev *pctrl_dev, > > > > + unsigned function, > > > > + unsigned group) > > > > +{ > > > > + struct berlin_pinctrl *pctrl = > > > > pinctrl_dev_get_drvdata(pctrl_dev); > > > > + struct berlin_pinctrl_group *group_desc = pctrl->groups + group; > > > > + struct berlin_pinctrl_function *function_desc = > > > > + pctrl->functions + function; > > > > + unsigned long flags; > > > > + u32 regval; > > > > + > > > > + spin_lock_irqsave(&pctrl->lock, flags); > > > > + > > > > + regval = readl(group_desc->reg); > > > > + regval &= group_desc->mask; > > > > + regval |= function_desc->muxval << group_desc->lsb; > > > > + writel(regval, group_desc->reg); > > > > > > Could we use relaxed version instead? > > > > We could, but this is not a performance issue here at all, so I guess we can > > keep writel(). > > Yes it's not a performance issue here but an issue for the system which is doing > PL310 L2 cache maintenance. If pinmux operation hold the l2x0_lock due to writel() > the important video/audio process which is cleaning PL310 cache must wait, thus > cause jitter. So I'd like relaxed version if we can. Then I don't need to add this > patch to mainline kernel when we upgrade internal tree. I'm not sure I got that. As I understand it, you will need to play video/audio *while* configuring the pinmux. But the pinmuxing configuration is done at boot time, and I don't think a video/audio is being played then. So I'm not certain a jitter will appear. What do you think ? Antoine -- Antoine Ténart, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 2/5] pinctrl: berlin: add a pinctrl driver for Marvell Berlin SoCs 2014-04-10 13:07 ` [PATCH RESEND 2/5] pinctrl: berlin: add a pinctrl driver for Marvell Berlin SoCs Antoine Ténart 2014-04-11 6:44 ` Jisheng Zhang @ 2014-04-11 9:03 ` Sebastian Hesselbarth 2014-04-11 12:37 ` Antoine Ténart 1 sibling, 1 reply; 30+ messages in thread From: Sebastian Hesselbarth @ 2014-04-11 9:03 UTC (permalink / raw) To: Antoine Ténart, linus.walleij Cc: alexandre.belloni, zmxu, jszhang, linux-arm-kernel, devicetree, linux-kernel On 04/10/2014 03:07 PM, Antoine Ténart wrote: > The Marvell Berlin boards have a group based pinmuxing mechanism. This > driver adds the support for the BG2CD, BG2 and BG2Q. We actually do not > need any information about the pins here and only have the definition > of the groups. Antoine, as always, thanks for providing this :) > Let's take the example of the uart0 pinmuxing on the BG2Q. Balls BK4 and > BH6 are muxed to respectively UART0 RX and TX if the group GSM12 is set > to mode 0: > > Group Modes Offset Base Offset LSB Bit Width > GSM12 3 sm_base 0x40 0x10 0x2 > > Ball Group Mode 0 Mode 1 Mode 2 > BK4 GSM12 UART0_RX IrDA0_RX GPIO9 > BH6 GSM12 UART0_TX IrDA0_TX GPIO10 Actually, I consider above mode table a very vital information that is missing from the driver below. Especially, I expect bg2 and bg2cd different in some modes and there is no way to look it up. It doesn't matter if we know all mode names now, but we should have a placeholder at least. Also, I know gpio<>pingroup relationship is very broken by design on berlin, but how are you planing to exploit that information? We will have some straight numbered gpios and need to determine what pingroup has to be switched. > So in order to configure BK4 -> UART0_TX and BH6 -> UART0_RX, we need > to set (sm_base + 0x40 + 0x10) &= ff3fffff. > > Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> > --- > drivers/pinctrl/Kconfig | 4 + > drivers/pinctrl/Makefile | 1 + > drivers/pinctrl/pinctrl-berlin.c | 498 +++++++++++++++++++++++++++++++++++++++ > drivers/pinctrl/pinctrl-berlin.h | 72 ++++++ > 4 files changed, 575 insertions(+) > create mode 100644 drivers/pinctrl/pinctrl-berlin.c > create mode 100644 drivers/pinctrl/pinctrl-berlin.h > > diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig > index e49324032611..2d9339a7bd05 100644 > --- a/drivers/pinctrl/Kconfig > +++ b/drivers/pinctrl/Kconfig > @@ -104,6 +104,10 @@ config PINCTRL_BCM2835 > select PINMUX > select PINCONF > > +config PINCTRL_BERLIN > + bool > + select PINMUX select PINCTRL too and drop it from the individual SoC configs later on. > config PINCTRL_CAPRI > bool "Broadcom Capri pinctrl driver" > depends on OF > diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile > index 4b835880cf80..fd5a01d4475f 100644 > --- a/drivers/pinctrl/Makefile > +++ b/drivers/pinctrl/Makefile > @@ -21,6 +21,7 @@ obj-$(CONFIG_PINCTRL_BF60x) += pinctrl-adi2-bf60x.o > obj-$(CONFIG_PINCTRL_AT91) += pinctrl-at91.o > obj-$(CONFIG_PINCTRL_BCM2835) += pinctrl-bcm2835.o > obj-$(CONFIG_PINCTRL_BAYTRAIL) += pinctrl-baytrail.o > +obj-$(CONFIG_PINCTRL_BERLIN) += pinctrl-berlin.o Please split the driver into common and soc-specific parts, and if you do please put it into a subfolder berlin, too. > obj-$(CONFIG_PINCTRL_CAPRI) += pinctrl-capri.o > obj-$(CONFIG_PINCTRL_IMX) += pinctrl-imx.o > obj-$(CONFIG_PINCTRL_IMX1_CORE) += pinctrl-imx1-core.o > diff --git a/drivers/pinctrl/pinctrl-berlin.c b/drivers/pinctrl/pinctrl-berlin.c > new file mode 100644 > index 000000000000..a377d6fbb127 > --- /dev/null > +++ b/drivers/pinctrl/pinctrl-berlin.c > @@ -0,0 +1,498 @@ > +/* > + * Marvell Berlin SoC pinctrl driver. > + * > + * Copyright (C) 2014 Marvell Technology Group Ltd. > + * > + * Antoine Ténart <antoine.tenart@free-electrons.com> > + * > + * This file is licensed under the terms of the GNU General Public > + * License version 2. This program is licensed "as is" without any > + * warranty of any kind, whether express or implied. > + */ > + > +#include <linux/io.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/of_device.h> > +#include <linux/pinctrl/pinctrl.h> > +#include <linux/pinctrl/pinmux.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > + > +#include "core.h" > +#include "pinctrl-utils.h" > +#include "pinctrl-berlin.h" > + > +#define NFCTS(bit_width) (1 << ((bit_width) + 1)) Number-of-FunCTionS ? You use that define twice, but maybe it is more clear to write it out where you use it. Otherwise it is kind of hard to get the idea. > +static const struct berlin_desc_group berlin2cd_pinctrl_groups[] = { First SoC in bg2 and bg2cd is possibly bg2. Use that one for the names please. > + /* GSM */ > + BERLIN_PINCTRL_GROUP("GSM0", "apb_base", 0x40, 0x2, 0x00), I'd love to see the mode/function tables in here. > + BERLIN_PINCTRL_GROUP("GSM1", "apb_base", 0x40, 0x2, 0x02), > + BERLIN_PINCTRL_GROUP("GSM2", "apb_base", 0x40, 0x2, 0x04), > + BERLIN_PINCTRL_GROUP("GSM3", "apb_base", 0x40, 0x2, 0x06), > + BERLIN_PINCTRL_GROUP("GSM4", "apb_base", 0x40, 0x2, 0x08), > + BERLIN_PINCTRL_GROUP("GSM5", "apb_base", 0x40, 0x2, 0x0a), > + BERLIN_PINCTRL_GROUP("GSM6", "apb_base", 0x40, 0x2, 0x0c), > + BERLIN_PINCTRL_GROUP("GSM7", "apb_base", 0x40, 0x1, 0x0e), > + BERLIN_PINCTRL_GROUP("GSM8", "apb_base", 0x40, 0x1, 0x0f), > + BERLIN_PINCTRL_GROUP("GSM9", "apb_base", 0x40, 0x1, 0x10), > + BERLIN_PINCTRL_GROUP("GSM10", "apb_base", 0x40, 0x1, 0x11), > + BERLIN_PINCTRL_GROUP("GSM11", "apb_base", 0x40, 0x1, 0x12), [...] > +static int berlin_pinctrl_get_group_count(struct pinctrl_dev *pctrl_dev) > +{ > + struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); > + > + return pctrl->ngroups; > +} > + > +static const char *berlin_pinctrl_get_group_name(struct pinctrl_dev *pctrl_dev, > + unsigned group) > +{ > + struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); > + > + return pctrl->groups[group].name; > +} > + > +static int berlin_pinctrl_dt_node_to_map(struct pinctrl_dev *pctrl_dev, > + struct device_node *node, > + struct pinctrl_map **map, > + unsigned *num_maps) > +{ > + struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); > + const char *group_name; > + char *function_name; > + unsigned reserved_maps = 0; > + u32 function; > + int ret; > + > + *map = NULL; > + *num_maps = 0; > + > + ret = pinctrl_utils_reserve_map(pctrl_dev, map, &reserved_maps, > + num_maps, 1); > + if (ret) { > + dev_err(pctrl->dev, "can't reserve map: %d\n", ret); > + return ret; > + } > + > + ret = of_property_read_u32(node, "berlin,function", &function); Properties should be _vendor_ prefixed, i.e. "marvell,function". > + if (ret) { > + dev_err(pctrl->dev, > + "missing 'berlin,function' property in node %s\n", > + node->name); > + return -EINVAL; > + } > + > + ret = of_property_read_string(node, "berlin,group", &group_name); ditto. > + if (ret) { > + dev_err(pctrl->dev, > + "missing 'berlin,group' property in node %s\n", > + node->name); > + return -EINVAL; > + } > + > + function_name = kzalloc(strlen(group_name) + 7, GFP_KERNEL); > + if (!function_name) > + return -ENOMEM; > + snprintf(function_name, strlen(group_name) + 7, "%s_mode%d", group_name, > + function); With proper mode tables above, this can refer to a const char* and you can get rid of allocation here. Also, below you already allocated function_names, how is this one different from the below? > + > + ret = pinctrl_utils_add_map_mux(pctrl_dev, map, &reserved_maps, > + num_maps, group_name, function_name); > + if (ret) { > + dev_err(pctrl->dev, "can't add map: %d\n", ret); > + return ret; > + } > + > + return 0; > +} > + > +static void berlin_pinctrl_dt_free_map(struct pinctrl_dev *pctrl_dev, > + struct pinctrl_map *map, > + unsigned nmaps) > +{ > + int i; > + for (i = 0; i < nmaps; i++) { > + if (map[i].type == PIN_MAP_TYPE_MUX_GROUP) { > + kfree(map[i].data.mux.group); > + kfree(map[i].data.mux.function); > + } > + } > + > + kfree(map); > +} > + > +static const struct pinctrl_ops berlin_pinctrl_ops = { > + .get_groups_count = &berlin_pinctrl_get_group_count, > + .get_group_name = &berlin_pinctrl_get_group_name, > + .dt_node_to_map = &berlin_pinctrl_dt_node_to_map, > + .dt_free_map = &berlin_pinctrl_dt_free_map, > +}; > + > +static int berlin_pinmux_get_functions_count(struct pinctrl_dev *pctrl_dev) > +{ > + struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); > + > + return pctrl->nfunctions; > +} > + > +static const char *berlin_pinmux_get_function_name(struct pinctrl_dev *pctrl_dev, > + unsigned function) > +{ > + struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); > + > + return pctrl->functions[function].name; > +} > + > +static int berlin_pinmux_get_function_groups(struct pinctrl_dev *pctrl_dev, > + unsigned function, > + const char * const **groups, > + unsigned * const num_groups) > +{ > + struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); > + > + *groups = &pctrl->functions[function].group; > + *num_groups = 1; > + > + return 0; > +} > + > +static int berlin_pinmux_enable(struct pinctrl_dev *pctrl_dev, > + unsigned function, > + unsigned group) > +{ > + struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); > + struct berlin_pinctrl_group *group_desc = pctrl->groups + group; > + struct berlin_pinctrl_function *function_desc = > + pctrl->functions + function; > + unsigned long flags; > + u32 regval; > + > + spin_lock_irqsave(&pctrl->lock, flags); > + > + regval = readl(group_desc->reg); > + regval &= group_desc->mask; > + regval |= function_desc->muxval << group_desc->lsb; > + writel(regval, group_desc->reg); > + > + spin_unlock_irqrestore(&pctrl->lock, flags); > + > + return 0; > +} > + > +static const struct pinmux_ops berlin_pinmux_ops = { > + .get_functions_count = &berlin_pinmux_get_functions_count, > + .get_function_name = &berlin_pinmux_get_function_name, > + .get_function_groups = &berlin_pinmux_get_function_groups, > + .enable = &berlin_pinmux_enable, > +}; > + > +static int berlin_pinctrl_build_state(struct platform_device *pdev, > + struct berlin_pinctrl_reg_base *bases, > + unsigned nbases) > +{ > + struct berlin_pinctrl *pctrl = platform_get_drvdata(pdev); > + int i, j, cur_fct = 0; > + > + pctrl->ngroups = pctrl->desc->ngroups; > + pctrl->nfunctions = 0; > + > + for (i = 0; i < pctrl->ngroups; i++) { > + struct berlin_desc_group const *desc = pctrl->desc->groups + i; > + pctrl->nfunctions += NFCTS(desc->bit_width); > + } As you need desc later on, make it global to this function. Then you can also walk through it with desc++ desc = pctrl->desc->groups; for (i = 0; i < pctl->ngroups; i++, desc++) pctrl->nfunctions = ... Having said that, the above assumes that each function is unique but IIRC the idea of the function table was to group pins/groups with the same function, e.g. function "gpio", groups 1,7,25,... > + pctrl->groups = devm_kzalloc(&pdev->dev, > + pctrl->ngroups * sizeof(struct berlin_pinctrl_group), > + GFP_KERNEL); > + if (!pctrl->groups) > + return -ENOMEM; > + > + pctrl->functions = devm_kzalloc(&pdev->dev, > + pctrl->ngroups * pctrl->nfunctions * > + sizeof(struct berlin_pinctrl_function), > + GFP_KERNEL); > + if (!pctrl->functions) > + return -ENOMEM; > + > + for (i = 0; i < pctrl->ngroups; i++) { > + struct berlin_desc_group const *desc = pctrl->desc->groups + i; > + struct berlin_pinctrl_group *group = pctrl->groups + i; > + > + group->name = desc->name; > + group->mask = GENMASK(desc->lsb + desc->bit_width - 1, > + desc->lsb); > + group->lsb = desc->lsb; > + > + for (j = 0; j < nbases; j++) { > + if (!strcmp(desc->base_name, bases[j].name)) { > + group->reg = bases[j].base + desc->offset; > + break; > + } > + } > + > + if (j == nbases) { > + dev_err(pctrl->dev, "cannot find base address for %s\n", > + desc->base_name); > + return -EINVAL; > + } TBH, I am not so happy with this name<>reg relationship. What about having two separate pin controllers instead? > + for (j = 0; j < NFCTS(desc->bit_width); j++) { > + struct berlin_pinctrl_function *function = > + pctrl->functions + cur_fct; > + char *function_name = devm_kzalloc(&pdev->dev, > + (strlen(desc->name) + 7) * sizeof(char), > + GFP_KERNEL); > + if (!function_name) > + return -ENOMEM; Allocating function_names again? > + > + snprintf(function_name, strlen(desc->name) + 7, > + "%s_mode%d", desc->name, j); > + > + function->name = function_name; > + function->group = desc->name; > + function->muxval = j; > + > + cur_fct++; > + } > + } > + > + return 0; > +} > + > +static struct pinctrl_desc berlin_pctrl_desc = { > + .name = "berlin-pinctrl", > + .pctlops = &berlin_pinctrl_ops, > + .pmxops = &berlin_pinmux_ops, > + .owner = THIS_MODULE, > +}; > + > +static const struct berlin_pinctrl_desc berlin2cd_pinctrl_data = { > + .groups = berlin2cd_pinctrl_groups, > + .ngroups = ARRAY_SIZE(berlin2cd_pinctrl_groups), > +}; > + > +static const struct berlin_pinctrl_desc berlin2q_pinctrl_data = { > + .groups = berlin2q_pinctrl_groups, > + .ngroups = ARRAY_SIZE(berlin2q_pinctrl_groups), > +}; > + > +static struct of_device_id berlin_pinctrl_match[] = { > + { > + .compatible = "marvell,berlin2cd-pinctrl", > + .data = &berlin2cd_pinctrl_data > + }, > + { > + .compatible = "marvell,berlin2-pinctrl", > + .data = &berlin2cd_pinctrl_data > + }, > + { > + .compatible = "marvell,berlin2q-pinctrl", > + .data = &berlin2q_pinctrl_data > + }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, berlin_pinctrl_match); Please sort alphabetically. > + > +static int berlin_pinctrl_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *node; > + struct property *prop; > + struct berlin_pinctrl *pctrl; > + struct berlin_pinctrl_reg_base *bases; > + const struct of_device_id *device; > + const char *reg_name; > + int ret, i = 0; > + unsigned nbases; > + > + if (!dev->of_node) { > + dev_err(dev, "device node not found\n"); There is no way you will see this driver being called without .of_node set. I'll really have an eye on anybody who will try to introduce non-DT platform_device registration ;) > + return -ENODEV; > + } > + node = dev->of_node; > + > + pctrl = devm_kzalloc(dev, sizeof(struct berlin_pinctrl), GFP_KERNEL); > + if (!pctrl) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, pctrl); > + > + spin_lock_init(&pctrl->lock); > + > + device = of_match_device(berlin_pinctrl_match, dev); With the comment above, you can move the assignment right to the declaration. Also, it will always match. > + if (!device) { > + dev_err(dev, "pinctrl didn't match\n"); > + return -ENODEV; > + } > + > + nbases = of_property_count_strings(node, "reg-names"); > + if (nbases < 0) { > + dev_err(dev, "missing 'reg-names' property in node %s\n", > + node->name); > + return -EINVAL; > + } See my comment about names<>reg relation. > + bases = devm_kzalloc(dev, > + nbases * sizeof(struct berlin_pinctrl_reg_base), > + GFP_KERNEL); > + if (!bases) > + return -ENOMEM; > + > + of_property_for_each_string(node, "reg-names", prop, reg_name) { > + struct resource *r = > + platform_get_resource_byname(pdev, IORESOURCE_MEM, > + reg_name); > + bases[i].base = devm_ioremap_resource(&pdev->dev, r); > + if (IS_ERR(bases[i].base)) > + return PTR_ERR(bases[i].base); > + > + bases[i].name = reg_name; > + i++; > + } > + > + pctrl->desc = (struct berlin_pinctrl_desc *)device->data; > + > + ret = berlin_pinctrl_build_state(pdev, bases, nbases); > + kfree(bases); > + if (ret) { > + dev_err(dev, "cannot build driver state: %d\n", ret); > + return ret; > + } > + > + pctrl->dev = &pdev->dev; > + > + pctrl->pctrl_dev = pinctrl_register(&berlin_pctrl_desc, dev, pctrl); > + if (!pctrl->pctrl_dev) { > + dev_err(dev, "failed to register pinctrl driver\n"); > + return -EINVAL; > + } > + > + return 0; > +} > + > +static struct platform_driver berlin_pinctrl_driver = { > + .probe = berlin_pinctrl_probe, > + .driver = { > + .name = "berlin-pinctrl", > + .owner = THIS_MODULE, > + .of_match_table = berlin_pinctrl_match, > + }, > +}; > +module_platform_driver(berlin_pinctrl_driver); > + > +MODULE_AUTHOR("Antoine Ténart <antoine.tenart@free-electrons.com>"); > +MODULE_DESCRIPTION("Marvell Berlin pinctrl driver"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/pinctrl/pinctrl-berlin.h b/drivers/pinctrl/pinctrl-berlin.h > new file mode 100644 > index 000000000000..db3e8a379e84 > --- /dev/null > +++ b/drivers/pinctrl/pinctrl-berlin.h > @@ -0,0 +1,72 @@ > +/* > + * Marvell Berlin SoC pinctrl driver. > + * > + * Copyright (C) 2014 Marvell Technology Group Ltd. > + * > + * Antoine Ténart <antoine.tenart@free-electrons.com> > + * > + * This file is licensed under the terms of the GNU General Public > + * License version 2. This program is licensed "as is" without any > + * warranty of any kind, whether express or implied. > + */ > + > +#ifndef __PINCTRL_BERLIN_H > +#define __PINCTRL_BERLIN_H > + > +struct berlin_desc_group { > + const char *name; > + const char *base_name; > + u32 offset; > + u32 bit_width; > + u32 lsb; You expect any offsets, bit_widths or lsbs at 2^32-1 ? ;) Sebastian > +}; > + > +struct berlin_pinctrl_base_reg { > + void __iomem *base; > + const char *name; > +}; > + > +struct berlin_pinctrl_desc { > + const struct berlin_desc_group *groups; > + unsigned ngroups; > +}; > + > +struct berlin_pinctrl_group { > + const char *name; > + void __iomem *reg; > + u32 mask; > + u32 lsb; > +}; > + > +struct berlin_pinctrl_function { > + const char *name; > + const char *group; > + u32 muxval; > +}; > + > +struct berlin_pinctrl { > + spinlock_t lock; > + struct device *dev; > + struct berlin_pinctrl_desc *desc; > + struct berlin_pinctrl_group *groups; > + unsigned ngroups; > + struct berlin_pinctrl_function *functions; > + unsigned nfunctions; > + struct pinctrl_dev *pctrl_dev; > +}; > + > +struct berlin_pinctrl_reg_base { > + const char *name; > + void __iomem *base; > +}; > + > +#define BERLIN_PINCTRL_GROUP(_name, _base_name, _offset, _width, _lsb) \ > + { \ > + .name = _name, \ > + .base_name = _base_name, \ > + .offset = _offset, \ > + .bit_width = _width, \ > + .lsb = _lsb, \ > + } > + > +#endif /* __PINCTRL_BERLIN_H */ > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 2/5] pinctrl: berlin: add a pinctrl driver for Marvell Berlin SoCs 2014-04-11 9:03 ` Sebastian Hesselbarth @ 2014-04-11 12:37 ` Antoine Ténart 2014-04-11 13:35 ` Sebastian Hesselbarth 0 siblings, 1 reply; 30+ messages in thread From: Antoine Ténart @ 2014-04-11 12:37 UTC (permalink / raw) To: Sebastian Hesselbarth Cc: linus.walleij, alexandre.belloni, zmxu, jszhang, linux-arm-kernel, devicetree, linux-kernel Sebastian, On Fri, Apr 11, 2014 at 11:03:48AM +0200, Sebastian Hesselbarth wrote: > On 04/10/2014 03:07 PM, Antoine Ténart wrote: > >The Marvell Berlin boards have a group based pinmuxing mechanism. This > >driver adds the support for the BG2CD, BG2 and BG2Q. We actually do not > >need any information about the pins here and only have the definition > >of the groups. > > Antoine, > > as always, thanks for providing this :) > > >Let's take the example of the uart0 pinmuxing on the BG2Q. Balls BK4 and > >BH6 are muxed to respectively UART0 RX and TX if the group GSM12 is set > >to mode 0: > > > >Group Modes Offset Base Offset LSB Bit Width > >GSM12 3 sm_base 0x40 0x10 0x2 > > > >Ball Group Mode 0 Mode 1 Mode 2 > >BK4 GSM12 UART0_RX IrDA0_RX GPIO9 > >BH6 GSM12 UART0_TX IrDA0_TX GPIO10 > > Actually, I consider above mode table a very vital information > that is missing from the driver below. Especially, I expect bg2 and > bg2cd different in some modes and there is no way to look it up. They are. > It doesn't matter if we know all mode names now, but we should > have a placeholder at least. > > Also, I know gpio<>pingroup relationship is very broken by design > on berlin, but how are you planing to exploit that information? > We will have some straight numbered gpios and need to determine > what pingroup has to be switched. I need to think about that. > >So in order to configure BK4 -> UART0_TX and BH6 -> UART0_RX, we need > >to set (sm_base + 0x40 + 0x10) &= ff3fffff. > > > >Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> > >--- > > drivers/pinctrl/Kconfig | 4 + > > drivers/pinctrl/Makefile | 1 + > > drivers/pinctrl/pinctrl-berlin.c | 498 +++++++++++++++++++++++++++++++++++++++ > > drivers/pinctrl/pinctrl-berlin.h | 72 ++++++ > > 4 files changed, 575 insertions(+) > > create mode 100644 drivers/pinctrl/pinctrl-berlin.c > > create mode 100644 drivers/pinctrl/pinctrl-berlin.h > > > >diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig > >index e49324032611..2d9339a7bd05 100644 > >--- a/drivers/pinctrl/Kconfig > >+++ b/drivers/pinctrl/Kconfig > >@@ -104,6 +104,10 @@ config PINCTRL_BCM2835 > > select PINMUX > > select PINCONF > > > >+config PINCTRL_BERLIN > >+ bool > >+ select PINMUX > > select PINCTRL too and drop it from the individual SoC configs > later on. Will do. > > config PINCTRL_CAPRI > > bool "Broadcom Capri pinctrl driver" > > depends on OF > >diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile > >index 4b835880cf80..fd5a01d4475f 100644 > >--- a/drivers/pinctrl/Makefile > >+++ b/drivers/pinctrl/Makefile > >@@ -21,6 +21,7 @@ obj-$(CONFIG_PINCTRL_BF60x) += pinctrl-adi2-bf60x.o > > obj-$(CONFIG_PINCTRL_AT91) += pinctrl-at91.o > > obj-$(CONFIG_PINCTRL_BCM2835) += pinctrl-bcm2835.o > > obj-$(CONFIG_PINCTRL_BAYTRAIL) += pinctrl-baytrail.o > >+obj-$(CONFIG_PINCTRL_BERLIN) += pinctrl-berlin.o > > Please split the driver into common and soc-specific parts, and if > you do please put it into a subfolder berlin, too. The only SoC specific part is the group structure. I'll need to have functions in each SoC specific part calling ... only the common ones. Do you have a better solution in mind ? > > obj-$(CONFIG_PINCTRL_CAPRI) += pinctrl-capri.o > > obj-$(CONFIG_PINCTRL_IMX) += pinctrl-imx.o > > obj-$(CONFIG_PINCTRL_IMX1_CORE) += pinctrl-imx1-core.o > >diff --git a/drivers/pinctrl/pinctrl-berlin.c b/drivers/pinctrl/pinctrl-berlin.c > >new file mode 100644 > >index 000000000000..a377d6fbb127 > >--- /dev/null > >+++ b/drivers/pinctrl/pinctrl-berlin.c > >@@ -0,0 +1,498 @@ > >+/* > >+ * Marvell Berlin SoC pinctrl driver. > >+ * > >+ * Copyright (C) 2014 Marvell Technology Group Ltd. > >+ * > >+ * Antoine Ténart <antoine.tenart@free-electrons.com> > >+ * > >+ * This file is licensed under the terms of the GNU General Public > >+ * License version 2. This program is licensed "as is" without any > >+ * warranty of any kind, whether express or implied. > >+ */ > >+ > >+#include <linux/io.h> > >+#include <linux/module.h> > >+#include <linux/of.h> > >+#include <linux/of_address.h> > >+#include <linux/of_device.h> > >+#include <linux/pinctrl/pinctrl.h> > >+#include <linux/pinctrl/pinmux.h> > >+#include <linux/platform_device.h> > >+#include <linux/slab.h> > >+ > >+#include "core.h" > >+#include "pinctrl-utils.h" > >+#include "pinctrl-berlin.h" > >+ > >+#define NFCTS(bit_width) (1 << ((bit_width) + 1)) > > Number-of-FunCTionS ? You use that define twice, but maybe it is Yes :) > more clear to write it out where you use it. Otherwise it is > kind of hard to get the idea. OK, with a comment describing what's happening. > >+static const struct berlin_desc_group berlin2cd_pinctrl_groups[] = { > > First SoC in bg2 and bg2cd is possibly bg2. Use that one for the > names please. Will do. > > >+ /* GSM */ > >+ BERLIN_PINCTRL_GROUP("GSM0", "apb_base", 0x40, 0x2, 0x00), > > I'd love to see the mode/function tables in here. > > >+ BERLIN_PINCTRL_GROUP("GSM1", "apb_base", 0x40, 0x2, 0x02), > >+ BERLIN_PINCTRL_GROUP("GSM2", "apb_base", 0x40, 0x2, 0x04), > >+ BERLIN_PINCTRL_GROUP("GSM3", "apb_base", 0x40, 0x2, 0x06), > >+ BERLIN_PINCTRL_GROUP("GSM4", "apb_base", 0x40, 0x2, 0x08), > >+ BERLIN_PINCTRL_GROUP("GSM5", "apb_base", 0x40, 0x2, 0x0a), > >+ BERLIN_PINCTRL_GROUP("GSM6", "apb_base", 0x40, 0x2, 0x0c), > >+ BERLIN_PINCTRL_GROUP("GSM7", "apb_base", 0x40, 0x1, 0x0e), > >+ BERLIN_PINCTRL_GROUP("GSM8", "apb_base", 0x40, 0x1, 0x0f), > >+ BERLIN_PINCTRL_GROUP("GSM9", "apb_base", 0x40, 0x1, 0x10), > >+ BERLIN_PINCTRL_GROUP("GSM10", "apb_base", 0x40, 0x1, 0x11), > >+ BERLIN_PINCTRL_GROUP("GSM11", "apb_base", 0x40, 0x1, 0x12), > [...] > >+static int berlin_pinctrl_get_group_count(struct pinctrl_dev *pctrl_dev) > >+{ > >+ struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); > >+ > >+ return pctrl->ngroups; > >+} > >+ > >+static const char *berlin_pinctrl_get_group_name(struct pinctrl_dev *pctrl_dev, > >+ unsigned group) > >+{ > >+ struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); > >+ > >+ return pctrl->groups[group].name; > >+} > >+ > >+static int berlin_pinctrl_dt_node_to_map(struct pinctrl_dev *pctrl_dev, > >+ struct device_node *node, > >+ struct pinctrl_map **map, > >+ unsigned *num_maps) > >+{ > >+ struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev); > >+ const char *group_name; > >+ char *function_name; > >+ unsigned reserved_maps = 0; > >+ u32 function; > >+ int ret; > >+ > >+ *map = NULL; > >+ *num_maps = 0; > >+ > >+ ret = pinctrl_utils_reserve_map(pctrl_dev, map, &reserved_maps, > >+ num_maps, 1); > >+ if (ret) { > >+ dev_err(pctrl->dev, "can't reserve map: %d\n", ret); > >+ return ret; > >+ } > >+ > >+ ret = of_property_read_u32(node, "berlin,function", &function); > > Properties should be _vendor_ prefixed, i.e. "marvell,function". Right. > >+ if (ret) { > >+ dev_err(pctrl->dev, > >+ "missing 'berlin,function' property in node %s\n", > >+ node->name); > >+ return -EINVAL; > >+ } > >+ > >+ ret = of_property_read_string(node, "berlin,group", &group_name); > > ditto. > > >+ if (ret) { > >+ dev_err(pctrl->dev, > >+ "missing 'berlin,group' property in node %s\n", > >+ node->name); > >+ return -EINVAL; > >+ } > >+ > >+ function_name = kzalloc(strlen(group_name) + 7, GFP_KERNEL); > >+ if (!function_name) > >+ return -ENOMEM; > >+ snprintf(function_name, strlen(group_name) + 7, "%s_mode%d", group_name, > >+ function); > > With proper mode tables above, this can refer to a const char* and you > can get rid of allocation here. Also, below you already allocated > function_names, how is this one different from the below? The one below is used to describe the available functions for a given group while this one describe a function found in the device tree. It is used to check the function we want to use in the device is provided by the driver. The function name used here may not actually exist. We could check the function actually exists here (and find the previously allocated function name), but this check is handled by the pinctrl framework. This would end up in a different behaviour than the expected one, imho. […] > >+static int berlin_pinctrl_build_state(struct platform_device *pdev, > >+ struct berlin_pinctrl_reg_base *bases, > >+ unsigned nbases) > >+{ > >+ struct berlin_pinctrl *pctrl = platform_get_drvdata(pdev); > >+ int i, j, cur_fct = 0; > >+ > >+ pctrl->ngroups = pctrl->desc->ngroups; > >+ pctrl->nfunctions = 0; > >+ > >+ for (i = 0; i < pctrl->ngroups; i++) { > >+ struct berlin_desc_group const *desc = pctrl->desc->groups + i; > >+ pctrl->nfunctions += NFCTS(desc->bit_width); > >+ } > > As you need desc later on, make it global to this function. Then you > can also walk through it with desc++ > > desc = pctrl->desc->groups; > for (i = 0; i < pctl->ngroups; i++, desc++) > pctrl->nfunctions = ... > > Having said that, the above assumes that each function is unique > but IIRC the idea of the function table was to group pins/groups > with the same function, e.g. function "gpio", groups 1,7,25,... Most of the functions you can use on the Berlin they will be unique and would only be used in one group, except for the 'gpio' one. > >+ pctrl->groups = devm_kzalloc(&pdev->dev, > >+ pctrl->ngroups * sizeof(struct berlin_pinctrl_group), > >+ GFP_KERNEL); > >+ if (!pctrl->groups) > >+ return -ENOMEM; > >+ > >+ pctrl->functions = devm_kzalloc(&pdev->dev, > >+ pctrl->ngroups * pctrl->nfunctions * > >+ sizeof(struct berlin_pinctrl_function), > >+ GFP_KERNEL); > >+ if (!pctrl->functions) > >+ return -ENOMEM; > >+ > >+ for (i = 0; i < pctrl->ngroups; i++) { > >+ struct berlin_desc_group const *desc = pctrl->desc->groups + i; > >+ struct berlin_pinctrl_group *group = pctrl->groups + i; > >+ > >+ group->name = desc->name; > >+ group->mask = GENMASK(desc->lsb + desc->bit_width - 1, > >+ desc->lsb); > >+ group->lsb = desc->lsb; > >+ > >+ for (j = 0; j < nbases; j++) { > >+ if (!strcmp(desc->base_name, bases[j].name)) { > >+ group->reg = bases[j].base + desc->offset; > >+ break; > >+ } > >+ } > >+ > >+ if (j == nbases) { > >+ dev_err(pctrl->dev, "cannot find base address for %s\n", > >+ desc->base_name); > >+ return -EINVAL; > >+ } > > TBH, I am not so happy with this name<>reg relationship. What about > having two separate pin controllers instead? I'm OK with having separate pin controllers. > >+ for (j = 0; j < NFCTS(desc->bit_width); j++) { > >+ struct berlin_pinctrl_function *function = > >+ pctrl->functions + cur_fct; > >+ char *function_name = devm_kzalloc(&pdev->dev, > >+ (strlen(desc->name) + 7) * sizeof(char), > >+ GFP_KERNEL); > >+ if (!function_name) > >+ return -ENOMEM; > > Allocating function_names again? > > >+ > >+ snprintf(function_name, strlen(desc->name) + 7, > >+ "%s_mode%d", desc->name, j); > >+ > >+ function->name = function_name; > >+ function->group = desc->name; > >+ function->muxval = j; > >+ > >+ cur_fct++; > >+ } > >+ } > >+ > >+ return 0; > >+} > >+ > >+static struct pinctrl_desc berlin_pctrl_desc = { > >+ .name = "berlin-pinctrl", > >+ .pctlops = &berlin_pinctrl_ops, > >+ .pmxops = &berlin_pinmux_ops, > >+ .owner = THIS_MODULE, > >+}; > >+ > >+static const struct berlin_pinctrl_desc berlin2cd_pinctrl_data = { > >+ .groups = berlin2cd_pinctrl_groups, > >+ .ngroups = ARRAY_SIZE(berlin2cd_pinctrl_groups), > >+}; > >+ > >+static const struct berlin_pinctrl_desc berlin2q_pinctrl_data = { > >+ .groups = berlin2q_pinctrl_groups, > >+ .ngroups = ARRAY_SIZE(berlin2q_pinctrl_groups), > >+}; > >+ > >+static struct of_device_id berlin_pinctrl_match[] = { > >+ { > >+ .compatible = "marvell,berlin2cd-pinctrl", > >+ .data = &berlin2cd_pinctrl_data > >+ }, > >+ { > >+ .compatible = "marvell,berlin2-pinctrl", > >+ .data = &berlin2cd_pinctrl_data > >+ }, > >+ { > >+ .compatible = "marvell,berlin2q-pinctrl", > >+ .data = &berlin2q_pinctrl_data > >+ }, > >+ {} > >+}; > >+MODULE_DEVICE_TABLE(of, berlin_pinctrl_match); > > Please sort alphabetically. Sure. > >+ > >+static int berlin_pinctrl_probe(struct platform_device *pdev) > >+{ > >+ struct device *dev = &pdev->dev; > >+ struct device_node *node; > >+ struct property *prop; > >+ struct berlin_pinctrl *pctrl; > >+ struct berlin_pinctrl_reg_base *bases; > >+ const struct of_device_id *device; > >+ const char *reg_name; > >+ int ret, i = 0; > >+ unsigned nbases; > >+ > >+ if (!dev->of_node) { > >+ dev_err(dev, "device node not found\n"); > > There is no way you will see this driver being called without > .of_node set. I'll really have an eye on anybody who will try > to introduce non-DT platform_device registration ;) > > >+ return -ENODEV; > >+ } > >+ node = dev->of_node; > >+ > >+ pctrl = devm_kzalloc(dev, sizeof(struct berlin_pinctrl), GFP_KERNEL); > >+ if (!pctrl) > >+ return -ENOMEM; > >+ > >+ platform_set_drvdata(pdev, pctrl); > >+ > >+ spin_lock_init(&pctrl->lock); > >+ > >+ device = of_match_device(berlin_pinctrl_match, dev); > > With the comment above, you can move the assignment right to the > declaration. Also, it will always match. > > >+ if (!device) { > >+ dev_err(dev, "pinctrl didn't match\n"); > >+ return -ENODEV; > >+ } > >+ > >+ nbases = of_property_count_strings(node, "reg-names"); > >+ if (nbases < 0) { > >+ dev_err(dev, "missing 'reg-names' property in node %s\n", > >+ node->name); > >+ return -EINVAL; > >+ } > > See my comment about names<>reg relation. > > >+ bases = devm_kzalloc(dev, > >+ nbases * sizeof(struct berlin_pinctrl_reg_base), > >+ GFP_KERNEL); > >+ if (!bases) > >+ return -ENOMEM; > > + > >+ of_property_for_each_string(node, "reg-names", prop, reg_name) { > >+ struct resource *r = > >+ platform_get_resource_byname(pdev, IORESOURCE_MEM, > >+ reg_name); > >+ bases[i].base = devm_ioremap_resource(&pdev->dev, r); > >+ if (IS_ERR(bases[i].base)) > >+ return PTR_ERR(bases[i].base); > >+ > >+ bases[i].name = reg_name; > >+ i++; > >+ } > >+ > >+ pctrl->desc = (struct berlin_pinctrl_desc *)device->data; > >+ > >+ ret = berlin_pinctrl_build_state(pdev, bases, nbases); > >+ kfree(bases); > >+ if (ret) { > >+ dev_err(dev, "cannot build driver state: %d\n", ret); > >+ return ret; > >+ } > >+ > >+ pctrl->dev = &pdev->dev; > >+ > >+ pctrl->pctrl_dev = pinctrl_register(&berlin_pctrl_desc, dev, pctrl); > >+ if (!pctrl->pctrl_dev) { > >+ dev_err(dev, "failed to register pinctrl driver\n"); > >+ return -EINVAL; > >+ } > >+ > >+ return 0; > >+} > >+ > >+static struct platform_driver berlin_pinctrl_driver = { > >+ .probe = berlin_pinctrl_probe, > >+ .driver = { > >+ .name = "berlin-pinctrl", > >+ .owner = THIS_MODULE, > >+ .of_match_table = berlin_pinctrl_match, > >+ }, > >+}; > >+module_platform_driver(berlin_pinctrl_driver); > >+ > >+MODULE_AUTHOR("Antoine Ténart <antoine.tenart@free-electrons.com>"); > >+MODULE_DESCRIPTION("Marvell Berlin pinctrl driver"); > >+MODULE_LICENSE("GPL"); > >diff --git a/drivers/pinctrl/pinctrl-berlin.h b/drivers/pinctrl/pinctrl-berlin.h > >new file mode 100644 > >index 000000000000..db3e8a379e84 > >--- /dev/null > >+++ b/drivers/pinctrl/pinctrl-berlin.h > >@@ -0,0 +1,72 @@ > >+/* > >+ * Marvell Berlin SoC pinctrl driver. > >+ * > >+ * Copyright (C) 2014 Marvell Technology Group Ltd. > >+ * > >+ * Antoine Ténart <antoine.tenart@free-electrons.com> > >+ * > >+ * This file is licensed under the terms of the GNU General Public > >+ * License version 2. This program is licensed "as is" without any > >+ * warranty of any kind, whether express or implied. > >+ */ > >+ > >+#ifndef __PINCTRL_BERLIN_H > >+#define __PINCTRL_BERLIN_H > >+ > >+struct berlin_desc_group { > >+ const char *name; > >+ const char *base_name; > >+ u32 offset; > >+ u32 bit_width; > >+ u32 lsb; > > You expect any offsets, bit_widths or lsbs at 2^32-1 ? ;) Sure :) I'll update. Thanks for the review ! Antoine -- Antoine Ténart, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 2/5] pinctrl: berlin: add a pinctrl driver for Marvell Berlin SoCs 2014-04-11 12:37 ` Antoine Ténart @ 2014-04-11 13:35 ` Sebastian Hesselbarth 2014-04-22 12:52 ` Linus Walleij 0 siblings, 1 reply; 30+ messages in thread From: Sebastian Hesselbarth @ 2014-04-11 13:35 UTC (permalink / raw) To: Antoine Ténart Cc: linus.walleij, alexandre.belloni, zmxu, jszhang, linux-arm-kernel, devicetree, linux-kernel On 04/11/2014 02:37 PM, Antoine Ténart wrote: > On Fri, Apr 11, 2014 at 11:03:48AM +0200, Sebastian Hesselbarth wrote: >> On 04/10/2014 03:07 PM, Antoine Ténart wrote: >>> The Marvell Berlin boards have a group based pinmuxing mechanism. This >>> driver adds the support for the BG2CD, BG2 and BG2Q. We actually do not >>> need any information about the pins here and only have the definition >>> of the groups. >> [...] >>> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile >>> index 4b835880cf80..fd5a01d4475f 100644 >>> --- a/drivers/pinctrl/Makefile >>> +++ b/drivers/pinctrl/Makefile >>> @@ -21,6 +21,7 @@ obj-$(CONFIG_PINCTRL_BF60x) += pinctrl-adi2-bf60x.o >>> obj-$(CONFIG_PINCTRL_AT91) += pinctrl-at91.o >>> obj-$(CONFIG_PINCTRL_BCM2835) += pinctrl-bcm2835.o >>> obj-$(CONFIG_PINCTRL_BAYTRAIL) += pinctrl-baytrail.o >>> +obj-$(CONFIG_PINCTRL_BERLIN) += pinctrl-berlin.o >> >> Please split the driver into common and soc-specific parts, and if >> you do please put it into a subfolder berlin, too. > > The only SoC specific part is the group structure. I'll need to have > functions in each SoC specific part calling ... only the common ones. > Do you have a better solution in mind ? We have a similar structure for mvebu, control is common, structure is not. You'll have one common driver to register pinctrl and switch a specific mux, and one file for each SoC holding the group structure and function table. If bg2 and bg2cd have the same group layout but different functions assigned to it, you can either strictly separate by SoC or separate by similarity. Have a look at e.g. pinctrl/mvebu/pinctrl-kirkwood.c, although SoCs are different with respect to individual functions, we have a variant mask to determine if a specific function is available on that SoC. But if bg2 and bg2cd are _very_ different from the functions assigned to each group, I'd rather have two separate files, too. Basically, folder structure will look like this: drivers/pinctrl +- berlin/ +- common.c : common code (pinctrl register, pinmux) +- common.h : common include (group structure, common callbacks) +- bg2.c : BG2 specific group/function tables +- bg2cd.c : BG2CD specific group/function tables +- bg2q.c : BG2Q specific group/function tables +- ... (BTW, as we know we are beneath drivers/pinctrl, drop the pinctrl- prefix) If we consider bg2cd as a variant of bg2, join the two files above and have a variant mask as we have for Kirkwood. [...] >>> + function_name = kzalloc(strlen(group_name) + 7, GFP_KERNEL); >>> + if (!function_name) >>> + return -ENOMEM; >>> + snprintf(function_name, strlen(group_name) + 7, "%s_mode%d", group_name, >>> + function); >> >> With proper mode tables above, this can refer to a const char* and you >> can get rid of allocation here. Also, below you already allocated >> function_names, how is this one different from the below? > > The one below is used to describe the available functions for a given group > while this one describe a function found in the device tree. It is used to check > the function we want to use in the device is provided by the driver. The Ah, ok. With numeric values for marvell,function and proper function tables for each of the groups, you can just look up the group and loop over the available functions. Or if you prefer strings, give a proper name to each function in the table and use that string as DT match. > function name used here may not actually exist. We could check the function > actually exists here (and find the previously allocated function name), but this > check is handled by the pinctrl framework. This would end up in a different > behaviour than the expected one, imho. [...] >>> +static int berlin_pinctrl_build_state(struct platform_device *pdev, >>> + struct berlin_pinctrl_reg_base *bases, >>> + unsigned nbases) >>> +{ >>> + struct berlin_pinctrl *pctrl = platform_get_drvdata(pdev); >>> + int i, j, cur_fct = 0; >>> + >>> + pctrl->ngroups = pctrl->desc->ngroups; >>> + pctrl->nfunctions = 0; >>> + >>> + for (i = 0; i < pctrl->ngroups; i++) { >>> + struct berlin_desc_group const *desc = pctrl->desc->groups + i; >>> + pctrl->nfunctions += NFCTS(desc->bit_width); >>> + } >> >> As you need desc later on, make it global to this function. Then you >> can also walk through it with desc++ >> >> desc = pctrl->desc->groups; >> for (i = 0; i < pctl->ngroups; i++, desc++) >> pctrl->nfunctions = ... >> >> Having said that, the above assumes that each function is unique >> but IIRC the idea of the function table was to group pins/groups >> with the same function, e.g. function "gpio", groups 1,7,25,... > > Most of the functions you can use on the Berlin they will be unique and would > only be used in one group, except for the 'gpio' one. Yeah, I had a similar discussion about it back then for mvebu. IIRC, the correct answer is: Have a list of functions with groups assigned to it no matter if there is only one group per function (or 40 per function as it will be for gpio). Maybe Linus can give an update on how to deal with it? Sebastian ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 2/5] pinctrl: berlin: add a pinctrl driver for Marvell Berlin SoCs 2014-04-11 13:35 ` Sebastian Hesselbarth @ 2014-04-22 12:52 ` Linus Walleij 2014-04-22 15:56 ` Antoine Ténart 0 siblings, 1 reply; 30+ messages in thread From: Linus Walleij @ 2014-04-22 12:52 UTC (permalink / raw) To: Sebastian Hesselbarth Cc: Antoine Ténart, Alexandre Belloni, zmxu, Jisheng Zhang, linux-arm-kernel, devicetree, linux-kernel On Fri, Apr 11, 2014 at 3:35 PM, Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> wrote: > On 04/11/2014 02:37 PM, Antoine Ténart wrote: >> On Fri, Apr 11, 2014 at 11:03:48AM +0200, Sebastian Hesselbarth wrote: >>> On 04/10/2014 03:07 PM, Antoine Ténart wrote: >>> Having said that, the above assumes that each function is unique >>> but IIRC the idea of the function table was to group pins/groups >>> with the same function, e.g. function "gpio", groups 1,7,25,... >> >> Most of the functions you can use on the Berlin they will be unique and >> would >> only be used in one group, except for the 'gpio' one. > > Yeah, I had a similar discussion about it back then for mvebu. IIRC, the > correct answer is: Have a list of functions with groups assigned to it > no matter if there is only one group per function (or 40 per function as > it will be for gpio). > > Maybe Linus can give an update on how to deal with it? Have you considered implementing pinmux_ops .gpio_request_enable(), .gpio_set_direction() and .gpio_disable_free() instead of defining groups for each and every GPIO? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 2/5] pinctrl: berlin: add a pinctrl driver for Marvell Berlin SoCs 2014-04-22 12:52 ` Linus Walleij @ 2014-04-22 15:56 ` Antoine Ténart 2014-04-23 14:05 ` Linus Walleij 0 siblings, 1 reply; 30+ messages in thread From: Antoine Ténart @ 2014-04-22 15:56 UTC (permalink / raw) To: Linus Walleij Cc: Sebastian Hesselbarth, Alexandre Belloni, zmxu, Jisheng Zhang, linux-arm-kernel, devicetree, linux-kernel Linus, On Tue, Apr 22, 2014 at 02:52:10PM +0200, Linus Walleij wrote: > On Fri, Apr 11, 2014 at 3:35 PM, Sebastian Hesselbarth > <sebastian.hesselbarth@gmail.com> wrote: > > On 04/11/2014 02:37 PM, Antoine Ténart wrote: > >> On Fri, Apr 11, 2014 at 11:03:48AM +0200, Sebastian Hesselbarth wrote: > >>> On 04/10/2014 03:07 PM, Antoine Ténart wrote: > > >>> Having said that, the above assumes that each function is unique > >>> but IIRC the idea of the function table was to group pins/groups > >>> with the same function, e.g. function "gpio", groups 1,7,25,... > >> > >> Most of the functions you can use on the Berlin they will be unique and > >> would > >> only be used in one group, except for the 'gpio' one. > > > > Yeah, I had a similar discussion about it back then for mvebu. IIRC, the > > correct answer is: Have a list of functions with groups assigned to it > > no matter if there is only one group per function (or 40 per function as > > it will be for gpio). > > > > Maybe Linus can give an update on how to deal with it? > > Have you considered implementing pinmux_ops > .gpio_request_enable(), .gpio_set_direction() and > .gpio_disable_free() instead of defining groups for each > and every GPIO? The function 'gpio' can be found on different groups. But the Berlin pin muxing does not allow to configure a pin individually. It is then not possible to mux GPIO pins individually. For example the 'gpio' function of group 'GSM2' on the BG2Q will mux GPIOs 17 *and* 18. Groups does not have more than a single 'gpio' function. Since the gpio_request_enable() comment says 'Implement this only if you can mux every pin individually as GPIO', I did not considered implementing these functions. Antoine -- Antoine Ténart, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 2/5] pinctrl: berlin: add a pinctrl driver for Marvell Berlin SoCs 2014-04-22 15:56 ` Antoine Ténart @ 2014-04-23 14:05 ` Linus Walleij 0 siblings, 0 replies; 30+ messages in thread From: Linus Walleij @ 2014-04-23 14:05 UTC (permalink / raw) To: Antoine Ténart Cc: Sebastian Hesselbarth, Alexandre Belloni, zmxu, Jisheng Zhang, linux-arm-kernel, devicetree, linux-kernel On Tue, Apr 22, 2014 at 5:56 PM, Antoine Ténart <antoine.tenart@free-electrons.com> wrote: > On Tue, Apr 22, 2014 at 02:52:10PM +0200, Linus Walleij wrote: >> On Fri, Apr 11, 2014 at 3:35 PM, Sebastian Hesselbarth >> <sebastian.hesselbarth@gmail.com> wrote: >> > On 04/11/2014 02:37 PM, Antoine Ténart wrote: >> >> On Fri, Apr 11, 2014 at 11:03:48AM +0200, Sebastian Hesselbarth wrote: >> >>> On 04/10/2014 03:07 PM, Antoine Ténart wrote: >> >> >>> Having said that, the above assumes that each function is unique >> >>> but IIRC the idea of the function table was to group pins/groups >> >>> with the same function, e.g. function "gpio", groups 1,7,25,... >> >> >> >> Most of the functions you can use on the Berlin they will be unique and >> >> would >> >> only be used in one group, except for the 'gpio' one. >> > >> > Yeah, I had a similar discussion about it back then for mvebu. IIRC, the >> > correct answer is: Have a list of functions with groups assigned to it >> > no matter if there is only one group per function (or 40 per function as >> > it will be for gpio). >> > >> > Maybe Linus can give an update on how to deal with it? >> >> Have you considered implementing pinmux_ops >> .gpio_request_enable(), .gpio_set_direction() and >> .gpio_disable_free() instead of defining groups for each >> and every GPIO? > > The function 'gpio' can be found on different groups. But the Berlin pin muxing > does not allow to configure a pin individually. It is then not possible to mux > GPIO pins individually. For example the 'gpio' function of group 'GSM2' on the > BG2Q will mux GPIOs 17 *and* 18. > > Groups does not have more than a single 'gpio' function. > > Since the gpio_request_enable() comment says 'Implement this only if you can mux > every pin individually as GPIO', I did not considered implementing these > functions. OK makes perfect sense. Then I guess the current implementation is the best alternative, but I may need to look closer. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH RESEND 3/5] ARM: berlin: add the pinctrl dependency for the Marvell Berlin SoCs 2014-04-10 13:07 [PATCH RESEND 0/5] ARM: berlin: add pinctrl support Antoine Ténart 2014-04-10 13:07 ` [PATCH RESEND 1/5] pinctrl: allows not to define the get_group_pins operation Antoine Ténart 2014-04-10 13:07 ` [PATCH RESEND 2/5] pinctrl: berlin: add a pinctrl driver for Marvell Berlin SoCs Antoine Ténart @ 2014-04-10 13:07 ` Antoine Ténart 2014-04-11 9:05 ` Sebastian Hesselbarth 2014-04-10 13:07 ` [PATCH RESEND 4/5] Documentation: add the Marvell Berlin pinctrl documentation Antoine Ténart 2014-04-10 13:07 ` [PATCH RESEND 5/5] ARM: dts: berlin: add the pinctrl node and muxing setup for uarts Antoine Ténart 4 siblings, 1 reply; 30+ messages in thread From: Antoine Ténart @ 2014-04-10 13:07 UTC (permalink / raw) To: sebastian.hesselbarth, linus.walleij Cc: Antoine Ténart, alexandre.belloni, zmxu, jszhang, linux-arm-kernel, devicetree, linux-kernel Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> --- arch/arm/mach-berlin/Kconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/mach-berlin/Kconfig b/arch/arm/mach-berlin/Kconfig index d3c5f14dc142..291f1cac6c3d 100644 --- a/arch/arm/mach-berlin/Kconfig +++ b/arch/arm/mach-berlin/Kconfig @@ -4,6 +4,8 @@ config ARCH_BERLIN select GENERIC_IRQ_CHIP select DW_APB_ICTL select DW_APB_TIMER_OF + select PINCTRL + select PINCTRL_BERLIN if ARCH_BERLIN -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 3/5] ARM: berlin: add the pinctrl dependency for the Marvell Berlin SoCs 2014-04-10 13:07 ` [PATCH RESEND 3/5] ARM: berlin: add the pinctrl dependency for the " Antoine Ténart @ 2014-04-11 9:05 ` Sebastian Hesselbarth 2014-04-17 13:13 ` Antoine Ténart 0 siblings, 1 reply; 30+ messages in thread From: Sebastian Hesselbarth @ 2014-04-11 9:05 UTC (permalink / raw) To: Antoine Ténart, linus.walleij Cc: alexandre.belloni, zmxu, jszhang, linux-arm-kernel, devicetree, linux-kernel On 04/10/2014 03:07 PM, Antoine Ténart wrote: > Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> > --- > arch/arm/mach-berlin/Kconfig | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm/mach-berlin/Kconfig b/arch/arm/mach-berlin/Kconfig > index d3c5f14dc142..291f1cac6c3d 100644 > --- a/arch/arm/mach-berlin/Kconfig > +++ b/arch/arm/mach-berlin/Kconfig > @@ -4,6 +4,8 @@ config ARCH_BERLIN > select GENERIC_IRQ_CHIP > select DW_APB_ICTL > select DW_APB_TIMER_OF > + select PINCTRL > + select PINCTRL_BERLIN As said in the review of the previous patch, drop PINCTRL here and select it in the PINCTRL_BERLIN Kconfig. Also, you'll need PINCTRL_BERLIN_BG2 and PINCTRL_BERLIN_BG2Q as we don't want to bloat each Berlin kernel with both drivers. Sebastian > > if ARCH_BERLIN > > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 3/5] ARM: berlin: add the pinctrl dependency for the Marvell Berlin SoCs 2014-04-11 9:05 ` Sebastian Hesselbarth @ 2014-04-17 13:13 ` Antoine Ténart 2014-04-17 13:24 ` Sebastian Hesselbarth 0 siblings, 1 reply; 30+ messages in thread From: Antoine Ténart @ 2014-04-17 13:13 UTC (permalink / raw) To: Sebastian Hesselbarth Cc: linus.walleij, alexandre.belloni, zmxu, jszhang, linux-arm-kernel, devicetree, linux-kernel Sebastian, On Fri, Apr 11, 2014 at 11:05:41AM +0200, Sebastian Hesselbarth wrote: > On 04/10/2014 03:07 PM, Antoine Ténart wrote: > >Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> > >--- > > arch/arm/mach-berlin/Kconfig | 2 ++ > > 1 file changed, 2 insertions(+) > > > >diff --git a/arch/arm/mach-berlin/Kconfig b/arch/arm/mach-berlin/Kconfig > >index d3c5f14dc142..291f1cac6c3d 100644 > >--- a/arch/arm/mach-berlin/Kconfig > >+++ b/arch/arm/mach-berlin/Kconfig > >@@ -4,6 +4,8 @@ config ARCH_BERLIN > > select GENERIC_IRQ_CHIP > > select DW_APB_ICTL > > select DW_APB_TIMER_OF > >+ select PINCTRL > >+ select PINCTRL_BERLIN > > As said in the review of the previous patch, drop PINCTRL here > and select it in the PINCTRL_BERLIN Kconfig. Also, you'll need > PINCTRL_BERLIN_BG2 and PINCTRL_BERLIN_BG2Q as we don't want to > bloat each Berlin kernel with both drivers. Regarding PINCTRL, I can't drop it, pin-controller drivers depend on it. drivers/pinctrl/Kconfig begins with: if PINCTRL Antoine -- Antoine Ténart, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 3/5] ARM: berlin: add the pinctrl dependency for the Marvell Berlin SoCs 2014-04-17 13:13 ` Antoine Ténart @ 2014-04-17 13:24 ` Sebastian Hesselbarth 0 siblings, 0 replies; 30+ messages in thread From: Sebastian Hesselbarth @ 2014-04-17 13:24 UTC (permalink / raw) To: Antoine Ténart Cc: linus.walleij, alexandre.belloni, zmxu, jszhang, linux-arm-kernel, devicetree, linux-kernel On 04/17/2014 03:13 PM, Antoine Ténart wrote: > On Fri, Apr 11, 2014 at 11:05:41AM +0200, Sebastian Hesselbarth wrote: >> On 04/10/2014 03:07 PM, Antoine Ténart wrote: >>> Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> >>> --- >>> arch/arm/mach-berlin/Kconfig | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/arch/arm/mach-berlin/Kconfig b/arch/arm/mach-berlin/Kconfig >>> index d3c5f14dc142..291f1cac6c3d 100644 >>> --- a/arch/arm/mach-berlin/Kconfig >>> +++ b/arch/arm/mach-berlin/Kconfig >>> @@ -4,6 +4,8 @@ config ARCH_BERLIN >>> select GENERIC_IRQ_CHIP >>> select DW_APB_ICTL >>> select DW_APB_TIMER_OF >>> + select PINCTRL >>> + select PINCTRL_BERLIN >> >> As said in the review of the previous patch, drop PINCTRL here >> and select it in the PINCTRL_BERLIN Kconfig. Also, you'll need >> PINCTRL_BERLIN_BG2 and PINCTRL_BERLIN_BG2Q as we don't want to >> bloat each Berlin kernel with both drivers. > > Regarding PINCTRL, I can't drop it, pin-controller drivers depend on it. > drivers/pinctrl/Kconfig begins with: > if PINCTRL Ah, ok. Then select PINCTRL here and the corresponding PINCTRL_BERLIN_foo in the individual SoC Kconfig entries. Sebastian ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH RESEND 4/5] Documentation: add the Marvell Berlin pinctrl documentation 2014-04-10 13:07 [PATCH RESEND 0/5] ARM: berlin: add pinctrl support Antoine Ténart ` (2 preceding siblings ...) 2014-04-10 13:07 ` [PATCH RESEND 3/5] ARM: berlin: add the pinctrl dependency for the " Antoine Ténart @ 2014-04-10 13:07 ` Antoine Ténart 2014-04-11 6:33 ` Jisheng Zhang 2014-04-11 9:13 ` Sebastian Hesselbarth 2014-04-10 13:07 ` [PATCH RESEND 5/5] ARM: dts: berlin: add the pinctrl node and muxing setup for uarts Antoine Ténart 4 siblings, 2 replies; 30+ messages in thread From: Antoine Ténart @ 2014-04-10 13:07 UTC (permalink / raw) To: sebastian.hesselbarth, linus.walleij Cc: Antoine Ténart, alexandre.belloni, zmxu, jszhang, linux-arm-kernel, devicetree, linux-kernel Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> --- .../bindings/pinctrl/marvell,berlin-pinctrl.txt | 43 ++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/marvell,berlin-pinctrl.txt diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,berlin-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/marvell,berlin-pinctrl.txt new file mode 100644 index 000000000000..e21538a37734 --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/marvell,berlin-pinctrl.txt @@ -0,0 +1,43 @@ +* Pinctrl driver for the Marvell Berlin SoCs + +The pins controlled by the Marvell Berlin controller are organized in groups. +Configuration is done by group, so no actual pin information is needed. + +Required properties: +- compatible: "marvell-berlin2cd-pinctrl", + "marvell-berlin2-pinctrl", + "marvell-berlin2q-pinctrl" +- reg: registers physical addresses and lengths of the pin controller. +- reg-names: name of the register regions. + +Please refer to pinctrl-bindings.txt in this directory for details of the +common pinctrl bindings used by client devices. + +A pinctrl should contains subnodes representing the pinctrl pin group +configurations, one per group. Each subnode has the group name and the muxing +function used. + +Required subnode-properties: +- berlin,group: a string describing the group's name. +- berlin,function: the function used to mux the group. + 0: function 0 + 1: function 1 + ... + +Example: + +pinctrl: pinctrl@0 { + compatible = "marvell,berlin2q-pinctrl"; + reg = <0xea0000 0x08>, <0xfc0000 0x44>; + reg-names = "global_base", "sm_base"; + + uart0_pmux: uart0-pmux { + berlin,group = "GSM12"; + berlin,function = <0>; + }; +} + +&uart0 { + pinctrl-0 = <&uart0_pmux>; + pinctrl-names = "default"; +}; -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 4/5] Documentation: add the Marvell Berlin pinctrl documentation 2014-04-10 13:07 ` [PATCH RESEND 4/5] Documentation: add the Marvell Berlin pinctrl documentation Antoine Ténart @ 2014-04-11 6:33 ` Jisheng Zhang 2014-04-11 8:12 ` Antoine Ténart 2014-04-11 9:13 ` Sebastian Hesselbarth 1 sibling, 1 reply; 30+ messages in thread From: Jisheng Zhang @ 2014-04-11 6:33 UTC (permalink / raw) To: Antoine Ténart Cc: sebastian.hesselbarth, linus.walleij, alexandre.belloni, Jimmy Xu, linux-arm-kernel, devicetree, linux-kernel On Thu, 10 Apr 2014 06:07:53 -0700 Antoine Ténart <antoine.tenart@free-electrons.com> wrote: > Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> > --- > .../bindings/pinctrl/marvell,berlin-pinctrl.txt | 43 > ++++++++++++++++++++++ 1 file changed, 43 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/pinctrl/marvell,berlin-pinctrl.txt > > diff --git > a/Documentation/devicetree/bindings/pinctrl/marvell,berlin-pinctrl.txt > b/Documentation/devicetree/bindings/pinctrl/marvell,berlin-pinctrl.txt new > file mode 100644 index 000000000000..e21538a37734 --- /dev/null > +++ b/Documentation/devicetree/bindings/pinctrl/marvell,berlin-pinctrl.txt > @@ -0,0 +1,43 @@ > +* Pinctrl driver for the Marvell Berlin SoCs > + > +The pins controlled by the Marvell Berlin controller are organized in > groups. +Configuration is done by group, so no actual pin information is > needed. + > +Required properties: > +- compatible: "marvell-berlin2cd-pinctrl", > + "marvell-berlin2-pinctrl", > + "marvell-berlin2q-pinctrl" > +- reg: registers physical addresses and lengths of the pin controller. > +- reg-names: name of the register regions. > + > +Please refer to pinctrl-bindings.txt in this directory for details of the > +common pinctrl bindings used by client devices. > + > +A pinctrl should contains subnodes representing the pinctrl pin group > +configurations, one per group. Each subnode has the group name and the > muxing +function used. > + > +Required subnode-properties: > +- berlin,group: a string describing the group's name. > +- berlin,function: the function used to mux the group. Would it be better to s/function/mode? Because in mrvl internal technology discussion and docs, we usually say "set pinmux Gy as mode z". Thanks, Jisheng ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 4/5] Documentation: add the Marvell Berlin pinctrl documentation 2014-04-11 6:33 ` Jisheng Zhang @ 2014-04-11 8:12 ` Antoine Ténart 2014-04-11 8:18 ` Jisheng Zhang 2014-04-11 8:22 ` Andrew Lunn 0 siblings, 2 replies; 30+ messages in thread From: Antoine Ténart @ 2014-04-11 8:12 UTC (permalink / raw) To: Jisheng Zhang Cc: sebastian.hesselbarth, linus.walleij, alexandre.belloni, Jimmy Xu, linux-arm-kernel, devicetree, linux-kernel Hi Jisheng, On Fri, Apr 11, 2014 at 02:33:18PM +0800, Jisheng Zhang wrote: > On Thu, 10 Apr 2014 06:07:53 -0700 > Antoine Ténart <antoine.tenart@free-electrons.com> wrote: > > +Required subnode-properties: > > +- berlin,group: a string describing the group's name. > > +- berlin,function: the function used to mux the group. > > Would it be better to s/function/mode? Because in mrvl internal technology > discussion and docs, we usually say "set pinmux Gy as mode z". The pinctrl framework uses 'function', so I guess it's better not to introduce an other keyword that may confuse people. But I agree this is not what's used in the Marvell's docs. What do you think about we mention it in this documentation ? Thanks for the feedback ! Antoine -- Antoine Ténart, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 4/5] Documentation: add the Marvell Berlin pinctrl documentation 2014-04-11 8:12 ` Antoine Ténart @ 2014-04-11 8:18 ` Jisheng Zhang 2014-04-11 8:22 ` Andrew Lunn 1 sibling, 0 replies; 30+ messages in thread From: Jisheng Zhang @ 2014-04-11 8:18 UTC (permalink / raw) To: Antoine Ténart Cc: sebastian.hesselbarth, linus.walleij, alexandre.belloni, Jimmy Xu, linux-arm-kernel, devicetree, linux-kernel Hi Antoine, On Fri, 11 Apr 2014 01:12:59 -0700 Antoine Ténart <antoine.tenart@free-electrons.com> wrote: > Hi Jisheng, > > On Fri, Apr 11, 2014 at 02:33:18PM +0800, Jisheng Zhang wrote: > > On Thu, 10 Apr 2014 06:07:53 -0700 > > Antoine Ténart <antoine.tenart@free-electrons.com> wrote: > > > +Required subnode-properties: > > > +- berlin,group: a string describing the group's name. > > > +- berlin,function: the function used to mux the group. > > > > Would it be better to s/function/mode? Because in mrvl internal technology > > discussion and docs, we usually say "set pinmux Gy as mode z". > > The pinctrl framework uses 'function', so I guess it's better not to > introduce an other keyword that may confuse people. But I agree this is not > what's used in the Marvell's docs. What do you think about we mention it in > this documentation ? That's a good idea. Thanks, Jisheng ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 4/5] Documentation: add the Marvell Berlin pinctrl documentation 2014-04-11 8:12 ` Antoine Ténart 2014-04-11 8:18 ` Jisheng Zhang @ 2014-04-11 8:22 ` Andrew Lunn 1 sibling, 0 replies; 30+ messages in thread From: Andrew Lunn @ 2014-04-11 8:22 UTC (permalink / raw) To: Antoine Ténart Cc: Jisheng Zhang, Jimmy Xu, devicetree, linus.walleij, linux-kernel, alexandre.belloni, linux-arm-kernel, sebastian.hesselbarth On Fri, Apr 11, 2014 at 10:12:59AM +0200, Antoine Ténart wrote: > Hi Jisheng, > > On Fri, Apr 11, 2014 at 02:33:18PM +0800, Jisheng Zhang wrote: > > On Thu, 10 Apr 2014 06:07:53 -0700 > > Antoine Ténart <antoine.tenart@free-electrons.com> wrote: > > > +Required subnode-properties: > > > +- berlin,group: a string describing the group's name. > > > +- berlin,function: the function used to mux the group. > > > > Would it be better to s/function/mode? Because in mrvl internal technology > > discussion and docs, we usually say "set pinmux Gy as mode z". > > The pinctrl framework uses 'function', so I guess it's better not to introduce > an other keyword that may confuse people. Agreed. DT is a standard, Marvell datasheets are just datasheets. > But I agree this is not what's used in the Marvell's docs. What do > you think about we mention it in this documentation ? Or ask for the datahsheet to be changed :-) Andrew ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 4/5] Documentation: add the Marvell Berlin pinctrl documentation 2014-04-10 13:07 ` [PATCH RESEND 4/5] Documentation: add the Marvell Berlin pinctrl documentation Antoine Ténart 2014-04-11 6:33 ` Jisheng Zhang @ 2014-04-11 9:13 ` Sebastian Hesselbarth 1 sibling, 0 replies; 30+ messages in thread From: Sebastian Hesselbarth @ 2014-04-11 9:13 UTC (permalink / raw) To: Antoine Ténart, linus.walleij Cc: alexandre.belloni, zmxu, jszhang, linux-arm-kernel, devicetree, linux-kernel On 04/10/2014 03:07 PM, Antoine Ténart wrote: Missing commit message. > Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> > --- > .../bindings/pinctrl/marvell,berlin-pinctrl.txt | 43 ++++++++++++++++++++++ > 1 file changed, 43 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pinctrl/marvell,berlin-pinctrl.txt > > diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,berlin-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/marvell,berlin-pinctrl.txt > new file mode 100644 > index 000000000000..e21538a37734 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pinctrl/marvell,berlin-pinctrl.txt > @@ -0,0 +1,43 @@ > +* Pinctrl driver for the Marvell Berlin SoCs > + > +The pins controlled by the Marvell Berlin controller are organized in groups. > +Configuration is done by group, so no actual pin information is needed. > + > +Required properties: > +- compatible: "marvell-berlin2cd-pinctrl", > + "marvell-berlin2-pinctrl", > + "marvell-berlin2q-pinctrl" s/marvell-/marvell,/g Also, you can add at least some prosa like "compatible: should be one of"... > +- reg: registers physical addresses and lengths of the pin controller. > +- reg-names: name of the register regions. > + > +Please refer to pinctrl-bindings.txt in this directory for details of the > +common pinctrl bindings used by client devices. > + > +A pinctrl should contains subnodes representing the pinctrl pin group s/pinctrl/pin-controller/g s/contains/contain > +configurations, one per group. Each subnode has the group name and the muxing > +function used. > + > +Required subnode-properties: > +- berlin,group: a string describing the group's name. > +- berlin,function: the function used to mux the group. > + 0: function 0 > + 1: function 1 > + ... s/berlin,/marvell,/ And I also prefer either DT binding includes for each function (even if the proper name is not known). > + > +Example: > + > +pinctrl: pinctrl@0 { I looked at ePAPR recommended node names yesterday. It uses foo-controller for all listed examples. Also, with the idea of splitting both pin-controllers into separate nodes the above (with broken unit-address) will become: soc_pinctrl: pin-controller@ea0000 { compatible = "marvell,berlin2q-soc-pinctrl"; reg = <0xea0000 0x08>; }; and sm_pinctrl: pin-controller@fc0000 { compatible = "marvell,berlin2q-sysmgr-pinctrl"; reg = <0xfc0000 0x44>; }; Sebastian > + compatible = "marvell,berlin2q-pinctrl"; > + reg = <0xea0000 0x08>, <0xfc0000 0x44>; > + reg-names = "global_base", "sm_base"; > + > + uart0_pmux: uart0-pmux { > + berlin,group = "GSM12"; > + berlin,function = <0>; > + }; > +} > + > +&uart0 { > + pinctrl-0 = <&uart0_pmux>; > + pinctrl-names = "default"; > +}; > ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH RESEND 5/5] ARM: dts: berlin: add the pinctrl node and muxing setup for uarts 2014-04-10 13:07 [PATCH RESEND 0/5] ARM: berlin: add pinctrl support Antoine Ténart ` (3 preceding siblings ...) 2014-04-10 13:07 ` [PATCH RESEND 4/5] Documentation: add the Marvell Berlin pinctrl documentation Antoine Ténart @ 2014-04-10 13:07 ` Antoine Ténart 2014-04-11 8:18 ` Andrew Lunn 4 siblings, 1 reply; 30+ messages in thread From: Antoine Ténart @ 2014-04-10 13:07 UTC (permalink / raw) To: sebastian.hesselbarth, linus.walleij Cc: Antoine Ténart, alexandre.belloni, zmxu, jszhang, linux-arm-kernel, devicetree, linux-kernel The uart0 pinmux configuration is in the dtsi because uart0 will always use uart0-pmux to work, no other possibility. Same thing for uart1. Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> --- arch/arm/boot/dts/berlin2.dtsi | 20 ++++++++++++++++++++ arch/arm/boot/dts/berlin2cd.dtsi | 13 +++++++++++++ arch/arm/boot/dts/berlin2q.dtsi | 20 ++++++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi index 56a1af2f1052..43eb90c36050 100644 --- a/arch/arm/boot/dts/berlin2.dtsi +++ b/arch/arm/boot/dts/berlin2.dtsi @@ -176,6 +176,22 @@ }; }; + pinctrl: pinctrl@0 { + compatible = "marvell,berlin2-pinctrl"; + reg = <0xea0000 0x08>, <0xfc0000 0x44>; + reg-names = "global_base", "apb_base"; + + uart0_pmux: uart0-pmux { + berlin,group = "GSM4"; + berlin,function = <0>; + }; + + uart1_pmux: uart1-pmux { + berlin,group = "GSM5"; + berlin,function = <1>; + }; + }; + apb@fc0000 { compatible = "simple-bus"; #address-cells = <1>; @@ -191,6 +207,8 @@ reg-io-width = <1>; interrupts = <8>; clocks = <&smclk>; + pinctrl-0 = <&uart0_pmux>; + pinctrl-names = "default"; status = "disabled"; }; @@ -201,6 +219,8 @@ reg-io-width = <1>; interrupts = <9>; clocks = <&smclk>; + pinctrl-0 = <&uart1_pmux>; + pinctrl-names = "default"; status = "disabled"; }; diff --git a/arch/arm/boot/dts/berlin2cd.dtsi b/arch/arm/boot/dts/berlin2cd.dtsi index 094968c27533..4c04cff89226 100644 --- a/arch/arm/boot/dts/berlin2cd.dtsi +++ b/arch/arm/boot/dts/berlin2cd.dtsi @@ -169,6 +169,17 @@ }; }; + pinctrl: pinctrl@0 { + compatible = "marvell,berlin2cd-pinctrl"; + reg = <0xea0000 0x08>, <0xfc0000 0x44>; + reg-names = "global_base", "apb_base"; + + uart0_pmux: uart0-pmux { + berlin,group = "G6"; + berlin,function = <0>; + }; + }; + apb@fc0000 { compatible = "simple-bus"; #address-cells = <1>; @@ -184,6 +195,8 @@ reg-io-width = <1>; interrupts = <8>; clocks = <&smclk>; + pinctrl-0 = <&uart0_pmux>; + pinctrl-names = "default"; status = "disabled"; }; diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi index 07452a7483fa..e6e556055dfc 100644 --- a/arch/arm/boot/dts/berlin2q.dtsi +++ b/arch/arm/boot/dts/berlin2q.dtsi @@ -183,6 +183,22 @@ }; }; + pinctrl: pinctrl@0 { + compatible = "marvell,berlin2q-pinctrl"; + reg = <0xea0000 0x4c>, <0xfcd000 0x44>; + reg-names = "global_base", "sm_base"; + + uart0_pmux: uart0-pmux { + berlin,group = "GSM12"; + berlin,function = <0>; + }; + + uart1_pmux: uart1-pmux { + berlin,group = "GSM14"; + berlin,function = <1>; + }; + }; + apb@fc0000 { compatible = "simple-bus"; #address-cells = <1>; @@ -198,6 +214,8 @@ interrupts = <8>; clocks = <&smclk>; reg-shift = <2>; + pinctrl-0 = <&uart0_pmux>; + pinctrl-names = "default"; status = "disabled"; }; @@ -208,6 +226,8 @@ interrupts = <9>; clocks = <&smclk>; reg-shift = <2>; + pinctrl-0 = <&uart1_pmux>; + pinctrl-names = "default"; status = "disabled"; }; -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 5/5] ARM: dts: berlin: add the pinctrl node and muxing setup for uarts 2014-04-10 13:07 ` [PATCH RESEND 5/5] ARM: dts: berlin: add the pinctrl node and muxing setup for uarts Antoine Ténart @ 2014-04-11 8:18 ` Andrew Lunn 2014-04-11 9:09 ` Antoine Ténart 0 siblings, 1 reply; 30+ messages in thread From: Andrew Lunn @ 2014-04-11 8:18 UTC (permalink / raw) To: Antoine Ténart Cc: sebastian.hesselbarth, linus.walleij, zmxu, jszhang, devicetree, linux-kernel, alexandre.belloni, linux-arm-kernel On Thu, Apr 10, 2014 at 03:07:54PM +0200, Antoine Ténart wrote: > The uart0 pinmux configuration is in the dtsi because uart0 will always > use uart0-pmux to work, no other possibility. Same thing for uart1. > > Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> > --- > arch/arm/boot/dts/berlin2.dtsi | 20 ++++++++++++++++++++ > arch/arm/boot/dts/berlin2cd.dtsi | 13 +++++++++++++ > arch/arm/boot/dts/berlin2q.dtsi | 20 ++++++++++++++++++++ > 3 files changed, 53 insertions(+) > > diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi > index 56a1af2f1052..43eb90c36050 100644 > --- a/arch/arm/boot/dts/berlin2.dtsi > +++ b/arch/arm/boot/dts/berlin2.dtsi > @@ -176,6 +176,22 @@ > }; > }; > > + pinctrl: pinctrl@0 { > + compatible = "marvell,berlin2-pinctrl"; > + reg = <0xea0000 0x08>, <0xfc0000 0x44>; > + reg-names = "global_base", "apb_base"; > + > + uart0_pmux: uart0-pmux { > + berlin,group = "GSM4"; > + berlin,function = <0>; > + }; > + > + uart1_pmux: uart1-pmux { > + berlin,group = "GSM5"; > + berlin,function = <1>; Hi Antoine This very much looks like black magic. I assume the data sheet is not available? So i think you need to document all possible combinations of values of group and function. Maybe you can add a file in arch/arm/boot/dts/include/dt-bindings/pinctrl/ with something like #define GSM4_UART0 0 #define GSM5_UART1 1 #define GSM12_UART0 1 #define GSM12_IrDA0 1 #define GSM12_GPIO 2 Andrew ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 5/5] ARM: dts: berlin: add the pinctrl node and muxing setup for uarts 2014-04-11 8:18 ` Andrew Lunn @ 2014-04-11 9:09 ` Antoine Ténart 2014-04-11 9:19 ` Sebastian Hesselbarth 0 siblings, 1 reply; 30+ messages in thread From: Antoine Ténart @ 2014-04-11 9:09 UTC (permalink / raw) To: Andrew Lunn Cc: sebastian.hesselbarth, linus.walleij, zmxu, jszhang, devicetree, linux-kernel, alexandre.belloni, linux-arm-kernel Hi Andrew, On Fri, Apr 11, 2014 at 10:18:59AM +0200, Andrew Lunn wrote: > On Thu, Apr 10, 2014 at 03:07:54PM +0200, Antoine Ténart wrote: > > The uart0 pinmux configuration is in the dtsi because uart0 will always > > use uart0-pmux to work, no other possibility. Same thing for uart1. > > > > Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> > > --- > > arch/arm/boot/dts/berlin2.dtsi | 20 ++++++++++++++++++++ > > arch/arm/boot/dts/berlin2cd.dtsi | 13 +++++++++++++ > > arch/arm/boot/dts/berlin2q.dtsi | 20 ++++++++++++++++++++ > > 3 files changed, 53 insertions(+) > > > > diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi > > index 56a1af2f1052..43eb90c36050 100644 > > --- a/arch/arm/boot/dts/berlin2.dtsi > > +++ b/arch/arm/boot/dts/berlin2.dtsi > > @@ -176,6 +176,22 @@ > > }; > > }; > > > > + pinctrl: pinctrl@0 { > > + compatible = "marvell,berlin2-pinctrl"; > > + reg = <0xea0000 0x08>, <0xfc0000 0x44>; > > + reg-names = "global_base", "apb_base"; > > + > > + uart0_pmux: uart0-pmux { > > + berlin,group = "GSM4"; > > + berlin,function = <0>; > > + }; > > + > > + uart1_pmux: uart1-pmux { > > + berlin,group = "GSM5"; > > + berlin,function = <1>; > > Hi Antoine > > This very much looks like black magic. > > I assume the data sheet is not available? So i think you need to > document all possible combinations of values of group and function. > Maybe you can add a file in > > arch/arm/boot/dts/include/dt-bindings/pinctrl/ > > with something like > > #define GSM4_UART0 0 > #define GSM5_UART1 1 > #define GSM12_UART0 1 > #define GSM12_IrDA0 1 > #define GSM12_GPIO 2 Groups' functions are not the same between BG2/BG2CD and BG2Q so multiple headers would be needed which may not be a good thing. Some platforms use black magic, and we can find a pinmux configuration for OMAP looking like: 0x128 (PIN_INPUT_PULLUP | MUX_MODE0) Weel, we could explicitly define the functions in the driver itself and use in the dt something like: berlin,group = "GSM4"; berlin,function = "uart0"; This is what's done on the sunxi's pinctrl. Antoine -- Antoine Ténart, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH RESEND 5/5] ARM: dts: berlin: add the pinctrl node and muxing setup for uarts 2014-04-11 9:09 ` Antoine Ténart @ 2014-04-11 9:19 ` Sebastian Hesselbarth 0 siblings, 0 replies; 30+ messages in thread From: Sebastian Hesselbarth @ 2014-04-11 9:19 UTC (permalink / raw) To: Antoine Ténart, Andrew Lunn Cc: linus.walleij, zmxu, jszhang, devicetree, linux-kernel, alexandre.belloni, linux-arm-kernel On 04/11/2014 11:09 AM, Antoine Ténart wrote: > Hi Andrew, > > On Fri, Apr 11, 2014 at 10:18:59AM +0200, Andrew Lunn wrote: >> On Thu, Apr 10, 2014 at 03:07:54PM +0200, Antoine Ténart wrote: >>> The uart0 pinmux configuration is in the dtsi because uart0 will always >>> use uart0-pmux to work, no other possibility. Same thing for uart1. >>> >>> Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> >>> --- >>> arch/arm/boot/dts/berlin2.dtsi | 20 ++++++++++++++++++++ >>> arch/arm/boot/dts/berlin2cd.dtsi | 13 +++++++++++++ >>> arch/arm/boot/dts/berlin2q.dtsi | 20 ++++++++++++++++++++ >>> 3 files changed, 53 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi >>> index 56a1af2f1052..43eb90c36050 100644 >>> --- a/arch/arm/boot/dts/berlin2.dtsi >>> +++ b/arch/arm/boot/dts/berlin2.dtsi >>> @@ -176,6 +176,22 @@ >>> }; >>> }; >>> >>> + pinctrl: pinctrl@0 { >>> + compatible = "marvell,berlin2-pinctrl"; >>> + reg = <0xea0000 0x08>, <0xfc0000 0x44>; >>> + reg-names = "global_base", "apb_base"; >>> + >>> + uart0_pmux: uart0-pmux { >>> + berlin,group = "GSM4"; >>> + berlin,function = <0>; >>> + }; >>> + >>> + uart1_pmux: uart1-pmux { >>> + berlin,group = "GSM5"; >>> + berlin,function = <1>; >> >> This very much looks like black magic. >> >> I assume the data sheet is not available? So i think you need to >> document all possible combinations of values of group and function. >> Maybe you can add a file in >> >> arch/arm/boot/dts/include/dt-bindings/pinctrl/ >> >> with something like >> >> #define GSM4_UART0 0 >> #define GSM5_UART1 1 >> #define GSM12_UART0 1 >> #define GSM12_IrDA0 1 >> #define GSM12_GPIO 2 > > Groups' functions are not the same between BG2/BG2CD and BG2Q so multiple > headers would be needed which may not be a good thing. Some platforms use black > magic, and we can find a pinmux configuration for OMAP looking like: > > 0x128 (PIN_INPUT_PULLUP | MUX_MODE0) And it is a constant pain-in-the-ass to understand it at all without looking it up in different places. For Berlin, the only publically available option is BSP code, and you don't want to look it up in there even once more than necessary. ;) I don't see a problem in having one binding for each (different) SoC at all. > Weel, we could explicitly define the functions in the driver itself and use in > the dt something like: > > berlin,group = "GSM4"; > berlin,function = "uart0"; > > This is what's done on the sunxi's pinctrl. We also have function strings in mvebu pinctrl. The name is very useful for /sys/kernel/debug/pinctrl/ but nothing more. Matching numbers is easier than matching string, so I suggest to stick with binding includes as Andrew suggested. Sebastian ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2014-04-23 14:05 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-04-10 13:07 [PATCH RESEND 0/5] ARM: berlin: add pinctrl support Antoine Ténart 2014-04-10 13:07 ` [PATCH RESEND 1/5] pinctrl: allows not to define the get_group_pins operation Antoine Ténart 2014-04-11 9:21 ` Sebastian Hesselbarth 2014-04-22 12:48 ` Linus Walleij 2014-04-22 15:58 ` Antoine Ténart 2014-04-10 13:07 ` [PATCH RESEND 2/5] pinctrl: berlin: add a pinctrl driver for Marvell Berlin SoCs Antoine Ténart 2014-04-11 6:44 ` Jisheng Zhang 2014-04-11 8:18 ` Antoine Ténart 2014-04-11 8:27 ` Jisheng Zhang 2014-04-11 9:21 ` Antoine Ténart 2014-04-11 9:03 ` Sebastian Hesselbarth 2014-04-11 12:37 ` Antoine Ténart 2014-04-11 13:35 ` Sebastian Hesselbarth 2014-04-22 12:52 ` Linus Walleij 2014-04-22 15:56 ` Antoine Ténart 2014-04-23 14:05 ` Linus Walleij 2014-04-10 13:07 ` [PATCH RESEND 3/5] ARM: berlin: add the pinctrl dependency for the " Antoine Ténart 2014-04-11 9:05 ` Sebastian Hesselbarth 2014-04-17 13:13 ` Antoine Ténart 2014-04-17 13:24 ` Sebastian Hesselbarth 2014-04-10 13:07 ` [PATCH RESEND 4/5] Documentation: add the Marvell Berlin pinctrl documentation Antoine Ténart 2014-04-11 6:33 ` Jisheng Zhang 2014-04-11 8:12 ` Antoine Ténart 2014-04-11 8:18 ` Jisheng Zhang 2014-04-11 8:22 ` Andrew Lunn 2014-04-11 9:13 ` Sebastian Hesselbarth 2014-04-10 13:07 ` [PATCH RESEND 5/5] ARM: dts: berlin: add the pinctrl node and muxing setup for uarts Antoine Ténart 2014-04-11 8:18 ` Andrew Lunn 2014-04-11 9:09 ` Antoine Ténart 2014-04-11 9:19 ` Sebastian Hesselbarth
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).