| Message ID | 1354204892-22762-5-git-send-email-zonque@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/29/2012 10:01 AM, Daniel Mack wrote: > The am33xx is capable of handling bch error correction modes, so > enable that feature in the driver. > > Signed-off-by: Daniel Mack <zonque@gmail.com> > --- > arch/arm/mach-omap2/gpmc-nand.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c > index f9f23a2..c8a72ba 100644 > --- a/arch/arm/mach-omap2/gpmc-nand.c > +++ b/arch/arm/mach-omap2/gpmc-nand.c > @@ -92,17 +92,18 @@ static int omap2_nand_gpmc_retime( > static bool gpmc_hwecc_bch_capable(enum omap_ecc ecc_opt) > { > /* support only OMAP3 class */ > - if (!cpu_is_omap34xx()) { > + if (!cpu_is_omap34xx() && !soc_is_am33xx()) { > pr_err("BCH ecc is not supported on this CPU\n"); > return 0; > } > > /* > - * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1. > - * Other chips may be added if confirmed to work. > + * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1 > + * and AM33xx derivates. Other chips may be added if confirmed to work. > */ > if ((ecc_opt == OMAP_ECC_BCH4_CODE_HW) && > - (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0))) { > + (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0)) && > + (!soc_is_am33xx())) { > pr_err("BCH 4-bit mode is not supported on this CPU\n"); > return 0; > } Sorry I should have seen this earlier. Ideally, this type of thing should be reflected by the device-tree/platform-data and we should get away from these cpu_is_xxxx macros for hardware features (where we can). Furthermore, we need to avoid including plat-omap/gpmc.h in drivers for the single zImage work (I see the omap nand driver is including gpmc.h). Tony, should this be addressed now or can we live this for the minute and fix-up later? Cheers Jon -- 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
On 11/29/2012 01:59 PM, Jon Hunter wrote: > > On 11/29/2012 10:01 AM, Daniel Mack wrote: >> The am33xx is capable of handling bch error correction modes, so >> enable that feature in the driver. >> >> Signed-off-by: Daniel Mack <zonque@gmail.com> >> --- >> arch/arm/mach-omap2/gpmc-nand.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c >> index f9f23a2..c8a72ba 100644 >> --- a/arch/arm/mach-omap2/gpmc-nand.c >> +++ b/arch/arm/mach-omap2/gpmc-nand.c >> @@ -92,17 +92,18 @@ static int omap2_nand_gpmc_retime( >> static bool gpmc_hwecc_bch_capable(enum omap_ecc ecc_opt) >> { >> /* support only OMAP3 class */ >> - if (!cpu_is_omap34xx()) { >> + if (!cpu_is_omap34xx() && !soc_is_am33xx()) { >> pr_err("BCH ecc is not supported on this CPU\n"); >> return 0; >> } >> >> /* >> - * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1. >> - * Other chips may be added if confirmed to work. >> + * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1 >> + * and AM33xx derivates. Other chips may be added if confirmed to work. >> */ >> if ((ecc_opt == OMAP_ECC_BCH4_CODE_HW) && >> - (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0))) { >> + (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0)) && >> + (!soc_is_am33xx())) { >> pr_err("BCH 4-bit mode is not supported on this CPU\n"); >> return 0; >> } > > Sorry I should have seen this earlier. Ideally, this type of thing > should be reflected by the device-tree/platform-data and we should get > away from these cpu_is_xxxx macros for hardware features (where we can). > Furthermore, we need to avoid including plat-omap/gpmc.h in drivers for > the single zImage work (I see the omap nand driver is including gpmc.h). > > Tony, should this be addressed now or can we live this for the minute > and fix-up later? Actually, I see that you do read the ecc mode from DT, so is this really needed? It would be good to eliminate this. Cheers Jon -- 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
On 29.11.2012 21:32, Jon Hunter wrote: > > On 11/29/2012 01:59 PM, Jon Hunter wrote: >> >> On 11/29/2012 10:01 AM, Daniel Mack wrote: >>> The am33xx is capable of handling bch error correction modes, so >>> enable that feature in the driver. >>> >>> Signed-off-by: Daniel Mack <zonque@gmail.com> >>> --- >>> arch/arm/mach-omap2/gpmc-nand.c | 9 +++++---- >>> 1 file changed, 5 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c >>> index f9f23a2..c8a72ba 100644 >>> --- a/arch/arm/mach-omap2/gpmc-nand.c >>> +++ b/arch/arm/mach-omap2/gpmc-nand.c >>> @@ -92,17 +92,18 @@ static int omap2_nand_gpmc_retime( >>> static bool gpmc_hwecc_bch_capable(enum omap_ecc ecc_opt) >>> { >>> /* support only OMAP3 class */ >>> - if (!cpu_is_omap34xx()) { >>> + if (!cpu_is_omap34xx() && !soc_is_am33xx()) { >>> pr_err("BCH ecc is not supported on this CPU\n"); >>> return 0; >>> } >>> >>> /* >>> - * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1. >>> - * Other chips may be added if confirmed to work. >>> + * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1 >>> + * and AM33xx derivates. Other chips may be added if confirmed to work. >>> */ >>> if ((ecc_opt == OMAP_ECC_BCH4_CODE_HW) && >>> - (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0))) { >>> + (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0)) && >>> + (!soc_is_am33xx())) { >>> pr_err("BCH 4-bit mode is not supported on this CPU\n"); >>> return 0; >>> } >> >> Sorry I should have seen this earlier. Ideally, this type of thing >> should be reflected by the device-tree/platform-data and we should get >> away from these cpu_is_xxxx macros for hardware features (where we can). >> Furthermore, we need to avoid including plat-omap/gpmc.h in drivers for >> the single zImage work (I see the omap nand driver is including gpmc.h). >> >> Tony, should this be addressed now or can we live this for the minute >> and fix-up later? > > Actually, I see that you do read the ecc mode from DT, so is this really > needed? It would be good to eliminate this. The ecc mode is read from DT, right. But the gpmc bindings can be used for many OMAP derivates in the future, and this check is there to make sure the configured settings are supported by the hardware after all, just like if the ecc mode would have been passed as static platform data. So what is it exactly that you want to get rid of? I agree though that we could solve this with via the of_device_id's data pointer of the matching driver. But as you said yourself, this can well be done later, and the problem here is that we still need the cpu_is_xxx() macros for older platforms, right? Thanks, Daniel -- 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
On 11/29/2012 02:42 PM, Daniel Mack wrote: > On 29.11.2012 21:32, Jon Hunter wrote: >> >> On 11/29/2012 01:59 PM, Jon Hunter wrote: >>> >>> On 11/29/2012 10:01 AM, Daniel Mack wrote: >>>> The am33xx is capable of handling bch error correction modes, so >>>> enable that feature in the driver. >>>> >>>> Signed-off-by: Daniel Mack <zonque@gmail.com> >>>> --- >>>> arch/arm/mach-omap2/gpmc-nand.c | 9 +++++---- >>>> 1 file changed, 5 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c >>>> index f9f23a2..c8a72ba 100644 >>>> --- a/arch/arm/mach-omap2/gpmc-nand.c >>>> +++ b/arch/arm/mach-omap2/gpmc-nand.c >>>> @@ -92,17 +92,18 @@ static int omap2_nand_gpmc_retime( >>>> static bool gpmc_hwecc_bch_capable(enum omap_ecc ecc_opt) >>>> { >>>> /* support only OMAP3 class */ >>>> - if (!cpu_is_omap34xx()) { >>>> + if (!cpu_is_omap34xx() && !soc_is_am33xx()) { >>>> pr_err("BCH ecc is not supported on this CPU\n"); >>>> return 0; >>>> } >>>> >>>> /* >>>> - * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1. >>>> - * Other chips may be added if confirmed to work. >>>> + * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1 >>>> + * and AM33xx derivates. Other chips may be added if confirmed to work. >>>> */ >>>> if ((ecc_opt == OMAP_ECC_BCH4_CODE_HW) && >>>> - (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0))) { >>>> + (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0)) && >>>> + (!soc_is_am33xx())) { >>>> pr_err("BCH 4-bit mode is not supported on this CPU\n"); >>>> return 0; >>>> } >>> >>> Sorry I should have seen this earlier. Ideally, this type of thing >>> should be reflected by the device-tree/platform-data and we should get >>> away from these cpu_is_xxxx macros for hardware features (where we can). >>> Furthermore, we need to avoid including plat-omap/gpmc.h in drivers for >>> the single zImage work (I see the omap nand driver is including gpmc.h). >>> >>> Tony, should this be addressed now or can we live this for the minute >>> and fix-up later? >> >> Actually, I see that you do read the ecc mode from DT, so is this really >> needed? It would be good to eliminate this. > > The ecc mode is read from DT, right. But the gpmc bindings can be used > for many OMAP derivates in the future, and this check is there to make > sure the configured settings are supported by the hardware after all, > just like if the ecc mode would have been passed as static platform > data. So what is it exactly that you want to get rid of? The above function. If there is a hardware bug that prevents us from using the hw-ecc mode that is supported (which is the case for omap3630 es1.0), then we should have a GPMC_ERRATA_xxx flag somewhere to indicate this and these errata flags should be populated at probe. > I agree though that we could solve this with via the of_device_id's data > pointer of the matching driver. But as you said yourself, this can well > be done later, and the problem here is that we still need the > cpu_is_xxx() macros for older platforms, right? Yes, but we cannot/shouldn't use these cpu_is_xxx macros from within driver code. Ok, here we can calling a platform function that is calling the macro, but for single zImage we cannot do that either. We cannot include platform headers in drivers for single zImage. We have to get away from that. Therefore, such information needs to be passed by platform data, device tree, etc. I will let Tony decide on how he wants us to handle this. Cheers Jon -- 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
On 29.11.2012 21:59, Jon Hunter wrote: > On 11/29/2012 02:42 PM, Daniel Mack wrote: >> On 29.11.2012 21:32, Jon Hunter wrote: >>> >>> On 11/29/2012 01:59 PM, Jon Hunter wrote: >>>> >>>> On 11/29/2012 10:01 AM, Daniel Mack wrote: >>>>> The am33xx is capable of handling bch error correction modes, so >>>>> enable that feature in the driver. >>>>> >>>>> Signed-off-by: Daniel Mack <zonque@gmail.com> >>>>> --- >>>>> arch/arm/mach-omap2/gpmc-nand.c | 9 +++++---- >>>>> 1 file changed, 5 insertions(+), 4 deletions(-) >>>>> >>>>> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c >>>>> index f9f23a2..c8a72ba 100644 >>>>> --- a/arch/arm/mach-omap2/gpmc-nand.c >>>>> +++ b/arch/arm/mach-omap2/gpmc-nand.c >>>>> @@ -92,17 +92,18 @@ static int omap2_nand_gpmc_retime( >>>>> static bool gpmc_hwecc_bch_capable(enum omap_ecc ecc_opt) >>>>> { >>>>> /* support only OMAP3 class */ >>>>> - if (!cpu_is_omap34xx()) { >>>>> + if (!cpu_is_omap34xx() && !soc_is_am33xx()) { >>>>> pr_err("BCH ecc is not supported on this CPU\n"); >>>>> return 0; >>>>> } >>>>> >>>>> /* >>>>> - * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1. >>>>> - * Other chips may be added if confirmed to work. >>>>> + * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1 >>>>> + * and AM33xx derivates. Other chips may be added if confirmed to work. >>>>> */ >>>>> if ((ecc_opt == OMAP_ECC_BCH4_CODE_HW) && >>>>> - (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0))) { >>>>> + (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0)) && >>>>> + (!soc_is_am33xx())) { >>>>> pr_err("BCH 4-bit mode is not supported on this CPU\n"); >>>>> return 0; >>>>> } >>>> >>>> Sorry I should have seen this earlier. Ideally, this type of thing >>>> should be reflected by the device-tree/platform-data and we should get >>>> away from these cpu_is_xxxx macros for hardware features (where we can). >>>> Furthermore, we need to avoid including plat-omap/gpmc.h in drivers for >>>> the single zImage work (I see the omap nand driver is including gpmc.h). >>>> >>>> Tony, should this be addressed now or can we live this for the minute >>>> and fix-up later? >>> >>> Actually, I see that you do read the ecc mode from DT, so is this really >>> needed? It would be good to eliminate this. >> >> The ecc mode is read from DT, right. But the gpmc bindings can be used >> for many OMAP derivates in the future, and this check is there to make >> sure the configured settings are supported by the hardware after all, >> just like if the ecc mode would have been passed as static platform >> data. So what is it exactly that you want to get rid of? > > The above function. > > If there is a hardware bug that prevents us from using the hw-ecc mode > that is supported (which is the case for omap3630 es1.0), then we should > have a GPMC_ERRATA_xxx flag somewhere to indicate this and these errata > flags should be populated at probe. > >> I agree though that we could solve this with via the of_device_id's data >> pointer of the matching driver. But as you said yourself, this can well >> be done later, and the problem here is that we still need the >> cpu_is_xxx() macros for older platforms, right? > > Yes, but we cannot/shouldn't use these cpu_is_xxx macros from within > driver code. Ok, here we can calling a platform function that is calling > the macro, but for single zImage we cannot do that either. We cannot > include platform headers in drivers for single zImage. We have to get > away from that. Therefore, such information needs to be passed by > platform data, device tree, etc. > > I will let Tony decide on how he wants us to handle this. Any idea yet about this detail? The other small Documentation changes you brought up are fixed in my tree and ready for resubmission. Daniel -- 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
* Daniel Mack <zonque@gmail.com> [121205 05:07]: > On 29.11.2012 21:59, Jon Hunter wrote: > > On 11/29/2012 02:42 PM, Daniel Mack wrote: > >> On 29.11.2012 21:32, Jon Hunter wrote: > >>> > >>> On 11/29/2012 01:59 PM, Jon Hunter wrote: > >>>> > >>>> On 11/29/2012 10:01 AM, Daniel Mack wrote: > >>>>> The am33xx is capable of handling bch error correction modes, so > >>>>> enable that feature in the driver. > >>>>> > >>>>> Signed-off-by: Daniel Mack <zonque@gmail.com> > >>>>> --- > >>>>> arch/arm/mach-omap2/gpmc-nand.c | 9 +++++---- > >>>>> 1 file changed, 5 insertions(+), 4 deletions(-) > >>>>> > >>>>> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c > >>>>> index f9f23a2..c8a72ba 100644 > >>>>> --- a/arch/arm/mach-omap2/gpmc-nand.c > >>>>> +++ b/arch/arm/mach-omap2/gpmc-nand.c > >>>>> @@ -92,17 +92,18 @@ static int omap2_nand_gpmc_retime( > >>>>> static bool gpmc_hwecc_bch_capable(enum omap_ecc ecc_opt) > >>>>> { > >>>>> /* support only OMAP3 class */ > >>>>> - if (!cpu_is_omap34xx()) { > >>>>> + if (!cpu_is_omap34xx() && !soc_is_am33xx()) { > >>>>> pr_err("BCH ecc is not supported on this CPU\n"); > >>>>> return 0; > >>>>> } > >>>>> > >>>>> /* > >>>>> - * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1. > >>>>> - * Other chips may be added if confirmed to work. > >>>>> + * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1 > >>>>> + * and AM33xx derivates. Other chips may be added if confirmed to work. > >>>>> */ > >>>>> if ((ecc_opt == OMAP_ECC_BCH4_CODE_HW) && > >>>>> - (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0))) { > >>>>> + (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0)) && > >>>>> + (!soc_is_am33xx())) { > >>>>> pr_err("BCH 4-bit mode is not supported on this CPU\n"); > >>>>> return 0; > >>>>> } > >>>> > >>>> Sorry I should have seen this earlier. Ideally, this type of thing > >>>> should be reflected by the device-tree/platform-data and we should get > >>>> away from these cpu_is_xxxx macros for hardware features (where we can). > >>>> Furthermore, we need to avoid including plat-omap/gpmc.h in drivers for > >>>> the single zImage work (I see the omap nand driver is including gpmc.h). > >>>> > >>>> Tony, should this be addressed now or can we live this for the minute > >>>> and fix-up later? > >>> > >>> Actually, I see that you do read the ecc mode from DT, so is this really > >>> needed? It would be good to eliminate this. > >> > >> The ecc mode is read from DT, right. But the gpmc bindings can be used > >> for many OMAP derivates in the future, and this check is there to make > >> sure the configured settings are supported by the hardware after all, > >> just like if the ecc mode would have been passed as static platform > >> data. So what is it exactly that you want to get rid of? > > > > The above function. > > > > If there is a hardware bug that prevents us from using the hw-ecc mode > > that is supported (which is the case for omap3630 es1.0), then we should > > have a GPMC_ERRATA_xxx flag somewhere to indicate this and these errata > > flags should be populated at probe. > > > >> I agree though that we could solve this with via the of_device_id's data > >> pointer of the matching driver. But as you said yourself, this can well > >> be done later, and the problem here is that we still need the > >> cpu_is_xxx() macros for older platforms, right? > > > > Yes, but we cannot/shouldn't use these cpu_is_xxx macros from within > > driver code. Ok, here we can calling a platform function that is calling > > the macro, but for single zImage we cannot do that either. We cannot > > include platform headers in drivers for single zImage. We have to get > > away from that. Therefore, such information needs to be passed by > > platform data, device tree, etc. > > > > I will let Tony decide on how he wants us to handle this. > > Any idea yet about this detail? The other small Documentation changes > you brought up are fixed in my tree and ready for resubmission. The plat/cpu.h file will disappear after the merge window, which means omap2+ related drivers cannot use cpu_is_omap macros. For legacy booting systems, this flag should be just passed in the platform_data from the platform init code. Then device tree can deal with it based on the compatible flag. Regards, Tony -- 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
Hi Tony, On 05.12.2012 18:19, Tony Lindgren wrote: > * Daniel Mack <zonque@gmail.com> [121205 05:07]: >> On 29.11.2012 21:59, Jon Hunter wrote: >>> On 11/29/2012 02:42 PM, Daniel Mack wrote: >>>> On 29.11.2012 21:32, Jon Hunter wrote: >>>>> >>>>> On 11/29/2012 01:59 PM, Jon Hunter wrote: >>>>>> >>>>>> On 11/29/2012 10:01 AM, Daniel Mack wrote: >>>>>>> The am33xx is capable of handling bch error correction modes, so >>>>>>> enable that feature in the driver. >>>>>>> >>>>>>> Signed-off-by: Daniel Mack <zonque@gmail.com> >>>>>>> --- >>>>>>> arch/arm/mach-omap2/gpmc-nand.c | 9 +++++---- >>>>>>> 1 file changed, 5 insertions(+), 4 deletions(-) >>>>>>> >>>>>>> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c >>>>>>> index f9f23a2..c8a72ba 100644 >>>>>>> --- a/arch/arm/mach-omap2/gpmc-nand.c >>>>>>> +++ b/arch/arm/mach-omap2/gpmc-nand.c >>>>>>> @@ -92,17 +92,18 @@ static int omap2_nand_gpmc_retime( >>>>>>> static bool gpmc_hwecc_bch_capable(enum omap_ecc ecc_opt) >>>>>>> { >>>>>>> /* support only OMAP3 class */ >>>>>>> - if (!cpu_is_omap34xx()) { >>>>>>> + if (!cpu_is_omap34xx() && !soc_is_am33xx()) { >>>>>>> pr_err("BCH ecc is not supported on this CPU\n"); >>>>>>> return 0; >>>>>>> } >>>>>>> >>>>>>> /* >>>>>>> - * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1. >>>>>>> - * Other chips may be added if confirmed to work. >>>>>>> + * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1 >>>>>>> + * and AM33xx derivates. Other chips may be added if confirmed to work. >>>>>>> */ >>>>>>> if ((ecc_opt == OMAP_ECC_BCH4_CODE_HW) && >>>>>>> - (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0))) { >>>>>>> + (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0)) && >>>>>>> + (!soc_is_am33xx())) { >>>>>>> pr_err("BCH 4-bit mode is not supported on this CPU\n"); >>>>>>> return 0; >>>>>>> } >>>>>> >>>>>> Sorry I should have seen this earlier. Ideally, this type of thing >>>>>> should be reflected by the device-tree/platform-data and we should get >>>>>> away from these cpu_is_xxxx macros for hardware features (where we can). >>>>>> Furthermore, we need to avoid including plat-omap/gpmc.h in drivers for >>>>>> the single zImage work (I see the omap nand driver is including gpmc.h). >>>>>> >>>>>> Tony, should this be addressed now or can we live this for the minute >>>>>> and fix-up later? >>>>> >>>>> Actually, I see that you do read the ecc mode from DT, so is this really >>>>> needed? It would be good to eliminate this. >>>> >>>> The ecc mode is read from DT, right. But the gpmc bindings can be used >>>> for many OMAP derivates in the future, and this check is there to make >>>> sure the configured settings are supported by the hardware after all, >>>> just like if the ecc mode would have been passed as static platform >>>> data. So what is it exactly that you want to get rid of? >>> >>> The above function. >>> >>> If there is a hardware bug that prevents us from using the hw-ecc mode >>> that is supported (which is the case for omap3630 es1.0), then we should >>> have a GPMC_ERRATA_xxx flag somewhere to indicate this and these errata >>> flags should be populated at probe. >>> >>>> I agree though that we could solve this with via the of_device_id's data >>>> pointer of the matching driver. But as you said yourself, this can well >>>> be done later, and the problem here is that we still need the >>>> cpu_is_xxx() macros for older platforms, right? >>> >>> Yes, but we cannot/shouldn't use these cpu_is_xxx macros from within >>> driver code. Ok, here we can calling a platform function that is calling >>> the macro, but for single zImage we cannot do that either. We cannot >>> include platform headers in drivers for single zImage. We have to get >>> away from that. Therefore, such information needs to be passed by >>> platform data, device tree, etc. >>> >>> I will let Tony decide on how he wants us to handle this. >> >> Any idea yet about this detail? The other small Documentation changes >> you brought up are fixed in my tree and ready for resubmission. > > The plat/cpu.h file will disappear after the merge window, which means > omap2+ related drivers cannot use cpu_is_omap macros. > > For legacy booting systems, this flag should be just passed in the > platform_data from the platform init code. Then device tree can > deal with it based on the compatible flag. > Ok, thanks for explaining. Does that mean this patch series should be postponed until after the merge window and then build upon that change or should we merge the patch in question here as is and then care for the cleanups after the window? I can also rebase this set on top of the removal patches if they exist already somewhere. Daniel -- 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
* Daniel Mack <zonque@gmail.com> [121205 09:29]: > On 05.12.2012 18:19, Tony Lindgren wrote: > > > > The plat/cpu.h file will disappear after the merge window, which means > > omap2+ related drivers cannot use cpu_is_omap macros. > > > > For legacy booting systems, this flag should be just passed in the > > platform_data from the platform init code. Then device tree can > > deal with it based on the compatible flag. > > > > Ok, thanks for explaining. > > Does that mean this patch series should be postponed until after the > merge window and then build upon that change or should we merge the > patch in question here as is and then care for the cleanups after the > window? Well to me it seems that you only have cpu_is_omap usage in arch/arm/mach-omap2/gpmc-nand.c, which will be OK. Only the code under drivers/* needs to be fixed for that. So your patches may be OK, but.. > I can also rebase this set on top of the removal patches if they exist > already somewhere. ..please check that your patches work with current linux next. It's too late to queue thing for v3.8 merge window at this point, as we want the patches sitting in linux next for a week at least before they get pulled. And we still need the acks for the device tree binding as well. So let's plan on queueing these after -rc1. Regards, Tony -- 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
On 12/05/2012 11:41 AM, Tony Lindgren wrote: > * Daniel Mack <zonque@gmail.com> [121205 09:29]: >> On 05.12.2012 18:19, Tony Lindgren wrote: >>> >>> The plat/cpu.h file will disappear after the merge window, which means >>> omap2+ related drivers cannot use cpu_is_omap macros. >>> >>> For legacy booting systems, this flag should be just passed in the >>> platform_data from the platform init code. Then device tree can >>> deal with it based on the compatible flag. >>> >> >> Ok, thanks for explaining. >> >> Does that mean this patch series should be postponed until after the >> merge window and then build upon that change or should we merge the >> patch in question here as is and then care for the cleanups after the >> window? > > Well to me it seems that you only have cpu_is_omap usage in > arch/arm/mach-omap2/gpmc-nand.c, which will be OK. Only the code > under drivers/* needs to be fixed for that. So your patches may > be OK, but.. The real problem here is that drivers/mtd/nand/omap2.c is including plat/gpmc.h and calling a function in arch/arm/mach-omap/gpmc-nand.c. So although Daniel's patches are not introducing any new problems for single zImage, they do highlight a problem that we have with the omap2 nand driver that needs to be addressed for single zImage. So either we fix this now or after merging Daniel's changes. Either is fine with me. Cheers Jon -- 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
* Jon Hunter <jon-hunter@ti.com> [121205 10:22]: > > On 12/05/2012 11:41 AM, Tony Lindgren wrote: > > * Daniel Mack <zonque@gmail.com> [121205 09:29]: > >> On 05.12.2012 18:19, Tony Lindgren wrote: > >>> > >>> The plat/cpu.h file will disappear after the merge window, which means > >>> omap2+ related drivers cannot use cpu_is_omap macros. > >>> > >>> For legacy booting systems, this flag should be just passed in the > >>> platform_data from the platform init code. Then device tree can > >>> deal with it based on the compatible flag. > >>> > >> > >> Ok, thanks for explaining. > >> > >> Does that mean this patch series should be postponed until after the > >> merge window and then build upon that change or should we merge the > >> patch in question here as is and then care for the cleanups after the > >> window? > > > > Well to me it seems that you only have cpu_is_omap usage in > > arch/arm/mach-omap2/gpmc-nand.c, which will be OK. Only the code > > under drivers/* needs to be fixed for that. So your patches may > > be OK, but.. > > The real problem here is that drivers/mtd/nand/omap2.c is including > plat/gpmc.h and calling a function in arch/arm/mach-omap/gpmc-nand.c. So > although Daniel's patches are not introducing any new problems for > single zImage, they do highlight a problem that we have with the omap2 > nand driver that needs to be addressed for single zImage. > > So either we fix this now or after merging Daniel's changes. Either is > fine with me. Ah I see. Yes it would be good to fix that issue first to avoid adding any new blockers for multiplatform build. Regards, Tony -- 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
On 05.12.2012 19:33, Tony Lindgren wrote: > * Jon Hunter <jon-hunter@ti.com> [121205 10:22]: >> >> On 12/05/2012 11:41 AM, Tony Lindgren wrote: >>> * Daniel Mack <zonque@gmail.com> [121205 09:29]: >>>> On 05.12.2012 18:19, Tony Lindgren wrote: >>>>> >>>>> The plat/cpu.h file will disappear after the merge window, which means >>>>> omap2+ related drivers cannot use cpu_is_omap macros. >>>>> >>>>> For legacy booting systems, this flag should be just passed in the >>>>> platform_data from the platform init code. Then device tree can >>>>> deal with it based on the compatible flag. >>>>> >>>> >>>> Ok, thanks for explaining. >>>> >>>> Does that mean this patch series should be postponed until after the >>>> merge window and then build upon that change or should we merge the >>>> patch in question here as is and then care for the cleanups after the >>>> window? >>> >>> Well to me it seems that you only have cpu_is_omap usage in >>> arch/arm/mach-omap2/gpmc-nand.c, which will be OK. Only the code >>> under drivers/* needs to be fixed for that. So your patches may >>> be OK, but.. >> >> The real problem here is that drivers/mtd/nand/omap2.c is including >> plat/gpmc.h and calling a function in arch/arm/mach-omap/gpmc-nand.c. So >> although Daniel's patches are not introducing any new problems for >> single zImage, they do highlight a problem that we have with the omap2 >> nand driver that needs to be addressed for single zImage. >> >> So either we fix this now or after merging Daniel's changes. Either is >> fine with me. > > Ah I see. Yes it would be good to fix that issue first to avoid > adding any new blockers for multiplatform build. Already fixed by Afzal here: https://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap.git;a=commitdiff;h=2ef9f3dd ... which is also in linux-next. Daniel -- 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
On 05.12.2012 18:41, Tony Lindgren wrote: > * Daniel Mack <zonque@gmail.com> [121205 09:29]: >> On 05.12.2012 18:19, Tony Lindgren wrote: >>> >>> The plat/cpu.h file will disappear after the merge window, which means >>> omap2+ related drivers cannot use cpu_is_omap macros. >>> >>> For legacy booting systems, this flag should be just passed in the >>> platform_data from the platform init code. Then device tree can >>> deal with it based on the compatible flag. >>> >> >> Ok, thanks for explaining. >> >> Does that mean this patch series should be postponed until after the >> merge window and then build upon that change or should we merge the >> patch in question here as is and then care for the cleanups after the >> window? > > Well to me it seems that you only have cpu_is_omap usage in > arch/arm/mach-omap2/gpmc-nand.c, which will be OK. Only the code > under drivers/* needs to be fixed for that. So your patches may > be OK, but.. > >> I can also rebase this set on top of the removal patches if they exist >> already somewhere. > > ..please check that your patches work with current linux next. > > It's too late to queue thing for v3.8 merge window at this point, > as we want the patches sitting in linux next for a week at least > before they get pulled. > > And we still need the acks for the device tree binding as well. Ok, I'll resubmit a v7 with the nits fixed that Jon pointed out, and then hope for the maintainer's Acks. > So let's plan on queueing these after -rc1. No problem really. The only thing is that I'll be on vacation when the merge window closes, hence I'd appreciate if you could already queue them up on your side, so they'll be taken care for :) Daniel -- 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
On 12/05/2012 12:40 PM, Daniel Mack wrote: > On 05.12.2012 19:33, Tony Lindgren wrote: >> * Jon Hunter <jon-hunter@ti.com> [121205 10:22]: >>> >>> On 12/05/2012 11:41 AM, Tony Lindgren wrote: >>>> * Daniel Mack <zonque@gmail.com> [121205 09:29]: >>>>> On 05.12.2012 18:19, Tony Lindgren wrote: >>>>>> >>>>>> The plat/cpu.h file will disappear after the merge window, which means >>>>>> omap2+ related drivers cannot use cpu_is_omap macros. >>>>>> >>>>>> For legacy booting systems, this flag should be just passed in the >>>>>> platform_data from the platform init code. Then device tree can >>>>>> deal with it based on the compatible flag. >>>>>> >>>>> >>>>> Ok, thanks for explaining. >>>>> >>>>> Does that mean this patch series should be postponed until after the >>>>> merge window and then build upon that change or should we merge the >>>>> patch in question here as is and then care for the cleanups after the >>>>> window? >>>> >>>> Well to me it seems that you only have cpu_is_omap usage in >>>> arch/arm/mach-omap2/gpmc-nand.c, which will be OK. Only the code >>>> under drivers/* needs to be fixed for that. So your patches may >>>> be OK, but.. >>> >>> The real problem here is that drivers/mtd/nand/omap2.c is including >>> plat/gpmc.h and calling a function in arch/arm/mach-omap/gpmc-nand.c. So >>> although Daniel's patches are not introducing any new problems for >>> single zImage, they do highlight a problem that we have with the omap2 >>> nand driver that needs to be addressed for single zImage. >>> >>> So either we fix this now or after merging Daniel's changes. Either is >>> fine with me. >> >> Ah I see. Yes it would be good to fix that issue first to avoid >> adding any new blockers for multiplatform build. > > Already fixed by Afzal here: > > https://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap.git;a=commitdiff;h=2ef9f3dd > > ... which is also in linux-next. Great! So we can drop this change. Cheers Jon -- 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
diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c index f9f23a2..c8a72ba 100644 --- a/arch/arm/mach-omap2/gpmc-nand.c +++ b/arch/arm/mach-omap2/gpmc-nand.c @@ -92,17 +92,18 @@ static int omap2_nand_gpmc_retime( static bool gpmc_hwecc_bch_capable(enum omap_ecc ecc_opt) { /* support only OMAP3 class */ - if (!cpu_is_omap34xx()) { + if (!cpu_is_omap34xx() && !soc_is_am33xx()) { pr_err("BCH ecc is not supported on this CPU\n"); return 0; } /* - * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1. - * Other chips may be added if confirmed to work. + * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1 + * and AM33xx derivates. Other chips may be added if confirmed to work. */ if ((ecc_opt == OMAP_ECC_BCH4_CODE_HW) && - (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0))) { + (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0)) && + (!soc_is_am33xx())) { pr_err("BCH 4-bit mode is not supported on this CPU\n"); return 0; }
The am33xx is capable of handling bch error correction modes, so enable that feature in the driver. Signed-off-by: Daniel Mack <zonque@gmail.com> --- arch/arm/mach-omap2/gpmc-nand.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)