Re: [PATCH] mmc: core: Fix PowerOff Notify suspend/resume

[Girish K S]

On 16 March 2012 10:43, Saugata Das  wrote:
> On 16 March 2012 09:19, Girish K S  wrote:
>> On 14 March 2012 20:53, Ulf Hansson  wrote:
>>> Hi Girish and Chris,
>>>
>>> I noticed that this has been pushed for 3.3, I think we need to make a
>>> revert of it asap if possible.
>>>
>>> I were unfortunately not able to review this patch earlier but it has issues
>>> I believe. It will break suspend/resume for eMMC devices supporting the
>>> SLEEP command and not the "poweroff notify" from eMMC 4.5.
>>
>> By break do you mean compilation break or system crash. can you please
>> post the log that caused the break.
>> I re checked it on eMMC 4.5, 4.41, high speed mmc card and normal mmc
>> card. I can compile successfully and
>> could test the suspend/ resume functionality without carsh.
>> If you provide the log, i can check more.
>>
>>>
>>> Please see my comments below.
>>>
>>>
>>> On 01/31/2012 11:14 AM, Girish K S wrote:

 Modified the mmc_poweroff to resume before sending the
 poweroff notification command. In sleep mode only AWAKE
 and RESET commands are allowed, so before sending the
 poweroff notification command resume from sleep mode and
 then send the notification command.

 POwerOff Notify is tested on a Synopsis Designware Host
 Controller(eMMC 4.5). The suspend to RAM and resume works fine.

 This patch is successfully applied on the Chris's mmc-next
 branch

 cc: Chris Ball
 Signed-off-by: Girish K S
 Tested-by: Girish K S
 ---
  drivers/mmc/core/core.c  |   28 
  drivers/mmc/core/mmc.c   |   17 -
  include/linux/mmc/card.h |    4 
  3 files changed, 36 insertions(+), 13 deletions(-)

 diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
 index bec0bf2..14ec575 100644
 --- a/drivers/mmc/core/core.c
 +++ b/drivers/mmc/core/core.c
 @@ -1239,7 +1239,8 @@ static void mmc_poweroff_notify(struct mmc_host
 *host)
        int err = 0;

        card = host->card;
 -
 +       mmc_claim_host(host);
 +
        /*
         * Send power notify command only if card
         * is mmc and notify state is powered ON
 @@ -1269,6 +1270,7 @@ static void mmc_poweroff_notify(struct mmc_host
 *host)
                /* Set the card state to no notification after the poweroff
 */
                card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION;
        }
 +       mmc_release_host(host);
  }

  /*
 @@ -1327,12 +1329,28 @@ static void mmc_power_up(struct mmc_host *host)

  void mmc_power_off(struct mmc_host *host)
  {
 +       int err = 0;
        mmc_host_clk_hold(host);

        host->ios.clock = 0;
        host->ios.vdd = 0;

 -       mmc_poweroff_notify(host);
 +       /*
 +        * For eMMC 4.5 device send AWAKE command before
 +        * POWER_OFF_NOTIFY command, because in sleep state
 +        * eMMC 4.5 devices respond to only RESET and AWAKE cmd
 +        */
 +       if (host->card&&  mmc_card_is_sleep(host->card)&&
 +           host->bus_ops->resume) {
 +               err = host->bus_ops->resume(host);
>>>
>>>
>>> This is just plain wrong. First we may have suspended the host from
>>> mmc_suspend_host then we enter this funtion (mmc_power_off) to cut the power
>>> to the card. Why do we even want to resume if we just did suspend?
>> for 4.5 case, cards wont respond to any command other than awake and
>> reset. so we resume before executing a switch
>> for poweroff notify. for non 4.5 card, it will resume and wont
>> continue in resume state because of the poweroff executed in the end.
>>>
>>> Moreover, this will actually mean that for mmc devices which are supporting
>>> SLEEP but not the new eMMC 4.5 feature poweroff_notify will leave this
>>> function in a resumed state (in other words, not in SLEEP state) which is
>>> not OK.
>> non 4.5 devices will not remain in resume state. Because mmc_set_ios
>> will power down the device.
>>>
>>>
 +
 +               if (!err)
 +                       mmc_poweroff_notify(host);
 +               else
 +                       pr_warning("%s: error %d during resume "
 +                                  "(continue with poweroff sequence)\n",
 +                                  mmc_hostname(host), err);
 +       }

        /*
         * Reset ocr mask to be the highest possible voltage supported for
 @@ -2386,12 +2404,6 @@ int mmc_suspend_host(struct mmc_host *host)
                 */
                if (mmc_try_claim_host(host)) {
                        if (host->bus_ops->suspend) {
 -                               /*
 -                                * For eMMC 4.5 device send notify command
 -                                * before sleep, because in sleep state
 eMMC 4.5

Re: [PATCH] mmc: core: Fix PowerOff Notify suspend/resume

[Girish K S]

On 16 March 2012 10:43, Saugata Das  wrote:
> On 16 March 2012 09:19, Girish K S  wrote:
>> On 14 March 2012 20:53, Ulf Hansson  wrote:
>>> Hi Girish and Chris,
>>>
>>> I noticed that this has been pushed for 3.3, I think we need to make a
>>> revert of it asap if possible.
>>>
>>> I were unfortunately not able to review this patch earlier but it has issues
>>> I believe. It will break suspend/resume for eMMC devices supporting the
>>> SLEEP command and not the "poweroff notify" from eMMC 4.5.
>>
>> By break do you mean compilation break or system crash. can you please
>> post the log that caused the break.
>> I re checked it on eMMC 4.5, 4.41, high speed mmc card and normal mmc
>> card. I can compile successfully and
>> could test the suspend/ resume functionality without carsh.
>> If you provide the log, i can check more.
>>
>>>
>>> Please see my comments below.
>>>
>>>
>>> On 01/31/2012 11:14 AM, Girish K S wrote:

 Modified the mmc_poweroff to resume before sending the
 poweroff notification command. In sleep mode only AWAKE
 and RESET commands are allowed, so before sending the
 poweroff notification command resume from sleep mode and
 then send the notification command.

 POwerOff Notify is tested on a Synopsis Designware Host
 Controller(eMMC 4.5). The suspend to RAM and resume works fine.

 This patch is successfully applied on the Chris's mmc-next
 branch

 cc: Chris Ball
 Signed-off-by: Girish K S
 Tested-by: Girish K S
 ---
  drivers/mmc/core/core.c  |   28 
  drivers/mmc/core/mmc.c   |   17 -
  include/linux/mmc/card.h |    4 
  3 files changed, 36 insertions(+), 13 deletions(-)

 diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
 index bec0bf2..14ec575 100644
 --- a/drivers/mmc/core/core.c
 +++ b/drivers/mmc/core/core.c
 @@ -1239,7 +1239,8 @@ static void mmc_poweroff_notify(struct mmc_host
 *host)
        int err = 0;

        card = host->card;
 -
 +       mmc_claim_host(host);
 +
        /*
         * Send power notify command only if card
         * is mmc and notify state is powered ON
 @@ -1269,6 +1270,7 @@ static void mmc_poweroff_notify(struct mmc_host
 *host)
                /* Set the card state to no notification after the poweroff
 */
                card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION;
        }
 +       mmc_release_host(host);
  }

  /*
 @@ -1327,12 +1329,28 @@ static void mmc_power_up(struct mmc_host *host)

  void mmc_power_off(struct mmc_host *host)
  {
 +       int err = 0;
        mmc_host_clk_hold(host);

        host->ios.clock = 0;
        host->ios.vdd = 0;

 -       mmc_poweroff_notify(host);
 +       /*
 +        * For eMMC 4.5 device send AWAKE command before
 +        * POWER_OFF_NOTIFY command, because in sleep state
 +        * eMMC 4.5 devices respond to only RESET and AWAKE cmd
 +        */
 +       if (host->card&&  mmc_card_is_sleep(host->card)&&
 +           host->bus_ops->resume) {
 +               err = host->bus_ops->resume(host);
>>>
>>>
>>> This is just plain wrong. First we may have suspended the host from
>>> mmc_suspend_host then we enter this funtion (mmc_power_off) to cut the power
>>> to the card. Why do we even want to resume if we just did suspend?
>> for 4.5 case, cards wont respond to any command other than awake and
>> reset. so we resume before executing a switch
>> for poweroff notify. for non 4.5 card, it will resume and wont
>> continue in resume state because of the poweroff executed in the end.
>>>
>>> Moreover, this will actually mean that for mmc devices which are supporting
>>> SLEEP but not the new eMMC 4.5 feature poweroff_notify will leave this
>>> function in a resumed state (in other words, not in SLEEP state) which is
>>> not OK.
>> non 4.5 devices will not remain in resume state. Because mmc_set_ios
>> will power down the device.
>>>
>>>
 +
 +               if (!err)
 +                       mmc_poweroff_notify(host);
 +               else
 +                       pr_warning("%s: error %d during resume "
 +                                  "(continue with poweroff sequence)\n",
 +                                  mmc_hostname(host), err);
 +       }

        /*
         * Reset ocr mask to be the highest possible voltage supported for
 @@ -2386,12 +2404,6 @@ int mmc_suspend_host(struct mmc_host *host)
                 */
                if (mmc_try_claim_host(host)) {
                        if (host->bus_ops->suspend) {
 -                               /*
 -                                * For eMMC 4.5 device send notify command
 -                                * before sleep, because in sleep state
 eMMC 4.5

Re: [PATCH RESEND 1/8] mmc: omap: Enable Auto CMD12

[S, Venkatraman]

On Thu, Mar 15, 2012 at 8:27 PM, Felipe Balbi  wrote:
> On Thu, Mar 15, 2012 at 08:03:35PM +0530, Venkatraman S wrote:
>> From: Balaji T K 
>>
>> Enable Auto-CMD12 for multi block read/write on HSMMC
>> Tested on OMAP4430, OMAP3430 and OMAP2430 SDP
>>
>> Signed-off-by: Balaji T K 
>
> BTW, since patches are flowing through you now, they should have your
> SoB line.
>

Sure. I'll edit and send again.
--
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: core: Fix PowerOff Notify suspend/resume

[Saugata Das]

On 16 March 2012 09:19, Girish K S  wrote:
> On 14 March 2012 20:53, Ulf Hansson  wrote:
>> Hi Girish and Chris,
>>
>> I noticed that this has been pushed for 3.3, I think we need to make a
>> revert of it asap if possible.
>>
>> I were unfortunately not able to review this patch earlier but it has issues
>> I believe. It will break suspend/resume for eMMC devices supporting the
>> SLEEP command and not the "poweroff notify" from eMMC 4.5.
>
> By break do you mean compilation break or system crash. can you please
> post the log that caused the break.
> I re checked it on eMMC 4.5, 4.41, high speed mmc card and normal mmc
> card. I can compile successfully and
> could test the suspend/ resume functionality without carsh.
> If you provide the log, i can check more.
>
>>
>> Please see my comments below.
>>
>>
>> On 01/31/2012 11:14 AM, Girish K S wrote:
>>>
>>> Modified the mmc_poweroff to resume before sending the
>>> poweroff notification command. In sleep mode only AWAKE
>>> and RESET commands are allowed, so before sending the
>>> poweroff notification command resume from sleep mode and
>>> then send the notification command.
>>>
>>> POwerOff Notify is tested on a Synopsis Designware Host
>>> Controller(eMMC 4.5). The suspend to RAM and resume works fine.
>>>
>>> This patch is successfully applied on the Chris's mmc-next
>>> branch
>>>
>>> cc: Chris Ball
>>> Signed-off-by: Girish K S
>>> Tested-by: Girish K S
>>> ---
>>>  drivers/mmc/core/core.c  |   28 
>>>  drivers/mmc/core/mmc.c   |   17 -
>>>  include/linux/mmc/card.h |    4 
>>>  3 files changed, 36 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>> index bec0bf2..14ec575 100644
>>> --- a/drivers/mmc/core/core.c
>>> +++ b/drivers/mmc/core/core.c
>>> @@ -1239,7 +1239,8 @@ static void mmc_poweroff_notify(struct mmc_host
>>> *host)
>>>        int err = 0;
>>>
>>>        card = host->card;
>>> -
>>> +       mmc_claim_host(host);
>>> +
>>>        /*
>>>         * Send power notify command only if card
>>>         * is mmc and notify state is powered ON
>>> @@ -1269,6 +1270,7 @@ static void mmc_poweroff_notify(struct mmc_host
>>> *host)
>>>                /* Set the card state to no notification after the poweroff
>>> */
>>>                card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION;
>>>        }
>>> +       mmc_release_host(host);
>>>  }
>>>
>>>  /*
>>> @@ -1327,12 +1329,28 @@ static void mmc_power_up(struct mmc_host *host)
>>>
>>>  void mmc_power_off(struct mmc_host *host)
>>>  {
>>> +       int err = 0;
>>>        mmc_host_clk_hold(host);
>>>
>>>        host->ios.clock = 0;
>>>        host->ios.vdd = 0;
>>>
>>> -       mmc_poweroff_notify(host);
>>> +       /*
>>> +        * For eMMC 4.5 device send AWAKE command before
>>> +        * POWER_OFF_NOTIFY command, because in sleep state
>>> +        * eMMC 4.5 devices respond to only RESET and AWAKE cmd
>>> +        */
>>> +       if (host->card&&  mmc_card_is_sleep(host->card)&&
>>> +           host->bus_ops->resume) {
>>> +               err = host->bus_ops->resume(host);
>>
>>
>> This is just plain wrong. First we may have suspended the host from
>> mmc_suspend_host then we enter this funtion (mmc_power_off) to cut the power
>> to the card. Why do we even want to resume if we just did suspend?
> for 4.5 case, cards wont respond to any command other than awake and
> reset. so we resume before executing a switch
> for poweroff notify. for non 4.5 card, it will resume and wont
> continue in resume state because of the poweroff executed in the end.
>>
>> Moreover, this will actually mean that for mmc devices which are supporting
>> SLEEP but not the new eMMC 4.5 feature poweroff_notify will leave this
>> function in a resumed state (in other words, not in SLEEP state) which is
>> not OK.
> non 4.5 devices will not remain in resume state. Because mmc_set_ios
> will power down the device.
>>
>>
>>> +
>>> +               if (!err)
>>> +                       mmc_poweroff_notify(host);
>>> +               else
>>> +                       pr_warning("%s: error %d during resume "
>>> +                                  "(continue with poweroff sequence)\n",
>>> +                                  mmc_hostname(host), err);
>>> +       }
>>>
>>>        /*
>>>         * Reset ocr mask to be the highest possible voltage supported for
>>> @@ -2386,12 +2404,6 @@ int mmc_suspend_host(struct mmc_host *host)
>>>                 */
>>>                if (mmc_try_claim_host(host)) {
>>>                        if (host->bus_ops->suspend) {
>>> -                               /*
>>> -                                * For eMMC 4.5 device send notify command
>>> -                                * before sleep, because in sleep state
>>> eMMC 4.5
>>> -                                * devices respond to only RESET and AWAKE
>>> cmd
>>> -                                */
>>> -                               mmc_poweroff

Re: [PATCH] mmc: core: Fix PowerOff Notify suspend/resume

[Girish K S]

On 14 March 2012 20:53, Ulf Hansson  wrote:
> Hi Girish and Chris,
>
> I noticed that this has been pushed for 3.3, I think we need to make a
> revert of it asap if possible.
>
> I were unfortunately not able to review this patch earlier but it has issues
> I believe. It will break suspend/resume for eMMC devices supporting the
> SLEEP command and not the "poweroff notify" from eMMC 4.5.

By break do you mean compilation break or system crash. can you please
post the log that caused the break.
I re checked it on eMMC 4.5, 4.41, high speed mmc card and normal mmc
card. I can compile successfully and
could test the suspend/ resume functionality without carsh.
If you provide the log, i can check more.

>
> Please see my comments below.
>
>
> On 01/31/2012 11:14 AM, Girish K S wrote:
>>
>> Modified the mmc_poweroff to resume before sending the
>> poweroff notification command. In sleep mode only AWAKE
>> and RESET commands are allowed, so before sending the
>> poweroff notification command resume from sleep mode and
>> then send the notification command.
>>
>> POwerOff Notify is tested on a Synopsis Designware Host
>> Controller(eMMC 4.5). The suspend to RAM and resume works fine.
>>
>> This patch is successfully applied on the Chris's mmc-next
>> branch
>>
>> cc: Chris Ball
>> Signed-off-by: Girish K S
>> Tested-by: Girish K S
>> ---
>>  drivers/mmc/core/core.c  |   28 
>>  drivers/mmc/core/mmc.c   |   17 -
>>  include/linux/mmc/card.h |    4 
>>  3 files changed, 36 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index bec0bf2..14ec575 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -1239,7 +1239,8 @@ static void mmc_poweroff_notify(struct mmc_host
>> *host)
>>        int err = 0;
>>
>>        card = host->card;
>> -
>> +       mmc_claim_host(host);
>> +
>>        /*
>>         * Send power notify command only if card
>>         * is mmc and notify state is powered ON
>> @@ -1269,6 +1270,7 @@ static void mmc_poweroff_notify(struct mmc_host
>> *host)
>>                /* Set the card state to no notification after the poweroff
>> */
>>                card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION;
>>        }
>> +       mmc_release_host(host);
>>  }
>>
>>  /*
>> @@ -1327,12 +1329,28 @@ static void mmc_power_up(struct mmc_host *host)
>>
>>  void mmc_power_off(struct mmc_host *host)
>>  {
>> +       int err = 0;
>>        mmc_host_clk_hold(host);
>>
>>        host->ios.clock = 0;
>>        host->ios.vdd = 0;
>>
>> -       mmc_poweroff_notify(host);
>> +       /*
>> +        * For eMMC 4.5 device send AWAKE command before
>> +        * POWER_OFF_NOTIFY command, because in sleep state
>> +        * eMMC 4.5 devices respond to only RESET and AWAKE cmd
>> +        */
>> +       if (host->card&&  mmc_card_is_sleep(host->card)&&
>> +           host->bus_ops->resume) {
>> +               err = host->bus_ops->resume(host);
>
>
> This is just plain wrong. First we may have suspended the host from
> mmc_suspend_host then we enter this funtion (mmc_power_off) to cut the power
> to the card. Why do we even want to resume if we just did suspend?
for 4.5 case, cards wont respond to any command other than awake and
reset. so we resume before executing a switch
for poweroff notify. for non 4.5 card, it will resume and wont
continue in resume state because of the poweroff executed in the end.
>
> Moreover, this will actually mean that for mmc devices which are supporting
> SLEEP but not the new eMMC 4.5 feature poweroff_notify will leave this
> function in a resumed state (in other words, not in SLEEP state) which is
> not OK.
non 4.5 devices will not remain in resume state. Because mmc_set_ios
will power down the device.
>
>
>> +
>> +               if (!err)
>> +                       mmc_poweroff_notify(host);
>> +               else
>> +                       pr_warning("%s: error %d during resume "
>> +                                  "(continue with poweroff sequence)\n",
>> +                                  mmc_hostname(host), err);
>> +       }
>>
>>        /*
>>         * Reset ocr mask to be the highest possible voltage supported for
>> @@ -2386,12 +2404,6 @@ int mmc_suspend_host(struct mmc_host *host)
>>                 */
>>                if (mmc_try_claim_host(host)) {
>>                        if (host->bus_ops->suspend) {
>> -                               /*
>> -                                * For eMMC 4.5 device send notify command
>> -                                * before sleep, because in sleep state
>> eMMC 4.5
>> -                                * devices respond to only RESET and AWAKE
>> cmd
>> -                                */
>> -                               mmc_poweroff_notify(host);
>>                                err = host->bus_ops->suspend(host);
>>                        }
>>                        mmc_do_release_host(host);
>> diff --git a/dri

Re: [PATCH 1/3] mmc: sdhci-pci: Add MSI support

[Chris Ball]

Hi Alexander,

On Tue, Mar 13 2012, Alexander Stein wrote:
> Signed-off-by: Alexander Stein 
> ---
>  drivers/mmc/host/sdhci-pci.c |6 ++
>  1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
> index 7165e6a..9382f27 100644
> --- a/drivers/mmc/host/sdhci-pci.c
> +++ b/drivers/mmc/host/sdhci-pci.c
> @@ -1379,6 +1379,8 @@ static int __devinit sdhci_pci_probe(struct pci_dev 
> *pdev,
>  
>   slots = chip->num_slots;/* Quirk may have changed this */
>  
> + pci_enable_msi(pdev);
> +
>   for (i = 0; i < slots; i++) {
>   slot = sdhci_pci_probe_slot(pdev, chip, first_bar, i);
>   if (IS_ERR(slot)) {
> @@ -1397,6 +1399,8 @@ static int __devinit sdhci_pci_probe(struct pci_dev 
> *pdev,
>   return 0;
>  
>  free:
> + pci_disable_msi(pdev);
> +
>   pci_set_drvdata(pdev, NULL);
>   kfree(chip);
>  
> @@ -1419,6 +1423,8 @@ static void __devexit sdhci_pci_remove(struct pci_dev 
> *pdev)
>   for (i = 0; i < chip->num_slots; i++)
>   sdhci_pci_remove_slot(chip->slots[i]);
>  
> + pci_disable_msi(pdev);
> +
>   pci_set_drvdata(pdev, NULL);
>   kfree(chip);
>   }

Thanks, I've merged all three patches to mmc-next for 3.4, using v2 of
patches 2 and 3, and adding Adrian's ACK to patch 2.

- 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] mmc: core: add the warning message when card didn't support HPI

[Chris Ball]

Hi,

On Sun, Mar 11 2012, Jaehoon Chung wrote:
> Though card didn't support HPI,someone could use the send_hpi_cmd().
> Then maybe didn't work fine.
> Because card->ext_csd.hpi_cmd didn't set.
> So if card didn't support hpi, return the waring message.
>
> And CMD12's flags is MMC_RSP_R1B.
>
> Signed-off-by: Jaehoon Chung 
> Signed-off-by: Kyungmin Park 
> ---
> Changelog v2:
>   - Fixed typo and removed the flags variable.
>
>  drivers/mmc/core/mmc_ops.c |   12 
>  1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 4d41fa9..30f45be 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -553,18 +553,22 @@ int mmc_send_hpi_cmd(struct mmc_card *card, u32 *status)
>  {
>   struct mmc_command cmd = {0};
>   unsigned int opcode;
> - unsigned int flags;
>   int err;
>  
> + if (!card->ext_csd.hpi) {
> + pr_warning("%s: Card didn't support HPI command\n",
> + mmc_hostname(card->host));
> + return -EINVAL;
> + }
> +
>   opcode = card->ext_csd.hpi_cmd;
>   if (opcode == MMC_STOP_TRANSMISSION)
> - flags = MMC_RSP_R1 | MMC_CMD_AC;
> + cmd.flags = MMC_RSP_R1B | MMC_CMD_AC;
>   else if (opcode == MMC_SEND_STATUS)
> - flags = MMC_RSP_R1 | MMC_CMD_AC;
> + cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
>  
>   cmd.opcode = opcode;
>   cmd.arg = card->rca << 16 | 1;
> - cmd.flags = flags;
>   cmd.cmd_timeout_ms = card->ext_csd.out_of_int_time;
>  
>   err = mmc_wait_for_cmd(card->host, &cmd, 0);

Thanks, pushed to mmc-next for 3.4.

- 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 REPOST 1/2] mmc: davinci: Eliminate spurious interrupts

[Chris Ball]

Hi,

On Sun, Mar 11 2012, Ido Yariv wrote:
> The davinci mmc interrupt handler fills the fifo, as long as the DXRDY
> or DRRDY bits are set in the status register.
>
> If interrupts fire during this loop, they will be handled by the
> handler, but the interrupt controller will still buffer these. As a
> result, the handler will be called again to serve these needlessly. In
> order to avoid these spurious interrupts, keep interrupts masked while
> filling the fifo.
>
> Signed-off-by: Ido Yariv 
> Tested-by: Rajashekhara, Sudhakar 

Thanks, I've pushed both of these two patches to mmc-next for 3.4 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] drivers: mmc: sdhci-s3c: fix broken compilation for non-Exynos SoCs

[Chris Ball]

Hi Marek,

On Fri, Mar 09 2012, Marek Szyprowski wrote:
> exynos4_sdhci_drv_data structure is not available on non-Exynos builds,
> that's why EXYNOS4_SDHCI_DRV_DATA macro has been introduced. This patch
> fixes commit 67819656 'mmc: sdhci-s3c: Add device tree support' to use
> that macro. This fixes broken build for pre-Exynos SoCs.
>
> Signed-off-by: Marek Szyprowski 
> ---
>  drivers/mmc/host/sdhci-s3c.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index ea0767e..984601e 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -859,7 +859,7 @@ MODULE_DEVICE_TABLE(platform, sdhci_s3c_driver_ids);
>  static const struct of_device_id sdhci_s3c_dt_match[] = {
>   { .compatible = "samsung,s3c6410-sdhci", },
>   { .compatible = "samsung,exynos4210-sdhci",
> - .data = &exynos4_sdhci_drv_data },
> + .data = EXYNOS4_SDHCI_DRV_DATA },
>   {},
>  };
>  MODULE_DEVICE_TABLE(of, sdhci_s3c_dt_match);

Thanks, pushed to mmc-next for 3.4 with Jaehoon'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] mmc: omap_hsmmc: set dto to 14 for all devices

[Chris Ball]

Hi Paul,

On Mon, Mar 12 2012, Paul Walmsley wrote:
> I don't think this is the right fix.  Steve Sakoman and I discussed this a 
> few months ago -- we did some debugging and Steve observed the timeouts on 
> multiple block writes (0x19):
> [...]
> So perhaps something like the following patch instead?  Unfortunately, I 
> don't have one of these crappy SD cards as far as I know, so I can't 
> really test it.
>
>
> - Paul
>
> From: Paul Walmsley 
> Date: Mon, 12 Mar 2012 04:46:29 -0600
> Subject: [PATCH] mmc: use really long write timeout to deal with crappy cards
>
> Several people have noticed that crappy SD cards take much longer to
> complete multiple block writes than the 300ms that Linux specifies.
> Try to work around this by using a three second write timeout instead.
>
> 
>
> Not-yet-signed-off-by: Paul Walmsley 

Sounds like a good idea -- want to send a signed-off patch for this?

Thanks,

- 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] mmc: sh_mobile_sdhi: support modular mmc-core with non-standard hotplug

[Chris Ball]

Hi Guennadi,

On Fri, Mar 09 2012, Guennadi Liakhovetski wrote:
> Currently if a platform wants to implement a non-standard card-detection
> method, it would need to call tmio_mmc_cd_wakeup(), which is an inline
> function, calling mmc_detect_change(). For this the platform would have
> to link mmc_core statically into the kernel, losing the ability to build
> it as a module. This patch adds a callback to the sh_mobile_sdhi driver,
> which eliminates this dependency.
>
> Signed-off-by: Guennadi Liakhovetski 
> ---
>
> v2:
>
> 1. remove a call to tmio_mmc_cd_wakeup(), call mmc_detect_change() 
>directly instead
> 2. document the new struct sh_mobile_sdhi_ops
>
>  drivers/mmc/host/sh_mobile_sdhi.c  |   11 ++-
>  include/linux/mmc/sh_mobile_sdhi.h |   11 ++-
>  2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c 
> b/drivers/mmc/host/sh_mobile_sdhi.c
> index dd288d6..8352054 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -100,6 +100,15 @@ static int sh_mobile_sdhi_write16_hook(struct 
> tmio_mmc_host *host, int addr)
>   return 0;
>  }
>  
> +static void sh_mobile_sdhi_cd_wakeup(const struct platform_device *pdev)
> +{
> + mmc_detect_change(dev_get_drvdata(&pdev->dev), msecs_to_jiffies(100));
> +}
> +
> +static const struct sh_mobile_sdhi_ops sdhi_ops = {
> + .cd_wakeup = sh_mobile_sdhi_cd_wakeup,
> +};
> +
>  static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
>  {
>   struct sh_mobile_sdhi *priv;
> @@ -121,7 +130,7 @@ static int __devinit sh_mobile_sdhi_probe(struct 
> platform_device *pdev)
>   p->pdata = mmc_data;
>  
>   if (p->init) {
> - ret = p->init(pdev);
> + ret = p->init(pdev, &sdhi_ops);
>   if (ret)
>   goto einit;
>   }

This patch doesn't apply because the "if (p->init)" conditional isn't in
mmc-next.  Maybe you're depending on a patch that I haven't taken yet?

> diff --git a/include/linux/mmc/sh_mobile_sdhi.h 
> b/include/linux/mmc/sh_mobile_sdhi.h
> index 686b85b..e94e620 100644
> --- a/include/linux/mmc/sh_mobile_sdhi.h
> +++ b/include/linux/mmc/sh_mobile_sdhi.h
> @@ -10,6 +10,14 @@ struct tmio_mmc_data;
>  #define SH_MOBILE_SDHI_IRQ_SDCARD"sdcard"
>  #define SH_MOBILE_SDHI_IRQ_SDIO  "sdio"
>  
> +/**
> + * struct sh_mobile_sdhi_ops - SDHI driver callbacks
> + * @cd_wakeup:   trigger a card-detection run
> + */
> +struct sh_mobile_sdhi_ops {
> + void (*cd_wakeup)(const struct platform_device *pdev);
> +};
> +
>  struct sh_mobile_sdhi_info {
>   int dma_slave_tx;
>   int dma_slave_rx;
> @@ -22,7 +30,8 @@ struct sh_mobile_sdhi_info {
>   int (*get_cd)(struct platform_device *pdev);
>  
>   /* callbacks for board specific setup code */
> - int (*init)(struct platform_device *pdev);
> + int (*init)(struct platform_device *pdev,
> + const struct sh_mobile_sdhi_ops *ops);
>   void (*cleanup)(struct platform_device *pdev);
>  };

- 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 v1 1/5] ARM: imx28: add basic dt support

[Dong Aisheng]

On Thu, Mar 15, 2012 at 07:24:19PM +0800, s.ha...@pengutronix.de wrote:
> On Thu, Mar 15, 2012 at 06:59:28PM +0800, Dong Aisheng wrote:
> > On Thu, Mar 15, 2012 at 02:53:29PM +0800, Lothar Waßmann wrote:
> > > Hi,
> > > 
> > > Dong Aisheng writes:
> > > > On Wed, Mar 14, 2012 at 10:16:43PM +0800, s.ha...@pengutronix.de wrote:
> > > > > On Wed, Mar 14, 2012 at 08:45:23PM +0800, Dong Aisheng wrote:
> > > > > > On Wed, Mar 14, 2012 at 01:23:51AM +0800, Grant Likely wrote:
> > > [...]
> > > > > > But it seems this needs pass mac address to fec driver via 
> > > > > > platforom data which is
> > > > > > not friendly to dt.
> > > > > > 
> > > > > > Another way may be changing fec driver to get the fixed part of mac 
> > > > > > address(first
> > > > > > two bytes) from device tree and read the left dynamical part from 
> > > > > > otp which i'm not
> > > > > > sure is better enough.
> > > > > > 
> > > > > > BTW, filling with zeros seems not work since it's invalid mac 
> > > > > > address.
> > > > > 
> > > > > Yes, that's the idea of using this value...
> > > > > 
> > > > But comparing to use an invalid value, why not just do not define that
> > > > local-mac-address property?
> > > > 
> > > Because a MAC address is by definition a GLOBALLY UNIQUE IDENTIFIER
> > > which is contrary to coding a "valid" default value for it somewhere!
> > > The owner of a certain MAC address range (OUI) is responsible for
> > > ensuring the uniqueness. Thus only they may assign a specific MAC
> > > address to a specific device.
> > > 
> > Yes, you're correct.
> > So i propose to read the MAC address from MX28 OTP in fec driver instead of
> > define it in device tree in my last mail.
> > http://www.spinics.net/lists/arm-kernel/msg165040.html
> > Do you have any comment on that way?
> 
> Yes, you could do that. Otherwise it's perfectly fine to pass the MAC
> address in the device tree, it's just not ok to put specific values
> into the kernel source. You could leave the values blank and let the
Yes, correct.

> bootloader fill them in. Or you could fill in valid values before you
> compile the devicetree, but be prepared for surprises once you have
> two boards in your network and use the devicetree for both.
> 

Thanks for the info.
I will submit that patch for discussion.

Regards
Dong Aisheng

--
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 v1 1/5] ARM: imx28: add basic dt support

[Dong Aisheng]

On Thu, Mar 15, 2012 at 07:22:04PM +0800, Lothar Waßmann wrote:
> Hi,
> 
> Dong Aisheng writes:
> > On Thu, Mar 15, 2012 at 02:53:29PM +0800, Lothar Waßmann wrote:
> > > Hi,
> > > 
> > > Dong Aisheng writes:
> > > > On Wed, Mar 14, 2012 at 10:16:43PM +0800, s.ha...@pengutronix.de wrote:
> > > > > On Wed, Mar 14, 2012 at 08:45:23PM +0800, Dong Aisheng wrote:
> > > > > > On Wed, Mar 14, 2012 at 01:23:51AM +0800, Grant Likely wrote:
> > > [...]
> > > > > > But it seems this needs pass mac address to fec driver via 
> > > > > > platforom data which is
> > > > > > not friendly to dt.
> > > > > > 
> > > > > > Another way may be changing fec driver to get the fixed part of mac 
> > > > > > address(first
> > > > > > two bytes) from device tree and read the left dynamical part from 
> > > > > > otp which i'm not
> > > > > > sure is better enough.
> > > > > > 
> > > > > > BTW, filling with zeros seems not work since it's invalid mac 
> > > > > > address.
> > > > > 
> > > > > Yes, that's the idea of using this value...
> > > > > 
> > > > But comparing to use an invalid value, why not just do not define that
> > > > local-mac-address property?
> > > > 
> > > Because a MAC address is by definition a GLOBALLY UNIQUE IDENTIFIER
> > > which is contrary to coding a "valid" default value for it somewhere!
> > > The owner of a certain MAC address range (OUI) is responsible for
> > > ensuring the uniqueness. Thus only they may assign a specific MAC
> > > address to a specific device.
> > > 
> > Yes, you're correct.
> > So i propose to read the MAC address from MX28 OTP in fec driver instead of
> > define it in device tree in my last mail.
> > http://www.spinics.net/lists/arm-kernel/msg165040.html
> > Do you have any comment on that way?
> > 
> That patch sets the OUI to the Freescale owned MAC range. Thus only
> Freescale products may be able to use the driver.
> 
No.
My proposal is only set the fixed part(first two octets) in board dts file,
each board knows it's value, and read the left 4 octets from OCOTP dynamically.
Obviously the freeescale board dts file is only for FSL platforms.
Other platforms can define their fix part of MAC address in their dts file.

