[PATCH v5 06/14] ARM: OMAP2+: gpmc: CS configuration helper

Hi Afzal,

On 06/13/2012 12:50 AM, Mohammed, Afzal wrote:

Hi Jon,

On Tue, Jun 12, 2012 at 23:39:32, Hunter, Jon wrote:

quoted

On 06/12/2012 07:58 AM, Mohammed, Afzal wrote:

quoted

quoted

Thinking again over it, I am feeling above is sufficient, reason same as
said earlier, to keep code simple & currently this is sufficient to
handle GPMC bit patterns for IPs presently available. What you are
suggesting is to take care of the imaginary case when new GPMC IP happens
with new device type or size, I think that should be handled when such a
scenario happens. Probably, it is better here to add a comment to make
intention clear.

That is one possibility but I think that more important reasons are ...

1. Readability, at first it appears that we are always configuring the
CS for NAND. However, this is not the case. Maybe a comment here can
help as you mentioned.

As far as the users of GPMC is considered there is no confusion. Eg. For
device type, they have been provided with two macros,

GPMC_CONFIG1_DEVICETYPE_NOR, GPMC_CONFIG1_DEVICETYPE_NAND

So for NOR, user can feel satisfied by giving NOR flag, little does he know
that driver doesn't do anything with the flag, but he still gets what he want
NOR flag is defined as zero. Yes even if he doesn't specify any type, device
type will be set as NOR, but then that is the default - the only other option

Sure, but reviewing the function it still looks odd from a readability
standpoint. At least it made me think "what is going on here ...". So a
comment is definitely needed.

quoted

2. A bad setting in the configuration passed. Hopefully, people will
stick to the flags but we know that we expect the device type to be a 0
or 2 and so should we check?

Value of device type is something driver has to worry about, not its users,
they have been provided with two flags, one for NAND & other for NOR.

Yes, but the driver does not seem to worry about it. In other words,
there is no error checking. Ok so not a big deal a comment should suffice.

Jon