Re: [PATCH 2/2] MAINTAINERS: Make cxl obsolete
On Wed, 10 Apr 2024, Michael Ellerman wrote: > > > > Could we do something similar, write a message at boottime when the > > CXL driver gets probed ? > > Yeah, I think so. > > There's still the problem that people tend not to look at dmesg until > something breaks, but at least we can try and get their attention. > People would get in the habit to look for that, if all maintainers adopted a convention such that a boot-time message would list every newly-orphaned driver in each release. Maybe the maintainer of the MAINTAINERS file could check that every newly orphaned driver got announced.
[PATCH v3] serial/pmac_zilog: Remove flawed mitigation for rx irq flood
The mitigation was intended to stop the irq completely. That may be
better than a hard lock-up but it turns out that you get a crash anyway
if you're using pmac_zilog as a serial console:
ttyPZ0: pmz: rx irq flood !
BUG: spinlock recursion on CPU#0, swapper/0
That's because the pr_err() call in pmz_receive_chars() results in
pmz_console_write() attempting to lock a spinlock already locked in
pmz_interrupt(). With CONFIG_DEBUG_SPINLOCK=y, this produces a fatal
BUG splat. The spinlock in question is the one in struct uart_port.
Even when it's not fatal, the serial port rx function ceases to work.
Also, the iteration limit doesn't play nicely with QEMU, as can be
seen in the bug report linked below.
A web search for other reports of the error message "pmz: rx irq flood"
didn't produce anything. So I don't think this code is needed any more.
Remove it.
Cc: Benjamin Herrenschmidt
Cc: Michael Ellerman
Cc: Nicholas Piggin
Cc: Christophe Leroy
Cc: "Aneesh Kumar K.V"
Cc: "Naveen N. Rao"
Cc: Andy Shevchenko
Cc: sta...@kernel.org
Cc: linux-m...@lists.linux-m68k.org
Link: https://github.com/vivier/qemu-m68k/issues/44
Link: https://lore.kernel.org/all/1078874617.9746.36.camel@gaston/
Acked-by: Michael Ellerman (powerpc)
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Finn Thain
---
Changed since v1:
- Reworked commit log according to comments from Andy Shevchenko.
Changed since v2:
- Added Acked-by and Cc tags.
---
drivers/tty/serial/pmac_zilog.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
index c8bf08c19c64..77691fbbf779 100644
--- a/drivers/tty/serial/pmac_zilog.c
+++ b/drivers/tty/serial/pmac_zilog.c
@@ -210,7 +210,6 @@ static bool pmz_receive_chars(struct uart_pmac_port *uap)
{
struct tty_port *port;
unsigned char ch, r1, drop, flag;
- int loops = 0;
/* Sanity check, make sure the old bug is no longer happening */
if (uap->port.state == NULL) {
@@ -291,24 +290,11 @@ static bool pmz_receive_chars(struct uart_pmac_port *uap)
if (r1 & Rx_OVR)
tty_insert_flip_char(port, 0, TTY_OVERRUN);
next_char:
- /* We can get stuck in an infinite loop getting char 0 when the
-* line is in a wrong HW state, we break that here.
-* When that happens, I disable the receive side of the driver.
-* Note that what I've been experiencing is a real irq loop
where
-* I'm getting flooded regardless of the actual port speed.
-* Something strange is going on with the HW
-*/
- if ((++loops) > 1000)
- goto flood;
ch = read_zsreg(uap, R0);
if (!(ch & Rx_CH_AV))
break;
}
- return true;
- flood:
- pmz_interrupt_control(uap, 0);
- pmz_error("pmz: rx irq flood !\n");
return true;
}
--
2.39.3
Re: [PATCH v2] serial/pmac_zilog: Remove flawed mitigation for rx irq flood
On Fri, 5 Apr 2024, Michael Ellerman wrote: > I assume you have tested this on an actual pmac, as well as qemu? > I tested the patched driver and its console functionality using Zilog SCC hardware in a Mac IIci, as well as QEMU's q800 virtual machine. That should suffice from a code coverage point-of-view, since pmz_receive_chars() is portable and independent of CONFIG_PPC_PMAC. Moreover, I don't know how to get my PowerMac G3 to execute the kludge that's to be removed here. I can't prove it's impossible, though.
Re: [PATCH] serial/pmac_zilog: Remove flawed mitigation for rx irq flood
On Fri, 5 Apr 2024, Michael Ellerman wrote: > >> > > --- > >> > (here is a good location for Cc:) > >> > >> Documentation/process/submitting-patches.rst indicats that it should > >> be above the "---" separator together with Acked-by etc. Has this > >> convention changed recently? > > The docs don't really say where to put the Cc: tags, although they are > mentioned along with other tags which clearly are intended to go above > the separator. > I see no ambiguity there. What's the point of copying the message headers into the message body unless it was intended that they form part of the commit log? > > I see, I will prepare a patch to discuss this aspect. > > FYI there was a discussion about this several years ago, where at least > some maintainers agreed that Cc: tags don't add much value and are > better placed below the --- separator. > Maintainers who merge patches almost always add tags. And they can use the Cc tags from the message headers if they wish to. Or they can omit them or remove them. I don't mind. And I'd be happy to resubmit the patch with different tags if that's what is needed by the workflow used by Jiri Slaby or Greg Kroah-Hartman.
[PATCH v2] serial/pmac_zilog: Remove flawed mitigation for rx irq flood
The mitigation was intended to stop the irq completely. That may be
better than a hard lock-up but it turns out that you get a crash anyway
if you're using pmac_zilog as a serial console:
ttyPZ0: pmz: rx irq flood !
BUG: spinlock recursion on CPU#0, swapper/0
That's because the pr_err() call in pmz_receive_chars() results in
pmz_console_write() attempting to lock a spinlock already locked in
pmz_interrupt(). With CONFIG_DEBUG_SPINLOCK=y, this produces a fatal
BUG splat. The spinlock in question is the one in struct uart_port.
Even when it's not fatal, the serial port rx function ceases to work.
Also, the iteration limit doesn't play nicely with QEMU, as can be
seen in the bug report linked below.
A web search for other reports of the error message "pmz: rx irq flood"
didn't produce anything. So I don't think this code is needed any more.
Remove it.
Link: https://github.com/vivier/qemu-m68k/issues/44
Link: https://lore.kernel.org/all/1078874617.9746.36.camel@gaston/
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Finn Thain
---
Changed since v1:
- Reworked commit log according to comments from Andy Shevchenko.
---
drivers/tty/serial/pmac_zilog.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
index c8bf08c19c64..77691fbbf779 100644
--- a/drivers/tty/serial/pmac_zilog.c
+++ b/drivers/tty/serial/pmac_zilog.c
@@ -210,7 +210,6 @@ static bool pmz_receive_chars(struct uart_pmac_port *uap)
{
struct tty_port *port;
unsigned char ch, r1, drop, flag;
- int loops = 0;
/* Sanity check, make sure the old bug is no longer happening */
if (uap->port.state == NULL) {
@@ -291,24 +290,11 @@ static bool pmz_receive_chars(struct uart_pmac_port *uap)
if (r1 & Rx_OVR)
tty_insert_flip_char(port, 0, TTY_OVERRUN);
next_char:
- /* We can get stuck in an infinite loop getting char 0 when the
-* line is in a wrong HW state, we break that here.
-* When that happens, I disable the receive side of the driver.
-* Note that what I've been experiencing is a real irq loop
where
-* I'm getting flooded regardless of the actual port speed.
-* Something strange is going on with the HW
-*/
- if ((++loops) > 1000)
- goto flood;
ch = read_zsreg(uap, R0);
if (!(ch & Rx_CH_AV))
break;
}
- return true;
- flood:
- pmz_interrupt_control(uap, 0);
- pmz_error("pmz: rx irq flood !\n");
return true;
}
--
2.39.3
Re: [PATCH] serial/pmac_zilog: Remove flawed mitigation for rx irq flood
On Thu, 4 Apr 2024, Andy Shevchenko wrote: > > > > > --- > > > (here is a good location for Cc:) > > > > Documentation/process/submitting-patches.rst indicats that it should > > be above the "---" separator together with Acked-by etc. Has this > > convention changed recently? > > I see, I will prepare a patch to discuss this aspect. > If you are going to veto patches on the basis of rules yet unwritten, I think you risk turning the kernel development process into a lottery. How many other patches presently under review will need to be dropped just in case they don't conform with possible future rules?
Re: [PATCH] serial/pmac_zilog: Remove flawed mitigation for rx irq flood
On Thu, 4 Apr 2024, Andy Shevchenko wrote:
> ...
>
> First of all, please read this
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#backtraces-in-commit-messages
> and amend the commit message accordingly.
>
Right -- the call chain is described in the commit log message so the
backtrace does not add value. And the timestamps, stack dump etc. are
irrelevant.
> > Cc: Benjamin Herrenschmidt
> > Cc: Michael Ellerman
> > Cc: Nicholas Piggin
> > Cc: Christophe Leroy
> > Cc: "Aneesh Kumar K.V"
> > Cc: "Naveen N. Rao"
> > Cc: linux-m...@lists.linux-m68k.org
>
> Second, please move these Cc to be after the '---' line
>
I thought they were placed above the line for audit (and signing)
purposes. There are thousands of Cc lines in the mainline commit messages
since v6.8.
> > Link: https://github.com/vivier/qemu-m68k/issues/44
> > Link: https://lore.kernel.org/all/1078874617.9746.36.camel@gaston/
>
> Missed Fixes tag?
>
Would this be ok: Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
I have to ask because some reviewers do not like to see a Fixes tag cite
that commit.
> > Signed-off-by: Finn Thain
> > ---
> (here is a good location for Cc:)
>
Documentation/process/submitting-patches.rst indicats that it should be
above the "---" separator together with Acked-by etc. Has this convention
changed recently?
Thanks for your review.
[PATCH] serial/pmac_zilog: Remove flawed mitigation for rx irq flood
The mitigation was intended to stop the irq completely. That might have been better than a hard lock-up but it turns out that you get a crash anyway if you're using pmac_zilog as a serial console. That's because the pr_err() call in pmz_receive_chars() results in pmz_console_write() attempting to lock a spinlock already locked in pmz_interrupt(). With CONFIG_DEBUG_SPINLOCK=y, this produces a fatal BUG splat like the one below. (The spinlock at 0x62e140 is the one in struct uart_port.) Even when it's not fatal, the serial port rx function ceases to work. Also, the iteration limit doesn't play nicely with QEMU. Please see bug report linked below. A web search for reports of the error message "pmz: rx irq flood" didn't produce anything. So I don't think this code is needed any more. Remove it. [ 14.56] ttyPZ0: pmz: rx irq flood ! [ 14.56] BUG: spinlock recursion on CPU#0, swapper/0 [ 14.56] lock: 0x62e140, .magic: dead4ead, .owner: swapper/0, .owner_cpu: 0 [ 14.56] CPU: 0 PID: 0 Comm: swapper Not tainted 6.8.0-mac-dbg-preempt-4-g4143b7b9144a #1 [ 14.56] Stack from 0059bcc4: [ 14.56] 0059bcc4 0056316f 0056316f 2700 004b6444 0059bce4 004ad8c6 0056316f [ 14.56] 0059bd10 004a6546 00556759 0062e140 dead4ead 0059f892 [ 14.56] 0062e140 0059bde8 005c03d0 0059bd24 0004daf6 0062e140 005567bf 0062e140 [ 14.56] 0059bd34 004b64c2 0062e140 0001 0059bd50 002e15ea 0062e140 0001 [ 14.56] 0059bde7 0059bde8 005c03d0 0059bdac 0005124e 005c03d0 005cdc00 002b [ 14.56] 005a3caa 005a3caa 0059bde8 0004ff00 0059be8b 00038200 000529ba [ 14.56] Call Trace: [<2700>] ret_from_kernel_thread+0xc/0x14 [ 14.56] [<004b6444>] _raw_spin_lock+0x0/0x28 [ 14.56] [<004ad8c6>] dump_stack+0x10/0x16 [ 14.56] [<004a6546>] spin_dump+0x6e/0x7c [ 14.56] [<0004daf6>] do_raw_spin_lock+0x9c/0xa6 [ 14.56] [<004b64c2>] _raw_spin_lock_irqsave+0x2a/0x34 [ 14.56] [<002e15ea>] pmz_console_write+0x32/0x9a [ 14.56] [<0005124e>] console_flush_all+0x112/0x3a2 [ 14.56] [<0004ff00>] console_trylock+0x0/0x7a [ 14.56] [<00038200>] parameq+0x48/0x6e [ 14.56] [<000529ba>] __printk_safe_enter+0x0/0x36 [ 14.56] [<0005113c>] console_flush_all+0x0/0x3a2 [ 14.56] [<000542c4>] prb_read_valid+0x0/0x1a [ 14.56] [<004b65a4>] _raw_spin_unlock+0x0/0x38 [ 14.56] [<0005151e>] console_unlock+0x40/0xb8 [ 14.56] [<00038200>] parameq+0x48/0x6e [ 14.56] [<002c778c>] __tty_insert_flip_string_flags+0x0/0x14e [ 14.56] [<00051798>] vprintk_emit+0x156/0x238 [ 14.56] [<00051894>] vprintk_default+0x1a/0x1e [ 14.56] [<000529a8>] vprintk+0x74/0x86 [ 14.56] [<004a6596>] _printk+0x12/0x16 [ 14.56] [<002e23be>] pmz_receive_chars+0x1cc/0x394 [ 14.56] [<004b6444>] _raw_spin_lock+0x0/0x28 [ 14.56] [<00038226>] parse_args+0x0/0x3a6 [ 14.56] [<004b6466>] _raw_spin_lock+0x22/0x28 [ 14.56] [<002e26b4>] pmz_interrupt+0x12e/0x1e0 [ 14.56] [<00048680>] arch_cpu_idle_enter+0x0/0x8 [ 14.56] [<00054ebc>] __handle_irq_event_percpu+0x24/0x106 [ 14.56] [<004ae576>] default_idle_call+0x0/0x46 [ 14.56] [<00055020>] handle_irq_event+0x30/0x90 [ 14.56] [<00058320>] handle_simple_irq+0x5e/0xc0 [ 14.56] [<00048688>] arch_cpu_idle_exit+0x0/0x8 [ 14.56] [<00054800>] generic_handle_irq+0x3c/0x4a [ 14.56] [<2978>] do_IRQ+0x24/0x3a [ 14.56] [<004ae508>] cpu_idle_poll.isra.0+0x0/0x6e [ 14.56] [<2874>] auto_irqhandler_fixup+0x4/0xc [ 14.56] [<004ae508>] cpu_idle_poll.isra.0+0x0/0x6e [ 14.56] [<004ae576>] default_idle_call+0x0/0x46 [ 14.56] [<004ae598>] default_idle_call+0x22/0x46 [ 14.56] [<00048710>] do_idle+0x6a/0xf0 [ 14.56] [<000486a6>] do_idle+0x0/0xf0 [ 14.56] [<000367d2>] find_task_by_pid_ns+0x0/0x2a [ 14.56] [<0005d064>] __rcu_read_lock+0x0/0x12 [ 14.56] [<00048a5a>] cpu_startup_entry+0x18/0x1c [ 14.56] [<00063a06>] __rcu_read_unlock+0x0/0x26 [ 14.56] [<004ae65a>] kernel_init+0x0/0xfa [ 14.56] [<0049c5a8>] strcpy+0x0/0x1e [ 14.56] [<004a6584>] _printk+0x0/0x16 [ 14.56] [<0049c72a>] strlen+0x0/0x22 [ 14.56] [<006452d4>] memblock_alloc_try_nid+0x0/0x82 [ 14.56] [<0063939a>] arch_post_acpi_subsys_init+0x0/0x8 [ 14.56] [<0063991e>] console_on_rootfs+0x0/0x60 [ 14.56] [<00638410>] _sinittext+0x410/0xadc [ 14.56] Cc: Benjamin Herrenschmidt Cc: M
Re: Does anyone use Appletalk?
On Wed, 1 Nov 2023, Arnd Bergmann wrote: > > If we had not removed all localtalk support already, ipddp might have > been used to bridge between a pre-ethernet mac running macip and an IP > based AFP server (netatalk or time machine). Without localtalk support, > that is not all that interesting of course. > That line of reasoning misunderstands the value of the localtalk code (and conveniently neglects the actual cost of keeping it in-tree). The existing zilog driver works on all 68k and powerpc Macs with built-in serial ports. If we were to complete that driver by adding the missing localtalk support, it would create new opportunities for creative users/developers who already run Linux on those systems. Those users/developers would surely derive value from that functionality in ways we cannot anticipate, as happens over and over again in the (retrocomputing) community. So the value of the missing zilog localtalk functionality would be proportional to the number of Linux systems out there with the necessary serial hardware. It's value is not a function of the potential business opportunities for your sponsors, despite the prevailing incentives. It was the potential value of the missing code for localtalk (Zilog SCC) and Apple Sound Chip that caused me to place that work near the top of my to-do list. But that was several years ago. Unfortunately, with bug fixing and recapping, I still can't find time to write the necessary code. So I can't object to the removal of the localtalk code. But I do object to the underhand manner in which it is done.
Re: [PATCH] MAINTAINERS: Exclude m68k-only drivers from powerpc entry
On Wed, 31 May 2023, Geert Uytterhoeven wrote: > On Wed, May 31, 2023 at 2:50 PM Michael Ellerman wrote: > > The powerpc section has a "F:" entry for drivers/macintosh, matching > > all files in or below drivers/macintosh. That is correct for the most > > part, but there are a couple of m68k-only drivers in the directory, so > > exclude those. > > > > Signed-off-by: Michael Ellerman > > Thanks for your patch! > > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -11916,6 +11916,8 @@ L: linuxppc-dev@lists.ozlabs.org > > S: Odd Fixes > > F: arch/powerpc/platforms/powermac/ > > F: drivers/macintosh/ > > +X: drivers/macintosh/adb-iop.c > > +X: drivers/macintosh/via-macii.c > > > > LINUX FOR POWERPC (32-BIT AND 64-BIT) > > M: Michael Ellerman > > LGTM, as there are already entries for these two files under > "M68K ON APPLE MACINTOSH". Right. I should have addded those "X files" in commit 2ae92e8b9b7. > Acked-by: Geert Uytterhoeven > > Which leads us to a related topic: Is Joshua still around? Should Finn > be added or replace Joshua in the "M68K ON APPLE MACINTOSH" entry? > CC Joshua. If he's not around perhaps we'll see some bounces. Anyway, I'd be willing to either share the M68K ON APPLE MACINTOSH role or replace Joshua if he no longer wants that job. But I hope he does still want it as there's always driver work to do.
Re: [RFC v2 01/39] Kconfig: introduce HAS_IOPORT option and select it as necessary
On Sun, 8 May 2022, I wrote: > > That suggests to me that we need a "bool CONFIG_WARINGS_INTO_ERRORS" to > control -Werror, which could be disabled for .config files (like make > allmodconfig) where it is not helping. > I just noticed that we already have CONFIG_WERROR. So perhaps something like this would help. diff --git a/init/Kconfig b/init/Kconfig index ddcbefe535e9..765d83fb148e 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -150,6 +150,8 @@ config WERROR However, if you have a new (or very old) compiler with odd and unusual warnings, or you have some architecture with problems, + or if you are using a compiler that doesn't happen to interpret + the C standards in quite the same way as some other compilers, you may need to disable this config option in order to successfully build the kernel.
Re: [RFC v2 01/39] Kconfig: introduce HAS_IOPORT option and select it as necessary
Hi Arnd, On Sat, 7 May 2022, Arnd Bergmann wrote: > On Sat, May 7, 2022 at 2:01 AM Finn Thain wrote: > > On Fri, 6 May 2022, Niklas Schnelle wrote: > > > On Fri, 2022-05-06 at 19:12 +1000, Finn Thain wrote: > > > > On Thu, 5 May 2022, Bjorn Helgaas wrote: > > > > > > > > > > I mooted a s390 inb() implementation like "return ~0" because that's > > > > > what happens on most arches when there's no device to respond to the > > > > > inb(). > > > > > > > > > > The HAS_IOPORT dependencies are fairly ugly IMHO, and they clutter > > > > > drivers that use I/O ports in some cases but not others. But maybe > > > > > it's the most practical way. > > > > > > > > > > > > > Do you mean, "the most practical way to avoid a compiler warning on > > > > s390"? What about "#pragma GCC diagnostic ignored"? > > > > > > This actually happens with clang. > > > > That suggests a clang bug to me. If you believe GCC should behave like > > clang, then I guess the pragma above really is the one you want. If you > > somehow feel that the kernel should cater to gcc and clang even where they > > disagree then you would have to use "#pragma clang diagnostic ignored". > > I don't see how you can blame the compiler for this. On architectures > with a zero PCI_IOBASE, an inb(0x2f8) literally becomes > > var = *(u8*)((NULL + 0x2f8); > > If you run a driver that does this, the kernel gets a page fault for > the NULL page > and reports an Oops. clang tells you 'warning: performing pointer > arithmetic on a null pointer has undefined behavior', which is not exactly > spot on, but close enough to warn you that you probably shouldn't do this. gcc > doesn't warn here, but it does warn about an array out-of-bounds access when > you pass such a pointer into memcpy or another string function. > The appeal to UB is weak IMHO. Pointer arithmetic with a zero value is unambiguous and the compiler generates the code to implement the expected behaviour just fine. UB is literally an omission in the standard. Well, low level programming has always been beyond the scope of C standards. If architectural-level code wants to do arithmetic with an arbitrary integer values, and the compiler doesn't like it, then the relevant warnings should be disabled for those expressions. > > > Apart from that, I think this would also fall under the same argument as > > > the original patch Linus unpulled. We would just paint over someting > > > that we know at compile time won't work: > > > > > > https://lore.kernel.org/lkml/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/ > > > > > > > I wasn't advocating adding any warnings. > > > > If you know at compile time that a driver won't work, the usual solution > > is scripts/config -d CONFIG_SOME_UNDESIRED_DRIVER. Why is that no > > longer appropriate for drivers that use IO ports? > > This was never an option, we rely on 'make allmodconfig' to build > without warnings on all architectures for finding regressions. "All modules on all architectures with all compilers and checkers with all warnings enabled"? That's not even vaguely realistic. How about, "All modules on all architectures with a nominated compiler with the appropriate warnings enabled." > Any driver that depends on architecture specific interfaces must not get > selected on architectures that don't have those interfaces. > Kconfig always met that need before we got saddled with -Werror. That suggests to me that we need a "bool CONFIG_WARINGS_INTO_ERRORS" to control -Werror, which could be disabled for .config files (like make allmodconfig) where it is not helping.
Re: [RFC v2 01/39] Kconfig: introduce HAS_IOPORT option and select it as necessary
On Fri, 6 May 2022, Niklas Schnelle wrote: > On Fri, 2022-05-06 at 19:12 +1000, Finn Thain wrote: > > > > On Thu, 5 May 2022, Bjorn Helgaas wrote: > > > > > On Thu, May 05, 2022 at 07:39:42PM +0200, Arnd Bergmann wrote: > > > > On Thu, May 5, 2022 at 6:10 PM Bjorn Helgaas wrote: > > > > > On Wed, May 04, 2022 at 11:31:28PM +0200, Arnd Bergmann wrote: > > > > > > The main goal is to avoid c), which is what happens on s390, > > > > > > but can also happen elsewhere. Catching b) would be nice as > > > > > > well, but is much harder to do from generic code as you'd need > > > > > > an architecture specific inline asm statement to insert a > > > > > > ex_table fixup, or a runtime conditional on each access. > > > > > > > > > > Or s390 could implement its own inb(). > > > > > > > > > > I'm hearing that generic powerpc kernels have to run both on > > > > > machines that have I/O port space and those that don't. That > > > > > makes me think s390 could do something similar. > > > > > > > > No, this is actually the current situation, and it makes > > > > absolutely no sense. s390 has no way of implementing inb()/outb() > > > > because there are no instructions for it and it cannot tunnel them > > > > through a virtual address mapping like on most of the other > > > > architectures. (it has special instructions for accessing memory > > > > space, which is not the same as a pointer dereference here). > > > > > > > > The existing implementation gets flagged as a NULL pointer > > > > dereference by a compiler warning because it effectively is. > > > > > > I think s390 currently uses the inb() in asm-generic/io.h, i.e., > > > "__raw_readb(PCI_IOBASE + addr)". I understand that's a NULL > > > pointer dereference because the default PCI_IOBASE is 0. > > > > > > I mooted a s390 inb() implementation like "return ~0" because that's > > > what happens on most arches when there's no device to respond to the > > > inb(). > > > > > > The HAS_IOPORT dependencies are fairly ugly IMHO, and they clutter > > > drivers that use I/O ports in some cases but not others. But maybe > > > it's the most practical way. > > > > > > > Do you mean, "the most practical way to avoid a compiler warning on > > s390"? What about "#pragma GCC diagnostic ignored"? > > This actually happens with clang. That suggests a clang bug to me. If you believe GCC should behave like clang, then I guess the pragma above really is the one you want. If you somehow feel that the kernel should cater to gcc and clang even where they disagree then you would have to use "#pragma clang diagnostic ignored". > Apart from that, I think this would also fall under the same argument as > the original patch Linus unpulled. We would just paint over someting > that we know at compile time won't work: > > https://lore.kernel.org/lkml/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/ > I wasn't advocating adding any warnings. If you know at compile time that a driver won't work, the usual solution is scripts/config -d CONFIG_SOME_UNDESIRED_DRIVER. Why is that no longer appropriate for drivers that use IO ports?
Re: [RFC v2 01/39] Kconfig: introduce HAS_IOPORT option and select it as necessary
On Thu, 5 May 2022, Bjorn Helgaas wrote: > On Thu, May 05, 2022 at 07:39:42PM +0200, Arnd Bergmann wrote: > > On Thu, May 5, 2022 at 6:10 PM Bjorn Helgaas wrote: > > > On Wed, May 04, 2022 at 11:31:28PM +0200, Arnd Bergmann wrote: > > > > > > > > The main goal is to avoid c), which is what happens on s390, but > > > > can also happen elsewhere. Catching b) would be nice as well, > > > > but is much harder to do from generic code as you'd need an > > > > architecture specific inline asm statement to insert a ex_table > > > > fixup, or a runtime conditional on each access. > > > > > > Or s390 could implement its own inb(). > > > > > > I'm hearing that generic powerpc kernels have to run both on machines > > > that have I/O port space and those that don't. That makes me think > > > s390 could do something similar. > > > > No, this is actually the current situation, and it makes absolutely no > > sense. s390 has no way of implementing inb()/outb() because there > > are no instructions for it and it cannot tunnel them through a virtual > > address mapping like on most of the other architectures. (it has special > > instructions for accessing memory space, which is not the same as > > a pointer dereference here). > > > > The existing implementation gets flagged as a NULL pointer dereference > > by a compiler warning because it effectively is. > > I think s390 currently uses the inb() in asm-generic/io.h, i.e., > "__raw_readb(PCI_IOBASE + addr)". I understand that's a NULL pointer > dereference because the default PCI_IOBASE is 0. > > I mooted a s390 inb() implementation like "return ~0" because that's > what happens on most arches when there's no device to respond to the > inb(). > > The HAS_IOPORT dependencies are fairly ugly IMHO, and they clutter > drivers that use I/O ports in some cases but not others. But maybe > it's the most practical way. > Do you mean, "the most practical way to avoid a compiler warning on s390"? What about "#pragma GCC diagnostic ignored"?
[PATCH v2] macintosh/via-pmu: Fix build failure when CONFIG_INPUT is disabled
drivers/macintosh/via-pmu-event.o: In function `via_pmu_event':
via-pmu-event.c:(.text+0x44): undefined reference to `input_event'
via-pmu-event.c:(.text+0x68): undefined reference to `input_event'
via-pmu-event.c:(.text+0x94): undefined reference to `input_event'
via-pmu-event.c:(.text+0xb8): undefined reference to `input_event'
drivers/macintosh/via-pmu-event.o: In function `via_pmu_event_init':
via-pmu-event.c:(.init.text+0x20): undefined reference to
`input_allocate_device'
via-pmu-event.c:(.init.text+0xc4): undefined reference to
`input_register_device'
via-pmu-event.c:(.init.text+0xd4): undefined reference to `input_free_device'
make[1]: *** [Makefile:1155: vmlinux] Error 1
make: *** [Makefile:350: __build_one_by_one] Error 2
Don't call into the input subsystem unless CONFIG_INPUT is built-in.
Cc: Christophe Leroy
Cc: Randy Dunlap
Cc: Geert Uytterhoeven
Reported-by: kernel test robot
Signed-off-by: Finn Thain
---
Changed since v1:
- Adopted IS_ENABLED to avoid an ifdef as suggested by Christophe.
- Added the ADB_PMU_EVENT symbol as suggested by Geert, though this
adds a new Kconfig symbol for little gain AFAICS.
---
drivers/macintosh/Kconfig | 4
drivers/macintosh/Makefile | 3 ++-
drivers/macintosh/via-pmu.c | 2 +-
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig
index 5cdc361da37c..3942db15a2b8 100644
--- a/drivers/macintosh/Kconfig
+++ b/drivers/macintosh/Kconfig
@@ -67,6 +67,10 @@ config ADB_PMU
this device; you should do so if your machine is one of those
mentioned above.
+config ADB_PMU_EVENT
+ def_bool y
+ depends on ADB_PMU && INPUT=y
+
config ADB_PMU_LED
bool "Support for the Power/iBook front LED"
depends on PPC_PMAC && ADB_PMU
diff --git a/drivers/macintosh/Makefile b/drivers/macintosh/Makefile
index 49819b1b6f20..712edcb3e0b0 100644
--- a/drivers/macintosh/Makefile
+++ b/drivers/macintosh/Makefile
@@ -12,7 +12,8 @@ obj-$(CONFIG_MAC_EMUMOUSEBTN) += mac_hid.o
obj-$(CONFIG_INPUT_ADBHID) += adbhid.o
obj-$(CONFIG_ANSLCD) += ans-lcd.o
-obj-$(CONFIG_ADB_PMU) += via-pmu.o via-pmu-event.o
+obj-$(CONFIG_ADB_PMU) += via-pmu.o
+obj-$(CONFIG_ADB_PMU_EVENT)+= via-pmu-event.o
obj-$(CONFIG_ADB_PMU_LED) += via-pmu-led.o
obj-$(CONFIG_PMAC_BACKLIGHT) += via-pmu-backlight.o
obj-$(CONFIG_ADB_CUDA) += via-cuda.o
diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index 399074306a74..495fd35b11de 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -1463,7 +1463,7 @@ pmu_handle_data(unsigned char *data, int len)
pmu_pass_intr(data, len);
/* len == 6 is probably a bad check. But how do I
* know what PMU versions send what events here? */
- if (len == 6) {
+ if (IS_ENABLED(CONFIG_ADB_PMU_EVENT) && len == 6) {
via_pmu_event(PMU_EVT_POWER, !!(data[1]&8));
via_pmu_event(PMU_EVT_LID, data[1]&1);
}
--
2.32.0
Re: [PATCH] macintosh: fix via-pmu and via-cuda build errors
On Thu, 7 Apr 2022, Christophe Leroy wrote: > Le 07/04/2022 à 05:16, Randy Dunlap a écrit : > > > > > > On 4/6/22 19:37, Randy Dunlap wrote: > >> When CONFIG_INPUT=m, the input_*() family of functions is not > >> available to builtin drivers. > >> > >> When CONFIG_RTC_CLASS is not set, rtc_tm_to_time64() is not defined. > >> > >> Fix multiple build errors by making these Kconfig symbols required by > >> ADB_CUDA (RTC_CLASS) and ADB_PMU (RTC_CLASS and INPUT). > > > > Ah yes, Finn has already fixed the INPUT problems here. > > https://patchwork.ozlabs.org/project/linuxppc-dev/patch/d987663bbed18d7dbf106db6066a759040b4e57a.1647837028.git.fth...@linux-m68k.org/ > > > > > Maybe that patch hasn't been merged anywhere yet? > > The patch has comments, I guess we are waiting for a new version ? > I'll send a new version, though I expect it will attract comments too.
Re: [PATCH] macintosh: fix via-pmu and via-cuda build errors
On Wed, 6 Apr 2022, Randy Dunlap wrote: > When CONFIG_RTC_CLASS is not set, rtc_tm_to_time64() is not defined. > > ... > > m68k-linux-ld: drivers/macintosh/via-pmu.o: in function `pmu_set_rtc_time': > drivers/macintosh/via-pmu.c:1758: undefined reference to `rtc_tm_to_time64' > m68k-linux-ld: drivers/macintosh/via-cuda.o: in function `cuda_set_rtc_time': > drivers/macintosh/via-cuda.c:797: undefined reference to `rtc_tm_to_time64' > > ... > This is a big hammer type of patch. We could possibly do (a) some > conditional code blocks for RTC_CLASS rtc_tm_to_time64() call sites also appear in several other files without conditionals: arch/powerpc/kernel/time.c arch/powerpc/platforms/8xx/m8xx_setup.c arch/powerpc/platforms/maple/time.c arch/powerpc/platforms/powermac/time.c Why not use mktime64() instead? That seems to be a common pattern for this kind of thing (without needing conditional code).
[PATCH v2] macintosh/via-pmu: Avoid compiler warnings when CONFIG_PROC_FS is disabled
drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' defined
but not used [-Wunused-function]
static int pmu_battery_proc_show(struct seq_file *m, void *v)
^
drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' defined
but not used [-Wunused-function]
static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
^~
drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined but
not used [-Wunused-function]
static int pmu_info_proc_show(struct seq_file *m, void *v)
^~
Add some #ifdefs to avoid unused code warnings when CONFIG_PROC_FS is
disabled.
Cc: Randy Dunlap
Cc: Christophe Leroy
Reported-by: Randy Dunlap
Suggested-by: Christophe Leroy
Signed-off-by: Finn Thain
---
Changed since v1:
- Simplified to take advantage of the fact that proc_mkdir() is stubbed
out when CONFIG_PROC_FS=n. Hence that call doesn't need to move
within the #ifdef.
---
drivers/macintosh/via-pmu.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index 2109129ea1bb..495fd35b11de 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -204,9 +204,11 @@ static int init_pmu(void);
static void pmu_start(void);
static irqreturn_t via_pmu_interrupt(int irq, void *arg);
static irqreturn_t gpio1_interrupt(int irq, void *arg);
+#ifdef CONFIG_PROC_FS
static int pmu_info_proc_show(struct seq_file *m, void *v);
static int pmu_irqstats_proc_show(struct seq_file *m, void *v);
static int pmu_battery_proc_show(struct seq_file *m, void *v);
+#endif
static void pmu_pass_intr(unsigned char *data, int len);
static const struct proc_ops pmu_options_proc_ops;
@@ -857,6 +859,7 @@ query_battery_state(void)
2, PMU_SMART_BATTERY_STATE, pmu_cur_battery+1);
}
+#ifdef CONFIG_PROC_FS
static int pmu_info_proc_show(struct seq_file *m, void *v)
{
seq_printf(m, "PMU driver version : %d\n", PMU_DRIVER_VERSION);
@@ -977,6 +980,7 @@ static const struct proc_ops pmu_options_proc_ops = {
.proc_release = single_release,
.proc_write = pmu_options_proc_write,
};
+#endif
#ifdef CONFIG_ADB
/* Send an ADB command */
--
2.32.0
Re: [PATCH] Subject: [PATCH] macintosh/via-pmu: Avoid compiler warnings when CONFIG_PROC_FS is disabled
Please disregard. I'll send v2 with corrected commit log entry.
[PATCH] Subject: [PATCH] macintosh/via-pmu: Avoid compiler warnings when CONFIG_PROC_FS is disabled
drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' defined
but not used [-Wunused-function]
static int pmu_battery_proc_show(struct seq_file *m, void *v)
^
drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' defined
but not used [-Wunused-function]
static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
^~
drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined but
not used [-Wunused-function]
static int pmu_info_proc_show(struct seq_file *m, void *v)
^~
Rearrange some code and add some #ifdefs to avoid unused code warnings
when CONFIG_PROC_FS is disabled.
Reported-by: Randy Dunlap
Cc: Randy Dunlap
Cc: Christophe Leroy
Signed-off-by: Finn Thain
---
drivers/macintosh/via-pmu.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index 2109129ea1bb..495fd35b11de 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -204,9 +204,11 @@ static int init_pmu(void);
static void pmu_start(void);
static irqreturn_t via_pmu_interrupt(int irq, void *arg);
static irqreturn_t gpio1_interrupt(int irq, void *arg);
+#ifdef CONFIG_PROC_FS
static int pmu_info_proc_show(struct seq_file *m, void *v);
static int pmu_irqstats_proc_show(struct seq_file *m, void *v);
static int pmu_battery_proc_show(struct seq_file *m, void *v);
+#endif
static void pmu_pass_intr(unsigned char *data, int len);
static const struct proc_ops pmu_options_proc_ops;
@@ -857,6 +859,7 @@ query_battery_state(void)
2, PMU_SMART_BATTERY_STATE, pmu_cur_battery+1);
}
+#ifdef CONFIG_PROC_FS
static int pmu_info_proc_show(struct seq_file *m, void *v)
{
seq_printf(m, "PMU driver version : %d\n", PMU_DRIVER_VERSION);
@@ -977,6 +980,7 @@ static const struct proc_ops pmu_options_proc_ops = {
.proc_release = single_release,
.proc_write = pmu_options_proc_write,
};
+#endif
#ifdef CONFIG_ADB
/* Send an ADB command */
--
2.32.0
Re: [PATCH] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled
On Mon, 21 Mar 2022, Christophe Leroy wrote:
> Le 21/03/2022 à 09:50, Finn Thain a écrit :
> > Hi Christophe,
> >
> > On Mon, 21 Mar 2022, Finn Thain wrote:
> >
> >> On Mon, 21 Mar 2022, Christophe Leroy wrote:
> >>
> >>> Le 19/03/2022 à 08:20, Finn Thain a écrit :
> >>>> drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show'
> >>>> defined but not used [-Wunused-function]
> >>>>static int pmu_battery_proc_show(struct seq_file *m, void *v)
> >>>> ^
> >>>> drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show'
> >>>> defined but not used [-Wunused-function]
> >>>>static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
> >>>> ^~
> >>>> drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show'
> >>>> defined but not used [-Wunused-function]
> >>>>static int pmu_info_proc_show(struct seq_file *m, void *v)
> >>>> ^~
> >>>>
> >>>> Rearrange some code and add some #ifdefs to avoid unused code warnings
> >>>> when CONFIG_PROC_FS is disabled.
> >>>
> >>> Why not just put those three functions inside an #ifdef CONFIG_PROC_FS ?
> >>>
> >>
> >> You'd get a warning about the prototypes ("declared static but never
> >> defined"). Rather than add an ifdef around the prototypes as well, I
> >> just reordered things a little.
> >
> > Oops, I was forgetting that I also added an ifdef around the new
> > prototype.
> >
> > The simplest solution is probably the patch below, as it better exploits
> > the stubbed-out proc_* API in include/linux/proc_fs.h.
> >
> > Was this what you had in mind?
>
> Yes that's exactly what I had in mind.
>
OK, I'll submit it formally.
Thanks for your review.
Re: [PATCH] macintosh/via-pmu: Fix build failure when CONFIG_INPUT is disabled
On Mon, 21 Mar 2022, Geert Uytterhoeven wrote: > On Mon, Mar 21, 2022 at 9:29 AM Finn Thain wrote: > > On Mon, 21 Mar 2022, Christophe Leroy wrote: > > > Le 21/03/2022 à 05:30, Finn Thain a écrit : > > > > drivers/macintosh/via-pmu-event.o: In function `via_pmu_event': > > > > via-pmu-event.c:(.text+0x44): undefined reference to `input_event' > > > > via-pmu-event.c:(.text+0x68): undefined reference to `input_event' > > > > via-pmu-event.c:(.text+0x94): undefined reference to `input_event' > > > > via-pmu-event.c:(.text+0xb8): undefined reference to `input_event' > > > > drivers/macintosh/via-pmu-event.o: In function `via_pmu_event_init': > > > > via-pmu-event.c:(.init.text+0x20): undefined reference to > > > > `input_allocate_device' > > > > via-pmu-event.c:(.init.text+0xc4): undefined reference to > > > > `input_register_device' > > > > via-pmu-event.c:(.init.text+0xd4): undefined reference to > > > > `input_free_device' > > > > make[1]: *** [Makefile:1155: vmlinux] Error 1 > > > > make: *** [Makefile:350: __build_one_by_one] Error 2 > > > > > > > > Don't call into the input subsystem unless CONFIG_INPUT is built-in. > > > > > > > > Reported-by: kernel test robot > > > > Cc: Michael Ellerman > > > > Cc: Geert Uytterhoeven > > > > Cc: Randy Dunlap > > > > Signed-off-by: Finn Thain > > > > --- > > > > This is equivalent to the patch I sent a couple of days ago. This one > > > > is slightly longer and adds a new symbol so that Kconfig logic can been > > > > used instead of Makefile logic in case reviewers prefer that. > > > > --- > > > > drivers/macintosh/Kconfig | 5 + > > > > drivers/macintosh/Makefile | 3 ++- > > > > drivers/macintosh/via-pmu.c | 2 ++ > > > > 3 files changed, 9 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig > > > > index 5cdc361da37c..b9102f051bbb 100644 > > > > --- a/drivers/macintosh/Kconfig > > > > +++ b/drivers/macintosh/Kconfig > > > > @@ -67,6 +67,11 @@ config ADB_PMU > > > > this device; you should do so if your machine is one of those > > > > mentioned above. > > > > > > > > +config ADB_PMU_EVENT > > > > + bool > > > > + depends on ADB_PMU && INPUT=y > > > > + default y > > > > > > Could be reduced to > > > > > > config ADB_PMU_EVENT > > > def_bool y if ADB_PMU && INPUT=y > > > > That's great but my question remains unanswered: why the aversion to > > conditionals in Makefiles, when that would be simpler (no new symbol)? > > While conditionals in Makefiles do exist, they are far less common, Perhaps the Kconfig solution is more common. I did look for it but didn't find it in recent commits. Maybe I didn't look hard enough. Mostly I see invisible Kconfig getting used for things that are hard to do in any simpler way. > and can be confusing. Kconfig can be confusing too. I don't think your approach reduces complexity, I think it increases it. > They're also harder to grep for. > E.g. "git grep via-pmu-event.o" after your alternative patch would > give: > > obj-$(CONFIG_ADB_PMU) += via-pmu-event.o > > but would miss the important surrounding part: > > ifeq ($(CONFIG_INPUT), y) > obj-$(CONFIG_ADB_PMU) += via-pmu-event.o > endif > > Keeping configuration logic in a single place (the Kconfig file) > avoids that. The extra symbol is invisible, so it doesn't hurt much. > The same argument applies to Kconfig. "git grep CONFIG_ADB_PMU_EVENT" doesn't even mention drivers/macintosh/Kconfig. If you do "git grep ADB_PMU_EVENT" you get: drivers/macintosh/Kconfig:config ADB_PMU_EVENT with no mention of the "important surrounding part" that you were concerned about: config ADB_PMU_EVENT def_bool y if ADB_PMU && INPUT=y Anyway, we don't have to debate this, as drivers/macintosh already has a maintainer who can decide the question. Both patches are available.
Re: [PATCH] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled
Hi Christophe,
On Mon, 21 Mar 2022, Finn Thain wrote:
> On Mon, 21 Mar 2022, Christophe Leroy wrote:
>
> > Le 19/03/2022 à 08:20, Finn Thain a écrit :
> > > drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show'
> > > defined but not used [-Wunused-function]
> > > static int pmu_battery_proc_show(struct seq_file *m, void *v)
> > > ^
> > > drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show'
> > > defined but not used [-Wunused-function]
> > > static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
> > > ^~
> > > drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined
> > > but not used [-Wunused-function]
> > > static int pmu_info_proc_show(struct seq_file *m, void *v)
> > > ^~
> > >
> > > Rearrange some code and add some #ifdefs to avoid unused code warnings
> > > when CONFIG_PROC_FS is disabled.
> >
> > Why not just put those three functions inside an #ifdef CONFIG_PROC_FS ?
> >
>
> You'd get a warning about the prototypes ("declared static but never
> defined"). Rather than add an ifdef around the prototypes as well, I
> just reordered things a little.
Oops, I was forgetting that I also added an ifdef around the new
prototype.
The simplest solution is probably the patch below, as it better exploits
the stubbed-out proc_* API in include/linux/proc_fs.h.
Was this what you had in mind?
diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index 2109129ea1bb..495fd35b11de 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -204,9 +204,11 @@ static int init_pmu(void);
static void pmu_start(void);
static irqreturn_t via_pmu_interrupt(int irq, void *arg);
static irqreturn_t gpio1_interrupt(int irq, void *arg);
+#ifdef CONFIG_PROC_FS
static int pmu_info_proc_show(struct seq_file *m, void *v);
static int pmu_irqstats_proc_show(struct seq_file *m, void *v);
static int pmu_battery_proc_show(struct seq_file *m, void *v);
+#endif
static void pmu_pass_intr(unsigned char *data, int len);
static const struct proc_ops pmu_options_proc_ops;
@@ -857,6 +859,7 @@ query_battery_state(void)
2, PMU_SMART_BATTERY_STATE, pmu_cur_battery+1);
}
+#ifdef CONFIG_PROC_FS
static int pmu_info_proc_show(struct seq_file *m, void *v)
{
seq_printf(m, "PMU driver version : %d\n", PMU_DRIVER_VERSION);
@@ -977,6 +980,7 @@ static const struct proc_ops pmu_options_proc_ops = {
.proc_release = single_release,
.proc_write = pmu_options_proc_write,
};
+#endif
#ifdef CONFIG_ADB
/* Send an ADB command */
Re: [PATCH] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled
On Mon, 21 Mar 2022, Christophe Leroy wrote:
> Le 19/03/2022 à 08:20, Finn Thain a écrit :
> > drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show'
> > defined but not used [-Wunused-function]
> > static int pmu_battery_proc_show(struct seq_file *m, void *v)
> > ^
> > drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show'
> > defined but not used [-Wunused-function]
> > static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
> > ^~
> > drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined
> > but not used [-Wunused-function]
> > static int pmu_info_proc_show(struct seq_file *m, void *v)
> > ^~
> >
> > Rearrange some code and add some #ifdefs to avoid unused code warnings
> > when CONFIG_PROC_FS is disabled.
>
> Why not just put those three functions inside an #ifdef CONFIG_PROC_FS ?
>
You'd get a warning about the prototypes ("declared static but never
defined"). Rather than add an ifdef around the prototypes as well, I just
reordered things a little.
Re: [PATCH] macintosh/via-pmu: Fix build failure when CONFIG_INPUT is disabled
On Mon, 21 Mar 2022, Christophe Leroy wrote: > > > Le 21/03/2022 à 05:30, Finn Thain a écrit : > > drivers/macintosh/via-pmu-event.o: In function `via_pmu_event': > > via-pmu-event.c:(.text+0x44): undefined reference to `input_event' > > via-pmu-event.c:(.text+0x68): undefined reference to `input_event' > > via-pmu-event.c:(.text+0x94): undefined reference to `input_event' > > via-pmu-event.c:(.text+0xb8): undefined reference to `input_event' > > drivers/macintosh/via-pmu-event.o: In function `via_pmu_event_init': > > via-pmu-event.c:(.init.text+0x20): undefined reference to > > `input_allocate_device' > > via-pmu-event.c:(.init.text+0xc4): undefined reference to > > `input_register_device' > > via-pmu-event.c:(.init.text+0xd4): undefined reference to > > `input_free_device' > > make[1]: *** [Makefile:1155: vmlinux] Error 1 > > make: *** [Makefile:350: __build_one_by_one] Error 2 > > > > Don't call into the input subsystem unless CONFIG_INPUT is built-in. > > > > Reported-by: kernel test robot > > Cc: Michael Ellerman > > Cc: Geert Uytterhoeven > > Cc: Randy Dunlap > > Signed-off-by: Finn Thain > > --- > > This is equivalent to the patch I sent a couple of days ago. This one > > is slightly longer and adds a new symbol so that Kconfig logic can been > > used instead of Makefile logic in case reviewers prefer that. > > --- > > drivers/macintosh/Kconfig | 5 + > > drivers/macintosh/Makefile | 3 ++- > > drivers/macintosh/via-pmu.c | 2 ++ > > 3 files changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig > > index 5cdc361da37c..b9102f051bbb 100644 > > --- a/drivers/macintosh/Kconfig > > +++ b/drivers/macintosh/Kconfig > > @@ -67,6 +67,11 @@ config ADB_PMU > > this device; you should do so if your machine is one of those > > mentioned above. > > > > +config ADB_PMU_EVENT > > + bool > > + depends on ADB_PMU && INPUT=y > > + default y > > Could be reduced to > > config ADB_PMU_EVENT > def_bool y if ADB_PMU && INPUT=y > That's great but my question remains unanswered: why the aversion to conditionals in Makefiles, when that would be simpler (no new symbol)?
[PATCH] macintosh/via-pmu: Fix build failure when CONFIG_INPUT is disabled
drivers/macintosh/via-pmu-event.o: In function `via_pmu_event':
via-pmu-event.c:(.text+0x44): undefined reference to `input_event'
via-pmu-event.c:(.text+0x68): undefined reference to `input_event'
via-pmu-event.c:(.text+0x94): undefined reference to `input_event'
via-pmu-event.c:(.text+0xb8): undefined reference to `input_event'
drivers/macintosh/via-pmu-event.o: In function `via_pmu_event_init':
via-pmu-event.c:(.init.text+0x20): undefined reference to
`input_allocate_device'
via-pmu-event.c:(.init.text+0xc4): undefined reference to
`input_register_device'
via-pmu-event.c:(.init.text+0xd4): undefined reference to `input_free_device'
make[1]: *** [Makefile:1155: vmlinux] Error 1
make: *** [Makefile:350: __build_one_by_one] Error 2
Don't call into the input subsystem unless CONFIG_INPUT is built-in.
Reported-by: kernel test robot
Cc: Michael Ellerman
Cc: Geert Uytterhoeven
Cc: Randy Dunlap
Signed-off-by: Finn Thain
---
This is equivalent to the patch I sent a couple of days ago. This one
is slightly longer and adds a new symbol so that Kconfig logic can been
used instead of Makefile logic in case reviewers prefer that.
---
drivers/macintosh/Kconfig | 5 +
drivers/macintosh/Makefile | 3 ++-
drivers/macintosh/via-pmu.c | 2 ++
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig
index 5cdc361da37c..b9102f051bbb 100644
--- a/drivers/macintosh/Kconfig
+++ b/drivers/macintosh/Kconfig
@@ -67,6 +67,11 @@ config ADB_PMU
this device; you should do so if your machine is one of those
mentioned above.
+config ADB_PMU_EVENT
+ bool
+ depends on ADB_PMU && INPUT=y
+ default y
+
config ADB_PMU_LED
bool "Support for the Power/iBook front LED"
depends on PPC_PMAC && ADB_PMU
diff --git a/drivers/macintosh/Makefile b/drivers/macintosh/Makefile
index 49819b1b6f20..712edcb3e0b0 100644
--- a/drivers/macintosh/Makefile
+++ b/drivers/macintosh/Makefile
@@ -12,7 +12,8 @@ obj-$(CONFIG_MAC_EMUMOUSEBTN) += mac_hid.o
obj-$(CONFIG_INPUT_ADBHID) += adbhid.o
obj-$(CONFIG_ANSLCD) += ans-lcd.o
-obj-$(CONFIG_ADB_PMU) += via-pmu.o via-pmu-event.o
+obj-$(CONFIG_ADB_PMU) += via-pmu.o
+obj-$(CONFIG_ADB_PMU_EVENT)+= via-pmu-event.o
obj-$(CONFIG_ADB_PMU_LED) += via-pmu-led.o
obj-$(CONFIG_PMAC_BACKLIGHT) += via-pmu-backlight.o
obj-$(CONFIG_ADB_CUDA) += via-cuda.o
diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index b1859e5340b3..022e2fd4397b 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -1468,12 +1468,14 @@ pmu_handle_data(unsigned char *data, int len)
if (pmu_battery_count)
query_battery_state();
pmu_pass_intr(data, len);
+#ifdef CONFIG_ADB_PMU_EVENT
/* len == 6 is probably a bad check. But how do I
* know what PMU versions send what events here? */
if (len == 6) {
via_pmu_event(PMU_EVT_POWER, !!(data[1]&8));
via_pmu_event(PMU_EVT_LID, data[1]&1);
}
+#endif
break;
default:
--
2.32.0
Re: [PATCH] macintosh/via-pmu: Fix build failure when CONFIG_INPUT is disabled
On Sat, 19 Mar 2022, Geert Uytterhoeven wrote:
> On Sat, Mar 19, 2022 at 5:23 AM Finn Thain wrote:
> > drivers/macintosh/via-pmu-event.o: In function `via_pmu_event':
> > via-pmu-event.c:(.text+0x44): undefined reference to `input_event'
> > via-pmu-event.c:(.text+0x68): undefined reference to `input_event'
> > via-pmu-event.c:(.text+0x94): undefined reference to `input_event'
> > via-pmu-event.c:(.text+0xb8): undefined reference to `input_event'
> > drivers/macintosh/via-pmu-event.o: In function `via_pmu_event_init':
> > via-pmu-event.c:(.init.text+0x20): undefined reference to
> > `input_allocate_device'
> > via-pmu-event.c:(.init.text+0xc4): undefined reference to
> > `input_register_device'
> > via-pmu-event.c:(.init.text+0xd4): undefined reference to
> > `input_free_device'
> > make[1]: *** [Makefile:1155: vmlinux] Error 1
> > make: *** [Makefile:350: __build_one_by_one] Error 2
> >
> > Don't call into the input subsystem unless CONFIG_INPUT is built-in.
> >
> > Reported-by: kernel test robot
> > Cc: Michael Ellerman
> > Cc: Geert Uytterhoeven
> > Signed-off-by: Finn Thain
>
> Thanks for your patch!
>
Thanks for your review.
> > --- a/drivers/macintosh/Makefile
> > +++ b/drivers/macintosh/Makefile
> > @@ -12,7 +12,10 @@ obj-$(CONFIG_MAC_EMUMOUSEBTN)+= mac_hid.o
> > obj-$(CONFIG_INPUT_ADBHID) += adbhid.o
> > obj-$(CONFIG_ANSLCD) += ans-lcd.o
> >
> > -obj-$(CONFIG_ADB_PMU) += via-pmu.o via-pmu-event.o
> > +obj-$(CONFIG_ADB_PMU) += via-pmu.o
> > +ifeq ($(CONFIG_INPUT), y)
> > +obj-$(CONFIG_ADB_PMU) += via-pmu-event.o
> > +endif
>
> Alternatively, you can introduce an invisible Kconfig symbol that
> is y if ADB_PMU && INPUT, to control the build of via-pmu.o.
>
There is some precent for that (DVB_AV7110_IR, PINCTRL_FALCON,
DVB_AV7110_IR) in recent code but it's a bit of a stretch.
Adding the new Kconfig symbol would add complexity and it would only get
used in two places.
I appreciate the general preference for declarative style over imperative.
But is there a stronger argument against conditionals in Makefiles?
> > obj-$(CONFIG_ADB_PMU_LED) += via-pmu-led.o
> > obj-$(CONFIG_PMAC_BACKLIGHT) += via-pmu-backlight.o
> > obj-$(CONFIG_ADB_CUDA) += via-cuda.o
> > diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
> > index 4b98bc26a94b..55afa6dfa263 100644
> > --- a/drivers/macintosh/via-pmu.c
> > +++ b/drivers/macintosh/via-pmu.c
> > @@ -1457,12 +1457,14 @@ pmu_handle_data(unsigned char *data, int len)
> > if (pmu_battery_count)
> > query_battery_state();
> > pmu_pass_intr(data, len);
> > +#ifdef CONFIG_INPUT
> > /* len == 6 is probably a bad check. But how do I
> > * know what PMU versions send what events here? */
> > if (len == 6) {
> > via_pmu_event(PMU_EVT_POWER, !!(data[1]&8));
> > via_pmu_event(PMU_EVT_LID, data[1]&1);
> > }
> > +#endif
>
> Additionally, if that new symbol is not enabled, a dummy via_pmu_event()
> can be provided, so you don't need to add an #ifdef to the driver anymore.
>
Adding a dummy function to be used in only one place seems questionable to
me. I'm not expecting new call sites to appear outside of that ifdef.
Some of the arguments against ifdefs (reduced test and checker coverage,
readability harm) don't really apply in this case.
[PATCH] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled
drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' defined
but not used [-Wunused-function]
static int pmu_battery_proc_show(struct seq_file *m, void *v)
^
drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' defined
but not used [-Wunused-function]
static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
^~
drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined but
not used [-Wunused-function]
static int pmu_info_proc_show(struct seq_file *m, void *v)
^~
Rearrange some code and add some #ifdefs to avoid unused code warnings
when CONFIG_PROC_FS is disabled.
Reported-by: Randy Dunlap
Cc: Randy Dunlap
Signed-off-by: Finn Thain
---
drivers/macintosh/via-pmu.c | 61 ++---
1 file changed, 36 insertions(+), 25 deletions(-)
diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index 55afa6dfa263..5ffebf29b630 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -173,10 +173,15 @@ static unsigned long async_req_locks;
#define NUM_IRQ_STATS 13
static unsigned int pmu_irq_stats[NUM_IRQ_STATS];
+#ifdef CONFIG_PROC_FS
static struct proc_dir_entry *proc_pmu_root;
static struct proc_dir_entry *proc_pmu_info;
static struct proc_dir_entry *proc_pmu_irqstats;
static struct proc_dir_entry *proc_pmu_options;
+static struct proc_dir_entry *proc_pmu_batt[PMU_MAX_BATTERIES];
+static void pmu_proc_setup(void);
+#endif
+
static int option_server_mode;
int pmu_battery_count;
@@ -185,7 +190,6 @@ unsigned int pmu_power_flags = PMU_PWR_AC_PRESENT;
struct pmu_battery_info pmu_batteries[PMU_MAX_BATTERIES];
static int query_batt_timer = BATTERY_POLLING_COUNT;
static struct adb_request batt_req;
-static struct proc_dir_entry *proc_pmu_batt[PMU_MAX_BATTERIES];
int asleep;
@@ -204,11 +208,7 @@ static int init_pmu(void);
static void pmu_start(void);
static irqreturn_t via_pmu_interrupt(int irq, void *arg);
static irqreturn_t gpio1_interrupt(int irq, void *arg);
-static int pmu_info_proc_show(struct seq_file *m, void *v);
-static int pmu_irqstats_proc_show(struct seq_file *m, void *v);
-static int pmu_battery_proc_show(struct seq_file *m, void *v);
static void pmu_pass_intr(unsigned char *data, int len);
-static const struct proc_ops pmu_options_proc_ops;
#ifdef CONFIG_ADB
const struct adb_driver via_pmu_driver = {
@@ -551,26 +551,9 @@ static int __init via_pmu_dev_init(void)
}
#endif /* CONFIG_PPC32 */
- /* Create /proc/pmu */
- proc_pmu_root = proc_mkdir("pmu", NULL);
- if (proc_pmu_root) {
- long i;
-
- for (i=0; i
Re: [PATCH] macintosh/via-pmu: Fix build failure when CONFIG_INPUT is disabled
On Fri, 18 Mar 2022, Randy Dunlap wrote: > > Hi Finn, > It builds without those reported errors, but I do see these warnings > since the robot-supplied .config file has: > # CONFIG_PROC_FS is not set > > > CC drivers/macintosh/via-pmu.o > ../drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' > defined but not used [-Wunused-function] > 897 | static int pmu_battery_proc_show(struct seq_file *m, void *v) > |^ > ../drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' > defined but not used [-Wunused-function] > 871 | static int pmu_irqstats_proc_show(struct seq_file *m, void *v) > |^~ > ../drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined > but not used [-Wunused-function] > 860 | static int pmu_info_proc_show(struct seq_file *m, void *v) > |^~ > I see. I'll send a separate patch for this issue.
[PATCH] macintosh/via-pmu: Fix build failure when CONFIG_INPUT is disabled
drivers/macintosh/via-pmu-event.o: In function `via_pmu_event':
via-pmu-event.c:(.text+0x44): undefined reference to `input_event'
via-pmu-event.c:(.text+0x68): undefined reference to `input_event'
via-pmu-event.c:(.text+0x94): undefined reference to `input_event'
via-pmu-event.c:(.text+0xb8): undefined reference to `input_event'
drivers/macintosh/via-pmu-event.o: In function `via_pmu_event_init':
via-pmu-event.c:(.init.text+0x20): undefined reference to
`input_allocate_device'
via-pmu-event.c:(.init.text+0xc4): undefined reference to
`input_register_device'
via-pmu-event.c:(.init.text+0xd4): undefined reference to `input_free_device'
make[1]: *** [Makefile:1155: vmlinux] Error 1
make: *** [Makefile:350: __build_one_by_one] Error 2
Don't call into the input subsystem unless CONFIG_INPUT is built-in.
Reported-by: kernel test robot
Cc: Michael Ellerman
Cc: Geert Uytterhoeven
Signed-off-by: Finn Thain
---
drivers/macintosh/Makefile | 5 -
drivers/macintosh/via-pmu.c | 2 ++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/macintosh/Makefile b/drivers/macintosh/Makefile
index 49819b1b6f20..eaf28b1c272f 100644
--- a/drivers/macintosh/Makefile
+++ b/drivers/macintosh/Makefile
@@ -12,7 +12,10 @@ obj-$(CONFIG_MAC_EMUMOUSEBTN)+= mac_hid.o
obj-$(CONFIG_INPUT_ADBHID) += adbhid.o
obj-$(CONFIG_ANSLCD) += ans-lcd.o
-obj-$(CONFIG_ADB_PMU) += via-pmu.o via-pmu-event.o
+obj-$(CONFIG_ADB_PMU) += via-pmu.o
+ifeq ($(CONFIG_INPUT), y)
+obj-$(CONFIG_ADB_PMU) += via-pmu-event.o
+endif
obj-$(CONFIG_ADB_PMU_LED) += via-pmu-led.o
obj-$(CONFIG_PMAC_BACKLIGHT) += via-pmu-backlight.o
obj-$(CONFIG_ADB_CUDA) += via-cuda.o
diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index 4b98bc26a94b..55afa6dfa263 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -1457,12 +1457,14 @@ pmu_handle_data(unsigned char *data, int len)
if (pmu_battery_count)
query_battery_state();
pmu_pass_intr(data, len);
+#ifdef CONFIG_INPUT
/* len == 6 is probably a bad check. But how do I
* know what PMU versions send what events here? */
if (len == 6) {
via_pmu_event(PMU_EVT_POWER, !!(data[1]&8));
via_pmu_event(PMU_EVT_LID, data[1]&1);
}
+#endif
break;
default:
--
2.32.0
Re: [PATCH] powerpc: Fix sigset_t copy
On Wed, 10 Nov 2021, Christophe Leroy wrote: > Le 10/11/2021 à 00:47, Finn Thain a écrit : > > > Christophe, I hope this change is the one you wanted to see upstream > > (?). If it is acceptable please add your signed-off-by tag. > > I'm on holidays, I was planing to handle this next week. > OK. I'll leave it with you.
[PATCH] powerpc: Fix sigset_t copy
From: Christophe Leroy
The conversion from __copy_from_user() to __get_user() introduced a
regression in __get_user_sigset() in v5.13. The bug was subsequently
copied and pasted in unsafe_get_user_sigset().
The regression was reported by users of the Xorg packages distributed in
Debian/powerpc --
"The symptoms are that the fb screen goes blank, with the backlight
remaining on and no errors logged in /var/log; wdm (or startx) run
with no effect (I tried logging in in the blind, with no effect).
And they are hard to kill, requiring 'kill -KILL ...'"
Fix the regression by casting the __get_user() assignment lvalue to u64
so that the entire struct gets copied.
Cc: Christophe Leroy
Cc: Christopher M. Riedl
Link:
https://lore.kernel.org/linuxppc-dev/FEtBUOuFPMN4zJy4bIOqz6C4xoliCbTxS7VtMKD6UZkbvEbycUceRgGAd7e9-trRdwVN3hWAbQi0qrNx8Zgn8niTQf2KPVdw-W35czDIaeQ=@protonmail.com/
Fixes: 887f3ceb51cd ("powerpc/signal32: Convert do_setcontext[_tm]() to user
access block")
Fixes: d3ccc9781560 ("powerpc/signal: Use __get_user() to copy sigset_t")
Reported-and-tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
Christophe, I hope this change is the one you wanted to see upstream (?).
If it is acceptable please add your signed-off-by tag.
---
arch/powerpc/kernel/signal.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/signal.h b/arch/powerpc/kernel/signal.h
index 1f07317964e4..44e736b88e91 100644
--- a/arch/powerpc/kernel/signal.h
+++ b/arch/powerpc/kernel/signal.h
@@ -23,10 +23,10 @@ static inline int __get_user_sigset(sigset_t *dst, const
sigset_t __user *src)
{
BUILD_BUG_ON(sizeof(sigset_t) != sizeof(u64));
- return __get_user(dst->sig[0], (u64 __user *)>sig[0]);
+ return __get_user(*(u64 *)>sig[0], (u64 __user *)>sig[0]);
}
#define unsafe_get_user_sigset(dst, src, label) \
- unsafe_get_user((dst)->sig[0], (u64 __user *)&(src)->sig[0], label)
+ unsafe_get_user(*(u64 *)&(dst)->sig[0], (u64 __user *)&(src)->sig[0],
label)
#ifdef CONFIG_VSX
extern unsigned long copy_vsx_to_user(void __user *to,
--
2.26.3
Re: Fwd: Fwd: X stopped working with 5.14 on iBook
On Thu, 4 Nov 2021, Christophe Leroy wrote:
> Le 02/11/2021 à 03:20, Finn Thain a écrit :
> > Hi Christopher,
> >
> > After many builds and tests, Stan and I were able to determine that this
> > regression only affects builds with CONFIG_USER_NS=y. That is,
> >
> > d3ccc9781560 + CONFIG_USER_NS=y --> fail
> > d3ccc9781560 + CONFIG_USER_NS=n --> okay
> > d3ccc9781560~ + CONFIG_USER_NS=y --> okay
> > d3ccc9781560~ + CONFIG_USER_NS=n --> okay
> >
> > Stan also tested a PowerMac G3 system and found that the regression is not
> > present there. Thus far, only PowerMac G4 systems are known to be affected
> > (Stan's Cube and Riccardo's PowerBook).
> >
> > I asked Stan to try v5.15-rc after reverting commit d3ccc9781560.
> > Unexpectedly, this build had the same issue. So, it appears there are
> > multiple bad commits that produce this Xorg failure, of which d3ccc9781560
> > is just the first.
> >
> > But there's no easy way to identify the other bad commits using bisection.
> > So I've addressed this message to you. Can you help fix this regression?
> >
>
> I'm wondering if this commit is really the cause of the problem.
>
> Are you using GCC 11 ?
>
> If yes, I think it could be a false positive, fixed by
> https://github.com/linuxppc/linux/commit/7315e457d6bc
>
> Can you try with GCC 10 or older ?
>
AFAIK, all of Stan's builds were made with gcc 10.
> Can you cherry pick 7315e457d6bc ("powerpc/uaccess: Fix __get_user() with
> CONFIG_CC_HAS_ASM_GOTO_OUTPUT") on top of d3ccc9781560 and see what happens ?
>
$ git checkout d3ccc9781560
$ git cherry-pick 7315e457d6bc
Auto-merging arch/powerpc/include/asm/uaccess.h
CONFLICT (content): Merge conflict in arch/powerpc/include/asm/uaccess.h
error: could not apply 7315e457d6bc... powerpc/uaccess: Fix __get_user() with
CONFIG_CC_HAS_ASM_GOTO_OUTPUT
There is no __get_user_asm2_goto in this tree, and __get_user_asm2 already
has the "=" constraint:
#define __get_user_asm2(x, addr, err) \
__asm__ __volatile__( \
"1: lwz%X2 %1, %2\n"\
"2: lwz%X2 %L1, %L2\n" \
"3:\n" \
".section .fixup,\"ax\"\n" \
"4: li %0,%3\n" \
" li %1,0\n" \
" li %1+1,0\n"\
" b 3b\n" \
".previous\n" \
EX_TABLE(1b, 4b)\
EX_TABLE(2b, 4b)\
: "=r" (err), "=" (x) \
: "m" (*addr), "i" (-EFAULT), "0" (err))
Re: Fwd: Fwd: X stopped working with 5.14 on iBook
On Wed, 3 Nov 2021, Andreas Schwab wrote: > On Nov 02 2021, Finn Thain wrote: > > > After many builds and tests, Stan and I were able to determine that this > > regression only affects builds with CONFIG_USER_NS=y. That is, > > > > d3ccc9781560 + CONFIG_USER_NS=y --> fail > > d3ccc9781560 + CONFIG_USER_NS=n --> okay > > d3ccc9781560~ + CONFIG_USER_NS=y --> okay > > d3ccc9781560~ + CONFIG_USER_NS=n --> okay > > On my iBook G4, X is working alright with 5.15 and CONFIG_USER_NS=y. > Stan said his Cube has these packages installed: # dpkg --list | grep Xorg ii xserver-xorg-core 2:1.20.11-1 powerpc Xorg X server - core server ii xserver-xorg-legacy2:1.20.11-1 powerpc setuid root Xorg server wrapper I gather that Riccardo also runs Debian SID. Perhaps there is some interaction between d3ccc9781560, CONFIG_USER_NS and the SUID wrapper... Does your Xorg installation use --enable-suid-wrapper, Andreas?
Re: Fwd: Fwd: X stopped working with 5.14 on iBook
Hi Christopher, After many builds and tests, Stan and I were able to determine that this regression only affects builds with CONFIG_USER_NS=y. That is, d3ccc9781560 + CONFIG_USER_NS=y --> fail d3ccc9781560 + CONFIG_USER_NS=n --> okay d3ccc9781560~ + CONFIG_USER_NS=y --> okay d3ccc9781560~ + CONFIG_USER_NS=n --> okay Stan also tested a PowerMac G3 system and found that the regression is not present there. Thus far, only PowerMac G4 systems are known to be affected (Stan's Cube and Riccardo's PowerBook). I asked Stan to try v5.15-rc after reverting commit d3ccc9781560. Unexpectedly, this build had the same issue. So, it appears there are multiple bad commits that produce this Xorg failure, of which d3ccc9781560 is just the first. But there's no easy way to identify the other bad commits using bisection. So I've addressed this message to you. Can you help fix this regression? Regards, Finn On Fri, 22 Oct 2021, Christophe Leroy wrote: > ... > > > > Forwarded Message > > Subject: Fwd: X stopped working with 5.14 on iBook > > Date: Fri, 22 Oct 2021 11:35:21 -0600 > > From: Stan Johnson > > To: Christopher M. Riedl > > CC: Finn Thain > > > > Hello Christopher Riedl, > > > > Please see the message below, in which a git bisect identifies a commit > > which may have stopped X from working on some PowerPC G4 systems > > (specifically the G4 PowerBook and Cube, possibly others). > > > > I'm not sure how to proceed with further tests. If the identified commit > > could not have caused the problem, then further testing may be needed. > > Please let me know if you need any additional information. > > > > Hopefully your e-mail filter will allow messages from yahoo.com addresses. > > > > thanks for your help > > > > -Stan Johnson > > > > Forwarded Message > > Subject: Re: X stopped working with 5.14 on iBook > > Date: Fri, 22 Oct 2021 11:25:14 -0600 > > From: Stan Johnson > > To: debian-powe...@lists.debian.org > > CC: Riccardo Mottola > > > > On 10/14/21 9:21 PM, Stan Johnson wrote: > > > ... > > > Debian's 5.10.0-8 config file works (as expected) with Debian's 5.10.0-8 > > > kernel source. > > > ... > > > X works with 5.14 using a tuned config file derived from 5.13 testing. > > > ... > > > > Update: > > > > The issue originally reported by Riccardo Mottola was that X wasn't > > working on a PowerBook G4 using Debian's default > > vmlinux-5.14.0-2-powerpc kernel. I was able to confirm that the X > > failure also occurs on a G4 Cube. My G4 Cube has Debian SID, > > sysvinit-core, Xfce and wdm installed. To test whether X works, I > > disabled wdm, then I log in at the text console and run "startx". When X > > fails, the screen goes blank and the backlight stays on; when X works, > > the normal desktop comes up. > > > > X works in mainline v5.12 built using a config file based on Debian's > > config-5.10.0-8-powerpc. > > > > X fails in mainline v5.13 built using a config file based on Debian's > > config-5.10.0-8-powerpc. > > > > With much help and advice from Finn Thain, I was able to run a bisect > > using a config file based on Debian's config-5.10.0-8-powerpc, with > > v5.12 "good" and v5.13 "bad". > > > > $ git reset --hard > > HEAD is now at 62fb9874f5da Linux 5.13 > > $ git bisect start v5.13 > > Updating files: 100% (12992/12992), done. > > Previous HEAD position was 62fb9874f5da Linux 5.13 > > HEAD is now at 9f4ad9e425a1 Linux 5.12 > > $ git bisect bad v5.13 > > $ git bisect good v5.12 > > Bisecting: 8739 revisions left to test after this (roughly 13 steps) > > > 85f3f17b5db2dd9f8a094a0ddc66135afd22] Merge branch 'md-fixes' of > > https://git.kernel.org/pub/scm/linux/kernel/git/song/md into block-5.13 > > > > After the bisect, git reports this: > > > > -- > > > > d3ccc9781560af051554017c702631560bdc0811 is the first bad commit > > commit d3ccc9781560af051554017c702631560bdc0811 > > Author: Christopher M. Riedl > > Date: Fri Feb 26 19:12:59 2021 -0600 > > > > powerpc/signal: Use __get_user() to copy sigset_t > > > > Usually sigset_t is exactly 8B which is a "trivial" size and does not > > warrant using __copy_from_user(). Use __get_user() directly in > > anticipation of future work to remove the trivial size optimizations > > from __copy_from_user(). > > > > The ppc32 implementation of get
[PATCH] powerpc/tau: Add 'static' storage qualifier to 'tau_work' definition
This patch prevents the following sparse warning. arch/powerpc/kernel/tau_6xx.c:199:1: sparse: sparse: symbol 'tau_work' was not declared. Should it be static? Reported-by: kernel test robot Signed-off-by: Finn Thain --- arch/powerpc/kernel/tau_6xx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/tau_6xx.c b/arch/powerpc/kernel/tau_6xx.c index b9a047d92ec0..8e83d19fe8fa 100644 --- a/arch/powerpc/kernel/tau_6xx.c +++ b/arch/powerpc/kernel/tau_6xx.c @@ -164,7 +164,7 @@ static void tau_work_func(struct work_struct *work) queue_work(tau_workq, work); } -DECLARE_WORK(tau_work, tau_work_func); +static DECLARE_WORK(tau_work, tau_work_func); /* * setup the TAU -- 2.26.3
Re: Debian SID kernel doesn't boot on PowerBook 3400c
On Fri, 6 Aug 2021, Stan Johnson wrote:
> $ egrep '(CONFIG_PPC_KUAP|CONFIG_VMAP_STACK)' .config
> CONFIG_PPC_KUAP=y
> CONFIG_PPC_KUAP_DEBUG=y
> CONFIG_VMAP_STACK=y
> $ strings vmlinux | fgrep "Linux version"
> Linux version 5.13.0-pmac-4-g63e3756d1bd ...
> $ cp vmlinux ../vmlinux-5.13.0-pmac-4-g63e3756d1bd-1
>
> 1) PB 3400c
> vmlinux-5.13.0-pmac-4-g63e3756d1bd-1
> Boots, no errors logging in at (text) fb console. Logging in via ssh and
> running "ls -Rail /usr/include" generated errors (and a hung ssh
> session). Once errors started, they repeated for almost every command.
> See pb3400c-63e3756d1bdf-1.txt.
>
> 2) Wallstreet
> vmlinux-5.13.0-pmac-4-g63e3756d1bd-1
> X login failed, there were errors ("Oops: Kernel access of bad area",
> "Oops: Exception in kernel mode"). Logging in via SSH, there were no
> additional errors after running "ls -Rail /usr/include" -- the errors
> did not escalate as they did on the PB 3400.
> See Wallstreet-63e3756d1bdf-1.txt.
>
...
> $ egrep '(CONFIG_PPC_KUAP|CONFIG_VMAP_STACK)' .config
> CONFIG_PPC_KUAP=y
> CONFIG_PPC_KUAP_DEBUG=y
> # CONFIG_VMAP_STACK is not set
> $ strings vmlinux | fgrep "Linux version"
> Linux version 5.13.0-pmac-4-g63e3756d1bd ...
> $ cp vmlinux ../vmlinux-5.13.0-pmac-4-g63e3756d1bd-2
>
> 3) PB 3400c
> vmlinux-5.13.0-pmac-4-g63e3756d1bd-2
> Filesystem was corrupt from the previous test (probably from all the
> errors during shutdown). After fixing the filesystem:
> Boots, no errors logging in at (text) fb console. Logging in via ssh and
> running "ls -Rail /usr/include" generated a few errors. There didn't
> seem to be as many errors as in the previous test, there were a few
> errors during shutdown but the shutdown was otherwise normal.
> See pb3400c-63e3756d1bdf-2.txt.
>
> 4) Wallstreet
> vmlinux-5.13.0-pmac-4-g63e3756d1bd-2
> X login worked, and there were no errors. There were no errors during
> ssh access.
> See Wallstreet-63e3756d1bdf-2.txt.
>
Thanks for collecting these results, Stan. Do you think that the
successful result from test 4) could have been just chance?
It appears that the bug affecting the Powerbook 3400 is unaffected by
CONFIG_VMAP_STACK.
Whereas the bug affecting the Powerbook G3 disappears when
CONFIG_VMAP_STACK is disabled (assuming the result from 4 is reliable).
Either way, these results reiterate that "Oops: Kernel access of bad area,
sig: 11" was not entirely resolved by "powerpc/32s: Fix napping restore in
data storage interrupt (DSI)".
Re: Debian SID kernel doesn't boot on PowerBook 3400c
On Fri, 6 Aug 2021, Christophe Leroy wrote: > > I have cooked a tentative fix for that KUAP stuff. > Could you try the branch 'bugtest' at https://github.com/chleroy/linux.git > Thanks, Christophe. Stan, please test the following build. $ git remote add chleroy-linux https://github.com/chleroy/linux.git -f -t bugtest ... $ git checkout chleroy-linux/bugtest HEAD is now at 63e3756d1bdf powerpc/interrupts: Also perform KUAP/KUEP lock and usertime accounting on NMI $ cp ../dot-config-powermac-5.13 .config $ scripts/config -e CONFIG_PPC_KUAP -e CONFIG_PPC_KUAP_DEBUG -e CONFIG_VMAP_STACK $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -j4 clean olddefconfig vmlinux $ egrep "CONFIG_PPC_KUAP|CONFIG_VMAP_STACK" .config $ strings vmlinux |grep "Linux version" If that kernel produces errors, I'd try a second build as well: $ scripts/config -e CONFIG_PPC_KUAP -e CONFIG_PPC_KUAP_DEBUG -d CONFIG_VMAP_STACK $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -j4 clean olddefconfig vmlinux $ egrep "CONFIG_PPC_KUAP|CONFIG_VMAP_STACK" .config $ strings vmlinux |grep "Linux version" Please boot using the same kernel parameters as last time and capture the serial console logs. In case we're still dealing with intermittent bugs it might be necessary to repeat these tests so I suggest you retain the vmlinux files.
Re: Debian SID kernel doesn't boot on PowerBook 3400c
On Fri, 6 Aug 2021, Christophe Leroy wrote:
> > > > >
> > > > > Can you check if they DO NOT happen at preceding commit c16728835~
> > > > >
> > >
> > > $ git checkout c16728835~
> > > Previous HEAD position was c16728835eec powerpc/32: Manage KUAP in C
> > > HEAD is now at 0b45359aa2df powerpc/8xx: Create C version of kuap
> > > save/restore/check helpers
> > > $ git am ../message.mbox
> > > warning: Patch sent with format=flowed; space at the end of lines might be
> > > lost.
> > > Applying: powerpc/32: Dismantle EXC_XFER_STD/LITE/TEMPLATE
> > > $ cp ../dot-config-powermac-5.13 .config
> > > $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -j4 clean
> > > olddefconfig vmlinux
> > >
> > > Linux version 5.12.0-rc3-pmac-00077-gc9f6e8dd045
> > >
> > > 3) PB 3400c
> > > Hangs at boot (Mac OS screen)
> > >
> > > 4) Wallstreet
> > > X fails, errors in console log (different than test 2), see
> > > Wallstreet_console-2.txt.
> > >
> >
> > This log shows that the errors "xfce4-session[1775]: bus error (7)" and
> > "kernel BUG at arch/powerpc/kernel/interrupt.c:49!" happen prior to commit
> > c16728835eec ("powerpc/32: Manage KUAP in C").
>
> As mentionned by Nic, this is due to r11 being cloberred. For the time being
> the only r11 clobber identified is the one I have provided a fix for. I'm
> wondering whether it was applied for all further tests or not.
>
Your fix was applied to this build with "git am ../message.mbox".
> ...
> > >
> > > >
> > > > > Could you test with CONFIG_PPC_KUAP and CONFIG_PPC_KUAP_DEBUG
> > > ...
> > >
> > > $scripts/config -e CONFIG_PPC_KUAP
> > > $ scripts/config -e CONFIG_PPC_KUAP_DEBUG
> > > $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -j4 clean
> > > olddefconfig vmlinux
> > > $ grep CONFIG_PPC_KUAP .config
> > > CONFIG_PPC_KUAP=y
> > > CONFIG_PPC_KUAP_DEBUG=y
> > >
> > > Linux version 5.12.0-rc3-pmac-00078-g5cac2bc3752
> > >
> > > 9) PB 3400c
> > > Hangs at boot (Mac OS screen)
> > >
> > > 10) Wallstreet
> > > X failed at first login, worked at second login, one error in console
> > > log ("BUG: Unable to handle kernel instruction fetch"), see
> > > Wallstreet_console-5.txt.
> > >
> >
> > One might expect to see "Kernel attempted to write user page (b3399774) -
> > exploit attempt?" again here (see c16728835eec build above) but instead
> > this log says "Oops: Kernel access of bad area, sig: 11".
>
> Maybe the test should be done a second time. As r11 is garbage it may or
> may not be a user address. If it is a user address the we get "Kernel
> attempted to write user page". If it is a random kernel address, we
> likely get "Kernel access of bad area" instead.
>
Your fix was applied here also.
Re: Debian SID kernel doesn't boot on PowerBook 3400c
(Christophe, you've seen some of this before, however there are new
results added at the end. I've Cc'd the mailing lists this time.)
On Wed, 4 Aug 2021, Stan Johnson wrote:
> On 8/4/21 8:41 PM, Finn Thain wrote:
>
> >
> > $ curl
> > https://lore.kernel.org/lkml/9b64dde3-6ebd-b446-41d9-61e8cb0d8...@csgroup.eu/raw
> > ../message.mbox
> ok
>
> $ sha1 ../message.mbox
> SHA1 (../message.mbox) = 436ce0adf893c46c84c54607f73c838897caeeea
>
> >
> > On Wed, 4 Aug 2021, Christophe Leroy wrote:
> >
> >> Can you check if they happen at commit c16728835
> >>
>
> $ git checkout c16728835eec
> Checking out files: 100% (20728/20728), done.
> Note: checking out 'c16728835eec'.
>
> You are in 'detached HEAD' state. You can look around, make experimental
> changes and commit them, and you can discard any commits you make in this
> state without impacting any branches by performing another checkout.
>
> If you want to create a new branch to retain commits you create, you may
> do so (now or later) by using -b with the checkout command again. Example:
>
> git checkout -b
>
> HEAD is now at c16728835eec powerpc/32: Manage KUAP in C
> $ git am ../message.mbox
> warning: Patch sent with format=flowed; space at the end of lines might be
> lost.
> Applying: powerpc/32: Dismantle EXC_XFER_STD/LITE/TEMPLATE
> $ cp ../dot-config-powermac-5.13 .config
> $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -j4 clean olddefconfig
> vmlinux
> $ strings vmlinux | fgrep 'Linux version'
> Linux version 5.12.0-rc3-pmac-00078-geb51c431b81 (johnson@ThinkPad)
> (powerpc-linux-gnu-gcc (Debian 8.3.0-2) 8.3.0, GNU ld (GNU Binutils for
> Debian) 2.31.1) #1 SMP Wed Aug 4 21:50:47 MDT 2021
>
> 1) PB 3400c
> Hangs at boot (Mac OS screen), no serial console output
>
> 2) Wallstreet
> X fails, errors ("Kernel attempted to write user page", "BUG: Unable to
> handle kernel instruction fetch"), see Wallstreet_console-1.txt.
>
The log shows that the error "Kernel attempted to write user page
(b3399774) - exploit attempt?" happens after commit c16728835eec
("powerpc/32: Manage KUAP in C").
> >>
> >> Can you check if they DO NOT happen at preceding commit c16728835~
> >>
>
> $ git checkout c16728835~
> Previous HEAD position was c16728835eec powerpc/32: Manage KUAP in C
> HEAD is now at 0b45359aa2df powerpc/8xx: Create C version of kuap
> save/restore/check helpers
> $ git am ../message.mbox
> warning: Patch sent with format=flowed; space at the end of lines might be
> lost.
> Applying: powerpc/32: Dismantle EXC_XFER_STD/LITE/TEMPLATE
> $ cp ../dot-config-powermac-5.13 .config
> $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -j4 clean olddefconfig
> vmlinux
>
> Linux version 5.12.0-rc3-pmac-00077-gc9f6e8dd045
>
> 3) PB 3400c
> Hangs at boot (Mac OS screen)
>
> 4) Wallstreet
> X fails, errors in console log (different than test 2), see
> Wallstreet_console-2.txt.
>
This log shows that the errors "xfce4-session[1775]: bus error (7)" and
"kernel BUG at arch/powerpc/kernel/interrupt.c:49!" happen prior to commit
c16728835eec ("powerpc/32: Manage KUAP in C").
>
> $ git checkout 0b45359aa2df
> ...
> HEAD is now at 0b45359aa2df powerpc/8xx: Create C version of kuap
> save/restore/check helpers
> $ git am ../message.mbox
> warning: Patch sent with format=flowed; space at the end of lines might be
> lost.
> Applying: powerpc/32: Dismantle EXC_XFER_STD/LITE/TEMPLATE
> $ cp ../dot-config-powermac-5.13 .config
> $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -j4 clean olddefconfig
> vmlinux
>
> Linux version 5.12.0-rc3-pmac-00077-ge06b29ce146
>
> 5) PB 3400c
> Hangs at boot (Mac OS screen)
>
> 6) Wallstreet
> X failed (X login succeeded, but setting up desktop failed), errors in
> console log, see Wallstreet_console-3.txt.
>
(No need for those two tests: it's exactly the same code and almost the
same failure modes: "kernel BUG at arch/powerpc/kernel/interrupt.c:50".)
On Thu, 5 Aug 2021, Stan Johnson wrote:
> On 8/5/21 12:47 AM, Finn Thain wrote:
>
> > On Wed, 4 Aug 2021, Christophe Leroy wrote:
> >
> >> Could you test without CONFIG_PPC_KUAP
> ...
>
> $ git checkout c16728835eec
> ...
> HEAD is now at c16728835eec powerpc/32: Manage KUAP in C
> $ git am ../message.mbox
> warning: Patch sent with format=flowed; space at the end of lines might be
> lost.
> Applying: powerpc/32: Dismantle EXC_XFER_STD/LITE/TEMPLATE
> $ cp ../dot-config-powermac-5.13 .config
> $ scripts/config -d CONFIG_PPC_KUAP
> $ make ARCH=powerpc C
Re: [PATCH] powerpc/32s: Fix napping restore in data storage interrupt (DSI)
On Wed, 4 Aug 2021, Christophe Leroy wrote:
>
> This patch is related to the bisect you did that pointed to 4c0104a83fc3
> ("powerpc/32: Dismantle EXC_XFER_STD/LITE/TEMPLATE")
>
> I think maybe the starting point should be to (manually) apply the patch
> on top of that commit in order to check that the bug to leaded to
> pointing that commit as 'first bad commit' is now gone.
>
Stan has now confirmed this. He applied this patch on top of 4c0104a83fc3,
and it did indeed resolve the bug that 'git bisect' isolated [1]. Thanks
Christophe.
[1]
https://lore.kernel.org/lkml/666e3ab4-372-27c2-4621-7cc393375...@linux-m68k.org/
Re: [PATCH v3 31/41] powerpc/32: Dismantle EXC_XFER_STD/LITE/TEMPLATE
On Tue, 3 Aug 2021, Stan Johnson wrote:
> Attached you will find the following six files:
>
> 1) config-5.13-patched_VMAP.txt
> 2) config-5.13-patched_NO_VMAP.txt
> 3) pb3400c-console-5.13-patched_VMAP.txt (using config 1)
> 4) pb3400c-console-5.13-patched_NO_VMAP.txt (using config 2)
> 5) ws-console-5.13-patched_VMAP.txt (using config 1)
> 6) ws-console-5.13-patched_NO_VMAP.txt (using config 2)
>
Thanks!
> The command lines in BootX were as follows:
>
> PB 3400c:
> root=/dev/sda13 console=ttyS0 video=chips65550:vmode:14,cmode:16
>
> Wallstreet:
> root=/dev/sda12 console=ttyS0 video=ofonly
>
> Notes:
>
> For 3), the patch seems to have fixed the "hang-at-boot" at the Mac
> OS screen for the PB 3400c.
I doubt that. I suspect that this is an unrelated failure that only
affects the Powerbook 3400 and only intermittently. I say that because
you've also observed this failure in v5.11.
So we should probably ignore this early-boot failure for the moment. Stan,
if it happens again, please reboot and retry. That may allow us to make
progress on the other bugs.
> After a successful boot, I didn't see any errors until I accessed the
> system via ssh. In an ssh window, I entered "dmesg" (no errors) followed
> by "ls -Rail /usr/bin", and while that was running, the errors appeared.
Since Stan has a yahoo email address that isn't allowed past the spam
filter, I'll paste that portion of the console log he sent --
Kernel attempted to write user page (78a930) - exploit attempt? (uid: 1000)
[ cut here ]
Bug: Write fault blocked by KUAP!
WARNING: CPU: 0 PID: 1619 at arch/powerpc/mm/fault.c:230
do_page_fault+0x484/0x720
Modules linked in:
CPU: 0 PID: 1619 Comm: sshd Not tainted 5.13.0-pmac-VMAP #10
NIP: c001b780 LR: c001b780 CTR:
REGS: cb981bc0 TRAP: 0700 Not tainted (5.13.0-pmac-VMAP)
MSR: 00021032 CR: 24942424 XER:
GPR00: c001b780 cb981c80 c151c1e0 0021 3bff 085b 0027 c8eb644c
GPR08: 0023 24942424 0076f8c8 000186a0
GPR16: afab5544 afab5540 afab553c afab5538 afab5534 afab5530 0004 0078a934
GPR24: 0078a970 0200 c1497b60 0078a930 0300 cb981cc0
NIP [c001b780] do_page_fault+0x484/0x720
LR [c001b780] do_page_fault+0x484/0x720
Call Trace:
[cb981c80] [c001b780] do_page_fault+0x484/0x720 (unreliable)
[cb981cb0] [c000424c] DataAccess_virt+0xd4/0xe4
--- interrupt: 300 at __copy_tofrom_user+0x110/0x20c
NIP: c001f9bc LR: c0172b04 CTR: 0001
REGS: cb981cc0 TRAP: 0300 Not tainted (5.13.0-pmac-VMAP)
MSR: 9032 CR: 442444e8 XER: 2000
DAR: 0078a930 DSISR: 0a00
GPR00: cb981d80 c151c1e0 0078a930 cb981db8 0004 0078a92c 0100
GPR08: 0122 10c279a1 1000 c1800034 242444e2 0076f8c8 000186a0
GPR16: afab5544 afab5540 afab553c afab5538 afab5534 afab5530 0004 0078a934
GPR24: 0078a970 0078a930 cb981dac cb981dac 0001 0004
NIP [c001f9bc] __copy_tofrom_user+0x110/0x20c
LR [c0172b04] core_sys_select+0x3e8/0x594
--- interrupt: 300
[cb981d80] [c0172960] core_sys_select+0x244/0x594 (unreliable)
[cb981ee0] [c0172d98] kern_select+0xe8/0x158
[cb981f30] [c001604c] ret_from_syscall+0x0/0x28
--- interrupt: c00 at 0xa7a4f388
NIP: a7a4f388 LR: a7a4f35c CTR:
REGS: cb981f40 TRAP: 0c00 Not tainted (5.13.0-pmac-VMAP)
MSR: d032 CR: 240044e2 XER: 2000
GPR00: 008e afab54e0 a73cc7d0 000c 0078a930 0078a970
GPR08: 0004 a79e45b0 28004462 0076f8c8 000186a0
GPR16: afab5544 afab5540 afab553c afab5538 afab5534 afab5530 0004 00770490
GPR24: afab552f 0004 0078a930 00771734 a7b2fff4 00798cb0
NIP [a7a4f388] 0xa7a4f388
LR [a7a4f35c] 0xa7a4f35c
--- interrupt: c00
Instruction dump:
3884aa30 3863012c 4807685d 807f0080 48042e41 2f83 419e0148 3c80c079
3c60c076 38841b6c 38630174 4801f701 <0fe0> 386b 4bfffe30 3c80c06b
---[ end trace c6ec12d4725e6f89 ]---
> I'll enter the same commands for the other three boots. It may be
> important that I didn't see errors until there was significant network
> access.
>
> For 4), the PB 3400c also booted normally. Errors started after
> logging in via ssh when I entered "dmesg". To be consistent with the
> first test, I followed that with "ls -Rail /usr/bin" and saw more
> errors. A normal reboot ("shutdown -r now") caused even more errors.
>
Here's the relevant portion of that log:
Kernel attempted to write user page (ba3bc0) - exploit attempt? (uid: 1000)
[ cut here ]
Bug: Write fault blocked by KUAP!
WARNING: CPU: 0 PID: 1609 at arch/powerpc/mm/fault.c:230
do_page_fault+0x484/0x720
Modules linked in:
CPU: 0 PID: 1609 Comm: bash Not tainted 5.13.0-pmac-NO_VMAP #11
NIP: c001b780 LR: c001b780 CTR:
REGS: c3c5bba0 TRAP: 0700 Not tainted (5.13.0-pmac-NO_VMAP)
MSR: 00021032 CR: 24442424 XER:
GPR00: c001b780 c3c5bc60 c3842ca0 0021 3bff
Re: [PATCH] powerpc/32s: Fix napping restore in data storage interrupt (DSI)
On Tue, 3 Aug 2021, Christophe Leroy wrote:
> When a DSI (Data Storage Interrupt) is taken while in NAP mode, r11
> doesn't survive the call to power_save_ppc32_restore().
>
> So use r1 instead of r11 as they both contain the virtual stack pointer
> at that point.
>
> Reported-by: Finn Thain
> Fixes: 4c0104a83fc3 ("powerpc/32: Dismantle EXC_XFER_STD/LITE/TEMPLATE")
Regarding that 'Fixes' tag, this patch has not fixed the failure below,
unfortunately. But there appears to be several bugs in play here. Can you
tell us which failure mode is associated with the bug addressed by this
patch?
[ cut here ]
kernel BUG at arch/powerpc/kernel/interrupt.c:49!
Oops: Exception in kernel mode, sig: 5 [#1]
BE PAGE_SIZE=4K MMU=Hash SMP NR_CPUS=2 PowerMac
Modules linked in:
CPU: 0 PID: 1859 Comm: xfce4-session Not tainted 5.13.0-pmac-VMAP #10
NIP: c0011474 LR: c0011464 CTR:
REGS: e2f75e40 TRAP: 0700 Not tainted (5.13.0-pmac-VMAP)
MSR: 00021032 CR: 2400446c XER: 2000
GPR00: c001604c e2f75f00 ca284a60 a5205eb0 0008 0020
GPR08: ffc0 0001 501200d9 ce030005 ca285010 00c1f778
GPR16: 00945b20 009402f8 0001 a6b87550 a51fd000 afb73220 a6b22c78 a6a6aecc
GPR24: ffc0 0020 0008 a5205eb0 e2f75f40 00ae
NIP [c0011474] system_call_exception+0x60/0x164
LR [c0011464] system_call_exception+0x50/0x164
Call Trace:
[e2f75f00] [9000] 0x9000 (unreliable)
[e2f75f30] [c001604c] ret_from_syscall+0x0/0x28
--- interrupt: c00 at 0xa69d6cb0
NIP: a69d6cb0 LR: a69d6c3c CTR:
REGS: e2f75f40 TRAP: 0c00 Not tainted (5.13.0-pmac-VMAP)
MSR: d032 CR: 2400446c XER: 2000
GPR00: 00ae a5205de0 a5687ca0 a5205eb0 0008 0020
GPR08: ffc0 401201ea 401200d9 c158f230 00c1f778
GPR16: 00945b20 009402f8 0001 a6b87550 a51fd000 afb73220 a6b22c78 a6a6aecc
GPR24: afb72fc8 0001 a5205f30 afb733dc a6b85ff4 a5205eb0
NIP [a69d6cb0] 0xa69d6cb0
LR [a69d6c3c] 0xa69d6c3c
--- interrupt: c00
Instruction dump:
7cdb3378 93810020 7cbc2b78 93a10024 7c9d2378 93e1002c 7d3f4b78 4800d629
817e0084 931e0088 69690002 5529fffe <0f09> 69694000 552997fe 0f09
---[ end trace c66c6c3c44806276 ]---
Re: Debian SID kernel doesn't boot on PowerBook 3400c
On Tue, 3 Aug 2021, Christophe Leroy wrote: > > Looks like the memory errors are linked to KUAP (Kernel Userspace Access > Protection). Based on the places the problems happen, I don't think > there are any invalid access, so there must be something wrong in the > KUAP logic, probably linked to some interrupts happenning in kernel mode > while the KUAP window is opened. And because is not selected by default > on book3s/32 until 5.14, probably nobody ever tested it in a real > environment before you. > > I think the issue may be linked to commit > https://github.com/linuxppc/linux/commit/c16728835 which happened > between 5.12 and 5.13. The messages, "Kernel attempted to write user page (c6207c) - exploit attempt? (uid: 0)", appear in the console logs generated by v5.13. Those logs come from the Powerbook G3 discussion in the other thread. Could that be the same bug?
Re: Debian SID kernel doesn't boot on PowerBook 3400c
On Tue, 3 Aug 2021, Stan Johnson wrote: > > I'm not sure of the issue you are referencing. If it's the Wallstreet > issue, I believe we were waiting to hear back from you regarding the > memory errors that crop up with CONFIG_VMAP_STACK=y and mem >464M. > Finn, if that is not correct, please let me know. > No, it's not correct. I sent a message dated 3 Aug 2021 with a patch from Christophe. I also sent (privately) a message with instructions for testing that patch. I will resend these now.
Re: [PATCH v3 31/41] powerpc/32: Dismantle EXC_XFER_STD/LITE/TEMPLATE
On Mon, 2 Aug 2021, LEROY Christophe wrote:
> Le 01/08/2021 à 03:21, Finn Thain a écrit :
> > On Sat, 31 Jul 2021, Christophe Leroy wrote:
> >
> > > >
> > > > Stan Johnson contacted me about a regression in mainline that he
> > > > observed on his G3 Powerbooks. Using 'git bisect' we determined that
> > > > this patch was the cause of the regression, i.e. commit 4c0104a83fc3
> > > > ("powerpc/32: Dismantle EXC_XFER_STD/LITE/TEMPLATE").
> > > >
> > > > When testing 4c0104a83fc and all subsequent builds, various user
> > > > processes were liable to segfault. Here is the console log that Stan
> > > > provided:
> > >
> > > Hi, i will be able to look at that more in details next week, however I
> > > have a few preliminary qurstions.
> > >
> > > Can you reliabily reproduce the problem with the said commit, and can
> > > you reliabily run without problem with the parent commit ?
> >
> > Yes and yes. (I already asked Stan to establish those things before I
> > contacted the list.)
>
> I think I found the problem with that commit. Can you retry with the following
> change:
>
> diff --git a/arch/powerpc/kernel/head_book3s_32.S
> b/arch/powerpc/kernel/head_book3s_32.S
> index 0a3d7d4a9ec4..a294103a91a1 100644
> --- a/arch/powerpc/kernel/head_book3s_32.S
> +++ b/arch/powerpc/kernel/head_book3s_32.S
> @@ -299,7 +299,7 @@ ALT_MMU_FTR_SECTION_END_IFSET(MMU_FTR_HPTE_TABLE)
> EXCEPTION_PROLOG_1
> EXCEPTION_PROLOG_2 0x300 DataAccess handle_dar_dsisr=1
> prepare_transfer_to_handler
> - lwz r5, _DSISR(r11)
> + lwz r5, _DSISR(r1)
> andis. r0, r5, DSISR_DABRMATCH@h
> bne-1f
> bl do_page_fault
That patch doesn't apply to mainline. This version might help.
diff --git a/arch/powerpc/kernel/head_book3s_32.S
b/arch/powerpc/kernel/head_book3s_32.S
index 764edd860ed4..68e5c0a7e99d 100644
--- a/arch/powerpc/kernel/head_book3s_32.S
+++ b/arch/powerpc/kernel/head_book3s_32.S
@@ -300,7 +300,7 @@ ALT_MMU_FTR_SECTION_END_IFSET(MMU_FTR_HPTE_TABLE)
EXCEPTION_PROLOG_1
EXCEPTION_PROLOG_2 INTERRUPT_DATA_STORAGE DataAccess handle_dar_dsisr=1
prepare_transfer_to_handler
- lwz r5, _DSISR(r11)
+ lwz r5, _DSISR(r1)
andis. r0, r5, DSISR_DABRMATCH@h
bne-1f
bl do_page_fault
Re: [PATCH v3 31/41] powerpc/32: Dismantle EXC_XFER_STD/LITE/TEMPLATE
On Sun, 1 Aug 2021, Stan Johnson wrote: > > > >> Could you try without CONFIG_VMAP_STACK > >> > > > > Stan, would you please test the following build: > > > > $ git checkout v5.13 > > $ cp ../dot-config-powermac-5.13 .config > > $ scripts/config -d CONFIG_VMAP_STACK > > $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -j4 clean olddefconfig > > vmlinux > > > Please see the attached serial console log (four boots): > 1) v5.13-mac, CONFIG_VMAP_STACK=y, mem=512M (fails) > 2) v5.13-mac, CONFIG_VMAP_STACK=y, mem=384M (works) > 3) v5.13-mac, CONFIG_VMAP_STACK=n, mem=512M (works) > 4) v5.13-mac, CONFIG_VMAP_STACK=n, mem=384M (works) > > My apologies if the extra boots were not needed (due to the time > difference, I'm trying to anticipate future requests). > > Cutting and pasting Finn's commands above builds a new kernel > (5.13.0-pmac) with the following in .config: > > $ fgrep VMAP .config > CONFIG_HAVE_ARCH_VMAP_STACK=y > CONFIG_VMAP_STACK=y > That's odd. It works correctly here: $ cp ../dot-config-powermac-5.13 .config $ grep CONFIG_VMAP_STACK .config CONFIG_VMAP_STACK=y $ scripts/config -d CONFIG_VMAP_STACK $ grep CONFIG_VMAP_STACK .config # CONFIG_VMAP_STACK is not set Anyway, I see that you resolved the problem: > ... > > $ fgrep VMAP .config > CONFIG_HAVE_ARCH_VMAP_STACK=y > # CONFIG_VMAP_STACK is not set > > 3) Same as 1 (512M) but with CONFIG_VMAP_STACK not set in v5.13. > Everything works (no problems with X, no errors logged). > > 4) Same as 2 (384M) but with CONFIG_VMAP_STACK not set in v5.13. > Everything works (no problems with X, no errors logged). > Thanks for collecting those results. It appears that Christophe was right. Disabling CONFIG_VMAP_STACK avoids the crashes in v5.13. (Enabling CONFIG_VMAP_STACK worked fine in v5.12.)
Re: [PATCH v3 31/41] powerpc/32: Dismantle EXC_XFER_STD/LITE/TEMPLATE
On Sat, 31 Jul 2021, Christophe Leroy wrote:
> >
> > Stan Johnson contacted me about a regression in mainline that he
> > observed on his G3 Powerbooks. Using 'git bisect' we determined that
> > this patch was the cause of the regression, i.e. commit 4c0104a83fc3
> > ("powerpc/32: Dismantle EXC_XFER_STD/LITE/TEMPLATE").
> >
> > When testing 4c0104a83fc and all subsequent builds, various user
> > processes were liable to segfault. Here is the console log that Stan
> > provided:
>
> Hi, i will be able to look at that more in details next week, however I
> have a few preliminary qurstions.
>
> Can you reliabily reproduce the problem with the said commit, and can
> you reliabily run without problem with the parent commit ?
Yes and yes. (I already asked Stan to establish those things before I
contacted the list.)
> I'm asking because at first look that commit doesn't bring any
> functionnal change.
>
> Coukd you provide your .config ?
>
Please see attached. My understanding is that all of Stan's builds were
performed like this:
$ cp ../dot-config-powermac-5.13 .config
$ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -j4 clean olddefconfig
vmlinux
> Could you try without CONFIG_VMAP_STACK
>
Stan, would you please test the following build:
$ git checkout v5.13
$ cp ../dot-config-powermac-5.13 .config
$ scripts/config -d CONFIG_VMAP_STACK
$ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -j4 clean olddefconfig
vmlinux#
# Automatically generated file; DO NOT EDIT.
# Linux/powerpc 5.13.0 Kernel Configuration
#
CONFIG_CC_VERSION_TEXT="powerpc-linux-gnu-gcc (btc) 6.4.0"
CONFIG_CC_IS_GCC=y
CONFIG_GCC_VERSION=60400
CONFIG_CLANG_VERSION=0
CONFIG_AS_IS_GNU=y
CONFIG_AS_VERSION=22800
CONFIG_LD_IS_BFD=y
CONFIG_LD_VERSION=22800
CONFIG_LLD_VERSION=0
CONFIG_CC_CAN_LINK=y
CONFIG_CC_CAN_LINK_STATIC=y
CONFIG_CC_HAS_ASM_GOTO=y
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_TABLE_SORT=y
CONFIG_THREAD_INFO_IN_TASK=y
#
# General setup
#
CONFIG_INIT_ENV_ARG_LIMIT=32
# CONFIG_COMPILE_TEST is not set
CONFIG_LOCALVERSION="-pmac"
CONFIG_LOCALVERSION_AUTO=y
CONFIG_BUILD_SALT=""
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_KERNEL_GZIP=y
# CONFIG_KERNEL_XZ is not set
CONFIG_DEFAULT_INIT=""
CONFIG_DEFAULT_HOSTNAME="(none)"
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
# CONFIG_WATCH_QUEUE is not set
# CONFIG_CROSS_MEMORY_ATTACH is not set
# CONFIG_USELIB is not set
# CONFIG_AUDIT is not set
CONFIG_HAVE_ARCH_AUDITSYSCALL=y
#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_IRQ_SHOW_LEVEL=y
CONFIG_GENERIC_IRQ_MIGRATION=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
# end of IRQ subsystem
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_ARCH_HAS_TICK_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CMOS_UPDATE=y
#
# Timers subsystem
#
CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ_COMMON=y
# CONFIG_HZ_PERIODIC is not set
CONFIG_NO_HZ_IDLE=y
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y
# end of Timers subsystem
CONFIG_BPF=y
CONFIG_HAVE_EBPF_JIT=y
#
# BPF subsystem
#
# CONFIG_BPF_SYSCALL is not set
# CONFIG_BPF_JIT is not set
# end of BPF subsystem
# CONFIG_PREEMPT_NONE is not set
CONFIG_PREEMPT_VOLUNTARY=y
# CONFIG_PREEMPT is not set
#
# CPU/Task time and stats accounting
#
CONFIG_TICK_CPU_ACCOUNTING=y
# CONFIG_VIRT_CPU_ACCOUNTING_NATIVE is not set
# CONFIG_IRQ_TIME_ACCOUNTING is not set
# CONFIG_BSD_PROCESS_ACCT is not set
# CONFIG_TASKSTATS is not set
# CONFIG_PSI is not set
# end of CPU/Task time and stats accounting
# CONFIG_CPU_ISOLATION is not set
#
# RCU Subsystem
#
CONFIG_TREE_RCU=y
# CONFIG_RCU_EXPERT is not set
CONFIG_SRCU=y
CONFIG_TREE_SRCU=y
CONFIG_RCU_STALL_COMMON=y
CONFIG_RCU_NEED_SEGCBLIST=y
# end of RCU Subsystem
# CONFIG_IKCONFIG is not set
# CONFIG_IKHEADERS is not set
CONFIG_LOG_BUF_SHIFT=15
CONFIG_LOG_CPU_MAX_BUF_SHIFT=0
CONFIG_PRINTK_SAFE_LOG_BUF_SHIFT=13
#
# Scheduler features
#
# end of Scheduler features
CONFIG_CGROUPS=y
# CONFIG_MEMCG is not set
# CONFIG_BLK_CGROUP is not set
# CONFIG_CGROUP_SCHED is not set
# CONFIG_CGROUP_PIDS is not set
# CONFIG_CGROUP_RDMA is not set
# CONFIG_CGROUP_FREEZER is not set
# CONFIG_CPUSETS is not set
# CONFIG_CGROUP_DEVICE is not set
# CONFIG_CGROUP_CPUACCT is not set
# CONFIG_CGROUP_MISC is not set
CONFIG_NAMESPACES=y
# CONFIG_UTS_NS is not set
# CONFIG_TIME_NS is not set
CONFIG_IPC_NS=y
# CONFIG_USER_NS is not set
CONFIG_PID_NS=y
CONFIG_NET_NS=y
# CONFIG_CHECKPOINT_RESTORE is not set
# CONFIG_SCHED_AUTOGROUP is not set
# CONFIG_SYSFS_DEPRECATED is not set
CONFIG_RELAY=y
CONFIG_BLK_DEV_INITRD=y
CONFIG_INITRAMFS_SOURCE=""
CONFIG_RD_GZIP=y
CONFIG_RD_BZIP2=y
CONFIG_RD_LZMA=y
CONFIG_RD_XZ=y
CONFIG_RD_LZO=y
CONFIG_RD_LZ4=y
CONFIG_RD_ZSTD=y
# CONFIG_BOOT_CONFIG is not set
CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y
# CONFIG_CC_OPTIMIZE_FOR_SIZE is not set
CONFIG_HAVE_LD_DEAD_CODE_DATA_ELIMINATION=y
CONFIG_LD_ORPHAN_WARN=y
CONFIG_SYSCTL=y
Re: [PATCH v3 31/41] powerpc/32: Dismantle EXC_XFER_STD/LITE/TEMPLATE
Hi Christophe,
On Fri, 12 Mar 2021, Christophe Leroy wrote:
> In order to get more control in exception prolog, dismantle all non
> standard exception macros, finishing with EXC_XFER_STD and EXC_XFER_LITE
> and EXC_XFER_TEMPLATE.
>
> Also remove transfer_to_handler_full and ret_from_except and
> ret_from_except_full as they are not used anymore.
>
> Last parameter of EXCEPTION() is now ignored, will be removed in a later
> patch to avoid too much churn.
>
> Signed-off-by: Christophe Leroy
> ---
> arch/powerpc/kernel/entry_32.S | 42 +---
> arch/powerpc/kernel/head_32.h| 21
> arch/powerpc/kernel/head_40x.S | 33 ---
> arch/powerpc/kernel/head_8xx.S | 12 +--
> arch/powerpc/kernel/head_book3s_32.S | 27 ++-
> arch/powerpc/kernel/head_booke.h | 49 +++-
> arch/powerpc/kernel/head_fsl_booke.S | 14 +---
> 7 files changed, 92 insertions(+), 106 deletions(-)
>
Stan Johnson contacted me about a regression in mainline that he observed
on his G3 Powerbooks. Using 'git bisect' we determined that this patch was
the cause of the regression, i.e. commit 4c0104a83fc3 ("powerpc/32:
Dismantle EXC_XFER_STD/LITE/TEMPLATE").
When testing 4c0104a83fc and all subsequent builds, various user processes
were liable to segfault. Here is the console log that Stan provided:
[0.00] printk: debug: ignoring loglevel setting.
[0.00] Total memory = 512MB; using 1024kB for hash table
[0.00] Activating Kernel Userspace Execution Prevention
[0.00] Activating Kernel Userspace Access Protection
[0.00] Linux version 5.12.0-rc3-pmac-00067-g4c0104a83fc
(johnson@ThinkPad) (powerpc-linux-gnu-gcc (Debian 8.3.0-2) 8.3.0, GNU ld (GNU
Binutils for Debian) 2.31.1) #22 SMP Fri Jul 30 12:15:00 MDT 2021
[0.00] ioremap() called early from probe_one_macio+0x130/0x268. Use
early_ioremap() instead
[0.00] Found a Gatwick mac-io controller, rev: 0, mapped at 0x(ptrval)
[0.00] ioremap() called early from probe_one_macio+0x130/0x268. Use
early_ioremap() instead
[0.00] Found a Heathrow mac-io controller, rev: 0, mapped at 0x(ptrval)
[0.00] PowerMac motherboard: PowerBook Wallstreet
[0.00] ioremap() called early from find_via_pmu+0x244/0x56c. Use
early_ioremap() instead
[0.00] PMU driver v2 initialized for PowerBook G3 Series, firmware: 0a
[0.00] Using PowerMac machine description
[0.00] printk: bootconsole [udbg0] enabled
[0.00] CPU maps initialized for 1 thread per core
[0.00] (thread shift is 0)
[0.00] -
[0.00] phys_mem_size = 0x2000
[0.00] dcache_bsize = 0x20
[0.00] icache_bsize = 0x20
[0.00] cpu_features = 0x0501a008
[0.00] possible= 0x277de14a
[0.00] always = 0x0100
[0.00] cpu_user_features = 0x8c01 0x
[0.00] mmu_features = 0x0001
[0.00] Hash_size = 0x10
[0.00] Hash_mask = 0x3fff
[0.00] -
[0.00] ioremap() called early from pmac_setup_arch+0x10c/0x294. Use
early_ioremap() instead
[0.00] nvram: OF partition at 0x1800
[0.00] nvram: XP partition at 0x1300
[0.00] nvram: NR partition at 0x1400
[0.00] Top of RAM: 0x2000, Total RAM: 0x2000
[0.00] Memory hole size: 0MB
[0.00] Zone ranges:
[0.00] DMA [mem 0x-0x1fff]
[0.00] Normal empty
[0.00] HighMem empty
[0.00] Movable zone start for each node
[0.00] Early memory node ranges
[0.00] node 0: [mem 0x-0x1fff]
[0.00] Initmem setup node 0 [mem 0x-0x1fff]
[0.00] On node 0 totalpages: 131072
[0.00] DMA zone: 1024 pages used for memmap
[0.00] DMA zone: 0 pages reserved
[0.00] DMA zone: 131072 pages, LIFO batch:31
[0.00] percpu: Embedded 13 pages/cpu s21644 r8192 d23412 u53248
[0.00] pcpu-alloc: s21644 r8192 d23412 u53248 alloc=13*4096
[0.00] pcpu-alloc: [0] 0 [0] 1
[0.00] Built 1 zonelists, mobility grouping on. Total pages: 130048
[0.00] Kernel command line: root=/dev/sda12 console=ttyS0 console=tty
printk.time earlyprintk ignore_loglevel video=ofonly
[0.00] Dentry cache hash table entries: 65536 (order: 6, 262144 bytes,
linear)
[0.00] Inode-cache hash table entries: 32768 (order: 5, 131072 bytes,
linear)
[0.00] mem auto-init: stack:off, heap alloc:off, heap free:off
[0.00] Memory: 498908K/524288K available (6756K kernel code, 352K
rwdata, 1276K rodata, 1232K init, 176K bss, 25380K reserved, 0K cma-reserved,
0K
[PATCH v2] powerpc/tau: Remove superfluous parameter in alloc_workqueue() call
This avoids an (optional) compiler warning:
arch/powerpc/kernel/tau_6xx.c: In function 'TAU_init':
arch/powerpc/kernel/tau_6xx.c:204:30: error: too many arguments for format
[-Werror=format-extra-args]
tau_workq = alloc_workqueue("tau", WQ_UNBOUND, 1, 0);
Reported-by: Naresh Kamboju
Fixes: b1c6a0a10bfa ("powerpc/tau: Convert from timer to workqueue")
Signed-off-by: Finn Thain
---
Changed since v1:
- Improved commit log message.
---
arch/powerpc/kernel/tau_6xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/tau_6xx.c b/arch/powerpc/kernel/tau_6xx.c
index 6c31af7f4fa8..b9a047d92ec0 100644
--- a/arch/powerpc/kernel/tau_6xx.c
+++ b/arch/powerpc/kernel/tau_6xx.c
@@ -201,7 +201,7 @@ static int __init TAU_init(void)
tau_int_enable = IS_ENABLED(CONFIG_TAU_INT) &&
!strcmp(cur_cpu_spec->platform, "ppc750");
- tau_workq = alloc_workqueue("tau", WQ_UNBOUND, 1, 0);
+ tau_workq = alloc_workqueue("tau", WQ_UNBOUND, 1);
if (!tau_workq)
return -ENOMEM;
--
2.26.3
[PATCH] powerpc/tau: Remove redundant parameter in alloc_workqueue() call
This avoids an (optional) compiler warning:
arch/powerpc/kernel/tau_6xx.c: In function 'TAU_init':
arch/powerpc/kernel/tau_6xx.c:204:30: error: too many arguments for format
[-Werror=format-extra-args]
tau_workq = alloc_workqueue("tau", WQ_UNBOUND, 1, 0);
Reported-by: Naresh Kamboju
Fixes: b1c6a0a10bfa ("powerpc/tau: Convert from timer to workqueue")
Signed-off-by: Finn Thain
---
arch/powerpc/kernel/tau_6xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/tau_6xx.c b/arch/powerpc/kernel/tau_6xx.c
index 6c31af7f4fa8..b9a047d92ec0 100644
--- a/arch/powerpc/kernel/tau_6xx.c
+++ b/arch/powerpc/kernel/tau_6xx.c
@@ -201,7 +201,7 @@ static int __init TAU_init(void)
tau_int_enable = IS_ENABLED(CONFIG_TAU_INT) &&
!strcmp(cur_cpu_spec->platform, "ppc750");
- tau_workq = alloc_workqueue("tau", WQ_UNBOUND, 1, 0);
+ tau_workq = alloc_workqueue("tau", WQ_UNBOUND, 1);
if (!tau_workq)
return -ENOMEM;
--
2.26.3
Re: remove the legacy ide driver
On Thu, 18 Mar 2021, Christoph Hellwig wrote: > Hi all, > > we've been trying to get rid of the legacy ide driver for a while now, > and finally scheduled a removal for 2021, which is three month old now. > > In general distros and most defconfigs have switched to libata long ago, > but there are a few exceptions. This series first switches over all > remaining defconfigs to use libata and then removes the legacy ide > driver. > > libata mostly covers all hardware supported by the legacy ide driver. > There are three mips drivers that are not supported, but the linux-mips > list could not identify any users of those. There also are two m68k > drivers that do not have libata equivalents, which might or might not > have users, so we'll need some input and possibly help from the m68k > community here. > A few months ago I wrote another patch to move some more platforms away from macide but it has not been tested yet. That is not to say you should wait. However, my patch does have some changes that are missing from your patch series, relating to ide platform devices in arch/m68k/mac/config.c. I hope to be able to test this patch before the 5.13 merge window closes.
Re: [PATCH] scsi: target/sbp: remove firewire SBP target driver
On Mon, 4 Jan 2021, Bart Van Assche wrote: > On 6/16/20 7:07 PM, Finn Thain wrote: > > On Tue, 16 Jun 2020, Bart Van Assche wrote: > >> As far as I know the sbp driver only has had one user ever and that > >> user is no longer user the sbp driver. > > > > So, you estimate the userbase at zero. Can you give a confidence > > level? Actual measurement is hard because when end users encounter > > breakage, they look for quick workarounds before they undertake post > > mortem, log collection, bug reporting, mailing list discussions, > > analysis etc. > > (replying to an e-mail from six months ago) > > Hi Finn, > > I am confident that my estimate is an accurate estimate since I have not > seen any sbp support requests, sbp bug reports nor any sbp bug fixes > since the sbp target driver has been accepted upstream. > That suggests to me that the code that you're hoping to remove 1) has no bugs, or 2) has no reported bugs, or 3) has no users at present. I am confident that your evidence does not support your conclusion (i.e. the code will never be used again). Sometimes, users only appear after the unreported bugs get fixed. I've seen it happen. > > Here's a different question: "Why remove it from the kernel tree?" > > > > If maintaining this code is a burden, is it not the kind of tax that > > all developers/users pay to all developers/users? Does this driver > > impose an unreasonably high burden for some reason? > > Yes. If anyone wants to change the interface between SCSI target core > and SCSI target drivers, all target drivers, including the sbp and FCoE > target driver have to be retested. I'm unaware of such an obligation. API changes happen often. When they do, we see good test coverage of commercially viable hardware, some best-effort testing of common hardware, and some perfunctory build testing. But that is missing the point, which was about a particular driver, not about development process. You have not shown how the target API is special, to support your claim that this driver imposes an unreasonable burden. In the interests of making forward progress in this discussion, shall we discuss the kind of SCSI Target API changes that you anticipate? > In other words, keeping unused target drivers inside the kernel tree > involves a significant maintenance burden for anyone who wants to modify > the interface between the SCSI target core and SCSI target drivers. > Keeping _any_ driver in the kernel involves a maintenance burden. There are two good ways to address that. Firstly, by improving the development process. For example, an API change is mostly mechanical work that lends itself to automated refactoring. Secondly, by involving all interested parties, so that the burden is shared. Of course, there are other ways. E.g. "don't ship code when doing so won't turn a profit". That, by the way, was the policy that gave us 10 billion Android devices (or more) that don't function with a mainline kernel. > Additionally, there is a good alternative available for the sbp driver. > Every system I know of that is equipped with a Firewire port also has an > Ethernet port. So users who want to provide SCSI target functionality on > such systems can use any SCSI transport protocol that is compatible with > Ethernet (iSCSI, iSER over soft-RoCE, SRP over soft-RoCE, ...). > Ethernet is not always an alternative. That was already discussed in this thread. But let's assume for a moment that you can migrate any and all users of this driver over to an ethernet driver. Why would the maintainers of that ethernet driver and its API accept that plan, if adding users would extend their maintenance and testing obligations? Do you think those maintainers should pay the "kind of tax that all developers/users pay to all developers/users?" > Thanks, > > Bart. >
[PATCH] MAINTAINERS: Update 68k Mac entry
Two files under drivers/macintosh are actually m68k-only. I think that patches for these files should be reviewed in the appropriate forum and merged via the appropriate tree, rather than falling to the powerpc maintainers to deal with. Update the "M68K ON APPLE MACINTOSH" section accordingly. Cc: Michael Ellerman Cc: Benjamin Herrenschmidt Cc: Joshua Thompson Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-m...@lists.linux-m68k.org Signed-off-by: Finn Thain --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 867157311dc8..e8fa0c9645d6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10322,6 +10322,8 @@ L: linux-m...@lists.linux-m68k.org S: Maintained W: http://www.mac.linux-m68k.org/ F: arch/m68k/mac/ +F: drivers/macintosh/adb-iop.c +F: drivers/macintosh/via-macii.c M68K ON HP9000/300 M: Philip Blundell -- 2.26.2
Re: [PATCH] macintosh/adb-iop: Send correct poll command
On Fri, 4 Dec 2020, Geert Uytterhoeven wrote:
> Hi Finn,
>
> On Fri, Nov 20, 2020 at 5:54 AM Finn Thain wrote:
> > The behaviour of the IOP firmware is not well documented but we do know
> > that IOP message reply data can be used to issue new ADB commands.
> > Use the message reply to better control autopoll behaviour by sending
> > a Talk Register 0 command after every ADB response, not unlike the
> > algorithm in the via-macii driver. This poll command is addressed to
> > that device which last received a Talk command (explicit or otherwise).
> >
> > Cc: Joshua Thompson
> > Fixes: fa3b5a9929fc ("macintosh/adb-iop: Implement idle -> sending state
> > transition")
>
> WARNING: Unknown commit id 'fa3b5a9929fc', maybe rebased or not pulled?
>
> 32226e817043?
>
Yes, that's the one. I accidentally gave a commit id from one of my
backport branches.
> > Tested-by: Stan Johnson
> > Signed-off-by: Finn Thain
>
> Thanks, will queue in the m68k for-v5.11 branch.
>
Thanks.
> Gr{oetje,eeting}s,
>
> Geert
>
>
Re: [PATCH] macintosh/adb-iop: Always wait for reply message from IOP
On Fri, 4 Dec 2020, Geert Uytterhoeven wrote:
> Hi Finn,
>
> On Fri, Nov 20, 2020 at 5:54 AM Finn Thain wrote:
> > A recent patch incorrectly altered the adb-iop state machine behaviour
> > and introduced a regression that can appear intermittently as a
> > malfunctioning ADB input device. This seems to be caused when reply
> > packets from different ADB commands become mixed up, especially during
> > the adb bus scan. Fix this by unconditionally entering the awaiting_reply
> > state after sending an explicit command, even when the ADB command won't
> > generate a reply from the ADB device.
> >
> > Cc: Joshua Thompson
> > Fixes: e2954e5f727f ("macintosh/adb-iop: Implement sending -> idle state
> > transition")
> > Tested-by: Stan Johnson
> > Signed-off-by: Finn Thain
>
> Thanks for your patch!
>
> > --- a/drivers/macintosh/adb-iop.c
> > +++ b/drivers/macintosh/adb-iop.c
> > @@ -84,10 +84,7 @@ static void adb_iop_complete(struct iop_msg *msg)
> >
> > local_irq_save(flags);
> >
> > - if (current_req->reply_expected)
> > - adb_iop_state = awaiting_reply;
> > - else
> > - adb_iop_done();
> > + adb_iop_state = awaiting_reply;
> >
> > local_irq_restore(flags);
> > }
> > @@ -95,8 +92,9 @@ static void adb_iop_complete(struct iop_msg *msg)
> > /*
> > * Listen for ADB messages from the IOP.
> > *
> > - * This will be called when unsolicited messages (usually replies to TALK
> > - * commands or autopoll packets) are received.
> > + * This will be called when unsolicited IOP messages are received.
> > + * These IOP messages can carry ADB autopoll responses and also occur
> > + * after explicit ADB commands.
> > */
> >
> > static void adb_iop_listen(struct iop_msg *msg)
> > @@ -123,8 +121,10 @@ static void adb_iop_listen(struct iop_msg *msg)
> > if (adb_iop_state == awaiting_reply) {
> > struct adb_request *req = current_req;
> >
> > - req->reply_len = amsg->count + 1;
> > - memcpy(req->reply, >cmd, req->reply_len);
> > + if (req->reply_expected) {
> > + req->reply_len = amsg->count + 1;
> > + memcpy(req->reply, >cmd,
> > req->reply_len);
> > + }
>
> So if we're not expecting a reply. It's ignored.
> Just wondering: what kind of messages are being dropped?
I believe they were empty, with flags == ADB_IOP_EXPLICIT|ADB_IOP_TIMEOUT.
> If reply packets from different ADB commands become mixed up, they are
> still (expected?) replies to messages we sent before. Why shouldn't we
> depend on receiving the replies?
>
It turns out that the IOP always generates reply messages, even when the
ADB command does not produce a reply packet (e.g. ADB Listen command).
The commit being fixed got that wrong.
So it's not really the ADB reply packets that are being mixed up, it's the
IOP messages that enclose them. The bug goes like this:
1. CPU sends a message to the IOP, expecting no response because this
message contains an ADB Listen command. The ADB command is now considered
complete.
2. CPU sends a second message to the IOP, this time expecting a response
because this message contains an ADB Talk command. This ADB command needs
a reply before it can be completed.
3. adb-iop driver receives an IOP message and assumes that it relates to
the Talk command. It's actually for the previous command. The Talk command
is now considered complete but it gets the wrong reply data.
4. adb-iop driver gets another IOP response message, which contains the
actual reply data for the Talk command, but this is dropped (the driver is
no longer in awaiting_reply state).
Please go ahead and add this analysis to the commit log if you think it
would help.
> >
> > req_done = true;
> > }
>
> Gr{oetje,eeting}s,
>
> Geert
>
>
[PATCH v2] m68k: Fix WARNING splat in pmac_zilog driver
Don't add platform resources that won't be used. This avoids a
recently-added warning from the driver core, that can show up on a
multi-platform kernel when !MACH_IS_MAC.
[ cut here ]
WARNING: CPU: 0 PID: 0 at drivers/base/platform.c:224
platform_get_irq_optional+0x8e/0xce
0 is an invalid IRQ number
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Not tainted 5.9.0-multi #1
Stack from 004b3f04:
004b3f04 00462c2f 00462c2f 004b3f20 0002e128 004754db 004b6ad4 004b3f4c
0002e19c 004754f7 00e0 00285ba0 0009 004b3f44
004754db 004b3f64 004b3f74 00285ba0 004754f7 00e0 0009 004754db
004fdf0c 005269e2 004fdf0c 004b3f88 00285cae 004b6964
004fdf0c 004b3fac 0051cc68 004b6964 004b6964 0200
0051cc3e 0023c18a 004b3fc0 0051cd8a 004fdf0c 0002 0052b43c 004b3fc8
Call Trace: [<0002e128>] __warn+0xa6/0xd6
[<0002e19c>] warn_slowpath_fmt+0x44/0x76
[<00285ba0>] platform_get_irq_optional+0x8e/0xce
[<00285ba0>] platform_get_irq_optional+0x8e/0xce
[<00285cae>] platform_get_irq+0x12/0x4c
[<0051cc68>] pmz_init_port+0x2a/0xa6
[<0051cc3e>] pmz_init_port+0x0/0xa6
[<0023c18a>] strlen+0x0/0x22
[<0051cd8a>] pmz_probe+0x34/0x88
[<0051cde6>] pmz_console_init+0x8/0x28
[<00511776>] console_init+0x1e/0x28
[<0005a3bc>] printk+0x0/0x16
[<0050a8a6>] start_kernel+0x368/0x4ce
[<005094f8>] _sinittext+0x4f8/0xc48
random: get_random_bytes called from print_oops_end_marker+0x56/0x80 with
crng_init=0
---[ end trace 392d8e82eed68d6c ]---
Commit a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is invalid"),
which introduced the WARNING, suggests that testing for irq == 0 is
undesirable. Instead of that comparison, just test for resource existence.
Cc: Michael Ellerman
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Joshua Thompson
Cc: Greg Kroah-Hartman
Cc: Jiri Slaby
Cc: sta...@vger.kernel.org # v5.8+
References: commit a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is
invalid")
Reported-by: Laurent Vivier
Signed-off-by: Finn Thain
---
Changed since v1:
- Add a comment to explain the need for the global structs.
- Expand the commit log to better explain the intention of the patch.
---
arch/m68k/mac/config.c | 17 +
drivers/tty/serial/pmac_zilog.c | 14 +-
2 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/arch/m68k/mac/config.c b/arch/m68k/mac/config.c
index 0ac53d87493c..2bea1799b8de 100644
--- a/arch/m68k/mac/config.c
+++ b/arch/m68k/mac/config.c
@@ -777,16 +777,12 @@ static struct resource scc_b_rsrcs[] = {
struct platform_device scc_a_pdev = {
.name = "scc",
.id = 0,
- .num_resources = ARRAY_SIZE(scc_a_rsrcs),
- .resource = scc_a_rsrcs,
};
EXPORT_SYMBOL(scc_a_pdev);
struct platform_device scc_b_pdev = {
.name = "scc",
.id = 1,
- .num_resources = ARRAY_SIZE(scc_b_rsrcs),
- .resource = scc_b_rsrcs,
};
EXPORT_SYMBOL(scc_b_pdev);
@@ -813,10 +809,15 @@ static void __init mac_identify(void)
/* Set up serial port resources for the console initcall. */
- scc_a_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase + 2;
- scc_a_rsrcs[0].end = scc_a_rsrcs[0].start;
- scc_b_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase;
- scc_b_rsrcs[0].end = scc_b_rsrcs[0].start;
+ scc_a_rsrcs[0].start = (resource_size_t)mac_bi_data.sccbase + 2;
+ scc_a_rsrcs[0].end = scc_a_rsrcs[0].start;
+ scc_a_pdev.num_resources = ARRAY_SIZE(scc_a_rsrcs);
+ scc_a_pdev.resource = scc_a_rsrcs;
+
+ scc_b_rsrcs[0].start = (resource_size_t)mac_bi_data.sccbase;
+ scc_b_rsrcs[0].end = scc_b_rsrcs[0].start;
+ scc_b_pdev.num_resources = ARRAY_SIZE(scc_b_rsrcs);
+ scc_b_pdev.resource = scc_b_rsrcs;
switch (macintosh_config->scc_type) {
case MAC_SCC_PSC:
diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
index 96e7aa479961..216b75ef5048 100644
--- a/drivers/tty/serial/pmac_zilog.c
+++ b/drivers/tty/serial/pmac_zilog.c
@@ -1693,22 +1693,26 @@ static int __init pmz_probe(void)
#else
+/* On PCI PowerMacs, pmz_probe() does an explicit search of the OpenFirmware
+ * tree to obtain the device_nodes needed to start the console before the
+ * macio driver. On Macs without OpenFirmware, global platform_devices take
+ * the place of those device_nodes.
+ */
extern struct platform_device scc_a_pdev, scc_b_pdev;
static int __init pmz_init_port(struct uart_pmac_port *uap)
{
- struct resource *r_ports;
- int irq;
+ struct resource *r_ports, *r_irq;
r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0
Re: [PATCH] m68k: Fix WARNING splat in pmac_zilog driver
On Fri, 20 Nov 2020, Geert Uytterhoeven wrote:
> Hi Finn,
>
> On Fri, Nov 20, 2020 at 5:51 AM Finn Thain wrote:
> > Don't add platform resources that won't be used. This avoids a
> > recently-added warning from the driver core, that can show up on a
> > multi-platform kernel when !MACH_IS_MAC.
> >
> > [ cut here ]
> > WARNING: CPU: 0 PID: 0 at drivers/base/platform.c:224
> > platform_get_irq_optional+0x8e/0xce
> > 0 is an invalid IRQ number
> > Modules linked in:
> > CPU: 0 PID: 0 Comm: swapper Not tainted 5.9.0-multi #1
> > Stack from 004b3f04:
> > 004b3f04 00462c2f 00462c2f 004b3f20 0002e128 004754db 004b6ad4
> > 004b3f4c
> > 0002e19c 004754f7 00e0 00285ba0 0009 004b3f44
> >
> > 004754db 004b3f64 004b3f74 00285ba0 004754f7 00e0 0009
> > 004754db
> > 004fdf0c 005269e2 004fdf0c 004b3f88 00285cae 004b6964
> >
> > 004fdf0c 004b3fac 0051cc68 004b6964 004b6964 0200
> >
> > 0051cc3e 0023c18a 004b3fc0 0051cd8a 004fdf0c 0002 0052b43c
> > 004b3fc8
> > Call Trace: [<0002e128>] __warn+0xa6/0xd6
> > [<0002e19c>] warn_slowpath_fmt+0x44/0x76
> > [<00285ba0>] platform_get_irq_optional+0x8e/0xce
> > [<00285ba0>] platform_get_irq_optional+0x8e/0xce
> > [<00285cae>] platform_get_irq+0x12/0x4c
> > [<0051cc68>] pmz_init_port+0x2a/0xa6
> > [<0051cc3e>] pmz_init_port+0x0/0xa6
> > [<0023c18a>] strlen+0x0/0x22
> > [<0051cd8a>] pmz_probe+0x34/0x88
> > [<0051cde6>] pmz_console_init+0x8/0x28
> > [<00511776>] console_init+0x1e/0x28
> > [<0005a3bc>] printk+0x0/0x16
> > [<0050a8a6>] start_kernel+0x368/0x4ce
> > [<005094f8>] _sinittext+0x4f8/0xc48
> > random: get_random_bytes called from print_oops_end_marker+0x56/0x80 with
> > crng_init=0
> > ---[ end trace 392d8e82eed68d6c ]---
> >
> > Cc: Michael Ellerman
> > Cc: Benjamin Herrenschmidt
> > Cc: Paul Mackerras
> > Cc: Joshua Thompson
> > Cc: Greg Kroah-Hartman
> > Cc: Jiri Slaby
> > Cc: sta...@vger.kernel.org # v5.8+
> > References: commit a85a6c86c25b ("driver core: platform: Clarify that IRQ 0
> > is invalid")
> > Reported-by: Laurent Vivier
> > Signed-off-by: Finn Thain
> > ---
> > The global platform_device structs provide the equivalent of a direct
> > search of the OpenFirmware tree, for platforms that don't have OF.
> > The purpose of that search is discussed in the comments in pmac_zilog.c:
> >
> > * First, we need to do a direct OF-based probe pass. We
> > * do that because we want serial console up before the
> > * macio stuffs calls us back
> >
> > The actual platform bus matching takes place later, with a module_initcall,
> > following the usual pattern.
>
> I think it would be good for this explanation to be part of the
> actual patch description above.
>
Thanks for your review.
I take that explanation as read because it was fundamental to the changes
I made to pmac_zilog.c back in 2009 with commit ec9cbe09899e ("pmac-zilog:
add platform driver").
IMO, being that it isn't news, it doesn't belong in the changelog.
However, I agree that it needs to be documented. How about I add a comment
to pmac_zilog.c?
> > --- a/arch/m68k/mac/config.c
> > +++ b/arch/m68k/mac/config.c
> > @@ -777,16 +777,12 @@ static struct resource scc_b_rsrcs[] = {
> > struct platform_device scc_a_pdev = {
> > .name = "scc",
> > .id = 0,
> > - .num_resources = ARRAY_SIZE(scc_a_rsrcs),
> > - .resource = scc_a_rsrcs,
> > };
> > EXPORT_SYMBOL(scc_a_pdev);
> >
> > struct platform_device scc_b_pdev = {
> > .name = "scc",
> > .id = 1,
> > - .num_resources = ARRAY_SIZE(scc_b_rsrcs),
> > - .resource = scc_b_rsrcs,
> > };
> > EXPORT_SYMBOL(scc_b_pdev);
> >
> > @@ -813,10 +809,15 @@ static void __init mac_identify(void)
> >
> > /* Set up serial port resources for the console initcall. */
> >
> > - scc_a_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase + 2;
> > - scc_a_rsrcs[0].end = scc_a_rsrcs[0].start;
> > - scc_b_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase;
> > - scc_b_rsrcs[0].end = scc_b_rsrcs[0].start;
> > + scc_a_rsrcs[0].start
[PATCH] macintosh/adb-iop: Send correct poll command
The behaviour of the IOP firmware is not well documented but we do know
that IOP message reply data can be used to issue new ADB commands.
Use the message reply to better control autopoll behaviour by sending
a Talk Register 0 command after every ADB response, not unlike the
algorithm in the via-macii driver. This poll command is addressed to
that device which last received a Talk command (explicit or otherwise).
Cc: Joshua Thompson
Fixes: fa3b5a9929fc ("macintosh/adb-iop: Implement idle -> sending state
transition")
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
drivers/macintosh/adb-iop.c | 40 +++--
1 file changed, 30 insertions(+), 10 deletions(-)
diff --git a/drivers/macintosh/adb-iop.c b/drivers/macintosh/adb-iop.c
index f3d1a460fbce..6b26b6a2c463 100644
--- a/drivers/macintosh/adb-iop.c
+++ b/drivers/macintosh/adb-iop.c
@@ -25,6 +25,7 @@
static struct adb_request *current_req;
static struct adb_request *last_req;
static unsigned int autopoll_devs;
+static u8 autopoll_addr;
static enum adb_iop_state {
idle,
@@ -41,6 +42,11 @@ static int adb_iop_autopoll(int);
static void adb_iop_poll(void);
static int adb_iop_reset_bus(void);
+/* ADB command byte structure */
+#define ADDR_MASK 0xF0
+#define OP_MASK 0x0C
+#define TALK0x0C
+
struct adb_driver adb_iop_driver = {
.name = "ISM IOP",
.probe= adb_iop_probe,
@@ -96,17 +102,24 @@ static void adb_iop_complete(struct iop_msg *msg)
static void adb_iop_listen(struct iop_msg *msg)
{
struct adb_iopmsg *amsg = (struct adb_iopmsg *)msg->message;
+ u8 addr = (amsg->cmd & ADDR_MASK) >> 4;
+ u8 op = amsg->cmd & OP_MASK;
unsigned long flags;
bool req_done = false;
local_irq_save(flags);
- /* Handle a timeout. Timeout packets seem to occur even after
-* we've gotten a valid reply to a TALK, presumably because of
-* autopolling.
+ /* Responses to Talk commands may be unsolicited as they are
+* produced when the IOP polls devices. They are mostly timeouts.
*/
-
- if (amsg->flags & ADB_IOP_EXPLICIT) {
+ if (op == TALK && ((1 << addr) & autopoll_devs))
+ autopoll_addr = addr;
+
+ switch (amsg->flags & (ADB_IOP_EXPLICIT |
+ ADB_IOP_AUTOPOLL |
+ ADB_IOP_TIMEOUT)) {
+ case ADB_IOP_EXPLICIT:
+ case ADB_IOP_EXPLICIT | ADB_IOP_TIMEOUT:
if (adb_iop_state == awaiting_reply) {
struct adb_request *req = current_req;
@@ -115,12 +128,16 @@ static void adb_iop_listen(struct iop_msg *msg)
req_done = true;
}
- } else if (!(amsg->flags & ADB_IOP_TIMEOUT)) {
- adb_input(>cmd, amsg->count + 1,
- amsg->flags & ADB_IOP_AUTOPOLL);
+ break;
+ case ADB_IOP_AUTOPOLL:
+ if (((1 << addr) & autopoll_devs) &&
+ amsg->cmd == ADB_READREG(addr, 0))
+ adb_input(>cmd, amsg->count + 1, 1);
+ break;
}
-
- msg->reply[0] = autopoll_devs ? ADB_IOP_AUTOPOLL : 0;
+ msg->reply[0] = autopoll_addr ? ADB_IOP_AUTOPOLL : 0;
+ msg->reply[1] = 0;
+ msg->reply[2] = autopoll_addr ? ADB_READREG(autopoll_addr, 0) : 0;
iop_complete_message(msg);
if (req_done)
@@ -233,6 +250,9 @@ static void adb_iop_set_ap_complete(struct iop_msg *msg)
struct adb_iopmsg *amsg = (struct adb_iopmsg *)msg->message;
autopoll_devs = (amsg->data[1] << 8) | amsg->data[0];
+ if (autopoll_devs & (1 << autopoll_addr))
+ return;
+ autopoll_addr = autopoll_devs ? (ffs(autopoll_devs) - 1) : 0;
}
static int adb_iop_autopoll(int devs)
--
2.26.2
[PATCH] macintosh/adb-iop: Always wait for reply message from IOP
A recent patch incorrectly altered the adb-iop state machine behaviour
and introduced a regression that can appear intermittently as a
malfunctioning ADB input device. This seems to be caused when reply
packets from different ADB commands become mixed up, especially during
the adb bus scan. Fix this by unconditionally entering the awaiting_reply
state after sending an explicit command, even when the ADB command won't
generate a reply from the ADB device.
Cc: Joshua Thompson
Fixes: e2954e5f727f ("macintosh/adb-iop: Implement sending -> idle state
transition")
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
drivers/macintosh/adb-iop.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/macintosh/adb-iop.c b/drivers/macintosh/adb-iop.c
index 6b26b6a2c463..0ee327249150 100644
--- a/drivers/macintosh/adb-iop.c
+++ b/drivers/macintosh/adb-iop.c
@@ -84,10 +84,7 @@ static void adb_iop_complete(struct iop_msg *msg)
local_irq_save(flags);
- if (current_req->reply_expected)
- adb_iop_state = awaiting_reply;
- else
- adb_iop_done();
+ adb_iop_state = awaiting_reply;
local_irq_restore(flags);
}
@@ -95,8 +92,9 @@ static void adb_iop_complete(struct iop_msg *msg)
/*
* Listen for ADB messages from the IOP.
*
- * This will be called when unsolicited messages (usually replies to TALK
- * commands or autopoll packets) are received.
+ * This will be called when unsolicited IOP messages are received.
+ * These IOP messages can carry ADB autopoll responses and also occur
+ * after explicit ADB commands.
*/
static void adb_iop_listen(struct iop_msg *msg)
@@ -123,8 +121,10 @@ static void adb_iop_listen(struct iop_msg *msg)
if (adb_iop_state == awaiting_reply) {
struct adb_request *req = current_req;
- req->reply_len = amsg->count + 1;
- memcpy(req->reply, >cmd, req->reply_len);
+ if (req->reply_expected) {
+ req->reply_len = amsg->count + 1;
+ memcpy(req->reply, >cmd, req->reply_len);
+ }
req_done = true;
}
--
2.26.2
[PATCH] m68k: Fix WARNING splat in pmac_zilog driver
Don't add platform resources that won't be used. This avoids a
recently-added warning from the driver core, that can show up on a
multi-platform kernel when !MACH_IS_MAC.
[ cut here ]
WARNING: CPU: 0 PID: 0 at drivers/base/platform.c:224
platform_get_irq_optional+0x8e/0xce
0 is an invalid IRQ number
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Not tainted 5.9.0-multi #1
Stack from 004b3f04:
004b3f04 00462c2f 00462c2f 004b3f20 0002e128 004754db 004b6ad4 004b3f4c
0002e19c 004754f7 00e0 00285ba0 0009 004b3f44
004754db 004b3f64 004b3f74 00285ba0 004754f7 00e0 0009 004754db
004fdf0c 005269e2 004fdf0c 004b3f88 00285cae 004b6964
004fdf0c 004b3fac 0051cc68 004b6964 004b6964 0200
0051cc3e 0023c18a 004b3fc0 0051cd8a 004fdf0c 0002 0052b43c 004b3fc8
Call Trace: [<0002e128>] __warn+0xa6/0xd6
[<0002e19c>] warn_slowpath_fmt+0x44/0x76
[<00285ba0>] platform_get_irq_optional+0x8e/0xce
[<00285ba0>] platform_get_irq_optional+0x8e/0xce
[<00285cae>] platform_get_irq+0x12/0x4c
[<0051cc68>] pmz_init_port+0x2a/0xa6
[<0051cc3e>] pmz_init_port+0x0/0xa6
[<0023c18a>] strlen+0x0/0x22
[<0051cd8a>] pmz_probe+0x34/0x88
[<0051cde6>] pmz_console_init+0x8/0x28
[<00511776>] console_init+0x1e/0x28
[<0005a3bc>] printk+0x0/0x16
[<0050a8a6>] start_kernel+0x368/0x4ce
[<005094f8>] _sinittext+0x4f8/0xc48
random: get_random_bytes called from print_oops_end_marker+0x56/0x80 with
crng_init=0
---[ end trace 392d8e82eed68d6c ]---
Cc: Michael Ellerman
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Joshua Thompson
Cc: Greg Kroah-Hartman
Cc: Jiri Slaby
Cc: sta...@vger.kernel.org # v5.8+
References: commit a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is
invalid")
Reported-by: Laurent Vivier
Signed-off-by: Finn Thain
---
The global platform_device structs provide the equivalent of a direct
search of the OpenFirmware tree, for platforms that don't have OF.
The purpose of that search is discussed in the comments in pmac_zilog.c:
* First, we need to do a direct OF-based probe pass. We
* do that because we want serial console up before the
* macio stuffs calls us back
The actual platform bus matching takes place later, with a module_initcall,
following the usual pattern.
---
arch/m68k/mac/config.c | 17 +
drivers/tty/serial/pmac_zilog.c | 9 -
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/arch/m68k/mac/config.c b/arch/m68k/mac/config.c
index 0ac53d87493c..2bea1799b8de 100644
--- a/arch/m68k/mac/config.c
+++ b/arch/m68k/mac/config.c
@@ -777,16 +777,12 @@ static struct resource scc_b_rsrcs[] = {
struct platform_device scc_a_pdev = {
.name = "scc",
.id = 0,
- .num_resources = ARRAY_SIZE(scc_a_rsrcs),
- .resource = scc_a_rsrcs,
};
EXPORT_SYMBOL(scc_a_pdev);
struct platform_device scc_b_pdev = {
.name = "scc",
.id = 1,
- .num_resources = ARRAY_SIZE(scc_b_rsrcs),
- .resource = scc_b_rsrcs,
};
EXPORT_SYMBOL(scc_b_pdev);
@@ -813,10 +809,15 @@ static void __init mac_identify(void)
/* Set up serial port resources for the console initcall. */
- scc_a_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase + 2;
- scc_a_rsrcs[0].end = scc_a_rsrcs[0].start;
- scc_b_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase;
- scc_b_rsrcs[0].end = scc_b_rsrcs[0].start;
+ scc_a_rsrcs[0].start = (resource_size_t)mac_bi_data.sccbase + 2;
+ scc_a_rsrcs[0].end = scc_a_rsrcs[0].start;
+ scc_a_pdev.num_resources = ARRAY_SIZE(scc_a_rsrcs);
+ scc_a_pdev.resource = scc_a_rsrcs;
+
+ scc_b_rsrcs[0].start = (resource_size_t)mac_bi_data.sccbase;
+ scc_b_rsrcs[0].end = scc_b_rsrcs[0].start;
+ scc_b_pdev.num_resources = ARRAY_SIZE(scc_b_rsrcs);
+ scc_b_pdev.resource = scc_b_rsrcs;
switch (macintosh_config->scc_type) {
case MAC_SCC_PSC:
diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
index 96e7aa479961..95abdb305d67 100644
--- a/drivers/tty/serial/pmac_zilog.c
+++ b/drivers/tty/serial/pmac_zilog.c
@@ -1697,18 +1697,17 @@ extern struct platform_device scc_a_pdev, scc_b_pdev;
static int __init pmz_init_port(struct uart_pmac_port *uap)
{
- struct resource *r_ports;
- int irq;
+ struct resource *r_ports, *r_irq;
r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0);
- irq = platform_get_irq(uap->pdev, 0);
- if (!r_ports || irq <= 0)
+ r_irq = platform_get_resource(uap->pdev, IORESOURCE_IRQ, 0);
+ if (!r_ports || !r_irq)
return -ENODEV;
Re: [PATCH] serial: pmac_zilog: don't init if zilog is not available
On Thu, 22 Oct 2020, Geert Uytterhoeven wrote: > > Thanks for your patch... > You're welcome. > I can't say I'm a fan of this... > Sorry. > > The real issue is this "extern struct platform_device scc_a_pdev, > scc_b_pdev", circumventing the driver framework. > > Can we get rid of that? > Is there a better alternative? pmz_probe() is called by console_initcall(pmz_console_init) when CONFIG_SERIAL_PMACZILOG_CONSOLE=y because this has to happen earlier than the normal platform bus probing which takes place later as a typical module_initcall.
Re: [PATCH] serial: pmac_zilog: don't init if zilog is not available
On Wed, 21 Oct 2020, Laurent Vivier wrote:
> Le 21/10/2020 à 01:43, Finn Thain a écrit :
>
> > Laurent, can we avoid the irq == 0 warning splat like this?
> >
> > diff --git a/drivers/tty/serial/pmac_zilog.c
> > b/drivers/tty/serial/pmac_zilog.c
> > index 96e7aa479961..7db600cd8cc7 100644
> > --- a/drivers/tty/serial/pmac_zilog.c
> > +++ b/drivers/tty/serial/pmac_zilog.c
> > @@ -1701,8 +1701,10 @@ static int __init pmz_init_port(struct
> > uart_pmac_port *uap)
> > int irq;
> >
> > r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0);
> > + if (!r_ports)
> > + return -ENODEV;
> > irq = platform_get_irq(uap->pdev, 0);
> > - if (!r_ports || irq <= 0)
> > + if (irq <= 0)
> > return -ENODEV;
> >
> > uap->port.mapbase = r_ports->start;
> >
>
> No, this doesn't fix the problem.
>
Then I had better stop guessing and start up Aranym...
The patch below seems to fix the problem for me. Does it work on your
system(s)?
diff --git a/arch/m68k/mac/config.c b/arch/m68k/mac/config.c
index a621fcc1a576..4e802f70333d 100644
--- a/arch/m68k/mac/config.c
+++ b/arch/m68k/mac/config.c
@@ -776,16 +776,12 @@ static struct resource scc_b_rsrcs[] = {
struct platform_device scc_a_pdev = {
.name = "scc",
.id = 0,
- .num_resources = ARRAY_SIZE(scc_a_rsrcs),
- .resource = scc_a_rsrcs,
};
EXPORT_SYMBOL(scc_a_pdev);
struct platform_device scc_b_pdev = {
.name = "scc",
.id = 1,
- .num_resources = ARRAY_SIZE(scc_b_rsrcs),
- .resource = scc_b_rsrcs,
};
EXPORT_SYMBOL(scc_b_pdev);
@@ -812,10 +808,15 @@ static void __init mac_identify(void)
/* Set up serial port resources for the console initcall. */
- scc_a_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase + 2;
- scc_a_rsrcs[0].end = scc_a_rsrcs[0].start;
- scc_b_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase;
- scc_b_rsrcs[0].end = scc_b_rsrcs[0].start;
+ scc_a_rsrcs[0].start = (resource_size_t)mac_bi_data.sccbase + 2;
+ scc_a_rsrcs[0].end = scc_a_rsrcs[0].start;
+ scc_a_pdev.num_resources = ARRAY_SIZE(scc_a_rsrcs);
+ scc_a_pdev.resource = scc_a_rsrcs;
+
+ scc_b_rsrcs[0].start = (resource_size_t)mac_bi_data.sccbase;
+ scc_b_rsrcs[0].end = scc_b_rsrcs[0].start;
+ scc_b_pdev.num_resources = ARRAY_SIZE(scc_b_rsrcs);
+ scc_b_pdev.resource = scc_b_rsrcs;
switch (macintosh_config->scc_type) {
case MAC_SCC_PSC:
diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
index 96e7aa479961..95abdb305d67 100644
--- a/drivers/tty/serial/pmac_zilog.c
+++ b/drivers/tty/serial/pmac_zilog.c
@@ -1697,18 +1697,17 @@ extern struct platform_device scc_a_pdev, scc_b_pdev;
static int __init pmz_init_port(struct uart_pmac_port *uap)
{
- struct resource *r_ports;
- int irq;
+ struct resource *r_ports, *r_irq;
r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0);
- irq = platform_get_irq(uap->pdev, 0);
- if (!r_ports || irq <= 0)
+ r_irq = platform_get_resource(uap->pdev, IORESOURCE_IRQ, 0);
+ if (!r_ports || !r_irq)
return -ENODEV;
uap->port.mapbase = r_ports->start;
uap->port.membase = (unsigned char __iomem *) r_ports->start;
uap->port.iotype = UPIO_MEM;
- uap->port.irq = irq;
+ uap->port.irq = r_irq->start;
uap->port.uartclk = ZS_CLOCK;
uap->port.fifosize = 1;
uap->port.ops = _pops;
Re: [PATCH] serial: pmac_zilog: don't init if zilog is not available
On Tue, 20 Oct 2020, Brad Boyer wrote: > > Wouldn't it be better to rearrange this code to only run if the devices > are present? This is a macio driver on pmac and a platform driver on > mac, so shouldn't it be possible to only run this code when the > appropriate entries are present in the right data structures? > > I didn't look at a lot of the other serial drivers, but some other mac > drivers have recently been updated to no longer have MACH_IS_MAC checks > due to being converted to platform drivers. > Actually, it's not simply a platform driver or macio driver. I think the console is supposed to be registered before the normal bus matching takes place. Hence this comment in pmac_zilog.c, /* * First, we need to do a direct OF-based probe pass. We * do that because we want serial console up before the * macio stuffs calls us back, and since that makes it * easier to pass the proper number of channels to * uart_register_driver() */ Laurent, can we avoid the irq == 0 warning splat like this? diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c index 96e7aa479961..7db600cd8cc7 100644 --- a/drivers/tty/serial/pmac_zilog.c +++ b/drivers/tty/serial/pmac_zilog.c @@ -1701,8 +1701,10 @@ static int __init pmz_init_port(struct uart_pmac_port *uap) int irq; r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0); + if (!r_ports) + return -ENODEV; irq = platform_get_irq(uap->pdev, 0); - if (!r_ports || irq <= 0) + if (irq <= 0) return -ENODEV; uap->port.mapbase = r_ports->start;
Re: [PATCH 1/9] kernel: add a PF_FORCE_COMPAT flag
On Sat, 19 Sep 2020, Arnd Bergmann wrote: > On Sat, Sep 19, 2020 at 6:21 PM Andy Lutomirski wrote: > > On Fri, Sep 18, 2020 at 8:16 AM Christoph Hellwig wrote: > > > On Fri, Sep 18, 2020 at 02:58:22PM +0100, Al Viro wrote: > > > > Said that, why not provide a variant that would take an explicit > > > > "is it compat" argument and use it there? And have the normal one > > > > pass in_compat_syscall() to that... > > > > > > That would help to not introduce a regression with this series yes. > > > But it wouldn't fix existing bugs when io_uring is used to access > > > read or write methods that use in_compat_syscall(). One example > > > that I recently ran into is drivers/scsi/sg.c. > > Ah, so reading /dev/input/event* would suffer from the same issue, and > that one would in fact be broken by your patch in the hypothetical case > that someone tried to use io_uring to read /dev/input/event on x32... > > For reference, I checked the socket timestamp handling that has a number > of corner cases with time32/time64 formats in compat mode, but none of > those appear to be affected by the problem. > > > Aside from the potentially nasty use of per-task variables, one thing > > I don't like about PF_FORCE_COMPAT is that it's one-way. If we're > > going to have a generic mechanism for this, shouldn't we allow a full > > override of the syscall arch instead of just allowing forcing compat > > so that a compat syscall can do a non-compat operation? > > The only reason it's needed here is that the caller is in a kernel > thread rather than a system call. Are there any possible scenarios where > one would actually need the opposite? > Quite possibly. The ext4 vs. compat getdents bug is still unresolved. Please see, https://lore.kernel.org/lkml/cafeaca9w+jk7_trttnl1p2es1knnpjx9wcuvhflwxlq9aug...@mail.gmail.com/ >Arnd >
[PATCH 5/5] powerpc/tau: Disable TAU between measurements
Enabling CONFIG_TAU_INT causes random crashes:
Unrecoverable exception 1700 at c0009414 (msr=1000)
Oops: Unrecoverable exception, sig: 6 [#1]
BE PAGE_SIZE=4K MMU=Hash SMP NR_CPUS=2 PowerMac
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.7.0-pmac-00043-gd5f545e1a8593 #5
NIP: c0009414 LR: c0009414 CTR: c00116fc
REGS: c0799eb8 TRAP: 1700 Not tainted (5.7.0-pmac-00043-gd5f545e1a8593)
MSR: 1000 CR: 22000228 XER: 0100
GPR00: c0799f70 c076e300 0080 0291c0ac 00e0 c076e300 00049032
GPR08: 0001 c00116fc dfbd3200 007f80a8
GPR16: c075ce04
GPR24: c075ce04 dfff8880 c07b c075ce04 0008 0001 c079ef98 c079ef5c
NIP [c0009414] arch_cpu_idle+0x24/0x6c
LR [c0009414] arch_cpu_idle+0x24/0x6c
Call Trace:
[c0799f70] [0001] 0x1 (unreliable)
[c0799f80] [c0060990] do_idle+0xd8/0x17c
[c0799fa0] [c0060ba4] cpu_startup_entry+0x20/0x28
[c0799fb0] [c072d220] start_kernel+0x434/0x44c
[c0799ff0] [3860] 0x3860
Instruction dump:
3d20c07b 7c0802a6
4e800421 7d2000a6
---[ end trace 3a0c9b5cb216db6b ]---
Resolve this problem by disabling each THRMn comparator when handling
the associated THRMn interrupt and by disabling the TAU entirely when
updating THRMn thresholds.
Fixes: 1da177e4c3f41 ("Linux-2.6.12-rc2")
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
arch/powerpc/kernel/tau_6xx.c | 65 +-
arch/powerpc/platforms/Kconfig | 9 ++---
2 files changed, 26 insertions(+), 48 deletions(-)
diff --git a/arch/powerpc/kernel/tau_6xx.c b/arch/powerpc/kernel/tau_6xx.c
index 614b5b272d9c6..0b4694b8d2482 100644
--- a/arch/powerpc/kernel/tau_6xx.c
+++ b/arch/powerpc/kernel/tau_6xx.c
@@ -42,8 +42,6 @@ static struct tau_temp
static bool tau_int_enable;
-#undef DEBUG
-
/* TODO: put these in a /proc interface, with some sanity checks, and maybe
* dynamic adjustment to minimize # of interrupts */
/* configurable values for step size and how much to expand the window when
@@ -67,42 +65,33 @@ static void set_thresholds(unsigned long cpu)
static void TAUupdate(int cpu)
{
- unsigned thrm;
-
-#ifdef DEBUG
- printk("TAUupdate ");
-#endif
+ u32 thrm;
+ u32 bits = THRM1_TIV | THRM1_TIN | THRM1_V;
/* if both thresholds are crossed, the step_sizes cancel out
* and the window winds up getting expanded twice. */
- if((thrm = mfspr(SPRN_THRM1)) & THRM1_TIV){ /* is valid? */
- if(thrm & THRM1_TIN){ /* crossed low threshold */
- if (tau[cpu].low >= step_size){
- tau[cpu].low -= step_size;
- tau[cpu].high -= (step_size - window_expand);
- }
- tau[cpu].grew = 1;
-#ifdef DEBUG
- printk("low threshold crossed ");
-#endif
+ thrm = mfspr(SPRN_THRM1);
+ if ((thrm & bits) == bits) {
+ mtspr(SPRN_THRM1, 0);
+
+ if (tau[cpu].low >= step_size) {
+ tau[cpu].low -= step_size;
+ tau[cpu].high -= (step_size - window_expand);
}
+ tau[cpu].grew = 1;
+ pr_debug("%s: low threshold crossed\n", __func__);
}
- if((thrm = mfspr(SPRN_THRM2)) & THRM1_TIV){ /* is valid? */
- if(thrm & THRM1_TIN){ /* crossed high threshold */
- if (tau[cpu].high <= 127-step_size){
- tau[cpu].low += (step_size - window_expand);
- tau[cpu].high += step_size;
- }
- tau[cpu].grew = 1;
-#ifdef DEBUG
- printk("high threshold crossed ");
-#endif
+ thrm = mfspr(SPRN_THRM2);
+ if ((thrm & bits) == bits) {
+ mtspr(SPRN_THRM2, 0);
+
+ if (tau[cpu].high <= 127 - step_size) {
+ tau[cpu].low += (step_size - window_expand);
+ tau[cpu].high += step_size;
}
+ tau[cpu].grew = 1;
+ pr_debug("%s: high threshold crossed\n", __func__);
}
-
-#ifdef DEBUG
- printk("grew = %d\n", tau[cpu].grew);
-#endif
}
#ifdef CONFIG_TAU_INT
@@ -127,17 +116,17 @@ void TAUException(struct pt_regs * regs)
static void tau_timeout(void * info)
{
int cpu;
- unsigned long flags;
int size;
int shrink;
- /* disabling interrupts *should* be okay */
- local_irq_save(flags);
cpu = smp_processor_id();
if (!tau_int_enable)
TAUupdate(cpu);
+ /* Stop thermal sensor comparisons and interrupts */
[PATCH 4/5] powerpc/tau: Check processor type before enabling TAU interrupt
According to Freescale's documentation, MPC74XX processors have an
erratum that prevents the TAU interrupt from working, so don't try to
use it when running on those processors.
Fixes: 1da177e4c3f41 ("Linux-2.6.12-rc2")
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
arch/powerpc/kernel/tau_6xx.c | 33 ++---
arch/powerpc/platforms/Kconfig | 5 ++---
2 files changed, 16 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/kernel/tau_6xx.c b/arch/powerpc/kernel/tau_6xx.c
index b8d7e7d498e0a..614b5b272d9c6 100644
--- a/arch/powerpc/kernel/tau_6xx.c
+++ b/arch/powerpc/kernel/tau_6xx.c
@@ -40,6 +40,8 @@ static struct tau_temp
unsigned char grew;
} tau[NR_CPUS];
+static bool tau_int_enable;
+
#undef DEBUG
/* TODO: put these in a /proc interface, with some sanity checks, and maybe
@@ -54,22 +56,13 @@ static struct tau_temp
static void set_thresholds(unsigned long cpu)
{
-#ifdef CONFIG_TAU_INT
- /*
-* setup THRM1,
-* threshold, valid bit, enable interrupts, interrupt when below
threshold
-*/
- mtspr(SPRN_THRM1, THRM1_THRES(tau[cpu].low) | THRM1_V | THRM1_TIE |
THRM1_TID);
+ u32 maybe_tie = tau_int_enable ? THRM1_TIE : 0;
- /* setup THRM2,
-* threshold, valid bit, enable interrupts, interrupt when above
threshold
-*/
- mtspr (SPRN_THRM2, THRM1_THRES(tau[cpu].high) | THRM1_V | THRM1_TIE);
-#else
- /* same thing but don't enable interrupts */
- mtspr(SPRN_THRM1, THRM1_THRES(tau[cpu].low) | THRM1_V | THRM1_TID);
- mtspr(SPRN_THRM2, THRM1_THRES(tau[cpu].high) | THRM1_V);
-#endif
+ /* setup THRM1, threshold, valid bit, interrupt when below threshold */
+ mtspr(SPRN_THRM1, THRM1_THRES(tau[cpu].low) | THRM1_V | maybe_tie |
THRM1_TID);
+
+ /* setup THRM2, threshold, valid bit, interrupt when above threshold */
+ mtspr(SPRN_THRM2, THRM1_THRES(tau[cpu].high) | THRM1_V | maybe_tie);
}
static void TAUupdate(int cpu)
@@ -142,9 +135,8 @@ static void tau_timeout(void * info)
local_irq_save(flags);
cpu = smp_processor_id();
-#ifndef CONFIG_TAU_INT
- TAUupdate(cpu);
-#endif
+ if (!tau_int_enable)
+ TAUupdate(cpu);
size = tau[cpu].high - tau[cpu].low;
if (size > min_window && ! tau[cpu].grew) {
@@ -225,6 +217,9 @@ static int __init TAU_init(void)
return 1;
}
+ tau_int_enable = IS_ENABLED(CONFIG_TAU_INT) &&
+!strcmp(cur_cpu_spec->platform, "ppc750");
+
tau_workq = alloc_workqueue("tau", WQ_UNBOUND, 1, 0);
if (!tau_workq)
return -ENOMEM;
@@ -234,7 +229,7 @@ static int __init TAU_init(void)
queue_work(tau_workq, _work);
pr_info("Thermal assist unit using %s, shrink_timer: %d ms\n",
- IS_ENABLED(CONFIG_TAU_INT) ? "interrupts" : "workqueue",
shrink_timer);
+ tau_int_enable ? "interrupts" : "workqueue", shrink_timer);
tau_initialized = 1;
return 0;
diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index fb7515b4fa9c6..9fe36f0b54c1a 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -223,9 +223,8 @@ config TAU
temperature within 2-4 degrees Celsius. This option shows the current
on-die temperature in /proc/cpuinfo if the cpu supports it.
- Unfortunately, on some chip revisions, this sensor is very inaccurate
- and in many cases, does not work at all, so don't assume the cpu
- temp is actually what /proc/cpuinfo says it is.
+ Unfortunately, this sensor is very inaccurate when uncalibrated, so
+ don't assume the cpu temp is actually what /proc/cpuinfo says it is.
config TAU_INT
bool "Interrupt driven TAU driver (DANGEROUS)"
--
2.26.2
[PATCH 2/5] powerpc/tau: Convert from timer to workqueue
Since commit 19dbdcb8039cf ("smp: Warn on function calls from softirq
context") the Thermal Assist Unit driver causes a warning like the
following when CONFIG_SMP is enabled.
[ cut here ]
WARNING: CPU: 0 PID: 0 at kernel/smp.c:428
smp_call_function_many_cond+0xf4/0x38c
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.7.0-pmac #3
NIP: c00b37a8 LR: c00b3abc CTR: c001218c
REGS: c0799c60 TRAP: 0700 Not tainted (5.7.0-pmac)
MSR: 00029032 CR: 42000224 XER:
GPR00: c00b3abc c0799d18 c076e300 c079ef5c c0011fec
GPR08: 0100 0100 8000 42000224 c079d040 c079d044
GPR16: 0001 0004 c0799da0 c079f054 c07a c07a
GPR24: c0011fec c079ef5c c079ef5c
NIP [c00b37a8] smp_call_function_many_cond+0xf4/0x38c
LR [c00b3abc] on_each_cpu+0x38/0x68
Call Trace:
[c0799d18] [] 0x (unreliable)
[c0799d68] [c00b3abc] on_each_cpu+0x38/0x68
[c0799d88] [c0096704] call_timer_fn.isra.26+0x20/0x7c
[c0799d98] [c0096b40] run_timer_softirq+0x1d4/0x3fc
[c0799df8] [c05b4368] __do_softirq+0x118/0x240
[c0799e58] [c0039c44] irq_exit+0xc4/0xcc
[c0799e68] [c000ade8] timer_interrupt+0x1b0/0x230
[c0799ea8] [c0013520] ret_from_except+0x0/0x14
--- interrupt: 901 at arch_cpu_idle+0x24/0x6c
LR = arch_cpu_idle+0x24/0x6c
[c0799f70] [0001] 0x1 (unreliable)
[c0799f80] [c0060990] do_idle+0xd8/0x17c
[c0799fa0] [c0060ba8] cpu_startup_entry+0x24/0x28
[c0799fb0] [c072d220] start_kernel+0x434/0x44c
[c0799ff0] [3860] 0x3860
Instruction dump:
8129f204 2f89 40beff98 3d20c07a 8929eec4 2f89 40beff88 0fe0
8122 552805de 550802ef 4182ff84 <0fe0> 3860 7f65db78 7f44d378
---[ end trace 34a886e47819c2eb ]---
Don't call on_each_cpu() from a timer callback, call it from a worker
thread instead.
Fixes: 1da177e4c3f41 ("Linux-2.6.12-rc2")
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
arch/powerpc/kernel/tau_6xx.c | 38 +--
1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/kernel/tau_6xx.c b/arch/powerpc/kernel/tau_6xx.c
index 976d5bc1b5176..268205cc347da 100644
--- a/arch/powerpc/kernel/tau_6xx.c
+++ b/arch/powerpc/kernel/tau_6xx.c
@@ -13,13 +13,14 @@
*/
#include
-#include
#include
#include
#include
#include
#include
#include
+#include
+#include
#include
#include
@@ -39,8 +40,6 @@ static struct tau_temp
unsigned char grew;
} tau[NR_CPUS];
-struct timer_list tau_timer;
-
#undef DEBUG
/* TODO: put these in a /proc interface, with some sanity checks, and maybe
@@ -50,7 +49,7 @@ struct timer_list tau_timer;
#define step_size 2 /* step size when temp goes out of
range */
#define window_expand 1 /* expand the window by this much */
/* configurable values for shrinking the window */
-#define shrink_timer 2*HZ/* period between shrinking the window */
+#define shrink_timer 2000/* period between shrinking the window */
#define min_window 2 /* minimum window size, degrees C */
static void set_thresholds(unsigned long cpu)
@@ -187,14 +186,18 @@ static void tau_timeout(void * info)
local_irq_restore(flags);
}
-static void tau_timeout_smp(struct timer_list *unused)
-{
+static struct workqueue_struct *tau_workq;
- /* schedule ourselves to be run again */
- mod_timer(_timer, jiffies + shrink_timer) ;
+static void tau_work_func(struct work_struct *work)
+{
+ msleep(shrink_timer);
on_each_cpu(tau_timeout, NULL, 0);
+ /* schedule ourselves to be run again */
+ queue_work(tau_workq, work);
}
+DECLARE_WORK(tau_work, tau_work_func);
+
/*
* setup the TAU
*
@@ -227,21 +230,16 @@ static int __init TAU_init(void)
return 1;
}
-
- /* first, set up the window shrinking timer */
- timer_setup(_timer, tau_timeout_smp, 0);
- tau_timer.expires = jiffies + shrink_timer;
- add_timer(_timer);
+ tau_workq = alloc_workqueue("tau", WQ_UNBOUND, 1, 0);
+ if (!tau_workq)
+ return -ENOMEM;
on_each_cpu(TAU_init_smp, NULL, 0);
- printk("Thermal assist unit ");
-#ifdef CONFIG_TAU_INT
- printk("using interrupts, ");
-#else
- printk("using timers, ");
-#endif
- printk("shrink_timer: %d jiffies\n", shrink_timer);
+ queue_work(tau_workq, _work);
+
+ pr_info("Thermal assist unit using %s, shrink_timer: %d ms\n",
+ IS_ENABLED(CONFIG_TAU_INT) ? "interrupts" : "workqueue",
shrink_timer);
tau_initialized = 1;
return 0;
--
2.26.2
[PATCH 3/5] powerpc/tau: Remove duplicated set_thresholds() call
The commentary at the call site seems to disagree with the code. The
conditional prevents calling set_thresholds() via the exception handler,
which appears to crash. Perhaps that's because it immediately triggers
another TAU exception. Anyway, calling set_thresholds() from TAUupdate()
is redundant because tau_timeout() does so.
Fixes: 1da177e4c3f41 ("Linux-2.6.12-rc2")
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
arch/powerpc/kernel/tau_6xx.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/arch/powerpc/kernel/tau_6xx.c b/arch/powerpc/kernel/tau_6xx.c
index 268205cc347da..b8d7e7d498e0a 100644
--- a/arch/powerpc/kernel/tau_6xx.c
+++ b/arch/powerpc/kernel/tau_6xx.c
@@ -110,11 +110,6 @@ static void TAUupdate(int cpu)
#ifdef DEBUG
printk("grew = %d\n", tau[cpu].grew);
#endif
-
-#ifndef CONFIG_TAU_INT /* tau_timeout will do this if not using interrupts */
- set_thresholds(cpu);
-#endif
-
}
#ifdef CONFIG_TAU_INT
--
2.26.2
[PATCH 1/5] powerpc/tau: Use appropriate temperature sample interval
According to the MPC750 Users Manual, the SITV value in Thermal
Management Register 3 is 13 bits long. The present code calculates the
SITV value as 60 * 500 cycles. This would overflow to give 10 us on
a 500 MHz CPU rather than the intended 60 us. (But according to the
Microprocessor Datasheet, there is also a factor of 266 that has to be
applied to this value on certain parts i.e. speed sort above 266 MHz.)
Always use the maximum cycle count, as recommended by the Datasheet.
Fixes: 1da177e4c3f41 ("Linux-2.6.12-rc2")
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
arch/powerpc/include/asm/reg.h | 2 +-
arch/powerpc/kernel/tau_6xx.c | 12
2 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 88e6c78100d9b..c750afc62887c 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -815,7 +815,7 @@
#define THRM1_TIN (1 << 31)
#define THRM1_TIV (1 << 30)
#define THRM1_THRES(x) ((x&0x7f)<<23)
-#define THRM3_SITV(x) ((x&0x3fff)<<1)
+#define THRM3_SITV(x) ((x & 0x1fff) << 1)
#define THRM1_TID (1<<2)
#define THRM1_TIE (1<<1)
#define THRM1_V(1<<0)
diff --git a/arch/powerpc/kernel/tau_6xx.c b/arch/powerpc/kernel/tau_6xx.c
index e2ab8a111b693..976d5bc1b5176 100644
--- a/arch/powerpc/kernel/tau_6xx.c
+++ b/arch/powerpc/kernel/tau_6xx.c
@@ -178,15 +178,11 @@ static void tau_timeout(void * info)
* complex sleep code needs to be added. One mtspr every time
* tau_timeout is called is probably not a big deal.
*
-* Enable thermal sensor and set up sample interval timer
-* need 20 us to do the compare.. until a nice 'cpu_speed' function
-* call is implemented, just assume a 500 mhz clock. It doesn't really
-* matter if we take too long for a compare since it's all interrupt
-* driven anyway.
-*
-* use a extra long time.. (60 us @ 500 mhz)
+* The "PowerPC 740 and PowerPC 750 Microprocessor Datasheet"
+* recommends that "the maximum value be set in THRM3 under all
+* conditions."
*/
- mtspr(SPRN_THRM3, THRM3_SITV(500*60) | THRM3_E);
+ mtspr(SPRN_THRM3, THRM3_SITV(0x1fff) | THRM3_E);
local_irq_restore(flags);
}
--
2.26.2
[PATCH 0/5] powerpc/tau: TAU driver fixes
This patch series fixes various bugs in the Thermal Assist Unit driver. It was tested on 266 MHz and 292 MHz PowerBook G3 laptops. Finn Thain (5): powerpc/tau: Use appropriate temperature sample interval powerpc/tau: Convert from timer to workqueue powerpc/tau: Remove duplicated set_thresholds() call powerpc/tau: Check processor type before enabling TAU interrupt powerpc/tau: Disable TAU between measurements arch/powerpc/include/asm/reg.h | 2 +- arch/powerpc/kernel/tau_6xx.c | 147 + arch/powerpc/platforms/Kconfig | 14 +--- 3 files changed, 62 insertions(+), 101 deletions(-) -- 2.26.2
Re: [PATCH 1/9] macintosh/via-macii: Access autopoll_devs when inside lock
On Sun, 9 Aug 2020, Guenter Roeck wrote: > Hi, > > On Sun, Jun 28, 2020 at 02:23:12PM +1000, Finn Thain wrote: > > The interrupt handler should be excluded when accessing the > > autopoll_devs variable. > > > > I am quite baffled by this patch. Other than adding an unnecessary lock > / unlock sequence, The new lock/unlock sequence means that the expression (autopoll_devs && !current_req) can be understood to be atomic. That makes it easier for me to follow (being that both variables are shared state). > accessing a variable (which is derived from another variable) from > inside or outside a lock does not make a difference. If autopoll_devs = > devs & 0xfffe is 0 inside the lock, it will just as much be 0 outside > the lock, and vice versa. > > Can you explain this in some more detail ? Not that is matters much > since the change already made it into mainline, but I would like to > understand what if anything I am missing here. > I think the new code is more readable and is obviously free of race conditions. It's not obvious to me why the old code was free of race conditions but if you can easily establish that by inspection then you are a better auditor than I am. Regardless, I'll stick with "Keep It Simple, Stupid".
Re: [PATCH 2/9] macintosh/via-macii: Poll the device most likely to respond
On Sun, 9 Aug 2020, Guenter Roeck wrote:
> Hi,
>
> On Sun, Jun 28, 2020 at 02:23:12PM +1000, Finn Thain wrote:
> > Poll the most recently polled device by default, rather than the lowest
> > device address that happens to be enabled in autopoll_devs. This improves
> > input latency. Re-use macii_queue_poll() rather than duplicate that logic.
> > This eliminates a static struct and function.
> >
> > Fixes: d95fd5fce88f0 ("m68k: Mac II ADB fixes") # v5.0+
> > Tested-by: Stan Johnson
> > Signed-off-by: Finn Thain
>
> With this patch applied, the qemu "q800" emulation no longer works and
> is stuck in early boot. Any idea why that might be the case, and/or how
> to debug it ?
>
The problem you're seeing was mentioned in the cover letter,
https://lore.kernel.org/linux-m68k/cover.1593318192.git.fth...@telegraphics.com.au/
Since this series was merged, Linus' tree is no longer compatible with
long-standing QEMU bugs.
The best way to fix this is to upgrade QEMU (latest is 5.1.0-rc3). Or use
the serial console instead of the framebuffer console.
I regret the inconvenience but the alternative was worse: adding code to
Linux to get compatibility with QEMU bugs (which were added to QEMU due to
Linux bugs).
My main concern is correct operation on actual hardware, as always. But
some QEMU developers are working on support for operating systems besides
Linux.
Therefore, I believe that both QEMU and Linux should aim for compatibility
with actual hardware and not bug compatibility with each other.
Re: [PATCH v2 13/16] scripts/kallsyms: move ignored symbol types to is_ignored_symbol()
On Mon, 20 Jul 2020, Masahiro Yamada wrote:
>
> I got a similar report before.
>
> I'd like to know whether or not
> this is the same issue as fixed by
> 7883a14339299773b2ce08dcfd97c63c199a9289
>
The problem can be observed with 3d77e6a8804ab ("Linux 5.7").
So it appears that 7883a14339299 ("scripts/kallsyms: fix wrong
kallsyms_relative_base") is not sufficient to fix it.
>
> Does your problem happen on the latest kernel?
Unfortunately this cross compiler (gcc 4.6.4) is too old to build
v5.8-rc1 or later. I will have to upgrade.
> Which version of binutils are you using?
>
This toolchain uses binutils 2.22.
In case it helps,
$ powerpc-linux-gnu-nm -n vmlinux |head
w mach_chrp
0005 a LG_CACHELINE_BYTES
0005 a LG_CACHELINE_BYTES
0005 a LG_CACHELINE_BYTES
000c a Hash_bits
001f a CACHELINE_MASK
001f a CACHELINE_MASK
001f a CACHELINE_MASK
0020 a CACHELINE_BYTES
0020 a CACHELINE_BYTES
0020 a CACHELINE_BYTES
0020 a reg
0003ffc0 a Hash_msk
c000 T _start
c000 A _stext
c000 A _text
c00c T __start
c054 t __after_mmu_off
c090 t turn_on_mmu
c0c4 T __secondary_hold
c0dc T __secondary_hold_spinloop
c0e0 T __secondary_hold_acknowledge
c100 t Reset
Re: [PATCH v2 13/16] scripts/kallsyms: move ignored symbol types to is_ignored_symbol()
On Sun, 24 Nov 2019, Masahiro Yamada wrote: > Collect the ignored patterns to is_ignored_symbol(). > > Signed-off-by: Masahiro Yamada This commit (887df76de67f5) caused a regression in my powerpc builds as it causes symbol names to disappear from backtraces: [ cut here ] WARNING: CPU: 0 PID: 0 at kernel/smp.c:433 _einittext+0x3f9e5120/0x3feb71b8 Modules linked in: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.4.0-rc7-pmac-00055-g887df76de67f5 #18 NIP: c00aef68 LR: c00af114 CTR: c001272c REGS: c0705c40 TRAP: 0700 Not tainted (5.4.0-rc7-pmac-00055-g887df76de67f5) MSR: 00029032 CR: 4244 XER: GPR00: 001f0100 c0705cf8 c06dc300 c070af1c c001258c ef7fb5bc GPR08: 0880 0100 0001 0100 4244 c0709040 0004 GPR16: 0001 c06022b4 c058297c 0022 8cb9 c06d84a0 c071 GPR24: c071 c070af1c c070af1c c001258c NIP [c00aef68] _einittext+0x3f9e5120/0x3feb71b8 LR [c00af114] _einittext+0x3f9e52cc/0x3feb71b8 Call Trace: [c0705cf8] [ef006320] 0xef006320 (unreliable) [c0705d38] [c00af114] _einittext+0x3f9e52cc/0x3feb71b8 [c0705d48] [c00af158] _einittext+0x3f9e5310/0x3feb71b8 [c0705d68] [c0012768] _einittext+0x3f948920/0x3feb71b8 [c0705d78] [c0092c04] _einittext+0x3f9c8dbc/0x3feb71b8 [c0705d88] [c0092d18] _einittext+0x3f9c8ed0/0x3feb71b8 [c0705da8] [c0093a2c] _einittext+0x3f9c9be4/0x3feb71b8 [c0705de8] [c0580224] _einittext+0x3feb63dc/0x3feb71b8 [c0705e48] [c00382ec] _einittext+0x3f96e4a4/0x3feb71b8 [c0705e58] [c000d2a0] _einittext+0x3f943458/0x3feb71b8 [c0705e88] [c001353c] _einittext+0x3f9496f4/0x3feb71b8 --- interrupt: 901 at _einittext+0x3f941058/0x3feb71b8 LR = _einittext+0x3f941058/0x3feb71b8 [c0705f50] [c06cc214] 0xc06cc214 (unreliable) [c0705f60] [c057fa20] _einittext+0x3feb5bd8/0x3feb71b8 [c0705f70] [c005de48] _einittext+0x3f994000/0x3feb71b8 [c0705f90] [c005e050] _einittext+0x3f994208/0x3feb71b8 [c0705fa0] [c0004cc8] _einittext+0x3f93ae80/0x3feb71b8 [c0705fb0] [c069a36c] _einittext+0x3ffd0524/0x4000 [c0705ff0] [3500] 0x3500 Instruction dump: 7c0803a6 7fa5eb78 7d808120 7ea6ab78 baa10014 38210040 4bfffbb0 7f64db78 7f85e378 484b31b1 7c601b78 4bfffdf4 <0fe0> 4bfffd60 9421ffe0 7c0802a6 ---[ end trace a06fef4788747c72 ]--- Prior to that (e.g. 97261e1e2240f), I get backtraces like this: [ cut here ] WARNING: CPU: 0 PID: 0 at kernel/smp.c:433 smp_call_function_many+0x318/0x320 Modules linked in: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.4.0-rc7-pmac-00054-g97261e1e2240f #20 NIP: c00aef68 LR: c00af114 CTR: c001272c REGS: c075dc40 TRAP: 0700 Not tainted (5.4.0-rc7-pmac-00054-g97261e1e2240f) MSR: 00029032 CR: 4244 XER: GPR00: 001f0100 c075dcf8 c0733300 c0762f1c c001258c ef7fb5bc GPR08: 0480 0100 0001 0100 4244 c0761040 0004 GPR16: 0001 c0658e58 c058297c 0022 8cb9 c072f4a0 c076 GPR24: c076 c0762f1c c0762f1c c001258c NIP [c00aef68] smp_call_function_many+0x318/0x320 LR [c00af114] smp_call_function+0x34/0x44 Call Trace: [c075dcf8] [ef006320] 0xef006320 (unreliable) [c075dd38] [c00af114] smp_call_function+0x34/0x44 [c075dd48] [c00af158] on_each_cpu+0x1c/0x4c [c075dd68] [c0012768] tau_timeout_smp+0x3c/0x4c [c075dd78] [c0092c04] call_timer_fn.isra.26+0x20/0x84 [c075dd88] [c0092d18] expire_timers+0xb0/0xc0 [c075dda8] [c0093a2c] run_timer_softirq+0xa4/0x1a4 [c075dde8] [c0580224] __do_softirq+0x11c/0x280 [c075de48] [c00382ec] irq_exit+0xc0/0xd4 [c075de58] [c000d2a0] timer_interrupt+0x154/0x260 [c075de88] [c001353c] ret_from_except+0x0/0x14 --- interrupt: 901 at arch_cpu_idle+0x24/0x78 LR = arch_cpu_idle+0x24/0x78 [c075df50] [c0723214] 0xc0723214 (unreliable) [c075df60] [c057fa20] default_idle_call+0x38/0x58 [c075df70] [c005de48] do_idle+0xd4/0x17c [c075df90] [c005e054] cpu_startup_entry+0x24/0x28 [c075dfa0] [c0004cc8] rest_init+0xa8/0xbc [c075dfb0] [c06f136c] start_kernel+0x40c/0x420 [c075dff0] [3500] 0x3500 Instruction dump: 7c0803a6 7fa5eb78 7d808120 7ea6ab78 baa10014 38210040 4bfffbb0 7f64db78 7f85e378 484b31b1 7c601b78 4bfffdf4 <0fe0> 4bfffd60 9421ffe0 7c0802a6 ---[ end trace 784c7f15ecd23941 ]--- Has anyone else observed these problems (either the WARNING from smp_call_function_many() or the missing symbol names)? What is the best way to fix this? Should I upgrade binutils?
[PATCH 4/9] macintosh/via-macii: Remove read_done state
The driver state machine may enter the 'read_done' state when leaving the
'idle' or 'reading' state. This transition is pointless, as is the extra
interrupt it requires. The interrupt is produced by the transceiver
(even when it has no data to send) because an extra EVEN/ODD toggle
was signalled by the driver. Drop the extra state to simplify the code.
Fixes: 1da177e4c3f41 ("Linux-2.6.12-rc2") # v5.0+
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
drivers/macintosh/via-macii.c | 70 ++-
1 file changed, 28 insertions(+), 42 deletions(-)
diff --git a/drivers/macintosh/via-macii.c b/drivers/macintosh/via-macii.c
index 6a5cd7de05baf..d29c87943ca46 100644
--- a/drivers/macintosh/via-macii.c
+++ b/drivers/macintosh/via-macii.c
@@ -110,7 +110,6 @@ static enum macii_state {
idle,
sending,
reading,
- read_done,
} macii_state;
static struct adb_request *current_req; /* first request struct in the queue */
@@ -411,8 +410,8 @@ static irqreturn_t macii_interrupt(int irq, void *arg)
reply_len = 1;
} else {
/* bus timeout */
- macii_state = read_done;
reply_len = 0;
+ break;
}
/* set ADB state = even for first data byte */
@@ -471,20 +470,6 @@ static irqreturn_t macii_interrupt(int irq, void *arg)
current_req = req->next;
if (req->done)
(*req->done)(req);
-
- if (!current_req)
- macii_queue_poll();
-
- if (current_req && macii_state == idle)
- macii_start();
-
- if (macii_state == idle) {
- /* reset to shift in */
- via[ACR] &= ~SR_OUT;
- x = via[SR];
- /* set ADB state idle - might get SRQ */
- via[B] = (via[B] & ~ST_MASK) | ST_IDLE;
- }
break;
}
} else {
@@ -511,12 +496,28 @@ static irqreturn_t macii_interrupt(int irq, void *arg)
} else if (status == ST_ODD && reply_len == 2) {
srq_asserted = true;
} else {
- macii_state = read_done;
+ macii_state = idle;
+
+ if (bus_timeout)
+ reply_len = 0;
+
+ if (reading_reply) {
+ struct adb_request *req = current_req;
+
+ req->reply_len = reply_len;
+
+ req->complete = 1;
+ current_req = req->next;
+ if (req->done)
+ (*req->done)(req);
+ } else if (reply_len && autopoll_devs) {
+ adb_input(reply_buf, reply_len, 0);
+ }
+ break;
}
}
- if (macii_state == reading &&
- reply_len < ARRAY_SIZE(reply_buf)) {
+ if (reply_len < ARRAY_SIZE(reply_buf)) {
reply_ptr++;
*reply_ptr = x;
reply_len++;
@@ -526,37 +527,22 @@ static irqreturn_t macii_interrupt(int irq, void *arg)
via[B] ^= ST_MASK;
break;
- case read_done:
- x = via[SR];
-
- if (bus_timeout)
- reply_len = 0;
-
- if (reading_reply) {
- reading_reply = 0;
- req = current_req;
- req->reply_len = reply_len;
- req->complete = 1;
- current_req = req->next;
- if (req->done)
- (*req->done)(req);
- } else if (reply_len && autopoll_devs)
- adb_input(reply_buf, reply_len, 0);
-
- macii_state = idle;
+ default:
+ break;
+ }
+ if (macii_state == idle) {
if (!current_req)
macii_queue_poll();
if (current_req)
macii_start();
- if (macii_state == idle)
+
[PATCH 9/9] macintosh/via-macii: Clarify definition of macii_init()
The function prototype correctly specifies the 'static' storage class.
Let the function definition match the declaration for better readability.
Signed-off-by: Finn Thain
---
drivers/macintosh/via-macii.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/macintosh/via-macii.c b/drivers/macintosh/via-macii.c
index 2f9be4ec7d345..060e03f2264bc 100644
--- a/drivers/macintosh/via-macii.c
+++ b/drivers/macintosh/via-macii.c
@@ -140,7 +140,7 @@ static int macii_probe(void)
}
/* Initialize the driver */
-int macii_init(void)
+static int macii_init(void)
{
unsigned long flags;
int err;
--
2.26.2
[PATCH 0/9] Macintosh II ADB driver fixes
Various issues with the via-macii driver have become apparent over the years. Some examples: - A Talk command response can be lost. This can result in phantom devices being probed or an incorrect device handler ID being retrieved. - A reply packet containing a null byte can get truncated. Such packets are sometimes generated by ADB keyboards. - A Talk Register 3 reply from device 15 (that is, command byte 0xFF) can be mistaken for a bus timeout (empty packet). This patch series contains fixes for all known bugs in the via-macii driver, plus a few code style improvements. It has been successfully tested on an Apple Centris 650 and qemu-system-m68k. The patched kernel does regress on past QEMU releases, due to ADB transceiver emulation bugs. Those bugs have been fixed in mainline QEMU. My thanks go to Mark Cave-Ayland for that effort and for figuring out the improvements to the signalling between the VIA and the transceiver. Note to -stable maintainers: these fixes can be cherry-picked without difficulty, if you have the 5 commits that appeared in v5.0: b52dce8738938 macintosh/via-macii: Synchronous bus reset 5f93d7081a47e macintosh/via-macii: Remove BUG_ON assertions 5ce6185c2ef4e macintosh/via-macii: Simplify locking 351e5ad327d07 macintosh/via-macii, macintosh/adb-iop: Modernize printk calls 47fd2060660e6 macintosh/via-macii, macintosh/adb-iop: Clean up whitespace Just for the sake of simplicity, the 'fixes' tags in this series limit backporting to 'v5.0+'. Finn Thain (9): macintosh/via-macii: Access autopoll_devs when inside lock macintosh/via-macii: Poll the device most likely to respond macintosh/via-macii: Handle /CTLR_IRQ signal correctly macintosh/via-macii: Remove read_done state macintosh/via-macii: Handle poll replies correctly macintosh/via-macii: Use bool type for reading_reply variable macintosh/via-macii: Use unsigned type for autopoll_devs variable macintosh/via-macii: Use the stack for reset request storage macintosh/via-macii: Clarify definition of macii_init() drivers/macintosh/via-macii.c | 324 +++--- 1 file changed, 179 insertions(+), 145 deletions(-) -- 2.26.2
[PATCH 8/9] macintosh/via-macii: Use the stack for reset request storage
The adb_request struct can be stored on the stack because the request
is synchronous and is completed before the function returns.
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
drivers/macintosh/via-macii.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/macintosh/via-macii.c b/drivers/macintosh/via-macii.c
index 447273967e1e8..2f9be4ec7d345 100644
--- a/drivers/macintosh/via-macii.c
+++ b/drivers/macintosh/via-macii.c
@@ -313,7 +313,7 @@ static void macii_poll(void)
/* Reset the bus */
static int macii_reset_bus(void)
{
- static struct adb_request req;
+ struct adb_request req;
/* Command = 0, Address = ignored */
adb_request(, NULL, ADBREQ_NOSEND, 1, ADB_BUSRESET);
--
2.26.2
[PATCH 7/9] macintosh/via-macii: Use unsigned type for autopoll_devs variable
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
drivers/macintosh/via-macii.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/macintosh/via-macii.c b/drivers/macintosh/via-macii.c
index e143ddb81de34..447273967e1e8 100644
--- a/drivers/macintosh/via-macii.c
+++ b/drivers/macintosh/via-macii.c
@@ -125,7 +125,7 @@ static bool srq_asserted;/* have to poll for the device
that asserted it */
static u8 last_cmd; /* the most recent command byte transmitted */
static u8 last_talk_cmd;/* the most recent Talk command byte transmitted */
static u8 last_poll_cmd; /* the most recent Talk R0 command byte transmitted */
-static int autopoll_devs; /* bits set are device addresses to be polled */
+static unsigned int autopoll_devs; /* bits set are device addresses to poll */
/* Check for MacII style ADB */
static int macii_probe(void)
@@ -291,7 +291,7 @@ static int macii_autopoll(int devs)
local_irq_save(flags);
/* bit 1 == device 1, and so on. */
- autopoll_devs = devs & 0xFFFE;
+ autopoll_devs = (unsigned int)devs & 0xFFFE;
if (!current_req) {
macii_queue_poll();
--
2.26.2
[PATCH 6/9] macintosh/via-macii: Use bool type for reading_reply variable
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
drivers/macintosh/via-macii.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/macintosh/via-macii.c b/drivers/macintosh/via-macii.c
index 8d5ef77b4a435..e143ddb81de34 100644
--- a/drivers/macintosh/via-macii.c
+++ b/drivers/macintosh/via-macii.c
@@ -116,7 +116,7 @@ static struct adb_request *current_req; /* first request
struct in the queue */
static struct adb_request *last_req; /* last request struct in the queue */
static unsigned char reply_buf[16];/* storage for autopolled replies */
static unsigned char *reply_ptr; /* next byte in reply_buf or req->reply */
-static int reading_reply;/* store reply in reply_buf else req->reply */
+static bool reading_reply; /* store reply in reply_buf else req->reply */
static int data_index; /* index of the next byte to send from req->data */
static int reply_len; /* number of bytes received in reply_buf or req->reply */
static int status; /* VIA's ADB status bits captured upon interrupt */
@@ -394,7 +394,7 @@ static irqreturn_t macii_interrupt(int irq, void *arg)
WARN_ON((status & ST_MASK) != ST_IDLE);
reply_ptr = reply_buf;
- reading_reply = 0;
+ reading_reply = false;
bus_timeout = false;
srq_asserted = false;
@@ -442,7 +442,7 @@ static irqreturn_t macii_interrupt(int irq, void *arg)
*/
macii_state = reading;
- reading_reply = 0;
+ reading_reply = false;
reply_ptr = reply_buf;
*reply_ptr = last_talk_cmd;
reply_len = 1;
@@ -456,7 +456,7 @@ static irqreturn_t macii_interrupt(int irq, void *arg)
if (req->reply_expected) {
macii_state = reading;
- reading_reply = 1;
+ reading_reply = true;
reply_ptr = req->reply;
*reply_ptr = req->data[1];
reply_len = 1;
@@ -466,7 +466,7 @@ static irqreturn_t macii_interrupt(int irq, void *arg)
} else if ((req->data[1] & OP_MASK) == TALK) {
macii_state = reading;
- reading_reply = 0;
+ reading_reply = false;
reply_ptr = reply_buf;
*reply_ptr = req->data[1];
reply_len = 1;
--
2.26.2
[PATCH 5/9] macintosh/via-macii: Handle poll replies correctly
Userspace applications may use /dev/adb to send Talk requests. Such
requests always have req->reply_expected == 1. The same is true of Talk
requests sent by the kernel, except for poll requests queued internally
by the via-macii driver. Those requests have req->reply_expected == 0.
Consequently, poll reply packets get treated like autopoll reply packets.
(It doesn't make sense to try to distinguish them.) Always enter 'reading'
state after a poll request, so that the reply gets collected and passed
to adb_input(), and none go missing.
All Talk replies passed to adb_input() come from polling or autopolling,
so call adb_input() with the autopoll parameter set to 1.
Fixes: d95fd5fce88f0 ("m68k: Mac II ADB fixes") # v5.0+
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
drivers/macintosh/via-macii.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/macintosh/via-macii.c b/drivers/macintosh/via-macii.c
index d29c87943ca46..8d5ef77b4a435 100644
--- a/drivers/macintosh/via-macii.c
+++ b/drivers/macintosh/via-macii.c
@@ -463,6 +463,21 @@ static irqreturn_t macii_interrupt(int irq, void *arg)
via[ACR] &= ~SR_OUT;
x = via[SR];
+ } else if ((req->data[1] & OP_MASK) == TALK) {
+ macii_state = reading;
+
+ reading_reply = 0;
+ reply_ptr = reply_buf;
+ *reply_ptr = req->data[1];
+ reply_len = 1;
+
+ via[ACR] &= ~SR_OUT;
+ x = via[SR];
+
+ req->complete = 1;
+ current_req = req->next;
+ if (req->done)
+ (*req->done)(req);
} else {
macii_state = idle;
@@ -510,8 +525,9 @@ static irqreturn_t macii_interrupt(int irq, void *arg)
current_req = req->next;
if (req->done)
(*req->done)(req);
- } else if (reply_len && autopoll_devs) {
- adb_input(reply_buf, reply_len, 0);
+ } else if (reply_len && autopoll_devs &&
+ reply_buf[0] == last_poll_cmd) {
+ adb_input(reply_buf, reply_len, 1);
}
break;
}
--
2.26.2
[PATCH 3/9] macintosh/via-macii: Handle /CTLR_IRQ signal correctly
I'm told that the /CTLR_IRQ signal from the ADB transceiver gets
interpreted by MacOS to mean SRQ, bus timeout or end-of-packet depending
on the circumstances, and that Linux's via-macii driver does not
correctly interpret this signal.
Instead, the via-macii driver interprets certain received byte values
(0x00 and 0xFF) as signalling end of packet and bus timeout
(respectively). Problem is, those values can also appear under other
circumstances.
This patch changes the bus timeout, end of packet and SRQ detection logic
to bring it closer to the logic that MacOS reportedly uses.
Fixes: 1da177e4c3f41 ("Linux-2.6.12-rc2") # v5.0+
Reported-by: Mark Cave-Ayland
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
drivers/macintosh/via-macii.c | 166 --
1 file changed, 97 insertions(+), 69 deletions(-)
diff --git a/drivers/macintosh/via-macii.c b/drivers/macintosh/via-macii.c
index d4f1a65c5f1fd..6a5cd7de05baf 100644
--- a/drivers/macintosh/via-macii.c
+++ b/drivers/macintosh/via-macii.c
@@ -80,6 +80,8 @@ static volatile unsigned char *via;
/* ADB command byte structure */
#define ADDR_MASK 0xF0
#define CMD_MASK 0x0F
+#define OP_MASK0x0C
+#define TALK 0x0C
static int macii_init_via(void);
static void macii_start(void);
@@ -119,9 +121,10 @@ static int reading_reply;/* store reply in
reply_buf else req->reply */
static int data_index; /* index of the next byte to send from req->data */
static int reply_len; /* number of bytes received in reply_buf or req->reply */
static int status; /* VIA's ADB status bits captured upon interrupt */
-static int last_status; /* status bits as at previous interrupt */
-static int srq_asserted; /* have to poll for the device that asserted it */
+static bool bus_timeout; /* no data was sent by the device */
+static bool srq_asserted;/* have to poll for the device that asserted it */
static u8 last_cmd; /* the most recent command byte transmitted */
+static u8 last_talk_cmd;/* the most recent Talk command byte transmitted */
static u8 last_poll_cmd; /* the most recent Talk R0 command byte transmitted */
static int autopoll_devs; /* bits set are device addresses to be polled */
@@ -170,7 +173,6 @@ static int macii_init_via(void)
/* Set up state: idle */
via[B] |= ST_IDLE;
- last_status = via[B] & (ST_MASK | CTLR_IRQ);
/* Shift register on input */
via[ACR] = (via[ACR] & ~SR_CTRL) | SR_EXT;
@@ -336,13 +338,6 @@ static void macii_start(void)
* And req->nbytes is the number of bytes of real data plus one.
*/
- /* store command byte */
- last_cmd = req->data[1];
-
- /* If this is a Talk Register 0 command, store the command byte */
- if ((last_cmd & CMD_MASK) == ADB_READREG(0, 0))
- last_poll_cmd = last_cmd;
-
/* Output mode */
via[ACR] |= SR_OUT;
/* Load data */
@@ -352,6 +347,9 @@ static void macii_start(void)
macii_state = sending;
data_index = 2;
+
+ bus_timeout = false;
+ srq_asserted = false;
}
/*
@@ -360,15 +358,17 @@ static void macii_start(void)
* generating shift register interrupts (SR_INT) for us. This means there has
* to be activity on the ADB bus. The chip will poll to achieve this.
*
- * The basic ADB state machine was left unchanged from the original MacII code
- * by Alan Cox, which was based on the CUDA driver for PowerMac.
- * The syntax of the ADB status lines is totally different on MacII,
- * though. MacII uses the states Command -> Even -> Odd -> Even ->...-> Idle
- * for sending and Idle -> Even -> Odd -> Even ->...-> Idle for receiving.
- * Start and end of a receive packet are signalled by asserting /IRQ on the
- * interrupt line (/IRQ means the CTLR_IRQ bit in port B; not to be confused
- * with the VIA shift register interrupt. /IRQ never actually interrupts the
- * processor, it's just an ordinary input.)
+ * The VIA Port B output signalling works as follows. After the ADB transceiver
+ * sees a transition on the PB4 and PB5 lines it will crank over the VIA shift
+ * register which eventually raises the SR_INT interrupt. The PB4/PB5 outputs
+ * are toggled with each byte as the ADB transaction progresses.
+ *
+ * Request with no reply expected (and empty transceiver buffer):
+ * CMD -> IDLE
+ * Request with expected reply packet (or with buffered autopoll packet):
+ * CMD -> EVEN -> ODD -> EVEN -> ... -> IDLE
+ * Unsolicited packet:
+ * IDLE -> EVEN -> ODD -> EVEN -> ... -> IDLE
*/
static irqreturn_t macii_interrupt(int irq, void *arg)
{
@@ -388,31 +388,31 @@ static irqreturn_t macii_interrupt(int irq, void *arg)
}
}
- last_status = status;
status = via[B] & (ST
[PATCH 2/9] macintosh/via-macii: Poll the device most likely to respond
Poll the most recently polled device by default, rather than the lowest
device address that happens to be enabled in autopoll_devs. This improves
input latency. Re-use macii_queue_poll() rather than duplicate that logic.
This eliminates a static struct and function.
Fixes: d95fd5fce88f0 ("m68k: Mac II ADB fixes") # v5.0+
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
drivers/macintosh/via-macii.c | 99 +++
1 file changed, 53 insertions(+), 46 deletions(-)
diff --git a/drivers/macintosh/via-macii.c b/drivers/macintosh/via-macii.c
index 6aa903529570d..d4f1a65c5f1fd 100644
--- a/drivers/macintosh/via-macii.c
+++ b/drivers/macintosh/via-macii.c
@@ -77,6 +77,10 @@ static volatile unsigned char *via;
#define ST_ODD 0x20/* ADB state: odd data byte */
#define ST_IDLE0x30/* ADB state: idle, nothing to
send */
+/* ADB command byte structure */
+#define ADDR_MASK 0xF0
+#define CMD_MASK 0x0F
+
static int macii_init_via(void);
static void macii_start(void);
static irqreturn_t macii_interrupt(int irq, void *arg);
@@ -117,7 +121,8 @@ static int reply_len; /* number of bytes received in
reply_buf or req->reply */
static int status; /* VIA's ADB status bits captured upon interrupt */
static int last_status; /* status bits as at previous interrupt */
static int srq_asserted; /* have to poll for the device that asserted it */
-static int command_byte; /* the most recent command byte transmitted */
+static u8 last_cmd; /* the most recent command byte transmitted */
+static u8 last_poll_cmd; /* the most recent Talk R0 command byte transmitted */
static int autopoll_devs; /* bits set are device addresses to be polled */
/* Check for MacII style ADB */
@@ -179,35 +184,49 @@ static int macii_init_via(void)
/* Send an ADB poll (Talk Register 0 command prepended to the request queue) */
static void macii_queue_poll(void)
{
- /* No point polling the active device as it will never assert SRQ, so
-* poll the next device in the autopoll list. This could leave us
-* stuck in a polling loop if an unprobed device is asserting SRQ.
-* In theory, that could only happen if a device was plugged in after
-* probing started. Unplugging it again will break the cycle.
-* (Simply polling the next higher device often ends up polling almost
-* every device (after wrapping around), which takes too long.)
-*/
- int device_mask;
- int next_device;
static struct adb_request req;
+ unsigned char poll_command;
+ unsigned int poll_addr;
+ /* This only polls devices in the autopoll list, which assumes that
+* unprobed devices never assert SRQ. That could happen if a device was
+* plugged in after the adb bus scan. Unplugging it again will resolve
+* the problem. This behaviour is similar to MacOS.
+*/
if (!autopoll_devs)
return;
- device_mask = (1 << (((command_byte & 0xF0) >> 4) + 1)) - 1;
- if (autopoll_devs & ~device_mask)
- next_device = ffs(autopoll_devs & ~device_mask) - 1;
- else
- next_device = ffs(autopoll_devs) - 1;
+ /* The device most recently polled may not be the best device to poll
+* right now. Some other device(s) may have signalled SRQ (the active
+* device won't do that). Or the autopoll list may have been changed.
+* Try polling the next higher address.
+*/
+ poll_addr = (last_poll_cmd & ADDR_MASK) >> 4;
+ if ((srq_asserted && last_cmd == last_poll_cmd) ||
+ !(autopoll_devs & (1 << poll_addr))) {
+ unsigned int higher_devs;
+
+ higher_devs = autopoll_devs & -(1 << (poll_addr + 1));
+ poll_addr = ffs(higher_devs ? higher_devs : autopoll_devs) - 1;
+ }
- adb_request(, NULL, ADBREQ_NOSEND, 1, ADB_READREG(next_device, 0));
+ /* Send a Talk Register 0 command */
+ poll_command = ADB_READREG(poll_addr, 0);
+
+ /* No need to repeat this Talk command. The transceiver will do that
+* as long as it is idle.
+*/
+ if (poll_command == last_cmd)
+ return;
+
+ adb_request(, NULL, ADBREQ_NOSEND, 1, poll_command);
req.sent = 0;
req.complete = 0;
req.reply_len = 0;
req.next = current_req;
- if (current_req != NULL) {
+ if (WARN_ON(current_req)) {
current_req =
} else {
current_req =
@@ -266,37 +285,22 @@ static int macii_write(struct adb_request *req)
/* Start auto-polling */
static int macii_autopoll(int devs)
{
- static struct adb_request req;
unsigned long flags;
- int err = 0;
local_irq_save(flags);
[PATCH 1/9] macintosh/via-macii: Access autopoll_devs when inside lock
The interrupt handler should be excluded when accessing the autopoll_devs
variable.
Fixes: d95fd5fce88f0 ("m68k: Mac II ADB fixes") # v5.0+
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
drivers/macintosh/via-macii.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/macintosh/via-macii.c b/drivers/macintosh/via-macii.c
index ac824d7b2dcfc..6aa903529570d 100644
--- a/drivers/macintosh/via-macii.c
+++ b/drivers/macintosh/via-macii.c
@@ -270,15 +270,12 @@ static int macii_autopoll(int devs)
unsigned long flags;
int err = 0;
+ local_irq_save(flags);
+
/* bit 1 == device 1, and so on. */
autopoll_devs = devs & 0xFFFE;
- if (!autopoll_devs)
- return 0;
-
- local_irq_save(flags);
-
- if (current_req == NULL) {
+ if (autopoll_devs && !current_req) {
/* Send a Talk Reg 0. The controller will repeatedly transmit
* this as long as it is idle.
*/
--
2.26.2
Re: [PATCH] scsi: target/sbp: remove firewire SBP target driver
On Tue, 16 Jun 2020, Martin K. Petersen wrote: > > However, keeping code around is not free. Right. And removing code isn't free either, if it forces people to find workarounds. > Core interfaces change frequently. Nobody enjoys having to tweak host > templates for 50 devices they have never even heard about. And yet some people seem to enjoy writing patches that are as trivial as they are invasive... You seem to be making an argument for more automation here, not an argument for less code. Or is there some upper bound to the size of the kernel, that might be lifted by adding maintainers? (Can you deliver a better product by adding more developers to your project?) > Also, we now live in a reality where there is a constant barrage of > build bots and code analyzers sending mail. So the effective cost of > keeping code around in the tree is going up. But if maintenance cost rises due to good analysis, the value of the code should rise too. So what's the problem? It seems to me that the real problem is too many analyses that generate pedantic noise and no tangible improvement in code quality or value. > I get a substantial amount of code analysis mail about drivers nobody > have touched in a decade or more. > When stable, mature code fails analysis, the analysis is also questionable (in the absence of real examples). > Consequently, I am much more inclined to remove drivers than I have been > in the past. But I am also very happy to bring them back if somebody > uses them or - even better - are willing to step up and maintain them. > You seem to be saying that 1) a driver should be removed when it no longer meets the present threshold for code quality and 2) that a low quality driver is eligible for re-addition because someone wants to use it. I don't think you can have it both ways. > I don't particularly like the notion of a driver being orphaned because > all that really means is that the driver transitions from being (at > least partially) somebody else's problem to being mine and mine alone. > Yes it's your problem but only on a best-effort basis. Many issues detected by automatic analyzers can be fixed with automatic code transformation tools. This kind of solution works tree-wide, so even if some defect in your driver is "yours and yours alone", the solution will probably come from others. This email, like yours, is just hand-waving. So feel free to ignore it or (preferably) provide evidence of real defects.
Re: [PATCH] scsi: target/sbp: remove firewire SBP target driver
On Tue, 16 Jun 2020, Martin K. Petersen wrote: > > I haven't used this driver for a long time, but I still own PowerMacs > > with firewire, and I know I'm not the only one. > I need to correct what I wrote above. I recall that years ago, when I needed to share storage from my Linux box to my PowerBook pismo, I used ethernet and the globalSAN iSCSI initiator for Mac OS X (which is no longer freely available AFAICS). When I said that I had used the SBP target driver before, I misremembered that iSCSI target setup. I've actually never used the SBP target driver. > I also have old 1394 hardware kicking around in the basement. But having > worked with FireWire shared storage targets in the past, I have zero > desire to ever touch any of that again. > > I could understand an objection if we were to entertain removing sbp2. > But really, how many people are setting up FireWire targets? > It's a good question. I don't know the answer. I have successfully used the Linux sbp2 host driver in the past, and will probably need to use it again. Likewise, I can see myself using the sbp target driver in the future because that might interoperate with MacOS 9, and might provide a bootable device to the PowerMac ROM. iSCSI cannot do those things.
Re: [PATCH] scsi: target/sbp: remove firewire SBP target driver
On Tue, 16 Jun 2020, Bart Van Assche wrote: > > As far as I know the sbp driver only has had one user ever and that user > is no longer user the sbp driver. So, you estimate the userbase at zero. Can you give a confidence level? Actual measurement is hard because when end users encounter breakage, they look for quick workarounds before they undertake post mortem, log collection, bug reporting, mailing list discussions, analysis etc. > So why to keep it in the kernel tree? Answer: for the same reason it was added to the tree. Here's a different question: "Why remove it from the kernel tree?" If maintaining this code is a burden, is it not the kind of tax that all developers/users pay to all developers/users? Does this driver impose an unreasonably high burden for some reason? The growth of a maintenance burden in general has lead to the invention of design patterns and tooling to minize it. So a good argument for removal would describe the nature of the problem, because some driver deficiencies can be fixed automatically, and some tooling deficiencies can compound an otherwise insignificant or common driver deficiency. There are spin-off benefits from legacy code besides process improvements. Building and testing this sort of code has regularly revealed erroneous corner cases in commits elsewhere like API changes and refactoring. Also, legacy code is used by new developers get experience in code modernization. And it provides more training material for neural networks that need to be taught to recognize patches that raise quality. Ten or twenty years ago, I doubt that anyone predicted these (and other) spin-off benefits. If we can't predict the benefit, how will we project the cost, and use that to justify deletion? Please see also, http://www.mac.linux-m68k.org/docs/obsolete.php
Re: [PATCH] scsi: target/sbp: remove firewire SBP target driver
On Mon, 15 Jun 2020, Chris Boot wrote: > On 15/06/2020 00:28, Finn Thain wrote: > > On Sun, 14 Jun 2020, Chris Boot wrote: > > > >> I expect that if someone finds this useful it can stick around (but > >> that's not my call). > > > > Who's call is that? If the patch had said "From: Martin K. Petersen" > > and "This driver is being removed because it has the following > > defects..." that would be some indication of a good-faith willingness > > to accept users as developers in the spirit of the GPL, which is what > > you seem to be alluding to (?). > > If you're asking me, I'd say it was martin's call: > > > SCSI TARGET SUBSYSTEM > > > > M: "Martin K. Petersen" > > > [...] > > F: drivers/target/ > > > > F: include/target/ > > > The question I asked you was intended to make you think. I wasn't asking you to search MAINTAINERS for "drivers/target" (I had already done so). Chris, you can find my name in that file too. That's because I see my role as custodian of that particular code. That code lives in the kernel.org tree because others put it there and because users find it useful -- not merely because it happens to please the official glorious MAINTAINER of said code. If you would ask, "who's call is it to delete drivers/nubus? or drivers/scsi/NCR5380.c?" my answer is, I have no idea. > >> I just don't have the time or inclination or hardware to be able to > >> maintain it anymore, so someone else would have to pick it up. > >> > > > > Which is why most drivers get orphaned, right? > > Sure, but that's not what Martin asked me to do, hence this patch. > Martin said, "I'd appreciate a patch to remove it" And Bart said, "do you want to keep this driver in the kernel tree?" AFAICT both comments are quite ambiguous. I don't see an actionable request, just an expression of interest from people doing their jobs. Note well: there is no pay check associated with having a MAINTAINERS file entry.
Re: [PATCH] scsi: target/sbp: remove firewire SBP target driver
On Sun, 14 Jun 2020, Chris Boot wrote: > I expect that if someone finds this useful it can stick around (but > that's not my call). Who's call is that? If the patch had said "From: Martin K. Petersen" and "This driver is being removed because it has the following defects..." that would be some indication of a good-faith willingness to accept users as developers in the spirit of the GPL, which is what you seem to be alluding to (?). > I just don't have the time or inclination or hardware to be able to > maintain it anymore, so someone else would have to pick it up. > Which is why most drivers get orphaned, right?
Re: [PATCH] scsi: target/sbp: remove firewire SBP target driver
On Sat, 13 Jun 2020, Chris Boot wrote: > I no longer have the time to maintain this subsystem nor the hardware to > test patches with. Then why not patch MAINTAINERS, and orphan it, as per usual practice? $ git log --oneline MAINTAINERS | grep -i orphan > It also doesn't appear to have any active users so I doubt anyone will > miss it. > It's not unusual that any Linux driver written more than 5 years ago "doesn't appear to have any active users". If a driver has been orphaned and broken in the past, and no-one stepped up to fix it within a reasonable period, removal would make sense. But that's not the case here. I haven't used this driver for a long time, but I still own PowerMacs with firewire, and I know I'm not the only one. > Signed-off-by: Chris Boot > --- > MAINTAINERS |9 - > drivers/target/Kconfig |1 - > drivers/target/Makefile |1 - > drivers/target/sbp/Kconfig | 12 - > drivers/target/sbp/Makefile |2 - > drivers/target/sbp/sbp_target.c | 2350 --- > drivers/target/sbp/sbp_target.h | 243 > 7 files changed, 2618 deletions(-) > delete mode 100644 drivers/target/sbp/Kconfig > delete mode 100644 drivers/target/sbp/Makefile > delete mode 100644 drivers/target/sbp/sbp_target.c > delete mode 100644 drivers/target/sbp/sbp_target.h >
Re: [PATCH 8/8] macintosh/adb-iop: Implement SRQ autopolling
On Mon, 1 Jun 2020, Geert Uytterhoeven wrote: > > > > Sure, it could be absorbed by both asm/mac_iop.h and > > drivers/macintosh/adb-iop.c [...] > > asm/mac_iop.h doesn't include asm/adb_iop.h (at least not in my tree, > but perhaps you have plans to change that?), so there's only a single > user. What I meant by "both" was that part of asm/adb_iop.h could be absorbed by drivers/macintosh.adb-iop.c and the rest by asm/mac_iop.h. (And some of it could be tossed out.) I suspect that much of arch/m68k/include/asm could get the same treatment. But I doubt that there is any pay off, because the headers rarely change where they relate to hardware characteristics.
Re: [PATCH 8/8] macintosh/adb-iop: Implement SRQ autopolling
On Sun, 31 May 2020, Geert Uytterhoeven wrote: > Hi Finn, > > On Sun, May 31, 2020 at 1:20 AM Finn Thain wrote: > > The adb_driver.autopoll method is needed during ADB bus scan and device > > address assignment. Implement this method so that the IOP's list of > > device addresses can be updated. When the list is empty, disable SRQ > > autopolling. > > > > Cc: Joshua Thompson > > Cc: Geert Uytterhoeven > > Tested-by: Stan Johnson > > Signed-off-by: Finn Thain > > Thanks for your patch! > > Acked-by: Geert Uytterhoeven > Thanks for your tag. > > arch/m68k/include/asm/adb_iop.h | 1 + > > drivers/macintosh/adb-iop.c | 32 ++-- > > As this header file is used by a single source file only, perhaps it > should just be absorbed by the latter? Sure, it could be absorbed by both asm/mac_iop.h and drivers/macintosh/adb-iop.c but I don't see the point... > Then you no longer need my Acked-by for future changes ;-) > But you shouldn't need to ack this kind of change in the first place. IMHO, the arch/m68k/mac maintainer should be the one to ack changes to both arch/m68k/include/asm/adb_iop.h and drivers/macintosh/adb-iop.c. Not that I'm complaining (thanks again for your ack!)
[PATCH 5/8] macintosh/adb-iop: Resolve static checker warnings
drivers/macintosh/adb-iop.c:215:28: warning: Using plain integer as NULL pointer
drivers/macintosh/adb-iop.c:170:5: warning: symbol 'adb_iop_probe' was not
declared. Should it be static?
drivers/macintosh/adb-iop.c:177:5: warning: symbol 'adb_iop_init' was not
declared. Should it be static?
drivers/macintosh/adb-iop.c:184:5: warning: symbol 'adb_iop_send_request' was
not declared. Should it be static?
drivers/macintosh/adb-iop.c:230:5: warning: symbol 'adb_iop_autopoll' was not
declared. Should it be static?
drivers/macintosh/adb-iop.c:236:6: warning: symbol 'adb_iop_poll' was not
declared. Should it be static?
drivers/macintosh/adb-iop.c:241:5: warning: symbol 'adb_iop_reset_bus' was not
declared. Should it be static?
Cc: Joshua Thompson
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
drivers/macintosh/adb-iop.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/macintosh/adb-iop.c b/drivers/macintosh/adb-iop.c
index 7ecc41bc7358..a2b28e09f2ce 100644
--- a/drivers/macintosh/adb-iop.c
+++ b/drivers/macintosh/adb-iop.c
@@ -166,21 +166,21 @@ static void adb_iop_start(void)
adb_iop_complete);
}
-int adb_iop_probe(void)
+static int adb_iop_probe(void)
{
if (!iop_ism_present)
return -ENODEV;
return 0;
}
-int adb_iop_init(void)
+static int adb_iop_init(void)
{
pr_info("adb: IOP ISM driver v0.4 for Unified ADB\n");
iop_listen(ADB_IOP, ADB_CHAN, adb_iop_listen, "ADB");
return 0;
}
-int adb_iop_send_request(struct adb_request *req, int sync)
+static int adb_iop_send_request(struct adb_request *req, int sync)
{
int err;
@@ -211,7 +211,7 @@ static int adb_iop_write(struct adb_request *req)
local_irq_save(flags);
- if (current_req != 0) {
+ if (current_req) {
last_req->next = req;
last_req = req;
} else {
@@ -227,18 +227,18 @@ static int adb_iop_write(struct adb_request *req)
return 0;
}
-int adb_iop_autopoll(int devs)
+static int adb_iop_autopoll(int devs)
{
/* TODO: how do we enable/disable autopoll? */
return 0;
}
-void adb_iop_poll(void)
+static void adb_iop_poll(void)
{
iop_ism_irq_poll(ADB_IOP);
}
-int adb_iop_reset_bus(void)
+static int adb_iop_reset_bus(void)
{
struct adb_request req;
--
2.26.2
[PATCH 7/8] macintosh/adb-iop: Implement sending -> idle state transition
On leaving the 'sending' state, proceed to the 'idle' state if no reply is
expected. Drop redundant test for adb_iop_state == sending && current_req.
Cc: Joshua Thompson
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
drivers/macintosh/adb-iop.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/macintosh/adb-iop.c b/drivers/macintosh/adb-iop.c
index f22792c95d4f..8594e4f9a830 100644
--- a/drivers/macintosh/adb-iop.c
+++ b/drivers/macintosh/adb-iop.c
@@ -77,15 +77,14 @@ static void adb_iop_done(void)
static void adb_iop_complete(struct iop_msg *msg)
{
- struct adb_request *req;
unsigned long flags;
local_irq_save(flags);
- req = current_req;
- if ((adb_iop_state == sending) && req && req->reply_expected) {
+ if (current_req->reply_expected)
adb_iop_state = awaiting_reply;
- }
+ else
+ adb_iop_done();
local_irq_restore(flags);
}
--
2.26.2
[PATCH 4/8] macintosh/adb-iop: Access current_req and adb_iop_state when inside lock
Drop the redundant local_irq_save/restore() from adb_iop_start() because
the caller has to do it anyway. This is the pattern used in via-macii.
Cc: Joshua Thompson
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
drivers/macintosh/adb-iop.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/macintosh/adb-iop.c b/drivers/macintosh/adb-iop.c
index c3089dacf2e2..7ecc41bc7358 100644
--- a/drivers/macintosh/adb-iop.c
+++ b/drivers/macintosh/adb-iop.c
@@ -137,7 +137,6 @@ static void adb_iop_listen(struct iop_msg *msg)
static void adb_iop_start(void)
{
- unsigned long flags;
struct adb_request *req;
struct adb_iopmsg amsg;
@@ -146,8 +145,6 @@ static void adb_iop_start(void)
if (!req)
return;
- local_irq_save(flags);
-
/* The IOP takes MacII-style packets, so strip the initial
* ADB_PACKET byte.
*/
@@ -161,7 +158,6 @@ static void adb_iop_start(void)
req->sent = 1;
adb_iop_state = sending;
- local_irq_restore(flags);
/* Now send it. The IOP manager will call adb_iop_complete
* when the message has been sent.
@@ -208,13 +204,13 @@ static int adb_iop_write(struct adb_request *req)
return -EINVAL;
}
- local_irq_save(flags);
-
req->next = NULL;
req->sent = 0;
req->complete = 0;
req->reply_len = 0;
+ local_irq_save(flags);
+
if (current_req != 0) {
last_req->next = req;
last_req = req;
@@ -223,10 +219,11 @@ static int adb_iop_write(struct adb_request *req)
last_req = req;
}
- local_irq_restore(flags);
-
if (adb_iop_state == idle)
adb_iop_start();
+
+ local_irq_restore(flags);
+
return 0;
}
--
2.26.2
[PATCH 3/8] macintosh/adb-iop: Adopt bus reset algorithm from via-macii driver
This algorithm is slightly shorter and avoids the surprising
adb_iop_start() call in adb_iop_poll().
Cc: Joshua Thompson
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
drivers/macintosh/adb-iop.c | 21 -
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/macintosh/adb-iop.c b/drivers/macintosh/adb-iop.c
index ca3b411b0742..c3089dacf2e2 100644
--- a/drivers/macintosh/adb-iop.c
+++ b/drivers/macintosh/adb-iop.c
@@ -238,24 +238,19 @@ int adb_iop_autopoll(int devs)
void adb_iop_poll(void)
{
- if (adb_iop_state == idle)
- adb_iop_start();
iop_ism_irq_poll(ADB_IOP);
}
int adb_iop_reset_bus(void)
{
- struct adb_request req = {
- .reply_expected = 0,
- .nbytes = 2,
- .data = { ADB_PACKET, 0 },
- };
-
- adb_iop_write();
- while (!req.complete) {
- adb_iop_poll();
- schedule();
- }
+ struct adb_request req;
+
+ /* Command = 0, Address = ignored */
+ adb_request(, NULL, ADBREQ_NOSEND, 1, ADB_BUSRESET);
+ adb_iop_send_request(, 1);
+
+ /* Don't want any more requests during the Global Reset low time. */
+ mdelay(3);
return 0;
}
--
2.26.2
[PATCH 2/8] macintosh/adb-iop: Correct comment text
This patch improves comment style and corrects some misunderstandings
in the text.
Cc: Joshua Thompson
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
drivers/macintosh/adb-iop.c | 29 +
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/macintosh/adb-iop.c b/drivers/macintosh/adb-iop.c
index ce28ff40fb9c..ca3b411b0742 100644
--- a/drivers/macintosh/adb-iop.c
+++ b/drivers/macintosh/adb-iop.c
@@ -101,11 +101,10 @@ static void adb_iop_listen(struct iop_msg *msg)
req = current_req;
- /* Handle a timeout. Timeout packets seem to occur even after */
- /* we've gotten a valid reply to a TALK, so I'm assuming that */
- /* a "timeout" is actually more like an "end-of-data" signal. */
- /* We need to send back a timeout packet to the IOP to shut */
- /* it up, plus complete the current request, if any. */
+ /* Handle a timeout. Timeout packets seem to occur even after
+* we've gotten a valid reply to a TALK, presumably because of
+* autopolling.
+*/
if (amsg->flags & ADB_IOP_TIMEOUT) {
msg->reply[0] = ADB_IOP_TIMEOUT | ADB_IOP_AUTOPOLL;
@@ -115,9 +114,6 @@ static void adb_iop_listen(struct iop_msg *msg)
adb_iop_end_req(req, idle);
}
} else {
- /* TODO: is it possible for more than one chunk of data */
- /* to arrive before the timeout? If so we need to */
- /* use reply_ptr here like the other drivers do. */
if ((adb_iop_state == awaiting_reply) &&
(amsg->flags & ADB_IOP_EXPLICIT)) {
req->reply_len = amsg->count + 1;
@@ -152,23 +148,24 @@ static void adb_iop_start(void)
local_irq_save(flags);
- /* The IOP takes MacII-style packets, so */
- /* strip the initial ADB_PACKET byte.*/
-
+ /* The IOP takes MacII-style packets, so strip the initial
+* ADB_PACKET byte.
+*/
amsg.flags = ADB_IOP_EXPLICIT;
amsg.count = req->nbytes - 2;
- /* amsg.data immediately follows amsg.cmd, effectively making */
- /* amsg.cmd a pointer to the beginning of a full ADB packet. */
+ /* amsg.data immediately follows amsg.cmd, effectively making
+* a pointer to the beginning of a full ADB packet.
+*/
memcpy(, req->data + 1, req->nbytes - 1);
req->sent = 1;
adb_iop_state = sending;
local_irq_restore(flags);
- /* Now send it. The IOP manager will call adb_iop_complete */
- /* when the packet has been sent. */
-
+ /* Now send it. The IOP manager will call adb_iop_complete
+* when the message has been sent.
+*/
iop_send_message(ADB_IOP, ADB_CHAN, req, sizeof(amsg), (__u8 *),
adb_iop_complete);
}
--
2.26.2
[PATCH 1/8] macintosh/adb-iop: Remove dead and redundant code
Cc: Joshua Thompson
Tested-by: Stan Johnson
Signed-off-by: Finn Thain
---
drivers/macintosh/adb-iop.c | 29 -
1 file changed, 29 deletions(-)
diff --git a/drivers/macintosh/adb-iop.c b/drivers/macintosh/adb-iop.c
index fca31640e3ef..ce28ff40fb9c 100644
--- a/drivers/macintosh/adb-iop.c
+++ b/drivers/macintosh/adb-iop.c
@@ -18,24 +18,16 @@
#include
#include
#include
-#include
#include
#include
#include
-#include
#include
#include
-/*#define DEBUG_ADB_IOP*/
-
static struct adb_request *current_req;
static struct adb_request *last_req;
-#if 0
-static unsigned char reply_buff[16];
-static unsigned char *reply_ptr;
-#endif
static enum adb_iop_state {
idle,
@@ -104,22 +96,11 @@ static void adb_iop_listen(struct iop_msg *msg)
struct adb_iopmsg *amsg = (struct adb_iopmsg *)msg->message;
struct adb_request *req;
unsigned long flags;
-#ifdef DEBUG_ADB_IOP
- int i;
-#endif
local_irq_save(flags);
req = current_req;
-#ifdef DEBUG_ADB_IOP
- printk("adb_iop_listen %p: rcvd packet, %d bytes: %02X %02X", req,
- (uint)amsg->count + 2, (uint)amsg->flags, (uint)amsg->cmd);
- for (i = 0; i < amsg->count; i++)
- printk(" %02X", (uint)amsg->data[i]);
- printk("\n");
-#endif
-
/* Handle a timeout. Timeout packets seem to occur even after */
/* we've gotten a valid reply to a TALK, so I'm assuming that */
/* a "timeout" is actually more like an "end-of-data" signal. */
@@ -163,9 +144,6 @@ static void adb_iop_start(void)
unsigned long flags;
struct adb_request *req;
struct adb_iopmsg amsg;
-#ifdef DEBUG_ADB_IOP
- int i;
-#endif
/* get the packet to send */
req = current_req;
@@ -174,13 +152,6 @@ static void adb_iop_start(void)
local_irq_save(flags);
-#ifdef DEBUG_ADB_IOP
- printk("adb_iop_start %p: sending packet, %d bytes:", req, req->nbytes);
- for (i = 0; i < req->nbytes; i++)
- printk(" %02X", (uint)req->data[i]);
- printk("\n");
-#endif
-
/* The IOP takes MacII-style packets, so */
/* strip the initial ADB_PACKET byte.*/
--
2.26.2
[PATCH 0/8] Mac ADB IOP driver fixes
The adb-iop driver was never finished. Some deficiencies have become apparent over the years. For example, - Mouse and/or keyboard may stop working if used together - SRQ autopoll list cannot be changed - Some bugs were found by inspection This patch series contains fixes for the known bugs in the driver, plus a few clean-ups. Finn Thain (8): macintosh/adb-iop: Remove dead and redundant code macintosh/adb-iop: Correct comment text macintosh/adb-iop: Adopt bus reset algorithm from via-macii driver macintosh/adb-iop: Access current_req and adb_iop_state when inside lock macintosh/adb-iop: Resolve static checker warnings macintosh/adb-iop: Implement idle -> sending state transition macintosh/adb-iop: Implement sending -> idle state transition macintosh/adb-iop: Implement SRQ autopolling arch/m68k/include/asm/adb_iop.h | 1 + drivers/macintosh/adb-iop.c | 186 +++- 2 files changed, 86 insertions(+), 101 deletions(-) -- 2.26.2
