mbox series

[0/5] Bring the BusLogic host bus adapter driver up to Y2021

Message ID alpine.DEB.2.21.2104141244520.44318@angie.orcam.me.uk
Headers show
Series Bring the BusLogic host bus adapter driver up to Y2021 | expand

Message

Maciej W. Rozycki April 14, 2021, 10:38 p.m. UTC
Hi,

 First of all, does anyone have a copy of: "MultiMaster UltraSCSI Host 
Adapters for PCI Systems: Technical Reference Manual" (pub. 3002493-E)?  
It used to live in the "Mylex Manuals and Documentation Archives" section 
of the Mylex web site <http://www.mylex.com/pub/manuals/index.htm>, 
specifically at: <http://www.mylex.com/pub/manuals/mmultra.pdf>.

 Another useful document might be: "Wide SCSI Host Adapters for PCI and 
EISA Systems: Technical Reference Manual" (pub. 3000763-A), which used to 
live at: <http://www.mylex.com/pub/manuals/widescsi.pdf>, linked from the 
same place.

 Sadly I didn't get to these resources while they were still there, and 
neither did archive.org, and now they not appear available from anywhere 
online.  I'm sure Leonard had this all, but, alas, he is long gone too.

 It looks to me like either or both documents would help understanding how 
the BusLogic devices (are supposed to) work and possibly deal with issues 
in a better way.

 So we are here owing to Christoph's recent ISA bounce buffering sweep: 
<https://lore.kernel.org/linux-scsi/20210331073001.46776-1-hch@lst.de/T/#m981284e74e93216626a0728ce1601ca18fca92e8> 
which has prompted me to verify the current version of Linux with my old 
server, which has been long equipped with venerable Linux 2.6.18 and which 
I now have available for general experimenting, and the BusLogic BT-958 
PCI SCSI host bus adapter the server has used for 20-something years now.  
This revealed numerous issues with the BusLogic driver.

 Firstly (1/5) it has suffered from some bitrot and messages produced have 
become messy from the lack of update for proper `pr_cont' support.

 Secondly (2/5) there has been a potential buffer overrun/stack corruption 
security issue from using an unbounded `vsprintf' call.

 Thirdly (3/5) it has become obvious the BusLogic driver would have been 
non-functional, should I have upgraded the kernel, at least with this 
configuration for some 8 years now, and the underlying cause has been a 
long-known issue with the MultiMaster firmware I have dealt with already, 
back in 2003.  To put it short the firmware cannot cope with commands that 
request an allocation length exceeding the length of actual data returned.

 I have originally observed it with a LOG SENSE command in the course of 
investigating why smartmontools bring the system to a death, and worked it 
around: <https://sourceforge.net/p/smartmontools/mailman/message/4993087/> 
by issuing the command twice, first just to obtain the allocation length 
required.  As it turns out we need a similar workaround in the kernel now.

 But in the course of investigating this issue I have discovered there is 
a second bottom to it and hence I have prepared follow-up changes (4-5/5) 
to address problems with our handling of Vital Product Data INQUIRY pages.

 See individual change descriptions for further details.

 Questions, comments, concerns?  Otherwise please apply.

  Maciej

Comments

Nix April 15, 2021, 12:42 p.m. UTC | #1
On 14 Apr 2021, Maciej W. Rozycki stated:

> Set the allocation length to 255 for the ATA Information VPD page 

> requested in the WRITE SAME handler, so as not to limit information 

> examined by `scsi_get_vpd_page' in the supported vital product data 

> pages unnecessarily.

>

> Originally it was thought that Areca hardware may have issues with a 

> valid allocation length supplied for a VPD inquiry, however older SCSI 

> standard revisions[1] consider 255 the maximum length allowed and what 


Aaaah. That explains a lot! (Not that I can remember what SCSI standard
rev that Areca firmware claimed to implement. I know I never updated the
firmware, so it's going to be something no newer than mid-2009 and
probably quite a bit older.)

> Nix,

>

>  I can see you're still around.  Would you therefore please be so kind 

> as to verify this change with your Areca hardware if you still have it?


It's been up in the loft for years, but I'll get it out this weekend and
give it a spin :) this'll let me make sure the disks still spin as well,
which matters for an in-case-of-lightning-strike disaster-recovery
backup box.

(I just hope this kernel boots on it at all. It's about three years
since I retired it... let's see!)

>  It looks to me like you were thinking in the right direction with: 

> <https://lore.kernel.org/linux-scsi/87vc3nuipg.fsf@spindle.srvr.nix/>. 


It's the sort of mistake I could see myself making: an easy mistake to
make when so many things in C require buffer size - 1 or you get a
disastrous security hole...

-- 
NULL && (void)
Joe Perches April 16, 2021, 2:08 a.m. UTC | #2
On Thu, 2021-04-15 at 00:39 +0200, Maciej W. Rozycki wrote:
> Update BusLogic driver's messaging system to use `pr_cont' for 
> continuation lines, bringing messy output:
> 
> pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
> scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *****
> scsi: Copyright 1995-1998 by Leonard N. Zubkoff <lnz@dandelion.com>
> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
> scsi0:   Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level
> scsi0:   PCI Bus: 0, Device: 19, Address:
> 0xE0012000,
> Host Adapter SCSI ID: 7
> scsi0:   Parity Checking: Enabled, Extended Translation: Enabled
> scsi0:   Synchronous Negotiation: Ultra, Wide Negotiation: Enabled
> scsi0:   Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled
> scsi0:   Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211
> scsi0:   Driver Queue Depth: 211, Host Adapter Queue Depth: 192
> scsi0:   Tagged Queue Depth:
> Automatic
> , Untagged Queue Depth: 3
> scsi0:   SCSI Bus Termination: Both Enabled
> , SCAM: Disabled
> 
> scsi0: *** BusLogic BT-958 Initialized Successfully ***
> scsi host0: BusLogic BT-958
> 
> back to order:
> 
> pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
> scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *****
> scsi: Copyright 1995-1998 by Leonard N. Zubkoff <lnz@dandelion.com>
> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
> scsi0:   Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level
> scsi0:   PCI Bus: 0, Device: 19, Address: 0xE0012000, Host Adapter SCSI ID: 7
> scsi0:   Parity Checking: Enabled, Extended Translation: Enabled
> scsi0:   Synchronous Negotiation: Ultra, Wide Negotiation: Enabled
> scsi0:   Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled
> scsi0:   Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211
> scsi0:   Driver Queue Depth: 211, Host Adapter Queue Depth: 192
> scsi0:   Tagged Queue Depth: Automatic, Untagged Queue Depth: 3
> scsi0:   SCSI Bus Termination: Both Enabled, SCAM: Disabled
> scsi0: *** BusLogic BT-958 Initialized Successfully ***
> scsi host0: BusLogic BT-958
> 
> Also diagnostic output such as with the `BusLogic=TraceConfiguration' 
> parameter is affected and becomes vertical and therefore hard to read.  
> This has now been corrected, e.g.:
> 
> pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
> blogic_cmd(86) Status = 30:  4 ==>  4: FF 05 93 00
> blogic_cmd(95) Status = 28: (Modify I/O Address)
> blogic_cmd(91) Status = 30:  1 ==>  1: 01
> blogic_cmd(04) Status = 30:  4 ==>  4: 41 41 35 30
> blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 1D
> scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *****
> scsi: Copyright 1995-1998 by Leonard N. Zubkoff <lnz@dandelion.com>
> blogic_cmd(04) Status = 30:  4 ==>  4: 41 41 35 30
> blogic_cmd(0B) Status = 30:  3 ==>  3: 00 08 07
> blogic_cmd(0D) Status = 30: 34 ==> 34: 03 01 07 04 00 00 00 00 00 00 00 00 00 00 00 00 FF 42 44 46 FF 00 00 00 00 00 00 00 00 00 FF 00 FF 00
> blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 1D
> blogic_cmd(84) Status = 30:  1 ==>  1: 37
> blogic_cmd(8B) Status = 30:  5 ==>  5: 39 35 38 20 20
> blogic_cmd(85) Status = 30:  1 ==>  1: 42
> blogic_cmd(86) Status = 30:  4 ==>  4: FF 05 93 00
> blogic_cmd(91) Status = 30: 64 ==> 64: 41 46 3E 20 39 35 38 20 20 00 C4 00 04 01 07 2F 07 04 35 FF FF FF FF FF FF FF FF FF FF 01 00 FE FF 08 FF FF 00 00 00 00 00 00 00 01 00 01 00 00 FF FF 00 00 00 00 00 00 00 00 00 00 00 00 00 FC
> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
> 
> etc.

In patch 2, vscnprintf should probably be used to make sure it's
0 terminated.

And while it's a lot more code, I'd prefer a solution that looks more
like the other commonly used kernel logging extension mechanisms
where adapter is placed before the format, ... in the argument list.

Today it's:

void blogic_msg(enum, fmt, adapter, ...);

without the __printf marking so there is one format/arg mismatch.

fyi: in the suggested patch below it's
-			blogic_info("BIOS Address: 0x%lX, ", adapter,
-					adapter->bios_addr);
+			blogic_cont(adapter, "BIOS Address: 0x%X, ",
+				    adapter->bios_addr);

I'd prefer
__printf(3, 4)
void blogic_msg(enum, adapter, fmt, ...)

(or maybe void blogic_msg(adapter, enum, fmt, ...))

And there's a simple addition of a blogic_cont macro and extension
to blogic_msg to simplify the logic and obviousness of the logging
extension lines too.

I suggest this done with coccinelle and a little typing:
---
 drivers/scsi/BusLogic.c | 496 +++++++++++++++++++++++++++++++-----------------
 drivers/scsi/BusLogic.h |  32 ++--
 2 files changed, 341 insertions(+), 187 deletions(-)

diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index ccb061ab0a0a..7a52371b5ab6 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -134,8 +134,10 @@ static char *blogic_cmd_failure_reason;
 
 static void blogic_announce_drvr(struct blogic_adapter *adapter)
 {
-	blogic_announce("***** BusLogic SCSI Driver Version " blogic_drvr_version " of " blogic_drvr_date " *****\n", adapter);
-	blogic_announce("Copyright 1995-1998 by Leonard N. Zubkoff <lnz@dandelion.com>\n", adapter);
+	blogic_announce(adapter,
+			"***** BusLogic SCSI Driver Version " blogic_drvr_version " of " blogic_drvr_date " *****\n");
+	blogic_announce(adapter,
+			"Copyright 1995-1998 by Leonard N. Zubkoff <lnz@dandelion.com>\n");
 }
 
 
@@ -198,8 +200,7 @@ static bool __init blogic_create_initccbs(struct blogic_adapter *adapter)
 		blk_pointer = dma_alloc_coherent(&adapter->pci_device->dev,
 				blk_size, &blkp, GFP_KERNEL);
 		if (blk_pointer == NULL) {
-			blogic_err("UNABLE TO ALLOCATE CCB GROUP - DETACHING\n",
-					adapter);
+			blogic_err(adapter, "UNABLE TO ALLOCATE CCB GROUP - DETACHING\n");
 			return false;
 		}
 		blogic_init_ccbs(adapter, blk_pointer, blk_size, blkp);
@@ -259,10 +260,13 @@ static void blogic_create_addlccbs(struct blogic_adapter *adapter,
 	}
 	if (adapter->alloc_ccbs > prev_alloc) {
 		if (print_success)
-			blogic_notice("Allocated %d additional CCBs (total now %d)\n", adapter, adapter->alloc_ccbs - prev_alloc, adapter->alloc_ccbs);
+			blogic_notice(adapter,
+				      "Allocated %d additional CCBs (total now %d)\n",
+				      adapter->alloc_ccbs - prev_alloc,
+				      adapter->alloc_ccbs);
 		return;
 	}
-	blogic_notice("Failed to allocate additional CCBs\n", adapter);
+	blogic_notice(adapter, "Failed to allocate additional CCBs\n");
 	if (adapter->drvr_qdepth > adapter->alloc_ccbs - adapter->tgt_count) {
 		adapter->drvr_qdepth = adapter->alloc_ccbs - adapter->tgt_count;
 		adapter->scsi_host->can_queue = adapter->drvr_qdepth;
@@ -441,7 +445,9 @@ static int blogic_cmd(struct blogic_adapter *adapter, enum blogic_opcode opcode,
 			goto done;
 		}
 		if (blogic_global_options.trace_config)
-			blogic_notice("blogic_cmd(%02X) Status = %02X: (Modify I/O Address)\n", adapter, opcode, statusreg.all);
+			blogic_notice(adapter,
+				      "blogic_cmd(%02X) Status = %02X: (Modify I/O Address)\n",
+				      opcode, statusreg.all);
 		result = 0;
 		goto done;
 	}
@@ -499,15 +505,16 @@ static int blogic_cmd(struct blogic_adapter *adapter, enum blogic_opcode opcode,
 	 */
 	if (blogic_global_options.trace_config) {
 		int i;
-		blogic_notice("blogic_cmd(%02X) Status = %02X: %2d ==> %2d:",
-				adapter, opcode, statusreg.all, replylen,
+		blogic_notice(adapter,
+			      "blogic_cmd(%02X) Status = %02X: %2d ==> %2d:",
+			      opcode, statusreg.all, replylen,
 				reply_b);
 		if (replylen > reply_b)
 			replylen = reply_b;
 		for (i = 0; i < replylen; i++)
-			blogic_notice(" %02X", adapter,
-					((unsigned char *) reply)[i]);
-		blogic_notice("\n", adapter);
+			blogic_cont(adapter, " %02X",
+				    ((unsigned char *)reply)[i]);
+		blogic_cont(adapter, "\n");
 	}
 	/*
 	   Process Command Invalid conditions.
@@ -717,23 +724,37 @@ static int __init blogic_init_mm_probeinfo(struct blogic_adapter *adapter)
 		pci_addr = base_addr1 = pci_resource_start(pci_device, 1);
 
 		if (pci_resource_flags(pci_device, 0) & IORESOURCE_MEM) {
-			blogic_err("BusLogic: Base Address0 0x%lX not I/O for MultiMaster Host Adapter\n", NULL, base_addr0);
-			blogic_err("at PCI Bus %d Device %d I/O Address 0x%lX\n", NULL, bus, device, io_addr);
+			blogic_err(NULL,
+				   "BusLogic: Base Address0 0x%lX not I/O for MultiMaster Host Adapter\n",
+				   base_addr0);
+			blogic_err(NULL,
+				   "at PCI Bus %d Device %d I/O Address 0x%lX\n",
+				   bus, device, io_addr);
 			continue;
 		}
 		if (pci_resource_flags(pci_device, 1) & IORESOURCE_IO) {
-			blogic_err("BusLogic: Base Address1 0x%lX not Memory for MultiMaster Host Adapter\n", NULL, base_addr1);
-			blogic_err("at PCI Bus %d Device %d PCI Address 0x%lX\n", NULL, bus, device, pci_addr);
+			blogic_err(NULL,
+				   "BusLogic: Base Address1 0x%lX not Memory for MultiMaster Host Adapter\n",
+				   base_addr1);
+			blogic_err(NULL,
+				   "at PCI Bus %d Device %d PCI Address 0x%lX\n",
+				   bus, device, pci_addr);
 			continue;
 		}
 		if (irq_ch == 0) {
-			blogic_err("BusLogic: IRQ Channel %d invalid for MultiMaster Host Adapter\n", NULL, irq_ch);
-			blogic_err("at PCI Bus %d Device %d I/O Address 0x%lX\n", NULL, bus, device, io_addr);
+			blogic_err(NULL,
+				   "BusLogic: IRQ Channel %d invalid for MultiMaster Host Adapter\n",
+				   irq_ch);
+			blogic_err(NULL,
+				   "at PCI Bus %d Device %d I/O Address 0x%lX\n",
+				   bus, device, io_addr);
 			continue;
 		}
 		if (blogic_global_options.trace_probe) {
-			blogic_notice("BusLogic: PCI MultiMaster Host Adapter detected at\n", NULL);
-			blogic_notice("BusLogic: PCI Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX\n", NULL, bus, device, io_addr, pci_addr);
+			blogic_notice(NULL, "BusLogic: PCI MultiMaster Host Adapter detected at\n");
+			blogic_notice(NULL,
+				      "BusLogic: PCI Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX\n",
+				      bus, device, io_addr, pci_addr);
 		}
 		/*
 		   Issue the Inquire PCI Host Adapter Information command to determine
@@ -819,7 +840,7 @@ static int __init blogic_init_mm_probeinfo(struct blogic_adapter *adapter)
 			nonpr_mmcount++;
 			mmcount++;
 		} else
-			blogic_warn("BusLogic: Too many Host Adapters detected\n", NULL);
+			blogic_warn(NULL, "BusLogic: Too many Host Adapters detected\n");
 	}
 	/*
 	   If the AutoSCSI "Use Bus And Device # For PCI Scanning Seq."
@@ -957,23 +978,37 @@ static int __init blogic_init_fp_probeinfo(struct blogic_adapter *adapter)
 		pci_addr = base_addr1 = pci_resource_start(pci_device, 1);
 #ifdef CONFIG_SCSI_FLASHPOINT
 		if (pci_resource_flags(pci_device, 0) & IORESOURCE_MEM) {
-			blogic_err("BusLogic: Base Address0 0x%lX not I/O for FlashPoint Host Adapter\n", NULL, base_addr0);
-			blogic_err("at PCI Bus %d Device %d I/O Address 0x%lX\n", NULL, bus, device, io_addr);
+			blogic_err(NULL,
+				   "BusLogic: Base Address0 0x%lX not I/O for FlashPoint Host Adapter\n",
+				   base_addr0);
+			blogic_err(NULL,
+				   "at PCI Bus %d Device %d I/O Address 0x%lX\n",
+				   bus, device, io_addr);
 			continue;
 		}
 		if (pci_resource_flags(pci_device, 1) & IORESOURCE_IO) {
-			blogic_err("BusLogic: Base Address1 0x%lX not Memory for FlashPoint Host Adapter\n", NULL, base_addr1);
-			blogic_err("at PCI Bus %d Device %d PCI Address 0x%lX\n", NULL, bus, device, pci_addr);
+			blogic_err(NULL,
+				   "BusLogic: Base Address1 0x%lX not Memory for FlashPoint Host Adapter\n",
+				   base_addr1);
+			blogic_err(NULL,
+				   "at PCI Bus %d Device %d PCI Address 0x%lX\n",
+				   bus, device, pci_addr);
 			continue;
 		}
 		if (irq_ch == 0) {
-			blogic_err("BusLogic: IRQ Channel %d invalid for FlashPoint Host Adapter\n", NULL, irq_ch);
-			blogic_err("at PCI Bus %d Device %d I/O Address 0x%lX\n", NULL, bus, device, io_addr);
+			blogic_err(NULL,
+				   "BusLogic: IRQ Channel %d invalid for FlashPoint Host Adapter\n",
+				   irq_ch);
+			blogic_err(NULL,
+				   "at PCI Bus %d Device %d I/O Address 0x%lX\n",
+				   bus, device, io_addr);
 			continue;
 		}
 		if (blogic_global_options.trace_probe) {
-			blogic_notice("BusLogic: FlashPoint Host Adapter detected at\n", NULL);
-			blogic_notice("BusLogic: PCI Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX\n", NULL, bus, device, io_addr, pci_addr);
+			blogic_notice(NULL, "BusLogic: FlashPoint Host Adapter detected at\n");
+			blogic_notice(NULL,
+				      "BusLogic: PCI Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX\n",
+				      bus, device, io_addr, pci_addr);
 		}
 		if (blogic_probeinfo_count < BLOGIC_MAX_ADAPTERS) {
 			struct blogic_probeinfo *probeinfo =
@@ -988,11 +1023,15 @@ static int __init blogic_init_fp_probeinfo(struct blogic_adapter *adapter)
 			probeinfo->pci_device = pci_dev_get(pci_device);
 			fpcount++;
 		} else
-			blogic_warn("BusLogic: Too many Host Adapters detected\n", NULL);
+			blogic_warn(NULL, "BusLogic: Too many Host Adapters detected\n");
 #else
-		blogic_err("BusLogic: FlashPoint Host Adapter detected at PCI Bus %d Device %d\n", NULL, bus, device);
-		blogic_err("BusLogic: I/O Address 0x%lX PCI Address 0x%lX, irq %d, but FlashPoint\n", NULL, io_addr, pci_addr, irq_ch);
-		blogic_err("BusLogic: support was omitted in this kernel configuration.\n", NULL);
+		blogic_err(NULL,
+			   "BusLogic: FlashPoint Host Adapter detected at PCI Bus %d Device %d\n",
+			   bus, device);
+		blogic_err(NULL,
+			   "BusLogic: I/O Address 0x%lX PCI Address 0x%lX, irq %d, but FlashPoint\n",
+			   io_addr, pci_addr, irq_ch);
+		blogic_err("BusLogic: support was omitted in this kernel configuration\n", NULL);
 #endif
 	}
 	/*
@@ -1098,15 +1137,19 @@ static bool blogic_failure(struct blogic_adapter *adapter, char *msg)
 {
 	blogic_announce_drvr(adapter);
 	if (adapter->adapter_bus_type == BLOGIC_PCI_BUS) {
-		blogic_err("While configuring BusLogic PCI Host Adapter at\n",
-				adapter);
-		blogic_err("Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX:\n", adapter, adapter->bus, adapter->dev, adapter->io_addr, adapter->pci_addr);
+		blogic_err(adapter, "While configuring BusLogic PCI Host Adapter at\n");
+		blogic_err(adapter,
+			   "Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX:\n",
+			   adapter->bus, adapter->dev, adapter->io_addr,
+			   adapter->pci_addr);
 	} else
-		blogic_err("While configuring BusLogic Host Adapter at I/O Address 0x%lX:\n", adapter, adapter->io_addr);
-	blogic_err("%s FAILED - DETACHING\n", adapter, msg);
+		blogic_err(adapter,
+			   "While configuring BusLogic Host Adapter at I/O Address 0x%lX:\n",
+			   adapter->io_addr);
+	blogic_err(adapter, "%s FAILED - DETACHING\n", msg);
 	if (blogic_cmd_failure_reason != NULL)
-		blogic_err("ADDITIONAL FAILURE INFO - %s\n", adapter,
-				blogic_cmd_failure_reason);
+		blogic_err(adapter, "ADDITIONAL FAILURE INFO - %s\n",
+			   blogic_cmd_failure_reason);
 	return false;
 }
 
@@ -1130,13 +1173,20 @@ static bool __init blogic_probe(struct blogic_adapter *adapter)
 		fpinfo->present = false;
 		if (!(FlashPoint_ProbeHostAdapter(fpinfo) == 0 &&
 					fpinfo->present)) {
-			blogic_err("BusLogic: FlashPoint Host Adapter detected at PCI Bus %d Device %d\n", adapter, adapter->bus, adapter->dev);
-			blogic_err("BusLogic: I/O Address 0x%lX PCI Address 0x%lX, but FlashPoint\n", adapter, adapter->io_addr, adapter->pci_addr);
-			blogic_err("BusLogic: Probe Function failed to validate it.\n", adapter);
+			blogic_err(adapter,
+				   "BusLogic: FlashPoint Host Adapter detected at PCI Bus %d Device %d\n",
+				   adapter->bus, adapter->dev);
+			blogic_err(adapter,
+				   "BusLogic: I/O Address 0x%lX PCI Address 0x%lX, but FlashPoint\n",
+				   adapter->io_addr, adapter->pci_addr);
+			blogic_err(adapter,
+				   "BusLogic: Probe Function failed to validate it\n");
 			return false;
 		}
 		if (blogic_global_options.trace_probe)
-			blogic_notice("BusLogic_Probe(0x%lX): FlashPoint Found\n", adapter, adapter->io_addr);
+			blogic_notice(adapter,
+				      "BusLogic_Probe(0x%lX): FlashPoint Found\n",
+				      adapter->io_addr);
 		/*
 		   Indicate the Host Adapter Probe completed successfully.
 		 */
@@ -1153,7 +1203,10 @@ static bool __init blogic_probe(struct blogic_adapter *adapter)
 	intreg.all = blogic_rdint(adapter);
 	georeg.all = blogic_rdgeom(adapter);
 	if (blogic_global_options.trace_probe)
-		blogic_notice("BusLogic_Probe(0x%lX): Status 0x%02X, Interrupt 0x%02X, Geometry 0x%02X\n", adapter, adapter->io_addr, statusreg.all, intreg.all, georeg.all);
+		blogic_notice(adapter,
+			      "BusLogic_Probe(0x%lX): Status 0x%02X, Interrupt 0x%02X, Geometry 0x%02X\n",
+			      adapter->io_addr, statusreg.all, intreg.all,
+			      georeg.all);
 	if (statusreg.all == 0 || statusreg.sr.diag_active ||
 			statusreg.sr.cmd_param_busy || statusreg.sr.rsvd ||
 			statusreg.sr.cmd_invalid || intreg.ir.rsvd != 0)
@@ -1232,7 +1285,9 @@ static bool blogic_hwreset(struct blogic_adapter *adapter, bool hard_reset)
 		udelay(100);
 	}
 	if (blogic_global_options.trace_hw_reset)
-		blogic_notice("BusLogic_HardwareReset(0x%lX): Diagnostic Active, Status 0x%02X\n", adapter, adapter->io_addr, statusreg.all);
+		blogic_notice(adapter,
+			      "BusLogic_HardwareReset(0x%lX): Diagnostic Active, Status 0x%02X\n",
+			      adapter->io_addr, statusreg.all);
 	if (timeout < 0)
 		return false;
 	/*
@@ -1252,7 +1307,9 @@ static bool blogic_hwreset(struct blogic_adapter *adapter, bool hard_reset)
 		udelay(100);
 	}
 	if (blogic_global_options.trace_hw_reset)
-		blogic_notice("BusLogic_HardwareReset(0x%lX): Diagnostic Completed, Status 0x%02X\n", adapter, adapter->io_addr, statusreg.all);
+		blogic_notice(adapter,
+			      "BusLogic_HardwareReset(0x%lX): Diagnostic Completed, Status 0x%02X\n",
+			      adapter->io_addr, statusreg.all);
 	if (timeout < 0)
 		return false;
 	/*
@@ -1268,7 +1325,9 @@ static bool blogic_hwreset(struct blogic_adapter *adapter, bool hard_reset)
 		udelay(100);
 	}
 	if (blogic_global_options.trace_hw_reset)
-		blogic_notice("BusLogic_HardwareReset(0x%lX): Host Adapter Ready, Status 0x%02X\n", adapter, adapter->io_addr, statusreg.all);
+		blogic_notice(adapter,
+			      "BusLogic_HardwareReset(0x%lX): Host Adapter Ready, Status 0x%02X\n",
+			      adapter->io_addr, statusreg.all);
 	if (timeout < 0)
 		return false;
 	/*
@@ -1280,11 +1339,11 @@ static bool blogic_hwreset(struct blogic_adapter *adapter, bool hard_reset)
 	if (statusreg.sr.diag_failed || !statusreg.sr.adapter_ready) {
 		blogic_cmd_failure_reason = NULL;
 		blogic_failure(adapter, "HARD RESET DIAGNOSTICS");
-		blogic_err("HOST ADAPTER STATUS REGISTER = %02X\n", adapter,
-				statusreg.all);
+		blogic_err(adapter, "HOST ADAPTER STATUS REGISTER = %02X\n",
+			   statusreg.all);
 		if (statusreg.sr.datain_ready)
-			blogic_err("HOST ADAPTER ERROR CODE = %d\n", adapter,
-					blogic_rddatain(adapter));
+			blogic_err(adapter, "HOST ADAPTER ERROR CODE = %d\n",
+				   blogic_rddatain(adapter));
 		return false;
 	}
 	/*
@@ -1324,9 +1383,10 @@ static bool __init blogic_checkadapter(struct blogic_adapter *adapter)
 	   Provide tracing information if requested and return.
 	 */
 	if (blogic_global_options.trace_probe)
-		blogic_notice("BusLogic_Check(0x%lX): MultiMaster %s\n", adapter,
-				adapter->io_addr,
-				(result ? "Found" : "Not Found"));
+		blogic_notice(adapter,
+			      "BusLogic_Check(0x%lX): MultiMaster %s\n",
+			      adapter->io_addr,
+			      (result ? "Found" : "Not Found"));
 	return result;
 }
 
@@ -1836,30 +1896,40 @@ static bool __init blogic_reportconfig(struct blogic_adapter *adapter)
 	char *tagq_msg = tagq_str;
 	int tgt_id;
 
-	blogic_info("Configuring BusLogic Model %s %s%s%s%s SCSI Host Adapter\n", adapter, adapter->model, blogic_adapter_busnames[adapter->adapter_bus_type], (adapter->wide ? " Wide" : ""), (adapter->differential ? " Differential" : ""), (adapter->ultra ? " Ultra" : ""));
-	blogic_info("  Firmware Version: %s, I/O Address: 0x%lX, IRQ Channel: %d/%s\n", adapter, adapter->fw_ver, adapter->io_addr, adapter->irq_ch, (adapter->level_int ? "Level" : "Edge"));
+	blogic_info(adapter,
+		    "Configuring BusLogic Model %s %s%s%s%s SCSI Host Adapter\n",
+		    adapter->model,
+		    blogic_adapter_busnames[adapter->adapter_bus_type],
+		    (adapter->wide ? " Wide" : ""),
+		    (adapter->differential ? " Differential" : ""),
+		    (adapter->ultra ? " Ultra" : ""));
+	blogic_info(adapter,
+		    "  Firmware Version: %s, I/O Address: 0x%lX, IRQ Channel: %d/%s\n",
+		    adapter->fw_ver, adapter->io_addr, adapter->irq_ch,
+		    (adapter->level_int ? "Level" : "Edge"));
 	if (adapter->adapter_bus_type != BLOGIC_PCI_BUS) {
-		blogic_info("  DMA Channel: ", adapter);
+		blogic_info(adapter, "  DMA Channel: ");
 		if (adapter->dma_ch > 0)
-			blogic_info("%d, ", adapter, adapter->dma_ch);
+			blogic_cont(adapter, "%d, ", adapter->dma_ch);
 		else
-			blogic_info("None, ", adapter);
+			blogic_cont(adapter, "None, ");
 		if (adapter->bios_addr > 0)
-			blogic_info("BIOS Address: 0x%lX, ", adapter,
-					adapter->bios_addr);
+			blogic_cont(adapter, "BIOS Address: 0x%X, ",
+				    adapter->bios_addr);
 		else
-			blogic_info("BIOS Address: None, ", adapter);
+			blogic_cont(adapter, "BIOS Address: None, ");
 	} else {
-		blogic_info("  PCI Bus: %d, Device: %d, Address: ", adapter,
-				adapter->bus, adapter->dev);
+		blogic_cont(adapter, "  PCI Bus: %d, Device: %d, Address: ",
+			    adapter->bus, adapter->dev);
 		if (adapter->pci_addr > 0)
-			blogic_info("0x%lX, ", adapter, adapter->pci_addr);
+			blogic_cont(adapter, "0x%lX, ", adapter->pci_addr);
 		else
-			blogic_info("Unassigned, ", adapter);
+			blogic_cont(adapter, "Unassigned, ");
 	}
-	blogic_info("Host Adapter SCSI ID: %d\n", adapter, adapter->scsi_id);
-	blogic_info("  Parity Checking: %s, Extended Translation: %s\n",
-			adapter, (adapter->parity ? "Enabled" : "Disabled"),
+	blogic_cont(adapter, "Host Adapter SCSI ID: %d\n", adapter->scsi_id);
+	blogic_info(adapter,
+		    "  Parity Checking: %s, Extended Translation: %s\n",
+			(adapter->parity ? "Enabled" : "Disabled"),
 			(adapter->ext_trans_enable ? "Enabled" : "Disabled"));
 	alltgt_mask &= ~(1 << adapter->scsi_id);
 	sync_ok = adapter->sync_ok & alltgt_mask;
@@ -1928,16 +1998,25 @@ static bool __init blogic_reportconfig(struct blogic_adapter *adapter)
 		tagq_str[adapter->scsi_id] = '#';
 		tagq_str[adapter->maxdev] = '\0';
 	}
-	blogic_info("  Synchronous Negotiation: %s, Wide Negotiation: %s\n",
-			adapter, syncmsg, widemsg);
-	blogic_info("  Disconnect/Reconnect: %s, Tagged Queuing: %s\n", adapter,
-			discon_msg, tagq_msg);
+	blogic_info(adapter,
+		    "  Synchronous Negotiation: %s, Wide Negotiation: %s\n",
+		    syncmsg, widemsg);
+	blogic_info(adapter,
+		    "  Disconnect/Reconnect: %s, Tagged Queuing: %s\n",
+		    discon_msg, tagq_msg);
 	if (blogic_multimaster_type(adapter)) {
-		blogic_info("  Scatter/Gather Limit: %d of %d segments, Mailboxes: %d\n", adapter, adapter->drvr_sglimit, adapter->adapter_sglimit, adapter->mbox_count);
-		blogic_info("  Driver Queue Depth: %d, Host Adapter Queue Depth: %d\n", adapter, adapter->drvr_qdepth, adapter->adapter_qdepth);
+		blogic_info(adapter,
+			    "  Scatter/Gather Limit: %d of %d segments, Mailboxes: %d\n",
+			    adapter->drvr_sglimit, adapter->adapter_sglimit,
+			    adapter->mbox_count);
+		blogic_info(adapter,
+			    "  Driver Queue Depth: %d, Host Adapter Queue Depth: %d\n",
+			    adapter->drvr_qdepth, adapter->adapter_qdepth);
 	} else
-		blogic_info("  Driver Queue Depth: %d, Scatter/Gather Limit: %d segments\n", adapter, adapter->drvr_qdepth, adapter->drvr_sglimit);
-	blogic_info("  Tagged Queue Depth: ", adapter);
+		blogic_info(adapter,
+			    "  Driver Queue Depth: %d, Scatter/Gather Limit: %d segments\n",
+			    adapter->drvr_qdepth, adapter->drvr_sglimit);
+	blogic_info(adapter, "  Tagged Queue Depth: ");
 	common_tagq_depth = true;
 	for (tgt_id = 1; tgt_id < adapter->maxdev; tgt_id++)
 		if (adapter->qdepth[tgt_id] != adapter->qdepth[0]) {
@@ -1946,24 +2025,28 @@ static bool __init blogic_reportconfig(struct blogic_adapter *adapter)
 		}
 	if (common_tagq_depth) {
 		if (adapter->qdepth[0] > 0)
-			blogic_info("%d", adapter, adapter->qdepth[0]);
+			blogic_cont(adapter, "%d", adapter->qdepth[0]);
 		else
-			blogic_info("Automatic", adapter);
+			blogic_cont(adapter, "Automatic");
 	} else
-		blogic_info("Individual", adapter);
-	blogic_info(", Untagged Queue Depth: %d\n", adapter,
-			adapter->untag_qdepth);
+		blogic_cont(adapter, "Individual");
+	blogic_cont(adapter, ", Untagged Queue Depth: %d\n",
+		    adapter->untag_qdepth);
 	if (adapter->terminfo_valid) {
 		if (adapter->wide)
-			blogic_info("  SCSI Bus Termination: %s", adapter,
-				(adapter->low_term ? (adapter->high_term ? "Both Enabled" : "Low Enabled") : (adapter->high_term ? "High Enabled" : "Both Disabled")));
+			blogic_info(adapter, "  SCSI Bus Termination: %s",
+				    (adapter->low_term
+				     ? (adapter->high_term ? "Both Enabled" : "Low Enabled")
+				     : (adapter->high_term ? "High Enabled" : "Both Disabled")));
 		else
-			blogic_info("  SCSI Bus Termination: %s", adapter,
-				(adapter->low_term ? "Enabled" : "Disabled"));
+			blogic_info(adapter, "  SCSI Bus Termination: %s",
+				    (adapter->low_term ? "Enabled" : "Disabled"));
 		if (adapter->scam)
-			blogic_info(", SCAM: %s", adapter,
-				(adapter->scam_enabled ? (adapter->scam_lev2 ? "Enabled, Level 2" : "Enabled, Level 1") : "Disabled"));
-		blogic_info("\n", adapter);
+			blogic_cont(adapter, ", SCAM: %s",
+				    (adapter->scam_enabled
+				     ? (adapter->scam_lev2 ? "Enabled, Level 2" : "Enabled, Level 1")
+				     : "Disabled"));
+		blogic_cont(adapter, "\n");
 	}
 	/*
 	   Indicate reporting the Host Adapter configuration completed
@@ -1981,8 +2064,8 @@ static bool __init blogic_reportconfig(struct blogic_adapter *adapter)
 static bool __init blogic_getres(struct blogic_adapter *adapter)
 {
 	if (adapter->irq_ch == 0) {
-		blogic_err("NO LEGAL INTERRUPT CHANNEL ASSIGNED - DETACHING\n",
-				adapter);
+		blogic_err(adapter,
+			   "NO LEGAL INTERRUPT CHANNEL ASSIGNED - DETACHING\n");
 		return false;
 	}
 	/*
@@ -1990,8 +2073,9 @@ static bool __init blogic_getres(struct blogic_adapter *adapter)
 	 */
 	if (request_irq(adapter->irq_ch, blogic_inthandler, IRQF_SHARED,
 				adapter->full_model, adapter) < 0) {
-		blogic_err("UNABLE TO ACQUIRE IRQ CHANNEL %d - DETACHING\n",
-				adapter, adapter->irq_ch);
+		blogic_err(adapter,
+			   "UNABLE TO ACQUIRE IRQ CHANNEL %d - DETACHING\n",
+			   adapter->irq_ch);
 		return false;
 	}
 	adapter->irq_acquired = true;
@@ -2000,7 +2084,9 @@ static bool __init blogic_getres(struct blogic_adapter *adapter)
 	 */
 	if (adapter->dma_ch > 0) {
 		if (request_dma(adapter->dma_ch, adapter->full_model) < 0) {
-			blogic_err("UNABLE TO ACQUIRE DMA CHANNEL %d - DETACHING\n", adapter, adapter->dma_ch);
+			blogic_err(adapter,
+				   "UNABLE TO ACQUIRE DMA CHANNEL %d - DETACHING\n",
+				   adapter->dma_ch);
 			return false;
 		}
 		set_dma_mode(adapter->dma_ch, DMA_MODE_CASCADE);
@@ -2148,12 +2234,12 @@ static bool blogic_initadapter(struct blogic_adapter *adapter)
 	 */
 done:
 	if (!adapter->adapter_initd) {
-		blogic_info("*** %s Initialized Successfully ***\n", adapter,
-				adapter->full_model);
-		blogic_info("\n", adapter);
+		blogic_info(adapter, "*** %s Initialized Successfully ***\n",
+			    adapter->full_model);
+		blogic_info(adapter, "\n");
 	} else
-		blogic_warn("*** %s Initialized Successfully ***\n", adapter,
-				adapter->full_model);
+		blogic_warn(adapter, "*** %s Initialized Successfully ***\n",
+			    adapter->full_model);
 	adapter->adapter_initd = true;
 
 	/*
@@ -2365,15 +2451,16 @@ static int __init blogic_init(void)
 	    kcalloc(BLOGIC_MAX_ADAPTERS, sizeof(struct blogic_probeinfo),
 			    GFP_KERNEL);
 	if (blogic_probeinfo_list == NULL) {
-		blogic_err("BusLogic: Unable to allocate Probe Info List\n",
-				NULL);
+		blogic_err(NULL,
+			   "BusLogic: Unable to allocate Probe Info List\n");
 		return -ENOMEM;
 	}
 
 	adapter = kzalloc(sizeof(struct blogic_adapter), GFP_KERNEL);
 	if (adapter == NULL) {
 		kfree(blogic_probeinfo_list);
-		blogic_err("BusLogic: Unable to allocate Prototype Host Adapter\n", NULL);
+		blogic_err(NULL,
+			   "BusLogic: Unable to allocate Prototype Host Adapter\n");
 		return -ENOMEM;
 	}
 
@@ -2633,8 +2720,9 @@ static int blogic_resultcode(struct blogic_adapter *adapter,
 	case BLOGIC_INVALID_OUTBOX_CODE:
 	case BLOGIC_INVALID_CMD_CODE:
 	case BLOGIC_BAD_CMD_PARAM:
-		blogic_warn("BusLogic Driver Protocol Error 0x%02X\n",
-				adapter, adapter_status);
+		blogic_warn(adapter,
+			    "BusLogic Driver Protocol Error 0x%02X\n",
+			    adapter_status);
 		fallthrough;
 	case BLOGIC_DATA_UNDERRUN:
 	case BLOGIC_DATA_OVERRUN:
@@ -2659,8 +2747,8 @@ static int blogic_resultcode(struct blogic_adapter *adapter,
 		hoststatus = DID_RESET;
 		break;
 	default:
-		blogic_warn("Unknown Host Adapter Status 0x%02X\n", adapter,
-				adapter_status);
+		blogic_warn(adapter, "Unknown Host Adapter Status 0x%02X\n",
+			    adapter_status);
 		hoststatus = DID_ERROR;
 		break;
 	}
@@ -2718,7 +2806,9 @@ static void blogic_scan_inbox(struct blogic_adapter *adapter)
 				   then there is most likely a bug in
 				   the Host Adapter firmware.
 				 */
-				blogic_warn("Illegal CCB #%ld status %d in Incoming Mailbox\n", adapter, ccb->serial, ccb->status);
+				blogic_warn(adapter,
+					    "Illegal CCB #%ld status %d in Incoming Mailbox\n",
+					    ccb->serial, ccb->status);
 			}
 		}
 		next_inbox->comp_code = BLOGIC_INBOX_FREE;
@@ -2753,7 +2843,9 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
 		if (ccb->opcode == BLOGIC_BDR) {
 			int tgt_id = ccb->tgt_id;
 
-			blogic_warn("Bus Device Reset CCB #%ld to Target %d Completed\n", adapter, ccb->serial, tgt_id);
+			blogic_warn(adapter,
+				    "Bus Device Reset CCB #%ld to Target %d Completed\n",
+				    ccb->serial, tgt_id);
 			blogic_inc_count(&adapter->tgt_stats[tgt_id].bdr_done);
 			adapter->tgt_flags[tgt_id].tagq_active = false;
 			adapter->cmds_since_rst[tgt_id] = 0;
@@ -2806,7 +2898,9 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
 			case BLOGIC_INBOX_FREE:
 			case BLOGIC_CMD_NOTFOUND:
 			case BLOGIC_INVALID_CCB:
-				blogic_warn("CCB #%ld to Target %d Impossible State\n", adapter, ccb->serial, ccb->tgt_id);
+				blogic_warn(adapter,
+					    "CCB #%ld to Target %d Impossible State\n",
+					    ccb->serial, ccb->tgt_id);
 				break;
 			case BLOGIC_CMD_COMPLETE_GOOD:
 				adapter->tgt_stats[ccb->tgt_id]
@@ -2816,8 +2910,9 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
 				command->result = DID_OK << 16;
 				break;
 			case BLOGIC_CMD_ABORT_BY_HOST:
-				blogic_warn("CCB #%ld to Target %d Aborted\n",
-					adapter, ccb->serial, ccb->tgt_id);
+				blogic_warn(adapter,
+					    "CCB #%ld to Target %d Aborted\n",
+					    ccb->serial, ccb->tgt_id);
 				blogic_inc_count(&adapter->tgt_stats[ccb->tgt_id].aborts_done);
 				command->result = DID_ABORT << 16;
 				break;
@@ -2829,16 +2924,26 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
 					    .cmds_complete++;
 					if (blogic_global_options.trace_err) {
 						int i;
-						blogic_notice("CCB #%ld Target %d: Result %X Host "
-								"Adapter Status %02X Target Status %02X\n", adapter, ccb->serial, ccb->tgt_id, command->result, ccb->adapter_status, ccb->tgt_status);
-						blogic_notice("CDB   ", adapter);
+						blogic_notice(adapter,
+							      "CCB #%ld Target %d: Result %X Host Adapter Status %02X Target Status %02X\n",
+							      ccb->serial,
+							      ccb->tgt_id,
+							      command->result,
+							      ccb->adapter_status,
+							      ccb->tgt_status);
+						blogic_notice(adapter,
+							      "CDB   ");
 						for (i = 0; i < ccb->cdblen; i++)
-							blogic_notice(" %02X", adapter, ccb->cdb[i]);
-						blogic_notice("\n", adapter);
-						blogic_notice("Sense ", adapter);
+							blogic_cont(adapter,
+								    " %02X",
+								    ccb->cdb[i]);
+						blogic_cont(adapter, "\n");
+						blogic_notice(adapter, "Sense ");
 						for (i = 0; i < ccb->sense_datalen; i++)
-							blogic_notice(" %02X", adapter, command->sense_buffer[i]);
-						blogic_notice("\n", adapter);
+							blogic_cont(adapter,
+								    " %02X",
+								    command->sense_buffer[i]);
+						blogic_cont(adapter, "\n");
 					}
 				}
 				break;
@@ -2925,7 +3030,8 @@ static irqreturn_t blogic_inthandler(int irq_ch, void *devid)
 				adapter->adapter_extreset = true;
 				break;
 			case FPOINT_INTERN_ERR:
-				blogic_warn("Internal FlashPoint Error detected - Resetting Host Adapter\n", adapter);
+				blogic_warn(adapter,
+					    "Internal FlashPoint Error detected - Resetting Host Adapter\n");
 				adapter->adapter_intern_err = true;
 				break;
 			}
@@ -2939,12 +3045,16 @@ static irqreturn_t blogic_inthandler(int irq_ch, void *devid)
 	   Reset the Host Adapter if requested.
 	 */
 	if (adapter->adapter_extreset) {
-		blogic_warn("Resetting %s due to External SCSI Bus Reset\n", adapter, adapter->full_model);
+		blogic_warn(adapter,
+			    "Resetting %s due to External SCSI Bus Reset\n",
+			    adapter->full_model);
 		blogic_inc_count(&adapter->ext_resets);
 		blogic_resetadapter(adapter, false);
 		adapter->adapter_extreset = false;
 	} else if (adapter->adapter_intern_err) {
-		blogic_warn("Resetting %s due to Host Adapter Internal Error\n", adapter, adapter->full_model);
+		blogic_warn(adapter,
+			    "Resetting %s due to Host Adapter Internal Error\n",
+			    adapter->full_model);
 		blogic_inc_count(&adapter->adapter_intern_errors);
 		blogic_resetadapter(adapter, true);
 		adapter->adapter_intern_err = false;
@@ -3142,8 +3252,9 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
 			&& tgt_flags->tagq_ok &&
 			(adapter->tagq_ok & (1 << tgt_id))) {
 		tgt_flags->tagq_active = true;
-		blogic_notice("Tagged Queuing now active for Target %d\n",
-					adapter, tgt_id);
+		blogic_notice(adapter,
+			      "Tagged Queuing now active for Target %d\n",
+			      tgt_id);
 	}
 	if (tgt_flags->tagq_active) {
 		enum blogic_queuetag queuetag = BLOGIC_SIMPLETAG;
@@ -3184,8 +3295,8 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
 				command->sense_buffer, ccb->sense_datalen,
 				DMA_FROM_DEVICE);
 	if (dma_mapping_error(&adapter->pci_device->dev, sense_buf)) {
-		blogic_err("DMA mapping for sense data buffer failed\n",
-				adapter);
+		blogic_err(adapter,
+			   "DMA mapping for sense data buffer failed\n");
 		blogic_dealloc_ccb(ccb, 0);
 		return SCSI_MLQUEUE_HOST_BUSY;
 	}
@@ -3204,12 +3315,12 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
 		 */
 		if (!blogic_write_outbox(adapter, BLOGIC_MBOX_START, ccb)) {
 			spin_unlock_irq(adapter->scsi_host->host_lock);
-			blogic_warn("Unable to write Outgoing Mailbox - Pausing for 1 second\n", adapter);
+			blogic_warn(adapter, "Unable to write Outgoing Mailbox - Pausing for 1 second\n");
 			blogic_delay(1);
 			spin_lock_irq(adapter->scsi_host->host_lock);
 			if (!blogic_write_outbox(adapter, BLOGIC_MBOX_START,
 						ccb)) {
-				blogic_warn("Still unable to write Outgoing Mailbox - Host Adapter Dead?\n", adapter);
+				blogic_warn(adapter, "Still unable to write Outgoing Mailbox - Host Adapter Dead?\n");
 				blogic_dealloc_ccb(ccb, 1);
 				command->result = DID_ERROR << 16;
 				command->scsi_done(command);
@@ -3259,13 +3370,16 @@ static int blogic_abort(struct scsi_cmnd *command)
 		if (ccb->command == command)
 			break;
 	if (ccb == NULL) {
-		blogic_warn("Unable to Abort Command to Target %d - No CCB Found\n", adapter, tgt_id);
+		blogic_warn(adapter, "Unable to Abort Command to Target %d - No CCB Found\n",
+			    tgt_id);
 		return SUCCESS;
 	} else if (ccb->status == BLOGIC_CCB_COMPLETE) {
-		blogic_warn("Unable to Abort Command to Target %d - CCB Completed\n", adapter, tgt_id);
+		blogic_warn(adapter, "Unable to Abort Command to Target %d - CCB Completed\n",
+			    tgt_id);
 		return SUCCESS;
 	} else if (ccb->status == BLOGIC_CCB_RESET) {
-		blogic_warn("Unable to Abort Command to Target %d - CCB Reset\n", adapter, tgt_id);
+		blogic_warn(adapter, "Unable to Abort Command to Target %d - CCB Reset\n",
+			    tgt_id);
 		return SUCCESS;
 	}
 	if (blogic_multimaster_type(adapter)) {
@@ -3283,16 +3397,18 @@ static int blogic_abort(struct scsi_cmnd *command)
 		 */
 		if (adapter->tgt_flags[tgt_id].tagq_active &&
 				adapter->fw_ver[0] < '5') {
-			blogic_warn("Unable to Abort CCB #%ld to Target %d - Abort Tag Not Supported\n", adapter, ccb->serial, tgt_id);
+			blogic_warn(adapter, "Unable to Abort CCB #%ld to Target %d - Abort Tag Not Supported\n",
+				    ccb->serial, tgt_id);
 			return FAILURE;
 		} else if (blogic_write_outbox(adapter, BLOGIC_MBOX_ABORT,
 					ccb)) {
-			blogic_warn("Aborting CCB #%ld to Target %d\n",
-					adapter, ccb->serial, tgt_id);
+			blogic_warn(adapter, "Aborting CCB #%ld to Target %d\n",
+				    ccb->serial, tgt_id);
 			blogic_inc_count(&adapter->tgt_stats[tgt_id].aborts_tried);
 			return SUCCESS;
 		} else {
-			blogic_warn("Unable to Abort CCB #%ld to Target %d - No Outgoing Mailboxes\n", adapter, ccb->serial, tgt_id);
+			blogic_warn(adapter, "Unable to Abort CCB #%ld to Target %d - No Outgoing Mailboxes\n",
+				    ccb->serial, tgt_id);
 			return FAILURE;
 		}
 	} else {
@@ -3300,8 +3416,8 @@ static int blogic_abort(struct scsi_cmnd *command)
 		   Call the FlashPoint SCCB Manager to abort execution of
 		   the CCB.
 		 */
-		blogic_warn("Aborting CCB #%ld to Target %d\n", adapter,
-				ccb->serial, tgt_id);
+		blogic_warn(adapter, "Aborting CCB #%ld to Target %d\n",
+			    ccb->serial, tgt_id);
 		blogic_inc_count(&adapter->tgt_stats[tgt_id].aborts_tried);
 		FlashPoint_AbortCCB(adapter->cardhandle, ccb);
 		/*
@@ -3333,8 +3449,8 @@ static int blogic_resetadapter(struct blogic_adapter *adapter, bool hard_reset)
 
 	if (!(blogic_hwreset(adapter, hard_reset) &&
 				blogic_initadapter(adapter))) {
-		blogic_err("Resetting %s Failed\n", adapter,
-						adapter->full_model);
+		blogic_err(adapter, "Resetting %s Failed\n",
+			   adapter->full_model);
 		return FAILURE;
 	}
 
@@ -3443,10 +3559,15 @@ static int blogic_diskparam(struct scsi_device *sdev, struct block_device *dev,
 		diskparam->cylinders = (unsigned long) capacity / (diskparam->heads * diskparam->sectors);
 		if (part_no < 4 && part_end_sector == diskparam->sectors) {
 			if (diskparam->cylinders != saved_cyl)
-				blogic_warn("Adopting Geometry %d/%d from Partition Table\n", adapter, diskparam->heads, diskparam->sectors);
+				blogic_warn(adapter,
+					    "Adopting Geometry %d/%d from Partition Table\n",
+					    diskparam->heads,
+					    diskparam->sectors);
 		} else if (part_end_head > 0 || part_end_sector > 0) {
-			blogic_warn("Warning: Partition Table appears to have Geometry %d/%d which is\n", adapter, part_end_head + 1, part_end_sector);
-			blogic_warn("not compatible with current BusLogic Host Adapter Geometry %d/%d\n", adapter, diskparam->heads, diskparam->sectors);
+			blogic_warn(adapter,
+				    "Warning: Partition Table appears to have Geometry %d/%d which is not compatible with current BusLogic Host Adapter Geometry %d/%d\n",
+				    part_end_head + 1, part_end_sector,
+				    diskparam->heads, diskparam->sectors);
 		}
 	}
 	kfree(buf);
@@ -3571,6 +3692,7 @@ Target	Requested Completed  Requested Completed  Requested Completed\n\
 	}
 	seq_printf(m, "\nExternal Host Adapter Resets: %d\n", adapter->ext_resets);
 	seq_printf(m, "Host Adapter Internal Errors: %d\n", adapter->adapter_intern_errors);
+
 	return 0;
 }
 
@@ -3579,41 +3701,51 @@ Target	Requested Completed  Requested Completed  Requested Completed\n\
   blogic_msg prints Driver Messages.
 */
 
-static void blogic_msg(enum blogic_msglevel msglevel, char *fmt,
-			struct blogic_adapter *adapter, ...)
+static void blogic_msg(enum blogic_msglevel msglevel,
+		       struct blogic_adapter *adapter, const char *fmt, ...)
 {
 	static char buf[BLOGIC_LINEBUF_SIZE];
-	static bool begin = true;
 	va_list args;
 	int len = 0;
+	const char *kern_level = blogic_msglevelmap[msglevel];
 
-	va_start(args, adapter);
-	len = vsprintf(buf, fmt, args);
+	va_start(args, fmt);
+	len = vscnprintf(buf, sizeof(buf), fmt, args);
 	va_end(args);
-	if (msglevel == BLOGIC_ANNOUNCE_LEVEL) {
+
+	switch (msglevel) {
+	case BLOGIC_ANNOUNCE_LEVEL: {
 		static int msglines = 0;
+
 		strcpy(&adapter->msgbuf[adapter->msgbuflen], buf);
 		adapter->msgbuflen += len;
 		if (++msglines <= 2)
-			printk("%sscsi: %s", blogic_msglevelmap[msglevel], buf);
-	} else if (msglevel == BLOGIC_INFO_LEVEL) {
+			printk("%sscsi: %s", kern_level, buf);
+		break;
+	}
+
+	case BLOGIC_INFO_LEVEL:
 		strcpy(&adapter->msgbuf[adapter->msgbuflen], buf);
 		adapter->msgbuflen += len;
-		if (begin) {
-			if (buf[0] != '\n' || len > 1)
-				printk("%sscsi%d: %s", blogic_msglevelmap[msglevel], adapter->host_no, buf);
-		} else
-			printk("%s", buf);
-	} else {
-		if (begin) {
-			if (adapter != NULL && adapter->adapter_initd)
-				printk("%sscsi%d: %s", blogic_msglevelmap[msglevel], adapter->host_no, buf);
-			else
-				printk("%s%s", blogic_msglevelmap[msglevel], buf);
-		} else
-			printk("%s", buf);
+		if (buf[0] != '\n' || len > 1)
+			printk("%sscsi%d: %s",
+			       kern_level, adapter->host_no, buf);
+		break;
+
+	case BLOGIC_CONT_LEVEL:
+		strcpy(&adapter->msgbuf[adapter->msgbuflen], buf);
+		adapter->msgbuflen += len;
+		printk("%s%s", kern_level, buf);
+		break;
+
+	default:
+		if (adapter && adapter->adapter_initd)
+			printk("%sscsi%d: %s",
+			       kern_level, adapter->host_no, buf);
+		else
+			printk("%s%s", kern_level, buf);
+		break;
 	}
-	begin = (buf[len - 1] == '\n');
 }
 
 
@@ -3691,7 +3823,9 @@ static int __init blogic_parseopts(char *options)
 					blogic_probe_options.probe134 = true;
 					break;
 				default:
-					blogic_err("BusLogic: Invalid Driver Options (invalid I/O Address 0x%lX)\n", NULL, io_addr);
+					blogic_err(NULL,
+						   "BusLogic: Invalid Driver Options (invalid I/O Address 0x%lX)\n",
+						   io_addr);
 					return 0;
 				}
 			} else if (blogic_parse(&options, "NoProbeISA"))
@@ -3712,7 +3846,9 @@ static int __init blogic_parseopts(char *options)
 				for (tgt_id = 0; tgt_id < BLOGIC_MAXDEV; tgt_id++) {
 					unsigned short qdepth = simple_strtoul(options, &options, 0);
 					if (qdepth > BLOGIC_MAX_TAG_DEPTH) {
-						blogic_err("BusLogic: Invalid Driver Options (invalid Queue Depth %d)\n", NULL, qdepth);
+						blogic_err(NULL,
+							   "BusLogic: Invalid Driver Options (invalid Queue Depth %d)\n",
+							   qdepth);
 						return 0;
 					}
 					drvr_opts->qdepth[tgt_id] = qdepth;
@@ -3721,12 +3857,16 @@ static int __init blogic_parseopts(char *options)
 					else if (*options == ']')
 						break;
 					else {
-						blogic_err("BusLogic: Invalid Driver Options (',' or ']' expected at '%s')\n", NULL, options);
+						blogic_err(NULL,
+							   "BusLogic: Invalid Driver Options (',' or ']' expected at '%s')\n",
+							   options);
 						return 0;
 					}
 				}
 				if (*options != ']') {
-					blogic_err("BusLogic: Invalid Driver Options (']' expected at '%s')\n", NULL, options);
+					blogic_err(NULL,
+						   "BusLogic: Invalid Driver Options (']' expected at '%s')\n",
+						   options);
 					return 0;
 				} else
 					options++;
@@ -3734,7 +3874,9 @@ static int __init blogic_parseopts(char *options)
 				unsigned short qdepth = simple_strtoul(options, &options, 0);
 				if (qdepth == 0 ||
 						qdepth > BLOGIC_MAX_TAG_DEPTH) {
-					blogic_err("BusLogic: Invalid Driver Options (invalid Queue Depth %d)\n", NULL, qdepth);
+					blogic_err(NULL,
+						   "BusLogic: Invalid Driver Options (invalid Queue Depth %d)\n",
+						   qdepth);
 					return 0;
 				}
 				drvr_opts->common_qdepth = qdepth;
@@ -3780,7 +3922,9 @@ static int __init blogic_parseopts(char *options)
 				unsigned short bus_settle_time =
 					simple_strtoul(options, &options, 0);
 				if (bus_settle_time > 5 * 60) {
-					blogic_err("BusLogic: Invalid Driver Options (invalid Bus Settle Time %d)\n", NULL, bus_settle_time);
+					blogic_err(NULL,
+						   "BusLogic: Invalid Driver Options (invalid Bus Settle Time %d)\n",
+						   bus_settle_time);
 					return 0;
 				}
 				drvr_opts->bus_settle_time = bus_settle_time;
@@ -3805,14 +3949,17 @@ static int __init blogic_parseopts(char *options)
 			if (*options == ',')
 				options++;
 			else if (*options != ';' && *options != '\0') {
-				blogic_err("BusLogic: Unexpected Driver Option '%s' ignored\n", NULL, options);
+				blogic_err(NULL,
+					   "BusLogic: Unexpected Driver Option '%s' ignored\n",
+					   options);
 				*options = '\0';
 			}
 		}
 		if (!(blogic_drvr_options_count == 0 ||
 			blogic_probeinfo_count == 0 ||
 			blogic_drvr_options_count == blogic_probeinfo_count)) {
-			blogic_err("BusLogic: Invalid Driver Options (all or no I/O Addresses must be specified)\n", NULL);
+			blogic_err(NULL,
+				   "BusLogic: Invalid Driver Options (all or no I/O Addresses must be specified)\n");
 			return 0;
 		}
 		/*
@@ -3866,7 +4013,8 @@ static int __init blogic_setup(char *str)
 	(void) get_options(str, ARRAY_SIZE(ints), ints);
 
 	if (ints[0] != 0) {
-		blogic_err("BusLogic: Obsolete Command Line Entry Format Ignored\n", NULL);
+		blogic_err(NULL,
+			   "BusLogic: Obsolete Command Line Entry Format Ignored\n");
 		return 0;
 	}
 	if (str == NULL || *str == '\0')
diff --git a/drivers/scsi/BusLogic.h b/drivers/scsi/BusLogic.h
index 6182cc8a0344..42333fba6005 100644
--- a/drivers/scsi/BusLogic.h
+++ b/drivers/scsi/BusLogic.h
@@ -108,30 +108,34 @@ enum blogic_msglevel {
 	BLOGIC_INFO_LEVEL = 1,
 	BLOGIC_NOTICE_LEVEL = 2,
 	BLOGIC_WARN_LEVEL = 3,
-	BLOGIC_ERR_LEVEL = 4
+	BLOGIC_ERR_LEVEL = 4,
+	BLOGIC_CONT_LEVEL = 5,
 };
 
-static char *blogic_msglevelmap[] = { KERN_NOTICE, KERN_NOTICE, KERN_NOTICE, KERN_WARNING, KERN_ERR };
+static char *blogic_msglevelmap[] = { KERN_NOTICE, KERN_NOTICE, KERN_NOTICE, KERN_WARNING, KERN_ERR, KERN_CONT };
 
 
 /*
   Define Driver Message macros.
 */
 
-#define blogic_announce(format, args...) \
-	blogic_msg(BLOGIC_ANNOUNCE_LEVEL, format, ##args)
+#define blogic_announce(adapter, fmt, ...)				\
+	blogic_msg(BLOGIC_ANNOUNCE_LEVEL, adapter, fmt, ##__VA_ARGS__)
 
-#define blogic_info(format, args...) \
-	blogic_msg(BLOGIC_INFO_LEVEL, format, ##args)
+#define blogic_cont(adapter, fmt, ...)					\
+	blogic_msg(BLOGIC_CONT_LEVEL, adapter, fmt, ##__VA_ARGS__)
 
-#define blogic_notice(format, args...) \
-	blogic_msg(BLOGIC_NOTICE_LEVEL, format, ##args)
+#define blogic_info(adapter, fmt, ...)					\
+	blogic_msg(BLOGIC_INFO_LEVEL, adapter, fmt, ##__VA_ARGS__)
 
-#define blogic_warn(format, args...) \
-	blogic_msg(BLOGIC_WARN_LEVEL, format, ##args)
+#define blogic_notice(adapter, fmt, ...)				\
+	blogic_msg(BLOGIC_NOTICE_LEVEL, adapter, fmt, ##__VA_ARGS__)
 
-#define blogic_err(format, args...) \
-	blogic_msg(BLOGIC_ERR_LEVEL, format, ##args)
+#define blogic_warn(adapter, fmt, ...)					\
+	blogic_msg(BLOGIC_WARN_LEVEL, adapter, fmt, ##__VA_ARGS__)
+
+#define blogic_err(adapter, fmt, ...)					\
+	blogic_msg(BLOGIC_ERR_LEVEL, adapter, fmt, ##__VA_ARGS__)
 
 
 /*
@@ -1289,7 +1293,9 @@ static int blogic_slaveconfig(struct scsi_device *);
 static void blogic_qcompleted_ccb(struct blogic_ccb *);
 static irqreturn_t blogic_inthandler(int, void *);
 static int blogic_resetadapter(struct blogic_adapter *, bool hard_reset);
-static void blogic_msg(enum blogic_msglevel, char *, struct blogic_adapter *, ...);
+__printf(3, 4)
+static void blogic_msg(enum blogic_msglevel, struct blogic_adapter *adapter,
+		       const char *fmt, ...);
 static int __init blogic_setup(char *);
 
 #endif				/* _BUSLOGIC_H */
Maciej W. Rozycki April 16, 2021, 10:48 a.m. UTC | #3
On Thu, 15 Apr 2021, Joe Perches wrote:

> In patch 2, vscnprintf should probably be used to make sure it's

> 0 terminated.


 Why?  C99 has this[1]:

"The vsnprintf function is equivalent to snprintf, with the variable 
argument list replaced by arg, which shall have been initialized by the 
va_start macro (and possibly subsequent va_arg calls)."

and then[2]:

"The snprintf function is equivalent to fprintf, except that the output 
is written into an array (specified by argument s) rather than to a 
stream.  If n is zero, nothing is written, and s may be a null pointer. 
Otherwise, output characters beyond the n-1st are discarded rather than 
being written to the array, and a null character is written at the end 
of the characters actually written into the array."

therefore output from `vsnprintf' is always null-terminated.

> And while it's a lot more code, I'd prefer a solution that looks more

> like the other commonly used kernel logging extension mechanisms

> where adapter is placed before the format, ... in the argument list.


 I agree having `adapter' as the second argument seems weird, so that is 
fine with me as a follow-up cleanup.  However as a user-visible change I 
think the fix I propose here ought to be applied first (and backported 
as suitable).  Then any internal clean-ups can follow, applied to trunk 
only.

> And there's a simple addition of a blogic_cont macro and extension

> to blogic_msg to simplify the logic and obviousness of the logging

> extension lines too.


 I did this first actually, before I realised a simpler change suitable 
for backporting could be done.  I'm not sure if that complex message 
routing via `blogic_msg' is worth having even, rather than calling 
`printk' or suitable variants directly.

References:

[1] "Programming languages -- C", INTERNATIONAL STANDARD, ISO/IEC 9899, 
    Second edition, 1999-12-01, Section 7.19.6.12 "The vsnprintf 
    function", p.293

[2] same, 7.19.6.5 "The snprintf function", p.289

  Maciej
Joe Perches April 16, 2021, 2:02 p.m. UTC | #4
On Fri, 2021-04-16 at 12:48 +0200, Maciej W. Rozycki wrote:
> I'm not sure if that complex message 
> routing via `blogic_msg' is worth having even, rather than calling 
> `printk' or suitable variants directly.

It's to allow the message content to be added to the internal
	&adapter->msgbuf[adapter->msgbuflen]
with strcpy for later use with blogic_show_info()/seq_write.
Maciej W. Rozycki April 16, 2021, 2:28 p.m. UTC | #5
On Fri, 16 Apr 2021, Joe Perches wrote:

> > I'm not sure if that complex message 
> > routing via `blogic_msg' is worth having even, rather than calling 
> > `printk' or suitable variants directly.
> 
> It's to allow the message content to be added to the internal
> 	&adapter->msgbuf[adapter->msgbuflen]
> with strcpy for later use with blogic_show_info()/seq_write.

 I know, but it's not clear to me if it's worth it (a potential buffer 
overrun there too, BTW).

  Maciej
Joe Perches April 16, 2021, 3:12 p.m. UTC | #6
On Fri, 2021-04-16 at 16:28 +0200, Maciej W. Rozycki wrote:
> On Fri, 16 Apr 2021, Joe Perches wrote:

> 

> > > I'm not sure if that complex message 

> > > routing via `blogic_msg' is worth having even, rather than calling 

> > > `printk' or suitable variants directly.

> > 

> > It's to allow the message content to be added to the internal

> > 	&adapter->msgbuf[adapter->msgbuflen]

> > with strcpy for later use with blogic_show_info()/seq_write.

> 

>  I know, but it's not clear to me if it's worth it (a potential buffer 

> overrun there too, BTW).


It's seq_ output so it's nominally an ABI.
But then again, I don't use this at all so I don't care much either.

It's also odd/bad form that one output KERN_<level> does not match
its blogic_<level> (blogic_info is emitted at KERN_NOTICE)
Maciej W. Rozycki April 16, 2021, 3:18 p.m. UTC | #7
On Thu, 15 Apr 2021, Nix wrote:

> > Set the allocation length to 255 for the ATA Information VPD page 

> > requested in the WRITE SAME handler, so as not to limit information 

> > examined by `scsi_get_vpd_page' in the supported vital product data 

> > pages unnecessarily.

> >

> > Originally it was thought that Areca hardware may have issues with a 

> > valid allocation length supplied for a VPD inquiry, however older SCSI 

> > standard revisions[1] consider 255 the maximum length allowed and what 

> 

> Aaaah. That explains a lot! (Not that I can remember what SCSI standard

> rev that Areca firmware claimed to implement. I know I never updated the

> firmware, so it's going to be something no newer than mid-2009 and

> probably quite a bit older.)


 From the original discussion I gather Areca sometimes acts as a 
pass-through device to actual storage hardware, so it may well have been 
decided for the firmware to take a conservative approach and interpret 
the low order byte only.  A genuine bug cannot be ruled out either of 
course, which I why I will appreciate your testing.

> >  I can see you're still around.  Would you therefore please be so kind 

> > as to verify this change with your Areca hardware if you still have it?

> 

> It's been up in the loft for years, but I'll get it out this weekend and

> give it a spin :) this'll let me make sure the disks still spin as well,

> which matters for an in-case-of-lightning-strike disaster-recovery

> backup box.

> 

