From: jon-hunter@ti.com (Jon Hunter)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC][PATCH 1/5] ARM: OMAP2+: gpmc: driver conversion
Date: Tue, 27 Mar 2012 10:31:56 -0500 [thread overview]
Message-ID: <4F71DD6C.5010305@ti.com> (raw)
In-Reply-To: <C8443D0743D26F4388EA172BF4E2A7A9317BD6E2@DBDE01.ent.ti.com>
Hi Afzal,
On 3/27/2012 0:12, Mohammed, Afzal wrote:
> Hi Jon,
>
> On Mon, Mar 26, 2012 at 23:12:26, Hunter, Jon wrote:
>> Also, I don't see where the gpmc_child->res and gpmc_child->num_res are
>> actually used. Are these needed?
>
> These are for the peripheral resources not in control of GPMC, like
> gpio irq. These are copied in gpmc_create_child.
Right, I see they are copied during gpmc_create_child. However, I don't
see where they are initialised before that. The function
gpmc_setup_child is only initialising gpmc_res and gpmc_num_res and not
res and num_res. So I still don't see who is initialising these before
they are copied.
>>> Gpmc_device_data is dedicated to each CS, gpmc_pdata is required
>>> at least to inform driver about clock rate.
>>
>> Ok, understood! So the struct gpmc_device_pdata only has a single
>> chip-select entry and so looking at the code you will have multiple
>> instances of this structure of a gpmc device that uses more than one
>> chip-select. Any reason you did it this way and not have a single pdata
>> struct for each device defining all chip-selects it uses?
>
> When coding started, multiple CS for a peripheral was not taken into
> consideration, later handling multiple CS was incorporated making it
> this way. But your suggestion seems better to me, will see if code can
> modified accordingly in later patch series (v2 already sent)
Ok great. I will wait for your v3.
>>
>>> Generally, as the change involved moving a lot of code, seems more reviews
>>> are on those than the actual changes than what I intended to get reviewed,
>>> next patch series will be modified not to move existing code, hence some
>>> of your suggested changes may not be present in it, probably those to be
>>> done as another cleanup patch.
>>
>> Yes I understand. However, it is a good opportunity to clean some of
>> this up even if it is existing code :-)
>
> Ok
I see you did not incorporate any clean-up in v2. Do you want me to send
you some patches to include?
Cheers
Jon
next prev parent reply other threads:[~2012-03-27 15:31 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-23 6:36 [RFC][PATCH 1/5] ARM: OMAP2+: gpmc: driver conversion Afzal Mohammed
2012-03-23 9:37 ` Cousson, Benoit
2012-03-23 10:20 ` Mohammed, Afzal
2012-03-23 15:39 ` Cousson, Benoit
2012-03-23 16:29 ` Felipe Balbi
2012-03-26 6:14 ` Mohammed, Afzal
2012-03-26 6:03 ` Mohammed, Afzal
2012-03-23 23:21 ` Jon Hunter
2012-03-26 8:04 ` Mohammed, Afzal
2012-03-26 17:42 ` Jon Hunter
2012-03-27 5:12 ` Mohammed, Afzal
2012-03-27 15:31 ` Jon Hunter [this message]
2012-03-28 5:04 ` 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=4F71DD6C.5010305@ti.com \
--to=jon-hunter@ti.com \
--cc=linux-arm-kernel@lists.infradead.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox