Linux-Aspeed Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks
From: Bartosz Golaszewski @ 2019-09-04  6:57 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190904061245.30770-1-rashmica.g@gmail.com>

?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

* [PATCH 4/4] gpio: Update documentation with ast2600 controllers
From: Bartosz Golaszewski @ 2019-09-04  7:02 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190904061245.30770-4-rashmica.g@gmail.com>

?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

* [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks
From: Andy Shevchenko @ 2019-09-04 16:27 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190904061245.30770-1-rashmica.g@gmail.com>

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

* [PATCH 3/4] gpio: Add in ast2600 details to Aspeed driver
From: Andy Shevchenko @ 2019-09-04 16:30 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190904061245.30770-3-rashmica.g@gmail.com>

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

* [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks
From: Rashmica Gupta @ 2019-09-04 23:17 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CAHp75Vd_6Rpt5=BjzV8YFCiFP7qsRrYHHo7+=gWwnZH-zT9jNw@mail.gmail.com>

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

* [PATCH 4/4] gpio: Update documentation with ast2600 controllers
From: Rashmica Gupta @ 2019-09-04 23:22 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CAMpxmJUGm3Zs8VHwHnXTQ2cKnpF0caR=7V4bBi1_sy1U2mWc0g@mail.gmail.com>

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

* [PATCH 3/4] gpio: Add in ast2600 details to Aspeed driver
From: Rashmica Gupta @ 2019-09-04 23:34 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CAHp75Ve0zEkuD-75aZ6FU+A=DvX8NvVvY3n9p_pYDyfa76sxoQ@mail.gmail.com>

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

* [PATCH] ARM: dts: aspeed-g4: Add all flash chips
From: Joel Stanley @ 2019-09-05  0:02 UTC (permalink / raw)
  To: linux-aspeed

The FMC supports five chip selects, so describe the five possible flash
chips.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 arch/arm/boot/dts/aspeed-g4.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
index e465cda40fe7..dffb595d30e4 100644
--- a/arch/arm/boot/dts/aspeed-g4.dtsi
+++ b/arch/arm/boot/dts/aspeed-g4.dtsi
@@ -67,6 +67,26 @@
 				compatible = "jedec,spi-nor";
 				status = "disabled";
 			};
+			flash at 1 {
+				reg = < 1 >;
+				compatible = "jedec,spi-nor";
+				status = "disabled";
+			};
+			flash at 2 {
+				reg = < 2 >;
+				compatible = "jedec,spi-nor";
+				status = "disabled";
+			};
+			flash at 3 {
+				reg = < 3 >;
+				compatible = "jedec,spi-nor";
+				status = "disabled";
+			};
+			flash at 4 {
+				reg = < 4 >;
+				compatible = "jedec,spi-nor";
+				status = "disabled";
+			};
 		};
 
 		spi: spi at 1e630000 {
-- 
2.23.0.rc1


^ permalink raw reply related

* [PATCH] ARM: dts: aspeed-g4: Add all flash chips
From: Andrew Jeffery @ 2019-09-05  0:33 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190905000221.31445-1-joel@jms.id.au>



On Thu, 5 Sep 2019, at 09:32, Joel Stanley wrote:
> The FMC supports five chip selects, so describe the five possible flash
> chips.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  arch/arm/boot/dts/aspeed-g4.dtsi | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
> index e465cda40fe7..dffb595d30e4 100644
> --- a/arch/arm/boot/dts/aspeed-g4.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g4.dtsi
> @@ -67,6 +67,26 @@
>  				compatible = "jedec,spi-nor";
>  				status = "disabled";
>  			};
> +			flash at 1 {
> +				reg = < 1 >;
> +				compatible = "jedec,spi-nor";
> +				status = "disabled";
> +			};
> +			flash at 2 {
> +				reg = < 2 >;
> +				compatible = "jedec,spi-nor";
> +				status = "disabled";
> +			};
> +			flash at 3 {
> +				reg = < 3 >;
> +				compatible = "jedec,spi-nor";
> +				status = "disabled";
> +			};
> +			flash at 4 {
> +				reg = < 4 >;
> +				compatible = "jedec,spi-nor";
> +				status = "disabled";
> +			};

The FMC supports parallel NOR and NAND interfaces too, but so far no-one has
cared for these options, so if they ever do we'll fix it then.

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

^ permalink raw reply

* [GIT PULL] ARM: aspeed: devicetree changes for 5.4, round two
From: Joel Stanley @ 2019-09-05  0:43 UTC (permalink / raw)
  To: linux-aspeed

Hello ARM maintainers,

Here are some late fixes I collected for the ASPEED boards.

I've thrown the commits on top of the ones in the first pull request, which
you've merged. I've not sent a second pull before so if that's not the done
thing then let me know what you prefer.

The following changes since commit 49b0f3be0b86292eed6f6aedadf4252131d9c111:

  ARM: dts: aspeed: swift: Add eMMC device (2019-08-22 15:34:20 +0930)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/joel/aspeed.git \
      tags/aspeed-5.4-devicetree-2

for you to fetch changes up to 89b97c429e2e77d695b5133572ca12ec256a4ea4:

  ARM: dts: aspeed-g5: Fixe gpio-ranges upper limit (2019-09-04 17:34:34 -0700)

----------------------------------------------------------------
ASPEED device tree updates for 5.4, second round

 - Alternate flash support for Vesnin
 - Minor cleanups and fixes

----------------------------------------------------------------
Eddie James (1):
      ARM: dts: aspeed: swift: Change power supplies to version 2

Ivan Mikhaylov (2):
      ARM: dts: aspeed: vesnin: Add wdt2 with alt-boot option
      ARM: dts: aspeed: vesnin: Add secondary SPI flash chip

Joel Stanley (2):
      ARM: dts: aspeed-g4: Add all flash chips
      ARM; dts: aspeed: mihawk: File should not be executable

Oscar A Perez (1):
      ARM: dts: aspeed-g5: Fixe gpio-ranges upper limit

 arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts |  0
 arch/arm/boot/dts/aspeed-bmc-opp-swift.dts  |  4 ++--
 arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts | 10 ++++++++++
 arch/arm/boot/dts/aspeed-g4.dtsi            | 20 ++++++++++++++++++++
 arch/arm/boot/dts/aspeed-g5.dtsi            |  2 +-
 5 files changed, 33 insertions(+), 3 deletions(-)
 mode change 100755 => 100644 arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts

^ permalink raw reply

* [PATCH v2 1/3] drivers/tty/serial/8250: Make Aspeed VUART SIRQ polarity configurable
From: Jeremy Kerr @ 2019-09-05  1:14 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190731013404.243755-1-osk@google.com>

Hi Oskar,

Looks good to me, some minor comments though:

