| Message ID | 20171015232214.ouj4j5pebuvlobvo@lenoch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 16/10/17 02:22, Ladislav Michl wrote: missing commit log > Signed-off-by: Ladislav Michl <ladis@linux-mips.org> > --- > drivers/mtd/onenand/omap2.c | 145 +++++++++++++++++++++++++++++++++----------- > 1 file changed, 110 insertions(+), 35 deletions(-) > > diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c > index 24a1388d3031..4811897a4e9f 100644 > --- a/drivers/mtd/onenand/omap2.c > +++ b/drivers/mtd/onenand/omap2.c > @@ -28,6 +28,8 @@ > #include <linux/mtd/mtd.h> > #include <linux/mtd/onenand.h> > #include <linux/mtd/partitions.h> > +#include <linux/of_device.h> > +#include <linux/omap-gpmc.h> > #include <linux/platform_device.h> > #include <linux/interrupt.h> > #include <linux/delay.h> > @@ -38,7 +40,6 @@ > #include <linux/gpio.h> > > #include <asm/mach/flash.h> > -#include <linux/platform_data/mtd-onenand-omap2.h> > > #include <linux/omap-dma.h> > > @@ -57,10 +58,8 @@ struct omap2_onenand { > struct completion irq_done; > struct completion dma_done; > int dma_channel; > - int freq; > - int (*setup)(void __iomem *base, int *freq_ptr); > struct regulator *regulator; > - u8 flags; > + bool is_omap2; > }; > > static void omap2_onenand_dma_cb(int lch, u16 ch_status, void *data) > @@ -90,6 +89,52 @@ static inline void write_reg(struct omap2_onenand *c, unsigned short value, > writew(value, c->onenand.base + reg); > } > > +/* Ensure sync read and sync write are disabled */ > +static void set_async_mode(struct omap2_onenand *c) omap2_onenand_set_async_mode() > +{ > + unsigned short reg = read_reg(c, ONENAND_REG_SYS_CFG1); need blank line > + reg &= ~ONENAND_SYS_CFG1_SYNC_READ & ~ONENAND_SYS_CFG1_SYNC_WRITE; > + write_reg(c, reg, ONENAND_REG_SYS_CFG1); > +} > + > +static void set_cfg(struct omap2_onenand *c, bool sr, bool sw, int latency) omap2_onenand_set_cfg() > +{ > + unsigned short reg = ONENAND_SYS_CFG1_RDY | ONENAND_SYS_CFG1_INT; > + > + reg |= (latency << ONENAND_SYS_CFG1_BRL_SHIFT) | > + ONENAND_SYS_CFG1_BL_16; > + if (latency > 5) > + reg |= ONENAND_SYS_CFG1_HF; > + if (latency > 7) > + reg |= ONENAND_SYS_CFG1_VHF; This will set both HF and VHF flags if latency is greater than 7. Is this what we want? > + if (sr) > + reg |= ONENAND_SYS_CFG1_SYNC_READ; > + if (sw) > + reg |= ONENAND_SYS_CFG1_SYNC_WRITE; > + > + write_reg(c, reg, ONENAND_REG_SYS_CFG1); > +} > + > +static int get_freq(struct omap2_onenand *c) omap2_onenand_get_freq() > +{ > + unsigned short ver = read_reg(c, ONENAND_REG_VERSION_ID); > + > + switch ((ver >> 4) & 0xf) { > + case 0: > + return 40; > + case 1: > + return 54; > + case 2: > + return 66; > + case 3: > + return 83; > + case 4: > + return 104; default: return -EINVAL; ? > + } > + > + return 0; > +} > + > static void wait_err(char *msg, int state, unsigned int ctrl, unsigned int intr) > { > printk(KERN_ERR "onenand_wait: %s! state %d ctrl 0x%04x intr 0x%04x\n", > @@ -153,7 +198,7 @@ static int omap2_onenand_wait(struct mtd_info *mtd, int state) > if (!(syscfg & ONENAND_SYS_CFG1_IOBE)) { > syscfg |= ONENAND_SYS_CFG1_IOBE; > write_reg(c, syscfg, ONENAND_REG_SYS_CFG1); > - if (c->flags & ONENAND_IN_OMAP34XX) > + if (!c->is_omap2) seems like you should have a is_omap3 flag as this quirk is for omap3 only. > /* Add a delay to let GPIO settle */ > syscfg = read_reg(c, ONENAND_REG_SYS_CFG1); > } > @@ -607,30 +652,46 @@ static int omap2_onenand_disable(struct mtd_info *mtd) > return ret; > } > > +static int omap2_onenand_get_dt(struct device *dev, struct omap2_onenand *c) > +{ > + struct device_node *child = dev->of_node; > + u32 cs, val; > + > + if (of_property_read_u32(child, "reg", &cs) < 0) { > + dev_err(dev, "reg not found in DT\n"); > + return -EINVAL; > + } > + c->gpmc_cs = cs; > + > + if (!of_property_read_u32(child, "dma-channel", &val)) > + c->dma_channel = val; > + else > + c->dma_channel = -1; > + > + c->is_omap2 = (bool)of_device_get_match_data(dev); > + > + /* TODO: gpio, regulator, unlocking */ > + > + return 0; > +} > + > static int omap2_onenand_probe(struct platform_device *pdev) > { > - struct omap_onenand_platform_data *pdata; > struct omap2_onenand *c; > struct onenand_chip *this; > - int r; > + int freq, r; > struct resource *res; > > - pdata = dev_get_platdata(&pdev->dev); > - if (pdata == NULL) { > - dev_err(&pdev->dev, "platform data missing\n"); > - return -ENODEV; > - } > - > c = kzalloc(sizeof(struct omap2_onenand), GFP_KERNEL); > if (!c) > return -ENOMEM; > > + r = omap2_onenand_get_dt(&pdev->dev, c); > + if (r) > + goto err_kfree; > + > init_completion(&c->irq_done); > init_completion(&c->dma_done); > - c->flags = pdata->flags; > - c->gpmc_cs = pdata->cs; > - c->gpio_irq = pdata->gpio_irq; > - c->dma_channel = pdata->dma_channel; > if (c->dma_channel < 0) { > /* if -1, don't use DMA */ > c->gpio_irq = 0; > @@ -659,16 +720,23 @@ static int omap2_onenand_probe(struct platform_device *pdev) > goto err_release_mem_region; > } > > - if (pdata->onenand_setup != NULL) { > - r = pdata->onenand_setup(c->onenand.base, &c->freq); > - if (r < 0) { > - dev_err(&pdev->dev, "Onenand platform setup failed: " > - "%d\n", r); > - goto err_iounmap; > - } > - c->setup = pdata->onenand_setup; > + set_async_mode(c); > + freq = get_freq(c); > + if (!freq) { > + dev_err(&pdev->dev, > + "Rate not detected, bad GPMC async timings?\n"); > + r = -ENODEV; > + goto err_iounmap; > } > > + r = gpmc_omap_onenand_set_sync_timings(&pdev->dev, c->gpmc_cs, freq); > + if (r < 0) { > + dev_err(&pdev->dev, "Cannot set sync timings: %d\n", r); > + goto err_iounmap; > + } > + if (r > 0) > + set_cfg(c, true, true, r); > + > if (c->gpio_irq) { > if ((r = gpio_request(c->gpio_irq, "OneNAND irq")) < 0) { > dev_err(&pdev->dev, "Failed to request GPIO%d for " > @@ -706,27 +774,27 @@ static int omap2_onenand_probe(struct platform_device *pdev) > > dev_info(&pdev->dev, "initializing on CS%d, phys base 0x%08lx, virtual " > "base %p, freq %d MHz\n", c->gpmc_cs, c->phys_base, > - c->onenand.base, c->freq); > + c->onenand.base, freq); > > c->pdev = pdev; > c->mtd.priv = &c->onenand; > > c->mtd.dev.parent = &pdev->dev; > - mtd_set_of_node(&c->mtd, pdata->of_node); > + mtd_set_of_node(&c->mtd, pdev->dev.of_node); > > this = &c->onenand; > if (c->dma_channel >= 0) { > this->wait = omap2_onenand_wait; > - if (c->flags & ONENAND_IN_OMAP34XX) { > - this->read_bufferram = omap3_onenand_read_bufferram; > - this->write_bufferram = omap3_onenand_write_bufferram; > - } else { > + if (c->is_omap2) { > this->read_bufferram = omap2_onenand_read_bufferram; > this->write_bufferram = omap2_onenand_write_bufferram; > + } else { > + this->read_bufferram = omap3_onenand_read_bufferram; > + this->write_bufferram = omap3_onenand_write_bufferram; > } > } > > - if (pdata->regulator_can_sleep) { > + if (0 /* TODO: pdata->regulator_can_sleep */) { > c->regulator = regulator_get(&pdev->dev, "vonenand"); > if (IS_ERR(c->regulator)) { > dev_err(&pdev->dev, "Failed to get regulator\n"); > @@ -737,14 +805,13 @@ static int omap2_onenand_probe(struct platform_device *pdev) > c->onenand.disable = omap2_onenand_disable; > } > > - if (pdata->skip_initial_unlocking) > + if (0 /* TODO: pdata->skip_initial_unlocking */) > this->options |= ONENAND_SKIP_INITIAL_UNLOCKING; > > if ((r = onenand_scan(&c->mtd, 1)) < 0) > goto err_release_regulator; > > - r = mtd_device_register(&c->mtd, pdata ? pdata->parts : NULL, > - pdata ? pdata->nr_parts : 0); > + r = mtd_device_register(&c->mtd, NULL, 0); > if (r) > goto err_release_onenand; > > @@ -794,12 +861,20 @@ static int omap2_onenand_remove(struct platform_device *pdev) > return 0; > } > > +static const struct of_device_id omap2_onenand_ids[] = { > + { .compatible = "ti,omap2-onenand", .data = (void *)true, }, > + { .compatible = "ti,omap3-onenand", .data = (void *)false, }, This doesn't clearly show what we're doing. Can we instead have a proper data structure for .data? If that is an overkill maybe we should avoid the of_data approach and instead do if (of_device_is_compatible(node, "ti,omap3-nand") c->need_wait_delay = true; > + { }, > +}; > +MODULE_DEVICE_TABLE(of, omap2_onenand_ids); > + > static struct platform_driver omap2_onenand_driver = { > .probe = omap2_onenand_probe, > .remove = omap2_onenand_remove, > .shutdown = omap2_onenand_shutdown, > .driver = { > .name = DRIVER_NAME, > + .of_match_table = of_match_ptr(omap2_onenand_ids), > }, > }; > > We need to update the DT binding doc as well devicetree/bindings/mtd$ vi gpmc-onenand.txt This should be done in a separate patch.
On Tue, Oct 17, 2017 at 11:33:56AM +0300, Roger Quadros wrote: > Hi, > > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > > On 16/10/17 02:22, Ladislav Michl wrote: > > missing commit log There will be enough future versions to add some :) > > Signed-off-by: Ladislav Michl <ladis@linux-mips.org> > > --- > > drivers/mtd/onenand/omap2.c | 145 +++++++++++++++++++++++++++++++++----------- > > 1 file changed, 110 insertions(+), 35 deletions(-) > > > > diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c > > index 24a1388d3031..4811897a4e9f 100644 > > --- a/drivers/mtd/onenand/omap2.c > > +++ b/drivers/mtd/onenand/omap2.c > > @@ -28,6 +28,8 @@ > > #include <linux/mtd/mtd.h> > > #include <linux/mtd/onenand.h> > > #include <linux/mtd/partitions.h> > > +#include <linux/of_device.h> > > +#include <linux/omap-gpmc.h> > > #include <linux/platform_device.h> > > #include <linux/interrupt.h> > > #include <linux/delay.h> > > @@ -38,7 +40,6 @@ > > #include <linux/gpio.h> > > > > #include <asm/mach/flash.h> > > -#include <linux/platform_data/mtd-onenand-omap2.h> > > > > #include <linux/omap-dma.h> > > > > @@ -57,10 +58,8 @@ struct omap2_onenand { > > struct completion irq_done; > > struct completion dma_done; > > int dma_channel; > > - int freq; > > - int (*setup)(void __iomem *base, int *freq_ptr); > > struct regulator *regulator; > > - u8 flags; > > + bool is_omap2; > > }; > > > > static void omap2_onenand_dma_cb(int lch, u16 ch_status, void *data) > > @@ -90,6 +89,52 @@ static inline void write_reg(struct omap2_onenand *c, unsigned short value, > > writew(value, c->onenand.base + reg); > > } > > > > +/* Ensure sync read and sync write are disabled */ > > +static void set_async_mode(struct omap2_onenand *c) > > omap2_onenand_set_async_mode() > > > +{ > > + unsigned short reg = read_reg(c, ONENAND_REG_SYS_CFG1); > need blank line > > > + reg &= ~ONENAND_SYS_CFG1_SYNC_READ & ~ONENAND_SYS_CFG1_SYNC_WRITE; > > + write_reg(c, reg, ONENAND_REG_SYS_CFG1); > > +} > > + > > +static void set_cfg(struct omap2_onenand *c, bool sr, bool sw, int latency) > > omap2_onenand_set_cfg() > > > +{ > > + unsigned short reg = ONENAND_SYS_CFG1_RDY | ONENAND_SYS_CFG1_INT; > > + > > + reg |= (latency << ONENAND_SYS_CFG1_BRL_SHIFT) | > > + ONENAND_SYS_CFG1_BL_16; > > + if (latency > 5) > > + reg |= ONENAND_SYS_CFG1_HF; > > + if (latency > 7) > > + reg |= ONENAND_SYS_CFG1_VHF; > > This will set both HF and VHF flags if latency is greater than 7. Is this what we want? That's why patches 1-5 are there, so you can verify... > > + if (sr) > > + reg |= ONENAND_SYS_CFG1_SYNC_READ; > > + if (sw) > > + reg |= ONENAND_SYS_CFG1_SYNC_WRITE; > > + > > + write_reg(c, reg, ONENAND_REG_SYS_CFG1); > > +} > > + > > +static int get_freq(struct omap2_onenand *c) > > omap2_onenand_get_freq() All other helper functions are unprefixed, but will change that. > > +{ > > + unsigned short ver = read_reg(c, ONENAND_REG_VERSION_ID); > > + > > + switch ((ver >> 4) & 0xf) { > > + case 0: > > + return 40; > > + case 1: > > + return 54; > > + case 2: > > + return 66; > > + case 3: > > + return 83; > > + case 4: > > + return 104; > > default: > return -EINVAL; > ? As we were returning -ENODEV in this case I'd use it. > > + } > > + > > + return 0; > > +} > > + > > static void wait_err(char *msg, int state, unsigned int ctrl, unsigned int intr) > > { > > printk(KERN_ERR "onenand_wait: %s! state %d ctrl 0x%04x intr 0x%04x\n", > > @@ -153,7 +198,7 @@ static int omap2_onenand_wait(struct mtd_info *mtd, int state) > > if (!(syscfg & ONENAND_SYS_CFG1_IOBE)) { > > syscfg |= ONENAND_SYS_CFG1_IOBE; > > write_reg(c, syscfg, ONENAND_REG_SYS_CFG1); > > - if (c->flags & ONENAND_IN_OMAP34XX) > > + if (!c->is_omap2) > > seems like you should have a is_omap3 flag as this quirk is for omap3 only. I was not sure about logic. Quirk is certainly not needed for omap2 and I'm not sure if OneNAND can be connected to omap4 and above. > > /* Add a delay to let GPIO settle */ > > syscfg = read_reg(c, ONENAND_REG_SYS_CFG1); > > } > > @@ -607,30 +652,46 @@ static int omap2_onenand_disable(struct mtd_info *mtd) > > return ret; > > } > > > > +static int omap2_onenand_get_dt(struct device *dev, struct omap2_onenand *c) > > +{ > > + struct device_node *child = dev->of_node; > > + u32 cs, val; > > + > > + if (of_property_read_u32(child, "reg", &cs) < 0) { > > + dev_err(dev, "reg not found in DT\n"); > > + return -EINVAL; > > + } > > + c->gpmc_cs = cs; > > + > > + if (!of_property_read_u32(child, "dma-channel", &val)) > > + c->dma_channel = val; > > + else > > + c->dma_channel = -1; > > + > > + c->is_omap2 = (bool)of_device_get_match_data(dev); > > + > > + /* TODO: gpio, regulator, unlocking */ > > + > > + return 0; > > +} > > + > > static int omap2_onenand_probe(struct platform_device *pdev) > > { > > - struct omap_onenand_platform_data *pdata; > > struct omap2_onenand *c; > > struct onenand_chip *this; > > - int r; > > + int freq, r; > > struct resource *res; > > > > - pdata = dev_get_platdata(&pdev->dev); > > - if (pdata == NULL) { > > - dev_err(&pdev->dev, "platform data missing\n"); > > - return -ENODEV; > > - } > > - > > c = kzalloc(sizeof(struct omap2_onenand), GFP_KERNEL); > > if (!c) > > return -ENOMEM; > > > > + r = omap2_onenand_get_dt(&pdev->dev, c); > > + if (r) > > + goto err_kfree; > > + > > init_completion(&c->irq_done); > > init_completion(&c->dma_done); > > - c->flags = pdata->flags; > > - c->gpmc_cs = pdata->cs; > > - c->gpio_irq = pdata->gpio_irq; > > - c->dma_channel = pdata->dma_channel; > > if (c->dma_channel < 0) { > > /* if -1, don't use DMA */ > > c->gpio_irq = 0; > > @@ -659,16 +720,23 @@ static int omap2_onenand_probe(struct platform_device *pdev) > > goto err_release_mem_region; > > } > > > > - if (pdata->onenand_setup != NULL) { > > - r = pdata->onenand_setup(c->onenand.base, &c->freq); > > - if (r < 0) { > > - dev_err(&pdev->dev, "Onenand platform setup failed: " > > - "%d\n", r); > > - goto err_iounmap; > > - } > > - c->setup = pdata->onenand_setup; > > + set_async_mode(c); > > + freq = get_freq(c); > > + if (!freq) { > > + dev_err(&pdev->dev, > > + "Rate not detected, bad GPMC async timings?\n"); > > + r = -ENODEV; > > + goto err_iounmap; > > } > > > > + r = gpmc_omap_onenand_set_sync_timings(&pdev->dev, c->gpmc_cs, freq); > > + if (r < 0) { > > + dev_err(&pdev->dev, "Cannot set sync timings: %d\n", r); > > + goto err_iounmap; > > + } > > + if (r > 0) > > + set_cfg(c, true, true, r); > > + > > if (c->gpio_irq) { > > if ((r = gpio_request(c->gpio_irq, "OneNAND irq")) < 0) { > > dev_err(&pdev->dev, "Failed to request GPIO%d for " > > @@ -706,27 +774,27 @@ static int omap2_onenand_probe(struct platform_device *pdev) > > > > dev_info(&pdev->dev, "initializing on CS%d, phys base 0x%08lx, virtual " > > "base %p, freq %d MHz\n", c->gpmc_cs, c->phys_base, > > - c->onenand.base, c->freq); > > + c->onenand.base, freq); > > > > c->pdev = pdev; > > c->mtd.priv = &c->onenand; > > > > c->mtd.dev.parent = &pdev->dev; > > - mtd_set_of_node(&c->mtd, pdata->of_node); > > + mtd_set_of_node(&c->mtd, pdev->dev.of_node); > > > > this = &c->onenand; > > if (c->dma_channel >= 0) { > > this->wait = omap2_onenand_wait; > > - if (c->flags & ONENAND_IN_OMAP34XX) { > > - this->read_bufferram = omap3_onenand_read_bufferram; > > - this->write_bufferram = omap3_onenand_write_bufferram; > > - } else { > > + if (c->is_omap2) { > > this->read_bufferram = omap2_onenand_read_bufferram; > > this->write_bufferram = omap2_onenand_write_bufferram; > > + } else { > > + this->read_bufferram = omap3_onenand_read_bufferram; > > + this->write_bufferram = omap3_onenand_write_bufferram; > > } > > } > > > > - if (pdata->regulator_can_sleep) { > > + if (0 /* TODO: pdata->regulator_can_sleep */) { > > c->regulator = regulator_get(&pdev->dev, "vonenand"); > > if (IS_ERR(c->regulator)) { > > dev_err(&pdev->dev, "Failed to get regulator\n"); > > @@ -737,14 +805,13 @@ static int omap2_onenand_probe(struct platform_device *pdev) > > c->onenand.disable = omap2_onenand_disable; > > } > > > > - if (pdata->skip_initial_unlocking) > > + if (0 /* TODO: pdata->skip_initial_unlocking */) > > this->options |= ONENAND_SKIP_INITIAL_UNLOCKING; > > > > if ((r = onenand_scan(&c->mtd, 1)) < 0) > > goto err_release_regulator; > > > > - r = mtd_device_register(&c->mtd, pdata ? pdata->parts : NULL, > > - pdata ? pdata->nr_parts : 0); > > + r = mtd_device_register(&c->mtd, NULL, 0); > > if (r) > > goto err_release_onenand; > > > > @@ -794,12 +861,20 @@ static int omap2_onenand_remove(struct platform_device *pdev) > > return 0; > > } > > > > +static const struct of_device_id omap2_onenand_ids[] = { > > + { .compatible = "ti,omap2-onenand", .data = (void *)true, }, > > + { .compatible = "ti,omap3-onenand", .data = (void *)false, }, > > This doesn't clearly show what we're doing. > > Can we instead have a proper data structure for .data? > If that is an overkill maybe we should avoid the of_data approach and instead do I'll go for overkill in this case. > if (of_device_is_compatible(node, "ti,omap3-nand") > c->need_wait_delay = true; > > > + { }, > > +}; > > +MODULE_DEVICE_TABLE(of, omap2_onenand_ids); > > + > > static struct platform_driver omap2_onenand_driver = { > > .probe = omap2_onenand_probe, > > .remove = omap2_onenand_remove, > > .shutdown = omap2_onenand_shutdown, > > .driver = { > > .name = DRIVER_NAME, > > + .of_match_table = of_match_ptr(omap2_onenand_ids), > > }, > > }; > > > > > > We need to update the DT binding doc as well > devicetree/bindings/mtd$ vi gpmc-onenand.txt > > This should be done in a separate patch. > > -- > cheers, > -roger -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 17/10/17 12:24, Ladislav Michl wrote: > On Tue, Oct 17, 2017 at 11:33:56AM +0300, Roger Quadros wrote: >> Hi, >> >> >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >> >> On 16/10/17 02:22, Ladislav Michl wrote: >> >> missing commit log > > There will be enough future versions to add some :) > >>> Signed-off-by: Ladislav Michl <ladis@linux-mips.org> >>> --- >>> drivers/mtd/onenand/omap2.c | 145 +++++++++++++++++++++++++++++++++----------- >>> 1 file changed, 110 insertions(+), 35 deletions(-) >>> >>> diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c >>> index 24a1388d3031..4811897a4e9f 100644 >>> --- a/drivers/mtd/onenand/omap2.c >>> +++ b/drivers/mtd/onenand/omap2.c >>> @@ -28,6 +28,8 @@ >>> #include <linux/mtd/mtd.h> >>> #include <linux/mtd/onenand.h> >>> #include <linux/mtd/partitions.h> >>> +#include <linux/of_device.h> >>> +#include <linux/omap-gpmc.h> >>> #include <linux/platform_device.h> >>> #include <linux/interrupt.h> >>> #include <linux/delay.h> >>> @@ -38,7 +40,6 @@ >>> #include <linux/gpio.h> >>> >>> #include <asm/mach/flash.h> >>> -#include <linux/platform_data/mtd-onenand-omap2.h> >>> >>> #include <linux/omap-dma.h> >>> >>> @@ -57,10 +58,8 @@ struct omap2_onenand { >>> struct completion irq_done; >>> struct completion dma_done; >>> int dma_channel; >>> - int freq; >>> - int (*setup)(void __iomem *base, int *freq_ptr); >>> struct regulator *regulator; >>> - u8 flags; >>> + bool is_omap2; >>> }; >>> >>> static void omap2_onenand_dma_cb(int lch, u16 ch_status, void *data) >>> @@ -90,6 +89,52 @@ static inline void write_reg(struct omap2_onenand *c, unsigned short value, >>> writew(value, c->onenand.base + reg); >>> } >>> >>> +/* Ensure sync read and sync write are disabled */ >>> +static void set_async_mode(struct omap2_onenand *c) >> >> omap2_onenand_set_async_mode() >> >>> +{ >>> + unsigned short reg = read_reg(c, ONENAND_REG_SYS_CFG1); >> need blank line >> >>> + reg &= ~ONENAND_SYS_CFG1_SYNC_READ & ~ONENAND_SYS_CFG1_SYNC_WRITE; >>> + write_reg(c, reg, ONENAND_REG_SYS_CFG1); >>> +} >>> + >>> +static void set_cfg(struct omap2_onenand *c, bool sr, bool sw, int latency) >> >> omap2_onenand_set_cfg() >> >>> +{ >>> + unsigned short reg = ONENAND_SYS_CFG1_RDY | ONENAND_SYS_CFG1_INT; >>> + >>> + reg |= (latency << ONENAND_SYS_CFG1_BRL_SHIFT) | >>> + ONENAND_SYS_CFG1_BL_16; >>> + if (latency > 5) >>> + reg |= ONENAND_SYS_CFG1_HF; >>> + if (latency > 7) >>> + reg |= ONENAND_SYS_CFG1_VHF; >> >> This will set both HF and VHF flags if latency is greater than 7. Is this what we want? > > That's why patches 1-5 are there, so you can verify... original code is if (gpmc_clk_ns < 15) /* >66MHz */ onenand_flags |= ONENAND_FLAG_HF; else onenand_flags &= ~ONENAND_FLAG_HF; if (gpmc_clk_ns < 12) /* >83MHz */ onenand_flags |= ONENAND_FLAG_VHF; else onenand_flags &= ~ONENAND_FLAG_VHF; if (onenand_flags & ONENAND_FLAG_VHF) latency = 8; else if (onenand_flags & ONENAND_FLAG_HF) latency = 6; else if (gpmc_clk_ns >= 25) /* 40 MHz*/ latency = 3; else latency = 4; which looks like both HF and VHF flags are set for >=83MHz. > >>> + if (sr) >>> + reg |= ONENAND_SYS_CFG1_SYNC_READ; >>> + if (sw) >>> + reg |= ONENAND_SYS_CFG1_SYNC_WRITE; >>> + >>> + write_reg(c, reg, ONENAND_REG_SYS_CFG1); >>> +} >>> + >>> +static int get_freq(struct omap2_onenand *c) >> >> omap2_onenand_get_freq() > > All other helper functions are unprefixed, but will change that. > >>> +{ >>> + unsigned short ver = read_reg(c, ONENAND_REG_VERSION_ID); >>> + >>> + switch ((ver >> 4) & 0xf) { >>> + case 0: >>> + return 40; >>> + case 1: >>> + return 54; >>> + case 2: >>> + return 66; >>> + case 3: >>> + return 83; >>> + case 4: >>> + return 104; >> >> default: >> return -EINVAL; >> ? > > As we were returning -ENODEV in this case I'd use it. > >>> + } >>> + >>> + return 0; >>> +} >>> + >>> static void wait_err(char *msg, int state, unsigned int ctrl, unsigned int intr) >>> { >>> printk(KERN_ERR "onenand_wait: %s! state %d ctrl 0x%04x intr 0x%04x\n", >>> @@ -153,7 +198,7 @@ static int omap2_onenand_wait(struct mtd_info *mtd, int state) >>> if (!(syscfg & ONENAND_SYS_CFG1_IOBE)) { >>> syscfg |= ONENAND_SYS_CFG1_IOBE; >>> write_reg(c, syscfg, ONENAND_REG_SYS_CFG1); >>> - if (c->flags & ONENAND_IN_OMAP34XX) >>> + if (!c->is_omap2) >> >> seems like you should have a is_omap3 flag as this quirk is for omap3 only. > > I was not sure about logic. Quirk is certainly not needed for omap2 and I'm > not sure if OneNAND can be connected to omap4 and above. > all OMAPs can connect to OneNAND. Just that nobody used them beyond omap3. >>> /* Add a delay to let GPIO settle */ >>> syscfg = read_reg(c, ONENAND_REG_SYS_CFG1); >>> } >>> @@ -607,30 +652,46 @@ static int omap2_onenand_disable(struct mtd_info *mtd) >>> return ret; >>> } >>> >>> +static int omap2_onenand_get_dt(struct device *dev, struct omap2_onenand *c) >>> +{ >>> + struct device_node *child = dev->of_node; >>> + u32 cs, val; >>> + >>> + if (of_property_read_u32(child, "reg", &cs) < 0) { >>> + dev_err(dev, "reg not found in DT\n"); >>> + return -EINVAL; >>> + } >>> + c->gpmc_cs = cs; >>> + >>> + if (!of_property_read_u32(child, "dma-channel", &val)) >>> + c->dma_channel = val; >>> + else >>> + c->dma_channel = -1; >>> + >>> + c->is_omap2 = (bool)of_device_get_match_data(dev); >>> + >>> + /* TODO: gpio, regulator, unlocking */ >>> + >>> + return 0; >>> +} >>> + >>> static int omap2_onenand_probe(struct platform_device *pdev) >>> { >>> - struct omap_onenand_platform_data *pdata; >>> struct omap2_onenand *c; >>> struct onenand_chip *this; >>> - int r; >>> + int freq, r; >>> struct resource *res; >>> >>> - pdata = dev_get_platdata(&pdev->dev); >>> - if (pdata == NULL) { >>> - dev_err(&pdev->dev, "platform data missing\n"); >>> - return -ENODEV; >>> - } >>> - >>> c = kzalloc(sizeof(struct omap2_onenand), GFP_KERNEL); >>> if (!c) >>> return -ENOMEM; >>> >>> + r = omap2_onenand_get_dt(&pdev->dev, c); >>> + if (r) >>> + goto err_kfree; >>> + >>> init_completion(&c->irq_done); >>> init_completion(&c->dma_done); >>> - c->flags = pdata->flags; >>> - c->gpmc_cs = pdata->cs; >>> - c->gpio_irq = pdata->gpio_irq; >>> - c->dma_channel = pdata->dma_channel; >>> if (c->dma_channel < 0) { >>> /* if -1, don't use DMA */ >>> c->gpio_irq = 0; >>> @@ -659,16 +720,23 @@ static int omap2_onenand_probe(struct platform_device *pdev) >>> goto err_release_mem_region; >>> } >>> >>> - if (pdata->onenand_setup != NULL) { >>> - r = pdata->onenand_setup(c->onenand.base, &c->freq); >>> - if (r < 0) { >>> - dev_err(&pdev->dev, "Onenand platform setup failed: " >>> - "%d\n", r); >>> - goto err_iounmap; >>> - } >>> - c->setup = pdata->onenand_setup; >>> + set_async_mode(c); >>> + freq = get_freq(c); >>> + if (!freq) { >>> + dev_err(&pdev->dev, >>> + "Rate not detected, bad GPMC async timings?\n"); >>> + r = -ENODEV; >>> + goto err_iounmap; >>> } >>> >>> + r = gpmc_omap_onenand_set_sync_timings(&pdev->dev, c->gpmc_cs, freq); >>> + if (r < 0) { >>> + dev_err(&pdev->dev, "Cannot set sync timings: %d\n", r); >>> + goto err_iounmap; >>> + } >>> + if (r > 0) >>> + set_cfg(c, true, true, r); >>> + >>> if (c->gpio_irq) { >>> if ((r = gpio_request(c->gpio_irq, "OneNAND irq")) < 0) { >>> dev_err(&pdev->dev, "Failed to request GPIO%d for " >>> @@ -706,27 +774,27 @@ static int omap2_onenand_probe(struct platform_device *pdev) >>> >>> dev_info(&pdev->dev, "initializing on CS%d, phys base 0x%08lx, virtual " >>> "base %p, freq %d MHz\n", c->gpmc_cs, c->phys_base, >>> - c->onenand.base, c->freq); >>> + c->onenand.base, freq); >>> >>> c->pdev = pdev; >>> c->mtd.priv = &c->onenand; >>> >>> c->mtd.dev.parent = &pdev->dev; >>> - mtd_set_of_node(&c->mtd, pdata->of_node); >>> + mtd_set_of_node(&c->mtd, pdev->dev.of_node); >>> >>> this = &c->onenand; >>> if (c->dma_channel >= 0) { >>> this->wait = omap2_onenand_wait; >>> - if (c->flags & ONENAND_IN_OMAP34XX) { >>> - this->read_bufferram = omap3_onenand_read_bufferram; >>> - this->write_bufferram = omap3_onenand_write_bufferram; >>> - } else { >>> + if (c->is_omap2) { >>> this->read_bufferram = omap2_onenand_read_bufferram; >>> this->write_bufferram = omap2_onenand_write_bufferram; >>> + } else { >>> + this->read_bufferram = omap3_onenand_read_bufferram; >>> + this->write_bufferram = omap3_onenand_write_bufferram; >>> } >>> } >>> >>> - if (pdata->regulator_can_sleep) { >>> + if (0 /* TODO: pdata->regulator_can_sleep */) { >>> c->regulator = regulator_get(&pdev->dev, "vonenand"); >>> if (IS_ERR(c->regulator)) { >>> dev_err(&pdev->dev, "Failed to get regulator\n"); >>> @@ -737,14 +805,13 @@ static int omap2_onenand_probe(struct platform_device *pdev) >>> c->onenand.disable = omap2_onenand_disable; >>> } >>> >>> - if (pdata->skip_initial_unlocking) >>> + if (0 /* TODO: pdata->skip_initial_unlocking */) >>> this->options |= ONENAND_SKIP_INITIAL_UNLOCKING; >>> >>> if ((r = onenand_scan(&c->mtd, 1)) < 0) >>> goto err_release_regulator; >>> >>> - r = mtd_device_register(&c->mtd, pdata ? pdata->parts : NULL, >>> - pdata ? pdata->nr_parts : 0); >>> + r = mtd_device_register(&c->mtd, NULL, 0); >>> if (r) >>> goto err_release_onenand; >>> >>> @@ -794,12 +861,20 @@ static int omap2_onenand_remove(struct platform_device *pdev) >>> return 0; >>> } >>> >>> +static const struct of_device_id omap2_onenand_ids[] = { >>> + { .compatible = "ti,omap2-onenand", .data = (void *)true, }, >>> + { .compatible = "ti,omap3-onenand", .data = (void *)false, }, >> >> This doesn't clearly show what we're doing. >> >> Can we instead have a proper data structure for .data? >> If that is an overkill maybe we should avoid the of_data approach and instead do > > I'll go for overkill in this case. > >> if (of_device_is_compatible(node, "ti,omap3-nand") >> c->need_wait_delay = true; >> >>> + { }, >>> +}; >>> +MODULE_DEVICE_TABLE(of, omap2_onenand_ids); >>> + >>> static struct platform_driver omap2_onenand_driver = { >>> .probe = omap2_onenand_probe, >>> .remove = omap2_onenand_remove, >>> .shutdown = omap2_onenand_shutdown, >>> .driver = { >>> .name = DRIVER_NAME, >>> + .of_match_table = of_match_ptr(omap2_onenand_ids), >>> }, >>> }; >>> >>> >> >> We need to update the DT binding doc as well >> devicetree/bindings/mtd$ vi gpmc-onenand.txt >> >> This should be done in a separate patch. >>
diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c index 24a1388d3031..4811897a4e9f 100644 --- a/drivers/mtd/onenand/omap2.c +++ b/drivers/mtd/onenand/omap2.c @@ -28,6 +28,8 @@ #include <linux/mtd/mtd.h> #include <linux/mtd/onenand.h> #include <linux/mtd/partitions.h> +#include <linux/of_device.h> +#include <linux/omap-gpmc.h> #include <linux/platform_device.h> #include <linux/interrupt.h> #include <linux/delay.h> @@ -38,7 +40,6 @@ #include <linux/gpio.h> #include <asm/mach/flash.h> -#include <linux/platform_data/mtd-onenand-omap2.h> #include <linux/omap-dma.h> @@ -57,10 +58,8 @@ struct omap2_onenand { struct completion irq_done; struct completion dma_done; int dma_channel; - int freq; - int (*setup)(void __iomem *base, int *freq_ptr); struct regulator *regulator; - u8 flags; + bool is_omap2; }; static void omap2_onenand_dma_cb(int lch, u16 ch_status, void *data) @@ -90,6 +89,52 @@ static inline void write_reg(struct omap2_onenand *c, unsigned short value, writew(value, c->onenand.base + reg); } +/* Ensure sync read and sync write are disabled */ +static void set_async_mode(struct omap2_onenand *c) +{ + unsigned short reg = read_reg(c, ONENAND_REG_SYS_CFG1); + reg &= ~ONENAND_SYS_CFG1_SYNC_READ & ~ONENAND_SYS_CFG1_SYNC_WRITE; + write_reg(c, reg, ONENAND_REG_SYS_CFG1); +} + +static void set_cfg(struct omap2_onenand *c, bool sr, bool sw, int latency) +{ + unsigned short reg = ONENAND_SYS_CFG1_RDY | ONENAND_SYS_CFG1_INT; + + reg |= (latency << ONENAND_SYS_CFG1_BRL_SHIFT) | + ONENAND_SYS_CFG1_BL_16; + if (latency > 5) + reg |= ONENAND_SYS_CFG1_HF; + if (latency > 7) + reg |= ONENAND_SYS_CFG1_VHF; + if (sr) + reg |= ONENAND_SYS_CFG1_SYNC_READ; + if (sw) + reg |= ONENAND_SYS_CFG1_SYNC_WRITE; + + write_reg(c, reg, ONENAND_REG_SYS_CFG1); +} + +static int get_freq(struct omap2_onenand *c) +{ + unsigned short ver = read_reg(c, ONENAND_REG_VERSION_ID); + + switch ((ver >> 4) & 0xf) { + case 0: + return 40; + case 1: + return 54; + case 2: + return 66; + case 3: + return 83; + case 4: + return 104; + } + + return 0; +} + static void wait_err(char *msg, int state, unsigned int ctrl, unsigned int intr) { printk(KERN_ERR "onenand_wait: %s! state %d ctrl 0x%04x intr 0x%04x\n", @@ -153,7 +198,7 @@ static int omap2_onenand_wait(struct mtd_info *mtd, int state) if (!(syscfg & ONENAND_SYS_CFG1_IOBE)) { syscfg |= ONENAND_SYS_CFG1_IOBE; write_reg(c, syscfg, ONENAND_REG_SYS_CFG1); - if (c->flags & ONENAND_IN_OMAP34XX) + if (!c->is_omap2) /* Add a delay to let GPIO settle */ syscfg = read_reg(c, ONENAND_REG_SYS_CFG1); } @@ -607,30 +652,46 @@ static int omap2_onenand_disable(struct mtd_info *mtd) return ret; } +static int omap2_onenand_get_dt(struct device *dev, struct omap2_onenand *c) +{ + struct device_node *child = dev->of_node; + u32 cs, val; + + if (of_property_read_u32(child, "reg", &cs) < 0) { + dev_err(dev, "reg not found in DT\n"); + return -EINVAL; + } + c->gpmc_cs = cs; + + if (!of_property_read_u32(child, "dma-channel", &val)) + c->dma_channel = val; + else + c->dma_channel = -1; + + c->is_omap2 = (bool)of_device_get_match_data(dev); + + /* TODO: gpio, regulator, unlocking */ + + return 0; +} + static int omap2_onenand_probe(struct platform_device *pdev) { - struct omap_onenand_platform_data *pdata; struct omap2_onenand *c; struct onenand_chip *this; - int r; + int freq, r; struct resource *res; - pdata = dev_get_platdata(&pdev->dev); - if (pdata == NULL) { - dev_err(&pdev->dev, "platform data missing\n"); - return -ENODEV; - } - c = kzalloc(sizeof(struct omap2_onenand), GFP_KERNEL); if (!c) return -ENOMEM; + r = omap2_onenand_get_dt(&pdev->dev, c); + if (r) + goto err_kfree; + init_completion(&c->irq_done); init_completion(&c->dma_done); - c->flags = pdata->flags; - c->gpmc_cs = pdata->cs; - c->gpio_irq = pdata->gpio_irq; - c->dma_channel = pdata->dma_channel; if (c->dma_channel < 0) { /* if -1, don't use DMA */ c->gpio_irq = 0; @@ -659,16 +720,23 @@ static int omap2_onenand_probe(struct platform_device *pdev) goto err_release_mem_region; } - if (pdata->onenand_setup != NULL) { - r = pdata->onenand_setup(c->onenand.base, &c->freq); - if (r < 0) { - dev_err(&pdev->dev, "Onenand platform setup failed: " - "%d\n", r); - goto err_iounmap; - } - c->setup = pdata->onenand_setup; + set_async_mode(c); + freq = get_freq(c); + if (!freq) { + dev_err(&pdev->dev, + "Rate not detected, bad GPMC async timings?\n"); + r = -ENODEV; + goto err_iounmap; } + r = gpmc_omap_onenand_set_sync_timings(&pdev->dev, c->gpmc_cs, freq); + if (r < 0) { + dev_err(&pdev->dev, "Cannot set sync timings: %d\n", r); + goto err_iounmap; + } + if (r > 0) + set_cfg(c, true, true, r); + if (c->gpio_irq) { if ((r = gpio_request(c->gpio_irq, "OneNAND irq")) < 0) { dev_err(&pdev->dev, "Failed to request GPIO%d for " @@ -706,27 +774,27 @@ static int omap2_onenand_probe(struct platform_device *pdev) dev_info(&pdev->dev, "initializing on CS%d, phys base 0x%08lx, virtual " "base %p, freq %d MHz\n", c->gpmc_cs, c->phys_base, - c->onenand.base, c->freq); + c->onenand.base, freq); c->pdev = pdev; c->mtd.priv = &c->onenand; c->mtd.dev.parent = &pdev->dev; - mtd_set_of_node(&c->mtd, pdata->of_node); + mtd_set_of_node(&c->mtd, pdev->dev.of_node); this = &c->onenand; if (c->dma_channel >= 0) { this->wait = omap2_onenand_wait; - if (c->flags & ONENAND_IN_OMAP34XX) { - this->read_bufferram = omap3_onenand_read_bufferram; - this->write_bufferram = omap3_onenand_write_bufferram; - } else { + if (c->is_omap2) { this->read_bufferram = omap2_onenand_read_bufferram; this->write_bufferram = omap2_onenand_write_bufferram; + } else { + this->read_bufferram = omap3_onenand_read_bufferram; + this->write_bufferram = omap3_onenand_write_bufferram; } } - if (pdata->regulator_can_sleep) { + if (0 /* TODO: pdata->regulator_can_sleep */) { c->regulator = regulator_get(&pdev->dev, "vonenand"); if (IS_ERR(c->regulator)) { dev_err(&pdev->dev, "Failed to get regulator\n"); @@ -737,14 +805,13 @@ static int omap2_onenand_probe(struct platform_device *pdev) c->onenand.disable = omap2_onenand_disable; } - if (pdata->skip_initial_unlocking) + if (0 /* TODO: pdata->skip_initial_unlocking */) this->options |= ONENAND_SKIP_INITIAL_UNLOCKING; if ((r = onenand_scan(&c->mtd, 1)) < 0) goto err_release_regulator; - r = mtd_device_register(&c->mtd, pdata ? pdata->parts : NULL, - pdata ? pdata->nr_parts : 0); + r = mtd_device_register(&c->mtd, NULL, 0); if (r) goto err_release_onenand; @@ -794,12 +861,20 @@ static int omap2_onenand_remove(struct platform_device *pdev) return 0; } +static const struct of_device_id omap2_onenand_ids[] = { + { .compatible = "ti,omap2-onenand", .data = (void *)true, }, + { .compatible = "ti,omap3-onenand", .data = (void *)false, }, + { }, +}; +MODULE_DEVICE_TABLE(of, omap2_onenand_ids); + static struct platform_driver omap2_onenand_driver = { .probe = omap2_onenand_probe, .remove = omap2_onenand_remove, .shutdown = omap2_onenand_shutdown, .driver = { .name = DRIVER_NAME, + .of_match_table = of_match_ptr(omap2_onenand_ids), }, };
Signed-off-by: Ladislav Michl <ladis@linux-mips.org> --- drivers/mtd/onenand/omap2.c | 145 +++++++++++++++++++++++++++++++++----------- 1 file changed, 110 insertions(+), 35 deletions(-)