Re: [PATCH v4 1/4] omap4 hsmmc: Adding card detect support for MMC1

[kishore kadiyala]

Hi Samuel,

Could you please review this patch which touches mfd/twl6030-irq.c for
card detect support
of MMC1 controller on OMAP4.

Regards,
Kishore

On Mon, Sep 27, 2010 at 1:25 PM, kishore kadiyala
 wrote:
> Cc: Samuel Ortiz 
>
> On Fri, Sep 24, 2010 at 10:43 PM, kishore kadiyala
>  wrote:
>> Adding card detect callback function and card detect configuration
>> function for MMC1 Controller on OMAP4.
>>
>> Card detect configuration function does initial configuration of the
>> MMC Control & PullUp-PullDown registers of Phoenix.
>>
>> For MMC1 Controller, card detect interrupt source is
>> twl6030 which is non-gpio. The card detect call back function provides
>> card present/absent status by reading MMC Control register present
>> on twl6030.
>>
>> Since OMAP4 doesn't use any GPIO line as used in OMAP3 for card detect,
>> the suspend/resume initialization which was done in omap_hsmmc_gpio_init
>> previously is moved to the probe thus making it generic for both OMAP3 & 
>> OMAP4.
>>
>> Cc: Tony Lindgren 
>> Cc: Andrew Morton 
>> Cc: Madhusudhan Chikkature 
>> Cc: Adrian Hunter 
>> Signed-off-by: Kishore Kadiyala 
>> ---
>>  arch/arm/mach-omap2/board-4430sdp.c |    7 +++-
>>  drivers/mfd/twl6030-irq.c           |   73 
>> +++
>>  drivers/mmc/host/omap_hsmmc.c       |    4 +-
>>  include/linux/i2c/twl.h             |   31 +++
>>  4 files changed, 112 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-4430sdp.c 
>> b/arch/arm/mach-omap2/board-4430sdp.c
>> index 9447644..a49f285 100644
>> --- a/arch/arm/mach-omap2/board-4430sdp.c
>> +++ b/arch/arm/mach-omap2/board-4430sdp.c
>> @@ -227,9 +227,14 @@ static int omap4_twl6030_hsmmc_late_init(struct device 
>> *dev)
>>        struct omap_mmc_platform_data *pdata = dev->platform_data;
>>
>>        /* Setting MMC1 Card detect Irq */
>> -       if (pdev->id == 0)
>> +       if (pdev->id == 0) {
>> +               ret = twl6030_mmc_card_detect_config();
>> +               if (ret)
>> +                       pr_err("Failed configuring MMC1 card detect\n");
>>                pdata->slots[0].card_detect_irq = TWL6030_IRQ_BASE +
>>                                                MMCDETECT_INTR_OFFSET;
>> +               pdata->slots[0].card_detect = twl6030_mmc_card_detect;
>> +       }
>>        return ret;
>>  }
>>
>> diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c
>> index 10bf228..2d3bb82 100644
>> --- a/drivers/mfd/twl6030-irq.c
>> +++ b/drivers/mfd/twl6030-irq.c
>> @@ -36,6 +36,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>
>>  /*
>>  * TWL6030 (unlike its predecessors, which had two level interrupt handling)
>> @@ -223,6 +224,78 @@ int twl6030_interrupt_mask(u8 bit_mask, u8 offset)
>>  }
>>  EXPORT_SYMBOL(twl6030_interrupt_mask);
>>
>> +int twl6030_mmc_card_detect_config(void)
>> +{
>> +       int ret;
>> +       u8 reg_val = 0;
>> +
>> +       /* Unmasking the Card detect Interrupt line for MMC1 from Phoenix */
>> +       twl6030_interrupt_unmask(TWL6030_MMCDETECT_INT_MASK,
>> +                                               REG_INT_MSK_LINE_B);
>> +       twl6030_interrupt_unmask(TWL6030_MMCDETECT_INT_MASK,
>> +                                               REG_INT_MSK_STS_B);
>> +       /*
>> +        * Intially Configuring MMC_CTRL for receving interrupts &
>> +        * Card status on TWL6030 for MMC1
>> +        */
>> +       ret = twl_i2c_read_u8(TWL6030_MODULE_ID0, ®_val, TWL6030_MMCCTRL);
>> +       if (ret < 0) {
>> +               pr_err("twl6030: Failed to read MMCCTRL, error %d\n", ret);
>> +               return ret;
>> +       }
>> +       reg_val &= ~VMMC_AUTO_OFF;
>> +       reg_val |= SW_FC;
>> +       ret = twl_i2c_write_u8(TWL6030_MODULE_ID0, reg_val, TWL6030_MMCCTRL);
>> +       if (ret < 0) {
>> +               pr_err("twl6030: Failed to write MMCCTRL, error %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       /* Configuring PullUp-PullDown register */
>> +       ret = twl_i2c_read_u8(TWL6030_MODULE_ID0, ®_val,
>> +                                               TWL6030_CFG_INPUT_PUPD3);
>> +       if (ret < 0) {
>> +               pr_err("twl6030: Failed to read CFG_INPUT_PUPD3, error %d\n",
>> +                                                                       ret);
>> +               return ret;
>> +       }
>> +       reg_val &= ~(MMC_PU | MMC_PD);
>> +       ret = twl_i2c_write_u8(TWL6030_MODULE_ID0, reg_val,
>> +                                               TWL6030_CFG_INPUT_PUPD3);
>> +       if (ret < 0) {
>> +               pr_err("twl6030: Failed to write CFG_INPUT_PUPD3, error 
>> %d\n",
>> +                                                                       ret);
>> +               return ret;
>> +       }
>> +       return 0;
>> +}
>> +EXPORT_SYMBOL(twl6030_mmc_card_detect_config);
>> +
>> +int twl6030_mmc_card_detect(struct device *dev, int slot)
>> +{
>> +       int ret = -EIO;
>> +       u8 read_reg = 

Re: [PATCH] MMC: Refine block layer waiting for card state

[Adrian Hunter]

On 27/09/10 06:32, Ethan Du wrote:


 The while loop when handling rw request may become deadloop in
case of bad card
 I've seen mmcqd gets blocked forever after a single error message:

mmcblk0: error -110 sending read/write command, response 0x900, card
status 0x80e00

 Also there was case of card reports status without error

mmcblk0: error -110 sending read/write command, response 0x900, card
status 0xe00

 After this error, the card can stay in prg state, and never comes back,
 and may not report any error further. So a break out condition
 should be set in mmc block layer:
* should not enter the waiting loop in case of error


How do you know there are not any errors where you do need to wait?


* should break out from the waiting loop, if card response with error
* should break out from the waiting loop when timeout

 These will not help with the card, one more thing to do:
* re-init the card in case of too many errors


Using mmc_detect_change() will give you a new block device.
That is probably a bad idea for a non-removable card.  Also
if the reinitialization is going to help, then why wait for
lots of errors before doing it.



Signed-off-by: Ethan Du
---
  drivers/mmc/card/block.c |   35 +++
  drivers/mmc/core/mmc.c   |9 +++--
  drivers/mmc/core/sd.c|8 ++--
  include/linux/mmc/card.h |3 +++
  include/linux/mmc/mmc.h  |1 +
  5 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index d545f79..1d304a2 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -316,12 +316,14 @@ out:
return err ? 0 : 1;
  }

+#define BUSY_TIMEOUT_MS (16 * 1024)
  static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
  {
struct mmc_blk_data *md = mq->data;
struct mmc_card *card = md->queue.card;
struct mmc_blk_request brq;
int ret = 1, disable_multi = 0;
+   unsigned long timeout = 0;

mmc_claim_host(card->host);

@@ -453,7 +455,9 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
*mq, struct request *req)
   brq.stop.resp[0], status);
}

-   if (!mmc_host_is_spi(card->host)&&  rq_data_dir(req) != READ) {
+   if (!mmc_host_is_spi(card->host)&&  rq_data_dir(req) != READ&&
+   !brq.cmd.error&&  !brq.data.error&&  !brq.stop.error) {
+   timeout = jiffies + msecs_to_jiffies(BUSY_TIMEOUT_MS);
do {
int err;

@@ -466,13 +470,22 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
*mq, struct request *req)
   req->rq_disk->disk_name, err);
goto cmd_err;
}
+   if (cmd.resp[0]&  R1_ERROR_MASK) {
+   printk(KERN_ERR "%s: card err %#x\n",
+   req->rq_disk->disk_name,
+   cmd.resp[0]);
+   goto cmd_err;
+   }
/*
 * Some cards mishandle the status bits,
 * so make sure to check both the busy
 * indication and the card state.
 */
-   } while (!(cmd.resp[0]&  R1_READY_FOR_DATA) ||
-   (R1_CURRENT_STATE(cmd.resp[0]) == 7));
+   if ((cmd.resp[0]&  R1_READY_FOR_DATA)&&
+   (R1_CURRENT_STATE(cmd.resp[0]) != 7))
+   break;
+   } while (time_before(jiffies, timeout));
+   /* Ignore timeout out */

  #if 0
if (cmd.resp[0]&  ~0x0900)
@@ -510,11 +523,11 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
*mq, struct request *req)

return 1;

