From: Grant Likely <grant.likely@secretlab•ca>
To: Shawn Guo <shawn.guo@linaro•org>
Cc: linux-mmc@vger•kernel.org, linux-next@vger•kernel.org,
linux-kernel@vger•kernel.org, cbouatmailru@gmail•com,
w.sang@pengutronix•de, cjb@laptop•org, arnd@arndb•de,
sfr@canb•auug.org.au, patches@linaro•org,
Shawn Guo <shawn.guo@freescale•com>
Subject: Re: [PATCH] mmc: sdhci: change sdhci-pltfm into a module
Date: Fri, 3 Jun 2011 14:03:16 -0600 [thread overview]
Message-ID: <20110603200316.GB17972@ponder.secretlab.ca> (raw)
In-Reply-To: <1306983470-18588-1-git-send-email-shawn.guo@linaro.org>
On Thu, Jun 02, 2011 at 10:57:50AM +0800, Shawn Guo wrote:
> From: Shawn Guo <shawn.guo@freescale•com>
>
> There are a couple of problems left from the sdhci pltfm and OF
> consolidation changes.
>
> * When building more than one sdhci-pltfm based drivers in the same
> image, linker will give multiple definition error on the sdhci-pltfm
> helper functions. For example right now, building sdhci-of-esdhc
> and sdhci-of-hlwd together is a valid combination from Kconfig view.
>
> * With the current build method, there is error with building the
> drivers as module, but module installation fails with modprobe.
>
> The patch fixes above problems by changing sdhci-pltfm into a module.
> To avoid EXPORT_SYMBOL on so many big endian IO accessors, it moves
> these accessors into sdhci-pltfm.h as the 'static inline' functions.
> As a result, sdhci.h needs to be included in sdhci-pltfm.h, and in
> turn can be removed from individual drivers which already include
> sdhci-pltfm.h.
Mostly looks good. One comment below about a static inline, but
otherwise you can add my:
Acked-by: Grant Likely <grant.likely@secretlab•ca>
> +static inline void sdhci_be32bs_writew(struct sdhci_host *host,
> + u16 val, int reg)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + int base = reg & ~0x3;
> + int shift = (reg & 0x2) * 8;
> +
> + switch (reg) {
> + case SDHCI_TRANSFER_MODE:
> + /*
> + * Postpone this write, we must do it together with a
> + * command write that is down below.
> + */
> + pltfm_host->xfer_mode_shadow = val;
> + return;
> + case SDHCI_COMMAND:
> + sdhci_be32bs_writel(host,
> + val << 16 | pltfm_host->xfer_mode_shadow,
> + SDHCI_TRANSFER_MODE);
> + return;
> + }
> + clrsetbits_be32(host->ioaddr + base, 0xffff << shift, val << shift);
> +}
This is really too big to be a static inline. Go ahead and make it an
exported symbol. Personally, I wouldn't worry about it and just exporting all
of these accessors. Making them inline doesn't buy much anyway since
they are used to initialize an ops table, and making them inline
forces each driver to instantiate its own copy.
g.
> +
> +static inline void sdhci_be32bs_writeb(struct sdhci_host *host, u8 val, int reg)
> +{
> + int base = reg & ~0x3;
> + int shift = (reg & 0x3) * 8;
> +
> + clrsetbits_be32(host->ioaddr + base , 0xff << shift, val << shift);
> +}
> +#endif /* CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER */
>
> extern void sdhci_get_of_property(struct platform_device *pdev);
>
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 1f66aca..18b0bd3 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -24,7 +24,6 @@
> #include <mach/gpio.h>
> #include <mach/sdhci.h>
>
> -#include "sdhci.h"
> #include "sdhci-pltfm.h"
>
> static u32 tegra_sdhci_readl(struct sdhci_host *host, int reg)
> --
> 1.7.4.1
>
>
next prev parent reply other threads:[~2011-06-03 20:03 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-01 3:04 linux-next: build failure after merge of the final tree (mmc tree related) Stephen Rothwell
2011-06-01 3:39 ` Shawn Guo
2011-06-02 2:57 ` [PATCH] mmc: sdhci: change sdhci-pltfm into a module Shawn Guo
2011-06-03 20:03 ` Grant Likely [this message]
2011-06-04 12:08 ` Shawn Guo
2011-06-05 6:56 ` Wolfram Sang
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=20110603200316.GB17972@ponder.secretlab.ca \
--to=grant.likely@secretlab$(echo .)ca \
--cc=arnd@arndb$(echo .)de \
--cc=cbouatmailru@gmail$(echo .)com \
--cc=cjb@laptop$(echo .)org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-mmc@vger$(echo .)kernel.org \
--cc=linux-next@vger$(echo .)kernel.org \
--cc=patches@linaro$(echo .)org \
--cc=sfr@canb$(echo .)auug.org.au \
--cc=shawn.guo@freescale$(echo .)com \
--cc=shawn.guo@linaro$(echo .)org \
--cc=w.sang@pengutronix$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox