All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Additional iProc GPIO support
@ 2016-05-02 20:51 Ray Jui
  2016-05-02 20:51 ` [PATCH v2 1/2] dt-bindings: Update iProc GPIO bindings Ray Jui
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Ray Jui @ 2016-05-02 20:51 UTC (permalink / raw)
  To: Linus Walleij, Alexandre Courbot, Rob Herring
  Cc: linux-kernel, linux-gpio, bcm-kernel-feedback-list, devicetree, Ray Jui

Add support to the iProc GPIO driver for v2 and v3 revisions of the iProc GPIO
controllers. The v2 revision is used on NSP with drive strength pinconf
feature disabled and the v3 revision is used on Stingray with all pinconf
features disabled

Changes from v1:
 - Changed to use compatible strings "brcm,iproc-gpio-v2" and
"brcm,iproc-gpio-v3" to deal with differences among various iProc based SoCs
 - Removed DT header "include/dt-bindings/pinctrl/brcm,iproc-gpio.h"

Patch series is developed based on Linux v4.6-rc1 and available at:
https://github.com/Broadcom/cygnus-linux/tree/iproc-gpio-v2

Ray Jui (2):
  dt-bindings: Update iProc GPIO bindings
  pinctrl: iproc: Add v2/v3 GPIO support

 .../bindings/pinctrl/brcm,iproc-gpio.txt           |  11 +-
 drivers/pinctrl/bcm/pinctrl-iproc-gpio.c           | 115 +++++++++++++++++++--
 2 files changed, 119 insertions(+), 7 deletions(-)

-- 
2.1.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/2] dt-bindings: Update iProc GPIO bindings
  2016-05-02 20:51 [PATCH v2 0/2] Additional iProc GPIO support Ray Jui
@ 2016-05-02 20:51 ` Ray Jui
  2016-05-04 13:20   ` Rob Herring
  2016-05-02 20:51 ` [PATCH v2 2/2] pinctrl: iproc: Add v2/v3 GPIO support Ray Jui
  2016-05-11  9:29 ` [PATCH v2 0/2] Additional iProc " Linus Walleij
  2 siblings, 1 reply; 8+ messages in thread
From: Ray Jui @ 2016-05-02 20:51 UTC (permalink / raw)
  To: Linus Walleij, Alexandre Courbot, Rob Herring
  Cc: linux-kernel, linux-gpio, bcm-kernel-feedback-list, devicetree, Ray Jui

Update the iProc GPIO binding document to add new compatible strings
"brcm,iproc-gpio-v2" and "brcm,iproc-gpio-v3" for the 2nd and 3rd
generation of the iProc GPIO controllers

Signed-off-by: Ray Jui <ray.jui@broadcom.com>
---
 Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt b/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt
index e427792..3a56649 100644
--- a/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt
+++ b/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt
@@ -4,7 +4,16 @@ Required properties:
 
 - compatible:
     Must be "brcm,cygnus-ccm-gpio", "brcm,cygnus-asiu-gpio",
-    "brcm,cygnus-crmu-gpio" or "brcm,iproc-gpio"
+    or "brcm,cygnus-crmu-gpio" for Cygnus SoCs
+
+    "brcm,iproc-gpio" for the first generation of the GPIO controller that
+    supports full-featured pinctrl and GPIO functions used in iProc based SoCs
+
+    "brcm,iproc-gpio-v2" for the second generation of the GPIO controller that
+    has the drive strength pinctrl support disabled, e.g., in the iProc NSP SoC
+
+    "brcm,iproc-gpio-v3" for the third generation of the GPIO controller that
+    has the general pinctrl support completely disabled
 
 - reg:
     Define the base and range of the I/O address space that contains SoC
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] pinctrl: iproc: Add v2/v3 GPIO support
  2016-05-02 20:51 [PATCH v2 0/2] Additional iProc GPIO support Ray Jui
  2016-05-02 20:51 ` [PATCH v2 1/2] dt-bindings: Update iProc GPIO bindings Ray Jui
@ 2016-05-02 20:51 ` Ray Jui
  2016-05-11  9:29 ` [PATCH v2 0/2] Additional iProc " Linus Walleij
  2 siblings, 0 replies; 8+ messages in thread
From: Ray Jui @ 2016-05-02 20:51 UTC (permalink / raw)
  To: Linus Walleij, Alexandre Courbot, Rob Herring
  Cc: linux-kernel, linux-gpio, bcm-kernel-feedback-list, devicetree,
	Ray Jui, Pramod Kumar

The iProc GPIO controller is shared among multiple iProc based SoCs.
In the 2nd generation of the controller integrated in NSP, the drive
strength pinctrl function is disabled. In the 3rd generation of this
controller integrated in Stingray, pinctrl is handled by another block
and this GPIO controller is solely used as a GPIO controller, and
therefore should not be registered to the pinconf framework

Introducing new compatible strings "brcm,iproc-gpio-v2" for NSP with
drive strength feature disabled and "brcm,iproc-gpio-v3" for stingray
with all PINCONF features disabled

This patch is developed based on the initial work from Yendapally
Reddy Dhananjaya <yrdreddy@broadcom.com> who attempted to disable drive
strength configuration for the iProc based NSP chip. In addition,
Pramod Kumar <pramodku@broadcom.com> also contributed to make the
support more generic across all currently supported PINCONF
functions in the iProc GPIO/PINCONF driver

Signed-off-by: Pramod Kumar <pramodku@broadcom.com>
Signed-off-by: Ray Jui <ray.jui@broadcom.com>
---
 drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 115 +++++++++++++++++++++++++++++--
 1 file changed, 109 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
index d530ab4..98f2400 100644
--- a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
+++ b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
@@ -66,6 +66,14 @@
 #define GPIO_DRV_STRENGTH_BITS       3
 #define GPIO_DRV_STRENGTH_BIT_MASK   ((1 << GPIO_DRV_STRENGTH_BITS) - 1)
 
+enum iproc_pinconf_param {
+	IPROC_PINCONF_DRIVE_STRENGTH = 0,
+	IPROC_PINCONF_BIAS_DISABLE,
+	IPROC_PINCONF_BIAS_PULL_UP,
+	IPROC_PINCONF_BIAS_PULL_DOWN,
+	IPROC_PINCON_MAX,
+};
+
 /*
  * Iproc GPIO core
  *
@@ -78,6 +86,10 @@
  * @num_banks: number of GPIO banks, each bank supports up to 32 GPIOs
  * @pinmux_is_supported: flag to indicate this GPIO controller contains pins
  * that can be individually muxed to GPIO
+ * @pinconf_disable: contains a list of PINCONF parameters that need to be
+ * disabled
+ * @nr_pinconf_disable: total number of PINCONF parameters that need to be
+ * disabled
  * @pctl: pointer to pinctrl_dev
  * @pctldesc: pinctrl descriptor
  */
@@ -94,6 +106,9 @@ struct iproc_gpio {
 
 	bool pinmux_is_supported;
 
+	enum pin_config_param *pinconf_disable;
+	unsigned int nr_pinconf_disable;
+
 	struct pinctrl_dev *pctl;
 	struct pinctrl_desc pctldesc;
 };
@@ -360,6 +375,65 @@ static int iproc_gpio_get(struct gpio_chip *gc, unsigned gpio)
 	return !!(readl(chip->base + offset) & BIT(shift));
 }
 