> (I just hope this kernel boots on it at all. It's about three years

> since I retired it... let's see!)


 FWIW if all else fails you can try this patch with the original kernel 
you used with the box.  This piece of code hasn't changed, so until I 
came up with the complete five-part solution proposed here I merely had 
the original commit reverted as it is so as to allow forward progress.

 In any case, as per the cover letter, I have upgraded from 2.6.18, much 
older, and this was the sole show-stopper for the machine, running SMP 
even, so chances are 5.11+ will work with your system as well.  The 
other plain 486/EISA/ATA box, similarly upgraded (now that I got its 
faulty odd industrial PSU finally replaced) works just fine with vanilla 
5.11.

 OTOH versions ~3.15 through to ~4.5 I have tried while bisecting this 
issue mostly failed to even start booting due to what looks like a 
heisenbug to me (e.g. switching from XZ to gzip for compression would 
make some, but not all versions/configurations boot occasionally), so 
YMMV.

 Overall we're not that bad with keeping stuff working, it's more new 
use that causes troubles sometimes.

> >  It looks to me like you were thinking in the right direction with: 

> > <https://lore.kernel.org/linux-scsi/87vc3nuipg.fsf@spindle.srvr.nix/>. 

> 

> It's the sort of mistake I could see myself making: an easy mistake to

> make when so many things in C require buffer size - 1 or you get a

> disastrous security hole...


 And here it's masking, except that with (256 - 1) rather than (512 - 1) 
as you suggested.

 Thank you for your input!

  Maciej
Khalid Aziz April 16, 2021, 7:34 p.m. UTC | #8
On 4/14/21 4:39 PM, Maciej W. Rozycki wrote:
> Update BusLogic driver's messaging system to use `pr_cont' for 

> continuation lines, bringing messy output:

> 

> pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17

> scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *****

> scsi: Copyright 1995-1998 by Leonard N. Zubkoff <lnz@dandelion.com>

> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter

> scsi0:   Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level

> scsi0:   PCI Bus: 0, Device: 19, Address:

> 0xE0012000,

> Host Adapter SCSI ID: 7

> scsi0:   Parity Checking: Enabled, Extended Translation: Enabled

> scsi0:   Synchronous Negotiation: Ultra, Wide Negotiation: Enabled

> scsi0:   Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled

> scsi0:   Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211

> scsi0:   Driver Queue Depth: 211, Host Adapter Queue Depth: 192

> scsi0:   Tagged Queue Depth:

> Automatic

> , Untagged Queue Depth: 3

> scsi0:   SCSI Bus Termination: Both Enabled

> , SCAM: Disabled

> 

> scsi0: *** BusLogic BT-958 Initialized Successfully ***

> scsi host0: BusLogic BT-958

> 

> back to order:

> 

> pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17

> scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *****

> scsi: Copyright 1995-1998 by Leonard N. Zubkoff <lnz@dandelion.com>

> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter

> scsi0:   Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level

> scsi0:   PCI Bus: 0, Device: 19, Address: 0xE0012000, Host Adapter SCSI ID: 7

> scsi0:   Parity Checking: Enabled, Extended Translation: Enabled

> scsi0:   Synchronous Negotiation: Ultra, Wide Negotiation: Enabled

> scsi0:   Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled

> scsi0:   Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211

> scsi0:   Driver Queue Depth: 211, Host Adapter Queue Depth: 192

> scsi0:   Tagged Queue Depth: Automatic, Untagged Queue Depth: 3

> scsi0:   SCSI Bus Termination: Both Enabled, SCAM: Disabled

> scsi0: *** BusLogic BT-958 Initialized Successfully ***

> scsi host0: BusLogic BT-958

> 

> Also diagnostic output such as with the `BusLogic=TraceConfiguration' 

> parameter is affected and becomes vertical and therefore hard to read.  

> This has now been corrected, e.g.:

> 

> pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17

> blogic_cmd(86) Status = 30:  4 ==>  4: FF 05 93 00

> blogic_cmd(95) Status = 28: (Modify I/O Address)

> blogic_cmd(91) Status = 30:  1 ==>  1: 01

> blogic_cmd(04) Status = 30:  4 ==>  4: 41 41 35 30

> blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 1D

> scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *****

> scsi: Copyright 1995-1998 by Leonard N. Zubkoff <lnz@dandelion.com>

> blogic_cmd(04) Status = 30:  4 ==>  4: 41 41 35 30

> blogic_cmd(0B) Status = 30:  3 ==>  3: 00 08 07

> blogic_cmd(0D) Status = 30: 34 ==> 34: 03 01 07 04 00 00 00 00 00 00 00 00 00 00 00 00 FF 42 44 46 FF 00 00 00 00 00 00 00 00 00 FF 00 FF 00

> blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 1D

> blogic_cmd(84) Status = 30:  1 ==>  1: 37

> blogic_cmd(8B) Status = 30:  5 ==>  5: 39 35 38 20 20

> blogic_cmd(85) Status = 30:  1 ==>  1: 42

> blogic_cmd(86) Status = 30:  4 ==>  4: FF 05 93 00

> blogic_cmd(91) Status = 30: 64 ==> 64: 41 46 3E 20 39 35 38 20 20 00 C4 00 04 01 07 2F 07 04 35 FF FF FF FF FF FF FF FF FF FF 01 00 FE FF 08 FF FF 00 00 00 00 00 00 00 01 00 01 00 00 FF FF 00 00 00 00 00 00 00 00 00 00 00 00 00 FC

> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter

> 

> etc.

> 

> Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>

> Fixes: 4bcc595ccd80 ("printk: reinstate KERN_CONT for printing continuation lines")

> Cc: stable@vger.kernel.org # v4.9+

> ---

>  drivers/scsi/BusLogic.c |    4 ++--

>  1 file changed, 2 insertions(+), 2 deletions(-)

> 

> linux-buslogic-pr-cont.diff

> Index: linux-macro-ide/drivers/scsi/BusLogic.c

> ===================================================================

> --- linux-macro-ide.orig/drivers/scsi/BusLogic.c

> +++ linux-macro-ide/drivers/scsi/BusLogic.c

> @@ -3603,7 +3603,7 @@ static void blogic_msg(enum blogic_msgle

>  			if (buf[0] != '\n' || len > 1)

>  				printk("%sscsi%d: %s", blogic_msglevelmap[msglevel], adapter->host_no, buf);

>  		} else

> -			printk("%s", buf);

> +			pr_cont("%s", buf);

>  	} else {

>  		if (begin) {

>  			if (adapter != NULL && adapter->adapter_initd)

> @@ -3611,7 +3611,7 @@ static void blogic_msg(enum blogic_msgle

>  			else

>  				printk("%s%s", blogic_msglevelmap[msglevel], buf);

>  		} else

> -			printk("%s", buf);

> +			pr_cont("%s", buf);

>  	}

>  	begin = (buf[len - 1] == '\n');

>  }

> 


Looks good.

Acked-by: Khalid Aziz <khalid@gonehiking.org>
Khalid Aziz April 16, 2021, 7:36 p.m. UTC | #9
On 4/14/21 4:38 PM, Maciej W. Rozycki wrote:
> Hi,

> 

>  First of all, does anyone have a copy of: "MultiMaster UltraSCSI Host 

> Adapters for PCI Systems: Technical Reference Manual" (pub. 3002493-E)?  

> It used to live in the "Mylex Manuals and Documentation Archives" section 

> of the Mylex web site <http://www.mylex.com/pub/manuals/index.htm>, 

> specifically at: <http://www.mylex.com/pub/manuals/mmultra.pdf>.

> 

>  Another useful document might be: "Wide SCSI Host Adapters for PCI and 

> EISA Systems: Technical Reference Manual" (pub. 3000763-A), which used to 

> live at: <http://www.mylex.com/pub/manuals/widescsi.pdf>, linked from the 

> same place.

> 

>  Sadly I didn't get to these resources while they were still there, and 

> neither did archive.org, and now they not appear available from anywhere 

> online.  I'm sure Leonard had this all, but, alas, he is long gone too.


These documents were all gone by the time I started working on this
driver in 2013.

--
Khalid
Khalid Aziz April 16, 2021, 8:41 p.m. UTC | #10
On 4/15/21 8:08 PM, Joe Perches wrote:
> And while it's a lot more code, I'd prefer a solution that looks more
> like the other commonly used kernel logging extension mechanisms
> where adapter is placed before the format, ... in the argument list.

Hi Joe,

I don't mind making these changes. It is quite a bit of code but
consistency with other kernel code is useful. Would you like to finalize
this patch, or would you prefer that I take this patch as starting point
and finalize it?

Thanks,
Khalid

> 
> Today it's:
> 
> void blogic_msg(enum, fmt, adapter, ...);
> 
> without the __printf marking so there is one format/arg mismatch.
> 
> fyi: in the suggested patch below it's
> -			blogic_info("BIOS Address: 0x%lX, ", adapter,
> -					adapter->bios_addr);
> +			blogic_cont(adapter, "BIOS Address: 0x%X, ",
> +				    adapter->bios_addr);
> 
> I'd prefer
> __printf(3, 4)
> void blogic_msg(enum, adapter, fmt, ...)
> 
> (or maybe void blogic_msg(adapter, enum, fmt, ...))
> 
> And there's a simple addition of a blogic_cont macro and extension
> to blogic_msg to simplify the logic and obviousness of the logging
> extension lines too.
> 
> I suggest this done with coccinelle and a little typing:
> ---
>  drivers/scsi/BusLogic.c | 496 +++++++++++++++++++++++++++++++-----------------
>  drivers/scsi/BusLogic.h |  32 ++--
>  2 files changed, 341 insertions(+), 187 deletions(-)
> 
> diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
> index ccb061ab0a0a..7a52371b5ab6 100644
> --- a/drivers/scsi/BusLogic.c
> +++ b/drivers/scsi/BusLogic.c
> @@ -134,8 +134,10 @@ static char *blogic_cmd_failure_reason;
>  
>  static void blogic_announce_drvr(struct blogic_adapter *adapter)
>  {
> -	blogic_announce("***** BusLogic SCSI Driver Version " blogic_drvr_version " of " blogic_drvr_date " *****\n", adapter);
> -	blogic_announce("Copyright 1995-1998 by Leonard N. Zubkoff <lnz@dandelion.com>\n", adapter);
> +	blogic_announce(adapter,
> +			"***** BusLogic SCSI Driver Version " blogic_drvr_version " of " blogic_drvr_date " *****\n");
> +	blogic_announce(adapter,
> +			"Copyright 1995-1998 by Leonard N. Zubkoff <lnz@dandelion.com>\n");
>  }
>  
>  
> @@ -198,8 +200,7 @@ static bool __init blogic_create_initccbs(struct blogic_adapter *adapter)
>  		blk_pointer = dma_alloc_coherent(&adapter->pci_device->dev,
>  				blk_size, &blkp, GFP_KERNEL);
>  		if (blk_pointer == NULL) {
> -			blogic_err("UNABLE TO ALLOCATE CCB GROUP - DETACHING\n",
> -					adapter);
> +			blogic_err(adapter, "UNABLE TO ALLOCATE CCB GROUP - DETACHING\n");
>  			return false;
>  		}
>  		blogic_init_ccbs(adapter, blk_pointer, blk_size, blkp);
> @@ -259,10 +260,13 @@ static void blogic_create_addlccbs(struct blogic_adapter *adapter,
>  	}
>  	if (adapter->alloc_ccbs > prev_alloc) {
>  		if (print_success)
> -			blogic_notice("Allocated %d additional CCBs (total now %d)\n", adapter, adapter->alloc_ccbs - prev_alloc, adapter->alloc_ccbs);
> +			blogic_notice(adapter,
> +				      "Allocated %d additional CCBs (total now %d)\n",
> +				      adapter->alloc_ccbs - prev_alloc,
> +				      adapter->alloc_ccbs);
>  		return;
>  	}
> -	blogic_notice("Failed to allocate additional CCBs\n", adapter);
> +	blogic_notice(adapter, "Failed to allocate additional CCBs\n");
>  	if (adapter->drvr_qdepth > adapter->alloc_ccbs - adapter->tgt_count) {
>  		adapter->drvr_qdepth = adapter->alloc_ccbs - adapter->tgt_count;
>  		adapter->scsi_host->can_queue = adapter->drvr_qdepth;
> @@ -441,7 +445,9 @@ static int blogic_cmd(struct blogic_adapter *adapter, enum blogic_opcode opcode,
>  			goto done;
>  		}
>  		if (blogic_global_options.trace_config)
> -			blogic_notice("blogic_cmd(%02X) Status = %02X: (Modify I/O Address)\n", adapter, opcode, statusreg.all);
> +			blogic_notice(adapter,
> +				      "blogic_cmd(%02X) Status = %02X: (Modify I/O Address)\n",
> +				      opcode, statusreg.all);
>  		result = 0;
>  		goto done;
>  	}
> @@ -499,15 +505,16 @@ static int blogic_cmd(struct blogic_adapter *adapter, enum blogic_opcode opcode,
>  	 */
>  	if (blogic_global_options.trace_config) {
>  		int i;
> -		blogic_notice("blogic_cmd(%02X) Status = %02X: %2d ==> %2d:",
> -				adapter, opcode, statusreg.all, replylen,
> +		blogic_notice(adapter,
> +			      "blogic_cmd(%02X) Status = %02X: %2d ==> %2d:",
> +			      opcode, statusreg.all, replylen,
>  				reply_b);
>  		if (replylen > reply_b)
>  			replylen = reply_b;
>  		for (i = 0; i < replylen; i++)
> -			blogic_notice(" %02X", adapter,
> -					((unsigned char *) reply)[i]);
> -		blogic_notice("\n", adapter);
> +			blogic_cont(adapter, " %02X",
> +				    ((unsigned char *)reply)[i]);
> +		blogic_cont(adapter, "\n");
>  	}
>  	/*
>  	   Process Command Invalid conditions.
> @@ -717,23 +724,37 @@ static int __init blogic_init_mm_probeinfo(struct blogic_adapter *adapter)
>  		pci_addr = base_addr1 = pci_resource_start(pci_device, 1);
>  
>  		if (pci_resource_flags(pci_device, 0) & IORESOURCE_MEM) {
> -			blogic_err("BusLogic: Base Address0 0x%lX not I/O for MultiMaster Host Adapter\n", NULL, base_addr0);
> -			blogic_err("at PCI Bus %d Device %d I/O Address 0x%lX\n", NULL, bus, device, io_addr);
> +			blogic_err(NULL,
> +				   "BusLogic: Base Address0 0x%lX not I/O for MultiMaster Host Adapter\n",
> +				   base_addr0);
> +			blogic_err(NULL,
> +				   "at PCI Bus %d Device %d I/O Address 0x%lX\n",
> +				   bus, device, io_addr);
>  			continue;
>  		}
>  		if (pci_resource_flags(pci_device, 1) & IORESOURCE_IO) {
> -			blogic_err("BusLogic: Base Address1 0x%lX not Memory for MultiMaster Host Adapter\n", NULL, base_addr1);
> -			blogic_err("at PCI Bus %d Device %d PCI Address 0x%lX\n", NULL, bus, device, pci_addr);
> +			blogic_err(NULL,
> +				   "BusLogic: Base Address1 0x%lX not Memory for MultiMaster Host Adapter\n",
> +				   base_addr1);
> +			blogic_err(NULL,
> +				   "at PCI Bus %d Device %d PCI Address 0x%lX\n",
> +				   bus, device, pci_addr);
>  			continue;
>  		}
>  		if (irq_ch == 0) {
> -			blogic_err("BusLogic: IRQ Channel %d invalid for MultiMaster Host Adapter\n", NULL, irq_ch);
> -			blogic_err("at PCI Bus %d Device %d I/O Address 0x%lX\n", NULL, bus, device, io_addr);
> +			blogic_err(NULL,
> +				   "BusLogic: IRQ Channel %d invalid for MultiMaster Host Adapter\n",
> +				   irq_ch);
> +			blogic_err(NULL,
> +				   "at PCI Bus %d Device %d I/O Address 0x%lX\n",
> +				   bus, device, io_addr);
>  			continue;
>  		}
>  		if (blogic_global_options.trace_probe) {
> -			blogic_notice("BusLogic: PCI MultiMaster Host Adapter detected at\n", NULL);
> -			blogic_notice("BusLogic: PCI Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX\n", NULL, bus, device, io_addr, pci_addr);
> +			blogic_notice(NULL, "BusLogic: PCI MultiMaster Host Adapter detected at\n");
> +			blogic_notice(NULL,
> +				      "BusLogic: PCI Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX\n",
> +				      bus, device, io_addr, pci_addr);
>  		}
>  		/*
>  		   Issue the Inquire PCI Host Adapter Information command to determine
> @@ -819,7 +840,7 @@ static int __init blogic_init_mm_probeinfo(struct blogic_adapter *adapter)
>  			nonpr_mmcount++;
>  			mmcount++;
>  		} else
> -			blogic_warn("BusLogic: Too many Host Adapters detected\n", NULL);
> +			blogic_warn(NULL, "BusLogic: Too many Host Adapters detected\n");
>  	}
>  	/*
>  	   If the AutoSCSI "Use Bus And Device # For PCI Scanning Seq."
> @@ -957,23 +978,37 @@ static int __init blogic_init_fp_probeinfo(struct blogic_adapter *adapter)
>  		pci_addr = base_addr1 = pci_resource_start(pci_device, 1);
>  #ifdef CONFIG_SCSI_FLASHPOINT
>  		if (pci_resource_flags(pci_device, 0) & IORESOURCE_MEM) {
> -			blogic_err("BusLogic: Base Address0 0x%lX not I/O for FlashPoint Host Adapter\n", NULL, base_addr0);
> -			blogic_err("at PCI Bus %d Device %d I/O Address 0x%lX\n", NULL, bus, device, io_addr);
> +			blogic_err(NULL,
> +				   "BusLogic: Base Address0 0x%lX not I/O for FlashPoint Host Adapter\n",
> +				   base_addr0);
> +			blogic_err(NULL,
> +				   "at PCI Bus %d Device %d I/O Address 0x%lX\n",
> +				   bus, device, io_addr);
>  			continue;
>  		}
>  		if (pci_resource_flags(pci_device, 1) & IORESOURCE_IO) {
> -			blogic_err("BusLogic: Base Address1 0x%lX not Memory for FlashPoint Host Adapter\n", NULL, base_addr1);
> -			blogic_err("at PCI Bus %d Device %d PCI Address 0x%lX\n", NULL, bus, device, pci_addr);
> +			blogic_err(NULL,
> +				   "BusLogic: Base Address1 0x%lX not Memory for FlashPoint Host Adapter\n",
> +				   base_addr1);
> +			blogic_err(NULL,
> +				   "at PCI Bus %d Device %d PCI Address 0x%lX\n",
> +				   bus, device, pci_addr);
>  			continue;
>  		}
>  		if (irq_ch == 0) {
> -			blogic_err("BusLogic: IRQ Channel %d invalid for FlashPoint Host Adapter\n", NULL, irq_ch);
> -			blogic_err("at PCI Bus %d Device %d I/O Address 0x%lX\n", NULL, bus, device, io_addr);
> +			blogic_err(NULL,
> +				   "BusLogic: IRQ Channel %d invalid for FlashPoint Host Adapter\n",
> +				   irq_ch);
> +			blogic_err(NULL,
> +				   "at PCI Bus %d Device %d I/O Address 0x%lX\n",
> +				   bus, device, io_addr);
>  			continue;
>  		}
>  		if (blogic_global_options.trace_probe) {
> -			blogic_notice("BusLogic: FlashPoint Host Adapter detected at\n", NULL);
> -			blogic_notice("BusLogic: PCI Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX\n", NULL, bus, device, io_addr, pci_addr);
> +			blogic_notice(NULL, "BusLogic: FlashPoint Host Adapter detected at\n");
> +			blogic_notice(NULL,
> +				      "BusLogic: PCI Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX\n",
> +				      bus, device, io_addr, pci_addr);
>  		}
>  		if (blogic_probeinfo_count < BLOGIC_MAX_ADAPTERS) {
>  			struct blogic_probeinfo *probeinfo =
> @@ -988,11 +1023,15 @@ static int __init blogic_init_fp_probeinfo(struct blogic_adapter *adapter)
>  			probeinfo->pci_device = pci_dev_get(pci_device);
>  			fpcount++;
>  		} else
> -			blogic_warn("BusLogic: Too many Host Adapters detected\n", NULL);
> +			blogic_warn(NULL, "BusLogic: Too many Host Adapters detected\n");
>  #else
> -		blogic_err("BusLogic: FlashPoint Host Adapter detected at PCI Bus %d Device %d\n", NULL, bus, device);
> -		blogic_err("BusLogic: I/O Address 0x%lX PCI Address 0x%lX, irq %d, but FlashPoint\n", NULL, io_addr, pci_addr, irq_ch);
> -		blogic_err("BusLogic: support was omitted in this kernel configuration.\n", NULL);
> +		blogic_err(NULL,
> +			   "BusLogic: FlashPoint Host Adapter detected at PCI Bus %d Device %d\n",
> +			   bus, device);
> +		blogic_err(NULL,
> +			   "BusLogic: I/O Address 0x%lX PCI Address 0x%lX, irq %d, but FlashPoint\n",
> +			   io_addr, pci_addr, irq_ch);
> +		blogic_err("BusLogic: support was omitted in this kernel configuration\n", NULL);
>  #endif
>  	}
>  	/*
> @@ -1098,15 +1137,19 @@ static bool blogic_failure(struct blogic_adapter *adapter, char *msg)
>  {
>  	blogic_announce_drvr(adapter);
>  	if (adapter->adapter_bus_type == BLOGIC_PCI_BUS) {
> -		blogic_err("While configuring BusLogic PCI Host Adapter at\n",
> -				adapter);
> -		blogic_err("Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX:\n", adapter, adapter->bus, adapter->dev, adapter->io_addr, adapter->pci_addr);
> +		blogic_err(adapter, "While configuring BusLogic PCI Host Adapter at\n");
> +		blogic_err(adapter,
> +			   "Bus %d Device %d I/O Address 0x%lX PCI Address 0x%lX:\n",
> +			   adapter->bus, adapter->dev, adapter->io_addr,
> +			   adapter->pci_addr);
>  	} else
> -		blogic_err("While configuring BusLogic Host Adapter at I/O Address 0x%lX:\n", adapter, adapter->io_addr);
> -	blogic_err("%s FAILED - DETACHING\n", adapter, msg);
> +		blogic_err(adapter,
> +			   "While configuring BusLogic Host Adapter at I/O Address 0x%lX:\n",
> +			   adapter->io_addr);
> +	blogic_err(adapter, "%s FAILED - DETACHING\n", msg);
>  	if (blogic_cmd_failure_reason != NULL)
> -		blogic_err("ADDITIONAL FAILURE INFO - %s\n", adapter,
> -				blogic_cmd_failure_reason);
> +		blogic_err(adapter, "ADDITIONAL FAILURE INFO - %s\n",
> +			   blogic_cmd_failure_reason);
>  	return false;
>  }
>  
> @@ -1130,13 +1173,20 @@ static bool __init blogic_probe(struct blogic_adapter *adapter)
>  		fpinfo->present = false;
>  		if (!(FlashPoint_ProbeHostAdapter(fpinfo) == 0 &&
>  					fpinfo->present)) {
> -			blogic_err("BusLogic: FlashPoint Host Adapter detected at PCI Bus %d Device %d\n", adapter, adapter->bus, adapter->dev);
> -			blogic_err("BusLogic: I/O Address 0x%lX PCI Address 0x%lX, but FlashPoint\n", adapter, adapter->io_addr, adapter->pci_addr);
> -			blogic_err("BusLogic: Probe Function failed to validate it.\n", adapter);
> +			blogic_err(adapter,
> +				   "BusLogic: FlashPoint Host Adapter detected at PCI Bus %d Device %d\n",
> +				   adapter->bus, adapter->dev);
> +			blogic_err(adapter,
> +				   "BusLogic: I/O Address 0x%lX PCI Address 0x%lX, but FlashPoint\n",
> +				   adapter->io_addr, adapter->pci_addr);
> +			blogic_err(adapter,
> +				   "BusLogic: Probe Function failed to validate it\n");
>  			return false;
>  		}
>  		if (blogic_global_options.trace_probe)
> -			blogic_notice("BusLogic_Probe(0x%lX): FlashPoint Found\n", adapter, adapter->io_addr);
> +			blogic_notice(adapter,
> +				      "BusLogic_Probe(0x%lX): FlashPoint Found\n",
> +				      adapter->io_addr);
>  		/*
>  		   Indicate the Host Adapter Probe completed successfully.
>  		 */
> @@ -1153,7 +1203,10 @@ static bool __init blogic_probe(struct blogic_adapter *adapter)
>  	intreg.all = blogic_rdint(adapter);
>  	georeg.all = blogic_rdgeom(adapter);
>  	if (blogic_global_options.trace_probe)
> -		blogic_notice("BusLogic_Probe(0x%lX): Status 0x%02X, Interrupt 0x%02X, Geometry 0x%02X\n", adapter, adapter->io_addr, statusreg.all, intreg.all, georeg.all);
> +		blogic_notice(adapter,
> +			      "BusLogic_Probe(0x%lX): Status 0x%02X, Interrupt 0x%02X, Geometry 0x%02X\n",
> +			      adapter->io_addr, statusreg.all, intreg.all,
> +			      georeg.all);
>  	if (statusreg.all == 0 || statusreg.sr.diag_active ||
>  			statusreg.sr.cmd_param_busy || statusreg.sr.rsvd ||
>  			statusreg.sr.cmd_invalid || intreg.ir.rsvd != 0)
> @@ -1232,7 +1285,9 @@ static bool blogic_hwreset(struct blogic_adapter *adapter, bool hard_reset)
>  		udelay(100);
>  	}
>  	if (blogic_global_options.trace_hw_reset)
> -		blogic_notice("BusLogic_HardwareReset(0x%lX): Diagnostic Active, Status 0x%02X\n", adapter, adapter->io_addr, statusreg.all);
> +		blogic_notice(adapter,
> +			      "BusLogic_HardwareReset(0x%lX): Diagnostic Active, Status 0x%02X\n",
> +			      adapter->io_addr, statusreg.all);
>  	if (timeout < 0)
>  		return false;
>  	/*
> @@ -1252,7 +1307,9 @@ static bool blogic_hwreset(struct blogic_adapter *adapter, bool hard_reset)
>  		udelay(100);
>  	}
>  	if (blogic_global_options.trace_hw_reset)
> -		blogic_notice("BusLogic_HardwareReset(0x%lX): Diagnostic Completed, Status 0x%02X\n", adapter, adapter->io_addr, statusreg.all);
> +		blogic_notice(adapter,
> +			      "BusLogic_HardwareReset(0x%lX): Diagnostic Completed, Status 0x%02X\n",
> +			      adapter->io_addr, statusreg.all);
>  	if (timeout < 0)
>  		return false;
>  	/*
> @@ -1268,7 +1325,9 @@ static bool blogic_hwreset(struct blogic_adapter *adapter, bool hard_reset)
>  		udelay(100);
>  	}
>  	if (blogic_global_options.trace_hw_reset)
> -		blogic_notice("BusLogic_HardwareReset(0x%lX): Host Adapter Ready, Status 0x%02X\n", adapter, adapter->io_addr, statusreg.all);
> +		blogic_notice(adapter,
> +			      "BusLogic_HardwareReset(0x%lX): Host Adapter Ready, Status 0x%02X\n",
> +			      adapter->io_addr, statusreg.all);
>  	if (timeout < 0)
>  		return false;
>  	/*
> @@ -1280,11 +1339,11 @@ static bool blogic_hwreset(struct blogic_adapter *adapter, bool hard_reset)
>  	if (statusreg.sr.diag_failed || !statusreg.sr.adapter_ready) {
>  		blogic_cmd_failure_reason = NULL;
>  		blogic_failure(adapter, "HARD RESET DIAGNOSTICS");
> -		blogic_err("HOST ADAPTER STATUS REGISTER = %02X\n", adapter,
> -				statusreg.all);
> +		blogic_err(adapter, "HOST ADAPTER STATUS REGISTER = %02X\n",
> +			   statusreg.all);
>  		if (statusreg.sr.datain_ready)
> -			blogic_err("HOST ADAPTER ERROR CODE = %d\n", adapter,
> -					blogic_rddatain(adapter));
> +			blogic_err(adapter, "HOST ADAPTER ERROR CODE = %d\n",
> +				   blogic_rddatain(adapter));
>  		return false;
>  	}
>  	/*
> @@ -1324,9 +1383,10 @@ static bool __init blogic_checkadapter(struct blogic_adapter *adapter)
>  	   Provide tracing information if requested and return.
>  	 */
>  	if (blogic_global_options.trace_probe)
> -		blogic_notice("BusLogic_Check(0x%lX): MultiMaster %s\n", adapter,
> -				adapter->io_addr,
> -				(result ? "Found" : "Not Found"));
> +		blogic_notice(adapter,
> +			      "BusLogic_Check(0x%lX): MultiMaster %s\n",
> +			      adapter->io_addr,
> +			      (result ? "Found" : "Not Found"));
>  	return result;
>  }
>  
> @@ -1836,30 +1896,40 @@ static bool __init blogic_reportconfig(struct blogic_adapter *adapter)
>  	char *tagq_msg = tagq_str;
>  	int tgt_id;
>  
> -	blogic_info("Configuring BusLogic Model %s %s%s%s%s SCSI Host Adapter\n", adapter, adapter->model, blogic_adapter_busnames[adapter->adapter_bus_type], (adapter->wide ? " Wide" : ""), (adapter->differential ? " Differential" : ""), (adapter->ultra ? " Ultra" : ""));
> -	blogic_info("  Firmware Version: %s, I/O Address: 0x%lX, IRQ Channel: %d/%s\n", adapter, adapter->fw_ver, adapter->io_addr, adapter->irq_ch, (adapter->level_int ? "Level" : "Edge"));
> +	blogic_info(adapter,
> +		    "Configuring BusLogic Model %s %s%s%s%s SCSI Host Adapter\n",
> +		    adapter->model,
> +		    blogic_adapter_busnames[adapter->adapter_bus_type],
> +		    (adapter->wide ? " Wide" : ""),
> +		    (adapter->differential ? " Differential" : ""),
> +		    (adapter->ultra ? " Ultra" : ""));
> +	blogic_info(adapter,
> +		    "  Firmware Version: %s, I/O Address: 0x%lX, IRQ Channel: %d/%s\n",
> +		    adapter->fw_ver, adapter->io_addr, adapter->irq_ch,
> +		    (adapter->level_int ? "Level" : "Edge"));
>  	if (adapter->adapter_bus_type != BLOGIC_PCI_BUS) {
> -		blogic_info("  DMA Channel: ", adapter);
> +		blogic_info(adapter, "  DMA Channel: ");
>  		if (adapter->dma_ch > 0)
> -			blogic_info("%d, ", adapter, adapter->dma_ch);
> +			blogic_cont(adapter, "%d, ", adapter->dma_ch);
>  		else
> -			blogic_info("None, ", adapter);
> +			blogic_cont(adapter, "None, ");
>  		if (adapter->bios_addr > 0)
> -			blogic_info("BIOS Address: 0x%lX, ", adapter,
> -					adapter->bios_addr);
> +			blogic_cont(adapter, "BIOS Address: 0x%X, ",
> +				    adapter->bios_addr);
>  		else
> -			blogic_info("BIOS Address: None, ", adapter);
> +			blogic_cont(adapter, "BIOS Address: None, ");
>  	} else {
> -		blogic_info("  PCI Bus: %d, Device: %d, Address: ", adapter,
> -				adapter->bus, adapter->dev);
> +		blogic_cont(adapter, "  PCI Bus: %d, Device: %d, Address: ",
> +			    adapter->bus, adapter->dev);
>  		if (adapter->pci_addr > 0)
> -			blogic_info("0x%lX, ", adapter, adapter->pci_addr);
> +			blogic_cont(adapter, "0x%lX, ", adapter->pci_addr);
>  		else
> -			blogic_info("Unassigned, ", adapter);
> +			blogic_cont(adapter, "Unassigned, ");
>  	}
> -	blogic_info("Host Adapter SCSI ID: %d\n", adapter, adapter->scsi_id);
> -	blogic_info("  Parity Checking: %s, Extended Translation: %s\n",
> -			adapter, (adapter->parity ? "Enabled" : "Disabled"),
> +	blogic_cont(adapter, "Host Adapter SCSI ID: %d\n", adapter->scsi_id);
> +	blogic_info(adapter,
> +		    "  Parity Checking: %s, Extended Translation: %s\n",
> +			(adapter->parity ? "Enabled" : "Disabled"),
>  			(adapter->ext_trans_enable ? "Enabled" : "Disabled"));
>  	alltgt_mask &= ~(1 << adapter->scsi_id);
>  	sync_ok = adapter->sync_ok & alltgt_mask;
> @@ -1928,16 +1998,25 @@ static bool __init blogic_reportconfig(struct blogic_adapter *adapter)
>  		tagq_str[adapter->scsi_id] = '#';
>  		tagq_str[adapter->maxdev] = '\0';
>  	}
> -	blogic_info("  Synchronous Negotiation: %s, Wide Negotiation: %s\n",
> -			adapter, syncmsg, widemsg);
> -	blogic_info("  Disconnect/Reconnect: %s, Tagged Queuing: %s\n", adapter,
> -			discon_msg, tagq_msg);
> +	blogic_info(adapter,
> +		    "  Synchronous Negotiation: %s, Wide Negotiation: %s\n",
> +		    syncmsg, widemsg);
> +	blogic_info(adapter,
> +		    "  Disconnect/Reconnect: %s, Tagged Queuing: %s\n",
> +		    discon_msg, tagq_msg);
>  	if (blogic_multimaster_type(adapter)) {
> -		blogic_info("  Scatter/Gather Limit: %d of %d segments, Mailboxes: %d\n", adapter, adapter->drvr_sglimit, adapter->adapter_sglimit, adapter->mbox_count);
> -		blogic_info("  Driver Queue Depth: %d, Host Adapter Queue Depth: %d\n", adapter, adapter->drvr_qdepth, adapter->adapter_qdepth);
> +		blogic_info(adapter,
> +			    "  Scatter/Gather Limit: %d of %d segments, Mailboxes: %d\n",
> +			    adapter->drvr_sglimit, adapter->adapter_sglimit,
> +			    adapter->mbox_count);
> +		blogic_info(adapter,
> +			    "  Driver Queue Depth: %d, Host Adapter Queue Depth: %d\n",
> +			    adapter->drvr_qdepth, adapter->adapter_qdepth);
>  	} else
> -		blogic_info("  Driver Queue Depth: %d, Scatter/Gather Limit: %d segments\n", adapter, adapter->drvr_qdepth, adapter->drvr_sglimit);
> -	blogic_info("  Tagged Queue Depth: ", adapter);
> +		blogic_info(adapter,
> +			    "  Driver Queue Depth: %d, Scatter/Gather Limit: %d segments\n",
> +			    adapter->drvr_qdepth, adapter->drvr_sglimit);
> +	blogic_info(adapter, "  Tagged Queue Depth: ");
>  	common_tagq_depth = true;
>  	for (tgt_id = 1; tgt_id < adapter->maxdev; tgt_id++)
>  		if (adapter->qdepth[tgt_id] != adapter->qdepth[0]) {
> @@ -1946,24 +2025,28 @@ static bool __init blogic_reportconfig(struct blogic_adapter *adapter)
>  		}
>  	if (common_tagq_depth) {
>  		if (adapter->qdepth[0] > 0)
> -			blogic_info("%d", adapter, adapter->qdepth[0]);
> +			blogic_cont(adapter, "%d", adapter->qdepth[0]);
>  		else
> -			blogic_info("Automatic", adapter);
> +			blogic_cont(adapter, "Automatic");
>  	} else
> -		blogic_info("Individual", adapter);
> -	blogic_info(", Untagged Queue Depth: %d\n", adapter,
> -			adapter->untag_qdepth);
> +		blogic_cont(adapter, "Individual");
> +	blogic_cont(adapter, ", Untagged Queue Depth: %d\n",
> +		    adapter->untag_qdepth);
>  	if (adapter->terminfo_valid) {
>  		if (adapter->wide)
> -			blogic_info("  SCSI Bus Termination: %s", adapter,
> -				(adapter->low_term ? (adapter->high_term ? "Both Enabled" : "Low Enabled") : (adapter->high_term ? "High Enabled" : "Both Disabled")));
> +			blogic_info(adapter, "  SCSI Bus Termination: %s",
> +				    (adapter->low_term
> +				     ? (adapter->high_term ? "Both Enabled" : "Low Enabled")
> +				     : (adapter->high_term ? "High Enabled" : "Both Disabled")));
>  		else
> -			blogic_info("  SCSI Bus Termination: %s", adapter,
> -				(adapter->low_term ? "Enabled" : "Disabled"));
> +			blogic_info(adapter, "  SCSI Bus Termination: %s",
> +				    (adapter->low_term ? "Enabled" : "Disabled"));
>  		if (adapter->scam)
> -			blogic_info(", SCAM: %s", adapter,
> -				(adapter->scam_enabled ? (adapter->scam_lev2 ? "Enabled, Level 2" : "Enabled, Level 1") : "Disabled"));
> -		blogic_info("\n", adapter);
> +			blogic_cont(adapter, ", SCAM: %s",
> +				    (adapter->scam_enabled
> +				     ? (adapter->scam_lev2 ? "Enabled, Level 2" : "Enabled, Level 1")
> +				     : "Disabled"));
> +		blogic_cont(adapter, "\n");
>  	}
>  	/*
>  	   Indicate reporting the Host Adapter configuration completed
> @@ -1981,8 +2064,8 @@ static bool __init blogic_reportconfig(struct blogic_adapter *adapter)
>  static bool __init blogic_getres(struct blogic_adapter *adapter)
>  {
>  	if (adapter->irq_ch == 0) {
> -		blogic_err("NO LEGAL INTERRUPT CHANNEL ASSIGNED - DETACHING\n",
> -				adapter);
> +		blogic_err(adapter,
> +			   "NO LEGAL INTERRUPT CHANNEL ASSIGNED - DETACHING\n");
>  		return false;
>  	}
>  	/*
> @@ -1990,8 +2073,9 @@ static bool __init blogic_getres(struct blogic_adapter *adapter)
>  	 */
>  	if (request_irq(adapter->irq_ch, blogic_inthandler, IRQF_SHARED,
>  				adapter->full_model, adapter) < 0) {
> -		blogic_err("UNABLE TO ACQUIRE IRQ CHANNEL %d - DETACHING\n",
> -				adapter, adapter->irq_ch);
> +		blogic_err(adapter,
> +			   "UNABLE TO ACQUIRE IRQ CHANNEL %d - DETACHING\n",
> +			   adapter->irq_ch);
>  		return false;
>  	}
>  	adapter->irq_acquired = true;
> @@ -2000,7 +2084,9 @@ static bool __init blogic_getres(struct blogic_adapter *adapter)
>  	 */
>  	if (adapter->dma_ch > 0) {
>  		if (request_dma(adapter->dma_ch, adapter->full_model) < 0) {
> -			blogic_err("UNABLE TO ACQUIRE DMA CHANNEL %d - DETACHING\n", adapter, adapter->dma_ch);
> +			blogic_err(adapter,
> +				   "UNABLE TO ACQUIRE DMA CHANNEL %d - DETACHING\n",
> +				   adapter->dma_ch);
>  			return false;
>  		}
>  		set_dma_mode(adapter->dma_ch, DMA_MODE_CASCADE);
> @@ -2148,12 +2234,12 @@ static bool blogic_initadapter(struct blogic_adapter *adapter)
>  	 */
>  done:
>  	if (!adapter->adapter_initd) {
> -		blogic_info("*** %s Initialized Successfully ***\n", adapter,
> -				adapter->full_model);
> -		blogic_info("\n", adapter);
> +		blogic_info(adapter, "*** %s Initialized Successfully ***\n",
> +			    adapter->full_model);
> +		blogic_info(adapter, "\n");
>  	} else
> -		blogic_warn("*** %s Initialized Successfully ***\n", adapter,
> -				adapter->full_model);
> +		blogic_warn(adapter, "*** %s Initialized Successfully ***\n",
> +			    adapter->full_model);
>  	adapter->adapter_initd = true;
>  
>  	/*
> @@ -2365,15 +2451,16 @@ static int __init blogic_init(void)
>  	    kcalloc(BLOGIC_MAX_ADAPTERS, sizeof(struct blogic_probeinfo),
>  			    GFP_KERNEL);
>  	if (blogic_probeinfo_list == NULL) {
> -		blogic_err("BusLogic: Unable to allocate Probe Info List\n",
> -				NULL);
> +		blogic_err(NULL,
> +			   "BusLogic: Unable to allocate Probe Info List\n");
>  		return -ENOMEM;
>  	}
>  
>  	adapter = kzalloc(sizeof(struct blogic_adapter), GFP_KERNEL);
>  	if (adapter == NULL) {
>  		kfree(blogic_probeinfo_list);
> -		blogic_err("BusLogic: Unable to allocate Prototype Host Adapter\n", NULL);
> +		blogic_err(NULL,
> +			   "BusLogic: Unable to allocate Prototype Host Adapter\n");
>  		return -ENOMEM;
>  	}
>  
> @@ -2633,8 +2720,9 @@ static int blogic_resultcode(struct blogic_adapter *adapter,
>  	case BLOGIC_INVALID_OUTBOX_CODE:
>  	case BLOGIC_INVALID_CMD_CODE:
>  	case BLOGIC_BAD_CMD_PARAM:
> -		blogic_warn("BusLogic Driver Protocol Error 0x%02X\n",
> -				adapter, adapter_status);
> +		blogic_warn(adapter,
> +			    "BusLogic Driver Protocol Error 0x%02X\n",
> +			    adapter_status);
>  		fallthrough;
>  	case BLOGIC_DATA_UNDERRUN:
>  	case BLOGIC_DATA_OVERRUN:
> @@ -2659,8 +2747,8 @@ static int blogic_resultcode(struct blogic_adapter *adapter,
>  		hoststatus = DID_RESET;
>  		break;
>  	default:
> -		blogic_warn("Unknown Host Adapter Status 0x%02X\n", adapter,
> -				adapter_status);
> +		blogic_warn(adapter, "Unknown Host Adapter Status 0x%02X\n",
> +			    adapter_status);
>  		hoststatus = DID_ERROR;
>  		break;
>  	}
> @@ -2718,7 +2806,9 @@ static void blogic_scan_inbox(struct blogic_adapter *adapter)
>  				   then there is most likely a bug in
>  				   the Host Adapter firmware.
>  				 */
> -				blogic_warn("Illegal CCB #%ld status %d in Incoming Mailbox\n", adapter, ccb->serial, ccb->status);
> +				blogic_warn(adapter,
> +					    "Illegal CCB #%ld status %d in Incoming Mailbox\n",
> +					    ccb->serial, ccb->status);
>  			}
>  		}
>  		next_inbox->comp_code = BLOGIC_INBOX_FREE;
> @@ -2753,7 +2843,9 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
>  		if (ccb->opcode == BLOGIC_BDR) {
>  			int tgt_id = ccb->tgt_id;
>  
> -			blogic_warn("Bus Device Reset CCB #%ld to Target %d Completed\n", adapter, ccb->serial, tgt_id);
> +			blogic_warn(adapter,
> +				    "Bus Device Reset CCB #%ld to Target %d Completed\n",
> +				    ccb->serial, tgt_id);
>  			blogic_inc_count(&adapter->tgt_stats[tgt_id].bdr_done);
>  			adapter->tgt_flags[tgt_id].tagq_active = false;
>  			adapter->cmds_since_rst[tgt_id] = 0;
> @@ -2806,7 +2898,9 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
>  			case BLOGIC_INBOX_FREE:
>  			case BLOGIC_CMD_NOTFOUND:
>  			case BLOGIC_INVALID_CCB:
> -				blogic_warn("CCB #%ld to Target %d Impossible State\n", adapter, ccb->serial, ccb->tgt_id);
> +				blogic_warn(adapter,
> +					    "CCB #%ld to Target %d Impossible State\n",
> +					    ccb->serial, ccb->tgt_id);
>  				break;
>  			case BLOGIC_CMD_COMPLETE_GOOD:
>  				adapter->tgt_stats[ccb->tgt_id]
> @@ -2816,8 +2910,9 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
>  				command->result = DID_OK << 16;
>  				break;
>  			case BLOGIC_CMD_ABORT_BY_HOST:
> -				blogic_warn("CCB #%ld to Target %d Aborted\n",
> -					adapter, ccb->serial, ccb->tgt_id);
> +				blogic_warn(adapter,
> +					    "CCB #%ld to Target %d Aborted\n",
> +					    ccb->serial, ccb->tgt_id);
>  				blogic_inc_count(&adapter->tgt_stats[ccb->tgt_id].aborts_done);
>  				command->result = DID_ABORT << 16;
>  				break;
> @@ -2829,16 +2924,26 @@ static void blogic_process_ccbs(struct blogic_adapter *adapter)
>  					    .cmds_complete++;
>  					if (blogic_global_options.trace_err) {
>  						int i;
> -						blogic_notice("CCB #%ld Target %d: Result %X Host "
> -								"Adapter Status %02X Target Status %02X\n", adapter, ccb->serial, ccb->tgt_id, command->result, ccb->adapter_status, ccb->tgt_status);
> -						blogic_notice("CDB   ", adapter);
> +						blogic_notice(adapter,
> +							      "CCB #%ld Target %d: Result %X Host Adapter Status %02X Target Status %02X\n",
> +							      ccb->serial,
> +							      ccb->tgt_id,
> +							      command->result,
> +							      ccb->adapter_status,
> +							      ccb->tgt_status);
> +						blogic_notice(adapter,
> +							      "CDB   ");
>  						for (i = 0; i < ccb->cdblen; i++)
> -							blogic_notice(" %02X", adapter, ccb->cdb[i]);
> -						blogic_notice("\n", adapter);
> -						blogic_notice("Sense ", adapter);
> +							blogic_cont(adapter,
> +								    " %02X",
> +								    ccb->cdb[i]);
> +						blogic_cont(adapter, "\n");
> +						blogic_notice(adapter, "Sense ");
>  						for (i = 0; i < ccb->sense_datalen; i++)
> -							blogic_notice(" %02X", adapter, command->sense_buffer[i]);
> -						blogic_notice("\n", adapter);
> +							blogic_cont(adapter,
> +								    " %02X",
> +								    command->sense_buffer[i]);
> +						blogic_cont(adapter, "\n");
>  					}
>  				}
>  				break;
> @@ -2925,7 +3030,8 @@ static irqreturn_t blogic_inthandler(int irq_ch, void *devid)
>  				adapter->adapter_extreset = true;
>  				break;
>  			case FPOINT_INTERN_ERR:
> -				blogic_warn("Internal FlashPoint Error detected - Resetting Host Adapter\n", adapter);
> +				blogic_warn(adapter,
> +					    "Internal FlashPoint Error detected - Resetting Host Adapter\n");
>  				adapter->adapter_intern_err = true;
>  				break;
>  			}
> @@ -2939,12 +3045,16 @@ static irqreturn_t blogic_inthandler(int irq_ch, void *devid)
>  	   Reset the Host Adapter if requested.
>  	 */
>  	if (adapter->adapter_extreset) {
> -		blogic_warn("Resetting %s due to External SCSI Bus Reset\n", adapter, adapter->full_model);
> +		blogic_warn(adapter,
> +			    "Resetting %s due to External SCSI Bus Reset\n",
> +			    adapter->full_model);
>  		blogic_inc_count(&adapter->ext_resets);
>  		blogic_resetadapter(adapter, false);
>  		adapter->adapter_extreset = false;
>  	} else if (adapter->adapter_intern_err) {
> -		blogic_warn("Resetting %s due to Host Adapter Internal Error\n", adapter, adapter->full_model);
> +		blogic_warn(adapter,
> +			    "Resetting %s due to Host Adapter Internal Error\n",
> +			    adapter->full_model);
>  		blogic_inc_count(&adapter->adapter_intern_errors);
>  		blogic_resetadapter(adapter, true);
>  		adapter->adapter_intern_err = false;
> @@ -3142,8 +3252,9 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
>  			&& tgt_flags->tagq_ok &&
>  			(adapter->tagq_ok & (1 << tgt_id))) {
>  		tgt_flags->tagq_active = true;
> -		blogic_notice("Tagged Queuing now active for Target %d\n",
> -					adapter, tgt_id);
> +		blogic_notice(adapter,
> +			      "Tagged Queuing now active for Target %d\n",
> +			      tgt_id);
>  	}
>  	if (tgt_flags->tagq_active) {
>  		enum blogic_queuetag queuetag = BLOGIC_SIMPLETAG;
> @@ -3184,8 +3295,8 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
>  				command->sense_buffer, ccb->sense_datalen,
>  				DMA_FROM_DEVICE);
>  	if (dma_mapping_error(&adapter->pci_device->dev, sense_buf)) {
> -		blogic_err("DMA mapping for sense data buffer failed\n",
> -				adapter);
> +		blogic_err(adapter,
> +			   "DMA mapping for sense data buffer failed\n");
>  		blogic_dealloc_ccb(ccb, 0);
>  		return SCSI_MLQUEUE_HOST_BUSY;
>  	}
> @@ -3204,12 +3315,12 @@ static int blogic_qcmd_lck(struct scsi_cmnd *command,
>  		 */
>  		if (!blogic_write_outbox(adapter, BLOGIC_MBOX_START, ccb)) {
>  			spin_unlock_irq(adapter->scsi_host->host_lock);
> -			blogic_warn("Unable to write Outgoing Mailbox - Pausing for 1 second\n", adapter);
> +			blogic_warn(adapter, "Unable to write Outgoing Mailbox - Pausing for 1 second\n");
>  			blogic_delay(1);
>  			spin_lock_irq(adapter->scsi_host->host_lock);
>  			if (!blogic_write_outbox(adapter, BLOGIC_MBOX_START,
>  						ccb)) {
> -				blogic_warn("Still unable to write Outgoing Mailbox - Host Adapter Dead?\n", adapter);
> +				blogic_warn(adapter, "Still unable to write Outgoing Mailbox - Host Adapter Dead?\n");
>  				blogic_dealloc_ccb(ccb, 1);
>  				command->result = DID_ERROR << 16;
>  				command->scsi_done(command);
> @@ -3259,13 +3370,16 @@ static int blogic_abort(struct scsi_cmnd *command)
>  		if (ccb->command == command)
>  			break;
>  	if (ccb == NULL) {
> -		blogic_warn("Unable to Abort Command to Target %d - No CCB Found\n", adapter, tgt_id);
> +		blogic_warn(adapter, "Unable to Abort Command to Target %d - No CCB Found\n",
> +			    tgt_id);
>  		return SUCCESS;
>  	} else if (ccb->status == BLOGIC_CCB_COMPLETE) {
> -		blogic_warn("Unable to Abort Command to Target %d - CCB Completed\n", adapter, tgt_id);
> +		blogic_warn(adapter, "Unable to Abort Command to Target %d - CCB Completed\n",
> +			    tgt_id);
>  		return SUCCESS;
>  	} else if (ccb->status == BLOGIC_CCB_RESET) {
> -		blogic_warn("Unable to Abort Command to Target %d - CCB Reset\n", adapter, tgt_id);
> +		blogic_warn(adapter, "Unable to Abort Command to Target %d - CCB Reset\n",
> +			    tgt_id);
>  		return SUCCESS;
>  	}
>  	if (blogic_multimaster_type(adapter)) {
> @@ -3283,16 +3397,18 @@ static int blogic_abort(struct scsi_cmnd *command)
>  		 */
>  		if (adapter->tgt_flags[tgt_id].tagq_active &&
>  				adapter->fw_ver[0] < '5') {
> -			blogic_warn("Unable to Abort CCB #%ld to Target %d - Abort Tag Not Supported\n", adapter, ccb->serial, tgt_id);
> +			blogic_warn(adapter, "Unable to Abort CCB #%ld to Target %d - Abort Tag Not Supported\n",
> +				    ccb->serial, tgt_id);
>  			return FAILURE;
>  		} else if (blogic_write_outbox(adapter, BLOGIC_MBOX_ABORT,
>  					ccb)) {
> -			blogic_warn("Aborting CCB #%ld to Target %d\n",
> -					adapter, ccb->serial, tgt_id);
> +			blogic_warn(adapter, "Aborting CCB #%ld to Target %d\n",
> +				    ccb->serial, tgt_id);
>  			blogic_inc_count(&adapter->tgt_stats[tgt_id].aborts_tried);
>  			return SUCCESS;
>  		} else {
> -			blogic_warn("Unable to Abort CCB #%ld to Target %d - No Outgoing Mailboxes\n", adapter, ccb->serial, tgt_id);
> +			blogic_warn(adapter, "Unable to Abort CCB #%ld to Target %d - No Outgoing Mailboxes\n",
> +				    ccb->serial, tgt_id);
>  			return FAILURE;
>  		}
>  	} else {
> @@ -3300,8 +3416,8 @@ static int blogic_abort(struct scsi_cmnd *command)
>  		   Call the FlashPoint SCCB Manager to abort execution of
>  		   the CCB.
>  		 */
> -		blogic_warn("Aborting CCB #%ld to Target %d\n", adapter,
> -				ccb->serial, tgt_id);
> +		blogic_warn(adapter, "Aborting CCB #%ld to Target %d\n",
> +			    ccb->serial, tgt_id);
>  		blogic_inc_count(&adapter->tgt_stats[tgt_id].aborts_tried);
>  		FlashPoint_AbortCCB(adapter->cardhandle, ccb);
>  		/*
> @@ -3333,8 +3449,8 @@ static int blogic_resetadapter(struct blogic_adapter *adapter, bool hard_reset)
>  
>  	if (!(blogic_hwreset(adapter, hard_reset) &&
>  				blogic_initadapter(adapter))) {
> -		blogic_err("Resetting %s Failed\n", adapter,
> -						adapter->full_model);
> +		blogic_err(adapter, "Resetting %s Failed\n",
> +			   adapter->full_model);
>  		return FAILURE;
>  	}
>  
> @@ -3443,10 +3559,15 @@ static int blogic_diskparam(struct scsi_device *sdev, struct block_device *dev,
>  		diskparam->cylinders = (unsigned long) capacity / (diskparam->heads * diskparam->sectors);
>  		if (part_no < 4 && part_end_sector == diskparam->sectors) {
>  			if (diskparam->cylinders != saved_cyl)
> -				blogic_warn("Adopting Geometry %d/%d from Partition Table\n", adapter, diskparam->heads, diskparam->sectors);
> +				blogic_warn(adapter,
> +					    "Adopting Geometry %d/%d from Partition Table\n",
> +					    diskparam->heads,
> +					    diskparam->sectors);
>  		} else if (part_end_head > 0 || part_end_sector > 0) {
> -			blogic_warn("Warning: Partition Table appears to have Geometry %d/%d which is\n", adapter, part_end_head + 1, part_end_sector);
> -			blogic_warn("not compatible with current BusLogic Host Adapter Geometry %d/%d\n", adapter, diskparam->heads, diskparam->sectors);
> +			blogic_warn(adapter,
> +				    "Warning: Partition Table appears to have Geometry %d/%d which is not compatible with current BusLogic Host Adapter Geometry %d/%d\n",
> +				    part_end_head + 1, part_end_sector,
> +				    diskparam->heads, diskparam->sectors);
>  		}
>  	}
>  	kfree(buf);
> @@ -3571,6 +3692,7 @@ Target	Requested Completed  Requested Completed  Requested Completed\n\
>  	}
>  	seq_printf(m, "\nExternal Host Adapter Resets: %d\n", adapter->ext_resets);
>  	seq_printf(m, "Host Adapter Internal Errors: %d\n", adapter->adapter_intern_errors);
> +
>  	return 0;
>  }
>  
> @@ -3579,41 +3701,51 @@ Target	Requested Completed  Requested Completed  Requested Completed\n\
>    blogic_msg prints Driver Messages.
>  */
>  
> -static void blogic_msg(enum blogic_msglevel msglevel, char *fmt,
> -			struct blogic_adapter *adapter, ...)
> +static void blogic_msg(enum blogic_msglevel msglevel,
> +		       struct blogic_adapter *adapter, const char *fmt, ...)
>  {
>  	static char buf[BLOGIC_LINEBUF_SIZE];
> -	static bool begin = true;
>  	va_list args;
>  	int len = 0;
> +	const char *kern_level = blogic_msglevelmap[msglevel];
>  
> -	va_start(args, adapter);
> -	len = vsprintf(buf, fmt, args);
> +	va_start(args, fmt);
> +	len = vscnprintf(buf, sizeof(buf), fmt, args);
>  	va_end(args);
> -	if (msglevel == BLOGIC_ANNOUNCE_LEVEL) {
> +
> +	switch (msglevel) {
> +	case BLOGIC_ANNOUNCE_LEVEL: {
>  		static int msglines = 0;
> +
>  		strcpy(&adapter->msgbuf[adapter->msgbuflen], buf);
>  		adapter->msgbuflen += len;
>  		if (++msglines <= 2)
> -			printk("%sscsi: %s", blogic_msglevelmap[msglevel], buf);
> -	} else if (msglevel == BLOGIC_INFO_LEVEL) {
> +			printk("%sscsi: %s", kern_level, buf);
> +		break;
> +	}
> +
> +	case BLOGIC_INFO_LEVEL:
>  		strcpy(&adapter->msgbuf[adapter->msgbuflen], buf);
>  		adapter->msgbuflen += len;
> -		if (begin) {
> -			if (buf[0] != '\n' || len > 1)
> -				printk("%sscsi%d: %s", blogic_msglevelmap[msglevel], adapter->host_no, buf);
> -		} else
> -			printk("%s", buf);
> -	} else {
> -		if (begin) {
> -			if (adapter != NULL && adapter->adapter_initd)
> -				printk("%sscsi%d: %s", blogic_msglevelmap[msglevel], adapter->host_no, buf);
> -			else
> -				printk("%s%s", blogic_msglevelmap[msglevel], buf);
> -		} else
> -			printk("%s", buf);
> +		if (buf[0] != '\n' || len > 1)
> +			printk("%sscsi%d: %s",
> +			       kern_level, adapter->host_no, buf);
> +		break;
> +
> +	case BLOGIC_CONT_LEVEL:
> +		strcpy(&adapter->msgbuf[adapter->msgbuflen], buf);
> +		adapter->msgbuflen += len;
> +		printk("%s%s", kern_level, buf);
> +		break;
> +
> +	default:
> +		if (adapter && adapter->adapter_initd)
> +			printk("%sscsi%d: %s",
> +			       kern_level, adapter->host_no, buf);
> +		else
> +			printk("%s%s", kern_level, buf);
> +		break;
>  	}
> -	begin = (buf[len - 1] == '\n');
>  }
>  
>  
> @@ -3691,7 +3823,9 @@ static int __init blogic_parseopts(char *options)
>  					blogic_probe_options.probe134 = true;
>  					break;
>  				default:
> -					blogic_err("BusLogic: Invalid Driver Options (invalid I/O Address 0x%lX)\n", NULL, io_addr);
> +					blogic_err(NULL,
> +						   "BusLogic: Invalid Driver Options (invalid I/O Address 0x%lX)\n",
> +						   io_addr);
>  					return 0;
>  				}
>  			} else if (blogic_parse(&options, "NoProbeISA"))
> @@ -3712,7 +3846,9 @@ static int __init blogic_parseopts(char *options)
>  				for (tgt_id = 0; tgt_id < BLOGIC_MAXDEV; tgt_id++) {
>  					unsigned short qdepth = simple_strtoul(options, &options, 0);
>  					if (qdepth > BLOGIC_MAX_TAG_DEPTH) {
> -						blogic_err("BusLogic: Invalid Driver Options (invalid Queue Depth %d)\n", NULL, qdepth);
> +						blogic_err(NULL,
> +							   "BusLogic: Invalid Driver Options (invalid Queue Depth %d)\n",
> +							   qdepth);
>  						return 0;
>  					}
>  					drvr_opts->qdepth[tgt_id] = qdepth;
> @@ -3721,12 +3857,16 @@ static int __init blogic_parseopts(char *options)
>  					else if (*options == ']')
>  						break;
>  					else {
> -						blogic_err("BusLogic: Invalid Driver Options (',' or ']' expected at '%s')\n", NULL, options);
> +						blogic_err(NULL,
> +							   "BusLogic: Invalid Driver Options (',' or ']' expected at '%s')\n",
> +							   options);
>  						return 0;
>  					}
>  				}
>  				if (*options != ']') {
> -					blogic_err("BusLogic: Invalid Driver Options (']' expected at '%s')\n", NULL, options);
> +					blogic_err(NULL,
> +						   "BusLogic: Invalid Driver Options (']' expected at '%s')\n",
> +						   options);
>  					return 0;
>  				} else
>  					options++;
> @@ -3734,7 +3874,9 @@ static int __init blogic_parseopts(char *options)
>  				unsigned short qdepth = simple_strtoul(options, &options, 0);
>  				if (qdepth == 0 ||
>  						qdepth > BLOGIC_MAX_TAG_DEPTH) {
> -					blogic_err("BusLogic: Invalid Driver Options (invalid Queue Depth %d)\n", NULL, qdepth);
> +					blogic_err(NULL,
> +						   "BusLogic: Invalid Driver Options (invalid Queue Depth %d)\n",
> +						   qdepth);
>  					return 0;
>  				}
>  				drvr_opts->common_qdepth = qdepth;
> @@ -3780,7 +3922,9 @@ static int __init blogic_parseopts(char *options)
>  				unsigned short bus_settle_time =
>  					simple_strtoul(options, &options, 0);
>  				if (bus_settle_time > 5 * 60) {
> -					blogic_err("BusLogic: Invalid Driver Options (invalid Bus Settle Time %d)\n", NULL, bus_settle_time);
> +					blogic_err(NULL,
> +						   "BusLogic: Invalid Driver Options (invalid Bus Settle Time %d)\n",
> +						   bus_settle_time);
>  					return 0;
>  				}
>  				drvr_opts->bus_settle_time = bus_settle_time;
> @@ -3805,14 +3949,17 @@ static int __init blogic_parseopts(char *options)
>  			if (*options == ',')
>  				options++;
>  			else if (*options != ';' && *options != '\0') {
> -				blogic_err("BusLogic: Unexpected Driver Option '%s' ignored\n", NULL, options);
> +				blogic_err(NULL,
> +					   "BusLogic: Unexpected Driver Option '%s' ignored\n",
> +					   options);
>  				*options = '\0';
>  			}
>  		}
>  		if (!(blogic_drvr_options_count == 0 ||
>  			blogic_probeinfo_count == 0 ||
>  			blogic_drvr_options_count == blogic_probeinfo_count)) {
> -			blogic_err("BusLogic: Invalid Driver Options (all or no I/O Addresses must be specified)\n", NULL);
> +			blogic_err(NULL,
> +				   "BusLogic: Invalid Driver Options (all or no I/O Addresses must be specified)\n");
>  			return 0;
>  		}
>  		/*
> @@ -3866,7 +4013,8 @@ static int __init blogic_setup(char *str)
>  	(void) get_options(str, ARRAY_SIZE(ints), ints);
>  
>  	if (ints[0] != 0) {
> -		blogic_err("BusLogic: Obsolete Command Line Entry Format Ignored\n", NULL);
> +		blogic_err(NULL,
> +			   "BusLogic: Obsolete Command Line Entry Format Ignored\n");
>  		return 0;
>  	}
>  	if (str == NULL || *str == '\0')
> diff --git a/drivers/scsi/BusLogic.h b/drivers/scsi/BusLogic.h
> index 6182cc8a0344..42333fba6005 100644
> --- a/drivers/scsi/BusLogic.h
> +++ b/drivers/scsi/BusLogic.h
> @@ -108,30 +108,34 @@ enum blogic_msglevel {
>  	BLOGIC_INFO_LEVEL = 1,
>  	BLOGIC_NOTICE_LEVEL = 2,
>  	BLOGIC_WARN_LEVEL = 3,
> -	BLOGIC_ERR_LEVEL = 4
> +	BLOGIC_ERR_LEVEL = 4,
> +	BLOGIC_CONT_LEVEL = 5,
>  };
>  
> -static char *blogic_msglevelmap[] = { KERN_NOTICE, KERN_NOTICE, KERN_NOTICE, KERN_WARNING, KERN_ERR };
> +static char *blogic_msglevelmap[] = { KERN_NOTICE, KERN_NOTICE, KERN_NOTICE, KERN_WARNING, KERN_ERR, KERN_CONT };
>  
>  
>  /*
>    Define Driver Message macros.
>  */
>  
> -#define blogic_announce(format, args...) \
> -	blogic_msg(BLOGIC_ANNOUNCE_LEVEL, format, ##args)
> +#define blogic_announce(adapter, fmt, ...)				\
> +	blogic_msg(BLOGIC_ANNOUNCE_LEVEL, adapter, fmt, ##__VA_ARGS__)
>  
> -#define blogic_info(format, args...) \
> -	blogic_msg(BLOGIC_INFO_LEVEL, format, ##args)
> +#define blogic_cont(adapter, fmt, ...)					\
> +	blogic_msg(BLOGIC_CONT_LEVEL, adapter, fmt, ##__VA_ARGS__)
>  
> -#define blogic_notice(format, args...) \
> -	blogic_msg(BLOGIC_NOTICE_LEVEL, format, ##args)
> +#define blogic_info(adapter, fmt, ...)					\
> +	blogic_msg(BLOGIC_INFO_LEVEL, adapter, fmt, ##__VA_ARGS__)
>  
> -#define blogic_warn(format, args...) \
> -	blogic_msg(BLOGIC_WARN_LEVEL, format, ##args)
> +#define blogic_notice(adapter, fmt, ...)				\
> +	blogic_msg(BLOGIC_NOTICE_LEVEL, adapter, fmt, ##__VA_ARGS__)
>  
> -#define blogic_err(format, args...) \
> -	blogic_msg(BLOGIC_ERR_LEVEL, format, ##args)
> +#define blogic_warn(adapter, fmt, ...)					\
> +	blogic_msg(BLOGIC_WARN_LEVEL, adapter, fmt, ##__VA_ARGS__)
> +
> +#define blogic_err(adapter, fmt, ...)					\
> +	blogic_msg(BLOGIC_ERR_LEVEL, adapter, fmt, ##__VA_ARGS__)
>  
>  
>  /*
> @@ -1289,7 +1293,9 @@ static int blogic_slaveconfig(struct scsi_device *);
>  static void blogic_qcompleted_ccb(struct blogic_ccb *);
>  static irqreturn_t blogic_inthandler(int, void *);
>  static int blogic_resetadapter(struct blogic_adapter *, bool hard_reset);
> -static void blogic_msg(enum blogic_msglevel, char *, struct blogic_adapter *, ...);
> +__printf(3, 4)
> +static void blogic_msg(enum blogic_msglevel, struct blogic_adapter *adapter,
> +		       const char *fmt, ...);
>  static int __init blogic_setup(char *);
>  
>  #endif				/* _BUSLOGIC_H */
> 
>
Maciej W. Rozycki April 16, 2021, 9:25 p.m. UTC | #11
On Fri, 16 Apr 2021, Khalid Aziz wrote:

> >  Sadly I didn't get to these resources while they were still there, and 

> > neither did archive.org, and now they not appear available from anywhere 

> > online.  I'm sure Leonard had this all, but, alas, he is long gone too.

> 

> These documents were all gone by the time I started working on this

> driver in 2013.


 According to my e-mail archives I got my BT-958 directly from Mylex brand 
new as KT-958 back in early 1998 (the rest of the system is a bit older).  
It wasn't up until 2003 when I was caught by the issue with the LOG SENSE 
command that I got interested in the programming details of the adapter.  

 At that time Mylex was in flux already, having been bought by LSI shortly 
before.  Support advised me what was there at Leonard's www.dandelion.com 
site was all that was available (I have a personal copy of the site) and 
they would suggest to switch to their current products.  So it was too 
late already ten years before you got at the driver.

 I'll yet double-check the contents of the KT-958 kit which I have kept, 
but if there was any technical documentation supplied there on a CD (which 
I doubt), I would have surely discovered it earlier.  It's away along with 
the server, remotely managed, ~160km/100mi from here, so it'll be some 
time before I get at it though.

 Still, maybe one of the SCSI old-timers has that stuff stashed somewhere.  
I have plenty of technical documentation going back to early to mid 1990s 
(some in the hard copy form), not necessarily readily available nowadays. 
Sadly lots of such stuff goes offline or is completely lost to the mist of 
time.

  Maciej
Joe Perches April 17, 2021, 12:09 a.m. UTC | #12
On Fri, 2021-04-16 at 14:41 -0600, Khalid Aziz wrote:
> On 4/15/21 8:08 PM, Joe Perches wrote:

> > And while it's a lot more code, I'd prefer a solution that looks more

> > like the other commonly used kernel logging extension mechanisms

> > where adapter is placed before the format, ... in the argument list.

> 

> Hi Joe,

> 

> I don't mind making these changes. It is quite a bit of code but

> consistency with other kernel code is useful. Would you like to finalize

> this patch, or would you prefer that I take this patch as starting point

> and finalize it?


Probably better to apply Maciej's patches first and then something
like this.

btw: the coccinelle script was

@@
expression a, b;
@@

	\(blogic_announce\|blogic_info\|blogic_notice\|blogic_warn\|blogic_err\)(
-		a, b
+		b, a
		, ...)
David Laight April 17, 2021, 11:39 a.m. UTC | #13
From: Maciej W. Rozycki

> Sent: 16 April 2021 11:49

> 

> On Thu, 15 Apr 2021, Joe Perches wrote:

> 

> > In patch 2, vscnprintf should probably be used to make sure it's

> > 0 terminated.

> 

>  Why?  C99 has this[1]:

> 

> "The vsnprintf function is equivalent to snprintf, with the variable

> argument list replaced by arg, which shall have been initialized by the

> va_start macro (and possibly subsequent va_arg calls)."


vscnprintf() is normally the function you want (not vsnprintf())
because the return value is the number of characters actually
put into the buffer, not the number that would have been written
had the buffer been long enough.
Return values larger than the buffer size are almost never
allowed for - and are probably a set of 'buffer overflow' bugs.

While probably justified by saying that it lets you malloc()
a big enough buffer and try again, the return value is almost
certainly just historic.

The original sprintf() libc code allocated a FILE structure on
stack set to fully-buffered with the current buffer pointer set
to the caller's buffer and a buffer length of MAXINT.
It then just called vprintf() to do the work.

snprintf() was done the same way, except the buffer length was
set and the 'write character' (or 'flush buffer') function
intercepted to avoid writes beyond the buffer end.
(Possibly by re-routing the writes to a global buffer.)
The return value from vprintf() gets returned to the user.

The Unix versions have always '\0' terminated the buffer.
Only Microsoft has ever released an snprintf() that doesn't
'\0' terminate the output - another source of bugs.

Personally I think bounded string functions should
return the buffer size on overflow.
This means sequences of:
	offset += xxx(buf + offset, sizeof buf - offset, ...);
are safe and the overflow can be detected right at the end.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Maciej W. Rozycki April 17, 2021, 2:01 p.m. UTC | #14
On Sat, 17 Apr 2021, David Laight wrote:

> > > In patch 2, vscnprintf should probably be used to make sure it's
> > > 0 terminated.
> > 
> >  Why?  C99 has this[1]:
> > 
> > "The vsnprintf function is equivalent to snprintf, with the variable
> > argument list replaced by arg, which shall have been initialized by the
> > va_start macro (and possibly subsequent va_arg calls)."
> 
> vscnprintf() is normally the function you want (not vsnprintf())
> because the return value is the number of characters actually
> put into the buffer, not the number that would have been written
> had the buffer been long enough.

 Good catch, thank you!  I'll respin the series then.  Thank you for the 
background story too!

  Maciej
Ondrej Zary April 18, 2021, 8:21 p.m. UTC | #15
On Friday 16 April 2021 23:25:18 Maciej W. Rozycki wrote:
> On Fri, 16 Apr 2021, Khalid Aziz wrote:

> 

> > >  Sadly I didn't get to these resources while they were still there, and 

> > > neither did archive.org, and now they not appear available from anywhere 

> > > online.  I'm sure Leonard had this all, but, alas, he is long gone too.

> > 

> > These documents were all gone by the time I started working on this

> > driver in 2013.

> 

>  According to my e-mail archives I got my BT-958 directly from Mylex brand 

> new as KT-958 back in early 1998 (the rest of the system is a bit older).  

> It wasn't up until 2003 when I was caught by the issue with the LOG SENSE 

> command that I got interested in the programming details of the adapter.  

> 

>  At that time Mylex was in flux already, having been bought by LSI shortly 

> before.  Support advised me what was there at Leonard's www.dandelion.com 

> site was all that was available (I have a personal copy of the site) and 

> they would suggest to switch to their current products.  So it was too 

> late already ten years before you got at the driver.

> 

>  I'll yet double-check the contents of the KT-958 kit which I have kept, 

> but if there was any technical documentation supplied there on a CD (which 

> I doubt), I would have surely discovered it earlier.  It's away along with 

> the server, remotely managed, ~160km/100mi from here, so it'll be some 

> time before I get at it though.

> 

>  Still, maybe one of the SCSI old-timers has that stuff stashed somewhere.  

> I have plenty of technical documentation going back to early to mid 1990s 

> (some in the hard copy form), not necessarily readily available nowadays. 

> Sadly lots of such stuff goes offline or is completely lost to the mist of 

> time.

> 

>   Maciej

> 


Found the 3000763 document here:
https://doc.lagout.org/science/0_Computer Science/0_Computer History/old-hardware/buslogic/3000763_PCI_EISA_Wide_SCSI_Tech_Ref_Dec94.pdf

There's also 3002593 there:
https://doc.lagout.org/science/0_Computer Science/0_Computer History/old-hardware/buslogic/

-- 
Ondrej Zary
Khalid Aziz April 19, 2021, 3:06 p.m. UTC | #16
On 4/18/21 2:21 PM, Ondrej Zary wrote:
> On Friday 16 April 2021 23:25:18 Maciej W. Rozycki wrote:

>> On Fri, 16 Apr 2021, Khalid Aziz wrote:

>>

>>>>  Sadly I didn't get to these resources while they were still there, and 

>>>> neither did archive.org, and now they not appear available from anywhere 

>>>> online.  I'm sure Leonard had this all, but, alas, he is long gone too.

>>>

>>> These documents were all gone by the time I started working on this

>>> driver in 2013.

>>

>>  According to my e-mail archives I got my BT-958 directly from Mylex brand 

>> new as KT-958 back in early 1998 (the rest of the system is a bit older).  

>> It wasn't up until 2003 when I was caught by the issue with the LOG SENSE 

>> command that I got interested in the programming details of the adapter.  

>>

>>  At that time Mylex was in flux already, having been bought by LSI shortly 

>> before.  Support advised me what was there at Leonard's www.dandelion.com 

>> site was all that was available (I have a personal copy of the site) and 

>> they would suggest to switch to their current products.  So it was too 

>> late already ten years before you got at the driver.

>>

>>  I'll yet double-check the contents of the KT-958 kit which I have kept, 

>> but if there was any technical documentation supplied there on a CD (which 

>> I doubt), I would have surely discovered it earlier.  It's away along with 

>> the server, remotely managed, ~160km/100mi from here, so it'll be some 

>> time before I get at it though.

>>

>>  Still, maybe one of the SCSI old-timers has that stuff stashed somewhere.  

>> I have plenty of technical documentation going back to early to mid 1990s 

>> (some in the hard copy form), not necessarily readily available nowadays. 

>> Sadly lots of such stuff goes offline or is completely lost to the mist of 

>> time.

>>

>>   Maciej

>>

> 

> Found the 3000763 document here:

> https://doc.lagout.org/science/0_Computer Science/0_Computer History/old-hardware/buslogic/3000763_PCI_EISA_Wide_SCSI_Tech_Ref_Dec94.pdf

> 

> There's also 3002593 there:

> https://doc.lagout.org/science/0_Computer Science/0_Computer History/old-hardware/buslogic/

> 


Thanks!!!

--
Khalid
Maciej W. Rozycki April 19, 2021, 4:01 p.m. UTC | #17
On Mon, 19 Apr 2021, Khalid Aziz wrote:

> On 4/18/21 2:21 PM, Ondrej Zary wrote:

> > 

> > Found the 3000763 document here:

> > https://doc.lagout.org/science/0_Computer Science/0_Computer History/old-hardware/buslogic/3000763_PCI_EISA_Wide_SCSI_Tech_Ref_Dec94.pdf

> > 

> > There's also 3002593 there:

> > https://doc.lagout.org/science/0_Computer Science/0_Computer History/old-hardware/buslogic/

> > 

> 

> Thanks!!!


 Ondrej: Thanks a lot indeed!  These documents seem to have the essential 
interface details all covered, except for Fast-20 SCSI adapters, which I 
guess are a minor modification from the software's point of view.

 Khalid: I have skimmed over these documents and I infer 24-bit addressing 
can be verified with any MultiMaster adapter, including ones that do have 
32-bit addressing implemented, by using the legacy Initialize Mailbox HBA 
command.  That could be used to stop Christoph's recent changes for older 
adapter support removal and replace them with proper fixes for whatever 
has become broken.  Is that something you'd be willing as the driver's 
maintainer to look into, or shall I?

  Maciej
Khalid Aziz April 20, 2021, 2:16 a.m. UTC | #18
On 4/19/21 10:01 AM, Maciej W. Rozycki wrote:
> On Mon, 19 Apr 2021, Khalid Aziz wrote:

> 

>> On 4/18/21 2:21 PM, Ondrej Zary wrote:

>>>

>>> Found the 3000763 document here:

>>> https://doc.lagout.org/science/0_Computer Science/0_Computer History/old-hardware/buslogic/3000763_PCI_EISA_Wide_SCSI_Tech_Ref_Dec94.pdf

>>>

>>> There's also 3002593 there:

>>> https://doc.lagout.org/science/0_Computer Science/0_Computer History/old-hardware/buslogic/

>>>

>>

>> Thanks!!!

> 

>  Ondrej: Thanks a lot indeed!  These documents seem to have the essential 

> interface details all covered, except for Fast-20 SCSI adapters, which I 

> guess are a minor modification from the software's point of view.

> 

>  Khalid: I have skimmed over these documents and I infer 24-bit addressing 

> can be verified with any MultiMaster adapter, including ones that do have 

> 32-bit addressing implemented, by using the legacy Initialize Mailbox HBA 

> command.  That could be used to stop Christoph's recent changes for older 

> adapter support removal and replace them with proper fixes for whatever 

> has become broken.  Is that something you'd be willing as the driver's 

> maintainer to look into, or shall I?

> 


Hi Maciej,

Do you mean use OpCode 01 (INITIALIZE MAILBOX) to set a 24-bit address
for mailbox in place of OpCode 81? Verifying the change would be a
challenge. Do you have an old adapter to test it with? If you do, go
ahead and make the changes. I will be happy to review. I have only a
BT-757 adapter.

Thanks,
Khalid
Maciej W. Rozycki April 20, 2021, 6:02 p.m. UTC | #19
On Mon, 19 Apr 2021, Khalid Aziz wrote:

> >  Khalid: I have skimmed over these documents and I infer 24-bit addressing 

> > can be verified with any MultiMaster adapter, including ones that do have 

> > 32-bit addressing implemented, by using the legacy Initialize Mailbox HBA 

> > command.  That could be used to stop Christoph's recent changes for older 

> > adapter support removal and replace them with proper fixes for whatever 

> > has become broken.  Is that something you'd be willing as the driver's 

> > maintainer to look into, or shall I?

> 

> Do you mean use OpCode 01 (INITIALIZE MAILBOX) to set a 24-bit address

> for mailbox in place of OpCode 81? Verifying the change would be a

> challenge. Do you have an old adapter to test it with? If you do, go

> ahead and make the changes. I will be happy to review. I have only a

> BT-757 adapter.


 Yes, but upon inspection it looks like our driver doesn't use that opcode 
and relies solely on 32-bit Mode Initialize Mailbox (0x81) even with ISA 
devices.  That makes sense as documentation indicates the firmware has 
been designed to be unified so that the same binary microcontroller code 
runs across all BusLogic MultiMaster devices.

 Anyway given the unified API it should be straightforward to simulate an 
older adapter with a newer one, except for host bus protocol differences.  
So verifying the workaround for broken BT-445S adapters continues to work 
once modernised is not going to be a problem as it can be unconditionally 
activated in a debug environment.  That would verify correct DMA bounce 
buffer operation under the new scheme.

 Verifying actual ISA operations (third-party DMA, etc.) cannot be made 
this way, but as I understand the issue there is merely with passing data 
structures around and that may not require too much attention beyond 
getting things syntactically correct, which I gather someone forgot to do 
with a change made a while ago.  So that should be doable as well.

 NB as noted before I only have a BT-958 readily wired for operation.  I 
don't expect I have any other BusLogic hardware, but I may yet have to 
double-check a stash of hardware I have accumulated over the years.  But 
that is overseas, so I won't be able to get at it before we're at least 
somewhat closer to normality.  If all else fails I could possibly buy one.

 I have respun the series now as promised.  Does your BT-757 adapter avoid 
the issue with trailing allocation somehow?

  Maciej
Khalid Aziz April 22, 2021, 2:36 a.m. UTC | #20
On 4/20/21 12:02 PM, Maciej W. Rozycki wrote:
> On Mon, 19 Apr 2021, Khalid Aziz wrote:

> 

>>>  Khalid: I have skimmed over these documents and I infer 24-bit addressing 

>>> can be verified with any MultiMaster adapter, including ones that do have 

>>> 32-bit addressing implemented, by using the legacy Initialize Mailbox HBA 

>>> command.  That could be used to stop Christoph's recent changes for older 

>>> adapter support removal and replace them with proper fixes for whatever 

>>> has become broken.  Is that something you'd be willing as the driver's 

>>> maintainer to look into, or shall I?

>>

>> Do you mean use OpCode 01 (INITIALIZE MAILBOX) to set a 24-bit address

>> for mailbox in place of OpCode 81? Verifying the change would be a

>> challenge. Do you have an old adapter to test it with? If you do, go

>> ahead and make the changes. I will be happy to review. I have only a

>> BT-757 adapter.

> 

>  Yes, but upon inspection it looks like our driver doesn't use that opcode 

> and relies solely on 32-bit Mode Initialize Mailbox (0x81) even with ISA 

> devices.  That makes sense as documentation indicates the firmware has 

> been designed to be unified so that the same binary microcontroller code 

> runs across all BusLogic MultiMaster devices.

> 

>  Anyway given the unified API it should be straightforward to simulate an 

> older adapter with a newer one, except for host bus protocol differences.  

> So verifying the workaround for broken BT-445S adapters continues to work 

> once modernised is not going to be a problem as it can be unconditionally 

> activated in a debug environment.  That would verify correct DMA bounce 

> buffer operation under the new scheme.

> 

>  Verifying actual ISA operations (third-party DMA, etc.) cannot be made 

> this way, but as I understand the issue there is merely with passing data 

> structures around and that may not require too much attention beyond 

> getting things syntactically correct, which I gather someone forgot to do 

> with a change made a while ago.  So that should be doable as well.


In theory this sounds reasonable, but without being able to test with a
real hardware I would be concerned about making this change.

> 

>  NB as noted before I only have a BT-958 readily wired for operation.  I 

> don't expect I have any other BusLogic hardware, but I may yet have to 

> double-check a stash of hardware I have accumulated over the years.  But 

> that is overseas, so I won't be able to get at it before we're at least 

> somewhat closer to normality.  If all else fails I could possibly buy one.

> 

>  I have respun the series now as promised.  Does your BT-757 adapter avoid 

> the issue with trailing allocation somehow?

> 


Well, my only test machine with a legacy PCI slot died some time back. I
have been working on putting together a replacement and have now been
able to get a working machine with a BT-950 adapter. I have not seen
issue with trailing allocation upto 5.12-rc8. I am going to try the top
of tree as well to make sure I do not run into this issue.

--
Khalid
Maciej W. Rozycki April 22, 2021, 4:27 p.m. UTC | #21
On Wed, 21 Apr 2021, Khalid Aziz wrote:

> >  Verifying actual ISA operations (third-party DMA, etc.) cannot be made 

> > this way, but as I understand the issue there is merely with passing data 

> > structures around and that may not require too much attention beyond 

> > getting things syntactically correct, which I gather someone forgot to do 

> > with a change made a while ago.  So that should be doable as well.

> 

> In theory this sounds reasonable, but without being able to test with a

> real hardware I would be concerned about making this change.


 Sometimes you have little choice really and that would be less disruptive 
than dropping support altogether.  Even if there's a small issue somewhere 
it's easier to fix by a competent developer who actually gets the hands on 
a piece of hardware than bringing back old code that has been removed and 
consequently not updated according to internal API evolution, etc.

 I had this issue with the defxx driver with which for years I didn't have 
a specimen of the EISA variant of the hardware handled.  I did my best to 
maintain EISA support however and while indeed I made a few of mistakes on 
the way, they were easy to straighten out once I finally did get my hands 
on an EISA piece.

> >  NB as noted before I only have a BT-958 readily wired for operation.  I 

> > don't expect I have any other BusLogic hardware, but I may yet have to 

> > double-check a stash of hardware I have accumulated over the years.  But 

> > that is overseas, so I won't be able to get at it before we're at least 

> > somewhat closer to normality.  If all else fails I could possibly buy one.

> > 

> >  I have respun the series now as promised.  Does your BT-757 adapter avoid 

> > the issue with trailing allocation somehow?

> 

> Well, my only test machine with a legacy PCI slot died some time back. I

> have been working on putting together a replacement and have now been

> able to get a working machine with a BT-950 adapter. I have not seen

> issue with trailing allocation upto 5.12-rc8. I am going to try the top

> of tree as well to make sure I do not run into this issue.


 I guess you won't see the issue with a FlashPoint adapter as they work in 
a different manner.  I think your EISA MultiMaster device is more likely 
to have a problem here.

 And AFAICT most SCSI commands (or at least the older ones which used to 
be there when development was still active with the MultiMaster devices) 
return exactly as much data as requested, so I guess the issue may have 
gone unnoticed.  I'll see if I can find some time to investigate this 
further now that we have proper documentation available, but meanwhile I 
do hope the workaround I have come up with 18 years ago already is good 
enough to keep it.

  Maciej
Khalid Aziz April 22, 2021, 6:07 p.m. UTC | #22
On 4/22/21 10:27 AM, Maciej W. Rozycki wrote:
> On Wed, 21 Apr 2021, Khalid Aziz wrote:

> 

>>>  Verifying actual ISA operations (third-party DMA, etc.) cannot be made 

>>> this way, but as I understand the issue there is merely with passing data 

>>> structures around and that may not require too much attention beyond 

>>> getting things syntactically correct, which I gather someone forgot to do 

>>> with a change made a while ago.  So that should be doable as well.

>>

>> In theory this sounds reasonable, but without being able to test with a

>> real hardware I would be concerned about making this change.

> 

>  Sometimes you have little choice really and that would be less disruptive 

> than dropping support altogether.  Even if there's a small issue somewhere 

> it's easier to fix by a competent developer who actually gets the hands on 

> a piece of hardware than bringing back old code that has been removed and 

> consequently not updated according to internal API evolution, etc.


We are talking about removing support for BT-445S with firmware version
older than 3.37. That is a very specific case. To continue support for
this very specific case, we have to add new code to use local bounce
buffer and we have no hardware to verify this new code. This will be new
code whether we add it now or later after we find someone even has this
very old card with old firmware. I would prefer to remove support for
now and add new code to add support for firmware version older than 3.37
back only if there is a need later. For now anyone who is using a
BT-445S and has updated firmware on their card will not see a change.

--
Khalid
Maciej W. Rozycki April 22, 2021, 11:19 p.m. UTC | #23
On Thu, 22 Apr 2021, Khalid Aziz wrote:

> >>>  Verifying actual ISA operations (third-party DMA, etc.) cannot be made 

> >>> this way, but as I understand the issue there is merely with passing data 

> >>> structures around and that may not require too much attention beyond 

> >>> getting things syntactically correct, which I gather someone forgot to do 

> >>> with a change made a while ago.  So that should be doable as well.

> >>

> >> In theory this sounds reasonable, but without being able to test with a

> >> real hardware I would be concerned about making this change.

> > 

> >  Sometimes you have little choice really and that would be less disruptive 

> > than dropping support altogether.  Even if there's a small issue somewhere 

> > it's easier to fix by a competent developer who actually gets the hands on 

> > a piece of hardware than bringing back old code that has been removed and 

> > consequently not updated according to internal API evolution, etc.

> 

> We are talking about removing support for BT-445S with firmware version

> older than 3.37.


 Umm, no.  As still quoted above I referred to ISA devices, such as the 
BT-545C.  ISA only has 24 address lines so no firmware change can make 
these devices address memory beyond 16MiB (whether as a bus master or with 
the aid of an 8237 DMA controller).

> That is a very specific case. To continue support for

> this very specific case, we have to add new code to use local bounce

> buffer and we have no hardware to verify this new code. This will be new

> code whether we add it now or later after we find someone even has this

> very old card with old firmware. I would prefer to remove support for

> now and add new code to add support for firmware version older than 3.37

> back only if there is a need later. For now anyone who is using a

> BT-445S and has updated firmware on their card will not see a change.


 As long as ISA support has been retained the BT-445S can just reuse the 
logic.

 I'm not strongly attached to ISA support though, and we continue 
supporting other SCSI HBAs for ISA.  But we do that even though they 
require a dedicated driver while with the unified MultiMaster architecture 
it would seem support for another host bus should be low-hanging fruit.

  Maciej