Re: [PATCH v2 26/44] mac53c94: Move the SCSI pointer to private command data

[Hannes Reinecke <hare@xxxxxxx>]

On 2/8/22 18:24, Bart Van Assche wrote:
> Set .cmd_size in the SCSI host template instead of using the SCSI pointer
> from struct scsi_cmnd. This patch prepares for removal of the SCSI pointer
> from struct scsi_cmnd.
>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> ---
>   drivers/scsi/mac53c94.c | 24 +++++++++++++-----------
>   drivers/scsi/mac53c94.h | 11 +++++++++++
>   2 files changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/mac53c94.c b/drivers/scsi/mac53c94.c
> index afa08309de36..f3005b38931f 100644
> --- a/drivers/scsi/mac53c94.c
> +++ b/drivers/scsi/mac53c94.c
> @@ -193,7 +193,8 @@ static void mac53c94_interrupt(int irq, void *dev_id)
>   	struct fsc_state *state = (struct fsc_state *) dev_id;
>   	struct mac53c94_regs __iomem *regs = state->regs;
>   	struct dbdma_regs __iomem *dma = state->dma;
> -	struct scsi_cmnd *cmd = state->current_req;
> +	struct scsi_cmnd *const cmd = state->current_req;
> +	struct scsi_pointer *const scsi_pointer = mac53c94_scsi_pointer(cmd);
>   	int nb, stat, seq, intr;
>   	static int mac53c94_errors;
>   
> @@ -263,10 +264,10 @@ static void mac53c94_interrupt(int irq, void *dev_id)
>   		/* set DMA controller going if any data to transfer */
>   		if ((stat & (STAT_MSG|STAT_CD)) == 0
>   		    && (scsi_sg_count(cmd) > 0 || scsi_bufflen(cmd))) {
> -			nb = cmd->SCp.this_residual;
> +			nb = scsi_pointer->this_residual;
>   			if (nb > 0xfff0)
>   				nb = 0xfff0;
> -			cmd->SCp.this_residual -= nb;
> +			scsi_pointer->this_residual -= nb;
>   			writeb(nb, &regs->count_lo);
>   			writeb(nb >> 8, &regs->count_mid);
>   			writeb(CMD_DMA_MODE + CMD_NOP, &regs->command);
> @@ -293,13 +294,13 @@ static void mac53c94_interrupt(int irq, void *dev_id)
>   			cmd_done(state, DID_ERROR << 16);
>   			return;
>   		}
> -		if (cmd->SCp.this_residual != 0
> +		if (scsi_pointer->this_residual != 0
>   		    && (stat & (STAT_MSG|STAT_CD)) == 0) {
>   			/* Set up the count regs to transfer more */
> -			nb = cmd->SCp.this_residual;
> +			nb = scsi_pointer->this_residual;
>   			if (nb > 0xfff0)
>   				nb = 0xfff0;
> -			cmd->SCp.this_residual -= nb;
> +			scsi_pointer->this_residual -= nb;
>   			writeb(nb, &regs->count_lo);
>   			writeb(nb >> 8, &regs->count_mid);
>   			writeb(CMD_DMA_MODE + CMD_NOP, &regs->command);
> @@ -321,8 +322,8 @@ static void mac53c94_interrupt(int irq, void *dev_id)
>   			cmd_done(state, DID_ERROR << 16);
>   			return;
>   		}
> -		cmd->SCp.Status = readb(&regs->fifo);
> -		cmd->SCp.Message = readb(&regs->fifo);
> +		scsi_pointer->Status = readb(&regs->fifo);
> +		scsi_pointer->Message = readb(&regs->fifo);
>   		writeb(CMD_ACCEPT_MSG, &regs->command);
>   		state->phase = busfreeing;
>   		break;
> @@ -330,8 +331,8 @@ static void mac53c94_interrupt(int irq, void *dev_id)
>   		if (intr != INTR_DISCONNECT) {
>   			printk(KERN_DEBUG "got intr %x when expected disconnect\n", intr);
>   		}
> -		cmd_done(state, (DID_OK << 16) + (cmd->SCp.Message << 8)
> -			 + cmd->SCp.Status);
> +		cmd_done(state, (DID_OK << 16) + (scsi_pointer->Message << 8)
> +			 + scsi_pointer->Status);
>   		break;
>   	default:
>   		printk(KERN_DEBUG "don't know about phase %d\n", state->phase);
> @@ -389,7 +390,7 @@ static void set_dma_cmds(struct fsc_state *state, struct scsi_cmnd *cmd)
>   	dma_cmd += OUTPUT_LAST - OUTPUT_MORE;
>   	dcmds[-1].command = cpu_to_le16(dma_cmd);
>   	dcmds->command = cpu_to_le16(DBDMA_STOP);
> -	cmd->SCp.this_residual = total;
> +	mac53c94_scsi_pointer(cmd)->this_residual = total;
>   }
>   
>   static struct scsi_host_template mac53c94_template = {
> @@ -401,6 +402,7 @@ static struct scsi_host_template mac53c94_template = {
>   	.this_id	= 7,
>   	.sg_tablesize	= SG_ALL,
>   	.max_segment_size = 65535,
> +	.cmd_size	= sizeof(struct mac53c94_cmd_priv),
>   };
>   
>   static int mac53c94_probe(struct macio_dev *mdev, const struct of_device_id *match)
> diff --git a/drivers/scsi/mac53c94.h b/drivers/scsi/mac53c94.h
> index 5df6e81f78a8..37d7d30f42ef 100644
> --- a/drivers/scsi/mac53c94.h
> +++ b/drivers/scsi/mac53c94.h
> @@ -212,4 +212,15 @@ struct mac53c94_regs {
>   #define CF4_TEST	0x02
>   #define CF4_BBTE	0x01
>   
> +struct mac53c94_cmd_priv {
> +	struct scsi_pointer scsi_pointer;
> +};
> +
> +static inline struct scsi_pointer *mac53c94_scsi_pointer(struct scsi_cmnd *cmd)
> +{
> +	struct mac53c94_cmd_priv *mcmd = scsi_cmd_priv(cmd);
> +
> +	return &mcmd->scsi_pointer;
> +}
> +
>   #endif /* _MAC53C94_H */

Also here: Why not use 'struct scsi_pointer' directly as command payload?

Cheers,

Hannes
--
Dr. Hannes Reinecke                Kernel Storage Architect
hare@xxxxxxx                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer