[PATCH v2] gpiolib: Take MUX usage into account

[PATCH v2] gpiolib: Take MUX usage into account

[Ramon Fried]

From: Stefan Wahren <stefan.wahren@i2se.com>

The user space like gpioinfo only see the GPIO usage but not the
MUX usage (e.g. I2C or SPI usage) of a pin. As a user we want to know which
pin is free/safe to use. So take the MUX usage of strict pinmux controllers
into account to get a more realistic view for ioctl GPIO_GET_LINEINFO_IOCTL.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
Tested-by: Ramon Fried <rfried.dev@gmail.com>
Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
---
v2: Address review from linus:
* ** Please notive logic was reversed **
* renamed pinctrl_gpio_is_in_use() to pinctrl_gpio_can_use_line()
* renamed pinmux_is_in_use() to pinmux_can_be_used_for_gpio()
* changed dev_err to dev_dbg (Linus suggested removing it altogether, I
  find it better to keep it for debug).

 drivers/gpio/gpiolib.c           |  3 ++-
 drivers/pinctrl/core.c           | 28 ++++++++++++++++++++++++++++
 drivers/pinctrl/pinmux.c         | 27 +++++++++++++++++++++++++++
 drivers/pinctrl/pinmux.h         |  8 ++++++++
 include/linux/pinctrl/consumer.h |  6 ++++++
 5 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f497003f119c..52937bf8e514 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1084,7 +1084,8 @@ static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		    test_bit(FLAG_IS_HOGGED, &desc->flags) ||
 		    test_bit(FLAG_USED_AS_IRQ, &desc->flags) ||
 		    test_bit(FLAG_EXPORT, &desc->flags) ||
-		    test_bit(FLAG_SYSFS, &desc->flags))
+		    test_bit(FLAG_SYSFS, &desc->flags) ||
+		    !pinctrl_gpio_can_use_line(chip->base + lineinfo.line_offset))
 			lineinfo.flags |= GPIOLINE_FLAG_KERNEL;
 		if (test_bit(FLAG_IS_OUT, &desc->flags))
 			lineinfo.flags |= GPIOLINE_FLAG_IS_OUT;
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index b70df27874d1..2bbd8ee93507 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -736,6 +736,34 @@ int pinctrl_get_group_selector(struct pinctrl_dev *pctldev,
 	return -EINVAL;
 }
 
+bool pinctrl_gpio_can_use_line(unsigned gpio)
+{
+	struct pinctrl_dev *pctldev;
+	struct pinctrl_gpio_range *range;
+	bool result;
+	int pin;
+
+	/*
+	 * Try to obtain GPIO range, if it fails
+	 * we're probably dealing with GPIO driver
+	 * without a backing pin controller - bail out.
+	 */
+	if (pinctrl_get_device_gpio_range(gpio, &pctldev, &range))
+		return true;
+
+	mutex_lock(&pctldev->mutex);
+
+	/* Convert to the pin controllers number space */
+	pin = gpio_to_pin(range, gpio);
+
+	result = pinmux_can_be_used_for_gpio(pctldev, pin);
+
+	mutex_unlock(&pctldev->mutex);
+
+	return result;
+}
+EXPORT_SYMBOL_GPL(pinctrl_gpio_can_use_line);
+
 /**
  * pinctrl_gpio_request() - request a single pin to be used as GPIO
  * @gpio: the GPIO pin number from the GPIO subsystem number space
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 020e54f843f9..7e42a5738d82 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -70,6 +70,33 @@ int pinmux_validate_map(const struct pinctrl_map *map, int i)
 	return 0;
 }
 
+/**
+ * pinmux_can_be_used_for_gpio() - check if a specific pin
+ *	is either muxed to a different function or used as gpio.
+ *
+ * @pin: the pin number in the global pin space
+ *
+ * Controllers not defined as strict will always return true,
+ * menaning that the gpio can be used.
+ */
+bool pinmux_can_be_used_for_gpio(struct pinctrl_dev *pctldev, unsigned pin)
+{
+	struct pin_desc *desc = pin_desc_get(pctldev, pin);
+	const struct pinmux_ops *ops = pctldev->desc->pmxops;
+
+	if (!desc) {
+		dev_dbg(pctldev->dev,
+			"pin %u is not registered so it cannot be requested\n",
+			pin);
+		return true;
+	}
+
+	if (ops->strict && desc->mux_usecount)
+		return false;
+
+	return !(ops->strict && !!desc->gpio_owner);
+}
+
 /**
  * pin_request() - request a single pin to be muxed in, typically for GPIO
  * @pin: the pin number in the global pin space
diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
index 794cb3a003ff..78c3a31be882 100644
--- a/drivers/pinctrl/pinmux.h
+++ b/drivers/pinctrl/pinmux.h
@@ -15,6 +15,8 @@ int pinmux_check_ops(struct pinctrl_dev *pctldev);
 
 int pinmux_validate_map(const struct pinctrl_map *map, int i);
 
+bool pinmux_can_be_used_for_gpio(struct pinctrl_dev *pctldev, unsigned pin);
+
 int pinmux_request_gpio(struct pinctrl_dev *pctldev,
 			struct pinctrl_gpio_range *range,
 			unsigned pin, unsigned gpio);
@@ -42,6 +44,12 @@ static inline int pinmux_validate_map(const struct pinctrl_map *map, int i)
 	return 0;
 }
 
+static inline bool pinmux_can_be_used_for_gpio(struct pinctrl_dev *pctldev,
+					       unsigned pin)
+{
+	return true;
+}
+
 static inline int pinmux_request_gpio(struct pinctrl_dev *pctldev,
 			struct pinctrl_gpio_range *range,
 			unsigned pin, unsigned gpio)
diff --git a/include/linux/pinctrl/consumer.h b/include/linux/pinctrl/consumer.h
index 86720a5a384f..7f8c7d9583d3 100644
--- a/include/linux/pinctrl/consumer.h
+++ b/include/linux/pinctrl/consumer.h
@@ -24,6 +24,7 @@ struct device;
 #ifdef CONFIG_PINCTRL
 
 /* External interface to pin control */
+extern bool pinctrl_gpio_can_use_line(unsigned gpio);
 extern int pinctrl_gpio_request(unsigned gpio);
 extern void pinctrl_gpio_free(unsigned gpio);
 extern int pinctrl_gpio_direction_input(unsigned gpio);
@@ -61,6 +62,11 @@ static inline int pinctrl_pm_select_idle_state(struct device *dev)
 
 #else /* !CONFIG_PINCTRL */
 
+static inline bool pinctrl_gpio_can_use_line(unsigned gpio)
+{
+	return true;
+}
+
 static inline int pinctrl_gpio_request(unsigned gpio)
 {
 	return 0;
-- 
2.20.1

Re: [PATCH v2] gpiolib: Take MUX usage into account

[Stefan Wahren]

Hi Ramon,

On 13.08.19 03:42, Ramon Fried wrote:
> From: Stefan Wahren <stefan.wahren@i2se.com>
>
> The user space like gpioinfo only see the GPIO usage but not the
> MUX usage (e.g. I2C or SPI usage) of a pin. As a user we want to know which
> pin is free/safe to use. So take the MUX usage of strict pinmux controllers
> into account to get a more realistic view for ioctl GPIO_GET_LINEINFO_IOCTL.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> Tested-by: Ramon Fried <rfried.dev@gmail.com>
> Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
> ---
> v2: Address review from linus:
> * ** Please notive logic was reversed **
> * renamed pinctrl_gpio_is_in_use() to pinctrl_gpio_can_use_line()
> * renamed pinmux_is_in_use() to pinmux_can_be_used_for_gpio()
> * changed dev_err to dev_dbg (Linus suggested removing it altogether, I
>   find it better to keep it for debug).
thanks for taking care of this.
>
>  drivers/gpio/gpiolib.c           |  3 ++-
>  drivers/pinctrl/core.c           | 28 ++++++++++++++++++++++++++++
>  drivers/pinctrl/pinmux.c         | 27 +++++++++++++++++++++++++++
>  drivers/pinctrl/pinmux.h         |  8 ++++++++
>  include/linux/pinctrl/consumer.h |  6 ++++++
>  5 files changed, 71 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index f497003f119c..52937bf8e514 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1084,7 +1084,8 @@ static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  		    test_bit(FLAG_IS_HOGGED, &desc->flags) ||
>  		    test_bit(FLAG_USED_AS_IRQ, &desc->flags) ||
>  		    test_bit(FLAG_EXPORT, &desc->flags) ||
> -		    test_bit(FLAG_SYSFS, &desc->flags))
> +		    test_bit(FLAG_SYSFS, &desc->flags) ||
> +		    !pinctrl_gpio_can_use_line(chip->base + lineinfo.line_offset))
>  			lineinfo.flags |= GPIOLINE_FLAG_KERNEL;
>  		if (test_bit(FLAG_IS_OUT, &desc->flags))
>  			lineinfo.flags |= GPIOLINE_FLAG_IS_OUT;
> diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
> index b70df27874d1..2bbd8ee93507 100644
> --- a/drivers/pinctrl/core.c
> +++ b/drivers/pinctrl/core.c
> @@ -736,6 +736,34 @@ int pinctrl_get_group_selector(struct pinctrl_dev *pctldev,
>  	return -EINVAL;
>  }
>  
> +bool pinctrl_gpio_can_use_line(unsigned gpio)
> +{
> +	struct pinctrl_dev *pctldev;
> +	struct pinctrl_gpio_range *range;
> +	bool result;
> +	int pin;
> +
> +	/*
> +	 * Try to obtain GPIO range, if it fails
> +	 * we're probably dealing with GPIO driver
> +	 * without a backing pin controller - bail out.
> +	 */
> +	if (pinctrl_get_device_gpio_range(gpio, &pctldev, &range))
> +		return true;
> +
> +	mutex_lock(&pctldev->mutex);
> +
> +	/* Convert to the pin controllers number space */
> +	pin = gpio_to_pin(range, gpio);
> +
> +	result = pinmux_can_be_used_for_gpio(pctldev, pin);
> +
> +	mutex_unlock(&pctldev->mutex);
> +
> +	return result;
> +}
> +EXPORT_SYMBOL_GPL(pinctrl_gpio_can_use_line);
> +
>  /**
>   * pinctrl_gpio_request() - request a single pin to be used as GPIO
>   * @gpio: the GPIO pin number from the GPIO subsystem number space
> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> index 020e54f843f9..7e42a5738d82 100644
> --- a/drivers/pinctrl/pinmux.c
> +++ b/drivers/pinctrl/pinmux.c
> @@ -70,6 +70,33 @@ int pinmux_validate_map(const struct pinctrl_map *map, int i)
>  	return 0;
>  }
>  
> +/**
> + * pinmux_can_be_used_for_gpio() - check if a specific pin
> + *	is either muxed to a different function or used as gpio.
> + *
> + * @pin: the pin number in the global pin space
> + *
> + * Controllers not defined as strict will always return true,
> + * menaning that the gpio can be used.
> + */
> +bool pinmux_can_be_used_for_gpio(struct pinctrl_dev *pctldev, unsigned pin)
> +{
> +	struct pin_desc *desc = pin_desc_get(pctldev, pin);
> +	const struct pinmux_ops *ops = pctldev->desc->pmxops;
> +
> +	if (!desc) {
> +		dev_dbg(pctldev->dev,
> +			"pin %u is not registered so it cannot be requested\n",
> +			pin);
> +		return true;

This return value looks strange to me.

Stefan

Re: [PATCH v2] gpiolib: Take MUX usage into account

[Fried, Ramon]

On 8/13/2019 08:38, Stefan Wahren wrote:
> Hi Ramon,
>
> On 13.08.19 03:42, Ramon Fried wrote:
>> From: Stefan Wahren <stefan.wahren@i2se.com>
>>
>> The user space like gpioinfo only see the GPIO usage but not the
>> MUX usage (e.g. I2C or SPI usage) of a pin. As a user we want to know which
>> pin is free/safe to use. So take the MUX usage of strict pinmux controllers
>> into account to get a more realistic view for ioctl GPIO_GET_LINEINFO_IOCTL.
>>
>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>> Tested-by: Ramon Fried <rfried.dev@gmail.com>
>> Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
>> ---
>> v2: Address review from linus:
>> * ** Please notive logic was reversed **
>> * renamed pinctrl_gpio_is_in_use() to pinctrl_gpio_can_use_line()
>> * renamed pinmux_is_in_use() to pinmux_can_be_used_for_gpio()
>> * changed dev_err to dev_dbg (Linus suggested removing it altogether, I
>>    find it better to keep it for debug).
> thanks for taking care of this.
>>   drivers/gpio/gpiolib.c           |  3 ++-
>>   drivers/pinctrl/core.c           | 28 ++++++++++++++++++++++++++++
>>   drivers/pinctrl/pinmux.c         | 27 +++++++++++++++++++++++++++
>>   drivers/pinctrl/pinmux.h         |  8 ++++++++
>>   include/linux/pinctrl/consumer.h |  6 ++++++
>>   5 files changed, 71 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
>> index f497003f119c..52937bf8e514 100644
>> --- a/drivers/gpio/gpiolib.c
>> +++ b/drivers/gpio/gpiolib.c
>> @@ -1084,7 +1084,8 @@ static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>>   		    test_bit(FLAG_IS_HOGGED, &desc->flags) ||
>>   		    test_bit(FLAG_USED_AS_IRQ, &desc->flags) ||
>>   		    test_bit(FLAG_EXPORT, &desc->flags) ||
>> -		    test_bit(FLAG_SYSFS, &desc->flags))
>> +		    test_bit(FLAG_SYSFS, &desc->flags) ||
>> +		    !pinctrl_gpio_can_use_line(chip->base + lineinfo.line_offset))
>>   			lineinfo.flags |= GPIOLINE_FLAG_KERNEL;
>>   		if (test_bit(FLAG_IS_OUT, &desc->flags))
>>   			lineinfo.flags |= GPIOLINE_FLAG_IS_OUT;
>> diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
>> index b70df27874d1..2bbd8ee93507 100644
>> --- a/drivers/pinctrl/core.c
>> +++ b/drivers/pinctrl/core.c
>> @@ -736,6 +736,34 @@ int pinctrl_get_group_selector(struct pinctrl_dev *pctldev,
>>   	return -EINVAL;
>>   }
>>   
>> +bool pinctrl_gpio_can_use_line(unsigned gpio)
>> +{
>> +	struct pinctrl_dev *pctldev;
>> +	struct pinctrl_gpio_range *range;
>> +	bool result;
>> +	int pin;
>> +
>> +	/*
>> +	 * Try to obtain GPIO range, if it fails
>> +	 * we're probably dealing with GPIO driver
>> +	 * without a backing pin controller - bail out.
>> +	 */
>> +	if (pinctrl_get_device_gpio_range(gpio, &pctldev, &range))
>> +		return true;
>> +
>> +	mutex_lock(&pctldev->mutex);
>> +
>> +	/* Convert to the pin controllers number space */
>> +	pin = gpio_to_pin(range, gpio);
>> +
>> +	result = pinmux_can_be_used_for_gpio(pctldev, pin);
>> +
>> +	mutex_unlock(&pctldev->mutex);
>> +
>> +	return result;
>> +}
>> +EXPORT_SYMBOL_GPL(pinctrl_gpio_can_use_line);
>> +
>>   /**
>>    * pinctrl_gpio_request() - request a single pin to be used as GPIO
>>    * @gpio: the GPIO pin number from the GPIO subsystem number space
>> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
>> index 020e54f843f9..7e42a5738d82 100644
>> --- a/drivers/pinctrl/pinmux.c
>> +++ b/drivers/pinctrl/pinmux.c
>> @@ -70,6 +70,33 @@ int pinmux_validate_map(const struct pinctrl_map *map, int i)
>>   	return 0;
>>   }
>>   
>> +/**
>> + * pinmux_can_be_used_for_gpio() - check if a specific pin
>> + *	is either muxed to a different function or used as gpio.
>> + *
>> + * @pin: the pin number in the global pin space
>> + *
>> + * Controllers not defined as strict will always return true,
>> + * menaning that the gpio can be used.
>> + */
>> +bool pinmux_can_be_used_for_gpio(struct pinctrl_dev *pctldev, unsigned pin)
>> +{
>> +	struct pin_desc *desc = pin_desc_get(pctldev, pin);
>> +	const struct pinmux_ops *ops = pctldev->desc->pmxops;
>> +
>> +	if (!desc) {
>> +		dev_dbg(pctldev->dev,
>> +			"pin %u is not registered so it cannot be requested\n",
>> +			pin);
>> +		return true;
> This return value looks strange to me.

Basically, it's just the reversed return value you returned in the 
original patch,
It means in this context that if the pin is not owned by a 
pin-controller it can be used for GPIO.

Thanks,
Ramon.

>
> Stefan
>

Re: [PATCH v2] gpiolib: Take MUX usage into account

[Stefan Wahren]

Am 13.08.19 um 08:10 schrieb Fried, Ramon:
>
> On 8/13/2019 08:38, Stefan Wahren wrote:
>> Hi Ramon,
>>
>> On 13.08.19 03:42, Ramon Fried wrote:
>>> From: Stefan Wahren <stefan.wahren@i2se.com>
>>>
>>> The user space like gpioinfo only see the GPIO usage but not the
>>> MUX usage (e.g. I2C or SPI usage) of a pin. As a user we want to
>>> know which
>>> pin is free/safe to use. So take the MUX usage of strict pinmux
>>> controllers
>>> into account to get a more realistic view for ioctl
>>> GPIO_GET_LINEINFO_IOCTL.
>>>
>>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>>> Tested-by: Ramon Fried <rfried.dev@gmail.com>
>>> Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
>>> ---
>>> v2: Address review from linus:
>>> * ** Please notive logic was reversed **
>>> * renamed pinctrl_gpio_is_in_use() to pinctrl_gpio_can_use_line()
>>> * renamed pinmux_is_in_use() to pinmux_can_be_used_for_gpio()
>>> * changed dev_err to dev_dbg (Linus suggested removing it altogether, I
>>>    find it better to keep it for debug).
>> thanks for taking care of this.
>>>   drivers/gpio/gpiolib.c           |  3 ++-
>>>   drivers/pinctrl/core.c           | 28 ++++++++++++++++++++++++++++
>>>   drivers/pinctrl/pinmux.c         | 27 +++++++++++++++++++++++++++
>>>   drivers/pinctrl/pinmux.h         |  8 ++++++++
>>>   include/linux/pinctrl/consumer.h |  6 ++++++
>>>   5 files changed, 71 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
>>> index f497003f119c..52937bf8e514 100644
>>> --- a/drivers/gpio/gpiolib.c
>>> +++ b/drivers/gpio/gpiolib.c
>>> @@ -1084,7 +1084,8 @@ static long gpio_ioctl(struct file *filp,
>>> unsigned int cmd, unsigned long arg)
>>>               test_bit(FLAG_IS_HOGGED, &desc->flags) ||
>>>               test_bit(FLAG_USED_AS_IRQ, &desc->flags) ||
>>>               test_bit(FLAG_EXPORT, &desc->flags) ||
>>> -            test_bit(FLAG_SYSFS, &desc->flags))
>>> +            test_bit(FLAG_SYSFS, &desc->flags) ||
>>> +            !pinctrl_gpio_can_use_line(chip->base +
>>> lineinfo.line_offset))
>>>               lineinfo.flags |= GPIOLINE_FLAG_KERNEL;
>>>           if (test_bit(FLAG_IS_OUT, &desc->flags))
>>>               lineinfo.flags |= GPIOLINE_FLAG_IS_OUT;
>>> diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
>>> index b70df27874d1..2bbd8ee93507 100644
>>> --- a/drivers/pinctrl/core.c
>>> +++ b/drivers/pinctrl/core.c
>>> @@ -736,6 +736,34 @@ int pinctrl_get_group_selector(struct
>>> pinctrl_dev *pctldev,
>>>       return -EINVAL;
>>>   }
>>>   +bool pinctrl_gpio_can_use_line(unsigned gpio)
>>> +{
>>> +    struct pinctrl_dev *pctldev;
>>> +    struct pinctrl_gpio_range *range;
>>> +    bool result;
>>> +    int pin;
>>> +
>>> +    /*
>>> +     * Try to obtain GPIO range, if it fails
>>> +     * we're probably dealing with GPIO driver
>>> +     * without a backing pin controller - bail out.
>>> +     */
>>> +    if (pinctrl_get_device_gpio_range(gpio, &pctldev, &range))
>>> +        return true;
>>> +
>>> +    mutex_lock(&pctldev->mutex);
>>> +
>>> +    /* Convert to the pin controllers number space */
>>> +    pin = gpio_to_pin(range, gpio);
>>> +
>>> +    result = pinmux_can_be_used_for_gpio(pctldev, pin);
>>> +
>>> +    mutex_unlock(&pctldev->mutex);
>>> +
>>> +    return result;
>>> +}
>>> +EXPORT_SYMBOL_GPL(pinctrl_gpio_can_use_line);
>>> +
>>>   /**
>>>    * pinctrl_gpio_request() - request a single pin to be used as GPIO
>>>    * @gpio: the GPIO pin number from the GPIO subsystem number space
>>> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
>>> index 020e54f843f9..7e42a5738d82 100644
>>> --- a/drivers/pinctrl/pinmux.c
>>> +++ b/drivers/pinctrl/pinmux.c
>>> @@ -70,6 +70,33 @@ int pinmux_validate_map(const struct pinctrl_map
>>> *map, int i)
>>>       return 0;
>>>   }
>>>   +/**
>>> + * pinmux_can_be_used_for_gpio() - check if a specific pin
>>> + *    is either muxed to a different function or used as gpio.
>>> + *
>>> + * @pin: the pin number in the global pin space
>>> + *
>>> + * Controllers not defined as strict will always return true,
>>> + * menaning that the gpio can be used.
>>> + */
>>> +bool pinmux_can_be_used_for_gpio(struct pinctrl_dev *pctldev,
>>> unsigned pin)
>>> +{
>>> +    struct pin_desc *desc = pin_desc_get(pctldev, pin);
>>> +    const struct pinmux_ops *ops = pctldev->desc->pmxops;
>>> +
>>> +    if (!desc) {
>>> +        dev_dbg(pctldev->dev,
>>> +            "pin %u is not registered so it cannot be requested\n",
>>> +            pin);
>>> +        return true;
>> This return value looks strange to me.
>
> Basically, it's just the reversed return value you returned in the
> original patch,
> It means in this context that if the pin is not owned by a
> pin-controller it can be used for GPIO.
As long as the provided pin is valid. Btw shouldn't we change the logic
in the debug message?
>
> Thanks,
> Ramon.
>
>>
>> Stefan
>>

Re: [PATCH v2] gpiolib: Take MUX usage into account

[Ramon Fried]

On Tue, 2019-08-13 at 18:32 +0200, Stefan Wahren wrote:
> Am 13.08.19 um 08:10 schrieb Fried, Ramon:
> > On 8/13/2019 08:38, Stefan Wahren wrote:
> > > Hi Ramon,
> > > 
> > > On 13.08.19 03:42, Ramon Fried wrote:
> > > > From: Stefan Wahren <stefan.wahren@i2se.com>
> > > > 
> > > > The user space like gpioinfo only see the GPIO usage but not
> > > > the
> > > > MUX usage (e.g. I2C or SPI usage) of a pin. As a user we want
> > > > to
> > > > know which
> > > > pin is free/safe to use. So take the MUX usage of strict pinmux
> > > > controllers
> > > > into account to get a more realistic view for ioctl
> > > > GPIO_GET_LINEINFO_IOCTL.
> > > > 
> > > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > > > Tested-by: Ramon Fried <rfried.dev@gmail.com>
> > > > Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
> > > > ---
> > > > v2: Address review from linus:
> > > > * ** Please notive logic was reversed **
> > > > * renamed pinctrl_gpio_is_in_use() to
> > > > pinctrl_gpio_can_use_line()
> > > > * renamed pinmux_is_in_use() to pinmux_can_be_used_for_gpio()
> > > > * changed dev_err to dev_dbg (Linus suggested removing it
> > > > altogether, I
> > > >    find it better to keep it for debug).
> > > thanks for taking care of this.
> > > >   drivers/gpio/gpiolib.c           |  3 ++-
> > > >   drivers/pinctrl/core.c           | 28
> > > > ++++++++++++++++++++++++++++
> > > >   drivers/pinctrl/pinmux.c         | 27
> > > > +++++++++++++++++++++++++++
> > > >   drivers/pinctrl/pinmux.h         |  8 ++++++++
> > > >   include/linux/pinctrl/consumer.h |  6 ++++++
> > > >   5 files changed, 71 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > > > index f497003f119c..52937bf8e514 100644
> > > > --- a/drivers/gpio/gpiolib.c
> > > > +++ b/drivers/gpio/gpiolib.c
> > > > @@ -1084,7 +1084,8 @@ static long gpio_ioctl(struct file *filp,
> > > > unsigned int cmd, unsigned long arg)
> > > >               test_bit(FLAG_IS_HOGGED, &desc->flags) ||
> > > >               test_bit(FLAG_USED_AS_IRQ, &desc->flags) ||
> > > >               test_bit(FLAG_EXPORT, &desc->flags) ||
> > > > -            test_bit(FLAG_SYSFS, &desc->flags))
> > > > +            test_bit(FLAG_SYSFS, &desc->flags) ||
> > > > +            !pinctrl_gpio_can_use_line(chip->base +
> > > > lineinfo.line_offset))
> > > >               lineinfo.flags |= GPIOLINE_FLAG_KERNEL;
> > > >           if (test_bit(FLAG_IS_OUT, &desc->flags))
> > > >               lineinfo.flags |= GPIOLINE_FLAG_IS_OUT;
> > > > diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
> > > > index b70df27874d1..2bbd8ee93507 100644
> > > > --- a/drivers/pinctrl/core.c
> > > > +++ b/drivers/pinctrl/core.c
> > > > @@ -736,6 +736,34 @@ int pinctrl_get_group_selector(struct
> > > > pinctrl_dev *pctldev,
> > > >       return -EINVAL;
> > > >   }
> > > >   +bool pinctrl_gpio_can_use_line(unsigned gpio)
> > > > +{
> > > > +    struct pinctrl_dev *pctldev;
> > > > +    struct pinctrl_gpio_range *range;
> > > > +    bool result;
> > > > +    int pin;
> > > > +
> > > > +    /*
> > > > +     * Try to obtain GPIO range, if it fails
> > > > +     * we're probably dealing with GPIO driver
> > > > +     * without a backing pin controller - bail out.
> > > > +     */
> > > > +    if (pinctrl_get_device_gpio_range(gpio, &pctldev, &range))
> > > > +        return true;
> > > > +
> > > > +    mutex_lock(&pctldev->mutex);
> > > > +
> > > > +    /* Convert to the pin controllers number space */
> > > > +    pin = gpio_to_pin(range, gpio);
> > > > +
> > > > +    result = pinmux_can_be_used_for_gpio(pctldev, pin);
> > > > +
> > > > +    mutex_unlock(&pctldev->mutex);
> > > > +
> > > > +    return result;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(pinctrl_gpio_can_use_line);
> > > > +
> > > >   /**
> > > >    * pinctrl_gpio_request() - request a single pin to be used
> > > > as GPIO
> > > >    * @gpio: the GPIO pin number from the GPIO subsystem number
> > > > space
> > > > diff --git a/drivers/pinctrl/pinmux.c
> > > > b/drivers/pinctrl/pinmux.c
> > > > index 020e54f843f9..7e42a5738d82 100644
> > > > --- a/drivers/pinctrl/pinmux.c
> > > > +++ b/drivers/pinctrl/pinmux.c
> > > > @@ -70,6 +70,33 @@ int pinmux_validate_map(const struct
> > > > pinctrl_map
> > > > *map, int i)
> > > >       return 0;
> > > >   }
> > > >   +/**
> > > > + * pinmux_can_be_used_for_gpio() - check if a specific pin
> > > > + *    is either muxed to a different function or used as gpio.
> > > > + *
> > > > + * @pin: the pin number in the global pin space
> > > > + *
> > > > + * Controllers not defined as strict will always return true,
> > > > + * menaning that the gpio can be used.
> > > > + */
> > > > +bool pinmux_can_be_used_for_gpio(struct pinctrl_dev *pctldev,
> > > > unsigned pin)
> > > > +{
> > > > +    struct pin_desc *desc = pin_desc_get(pctldev, pin);
> > > > +    const struct pinmux_ops *ops = pctldev->desc->pmxops;
> > > > +
> > > > +    if (!desc) {
> > > > +        dev_dbg(pctldev->dev,
> > > > +            "pin %u is not registered so it cannot be
> > > > requested\n",
> > > > +            pin);
> > > > +        return true;
> > > This return value looks strange to me.
> > 
> > Basically, it's just the reversed return value you returned in the
> > original patch,
> > It means in this context that if the pin is not owned by a
> > pin-controller it can be used for GPIO.
> As long as the provided pin is valid. Btw shouldn't we change the
> logic
> in the debug message?
Good catch. yes we should.
I'll send V3 shortly.
Thanks,
Ramon.
> > Thanks,
> > Ramon.
> > 
> > > Stefan
> > >