No subject
Wed Jun 1 12:03:18 EDT 2011
design choice to connect card CD/WP to the pads or not, which the
controller CD/WP signal/function is available on.
> > + ESDHC_WP_GPIO, /* external gpio pin for WP */
> > +};
> > +
> > +enum cd_types {
> > + ESDHC_CD_NONE, /* no CD, neither signal nor gpio */
> > + ESDHC_CD_SIGNAL, /* mmc internal CD signal */
>
> ditto
>
ditto
> > + ESDHC_CD_GPIO, /* external gpio pin for CD */
> > + ESDHC_CD_PERMANENT, /* no CD, card permanently wired to host */
> > +};
> > +
> > /**
> > - * struct esdhc_platform_data - optional platform data for esdhc on i.MX
> > - *
> > - * strongly recommended for i.MX25/35, not needed for other variants
> > + * struct esdhc_platform_data - platform data for esdhc on i.MX
> > *
> > - * @wp_gpio: gpio for write_protect (-EINVAL if unused)
> > - * @cd_gpio: gpio for card_detect interrupt (-EINVAL if unused)
> > + * @wp_gpio: gpio for write_protect
> > + * @cd_gpio: gpio for card_detect interrupt
> > + * @wp_type: type of write_protect method (see wp_types enum above)
> > + * @cd_type: type of card_detect method (see cd_types enum above)
> > */
> >
> > struct esdhc_platform_data {
> > unsigned int wp_gpio;
> > unsigned int cd_gpio;
> > + enum wp_types wp_type;
> > + enum cd_types cd_type;
> > };
> > #endif /* __ASM_ARCH_IMX_ESDHC_H */
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> > index 79b7a9a..9cba6eb 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -29,7 +29,6 @@
> > #define SDHCI_VENDOR_SPEC 0xC0
> > #define SDHCI_VENDOR_SPEC_SDIO_QUIRK 0x00000002
> >
> > -#define ESDHC_FLAG_GPIO_FOR_CD (1 << 0)
> > /*
> > * The CMDTYPE of the CMD register (offset 0xE) should be set to
> > * "11" when the STOP CMD12 is issued on imx53 to abort one
> > @@ -70,19 +69,15 @@ static inline void esdhc_clrset_le(struct sdhci_host *host, u32 mask, u32 val, i
> >
> > static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
> > {
> > - struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > - struct pltfm_imx_data *imx_data = pltfm_host->priv;
> > + struct esdhc_platform_data *boarddata =
> > + host->mmc->parent->platform_data;
> >
> > - /* fake CARD_PRESENT flag on mx25/35 */
> > + /* fake CARD_PRESENT flag */
> > u32 val = readl(host->ioaddr + reg);
> >
> > if (unlikely((reg == SDHCI_PRESENT_STATE)
> > - && (imx_data->flags & ESDHC_FLAG_GPIO_FOR_CD))) {
> > - struct esdhc_platform_data *boarddata =
> > - host->mmc->parent->platform_data;
> > -
> > - if (boarddata && gpio_is_valid(boarddata->cd_gpio)
> > - && gpio_get_value(boarddata->cd_gpio))
> > + && gpio_is_valid(boarddata->cd_gpio))) {
> > + if (gpio_get_value(boarddata->cd_gpio))
> > /* no card, if a valid gpio says so... */
> > val &= ~SDHCI_CARD_PRESENT;
> > else
> > @@ -97,12 +92,13 @@ static void esdhc_writel_le(struct sdhci_host *host, u32 val, int reg)
> > {
> > struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > struct pltfm_imx_data *imx_data = pltfm_host->priv;
> > + struct esdhc_platform_data *boarddata =
> > + host->mmc->parent->platform_data;
> >
> > if (unlikely((reg == SDHCI_INT_ENABLE || reg == SDHCI_SIGNAL_ENABLE)
> > - && (imx_data->flags & ESDHC_FLAG_GPIO_FOR_CD)))
> > + && (boarddata->cd_type == ESDHC_CD_GPIO)))
> > /*
> > * these interrupts won't work with a custom card_detect gpio
> > - * (only applied to mx25/35)
> > */
> > val &= ~(SDHCI_INT_CARD_REMOVE | SDHCI_INT_CARD_INSERT);
> >
> > @@ -201,6 +197,18 @@ static unsigned int esdhc_pltfm_get_min_clock(struct sdhci_host *host)
> > return clk_get_rate(pltfm_host->clk) / 256 / 16;
> > }
> >
> > +static unsigned int esdhc_pltfm_get_ro(struct sdhci_host *host)
> > +{
> > + struct esdhc_platform_data *boarddata =
> > + host->mmc->parent->platform_data;
> > +
> > + if (gpio_is_valid(boarddata->wp_gpio))
> > + return gpio_get_value(boarddata->wp_gpio);
> > + else
> > + return !(readl(host->ioaddr + SDHCI_PRESENT_STATE) &
> > + SDHCI_WRITE_PROTECT);
> > +}
>
> Aren't you missing the NONE case here? Plus, I don't like having a
> get_ro-function for the SIGNAL case, because in that case it is
> superfluous. Though, I am not feeling strong about this if it makes the
> rest of the code messier.
>
OK, I will go back to the existing one, which only handles gpio case
in esdhc_pltfm_get_ro and get it assigned to .get_ro only in case of
ESDHC_WP_GPIO.
> > +
> > static struct sdhci_ops sdhci_esdhc_ops = {
> > .read_l = esdhc_readl_le,
> > .read_w = esdhc_readw_le,
> > @@ -212,6 +220,7 @@ static struct sdhci_ops sdhci_esdhc_ops = {
> > .get_min_clock = esdhc_pltfm_get_min_clock,
> > .get_max_blk_size = esdhc_pltfm_get_max_blk_size,
> > .get_max_blk_count = esdhc_pltfm_get_max_blk_count,
> > + .get_ro = esdhc_pltfm_get_ro,
> > };
> >
> > static struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
> > @@ -221,17 +230,6 @@ static struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
> > .ops = &sdhci_esdhc_ops,
> > };
> >
> > -static unsigned int esdhc_pltfm_get_ro(struct sdhci_host *host)
> > -{
> > - struct esdhc_platform_data *boarddata =
> > - host->mmc->parent->platform_data;
> > -
> > - if (boarddata && gpio_is_valid(boarddata->wp_gpio))
> > - return gpio_get_value(boarddata->wp_gpio);
> > - else
> > - return -ENOSYS;
> > -}
> > -
> > static irqreturn_t cd_irq(int irq, void *data)
> > {
> > struct sdhci_host *sdhost = (struct sdhci_host *)data;
> > @@ -272,23 +270,33 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
> > if (!cpu_is_mx25())
> > host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
> >
> > - if (cpu_is_mx25() || cpu_is_mx35()) {
> > - /* write_protect can't be routed to controller, use gpio */
> > - sdhci_esdhc_ops.get_ro = esdhc_pltfm_get_ro;
> > - }
> > -
> > if (!(cpu_is_mx25() || cpu_is_mx35() || cpu_is_mx51()))
> > imx_data->flags |= ESDHC_FLAG_MULTIBLK_NO_INT;
> >
> > boarddata = host->mmc->parent->platform_data;
> > - if (boarddata) {
> > + if (!boarddata) {
> > + dev_err(mmc_dev(host->mmc), "no board data!\n");
> > + err = -EINVAL;
> > + goto no_board_data;
> > + }
> > +
> > + /* write_protect */
> > + if (boarddata->wp_type == ESDHC_WP_GPIO) {
> > err = gpio_request_one(boarddata->wp_gpio, GPIOF_IN, "ESDHC_WP");
> > if (err) {
> > dev_warn(mmc_dev(host->mmc),
> > - "no write-protect pin available!\n");
> > - boarddata->wp_gpio = err;
> > + "no write-protect pin available!\n");
> > + boarddata->wp_gpio = -EINVAL;
> > }
> > + } else
> > + boarddata->wp_gpio = -EINVAL;
>
> else-block needs braces
>
OK
--
Regards,
Shawn
More information about the linux-arm-kernel
mailing list