> +
> +What:		/sys/bus/platform/drivers/aspeed-vuart/*/sirq_polarity
> +Date:		July 2019
> +Contact:	Oskar Senft <osk@google.com>
> +Description:	Configures the polarity of the serial interrupt to the
> +		host via the BMC LPC bus.

Can you mention what the value represents? 1/0 don't really indicate a
specific polarity.

Alternatively, we could use descriptive values (say, "active-low" /
"idle-low").

> @@ -310,6 +379,7 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	u32 clk, prop;
>  	int rc;
> +	struct of_phandle_args espi_enabled_args;

Minor: can you reverse-christmas-tree this?

> @@ -402,6 +472,18 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
>  
>  	vuart->line = rc;
>  
> +	rc = of_parse_phandle_with_fixed_args(
> +		np, "espi-enabled", 2, 0, &espi_enabled_args);
> +	if (rc < 0) {
> +		dev_warn(&pdev->dev, "espi-enabled property not found\n");

In the binding spec, you've listed this property at optional, but here
we dev_warn() if its not present. Can we default to existing behaviour
if it's not there?

That may just be a matter of changing this to dev_debug.

Cheers,


Jeremy


^ permalink raw reply

* [PATCH v2 0/4] Add ast2600 gpio support
From: Rashmica Gupta @ 2019-09-05  1:16 UTC (permalink / raw)
  To: linux-aspeed

v2: More verbose commit messages, using DIV_ROUND_UP().

Rashmica Gupta (4):
  gpio/aspeed: Fix incorrect number of banks
  gpio/aspeed: Setup irqchip dynamically
  gpio: Add in ast2600 details to Aspeed driver
  gpio: dt-bindings: Update documentation with ast2600 controllers

 drivers/gpio/gpio-aspeed.c                    | 48 ++++++++++++++-----
 .../devicetree/bindings/gpio/gpio-aspeed.txt  |  3 +-
 2 files changed, 38 insertions(+), 13 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH v2 1/4] gpio/aspeed: Fix incorrect number of banks
From: Rashmica Gupta @ 2019-09-05  1:16 UTC (permalink / raw)
  To: linux-aspeed

The current calculation for the number of GPIO banks is only correct if
the number of GPIOs is a multiple of 32 (if there were 31 GPIOs we would
currently say there are 0 banks, which is incorrect).

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..b83e23aecd18 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 = DIV_ROUND_UP(gpio->config->nr_gpios, 32);
 	gpio->dcache = devm_kcalloc(&pdev->dev,
 				    banks, sizeof(u32), GFP_KERNEL);
 	if (!gpio->dcache)
-- 
2.20.1


^ permalink raw reply related

* [PATCH v2 2/3] dt-bindings: serial: 8250: Add documentation for espi-enabled.
From: Jeremy Kerr @ 2019-09-05  1:16 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <b4670171-e161-4d7a-91dc-a1e5a95f7dbc@www.fastmail.com>

Hi Oskar,

> Given it's ASPEED-specific I expect you should use a vendor prefix for the
> property, e.g. aspeed,espi-enabled.
> 
> However, as I understand it you want to determine what polarity the SIRQ
> should be regardless of which of eSPI or LPC are enabled, so I don't think
> the property name should be an explicit statement about eSPI. Maybe
> "aspeed,sirq-polarity-sense"?

Yep, +1 on Andrew's comments here. This property isn't an indication on
whether espi is enabled, but a method to detect it.

Cheers,


Jeremy


^ permalink raw reply

* [PATCH v2 2/4] gpio/aspeed: Setup irqchip dynamically
From: Rashmica Gupta @ 2019-09-05  1:17 UTC (permalink / raw)
  To: linux-aspeed

This is in preparation for adding ast2600 support. The ast2600 requires two
GPIO drivers which each 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 b83e23aecd18..16c6eaf70857 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 related

* [PATCH v2 3/4] gpio: Add in ast2600 details to Aspeed driver
From: Rashmica Gupta @ 2019-09-05  1:17 UTC (permalink / raw)
  To: linux-aspeed

The ast2600 is a new generation of SoC from ASPEED. Similarly to the
ast2400 and ast2500, it has a GPIO controller for it's 3.6V GPIO pins.
Additionally, it has a GPIO controller for 36 1.8V GPIO pins. These
voltages are fixed and cannot be configured via pinconf, so we need two
separate drivers for them.

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

diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index 16c6eaf70857..4723b8780a8c 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 = DIV_ROUND_UP(gpio->config->nr_gpios, 32);
+	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,33 @@ 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 related

* [PATCH v2 4/4] gpio: dt-bindings: Update documentation with ast2600 controllers
From: Rashmica Gupta @ 2019-09-05  1:18 UTC (permalink / raw)
  To: linux-aspeed

The ast2600 is a new generation of SoC from ASPEED. Similarly to the
ast2400 and ast2500, it has a GPIO controller for it's 3.6V GPIO pins.
Additionally, it has a GPIO controller for 36 1.8V GPIO pins. These
voltages are fixed and cannot be configured via pinconf, so we have two
separate drivers for them.

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 related

* [PATCH v2 1/4] gpio/aspeed: Fix incorrect number of banks
From: Andrew Jeffery @ 2019-09-05  3:47 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190905011635.15902-1-rashmica.g@gmail.com>



On Thu, 5 Sep 2019, at 10:46, Rashmica Gupta wrote:
> The current calculation for the number of GPIO banks is only correct if
> the number of GPIOs is a multiple of 32 (if there were 31 GPIOs we would
> currently say there are 0 banks, which is incorrect).
> 
> Fixes: 361b79119a4b7 ('gpio: Add Aspeed driver')
> 
> Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  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..b83e23aecd18 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 = DIV_ROUND_UP(gpio->config->nr_gpios, 32);
>  	gpio->dcache = devm_kcalloc(&pdev->dev,
>  				    banks, sizeof(u32), GFP_KERNEL);
>  	if (!gpio->dcache)
> -- 
> 2.20.1
> 
>

^ permalink raw reply

* [PATCH v2 3/4] gpio: Add in ast2600 details to Aspeed driver
From: Andrew Jeffery @ 2019-09-05  3:57 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190905011732.16059-1-rashmica.g@gmail.com>



On Thu, 5 Sep 2019, at 10:47, Rashmica Gupta wrote:
> The ast2600 is a new generation of SoC from ASPEED. Similarly to the
> ast2400 and ast2500, it has a GPIO controller for it's 3.6V GPIO pins.
> Additionally, it has a GPIO controller for 36 1.8V GPIO pins. These
> voltages are fixed and cannot be configured via pinconf, so we need two
> separate drivers for them.

Working backwards, we don't really have multiple drivers, just different
configurations for the same driver. So I think this should be reworded.

Also it's not really the voltage differences that are driving the different
configurations but rather that there are two separate sets of registers
in the 2600 with overlapping bank names (they happen to be split into
3.3V and 1.8V groups). The key point being that there aren't just more
GPIO registers tacked on the end of the original 3.3V group.

> 
> Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
> ---
>  drivers/gpio/gpio-aspeed.c | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
> index 16c6eaf70857..4723b8780a8c 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 = DIV_ROUND_UP(gpio->config->nr_gpios, 32);
> +	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,33 @@ 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 */

