Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Guenter Roeck]

Hi,

On Thu, Jun 01, 2023 at 11:22:17PM +0300, Sergey Shtylyov wrote:
> IRQ0 is no longer returned by platform_get_irq() and its ilk -- they now
> return -EINVAL instead.  However, the kernel code supporting SH3/4-based
> SoCs still 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 are using IRQ0 for the SMSC911x compatible Ethernet chip.
> 

Unfortunately it also affects all sh4 emulations in qemu, and results in
boot stalls with those. There isn't a relevant log to attach because there
is no error message - booting just stalls until the emulation is aborted.

Reverting this patch fixes the problem.

Bisect log is attached for reference. Note that bisect requires applying
commit 7497840d462c ("sh: Provide unxlate_dev_mem_ptr() in asm/io.h"),
which is also the reason why the problem was not observed earlier since
it was hiding behind a build failure.

Guenter

---
# bad: [c17414a273b81fe4e34e11d69fc30cc8b1431614] Merge tag 'sh-for-v6.5-tag1' of git://git.kernel.org/pub/scm/linux/kernel/git/glaubitz/sh-linux
# good: [b5641a5d8b8b14643bfe3d017d64da90a5c55479] mm: don't do validate_mm() unnecessarily and without mmap locking
git bisect start 'HEAD' 'b5641a5d8b8b'
# good: [15ac468614e5e4fee82e1eb32568f427b0e51adc] Merge tag 'media/v6.5-1' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
git bisect good 15ac468614e5e4fee82e1eb32568f427b0e51adc
# good: [73a3fcdaa73200e38e38f7e8a32c9b901c5b95b5] Merge tag 'f2fs-for-6.5-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs
git bisect good 73a3fcdaa73200e38e38f7e8a32c9b901c5b95b5
# good: [6843306689aff3aea608e4d2630b2a5a0137f827] Merge tag 'net-6.5-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net
git bisect good 6843306689aff3aea608e4d2630b2a5a0137f827
# good: [afa92949124abc25ddab1789dd654214e2e1b040] dt-bindings: phy: cdns,salvo: add property cdns,usb2-disconnect-threshold-microvolt
git bisect good afa92949124abc25ddab1789dd654214e2e1b040
# good: [37bd215fc48ef2a399f836d62d2e4a166efb31be] phy: qualcomm: fix indentation in Makefile
git bisect good 37bd215fc48ef2a399f836d62d2e4a166efb31be
# bad: [7497840d462c8f54c4888c22ab3726a8cde4b9a2] sh: Provide unxlate_dev_mem_ptr() in asm/io.h
git bisect bad 7497840d462c8f54c4888c22ab3726a8cde4b9a2
# bad: [01658fe3d6c02992846a038c8111e70ace169295] sh: Refactor header include path addition
git bisect bad 01658fe3d6c02992846a038c8111e70ace169295
# bad: [a8ac2961148e8c720dc760f2e06627cd5c55a154] sh: Avoid using IRQ0 on SH3 and SH4
git bisect bad a8ac2961148e8c720dc760f2e06627cd5c55a154
# good: [bc9d1f0cecd2407cfb2364a7d4be2f52d1d46a9d] sh: j2: Use ioremap() to translate device tree address into kernel memory
git bisect good bc9d1f0cecd2407cfb2364a7d4be2f52d1d46a9d
# first bad commit: [a8ac2961148e8c720dc760f2e06627cd5c55a154] sh: Avoid using IRQ0 on SH3 and SH4

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[John Paul Adrian Glaubitz]

Hi Guenter!

On Thu, 2023-07-06 at 06:57 -0700, Guenter Roeck wrote:
> On Thu, Jun 01, 2023 at 11:22:17PM +0300, Sergey Shtylyov wrote:
> > IRQ0 is no longer returned by platform_get_irq() and its ilk -- they now
> > return -EINVAL instead.  However, the kernel code supporting SH3/4-based
> > SoCs still 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 are using IRQ0 for the SMSC911x compatible Ethernet chip.
> > 
> 
> Unfortunately it also affects all sh4 emulations in qemu, and results in
> boot stalls with those. There isn't a relevant log to attach because there
> is no error message - booting just stalls until the emulation is aborted.
> 
> Reverting this patch fixes the problem.
> 
> Bisect log is attached for reference. Note that bisect requires applying
> commit 7497840d462c ("sh: Provide unxlate_dev_mem_ptr() in asm/io.h"),
> which is also the reason why the problem was not observed earlier since
> it was hiding behind a build failure.

Interesting. My naive understanding was that IRQ0 is no longer usable in the
kernel as Sergey claimed. Was that not correct?

Adrian

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

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Guenter Roeck]

On 7/6/23 07:01, John Paul Adrian Glaubitz wrote:
> Hi Guenter!
> 
> On Thu, 2023-07-06 at 06:57 -0700, Guenter Roeck wrote:
>> On Thu, Jun 01, 2023 at 11:22:17PM +0300, Sergey Shtylyov wrote:
>>> IRQ0 is no longer returned by platform_get_irq() and its ilk -- they now
>>> return -EINVAL instead.  However, the kernel code supporting SH3/4-based
>>> SoCs still 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 are using IRQ0 for the SMSC911x compatible Ethernet chip.
>>>
>>
>> Unfortunately it also affects all sh4 emulations in qemu, and results in
>> boot stalls with those. There isn't a relevant log to attach because there
>> is no error message - booting just stalls until the emulation is aborted.
>>
>> Reverting this patch fixes the problem.
>>
>> Bisect log is attached for reference. Note that bisect requires applying
>> commit 7497840d462c ("sh: Provide unxlate_dev_mem_ptr() in asm/io.h"),
>> which is also the reason why the problem was not observed earlier since
>> it was hiding behind a build failure.
> 
> Interesting. My naive understanding was that IRQ0 is no longer usable in the
> kernel as Sergey claimed. Was that not correct?
> 

I have no idea. All I know is that all my sh4 qemu emulations hang during boot
after this patch is applied. I don't have CONFIG_SMSC911X enabled in my builds.
I also don't see any error messages from failed IRQ requests after reverting
this patch.

Guenter

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Sergey Shtylyov]

On 7/6/23 5:01 PM, John Paul Adrian Glaubitz wrote:
[...]

