[PATCH] gpio/serial: revert "linux,first-pin" property handling

[PATCH] gpio/serial: revert "linux,first-pin" property handling

[Linus Walleij]

This is not a legal device tree property, because its binding has
not been reviewed and approved, nor does it exist in any device
tree binding document.

It is further wrong, because it is added to the GPIO offset which
is by definition controller-local.

Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
This got in because I was stressed and was not paying attention.
Sorry about not noticing earlier, but I usually strongly nix any
such properties, please discuss this on the device tree list
first.
---
 drivers/gpio/gpio-exar.c            | 25 +++++++++----------------
 drivers/tty/serial/8250/8250_exar.c |  2 --
 2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
index fb8d304cfa17..d02c5260525f 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -31,7 +31,6 @@ struct exar_gpio_chip {
 	int index;
 	void __iomem *regs;
 	char name[20];
-	unsigned int first_pin;
 };
 
 static void exar_update(struct gpio_chip *chip, unsigned int reg, int val,
@@ -53,9 +52,9 @@ static int exar_set_direction(struct gpio_chip *chip, int direction,
 			      unsigned int offset)
 {
 	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
-	unsigned int addr = (offset + exar_gpio->first_pin) / 8 ?
+	unsigned int addr = offset / 8 ?
 		EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
-	unsigned int bit  = (offset + exar_gpio->first_pin) % 8;
+	unsigned int bit  = offset % 8;
 
 	exar_update(chip, addr, direction, bit);
 	return 0;
@@ -76,9 +75,9 @@ static int exar_get(struct gpio_chip *chip, unsigned int reg)
 static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
 {
 	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
-	unsigned int addr = (offset + exar_gpio->first_pin) / 8 ?
+	unsigned int addr = offset / 8 ?
 		EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
-	unsigned int bit  = (offset + exar_gpio->first_pin) % 8;
+	unsigned int bit  = offset % 8;
 
 	return !!(exar_get(chip, addr) & BIT(bit));
 }
@@ -86,9 +85,9 @@ static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
 static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
 {
 	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
-	unsigned int addr = (offset + exar_gpio->first_pin) / 8 ?
+	unsigned int addr = offset / 8 ?
 		EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
-	unsigned int bit  = (offset + exar_gpio->first_pin) % 8;
+	unsigned int bit  = offset % 8;
 
 	return !!(exar_get(chip, addr) & BIT(bit));
 }
@@ -97,9 +96,9 @@ static void exar_set_value(struct gpio_chip *chip, unsigned int offset,
 			   int value)
 {
 	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
-	unsigned int addr = (offset + exar_gpio->first_pin) / 8 ?
+	unsigned int addr = offset / 8 ?
 		EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
-	unsigned int bit  = (offset + exar_gpio->first_pin) % 8;
+	unsigned int bit  = offset % 8;
 
 	exar_update(chip, addr, value, bit);
 }
@@ -120,7 +119,7 @@ static int gpio_exar_probe(struct platform_device *pdev)
 {
 	struct pci_dev *pcidev = to_pci_dev(pdev->dev.parent);
 	struct exar_gpio_chip *exar_gpio;
-	u32 first_pin, ngpios;
+	u32 ngpios;
 	void __iomem *p;
 	int index, ret;
 
@@ -132,11 +131,6 @@ static int gpio_exar_probe(struct platform_device *pdev)
 	if (!p)
 		return -ENOMEM;
 
-	ret = device_property_read_u32(&pdev->dev, "linux,first-pin",
-				       &first_pin);
-	if (ret)
-		return ret;
-
 	ret = device_property_read_u32(&pdev->dev, "ngpios", &ngpios);
 	if (ret)
 		return ret;
@@ -161,7 +155,6 @@ static int gpio_exar_probe(struct platform_device *pdev)
 	exar_gpio->gpio_chip.ngpio = ngpios;
 	exar_gpio->regs = p;
 	exar_gpio->index = index;
-	exar_gpio->first_pin = first_pin;
 
 	ret = devm_gpiochip_add_data(&pdev->dev,
 				     &exar_gpio->gpio_chip, exar_gpio);
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index b5c98e5bf524..4fb3a4ed4a1f 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -261,7 +261,6 @@ __xr17v35x_register_gpio(struct pci_dev *pcidev,
 }
 
 static const struct property_entry exar_gpio_properties[] = {
-	PROPERTY_ENTRY_U32("linux,first-pin", 0),
 	PROPERTY_ENTRY_U32("ngpios", 16),
 	{ }
 };
@@ -326,7 +325,6 @@ static int iot2040_rs485_config(struct uart_port *port,
 }
 
 static const struct property_entry iot2040_gpio_properties[] = {
-	PROPERTY_ENTRY_U32("linux,first-pin", 10),
 	PROPERTY_ENTRY_U32("ngpios", 1),
 	{ }
 };
-- 
2.9.4

Re: [PATCH] gpio/serial: revert "linux,first-pin" property handling

[Jan Kiszka]

On 2017-07-10 16:09, Linus Walleij wrote:
> This is not a legal device tree property, because its binding has
> not been reviewed and approved, nor does it exist in any device
> tree binding document.
> 
> It is further wrong, because it is added to the GPIO offset which
> is by definition controller-local.
> 
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> This got in because I was stressed and was not paying attention.
> Sorry about not noticing earlier, but I usually strongly nix any
> such properties, please discuss this on the device tree list
> first.
> ---
>  drivers/gpio/gpio-exar.c            | 25 +++++++++----------------
>  drivers/tty/serial/8250/8250_exar.c |  2 --
>  2 files changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
> index fb8d304cfa17..d02c5260525f 100644
> --- a/drivers/gpio/gpio-exar.c
> +++ b/drivers/gpio/gpio-exar.c
> @@ -31,7 +31,6 @@ struct exar_gpio_chip {
>  	int index;
>  	void __iomem *regs;
>  	char name[20];
> -	unsigned int first_pin;
>  };
>  
>  static void exar_update(struct gpio_chip *chip, unsigned int reg, int val,
> @@ -53,9 +52,9 @@ static int exar_set_direction(struct gpio_chip *chip, int direction,
>  			      unsigned int offset)
>  {
>  	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
> -	unsigned int addr = (offset + exar_gpio->first_pin) / 8 ?
> +	unsigned int addr = offset / 8 ?
>  		EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
> -	unsigned int bit  = (offset + exar_gpio->first_pin) % 8;
> +	unsigned int bit  = offset % 8;
>  
>  	exar_update(chip, addr, direction, bit);
>  	return 0;
> @@ -76,9 +75,9 @@ static int exar_get(struct gpio_chip *chip, unsigned int reg)
>  static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
>  {
>  	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
> -	unsigned int addr = (offset + exar_gpio->first_pin) / 8 ?
> +	unsigned int addr = offset / 8 ?
>  		EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
> -	unsigned int bit  = (offset + exar_gpio->first_pin) % 8;
> +	unsigned int bit  = offset % 8;
>  
>  	return !!(exar_get(chip, addr) & BIT(bit));
>  }
> @@ -86,9 +85,9 @@ static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
>  static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
>  {
>  	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
> -	unsigned int addr = (offset + exar_gpio->first_pin) / 8 ?
> +	unsigned int addr = offset / 8 ?
>  		EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
> -	unsigned int bit  = (offset + exar_gpio->first_pin) % 8;
> +	unsigned int bit  = offset % 8;
>  
>  	return !!(exar_get(chip, addr) & BIT(bit));
>  }
> @@ -97,9 +96,9 @@ static void exar_set_value(struct gpio_chip *chip, unsigned int offset,
>  			   int value)
>  {
>  	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
> -	unsigned int addr = (offset + exar_gpio->first_pin) / 8 ?
> +	unsigned int addr = offset / 8 ?
>  		EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
> -	unsigned int bit  = (offset + exar_gpio->first_pin) % 8;
> +	unsigned int bit  = offset % 8;
>  
>  	exar_update(chip, addr, value, bit);
>  }
> @@ -120,7 +119,7 @@ static int gpio_exar_probe(struct platform_device *pdev)
>  {
>  	struct pci_dev *pcidev = to_pci_dev(pdev->dev.parent);
>  	struct exar_gpio_chip *exar_gpio;
> -	u32 first_pin, ngpios;
> +	u32 ngpios;
>  	void __iomem *p;
>  	int index, ret;
>  
> @@ -132,11 +131,6 @@ static int gpio_exar_probe(struct platform_device *pdev)
>  	if (!p)
>  		return -ENOMEM;
>  
> -	ret = device_property_read_u32(&pdev->dev, "linux,first-pin",
> -				       &first_pin);
> -	if (ret)
> -		return ret;
> -
>  	ret = device_property_read_u32(&pdev->dev, "ngpios", &ngpios);
>  	if (ret)
>  		return ret;
> @@ -161,7 +155,6 @@ static int gpio_exar_probe(struct platform_device *pdev)
>  	exar_gpio->gpio_chip.ngpio = ngpios;
>  	exar_gpio->regs = p;
>  	exar_gpio->index = index;
> -	exar_gpio->first_pin = first_pin;
>  
>  	ret = devm_gpiochip_add_data(&pdev->dev,
>  				     &exar_gpio->gpio_chip, exar_gpio);
> diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
> index b5c98e5bf524..4fb3a4ed4a1f 100644
> --- a/drivers/tty/serial/8250/8250_exar.c
> +++ b/drivers/tty/serial/8250/8250_exar.c
> @@ -261,7 +261,6 @@ __xr17v35x_register_gpio(struct pci_dev *pcidev,
>  }
>  
>  static const struct property_entry exar_gpio_properties[] = {
> -	PROPERTY_ENTRY_U32("linux,first-pin", 0),
>  	PROPERTY_ENTRY_U32("ngpios", 16),
>  	{ }
>  };
> @@ -326,7 +325,6 @@ static int iot2040_rs485_config(struct uart_port *port,
>  }
>  
>  static const struct property_entry iot2040_gpio_properties[] = {
> -	PROPERTY_ENTRY_U32("linux,first-pin", 10),
>  	PROPERTY_ENTRY_U32("ngpios", 1),
>  	{ }
>  };
> 

Sorry for the mess, I didn't know the proper process in details.

However, just reverting breaks the IOT2000, need to check if there are
even physical risks. How can we quickly resolve this to proper mechanism?

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] gpio/serial: revert "linux,first-pin" property handling

[Jan Kiszka]

On 2017-07-10 16:23, Jan Kiszka wrote:
> On 2017-07-10 16:09, Linus Walleij wrote:
>> This is not a legal device tree property, because its binding has
>> not been reviewed and approved, nor does it exist in any device
>> tree binding document.
>>
>> It is further wrong, because it is added to the GPIO offset which
>> is by definition controller-local.

BTW, I don't get this second statement. The parameter passed in is
controlling which pin of the GPIO device can be controlled by the driver
at all. And that's also a very private interface between 8250_exar and
gpio-exar: the former tells the latter which pins are physically
available to drive, because the former operates the others already.

>>
>> Cc: Jan Kiszka <jan.kiszka@siemens.com>
>> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>> This got in because I was stressed and was not paying attention.
>> Sorry about not noticing earlier, but I usually strongly nix any
>> such properties, please discuss this on the device tree list
>> first.
>> ---
>>  drivers/gpio/gpio-exar.c            | 25 +++++++++----------------
>>  drivers/tty/serial/8250/8250_exar.c |  2 --
>>  2 files changed, 9 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
>> index fb8d304cfa17..d02c5260525f 100644
>> --- a/drivers/gpio/gpio-exar.c
>> +++ b/drivers/gpio/gpio-exar.c
>> @@ -31,7 +31,6 @@ struct exar_gpio_chip {
>>  	int index;
>>  	void __iomem *regs;
>>  	char name[20];
>> -	unsigned int first_pin;
>>  };
>>  
>>  static void exar_update(struct gpio_chip *chip, unsigned int reg, int val,
>> @@ -53,9 +52,9 @@ static int exar_set_direction(struct gpio_chip *chip, int direction,
>>  			      unsigned int offset)
>>  {
>>  	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
>> -	unsigned int addr = (offset + exar_gpio->first_pin) / 8 ?
>> +	unsigned int addr = offset / 8 ?
>>  		EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
>> -	unsigned int bit  = (offset + exar_gpio->first_pin) % 8;
>> +	unsigned int bit  = offset % 8;
>>  
>>  	exar_update(chip, addr, direction, bit);
>>  	return 0;
>> @@ -76,9 +75,9 @@ static int exar_get(struct gpio_chip *chip, unsigned int reg)
>>  static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
>>  {
>>  	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
>> -	unsigned int addr = (offset + exar_gpio->first_pin) / 8 ?
>> +	unsigned int addr = offset / 8 ?
>>  		EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
>> -	unsigned int bit  = (offset + exar_gpio->first_pin) % 8;
>> +	unsigned int bit  = offset % 8;
>>  
>>  	return !!(exar_get(chip, addr) & BIT(bit));
>>  }
>> @@ -86,9 +85,9 @@ static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
>>  static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
>>  {
>>  	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
>> -	unsigned int addr = (offset + exar_gpio->first_pin) / 8 ?
>> +	unsigned int addr = offset / 8 ?
>>  		EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
>> -	unsigned int bit  = (offset + exar_gpio->first_pin) % 8;
>> +	unsigned int bit  = offset % 8;
>>  
>>  	return !!(exar_get(chip, addr) & BIT(bit));
>>  }
>> @@ -97,9 +96,9 @@ static void exar_set_value(struct gpio_chip *chip, unsigned int offset,
>>  			   int value)
>>  {
>>  	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
>> -	unsigned int addr = (offset + exar_gpio->first_pin) / 8 ?
>> +	unsigned int addr = offset / 8 ?
>>  		EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
>> -	unsigned int bit  = (offset + exar_gpio->first_pin) % 8;
>> +	unsigned int bit  = offset % 8;
>>  
>>  	exar_update(chip, addr, value, bit);
>>  }
>> @@ -120,7 +119,7 @@ static int gpio_exar_probe(struct platform_device *pdev)
>>  {
>>  	struct pci_dev *pcidev = to_pci_dev(pdev->dev.parent);
>>  	struct exar_gpio_chip *exar_gpio;
>> -	u32 first_pin, ngpios;
>> +	u32 ngpios;
>>  	void __iomem *p;
>>  	int index, ret;
>>  
>> @@ -132,11 +131,6 @@ static int gpio_exar_probe(struct platform_device *pdev)
>>  	if (!p)
>>  		return -ENOMEM;
>>  
>> -	ret = device_property_read_u32(&pdev->dev, "linux,first-pin",
>> -				       &first_pin);
>> -	if (ret)
>> -		return ret;
>> -
>>  	ret = device_property_read_u32(&pdev->dev, "ngpios", &ngpios);
>>  	if (ret)
>>  		return ret;
>> @@ -161,7 +155,6 @@ static int gpio_exar_probe(struct platform_device *pdev)
>>  	exar_gpio->gpio_chip.ngpio = ngpios;
>>  	exar_gpio->regs = p;
>>  	exar_gpio->index = index;
>> -	exar_gpio->first_pin = first_pin;
>>  
>>  	ret = devm_gpiochip_add_data(&pdev->dev,
>>  				     &exar_gpio->gpio_chip, exar_gpio);
>> diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
>> index b5c98e5bf524..4fb3a4ed4a1f 100644
>> --- a/drivers/tty/serial/8250/8250_exar.c
>> +++ b/drivers/tty/serial/8250/8250_exar.c
>> @@ -261,7 +261,6 @@ __xr17v35x_register_gpio(struct pci_dev *pcidev,
>>  }
>>  
>>  static const struct property_entry exar_gpio_properties[] = {
>> -	PROPERTY_ENTRY_U32("linux,first-pin", 0),
>>  	PROPERTY_ENTRY_U32("ngpios", 16),
>>  	{ }
>>  };
>> @@ -326,7 +325,6 @@ static int iot2040_rs485_config(struct uart_port *port,
>>  }
>>  
>>  static const struct property_entry iot2040_gpio_properties[] = {
>> -	PROPERTY_ENTRY_U32("linux,first-pin", 10),
>>  	PROPERTY_ENTRY_U32("ngpios", 1),
>>  	{ }
>>  };
>>
> 
> Sorry for the mess, I didn't know the proper process in details.
> 
> However, just reverting breaks the IOT2000, need to check if there are
> even physical risks. How can we quickly resolve this to proper mechanism?

Ping, before we half-break things with such a revert.

To summarize what we discussed during the patch development:

This is a private, non-device-tree interface between the creator,
8250_exar, and the GPIO device. The original proposal was to not use
device properties at all, rather some platform-data-like pattern [1].
Then we switched to properties [2], but that implicitly creates an
interfaces in the space that is under device tree control. But, for the
given use case, gpio-exar will never be parametrized via device trees or
ACPI.

So, how to proceed?

Jan

[1]
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1399507.html
[2]
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1407975.html

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] gpio/serial: revert "linux,first-pin" property handling

[Linus Walleij]

On Tue, Jul 11, 2017 at 5:42 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-07-10 16:23, Jan Kiszka wrote:
>> On 2017-07-10 16:09, Linus Walleij wrote:
>>> This is not a legal device tree property, because its binding has
>>> not been reviewed and approved, nor does it exist in any device
>>> tree binding document.
>>>
>>> It is further wrong, because it is added to the GPIO offset which
>>> is by definition controller-local.
>
> BTW, I don't get this second statement. The parameter passed in is
> controlling which pin of the GPIO device can be controlled by the driver
> at all. And that's also a very private interface between 8250_exar and
> gpio-exar: the former tells the latter which pins are physically
> available to drive, because the former operates the others already.

Then it seems like the big confusion here is the "linux,*" prefix in
the DT binding.

We only use that for information which is necessary evil Linux-specific
stuff.

What you are talking about should be hardware-and-vendor specific
info and prefixed "exar,*".

Can you propose a patch simply changing the binding and try to get
the DT maintainers ACK on it?

Yours,
Linus Walleij

Re: [PATCH] gpio/serial: revert "linux,first-pin" property handling

[Jan Kiszka]

On 2017-07-12 11:30, Linus Walleij wrote:
> On Tue, Jul 11, 2017 at 5:42 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2017-07-10 16:23, Jan Kiszka wrote:
>>> On 2017-07-10 16:09, Linus Walleij wrote:
>>>> This is not a legal device tree property, because its binding has
>>>> not been reviewed and approved, nor does it exist in any device
>>>> tree binding document.
>>>>
>>>> It is further wrong, because it is added to the GPIO offset which
>>>> is by definition controller-local.
>>
>> BTW, I don't get this second statement. The parameter passed in is
>> controlling which pin of the GPIO device can be controlled by the driver
>> at all. And that's also a very private interface between 8250_exar and
>> gpio-exar: the former tells the latter which pins are physically
>> available to drive, because the former operates the others already.
> 
> Then it seems like the big confusion here is the "linux,*" prefix in
> the DT binding.
> 
> We only use that for information which is necessary evil Linux-specific
> stuff.
> 
> What you are talking about should be hardware-and-vendor specific
> info and prefixed "exar,*".

"exar," or rather "gpio-exar,", like in [1]?

> 
> Can you propose a patch simply changing the binding and try to get
> the DT maintainers ACK on it?

Sure, will do then.

Jan

[1]
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1407975.html

Re: [PATCH] gpio/serial: revert "linux,first-pin" property handling

[Linus Walleij]

On Wed, Jul 12, 2017 at 1:30 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-07-12 11:30, Linus Walleij wrote:

>> What you are talking about should be hardware-and-vendor specific
>> info and prefixed "exar,*".
>
> "exar," or rather "gpio-exar,", like in [1]?

HW-specific DT bindings have this form: "vendor,foo".

So "exar,gpio-first-exported-pin" is some kind of overstating of the
fact, but correct.

The bindings (put with the rest of the exar DT bindings if there
are such somewhere in Documentation/devicetree/bindings/* else
in a new file) should make it very clear that this is a
hardware-specific thing telling us which hardware pin is the
first offset to be used for GPIO.

Yours,
Linus Walleij

Re: [PATCH] gpio/serial: revert "linux,first-pin" property handling

[Jan Kiszka]

On 2017-07-12 13:39, Linus Walleij wrote:
> On Wed, Jul 12, 2017 at 1:30 PM, Jan Kiszka <jan.kiszka-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org> wrote:
>> On 2017-07-12 11:30, Linus Walleij wrote:
> 
>>> What you are talking about should be hardware-and-vendor specific
>>> info and prefixed "exar,*".
>>
>> "exar," or rather "gpio-exar,", like in [1]?
> 
> HW-specific DT bindings have this form: "vendor,foo".
> 
> So "exar,gpio-first-exported-pin" is some kind of overstating of the
> fact, but correct.

OK.

> 
> The bindings (put with the rest of the exar DT bindings if there
> are such somewhere in Documentation/devicetree/bindings/* else
> in a new file) should make it very clear that this is a
> hardware-specific thing telling us which hardware pin is the
> first offset to be used for GPIO.

This binding will lack any compatible value, so I'm not yet sure how to
name/document things properly. Is there a precedence for such a case?
Well, I can add some gpio-exar.txt and just put those two properties in it.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html