Hi Roger,
On Thu, Oct 12, 2017 at 06:05:31PM +0300, Roger Quadros wrote:
[snip]
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
> On 12/10/17 15:27, Ladislav Michl wrote:
> > Not being Tony, I try to answer anyway...
> >
> > I've done patchset which removes gpmc_probe_onenand_child and uses
> > gpmc_probe_generic_child (renamed to gpmc_probe_child) to setup OneNAND
> > async timings. Sync timings is setup from mtd/onenand/omap2.c after
> > chip frequency is known.
> >
> > I've not sent it yet as device datasheet is available under NDA only,
> > I do not have it and thus I'm left with equivalent source modifications
> > without really understanding what should be done and why.
> >
> > That said, I sent this patch to fix OneNAND support (for IGEPv2 board
> > at least) and leave discussion about making things nice for later.
> >
> > I'm proposing killing gpmc_onenand_data first and teaching OneNAND
> > to get its configuration from DT. Later I'll cleanup sync/async
> > patchset and send it for review unless someone is willing to take
> > it over to speed things up a bit - then I'll send it as it is :-)
>
> sounds fine to me.
> >
> > Side question: in tree OneNAND DT nodes are providing timing information.
> > I have no clue where this timing information is coming from and driver
> > completely ignores it and computes timings on its own. Any hints here?
>
> My guess is nobody has access to the datasheet and we're relying on bootloader
> set timings or timings that were already hardcoded in the kernel.
>
> You can use CONFIG_OMAP_GPMC_DEBUG to dump the timings set by the bootloader
> in DT format.
>
> You can also compare the timings before and after gpmc driver loads.
That will not help a lot, as these questions will remain unanswered using
above method:
- why SYNC_WRITE cannot be used on 24xx?
- why are we letting gpio line settle only on 34xx?
- why we are not using SYNC_WRITE with 54 MHz freq?
- how should be omap_onenand_platform_data flags set by default?
probably few more I do not remember, but will show up once I start
polishing patchset :)
Anyway, here's first untested version, just to show, that we need
to touch arch/arm/mach-omap2/gpmc-onenand.c anyway, just to make
gpmc_onenand_setup directly callable without global
omap_onenand_platform_data (and later move it into omap-gpmc)
So unless someone comes with better idea, I'll drop all global
variables from arch/arm/mach-omap2/gpmc-onenand.c and export
gpmc_onenand_setup to get rid of omap_onenand_platform_data.
After that patch gets merged, we can make additional patch on
top of following one, which will drop use of omap_onenand_platform_data
from omap2 onenand mtd driver. There are simply too many trees to
get things merged to make it easy task :-/
Also note that no DT only kernel sets flags, regulator, gpio and yet
noone complained, so we could leave it for later - and use more recent
APIs (gpiolib, etc).
diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c
index 29cc0c21138c..85fac7aed2dc 100644
--- a/drivers/mtd/onenand/omap2.c
+++ b/drivers/mtd/onenand/omap2.c
@@ -28,12 +28,15 @@
#include <linux/mtd/mtd.h>
#include <linux/mtd/onenand.h>
#include <linux/mtd/partitions.h>
+#include <linux/of.h>
+#include <linux/of_device.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/sys_soc.h>
#include <linux/regulator/consumer.h>
#include <linux/gpio.h>
@@ -59,7 +62,7 @@ struct omap2_onenand {
int dma_channel;
int freq;
struct regulator *regulator;
- u8 flags;
+ bool gpio_settle;
};
static void omap2_onenand_dma_cb(int lch, u16 ch_status, void *data)
@@ -152,7 +155,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->gpio_settle)
/* Add a delay to let GPIO settle */
syscfg = read_reg(c, ONENAND_REG_SYS_CFG1);
}
@@ -606,6 +609,32 @@ 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;
+
+ /* TODO: gpio, regulator, unlocking */
+
+ return 0;
+}
+
+static const struct soc_device_attribute omap2_soc_devices[] = {
+ { .family = "OMAP2*" },
+ { /* sentinel */ }
+};
+
static int omap2_onenand_probe(struct platform_device *pdev)
{
struct omap_onenand_platform_data *pdata;
@@ -624,12 +653,12 @@ static int omap2_onenand_probe(struct platform_device *pdev)
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;
@@ -710,21 +739,22 @@ static int omap2_onenand_probe(struct platform_device *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 (soc_device_match(omap2_soc_devices)) {
this->read_bufferram = omap2_onenand_read_bufferram;
this->write_bufferram = omap2_onenand_write_bufferram;
+ } else {
+ c->gpio_settle = true;
+ 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");
@@ -735,14 +765,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;
@@ -792,12 +821,19 @@ static int omap2_onenand_remove(struct platform_device *pdev)
return 0;
}
+static const struct of_device_id omap2_onenand_ids[] = {
+ { .compatible = "ti,omap2-onenand", },
+ {},
+};
+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),
},
};
--
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
