All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jon Hunter <jon-hunter@ti.com>
To: "Hiremath, Vaibhav" <hvaibhav@ti.com>
Cc: Richard Cochran <richardcochran@gmail.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"Mohammed, Afzal" <afzal@ti.com>,
	Russell King <linux@arm.linux.org.uk>,
	Arnd Bergmann <arnd@arndb.de>, Tony Lindgren <tony@atomide.com>,
	David Miller <davem@davemloft.net>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>
Subject: Re: [PATCH 1/5] ARM: OMAP2+: gpmc: Fix kernel BUG for DT boot mode
Date: Thu, 18 Oct 2012 13:30:31 -0500	[thread overview]
Message-ID: <50804AC7.3000309@ti.com> (raw)
In-Reply-To: <79CD15C6BA57404B839C016229A409A83EB43D0F@DBDE01.ent.ti.com>


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?

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, we just need to fix
gpmc_mem_init().

Jon

WARNING: multiple messages have this Message-ID (diff)
From: jon-hunter@ti.com (Jon Hunter)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/5] ARM: OMAP2+: gpmc: Fix kernel BUG for DT boot mode
Date: Thu, 18 Oct 2012 13:30:31 -0500	[thread overview]
Message-ID: <50804AC7.3000309@ti.com> (raw)
In-Reply-To: <79CD15C6BA57404B839C016229A409A83EB43D0F@DBDE01.ent.ti.com>


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?

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, we just need to fix
gpmc_mem_init().

Jon

  reply	other threads:[~2012-10-18 18:30 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-15 19:16 [PATCH 0/5] am335x fixes for 3.7-rc2 Richard Cochran
2012-10-15 19:16 ` Richard Cochran
2012-10-15 19:16 ` [PATCH 1/5] ARM: OMAP2+: gpmc: Fix kernel BUG for DT boot mode Richard Cochran
2012-10-15 19:16   ` Richard Cochran
2012-10-16 17:48   ` Tony Lindgren
2012-10-16 17:48     ` Tony Lindgren
2012-10-16 20:58     ` Jon Hunter
2012-10-16 20:58       ` Jon Hunter
2012-10-16 20:58       ` Jon Hunter
2012-10-16 21:26       ` Tony Lindgren
2012-10-16 21:26         ` Tony Lindgren
2012-10-17 14:41         ` Jon Hunter
2012-10-17 14:41           ` Jon Hunter
2012-10-17 14:41           ` Jon Hunter
2012-10-17 16:13           ` Tony Lindgren
2012-10-17 16:13             ` Tony Lindgren
2012-10-16 19:47   ` Jon Hunter
2012-10-16 19:47     ` Jon Hunter
2012-10-16 19:47     ` Jon Hunter
2012-10-18 16:16     ` Hiremath, Vaibhav
2012-10-18 16:16       ` Hiremath, Vaibhav
2012-10-18 16:42       ` Jon Hunter
2012-10-18 16:42         ` Jon Hunter
2012-10-18 18:04         ` Hiremath, Vaibhav
2012-10-18 18:04           ` Hiremath, Vaibhav
2012-10-18 18:30           ` Jon Hunter [this message]
2012-10-18 18:30             ` Jon Hunter
2012-10-18 18:39             ` Hiremath, Vaibhav
2012-10-18 18:39               ` Hiremath, Vaibhav
2012-10-18 18:46               ` Jon Hunter
2012-10-18 18:46                 ` Jon Hunter
2012-10-15 19:16 ` [PATCH 2/5] ARM: OMAP3+: hwmod: Add AM33XX HWMOD data for davinci_mdio Richard Cochran
2012-10-15 19:16   ` Richard Cochran
2012-10-16 17:50   ` Tony Lindgren
2012-10-16 17:50     ` Tony Lindgren
2012-10-15 19:16 ` [PATCH 3/5] net: davinci_mdio: Fix type mistake in calling runtime-pm api Richard Cochran
2012-10-15 19:16   ` Richard Cochran
2012-10-18 16:13   ` Hiremath, Vaibhav
2012-10-18 16:13     ` Hiremath, Vaibhav
2012-10-15 19:16 ` [PATCH 4/5] net: cpsw: Add parent<->child relation support between cpsw and mdio Richard Cochran
2012-10-15 19:16   ` Richard Cochran
2012-10-18 16:13   ` Hiremath, Vaibhav
2012-10-18 16:13     ` Hiremath, Vaibhav
2012-10-15 19:16 ` [PATCH 5/5] arm/dts: am33xx: Add cpsw and mdio module nodes for AM33XX Richard Cochran
2012-10-15 19:16   ` Richard Cochran
2012-10-16 17:51   ` Tony Lindgren
2012-10-16 17:51     ` Tony Lindgren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50804AC7.3000309@ti.com \
    --to=jon-hunter@ti.com \
    --cc=afzal@ti.com \
    --cc=arnd@arndb.de \
    --cc=davem@davemloft.net \
    --cc=hvaibhav@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=richardcochran@gmail.com \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.