Re: [PATCH 1/5] ARM: OMAP2+: gpmc: Fix kernel BUG for DT boot mode

On 10/18/2012 01:39 PM, Hiremath, Vaibhav wrote:

On Fri, Oct 19, 2012 at 00:00:31, Hunter, Jon wrote:

quoted

On 10/18/2012 01:04 PM, Hiremath, Vaibhav wrote:

quoted

On Thu, Oct 18, 2012 at 22:12:07, Hunter, Jon wrote:

...

quoted

quoted

Yes, but do you also see the bug that is hiding in gpmc_mem_init()?

My point is to highlight this and not hide it, so that we can fix it
now. Otherwise if we wait until we enable the gpmc driver with DT and
this could hinder the DT migration later.

As I already mentioned in my previous response, your patch is required 
irrespective of this patch. I would consider your patch as a cleanup patch.


Both the patches are independent, your patch is handling the error path 
properly, whereas, my patch makes sure that you don't unnecessarily probe 
GPMC if you are booting from DT and GPMC node is not present, as described 
above.

Your patch hides a bug. That's my point. How do you expect am335x ever
to support gpmc devices if this bug is not addressed?

Jon,
May be my commit description was mis-leading to you.
I am not commenting anything on your bug-fix, but do not agree that it is 
anything to do with hiding a bug.

So we can agree is disagree on that ;-)

I only agree with you on one point, if someone wants to change the board-
file to use GPMC with DT boot mode, then he will not be able to use it.

quoted

So I think that you are over-simplifying it when you say that my patch
is just a clean-up patch. I agree that it is adding appropriate error
handling, but it also highlights the presence of a bug by allowing the
probe to fail.

Anyway, I don't care to debate this any further, 

Me neither...

quoted

we just need to fix
gpmc_mem_init().

Agreed, and that's what your patch rightly doing it.

No. My patch does not fix the _actual_ bug, it is still there. Why do
you think that the probe is still failing for am335x? Without fixing it
am335x will never be able to support gpmc. So gpmc_mem_init() still
needs to be fixed.

Jon