If there are 36 GPIOs then this configuration is suggesting that all of them
are capable of input and output. A handy observation here is that the first
36 GPIOs of the 3.3V GPIO controller in the 2600 also have both capabilities,
so we can re-use the 3.3V configuration if we can limit the number of GPIOs
somehow.

The devicetree binding already describes an `ngpios` property so perhaps
we could make use of this to use the same properties struct instance for both
controllers in the 2600: Require that the property be present for 2600-
compatible devicetree nodes and test for its presence in probe(), then fall
back to the hard-coded value in the config struct if it is not (this keeps
devicetree compatibility for the 2400 and 2500 drivers).

This way we can eliminate the aspeed,ast2600-1-8v-gpio compatible string
below (we just use aspeed,ast2600-gpio for both controllers).

Thoughts?

Andrew

> +	{ },
> +};
> +
> +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

* Re: [PATCH v2 4/4] gpio: dt-bindings: Update documentation with ast2600 controllers
From: Andrew Jeffery @ 2019-09-05  3:59 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190905011800.16156-1-rashmica.g@gmail.com>



On Thu, 5 Sep 2019, at 10:48, Rashmica Gupta wrote:
> The ast2600 is a new generation of SoC from ASPEED. Similarly to the
> ast2400 and ast2500, it has a GPIO controller for it's 3.6V GPIO pins.
> Additionally, it has a GPIO controller for 36 1.8V GPIO pins. These
> voltages are fixed and cannot be configured via pinconf, so we have two
> separate drivers for them.

See 3/4 for discussion about the commit message.

> 
> 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"

See the discussion on patch 3/4 about how we might eliminate the
aspeed,ast2600-1-8v-gpio compatible string.

Also, this patch should be the first in the series and start the subject with
"dt-bindings: gpio: aspeed: ..."

Cheers,

Andrew

^ permalink raw reply

* [PATCH] ARM: dts: aspeed-g4: Add all flash chips
From: Cédric Le Goater @ 2019-09-05  6:43 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190905000221.31445-1-joel@jms.id.au>

On 05/09/2019 02:02, Joel Stanley wrote:
> The FMC supports five chip selects, so describe the five possible flash
> chips.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>


Reviewed-by: C?dric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  arch/arm/boot/dts/aspeed-g4.dtsi | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
> index e465cda40fe7..dffb595d30e4 100644
> --- a/arch/arm/boot/dts/aspeed-g4.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g4.dtsi
> @@ -67,6 +67,26 @@
>  				compatible = "jedec,spi-nor";
>  				status = "disabled";
>  			};
> +			flash at 1 {
> +				reg = < 1 >;
> +				compatible = "jedec,spi-nor";
> +				status = "disabled";
> +			};
> +			flash at 2 {
> +				reg = < 2 >;
> +				compatible = "jedec,spi-nor";
> +				status = "disabled";
> +			};
> +			flash at 3 {
> +				reg = < 3 >;
> +				compatible = "jedec,spi-nor";
> +				status = "disabled";
> +			};
> +			flash at 4 {
> +				reg = < 4 >;
> +				compatible = "jedec,spi-nor";
> +				status = "disabled";
> +			};
>  		};
>  
>  		spi: spi at 1e630000 {
> 


^ permalink raw reply

* [PATCH] ARM: dts: aspeed-g4: Add all flash chips
From: Cédric Le Goater @ 2019-09-05  6:45 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <d9805fa2-db79-457b-a166-7c84e1608128@www.fastmail.com>

On 05/09/2019 02:33, Andrew Jeffery wrote:
> 
> 
> On Thu, 5 Sep 2019, at 09:32, Joel Stanley wrote:
>> The FMC supports five chip selects, so describe the five possible flash
>> chips.
>>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> ---
>>  arch/arm/boot/dts/aspeed-g4.dtsi | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
>> index e465cda40fe7..dffb595d30e4 100644
>> --- a/arch/arm/boot/dts/aspeed-g4.dtsi
>> +++ b/arch/arm/boot/dts/aspeed-g4.dtsi
>> @@ -67,6 +67,26 @@
>>  				compatible = "jedec,spi-nor";
>>  				status = "disabled";
>>  			};
>> +			flash at 1 {
>> +				reg = < 1 >;
>> +				compatible = "jedec,spi-nor";
>> +				status = "disabled";
>> +			};
>> +			flash at 2 {
>> +				reg = < 2 >;
>> +				compatible = "jedec,spi-nor";
>> +				status = "disabled";
>> +			};
>> +			flash at 3 {
>> +				reg = < 3 >;
>> +				compatible = "jedec,spi-nor";
>> +				status = "disabled";
>> +			};
>> +			flash at 4 {
>> +				reg = < 4 >;
>> +				compatible = "jedec,spi-nor";
>> +				status = "disabled";
>> +			};
> 
> The FMC supports parallel NOR and NAND interfaces too, but so far no-one has
> cared for these options, so if they ever do we'll fix it then.

