Re: [PATCH v4 11/27] mtd: nand: omap: Clean up device tree support
On Thu, Dec 03, 2015 at 11:27:13AM +0530, Roger Quadros wrote:
> On 03/12/15 09:59, Brian Norris wrote:
> > On Tue, Oct 06, 2015 at 01:35:48PM +0300, Roger Quadros wrote:
> >> arch/arm/mach-omap2/gpmc-nand.c | 5 +-
> >> drivers/memory/omap-gpmc.c | 143
> >> +++
> >> drivers/mtd/nand/omap2.c | 136
> >> +
> >> include/linux/platform_data/mtd-nand-omap2.h | 3 +-
> >> 4 files changed, 155 insertions(+), 132 deletions(-)
> >
> > Also, this is going to be hard to manage across trees, as you touch
> > three drivers all at once. Is it not possible to split any of this apart
> > better?
>
> Will need some more effort and I can do it. Butm if we're going to start
> an with immutable branch with everything in, is it worth the effort?
I don't think I noticed the "everything in it" part. I was assuming
you'd have non-MTD changes in the immutable branch, and MTD stuff on
top. But that is indeed more difficult. I'm fine with everything going
in one branch, and pulling that all into MTD.
Then the hangup is that you'll need to have some of l2-mtd.git in that
branch as a prerequisite, if you're going to rebase. Or else I have to
fix it up when merging back into l2-mtd.git...
...
> >> diff --git a/include/linux/platform_data/mtd-nand-omap2.h
> >> b/include/linux/platform_data/mtd-nand-omap2.h
> >> index a067f58..ff27e5a 100644
> >> --- a/include/linux/platform_data/mtd-nand-omap2.h
> >> +++ b/include/linux/platform_data/mtd-nand-omap2.h
> >> @@ -76,11 +76,10 @@ struct omap_nand_platform_data {
> >>int devsize;
> >>enum omap_ecc ecc_opt;
> >>
> >> - /* for passing the partitions */
> >> - struct device_node *of_node;
> >>struct device_node *elm_of_node;
> >>
> >>/* deprecated */
> >>struct gpmc_nand_regs reg;
> >> + struct device_node *of_node;
> >
> > I'm a little confused here. Do you have a mixed platform data / device
> > tree setup here? That's odd. (It also seems if that was really
> > necessary, you could have the board file set pdev->dev.of_node before
> > registering it, then you don't need this field.) But really, if you're
> > partly using device tree, can't you just convert completely? Or is this
> > a two-phase process, and you're planning to convert omap2 to full device
> > tree?
>
> The existing device tree implementation for omap2-nand was like this:
> omap-gpmc.c driver was creating a platform device for the nand device and
> passing the device node via platform data.
>
> After this series we no longer do this and that's why of_node is marked
> deprecated in platform data. I might as well could just get rid of it
> but wasn't sure of how we're going to integrate the changes into the
> subsystem trees so just marked it deprecated.
Whoops, sorry I didn't read the "deprecated" header there this time. I
thought the movement was odd. *facepalm*
But yes, especially if we're putting everything in one branch, it'd be
more clear to simply kill off things instead of marking things
deprecated. It's especially confusing, since you're actually still using
the field.
Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 11/27] mtd: nand: omap: Clean up device tree support
Brian,
On 03/12/15 09:59, Brian Norris wrote:
> Hi Roger,
>
> On Tue, Oct 06, 2015 at 01:35:48PM +0300, Roger Quadros wrote:
>> Move NAND specific device tree parsing to NAND driver.
>>
>> The NAND controller node must have a compatible id, register space
>> resource and interrupt resource.
>>
>> Signed-off-by: Roger Quadros
>
> This one's going to need rebased, as there are some conflicting of_node
> changes in l2-mtd.git.
Al right. I'll rebase on top of l2-mtd.git
>
>> ---
>> v4: Warn if using older incompatible DT i.e. compatible property not present
>> in nand node.
>>
>> arch/arm/mach-omap2/gpmc-nand.c | 5 +-
>> drivers/memory/omap-gpmc.c | 143
>> +++
>> drivers/mtd/nand/omap2.c | 136 +
>> include/linux/platform_data/mtd-nand-omap2.h | 3 +-
>> 4 files changed, 155 insertions(+), 132 deletions(-)
>
> Also, this is going to be hard to manage across trees, as you touch
> three drivers all at once. Is it not possible to split any of this apart
> better?
Will need some more effort and I can do it. Butm if we're going to start
an with immutable branch with everything in, is it worth the effort?
>
>>
>> diff --git a/arch/arm/mach-omap2/gpmc-nand.c
>> b/arch/arm/mach-omap2/gpmc-nand.c
>> index ffe646a..e07ca27 100644
>> --- a/arch/arm/mach-omap2/gpmc-nand.c
>> +++ b/arch/arm/mach-omap2/gpmc-nand.c
>> @@ -95,10 +95,7 @@ int gpmc_nand_init(struct omap_nand_platform_data
>> *gpmc_nand_data,
>> gpmc_nand_res[1].start = gpmc_get_irq();
>>
>> memset(&s, 0, sizeof(struct gpmc_settings));
>> -if (gpmc_nand_data->of_node)
>> -gpmc_read_settings_dt(gpmc_nand_data->of_node, &s);
>> -else
>> -gpmc_set_legacy(gpmc_nand_data, &s);
>> +gpmc_set_legacy(gpmc_nand_data, &s);
>>
>> s.device_nand = true;
>>
>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
>> index e75226d..318c187 100644
>> --- a/drivers/memory/omap-gpmc.c
>> +++ b/drivers/memory/omap-gpmc.c
>> @@ -29,7 +29,6 @@
>> #include
>> #include
>> #include
>> -#include
>> #include
>>
>>
>> -ppdata.of_node = pdata->of_node;
>> -mtd_device_parse_register(mtd, NULL, &ppdata, pdata->parts,
>> - pdata->nr_parts);
>> +if (dev->of_node) {
>> +ppdata.of_node = dev->of_node;
>
> The latest l2-mtd.git changed how the partitions' of_node is passed. Now
> this is handled by nand_set_flash_node().
OK.
>
>> +mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
>> +
>> +} else {
>> +mtd_device_register(mtd, pdata->parts, pdata->nr_parts);
>> +}
>>
>> platform_set_drvdata(pdev, mtd);
>>
>> @@ -2083,11 +2173,17 @@ static int omap_nand_remove(struct platform_device
>> *pdev)
>> return 0;
>> }
>>
>> +static const struct of_device_id omap_nand_ids[] = {
>> +{ .compatible = "ti,omap2-nand", },
>> +{},
>> +};
>> +
>> static struct platform_driver omap_nand_driver = {
>> .probe = omap_nand_probe,
>> .remove = omap_nand_remove,
>> .driver = {
>> .name = DRIVER_NAME,
>> +.of_match_table = of_match_ptr(omap_nand_ids),
>> },
>> };
>>
>> diff --git a/include/linux/platform_data/mtd-nand-omap2.h
>> b/include/linux/platform_data/mtd-nand-omap2.h
>> index a067f58..ff27e5a 100644
>> --- a/include/linux/platform_data/mtd-nand-omap2.h
>> +++ b/include/linux/platform_data/mtd-nand-omap2.h
>> @@ -76,11 +76,10 @@ struct omap_nand_platform_data {
>> int devsize;
>> enum omap_ecc ecc_opt;
>>
>> -/* for passing the partitions */
>> -struct device_node *of_node;
>> struct device_node *elm_of_node;
>>
>> /* deprecated */
>> struct gpmc_nand_regs reg;
>> +struct device_node *of_node;
>
> I'm a little confused here. Do you have a mixed platform data / device
> tree setup here? That's odd. (It also seems if that was really
> necessary, you could have the board file set pdev->dev.of_node before
> registering it, then you don't need this field.) But really, if you're
> partly using device tree, can't you just convert completely? Or is this
> a two-phase process, and you're planning to convert omap2 to full device
> tree?
The existing device tree implementation for omap2-nand was like this:
omap-gpmc.c driver was creating a platform device for the nand device and
passing the device node via platform data.
After this series we no longer do this and that's why of_node is marked
deprecated in platform data. I might as well could just get rid of it
but wasn't sure of how we're going to integrate the changes into the
subsystem trees so just marked it deprecated.
cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at htt
Re: [PATCH v4 11/27] mtd: nand: omap: Clean up device tree support
Hi Roger,
On Tue, Oct 06, 2015 at 01:35:48PM +0300, Roger Quadros wrote:
> Move NAND specific device tree parsing to NAND driver.
>
> The NAND controller node must have a compatible id, register space
> resource and interrupt resource.
>
> Signed-off-by: Roger Quadros
This one's going to need rebased, as there are some conflicting of_node
changes in l2-mtd.git.
> ---
> v4: Warn if using older incompatible DT i.e. compatible property not present
> in nand node.
>
> arch/arm/mach-omap2/gpmc-nand.c | 5 +-
> drivers/memory/omap-gpmc.c | 143
> +++
> drivers/mtd/nand/omap2.c | 136 +
> include/linux/platform_data/mtd-nand-omap2.h | 3 +-
> 4 files changed, 155 insertions(+), 132 deletions(-)
Also, this is going to be hard to manage across trees, as you touch
three drivers all at once. Is it not possible to split any of this apart
better?
>
> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
> index ffe646a..e07ca27 100644
> --- a/arch/arm/mach-omap2/gpmc-nand.c
> +++ b/arch/arm/mach-omap2/gpmc-nand.c
> @@ -95,10 +95,7 @@ int gpmc_nand_init(struct omap_nand_platform_data
> *gpmc_nand_data,
> gpmc_nand_res[1].start = gpmc_get_irq();
>
> memset(&s, 0, sizeof(struct gpmc_settings));
> - if (gpmc_nand_data->of_node)
> - gpmc_read_settings_dt(gpmc_nand_data->of_node, &s);
> - else
> - gpmc_set_legacy(gpmc_nand_data, &s);
> + gpmc_set_legacy(gpmc_nand_data, &s);
>
> s.device_nand = true;
>
> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
> index e75226d..318c187 100644
> --- a/drivers/memory/omap-gpmc.c
> +++ b/drivers/memory/omap-gpmc.c
> @@ -29,7 +29,6 @@
> #include
> #include
> #include
> -#include
> #include
>
> #include
> @@ -1716,105 +1715,6 @@ static void __maybe_unused
> gpmc_read_timings_dt(struct device_node *np,
> of_property_read_bool(np, "gpmc,time-para-granularity");
> }
>
> -#if IS_ENABLED(CONFIG_MTD_NAND)
> -
> -static const char * const nand_xfer_types[] = {
> - [NAND_OMAP_PREFETCH_POLLED] = "prefetch-polled",
> - [NAND_OMAP_POLLED] = "polled",
> - [NAND_OMAP_PREFETCH_DMA]= "prefetch-dma",
> - [NAND_OMAP_PREFETCH_IRQ]= "prefetch-irq",
> -};
> -
> -static int gpmc_probe_nand_child(struct platform_device *pdev,
> - struct device_node *child)
> -{
> - u32 val;
> - const char *s;
> - struct gpmc_timings gpmc_t;
> - struct omap_nand_platform_data *gpmc_nand_data;
> -
> - if (of_property_read_u32(child, "reg", &val) < 0) {
> - dev_err(&pdev->dev, "%s has no 'reg' property\n",
> - child->full_name);
> - return -ENODEV;
> - }
> -
> - gpmc_nand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_nand_data),
> - GFP_KERNEL);
> - if (!gpmc_nand_data)
> - return -ENOMEM;
> -
> - gpmc_nand_data->cs = val;
> - gpmc_nand_data->of_node = child;
> -
> - /* Detect availability of ELM module */
> - gpmc_nand_data->elm_of_node = of_parse_phandle(child, "ti,elm-id", 0);
> - if (gpmc_nand_data->elm_of_node == NULL)
> - gpmc_nand_data->elm_of_node =
> - of_parse_phandle(child, "elm_id", 0);
> -
> - /* select ecc-scheme for NAND */
> - if (of_property_read_string(child, "ti,nand-ecc-opt", &s)) {
> - pr_err("%s: ti,nand-ecc-opt not found\n", __func__);
> - return -ENODEV;
> - }
> -
> - if (!strcmp(s, "sw"))
> - gpmc_nand_data->ecc_opt = OMAP_ECC_HAM1_CODE_SW;
> - else if (!strcmp(s, "ham1") ||
> - !strcmp(s, "hw") || !strcmp(s, "hw-romcode"))
> - gpmc_nand_data->ecc_opt =
> - OMAP_ECC_HAM1_CODE_HW;
> - else if (!strcmp(s, "bch4"))
> - if (gpmc_nand_data->elm_of_node)
> - gpmc_nand_data->ecc_opt =
> - OMAP_ECC_BCH4_CODE_HW;
> - else
> - gpmc_nand_data->ecc_opt =
> - OMAP_ECC_BCH4_CODE_HW_DETECTION_SW;
> - else if (!strcmp(s, "bch8"))
> - if (gpmc_nand_data->elm_of_node)
> - gpmc_nand_data->ecc_opt =
> - OMAP_ECC_BCH8_CODE_HW;
> - else
> - gpmc_nand_data->ecc_opt =
> - OMAP_ECC_BCH8_CODE_HW_DETECTION_SW;
> - else if (!strcmp(s, "bch16"))
> - if (gpmc_nand_data->elm_of_node)
> - gpmc_nand_data->ecc_opt =
> - OMAP_ECC_BCH16_CODE_HW;
> - else
> - pr_err("%s: BCH16 requires ELM support\n", __func__);
> - else
> - pr_err("%s: ti,na
