Just a few style comments:
+static int zorro_esp_irq_pending(struct esp *);
+static int cyber_esp_irq_pending(struct esp *);
+static int fastlane_esp_irq_pending(struct esp *);
Please avoid forward declarations wherever possible.
+struct zorro_driver_data {
+ const char *name;
+ unsigned long offset;
+ unsigned long dma_offset;
+ int absolute; /* offset is absolute address */
+ int scsi_option;
+ int (*irq_pending)(struct esp *esp);
+ void (*dma_invalidate)(struct esp *esp);
+ void (*send_dma_cmd)(struct esp *esp, u32 dma_addr, u32 esp_count,
+ u32 dma_count, int write, u8 cmd);
+};
Please use different esp_driver_ops instances for the different board
types.
+static const struct zorro_driver_data cyberstormI_data = {
+ .name = "CyberStormI",
+ .offset = 0xf400,
+ .dma_offset = 0xf800,
+ .absolute = 0,
+ .scsi_option = 0,
You can remove all the xero initializations on static data.
Also please align the = signs with tabs in struct declarations.
+static dma_addr_t zorro_esp_map_single(struct esp *esp, void *buf,
+ size_t sz, int dir)
+{
+ return dma_map_single(esp->dev, buf, sz, dir);
+}
+
+static int zorro_esp_map_sg(struct esp *esp, struct scatterlist *sg,
+ int num_sg, int dir)
+{
+ return dma_map_sg(esp->dev, sg, num_sg, dir);
+}
+
+static void zorro_esp_unmap_single(struct esp *esp, dma_addr_t addr,
+ size_t sz, int dir)
+{
+ dma_unmap_single(esp->dev, addr, sz, dir);
+}
+
+static void zorro_esp_unmap_sg(struct esp *esp, struct scatterlist *sg,
+ int num_sg, int dir)
+{
+ dma_unmap_sg(esp->dev, sg, num_sg, dir);
+}
These wrappers seem rather pointless.
+/* PIO macros as used in mac_esp.c.
+ * Note that addr and fifo arguments are local-scope variables declared
+ * in zorro_esp_send_pio_cmd(), the macros are only used in that function,
+ * and addr and fifo are referenced in each use of the macros so there
+ * is no need to pass them as macro parameters.
+ */
Please use normal Linux comment style:
/*
* Blah, blah, blah.
*/
+#define ZORRO_ESP_PIO_LOOP(operands, reg1) \
+ { \
+ asm volatile ( \
+ "1: moveb " operands "\n" \
+ " subqw #1,%1 \n" \
+ " jbne 1b \n" \
+ : "+a" (addr), "+r" (reg1) \
+ : "a" (fifo)); \
+ }
What is the point of the curly braces around the asm statement?
+static void zorro_esp_send_pio_cmd(struct esp *esp, u32 addr, u32 esp_count,
+ u32 dma_count, int write, u8 cmd)
+{
+ struct zorro_esp_priv *zep = dev_get_drvdata(esp->dev);
+ u8 __iomem *fifo = esp->regs + ESP_FDATA * 16;
+ u8 phase = esp->sreg & ESP_STAT_PMASK;
+
+ cmd &= ~ESP_CMD_DMA;
+
+ if (write) {
It seems like this routine would benefit from being split into a read
and a write one.
+// Blizzard 1230/60 SCSI-IV DMA
/* ... */
+ /* Use PIO if transferring message bytes to esp->command_block_dma.
+ * PIO requires a virtual address, so substitute esp->command_block
+ * for addr.
+ */
Comment style, again.
+static struct esp_driver_ops zorro_esp_ops = {
should be marked const.
+ .esp_write8 = zorro_esp_write8,
Just use a space after the '='.
+
+ if (zep->zorro3) {
+ /* Only Fastlane Z3 for now - add switch for correct struct
+ * dma_registers size if adding any more
+ */
+ esp->dma_regs = ioremap_nocache(dmaaddr,
+ sizeof(struct fastlane_dma_registers));
+ } else
+ esp->dma_regs = (void __iomem *)ZTWO_VADDR(dmaaddr);
doesn't this need a __force (and a comment on why it is ѕafe) to make
sparse happy?
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
