[PATCH 0/2] [libata] sata_mv: Add support for Marvell's integrated SATA controller

[PATCH 0/2] [libata] sata_mv: Add support for Marvell's integrated SATA controller

[saeed.bishara]

From: Saeed Bishara <saeed@marvell.com>

This patch series adds support for the integrated SATA controller that can be found in some of Marvell's SoC's. This controller is based on the 88SX7042, but it connected directly to the internal bus of the SoC and not via PCI.

[PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[saeed.bishara]

From: Saeed Bishara <saeed@marvell.com>

The integrated SATA controller is connected directly to the SoC's
internal bus, not via PCI interface. this patch removes the dependency
on the PCI interface.

Signed-off-by: Saeed Bishara <saeed@marvell.com>
---
 drivers/ata/Kconfig   |    2 +-
 drivers/ata/sata_mv.c |  113 ++++++++++++++++++++++++++++++-------------------
 2 files changed, 71 insertions(+), 44 deletions(-)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index ba63619..c60842b 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -69,7 +69,7 @@ config ATA_PIIX
 
 config SATA_MV
 	tristate "Marvell SATA support (HIGHLY EXPERIMENTAL)"
-	depends on PCI && EXPERIMENTAL
+	depends on EXPERIMENTAL
 	help
 	  This option enables support for the Marvell Serial ATA family.
 	  Currently supports 88SX[56]0[48][01] chips.
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 97c3e11..cbd3f1b 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -421,7 +421,9 @@ static void mv_error_handler(struct ata_port *ap);
 static void mv_post_int_cmd(struct ata_queued_cmd *qc);
 static void mv_eh_freeze(struct ata_port *ap);
 static void mv_eh_thaw(struct ata_port *ap);
+#ifdef CONFIG_PCI
 static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
+#endif
 
 static void mv5_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
 			   unsigned int port);
@@ -638,14 +640,14 @@ static const struct pci_device_id mv_pci_tbl[] = {
 
 	{ }			/* terminate list */
 };
-
+#ifdef CONFIG_PCI
 static struct pci_driver mv_pci_driver = {
 	.name			= DRV_NAME,
 	.id_table		= mv_pci_tbl,
 	.probe			= mv_init_one,
 	.remove			= ata_pci_remove_one,
 };
-
+#endif
 static const struct mv_hw_ops mv5xxx_ops = {
 	.phy_errata		= mv5_phy_errata,
 	.enable_leds		= mv5_enable_leds,
@@ -665,45 +667,6 @@ static const struct mv_hw_ops mv6xxx_ops = {
 };
 
 /*
- * module options
- */
-static int msi;	      /* Use PCI msi; either zero (off, default) or non-zero */
-
-
-/* move to PCI layer or libata core? */
-static int pci_go_64(struct pci_dev *pdev)
-{
-	int rc;
-
-	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK)) {
-		rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
-		if (rc) {
-			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
-			if (rc) {
-				dev_printk(KERN_ERR, &pdev->dev,
-					   "64-bit DMA enable failed\n");
-				return rc;
-			}
-		}
-	} else {
-		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
-		if (rc) {
-			dev_printk(KERN_ERR, &pdev->dev,
-				   "32-bit DMA enable failed\n");
-			return rc;
-		}
-		rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
-		if (rc) {
-			dev_printk(KERN_ERR, &pdev->dev,
-				   "32-bit consistent DMA enable failed\n");
-			return rc;
-		}
-	}
-
-	return rc;
-}
-
-/*
  * Functions
  */
 
@@ -2578,8 +2541,10 @@ static int mv_init_host(struct ata_host *host, unsigned int board_idx)
 
 		mv_port_init(&ap->ioaddr, port_mmio);
 
+#ifdef CONFIG_PCI
 		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, -1, "mmio");
 		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, offset, "port");
+#endif
 	}
 
 	for (hc = 0; hc < n_hc; hc++) {
@@ -2616,6 +2581,47 @@ done:
 	return rc;
 }
 
+#ifdef CONFIG_PCI
+
+/*
+ * module options
+ */
+static int msi;	      /* Use PCI msi; either zero (off, default) or non-zero */
+
+
+/* move to PCI layer or libata core? */
+static int pci_go_64(struct pci_dev *pdev)
+{
+	int rc;
+
+	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK)) {
+		rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
+		if (rc) {
+			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
+			if (rc) {
+				dev_printk(KERN_ERR, &pdev->dev,
+					   "64-bit DMA enable failed\n");
+				return rc;
+			}
+		}
+	} else {
+		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
+		if (rc) {
+			dev_printk(KERN_ERR, &pdev->dev,
+				   "32-bit DMA enable failed\n");
+			return rc;
+		}
+		rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
+		if (rc) {
+			dev_printk(KERN_ERR, &pdev->dev,
+				   "32-bit consistent DMA enable failed\n");
+			return rc;
+		}
+	}
+
+	return rc;
+}
+
 /**
  *      mv_print_info - Dump key info to kernel log for perusal.
  *      @host: ATA host to print info about
@@ -2721,15 +2727,34 @@ static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 				 IS_GEN_I(hpriv) ? &mv5_sht : &mv6_sht);
 }
 
-static int __init mv_init(void)
+static int __init mv_pci_register_driver(void)
 {
 	return pci_register_driver(&mv_pci_driver);
 }
 
-static void __exit mv_exit(void)
+static void __exit mv_pci_unregister_driver(void)
 {
 	pci_unregister_driver(&mv_pci_driver);
 }
+#else
+static int __init mv_pci_register_driver(void)
+{
+	return 0;
+}
+
+static void __exit mv_pci_unregister_driver(void)
+{
+}
+#endif
+static int __init mv_init(void)
+{
+	return mv_pci_register_driver();
+}
+
+static void __exit mv_exit(void)
+{
+	mv_pci_unregister_driver();
+}
 
 MODULE_AUTHOR("Brett Russ");
 MODULE_DESCRIPTION("SCSI low-level driver for Marvell SATA controllers");
@@ -2737,8 +2762,10 @@ MODULE_LICENSE("GPL");
 MODULE_DEVICE_TABLE(pci, mv_pci_tbl);
 MODULE_VERSION(DRV_VERSION);
 
+#ifdef CONFIG_PCI
 module_param(msi, int, 0444);
 MODULE_PARM_DESC(msi, "Enable use of PCI MSI (0=off, 1=on)");
+#endif
 
 module_init(mv_init);
 module_exit(mv_exit);
-- 
1.5.0.6

[PATCH 2/2] [libata] sata_mv: Support integrated controllers

[saeed.bishara]

From: Saeed Bishara <saeed@marvell.com>

Marvell's Orion SoC includes SATA controllers based on Marvell's
PCI-to-SATA 88SX controllers. The integrated SATA unit is connected
directly to the internal bus of the Orion SoC, and not via PCI.
This patch extends the libATA sata_mv driver to support those
controllers.

Signed-off-by: Saeed Bishara <saeed@marvell.com>
---
 drivers/ata/sata_mv.c   |  392 ++++++++++++++++++++++++++++++++++++++++-------
 include/linux/sata_mv.h |   21 +++
 2 files changed, 358 insertions(+), 55 deletions(-)
 create mode 100644 include/linux/sata_mv.h

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index cbd3f1b..cace07c 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -67,6 +67,8 @@
 #include <linux/interrupt.h>
 #include <linux/dma-mapping.h>
 #include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/sata_mv.h>
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_cmnd.h>
 #include <scsi/scsi_device.h>
@@ -125,6 +127,9 @@ enum {
 	/* Host Flags */
 	MV_FLAG_DUAL_HC		= (1 << 30),  /* two SATA Host Controllers */
 	MV_FLAG_IRQ_COALESCE	= (1 << 29),  /* IRQ coalescing capability */
+	/* integrated controllers, no PCI interface */
+	MV_FLAG_INTEGRATED = (1 << 28),
+
 	MV_COMMON_FLAGS		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO | ATA_FLAG_NO_ATAPI |
 				  ATA_FLAG_PIO_POLLING,
@@ -170,6 +175,8 @@ enum {
 
 	HC_MAIN_IRQ_CAUSE_OFS	= 0x1d60,
 	HC_MAIN_IRQ_MASK_OFS	= 0x1d64,
+	HC_INTEGRATED_MAIN_IRQ_CAUSE_OFS = 0x20020,
+	HC_INTEGRATED_MAIN_IRQ_MASK_OFS = 0x20024,
 	PORT0_ERR		= (1 << 0),	/* shift by port # */
 	PORT0_DONE		= (1 << 1),	/* shift by port # */
 	HC0_IRQ_PEND		= 0x1ff,	/* bits 0-8 = HC0's ports */
@@ -185,12 +192,14 @@ enum {
 	TWSI_INT		= (1 << 24),
 	HC_MAIN_RSVD		= (0x7f << 25),	/* bits 31-25 */
 	HC_MAIN_RSVD_5		= (0x1fff << 19), /* bits 31-19 */
+	HC_MAIN_RSVD_INTEGRATED = (0x3fffffb << 6),     /* bits 31-9, 7-6 */
 	HC_MAIN_MASKED_IRQS	= (TRAN_LO_DONE | TRAN_HI_DONE |
 				   PORTS_0_7_COAL_DONE | GPIO_INT | TWSI_INT |
 				   HC_MAIN_RSVD),
 	HC_MAIN_MASKED_IRQS_5	= (PORTS_0_3_COAL_DONE | PORTS_4_7_COAL_DONE |
 				   HC_MAIN_RSVD_5),
-
+	HC_MAIN_MASKED_IRQS_INTEGRATED = (PORTS_0_3_COAL_DONE |
+					  HC_MAIN_RSVD_INTEGRATED),
 	/* SATAHC registers */
 	HC_CFG_OFS		= 0,
 
@@ -312,6 +321,7 @@ enum {
 #define IS_GEN_I(hpriv) ((hpriv)->hp_flags & MV_HP_GEN_I)
 #define IS_GEN_II(hpriv) ((hpriv)->hp_flags & MV_HP_GEN_II)
 #define IS_GEN_IIE(hpriv) ((hpriv)->hp_flags & MV_HP_GEN_IIE)
+#define HAS_PCI(host) (!((host)->ports[0]->flags & MV_FLAG_INTEGRATED))
 
 enum {
 	/* DMA boundary 0xffff is required by the s/g splitting
@@ -336,6 +346,7 @@ enum chip_type {
 	chip_608x,
 	chip_6042,
 	chip_7042,
+	chip_integrated,
 };
 
 /* Command ReQuest Block: 32B */
@@ -398,11 +409,15 @@ struct mv_hw_ops {
 	int (*reset_hc)(struct mv_host_priv *hpriv, void __iomem *mmio,
 			unsigned int n_hc);
 	void (*reset_flash)(struct mv_host_priv *hpriv, void __iomem *mmio);
-	void (*reset_bus)(struct pci_dev *pdev, void __iomem *mmio);
+	void (*reset_bus)(struct ata_host *host, void __iomem *mmio);
 };
 
 struct mv_host_priv {
 	u32			hp_flags;
+	int			n_ports;
+	void __iomem		*base;
+	void __iomem		*main_cause_reg_addr;
+	void __iomem		*main_mask_reg_addr;
 	struct mv_port_signal	signal[8];
 	const struct mv_hw_ops	*ops;
 };
@@ -422,8 +437,11 @@ static void mv_post_int_cmd(struct ata_queued_cmd *qc);
 static void mv_eh_freeze(struct ata_port *ap);
 static void mv_eh_thaw(struct ata_port *ap);
 #ifdef CONFIG_PCI
-static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
+static int mv_pci_init_one(struct pci_dev *pdev,
+			   const struct pci_device_id *ent);
 #endif
+static int mv_platform_probe(struct platform_device *pdev);
+static int __devexit mv_platform_remove(struct platform_device *pdev);
 
 static void mv5_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
 			   unsigned int port);
@@ -433,7 +451,7 @@ static void mv5_read_preamp(struct mv_host_priv *hpriv, int idx,
 static int mv5_reset_hc(struct mv_host_priv *hpriv, void __iomem *mmio,
 			unsigned int n_hc);
 static void mv5_reset_flash(struct mv_host_priv *hpriv, void __iomem *mmio);
-static void mv5_reset_bus(struct pci_dev *pdev, void __iomem *mmio);
+static void mv5_reset_bus(struct ata_host *host, void __iomem *mmio);
 
 static void mv6_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
 			   unsigned int port);
@@ -443,7 +461,16 @@ static void mv6_read_preamp(struct mv_host_priv *hpriv, int idx,
 static int mv6_reset_hc(struct mv_host_priv *hpriv, void __iomem *mmio,
 			unsigned int n_hc);
 static void mv6_reset_flash(struct mv_host_priv *hpriv, void __iomem *mmio);
-static void mv_reset_pci_bus(struct pci_dev *pdev, void __iomem *mmio);
+static void mv_integrated_enable_leds(struct mv_host_priv *hpriv,
+				      void __iomem *mmio);
+static void mv_integrated_read_preamp(struct mv_host_priv *hpriv, int idx,
+				      void __iomem *mmio);
+static int mv_integrated_reset_hc(struct mv_host_priv *hpriv,
+				  void __iomem *mmio, unsigned int n_hc);
+static void mv_integrated_reset_flash(struct mv_host_priv *hpriv,
+				      void __iomem *mmio);
+static void mv_integrated_reset_bus(struct ata_host *host, void __iomem *mmio);
+static void mv_reset_pci_bus(struct ata_host *host, void __iomem *mmio);
 static void mv_channel_reset(struct mv_host_priv *hpriv, void __iomem *mmio,
 			     unsigned int port_no);
 
@@ -611,6 +638,12 @@ static const struct ata_port_info mv_port_info[] = {
 		.udma_mask	= ATA_UDMA6,
 		.port_ops	= &mv_iie_ops,
 	},
+	{  /* chip_integrated */
+		.flags = MV_COMMON_FLAGS | MV_FLAG_INTEGRATED,
+		.pio_mask = 0x1f,      /* pio0-4 */
+		.udma_mask = ATA_UDMA6,
+		.port_ops = &mv_iie_ops,
+	},
 };
 
 static const struct pci_device_id mv_pci_tbl[] = {
@@ -644,10 +677,21 @@ static const struct pci_device_id mv_pci_tbl[] = {
 static struct pci_driver mv_pci_driver = {
 	.name			= DRV_NAME,
 	.id_table		= mv_pci_tbl,
-	.probe			= mv_init_one,
+	.probe			= mv_pci_init_one,
 	.remove			= ata_pci_remove_one,
 };
 #endif
+
+static struct platform_driver mv_platform_driver = {
+	.probe			= mv_platform_probe,
+	.remove			= __devexit_p(mv_platform_remove),
+	.driver			= {
+				   .name = DRV_NAME,
+				   .owner = THIS_MODULE,
+				  },
+};
+
+
 static const struct mv_hw_ops mv5xxx_ops = {
 	.phy_errata		= mv5_phy_errata,
 	.enable_leds		= mv5_enable_leds,
@@ -666,6 +710,15 @@ static const struct mv_hw_ops mv6xxx_ops = {
 	.reset_bus		= mv_reset_pci_bus,
 };
 
+static const struct mv_hw_ops mv_integrated_ops = {
+	.phy_errata		= mv6_phy_errata,
+	.enable_leds		= mv_integrated_enable_leds,
+	.read_preamp		= mv_integrated_read_preamp,
+	.reset_hc		= mv_integrated_reset_hc,
+	.reset_flash		= mv_integrated_reset_flash,
+	.reset_bus		= mv_integrated_reset_bus,
+};
+
 /*
  * Functions
  */
@@ -704,9 +757,15 @@ static inline void __iomem *mv_port_base(void __iomem *base, unsigned int port)
 		(mv_hardport_from_port(port) * MV_PORT_REG_SZ);
 }
 
+static inline void __iomem *mv_host_base(struct ata_host *host)
+{
+	struct mv_host_priv *hpriv = host->private_data;
+	return hpriv->base;
+}
+
 static inline void __iomem *mv_ap_base(struct ata_port *ap)
 {
-	return mv_port_base(ap->host->iomap[MV_PRIMARY_BAR], ap->port_no);
+	return mv_port_base(mv_host_base(ap->host), ap->port_no);
 }
 
 static inline int mv_get_hc_count(unsigned long port_flags)
@@ -1551,16 +1610,21 @@ static void mv_intr_edma(struct ata_port *ap)
  */
 static void mv_host_intr(struct ata_host *host, u32 relevant, unsigned int hc)
 {
-	void __iomem *mmio = host->iomap[MV_PRIMARY_BAR];
+	struct mv_host_priv *hpriv = host->private_data;
+	void __iomem *mmio = hpriv->base;
 	void __iomem *hc_mmio = mv_hc_base(mmio, hc);
 	u32 hc_irq_cause;
-	int port, port0;
+	int port, port0, last_port;
 
 	if (hc == 0)
 		port0 = 0;
 	else
 		port0 = MV_PORTS_PER_HC;
 
+	if (HAS_PCI(host))
+		last_port = port0 + MV_PORTS_PER_HC;
+	else
+		last_port = port0 + hpriv->n_ports;
 	/* we'll need the HC success int register in most cases */
 	hc_irq_cause = readl(hc_mmio + HC_IRQ_CAUSE_OFS);
 	if (!hc_irq_cause)
@@ -1571,7 +1635,7 @@ static void mv_host_intr(struct ata_host *host, u32 relevant, unsigned int hc)
 	VPRINTK("ENTER, hc%u relevant=0x%08x HC IRQ cause=0x%08x\n",
 		hc, relevant, hc_irq_cause);
 
-	for (port = port0; port < port0 + MV_PORTS_PER_HC; port++) {
+	for (port = port0; port < port0 + last_port; port++) {
 		struct ata_port *ap = host->ports[port];
 		struct mv_port_priv *pp = ap->private_data;
 		int have_err_bits, hard_port, shift;
@@ -1665,11 +1729,12 @@ static void mv_pci_error(struct ata_host *host, void __iomem *mmio)
 static irqreturn_t mv_interrupt(int irq, void *dev_instance)
 {
 	struct ata_host *host = dev_instance;
+	struct mv_host_priv *hpriv = host->private_data;
 	unsigned int hc, handled = 0, n_hcs;
-	void __iomem *mmio = host->iomap[MV_PRIMARY_BAR];
+	void __iomem *mmio = hpriv->base;
 	u32 irq_stat;
 
-	irq_stat = readl(mmio + HC_MAIN_IRQ_CAUSE_OFS);
+	irq_stat = readl(hpriv->main_cause_reg_addr);
 
 	/* check the cases where we either have nothing pending or have read
 	 * a bogus register value which can indicate HW removal or PCI fault
@@ -1680,7 +1745,7 @@ static irqreturn_t mv_interrupt(int irq, void *dev_instance)
 	n_hcs = mv_get_hc_count(host->ports[0]->flags);
 	spin_lock(&host->lock);
 
-	if (unlikely(irq_stat & PCI_ERR)) {
+	if (unlikely(irq_stat & PCI_ERR) && HAS_PCI(host)) {
 		mv_pci_error(host, mmio);
 		handled = 1;
 		goto out_unlock;	/* skip all other HC irq handling */
@@ -1727,7 +1792,8 @@ static unsigned int mv5_scr_offset(unsigned int sc_reg_in)
 
 static int mv5_scr_read(struct ata_port *ap, unsigned int sc_reg_in, u32 *val)
 {
-	void __iomem *mmio = ap->host->iomap[MV_PRIMARY_BAR];
+	struct mv_host_priv *hpriv = ap->host->private_data;
+	void __iomem *mmio = hpriv->base;
 	void __iomem *addr = mv5_phy_base(mmio, ap->port_no);
 	unsigned int ofs = mv5_scr_offset(sc_reg_in);
 
@@ -1740,7 +1806,8 @@ static int mv5_scr_read(struct ata_port *ap, unsigned int sc_reg_in, u32 *val)
 
 static int mv5_scr_write(struct ata_port *ap, unsigned int sc_reg_in, u32 val)
 {
-	void __iomem *mmio = ap->host->iomap[MV_PRIMARY_BAR];
+	struct mv_host_priv *hpriv = ap->host->private_data;
+	void __iomem *mmio = hpriv->base;
 	void __iomem *addr = mv5_phy_base(mmio, ap->port_no);
 	unsigned int ofs = mv5_scr_offset(sc_reg_in);
 
@@ -1751,8 +1818,9 @@ static int mv5_scr_write(struct ata_port *ap, unsigned int sc_reg_in, u32 val)
 		return -EINVAL;
 }
 
-static void mv5_reset_bus(struct pci_dev *pdev, void __iomem *mmio)
+static void mv5_reset_bus(struct ata_host *host, void __iomem *mmio)
 {
+	struct pci_dev *pdev = to_pci_dev(host->dev);
 	int early_5080;
 
 	early_5080 = (pdev->device == 0x5080) && (pdev->revision == 0);
@@ -1763,7 +1831,7 @@ static void mv5_reset_bus(struct pci_dev *pdev, void __iomem *mmio)
 		writel(tmp, mmio + MV_PCI_EXP_ROM_BAR_CTL);
 	}
 
-	mv_reset_pci_bus(pdev, mmio);
+	mv_reset_pci_bus(host, mmio);
 }
 
 static void mv5_reset_flash(struct mv_host_priv *hpriv, void __iomem *mmio)
@@ -1887,7 +1955,7 @@ static int mv5_reset_hc(struct mv_host_priv *hpriv, void __iomem *mmio,
 
 #undef ZERO
 #define ZERO(reg) writel(0, mmio + (reg))
-static void mv_reset_pci_bus(struct pci_dev *pdev, void __iomem *mmio)
+static void mv_reset_pci_bus(struct ata_host *host, void __iomem *mmio)
 {
 	u32 tmp;
 
@@ -2076,6 +2144,93 @@ static void mv6_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
 	writel(m2, port_mmio + PHY_MODE2);
 }
 
+/* TODO: use the generic LED interface to configure the SATA Presence */
+/* & Acitivy LEDs on the board */
+static void mv_integrated_enable_leds(struct mv_host_priv *hpriv,
+				      void __iomem *mmio)
+{
+	return;
+}
+
+static void mv_integrated_read_preamp(struct mv_host_priv *hpriv, int idx,
+			   void __iomem *mmio)
+{
+	void __iomem *port_mmio;
+	u32 tmp;
+
+	port_mmio = mv_port_base(mmio, idx);
+	tmp = readl(port_mmio + PHY_MODE2);
+
+	hpriv->signal[idx].amps = tmp & 0x700;	/* bits 10:8 */
+	hpriv->signal[idx].pre = tmp & 0xe0;	/* bits 7:5 */
+}
+
+#undef ZERO
+#define ZERO(reg) writel(0, port_mmio + (reg))
+static void mv_integrated_reset_hc_port(struct mv_host_priv *hpriv,
+					void __iomem *mmio, unsigned int port)
+{
+	void __iomem *port_mmio = mv_port_base(mmio, port);
+
+	writelfl(EDMA_DS, port_mmio + EDMA_CMD_OFS);
+
+	mv_channel_reset(hpriv, mmio, port);
+
+	ZERO(0x028);		/* command */
+	writel(0x101f, port_mmio + EDMA_CFG_OFS);
+	ZERO(0x004);		/* timer */
+	ZERO(0x008);		/* irq err cause */
+	ZERO(0x00c);		/* irq err mask */
+	ZERO(0x010);		/* rq bah */
+	ZERO(0x014);		/* rq inp */
+	ZERO(0x018);		/* rq outp */
+	ZERO(0x01c);		/* respq bah */
+	ZERO(0x024);		/* respq outp */
+	ZERO(0x020);		/* respq inp */
+	ZERO(0x02c);		/* test control */
+	writel(0xbc, port_mmio + EDMA_IORDY_TMOUT);
+}
+
+#undef ZERO
+
+#define ZERO(reg) writel(0, hc_mmio + (reg))
+static void mv_integrated_reset_one_hc(struct mv_host_priv *hpriv,
+				       void __iomem *mmio)
+{
+	void __iomem *hc_mmio = mv_hc_base(mmio, 0);
+
+	ZERO(0x00c);
+	ZERO(0x010);
+	ZERO(0x014);
+
+}
+
+#undef ZERO
+
+static int mv_integrated_reset_hc(struct mv_host_priv *hpriv,
+				  void __iomem *mmio, unsigned int n_hc)
+{
+	unsigned int port;
+
+	for (port = 0; port < hpriv->n_ports; port++)
+		mv_integrated_reset_hc_port(hpriv, mmio, port);
+
+	mv_integrated_reset_one_hc(hpriv, mmio);
+
+	return 0;
+}
+
+static void mv_integrated_reset_flash(struct mv_host_priv *hpriv,
+				      void __iomem *mmio)
+{
+	return;
+}
+
+static void mv_integrated_reset_bus(struct ata_host *host, void __iomem *mmio)
+{
+	return;
+}
+
 static void mv_channel_reset(struct mv_host_priv *hpriv, void __iomem *mmio,
 			     unsigned int port_no)
 {
@@ -2240,7 +2395,7 @@ static int mv_hardreset(struct ata_link *link, unsigned int *class,
 {
 	struct ata_port *ap = link->ap;
 	struct mv_host_priv *hpriv = ap->host->private_data;
-	void __iomem *mmio = ap->host->iomap[MV_PRIMARY_BAR];
+	void __iomem *mmio = hpriv->base;
 
 	mv_stop_dma(ap);
 
@@ -2286,7 +2441,7 @@ static void mv_post_int_cmd(struct ata_queued_cmd *qc)
 
 static void mv_eh_freeze(struct ata_port *ap)
 {
-	void __iomem *mmio = ap->host->iomap[MV_PRIMARY_BAR];
+	struct mv_host_priv *hpriv = ap->host->private_data;
 	unsigned int hc = (ap->port_no > 3) ? 1 : 0;
 	u32 tmp, mask;
 	unsigned int shift;
@@ -2300,13 +2455,14 @@ static void mv_eh_freeze(struct ata_port *ap)
 	mask = 0x3 << shift;
 
 	/* disable assertion of portN err, done events */
-	tmp = readl(mmio + HC_MAIN_IRQ_MASK_OFS);
-	writelfl(tmp & ~mask, mmio + HC_MAIN_IRQ_MASK_OFS);
+	tmp = readl(hpriv->main_mask_reg_addr);
+	writelfl(tmp & ~mask, hpriv->main_mask_reg_addr);
 }
 
 static void mv_eh_thaw(struct ata_port *ap)
 {
-	void __iomem *mmio = ap->host->iomap[MV_PRIMARY_BAR];
+	struct mv_host_priv *hpriv = ap->host->private_data;
+	void __iomem *mmio = hpriv->base;
 	unsigned int hc = (ap->port_no > 3) ? 1 : 0;
 	void __iomem *hc_mmio = mv_hc_base(mmio, hc);
 	void __iomem *port_mmio = mv_ap_base(ap);
@@ -2333,8 +2489,8 @@ static void mv_eh_thaw(struct ata_port *ap)
 	writel(hc_irq_cause, hc_mmio + HC_IRQ_CAUSE_OFS);
 
 	/* enable assertion of portN err, done events */
-	tmp = readl(mmio + HC_MAIN_IRQ_MASK_OFS);
-	writelfl(tmp | mask, mmio + HC_MAIN_IRQ_MASK_OFS);
+	tmp = readl(hpriv->main_mask_reg_addr);
+	writelfl(tmp | mask, hpriv->main_mask_reg_addr);
 }
 
 /**
@@ -2471,9 +2627,13 @@ static int mv_chip_id(struct ata_host *host, unsigned int board_idx)
 			break;
 		}
 		break;
+	case chip_integrated:
+		hpriv->ops = &mv_integrated_ops;
+		hp_flags |= MV_HP_ERRATA_60X1C0;
+		break;
 
 	default:
-		dev_printk(KERN_ERR, &pdev->dev,
+		dev_printk(KERN_ERR, host->dev,
 			   "BUG: invalid board index %u\n", board_idx);
 		return 1;
 	}
@@ -2497,17 +2657,26 @@ static int mv_chip_id(struct ata_host *host, unsigned int board_idx)
 static int mv_init_host(struct ata_host *host, unsigned int board_idx)
 {
 	int rc = 0, n_hc, port, hc;
-	struct pci_dev *pdev = to_pci_dev(host->dev);
-	void __iomem *mmio = host->iomap[MV_PRIMARY_BAR];
 	struct mv_host_priv *hpriv = host->private_data;
-
-	/* global interrupt mask */
-	writel(0, mmio + HC_MAIN_IRQ_MASK_OFS);
+	void __iomem *mmio = hpriv->base;
 
 	rc = mv_chip_id(host, board_idx);
 	if (rc)
 		goto done;
 
+	if (HAS_PCI(host)) {
+		hpriv->main_cause_reg_addr = hpriv->base +
+		  HC_MAIN_IRQ_CAUSE_OFS;
+		hpriv->main_mask_reg_addr = hpriv->base + HC_MAIN_IRQ_MASK_OFS;
+	} else {
+		hpriv->main_cause_reg_addr = hpriv->base +
+		  HC_INTEGRATED_MAIN_IRQ_CAUSE_OFS;
+		hpriv->main_mask_reg_addr = hpriv->base +
+		  HC_INTEGRATED_MAIN_IRQ_MASK_OFS;
+	}
+	/* global interrupt mask */
+	writel(0, hpriv->main_mask_reg_addr);
+
 	n_hc = mv_get_hc_count(host->ports[0]->flags);
 
 	for (port = 0; port < host->n_ports; port++)
@@ -2518,7 +2687,7 @@ static int mv_init_host(struct ata_host *host, unsigned int board_idx)
 		goto done;
 
 	hpriv->ops->reset_flash(hpriv, mmio);
-	hpriv->ops->reset_bus(pdev, mmio);
+	hpriv->ops->reset_bus(host, mmio);
 	hpriv->ops->enable_leds(hpriv, mmio);
 
 	for (port = 0; port < host->n_ports; port++) {
@@ -2537,13 +2706,15 @@ static int mv_init_host(struct ata_host *host, unsigned int board_idx)
 	for (port = 0; port < host->n_ports; port++) {
 		struct ata_port *ap = host->ports[port];
 		void __iomem *port_mmio = mv_port_base(mmio, port);
-		unsigned int offset = port_mmio - mmio;
 
 		mv_port_init(&ap->ioaddr, port_mmio);
 
 #ifdef CONFIG_PCI
-		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, -1, "mmio");
-		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, offset, "port");
+		if (HAS_PCI(host)) {
+			unsigned int offset = port_mmio - mmio;
+			ata_port_pbar_desc(ap, MV_PRIMARY_BAR, -1, "mmio");
+			ata_port_pbar_desc(ap, MV_PRIMARY_BAR, offset, "port");
+		}
 #endif
 	}
 
@@ -2559,26 +2730,123 @@ static int mv_init_host(struct ata_host *host, unsigned int board_idx)
 		writelfl(0, hc_mmio + HC_IRQ_CAUSE_OFS);
 	}
 
-	/* Clear any currently outstanding host interrupt conditions */
-	writelfl(0, mmio + PCI_IRQ_CAUSE_OFS);
+	if (HAS_PCI(host)) {
+		/* Clear any currently outstanding host interrupt conditions */
+		writelfl(0, mmio + PCI_IRQ_CAUSE_OFS);
 
-	/* and unmask interrupt generation for host regs */
-	writelfl(PCI_UNMASK_ALL_IRQS, mmio + PCI_IRQ_MASK_OFS);
+		/* and unmask interrupt generation for host regs */
+		writelfl(PCI_UNMASK_ALL_IRQS, mmio + PCI_IRQ_MASK_OFS);
+		if (IS_GEN_I(hpriv))
+			writelfl(~HC_MAIN_MASKED_IRQS_5,
+				 hpriv->main_mask_reg_addr);
+		else
+			writelfl(~HC_MAIN_MASKED_IRQS,
+				 hpriv->main_mask_reg_addr);
+
+		VPRINTK("HC MAIN IRQ cause/mask=0x%08x/0x%08x "
+			"PCI int cause/mask=0x%08x/0x%08x\n",
+			readl(hpriv->main_cause_reg_addr),
+			readl(hpriv->main_mask_reg_addr),
+			readl(mmio + PCI_IRQ_CAUSE_OFS),
+			readl(mmio + PCI_IRQ_MASK_OFS));
+	} else {
+		writelfl(~HC_MAIN_MASKED_IRQS_INTEGRATED,
+			 hpriv->main_mask_reg_addr);
+		VPRINTK("HC MAIN IRQ cause/mask=0x%08x/0x%08x\n",
+			readl(hpriv->main_cause_reg_addr),
+			readl(hpriv->main_mask_reg_addr));
+	}
+done:
 
-	if (IS_GEN_I(hpriv))
-		writelfl(~HC_MAIN_MASKED_IRQS_5, mmio + HC_MAIN_IRQ_MASK_OFS);
-	else
-		writelfl(~HC_MAIN_MASKED_IRQS, mmio + HC_MAIN_IRQ_MASK_OFS);
+	return rc;
+}
 
-	VPRINTK("HC MAIN IRQ cause/mask=0x%08x/0x%08x "
-		"PCI int cause/mask=0x%08x/0x%08x\n",
-		readl(mmio + HC_MAIN_IRQ_CAUSE_OFS),
-		readl(mmio + HC_MAIN_IRQ_MASK_OFS),
-		readl(mmio + PCI_IRQ_CAUSE_OFS),
-		readl(mmio + PCI_IRQ_MASK_OFS));
+/**
+ *      mv_platform_probe - handle a positive probe of an integrated Marvell
+ *      host
+ *      @pdev: platform device found
+ *
+ *      LOCKING:
+ *      Inherited from caller.
+ */
+static int mv_platform_probe(struct platform_device *pdev)
+{
+	static int printed_version;
+	const struct mv_sata_platform_data *mv_platform_data;
+	const struct ata_port_info *ppi[] =
+	    { &mv_port_info[chip_integrated], NULL };
+	struct ata_host *host;
+	struct mv_host_priv *hpriv;
+	struct resource *res;
+	int n_ports, rc;
 
-done:
-	return rc;
+	if (!printed_version++)
+		dev_printk(KERN_INFO, &pdev->dev, "version " DRV_VERSION "\n");
+
+	/*
+	 * Simple resource validation ..
+	 */
+	if (unlikely(pdev->num_resources != 2)) {
+		dev_err(&pdev->dev, "invalid number of resources\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Get the register base first
+	 */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (res == NULL)
+		return -EINVAL;
+
+	/* allocate host */
+	mv_platform_data = pdev->dev.platform_data;
+	n_ports = mv_platform_data->n_ports;
+
+	host = ata_host_alloc_pinfo(&pdev->dev, ppi, n_ports);
+	hpriv = devm_kzalloc(&pdev->dev, sizeof(*hpriv), GFP_KERNEL);
+
+	if (!host || !hpriv)
+		return -ENOMEM;
+	host->private_data = hpriv;
+	hpriv->n_ports = n_ports;
+
+	host->iomap = NULL;
+	hpriv->base = ioremap(res->start, res->end - res->start + 1);
+	hpriv->base -= MV_SATAHC0_REG_BASE;
+
+	pdev->dev.coherent_dma_mask = DMA_32BIT_MASK;
+
+	/* initialize adapter */
+	rc = mv_init_host(host, chip_integrated);
+	if (rc)
+		return rc;
+
+	dev_printk(KERN_INFO, &pdev->dev,
+		   "slots %u ports %d\n", (unsigned)MV_MAX_Q_DEPTH,
+		   host->n_ports);
+
+	return ata_host_activate(host, platform_get_irq(pdev, 0), mv_interrupt,
+				 IRQF_SHARED, &mv6_sht);
+}
+
+/*
+ *
+ *      mv_platform_remove    -       unplug a platform interface
+ *      @pdev: platform device
+ *
+ *      A platform bus SATA device has been unplugged. Perform the needed
+ *      cleanup. Also called on module unload for any active devices.
+ */
+static int __devexit mv_platform_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct mv_host_priv *hpriv = host->private_data;
+	void __iomem *base = hpriv->base;
+
+	ata_host_detach(host);
+	iounmap(base);
+	return 0;
 }
 
 #ifdef CONFIG_PCI
@@ -2665,14 +2933,15 @@ static void mv_print_info(struct ata_host *host)
 }
 
 /**
- *      mv_init_one - handle a positive probe of a Marvell host
+ *      mv_pci_init_one - handle a positive probe of a PCI Marvell host
  *      @pdev: PCI device found
  *      @ent: PCI device ID entry for the matched host
  *
  *      LOCKING:
  *      Inherited from caller.
  */
-static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
+static int mv_pci_init_one(struct pci_dev *pdev,
+			   const struct pci_device_id *ent)
 {
 	static int printed_version;
 	unsigned int board_idx = (unsigned int)ent->driver_data;
@@ -2692,6 +2961,7 @@ static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (!host || !hpriv)
 		return -ENOMEM;
 	host->private_data = hpriv;
+	hpriv->n_ports = n_ports;
 
 	/* acquire resources */
 	rc = pcim_enable_device(pdev);
@@ -2704,6 +2974,7 @@ static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (rc)
 		return rc;
 	host->iomap = pcim_iomap_table(pdev);
+	hpriv->base = host->iomap[MV_PRIMARY_BAR];
 
 	rc = pci_go_64(pdev);
 	if (rc)
@@ -2732,7 +3003,7 @@ static int __init mv_pci_register_driver(void)
 	return pci_register_driver(&mv_pci_driver);
 }
 
-static void __exit mv_pci_unregister_driver(void)
+static void mv_pci_unregister_driver(void)
 {
 	pci_unregister_driver(&mv_pci_driver);
 }
@@ -2748,12 +3019,23 @@ static void __exit mv_pci_unregister_driver(void)
 #endif
 static int __init mv_init(void)
 {
-	return mv_pci_register_driver();
+	int retval = 0;
+
+	retval = mv_pci_register_driver();
+	if (retval < 0)
+		return retval;
+
+	retval = platform_driver_register(&mv_platform_driver);
+	if (retval < 0)
+		mv_pci_unregister_driver();
+
+	return retval;
 }
 
 static void __exit mv_exit(void)
 {
 	mv_pci_unregister_driver();
+	platform_driver_unregister(&mv_platform_driver);
 }
 
 MODULE_AUTHOR("Brett Russ");
diff --git a/include/linux/sata_mv.h b/include/linux/sata_mv.h
new file mode 100644
index 0000000..b0fa7cd
--- /dev/null
+++ b/include/linux/sata_mv.h
@@ -0,0 +1,21 @@
+/*
+ * Marvell integrated SATA platfrom device data definition file.
+ *
+ * Saeed Bishara <saeed@marvell.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef __LINUX_SATA_MV_H__
+#define __LINUX_SATA_MV_H__
+
+/*
+ * Sata private data
+ */
+struct mv_sata_platform_data {
+	int	n_ports; /* number of sata ports */
+};
+
+#endif
-- 
1.5.0.6

Re: [PATCH 2/2] [libata] sata_mv: Support integrated controllers

[Jeff Garzik]

saeed.bishara@gmail.com wrote:
> From: Saeed Bishara <saeed@marvell.com>
> 
> Marvell's Orion SoC includes SATA controllers based on Marvell's
> PCI-to-SATA 88SX controllers. The integrated SATA unit is connected
> directly to the internal bus of the Orion SoC, and not via PCI.
> This patch extends the libATA sata_mv driver to support those
> controllers.
> 
> Signed-off-by: Saeed Bishara <saeed@marvell.com>

Overall, looks pretty good.

My main comment is that several changes, like

-	void (*reset_bus)(struct pci_dev *pdev, void __iomem *mmio);
+	void (*reset_bus)(struct ata_host *host, void __iomem *mmio);

and

-	if (unlikely(irq_stat & PCI_ERR)) {
+	if (unlikely(irq_stat & PCI_ERR) && HAS_PCI(host)) {

belong in patch #1, the PCI isolation patch, rather than this patch 
implementing support for integrated controllers.

Finally, don't hardcode a 32-bit DMA mask in your platform_device setup, 
pass that mask in via mv_sata_platform_data.

	Jeff

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[Jeff Garzik]

saeed.bishara@gmail.com wrote:
> From: Saeed Bishara <saeed@marvell.com>
> 
> The integrated SATA controller is connected directly to the SoC's
> internal bus, not via PCI interface. this patch removes the dependency
> on the PCI interface.
> 
> Signed-off-by: Saeed Bishara <saeed@marvell.com>
> ---
>  drivers/ata/Kconfig   |    2 +-
>  drivers/ata/sata_mv.c |  113 ++++++++++++++++++++++++++++++-------------------
>  2 files changed, 71 insertions(+), 44 deletions(-)


Overall, a good patch, though some minor revisions are needed (comments 
inline).



> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index ba63619..c60842b 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -69,7 +69,7 @@ config ATA_PIIX
>  
>  config SATA_MV
>  	tristate "Marvell SATA support (HIGHLY EXPERIMENTAL)"
> -	depends on PCI && EXPERIMENTAL
> +	depends on EXPERIMENTAL
>  	help
>  	  This option enables support for the Marvell Serial ATA family.
>  	  Currently supports 88SX[56]0[48][01] chips.
> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> index 97c3e11..cbd3f1b 100644
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
> @@ -421,7 +421,9 @@ static void mv_error_handler(struct ata_port *ap);
>  static void mv_post_int_cmd(struct ata_queued_cmd *qc);
>  static void mv_eh_freeze(struct ata_port *ap);
>  static void mv_eh_thaw(struct ata_port *ap);
> +#ifdef CONFIG_PCI
>  static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
> +#endif

with the proper ordering (see comments below), you can remove this 
prototype altogether


>  static void mv5_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
>  			   unsigned int port);
> @@ -638,14 +640,14 @@ static const struct pci_device_id mv_pci_tbl[] = {
>  
>  	{ }			/* terminate list */
>  };
> -
> +#ifdef CONFIG_PCI
>  static struct pci_driver mv_pci_driver = {
>  	.name			= DRV_NAME,
>  	.id_table		= mv_pci_tbl,
>  	.probe			= mv_init_one,
>  	.remove			= ata_pci_remove_one,
>  };
> -
> +#endif

I would prefer that you move this, and the pci_device_id table, down to 
the bottom of the file with the rest of the #CONFIG_PCI section of code 
that you have nicely arranged.


> @@ -2616,6 +2581,47 @@ done:
>  	return rc;
>  }
>  
> +#ifdef CONFIG_PCI
> +
> +/*
> + * module options
> + */
> +static int msi;	      /* Use PCI msi; either zero (off, default) or non-zero */
> +
> +
> +/* move to PCI layer or libata core? */
> +static int pci_go_64(struct pci_dev *pdev)
> +{
> +	int rc;
> +
> +	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK)) {
> +		rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
> +		if (rc) {
> +			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
> +			if (rc) {
> +				dev_printk(KERN_ERR, &pdev->dev,
> +					   "64-bit DMA enable failed\n");
> +				return rc;
> +			}
> +		}
> +	} else {
> +		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
> +		if (rc) {
> +			dev_printk(KERN_ERR, &pdev->dev,
> +				   "32-bit DMA enable failed\n");
> +			return rc;
> +		}
> +		rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
> +		if (rc) {
> +			dev_printk(KERN_ERR, &pdev->dev,
> +				   "32-bit consistent DMA enable failed\n");
> +			return rc;
> +		}
> +	}
> +
> +	return rc;
> +}
> +
>  /**
>   *      mv_print_info - Dump key info to kernel log for perusal.
>   *      @host: ATA host to print info about
> @@ -2721,15 +2727,34 @@ static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  				 IS_GEN_I(hpriv) ? &mv5_sht : &mv6_sht);
>  }
>  

nicely done, this [once my comments are taken] arranges everything into 
a single PCI-specific area.


> -static int __init mv_init(void)
> +static int __init mv_pci_register_driver(void)
>  {
>  	return pci_register_driver(&mv_pci_driver);
>  }
>  
> -static void __exit mv_exit(void)
> +static void __exit mv_pci_unregister_driver(void)
>  {
>  	pci_unregister_driver(&mv_pci_driver);
>  }
> +#else
> +static int __init mv_pci_register_driver(void)
> +{
> +	return 0;
> +}
> +
> +static void __exit mv_pci_unregister_driver(void)
> +{
> +}
> +#endif
> +static int __init mv_init(void)
> +{
> +	return mv_pci_register_driver();
> +}
> +
> +static void __exit mv_exit(void)
> +{
> +	mv_pci_unregister_driver();
> +}

The way we do multi-bus drivers that you do something like

static int mv_init(void)
{
	#ifdef CONFIG_PCI
	rc = pci_register_driver(&mv_driver);
	if (rc)
		return rc;
	#endif

	#ifdef MY_MARVELL_SOC_EMBEDDED_CHIP
	rc = my_soc_register_platform_driver(&platform_driver);
	if (rc)
		goto err_out;
	#endif

	return 0;

err_out:
#ifdef CONFIG_PCI
	pci_unregister_driver(&mv_driver);
#endif
	return rc;
}

The module unregister function mv_exit() should do something similar, in 
reverse order.

Even embedded SoC controllers need to register a driver (usually a 
platform_driver with platform_devices).



>  MODULE_AUTHOR("Brett Russ");
>  MODULE_DESCRIPTION("SCSI low-level driver for Marvell SATA controllers");
> @@ -2737,8 +2762,10 @@ MODULE_LICENSE("GPL");
>  MODULE_DEVICE_TABLE(pci, mv_pci_tbl);
>  MODULE_VERSION(DRV_VERSION);
>  
> +#ifdef CONFIG_PCI
>  module_param(msi, int, 0444);
>  MODULE_PARM_DESC(msi, "Enable use of PCI MSI (0=off, 1=on)");
> +#endif

agreed -- in contrast with my comments above, please keep this module 
option right where it is, and add the #ifdef CONFIG_PCI precisely as you 
have done.  do /not/ move it into the CONFIG_PCI section referred to in 
my other comments.

	Jeff

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[Jeff Garzik]

saeed.bishara@gmail.com wrote:
> From: Saeed Bishara <saeed@marvell.com>
> 
> The integrated SATA controller is connected directly to the SoC's
> internal bus, not via PCI interface. this patch removes the dependency
> on the PCI interface.
> 
> Signed-off-by: Saeed Bishara <saeed@marvell.com>
> ---
>  drivers/ata/Kconfig   |    2 +-
>  drivers/ata/sata_mv.c |  113 ++++++++++++++++++++++++++++++-------------------
>  2 files changed, 71 insertions(+), 44 deletions(-)

oh, since Mark Lord is also touching the sata_mv driver, it would be a 
courtesy to CC him ...

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[saeed bishara]

>
>
Here is the updated patch according your comments:
>From af124a0fd4984db4fbc8638e0af4e6a41561588d Mon Sep 17 00:00:00 2001
From: Saeed Bishara <saeed@marvell.com>
Date: Sun, 2 Dec 2007 10:43:10 +0200
Subject: [PATCH] sata_mv: Remove PCI dependency

The integrated SATA controller is connected directly to the SoC's
internal bus, not via PCI interface. this patch removes the dependency
on the PCI interface.

Signed-off-by: Saeed Bishara <saeed@marvell.com>
---
 drivers/ata/Kconfig   |    2 +-
 drivers/ata/sata_mv.c |  133 ++++++++++++++++++++++++++++---------------------
 2 files changed, 77 insertions(+), 58 deletions(-)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index ba63619..c60842b 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -69,7 +69,7 @@ config ATA_PIIX

 config SATA_MV
 	tristate "Marvell SATA support (HIGHLY EXPERIMENTAL)"
-	depends on PCI && EXPERIMENTAL
+	depends on EXPERIMENTAL
 	help
 	  This option enables support for the Marvell Serial ATA family.
 	  Currently supports 88SX[56]0[48][01] chips.
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index a43f64d..4b94aeb 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -125,6 +125,9 @@ enum {
 	/* Host Flags */
 	MV_FLAG_DUAL_HC		= (1 << 30),  /* two SATA Host Controllers */
 	MV_FLAG_IRQ_COALESCE	= (1 << 29),  /* IRQ coalescing capability */
+	/* SoC integrated controllers, no PCI interface */
+	MV_FLAG_SOC = (1 << 28),
+
 	MV_COMMON_FLAGS		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO | ATA_FLAG_NO_ATAPI |
 				  ATA_FLAG_PIO_POLLING,
@@ -312,6 +315,7 @@ enum {
 #define IS_GEN_I(hpriv) ((hpriv)->hp_flags & MV_HP_GEN_I)
 #define IS_GEN_II(hpriv) ((hpriv)->hp_flags & MV_HP_GEN_II)
 #define IS_GEN_IIE(hpriv) ((hpriv)->hp_flags & MV_HP_GEN_IIE)
+#define HAS_PCI(host) (!((host)->ports[0]->flags & MV_FLAG_SOC))

 enum {
 	/* DMA boundary 0xffff is required by the s/g splitting
@@ -398,7 +402,7 @@ struct mv_hw_ops {
 	int (*reset_hc)(struct mv_host_priv *hpriv, void __iomem *mmio,
 			unsigned int n_hc);
 	void (*reset_flash)(struct mv_host_priv *hpriv, void __iomem *mmio);
-	void (*reset_bus)(struct pci_dev *pdev, void __iomem *mmio);
+	void (*reset_bus)(struct ata_host *host, void __iomem *mmio);
 };

 struct mv_host_priv {
@@ -421,7 +425,6 @@ static void mv_error_handler(struct ata_port *ap);
 static void mv_post_int_cmd(struct ata_queued_cmd *qc);
 static void mv_eh_freeze(struct ata_port *ap);
 static void mv_eh_thaw(struct ata_port *ap);
-static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);

 static void mv5_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
 			   unsigned int port);
@@ -431,7 +434,7 @@ static void mv5_read_preamp(struct mv_host_priv
*hpriv, int idx,
 static int mv5_reset_hc(struct mv_host_priv *hpriv, void __iomem *mmio,
 			unsigned int n_hc);
 static void mv5_reset_flash(struct mv_host_priv *hpriv, void __iomem *mmio);
-static void mv5_reset_bus(struct pci_dev *pdev, void __iomem *mmio);
+static void mv5_reset_bus(struct ata_host *host, void __iomem *mmio);

 static void mv6_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
 			   unsigned int port);
@@ -441,7 +444,7 @@ static void mv6_read_preamp(struct mv_host_priv
*hpriv, int idx,
 static int mv6_reset_hc(struct mv_host_priv *hpriv, void __iomem *mmio,
 			unsigned int n_hc);
 static void mv6_reset_flash(struct mv_host_priv *hpriv, void __iomem *mmio);
-static void mv_reset_pci_bus(struct pci_dev *pdev, void __iomem *mmio);
+static void mv_reset_pci_bus(struct ata_host *host, void __iomem *mmio);
 static void mv_channel_reset(struct mv_host_priv *hpriv, void __iomem *mmio,
 			     unsigned int port_no);

@@ -639,13 +642,6 @@ static const struct pci_device_id mv_pci_tbl[] = {
 	{ }			/* terminate list */
 };

-static struct pci_driver mv_pci_driver = {
-	.name			= DRV_NAME,
-	.id_table		= mv_pci_tbl,
-	.probe			= mv_init_one,
-	.remove			= ata_pci_remove_one,
-};
-
 static const struct mv_hw_ops mv5xxx_ops = {
 	.phy_errata		= mv5_phy_errata,
 	.enable_leds		= mv5_enable_leds,
@@ -665,45 +661,6 @@ static const struct mv_hw_ops mv6xxx_ops = {
 };

 /*
- * module options
- */
-static int msi;	      /* Use PCI msi; either zero (off, default) or non-zero */
-
-
-/* move to PCI layer or libata core? */
-static int pci_go_64(struct pci_dev *pdev)
-{
-	int rc;
-
-	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK)) {
-		rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
-		if (rc) {
-			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
-			if (rc) {
-				dev_printk(KERN_ERR, &pdev->dev,
-					   "64-bit DMA enable failed\n");
-				return rc;
-			}
-		}
-	} else {
-		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
-		if (rc) {
-			dev_printk(KERN_ERR, &pdev->dev,
-				   "32-bit DMA enable failed\n");
-			return rc;
-		}
-		rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
-		if (rc) {
-			dev_printk(KERN_ERR, &pdev->dev,
-				   "32-bit consistent DMA enable failed\n");
-			return rc;
-		}
-	}
-
-	return rc;
-}
-
-/*
  * Functions
  */

@@ -1717,7 +1674,7 @@ static irqreturn_t mv_interrupt(int irq, void
*dev_instance)
 	n_hcs = mv_get_hc_count(host->ports[0]->flags);
 	spin_lock(&host->lock);

-	if (unlikely(irq_stat & PCI_ERR)) {
+	if (unlikely(irq_stat & PCI_ERR) && HAS_PCI(host)) {
 		mv_pci_error(host, mmio);
 		handled = 1;
 		goto out_unlock;	/* skip all other HC irq handling */
@@ -1788,8 +1745,9 @@ static int mv5_scr_write(struct ata_port *ap,
unsigned int sc_reg_in, u32 val)
 		return -EINVAL;
 }

-static void mv5_reset_bus(struct pci_dev *pdev, void __iomem *mmio)
+static void mv5_reset_bus(struct ata_host *host, void __iomem *mmio)
 {
+	struct pci_dev *pdev = to_pci_dev(host->dev);
 	int early_5080;

 	early_5080 = (pdev->device == 0x5080) && (pdev->revision == 0);
@@ -1800,7 +1758,7 @@ static void mv5_reset_bus(struct pci_dev *pdev,
void __iomem *mmio)
 		writel(tmp, mmio + MV_PCI_EXP_ROM_BAR_CTL);
 	}

-	mv_reset_pci_bus(pdev, mmio);
+	mv_reset_pci_bus(host, mmio);
 }

 static void mv5_reset_flash(struct mv_host_priv *hpriv, void __iomem *mmio)
@@ -1924,7 +1882,7 @@ static int mv5_reset_hc(struct mv_host_priv
*hpriv, void __iomem *mmio,

 #undef ZERO
 #define ZERO(reg) writel(0, mmio + (reg))
-static void mv_reset_pci_bus(struct pci_dev *pdev, void __iomem *mmio)
+static void mv_reset_pci_bus(struct ata_host *host, void __iomem *mmio)
 {
 	u32 tmp;

@@ -2534,7 +2492,6 @@ static int mv_chip_id(struct ata_host *host,
unsigned int board_idx)
 static int mv_init_host(struct ata_host *host, unsigned int board_idx)
 {
 	int rc = 0, n_hc, port, hc;
-	struct pci_dev *pdev = to_pci_dev(host->dev);
 	void __iomem *mmio = host->iomap[MV_PRIMARY_BAR];
 	struct mv_host_priv *hpriv = host->private_data;

@@ -2555,7 +2512,7 @@ static int mv_init_host(struct ata_host *host,
unsigned int board_idx)
 		goto done;

 	hpriv->ops->reset_flash(hpriv, mmio);
-	hpriv->ops->reset_bus(pdev, mmio);
+	hpriv->ops->reset_bus(host, mmio);
 	hpriv->ops->enable_leds(hpriv, mmio);

 	for (port = 0; port < host->n_ports; port++) {
@@ -2578,8 +2535,10 @@ static int mv_init_host(struct ata_host *host,
unsigned int board_idx)

 		mv_port_init(&ap->ioaddr, port_mmio);

+#ifdef CONFIG_PCI
 		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, -1, "mmio");
 		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, offset, "port");
+#endif
 	}

 	for (hc = 0; hc < n_hc; hc++) {
@@ -2616,6 +2575,55 @@ done:
 	return rc;
 }

+#ifdef CONFIG_PCI
+static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
+
+static struct pci_driver mv_pci_driver = {
+	.name			= DRV_NAME,
+	.id_table		= mv_pci_tbl,
+	.probe			= mv_init_one,
+	.remove			= ata_pci_remove_one,
+};
+
+/*
+ * module options
+ */
+static int msi;	      /* Use PCI msi; either zero (off, default) or non-zero */
+
+
+/* move to PCI layer or libata core? */
+static int pci_go_64(struct pci_dev *pdev)
+{
+	int rc;
+
+	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK)) {
+		rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
+		if (rc) {
+			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
+			if (rc) {
+				dev_printk(KERN_ERR, &pdev->dev,
+					   "64-bit DMA enable failed\n");
+				return rc;
+			}
+		}
+	} else {
+		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
+		if (rc) {
+			dev_printk(KERN_ERR, &pdev->dev,
+				   "32-bit DMA enable failed\n");
+			return rc;
+		}
+		rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
+		if (rc) {
+			dev_printk(KERN_ERR, &pdev->dev,
+				   "32-bit consistent DMA enable failed\n");
+			return rc;
+		}
+	}
+
+	return rc;
+}
+
 /**
  *      mv_print_info - Dump key info to kernel log for perusal.
  *      @host: ATA host to print info about
@@ -2720,15 +2728,24 @@ static int mv_init_one(struct pci_dev *pdev,
const struct pci_device_id *ent)
 	return ata_host_activate(host, pdev->irq, mv_interrupt, IRQF_SHARED,
 				 IS_GEN_I(hpriv) ? &mv5_sht : &mv6_sht);
 }
+#endif

 static int __init mv_init(void)
 {
-	return pci_register_driver(&mv_pci_driver);
+	int rc;
+#ifdef CONFIG_PCI
+	rc = pci_register_driver(&mv_pci_driver);
+	if (rc)
+		return rc;
+#endif
+	return 0;
 }

 static void __exit mv_exit(void)
 {
+#ifdef CONFIG_PCI
 	pci_unregister_driver(&mv_pci_driver);
+#endif
 }

 MODULE_AUTHOR("Brett Russ");
@@ -2737,8 +2754,10 @@ MODULE_LICENSE("GPL");
 MODULE_DEVICE_TABLE(pci, mv_pci_tbl);
 MODULE_VERSION(DRV_VERSION);

+#ifdef CONFIG_PCI
 module_param(msi, int, 0444);
 MODULE_PARM_DESC(msi, "Enable use of PCI MSI (0=off, 1=on)");
+#endif

 module_init(mv_init);
 module_exit(mv_exit);
-- 
1.5.0.6

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[saeed bishara]

> Here is the updated patch according your comments:
> From af124a0fd4984db4fbc8638e0af4e6a41561588d Mon Sep 17 00:00:00 2001
> From: Saeed Bishara <saeed@marvell.com>
> Date: Sun, 2 Dec 2007 10:43:10 +0200
> Subject: [PATCH] sata_mv: Remove PCI dependency
>
> The integrated SATA controller is connected directly to the SoC's
> internal bus, not via PCI interface. this patch removes the dependency
> on the PCI interface.
guys, any other comments for this patch?

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[Jeff Garzik]

saeed bishara wrote:
>> Here is the updated patch according your comments:
>> From af124a0fd4984db4fbc8638e0af4e6a41561588d Mon Sep 17 00:00:00 2001
>> From: Saeed Bishara <saeed@marvell.com>
>> Date: Sun, 2 Dec 2007 10:43:10 +0200
>> Subject: [PATCH] sata_mv: Remove PCI dependency
>>
>> The integrated SATA controller is connected directly to the SoC's
>> internal bus, not via PCI interface. this patch removes the dependency
>> on the PCI interface.
> guys, any other comments for this patch?

sorry, I've been away dealing with some family matters.  So, the "patch 
queue and review" stuff on my side hasn't moved during the holidays. 
Should get going again now...

	Jeff

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[Jeff Garzik]

saeed bishara wrote:
> -	if (unlikely(irq_stat & PCI_ERR)) {
> +	if (unlikely(irq_stat & PCI_ERR) && HAS_PCI(host)) {
>  		mv_pci_error(host, mmio);
>  		handled = 1;
>  		goto out_unlock;	/* skip all other HC irq handling */

the unlikely() should cover the entire expression.



>  static int __init mv_init(void)
>  {
> -	return pci_register_driver(&mv_pci_driver);
> +	int rc;
> +#ifdef CONFIG_PCI
> +	rc = pci_register_driver(&mv_pci_driver);
> +	if (rc)
> +		return rc;
> +#endif
> +	return 0;
>  }

I would do

{
	int rc = -ENODEV;

	#ifdef CONFIG_PCI
	rc = pci_register_driver(...);
	#endif

	return rc;
}

to ensure sane non-SoC, non-PCI behavior (which this patch now enables).

Finally, in Kconfig, even when removing the CONFIG_PCI dependency, the 
driver suddenly has other unsatified dependencies:  CONFIG_HAS_DMA and 
CONFIG_HAS_IOMEM.  Those two need to be added to the Kconfig dep list.

I would have made these minor corrections myself, but git-am (main 
kernel patch-apply tool, for git users) doesn't seem to like the patch:

Applying sata_mv: Remove PCI dependency

fatal: corrupt patch at line 59

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[saeed bishara]

[-- Attachment #1: Type: text/plain, Size: 1400 bytes --]

On 1/16/08, Jeff Garzik <jeff@garzik.org> wrote:
> saeed bishara wrote:
> > -	if (unlikely(irq_stat & PCI_ERR)) {
> > +	if (unlikely(irq_stat & PCI_ERR) && HAS_PCI(host)) {
> >  		mv_pci_error(host, mmio);
> >  		handled = 1;
> >  		goto out_unlock;	/* skip all other HC irq handling */
>
> the unlikely() should cover the entire expression.
this will be applied into the new patch
>
>
>
> >  static int __init mv_init(void)
> >  {
> > -	return pci_register_driver(&mv_pci_driver);
> > +	int rc;
> > +#ifdef CONFIG_PCI
> > +	rc = pci_register_driver(&mv_pci_driver);
> > +	if (rc)
> > +		return rc;
> > +#endif
> > +	return 0;
> >  }
>
> I would do
>
> {
> 	int rc = -ENODEV;
>
> 	#ifdef CONFIG_PCI
> 	rc = pci_register_driver(...);
> 	#endif
>
> 	return rc;
> }
ditto
>
> to ensure sane non-SoC, non-PCI behavior (which this patch now enables).
>
> Finally, in Kconfig, even when removing the CONFIG_PCI dependency, the
> driver suddenly has other unsatified dependencies:  CONFIG_HAS_DMA and
> CONFIG_HAS_IOMEM.  Those two need to be added to the Kconfig dep list.
but ATA is already depends on HAS_IOMEM! so it's not needed by the
driver's entry.
>
> I would have made these minor corrections myself, but git-am (main
> kernel patch-apply tool, for git users) doesn't seem to like the patch:
>
> Applying sata_mv: Remove PCI dependency
>
> fatal: corrupt patch at line 59
I attached the new patch

[-- Attachment #2: 0001-sata_mv-Remove-PCI-dependency.patch --]
[-- Type: application/octet-stream, Size: 9544 bytes --]

From c5f5af7f2eed2fe42e28374257b983922dfd3db5 Mon Sep 17 00:00:00 2001
From: Saeed Bishara <saeed@marvell.com>
Date: Sun, 2 Dec 2007 10:43:10 +0200
Subject: [PATCH] sata_mv: Remove PCI dependency

The integrated SATA controller is connected directly to the SoC's
internal bus, not via PCI interface. this patch removes the dependency
on the PCI interface.

Signed-off-by: Saeed Bishara <saeed@marvell.com>
---
 drivers/ata/Kconfig   |    2 +-
 drivers/ata/sata_mv.c |  131 +++++++++++++++++++++++++++---------------------
 2 files changed, 75 insertions(+), 58 deletions(-)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index ba63619..c60842b 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -69,7 +69,7 @@ config ATA_PIIX
 
 config SATA_MV
 	tristate "Marvell SATA support (HIGHLY EXPERIMENTAL)"
-	depends on PCI && EXPERIMENTAL
+	depends on EXPERIMENTAL
 	help
 	  This option enables support for the Marvell Serial ATA family.
 	  Currently supports 88SX[56]0[48][01] chips.
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index a43f64d..9b0d590 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -125,6 +125,9 @@ enum {
 	/* Host Flags */
 	MV_FLAG_DUAL_HC		= (1 << 30),  /* two SATA Host Controllers */
 	MV_FLAG_IRQ_COALESCE	= (1 << 29),  /* IRQ coalescing capability */
+	/* SoC integrated controllers, no PCI interface */
+	MV_FLAG_SOC = (1 << 28),
+
 	MV_COMMON_FLAGS		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO | ATA_FLAG_NO_ATAPI |
 				  ATA_FLAG_PIO_POLLING,
@@ -312,6 +315,7 @@ enum {
 #define IS_GEN_I(hpriv) ((hpriv)->hp_flags & MV_HP_GEN_I)
 #define IS_GEN_II(hpriv) ((hpriv)->hp_flags & MV_HP_GEN_II)
 #define IS_GEN_IIE(hpriv) ((hpriv)->hp_flags & MV_HP_GEN_IIE)
+#define HAS_PCI(host) (!((host)->ports[0]->flags & MV_FLAG_SOC))
 
 enum {
 	/* DMA boundary 0xffff is required by the s/g splitting
@@ -398,7 +402,7 @@ struct mv_hw_ops {
 	int (*reset_hc)(struct mv_host_priv *hpriv, void __iomem *mmio,
 			unsigned int n_hc);
 	void (*reset_flash)(struct mv_host_priv *hpriv, void __iomem *mmio);
-	void (*reset_bus)(struct pci_dev *pdev, void __iomem *mmio);
+	void (*reset_bus)(struct ata_host *host, void __iomem *mmio);
 };
 
 struct mv_host_priv {
@@ -421,7 +425,6 @@ static void mv_error_handler(struct ata_port *ap);
 static void mv_post_int_cmd(struct ata_queued_cmd *qc);
 static void mv_eh_freeze(struct ata_port *ap);
 static void mv_eh_thaw(struct ata_port *ap);
-static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
 
 static void mv5_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
 			   unsigned int port);
@@ -431,7 +434,7 @@ static void mv5_read_preamp(struct mv_host_priv *hpriv, int idx,
 static int mv5_reset_hc(struct mv_host_priv *hpriv, void __iomem *mmio,
 			unsigned int n_hc);
 static void mv5_reset_flash(struct mv_host_priv *hpriv, void __iomem *mmio);
-static void mv5_reset_bus(struct pci_dev *pdev, void __iomem *mmio);
+static void mv5_reset_bus(struct ata_host *host, void __iomem *mmio);
 
 static void mv6_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
 			   unsigned int port);
@@ -441,7 +444,7 @@ static void mv6_read_preamp(struct mv_host_priv *hpriv, int idx,
 static int mv6_reset_hc(struct mv_host_priv *hpriv, void __iomem *mmio,
 			unsigned int n_hc);
 static void mv6_reset_flash(struct mv_host_priv *hpriv, void __iomem *mmio);
-static void mv_reset_pci_bus(struct pci_dev *pdev, void __iomem *mmio);
+static void mv_reset_pci_bus(struct ata_host *host, void __iomem *mmio);
 static void mv_channel_reset(struct mv_host_priv *hpriv, void __iomem *mmio,
 			     unsigned int port_no);
 
@@ -639,13 +642,6 @@ static const struct pci_device_id mv_pci_tbl[] = {
 	{ }			/* terminate list */
 };
 
-static struct pci_driver mv_pci_driver = {
-	.name			= DRV_NAME,
-	.id_table		= mv_pci_tbl,
-	.probe			= mv_init_one,
-	.remove			= ata_pci_remove_one,
-};
-
 static const struct mv_hw_ops mv5xxx_ops = {
 	.phy_errata		= mv5_phy_errata,
 	.enable_leds		= mv5_enable_leds,
@@ -665,45 +661,6 @@ static const struct mv_hw_ops mv6xxx_ops = {
 };
 
 /*
- * module options
- */
-static int msi;	      /* Use PCI msi; either zero (off, default) or non-zero */
-
-
-/* move to PCI layer or libata core? */
-static int pci_go_64(struct pci_dev *pdev)
-{
-	int rc;
-
-	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK)) {
-		rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
-		if (rc) {
-			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
-			if (rc) {
-				dev_printk(KERN_ERR, &pdev->dev,
-					   "64-bit DMA enable failed\n");
-				return rc;
-			}
-		}
-	} else {
-		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
-		if (rc) {
-			dev_printk(KERN_ERR, &pdev->dev,
-				   "32-bit DMA enable failed\n");
-			return rc;
-		}
-		rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
-		if (rc) {
-			dev_printk(KERN_ERR, &pdev->dev,
-				   "32-bit consistent DMA enable failed\n");
-			return rc;
-		}
-	}
-
-	return rc;
-}
-
-/*
  * Functions
  */
 
@@ -1717,7 +1674,7 @@ static irqreturn_t mv_interrupt(int irq, void *dev_instance)
 	n_hcs = mv_get_hc_count(host->ports[0]->flags);
 	spin_lock(&host->lock);
 
-	if (unlikely(irq_stat & PCI_ERR)) {
+	if (unlikely((irq_stat & PCI_ERR) && HAS_PCI(host))) {
 		mv_pci_error(host, mmio);
 		handled = 1;
 		goto out_unlock;	/* skip all other HC irq handling */
@@ -1788,8 +1745,9 @@ static int mv5_scr_write(struct ata_port *ap, unsigned int sc_reg_in, u32 val)
 		return -EINVAL;
 }
 
-static void mv5_reset_bus(struct pci_dev *pdev, void __iomem *mmio)
+static void mv5_reset_bus(struct ata_host *host, void __iomem *mmio)
 {
+	struct pci_dev *pdev = to_pci_dev(host->dev);
 	int early_5080;
 
 	early_5080 = (pdev->device == 0x5080) && (pdev->revision == 0);
@@ -1800,7 +1758,7 @@ static void mv5_reset_bus(struct pci_dev *pdev, void __iomem *mmio)
 		writel(tmp, mmio + MV_PCI_EXP_ROM_BAR_CTL);
 	}
 
-	mv_reset_pci_bus(pdev, mmio);
+	mv_reset_pci_bus(host, mmio);
 }
 
 static void mv5_reset_flash(struct mv_host_priv *hpriv, void __iomem *mmio)
@@ -1924,7 +1882,7 @@ static int mv5_reset_hc(struct mv_host_priv *hpriv, void __iomem *mmio,
 
 #undef ZERO
 #define ZERO(reg) writel(0, mmio + (reg))
-static void mv_reset_pci_bus(struct pci_dev *pdev, void __iomem *mmio)
+static void mv_reset_pci_bus(struct ata_host *host, void __iomem *mmio)
 {
 	u32 tmp;
 
@@ -2534,7 +2492,6 @@ static int mv_chip_id(struct ata_host *host, unsigned int board_idx)
 static int mv_init_host(struct ata_host *host, unsigned int board_idx)
 {
 	int rc = 0, n_hc, port, hc;
-	struct pci_dev *pdev = to_pci_dev(host->dev);
 	void __iomem *mmio = host->iomap[MV_PRIMARY_BAR];
 	struct mv_host_priv *hpriv = host->private_data;
 
@@ -2555,7 +2512,7 @@ static int mv_init_host(struct ata_host *host, unsigned int board_idx)
 		goto done;
 
 	hpriv->ops->reset_flash(hpriv, mmio);
-	hpriv->ops->reset_bus(pdev, mmio);
+	hpriv->ops->reset_bus(host, mmio);
 	hpriv->ops->enable_leds(hpriv, mmio);
 
 	for (port = 0; port < host->n_ports; port++) {
@@ -2578,8 +2535,10 @@ static int mv_init_host(struct ata_host *host, unsigned int board_idx)
 
 		mv_port_init(&ap->ioaddr, port_mmio);
 
+#ifdef CONFIG_PCI
 		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, -1, "mmio");
 		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, offset, "port");
+#endif
 	}
 
 	for (hc = 0; hc < n_hc; hc++) {
@@ -2616,6 +2575,55 @@ done:
 	return rc;
 }
 
+#ifdef CONFIG_PCI
+static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
+
+static struct pci_driver mv_pci_driver = {
+	.name			= DRV_NAME,
+	.id_table		= mv_pci_tbl,
+	.probe			= mv_init_one,
+	.remove			= ata_pci_remove_one,
+};
+
+/*
+ * module options
+ */
+static int msi;	      /* Use PCI msi; either zero (off, default) or non-zero */
+
+
+/* move to PCI layer or libata core? */
+static int pci_go_64(struct pci_dev *pdev)
+{
+	int rc;
+
+	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK)) {
+		rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
+		if (rc) {
+			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
+			if (rc) {
+				dev_printk(KERN_ERR, &pdev->dev,
+					   "64-bit DMA enable failed\n");
+				return rc;
+			}
+		}
+	} else {
+		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
+		if (rc) {
+			dev_printk(KERN_ERR, &pdev->dev,
+				   "32-bit DMA enable failed\n");
+			return rc;
+		}
+		rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
+		if (rc) {
+			dev_printk(KERN_ERR, &pdev->dev,
+				   "32-bit consistent DMA enable failed\n");
+			return rc;
+		}
+	}
+
+	return rc;
+}
+
 /**
  *      mv_print_info - Dump key info to kernel log for perusal.
  *      @host: ATA host to print info about
@@ -2720,15 +2728,22 @@ static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	return ata_host_activate(host, pdev->irq, mv_interrupt, IRQF_SHARED,
 				 IS_GEN_I(hpriv) ? &mv5_sht : &mv6_sht);
 }
+#endif
 
 static int __init mv_init(void)
 {
-	return pci_register_driver(&mv_pci_driver);
+	int rc = -ENODEV;
+#ifdef CONFIG_PCI
+	rc = pci_register_driver(&mv_pci_driver);
+#endif
+	return rc;
 }
 
 static void __exit mv_exit(void)
 {
+#ifdef CONFIG_PCI
 	pci_unregister_driver(&mv_pci_driver);
+#endif
 }
 
 MODULE_AUTHOR("Brett Russ");
@@ -2737,8 +2752,10 @@ MODULE_LICENSE("GPL");
 MODULE_DEVICE_TABLE(pci, mv_pci_tbl);
 MODULE_VERSION(DRV_VERSION);
 
+#ifdef CONFIG_PCI
 module_param(msi, int, 0444);
 MODULE_PARM_DESC(msi, "Enable use of PCI MSI (0=off, 1=on)");
+#endif
 
 module_init(mv_init);
 module_exit(mv_exit);
-- 
1.5.0.6

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[Mark Lord]

saeed bishara wrote:
>
> I attached the new patch
..

Try again, please.
This time, post the patch *inline* in the body of the email,
so that it can more easily be seen, read, and commented on.

Beware of many email clients that mangle inline patches, though.

Cheers

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[saeed bishara]

On 1/16/08, Mark Lord <liml@rtr.ca> wrote:
> saeed bishara wrote:
> >
> > I attached the new patch
> ..
>
> Try again, please.
> This time, post the patch *inline* in the body of the email,
> so that it can more easily be seen, read, and commented on.
>
> Beware of many email clients that mangle inline patches, though.
>
> Cheers
>
Here is the same patch inlined:
>From c5f5af7f2eed2fe42e28374257b983922dfd3db5 Mon Sep 17 00:00:00 2001
From: Saeed Bishara <saeed@marvell.com>
Date: Sun, 2 Dec 2007 10:43:10 +0200
Subject: [PATCH] sata_mv: Remove PCI dependency

The integrated SATA controller is connected directly to the SoC's
internal bus, not via PCI interface. this patch removes the dependency
on the PCI interface.

Signed-off-by: Saeed Bishara <saeed@marvell.com>
---
 drivers/ata/Kconfig   |    2 +-
 drivers/ata/sata_mv.c |  131 +++++++++++++++++++++++++++---------------------
 2 files changed, 75 insertions(+), 58 deletions(-)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index ba63619..c60842b 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -69,7 +69,7 @@ config ATA_PIIX

 config SATA_MV
 	tristate "Marvell SATA support (HIGHLY EXPERIMENTAL)"
-	depends on PCI && EXPERIMENTAL
+	depends on EXPERIMENTAL
 	help
 	  This option enables support for the Marvell Serial ATA family.
 	  Currently supports 88SX[56]0[48][01] chips.
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index a43f64d..9b0d590 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -125,6 +125,9 @@ enum {
 	/* Host Flags */
 	MV_FLAG_DUAL_HC		= (1 << 30),  /* two SATA Host Controllers */
 	MV_FLAG_IRQ_COALESCE	= (1 << 29),  /* IRQ coalescing capability */
+	/* SoC integrated controllers, no PCI interface */
+	MV_FLAG_SOC = (1 << 28),
+
 	MV_COMMON_FLAGS		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO | ATA_FLAG_NO_ATAPI |
 				  ATA_FLAG_PIO_POLLING,
@@ -312,6 +315,7 @@ enum {
 #define IS_GEN_I(hpriv) ((hpriv)->hp_flags & MV_HP_GEN_I)
 #define IS_GEN_II(hpriv) ((hpriv)->hp_flags & MV_HP_GEN_II)
 #define IS_GEN_IIE(hpriv) ((hpriv)->hp_flags & MV_HP_GEN_IIE)
+#define HAS_PCI(host) (!((host)->ports[0]->flags & MV_FLAG_SOC))

 enum {
 	/* DMA boundary 0xffff is required by the s/g splitting
@@ -398,7 +402,7 @@ struct mv_hw_ops {
 	int (*reset_hc)(struct mv_host_priv *hpriv, void __iomem *mmio,
 			unsigned int n_hc);
 	void (*reset_flash)(struct mv_host_priv *hpriv, void __iomem *mmio);
-	void (*reset_bus)(struct pci_dev *pdev, void __iomem *mmio);
+	void (*reset_bus)(struct ata_host *host, void __iomem *mmio);
 };

 struct mv_host_priv {
@@ -421,7 +425,6 @@ static void mv_error_handler(struct ata_port *ap);
 static void mv_post_int_cmd(struct ata_queued_cmd *qc);
 static void mv_eh_freeze(struct ata_port *ap);
 static void mv_eh_thaw(struct ata_port *ap);
-static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);

 static void mv5_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
 			   unsigned int port);
@@ -431,7 +434,7 @@ static void mv5_read_preamp(struct mv_host_priv
*hpriv, int idx,
 static int mv5_reset_hc(struct mv_host_priv *hpriv, void __iomem *mmio,
 			unsigned int n_hc);
 static void mv5_reset_flash(struct mv_host_priv *hpriv, void __iomem *mmio);
-static void mv5_reset_bus(struct pci_dev *pdev, void __iomem *mmio);
+static void mv5_reset_bus(struct ata_host *host, void __iomem *mmio);

 static void mv6_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
 			   unsigned int port);
@@ -441,7 +444,7 @@ static void mv6_read_preamp(struct mv_host_priv
*hpriv, int idx,
 static int mv6_reset_hc(struct mv_host_priv *hpriv, void __iomem *mmio,
 			unsigned int n_hc);
 static void mv6_reset_flash(struct mv_host_priv *hpriv, void __iomem *mmio);
-static void mv_reset_pci_bus(struct pci_dev *pdev, void __iomem *mmio);
+static void mv_reset_pci_bus(struct ata_host *host, void __iomem *mmio);
 static void mv_channel_reset(struct mv_host_priv *hpriv, void __iomem *mmio,
 			     unsigned int port_no);

@@ -639,13 +642,6 @@ static const struct pci_device_id mv_pci_tbl[] = {
 	{ }			/* terminate list */
 };

-static struct pci_driver mv_pci_driver = {
-	.name			= DRV_NAME,
-	.id_table		= mv_pci_tbl,
-	.probe			= mv_init_one,
-	.remove			= ata_pci_remove_one,
-};
-
 static const struct mv_hw_ops mv5xxx_ops = {
 	.phy_errata		= mv5_phy_errata,
 	.enable_leds		= mv5_enable_leds,
@@ -665,45 +661,6 @@ static const struct mv_hw_ops mv6xxx_ops = {
 };

 /*
- * module options
- */
-static int msi;	      /* Use PCI msi; either zero (off, default) or non-zero */
-
-
-/* move to PCI layer or libata core? */
-static int pci_go_64(struct pci_dev *pdev)
-{
-	int rc;
-
-	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK)) {
-		rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
-		if (rc) {
-			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
-			if (rc) {
-				dev_printk(KERN_ERR, &pdev->dev,
-					   "64-bit DMA enable failed\n");
-				return rc;
-			}
-		}
-	} else {
-		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
-		if (rc) {
-			dev_printk(KERN_ERR, &pdev->dev,
-				   "32-bit DMA enable failed\n");
-			return rc;
-		}
-		rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
-		if (rc) {
-			dev_printk(KERN_ERR, &pdev->dev,
-				   "32-bit consistent DMA enable failed\n");
-			return rc;
-		}
-	}
-
-	return rc;
-}
-
-/*
  * Functions
  */

@@ -1717,7 +1674,7 @@ static irqreturn_t mv_interrupt(int irq, void
*dev_instance)
 	n_hcs = mv_get_hc_count(host->ports[0]->flags);
 	spin_lock(&host->lock);

-	if (unlikely(irq_stat & PCI_ERR)) {
+	if (unlikely((irq_stat & PCI_ERR) && HAS_PCI(host))) {
 		mv_pci_error(host, mmio);
 		handled = 1;
 		goto out_unlock;	/* skip all other HC irq handling */
@@ -1788,8 +1745,9 @@ static int mv5_scr_write(struct ata_port *ap,
unsigned int sc_reg_in, u32 val)
 		return -EINVAL;
 }

-static void mv5_reset_bus(struct pci_dev *pdev, void __iomem *mmio)
+static void mv5_reset_bus(struct ata_host *host, void __iomem *mmio)
 {
+	struct pci_dev *pdev = to_pci_dev(host->dev);
 	int early_5080;

 	early_5080 = (pdev->device == 0x5080) && (pdev->revision == 0);
@@ -1800,7 +1758,7 @@ static void mv5_reset_bus(struct pci_dev *pdev,
void __iomem *mmio)
 		writel(tmp, mmio + MV_PCI_EXP_ROM_BAR_CTL);
 	}

-	mv_reset_pci_bus(pdev, mmio);
+	mv_reset_pci_bus(host, mmio);
 }

 static void mv5_reset_flash(struct mv_host_priv *hpriv, void __iomem *mmio)
@@ -1924,7 +1882,7 @@ static int mv5_reset_hc(struct mv_host_priv
*hpriv, void __iomem *mmio,

 #undef ZERO
 #define ZERO(reg) writel(0, mmio + (reg))
-static void mv_reset_pci_bus(struct pci_dev *pdev, void __iomem *mmio)
+static void mv_reset_pci_bus(struct ata_host *host, void __iomem *mmio)
 {
 	u32 tmp;

@@ -2534,7 +2492,6 @@ static int mv_chip_id(struct ata_host *host,
unsigned int board_idx)
 static int mv_init_host(struct ata_host *host, unsigned int board_idx)
 {
 	int rc = 0, n_hc, port, hc;
-	struct pci_dev *pdev = to_pci_dev(host->dev);
 	void __iomem *mmio = host->iomap[MV_PRIMARY_BAR];
 	struct mv_host_priv *hpriv = host->private_data;

@@ -2555,7 +2512,7 @@ static int mv_init_host(struct ata_host *host,
unsigned int board_idx)
 		goto done;

 	hpriv->ops->reset_flash(hpriv, mmio);
-	hpriv->ops->reset_bus(pdev, mmio);
+	hpriv->ops->reset_bus(host, mmio);
 	hpriv->ops->enable_leds(hpriv, mmio);

 	for (port = 0; port < host->n_ports; port++) {
@@ -2578,8 +2535,10 @@ static int mv_init_host(struct ata_host *host,
unsigned int board_idx)

 		mv_port_init(&ap->ioaddr, port_mmio);

+#ifdef CONFIG_PCI
 		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, -1, "mmio");
 		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, offset, "port");
+#endif
 	}

 	for (hc = 0; hc < n_hc; hc++) {
@@ -2616,6 +2575,55 @@ done:
 	return rc;
 }

+#ifdef CONFIG_PCI
+static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
+
+static struct pci_driver mv_pci_driver = {
+	.name			= DRV_NAME,
+	.id_table		= mv_pci_tbl,
+	.probe			= mv_init_one,
+	.remove			= ata_pci_remove_one,
+};
+
+/*
+ * module options
+ */
+static int msi;	      /* Use PCI msi; either zero (off, default) or non-zero */
+
+
+/* move to PCI layer or libata core? */
+static int pci_go_64(struct pci_dev *pdev)
+{
+	int rc;
+
+	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK)) {
+		rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
+		if (rc) {
+			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
+			if (rc) {
+				dev_printk(KERN_ERR, &pdev->dev,
+					   "64-bit DMA enable failed\n");
+				return rc;
+			}
+		}
+	} else {
+		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
+		if (rc) {
+			dev_printk(KERN_ERR, &pdev->dev,
+				   "32-bit DMA enable failed\n");
+			return rc;
+		}
+		rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
+		if (rc) {
+			dev_printk(KERN_ERR, &pdev->dev,
+				   "32-bit consistent DMA enable failed\n");
+			return rc;
+		}
+	}
+
+	return rc;
+}
+
 /**
  *      mv_print_info - Dump key info to kernel log for perusal.
  *      @host: ATA host to print info about
@@ -2720,15 +2728,22 @@ static int mv_init_one(struct pci_dev *pdev,
const struct pci_device_id *ent)
 	return ata_host_activate(host, pdev->irq, mv_interrupt, IRQF_SHARED,
 				 IS_GEN_I(hpriv) ? &mv5_sht : &mv6_sht);
 }
+#endif

 static int __init mv_init(void)
 {
-	return pci_register_driver(&mv_pci_driver);
+	int rc = -ENODEV;
+#ifdef CONFIG_PCI
+	rc = pci_register_driver(&mv_pci_driver);
+#endif
+	return rc;
 }

 static void __exit mv_exit(void)
 {
+#ifdef CONFIG_PCI
 	pci_unregister_driver(&mv_pci_driver);
+#endif
 }

 MODULE_AUTHOR("Brett Russ");
@@ -2737,8 +2752,10 @@ MODULE_LICENSE("GPL");
 MODULE_DEVICE_TABLE(pci, mv_pci_tbl);
 MODULE_VERSION(DRV_VERSION);

+#ifdef CONFIG_PCI
 module_param(msi, int, 0444);
 MODULE_PARM_DESC(msi, "Enable use of PCI MSI (0=off, 1=on)");
+#endif

 module_init(mv_init);
 module_exit(mv_exit);
-- 
1.5.0.6

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[Jeff Garzik]

saeed bishara wrote:
> On 1/16/08, Mark Lord <liml@rtr.ca> wrote:
>> saeed bishara wrote:
>>> I attached the new patch
>> ..
>>
>> Try again, please.
>> This time, post the patch *inline* in the body of the email,
>> so that it can more easily be seen, read, and commented on.
>>
>> Beware of many email clients that mangle inline patches, though.
>>
>> Cheers
>>
> Here is the same patch inlined:
>>From c5f5af7f2eed2fe42e28374257b983922dfd3db5 Mon Sep 17 00:00:00 2001
> From: Saeed Bishara <saeed@marvell.com>
> Date: Sun, 2 Dec 2007 10:43:10 +0200
> Subject: [PATCH] sata_mv: Remove PCI dependency
> 
> The integrated SATA controller is connected directly to the SoC's
> internal bus, not via PCI interface. this patch removes the dependency
> on the PCI interface.
> 
> Signed-off-by: Saeed Bishara <saeed@marvell.com>
> ---
>  drivers/ata/Kconfig   |    2 +-
>  drivers/ata/sata_mv.c |  131 +++++++++++++++++++++++++++---------------------
>  2 files changed, 75 insertions(+), 58 deletions(-)

ACK the changes; but both git-am(1) and patch(1) report that this is a 
corrupt patch.  patch(1) says,


[jgarzik@pretzel libata-dev]$ patch -sp1 < /g/tmp/mbox
patch: **** malformed patch at line 151: *hpriv, int idx,



(patch is generally more forgiving than git-am)

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[saeed bishara]

>
> ACK the changes; but both git-am(1) and patch(1) report that this is a
> corrupt patch.  patch(1) says,
>
>
> [jgarzik@pretzel libata-dev]$ patch -sp1 < /g/tmp/mbox
> patch: **** malformed patch at line 151: *hpriv, int idx,
>
>
>
> (patch is generally more forgiving than git-am)
ok, I'll send the patch as [PATCH v2] in separate email with git-send-email.

saeed

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[Tejun Heo]

Jeff Garzik wrote:
> saeed bishara wrote:
>> -    if (unlikely(irq_stat & PCI_ERR)) {
>> +    if (unlikely(irq_stat & PCI_ERR) && HAS_PCI(host)) {
>>          mv_pci_error(host, mmio);
>>          handled = 1;
>>          goto out_unlock;    /* skip all other HC irq handling */
> 
> the unlikely() should cover the entire expression.

Hmm... I also sometimes hesitate about these things.  I think when the
first condition is unlikely but the latter isn't so, it's better to tell
the compiler that the first part is unlikely and let it figure out the
rest.  Another thing is using unlikely on the return value of helper
function like the following.

static bool test_some_condition(arg)
{
	return unlikely(unlikely_test_on_arg(arg));
}

I haven't looked at the generated code yet but hope the compiler can
DTRT if the function body is visible when compiling.  Has anyone played
with these?

Thanks.

-- 
tejun

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[Jeff Garzik]

Tejun Heo wrote:
> Jeff Garzik wrote:
>> saeed bishara wrote:
>>> -    if (unlikely(irq_stat & PCI_ERR)) {
>>> +    if (unlikely(irq_stat & PCI_ERR) && HAS_PCI(host)) {
>>>          mv_pci_error(host, mmio);
>>>          handled = 1;
>>>          goto out_unlock;    /* skip all other HC irq handling */
>> the unlikely() should cover the entire expression.
> 
> Hmm... I also sometimes hesitate about these things.  I think when the
> first condition is unlikely but the latter isn't so, it's better to tell
> the compiler that the first part is unlikely and let it figure out the
> rest.  Another thing is using unlikely on the return value of helper
> function like the following.
> 
> static bool test_some_condition(arg)
> {
> 	return unlikely(unlikely_test_on_arg(arg));
> }
> 
> I haven't looked at the generated code yet but hope the compiler can
> DTRT if the function body is visible when compiling.  Has anyone played
> with these?

Think about the question being answered -- this is branch prediction. 
After all tests in an 'if' are complete, the question is:  do we take 
this branch or not?  (yes/no)

And while it is valid to note unlikely() as done above, it doesn't 
necessarily give the compiler the best idea about how to predict the 
ultimate end result of all the tests, which is the decision on whether 
or not to take the branch.

	Jeff

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Jeff Garzik wrote:
>>> saeed bishara wrote:
>>>> -    if (unlikely(irq_stat & PCI_ERR)) {
>>>> +    if (unlikely(irq_stat & PCI_ERR) && HAS_PCI(host)) {
>>>>          mv_pci_error(host, mmio);
>>>>          handled = 1;
>>>>          goto out_unlock;    /* skip all other HC irq handling */
>>> the unlikely() should cover the entire expression.
>>
>> Hmm... I also sometimes hesitate about these things.  I think when the
>> first condition is unlikely but the latter isn't so, it's better to tell
>> the compiler that the first part is unlikely and let it figure out the
>> rest.  Another thing is using unlikely on the return value of helper
>> function like the following.
>>
>> static bool test_some_condition(arg)
>> {
>>     return unlikely(unlikely_test_on_arg(arg));
>> }
>>
>> I haven't looked at the generated code yet but hope the compiler can
>> DTRT if the function body is visible when compiling.  Has anyone played
>> with these?
> 
> Think about the question being answered -- this is branch prediction.
> After all tests in an 'if' are complete, the question is:  do we take
> this branch or not?  (yes/no)

Well, if (a && b) is not a single branch.  It's two (or more) branches.
 Let's say a is unlikely but b is; then with unlikely(a && b), the
compiler do something like the following.

	jmp L0 if !a
	jmp L0 if !b
	jmp L1
.L0	continue excution after if block

---------
cold unlikely block
.L1	body of if

Or some other form.  With (unlikely(a) && b), the compiler knows the
optimal form is...

	jmp L1 if a
.L0	continue excution after if block

---------
cold unlikely block
.L1	jmp L0 if !b
	body of if

> And while it is valid to note unlikely() as done above, it doesn't
> necessarily give the compiler the best idea about how to predict the
> ultimate end result of all the tests, which is the decision on whether
> or not to take the branch.

It actually gives the compiler better information to optimize with and
test conditions in if() can get quite involved and it also gives better
idea of what the writer was thinking when writing the likely/unlikely
when the code is read later.

-- 
tejun

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[saeed bishara]

Hi Jeff,
 what is the status of this patch?
I'd like to have this patch, and a second part that adds support for
the SoC devices, to be merged into 2.6.25. please note that my patch
collides with Mark's patches, I don't mind to rebase mine above Mark's
patches, but, in that case I need to wait till you ack the later ones.
right?

saeed

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[Mark Lord]

saeed bishara wrote:
> Hi Jeff,
>  what is the status of this patch?
> I'd like to have this patch, and a second part that adds support for
> the SoC devices, to be merged into 2.6.25. please note that my patch
> collides with Mark's patches, I don't mind to rebase mine above Mark's
> patches, but, in that case I need to wait till you ack the later ones.
> right?
..

It would be great if you could do that, and it would help Jeff immensely.

I think Jeff is in Australia right now, at linux.conf.au for the week,
so he is going to be slow responding to patches and stuff now.

Cheers

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[Jeff Garzik]

Mark Lord wrote:
> saeed bishara wrote:
>> Hi Jeff,
>>  what is the status of this patch?
>> I'd like to have this patch, and a second part that adds support for
>> the SoC devices, to be merged into 2.6.25. please note that my patch
>> collides with Mark's patches, I don't mind to rebase mine above Mark's
>> patches, but, in that case I need to wait till you ack the later ones.
>> right?
> ..
> 
> It would be great if you could do that, and it would help Jeff immensely.
> 
> I think Jeff is in Australia right now, at linux.conf.au for the week,
> so he is going to be slow responding to patches and stuff now.

Thanks for the backstop :)  Not in Australia, just learning to take 
weekends off, like my wife tells me to :)

BTW, in order to avoid having to resend the sata_mv NCQ patchset, I'm 
going to order that in front of saeed's patch.  At this point saeed's 
single patch is easier to rediff than a ~15 patch series, most certainly.

	Jeff

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[Jeff Garzik]

saeed bishara wrote:
> Hi Jeff,
>  what is the status of this patch?
> I'd like to have this patch, and a second part that adds support for
> the SoC devices, to be merged into 2.6.25. please note that my patch
> collides with Mark's patches, I don't mind to rebase mine above Mark's
> patches, but, in that case I need to wait till you ack the later ones.
> right?

Sorry, I found your 1/17 patch buried inside a thread.

In the future, it would be helpful to rewrite the subject from

	Re: [patch 1/2] sata_mv: ...

to

	[patch v2] sata_mv: ...

so that its clear that is a patch to be queued.

	Jeff

[PATCH 0/2] [libata] sata_mv: Add support for Marvell's integrated SATA controller

[saeed.bishara]

From: Saeed Bishara <saeed@marvell.com>

This patch series adds support for the integrated SATA controller that can be found in some of Marvell's SoC's. This controller is based on the 88SX7042, but it connected directly to the internal bus of the SoC and not via PCI.

[PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[saeed.bishara]

From: Saeed Bishara <saeed@marvell.com>

The integrated SATA controller is connected directly to the SoC's
internal bus, not via PCI interface. this patch removes the dependency
on the PCI interface.

Signed-off-by: Saeed Bishara <saeed@marvell.com>
---
 drivers/ata/Kconfig   |    2 +-
 drivers/ata/sata_mv.c |  113 ++++++++++++++++++++++++++++++-------------------
 2 files changed, 71 insertions(+), 44 deletions(-)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index ba63619..c60842b 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -69,7 +69,7 @@ config ATA_PIIX
 
 config SATA_MV
 	tristate "Marvell SATA support (HIGHLY EXPERIMENTAL)"
-	depends on PCI && EXPERIMENTAL
+	depends on EXPERIMENTAL
 	help
 	  This option enables support for the Marvell Serial ATA family.
 	  Currently supports 88SX[56]0[48][01] chips.
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 97c3e11..cbd3f1b 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -421,7 +421,9 @@ static void mv_error_handler(struct ata_port *ap);
 static void mv_post_int_cmd(struct ata_queued_cmd *qc);
 static void mv_eh_freeze(struct ata_port *ap);
 static void mv_eh_thaw(struct ata_port *ap);
+#ifdef CONFIG_PCI
 static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
+#endif
 
 static void mv5_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
 			   unsigned int port);
@@ -638,14 +640,14 @@ static const struct pci_device_id mv_pci_tbl[] = {
 
 	{ }			/* terminate list */
 };
-
+#ifdef CONFIG_PCI
 static struct pci_driver mv_pci_driver = {
 	.name			= DRV_NAME,
 	.id_table		= mv_pci_tbl,
 	.probe			= mv_init_one,
 	.remove			= ata_pci_remove_one,
 };
-
+#endif
 static const struct mv_hw_ops mv5xxx_ops = {
 	.phy_errata		= mv5_phy_errata,
 	.enable_leds		= mv5_enable_leds,
@@ -665,45 +667,6 @@ static const struct mv_hw_ops mv6xxx_ops = {
 };
 
 /*
- * module options
- */
-static int msi;	      /* Use PCI msi; either zero (off, default) or non-zero */
-
-
-/* move to PCI layer or libata core? */
-static int pci_go_64(struct pci_dev *pdev)
-{
-	int rc;
-
-	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK)) {
-		rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
-		if (rc) {
-			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
-			if (rc) {
-				dev_printk(KERN_ERR, &pdev->dev,
-					   "64-bit DMA enable failed\n");
-				return rc;
-			}
-		}
-	} else {
-		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
-		if (rc) {
-			dev_printk(KERN_ERR, &pdev->dev,
-				   "32-bit DMA enable failed\n");
-			return rc;
-		}
-		rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
-		if (rc) {
-			dev_printk(KERN_ERR, &pdev->dev,
-				   "32-bit consistent DMA enable failed\n");
-			return rc;
-		}
-	}
-
-	return rc;
-}
-
-/*
  * Functions
  */
 
@@ -2578,8 +2541,10 @@ static int mv_init_host(struct ata_host *host, unsigned int board_idx)
 
 		mv_port_init(&ap->ioaddr, port_mmio);
 
+#ifdef CONFIG_PCI
 		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, -1, "mmio");
 		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, offset, "port");
+#endif
 	}
 
 	for (hc = 0; hc < n_hc; hc++) {
@@ -2616,6 +2581,47 @@ done:
 	return rc;
 }
 
+#ifdef CONFIG_PCI
+
+/*
+ * module options
+ */
+static int msi;	      /* Use PCI msi; either zero (off, default) or non-zero */
+
+
+/* move to PCI layer or libata core? */
+static int pci_go_64(struct pci_dev *pdev)
+{
+	int rc;
+
+	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK)) {
+		rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
+		if (rc) {
+			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
+			if (rc) {
+				dev_printk(KERN_ERR, &pdev->dev,
+					   "64-bit DMA enable failed\n");
+				return rc;
+			}
+		}
+	} else {
+		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
+		if (rc) {
+			dev_printk(KERN_ERR, &pdev->dev,
+				   "32-bit DMA enable failed\n");
+			return rc;
+		}
+		rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
+		if (rc) {
+			dev_printk(KERN_ERR, &pdev->dev,
+				   "32-bit consistent DMA enable failed\n");
+			return rc;
+		}
+	}
+
+	return rc;
+}
+
 /**
  *      mv_print_info - Dump key info to kernel log for perusal.
  *      @host: ATA host to print info about
@@ -2721,15 +2727,34 @@ static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 				 IS_GEN_I(hpriv) ? &mv5_sht : &mv6_sht);
 }
 
-static int __init mv_init(void)
+static int __init mv_pci_register_driver(void)
 {
 	return pci_register_driver(&mv_pci_driver);
 }
 
-static void __exit mv_exit(void)
+static void __exit mv_pci_unregister_driver(void)
 {
 	pci_unregister_driver(&mv_pci_driver);
 }
+#else
+static int __init mv_pci_register_driver(void)
+{
+	return 0;
+}
+
+static void __exit mv_pci_unregister_driver(void)
+{
+}
+#endif
+static int __init mv_init(void)
+{
+	return mv_pci_register_driver();
+}
+
+static void __exit mv_exit(void)
+{
+	mv_pci_unregister_driver();
+}
 
 MODULE_AUTHOR("Brett Russ");
 MODULE_DESCRIPTION("SCSI low-level driver for Marvell SATA controllers");
@@ -2737,8 +2762,10 @@ MODULE_LICENSE("GPL");
 MODULE_DEVICE_TABLE(pci, mv_pci_tbl);
 MODULE_VERSION(DRV_VERSION);
 
+#ifdef CONFIG_PCI
 module_param(msi, int, 0444);
 MODULE_PARM_DESC(msi, "Enable use of PCI MSI (0=off, 1=on)");
+#endif
 
 module_init(mv_init);
 module_exit(mv_exit);
-- 
1.5.0.6

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[saeed bishara]

On 12/2/07, saeed.bishara@gmail.com <saeed.bishara@gmail.com> wrote:
>
rebased the patch with the latest updates:
The integrated SATA controller is connected directly to the SoC's
internal bus, not via PCI interface. this patch removes the dependency
on the PCI interface.

Signed-off-by: Saeed Bishara <saeed@marvell.com>
---
 drivers/ata/Kconfig   |    2 +-
 drivers/ata/sata_mv.c |  113 ++++++++++++++++++++++++++++++-------------------
 2 files changed, 71 insertions(+), 44 deletions(-)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 2478cca..06e366f 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -69,7 +69,7 @@ config ATA_PIIX

 config SATA_MV
 	tristate "Marvell SATA support (HIGHLY EXPERIMENTAL)"
-	depends on PCI && EXPERIMENTAL
+	depends on EXPERIMENTAL
 	help
 	  This option enables support for the Marvell Serial ATA family.
 	  Currently supports 88SX[56]0[48][01] chips.
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 8d864e5..a778ab7 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -428,7 +428,9 @@ static void mv_error_handler(struct ata_port *ap);
 static void mv_post_int_cmd(struct ata_queued_cmd *qc);
 static void mv_eh_freeze(struct ata_port *ap);
 static void mv_eh_thaw(struct ata_port *ap);
+#ifdef CONFIG_PCI
 static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
+#endif

 static void mv5_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
 			   unsigned int port);
@@ -647,14 +649,14 @@ static const struct pci_device_id mv_pci_tbl[] = {

 	{ }			/* terminate list */
 };
-
+#ifdef CONFIG_PCI
 static struct pci_driver mv_pci_driver = {
 	.name			= DRV_NAME,
 	.id_table		= mv_pci_tbl,
 	.probe			= mv_init_one,
 	.remove			= ata_pci_remove_one,
 };
-
+#endif
 static const struct mv_hw_ops mv5xxx_ops = {
 	.phy_errata		= mv5_phy_errata,
 	.enable_leds		= mv5_enable_leds,
@@ -674,45 +676,6 @@ static const struct mv_hw_ops mv6xxx_ops = {
 };

 /*
- * module options
- */
-static int msi;	      /* Use PCI msi; either zero (off, default) or non-zero */
-
-
-/* move to PCI layer or libata core? */
-static int pci_go_64(struct pci_dev *pdev)
-{
-	int rc;
-
-	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK)) {
-		rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
-		if (rc) {
-			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
-			if (rc) {
-				dev_printk(KERN_ERR, &pdev->dev,
-					   "64-bit DMA enable failed\n");
-				return rc;
-			}
-		}
-	} else {
-		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
-		if (rc) {
-			dev_printk(KERN_ERR, &pdev->dev,
-				   "32-bit DMA enable failed\n");
-			return rc;
-		}
-		rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
-		if (rc) {
-			dev_printk(KERN_ERR, &pdev->dev,
-				   "32-bit consistent DMA enable failed\n");
-			return rc;
-		}
-	}
-
-	return rc;
-}
-
-/*
  * Functions
  */

@@ -2600,8 +2563,10 @@ static int mv_init_host(struct ata_host *host,
unsigned int board_idx)

 		mv_port_init(&ap->ioaddr, port_mmio);

+#ifdef CONFIG_PCI
 		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, -1, "mmio");
 		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, offset, "port");
+#endif
 	}

 	for (hc = 0; hc < n_hc; hc++) {
@@ -2638,6 +2603,47 @@ done:
 	return rc;
 }

+#ifdef CONFIG_PCI
+
+/*
+ * module options
+ */
+static int msi;	      /* Use PCI msi; either zero (off, default) or non-zero */
+
+
+/* move to PCI layer or libata core? */
+static int pci_go_64(struct pci_dev *pdev)
+{
+	int rc;
+
+	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK)) {
+		rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
+		if (rc) {
+			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
+			if (rc) {
+				dev_printk(KERN_ERR, &pdev->dev,
+					   "64-bit DMA enable failed\n");
+				return rc;
+			}
+		}
+	} else {
+		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
+		if (rc) {
+			dev_printk(KERN_ERR, &pdev->dev,
+				   "32-bit DMA enable failed\n");
+			return rc;
+		}
+		rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
+		if (rc) {
+			dev_printk(KERN_ERR, &pdev->dev,
+				   "32-bit consistent DMA enable failed\n");
+			return rc;
+		}
+	}
+
+	return rc;
+}
+
 /**
  *      mv_print_info - Dump key info to kernel log for perusal.
  *      @host: ATA host to print info about
@@ -2743,15 +2749,34 @@ static int mv_init_one(struct pci_dev *pdev,
const struct pci_device_id *ent)
 				 IS_GEN_I(hpriv) ? &mv5_sht : &mv6_sht);
 }

-static int __init mv_init(void)
+static int __init mv_pci_register_driver(void)
 {
 	return pci_register_driver(&mv_pci_driver);
 }

-static void __exit mv_exit(void)
+static void __exit mv_pci_unregister_driver(void)
 {
 	pci_unregister_driver(&mv_pci_driver);
 }
+#else
+static int __init mv_pci_register_driver(void)
+{
+	return 0;
+}
+
+static void __exit mv_pci_unregister_driver(void)
+{
+}
+#endif
+static int __init mv_init(void)
+{
+	return mv_pci_register_driver();
+}
+
+static void __exit mv_exit(void)
+{
+	mv_pci_unregister_driver();
+}

 MODULE_AUTHOR("Brett Russ");
 MODULE_DESCRIPTION("SCSI low-level driver for Marvell SATA controllers");
@@ -2759,8 +2784,10 @@ MODULE_LICENSE("GPL");
 MODULE_DEVICE_TABLE(pci, mv_pci_tbl);
 MODULE_VERSION(DRV_VERSION);

+#ifdef CONFIG_PCI
 module_param(msi, int, 0444);
 MODULE_PARM_DESC(msi, "Enable use of PCI MSI (0=off, 1=on)");
+#endif

 module_init(mv_init);
 module_exit(mv_exit);
-- 
1.5.0.6

Re: [PATCH 1/2] [libata] sata_mv: Remove PCI dependency

[saeed bishara]

On 12/2/07, saeed.bishara@gmail.com <saeed.bishara@gmail.com> wrote:
> From: Saeed Bishara <saeed@marvell.com>
>
> The integrated SATA controller is connected directly to the SoC's
> internal bus, not via PCI interface. this patch removes the dependency
> on the PCI interface.
>
> Signed-off-by: Saeed Bishara <saeed@marvell.com>
> ---
>  drivers/ata/Kconfig   |    2 +-
>  drivers/ata/sata_mv.c |  113
> ++++++++++++++++++++++++++++++-------------------
>  2 files changed, 71 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index ba63619..c60842b 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -69,7 +69,7 @@ config ATA_PIIX
>
>  config SATA_MV
>  	tristate "Marvell SATA support (HIGHLY EXPERIMENTAL)"
> -	depends on PCI && EXPERIMENTAL
> +	depends on EXPERIMENTAL
>  	help
>  	  This option enables support for the Marvell Serial ATA family.
>  	  Currently supports 88SX[56]0[48][01] chips.
> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> index 97c3e11..cbd3f1b 100644
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
> @@ -421,7 +421,9 @@ static void mv_error_handler(struct ata_port *ap);
>  static void mv_post_int_cmd(struct ata_queued_cmd *qc);
>  static void mv_eh_freeze(struct ata_port *ap);
>  static void mv_eh_thaw(struct ata_port *ap);
> +#ifdef CONFIG_PCI
>  static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id
> *ent);
> +#endif
>
>  static void mv5_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
>  			   unsigned int port);
> @@ -638,14 +640,14 @@ static const struct pci_device_id mv_pci_tbl[] = {
>
>  	{ }			/* terminate list */
>  };
> -
> +#ifdef CONFIG_PCI
>  static struct pci_driver mv_pci_driver = {
>  	.name			= DRV_NAME,
>  	.id_table		= mv_pci_tbl,
>  	.probe			= mv_init_one,
>  	.remove			= ata_pci_remove_one,
>  };
> -
> +#endif
>  static const struct mv_hw_ops mv5xxx_ops = {
>  	.phy_errata		= mv5_phy_errata,
>  	.enable_leds		= mv5_enable_leds,
> @@ -665,45 +667,6 @@ static const struct mv_hw_ops mv6xxx_ops = {
>  };
>
>  /*
> - * module options
> - */
> -static int msi;	      /* Use PCI msi; either zero (off, default) or
> non-zero */
> -
> -
> -/* move to PCI layer or libata core? */
> -static int pci_go_64(struct pci_dev *pdev)
> -{
> -	int rc;
> -
> -	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK)) {
> -		rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
> -		if (rc) {
> -			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
> -			if (rc) {
> -				dev_printk(KERN_ERR, &pdev->dev,
> -					   "64-bit DMA enable failed\n");
> -				return rc;
> -			}
> -		}
> -	} else {
> -		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
> -		if (rc) {
> -			dev_printk(KERN_ERR, &pdev->dev,
> -				   "32-bit DMA enable failed\n");
> -			return rc;
> -		}
> -		rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
> -		if (rc) {
> -			dev_printk(KERN_ERR, &pdev->dev,
> -				   "32-bit consistent DMA enable failed\n");
> -			return rc;
> -		}
> -	}
> -
> -	return rc;
> -}
> -
> -/*
>   * Functions
>   */
>
> @@ -2578,8 +2541,10 @@ static int mv_init_host(struct ata_host *host,
> unsigned int board_idx)
>
>  		mv_port_init(&ap->ioaddr, port_mmio);
>
> +#ifdef CONFIG_PCI
>  		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, -1, "mmio");
>  		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, offset, "port");
> +#endif
>  	}
>
>  	for (hc = 0; hc < n_hc; hc++) {
> @@ -2616,6 +2581,47 @@ done:
>  	return rc;
>  }
>
> +#ifdef CONFIG_PCI
> +
> +/*
> + * module options
> + */
> +static int msi;	      /* Use PCI msi; either zero (off, default) or
> non-zero */
> +
> +
> +/* move to PCI layer or libata core? */
> +static int pci_go_64(struct pci_dev *pdev)
> +{
> +	int rc;
> +
> +	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK)) {
> +		rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
> +		if (rc) {
> +			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
> +			if (rc) {
> +				dev_printk(KERN_ERR, &pdev->dev,
> +					   "64-bit DMA enable failed\n");
> +				return rc;
> +			}
> +		}
> +	} else {
> +		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
> +		if (rc) {
> +			dev_printk(KERN_ERR, &pdev->dev,
> +				   "32-bit DMA enable failed\n");
> +			return rc;
> +		}
> +		rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
> +		if (rc) {
> +			dev_printk(KERN_ERR, &pdev->dev,
> +				   "32-bit consistent DMA enable failed\n");
> +			return rc;
> +		}
> +	}
> +
> +	return rc;
> +}
> +
>  /**
>   *      mv_print_info - Dump key info to kernel log for perusal.
>   *      @host: ATA host to print info about
> @@ -2721,15 +2727,34 @@ static int mv_init_one(struct pci_dev *pdev, const
> struct pci_device_id *ent)
>  				 IS_GEN_I(hpriv) ? &mv5_sht : &mv6_sht);
>  }
>
> -static int __init mv_init(void)
> +static int __init mv_pci_register_driver(void)
>  {
>  	return pci_register_driver(&mv_pci_driver);
>  }
>
> -static void __exit mv_exit(void)
> +static void __exit mv_pci_unregister_driver(void)
>  {
>  	pci_unregister_driver(&mv_pci_driver);
>  }
> +#else
> +static int __init mv_pci_register_driver(void)
> +{
> +	return 0;
> +}
> +
> +static void __exit mv_pci_unregister_driver(void)
> +{
> +}
> +#endif
> +static int __init mv_init(void)
> +{
> +	return mv_pci_register_driver();
> +}
> +
> +static void __exit mv_exit(void)
> +{
> +	mv_pci_unregister_driver();
> +}
>
>  MODULE_AUTHOR("Brett Russ");
>  MODULE_DESCRIPTION("SCSI low-level driver for Marvell SATA controllers");
> @@ -2737,8 +2762,10 @@ MODULE_LICENSE("GPL");
>  MODULE_DEVICE_TABLE(pci, mv_pci_tbl);
>  MODULE_VERSION(DRV_VERSION);
>
> +#ifdef CONFIG_PCI
>  module_param(msi, int, 0444);
>  MODULE_PARM_DESC(msi, "Enable use of PCI MSI (0=off, 1=on)");
> +#endif
>
>  module_init(mv_init);
>  module_exit(mv_exit);
> --
> 1.5.0.6
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>