LKML Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks
@ 2019-09-04  6:12 Rashmica Gupta
  2019-09-04  6:12 ` [PATCH 2/4] gpio/aspeed: Setup irqchip dynamically Rashmica Gupta
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Rashmica Gupta @ 2019-09-04  6:12 UTC (permalink / raw)
  To: linus.walleij, linux-gpio, bgolaszewski
  Cc: linux-arm-kernel, linux-aspeed, linux-kernel, joel, andrew,
	Rashmica Gupta

Fixes: 361b79119a4b7 ('gpio: Add Aspeed driver')

Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
---
 drivers/gpio/gpio-aspeed.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index 9defe25d4721..77752b2624e8 100644
--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c
@@ -1165,7 +1165,7 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev)
 	gpio->chip.base = -1;
 
 	/* Allocate a cache of the output registers */
-	banks = gpio->config->nr_gpios >> 5;
+	banks = (gpio->config->nr_gpios >> 5) + 1;
 	gpio->dcache = devm_kcalloc(&pdev->dev,
 				    banks, sizeof(u32), GFP_KERNEL);
 	if (!gpio->dcache)
-- 
2.20.1


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

* [PATCH 2/4] gpio/aspeed: Setup irqchip dynamically
  2019-09-04  6:12 [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks Rashmica Gupta
@ 2019-09-04  6:12 ` Rashmica Gupta
  2019-09-04  6:12 ` [PATCH 3/4] gpio: Add in ast2600 details to Aspeed driver Rashmica Gupta
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Rashmica Gupta @ 2019-09-04  6:12 UTC (permalink / raw)
  To: linus.walleij, linux-gpio, bgolaszewski
  Cc: linux-arm-kernel, linux-aspeed, linux-kernel, joel, andrew,
	Rashmica Gupta

This is in preparation for ast2600 as we will have two gpio drivers
which need their own irqchip.

Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
---
 drivers/gpio/gpio-aspeed.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index 77752b2624e8..60ab042c9129 100644
--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c
@@ -52,6 +52,7 @@ struct aspeed_gpio_config {
  */
 struct aspeed_gpio {
 	struct gpio_chip chip;
+	struct irq_chip irqc;
 	spinlock_t lock;
 	void __iomem *base;
 	int irq;
@@ -681,14 +682,6 @@ static void aspeed_gpio_irq_handler(struct irq_desc *desc)
 	chained_irq_exit(ic, desc);
 }
 
-static struct irq_chip aspeed_gpio_irqchip = {
-	.name		= "aspeed-gpio",
-	.irq_ack	= aspeed_gpio_irq_ack,
-	.irq_mask	= aspeed_gpio_irq_mask,
-	.irq_unmask	= aspeed_gpio_irq_unmask,
-	.irq_set_type	= aspeed_gpio_set_type,
-};
-
 static void set_irq_valid_mask(struct aspeed_gpio *gpio)
 {
 	const struct aspeed_bank_props *props = gpio->config->props;
@@ -1192,7 +1185,12 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev)
 
 		gpio->irq = rc;
 		girq = &gpio->chip.irq;
-		girq->chip = &aspeed_gpio_irqchip;
+		girq->chip = &gpio->irqc;
+		girq->chip->name = dev_name(&pdev->dev);
+		girq->chip->irq_ack = aspeed_gpio_irq_ack;
+		girq->chip->irq_mask = aspeed_gpio_irq_mask;
+		girq->chip->irq_unmask = aspeed_gpio_irq_unmask;
+		girq->chip->irq_set_type = aspeed_gpio_set_type;
 		girq->parent_handler = aspeed_gpio_irq_handler;
 		girq->num_parents = 1;
 		girq->parents = devm_kcalloc(&pdev->dev, 1,
-- 
2.20.1


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

* [PATCH 3/4] gpio: Add in ast2600 details to Aspeed driver
  2019-09-04  6:12 [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks Rashmica Gupta
  2019-09-04  6:12 ` [PATCH 2/4] gpio/aspeed: Setup irqchip dynamically Rashmica Gupta
