[027/117] advansys: Convert to the scsi_status union

Message ID 20210420000845.25873-28-bvanassche@acm.org
State New
Headers show
Series
  • Make better use of static type checking
Related show

Commit Message

Bart Van Assche April 20, 2021, 12:07 a.m.
An explanation of the purpose of this patch is available in the patch
"scsi: Introduce the scsi_status union".

Cc: Matthew Wilcox <willy@infradead.org>
Cc: Hannes Reinecke <hare@suse.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/advansys.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Matthew Wilcox (Oracle) April 20, 2021, 1:49 a.m. | #1
On Mon, Apr 19, 2021 at 05:07:15PM -0700, Bart Van Assche wrote:
> An explanation of the purpose of this patch is available in the patch

> "scsi: Introduce the scsi_status union".


That is not the correct way to write a changelog.
Douglas Gilbert April 20, 2021, 2:27 a.m. | #2
On 2021-04-19 9:49 p.m., Matthew Wilcox wrote:
> On Mon, Apr 19, 2021 at 05:07:15PM -0700, Bart Van Assche wrote:

>> An explanation of the purpose of this patch is available in the patch

>> "scsi: Introduce the scsi_status union".

> 

> That is not the correct way to write a changelog.

> 


And it is non-bisectable (I guess) and could only be made bisectable
(without some ugly unions) by rolling it up into one patch. But having
separate patches definitely makes it easier for me to look at the
sg and scsi_debug driver changes, which look fine at first glance.

Is there any way to mark a patchset like this non-bisectable? And
I think a separate patch that explains why this is being done (cause
the cover-sheet gets lost). Then git might think of a way not to
repeat that explanation 107 times.

Doug Gilbert
Bart Van Assche April 20, 2021, 3:17 a.m. | #3
On 4/19/21 6:49 PM, Matthew Wilcox wrote:
> On Mon, Apr 19, 2021 at 05:07:15PM -0700, Bart Van Assche wrote:

>> An explanation of the purpose of this patch is available in the patch

>> "scsi: Introduce the scsi_status union".

> 

> That is not the correct way to write a changelog.


Thanks for having taken a look. Once there is agreement about the
approach of this patch series I can write a script that replaces the
above text with the description it refers to.

Bart.
Bart Van Assche April 20, 2021, 3:20 a.m. | #4
On 4/19/21 7:27 PM, Douglas Gilbert wrote:
> And it is non-bisectable (I guess) and could only be made bisectable

> (without some ugly unions) by rolling it up into one patch. But having

> separate patches definitely makes it easier for me to look at the

> sg and scsi_debug driver changes, which look fine at first glance.

> 

> Is there any way to mark a patchset like this non-bisectable? And

> I think a separate patch that explains why this is being done (cause

> the cover-sheet gets lost). Then git might think of a way not to

> repeat that explanation 107 times.


Hi Doug,

If this would be considered useful I can integrate the text from the
cover letter into the description of one of the patches in this series.

This series should be fully bisectable. If not, it means that I made a
mistake.

Thanks,

Bart.
Matthew Wilcox (Oracle) April 20, 2021, 3:23 a.m. | #5
On Mon, Apr 19, 2021 at 08:17:23PM -0700, Bart Van Assche wrote:
> On 4/19/21 6:49 PM, Matthew Wilcox wrote:

> > On Mon, Apr 19, 2021 at 05:07:15PM -0700, Bart Van Assche wrote:

> >> An explanation of the purpose of this patch is available in the patch

> >> "scsi: Introduce the scsi_status union".

> > 

> > That is not the correct way to write a changelog.

> 

> Thanks for having taken a look. Once there is agreement about the

> approach of this patch series I can write a script that replaces the

> above text with the description it refers to.


... I haven't taken a look.  I have no idea what this patch does,
and I can't provide feedback on the approach.  Because I haven't seen
the approach.  All I've seen is this patch, and the one to sym53c8xx.
Take a look at those patches in isolation -- would you be able to provide
any kind of sensible feedback?
Bart Van Assche April 20, 2021, 3:01 p.m. | #6
On 4/19/21 8:23 PM, Matthew Wilcox wrote:
> ... I haven't taken a look.  I have no idea what this patch does,

> and I can't provide feedback on the approach.  Because I haven't seen

> the approach.  All I've seen is this patch, and the one to sym53c8xx.

> Take a look at those patches in isolation -- would you be able to provide

> any kind of sensible feedback?


Hi Matthew,

Please take a look at the description that is available at
https://lore.kernel.org/linux-scsi/20210420000845.25873-12-bvanassche@acm.org/T/#u

Thanks,

Bart.

Patch

diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index e9516de8c18b..9b8d463e1bfd 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -5980,7 +5980,7 @@  static void adv_isr_callback(ADV_DVC_VAR *adv_dvc_varp, ADV_SCSI_REQ_Q *scsiqp)
 	/*
 	 * 'done_status' contains the command's ending status.
 	 */
-	scp->result = 0;
+	scp->status.combined = 0;
 	switch (scsiqp->done_status) {
 	case QD_NO_ERROR:
 		ASC_DBG(2, "QD_NO_ERROR\n");
@@ -6732,7 +6732,7 @@  static void asc_isr_callback(ASC_DVC_VAR *asc_dvc_varp, ASC_QDONE_INFO *qdonep)
 	/*
 	 * 'qdonep' contains the command's ending status.
 	 */
-	scp->result = 0;
+	scp->status.combined = 0;
 	switch (qdonep->d3.done_stat) {
 	case QD_NO_ERROR:
 		ASC_DBG(2, "QD_NO_ERROR\n");