Linux MultiMedia Card development
 help / color / mirror / Atom feed
* Re: [PATCH 1/1] mmc: mmc: Relax checking for switch errors after HS200 switch
From: Jaehoon Chung @ 2016-12-01  9:18 UTC (permalink / raw)
  To: Adrian Hunter, Ulf Hansson
  Cc: linux-mmc, Chaotian Jing, Stephen Boyd, Michael Walle, Yong Mao,
	Shawn Lin
In-Reply-To: <49e45a44-64ff-3898-ef44-aaef0f103ca5@intel.com>

Hi Adrian,

On 12/01/2016 05:15 PM, Adrian Hunter wrote:
> On 24/11/16 14:02, Adrian Hunter wrote:
>> The JEDEC specification indicates CMD13 can be used after a HS200 switch
>> to check for errors. However in practice some boards experience CRC errors
>> in the CMD13 response. Consequently, for HS200, CRC errors are not a
>> reliable way to know the switch failed. If there really is a problem, we
>> would expect tuning will fail and the result ends up the same. So change
>> the error condition to ignore CRC errors in that case.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> 
> Any comments on this?

I agreed your opinion..CMD13 can't know whether switch is failed or not with CRC error.
It might just know whether HS200 is working fine or not with CRC error.

If CRC error is occurred, then user can knows when transfer some data.
Then i think it's more easier to debug than now..does it make sense?

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> 
> 
>> ---
>>  drivers/mmc/core/mmc.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 3268fcd3378d..34d30e2a09ff 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1223,7 +1223,12 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
>>  	mmc_set_timing(host, MMC_TIMING_MMC_HS200);
>>  
>>  	err = mmc_switch_status(card);
>> -	if (err)
>> +	/*
>> +	 * For HS200, CRC errors are not a reliable way to know the switch
>> +	 * failed. If there really is a problem, we would expect tuning will
>> +	 * fail and the result ends up the same.
>> +	 */
>> +	if (err && err != -EILSEQ)
>>  		goto out_err;
>>  
>>  	mmc_set_bus_speed(card);
>> @@ -1387,6 +1392,14 @@ static int mmc_select_hs200(struct mmc_card *card)
>>  
>>  		err = mmc_switch_status(card);
>>  		/*
>> +		 * For HS200, CRC errors are not a reliable way to know the
>> +		 * switch failed. If there really is a problem, we would expect
>> +		 * tuning will fail and the result ends up the same.
>> +		 */
>> +		if (err == -EILSEQ)
>> +			err = 0;
>> +
>> +		/*
>>  		 * mmc_select_timing() assumes timing has not changed if
>>  		 * it is a switch error.
>>  		 */
>>
> 
> 
> 
> 


^ permalink raw reply

* Re: [PATCH 1/1] mmc: mmc: Relax checking for switch errors after HS200 switch
From: Adrian Hunter @ 2016-12-01  8:15 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: linux-mmc, Jaehoon Chung, Chaotian Jing, Stephen Boyd,
	Michael Walle, Yong Mao, Shawn Lin
In-Reply-To: <1479988969-1747-2-git-send-email-adrian.hunter@intel.com>

On 24/11/16 14:02, Adrian Hunter wrote:
> The JEDEC specification indicates CMD13 can be used after a HS200 switch
> to check for errors. However in practice some boards experience CRC errors
> in the CMD13 response. Consequently, for HS200, CRC errors are not a
> reliable way to know the switch failed. If there really is a problem, we
> would expect tuning will fail and the result ends up the same. So change
> the error condition to ignore CRC errors in that case.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Any comments on this?


> ---
>  drivers/mmc/core/mmc.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 3268fcd3378d..34d30e2a09ff 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1223,7 +1223,12 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
>  	mmc_set_timing(host, MMC_TIMING_MMC_HS200);
>  
>  	err = mmc_switch_status(card);
> -	if (err)
> +	/*
> +	 * For HS200, CRC errors are not a reliable way to know the switch
> +	 * failed. If there really is a problem, we would expect tuning will
> +	 * fail and the result ends up the same.
> +	 */
> +	if (err && err != -EILSEQ)
>  		goto out_err;
>  
>  	mmc_set_bus_speed(card);
> @@ -1387,6 +1392,14 @@ static int mmc_select_hs200(struct mmc_card *card)
>  
>  		err = mmc_switch_status(card);
>  		/*
> +		 * For HS200, CRC errors are not a reliable way to know the
> +		 * switch failed. If there really is a problem, we would expect
> +		 * tuning will fail and the result ends up the same.
> +		 */
> +		if (err == -EILSEQ)
> +			err = 0;
> +
> +		/*
>  		 * mmc_select_timing() assumes timing has not changed if
>  		 * it is a switch error.
>  		 */
> 


^ permalink raw reply

* Re: [PATCH] mmc: sdhci-s3c: add spin_unlock_irq() before calling clk_round_rate
From: Ulf Hansson @ 2016-12-01  8:14 UTC (permalink / raw)
  To: Jaehoon Chung; +Cc: linux-mmc@vger.kernel.org, Adrian Hunter
In-Reply-To: <20161130060542.18225-1-jh80.chung@samsung.com>

