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

[Jeff Garzik <jeff@garzik.org>]

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