[PATCH 0/3] ARM: davinci: fix ethernet support on da850-evm

[PATCH 0/3] ARM: davinci: fix ethernet support on da850-evm

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Ethernet doesn't work on da850-evm in legacy boot mode since v4.19. This
series addresses two problems I've found. I guess with this kind of
intrusive changes in the GPIO driver, other boards may be broken at the
moment too.

Bartosz Golaszewski (3):
  ARM: davinci: define gpio interrupts as separate resources
  gpio: davinci: restore a way to manually specify the GPIO base
  ARM: davinci: fix da850-evm boot in legacy mode

 arch/arm/mach-davinci/da850.c              |  4 ++-
 arch/arm/mach-davinci/devices-da8xx.c      | 42 +++++++++++++++++++++-
 drivers/gpio/gpio-davinci.c                |  2 +-
 include/linux/platform_data/gpio-davinci.h |  2 ++
 4 files changed, 47 insertions(+), 3 deletions(-)

-- 
2.19.1

[PATCH 1/3] ARM: davinci: define gpio interrupts as separate resources

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Since commit eb3744a2dd01 ("gpio: davinci: Do not assume continuous
IRQ numbering") the davinci GPIO driver fails to probe if we boot
in legacy mode from any of the board files. Since the driver now
expects every interrupt to be defined as a separate resource, split
the definition in devices-da8xx.c instead of having a single continuous
interrupt range.

Fixes: eb3744a2dd01 ("gpio: davinci: Do not assume continuous IRQ numbering")
Cc: stable@vger.kernel.org
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/mach-davinci/devices-da8xx.c | 42 ++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index 1fd3619f6a09..8c4ae9866e3c 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -701,9 +701,49 @@ static struct resource da8xx_gpio_resources[] = {
 	},
 	{ /* interrupt */
 		.start	= IRQ_DA8XX_GPIO0,
-		.end	= IRQ_DA8XX_GPIO8,
+		.end	= IRQ_DA8XX_GPIO0,
 		.flags	= IORESOURCE_IRQ,
 	},
+	{
+		.start	= IRQ_DA8XX_GPIO1,
+		.end	= IRQ_DA8XX_GPIO1,
+		.flags	= IORESOURCE_IRQ,
+	},
+	{
+		.start  = IRQ_DA8XX_GPIO2,
+		.end    = IRQ_DA8XX_GPIO2,
+		.flags  = IORESOURCE_IRQ,
+	},
+	{
+		.start  = IRQ_DA8XX_GPIO3,
+		.end    = IRQ_DA8XX_GPIO3,
+		.flags  = IORESOURCE_IRQ,
+	},
+	{
+		.start  = IRQ_DA8XX_GPIO4,
+		.end    = IRQ_DA8XX_GPIO4,
+		.flags  = IORESOURCE_IRQ,
+	},
+	{
+		.start  = IRQ_DA8XX_GPIO5,
+		.end    = IRQ_DA8XX_GPIO5,
+		.flags  = IORESOURCE_IRQ,
+	},
+	{
+		.start  = IRQ_DA8XX_GPIO6,
+		.end    = IRQ_DA8XX_GPIO6,
+		.flags  = IORESOURCE_IRQ,
+	},
+	{
+		.start  = IRQ_DA8XX_GPIO7,
+		.end    = IRQ_DA8XX_GPIO7,
+		.flags  = IORESOURCE_IRQ,
+	},
+	{
+		.start  = IRQ_DA8XX_GPIO8,
+		.end    = IRQ_DA8XX_GPIO8,
+		.flags  = IORESOURCE_IRQ,
+	}
 };
 
 static struct platform_device da8xx_gpio_device = {
-- 
2.19.1

Re: [PATCH 1/3] ARM: davinci: define gpio interrupts as separate resources

[Sekhar Nori]

On 13/11/18 7:20 PM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Since commit eb3744a2dd01 ("gpio: davinci: Do not assume continuous
> IRQ numbering") the davinci GPIO driver fails to probe if we boot
> in legacy mode from any of the board files. Since the driver now
> expects every interrupt to be defined as a separate resource, split
> the definition in devices-da8xx.c instead of having a single continuous
> interrupt range.
> 
> Fixes: eb3744a2dd01 ("gpio: davinci: Do not assume continuous IRQ numbering")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

There are a number of other boards that need such fixing too. And the
commit in question does not do a good job of explaining why it was
needed in the first place. The description  just repeats what can be
inferred by reading the patch.


    gpio: davinci: Do not assume continuous IRQ numbering

    Currently the driver assumes that the interrupts are continuous
    and does platform_get_irq only once and assumes the rest are continuous,
    instead call platform_get_irq for all the interrupts and store them
    in an array for later use.

    Signed-off-by: Keerthy <j-keerthy@ti.com>
    Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>
    Signed-off-by: Linus Walleij <linus.walleij@linaro.org>


Can we revert the offending commit instead?

Thanks,
Sekhar

Re: [PATCH 1/3] ARM: davinci: define gpio interrupts as separate resources

[J, KEERTHY]

On 11/20/2018 2:22 AM, Sekhar Nori wrote:
> On 13/11/18 7:20 PM, Bartosz Golaszewski wrote:
>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>
>> Since commit eb3744a2dd01 ("gpio: davinci: Do not assume continuous
>> IRQ numbering") the davinci GPIO driver fails to probe if we boot
>> in legacy mode from any of the board files. Since the driver now
>> expects every interrupt to be defined as a separate resource, split
>> the definition in devices-da8xx.c instead of having a single continuous
>> interrupt range.
>>
>> Fixes: eb3744a2dd01 ("gpio: davinci: Do not assume continuous IRQ numbering")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> There are a number of other boards that need such fixing too. And the
> commit in question does not do a good job of explaining why it was
> needed in the first place. The description  just repeats what can be
> inferred by reading the patch.

Cc Lokesh

Sekhar,

DT explicitly mentions every IRQ number. The patch in discussion
explicitly calls platform_get_irq for all the interrupts which to me is 
the right thing to do as: platform_get_irq--> 
of_irq_get-->irq_create_of_mapping--> sequence is to be done for every IRQ.

k3-am654 definitely will need explicit calls to platform_get_irq as it 
will be involving interrupt router and interrupt numbers need not be 
continuous.

So i do not think reverting the patch is the right idea.

Regards,
Keerthy

> 
> 
>      gpio: davinci: Do not assume continuous IRQ numbering
> 
>      Currently the driver assumes that the interrupts are continuous
>      and does platform_get_irq only once and assumes the rest are continuous,
>      instead call platform_get_irq for all the interrupts and store them
>      in an array for later use.
> 
>      Signed-off-by: Keerthy <j-keerthy@ti.com>
>      Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>
>      Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> 
> 
> Can we revert the offending commit instead?
> 
> Thanks,
> Sekhar
> 

Re: [PATCH 1/3] ARM: davinci: define gpio interrupts as separate resources

[Sekhar Nori]

On 20/11/18 12:08 PM, J, KEERTHY wrote:
> 
> 
> On 11/20/2018 2:22 AM, Sekhar Nori wrote:
>> On 13/11/18 7:20 PM, Bartosz Golaszewski wrote:
>>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>
>>> Since commit eb3744a2dd01 ("gpio: davinci: Do not assume continuous
>>> IRQ numbering") the davinci GPIO driver fails to probe if we boot
>>> in legacy mode from any of the board files. Since the driver now
>>> expects every interrupt to be defined as a separate resource, split
>>> the definition in devices-da8xx.c instead of having a single continuous
>>> interrupt range.
>>>
>>> Fixes: eb3744a2dd01 ("gpio: davinci: Do not assume continuous IRQ
>>> numbering")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>
>> There are a number of other boards that need such fixing too. And the
>> commit in question does not do a good job of explaining why it was
>> needed in the first place. The description  just repeats what can be
>> inferred by reading the patch.
> 
> Cc Lokesh
> 
> Sekhar,
> 
> DT explicitly mentions every IRQ number. The patch in discussion
> explicitly calls platform_get_irq for all the interrupts which to me is
> the right thing to do as: platform_get_irq-->
> of_irq_get-->irq_create_of_mapping--> sequence is to be done for every IRQ.
> 
> k3-am654 definitely will need explicit calls to platform_get_irq as it
> will be involving interrupt router and interrupt numbers need not be
> continuous.
> 
> So i do not think reverting the patch is the right idea.

Well, all of this description of patch motivation should have been in
the patch description to begin with.

Bartosz, can you please extend this patch to fix this problem for other
DaVinci SoCs too? I am on the road this week, but will do my best to
queue these fixes at the earliest .

Thanks,
Sekhar

Re: [PATCH 1/3] ARM: davinci: define gpio interrupts as separate resources

[Bartosz Golaszewski]

śr., 21 lis 2018 o 00:07 Sekhar Nori <nsekhar@ti.com> napisał(a):
>
> On 20/11/18 12:08 PM, J, KEERTHY wrote:
> >
> >
> > On 11/20/2018 2:22 AM, Sekhar Nori wrote:
> >> On 13/11/18 7:20 PM, Bartosz Golaszewski wrote:
> >>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >>>
> >>> Since commit eb3744a2dd01 ("gpio: davinci: Do not assume continuous
> >>> IRQ numbering") the davinci GPIO driver fails to probe if we boot
> >>> in legacy mode from any of the board files. Since the driver now
> >>> expects every interrupt to be defined as a separate resource, split
> >>> the definition in devices-da8xx.c instead of having a single continuous
> >>> interrupt range.
> >>>
> >>> Fixes: eb3744a2dd01 ("gpio: davinci: Do not assume continuous IRQ
> >>> numbering")
> >>> Cc: stable@vger.kernel.org
> >>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >>
> >> There are a number of other boards that need such fixing too. And the
> >> commit in question does not do a good job of explaining why it was
> >> needed in the first place. The description  just repeats what can be
> >> inferred by reading the patch.
> >
> > Cc Lokesh
> >
> > Sekhar,
> >
> > DT explicitly mentions every IRQ number. The patch in discussion
> > explicitly calls platform_get_irq for all the interrupts which to me is
> > the right thing to do as: platform_get_irq-->
> > of_irq_get-->irq_create_of_mapping--> sequence is to be done for every IRQ.
> >
> > k3-am654 definitely will need explicit calls to platform_get_irq as it
> > will be involving interrupt router and interrupt numbers need not be
> > continuous.
> >
> > So i do not think reverting the patch is the right idea.
>
> Well, all of this description of patch motivation should have been in
> the patch description to begin with.
>
> Bartosz, can you please extend this patch to fix this problem for other
> DaVinci SoCs too? I am on the road this week, but will do my best to
> queue these fixes at the earliest .
>
> Thanks,
> Sekhar

Ok, to make it easier for you, I'll resend all the patches addressing
the GPIO issue.

Bart

[PATCH 2/3] gpio: davinci: restore a way to manually specify the GPIO base

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Commit 587f7a694f01 ("gpio: davinci: Use dev name for label and
automatic base selection") broke the network support in legacy boot
mode for da850-evm since we can no longer request the MDIO clock GPIO.

Other boards may be broken too, which I haven't tested.

The problem is in the fact that most board files still use the legacy
GPIO API where lines are requested by numbers rather than descriptors.

While this should be fixed eventually, in order to unbreak the board
for now - provide a way to manually specify the GPIO base in platform
data.

Fixes: 587f7a694f01 ("gpio: davinci: Use dev name for label and automatic base selection")
Cc: stable@vger.kernel.org
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/gpio/gpio-davinci.c                | 2 +-
 include/linux/platform_data/gpio-davinci.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 5c1564fcc24e..bdb29e51b417 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -258,7 +258,7 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 	chips->chip.set = davinci_gpio_set;
 
 	chips->chip.ngpio = ngpio;
-	chips->chip.base = -1;
+	chips->chip.base = pdata->no_auto_base ? pdata->base : -1;
 
 #ifdef CONFIG_OF_GPIO
 	chips->chip.of_gpio_n_cells = 2;
diff --git a/include/linux/platform_data/gpio-davinci.h b/include/linux/platform_data/gpio-davinci.h
index f92a47e18034..a93841bfb9f7 100644
--- a/include/linux/platform_data/gpio-davinci.h
+++ b/include/linux/platform_data/gpio-davinci.h
@@ -17,6 +17,8 @@
 #define __DAVINCI_GPIO_PLATFORM_H
 
 struct davinci_gpio_platform_data {
+	bool	no_auto_base;
+	u32	base;
 	u32	ngpio;
 	u32	gpio_unbanked;
 };
-- 
2.19.1

Re: [PATCH 2/3] gpio: davinci: restore a way to manually specify the GPIO base

[Linus Walleij]

On Tue, Nov 13, 2018 at 2:51 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Commit 587f7a694f01 ("gpio: davinci: Use dev name for label and
> automatic base selection") broke the network support in legacy boot
> mode for da850-evm since we can no longer request the MDIO clock GPIO.
>
> Other boards may be broken too, which I haven't tested.
>
> The problem is in the fact that most board files still use the legacy
> GPIO API where lines are requested by numbers rather than descriptors.
>
> While this should be fixed eventually, in order to unbreak the board
> for now - provide a way to manually specify the GPIO base in platform
> data.
>
> Fixes: 587f7a694f01 ("gpio: davinci: Use dev name for label and automatic base selection")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

I guess this will be merged throug ARM SoC?

Yours,
Linus Walleij

Re: [PATCH 2/3] gpio: davinci: restore a way to manually specify the GPIO base

[Bartosz Golaszewski]

pon., 19 lis 2018 o 14:19 Linus Walleij <linus.walleij@linaro.org> napisał(a):
>
> On Tue, Nov 13, 2018 at 2:51 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Commit 587f7a694f01 ("gpio: davinci: Use dev name for label and
> > automatic base selection") broke the network support in legacy boot
> > mode for da850-evm since we can no longer request the MDIO clock GPIO.
> >
> > Other boards may be broken too, which I haven't tested.
> >
> > The problem is in the fact that most board files still use the legacy
> > GPIO API where lines are requested by numbers rather than descriptors.
> >
> > While this should be fixed eventually, in order to unbreak the board
> > for now - provide a way to manually specify the GPIO base in platform
> > data.
> >
> > Fixes: 587f7a694f01 ("gpio: davinci: Use dev name for label and automatic base selection")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>
> I guess this will be merged throug ARM SoC?
>

Yes, if Sekhar's fine with that.

Bart

[PATCH 3/3] ARM: davinci: fix da850-evm boot in legacy mode

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Commit 587f7a694f01 ("gpio: davinci: Use dev name for label and
automatic base selection") broke the network support in legacy boot
mode for da850-evm since we can no longer request the MDIO clock GPIO.

We now have the option to specify the GPIO base manually for davinci,
so add the relevant fields to platform data.

Fixes: 587f7a694f01 ("gpio: davinci: Use dev name for label and automatic base selection")
Cc: stable@vger.kernel.org
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/mach-davinci/da850.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 4528bbf0c861..e7b78df2bfef 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -719,7 +719,9 @@ int __init da850_register_vpif_capture(struct vpif_capture_config
 }
 
 static struct davinci_gpio_platform_data da850_gpio_platform_data = {
-	.ngpio = 144,
+	.no_auto_base	= true,
+	.base		= 0,
+	.ngpio		= 144,
 };
 
 int __init da850_register_gpio(void)
-- 
2.19.1

Re: [PATCH 3/3] ARM: davinci: fix da850-evm boot in legacy mode

[Sekhar Nori]

Hi Bartosz,


On 13/11/18 7:20 PM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Commit 587f7a694f01 ("gpio: davinci: Use dev name for label and
> automatic base selection") broke the network support in legacy boot
> mode for da850-evm since we can no longer request the MDIO clock GPIO.
> 
> We now have the option to specify the GPIO base manually for davinci,
> so add the relevant fields to platform data.
> 
> Fixes: 587f7a694f01 ("gpio: davinci: Use dev name for label and automatic base selection")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>


> ---
>  arch/arm/mach-davinci/da850.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Similar changes are needed for other 5 DaVinci SoCs too. Can you do
those in this patch itself?

Thanks,
Sekhar

Re: [PATCH 0/3] ARM: davinci: fix ethernet support on da850-evm

[Linus Walleij]

On Tue, Nov 13, 2018 at 2:51 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Ethernet doesn't work on da850-evm in legacy boot mode since v4.19. This
> series addresses two problems I've found. I guess with this kind of
> intrusive changes in the GPIO driver, other boards may be broken at the
> moment too.
>
> Bartosz Golaszewski (3):
>   ARM: davinci: define gpio interrupts as separate resources
>   gpio: davinci: restore a way to manually specify the GPIO base
>   ARM: davinci: fix da850-evm boot in legacy mode

I have some patches starting to move DaVinci over to using only
descriptor tables, but that's not somthing we can pull off overnight
so I guess we have to go for this solution.

Can we have ACKs from the DaVinci maintainers so I can merge
all three patches through the GPIO tree fixes? Else I'm fine
with merging through ARM SoC.

Yours,
Linus Walleij

Re: [PATCH 0/3] ARM: davinci: fix ethernet support on da850-evm

[Bartosz Golaszewski]

pt., 16 lis 2018 o 22:51 Linus Walleij <linus.walleij@linaro.org> napisał(a):
>
> On Tue, Nov 13, 2018 at 2:51 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Ethernet doesn't work on da850-evm in legacy boot mode since v4.19. This
> > series addresses two problems I've found. I guess with this kind of
> > intrusive changes in the GPIO driver, other boards may be broken at the
> > moment too.
> >
> > Bartosz Golaszewski (3):
> >   ARM: davinci: define gpio interrupts as separate resources
> >   gpio: davinci: restore a way to manually specify the GPIO base
> >   ARM: davinci: fix da850-evm boot in legacy mode
>
> I have some patches starting to move DaVinci over to using only
> descriptor tables, but that's not somthing we can pull off overnight
> so I guess we have to go for this solution.
>

Nice, would you mind sharing the development branch? I'm interested in
it as well and may be doing some work on that early 2019 so let's sync
to avoid duplicating the effort.

Bart

> Can we have ACKs from the DaVinci maintainers so I can merge
> all three patches through the GPIO tree fixes? Else I'm fine
> with merging through ARM SoC.
>
> Yours,
> Linus Walleij

Re: [PATCH 0/3] ARM: davinci: fix ethernet support on da850-evm

[Linus Walleij]

On Mon, Nov 19, 2018 at 10:11 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> pt., 16 lis 2018 o 22:51 Linus Walleij <linus.walleij@linaro.org> napisał(a):

> > I have some patches starting to move DaVinci over to using only
> > descriptor tables, but that's not somthing we can pull off overnight
> > so I guess we have to go for this solution.
> >
>
> Nice, would you mind sharing the development branch? I'm interested in
> it as well and may be doing some work on that early 2019 so let's sync
> to avoid duplicating the effort.

Just a stab:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/commit/?h=gpio-descriptors-leds&id=b4db69c2a066a87c990a6757720284df88f11f57

It will also require this patch (as I realized after talking to the PXA
people):
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/commit/?h=gpio-descriptors-mmc-cd-wp&id=1911a4fe1ed8805080d8c7e0adc298f740531c3b

I guess I should queue the naming patch in some immutable branch
or something so it can be pulled in all over the place.

Yours,
Linus Walleij