| Message ID | 20210420000845.25873-28-bvanassche@acm.org |
|---|---|
| State | New |
| Headers | show |
| Series | Make better use of static type checking | expand |
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.
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
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.
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.
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?
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.
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");
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(-)