On Fri, Oct 19, 2012 at 00:00:31, Hunter, Jon wrote: > > On 10/18/2012 01:04 PM, Hiremath, Vaibhav wrote: > > On Thu, Oct 18, 2012 at 22:12:07, Hunter, Jon wrote: > > ... > > >> 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. 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. > 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... > we just need to fix > gpmc_mem_init(). > Agreed, and that's what your patch rightly doing it. Thanks, Vaibhav > Jon > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html