[PATCH 1/2] m68k: Drop Atari EtherNAT support when it's not configured in

[PATCH 1/2] m68k: Drop Atari EtherNAT support when it's not configured in

[Eero Tamminen]

Not having it configured, but crash backtraces going through its IRQ
handling code, is confusing (found while debugging issues in Atari
emulation).

Signed-off-by: Eero Tamminen <oak@helsinkinet.fi>
---
 arch/m68k/atari/ataints.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/m68k/atari/ataints.c b/arch/m68k/atari/ataints.c
index 0465444ce..fa189eeb7 100644
--- a/arch/m68k/atari/ataints.c
+++ b/arch/m68k/atari/ataints.c
@@ -179,6 +179,7 @@ static struct irq_chip atari_mfptimer_chip = {
 };
 
 
+#ifdef CONFIG_ATARI_ETHERNAT
 /*
  * EtherNAT CPLD interrupt handling
  * CPLD interrupt register is at phys. 0x80000023
@@ -251,6 +252,7 @@ static struct irq_chip atari_ethernat_chip = {
 	.irq_enable	= atari_ethernat_enable,
 	.irq_disable	= atari_ethernat_disable,
 };
+#endif
 
 /*
  * void atari_init_IRQ (void)
@@ -343,12 +345,13 @@ void __init atari_init_IRQ(void)
 			stmfp_base.name, &stmfp_base))
 		pr_err("Couldn't register %s interrupt\n", stmfp_base.name);
 
+#ifdef CONFIG_ATARI_ETHERNAT
 	/*
 	 * EtherNAT ethernet / USB interrupt handlers
 	 */
-
 	m68k_setup_irq_controller(&atari_ethernat_chip, handle_simple_irq,
 				  139, 2);
+#endif
 }
 
 
-- 
2.39.5

[PATCH 2/2] m68k: Set reasonable PSG port-A default value

[Eero Tamminen]

Fixes continuous IDE & DSP reset requests on Atari Falcon from 2Hz
floppy media change detection if those bits happen to be set (to
a different value than ROM defaults) before Linux starts.

All successive port-A requests preserve unrelated bits.  If bits 4 & 6
are set, that means every PSG port-A request, such as floppy media
change detection, being also Falcon DSP & IDE reset request.

Signed-off-by: Eero Tamminen <oak@helsinkinet.fi>
---
 Documentation/arch/m68k/kernel-options.rst |  4 +++-
 arch/m68k/atari/config.c                   | 10 ++++++----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/Documentation/arch/m68k/kernel-options.rst b/Documentation/arch/m68k/kernel-options.rst
index 2008a20b4..cae270018 100644
--- a/Documentation/arch/m68k/kernel-options.rst
+++ b/Documentation/arch/m68k/kernel-options.rst
@@ -674,8 +674,10 @@ items:
 	set RTS of the MIDI ACIA high
   snd6:
 	set bit 6 of the PSG port A
+	(Falcon: internal speaker on/off, others: monitor jack GPO pin)
   snd7:
-	set bit 6 of the PSG port A
+	set bit 7 of the PSG port A
+	(Falcon: IDE drive on/off, TT: SCC-A LAN/serial2)
 
 It doesn't make sense to mention a switch more than once (no
 difference to only once), but you can give as many switches as you
diff --git a/arch/m68k/atari/config.c b/arch/m68k/atari/config.c
index b48a0606a..0e4bd90fe 100644
--- a/arch/m68k/atari/config.c
+++ b/arch/m68k/atari/config.c
@@ -188,6 +188,7 @@ early_param("switches", atari_switches_setup);
 void __init config_atari(void)
 {
 	unsigned short tos_version;
+	unsigned char porta_init = 0x07;
 
 	memset(&atari_hw_present, 0, sizeof(atari_hw_present));
 
@@ -212,11 +213,12 @@ void __init config_atari(void)
 	if (atari_switches & ATARI_SWITCH_MIDI)
 		acia.mid_ctrl = ACIA_DIV16 | ACIA_D8N1S | ACIA_RHTID;
 	if (atari_switches & (ATARI_SWITCH_SND6|ATARI_SWITCH_SND7)) {
-		sound_ym.rd_data_reg_sel = 14;
-		sound_ym.wd_data = sound_ym.rd_data_reg_sel |
-				   ((atari_switches&ATARI_SWITCH_SND6) ? 0x40 : 0) |
-				   ((atari_switches&ATARI_SWITCH_SND7) ? 0x80 : 0);
+		porta_init |= ((atari_switches&ATARI_SWITCH_SND6) ? 0x40 : 0) |
+			      ((atari_switches&ATARI_SWITCH_SND7) ? 0x80 : 0);
 	}
+	/* Set PSG port-A defaults */
+	sound_ym.rd_data_reg_sel = 14;
+	sound_ym.wd_data = porta_init;
 
 	/* ++bjoern:
 	 * Determine hardware present
-- 
2.39.5

Re: [PATCH 1/2] m68k: Drop Atari EtherNAT support when it's not configured in

[John Paul Adrian Glaubitz]

On Mon, 2024-09-23 at 00:48 +0300, Eero Tamminen wrote:
> Not having it configured, but crash backtraces going through its IRQ
> handling code, is confusing (found while debugging issues in Atari
> emulation).

Please rephrase the patch title. The word "drop" has a very specific
meaning in this context, namely removing a feature. I would rather
call it "disable".

Also, I think the longer description is slightly hard to parse due to
too much brevity.

Adrian

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

Re: [PATCH 1/2] m68k: Drop Atari EtherNAT support when it's not configured in

[Eero Tamminen]

Hi,

On 23.9.2024 0.58, John Paul Adrian Glaubitz wrote:
> On Mon, 2024-09-23 at 00:48 +0300, Eero Tamminen wrote:
>> Not having it configured, but crash backtraces going through its IRQ
>> handling code, is confusing (found while debugging issues in Atari
>> emulation).
> 
> Please rephrase the patch title. The word "drop" has a very specific
> meaning in this context, namely removing a feature. I would rather
> call it "disable".

Maybe "Call Atari EtherNAT functions only when it's configured in"?


> Also, I think the longer description is slightly hard to parse due to
> too much brevity.

Would this be easier to parse?

"Found while debugging emulation problems.  EtherNAT functions were 
visible in some backtraces, although EtherNAT config option was 
disabled, and emulator did not support EtherNAT."

(This is from many years ago, these patches have been part of Hatari 
emulator, I just haven't posted them here before.)


	- Eero

Re: [PATCH 1/2] m68k: Drop Atari EtherNAT support when it's not configured in

[John Paul Adrian Glaubitz]

On Mon, 2024-09-23 at 11:39 +0300, Eero Tamminen wrote:
> Hi,
> 
> On 23.9.2024 0.58, John Paul Adrian Glaubitz wrote:
> > On Mon, 2024-09-23 at 00:48 +0300, Eero Tamminen wrote:
> > > Not having it configured, but crash backtraces going through its IRQ
> > > handling code, is confusing (found while debugging issues in Atari
> > > emulation).
> > 
> > Please rephrase the patch title. The word "drop" has a very specific
> > meaning in this context, namely removing a feature. I would rather
> > call it "disable".
> 
> Maybe "Call Atari EtherNAT functions only when it's configured in"?

Yes, that's much better. Maybe eben use:

	"Call Atari EtherNAT functions only with CONFIG_FOO_BAR enabled"

> > Also, I think the longer description is slightly hard to parse due to
> > too much brevity.
> 
> Would this be easier to parse?
> 
> "Found while debugging emulation problems.  EtherNAT functions were 
> visible in some backtraces, although EtherNAT config option was 
> disabled, and emulator did not support EtherNAT."

Yes, this is way better to parse. I would add a "the" before "emulator" though.

> (This is from many years ago, these patches have been part of Hatari 
> emulator, I just haven't posted them here before.)

Thanks,
Adrian

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

Re: [PATCH 1/2] m68k: Drop Atari EtherNAT support when it's not configured in

[Michael Schmitz]

Eero,

thanks for your patch!

On 23/09/24 09:48, Eero Tamminen wrote:
> Not having it configured, but crash backtraces going through its IRQ
> handling code, is confusing (found while debugging issues in Atari

I don't see how backtraces could go through the EtherNAT interrupt code 
- the interrupt only gets enabled if the hardware has been detected.

Maybe you meant you see the generic IRQ handling code called for the 
EtherNAT's interrupt controller?

> emulation).
>
> Signed-off-by: Eero Tamminen <oak@helsinkinet.fi>
> ---
>   arch/m68k/atari/ataints.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/m68k/atari/ataints.c b/arch/m68k/atari/ataints.c
> index 0465444ce..fa189eeb7 100644
> --- a/arch/m68k/atari/ataints.c
> +++ b/arch/m68k/atari/ataints.c
> @@ -179,6 +179,7 @@ static struct irq_chip atari_mfptimer_chip = {
>   };
>   
>   
> +#ifdef CONFIG_ATARI_ETHERNAT

I believe we now use

#if IS_ENABLED(CONFIG_ATARI_ETHERNAT)

but that's perhaps a matter of taste (CONFIG_ATARI_ETHERNAT can't take 
the value 'm').

OK by me if it helps with your debugging ...

Cheers,

     Michael


>   /*
>    * EtherNAT CPLD interrupt handling
>    * CPLD interrupt register is at phys. 0x80000023
> @@ -251,6 +252,7 @@ static struct irq_chip atari_ethernat_chip = {
>   	.irq_enable	= atari_ethernat_enable,
>   	.irq_disable	= atari_ethernat_disable,
>   };
> +#endif
>   
>   /*
>    * void atari_init_IRQ (void)
> @@ -343,12 +345,13 @@ void __init atari_init_IRQ(void)
>   			stmfp_base.name, &stmfp_base))
>   		pr_err("Couldn't register %s interrupt\n", stmfp_base.name);
>   
> +#ifdef CONFIG_ATARI_ETHERNAT
>   	/*
>   	 * EtherNAT ethernet / USB interrupt handlers
>   	 */
> -
>   	m68k_setup_irq_controller(&atari_ethernat_chip, handle_simple_irq,
>   				  139, 2);
> +#endif
>   }
>   
>   

Re: [PATCH 1/2] m68k: Drop Atari EtherNAT support when it's not configured in

[Eero Tamminen]

Hi,

On 24.9.2024 4.03, Michael Schmitz wrote:
> On 23/09/24 09:48, Eero Tamminen wrote:
>> Not having it configured, but crash backtraces going through its IRQ
>> handling code, is confusing (found while debugging issues in Atari
> 
> I don't see how backtraces could go through the EtherNAT interrupt code 
> - the interrupt only gets enabled if the hardware has been detected.
> 
> Maybe you meant you see the generic IRQ handling code called for the 
> EtherNAT's interrupt controller?

It was many years ago, but I _did_ see one of these functions in Hatari 
debugger's backtrace, although I had explicitly disabled EtherNAT support.

I don't any more remember what I was debugging, but I think it's 
something already fixed / supported in Hatari.

=> Better just to drop that comment?


>> +#ifdef CONFIG_ATARI_ETHERNAT
> 
> I believe we now use
> 
> #if IS_ENABLED(CONFIG_ATARI_ETHERNAT)

OK.


> but that's perhaps a matter of taste (CONFIG_ATARI_ETHERNAT can't take 
> the value 'm').
 >
> OK by me if it helps with your debugging ...

The issue triggering it is unlikely to appear again, so it's not that 
important.  But I think those if(def)s are still the correct thing to do.


	- Eero

PS. As m68k Linux support is rather old, most of code appears to be 
still using ifdefs:

$ git grep "ifdef *CONFIG_" arch/m68k/ | wc -l
336

$ git grep IS_ENABLED arch/m68k/ | wc -l
62

$ git grep -c "ifdef CONFIG_" arch/m68k/atari/
arch/m68k/atari/ataints.c:2
arch/m68k/atari/config.c:7
arch/m68k/atari/nvram.c:1

$ git grep -c IS_ENABLED arch/m68k/atari/
arch/m68k/atari/config.c:3

Re: [PATCH 1/2] m68k: Drop Atari EtherNAT support when it's not configured in

[Andreas Schwab]

On Sep 24 2024, Eero Tamminen wrote:

