| Message ID | 20110716210420.234549871@rtp-net.org |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
Hello. On 17-07-2011 1:04, Arnaud Patard (Rtp) wrote: > Add basic support for pata on iMX. It has been tested only on imx51. > SDMA support will probably be added later so this version supports only > PIO. > Signed-off-by: Arnaud Patard<arnaud.patard@rtp-net.org> [...] > Index: linux-2.6-submit/drivers/ata/pata_imx.c > =================================================================== > --- /dev/null 1970-01-01 00:00:00.000000000 +0000 > +++ linux-2.6-submit/drivers/ata/pata_imx.c 2011-07-16 22:35:41.000000000 +0200 > @@ -0,0 +1,254 @@ [...] > + * TODO: > + * - dmaengine support > + * - check if timing stuff needed Sure, it's needed, especially if you want to support DMA. > +#define PATA_IMX_ATA_CONTROL 0x24 > +#define PATA_IMX_ATA_CTRL_FIFO_RST_B (1<<7) > +#define PATA_IMX_ATA_CTRL_ATA_RST_B (1<<6) > +#define PATA_IMX_ATA_CTRL_IORDY_EN (1<<0) So, it seems to support advanced PIO modes... > +static int __devinit pata_imx_probe(struct platform_device *pdev) > +{ > + struct ata_host *host; > + struct ata_port *ap; > + struct pata_imx_priv *priv; > + int irq = 0; > + struct resource *io_res; > + struct resource *irq_res; > + > + io_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (io_res == NULL) > + return -EINVAL; > + > + irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); Why not use platform_get_irq()? > + if (irq_res == NULL) > + return -EINVAL; > + irq = irq_res->start; > + > + priv = kzalloc(sizeof(struct pata_imx_priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->clk = clk_get(&pdev->dev, "imx-pata"); Does iMX support clkdev? [...] > + ap->ops = &pata_imx_port_ops; > + /* pio 0-4 */ > + ap->pio_mask = ATA_PIO4; But you don't support PIO1-4! > + ap->flags |= ATA_FLAG_SLAVE_POSS; > + > + priv->host_regs = devm_ioremap(&pdev->dev, io_res->start, 0x40); > + ap->ioaddr.cmd_addr = devm_ioremap(&pdev->dev, > + io_res->start + PATA_IMX_DRIVE_DATA, > + 0x20); > + ap->ioaddr.ctl_addr = devm_ioremap(&pdev->dev, > + io_res->start + PATA_IMX_DRIVE_CONTROL, > + 0x04); Why not ioremap all registers as one block? > + ata_port_desc(ap, "cmd 0x%llx ctl 0x%llx", > + (unsigned long long)io_res->start + PATA_IMX_DRIVE_DATA, > + (unsigned long long)io_res->start + PATA_IMX_DRIVE_CONTROL); > + > + /* deassert resets */ > + __raw_writel(PATA_IMX_ATA_CTRL_FIFO_RST_B | > + PATA_IMX_ATA_CTRL_ATA_RST_B | > + PATA_IMX_ATA_CTRL_IORDY_EN, You can't enable IORDY by default, only when the drive supports it! [...] > +#ifdef CONFIG_PM > +static int pata_imx_suspend(struct device *dev) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + struct ata_host *host = platform_get_drvdata(pdev); You can just use dev_get_drvdata(), without calling to_platfrom_device(). > + struct pata_imx_priv *priv = host->private_data; > + int ret; > + > + ret = ata_host_suspend(host, PMSG_SUSPEND); > + Empty line not needed here... > + if (!ret) > + __raw_writel(0, priv->host_regs + PATA_IMX_ATA_INT_EN); [...] > +static int pata_imx_resume(struct device *dev) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + struct ata_host *host = platform_get_drvdata(pdev); Just use dev_get_drvdata(). > + struct pata_imx_priv *priv = host->private_data; [...] WBR, Sergei -- 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
Sergei Shtylyov <sshtylyov@mvista.com> writes: > Hello. Hi, > > On 17-07-2011 1:04, Arnaud Patard (Rtp) wrote: > >> Add basic support for pata on iMX. It has been tested only on imx51. >> SDMA support will probably be added later so this version supports only >> PIO. > >> Signed-off-by: Arnaud Patard<arnaud.patard@rtp-net.org> > [...] >> Index: linux-2.6-submit/drivers/ata/pata_imx.c >> =================================================================== >> --- /dev/null 1970-01-01 00:00:00.000000000 +0000 >> +++ linux-2.6-submit/drivers/ata/pata_imx.c 2011-07-16 22:35:41.000000000 +0200 >> @@ -0,0 +1,254 @@ > [...] >> + * TODO: >> + * - dmaengine support >> + * - check if timing stuff needed > > Sure, it's needed, especially if you want to support DMA. > >> +#define PATA_IMX_ATA_CONTROL 0x24 >> +#define PATA_IMX_ATA_CTRL_FIFO_RST_B (1<<7) >> +#define PATA_IMX_ATA_CTRL_ATA_RST_B (1<<6) >> +#define PATA_IMX_ATA_CTRL_IORDY_EN (1<<0) > > So, it seems to support advanced PIO modes... > >> +static int __devinit pata_imx_probe(struct platform_device *pdev) >> +{ >> + struct ata_host *host; >> + struct ata_port *ap; >> + struct pata_imx_priv *priv; >> + int irq = 0; >> + struct resource *io_res; >> + struct resource *irq_res; >> + >> + io_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + if (io_res == NULL) >> + return -EINVAL; >> + >> + irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); > > Why not use platform_get_irq()? No special reason. Was using get_resource() for mem, so I used it for irq too. > >> + if (irq_res == NULL) >> + return -EINVAL; >> + irq = irq_res->start; >> + >> + priv = kzalloc(sizeof(struct pata_imx_priv), GFP_KERNEL); >> + if (!priv) >> + return -ENOMEM; >> + >> + priv->clk = clk_get(&pdev->dev, "imx-pata"); > > Does iMX support clkdev? > > [...] iirc, it's not supported >> + ap->ops = &pata_imx_port_ops; >> + /* pio 0-4 */ >> + ap->pio_mask = ATA_PIO4; > > But you don't support PIO1-4! > >> + ap->flags |= ATA_FLAG_SLAVE_POSS; >> + >> + priv->host_regs = devm_ioremap(&pdev->dev, io_res->start, 0x40); >> + ap->ioaddr.cmd_addr = devm_ioremap(&pdev->dev, >> + io_res->start + PATA_IMX_DRIVE_DATA, >> + 0x20); >> + ap->ioaddr.ctl_addr = devm_ioremap(&pdev->dev, >> + io_res->start + PATA_IMX_DRIVE_CONTROL, >> + 0x04); > > Why not ioremap all registers as one block? > the manual doesn't talk about the address space in between so I prefered to be safe and use 2 parts, like the manual. >> + ata_port_desc(ap, "cmd 0x%llx ctl 0x%llx", >> + (unsigned long long)io_res->start + PATA_IMX_DRIVE_DATA, >> + (unsigned long long)io_res->start + PATA_IMX_DRIVE_CONTROL); >> + >> + /* deassert resets */ >> + __raw_writel(PATA_IMX_ATA_CTRL_FIFO_RST_B | >> + PATA_IMX_ATA_CTRL_ATA_RST_B | >> + PATA_IMX_ATA_CTRL_IORDY_EN, > > You can't enable IORDY by default, only when the drive supports it! > hm... ok. The original code was enabling it in the machine file and fsl driver doesn't handle IORDY at all so I'm not sure how to do that and make sure it won't break. I guess I'll have to see if it break on my system to test it but it's not a really complete test. > [...] >> +#ifdef CONFIG_PM >> +static int pata_imx_suspend(struct device *dev) >> +{ >> + struct platform_device *pdev = to_platform_device(dev); >> + struct ata_host *host = platform_get_drvdata(pdev); > > You can just use dev_get_drvdata(), without calling to_platfrom_device(). > ok. Arnaud -- 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
Hello. On 21-07-2011 19:59, Arnaud Patard (Rtp) wrote: >>> Add basic support for pata on iMX. It has been tested only on imx51. >>> SDMA support will probably be added later so this version supports only >>> PIO. >>> Signed-off-by: Arnaud Patard<arnaud.patard@rtp-net.org> >> [...] >>> Index: linux-2.6-submit/drivers/ata/pata_imx.c >>> =================================================================== >>> --- /dev/null 1970-01-01 00:00:00.000000000 +0000 >>> +++ linux-2.6-submit/drivers/ata/pata_imx.c 2011-07-16 22:35:41.000000000 +0200 >>> @@ -0,0 +1,254 @@ >> [...] >>> +static int __devinit pata_imx_probe(struct platform_device *pdev) >>> +{ >>> + struct ata_host *host; >>> + struct ata_port *ap; >>> + struct pata_imx_priv *priv; >>> + int irq = 0; >>> + struct resource *io_res; >>> + struct resource *irq_res; >>> + >>> + io_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> + if (io_res == NULL) >>> + return -EINVAL; >>> + >>> + irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); >> Why not use platform_get_irq()? > No special reason. Was using get_resource() for mem, so I used it for > irq too. platform_get_irq() would yield smaller code. >>> + if (irq_res == NULL) >>> + return -EINVAL; >>> + irq = irq_res->start; >>> + >>> + priv = kzalloc(sizeof(struct pata_imx_priv), GFP_KERNEL); >>> + if (!priv) >>> + return -ENOMEM; >>> + >>> + priv->clk = clk_get(&pdev->dev, "imx-pata"); >> >> Does iMX support clkdev? >> >> [...] > iirc, it's not supported Then it's good idea to pass the clock name thru the platfrom data. WBR, Sergei -- 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
Index: linux-2.6-submit/drivers/ata/Kconfig =================================================================== --- linux-2.6-submit.orig/drivers/ata/Kconfig 2011-07-16 22:29:41.000000000 +0200 +++ linux-2.6-submit/drivers/ata/Kconfig 2011-07-16 22:31:38.000000000 +0200 @@ -467,6 +467,15 @@ config PATA_ICSIDE interface card. This is not required for ICS partition support. If you are unsure, say N to this. +config PATA_IMX + tristate "PATA support for Freescale iMX (Experimental)" + depends on ARCH_MX5 && EXPERIMENTAL + help + This option enables support for the PATA host available on Freescale + iMX SoCs. + + If unsure, say N. + config PATA_IT8213 tristate "IT8213 PATA support (Experimental)" depends on PCI && EXPERIMENTAL Index: linux-2.6-submit/drivers/ata/Makefile =================================================================== --- linux-2.6-submit.orig/drivers/ata/Makefile 2011-07-16 22:29:41.000000000 +0200 +++ linux-2.6-submit/drivers/ata/Makefile 2011-07-16 22:31:38.000000000 +0200 @@ -48,6 +48,7 @@ obj-$(CONFIG_PATA_HPT37X) += pata_hpt37x obj-$(CONFIG_PATA_HPT3X2N) += pata_hpt3x2n.o obj-$(CONFIG_PATA_HPT3X3) += pata_hpt3x3.o obj-$(CONFIG_PATA_ICSIDE) += pata_icside.o +obj-$(CONFIG_PATA_IMX) += pata_imx.o obj-$(CONFIG_PATA_IT8213) += pata_it8213.o obj-$(CONFIG_PATA_IT821X) += pata_it821x.o obj-$(CONFIG_PATA_JMICRON) += pata_jmicron.o Index: linux-2.6-submit/drivers/ata/pata_imx.c =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ linux-2.6-submit/drivers/ata/pata_imx.c 2011-07-16 22:35:41.000000000 +0200 @@ -0,0 +1,254 @@ +/* + * Freescale iMX PATA driver + * + * Copyright (C) 2011 Arnaud Patard <arnaud.patard@rtp-net.org> + * + * Based on pata_platform - Copyright (C) 2006 - 2007 Paul Mundt + * + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file "COPYING" in the main directory of this archive + * for more details. + * + * TODO: + * - dmaengine support + * - check if timing stuff needed + */ +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/init.h> +#include <linux/blkdev.h> +#include <scsi/scsi_host.h> +#include <linux/ata.h> +#include <linux/libata.h> +#include <linux/platform_device.h> +#include <linux/clk.h> + +#define DRV_NAME "pata_imx" + +#define PATA_IMX_ATA_CONTROL 0x24 +#define PATA_IMX_ATA_CTRL_FIFO_RST_B (1<<7) +#define PATA_IMX_ATA_CTRL_ATA_RST_B (1<<6) +#define PATA_IMX_ATA_CTRL_IORDY_EN (1<<0) +#define PATA_IMX_ATA_INT_EN 0x2C +#define PATA_IMX_ATA_INTR_ATA_INTRQ2 (1<<3) +#define PATA_IMX_DRIVE_DATA 0xA0 +#define PATA_IMX_DRIVE_CONTROL 0xD8 + +struct pata_imx_priv { + struct clk *clk; + /* timings/interrupt/control regs */ + u8 *host_regs; +}; + +static int pata_imx_set_mode(struct ata_link *link, struct ata_device **unused) +{ + struct ata_device *dev; + + ata_for_each_dev(dev, link, ENABLED) { + dev->pio_mode = dev->xfer_mode = XFER_PIO_0; + dev->xfer_shift = ATA_SHIFT_PIO; + dev->flags |= ATA_DFLAG_PIO; + ata_dev_printk(dev, KERN_INFO, "configured for PIO\n"); + } + return 0; +} + +static struct scsi_host_template pata_imx_sht = { + ATA_PIO_SHT(DRV_NAME), +}; + +static struct ata_port_operations pata_imx_port_ops = { + .inherits = &ata_sff_port_ops, + .sff_data_xfer = ata_sff_data_xfer_noirq, + .cable_detect = ata_cable_unknown, + .set_mode = pata_imx_set_mode, +}; + +static void pata_imx_setup_port(struct ata_ioports *ioaddr) +{ + /* Fixup the port shift for platforms that need it */ + ioaddr->data_addr = ioaddr->cmd_addr + (ATA_REG_DATA << 2); + ioaddr->error_addr = ioaddr->cmd_addr + (ATA_REG_ERR << 2); + ioaddr->feature_addr = ioaddr->cmd_addr + (ATA_REG_FEATURE << 2); + ioaddr->nsect_addr = ioaddr->cmd_addr + (ATA_REG_NSECT << 2); + ioaddr->lbal_addr = ioaddr->cmd_addr + (ATA_REG_LBAL << 2); + ioaddr->lbam_addr = ioaddr->cmd_addr + (ATA_REG_LBAM << 2); + ioaddr->lbah_addr = ioaddr->cmd_addr + (ATA_REG_LBAH << 2); + ioaddr->device_addr = ioaddr->cmd_addr + (ATA_REG_DEVICE << 2); + ioaddr->status_addr = ioaddr->cmd_addr + (ATA_REG_STATUS << 2); + ioaddr->command_addr = ioaddr->cmd_addr + (ATA_REG_CMD << 2); +} + +static int __devinit pata_imx_probe(struct platform_device *pdev) +{ + struct ata_host *host; + struct ata_port *ap; + struct pata_imx_priv *priv; + int irq = 0; + struct resource *io_res; + struct resource *irq_res; + + io_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (io_res == NULL) + return -EINVAL; + + irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); + if (irq_res == NULL) + return -EINVAL; + irq = irq_res->start; + + priv = kzalloc(sizeof(struct pata_imx_priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->clk = clk_get(&pdev->dev, "imx-pata"); + if (!priv->clk) + dev_warn(&pdev->dev, "Failed to get clock\n"); + else + clk_enable(priv->clk); + + + host = ata_host_alloc(&pdev->dev, 1); + if (!host) + goto free_priv; + + host->private_data = priv; + ap = host->ports[0]; + + ap->ops = &pata_imx_port_ops; + /* pio 0-4 */ + ap->pio_mask = ATA_PIO4; + ap->flags |= ATA_FLAG_SLAVE_POSS; + + priv->host_regs = devm_ioremap(&pdev->dev, io_res->start, 0x40); + ap->ioaddr.cmd_addr = devm_ioremap(&pdev->dev, + io_res->start + PATA_IMX_DRIVE_DATA, + 0x20); + ap->ioaddr.ctl_addr = devm_ioremap(&pdev->dev, + io_res->start + PATA_IMX_DRIVE_CONTROL, + 0x04); + if (!priv->host_regs || !ap->ioaddr.cmd_addr || !ap->ioaddr.ctl_addr) { + dev_err(&pdev->dev, "failed to map IO/CTL base\n"); + goto free_priv; + } + + ap->ioaddr.altstatus_addr = ap->ioaddr.ctl_addr; + + pata_imx_setup_port(&ap->ioaddr); + + ata_port_desc(ap, "cmd 0x%llx ctl 0x%llx", + (unsigned long long)io_res->start + PATA_IMX_DRIVE_DATA, + (unsigned long long)io_res->start + PATA_IMX_DRIVE_CONTROL); + + /* deassert resets */ + __raw_writel(PATA_IMX_ATA_CTRL_FIFO_RST_B | + PATA_IMX_ATA_CTRL_ATA_RST_B | + PATA_IMX_ATA_CTRL_IORDY_EN, + priv->host_regs + PATA_IMX_ATA_CONTROL); + /* enable interrupts */ + __raw_writel(PATA_IMX_ATA_INTR_ATA_INTRQ2, + priv->host_regs + PATA_IMX_ATA_INT_EN); + + /* activate */ + return ata_host_activate(host, irq, ata_sff_interrupt, 0, + &pata_imx_sht); + +free_priv: + if (priv->clk) + clk_disable(priv->clk); + kfree(priv); + return -ENOMEM; +} + +static int __devexit pata_imx_remove(struct platform_device *pdev) +{ + struct ata_host *host = dev_get_drvdata(&pdev->dev); + struct pata_imx_priv *priv = host->private_data; + + ata_host_detach(host); + + __raw_writel(0, priv->host_regs + PATA_IMX_ATA_INT_EN); + + if (priv->clk) + clk_disable(priv->clk); + + kfree(priv); + + return 0; +} + +#ifdef CONFIG_PM +static int pata_imx_suspend(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + struct ata_host *host = platform_get_drvdata(pdev); + struct pata_imx_priv *priv = host->private_data; + int ret; + + ret = ata_host_suspend(host, PMSG_SUSPEND); + + if (!ret) + __raw_writel(0, priv->host_regs + PATA_IMX_ATA_INT_EN); + + if (priv->clk) + clk_disable(priv->clk); + + return ret; +} + +static int pata_imx_resume(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + struct ata_host *host = platform_get_drvdata(pdev); + struct pata_imx_priv *priv = host->private_data; + + if (priv->clk) + clk_enable(priv->clk); + + __raw_writel(PATA_IMX_ATA_CTRL_FIFO_RST_B | + PATA_IMX_ATA_CTRL_ATA_RST_B | + PATA_IMX_ATA_CTRL_IORDY_EN, + priv->host_regs + PATA_IMX_ATA_CONTROL); + + __raw_writel(PATA_IMX_ATA_INTR_ATA_INTRQ2, + priv->host_regs + PATA_IMX_ATA_INT_EN); + + ata_host_resume(host); + + return 0; +} + +static const struct dev_pm_ops pata_imx_pm_ops = { + .suspend = pata_imx_suspend, + .resume = pata_imx_resume, +}; +#endif + +static struct platform_driver pata_imx_driver = { + .probe = pata_imx_probe, + .remove = __devexit_p(pata_imx_remove), + .driver = { + .name = DRV_NAME, + .owner = THIS_MODULE, +#ifdef CONFIG_PM + .pm = &pata_imx_pm_ops, +#endif + }, +}; + +static int __init pata_imx_init(void) +{ + return platform_driver_register(&pata_imx_driver); +} + +static void __exit pata_imx_exit(void) +{ + platform_driver_unregister(&pata_imx_driver); +} +module_init(pata_imx_init); +module_exit(pata_imx_exit); + +MODULE_AUTHOR("Arnaud Patard <arnaud.patard@rtp-net.org>"); +MODULE_DESCRIPTION("low-level driver for iMX PATA"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:" DRV_NAME);
Add basic support for pata on iMX. It has been tested only on imx51. SDMA support will probably be added later so this version supports only PIO. Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org> -- 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