* [RESEND PATCH] mtd: nand: omap2: Remove omap_nand_platform_data
@ 2017-10-06 10:35 Ladislav Michl
2017-10-10 9:40 ` Boris Brezillon
0 siblings, 1 reply; 6+ messages in thread
From: Ladislav Michl @ 2017-10-06 10:35 UTC (permalink / raw)
To: linux-mtd; +Cc: Boris Brezillon, Richard Weinberger, linux-omap
As driver is now configured using DT, omap_nand_platform_data structure
is no longer needed.
Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
Resend to linux-mtd as other part this patch was depending on
was merged months ago via linux-omap tree...
drivers/mtd/nand/omap2.c | 40 +++++++---------------------
include/linux/platform_data/mtd-nand-omap2.h | 17 ------------
2 files changed, 10 insertions(+), 47 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 54540c8fa1a2..b1fc070c8279 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1588,8 +1588,7 @@ static bool is_elm_present(struct omap_nand_info *info,
return true;
}
-static bool omap2_nand_ecc_check(struct omap_nand_info *info,
- struct omap_nand_platform_data *pdata)
+static bool omap2_nand_ecc_check(struct omap_nand_info *info)
{
bool ecc_needs_bch, ecc_needs_omap_bch, ecc_needs_elm;
@@ -1804,7 +1803,6 @@ static const struct mtd_ooblayout_ops omap_sw_ooblayout_ops = {
static int omap_nand_probe(struct platform_device *pdev)
{
struct omap_nand_info *info;
- struct omap_nand_platform_data *pdata = NULL;
struct mtd_info *mtd;
struct nand_chip *nand_chip;
int err;
@@ -1814,6 +1812,9 @@ static int omap_nand_probe(struct platform_device *pdev)
int min_oobbytes = BADBLOCK_MARKER_LENGTH;
int oobbytes_per_step;
+ if (!dev->of_node)
+ return -EINVAL;
+
info = devm_kzalloc(&pdev->dev, sizeof(struct omap_nand_info),
GFP_KERNEL);
if (!info)
@@ -1821,29 +1822,9 @@ static int omap_nand_probe(struct platform_device *pdev)
info->pdev = pdev;
- if (dev->of_node) {
- if (omap_get_dt_info(dev, info))
- return -EINVAL;
- } else {
- pdata = dev_get_platdata(&pdev->dev);
- if (!pdata) {
- dev_err(&pdev->dev, "platform data missing\n");
- return -EINVAL;
- }
-
- info->gpmc_cs = pdata->cs;
- info->reg = pdata->reg;
- info->ecc_opt = pdata->ecc_opt;
- if (pdata->dev_ready)
- dev_info(&pdev->dev, "pdata->dev_ready is deprecated\n");
-
- info->xfer_type = pdata->xfer_type;
- info->devsize = pdata->devsize;
- info->elm_of_node = pdata->elm_of_node;
- info->flash_bbt = pdata->flash_bbt;
- }
+ if (omap_get_dt_info(dev, info))
+ return -EINVAL;
- platform_set_drvdata(pdev, info);
info->ops = gpmc_omap_get_nand_ops(&info->reg, info->gpmc_cs);
if (!info->ops) {
dev_err(&pdev->dev, "Failed to get GPMC->NAND interface\n");
@@ -2002,7 +1983,7 @@ static int omap_nand_probe(struct platform_device *pdev)
goto return_error;
}
- if (!omap2_nand_ecc_check(info, pdata)) {
+ if (!omap2_nand_ecc_check(info)) {
err = -EINVAL;
goto return_error;
}
@@ -2167,10 +2148,9 @@ static int omap_nand_probe(struct platform_device *pdev)
if (err)
goto return_error;
- if (dev->of_node)
- mtd_device_register(mtd, NULL, 0);
- else
- mtd_device_register(mtd, pdata->parts, pdata->nr_parts);
+ err = mtd_device_register(mtd, NULL, 0);
+ if (err)
+ goto return_error;
platform_set_drvdata(pdev, mtd);
diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
index 25e267f1970c..619df2431e75 100644
--- a/include/linux/platform_data/mtd-nand-omap2.h
+++ b/include/linux/platform_data/mtd-nand-omap2.h
@@ -64,21 +64,4 @@ struct gpmc_nand_regs {
void __iomem *gpmc_bch_result5[GPMC_BCH_NUM_REMAINDER];
void __iomem *gpmc_bch_result6[GPMC_BCH_NUM_REMAINDER];
};
-
-struct omap_nand_platform_data {
- int cs;
- struct mtd_partition *parts;
- int nr_parts;
- bool flash_bbt;
- enum nand_io xfer_type;
- int devsize;
- enum omap_ecc ecc_opt;
-
- struct device_node *elm_of_node;
-
- /* deprecated */
- struct gpmc_nand_regs reg;
- struct device_node *of_node;
- bool dev_ready;
-};
#endif
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] mtd: nand: omap2: Remove omap_nand_platform_data
2017-10-06 10:35 [RESEND PATCH] mtd: nand: omap2: Remove omap_nand_platform_data Ladislav Michl
@ 2017-10-10 9:40 ` Boris Brezillon
2017-10-12 11:28 ` Roger Quadros
0 siblings, 1 reply; 6+ messages in thread
From: Boris Brezillon @ 2017-10-10 9:40 UTC (permalink / raw)
To: Ladislav Michl; +Cc: linux-mtd, linux-omap, Richard Weinberger
On Fri, 6 Oct 2017 12:35:43 +0200
Ladislav Michl <ladis@linux-mips.org> wrote:
> As driver is now configured using DT, omap_nand_platform_data structure
> is no longer needed.
>
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> ---
> Resend to linux-mtd as other part this patch was depending on
> was merged months ago via linux-omap tree...
>
> drivers/mtd/nand/omap2.c | 40 +++++++---------------------
> include/linux/platform_data/mtd-nand-omap2.h | 17 ------------
> 2 files changed, 10 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index 54540c8fa1a2..b1fc070c8279 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -1588,8 +1588,7 @@ static bool is_elm_present(struct omap_nand_info *info,
> return true;
> }
>
> -static bool omap2_nand_ecc_check(struct omap_nand_info *info,
> - struct omap_nand_platform_data *pdata)
> +static bool omap2_nand_ecc_check(struct omap_nand_info *info)
> {
> bool ecc_needs_bch, ecc_needs_omap_bch, ecc_needs_elm;
>
> @@ -1804,7 +1803,6 @@ static const struct mtd_ooblayout_ops omap_sw_ooblayout_ops = {
> static int omap_nand_probe(struct platform_device *pdev)
> {
> struct omap_nand_info *info;
> - struct omap_nand_platform_data *pdata = NULL;
> struct mtd_info *mtd;
> struct nand_chip *nand_chip;
> int err;
> @@ -1814,6 +1812,9 @@ static int omap_nand_probe(struct platform_device *pdev)
> int min_oobbytes = BADBLOCK_MARKER_LENGTH;
> int oobbytes_per_step;
>
> + if (!dev->of_node)
> + return -EINVAL;
> +
Is this really needed? I expect omap_get_dt_info() to return an error
when dev->of_node is NULL.
> info = devm_kzalloc(&pdev->dev, sizeof(struct omap_nand_info),
> GFP_KERNEL);
> if (!info)
> @@ -1821,29 +1822,9 @@ static int omap_nand_probe(struct platform_device *pdev)
>
> info->pdev = pdev;
>
> - if (dev->of_node) {
> - if (omap_get_dt_info(dev, info))
> - return -EINVAL;
> - } else {
> - pdata = dev_get_platdata(&pdev->dev);
> - if (!pdata) {
> - dev_err(&pdev->dev, "platform data missing\n");
> - return -EINVAL;
> - }
> -
> - info->gpmc_cs = pdata->cs;
> - info->reg = pdata->reg;
> - info->ecc_opt = pdata->ecc_opt;
> - if (pdata->dev_ready)
> - dev_info(&pdev->dev, "pdata->dev_ready is deprecated\n");
> -
> - info->xfer_type = pdata->xfer_type;
> - info->devsize = pdata->devsize;
> - info->elm_of_node = pdata->elm_of_node;
> - info->flash_bbt = pdata->flash_bbt;
> - }
> + if (omap_get_dt_info(dev, info))
> + return -EINVAL;
>
> - platform_set_drvdata(pdev, info);
This removal seems unrelated to the change you're describing in the
commit log. I'm not saying we should keep this platform_set_drvdata()
if it's useless, but it should be done in a separate patch.
> info->ops = gpmc_omap_get_nand_ops(&info->reg, info->gpmc_cs);
> if (!info->ops) {
> dev_err(&pdev->dev, "Failed to get GPMC->NAND interface\n");
> @@ -2002,7 +1983,7 @@ static int omap_nand_probe(struct platform_device *pdev)
> goto return_error;
> }
>
> - if (!omap2_nand_ecc_check(info, pdata)) {
> + if (!omap2_nand_ecc_check(info)) {
> err = -EINVAL;
> goto return_error;
> }
> @@ -2167,10 +2148,9 @@ static int omap_nand_probe(struct platform_device *pdev)
> if (err)
> goto return_error;
>
> - if (dev->of_node)
> - mtd_device_register(mtd, NULL, 0);
> - else
> - mtd_device_register(mtd, pdata->parts, pdata->nr_parts);
> + err = mtd_device_register(mtd, NULL, 0);
> + if (err)
> + goto return_error;
>
> platform_set_drvdata(pdev, mtd);
>
> diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
> index 25e267f1970c..619df2431e75 100644
> --- a/include/linux/platform_data/mtd-nand-omap2.h
> +++ b/include/linux/platform_data/mtd-nand-omap2.h
> @@ -64,21 +64,4 @@ struct gpmc_nand_regs {
> void __iomem *gpmc_bch_result5[GPMC_BCH_NUM_REMAINDER];
> void __iomem *gpmc_bch_result6[GPMC_BCH_NUM_REMAINDER];
> };
> -
> -struct omap_nand_platform_data {
> - int cs;
> - struct mtd_partition *parts;
> - int nr_parts;
> - bool flash_bbt;
> - enum nand_io xfer_type;
> - int devsize;
> - enum omap_ecc ecc_opt;
> -
> - struct device_node *elm_of_node;
> -
> - /* deprecated */
> - struct gpmc_nand_regs reg;
> - struct device_node *of_node;
> - bool dev_ready;
> -};
> #endif
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] mtd: nand: omap2: Remove omap_nand_platform_data
2017-10-10 9:40 ` Boris Brezillon
@ 2017-10-12 11:28 ` Roger Quadros
2017-10-12 11:38 ` Boris Brezillon
0 siblings, 1 reply; 6+ messages in thread
From: Roger Quadros @ 2017-10-12 11:28 UTC (permalink / raw)
To: Boris Brezillon, Ladislav Michl; +Cc: linux-mtd, linux-omap, Richard Weinberger
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 10/10/17 12:40, Boris Brezillon wrote:
> On Fri, 6 Oct 2017 12:35:43 +0200
> Ladislav Michl <ladis@linux-mips.org> wrote:
>
>> As driver is now configured using DT, omap_nand_platform_data structure
>> is no longer needed.
>>
>> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
>> ---
>> Resend to linux-mtd as other part this patch was depending on
>> was merged months ago via linux-omap tree...
>>
>> drivers/mtd/nand/omap2.c | 40 +++++++---------------------
>> include/linux/platform_data/mtd-nand-omap2.h | 17 ------------
>> 2 files changed, 10 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>> index 54540c8fa1a2..b1fc070c8279 100644
>> --- a/drivers/mtd/nand/omap2.c
>> +++ b/drivers/mtd/nand/omap2.c
>> @@ -1588,8 +1588,7 @@ static bool is_elm_present(struct omap_nand_info *info,
>> return true;
>> }
>>
>> -static bool omap2_nand_ecc_check(struct omap_nand_info *info,
>> - struct omap_nand_platform_data *pdata)
>> +static bool omap2_nand_ecc_check(struct omap_nand_info *info)
>> {
>> bool ecc_needs_bch, ecc_needs_omap_bch, ecc_needs_elm;
>>
>> @@ -1804,7 +1803,6 @@ static const struct mtd_ooblayout_ops omap_sw_ooblayout_ops = {
>> static int omap_nand_probe(struct platform_device *pdev)
>> {
>> struct omap_nand_info *info;
>> - struct omap_nand_platform_data *pdata = NULL;
>> struct mtd_info *mtd;
>> struct nand_chip *nand_chip;
>> int err;
>> @@ -1814,6 +1812,9 @@ static int omap_nand_probe(struct platform_device *pdev)
>> int min_oobbytes = BADBLOCK_MARKER_LENGTH;
>> int oobbytes_per_step;
>>
>> + if (!dev->of_node)
>> + return -EINVAL;
>> +
>
> Is this really needed? I expect omap_get_dt_info() to return an error
> when dev->of_node is NULL.
>
>> info = devm_kzalloc(&pdev->dev, sizeof(struct omap_nand_info),
>> GFP_KERNEL);
>> if (!info)
>> @@ -1821,29 +1822,9 @@ static int omap_nand_probe(struct platform_device *pdev)
>>
>> info->pdev = pdev;
>>
>> - if (dev->of_node) {
>> - if (omap_get_dt_info(dev, info))
>> - return -EINVAL;
>> - } else {
>> - pdata = dev_get_platdata(&pdev->dev);
>> - if (!pdata) {
>> - dev_err(&pdev->dev, "platform data missing\n");
>> - return -EINVAL;
>> - }
>> -
>> - info->gpmc_cs = pdata->cs;
>> - info->reg = pdata->reg;
>> - info->ecc_opt = pdata->ecc_opt;
>> - if (pdata->dev_ready)
>> - dev_info(&pdev->dev, "pdata->dev_ready is deprecated\n");
>> -
>> - info->xfer_type = pdata->xfer_type;
>> - info->devsize = pdata->devsize;
>> - info->elm_of_node = pdata->elm_of_node;
>> - info->flash_bbt = pdata->flash_bbt;
>> - }
>> + if (omap_get_dt_info(dev, info))
>> + return -EINVAL;
how about
err = omap_get_dt_info(dev, info);
if (err)
return err;
>>
>> - platform_set_drvdata(pdev, info);
>
> This removal seems unrelated to the change you're describing in the
> commit log. I'm not saying we should keep this platform_set_drvdata()
> if it's useless, but it should be done in a separate patch.
we do use platform_get_drvdata() in omap_nand_remove() so I suppose
we can't get rid of platorm_set_drvdata().
We need to test if the patch survives a suspend/resume.
>
>> info->ops = gpmc_omap_get_nand_ops(&info->reg, info->gpmc_cs);
>> if (!info->ops) {
>> dev_err(&pdev->dev, "Failed to get GPMC->NAND interface\n");
>> @@ -2002,7 +1983,7 @@ static int omap_nand_probe(struct platform_device *pdev)
>> goto return_error;
>> }
>>
>> - if (!omap2_nand_ecc_check(info, pdata)) {
>> + if (!omap2_nand_ecc_check(info)) {
>> err = -EINVAL;
>> goto return_error;
>> }
>> @@ -2167,10 +2148,9 @@ static int omap_nand_probe(struct platform_device *pdev)
>> if (err)
>> goto return_error;
>>
>> - if (dev->of_node)
>> - mtd_device_register(mtd, NULL, 0);
>> - else
>> - mtd_device_register(mtd, pdata->parts, pdata->nr_parts);
>> + err = mtd_device_register(mtd, NULL, 0);
>> + if (err)
>> + goto return_error;
>>
>> platform_set_drvdata(pdev, mtd);
>>
>> diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
>> index 25e267f1970c..619df2431e75 100644
>> --- a/include/linux/platform_data/mtd-nand-omap2.h
>> +++ b/include/linux/platform_data/mtd-nand-omap2.h
>> @@ -64,21 +64,4 @@ struct gpmc_nand_regs {
>> void __iomem *gpmc_bch_result5[GPMC_BCH_NUM_REMAINDER];
>> void __iomem *gpmc_bch_result6[GPMC_BCH_NUM_REMAINDER];
>> };
>> -
>> -struct omap_nand_platform_data {
>> - int cs;
>> - struct mtd_partition *parts;
>> - int nr_parts;
>> - bool flash_bbt;
>> - enum nand_io xfer_type;
>> - int devsize;
>> - enum omap_ecc ecc_opt;
>> -
>> - struct device_node *elm_of_node;
>> -
>> - /* deprecated */
>> - struct gpmc_nand_regs reg;
>> - struct device_node *of_node;
>> - bool dev_ready;
>> -};
>> #endif
>>
>> ______________________________________________________
>> Linux MTD discussion mailing list
>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
cheers,
-roger
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] mtd: nand: omap2: Remove omap_nand_platform_data
2017-10-12 11:28 ` Roger Quadros
@ 2017-10-12 11:38 ` Boris Brezillon
2017-10-12 14:09 ` Roger Quadros
0 siblings, 1 reply; 6+ messages in thread
From: Boris Brezillon @ 2017-10-12 11:38 UTC (permalink / raw)
To: Roger Quadros; +Cc: Ladislav Michl, linux-mtd, linux-omap, Richard Weinberger
On Thu, 12 Oct 2017 14:28:45 +0300
Roger Quadros <rogerq@ti.com> wrote:
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
> On 10/10/17 12:40, Boris Brezillon wrote:
> > On Fri, 6 Oct 2017 12:35:43 +0200
> > Ladislav Michl <ladis@linux-mips.org> wrote:
> >
> >> As driver is now configured using DT, omap_nand_platform_data structure
> >> is no longer needed.
> >>
> >> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> >> ---
> >> Resend to linux-mtd as other part this patch was depending on
> >> was merged months ago via linux-omap tree...
> >>
> >> drivers/mtd/nand/omap2.c | 40 +++++++---------------------
> >> include/linux/platform_data/mtd-nand-omap2.h | 17 ------------
> >> 2 files changed, 10 insertions(+), 47 deletions(-)
> >>
> >> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> >> index 54540c8fa1a2..b1fc070c8279 100644
> >> --- a/drivers/mtd/nand/omap2.c
> >> +++ b/drivers/mtd/nand/omap2.c
> >> @@ -1588,8 +1588,7 @@ static bool is_elm_present(struct omap_nand_info *info,
> >> return true;
> >> }
> >>
> >> -static bool omap2_nand_ecc_check(struct omap_nand_info *info,
> >> - struct omap_nand_platform_data *pdata)
> >> +static bool omap2_nand_ecc_check(struct omap_nand_info *info)
> >> {
> >> bool ecc_needs_bch, ecc_needs_omap_bch, ecc_needs_elm;
> >>
> >> @@ -1804,7 +1803,6 @@ static const struct mtd_ooblayout_ops omap_sw_ooblayout_ops = {
> >> static int omap_nand_probe(struct platform_device *pdev)
> >> {
> >> struct omap_nand_info *info;
> >> - struct omap_nand_platform_data *pdata = NULL;
> >> struct mtd_info *mtd;
> >> struct nand_chip *nand_chip;
> >> int err;
> >> @@ -1814,6 +1812,9 @@ static int omap_nand_probe(struct platform_device *pdev)
> >> int min_oobbytes = BADBLOCK_MARKER_LENGTH;
> >> int oobbytes_per_step;
> >>
> >> + if (!dev->of_node)
> >> + return -EINVAL;
> >> +
> >
> > Is this really needed? I expect omap_get_dt_info() to return an error
> > when dev->of_node is NULL.
> >
> >> info = devm_kzalloc(&pdev->dev, sizeof(struct omap_nand_info),
> >> GFP_KERNEL);
> >> if (!info)
> >> @@ -1821,29 +1822,9 @@ static int omap_nand_probe(struct platform_device *pdev)
> >>
> >> info->pdev = pdev;
> >>
> >> - if (dev->of_node) {
> >> - if (omap_get_dt_info(dev, info))
> >> - return -EINVAL;
> >> - } else {
> >> - pdata = dev_get_platdata(&pdev->dev);
> >> - if (!pdata) {
> >> - dev_err(&pdev->dev, "platform data missing\n");
> >> - return -EINVAL;
> >> - }
> >> -
> >> - info->gpmc_cs = pdata->cs;
> >> - info->reg = pdata->reg;
> >> - info->ecc_opt = pdata->ecc_opt;
> >> - if (pdata->dev_ready)
> >> - dev_info(&pdev->dev, "pdata->dev_ready is deprecated\n");
> >> -
> >> - info->xfer_type = pdata->xfer_type;
> >> - info->devsize = pdata->devsize;
> >> - info->elm_of_node = pdata->elm_of_node;
> >> - info->flash_bbt = pdata->flash_bbt;
> >> - }
> >> + if (omap_get_dt_info(dev, info))
> >> + return -EINVAL;
>
> how about
>
> err = omap_get_dt_info(dev, info);
> if (err)
> return err;
Already done like that in v2 [1]
>
> >>
> >> - platform_set_drvdata(pdev, info);
> >
> > This removal seems unrelated to the change you're describing in the
> > commit log. I'm not saying we should keep this platform_set_drvdata()
> > if it's useless, but it should be done in a separate patch.
>
> we do use platform_get_drvdata() in omap_nand_remove() so I suppose
> we can't get rid of platorm_set_drvdata().
There's another platform_set_drvdata() later in the probe function, I
think this one is indeed useless (see patch [2])
[1]http://patchwork.ozlabs.org/patch/823820/
[2]http://patchwork.ozlabs.org/patch/823839/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] mtd: nand: omap2: Remove omap_nand_platform_data
2017-10-12 11:38 ` Boris Brezillon
@ 2017-10-12 14:09 ` Roger Quadros
2017-10-12 14:15 ` Boris Brezillon
0 siblings, 1 reply; 6+ messages in thread
From: Roger Quadros @ 2017-10-12 14:09 UTC (permalink / raw)
To: Boris Brezillon; +Cc: Ladislav Michl, linux-mtd, linux-omap, Richard Weinberger
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 12/10/17 14:38, Boris Brezillon wrote:
> On Thu, 12 Oct 2017 14:28:45 +0300
> Roger Quadros <rogerq@ti.com> wrote:
>
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>>
>> On 10/10/17 12:40, Boris Brezillon wrote:
>>> On Fri, 6 Oct 2017 12:35:43 +0200
>>> Ladislav Michl <ladis@linux-mips.org> wrote:
>>>
>>>> As driver is now configured using DT, omap_nand_platform_data structure
>>>> is no longer needed.
>>>>
>>>> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
>>>> ---
>>>> Resend to linux-mtd as other part this patch was depending on
>>>> was merged months ago via linux-omap tree...
>>>>
>>>> drivers/mtd/nand/omap2.c | 40 +++++++---------------------
>>>> include/linux/platform_data/mtd-nand-omap2.h | 17 ------------
>>>> 2 files changed, 10 insertions(+), 47 deletions(-)
>>>>
>>>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>>>> index 54540c8fa1a2..b1fc070c8279 100644
>>>> --- a/drivers/mtd/nand/omap2.c
>>>> +++ b/drivers/mtd/nand/omap2.c
>>>> @@ -1588,8 +1588,7 @@ static bool is_elm_present(struct omap_nand_info *info,
>>>> return true;
>>>> }
>>>>
>>>> -static bool omap2_nand_ecc_check(struct omap_nand_info *info,
>>>> - struct omap_nand_platform_data *pdata)
>>>> +static bool omap2_nand_ecc_check(struct omap_nand_info *info)
>>>> {
>>>> bool ecc_needs_bch, ecc_needs_omap_bch, ecc_needs_elm;
>>>>
>>>> @@ -1804,7 +1803,6 @@ static const struct mtd_ooblayout_ops omap_sw_ooblayout_ops = {
>>>> static int omap_nand_probe(struct platform_device *pdev)
>>>> {
>>>> struct omap_nand_info *info;
>>>> - struct omap_nand_platform_data *pdata = NULL;
>>>> struct mtd_info *mtd;
>>>> struct nand_chip *nand_chip;
>>>> int err;
>>>> @@ -1814,6 +1812,9 @@ static int omap_nand_probe(struct platform_device *pdev)
>>>> int min_oobbytes = BADBLOCK_MARKER_LENGTH;
>>>> int oobbytes_per_step;
>>>>
>>>> + if (!dev->of_node)
>>>> + return -EINVAL;
>>>> +
>>>
>>> Is this really needed? I expect omap_get_dt_info() to return an error
>>> when dev->of_node is NULL.
>>>
>>>> info = devm_kzalloc(&pdev->dev, sizeof(struct omap_nand_info),
>>>> GFP_KERNEL);
>>>> if (!info)
>>>> @@ -1821,29 +1822,9 @@ static int omap_nand_probe(struct platform_device *pdev)
>>>>
>>>> info->pdev = pdev;
>>>>
>>>> - if (dev->of_node) {
>>>> - if (omap_get_dt_info(dev, info))
>>>> - return -EINVAL;
>>>> - } else {
>>>> - pdata = dev_get_platdata(&pdev->dev);
>>>> - if (!pdata) {
>>>> - dev_err(&pdev->dev, "platform data missing\n");
>>>> - return -EINVAL;
>>>> - }
>>>> -
>>>> - info->gpmc_cs = pdata->cs;
>>>> - info->reg = pdata->reg;
>>>> - info->ecc_opt = pdata->ecc_opt;
>>>> - if (pdata->dev_ready)
>>>> - dev_info(&pdev->dev, "pdata->dev_ready is deprecated\n");
>>>> -
>>>> - info->xfer_type = pdata->xfer_type;
>>>> - info->devsize = pdata->devsize;
>>>> - info->elm_of_node = pdata->elm_of_node;
>>>> - info->flash_bbt = pdata->flash_bbt;
>>>> - }
>>>> + if (omap_get_dt_info(dev, info))
>>>> + return -EINVAL;
>>
>> how about
>>
>> err = omap_get_dt_info(dev, info);
>> if (err)
>> return err;
>
> Already done like that in v2 [1]
>
>>
>>>>
>>>> - platform_set_drvdata(pdev, info);
>>>
>>> This removal seems unrelated to the change you're describing in the
>>> commit log. I'm not saying we should keep this platform_set_drvdata()
>>> if it's useless, but it should be done in a separate patch.
>>
>> we do use platform_get_drvdata() in omap_nand_remove() so I suppose
>> we can't get rid of platorm_set_drvdata()
>
> There's another platform_set_drvdata() later in the probe function, I
> think this one is indeed useless (see patch [2])
I agree.
>
> [1]http://patchwork.ozlabs.org/patch/823820/
> [2]http://patchwork.ozlabs.org/patch/823839/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
cheers,
-roger
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] mtd: nand: omap2: Remove omap_nand_platform_data
2017-10-12 14:09 ` Roger Quadros
@ 2017-10-12 14:15 ` Boris Brezillon
0 siblings, 0 replies; 6+ messages in thread
From: Boris Brezillon @ 2017-10-12 14:15 UTC (permalink / raw)
To: Roger Quadros; +Cc: Ladislav Michl, linux-mtd, linux-omap, Richard Weinberger
On Thu, 12 Oct 2017 17:09:26 +0300
Roger Quadros <rogerq@ti.com> wrote:
>
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
> On 12/10/17 14:38, Boris Brezillon wrote:
> > On Thu, 12 Oct 2017 14:28:45 +0300
> > Roger Quadros <rogerq@ti.com> wrote:
> >
> >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> >>
> >> On 10/10/17 12:40, Boris Brezillon wrote:
> >>> On Fri, 6 Oct 2017 12:35:43 +0200
> >>> Ladislav Michl <ladis@linux-mips.org> wrote:
> >>>
> >>>> As driver is now configured using DT, omap_nand_platform_data structure
> >>>> is no longer needed.
> >>>>
> >>>> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> >>>> ---
> >>>> Resend to linux-mtd as other part this patch was depending on
> >>>> was merged months ago via linux-omap tree...
> >>>>
> >>>> drivers/mtd/nand/omap2.c | 40 +++++++---------------------
> >>>> include/linux/platform_data/mtd-nand-omap2.h | 17 ------------
> >>>> 2 files changed, 10 insertions(+), 47 deletions(-)
> >>>>
> >>>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> >>>> index 54540c8fa1a2..b1fc070c8279 100644
> >>>> --- a/drivers/mtd/nand/omap2.c
> >>>> +++ b/drivers/mtd/nand/omap2.c
> >>>> @@ -1588,8 +1588,7 @@ static bool is_elm_present(struct omap_nand_info *info,
> >>>> return true;
> >>>> }
> >>>>
> >>>> -static bool omap2_nand_ecc_check(struct omap_nand_info *info,
> >>>> - struct omap_nand_platform_data *pdata)
> >>>> +static bool omap2_nand_ecc_check(struct omap_nand_info *info)
> >>>> {
> >>>> bool ecc_needs_bch, ecc_needs_omap_bch, ecc_needs_elm;
> >>>>
> >>>> @@ -1804,7 +1803,6 @@ static const struct mtd_ooblayout_ops omap_sw_ooblayout_ops = {
> >>>> static int omap_nand_probe(struct platform_device *pdev)
> >>>> {
> >>>> struct omap_nand_info *info;
> >>>> - struct omap_nand_platform_data *pdata = NULL;
> >>>> struct mtd_info *mtd;
> >>>> struct nand_chip *nand_chip;
> >>>> int err;
> >>>> @@ -1814,6 +1812,9 @@ static int omap_nand_probe(struct platform_device *pdev)
> >>>> int min_oobbytes = BADBLOCK_MARKER_LENGTH;
> >>>> int oobbytes_per_step;
> >>>>
> >>>> + if (!dev->of_node)
> >>>> + return -EINVAL;
> >>>> +
> >>>
> >>> Is this really needed? I expect omap_get_dt_info() to return an error
> >>> when dev->of_node is NULL.
> >>>
> >>>> info = devm_kzalloc(&pdev->dev, sizeof(struct omap_nand_info),
> >>>> GFP_KERNEL);
> >>>> if (!info)
> >>>> @@ -1821,29 +1822,9 @@ static int omap_nand_probe(struct platform_device *pdev)
> >>>>
> >>>> info->pdev = pdev;
> >>>>
> >>>> - if (dev->of_node) {
> >>>> - if (omap_get_dt_info(dev, info))
> >>>> - return -EINVAL;
> >>>> - } else {
> >>>> - pdata = dev_get_platdata(&pdev->dev);
> >>>> - if (!pdata) {
> >>>> - dev_err(&pdev->dev, "platform data missing\n");
> >>>> - return -EINVAL;
> >>>> - }
> >>>> -
> >>>> - info->gpmc_cs = pdata->cs;
> >>>> - info->reg = pdata->reg;
> >>>> - info->ecc_opt = pdata->ecc_opt;
> >>>> - if (pdata->dev_ready)
> >>>> - dev_info(&pdev->dev, "pdata->dev_ready is deprecated\n");
> >>>> -
> >>>> - info->xfer_type = pdata->xfer_type;
> >>>> - info->devsize = pdata->devsize;
> >>>> - info->elm_of_node = pdata->elm_of_node;
> >>>> - info->flash_bbt = pdata->flash_bbt;
> >>>> - }
> >>>> + if (omap_get_dt_info(dev, info))
> >>>> + return -EINVAL;
> >>
> >> how about
> >>
> >> err = omap_get_dt_info(dev, info);
> >> if (err)
> >> return err;
> >
> > Already done like that in v2 [1]
> >
> >>
> >>>>
> >>>> - platform_set_drvdata(pdev, info);
> >>>
> >>> This removal seems unrelated to the change you're describing in the
> >>> commit log. I'm not saying we should keep this platform_set_drvdata()
> >>> if it's useless, but it should be done in a separate patch.
> >>
> >> we do use platform_get_drvdata() in omap_nand_remove() so I suppose
> >> we can't get rid of platorm_set_drvdata()
> >
> > There's another platform_set_drvdata() later in the probe function, I
> > think this one is indeed useless (see patch [2])
>
> I agree.
>
> >
> > [1]http://patchwork.ozlabs.org/patch/823820/
> > [2]http://patchwork.ozlabs.org/patch/823839/
Feel free to add your Acked-by/Tested-by on these patches.
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-12 14:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-06 10:35 [RESEND PATCH] mtd: nand: omap2: Remove omap_nand_platform_data Ladislav Michl
2017-10-10 9:40 ` Boris Brezillon
2017-10-12 11:28 ` Roger Quadros
2017-10-12 11:38 ` Boris Brezillon
2017-10-12 14:09 ` Roger Quadros
2017-10-12 14:15 ` Boris Brezillon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).