From: Jon Hunter <jon-hunter@ti.com>
To: "Mohammed, Afzal" <afzal@ti.com>
Cc: "tony@atomide.com" <tony@atomide.com>,
"paul@pwsan.com" <paul@pwsan.com>,
"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v4 3/3] ARM: OMAP2+: onenand: prepare for gpmc driver migration
Date: Tue, 26 Jun 2012 09:39:07 -0500 [thread overview]
Message-ID: <4FE9C98B.2020003@ti.com> (raw)
In-Reply-To: <C8443D0743D26F4388EA172BF4E2A7A93E99F002@DBDE01.ent.ti.com>
Hi Afzal,
On 06/26/2012 03:29 AM, Mohammed, Afzal wrote:
> Hi Jon,
>
> On Mon, Jun 25, 2012 at 21:42:14, Hunter, Jon wrote:
>> On 06/22/2012 04:01 AM, Afzal Mohammed wrote:
>
>>> +static int hf, vhf, sync_read, sync_write, latency;
>>
>> I am wondering if we can remove hf, vhf, sync_read/write variables
>> completely. We already have flags from sync_read/write and so we could
>> just use the cfg->flags variable and remove sync_read/write variables.
>
> For default frequency, sync_write can get turned off, so flag may or
> may not be same as sync_write
Good point. I missed that.
>>
>> At the same time, we could create flags for ONENAND_FREQ_HF and
>> ONENAND_FREQ_VHF or something like that. It could be nice to store the
>> latency in onenand_data too. In other words, keep all the configuration
>> in one place.
>
> I have a feeling as though platform data fields should not be altered once
> platform device is registered (as platform data being specific to the
> board, thinking further, should they be const?, except for a case
> where it is created by a common helper function for multiple boards with
> varying capabilities of peripheral).
>
> Other than sync_read, all others like hf, vhf, latency, sync_write are
> updated during driver callback, so if we are going to put these in
> platform private data fields, platform private data fields has to be
> updated after platform device is registered.
May be this is splitting hairs then but I wonder if we should just have
a single global variable called onenand_flags for storing the current
state of sync_read, sync_write, vhf and hf. At least this would be only
one global instead of 4. Not a big deal.
Cheers
Jon
WARNING: multiple messages have this Message-ID (diff)
From: jon-hunter@ti.com (Jon Hunter)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4 3/3] ARM: OMAP2+: onenand: prepare for gpmc driver migration
Date: Tue, 26 Jun 2012 09:39:07 -0500 [thread overview]
Message-ID: <4FE9C98B.2020003@ti.com> (raw)
In-Reply-To: <C8443D0743D26F4388EA172BF4E2A7A93E99F002@DBDE01.ent.ti.com>
Hi Afzal,
On 06/26/2012 03:29 AM, Mohammed, Afzal wrote:
> Hi Jon,
>
> On Mon, Jun 25, 2012 at 21:42:14, Hunter, Jon wrote:
>> On 06/22/2012 04:01 AM, Afzal Mohammed wrote:
>
>>> +static int hf, vhf, sync_read, sync_write, latency;
>>
>> I am wondering if we can remove hf, vhf, sync_read/write variables
>> completely. We already have flags from sync_read/write and so we could
>> just use the cfg->flags variable and remove sync_read/write variables.
>
> For default frequency, sync_write can get turned off, so flag may or
> may not be same as sync_write
Good point. I missed that.
>>
>> At the same time, we could create flags for ONENAND_FREQ_HF and
>> ONENAND_FREQ_VHF or something like that. It could be nice to store the
>> latency in onenand_data too. In other words, keep all the configuration
>> in one place.
>
> I have a feeling as though platform data fields should not be altered once
> platform device is registered (as platform data being specific to the
> board, thinking further, should they be const?, except for a case
> where it is created by a common helper function for multiple boards with
> varying capabilities of peripheral).
>
> Other than sync_read, all others like hf, vhf, latency, sync_write are
> updated during driver callback, so if we are going to put these in
> platform private data fields, platform private data fields has to be
> updated after platform device is registered.
May be this is splitting hairs then but I wonder if we should just have
a single global variable called onenand_flags for storing the current
state of sync_read, sync_write, vhf and hf. At least this would be only
one global instead of 4. Not a big deal.
Cheers
Jon
next prev parent reply other threads:[~2012-06-26 14:39 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-22 9:00 [PATCH v4 0/3] Prepare for GPMC driver conversion Afzal Mohammed
2012-06-22 9:00 ` Afzal Mohammed
2012-06-22 9:00 ` [PATCH v4 1/3] ARM: OMAP2+: nand: unify init functions Afzal Mohammed
2012-06-22 9:00 ` Afzal Mohammed
2012-06-25 15:29 ` Jon Hunter
2012-06-25 15:29 ` Jon Hunter
2012-06-26 8:35 ` Mohammed, Afzal
2012-06-26 8:35 ` Mohammed, Afzal
2012-06-22 9:00 ` [PATCH v4 2/3] ARM: OMAP2+: gpmc: handle additional timings Afzal Mohammed
2012-06-22 9:00 ` Afzal Mohammed
2012-06-22 9:01 ` [PATCH v4 3/3] ARM: OMAP2+: onenand: prepare for gpmc driver migration Afzal Mohammed
2012-06-22 9:01 ` Afzal Mohammed
2012-06-25 16:12 ` Jon Hunter
2012-06-25 16:12 ` Jon Hunter
2012-06-26 8:29 ` Mohammed, Afzal
2012-06-26 8:29 ` Mohammed, Afzal
2012-06-26 14:39 ` Jon Hunter [this message]
2012-06-26 14:39 ` Jon Hunter
2012-06-26 14:56 ` Jon Hunter
2012-06-26 14:56 ` Jon Hunter
2012-06-27 6:53 ` Mohammed, Afzal
2012-06-27 6:53 ` Mohammed, Afzal
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=4FE9C98B.2020003@ti.com \
--to=jon-hunter@ti.com \
--cc=afzal@ti.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-omap@vger.kernel.org \
--cc=paul@pwsan.com \
--cc=tony@atomide.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.