All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tony Lindgren <tony@atomide.com>
To: "Syed Mohammed, Khasim" <x0khasim@ti.com>
Cc: Linux OMAP <linux-omap-open-source@linux.omap.com>
Subject: Re: [PATCH 5/12] OMAP3430 support in mach-omap2 folder
Date: Tue, 12 Jun 2007 23:05:44 -0700	[thread overview]
Message-ID: <20070613060543.GC4030@atomide.com> (raw)
In-Reply-To: <9C23CDD79DA20A479D4615857B2E2C47011E594C@dlee13.ent.ti.com>

* Syed Mohammed, Khasim <x0khasim@ti.com> [070612 20:48]:
> >> > We have now common debug card interface. Check
> >> > arch/arm/plat-omap/debug-devices.c and do similar calls like
> >> > board-h4.c {debug_card_init}.
> >>
> >> For now, I would like to continue with this. I will incorporate this
> >comment as a separate clean up patch.
> >
> >Can you please fix what Trilok suggested? Remember, we want to use
> >the same patch for sending it upstream eventually.
> >
> I think it's not a correct approach to use debug_card_init for 2430 SDP. I have following view point.
> 
> 1. There is no debug card on 3430 or 2430 SDP as it was on H4.
> 
> 2. There will be #if's for 2430 and 3430 because many statements doesn't hold correct for 2430 / 3430
> 
>         smc91x_resources[0].start = addr + 0x300;
>         smc91x_resources[0].end   = addr + 0x30f;
> 
> The offset address for 3430 is 0x0 and 0x0f so, one if here.
> 
>         led_resources[0].start = addr;
>         led_resources[0].end   = addr + SZ_4K - 1;
> 
> These things are not required. If tomorrow we add more for 2430 / 3430 then this function has to be split anyway.
> 
> 3. The function is exported as an extern from board.h. Which is mainly meant for Nokia N series, this should be corrected. Why should we include board.h for other boards that might not use these structures.
> 
> 4. The Configuration doesn't show in Menuconfig as there is no statement next to bool.
> 	config OMAP_DEBUG_DEVICES
>         bool
>         help
>           For debug cards on TI reference boards.
> 
> The macro CONFIG_OMAP_DEBUG_DEVICES doesn't get enabled for 2430 / 3430.
> 
> 5. Having a complete debug_devices and corresponding file for 2430 / 3430 doesn't make sense, there are no LEDs or such debug devices on these boards. Unfortuanely SMSC is same, so its bit confusing I believe. But this Ethernet controller can be handled in a much simpler way, why to unnecessarily complex it?
> 
> Please don't take me in a defensive mood. I am trying to convince that this method doesn't hold good for 2430 / 3430. And what ever we were doing in posted patch is a more optimal approach. But if still you feel that this is what has to be done, then fine, I can add more ifs and elses and get the code working for 2430 / 3430.
> 
> Let me know your views at the earliest.

OK, thanks for explaining. Let's keep what you had, then later on we
can use the sdp-common.c for 2430-sdp and 3430-sdp.

Regards,

Tony

      reply	other threads:[~2007-06-13  6:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-31 17:58 [PATCH 5/12] OMAP3430 support in mach-omap2 folder Syed Mohammed, Khasim
2007-06-01  6:21 ` Trilok Soni
2007-06-01 12:38   ` Syed Mohammed, Khasim
2007-06-12 18:23     ` Tony Lindgren
2007-06-13  3:47       ` Syed Mohammed, Khasim
2007-06-13  6:05         ` Tony Lindgren [this message]

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=20070613060543.GC4030@atomide.com \
    --to=tony@atomide.com \
    --cc=linux-omap-open-source@linux.omap.com \
    --cc=x0khasim@ti.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.