aha152x: Clean up struct scsi_pointer usage

Commit Message

Finn Thain Feb. 21, 2022, 11:09 p.m. UTC

Bring aha152x into line with 10 other drivers which assign
scsi_host_template.cmd_size = sizeof(struct scsi_pointer)
and avoid the "struct foo { struct bar; };" silliness.

Remove a pointless scsi_pointer->have_data_in assignment.

Signed-off-by: Finn Thain <[e-mail]>
---
 drivers/scsi/aha152x.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

Comments

Christoph Hellwig Feb. 22, 2022, 8 a.m. UTC | #1

On Tue, Feb 22, 2022 at 10:09:42AM +1100, Finn Thain wrote:
> Bring aha152x into line with 10 other drivers which assign
> scsi_host_template.cmd_size = sizeof(struct scsi_pointer)
> and avoid the "struct foo { struct bar; };" silliness.
> 
> Remove a pointless scsi_pointer->have_data_in assignment.

I think this going in the wrong direction.  The scsi_pointer should
go away entirelym and the fields actually used by the driver should move
into the aha152x_cmd_priv structure instead.

Same for all other drivers still using the scsi_pointer.

Finn Thain Feb. 22, 2022, 11:08 p.m. UTC | #2

Hi Christoph,

On Tue, 22 Feb 2022, Christoph Hellwig wrote:

> On Tue, Feb 22, 2022 at 10:09:42AM +1100, Finn Thain wrote:
> > Bring aha152x into line with 10 other drivers which assign
> > scsi_host_template.cmd_size = sizeof(struct scsi_pointer)
> > and avoid the "struct foo { struct bar; };" silliness.
> > 
> > Remove a pointless scsi_pointer->have_data_in assignment.
> 
> I think this going in the wrong direction.  The scsi_pointer should go 
> away entirelym and the fields actually used by the driver should move 
> into the aha152x_cmd_priv structure instead.
> 
> Same for all other drivers still using the scsi_pointer.
> 

This patch is addressing an inconsistency in the patches already accepted 
into 5.18/scsi-staging in Martin's repo.

A number of Bart's patches had the same effect as the patch you're 
objecting to here. Hence,

$ git grep "cmd_size.*scsi_pointer"
drivers/scsi/a2091.c:               .cmd_size = sizeof(struct scsi_pointer),
drivers/scsi/a3000.c:	            .cmd_size = sizeof(struct scsi_pointer),
drivers/scsi/fdomain.c:             .cmd_size = sizeof(struct scsi_pointer),
drivers/scsi/gvp11.c:	            .cmd_size = sizeof(struct scsi_pointer),
drivers/scsi/imm.c:                 .cmd_size = sizeof(struct scsi_pointer),
drivers/scsi/mvme147.c:             .cmd_size = sizeof(struct scsi_pointer),
drivers/scsi/pcmcia/nsp_cs.c:       .cmd_size = sizeof(struct scsi_pointer),
drivers/scsi/pcmcia/sym53c500_cs.c: .cmd_size = sizeof(struct scsi_pointer),
drivers/scsi/ppa.c:                 .cmd_size = sizeof(struct scsi_pointer),
drivers/scsi/sgiwd93.c:             .cmd_size = sizeof(struct scsi_pointer),

Since that series was very popular with reviewers, and being that this 
patch is just more of the same, I have no idea as to how to proceed.

Are you asking me to rework Bart's series? Or are you asking Martin to 
drop it, or both, or neither...

Christoph Hellwig Feb. 23, 2022, 6:30 a.m. UTC | #3

On Wed, Feb 23, 2022 at 10:08:08AM +1100, Finn Thain wrote:
> Are you asking me to rework Bart's series? Or are you asking Martin to 
> drop it, or both, or neither...

Neither.  Either leave the as-far as I can tell harmless inconsistency
as-is, or if you want to fix it up send a patch that removes the usage
of scsi_pointer in aha152x entirely by adding the actually used members
to the private structure.

Patch

diff --git a/drivers/scsi/aha152x.c b/drivers/scsi/aha152x.c
index 34b2378075fd..70f49fba66be 100644
--- a/drivers/scsi/aha152x.c
+++ b/drivers/scsi/aha152x.c
@@ -316,15 +316,9 @@  enum {
 	check_condition = 0x0800,	/* requesting sense after CHECK CONDITION */
 };
 
-struct aha152x_cmd_priv {
-	struct scsi_pointer scsi_pointer;
-};
-
 static struct scsi_pointer *aha152x_scsi_pointer(struct scsi_cmnd *cmd)
 {
-	struct aha152x_cmd_priv *acmd = scsi_cmd_priv(cmd);
-
-	return &acmd->scsi_pointer;
+	return scsi_cmd_priv(cmd);
 }
 
 MODULE_AUTHOR("Jürgen Fischer");
@@ -931,7 +925,6 @@  static int aha152x_internal_queue(struct scsi_cmnd *SCpnt,
 	scsi_pointer->phase	   = not_issued | phase;
 	scsi_pointer->Status	   = 0x1; /* Ilegal status by SCSI standard */
 	scsi_pointer->Message	   = 0;
-	scsi_pointer->have_data_in = 0;
 	scsi_pointer->sent_command = 0;
 
 	if (scsi_pointer->phase & (resetting | check_condition)) {
@@ -2971,7 +2964,7 @@  static struct scsi_host_template aha152x_driver_template = {
 	.sg_tablesize			= SG_ALL,
 	.dma_boundary			= PAGE_SIZE - 1,
 	.slave_alloc			= aha152x_adjust_queue,
-	.cmd_size			= sizeof(struct aha152x_cmd_priv),
+	.cmd_size			= sizeof(struct scsi_pointer),
 };
 
 #if !defined(AHA152X_PCMCIA)