Hi,
On 11/02/17 15:03, Ladislav Michl wrote:
> OneNAND base address from FDT is ignored, so driver only works
> for bootloader configured chipselect. Bring gpmc_probe_onenand_child
> closer to gpmc_probe_generic_child to honour FDT configured base
> address and to make future merge with gpmc_probe_generic_child
> easier.
>
> Signed-off-by: Ladislav Michl <ladis@xxxxxxxxxxxxxx>
> ---
> arch/arm/mach-omap2/gpmc-onenand.c | 52 ++++++++++++-------------
> drivers/memory/omap-gpmc.c | 48 +++++++++++++++++++++--
> include/linux/platform_data/mtd-onenand-omap2.h | 1 +
> 3 files changed, 70 insertions(+), 31 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/gpmc-onenand.c b/arch/arm/mach-omap2/gpmc-onenand.c
> index 2944af820558..3dd7db3a2b46 100644
> --- a/arch/arm/mach-omap2/gpmc-onenand.c
> +++ b/arch/arm/mach-omap2/gpmc-onenand.c
> @@ -35,17 +35,6 @@ static unsigned latency;
>
> static struct omap_onenand_platform_data *gpmc_onenand_data;
>
> -static struct resource gpmc_onenand_resource = {
> - .flags = IORESOURCE_MEM,
> -};
> -
> -static struct platform_device gpmc_onenand_device = {
> - .name = "omap2-onenand",
> - .id = -1,
> - .num_resources = 1,
> - .resource = &gpmc_onenand_resource,
> -};
> -
> static struct gpmc_settings onenand_async = {
> .device_width = GPMC_DEVWIDTH_16BIT,
> .mux_add_data = GPMC_MUX_AD,
> @@ -348,13 +337,12 @@ static int omap2_onenand_setup_sync(void __iomem *onenand_base, int *freq_ptr)
>
> static int gpmc_onenand_setup(void __iomem *onenand_base, int *freq_ptr)
> {
> - struct device *dev = &gpmc_onenand_device.dev;
> unsigned l = ONENAND_SYNC_READ | ONENAND_SYNC_READWRITE;
> int ret;
>
> ret = omap2_onenand_setup_async(onenand_base);
> if (ret) {
> - dev_err(dev, "unable to set to async mode\n");
> + pr_err("OneNAND: unable to set to async mode\n");
> return ret;
> }
>
> @@ -363,22 +351,21 @@ static int gpmc_onenand_setup(void __iomem *onenand_base, int *freq_ptr)
>
> ret = omap2_onenand_setup_sync(onenand_base, freq_ptr);
> if (ret)
> - dev_err(dev, "unable to set to sync mode\n");
> + pr_err("OneNAND: unable to set to sync mode\n");
> return ret;
> }
>
> int gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
This function should be removed entirely as nobody seems to be using it.
Or am I wrong?
> {
> int err;
> - struct device *dev = &gpmc_onenand_device.dev;
> + struct platform_device *pdev;
>
> gpmc_onenand_data = _onenand_data;
> gpmc_onenand_data->onenand_setup = gpmc_onenand_setup;
> - gpmc_onenand_device.dev.platform_data = gpmc_onenand_data;
>
> if (cpu_is_omap24xx() &&
> (gpmc_onenand_data->flags & ONENAND_SYNC_READWRITE)) {
> - dev_warn(dev, "OneNAND using only SYNC_READ on 24xx\n");
> + pr_warn("OneNAND using only SYNC_READ on 24xx\n");
> gpmc_onenand_data->flags &= ~ONENAND_SYNC_READWRITE;
> gpmc_onenand_data->flags |= ONENAND_SYNC_READ;
> }
> @@ -388,22 +375,31 @@ int gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
> else
> gpmc_onenand_data->flags &= ~ONENAND_IN_OMAP34XX;
>
> - err = gpmc_cs_request(gpmc_onenand_data->cs, ONENAND_IO_SIZE,
> - (unsigned long *)&gpmc_onenand_resource.start);
> - if (err < 0) {
> - dev_err(dev, "Cannot request GPMC CS %d, error %d\n",
> - gpmc_onenand_data->cs, err);
> - return err;
> + pdev = platform_device_alloc("omap2-onenand", gpmc_onenand_data->cs);
> + if (!pdev) {
> + err = -ENOMEM;
> + goto out_free_cs;
> }
>
> - gpmc_onenand_resource.end = gpmc_onenand_resource.start +
> - ONENAND_IO_SIZE - 1;
> + err = platform_device_add_resources(pdev,
> + gpmc_onenand_data->resource, 1);
> + if (err)
> + goto out_free_pdev;
>
> - err = platform_device_register(&gpmc_onenand_device);
> + pdev->dev.platform_data = gpmc_onenand_data;
> +
> + err = platform_device_add(pdev);
> if (err) {
> - dev_err(dev, "Unable to register OneNAND device\n");
> - gpmc_cs_free(gpmc_onenand_data->cs);
> + dev_err(&pdev->dev, "Unable to register OneNAND device\n");
> + goto out_free_pdev;
> }
>
> + return 0;
> +
> +out_free_pdev:
> + platform_device_put(pdev);
> +out_free_cs:
> + gpmc_cs_free(gpmc_onenand_data->cs);
> +
> return err;
> }
> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
Please put the memory/omap-gpmc.c changes in to a separate patch.
> index bf0fe0137dfe..725fceb2146a 100644
> --- a/drivers/memory/omap-gpmc.c
> +++ b/drivers/memory/omap-gpmc.c
> @@ -1926,21 +1926,63 @@ static void __maybe_unused gpmc_read_timings_dt(struct device_node *np,
> static int gpmc_probe_onenand_child(struct platform_device *pdev,
> struct device_node *child)
> {
> - u32 val;
> + u32 cs, val;
> + int ret;
> + unsigned long base;
> + struct resource res;
> struct omap_onenand_platform_data *gpmc_onenand_data;
>
> - if (of_property_read_u32(child, "reg", &val) < 0) {
> + if (of_property_read_u32(child, "reg", &cs) < 0) {
> dev_err(&pdev->dev, "%s has no 'reg' property\n",
> child->full_name);
> return -ENODEV;
> }
>
> + memset(&res, 0, sizeof(res));
> + res.flags = IORESOURCE_MEM;
> + if (of_address_to_resource(child, 0, &res) < 0) {
> + dev_err(&pdev->dev, "%s has malformed 'reg' property\n",
> + child->full_name);
> + return -ENODEV;
> + }
> +
> + ret = gpmc_cs_request(cs, resource_size(&res), &base);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "cannot request GPMC CS %d\n", cs);
> + return ret;
> + }
> + gpmc_cs_set_name(cs, child->name);
> +
> + dev_warn(&pdev->dev, "OneNAND: 0x%x-0x%x\n", res.start, res.end);
dev_dbg() or get rid of it altogether?
> + /* CS must be disabled while making changes to gpmc configuration */
> + gpmc_cs_disable_mem(cs);
> +
> + ret = gpmc_cs_remap(cs, res.start);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "cannot remap GPMC CS %d to 0x%x\n",
> + cs, res.start);
> + if (res.start < GPMC_MEM_START) {
> + dev_info(&pdev->dev,
> + "GPMC CS %d start cannot be lesser than 0x%x\n",
> + cs, GPMC_MEM_START);
> + } else if (res.end > GPMC_MEM_END) {
> + dev_info(&pdev->dev,
> + "GPMC CS %d end cannot be greater than 0x%x\n",
> + cs, GPMC_MEM_END);
> + }
> + return ret;
> + }
> +
> + /* Enable CS region */
> + gpmc_cs_enable_mem(cs);
> +
> gpmc_onenand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_onenand_data),
> GFP_KERNEL);
> if (!gpmc_onenand_data)
> return -ENOMEM;
>
> - gpmc_onenand_data->cs = val;
> + gpmc_onenand_data->cs = cs;
> + gpmc_onenand_data->resource = &res;
> gpmc_onenand_data->of_node = child;
> gpmc_onenand_data->dma_channel = -1;
>
> diff --git a/include/linux/platform_data/mtd-onenand-omap2.h b/include/linux/platform_data/mtd-onenand-omap2.h
> index 56ff0e6f5ad1..1c8363d3253b 100644
> --- a/include/linux/platform_data/mtd-onenand-omap2.h
> +++ b/include/linux/platform_data/mtd-onenand-omap2.h
> @@ -19,6 +19,7 @@
>
> struct omap_onenand_platform_data {
> int cs;
> + struct resource *resource;
This should no longer be required as we're getting the resource information
from DT.
> int gpio_irq;
> struct mtd_partition *parts;
> int nr_parts;
>
--
cheers,
-roger
--
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
