[RFC][PATCH 1/5] ARM: OMAP2+: gpmc: driver conversion

[Jon Hunter]

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