[PATCH 0/4] m68k/mac: Various cleanups and fixes

[PATCH 0/4] m68k/mac: Various cleanups and fixes

[Finn Thain]

Finn Thain (4):
  m68k/mac: More printk modernization
  m68k/mac: Disentangle VIA and OSS initialization
  m68k/mac: Disentangle VIA/RBV and NuBus initialization
  m68k/mac: Add mutual exclusion for IOP interrupt polling

 arch/m68k/include/asm/mac_iop.h |  1 +
 arch/m68k/mac/baboon.c          |  2 +-
 arch/m68k/mac/config.c          |  2 +-
 arch/m68k/mac/iop.c             | 13 ++++++++--
 arch/m68k/mac/oss.c             | 16 ++++---------
 arch/m68k/mac/psc.c             |  6 ++---
 arch/m68k/mac/via.c             | 53 ++++++++++++++++-------------------------
 drivers/macintosh/adb-iop.c     |  4 +---
 drivers/nubus/nubus.c           | 13 ----------
 9 files changed, 42 insertions(+), 68 deletions(-)

-- 
2.13.6

[PATCH 4/4] m68k/mac: Add mutual exclusion for IOP interrupt polling

[Finn Thain]

The IOP interrupt handler iop_ism_irq() is used by the adb-iop
driver to poll for ADB request completion. Unfortunately, it is not
re-entrant. Fix the race condition by adding an iop_ism_irq_poll()
function with suitable mutual exclusion.

Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: linuxppc-dev@lists.ozlabs.org
---
 arch/m68k/include/asm/mac_iop.h | 1 +
 arch/m68k/mac/iop.c             | 9 +++++++++
 drivers/macintosh/adb-iop.c     | 4 +---
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/m68k/include/asm/mac_iop.h b/arch/m68k/include/asm/mac_iop.h
index 42566fd052bc..d2a08e004e2c 100644
--- a/arch/m68k/include/asm/mac_iop.h
+++ b/arch/m68k/include/asm/mac_iop.h
@@ -158,6 +158,7 @@ extern void iop_complete_message(struct iop_msg *);
 extern void iop_upload_code(uint, __u8 *, uint, __u16);
 extern void iop_download_code(uint, __u8 *, uint, __u16);
 extern __u8 *iop_compare_code(uint, __u8 *, uint, __u16);
+extern void iop_ism_irq_poll(uint);
 
 extern void iop_register_interrupts(void);
 
diff --git a/arch/m68k/mac/iop.c b/arch/m68k/mac/iop.c
index a2ea52db7d18..9bfa17015768 100644
--- a/arch/m68k/mac/iop.c
+++ b/arch/m68k/mac/iop.c
@@ -598,3 +598,12 @@ irqreturn_t iop_ism_irq(int irq, void *dev_id)
 	}
 	return IRQ_HANDLED;
 }
+
+void iop_ism_irq_poll(uint iop_num)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+	iop_ism_irq(0, (void *)iop_num);
+	local_irq_restore(flags);
+}
diff --git a/drivers/macintosh/adb-iop.c b/drivers/macintosh/adb-iop.c
index f5f4da3d0b67..4b0ad3995497 100644
--- a/drivers/macintosh/adb-iop.c
+++ b/drivers/macintosh/adb-iop.c
@@ -29,8 +29,6 @@
 
 /*#define DEBUG_ADB_IOP*/
 
-extern void iop_ism_irq(int, void *);
-
 static struct adb_request *current_req;
 static struct adb_request *last_req;
 #if 0
@@ -265,7 +263,7 @@ int adb_iop_autopoll(int devs)
 void adb_iop_poll(void)
 {
 	if (adb_iop_state == idle) adb_iop_start();
-	iop_ism_irq(0, (void *) ADB_IOP);
+	iop_ism_irq_poll(ADB_IOP);
 }
 
 int adb_iop_reset_bus(void)
-- 
2.13.6

[PATCH 1/4] m68k/mac: More printk modernization

[Finn Thain]

Log message fragments used to be printed on one line but now get split up.
Fix this. Also, suppress log spam that merely prints known pointer values.

Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 arch/m68k/mac/baboon.c |  2 +-
 arch/m68k/mac/iop.c    |  4 ++--
 arch/m68k/mac/psc.c    |  6 ++----
 arch/m68k/mac/via.c    | 18 +++++-------------
 4 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/arch/m68k/mac/baboon.c b/arch/m68k/mac/baboon.c
index 514acde3cd40..0d154c909e17 100644
--- a/arch/m68k/mac/baboon.c
+++ b/arch/m68k/mac/baboon.c
@@ -36,7 +36,7 @@ void __init baboon_init(void)
 	baboon = (struct baboon *) BABOON_BASE;
 	baboon_present = 1;
 
-	printk("Baboon detected at %p\n", baboon);
+	pr_debug("Baboon detected at %p\n", baboon);
 }
 
 /*
diff --git a/arch/m68k/mac/iop.c b/arch/m68k/mac/iop.c
index 4c1e606e7d03..a2ea52db7d18 100644
--- a/arch/m68k/mac/iop.c
+++ b/arch/m68k/mac/iop.c
@@ -273,10 +273,10 @@ void __init iop_init(void)
 	int i;
 
 	if (iop_scc_present) {
-		pr_info("IOP: detected SCC IOP at %p\n", iop_base[IOP_NUM_SCC]);
+		pr_debug("SCC IOP detected at %p\n", iop_base[IOP_NUM_SCC]);
 	}
 	if (iop_ism_present) {
-		pr_info("IOP: detected ISM IOP at %p\n", iop_base[IOP_NUM_ISM]);
+		pr_debug("ISM IOP detected at %p\n", iop_base[IOP_NUM_ISM]);
 		iop_start(iop_base[IOP_NUM_ISM]);
 		iop_alive(iop_base[IOP_NUM_ISM]); /* clears the alive flag */
 	}
diff --git a/arch/m68k/mac/psc.c b/arch/m68k/mac/psc.c
index 439a2a2e5874..8d547df4e16c 100644
--- a/arch/m68k/mac/psc.c
+++ b/arch/m68k/mac/psc.c
@@ -42,7 +42,7 @@ static void psc_debug_dump(void)
 		return;
 
 	for (i = 0x30 ; i < 0x70 ; i += 0x10) {
-		printk("PSC #%d:  IFR = 0x%02X IER = 0x%02X\n",
+		printk(KERN_DEBUG "PSC #%d:  IFR = 0x%02X IER = 0x%02X\n",
 			i >> 4,
 			(int) psc_read_byte(pIFRbase + i),
 			(int) psc_read_byte(pIERbase + i));
@@ -59,14 +59,12 @@ static __init void psc_dma_die_die_die(void)
 {
 	int i;
 
-	printk("Killing all PSC DMA channels...");
 	for (i = 0 ; i < 9 ; i++) {
 		psc_write_word(PSC_CTL_BASE + (i << 4), 0x8800);
 		psc_write_word(PSC_CTL_BASE + (i << 4), 0x1000);
 		psc_write_word(PSC_CMD_BASE + (i << 5), 0x1100);
 		psc_write_word(PSC_CMD_BASE + (i << 5) + 0x10, 0x1100);
 	}
-	printk("done!\n");
 }
 
 /*
@@ -92,7 +90,7 @@ void __init psc_init(void)
 
 	psc = (void *) PSC_BASE;
 
-	printk("PSC detected at %p\n", psc);
+	pr_debug("PSC detected at %p\n", psc);
 
 	psc_dma_die_die_die();
 
diff --git a/arch/m68k/mac/via.c b/arch/m68k/mac/via.c
index 16629e91feba..05021381af0b 100644
--- a/arch/m68k/mac/via.c
+++ b/arch/m68k/mac/via.c
@@ -121,18 +121,21 @@ void via_debug_dump(void);
 
 void __init via_init(void)
 {
+	via1 = (void *)VIA1_BASE;
+	pr_debug("VIA1 detected at %p\n", via1);
+
 	switch(macintosh_config->via_type) {
 
 		/* IIci, IIsi, IIvx, IIvi (P6xx), LC series */
 
 		case MAC_VIA_IICI:
-			via1 = (void *) VIA1_BASE;
 			if (macintosh_config->ident == MAC_MODEL_IIFX) {
 				via2 = NULL;
 				rbv_present = 0;
 				oss_present = 1;
 			} else {
 				via2 = (void *) RBV_BASE;
+				pr_debug("VIA2 (RBV) detected at %p\n", via2);
 				rbv_present = 1;
 				oss_present = 0;
 			}
@@ -154,8 +157,8 @@ void __init via_init(void)
 
 		case MAC_VIA_QUADRA:
 		case MAC_VIA_II:
-			via1 = (void *) VIA1_BASE;
 			via2 = (void *) VIA2_BASE;
+			pr_debug("VIA2 detected at %p\n", via2);
 			rbv_present = 0;
 			oss_present = 0;
 			rbv_clear = 0x00;
@@ -168,17 +171,6 @@ void __init via_init(void)
 			panic("UNKNOWN VIA TYPE");
 	}
 
-	printk(KERN_INFO "VIA1 at %p is a 6522 or clone\n", via1);
-
-	printk(KERN_INFO "VIA2 at %p is ", via2);
-	if (rbv_present) {
-		printk("an RBV\n");
-	} else if (oss_present) {
-		printk("an OSS\n");
-	} else {
-		printk("a 6522 or clone\n");
-	}
-
 #ifdef DEBUG_VIA
 	via_debug_dump();
 #endif
-- 
2.13.6

[PATCH 2/4] m68k/mac: Disentangle VIA and OSS initialization

[Finn Thain]

macintosh_config->via_type is meaningless on Mac IIfx (i.e. the only
model with OSS chip), so skip the via_type switch statement.

Call oss_init() before via_init() because it is more important and
because that is the right place to initialize the oss_present flag.

On this model, bringing forward oss_init() and delaying via_init()
is no problem because those functions are independent.

The only requirement here is that oss_register_interrupts() happens
after via_init(). That is, mac_init_IRQ() happens after config_mac().

Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 arch/m68k/mac/config.c |  2 +-
 arch/m68k/mac/oss.c    |  8 ++++----
 arch/m68k/mac/via.c    | 32 +++++++++++++-------------------
 3 files changed, 18 insertions(+), 24 deletions(-)

diff --git a/arch/m68k/mac/config.c b/arch/m68k/mac/config.c
index 22123f7e8f75..16cd5cea5207 100644
--- a/arch/m68k/mac/config.c
+++ b/arch/m68k/mac/config.c
@@ -898,8 +898,8 @@ static void __init mac_identify(void)
 		mac_bi_data.id, mac_bi_data.cpuid, mac_bi_data.memsize);
 
 	iop_init();
-	via_init();
 	oss_init();
+	via_init();
 	psc_init();
 	baboon_init();
 
diff --git a/arch/m68k/mac/oss.c b/arch/m68k/mac/oss.c
index ca84dcf41fc9..61906eb67d0f 100644
--- a/arch/m68k/mac/oss.c
+++ b/arch/m68k/mac/oss.c
@@ -31,18 +31,18 @@ volatile struct mac_oss *oss;
 
 /*
  * Initialize the OSS
- *
- * The OSS "detection" code is actually in via_init() which is always called
- * before us. Thus we can count on oss_present being valid on entry.
  */
 
 void __init oss_init(void)
 {
 	int i;
 
-	if (!oss_present) return;
+	if (macintosh_config->ident != MAC_MODEL_IIFX)
+		return;
 
 	oss = (struct mac_oss *) OSS_BASE;
+	pr_debug("OSS detected at %p", oss);
+	oss_present = 1;
 
 	/* Disable all interrupts. Unlike a VIA it looks like we    */
 	/* do this by setting the source's interrupt level to zero. */
diff --git a/arch/m68k/mac/via.c b/arch/m68k/mac/via.c
index 05021381af0b..19ad46ba4787 100644
--- a/arch/m68k/mac/via.c
+++ b/arch/m68k/mac/via.c
@@ -113,10 +113,6 @@ void via_debug_dump(void);
  * First we figure out where they actually _are_ as well as what type of
  * VIA we have for VIA2 (it could be a real VIA or an RBV or even an OSS.)
  * Then we pretty much clear them out and disable all IRQ sources.
- *
- * Note: the OSS is actually "detected" here and not in oss_init(). It just
- *	 seems more logical to do it here since via_init() needs to know
- *	 these things anyways.
  */
 
 void __init via_init(void)
@@ -124,21 +120,18 @@ void __init via_init(void)
 	via1 = (void *)VIA1_BASE;
 	pr_debug("VIA1 detected at %p\n", via1);
 
-	switch(macintosh_config->via_type) {
+	if (oss_present) {
+		via2 = NULL;
+		rbv_present = 0;
+	} else {
+		switch (macintosh_config->via_type) {
 
 		/* IIci, IIsi, IIvx, IIvi (P6xx), LC series */
 
 		case MAC_VIA_IICI:
-			if (macintosh_config->ident == MAC_MODEL_IIFX) {
-				via2 = NULL;
-				rbv_present = 0;
-				oss_present = 1;
-			} else {
-				via2 = (void *) RBV_BASE;
-				pr_debug("VIA2 (RBV) detected at %p\n", via2);
-				rbv_present = 1;
-				oss_present = 0;
-			}
+			via2 = (void *)RBV_BASE;
+			pr_debug("VIA2 (RBV) detected at %p\n", via2);
+			rbv_present = 1;
 			if (macintosh_config->ident == MAC_MODEL_LCIII) {
 				rbv_clear = 0x00;
 			} else {
@@ -160,15 +153,16 @@ void __init via_init(void)
 			via2 = (void *) VIA2_BASE;
 			pr_debug("VIA2 detected at %p\n", via2);
 			rbv_present = 0;
-			oss_present = 0;
 			rbv_clear = 0x00;
 			gIER = vIER;
 			gIFR = vIFR;
 			gBufA = vBufA;
 			gBufB = vBufB;
 			break;
+
 		default:
 			panic("UNKNOWN VIA TYPE");
+		}
 	}
 
 #ifdef DEBUG_VIA
@@ -295,9 +289,9 @@ void via_debug_dump(void)
 		(uint) via1[vDirA], (uint) via1[vDirB], (uint) via1[vACR]);
 	printk(KERN_DEBUG "         PCR = 0x%02X  IFR = 0x%02X IER = 0x%02X\n",
 		(uint) via1[vPCR], (uint) via1[vIFR], (uint) via1[vIER]);
-	if (oss_present) {
-		printk(KERN_DEBUG "VIA2: <OSS>\n");
-	} else if (rbv_present) {
+	if (!via2)
+		return;
+	if (rbv_present) {
 		printk(KERN_DEBUG "VIA2:  IFR = 0x%02X  IER = 0x%02X\n",
 			(uint) via2[rIFR], (uint) via2[rIER]);
 		printk(KERN_DEBUG "      SIFR = 0x%02X SIER = 0x%02X\n",
-- 
2.13.6

[PATCH 3/4] m68k/mac: Disentangle VIA/RBV and NuBus initialization

[Finn Thain]

The Nubus subsystem should not be concerned with differences between VIA,
RBV and OSS platforms. It should be portable across Macs and PowerMacs.
This goal has implications for the initialization code relating to bus
locking and slot interrupts.

During Nubus initialization, bus transactions are "unlocked": on VIA2 and
RBV machines, via_nubus_init() sets a bit in the via2[gBufB] register to
allow bus-mastering Nubus cards to arbitrate for the bus. This happens
upon subsys_initcall(nubus_init). But because nubus_init() has no effect
on card state, this sequence is arbitrary.

Moreover, when Penguin is used to boot Linux, the bus is already unlocked
when Linux starts. On OSS machines there's no attempt to unlock Nubus
transactions at all. (Maybe there's no benefit on that platform or maybe
no-one knows how.)

All of this demonstrates that there's no benefit in locking out
bus-mastering cards, as yet. (If the need arises, we could lock the bus
for the duration of a timing-critical operation.) NetBSD unlocks the
Nubus early (at VIA initialization) and we can do the same.

via_nubus_init() is also responsible for some VIA interrupt setup that
should happen earlier than subsys_initcall(nubus_init). And actually, the
Nubus subsystem need not be involved with slot interrupts: SLOT2IRQ
works fine because Nubus slot IRQs are geographically assigned
(regardless of platform).

For certain platforms with PDS slots, some Nubus IRQs may be platform
IRQs and this is not something that the NuBus subsystem should worry
about. So let's invoke via_nubus_init() earlier and make the platform
responsible for bus unlocking and interrupt setup instead of the NuBus
subsystem.

Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 arch/m68k/mac/oss.c   |  8 --------
 arch/m68k/mac/via.c   |  5 ++++-
 drivers/nubus/nubus.c | 13 -------------
 3 files changed, 4 insertions(+), 22 deletions(-)

diff --git a/arch/m68k/mac/oss.c b/arch/m68k/mac/oss.c
index 61906eb67d0f..21b85605db58 100644
--- a/arch/m68k/mac/oss.c
+++ b/arch/m68k/mac/oss.c
@@ -52,14 +52,6 @@ void __init oss_init(void)
 }
 
 /*
- * Initialize OSS for Nubus access
- */
-
-void __init oss_nubus_init(void)
-{
-}
-
-/*
  * Handle miscellaneous OSS interrupts.
  */
 
diff --git a/arch/m68k/mac/via.c b/arch/m68k/mac/via.c
index 19ad46ba4787..c38fab213e9f 100644
--- a/arch/m68k/mac/via.c
+++ b/arch/m68k/mac/via.c
@@ -106,6 +106,7 @@ static int gIER,gIFR,gBufA,gBufB;
 static u8 nubus_disabled;
 
 void via_debug_dump(void);
+static void via_nubus_init(void);
 
 /*
  * Initialize the VIAs
@@ -238,6 +239,8 @@ void __init via_init(void)
 		via2[vACR] &= ~0x03; /* disable port A & B latches */
 	}
 
+	via_nubus_init();
+
 	/* Everything below this point is VIA2 only... */
 
 	if (rbv_present)
@@ -359,7 +362,7 @@ int via_get_cache_disable(void)
  * Initialize VIA2 for Nubus access
  */
 
-void __init via_nubus_init(void)
+static void __init via_nubus_init(void)
 {
 	/* unlock nubus transactions */
 
diff --git a/drivers/nubus/nubus.c b/drivers/nubus/nubus.c
index df431e8a0631..4f50f30650cd 100644
--- a/drivers/nubus/nubus.c
+++ b/drivers/nubus/nubus.c
@@ -18,11 +18,6 @@
 #include <asm/setup.h>
 #include <asm/page.h>
 #include <asm/hwtest.h>
-#include <asm/mac_via.h>
-#include <asm/mac_oss.h>
-
-extern void via_nubus_init(void);
-extern void oss_nubus_init(void);
 
 /* Constants */
 
@@ -840,14 +835,6 @@ static int __init nubus_init(void)
 	if (!MACH_IS_MAC)
 		return 0;
 
-	/* Initialize the NuBus interrupts */
-	if (oss_present) {
-		oss_nubus_init();
-	} else {
-		via_nubus_init();
-	}
-
-	/* And probe */
 	pr_info("NuBus: Scanning NuBus slots.\n");
 	nubus_devices = NULL;
 	nubus_boards = NULL;
-- 
2.13.6

Re: [PATCH 1/4] m68k/mac: More printk modernization

[Kars de Jong]

2017-10-27 4:45 GMT+02:00 Finn Thain <fthain@telegraphics.com.au>:
> Log message fragments used to be printed on one line but now get split up.
> Fix this. Also, suppress log spam that merely prints known pointer values.
>
> Tested-by: Stan Johnson <userm57@yahoo.com>
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> ---
>  arch/m68k/mac/baboon.c |  2 +-
>  arch/m68k/mac/iop.c    |  4 ++--
>  arch/m68k/mac/psc.c    |  6 ++----
>  arch/m68k/mac/via.c    | 18 +++++-------------
>  4 files changed, 10 insertions(+), 20 deletions(-)

...

> diff --git a/arch/m68k/mac/psc.c b/arch/m68k/mac/psc.c
> index 439a2a2e5874..8d547df4e16c 100644
> --- a/arch/m68k/mac/psc.c
> +++ b/arch/m68k/mac/psc.c
> @@ -42,7 +42,7 @@ static void psc_debug_dump(void)
>                 return;
>
>         for (i = 0x30 ; i < 0x70 ; i += 0x10) {
> -               printk("PSC #%d:  IFR = 0x%02X IER = 0x%02X\n",
> +               printk(KERN_DEBUG "PSC #%d:  IFR = 0x%02X IER = 0x%02X\n",
>                         i >> 4,
>                         (int) psc_read_byte(pIFRbase + i),
>                         (int) psc_read_byte(pIERbase + i));

Any particular reason why you didn't use pr_debug() here? I'm guessing
it's because this is not a known pointer value?

Kars.

Re: [PATCH 2/4] m68k/mac: Disentangle VIA and OSS initialization

[Kars de Jong]

2017-10-27 4:45 GMT+02:00 Finn Thain <fthain@telegraphics.com.au>:
>
> macintosh_config->via_type is meaningless on Mac IIfx (i.e. the only
> model with OSS chip), so skip the via_type switch statement.
>
> Call oss_init() before via_init() because it is more important and
> because that is the right place to initialize the oss_present flag.
>
> On this model, bringing forward oss_init() and delaying via_init()
> is no problem because those functions are independent.
>
> The only requirement here is that oss_register_interrupts() happens
> after via_init(). That is, mac_init_IRQ() happens after config_mac().
>
> Tested-by: Stan Johnson <userm57@yahoo.com>

Was this tested on a Mac IIfx and on at least one other Mac?

> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

Reviewed-by: Kars de Jong <jongk@linux-m68k.org>

Re: [PATCH 2/4] m68k/mac: Disentangle VIA and OSS initialization

[Kars de Jong]

2017-10-27 4:45 GMT+02:00 Finn Thain <fthain@telegraphics.com.au>:
> macintosh_config->via_type is meaningless on Mac IIfx (i.e. the only
> model with OSS chip), so skip the via_type switch statement.
>
> Call oss_init() before via_init() because it is more important and
> because that is the right place to initialize the oss_present flag.
>
> On this model, bringing forward oss_init() and delaying via_init()
> is no problem because those functions are independent.
>
> The only requirement here is that oss_register_interrupts() happens
> after via_init(). That is, mac_init_IRQ() happens after config_mac().
>
> Tested-by: Stan Johnson <userm57@yahoo.com>

Was this tested on a Mac IIfx and on at least one other Mac?

> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

Reviewed-by: Kars de Jong <jongk@linux-m68k.org>

(Geert, Finn: sorry about the spam. Bloody Gmail...)

Kars.

Re: [PATCH 2/4] m68k/mac: Disentangle VIA and OSS initialization

[Finn Thain]

On Fri, 27 Oct 2017, Kars de Jong wrote:

> 2017-10-27 4:45 GMT+02:00 Finn Thain <fthain@telegraphics.com.au>:
> 
> > macintosh_config->via_type is meaningless on Mac IIfx (i.e. the only 
> > model with OSS chip), so skip the via_type switch statement.
> >
> > Call oss_init() before via_init() because it is more important and 
> > because that is the right place to initialize the oss_present flag.
> >
> > On this model, bringing forward oss_init() and delaying via_init() is 
> > no problem because those functions are independent.
> >
> > The only requirement here is that oss_register_interrupts() happens 
> > after via_init(). That is, mac_init_IRQ() happens after config_mac().
> >
> > Tested-by: Stan Johnson <userm57@yahoo.com>
> >
> 
> Was this tested on a Mac IIfx and on at least one other Mac?
> 

Yes, this works fine on IIfx and other Macs. And in theory, it should be 
safe to disable all OSS interrupts before messing with VIA1. (And if 
!oss_present, oss_init() does nothing anyway, so the sequence of calls 
unimportant.)

The comment implies that there is no better reason for the existing 
tangled code than "it seems more logical" (to that author anyway).

> 
> > Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> >
> 
> Reviewed-by: Kars de Jong <jongk@linux-m68k.org>
> 

Thanks for the review.

-- 

> 
> > ---
> >  arch/m68k/mac/config.c |  2 +-
> >  arch/m68k/mac/oss.c    |  8 ++++----
> >  arch/m68k/mac/via.c    | 32 +++++++++++++-------------------
> >  3 files changed, 18 insertions(+), 24 deletions(-)
> >
> > diff --git a/arch/m68k/mac/config.c b/arch/m68k/mac/config.c
> > index 22123f7e8f75..16cd5cea5207 100644
> > --- a/arch/m68k/mac/config.c
> > +++ b/arch/m68k/mac/config.c
> > @@ -898,8 +898,8 @@ static void __init mac_identify(void)
> >                 mac_bi_data.id, mac_bi_data.cpuid, mac_bi_data.memsize);
> >
> >         iop_init();
> > -       via_init();
> >         oss_init();
> > +       via_init();
> >         psc_init();
> >         baboon_init();
> >
> > diff --git a/arch/m68k/mac/oss.c b/arch/m68k/mac/oss.c
> > index ca84dcf41fc9..61906eb67d0f 100644
> > --- a/arch/m68k/mac/oss.c
> > +++ b/arch/m68k/mac/oss.c
> > @@ -31,18 +31,18 @@ volatile struct mac_oss *oss;
> >
> >  /*
> >   * Initialize the OSS
> > - *
> > - * The OSS "detection" code is actually in via_init() which is always
> > called
> > - * before us. Thus we can count on oss_present being valid on entry.
> >   */
> >
> >  void __init oss_init(void)
> >  {
> >         int i;
> >
> > -       if (!oss_present) return;
> > +       if (macintosh_config->ident != MAC_MODEL_IIFX)
> > +               return;
> >
> >         oss = (struct mac_oss *) OSS_BASE;
> > +       pr_debug("OSS detected at %p", oss);
> > +       oss_present = 1;
> >
> >         /* Disable all interrupts. Unlike a VIA it looks like we    */
> >         /* do this by setting the source's interrupt level to zero. */
> > diff --git a/arch/m68k/mac/via.c b/arch/m68k/mac/via.c
> > index 05021381af0b..19ad46ba4787 100644
> > --- a/arch/m68k/mac/via.c
> > +++ b/arch/m68k/mac/via.c
> > @@ -113,10 +113,6 @@ void via_debug_dump(void);
> >   * First we figure out where they actually _are_ as well as what type of
> >   * VIA we have for VIA2 (it could be a real VIA or an RBV or even an OSS.)
> >   * Then we pretty much clear them out and disable all IRQ sources.
> > - *
> > - * Note: the OSS is actually "detected" here and not in oss_init(). It
> > just
> > - *      seems more logical to do it here since via_init() needs to know
> > - *      these things anyways.
> >   */
> >
> >  void __init via_init(void)
> > @@ -124,21 +120,18 @@ void __init via_init(void)
> >         via1 = (void *)VIA1_BASE;
> >         pr_debug("VIA1 detected at %p\n", via1);
> >
> > -       switch(macintosh_config->via_type) {
> > +       if (oss_present) {
> > +               via2 = NULL;
> > +               rbv_present = 0;
> > +       } else {
> > +               switch (macintosh_config->via_type) {
> >
> >                 /* IIci, IIsi, IIvx, IIvi (P6xx), LC series */
> >
> >                 case MAC_VIA_IICI:
> > -                       if (macintosh_config->ident == MAC_MODEL_IIFX) {
> > -                               via2 = NULL;
> > -                               rbv_present = 0;
> > -                               oss_present = 1;
> > -                       } else {
> > -                               via2 = (void *) RBV_BASE;
> > -                               pr_debug("VIA2 (RBV) detected at %p\n",
> > via2);
> > -                               rbv_present = 1;
> > -                               oss_present = 0;
> > -                       }
> > +                       via2 = (void *)RBV_BASE;
> > +                       pr_debug("VIA2 (RBV) detected at %p\n", via2);
> > +                       rbv_present = 1;
> >                         if (macintosh_config->ident == MAC_MODEL_LCIII) {
> >                                 rbv_clear = 0x00;
> >                         } else {
> > @@ -160,15 +153,16 @@ void __init via_init(void)
> >                         via2 = (void *) VIA2_BASE;
> >                         pr_debug("VIA2 detected at %p\n", via2);
> >                         rbv_present = 0;
> > -                       oss_present = 0;
> >                         rbv_clear = 0x00;
> >                         gIER = vIER;
> >                         gIFR = vIFR;
> >                         gBufA = vBufA;
> >                         gBufB = vBufB;
> >                         break;
> > +
> >                 default:
> >                         panic("UNKNOWN VIA TYPE");
> > +               }
> >         }
> >
> >  #ifdef DEBUG_VIA
> > @@ -295,9 +289,9 @@ void via_debug_dump(void)
> >                 (uint) via1[vDirA], (uint) via1[vDirB], (uint) via1[vACR]);
> >         printk(KERN_DEBUG "         PCR = 0x%02X  IFR = 0x%02X IER =
> > 0x%02X\n",
> >                 (uint) via1[vPCR], (uint) via1[vIFR], (uint) via1[vIER]);
> > -       if (oss_present) {
> > -               printk(KERN_DEBUG "VIA2: <OSS>\n");
> > -       } else if (rbv_present) {
> > +       if (!via2)
> > +               return;
> > +       if (rbv_present) {
> >                 printk(KERN_DEBUG "VIA2:  IFR = 0x%02X  IER = 0x%02X\n",
> >                         (uint) via2[rIFR], (uint) via2[rIER]);
> >                 printk(KERN_DEBUG "      SIFR = 0x%02X SIER = 0x%02X\n",
> > --
> > 2.13.6
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> 

Re: [PATCH 1/4] m68k/mac: More printk modernization

[Finn Thain]

On Fri, 27 Oct 2017, Kars de Jong wrote:

> 2017-10-27 4:45 GMT+02:00 Finn Thain <fthain@telegraphics.com.au>:
> > Log message fragments used to be printed on one line but now get split 
> > up. Fix this. Also, suppress log spam that merely prints known pointer 
> > values.
> >
> > Tested-by: Stan Johnson <userm57@yahoo.com>
> > Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> > ---
> >  arch/m68k/mac/baboon.c |  2 +-
> >  arch/m68k/mac/iop.c    |  4 ++--
> >  arch/m68k/mac/psc.c    |  6 ++----
> >  arch/m68k/mac/via.c    | 18 +++++-------------
> >  4 files changed, 10 insertions(+), 20 deletions(-)
> 
> ...
> 
> > diff --git a/arch/m68k/mac/psc.c b/arch/m68k/mac/psc.c
> > index 439a2a2e5874..8d547df4e16c 100644
> > --- a/arch/m68k/mac/psc.c
> > +++ b/arch/m68k/mac/psc.c
> > @@ -42,7 +42,7 @@ static void psc_debug_dump(void)
> >                 return;
> >
> >         for (i = 0x30 ; i < 0x70 ; i += 0x10) {
> > -               printk("PSC #%d:  IFR = 0x%02X IER = 0x%02X\n",
> > +               printk(KERN_DEBUG "PSC #%d:  IFR = 0x%02X IER = 0x%02X\n",
> >                         i >> 4,
> >                         (int) psc_read_byte(pIFRbase + i),
> >                         (int) psc_read_byte(pIERbase + i));
> 
> Any particular reason why you didn't use pr_debug() here? I'm guessing 
> it's because this is not a known pointer value?
> 

It's because the call to psc_debug_dump() is already conditional on #ifdef 
DEBUG_PSC.

Having the printk conditional on both DEBUG and DEBUG_PSC would be 
annoying. And I didn't want an unconditional call to psc_debug_dump() 
because I think PSC_DEBUG could become more useful given that PSC support 
is woefully incomplete.

Generally, I like to avoid pr_debug(). But printk() now works differently, 
and now I have to avoid pr_cont(). And if I can avoid pr_cont() then it 
actually makes sense to use pr_debug().

-- 

> Kars.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/4] m68k/mac: More printk modernization

[Finn Thain]

On Sat, 28 Oct 2017, I wrote:

> > Any particular reason why you didn't use pr_debug() here? I'm guessing 
> > it's because this is not a known pointer value?
> > 
> 
> It's because the call to psc_debug_dump() is already conditional on 
> #ifdef DEBUG_PSC.
> 
> Having the printk conditional on both DEBUG and DEBUG_PSC would be 
> annoying. And I didn't want an unconditional call to psc_debug_dump() 
> because I think PSC_DEBUG could become more useful given that PSC 
> support is woefully incomplete.
> 

Perhaps PSC_DEBUG should be scrapped in favour of DEBUG. Presently 
DEBUG_PSC is set and I think that's useful as long as those drivers are 
incomplete. So we would end up with this:


#define DEBUG

#include ...
...

static void psc_debug_dump(void)
{
	...
	pr_debug(...);
	...
}

void __init psc_init(void)
{
	...
#if DEBUG
	psc_debug_dump()
#endif
	...
}


In this version, the "#define DEBUG" at the top of the file has obscure 
side effects (not just in printk.h) considering all of the headers that 
get included, and their includes, and so on. I still prefer the patch that 
I sent.

-- 

Re: [PATCH 0/4] m68k/mac: Various cleanups and fixes

[Geert Uytterhoeven]

Hi Finn,

On Fri, Oct 27, 2017 at 4:45 AM, Finn Thain <fthain@telegraphics.com.au> wrote:
> Finn Thain (4):
>   m68k/mac: More printk modernization
>   m68k/mac: Disentangle VIA and OSS initialization
>   m68k/mac: Disentangle VIA/RBV and NuBus initialization
>   m68k/mac: Add mutual exclusion for IOP interrupt polling

Thanks, applied, and queued for v4.15.

Gr{oetje,eeting}s,

                        Geert

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

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