New Aspeed SoCs only have SPI support. So I don't think the other interfaces
were ever used.

C. 
 
> 
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> 


^ permalink raw reply

* [PATCH v2 3/4] gpio: Add in ast2600 details to Aspeed driver
From: Rashmica Gupta @ 2019-09-05  7:08 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <40601711-5fcf-40a0-bfc2-ae5043948a41@www.fastmail.com>

On Thu, 2019-09-05 at 13:27 +0930, Andrew Jeffery wrote:
> 
> On Thu, 5 Sep 2019, at 10:47, Rashmica Gupta wrote:
> > The ast2600 is a new generation of SoC from ASPEED. Similarly to
> > the
> > ast2400 and ast2500, it has a GPIO controller for it's 3.6V GPIO
> > pins.
> > Additionally, it has a GPIO controller for 36 1.8V GPIO pins. These
> > voltages are fixed and cannot be configured via pinconf, so we need
> > two
> > separate drivers for them.
> 
> Working backwards, we don't really have multiple drivers, just
> different
> configurations for the same driver. So I think this should be
> reworded.
> 
> Also it's not really the voltage differences that are driving the
> different
> configurations but rather that there are two separate sets of
> registers
> in the 2600 with overlapping bank names (they happen to be split into
> 3.3V and 1.8V groups). The key point being that there aren't just
> more
> GPIO registers tacked on the end of the original 3.3V group.
> 

Good point! I'll reword this. Also they are 3.3V, I'm not sure why I
wrote 3.6V in my patches.

> > Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
> > ---
> >  drivers/gpio/gpio-aspeed.c | 30 ++++++++++++++++++++++++++++--
> >  1 file changed, 28 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-
> > aspeed.c
> > index 16c6eaf70857..4723b8780a8c 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 = DIV_ROUND_UP(gpio->config->nr_gpios, 32);
> > +	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,33 @@ 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 */
> 
> If there are 36 GPIOs then this configuration is suggesting that all
> of them
> are capable of input and output. A handy observation here is that the
> first
> 36 GPIOs of the 3.3V GPIO controller in the 2600 also have both
> capabilities,
> so we can re-use the 3.3V configuration if we can limit the number of
> GPIOs
> somehow.
> 
> The devicetree binding already describes an `ngpios` property so
> perhaps
> we could make use of this to use the same properties struct instance
> for both
> controllers in the 2600: Require that the property be present for
> 2600-
> compatible devicetree nodes and test for its presence in probe(),
> then fall
> back to the hard-coded value in the config struct if it is not (this
> keeps
> devicetree compatibility for the 2400 and 2500 drivers).
> 
> This way we can eliminate the aspeed,ast2600-1-8v-gpio compatible
> string
> below (we just use aspeed,ast2600-gpio for both controllers).
> 
> Thoughts?

I like this idea. Once I have confirmation that all the 1.8V pins are
indeed input and output capable I'll send out a v3.

> 
> Andrew
> 
> > +	{ },
> > +};
> > +
> > +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

* [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks
From: Andy Shevchenko @ 2019-09-05  8:08 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <dd62da5f10c06fae1823bf8338c2acc83fe40a40.camel@gmail.com>

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

* [PATCH 3/4] gpio: Add in ast2600 details to Aspeed driver
From: Andy Shevchenko @ 2019-09-05  8:10 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1ca6ffddd2452e218ef19ea84ac6c6277e1a9725.camel@gmail.com>

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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox