| Message ID | 1520615640-9153-2-git-send-email-matheus@castello.eng.br |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v4,1/3] dt-bindings: pinctrl: bcm2835-gpio: Add generic pinctrl support | expand |
On Fri, Mar 09, 2018 at 01:13:58PM -0400, Matheus Castello wrote: > Added generic pin configuration and multiplexing support, > and shoud be preferred than brcm legacy one. > > Signed-off-by: Matheus Castello <matheus@castello.eng.br> > --- > .../devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt > index 2569866..58b4720 100644 > --- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt > +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt > @@ -36,6 +36,25 @@ listed. In other words, a subnode that lists only a mux function implies no > information about any pull configuration. Similarly, a subnode that lists only > a pul parameter implies no information about the mux function. > > +This driver supports the generic pin multiplexing and configuration Bindings describe h/w, not drivers. > +bindings. For details on each properties, you can refer to > +./pinctrl-bindings.txt. > + > +Required sub-node properties: > + - pins > + - function > + > +Optional sub-node properties: > + - bias-disable > + - bias-pull-up > + - bias-pull-down > + - output-high > + - output-low > + > +Legacy pin configuration and multiplexing binding: > +*** (Its use is deprecated, use generic multiplexing and configuration > +bindings instead) > + > Required subnode-properties: > - brcm,pins: An array of cells. Each cell contains the ID of a pin. Valid IDs > are the integer GPIO IDs; 0==GPIO0, 1==GPIO1, ... 53==GPIO53. > -- > 2.7.4 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob, sorry I used allwinner,sunxi-pinctrl.txt from Maxime Ripard as a base, I used the same words, I thought it would be correct. I will modify this to: The BCM2835 pin configuration and multiplexing supports the generic bindings. For details on each properties, you can refer to ./pinctrl-bindings.txt. If it's okay for you let me know, so I can send the v5 patch. Best Regards Matheus Castello On 03/18/2018 08:48 AM, Rob Herring wrote: > On Fri, Mar 09, 2018 at 01:13:58PM -0400, Matheus Castello wrote: >> Added generic pin configuration and multiplexing support, >> and shoud be preferred than brcm legacy one. >> >> Signed-off-by: Matheus Castello <matheus@castello.eng.br> >> --- >> .../devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt >> index 2569866..58b4720 100644 >> --- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt >> +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt >> @@ -36,6 +36,25 @@ listed. In other words, a subnode that lists only a mux function implies no >> information about any pull configuration. Similarly, a subnode that lists only >> a pul parameter implies no information about the mux function. >> >> +This driver supports the generic pin multiplexing and configuration > > Bindings describe h/w, not drivers. > >> +bindings. For details on each properties, you can refer to >> +./pinctrl-bindings.txt. >> + >> +Required sub-node properties: >> + - pins >> + - function >> + >> +Optional sub-node properties: >> + - bias-disable >> + - bias-pull-up >> + - bias-pull-down >> + - output-high >> + - output-low >> + >> +Legacy pin configuration and multiplexing binding: >> +*** (Its use is deprecated, use generic multiplexing and configuration >> +bindings instead) >> + >> Required subnode-properties: >> - brcm,pins: An array of cells. Each cell contains the ID of a pin. Valid IDs >> are the integer GPIO IDs; 0==GPIO0, 1==GPIO1, ... 53==GPIO53. >> -- >> 2.7.4 >> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
This series adds support for generic binding for pinctrl bcm2835 driver, and add the code for set output buffer of a pin using the output-low and output-high generic properties. Tested on Raspberry Pi Zero W, based on bcm2835 SoC. Changes since v4: (Suggested by Rob Herring) - Change dt-bindings docs driver reference to hardware in case the BCM2835 pin configuration and multiplexing Changes since v3: (Suggested by Stefan Wahren) - Change dt-bindings docs patch order and subject Changes since v2: (Suggested by Eric Anholt) - Remove PACK and UNPACK macros - Use pinconf_to_config_* functions (Suggested by Stefan Wahren) - Fold Kconfig changes with the driver changes in a single patch - Add devicetree bindings documentations about generic properties support - Add devicetree bindings maintainers Matheus Castello (3): dt-bindings: pinctrl: bcm2835-gpio: Add generic pinctrl support pinctrl: bcm2835: Add support for generic pinctrl binding pinctrl: bcm2835: Add support for output-low output-high properties .../bindings/pinctrl/brcm,bcm2835-gpio.txt | 18 +++++ drivers/pinctrl/bcm/Kconfig | 1 + drivers/pinctrl/bcm/pinctrl-bcm2835.c | 92 ++++++++++++++-------- 3 files changed, 78 insertions(+), 33 deletions(-) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Matheus, > Matheus Castello <matheus@castello.eng.br> hat am 11. April 2018 um 06:58 geschrieben: > > > To keep driver up to date we add generic pinctrl binding support, which covers > the features used in this driver and has additional node properties that this > SoC has compatibility, so enabling future implementations of these properties > without the need to create new node properties in the device trees. > > The logic of this change maintain the old brcm legacy binding support in order > to keep the ABI stable. > > Signed-off-by: Matheus Castello <matheus@castello.eng.br> > --- > > A brief explanation of what I did: > > Add pinconf-generic header for use the defines and pinctrl-generic API. > > Add dt-bindings pinctrl bcm2835 header to use functions selections and > pulls definitions, which functions definitions where duplicated in the > enum bcm2835_fsel, I removed the duplicate defines from enum. > > In the bcm2835_pctl_dt_node_to_map_pull I used the generic macro for > pack the legacy param and arguments, since it will be unpacked along with > generic properties that is packed with this same macro. > > In bcm2835_pctl_dt_node_to_map I thougt it was better, and simpler, to use > pinctrl-generic parse code instead of parsing it inside the driver, so code > first check for generic binding parse, if something is parsed then it is > assumed that are using the new generic style, and when nothing is found then > parse continues to search for legacy properties. > > In the bcm2835_pinconf_set was changed the unpack legacy by the generic, and > was added a switch for the parameter tests, since pinctrl generic uses 3 > properties to define the states of the pull instead of one with arguments, that > was the reason too that bcm2835_pull_config_set function was added, for reuse > the code that set state of pull. > > drivers/pinctrl/bcm/Kconfig | 1 + > drivers/pinctrl/bcm/pinctrl-bcm2835.c | 87 ++++++++++++++++++++++------------- > 2 files changed, 55 insertions(+), 33 deletions(-) > > diff --git a/drivers/pinctrl/bcm/Kconfig b/drivers/pinctrl/bcm/Kconfig > index e8c4e4f..0f38d51 100644 > --- a/drivers/pinctrl/bcm/Kconfig > +++ b/drivers/pinctrl/bcm/Kconfig > @@ -20,6 +20,7 @@ config PINCTRL_BCM2835 > bool > select PINMUX > select PINCONF > + select GENERIC_PINCONF > select GPIOLIB_IRQCHIP > > config PINCTRL_IPROC_GPIO > diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c > index 785c366..010c565 100644 > --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c > +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c > @@ -36,11 +36,13 @@ > #include <linux/pinctrl/pinconf.h> > #include <linux/pinctrl/pinctrl.h> > #include <linux/pinctrl/pinmux.h> > +#include <linux/pinctrl/pinconf-generic.h> > #include <linux/platform_device.h> > #include <linux/seq_file.h> > #include <linux/slab.h> > #include <linux/spinlock.h> > #include <linux/types.h> > +#include <dt-bindings/pinctrl/bcm2835.h> > > #define MODULE_NAME "pinctrl-bcm2835" > #define BCM2835_NUM_GPIOS 54 > @@ -75,10 +77,6 @@ enum bcm2835_pinconf_param { > BCM2835_PINCONF_PARAM_PULL, Since we use bcm2835_pinconf_param and pin_config_param in bcm2835_pinconf_set, there are potential conflicts. According to include/linux/pinctrl/pinconf-generic.h: * @PIN_CONFIG_END: this is the last enumerator for pin configurations, if * you need to pass in custom configurations to the pin controller, use * PIN_CONFIG_END+1 as the base offset. Please adjust BCM2835_PINCONF_PARAM_PULL accordingly. Sorry for not notice this before. Stefan -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Matheus, > Matheus Castello <matheus@castello.eng.br> hat am 11. April 2018 um 06:58 geschrieben: > > > To keep driver up to date we add generic pinctrl binding support, which covers > the features used in this driver and has additional node properties that this > SoC has compatibility, so enabling future implementations of these properties > without the need to create new node properties in the device trees. > > The logic of this change maintain the old brcm legacy binding support in order > to keep the ABI stable. > > Signed-off-by: Matheus Castello <matheus@castello.eng.br> > --- > > A brief explanation of what I did: > > Add pinconf-generic header for use the defines and pinctrl-generic API. > > Add dt-bindings pinctrl bcm2835 header to use functions selections and > pulls definitions, which functions definitions where duplicated in the > enum bcm2835_fsel, I removed the duplicate defines from enum. > > In the bcm2835_pctl_dt_node_to_map_pull I used the generic macro for > pack the legacy param and arguments, since it will be unpacked along with > generic properties that is packed with this same macro. > > In bcm2835_pctl_dt_node_to_map I thougt it was better, and simpler, to use > pinctrl-generic parse code instead of parsing it inside the driver, so code > first check for generic binding parse, if something is parsed then it is > assumed that are using the new generic style, and when nothing is found then > parse continues to search for legacy properties. > > In the bcm2835_pinconf_set was changed the unpack legacy by the generic, and > was added a switch for the parameter tests, since pinctrl generic uses 3 > properties to define the states of the pull instead of one with arguments, that > was the reason too that bcm2835_pull_config_set function was added, for reuse > the code that set state of pull. > > drivers/pinctrl/bcm/Kconfig | 1 + > drivers/pinctrl/bcm/pinctrl-bcm2835.c | 87 ++++++++++++++++++++++------------- > 2 files changed, 55 insertions(+), 33 deletions(-) > > diff --git a/drivers/pinctrl/bcm/Kconfig b/drivers/pinctrl/bcm/Kconfig > index e8c4e4f..0f38d51 100644 > --- a/drivers/pinctrl/bcm/Kconfig > +++ b/drivers/pinctrl/bcm/Kconfig > @@ -20,6 +20,7 @@ config PINCTRL_BCM2835 > bool > select PINMUX > select PINCONF > + select GENERIC_PINCONF > select GPIOLIB_IRQCHIP > > config PINCTRL_IPROC_GPIO > diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c > index 785c366..010c565 100644 > --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c > +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c > ... > @@ -917,37 +913,62 @@ static int bcm2835_pinconf_get(struct pinctrl_dev *pctldev, > return -ENOTSUPP; > } > > +static void bcm2835_pull_config_set(struct bcm2835_pinctrl *pc, > + unsigned pin, unsigned arg) checkpatch is complaining about missing "int" here. Please fix this here. > +{ > + u32 off, bit; > + > + off = GPIO_REG_OFFSET(pin); > + bit = GPIO_REG_SHIFT(pin); > + > + bcm2835_gpio_wr(pc, GPPUD, arg & 3); > + /* > + * BCM2835 datasheet say to wait 150 cycles, but not of what. > + * But the VideoCore firmware delay for this operation > + * based nearly on the same amount of VPU cycles and this clock > + * runs at 250 MHz. > + */ checkpatch complains here about comment alignment. Please fix it. Stefan > + udelay(1); > + bcm2835_gpio_wr(pc, GPPUDCLK0 + (off * 4), BIT(bit)); > + udelay(1); > + bcm2835_gpio_wr(pc, GPPUDCLK0 + (off * 4), 0); > +} > + > static int bcm2835_pinconf_set(struct pinctrl_dev *pctldev, > unsigned pin, unsigned long *configs, > unsigned num_configs) > { > struct bcm2835_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev); > - enum bcm2835_pinconf_param param; > - u16 arg; > - u32 off, bit; > + u32 param, arg; > int i; > > for (i = 0; i < num_configs; i++) { > - param = BCM2835_PINCONF_UNPACK_PARAM(configs[i]); > - arg = BCM2835_PINCONF_UNPACK_ARG(configs[i]); > + param = pinconf_to_config_param(configs[i]); > + arg = pinconf_to_config_argument(configs[i]); > > - if (param != BCM2835_PINCONF_PARAM_PULL) > - return -EINVAL; > + switch (param) { > + /* Set legacy brcm,pull */ > + case BCM2835_PINCONF_PARAM_PULL: > + bcm2835_pull_config_set(pc, pin, arg); > + break; > > - off = GPIO_REG_OFFSET(pin); > - bit = GPIO_REG_SHIFT(pin); > + /* Set pull generic bindings */ > + case PIN_CONFIG_BIAS_DISABLE: > + bcm2835_pull_config_set(pc, pin, BCM2835_PUD_OFF); > + break; > > - bcm2835_gpio_wr(pc, GPPUD, arg & 3); > - /* > - * BCM2835 datasheet say to wait 150 cycles, but not of what. > - * But the VideoCore firmware delay for this operation > - * based nearly on the same amount of VPU cycles and this clock > - * runs at 250 MHz. > - */ > - udelay(1); > - bcm2835_gpio_wr(pc, GPPUDCLK0 + (off * 4), BIT(bit)); > - udelay(1); > - bcm2835_gpio_wr(pc, GPPUDCLK0 + (off * 4), 0); > + case PIN_CONFIG_BIAS_PULL_DOWN: > + bcm2835_pull_config_set(pc, pin, BCM2835_PUD_DOWN); > + break; > + > + case PIN_CONFIG_BIAS_PULL_UP: > + bcm2835_pull_config_set(pc, pin, BCM2835_PUD_UP); > + break; > + > + default: > + return -EINVAL; > + > + } /* switch param type */ > } /* for each config */ > > return 0; > -- > 2.7.4 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 11, 2018 at 6:58 AM, Matheus Castello <matheus@castello.eng.br> wrote: > To keep driver up to date we add generic pinctrl binding support, which covers > the features used in this driver and has additional node properties that this > SoC has compatibility, so enabling future implementations of these properties > without the need to create new node properties in the device trees. > > The logic of this change maintain the old brcm legacy binding support in order > to keep the ABI stable. > > Signed-off-by: Matheus Castello <matheus@castello.eng.br> Looking good, just need to address Stefan's comments. (I already merged patch 1 so you do not need to resend this.) The driver is kind of part of what Stefan and Eric maintains but it would be nice to get a nod from Stephen Warren as well as in my mind he is also maintainer for this. Please include him on subsequent postings as well. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 11, 2018 at 6:58 AM, Matheus Castello <matheus@castello.eng.br> wrote: > Properties to set initial value of pin output buffer. > This can be useful for configure hardware in overlay files, and in early boot > for checking it states in QA sanity tests. > > Signed-off-by: Matheus Castello <matheus@castello.eng.br> Will apply this when I apply 2/3. You only need to resend patch 2+3. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
This series adds support for generic binding for pinctrl bcm2835 driver, and add the code for set output buffer of a pin using the output-low and output-high generic properties. Tested on Raspberry Pi Zero W, based on bcm2835 SoC. Changes since v5: (Suggested by Stefan Wahren) - Fix checkpatch warnings - Use PIN_CONFIG_END+1 for bcm2835_pinconf_param for our pull legacy configuration (Suggested by Linus Walleij) - Only resend patch 2+3 - Add Stephen Warren for subsequent postings Changes since v4: (Suggested by Rob Herring) - Change dt-bindings docs driver reference to hardware in case the BCM2835 pin configuration and multiplexing Changes since v3: (Suggested by Stefan Wahren) - Change dt-bindings docs patch order and subject Changes since v2: (Suggested by Eric Anholt) - Remove PACK and UNPACK macros - Use pinconf_to_config_* functions (Suggested by Stefan Wahren) - Fold Kconfig changes with the driver changes in a single patch - Add devicetree bindings documentations about generic properties support - Add devicetree bindings maintainers Matheus Castello (2): pinctrl: bcm2835: Add support for generic pinctrl binding pinctrl: bcm2835: Add support for output-low output-high properties drivers/pinctrl/bcm/Kconfig | 1 + drivers/pinctrl/bcm/pinctrl-bcm2835.c | 100 +++++++++++++++++++++------------- 2 files changed, 64 insertions(+), 37 deletions(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt index 2569866..58b4720 100644 --- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt @@ -36,6 +36,25 @@ listed. In other words, a subnode that lists only a mux function implies no information about any pull configuration. Similarly, a subnode that lists only a pul parameter implies no information about the mux function. +This driver supports the generic pin multiplexing and configuration +bindings. For details on each properties, you can refer to +./pinctrl-bindings.txt. + +Required sub-node properties: + - pins + - function + +Optional sub-node properties: + - bias-disable + - bias-pull-up + - bias-pull-down + - output-high + - output-low + +Legacy pin configuration and multiplexing binding: +*** (Its use is deprecated, use generic multiplexing and configuration +bindings instead) + Required subnode-properties: - brcm,pins: An array of cells. Each cell contains the ID of a pin. Valid IDs are the integer GPIO IDs; 0==GPIO0, 1==GPIO1, ... 53==GPIO53.
Added generic pin configuration and multiplexing support, and shoud be preferred than brcm legacy one. Signed-off-by: Matheus Castello <matheus@castello.eng.br> --- .../devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)