+/*
+ * Mapping of the iProc PINCONF parameters to the generic pin configuration
+ * parameters
+ */
+static const enum pin_config_param iproc_pinconf_disable_map[] = {
+	[IPROC_PINCONF_DRIVE_STRENGTH] = PIN_CONFIG_DRIVE_STRENGTH,
+	[IPROC_PINCONF_BIAS_DISABLE] = PIN_CONFIG_BIAS_DISABLE,
+	[IPROC_PINCONF_BIAS_PULL_UP] = PIN_CONFIG_BIAS_PULL_UP,
+	[IPROC_PINCONF_BIAS_PULL_DOWN] = PIN_CONFIG_BIAS_PULL_DOWN,
+};
+
+static bool iproc_pinconf_param_is_disabled(struct iproc_gpio *chip,
+					    enum pin_config_param param)
+{
+	unsigned int i;
+
+	if (!chip->nr_pinconf_disable)
+		return false;
+
+	for (i = 0; i < chip->nr_pinconf_disable; i++)
+		if (chip->pinconf_disable[i] == param)
+			return true;
+
+	return false;
+}
+
+static int iproc_pinconf_disable_map_create(struct iproc_gpio *chip,
+					    unsigned long disable_mask)
+{
+	unsigned int map_size = ARRAY_SIZE(iproc_pinconf_disable_map);
+	unsigned int bit, nbits = 0;
+
+	/* figure out total number of PINCONF parameters to disable */
+	for_each_set_bit(bit, &disable_mask, map_size)
+		nbits++;
+
+	if (!nbits)
+		return 0;
+
+	/*
+	 * Allocate an array to store PINCONF parameters that need to be
+	 * disabled
+	 */
+	chip->pinconf_disable = devm_kcalloc(chip->dev, nbits,
+					     sizeof(*chip->pinconf_disable),
+					     GFP_KERNEL);
+	if (!chip->pinconf_disable)
+		return -ENOMEM;
+
+	chip->nr_pinconf_disable = nbits;
+
+	/* now store these parameters */
+	nbits = 0;
+	for_each_set_bit(bit, &disable_mask, map_size)
+		chip->pinconf_disable[nbits++] = iproc_pinconf_disable_map[bit];
+
+	return 0;
+}
+
 static int iproc_get_groups_count(struct pinctrl_dev *pctldev)
 {
 	return 1;
@@ -500,6 +574,9 @@ static int iproc_pin_config_get(struct pinctrl_dev *pctldev, unsigned pin,
 	bool disable, pull_up;
 	int ret;
 
+	if (iproc_pinconf_param_is_disabled(chip, param))
+		return -ENOTSUPP;
+
 	switch (param) {
 	case PIN_CONFIG_BIAS_DISABLE:
 		iproc_gpio_get_pull(chip, gpio, &disable, &pull_up);
@@ -548,6 +625,10 @@ static int iproc_pin_config_set(struct pinctrl_dev *pctldev, unsigned pin,
 
 	for (i = 0; i < num_configs; i++) {
 		param = pinconf_to_config_param(configs[i]);
+
+		if (iproc_pinconf_param_is_disabled(chip, param))
+			return -ENOTSUPP;
+
 		arg = pinconf_to_config_argument(configs[i]);
 
 		switch (param) {
@@ -642,7 +723,9 @@ static const struct of_device_id iproc_gpio_of_match[] = {
 	{ .compatible = "brcm,cygnus-asiu-gpio" },
 	{ .compatible = "brcm,cygnus-crmu-gpio" },
 	{ .compatible = "brcm,iproc-gpio" },
-	{ }
+	{ .compatible = "brcm,iproc-gpio-v2" },
+	{ .compatible = "brcm,iproc-gpio-v3" },
+	{ /* sentinel */ }
 };
 
 static int iproc_gpio_probe(struct platform_device *pdev)
@@ -651,8 +734,16 @@ static int iproc_gpio_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct iproc_gpio *chip;
 	struct gpio_chip *gc;
-	u32 ngpios;
+	u32 ngpios, pinconf_disable_mask = 0;
 	int irq, ret;
+	bool no_pinconf = false;
+
+	/* v2 does not support drive strength config */
+	if (of_device_is_compatible(dev->of_node, "brcm,iproc-gpio-v2"))
+		pinconf_disable_mask = BIT(IPROC_PINCONF_DRIVE_STRENGTH);
+	/* v3 does not support pinconf */
+	else if (of_device_is_compatible(dev->of_node, "brcm,iproc-gpio-v3"))
+		no_pinconf = true;
 
 	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
 	if (!chip)
@@ -707,10 +798,22 @@ static int iproc_gpio_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = iproc_gpio_register_pinconf(chip);
-	if (ret) {
-		dev_err(dev, "unable to register pinconf\n");
-		goto err_rm_gpiochip;
+	if (!no_pinconf) {
+		ret = iproc_gpio_register_pinconf(chip);
+		if (ret) {
+			dev_err(dev, "unable to register pinconf\n");
+			goto err_rm_gpiochip;
+		}
+
+		if (pinconf_disable_mask) {
+			ret = iproc_pinconf_disable_map_create(chip,
+							 pinconf_disable_mask);
+			if (ret) {
+				dev_err(dev,
+					"unable to create pinconf disable map\n");
+				goto err_unregister_pinconf;
+			}
+		}
 	}
 
 	/* optional GPIO interrupt support */
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: Update iProc GPIO bindings
  2016-05-02 20:51 ` [PATCH v2 1/2] dt-bindings: Update iProc GPIO bindings Ray Jui
@ 2016-05-04 13:20   ` Rob Herring
  2016-05-04 16:29     ` Ray Jui
  0 siblings, 1 reply; 8+ messages in thread
From: Rob Herring @ 2016-05-04 13:20 UTC (permalink / raw)
  To: Ray Jui
  Cc: Linus Walleij, Alexandre Courbot, linux-kernel, linux-gpio,
	bcm-kernel-feedback-list, devicetree

On Mon, May 02, 2016 at 01:51:47PM -0700, Ray Jui wrote:
> Update the iProc GPIO binding document to add new compatible strings
> "brcm,iproc-gpio-v2" and "brcm,iproc-gpio-v3" for the 2nd and 3rd
> generation of the iProc GPIO controllers
> 
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> ---
>  Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt b/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt
> index e427792..3a56649 100644
> --- a/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt
> @@ -4,7 +4,16 @@ Required properties:
>  
>  - compatible:
>      Must be "brcm,cygnus-ccm-gpio", "brcm,cygnus-asiu-gpio",
> -    "brcm,cygnus-crmu-gpio" or "brcm,iproc-gpio"
> +    or "brcm,cygnus-crmu-gpio" for Cygnus SoCs
> +
> +    "brcm,iproc-gpio" for the first generation of the GPIO controller that
> +    supports full-featured pinctrl and GPIO functions used in iProc based SoCs
> +
> +    "brcm,iproc-gpio-v2" for the second generation of the GPIO controller that
> +    has the drive strength pinctrl support disabled, e.g., in the iProc NSP SoC
> +
> +    "brcm,iproc-gpio-v3" for the third generation of the GPIO controller that
> +    has the general pinctrl support completely disabled

You can have these for driver matching, but you still need SoC specific 
compatible strings.

Rob

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: Update iProc GPIO bindings
  2016-05-04 13:20   ` Rob Herring
@ 2016-05-04 16:29     ` Ray Jui
  2016-05-11 14:06       ` Rob Herring
  0 siblings, 1 reply; 8+ messages in thread
From: Ray Jui @ 2016-05-04 16:29 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linus Walleij, Alexandre Courbot, linux-kernel, linux-gpio,
	bcm-kernel-feedback-list, devicetree

Hi Rob,

On 5/4/2016 6:20 AM, Rob Herring wrote:
> On Mon, May 02, 2016 at 01:51:47PM -0700, Ray Jui wrote:
>> Update the iProc GPIO binding document to add new compatible strings
>> "brcm,iproc-gpio-v2" and "brcm,iproc-gpio-v3" for the 2nd and 3rd
>> generation of the iProc GPIO controllers
>>
>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>> ---
>>  Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt | 11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt b/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt
>> index e427792..3a56649 100644
>> --- a/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt
>> +++ b/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt
>> @@ -4,7 +4,16 @@ Required properties:
>>
>>  - compatible:
>>      Must be "brcm,cygnus-ccm-gpio", "brcm,cygnus-asiu-gpio",
>> -    "brcm,cygnus-crmu-gpio" or "brcm,iproc-gpio"
>> +    or "brcm,cygnus-crmu-gpio" for Cygnus SoCs
>> +
>> +    "brcm,iproc-gpio" for the first generation of the GPIO controller that
>> +    supports full-featured pinctrl and GPIO functions used in iProc based SoCs
>> +
>> +    "brcm,iproc-gpio-v2" for the second generation of the GPIO controller that
>> +    has the drive strength pinctrl support disabled, e.g., in the iProc NSP SoC
>> +
>> +    "brcm,iproc-gpio-v3" for the third generation of the GPIO controller that
>> +    has the general pinctrl support completely disabled
>
> You can have these for driver matching, but you still need SoC specific
> compatible strings.
>
> Rob
>

I think I'm missing something and hope you can help to clarify here. It 
looks like the notion of v2, v3 should only be used if there's an indeed 
an revision update on the controller IP itself, correct?

In our case, if the same revision of GPIO controller is used but instead 
synthesized differently with different SoCs, then you are suggesting 
here that it should be dealt with SoC specific compatible string, correct?

For example, for the GPIO controller on NSP where drive strength is 
disabled, the compatible string in DT should look like this?

compatible = "brcm,iproc-gpio", "brcm,iproc-gpio-nsp";

For the GPIO controller on Stingray where pinconf is completely 
disabled, the compatible string in DT should look like:

compatible = "brcm,iproc-gpio", "brcm,iproc-gpio-stingray";

Is that correct?

Thanks,

Ray

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/2] Additional iProc GPIO support
  2016-05-02 20:51 [PATCH v2 0/2] Additional iProc GPIO support Ray Jui
  2016-05-02 20:51 ` [PATCH v2 1/2] dt-bindings: Update iProc GPIO bindings Ray Jui
  2016-05-02 20:51 ` [PATCH v2 2/2] pinctrl: iproc: Add v2/v3 GPIO support Ray Jui
@ 2016-05-11  9:29 ` Linus Walleij
  2 siblings, 0 replies; 8+ messages in thread
From: Linus Walleij @ 2016-05-11  9:29 UTC (permalink / raw)
  To: Ray Jui
  Cc: Alexandre Courbot, Rob Herring, linux-kernel, linux-gpio,
	bcm-kernel-feedback-list, devicetree

On Mon, May 2, 2016 at 10:51 PM, Ray Jui <ray.jui@broadcom.com> wrote:

> Add support to the iProc GPIO driver for v2 and v3 revisions of the iProc GPIO
> controllers. The v2 revision is used on NSP with drive strength pinconf
> feature disabled and the v3 revision is used on Stingray with all pinconf
> features disabled
>
> Changes from v1:
>  - Changed to use compatible strings "brcm,iproc-gpio-v2" and
> "brcm,iproc-gpio-v3" to deal with differences among various iProc based SoCs
>  - Removed DT header "include/dt-bindings/pinctrl/brcm,iproc-gpio.h"
>
> Patch series is developed based on Linux v4.6-rc1 and available at:
> https://github.com/Broadcom/cygnus-linux/tree/iproc-gpio-v2

I'm waiting for the discussion about the compatible strings
to resolve.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: Update iProc GPIO bindings
  2016-05-04 16:29     ` Ray Jui
@ 2016-05-11 14:06       ` Rob Herring
  2016-05-11 15:50         ` Ray Jui
  0 siblings, 1 reply; 8+ messages in thread
From: Rob Herring @ 2016-05-11 14:06 UTC (permalink / raw)
  To: Ray Jui
  Cc: Linus Walleij, Alexandre Courbot, linux-kernel, linux-gpio,
	bcm-kernel-feedback-list, devicetree

On Wed, May 04, 2016 at 09:29:04AM -0700, Ray Jui wrote:
> Hi Rob,
> 
> On 5/4/2016 6:20 AM, Rob Herring wrote:
> >On Mon, May 02, 2016 at 01:51:47PM -0700, Ray Jui wrote:
> >>Update the iProc GPIO binding document to add new compatible strings
> >>"brcm,iproc-gpio-v2" and "brcm,iproc-gpio-v3" for the 2nd and 3rd
> >>generation of the iProc GPIO controllers
> >>
> >>Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> >>---
> >> Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt | 11 ++++++++++-
> >> 1 file changed, 10 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt b/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt
> >>index e427792..3a56649 100644
> >>--- a/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt
> >>+++ b/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt
> >>@@ -4,7 +4,16 @@ Required properties:
> >>
> >> - compatible:
> >>     Must be "brcm,cygnus-ccm-gpio", "brcm,cygnus-asiu-gpio",
> >>-    "brcm,cygnus-crmu-gpio" or "brcm,iproc-gpio"
> >>+    or "brcm,cygnus-crmu-gpio" for Cygnus SoCs
> >>+
> >>+    "brcm,iproc-gpio" for the first generation of the GPIO controller that
> >>+    supports full-featured pinctrl and GPIO functions used in iProc based SoCs
> >>+
> >>+    "brcm,iproc-gpio-v2" for the second generation of the GPIO controller that
> >>+    has the drive strength pinctrl support disabled, e.g., in the iProc NSP SoC
> >>+
> >>+    "brcm,iproc-gpio-v3" for the third generation of the GPIO controller that
> >>+    has the general pinctrl support completely disabled
> >
> >You can have these for driver matching, but you still need SoC specific
> >compatible strings.
> >
> >Rob
> >
> 
> I think I'm missing something and hope you can help to clarify here. It
> looks like the notion of v2, v3 should only be used if there's an indeed an
> revision update on the controller IP itself, correct?

Yes, but only if v2, v3 is actually a meaningful number rather than 
something made up. You have to have a well defined process of IP 
revisions which I have not seen during my time in chip companies, so I'm 
generally suspicious of version numbers. The exception is FPGA IP 
blocks.

> In our case, if the same revision of GPIO controller is used but instead
> synthesized differently with different SoCs, then you are suggesting here
> that it should be dealt with SoC specific compatible string, correct?

Yes, that and/or integration differences is why you need SoC specific 
compatible strings. A block could be "identical" but have different max 
frequency for example.
 
> For example, for the GPIO controller on NSP where drive strength is
> disabled, the compatible string in DT should look like this?
> 
> compatible = "brcm,iproc-gpio", "brcm,iproc-gpio-nsp";

Yes, but reverse the order. Most specific to least specific.

> 
> For the GPIO controller on Stingray where pinconf is completely disabled,
> the compatible string in DT should look like:
> 
> compatible = "brcm,iproc-gpio", "brcm,iproc-gpio-stingray";
> 
> Is that correct?
> 
> Thanks,
> 
> Ray

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: Update iProc GPIO bindings
  2016-05-11 14:06       ` Rob Herring
@ 2016-05-11 15:50         ` Ray Jui
  0 siblings, 0 replies; 8+ messages in thread
From: Ray Jui @ 2016-05-11 15:50 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linus Walleij, Alexandre Courbot, linux-kernel, linux-gpio,
	bcm-kernel-feedback-list, devicetree

Hi Rob,

On 5/11/2016 7:06 AM, Rob Herring wrote:
> On Wed, May 04, 2016 at 09:29:04AM -0700, Ray Jui wrote:
>> Hi Rob,
>>
>> On 5/4/2016 6:20 AM, Rob Herring wrote:
>>> On Mon, May 02, 2016 at 01:51:47PM -0700, Ray Jui wrote:
>>>> Update the iProc GPIO binding document to add new compatible strings
>>>> "brcm,iproc-gpio-v2" and "brcm,iproc-gpio-v3" for the 2nd and 3rd
>>>> generation of the iProc GPIO controllers
>>>>
>>>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>>>> ---
>>>> Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt | 11 ++++++++++-
>>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt b/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt
>>>> index e427792..3a56649 100644
>>>> --- a/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt
>>>> +++ b/Documentation/devicetree/bindings/pinctrl/brcm,iproc-gpio.txt
>>>> @@ -4,7 +4,16 @@ Required properties:
>>>>
>>>> - compatible:
>>>>     Must be "brcm,cygnus-ccm-gpio", "brcm,cygnus-asiu-gpio",
>>>> -    "brcm,cygnus-crmu-gpio" or "brcm,iproc-gpio"
>>>> +    or "brcm,cygnus-crmu-gpio" for Cygnus SoCs
>>>> +
>>>> +    "brcm,iproc-gpio" for the first generation of the GPIO controller that
>>>> +    supports full-featured pinctrl and GPIO functions used in iProc based SoCs
>>>> +
>>>> +    "brcm,iproc-gpio-v2" for the second generation of the GPIO controller that
>>>> +    has the drive strength pinctrl support disabled, e.g., in the iProc NSP SoC
>>>> +
>>>> +    "brcm,iproc-gpio-v3" for the third generation of the GPIO controller that
>>>> +    has the general pinctrl support completely disabled
>>>
>>> You can have these for driver matching, but you still need SoC specific
>>> compatible strings.
>>>
>>> Rob
>>>
>>
>> I think I'm missing something and hope you can help to clarify here. It
>> looks like the notion of v2, v3 should only be used if there's an indeed an
>> revision update on the controller IP itself, correct?
>
> Yes, but only if v2, v3 is actually a meaningful number rather than
> something made up. You have to have a well defined process of IP
> revisions which I have not seen during my time in chip companies, so I'm
> generally suspicious of version numbers. The exception is FPGA IP
> blocks.
>
>> In our case, if the same revision of GPIO controller is used but instead
>> synthesized differently with different SoCs, then you are suggesting here
>> that it should be dealt with SoC specific compatible string, correct?
>
> Yes, that and/or integration differences is why you need SoC specific
> compatible strings. A block could be "identical" but have different max
> frequency for example.
>
>> For example, for the GPIO controller on NSP where drive strength is
>> disabled, the compatible string in DT should look like this?
>>
>> compatible = "brcm,iproc-gpio", "brcm,iproc-gpio-nsp";
>
> Yes, but reverse the order. Most specific to least specific.
>
>>
>> For the GPIO controller on Stingray where pinconf is completely disabled,
>> the compatible string in DT should look like:
>>
>> compatible = "brcm,iproc-gpio", "brcm,iproc-gpio-stingray";
>>
>> Is that correct?
>>
>> Thanks,
>>
>> Ray

Thanks for the clarification. I think I've got all the information I 
need. Will revise and send out PATCH v3 when I get a chance.

Thanks,

Ray

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-05-11 15:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-02 20:51 [PATCH v2 0/2] Additional iProc GPIO support Ray Jui
2016-05-02 20:51 ` [PATCH v2 1/2] dt-bindings: Update iProc GPIO bindings Ray Jui
2016-05-04 13:20   ` Rob Herring
2016-05-04 16:29     ` Ray Jui
2016-05-11 14:06       ` Rob Herring
2016-05-11 15:50         ` Ray Jui
2016-05-02 20:51 ` [PATCH v2 2/2] pinctrl: iproc: Add v2/v3 GPIO support Ray Jui
2016-05-11  9:29 ` [PATCH v2 0/2] Additional iProc " Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.