Re: [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks

2019-09-11 Thread Vijay Khemka 


On 9/11/19, 5:16 AM, "Linux-aspeed on behalf of Rashmica Gupta" 
 wrote:

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

Signed-off-by: Rashmica Gupta 
---
 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

Re: [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks

2019-09-05 Thread Andy Shevchenko 
On Thu, Sep 5, 2019 at 2:17 AM Rashmica Gupta  wrote:
> On Wed, 2019-09-04 at 19:27 +0300, Andy Shevchenko wrote:
> > On Wed, Sep 4, 2019 at 9:14 AM Rashmica Gupta 
> > 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

Re: [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks

2019-09-04 Thread Rashmica Gupta 
On Wed, 2019-09-04 at 19:27 +0300, Andy Shevchenko wrote:
> On Wed, Sep 4, 2019 at 9:14 AM Rashmica Gupta 
> wrote:
> > Fixes: 361b79119a4b7 ('gpio: Add Aspeed driver')
> > 
> > Signed-off-by: Rashmica Gupta 
> > /* 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);
> 
>

Re: [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks

2019-09-04 Thread Andy Shevchenko 
On Wed, Sep 4, 2019 at 9:14 AM Rashmica Gupta  wrote:
>
> Fixes: 361b79119a4b7 ('gpio: Add Aspeed driver')
>
> Signed-off-by: Rashmica Gupta 

> /* 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

Re: [PATCH 1/4] gpio/aspeed: Fix incorrect number of banks

2019-09-03 Thread Bartosz Golaszewski 
śr., 4 wrz 2019 o 08:13 Rashmica Gupta  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 
> ---
>  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
>