>> On Thu, Jun 01, 2023 at 11:22:17PM +0300, Sergey Shtylyov wrote:
>>> IRQ0 is no longer returned by platform_get_irq() and its ilk -- they now
>>> return -EINVAL instead.  However, the kernel code supporting SH3/4-based
>>> SoCs still 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 are using IRQ0 for the SMSC911x compatible Ethernet chip.
>>>
>>
>> Unfortunately it also affects all sh4 emulations in qemu, and results in
>> boot stalls with those. There isn't a relevant log to attach because there
>> is no error message - booting just stalls until the emulation is aborted.
>>
>> Reverting this patch fixes the problem.
>>
>> Bisect log is attached for reference. Note that bisect requires applying
>> commit 7497840d462c ("sh: Provide unxlate_dev_mem_ptr() in asm/io.h"),
>> which is also the reason why the problem was not observed earlier since
>> it was hiding behind a build failure.
> 
> Interesting. My naive understanding was that IRQ0 is no longer usable in the
> kernel as Sergey claimed. Was that not correct?

    According to Linus, using IRQ0 should only be allowed in the arch/ code
(usually the i8253 driver). As for the platform devices, this have been finally
enforced by this patch.

> Adrian

MBR, Sergey

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Sergey Shtylyov]

On 7/8/23 11:35 PM, Sergey Shtylyov wrote:
[...]

>>> On Thu, Jun 01, 2023 at 11:22:17PM +0300, Sergey Shtylyov wrote:
>>>> IRQ0 is no longer returned by platform_get_irq() and its ilk -- they now
>>>> return -EINVAL instead.  However, the kernel code supporting SH3/4-based
>>>> SoCs still 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 are using IRQ0 for the SMSC911x compatible Ethernet chip.
>>>>
>>>
>>> Unfortunately it also affects all sh4 emulations in qemu, and results in
>>> boot stalls with those. There isn't a relevant log to attach because there
>>> is no error message - booting just stalls until the emulation is aborted.
>>>
>>> Reverting this patch fixes the problem.
>>>
>>> Bisect log is attached for reference. Note that bisect requires applying
>>> commit 7497840d462c ("sh: Provide unxlate_dev_mem_ptr() in asm/io.h"),
>>> which is also the reason why the problem was not observed earlier since
>>> it was hiding behind a build failure.
>>
>> Interesting. My naive understanding was that IRQ0 is no longer usable in the
>> kernel as Sergey claimed. Was that not correct?
> 
>     According to Linus, using IRQ0 should only be allowed in the arch/ code
> (usually the i8253 driver). As for the platform devices, this have been finally
> enforced by this patch.

   Sorry, I meant *that* patch:

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

>> Adrian

MBR, Sergey

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Geert Uytterhoeven]

Hi Günter,

On Thu, Jul 6, 2023 at 4:03 PM Guenter Roeck <linux@roeck-us.net> wrote:
> On Thu, Jun 01, 2023 at 11:22:17PM +0300, Sergey Shtylyov wrote:
> > IRQ0 is no longer returned by platform_get_irq() and its ilk -- they now
> > return -EINVAL instead.  However, the kernel code supporting SH3/4-based
> > SoCs still 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 are using IRQ0 for the SMSC911x compatible Ethernet chip.
> >
>
> Unfortunately it also affects all sh4 emulations in qemu, and results in
> boot stalls with those. There isn't a relevant log to attach because there
> is no error message - booting just stalls until the emulation is aborted.

Which sh4 platforms in particular?

I booted a kernel with this patch on rts7751r2d (QEMU) and landisk
(physical) two days ago.

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] sh: Avoid using IRQ0 on SH3 and SH4

[Guenter Roeck]

On 7/6/23 08:39, Geert Uytterhoeven wrote:
> Hi Günter,
> 
> On Thu, Jul 6, 2023 at 4:03 PM Guenter Roeck <linux@roeck-us.net> wrote:
>> On Thu, Jun 01, 2023 at 11:22:17PM +0300, Sergey Shtylyov wrote:
>>> IRQ0 is no longer returned by platform_get_irq() and its ilk -- they now
>>> return -EINVAL instead.  However, the kernel code supporting SH3/4-based
>>> SoCs still 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 are using IRQ0 for the SMSC911x compatible Ethernet chip.
>>>
>>
>> Unfortunately it also affects all sh4 emulations in qemu, and results in
>> boot stalls with those. There isn't a relevant log to attach because there
>> is no error message - booting just stalls until the emulation is aborted.
> 
> Which sh4 platforms in particular?
> 
> I booted a kernel with this patch on rts7751r2d (QEMU) and landisk
> (physical) two days ago.
> 

It is r2d. Example qemu command line:

qemu-system-sh4 -M r2d -kernel arch/sh/boot/zImage -no-reboot \
	-initrd rootfs.cpio -device rtl8139,netdev=net0 -netdev user,id=net0 \
	-append "rdinit=/sbin/init console=ttySC1,115200 earlycon=scif,mmio16,0xffe80000 noiotrap" \
	-serial null -serial stdio -nographic -monitor null

Example set of logs:

https://kerneltests.org/builders/qemu-sh-master/builds/5/steps/qemubuildcommand/logs/stdio

Guenter

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[John Paul Adrian Glaubitz]

Hi Geert!

On Thu, 2023-07-06 at 17:39 +0200, Geert Uytterhoeven wrote:
> Which sh4 platforms in particular?
> 
> I booted a kernel with this patch on rts7751r2d (QEMU) and landisk
> (physical) two days ago.

I gave it a try with the command line Guenter suggested and indeed the kernel locks
up right here with the patch applied and boots fine without it:

Creating 4 MTD partitions on "physmap-flash":
0x000000000000-0x000000040000 : "U-Boot"
0x000000040000-0x000000080000 : "Environment"
0x000000080000-0x000000240000 : "Kernel"
0x000000240000-0x000001000000 : "Flash_FS"
8139too: 8139too Fast Ethernet driver 0.9.28
8139too 0000:00:01.0: This (id 10ec:8139 rev 20) is an enhanced 8139C+ chip, use 8139cp
sm501-usb sm501-usb: SM501 OHCI
sm501-usb sm501-usb: new USB bus registered, assigned bus number 1
sm501-usb sm501-usb: irq 116, io mem 0x13e40000
usb usb1: New USB device found, idVendor=1d6b, idProduct=0001, bcdDevice= 6.04
usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
usb usb1: Product: SM501 OHCI
usb usb1: Manufacturer: Linux 6.4.0-12069-gc17414a273b8 ohci_hcd
usb usb1: SerialNumber: sm501-usb
hub 1-0:1.0: USB hub found
hub 1-0:1.0: 2 ports detected
usbcore: registered new interface driver usb-storage
rtc-r9701 spi0.0: cannot read RTC register
usbcore: registered new interface driver usbhid
usbhid: USB HID core driver
NET: Registered PF_PACKET protocol family
heartbeat: version 0.1.2 loaded
ata1: found unknown device (class 0)
(stops here)

Using rts7751r2dplus_defconfig and the following command line:

qemu-system-sh4 -M r2d -kernel vmlinuz-6.5-rc1 -hda debian_sid_sh4_standard.qcow2 -no-reboot -device rtl8139,netdev=net0 -netdev user,id=net0 -append "root=/dev/sda1 console=ttySC1,115200
earlycon=scif,mmio16,0xffe80000 noiotrap" -serial null -serial stdio -nographic -monitor null

And using this old qcow2 image:

> https://people.debian.org/~aurel32/qemu/sh4/debian_sid_sh4_standard.qcow2

Maybe it's a configuration issue if it works for you?

Adrian

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

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Guenter Roeck]

On 7/6/23 09:38, John Paul Adrian Glaubitz wrote:
> Hi Geert!
> 
> On Thu, 2023-07-06 at 17:39 +0200, Geert Uytterhoeven wrote:
>> Which sh4 platforms in particular?
>>
>> I booted a kernel with this patch on rts7751r2d (QEMU) and landisk
>> (physical) two days ago.
> 
> I gave it a try with the command line Guenter suggested and indeed the kernel locks
> up right here with the patch applied and boots fine without it:
> 
> Creating 4 MTD partitions on "physmap-flash":
> 0x000000000000-0x000000040000 : "U-Boot"
> 0x000000040000-0x000000080000 : "Environment"
> 0x000000080000-0x000000240000 : "Kernel"
> 0x000000240000-0x000001000000 : "Flash_FS"
> 8139too: 8139too Fast Ethernet driver 0.9.28
> 8139too 0000:00:01.0: This (id 10ec:8139 rev 20) is an enhanced 8139C+ chip, use 8139cp
> sm501-usb sm501-usb: SM501 OHCI
> sm501-usb sm501-usb: new USB bus registered, assigned bus number 1
> sm501-usb sm501-usb: irq 116, io mem 0x13e40000
> usb usb1: New USB device found, idVendor=1d6b, idProduct=0001, bcdDevice= 6.04
> usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
> usb usb1: Product: SM501 OHCI
> usb usb1: Manufacturer: Linux 6.4.0-12069-gc17414a273b8 ohci_hcd
> usb usb1: SerialNumber: sm501-usb
> hub 1-0:1.0: USB hub found
> hub 1-0:1.0: 2 ports detected
> usbcore: registered new interface driver usb-storage
> rtc-r9701 spi0.0: cannot read RTC register
> usbcore: registered new interface driver usbhid
> usbhid: USB HID core driver
> NET: Registered PF_PACKET protocol family
> heartbeat: version 0.1.2 loaded
> ata1: found unknown device (class 0)
> (stops here)
> 
> Using rts7751r2dplus_defconfig and the following command line:
> 
> qemu-system-sh4 -M r2d -kernel vmlinuz-6.5-rc1 -hda debian_sid_sh4_standard.qcow2 -no-reboot -device rtl8139,netdev=net0 -netdev user,id=net0 -append "root=/dev/sda1 console=ttySC1,115200
> earlycon=scif,mmio16,0xffe80000 noiotrap" -serial null -serial stdio -nographic -monitor null
> 
> And using this old qcow2 image:
> 
>> https://people.debian.org/~aurel32/qemu/sh4/debian_sid_sh4_standard.qcow2
> 
> Maybe it's a configuration issue if it works for you?
> 

I tried rts7751r2dplus_defconfig with no modifications and the following minimized
qemu command line.

qemu-system-sh4 -M r2d -kernel arch/sh/boot/zImage -serial null -serial stdio -nographic -monitor null

This hangs after "heartbeat: version 0.1.2 loaded", so it doesn't
even get to the point where it would try to load a root file system.
After reverting this patch, I get
---[ end Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) ]---
as expected.

I tried with qemu version 6.2, 7.1, 7.2, and 8.0.

Guenter

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[John Paul Adrian Glaubitz]

Hi Guenter!

On Thu, 2023-07-06 at 10:04 -0700, Guenter Roeck wrote:
> I tried rts7751r2dplus_defconfig with no modifications and the following minimized
> qemu command line.
> 
> qemu-system-sh4 -M r2d -kernel arch/sh/boot/zImage -serial null -serial stdio -nographic -monitor null
> 
> This hangs after "heartbeat: version 0.1.2 loaded", so it doesn't
> even get to the point where it would try to load a root file system.
> After reverting this patch, I get
> ---[ end Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) ]---
> as expected.
> 
> I tried with qemu version 6.2, 7.1, 7.2, and 8.0.

I'm curios to hear what Geert and Sergey have to say. I hope we don't have to revert this :-(.

Adrian

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

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Sergey Shtylyov]

On 7/6/23 8:41 PM, John Paul Adrian Glaubitz wrote:

>> I tried rts7751r2dplus_defconfig with no modifications and the following minimized
>> qemu command line.
>>
>> qemu-system-sh4 -M r2d -kernel arch/sh/boot/zImage -serial null -serial stdio -nographic -monitor null
>>
>> This hangs after "heartbeat: version 0.1.2 loaded", so it doesn't
>> even get to the point where it would try to load a root file system.
>> After reverting this patch, I get
>> ---[ end Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) ]---
>> as expected.
>>
>> I tried with qemu version 6.2, 7.1, 7.2, and 8.0.
> 
> I'm curios to hear what Geert and Sergey have to say. 

   Sorry for a late reaction, I was busy reviewing some PATA patches and a static analyzer
work here, at OMP...

> I hope we don't have to revert this :-(.

   I think we should be able to find the root cause soon.
   I prolly should learn Qemu some day... :-)

> Adrian

MBR, Sergey

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Geert Uytterhoeven]

Hi Adrian,

