| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <521C5A8B.2030607@atmel.com>
Date: Tue, 27 Aug 2013 09:51:39 +0200
From: Nicolas Ferre <nicolas.ferre@...el.com>
To: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@...osoft.com>
CC: boris brezillon <b.brezillon@...rkiz.com>,
Rob Herring <rob.herring@...xeda.com>,
Pawel Moll <pawel.moll@....com>,
Mark Rutland <mark.rutland@....com>,
Stephen Warren <swarren@...dotorg.org>,
Ian Campbell <ian.campbell@...rix.com>,
Rob Landley <rob@...dley.net>,
Russell King <linux@....linux.org.uk>,
Linus Walleij <linus.walleij@...aro.org>,
"Jiri Kosina" <jkosina@...e.cz>,
Masanari Iida <standby24x7@...il.com>,
"Richard Genoud" <richard.genoud@...il.com>,
Heiko Stuebner <heiko@...ech.de>,
"James Hogan" <james.hogan@...tec.com>,
<devicetree@...r.kernel.org>, <linux-doc@...r.kernel.org>,
<linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>
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@...rkiz.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@...ff400 {
>>>> };
>>>> };
>>>> +or
>>>> +
>>>> +pinctrl@...ff400 {
>>>> + #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@...ff200 {
>>>> 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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists