| Message ID | 20171017150358.znjlwp2cazngtpqw@lenoch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 17/10/17 18:03, Ladislav Michl wrote: > On Tue, Oct 17, 2017 at 01:20:11PM +0300, Roger Quadros wrote: >> 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. > > Before describing new binding, we should agree, how it will look like. > Bellow is semi-v2 with GPIO, regulator and ONENAND_SKIP_INITIAL_UNLOCKING > done - somehow. > GPIO is pretty usual, unlocking could be probably defined at common > onenand level and regulator should be probably also described in DT. > > Ideas? Looks good to me. One question below. > > diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c > index 24a1388d3031..7ab56383ff7a 100644 > --- a/drivers/mtd/onenand/omap2.c > +++ b/drivers/mtd/onenand/omap2.c > @@ -28,17 +28,18 @@ > #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> > #include <linux/dma-mapping.h> > #include <linux/io.h> > #include <linux/slab.h> > +#include <linux/gpio/consumer.h> > #include <linux/regulator/consumer.h> > -#include <linux/gpio.h> > > #include <asm/mach/flash.h> > -#include <linux/platform_data/mtd-onenand-omap2.h> > > #include <linux/omap-dma.h> > > @@ -50,17 +51,22 @@ struct omap2_onenand { > struct platform_device *pdev; > int gpmc_cs; > unsigned long phys_base; > - unsigned int mem_size; > - int gpio_irq; > + struct gpio_desc *gpiod; > struct mtd_info mtd; > struct onenand_chip 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 gpio_quirk; > +}; > + > +struct omap2_onenand_devdata { > + bool gpio_quirk; > + int (*read_bufferram)(struct mtd_info *mtd, int area, > + unsigned char *buffer, int offset, size_t count); > + int (*write_bufferram)(struct mtd_info *mtd, int area, > + const unsigned char *buffer, int offset, size_t count); > }; > > static void omap2_onenand_dma_cb(int lch, u16 ch_status, void *data) > @@ -90,6 +96,55 @@ 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 omap2_onenand_set_async_mode(struct omap2_onenand *c) > +{ > + unsigned short reg; > + > + 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 omap2_onenand_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 omap2_onenand_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 -ENODEV; > +} > + > 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,19 +208,19 @@ 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->gpio_quirk) > /* Add a delay to let GPIO settle */ > syscfg = read_reg(c, ONENAND_REG_SYS_CFG1); > } > > reinit_completion(&c->irq_done); > - if (c->gpio_irq) { > - result = gpio_get_value(c->gpio_irq); > - if (result == -1) { > + if (c->gpiod) { > + result = gpiod_get_value(c->gpiod); > + if (result < 0) { > ctrl = read_reg(c, ONENAND_REG_CTRL_STATUS); > intr = read_reg(c, ONENAND_REG_INTERRUPT); > wait_err("gpio error", state, ctrl, intr); > - return -EIO; > + return result; > } > } else > result = 0; > @@ -609,142 +664,143 @@ static int omap2_onenand_disable(struct mtd_info *mtd) > > static int omap2_onenand_probe(struct platform_device *pdev) > { > - struct omap_onenand_platform_data *pdata; > + u32 val; > + int freq, r; > + unsigned int mem_size; > + struct resource *res; > struct omap2_onenand *c; > struct onenand_chip *this; > - int r; > - struct resource *res; > + const struct omap2_onenand_devdata *devdata; > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (res == NULL) { > + dev_err(dev, "error getting memory resource\n"); > + return -EINVAL; > + } > > - pdata = dev_get_platdata(&pdev->dev); > - if (pdata == NULL) { > - dev_err(&pdev->dev, "platform data missing\n"); > - return -ENODEV; > + r = of_property_read_u32(np, "reg", &val); > + if (r) { > + dev_err(dev, "reg not found in DT\n"); > + return r; > } > > - c = kzalloc(sizeof(struct omap2_onenand), GFP_KERNEL); > + c = devm_kzalloc(dev, sizeof(struct omap2_onenand), GFP_KERNEL); > if (!c) > return -ENOMEM; > > - 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; > - } > - > - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - if (res == NULL) { > - r = -EINVAL; > - dev_err(&pdev->dev, "error getting memory resource\n"); > - goto err_kfree; > - } > + devdata = of_device_get_match_data(dev); > + this = &c->onenand; > > + c->gpmc_cs = val; > + c->dma_channel = -1; > + c->gpio_quirk = devdata->gpio_quirk; > c->phys_base = res->start; > - c->mem_size = resource_size(res); > - > - if (request_mem_region(c->phys_base, c->mem_size, > - pdev->dev.driver->name) == NULL) { > - dev_err(&pdev->dev, "Cannot reserve memory region at 0x%08lx, size: 0x%x\n", > - c->phys_base, c->mem_size); > - r = -EBUSY; > - goto err_kfree; > - } > - c->onenand.base = ioremap(c->phys_base, c->mem_size); > - if (c->onenand.base == NULL) { > - r = -ENOMEM; > - 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; > - } > + mem_size = resource_size(res); > + if (devm_request_mem_region(dev, c->phys_base, mem_size, > + dev->driver->name) == NULL) { > > - if (c->gpio_irq) { > - if ((r = gpio_request(c->gpio_irq, "OneNAND irq")) < 0) { > - dev_err(&pdev->dev, "Failed to request GPIO%d for " > - "OneNAND\n", c->gpio_irq); > - goto err_iounmap; > - } > - gpio_direction_input(c->gpio_irq); > - > - if ((r = request_irq(gpio_to_irq(c->gpio_irq), > - omap2_onenand_interrupt, IRQF_TRIGGER_RISING, > - pdev->dev.driver->name, c)) < 0) > - goto err_release_gpio; > + dev_err(dev, "Cannot reserve memory region at 0x%08lx, size: 0x%x\n", > + c->phys_base, mem_size); > + return -EBUSY; > } > + c->onenand.base = devm_ioremap(dev, c->phys_base, mem_size); > + if (c->onenand.base == NULL) > + return -ENOMEM; > > - if (c->dma_channel >= 0) { > - r = omap_request_dma(0, pdev->dev.driver->name, > - omap2_onenand_dma_cb, (void *) c, > - &c->dma_channel); > - if (r == 0) { > - omap_set_dma_write_mode(c->dma_channel, > - OMAP_DMA_WRITE_NON_POSTED); > - omap_set_dma_src_data_pack(c->dma_channel, 1); > - omap_set_dma_src_burst_mode(c->dma_channel, > - OMAP_DMA_DATA_BURST_8); > - omap_set_dma_dest_data_pack(c->dma_channel, 1); > - omap_set_dma_dest_burst_mode(c->dma_channel, > - OMAP_DMA_DATA_BURST_8); > - } else { > - dev_info(&pdev->dev, > + if (!of_property_read_u32(np, "dma-channel", &val)) { > + c->dma_channel = val; > + r = omap_request_dma(0, dev->driver->name, > + omap2_onenand_dma_cb, (void *) c, > + &c->dma_channel); > + if (r) { > + dev_info(dev, > "failed to allocate DMA for OneNAND, " > "using PIO instead\n"); > c->dma_channel = -1; > } > } > > - 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->pdev = pdev; > - c->mtd.priv = &c->onenand; > - > - c->mtd.dev.parent = &pdev->dev; > - mtd_set_of_node(&c->mtd, pdata->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 { > - this->read_bufferram = omap2_onenand_read_bufferram; > - this->write_bufferram = omap2_onenand_write_bufferram; > + this->read_bufferram = devdata->read_bufferram; > + this->write_bufferram = devdata->write_bufferram; > + > + c->gpiod = devm_gpiod_get(dev, "OneNAND irq", GPIOD_IN); > + if (IS_ERR(c->gpiod)) { > + r = PTR_ERR(c->gpiod); > + /* Just try again if this happens */ > + if (r != -EPROBE_DEFER) > + dev_err(dev, "error getting gpio (%d)\n", r); > + goto err_release_dma; > } > + r = devm_request_irq(dev, gpiod_to_irq(c->gpiod), > + omap2_onenand_interrupt, IRQF_TRIGGER_RISING, > + "OneNAND irq", c); > + if (r) > + goto err_release_dma; > } > > - if (pdata->regulator_can_sleep) { > - c->regulator = regulator_get(&pdev->dev, "vonenand"); > + if (of_property_read_bool(np, "skip-initial-unlocking")) > + this->options |= ONENAND_SKIP_INITIAL_UNLOCKING; > + > + if (of_property_read_bool(np, "regulator-can-sleep")) { why do we need "regulator-can-sleep" property? > + c->regulator = devm_regulator_get(dev, "vonenand"); how about just "vdd" instead of "vonenand" ? > if (IS_ERR(c->regulator)) { > - dev_err(&pdev->dev, "Failed to get regulator\n"); > r = PTR_ERR(c->regulator); > + dev_err(dev, "failed to get regulator (%d)\n", r); > goto err_release_dma; > } > - c->onenand.enable = omap2_onenand_enable; > - c->onenand.disable = omap2_onenand_disable; > + this->enable = omap2_onenand_enable; > + this->disable = omap2_onenand_disable; > } > > - if (pdata->skip_initial_unlocking) > - this->options |= ONENAND_SKIP_INITIAL_UNLOCKING; > + init_completion(&c->irq_done); > + init_completion(&c->dma_done); > + > + omap2_onenand_set_async_mode(c); > + freq = omap2_onenand_get_freq(c); > + if (freq < 0) { > + dev_err(&pdev->dev, > + "Rate not detected, bad GPMC async timings?\n"); > + r = freq; > + goto err_release_dma; > + } > + > + 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_release_dma; > + } > + if (r > 0) > + omap2_onenand_set_cfg(c, true, true, r); > + > + if (c->dma_channel >= 0) { > + omap_set_dma_write_mode(c->dma_channel, > + OMAP_DMA_WRITE_NON_POSTED); > + omap_set_dma_src_data_pack(c->dma_channel, 1); > + omap_set_dma_src_burst_mode(c->dma_channel, > + OMAP_DMA_DATA_BURST_8); > + omap_set_dma_dest_data_pack(c->dma_channel, 1); > + omap_set_dma_dest_burst_mode(c->dma_channel, > + OMAP_DMA_DATA_BURST_8); > + } > + > + 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, freq); > + > + c->pdev = pdev; > + c->mtd.priv = &c->onenand; > + c->mtd.dev.parent = &pdev->dev; > + mtd_set_of_node(&c->mtd, pdev->dev.of_node); > > if ((r = onenand_scan(&c->mtd, 1)) < 0) > - goto err_release_regulator; > + goto err_release_dma; > > - 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; > > @@ -754,22 +810,9 @@ static int omap2_onenand_probe(struct platform_device *pdev) > > err_release_onenand: > onenand_release(&c->mtd); > -err_release_regulator: > - regulator_put(c->regulator); > err_release_dma: > if (c->dma_channel != -1) > omap_free_dma(c->dma_channel); > - if (c->gpio_irq) > - free_irq(gpio_to_irq(c->gpio_irq), c); > -err_release_gpio: > - if (c->gpio_irq) > - gpio_free(c->gpio_irq); > -err_iounmap: > - iounmap(c->onenand.base); > -err_release_mem_region: > - release_mem_region(c->phys_base, c->mem_size); > -err_kfree: > - kfree(c); > > return r; > } > @@ -779,27 +822,43 @@ static int omap2_onenand_remove(struct platform_device *pdev) > struct omap2_onenand *c = dev_get_drvdata(&pdev->dev); > > onenand_release(&c->mtd); > - regulator_put(c->regulator); > if (c->dma_channel != -1) > omap_free_dma(c->dma_channel); > omap2_onenand_shutdown(pdev); > - if (c->gpio_irq) { > - free_irq(gpio_to_irq(c->gpio_irq), c); > - gpio_free(c->gpio_irq); > - } > - iounmap(c->onenand.base); > - release_mem_region(c->phys_base, c->mem_size); > - kfree(c); > > return 0; > } > > +static const struct omap2_onenand_devdata omap2_devdata = { > + .gpio_quirk = false, > + .read_bufferram = omap2_onenand_read_bufferram, > + .write_bufferram = omap2_onenand_write_bufferram, > +}; > + > +static const struct omap2_onenand_devdata omap3_devdata = { > + .gpio_quirk = true, > + .read_bufferram = omap3_onenand_read_bufferram, > + .write_bufferram = omap3_onenand_write_bufferram, > +}; > + > +static const struct of_device_id omap2_onenand_ids[] = { > + { > + .compatible = "ti,omap2-onenand", > + .data = &omap2_devdata, > + }, { > + .compatible = "ti,omap3-onenand", > + .data = &omap3_devdata, > + }, {}, > +}; > +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), > }, > }; > >
Hi, On Thu, Oct 19, 2017 at 03:56:30PM +0300, Roger Quadros wrote: > Hi, > > On 17/10/17 18:03, Ladislav Michl wrote: [snip] > > Before describing new binding, we should agree, how it will look like. > > Bellow is semi-v2 with GPIO, regulator and ONENAND_SKIP_INITIAL_UNLOCKING > > done - somehow. > > GPIO is pretty usual, unlocking could be probably defined at common > > onenand level and regulator should be probably also described in DT. > > > > Ideas? > > Looks good to me. One question below. > > > > > diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c > > index 24a1388d3031..7ab56383ff7a 100644 > > --- a/drivers/mtd/onenand/omap2.c > > +++ b/drivers/mtd/onenand/omap2.c > > @@ -28,17 +28,18 @@ > > #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> > > #include <linux/dma-mapping.h> > > #include <linux/io.h> > > #include <linux/slab.h> > > +#include <linux/gpio/consumer.h> > > #include <linux/regulator/consumer.h> > > -#include <linux/gpio.h> > > > > #include <asm/mach/flash.h> > > -#include <linux/platform_data/mtd-onenand-omap2.h> > > > > #include <linux/omap-dma.h> > > > > @@ -50,17 +51,22 @@ struct omap2_onenand { > > struct platform_device *pdev; > > int gpmc_cs; > > unsigned long phys_base; > > - unsigned int mem_size; > > - int gpio_irq; > > + struct gpio_desc *gpiod; > > struct mtd_info mtd; > > struct onenand_chip 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 gpio_quirk; > > +}; > > + > > +struct omap2_onenand_devdata { > > + bool gpio_quirk; > > + int (*read_bufferram)(struct mtd_info *mtd, int area, > > + unsigned char *buffer, int offset, size_t count); > > + int (*write_bufferram)(struct mtd_info *mtd, int area, > > + const unsigned char *buffer, int offset, size_t count); > > }; > > > > static void omap2_onenand_dma_cb(int lch, u16 ch_status, void *data) > > @@ -90,6 +96,55 @@ 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 omap2_onenand_set_async_mode(struct omap2_onenand *c) > > +{ > > + unsigned short reg; > > + > > + 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 omap2_onenand_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 omap2_onenand_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 -ENODEV; > > +} > > + > > 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,19 +208,19 @@ 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->gpio_quirk) > > /* Add a delay to let GPIO settle */ > > syscfg = read_reg(c, ONENAND_REG_SYS_CFG1); > > } > > > > reinit_completion(&c->irq_done); > > - if (c->gpio_irq) { > > - result = gpio_get_value(c->gpio_irq); > > - if (result == -1) { > > + if (c->gpiod) { > > + result = gpiod_get_value(c->gpiod); > > + if (result < 0) { > > ctrl = read_reg(c, ONENAND_REG_CTRL_STATUS); > > intr = read_reg(c, ONENAND_REG_INTERRUPT); > > wait_err("gpio error", state, ctrl, intr); > > - return -EIO; > > + return result; > > } > > } else > > result = 0; > > @@ -609,142 +664,143 @@ static int omap2_onenand_disable(struct mtd_info *mtd) > > > > static int omap2_onenand_probe(struct platform_device *pdev) > > { > > - struct omap_onenand_platform_data *pdata; > > + u32 val; > > + int freq, r; > > + unsigned int mem_size; > > + struct resource *res; > > struct omap2_onenand *c; > > struct onenand_chip *this; > > - int r; > > - struct resource *res; > > + const struct omap2_onenand_devdata *devdata; > > + struct device *dev = &pdev->dev; > > + struct device_node *np = dev->of_node; > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (res == NULL) { > > + dev_err(dev, "error getting memory resource\n"); > > + return -EINVAL; > > + } > > > > - pdata = dev_get_platdata(&pdev->dev); > > - if (pdata == NULL) { > > - dev_err(&pdev->dev, "platform data missing\n"); > > - return -ENODEV; > > + r = of_property_read_u32(np, "reg", &val); > > + if (r) { > > + dev_err(dev, "reg not found in DT\n"); > > + return r; > > } > > > > - c = kzalloc(sizeof(struct omap2_onenand), GFP_KERNEL); > > + c = devm_kzalloc(dev, sizeof(struct omap2_onenand), GFP_KERNEL); > > if (!c) > > return -ENOMEM; > > > > - 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; > > - } > > - > > - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > - if (res == NULL) { > > - r = -EINVAL; > > - dev_err(&pdev->dev, "error getting memory resource\n"); > > - goto err_kfree; > > - } > > + devdata = of_device_get_match_data(dev); > > + this = &c->onenand; > > > > + c->gpmc_cs = val; > > + c->dma_channel = -1; > > + c->gpio_quirk = devdata->gpio_quirk; > > c->phys_base = res->start; > > - c->mem_size = resource_size(res); > > - > > - if (request_mem_region(c->phys_base, c->mem_size, > > - pdev->dev.driver->name) == NULL) { > > - dev_err(&pdev->dev, "Cannot reserve memory region at 0x%08lx, size: 0x%x\n", > > - c->phys_base, c->mem_size); > > - r = -EBUSY; > > - goto err_kfree; > > - } > > - c->onenand.base = ioremap(c->phys_base, c->mem_size); > > - if (c->onenand.base == NULL) { > > - r = -ENOMEM; > > - 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; > > - } > > + mem_size = resource_size(res); > > + if (devm_request_mem_region(dev, c->phys_base, mem_size, > > + dev->driver->name) == NULL) { > > > > - if (c->gpio_irq) { > > - if ((r = gpio_request(c->gpio_irq, "OneNAND irq")) < 0) { > > - dev_err(&pdev->dev, "Failed to request GPIO%d for " > > - "OneNAND\n", c->gpio_irq); > > - goto err_iounmap; > > - } > > - gpio_direction_input(c->gpio_irq); > > - > > - if ((r = request_irq(gpio_to_irq(c->gpio_irq), > > - omap2_onenand_interrupt, IRQF_TRIGGER_RISING, > > - pdev->dev.driver->name, c)) < 0) > > - goto err_release_gpio; > > + dev_err(dev, "Cannot reserve memory region at 0x%08lx, size: 0x%x\n", > > + c->phys_base, mem_size); > > + return -EBUSY; > > } > > + c->onenand.base = devm_ioremap(dev, c->phys_base, mem_size); > > + if (c->onenand.base == NULL) > > + return -ENOMEM; > > > > - if (c->dma_channel >= 0) { > > - r = omap_request_dma(0, pdev->dev.driver->name, > > - omap2_onenand_dma_cb, (void *) c, > > - &c->dma_channel); > > - if (r == 0) { > > - omap_set_dma_write_mode(c->dma_channel, > > - OMAP_DMA_WRITE_NON_POSTED); > > - omap_set_dma_src_data_pack(c->dma_channel, 1); > > - omap_set_dma_src_burst_mode(c->dma_channel, > > - OMAP_DMA_DATA_BURST_8); > > - omap_set_dma_dest_data_pack(c->dma_channel, 1); > > - omap_set_dma_dest_burst_mode(c->dma_channel, > > - OMAP_DMA_DATA_BURST_8); > > - } else { > > - dev_info(&pdev->dev, > > + if (!of_property_read_u32(np, "dma-channel", &val)) { > > + c->dma_channel = val; > > + r = omap_request_dma(0, dev->driver->name, > > + omap2_onenand_dma_cb, (void *) c, > > + &c->dma_channel); > > + if (r) { > > + dev_info(dev, > > "failed to allocate DMA for OneNAND, " > > "using PIO instead\n"); > > c->dma_channel = -1; > > } > > } > > > > - 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->pdev = pdev; > > - c->mtd.priv = &c->onenand; > > - > > - c->mtd.dev.parent = &pdev->dev; > > - mtd_set_of_node(&c->mtd, pdata->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 { > > - this->read_bufferram = omap2_onenand_read_bufferram; > > - this->write_bufferram = omap2_onenand_write_bufferram; > > + this->read_bufferram = devdata->read_bufferram; > > + this->write_bufferram = devdata->write_bufferram; > > + > > + c->gpiod = devm_gpiod_get(dev, "OneNAND irq", GPIOD_IN); > > + if (IS_ERR(c->gpiod)) { > > + r = PTR_ERR(c->gpiod); > > + /* Just try again if this happens */ > > + if (r != -EPROBE_DEFER) > > + dev_err(dev, "error getting gpio (%d)\n", r); > > + goto err_release_dma; > > } > > + r = devm_request_irq(dev, gpiod_to_irq(c->gpiod), > > + omap2_onenand_interrupt, IRQF_TRIGGER_RISING, > > + "OneNAND irq", c); > > + if (r) > > + goto err_release_dma; > > } > > > > - if (pdata->regulator_can_sleep) { > > - c->regulator = regulator_get(&pdev->dev, "vonenand"); > > + if (of_property_read_bool(np, "skip-initial-unlocking")) > > + this->options |= ONENAND_SKIP_INITIAL_UNLOCKING; > > + > > + if (of_property_read_bool(np, "regulator-can-sleep")) { > > why do we need "regulator-can-sleep" property? Comes from commit 9ac4e613a88d ("mtd: OneNAND: OMAP2/3: prevent regulator sleeping while OneNAND is in use"). Now looking at it, we do not need such property, just a regulator to use - driver will use regulator if there is regulator consumer reference in onenand node. > > + c->regulator = devm_regulator_get(dev, "vonenand"); > > how about just "vdd" instead of "vonenand" ? Well, other subsystems are using this naming scheme, ie. vmmc, vwlan, vcpu0, etc. But I do not have any strong preference here. So with regulator-can-sleep property drop, we could simply use: devm_regulator_get_optional(dev, "vonenand"); We still need to privide something to control initial unlocking, see commit c93ff6bf16523d ("mtd: omap: add new variable to platform data to control onenand unlocking"). How ever it not used anywhere, so maybye just drop to be readded later once needed? > > if (IS_ERR(c->regulator)) { > > - dev_err(&pdev->dev, "Failed to get regulator\n"); > > r = PTR_ERR(c->regulator); > > + dev_err(dev, "failed to get regulator (%d)\n", r); > > goto err_release_dma; > > } > > - c->onenand.enable = omap2_onenand_enable; > > - c->onenand.disable = omap2_onenand_disable; > > + this->enable = omap2_onenand_enable; > > + this->disable = omap2_onenand_disable; > > } > > > > - if (pdata->skip_initial_unlocking) > > - this->options |= ONENAND_SKIP_INITIAL_UNLOCKING; > > + init_completion(&c->irq_done); > > + init_completion(&c->dma_done); > > + > > + omap2_onenand_set_async_mode(c); > > + freq = omap2_onenand_get_freq(c); > > + if (freq < 0) { > > + dev_err(&pdev->dev, > > + "Rate not detected, bad GPMC async timings?\n"); > > + r = freq; > > + goto err_release_dma; > > + } > > + > > + 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_release_dma; > > + } > > + if (r > 0) > > + omap2_onenand_set_cfg(c, true, true, r); > > + > > + if (c->dma_channel >= 0) { > > + omap_set_dma_write_mode(c->dma_channel, > > + OMAP_DMA_WRITE_NON_POSTED); > > + omap_set_dma_src_data_pack(c->dma_channel, 1); > > + omap_set_dma_src_burst_mode(c->dma_channel, > > + OMAP_DMA_DATA_BURST_8); > > + omap_set_dma_dest_data_pack(c->dma_channel, 1); > > + omap_set_dma_dest_burst_mode(c->dma_channel, > > + OMAP_DMA_DATA_BURST_8); > > + } > > + > > + 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, freq); > > + > > + c->pdev = pdev; > > + c->mtd.priv = &c->onenand; > > + c->mtd.dev.parent = &pdev->dev; > > + mtd_set_of_node(&c->mtd, pdev->dev.of_node); > > > > if ((r = onenand_scan(&c->mtd, 1)) < 0) > > - goto err_release_regulator; > > + goto err_release_dma; > > > > - 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; > > > > @@ -754,22 +810,9 @@ static int omap2_onenand_probe(struct platform_device *pdev) > > > > err_release_onenand: > > onenand_release(&c->mtd); > > -err_release_regulator: > > - regulator_put(c->regulator); > > err_release_dma: > > if (c->dma_channel != -1) > > omap_free_dma(c->dma_channel); > > - if (c->gpio_irq) > > - free_irq(gpio_to_irq(c->gpio_irq), c); > > -err_release_gpio: > > - if (c->gpio_irq) > > - gpio_free(c->gpio_irq); > > -err_iounmap: > > - iounmap(c->onenand.base); > > -err_release_mem_region: > > - release_mem_region(c->phys_base, c->mem_size); > > -err_kfree: > > - kfree(c); > > > > return r; > > } > > @@ -779,27 +822,43 @@ static int omap2_onenand_remove(struct platform_device *pdev) > > struct omap2_onenand *c = dev_get_drvdata(&pdev->dev); > > > > onenand_release(&c->mtd); > > - regulator_put(c->regulator); > > if (c->dma_channel != -1) > > omap_free_dma(c->dma_channel); > > omap2_onenand_shutdown(pdev); > > - if (c->gpio_irq) { > > - free_irq(gpio_to_irq(c->gpio_irq), c); > > - gpio_free(c->gpio_irq); > > - } > > - iounmap(c->onenand.base); > > - release_mem_region(c->phys_base, c->mem_size); > > - kfree(c); > > > > return 0; > > } > > > > +static const struct omap2_onenand_devdata omap2_devdata = { > > + .gpio_quirk = false, > > + .read_bufferram = omap2_onenand_read_bufferram, > > + .write_bufferram = omap2_onenand_write_bufferram, > > +}; > > + > > +static const struct omap2_onenand_devdata omap3_devdata = { > > + .gpio_quirk = true, > > + .read_bufferram = omap3_onenand_read_bufferram, > > + .write_bufferram = omap3_onenand_write_bufferram, > > +}; > > + > > +static const struct of_device_id omap2_onenand_ids[] = { > > + { > > + .compatible = "ti,omap2-onenand", > > + .data = &omap2_devdata, > > + }, { > > + .compatible = "ti,omap3-onenand", > > + .data = &omap3_devdata, > > + }, {}, > > +}; > > +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), > > }, > > }; > > > > > > -- > cheers, > -roger > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki -- 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
On 19/10/17 16:42, Ladislav Michl wrote: > Hi, > > On Thu, Oct 19, 2017 at 03:56:30PM +0300, Roger Quadros wrote: >> Hi, >> >> On 17/10/17 18:03, Ladislav Michl wrote: > [snip] >>> Before describing new binding, we should agree, how it will look like. >>> Bellow is semi-v2 with GPIO, regulator and ONENAND_SKIP_INITIAL_UNLOCKING >>> done - somehow. >>> GPIO is pretty usual, unlocking could be probably defined at common >>> onenand level and regulator should be probably also described in DT. >>> >>> Ideas? >> >> Looks good to me. One question below. >> >>> >>> diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c >>> index 24a1388d3031..7ab56383ff7a 100644 >>> --- a/drivers/mtd/onenand/omap2.c >>> +++ b/drivers/mtd/onenand/omap2.c >>> @@ -28,17 +28,18 @@ >>> #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> >>> #include <linux/dma-mapping.h> >>> #include <linux/io.h> >>> #include <linux/slab.h> >>> +#include <linux/gpio/consumer.h> >>> #include <linux/regulator/consumer.h> >>> -#include <linux/gpio.h> >>> >>> #include <asm/mach/flash.h> >>> -#include <linux/platform_data/mtd-onenand-omap2.h> >>> >>> #include <linux/omap-dma.h> >>> >>> @@ -50,17 +51,22 @@ struct omap2_onenand { >>> struct platform_device *pdev; >>> int gpmc_cs; >>> unsigned long phys_base; >>> - unsigned int mem_size; >>> - int gpio_irq; >>> + struct gpio_desc *gpiod; >>> struct mtd_info mtd; >>> struct onenand_chip 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 gpio_quirk; >>> +}; >>> + >>> +struct omap2_onenand_devdata { >>> + bool gpio_quirk; >>> + int (*read_bufferram)(struct mtd_info *mtd, int area, >>> + unsigned char *buffer, int offset, size_t count); >>> + int (*write_bufferram)(struct mtd_info *mtd, int area, >>> + const unsigned char *buffer, int offset, size_t count); >>> }; >>> >>> static void omap2_onenand_dma_cb(int lch, u16 ch_status, void *data) >>> @@ -90,6 +96,55 @@ 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 omap2_onenand_set_async_mode(struct omap2_onenand *c) >>> +{ >>> + unsigned short reg; >>> + >>> + 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 omap2_onenand_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 omap2_onenand_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 -ENODEV; >>> +} >>> + >>> 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,19 +208,19 @@ 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->gpio_quirk) >>> /* Add a delay to let GPIO settle */ >>> syscfg = read_reg(c, ONENAND_REG_SYS_CFG1); >>> } >>> >>> reinit_completion(&c->irq_done); >>> - if (c->gpio_irq) { >>> - result = gpio_get_value(c->gpio_irq); >>> - if (result == -1) { >>> + if (c->gpiod) { >>> + result = gpiod_get_value(c->gpiod); >>> + if (result < 0) { >>> ctrl = read_reg(c, ONENAND_REG_CTRL_STATUS); >>> intr = read_reg(c, ONENAND_REG_INTERRUPT); >>> wait_err("gpio error", state, ctrl, intr); >>> - return -EIO; >>> + return result; >>> } >>> } else >>> result = 0; >>> @@ -609,142 +664,143 @@ static int omap2_onenand_disable(struct mtd_info *mtd) >>> >>> static int omap2_onenand_probe(struct platform_device *pdev) >>> { >>> - struct omap_onenand_platform_data *pdata; >>> + u32 val; >>> + int freq, r; >>> + unsigned int mem_size; >>> + struct resource *res; >>> struct omap2_onenand *c; >>> struct onenand_chip *this; >>> - int r; >>> - struct resource *res; >>> + const struct omap2_onenand_devdata *devdata; >>> + struct device *dev = &pdev->dev; >>> + struct device_node *np = dev->of_node; >>> + >>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> + if (res == NULL) { >>> + dev_err(dev, "error getting memory resource\n"); >>> + return -EINVAL; >>> + } >>> >>> - pdata = dev_get_platdata(&pdev->dev); >>> - if (pdata == NULL) { >>> - dev_err(&pdev->dev, "platform data missing\n"); >>> - return -ENODEV; >>> + r = of_property_read_u32(np, "reg", &val); >>> + if (r) { >>> + dev_err(dev, "reg not found in DT\n"); >>> + return r; >>> } >>> >>> - c = kzalloc(sizeof(struct omap2_onenand), GFP_KERNEL); >>> + c = devm_kzalloc(dev, sizeof(struct omap2_onenand), GFP_KERNEL); >>> if (!c) >>> return -ENOMEM; >>> >>> - 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; >>> - } >>> - >>> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> - if (res == NULL) { >>> - r = -EINVAL; >>> - dev_err(&pdev->dev, "error getting memory resource\n"); >>> - goto err_kfree; >>> - } >>> + devdata = of_device_get_match_data(dev); >>> + this = &c->onenand; >>> >>> + c->gpmc_cs = val; >>> + c->dma_channel = -1; >>> + c->gpio_quirk = devdata->gpio_quirk; >>> c->phys_base = res->start; >>> - c->mem_size = resource_size(res); >>> - >>> - if (request_mem_region(c->phys_base, c->mem_size, >>> - pdev->dev.driver->name) == NULL) { >>> - dev_err(&pdev->dev, "Cannot reserve memory region at 0x%08lx, size: 0x%x\n", >>> - c->phys_base, c->mem_size); >>> - r = -EBUSY; >>> - goto err_kfree; >>> - } >>> - c->onenand.base = ioremap(c->phys_base, c->mem_size); >>> - if (c->onenand.base == NULL) { >>> - r = -ENOMEM; >>> - 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; >>> - } >>> + mem_size = resource_size(res); >>> + if (devm_request_mem_region(dev, c->phys_base, mem_size, >>> + dev->driver->name) == NULL) { >>> >>> - if (c->gpio_irq) { >>> - if ((r = gpio_request(c->gpio_irq, "OneNAND irq")) < 0) { >>> - dev_err(&pdev->dev, "Failed to request GPIO%d for " >>> - "OneNAND\n", c->gpio_irq); >>> - goto err_iounmap; >>> - } >>> - gpio_direction_input(c->gpio_irq); >>> - >>> - if ((r = request_irq(gpio_to_irq(c->gpio_irq), >>> - omap2_onenand_interrupt, IRQF_TRIGGER_RISING, >>> - pdev->dev.driver->name, c)) < 0) >>> - goto err_release_gpio; >>> + dev_err(dev, "Cannot reserve memory region at 0x%08lx, size: 0x%x\n", >>> + c->phys_base, mem_size); >>> + return -EBUSY; >>> } >>> + c->onenand.base = devm_ioremap(dev, c->phys_base, mem_size); >>> + if (c->onenand.base == NULL) >>> + return -ENOMEM; >>> >>> - if (c->dma_channel >= 0) { >>> - r = omap_request_dma(0, pdev->dev.driver->name, >>> - omap2_onenand_dma_cb, (void *) c, >>> - &c->dma_channel); >>> - if (r == 0) { >>> - omap_set_dma_write_mode(c->dma_channel, >>> - OMAP_DMA_WRITE_NON_POSTED); >>> - omap_set_dma_src_data_pack(c->dma_channel, 1); >>> - omap_set_dma_src_burst_mode(c->dma_channel, >>> - OMAP_DMA_DATA_BURST_8); >>> - omap_set_dma_dest_data_pack(c->dma_channel, 1); >>> - omap_set_dma_dest_burst_mode(c->dma_channel, >>> - OMAP_DMA_DATA_BURST_8); >>> - } else { >>> - dev_info(&pdev->dev, >>> + if (!of_property_read_u32(np, "dma-channel", &val)) { >>> + c->dma_channel = val; >>> + r = omap_request_dma(0, dev->driver->name, >>> + omap2_onenand_dma_cb, (void *) c, >>> + &c->dma_channel); >>> + if (r) { >>> + dev_info(dev, >>> "failed to allocate DMA for OneNAND, " >>> "using PIO instead\n"); >>> c->dma_channel = -1; >>> } >>> } >>> >>> - 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->pdev = pdev; >>> - c->mtd.priv = &c->onenand; >>> - >>> - c->mtd.dev.parent = &pdev->dev; >>> - mtd_set_of_node(&c->mtd, pdata->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 { >>> - this->read_bufferram = omap2_onenand_read_bufferram; >>> - this->write_bufferram = omap2_onenand_write_bufferram; >>> + this->read_bufferram = devdata->read_bufferram; >>> + this->write_bufferram = devdata->write_bufferram; >>> + >>> + c->gpiod = devm_gpiod_get(dev, "OneNAND irq", GPIOD_IN); >>> + if (IS_ERR(c->gpiod)) { >>> + r = PTR_ERR(c->gpiod); >>> + /* Just try again if this happens */ >>> + if (r != -EPROBE_DEFER) >>> + dev_err(dev, "error getting gpio (%d)\n", r); >>> + goto err_release_dma; >>> } >>> + r = devm_request_irq(dev, gpiod_to_irq(c->gpiod), >>> + omap2_onenand_interrupt, IRQF_TRIGGER_RISING, >>> + "OneNAND irq", c); >>> + if (r) >>> + goto err_release_dma; >>> } >>> >>> - if (pdata->regulator_can_sleep) { >>> - c->regulator = regulator_get(&pdev->dev, "vonenand"); >>> + if (of_property_read_bool(np, "skip-initial-unlocking")) >>> + this->options |= ONENAND_SKIP_INITIAL_UNLOCKING; >>> + >>> + if (of_property_read_bool(np, "regulator-can-sleep")) { >> >> why do we need "regulator-can-sleep" property? > > Comes from commit 9ac4e613a88d ("mtd: OneNAND: OMAP2/3: prevent regulator > sleeping while OneNAND is in use"). Now looking at it, we do not need such > property, just a regulator to use - driver will use regulator if there is > regulator consumer reference in onenand node. >>>> + c->regulator = devm_regulator_get(dev, "vonenand"); >> >> how about just "vdd" instead of "vonenand" ? > > Well, other subsystems are using this naming scheme, ie. vmmc, vwlan, vcpu0, > etc. But I do not have any strong preference here. > I see it this way. We already know that the supply is for oneneand since the property is in the onenand node. The property must only state the supply name e.g. vdd3v3, vdd5v, or just vdd. > So with regulator-can-sleep property drop, we could simply use: > devm_regulator_get_optional(dev, "vonenand"); right. > > We still need to privide something to control initial unlocking, see commit > c93ff6bf16523d ("mtd: omap: add new variable to platform data to control > onenand unlocking"). How ever it not used anywhere, so maybye just drop > to be readded later once needed? > And this commit b3dcfd35244e ("mtd: onenand: add new option to control initial onenand unlocking") But none of them explain why exactly it is needed. If none of the platforms are using it I'm OK with getting rid of it. >>> if (IS_ERR(c->regulator)) { >>> - dev_err(&pdev->dev, "Failed to get regulator\n"); >>> r = PTR_ERR(c->regulator); >>> + dev_err(dev, "failed to get regulator (%d)\n", r); >>> goto err_release_dma; >>> } >>> - c->onenand.enable = omap2_onenand_enable; >>> - c->onenand.disable = omap2_onenand_disable; >>> + this->enable = omap2_onenand_enable; >>> + this->disable = omap2_onenand_disable; >>> } >>> >>> - if (pdata->skip_initial_unlocking) >>> - this->options |= ONENAND_SKIP_INITIAL_UNLOCKING; >>> + init_completion(&c->irq_done); >>> + init_completion(&c->dma_done); >>> + >>> + omap2_onenand_set_async_mode(c); >>> + freq = omap2_onenand_get_freq(c); >>> + if (freq < 0) { >>> + dev_err(&pdev->dev, >>> + "Rate not detected, bad GPMC async timings?\n"); >>> + r = freq; >>> + goto err_release_dma; >>> + } >>> + >>> + 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_release_dma; >>> + } >>> + if (r > 0) >>> + omap2_onenand_set_cfg(c, true, true, r); >>> + >>> + if (c->dma_channel >= 0) { >>> + omap_set_dma_write_mode(c->dma_channel, >>> + OMAP_DMA_WRITE_NON_POSTED); >>> + omap_set_dma_src_data_pack(c->dma_channel, 1); >>> + omap_set_dma_src_burst_mode(c->dma_channel, >>> + OMAP_DMA_DATA_BURST_8); >>> + omap_set_dma_dest_data_pack(c->dma_channel, 1); >>> + omap_set_dma_dest_burst_mode(c->dma_channel, >>> + OMAP_DMA_DATA_BURST_8); >>> + } >>> + >>> + 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, freq); >>> + >>> + c->pdev = pdev; >>> + c->mtd.priv = &c->onenand; >>> + c->mtd.dev.parent = &pdev->dev; >>> + mtd_set_of_node(&c->mtd, pdev->dev.of_node); >>> >>> if ((r = onenand_scan(&c->mtd, 1)) < 0) >>> - goto err_release_regulator; >>> + goto err_release_dma; >>> >>> - 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; >>> >>> @@ -754,22 +810,9 @@ static int omap2_onenand_probe(struct platform_device *pdev) >>> >>> err_release_onenand: >>> onenand_release(&c->mtd); >>> -err_release_regulator: >>> - regulator_put(c->regulator); >>> err_release_dma: >>> if (c->dma_channel != -1) >>> omap_free_dma(c->dma_channel); >>> - if (c->gpio_irq) >>> - free_irq(gpio_to_irq(c->gpio_irq), c); >>> -err_release_gpio: >>> - if (c->gpio_irq) >>> - gpio_free(c->gpio_irq); >>> -err_iounmap: >>> - iounmap(c->onenand.base); >>> -err_release_mem_region: >>> - release_mem_region(c->phys_base, c->mem_size); >>> -err_kfree: >>> - kfree(c); >>> >>> return r; >>> } >>> @@ -779,27 +822,43 @@ static int omap2_onenand_remove(struct platform_device *pdev) >>> struct omap2_onenand *c = dev_get_drvdata(&pdev->dev); >>> >>> onenand_release(&c->mtd); >>> - regulator_put(c->regulator); >>> if (c->dma_channel != -1) >>> omap_free_dma(c->dma_channel); >>> omap2_onenand_shutdown(pdev); >>> - if (c->gpio_irq) { >>> - free_irq(gpio_to_irq(c->gpio_irq), c); >>> - gpio_free(c->gpio_irq); >>> - } >>> - iounmap(c->onenand.base); >>> - release_mem_region(c->phys_base, c->mem_size); >>> - kfree(c); >>> >>> return 0; >>> } >>> >>> +static const struct omap2_onenand_devdata omap2_devdata = { >>> + .gpio_quirk = false, >>> + .read_bufferram = omap2_onenand_read_bufferram, >>> + .write_bufferram = omap2_onenand_write_bufferram, >>> +}; >>> + >>> +static const struct omap2_onenand_devdata omap3_devdata = { >>> + .gpio_quirk = true, >>> + .read_bufferram = omap3_onenand_read_bufferram, >>> + .write_bufferram = omap3_onenand_write_bufferram, >>> +}; >>> + >>> +static const struct of_device_id omap2_onenand_ids[] = { >>> + { >>> + .compatible = "ti,omap2-onenand", >>> + .data = &omap2_devdata, >>> + }, { >>> + .compatible = "ti,omap3-onenand", >>> + .data = &omap3_devdata, >>> + }, {}, >>> +}; >>> +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), >>> }, >>> }; >>> >>> >>
diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c index 24a1388d3031..7ab56383ff7a 100644 --- a/drivers/mtd/onenand/omap2.c +++ b/drivers/mtd/onenand/omap2.c @@ -28,17 +28,18 @@ #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> #include <linux/dma-mapping.h> #include <linux/io.h> #include <linux/slab.h> +#include <linux/gpio/consumer.h> #include <linux/regulator/consumer.h> -#include <linux/gpio.h> #include <asm/mach/flash.h> -#include <linux/platform_data/mtd-onenand-omap2.h> #include <linux/omap-dma.h> @@ -50,17 +51,22 @@ struct omap2_onenand { struct platform_device *pdev; int gpmc_cs; unsigned long phys_base; - unsigned int mem_size; - int gpio_irq; + struct gpio_desc *gpiod; struct mtd_info mtd; struct onenand_chip 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 gpio_quirk; +}; + +struct omap2_onenand_devdata { + bool gpio_quirk; + int (*read_bufferram)(struct mtd_info *mtd, int area, + unsigned char *buffer, int offset, size_t count); + int (*write_bufferram)(struct mtd_info *mtd, int area, + const unsigned char *buffer, int offset, size_t count); }; static void omap2_onenand_dma_cb(int lch, u16 ch_status, void *data) @@ -90,6 +96,55 @@ 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 omap2_onenand_set_async_mode(struct omap2_onenand *c) +{ + unsigned short reg; + + 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 omap2_onenand_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 omap2_onenand_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 -ENODEV; +} + 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,19 +208,19 @@ 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->gpio_quirk) /* Add a delay to let GPIO settle */ syscfg = read_reg(c, ONENAND_REG_SYS_CFG1); } reinit_completion(&c->irq_done); - if (c->gpio_irq) { - result = gpio_get_value(c->gpio_irq); - if (result == -1) { + if (c->gpiod) { + result = gpiod_get_value(c->gpiod); + if (result < 0) { ctrl = read_reg(c, ONENAND_REG_CTRL_STATUS); intr = read_reg(c, ONENAND_REG_INTERRUPT); wait_err("gpio error", state, ctrl, intr); - return -EIO; + return result; } } else result = 0; @@ -609,142 +664,143 @@ static int omap2_onenand_disable(struct mtd_info *mtd) static int omap2_onenand_probe(struct platform_device *pdev) { - struct omap_onenand_platform_data *pdata; + u32 val; + int freq, r; + unsigned int mem_size; + struct resource *res; struct omap2_onenand *c; struct onenand_chip *this; - int r; - struct resource *res; + const struct omap2_onenand_devdata *devdata; + struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (res == NULL) { + dev_err(dev, "error getting memory resource\n"); + return -EINVAL; + } - pdata = dev_get_platdata(&pdev->dev); - if (pdata == NULL) { - dev_err(&pdev->dev, "platform data missing\n"); - return -ENODEV; + r = of_property_read_u32(np, "reg", &val); + if (r) { + dev_err(dev, "reg not found in DT\n"); + return r; } - c = kzalloc(sizeof(struct omap2_onenand), GFP_KERNEL); + c = devm_kzalloc(dev, sizeof(struct omap2_onenand), GFP_KERNEL); if (!c) return -ENOMEM; - 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; - } - - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (res == NULL) { - r = -EINVAL; - dev_err(&pdev->dev, "error getting memory resource\n"); - goto err_kfree; - } + devdata = of_device_get_match_data(dev); + this = &c->onenand; + c->gpmc_cs = val; + c->dma_channel = -1; + c->gpio_quirk = devdata->gpio_quirk; c->phys_base = res->start; - c->mem_size = resource_size(res); - - if (request_mem_region(c->phys_base, c->mem_size, - pdev->dev.driver->name) == NULL) { - dev_err(&pdev->dev, "Cannot reserve memory region at 0x%08lx, size: 0x%x\n", - c->phys_base, c->mem_size); - r = -EBUSY; - goto err_kfree; - } - c->onenand.base = ioremap(c->phys_base, c->mem_size); - if (c->onenand.base == NULL) { - r = -ENOMEM; - 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; - } + mem_size = resource_size(res); + if (devm_request_mem_region(dev, c->phys_base, mem_size, + dev->driver->name) == NULL) { - if (c->gpio_irq) { - if ((r = gpio_request(c->gpio_irq, "OneNAND irq")) < 0) { - dev_err(&pdev->dev, "Failed to request GPIO%d for " - "OneNAND\n", c->gpio_irq); - goto err_iounmap; - } - gpio_direction_input(c->gpio_irq); - - if ((r = request_irq(gpio_to_irq(c->gpio_irq), - omap2_onenand_interrupt, IRQF_TRIGGER_RISING, - pdev->dev.driver->name, c)) < 0) - goto err_release_gpio; + dev_err(dev, "Cannot reserve memory region at 0x%08lx, size: 0x%x\n", + c->phys_base, mem_size); + return -EBUSY; } + c->onenand.base = devm_ioremap(dev, c->phys_base, mem_size); + if (c->onenand.base == NULL) + return -ENOMEM; - if (c->dma_channel >= 0) { - r = omap_request_dma(0, pdev->dev.driver->name, - omap2_onenand_dma_cb, (void *) c, - &c->dma_channel); - if (r == 0) { - omap_set_dma_write_mode(c->dma_channel, - OMAP_DMA_WRITE_NON_POSTED); - omap_set_dma_src_data_pack(c->dma_channel, 1); - omap_set_dma_src_burst_mode(c->dma_channel, - OMAP_DMA_DATA_BURST_8); - omap_set_dma_dest_data_pack(c->dma_channel, 1); - omap_set_dma_dest_burst_mode(c->dma_channel, - OMAP_DMA_DATA_BURST_8); - } else { - dev_info(&pdev->dev, + if (!of_property_read_u32(np, "dma-channel", &val)) { + c->dma_channel = val; + r = omap_request_dma(0, dev->driver->name, + omap2_onenand_dma_cb, (void *) c, + &c->dma_channel); + if (r) { + dev_info(dev, "failed to allocate DMA for OneNAND, " "using PIO instead\n"); c->dma_channel = -1; } } - 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->pdev = pdev; - c->mtd.priv = &c->onenand; - - c->mtd.dev.parent = &pdev->dev; - mtd_set_of_node(&c->mtd, pdata->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 { - this->read_bufferram = omap2_onenand_read_bufferram; - this->write_bufferram = omap2_onenand_write_bufferram; + this->read_bufferram = devdata->read_bufferram; + this->write_bufferram = devdata->write_bufferram; + + c->gpiod = devm_gpiod_get(dev, "OneNAND irq", GPIOD_IN); + if (IS_ERR(c->gpiod)) { + r = PTR_ERR(c->gpiod); + /* Just try again if this happens */ + if (r != -EPROBE_DEFER) + dev_err(dev, "error getting gpio (%d)\n", r); + goto err_release_dma; } + r = devm_request_irq(dev, gpiod_to_irq(c->gpiod), + omap2_onenand_interrupt, IRQF_TRIGGER_RISING, + "OneNAND irq", c); + if (r) + goto err_release_dma; } - if (pdata->regulator_can_sleep) { - c->regulator = regulator_get(&pdev->dev, "vonenand"); + if (of_property_read_bool(np, "skip-initial-unlocking")) + this->options |= ONENAND_SKIP_INITIAL_UNLOCKING; + + if (of_property_read_bool(np, "regulator-can-sleep")) { + c->regulator = devm_regulator_get(dev, "vonenand"); if (IS_ERR(c->regulator)) { - dev_err(&pdev->dev, "Failed to get regulator\n"); r = PTR_ERR(c->regulator); + dev_err(dev, "failed to get regulator (%d)\n", r); goto err_release_dma; } - c->onenand.enable = omap2_onenand_enable; - c->onenand.disable = omap2_onenand_disable; + this->enable = omap2_onenand_enable; + this->disable = omap2_onenand_disable; } - if (pdata->skip_initial_unlocking) - this->options |= ONENAND_SKIP_INITIAL_UNLOCKING; + init_completion(&c->irq_done); + init_completion(&c->dma_done); + + omap2_onenand_set_async_mode(c); + freq = omap2_onenand_get_freq(c); + if (freq < 0) { + dev_err(&pdev->dev, + "Rate not detected, bad GPMC async timings?\n"); + r = freq; + goto err_release_dma; + } + + 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_release_dma; + } + if (r > 0) + omap2_onenand_set_cfg(c, true, true, r); + + if (c->dma_channel >= 0) { + omap_set_dma_write_mode(c->dma_channel, + OMAP_DMA_WRITE_NON_POSTED); + omap_set_dma_src_data_pack(c->dma_channel, 1); + omap_set_dma_src_burst_mode(c->dma_channel, + OMAP_DMA_DATA_BURST_8); + omap_set_dma_dest_data_pack(c->dma_channel, 1); + omap_set_dma_dest_burst_mode(c->dma_channel, + OMAP_DMA_DATA_BURST_8); + } + + 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, freq); + + c->pdev = pdev; + c->mtd.priv = &c->onenand; + c->mtd.dev.parent = &pdev->dev; + mtd_set_of_node(&c->mtd, pdev->dev.of_node); if ((r = onenand_scan(&c->mtd, 1)) < 0) - goto err_release_regulator; + goto err_release_dma; - 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; @@ -754,22 +810,9 @@ static int omap2_onenand_probe(struct platform_device *pdev) err_release_onenand: onenand_release(&c->mtd); -err_release_regulator: - regulator_put(c->regulator); err_release_dma: if (c->dma_channel != -1) omap_free_dma(c->dma_channel); - if (c->gpio_irq) - free_irq(gpio_to_irq(c->gpio_irq), c); -err_release_gpio: - if (c->gpio_irq) - gpio_free(c->gpio_irq); -err_iounmap: - iounmap(c->onenand.base); -err_release_mem_region: - release_mem_region(c->phys_base, c->mem_size); -err_kfree: - kfree(c); return r; } @@ -779,27 +822,43 @@ static int omap2_onenand_remove(struct platform_device *pdev) struct omap2_onenand *c = dev_get_drvdata(&pdev->dev); onenand_release(&c->mtd); - regulator_put(c->regulator); if (c->dma_channel != -1) omap_free_dma(c->dma_channel); omap2_onenand_shutdown(pdev); - if (c->gpio_irq) { - free_irq(gpio_to_irq(c->gpio_irq), c); - gpio_free(c->gpio_irq); - } - iounmap(c->onenand.base); - release_mem_region(c->phys_base, c->mem_size); - kfree(c); return 0; } +static const struct omap2_onenand_devdata omap2_devdata = { + .gpio_quirk = false, + .read_bufferram = omap2_onenand_read_bufferram, + .write_bufferram = omap2_onenand_write_bufferram, +}; + +static const struct omap2_onenand_devdata omap3_devdata = { + .gpio_quirk = true, + .read_bufferram = omap3_onenand_read_bufferram, + .write_bufferram = omap3_onenand_write_bufferram, +}; + +static const struct of_device_id omap2_onenand_ids[] = { + { + .compatible = "ti,omap2-onenand", + .data = &omap2_devdata, + }, { + .compatible = "ti,omap3-onenand", + .data = &omap3_devdata, + }, {}, +}; +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), }, };