On Thu, Feb 28, 2013 at 22:42:37, Hunter, Jon wrote:
>
> On 02/28/2013 09:52 AM, Jon Hunter wrote:
> >
> > On 02/28/2013 12:05 AM, Philip, Avinash wrote:
> >> On Tue, Feb 26, 2013 at 23:00:31, Hunter, Jon wrote:
> >>> The GPMC has various different configuration options such as bus-width,
> >>> synchronous or asychronous mode selection, burst mode options etc.
> >>> Currently, there is no common function for configuring these options and
> >>> various devices set these options by either programming the GPMC CONFIG1
> >>> register directly or by calling gpmc_cs_configure() to set some of the
> >>> options.
> >>>
> >>> Add a new function for configuring all of the GPMC options. Having a common
> >>> function for configuring this options will simplify code and ease the
> >>> migration to device-tree.
> >>>
> >>> Signed-off-by: Jon Hunter <jon-hunter@xxxxxx>
> >>> ---
> >>> arch/arm/mach-omap2/gpmc.c | 65 ++++++++++++++++++++++++++++++++++++++++++++
> >>> arch/arm/mach-omap2/gpmc.h | 6 ++++
> >>> 2 files changed, 71 insertions(+)
> >>>
> >>> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> >>> index 465cac9..fb8dfd2 100644
> >>> --- a/arch/arm/mach-omap2/gpmc.c
> >>> +++ b/arch/arm/mach-omap2/gpmc.c
> >>> @@ -1137,6 +1137,71 @@ int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
> >>> return 0;
> >>> }
> >>>
> >>> +int gpmc_cs_program_settings(int cs, struct gpmc_settings *p)
> >>> +{
> >>> + u32 config1;
> >>> +
> >>> + if ((!p->device_width) || (p->device_width > GPMC_DEVWIDTH_16BIT)) {
> >>> + pr_err("%s: invalid width %d!", __func__, p->device_width);
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + /* Address-data multiplexing not supported for NAND devices */
> >>> + if (p->device_nand && p->mux_add_data) {
> >>> + pr_err("%s: invalid configuration!\n", __func__);
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + /* Page/burst mode supports lengths of 4, 8 and 16 bytes */
> >>> + if (p->burst_read || p->burst_write) {
> >>> + switch (p->burst_len) {
> >>> + case GPMC_BURST_4:
> >>> + case GPMC_BURST_8:
> >>> + case GPMC_BURST_16:
> >>> + break;
> >>> + default:
> >>> + pr_err("%s: invalid page/burst-length (%d)\n",
> >>> + __func__, p->burst_len);
> >>> + return -EINVAL;
> >>> + }
> >>> + }
> >>> +
> >>> + if ((p->wait_on_read || p->wait_on_write) &&
> >>> + (p->wait_pin > gpmc_nr_waitpins)) {
> >>> + pr_err("%s: invalid wait-pin (%d)\n", __func__, p->wait_pin);
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + config1 = GPMC_CONFIG1_DEVICESIZE((p->device_width - 1));
> >>
> >> Can you consider read_modify approach?
> >
> > I was purposely trying to avoid that. The intent here is to program all
> > the settings and get away from any boot-loader dependencies. If we use a
> > read-modify approach then it will never be clear in the kernel what
> > should actually be programmed.
>
> By the way, it would be good to know what your motivation for this is.
>
> My goal is for all gpmc settings to eventually live in the DT blob and
> there be no dependency on the bootloader whatsoever. That may mean that
> settings are re-programmed again by the kernel but IMO that is best.
Yes I understood now and the right thing to do.
Suggested read modify approach because of some confusion as GPMC_CONFIG1 is already
modified in gpmc_cs_set_timings() and is called from omap2_nand_gpmc_retime().
So the data modified in gpmc_cs_set_timings() will lost (not significant)
May be better and cleaner solution is to remove GPMC_CONFIG1 modification in
gpmc_cs_set_timings() also.
Thanks
Avinash
>
> Cheers
> Jon
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