On 30 November 2016 at 07:05, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Before calling clk_round_rate(), put the spin_unlock_irq() in
> sdhci_s3c_consider_clock() function.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci-s3c.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index 784c5a8..de219ca 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -121,7 +121,9 @@ static unsigned int sdhci_s3c_consider_clock(struct sdhci_s3c *ourhost,
>          * speed possible with selected clock source and skip the division.
>          */
>         if (ourhost->no_divider) {
> +               spin_unlock_irq(&ourhost->host->lock);
>                 rate = clk_round_rate(clksrc, wanted);
> +               spin_lock_irq(&ourhost->host->lock);
>                 return wanted - rate;
>         }
>
> --
> 2.10.2
>

^ permalink raw reply

* Re: [PATCH v3 2/2] mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio controller sub-vended by NI
From: Ulf Hansson @ 2016-12-01  8:14 UTC (permalink / raw)
  To: Zach Brown
  Cc: Adrian Hunter, linux-mmc@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <1480360599-13295-3-git-send-email-zach.brown@ni.com>

On 28 November 2016 at 20:16, Zach Brown <zach.brown@ni.com> wrote:
> On NI 9037 boards the max SDIO frequency is limited by trace lengths
> and other layout choices. The max SDIO frequency is stored in an ACPI
> table.
>
> The driver reads the ACPI entry MXFQ during sdio_probe_slot and sets the
> f_max field of the host.
>
> Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
> Reviewed-by: Jaeden Amero <jaeden.amero@ni.com>
> Reviewed-by: Josh Cartwright <joshc@ni.com>
> Signed-off-by: Zach Brown <zach.brown@ni.com>

Thanks, applied for next!

Kind regards
Uffe


> ---
>  drivers/mmc/host/sdhci-pci-core.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 9741505..c9e51b1 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -27,6 +27,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/mmc/slot-gpio.h>
>  #include <linux/mmc/sdhci-pci-data.h>
> +#include <linux/acpi.h>
>
>  #include "sdhci.h"
>  #include "sdhci-pci.h"
> @@ -375,8 +376,39 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
>         return 0;
>  }
>
> +#ifdef CONFIG_ACPI
> +static int ni_set_max_freq(struct sdhci_pci_slot *slot)
> +{
> +       acpi_status status;
> +       unsigned long long max_freq;
> +
> +       status = acpi_evaluate_integer(ACPI_HANDLE(&slot->chip->pdev->dev),
> +                                      "MXFQ", NULL, &max_freq);
> +       if (ACPI_FAILURE(status)) {
> +               dev_err(&slot->chip->pdev->dev,
> +                       "MXFQ not found in acpi table\n");
> +               return -EINVAL;
> +       }
> +
> +       slot->host->mmc->f_max = max_freq * 1000000;
> +
> +       return 0;
> +}
> +#else
> +static inline int ni_set_max_freq(struct sdhci_pci_slot *slot)
> +{
> +       return 0;
> +}
> +#endif
> +
>  static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>  {
> +       int err;
> +
> +       err = ni_set_max_freq(slot);
> +       if (err)
> +               return err;
> +
>         slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
>                                  MMC_CAP_WAIT_WHILE_BUSY;
>         return 0;
> --
> 2.7.4
>

^ permalink raw reply

* Re: [PATCH v3 1/2] mmc: sdhci-pci: Add PCI ID for Intel byt sdio host controller sub-vended by NI
From: Ulf Hansson @ 2016-12-01  8:14 UTC (permalink / raw)
  To: Zach Brown
  Cc: Adrian Hunter, linux-mmc@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <1480360599-13295-2-git-send-email-zach.brown@ni.com>

On 28 November 2016 at 20:16, Zach Brown <zach.brown@ni.com> wrote:
> Add PCI ID for Intel byt sdio host controller sub-vended by NI.
>
> The controller has different behavior because of the board layout NI
> puts it on.
>
> Signed-off-by: Zach Brown <zach.brown@ni.com>

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci-pci-core.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 1d9e00a..9741505 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -375,6 +375,13 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
>         return 0;
>  }
>
> +static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
> +{
> +       slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
> +                                MMC_CAP_WAIT_WHILE_BUSY;
> +       return 0;
> +}
> +
>  static int byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>  {
>         slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
> @@ -447,6 +454,15 @@ static const struct sdhci_pci_fixes sdhci_intel_byt_emmc = {
>         .ops            = &sdhci_intel_byt_ops,
>  };
>
> +static const struct sdhci_pci_fixes sdhci_ni_byt_sdio = {
> +       .quirks         = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
> +       .quirks2        = SDHCI_QUIRK2_HOST_OFF_CARD_ON |
> +                         SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> +       .allow_runtime_pm = true,
> +       .probe_slot     = ni_byt_sdio_probe_slot,
> +       .ops            = &sdhci_intel_byt_ops,
> +};
> +
>  static const struct sdhci_pci_fixes sdhci_intel_byt_sdio = {
>         .quirks         = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
>         .quirks2        = SDHCI_QUIRK2_HOST_OFF_CARD_ON |
> @@ -1079,6 +1095,14 @@ static const struct pci_device_id pci_ids[] = {
>         {
>                 .vendor         = PCI_VENDOR_ID_INTEL,
>                 .device         = PCI_DEVICE_ID_INTEL_BYT_SDIO,
> +               .subvendor      = PCI_VENDOR_ID_NI,
> +               .subdevice      = 0x7884,
> +               .driver_data    = (kernel_ulong_t)&sdhci_ni_byt_sdio,
> +       },
> +
> +       {
> +               .vendor         = PCI_VENDOR_ID_INTEL,
> +               .device         = PCI_DEVICE_ID_INTEL_BYT_SDIO,
>                 .subvendor      = PCI_ANY_ID,
>                 .subdevice      = PCI_ANY_ID,
>                 .driver_data    = (kernel_ulong_t)&sdhci_intel_byt_sdio,
> --
> 2.7.4
>

^ permalink raw reply

* Re: [PATCH 2/4] mmc: sdhci: Fix recovery from tuning timeout
From: Ulf Hansson @ 2016-12-01  8:01 UTC (permalink / raw)
  To: Adrian Hunter; +Cc: linux-mmc, Dan O'Donovan
In-Reply-To: <aa7368ef-4711-4b03-c9cb-df09344f2cb5@intel.com>

On 30 November 2016 at 11:17, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 30/11/16 12:06, Ulf Hansson wrote:
>> On 30 November 2016 at 10:20, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> Clearing the tuning bits should reset the tuning circuit. However there is
>>> more to do. Reset the command and data lines for good measure, and then
>>> for eMMC ensure the card is not still trying to process a tuning command by
>>> sending a stop command.
>>>
>>> Note the JEDEC eMMC specification says the stop command (CMD12) can be used
>>> to stop a tuning command (CMD21) whereas the SD specification is silent on
>>> the subject with respect to the SD tuning command (CMD19). Considering that
>>> CMD12 is not a valid SDIO command, the stop command is sent only when the
>>> tuning command is CMD21 i.e. for eMMC. That addresses cases seen so far
>>> which have been on eMMC.
>>>
>>> Note that this replaces the commit fe5fb2e3b58f ("mmc: sdhci: Reset cmd and
>>> data circuits after tuning failure") which is being reverted for v4.9+.
>>>
>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>> Tested-by: Dan O'Donovan <dan@emutex.com>
>>> Cc: stable@vger.kernel.org
>>> ---
>>>  drivers/mmc/host/sdhci.c | 20 ++++++++++++++++++++
>>>  1 file changed, 20 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index e761fe2aa99e..1d72a51287d4 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -2095,7 +2095,27 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
>>>                         ctrl &= ~SDHCI_CTRL_EXEC_TUNING;
>>>                         sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
>>>
>>> +                       sdhci_do_reset(host, SDHCI_RESET_CMD);
>>> +                       sdhci_do_reset(host, SDHCI_RESET_DATA);
>>> +
>>>                         err = -EIO;
>>> +
>>> +                       if (cmd.opcode != MMC_SEND_TUNING_BLOCK_HS200)
>>> +                               goto out;
>>> +
>>> +                       sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
>>> +                       sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>>> +
>>> +                       spin_unlock_irqrestore(&host->lock, flags);
>>> +
>>> +                       memset(&cmd, 0, sizeof(cmd));
>>> +                       cmd.opcode = MMC_STOP_TRANSMISSION;
>>> +                       cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
>>> +                       cmd.busy_timeout = 50;
>>> +                       mmc_wait_for_cmd(mmc, &cmd, 0);
>>
>> No, please don't add more hacks to send commands internally from sdhci.
>>
>> Maybe even before you start fix the problems for tuning, perhaps you
>> try to clean up the current code when sending CMD21/19 in
>> sdhci_execute_tuning()?
>>
>> Moreover, according to the change log above, it seems like a generic
>> thing to send CMD12 to abort tuning. In such case, we could either
>> make the core deal with it in the error path - or we could implement a
>> "mmc_abort_tuning()" function, host drivers may call when needed.
>
> I am not sure a cleanup would apply cleanly to stable trees.  It would be
> nicer to have these patches for stable and then a cleanup on top.  Would
> that be acceptable?
>

Yes. That's ok. Can you please re-spin with cleanups on top then.

Kind regards
Uffe

^ permalink raw reply

* Re: [PATCH] mmc: queue: add #ifdef around bounce buffer code
From: Ulf Hansson @ 2016-12-01  8:00 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linus Walleij, Adrian Hunter, Jens Axboe, Harjani Ritesh,
	linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20161130210427.1721327-1-arnd@arndb.de>

On 30 November 2016 at 22:03, Arnd Bergmann <arnd@arndb.de> wrote:
> The two functions that got split out from the bounce
> buffer handling are now unused when CONFIG_MMC_BLOCK_BOUNCE
> is disabled:
>
> mmc/card/queue.c:212:12: error: 'mmc_queue_alloc_bounce_sgs' defined but not used [-Werror=unused-function]
> mmc/card/queue.c:189:13: error: 'mmc_queue_alloc_bounce_bufs' defined but not used [-Werror=unused-function]
>
> This adds a new #ifdef around them.
>
> Fixes: 0a6321310492 ("mmc: queue: Factor out mmc_queue_alloc_bounce_bufs()")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Thanks for the fix. This time I have already amended the changes that
causes the problem and pushed them for next by yesterday.

Kind regards
Uffe

> ---
>  drivers/mmc/card/queue.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
> index 8d10fab701b6..6ae6bfb8b221 100644
> --- a/drivers/mmc/card/queue.c
> +++ b/drivers/mmc/card/queue.c
> @@ -186,6 +186,7 @@ static void mmc_queue_setup_discard(struct request_queue *q,
>                 queue_flag_set_unlocked(QUEUE_FLAG_SECERASE, q);
>  }
>
> +#ifdef CONFIG_MMC_BLOCK_BOUNCE
>  static bool mmc_queue_alloc_bounce_bufs(struct mmc_queue *mq,
>                                         unsigned int bouncesz)
>  {
> @@ -226,6 +227,7 @@ static int mmc_queue_alloc_bounce_sgs(struct mmc_queue *mq,
>
>         return 0;
>  }
> +#endif
>
>  static int mmc_queue_alloc_sgs(struct mmc_queue *mq, int max_segs)
>  {
> --
> 2.9.0
>

^ permalink raw reply

* Re: [PATCH v3 2/2] mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio controller sub-vended by NI
From: Adrian Hunter @ 2016-12-01  7:52 UTC (permalink / raw)
  To: Zach Brown, ulf.hansson; +Cc: linux-mmc, linux-kernel
In-Reply-To: <1480360599-13295-3-git-send-email-zach.brown@ni.com>

On 28/11/16 21:16, Zach Brown wrote:
> On NI 9037 boards the max SDIO frequency is limited by trace lengths
> and other layout choices. The max SDIO frequency is stored in an ACPI
> table.
> 
> The driver reads the ACPI entry MXFQ during sdio_probe_slot and sets the
> f_max field of the host.
> 
> Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
> Reviewed-by: Jaeden Amero <jaeden.amero@ni.com>
> Reviewed-by: Josh Cartwright <joshc@ni.com>
> Signed-off-by: Zach Brown <zach.brown@ni.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-pci-core.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 9741505..c9e51b1 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -27,6 +27,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/mmc/slot-gpio.h>
>  #include <linux/mmc/sdhci-pci-data.h>
> +#include <linux/acpi.h>
>  
>  #include "sdhci.h"
>  #include "sdhci-pci.h"
> @@ -375,8 +376,39 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_ACPI
> +static int ni_set_max_freq(struct sdhci_pci_slot *slot)
> +{
> +	acpi_status status;
> +	unsigned long long max_freq;
> +
> +	status = acpi_evaluate_integer(ACPI_HANDLE(&slot->chip->pdev->dev),
> +				       "MXFQ", NULL, &max_freq);
> +	if (ACPI_FAILURE(status)) {
> +		dev_err(&slot->chip->pdev->dev,
> +			"MXFQ not found in acpi table\n");
> +		return -EINVAL;
> +	}
> +
> +	slot->host->mmc->f_max = max_freq * 1000000;
> +
> +	return 0;
> +}
> +#else
> +static inline int ni_set_max_freq(struct sdhci_pci_slot *slot)
> +{
> +	return 0;
> +}
> +#endif
> +
>  static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>  {
> +	int err;
> +
> +	err = ni_set_max_freq(slot);
> +	if (err)
> +		return err;
> +
>  	slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
>  				 MMC_CAP_WAIT_WHILE_BUSY;
>  	return 0;
> 


^ permalink raw reply

* Re: [PATCH v3 1/2] mmc: sdhci-pci: Add PCI ID for Intel byt sdio host controller sub-vended by NI
From: Adrian Hunter @ 2016-12-01  7:52 UTC (permalink / raw)
  To: Zach Brown, ulf.hansson; +Cc: linux-mmc, linux-kernel
In-Reply-To: <1480360599-13295-2-git-send-email-zach.brown@ni.com>

On 28/11/16 21:16, Zach Brown wrote:
> Add PCI ID for Intel byt sdio host controller sub-vended by NI.
> 
> The controller has different behavior because of the board layout NI
> puts it on.
> 
> Signed-off-by: Zach Brown <zach.brown@ni.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-pci-core.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 1d9e00a..9741505 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -375,6 +375,13 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
>  	return 0;
>  }
>  
> +static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
> +{
> +	slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
> +				 MMC_CAP_WAIT_WHILE_BUSY;
> +	return 0;
> +}
> +
>  static int byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>  {
>  	slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
> @@ -447,6 +454,15 @@ static const struct sdhci_pci_fixes sdhci_intel_byt_emmc = {
>  	.ops		= &sdhci_intel_byt_ops,
>  };
>  
> +static const struct sdhci_pci_fixes sdhci_ni_byt_sdio = {
> +	.quirks		= SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
> +	.quirks2	= SDHCI_QUIRK2_HOST_OFF_CARD_ON |
> +			  SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> +	.allow_runtime_pm = true,
> +	.probe_slot	= ni_byt_sdio_probe_slot,
> +	.ops		= &sdhci_intel_byt_ops,
> +};
> +
>  static const struct sdhci_pci_fixes sdhci_intel_byt_sdio = {
>  	.quirks		= SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
>  	.quirks2	= SDHCI_QUIRK2_HOST_OFF_CARD_ON |
> @@ -1079,6 +1095,14 @@ static const struct pci_device_id pci_ids[] = {
>  	{
>  		.vendor		= PCI_VENDOR_ID_INTEL,
>  		.device		= PCI_DEVICE_ID_INTEL_BYT_SDIO,
> +		.subvendor	= PCI_VENDOR_ID_NI,
> +		.subdevice	= 0x7884,
> +		.driver_data	= (kernel_ulong_t)&sdhci_ni_byt_sdio,
> +	},
> +
> +	{
> +		.vendor		= PCI_VENDOR_ID_INTEL,
> +		.device		= PCI_DEVICE_ID_INTEL_BYT_SDIO,
>  		.subvendor	= PCI_ANY_ID,
>  		.subdevice	= PCI_ANY_ID,
>  		.driver_data	= (kernel_ulong_t)&sdhci_intel_byt_sdio,
> 

^ permalink raw reply

* Re: [PATCH] mmc: sdhci-s3c: add spin_unlock_irq() before calling clk_round_rate
From: Adrian Hunter @ 2016-12-01  6:58 UTC (permalink / raw)
  To: Jaehoon Chung, linux-mmc; +Cc: ulf.hansson
In-Reply-To: <20161130060542.18225-1-jh80.chung@samsung.com>

On 30/11/16 08:05, Jaehoon Chung wrote:
> Before calling clk_round_rate(), put the spin_unlock_irq() in
> sdhci_s3c_consider_clock() function.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-s3c.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index 784c5a8..de219ca 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -121,7 +121,9 @@ static unsigned int sdhci_s3c_consider_clock(struct sdhci_s3c *ourhost,
>  	 * speed possible with selected clock source and skip the division.
>  	 */
>  	if (ourhost->no_divider) {
> +		spin_unlock_irq(&ourhost->host->lock);
>  		rate = clk_round_rate(clksrc, wanted);
> +		spin_lock_irq(&ourhost->host->lock);
>  		return wanted - rate;
>  	}
>  
> 


^ permalink raw reply

* Re: [PATCH] mmc: sdhci-s3c: add spin_unlock_irq() before calling clk_round_rate
From: Jaehoon Chung @ 2016-12-01  4:31 UTC (permalink / raw)
  To: linux-mmc; +Cc: ulf.hansson, adrian.hunter
In-Reply-To: <20161130060542.18225-1-jh80.chung@samsung.com>

On 11/30/2016 03:05 PM, Jaehoon Chung wrote:
> Before calling clk_round_rate(), put the spin_unlock_irq() in
> sdhci_s3c_consider_clock() function.

this patch is for fixing the below issue.

  273.465261] BUG: scheduling while atomic: mmcqd/0/96/0x00000002
 [  273.469782] Modules linked in:
 [  273.472819] Preemption disabled at:[  273.476121] [<  (null)>]   (null)
 [  273.479424] CPU: 3 PID: 96 Comm: mmcqd/0 Not tainted 4.9.0-rc5-00106-g4d698fc-dirty #6
 [  273.487316] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
 [  273.493419] [<c010f0f4>] (unwind_backtrace) from [<c010ae24>] (show_stack+0x10/0x14)
 [  273.501127] [<c010ae24>] (show_stack) from [<c033cb68>] (dump_stack+0x78/0x8c)
 [  273.508333] [<c033cb68>] (dump_stack) from [<c013ddd4>] (__schedule_bug+0x84/0xd4)
 [  273.515884] [<c013ddd4>] (__schedule_bug) from [<c072e788>] (__schedule+0x398/0x480)
 [  273.523605] [<c072e788>] (__schedule) from [<c072e8bc>] (schedule+0x4c/0xac)
 [  273.530634] [<c072e8bc>] (schedule) from [<c072ed24>] (schedule_preempt_disabled+0x14/0x20)
 [  273.538969] [<c072ed24>] (schedule_preempt_disabled) from [<c07312a4>] (__mutex_lock_slowpath+0x19c/0x3fc)
 [  273.548603] [<c07312a4>] (__mutex_lock_slowpath) from [<c0731510>] (mutex_lock+0xc/0x24)
 [  273.556678] [<c0731510>] (mutex_lock) from [<c03a7614>] (clk_prepare_lock+0x50/0xf8)
 [  273.564403] [<c03a7614>] (clk_prepare_lock) from [<c03a8c34>] (clk_round_rate+0x18/0x60)
 [  273.572475] [<c03a8c34>] (clk_round_rate) from [<c057e274>] (sdhci_s3c_set_clock+0x1e0/0x1e8)
 [  273.580980] [<c057e274>] (sdhci_s3c_set_clock) from [<c057e2a0>] (sdhci_cmu_set_clock+0x24/0x17c)
 [  273.589831] [<c057e2a0>] (sdhci_cmu_set_clock) from [<c057d4d0>] (sdhci_set_ios+0x7c/0x408)
 [  273.598164] [<c057d4d0>] (sdhci_set_ios) from [<c057d378>] (sdhci_runtime_resume_host+0x70/0x14c)
 [  273.607025] [<c057d378>] (sdhci_runtime_resume_host) from [<c044a690>] (__rpm_callback+0x2c/0x60)
 [  273.615875] [<c044a690>] (__rpm_callback) from [<c044a71c>] (rpm_callback+0x58/0x80)
 [  273.623599] [<c044a71c>] (rpm_callback) from [<c044b5cc>] (rpm_resume+0x35c/0x560)
 [  273.631150] [<c044b5cc>] (rpm_resume) from [<c044b81c>] (__pm_runtime_resume+0x4c/0x64)
 [  273.639137] [<c044b81c>] (__pm_runtime_resume) from [<c056718c>] (__mmc_claim_host+0x17c/0x1b8)
 [  273.647821] [<c056718c>] (__mmc_claim_host) from [<c0578094>] (mmc_blk_issue_rq+0x2cc/0x570)
 [  273.656236] [<c0578094>] (mmc_blk_issue_rq) from [<c0578410>] (mmc_queue_thread+0xd8/0x188)
 [  273.664571] [<c0578410>] (mmc_queue_thread) from [<c0138bf8>] (kthread+0xdc/0xf4)
 [  273.672035] [<c0138bf8>] (kthread) from [<c0107878>] (ret_from_fork+0x14/0x3c)
 [  273.680046] ------------[ cut here ]------------
 [  273.684681] WARNING: CPU: 3 PID: 96 at kernel/sched/core.c:3182 _raw_spin_unlock_irq+0x14/0x40
 [  273.692452] DEBUG_LOCKS_WARN_ON(val > preempt_count())
 Modules linked in:
 [  273.699285] CPU: 3 PID: 96 Comm: mmcqd/0 Tainted: G        W       4.9.0-rc5-00106-g4d698fc-dirty #6
 [  273.708394] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
 [  273.714488] [<c010f0f4>] (unwind_backtrace) from [<c010ae24>] (show_stack+0x10/0x14)
 [  273.722202] [<c010ae24>] (show_stack) from [<c033cb68>] (dump_stack+0x78/0x8c)
 [  273.729406] [<c033cb68>] (dump_stack) from [<c011d3ec>] (__warn+0xe8/0x100)
 [  273.736349] [<c011d3ec>] (__warn) from [<c011d43c>] (warn_slowpath_fmt+0x38/0x48)
 [  273.743814] [<c011d43c>] (warn_slowpath_fmt) from [<c0733404>] (_raw_spin_unlock_irq+0x14/0x40)
 [  273.752496] [<c0733404>] (_raw_spin_unlock_irq) from [<c057e2e0>] (sdhci_cmu_set_clock+0x64/0x17c)
 [  273.761433] [<c057e2e0>] (sdhci_cmu_set_clock) from [<c057d4d0>] (sdhci_set_ios+0x7c/0x408)
 [  273.769765] [<c057d4d0>] (sdhci_set_ios) from [<c057d378>] (sdhci_runtime_resume_host+0x70/0x14c)
 [  273.778623] [<c057d378>] (sdhci_runtime_resume_host) from [<c044a690>] (__rpm_callback+0x2c/0x60)
 [  273.787475] [<c044a690>] (__rpm_callback) from [<c044a71c>] (rpm_callback+0x58/0x80)
 [  273.795199] [<c044a71c>] (rpm_callback) from [<c044b5cc>] (rpm_resume+0x35c/0x560)
 [  273.802750] [<c044b5cc>] (rpm_resume) from [<c044b81c>] (__pm_runtime_resume+0x4c/0x64)
 [  273.810737] [<c044b81c>] (__pm_runtime_resume) from [<c056718c>] (__mmc_claim_host+0x17c/0x1b8)
 [  273.819420] [<c056718c>] (__mmc_claim_host) from [<c0578094>] (mmc_blk_issue_rq+0x2cc/0x570)
 [  273.827837] [<c0578094>] (mmc_blk_issue_rq) from [<c0578410>] (mmc_queue_thread+0xd8/0x188)
 [  273.836171] [<c0578410>] (mmc_queue_thread) from [<c0138bf8>] (kthread+0xdc/0xf4)
 [  273.843635] [<c0138bf8>] (kthread) from [<c0107878>] (ret_from_fork+0x14/0x3c)

If my approach is wrong, let me know, plz.

Best Regards,
Jaehoon Chung

> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/mmc/host/sdhci-s3c.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index 784c5a8..de219ca 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -121,7 +121,9 @@ static unsigned int sdhci_s3c_consider_clock(struct sdhci_s3c *ourhost,
>  	 * speed possible with selected clock source and skip the division.
>  	 */
>  	if (ourhost->no_divider) {
> +		spin_unlock_irq(&ourhost->host->lock);
>  		rate = clk_round_rate(clksrc, wanted);
> +		spin_lock_irq(&ourhost->host->lock);
>  		return wanted - rate;
>  	}
>  
> 


^ permalink raw reply

* Re: arasan,sdhci.txt "compatibility" DT binding
From: Shawn Lin @ 2016-12-01  4:09 UTC (permalink / raw)
  To: Mason, linux-mmc, Adrian Hunter
  Cc: shawn.lin, Michal Simek, Rameshwar Sahu, Linux ARM,
	Soren Brinkmann, Michal Simek, Anton Vorontsov, Xiaobo Xie,
	Suman Tripathi, Linus Walleij, Maxime Ripard, Arnd Bergmann,
	Rob Herring, Zach Brown, Ulf Hansson, Douglas Anderson,
	Heiko Stuebner, Jisheng Zhang, Suneel Garapati, Russell King
In-Reply-To: <583C50E7.6030400@free.fr>

On 2016/11/28 23:44, Mason wrote:
> Hello,
>
> @Shawn Lin, could you take a look below and tell me exactly
> which IP core(s) Rockchip is using in its SoCs?
>

 From the Host Controller version register (0xfe)
bit[7:0]: 0x2 : specification version number is 3.00
bit[15:8]: 0x10: Vendor version number is 1.0


Command Queueing version register (0x200)
bit[11:8]: 0x5  eMMC Major version number
bit[7:4]: 0x1 eMMC manor version number
bit[3:0]: 0x0 eMMC version suffix

User guide "eMMC 5.1/SD3.0/SDIO3.0 Host Controller"
Revision number: 1.14
Released on Dec. 2014

> Based on the feedback I received, here is an updated list of
> compatible strings and controller versions dealt with by the
> drivers/mmc/host/sdhci-of-arasan.c code.
>
>
> Xilinx Zynq:
> "SD2.0 / SDIO2.0 / MMC3.31 AHB Host Controller"
> "arasan,sdhci-8.9a"
> NB: 8.9a is the documentation revision (dated 2011-10-19)
> subsequent tweaks labeled 9.0a, 9.1a, 9.2a
>
> Xilinx ZynqMP:
> "SD3.0 / SDIO3.0 / eMMC4.51 AHB Host Controller"
> "arasan,sdhci-8.9a"
> NB: using the same compatible string as Zynq
>
> Sigma SMP87xx
> "SD3.0 / SDIO3.0 / eMMC4.4 AHB Host Controller"
> no compatible string yet, platform-specific init required
>
> APM:
> "SD3.0 / SDIO3.0 / eMMC4.41 AHB Host Controller"
> "arasan,sdhci-4.9a"
> NB: 4.9a appears to be the documentation revision
> no functional diff with "arasan,sdhci-8.9a"
>
> Rockchip
> Exact IP unknown, waiting for Shawn's answer
> "arasan,sdhci-5.1"
> NB: 5.1 appears to refer to the eMMC standard supported
>
>
> On a final note, there are many variations of the Arasan IP.
> I've tracked down at least the following:
>
> SD_2.0_SDIO_2.0__MMC_3.31_AHB_Host_Controller.pdf
> SD_3.0_SDIO_3.0_eMMC_4.41_OCP_Host_Controller.pdf
> SD_3.0_SDIO_3.0_eMMC_4.4__AHB_Host_Controller.pdf
> SD_3.0_SDIO_3.0_eMMC_4.51_Host_Controller.pdf
> SD_3.0_SDIO_3.0_eMMC_4.5__Host_Controller.pdf
> SD_4.1_SDIO_4.1_eMMC_4.51_Host_Controller.pdf
> SD_4.1_SDIO_4.1_eMMC_5.1__Host_Controller.pdf
>
> It seems to me the compatible string should specify
> the SD/SDIO version AND the eMMC version, since it
> seems many combinations are allowed, e.g. eMMC 4.51
> has two possible SD versions.
>
> What do you think?
>
> Regards.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Best Regards
Shawn Lin


^ permalink raw reply

* [PATCH] mmc: sdhci-of-at91: remove bogus MMC_SDHCI_IO_ACCESSORS select
From: Masahiro Yamada @ 2016-12-01  4:05 UTC (permalink / raw)
  To: linux-mmc
  Cc: Masahiro Yamada, Adrian Hunter, linux-kernel, Stefan Wahren,
	Al Cooper, Wolfram Sang, Andrei Pistirica, Simon Horman,
	Geert Uytterhoeven, Ulf Hansson

I see no override of read/write callbacks in sdhci-of-at91.c.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

BTW, this config may not be so useful in recent multi-platforms.
Perhaps, is it better to remove this config entirely instead of
the AT91 fix only.


 drivers/mmc/host/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 39f6f96..8ac1640 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -135,7 +135,6 @@ config MMC_SDHCI_OF_AT91
 	tristate "SDHCI OF support for the Atmel SDMMC controller"
 	depends on MMC_SDHCI_PLTFM
 	depends on OF
-	select MMC_SDHCI_IO_ACCESSORS
 	help
 	  This selects the Atmel SDMMC driver
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH v2 2/2] mmc: sdhci-cadence: add Cadence SD4HC support
From: Masahiro Yamada @ 2016-12-01  3:36 UTC (permalink / raw)
  To: linux-mmc-u79uwXL29TY76Z2rM5mHXA
  Cc: Adrian Hunter, Ulf Hansson, Masahiro Yamada,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Stefan Wahren, Rob Herring,
	Al Cooper, Wolfram Sang, Andrei Pistirica, Mark Rutland,
	Simon Horman, Eric Anholt
In-Reply-To: <1480563366-7259-1-git-send-email-yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>

Add a driver for the Cadence SD4HC SD/SDIO/eMMC Controller.

For SD, it basically relies on the SDHCI standard code.
For eMMC, this driver provides some callbacks to support the
hardware part that is specific to this IP design.

Signed-off-by: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
---

Changes in v2:
  - Remove unnecessary "select MMC_SDHCI_IO_ACCESSORS"

 .../devicetree/bindings/mmc/sdhci-cadence.txt      |  31 +++
 drivers/mmc/host/Kconfig                           |  11 +
 drivers/mmc/host/Makefile                          |   1 +
 drivers/mmc/host/sdhci-cadence.c                   | 279 +++++++++++++++++++++
 4 files changed, 322 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
 create mode 100644 drivers/mmc/host/sdhci-cadence.c

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
new file mode 100644
index 0000000..c18aaee
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
@@ -0,0 +1,31 @@
+* Cadence SD/SDIO/eMMC Host Controller
+
+Required properties:
+- compatible: should be "cdns,sd4hc".
+- reg: offset and length of the register set for the device.  The region should
+  contain both Host Register Set (HRS) and Slot Register Set (SRS).
+- interrupts: a single interrupt specifier.
+- clocks: phandle to the input clock.
+
+Optional properties:
+For eMMC configuration, supported speed modes are not provided by the SDHCI
+Capabilities Register.  Instead, the following properties should be specified
+if supported.  See mmc.txt for details.
+- mmc-ddr-1_8v
+- mmc-ddr-1_2v
+- mmc-hs200-1_8v
+- mmc-hs200-1_2v
+- mmc-hs400-1_8v
+- mmc-hs400-1_2v
+
+Example:
+	emmc: sdhci@5a000000 {
+		compatible = "cdns,sd4hc";
+		reg = <0x5a000000 0x400>;
+		interrupts = <0 78 4>;
+		clocks = <&clk 4>;
+		bus-width = <8>;
+		mmc-ddr-1_8v;
+		mmc-hs200-1_8v;
+		mmc-hs400-1_8v;
+	};
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 580b5a1..39f6f96 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -165,6 +165,17 @@ config MMC_SDHCI_OF_HLWD
 
 	  If unsure, say N.
 
+config MMC_SDHCI_CADENCE
+	tristate "SDHCI support for the Cadence SD/SDIO/eMMC controller"
+	depends on MMC_SDHCI_PLTFM
+	depends on OF
+	help
+	  This selects the Cadence SD/SDIO/eMMC driver.
+
+	  If you have a controller with this interface, say Y or M here.
+
+	  If unsure, say N.
+
 config MMC_SDHCI_CNS3XXX
 	tristate "SDHCI support on the Cavium Networks CNS3xxx SoC"
 	depends on ARCH_CNS3XXX
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index e49a82a..55f7193 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_MMC_REALTEK_PCI)	+= rtsx_pci_sdmmc.o
 obj-$(CONFIG_MMC_REALTEK_USB)	+= rtsx_usb_sdmmc.o
 
 obj-$(CONFIG_MMC_SDHCI_PLTFM)		+= sdhci-pltfm.o
+obj-$(CONFIG_MMC_SDHCI_CADENCE)		+= sdhci-cadence.o
 obj-$(CONFIG_MMC_SDHCI_CNS3XXX)		+= sdhci-cns3xxx.o
 obj-$(CONFIG_MMC_SDHCI_ESDHC_IMX)	+= sdhci-esdhc-imx.o
 obj-$(CONFIG_MMC_SDHCI_DOVE)		+= sdhci-dove.o
diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
new file mode 100644
index 0000000..a7daf00
--- /dev/null
+++ b/drivers/mmc/host/sdhci-cadence.c
@@ -0,0 +1,279 @@
+/*
+ * Copyright (C) 2016 Socionext Inc.
+ *   Author: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/bitops.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/mmc/host.h>
+
+#include "sdhci-pltfm.h"
+
+/* HRS - Host Register Set (specific to Cadence) */
+#define SDHCI_CDNS_HRS04		0x10		/* PHY access port */
+#define   SDHCI_CDNS_HRS04_ACK			BIT(26)
+#define   SDHCI_CDNS_HRS04_RD			BIT(25)
+#define   SDHCI_CDNS_HRS04_WR			BIT(24)
+#define   SDHCI_CDNS_HRS04_RDATA_SHIFT		12
+#define   SDHCI_CDNS_HRS04_WDATA_SHIFT		8
+#define   SDHCI_CDNS_HRS04_ADDR_SHIFT		0
+
+#define SDHCI_CDNS_HRS06		0x18		/* eMMC control */
+#define   SDHCI_CDNS_HRS06_TUNE_UP		BIT(15)
+#define   SDHCI_CDNS_HRS06_TUNE_SHIFT		8
+#define   SDHCI_CDNS_HRS06_TUNE_MASK		0x3f
+#define   SDHCI_CDNS_HRS06_MODE_MASK		0x7
+#define   SDHCI_CDNS_HRS06_MODE_SD		0x0
+#define   SDHCI_CDNS_HRS06_MODE_MMC_SDR		0x2
+#define   SDHCI_CDNS_HRS06_MODE_MMC_DDR		0x3
+#define   SDHCI_CDNS_HRS06_MODE_MMC_HS200	0x4
+#define   SDHCI_CDNS_HRS06_MODE_MMC_HS400	0x5
+
+/* SRS - Slot Register Set (SDHCI-compatible) */
+#define SDHCI_CDNS_SRS_BASE		0x200
+
+/* PHY */
+#define SDHCI_CDNS_PHY_DLY_SD_HS	0x00
+#define SDHCI_CDNS_PHY_DLY_SD_DEFAULT	0x01
+#define SDHCI_CDNS_PHY_DLY_UHS_SDR12	0x02
+#define SDHCI_CDNS_PHY_DLY_UHS_SDR25	0x03
+#define SDHCI_CDNS_PHY_DLY_UHS_SDR50	0x04
+#define SDHCI_CDNS_PHY_DLY_UHS_DDR50	0x05
+#define SDHCI_CDNS_PHY_DLY_EMMC_LEGACY	0x06
+#define SDHCI_CDNS_PHY_DLY_EMMC_SDR	0x07
+#define SDHCI_CDNS_PHY_DLY_EMMC_DDR	0x08
+
+/*
+ * The tuned val register is 6 bit-wide, but not the whole of the range is
+ * available.  The range 0-42 seems to be available (then 43 wraps around to 0)
+ * but I am not quite sure if it is official.  Use only 0 to 39 for safety.
+ */
+#define SDHCI_CDNS_MAX_TUNING_LOOP	40
+
+struct sdhci_cdns_priv {
+	void __iomem *hrs_addr;
+};
+
+static void sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv,
+				     u8 addr, u8 data)
+{
+	void __iomem *reg = priv->hrs_addr + SDHCI_CDNS_HRS04;
+	u32 tmp;
+
+	tmp = (data << SDHCI_CDNS_HRS04_WDATA_SHIFT) |
+	      (addr << SDHCI_CDNS_HRS04_ADDR_SHIFT);
+	writel(tmp, reg);
+
+	tmp |= SDHCI_CDNS_HRS04_WR;
+	writel(tmp, reg);
+
+	tmp &= ~SDHCI_CDNS_HRS04_WR;
+	writel(tmp, reg);
+}
+
+static void sdhci_cdns_phy_init(struct sdhci_cdns_priv *priv)
+{
+	sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_HS, 4);
+	sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_DEFAULT, 4);
+	sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_LEGACY, 9);
+	sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_SDR, 2);
+	sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_DDR, 3);
+}
+
+static inline void *sdhci_cdns_priv(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+
+	return sdhci_pltfm_priv(pltfm_host);
+}
+
+static unsigned int sdhci_cdns_get_timeout_clock(struct sdhci_host *host)
+{
+	/*
+	 * Cadence's spec says the Timeout Clock Frequency is the same as the
+	 * Base Clock Frequency.  Divide it by 1000 to return a value in kHz.
+	 */
+	return host->max_clk / 1000;
+}
+
+static int sdhci_cdns_set_tune_val(struct sdhci_host *host, unsigned int val)
+{
+	struct sdhci_cdns_priv *priv = sdhci_cdns_priv(host);
+	void __iomem *reg = priv->hrs_addr + SDHCI_CDNS_HRS06;
+	u32 tmp;
+
+	if (WARN_ON(val > SDHCI_CDNS_HRS06_TUNE_MASK))
+		return -EINVAL;
+
+	tmp = readl(reg);
+	tmp &= ~(SDHCI_CDNS_HRS06_TUNE_MASK << SDHCI_CDNS_HRS06_TUNE_SHIFT);
+	tmp |= val << SDHCI_CDNS_HRS06_TUNE_SHIFT;
+	tmp |= SDHCI_CDNS_HRS06_TUNE_UP;
+	writel(tmp, reg);
+
+	return readl_poll_timeout(reg, tmp, !(tmp & SDHCI_CDNS_HRS06_TUNE_UP),
+				  0, 1);
+}
+
+static int sdhci_cdns_execute_tuning(struct sdhci_host *host, u32 opcode)
+{
+	int max_streak = 0;
+	int cur_streak = 0;
+	int end_of_streak, i;
+
+	/*
+	 * This handler only implements the eMMC tuning that is specific to
+	 * this controller.  Fall back to the standard method for SD timing.
+	 */
+	if (host->timing != MMC_TIMING_MMC_HS200)
+		return -ENOTSUPP;
+
+	if (WARN_ON(opcode != MMC_SEND_TUNING_BLOCK_HS200))
+		return -EINVAL;
+
+	for (i = 0; i < SDHCI_CDNS_MAX_TUNING_LOOP; i++) {
+		if (sdhci_cdns_set_tune_val(host, i) ||
+		    mmc_send_tuning(host->mmc, opcode, NULL)) { /* bad */
+			cur_streak = 0;
+		} else { /* good */
+			cur_streak++;
+			max_streak = max(max_streak, cur_streak);
+			end_of_streak = i;
+		}
+	}
+
+	if (!max_streak) {
+		dev_err(mmc_dev(host->mmc), "no tuning point found\n");
+		return -EIO;
+	}
+
+	return sdhci_cdns_set_tune_val(host, end_of_streak - max_streak / 2);
+}
+
+static void sdhci_cdns_set_uhs_signaling(struct sdhci_host *host,
+					 unsigned int timing)
+{
+	struct sdhci_cdns_priv *priv = sdhci_cdns_priv(host);
+	u32 mode, tmp;
+
+	switch (timing) {
+	case MMC_TIMING_MMC_HS:
+		mode = SDHCI_CDNS_HRS06_MODE_MMC_SDR;
+		break;
+	case MMC_TIMING_MMC_DDR52:
+		mode = SDHCI_CDNS_HRS06_MODE_MMC_DDR;
+		break;
+	case MMC_TIMING_MMC_HS200:
+		mode = SDHCI_CDNS_HRS06_MODE_MMC_HS200;
+		break;
+	case MMC_TIMING_MMC_HS400:
+		mode = SDHCI_CDNS_HRS06_MODE_MMC_HS400;
+		break;
+	default:
+		mode = SDHCI_CDNS_HRS06_MODE_SD;
+		break;
+	}
+
+	/* The speed mode for eMMC is selected by HRS06 register */
+	tmp = readl(priv->hrs_addr + SDHCI_CDNS_HRS06);
+	tmp &= ~SDHCI_CDNS_HRS06_MODE_MASK;
+	tmp |= mode;
+	writel(tmp, priv->hrs_addr + SDHCI_CDNS_HRS06);
+
+	/* For SD, fall back to the default handler */
+	if (mode == SDHCI_CDNS_HRS06_MODE_SD)
+		sdhci_set_uhs_signaling(host, timing);
+}
+
+static const struct sdhci_ops sdhci_cdns_ops = {
+	.set_clock = sdhci_set_clock,
+	.get_timeout_clock = sdhci_cdns_get_timeout_clock,
+	.set_bus_width = sdhci_set_bus_width,
+	.reset = sdhci_reset,
+	.platform_execute_tuning = sdhci_cdns_execute_tuning,
+	.set_uhs_signaling = sdhci_cdns_set_uhs_signaling,
+};
+
+static const struct sdhci_pltfm_data sdhci_cdns_pltfm_data = {
+	.ops = &sdhci_cdns_ops,
+};
+
+static int sdhci_cdns_probe(struct platform_device *pdev)
+{
+	struct sdhci_host *host;
+	struct sdhci_pltfm_host *pltfm_host;
+	struct sdhci_cdns_priv *priv;
+	struct clk *clk;
+	int ret;
+
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	ret = clk_prepare_enable(clk);
+	if (ret)
+		return ret;
+
+	host = sdhci_pltfm_init(pdev, &sdhci_cdns_pltfm_data, sizeof(*priv));
+	if (IS_ERR(host)) {
+		ret = PTR_ERR(host);
+		goto disable_clk;
+	}
+
+	pltfm_host = sdhci_priv(host);
+	pltfm_host->clk = clk;
+
+	priv = sdhci_cdns_priv(host);
+	priv->hrs_addr = host->ioaddr;
+	host->ioaddr += SDHCI_CDNS_SRS_BASE;
+
+	ret = mmc_of_parse(host->mmc);
+	if (ret)
+		goto free;
+
+	sdhci_cdns_phy_init(priv);
+
+	ret = sdhci_add_host(host);
+	if (ret)
+		goto free;
+
+	return 0;
+free:
+	sdhci_pltfm_free(pdev);
+disable_clk:
+	clk_disable_unprepare(clk);
+
+	return ret;
+}
+
+static const struct of_device_id sdhci_cdns_match[] = {
+	{ .compatible = "cdns,sd4hc" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, sdhci_cdns_match);
+
+static struct platform_driver sdhci_cdns_driver = {
+	.driver = {
+		.name = "sdhci-cdns",
+		.pm = &sdhci_pltfm_pmops,
+		.of_match_table = sdhci_cdns_match,
+	},
+	.probe = sdhci_cdns_probe,
+	.remove = sdhci_pltfm_unregister,
+};
+module_platform_driver(sdhci_cdns_driver);
+
+MODULE_AUTHOR("Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>");
+MODULE_DESCRIPTION("Cadence SD/SDIO/eMMC Host Controller Driver");
+MODULE_LICENSE("GPL");
-- 
2.7.4

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

^ permalink raw reply related

* [PATCH v2 1/2] mmc: sdhci: continue normal tuning if unsupported by platform tuning
From: Masahiro Yamada @ 2016-12-01  3:36 UTC (permalink / raw)
  To: linux-mmc; +Cc: Adrian Hunter, Ulf Hansson, Masahiro Yamada, linux-kernel
In-Reply-To: <1480563366-7259-1-git-send-email-yamada.masahiro@socionext.com>

Some SDHCI-compat controllers support not only SD, but also eMMC,
but they use different commands for tuning: CMD19 for SD, CMD21 for
eMMC.

Due to the difference of the underlying mechanism, some controllers
(at least, the Cadence IP is the case) provide their own registers
for the eMMC tuning.

This commit will be useful when we want to use platform-specific
tuning (to support eMMC HS200), but still let it reuse the code
provided by sdhci_execute_tuning() for SD timing.

If sdhci_ops::platform_execute_tuning receives a timing it does not
take care of, it can return -ENOTSUPP.  Then, it will fall back to
the SDHCI standard tuning.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

I want to use this in the next commit.

The Cadence IP supports eMMC as well as SD.

The tuning for SD is pretty simple; just set the "Execute Tuning" bit
of the HOST_CONTROL2 register.  So, I can re-use the
sdhci_execute_tuning().

On the other hand, Cadence provides its own way for eMMC HS200 tuning;
I need to touch some registers that are specific to Cadence's design.


Changes in v2: None

 drivers/mmc/host/sdhci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 42ef3eb..cdce489 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2004,7 +2004,9 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	if (host->ops->platform_execute_tuning) {
 		spin_unlock_irqrestore(&host->lock, flags);
 		err = host->ops->platform_execute_tuning(host, opcode);
-		return err;
+		if (err != -ENOTSUPP)
+			return err;
+		spin_lock_irqsave(&host->lock, flags);
 	}
 
 	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 0/2] mmc: sdhci: one expansion for SDHCI base code and add Cadence SDHCI driver
From: Masahiro Yamada @ 2016-12-01  3:36 UTC (permalink / raw)
  To: linux-mmc-u79uwXL29TY76Z2rM5mHXA
  Cc: Adrian Hunter, Ulf Hansson, Masahiro Yamada,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Stefan Wahren, Rob Herring,
	Al Cooper, Wolfram Sang, Andrei Pistirica, Mark Rutland,
	Simon Horman, Eric Anholt

1/2 tweaks sdhci_execute_tuning(), which I want to use for 2/2.

2/2 adds a new driver for Cadence's controller IP.


Changes in v2:
  - Remove unnecessary "select MMC_SDHCI_IO_ACCESSORS"

Masahiro Yamada (2):
  mmc: sdhci: continue normal tuning if unsupported by platform tuning
  mmc: sdhci-cadence: add Cadence SD4HC support

 .../devicetree/bindings/mmc/sdhci-cadence.txt      |  31 +++
 drivers/mmc/host/Kconfig                           |  11 +
 drivers/mmc/host/Makefile                          |   1 +
 drivers/mmc/host/sdhci-cadence.c                   | 279 +++++++++++++++++++++
 drivers/mmc/host/sdhci.c                           |   4 +-
 5 files changed, 325 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
 create mode 100644 drivers/mmc/host/sdhci-cadence.c

-- 
2.7.4

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

^ permalink raw reply

* Re: [PATCH v6 3/5] Documentation: synopsys-dw-mshc: add binding for fifo quirks
From: Jun Nie @ 2016-12-01  1:35 UTC (permalink / raw)
  To: Shawn Guo, xie.baoyou, Rob Herring, mark.rutland-5wv7dgnIgG8
  Cc: Ulf Hansson, Jaehoon Chung, Jason Liu,
	chen.chaokai-Th6q7B73Y6EnDS1+zs4M5A,
	lai.binz-Th6q7B73Y6EnDS1+zs4M5A, linux-mmc, Jun Nie,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CABymUCN_+kJC2NZaPuqOg1C_Q2ASmiGwbwWCtfiLjGwSqPjgkQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

2016-11-24 10:19 GMT+08:00 Jun Nie <jun.nie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:
> 2016-11-18 14:29 GMT+08:00 Jun Nie <jun.nie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:
>> Add fifo-addr property and fifo-watermark-quirk property to
>> synopsys-dw-mshc bindings. It is intended to provide more
>> dt interface to support SoCs specific configuration.
>>
>> Signed-off-by: Jun Nie <jun.nie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>> index 4e00e85..8bf2e41 100644
>> --- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>> +++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>> @@ -76,6 +76,17 @@ Optional properties:
>>
>>  * broken-cd: as documented in mmc core bindings.
>>
>> +* data-addr: Override fifo address with value provided by DT. The default FIFO reg
>> +  offset is assumed as 0x100 (version < 0x240A) and 0x200(version >= 0x240A) by
>> +  driver. If the controller does not follow this rule, please use this property
>> +  to set fifo address in device tree.
>> +
>> +* fifo-watermark-aligned: Data done irq is expected if data length is less than
>> +  watermark in PIO mode. But fifo watermark is requested to be aligned with data
>> +  length in some SoC so that TX/RX irq can be generated with data done irq. Add this
>> +  watermark quirk to mark this requirement and force fifo watermark setting
>> +  accordingly.
>> +
>>  * vmmc-supply: The phandle to the regulator to use for vmmc.  If this is
>>    specified we'll defer probe until we can find this regulator.
>>
>> @@ -103,6 +114,8 @@ board specific portions as listed below.
>>                 interrupts = <0 75 0>;
>>                 #address-cells = <1>;
>>                 #size-cells = <0>;
>> +               data-addr = <0x200>;
>> +               fifo-watermark-aligned;
>>         };
>>
>>  [board specific internal DMA resources]
>> --
>> 1.9.1
>>
> Hi Rob & Mark,
>
> Could you help review and act this patch if you think it is OK? Thank you!
>
> Jun


Hi Rob & Mark,

Could you help comment or act on this patch in revsion 6? Thank you!

Jun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v6 1/5] mmc: dt-bindings: add ZTE ZX296718 MMC bindings
From: Jun Nie @ 2016-12-01  1:34 UTC (permalink / raw)
  To: Shawn Guo, xie.baoyou, Rob Herring, mark.rutland
  Cc: Ulf Hansson, Jaehoon Chung, Jason Liu, chen.chaokai, lai.binz,
	linux-mmc, Jun Nie, devicetree
In-Reply-To: <CABymUCOvpQv7D2fi7EhWUZb_B8iwvQQ7-uW0gPrSdsFabMcPbA@mail.gmail.com>

2016-11-24 10:17 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
> 2016-11-18 14:29 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
>> Document the device-tree binding of ZTE MMC host on
>> ZX296718 SoC.
>>
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> ---
>>  .../devicetree/bindings/mmc/zx-dw-mshc.txt         | 35 ++++++++++++++++++++++
>>  1 file changed, 35 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>> new file mode 100644
>> index 0000000..c175c4b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>> @@ -0,0 +1,35 @@
>> +* ZTE specific extensions to the Synopsys Designware Mobile Storage
>> +  Host Controller
>> +
>> +The Synopsys designware mobile storage host controller is used to interface
>> +a SoC with storage medium such as eMMC or SD/MMC cards. This file documents
>> +differences between the core Synopsys dw mshc controller properties described
>> +by synopsys-dw-mshc.txt and the properties used by the ZTE specific
>> +extensions to the Synopsys Designware Mobile Storage Host Controller.
>> +
>> +Required Properties:
>> +
>> +* compatible: should be
>> +       - "zte,zx296718-dw-mshc": for ZX SoCs
>> +
>> +Example:
>> +
>> +       mmc1: mmc@1110000 {
>> +               compatible = "zte,zx296718-dw-mshc";
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +               reg = <0x01110000 0x1000>;
>> +               interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
>> +               fifo-depth = <32>;
>> +               data-addr = <0x200>;
>> +               fifo-watermark-aligned;
>> +               bus-width = <4>;
>> +               clock-frequency = <50000000>;
>> +               clocks = <&topcrm SD0_AHB>, <&topcrm SD0_WCLK>;
>> +               clock-names = "biu", "ciu";
>> +               num-slots = <1>;
>> +               max-frequency = <50000000>;
>> +               cap-sdio-irq;
>> +               cap-sd-highspeed;
>> +               status = "disabled";
>> +       };
>> --
>> 1.9.1
>>
>
> Hi Rob & Mark,
>
> Could you help review and act this patch if you think it is OK? Thank you!
>
> Jun

Hi Rob & Mark,

Could you help comment or act on this patch? Thank you!

Jun

^ permalink raw reply

* Re: [PATCH V8 01/20] mmc: block: Restore line inadvertently removed with packed commands
From: Linus Walleij @ 2016-11-30 21:25 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
	Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
	Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
	Harjani Ritesh, Venu Byravarasu
In-Reply-To: <1480414167-15577-2-git-send-email-adrian.hunter@intel.com>

On Tue, Nov 29, 2016 at 11:09 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:

> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Ooops my mistake, thanks.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH] mmc: queue: add #ifdef around bounce buffer code
From: Arnd Bergmann @ 2016-11-30 21:03 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Arnd Bergmann, Linus Walleij, Adrian Hunter, Jens Axboe,
	Harjani Ritesh, linux-mmc, linux-kernel

The two functions that got split out from the bounce
buffer handling are now unused when CONFIG_MMC_BLOCK_BOUNCE
is disabled:

mmc/card/queue.c:212:12: error: 'mmc_queue_alloc_bounce_sgs' defined but not used [-Werror=unused-function]
mmc/card/queue.c:189:13: error: 'mmc_queue_alloc_bounce_bufs' defined but not used [-Werror=unused-function]

This adds a new #ifdef around them.

Fixes: 0a6321310492 ("mmc: queue: Factor out mmc_queue_alloc_bounce_bufs()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/mmc/card/queue.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index 8d10fab701b6..6ae6bfb8b221 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -186,6 +186,7 @@ static void mmc_queue_setup_discard(struct request_queue *q,
 		queue_flag_set_unlocked(QUEUE_FLAG_SECERASE, q);
 }
 
+#ifdef CONFIG_MMC_BLOCK_BOUNCE
 static bool mmc_queue_alloc_bounce_bufs(struct mmc_queue *mq,
 					unsigned int bouncesz)
 {
@@ -226,6 +227,7 @@ static int mmc_queue_alloc_bounce_sgs(struct mmc_queue *mq,
 
 	return 0;
 }
+#endif
 
 static int mmc_queue_alloc_sgs(struct mmc_queue *mq, int max_segs)
 {
-- 
2.9.0

^ permalink raw reply related

* Re: [PATCH] mmc: Hynix: add QUIRK_NOTIFY_POWEROFF_ON_SLEEP
From: Thierry Escande @ 2016-11-30 17:05 UTC (permalink / raw)
  To: Alex Lemberg, Ulf Hansson; +Cc: linux-mmc
In-Reply-To: <A6F6C92B-1E3A-4326-88B7-E3E9100E2DD8@sandisk.com>

Hi,

On 31/10/2016 11:14, Alex Lemberg wrote:
> Hi,
>
> By the eMMC5.0 spec, before sending Sleep command (CMD5), hosts may set the
> POWER_OFF_NOTIFICATION byte to SLEEP_NOTIFICATION (0x04).
> Isn’t this a case in this patch?
> If yes, why not sending SLEEP_NOTIFICATION instead of POWER_OFF_SHORT?
I cannot tell if this was really what the Hynix engineers meant here.

> In the past we had a discussion on this topic, but the solution became more complicated because
> of possibly long timeout (max 83 seconds) on SLEEP_NOTIFICATION.
> https://marc.info/?t=143374696600002&r=1&w=1
>
> But still, in case we want to support POWER_OFF_NOTIFICATION before Sleep command,
> and in case we want to be eMMC spec aligned, I believe the right thing to do
> is to support SLEEP_NOTIFICATION…
I can give a try to this patchset to see if it solves the issue and get 
back to you afterward.

Regards,
  Thierry

>
> Thanks,
> Alex
>
> On 10/27/16, 10:49 PM, "linux-mmc-owner@vger.kernel.org on behalf of Ulf Hansson" <linux-mmc-owner@vger.kernel.org on behalf of ulf.hansson@linaro.org> wrote:
>
>> On 27 October 2016 at 17:06, Thierry Escande
>> <thierry.escande@collabora.com> wrote:
>>> Hi Ulf,
>>>
>>> On 25/10/2016 12:03, Ulf Hansson wrote:
>>>>
>>>> On 3 October 2016 at 16:19, Thierry Escande
>>>> <thierry.escande@collabora.com> wrote:
>>>>>
>>>>> From: zhaojohn <john.zhao@intel.com>
>>>>>
>>>>> Hynix eMMC devices sometimes take 50% longer to resume from sleep.
>>>>> Based on a recommendation from Hynix, send a Power-Off Notification
>>>>> before going to S3 to restore a resume time consistently within spec.
>>>>
>>>>
>>>> Could you also share what mmc controller and SoC you get this results
>>>> from?
>>>>
>>>> More precisely, are you using MMC_CAP_WAIT_WHILE_BUSY?
>>>
>>> This occurs on a braswell based chromebook, using the acpi sdhci controller.
>>> So yes, using MMC_CAP_WAIT_WHILE_BUSY.
>>>
>>> [...]
>>>>>
>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>> index f2d185c..46a4562 100644
>>>>> --- a/drivers/mmc/core/mmc.c
>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>> @@ -1925,8 +1925,14 @@ static int _mmc_suspend(struct mmc_host *host,
>>>>> bool is_suspend)
>>>>>         if (mmc_can_poweroff_notify(host->card) &&
>>>>>                 ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
>>>>>                 err = mmc_poweroff_notify(host->card, notify_type);
>>>>> -       else if (mmc_can_sleep(host->card))
>>>>> +       else if (mmc_can_sleep(host->card)) {
>>>>> +               if (host->card->quirks &
>>>>> MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP) {
>>>>> +                       err = mmc_poweroff_notify(host->card,
>>>>> notify_type);
>>>>> +                       if (err)
>>>>> +                               goto out;
>>>>> +               }
>>>>
>>>>
>>>> So, I am curious to know from a power management point of view; how
>>>> does the card behave comparing the sleep and power off notification
>>>> command?
>>>>
>>>> Is the card in a low power state after the power off notification has
>>>> been received? If so, did you manage to do some measurement for that
>>>> or perhaps the data-sheet tells about this? It would be interesting to
>>>> know if there were any differences between sleep and power off
>>>> notification in this regards.
>>>
>>> I do not have any clue about that. It appears only with Hynix emmc and the
>>> fix has been approved by Hynix engineers... It seems that if not powered
>>> off, the firmware does some garbage collection when resuming and it takes
>>> more time...
>>
>> Okay, I see. So before I continue reviewing, can you please also tell
>> what regulators to the card that is being cut while powering off in
>> this path.
>>
>> VMMC, VQMMC?
>>
>> Kind regards
>> Uffe
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: [PATCH] mmc: pwrseq: add support for Marvell SD8787 chip
From: Ulf Hansson @ 2016-11-30 14:39 UTC (permalink / raw)
  To: Javier Martinez Canillas, Matt Ranostay
  Cc: linux-wireless@vger.kernel.org, Linux Kernel,
	devicetree@vger.kernel.org, linux-mmc@vger.kernel.org,
	Tony Lindgren, Mark Rutland, Srinivas Kandagatla
In-Reply-To: <CABxcv=nc1qjtSjzfcuGTo-zUpuWdRT6OR7MZCCWaoeq4Co3Uew@mail.gmail.com>

On 30 November 2016 at 14:11, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> Hello Matt,
>
> On Tue, Nov 29, 2016 at 10:20 PM, Matt Ranostay
> <matt@ranostay.consulting> wrote:
>> On Tue, Nov 29, 2016 at 9:13 AM, Javier Martinez Canillas
>
> [snip]
>
>>
>>
>>>> +- pwndn-gpio: contains a power down GPIO specifier.
>>>> +- reset-gpio: contains a reset GPIO specifier.
>>>> +
>>>
>>> I wonder if we really need a custom power sequence provider for just
>>> this SDIO WiFI chip though. AFAICT the only missing piece in
>>> mmc-pwrseq-simple is the power down GPIO property, so maybe
>>> mmc-pwrseq-simple could be extended instead to have an optional
>>> powerdown-gpios property and instead in the Marvell SD8787 DT binding
>>> can be mentioned which mmc-pwrseq-simple properties are required for
>>> the device.
>>>
>>
>> The reason we didn't do that is we need delay between the two
>> assertions/desertions of GPIOs. It wouldn't seems good practice to
>> hack the pwrseq-simple for this...
>>
>
> Yes, I noticed that. I wouldn't say that it would be a hack for the
> pwrseq-simple since it already has a "post-power-on-delay-ms" DT
> property, so AFAICT it would just be adding a "pre-power-on-delay-ms"
> property for your use case.
>
> It would also be more consistent since it would support a delay for
> pre and post power callbacks. It would also make you avoid hardcoding
> the 300 msec wait, in case other device has a similar need but with a
> different wait time.
>
> In summary, I think that devices having a power (or power down) and
> enable GPIO, and needing to wait between the GPIO toggling are common.
> So I would prefer to make pwrseq-simple usable for these instead of
> adding device specific power sequence providers. But it's just my
> opinion and not my call :-)

This is a good idea. Please try out this approach.

[...]

Kind regards
Uffe

^ permalink raw reply

* Re: arasan,sdhci.txt "compatibility" DT binding
From: Michal Simek @ 2016-11-30 13:17 UTC (permalink / raw)
  To: Sebastian Frias, Rameshwar Sahu, Mason
  Cc: Arnd Bergmann, linux-mmc, Shawn Lin, Adrian Hunter, Michal Simek,
	Linux ARM, Soren Brinkmann, Michal Simek, Anton Vorontsov,
	Xiaobo Xie, Suman Tripathi, Linus Walleij, Maxime Ripard,
	Rob Herring, Zach Brown, Ulf Hansson, Douglas Anderson,
	Heiko Stuebner, Jisheng Zhang, Suneel 
In-Reply-To: <648bd95b-5be0-b39f-a956-af741c6407e4@laposte.net>

On 30.11.2016 11:51, Sebastian Frias wrote:
> On 29/11/16 08:29, Rameshwar Sahu wrote:
>> Hi Mason,
>>
>> Nowhere in the documentation do they specify an "IP version".
>> Some documents do provide a revision number, but that's just
>> a *documentation* revision number, e.g.
>>
>> changes in version 3.6 : fix typos
>> changes in version 9.1a : update company logo
>>
>> That's why Xilinx used "arasan,sdhci-8.9a" and APM used
>> "arasan,sdhci-4.9a". These are documentation revisions.
>> In my opinion, that information is mostly worthless.
>>
> 
> For the record, the important information conveyed by Rameshwar's
> email is the following:
> 
> Arasan SD/SDIO/eMMC IP has a register which tells about the SD
> specification version and  Vendor version number
> Reg Name: Host controller version register (offset 0FEh)
> bit [15:8] is for vendor version number,
> But, I have seen that Arasaan vendor version number is same as
> document revision number.
> 
> (At first I had ignored the email because it repeated Mason's email
> without quoting, but then I realised it contained some information)
> 
> 

Values on real HW.

ZynqMP device
xsdb% mrd 0xFF1600FC
FF1600FC:   10020000

Zynq device:
xsdb% mrd 0xE01000FC
E01000FC:   89010000

Based on docs I have access to.

Specification_Version_Number (bits:23:16)
00 - SD Host Specification version 1.0
01 - SD Host Specification version 2.00 including only the feature of
the Test Register
02 - SD Host Specification version 3.00


Vendor_Version_Number 	31:24

with 32bit access from FC

Thanks,
Michal


^ permalink raw reply

* Re: [PATCH] mmc: pwrseq: add support for Marvell SD8787 chip
From: Javier Martinez Canillas @ 2016-11-30 13:11 UTC (permalink / raw)
  To: Matt Ranostay
  Cc: linux-wireless@vger.kernel.org, Linux Kernel,
	devicetree@vger.kernel.org, linux-mmc@vger.kernel.org,
	Tony Lindgren, Ulf Hansson, Mark Rutland, Srinivas Kandagatla
In-Reply-To: <CAJ_EiST_bpBm+spPuWH-a+47t4qVsDVFjUm=a+TtL-BWN3DHdw@mail.gmail.com>

Hello Matt,

On Tue, Nov 29, 2016 at 10:20 PM, Matt Ranostay
<matt@ranostay.consulting> wrote:
> On Tue, Nov 29, 2016 at 9:13 AM, Javier Martinez Canillas

[snip]

>
>
>>> +- pwndn-gpio: contains a power down GPIO specifier.
>>> +- reset-gpio: contains a reset GPIO specifier.
>>> +
>>
>> I wonder if we really need a custom power sequence provider for just
>> this SDIO WiFI chip though. AFAICT the only missing piece in
>> mmc-pwrseq-simple is the power down GPIO property, so maybe
>> mmc-pwrseq-simple could be extended instead to have an optional
>> powerdown-gpios property and instead in the Marvell SD8787 DT binding
>> can be mentioned which mmc-pwrseq-simple properties are required for
>> the device.
>>
>
> The reason we didn't do that is we need delay between the two
> assertions/desertions of GPIOs. It wouldn't seems good practice to
> hack the pwrseq-simple for this...
>

Yes, I noticed that. I wouldn't say that it would be a hack for the
pwrseq-simple since it already has a "post-power-on-delay-ms" DT
property, so AFAICT it would just be adding a "pre-power-on-delay-ms"
property for your use case.

It would also be more consistent since it would support a delay for
pre and post power callbacks. It would also make you avoid hardcoding
the 300 msec wait, in case other device has a similar need but with a
different wait time.

In summary, I think that devices having a power (or power down) and
enable GPIO, and needing to wait between the GPIO toggling are common.
So I would prefer to make pwrseq-simple usable for these instead of
adding device specific power sequence providers. But it's just my
opinion and not my call :-)

>>> +Example:
>>> +
>>> +       wifi_pwrseq: wifi_pwrseq {
>>> +               compatible = "mmc-pwrseq-sd8787";
>>> +               pwrdn-gpio = <&twl_gpio 0 GPIO_ACTIVE_LOW>;
>>> +               reset-gpio = <&twl_gpio 1 GPIO_ACTIVE_LOW>;
>>> +       }
>>> diff --git a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt b/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
>>
>> Does this patch depend on a previous posted series? I don't see this
>> file in today's linux-next...
>
> Got renamed to ->
> Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt it
> seems very recently.
>

I see, that's what I thought but I wasn't able to find the commit /
patch that did it.

>>
>>> index c421aba0a5bc..08fd65d35725 100644
>>> --- a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
>>> +++ b/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
>>> @@ -32,6 +32,9 @@ Optional properties:
>>>                  so that the wifi chip can wakeup host platform under certain condition.
>>>                  during system resume, the irq will be disabled to make sure
>>>                  unnecessary interrupt is not received.
>>> +  - vmmc-supply: a phandle of a regulator, supplying VCC to the card
>>> +  - mmc-pwrseq:  phandle to the MMC power sequence node. See "mmc-pwrseq-*"
>>> +                for documentation of MMC power sequence bindings.
>>>
>>>  Example:
>>>
>>> @@ -44,6 +47,7 @@ so that firmware can wakeup host using this device side pin.
>>>  &mmc3 {
>>>         status = "okay";
>>>         vmmc-supply = <&wlan_en_reg>;
>>> +       mmc-pwrseq = <&wifi_pwrseq>;
>>>         bus-width = <4>;
>>>         cap-power-off-card;
>>>         keep-power-in-suspend;
>>
>> I think this change should be split in a separate patch as well.
>>

You didn't answer about this, but I guess you agree it should be in a
separate patch.

Best regards,
Javier

^ permalink raw reply

* Re: arasan,sdhci.txt "compatibility" DT binding
From: Sebastian Frias @ 2016-11-30 10:51 UTC (permalink / raw)
  To: Rameshwar Sahu, Mason
  Cc: Mark Rutland, Ulf Hansson, Suman Tripathi, Heiko Stuebner,
	Shawn Lin, Adrian Hunter, Jisheng Zhang, Russell King,
	Anton Vorontsov, Michal Simek, Linux ARM, Linus Walleij,
	P L Sai Krishna, Zach Brown, Arnd Bergmann, Suneel Garapati,
	Soren Brinkmann, Michal Simek, linux-mmc, Douglas Anderson,
	Maxime Ripard, Xiaobo Xie
In-Reply-To: <CAFd313zQHmFQ61Y4UEvQAvhhVeNFe=nNyjXUKfYXiTKnvm9VWQ@mail.gmail.com>

On 29/11/16 08:29, Rameshwar Sahu wrote:
> Hi Mason,
> 
> Nowhere in the documentation do they specify an "IP version".
> Some documents do provide a revision number, but that's just
> a *documentation* revision number, e.g.
> 
> changes in version 3.6 : fix typos
> changes in version 9.1a : update company logo
> 
> That's why Xilinx used "arasan,sdhci-8.9a" and APM used
> "arasan,sdhci-4.9a". These are documentation revisions.
> In my opinion, that information is mostly worthless.
> 

For the record, the important information conveyed by Rameshwar's
email is the following:

Arasan SD/SDIO/eMMC IP has a register which tells about the SD
specification version and  Vendor version number
Reg Name: Host controller version register (offset 0FEh)
bit [15:8] is for vendor version number,
But, I have seen that Arasaan vendor version number is same as
document revision number.

(At first I had ignored the email because it repeated Mason's email
without quoting, but then I realised it contained some information)



> 
> On Mon, Nov 28, 2016 at 10:22 PM, Mason <slash.tmp@free.fr> wrote:
>> On 28/11/2016 17:15, Arnd Bergmann wrote:
>>
>>> On Monday, November 28, 2016 4:44:39 PM CET Mason wrote:
>>>
>>>> Hello,
>>>>
>>>> @Shawn Lin, could you take a look below and tell me exactly
>>>> which IP core(s) Rockchip is using in its SoCs?
>>>>
>>>> Based on the feedback I received, here is an updated list of
>>>> compatible strings and controller versions dealt with by the
>>>> drivers/mmc/host/sdhci-of-arasan.c code.
>>>>
>>>>
>>>> Xilinx Zynq:
>>>> "SD2.0 / SDIO2.0 / MMC3.31 AHB Host Controller"
>>>> "arasan,sdhci-8.9a"
>>>> NB: 8.9a is the documentation revision (dated 2011-10-19)
>>>> subsequent tweaks labeled 9.0a, 9.1a, 9.2a
>>>>
>>>> Xilinx ZynqMP:
>>>> "SD3.0 / SDIO3.0 / eMMC4.51 AHB Host Controller"
>>>> "arasan,sdhci-8.9a"
>>>> NB: using the same compatible string as Zynq
>>>>
>>>> Sigma SMP87xx
>>>> "SD3.0 / SDIO3.0 / eMMC4.4 AHB Host Controller"
>>>> no compatible string yet, platform-specific init required
>>>>
>>>> APM:
>>>> "SD3.0 / SDIO3.0 / eMMC4.41 AHB Host Controller"
>>>> "arasan,sdhci-4.9a"
>>>> NB: 4.9a appears to be the documentation revision
>>>> no functional diff with "arasan,sdhci-8.9a"
>>>>
>>>> Rockchip
>>>> Exact IP unknown, waiting for Shawn's answer
>>>> "arasan,sdhci-5.1"
>>>> NB: 5.1 appears to refer to the eMMC standard supported
>>>>
>>>>
>>>> On a final note, there are many variations of the Arasan IP.
>>>> I've tracked down at least the following:
>>>>
>>>> SD_2.0_SDIO_2.0__MMC_3.31_AHB_Host_Controller.pdf
>>>> SD_3.0_SDIO_3.0_eMMC_4.41_OCP_Host_Controller.pdf
>>>> SD_3.0_SDIO_3.0_eMMC_4.4__AHB_Host_Controller.pdf
>>>> SD_3.0_SDIO_3.0_eMMC_4.51_Host_Controller.pdf
>>>> SD_3.0_SDIO_3.0_eMMC_4.5__Host_Controller.pdf
>>>> SD_4.1_SDIO_4.1_eMMC_4.51_Host_Controller.pdf
>>>> SD_4.1_SDIO_4.1_eMMC_5.1__Host_Controller.pdf
>>>>
>>>> It seems to me the compatible string should specify
>>>> the SD/SDIO version AND the eMMC version, since it
>>>> seems many combinations are allowed, e.g. eMMC 4.51
>>>> has two possible SD versions.
>>>>
>>>> What do you think?
>>>
>>> It seems wrong to have the eMMC or SD version in the compatible
>>> string.  Is that the only difference between the documents you
>>> found? Normally there should be a version of IP block itself,
>>> besides the supported protocol.
>>
>> But that is exactly the problem :-)
>>
>> Nowhere in the documentation do they specify an "IP version".
>> Some documents do provide a revision number, but that's just
>> a *documentation* revision number, e.g.
>>
>> changes in version 3.6 : fix typos
>> changes in version 9.1a : update company logo
>>
>> That's why Xilinx used "arasan,sdhci-8.9a" and APM used
>> "arasan,sdhci-4.9a". These are documentation revisions.
>> In my opinion, that information is mostly worthless.
>>
>>
>> Looking more closely at SD_3.0_SDIO_3.0_eMMC_4.4__AHB_Host_Controller.pdf
>> (User Guide, which has more info than Datasheet) I see this:
>>
>> Changed Host Controller Version Register value from 16'h0002 to 16'h7501
>> Changed Host Controller Version Register value from 16'h8301 to 16'h8401
>> Changed Host Controller Version Register value from 16'h8401 to 16'h8501
>> Changed Host Controller Version Register to 16'h9502
>> Changed Host Controller Version Register to 16'h9602
>> Changed Host Controller Version Register to 16'h9902
>>
>> Host controller version register (offset 0FEh)
>>
>> Vendor Version Number 15:8
>> HwInit=0x99
>> This status is reserved for the vendor version number.
>> The HD should not use this status.
>>
>> Specification Version Number 7:0
>> HwInit=0x02
>> This status indicates the Host Controller Spec. Version.
>> The upper and lower 4-bits indicate the version.
>> Description
>> 00 - SD Host Specification version 1.0
>> 01 - SD Host Specification version 2.00
>> including only the feature of the Test Register
>> 02 - SD Host Specification Version 3.00
>> others - Reserved
>>
>> I'm not sure what this "Vendor Version Number" specifies, nor if is
>> guaranteed to be unique across controllers.
>>
>> In SD_3.0_SDIO_3.0_eMMC_4.5__Host_Controller_UserGuide.pdf,
>> they write "The Vendor Version Number is set to 0x10 (1.0)"
>>
>> I don't have a UserGuide for "arasan,sdhci-5.1".
>>
>> Regards.
>>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox