Re: [PATCH] pinctrl: bc2835: Add brcm,level property

[Stefan Wahren]

Hi Matheus,

sorry for my late reply.

> Matheus Castello  hat am 22. Februar 2018 um 18:44 
> geschrieben:
> 
> 
> Property to set initial value of pin output buffer.
> 
> Signed-off-by: Matheus Castello 
> ---
>  .../bindings/pinctrl/brcm,bcm2835-gpio.txt |  5 +-
>  drivers/pinctrl/bcm/pinctrl-bcm2835.c  | 90 
> +-
>  2 files changed, 75 insertions(+), 20 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt 
> b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
> index 2569866..6834f1d 100644
> --- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt

Changes to the devicetree binding should be a separate patch. Please read [1] 
and use the get_maintainers script for all patches.

[1] - 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/submitting-patches.txt

> @@ -54,8 +54,11 @@ Optional subnode-properties:
>0: none
>1: down
>2: up
> +- brcm,level: Integer, representing the value of output buffer to apply to 
> the pin(s):
> +  0: low
> +  1: high

As Linus suggested please don't introduce new legacy properties. We already 
have generic ones for this. After mention the supported properties, simply make 
a reference to the generic binding.

>  
> -Each of brcm,function and brcm,pull may contain either a single value which
> +Each of brcm,function, brcm,pull and brcm,level may contain either a single 
> value which
>  will be applied to all pins in brcm,pins, or 1 value for each entry in
>  brcm,pins.
>  
> diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c 
> b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> index 785c366..0ace548 100644
> --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> @@ -73,6 +73,8 @@
>  enum bcm2835_pinconf_param {
>   /* argument: bcm2835_pinconf_pull */
>   BCM2835_PINCONF_PARAM_PULL,
> + /* argument: bcm2835_pinconf_level */
> + BCM2835_PINCONF_PARAM_LEVEL,
>  };
>  
>  #define BCM2835_PINCONF_PACK(_param_, _arg_) ((_param_) << 16 | (_arg_))
> @@ -725,16 +727,42 @@ static int bcm2835_pctl_dt_node_to_map_pull(struct 
> bcm2835_pinctrl *pc,
>   return 0;
>  }
>  
> +static int bcm2835_pctl_dt_node_to_map_level(struct bcm2835_pinctrl *pc,
> + struct device_node *np, u32 pin, u32 level,
> + struct pinctrl_map **maps)
> +{
> + struct pinctrl_map *map = *maps;
> + unsigned long *configs;
> +
> + if (level > 2) {
> + dev_err(pc->dev, "%pOF: invalid brcm,level %d\n", np, level);
> + return -EINVAL;
> + }
> +
> + configs = kzalloc(sizeof(*configs), GFP_KERNEL);
> + if (!configs)
> + return -ENOMEM;
> + configs[0] = BCM2835_PINCONF_PACK(BCM2835_PINCONF_PARAM_LEVEL, level);
> +
> + map->type = PIN_MAP_TYPE_CONFIGS_PIN;
> + map->data.configs.group_or_pin = bcm2835_gpio_pins[pin].name;
> + map->data.configs.configs = configs;
> + map->data.configs.num_configs = 1;
> + (*maps)++;
> +
> + return 0;
> +}
> +
>  static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev *pctldev,
>   struct device_node *np,
>   struct pinctrl_map **map, unsigned *num_maps)
>  {
>   struct bcm2835_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
> - struct property *pins, *funcs, *pulls;
> - int num_pins, num_funcs, num_pulls, maps_per_pin;
> + struct property *pins, *funcs, *pulls, *levels;
> + int num_pins, num_funcs, num_pulls, num_levels, maps_per_pin;
>   struct pinctrl_map *maps, *cur_map;
>   int i, err;
> - u32 pin, func, pull;
> + u32 pin, func, pull, level;
>  
>   pins = of_find_property(np, "brcm,pins", NULL);
>   if (!pins) {
> @@ -744,6 +772,7 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
> *pctldev,
>  
>   funcs = of_find_property(np, "brcm,function", NULL);
>   pulls = of_find_property(np, "brcm,pull", NULL);
> + levels = of_find_property(np, "brcm,level", NULL);
>  
>   if (!funcs && !pulls) {
>   dev_err(pc->dev,
> @@ -755,6 +784,7 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
> *pctldev,
>   num_pins = pins->length / 4;
>   num_funcs = funcs ? (funcs->length / 4) : 0;
>   num_pulls = pulls ? (pulls->length / 4) : 0;
> + num_levels = levels ? (levels->length / 4) : 0;
>  
>   if (num_funcs > 1 && num_funcs != num_pins) {
>   dev_err(pc->dev,
> @@ -770,11 +800,20 @@ static int bcm2835_pctl_dt_node_to_map(struct 
> pinctrl_dev *pctldev,
>   return -EINVAL;
>   }
>  
> + if (num_levels > 1 && num_levels != num_pins) {
> + dev_err(pc->dev,
> + "%pOF: brcm,level must have 1 or %d entries\n",
> + np, num_pins);
> +  

Re: [PATCH] pinctrl: bc2835: Add brcm,level property

[Stefan Wahren]

Hi Matheus,

sorry for my late reply.

> Matheus Castello  hat am 22. Februar 2018 um 18:44 
> geschrieben:
> 
> 
> Property to set initial value of pin output buffer.
> 
> Signed-off-by: Matheus Castello 
> ---
>  .../bindings/pinctrl/brcm,bcm2835-gpio.txt |  5 +-
>  drivers/pinctrl/bcm/pinctrl-bcm2835.c  | 90 
> +-
>  2 files changed, 75 insertions(+), 20 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt 
> b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
> index 2569866..6834f1d 100644
> --- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt

Changes to the devicetree binding should be a separate patch. Please read [1] 
and use the get_maintainers script for all patches.

[1] - 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/submitting-patches.txt

> @@ -54,8 +54,11 @@ Optional subnode-properties:
>0: none
>1: down
>2: up
> +- brcm,level: Integer, representing the value of output buffer to apply to 
> the pin(s):
> +  0: low
> +  1: high

As Linus suggested please don't introduce new legacy properties. We already 
have generic ones for this. After mention the supported properties, simply make 
a reference to the generic binding.

>  
> -Each of brcm,function and brcm,pull may contain either a single value which
> +Each of brcm,function, brcm,pull and brcm,level may contain either a single 
> value which
>  will be applied to all pins in brcm,pins, or 1 value for each entry in
>  brcm,pins.
>  
> diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c 
> b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> index 785c366..0ace548 100644
> --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> @@ -73,6 +73,8 @@
>  enum bcm2835_pinconf_param {
>   /* argument: bcm2835_pinconf_pull */
>   BCM2835_PINCONF_PARAM_PULL,
> + /* argument: bcm2835_pinconf_level */
> + BCM2835_PINCONF_PARAM_LEVEL,
>  };
>  
>  #define BCM2835_PINCONF_PACK(_param_, _arg_) ((_param_) << 16 | (_arg_))
> @@ -725,16 +727,42 @@ static int bcm2835_pctl_dt_node_to_map_pull(struct 
> bcm2835_pinctrl *pc,
>   return 0;
>  }
>  
> +static int bcm2835_pctl_dt_node_to_map_level(struct bcm2835_pinctrl *pc,
> + struct device_node *np, u32 pin, u32 level,
> + struct pinctrl_map **maps)
> +{
> + struct pinctrl_map *map = *maps;
> + unsigned long *configs;
> +
> + if (level > 2) {
> + dev_err(pc->dev, "%pOF: invalid brcm,level %d\n", np, level);
> + return -EINVAL;
> + }
> +
> + configs = kzalloc(sizeof(*configs), GFP_KERNEL);
> + if (!configs)
> + return -ENOMEM;
> + configs[0] = BCM2835_PINCONF_PACK(BCM2835_PINCONF_PARAM_LEVEL, level);
> +
> + map->type = PIN_MAP_TYPE_CONFIGS_PIN;
> + map->data.configs.group_or_pin = bcm2835_gpio_pins[pin].name;
> + map->data.configs.configs = configs;
> + map->data.configs.num_configs = 1;
> + (*maps)++;
> +
> + return 0;
> +}
> +
>  static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev *pctldev,
>   struct device_node *np,
>   struct pinctrl_map **map, unsigned *num_maps)
>  {
>   struct bcm2835_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
> - struct property *pins, *funcs, *pulls;
> - int num_pins, num_funcs, num_pulls, maps_per_pin;
> + struct property *pins, *funcs, *pulls, *levels;
> + int num_pins, num_funcs, num_pulls, num_levels, maps_per_pin;
>   struct pinctrl_map *maps, *cur_map;
>   int i, err;
> - u32 pin, func, pull;
> + u32 pin, func, pull, level;
>  
>   pins = of_find_property(np, "brcm,pins", NULL);
>   if (!pins) {
> @@ -744,6 +772,7 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
> *pctldev,
>  
>   funcs = of_find_property(np, "brcm,function", NULL);
>   pulls = of_find_property(np, "brcm,pull", NULL);
> + levels = of_find_property(np, "brcm,level", NULL);
>  
>   if (!funcs && !pulls) {
>   dev_err(pc->dev,
> @@ -755,6 +784,7 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
> *pctldev,
>   num_pins = pins->length / 4;
>   num_funcs = funcs ? (funcs->length / 4) : 0;
>   num_pulls = pulls ? (pulls->length / 4) : 0;
> + num_levels = levels ? (levels->length / 4) : 0;
>  
>   if (num_funcs > 1 && num_funcs != num_pins) {
>   dev_err(pc->dev,
> @@ -770,11 +800,20 @@ static int bcm2835_pctl_dt_node_to_map(struct 
> pinctrl_dev *pctldev,
>   return -EINVAL;
>   }
>  
> + if (num_levels > 1 && num_levels != num_pins) {
> + dev_err(pc->dev,
> + "%pOF: brcm,level must have 1 or %d entries\n",
> + np, num_pins);
> + return -EINVAL;
> + }
> +
>   

Re: [PATCH] pinctrl: bc2835: Add brcm,level property

[Linus Walleij]

Hi Matheus,

sorry for top-posting, but new DT properties have to be ACKed
by the device tree maintainers, so quoting it all.

I understand why you use the legacy bcm-specific bindings for this,
but in theory nothing really stops you from just using
"output-high" and "output-low" from the generic properties,
even if you are not using the generic code to handle them but
instead parse them just like in this driver.

The only reason I could think of not to use them would be consistency,
like it would look not so elegant.

But if we look at the pin control bindings from a birds eye view
the big problem with consistency is the proliferation of custom
vendor bindings like this.

So there is a bit of conflict of interest here.

Yours,
Linus Walleij


On Thu, Feb 22, 2018 at 6:44 PM, Matheus Castello
 wrote:
> Property to set initial value of pin output buffer.
>
> Signed-off-by: Matheus Castello 
> ---
>  .../bindings/pinctrl/brcm,bcm2835-gpio.txt |  5 +-
>  drivers/pinctrl/bcm/pinctrl-bcm2835.c  | 90 
> +-
>  2 files changed, 75 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt 
> b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
> index 2569866..6834f1d 100644
> --- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
> @@ -54,8 +54,11 @@ Optional subnode-properties:
>0: none
>1: down
>2: up
> +- brcm,level: Integer, representing the value of output buffer to apply to 
> the pin(s):
> +  0: low
> +  1: high
>
> -Each of brcm,function and brcm,pull may contain either a single value which
> +Each of brcm,function, brcm,pull and brcm,level may contain either a single 
> value which
>  will be applied to all pins in brcm,pins, or 1 value for each entry in
>  brcm,pins.
>
> diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c 
> b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> index 785c366..0ace548 100644
> --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> @@ -73,6 +73,8 @@
>  enum bcm2835_pinconf_param {
> /* argument: bcm2835_pinconf_pull */
> BCM2835_PINCONF_PARAM_PULL,
> +   /* argument: bcm2835_pinconf_level */
> +   BCM2835_PINCONF_PARAM_LEVEL,
>  };
>
>  #define BCM2835_PINCONF_PACK(_param_, _arg_) ((_param_) << 16 | (_arg_))
> @@ -725,16 +727,42 @@ static int bcm2835_pctl_dt_node_to_map_pull(struct 
> bcm2835_pinctrl *pc,
> return 0;
>  }
>
> +static int bcm2835_pctl_dt_node_to_map_level(struct bcm2835_pinctrl *pc,
> +   struct device_node *np, u32 pin, u32 level,
> +   struct pinctrl_map **maps)
> +{
> +   struct pinctrl_map *map = *maps;
> +   unsigned long *configs;
> +
> +   if (level > 2) {
> +   dev_err(pc->dev, "%pOF: invalid brcm,level %d\n", np, level);
> +   return -EINVAL;
> +   }
> +
> +   configs = kzalloc(sizeof(*configs), GFP_KERNEL);
> +   if (!configs)
> +   return -ENOMEM;
> +   configs[0] = BCM2835_PINCONF_PACK(BCM2835_PINCONF_PARAM_LEVEL, level);
> +
> +   map->type = PIN_MAP_TYPE_CONFIGS_PIN;
> +   map->data.configs.group_or_pin = bcm2835_gpio_pins[pin].name;
> +   map->data.configs.configs = configs;
> +   map->data.configs.num_configs = 1;
> +   (*maps)++;
> +
> +   return 0;
> +}
> +
>  static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev *pctldev,
> struct device_node *np,
> struct pinctrl_map **map, unsigned *num_maps)
>  {
> struct bcm2835_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
> -   struct property *pins, *funcs, *pulls;
> -   int num_pins, num_funcs, num_pulls, maps_per_pin;
> +   struct property *pins, *funcs, *pulls, *levels;
> +   int num_pins, num_funcs, num_pulls, num_levels, maps_per_pin;
> struct pinctrl_map *maps, *cur_map;
> int i, err;
> -   u32 pin, func, pull;
> +   u32 pin, func, pull, level;
>
> pins = of_find_property(np, "brcm,pins", NULL);
> if (!pins) {
> @@ -744,6 +772,7 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
> *pctldev,
>
> funcs = of_find_property(np, "brcm,function", NULL);
> pulls = of_find_property(np, "brcm,pull", NULL);
> +   levels = of_find_property(np, "brcm,level", NULL);
>
> if (!funcs && !pulls) {
> dev_err(pc->dev,
> @@ -755,6 +784,7 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
> *pctldev,
> num_pins = pins->length / 4;
> num_funcs = funcs ? (funcs->length / 4) : 0;
> num_pulls = pulls ? (pulls->length / 4) : 0;
> +   num_levels = levels ? (levels->length / 4) : 0;
>
> if (num_funcs > 1 && num_funcs != num_pins) {
> dev_err(pc->dev,
> @@ -770,11 +800,20 @@ static int 

Re: [PATCH] pinctrl: bc2835: Add brcm,level property

[Linus Walleij]

Hi Matheus,

sorry for top-posting, but new DT properties have to be ACKed
by the device tree maintainers, so quoting it all.

I understand why you use the legacy bcm-specific bindings for this,
but in theory nothing really stops you from just using
"output-high" and "output-low" from the generic properties,
even if you are not using the generic code to handle them but
instead parse them just like in this driver.

The only reason I could think of not to use them would be consistency,
like it would look not so elegant.

But if we look at the pin control bindings from a birds eye view
the big problem with consistency is the proliferation of custom
vendor bindings like this.

So there is a bit of conflict of interest here.

Yours,
Linus Walleij


On Thu, Feb 22, 2018 at 6:44 PM, Matheus Castello
 wrote:
> Property to set initial value of pin output buffer.
>
> Signed-off-by: Matheus Castello 
> ---
>  .../bindings/pinctrl/brcm,bcm2835-gpio.txt |  5 +-
>  drivers/pinctrl/bcm/pinctrl-bcm2835.c  | 90 
> +-
>  2 files changed, 75 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt 
> b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
> index 2569866..6834f1d 100644
> --- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
> @@ -54,8 +54,11 @@ Optional subnode-properties:
>0: none
>1: down
>2: up
> +- brcm,level: Integer, representing the value of output buffer to apply to 
> the pin(s):
> +  0: low
> +  1: high
>
> -Each of brcm,function and brcm,pull may contain either a single value which
> +Each of brcm,function, brcm,pull and brcm,level may contain either a single 
> value which
>  will be applied to all pins in brcm,pins, or 1 value for each entry in
>  brcm,pins.
>
> diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c 
> b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> index 785c366..0ace548 100644
> --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> @@ -73,6 +73,8 @@
>  enum bcm2835_pinconf_param {
> /* argument: bcm2835_pinconf_pull */
> BCM2835_PINCONF_PARAM_PULL,
> +   /* argument: bcm2835_pinconf_level */
> +   BCM2835_PINCONF_PARAM_LEVEL,
>  };
>
>  #define BCM2835_PINCONF_PACK(_param_, _arg_) ((_param_) << 16 | (_arg_))
> @@ -725,16 +727,42 @@ static int bcm2835_pctl_dt_node_to_map_pull(struct 
> bcm2835_pinctrl *pc,
> return 0;
>  }
>
> +static int bcm2835_pctl_dt_node_to_map_level(struct bcm2835_pinctrl *pc,
> +   struct device_node *np, u32 pin, u32 level,
> +   struct pinctrl_map **maps)
> +{
> +   struct pinctrl_map *map = *maps;
> +   unsigned long *configs;
> +
> +   if (level > 2) {
> +   dev_err(pc->dev, "%pOF: invalid brcm,level %d\n", np, level);
> +   return -EINVAL;
> +   }
> +
> +   configs = kzalloc(sizeof(*configs), GFP_KERNEL);
> +   if (!configs)
> +   return -ENOMEM;
> +   configs[0] = BCM2835_PINCONF_PACK(BCM2835_PINCONF_PARAM_LEVEL, level);
> +
> +   map->type = PIN_MAP_TYPE_CONFIGS_PIN;
> +   map->data.configs.group_or_pin = bcm2835_gpio_pins[pin].name;
> +   map->data.configs.configs = configs;
> +   map->data.configs.num_configs = 1;
> +   (*maps)++;
> +
> +   return 0;
> +}
> +
>  static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev *pctldev,
> struct device_node *np,
> struct pinctrl_map **map, unsigned *num_maps)
>  {
> struct bcm2835_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
> -   struct property *pins, *funcs, *pulls;
> -   int num_pins, num_funcs, num_pulls, maps_per_pin;
> +   struct property *pins, *funcs, *pulls, *levels;
> +   int num_pins, num_funcs, num_pulls, num_levels, maps_per_pin;
> struct pinctrl_map *maps, *cur_map;
> int i, err;
> -   u32 pin, func, pull;
> +   u32 pin, func, pull, level;
>
> pins = of_find_property(np, "brcm,pins", NULL);
> if (!pins) {
> @@ -744,6 +772,7 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
> *pctldev,
>
> funcs = of_find_property(np, "brcm,function", NULL);
> pulls = of_find_property(np, "brcm,pull", NULL);
> +   levels = of_find_property(np, "brcm,level", NULL);
>
> if (!funcs && !pulls) {
> dev_err(pc->dev,
> @@ -755,6 +784,7 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
> *pctldev,
> num_pins = pins->length / 4;
> num_funcs = funcs ? (funcs->length / 4) : 0;
> num_pulls = pulls ? (pulls->length / 4) : 0;
> +   num_levels = levels ? (levels->length / 4) : 0;
>
> if (num_funcs > 1 && num_funcs != num_pins) {
> dev_err(pc->dev,
> @@ -770,11 +800,20 @@ static int bcm2835_pctl_dt_node_to_map(struct 
> pinctrl_dev 

Re: [PATCH] pinctrl: bc2835: Add brcm,level property

[Matheus Castello]

Property 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.

This modification maintain the legacy property style, but the idea is to 
continue working in this driver to implement support for the generic binding 
properties too. So in the future this can be used to implement the 
output-high and output-low generic properties.

Signed-off-by: Matheus Castello 
---
 .../bindings/pinctrl/brcm,bcm2835-gpio.txt |  5 +-
 drivers/pinctrl/bcm/pinctrl-bcm2835.c  | 90 +-
 2 files changed, 75 insertions(+), 20 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt 
b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
index 2569866..6834f1d 100644
--- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
+++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
@@ -54,8 +54,11 @@ Optional subnode-properties:
   0: none
   1: down
   2: up
+- brcm,level: Integer, representing the value of output buffer to apply to the 
pin(s):
+  0: low
+  1: high
 
-Each of brcm,function and brcm,pull may contain either a single value which
+Each of brcm,function, brcm,pull and brcm,level may contain either a single 
value which
 will be applied to all pins in brcm,pins, or 1 value for each entry in
 brcm,pins.
 
diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c 
b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index 785c366..0ace548 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -73,6 +73,8 @@
 enum bcm2835_pinconf_param {
/* argument: bcm2835_pinconf_pull */
BCM2835_PINCONF_PARAM_PULL,
+   /* argument: bcm2835_pinconf_level */
+   BCM2835_PINCONF_PARAM_LEVEL,
 };
 
 #define BCM2835_PINCONF_PACK(_param_, _arg_) ((_param_) << 16 | (_arg_))
@@ -725,16 +727,42 @@ static int bcm2835_pctl_dt_node_to_map_pull(struct 
bcm2835_pinctrl *pc,
return 0;
 }
 
+static int bcm2835_pctl_dt_node_to_map_level(struct bcm2835_pinctrl *pc,
+   struct device_node *np, u32 pin, u32 level,
+   struct pinctrl_map **maps)
+{
+   struct pinctrl_map *map = *maps;
+   unsigned long *configs;
+
+   if (level > 2) {
+   dev_err(pc->dev, "%pOF: invalid brcm,level %d\n", np, level);
+   return -EINVAL;
+   }
+
+   configs = kzalloc(sizeof(*configs), GFP_KERNEL);
+   if (!configs)
+   return -ENOMEM;
+   configs[0] = BCM2835_PINCONF_PACK(BCM2835_PINCONF_PARAM_LEVEL, level);
+
+   map->type = PIN_MAP_TYPE_CONFIGS_PIN;
+   map->data.configs.group_or_pin = bcm2835_gpio_pins[pin].name;
+   map->data.configs.configs = configs;
+   map->data.configs.num_configs = 1;
+   (*maps)++;
+
+   return 0;
+}
+
 static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev *pctldev,
struct device_node *np,
struct pinctrl_map **map, unsigned *num_maps)
 {
struct bcm2835_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
-   struct property *pins, *funcs, *pulls;
-   int num_pins, num_funcs, num_pulls, maps_per_pin;
+   struct property *pins, *funcs, *pulls, *levels;
+   int num_pins, num_funcs, num_pulls, num_levels, maps_per_pin;
struct pinctrl_map *maps, *cur_map;
int i, err;
-   u32 pin, func, pull;
+   u32 pin, func, pull, level;
 
pins = of_find_property(np, "brcm,pins", NULL);
if (!pins) {
@@ -744,6 +772,7 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
*pctldev,
 
funcs = of_find_property(np, "brcm,function", NULL);
pulls = of_find_property(np, "brcm,pull", NULL);
+   levels = of_find_property(np, "brcm,level", NULL);
 
if (!funcs && !pulls) {
dev_err(pc->dev,
@@ -755,6 +784,7 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
*pctldev,
num_pins = pins->length / 4;
num_funcs = funcs ? (funcs->length / 4) : 0;
num_pulls = pulls ? (pulls->length / 4) : 0;
+   num_levels = levels ? (levels->length / 4) : 0;
 
if (num_funcs > 1 && num_funcs != num_pins) {
dev_err(pc->dev,
@@ -770,11 +800,20 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
*pctldev,
return -EINVAL;
}
 
+   if (num_levels > 1 && num_levels != num_pins) {
+   dev_err(pc->dev,
+   "%pOF: brcm,level must have 1 or %d entries\n",
+   np, num_pins);
+   return -EINVAL;
+   }
+
maps_per_pin = 0;
if (num_funcs)
maps_per_pin++;
if (num_pulls)
maps_per_pin++;
+   if (num_levels)
+   maps_per_pin++;
cur_map = maps = kzalloc(num_pins * maps_per_pin * sizeof(*maps),

Re: [PATCH] pinctrl: bc2835: Add brcm,level property

[Matheus Castello]

Property 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.

This modification maintain the legacy property style, but the idea is to 
continue working in this driver to implement support for the generic binding 
properties too. So in the future this can be used to implement the 
output-high and output-low generic properties.

Signed-off-by: Matheus Castello 
---
 .../bindings/pinctrl/brcm,bcm2835-gpio.txt |  5 +-
 drivers/pinctrl/bcm/pinctrl-bcm2835.c  | 90 +-
 2 files changed, 75 insertions(+), 20 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt 
b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
index 2569866..6834f1d 100644
--- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
+++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
@@ -54,8 +54,11 @@ Optional subnode-properties:
   0: none
   1: down
   2: up
+- brcm,level: Integer, representing the value of output buffer to apply to the 
pin(s):
+  0: low
+  1: high
 
-Each of brcm,function and brcm,pull may contain either a single value which
+Each of brcm,function, brcm,pull and brcm,level may contain either a single 
value which
 will be applied to all pins in brcm,pins, or 1 value for each entry in
 brcm,pins.
 
diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c 
b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index 785c366..0ace548 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -73,6 +73,8 @@
 enum bcm2835_pinconf_param {
/* argument: bcm2835_pinconf_pull */
BCM2835_PINCONF_PARAM_PULL,
+   /* argument: bcm2835_pinconf_level */
+   BCM2835_PINCONF_PARAM_LEVEL,
 };
 
 #define BCM2835_PINCONF_PACK(_param_, _arg_) ((_param_) << 16 | (_arg_))
@@ -725,16 +727,42 @@ static int bcm2835_pctl_dt_node_to_map_pull(struct 
bcm2835_pinctrl *pc,
return 0;
 }
 
+static int bcm2835_pctl_dt_node_to_map_level(struct bcm2835_pinctrl *pc,
+   struct device_node *np, u32 pin, u32 level,
+   struct pinctrl_map **maps)
+{
+   struct pinctrl_map *map = *maps;
+   unsigned long *configs;
+
+   if (level > 2) {
+   dev_err(pc->dev, "%pOF: invalid brcm,level %d\n", np, level);
+   return -EINVAL;
+   }
+
+   configs = kzalloc(sizeof(*configs), GFP_KERNEL);
+   if (!configs)
+   return -ENOMEM;
+   configs[0] = BCM2835_PINCONF_PACK(BCM2835_PINCONF_PARAM_LEVEL, level);
+
+   map->type = PIN_MAP_TYPE_CONFIGS_PIN;
+   map->data.configs.group_or_pin = bcm2835_gpio_pins[pin].name;
+   map->data.configs.configs = configs;
+   map->data.configs.num_configs = 1;
+   (*maps)++;
+
+   return 0;
+}
+
 static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev *pctldev,
struct device_node *np,
struct pinctrl_map **map, unsigned *num_maps)
 {
struct bcm2835_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
-   struct property *pins, *funcs, *pulls;
-   int num_pins, num_funcs, num_pulls, maps_per_pin;
+   struct property *pins, *funcs, *pulls, *levels;
+   int num_pins, num_funcs, num_pulls, num_levels, maps_per_pin;
struct pinctrl_map *maps, *cur_map;
int i, err;
-   u32 pin, func, pull;
+   u32 pin, func, pull, level;
 
pins = of_find_property(np, "brcm,pins", NULL);
if (!pins) {
@@ -744,6 +772,7 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
*pctldev,
 
funcs = of_find_property(np, "brcm,function", NULL);
pulls = of_find_property(np, "brcm,pull", NULL);
+   levels = of_find_property(np, "brcm,level", NULL);
 
if (!funcs && !pulls) {
dev_err(pc->dev,
@@ -755,6 +784,7 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
*pctldev,
num_pins = pins->length / 4;
num_funcs = funcs ? (funcs->length / 4) : 0;
num_pulls = pulls ? (pulls->length / 4) : 0;
+   num_levels = levels ? (levels->length / 4) : 0;
 
if (num_funcs > 1 && num_funcs != num_pins) {
dev_err(pc->dev,
@@ -770,11 +800,20 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
*pctldev,
return -EINVAL;
}
 
+   if (num_levels > 1 && num_levels != num_pins) {
+   dev_err(pc->dev,
+   "%pOF: brcm,level must have 1 or %d entries\n",
+   np, num_pins);
+   return -EINVAL;
+   }
+
maps_per_pin = 0;
if (num_funcs)
maps_per_pin++;
if (num_pulls)
maps_per_pin++;
+   if (num_levels)
+   maps_per_pin++;
cur_map = maps = kzalloc(num_pins * maps_per_pin * sizeof(*maps),
GFP_KERNEL);
if (!maps)

Re: [PATCH] pinctrl: bc2835: Add brcm,level property

[Eric Anholt]

Matheus Castello  writes:

> Property to set initial value of pin output buffer.

In the commit message, could you expand on the motivation for adding
this support?  Also, couldn't we reuse some existing
pinctrl-bindings.txt generic property?


signature.asc
Description: PGP signature

Re: [PATCH] pinctrl: bc2835: Add brcm,level property

[Eric Anholt]

Matheus Castello  writes:

> Property to set initial value of pin output buffer.

In the commit message, could you expand on the motivation for adding
this support?  Also, couldn't we reuse some existing
pinctrl-bindings.txt generic property?


signature.asc
Description: PGP signature