From: jon-hunter@ti•com (Jon Hunter)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v4 01/39] ARM: OMAP2+: gpmc: driver conversion
Date: Mon, 7 May 2012 11:23:34 -0500 [thread overview]
Message-ID: <4FA7F706.1020706@ti.com> (raw)
In-Reply-To: <C8443D0743D26F4388EA172BF4E2A7A93E98712C@DBDE01.ent.ti.com>
Hi Afzal,
On 05/07/2012 05:57 AM, Mohammed, Afzal wrote:
> Hi Jon,
>
> On Fri, May 04, 2012 at 21:50:16, Hunter, Jon wrote:
>>> As mentioned in the cover letter,
>>>
>>> "Additional features that currently boards in mainline does not make
>>> use of like, waitpin interrupt handling, changes to leverage revision
>>> 6 IP differences has not been incorporated."
>>>
>>> Priority in this series is to convert into a driver, get all boards working
>>> on mainline. Once all boards are working with gpmc driver, these features
>>> which are not required for currently supported boards can be added later.
>>
>> Yes, but I meant why 2 and not say 5? Anyway, I think that this should
>> be marked with a comment like a TODO so it is clear that this needs to
>> be re-visited.
>
> Ok, it will be marked with TODO
>
>>>> I think that it make more sense to have the wait-pin information part of
>>>> the gpmc_cs_data structure for the following reasons ...
>>>
>>> Waitpin information is indeed a part of cs as far as boards are concerned,
>>> it is only that it gets propogated to device
>>
>> Why does the device's driver care? From my point of view, the wait-pin
>> is just part of the interface setup/configuration. The external device
>> may require this and assumes that this has been setup along with the CS
>> and interface timing, but the device's driver should not care. Remember
>> this is just a ready signal used to stall the access. Once configured,
>> software should be unaware of it.
>
> By device, it is referred to gpmc device which only gpmc driver is aware,
> the peripheral device's driver is not at all aware.
>
>>>> Also, any reason why waitpin_polarity is an int? I see you define LOW as
>>>> -1, but I not sure why LOW cannot be 0 as 0 is programmed into the
>>>> register for an active low wait signal.
>>>
>>> Only intention is not to alter default waitpin polarity value, i.e. if
>>> any board does make use of it w/o knowledge of Kernel, I don't want to
>>> break it, there may be an argument saying that the board code is buggy,
>>> while if some board does so, it is, but don't want to break working one.
>>>
>>> Here unless user explicitly set the flag, it does nothing on polarity
>>
>> Ok. Do such scenario's exist today? Please note that board code will be
>> removed in the future and device-tree will replace. So if there are no
>> cases today, I would not be concerned. Unless this could be something
>> that has already been configured by the bootloader. However, in that
>> case would be even call this function?
>
> Let me check this, here only intent was to play safe, as I have
> only two boards to test.
>
>>>>> +int gpmc_cs_reconfigure(char *name, int id, struct gpmc_cs_data *cs)
>>>>
>>>> What scenario is this function used in? May be worth adding a comment
>>>> about the function.
>>>
>>> Ok, it was required for OneNAND, as it needs to reconfigure
>>
>> Ok, but why is that? Unless this is something generic about one-nand I
>> don't comprehend. I have a high-level understanding of one-nand, but I
>> am not familiar with the specifics that require it to be reconfigured.
>
> Not too familiar with OneNAND, from the code it has been understood
> that to set into sync mode, first it may have to set to async mode, read
> frequency from OneNAND, then setup sync mode for that frequency.
>
>
>>>>> + gpmc_setup_writeprotect(gpmc);
>>>>
>>>> Write protect is a pin and there is only one. Like the waitpins and CS
>>>> signals this needs to be associated with a device. It would make sense
>>>> that this is associated with the cs data.
>>>
>>> As far as platform are concerned, it is associated with cs, it is only
>>> that while configuring CS, it is propagated such that it is done once.
>>
>> Hmmm ... but here it looks like if write-protect is used you are going
>> to turn it on. A feature like this seems that it does need to be handled
>> by the device's driver. Enabling and disabling write-protect are
>> functions that I would expect are exported to the device's driver and
>> not directly enabled in the gpmc driver during probe. However, maybe is
>> could be valid for the write protect to be enabled by default which
>> could make sense. However, I don't see anywhere else in the driver to
>> control it.
>>
>> Shouldn't we warn on multiple devices trying to use write-protect when
>> parsing their configuration?
>
> Even if a single device sets write protect, kernel will log it.
That does not seem sufficient. It should be flagged as an error.
Write protect is a resource like the CS and waitpins and so I would have
thought that it should be reserved in the same way.
Jon
next prev parent reply other threads:[~2012-05-07 16:23 UTC|newest]
Thread overview: 67+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-01 12:19 [PATCH v4 00/39] OMAP GPMC driver conversion Afzal Mohammed
2012-05-01 12:19 ` [PATCH v4 01/39] ARM: OMAP2+: gpmc: " Afzal Mohammed
2012-05-01 17:53 ` Jon Hunter
2012-05-03 8:23 ` Mohammed, Afzal
2012-05-04 16:20 ` Jon Hunter
2012-05-07 10:57 ` Mohammed, Afzal
2012-05-07 16:23 ` Jon Hunter [this message]
2012-05-08 6:27 ` Mohammed, Afzal
2012-05-04 16:27 ` Jon Hunter
2012-05-07 11:01 ` Mohammed, Afzal
2012-05-07 16:02 ` Jon Hunter
2012-05-08 6:18 ` Mohammed, Afzal
2012-05-08 15:08 ` Jon Hunter
2012-05-10 5:10 ` Mohammed, Afzal
2012-05-01 12:19 ` [PATCH v4 02/39] ARM: OMAP2+: gpmc: Adapt to HWMOD Afzal Mohammed
2012-05-01 20:41 ` Jon Hunter
2012-05-03 8:37 ` Mohammed, Afzal
2012-05-04 16:30 ` Jon Hunter
2012-05-07 11:02 ` Mohammed, Afzal
2012-05-07 16:12 ` Jon Hunter
2012-05-08 6:24 ` Mohammed, Afzal
2012-05-08 15:13 ` Jon Hunter
2012-05-10 5:17 ` Mohammed, Afzal
2012-05-01 12:19 ` [PATCH v4 03/39] ARM: OMAP2+: gpmc: register details for nand driver Afzal Mohammed
2012-05-01 12:20 ` [PATCH v4 04/39] ARM: OMAP2+: gpmc: Acquire NAND CS value Afzal Mohammed
2012-05-01 21:16 ` Jon Hunter
2012-05-03 8:42 ` Mohammed, Afzal
2012-05-01 12:20 ` [PATCH v4 05/39] ARM: OMAP2+: nand: create platform data structure Afzal Mohammed
2012-05-01 12:20 ` [PATCH v4 06/39] ARM: OMAP2+: onenand: return value in init function Afzal Mohammed
2012-05-01 16:49 ` Sergei Shtylyov
2012-05-03 8:54 ` Mohammed, Afzal
2012-05-01 12:20 ` [PATCH v4 07/39] ARM: OMAP2+: gpmc-nand: Adapt to use gpmc driver Afzal Mohammed
2012-05-01 12:20 ` [PATCH v4 08/39] ARM: OMAP2+: gpmc-onenand: " Afzal Mohammed
2012-05-01 12:20 ` [PATCH v4 09/39] ARM: OMAP2+: flash: Adapt to " Afzal Mohammed
2012-05-01 12:20 ` [PATCH v4 10/39] ARM: OMAP2+: gpmc-smsc911x: Adapt to use " Afzal Mohammed
2012-05-01 12:20 ` [PATCH v4 11/39] ARM: OMAP2+: gpmc-smc91x: " Afzal Mohammed
2012-05-01 12:21 ` [PATCH v4 12/39] ARM: OMAP2+: gpmc-tusb6010: Adapt to " Afzal Mohammed
2012-05-01 12:21 ` [PATCH v4 13/39] ARM: OMAP3: hwmod data: add gpmc Afzal Mohammed
2012-05-06 2:09 ` Paul Walmsley
2012-05-01 12:21 ` [PATCH v4 14/39] ARM: OMAP2xxx: " Afzal Mohammed
2012-05-06 2:09 ` Paul Walmsley
2012-05-01 12:21 ` [PATCH v4 15/39] mtd: nand: omap2: obtain memory from resource Afzal Mohammed
2012-05-01 12:21 ` [PATCH v4 16/39] mtd: nand: omap2: use gpmc provided irqs Afzal Mohammed
2012-05-01 12:21 ` [PATCH v4 17/39] mtd: nand: omap2: handle nand on gpmc Afzal Mohammed
2012-05-01 12:21 ` [PATCH v4 18/39] mtd: onenand: omap: obtain memory from resource Afzal Mohammed
2012-05-01 12:21 ` [PATCH v4 19/39] ARM: OMAP2+: board omap3evm: gpmc driver adaptation Afzal Mohammed
2012-05-01 12:21 ` [PATCH v4 20/39] ARM: OMAP2+: board omap3beagle: " Afzal Mohammed
2012-05-01 12:22 ` [PATCH v4 21/39] ARM: OMAP2+: board apollon: " Afzal Mohammed
2012-05-01 12:22 ` [PATCH v4 22/39] ARM: OMAP2+: board h4: " Afzal Mohammed
2012-05-01 12:22 ` [PATCH v4 23/39] ARM: OMAP2+: board 3630sdp: " Afzal Mohammed
2012-05-01 12:22 ` [PATCH v4 24/39] ARM: OMAP2+: board 3430sdp: " Afzal Mohammed
2012-05-01 12:22 ` [PATCH v4 25/39] ARM: OMAP2+: board 2430sdp: " Afzal Mohammed
2012-05-01 12:22 ` [PATCH v4 26/39] ARM: OMAP2+: board cm-t3517: " Afzal Mohammed
2012-05-01 12:22 ` [PATCH v4 27/39] ARM: OMAP2+: board cm-t35: " Afzal Mohammed
2012-05-01 12:22 ` [PATCH v4 28/39] ARM: OMAP2+: board ldp: " Afzal Mohammed
2012-05-01 12:22 ` [PATCH v4 29/39] ARM: OMAP2+: board n8x0: " Afzal Mohammed
2012-05-01 12:23 ` [PATCH v4 30/39] ARM: OMAP2+: board omap3logic: " Afzal Mohammed
2012-05-01 12:23 ` [PATCH v4 31/39] ARM: OMAP2+: board omap3pandora: " Afzal Mohammed
2012-05-01 12:23 ` [PATCH v4 32/39] ARM: OMAP2+: board omap3stalker: " Afzal Mohammed
2012-05-01 12:23 ` [PATCH v4 33/39] ARM: OMAP2+: board omap4pcm049: " Afzal Mohammed
2012-05-01 12:23 ` [PATCH v4 34/39] ARM: OMAP2+: board overo: " Afzal Mohammed
2012-05-01 12:23 ` [PATCH v4 35/39] ARM: OMAP2+: board rm680: " Afzal Mohammed
2012-05-01 12:23 ` [PATCH v4 36/39] ARM: OMAP2+: board rx51: " Afzal Mohammed
2012-05-01 12:23 ` [PATCH v4 37/39] ARM: OMAP2+: board zoom-debugboard: " Afzal Mohammed
2012-05-01 12:23 ` [PATCH v4 38/39] OMAP3: igep0020: Add support for Micron NAND Flash storage memory Afzal Mohammed
2012-05-01 12:23 ` [PATCH v4 39/39] ARM: OMAP2+: board igep0020: gpmc driver adaptation Afzal Mohammed
2012-05-08 11:27 ` [PATCH v4 00/39] OMAP GPMC driver conversion 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=4FA7F706.1020706@ti.com \
--to=jon-hunter@ti$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox