Skip to contentSkip to conversation

aic94xx: sas code clean-up phase 2

PATCH In review Alexis Bruemmer 2 participants 2 messages Jun 6, 2006 – Jun 8, 2006 SCSI

lore
Share & tools
lore
#0 lore raw

cover aic94xx: sas code clean-up phase 2

Here is round 2 of the aic94xx supporting sas code clean-up.

Again these patches are being created to address the comments in the following email:
http://marc.theaimsgroup.com/?l=linux-scsi&m=114624643916659&w=2

The address the following issues:

1) Merge sas header files into libsas.h and sas.h and move them into the include/scsi dir
2) Use available list_for_each_entry_safe_reverse() function
3) Remove the //depot SCM comments
4) Remove the sas_common.c file
5) Remove the long queue implementation comment-- no longer valid
6) Use bitops for testing, setting and clearing bits
7) Remove various inline functions

There are a few issues that remain, but before I address them I would like some clarification:

-all the code should be renamed to libsas to make sure this is helpers
for a host-based sas implementation, including the config option

Does this mean that you would like to see all the files under
the drivers/scsi/sas/ dir compressed into one drivers/scsi/libsas.c file or that the files should just be moved to driver/scsi and start with libsas_*.c

-sas_frames*.h need to be changed to avoid on the wite bitfields which
are completely non-portable. That'll cause quite a bit of churn all over the codebase unfortuantely. some of those headers should be merged with the current scsi_transport_sas.h, e.g. the latter should lose various constants that are in sas.h

why? There are bit fields in many places under drivers/scsi. There
shouldn't be any issues between 32 and 64 bit with the u8 definition and we haven't seen any issues across the two architectures we have tested here (ppc64 and x86_64). I just don't see the portability issue in this case, so please elaborate :).

- drivers/scsi/sas/README needs to be updated to match reality and move to Documentation/

What exactly do we want to talk about in this readme file. Describe
scsi_transport_sas.c? Explain the driver/scsi/sas/ dir?

- expander_conf.c should go away from the kernel tree.

Is there plan to implement this somewhere else? If not should it stay
for the moment.

Once these areas have been well discussed and we are all on the same page I will start on phase 3. The 7 patches have been attached for feedback. Thanks as always for your comments.

Regards,

Alexis

#1 lore raw
Quoted 10 lines · Alexis Bruemmer

There are a few issues that remain, but before I address them I would like some clarification:

-all the code should be renamed to libsas to make sure this is helpers
for a host-based sas implementation, including the config option

Does this mean that you would like to see all the files under
the drivers/scsi/sas/ dir compressed into one drivers/scsi/libsas.c file or that the files should just be moved to driver/scsi and start with libsas_*.c

I think it's enough code to warrant more than a source file. Following the libata example you could call them drivers/scsi/libsas-*.c or give it a subdirectory. It doesn't matter too much how to do it exactly.

Quoted 11 lines

-sas_frames*.h need to be changed to avoid on the wite bitfields which
are completely non-portable. That'll cause quite a bit of churn all over the codebase unfortuantely. some of those headers should be merged with the current scsi_transport_sas.h, e.g. the latter should lose various constants that are in sas.h

why? There are bit fields in many places under drivers/scsi. There
shouldn't be any issues between 32 and 64 bit with the u8 definition and we haven't seen any issues across the two architectures we have tested here (ppc64 and x86_64). I just don't see the portability issue in this case, so please elaborate :).

There's not gurantee about ordering of bit fields. Besides that they defeat us doing proper sparse endian checking. Please use proper mask and shift operations.

Quoted 5 lines

- drivers/scsi/sas/README needs to be updated to match reality and move to Documentation/

What exactly do we want to talk about in this readme file. Describe
scsi_transport_sas.c? Explain the driver/scsi/sas/ dir?

I don't care too much. At least remove bits that mention things not in the tree or are flatout wrong for the current codebase.

Quoted 4 lines

- expander_conf.c should go away from the kernel tree.

Is there plan to implement this somewhere else? If not should it stay
for the moment.

Doug's smp_utils. If you find anything in there that's missing please send patches to Doug.