On Thu, Jul 6, 2023 at 6:39 PM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On Thu, 2023-07-06 at 17:39 +0200, Geert Uytterhoeven wrote:
> > Which sh4 platforms in particular?
> >
> > I booted a kernel with this patch on rts7751r2d (QEMU) and landisk
> > (physical) two days ago.
>
> I gave it a try with the command line Guenter suggested and indeed the kernel locks
> up right here with the patch applied and boots fine without it:
>
> Creating 4 MTD partitions on "physmap-flash":
> 0x000000000000-0x000000040000 : "U-Boot"
> 0x000000040000-0x000000080000 : "Environment"
> 0x000000080000-0x000000240000 : "Kernel"
> 0x000000240000-0x000001000000 : "Flash_FS"
> 8139too: 8139too Fast Ethernet driver 0.9.28
> 8139too 0000:00:01.0: This (id 10ec:8139 rev 20) is an enhanced 8139C+ chip, use 8139cp
> sm501-usb sm501-usb: SM501 OHCI
> sm501-usb sm501-usb: new USB bus registered, assigned bus number 1
> sm501-usb sm501-usb: irq 116, io mem 0x13e40000
> usb usb1: New USB device found, idVendor=1d6b, idProduct=0001, bcdDevice= 6.04
> usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
> usb usb1: Product: SM501 OHCI
> usb usb1: Manufacturer: Linux 6.4.0-12069-gc17414a273b8 ohci_hcd
> usb usb1: SerialNumber: sm501-usb
> hub 1-0:1.0: USB hub found
> hub 1-0:1.0: 2 ports detected
> usbcore: registered new interface driver usb-storage
> rtc-r9701 spi0.0: cannot read RTC register
> usbcore: registered new interface driver usbhid
> usbhid: USB HID core driver
> NET: Registered PF_PACKET protocol family
> heartbeat: version 0.1.2 loaded
> ata1: found unknown device (class 0)
> (stops here)
>
> Using rts7751r2dplus_defconfig and the following command line:
>
> qemu-system-sh4 -M r2d -kernel vmlinuz-6.5-rc1 -hda debian_sid_sh4_standard.qcow2 -no-reboot -device rtl8139,netdev=net0 -netdev user,id=net0 -append "root=/dev/sda1 console=ttySC1,115200
> earlycon=scif,mmio16,0xffe80000 noiotrap" -serial null -serial stdio -nographic -monitor null
>
> And using this old qcow2 image:
>
> > https://people.debian.org/~aurel32/qemu/sh4/debian_sid_sh4_standard.qcow2
>
> Maybe it's a configuration issue if it works for you?

I am not using rts7751r2dplus_defconfig, but my own[*] config,
which boots fine into initrd userspace.

I can reproduce the issue with rts7751r2dplus_defconfig, but I may
not be able to look into it today...

[*] Actually sh4.miniconf from Rob Landley.

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] sh: Avoid using IRQ0 on SH3 and SH4

[Geert Uytterhoeven]

On Fri, Jul 7, 2023 at 9:14 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Thu, Jul 6, 2023 at 6:39 PM John Paul Adrian Glaubitz
> <glaubitz@physik.fu-berlin.de> wrote:
> > On Thu, 2023-07-06 at 17:39 +0200, Geert Uytterhoeven wrote:
> > > Which sh4 platforms in particular?
> > >
> > > I booted a kernel with this patch on rts7751r2d (QEMU) and landisk
> > > (physical) two days ago.
> >
> > I gave it a try with the command line Guenter suggested and indeed the kernel locks
> > up right here with the patch applied and boots fine without it:
> >
> > Creating 4 MTD partitions on "physmap-flash":
> > 0x000000000000-0x000000040000 : "U-Boot"
> > 0x000000040000-0x000000080000 : "Environment"
> > 0x000000080000-0x000000240000 : "Kernel"
> > 0x000000240000-0x000001000000 : "Flash_FS"
> > 8139too: 8139too Fast Ethernet driver 0.9.28
> > 8139too 0000:00:01.0: This (id 10ec:8139 rev 20) is an enhanced 8139C+ chip, use 8139cp
> > sm501-usb sm501-usb: SM501 OHCI
> > sm501-usb sm501-usb: new USB bus registered, assigned bus number 1
> > sm501-usb sm501-usb: irq 116, io mem 0x13e40000
> > usb usb1: New USB device found, idVendor=1d6b, idProduct=0001, bcdDevice= 6.04
> > usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
> > usb usb1: Product: SM501 OHCI
> > usb usb1: Manufacturer: Linux 6.4.0-12069-gc17414a273b8 ohci_hcd
> > usb usb1: SerialNumber: sm501-usb
> > hub 1-0:1.0: USB hub found
> > hub 1-0:1.0: 2 ports detected
> > usbcore: registered new interface driver usb-storage
> > rtc-r9701 spi0.0: cannot read RTC register
> > usbcore: registered new interface driver usbhid
> > usbhid: USB HID core driver
> > NET: Registered PF_PACKET protocol family
> > heartbeat: version 0.1.2 loaded
> > ata1: found unknown device (class 0)
> > (stops here)
> >
> > Using rts7751r2dplus_defconfig and the following command line:
> >
> > qemu-system-sh4 -M r2d -kernel vmlinuz-6.5-rc1 -hda debian_sid_sh4_standard.qcow2 -no-reboot -device rtl8139,netdev=net0 -netdev user,id=net0 -append "root=/dev/sda1 console=ttySC1,115200
> > earlycon=scif,mmio16,0xffe80000 noiotrap" -serial null -serial stdio -nographic -monitor null
> >
> > And using this old qcow2 image:
> >
> > > https://people.debian.org/~aurel32/qemu/sh4/debian_sid_sh4_standard.qcow2
> >
> > Maybe it's a configuration issue if it works for you?
>
> I am not using rts7751r2dplus_defconfig, but my own[*] config,
> which boots fine into initrd userspace.
>
> I can reproduce the issue with rts7751r2dplus_defconfig, but I may
> not be able to look into it today...

Disabling CONFIG_USB_OHCI_HCD fixes the hang.

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] sh: Avoid using IRQ0 on SH3 and SH4

[John Paul Adrian Glaubitz]

Hi Geert!

On Fri, 2023-07-07 at 10:48 +0200, Geert Uytterhoeven wrote:
> > I am not using rts7751r2dplus_defconfig, but my own[*] config,
> > which boots fine into initrd userspace.
> > 
> > I can reproduce the issue with rts7751r2dplus_defconfig, but I may
> > not be able to look into it today...
> 
> Disabling CONFIG_USB_OHCI_HCD fixes the hang.

