* [RFT][PATCH 1/2] gpio: bcm-kona: Return error if requesting an already taken gpio
@ 2015-04-10 1:28 Axel Lin
2015-04-10 1:30 ` [RFT][PATCH 2/2] gpio: bcm-kona: Implement get_direction callback Axel Lin
2015-04-10 16:05 ` [RFT][PATCH 1/2] gpio: bcm-kona: Return error if requesting an already taken gpio Ray Jui
0 siblings, 2 replies; 7+ messages in thread
From: Axel Lin @ 2015-04-10 1:28 UTC (permalink / raw)
To: Linus Walleij
Cc: Ray Jui, Alexandre Courbot, bcm-kernel-feedback-list, linux-gpio
If a gpio is already taken, .gpio_request should return error.
Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
drivers/gpio/gpio-bcm-kona.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c
index b164ce8..632352d 100644
--- a/drivers/gpio/gpio-bcm-kona.c
+++ b/drivers/gpio/gpio-bcm-kona.c
@@ -106,7 +106,7 @@ static void bcm_kona_gpio_lock_gpio(struct bcm_kona_gpio *kona_gpio,
spin_unlock_irqrestore(&kona_gpio->lock, flags);
}
-static void bcm_kona_gpio_unlock_gpio(struct bcm_kona_gpio *kona_gpio,
+static int bcm_kona_gpio_unlock_gpio(struct bcm_kona_gpio *kona_gpio,
unsigned gpio)
{
u32 val;
@@ -116,10 +116,17 @@ static void bcm_kona_gpio_unlock_gpio(struct bcm_kona_gpio *kona_gpio,
spin_lock_irqsave(&kona_gpio->lock, flags);
val = readl(kona_gpio->reg_base + GPIO_PWD_STATUS(bank_id));
+ if (!(val & BIT(gpio))) {
+ spin_unlock_irqrestore(&kona_gpio->lock, flags);
+ return -EBUSY;
+ }
+
val &= ~BIT(gpio);
bcm_kona_gpio_write_lock_regs(kona_gpio->reg_base, bank_id, val);
spin_unlock_irqrestore(&kona_gpio->lock, flags);
+
+ return 0;
}
static void bcm_kona_gpio_set(struct gpio_chip *chip, unsigned gpio, int value)
@@ -185,8 +192,7 @@ static int bcm_kona_gpio_request(struct gpio_chip *chip, unsigned gpio)
{
struct bcm_kona_gpio *kona_gpio = to_kona_gpio(chip);
- bcm_kona_gpio_unlock_gpio(kona_gpio, gpio);
- return 0;
+ return bcm_kona_gpio_unlock_gpio(kona_gpio, gpio);
}
static void bcm_kona_gpio_free(struct gpio_chip *chip, unsigned gpio)
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFT][PATCH 2/2] gpio: bcm-kona: Implement get_direction callback
2015-04-10 1:28 [RFT][PATCH 1/2] gpio: bcm-kona: Return error if requesting an already taken gpio Axel Lin
@ 2015-04-10 1:30 ` Axel Lin
2015-04-10 16:13 ` Ray Jui
2015-04-10 16:05 ` [RFT][PATCH 1/2] gpio: bcm-kona: Return error if requesting an already taken gpio Ray Jui
1 sibling, 1 reply; 7+ messages in thread
From: Axel Lin @ 2015-04-10 1:30 UTC (permalink / raw)
To: Linus Walleij
Cc: Ray Jui, Alexandre Courbot, bcm-kernel-feedback-list, linux-gpio
Implement gpio_chip's get_direction() callback, that lets other drivers get
particular GPIOs direction using gpiod_get_direction().
Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
drivers/gpio/gpio-bcm-kona.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c
index 632352d..3649ed5 100644
--- a/drivers/gpio/gpio-bcm-kona.c
+++ b/drivers/gpio/gpio-bcm-kona.c
@@ -129,6 +129,14 @@ static int bcm_kona_gpio_unlock_gpio(struct bcm_kona_gpio *kona_gpio,
return 0;
}
+static int bcm_kona_gpio_get_dir(struct gpio_chip *chip, unsigned gpio)
+{
+ struct bcm_kona_gpio *kona_gpio = to_kona_gpio(chip);
+ void __iomem *reg_base = kona_gpio->reg_base;
+
+ return readl(reg_base + GPIO_CONTROL(gpio)) & GPIO_GPCTR0_IOTR_MASK;
+}
+
static void bcm_kona_gpio_set(struct gpio_chip *chip, unsigned gpio, int value)
{
struct bcm_kona_gpio *kona_gpio;
@@ -142,12 +150,8 @@ static void bcm_kona_gpio_set(struct gpio_chip *chip, unsigned gpio, int value)
reg_base = kona_gpio->reg_base;
spin_lock_irqsave(&kona_gpio->lock, flags);
- /* determine the GPIO pin direction */
- val = readl(reg_base + GPIO_CONTROL(gpio));
- val &= GPIO_GPCTR0_IOTR_MASK;
-
/* this function only applies to output pin */
- if (GPIO_GPCTR0_IOTR_CMD_INPUT == val)
+ if (bcm_kona_gpio_get_dir(chip, gpio) == GPIOF_DIR_IN)
goto out;
reg_offset = value ? GPIO_OUT_SET(bank_id) : GPIO_OUT_CLEAR(bank_id);
@@ -173,13 +177,12 @@ static int bcm_kona_gpio_get(struct gpio_chip *chip, unsigned gpio)
reg_base = kona_gpio->reg_base;
spin_lock_irqsave(&kona_gpio->lock, flags);
- /* determine the GPIO pin direction */
- val = readl(reg_base + GPIO_CONTROL(gpio));
- val &= GPIO_GPCTR0_IOTR_MASK;
+ if (bcm_kona_gpio_get_dir(chip, gpio) == GPIOF_DIR_IN)
+ reg_offset = GPIO_IN_STATUS(bank_id);
+ else
+ reg_offset = GPIO_OUT_STATUS(bank_id);
/* read the GPIO bank status */
- reg_offset = (GPIO_GPCTR0_IOTR_CMD_INPUT == val) ?
- GPIO_IN_STATUS(bank_id) : GPIO_OUT_STATUS(bank_id);
val = readl(reg_base + reg_offset);
spin_unlock_irqrestore(&kona_gpio->lock, flags);
@@ -316,6 +319,7 @@ static struct gpio_chip template_chip = {
.owner = THIS_MODULE,
.request = bcm_kona_gpio_request,
.free = bcm_kona_gpio_free,
+ .get_direction = bcm_kona_gpio_get_dir,
.direction_input = bcm_kona_gpio_direction_input,
.get = bcm_kona_gpio_get,
.direction_output = bcm_kona_gpio_direction_output,
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFT][PATCH 2/2] gpio: bcm-kona: Implement get_direction callback
2015-04-10 1:30 ` [RFT][PATCH 2/2] gpio: bcm-kona: Implement get_direction callback Axel Lin
@ 2015-04-10 16:13 ` Ray Jui
2015-04-10 23:13 ` Axel Lin
0 siblings, 1 reply; 7+ messages in thread
From: Ray Jui @ 2015-04-10 16:13 UTC (permalink / raw)
To: Axel Lin, Linus Walleij
Cc: Alexandre Courbot, bcm-kernel-feedback-list, linux-gpio
Hi Axel,
On 4/9/2015 6:30 PM, Axel Lin wrote:
> Implement gpio_chip's get_direction() callback, that lets other drivers get
> particular GPIOs direction using gpiod_get_direction().
>
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---
> drivers/gpio/gpio-bcm-kona.c | 24 ++++++++++++++----------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c
> index 632352d..3649ed5 100644
> --- a/drivers/gpio/gpio-bcm-kona.c
> +++ b/drivers/gpio/gpio-bcm-kona.c
> @@ -129,6 +129,14 @@ static int bcm_kona_gpio_unlock_gpio(struct bcm_kona_gpio *kona_gpio,
> return 0;
> }
>
> +static int bcm_kona_gpio_get_dir(struct gpio_chip *chip, unsigned gpio)
> +{
> + struct bcm_kona_gpio *kona_gpio = to_kona_gpio(chip);
> + void __iomem *reg_base = kona_gpio->reg_base;
> +
> + return readl(reg_base + GPIO_CONTROL(gpio)) & GPIO_GPCTR0_IOTR_MASK;
You are making assumption of actual values of GPIOF_DIR_IN and
GPIOF_DIR_OUT. I think it would be better to return GPIOF_DIR_IN or
GPIOF_DIR_OUT based on the register readling.
In addition, with your change, when bcm_kona_gpio_get_dir is called
within bcm_kona_gpio_set and bcm_kona_gpio_get, it's protected by the
kona_gpio->lock spinlock. But no lock protection while being invoked
through the upper layer get_direction callback.
Thanks,
Ray
> +}
> +
> static void bcm_kona_gpio_set(struct gpio_chip *chip, unsigned gpio, int value)
> {
> struct bcm_kona_gpio *kona_gpio;
> @@ -142,12 +150,8 @@ static void bcm_kona_gpio_set(struct gpio_chip *chip, unsigned gpio, int value)
> reg_base = kona_gpio->reg_base;
> spin_lock_irqsave(&kona_gpio->lock, flags);
>
> - /* determine the GPIO pin direction */
> - val = readl(reg_base + GPIO_CONTROL(gpio));
> - val &= GPIO_GPCTR0_IOTR_MASK;
> -
> /* this function only applies to output pin */
> - if (GPIO_GPCTR0_IOTR_CMD_INPUT == val)
> + if (bcm_kona_gpio_get_dir(chip, gpio) == GPIOF_DIR_IN)
> goto out;
>
> reg_offset = value ? GPIO_OUT_SET(bank_id) : GPIO_OUT_CLEAR(bank_id);
> @@ -173,13 +177,12 @@ static int bcm_kona_gpio_get(struct gpio_chip *chip, unsigned gpio)
> reg_base = kona_gpio->reg_base;
> spin_lock_irqsave(&kona_gpio->lock, flags);
>
> - /* determine the GPIO pin direction */
> - val = readl(reg_base + GPIO_CONTROL(gpio));
> - val &= GPIO_GPCTR0_IOTR_MASK;
> + if (bcm_kona_gpio_get_dir(chip, gpio) == GPIOF_DIR_IN)
> + reg_offset = GPIO_IN_STATUS(bank_id);
> + else
> + reg_offset = GPIO_OUT_STATUS(bank_id);
>
> /* read the GPIO bank status */
> - reg_offset = (GPIO_GPCTR0_IOTR_CMD_INPUT == val) ?
> - GPIO_IN_STATUS(bank_id) : GPIO_OUT_STATUS(bank_id);
> val = readl(reg_base + reg_offset);
>
> spin_unlock_irqrestore(&kona_gpio->lock, flags);
> @@ -316,6 +319,7 @@ static struct gpio_chip template_chip = {
> .owner = THIS_MODULE,
> .request = bcm_kona_gpio_request,
> .free = bcm_kona_gpio_free,
> + .get_direction = bcm_kona_gpio_get_dir,
> .direction_input = bcm_kona_gpio_direction_input,
> .get = bcm_kona_gpio_get,
> .direction_output = bcm_kona_gpio_direction_output,
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFT][PATCH 2/2] gpio: bcm-kona: Implement get_direction callback
2015-04-10 16:13 ` Ray Jui
@ 2015-04-10 23:13 ` Axel Lin
2015-04-13 4:15 ` Ray Jui
0 siblings, 1 reply; 7+ messages in thread
From: Axel Lin @ 2015-04-10 23:13 UTC (permalink / raw)
To: Ray Jui
Cc: Linus Walleij, Alexandre Courbot, bcm-kernel-feedback-list, linux-gpio
2015-04-11 0:13 GMT+08:00 Ray Jui <rjui@broadcom.com>:
> Hi Axel,
>
> On 4/9/2015 6:30 PM, Axel Lin wrote:
>> Implement gpio_chip's get_direction() callback, that lets other drivers get
>> particular GPIOs direction using gpiod_get_direction().
>>
>> Signed-off-by: Axel Lin <axel.lin@ingics.com>
>> ---
>> drivers/gpio/gpio-bcm-kona.c | 24 ++++++++++++++----------
>> 1 file changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c
>> index 632352d..3649ed5 100644
>> --- a/drivers/gpio/gpio-bcm-kona.c
>> +++ b/drivers/gpio/gpio-bcm-kona.c
>> @@ -129,6 +129,14 @@ static int bcm_kona_gpio_unlock_gpio(struct bcm_kona_gpio *kona_gpio,
>> return 0;
>> }
>>
>> +static int bcm_kona_gpio_get_dir(struct gpio_chip *chip, unsigned gpio)
>> +{
>> + struct bcm_kona_gpio *kona_gpio = to_kona_gpio(chip);
>> + void __iomem *reg_base = kona_gpio->reg_base;
>> +
>> + return readl(reg_base + GPIO_CONTROL(gpio)) & GPIO_GPCTR0_IOTR_MASK;
>
> You are making assumption of actual values of GPIOF_DIR_IN and
> GPIOF_DIR_OUT. I think it would be better to return GPIOF_DIR_IN or
> GPIOF_DIR_OUT based on the register readling.
Yes, it's because GPIO_GPCTR0_IOTR_CMD_INPUT == GPIOF_DIR_IN and
GPIO_GPCTR0_IOTR_CMD_0UTPUT == GPIOF_DIR_OUT.
I can change it to below if you prefer, but current patch is shorter
and logically exactly the same.
if (readl(reg_base + GPIO_CONTROL(gpio)) & GPIO_GPCTR0_IOTR_MASK)
return GPIOF_DIR_IN;
else
return GPIOF_DIR_OUT;
>
> In addition, with your change, when bcm_kona_gpio_get_dir is called
> within bcm_kona_gpio_set and bcm_kona_gpio_get, it's protected by the
> kona_gpio->lock spinlock. But no lock protection while being invoked
> through the upper layer get_direction callback.
It is because bcm_kona_gpio_get_dir is a single register read.
In bcm_kona_gpio_set/bcm_kona_gpio_get, the lock is to ensure
read/write to both the DIR and LEVEL registers so we won't set/get
wrong level value in wrong DIR status.
Regards,
Axel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFT][PATCH 2/2] gpio: bcm-kona: Implement get_direction callback
2015-04-10 23:13 ` Axel Lin
@ 2015-04-13 4:15 ` Ray Jui
0 siblings, 0 replies; 7+ messages in thread
From: Ray Jui @ 2015-04-13 4:15 UTC (permalink / raw)
To: Axel Lin
Cc: Linus Walleij, Alexandre Courbot, bcm-kernel-feedback-list, linux-gpio
Hi Axel,
On 4/10/2015 4:13 PM, Axel Lin wrote:
> 2015-04-11 0:13 GMT+08:00 Ray Jui <rjui@broadcom.com>:
>> Hi Axel,
>>
>> On 4/9/2015 6:30 PM, Axel Lin wrote:
>>> Implement gpio_chip's get_direction() callback, that lets other drivers get
>>> particular GPIOs direction using gpiod_get_direction().
>>>
>>> Signed-off-by: Axel Lin <axel.lin@ingics.com>
>>> ---
>>> drivers/gpio/gpio-bcm-kona.c | 24 ++++++++++++++----------
>>> 1 file changed, 14 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c
>>> index 632352d..3649ed5 100644
>>> --- a/drivers/gpio/gpio-bcm-kona.c
>>> +++ b/drivers/gpio/gpio-bcm-kona.c
>>> @@ -129,6 +129,14 @@ static int bcm_kona_gpio_unlock_gpio(struct bcm_kona_gpio *kona_gpio,
>>> return 0;
>>> }
>>>
>>> +static int bcm_kona_gpio_get_dir(struct gpio_chip *chip, unsigned gpio)
>>> +{
>>> + struct bcm_kona_gpio *kona_gpio = to_kona_gpio(chip);
>>> + void __iomem *reg_base = kona_gpio->reg_base;
>>> +
>>> + return readl(reg_base + GPIO_CONTROL(gpio)) & GPIO_GPCTR0_IOTR_MASK;
>>
>> You are making assumption of actual values of GPIOF_DIR_IN and
>> GPIOF_DIR_OUT. I think it would be better to return GPIOF_DIR_IN or
>> GPIOF_DIR_OUT based on the register readling.
>
> Yes, it's because GPIO_GPCTR0_IOTR_CMD_INPUT == GPIOF_DIR_IN and
> GPIO_GPCTR0_IOTR_CMD_0UTPUT == GPIOF_DIR_OUT.
>
> I can change it to below if you prefer, but current patch is shorter
> and logically exactly the same.
>
> if (readl(reg_base + GPIO_CONTROL(gpio)) & GPIO_GPCTR0_IOTR_MASK)
> return GPIOF_DIR_IN;
> else
> return GPIOF_DIR_OUT;
>
Or one line like:
return readl(reg_base + GPIO_CONTROL(gpio)) & GPIO_GPCTR0_IOTR_MASK
? GPIOF_DIR_IN : GPIOF_DIR_OUT;
>>
>> In addition, with your change, when bcm_kona_gpio_get_dir is called
>> within bcm_kona_gpio_set and bcm_kona_gpio_get, it's protected by the
>> kona_gpio->lock spinlock. But no lock protection while being invoked
>> through the upper layer get_direction callback.
>
> It is because bcm_kona_gpio_get_dir is a single register read.
> In bcm_kona_gpio_set/bcm_kona_gpio_get, the lock is to ensure
> read/write to both the DIR and LEVEL registers so we won't set/get
> wrong level value in wrong DIR status.
Yes, you are right. The locking part is fine!
>
> Regards,
> Axel
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFT][PATCH 1/2] gpio: bcm-kona: Return error if requesting an already taken gpio
2015-04-10 1:28 [RFT][PATCH 1/2] gpio: bcm-kona: Return error if requesting an already taken gpio Axel Lin
2015-04-10 1:30 ` [RFT][PATCH 2/2] gpio: bcm-kona: Implement get_direction callback Axel Lin
@ 2015-04-10 16:05 ` Ray Jui
2015-04-10 23:01 ` Axel Lin
1 sibling, 1 reply; 7+ messages in thread
From: Ray Jui @ 2015-04-10 16:05 UTC (permalink / raw)
To: Axel Lin, Linus Walleij
Cc: Alexandre Courbot, bcm-kernel-feedback-list, linux-gpio
Hi Axel,
On 4/9/2015 6:28 PM, Axel Lin wrote:
> If a gpio is already taken, .gpio_request should return error.
Isn't the protection logic already there at the higher layer in gpiolib?
Ray
>
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---
> drivers/gpio/gpio-bcm-kona.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c
> index b164ce8..632352d 100644
> --- a/drivers/gpio/gpio-bcm-kona.c
> +++ b/drivers/gpio/gpio-bcm-kona.c
> @@ -106,7 +106,7 @@ static void bcm_kona_gpio_lock_gpio(struct bcm_kona_gpio *kona_gpio,
> spin_unlock_irqrestore(&kona_gpio->lock, flags);
> }
>
> -static void bcm_kona_gpio_unlock_gpio(struct bcm_kona_gpio *kona_gpio,
> +static int bcm_kona_gpio_unlock_gpio(struct bcm_kona_gpio *kona_gpio,
> unsigned gpio)
> {
> u32 val;
> @@ -116,10 +116,17 @@ static void bcm_kona_gpio_unlock_gpio(struct bcm_kona_gpio *kona_gpio,
> spin_lock_irqsave(&kona_gpio->lock, flags);
>
> val = readl(kona_gpio->reg_base + GPIO_PWD_STATUS(bank_id));
> + if (!(val & BIT(gpio))) {
> + spin_unlock_irqrestore(&kona_gpio->lock, flags);
> + return -EBUSY;
> + }
> +
> val &= ~BIT(gpio);
> bcm_kona_gpio_write_lock_regs(kona_gpio->reg_base, bank_id, val);
>
> spin_unlock_irqrestore(&kona_gpio->lock, flags);
> +
> + return 0;
> }
>
> static void bcm_kona_gpio_set(struct gpio_chip *chip, unsigned gpio, int value)
> @@ -185,8 +192,7 @@ static int bcm_kona_gpio_request(struct gpio_chip *chip, unsigned gpio)
> {
> struct bcm_kona_gpio *kona_gpio = to_kona_gpio(chip);
>
> - bcm_kona_gpio_unlock_gpio(kona_gpio, gpio);
> - return 0;
> + return bcm_kona_gpio_unlock_gpio(kona_gpio, gpio);
> }
>
> static void bcm_kona_gpio_free(struct gpio_chip *chip, unsigned gpio)
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-04-13 4:15 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-10 1:28 [RFT][PATCH 1/2] gpio: bcm-kona: Return error if requesting an already taken gpio Axel Lin
2015-04-10 1:30 ` [RFT][PATCH 2/2] gpio: bcm-kona: Implement get_direction callback Axel Lin
2015-04-10 16:13 ` Ray Jui
2015-04-10 23:13 ` Axel Lin
2015-04-13 4:15 ` Ray Jui
2015-04-10 16:05 ` [RFT][PATCH 1/2] gpio: bcm-kona: Return error if requesting an already taken gpio Ray Jui
2015-04-10 23:01 ` Axel Lin
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.