Messages in this thread | | | Date | Tue, 27 Aug 2013 09:51:39 +0200 | From | Nicolas Ferre <> | Subject | Re: [RFC PATCH 2/3] pinctrl: at91: add support for generic pinconf |
| |
On 26/08/2013 21:18, Jean-Christophe PLAGNIOL-VILLARD : > On 20:45 Mon 26 Aug , boris brezillon wrote: >> Hello Jean-Christophe, >> >> Le 26/08/2013 19:53, Jean-Christophe PLAGNIOL-VILLARD a écrit : >>> On 23:37 Sat 24 Aug , Boris BREZILLON wrote: >>>> Add support for generic pin configuration to pinctrl-at91 driver. >>>> >>>> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com> >>>> --- >>>> .../bindings/pinctrl/atmel,at91-pinctrl.txt | 43 +++- >>>> drivers/pinctrl/Kconfig | 2 +- >>>> drivers/pinctrl/pinctrl-at91.c | 265 ++++++++++++++++++-- >>>> 3 files changed, 289 insertions(+), 21 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt >>>> index 7ccae49..7a7c0c4 100644 >>>> --- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt >>>> +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt >>>> @@ -18,7 +18,9 @@ mode) this pin can work on and the 'config' configures various pad settings >>>> such as pull-up, multi drive, etc. >>>> Required properties for iomux controller: >>>> -- compatible: "atmel,at91rm9200-pinctrl" >>>> +- compatible: "atmel,at91rm9200-pinctrl" or "atmel,at91sam9x5-pinctrl". >>>> + Add "generic-pinconf" to the compatible string list to use the generic pin >>>> + configuration syntax. >>>> - atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be >>>> configured in this periph mode. All the periph and bank need to be describe. >>>> @@ -83,6 +85,11 @@ Required properties for pin configuration node: >>>> setting. The format is atmel,pins = <PIN_BANK PIN_BANK_NUM PERIPH CONFIG>. >>>> The PERIPH 0 means gpio, PERIPH 1 is periph A, PERIPH 2 is periph B... >>>> PIN_BANK 0 is pioA, PIN_BANK 1 is pioB... >>>> + Dependending on the presence of the "generic-pinconf" string in the >>>> + compatible property the 4th cell is: >>>> + * a phandle referencing a generic pin config node (refer to >>>> + pinctrl-bindings.txt) >>>> + * an integer defining the pin config (see the following description) >>>> Bits used for CONFIG: >>>> PULL_UP (1 << 0): indicate this pin need a pull up. >>>> @@ -132,6 +139,40 @@ pinctrl@fffff400 { >>>> }; >>>> }; >>>> +or >>>> + >>>> +pinctrl@fffff400 { >>>> + #address-cells = <1>; >>>> + #size-cells = <1>; >>>> + ranges; >>>> + compatible = "atmel,at91rm9200-pinctrl", "generic-pinconf", "simple-bus"; >>> nack your break the backword compatibility >>> >>> if we use a old kernel with this new dt nothing will work >>> as the old kernel will never known the the "generic-pinconf" means anything >> >> Your're right, I didn't think of this case (old kernel with new dt). >> >>> if we want to use generic-pinconf support you *CAN NOT* use atmel,at91rm9200-pinctrl
One. I did not spot.
>>> in the compatible >> >> What about using "atmel,at91xx-pinconf" instead of >> "atmel,at91xx-pinctrl" to notify >> the generic pinconf compatibility (as done by single pinctrl driver) ? > no as the rm9200 IP and sam9x5 IP are only partially compatible > you MUST distinguish them
Two? WTF!
Jean-Christophe, YOU MUST NOT SCREAM in emails, okay?!
>>>> + reg = <0xfffff400 0x600>; >>>> + >>>> + atmel,mux-mask = < >>>> + /* A B */ >>>> + 0xffffffff 0xffc00c3b /* pioA */ >>>> + 0xffffffff 0x7fff3ccf /* pioB */ >>>> + 0xffffffff 0x007fffff /* pioC */ >>>> + >; >>>> + >>>> + pcfg_none: pcfg_none { >>>> + bias-disable; >>>> + }; >>>> + >>>> + pcfg_pull_up: pcfg_pull_up { >>>> + bias-pullup; >>>> + }; >>>> + >>>> + /* shared pinctrl settings */ >>>> + dbgu { >>>> + pinctrl_dbgu: dbgu-0 { >>>> + atmel,pins = >>>> + <1 14 0x1 &pcfg_none /* PB14 periph A */ >>>> + 1 15 0x1 &pcfg_pull_up>; /* PB15 periph A with pullup */ >>>> + }; >>>> + }; >>>> +}; >>>> + >>>> dbgu: serial@fffff200 { >>>> compatible = "atmel,at91sam9260-usart"; >>>> reg = <0xfffff200 0x200>; >>>> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig >>>> index bdb1a87..55a4f2c 100644 >>>> --- a/drivers/pinctrl/Kconfig >>>> +++ b/drivers/pinctrl/Kconfig >>>> @@ -54,7 +54,7 @@ config PINCTRL_AT91 >>>> depends on OF >>>> depends on ARCH_AT91 >>>> select PINMUX >>>> - select PINCONF >>>> + select GENERIC_PINCONF >>>> help >>>> Say Y here to enable the at91 pinctrl driver >>>> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c >>>> index 7cce066..1994dd2 100644 >>>> --- a/drivers/pinctrl/pinctrl-at91.c >>>> +++ b/drivers/pinctrl/pinctrl-at91.c >>>> @@ -23,6 +23,7 @@ >>>> #include <linux/gpio.h> >>>> #include <linux/pinctrl/machine.h> >>>> #include <linux/pinctrl/pinconf.h> >>>> +#include <linux/pinctrl/pinconf-generic.h> >>>> #include <linux/pinctrl/pinctrl.h> >>>> #include <linux/pinctrl/pinmux.h> >>>> /* Since we request GPIOs from ourself */ >>>> @@ -32,6 +33,7 @@ >>>> #include <mach/at91_pio.h> >>>> #include "core.h" >>>> +#include "pinconf.h" >>>> #define MAX_NB_GPIO_PER_BANK 32 >>>> @@ -85,6 +87,21 @@ enum at91_mux { >>>> AT91_MUX_PERIPH_D = 4, >>>> }; >>>> +struct at91_generic_pinconf { >>>> + unsigned long *configs; >>>> + unsigned int nconfigs; >>>> +}; >>>> + >>>> +enum at91_pinconf_type { >>>> + AT91_PINCONF_NATIVE, >>>> + AT91_PINCONF_GENERIC, >>>> +}; >>>> + >>>> +union at91_pinconf { >>>> + unsigned long native; >>>> + struct at91_generic_pinconf generic; >>>> +}; >>>> + >>>> /** >>>> * struct at91_pmx_pin - describes an At91 pin mux >>>> * @bank: the bank of the pin >>>> @@ -93,10 +110,11 @@ enum at91_mux { >>>> * @conf: the configuration of the pin: PULL_UP, MULTIDRIVE etc... >>>> */ >>>> struct at91_pmx_pin { >>>> - uint32_t bank; >>>> - uint32_t pin; >>>> - enum at91_mux mux; >>>> - unsigned long conf; >>>> + uint32_t bank; >>>> + uint32_t pin; >>>> + enum at91_mux mux; >>>> + enum at91_pinconf_type conftype; >>>> + union at91_pinconf conf; >>>> }; >>>> /** >>>> @@ -278,8 +296,16 @@ static int at91_dt_node_to_map(struct pinctrl_dev *pctldev, >>>> new_map[i].type = PIN_MAP_TYPE_CONFIGS_PIN; >>>> new_map[i].data.configs.group_or_pin = >>>> pin_get_name(pctldev, grp->pins[i]); >>>> - new_map[i].data.configs.configs = &grp->pins_conf[i].conf; >>>> - new_map[i].data.configs.num_configs = 1; >>>> + if (!pctldev->desc->confops->is_generic) { >>>> + new_map[i].data.configs.configs = >>>> + &grp->pins_conf[i].conf.native; >>>> + new_map[i].data.configs.num_configs = 1; >>>> + } else { >>>> + new_map[i].data.configs.configs = >>>> + grp->pins_conf[i].conf.generic.configs; >>>> + new_map[i].data.configs.num_configs = >>>> + grp->pins_conf[i].conf.generic.nconfigs; >>>> + } >>>> } >>>> dev_dbg(pctldev->dev, "maps: function %s group %s num %d\n", >>>> @@ -325,7 +351,7 @@ static void at91_mux_disable_interrupt(void __iomem *pio, unsigned mask) >>>> static unsigned at91_mux_get_pullup(void __iomem *pio, unsigned pin) >>>> { >>>> - return (readl_relaxed(pio + PIO_PUSR) >> pin) & 0x1; >>>> + return !((readl_relaxed(pio + PIO_PUSR) >> pin) & 0x1); >>>> } >>>> static void at91_mux_set_pullup(void __iomem *pio, unsigned mask, bool on) >>>> @@ -407,6 +433,16 @@ static enum at91_mux at91_mux_get_periph(void __iomem *pio, unsigned mask) >>>> return select + 1; >>>> } >>>> +static bool at91_mux_get_output(void __iomem *pio, unsigned pin) >>>> +{ >>>> + return (readl_relaxed(pio + PIO_OSR) >> pin) & 0x1; >>>> +} >>>> + >>>> +static void at91_mux_set_output(void __iomem *pio, unsigned mask, bool is_on) >>>> +{ >>>> + __raw_writel(mask, pio + (is_on ? PIO_OER : PIO_ODR)); >>>> +} >>>> + >>>> static bool at91_mux_get_deglitch(void __iomem *pio, unsigned pin) >>>> { >>>> return (__raw_readl(pio + PIO_IFSR) >> pin) & 0x1; >>>> @@ -445,7 +481,7 @@ static void at91_mux_pio3_set_debounce(void __iomem *pio, unsigned mask, >>>> static bool at91_mux_pio3_get_pulldown(void __iomem *pio, unsigned pin) >>>> { >>>> - return (__raw_readl(pio + PIO_PPDSR) >> pin) & 0x1; >>>> + return !((__raw_readl(pio + PIO_PPDSR) >> pin) & 0x1); >>>> } >>>> static void at91_mux_pio3_set_pulldown(void __iomem *pio, unsigned mask, bool is_on) >>>> @@ -492,12 +528,17 @@ static struct at91_pinctrl_mux_ops at91sam9x5_ops = { >>>> static void at91_pin_dbg(const struct device *dev, const struct at91_pmx_pin *pin) >>>> { >>>> if (pin->mux) { >>>> - dev_dbg(dev, "pio%c%d configured as periph%c with conf = 0x%lu\n", >>>> - pin->bank + 'A', pin->pin, pin->mux - 1 + 'A', pin->conf); >>>> + dev_dbg(dev, "pio%c%d configured as periph%c", >>>> + pin->bank + 'A', pin->pin, pin->mux - 1 + 'A'); >>>> } else { >>>> - dev_dbg(dev, "pio%c%d configured as gpio with conf = 0x%lu\n", >>>> - pin->bank + 'A', pin->pin, pin->conf); >>>> + dev_dbg(dev, "pio%c%d configured as gpio", >>>> + pin->bank + 'A', pin->pin); >>>> } >>>> + >>>> + if (pin->conftype == AT91_PINCONF_NATIVE) >>>> + dev_dbg(dev, " with conf = 0x%lu\n", pin->conf.native); >>>> + else >>>> + dev_dbg(dev, "\n"); >>> do not change debug output >> >> I do not change the debug output for the native pinconf binding, but >> I cannot print the config as >> a single interger in hex format if the generic pinconf is used. >> I must translate each config entry to a "property=value" string, or >> translate the generic config >> array to a single native config integer. >> >> Do you have something easier in mind ? > > no but I do not want to brake current automatic test tools > > propose something with this in mind > > Best Regards, > J. >
-- Nicolas Ferre -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |