| Message ID | 20211217102945.17432-4-rogerq@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | memory: omap-gpmc: Add AM64 SoC support | expand |
On 17/12/2021 11:29, Roger Quadros wrote: > As more compatibles can be added to the GPMC NAND controller driver > use a compatible match table. > > Signed-off-by: Roger Quadros <rogerq@kernel.org> > --- > drivers/memory/omap-gpmc.c | 8 +++++++- > drivers/mtd/nand/raw/omap2.c | 2 +- > include/linux/platform_data/mtd-nand-omap2.h | 5 +++++ > 3 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c > index 624153048182..814ddb45c13d 100644 > --- a/drivers/memory/omap-gpmc.c > +++ b/drivers/memory/omap-gpmc.c > @@ -2091,6 +2091,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, > u32 val; > struct gpio_desc *waitpin_desc = NULL; > struct gpmc_device *gpmc = platform_get_drvdata(pdev); > + bool is_nand = false; > > if (of_property_read_u32(child, "reg", &cs) < 0) { > dev_err(&pdev->dev, "%pOF has no 'reg' property\n", > @@ -2183,7 +2184,12 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, > } > } > > - if (of_device_is_compatible(child, "ti,omap2-nand")) { > +#if defined(CONFIG_MTD_NAND_OMAP2) if (IS_ENABLED()) is preferred. If needed, you could make omap_nand_ids symbol visible always (so without ifdef around it), because extern structure should not have impact when not defined (if I recall correctly...). > + if (of_match_node(omap_nand_ids, child)) > + is_nand = true; > +#endif > + > + if (is_nand) { > /* NAND specific setup */ > val = 8; > of_property_read_u32(child, "nand-bus-width", &val); > diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c > index b26d4947af02..fff834ee726f 100644 > --- a/drivers/mtd/nand/raw/omap2.c > +++ b/drivers/mtd/nand/raw/omap2.c > @@ -2352,7 +2352,7 @@ static int omap_nand_remove(struct platform_device *pdev) > return ret; > } > > -static const struct of_device_id omap_nand_ids[] = { > +const struct of_device_id omap_nand_ids[] = { > { .compatible = "ti,omap2-nand", }, > {}, > }; I think OMAP2 NAND driver can be a module, so this should have EXPORT_SYMBOL. > diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h > index de6ada739121..e1bb90a8db03 100644 > --- a/include/linux/platform_data/mtd-nand-omap2.h > +++ b/include/linux/platform_data/mtd-nand-omap2.h > @@ -61,4 +61,9 @@ struct gpmc_nand_regs { > void __iomem *gpmc_bch_result5[GPMC_BCH_NUM_REMAINDER]; > void __iomem *gpmc_bch_result6[GPMC_BCH_NUM_REMAINDER]; > }; > + > +#if defined(CONFIG_MTD_NAND_OMAP2) > +extern const struct of_device_id omap_nand_ids[]; > +#endif > + > #endif > Best regards, Krzysztof
On 17/12/2021 17:21, Krzysztof Kozlowski wrote: > On 17/12/2021 11:29, Roger Quadros wrote: >> As more compatibles can be added to the GPMC NAND controller driver >> use a compatible match table. >> >> Signed-off-by: Roger Quadros <rogerq@kernel.org> >> --- >> drivers/memory/omap-gpmc.c | 8 +++++++- >> drivers/mtd/nand/raw/omap2.c | 2 +- >> include/linux/platform_data/mtd-nand-omap2.h | 5 +++++ >> 3 files changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c >> index 624153048182..814ddb45c13d 100644 >> --- a/drivers/memory/omap-gpmc.c >> +++ b/drivers/memory/omap-gpmc.c >> @@ -2091,6 +2091,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, >> u32 val; >> struct gpio_desc *waitpin_desc = NULL; >> struct gpmc_device *gpmc = platform_get_drvdata(pdev); >> + bool is_nand = false; >> >> if (of_property_read_u32(child, "reg", &cs) < 0) { >> dev_err(&pdev->dev, "%pOF has no 'reg' property\n", >> @@ -2183,7 +2184,12 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, >> } >> } >> >> - if (of_device_is_compatible(child, "ti,omap2-nand")) { >> +#if defined(CONFIG_MTD_NAND_OMAP2) > > if (IS_ENABLED()) is preferred. If needed, you could make omap_nand_ids > symbol visible always (so without ifdef around it), because extern > structure should not have impact when not defined (if I recall > correctly...). OK. > >> + if (of_match_node(omap_nand_ids, child)) >> + is_nand = true; >> +#endif >> + >> + if (is_nand) { >> /* NAND specific setup */ >> val = 8; >> of_property_read_u32(child, "nand-bus-width", &val); >> diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c >> index b26d4947af02..fff834ee726f 100644 >> --- a/drivers/mtd/nand/raw/omap2.c >> +++ b/drivers/mtd/nand/raw/omap2.c >> @@ -2352,7 +2352,7 @@ static int omap_nand_remove(struct platform_device *pdev) >> return ret; >> } >> >> -static const struct of_device_id omap_nand_ids[] = { >> +const struct of_device_id omap_nand_ids[] = { >> { .compatible = "ti,omap2-nand", }, >> {}, >> }; > > I think OMAP2 NAND driver can be a module, so this should have > EXPORT_SYMBOL. Indeed. Good catch! > >> diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h >> index de6ada739121..e1bb90a8db03 100644 >> --- a/include/linux/platform_data/mtd-nand-omap2.h >> +++ b/include/linux/platform_data/mtd-nand-omap2.h >> @@ -61,4 +61,9 @@ struct gpmc_nand_regs { >> void __iomem *gpmc_bch_result5[GPMC_BCH_NUM_REMAINDER]; >> void __iomem *gpmc_bch_result6[GPMC_BCH_NUM_REMAINDER]; >> }; >> + >> +#if defined(CONFIG_MTD_NAND_OMAP2) >> +extern const struct of_device_id omap_nand_ids[]; >> +#endif >> + >> #endif >> > > > Best regards, > Krzysztof > cheers, -roger
On 17/12/2021 17:21, Krzysztof Kozlowski wrote: > On 17/12/2021 11:29, Roger Quadros wrote: >> As more compatibles can be added to the GPMC NAND controller driver >> use a compatible match table. >> >> Signed-off-by: Roger Quadros <rogerq@kernel.org> >> --- >> drivers/memory/omap-gpmc.c | 8 +++++++- >> drivers/mtd/nand/raw/omap2.c | 2 +- >> include/linux/platform_data/mtd-nand-omap2.h | 5 +++++ >> 3 files changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c >> index 624153048182..814ddb45c13d 100644 >> --- a/drivers/memory/omap-gpmc.c >> +++ b/drivers/memory/omap-gpmc.c >> @@ -2091,6 +2091,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, >> u32 val; >> struct gpio_desc *waitpin_desc = NULL; >> struct gpmc_device *gpmc = platform_get_drvdata(pdev); >> + bool is_nand = false; >> >> if (of_property_read_u32(child, "reg", &cs) < 0) { >> dev_err(&pdev->dev, "%pOF has no 'reg' property\n", >> @@ -2183,7 +2184,12 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, >> } >> } >> >> - if (of_device_is_compatible(child, "ti,omap2-nand")) { >> +#if defined(CONFIG_MTD_NAND_OMAP2) > > if (IS_ENABLED()) is preferred. If needed, you could make omap_nand_ids > symbol visible always (so without ifdef around it), because extern > structure should not have impact when not defined (if I recall > correctly...). > >> + if (of_match_node(omap_nand_ids, child)) >> + is_nand = true; >> +#endif >> + >> + if (is_nand) { >> /* NAND specific setup */ >> val = 8; >> of_property_read_u32(child, "nand-bus-width", &val); >> diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c >> index b26d4947af02..fff834ee726f 100644 >> --- a/drivers/mtd/nand/raw/omap2.c >> +++ b/drivers/mtd/nand/raw/omap2.c >> @@ -2352,7 +2352,7 @@ static int omap_nand_remove(struct platform_device *pdev) >> return ret; >> } >> >> -static const struct of_device_id omap_nand_ids[] = { >> +const struct of_device_id omap_nand_ids[] = { >> { .compatible = "ti,omap2-nand", }, >> {}, >> }; > > I think OMAP2 NAND driver can be a module, so this should have > EXPORT_SYMBOL. To make it work in all combinations (e.g. omap_gpmc built in and nand/raw/omap2.c as module) I had to define omap_nand_ids table as static in the linux/platform_data/mtd-nand-omap2.h header. EXPORT_SYMBOL will of course be not required there. ;) > >> diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h >> index de6ada739121..e1bb90a8db03 100644 >> --- a/include/linux/platform_data/mtd-nand-omap2.h >> +++ b/include/linux/platform_data/mtd-nand-omap2.h >> @@ -61,4 +61,9 @@ struct gpmc_nand_regs { >> void __iomem *gpmc_bch_result5[GPMC_BCH_NUM_REMAINDER]; >> void __iomem *gpmc_bch_result6[GPMC_BCH_NUM_REMAINDER]; >> }; >> + >> +#if defined(CONFIG_MTD_NAND_OMAP2) >> +extern const struct of_device_id omap_nand_ids[]; >> +#endif >> + >> #endif >> > > > Best regards, > Krzysztof > cheers, -roger
On 20/12/2021 11:53, Roger Quadros wrote: > > > On 17/12/2021 17:21, Krzysztof Kozlowski wrote: >> On 17/12/2021 11:29, Roger Quadros wrote: >>> As more compatibles can be added to the GPMC NAND controller driver >>> use a compatible match table. >>> >>> Signed-off-by: Roger Quadros <rogerq@kernel.org> >>> --- >>> drivers/memory/omap-gpmc.c | 8 +++++++- >>> drivers/mtd/nand/raw/omap2.c | 2 +- >>> include/linux/platform_data/mtd-nand-omap2.h | 5 +++++ >>> 3 files changed, 13 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c >>> index 624153048182..814ddb45c13d 100644 >>> --- a/drivers/memory/omap-gpmc.c >>> +++ b/drivers/memory/omap-gpmc.c >>> @@ -2091,6 +2091,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, >>> u32 val; >>> struct gpio_desc *waitpin_desc = NULL; >>> struct gpmc_device *gpmc = platform_get_drvdata(pdev); >>> + bool is_nand = false; >>> >>> if (of_property_read_u32(child, "reg", &cs) < 0) { >>> dev_err(&pdev->dev, "%pOF has no 'reg' property\n", >>> @@ -2183,7 +2184,12 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, >>> } >>> } >>> >>> - if (of_device_is_compatible(child, "ti,omap2-nand")) { >>> +#if defined(CONFIG_MTD_NAND_OMAP2) >> >> if (IS_ENABLED()) is preferred. If needed, you could make omap_nand_ids >> symbol visible always (so without ifdef around it), because extern >> structure should not have impact when not defined (if I recall >> correctly...). >> >>> + if (of_match_node(omap_nand_ids, child)) >>> + is_nand = true; >>> +#endif >>> + >>> + if (is_nand) { >>> /* NAND specific setup */ >>> val = 8; >>> of_property_read_u32(child, "nand-bus-width", &val); >>> diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c >>> index b26d4947af02..fff834ee726f 100644 >>> --- a/drivers/mtd/nand/raw/omap2.c >>> +++ b/drivers/mtd/nand/raw/omap2.c >>> @@ -2352,7 +2352,7 @@ static int omap_nand_remove(struct platform_device *pdev) >>> return ret; >>> } >>> >>> -static const struct of_device_id omap_nand_ids[] = { >>> +const struct of_device_id omap_nand_ids[] = { >>> { .compatible = "ti,omap2-nand", }, >>> {}, >>> }; >> >> I think OMAP2 NAND driver can be a module, so this should have >> EXPORT_SYMBOL. > > To make it work in all combinations (e.g. omap_gpmc built in and > nand/raw/omap2.c as module) I had to define omap_nand_ids table as static > in the linux/platform_data/mtd-nand-omap2.h header. > > EXPORT_SYMBOL will of course be not required there. ;) > Which case exactly does it require to be static in the header? Best regards, Krzysztof
On 20/12/2021 13:05, Krzysztof Kozlowski wrote: > On 20/12/2021 11:53, Roger Quadros wrote: >> >> >> On 17/12/2021 17:21, Krzysztof Kozlowski wrote: >>> On 17/12/2021 11:29, Roger Quadros wrote: >>>> As more compatibles can be added to the GPMC NAND controller driver >>>> use a compatible match table. >>>> >>>> Signed-off-by: Roger Quadros <rogerq@kernel.org> >>>> --- >>>> drivers/memory/omap-gpmc.c | 8 +++++++- >>>> drivers/mtd/nand/raw/omap2.c | 2 +- >>>> include/linux/platform_data/mtd-nand-omap2.h | 5 +++++ >>>> 3 files changed, 13 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c >>>> index 624153048182..814ddb45c13d 100644 >>>> --- a/drivers/memory/omap-gpmc.c >>>> +++ b/drivers/memory/omap-gpmc.c >>>> @@ -2091,6 +2091,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, >>>> u32 val; >>>> struct gpio_desc *waitpin_desc = NULL; >>>> struct gpmc_device *gpmc = platform_get_drvdata(pdev); >>>> + bool is_nand = false; >>>> >>>> if (of_property_read_u32(child, "reg", &cs) < 0) { >>>> dev_err(&pdev->dev, "%pOF has no 'reg' property\n", >>>> @@ -2183,7 +2184,12 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, >>>> } >>>> } >>>> >>>> - if (of_device_is_compatible(child, "ti,omap2-nand")) { >>>> +#if defined(CONFIG_MTD_NAND_OMAP2) >>> >>> if (IS_ENABLED()) is preferred. If needed, you could make omap_nand_ids >>> symbol visible always (so without ifdef around it), because extern >>> structure should not have impact when not defined (if I recall >>> correctly...). >>> >>>> + if (of_match_node(omap_nand_ids, child)) >>>> + is_nand = true; >>>> +#endif >>>> + >>>> + if (is_nand) { >>>> /* NAND specific setup */ >>>> val = 8; >>>> of_property_read_u32(child, "nand-bus-width", &val); >>>> diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c >>>> index b26d4947af02..fff834ee726f 100644 >>>> --- a/drivers/mtd/nand/raw/omap2.c >>>> +++ b/drivers/mtd/nand/raw/omap2.c >>>> @@ -2352,7 +2352,7 @@ static int omap_nand_remove(struct platform_device *pdev) >>>> return ret; >>>> } >>>> >>>> -static const struct of_device_id omap_nand_ids[] = { >>>> +const struct of_device_id omap_nand_ids[] = { >>>> { .compatible = "ti,omap2-nand", }, >>>> {}, >>>> }; >>> >>> I think OMAP2 NAND driver can be a module, so this should have >>> EXPORT_SYMBOL. >> >> To make it work in all combinations (e.g. omap_gpmc built in and >> nand/raw/omap2.c as module) I had to define omap_nand_ids table as static >> in the linux/platform_data/mtd-nand-omap2.h header. >> >> EXPORT_SYMBOL will of course be not required there. ;) >> > Which case exactly does it require to be static in the header? Maybe there is a better way to do it. e.g. If omap_gpmc.c is built-in and nand/raw/omap2.c is not built or built as a module, what better way we can use? -- cheers, -roger
On 20/12/2021 12:51, Roger Quadros wrote: > > > On 20/12/2021 13:05, Krzysztof Kozlowski wrote: >> On 20/12/2021 11:53, Roger Quadros wrote: >>> >>> >>> On 17/12/2021 17:21, Krzysztof Kozlowski wrote: >>>> On 17/12/2021 11:29, Roger Quadros wrote: >>>>> As more compatibles can be added to the GPMC NAND controller driver >>>>> use a compatible match table. >>>>> >>>>> Signed-off-by: Roger Quadros <rogerq@kernel.org> >>>>> --- >>>>> drivers/memory/omap-gpmc.c | 8 +++++++- >>>>> drivers/mtd/nand/raw/omap2.c | 2 +- >>>>> include/linux/platform_data/mtd-nand-omap2.h | 5 +++++ >>>>> 3 files changed, 13 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c >>>>> index 624153048182..814ddb45c13d 100644 >>>>> --- a/drivers/memory/omap-gpmc.c >>>>> +++ b/drivers/memory/omap-gpmc.c >>>>> @@ -2091,6 +2091,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, >>>>> u32 val; >>>>> struct gpio_desc *waitpin_desc = NULL; >>>>> struct gpmc_device *gpmc = platform_get_drvdata(pdev); >>>>> + bool is_nand = false; >>>>> >>>>> if (of_property_read_u32(child, "reg", &cs) < 0) { >>>>> dev_err(&pdev->dev, "%pOF has no 'reg' property\n", >>>>> @@ -2183,7 +2184,12 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, >>>>> } >>>>> } >>>>> >>>>> - if (of_device_is_compatible(child, "ti,omap2-nand")) { >>>>> +#if defined(CONFIG_MTD_NAND_OMAP2) >>>> >>>> if (IS_ENABLED()) is preferred. If needed, you could make omap_nand_ids >>>> symbol visible always (so without ifdef around it), because extern >>>> structure should not have impact when not defined (if I recall >>>> correctly...). >>>> >>>>> + if (of_match_node(omap_nand_ids, child)) >>>>> + is_nand = true; >>>>> +#endif >>>>> + >>>>> + if (is_nand) { >>>>> /* NAND specific setup */ >>>>> val = 8; >>>>> of_property_read_u32(child, "nand-bus-width", &val); >>>>> diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c >>>>> index b26d4947af02..fff834ee726f 100644 >>>>> --- a/drivers/mtd/nand/raw/omap2.c >>>>> +++ b/drivers/mtd/nand/raw/omap2.c >>>>> @@ -2352,7 +2352,7 @@ static int omap_nand_remove(struct platform_device *pdev) >>>>> return ret; >>>>> } >>>>> >>>>> -static const struct of_device_id omap_nand_ids[] = { >>>>> +const struct of_device_id omap_nand_ids[] = { >>>>> { .compatible = "ti,omap2-nand", }, >>>>> {}, >>>>> }; >>>> >>>> I think OMAP2 NAND driver can be a module, so this should have >>>> EXPORT_SYMBOL. >>> >>> To make it work in all combinations (e.g. omap_gpmc built in and >>> nand/raw/omap2.c as module) I had to define omap_nand_ids table as static >>> in the linux/platform_data/mtd-nand-omap2.h header. >>> >>> EXPORT_SYMBOL will of course be not required there. ;) >>> >> Which case exactly does it require to be static in the header? > > Maybe there is a better way to do it. > e.g. If omap_gpmc.c is built-in and nand/raw/omap2.c is not built or built as > a module, what better way we can use? Ah, you are right, that is the tricky configuration. It could be a separate built-in source file which is being selected by OMAP_GPMC and MTD_NAND_OMAP2, but it would be also an overkill for one-item array. I guess your solution with header is the easiest. Best regards, Krzysztof
diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c index 624153048182..814ddb45c13d 100644 --- a/drivers/memory/omap-gpmc.c +++ b/drivers/memory/omap-gpmc.c @@ -2091,6 +2091,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, u32 val; struct gpio_desc *waitpin_desc = NULL; struct gpmc_device *gpmc = platform_get_drvdata(pdev); + bool is_nand = false; if (of_property_read_u32(child, "reg", &cs) < 0) { dev_err(&pdev->dev, "%pOF has no 'reg' property\n", @@ -2183,7 +2184,12 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, } } - if (of_device_is_compatible(child, "ti,omap2-nand")) { +#if defined(CONFIG_MTD_NAND_OMAP2) + if (of_match_node(omap_nand_ids, child)) + is_nand = true; +#endif + + if (is_nand) { /* NAND specific setup */ val = 8; of_property_read_u32(child, "nand-bus-width", &val); diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c index b26d4947af02..fff834ee726f 100644 --- a/drivers/mtd/nand/raw/omap2.c +++ b/drivers/mtd/nand/raw/omap2.c @@ -2352,7 +2352,7 @@ static int omap_nand_remove(struct platform_device *pdev) return ret; } -static const struct of_device_id omap_nand_ids[] = { +const struct of_device_id omap_nand_ids[] = { { .compatible = "ti,omap2-nand", }, {}, }; diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h index de6ada739121..e1bb90a8db03 100644 --- a/include/linux/platform_data/mtd-nand-omap2.h +++ b/include/linux/platform_data/mtd-nand-omap2.h @@ -61,4 +61,9 @@ struct gpmc_nand_regs { void __iomem *gpmc_bch_result5[GPMC_BCH_NUM_REMAINDER]; void __iomem *gpmc_bch_result6[GPMC_BCH_NUM_REMAINDER]; }; + +#if defined(CONFIG_MTD_NAND_OMAP2) +extern const struct of_device_id omap_nand_ids[]; +#endif + #endif
As more compatibles can be added to the GPMC NAND controller driver use a compatible match table. Signed-off-by: Roger Quadros <rogerq@kernel.org> --- drivers/memory/omap-gpmc.c | 8 +++++++- drivers/mtd/nand/raw/omap2.c | 2 +- include/linux/platform_data/mtd-nand-omap2.h | 5 +++++ 3 files changed, 13 insertions(+), 2 deletions(-)