public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
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

  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