LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Roger Quadros <rogerq@ti.com>
To: <semen.protsenko@globallogic.com>
Cc: Tony Lindgren <tony@atomide.com>,
	linux-omap <linux-omap@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/3] ARM: OMAP2+: gpmc: Do not modify LIMITEDADDRESS on new architectures
Date: Mon, 26 Jan 2015 17:46:13 +0200	[thread overview]
Message-ID: <54C66145.90400@ti.com> (raw)
In-Reply-To: <CAJOTznVwfMO7EmxcSEAx_v8cBe8apE_buAy2dLORxa-w+nQPLQ@mail.gmail.com>

On 26/01/15 15:07, Sam Protsenko wrote:
> On Mon, Jan 26, 2015 at 11:50 AM, Roger Quadros <rogerq@ti.com> wrote:
>> Hi,
>>
>> On 24/01/15 22:28, Semen Protsenko wrote:
>>> New OMAP-based architectures (like OMAP5, DRA7XX, AM572X) don't have
>>> LIMITEDADDRESS bit in GPMC_CONFIG register (this bit marked as
>>> RESERVED). Seems like these SoCs have new revision of GPMC IP-core
>>> (despite of same GPMC_REVISION value as for earlier SoCs).
>>>
>>> According to OMAP5 TRM: "Nonmultiplexed NOR flash devices are
>>> supported by the GPMC, but their use is highly limited. Because only
>>> 10 address pins are available on the GPMC interface, the maximum device
>>> size supported is 2KiB."
>>>
>>> From [1]:
>>>  - OMAP4 GPMC_CONFIG has LIMITEDADDRESS bit, hence supports both
>>>    limited and full address mode.
>>>  - OMAP5 GPMC_CONFIG doesn't have LIMITEDDADDRESS bit.
>>
>> According to OMAP5432 TRM SWPU249AC, I can see LIMITEDADDRESS bit in GPMC_CONFIG.
>> Which TRM version are you referring to?
>>
> 
> Wow, it seems I mixed it with OMAP57xx TRM. Good catch!
> 
>>>    OMAP5 supports only limited address mode for nonmultiplexed flashes
>>>    In this mode only A1-A10 lines are being modified by GPMC, which
>>>    leaves us (on flash devices with 1 word = 2 bytes) only
>>>    2^10 * 2 = 2KiB memory that we can access.
>>>  - DRA7XX GPMC_CONFIG doesn't have LIMITEDADDRESS bit.
>>>    DRA7XX supports only full address mode for nonmultiplexed flashes
>>>    (current TRM says contrary, but according to [1] it's typo and
>>>    gonna be fixed in new DRA7XX TRMs). In full address mode all
>>>    A1-A26 lines are modified by GPMC, so one can address up to 128 MiB.
>>>
>>> This patch ensures that GPMC driver doesn't try to modify LIMITEDADDRESS
>>> bit on new OMAP-based devices, because such an action could possibly
>>> lead to undefined behavior in GPMC state-machine (this bit is marked as
>>> RESERVED now).
>>>
>>> [1] http://e2e.ti.com/support/omap/f/885/t/396939
>>>
>>> Signed-off-by: Semen Protsenko <semen.protsenko@globallogic.com>
>>> ---
>>>  drivers/memory/omap-gpmc.c |   32 ++++++++++++++++++++++++--------
>>>  1 file changed, 24 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
>>> index db77adb..477d0ba 100644
>>> --- a/drivers/memory/omap-gpmc.c
>>> +++ b/drivers/memory/omap-gpmc.c
>>> @@ -108,6 +108,7 @@
>>>  #define      GPMC_HAS_WR_ACCESS              0x1
>>>  #define      GPMC_HAS_WR_DATA_MUX_BUS        0x2
>>>  #define      GPMC_HAS_MUX_AAD                0x4
>>> +#define      GPMC_HAS_LIMITEDADDRESS         0x8
>>>
>>>  #define GPMC_NR_WAITPINS             4
>>>
>>> @@ -1709,6 +1710,18 @@ static int gpmc_probe_onenand_child(struct platform_device *pdev,
>>>  }
>>>  #endif
>>>
>>> +static void gpmc_disable_limited(void)
>>> +{
>>> +     if (gpmc_capability & GPMC_HAS_LIMITEDADDRESS) {
>>> +             u32 val;
>>> +
>>> +             /* Clear limited address i.e. enable A26-A11 */
>>> +             val = gpmc_read_reg(GPMC_CONFIG);
>>> +             val &= ~GPMC_CONFIG_LIMITEDADDRESS;
>>> +             gpmc_write_reg(GPMC_CONFIG, val);
>>> +     }
>>> +}
>>> +
>>>  /**
>>>   * gpmc_probe_generic_child - configures the gpmc for a child device
>>>   * @pdev:    pointer to gpmc platform device
>>> @@ -1726,7 +1739,6 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>>>       unsigned long base;
>>>       const char *name;
>>>       int ret, cs;
>>> -     u32 val;
>>>
>>>       if (of_property_read_u32(child, "reg", &cs) < 0) {
>>>               dev_err(&pdev->dev, "%s has no 'reg' property\n",
>>> @@ -1805,10 +1817,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>>>               goto err;
>>>       }
>>>
>>> -     /* Clear limited address i.e. enable A26-A11 */
>>> -     val = gpmc_read_reg(GPMC_CONFIG);
>>> -     val &= ~GPMC_CONFIG_LIMITEDADDRESS;
>>> -     gpmc_write_reg(GPMC_CONFIG, val);
>>> +     gpmc_disable_limited();
>>>
>>>       /* Enable CS region */
>>>       gpmc_cs_enable_mem(cs);
>>> @@ -1935,14 +1944,21 @@ static int gpmc_probe(struct platform_device *pdev)
>>>        * devices support the addr-addr-data multiplex protocol.
>>>        *
>>>        * GPMC IP revisions:
>>> -      * - OMAP24xx                   = 2.0
>>> -      * - OMAP3xxx                   = 5.0
>>> -      * - OMAP44xx/54xx/AM335x       = 6.0
>>> +      * - OMAP24xx                           = 2.0
>>> +      * - OMAP3xxx                           = 5.0
>>> +      * - OMAP44xx/54xx/AM335x/DRA7XX        = 6.0
>>>        */
>>>       if (GPMC_REVISION_MAJOR(l) > 0x4)
>>>               gpmc_capability = GPMC_HAS_WR_ACCESS | GPMC_HAS_WR_DATA_MUX_BUS;
>>>       if (GPMC_REVISION_MAJOR(l) > 0x5)
>>>               gpmc_capability |= GPMC_HAS_MUX_AAD;
>>> +     if (GPMC_REVISION_MAJOR(l) < 0x6)
>>> +             gpmc_capability |= GPMC_HAS_LIMITEDADDRESS;
>>> +#if !defined(CONFIG_SOC_OMAP5) && !defined(CONFIG_SOC_DRA7XX)
>>> +     if (GPMC_REVISION_MAJOR(l) == 0x6)
>>> +             gpmc_capability |= GPMC_HAS_LIMITEDADDRESS;
>>> +#endif
>>> +
>>
>> This logic will prevent GPMC_HAS_LIMITEDADDRESS to be set on OMAP4 if we have
>> either SOC_OMAP5 or SOC_DRA7XX enabled. So it is not good.
>>
> 
> Why? OMAP4 devices have 0x6 revision of GPMC, and as I remember they don't
> define SOC_OMAP5. So this logic enables limited address capability for OMAP4.
> Am I missing something?

OMAP4 devices define ARCH_OMAP4.

Consider this case
ARCH_OMAP4=y, SOC_OMAP5=y, SOC_DRA7=y.

In this case LIMITEDADDRESS flag won't be set.

> 
>> I think this patch is unnecessary as we are only setting the reserved bit to 0.
>> It is safe to set RESERVED bit to 0. We are not modifying it as RESERVED bit is
>> meant to be 0. If bootloader is setting the reserved bit to 1, it is a bug in
>> the bootloader.
>>
> 
> Ok, let's drop this patch then. Anyway, I'm gonna reuse it next week
> for another patch
> (I want to make it possible to enable/disable limited address mode via
> dts bool option).
> Also, can you point me out where I can read about RESERVED bit should
> be always 0?

bit definition of RESERVED bit of GPMC_CONFIG register in DRA7 TRM states
"Write 0 for future compatibility. Read returns 0."

cheers,
-roger

  reply	other threads:[~2015-01-26 15:46 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-24 20:28 [PATCH 1/3] ARM: OMAP2+: gpmc: Fix writing in gpmc_cs_set_memconf Semen Protsenko
2015-01-24 20:28 ` [PATCH 2/3] ARM: OMAP2+: gpmc: make gpmc_cs_get_name() static Semen Protsenko
2015-01-26  9:20   ` Roger Quadros
2015-02-02 17:07     ` Tony Lindgren
2015-01-24 20:28 ` [PATCH 3/3] ARM: OMAP2+: gpmc: Do not modify LIMITEDADDRESS on new architectures Semen Protsenko
2015-01-26  9:50   ` Roger Quadros
2015-01-26 13:07     ` Sam Protsenko
2015-01-26 15:46       ` Roger Quadros [this message]
2015-01-26  9:34 ` [PATCH 1/3] ARM: OMAP2+: gpmc: Fix writing in gpmc_cs_set_memconf Roger Quadros
2015-02-02 17:08   ` Tony Lindgren
2015-02-03  9:30     ` Roger Quadros

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54C66145.90400@ti.com \
    --to=rogerq@ti.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=semen.protsenko@globallogic.com \
    --cc=tony@atomide.com \
    --subject='Re: [PATCH 3/3] ARM: OMAP2+: gpmc: Do not modify LIMITEDADDRESS on new architectures' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).