[PATCH v3] sh: avoid using IRQ0 on SH3/4

[PATCH v3] sh: avoid using IRQ0 on SH3/4

[Sergey Shtylyov]

Using IRQ0 by the platform devices is going to be disallowed soon (see [1])
and even now, when IRQ0 is about to be returned by platfrom_get_irq(), you
see a big warning.  The code supporting SH3/4 SoCs maps the IRQ #s starting
at 0 -- modify that code to start the IRQ #s from 16 instead.

The patch should mostly affect the AP-SH4A-3A/AP-SH4AD-0A boards as they
indeed use IRQ0 for the SMSC911x compatible Ethernet chip...

[1] https://lore.kernel.org/all/025679e1-1f0a-ae4b-4369-01164f691511@omp.ru/

Fixes: a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is invalid")
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

---
The patch is against Linus Torvalds' 'linux.git' repo.

Changes in version 3:
- added an appropriate Fixes: tag and added a passage about it to the patch
  description;
- added actual cases of the boards using IRQ0 to the patch description;
- added Geert Uytterhoeven's and John Paul Adrian Glaubitz's tags;
- updated the link to point to the version 2 of the patch.

Changes in version 2:
- changed cmp/ge to cmp/hs in the assembly code.

 arch/sh/kernel/cpu/sh3/entry.S |    4 ++--
 include/linux/sh_intc.h        |    6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

Index: linux/arch/sh/kernel/cpu/sh3/entry.S
===================================================================
--- linux.orig/arch/sh/kernel/cpu/sh3/entry.S
+++ linux/arch/sh/kernel/cpu/sh3/entry.S
@@ -470,9 +470,9 @@ ENTRY(handle_interrupt)
 	mov	r4, r0		! save vector->jmp table offset for later
 
 	shlr2	r4		! vector to IRQ# conversion
-	add	#-0x10, r4
 
-	cmp/pz	r4		! is it a valid IRQ?
+	mov	#0x10, r5
+	cmp/hs	r5, r4		! is it a valid IRQ?
 	bt	10f
 
 	/*
Index: linux/include/linux/sh_intc.h
===================================================================
--- linux.orig/include/linux/sh_intc.h
+++ linux/include/linux/sh_intc.h
@@ -13,9 +13,9 @@
 /*
  * Convert back and forth between INTEVT and IRQ values.
  */
-#ifdef CONFIG_CPU_HAS_INTEVT
-#define evt2irq(evt)		(((evt) >> 5) - 16)
-#define irq2evt(irq)		(((irq) + 16) << 5)
+#ifdef CONFIG_CPU_HAS_INTEVT	/* Avoid IRQ0 (invalid for platform devices) */
+#define evt2irq(evt)		((evt) >> 5)
+#define irq2evt(irq)		((irq) << 5)
 #else
 #define evt2irq(evt)		(evt)
 #define irq2evt(irq)		(irq)

Re: [PATCH v3] sh: avoid using IRQ0 on SH3/4

[Sergey Shtylyov]

On 4/27/22 9:46 PM, Sergey Shtylyov wrote:

> Using IRQ0 by the platform devices is going to be disallowed soon (see [1])
> and even now, when IRQ0 is about to be returned by platfrom_get_irq(), you

   Oops, it's platform_get_irq(). :-/

> see a big warning.  The code supporting SH3/4 SoCs maps the IRQ #s starting
> at 0 -- modify that code to start the IRQ #s from 16 instead.
> 
> The patch should mostly affect the AP-SH4A-3A/AP-SH4AD-0A boards as they
> indeed use IRQ0 for the SMSC911x compatible Ethernet chip...
> 
> [1] https://lore.kernel.org/all/025679e1-1f0a-ae4b-4369-01164f691511@omp.ru/
> 
> Fixes: a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is invalid")
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

[...]

MBR, Sergey

Re: [PATCH v3] sh: avoid using IRQ0 on SH3/4

[John Paul Adrian Glaubitz]

Hi Sergey!

On 4/27/22 20:46, Sergey Shtylyov wrote:
> Using IRQ0 by the platform devices is going to be disallowed soon (see [1])
> and even now, when IRQ0 is about to be returned by platfrom_get_irq(), you
> see a big warning.  The code supporting SH3/4 SoCs maps the IRQ #s starting
> at 0 -- modify that code to start the IRQ #s from 16 instead.
> 
> The patch should mostly affect the AP-SH4A-3A/AP-SH4AD-0A boards as they
> indeed use IRQ0 for the SMSC911x compatible Ethernet chip...

Maybe try getting it landed through Andrew Morton's tree?

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH v3] sh: avoid using IRQ0 on SH3/4

[Greg Kroah-Hartman]

On Fri, Apr 29, 2022 at 04:24:04PM +0200, John Paul Adrian Glaubitz wrote:
> Hi Sergey!
> 
> On 4/27/22 20:46, Sergey Shtylyov wrote:
> > Using IRQ0 by the platform devices is going to be disallowed soon (see [1])
> > and even now, when IRQ0 is about to be returned by platfrom_get_irq(), you
> > see a big warning.  The code supporting SH3/4 SoCs maps the IRQ #s starting
> > at 0 -- modify that code to start the IRQ #s from 16 instead.
> > 
> > The patch should mostly affect the AP-SH4A-3A/AP-SH4AD-0A boards as they
> > indeed use IRQ0 for the SMSC911x compatible Ethernet chip...
> 
> Maybe try getting it landed through Andrew Morton's tree?

I can take it if needed, why does sh patches go through Andrew's tree?
Is there no sh maintainer tree anymore?

thanks,

greg k-h

Re: [PATCH v3] sh: avoid using IRQ0 on SH3/4

[Rich Felker]

On Fri, Apr 29, 2022 at 04:24:04PM +0200, John Paul Adrian Glaubitz wrote:
> Hi Sergey!
> 
> On 4/27/22 20:46, Sergey Shtylyov wrote:
> > Using IRQ0 by the platform devices is going to be disallowed soon (see [1])
> > and even now, when IRQ0 is about to be returned by platfrom_get_irq(), you
> > see a big warning.  The code supporting SH3/4 SoCs maps the IRQ #s starting
> > at 0 -- modify that code to start the IRQ #s from 16 instead.
> > 
> > The patch should mostly affect the AP-SH4A-3A/AP-SH4AD-0A boards as they
> > indeed use IRQ0 for the SMSC911x compatible Ethernet chip...
> 
> Maybe try getting it landed through Andrew Morton's tree?

Hi. I'm alive and looking at it. If it needs to go in for this cycle I
will send a pull request for just this and anything else critical. Was
trying to get to it last night but had some unpleasant surprises come
up that took me away from the computer.

Rich

Re: [PATCH v3] sh: avoid using IRQ0 on SH3/4

[Sergey Shtylyov]

On 4/29/22 8:16 PM, Rich Felker wrote:

[...]
>>> Using IRQ0 by the platform devices is going to be disallowed soon (see [1])
>>> and even now, when IRQ0 is about to be returned by platfrom_get_irq(), you
>>> see a big warning.  The code supporting SH3/4 SoCs maps the IRQ #s starting
>>> at 0 -- modify that code to start the IRQ #s from 16 instead.
>>>
>>> The patch should mostly affect the AP-SH4A-3A/AP-SH4AD-0A boards as they
>>> indeed use IRQ0 for the SMSC911x compatible Ethernet chip...
>>
>> Maybe try getting it landed through Andrew Morton's tree?
> 
> Hi. I'm alive and looking at it. If it needs to go in for this cycle I
> will send a pull request for just this and anything else critical. Was

   Well, now using IRQ0 just causes a WARNing in platform_get_irq() -- I don't
think fixing it is critical enough. Starting from 5.19-rc1 the SMSC91xx driver
should stop working on the mentioned boards.
   But let me look at the SMSC driver itself, I haven't done this yet...

> trying to get to it last night but had some unpleasant surprises come
> up that took me away from the computer.

   :-(

> Rich

MBR, Sergey

Re: [PATCH v3] sh: avoid using IRQ0 on SH3/4

[Sergey Shtylyov]

On 5/1/22 8:58 PM, Sergey Shtylyov wrote:
[...]
>>>> Using IRQ0 by the platform devices is going to be disallowed soon (see [1])
>>>> and even now, when IRQ0 is about to be returned by platfrom_get_irq(), you
>>>> see a big warning.  The code supporting SH3/4 SoCs maps the IRQ #s starting
>>>> at 0 -- modify that code to start the IRQ #s from 16 instead.
>>>>
>>>> The patch should mostly affect the AP-SH4A-3A/AP-SH4AD-0A boards as they
>>>> indeed use IRQ0 for the SMSC911x compatible Ethernet chip...
>>>
>>> Maybe try getting it landed through Andrew Morton's tree?
>>
>> Hi. I'm alive and looking at it. If it needs to go in for this cycle I
>> will send a pull request for just this and anything else critical. Was
> 
>    Well, now using IRQ0 just causes a WARNing in platform_get_irq() -- I don't
> think fixing it is critical enough. Starting from 5.19-rc1 the SMSC91xx driver
> should stop working on the mentioned boards.
>    But let me look at the SMSC driver itself, I haven't done this yet...

   Looks like these boards were borked back in 2015 by this commit:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=965b2aa78fbcb831acf4f669f494da201f4bcace

   It stopped accepting IRQ0 for no apparent reason. :-/

[...]

>> Rich

MBR, Sergey

Re: [PATCH v3] sh: avoid using IRQ0 on SH3/4

[Rob Landley]

On 4/29/22 09:24, John Paul Adrian Glaubitz wrote:
> Hi Sergey!
> 
> On 4/27/22 20:46, Sergey Shtylyov wrote:
>> Using IRQ0 by the platform devices is going to be disallowed soon (see [1])
>> and even now, when IRQ0 is about to be returned by platfrom_get_irq(), you
>> see a big warning.  The code supporting SH3/4 SoCs maps the IRQ #s starting
>> at 0 -- modify that code to start the IRQ #s from 16 instead.
>> 
>> The patch should mostly affect the AP-SH4A-3A/AP-SH4AD-0A boards as they
>> indeed use IRQ0 for the SMSC911x compatible Ethernet chip...
> 
> Maybe try getting it landed through Andrew Morton's tree?

As I told him in IRC, the problem is still that sh4 never gives me a shell
prompt with this patch applied. I just reconfirmed it against current git:

Freeing unused kernel image (initmem) memory: 124K
This architecture does not have kernel memory protection.
Run /init as init process
mountpoint: dev/pts: No such file or directory
8139cp 0000:00:02.0 eth0: link up, 100Mbps, full-duplex, lpa 0x05E1

It makes it partway through the init script, but it hangs with qemu-system-sh4
stuck in a CPU-eating loop before finishing. Without the patch, I get a shell
prompt.

If you don't want to build the userspace from source with mkroot, the last
release's binary tarball is 4 megs and reproduced the problem just fine. First,
confirm it works as-shipped:

$ wget https://landley.net/toybox/downloads/binaries/mkroot/latest/sh4.tgz
...
$ tar xvf sh4.tgz
...
$ cd sh4
$ ./qemu-sh4.sh
...
printk: console [netcon0] enabled
netconsole: network logging started
Freeing unused kernel image (initmem) memory: 116K
This architecture does not have kernel memory protection.
Run /init as init process
8139cp 0000:00:02.0 eth0: link up, 100Mbps, full-duplex, lpa 0x05E1
random: fast init done
Type exit when done.
# exit
reboot: Restarting system

landley@driftwood:~/sub/sh4$

Once you've confirmed that works with your qemu-system-sh4 install, replace the
kernel using the config in that directory:

$ git clone ~/linux/linux linux
...
$ cd linux
$ patch -p1 -i ~/linux/sh4irq.eml
...
$ CROSS_COMPILE=~/mcm/ccc/sh4-linux-musl-cross/bin/sh4-linux-musl- make \
  ARCH=sh allnoconfig KCONFIG_ALLCONFIG=../miniconfig-sh4
...
$ CROSS_COMPILE=~/mcm/ccc/sh4-linux-musl-cross/bin/sh4-linux-musl- make \
  ARCH=sh -j $(nproc)
...
$ cp arch/sh/boot/zImage ..
$ cd ..
$ ./qemu-*.sh
...

and it hangs without ever saying "random: fast init done" or giving a prompt.

(You could also use the linux-fullconfig file to build your kernel, but you'll
have to say "n" to a bunch of make oldconfig questions.)

> Adrian

Rob

Re: [PATCH v3] sh: avoid using IRQ0 on SH3/4

[John Paul Adrian Glaubitz]

Hi Rob!

On 4/30/22 12:30, Rob Landley wrote:
>> Maybe try getting it landed through Andrew Morton's tree?
> 
> As I told him in IRC, the problem is still that sh4 never gives me a shell
> prompt with this patch applied. I just reconfirmed it against current git:
> 
> Freeing unused kernel image (initmem) memory: 124K
> This architecture does not have kernel memory protection.
> Run /init as init process
> mountpoint: dev/pts: No such file or directory
> 8139cp 0000:00:02.0 eth0: link up, 100Mbps, full-duplex, lpa 0x05E1
> 
> It makes it partway through the init script, but it hangs with qemu-system-sh4
> stuck in a CPU-eating loop before finishing. Without the patch, I get a shell
> prompt.

Oh, I wasn't aware of that. I did not experience the issue on my SH7785LCR but I can
retest against current git with the patch applied on top.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH v3] sh: avoid using IRQ0 on SH3/4

[Geert Uytterhoeven]

Hi Rob,

On Sat, Apr 30, 2022 at 3:52 PM Rob Landley <rob@landley.net> wrote:
> On 4/29/22 09:24, John Paul Adrian Glaubitz wrote:
> > On 4/27/22 20:46, Sergey Shtylyov wrote:
> >> Using IRQ0 by the platform devices is going to be disallowed soon (see [1])
> >> and even now, when IRQ0 is about to be returned by platfrom_get_irq(), you
> >> see a big warning.  The code supporting SH3/4 SoCs maps the IRQ #s starting
> >> at 0 -- modify that code to start the IRQ #s from 16 instead.
> >>
> >> The patch should mostly affect the AP-SH4A-3A/AP-SH4AD-0A boards as they
> >> indeed use IRQ0 for the SMSC911x compatible Ethernet chip...

> As I told him in IRC, the problem is still that sh4 never gives me a shell
> prompt with this patch applied. I just reconfirmed it against current git:
>
> Freeing unused kernel image (initmem) memory: 124K
> This architecture does not have kernel memory protection.
> Run /init as init process
> mountpoint: dev/pts: No such file or directory
> 8139cp 0000:00:02.0 eth0: link up, 100Mbps, full-duplex, lpa 0x05E1
>
> It makes it partway through the init script, but it hangs with qemu-system-sh4
> stuck in a CPU-eating loop before finishing. Without the patch, I get a shell
> prompt.

I regularly test on qemu rts7751r2d, but couldn't produce your
issue.  Until I tried "ifconfig eth0 up", which causes a lock-up.
Interestingly, the 8139 irq was 112 with and without Sergey's patch,
so there must be an irq remapping missing.