> Anyway there is no definite spec how the MAC address(es) are stored
> in the fuse map. Thus reading the MAC from there is more or less
> platform specific.
> 
It's just provide one more option since there are customers storing the MAC
in the fuse map.

> Currently I'm setting up the MAC address for our TX28 from the fuses
> in the platform code passed via platform_data, but that will obviously
> not work with DT.
> 
Non-dt can also use my proposal, then you only need to pass the fixed part of
MAC via platfrom data, the left will be read by fec driver.

> The correct way would probably be to pass the MAC from the bootloader
> via a DT blob. But that would require all bootloaders to be updated
> first to support DTS. :(
> 
But uboot will face the same problem that can not define a valid MAC
in dts, did i undertand correctly?

> An intermediate solution could be using OF_DEV_AUXDATA to pass a
> platform_data struct that may contain a MAC address set up by the
> platform code.
> 
Yes, it's a way but i'm afraid will not get accepted by dt people.

Regards
Dong Aisheng

--
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 0/5] MMC: mmci: Provide bindings for Device Tree

[Arnd Bergmann]

On Thursday 15 March 2012, Per Forlin wrote:
> On Thu, Mar 15, 2012 at 4:32 PM, Arnd Bergmann  wrote:
> > On Thursday 15 March 2012, Lee Jones wrote:
> >> > I would like to see what the minimal required change is to support DT
> >> > for mmci without factorization.
> >> > 1. Minimal change in mmci.
> >> > 2. Add mmci_dt.c which contains the DT-populate code.
> >> >
> >> > The factorization could be done as step 2 I think.
> >> >
> >> > What do you say?
> >>
> >> I'm wondering what the difference is as the work has already been done.
> >>
> >> It was Arnd's suggestion to separate out the two types of variants, and
> >> I'm quite fond of the new (fully featured) layout.
> >
> > Right, I usually prefer cleanups or other refactoring to be done first, and
> > then features added on top.
> >
> > You could in theory add have just patches 3/4/5 all applied without
> > the refactoring, but that I would be worried that this causes dependencies
> > between the mmci driver and ux500 specific functionality like the
> > stedma40_filter function.
> >
> About DT
> I think I need to catch up on the DT-design a bit. I will try to catch
> up on the DT-implementation and maybe then the refactorization will
> make sense to me :)
> 
> Board specific data in mmci-driver
> The stedma40 filter function is not really specific for ux500. ux500
> use stedma40 but it should be possible to replace this DMA.IP with
> some other DMA-controller. This is board specific configuration. You
> should not need to change the mmci-driver just because the dma-driver
> has changed, right?
> Or will the board-configuration now be placed in mmci-ux500?

Right, the DMA configuration does not really belong in there, but
the voltage setup might (unless we convert that to the regulator
setup).

> Common DT populate code for all mmc host drivers
> Some parts of the DT-population is common for all the host driver, for
> instance setting the CAP and CAP2. This code should be moved to a
> common place to be used by other host drivers as well.

Good idea, yes. We should also have a generic mmc binding document
that describes how to set flags like 8-bit mode. We already have
two conflicting variants in .dts files and we should make sure
we bring everybody back together here.

> About refactoring
> There are numbers of patches stashed up for MMCI waiting for verdict
> here 
> http://git.kernel.org/?p=linux/kernel/git/linusw/linux-stericsson.git;a=shortlog;h=refs/heads/mmci.
> If doing a refactorization please rebase it on top of these patches.
> This needs to be done carefully, there may be more to considerations
> than just DT. Maybe the timing is better to do this for 4.5, just
> after 4.4 is closed. Then we can make sure that all new patches are
> made on top of the refactorized base.

Yes, thanks for the info. Doing this for 3.4 is definitely out of the
question then, and hopefully we can use the new dma bindings when it's
done for 3.5

Arnd
--
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 0/5] MMC: mmci: Provide bindings for Device Tree

[Russell King - ARM Linux]

On Thu, Mar 15, 2012 at 08:12:49PM +0100, Per Forlin wrote:
> About refactoring
> There are numbers of patches stashed up for MMCI waiting for verdict
> here 
> http://git.kernel.org/?p=linux/kernel/git/linusw/linux-stericsson.git;a=shortlog;h=refs/heads/mmci.
> If doing a refactorization please rebase it on top of these patches.

