Skip to contentSkip to conversation

drivers/scsi/scsi_error.c should #include "scsi_transport_api.h"

PATCH In review Adrian Bunk 4 participants 5 messages Nov 29, 2006 – Jan 2, 2007 SCSI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drivers/scsi/scsi_error.c should #include "scsi_transport_api.h"

Every file should #include the headers containing the prototypes for its global functions.

drivers/scsi/scsi_error.c renamed from drivers/scsi/scsi_error.c.old +1 -0
@@ -36,6 +36,7 @@
 
 #include "scsi_priv.h"
 #include "scsi_logging.h"
+#include "scsi_transport_api.h"
 
 #define SENSE_TIMEOUT		(10*HZ)
 #define START_UNIT_TIMEOUT	(30*HZ)
#1 lore raw
Quoted 1 line · Adrian Bunk

+#include "scsi_transport_api.h"

scsi_transport_api.h is a weird little file. It's not included by anything in the drivers/scsi directory, only drivers/scsi/libsas/sas_scsi_host.c:#include "../scsi_transport_api.h" drivers/ata/libata-eh.c:#include "../scsi/scsi_transport_api.h"

To me, that says it should be living in include/scsi/ somewhere ... maybe just put the one function prototype into scsi_eh.h?

#2 lore raw
Quoted 10 lines · Matthew Wilcox
Quoted 1 line · Adrian Bunk

+#include "scsi_transport_api.h"

scsi_transport_api.h is a weird little file. It's not included by anything in the drivers/scsi directory, only drivers/scsi/libsas/sas_scsi_host.c:#include "../scsi_transport_api.h" drivers/ata/libata-eh.c:#include "../scsi/scsi_transport_api.h"

To me, that says it should be living in include/scsi/ somewhere ... maybe just put the one function prototype into scsi_eh.h?

would it only go in include/scsi if it intends to be an exported api for LLDD's and/or user apps ? and stay in drivers/scsi if its an internal api within the scsi subsystem itself ?

Based on who uses it, I would say its internal right now.

-- james s

#3 lore raw
Quoted 3 lines · James Smart

would it only go in include/scsi if it intends to be an exported api for LLDD's and/or user apps ? and stay in drivers/scsi if its an internal api within the scsi subsystem itself ?

It isn't clear to me that's the intended use of include/scsi. If it is, it's already being violated, eg by

$ find * -type f |xargs grep scsi_host_scan_allowed
drivers/scsi/scsi_scan.c:       if (scsi_host_scan_allowed(shost))
drivers/scsi/scsi_scan.c:       if (scsi_host_scan_allowed(shost))
drivers/scsi/scsi_scan.c:       if (scsi_host_scan_allowed(shost)) {
drivers/scsi/scsi_scan.c:       if (!scsi_host_scan_allowed(shost))
include/scsi/scsi_host.h: * scsi_host_scan_allowed - Is scanning of this
host allowed
include/scsi/scsi_host.h:static inline int scsi_host_scan_allowed(struct
Scsi_Host *shost)

(a good candidate to be moved to scsi_scan.c, in fact!)

scsi_host_state_name, scsi_normalize_sense, scsi_reset_provider, scsi_test_unit_ready, scsi_put_command are all in similar usage to scsi_schedule_eh. There's probably more, I just picked some likely looking candidates.

#4 lore raw
In reply to Matthew Wilcox
Quoted 10 lines · Matthew Wilcox
Quoted 1 line · Adrian Bunk

+#include "scsi_transport_api.h"

scsi_transport_api.h is a weird little file. It's not included by anything in the drivers/scsi directory, only drivers/scsi/libsas/sas_scsi_host.c:#include "../scsi_transport_api.h" drivers/ata/libata-eh.c:#include "../scsi/scsi_transport_api.h"

To me, that says it should be living in include/scsi/ somewhere ... maybe just put the one function prototype into scsi_eh.h?

Yes, it should probably go into scsi_eh.h.