I also test regularly on landisk, where 8139 Ethernet works fine.
Turns out landisk uses arch/sh/drivers/pci/fixups-landisk.c to fixup
the irq...

arch/sh/include/mach-common/mach/r2d.h has:
#define R2D_FPGA_IRQ_BASE       100
Subtracting 16 here does not help.

With this (gmail-whitespace-damaged) patch:

--- a/arch/sh/drivers/pci/fixups-rts7751r2d.c
+++ b/arch/sh/drivers/pci/fixups-rts7751r2d.c
@@ -31,9 +31,9 @@ static char lboxre2_irq_tab[] = {
 int pcibios_map_platform_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
 {
        if (mach_is_lboxre2())
-               return lboxre2_irq_tab[slot];
+               return lboxre2_irq_tab[slot] - 16;
        else
-               return rts7751r2d_irq_tab[slot];
+               return rts7751r2d_irq_tab[slot] - 16;
 }

 int pci_fixup_pcic(struct pci_channel *chan)

it no longer crashes, but ifconfig still fails:

/ # ifconfig eth0 up
ifconfig: ioctl 0x8914 failed: Invalid argument

Note that there are more implementations of pcibios_map_platform_irq()
that do not use evt2irq(), and thus are probably broken by this patch.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3] sh: avoid using IRQ0 on SH3/4

[Rob Landley]

On 5/2/22 03:37, Geert Uytterhoeven wrote:
> Hi Rob,
...
> Until I tried "ifconfig eth0 up", which causes a lock-up.
> Interestingly, the 8139 irq was 112 with and without Sergey's patch,
> so there must be an irq remapping missing.

Yup, that's it.

> I also test regularly on landisk, where 8139 Ethernet works fine.
> Turns out landisk uses arch/sh/drivers/pci/fixups-landisk.c to fixup
> the irq...

I didn't think the patch was wrong per se, just that something broke when
jiggled. :(

> arch/sh/include/mach-common/mach/r2d.h has:
> #define R2D_FPGA_IRQ_BASE       100
> Subtracting 16 here does not help.
> 
> With this (gmail-whitespace-damaged) patch:
> 
> --- a/arch/sh/drivers/pci/fixups-rts7751r2d.c
> +++ b/arch/sh/drivers/pci/fixups-rts7751r2d.c
> @@ -31,9 +31,9 @@ static char lboxre2_irq_tab[] = {
>  int pcibios_map_platform_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
>  {
>         if (mach_is_lboxre2())
> -               return lboxre2_irq_tab[slot];
> +               return lboxre2_irq_tab[slot] - 16;
>         else
> -               return rts7751r2d_irq_tab[slot];
> +               return rts7751r2d_irq_tab[slot] - 16;
>  }
> 
>  int pci_fixup_pcic(struct pci_channel *chan)
> 
> it no longer crashes, but ifconfig still fails:
> 
> / # ifconfig eth0 up
> ifconfig: ioctl 0x8914 failed: Invalid argument

Sounds like it's now outside of the IRQ range allocation, but I can't find where
that's requested when registering the controller? (What is a "swizzle" anyway?)

I'm looking at kernel/cpu/sh4/setup-sh7750.c but I don't understand why it might
work for landisk but not there. (Bit out of my depth in this plumbing.
Head-scratching at include/linux/sh_intc.h #defining DECLARE_INTC_DESC()... hard
to work backwards to find where this stuff STARTS...)

> Note that there are more implementations of pcibios_map_platform_irq()
> that do not use evt2irq(), and thus are probably broken by this patch.

Yup. Sounds like something could be consolidated. Unfortunately I only have 4
test systems for this platform, only 2 of which are easy to cycle...

> Gr{oetje,eeting}s,
> 
>                         Geert

I can try sticking printk() into this to track it down if you haven't got any
more time to look at it. I don't understand this plumbing very well but "error
return code comes from here, that tested this variable, which was set here..."
is generally a deterministic approach, if glacial.

Rob

Re: [PATCH v3] sh: avoid using IRQ0 on SH3/4

[Geert Uytterhoeven]

Hi Rob,

On Mon, May 2, 2022 at 10:02 PM Rob Landley <rob@landley.net> wrote:
> Sounds like it's now outside of the IRQ range allocation, but I can't find where
> that's requested when registering the controller? (What is a "swizzle" anyway?)

PCI slots have 4 interrupts (#A, #B, #C, #D). In machines with
multiple slots, the interrupts lines are "swizzled", to avoid that all cards
using a single interrupt are mapped to the same host interrupt.

Typically, the mapping is:

    host_irq = bus_irqs[(slot + irq_pin) % 4];

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3] sh: avoid using IRQ0 on SH3/4

[Maciej W. Rozycki]

On Tue, 3 May 2022, Geert Uytterhoeven wrote:

> > Sounds like it's now outside of the IRQ range allocation, but I can't find where
> > that's requested when registering the controller? (What is a "swizzle" anyway?)
> 
> PCI slots have 4 interrupts (#A, #B, #C, #D). In machines with
> multiple slots, the interrupts lines are "swizzled", to avoid that all cards
> using a single interrupt are mapped to the same host interrupt.

 Especially as single-function devices are required to use INTA.

  Maciej

Re: [PATCH v3] sh: avoid using IRQ0 on SH3/4

[Sergey Shtylyov]

On 5/2/22 11:37 AM, Geert Uytterhoeven wrote:

[...]
>>>> Using IRQ0 by the platform devices is going to be disallowed soon (see [1])
>>>> and even now, when IRQ0 is about to be returned by platfrom_get_irq(), you
>>>> see a big warning.  The code supporting SH3/4 SoCs maps the IRQ #s starting
>>>> at 0 -- modify that code to start the IRQ #s from 16 instead.
>>>>
>>>> The patch should mostly affect the AP-SH4A-3A/AP-SH4AD-0A boards as they
>>>> indeed use IRQ0 for the SMSC911x compatible Ethernet chip...
> 
>> As I told him in IRC, the problem is still that sh4 never gives me a shell
>> prompt with this patch applied. I just reconfirmed it against current git:
>>
>> Freeing unused kernel image (initmem) memory: 124K
>> This architecture does not have kernel memory protection.
>> Run /init as init process
>> mountpoint: dev/pts: No such file or directory
>> 8139cp 0000:00:02.0 eth0: link up, 100Mbps, full-duplex, lpa 0x05E1
>>
>> It makes it partway through the init script, but it hangs with qemu-system-sh4
>> stuck in a CPU-eating loop before finishing. Without the patch, I get a shell
>> prompt.
> 
> I regularly test on qemu rts7751r2d, but couldn't produce your
> issue.  Until I tried "ifconfig eth0 up", which causes a lock-up.
> Interestingly, the 8139 irq was 112 with and without Sergey's patch,
> so there must be an irq remapping missing.
> 
> I also test regularly on landisk, where 8139 Ethernet works fine.
> Turns out landisk uses arch/sh/drivers/pci/fixups-landisk.c to fixup
> the irq...
> 
> arch/sh/include/mach-common/mach/r2d.h has:
> #define R2D_FPGA_IRQ_BASE       100
> Subtracting 16 here does not help.

   Why subtract when you contrariwise need to add? :-)

> With this (gmail-whitespace-damaged) patch:
> 
> --- a/arch/sh/drivers/pci/fixups-rts7751r2d.c
> +++ b/arch/sh/drivers/pci/fixups-rts7751r2d.c
> @@ -31,9 +31,9 @@ static char lboxre2_irq_tab[] = {
>  int pcibios_map_platform_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
>  {
>         if (mach_is_lboxre2())
> -               return lboxre2_irq_tab[slot];
> +               return lboxre2_irq_tab[slot] - 16;

   This table contains the values #define'd via evt2irq(), so
shouldn't need to subtract anything...

>         else
> -               return rts7751r2d_irq_tab[slot];
> +               return rts7751r2d_irq_tab[slot] - 16;

   How about + 16?

>  }
> 
>  int pci_fixup_pcic(struct pci_channel *chan)
> 
> it no longer crashes, but ifconfig still fails:
> 
> / # ifconfig eth0 up
> ifconfig: ioctl 0x8914 failed: Invalid argument

   I'm still not sure you used the correct IRQ #s...

> Note that there are more implementations of pcibios_map_platform_irq()
> that do not use evt2irq(), and thus are probably broken by this patch.

   That doesn't sound encouraging... :-/

> Gr{oetje,eeting}s,
> 
>                         Geert

MBR, Sergey

Re: [PATCH v3] sh: avoid using IRQ0 on SH3/4

[Geert Uytterhoeven]

Hi Sergey,

On Mon, May 2, 2022 at 10:56 PM Sergey Shtylyov <s.shtylyov@omp.ru> wrote:
> On 5/2/22 11:37 AM, Geert Uytterhoeven wrote:
> >>>> Using IRQ0 by the platform devices is going to be disallowed soon (see [1])
> >>>> and even now, when IRQ0 is about to be returned by platfrom_get_irq(), you
> >>>> see a big warning.  The code supporting SH3/4 SoCs maps the IRQ #s starting
> >>>> at 0 -- modify that code to start the IRQ #s from 16 instead.
> >>>>
> >>>> The patch should mostly affect the AP-SH4A-3A/AP-SH4AD-0A boards as they
> >>>> indeed use IRQ0 for the SMSC911x compatible Ethernet chip...
> >
> >> As I told him in IRC, the problem is still that sh4 never gives me a shell
> >> prompt with this patch applied. I just reconfirmed it against current git:
> >>
> >> Freeing unused kernel image (initmem) memory: 124K
> >> This architecture does not have kernel memory protection.
> >> Run /init as init process
> >> mountpoint: dev/pts: No such file or directory
> >> 8139cp 0000:00:02.0 eth0: link up, 100Mbps, full-duplex, lpa 0x05E1
> >>
> >> It makes it partway through the init script, but it hangs with qemu-system-sh4
> >> stuck in a CPU-eating loop before finishing. Without the patch, I get a shell
> >> prompt.
> >
> > I regularly test on qemu rts7751r2d, but couldn't produce your
> > issue.  Until I tried "ifconfig eth0 up", which causes a lock-up.
> > Interestingly, the 8139 irq was 112 with and without Sergey's patch,
> > so there must be an irq remapping missing.
> >
> > I also test regularly on landisk, where 8139 Ethernet works fine.
> > Turns out landisk uses arch/sh/drivers/pci/fixups-landisk.c to fixup
> > the irq...
> >
> > arch/sh/include/mach-common/mach/r2d.h has:
> > #define R2D_FPGA_IRQ_BASE       100
> > Subtracting 16 here does not help.
>
>    Why subtract when you contrariwise need to add? :-)

Thanks, adding 16 here fixed the issue:

/ # ifconfig eth0 up
8139cp 0000:00:02.0 eth0: link up, 100Mbps, full-duplex, lpa 0x05E1

> > With this (gmail-whitespace-damaged) patch:
> >
> > --- a/arch/sh/drivers/pci/fixups-rts7751r2d.c
> > +++ b/arch/sh/drivers/pci/fixups-rts7751r2d.c
> > @@ -31,9 +31,9 @@ static char lboxre2_irq_tab[] = {
> >  int pcibios_map_platform_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
> >  {
> >         if (mach_is_lboxre2())
> > -               return lboxre2_irq_tab[slot];
> > +               return lboxre2_irq_tab[slot] - 16;
>
>    This table contains the values #define'd via evt2irq(), so
> shouldn't need to subtract anything...
>
> >         else
> > -               return rts7751r2d_irq_tab[slot];
> > +               return rts7751r2d_irq_tab[slot] - 16;
>
>    How about + 16?

Doesn't work, but changing R2D_FPGA_IRQ_BASE does work, see
above.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3] sh: avoid using IRQ0 on SH3/4

[Sergey Shtylyov]

On 5/2/22 11:56 PM, Sergey Shtylyov wrote:

[...]
>>>>> Using IRQ0 by the platform devices is going to be disallowed soon (see [1])
>>>>> and even now, when IRQ0 is about to be returned by platfrom_get_irq(), you
>>>>> see a big warning.  The code supporting SH3/4 SoCs maps the IRQ #s starting
>>>>> at 0 -- modify that code to start the IRQ #s from 16 instead.
>>>>>
>>>>> The patch should mostly affect the AP-SH4A-3A/AP-SH4AD-0A boards as they
>>>>> indeed use IRQ0 for the SMSC911x compatible Ethernet chip...
>>
>>> As I told him in IRC, the problem is still that sh4 never gives me a shell
>>> prompt with this patch applied. I just reconfirmed it against current git:
>>>
>>> Freeing unused kernel image (initmem) memory: 124K
>>> This architecture does not have kernel memory protection.
>>> Run /init as init process
>>> mountpoint: dev/pts: No such file or directory
>>> 8139cp 0000:00:02.0 eth0: link up, 100Mbps, full-duplex, lpa 0x05E1
>>>
>>> It makes it partway through the init script, but it hangs with qemu-system-sh4
>>> stuck in a CPU-eating loop before finishing. Without the patch, I get a shell
>>> prompt.
>>
>> I regularly test on qemu rts7751r2d, but couldn't produce your
>> issue.  Until I tried "ifconfig eth0 up", which causes a lock-up.
>> Interestingly, the 8139 irq was 112 with and without Sergey's patch,
>> so there must be an irq remapping missing.
>>
>> I also test regularly on landisk, where 8139 Ethernet works fine.
>> Turns out landisk uses arch/sh/drivers/pci/fixups-landisk.c to fixup
>> the irq...
>>
>> arch/sh/include/mach-common/mach/r2d.h has:
>> #define R2D_FPGA_IRQ_BASE       100
>> Subtracting 16 here does not help.
> 
>    Why subtract when you contrariwise need to add? :-)
> 
>> With this (gmail-whitespace-damaged) patch:
>>
>> --- a/arch/sh/drivers/pci/fixups-rts7751r2d.c
>> +++ b/arch/sh/drivers/pci/fixups-rts7751r2d.c
>> @@ -31,9 +31,9 @@ static char lboxre2_irq_tab[] = {
>>  int pcibios_map_platform_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
>>  {
>>         if (mach_is_lboxre2())
>> -               return lboxre2_irq_tab[slot];
>> +               return lboxre2_irq_tab[slot] - 16;
> 
>    This table contains the values #define'd via evt2irq(), so
> shouldn't need to subtract anything...
> 
>>         else
>> -               return rts7751r2d_irq_tab[slot];
>> +               return rts7751r2d_irq_tab[slot] - 16;
> 
>    How about + 16?
> 
>>  }
>>
>>  int pci_fixup_pcic(struct pci_channel *chan)
>>
>> it no longer crashes, but ifconfig still fails:
>>
>> / # ifconfig eth0 up
>> ifconfig: ioctl 0x8914 failed: Invalid argument
> 
>    I'm still not sure you used the correct IRQ #s...
> 
>> Note that there are more implementations of pcibios_map_platform_irq()
>> that do not use evt2irq(), and thus are probably broken by this patch.
> 
>    That doesn't sound encouraging... :-/

   Actually, of those only Dreamcast didn't use evt2irq()...
   Now I'm wondering whether all #define *_IRQ_BASE should be visited as well...

>> Gr{oetje,eeting}s,
>>
>>                         Geert

MBR, Sergey