> Hi,
>
> On 24.9.2024 4.03, Michael Schmitz wrote:
>> On 23/09/24 09:48, Eero Tamminen wrote:
>>> Not having it configured, but crash backtraces going through its IRQ
>>> handling code, is confusing (found while debugging issues in Atari
>> I don't see how backtraces could go through the EtherNAT interrupt code
>> - the interrupt only gets enabled if the hardware has been detected.
>> Maybe you meant you see the generic IRQ handling code called for the
>> EtherNAT's interrupt controller?
>
> It was many years ago, but I _did_ see one of these functions in Hatari
> debugger's backtrace, although I had explicitly disabled EtherNAT support.

That can easily happen if a word on the stack happens to match the
address range of an unrelated function.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH 1/2] m68k: Drop Atari EtherNAT support when it's not configured in

[Eero Tamminen]

Hi,

On 25.9.2024 0.09, Andreas Schwab wrote:
> On Sep 24 2024, Eero Tamminen wrote:
>> On 24.9.2024 4.03, Michael Schmitz wrote:
>>> On 23/09/24 09:48, Eero Tamminen wrote:
>>>> Not having it configured, but crash backtraces going through its IRQ
>>>> handling code, is confusing (found while debugging issues in Atari
>>> I don't see how backtraces could go through the EtherNAT interrupt code
>>> - the interrupt only gets enabled if the hardware has been detected.
>>> Maybe you meant you see the generic IRQ handling code called for the
>>> EtherNAT's interrupt controller?
>>
>> It was many years ago, but I _did_ see one of these functions in Hatari
>> debugger's backtrace, although I had explicitly disabled EtherNAT support.
> 
> That can easily happen if a word on the stack happens to match the
> address range of an unrelated function.

Hatari debugger backtrace does not rely on frame pointers or stack 
content unwinding, i.e. it's immune to stack corruption.

Backtrace is a by-product of Hatari profiler, which tracks every 
emulated (CPU & DSP) instruction and constructs call-graph while the 
code runs, by comparing PC register values against function addresses.

---

LILO functionality in Hatari reads function addresses from the 
(specified) "Symbol.map" file (in addition to starting kernel). After 
typing "profile on" in debugger, one sees backtrace every time debugger 
is invoked.

(To trace all kernel function calls, one can use Hatari "--trace 
cpu_symbols" option, which can be toggled in the debugger, either 
manually, or automatically by a breakpoint.  To trace a subset of the 
function calls, one can remove others from the "Symbols.map" file.)


	- Eero

PS. To do similar instruction tracking for PCs would need special data 
collection HW and offline data processing (ARM ETM [1] & Intel PTI [2]), 
but when emulating old/slow m68k machines, it can all be done in SW at 
real-time.

[1] 
https://developer.arm.com/documentation/ddi0488/h/embedded-trace-macrocell/about-etm
[2] https://lldb.llvm.org/use/intel_pt.html

Re: [PATCH 1/2] m68k: Drop Atari EtherNAT support when it's not configured in

[Andreas Schwab]

On Sep 25 2024, Eero Tamminen wrote:

> Hatari debugger backtrace does not rely on frame pointers or stack content
> unwinding, i.e. it's immune to stack corruption.

That has nothing to do with stack corruption.  Valid user-space
addresses can very well happen to match kernel adresses, since the
address spaces overlap.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH 1/2] m68k: Drop Atari EtherNAT support when it's not configured in

[Eero Tamminen]

Hi,

On 25.9.2024 20.13, Andreas Schwab wrote:
> On Sep 25 2024, Eero Tamminen wrote:
> 
>> Hatari debugger backtrace does not rely on frame pointers or stack content
>> unwinding, i.e. it's immune to stack corruption.
> 
> That has nothing to do with stack corruption.  Valid user-space
> addresses can very well happen to match kernel adresses, since the
> address spaces overlap.

Hatari profiler & debugger (currently) support only physical addresses, 
so that's not the case.


	- Eero

Lack of MMU / address translation support in them is not a problem for 
what they're normally used [1], as neither TOS, nor 99.9% of the stuff 
run under it, use/need that.

However, with Linux, it means them being mainly useful for debugging 
what happens on kernel side.  I do not think that to be too much of a 
problem though, as kernel side is the hard part and there are good 
alternatives for debugging user-space.

---

[1] E.g. optimizations in latest ScummVM upstream releases that make it 
work OK on original Atari TT & Falcon HW, and (enhanced) Doom I version 
that runs OK on original Atari Falcon.