On 10/15/18 8:25 AM, Finn Thain wrote:
On Mon, 15 Oct 2018, Hannes Reinecke wrote:
On 10/14/18 8:12 AM, Finn Thain wrote:
As a temporary measure, the code to implement PIO transfers was
duplicated in zorro_esp and mac_esp. Now that it has stabilized
move the common code into the core driver but don't build it unless
needed.
This replaces the inline assembler with more portable writesb() calls.
Optimizing the m68k writesb() implementation is a separate patch.
Tested-by: Stan Johnson <userm57@xxxxxxxxx>
Signed-off-by: Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx>
Tested-by: Michael Schmitz <schmitzmic@xxxxxxxxx>
---
Changed since v1:
- Use shost_printk() instead of pr_err().
- Add new symbol CONFIG_SCSI_ESP_PIO to Kconfig.
---
drivers/scsi/Kconfig | 6 +
drivers/scsi/esp_scsi.c | 128 +++++++++++++++++++++
drivers/scsi/esp_scsi.h | 5 +
drivers/scsi/mac_esp.c | 173 +----------------------------
drivers/scsi/zorro_esp.c | 232 ++++++---------------------------------
5 files changed, 179 insertions(+), 365 deletions(-)
[ .. ]
diff --git a/drivers/scsi/esp_scsi.h b/drivers/scsi/esp_scsi.h
index d0c032803749..2590e5eda595 100644
--- a/drivers/scsi/esp_scsi.h
+++ b/drivers/scsi/esp_scsi.h
@@ -431,6 +431,7 @@ struct esp_driver_ops {
struct esp {
void __iomem *regs;
void __iomem *dma_regs;
+ u8 __iomem *fifo_reg;
const struct esp_driver_ops *ops;
@@ -540,6 +541,7 @@ struct esp {
void *dma;
int dmarev;
+ int send_cmd_error;
int send_cmd_residual;
};
These variables are only used within esp_send_pio_cmd(); consider making them
conditional based on the config option.
In the case of send_cmd_residual, that would mean a second #ifdef added to
esp_data_bytes_sent() where it gets used. I'm happy to comply but I fear
that all these #ifdefs may harm readability...
There are already other variables in struct esp that may go unused, such
as dma_regs, that don't have #ifdefs to elide them. Are these also
problematic in some way?
The unused fields in the struct are not so much an issue; in fact, it
rather complicated things when having individual fields in the struct
surrounded by CONFIG_XXX, as then the order of the fields would change
depending on the configuration. Which makes it really hard to debug ..
However, the function declaration really is a worry, as the actual
function body only exists when the config option is enabled.
So either add a dummy function or surround the function declaration by
CONFIG_ESP_PIO.
Otherwise I think Dan Carpenter and the likes are guaranteed to send you
a nice mail complaining about this ...
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@xxxxxxx +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