@ 2019-09-04  6:12 ` Rashmica Gupta
  2019-09-04 16:30   ` Andy Shevchenko
  2019-09-04  6:12 ` [PATCH 4/4] gpio: Update documentation with ast2600 controllers Rashmica Gupta
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Rashmica Gupta @ 2019-09-04  6:12 UTC (permalink / raw)
  To: linus.walleij, linux-gpio, bgolaszewski
  Cc: linux-arm-kernel, linux-aspeed, linux-kernel, joel, andrew,
	Rashmica Gupta

The ast2600 has two gpio controllers, one for 3.6V GPIOS and one for 1.8V GPIOS.

Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
---
 drivers/gpio/gpio-aspeed.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index 60ab042c9129..98881c99d0b9 100644
--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c
@@ -662,12 +662,14 @@ static void aspeed_gpio_irq_handler(struct irq_desc *desc)
 	struct gpio_chip *gc = irq_desc_get_handler_data(desc);
 	struct irq_chip *ic = irq_desc_get_chip(desc);
 	struct aspeed_gpio *data = gpiochip_get_data(gc);
-	unsigned int i, p, girq;
+	unsigned int i, p, girq, banks;
 	unsigned long reg;
+	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
 
 	chained_irq_enter(ic, desc);
 
-	for (i = 0; i < ARRAY_SIZE(aspeed_gpio_banks); i++) {
+	banks = (gpio->config->nr_gpios >> 5) + 1;
+	for (i = 0; i < banks; i++) {
 		const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i];
 
 		reg = ioread32(bank_reg(data, bank, reg_irq_status));
@@ -1108,9 +1110,32 @@ static const struct aspeed_gpio_config ast2500_config =
 	/* 232 for simplicity, actual number is 228 (4-GPIO hole in GPIOAB) */
 	{ .nr_gpios = 232, .props = ast2500_bank_props, };
 
+static const struct aspeed_bank_props ast2600_bank_props[] = {
+	/*     input	  output   */
+	{5, 0xffffffff,  0x0000ffff}, /* U/V/W/X */
+	{6, 0xffff0000,  0x0fff0000}, /* Y/Z */
+	{ },
+};
+
+static const struct aspeed_gpio_config ast2600_config =
+	/* 208 3.6V GPIOs */
+	{ .nr_gpios = 208, .props = ast2600_bank_props, };
+
+static const struct aspeed_bank_props ast2600_1_8v_bank_props[] = {
+	/*     input	  output   */
+	{1, 0x0000000f,  0x0000000f}, /* E */
+	{ },
+};
+
+static const struct aspeed_gpio_config ast2600_1_8v_config =
+	/* 36 1.8V GPIOs */
+	{ .nr_gpios = 36, .props = ast2600_1_8v_bank_props, };
+
 static const struct of_device_id aspeed_gpio_of_table[] = {
 	{ .compatible = "aspeed,ast2400-gpio", .data = &ast2400_config, },
 	{ .compatible = "aspeed,ast2500-gpio", .data = &ast2500_config, },
+	{ .compatible = "aspeed,ast2600-gpio", .data = &ast2600_config, },
+	{ .compatible = "aspeed,ast2600-1-8v-gpio", .data = &ast2600_1_8v_config,},
 	{}
 };
 MODULE_DEVICE_TABLE(of, aspeed_gpio_of_table);
-- 
2.20.1


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

* [PATCH 4/4] gpio: Update documentation with ast2600 controllers
  2019-09-04  6:12 [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks Rashmica Gupta
  2019-09-04  6:12 ` [PATCH 2/4] gpio/aspeed: Setup irqchip dynamically Rashmica Gupta
  2019-09-04  6:12 ` [PATCH 3/4] gpio: Add in ast2600 details to Aspeed driver Rashmica Gupta
@ 2019-09-04  6:12 ` Rashmica Gupta
  2019-09-04  7:02   ` Bartosz Golaszewski
  2019-09-04  6:57 ` [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks Bartosz Golaszewski
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Rashmica Gupta @ 2019-09-04  6:12 UTC (permalink / raw)
  To: linus.walleij, linux-gpio, bgolaszewski
  Cc: linux-arm-kernel, linux-aspeed, linux-kernel, joel, andrew,
	Rashmica Gupta

Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
---
 Documentation/devicetree/bindings/gpio/gpio-aspeed.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-aspeed.txt b/Documentation/devicetree/bindings/gpio/gpio-aspeed.txt
index 7e9b586770b0..cd388797e07c 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-aspeed.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-aspeed.txt
@@ -2,7 +2,8 @@ Aspeed GPIO controller Device Tree Bindings
 -------------------------------------------
 
 Required properties:
-- compatible		: Either "aspeed,ast2400-gpio" or "aspeed,ast2500-gpio"
+- compatible		: Either "aspeed,ast2400-gpio", "aspeed,ast2500-gpio",
+					  "aspeed,ast2600-gpio", or "aspeed,ast2600-1-8v-gpio"
 
 - #gpio-cells 		: Should be two
 			  - First cell is the GPIO line number
-- 
2.20.1


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

* Re: [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks
  2019-09-04  6:12 [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks Rashmica Gupta
                   ` (2 preceding siblings ...)
  2019-09-04  6:12 ` [PATCH 4/4] gpio: Update documentation with ast2600 controllers Rashmica Gupta
@ 2019-09-04  6:57 ` Bartosz Golaszewski
  2019-09-04 16:27 ` Andy Shevchenko
  2019-09-11 17:48 ` Vijay Khemka
  5 siblings, 0 replies; 14+ messages in thread
From: Bartosz Golaszewski @ 2019-09-04  6:57 UTC (permalink / raw)
  To: Rashmica Gupta
  Cc: Linus Walleij, linux-gpio, arm-soc, linux-aspeed, LKML,
	Joel Stanley, Andrew Jeffery

śr., 4 wrz 2019 o 08:13 Rashmica Gupta <rashmica.g@gmail.com> napisał(a):
>
> Fixes: 361b79119a4b7 ('gpio: Add Aspeed driver')
>

Please, add a proper commit description. Checkpatch should have warned
you about it.

Bart

> Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
> ---
>  drivers/gpio/gpio-aspeed.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
> index 9defe25d4721..77752b2624e8 100644
> --- a/drivers/gpio/gpio-aspeed.c
> +++ b/drivers/gpio/gpio-aspeed.c
> @@ -1165,7 +1165,7 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev)
>         gpio->chip.base = -1;
>
>         /* Allocate a cache of the output registers */
> -       banks = gpio->config->nr_gpios >> 5;
> +       banks = (gpio->config->nr_gpios >> 5) + 1;
>         gpio->dcache = devm_kcalloc(&pdev->dev,
>                                     banks, sizeof(u32), GFP_KERNEL);
>         if (!gpio->dcache)
> --
> 2.20.1
>

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

* Re: [PATCH 4/4] gpio: Update documentation with ast2600 controllers
  2019-09-04  6:12 ` [PATCH 4/4] gpio: Update documentation with ast2600 controllers Rashmica Gupta
@ 2019-09-04  7:02   ` Bartosz Golaszewski
  2019-09-04 23:22     ` Rashmica Gupta
  0 siblings, 1 reply; 14+ messages in thread
From: Bartosz Golaszewski @ 2019-09-04  7:02 UTC (permalink / raw)
  To: Rashmica Gupta
  Cc: Linus Walleij, linux-gpio, arm-soc, linux-aspeed, LKML,
	Joel Stanley, Andrew Jeffery

śr., 4 wrz 2019 o 08:13 Rashmica Gupta <rashmica.g@gmail.com> napisał(a):
>

Again, this needs a proper commit description and the subject should
start with "dt-bindings: ...".

You also need to Cc the device-tree maintainers. Use
scripts/get_maintainer.pl to list all people that should get this
patch.

Bart

> Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
> ---
>  Documentation/devicetree/bindings/gpio/gpio-aspeed.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-aspeed.txt b/Documentation/devicetree/bindings/gpio/gpio-aspeed.txt
> index 7e9b586770b0..cd388797e07c 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-aspeed.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-aspeed.txt
> @@ -2,7 +2,8 @@ Aspeed GPIO controller Device Tree Bindings
>  -------------------------------------------
>
>  Required properties:
> -- compatible           : Either "aspeed,ast2400-gpio" or "aspeed,ast2500-gpio"
> +- compatible           : Either "aspeed,ast2400-gpio", "aspeed,ast2500-gpio",
> +                                         "aspeed,ast2600-gpio", or "aspeed,ast2600-1-8v-gpio"
>
>  - #gpio-cells          : Should be two
>                           - First cell is the GPIO line number
> --
> 2.20.1
>

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

* Re: [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks
  2019-09-04  6:12 [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks Rashmica Gupta
                   ` (3 preceding siblings ...)
  2019-09-04  6:57 ` [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks Bartosz Golaszewski
@ 2019-09-04 16:27 ` Andy Shevchenko
  2019-09-04 23:17   ` Rashmica Gupta
  2019-09-11 17:48 ` Vijay Khemka
  5 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2019-09-04 16:27 UTC (permalink / raw)
  To: Rashmica Gupta
  Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
	linux-arm Mailing List, linux-aspeed, Linux Kernel Mailing List,
	Joel Stanley, Andrew Jeffery

On Wed, Sep 4, 2019 at 9:14 AM Rashmica Gupta <rashmica.g@gmail.com> wrote:
>
> Fixes: 361b79119a4b7 ('gpio: Add Aspeed driver')
>
> Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>

>         /* Allocate a cache of the output registers */
> -       banks = gpio->config->nr_gpios >> 5;
> +       banks = (gpio->config->nr_gpios >> 5) + 1;

Shouldn't be rather DIV_ROUND_UP(nr_gpios, sizeof(u32)) ?

>         gpio->dcache = devm_kcalloc(&pdev->dev,
>                                     banks, sizeof(u32), GFP_KERNEL);


-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH 3/4] gpio: Add in ast2600 details to Aspeed driver
  2019-09-04  6:12 ` [PATCH 3/4] gpio: Add in ast2600 details to Aspeed driver Rashmica Gupta
@ 2019-09-04 16:30   ` Andy Shevchenko
  2019-09-04 23:34     ` Rashmica Gupta
  0 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2019-09-04 16:30 UTC (permalink / raw)
  To: Rashmica Gupta
  Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
	linux-arm Mailing List, linux-aspeed, Linux Kernel Mailing List,
	Joel Stanley, Andrew Jeffery

On Wed, Sep 4, 2019 at 9:14 AM Rashmica Gupta <rashmica.g@gmail.com> wrote:
>
> The ast2600 has two gpio controllers, one for 3.6V GPIOS and one for 1.8V GPIOS.
>
> Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>

> -       for (i = 0; i < ARRAY_SIZE(aspeed_gpio_banks); i++) {
> +       banks = (gpio->config->nr_gpios >> 5) + 1;

Same comment as per the other patch.

> +       for (i = 0; i < banks; i++) {

> +static const struct aspeed_bank_props ast2600_bank_props[] = {
> +       /*     input      output   */
> +       {5, 0xffffffff,  0x0000ffff}, /* U/V/W/X */
> +       {6, 0xffff0000,  0x0fff0000}, /* Y/Z */

Perhaps GENMASK() for all values?

> +       { },

Comma is not needed here.

> +};
> +
> +static const struct aspeed_gpio_config ast2600_config =
> +       /* 208 3.6V GPIOs */

> +       { .nr_gpios = 208, .props = ast2600_bank_props, };

Seems curly braces missed their places.

> +static const struct aspeed_bank_props ast2600_1_8v_bank_props[] = {
> +       /*     input      output   */
> +       {1, 0x0000000f,  0x0000000f}, /* E */

GENMASK()?

> +       { },

No comma.

> +};

> +static const struct aspeed_gpio_config ast2600_1_8v_config =
> +       /* 36 1.8V GPIOs */
> +       { .nr_gpios = 36, .props = ast2600_1_8v_bank_props, };

Location of the curly braces?

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks
  2019-09-04 16:27 ` Andy Shevchenko
@ 2019-09-04 23:17   ` Rashmica Gupta
  2019-09-05  8:08     ` Andy Shevchenko
  0 siblings, 1 reply; 14+ messages in thread
From: Rashmica Gupta @ 2019-09-04 23:17 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
	linux-arm Mailing List, linux-aspeed, Linux Kernel Mailing List,
	Joel Stanley, Andrew Jeffery

On Wed, 2019-09-04 at 19:27 +0300, Andy Shevchenko wrote:
> On Wed, Sep 4, 2019 at 9:14 AM Rashmica Gupta <rashmica.g@gmail.com>
> wrote:
> > Fixes: 361b79119a4b7 ('gpio: Add Aspeed driver')
> > 
> > Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
> >         /* Allocate a cache of the output registers */
> > -       banks = gpio->config->nr_gpios >> 5;
> > +       banks = (gpio->config->nr_gpios >> 5) + 1;
> 
> Shouldn't be rather DIV_ROUND_UP(nr_gpios, sizeof(u32)) ?

I agree that DIV_ROUND_UP is the right thing to use here, but wouldn't
it be DIV_ROUND_UP(nr_gpios, 32)?

> 
> >         gpio->dcache = devm_kcalloc(&pdev->dev,
> >                                     banks, sizeof(u32),
> > GFP_KERNEL);
> 
> 


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

* Re: [PATCH 4/4] gpio: Update documentation with ast2600 controllers
  2019-09-04  7:02   ` Bartosz Golaszewski
@ 2019-09-04 23:22     ` Rashmica Gupta
  0 siblings, 0 replies; 14+ messages in thread
From: Rashmica Gupta @ 2019-09-04 23:22 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Linus Walleij, linux-gpio, arm-soc, linux-aspeed, LKML,
	Joel Stanley, Andrew Jeffery

On Wed, 2019-09-04 at 09:02 +0200, Bartosz Golaszewski wrote:
> śr., 4 wrz 2019 o 08:13 Rashmica Gupta <rashmica.g@gmail.com>
> napisał(a):
> 
> Again, this needs a proper commit description 

I figured as similar patches have gone through with just the one line
and there isn't anything more to say that the one line message that
this would be ok.

>and the subject should
> start with "dt-bindings: ...".
> 
True.


> You also need to Cc the device-tree maintainers. Use
> scripts/get_maintainer.pl to list all people that should get this
> patch.

Must have missed that one somehow.

> 
> Bart
> 
> > Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/gpio/gpio-aspeed.txt | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/gpio/gpio-
aspeed.txt 
> > b/Documentation/devicetree/bindings/gpio/gpio-aspeed.txt
> > index 7e9b586770b0..cd388797e07c 100644
> > --- a/Documentation/devicetree/bindings/gpio/gpio-aspeed.txt
> > +++ b/Documentation/devicetree/bindings/gpio/gpio-aspeed.txt
> > @@ -2,7 +2,8 @@ Aspeed GPIO controller Device Tree Bindings
> >  -------------------------------------------
> > 
> >  Required properties:
> > -- compatible           : Either "aspeed,ast2400-gpio" or
> > "aspeed,ast2500-gpio"
> > +- compatible           : Either "aspeed,ast2400-gpio",
> > "aspeed,ast2500-gpio",
> > +                                         "aspeed,ast2600-gpio", or
> > "aspeed,ast2600-1-8v-gpio"
> > 
> >  - #gpio-cells          : Should be two
> >                           - First cell is the GPIO line number
> > --
> > 2.20.1
> > 


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

* Re: [PATCH 3/4] gpio: Add in ast2600 details to Aspeed driver
  2019-09-04 16:30   ` Andy Shevchenko
@ 2019-09-04 23:34     ` Rashmica Gupta
  2019-09-05  8:10       ` Andy Shevchenko
  0 siblings, 1 reply; 14+ messages in thread
From: Rashmica Gupta @ 2019-09-04 23:34 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
	linux-arm Mailing List, linux-aspeed, Linux Kernel Mailing List,
	Joel Stanley, Andrew Jeffery

On Wed, 2019-09-04 at 19:30 +0300, Andy Shevchenko wrote:
> On Wed, Sep 4, 2019 at 9:14 AM Rashmica Gupta <rashmica.g@gmail.com>
> wrote:
> > The ast2600 has two gpio controllers, one for 3.6V GPIOS and one
> > for 1.8V GPIOS.
> > 
> > Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
> > -       for (i = 0; i < ARRAY_SIZE(aspeed_gpio_banks); i++) {
> > +       banks = (gpio->config->nr_gpios >> 5) + 1;
> 
> Same comment as per the other patch.
> 
> > +       for (i = 0; i < banks; i++) {
> > +static const struct aspeed_bank_props ast2600_bank_props[] = {
> > +       /*     input      output   */
> > +       {5, 0xffffffff,  0x0000ffff}, /* U/V/W/X */
> > +       {6, 0xffff0000,  0x0fff0000}, /* Y/Z */
> 
> Perhaps GENMASK() for all values?

Perhaps this and your other comments below would be best addressed in
an additional cleanup patch? This patch follows the formatting of the
existing code and it's not very clean to differ from that or to change
the formatting of the current code in this patch.


> 
> > +       { },
> 
> Comma is not needed here.
> 
> > +};
> > +
> > +static const struct aspeed_gpio_config ast2600_config =
> > +       /* 208 3.6V GPIOs */
> > +       { .nr_gpios = 208, .props = ast2600_bank_props, };
> 
> Seems curly braces missed their places.
> 
> > +static const struct aspeed_bank_props ast2600_1_8v_bank_props[] =
> > {
> > +       /*     input      output   */
> > +       {1, 0x0000000f,  0x0000000f}, /* E */
> 
> GENMASK()?
> 
> > +       { },
> 
> No comma.
> 
> > +};
> > +static const struct aspeed_gpio_config ast2600_1_8v_config =
> > +       /* 36 1.8V GPIOs */
> > +       { .nr_gpios = 36, .props = ast2600_1_8v_bank_props, };
> 
> Location of the curly braces?
> 


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

* Re: [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks
  2019-09-04 23:17   ` Rashmica Gupta
@ 2019-09-05  8:08     ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2019-09-05  8:08 UTC (permalink / raw)
  To: Rashmica Gupta
  Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
	linux-arm Mailing List, linux-aspeed, Linux Kernel Mailing List,
	Joel Stanley, Andrew Jeffery

On Thu, Sep 5, 2019 at 2:17 AM Rashmica Gupta <rashmica.g@gmail.com> wrote:
> On Wed, 2019-09-04 at 19:27 +0300, Andy Shevchenko wrote:
> > On Wed, Sep 4, 2019 at 9:14 AM Rashmica Gupta <rashmica.g@gmail.com>
> > wrote:

> > > -       banks = gpio->config->nr_gpios >> 5;
> > > +       banks = (gpio->config->nr_gpios >> 5) + 1;
> >
> > Shouldn't be rather DIV_ROUND_UP(nr_gpios, sizeof(u32)) ?
>
> I agree that DIV_ROUND_UP is the right thing to use here, but wouldn't
> it be DIV_ROUND_UP(nr_gpios, 32)?

Right. Either this or BITS_PER_TYPE(u32).

> > >         gpio->dcache = devm_kcalloc(&pdev->dev,
> > >                                     banks, sizeof(u32),
> > > GFP_KERNEL);

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH 3/4] gpio: Add in ast2600 details to Aspeed driver
  2019-09-04 23:34     ` Rashmica Gupta
@ 2019-09-05  8:10       ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2019-09-05  8:10 UTC (permalink / raw)
  To: Rashmica Gupta
  Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
	linux-arm Mailing List, linux-aspeed, Linux Kernel Mailing List,
	Joel Stanley, Andrew Jeffery

On Thu, Sep 5, 2019 at 2:34 AM Rashmica Gupta <rashmica.g@gmail.com> wrote:>
> On Wed, 2019-09-04 at 19:30 +0300, Andy Shevchenko wrote:

> Perhaps this and your other comments below would be best addressed in
> an additional cleanup patch? This patch follows the formatting of the
> existing code and it's not very clean to differ from that or to change
> the formatting of the current code in this patch.

OK.

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks
  2019-09-04  6:12 [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks Rashmica Gupta
                   ` (4 preceding siblings ...)
  2019-09-04 16:27 ` Andy Shevchenko
@ 2019-09-11 17:48 ` Vijay Khemka
  5 siblings, 0 replies; 14+ messages in thread
From: Vijay Khemka @ 2019-09-11 17:48 UTC (permalink / raw)
  To: Rashmica Gupta, linus.walleij, linux-gpio, bgolaszewski
  Cc: linux-aspeed, linux-kernel, linux-arm-kernel



On 9/11/19, 5:16 AM, "Linux-aspeed on behalf of Rashmica Gupta" <linux-aspeed-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of rashmica.g@gmail.com> wrote:

    Fixes: 361b79119a4b7 ('gpio: Add Aspeed driver')
    
    Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
    ---
     drivers/gpio/gpio-aspeed.c | 2 +-
     1 file changed, 1 insertion(+), 1 deletion(-)
    
    diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
    index 9defe25d4721..77752b2624e8 100644
    --- a/drivers/gpio/gpio-aspeed.c
    +++ b/drivers/gpio/gpio-aspeed.c
    @@ -1165,7 +1165,7 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev)
     	gpio->chip.base = -1;
     
     	/* Allocate a cache of the output registers */
    -	banks = gpio->config->nr_gpios >> 5;
    +	banks = (gpio->config->nr_gpios >> 5) + 1;
If number of gpios are 32 then it should be only 1 bank, as per above it is 2 bank.
     	gpio->dcache = devm_kcalloc(&pdev->dev,
     				    banks, sizeof(u32), GFP_KERNEL);
     	if (!gpio->dcache)
    -- 
    2.20.1
    
    


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

end of thread, back to index

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-04  6:12 [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks Rashmica Gupta
2019-09-04  6:12 ` [PATCH 2/4] gpio/aspeed: Setup irqchip dynamically Rashmica Gupta
2019-09-04  6:12 ` [PATCH 3/4] gpio: Add in ast2600 details to Aspeed driver Rashmica Gupta
2019-09-04 16:30   ` Andy Shevchenko
2019-09-04 23:34     ` Rashmica Gupta
2019-09-05  8:10       ` Andy Shevchenko
2019-09-04  6:12 ` [PATCH 4/4] gpio: Update documentation with ast2600 controllers Rashmica Gupta
2019-09-04  7:02   ` Bartosz Golaszewski
2019-09-04 23:22     ` Rashmica Gupta
2019-09-04  6:57 ` [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks Bartosz Golaszewski
2019-09-04 16:27 ` Andy Shevchenko
2019-09-04 23:17   ` Rashmica Gupta
2019-09-05  8:08     ` Andy Shevchenko
2019-09-11 17:48 ` Vijay Khemka

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox