All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tony Lindgren <tony@atomide.com>
To: Ohad Ben-Cohen <ohad@wizery.com>
Cc: "Igor Grinberg" <grinberg@compulab.co.il>,
	"Russell King - ARM Linux" <linux@arm.linux.org.uk>,
	linux-omap@vger.kernel.org,
	"Santosh Shilimkar" <santosh.shilimkar@ti.com>,
	"Cousson, Benoit" <b-cousson@ti.com>,
	"Paul Walmsley" <paul@pwsan.com>, "Kevin Hilman" <khilman@ti.com>,
	"Afzal Mohammed" <afzal@ti.com>,
	"Péter Ujfalusi" <peter.ujfalusi@ti.com>
Subject: Re: Errors at boot time from OMAP4430SDP (and a whinge about serial stuff)
Date: Fri, 19 Oct 2012 10:07:59 -0700	[thread overview]
Message-ID: <20121019170759.GK4730@atomide.com> (raw)
In-Reply-To: <CAK=Wgba_-k0kCHah6t6L4=qt838j30ZRVRbgAdJ1yzHDyA8frw@mail.gmail.com>

* Ohad Ben-Cohen <ohad@wizery.com> [121018 10:00]:
> Hi Igor,
> 
> On Wed, Oct 17, 2012 at 2:43 PM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> > You are adding declarations inside the common-board-devices.h,
> > but the implementation inside the devices.c.
> > I can see that devices.c has only on-soc devices in it.
> > So, I would expect to have the wl12xx_board_init() function implementation
> > inside the common-board-devices.c file.
> 
> I really don't mind. Tony do you have any preference?

Yes common-board-devices.c would be better.
 
> > Another minor nit: I don't think you need to mark the declaration as __init,
> > only the implementation, or is it for documenting it so?
> 
> It may be, but I don't really mind removing it. Let's remove it if
> we'll move to common-board-devices.c, otherwise it probably isn't
> worth the noise.

OK
 
> > Instead of the above, wouldn't it be better to have:
> > #if defined(CONFIG_WL12XX_PLATFORM_DATA)
> > int __init wl12xx_board_init(struct wl12xx_platform_data *wlan_data, int irq_gpio)
> > {
> > ...
> > }
> > #else
> > inline int wl12xx_board_init(struct wl12xx_platform_data *wlan_data, int irq_gpio)
> > {
> >         return 0;
> > }
> > #endif
> 
> I think readability-wise we're probably better off without the #ifdef.

We could optimize away a minimal amount of code for many configurations
with the ifdef? :)

Regards,

Tony

  reply	other threads:[~2012-10-19 17:08 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-12 15:54 Errors at boot time from OMAP4430SDP (and a whinge about serial stuff) Russell King - ARM Linux
2012-10-12 16:24 ` Tony Lindgren
2012-10-12 16:31   ` Russell King - ARM Linux
2012-10-12 16:34   ` Benoit Cousson
2012-10-12 16:56     ` Tony Lindgren
2012-10-12 18:47   ` Kevin Hilman
2012-10-14 18:30   ` Ohad Ben-Cohen
2012-10-16 18:10     ` Tony Lindgren
2012-10-16 18:26       ` Tony Lindgren
2012-10-17  9:10         ` Ohad Ben-Cohen
2012-10-17 12:43           ` Igor Grinberg
2012-10-18 16:58             ` Ohad Ben-Cohen
2012-10-19 17:07               ` Tony Lindgren [this message]
2012-10-21 14:54                 ` Ohad Ben-Cohen
2012-10-23  7:37                   ` Igor Grinberg
2012-10-23  7:51                     ` Ohad Ben-Cohen
2012-10-23  9:46                       ` Igor Grinberg
2012-10-24  1:54                   ` Tony Lindgren
2012-10-24 11:15                     ` Ohad Ben-Cohen
2012-10-25 19:03                       ` Tony Lindgren
2012-10-15  5:54   ` Mohammed, Afzal
2012-10-16 18:12     ` Tony Lindgren
2012-10-16 19:24       ` Benoit Cousson
2012-10-16 19:53         ` Tony Lindgren
2012-10-15  7:37   ` Péter Ujfalusi
2012-10-23 10:10 ` Russell King - ARM Linux
2012-10-25  1:09   ` Tony Lindgren
2012-10-25 16:05     ` Balaji T K
2012-10-25 17:24     ` Kevin Hilman
2012-10-25 17:38       ` Kevin Hilman
2012-10-25 17:46         ` 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=20121019170759.GK4730@atomide.com \
    --to=tony@atomide.com \
    --cc=afzal@ti.com \
    --cc=b-cousson@ti.com \
    --cc=grinberg@compulab.co.il \
    --cc=khilman@ti.com \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=ohad@wizery.com \
    --cc=paul@pwsan.com \
    --cc=peter.ujfalusi@ti.com \
    --cc=santosh.shilimkar@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.