This patch series adds the following fixes to the iProc GPIO driver
- Fix Warning message given for shared irqchip data structure
- Fix pinconfig of pull-up/down and drive strength for AON/CRMU GPIOs
This patch set is based on Linux-5.2-rc4.
Changes from v1:
- Add Fixes tags in both patches
Li Jin (1):
gpio: iproc-gpio: Fix incorrect pinconf configurations
Rayagonda Kokatanur (1):
gpio: iproc-gpio: Handle interrupts for multiple instances
drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 117 +++++++++++++++++++++++--------
1 file changed, 88 insertions(+), 29 deletions(-)
--
2.7.4
From: Li Jin <[email protected]>
Fix drive strength for AON/CRMU controller; fix pull-up/down setting
for CCM/CDRU controller.
Fixes: 616043d58a89 ("pinctrl: Rename gpio driver from cygnus to iproc")
Signed-off-by: Li Jin <[email protected]>
---
drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 96 +++++++++++++++++++++++++-------
1 file changed, 77 insertions(+), 19 deletions(-)
diff --git a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
index b70058c..20b9864 100644
--- a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
+++ b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
@@ -54,8 +54,12 @@
/* drive strength control for ASIU GPIO */
#define IPROC_GPIO_ASIU_DRV0_CTRL_OFFSET 0x58
-/* drive strength control for CCM/CRMU (AON) GPIO */
-#define IPROC_GPIO_DRV0_CTRL_OFFSET 0x00
+/* pinconf for CCM GPIO */
+#define IPROC_GPIO_PULL_DN_OFFSET 0x10
+#define IPROC_GPIO_PULL_UP_OFFSET 0x14
+
+/* pinconf for CRMU(aon) GPIO and CCM GPIO*/
+#define IPROC_GPIO_DRV_CTRL_OFFSET 0x00
#define GPIO_BANK_SIZE 0x200
#define NGPIOS_PER_BANK 32
@@ -76,6 +80,12 @@ enum iproc_pinconf_param {
IPROC_PINCON_MAX,
};
+enum iproc_pinconf_ctrl_type {
+ IOCTRL_TYPE_AON = 1,
+ IOCTRL_TYPE_CDRU,
+ IOCTRL_TYPE_INVALID,
+};
+
/*
* Iproc GPIO core
*
@@ -100,6 +110,7 @@ struct iproc_gpio {
void __iomem *base;
void __iomem *io_ctrl;
+ enum iproc_pinconf_ctrl_type io_ctrl_type;
raw_spinlock_t lock;
@@ -461,20 +472,44 @@ static const struct pinctrl_ops iproc_pctrl_ops = {
static int iproc_gpio_set_pull(struct iproc_gpio *chip, unsigned gpio,
bool disable, bool pull_up)
{
+ void __iomem *base;
unsigned long flags;
+ unsigned int shift;
+ u32 val_1, val_2;
raw_spin_lock_irqsave(&chip->lock, flags);
-
- if (disable) {
- iproc_set_bit(chip, IPROC_GPIO_RES_EN_OFFSET, gpio, false);
+ if (chip->io_ctrl_type == IOCTRL_TYPE_CDRU) {
+ base = chip->io_ctrl;
+ shift = IPROC_GPIO_SHIFT(gpio);
+
+ val_1 = readl(base + IPROC_GPIO_PULL_UP_OFFSET);
+ val_2 = readl(base + IPROC_GPIO_PULL_DN_OFFSET);
+ if (disable) {
+ /* no pull-up or pull-down */
+ val_1 &= ~BIT(shift);
+ val_2 &= ~BIT(shift);
+ } else if (pull_up) {
+ val_1 |= BIT(shift);
+ val_2 &= ~BIT(shift);
+ } else {
+ val_1 &= ~BIT(shift);
+ val_2 |= BIT(shift);
+ }
+ writel(val_1, base + IPROC_GPIO_PULL_UP_OFFSET);
+ writel(val_2, base + IPROC_GPIO_PULL_DN_OFFSET);
} else {
- iproc_set_bit(chip, IPROC_GPIO_PAD_RES_OFFSET, gpio,
- pull_up);
- iproc_set_bit(chip, IPROC_GPIO_RES_EN_OFFSET, gpio, true);
+ if (disable) {
+ iproc_set_bit(chip, IPROC_GPIO_RES_EN_OFFSET, gpio,
+ false);
+ } else {
+ iproc_set_bit(chip, IPROC_GPIO_PAD_RES_OFFSET, gpio,
+ pull_up);
+ iproc_set_bit(chip, IPROC_GPIO_RES_EN_OFFSET, gpio,
+ true);
+ }
}
raw_spin_unlock_irqrestore(&chip->lock, flags);
-
dev_dbg(chip->dev, "gpio:%u set pullup:%d\n", gpio, pull_up);
return 0;
@@ -483,14 +518,35 @@ static int iproc_gpio_set_pull(struct iproc_gpio *chip, unsigned gpio,
static void iproc_gpio_get_pull(struct iproc_gpio *chip, unsigned gpio,
bool *disable, bool *pull_up)
{
+ void __iomem *base;
unsigned long flags;
+ unsigned int shift;
+ u32 val_1, val_2;
raw_spin_lock_irqsave(&chip->lock, flags);
- *disable = !iproc_get_bit(chip, IPROC_GPIO_RES_EN_OFFSET, gpio);
- *pull_up = iproc_get_bit(chip, IPROC_GPIO_PAD_RES_OFFSET, gpio);
+ if (chip->io_ctrl_type == IOCTRL_TYPE_CDRU) {
+ base = chip->io_ctrl;
+ shift = IPROC_GPIO_SHIFT(gpio);
+
+ val_1 = readl(base + IPROC_GPIO_PULL_UP_OFFSET) & BIT(shift);
+ val_2 = readl(base + IPROC_GPIO_PULL_DN_OFFSET) & BIT(shift);
+
+ *pull_up = val_1 ? true : false;
+ *disable = (val_1 | val_2) ? false : true;
+
+ } else {
+ *disable = !iproc_get_bit(chip, IPROC_GPIO_RES_EN_OFFSET, gpio);
+ *pull_up = iproc_get_bit(chip, IPROC_GPIO_PAD_RES_OFFSET, gpio);
+ }
raw_spin_unlock_irqrestore(&chip->lock, flags);
}
+#define DRV_STRENGTH_OFFSET(gpio, bit, type) ((type) == IOCTRL_TYPE_AON ? \
+ ((2 - (bit)) * 4 + IPROC_GPIO_DRV_CTRL_OFFSET) : \
+ ((type) == IOCTRL_TYPE_CDRU) ? \
+ ((bit) * 4 + IPROC_GPIO_DRV_CTRL_OFFSET) : \
+ ((bit) * 4 + IPROC_GPIO_REG(gpio, IPROC_GPIO_ASIU_DRV0_CTRL_OFFSET)))
+
static int iproc_gpio_set_strength(struct iproc_gpio *chip, unsigned gpio,
unsigned strength)
{
@@ -505,11 +561,8 @@ static int iproc_gpio_set_strength(struct iproc_gpio *chip, unsigned gpio,
if (chip->io_ctrl) {
base = chip->io_ctrl;
- offset = IPROC_GPIO_DRV0_CTRL_OFFSET;
} else {
base = chip->base;
- offset = IPROC_GPIO_REG(gpio,
- IPROC_GPIO_ASIU_DRV0_CTRL_OFFSET);
}
shift = IPROC_GPIO_SHIFT(gpio);
@@ -520,11 +573,11 @@ static int iproc_gpio_set_strength(struct iproc_gpio *chip, unsigned gpio,
raw_spin_lock_irqsave(&chip->lock, flags);
strength = (strength / 2) - 1;
for (i = 0; i < GPIO_DRV_STRENGTH_BITS; i++) {
+ offset = DRV_STRENGTH_OFFSET(gpio, i, chip->io_ctrl_type);
val = readl(base + offset);
val &= ~BIT(shift);
val |= ((strength >> i) & 0x1) << shift;
writel(val, base + offset);
- offset += 4;
}
raw_spin_unlock_irqrestore(&chip->lock, flags);
@@ -541,11 +594,8 @@ static int iproc_gpio_get_strength(struct iproc_gpio *chip, unsigned gpio,
if (chip->io_ctrl) {
base = chip->io_ctrl;
- offset = IPROC_GPIO_DRV0_CTRL_OFFSET;
} else {
base = chip->base;
- offset = IPROC_GPIO_REG(gpio,
- IPROC_GPIO_ASIU_DRV0_CTRL_OFFSET);
}
shift = IPROC_GPIO_SHIFT(gpio);
@@ -553,10 +603,10 @@ static int iproc_gpio_get_strength(struct iproc_gpio *chip, unsigned gpio,
raw_spin_lock_irqsave(&chip->lock, flags);
*strength = 0;
for (i = 0; i < GPIO_DRV_STRENGTH_BITS; i++) {
+ offset = DRV_STRENGTH_OFFSET(gpio, i, chip->io_ctrl_type);
val = readl(base + offset) & BIT(shift);
val >>= shift;
*strength += (val << i);
- offset += 4;
}
/* convert to mA */
@@ -734,6 +784,7 @@ static int iproc_gpio_probe(struct platform_device *pdev)
u32 ngpios, pinconf_disable_mask = 0;
int irq, ret;
bool no_pinconf = false;
+ enum iproc_pinconf_ctrl_type io_ctrl_type = IOCTRL_TYPE_INVALID;
/* NSP does not support drive strength config */
if (of_device_is_compatible(dev->of_node, "brcm,iproc-nsp-gpio"))
@@ -764,8 +815,15 @@ static int iproc_gpio_probe(struct platform_device *pdev)
dev_err(dev, "unable to map I/O memory\n");
return PTR_ERR(chip->io_ctrl);
}
+ if (of_device_is_compatible(dev->of_node,
+ "brcm,cygnus-ccm-gpio"))
+ io_ctrl_type = IOCTRL_TYPE_CDRU;
+ else
+ io_ctrl_type = IOCTRL_TYPE_AON;
}
+ chip->io_ctrl_type = io_ctrl_type;
+
if (of_property_read_u32(dev->of_node, "ngpios", &ngpios)) {
dev_err(&pdev->dev, "missing ngpios DT property\n");
return -ENODEV;
--
2.7.4
From: Rayagonda Kokatanur <[email protected]>
When multiple instance of iproc-gpio chips are present, a fix up
message[1] is printed during the probe of second and later instances.
This issue is because driver sharing same irq_chip data structure
among multiple instances of driver.
Fix this by allocating irq_chip data structure per instance of
iproc-gpio.
[1] fix up message addressed by this patch
[ 7.862208] gpio gpiochip2: (689d0000.gpio): detected irqchip that
is shared with multiple gpiochips: please fix the driver.
Fixes: 616043d58a89 ("pinctrl: Rename gpio driver from cygnus to iproc")
Signed-off-by: Rayagonda Kokatanur <[email protected]>
---
drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
index 20b9864..8729f47 100644
--- a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
+++ b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
@@ -114,6 +114,7 @@ struct iproc_gpio {
raw_spinlock_t lock;
+ struct irq_chip irqchip;
struct gpio_chip gc;
unsigned num_banks;
@@ -302,14 +303,6 @@ static int iproc_gpio_irq_set_type(struct irq_data *d, unsigned int type)
return 0;
}
-static struct irq_chip iproc_gpio_irq_chip = {
- .name = "bcm-iproc-gpio",
- .irq_ack = iproc_gpio_irq_ack,
- .irq_mask = iproc_gpio_irq_mask,
- .irq_unmask = iproc_gpio_irq_unmask,
- .irq_set_type = iproc_gpio_irq_set_type,
-};
-
/*
* Request the Iproc IOMUX pinmux controller to mux individual pins to GPIO
*/
@@ -875,14 +868,22 @@ static int iproc_gpio_probe(struct platform_device *pdev)
/* optional GPIO interrupt support */
irq = platform_get_irq(pdev, 0);
if (irq) {
- ret = gpiochip_irqchip_add(gc, &iproc_gpio_irq_chip, 0,
+ chip->irqchip.name = "bcm-iproc-gpio";
+ chip->irqchip.irq_ack = iproc_gpio_irq_ack;
+ chip->irqchip.irq_mask = iproc_gpio_irq_mask;
+ chip->irqchip.irq_unmask = iproc_gpio_irq_unmask;
+ chip->irqchip.irq_set_type = iproc_gpio_irq_set_type;
+ chip->irqchip.irq_enable = iproc_gpio_irq_unmask;
+ chip->irqchip.irq_disable = iproc_gpio_irq_mask;
+
+ ret = gpiochip_irqchip_add(gc, &chip->irqchip, 0,
handle_simple_irq, IRQ_TYPE_NONE);
if (ret) {
dev_err(dev, "no GPIO irqchip\n");
goto err_rm_gpiochip;
}
- gpiochip_set_chained_irqchip(gc, &iproc_gpio_irq_chip, irq,
+ gpiochip_set_chained_irqchip(gc, &chip->irqchip, irq,
iproc_gpio_irq_handler);
}
--
2.7.4
On Thu, Aug 29, 2019 at 5:52 AM Srinath Mannam
<[email protected]> wrote:
> From: Li Jin <[email protected]>
>
> Fix drive strength for AON/CRMU controller; fix pull-up/down setting
> for CCM/CDRU controller.
>
> Fixes: 616043d58a89 ("pinctrl: Rename gpio driver from cygnus to iproc")
> Signed-off-by: Li Jin <[email protected]>
No response from maintainers for two weeks, so patch applied.
Yours,
Linus Walleij
On Thu, Aug 29, 2019 at 5:52 AM Srinath Mannam
<[email protected]> wrote:
> From: Rayagonda Kokatanur <[email protected]>
>
> When multiple instance of iproc-gpio chips are present, a fix up
> message[1] is printed during the probe of second and later instances.
>
> This issue is because driver sharing same irq_chip data structure
> among multiple instances of driver.
>
> Fix this by allocating irq_chip data structure per instance of
> iproc-gpio.
>
> [1] fix up message addressed by this patch
> [ 7.862208] gpio gpiochip2: (689d0000.gpio): detected irqchip that
> is shared with multiple gpiochips: please fix the driver.
>
> Fixes: 616043d58a89 ("pinctrl: Rename gpio driver from cygnus to iproc")
> Signed-off-by: Rayagonda Kokatanur <[email protected]>
Patch applied, I had to rewrite it a bit to fit the new code that
set up the irqchip when adding the gpio_chip, please check that
the result works.
Yours,
Linus Walleij
On 9/11/19 2:34 AM, Linus Walleij wrote:
> On Thu, Aug 29, 2019 at 5:52 AM Srinath Mannam
> <[email protected]> wrote:
>
>> From: Li Jin <[email protected]>
>>
>> Fix drive strength for AON/CRMU controller; fix pull-up/down setting
>> for CCM/CDRU controller.
>>
>> Fixes: 616043d58a89 ("pinctrl: Rename gpio driver from cygnus to iproc")
>> Signed-off-by: Li Jin <[email protected]>
>
> No response from maintainers for two weeks, so patch applied.
>
These patches were actually all internally reviewed by Broadcom
maintainers before sending out to the mailing list.
Obviously you wouldn't know about that, :)
One of us should have explicitly given our ACK, sorry...
Thanks,
Ray
> Yours,
> Linus Walleij
>
On Wed, Sep 11, 2019 at 5:55 PM Ray Jui <[email protected]> wrote:
> These patches were actually all internally reviewed by Broadcom
> maintainers before sending out to the mailing list.
>
> Obviously you wouldn't know about that, :)
>
> One of us should have explicitly given our ACK, sorry...
It's fine, the process is not perfect.
Thanks!
Linus Walleij
Hi Linus,
We have tested patch with your changes, it works fine.
Thanks a lot for all the help.
Regards,
Srinath.
On Wed, Sep 11, 2019 at 3:13 PM Linus Walleij <[email protected]> wrote:
>
> On Thu, Aug 29, 2019 at 5:52 AM Srinath Mannam
> <[email protected]> wrote:
>
> > From: Rayagonda Kokatanur <[email protected]>
> >
> > When multiple instance of iproc-gpio chips are present, a fix up
> > message[1] is printed during the probe of second and later instances.
> >
> > This issue is because driver sharing same irq_chip data structure
> > among multiple instances of driver.
> >
> > Fix this by allocating irq_chip data structure per instance of
> > iproc-gpio.
> >
> > [1] fix up message addressed by this patch
> > [ 7.862208] gpio gpiochip2: (689d0000.gpio): detected irqchip that
> > is shared with multiple gpiochips: please fix the driver.
> >
> > Fixes: 616043d58a89 ("pinctrl: Rename gpio driver from cygnus to iproc")
> > Signed-off-by: Rayagonda Kokatanur <[email protected]>
>
> Patch applied, I had to rewrite it a bit to fit the new code that
> set up the irqchip when adding the gpio_chip, please check that
> the result works.
>
> Yours,
> Linus Walleij