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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to