Hi
On 20/10/17 21:19, Ladislav Michl wrote:
> Hi,
>
> On Fri, Oct 20, 2017 at 02:52:09PM +0300, Roger Quadros wrote:
>> Hi,
>>
>> On 20/10/17 13:19, Ladislav Michl wrote:
>>> Hi,
>>>
>>> On Fri, Oct 20, 2017 at 10:58:29AM +0300, Roger Quadros wrote:
>>>> Hi,
>>>>
>>>> On 19/10/17 17:45, Ladislav Michl wrote:
>>>>> On Thu, Oct 19, 2017 at 05:02:51PM +0300, Roger Quadros wrote:
>>>>>> On 19/10/17 16:42, Ladislav Michl wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Thu, Oct 19, 2017 at 03:56:30PM +0300, Roger Quadros wrote:
>>>>> [snip]
>>>>>>>> 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.
>>>>>
>>>>> Okay, I'm convinced.
>>>>>
>>>>>>> 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.
>>>>>
>>>>> No, there is no single user.
>>>>>
>>>>> So with those changs combined we have so far:
>>>>> (How to pass parameters to omap2_onenand_set_cfg and how to get timings
>>>>> right still needs to be investigated)
>>>>>
>>>>
>>>> I think what you are doing currently is fine. See below.
>>>>
>>>>> ---
>>>>> drivers/mtd/onenand/omap2.c | 324 ++++++++++++++++++++++++++------------------
>>>>> 1 file changed, 190 insertions(+), 134 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c
>>>>> index 24a1388d3031..83847033510a 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;
>>>>
>>>> Not sure if this is the right error code.
>>>> The device is there. We just don't support the reported frequency.
>>>
>>> We do not know yet device is there, MTG scans for device later. I just kept
>>> the logic we had before.
>>
>> OK, let's keep the logic unchanged. we can improve it separately.
>>
>> Just some ideas for later.
>>
>> There are 2 things we need to check for.
>> 1) ASYNC timings could be incorrect so read returns invalid value. To check if that
>> happened we could read the manufacturer register and if it is invalid, complain about
>> incorrect gpmc ASYNC settings/timings.
>>
>> Fortunately there are only 2 valid manufacturers, so it should be easy.
>> https://github.com/torvalds/linux/blob/master/include/linux/mtd/onenand.h#L211
>>
>> 2) If manufacturer register was read fine then we assume that frequency register will be
>> valid. If we don't support the frequency then we say exactly that.
>
> Well, I was thinking to move it away from omap2 mtd driver and move it to onenand_base,
> see reasoning bellow.
>
>>> Premiliary datasheet lists VERSION reg yet undefined
>>> https://www.digchip.com/datasheets/download_datasheet.php?id=1089403&part-number=KFG1G16Q2A
>>> so not sure what's inside and later datasheets do not describe it either.
>>>
>>>>> +}
>>>>> +
>>>>> 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,140 @@ 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;
>>>>> +
>>>>> + mem_size = resource_size(res);
>>>>> + if (devm_request_mem_region(dev, c->phys_base, mem_size,
>>>>> + dev->driver->name) == NULL) {
>>>>> +
>>>>> + dev_err(dev, "Cannot reserve memory region at 0x%08lx, size: 0x%x\n",
>>>>> + c->phys_base, mem_size);
>>>>> + return -EBUSY;
>>>>> }
>>>>> - c->onenand.base = ioremap(c->phys_base, c->mem_size);
>>>>> - if (c->onenand.base == NULL) {
>>>>> - r = -ENOMEM;
>>>>> - goto err_release_mem_region;
>>>>> + c->onenand.base = devm_ioremap(dev, c->phys_base, mem_size);
>>>>> + if (c->onenand.base == NULL)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> + 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");
>>>> Don't split the print message. It is hard to find while grepping during debug.
>>>
>>> Will fix all that issues in the next version.
>>>
>>>>> + c->dma_channel = -1;
>>>>> + }
>>>>> }
>>>>>
>>>>> - 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;
>>>>> + if (c->dma_channel >= 0) {
>>>>> + this->wait = omap2_onenand_wait;
>>>>> + 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);
>>>>
>>>> Use a uniform format
>>>> "error getting gpio: %d", r:
>>>>
>>>>
>>>>> + goto err_release_dma;
>>>>> }
>>>>> - c->setup = pdata->onenand_setup;
>>>>> + 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;
>>>>> + }
>>>>> +
>>>>> + c->regulator = devm_regulator_get(dev, "vdd");
>
> As a side note, datasheet mentions two independent Vcc, one for memory
> itself and one for i/o.
Actually getting these regulators is not OMAP specific either. Could be done
in onenand_base, before reading the ID registers.
Both regulators can be optional.
So I'm ok if we get rid of regulator code if no OMAP users are affected.
>
>>>>> + if (IS_ERR(c->regulator)) {
>>>>> + r = PTR_ERR(c->regulator);
>>>>> + dev_err(dev, "failed to get regulator (%d)\n", r);
>>>>
>>>> here too.
>>>>
>>>>> + goto err_release_dma;
>>>>> + }
>>>>> + if (c->regulator) {
>>>>> + this->enable = omap2_onenand_enable;
>>>>> + this->disable = omap2_onenand_disable;
>>>>> }
>>>>>
>>>>> - 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);
>>>>
>>>> don't split print message.
>>>>
>>>>> - goto err_iounmap;
>>>>> + 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");
>>>>
>>>> Message is misleading. Should be,
>>>> "Unsupported device. MFR_ID:DEV_ID:VER_ID = %x:%x:%x"
>>>>
>>>> We should dump the device ID register in case someone wants to report the error
>>>> and we need to add support for this new device. Not that it will happen
>>>> but is just good practice.
>>>
>>> Yes, message has historical reasons (same as this splitted lines), but see
>>> bellow.
>>>
>>>>> + r = freq;
>>>>> + goto err_release_dma;
>>>>> }
>>>>> - 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;
>>>>
>>>> Is it possible to operate oneNAND completely in ASYNC mode?
>>>> If so then the below call should be conditional. Only if we need sync_read/write.
>>>
>>> According to datasheet and origilal code, yes. I haven't tried yet (away from
>>> the lab).
>>>
>>>> We still need to figure out how to get the read/write sync flags here right?
>>>> I suggest to add new DT properties for this.
>>>> e.g.
>>>> ti,onenand-sync-read;
>>>> ti,onenand-sync-write;
>>>>
>>>>
>>>> If any one or both are set we use SYNC timings.
>>>
>>> What is we change logic a bit here? I still do think those gpmc generic
>>> properties are sufficient.
>>>
>>> Code contains following when setting asyns timings:
>>> /*
>>> * Note that we need to keep sync_write set for the call to
>>> * omap2_onenand_set_async_mode() to work to detect the onenand
>>> * supported clock rate for the sync timings.
>>> */
>>> s->sync_read = false;
>>> s->sync_write = true;
>>
>> This comes from commit e7b11dc7b77b ("ARM: OMAP2+: Fix onenand rate detection
>> to avoid filesystem corruption")
>>
>> Looks like it was a quick fix. But IMO it is not correct.
>> The real problem is that the ASYNC timings in omap2_onenand_calc_async_timings()
>> are not suitable for n900 for ASYNC mode (too fast for ASYNC mode?).
>
> Seems reasobale but certainly needs to be verified. Tony?
>
>> Another problem is we're not really using the DT provided GPMC timings.
>> I don't see a call to gpmc_read_timings_dt() anywhere in the oneNAND path.
>
> Correct, that's also why I sent simple patch which solved my problem and look
> what you did with that :)
>
>> ASYNC mode *should* have sync_read and sync_write set to false.
>
> I tried and it still works. But would be nice to find someone with n900 to verify.
>
Tony can help us with that.
>>>
>>> When DT mode contains async timings, lets use them and do not bother with
>>> sync ones.
>>
>> This I agree.
>>
>> If it contains sync timings, then it should be those which
>>> certainly works [*]. OneNAND node can contain something like
>>
>> Optimized SYNC timings will not work for ASYNC mode so I don't see how we can get
>> away without providing ASYNC timings.
>>
>> Which now makes me to wonder if we will really need a way to completely specify
>> both ASYNC and SYNC mode timings for optimal results. see below.
>>
>>> 'ti,onenand-optimize-timings' and driver will attempt to get frequency
>>> and set better timings. This will be done after devce probe, so we know
>>> device indentification and we can print it if obtaining frequency fails.
>>> Also, this way we do not need special handlings for OneNAND nodes.
>>>
>>> [*] As Version ID reg is not described in datasheet, but driver relies on
>>> its content, we should leave on DT node author to make sure sync mode works.
>>>
>>>>> + 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);
>>>>
>>>> right. let's keep this format for all messages.
>>>>
>>>>> + goto err_release_dma;
>>>>> }
>>>>> + if (r > 0)
>>>>> + omap2_onenand_set_cfg(c, true, true, r);
>>>>
>>>> Use the 2 new DT properties to figure out the sr, sw parameters.
>>>
>>> With the above proposal we would need both
>>> ti,onenand-sync-read;
>>> ti,onenand-sync-write;
>>> and
>>> gpmc,sync-read;
>>> gpmc,sync-write;
>>>
>>> Is it really worth it?
>>
>> If you see it as 2 separate complete timing specs, it probably is worth it and solves
>> all our problems.
>>
>> something like this
>>
>> onenand {
>>
>> /* Mandatory ASYNC timings for default mode */
>> gpmc,rd-cycle-ns = <..>;
>> /* sync-read and sync-write not set */
>> ...
>>
>> sync_timings@0 {
>> /* Optional SYNC mode optimized timings */
>> gpmc,sync-clk-ps = <15000>;
>> gpmc,sync-read;
>> gpmc,sync-write;
>> ...
>> }
>> };
>>
>> We can let the omap-gpmc.c read and program the sync_timings in gpmc_omap_onenand_set_sync_timings().
>>
>> Also it can verify that if gpmc,sync-clk-ps doesn't fit into the provided freq parameter, we fail.
>>
>> We can do away with magic timing calculations and just relay on DT to provide us the right timings.
>> Migrating current oneNAND users should be easy. Ask them to dump GPMC gpmc_cs_show_timings()
>> after omap2_onenand_calc_async_timings() and after omap2_onenand_calc_async_timings() and we
>> shove those values into the DT.
>>
>> So gpmc_omap_onenand_set_sync_timings(freq, &latency) should reduce to
>>
>> - check if DT provided sync-clk-ps is suitable for provided OneNAND freq else return error code.
>> - gpmc_read_settings_dt(sync_timings_node)
>> - gpmc_read_timings_dt(sync_timings_node)
>> - gpmc_cs_program_settings()
>> - gpmc_cs_set_timings()
>> - &latency = dev_t->cyc_iaa - 1;
>> - return 0
>>
>> The latency parameter dev_t->cyc_iaa need not be calculated. It can be provided by DT as well.
>> Will need to add a new property though to gpmc DT bindings.
>
> If you look at original code path, async timings were programed and Version ID
> register read (they should really change its name). Before doing that, sync
> mode flags were disable in sys_cfg1 reg. And after reading frequency new sync
> timings was calculated and programmed into gpmc. Chip probe function was called
> _after_ all that, meaning that gpmc is in sync mode and onenand driver is still
> able to read manufacturer and product id. Also note that probe function
> temporarily clears sync mode flags in sys_cfg1 reg.
>
> So we should be able to read Version ID reg in probe function as well and
> provide replacable function to setup timings etc. in onenand_base as this
> really is not omap specific. Also our omap2_onenand_set_cfg function is not
> omap2 specific.
>
> And finally onenand_sync_read_bufferram seems to enable sync read flag only
> during bufferram read, while omap2 driver keeps it enabled all the time.
>
> Seems we do not need async timings at all as long as sync one are not too tight.
Yeah, if we assume that DT will provide the right *max* frequency for the oneNAND,
we can skip the ASYNC programming altogether.
On the other hand. if DT provides only ASYNC timings, we continue to use only that.
This simplifies things a lot. Only one Timing set in DT.
>
> Adding Kyungmin Park to cc list...
>
<snip>
--
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