Quite a number of these are in linux-next via my tree:

http://ftp.arm.linux.org.uk/git/?p=linux-next.git;a=shortlog;h=7437cfa532842ce75189826742bddf1ba137f58e

There's 10 there, out of the 17 in Linus' tree.  See my previous replies
on why that's the case.
--
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/5] MMC: mmci: Seperate ARM variants from generic code

[Russell King - ARM Linux]

On Thu, Mar 15, 2012 at 06:23:32PM +, Arnd Bergmann wrote:
> On Thursday 15 March 2012, Russell King - ARM Linux wrote:
> > On Thu, Mar 15, 2012 at 05:46:56PM +, Arnd Bergmann wrote:
> > > Can you elaborate? I suggested the split in order to keep the ux500
> > > specific parts local to one file. With the device tree conversion,
> > > we really want to have them out of the platform code, but sticking them
> > > into the main driver seems wrong, too.
> > 
> > You're talking about the data structures which describe what quirks to
> > apply for the ux500 parts, rather than the actual quirks themselves.
> > That's not a particularly clever thing to do because it separates out
> > the selection of the works from the rest of the driver, which means
> > simply searching for the flags to find out what's applied for what has
> > to span several files.
> 
> Right, that is a disadvantage, but it's also how a lot of other
> drivers work, in particular sdhci.
> 
> My reasoning is roughly this:
> 
> * we want to get rid of arch/arm/mach-ux500/board-mop500-sdi.c
> * the sdi0_configure() and mop500_sdi0_vdd_handler() functions
>   need to be moved somewhere in order to do that, so they should
>   be with the driver.

I don't believe they should.  MMCI _is_ the peripheral itself.  All the
stuff that's in board-mop500-sdi.c seems to be doing is configuration
type stuff for the SoC that its been integrated into.  Are you seriously
advocating having multiple drivers for essentially the same hardware?

> * we don't want them in mmci.c, so we create a new file for these.
> * in order to call do the setup from sdi0_configure right, it needs
>   to be done in the probe() function

I've always disagreed with this whole approach of "drivers should callback
into generic code to claim their GPIOs".  If the GPIO is dedicated by a
platform to a function, then that function is there independent of whether
the driver is present or not.  In many cases, it is totally _wrong_ to
leave a signal which should be driven unconfigured - if the board design
is such that its intended to be an output and there's no pull-up, it
could end up floating at half-supply which will eat power.

I have always been of the opinion that resources like GPIOs which are
under platform control should be requested and configured by the platform
and not by their respective drivers.

Have you noticed how much of the configuration is tweaked depending on
which init function is called?

> * if we want to have a separate probe function, we also need to
>   have a separate amba_driver structure
> * the variant_data in mmci.c belongs with the amba_id, so that
>   also gets moved to the ux500 file.
> 
> If you have a better solution for one or more of these, I'm
> all ears.

Are you really advocating having mmci_foo.c where foo is an ever
increasing number of SoCs with the MMCI integrated because you want
to integrate SoC specific details into the MMCI peripheral driver?

> The alternatives that I can see are:
> a) keep using auxdata to supply a platform_data pointer and
>do everything in the main driver. Problem: we want to
>avoid auxdata if possible
> b) move the code from patch 4 into mmci.c using #ifdef.
>Problem: it's ugly code that has nothing to do with mmci
>in general.
> c) use the regulator framework to do the voltage selection
>here, and have only generic code in mmci.c. This may
>be the best solution, but I have no idea if this is
>actually possible, or how to do it.

Let's ignore the DMA stuff for the time being - that's a separate problem
which needs to be addressed once we work out how to deal with how DMA
engine stuff gets resolved with DT based systems.

The remainder of it (mmci_platform_data) looks like it can be easily
represented as DT properties, the exception to that is the ios_handler.
I think the right solution for that is to move to the regulator framework,
but allow the driver to continue having the ios callback if there is
no attached regulator.  It needs that because of the four integrated
regulator control bits (which are entirely undefined.)

We _do_ need a solution for GPIO-controlled voltage regulators, as these
are actually quite common - see the various soc-common using PCMCIA
drivers.

So I think actually a bit more thought is required here, and there's a
need to use some of the facilities such as regulator which are now present
that weren't when the driver was originally written some 9 years ago.
--
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/5] MMC: mmci: Seperate ARM variants from generic code

[Arnd Bergmann]

On Thursday 15 March 2012, Russell King - ARM Linux wrote:
> On Thu, Mar 15, 2012 at 05:46:56PM +, Arnd Bergmann wrote:
> > Can you elaborate? I suggested the split in order to keep the ux500
> > specific parts local to one file. With the device tree conversion,
> > we really want to have them out of the platform code, but sticking them
> > into the main driver seems wrong, too.
> 
> You're talking about the data structures which describe what quirks to
> apply for the ux500 parts, rather than the actual quirks themselves.
> That's not a particularly clever thing to do because it separates out
> the selection of the works from the rest of the driver, which means
> simply searching for the flags to find out what's applied for what has
> to span several files.

Right, that is a disadvantage, but it's also how a lot of other
drivers work, in particular sdhci.

My reasoning is roughly this:

* we want to get rid of arch/arm/mach-ux500/board-mop500-sdi.c
* the sdi0_configure() and mop500_sdi0_vdd_handler() functions
  need to be moved somewhere in order to do that, so they should
  be with the driver.
* we don't want them in mmci.c, so we create a new file for these.
* in order to call do the setup from sdi0_configure right, it needs
  to be done in the probe() function
* if we want to have a separate probe function, we also need to
  have a separate amba_driver structure
* the variant_data in mmci.c belongs with the amba_id, so that
  also gets moved to the ux500 file.

If you have a better solution for one or more of these, I'm
all ears.

The alternatives that I can see are:
a) keep using auxdata to supply a platform_data pointer and
   do everything in the main driver. Problem: we want to
   avoid auxdata if possible
b) move the code from patch 4 into mmci.c using #ifdef.
   Problem: it's ugly code that has nothing to do with mmci
   in general.
c) use the regulator framework to do the voltage selection
   here, and have only generic code in mmci.c. This may
   be the best solution, but I have no idea if this is
   actually possible, or how to do it.

arnd

--
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 0/5] MMC: mmci: Provide bindings for Device Tree

[Per Forlin]

On Thu, Mar 15, 2012 at 4:32 PM, Arnd Bergmann  wrote:
> On Thursday 15 March 2012, Lee Jones wrote:
>> > I would like to see what the minimal required change is to support DT
>> > for mmci without factorization.
>> > 1. Minimal change in mmci.
>> > 2. Add mmci_dt.c which contains the DT-populate code.
>> >
>> > The factorization could be done as step 2 I think.
>> >
>> > What do you say?
>>
>> I'm wondering what the difference is as the work has already been done.
>>
>> It was Arnd's suggestion to separate out the two types of variants, and
>> I'm quite fond of the new (fully featured) layout.
>
> Right, I usually prefer cleanups or other refactoring to be done first, and
> then features added on top.
>
> You could in theory add have just patches 3/4/5 all applied without
> the refactoring, but that I would be worried that this causes dependencies
> between the mmci driver and ux500 specific functionality like the
> stedma40_filter function.
>
About DT
I think I need to catch up on the DT-design a bit. I will try to catch
up on the DT-implementation and maybe then the refactorization will
make sense to me :)

Board specific data in mmci-driver
The stedma40 filter function is not really specific for ux500. ux500
use stedma40 but it should be possible to replace this DMA.IP with
some other DMA-controller. This is board specific configuration. You
should not need to change the mmci-driver just because the dma-driver
has changed, right?
Or will the board-configuration now be placed in mmci-ux500?

Common DT populate code for all mmc host drivers
Some parts of the DT-population is common for all the host driver, for
instance setting the CAP and CAP2. This code should be moved to a
common place to be used by other host drivers as well.

About refactoring
There are numbers of patches stashed up for MMCI waiting for verdict
here 
http://git.kernel.org/?p=linux/kernel/git/linusw/linux-stericsson.git;a=shortlog;h=refs/heads/mmci.
If doing a refactorization please rebase it on top of these patches.
This needs to be done carefully, there may be more to considerations
than just DT. Maybe the timing is better to do this for 4.5, just
after 4.4 is closed. Then we can make sure that all new patches are
made on top of the refactorized base.

BR
Per
--
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 5/5] MMC: mmci: Add required documentation for Device Tree bindings

[Russell King - ARM Linux]

On Thu, Mar 15, 2012 at 05:53:25PM +, Arnd Bergmann wrote:
> On Wednesday 14 March 2012, Lee Jones wrote:
> > +
> > +Optional properties:
> > +- ocr_mask   : contains the regulator oscillator mask
> 
> Not sure about this one. If it's for the regulators, we should be
> using the regulator bindings.

It's also nothing to do with oscillators.
--
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 5/5] MMC: mmci: Add required documentation for Device Tree bindings

[Russell King - ARM Linux]

On Thu, Mar 15, 2012 at 05:49:07PM +, Arnd Bergmann wrote:
> On Thursday 15 March 2012, Russell King - ARM Linux wrote:
> > 
> > On Wed, Mar 14, 2012 at 02:20:03PM +, Lee Jones wrote:
> > > +ux500 variant specific properties:
> > > +- gpio-en: contains the level-shifter enable gpio
> > > +- gpio-vsel  : contains the level-shifter voltage select gpio
> > > +- tx_dst_dev_type: contains the tx destination device (DMA) 
> > > +- rx_src_dev_type: contains the rx source device (DMA)
> > 
> > So what about the DMA support being proposed elsewhere?
> > 
> > This sounds like we're inventing one DMA solution for one bit of the tree
> > and another DMA solution for another bit.  This is madness.
> 
> Agreed. I originally made a similar comment for an early version but had not
> noticed that it made it into this one. This should be using the proper DMA
> bindings, or just auxdata while we're not entirely there yet.

It's something that I'm also concerned with the main OF DMA patches.  It's
not at all clear what we want from DMA bindings yet because we've hardly
started converting from platform specific DMA APIs to the DMA engine based
APIs.

The way DMA channels are selected by DMA APIs is significantly different
from the DMA engine based API, and I remain worried that we're locking in
to a platform specific DMA API method.

I think the DMA parts should be left alone until there has been much more
of an effort to convert to DMA engine based APIs for the various platforms.

Note: OMAP DMA is something I've been asked to take over, and is something
I've been looking at over the last couple of weeks so far.
--
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/5] MMC: mmci: Seperate ARM variants from generic code

[Russell King - ARM Linux]

On Thu, Mar 15, 2012 at 05:46:56PM +, Arnd Bergmann wrote:
> Can you elaborate? I suggested the split in order to keep the ux500
> specific parts local to one file. With the device tree conversion,
> we really want to have them out of the platform code, but sticking them
> into the main driver seems wrong, too.

You're talking about the data structures which describe what quirks to
apply for the ux500 parts, rather than the actual quirks themselves.
That's not a particularly clever thing to do because it separates out
the selection of the works from the rest of the driver, which means
simply searching for the flags to find out what's applied for what has
to span several files.
--
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 5/5] MMC: mmci: Add required documentation for Device Tree bindings

[Arnd Bergmann]

On Wednesday 14 March 2012, Lee Jones wrote:

Some more comments about the individual properties

> +Required properties:
> +- compatible : contains "arm,pl18x", "arm,primecell".

Better use the exact number for 'x' here.

> +- reg: contains pl18x registers and length.
> +- interrupts : contains the device IRQ(s).

You should document what the first and second interrupts are,
respectively, if there is more than one.

> +- arm,primecell-periphid : contains the PrimeCell Peripheral ID.

This one should be optional.

> +
> +Optional properties:
> +- ocr_mask   : contains the regulator oscillator mask

Not sure about this one. If it's for the regulators, we should be
using the regulator bindings.

> +- wp-gpios   : contains any write protect (ro) gpios
> +- cd-gpios   : contains any card detection gpios
> +- cd-invert  : indicates whether the cd gpio is inverted 
> +- clock_frequency: contains the maximum operating frequency

The name should be 'clock-frequency', not 'clock_frequency'.

> +- mmc_cap_4_bit_data : indicates whether device is 4 bit capable
> +- mmc_cap_8_bit_data : indicates whether device is 8 bit capable
> +- mmc_cap_mmc_highspeed  : indicates whether MMC is high speed capable
> +- mmc_cap_sd_highspeed   : indicates whether SD is high speed capable

I would strongly recommend using the same naming as sdhci here. I think
tegra has set a precedent.

> +ux500 variant specific properties:
> +- gpio-en: contains the level-shifter enable gpio
> +- gpio-vsel  : contains the level-shifter voltage select gpio

I think these need to be 'en-gpios' and 'vsel-gpios', in order for the
gpio binding to work.

> +- tx_dst_dev_type: contains the tx destination device (DMA) 
> +- rx_src_dev_type: contains the rx source device (DMA)

As Russell commented already, these should not be here.

Arnd
--
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 5/5] MMC: mmci: Add required documentation for Device Tree bindings

[Arnd Bergmann]

On Thursday 15 March 2012, Russell King - ARM Linux wrote:
> 
> On Wed, Mar 14, 2012 at 02:20:03PM +, Lee Jones wrote:
> > +ux500 variant specific properties:
> > +- gpio-en: contains the level-shifter enable gpio
> > +- gpio-vsel  : contains the level-shifter voltage select gpio
> > +- tx_dst_dev_type: contains the tx destination device (DMA) 
> > +- rx_src_dev_type: contains the rx source device (DMA)
> 
> So what about the DMA support being proposed elsewhere?
> 
> This sounds like we're inventing one DMA solution for one bit of the tree
> and another DMA solution for another bit.  This is madness.

Agreed. I originally made a similar comment for an early version but had not
noticed that it made it into this one. This should be using the proper DMA
bindings, or just auxdata while we're not entirely there yet.

Arnd
--
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/5] MMC: mmci: Seperate ARM variants from generic code

[Arnd Bergmann]

On Thursday 15 March 2012, Russell King - ARM Linux wrote:
> On Thu, Mar 15, 2012 at 05:36:04PM +, Lee Jones wrote:
> > On 15/03/12 17:32, Russell King - ARM Linux wrote:
> > > On Wed, Mar 14, 2012 at 02:20:00PM +, Lee Jones wrote:
> > >> This is a step in the right direction for future Device
> > >> Tree support. It will allow variant specific attributes
> > >> to be collected from a Device Tree without overloading
> > >> the MMCI core. It will also provide additional future
> > >> variants a cleaner way to add support.
> > > 
> > > NAK.  You can't have two driver structures with the same name.  The names
> > > are sysfs object names.
> > 
> > Well spotted.
> > 
> > I've already changed this in v2. :)
> 
> I'm not in favour of splitting the two variants either btw.

Can you elaborate? I suggested the split in order to keep the ux500
specific parts local to one file. With the device tree conversion,
we really want to have them out of the platform code, but sticking them
into the main driver seems wrong, too.

I don't mind leaving the ARM and u300 specific bits in the main driver,
but I think having the ux500 parts separate does indeed help.

Arnd
--
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: mmci: assume maintainership

[Linus Walleij]

On Thu, Mar 15, 2012 at 6:30 PM, Russell King  wrote:

> Actually, I still want to be involved because I'm far from convinced with
> some of the patches coming from Ulf are correct for ARMs implementation.

OK fine with me.

> That's why I'm purposely not applying the MMCI patches in the patch system
> in a particularly fast manner - I want there to be a decent amount of
> testing between each set of patches.

I understand this. I will try to help out more then, I have
the following systems:

- ARM Integrator/CP (PL181!)
- ARM RealView PB1176 (PL180)
- U300 (PL180 derivate)
- U8500 (PL180 derivate)

For the next round of series from Ulf I'll be happy to stress
these a bit.

> For example, the next patch in the series to be applied is:
>
> "mmc: mmci: Decrease current consumption in suspend"
(...)
> As there's an interdependence between the various patches, I can't apply
> any of the later patches even if I wanted to without these "power saving"
> patches.

OK Ulf, can you reorder your patch queue pushing that patch
towards the end so Russell can (hopefully) apply the less
controversial stuff (like the SDIO fixes and so on)?

> So, it's not really unmaintained or orphaned...

OK, shall I propose a patch simply adding you as maintainer?

Yours,
Linus Walleij
--
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/5] MMC: mmci: Seperate ARM variants from generic code

[Lee Jones]

On 15/03/12 17:36, Lee Jones wrote:
> On 15/03/12 17:32, Russell King - ARM Linux wrote:
>> On Wed, Mar 14, 2012 at 02:20:00PM +, Lee Jones wrote:
>>> This is a step in the right direction for future Device
>>> Tree support. It will allow variant specific attributes
>>> to be collected from a Device Tree without overloading
>>> the MMCI core. It will also provide additional future
>>> variants a cleaner way to add support.
>>
>> NAK.  You can't have two driver structures with the same name.  The names
>> are sysfs object names.
> 
> Well spotted.
> 
> I've already changed this in v2. :)

In case you were interested, see:

http://git.linaro.org/gitweb?p=people/ljones/linux-3.0-ux500.git;a=shortlog;h=refs/heads/device-tree-snowball-mmci

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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/5] MMC: mmci: Seperate ARM variants from generic code

[Russell King - ARM Linux]

On Thu, Mar 15, 2012 at 05:36:04PM +, Lee Jones wrote:
> On 15/03/12 17:32, Russell King - ARM Linux wrote:
> > On Wed, Mar 14, 2012 at 02:20:00PM +, Lee Jones wrote:
> >> This is a step in the right direction for future Device
> >> Tree support. It will allow variant specific attributes
> >> to be collected from a Device Tree without overloading
> >> the MMCI core. It will also provide additional future
> >> variants a cleaner way to add support.
> > 
> > NAK.  You can't have two driver structures with the same name.  The names
> > are sysfs object names.
> 
> Well spotted.
> 
> I've already changed this in v2. :)

I'm not in favour of splitting the two variants either btw.
--
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/5] MMC: mmci: Seperate ARM variants from generic code

[Lee Jones]

On 15/03/12 17:32, Russell King - ARM Linux wrote:
> On Wed, Mar 14, 2012 at 02:20:00PM +, Lee Jones wrote:
>> This is a step in the right direction for future Device
>> Tree support. It will allow variant specific attributes
>> to be collected from a Device Tree without overloading
>> the MMCI core. It will also provide additional future
>> variants a cleaner way to add support.
> 
> NAK.  You can't have two driver structures with the same name.  The names
> are sysfs object names.

Well spotted.

I've already changed this in v2. :)

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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 5/5] MMC: mmci: Add required documentation for Device Tree bindings

[Russell King - ARM Linux]

On Wed, Mar 14, 2012 at 02:20:03PM +, Lee Jones wrote:
> +ux500 variant specific properties:
> +- gpio-en: contains the level-shifter enable gpio
> +- gpio-vsel  : contains the level-shifter voltage select gpio
> +- tx_dst_dev_type: contains the tx destination device (DMA) 
> +- rx_src_dev_type: contains the rx source device (DMA)

So what about the DMA support being proposed elsewhere?

This sounds like we're inventing one DMA solution for one bit of the tree
and another DMA solution for another bit.  This is madness.

Note that ARM boards like Versatile also have DMA support, even though
most of those platforms are diabolically crippled to the point of it being
useless.  I believe it works on a few select boards...
--
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/5] MMC: mmci: Seperate ARM variants from generic code

[Russell King - ARM Linux]

On Wed, Mar 14, 2012 at 02:20:00PM +, Lee Jones wrote:
> This is a step in the right direction for future Device
> Tree support. It will allow variant specific attributes
> to be collected from a Device Tree without overloading
> the MMCI core. It will also provide additional future
> variants a cleaner way to add support.

NAK.  You can't have two driver structures with the same name.  The names
are sysfs object names.
--
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: mmci: assume maintainership

[Russell King]

On Thu, Mar 15, 2012 at 03:30:36PM +0100, Linus Walleij wrote:
> From: Linus Walleij 
> 
> So since this driver is crucial for us (as in ST-Ericsson)
> to have actively maintained, and since it is marked orphaned
> I will assume maintainership for it starting with the 3.5
> integration cycle. The idea is to collect patches and send
> either patch sets of pull requests to Chris.

Actually, I still want to be involved because I'm far from convinced with
some of the patches coming from Ulf are correct for ARMs implementation.
That's why I'm purposely not applying the MMCI patches in the patch system
in a particularly fast manner - I want there to be a decent amount of
testing between each set of patches.

For example, the next patch in the series to be applied is:

"mmc: mmci: Decrease current consumption in suspend"

and as I've explained before, it does this by saving the MMCI power and
clock registers and clearing them.  This has the effect of stopping the
clock, and in the case of ARMs hardware, removing power from the card.
Therefore, the card will require full bring up, including reassigning
the card address for MMC cards on resume, and simply starting the next
command will not be possible.

As I've already tried to explain, on the normal power management suspend
callback, these registers should _already_ have been placed into their
"card not present" state, so saving and zeroing them has no useful effect.

Moreover, saving and restoring them across suspend is wrong because on
ARM hardware there's a sequence of power-off, power-up, power-on which
needs to be gone through to restore power.

So, I've queued up the 10 patches which I'm happy to take for v3.4, and
that's all I'm taking for v3.4 until I can find the strength to have
another discussion with Ulf over this issue.

As there's an interdependence between the various patches, I can't apply
any of the later patches even if I wanted to without these "power saving"
patches.

So, it's not really unmaintained or orphaned...

-- 
Russell King
 Linux kernel2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
--
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 0/5] MMC: mmci: Provide bindings for Device Tree

[Lee Jones]

On 15/03/12 15:32, Arnd Bergmann wrote:
> On Thursday 15 March 2012, Lee Jones wrote:
>>> I would like to see what the minimal required change is to support DT
>>> for mmci without factorization.
>>> 1. Minimal change in mmci.
>>> 2. Add mmci_dt.c which contains the DT-populate code.
>>>
>>> The factorization could be done as step 2 I think.
>>>
>>> What do you say?
>>
>> I'm wondering what the difference is as the work has already been done.
>>
>> It was Arnd's suggestion to separate out the two types of variants, and
>> I'm quite fond of the new (fully featured) layout.
> 
> Right, I usually prefer cleanups or other refactoring to be done first, and
> then features added on top.
> 
> You could in theory add have just patches 3/4/5 all applied without
> the refactoring, but that I would be worried that this causes dependencies
> between the mmci driver and ux500 specific functionality like the
> stedma40_filter function.

It's a possibility that we may have run into that even with this new
layout. Remember you suggested that the u300 variant may be better off
going into the ARM variant's file, but I wasn't quite sure, as it was an
ST product. Well the afore mentioned function may still cause us some
trouble. I am in the mists of some regression testing now. I'll report
back when I know more.

Kind regards,
Lee


-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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 0/5] MMC: mmci: Provide bindings for Device Tree

[Arnd Bergmann]

On Thursday 15 March 2012, Lee Jones wrote:
> > I would like to see what the minimal required change is to support DT
> > for mmci without factorization.
> > 1. Minimal change in mmci.
> > 2. Add mmci_dt.c which contains the DT-populate code.
> > 
> > The factorization could be done as step 2 I think.
> > 
> > What do you say?
> 
> I'm wondering what the difference is as the work has already been done.
> 
> It was Arnd's suggestion to separate out the two types of variants, and
> I'm quite fond of the new (fully featured) layout.

Right, I usually prefer cleanups or other refactoring to be done first, and
then features added on top.

You could in theory add have just patches 3/4/5 all applied without
the refactoring, but that I would be worried that this causes dependencies
between the mmci driver and ux500 specific functionality like the
stedma40_filter function.

Arnd
--
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 0/5] MMC: mmci: Provide bindings for Device Tree

[Lee Jones]

Hi Per,

> I would like to see what the minimal required change is to support DT
> for mmci without factorization.
> 1. Minimal change in mmci.
> 2. Add mmci_dt.c which contains the DT-populate code.
> 
> The factorization could be done as step 2 I think.
> 
> What do you say?

I'm wondering what the difference is as the work has already been done.

It was Arnd's suggestion to separate out the two types of variants, and
I'm quite fond of the new (fully featured) layout.

Kind regards,
Lee

> On Wed, Mar 14, 2012 at 3:19 PM, Lee Jones  wrote:
>> This patch-set splits out ux500 and ARM specific variants from mmci
>> core code, applies generic bindings for use by all variants, then
>> enables the ux500 variant for use with Device Tree.
>>
>>  Documentation/devicetree/bindings/mmc/mmci.txt |   27 +++
>>  arch/arm/boot/dts/db8500.dtsi  |6 +-
>>  arch/arm/boot/dts/snowball.dts |   30 +++-
>>  arch/arm/mach-ux500/board-mop500.c |3 +-
>>  drivers/mmc/host/Makefile  |2 +-
>>  drivers/mmc/host/mmci-arm.c|   65 ++
>>  drivers/mmc/host/mmci-ux500.c  |  260 
>> 
>>  drivers/mmc/host/mmci.c|  194 +-
>>  drivers/mmc/host/mmci.h|   37 
>>  9 files changed, 473 insertions(+), 151 deletions(-)
>>
>> --
>> 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


-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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 3/5] MMC: mmci: Add generic Device Tree bindings to mmci core code

[Lee Jones]

