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@xxxxxxxxxxxxxx>
>>>>> ---
>>>>> 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),
> },
> };
>
>
--
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