Thanks, I was expecting a driver issue. I guess we need to look into the
OHCI driver or the SH-specific part and check whether the IRQ numbers need
to be adjusted there as well.

Adrian

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

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[John Paul Adrian Glaubitz]

Hi Geert!

On Fri, 2023-07-07 at 10:48 +0200, Geert Uytterhoeven wrote:
> > I can reproduce the issue with rts7751r2dplus_defconfig, but I may
> > not be able to look into it today...
> 
> Disabling CONFIG_USB_OHCI_HCD fixes the hang.

I picked rts7751r2dplus_defconfig, disabled CONFIG_USB_OHCI_HCD but it still
hangs for me. Are you sure it's CONFIG_USB_OHCI_HCD and not something else?

Adrian

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

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Guenter Roeck]

On 7/7/23 02:53, John Paul Adrian Glaubitz wrote:
> Hi Geert!
> 
> On Fri, 2023-07-07 at 10:48 +0200, Geert Uytterhoeven wrote:
>>> I can reproduce the issue with rts7751r2dplus_defconfig, but I may
>>> not be able to look into it today...
>>
>> Disabling CONFIG_USB_OHCI_HCD fixes the hang.
> 
> I picked rts7751r2dplus_defconfig, disabled CONFIG_USB_OHCI_HCD but it still
> hangs for me. Are you sure it's CONFIG_USB_OHCI_HCD and not something else?
> 

It seems to be related. With this patch reverted, ohci_irq() gets a single
interrupt, and boot continues. With this patch in place, ohci_irq() does
not get any interrupts, and boot hangs with qemu at 100% CPU. I confirmed
this by disabling CONFIG_MFD_SM501. After that, the hang is no longer seen.
Of course, that also means that OHCI and other emulated sm501 functionality
no longer works.

My suspicion is that something goes wrong with interrupt routing to
SM501 and with it to ohci_irq(), but that is just a wild guess.

Guenter

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Guenter Roeck]

On 7/7/23 08:19, John Paul Adrian Glaubitz wrote:
> Hi Guenter!
> 
>> On Jul 7, 2023, at 4:35 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 7/7/23 02:53, John Paul Adrian Glaubitz wrote:
>>> Hi Geert!
>>> On Fri, 2023-07-07 at 10:48 +0200, Geert Uytterhoeven wrote:
>>>>> I can reproduce the issue with rts7751r2dplus_defconfig, but I may
>>>>> not be able to look into it today...
>>>>
>>>> Disabling CONFIG_USB_OHCI_HCD fixes the hang.
>>> I picked rts7751r2dplus_defconfig, disabled CONFIG_USB_OHCI_HCD but it still
>>> hangs for me. Are you sure it's CONFIG_USB_OHCI_HCD and not something else?
>>
>> It seems to be related. With this patch reverted, ohci_irq() gets a single
>> interrupt, and boot continues. With this patch in place, ohci_irq() does
>> not get any interrupts, and boot hangs with qemu at 100% CPU. I confirmed
>> this by disabling CONFIG_MFD_SM501. After that, the hang is no longer seen.
>> Of course, that also means that OHCI and other emulated sm501 functionality
>> no longer works.
>>
>> My suspicion is that something goes wrong with interrupt routing to
>> SM501 and with it to ohci_irq(), but that is just a wild guess.
> 
> Looking at drivers/mfd/sm501.c, sm501_plat_probe() is explicitly requesting IRQ 0 which is no longer supported on SH:
> 
>  > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mfd/sm501.c <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mfd/sm501.c>
> 
> So, we need to shift this by 16, I guess.
> 

It isn't interrupt 0, it is interrupt _index_ 0. See platform_get_irq_optional()
which calls platform_get_resource(dev, IORESOURCE_IRQ, num). There are lots
of calls to platform_get_irq() with index 0 in the kernel.

Guenter

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Sergey Shtylyov]

On 7/6/23 7:38 PM, John Paul Adrian Glaubitz wrote:
[...]

>> Which sh4 platforms in particular?
>>
>> I booted a kernel with this patch on rts7751r2d (QEMU) and landisk
>> (physical) two days ago.
> 
> I gave it a try with the command line Guenter suggested and indeed the kernel locks
> up right here with the patch applied and boots fine without it:
> 
> Creating 4 MTD partitions on "physmap-flash":
> 0x000000000000-0x000000040000 : "U-Boot"
> 0x000000040000-0x000000080000 : "Environment"
> 0x000000080000-0x000000240000 : "Kernel"
> 0x000000240000-0x000001000000 : "Flash_FS"
> 8139too: 8139too Fast Ethernet driver 0.9.28
> 8139too 0000:00:01.0: This (id 10ec:8139 rev 20) is an enhanced 8139C+ chip, use 8139cp
> sm501-usb sm501-usb: SM501 OHCI
> sm501-usb sm501-usb: new USB bus registered, assigned bus number 1
> sm501-usb sm501-usb: irq 116, io mem 0x13e40000

   I guess it was irq 100 before my patch. What if you undo the following
part of my patch?

Index: linux/arch/sh/include/mach-common/mach/r2d.h
===================================================================
--- linux.orig/arch/sh/include/mach-common/mach/r2d.h
+++ linux/arch/sh/include/mach-common/mach/r2d.h
@@ -47,7 +47,7 @@
 
 #define IRLCNTR1	(PA_BCR + 0)	/* Interrupt Control Register1 */
 
-#define R2D_FPGA_IRQ_BASE	100
+#define R2D_FPGA_IRQ_BASE	(100 + 16)
 
 #define IRQ_VOYAGER		(R2D_FPGA_IRQ_BASE + 0)
 #define IRQ_EXT			(R2D_FPGA_IRQ_BASE + 1)

> usb usb1: New USB device found, idVendor=1d6b, idProduct=0001, bcdDevice= 6.04
> usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
> usb usb1: Product: SM501 OHCI
> usb usb1: Manufacturer: Linux 6.4.0-12069-gc17414a273b8 ohci_hcd
> usb usb1: SerialNumber: sm501-usb
> hub 1-0:1.0: USB hub found
> hub 1-0:1.0: 2 ports detected
> usbcore: registered new interface driver usb-storage
> rtc-r9701 spi0.0: cannot read RTC register
> usbcore: registered new interface driver usbhid
> usbhid: USB HID core driver
> NET: Registered PF_PACKET protocol family
> heartbeat: version 0.1.2 loaded
> ata1: found unknown device (class 0)
[...]

> Adrian

MBR, Sergey

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Guenter Roeck]

On 7/8/23 13:52, Sergey Shtylyov wrote:
> On 7/6/23 7:38 PM, John Paul Adrian Glaubitz wrote:
> [...]
> 
>>> Which sh4 platforms in particular?
>>>
>>> I booted a kernel with this patch on rts7751r2d (QEMU) and landisk
>>> (physical) two days ago.
>>
>> I gave it a try with the command line Guenter suggested and indeed the kernel locks
>> up right here with the patch applied and boots fine without it:
>>
>> Creating 4 MTD partitions on "physmap-flash":
>> 0x000000000000-0x000000040000 : "U-Boot"
>> 0x000000040000-0x000000080000 : "Environment"
>> 0x000000080000-0x000000240000 : "Kernel"
>> 0x000000240000-0x000001000000 : "Flash_FS"
>> 8139too: 8139too Fast Ethernet driver 0.9.28
>> 8139too 0000:00:01.0: This (id 10ec:8139 rev 20) is an enhanced 8139C+ chip, use 8139cp
>> sm501-usb sm501-usb: SM501 OHCI
>> sm501-usb sm501-usb: new USB bus registered, assigned bus number 1
>> sm501-usb sm501-usb: irq 116, io mem 0x13e40000
> 
>     I guess it was irq 100 before my patch. What if you undo the following
> part of my patch?
> 

No, that doesn't help.

Guenter

> Index: linux/arch/sh/include/mach-common/mach/r2d.h
> ===================================================================
> --- linux.orig/arch/sh/include/mach-common/mach/r2d.h
> +++ linux/arch/sh/include/mach-common/mach/r2d.h
> @@ -47,7 +47,7 @@
>   
>   #define IRLCNTR1	(PA_BCR + 0)	/* Interrupt Control Register1 */
>   
> -#define R2D_FPGA_IRQ_BASE	100
> +#define R2D_FPGA_IRQ_BASE	(100 + 16)
>   
>   #define IRQ_VOYAGER		(R2D_FPGA_IRQ_BASE + 0)
>   #define IRQ_EXT			(R2D_FPGA_IRQ_BASE + 1)
> 
>> usb usb1: New USB device found, idVendor=1d6b, idProduct=0001, bcdDevice= 6.04
>> usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
>> usb usb1: Product: SM501 OHCI
>> usb usb1: Manufacturer: Linux 6.4.0-12069-gc17414a273b8 ohci_hcd
>> usb usb1: SerialNumber: sm501-usb
>> hub 1-0:1.0: USB hub found
>> hub 1-0:1.0: 2 ports detected
>> usbcore: registered new interface driver usb-storage
>> rtc-r9701 spi0.0: cannot read RTC register
>> usbcore: registered new interface driver usbhid
>> usbhid: USB HID core driver
>> NET: Registered PF_PACKET protocol family
>> heartbeat: version 0.1.2 loaded
>> ata1: found unknown device (class 0)
> [...]
> 
>> Adrian
> 
> MBR, Sergey

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[John Paul Adrian Glaubitz]

Hello Guenter!

On Sat, 2023-07-08 at 15:06 -0700, Guenter Roeck wrote:
> On 7/8/23 13:52, Sergey Shtylyov wrote:
> > On 7/6/23 7:38 PM, John Paul Adrian Glaubitz wrote:
> > [...]
> > 
> > > > Which sh4 platforms in particular?
> > > > 
> > > > I booted a kernel with this patch on rts7751r2d (QEMU) and landisk
> > > > (physical) two days ago.
> > > 
> > > I gave it a try with the command line Guenter suggested and indeed the kernel locks
> > > up right here with the patch applied and boots fine without it:
> > > 
> > > Creating 4 MTD partitions on "physmap-flash":
> > > 0x000000000000-0x000000040000 : "U-Boot"
> > > 0x000000040000-0x000000080000 : "Environment"
> > > 0x000000080000-0x000000240000 : "Kernel"
> > > 0x000000240000-0x000001000000 : "Flash_FS"
> > > 8139too: 8139too Fast Ethernet driver 0.9.28
> > > 8139too 0000:00:01.0: This (id 10ec:8139 rev 20) is an enhanced 8139C+ chip, use 8139cp
> > > sm501-usb sm501-usb: SM501 OHCI
> > > sm501-usb sm501-usb: new USB bus registered, assigned bus number 1
> > > sm501-usb sm501-usb: irq 116, io mem 0x13e40000
> > 
> >     I guess it was irq 100 before my patch. What if you undo the following
> > part of my patch?
> > 
> 
> No, that doesn't help.

Since the SM501 works fine with the patch on my SH7785LCR board, I assume it's
related to the IRQ code for the r2d2 board.

Looking at arch/sh/boards/mach-r2d/irq.c, there is some IRQ translation going
on and maybe that's the part where we need to correct the offset by 16?

Adrian

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

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Guenter Roeck]

On 7/8/23 16:13, John Paul Adrian Glaubitz wrote:
> Hello Guenter!
> 
> On Sat, 2023-07-08 at 15:06 -0700, Guenter Roeck wrote:
>> On 7/8/23 13:52, Sergey Shtylyov wrote:
>>> On 7/6/23 7:38 PM, John Paul Adrian Glaubitz wrote:
>>> [...]
>>>
>>>>> Which sh4 platforms in particular?
>>>>>
>>>>> I booted a kernel with this patch on rts7751r2d (QEMU) and landisk
>>>>> (physical) two days ago.
>>>>
>>>> I gave it a try with the command line Guenter suggested and indeed the kernel locks
>>>> up right here with the patch applied and boots fine without it:
>>>>
>>>> Creating 4 MTD partitions on "physmap-flash":
>>>> 0x000000000000-0x000000040000 : "U-Boot"
>>>> 0x000000040000-0x000000080000 : "Environment"
>>>> 0x000000080000-0x000000240000 : "Kernel"
>>>> 0x000000240000-0x000001000000 : "Flash_FS"
>>>> 8139too: 8139too Fast Ethernet driver 0.9.28
>>>> 8139too 0000:00:01.0: This (id 10ec:8139 rev 20) is an enhanced 8139C+ chip, use 8139cp
>>>> sm501-usb sm501-usb: SM501 OHCI
>>>> sm501-usb sm501-usb: new USB bus registered, assigned bus number 1
>>>> sm501-usb sm501-usb: irq 116, io mem 0x13e40000
>>>
>>>      I guess it was irq 100 before my patch. What if you undo the following
>>> part of my patch?
>>>
>>
>> No, that doesn't help.
> 
> Since the SM501 works fine with the patch on my SH7785LCR board, I assume it's
> related to the IRQ code for the r2d2 board.
> 
> Looking at arch/sh/boards/mach-r2d/irq.c, there is some IRQ translation going
> on and maybe that's the part where we need to correct the offset by 16?
> 

I don't know the sh code at all. I only know that the sh4 qemu emulation
(both little and big endian) is broken since your patch has been applied.
Everything else would be just wild guesses from my side.

Guenter

RE: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[David Laight]

From: John Paul Adrian Glaubitz
> Sent: 09 July 2023 00:13
....
> Looking at arch/sh/boards/mach-r2d/irq.c, there is some IRQ translation going
> on and maybe that's the part where we need to correct the offset by 16?

Would it be less problematic to use (say) 16 for IRQ_0
leaving IRQ_1+ as 1+ ?

At least that would only cause issues for code that needed
to use IRQ_0.

(It has to be said that making IRQ 0 invalid seemed wrong
to me. x86 (IBM PC) gets away with it because IRQ 0 is
always assigned to platform specific hardware.)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[John Paul Adrian Glaubitz]

Hi David!

On Wed, 2023-07-12 at 08:12 +0000, David Laight wrote:
> From: John Paul Adrian Glaubitz
> > Sent: 09 July 2023 00:13
> ....
> > Looking at arch/sh/boards/mach-r2d/irq.c, there is some IRQ translation going
> > on and maybe that's the part where we need to correct the offset by 16?
> 
> Would it be less problematic to use (say) 16 for IRQ_0
> leaving IRQ_1+ as 1+ ?

That would make things more complicated as IRQ0 would have to be
handled individually.

> At least that would only cause issues for code that needed
> to use IRQ_0.

What issues are you seeing or expecting?

> (It has to be said that making IRQ 0 invalid seemed wrong
> to me. x86 (IBM PC) gets away with it because IRQ 0 is
> always assigned to platform specific hardware.)

It's invalid for driver code, not for architecture code.

Adrian

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

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Sergei Shtylyov]

On 7/12/23 11:12 AM, David Laight wrote:

>> Sent: 09 July 2023 00:13
> ....
>> Looking at arch/sh/boards/mach-r2d/irq.c, there is some IRQ translation going
>> on and maybe that's the part where we need to correct the offset by 16?
> 
> Would it be less problematic to use (say) 16 for IRQ_0
> leaving IRQ_1+ as 1+ ?

   I don't think so.

> At least that would only cause issues for code that needed
> to use IRQ_0.
> 
> (It has to be said that making IRQ 0 invalid seemed wrong
> to me. x86 (IBM PC) gets away with it because IRQ 0 is
> always assigned to platform specific hardware.)

   Not only x86, IIRC.
   Have you seen the commit below?

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

   IOW, try arguing with Linus. :-)

> 	David

MBR, Sergey

RE: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[David Laight]

...
>    IOW, try arguing with Linus. :-)

I always lose :-)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Guenter Roeck]

On 7/12/23 01:20, Sergei Shtylyov wrote:
> On 7/12/23 11:12 AM, David Laight wrote:
> 
>>> Sent: 09 July 2023 00:13
>> ....
>>> Looking at arch/sh/boards/mach-r2d/irq.c, there is some IRQ translation going
>>> on and maybe that's the part where we need to correct the offset by 16?
>>
>> Would it be less problematic to use (say) 16 for IRQ_0
>> leaving IRQ_1+ as 1+ ?
> 
>     I don't think so.
> 
>> At least that would only cause issues for code that needed
>> to use IRQ_0.
>>
>> (It has to be said that making IRQ 0 invalid seemed wrong
>> to me. x86 (IBM PC) gets away with it because IRQ 0 is
>> always assigned to platform specific hardware.)
> 
>     Not only x86, IIRC.
>     Have you seen the commit below?
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ce753ad1549cbe9ccaea4c06a1f5fa47432c8289
> 

Quoting:

using IRQ0 is considered invalid (according to Linus) outside the arch/ code
                                                       ^^^^^^^^^^^^^^^^^^^^^^

The changes here were made _in_ the arch code. While there may be valid
arguments for doing that, quoting the above commit as reason isn't really
sufficient.

Guenter

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[John Paul Adrian Glaubitz]

Hi!

On Wed, 2023-07-12 at 08:38 -0700, Guenter Roeck wrote:
> > 
> >     Not only x86, IIRC.
> >     Have you seen the commit below?
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ce753ad1549cbe9ccaea4c06a1f5fa47432c8289
> > 
> 
> Quoting:
> 
> using IRQ0 is considered invalid (according to Linus) outside the arch/ code
>                                                        ^^^^^^^^^^^^^^^^^^^^^^
> 
> The changes here were made _in_ the arch code. While there may be valid
> arguments for doing that, quoting the above commit as reason isn't really
> sufficient.

Well, the changes we made inside arch/sh were necessary to achieve the desired
effects in the code in drivers such as the SM501 controller or the SH companion
chips.

Adrian

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

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Sergey Shtylyov]

On 7/12/23 6:38 PM, Guenter Roeck wrote:
[...]

>>>> Sent: 09 July 2023 00:13
>>> ....
>>>> Looking at arch/sh/boards/mach-r2d/irq.c, there is some IRQ translation going
>>>> on and maybe that's the part where we need to correct the offset by 16?
>>>
>>> Would it be less problematic to use (say) 16 for IRQ_0
>>> leaving IRQ_1+ as 1+ ?
>>
>>     I don't think so.
>>
>>> At least that would only cause issues for code that needed
>>> to use IRQ_0.
>>>
>>> (It has to be said that making IRQ 0 invalid seemed wrong
>>> to me. x86 (IBM PC) gets away with it because IRQ 0 is
>>> always assigned to platform specific hardware.)
>>
>>     Not only x86, IIRC.
>>     Have you seen the commit below?
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ce753ad1549cbe9ccaea4c06a1f5fa47432c8289
>>
> 
> Quoting:
> 
> using IRQ0 is considered invalid (according to Linus) outside the arch/ code
>                                                       ^^^^^^^^^^^^^^^^^^^^^^> The changes here were made _in_ the arch code. While there may be valid
> arguments for doing that, quoting the above commit as reason isn't really
> sufficient.

   It seems you still don't understand... The i8253 drivers using IRQ0 live in
the arch/{mips|x86}/kernel/ code, so they're allowed to claim IRQ0 via calling
request_irq() (MIPS still does it.  They are NOT the platform drivers we're
dealing with here...

> Guenter

MBR, Sergey

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Sergey Shtylyov]

On 7/12/23 7:22 PM, Sergey Shtylyov wrote:

> [...]
> 
>>>>> Sent: 09 July 2023 00:13
>>>> ....
>>>>> Looking at arch/sh/boards/mach-r2d/irq.c, there is some IRQ translation going
>>>>> on and maybe that's the part where we need to correct the offset by 16?
>>>>
>>>> Would it be less problematic to use (say) 16 for IRQ_0
>>>> leaving IRQ_1+ as 1+ ?
>>>
>>>     I don't think so.
>>>
>>>> At least that would only cause issues for code that needed
>>>> to use IRQ_0.
>>>>
>>>> (It has to be said that making IRQ 0 invalid seemed wrong
>>>> to me. x86 (IBM PC) gets away with it because IRQ 0 is
>>>> always assigned to platform specific hardware.)
>>>
>>>     Not only x86, IIRC.
>>>     Have you seen the commit below?
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ce753ad1549cbe9ccaea4c06a1f5fa47432c8289
>>>
>>
>> Quoting:
>>
>> using IRQ0 is considered invalid (according to Linus) outside the arch/ code
>>                                                       ^^^^^^^^^^^^^^^^^^^^^^> The changes here were made _in_ the arch code. While there may be valid
>> arguments for doing that, quoting the above commit as reason isn't really
>> sufficient.
> 
>    It seems you still don't understand... The i8253 drivers using IRQ0 live in
> the arch/{mips|x86}/kernel/ code, so they're allowed to claim IRQ0 via calling
> request_irq() (MIPS still does it.

   x86 too. PowerPC used to do that too, IIRC...

>> They are NOT the platform drivers we're dealing with here...

>> Guenter

MBR, Sergey

Re: [PATCH] sh: Avoid using IRQ0 on SH3 and SH4

[Guenter Roeck]

On Thu, Jul 06, 2023 at 06:57:04AM -0700, Guenter Roeck wrote:
> Hi,
> 
> On Thu, Jun 01, 2023 at 11:22:17PM +0300, Sergey Shtylyov wrote:
> > IRQ0 is no longer returned by platform_get_irq() and its ilk -- they now
> > return -EINVAL instead.  However, the kernel code supporting SH3/4-based
> > SoCs still 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 are using IRQ0 for the SMSC911x compatible Ethernet chip.
> > 
> 
> Unfortunately it also affects all sh4 emulations in qemu, and results in
> boot stalls with those. There isn't a relevant log to attach because there
> is no error message - booting just stalls until the emulation is aborted.
> 
> Reverting this patch fixes the problem.
> 
> Bisect log is attached for reference. Note that bisect requires applying
> commit 7497840d462c ("sh: Provide unxlate_dev_mem_ptr() in asm/io.h"),
> which is also the reason why the problem was not observed earlier since
> it was hiding behind a build failure.
> 

Since -rc1 is coming up and there has been no progress, it is time to start
tracking this problem as regression.

#regzbot ^introduced a8ac2961148e
#regzbot title sh4: Boot stall with qemu emulations
#regzbot ignore-activity

> Guenter
> 
> ---
> # bad: [c17414a273b81fe4e34e11d69fc30cc8b1431614] Merge tag 'sh-for-v6.5-tag1' of git://git.kernel.org/pub/scm/linux/kernel/git/glaubitz/sh-linux
> # good: [b5641a5d8b8b14643bfe3d017d64da90a5c55479] mm: don't do validate_mm() unnecessarily and without mmap locking
> git bisect start 'HEAD' 'b5641a5d8b8b'
> # good: [15ac468614e5e4fee82e1eb32568f427b0e51adc] Merge tag 'media/v6.5-1' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
> git bisect good 15ac468614e5e4fee82e1eb32568f427b0e51adc
> # good: [73a3fcdaa73200e38e38f7e8a32c9b901c5b95b5] Merge tag 'f2fs-for-6.5-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs
> git bisect good 73a3fcdaa73200e38e38f7e8a32c9b901c5b95b5
> # good: [6843306689aff3aea608e4d2630b2a5a0137f827] Merge tag 'net-6.5-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net
> git bisect good 6843306689aff3aea608e4d2630b2a5a0137f827
> # good: [afa92949124abc25ddab1789dd654214e2e1b040] dt-bindings: phy: cdns,salvo: add property cdns,usb2-disconnect-threshold-microvolt
> git bisect good afa92949124abc25ddab1789dd654214e2e1b040
> # good: [37bd215fc48ef2a399f836d62d2e4a166efb31be] phy: qualcomm: fix indentation in Makefile
> git bisect good 37bd215fc48ef2a399f836d62d2e4a166efb31be
> # bad: [7497840d462c8f54c4888c22ab3726a8cde4b9a2] sh: Provide unxlate_dev_mem_ptr() in asm/io.h
> git bisect bad 7497840d462c8f54c4888c22ab3726a8cde4b9a2
> # bad: [01658fe3d6c02992846a038c8111e70ace169295] sh: Refactor header include path addition
> git bisect bad 01658fe3d6c02992846a038c8111e70ace169295
> # bad: [a8ac2961148e8c720dc760f2e06627cd5c55a154] sh: Avoid using IRQ0 on SH3 and SH4
> git bisect bad a8ac2961148e8c720dc760f2e06627cd5c55a154
> # good: [bc9d1f0cecd2407cfb2364a7d4be2f52d1d46a9d] sh: j2: Use ioremap() to translate device tree address into kernel memory
> git bisect good bc9d1f0cecd2407cfb2364a7d4be2f52d1d46a9d
> # first bad commit: [a8ac2961148e8c720dc760f2e06627cd5c55a154] sh: Avoid using IRQ0 on SH3 and SH4