On 15/03/12 15:12, Per Forlin wrote:
> On Wed, Mar 14, 2012 at 3:20 PM, Lee Jones  wrote:
>> This adds the necessary bindings for collection of shared
>> attributes used in the mmci driver.
>>
>> Signed-off-by: Lee Jones 
>> ---
>>  drivers/mmc/host/mmci.c |   43 +++
>>  1 files changed, 43 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
>> index 23b41a5..9132ca8 100644
>> --- a/drivers/mmc/host/mmci.c
>> +++ b/drivers/mmc/host/mmci.c
>> @@ -30,6 +30,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>
>>  #include 
>>  #include 
>> @@ -1056,11 +1057,47 @@ static const struct mmc_host_ops mmci_ops = {
>>.get_cd = mmci_get_cd,
>>  };
>>
>> +#ifdef CONFIG_OF
>> +static void mmci_dt_populate_generic_pdata(struct device_node *np,
>> +   struct mmci_platform_data *pdata)
>> +{
>> +   const void *prop;
>> +   int len;
>> +
>> +   of_property_read_u32(np, "wp-gpios", &pdata->gpio_wp);
>> +   if (!pdata->gpio_wp)
>> +   pdata->gpio_wp = -1;
>> +
>> +   of_property_read_u32(np, "cd-gpios", &pdata->gpio_cd);
>> +   if (!pdata->gpio_cd)
>> +   pdata->gpio_cd = -1;
>> +
>> +   if (of_get_property(np, "cd-invert", NULL))
>> +   pdata->cd_invert = true;
>> +   else
>> +   pdata->cd_invert = false;
>> +
>> +   of_property_read_u32(np, "clock_frequency", &pdata->f_max);
>> +   if (!pdata->f_max)
>> +   pr_warning("%s has no 'clock_frequency' property\n", 
>> np->full_name);
>> +
>> +   if (of_get_property(np, "mmc_cap_4_bit_data", NULL))
> I have no previous experience with DT. Could you please bring some
> light on this.
> Is it really necessary to represent each bit in the CAP with a string?

No, you can use a straight integer in the DT blob, but this makes it
much easier to read.

> To add CAP_ERASE for instance I need to change the code here and
> update the DT, right?

That's right, but no MMCI user makes use of it, so it would just be
unnecessary/unused code clogging up the core driver.

Kind regards,
Lee

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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 RESEND 4/8] mmc: omap: context save after enabling runtime pm

[Shubhrajyoti]

On Thursday 15 March 2012 08:03 PM, Venkatraman S wrote:
> From: Balaji T K 
>
> call context save api after enabling runtime pm
> to make sure register access in context save api
If I am not mistaken  the api  would
store the number of power state changes and accesses no registers.

Am I missing something?

>  happens with clk enabled.
>
> Signed-off-by: Balaji T K 
> ---
>  drivers/mmc/host/omap_hsmmc.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 0f8d34b..9fa2f39 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1871,8 +1871,6 @@ static int __init omap_hsmmc_probe(struct 
> platform_device *pdev)
>   goto err1;
>   }
>  
> - omap_hsmmc_context_save(host);
> -
>   if (host->pdata->controller_flags & OMAP_HSMMC_BROKEN_MULTIBLOCK_READ) {
>   dev_info(&pdev->dev, "multiblock reads disabled due to 35xx 
> erratum 2.1.1.128; MMC read performance may suffer\n");
>   mmc->caps2 |= MMC_CAP2_NO_MULTI_READ;
> @@ -1883,6 +1881,8 @@ static int __init omap_hsmmc_probe(struct 
> platform_device *pdev)
>   pm_runtime_set_autosuspend_delay(host->dev, MMC_AUTOSUSPEND_DELAY);
>   pm_runtime_use_autosuspend(host->dev);
>  
> + omap_hsmmc_context_save(host);
> +
>   if (cpu_is_omap2430()) {
>   host->dbclk = clk_get(&pdev->dev, "mmchsdb_fck");
>   /*

--
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 3/5] MMC: mmci: Add generic Device Tree bindings to mmci core code

[Per Forlin]

On Wed, Mar 14, 2012 at 3:20 PM, Lee Jones  wrote:
> This adds the necessary bindings for collection of shared
> attributes used in the mmci driver.
>
> Signed-off-by: Lee Jones 
> ---
>  drivers/mmc/host/mmci.c |   43 +++
>  1 files changed, 43 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 23b41a5..9132ca8 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -30,6 +30,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> @@ -1056,11 +1057,47 @@ static const struct mmc_host_ops mmci_ops = {
>        .get_cd         = mmci_get_cd,
>  };
>
> +#ifdef CONFIG_OF
> +static void mmci_dt_populate_generic_pdata(struct device_node *np,
> +                                       struct mmci_platform_data *pdata)
> +{
> +       const void *prop;
> +       int len;
> +
> +       of_property_read_u32(np, "wp-gpios", &pdata->gpio_wp);
> +       if (!pdata->gpio_wp)
> +               pdata->gpio_wp = -1;
> +
> +       of_property_read_u32(np, "cd-gpios", &pdata->gpio_cd);
> +       if (!pdata->gpio_cd)
> +               pdata->gpio_cd = -1;
> +
> +       if (of_get_property(np, "cd-invert", NULL))
> +               pdata->cd_invert = true;
> +       else
> +               pdata->cd_invert = false;
> +
> +       of_property_read_u32(np, "clock_frequency", &pdata->f_max);
> +       if (!pdata->f_max)
> +               pr_warning("%s has no 'clock_frequency' property\n", 
> np->full_name);
> +
> +       if (of_get_property(np, "mmc_cap_4_bit_data", NULL))
I have no previous experience with DT. Could you please bring some
light on this.
Is it really necessary to represent each bit in the CAP with a string?
To add CAP_ERASE for instance I need to change the code here and
update the DT, right?

BR
Per
--
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: mmci: assume maintainership

[Per Förlin]

On 03/15/2012 03:30 PM, Linus WALLEIJ wrote:
> From: Linus Walleij 
> 
> So since this driver is crucial for us (as in ST-Ericsson)
> to have actively maintained, and since it is marked orphaned
> I will assume maintainership for it starting with the 3.5
> integration cycle. The idea is to collect patches and send
> either patch sets of pull requests to Chris.
> 
> Cc: Ulf Hansson 
> Cc: Per Forlin 
> Cc: Pawel Moll 
> Cc: Russell King 
> Signed-off-by: Linus Walleij 
> ---
>  MAINTAINERS |3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3321d75..3f280f6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -630,7 +630,8 @@ F:drivers/input/serio/ambakmi.*
>  F:   include/linux/amba/kmi.h
>  
>  ARM PRIMECELL MMCI PL180/1 DRIVER
> -S:   Orphan
> +M:   Linus Walleij 
> +L:   linux-mmc@vger.kernel.org
>  F:   drivers/mmc/host/mmci.*
>  
>  ARM PRIMECELL BUS SUPPORT

Acked-by: Per Forlin 

Thanks,
Per
--
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 0/5] MMC: mmci: Provide bindings for Device Tree

[Per Forlin]

Hi,

I would like to see what the minimal required change is to support DT
for mmci without factorization.
1. Minimal change in mmci.
2. Add mmci_dt.c which contains the DT-populate code.

The factorization could be done as step 2 I think.

What do you say?

BR
Per

On Wed, Mar 14, 2012 at 3:19 PM, Lee Jones  wrote:
> This patch-set splits out ux500 and ARM specific variants from mmci
> core code, applies generic bindings for use by all variants, then
> enables the ux500 variant for use with Device Tree.
>
>  Documentation/devicetree/bindings/mmc/mmci.txt |   27 +++
>  arch/arm/boot/dts/db8500.dtsi                  |    6 +-
>  arch/arm/boot/dts/snowball.dts                 |   30 +++-
>  arch/arm/mach-ux500/board-mop500.c             |    3 +-
>  drivers/mmc/host/Makefile                      |    2 +-
>  drivers/mmc/host/mmci-arm.c                    |   65 ++
>  drivers/mmc/host/mmci-ux500.c                  |  260 
> 
>  drivers/mmc/host/mmci.c                        |  194 +-
>  drivers/mmc/host/mmci.h                        |   37 
>  9 files changed, 473 insertions(+), 151 deletions(-)
>
> --
> 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 RESEND 8/8] mmc: omap4: hsmmc: fix module re-insertion

[Felipe Balbi]

Hi,

On Thu, Mar 15, 2012 at 08:03:42PM +0530, Venkatraman S wrote:
> From: Balaji T K 
> 
> OMAP4 and OMAP3 HSMMC IP registers differ by 0x100 offset.
> Addng the offset to platform_device resource structure
> increments the start address for every insmod operation.
> MMC command fails on re-insertion as module due to incorrect register base.
> Fix this by updating the ioremap base address only.
> 
> Signed-off-by: Balaji T K 
> ---
>  drivers/mmc/host/omap_hsmmc.c |4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 4476b26..f324cf4 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1813,8 +1813,6 @@ static int __devinit omap_hsmmc_probe(struct 
> platform_device *pdev)
>   if (res == NULL || irq < 0)
>   return -ENXIO;
>  
> - res->start += pdata->reg_offset;
> - res->end += pdata->reg_offset;
>   res = request_mem_region(res->start, resource_size(res), pdev->name);
>   if (res == NULL)
>   return -EBUSY;
> @@ -1838,7 +1836,7 @@ static int __devinit omap_hsmmc_probe(struct 
> platform_device *pdev)
>   host->dma_ch= -1;
>   host->irq   = irq;
>   host->slot_id   = 0;
> - host->mapbase   = res->start;
> + host->mapbase   = res->start + pdata->reg_offset;

could this be done with a revision check at some point so we drop the
pdata requirement ? Not part of $SUBJECT though, because you're just
moving the increment.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH RESEND 4/8] mmc: omap: context save after enabling runtime pm

[Felipe Balbi]

On Thu, Mar 15, 2012 at 08:03:38PM +0530, Venkatraman S wrote:
> From: Balaji T K 
> 
> call context save api after enabling runtime pm
> to make sure register access in context save api happens with clk enabled.
> 
> Signed-off-by: Balaji T K 

Cc stable ?

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH RESEND 3/8] mmc: omap: use runtime put sync in probe error patch

[Felipe Balbi]

On Thu, Mar 15, 2012 at 08:03:37PM +0530, Venkatraman S wrote:
> From: Balaji T K 
> 
> pm_runtime_put_sync instead of autosuspend pm runtime API
> because iounmap(host->base) follows immediately.
> 
> Reported-by: Rajendra Nayak 
> Signed-off-by: Balaji T K 

should this one go to stable too ?

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH RESEND 1/8] mmc: omap: Enable Auto CMD12

[Felipe Balbi]

On Thu, Mar 15, 2012 at 08:03:35PM +0530, Venkatraman S wrote:
> From: Balaji T K 
> 
> Enable Auto-CMD12 for multi block read/write on HSMMC
> Tested on OMAP4430, OMAP3430 and OMAP2430 SDP
> 
> Signed-off-by: Balaji T K 

BTW, since patches are flowing through you now, they should have your
SoB line.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH RESEND 1/8] mmc: omap: Enable Auto CMD12

[Felipe Balbi]

Hi,

On Thu, Mar 15, 2012 at 08:03:35PM +0530, Venkatraman S wrote:
> From: Balaji T K 
> @@ -766,6 +769,8 @@ omap_hsmmc_start_command(struct omap_hsmmc_host *host, 
> struct mmc_command *cmd,
>   cmdtype = 0x3;
>  
>   cmdreg = (cmd->opcode << 24) | (resptype << 16) | (cmdtype << 22);
> + if ((host->flags & AUTO_CMD12) && mmc_op_multi(cmd->opcode))

This should have braces too.

> @@ -837,11 +842,16 @@ omap_hsmmc_xfer_done(struct omap_hsmmc_host *host, 
> struct mmc_data *data)
>   else
>   data->bytes_xfered = 0;
>  
> - if (!data->stop) {
> + if (data->stop && ((!(host->flags & AUTO_CMD12)) || data->error))
> + omap_hsmmc_start_command(host, data->stop, NULL);
> + else {
> + if (data->stop)
> + data->stop->resp[0] = OMAP_HSMMC_READ(host->base,
> + RSP76);
>   omap_hsmmc_request_done(host, data->mrq);
> - return;
>   }
> - omap_hsmmc_start_command(host, data->stop, NULL);
> +
> + return;

return is unnecessary.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH] mmc: mmci: assume maintainership

[Pawel Moll]

On Thu, 2012-03-15 at 14:30 +, Linus Walleij wrote:
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3321d75..3f280f6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -630,7 +630,8 @@ F:drivers/input/serio/ambakmi.*
>  F:   include/linux/amba/kmi.h
>  
>  ARM PRIMECELL MMCI PL180/1 DRIVER
> -S:   Orphan

You probably meant

+S: Maintained

> +M:   Linus Walleij 
> +L:   linux-mmc@vger.kernel.org
>  F:   drivers/mmc/host/mmci.*
>  
>  ARM PRIMECELL BUS SUPPORT

By all means!

Acked-by: Pawel Moll 

Cheers!

Paweł


--
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] mmc: mmci: assume maintainership

[Linus Walleij]

From: Linus Walleij 

So since this driver is crucial for us (as in ST-Ericsson)
to have actively maintained, and since it is marked orphaned
I will assume maintainership for it starting with the 3.5
integration cycle. The idea is to collect patches and send
either patch sets of pull requests to Chris.

Cc: Ulf Hansson 
Cc: Per Forlin 
Cc: Pawel Moll 
Cc: Russell King 
Signed-off-by: Linus Walleij 
---
 MAINTAINERS |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3321d75..3f280f6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -630,7 +630,8 @@ F:  drivers/input/serio/ambakmi.*
 F: include/linux/amba/kmi.h
 
 ARM PRIMECELL MMCI PL180/1 DRIVER
-S: Orphan
+M: Linus Walleij 
+L: linux-mmc@vger.kernel.org
 F: drivers/mmc/host/mmci.*
 
 ARM PRIMECELL BUS SUPPORT
-- 
1.7.9.2

--
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: mmci: assume maintainership

[Ulf Hansson]

Hi Linus,

Great initiative! Of course you get my Ack.

Acked-by: Ulf Hansson 

Br
Ulf Hansson

On 03/15/2012 03:30 PM, Linus WALLEIJ wrote:

From: Linus Walleij

So since this driver is crucial for us (as in ST-Ericsson)
to have actively maintained, and since it is marked orphaned
I will assume maintainership for it starting with the 3.5
integration cycle. The idea is to collect patches and send
either patch sets of pull requests to Chris.

Cc: Ulf Hansson
Cc: Per Forlin
Cc: Pawel Moll
Cc: Russell King
Signed-off-by: Linus Walleij
---
  MAINTAINERS |3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3321d75..3f280f6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -630,7 +630,8 @@ F:  drivers/input/serio/ambakmi.*
  F:include/linux/amba/kmi.h

  ARM PRIMECELL MMCI PL180/1 DRIVER
-S: Orphan
+M: Linus Walleij
+L: linux-mmc@vger.kernel.org
  F:drivers/mmc/host/mmci.*

  ARM PRIMECELL BUS SUPPORT


--
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 RESEND 8/8] mmc: omap4: hsmmc: fix module re-insertion

[Venkatraman S]

From: Balaji T K 

OMAP4 and OMAP3 HSMMC IP registers differ by 0x100 offset.
Addng the offset to platform_device resource structure
increments the start address for every insmod operation.
MMC command fails on re-insertion as module due to incorrect register base.
Fix this by updating the ioremap base address only.

Signed-off-by: Balaji T K 
---
 drivers/mmc/host/omap_hsmmc.c |4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 4476b26..f324cf4 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1813,8 +1813,6 @@ static int __devinit omap_hsmmc_probe(struct 
platform_device *pdev)
if (res == NULL || irq < 0)
return -ENXIO;
 
-   res->start += pdata->reg_offset;
-   res->end += pdata->reg_offset;
res = request_mem_region(res->start, resource_size(res), pdev->name);
if (res == NULL)
return -EBUSY;
@@ -1838,7 +1836,7 @@ static int __devinit omap_hsmmc_probe(struct 
platform_device *pdev)
host->dma_ch= -1;
host->irq   = irq;
host->slot_id   = 0;
-   host->mapbase   = res->start;
+   host->mapbase   = res->start + pdata->reg_offset;
host->base  = ioremap(host->mapbase, SZ_4K);
host->power_mode = MMC_POWER_OFF;
host->flags = AUTO_CMD12;
-- 
1.7.10.rc0.41.gfa678

--
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 RESEND 7/8] mmc: host: omap_hsmmc: convert to module_platform_driver

[Venkatraman S]

From: Felipe Balbi 

this will delete some boilerplate code, no functional
changes.

Signed-off-by: Felipe Balbi 
---
 drivers/mmc/host/omap_hsmmc.c |   16 +---
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 67cb63e..4476b26 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -2200,21 +2200,7 @@ static struct platform_driver omap_hsmmc_driver = {
},
 };
 
-static int __init omap_hsmmc_init(void)
-{
-   /* Register the MMC driver */
-   return platform_driver_register(&omap_hsmmc_driver);
-}
-
-static void __exit omap_hsmmc_cleanup(void)
-{
-   /* Unregister MMC driver */
-   platform_driver_unregister(&omap_hsmmc_driver);
-}
-
-module_init(omap_hsmmc_init);
-module_exit(omap_hsmmc_cleanup);
-
+module_platform_driver(omap_hsmmc_driver);
 MODULE_DESCRIPTION("OMAP High Speed Multimedia Card driver");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:" DRIVER_NAME);
-- 
1.7.10.rc0.41.gfa678

--
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 RESEND 6/8] mmc: host: omap_hsmmc: make it behave well as a module

[Venkatraman S]

From: Felipe Balbi 

if we put probe() on __init section, that will never
work for multiple module insertions/removals.

In order to make it work properly, move probe to
__devinit section and use platform_driver_register()
instead of platform_driver_probe().

Signed-off-by: Felipe Balbi 
---
 drivers/mmc/host/omap_hsmmc.c |9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 6b30782..67cb63e 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1780,7 +1780,7 @@ static inline struct omap_mmc_platform_data
 }
 #endif
 
-static int __init omap_hsmmc_probe(struct platform_device *pdev)
+static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
 {
struct omap_mmc_platform_data *pdata = pdev->dev.platform_data;
struct mmc_host *mmc;
@@ -2032,7 +2032,7 @@ err:
return ret;
 }
 
-static int omap_hsmmc_remove(struct platform_device *pdev)
+static int __devexit omap_hsmmc_remove(struct platform_device *pdev)
 {
struct omap_hsmmc_host *host = platform_get_drvdata(pdev);
struct resource *res;
@@ -2190,7 +2190,8 @@ static struct dev_pm_ops omap_hsmmc_dev_pm_ops = {
 };
 
 static struct platform_driver omap_hsmmc_driver = {
-   .remove = omap_hsmmc_remove,
+   .probe  = omap_hsmmc_probe,
+   .remove = __devexit_p(omap_hsmmc_remove),
.driver = {
.name = DRIVER_NAME,
.owner = THIS_MODULE,
@@ -2202,7 +2203,7 @@ static struct platform_driver omap_hsmmc_driver = {
 static int __init omap_hsmmc_init(void)
 {
/* Register the MMC driver */
-   return platform_driver_probe(&omap_hsmmc_driver, omap_hsmmc_probe);
+   return platform_driver_register(&omap_hsmmc_driver);
 }
 
 static void __exit omap_hsmmc_cleanup(void)
-- 
1.7.10.rc0.41.gfa678

--
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 RESEND 5/8] mmc: host: omap_hsmmc: trivial cleanups

[Venkatraman S]

From: Felipe Balbi 

a bunch of non-functional cleanups to the omap_hsmmc
driver.

It basically decreases indentation level, drop unneded
dereferences and drop unneded accesses to the platform_device
structure.

Signed-off-by: Felipe Balbi 
---
 drivers/mmc/host/omap_hsmmc.c |  147 -
 1 file changed, 70 insertions(+), 77 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 9fa2f39..6b30782 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -2037,30 +2037,28 @@ static int omap_hsmmc_remove(struct platform_device 
*pdev)
struct omap_hsmmc_host *host = platform_get_drvdata(pdev);
struct resource *res;
 
-   if (host) {
-   pm_runtime_get_sync(host->dev);
-   mmc_remove_host(host->mmc);
-   if (host->use_reg)
-   omap_hsmmc_reg_put(host);
-   if (host->pdata->cleanup)
-   host->pdata->cleanup(&pdev->dev);
-   free_irq(host->irq, host);
-   if (mmc_slot(host).card_detect_irq)
-   free_irq(mmc_slot(host).card_detect_irq, host);
-
-   pm_runtime_put_sync(host->dev);
-   pm_runtime_disable(host->dev);
-   clk_put(host->fclk);
-   if (host->got_dbclk) {
-   clk_disable(host->dbclk);
-   clk_put(host->dbclk);
-   }
+   pm_runtime_get_sync(host->dev);
+   mmc_remove_host(host->mmc);
+   if (host->use_reg)
+   omap_hsmmc_reg_put(host);
+   if (host->pdata->cleanup)
+   host->pdata->cleanup(&pdev->dev);
+   free_irq(host->irq, host);
+   if (mmc_slot(host).card_detect_irq)
+   free_irq(mmc_slot(host).card_detect_irq, host);
 
-   mmc_free_host(host->mmc);
-   iounmap(host->base);
-   omap_hsmmc_gpio_free(pdev->dev.platform_data);
+   pm_runtime_put_sync(host->dev);
+   pm_runtime_disable(host->dev);
+   clk_put(host->fclk);
+   if (host->got_dbclk) {
+   clk_disable(host->dbclk);
+   clk_put(host->dbclk);
}
 
+   mmc_free_host(host->mmc);
+   iounmap(host->base);
+   omap_hsmmc_gpio_free(pdev->dev.platform_data);
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (res)
release_mem_region(res->start, resource_size(res));
@@ -2073,49 +2071,45 @@ static int omap_hsmmc_remove(struct platform_device 
*pdev)
 static int omap_hsmmc_suspend(struct device *dev)
 {
int ret = 0;
-   struct platform_device *pdev = to_platform_device(dev);
-   struct omap_hsmmc_host *host = platform_get_drvdata(pdev);
+   struct omap_hsmmc_host *host = dev_get_drvdata(dev);
 
-   if (host && host->suspended)
+   if (!host)
return 0;
 
-   if (host) {
-   pm_runtime_get_sync(host->dev);
-   host->suspended = 1;
-   if (host->pdata->suspend) {
-   ret = host->pdata->suspend(&pdev->dev,
-   host->slot_id);
-   if (ret) {
-   dev_dbg(mmc_dev(host->mmc),
-   "Unable to handle MMC board"
-   " level suspend\n");
-   host->suspended = 0;
-   return ret;
-   }
-   }
-   ret = mmc_suspend_host(host->mmc);
+   if (host && host->suspended)
+   return 0;
 
+   pm_runtime_get_sync(host->dev);
+   host->suspended = 1;
+   if (host->pdata->suspend) {
+   ret = host->pdata->suspend(dev, host->slot_id);
if (ret) {
+   dev_dbg(dev, "Unable to handle MMC board"
+   " level suspend\n");
host->suspended = 0;
-   if (host->pdata->resume) {
-   ret = host->pdata->resume(&pdev->dev,
- host->slot_id);
-   if (ret)
-   dev_dbg(mmc_dev(host->mmc),
-   "Unmask interrupt failed\n");
-   }
-   goto err;
+   return ret;
}
+   }
+   ret = mmc_suspend_host(host->mmc);
 
-   if (!(host->mmc->pm_flags & MMC_PM_KEEP_POWER)) {
-   omap_hsmmc_disable_irq(host);
-   OMAP_HSMMC_WRITE(host->base, HCTL,
-   OMAP_HSMMC_READ(host->base, HCTL) & ~SDBP);
+   if (ret) {
+   host->suspended = 0;
+   if (host->pdata->resume) {
+

[PATCH RESEND 4/8] mmc: omap: context save after enabling runtime pm

[Venkatraman S]

From: Balaji T K 

call context save api after enabling runtime pm
to make sure register access in context save api happens with clk enabled.

Signed-off-by: Balaji T K 
---
 drivers/mmc/host/omap_hsmmc.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 0f8d34b..9fa2f39 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1871,8 +1871,6 @@ static int __init omap_hsmmc_probe(struct platform_device 
*pdev)
goto err1;
}
 
-   omap_hsmmc_context_save(host);
-
if (host->pdata->controller_flags & OMAP_HSMMC_BROKEN_MULTIBLOCK_READ) {
dev_info(&pdev->dev, "multiblock reads disabled due to 35xx 
erratum 2.1.1.128; MMC read performance may suffer\n");
mmc->caps2 |= MMC_CAP2_NO_MULTI_READ;
@@ -1883,6 +1881,8 @@ static int __init omap_hsmmc_probe(struct platform_device 
*pdev)
pm_runtime_set_autosuspend_delay(host->dev, MMC_AUTOSUSPEND_DELAY);
pm_runtime_use_autosuspend(host->dev);
 
+   omap_hsmmc_context_save(host);
+
if (cpu_is_omap2430()) {
host->dbclk = clk_get(&pdev->dev, "mmchsdb_fck");
/*
-- 
1.7.10.rc0.41.gfa678

--
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 RESEND 3/8] mmc: omap: use runtime put sync in probe error patch

[Venkatraman S]

From: Balaji T K 

pm_runtime_put_sync instead of autosuspend pm runtime API
because iounmap(host->base) follows immediately.

Reported-by: Rajendra Nayak 
Signed-off-by: Balaji T K 
---
 drivers/mmc/host/omap_hsmmc.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index db8af43..0f8d34b 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -2014,8 +2014,7 @@ err_reg:
 err_irq_cd_init:
free_irq(host->irq, host);
 err_irq:
-   pm_runtime_mark_last_busy(host->dev);
-   pm_runtime_put_autosuspend(host->dev);
+   pm_runtime_put_sync(host->dev);
pm_runtime_disable(host->dev);
clk_put(host->fclk);
if (host->got_dbclk) {
-- 
1.7.10.rc0.41.gfa678

--
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 RESEND 2/8] mmc: omap: add DDR support to omap_hsmmc

[Venkatraman S]

From: Balaji T K 

Add Dual data rate support for omap_hsmmc

Signed-off-by: Balaji T K 
---
 drivers/mmc/host/omap_hsmmc.c |5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index b1e9be7..db8af43 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -92,6 +92,7 @@
 #define MSBS   (1 << 5)
 #define BCE(1 << 1)
 #define FOUR_BIT   (1 << 1)
+#define DDR(1 << 19)
 #define DW8(1 << 5)
 #define CC 0x1
 #define TC 0x02
@@ -523,6 +524,10 @@ static void omap_hsmmc_set_bus_width(struct 
omap_hsmmc_host *host)
u32 con;
 
con = OMAP_HSMMC_READ(host->base, CON);
+   if (ios->timing == MMC_TIMING_UHS_DDR50)
+   con |= DDR; /* configure in DDR mode */
+   else
+   con &= ~DDR;
switch (ios->bus_width) {
case MMC_BUS_WIDTH_8:
OMAP_HSMMC_WRITE(host->base, CON, con | DW8);
-- 
1.7.10.rc0.41.gfa678

--
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 RESEND 1/8] mmc: omap: Enable Auto CMD12

[Venkatraman S]

From: Balaji T K 

Enable Auto-CMD12 for multi block read/write on HSMMC
Tested on OMAP4430, OMAP3430 and OMAP2430 SDP

Signed-off-by: Balaji T K 
---
 drivers/mmc/host/omap_hsmmc.c |   17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index f29e1a2..b1e9be7 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -85,6 +85,7 @@
 #define BRR_ENABLE (1 << 5)
 #define DTO_ENABLE (1 << 20)
 #define INIT_STREAM(1 << 1)
+#define ACEN_ACMD12(1 << 2)
 #define DP_SELECT  (1 << 21)
 #define DDIR   (1 << 4)
 #define DMA_EN 0x1
@@ -115,6 +116,7 @@
 #define OMAP_MMC_MAX_CLOCK 5200
 #define DRIVER_NAME"omap_hsmmc"
 
+#define AUTO_CMD12 (1 << 0)/* Auto CMD12 support */
 /*
  * One controller can have multiple slots, like on some omap boards using
  * omap.c controller driver. Luckily this is not currently done on any known
@@ -175,6 +177,7 @@ struct omap_hsmmc_host {
int reqs_blocked;
int use_reg;
int req_in_progress;
+   unsigned intflags;
struct omap_hsmmc_next  next_data;
 
struct  omap_mmc_platform_data  *pdata;
@@ -766,6 +769,8 @@ omap_hsmmc_start_command(struct omap_hsmmc_host *host, 
struct mmc_command *cmd,
cmdtype = 0x3;
 
cmdreg = (cmd->opcode << 24) | (resptype << 16) | (cmdtype << 22);
+   if ((host->flags & AUTO_CMD12) && mmc_op_multi(cmd->opcode))
+   cmdreg |= ACEN_ACMD12;
 
if (data) {
cmdreg |= DP_SELECT | MSBS | BCE;
@@ -837,11 +842,16 @@ omap_hsmmc_xfer_done(struct omap_hsmmc_host *host, struct 
mmc_data *data)
else
data->bytes_xfered = 0;
 
-   if (!data->stop) {
+   if (data->stop && ((!(host->flags & AUTO_CMD12)) || data->error))
+   omap_hsmmc_start_command(host, data->stop, NULL);
+   else {
+   if (data->stop)
+   data->stop->resp[0] = OMAP_HSMMC_READ(host->base,
+   RSP76);
omap_hsmmc_request_done(host, data->mrq);
-   return;
}
-   omap_hsmmc_start_command(host, data->stop, NULL);
+
+   return;
 }
 
 /*
@@ -1826,6 +1836,7 @@ static int __init omap_hsmmc_probe(struct platform_device 
*pdev)
host->mapbase   = res->start;
host->base  = ioremap(host->mapbase, SZ_4K);
host->power_mode = MMC_POWER_OFF;
+   host->flags = AUTO_CMD12;
host->next_data.cookie = 1;
 
platform_set_drvdata(pdev, host);
-- 
1.7.10.rc0.41.gfa678

--
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 0/8][git pull] mmc: omap: Assorted fixes for 3.4 merge window

[Venkatraman S]

Chris,
   Here are a group of fixes posted by Felipe and Balaji for the
OMAP hsmmc driver in the past few days. 
   I've rebased them to the lastest mmc-next and posted them
here again. These have also been tested on OMAP4 development platform.

Please feel to apply directly or pull if that's convenient.

The following changes since commit 5f0390f10c0e9c9c504cdbf4af802252628a2490:

  mmc: omap_hsmmc: Avoid a regulator voltage change with dt (2012-03-14 
11:33:20 -0400)

are available in the git repository at:

  git://github.com/svenkatr/linux.git omap-mmc-pending-for-3.4

for you to fetch changes up to a6caaa13374ab72e37f9cb2e4cebfe3d266fbaf3:

  mmc: omap4: hsmmc: fix module re-insertion (2012-03-15 19:45:49 +0530)


Balaji T K (5):
  mmc: omap: Enable Auto CMD12
  mmc: omap: add DDR support to omap_hsmmc
  mmc: omap: use runtime put sync in probe error patch
  mmc: omap: context save after enabling runtime pm
  mmc: omap4: hsmmc: fix module re-insertion

Felipe Balbi (3):
  mmc: host: omap_hsmmc: trivial cleanups
  mmc: host: omap_hsmmc: make it behave well as a module
  mmc: host: omap_hsmmc: convert to module_platform_driver

 drivers/mmc/host/omap_hsmmc.c |  203 
++---
 1 file changed, 98 insertions(+), 105 deletions(-)

--
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 v1 1/5] ARM: imx28: add basic dt support

[s.ha...@pengutronix.de]

On Thu, Mar 15, 2012 at 06:59:28PM +0800, Dong Aisheng wrote:
> On Thu, Mar 15, 2012 at 02:53:29PM +0800, Lothar Waßmann wrote:
> > Hi,
> > 
> > Dong Aisheng writes:
> > > On Wed, Mar 14, 2012 at 10:16:43PM +0800, s.ha...@pengutronix.de wrote:
> > > > On Wed, Mar 14, 2012 at 08:45:23PM +0800, Dong Aisheng wrote:
> > > > > On Wed, Mar 14, 2012 at 01:23:51AM +0800, Grant Likely wrote:
> > [...]
> > > > > But it seems this needs pass mac address to fec driver via platforom 
> > > > > data which is
> > > > > not friendly to dt.
> > > > > 
> > > > > Another way may be changing fec driver to get the fixed part of mac 
> > > > > address(first
> > > > > two bytes) from device tree and read the left dynamical part from otp 
> > > > > which i'm not
> > > > > sure is better enough.
> > > > > 
> > > > > BTW, filling with zeros seems not work since it's invalid mac address.
> > > > 
> > > > Yes, that's the idea of using this value...
> > > > 
> > > But comparing to use an invalid value, why not just do not define that
> > > local-mac-address property?
> > > 
> > Because a MAC address is by definition a GLOBALLY UNIQUE IDENTIFIER
> > which is contrary to coding a "valid" default value for it somewhere!
> > The owner of a certain MAC address range (OUI) is responsible for
> > ensuring the uniqueness. Thus only they may assign a specific MAC
> > address to a specific device.
> > 
> Yes, you're correct.
> So i propose to read the MAC address from MX28 OTP in fec driver instead of
> define it in device tree in my last mail.
> http://www.spinics.net/lists/arm-kernel/msg165040.html
> Do you have any comment on that way?

Yes, you could do that. Otherwise it's perfectly fine to pass the MAC
address in the device tree, it's just not ok to put specific values
into the kernel source. You could leave the values blank and let the
bootloader fill them in. Or you could fill in valid values before you
compile the devicetree, but be prepared for surprises once you have
two boards in your network and use the devicetree for both.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
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 v1 1/5] ARM: imx28: add basic dt support

[Lothar Waßmann]

Hi,

Dong Aisheng writes:
> On Thu, Mar 15, 2012 at 02:53:29PM +0800, Lothar Waßmann wrote:
> > Hi,
> > 
> > Dong Aisheng writes:
> > > On Wed, Mar 14, 2012 at 10:16:43PM +0800, s.ha...@pengutronix.de wrote:
> > > > On Wed, Mar 14, 2012 at 08:45:23PM +0800, Dong Aisheng wrote:
> > > > > On Wed, Mar 14, 2012 at 01:23:51AM +0800, Grant Likely wrote:
> > [...]
> > > > > But it seems this needs pass mac address to fec driver via platforom 
> > > > > data which is
> > > > > not friendly to dt.
> > > > > 
> > > > > Another way may be changing fec driver to get the fixed part of mac 
> > > > > address(first
> > > > > two bytes) from device tree and read the left dynamical part from otp 
> > > > > which i'm not
> > > > > sure is better enough.
> > > > > 
> > > > > BTW, filling with zeros seems not work since it's invalid mac address.
> > > > 
> > > > Yes, that's the idea of using this value...
> > > > 
> > > But comparing to use an invalid value, why not just do not define that
> > > local-mac-address property?
> > > 
> > Because a MAC address is by definition a GLOBALLY UNIQUE IDENTIFIER
> > which is contrary to coding a "valid" default value for it somewhere!
> > The owner of a certain MAC address range (OUI) is responsible for
> > ensuring the uniqueness. Thus only they may assign a specific MAC
> > address to a specific device.
> > 
> Yes, you're correct.
> So i propose to read the MAC address from MX28 OTP in fec driver instead of
> define it in device tree in my last mail.
> http://www.spinics.net/lists/arm-kernel/msg165040.html
> Do you have any comment on that way?
> 
That patch sets the OUI to the Freescale owned MAC range. Thus only
Freescale products may be able to use the driver.

Anyway there is no definite spec how the MAC address(es) are stored
in the fuse map. Thus reading the MAC from there is more or less
platform specific.

Currently I'm setting up the MAC address for our TX28 from the fuses
in the platform code passed via platform_data, but that will obviously
not work with DT.

The correct way would probably be to pass the MAC from the bootloader
via a DT blob. But that would require all bootloaders to be updated
first to support DTS. :(

An intermediate solution could be using OF_DEV_AUXDATA to pass a
platform_data struct that may contain a MAC address set up by the
platform code.


Lothar Waßmann
-- 
___

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | i...@karo-electronics.de
___
--
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 v1 1/5] ARM: imx28: add basic dt support

[Dong Aisheng]

On Thu, Mar 15, 2012 at 02:53:29PM +0800, Lothar Waßmann wrote:
> Hi,
> 
> Dong Aisheng writes:
> > On Wed, Mar 14, 2012 at 10:16:43PM +0800, s.ha...@pengutronix.de wrote:
> > > On Wed, Mar 14, 2012 at 08:45:23PM +0800, Dong Aisheng wrote:
> > > > On Wed, Mar 14, 2012 at 01:23:51AM +0800, Grant Likely wrote:
> [...]
> > > > But it seems this needs pass mac address to fec driver via platforom 
> > > > data which is
> > > > not friendly to dt.
> > > > 
> > > > Another way may be changing fec driver to get the fixed part of mac 
> > > > address(first
> > > > two bytes) from device tree and read the left dynamical part from otp 
> > > > which i'm not
> > > > sure is better enough.
> > > > 
> > > > BTW, filling with zeros seems not work since it's invalid mac address.
> > > 
> > > Yes, that's the idea of using this value...
> > > 
> > But comparing to use an invalid value, why not just do not define that
> > local-mac-address property?
> > 
> Because a MAC address is by definition a GLOBALLY UNIQUE IDENTIFIER
> which is contrary to coding a "valid" default value for it somewhere!
> The owner of a certain MAC address range (OUI) is responsible for
> ensuring the uniqueness. Thus only they may assign a specific MAC
> address to a specific device.
> 
Yes, you're correct.
So i propose to read the MAC address from MX28 OTP in fec driver instead of
define it in device tree in my last mail.
http://www.spinics.net/lists/arm-kernel/msg165040.html
Do you have any comment on that way?

Regards
Dong Aisheng

--
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 v1 1/5] ARM: imx28: add basic dt support

[Lothar Waßmann]

Hi,

Dong Aisheng writes:
> On Wed, Mar 14, 2012 at 10:16:43PM +0800, s.ha...@pengutronix.de wrote:
> > On Wed, Mar 14, 2012 at 08:45:23PM +0800, Dong Aisheng wrote:
> > > On Wed, Mar 14, 2012 at 01:23:51AM +0800, Grant Likely wrote:
[...]
> > > But it seems this needs pass mac address to fec driver via platforom data 
> > > which is
> > > not friendly to dt.
> > > 
> > > Another way may be changing fec driver to get the fixed part of mac 
> > > address(first
> > > two bytes) from device tree and read the left dynamical part from otp 
> > > which i'm not
> > > sure is better enough.
> > > 
> > > BTW, filling with zeros seems not work since it's invalid mac address.
> > 
> > Yes, that's the idea of using this value...
> > 
> But comparing to use an invalid value, why not just do not define that
> local-mac-address property?
> 
Because a MAC address is by definition a GLOBALLY UNIQUE IDENTIFIER
which is contrary to coding a "valid" default value for it somewhere!
The owner of a certain MAC address range (OUI) is responsible for
ensuring the uniqueness. Thus only they may assign a specific MAC
address to a specific device.


Lothar Waßmann
-- 
___

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | i...@karo-electronics.de
___
--
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