| Message ID | 1352220277-4251-1-git-send-email-matthieu.castet@parrot.com |
|---|---|
| State | New, archived |
| Headers | show |
On 11/06/2012 10:44 AM, Matthieu CASTET wrote: > Signed-off-by: Matthieu CASTET <matthieu.castet@parrot.com> I think you need to have something in the changelog for the patch describing why this change is needed and what device this has been tested on. > --- > arch/arm/mach-omap2/gpmc.c | 7 ++++++- > arch/arm/plat-omap/include/plat/gpmc.h | 2 ++ > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c > index 8ab1e1b..3957ffc 100644 > --- a/arch/arm/mach-omap2/gpmc.c > +++ b/arch/arm/mach-omap2/gpmc.c > @@ -333,8 +333,13 @@ int gpmc_cs_set_timings(int cs, const struct gpmc_timings *t) > > if (gpmc_capability & GPMC_HAS_WR_DATA_MUX_BUS) > GPMC_SET_ONE(GPMC_CS_CONFIG6, 16, 19, wr_data_mux_bus); > - if (gpmc_capability & GPMC_HAS_WR_ACCESS) > + if (gpmc_capability & GPMC_HAS_WR_ACCESS) { > + /* XXX check on which hardware it is supported */ I understand that you may not have all the documentation but lets fix this now. > + GPMC_SET_ONE(GPMC_CS_CONFIG6, 0, 3, busturnaround); > + GPMC_SET_ONE(GPMC_CS_CONFIG6, 8, 11, cycle2cycledelay); Looking at the documentation for OMAP2420, OMAP3430, OMAP4430 and OMAP5430, the above fields are present and same size location for all OMAP devices. So this does not need to be done under the HAS_WR_ACCESS field test. In fact, I believe that Afzal was going to add these fields in a patch and was doing it for all devices [1]. > GPMC_SET_ONE(GPMC_CS_CONFIG6, 24, 28, wr_access); > + } > > /* caller is expected to have initialized CONFIG1 to cover > * at least sync vs async > diff --git a/arch/arm/plat-omap/include/plat/gpmc.h b/arch/arm/plat-omap/include/plat/gpmc.h > index 2e6e259..34ca454 100644 > --- a/arch/arm/plat-omap/include/plat/gpmc.h > +++ b/arch/arm/plat-omap/include/plat/gpmc.h > @@ -131,6 +131,8 @@ struct gpmc_timings { > /* The following are only on OMAP3430 */ > u16 wr_access; /* WRACCESSTIME */ > u16 wr_data_mux_bus; /* WRDATAONADMUXBUS */ > + u16 cycle2cycledelay; /* CYCLE2CYCLEDELAY */ > + u16 busturnaround; /* BUSTURNAROUND */ So you should be able to move these out of OMAP3430 specific as they are generic. Cheers Jon [1] http://marc.info/?l=linux-omap&m=134037095705900&w=2
Jon Hunter a écrit : > On 11/06/2012 10:44 AM, Matthieu CASTET wrote: >> >> /* caller is expected to have initialized CONFIG1 to cover >> * at least sync vs async >> diff --git a/arch/arm/plat-omap/include/plat/gpmc.h b/arch/arm/plat-omap/include/plat/gpmc.h >> index 2e6e259..34ca454 100644 >> --- a/arch/arm/plat-omap/include/plat/gpmc.h >> +++ b/arch/arm/plat-omap/include/plat/gpmc.h >> @@ -131,6 +131,8 @@ struct gpmc_timings { >> /* The following are only on OMAP3430 */ >> u16 wr_access; /* WRACCESSTIME */ >> u16 wr_data_mux_bus; /* WRDATAONADMUXBUS */ >> + u16 cycle2cycledelay; /* CYCLE2CYCLEDELAY */ >> + u16 busturnaround; /* BUSTURNAROUND */ > > So you should be able to move these out of OMAP3430 specific as they are > generic. Thanks for the quick review. I will post another patch, unless this is already done in Afzal patch (Is there a tree where I can get Afzal pending patches ?) Matthieu
On 11/06/2012 12:00 PM, Matthieu CASTET wrote: > Jon Hunter a écrit : >> On 11/06/2012 10:44 AM, Matthieu CASTET wrote: >>> >>> /* caller is expected to have initialized CONFIG1 to cover >>> * at least sync vs async >>> diff --git a/arch/arm/plat-omap/include/plat/gpmc.h b/arch/arm/plat-omap/include/plat/gpmc.h >>> index 2e6e259..34ca454 100644 >>> --- a/arch/arm/plat-omap/include/plat/gpmc.h >>> +++ b/arch/arm/plat-omap/include/plat/gpmc.h >>> @@ -131,6 +131,8 @@ struct gpmc_timings { >>> /* The following are only on OMAP3430 */ >>> u16 wr_access; /* WRACCESSTIME */ >>> u16 wr_data_mux_bus; /* WRDATAONADMUXBUS */ >>> + u16 cycle2cycledelay; /* CYCLE2CYCLEDELAY */ >>> + u16 busturnaround; /* BUSTURNAROUND */ >> >> So you should be able to move these out of OMAP3430 specific as they are >> generic. > Thanks for the quick review. > > I will post another patch, unless this is already done in Afzal patch (Is there > a tree where I can get Afzal pending patches ?) Afzal keeps his kernel tree on gitorious [1]. However, I am not sure what Afzal's plans are for the remaining patches not yet merged and which branch you should look at (I have a lot of problems viewing anything on gitorious these days). Afzal, let us know how you prefer to handle this. Cheers Jon [1] http://gitorious.org/x0148406-public/linux-kernel
+ Tony, Daniel Hi, On Wed, Nov 07, 2012 at 02:04:03, Hunter, Jon wrote: > On 11/06/2012 12:00 PM, Matthieu CASTET wrote: > > I will post another patch, unless this is already done in Afzal patch (Is there > > a tree where I can get Afzal pending patches ?) > Afzal keeps his kernel tree on gitorious [1]. However, I am not sure > what Afzal's plans are for the remaining patches not yet merged and > which branch you should look at (I have a lot of problems viewing > anything on gitorious these days). > > Afzal, let us know how you prefer to handle this. My initial plan was to do timing related changes first and then dt conversion. But at the present moment, it seems higher priority has to be given for dt conversion over timing changes (it involves using generic timing routine for all required gpmc peripherals, handling additional timings inclusive of $subject) and hence timing related changes were put in suspended animation for now. And Daniel has started working on gpmc dt. Let us take Tony's opinion on how to deal with this, Tony ? Following are the pending changes w.r.t timing available @[1] (please note that these would have to be rebased over branch/tag specified by Tony in reply to Matthieu's patch 3/3) a. d42eafa ARM: OMAP2+: nand: remove redundant rounding b. f229aba ARM: OMAP2+: gpmc: handle additional timings c. 14cbb87 ARM: OMAP2+: gpmc: generic timing calculation d. 9830264 ARM: OMAP2+: onenand: generic timing calculation e. 5f33ea5 ARM: OMAP2+: smc91x: generic timing calculation f. 9876020 ARM: OMAP2+: tusb6010: generic timing calculation (HEAD) 'b' is a superset of $subject Originally 'a' and 'b' was part of gpmc cleanup series for common zImage, then Tony requested for minimal changes for it, hence 'a' & 'b' was left out in the pull request (picked up by Tony), so that gpmc common zImage cleanup series would not create any timing related issues. One thing to note is that cycle2cycledelay and busturnaround field's would get zeroed out with $subject patch for those peripheral's that call gpmc_cs_set_timings(). If there are any boards silently depending on bootloader setting these values, it may be affected, so this change would need to be verified for existing boards in mainline. Perhaps 'b' (for $subject patch) can be taken ahead if Tony is happy with it. And regarding patches 2 & 3 of Matthieu's series that calculate timings, I was wondering whether generic timing routine (c) can learn from it as well as vice-versa. Also with gpmc common zImage series, omap-nand driver does not have access to timing related gpmc functions, a new gpmc function would have to be exported that translates onfi timings to gpmc (or educate generic timing routine with onfi translation too ?) Regards Afzal [a] git://gitorious.org/x0148406-public/linux-kernel.git gpmc-timing
* Mohammed, Afzal <afzal@ti.com> [121107 01:00]: > + Tony, Daniel > > Hi, > > On Wed, Nov 07, 2012 at 02:04:03, Hunter, Jon wrote: > > On 11/06/2012 12:00 PM, Matthieu CASTET wrote: > > > > I will post another patch, unless this is already done in Afzal patch (Is there > > > a tree where I can get Afzal pending patches ?) > > > Afzal keeps his kernel tree on gitorious [1]. However, I am not sure > > what Afzal's plans are for the remaining patches not yet merged and > > which branch you should look at (I have a lot of problems viewing > > anything on gitorious these days). > > > > Afzal, let us know how you prefer to handle this. > > My initial plan was to do timing related changes first and then dt > conversion. But at the present moment, it seems higher priority has > to be given for dt conversion over timing changes (it involves > using generic timing routine for all required gpmc peripherals, > handling additional timings inclusive of $subject) and hence timing > related changes were put in suspended animation for now. > > And Daniel has started working on gpmc dt. Let us take Tony's > opinion on how to deal with this, Tony ? Up to you to figure out the ordering. > Following are the pending changes w.r.t timing available @[1] > (please note that these would have to be rebased over branch/tag > specified by Tony in reply to Matthieu's patch 3/3) > a. d42eafa ARM: OMAP2+: nand: remove redundant rounding > b. f229aba ARM: OMAP2+: gpmc: handle additional timings > c. 14cbb87 ARM: OMAP2+: gpmc: generic timing calculation > d. 9830264 ARM: OMAP2+: onenand: generic timing calculation > e. 5f33ea5 ARM: OMAP2+: smc91x: generic timing calculation > f. 9876020 ARM: OMAP2+: tusb6010: generic timing calculation (HEAD) > > 'b' is a superset of $subject > > Originally 'a' and 'b' was part of gpmc cleanup series for common > zImage, then Tony requested for minimal changes for it, hence > 'a' & 'b' was left out in the pull request (picked up by Tony), > so that gpmc common zImage cleanup series would not create any > timing related issues. Maybe send pull requests for the ones that are ready to go? They should be done against what we have in clean-up probably, so omap-for-v3.8/cleanup-headers-prepare-multiplatform-v3 or against omap-for-v3.8/cleanup-headers-gpmc it that merges easily with the cleanup branch. > One thing to note is that cycle2cycledelay and busturnaround field's > would get zeroed out with $subject patch for those peripheral's that > call gpmc_cs_set_timings(). If there are any boards silently > depending on bootloader setting these values, it may be affected, so > this change would need to be verified for existing boards in mainline. > > Perhaps 'b' (for $subject patch) can be taken ahead if Tony is > happy with it. > > And regarding patches 2 & 3 of Matthieu's series that calculate > timings, I was wondering whether generic timing routine (c) can > learn from it as well as vice-versa. Also with gpmc common zImage > series, omap-nand driver does not have access to timing related > gpmc functions, a new gpmc function would have to be exported > that translates onfi timings to gpmc (or educate generic timing > routine with onfi translation too ?) Right, once we enable CONFIG_ARCH_MULTIPLATFORM, drivers trying to include <mach/*.h> or <plat/*.h> files will fail to build. Regards, Tony > [a] git://gitorious.org/x0148406-public/linux-kernel.git gpmc-timing
Hi Tony, On Thu, Nov 08, 2012 at 03:10:14, Tony Lindgren wrote: > * Mohammed, Afzal <afzal@ti.com> [121107 01:00]: > > And Daniel has started working on gpmc dt. Let us take Tony's > > opinion on how to deal with this, Tony ? > Up to you to figure out the ordering. > Maybe send pull requests for the ones that are ready to go? Pending timing related patches, a. ARM: OMAP2+: nand: remove redundant rounding b. ARM: OMAP2+: gpmc: handle additional timings c. ARM: OMAP2+: gpmc: generic timing calculation d. ARM: OMAP2+: onenand: generic timing calculation e. ARM: OMAP2+: smc91x: generic timing calculation f. ARM: OMAP2+: tusb6010: generic timing calculation can be divided to two categories, 1. timing cleanup (a,b) 2. migrate to generic timing routine (c-f) '1' has been tested on omap3evm (also with the help of a local patch adding onenand support). '2' has been verified on omap3evm onenand async operation (support for omap3evm onenand is not present in mainline) and by simulation. These changes could not be validated on boards supported in mainline that make use of runtime timing calculation as I do not have those boards. I will be sending pull request containing both sets 1 & 2, please let me know in case you have a different opinion. Regards Afzal
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c index 8ab1e1b..3957ffc 100644 --- a/arch/arm/mach-omap2/gpmc.c +++ b/arch/arm/mach-omap2/gpmc.c @@ -333,8 +333,13 @@ int gpmc_cs_set_timings(int cs, const struct gpmc_timings *t) if (gpmc_capability & GPMC_HAS_WR_DATA_MUX_BUS) GPMC_SET_ONE(GPMC_CS_CONFIG6, 16, 19, wr_data_mux_bus); - if (gpmc_capability & GPMC_HAS_WR_ACCESS) + if (gpmc_capability & GPMC_HAS_WR_ACCESS) { + /* XXX check on which hardware it is supported */ + GPMC_SET_ONE(GPMC_CS_CONFIG6, 0, 3, busturnaround); + GPMC_SET_ONE(GPMC_CS_CONFIG6, 8, 11, cycle2cycledelay); + GPMC_SET_ONE(GPMC_CS_CONFIG6, 24, 28, wr_access); + } /* caller is expected to have initialized CONFIG1 to cover * at least sync vs async diff --git a/arch/arm/plat-omap/include/plat/gpmc.h b/arch/arm/plat-omap/include/plat/gpmc.h index 2e6e259..34ca454 100644 --- a/arch/arm/plat-omap/include/plat/gpmc.h +++ b/arch/arm/plat-omap/include/plat/gpmc.h @@ -131,6 +131,8 @@ struct gpmc_timings { /* The following are only on OMAP3430 */ u16 wr_access; /* WRACCESSTIME */ u16 wr_data_mux_bus; /* WRDATAONADMUXBUS */ + u16 cycle2cycledelay; /* CYCLE2CYCLEDELAY */ + u16 busturnaround; /* BUSTURNAROUND */ }; struct gpmc_nand_regs {
Signed-off-by: Matthieu CASTET <matthieu.castet@parrot.com> --- arch/arm/mach-omap2/gpmc.c | 7 ++++++- arch/arm/plat-omap/include/plat/gpmc.h | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-)