- cmd_err:
-   /*
-* If this is an SD card and we're writing, we can first
-* mark the known good sectors as ok.
-*
+cmd_err:
+   /*
+* If this is an SD card and we're writing, we can first
+* mark the known good sectors as ok.
+*
 * If the card is not SD, we can still ok written sectors
 * as reported by the controller (which might be less than
 * the real number of written sectors, but never more).
@@ -541,6 +554,12 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
*mq, struct request *req)
ret = __blk_end_request(req, -EIO, blk_rq_cur_bytes(req));
spin_unlock_irq(&md->lock);

+   card->err_count++;
+   if (card->err_count>= ERR_TRIGGER_REINIT) {
+   printk(KERN_WARNING "%s: re-init the card d

Re: [PATCH] sdhci-s3c: support non-standard clock setting for c210

[Kyungmin Park]

To Ben,

Maybe you missing this patch. It's the jaehoon's approach.

Thank you,
Kyungmin Park

2010/8/31 Jaehoon Chung :
> This is sdhci-s3c patch for c210.
> c210 didn't use divider of host controller.
>
> Host Controller need other clock setting methods.
>
> So I add the callback functions for s5pc210.
> also I set 400KHz for initial clock.
>
> Signed-off-by: Jaehoon Chung 
>  Signed-off-by: Kyungmin Park 
>
> ---
>  arch/arm/plat-samsung/include/plat/sdhci.h |   19 
>  drivers/mmc/host/sdhci-s3c.c               |   68 
> 
>  2 files changed, 87 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/plat-samsung/include/plat/sdhci.h 
> b/arch/arm/plat-samsung/include/plat/sdhci.h
> index 30844c2..7c75ee3 100644
> --- a/arch/arm/plat-samsung/include/plat/sdhci.h
> +++ b/arch/arm/plat-samsung/include/plat/sdhci.h
> @@ -15,6 +15,8 @@
>  #ifndef __PLAT_S3C_SDHCI_H
>  #define __PLAT_S3C_SDHCI_H __FILE__
>
> +#include 
> +
>  struct platform_device;
>  struct mmc_host;
>  struct mmc_card;
> @@ -288,4 +290,21 @@ static inline void s5pv210_default_sdhci3(void) { }
>
>  #endif /* CONFIG_S5PV210_SETUP_SDHCI */
>
> +/* re-define device name depending on support. */
> +static inline void s3c_hsmmc_setname(char *name)
> +{
> +#ifdef CONFIG_S3C_DEV_HSMMC
> +       s3c_device_hsmmc0.name = name;
> +#endif
> +#ifdef CONFIG_S3C_DEV_HSMMC1
> +       s3c_device_hsmmc1.name = name;
> +#endif
> +#ifdef CONFIG_S3C_DEV_HSMMC2
> +       s3c_device_hsmmc2.name = name;
> +#endif
> +#ifdef CONFIG_S3C_DEV_HSMMC3
> +       s3c_device_hsmmc3.name = name;
> +#endif
> +}
> +
>  #endif /* __PLAT_S3C_SDHCI_H */
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index 71ad416..3927793 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -52,6 +52,11 @@ struct sdhci_s3c {
>        struct clk              *clk_bus[MAX_BUS_CLK];
>  };
>
> +enum soc_type {
> +       TYPE_SAMSUNG,   /* S5PC1XX, S3C... */
> +       TYPE_S5PC210,   /* S5PC210 */
> +};
> +
>  static inline struct sdhci_s3c *to_s3c(struct sdhci_host *host)
>  {
>        return sdhci_priv(host);
> @@ -232,6 +237,52 @@ static unsigned int sdhci_s3c_get_min_clock(struct 
> sdhci_host *host)
>        return min;
>  }
>
> +/**
> +* sdhci_s3c_get_max_clk - callback to get maximum clock frequency.
> +*/
> +static unsigned int sdhci_s5pc210_get_max_clock(struct sdhci_host *host)
> +{
> +       struct sdhci_s3c *ourhost = to_s3c(host);
> +       unsigned int rate;
> +       int ptr = ourhost->cur_clk;
> +
> +       rate = clk_round_rate(ourhost->clk_bus[ptr], UINT_MAX);
> +
> +       return rate;
> +}
> +
> +/**
> + * sdhci_s3c_get_min_clock - callback to get minimal supported clock value
> +*/
> +static unsigned int sdhci_s5pc210_get_min_clock(struct sdhci_host *host)
> +{
> +       struct sdhci_s3c *ourhost = to_s3c(host);
> +       unsigned int rate;
> +       int ptr = ourhost->cur_clk;
> +
> +       rate = clk_round_rate(ourhost->clk_bus[ptr], 40);
> +
> +       return rate;
> +}
> +
> +/**
> + * sdhci_s5pc210_set_clock - callback on clock change
> +*/
> +static void sdhci_s5pc210_set_clock(struct sdhci_host *host,
> +               unsigned int clock)
> +{
> +       struct sdhci_s3c *ourhost = to_s3c(host);
> +
> +       if (clock == 0)
> +               return;
> +
> +       sdhci_s3c_set_clock(host, clock);
> +
> +       clk_set_rate(ourhost->clk_bus[ourhost->cur_clk], clock);
> +
> +       host->clock = clock;
> +}
> +
>  static struct sdhci_ops sdhci_s3c_ops = {
>        .get_max_clock          = sdhci_s3c_get_max_clk,
>        .set_clock              = sdhci_s3c_set_clock,
> @@ -395,6 +446,12 @@ static int __devinit sdhci_s3c_probe(struct 
> platform_device *pdev)
>        host->quirks = 0;
>        host->irq = irq;
>
> +       if (pdev->id_entry->driver_data == TYPE_S5PC210) {
> +               sdhci_s3c_ops.set_clock = sdhci_s5pc210_set_clock;
> +               sdhci_s3c_ops.get_min_clock = sdhci_s5pc210_get_min_clock;
> +               sdhci_s3c_ops.get_max_clock = sdhci_s5pc210_get_max_clock;
> +       }
> +
>        /* Setup quirks for the controller */
>        host->quirks |= SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
>        host->quirks |= SDHCI_QUIRK_NO_HISPD_BIT;
> @@ -520,6 +577,16 @@ static int sdhci_s3c_resume(struct platform_device *dev)
>  #define sdhci_s3c_resume NULL
>  #endif
>
> +static struct platform_device_id sdhci_driver_ids[] = {
> +       {
> +               .name           = "s3c-sdhci",
> +               .driver_data    = TYPE_SAMSUNG,
> +       }, {
> +               .name           = "s5pc210-sdhci",
> +               .driver_data    = TYPE_S5PC210,
> +       }, { },
> +};
> +
>  static struct platform_driver sdhci_s3c_driver = {
>        .probe          = sdhci_s3c_probe,
>        .remove         = __devexit_p(sdhci_s3c_remove),
> @@ -529,6 +596,7 @@ static struct platform_driver sdhci_s3c_driver = {
>                .owner  = THIS_MODULE,
>                

Re: [RFC RESEND] sdhci-s3c: support clock enable/disable (clock-gating)

[Jaehoon Chung]

Jae hoon Chung wrote:
> 2010/9/17 Matt Fleming :
>> On Thu, Sep 16, 2010 at 03:46:50PM +0900, Jaehoon Chung wrote:
>>> Hi all,
>>>   This is a RFC patch that support clock-gating for saving power 
>>> consumption.
>>>   I found mmc_host_enable/mmc_host_disable function in core.c
>>>   (using MMC_CAP_DSIABLE. i think that use when host enable/disable)
>>>   So, i used that functions and implemented some functions in sdhci-s3c.c & 
>>> sdhci.c
>>>
>>> i want any feedback. how do you think about this patch?
>>> Plz let me know...
>>>
>>> Thank you all
>>>
>>>  Signed-off-by: Jaehoon Chung 
>>>  Signed-off-by: Kyungmin Park 
>>>
>>> ---
>>>  drivers/mmc/host/sdhci-s3c.c |   36 
>>>  drivers/mmc/host/sdhci.c |   30 ++
>>>  drivers/mmc/host/sdhci.h |4 
>>>  3 files changed, 70 insertions(+), 0 deletions(-)
>> [...]
>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 401527d..fa2e55d 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -1245,7 +1245,37 @@ out:
>>>   spin_unlock_irqrestore(&host->lock, flags);
>>>  }
>>>
>>> +static int sdhci_enable_clk(struct mmc_host *mmc)
>>> +{
>>> + struct sdhci_host *host = mmc_priv(mmc);
>>> + int ret = 0;
>>> +
>>> + if (host->old_clock != 0 && host->clock == 0) {
>>> + if (host->ops->enable)
>>> + ret = host->ops->enable(host);
>>> + sdhci_set_clock(host, host->old_clock);
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int sdhci_disable_clk(struct mmc_host *mmc, int lazy)
>>> +{
>>> + struct sdhci_host *host = mmc_priv(mmc);
>>> + int ret = 0;
>>> +
>>> + if (host->clock != 0) {
>>> + host->old_clock = host->clock;
>>> + sdhci_set_clock(host, 0);
>>> + if (host->ops->disable)
>>> + ret = host->ops->disable(host, lazy);
>>> + }
>>> + return ret;
>>> +}
>>> +
>>>  static const struct mmc_host_ops sdhci_ops = {
>>> + .enable = sdhci_enable_clk,
>>> + .disable= sdhci_disable_clk,
>>>   .request= sdhci_request,
>>>   .set_ios= sdhci_set_ios,
>>>   .get_ro = sdhci_get_ro,
>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>> index d316bc7..0c6f143 100644
>>> --- a/drivers/mmc/host/sdhci.h
>>> +++ b/drivers/mmc/host/sdhci.h
>>> @@ -278,6 +278,8 @@ struct sdhci_host {
>>>   unsigned inttimeout_clk;/* Timeout freq (KHz) */
>>>
>>>   unsigned intclock;  /* Current clock (MHz) */
>>> + unsigned intold_clock;  /* Old clock (MHz) */
>>> + unsigned intclk_cnt;/* Clock user count */
>>>   u8  pwr;/* Current voltage */
>>>
>>>   struct mmc_request  *mrq;   /* Current request */
>>> @@ -323,6 +325,8 @@ struct sdhci_ops {
>>>   unsigned int(*get_max_clock)(struct sdhci_host *host);
>>>   unsigned int(*get_min_clock)(struct sdhci_host *host);
>>>   unsigned int(*get_timeout_clock)(struct sdhci_host *host);
>>> + int (*enable)(struct sdhci_host *host);
>>> + int (*disable)(struct sdhci_host *host, int lazy);
>>>  };
>>>
>>>  #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
>> I could have misunderstood something, but do you really need this new
>> 'old_clock' member? Is the previous clock value not stored in
>> host->ios.clock?
> 
> Thanks Matt for your comment.
> if host->ios.clock set zero, i think that need previous value (for
> example 52MHz..)
> So i add 'old_clock' member...i will test, after removed 'old_clock'
> 
> anyother doubt? and problem?
> Thanks
> 
Hi Chris.
I want to know how do you think about this patch.
If you have any doubt in this patch, let me know plz.

Thanks
Jaehoon Chung
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2 2/2] sdhci-s3c: Add support no internal clock divider in host controller

[Kukjin Kim]

Jeongbae Seo wrote:
> 
> Ben Dooks worte:
> > On 17/09/10 10:45, Kukjin Kim wrote:
> > > From: Hyuk Lee 
> > >
> > > This patch adds to support no internal clock divider in SDHCI.
> > > The external clock divider can be used to make a proper clock
> > > because SDHCI doesn't support internal clock divider by itself.
> > >
> > > Signed-off-by: Hyuk Lee 
> > > Signed-off-by: Jeongbae Seo 
> > > Signed-off-by: Kukjin Kim 
> > > ---
> > >  drivers/mmc/host/sdhci-s3c.c |   60
> > ++
> > >  1 files changed, 60 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/mmc/host/sdhci-s3c.c
b/drivers/mmc/host/sdhci-s3c.c
> > > index e6e0438..5ad5ed7 100644
> > > --- a/drivers/mmc/host/sdhci-s3c.c
> > > +++ b/drivers/mmc/host/sdhci-s3c.c
> > > @@ -96,6 +96,13 @@ static unsigned int sdhci_s3c_get_max_clk(struct
> > sdhci_host *host)
> > >   unsigned int rate, max;
> > >   int clk;
> > >
> > > + /*
> > > +  * There is only one clock source(sclk) if there is no clock
> > divider
> > > +  * in the host controller
> > > +  */
> > > + if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK)
> > > + return clk_round_rate(ourhost->clk_bus[2], UINT_MAX);
> >
> > interesting,  doesn't have a second parameter to
> > clk_round_rate().
> >
> Hi Ben,
> 
> Thanks for your comments.
> 
> When I see clk_round_rate in , which function is defined
> as "long clk_round_rate(struct clk *clk, unsigned long rate)" that has two
> parameters.
> Please let me know if you have another meaning for this.
> 
> >
> > >   /* note, a reset will reset the clock source */
> > >
> > >   sdhci_s3c_check_sclk(host);
> > > @@ -130,6 +137,15 @@ static unsigned int
sdhci_s3c_consider_clock(struct
> > sdhci_s3c *ourhost,
> > >   if (!clksrc)
> > >   return UINT_MAX;
> > >
> > > + /*
> > > +  * There is only one clock source(sclk) if there is no clock
> > divider
> > > +  * in the host controller
> > > +  */
> > > + if (ourhost->host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK) {
> > > + rate = clk_round_rate(clksrc, wanted);
> > > + return wanted - rate;
> > > + }
> >
> > Why does this need a quirk, instead of just having one clock available
> > in the list of usable clocks?
> >
> 
> The available clock is made by dividing a clock source with a certain
> divider value.
> Most of the host controller has this capability that can divide a clock
what
> we want.
> However, the host controller of both S5PC210 and S5PV310 don't have this
so
> we have to
> Add additional routine to make a proper clock with outland clock divider
> instead of
> Internal clock divider in host controller.
> 
Hi Ben,

If any your suggestion in this case, S5PV310/S5PC210 has no clock divider in
the HSMMC IP block, please kindly let me know. Will address comments from
you.

As you know, there are many solution for this...But I'd like to use your
preference in here.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim , Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] sdhci: correct f_min in sd 3.0

[zhangfei gao]

On Tue, Sep 28, 2010 at 12:05 AM, Philip Rakity  wrote:
>
> The code snippet
>> +                     for (div = 2; div < SDHCI_MAX_DIV_SPEC_300; div += 2) {
>>                               if ((host->max_clk / div) <= clock)
>>                                       break;
>
> is not correct.  In sdio 3.0 the value is not a power of 2 for the divider 
> but the value to be divided down-- all the bits count

That's the reason using div += 2 instead of div *= 2

> 2) 10-bit Divided Clock
> Host Controller Version 3.00 supports this mandatory mode instead of the
> 8-bit Divided Clock Mode.  The length of the divider is extended to 10 bits 
> and all
> divider values shall be supported.
> 3FFh    1/2046  Divided Clock
>
> Philip
>
> Hi Zhangfei,
>
> On Sat, Sep 25, 2010 at 12:23:33AM -0400, zhangfei gao wrote:
>> From: Zhangfei Gao 
>> Date: Mon, 20 Sep 2010 15:15:18 -0400
>> Subject: [PATCH] sdhci: correct f_min in sd 3.0
>>
>> Signed-off-by: Zhangfei Gao 
>> ---
>>  drivers/mmc/host/sdhci.c |    8 +---
>>  drivers/mmc/host/sdhci.h |    3 +++
>>  2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 5928b0a..829e78a 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1007,14 +1007,14 @@ static void sdhci_set_clock(struct sdhci_host *host, 
>> unsigned int clock)
>>               if (host->max_clk <= clock)
>>                       div = 1;
>>               else {
>> -                     for (div = 2; div < 2046; div += 2) {
>> +                     for (div = 2; div < SDHCI_MAX_DIV_SPEC_300; div += 2) {
>>                               if ((host->max_clk / div) <= clock)
>>                                       break;
>>                       }
>>               }
>>       } else {
>>               /* Version 2.00 divisors must be a power of 2. */
>> -             for (div = 1; div < 256; div *= 2) {
>> +             for (div = 1; div < SDHCI_MAX_DIV_SPEC_200; div *= 2) {
>>                       if ((host->max_clk / div) <= clock)
>>                               break;
>>               }
>> @@ -1836,8 +1836,10 @@ int sdhci_add_host(struct sdhci_host *host)
>>       mmc->ops = &sdhci_ops;
>>       if (host->ops->get_min_clock)
>>               mmc->f_min = host->ops->get_min_clock(host);
>> +     else if (host->version >= SDHCI_SPEC_300)
>> +             mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_300;
>>       else
>> -             mmc->f_min = host->max_clk / 256;
>> +             mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;
>>       mmc->f_max = host->max_clk;
>>       mmc->caps |= MMC_CAP_SDIO_IRQ;
>>
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index ae28a31..d35fb3d 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -185,6 +185,9 @@
>>  #define   SDHCI_SPEC_200     1
>>  #define   SDHCI_SPEC_300     2
>>
>> +#define      SDHCI_MAX_DIV_SPEC_200  256
>> +#define      SDHCI_MAX_DIV_SPEC_300  2046
>> +
>>  struct sdhci_ops;
>>
>>  struct sdhci_host {
>
> Thanks very much, I've applied this to mmc-next now.
>
> - Chris.
> --
> Chris Ball      
> One Laptop Per Child
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] mmc: fix init f_min

[Hein_Tibosch]

 On 28-9-2010 11:34, zhangfei gao wrote:
>
> Just curious how to get 40, which is slowest speed for
> initializaion, is this OK for all card?
>
Hi Zhangfei,

It looks as if you missed another thread about f_min recently?

See: https://patchwork.kernel.org/patch/177932/

This was in response to complaints about 400 Khz being too high for some ... 
platforms

But note that 400 Khz is the highest possible speed to initialize an SD card,

Hein Tibosch

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] sdhci: correct f_min in sd 3.0

[Philip Rakity]

The code snippet
> + for (div = 2; div < SDHCI_MAX_DIV_SPEC_300; div += 2) {
>   if ((host->max_clk / div) <= clock)
>   break;

is not correct.  In sdio 3.0 the value is not a power of 2 for the divider but 
the value to be divided down-- all the bits count
2) 10-bit Divided Clock
Host Controller Version 3.00 supports this mandatory mode instead of the
8-bit Divided Clock Mode.  The length of the divider is extended to 10 bits and 
all
divider values shall be supported.
3FFh1/2046  Divided Clock

Philip

Hi Zhangfei,

On Sat, Sep 25, 2010 at 12:23:33AM -0400, zhangfei gao wrote:
> From: Zhangfei Gao 
> Date: Mon, 20 Sep 2010 15:15:18 -0400
> Subject: [PATCH] sdhci: correct f_min in sd 3.0
> 
> Signed-off-by: Zhangfei Gao 
> ---
>  drivers/mmc/host/sdhci.c |8 +---
>  drivers/mmc/host/sdhci.h |3 +++
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 5928b0a..829e78a 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1007,14 +1007,14 @@ static void sdhci_set_clock(struct sdhci_host *host, 
> unsigned int clock)
>   if (host->max_clk <= clock)
>   div = 1;
>   else {
> - for (div = 2; div < 2046; div += 2) {
> + for (div = 2; div < SDHCI_MAX_DIV_SPEC_300; div += 2) {
>   if ((host->max_clk / div) <= clock)
>   break;
>   }
>   }
>   } else {
>   /* Version 2.00 divisors must be a power of 2. */
> - for (div = 1; div < 256; div *= 2) {
> + for (div = 1; div < SDHCI_MAX_DIV_SPEC_200; div *= 2) {
>   if ((host->max_clk / div) <= clock)
>   break;
>   }
> @@ -1836,8 +1836,10 @@ int sdhci_add_host(struct sdhci_host *host)
>   mmc->ops = &sdhci_ops;
>   if (host->ops->get_min_clock)
>   mmc->f_min = host->ops->get_min_clock(host);
> + else if (host->version >= SDHCI_SPEC_300)
> + mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_300;
>   else
> - mmc->f_min = host->max_clk / 256;
> + mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;
>   mmc->f_max = host->max_clk;
>   mmc->caps |= MMC_CAP_SDIO_IRQ;
>  
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index ae28a31..d35fb3d 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -185,6 +185,9 @@
>  #define   SDHCI_SPEC_200 1
>  #define   SDHCI_SPEC_300 2
>  
> +#define  SDHCI_MAX_DIV_SPEC_200  256
> +#define  SDHCI_MAX_DIV_SPEC_300  2046
> +
>  struct sdhci_ops;
>  
>  struct sdhci_host {

Thanks very much, I've applied this to mmc-next now.

- Chris.
-- 
Chris Ball  
One Laptop Per Child
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2 2/2] sdhci-s3c: Add support no internal clock divider in host controller

[Jeongbae Seo]

Ben Dooks worte:
> On 17/09/10 10:45, Kukjin Kim wrote:
> > From: Hyuk Lee 
> >
> > This patch adds to support no internal clock divider in SDHCI.
> > The external clock divider can be used to make a proper clock
> > because SDHCI doesn't support internal clock divider by itself.
> >
> > Signed-off-by: Hyuk Lee 
> > Signed-off-by: Jeongbae Seo 
> > Signed-off-by: Kukjin Kim 
> > ---
> >  drivers/mmc/host/sdhci-s3c.c |   60
> ++
> >  1 files changed, 60 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> > index e6e0438..5ad5ed7 100644
> > --- a/drivers/mmc/host/sdhci-s3c.c
> > +++ b/drivers/mmc/host/sdhci-s3c.c
> > @@ -96,6 +96,13 @@ static unsigned int sdhci_s3c_get_max_clk(struct
> sdhci_host *host)
> > unsigned int rate, max;
> > int clk;
> >
> > +   /*
> > +* There is only one clock source(sclk) if there is no clock
> divider
> > +* in the host controller
> > +*/
> > +   if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK)
> > +   return clk_round_rate(ourhost->clk_bus[2], UINT_MAX);
> 
> interesting,  doesn't have a second parameter to
> clk_round_rate().
> 
Hi Ben,

Thanks for your comments.

When I see clk_round_rate in , which function is defined 
as "long clk_round_rate(struct clk *clk, unsigned long rate)" that has two
parameters.
Please let me know if you have another meaning for this.

> 
> > /* note, a reset will reset the clock source */
> >
> > sdhci_s3c_check_sclk(host);
> > @@ -130,6 +137,15 @@ static unsigned int sdhci_s3c_consider_clock(struct
> sdhci_s3c *ourhost,
> > if (!clksrc)
> > return UINT_MAX;
> >
> > +   /*
> > +* There is only one clock source(sclk) if there is no clock
> divider
> > +* in the host controller
> > +*/
> > +   if (ourhost->host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK) {
> > +   rate = clk_round_rate(clksrc, wanted);
> > +   return wanted - rate;
> > +   }
> 
> Why does this need a quirk, instead of just having one clock available
> in the list of usable clocks?
> 

The available clock is made by dividing a clock source with a certain
divider value.
Most of the host controller has this capability that can divide a clock what
we want.
However, the host controller of both S5PC210 and S5PV310 don't have this so
we have to
Add additional routine to make a proper clock with outland clock divider
instead of
Internal clock divider in host controller.

Best Regards,
Jeongbae Seo


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] mmc: fix init f_min

[zhangfei gao]

On Mon, Sep 27, 2010 at 5:33 PM, David Vrabel  wrote:
> zhangfei gao wrote:
>> On Mon, Sep 20, 2010 at 10:34 AM, David Vrabel  wrote:
>>> zhangfei gao wrote:
 index 5db49b1..9114c87 100644
 --- a/drivers/mmc/core/core.c
 +++ b/drivers/mmc/core/core.c
 @@ -910,9 +910,9 @@ static void mmc_power_up(struct mmc_host *host)
       if (host->f_min > 40) {
               pr_warning("%s: Minimum clock frequency too high for "
                               "identification mode\n", mmc_hostname(host));
 -             host->ios.clock = host->f_min;
 -     } else
               host->ios.clock = 40;
 +     } else
 +             host->ios.clock = host->f_min;
>>> NAK.
>>>
>>> The code is already correctly requesting 400 kHz (unless the controller
>>> can't go that slow).
>>
>> Original code
>> if (host->f_min > 40) {
>>                 pr_warning("%s: Minimum clock frequency too high for "
>>                                 "identification mode\n", mmc_hostname(host));
>>                 host->ios.clock = host->f_min;
>>         } else
>>                 host->ios.clock = 40;
>>
>> With this code, the init clock is would be at lease 400 kHz, and no
>> matter how bigger the host->f_min it is, is this really correct?
>
> It's not clear what you're saying or why you think the original code is
> wrong.  Consider a host controller that reports a minimum frequency of 1
> Hz.  Obviously we wouldn't want to do the card identification at this
> clock rate, yes?

Thanks for your explanation.

The issue you met is f_min is too slow.
The issue we met is f_min is too high which also cause init fail,
But it is OK, we could prevent in other place, duringinit set
mmc->f_min = host->max_clk/256 in sdc 2.0 and mmc->f_min =
host->max_clk/2046 in sdc 3.0

Just curious how to get 40, which is slowest speed for
initializaion, is this OK for all card?

>
> David
> --
> David Vrabel, Senior Software Engineer, Drivers
> CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
> Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/
>
>
> Member of the CSR plc group of companies. CSR plc registered in England and 
> Wales, registered number 4187346, registered office Churchill House, 
> Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] sdhci: correct f_min in sd 3.0

[Chris Ball]

Hi Zhangfei,

On Sat, Sep 25, 2010 at 12:23:33AM -0400, zhangfei gao wrote:
> From: Zhangfei Gao 
> Date: Mon, 20 Sep 2010 15:15:18 -0400
> Subject: [PATCH] sdhci: correct f_min in sd 3.0
> 
> Signed-off-by: Zhangfei Gao 
> ---
>  drivers/mmc/host/sdhci.c |8 +---
>  drivers/mmc/host/sdhci.h |3 +++
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 5928b0a..829e78a 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1007,14 +1007,14 @@ static void sdhci_set_clock(struct sdhci_host *host, 
> unsigned int clock)
>   if (host->max_clk <= clock)
>   div = 1;
>   else {
> - for (div = 2; div < 2046; div += 2) {
> + for (div = 2; div < SDHCI_MAX_DIV_SPEC_300; div += 2) {
>   if ((host->max_clk / div) <= clock)
>   break;
>   }
>   }
>   } else {
>   /* Version 2.00 divisors must be a power of 2. */
> - for (div = 1; div < 256; div *= 2) {
> + for (div = 1; div < SDHCI_MAX_DIV_SPEC_200; div *= 2) {
>   if ((host->max_clk / div) <= clock)
>   break;
>   }
> @@ -1836,8 +1836,10 @@ int sdhci_add_host(struct sdhci_host *host)
>   mmc->ops = &sdhci_ops;
>   if (host->ops->get_min_clock)
>   mmc->f_min = host->ops->get_min_clock(host);
> + else if (host->version >= SDHCI_SPEC_300)
> + mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_300;
>   else
> - mmc->f_min = host->max_clk / 256;
> + mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;
>   mmc->f_max = host->max_clk;
>   mmc->caps |= MMC_CAP_SDIO_IRQ;
>  
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index ae28a31..d35fb3d 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -185,6 +185,9 @@
>  #define   SDHCI_SPEC_200 1
>  #define   SDHCI_SPEC_300 2
>  
> +#define  SDHCI_MAX_DIV_SPEC_200  256
> +#define  SDHCI_MAX_DIV_SPEC_300  2046
> +
>  struct sdhci_ops;
>  
>  struct sdhci_host {

Thanks very much, I've applied this to mmc-next now.

- Chris.
-- 
Chris Ball  
One Laptop Per Child
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 1/2] mmc: Add helper function to check if a card is removable

[Matt Fleming]

On Mon, Sep 27, 2010 at 04:01:32PM +0100, Chris Ball wrote:
> 
> Applied to mmc-next with Wolfram's ACK and the following style change,
> thanks very much.

Thanks Chris!
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 2/2] mmc: sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case

[Chris Ball]

Hi Matt,

On Mon, Sep 27, 2010 at 09:42:20AM +0100, Matt Fleming wrote:
> From: Jaehoon Chung 
> 
> When a controller requires SDHCI_QUIRK_BROKEN_CARD_DETECTION, we poll
> for card insertion/removal, and that creates interrupts.  There's no
> need to be doing this if we have a non-removable card.
> 
> This patch requires cards to be removable before we're willing to set
> MMC_CAP_NEEDS_POLL.
> 
> Signed-off-by: Jaehoon Chung 
> Acked-by: Kyungmin Park 
> Acked-by: Matt Fleming 
> [cjb: modified changelog and code indentation]
> Signed-off-by: Chris Ball 

Thanks, applied to mmc-next with Wolfram's ACK.

- Chris.
-- 
Chris Ball  
One Laptop Per Child
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 1/2] mmc: Add helper function to check if a card is removable

[Chris Ball]

Hi Matt,

On Mon, Sep 27, 2010 at 09:42:19AM +0100, Matt Fleming wrote:
> There are two checks that need to be made when determining whether a
> card is removable. A host controller may set MMC_CAP_NONREMOVABLE if the
> controller does not support removing cards (e.g. eMMC), in which case
> the card is physically non-removable. Also the 'mmc_assume_removable'
> module parameter can be configured at module load time, in which case
> the card may be logically non-removable.
> 
> A helper function keeps the logic in one place so that code always
> checks both conditions.
> 
> Because this new function is likely to be called from modules we now
> need to export the mmc_assume_removable symbol.
> 
> Signed-off-by: Matt Fleming 
> Acked-by: Kyungmin Park 
> Tested-by: Jaehoon Chung 

Applied to mmc-next with Wolfram's ACK and the following style change,
thanks very much.

diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 23a4864..2e0fe62 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -272,7 +272,7 @@ extern int mmc_assume_removable;
 
 static inline int mmc_card_is_removable(struct mmc_host *host)
 {
-   return (!(host->caps & MMC_CAP_NONREMOVABLE) && mmc_assume_removable);
+   return !(host->caps & MMC_CAP_NONREMOVABLE) && mmc_assume_removable;
 }
 
 #endif
-- 
Chris Ball  
One Laptop Per Child
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH (mmc-next) 2/3] mmc: split the sdhci.h to help platforms that uses shdci-pltfm d.d.

[Wolfram Sang]

> Hmm, it's not necessary to move them, indeed.
> They can stay in the original header if you prefer.
> No issues on my side at all.
> Let me know so I'll rework the patch and send it again.

You should know that yourself by now ;)

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature

Re: [PATCH (mmc-next) 1/3] mmc: add suspend/resume in the sdhci-pltfm driver

[Peppe CAVALLARO]

On 09/27/2010 04:15 PM, Wolfram Sang wrote:
> 
> Better, but you still do not return the retval from the sdhci-functions.
> And please annotate the #endif with the define it depends on as a
> comment. And wait for more comments and respin the series as a whole,
> not every patch independently. That soon gets a mess.

Hi Wolfram,
Thanks for reviewing it. I'll modify the patch and send it again to you.

Regards,
Peppe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH (mmc-next) 2/3] mmc: split the sdhci.h to help platforms that uses shdci-pltfm d.d.

[Peppe CAVALLARO]

On 09/27/2010 04:13 PM, Wolfram Sang wrote:
> On Mon, Sep 27, 2010 at 02:57:50PM +0200, Peppe CAVALLARO wrote:
> OK for the host-struct. What about the io-accessors? Do we really need
> them?

Hi Wolfram.

Hmm, it's not necessary to move them, indeed.
They can stay in the original header if you prefer.
No issues on my side at all.
Let me know so I'll rework the patch and send it again.

Regards,
Peppe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH (mmc-next) 3/3] mmc: fix a warning when compile the sdhci d.d.

[Wolfram Sang]

> > should detect that BUG never returns). The include-removal could be in a
> > seperate patch, not sure if it is worth, though.
> 
> It can include in a separate patch.
> It makes sense if apply the previous patch to split the header file
> because we remove duplicated include files.

I understand, but I am still sceptical. If the source file itself uses
something from the h-file, it should include it IMHO. Don't know if it
is common sense this way.

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature

Re: [PATCH (mmc-next) 1/3] mmc: add suspend/resume in the sdhci-pltfm driver

[Wolfram Sang]

On Mon, Sep 27, 2010 at 03:24:02PM +0200, Peppe CAVALLARO wrote:
> On 09/27/2010 02:57 PM, Peppe CAVALLARO wrote:
> > On 09/27/2010 12:37 PM, Wolfram Sang wrote:
> >  > On Thu, Sep 23, 2010 at 11:14:24AM +0200, Giuseppe CAVALLARO wrote:
> >  >> Signed-off-by: Giuseppe Cavallaro 
> >  >
> >  > I'd prefer it the way sdhci-mv.c is doing it (just one #if-block and
> >  > returning the code from the sdhci_*-functions). Then it should be fine.
> > 
> > Okay! I can rework it without any problems.
> 
> Hi Wolfram
> attached the new patch:
> Let me know if it's ok.

Better, but you still do not return the retval from the sdhci-functions.
And please annotate the #endif with the define it depends on as a
comment. And wait for more comments and respin the series as a whole,
not every patch independently. That soon gets a mess.

> From 847f635d0824c88f6575d86f5e9f50283883cd60 Mon Sep 17 00:00:00 2001
> From: Giuseppe Cavallaro 
> Date: Thu, 23 Sep 2010 10:13:00 +0200
> Subject: [PATCH (mmc-next)] mmc: add suspend/resume in the sdhci-pltfm driver 
> (V2)
> 
> Signed-off-by: Giuseppe Cavallaro 
> ---
>  drivers/mmc/host/sdhci-pltfm.c |   23 +++
>  1 files changed, 23 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
> index e045e3c..c43f954 100644
> --- a/drivers/mmc/host/sdhci-pltfm.c
> +++ b/drivers/mmc/host/sdhci-pltfm.c
> @@ -165,6 +165,27 @@ static const struct platform_device_id sdhci_pltfm_ids[] 
> = {
>  };
>  MODULE_DEVICE_TABLE(platform, sdhci_pltfm_ids);
>  
> +#ifdef CONFIG_PM
> +static int sdhci_pltfm_suspend(struct platform_device *dev, pm_message_t pm)
> +{
> + struct sdhci_host *host = platform_get_drvdata(dev);
> +
> + sdhci_suspend_host(host, pm);
> + return 0;
> +}
> +
> +static int sdhci_pltfm_resume(struct platform_device *dev)
> +{
> + struct sdhci_host *host = platform_get_drvdata(dev);
> +
> + sdhci_resume_host(host);
> + return 0;
> +}
> +#else
> +#define sdhci_pltfm_suspend  NULL
> +#define sdhci_pltfm_resume   NULL
> +#endif
> +
>  static struct platform_driver sdhci_pltfm_driver = {
>   .driver = {
>   .name   = "sdhci",
> @@ -173,6 +194,8 @@ static struct platform_driver sdhci_pltfm_driver = {
>   .probe  = sdhci_pltfm_probe,
>   .remove = __devexit_p(sdhci_pltfm_remove),
>   .id_table   = sdhci_pltfm_ids,
> + .suspend= sdhci_pltfm_suspend,
> + .resume = sdhci_pltfm_resume,
>  };
>  
>  
> /*\
> -- 
> 1.5.5.6
> 


-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature

Re: [PATCH (mmc-next) 2/3] mmc: split the sdhci.h to help platforms that uses shdci-pltfm d.d.

[Wolfram Sang]

On Mon, Sep 27, 2010 at 02:57:50PM +0200, Peppe CAVALLARO wrote:
> Hi Wolfram,
> 
> On 09/27/2010 12:43 PM, Wolfram Sang wrote:
> > On Thu, Sep 23, 2010 at 11:14:25AM +0200, Giuseppe CAVALLARO wrote:
> >> Some platforms based on the shdci-pltfm device driver need to
> >> set own quirks (that currently are in drivers/mmc/host/sdhci.h).
> >>
> >> This patch splits this header file in two parts:
> >>
> >> o drivers/mmc/host/sdhci.h
> >>  it includes the HC registers
> >>
> >> o include/linux/mmc/sdhci.h
> >>  it includes the private structures, callbacks, quirks etc.
> >>
> >> So, instead of including the shdci.h from devices/mmc/host, all
> >> the  platforms based on shdci-pltfm will be able to only include:
> >> include/linux/mmc/sdhci.h and include/linux/sdhci-pltfm.h.
> >>
> >> This has been tested on STM targets (STx7106, STx7108, STx5206).
> >>
> >> Note: drivers/mmc/host/sdhci.h also includes the linux/mmc/sdhci.h
> >> and no modifications should be needed on other sdhci- drivers.
> >>
> >> Signed-off-by: Giuseppe Cavallaro 
> >
> > IMO this is too much exporting here. I can't see a reason to export the
> > sdhci_host-structure, for example. My idea would be to start with a
> > minimal approach and just copy over the stuff we need now (the quirks).
> > If we need more later, we add it seperately then.
> 
> I had just started doing that but I decided to split the file to solve
> the issue for the .init call (we discussed in the thread
> http://marc.info/?l=linux-mmc&m=128523426925028&w=2).
> Indeed, this approach avoids to add other callbacks and reuses the
> existent code.

OK for the host-struct. What about the io-accessors? Do we really need
them?

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature

Re: [PATCH (mmc-next) 1/3] mmc: add suspend/resume in the sdhci-pltfm driver

[Peppe CAVALLARO]

On 09/27/2010 02:57 PM, Peppe CAVALLARO wrote:
> On 09/27/2010 12:37 PM, Wolfram Sang wrote:
>  > On Thu, Sep 23, 2010 at 11:14:24AM +0200, Giuseppe CAVALLARO wrote:
>  >> Signed-off-by: Giuseppe Cavallaro 
>  >
>  > I'd prefer it the way sdhci-mv.c is doing it (just one #if-block and
>  > returning the code from the sdhci_*-functions). Then it should be fine.
> 
> Okay! I can rework it without any problems.

Hi Wolfram
attached the new patch:
Let me know if it's ok.

Best Regards,
Peppe


> 
> Peppe
> 
>  >
>  >> ---
>  >> drivers/mmc/host/sdhci-pltfm.c | 22 ++
>  >> 1 files changed, 22 insertions(+), 0 deletions(-)
>  >>
>  >> diff --git a/drivers/mmc/host/sdhci-pltfm.c 
> b/drivers/mmc/host/sdhci-pltfm.c
>  >> index e045e3c..89ea64b 100644
>  >> --- a/drivers/mmc/host/sdhci-pltfm.c
>  >> +++ b/drivers/mmc/host/sdhci-pltfm.c
>  >> @@ -165,6 +165,24 @@ static const struct platform_device_id 
> sdhci_pltfm_ids[] = {
>  >> };
>  >> MODULE_DEVICE_TABLE(platform, sdhci_pltfm_ids);
>  >>
>  >> +#ifdef CONFIG_PM
>  >> +static int sdhci_pltfm_suspend(struct platform_device *dev, pm_message_t 
> pm)
>  >> +{
>  >> + struct sdhci_host *host = platform_get_drvdata(dev);
>  >> +
>  >> + sdhci_suspend_host(host, pm);
>  >> + return 0;
>  >> +}
>  >> +
>  >> +static int sdhci_pltfm_resume(struct platform_device *dev)
>  >> +{
>  >> + struct sdhci_host *host = platform_get_drvdata(dev);
>  >> +
>  >> + sdhci_resume_host(host);
>  >> + return 0;
>  >> +}
>  >> +#endif
>  >> +
>  >> static struct platform_driver sdhci_pltfm_driver = {
>  >> .driver = {
>  >> .name = "sdhci",
>  >> @@ -173,6 +191,10 @@ static struct platform_driver sdhci_pltfm_driver = {
>  >> .probe = sdhci_pltfm_probe,
>  >> .remove = __devexit_p(sdhci_pltfm_remove),
>  >> .id_table = sdhci_pltfm_ids,
>  >> +#ifdef CONFIG_PM
>  >> + .suspend = sdhci_pltfm_suspend,
>  >> + .resume = sdhci_pltfm_resume,
>  >> +#endif
>  >> };
>  >>
>  >> 
> /*\
>  >> --
>  >> 1.5.5.6
>  >>
>  >> --
>  >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>  >> the body of a message to majord...@vger.kernel.org
>  >> More majordomo info at http://vger.kernel.org/majordomo-info.html
>  >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> 

From 847f635d0824c88f6575d86f5e9f50283883cd60 Mon Sep 17 00:00:00 2001
From: Giuseppe Cavallaro 
Date: Thu, 23 Sep 2010 10:13:00 +0200
Subject: [PATCH (mmc-next)] mmc: add suspend/resume in the sdhci-pltfm driver (V2)

Signed-off-by: Giuseppe Cavallaro 
---
 drivers/mmc/host/sdhci-pltfm.c |   23 +++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index e045e3c..c43f954 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -165,6 +165,27 @@ static const struct platform_device_id sdhci_pltfm_ids[] = {
 };
 MODULE_DEVICE_TABLE(platform, sdhci_pltfm_ids);
 
+#ifdef CONFIG_PM
+static int sdhci_pltfm_suspend(struct platform_device *dev, pm_message_t pm)
+{
+	struct sdhci_host *host = platform_get_drvdata(dev);
+
+	sdhci_suspend_host(host, pm);
+	return 0;
+}
+
+static int sdhci_pltfm_resume(struct platform_device *dev)
+{
+	struct sdhci_host *host = platform_get_drvdata(dev);
+
+	sdhci_resume_host(host);
+	return 0;
+}
+#else
+#define sdhci_pltfm_suspend	NULL
+#define sdhci_pltfm_resume	NULL
+#endif
+
 static struct platform_driver sdhci_pltfm_driver = {
 	.driver = {
 		.name	= "sdhci",
@@ -173,6 +194,8 @@ static struct platform_driver sdhci_pltfm_driver = {
 	.probe		= sdhci_pltfm_probe,
 	.remove		= __devexit_p(sdhci_pltfm_remove),
 	.id_table	= sdhci_pltfm_ids,
+	.suspend	= sdhci_pltfm_suspend,
+	.resume		= sdhci_pltfm_resume,
 };
 
 /*\
-- 
1.5.5.6

Re: [PATCH (mmc-next) 2/3] mmc: split the sdhci.h to help platforms that uses shdci-pltfm d.d.

[Peppe CAVALLARO]

Hi Wolfram,

On 09/27/2010 12:43 PM, Wolfram Sang wrote:
> On Thu, Sep 23, 2010 at 11:14:25AM +0200, Giuseppe CAVALLARO wrote:
>> Some platforms based on the shdci-pltfm device driver need to
>> set own quirks (that currently are in drivers/mmc/host/sdhci.h).
>>
>> This patch splits this header file in two parts:
>>
>> o drivers/mmc/host/sdhci.h
>>  it includes the HC registers
>>
>> o include/linux/mmc/sdhci.h
>>  it includes the private structures, callbacks, quirks etc.
>>
>> So, instead of including the shdci.h from devices/mmc/host, all
>> the  platforms based on shdci-pltfm will be able to only include:
>> include/linux/mmc/sdhci.h and include/linux/sdhci-pltfm.h.
>>
>> This has been tested on STM targets (STx7106, STx7108, STx5206).
>>
>> Note: drivers/mmc/host/sdhci.h also includes the linux/mmc/sdhci.h
>> and no modifications should be needed on other sdhci- drivers.
>>
>> Signed-off-by: Giuseppe Cavallaro 
>
> IMO this is too much exporting here. I can't see a reason to export the
> sdhci_host-structure, for example. My idea would be to start with a
> minimal approach and just copy over the stuff we need now (the quirks).
> If we need more later, we add it seperately then.

I had just started doing that but I decided to split the file to solve
the issue for the .init call (we discussed in the thread
http://marc.info/?l=linux-mmc&m=128523426925028&w=2).
Indeed, this approach avoids to add other callbacks and reuses the
existent code.

Let me know.

Regards,
Peppe


>> ---
>>  drivers/mmc/host/sdhci.h  |  271 
>> ++---
>>  include/linux/mmc/sdhci.h |  269 
>> 
>>  2 files changed, 280 insertions(+), 260 deletions(-)
>>  create mode 100644 include/linux/mmc/sdhci.h
>>
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index ae28a31..3b96e4a 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -1,5 +1,7 @@
>>  /*
>> - *  linux/drivers/mmc/host/sdhci.h - Secure Digital Host Controller 
>> Interface driver
>> + *  linux/drivers/mmc/host/sdhci.h - Secure Digital Host Controller 
>> Interface
>> + *
>> + * Host Controller registers.
>>   *
>>   *  Copyright (C) 2005-2008 Pierre Ossman, All Rights Reserved.
>>   *
>> @@ -8,13 +10,10 @@
>>   * the Free Software Foundation; either version 2 of the License, or (at
>>   * your option) any later version.
>>   */
>> -#ifndef __SDHCI_H
>> -#define __SDHCI_H
>> +#ifndef __SDHCI_HW_H
>> +#define __SDHCI_HW_H
>>
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>> +#include 
>>
>>  /*
>>   * Controller registers
>> @@ -181,256 +180,8 @@
>>  #define  SDHCI_VENDOR_VER_SHIFT 8
>>  #define  SDHCI_SPEC_VER_MASK0x00FF
>>  #define  SDHCI_SPEC_VER_SHIFT   0
>> -#define   SDHCI_SPEC_1000
>> -#define   SDHCI_SPEC_2001
>> -#define   SDHCI_SPEC_3002
>> -
>> -struct sdhci_ops;
>> -
>> -struct sdhci_host {
>> -/* Data set by hardware interface driver */
>> -const char  *hw_name;   /* Hardware bus name */
>> -
>> -unsigned intquirks; /* Deviations from spec. */
>> -
>> -/* Controller doesn't honor resets unless we touch the clock register */
>> -#define SDHCI_QUIRK_CLOCK_BEFORE_RESET  (1<<0)
>> -/* Controller has bad caps bits, but really supports DMA */
>> -#define SDHCI_QUIRK_FORCE_DMA   (1<<1)
>> -/* Controller doesn't like to be reset when there is no card inserted. */
>> -#define SDHCI_QUIRK_NO_CARD_NO_RESET(1<<2)
>> -/* Controller doesn't like clearing the power reg before a change */
>> -#define SDHCI_QUIRK_SINGLE_POWER_WRITE  (1<<3)
>> -/* Controller has flaky internal state so reset it on each ios change */
>> -#define SDHCI_QUIRK_RESET_CMD_DATA_ON_IOS   (1<<4)
>> -/* Controller has an unusable DMA engine */
>> -#define SDHCI_QUIRK_BROKEN_DMA  (1<<5)
>> -/* Controller has an unusable ADMA engine */
>> -#define SDHCI_QUIRK_BROKEN_ADMA (1<<6)
>> -/* Controller can only DMA from 32-bit aligned addresses */
>> -#define SDHCI_QUIRK_32BIT_DMA_ADDR  (1<<7)
>> -/* Controller can only DMA chunk sizes that are a multiple of 32 bits */
>> -#define SDHCI_QUIRK_32BIT_DMA_SIZE  (1<<8)
>> -/* Controller can only ADMA chunks that are a multiple of 32 bits */
>> -#define SDHCI_QUIRK_32BIT_ADMA_SIZE (1<<9)
>> -/* Controller needs to be reset after each request to stay stable */
>> -#define SDHCI_QUIRK_RESET_AFTER_REQUEST (1<<10)
>> -/* Controller needs voltage and power writes to happen separately */
>> -#define SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER (1<<11)
>> -/* Controller provides an incorrect timeout value for transfers */
>> -#define SDHCI_QUIRK_BROKEN_TIMEOUT_VAL  (1<<12)
>> -/* Controller has an issue with buffer bits 

Re: [PATCH (mmc-next) 1/3] mmc: add suspend/resume in the sdhci-pltfm driver

[Peppe CAVALLARO]

On 09/27/2010 12:37 PM, Wolfram Sang wrote:
> On Thu, Sep 23, 2010 at 11:14:24AM +0200, Giuseppe CAVALLARO wrote:
>> Signed-off-by: Giuseppe Cavallaro 
> 
> I'd prefer it the way sdhci-mv.c is doing it (just one #if-block and
> returning the code from the sdhci_*-functions). Then it should be fine.

Okay! I can rework it without any problems.

Peppe

> 
>> ---
>>  drivers/mmc/host/sdhci-pltfm.c |   22 ++
>>  1 files changed, 22 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
>> index e045e3c..89ea64b 100644
>> --- a/drivers/mmc/host/sdhci-pltfm.c
>> +++ b/drivers/mmc/host/sdhci-pltfm.c
>> @@ -165,6 +165,24 @@ static const struct platform_device_id 
>> sdhci_pltfm_ids[] = {
>>  };
>>  MODULE_DEVICE_TABLE(platform, sdhci_pltfm_ids);
>>  
>> +#ifdef CONFIG_PM
>> +static int sdhci_pltfm_suspend(struct platform_device *dev, pm_message_t pm)
>> +{
>> +struct sdhci_host *host = platform_get_drvdata(dev);
>> +
>> +sdhci_suspend_host(host, pm);
>> +return 0;
>> +}
>> +
>> +static int sdhci_pltfm_resume(struct platform_device *dev)
>> +{
>> +struct sdhci_host *host = platform_get_drvdata(dev);
>> +
>> +sdhci_resume_host(host);
>> +return 0;
>> +}
>> +#endif
>> +
>>  static struct platform_driver sdhci_pltfm_driver = {
>>  .driver = {
>>  .name   = "sdhci",
>> @@ -173,6 +191,10 @@ static struct platform_driver sdhci_pltfm_driver = {
>>  .probe  = sdhci_pltfm_probe,
>>  .remove = __devexit_p(sdhci_pltfm_remove),
>>  .id_table   = sdhci_pltfm_ids,
>> +#ifdef CONFIG_PM
>> +.suspend = sdhci_pltfm_suspend,
>> +.resume = sdhci_pltfm_resume,
>> +#endif
>>  };
>>  
>>  
>> /*\
>> -- 
>> 1.5.5.6
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH (mmc-next) 3/3] mmc: fix a warning when compile the sdhci d.d.

[Peppe CAVALLARO]

Hi Wolfram,

On 09/27/2010 12:54 PM, Wolfram Sang wrote:
> On Thu, Sep 23, 2010 at 11:14:26AM +0200, Giuseppe CAVALLARO wrote:
>> This patch fixes a warning when compile the sdhci:
>>   pwr may be used uninitialized in sdhci_set_power
>> Also removes some include files that live in sdhci.h
>> header file.
>>
>> Signed-off-by: Giuseppe Cavallaro 
> 
> Okay for the set_power-part (what compiler are you using? Newer one

I tried with both 4.2.4 and 4.4.4 without any problems.

> should detect that BUG never returns). The include-removal could be in a
> seperate patch, not sure if it is worth, though.

It can include in a separate patch.
It makes sense if apply the previous patch to split the header file
because we remove duplicated include files.

Regards,
Peppe

> 
>> ---
>>  drivers/mmc/host/sdhci.c |   11 ++-
>>  1 files changed, 2 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 5928b0a..058dacd 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -18,13 +18,8 @@
>>  #include 
>>  #include 
>>  #include 
>> -#include 
>>  #include 
>>  
>> -#include 
>> -
>> -#include 
>> -
>>  #include "sdhci.h"
>>  
>>  #define DRIVER_NAME "sdhci"
>> @@ -1050,11 +1045,9 @@ out:
>>  
>>  static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
>>  {
>> -u8 pwr;
>> +u8 pwr = 0;
>>  
>> -if (power == (unsigned short)-1)
>> -pwr = 0;
>> -else {
>> +if (power != (unsigned short)-1) {
>>  switch (1 << power) {
>>  case MMC_VDD_165_195:
>>  pwr = SDHCI_POWER_180;
>> -- 
>> 1.5.5.6
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] sdhci-pltfm: add call back function

[Wolfram Sang]

On Mon, Sep 27, 2010 at 09:06:57AM +0800, Eric Miao wrote:
> On Mon, Sep 27, 2010 at 12:09 AM, Wolfram Sang  wrote:
> > On Sun, Sep 26, 2010 at 04:52:06AM -0400, zhangfei gao wrote:
> >> Add several call back to sdhci-pltfm.c, help give suggestion
> >
> > Just formal things, without looking at the code yet.
> >
> > Make seperate patches out of these, everyone with a proper description.
> >
> >> 1. struct sdhci_host *(*alloc_host)(struct device *dev), since
> >> specific driver need some private variable to allocate and free,
> >> including clk.
> >> 2. unsigned int  (*get_quirk)(struct sdhci_host *host); add this
> >> because one driver may serve several device, each one may require
> >> different quirk, and specific driver is right one to know.
> >> 3. void (*set_max_speed)(struct sdhci_host *host); this should be done
> >> after add_host, which impact f_max.
> >> 4. int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data
> >> *pdata, void* priv_pdata); copy from Wolfram Sang's patch to transfer
> >> platform data, copy here for test.
> >
> 
> Just a rough idea, considering the potential differences (I believe
> there will be
> more in the future) between the SDHCI of different SoCs, would it make more
> sense to make sdhci-pltfm.c as a common function library for sdhci-.c?

Yeah, I think Alan Cox mentioned this idea, too. My guess is that it
will be well received, if somebody actually does it ;) Up to that point,
it probably makes sense to keep redundancy low by the means we have
today, i.e. pltfm. That should help a later migration process.

Regards,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature

Re: [PATCH (mmc-next) 3/3] mmc: fix a warning when compile the sdhci d.d.

[Wolfram Sang]

On Thu, Sep 23, 2010 at 11:14:26AM +0200, Giuseppe CAVALLARO wrote:
> This patch fixes a warning when compile the sdhci:
>   pwr may be used uninitialized in sdhci_set_power
> Also removes some include files that live in sdhci.h
> header file.
> 
> Signed-off-by: Giuseppe Cavallaro 

Okay for the set_power-part (what compiler are you using? Newer one
should detect that BUG never returns). The include-removal could be in a
seperate patch, not sure if it is worth, though.

> ---
>  drivers/mmc/host/sdhci.c |   11 ++-
>  1 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 5928b0a..058dacd 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -18,13 +18,8 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  
> -#include 
> -
> -#include 
> -
>  #include "sdhci.h"
>  
>  #define DRIVER_NAME "sdhci"
> @@ -1050,11 +1045,9 @@ out:
>  
>  static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
>  {
> - u8 pwr;
> + u8 pwr = 0;
>  
> - if (power == (unsigned short)-1)
> - pwr = 0;
> - else {
> + if (power != (unsigned short)-1) {
>   switch (1 << power) {
>   case MMC_VDD_165_195:
>   pwr = SDHCI_POWER_180;
> -- 
> 1.5.5.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature

Re: [PATCH (mmc-next) 2/3] mmc: split the sdhci.h to help platforms that uses shdci-pltfm d.d.

[Wolfram Sang]

On Thu, Sep 23, 2010 at 11:14:25AM +0200, Giuseppe CAVALLARO wrote:
> Some platforms based on the shdci-pltfm device driver need to
> set own quirks (that currently are in drivers/mmc/host/sdhci.h).
> 
> This patch splits this header file in two parts:
> 
> o drivers/mmc/host/sdhci.h
>  it includes the HC registers
> 
> o include/linux/mmc/sdhci.h
>  it includes the private structures, callbacks, quirks etc.
> 
> So, instead of including the shdci.h from devices/mmc/host, all
> the  platforms based on shdci-pltfm will be able to only include:
> include/linux/mmc/sdhci.h and include/linux/sdhci-pltfm.h.
> 
> This has been tested on STM targets (STx7106, STx7108, STx5206).
> 
> Note: drivers/mmc/host/sdhci.h also includes the linux/mmc/sdhci.h
> and no modifications should be needed on other sdhci- drivers.
> 
> Signed-off-by: Giuseppe Cavallaro 

IMO this is too much exporting here. I can't see a reason to export the
sdhci_host-structure, for example. My idea would be to start with a
minimal approach and just copy over the stuff we need now (the quirks).
If we need more later, we add it seperately then.

> ---
>  drivers/mmc/host/sdhci.h  |  271 
> ++---
>  include/linux/mmc/sdhci.h |  269 
>  2 files changed, 280 insertions(+), 260 deletions(-)
>  create mode 100644 include/linux/mmc/sdhci.h
> 
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index ae28a31..3b96e4a 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -1,5 +1,7 @@
>  /*
> - *  linux/drivers/mmc/host/sdhci.h - Secure Digital Host Controller 
> Interface driver
> + *  linux/drivers/mmc/host/sdhci.h - Secure Digital Host Controller Interface
> + *
> + * Host Controller registers.
>   *
>   *  Copyright (C) 2005-2008 Pierre Ossman, All Rights Reserved.
>   *
> @@ -8,13 +10,10 @@
>   * the Free Software Foundation; either version 2 of the License, or (at
>   * your option) any later version.
>   */
> -#ifndef __SDHCI_H
> -#define __SDHCI_H
> +#ifndef __SDHCI_HW_H
> +#define __SDHCI_HW_H
>  
> -#include 
> -#include 
> -#include 
> -#include 
> +#include 
>  
>  /*
>   * Controller registers
> @@ -181,256 +180,8 @@
>  #define  SDHCI_VENDOR_VER_SHIFT  8
>  #define  SDHCI_SPEC_VER_MASK 0x00FF
>  #define  SDHCI_SPEC_VER_SHIFT0
> -#define   SDHCI_SPEC_100 0
> -#define   SDHCI_SPEC_200 1
> -#define   SDHCI_SPEC_300 2
> -
> -struct sdhci_ops;
> -
> -struct sdhci_host {
> - /* Data set by hardware interface driver */
> - const char  *hw_name;   /* Hardware bus name */
> -
> - unsigned intquirks; /* Deviations from spec. */
> -
> -/* Controller doesn't honor resets unless we touch the clock register */
> -#define SDHCI_QUIRK_CLOCK_BEFORE_RESET   (1<<0)
> -/* Controller has bad caps bits, but really supports DMA */
> -#define SDHCI_QUIRK_FORCE_DMA(1<<1)
> -/* Controller doesn't like to be reset when there is no card inserted. */
> -#define SDHCI_QUIRK_NO_CARD_NO_RESET (1<<2)
> -/* Controller doesn't like clearing the power reg before a change */
> -#define SDHCI_QUIRK_SINGLE_POWER_WRITE   (1<<3)
> -/* Controller has flaky internal state so reset it on each ios change */
> -#define SDHCI_QUIRK_RESET_CMD_DATA_ON_IOS(1<<4)
> -/* Controller has an unusable DMA engine */
> -#define SDHCI_QUIRK_BROKEN_DMA   (1<<5)
> -/* Controller has an unusable ADMA engine */
> -#define SDHCI_QUIRK_BROKEN_ADMA  (1<<6)
> -/* Controller can only DMA from 32-bit aligned addresses */
> -#define SDHCI_QUIRK_32BIT_DMA_ADDR   (1<<7)
> -/* Controller can only DMA chunk sizes that are a multiple of 32 bits */
> -#define SDHCI_QUIRK_32BIT_DMA_SIZE   (1<<8)
> -/* Controller can only ADMA chunks that are a multiple of 32 bits */
> -#define SDHCI_QUIRK_32BIT_ADMA_SIZE  (1<<9)
> -/* Controller needs to be reset after each request to stay stable */
> -#define SDHCI_QUIRK_RESET_AFTER_REQUEST  (1<<10)
> -/* Controller needs voltage and power writes to happen separately */
> -#define SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER  (1<<11)
> -/* Controller provides an incorrect timeout value for transfers */
> -#define SDHCI_QUIRK_BROKEN_TIMEOUT_VAL   (1<<12)
> -/* Controller has an issue with buffer bits for small transfers */
> -#define SDHCI_QUIRK_BROKEN_SMALL_PIO (1<<13)
> -/* Controller does not provide transfer-complete interrupt when not busy */
> -#define SDHCI_QUIRK_NO_BUSY_IRQ  (1<<14)
> -/* Controller has unreliable card detection */
> -#define SDHCI_QUIRK_BROKEN_CARD_DETECTION(1<<15)
> -/* Controller reports inverted write-protect state */
> -#define SDHCI_QUIRK_INVERTED_WRITE_PROTECT  

Re: [PATCH (mmc-next) 1/3] mmc: add suspend/resume in the sdhci-pltfm driver

[Wolfram Sang]

On Thu, Sep 23, 2010 at 11:14:24AM +0200, Giuseppe CAVALLARO wrote:
> Signed-off-by: Giuseppe Cavallaro 

I'd prefer it the way sdhci-mv.c is doing it (just one #if-block and
returning the code from the sdhci_*-functions). Then it should be fine.

> ---
>  drivers/mmc/host/sdhci-pltfm.c |   22 ++
>  1 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
> index e045e3c..89ea64b 100644
> --- a/drivers/mmc/host/sdhci-pltfm.c
> +++ b/drivers/mmc/host/sdhci-pltfm.c
> @@ -165,6 +165,24 @@ static const struct platform_device_id sdhci_pltfm_ids[] 
> = {
>  };
>  MODULE_DEVICE_TABLE(platform, sdhci_pltfm_ids);
>  
> +#ifdef CONFIG_PM
> +static int sdhci_pltfm_suspend(struct platform_device *dev, pm_message_t pm)
> +{
> + struct sdhci_host *host = platform_get_drvdata(dev);
> +
> + sdhci_suspend_host(host, pm);
> + return 0;
> +}
> +
> +static int sdhci_pltfm_resume(struct platform_device *dev)
> +{
> + struct sdhci_host *host = platform_get_drvdata(dev);
> +
> + sdhci_resume_host(host);
> + return 0;
> +}
> +#endif
> +
>  static struct platform_driver sdhci_pltfm_driver = {
>   .driver = {
>   .name   = "sdhci",
> @@ -173,6 +191,10 @@ static struct platform_driver sdhci_pltfm_driver = {
>   .probe  = sdhci_pltfm_probe,
>   .remove = __devexit_p(sdhci_pltfm_remove),
>   .id_table   = sdhci_pltfm_ids,
> +#ifdef CONFIG_PM
> + .suspend = sdhci_pltfm_suspend,
> + .resume = sdhci_pltfm_resume,
> +#endif
>  };
>  
>  
> /*\
> -- 
> 1.5.5.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature

Re: [PATCH V2 2/2] mmc: sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case

[Wolfram Sang]

On Mon, Sep 27, 2010 at 09:42:20AM +0100, Matt Fleming wrote:
> From: Jaehoon Chung 
> 
> When a controller requires SDHCI_QUIRK_BROKEN_CARD_DETECTION, we poll
> for card insertion/removal, and that creates interrupts.  There's no
> need to be doing this if we have a non-removable card.
> 
> This patch requires cards to be removable before we're willing to set
> MMC_CAP_NEEDS_POLL.
> 
> Signed-off-by: Jaehoon Chung 
> Acked-by: Kyungmin Park 
> Acked-by: Matt Fleming 
> [cjb: modified changelog and code indentation]
> Signed-off-by: Chris Ball 

Acked-by: Wolfram Sang 

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature

Re: [PATCH V2 1/2] mmc: Add helper function to check if a card is removable

[Wolfram Sang]

On Mon, Sep 27, 2010 at 09:42:19AM +0100, Matt Fleming wrote:
> There are two checks that need to be made when determining whether a
> card is removable. A host controller may set MMC_CAP_NONREMOVABLE if the
> controller does not support removing cards (e.g. eMMC), in which case
> the card is physically non-removable. Also the 'mmc_assume_removable'
> module parameter can be configured at module load time, in which case
> the card may be logically non-removable.
> 
> A helper function keeps the logic in one place so that code always
> checks both conditions.
> 
> Because this new function is likely to be called from modules we now
> need to export the mmc_assume_removable symbol.
> 
> Signed-off-by: Matt Fleming 
> Acked-by: Kyungmin Park 
> Tested-by: Jaehoon Chung 

Acked-by: Wolfram Sang 

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature

Re: [patch] mmc: fix init f_min

[David Vrabel]

zhangfei gao wrote:
> On Mon, Sep 20, 2010 at 10:34 AM, David Vrabel  wrote:
>> zhangfei gao wrote:
>>> index 5db49b1..9114c87 100644
>>> --- a/drivers/mmc/core/core.c
>>> +++ b/drivers/mmc/core/core.c
>>> @@ -910,9 +910,9 @@ static void mmc_power_up(struct mmc_host *host)
>>>   if (host->f_min > 40) {
>>>   pr_warning("%s: Minimum clock frequency too high for "
>>>   "identification mode\n", mmc_hostname(host));
>>> - host->ios.clock = host->f_min;
>>> - } else
>>>   host->ios.clock = 40;
>>> + } else
>>> + host->ios.clock = host->f_min;
>> NAK.
>>
>> The code is already correctly requesting 400 kHz (unless the controller
>> can't go that slow).
> 
> Original code
> if (host->f_min > 40) {
> pr_warning("%s: Minimum clock frequency too high for "
> "identification mode\n", mmc_hostname(host));
> host->ios.clock = host->f_min;
> } else
> host->ios.clock = 40;
> 
> With this code, the init clock is would be at lease 400 kHz, and no
> matter how bigger the host->f_min it is, is this really correct?

It's not clear what you're saying or why you think the original code is
wrong.  Consider a host controller that reports a minimum frequency of 1
Hz.  Obviously we wouldn't want to do the card identification at this
clock rate, yes?

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case

[Matt Fleming]

On Sun, Sep 26, 2010 at 03:31:22PM +0800, zhangfei gao wrote:
> On Thu, Sep 16, 2010 at 5:02 PM, Matt Fleming  wrote:
> > On Thu, Sep 16, 2010 at 03:20:08AM +0100, Chris Ball wrote:
> >> Hi Matt,
> >>
> >> On Wed, Sep 15, 2010 at 09:38:55PM +0100, Chris Ball wrote:
> >> > On Wed, Sep 15, 2010 at 04:11:42PM +0100, Matt Fleming wrote:
> >> > > Chris, are you OK to pick this up (including Jaehoon's change)? Or
> >> > > would you prefer me to resubmit?
> >> >
> >> > Thanks, that's fine, I've applied both patches to mmc-next:
> >>
> >> This (patch 2/2) doesn't resolve symbols properly, so I've dropped it:
> >>
> >>   Building modules, stage 2.
> >>   MODPOST 448 modules
> >> ERROR: "mmc_assume_removable" [drivers/mmc/host/sdhci.ko] undefined!
> >> make[1]: *** [__modpost] Error 1
> >> make: *** [modules] Error 2
> >>
> >> There's no EXPORT_SYMBOL(mmc_assume_removable) in core/core.c, yet it's
> >> being referenced in host/sdhci.c.
> >
> > Bah! That'll teach me for not compiling an allmodconfig kernel. I'll
> > fix this up and resubmit.
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> 
> Any new patch of this, then we could re-use SDHCI_QUIRK_BROKEN_CARD_DETECTION 
> :)

Thanks a lot for the reminder! I've just re-sent these two patches.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 1/2] mmc: Add helper function to check if a card is removable

[Matt Fleming]

There are two checks that need to be made when determining whether a
card is removable. A host controller may set MMC_CAP_NONREMOVABLE if the
controller does not support removing cards (e.g. eMMC), in which case
the card is physically non-removable. Also the 'mmc_assume_removable'
module parameter can be configured at module load time, in which case
the card may be logically non-removable.

A helper function keeps the logic in one place so that code always
checks both conditions.

Because this new function is likely to be called from modules we now
need to export the mmc_assume_removable symbol.

Signed-off-by: Matt Fleming 
Acked-by: Kyungmin Park 
Tested-by: Jaehoon Chung 
---
 drivers/mmc/core/core.c  |1 +
 drivers/mmc/core/core.h  |1 -
 drivers/mmc/core/mmc.c   |2 +-
 drivers/mmc/core/sd.c|2 +-
 include/linux/mmc/host.h |8 
 5 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 5db49b1..7c12612 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -58,6 +58,7 @@ int mmc_assume_removable;
 #else
 int mmc_assume_removable = 1;
 #endif
+EXPORT_SYMBOL(mmc_assume_removable);
 module_param_named(removable, mmc_assume_removable, bool, 0644);
 MODULE_PARM_DESC(
removable,
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 9d9eef5..a2ca770 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -58,7 +58,6 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr);
 
 /* Module parameters */
 extern int use_spi_crc;
-extern int mmc_assume_removable;
 
 /* Debugfs information for hosts and cards */
 void mmc_add_host_debugfs(struct mmc_host *host);
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 6909a54..6570c03 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -685,7 +685,7 @@ static void mmc_attach_bus_ops(struct mmc_host *host)
 {
const struct mmc_bus_ops *bus_ops;
 
-   if (host->caps & MMC_CAP_NONREMOVABLE || !mmc_assume_removable)
+   if (!mmc_card_is_removable(host))
bus_ops = &mmc_ops_unsafe;
else
bus_ops = &mmc_ops;
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 0f52410..bc745e1 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -750,7 +750,7 @@ static void mmc_sd_attach_bus_ops(struct mmc_host *host)
 {
const struct mmc_bus_ops *bus_ops;
 
-   if (host->caps & MMC_CAP_NONREMOVABLE || !mmc_assume_removable)
+   if (!mmc_card_is_removable(host))
bus_ops = &mmc_sd_ops_unsafe;
else
bus_ops = &mmc_sd_ops;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index ded4017..23a4864 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -267,5 +267,13 @@ static inline void mmc_set_disable_delay(struct mmc_host 
*host,
host->disable_delay = disable_delay;
 }
 
+/* Module parameter */
+extern int mmc_assume_removable;
+
+static inline int mmc_card_is_removable(struct mmc_host *host)
+{
+   return (!(host->caps & MMC_CAP_NONREMOVABLE) && mmc_assume_removable);
+}
+
 #endif
 
-- 
1.7.2.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 0/2] MMC removable helper function

[Matt Fleming]

Chris, I've rebased this mmc_assume_removable patch series against
mmc-next and fixed up the compilation breakage with allmodconfig.

Note, I've left your Signed-off-by on Jaehoon's patch because you
modified the patch slightly (and because it's going through your tree
anyway). I hope that's OK.

Jaehoon Chung (1):
  mmc: sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case

Matt Fleming (1):
  mmc: Add helper function to check if a card is removable

 drivers/mmc/core/core.c  |1 +
 drivers/mmc/core/core.h  |1 -
 drivers/mmc/core/mmc.c   |2 +-
 drivers/mmc/core/sd.c|2 +-
 drivers/mmc/host/sdhci.c |3 ++-
 include/linux/mmc/host.h |8 
 6 files changed, 13 insertions(+), 4 deletions(-)

-- 
1.7.2.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 2/2] mmc: sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case

[Matt Fleming]

From: Jaehoon Chung 

When a controller requires SDHCI_QUIRK_BROKEN_CARD_DETECTION, we poll
for card insertion/removal, and that creates interrupts.  There's no
need to be doing this if we have a non-removable card.

This patch requires cards to be removable before we're willing to set
MMC_CAP_NEEDS_POLL.

Signed-off-by: Jaehoon Chung 
Acked-by: Kyungmin Park 
Acked-by: Matt Fleming 
[cjb: modified changelog and code indentation]
Signed-off-by: Chris Ball 
---
 drivers/mmc/host/sdhci.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 5928b0a..d851315 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1847,7 +1847,8 @@ int sdhci_add_host(struct sdhci_host *host)
if (caps & SDHCI_CAN_DO_HISPD)
mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
 
-   if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
+   if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
+   mmc_card_is_removable(mmc))
mmc->caps |= MMC_CAP_NEEDS_POLL;
 
mmc->ocr_avail = 0;
-- 
1.7.2.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/4] omap4 hsmmc: Adding card detect support for MMC1

[kishore kadiyala]

Cc: Samuel Ortiz 

On Fri, Sep 24, 2010 at 10:43 PM, kishore kadiyala
 wrote:
> Adding card detect callback function and card detect configuration
> function for MMC1 Controller on OMAP4.
>
> Card detect configuration function does initial configuration of the
> MMC Control & PullUp-PullDown registers of Phoenix.
>
> For MMC1 Controller, card detect interrupt source is
> twl6030 which is non-gpio. The card detect call back function provides
> card present/absent status by reading MMC Control register present
> on twl6030.
>
> Since OMAP4 doesn't use any GPIO line as used in OMAP3 for card detect,
> the suspend/resume initialization which was done in omap_hsmmc_gpio_init
> previously is moved to the probe thus making it generic for both OMAP3 & 
> OMAP4.
>
> Cc: Tony Lindgren 
> Cc: Andrew Morton 
> Cc: Madhusudhan Chikkature 
> Cc: Adrian Hunter 
> Signed-off-by: Kishore Kadiyala 
> ---
>  arch/arm/mach-omap2/board-4430sdp.c |    7 +++-
>  drivers/mfd/twl6030-irq.c           |   73 
> +++
>  drivers/mmc/host/omap_hsmmc.c       |    4 +-
>  include/linux/i2c/twl.h             |   31 +++
>  4 files changed, 112 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-4430sdp.c 
> b/arch/arm/mach-omap2/board-4430sdp.c
> index 9447644..a49f285 100644
> --- a/arch/arm/mach-omap2/board-4430sdp.c
> +++ b/arch/arm/mach-omap2/board-4430sdp.c
> @@ -227,9 +227,14 @@ static int omap4_twl6030_hsmmc_late_init(struct device 
> *dev)
>        struct omap_mmc_platform_data *pdata = dev->platform_data;
>
>        /* Setting MMC1 Card detect Irq */
> -       if (pdev->id == 0)
> +       if (pdev->id == 0) {
> +               ret = twl6030_mmc_card_detect_config();
> +               if (ret)
> +                       pr_err("Failed configuring MMC1 card detect\n");
>                pdata->slots[0].card_detect_irq = TWL6030_IRQ_BASE +
>                                                MMCDETECT_INTR_OFFSET;
> +               pdata->slots[0].card_detect = twl6030_mmc_card_detect;
> +       }
>        return ret;
>  }
>
> diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c
> index 10bf228..2d3bb82 100644
> --- a/drivers/mfd/twl6030-irq.c
> +++ b/drivers/mfd/twl6030-irq.c
> @@ -36,6 +36,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  /*
>  * TWL6030 (unlike its predecessors, which had two level interrupt handling)
> @@ -223,6 +224,78 @@ int twl6030_interrupt_mask(u8 bit_mask, u8 offset)
>  }
>  EXPORT_SYMBOL(twl6030_interrupt_mask);
>
> +int twl6030_mmc_card_detect_config(void)
> +{
> +       int ret;
> +       u8 reg_val = 0;
> +
> +       /* Unmasking the Card detect Interrupt line for MMC1 from Phoenix */
> +       twl6030_interrupt_unmask(TWL6030_MMCDETECT_INT_MASK,
> +                                               REG_INT_MSK_LINE_B);
> +       twl6030_interrupt_unmask(TWL6030_MMCDETECT_INT_MASK,
> +                                               REG_INT_MSK_STS_B);
> +       /*
> +        * Intially Configuring MMC_CTRL for receving interrupts &
> +        * Card status on TWL6030 for MMC1
> +        */
> +       ret = twl_i2c_read_u8(TWL6030_MODULE_ID0, ®_val, TWL6030_MMCCTRL);
> +       if (ret < 0) {
> +               pr_err("twl6030: Failed to read MMCCTRL, error %d\n", ret);
> +               return ret;
> +       }
> +       reg_val &= ~VMMC_AUTO_OFF;
> +       reg_val |= SW_FC;
> +       ret = twl_i2c_write_u8(TWL6030_MODULE_ID0, reg_val, TWL6030_MMCCTRL);
> +       if (ret < 0) {
> +               pr_err("twl6030: Failed to write MMCCTRL, error %d\n", ret);
> +               return ret;
> +       }
> +
> +       /* Configuring PullUp-PullDown register */
> +       ret = twl_i2c_read_u8(TWL6030_MODULE_ID0, ®_val,
> +                                               TWL6030_CFG_INPUT_PUPD3);
> +       if (ret < 0) {
> +               pr_err("twl6030: Failed to read CFG_INPUT_PUPD3, error %d\n",
> +                                                                       ret);
> +               return ret;
> +       }
> +       reg_val &= ~(MMC_PU | MMC_PD);
> +       ret = twl_i2c_write_u8(TWL6030_MODULE_ID0, reg_val,
> +                                               TWL6030_CFG_INPUT_PUPD3);
> +       if (ret < 0) {
> +               pr_err("twl6030: Failed to write CFG_INPUT_PUPD3, error %d\n",
> +                                                                       ret);
> +               return ret;
> +       }
> +       return 0;
> +}
> +EXPORT_SYMBOL(twl6030_mmc_card_detect_config);
> +
> +int twl6030_mmc_card_detect(struct device *dev, int slot)
> +{
> +       int ret = -EIO;
> +       u8 read_reg = 0;
> +       struct platform_device *pdev = to_platform_device(dev);
> +
> +       if (pdev->id) {
> +               /* TWL6030 provide's Card detect support for
> +                * only MMC1 controller.
> +                */
> +               pr_err("Unkown MMC controller %d in %s\n", pdev->id, 
> __func__);
> +               return r

Re: [PATCH v4 1/4] omap4 hsmmc: Adding card detect support for MMC1

[kishore kadiyala]

Hi Tony,

On Sat, Sep 25, 2010 at 6:08 AM, Tony Lindgren  wrote:
> * kishore kadiyala  [100924 10:05]:
>> Adding card detect callback function and card detect configuration
>> function for MMC1 Controller on OMAP4.
>>
>> Card detect configuration function does initial configuration of the
>> MMC Control & PullUp-PullDown registers of Phoenix.
>>
>> For MMC1 Controller, card detect interrupt source is
>> twl6030 which is non-gpio. The card detect call back function provides
>> card present/absent status by reading MMC Control register present
>> on twl6030.
>>
>> Since OMAP4 doesn't use any GPIO line as used in OMAP3 for card detect,
>> the suspend/resume initialization which was done in omap_hsmmc_gpio_init
>> previously is moved to the probe thus making it generic for both OMAP3 & 
>> OMAP4.
>
>> --- a/drivers/mfd/twl6030-irq.c
>> +++ b/drivers/mfd/twl6030-irq.c
>
> Looks like this patch should be sent to Samuel Ortiz as it's mostly
> mfd related.

Thanks , I will send this patch to Samuel.



Regards,
Kishore
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] tmio_mmc: Allow 2 byte requests in 4-bit mode

[Samuel Ortiz]

On Sun, Sep 26, 2010 at 12:46:06PM +0100, Matt Fleming wrote:
> On Thu, Sep 23, 2010 at 08:11:26PM +0100, Ian Molton wrote:
> > If its not already gone through...
> > 
> > Acked-by: Ian Molton 
> 
> Cheers Ian.
> 
> I think Sam applied this patch but was waiting for your Acked-by.
Correct. I'll add Ian's Acked-by, thanks Ian.

Cheers,
Samuel.


-- 
Intel Open Source Technology Centre
http://oss.intel.com/
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html