On Thu, 2018-04-19 at 10:10 +0200, Johannes Thumshirn wrote:
> On Wed, Apr 18, 2018 at 04:00:43PM +0000, Bart Van Assche wrote:
> > Thank you for having come up with this so quickly. Something I do not
> > like about this patch series is that several new very short helper functions
> > are introduced, e.g. set_scsi_result(), clear_scsi_result(),
> > to_scsi_result()
> > and from_scsi_result(). If we would make scsi_result a union of a 32-bit
> > integer and a struct with the driver, host, msg and status bytes then we
> > would not need any of these new helper functions. Additionally, that
> > approach
> > would allow us to eliminate the {set,get}_{driver,host,msg,status}_byte()
> > functions.
>
> Honestly I don't really like these mini accessor functions as well.
> But when using a union we loose all the benefits of the enums as
> drivers still can touch the compound result value.
Hello Johannes,
How about reworking this patch series as follows:
- Modify the bsg driver such that it uses another field than the SCSI result for
storing its -E... result code.
- The next patch changes scsi_result from an int into a union and all SCSI code
that sets or uses the result is modified to use the result member of the
union.
- Modify the SCSI core and all LLDs to access the .driver/.host/.msg/.status
bytes of the union instead of .result.
- Remove the {set,get}_{driver,host,msg,status}_byte() helper functions.
- Remove the .result member of the union.
This approach has the following advantages:
- No new helper functions have to be introduced.
- The LLD changes can be split into one patch per LLD driver and one patch for
the
SCSI core. I think this will make reviewing a lot easier.
Thanks,
Bart.
