Re: [PATCH 1/3] dt-bindings: mmc: sh_mmcif: Document r8a7743 DT bindings

[Ulf Hansson]

On 12 July 2017 at 12:03, Chris Paterson  wrote:
> Signed-off-by: Chris Paterson 
>
> diff --git a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt 
> b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> index c32dc5a..703e18c 100644
> --- a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> +++ b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> @@ -11,6 +11,7 @@ Required properties:
> - "renesas,mmcif-r7s72100" for the MMCIF found in r7s72100 SoCs
> - "renesas,mmcif-r8a73a4" for the MMCIF found in r8a73a4 SoCs
> - "renesas,mmcif-r8a7740" for the MMCIF found in r8a7740 SoCs
> +   - "renesas,mmcif-r8a7743" for the MMCIF found in r8a7743 SoCs
> - "renesas,mmcif-r8a7778" for the MMCIF found in r8a7778 SoCs
> - "renesas,mmcif-r8a7790" for the MMCIF found in r8a7790 SoCs
> - "renesas,mmcif-r8a7791" for the MMCIF found in r8a7791 SoCs
> @@ -21,7 +22,7 @@ Required properties:
>  - interrupts: Some SoCs have only 1 shared interrupt, while others have 
> either
>2 or 3 individual interrupts (error, int, card detect). Below is the number
>of interrupts for each SoC:
> -1: r8a73a4, r8a7778, r8a7790, r8a7791, r8a7793, r8a7794
> +1: r8a73a4, r8a7743, r8a7778, r8a7790, r8a7791, r8a7793, r8a7794
>  2: r8a7740, sh73a0
>  3: r7s72100
>
> --
> 1.9.1
>

Thanks, applied for next!

Kind regards
Uffe

Re: [PATCH v3] mmc: tmio-mmc: fix bad pointer math

[Ulf Hansson]

On 12 July 2017 at 17:40, Chris Brandt  wrote:
> The existing code gives an incorrect pointer value.
> The buffer pointer 'buf' was of type unsigned short *, and 'count' was a
> number in bytes. A cast of buf should have been used.
>
> However, instead of casting, just change the code to use u32 pointers.
>
> Reported-by: Dan Carpenter 
> Fixes: 8185e51f358a: ("mmc: tmio-mmc: add support for 32bit data port")
> Signed-off-by: Chris Brandt 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied for fixes and added a stable tag.

Kind regards
Uffe

> ---
> v3:
>  * Merged lines
>  * Added Reviewed-by
> v2:
>  * Use u32 pointers instead of casting
> ---
>  drivers/mmc/host/tmio_mmc_core.c | 19 +--
>  1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc_core.c 
> b/drivers/mmc/host/tmio_mmc_core.c
> index 77e7b56a9099..db779732fd2e 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -415,30 +415,29 @@ static void tmio_mmc_transfer_data(struct tmio_mmc_host 
> *host,
>  * Transfer the data
>  */
> if (host->pdata->flags & TMIO_MMC_32BIT_DATA_PORT) {
> -   u8 data[4] = { };
> +   u32 data = 0;
> +   u32 *buf32 = (u32 *)buf;
>
> if (is_read)
> -   sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT, (u32 *)buf,
> +   sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT, buf32,
>count >> 2);
> else
> -   sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT, (u32 
> *)buf,
> +   sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT, buf32,
> count >> 2);
>
> /* if count was multiple of 4 */
> if (!(count & 0x3))
> return;
>
> -   buf8 = (u8 *)(buf + (count >> 2));
> +   buf32 += count >> 2;
> count %= 4;
>
> if (is_read) {
> -   sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT,
> -  (u32 *)data, 1);
> -   memcpy(buf8, data, count);
> +   sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT, , 1);
> +   memcpy(buf32, , count);
> } else {
> -   memcpy(data, buf8, count);
> -   sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT,
> -   (u32 *)data, 1);
> +   memcpy(, buf32, count);
> +   sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT, , 1);
> }
>
> return;
> --
> 2.13.0
>
>

Re: Rebasing mmc/next

[Ulf Hansson]

On 20 June 2017 at 11:06, Geert Uytterhoeven <ge...@linux-m68k.org> wrote:
> Hi Ulf,
>
> On Tue, Jun 20, 2017 at 10:07 AM, Ulf Hansson <ulf.hans...@linaro.org> wrote:
>> On 20 June 2017 at 09:17, Geert Uytterhoeven <ge...@linux-m68k.org> wrote:
>>> It looks like you rebase mmc/next almost daily. Is there any specific reason
>>> for that?
>>
>> I don't do it daily, but often, yes. :-)
>
> A few years ago, I got bashed by Linus for rebasing the m68k "for-linus"
> branch on every -rc.

Perhaps something to be discussed a kernel/mainainer-summit, cause at
the moment I don't think there is given policy.

>
>>> I'm asking because I create a "renesas-drivers" tree on a regular basis
>>> (cfr. e.g. https://www.spinics.net/lists/linux-renesas-soc/msg15111.html).
>>> This tree is meant to ease development of platform support and drivers
>>> for Renesas ARM SoCs. It is created by merging (a) the for-next branches
>>> of various subsystem trees and (b) branches with driver code submitted
>>> or planned for submission to maintainers into the development branch of
>>> Simon Horman's renesas.git tree.
>>
>> If you are asking me to keep my next branch immutable, then please no,
>> I don't like to do that. Reason explained below.
>
> 100% immutable is not needed.
>
>> I don't have a problem to share specific renesas mmc branches with
>> you, if that helps?
>
> Hmm, that would be more work on your side. Plus communication overhead.

OK.

[...]

>
>> I am trying to understand the purpose of your renesas integration
>> tree, and why it's a problem for you to pick up my re-based branch?
>> Could you perhaps elaborate on this?
>
> The purpose is to make it easier for the upstream Renesas kernel team and
> associated testers to consume our work-in-progress.
>
> Hence I merge the for-next branches of selected subsystems, followed by
> topic branches with work-in-progress driver code.
>
> E.g. Simon had asked me to include
> git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git
> topic/sdhi-gen3-dma-2017
> That branch was based on your mmc/next.

I guess there are two other options to deal with this.

1) Don't merge my next branch, in case there is another branch based
on it. Instead, in this case, it will be Simon's responsibility of
re-basing his branch on top of mmc next, as to get the latest changes.
2) Base your WIP branches on top of some commit from Linus tree. Then
you can keep merging my next branch and any other "for-next" branch
that is often being re-based.

>
> But by the time I created the renesas-drivers release, mmc/next had been
> rebased.  Hence after merging the new mmc/next, I had to rebase his work:
> https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/log/?h=topic/sdhi-gen3-dma-2017-rebased1
>
> I hope this explains our problem.

Yeah, thanks for the explanation - and apologize for the delay.

Kind regards
Uffe

Re: [PATCH mmc/next v3 0/4] mmc: renesas_sdhi: add support for R-Car Gen3 SDHI DMAC

[Ulf Hansson]

On 21 June 2017 at 16:00, Simon Horman  wrote:
> Hi,
>
> this series adds support for the internal DMAC used by r8a779[56] SoCs.
> This is achieved by adding a new variant of the SDHI driver for this
> DMA controller with compat strings for the r8a779[56] SoCs.
> Compat strings for these SoCs are also removed from the existing SYS DMAC
> variant of the SDHI driver.
>
> Based on mmc/next
>
> Headline performance boost: 9.5MB/s -> 39.7MB/s
> Details below.
>
> Simon Horman (3):
>   mmc: tmio, renesas-sdhi: add dataend to DMA ops
>   mmc: renesas-sdhi: add support for R-Car Gen3 SDHI DMAC
>   mmc: renesas-sdhi: remove gen3 support from SYS-DMAC driver
>
> Yoshihiro Shimoda (1):
>   mmc: tmio, renesas-sdhi: add max_{segs,blk_count} to tmio_mmc_data
>
>  drivers/mmc/host/Kconfig  |  19 ++
>  drivers/mmc/host/Makefile |   8 +-
>  drivers/mmc/host/renesas_sdhi.h   |   2 +
>  drivers/mmc/host/renesas_sdhi_core.c  |   2 +
>  drivers/mmc/host/renesas_sdhi_internal_dmac.c | 271 
> ++
>  drivers/mmc/host/renesas_sdhi_sys_dmac.c  |  29 +--
>  drivers/mmc/host/tmio_mmc.h   |   2 +
>  drivers/mmc/host/tmio_mmc_core.c  |  16 +-
>  include/linux/mfd/tmio.h  |   2 +
>  9 files changed, 323 insertions(+), 28 deletions(-)
>  create mode 100644 drivers/mmc/host/renesas_sdhi_internal_dmac.c

Thanks, applied patch 1->3 for next!

[...]

Kind regards
Uffe

Re: [PATCH] mmc: tmio: don't wait on R-Car2+ when handling the clock

[Ulf Hansson]

On 28 June 2017 at 17:23, Wolfram Sang  wrote:
> Our hardware engineers confirmed that it is unnecessary to wait when
> turning the clock on/off. The documentation was a tad vague, so we
> used to play safe.
>
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

Kind regards
Uffe

> ---
>
> Tested on H2 and M3-W. Based on top of mmc/next with Simon's Gen3 DMA patches.
>
>  drivers/mmc/host/tmio_mmc_core.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc_core.c 
> b/drivers/mmc/host/tmio_mmc_core.c
> index 77e7b56a909933..1851c883bfc82a 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -207,7 +207,10 @@ static void tmio_mmc_clk_start(struct tmio_mmc_host 
> *host)
>  {
> sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, CLK_CTL_SCLKEN |
> sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
> -   msleep(host->pdata->flags & TMIO_MMC_MIN_RCAR2 ? 1 : 10);
> +
> +   /* HW engineers overrode docs: no sleep needed on R-Car2+ */
> +   if (!(host->pdata->flags & TMIO_MMC_MIN_RCAR2))
> +   msleep(10);
>
> if (host->pdata->flags & TMIO_MMC_HAVE_HIGH_REG) {
> sd_ctrl_write16(host, CTL_CLK_AND_WAIT_CTL, 0x0100);
> @@ -224,7 +227,10 @@ static void tmio_mmc_clk_stop(struct tmio_mmc_host *host)
>
> sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN &
> sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
> -   msleep(host->pdata->flags & TMIO_MMC_MIN_RCAR2 ? 5 : 10);
> +
> +   /* HW engineers overrode docs: no sleep needed on R-Car2+ */
> +   if (!(host->pdata->flags & TMIO_MMC_MIN_RCAR2))
> +   msleep(10);
>  }
>
>  static void tmio_mmc_set_clock(struct tmio_mmc_host *host,
> --
> 2.11.0
>
> --
> 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: renesas_sdhi_core: on R-Car 2+, make use of CBSY bit

[Ulf Hansson]

On 28 June 2017 at 17:21, Wolfram Sang  wrote:
> Most registers need to wait until the command is completed, not
> necessarily until the bus is free. At least, R-Car 2+ SoCs can signal
> that via the CBSY bit, so let's use it there instead of SCLKDIVEN to
> save a little bit of delay.
>
> Signed-off-by: Wolfram Sang 


Thanks, applied for next!

Kind regards
Uffe

> ---
>
> Tested on H2 and M3-W.
>
> Change since V1:
>
> Rebased on top of mmc/next with Simon's Gen3 DMA patches.
>
>  drivers/mmc/host/renesas_sdhi_core.c | 17 -
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c 
> b/drivers/mmc/host/renesas_sdhi_core.c
> index 569bcdd5e6537a..be806d3e9afeec 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -398,12 +398,14 @@ static void renesas_sdhi_hw_reset(struct tmio_mmc_host 
> *host)
>sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
>  }
>
> -static int renesas_sdhi_wait_idle(struct tmio_mmc_host *host)
> +static int renesas_sdhi_wait_idle(struct tmio_mmc_host *host, u32 bit)
>  {
> int timeout = 1000;
> +   /* CBSY is set when busy, SCLKDIVEN is cleared when busy */
> +   u32 wait_state = (bit == TMIO_STAT_CMD_BUSY ? TMIO_STAT_CMD_BUSY : 0);
>
> -   while (--timeout && !(sd_ctrl_read16_and_16_as_32(host, CTL_STATUS)
> - & TMIO_STAT_SCLKDIVEN))
> +   while (--timeout && (sd_ctrl_read16_and_16_as_32(host, CTL_STATUS)
> + & bit) == wait_state)
> udelay(1);
>
> if (!timeout) {
> @@ -416,17 +418,22 @@ static int renesas_sdhi_wait_idle(struct tmio_mmc_host 
> *host)
>
>  static int renesas_sdhi_write16_hook(struct tmio_mmc_host *host, int addr)
>  {
> +   u32 bit = TMIO_STAT_SCLKDIVEN;
> +
> switch (addr) {
> case CTL_SD_CMD:
> case CTL_STOP_INTERNAL_ACTION:
> case CTL_XFER_BLK_COUNT:
> -   case CTL_SD_CARD_CLK_CTL:
> case CTL_SD_XFER_LEN:
> case CTL_SD_MEM_CARD_OPT:
> case CTL_TRANSACTION_CTL:
> case CTL_DMA_ENABLE:
> case EXT_ACC:
> -   return renesas_sdhi_wait_idle(host);
> +   if (host->pdata->flags & TMIO_MMC_MIN_RCAR2)
> +   bit = TMIO_STAT_CMD_BUSY;
> +   /* fallthrough */
> +   case CTL_SD_CARD_CLK_CTL:
> +   return renesas_sdhi_wait_idle(host, bit);
> }
>
> return 0;
> --
> 2.11.0
>
> --
> 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: [RFT] mmc: tmio: fix CMD12 (STOP) handling

[Ulf Hansson]

On 3 July 2017 at 21:28, Wolfram Sang  wrote:
> I always anticipated this code to be not correct, but now I had a test
> case to prove it. According to all documentation I have, setting the
> TMIO_STOP_STP bit ever only worked during block transfers. This bit is
> like manually enforcing an autocmd12 during a so far seamless transfer.
> It does NOT work when the block transfer had errors. It also does NOT
> work with any other cmd except block commands. For all those, CMD12 has
> to be treated like any other command. So, basically, we could use this
> bit only for mrq->data->stop cmds. But for these, we happily use the
> autocmd12 feature using the TMIO_STOP_SEC bit. As a result, the above
> bit is not useful for us and we need to treat CMD12 as a regular cmd
> always. Just remove the special handling code. Note that the BSP
> recognized this issue as well yet had a more cautious solution to the
> problem [1]. Which is understandable but makes CMD12 handling even more
> complicated.
>
> Checked with a Renesas Salvator-X/M3-W which needed to send CMD12 when
> retuning one of my SD cards.
>
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=2838a2ff8ca776f6d18b7fbbe75f3df8dd64183a
>
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

Should we add a stable tag?

Kind regards
Uffe

> ---
>
> So, this is a friendly call for further testing to make sure no regressions
> happen. I also put the authors of the BSP patch to CC to check if this patch
> also matches their use case. I hope this is fine for these people, please
> accept my apologies if not. I just really like to have your opinion on this
> patch.
>
> Geert, Simon: any chance you can run this patch on your board farms?
>
> In any case: as far as my understanding goes, if I am wrong with my
> assumptions, the worst case is that for older SoCs CMD12 handling might become
> a tad more inefficient because we now do in software what was expected to be
> done in hardware. However, I am quite sure that the HW feature was always
> over-estimated and CMD12 support is simply broken. For my test case and the 
> BSP
> patch above, it definately was.
>
> Thanks for any assistance,
>
>Wolfram
>
>
>  drivers/mmc/host/tmio_mmc_core.c | 6 --
>  1 file changed, 6 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc_core.c 
> b/drivers/mmc/host/tmio_mmc_core.c
> index 1851c883bfc82a..fbcd56c58bce24 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -355,12 +355,6 @@ static int tmio_mmc_start_command(struct tmio_mmc_host 
> *host,
> int c = cmd->opcode;
> u32 irq_mask = TMIO_MASK_CMD;
>
> -   /* CMD12 is handled by hardware */
> -   if (cmd->opcode == MMC_STOP_TRANSMISSION && !cmd->arg) {
> -   sd_ctrl_write16(host, CTL_STOP_INTERNAL_ACTION, 
> TMIO_STOP_STP);
> -   return 0;
> -   }
> -
> switch (mmc_resp_type(cmd)) {
> case MMC_RSP_NONE: c |= RESP_NONE; break;
> case MMC_RSP_R1:
> --
> 2.11.0
>
> --
> 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/4] tmio/sdhi: use max sd_buf size & clean header file before

[Ulf Hansson]

On 30 June 2017 at 12:56, Wolfram Sang  wrote:
> From: Wolfram Sang 
>
> The main patch is patch 4 which uses the maximum sd_buf size, i.e. 32 bit on
> Gen2 and 64 bit on Gen3. Instead of a new flag, we use the bus_shift
> information we already have. See the patch description for details.
>
> Before fixing this, I wanted to remove a magic value in that function which is
> done in patch 3.
>
> When adding the new define, I noticed that some description is missing in the
> header which is fixed with patch 2.
>
> When adding the description, I noticed this meanwhile obsolete define which
> gets removed in patch 1.
>
> That's the way it went :)
>
> Tested on Salvator-X/M3-W and Lager/H2.

Thanks, applied for next!

Kind regards
Uffe

>
> Regards,
>
>Wolfram
>
> Wolfram Sang (4):
>   mmc: tmio: remove obsolete TMIO_BBS
>   mmc: tmio: add references to bit defines in the header
>   mmc: tmio: no magic values when enabling DMA
>   mmc: sdhi: use maximum width for the sdbuf register
>
>  drivers/mmc/host/renesas_sdhi_core.c | 5 ++---
>  drivers/mmc/host/tmio_mmc.h  | 7 +--
>  2 files changed, 7 insertions(+), 5 deletions(-)
>
> --
> 2.11.0
>
> --
> 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: Rebasing mmc/next

[Ulf Hansson]

On 20 June 2017 at 09:17, Geert Uytterhoeven  wrote:
> Hi Ulf,
>
> It looks like you rebase mmc/next almost daily. Is there any specific reason
> for that?

I don't do it daily, but often, yes. :-)

>
> I'm asking because I create a "renesas-drivers" tree on a regular basis
> (cfr. e.g. https://www.spinics.net/lists/linux-renesas-soc/msg15111.html).
> This tree is meant to ease development of platform support and drivers
> for Renesas ARM SoCs. It is created by merging (a) the for-next branches
> of various subsystem trees and (b) branches with driver code submitted
> or planned for submission to maintainers into the development branch of
> Simon Horman's renesas.git tree.

If you are asking me to keep my next branch immutable, then please no,
I don't like to do that. Reason explained below.

I don't have a problem to share specific renesas mmc branches with
you, if that helps?

>
> If for (b), people submit driver code based on mmc/next, it may start
> to conflict
> with subsequent mmc/next releases soon, requiring the submitter or me to
> rebase the code before including it in renesas-drivers.
>
> Most subsystem maintainers don't rebase their for-next branch, unless there's
> a very good reason for it (e.g. a serious breakage hindering bisection).

I do it for a couple of reasons.

First, sometimes I apply changes before people have provided enough
tested by tags, to instead allow the changes to be tested in
linux-next. This may lead to some situations when I need to re-base my
next branch.
*) Something breaks, then I need to drop the changes. Revert doesn't
play well here, especially if it's a series of changes.
**) Avoid breaking bisect.
***) I want to give people cred, adding peoples tested-by, reviewed-by
tags, after the changes have been queued on my next branch.

Second, even if the changes queued on next has been thoroughly tested,
sometimes error reports still show up. I most cases I prefer to avoid
breaking bisect, which then leaves me in no other option, but
re-basing my branch (to either drop changes or amend them).

I guess what I can do, is to host a pre-next branch, which serves as
my pre-integration branch, before I moves things to next. However,
this does put some more administrative work on me, so I would like to
avoid that.

I am trying to understand the purpose of your renesas integration
tree, and why it's a problem for you to pick up my re-based branch?
Could you perhaps elaborate on this?

Kind regards
Uffe

Re: [PATCH mmc/next v3 1/2] mmc: tmio: improve checkpatch cleanness

[Ulf Hansson]

+Lee

On 16 June 2017 at 18:11, Simon Horman  wrote:
> Trivial updates to improve checkpatch cleanness.
>
> Signed-off-by: Simon Horman 
> ---
> v3
> * Really don't add bogus ',' after KERN_DEBUG
> * Adjustments as per Wolfram's preferences
>
> v2
> * Don't add bogus ',' after KERN_DEBUG
> * Leave some lines over 80 columns as it seems to be the more readable option
>   (very subjective)
> ---
>  drivers/mmc/host/renesas_sdhi.h  |  2 +-
>  drivers/mmc/host/tmio_mmc.c  |  8 ++---
>  drivers/mmc/host/tmio_mmc.h  | 17 ++-
>  drivers/mmc/host/tmio_mmc_core.c | 66 
> +---
>  include/linux/mfd/tmio.h | 33 ++--

This change requires an ack from Lee Jones, the mfd maintainer.
However, because of the trivial nature of the change, I will apply it
anyways. If there are any objections later on from Lee, than I can
drop it.

Thanks, applied for next!

Kind regards
Uffe

>  5 files changed, 66 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
> index eb3ea15ff92d..6b4a79508c6b 100644
> --- a/drivers/mmc/host/renesas_sdhi.h
> +++ b/drivers/mmc/host/renesas_sdhi.h
> @@ -27,7 +27,7 @@ struct renesas_sdhi_of_data {
> unsigned long capabilities2;
> enum dma_slave_buswidth dma_buswidth;
> dma_addr_t dma_rx_offset;
> -   unsigned bus_shift;
> +   unsigned int bus_shift;
> int scc_offset;
> struct renesas_sdhi_scc *taps;
> int taps_num;
> diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c
> index 61cf36fb270b..64b7e9f18361 100644
> --- a/drivers/mmc/host/tmio_mmc.c
> +++ b/drivers/mmc/host/tmio_mmc.c
> @@ -104,8 +104,8 @@ static int tmio_mmc_probe(struct platform_device *pdev)
> goto host_free;
>
> ret = devm_request_irq(>dev, irq, tmio_mmc_irq,
> -   IRQF_TRIGGER_FALLING,
> -   dev_name(>dev), host);
> +  IRQF_TRIGGER_FALLING,
> +  dev_name(>dev), host);
> if (ret)
> goto host_remove;
>
> @@ -132,6 +132,7 @@ static int tmio_mmc_remove(struct platform_device *pdev)
>
> if (mmc) {
> struct tmio_mmc_host *host = mmc_priv(mmc);
> +
> tmio_mmc_host_remove(host);
> if (cell->disable)
> cell->disable(pdev);
> @@ -145,8 +146,7 @@ static int tmio_mmc_remove(struct platform_device *pdev)
>  static const struct dev_pm_ops tmio_mmc_dev_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(tmio_mmc_suspend, tmio_mmc_resume)
> SET_RUNTIME_PM_OPS(tmio_mmc_host_runtime_suspend,
> -   tmio_mmc_host_runtime_resume,
> -   NULL)
> +  tmio_mmc_host_runtime_resume, NULL)
>  };
>
>  static struct platform_driver tmio_mmc_driver = {
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 768c8abaedda..6ad6704175dc 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -239,24 +239,26 @@ static inline u16 sd_ctrl_read16(struct tmio_mmc_host 
> *host, int addr)
>  }
>
>  static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
> -   u16 *buf, int count)
> + u16 *buf, int count)
>  {
> readsw(host->ctl + (addr << host->bus_shift), buf, count);
>  }
>
> -static inline u32 sd_ctrl_read16_and_16_as_32(struct tmio_mmc_host *host, 
> int addr)
> +static inline u32 sd_ctrl_read16_and_16_as_32(struct tmio_mmc_host *host,
> + int addr)
>  {
> return readw(host->ctl + (addr << host->bus_shift)) |
>readw(host->ctl + ((addr + 2) << host->bus_shift)) << 16;
>  }
>
>  static inline void sd_ctrl_read32_rep(struct tmio_mmc_host *host, int addr,
> -   u32 *buf, int count)
> + u32 *buf, int count)
>  {
> readsl(host->ctl + (addr << host->bus_shift), buf, count);
>  }
>
> -static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr, u16 
> val)
> +static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr,
> +  u16 val)
>  {
> /* If there is a hook and it returns non-zero then there
>  * is an error and the write should be skipped
> @@ -267,19 +269,20 @@ static inline void sd_ctrl_write16(struct tmio_mmc_host 
> *host, int addr, u16 val
>  }
>
>  static inline void sd_ctrl_write16_rep(struct tmio_mmc_host *host, int addr,
> -   u16 *buf, int count)
> +  u16 *buf, int count)
>  {
> writesw(host->ctl + (addr << host->bus_shift), buf, count);
>  }
>
> -static inline void sd_ctrl_write32_as_16_and_16(struct tmio_mmc_host *host, 
> int 

Re: [PATCH mmc/next v3 2/2] mmc: renesas-sdhi: improve checkpatch cleanness

[Ulf Hansson]

On 16 June 2017 at 18:11, Simon Horman  wrote:
> Trivial updates to improve checkpatch cleanness.
>
> Signed-off-by: Simon Horman 
> Reviewed-by: Wolfram Sang 
> Tested-by: Wolfram Sang 

Thanks, applied for next!

Kind regards
Uffe

> ---
> v2
> * Correct one-line comment style
> * Leave bus_shift size comments unchanged
> ---
>  drivers/mmc/host/renesas_sdhi.h  |  2 +-
>  drivers/mmc/host/renesas_sdhi_core.c | 43 
> 
>  drivers/mmc/host/renesas_sdhi_sys_dmac.c | 25 +++
>  3 files changed, 37 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
> index 6b4a79508c6b..ca83acc113b8 100644
> --- a/drivers/mmc/host/renesas_sdhi.h
> +++ b/drivers/mmc/host/renesas_sdhi.h
> @@ -34,6 +34,6 @@ struct renesas_sdhi_of_data {
>  };
>
>  int renesas_sdhi_probe(struct platform_device *pdev,
> -   const struct tmio_mmc_dma_ops *dma_ops);
> +  const struct tmio_mmc_dma_ops *dma_ops);
>  int renesas_sdhi_remove(struct platform_device *pdev);
>  #endif
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c 
> b/drivers/mmc/host/renesas_sdhi_core.c
> index f4690cba3443..a4fb07d0ea91 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -45,7 +45,8 @@
>  #define SDHI_VER_GEN3_SD   0xcc10
>  #define SDHI_VER_GEN3_SDMMC0xcd10
>
> -#define host_to_priv(host) container_of((host)->pdata, struct renesas_sdhi, 
> mmc_data)
> +#define host_to_priv(host) \
> +   container_of((host)->pdata, struct renesas_sdhi, mmc_data)
>
>  struct renesas_sdhi {
> struct clk *clk;
> @@ -94,6 +95,7 @@ static int renesas_sdhi_clk_enable(struct tmio_mmc_host 
> *host)
> struct mmc_host *mmc = host->mmc;
> struct renesas_sdhi *priv = host_to_priv(host);
> int ret = clk_prepare_enable(priv->clk);
> +
> if (ret < 0)
> return ret;
>
> @@ -125,7 +127,7 @@ static int renesas_sdhi_clk_enable(struct tmio_mmc_host 
> *host)
>  }
>
>  static unsigned int renesas_sdhi_clk_update(struct tmio_mmc_host *host,
> - unsigned int new_clock)
> +   unsigned int new_clock)
>  {
> struct renesas_sdhi *priv = host_to_priv(host);
> unsigned int freq, diff, best_freq = 0, diff_min = ~0;
> @@ -175,11 +177,12 @@ static int renesas_sdhi_card_busy(struct mmc_host *mmc)
>  {
> struct tmio_mmc_host *host = mmc_priv(mmc);
>
> -   return !(sd_ctrl_read16_and_16_as_32(host, CTL_STATUS) & 
> TMIO_STAT_DAT0);
> +   return !(sd_ctrl_read16_and_16_as_32(host, CTL_STATUS) &
> +TMIO_STAT_DAT0);
>  }
>
>  static int renesas_sdhi_start_signal_voltage_switch(struct mmc_host *mmc,
> - struct mmc_ios *ios)
> +   struct mmc_ios *ios)
>  {
> struct tmio_mmc_host *host = mmc_priv(mmc);
> struct renesas_sdhi *priv = host_to_priv(host);
> @@ -285,7 +288,7 @@ static unsigned int renesas_sdhi_init_tuning(struct 
> tmio_mmc_host *host)
>  }
>
>  static void renesas_sdhi_prepare_tuning(struct tmio_mmc_host *host,
> -unsigned long tap)
> +   unsigned long tap)
>  {
> struct renesas_sdhi *priv = host_to_priv(host);
>
> @@ -318,9 +321,9 @@ static int renesas_sdhi_select_tuning(struct 
> tmio_mmc_host *host)
> tap_start = 0;
> tap_end = 0;
> for (i = 0; i < host->tap_num * 2; i++) {
> -   if (test_bit(i, host->taps))
> +   if (test_bit(i, host->taps)) {
> ntap++;
> -   else {
> +   } else {
> if (ntap > tap_cnt) {
> tap_start = i - ntap;
> tap_end = i - 1;
> @@ -352,7 +355,6 @@ static int renesas_sdhi_select_tuning(struct 
> tmio_mmc_host *host)
> return 0;
>  }
>
> -
>  static bool renesas_sdhi_check_scc_error(struct tmio_mmc_host *host)
>  {
> struct renesas_sdhi *priv = host_to_priv(host);
> @@ -414,8 +416,7 @@ static int renesas_sdhi_wait_idle(struct tmio_mmc_host 
> *host)
>
>  static int renesas_sdhi_write16_hook(struct tmio_mmc_host *host, int addr)
>  {
> -   switch (addr)
> -   {
> +   switch (addr) {
> case CTL_SD_CMD:
> case CTL_STOP_INTERNAL_ACTION:
> case CTL_XFER_BLK_COUNT:
> @@ -432,7 +433,7 @@ static int renesas_sdhi_write16_hook(struct tmio_mmc_host 
> *host, int addr)
>  }
>
>  static int renesas_sdhi_multi_io_quirk(struct mmc_card *card,
> -unsigned int direction, int blk_size)
> +

Re: [PATCH 1/2] soc: renesas: rcar-sysc: Use GENPD_FLAG_ALWAYS_ON

[Ulf Hansson]

On 12 June 2017 at 11:23, Geert Uytterhoeven <geert+rene...@glider.be> wrote:
> Improve handling of always-on PM domains by using the
> GENPD_FLAG_ALWAYS_ON flag introduced in commit ffaa42e8a40b7f10 ("PM /
> Domains: Enable users of genpd to specify always on PM domains").
>
> Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be>

Reviewed-by: Ulf Hansson <ulf.hans...@linaro.org>

Kind regards
Uffe

> ---
>  drivers/soc/renesas/rcar-sysc.c | 28 
>  drivers/soc/renesas/rcar-sysc.h |  2 --
>  2 files changed, 4 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/soc/renesas/rcar-sysc.c b/drivers/soc/renesas/rcar-sysc.c
> index dcad5c42a5e81c87..7c8da3c90011422f 100644
> --- a/drivers/soc/renesas/rcar-sysc.c
> +++ b/drivers/soc/renesas/rcar-sysc.c
> @@ -181,17 +181,6 @@ static int rcar_sysc_pd_power_off(struct 
> generic_pm_domain *genpd)
> struct rcar_sysc_pd *pd = to_rcar_pd(genpd);
>
> pr_debug("%s: %s\n", __func__, genpd->name);
> -
> -   if (pd->flags & PD_NO_CR) {
> -   pr_debug("%s: Cannot control %s\n", __func__, genpd->name);
> -   return -EBUSY;
> -   }
> -
> -   if (pd->flags & PD_BUSY) {
> -   pr_debug("%s: %s busy\n", __func__, genpd->name);
> -   return -EBUSY;
> -   }
> -
> return rcar_sysc_power_down(>ch);
>  }
>
> @@ -200,12 +189,6 @@ static int rcar_sysc_pd_power_on(struct 
> generic_pm_domain *genpd)
> struct rcar_sysc_pd *pd = to_rcar_pd(genpd);
>
> pr_debug("%s: %s\n", __func__, genpd->name);
> -
> -   if (pd->flags & PD_NO_CR) {
> -   pr_debug("%s: Cannot control %s\n", __func__, genpd->name);
> -   return 0;
> -   }
> -
> return rcar_sysc_power_up(>ch);
>  }
>
> @@ -223,8 +206,7 @@ static void __init rcar_sysc_pd_setup(struct rcar_sysc_pd 
> *pd)
>  * only be turned off if the CPU is not in use.
>  */
> pr_debug("PM domain %s contains %s\n", name, "CPU");
> -   pd->flags |= PD_BUSY;
> -   gov = _domain_always_on_gov;
> +   genpd->flags |= GENPD_FLAG_ALWAYS_ON;
> } else if (pd->flags & PD_SCU) {
> /*
>  * This domain contains an SCU and cache-controller, and
> @@ -232,19 +214,17 @@ static void __init rcar_sysc_pd_setup(struct 
> rcar_sysc_pd *pd)
>  * not in use.
>  */
> pr_debug("PM domain %s contains %s\n", name, "SCU");
> -   pd->flags |= PD_BUSY;
> -   gov = _domain_always_on_gov;
> +   genpd->flags |= GENPD_FLAG_ALWAYS_ON;
> } else if (pd->flags & PD_NO_CR) {
> /*
>  * This domain cannot be turned off.
>  */
> -   pd->flags |= PD_BUSY;
> -   gov = _domain_always_on_gov;
> +   genpd->flags |= GENPD_FLAG_ALWAYS_ON;
> }
>
> if (!(pd->flags & (PD_CPU | PD_SCU))) {
> /* Enable Clock Domain for I/O devices */
> -   genpd->flags = GENPD_FLAG_PM_CLK;
> +   genpd->flags |= GENPD_FLAG_PM_CLK;
> if (has_cpg_mstp) {
> genpd->attach_dev = cpg_mstp_attach_dev;
> genpd->detach_dev = cpg_mstp_detach_dev;
> diff --git a/drivers/soc/renesas/rcar-sysc.h b/drivers/soc/renesas/rcar-sysc.h
> index 07edb049a401196c..1a5bebaf54ba191c 100644
> --- a/drivers/soc/renesas/rcar-sysc.h
> +++ b/drivers/soc/renesas/rcar-sysc.h
> @@ -20,8 +20,6 @@
>  #define PD_SCU BIT(1)  /* Area contains SCU and L2 cache */
>  #define PD_NO_CR   BIT(2)  /* Area lacks PWR{ON,OFF}CR registers */
>
> -#define PD_BUSYBIT(3)  /* Busy, for internal use only */
> -
>  #define PD_CPU_CR  PD_CPU/* CPU area has CR (R-Car H1) */
>  #define PD_CPU_NOCRPD_CPU | PD_NO_CR /* CPU area lacks CR (R-Car Gen2/3) 
> */
>  #define PD_ALWAYS_ON   PD_NO_CR  /* Always-on area */
> --
> 2.7.4
>

Re: [PATCH 2/2] ARM: shmobile: pm-rmobile: Use GENPD_FLAG_ALWAYS_ON

[Ulf Hansson]

On 12 June 2017 at 11:23, Geert Uytterhoeven <geert+rene...@glider.be> wrote:
> Improve handling of always-on PM domains by using the
> GENPD_FLAG_ALWAYS_ON flag introduced in commit ffaa42e8a40b7f10 ("PM /
> Domains: Enable users of genpd to specify always on PM domains").
>
> Note that the PM domain containing the serial console is still handled
> locally.
>
> Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be>

Reviewed-by: Ulf Hansson <ulf.hans...@linaro.org>

Kind regards
Uffe


> ---
>  arch/arm/mach-shmobile/pm-rmobile.c | 19 ---
>  1 file changed, 4 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm/mach-shmobile/pm-rmobile.c 
> b/arch/arm/mach-shmobile/pm-rmobile.c
> index 175bd3d91ebcbcb4..e5fa6a3cf1ddf159 100644
> --- a/arch/arm/mach-shmobile/pm-rmobile.c
> +++ b/arch/arm/mach-shmobile/pm-rmobile.c
> @@ -130,7 +130,7 @@ static void rmobile_init_pm_domain(struct 
> rmobile_pm_domain *rmobile_pd)
> struct generic_pm_domain *genpd = _pd->genpd;
> struct dev_power_governor *gov = rmobile_pd->gov;
>
> -   genpd->flags = GENPD_FLAG_PM_CLK;
> +   genpd->flags |= GENPD_FLAG_PM_CLK;
> genpd->dev_ops.active_wakeup= rmobile_pd_active_wakeup;
> genpd->power_off= rmobile_pd_power_down;
> genpd->power_on = rmobile_pd_power_up;
> @@ -140,14 +140,6 @@ static void rmobile_init_pm_domain(struct 
> rmobile_pm_domain *rmobile_pd)
> pm_genpd_init(genpd, gov ? : _qos_governor, false);
>  }
>
> -static int rmobile_pd_suspend_busy(void)
> -{
> -   /*
> -* This domain should not be turned off.
> -*/
> -   return -EBUSY;
> -}
> -
>  static int rmobile_pd_suspend_console(void)
>  {
> /*
> @@ -260,8 +252,7 @@ static void __init rmobile_setup_pm_domain(struct 
> device_node *np,
>  * only be turned off if the CPU is not in use.
>  */
> pr_debug("PM domain %s contains CPU\n", name);
> -   pd->gov = _domain_always_on_gov;
> -   pd->suspend = rmobile_pd_suspend_busy;
> +   pd->genpd.flags |= GENPD_FLAG_ALWAYS_ON;
> break;
>
> case PD_CONSOLE:
> @@ -277,8 +268,7 @@ static void __init rmobile_setup_pm_domain(struct 
> device_node *np,
>  * is not in use.
>  */
> pr_debug("PM domain %s contains Coresight-ETM\n", name);
> -   pd->gov = _domain_always_on_gov;
> -   pd->suspend = rmobile_pd_suspend_busy;
> +   pd->genpd.flags |= GENPD_FLAG_ALWAYS_ON;
> break;
>
> case PD_MEMCTL:
> @@ -287,8 +277,7 @@ static void __init rmobile_setup_pm_domain(struct 
> device_node *np,
>  * should only be turned off if memory is not in use.
>  */
> pr_debug("PM domain %s contains MEMCTL\n", name);
> -   pd->gov = _domain_always_on_gov;
> -   pd->suspend = rmobile_pd_suspend_busy;
> +   pd->genpd.flags |= GENPD_FLAG_ALWAYS_ON;
> break;
>
> case PD_NORMAL:
> --
> 2.7.4
>

Re: [RFC PATCH v2 0/2] mmc: core: process ECC errors raised in stop cmds

[Ulf Hansson]

On 8 April 2017 at 22:20, Wolfram Sang  wrote:
> Here is the second RFC for handling ECC errors flagged in the stop command
> after a multiblock transfer. It is still RFC because I could only test it by
> inducing ECC errors in software (see patch for TMIO below). Shimoda-san, can
> you try this series with the SD tester again? That would be very kind.
>
> Other than that, I hope the patch descriptions and comments explain the single
> steps. Looking forward for thoughts.

Apologize for the delay!

I looked into these and the changes seems reasonable. I have applied
them for next to allow them to get some test coverage.

Thanks and kind regards
Uffe

>
> Kind regards,
>
>Wolfram
>
> Changes since RFC v1:
>
> * rebased to mmc/next as of today
> * reworded commit message for patch 1
> * added tested-tag from Shimoda-san for patch 1
> * added patch 2
>
> Wolfram Sang (2):
>   mmc: core: check also R1 response for stop commands
>   mmc: core: for data errors, take response of stop cmd into account
>
>  drivers/mmc/core/block.c | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> Patch to simulate ECC errors:
>
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index a2d92f10501bdd..9773c7e5e4d154 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -553,6 +553,8 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host)
> }
>
> if (stop) {
> +   static unsigned int induce_cnt = 0;
> +
> if (stop->opcode != MMC_STOP_TRANSMISSION || stop->arg)
> dev_err(>pdev->dev, "unsupported stop: 
> CMD%u,0x%x. We did CMD12,0\n",
> stop->opcode, stop->arg);
> @@ -560,6 +562,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host)
> /* fill in response from auto CMD12 */
> stop->resp[0] = sd_ctrl_read16_and_16_as_32(host, 
> CTL_RESPONSE);
>
> +   if (induce_cnt++ % 100 == 0)
> +   stop->resp[0] |= R1_CARD_ECC_FAILED;
> +
> sd_ctrl_write16(host, CTL_STOP_INTERNAL_ACTION, 0);
> }
>
> --
> 2.11.0
>

Re: SDHI clock imbalance

[Ulf Hansson]

On 2 June 2017 at 10:47, Geert Uytterhoeven  wrote:
> Hi Wolfram, Ulf, Simon,
>
> While investigating suspend/resume for the R-Car Gen3 clock driver, I
> noticed a clock imbalance for SDHI on Salvator-X.
>
> After boot:
>
> # head -2 /sys/kernel/debug/clk/clk_summary
>clock enable_cnt prepare_cnt  rate  accuracy phase
> -
> # grep sd /sys/kernel/debug/clk/clk_summary
>  .sdsrc   33 39984  0 0
> sd3   11 625  0 0
>sdif3  12 625  0 0
> sd2   11   19992  0 0
>sdif2  12   19992  0 0
> sd1   00   19992  0 0
>sdif1  00   19992  0 0
> sd0   11 625  0 0
>sdif0  12 625  0 0
>
> After s2idle suspend/resume:
>
> # grep sd /sys/kernel/debug/clk/clk_summary
>  .sdsrc   33   39984  0 0
> sd3   11 625  0 0
>sdif3  22 625  0 0
> sd2   11   19992  0 0
>sdif2  22   19992  0 0
> sd1   00   19992  0 0
>sdif1  00   19992  0 0
> sd0   11 625  0 0
>sdif0  22 625  0 0
>
> Enable counts are 1 before suspend, and 2 after resume.
>
>
> Boot: Enabled once (also at hardware level):
>
> platform_drv_probe
> renesas_sdhi_sys_dmac_probe
> renesas_sdhi_probe
> tmio_mmc_host_probe
> renesas_sdhi_clk_enable

The driver calls pm_runtime_set_active() during ->probe(), which means
genpd's ->runtime_resume() callback isn't invoked during that point.
In other words, the clocks managed by the clock domain isn't enabled
during ->probe() as genpd's doesn't get to run pm_clk_resume() from
its ->runtime_resume() callback. Unless I am missing something. :-)

I think this is the reason to the following problems. How to fix it?

The driver needs to call pm_runtime_get_sync() instead of
pm_runtime_set_active(), however that may requires some careful
changes if one wants the driver to be able to enable clocks also when
CONFIG_PM is unset. If not, it's pretty easy, else I would do
something like below.

Add a "init" flag to host struct, and set that flag before calling
pm_runtime_get_sync() in ->probe(). When the driver's
->runtime_resume() callbacks get called when the "init" flag is set,
just do an early return 0, as t means ->probe() is running and has
already taken care of the enabling the clock. When probe is done, and
before dropping runtime usage count with pm_runtime_put(), reset the
"init" flag.

>
>
> Suspend: Disabled once (also at hardware level):
>
> suspend_devices_and_enter
> dpm_suspend_start
> dpm_suspend
> __device_suspend
> dpm_run_callback
> pm_runtime_force_suspend
> genpd_runtime_suspend
> pm_generic_runtime_suspend
> tmio_mmc_host_runtime_suspend
> renesas_sdhi_clk_disable
>
>
> Resume: Enabled twice (first one enables at hardware level):
>
> dpm_resume_noirq
> device_resume_noirq
> dpm_run_callback
> pm_genpd_resume_noirq
> pm_runtime_force_resume
> genpd_runtime_resume
> genpd_start_dev
> pm_clk_resume   (1)
> __genpd_runtime_resume
> pm_generic_runtime_resume
> tmio_mmc_host_runtime_resume
> renesas_sdhi_clk_enable (2)
>
>
> During subsequent suspends, the clock is disabled twice (last one disables
> at hardware level), as expected:
>
> suspend_devices_and_enter
> dpm_suspend_start
> dpm_suspend
> __device_suspend
> dpm_run_callback
> pm_runtime_force_suspend
> genpd_runtime_suspend
> pm_generic_runtime_suspend
> tmio_mmc_host_runtime_suspend
> renesas_sdhi_clk_disable (1)
> genpd_stop_dev
> pm_clk_suspend   (2)
>
>
> From now on, the imbalance is gone.
>
> Note that at boot and initial suspend, genpd does not seem to 

Re: [PATCH] mmc: tmio: make sure SDIO gets reinitialized after resume

[Ulf Hansson]

On 23 May 2017 at 15:34, Wolfram Sang  wrote:
> To achieve that, we set the registers in the generic HW reset routine
> which gets called at both, init and resume. We also make sure to move
> SDIO initialization before reset gets called in probe().
>
> Signed-off-by: Wolfram Sang 
> Tested-by: Masaharu Hayakawa 

Thanks, applied for next!

Kind regards
Uffe

> ---
>
> Changes since RFT:
> * added tag from Hayakawa-san (thanks!)
> * rebased to mmc/next (on top of the DMA refactor series)
>
>  drivers/mmc/host/tmio_mmc_core.c | 17 ++---
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc_core.c 
> b/drivers/mmc/host/tmio_mmc_core.c
> index e1ad461c4b8c8f..ed40681253497a 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -265,6 +265,12 @@ static void tmio_mmc_reset(struct tmio_mmc_host *host)
> if (host->pdata->flags & TMIO_MMC_HAVE_HIGH_REG)
> sd_ctrl_write16(host, CTL_RESET_SDIO, 0x0001);
> msleep(10);
> +
> +   if (host->pdata->flags & TMIO_MMC_SDIO_IRQ) {
> +   sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
> +   sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001);
> +   }
> +
>  }
>
>  static void tmio_mmc_reset_work(struct work_struct *work)
> @@ -1280,6 +1286,10 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
> if (_host->native_hotplug)
> pm_runtime_get_noresume(>dev);
>
> +   _host->sdio_irq_enabled = false;
> +   if (pdata->flags & TMIO_MMC_SDIO_IRQ)
> +   _host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
> +
> tmio_mmc_clk_stop(_host);
> tmio_mmc_reset(_host);
>
> @@ -1296,13 +1306,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
>
> _host->sdcard_irq_mask &= ~irq_mask;
>
> -   _host->sdio_irq_enabled = false;
> -   if (pdata->flags & TMIO_MMC_SDIO_IRQ) {
> -   _host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
> -   sd_ctrl_write16(_host, CTL_SDIO_IRQ_MASK, 
> _host->sdio_irq_mask);
> -   sd_ctrl_write16(_host, CTL_TRANSACTION_CTL, 0x0001);
> -   }
> -
> spin_lock_init(&_host->lock);
> mutex_init(&_host->ios_lock);
>
> --
> 2.11.0
>
> --
> 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 4/7] mmc: use proper name for the R-Car SoC

[Ulf Hansson]

On 30 May 2017 at 09:16, Lee Jones <lee.jo...@linaro.org> wrote:
> On Mon, 29 May 2017, Ulf Hansson wrote:
>
>> On 28 May 2017 at 11:30, Wolfram Sang <wsa+rene...@sang-engineering.com> 
>> wrote:
>> > It is 'R-Car', not 'RCar'. No code or binding changes, only descriptive 
>> > text.
>> >
>> > Signed-off-by: Wolfram Sang <wsa+rene...@sang-engineering.com>
>>
>> Thanks, applied for next!
>>
>> For mfd, Lee, please tell if you have any issues me picking this via
>> my mmc tree.
>
> I don't, but it is normal procedure to wait for an Ack from all
> Maintainers involved before applying. ;)

Yeah! However this was that trivial so I went ahead. :-)

Kind regards
Uffe

Re: [PATCH 4/7] mmc: use proper name for the R-Car SoC

[Ulf Hansson]

On 28 May 2017 at 11:30, Wolfram Sang  wrote:
> It is 'R-Car', not 'RCar'. No code or binding changes, only descriptive text.
>
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

For mfd, Lee, please tell if you have any issues me picking this via
my mmc tree.

Kind regards
Uffe

> ---
> I suggest this trivial patch should be picked individually per susbsystem.
>
>  drivers/mmc/host/renesas_sdhi_core.c | 2 +-
>  include/linux/mfd/tmio.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c 
> b/drivers/mmc/host/renesas_sdhi_core.c
> index 846ee1a8e5a675..397b3e29977ea8 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -130,7 +130,7 @@ static unsigned int renesas_sdhi_clk_update(struct 
> tmio_mmc_host *host,
> unsigned int freq, diff, best_freq = 0, diff_min = ~0;
> int i, ret;
>
> -   /* tested only on RCar Gen2+ currently; may work for others */
> +   /* tested only on R-Car Gen2+ currently; may work for others */
> if (!(host->pdata->flags & TMIO_MMC_MIN_RCAR2))
> return clk_get_rate(priv->clk);
>
> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
> index a1520d88ebf3a3..c83c16b931a886 100644
> --- a/include/linux/mfd/tmio.h
> +++ b/include/linux/mfd/tmio.h
> @@ -66,7 +66,7 @@
>   */
>  #define TMIO_MMC_SDIO_IRQ  (1 << 2)
>
> -/* Some features are only available or tested on RCar Gen2 or later */
> +/* Some features are only available or tested on R-Car Gen2 or later */
>  #define TMIO_MMC_MIN_RCAR2 (1 << 3)
>
>  /*
> --
> 2.11.0
>

Re: [PATCH v2 0/6] tmio/sdhi: add cmd23 support

[Ulf Hansson]

On 19 May 2017 at 15:31, Wolfram Sang  wrote:
> This series adds CMD23 support to SDHI. It was tested on H2 (Gen2, Lager) and
> M3W (Gen3, Salvator-X). The test procedure can be found here:
>
> http://elinux.org/Tests:SDHI-CMD23
>
> Patches are based on mmc/next from today. A branch can be found here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
> renesas/topic/sdhi-cmd23
>
> Changes since RFC:
>
> * rebased on top of Simon's DMA rework now in mmc/next
> * added Geerts tags
> * cosmetic change in patch 6, put new capability in a separate line
>
> Thanks,
>
>Wolfram
>
>
> Wolfram Sang (6):
>   mmc: tmio: make tmio_mmc_request function more readable
>   mmc: tmio: refactor handling mrq
>   mmc: tmio: remove outdated comment
>   mmc: tmio: move finish_request function further down
>   mmc: tmio: add CMD23 support
>   mmc: sdhi: add CMD23 support to R-Car Gen2 & Gen3
>
>  drivers/mmc/host/renesas_sdhi_sys_dmac.c |   6 +-
>  drivers/mmc/host/tmio_mmc_core.c | 135 
> ++-
>  2 files changed, 82 insertions(+), 59 deletions(-)


Thanks, applied for next!

Kind regards
Uffe

Re: [PATCH v2 0/6] mmc: renesas-sdhi: refactor DMA support

[Ulf Hansson]

On 18 May 2017 at 22:14, Wolfram Sang  wrote:
> On Wed, May 10, 2017 at 11:25:24AM +0200, Simon Horman wrote:
>> Hi Wolfram, Hi Ulf, Hi Arnd, Hi all,
>>
>> the intention of this patch-set is to refactor the DMA support in
>> the Renesas SDHI driver in order to make it easier to add support
>> for using the SDHI hardware with different DMA implementations.
>>
>> This is based on earlier work, posted as "[PATCH/RFC v3 0/6] mmc:
>> renesas_sdhi: add R-Car Gen-3 DMA support". It attempts to implement
>> the reworking of the driver proposed by Arnd[1] in his review of that
>> patch-set.
>>
>> [1] http://www.spinics.net/lists/linux-mmc/msg38004.html
>>
>> Unlike that patch-set this patch-set does not add support for
>> R-Car Gen-3 DMA. Rather it focuses on refactoring the code.
>>
>> Changes between RFC and v2:
>>
>> * Drop filenames from comment at top of source
>> * Consistently check for if (host->dma_ops) before using dma_ops.
>>
>>
>> Simon Horman (6):
>>   mmc: tmio: drop filenames from comment at top of source
>>   mmc: renesas-sdhi, tmio: make dma more modular
>>   mmc: tmio: rename tmio_mmc_{pio => core}.c
>>   mmc: renesas-sdhi: rename tmio_mmc_dma.c => renesas_sdhi_sys_dmac.c
>>   mmc: renesas-sdhi: rename sh_mobile_sdhi.c => renesas_sdhi_core.c
>>   mmc: renesas-sdhi: make renesas_sdhi_sys_dmac main module file
>
> Thanks Simon for this series! The file layout looks in deed much better
> now. And some light testing on my Lager didn't show any regressions. I
> have only minor comments which do not have anything to do with the code,
> so already here:
>
> Reviewed-by: Wolfram Sang 
>
> The comments:
>
> * MAINTAINERS file needs updating because of the new filenames
> * I'd prefer the _GPL variant of EXPORT_SYMBOL unless we have a reason
>   to not use it?
> * maybe this is also a good time to update the Renesas copyrights in
>   file headers?
> * checkpatch reports some whitespace errors on patch 6. I assume they
>   were already there in the code you moved around. Still, it might be
>   a nice occasion to fix those?
>
> Thanks again, nice work!
>
>Wolfram
>

I have applied this for next, assuming Simon addresses Wolfram's
comments on top.

Thanks and kind regards
Uffe

Re: [PATCH v4] mmc: sh_mmcif: Document r7s72100 DT bindings

[Ulf Hansson]

On 22 March 2017 at 15:42, Chris Brandt  wrote:
> Signed-off-by: Chris Brandt 
> Reviewed-by: Geert Uytterhoeven 
> Reviewed-by: Simon Horman 
> Acked-by: Rob Herring 

Thanks, applied for next!

Kind regards
Uffe

> ---
> v4:
> * added Reviewed-by and Acked-by
> v3:
> * added list of how many interrupts each SOC has
> v2:
> * added interrupt description
> ---
>  Documentation/devicetree/bindings/mmc/renesas,mmcif.txt | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt 
> b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> index e4ba92aa..c32dc5a 100644
> --- a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> +++ b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> @@ -8,6 +8,7 @@ Required properties:
>
>  - compatible: should be "renesas,mmcif-", "renesas,sh-mmcif" as a
>fallback. Examples with  are:
> +   - "renesas,mmcif-r7s72100" for the MMCIF found in r7s72100 SoCs
> - "renesas,mmcif-r8a73a4" for the MMCIF found in r8a73a4 SoCs
> - "renesas,mmcif-r8a7740" for the MMCIF found in r8a7740 SoCs
> - "renesas,mmcif-r8a7778" for the MMCIF found in r8a7778 SoCs
> @@ -17,6 +18,13 @@ Required properties:
> - "renesas,mmcif-r8a7794" for the MMCIF found in r8a7794 SoCs
> - "renesas,mmcif-sh73a0" for the MMCIF found in sh73a0 SoCs
>
> +- interrupts: Some SoCs have only 1 shared interrupt, while others have 
> either
> +  2 or 3 individual interrupts (error, int, card detect). Below is the number
> +  of interrupts for each SoC:
> +1: r8a73a4, r8a7778, r8a7790, r8a7791, r8a7793, r8a7794
> +2: r8a7740, sh73a0
> +3: r7s72100
> +
>  - clocks: reference to the functional clock
>
>  - dmas: reference to the DMA channels, one per channel name listed in the
> --
> 2.10.1
>
>

Re: [PATCH v3] mmc: sh_mmcif: Document r7s72100 DT bindings

[Ulf Hansson]

On 22 March 2017 at 14:17, Chris Brandt  wrote:
> This one seems to have been forgotten.
>
> Should I resend with all the acks and reviews?

Seems like you didn't send it to linux-mmc, so it wasn't picked up by
the patchtracker. Please re-send so I can pick it up.

Kind regards
Uffe

>
> Chris
>
>
>
> On Wednesday, January 18, 2017, Rob Herring wrote:
>> On Wed, Jan 11, 2017 at 11:14:52PM -0500, Chris Brandt wrote:
>> > Signed-off-by: Chris Brandt 
>> >
>> > ---
>> > v3:
>> > * added list of how many interrupts each SOC has
>> > v2:
>> > * added interrupt description
>> > ---
>> >  Documentation/devicetree/bindings/mmc/renesas,mmcif.txt | 8 
>> >  1 file changed, 8 insertions(+)
>>
>> Acked-by: Rob Herring 
>
>
>
> On Thursday, January 12, 2017, Simon Horman wrote:
>> On Thu, Jan 12, 2017 at 08:36:10AM +0100, Geert Uytterhoeven wrote:
>> > Hi Chris,
>> >
>> > On Thu, Jan 12, 2017 at 5:14 AM, Chris Brandt 
>> wrote:
>> > > Signed-off-by: Chris Brandt 
>> > >
>> > > ---
>> > > v3:
>> > > * added list of how many interrupts each SOC has
>> > > v2:
>> > > * added interrupt description
>> >
>> > Thanks or the update!
>> >
>> > Reviewed-by: Geert Uytterhoeven 
>>
>> Reviewed-by: Simon Horman 
>
>
> On Thursday, January 12, 2017, Geert Uytterhoeven wrote:
>> On Thu, Jan 12, 2017 at 5:14 AM, Chris Brandt 
>> wrote:
>> > Signed-off-by: Chris Brandt 
>> >
>> > ---
>> > v3:
>> > * added list of how many interrupts each SOC has
>> > v2:
>> > * added interrupt description
>>
>> Thanks or the update!
>>
>> Reviewed-by: Geert Uytterhoeven 
>

Re: [PATCH] mmc: tmio: always get number of taps

[Ulf Hansson]

On 17 March 2017 at 10:04, Wolfram Sang
 wrote:
> From: Masaharu Hayakawa 
>
> Current code gets number of taps only once and keeps the value. This is
> not correct, we need to obtain it every time before executing tuning,
> so remove the outer if-block.
>
> Signed-off-by: Masaharu Hayakawa 
> [wsa: extracted from a larger patch and reworded commit message]
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

Kind regards
Uffe



> ---
>  drivers/mmc/host/tmio_mmc_pio.c | 16 +++-
>  1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index 5b01d22932cdbf..a2d92f10501bdd 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -815,16 +815,14 @@ static int tmio_mmc_execute_tuning(struct mmc_host 
> *mmc, u32 opcode)
> struct tmio_mmc_host *host = mmc_priv(mmc);
> int i, ret = 0;
>
> -   if (!host->tap_num) {
> -   if (!host->init_tuning || !host->select_tuning)
> -   /* Tuning is not supported */
> -   goto out;
> +   if (!host->init_tuning || !host->select_tuning)
> +   /* Tuning is not supported */
> +   goto out;
>
> -   host->tap_num = host->init_tuning(host);
> -   if (!host->tap_num)
> -   /* Tuning is not supported */
> -   goto out;
> -   }
> +   host->tap_num = host->init_tuning(host);
> +   if (!host->tap_num)
> +   /* Tuning is not supported */
> +   goto out;
>
> if (host->tap_num * 2 >= sizeof(host->taps) * BITS_PER_BYTE) {
> dev_warn_once(>pdev->dev,
> --
> 2.11.0
>
> --
> 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: [RFC] mmc: tmio: always unmap DMA before waiting for interrupt

[Ulf Hansson]

On 16 March 2017 at 11:56, Wolfram Sang
 wrote:
> In the (maybe academical) case, we don't get a DATAEND interrupt after
> DMA completed, we will wait endlessly for the completion to complete.
> This is not bad per se, since we have a more generic completion tracking
> a timeout. In that rare case, however, the DMA buffer will not get
> unmapped and we have a leak. Reorder the code, so unmapping will always
> take place.
>
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

Kind regards
Uffe

> ---
>
> It's probably academical, still I think it is better to not have any leaks in
> favor of slightly more lock hazzling. Open for opionions, though, this is why
> I send out as RFC.
>
>  drivers/mmc/host/tmio_mmc_dma.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
> index c7684fa91f1f9c..e2093db2b7ffce 100644
> --- a/drivers/mmc/host/tmio_mmc_dma.c
> +++ b/drivers/mmc/host/tmio_mmc_dma.c
> @@ -47,8 +47,6 @@ static void tmio_mmc_dma_callback(void *arg)
>  {
> struct tmio_mmc_host *host = arg;
>
> -   wait_for_completion(>dma_dataend);
> -
> spin_lock_irq(>lock);
>
> if (!host->data)
> @@ -63,6 +61,11 @@ static void tmio_mmc_dma_callback(void *arg)
>  host->sg_ptr, host->sg_len,
>  DMA_TO_DEVICE);
>
> +   spin_unlock_irq(>lock);
> +
> +   wait_for_completion(>dma_dataend);
> +
> +   spin_lock_irq(>lock);
> tmio_mmc_do_data_irq(host);
>  out:
> spin_unlock_irq(>lock);
> --
> 2.11.0
>
> --
> 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 v3 0/4] mmc: tmio: properly report status from autocmd12

[Ulf Hansson]

On 14 March 2017 at 11:09, Wolfram Sang
 wrote:
> New in V3:
>
> * rebased to v4.11-rc2 and retested
> * added one more tag from Simon
>
> New in V2:
> * Fix some more comments in patch 2.
> * Add Simon's tags
>
> SDHI automatically sends CMD12 when the desired amount of data was transferred
> after multi block commands. However, the response from that CMD12 was never
> reported back to the mmc core, so that errors like ECC might go unnoticed. 
> This
> series aims to fix that and clean up minor issues on the way when dealing with
> the autocmd12 feature. Testing documentation can be found here [1] and thanks
> must go to Shimoda-san for additional testing. Actually handling the ECC 
> errors
> in the core is a seperate issue and I'll post a patch for it right after this.
>
> The patches are based on mmc-next as of today. A branch can be found here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
> renesas/topic/sdhi-autocmd12-resp-v3
>
> Thanks and all the best,
>
>Wolfram
>
> [1] http://elinux.org/Tests:SDHI-autocmd12-responses
>
> Wolfram Sang (4):
>   mmc: host: tmio: use defines for CTL_STOP_INTERNAL_ACTION values
>   mmc: host: tmio: fix minor typos in comments
>   mmc: host: tmio: don't BUG on unsupported stop commands
>   mmc: host: tmio: fill in response from auto cmd12
>
>  drivers/mmc/host/tmio_mmc.h | 10 +++---
>  drivers/mmc/host/tmio_mmc_pio.c | 16 ++--
>  2 files changed, 17 insertions(+), 9 deletions(-)
>
> --

Thanks, applied for next!

Kind regards
Uffe

Re: [PATCH v2] mmc: host: tmio: ensure end of DMA and SD access are in sync

[Ulf Hansson]

On 17 February 2017 at 19:22, Wolfram Sang
 wrote:
> The current code assumes that DMA is finished before SD access end is
> flagged. Thus, it schedules the 'dma_complete' tasklet in the SD card
> interrupt routine when DATAEND is set. The assumption is not safe,
> though. Even by mounting an SD card, it can be seen that sometimes DMA
> complete is first, sometimes DATAEND. It seems they are usually close
> enough timewise to not cause problems. However, a customer reported that
> with CMD53 sometimes things really break apart. As a result, the BSP has
> a patch which introduces flags for both events and makes sure both flags
> are set before scheduling the tasklet. The customer accepted the patch,
> yet it doesn't seem a proper upstream solution to me.
>
> This patch refactors the code to replace the tasklet with already
> existing and more lightweight mechanisms. First of all, we set the
> callback in a DMA descriptor to automatically get notified when DMA is
> done. In the callback, we then use a completion to make sure the SD
> access has already ended. Then, we proceed as before.
>
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

Kind regards
Uffe

> ---
>
> Changes since V1/RFC:
>
> * removed #ifdef DEBUG
> * rebased to latest mmc/next
> * did some more performance testing. Couldn't spot any difference
>
>  drivers/mmc/host/tmio_mmc.h |  2 +-
>  drivers/mmc/host/tmio_mmc_dma.c | 58 
> -
>  drivers/mmc/host/tmio_mmc_pio.c |  4 +--
>  3 files changed, 37 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 2b349d48fb9a8a..891d400d2a7cf2 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -137,7 +137,7 @@ struct tmio_mmc_host {
> boolforce_pio;
> struct dma_chan *chan_rx;
> struct dma_chan *chan_tx;
> -   struct tasklet_struct   dma_complete;
> +   struct completion   dma_dataend;
> struct tasklet_struct   dma_issue;
> struct scatterlist  bounce_sg;
> u8  *bounce_buf;
> diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
> index fa8a936a3d9ba1..c7684fa91f1f9c 100644
> --- a/drivers/mmc/host/tmio_mmc_dma.c
> +++ b/drivers/mmc/host/tmio_mmc_dma.c
> @@ -43,6 +43,31 @@ void tmio_mmc_abort_dma(struct tmio_mmc_host *host)
> tmio_mmc_enable_dma(host, true);
>  }
>
> +static void tmio_mmc_dma_callback(void *arg)
> +{
> +   struct tmio_mmc_host *host = arg;
> +
> +   wait_for_completion(>dma_dataend);
> +
> +   spin_lock_irq(>lock);
> +
> +   if (!host->data)
> +   goto out;
> +
> +   if (host->data->flags & MMC_DATA_READ)
> +   dma_unmap_sg(host->chan_rx->device->dev,
> +host->sg_ptr, host->sg_len,
> +DMA_FROM_DEVICE);
> +   else
> +   dma_unmap_sg(host->chan_tx->device->dev,
> +host->sg_ptr, host->sg_len,
> +DMA_TO_DEVICE);
> +
> +   tmio_mmc_do_data_irq(host);
> +out:
> +   spin_unlock_irq(>lock);
> +}
> +
>  static void tmio_mmc_start_dma_rx(struct tmio_mmc_host *host)
>  {
> struct scatterlist *sg = host->sg_ptr, *sg_tmp;
> @@ -88,6 +113,10 @@ static void tmio_mmc_start_dma_rx(struct tmio_mmc_host 
> *host)
> DMA_DEV_TO_MEM, DMA_CTRL_ACK);
>
> if (desc) {
> +   reinit_completion(>dma_dataend);
> +   desc->callback = tmio_mmc_dma_callback;
> +   desc->callback_param = host;
> +
> cookie = dmaengine_submit(desc);
> if (cookie < 0) {
> desc = NULL;
> @@ -162,6 +191,10 @@ static void tmio_mmc_start_dma_tx(struct tmio_mmc_host 
> *host)
> DMA_MEM_TO_DEV, DMA_CTRL_ACK);
>
> if (desc) {
> +   reinit_completion(>dma_dataend);
> +   desc->callback = tmio_mmc_dma_callback;
> +   desc->callback_param = host;
> +
> cookie = dmaengine_submit(desc);
> if (cookie < 0) {
> desc = NULL;
> @@ -221,29 +254,6 @@ static void tmio_mmc_issue_tasklet_fn(unsigned long priv)
> dma_async_issue_pending(chan);
>  }
>
> -static void tmio_mmc_tasklet_fn(unsigned long arg)
> -{
> -   struct tmio_mmc_host *host = (struct tmio_mmc_host *)arg;
> -
> -   spin_lock_irq(>lock);
> -
> -   if (!host->data)
> -   goto out;
> -
> -   if (host->data->flags & MMC_DATA_READ)
> -   dma_unmap_sg(host->chan_rx->device->dev,
> -host->sg_ptr, host->sg_len,
> -DMA_FROM_DEVICE);
> -   else
> -   dma_unmap_sg(host->chan_tx->device->dev,
> -   

Re: [PATCH v2 4/4] mmc: host: tmio: fill in response from auto cmd12

[Ulf Hansson]

On 16 February 2017 at 09:37, Wolfram Sang <w...@the-dreams.de> wrote:
> Hi Ulf,
>
> On Thu, Feb 16, 2017 at 08:57:36AM +0100, Ulf Hansson wrote:
>> On 15 February 2017 at 16:02, Wolfram Sang <w...@the-dreams.de> wrote:
>> >
>> >> > I see. Ulf, do you think it makes sense to extend the condition when to
>> >> > call mmc_blk_cmd_recovery() with checking if stop.resp[0] has one of the
>> >> > R1_* bits set which are marked with 'ex' (and probably 'erx', too)? I
>> >> > agree with Shimoda-san, that the core is a good place to do it, since it
>> >> > is about parsing the R1 and not the status bits of the host hardware.
>> >>
>> >> The method we use to indicate a stop command error to the mmc core, is
>> >> to set ->stop.error in the host driver before completing the request.
>> >> Perhaps set it to -EIO or -EILSEQ.
>> >>
>> >> In that way mmc_blk_err_check() sees the error and invokes the
>> >> mmc_blk_cmd_recovery() to deal with it (response parsing etc).
>> >>
>> >> Does that work for you?
>> >
>> > It would work, yes. Since R1 response format is hardware independent, I
>> > wondered if checking for ECC errors wouldn't be better suited in the
>> > core. We roughly need something like this:
>> >
>> > if (stop.resp[0] & R1_CARD_ECC_FAILED)
>> > stop.error = -EIO;
>> >
>> > We can copy this into every driver, of course. Yet, I wondered if we
>> > couldn't have a helper function mapping the R1 error bits to an
>> > apropriate error value and call that just before the check in
>> > mmc_blk_err_check().
>> >
>> > Do you get what I mean?
>>
>> I get it - and yes you have a point.
>
> Cool.
>
>> By looking at the code in mmc_blk_err_check() and
>> mmc_blk_cmd_recovery(), it deserves a clean-up. That said, I don't
>
> What do you mean with clean-up here? I would have just added the helper

...perhaps some re-factoring as the functions do lots of stuff.

> function checking R1 error bits and setting stop.error accordingly.

That's ok, I don't require you to do the clean up, but it would be nice. :-)

>
>> want to treat R1_CARD_ECC_FAILED as a special case.
>>
>> So if you decide to add this check in the core (which I am open to),
>> we should also add checks the other potential R1 errors, to be
>> consistent.
>
> I agree. That's what I meant with "checking if stop.resp[0] has one of
> the R1_* bits set which are marked with 'ex' (and probably 'erx',
> too)?". I think these are the candidates we care about.
>
> Thanks,
>
>Wolfram

Kind regards
Uffe

Re: [RFC] mmc: host: tmio: ensure end of DMA and SD access are in sync

[Ulf Hansson]

On 15 February 2017 at 19:05, Wolfram Sang
 wrote:
> The current code assumes that DMA is finished before SD access end is
> flagged. Thus, it schedules the 'dma_complete' tasklet in the SD card
> interrupt routine when DATAEND is set. The assumption is not safe,
> though. Even by mounting an SD card, it can be seen that sometimes DMA
> complete is first, sometimes DATAEND. It seems they are usually close
> enough timewise to not cause problems. However, a customer reported that
> with CMD53 sometimes things really break apart. As a result, the BSP has
> a patch which introduces flags for both events and makes sure both flags
> are set before scheduling the tasklet. The customer accepted the patch,
> yet it doesn't seem a proper upstream solution to me.
>
> This patch refactors the code to replace the tasklet with already
> existing and more lightweight mechanisms. First of all, we set the
> callback in a DMA descriptor to automatically get notified when DMA is
> done. In the callback, we then use a completion to make sure the SD
> access has already ended. Then, we proceed as before.

I have similar experience and a HW behaviour. Your reasoning seems
correct to me.

>
> Signed-off-by: Wolfram Sang 
> ---
>
> I tried various synchronization approaches and liked this one best.
>
> There are a couple of reasons this patch is RFC:
>
> The #ifdef's need to go but are handy for now.
>
> More testing is needed. While it worked fine for me writing terrabytes to
> different cards (still trying to break the wear-levelling), broader testing
> with different workloads and use-cases is largely welcome. I specifically
> couldn't test with CMD53 (SDIO) which originally caused the problem for the
> customer because that seems not working with my Gen2 board. I tried to fix 
> SDIO
> on Gen2 as a side-task but didn't find anything obvious on a glimpse. SDIO
> works with my Gen3 boards but for that we don't have DMA upstream yet.
> Upstreaming Gen3 DMA will need some bigger DMA refactoring, so it might be
> worth waiting until the refactoring is done. In a nutshell, there are
> dependency issues currently.
>
> So, calling for early review here. And opinions how to proceed. I am not sure
> we want this in renesas-drivers until the SDIO functionality has been 
> verified?
> Because it is the reason this patch exists in the first place :)

If there are no regressions, we could consider this as a nice
clean-up, instead of waiting for the dependencies to be resolved.

Did you run some performance test?

>
> The branch is here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
> renesas/topic/sdhi-dma-sync
>
> The test description is here:
>
> http://elinux.org/Tests:SDHI-DMA-Sync
>
> Cheers and thanks,
>
>Wolfram
>
>
>  drivers/mmc/host/tmio_mmc.h |  2 +-
>  drivers/mmc/host/tmio_mmc_dma.c | 71 
> +++--
>  drivers/mmc/host/tmio_mmc_pio.c |  4 +--
>  3 files changed, 50 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 2b349d48fb9a8a..891d400d2a7cf2 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -137,7 +137,7 @@ struct tmio_mmc_host {
> boolforce_pio;
> struct dma_chan *chan_rx;
> struct dma_chan *chan_tx;
> -   struct tasklet_struct   dma_complete;
> +   struct completion   dma_dataend;
> struct tasklet_struct   dma_issue;

The next step would be to convert the driver to a use threaded IRQ
handler in favour of the dma_issue tasklet. That should also work,
right!? :-)

> struct scatterlist  bounce_sg;
> u8  *bounce_buf;
> diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
> index fa8a936a3d9ba1..d2b6aed644f361 100644
> --- a/drivers/mmc/host/tmio_mmc_dma.c
> +++ b/drivers/mmc/host/tmio_mmc_dma.c
> @@ -43,6 +44,43 @@ void tmio_mmc_abort_dma(struct tmio_mmc_host *host)
> tmio_mmc_enable_dma(host, true);
>  }
>
> +static void tmio_mmc_dma_callback(void *arg)
> +{
> +   struct tmio_mmc_host *host = arg;
> +
> +#ifdef DEBUG
> +   u32 status = sd_ctrl_read16_and_16_as_32(host, CTL_STATUS);
> +
> +   dev_dbg(>pdev->dev, "DMA complete (0x%08x)\n", status);
> +#endif
> +
> +   wait_for_completion(>dma_dataend);
> +
> +#ifdef DEBUG
> +   status = sd_ctrl_read16_and_16_as_32(host, CTL_STATUS);
> +
> +   dev_dbg(>pdev->dev, "DATAEND complete (0x%08x)\n", status);
> +#endif
> +
> +   spin_lock_irq(>lock);
> +
> +   if (!host->data)
> +   goto out;
> +
> +   if (host->data->flags & MMC_DATA_READ)
> +   dma_unmap_sg(host->chan_rx->device->dev,
> +host->sg_ptr, host->sg_len,
> +DMA_FROM_DEVICE);
> +   else
> +   

Re: [PATCH v2 4/4] mmc: host: tmio: fill in response from auto cmd12

[Ulf Hansson]

On 15 February 2017 at 16:02, Wolfram Sang  wrote:
>
>> > I see. Ulf, do you think it makes sense to extend the condition when to
>> > call mmc_blk_cmd_recovery() with checking if stop.resp[0] has one of the
>> > R1_* bits set which are marked with 'ex' (and probably 'erx', too)? I
>> > agree with Shimoda-san, that the core is a good place to do it, since it
>> > is about parsing the R1 and not the status bits of the host hardware.
>>
>> The method we use to indicate a stop command error to the mmc core, is
>> to set ->stop.error in the host driver before completing the request.
>> Perhaps set it to -EIO or -EILSEQ.
>>
>> In that way mmc_blk_err_check() sees the error and invokes the
>> mmc_blk_cmd_recovery() to deal with it (response parsing etc).
>>
>> Does that work for you?
>
> It would work, yes. Since R1 response format is hardware independent, I
> wondered if checking for ECC errors wouldn't be better suited in the
> core. We roughly need something like this:
>
> if (stop.resp[0] & R1_CARD_ECC_FAILED)
> stop.error = -EIO;
>
> We can copy this into every driver, of course. Yet, I wondered if we
> couldn't have a helper function mapping the R1 error bits to an
> apropriate error value and call that just before the check in
> mmc_blk_err_check().
>
> Do you get what I mean?

I get it - and yes you have a point.

By looking at the code in mmc_blk_err_check() and
mmc_blk_cmd_recovery(), it deserves a clean-up. That said, I don't
want to treat R1_CARD_ECC_FAILED as a special case.

So if you decide to add this check in the core (which I am open to),
we should also add checks the other potential R1 errors, to be
consistent.

Kind regards
Uffe

Re: [PULL REQUEST] renesas/topic/sdhi-autocmd12-resp for renesas drivers

[Ulf Hansson]

On 13 February 2017 at 11:04, Geert Uytterhoeven  wrote:
> Hi Wolfram,
>
> On Sun, Feb 12, 2017 at 12:27 PM, Wolfram Sang  wrote:
>> here is a topic branch for renesas-drivers to report back autocmd12
>> responses for SDHI. It is based on mmc/next. Please pull.
>>
>> Kind regards,
>>
>>Wolfram

Hi Wolfram,

I don't get it, why did you send this pull request to Geert and not me?

I rather pick all mmc changes via my mmc tree, to avoid conflicts etc
- unless there are some specific reasons to not. Are there?

Kind regards
Uffe

>>
>>
>> The following changes since commit 0c3630150c9af658e7375c509c670fadf052cca8:
>>
>>   mmc: core: start to break apart mmc_start_areq() (2017-02-03 10:50:23 
>> +0100)
>>
>> are available in the git repository at:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
>> renesas/topic/sdhi-autocmd12-resp
>>
>> for you to fetch changes up to 22b13c28ac6b92f3acf7a5a91575b9da004c853a:
>>
>>   mmc: host: tmio: fill in response from auto cmd12 (2017-02-07 11:47:26 
>> +0100)
>
> Thank you, scheduled for inclusion in next renesas-drivers release
> (probably renesas-drivers-2017-02-21-v4.10).
>
> BTW, it still merges cleanly into today's linux-next, despite Ulf rebasing
> his mmc/next branch almost on a daily basis.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds

Re: [RESEND PATCH 2/2] PM / Domains: Fix asynchronous execution of *noirq() callbacks

[Ulf Hansson]

On 7 February 2017 at 14:23, Geert Uytterhoeven <ge...@linux-m68k.org> wrote:
> Hi Ulf, Rafael,
>
> On Thu, Jan 12, 2017 at 6:17 PM, Ulf Hansson <ulf.hans...@linaro.org> wrote:
>> As the PM core may invoke the *noirq() callbacks asynchronously, the
>> current lock-less approach in genpd doesn't work. The consequence is that
>> we may find concurrent operations racing to power on/off the PM domain.
>>
>> As of now, no immediate errors has been reported, but it's probably only a
>> matter time. Therefor let's fix the problem now before this becomes a real
>> issue, by deploying the locking scheme to the relevant functions.
>>
>> Reported-by: Brian Norris <briannor...@chromium.org>
>> Signed-off-by: Ulf Hansson <ulf.hans...@linaro.org>
>
> This is commit ef4f7e2c8335a56b in pm/linux-next.
>
>> ---
>>  drivers/base/power/domain.c | 62 
>> -
>>  1 file changed, 33 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index fd2e3e1..6b23d82 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -729,16 +729,17 @@ static bool genpd_dev_active_wakeup(struct 
>> generic_pm_domain *genpd,
>
>> @@ -1070,13 +1070,17 @@ static void genpd_syscore_switch(struct device *dev, 
>> bool suspend)
>> if (!pm_genpd_present(genpd))
>> return;
>>
>> +   genpd_lock(genpd);
>> +
>> if (suspend) {
>> genpd->suspended_count++;
>> -   genpd_sync_power_off(genpd);
>> +   genpd_sync_power_off(genpd, 0);
>> } else {
>> -   genpd_sync_power_on(genpd);
>> +   genpd_sync_power_on(genpd, 0);
>> genpd->suspended_count--;
>> }
>> +
>> +   genpd_unlock(genpd);
>>  }
>
> This causes the following BUG on all my Renesas arm32 boards, where the
> system timer is an IRQ safe device:
>
> BUG: sleeping function called from invalid context at 
> kernel/locking/mutex.c:232
> in_atomic(): 0, irqs_disabled(): 128, pid: 1751, name: s2ram
> CPU: 0 PID: 1751 Comm: s2ram Not tainted
> 4.10.0-rc7-koelsch-05643-g27f4c73972a614fe #3354
> Hardware name: Generic R8A7791 (Flattened Device Tree)
> [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
> [] (show_stack) from [] (dump_stack+0x7c/0x9c)
> [] (dump_stack) from [] (___might_sleep+0x124/0x160)
> [] (___might_sleep) from [] (mutex_lock+0x18/0x60)
> [] (mutex_lock) from [] (genpd_syscore_switch+0x2c/0x7c)
> [] (genpd_syscore_switch) from []
> (sh_cmt_clock_event_suspend+0x18/0x28)
> [] (sh_cmt_clock_event_suspend) from []
> (clockevents_suspend+0x40/0x54)
> [] (clockevents_suspend) from []
> (timekeeping_suspend+0x23c/0x278)
> [] (timekeeping_suspend) from []
> (syscore_suspend+0x88/0x138)
> [] (syscore_suspend) from []
> (suspend_devices_and_enter+0x290/0x470)
> [] (suspend_devices_and_enter) from []
> (pm_suspend+0x228/0x280)
> [] (pm_suspend) from [] (state_store+0xac/0xcc)
> [] (state_store) from [] (kernfs_fop_write+0x160/0x19c)
> [] (kernfs_fop_write) from [] (__vfs_write+0x20/0x108)
> [] (__vfs_write) from [] (vfs_write+0xb8/0x144)
> [] (vfs_write) from [] (SyS_write+0x40/0x80)
> [] (SyS_write) from [] (ret_fast_syscall+0x0/0x34)
>
> Reverting the commit fixes the issue.

Geert, thanks for reporting! Instead of a revert, I believe a better
option is to only partly revert the change.

What I suggest is to keep the locks for the *noirq() callbacks, but
remove it for the pm_genpd_syscore* functions.

Actually the change-log I wrote for the related commit, don't mention
the changes it introduces to the syscore API - my bad! I apologize for
the inconvenience.

Allow me to send a fix on top asap. Whether Rafael can fold it into
the existing commit or add on top, I leave to him to decide.

Kind regards
Uffe

Re: [PATCH v6 0/3] mmc: sh_mobile_sdhi: fix missing r7s72100 clocks

[Ulf Hansson]

On 26 January 2017 at 11:12, Wolfram Sang  wrote:
>
>> Thanks, applied patch1 and patch2 for next. Patch3 is for Simon, I
>> guess!? (And I can add Rob's ack afterwards).
>
> Can you add my tags as well. They got dropped somehow:
>
> Reviewed-by: Wolfram Sang 
>

Done, thanks!

Kind regards
Uffe

Re: [PATCH v5 2/3] mmc: sh_mobile_sdhi: explain clock bindings

[Ulf Hansson]

On 26 January 2017 at 15:39, Rob Herring  wrote:
> On Mon, Jan 23, 2017 at 11:56 AM, Chris Brandt  
> wrote:
>> Hello Rob,
>>
>>
>> On Monday, January 23, 2017, Rob Herring wrote:
>>> > --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
>>> > +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
>>> > @@ -25,8 +25,32 @@ Required properties:
>>> > "renesas,sdhi-r8a7795" - SDHI IP on R8A7795 SoC
>>> > "renesas,sdhi-r8a7796" - SDHI IP on R8A7796 SoC
>>> >
>>> > +- clocks: Most controllers only have 1 clock source per channel.
>>> However, on
>>> > + some variations of this controller, the internal card detection
>>>
>>> This should be explicit as to which compatible strings have 2 clocks and
>>> which have 1 clock.
>>
>> OK. I'll make a list like I did for sh_mmcif:
>>
>> https://patchwork.kernel.org/patch/9512091/
>>
>>
>>
>>> > +
>>> > +Example showing 2 clocks:
>>> > +   sdhi0: sd@e804e000 {
>>>
>>> mmc@...
>>
>> I'm confused. I see that for all SDHI controllers, it either "sd@" or 
>> "sdhci@".
>>
>> $ grep sdhi $(find arch/arm/boot/dts -name "*.dtsi")
>>
>> $ grep sdhci $(find arch/arm/boot/dts -name "*.dtsi")
>
> Yes, there's lots of variation. Node names are supposed to be generic
> for their class of device (e.g. ethernet, pci, usb,
> interrupt-controller, etc.). I'd be fine with "sd", but think "mmc" is
> more common. Either way, we should pick one moving forward. "sdhci"
> should not be used as that's a specific implementation.

I think we discussed this earlier. The problem with these devices is
that it covers several type of devices, so it's hard to find a generic
node name.

The devices are: MMC, eMMC, SD, SDIO. I don't have strong opinions on
whether to chose a generic node name, and perhaps "mmc" is the best we
can get?

Whatever we decide, we should update all mmc DTS docs, to avoid future
confusion.

Kind regards
Uffe

Re: [PATCH v3 2/3] mmc: sh_mobile_sdhi: explain clock bindings

[Ulf Hansson]

On 20 January 2017 at 17:05, Chris Brandt <chris.bra...@renesas.com> wrote:
> Hello Ulf,
>
> Friday, January 20, 2017, Ulf Hansson wrote:
>> > +- clocks: Most controllers only have 1 clock source per channel.
>> However, some
>> > + have a second clock dedicated to card detection. If 2 clocks
>> are
>> > + specified, you must name them as "core" and "cd". If the
>> controller
>> > + only has 1 clock, naming is not required.
>>
>> Could you please elaborate a bit on the card detection clock?
>>
>> I guess that there is some kind of internal card detection logic (native
>> card detect) in the SDHI IP, which requires a separate clock for it to
>> work? Perhaps you can state that somehow?
>
>
> The reality is that the chip guys cut up the standard SDHI IP to add a
> 'cool new feature', but all I want to do is put it back the way it was.
>
> NOTE: The design guys like to reuse IP blocks from previous designs that they
> know worked and didn't have bugs. So, there is a good chance you will see this
> change in future RZ/A devices (or even other future Renesas SoC devices).
> To remove an unwanted feature adds additional design risk of breaking
> something elseand given the cost of redoing silicon mask layers...no
> engineer wants that mistake on their hands.

So, one should be aware of that runtime PM support in these case is
going to be suboptimal.
For example, when using this native card detect, you will need to keep
the controller runtime PM resumed as the be able to keep the clock on
and to be able to fetch the irq. Wasting power.

Most SoC vendors are therefore using a GPIO card detect instead,
although I assume you already knew that. :-)

[...]

Kind regards
Uffe

Re: [PATCH v3 2/3] mmc: sh_mobile_sdhi: explain clock bindings

[Ulf Hansson]

On 18 January 2017 at 18:25, Chris Brandt  wrote:
> In the case of a single clock source, you don't need names. However,
> if the controller has 2 clock sources, you need to name them correctly
> so the driver can find the 2nd one.
>
> Signed-off-by: Chris Brandt 
> ---
> v2:
> * fix spelling and change wording
> * changed clock name from "carddetect" to "cd"
> ---
>  Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 21 +
>  1 file changed, 21 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt 
> b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> index a1650ed..90370cd 100644
> --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> @@ -25,8 +25,29 @@ Required properties:
> "renesas,sdhi-r8a7795" - SDHI IP on R8A7795 SoC
> "renesas,sdhi-r8a7796" - SDHI IP on R8A7796 SoC
>
> +- clocks: Most controllers only have 1 clock source per channel. However, 
> some
> + have a second clock dedicated to card detection. If 2 clocks are
> + specified, you must name them as "core" and "cd". If the controller
> + only has 1 clock, naming is not required.

Could you please elaborate a bit on the card detection clock?

I guess that there is some kind of internal card detection logic
(native card detect) in the SDHI IP, which requires a separate clock
for it to work? Perhaps you can state that somehow?

> +
>  Optional properties:
>  - toshiba,mmc-wrprotect-disable: write-protect detection is unavailable
>  - pinctrl-names: should be "default", "state_uhs"
>  - pinctrl-0: should contain default/high speed pin ctrl
>  - pinctrl-1: should contain uhs mode pin ctrl
> +
> +Example showing 2 clocks:
> +   sdhi0: sd@e804e000 {
> +   compatible = "renesas,sdhi-r7s72100";
> +   reg = <0xe804e000 0x100>;
> +   interrupts =  + GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH
> + GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
> +
> +   clocks = <_clks R7S72100_CLK_SDHI00>,
> +<_clks R7S72100_CLK_SDHI01>;
> +   clock-names = "core", "cd";
> +   cap-sd-highspeed;
> +   cap-sdio-irq;
> +   status = "disabled";

The last line seems a bit odd to include in an example.

> +   };
> --
> 2.10.1
>
>

Kind regards
Uffe

Re: [PATCH v3 1/3] mmc: sh_mobile_sdhi: add support for 2 clocks

[Ulf Hansson]

On 18 January 2017 at 18:25, Chris Brandt  wrote:
> Some controllers have 2 clock sources instead of 1, so they both need
> to be turned on/off.

This doesn't tell me enough. Please elaborate.

For example, tell how you treat the clocks, which of them that is
optional and why.

>
> Signed-off-by: Chris Brandt 
> ---
> v2:
> * changed clk2 to clk_cd
> * disable clk if clk_cd enable fails
> * changed clock name from "carddetect" to "cd"
> ---
>  drivers/mmc/host/sh_mobile_sdhi.c | 13 +
>  1 file changed, 13 insertions(+)
>
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c 
> b/drivers/mmc/host/sh_mobile_sdhi.c
> index 59db14b..a3f995e 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -143,6 +143,7 @@ MODULE_DEVICE_TABLE(of, sh_mobile_sdhi_of_match);
>
>  struct sh_mobile_sdhi {
> struct clk *clk;
> +   struct clk *clk_cd;
> struct tmio_mmc_data mmc_data;
> struct tmio_mmc_dma dma_priv;
> struct pinctrl *pinctrl;
> @@ -190,6 +191,12 @@ static int sh_mobile_sdhi_clk_enable(struct 
> tmio_mmc_host *host)
> if (ret < 0)
> return ret;
>
> +   ret = clk_prepare_enable(priv->clk_cd);
> +   if (ret < 0) {
> +   clk_disable_unprepare(priv->clk);
> +   return ret;
> +   }
> +
> /*
>  * The clock driver may not know what maximum frequency
>  * actually works, so it should be set with the max-frequency
> @@ -255,6 +262,8 @@ static void sh_mobile_sdhi_clk_disable(struct 
> tmio_mmc_host *host)
> struct sh_mobile_sdhi *priv = host_to_priv(host);
>
> clk_disable_unprepare(priv->clk);
> +   if (priv->clk_cd)
> +   clk_disable_unprepare(priv->clk_cd);
>  }
>
>  static int sh_mobile_sdhi_card_busy(struct mmc_host *mmc)
> @@ -572,6 +581,10 @@ static int sh_mobile_sdhi_probe(struct platform_device 
> *pdev)
> goto eprobe;
> }
>
> +   priv->clk_cd = devm_clk_get(>dev, "cd");
> +   if (IS_ERR(priv->clk_cd))
> +   priv->clk_cd = NULL;

Is this clock solely about card detection? So in cases when you have a
GPIO card detect, the clock isn't needed?

Just trying to understand things a bit better...

> +
> priv->pinctrl = devm_pinctrl_get(>dev);
> if (!IS_ERR(priv->pinctrl)) {
> priv->pins_default = pinctrl_lookup_state(priv->pinctrl,
> --
> 2.10.1
>
>

Kind regards
Uffe

Re: [PATCH] mmc: host: tmio: drop superfluous exit path

[Ulf Hansson]

On 6 January 2017 at 09:38, Wolfram Sang
 wrote:
> The probe exit path on error does nothing since commit 94b110aff8679b
> ("mmc: tmio: add tmio_mmc_host_alloc/free()"), so we can bail out
> immediately.
>
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/tmio_mmc_pio.c | 20 ++--
>  1 file changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index d0d1743b1c002b..2e2df63ce92658 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -1151,7 +1151,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
>
> ret = mmc_of_parse(mmc);
> if (ret < 0)
> -   goto host_free;
> +   return ret;
>
> _host->pdata = pdata;
> platform_set_drvdata(pdev, mmc);
> @@ -1161,14 +1161,12 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
>
> ret = tmio_mmc_init_ocr(_host);
> if (ret < 0)
> -   goto host_free;
> +   return ret;
>
> _host->ctl = devm_ioremap(>dev,
>   res_ctl->start, resource_size(res_ctl));
> -   if (!_host->ctl) {
> -   ret = -ENOMEM;
> -   goto host_free;
> -   }
> +   if (!_host->ctl)
> +   return -ENOMEM;
>
> tmio_mmc_ops.card_busy = _host->card_busy;
> tmio_mmc_ops.start_signal_voltage_switch = 
> _host->start_signal_voltage_switch;
> @@ -1206,10 +1204,8 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
>  * Check the sanity of mmc->f_min to prevent tmio_mmc_set_clock() from
>  * looping forever...
>  */
> -   if (mmc->f_min == 0) {
> -   ret = -EINVAL;
> -   goto host_free;
> -   }
> +   if (mmc->f_min == 0)
> +   return -EINVAL;
>
> /*
>  * While using internal tmio hardware logic for card detection, we 
> need
> @@ -1274,10 +1270,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
> }
>
> return 0;
> -
> -host_free:
> -
> -   return ret;
>  }
>  EXPORT_SYMBOL(tmio_mmc_host_probe);
>
> --
> 2.11.0
>
> --
> 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: tmio: use SDIO master interrupt bit only when allowed

[Ulf Hansson]

On 9 December 2016 at 17:51, Wolfram Sang
 wrote:
> The master bit to enable SDIO interrupts can only be accessed if
> SCLKDIVEN bit allows that. However, the core uses the SDIO enable
> callback at times when SCLKDIVEN forbids the change. This leads to
> "timeout waiting for SD bus idle" messages.
>
> We now activate the master bit in probe once if SDIO is supported. IRQ
> en-/disabling will be done now by the individual IRQ enablement bits
> only.
>
> Signed-off-by: Wolfram Sang 
> Reviewed-by: Yasushi SHOJI 

Thanks, applied for next!

Kind regards
Uffe

> ---
>
> No change from RFC, only Rev-by added (which included testing).
>
>  drivers/mmc/host/tmio_mmc_pio.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index 7ef24ec620b542..526e52719f81b9 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -140,12 +140,10 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host 
> *mmc, int enable)
>
> host->sdio_irq_mask = TMIO_SDIO_MASK_ALL &
> ~TMIO_SDIO_STAT_IOIRQ;
> -   sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001);
> sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
> } else if (!enable && host->sdio_irq_enabled) {
> host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
> sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
> -   sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x);
>
> host->sdio_irq_enabled = false;
> pm_runtime_mark_last_busy(mmc_dev(mmc));
> @@ -1203,7 +1201,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
> if (pdata->flags & TMIO_MMC_SDIO_IRQ) {
> _host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
> sd_ctrl_write16(_host, CTL_SDIO_IRQ_MASK, 
> _host->sdio_irq_mask);
> -   sd_ctrl_write16(_host, CTL_TRANSACTION_CTL, 0x);
> +   sd_ctrl_write16(_host, CTL_TRANSACTION_CTL, 0x0001);
> }
>
> spin_lock_init(&_host->lock);
> @@ -1251,6 +1249,9 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host)
> struct platform_device *pdev = host->pdev;
> struct mmc_host *mmc = host->mmc;
>
> +   if (host->pdata->flags & TMIO_MMC_SDIO_IRQ)
> +   sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x);
> +
> if (!host->native_hotplug)
> pm_runtime_get_sync(>dev);
>
> --
> 2.10.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 0/9] mmc: sh_mobile_sdhi: add HS200 support

[Ulf Hansson]

On 12 December 2016 at 20:51, Wolfram Sang
 wrote:
> Here is a series adding HS200 support to the SDHI driver. Building the actual
> functionality on top of SDR104 support was rather easy. However, I figured the
> checks for enabling tuning were rather scattered over the driver and could be
> further improved. So I did some refactoring of that to make adding HS200
> support then a one-liner :)
>
> This has been tested on Salvator-X boards with R-Car H3 and M3-W SoCs. It has
> also been tested on a Lager board (R-Car H2) checking SDR50/104 for
> regressions. The whole test document showing significant speed-ups reading 
> from
> the on-board eMMC can be found here:
>
> http://elinux.org/Tests:eMMC-HS
>
> and the branch can be found here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
> renesas/topic/sdhi-emmc-hs
>
> Please comment, review, apply...
>
> Thanks,
>
>Wolfram
>
>
> Wolfram Sang (9):
>   mmc: sh_mobile_sdhi: simplify accessing DT data
>   mmc: sh_mobile_sdhi: improve prerequisite for hw_reset
>   mmc: sh_mobile_sdhi: improve prerequisites for tuning
>   mmc: sh_mobile_sdhi: remove superfluous check in hw_reset
>   mmc: sh_mobile_sdhi: remove superfluous check in init_tuning
>   mmc: sh_mobile_sdhi: remove superfluous check in SCC error check
>   mmc: sh_mobile_sdhi: enable HS200
>   arm64: dts: r8a7795: enable HS200 for eMMC
>   arm64: dts: r8a7796: enable HS200 for eMMC
>
>  arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts |  1 +
>  arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts |  1 +
>  drivers/mmc/host/sh_mobile_sdhi.c  | 67 
> --
>  3 files changed, 27 insertions(+), 42 deletions(-)
>

Thanks, applied patch 1 -> 7 for next. DTS changes in patch 8 and 9
are for Simon to pick up.

Kind regards
Uffe

> --
> 2.10.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/next] mmc: sh_mobile_sdhi: remove support for sh7372

[Ulf Hansson]

On 24 November 2016 at 11:48, Simon Horman  wrote:
> Remove documentation of support for the SH7372 (SH-Mobile AP4) from the MMC
> driver. The driver itself appears to have no SH7372 specific code.
>
> Commit edf4100906044225 ("ARM: shmobile: sh7372 dtsi: Remove Legacy file")
> removes this SoC from the kernel in v4.1.
>
> Signed-off-by: Simon Horman 

Thanks, applied for next!

Kind regards
Uffe

> ---
>  Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt 
> b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> index 13df9c2399c3..1db9e74bb9c1 100644
> --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> @@ -11,7 +11,6 @@ optional bindings can be used.
>
>  Required properties:
>  - compatible:  "renesas,sdhi-shmobile" - a generic sh-mobile SDHI unit
> -   "renesas,sdhi-sh7372" - SDHI IP on SH7372 SoC
> "renesas,sdhi-sh73a0" - SDHI IP on SH73A0 SoC
> "renesas,sdhi-r8a73a4" - SDHI IP on R8A73A4 SoC
> "renesas,sdhi-r8a7740" - SDHI IP on R8A7740 SoC
> --
> 2.7.0.rc3.207.g0ac5344
>

Re: [PATCH mmc/next] mmc: sh_mmcif: Document r8a73a4, r8a7778 and sh73a0 DT bindings

[Ulf Hansson]

On 25 November 2016 at 08:56, Simon Horman  wrote:
> Simply document new compatibility strings as the driver is already
> activated using a fallback compatibility string.
>
> These compat strings are in keeping with those for all other
> Renesas ARM based SoCs with sh_mmcif enabled in mainline.
>
> Signed-off-by: Simon Horman 

Thanks, applied for next!

Kind regards
Uffe

> ---
> Reposted with r8a7778 instead of r8a7779 in subject
>
> I have also posted patches to use these new compat strings
> to bring the DT files of the SoCs in question in-line with those
> for other Renesas ARM based SoCs with sh_mmcif enabled in mainline.
> ---
>  Documentation/devicetree/bindings/mmc/renesas,mmcif.txt | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt 
> b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> index ff611fa66871..e4ba92aa035e 100644
> --- a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> +++ b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> @@ -8,11 +8,14 @@ Required properties:
>
>  - compatible: should be "renesas,mmcif-", "renesas,sh-mmcif" as a
>fallback. Examples with  are:
> +   - "renesas,mmcif-r8a73a4" for the MMCIF found in r8a73a4 SoCs
> - "renesas,mmcif-r8a7740" for the MMCIF found in r8a7740 SoCs
> +   - "renesas,mmcif-r8a7778" for the MMCIF found in r8a7778 SoCs
> - "renesas,mmcif-r8a7790" for the MMCIF found in r8a7790 SoCs
> - "renesas,mmcif-r8a7791" for the MMCIF found in r8a7791 SoCs
> - "renesas,mmcif-r8a7793" for the MMCIF found in r8a7793 SoCs
> - "renesas,mmcif-r8a7794" for the MMCIF found in r8a7794 SoCs
> +   - "renesas,mmcif-sh73a0" for the MMCIF found in sh73a0 SoCs
>
>  - clocks: reference to the functional clock
>
> --
> 2.7.0.rc3.207.g0ac5344
>

Re: [PATCH 1/2] mmc: tmio: fix wrong bitmask for SDIO irqs

[Ulf Hansson]

On 13 November 2016 at 15:29, Wolfram Sang
 wrote:
> Commit 7729c7a232a953 ("mmc: tmio: Provide separate interrupt handlers")
> refactored the sdio irq handler and wrongly used the mask for SD irqs,
> not for SDIO irqs. This doesn't really matter in practice because both
> values keep the only interrupt we are interested in. But still, this is
> wrong and wants to be fixed.
>
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/tmio_mmc_pio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index dbc3cb14f3321b..2b04e6e87192c7 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -741,7 +741,7 @@ static void tmio_mmc_sdio_irq(int irq, void *devid)
> return;
>
> status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
> -   ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
> +   ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdio_irq_mask;
>
> sdio_status = status & ~TMIO_SDIO_MASK_ALL;
> if (pdata->flags & TMIO_MMC_SDIO_STATUS_QUIRK)
> --
> 2.9.3
>
> --
> 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 v8 0/6] UHS-I SDR-104 support for sh_mobile_sdhi

[Ulf Hansson]

On 10 November 2016 at 12:45, Wolfram Sang  wrote:
> On Thu, Nov 03, 2016 at 03:15:58PM +0100, Simon Horman wrote:
>> Hi,
>>
>> this series is based on work by Ai Kyuse to add UHS-I SDR-104 support for
>> sh_mobile_sdhi. It builds on work by Shinobu Uehara, Rob Taylor, William
>> Towle and Ian Molton, Ben Hutchings, Wolfram Sang and others to add UHS-I
>> SDR-50 support to the same driver.
>>
>> It is based on the next branch of the mmc tree.
>>
>> To aid review the following git branch is provided:
>> * https:://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git 
>> topic/sdr104-driver-v8
>>
>> Overview of changes since v7:
>> * Correct inverted logic in sh_mobile_sdhi_hw_reset
>> * Correct value of SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN
>>
>> Please see http://elinux.org/Tests:SD-SDHI-SDR104 for indicative tests
>> results.
>
> Successfully tested on a Lager Board with a SanDisk card and a Samsung
> card which used to cause issues before.
>
> Tested-by: Wolfram Sang 
>
> Ulf, I think this series is ready to go for v4.10. All issues we know of
> are related to pinmuxing and will be solved elsewhere. We currently have
> hacks for that so it is quite clear the issues are not caused by this
> series.
>

Thanks, applied for next!

I did have to fix conflict and also to resolve some checkpatch
warnings - but no worries this time!

Kind regards
Uffe

Re: [PATCH v3 0/3] mmc: sh_mobile_sdhi: Add r7s72100 support

[Ulf Hansson]

On 12 September 2016 at 16:15, Chris Brandt  wrote:
> For the most part, the SDHI controller in the RZ/A1 (r7s72100)
> is the same as other Renesas SoC...except for the fact that the
> data port register was widened to 32-bits and the 16-bit accesses
> in the current tmio driver aren't going to cut it.
>
> Also, the tmio driver allows you to pass in what voltages you
> support via the ocr_mask. For the RZ/A1, it's just simple 3.3v.
> So, I added the ability to pass that when using DT.
>
> Chris Brandt (3):
>   mmc: sh_mobile_sdhi: add ocr_mask option
>   mmc: tmio-mmc: add support for 32bit data port
>   mmc: sh_mobile_sdhi: Add r7s72100 support
>
>  Documentation/devicetree/bindings/mmc/tmio_mmc.txt |  1 +
>  drivers/mmc/host/sh_mobile_sdhi.c  |  9 +++
>  drivers/mmc/host/tmio_mmc.h| 12 +
>  drivers/mmc/host/tmio_mmc_pio.c| 30 
> ++
>  include/linux/mfd/tmio.h   |  5 
>  5 files changed, 57 insertions(+)
>
> --
> 2.9.2
>
>

Thanks, applied for next!

Kind regards
Uffe

Re: [PATCH v3 2/3] mmc: tmio-mmc: add support for 32bit data port

[Ulf Hansson]

On 14 October 2016 at 15:18, Chris Brandt <chris.bra...@renesas.com> wrote:
> On 9/22/2016, Ulf Hansson wrote:
>> To: Chris Brandt <chris.bra...@renesas.com>
>> Cc: Wolfram Sang <wsa+rene...@sang-engineering.com>; Sergei Shtylyov
>> <sergei.shtyl...@cogentembedded.com>; Geert Uytterhoeven <geert@linux-
>> m68k.org>; Simon Horman <horms+rene...@verge.net.au>; linux-mmc > m...@vger.kernel.org>; Linux-Renesas <linux-renesas-soc@vger.kernel.org>;
>> Lee Jones <l...@kernel.org>
>> Subject: Re: [PATCH v3 2/3] mmc: tmio-mmc: add support for 32bit data port
>>
>> + Lee
>>
>> On 12 September 2016 at 16:15, Chris Brandt <chris.bra...@renesas.com>
>> wrote:
>> > For the r7s72100 SOC, the DATA_PORT register was changed to 32-bits wide.
>> > Therefore a new flag has been created that will allow 32-bit
>> > reads/writes to the DATA_PORT register instead of 16-bit (because
>> > 16-bits accesses are not supported).
>> >
>> > Signed-off-by: Chris Brandt <chris.bra...@renesas.com>
>> > ---
>> > v3:
>> > * changed loops to memcpy
>> > v2:
>> > * changed 'data * 0xFF' to 'data & 0xFF'
>> > * added 'const' for sd_ctrl_write32_rep
>> > ---
>> >  drivers/mmc/host/tmio_mmc.h | 12 
>> >  drivers/mmc/host/tmio_mmc_pio.c | 30 ++
>> >  include/linux/mfd/tmio.h|  5 +
>>
>> This header file needs to be split up. The mmc specific bits, should be
>> moved to a local header file under driver/mmc/host/*.
>>
>> Sure, we don't need to do that as part of $subject patch, but then I need
>> an ack from Lee Jones, the mfd maintainer. I have added him on cc.
>
>
> Is this still waiting on an ACK from Lee Jones because it touches the tmio.h 
> file under include/linux/mfd/ ?
>

Yes, but more important is Wolfram's ack as he maintains
drivers/mmc/host/tmio_mmc*

Kind regards
Uffe

Re: [PATCH v3 0/6] tmio: add support for eMMC with 8 bit bus width

[Ulf Hansson]

On 19 September 2016 at 22:57, Wolfram Sang
 wrote:
> This series enables SDHI instances on R-Car Gen3 to access eMMC with 8 bit bus
> width. I think the patch descriptions speak for themselves.
>
> I decided to not protect this new feature with a flag because it needs
> specifically to be enabled by setting the bus width to 8. No legacy platform
> does that.
>
> Note that I decided to use the pattern that pinctrl-0 is 3.3v and
> pinctrl-1 is 1.8v, although the eMMC is fixed at 1.8v. I tried a few ways to
> only use pinctrl-0 being 1.8v here, but they all ended up to be confusing for
> users IMO, so I sticked to the most consistent solution after all.
>
> Changes since V2:
>
> * correct node name for sd2_uhs in both devicetrees
>
> Changes since V1:
>
> * merged the two distinct series (drivers + DTS) into one: patches 1-4 are for
>   Ulf, I will ping Simon for patches 5+6 when the time is ready.
>
> * DTS now contains 'non-removable' and we have a software workaround for now.
>
> These patches are based on top of Simon's sdr104-v7 patches but they apply to
> current mmc/next as well. A branch can be found here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
> renesas/topic/sdhi-8bit-emmc
>
> Please review, comment, apply...
>
>Wolfram
>
> Wolfram Sang (6):
>   mmc: add define for R1 response without CRC
>   mmc: rtsx_pci: use new macro for R1 without CRC
>   mmc: rtsx_usb: use new macro for R1 without CRC
>   mmc: tmio: add eMMC support
>   arm64: dts: r8a7795: salvator: enable on-board eMMC
>   arm64: dts: r8a7796: salvator: enable on board eMMC
>
>  arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts | 43 +
>  arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 44 
> +-
>  drivers/mmc/host/rtsx_pci_sdmmc.c  |  2 +-
>  drivers/mmc/host/rtsx_usb_sdmmc.c  |  2 +-
>  drivers/mmc/host/tmio_mmc.h|  3 ++
>  drivers/mmc/host/tmio_mmc_pio.c| 38 +--
>  include/linux/mmc/core.h   |  3 ++
>  7 files changed, 120 insertions(+), 15 deletions(-)
>

Thanks, applied patch 1->4 for next. I assume Simon will pick up the
DT changes, sooner or later.

Kind regards
Uffe

Re: [PATCH v3 2/3] mmc: tmio-mmc: add support for 32bit data port

[Ulf Hansson]

+ Lee

On 12 September 2016 at 16:15, Chris Brandt  wrote:
> For the r7s72100 SOC, the DATA_PORT register was changed to 32-bits wide.
> Therefore a new flag has been created that will allow 32-bit reads/writes
> to the DATA_PORT register instead of 16-bit (because 16-bits accesses are
> not supported).
>
> Signed-off-by: Chris Brandt 
> ---
> v3:
> * changed loops to memcpy
> v2:
> * changed 'data * 0xFF' to 'data & 0xFF'
> * added 'const' for sd_ctrl_write32_rep
> ---
>  drivers/mmc/host/tmio_mmc.h | 12 
>  drivers/mmc/host/tmio_mmc_pio.c | 30 ++
>  include/linux/mfd/tmio.h|  5 +

This header file needs to be split up. The mmc specific bits, should
be moved to a local header file under driver/mmc/host/*.

Sure, we don't need to do that as part of $subject patch, but then I
need an ack from Lee Jones, the mfd maintainer. I have added him on
cc.

>  3 files changed, 47 insertions(+)
>
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index ecb99fc..a99e634 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -237,6 +237,12 @@ static inline u32 sd_ctrl_read16_and_16_as_32(struct 
> tmio_mmc_host *host, int ad
>readw(host->ctl + ((addr + 2) << host->bus_shift)) << 16;
>  }
>
> +static inline void sd_ctrl_read32_rep(struct tmio_mmc_host *host, int addr,
> +   u32 *buf, int count)
> +{
> +   readsl(host->ctl + (addr << host->bus_shift), buf, count);
> +}
> +
>  static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr, u16 
> val)
>  {
> /* If there is a hook and it returns non-zero then there
> @@ -259,4 +265,10 @@ static inline void sd_ctrl_write32_as_16_and_16(struct 
> tmio_mmc_host *host, int
> writew(val >> 16, host->ctl + ((addr + 2) << host->bus_shift));
>  }
>
> +static inline void sd_ctrl_write32_rep(struct tmio_mmc_host *host, int addr,
> +   const u32 *buf, int count)
> +{
> +   writesl(host->ctl + (addr << host->bus_shift), buf, count);
> +}
> +
>  #endif
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index 017a4dc..59fac7d 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -443,6 +443,36 @@ static void tmio_mmc_transfer_data(struct tmio_mmc_host 
> *host,
> /*
>  * Transfer the data
>  */
> +   if (host->pdata->flags & TMIO_MMC_32BIT_DATA_PORT) {
> +   u8 data[4] = { };
> +
> +   if (is_read)
> +   sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT, (u32 *)buf,
> +  count >> 2);
> +   else
> +   sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT, (u32 
> *)buf,
> +   count >> 2);
> +
> +   /* if count was multiple of 4 */
> +   if (!(count & 0x3))
> +   return;
> +
> +   buf8 = (u8 *)(buf + (count >> 2));
> +   count %= 4;
> +
> +   if (is_read) {
> +   sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT,
> +  (u32 *)data, 1);
> +   memcpy(buf8, data, count);
> +   } else {
> +   memcpy(data, buf8, count);
> +   sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT,
> +   (u32 *)data, 1);
> +   }
> +
> +   return;
> +   }
> +
> if (is_read)
> sd_ctrl_read16_rep(host, CTL_SD_DATA_PORT, buf, count >> 1);
> else
> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
> index 3b95dc7..0dbcb7e 100644
> --- a/include/linux/mfd/tmio.h
> +++ b/include/linux/mfd/tmio.h
> @@ -100,6 +100,11 @@
>  #define TMIO_MMC_SDIO_STATUS_QUIRK (1 << 8)
>
>  /*
> + * Some controllers have a 32-bit wide data port register
> + */
> +#define TMIO_MMC_32BIT_DATA_PORT   (1 << 9)
> +
> +/*
>   * Some controllers allows to set SDx actual clock
>   */
>  #define TMIO_MMC_CLK_ACTUAL(1 << 10)
> --
> 2.9.2
>
>

Kind regards
Uffe

Re: [PATCH v3 1/3] mmc: sh_mobile_sdhi: add ocr_mask option

[Ulf Hansson]

On 13 September 2016 at 17:59, Chris Brandt <chris.bra...@renesas.com> wrote:
> Hello Uffe
>
> Thank you for your reply.
>
> On 9/13/2016, Ulf Hansson wrote:
>> Because, if you have an external regulator feeding the mmc with power you
>> should set it up and use it.
>
> But in many board designs, there is no control over the regulator supply (or 
> you would rather have your bootloader do that, not the kernel).
>
> So what this patch is doing is setting it to a default of 3.3v (for just this 
> SoC) and if someone wants to add a regulator in the board's DT file, that 
> will override the ocr_mask set by the driver.

That's perfectly okay to me! I just wanted to get clear picture of
*why* this change was needed.

>
>
> I'm assuming I'll have to do the same for MMC (sh_mmcif.c) once I get to that 
> driver (I need to find my MMC PLUS card first for testing).

Okay!

Kind regards
Uffe

Re: [PATCH v3 1/3] mmc: sh_mobile_sdhi: add ocr_mask option

[Ulf Hansson]

On 13 September 2016 at 15:50, Chris Brandt  wrote:
> Here's a question:
>
> The DT regulator method is good if you want to be able to control the 
> regulator at run-time by the system.
>
> But for MMC and SDHI, why isn't there a way to just set the OCR voltage in 
> the board's DT if it's fixed (other than making a fixed regulator node)?
> Why isn't there a mmc-ocr-mask property? That's really what I want and it 
> seems like it would make a lot of dts files more simple.

Because, if you have an external regulator feeding the mmc with power
you should set it up and use it.

Now, we do have cases where it's actually the MMC controller that
manges the power to the card. So no external regulators being used.
For these case we have the "voltage-ranges" DT binding, but I don't
think you should be using that for these cases. :-)

Kind regards
Uffe

Re: [PATCH v3 1/3] mmc: sh_mobile_sdhi: add ocr_mask option

[Ulf Hansson]

On 12 September 2016 at 16:15, Chris Brandt  wrote:
> In moving platforms from board files to DT, there still needs to be a way
> to set the ocr_mask setting for the tmio driver during probe. Without this
> setting, the probe will fail because the supported voltages are not known.

Regarding the ocr_mask; How do these SoCs provides the power to the mmc/sd card?

Do note, I am *not* talking about the I/O voltage but the core power
to the card.

The reason for raising the question is that we have infrastructures in
the mmc core which can create the ocr_mask, by parsing a regulator's
voltage range. This is the recommended method to use, instead of using
hard coded ocr mask values.

Kind regards
Uffe

>
> This patch will also traditional platform registration platforms to
> migrate to DT.
>
> Signed-off-by: Chris Brandt 
> ---
>  drivers/mmc/host/sh_mobile_sdhi.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c 
> b/drivers/mmc/host/sh_mobile_sdhi.c
> index 5334f24..b033500 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -59,6 +59,7 @@ enum tmio_mmc_dmac_type {
>
>  struct sh_mobile_sdhi_of_data {
> unsigned long tmio_flags;
> +   u32   tmio_ocr_mask;
> unsigned long capabilities;
> unsigned long capabilities2;
> enum dma_slave_buswidth dma_buswidth;
> @@ -630,6 +631,7 @@ static int sh_mobile_sdhi_probe(struct platform_device 
> *pdev)
> const struct sh_mobile_sdhi_of_data *of_data = of_id->data;
>
> mmc_data->flags |= of_data->tmio_flags;
> +   mmc_data->ocr_mask = of_data->tmio_ocr_mask;
> mmc_data->capabilities |= of_data->capabilities;
> mmc_data->capabilities2 |= of_data->capabilities2;
> mmc_data->dma_rx_offset = of_data->dma_rx_offset;
> --
> 2.9.2
>
>

Re: [PATCH v7 0/6] UHS-I SDR-104 support for sh_mobile_sdhi

[Ulf Hansson]

On 13 September 2016 at 12:56, Simon Horman  wrote:
>
> Hi,
>
> this series is based on work by Ai Kyuse to add UHS-I SDR-104 support for
> sh_mobile_sdhi. It builds on work by Shinobu Uehara, Rob Taylor, William
> Towle and Ian Molton, Ben Hutchings, Wolfram Sang and others to add UHS-I
> SDR-50 support to the same driver.
>
> It is based on a merge of the next branches of the mmc tree.
>
> To aid review the following git branch is provided:
> * https:://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git 
> topic/sdr104-driver-v7
>
> Overview of changes since v6:
> * Address review by Ulf
>   - Detailed in per-patch changelogs
> * Move integration (arm/arm64 dt) patches to separate patchset
>
> Please see http://elinux.org/Tests:SD-SDHI-SDR104 for indicative tests
> results.
>
>
> Ai Kyuse (3):
>   mmc: tmio: enhance illegal sequence handling
>   mmc: tmio: Add hw reset support
>   mmc: tmio: Add tuning support
>
> Simon Horman (3):
>   mmc: core: Add helper to see if a host can be retuned
>   mmc: tmio: document mandatory and optional callbacks
>   mmc: sh_mobile_sdhi: Add tuning support
>
>  drivers/mmc/host/sh_mobile_sdhi.c | 265 
> +-
>  drivers/mmc/host/tmio_mmc.h   |  18 ++-
>  drivers/mmc/host/tmio_mmc_pio.c   |  87 -
>  include/linux/mmc/host.h  |   5 +
>  4 files changed, 367 insertions(+), 8 deletions(-)
>
> --
> 2.7.0.rc3.207.g0ac5344
>

This looks good to me, although I would like to get an ack from
Wolfram before I queue this up.

Kind regards
Uffe

Re: [PATCH] mmc: sh_mobile_sdhi: Add r8a7796 support

[Ulf Hansson]

On 6 September 2016 at 12:38, Simon Horman  wrote:
> From: Ai Kyuse 
>
> Add support for r8a7796 SoC.
>
> Signed-off-by: Ai Kyuse 
> Signed-off-by: Simon Horman 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied for next!

Kind regards
Uffe

> ---
>  Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 1 +
>  drivers/mmc/host/sh_mobile_sdhi.c  | 1 +
>  2 files changed, 2 insertions(+)
> ---
>  Based on mmc/next
>
> diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt 
> b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> index 0f610d4b5b00..13df9c2399c3 100644
> --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> @@ -23,6 +23,7 @@ Required properties:
> "renesas,sdhi-r8a7793" - SDHI IP on R8A7793 SoC
> "renesas,sdhi-r8a7794" - SDHI IP on R8A7794 SoC
> "renesas,sdhi-r8a7795" - SDHI IP on R8A7795 SoC
> +   "renesas,sdhi-r8a7796" - SDHI IP on R8A7796 SoC
>
>  Optional properties:
>  - toshiba,mmc-wrprotect-disable: write-protect detection is unavailable
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c 
> b/drivers/mmc/host/sh_mobile_sdhi.c
> index d679c8a533b6..49edff7fee49 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -94,6 +94,7 @@ static const struct of_device_id sh_mobile_sdhi_of_match[] 
> = {
> { .compatible = "renesas,sdhi-r8a7793", .data = 
> _rcar_gen2_compatible, },
> { .compatible = "renesas,sdhi-r8a7794", .data = 
> _rcar_gen2_compatible, },
> { .compatible = "renesas,sdhi-r8a7795", .data = 
> _rcar_gen3_compatible, },
> +   { .compatible = "renesas,sdhi-r8a7796", .data = 
> _rcar_gen3_compatible, },
> {},
>  };
>  MODULE_DEVICE_TABLE(of, sh_mobile_sdhi_of_match);
> --
> 2.7.0.rc3.207.g0ac5344
>

Re: [PATCH 2/2] mmc: host: sh_mobile_sdhi: don't populate unneeded functions

[Ulf Hansson]

On 5 September 2016 at 10:18, Wolfram Sang  wrote:
>
>> > It can be argued that this tag could be added:
>> >
>> > Fixes: 452e5eef6d311e ("mmc: tmio: Add UHS-I mode support")
>> >
>> > I don't know how well it applies, though, because the code has been
>> > modified a lot recently. But we can try.
>>
>> Please tell me if you would like me to drop any of the changes I
>> queued up in this series.
>>
>> Of course, I can also amend the change log, just tell me.
>
> If you could add the Fixes: tag, this would be welcome. Thanks, Ulf!
>

Thanks, done!

Kind regards
Uffe

Re: [PATCH v6 5/9] mmc: tmio: Add tuning support

[Ulf Hansson]

On 2 September 2016 at 13:17, Simon Horman <horms+rene...@verge.net.au> wrote:
> From: Ai Kyuse <ai.kyuse...@renesas.com>
>
> Add tuning support for use with SDR104 mode
>
> Signed-off-by: Ai Kyuse <ai.kyuse...@renesas.com>
> Signed-off-by: Simon Horman <horms+rene...@verge.net.au>
> ---
> v6 [Simon Horman]
> * As suggested by Ulf Hansson:
>   - Restore saved tuning parameters on resume
>
> v5 [Simon Horman]
> * As suggested by Ulf Hansson:
>   - Move hw reset support into a separate patch
>   - Use more descriptive name for callback to check for SSC error
>   - Rely on core to retune in case of -EILSEQ
>   - Document mandatory and optional callbacks
>
> v4 [Simon Horman]
> * As suggested by Wolfram Sang:
>   - Do not perform tuning if host->select_tuning is not set:
> it seems to make little sense to do so and moreover there is currently
> no such use-case
>   - Do not add mrc->sbc handling from tmio_mmc_request,
> this is a hang-over from earlier versions of this patchset which
> did not use core infrastructure for retuning
>   - Tidy up local variable usage
> * Correct index passed to prepare_tuning(): this seems to have
>   been the last piece of resolving the timeouts during tuning puzzle
> * Further cleanups to tmio_mmc_execute_tuning():
>   - Ensure tap is sized proportionally to its members
>   - Remove stray '*' in comment
>   - Use mmc rather than host->mmc, these are equivalent but
> the former seems tidier
>   - Correct inverted logic in setting tap values
> * Re-introduce retuning support. This was removed in v3.
>
> v3 [Simon Horman]
> * As suggested by Kuninori Morimoto:
>   - Do not add unused retuning callback to struct tmio_mmc_host
>   - Change return type of prepare_tuning callback to void
>   - Add tap_size parameter to select_tuning callback
>
> v2 [Simon Horman]
> * As suggested by Kuninori Morimoto:
>   - Actually remove unnecessary TMIO_MMC_HAS_UHS_SCC define
> * As suggested by Wolfram Sang:
>   - Rely on core to call tuning. This simplifies things somewhat.
>   - Use mmc_send_tuning()
> - A side affect of this appears to be that we now see some recoverable
>   errors logged during tuning. These are typically corrected by
>   subsequent tuning. It is the logging that is the apparent side effect
>   of this change.
>   e.g.
>   sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt 
> (CMD19)
>   sh_mobile_sdhi ee10.sd: Tuning procedure failed
> * Use bool rather than unsigned long to pass test status
>   to select_tuning() callback
> * Do not retune if init_tuning callback is not present or
>   indicates that there are no taps present
> * Retune on hardware reset
>
> v1 [Simon Horman]
> * Omit start_signal_voltage_switch and tmio_mmc_card_busy changes which are
>   already present in mainline in a different form
> * Return num from init_tuning rather than passing an extra parameter
>   to hold the return value
> * Only call host->init_tuning if it is non-NULL
> * Place tmio_mmc_execute_tuning() such that no new forward declarations are
>   required
> * Remove unused TMIO_MMC_HAS_UHS_SCC define
>
> v0 [Ai Kyuse]
> ---
>  drivers/mmc/host/tmio_mmc.h | 12 +++
>  drivers/mmc/host/tmio_mmc_pio.c | 69 
> +
>  2 files changed, 81 insertions(+)
>
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 4b71f31fba63..4b501f2d529f 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -150,6 +150,7 @@ struct tmio_mmc_host {
> struct mutexios_lock;   /* protect set_ios() context 
> */
> boolnative_hotplug;
> boolsdio_irq_enabled;
> +   u32 scc_tappos;
>
> /* Mandatory callback */
> int (*clk_enable)(struct tmio_mmc_host *host);
> @@ -165,6 +166,17 @@ struct tmio_mmc_host {
>struct mmc_ios *ios);
> int (*write16_hook)(struct tmio_mmc_host *host, int addr);
> void (*hw_reset)(struct tmio_mmc_host *host);
> +   void (*prepare_tuning)(struct tmio_mmc_host *host, unsigned long tap);
> +   bool (*check_scc_error)(struct tmio_mmc_host *host);
> +
> +   /* Mandatory callback for tuning to occur which is
> +* optional for SDR50 and mandatory for SDR104 */
> +   unsigned int (*init_tuning)(struct tmio_mmc_host *host);
> +   int (*select_tuning)(struct tmio_mmc_host *host);
> +
> +   /* Tuning values: 1 for success, 0 for failure */
> +   DECLARE_BITMAP(taps, BITS_PER_

Re: [PATCH 2/2] mmc: host: sh_mobile_sdhi: don't populate unneeded functions

[Ulf Hansson]

On 2 September 2016 at 07:23, Wolfram Sang  wrote:
> Magnus,
>
>> To my eye it looks like this patch might be adding a fix for a bug
>> introduced by another patch. R-Car Gen2 and later are quite recent
>> SoCs but I believe support for other mobile chips and earlier R-Car
>> SoCs also also covered by the SDHI driver. Also there are theTMIO
>> chips that share some software and are related but a bit different. So
>> in my opinion we need to thread lightly to avoid breakage.
>
> I'm very much with you. This is the very much reason I introduced
> TMIO_MMC_MIN_RCAR2 in 3d376fb2ea907f ("mmc: tmio/sdhi: introduce flag
> for RCar 2+ specific features") in the first place to be able to
> "protect" old hardware from new features. It was not done before! This
> is also the reason why we have commits like a21553c9e0c236 ("mmc:
> tmio/sdhi: distinguish between SCLKDIVEN and ILL_FUNC") documenting a
> difference between some old TMIO variant and our current SDHI. I spent
> quite some time finding old TMIO information somewhere for that.
>
>> My concern is what happened before this patch was applied. It looks
>> like the callbacks were installed for all hardware types which makes
>> me wonder because UHS/SDR support is only available for quite recent
>> hardware.
>
> I didn't protect these callbacks before because I assumed they are only
> called when SDR support is enabled via devicetree or platform data.
> Which is not the case for all the old platforms. I sadly missed that
> card_busy() is used when polling an SDIO card in case "broken-cd" is
> used. That was a detail I overlooked, sorry. Given my work outlined
> above I don't think one can deduce that I don't care about regressing
> old hardware, though.
>
>> The ->card_busy() callback is not yet in mainline or -next. It was
>> introduced in:
>> 0157290 mmc: host: sh_mobile_sdhi: move card_busy from tmio to sdhi
>
> Not quite, it was introduced with 452e5eef6d311e ("mmc: tmio: Add UHS-I
> mode support"). The commit you mentioned moved it from tmio*.c to
> sdhi*.c
>
>> If this patch is fixing a recent commit then perhaps some patches
>> should be squashed together to prevent bisection breakage or if a
>> patch is already part of mainline then a "Fixes:" tag might be
>> suitable.
>
> It can be argued that this tag could be added:
>
> Fixes: 452e5eef6d311e ("mmc: tmio: Add UHS-I mode support")
>
> I don't know how well it applies, though, because the code has been
> modified a lot recently. But we can try.

Please tell me if you would like me to drop any of the changes I
queued up in this series.

Of course, I can also amend the change log, just tell me.

Kind regards
Uffe

Re: [PATCH v4 2/4] mmc: tmio: Add tuning support

[Ulf Hansson]

On 1 September 2016 at 10:37, Simon Horman <ho...@verge.net.au> wrote:
> On Thu, Sep 01, 2016 at 08:46:44AM +0200, Simon Horman wrote:
>> On Wed, Aug 31, 2016 at 09:38:40AM +0200, Ulf Hansson wrote:
>> > On 30 August 2016 at 22:51, Simon Horman <ho...@verge.net.au> wrote:
>> > > On Mon, Aug 29, 2016 at 04:05:55PM +0200, Ulf Hansson wrote:
>> > >> On 29 August 2016 at 14:05, Simon Horman <ho...@verge.net.au> wrote:
>> > >> > On Fri, Aug 26, 2016 at 10:01:35AM +0200, Ulf Hansson wrote:
>> > >> >> On 25 August 2016 at 14:04, Simon Horman <ho...@verge.net.au> wrote:
>> > >> >> > On Tue, Aug 23, 2016 at 05:02:56PM +0200, Ulf Hansson wrote:
>> > >> >
>> > >> > ...
>> > >
>> > > ...
>> > >
>> > >> > @@ -1271,6 +1278,7 @@ int tmio_mmc_host_runtime_suspend(struct device 
>> > >> > *dev)
>> > >> >
>> > >> > mmc_retune_timer_stop(host->mmc);
>> > >> > mmc_retune_needed(host->mmc);
>> > >> > +   host->use_saved_taps = true;
>> > >>
>> > >> I don't think you should trigger a re-tune here at all. Moreover you
>> > >> don't need to keep track of whether you have valid tap values by using
>> > >> the ->use_saved_taps variable, the mmc core deals with this for you.
>> > >>
>> > >> Instead, you should restore the tap values by invoking
>> > >> host->select_tuning() from the ->runtime_resume() callback, although
>> > >> only when host->mmc->can_retune == 1. We should probably add a helper
>> > >> function in the mmc core for this check, instead of accessing
>> > >> "can_retune" directly.
>> > >
>> > > Thanks, I was wondering what the best way to handle this is.
>> > >
>> > > I tried your suggestion above but it seems that host->mmc->can_retune is
>> > > zero when ->runtime_resume() is called because of the following call 
>> > > paths:
>> > >
>> > > a) _mmc_sd_suspend()
>> > >  -> mmc_power_off()
>> > > -> mmc_set_initial_state()
>> > >-> mmc_retune_disable
>> > >and
>> > > b) mmc_sd_runtime_resume()
>> > >  -> mmc_power_up
>> > > -> mmc_set_initial_state()
>> > >-> mmc_retune_disable
>> > >
>> > >> > tmio_mmc_disable_mmc_irqs(host, TMIO_MASK_ALL);
>> >
>> > This is exactly what shall happen :-)
>> >
>> > If the mmc core suspends the card, it means it will also re-initialize
>> > the card when resuming it. In that way the regular tuning sequence
>> > will take place as a part of re-initialization of the card, so you
>> > definitely must not restore tap values in these case. That is why you
>> > should be using the can_retune to know when to restore the values.
>>
>> Understood. In that case things are working as expected.
>
> I do have one question. It seems to me that  host->mmc->can_retune == 1 on
> resume. If so, when would the saved tap values be used?  I feel that I am
> missing something here.

There are different "resumes" here, so this gets a bit tricky to
discuss around. Let me give it another try.

First, let's assume we have a card inserted, initialized and tuned.
This also means we have a cache of tuning (tap) values in the host.

When the *host* runtime PM suspends, which isn't related to when the
card suspends, typically the card remains powered/initialized. Then,
when the host runtime PM resumes the tuning values can be restored in
the host controller, as those should still be working because the card
has remained initialized.

When the card system PM suspends, the card will be put to sleep/power
off (can_retune == 0). Sooner or later the host also becomes runtime
PM suspended in this sequence.

When the card system PM resumes, the card will be woken up from sleep,
power on and the re-initializations starts. Before the
re-initializations starts, the host will be runtime PM resumed, but
since can_retune == 0, no tuning values will be restored (correct).
When the re-initialization sequence completes of the card, can_retune
== 1 as a new tuning sequence has also been completed. This also means
refreshed cached tunings values in the host, which it can use the next
time it will be runtime PM resumed.

Hope this made more sense now.

[...]

Kind regards
Uffe

Re: [PATCH v5 03/11] mmc: tmio: Add hw reset support

[Ulf Hansson]

On 30 August 2016 at 23:09, Simon Horman <horms+rene...@verge.net.au> wrote:
> From: Ai Kyuse <ai.kyuse...@renesas.com>
>
> Add hw reset support.
>
> Signed-off-by: Ai Kyuse <ai.kyuse...@renesas.com>
> Signed-off-by: Simon Horman <horms+rene...@verge.net.au>
> ---
> This is required by tuning support which will
> be introduced by follow-up patches.
>
> v5 [Simon Horman]
> * As suggested by Ulf Hansson
>   - Broke out of a larger patch
> ---
>  drivers/mmc/host/tmio_mmc.h |  1 +
>  drivers/mmc/host/tmio_mmc_pio.c | 12 
>  2 files changed, 13 insertions(+)
>
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 4b36cb5c2d9c..4b71f31fba63 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -164,6 +164,7 @@ struct tmio_mmc_host {
> int (*start_signal_voltage_switch)(struct mmc_host *mmc,
>struct mmc_ios *ios);
> int (*write16_hook)(struct tmio_mmc_host *host, int addr);
> +   void (*hw_reset)(struct tmio_mmc_host *host);
>  };
>
>  struct tmio_mmc_host *tmio_mmc_host_alloc(struct platform_device *pdev);
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index 806308ac93e7..90758647bae6 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -756,6 +756,17 @@ static int tmio_mmc_start_data(struct tmio_mmc_host 
> *host,
> return 0;
>  }
>
> +static void tmio_mmc_hw_reset(struct mmc_host *mmc)
> +{
> +   struct tmio_mmc_host *host = mmc_priv(mmc);
> +
> +   if (host->hw_reset)
> +   host->hw_reset(host);
> +
> +   mmc_retune_timer_stop(host->mmc);
> +   mmc_retune_needed(host->mmc);

Both the above tuning calls are completely pointless as the mmc core
will run a reinitialization of the card when a ->hw_reset() host ops
is invoked.
That means a regular tuning will happen as part of the initialization
of the card, so you don't need to trigger it from here as well.

So if that's the only reason to why you need to add the hw_reset()
support, I think you should drop it instead.

> +}
> +
>  /* Process requests from the MMC layer */
>  static void tmio_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  {
> @@ -970,6 +981,7 @@ static struct mmc_host_ops tmio_mmc_ops = {
> .get_cd = mmc_gpio_get_cd,
> .enable_sdio_irq = tmio_mmc_enable_sdio_irq,
> .multi_io_quirk = tmio_multi_io_quirk,
> +   .hw_reset   = tmio_mmc_hw_reset,
>  };
>
>  static int tmio_mmc_init_ocr(struct tmio_mmc_host *host)
> --
> 2.7.0.rc3.207.g0ac5344
>

Kind regards
Uffe

Re: [PATCH v4 2/4] mmc: tmio: Add tuning support

[Ulf Hansson]

On 30 August 2016 at 22:51, Simon Horman <ho...@verge.net.au> wrote:
> On Mon, Aug 29, 2016 at 04:05:55PM +0200, Ulf Hansson wrote:
>> On 29 August 2016 at 14:05, Simon Horman <ho...@verge.net.au> wrote:
>> > On Fri, Aug 26, 2016 at 10:01:35AM +0200, Ulf Hansson wrote:
>> >> On 25 August 2016 at 14:04, Simon Horman <ho...@verge.net.au> wrote:
>> >> > On Tue, Aug 23, 2016 at 05:02:56PM +0200, Ulf Hansson wrote:
>> >
>> > ...
>
> ...
>
>> > @@ -1271,6 +1278,7 @@ int tmio_mmc_host_runtime_suspend(struct device *dev)
>> >
>> > mmc_retune_timer_stop(host->mmc);
>> > mmc_retune_needed(host->mmc);
>> > +   host->use_saved_taps = true;
>>
>> I don't think you should trigger a re-tune here at all. Moreover you
>> don't need to keep track of whether you have valid tap values by using
>> the ->use_saved_taps variable, the mmc core deals with this for you.
>>
>> Instead, you should restore the tap values by invoking
>> host->select_tuning() from the ->runtime_resume() callback, although
>> only when host->mmc->can_retune == 1. We should probably add a helper
>> function in the mmc core for this check, instead of accessing
>> "can_retune" directly.
>
> Thanks, I was wondering what the best way to handle this is.
>
> I tried your suggestion above but it seems that host->mmc->can_retune is
> zero when ->runtime_resume() is called because of the following call paths:
>
> a) _mmc_sd_suspend()
>  -> mmc_power_off()
> -> mmc_set_initial_state()
>-> mmc_retune_disable
>and
> b) mmc_sd_runtime_resume()
>  -> mmc_power_up
> -> mmc_set_initial_state()
>-> mmc_retune_disable
>
>> > tmio_mmc_disable_mmc_irqs(host, TMIO_MASK_ALL);

This is exactly what shall happen :-)

If the mmc core suspends the card, it means it will also re-initialize
the card when resuming it. In that way the regular tuning sequence
will take place as a part of re-initialization of the card, so you
definitely must not restore tap values in these case. That is why you
should be using the can_retune to know when to restore the values.

>
>
> I plan to repost this patchset without the tap restoration code.
> This is because I have a number of changes locally, in particular
> to address other parts of your review, and I would like
> them to see the light of day.
>
> I will mark the tap restoration as a TODO item which we can address
> before merging the code in question.

It shouldn't be that hard to implement this, so I rather see that we
get this right from the beginning.

As matter of fact it probably requires less code than you had in your
initial approach, especially since I don't think you need to deal with
anything tuning related in the ->runtime_suspend() callback, but only
in ->runtime_resume().

Kind regards
Uffe

Re: [PATCH v4 2/4] mmc: tmio: Add tuning support

[Ulf Hansson]

On 29 August 2016 at 14:05, Simon Horman <ho...@verge.net.au> wrote:
> On Fri, Aug 26, 2016 at 10:01:35AM +0200, Ulf Hansson wrote:
>> On 25 August 2016 at 14:04, Simon Horman <ho...@verge.net.au> wrote:
>> > On Tue, Aug 23, 2016 at 05:02:56PM +0200, Ulf Hansson wrote:
>
> ...
>
>> >> >> I am wondering whether it would it be possible to keep a cache of the
>> >> >> currently used tuning values and then restore these values at
>> >> >> runtime_resume?
>> >> >>
>> >> >> In that way you wouldn't need to force new re-tuning cycle here.
>> >> >
>> >> > I will check with the hardware people to see if it is practical from
>> >> > that POV.
>> >>
>> >> Okay!
>> >
>> > The feedback I received is something like this:
>> >
>> > * The tap values calculated during retuning depend on the temperature of
>> >   the SoC and card.
>> > * So after resume the values may be invalid.
>>
>> They values may also become invalid during long continues transfers,
>> which prevents the ->runtime_suspend() callback to be invoked -
>> because the temperature may increase.
>>
>> > * It may be possible to re-use the TAP values and re-tune if a transfer
>> >   fails but the people I spoke with were unsure of the merit of that
>> >   approach
>>
>> There's is a huge benefit!
>>
>> You will get a significant decreased latency at ->runtime_resume() as
>> you don't need to run a complete re-tuning cycle.
>>
>> I can't really give you fresh numbers as it's a long time I tried this
>> myself, although if you did some measurements on this it would be
>> nice! :-)
>>
>> >
>> > Personally my feeling is that we should initiate a retune on resume for
>> > now as that seems to be the safest option.
>>
>> I don't agree. :-) I think it's better to postpone re-tune until it's
>> actually needed.
>>
>> Re-tune happens in the request error handling path, but also if you
>> configure a re-tuning period. That ought to be sufficient, don't you
>> think?
>
> Hi Ulf,
>
> Is something like this close to what you have in mind?
>
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c 
> b/drivers/mmc/host/sh_mobile_sdhi.c
> index 94e79449c533..fc43cf5ce958 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -357,11 +357,9 @@ static void sh_mobile_sdhi_prepare_tuning(struct 
> tmio_mmc_host *host,
>
>  #define SH_MOBILE_SDHI_MAX_TAP 3
>
> -static int sh_mobile_sdhi_select_tuning(struct tmio_mmc_host *host,
> -   bool *tap, int tap_size)
> +static int sh_mobile_sdhi_select_tuning(struct tmio_mmc_host *host)
>  {
> struct sh_mobile_sdhi *priv = host_to_priv(host);
> -   unsigned long tap_num;  /* total number of taps */
> unsigned long tap_cnt;  /* counter of tuning success */
> unsigned long tap_set;  /* tap position */
> unsigned long tap_start;/* start position of tuning success */
> @@ -372,14 +370,6 @@ static int sh_mobile_sdhi_select_tuning(struct 
> tmio_mmc_host *host,
> /* Clear SCC_RVSREQ */
> sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSREQ, 0);
>
> -   /* Select SCC */
> -   tap_num = (sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL) >>
> -  SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT) &
> -   SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_MASK;
> -
> -   if (tap_num * 2 != tap_size)
> -   return -EINVAL;
> -
> /*
>  * Find the longest consecutive run of successful probes.  If that
>  * is more than SH_MOBILE_SDHI_MAX_TAP probes long then use the
> @@ -389,8 +379,8 @@ static int sh_mobile_sdhi_select_tuning(struct 
> tmio_mmc_host *host,
> ntap = 0;
> tap_start = 0;
> tap_end = 0;
> -   for (i = 0; i < tap_num * 2; i++) {
> -   if (tap[i] == 0)
> +   for (i = 0; i < host->tap_num * 2; i++) {
> +   if (test_bit(i, host->taps))
> ntap++;
> else {
> if (ntap > tap_cnt) {
> @@ -409,7 +399,7 @@ static int sh_mobile_sdhi_select_tuning(struct 
> tmio_mmc_host *host,
> }
>
> if (tap_cnt >= SH_MOBILE_SDHI_MAX_TAP)
> -   tap_set = (tap_start + tap_end) / 2 % tap_num;
> +   tap_set = (tap_start + tap_end) / 2 % host->tap_num;
> else
> return -EIO;
&

Re: [PATCH v4 2/4] mmc: tmio: Add tuning support

[Ulf Hansson]

On 25 August 2016 at 14:04, Simon Horman <ho...@verge.net.au> wrote:
> On Tue, Aug 23, 2016 at 05:02:56PM +0200, Ulf Hansson wrote:
>> [...]
>>
>> >> > +static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
>> >> > +{
>> >> > +   struct tmio_mmc_host *host = mmc_priv(mmc);
>> >> > +   unsigned int num;
>> >> > +   int i, ret = 0;
>> >> > +   bool *tap;
>> >> > +
>> >> > +   if (!host->init_tuning || !host->select_tuning)
>> >>
>> >> When defining these callbacks, it would be nice to know which ones are
>> >> optional and which ones are required.
>> >
>> > Would some comments in struct tmio_mmc_host be an appropriate way
>> > to achieve that?
>>
>> Yes, that would be nice!
>>
>> [...]
>>
>> >> >  static int tmio_mmc_init_ocr(struct tmio_mmc_host *host)
>> >> > @@ -1202,6 +1276,9 @@ int tmio_mmc_host_runtime_suspend(struct device 
>> >> > *dev)
>> >> > struct mmc_host *mmc = dev_get_drvdata(dev);
>> >> > struct tmio_mmc_host *host = mmc_priv(mmc);
>> >> >
>> >> > +   mmc_retune_timer_stop(host->mmc);
>> >> > +   mmc_retune_needed(host->mmc);
>> >>
>> >> I am wondering whether it would it be possible to keep a cache of the
>> >> currently used tuning values and then restore these values at
>> >> runtime_resume?
>> >>
>> >> In that way you wouldn't need to force new re-tuning cycle here.
>> >
>> > I will check with the hardware people to see if it is practical from
>> > that POV.
>>
>> Okay!
>
> The feedback I received is something like this:
>
> * The tap values calculated during retuning depend on the temperature of
>   the SoC and card.
> * So after resume the values may be invalid.

They values may also become invalid during long continues transfers,
which prevents the ->runtime_suspend() callback to be invoked -
because the temperature may increase.

> * It may be possible to re-use the TAP values and re-tune if a transfer
>   fails but the people I spoke with were unsure of the merit of that
>   approach

There's is a huge benefit!

You will get a significant decreased latency at ->runtime_resume() as
you don't need to run a complete re-tuning cycle.

I can't really give you fresh numbers as it's a long time I tried this
myself, although if you did some measurements on this it would be
nice! :-)

>
> Personally my feeling is that we should initiate a retune on resume for
> now as that seems to be the safest option.

I don't agree. :-) I think it's better to postpone re-tune until it's
actually needed.

Re-tune happens in the request error handling path, but also if you
configure a re-tuning period. That ought to be sufficient, don't you
think?

[...]

Kind regards
Uffe

Re: [PATCH 0/2] mmc: sh_mobile_sdhi: use SDR & card_busy only on Gen2+

[Ulf Hansson]

On 24 August 2016 at 11:34, Wolfram Sang
 wrote:
> This series hopefully fixes a performance regression for r8a73a4 (pre Gen2)
> caused by a not working card_busy. Because we don't have documentation for its
> SDHI (and I don't have the hardware), just enable card_busy and SDR only for
> machines known to have hardware support for it.
>
> Geert, can you test, please?
>
> Wolfram Sang (2):
>   mmc: host: sh_mobile_sdhi: move card_busy from tmio to sdhi
>   mmc: host: sh_mobile_sdhi: don't populate unneeded functions
>
>  drivers/mmc/host/sh_mobile_sdhi.c | 16 +++-
>  drivers/mmc/host/tmio_mmc.h   |  1 +
>  drivers/mmc/host/tmio_mmc_pio.c   |  9 +
>  3 files changed, 17 insertions(+), 9 deletions(-)
>

Thanks, applied for next!

Kind regards
Uffe

Re: [PATCH v4 2/4] mmc: tmio: Add tuning support

[Ulf Hansson]

On 27 July 2016 at 06:13, Simon Horman  wrote:
> From: Ai Kyuse 
>
> Add tuning support for use with SDR104 mode
>
> Signed-off-by: Ai Kyuse 
> Signed-off-by: Simon Horman 
> ---
> v4 [Simon Horman]
> As suggested by Wolfram Sang:
>   - Do not perform tuning if host->select_tuning is not set:
> it seems to make little sense to do so and moreover there is currently
> no such use-case
>   - Do not add mrc->sbc handling from tmio_mmc_request,
> this is a hang-over from earlier versions of this patchset which
> did not use core infrastructure for retuning
>   - Tidy up local variable usage
> * Correct index passed to prepare_tuning(): this seems to have
>   been the last piece of resolving the timeouts during tuning puzzle
> * Further cleanups to tmio_mmc_execute_tuning():
>   - Ensure tap is sized proportionally to its members
>   - Remove stray '*' in comment
>   - Use mmc rather than host->mmc, these are equivalent but
> the former seems tidier
>   - Correct inverted logic in setting tap values
> * Re-introduce retuning support. This was removed in v3.
>
> v3 [Simon Horman]
> * As suggested by Kuninori Morimoto:
>   - Do not add unused retuning callback to struct tmio_mmc_host
>   - Change return type of prepare_tuning callback to void
>   - Add tap_size parameter to select_tuning callback
>
> v2 [Simon Horman]
> * As suggested by Kuninori Morimoto:
>   - Actually remove unnecessary TMIO_MMC_HAS_UHS_SCC define
> * As suggested by Wolfram Sang:
>   - Rely on core to call tuning. This simplifies things somewhat.
>   - Use mmc_send_tuning()
> - A side affect of this appears to be that we now see some recoverable
>   errors logged during tuning. These are typically corrected by
>   subsequent tuning. It is the logging that is the apparent side effect
>   of this change.
>   e.g.
>   sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt 
> (CMD19)
>   sh_mobile_sdhi ee10.sd: Tuning procedure failed
> * Use bool rather than unsigned long to pass test status
>   to select_tuning() callback
> * Do not retune if init_tuning callback is not present or
>   indicates that there are no taps present
> * Retune on hardware reset
>
> v1 [Simon Horman]
> * Omit start_signal_voltage_switch and tmio_mmc_card_busy changes which are
>   already present in mainline in a different form
> * Return num from init_tuning rather than passing an extra parameter
>   to hold the return value
> * Only call host->init_tuning if it is non-NULL
> * Place tmio_mmc_execute_tuning() such that no new forward declarations are
>   required
> * Remove unused TMIO_MMC_HAS_UHS_SCC define
>
> v0 [Ai Kyuse]
>
> Signed-off-by: Simon Horman 
> ---
>  drivers/mmc/host/tmio_mmc.h |  7 
>  drivers/mmc/host/tmio_mmc_pio.c | 77 
> +
>  2 files changed, 84 insertions(+)
>
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 7f63ec05bdf4..316b0c3fe745 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -150,6 +150,7 @@ struct tmio_mmc_host {
> struct mutexios_lock;   /* protect set_ios() context 
> */
> boolnative_hotplug;
> boolsdio_irq_enabled;
> +   u32 scc_tappos;
>
> int (*write16_hook)(struct tmio_mmc_host *host, int addr);
> int (*clk_enable)(struct tmio_mmc_host *host);
> @@ -160,6 +161,12 @@ struct tmio_mmc_host {
>   unsigned int direction, int blk_size);
> int (*start_signal_voltage_switch)(struct mmc_host *mmc,
>struct mmc_ios *ios);
> +   unsigned int (*init_tuning)(struct tmio_mmc_host *host);
> +   void (*prepare_tuning)(struct tmio_mmc_host *host, unsigned long tap);
> +   int (*select_tuning)(struct tmio_mmc_host *host, bool *tap,
> +int tap_size);
> +   bool (*retuning)(struct tmio_mmc_host *host);
> +   void (*hw_reset)(struct tmio_mmc_host *host);

Please add the HW reset support in separate patch. I guess you need it
to go in before the tuning support.

>  };
>
>  struct tmio_mmc_host *tmio_mmc_host_alloc(struct platform_device *pdev);
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index a9d07b5f3c63..83b5148a2684 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -36,6 +36,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -298,6 +299,15 @@ static void tmio_mmc_finish_request(struct tmio_mmc_host 
> *host)
> if (mrq->cmd->error || (mrq->data && mrq->data->error))
> tmio_mmc_abort_dma(host);
>
> +   if (host->retuning) {
> +   int 

Re: [PATCH 4/4] PM / Runtime: Defer resuming of the device in pm_runtime_force_resume()

[Ulf Hansson]

On 28 June 2016 at 18:26, Geert Uytterhoeven <ge...@linux-m68k.org> wrote:
> Hi Ulf, Rafael,
>
> On Tue, May 17, 2016 at 1:41 PM, Ulf Hansson <ulf.hans...@linaro.org> wrote:
>> When the pm_runtime_force_suspend|resume() helpers were invented, we still
>> had CONFIG_PM_RUNTIME and CONFIG_PM_SLEEP as separate Kconfig options.
>>
>> To make sure these helpers worked for all combinations and without
>> introducing too much of complexity, the device was always resumed in
>> pm_runtime_force_resume().
>>
>> More precisely, when CONFIG_PM_SLEEP was set and CONFIG_PM_RUNTIME was
>> unset, we needed to resume the device as the subsystem/driver couldn't
>> rely on using runtime PM to do it.
>>
>> As the CONFIG_PM_RUNTIME option was merged into CONFIG_PM a while ago, it
>> removed this combination, of using CONFIG_PM_SLEEP without the earlier
>> CONFIG_PM_RUNTIME.
>>
>> For this reason we can now rely on the subsystem/driver to use runtime PM
>> to resume the device, instead of forcing that to be done in all cases. In
>> other words, let's defer this to a later point when it's actually needed.
>>
>> Signed-off-by: Ulf Hansson <ulf.hans...@linaro.org>
>> ---
>>  drivers/base/power/runtime.c | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
>> index 09e4eb1..1db7b46 100644
>> --- a/drivers/base/power/runtime.c
>> +++ b/drivers/base/power/runtime.c
>> @@ -1509,6 +1509,17 @@ int pm_runtime_force_resume(struct device *dev)
>> if (!pm_runtime_status_suspended(dev))
>> goto out;
>>
>> +   /*
>> +* The PM core increases the runtime PM usage count in the system PM
>> +* prepare phase. If the count is greather than 1 at this point, 
>> someone
>> +* else has also increased it. In that case, invoke the runtime 
>> resume
>> +* callback for the device as that is likely what is expected. In 
>> other
>> +* case we trust the subsystem/driver to runtime resume the device 
>> when
>> +* it's actually needed.
>> +*/
>> +   if (atomic_read(>power.usage_count) < 2)
>> +   goto out;
>> +
>> ret = pm_runtime_set_active(dev);
>> if (ret)
>> goto out;
>
> This patch (commit eb13a0a1b6d5d5c2 in pm/linux-next) breaks resume on

Thanks for reporting!

> sh73a0/kzm9g and r8a73a4/ape6evm. On these boards, the Ethernet controller is 
> a
> child of a local bus (bsc), whose clock (zb) is controlled through pm_clk and
> simple-pm-bus, cfr.
>
> arch/arm/boot/dts/r8a73a4-ape6evm.dts
> arch/arm/boot/dts/r8a73a4.dtsi
> arch/arm/boot/dts/sh73a0-kzm9g.dts
> arch/arm/boot/dts/sh73a0.dtsi
>
> During resume, the bus clock is not enabled, causing an imprecise abort
> when accessing the Ethernet controller's registers. With some debug code
> added:

I was looking into this in some more detail.

As you are stating that Ethernet device is a child device to the local
bus device, I would have expected that the pm_runtime_get_sync()
invoked during ->probe() in the Ethernet driver should cause its
parent device (the bus device) to be "forced" resumed.

That's because the pm_runtime_get_sync() should trigger an increased
usage count of the parent device. Later, when
pm_runtime_force_resume() validates the count for the bus device, it
should be 2 and thus it should lead to that it decides to *not* defer
the resume of the device. Apparently this isn't happening for some
reason...

Perhaps there's a pm_suspend_ignore_children() set for the parent?
Or perhaps the parent/child relationship isn't set up correctly?

>
> PM: Syncing filesystems ... done.
> PM: Preparing system for sleep (mem)
> Freezing user space processes ... (elapsed 0.001 seconds) done.
> Double checking all user space processes after OOM killer
> disable... (elapsed 0.000 seconds)
> Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
> PM: Suspending system (mem)
> smsc911x: smsc911x_suspend
> simple-pm-bus fec1.bus: simple_pm_bus_suspend
> PM: suspend of devices complete after 21.484 msecs
> PM: suspend devices took 0.030 seconds
> PM: late suspend of devices complete after 0.488 msecs
> cpg_div6_clock_disable: zb
> rmobile_pd_power_down: a3sp
> PM: noirq suspend of devices complete after 8.300 msecs
> Disabling non-boot CPUs ...
> CPU1: shutdown
>
> PM: early resume of devices complete after 0.488 msecs
> simple-pm-bus fec1.bus: 

Re: [PATCH] mmc: tmio: make a cast explicit

[Ulf Hansson]

On 27 May 2016 at 14:10, Wolfram Sang  wrote:
> From: Wolfram Sang 
>
> Sparse complains about the implicit cast. Making it explicit is indeed
> better coding style.
>
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/tmio_mmc.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 7d1174c789219a..5f2919ceced12d 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -265,7 +265,7 @@ static inline void sd_ctrl_write16_rep(struct 
> tmio_mmc_host *host, int addr,
>
>  static inline void sd_ctrl_write32_as_16_and_16(struct tmio_mmc_host *host, 
> int addr, u32 val)
>  {
> -   writew(val, host->ctl + (addr << host->bus_shift));
> +   writew(val & 0x, host->ctl + (addr << host->bus_shift));
> writew(val >> 16, host->ctl + ((addr + 2) << host->bus_shift));
>  }
>
> --
> 2.8.1
>

Re: [PATCH] mmc: sh_mobile_sdhi: properly document R-Car versions

[Ulf Hansson]

On 11 May 2016 at 14:46, Wolfram Sang  wrote:
> From: Wolfram Sang 
>
> Replace hardcoded values with meaningful names and document what we
> know.
>
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sh_mobile_sdhi.c | 14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c 
> b/drivers/mmc/host/sh_mobile_sdhi.c
> index f750f9494410b0..c3b651bf89cb4a 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -39,6 +39,12 @@
>
>  #define EXT_ACC   0xe4
>
> +#define SDHI_VER_GEN2_SDR500x490c
> +/* very old datasheets said 0x490c for SDR104, too. They are wrong! */
> +#define SDHI_VER_GEN2_SDR104   0xcb0d
> +#define SDHI_VER_GEN3_SD   0xcc10
> +#define SDHI_VER_GEN3_SDMMC0xcd10
> +
>  #define host_to_priv(host) container_of((host)->pdata, struct 
> sh_mobile_sdhi, mmc_data)
>
>  struct sh_mobile_sdhi_of_data {
> @@ -109,14 +115,14 @@ static void sh_mobile_sdhi_sdbuf_width(struct 
> tmio_mmc_host *host, int width)
>  *  sh_mobile_sdhi_of_data :: dma_buswidth
>  */
> switch (sd_ctrl_read16(host, CTL_VERSION)) {
> -   case 0x490C:
> +   case SDHI_VER_GEN2_SDR50:
> val = (width == 32) ? 0x0001 : 0x;
> break;
> -   case 0xCB0D:
> +   case SDHI_VER_GEN2_SDR104:
> val = (width == 32) ? 0x : 0x0001;
> break;
> -   case 0xCC10: /* Gen3, SD only */
> -   case 0xCD10: /* Gen3, SD + MMC */
> +   case SDHI_VER_GEN3_SD:
> +   case SDHI_VER_GEN3_SDMMC:
> if (width == 64)
> val = 0x;
> else if (width == 32)
> --
> 2.8.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: sh_mobile_sdhi: enable SDIO IRQs for RCar Gen3

[Ulf Hansson]

On 9 May 2016 at 17:01, Wolfram Sang  wrote:
> From: Wolfram Sang 
>
> Tested on a Salvator-X board with a Spectec SDW-823 WLAN card.
>
> Signed-off-by: Wolfram Sang 

Just to let you know, this one was queued a while ago so it was
included in the 4.7 pull request.

Thanks!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sh_mobile_sdhi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c 
> b/drivers/mmc/host/sh_mobile_sdhi.c
> index 5309c73be1f04f..f750f9494410b0 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -71,7 +71,7 @@ static const struct sh_mobile_sdhi_of_data 
> of_rcar_gen2_compatible = {
>  static const struct sh_mobile_sdhi_of_data of_rcar_gen3_compatible = {
> .tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_WRPROTECT_DISABLE 
> |
>   TMIO_MMC_CLK_ACTUAL | TMIO_MMC_MIN_RCAR2,
> -   .capabilities   = MMC_CAP_SD_HIGHSPEED,
> +   .capabilities   = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
> .bus_shift  = 2,
>  };
>
> --
> 2.7.0
>
> --
> 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] MAINTAINERS: update entry for TMIO MMC driver

[Ulf Hansson]

On 9 May 2016 at 10:26, Wolfram Sang  wrote:
> From: Wolfram Sang 
>
> I have some more additions planned for this driver, so I'd like to get
> notified of other changes and coordinate them. Drop Ian as maintainer
> because he hasn't been involved in development for a while. Thanks for
> all the initial work, of course! Also, reflect the recent changes to
> the include file layout.
>
> Signed-off-by: Wolfram Sang 
> Cc: Ian Molton 

Thanks, applied for next!

> ---
>
> Ian: If you'd like to stay involved, please speak up and I'll change my patch
> accordingly.

As I applied this patch with quite short delay, I will instead
recommend Ian to send a patch re-adding himself as co-maintainer. I am
happy to apply such patch if it shows up.

Kind regards
Uffe

>
>  MAINTAINERS | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 42e65d128d015e..d22540c44b534b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11245,14 +11245,13 @@ S:Maintained
>  F: drivers/media/i2c/tc358743*
>  F: include/media/i2c/tc358743.h
>
> -TMIO MMC DRIVER
> -M: Ian Molton 
> +TMIO/SDHI MMC DRIVER
> +M: Wolfram Sang 
>  L: linux-...@vger.kernel.org
> -S: Maintained
> +S: Supported
>  F: drivers/mmc/host/tmio_mmc*
>  F: drivers/mmc/host/sh_mobile_sdhi.c
> -F: include/linux/mmc/tmio.h
> -F: include/linux/mmc/sh_mobile_sdhi.h
> +F: include/linux/mfd/tmio.h
>
>  TMP401 HARDWARE MONITOR DRIVER
>  M: Guenter Roeck 
> --
> 2.7.0
>
> --
> 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: sdio: fall back to SDIO 1.0 for broken 1.1 cards

[Ulf Hansson]

On 9 May 2016 at 09:59, Wolfram Sang  wrote:
> From: Wolfram Sang 
>
> I have two SDIO WLAN cards which specify being SDIO Rev. 1.1 cards but
> their FUNCE tuple reports the smaller size of a Rev 1.0 card. So,
> enforce 1.0 on these cards to avoid reading the not present registers.
> They are not really used anyhow. My cards initialize properly after this
> patch.
>
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/core/sdio_cis.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/sdio_cis.c b/drivers/mmc/core/sdio_cis.c
> index 6f6fc527a26338..dcb3dee59fa5f2 100644
> --- a/drivers/mmc/core/sdio_cis.c
> +++ b/drivers/mmc/core/sdio_cis.c
> @@ -177,8 +177,13 @@ static int cistpl_funce_func(struct mmc_card *card, 
> struct sdio_func *func,
> vsn = func->card->cccr.sdio_vsn;
> min_size = (vsn == SDIO_SDIO_REV_1_00) ? 28 : 42;
>
> -   if (size < min_size)
> +   if (size == 28 && vsn == SDIO_SDIO_REV_1_10) {
> +   pr_warn("%s: card has broken SDIO 1.1 CIS, forcing SDIO 
> 1.0\n",
> +   mmc_hostname(card->host));
> +   vsn = SDIO_SDIO_REV_1_00;
> +   } else if (size < min_size) {
> return -EINVAL;
> +   }
>
> /* TPLFE_MAX_BLK_SIZE */
> func->max_blksize = buf[12] | (buf[13] << 8);
> --
> 2.7.0
>
> --
> 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] ARM: shmobile: pm-rmobile: Postpone call to pm_genpd_init()

[Ulf Hansson]

On 4 May 2016 at 10:27, Geert Uytterhoeven <geert+rene...@glider.be> wrote:
> All local setup of the generic_pm_domain structure should have been
> completed before calling pm_genpd_init().
>
> Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be>

Reviewed-by: Ulf Hansson <ulf.hans...@linaro.org>

Kind regards
Uffe

> ---
>  arch/arm/mach-shmobile/pm-rmobile.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-shmobile/pm-rmobile.c 
> b/arch/arm/mach-shmobile/pm-rmobile.c
> index ecc92bad3500c1c1..175bd3d91ebcbcb4 100644
> --- a/arch/arm/mach-shmobile/pm-rmobile.c
> +++ b/arch/arm/mach-shmobile/pm-rmobile.c
> @@ -131,13 +131,13 @@ static void rmobile_init_pm_domain(struct 
> rmobile_pm_domain *rmobile_pd)
> struct dev_power_governor *gov = rmobile_pd->gov;
>
> genpd->flags = GENPD_FLAG_PM_CLK;
> -   pm_genpd_init(genpd, gov ? : _qos_governor, false);
> genpd->dev_ops.active_wakeup= rmobile_pd_active_wakeup;
> genpd->power_off= rmobile_pd_power_down;
> genpd->power_on = rmobile_pd_power_up;
> genpd->attach_dev   = cpg_mstp_attach_dev;
> genpd->detach_dev   = cpg_mstp_detach_dev;
> __rmobile_pd_power_up(rmobile_pd, false);
> +   pm_genpd_init(genpd, gov ? : _qos_governor, false);
>  }
>
>  static int rmobile_pd_suspend_busy(void)
> --
> 1.9.1
>

Re: [PATCH] mmc: sh_mobile_sdhi: remove obsolete include file

[Ulf Hansson]

On 28 April 2016 at 08:18, Wolfram Sang  wrote:
> From: Wolfram Sang 
>
> A few SH boards include the file but don't make use of it (no named
> interrupts). The SDHI code removed support for this feature as well.
> So, drop the references and ultimately remove the unneeded file.
>
> Signed-off-by: Wolfram Sang 
> Acked-by: Rich Felker 
> Acked-by: Yoshinori Sato 

Thanks, applied for next!

Kind regards
Uffe

> ---
>  arch/sh/boards/board-sh7757lcr.c |  1 -
>  arch/sh/boards/mach-ap325rxa/setup.c |  1 -
>  arch/sh/boards/mach-ecovec24/setup.c |  1 -
>  arch/sh/boards/mach-kfr2r09/setup.c  |  1 -
>  arch/sh/boards/mach-migor/setup.c|  1 -
>  arch/sh/boards/mach-se/7724/setup.c  |  1 -
>  include/linux/mmc/sh_mobile_sdhi.h   | 10 --
>  7 files changed, 16 deletions(-)
>  delete mode 100644 include/linux/mmc/sh_mobile_sdhi.h
>
> diff --git a/arch/sh/boards/board-sh7757lcr.c 
> b/arch/sh/boards/board-sh7757lcr.c
> index 324599bfad1420..0104c8199c48fe 100644
> --- a/arch/sh/boards/board-sh7757lcr.c
> +++ b/arch/sh/boards/board-sh7757lcr.c
> @@ -20,7 +20,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/sh/boards/mach-ap325rxa/setup.c 
> b/arch/sh/boards/mach-ap325rxa/setup.c
> index 62c3b81300ed28..de8393cb7313bc 100644
> --- a/arch/sh/boards/mach-ap325rxa/setup.c
> +++ b/arch/sh/boards/mach-ap325rxa/setup.c
> @@ -15,7 +15,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/sh/boards/mach-ecovec24/setup.c 
> b/arch/sh/boards/mach-ecovec24/setup.c
> index a9c0c07386fddd..6d612792f6b8ec 100644
> --- a/arch/sh/boards/mach-ecovec24/setup.c
> +++ b/arch/sh/boards/mach-ecovec24/setup.c
> @@ -13,7 +13,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/sh/boards/mach-kfr2r09/setup.c 
> b/arch/sh/boards/mach-kfr2r09/setup.c
> index 6bd9230e64e300..5deb2d82f19f78 100644
> --- a/arch/sh/boards/mach-kfr2r09/setup.c
> +++ b/arch/sh/boards/mach-kfr2r09/setup.c
> @@ -11,7 +11,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/sh/boards/mach-migor/setup.c 
> b/arch/sh/boards/mach-migor/setup.c
> index 7a04da3efce402..5de60a77eaa1ab 100644
> --- a/arch/sh/boards/mach-migor/setup.c
> +++ b/arch/sh/boards/mach-migor/setup.c
> @@ -13,7 +13,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/sh/boards/mach-se/7724/setup.c 
> b/arch/sh/boards/mach-se/7724/setup.c
> index e0e1df136642cd..f1fecd395679ae 100644
> --- a/arch/sh/boards/mach-se/7724/setup.c
> +++ b/arch/sh/boards/mach-se/7724/setup.c
> @@ -15,7 +15,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/include/linux/mmc/sh_mobile_sdhi.h 
> b/include/linux/mmc/sh_mobile_sdhi.h
> deleted file mode 100644
> index 95d6f0314a7ded..00
> --- a/include/linux/mmc/sh_mobile_sdhi.h
> +++ /dev/null
> @@ -1,10 +0,0 @@
> -#ifndef LINUX_MMC_SH_MOBILE_SDHI_H
> -#define LINUX_MMC_SH_MOBILE_SDHI_H
> -
> -#include 
> -
> -#define SH_MOBILE_SDHI_IRQ_CARD_DETECT "card_detect"
> -#define SH_MOBILE_SDHI_IRQ_SDCARD  "sdcard"
> -#define SH_MOBILE_SDHI_IRQ_SDIO"sdio"
> -
> -#endif /* LINUX_MMC_SH_MOBILE_SDHI_H */
> --
> 2.7.0
>

Re: [PATCH v3 0/5] mmc: tmio: make CTL_STATUS handling consistent

[Ulf Hansson]

On 27 April 2016 at 18:51, Wolfram Sang  wrote:
> It took me a little to discover that CTL_STATUS has a different handling than
> the other registers. CTL_STATUS and CTL_STATUS2, both u16, are merged into a
> virtual u32. However, CTL_STATUS2 was also directly accessed.
>
> Clean this up, make this consistent, and document this. Making the driver less
> complex and easier to work with.
>
> Tested on Renesas R-Car Gen2 and Gen3.
>
> Changes since V2:
>
> * rebased to mmc/next. Sorry Ulf, my branch had an older version of your next
>   included and I missed the update :(

No worries! The re-spin was probably more work at your end than at mine. :-)

>
>
> Wolfram Sang (5):
>   mmc: tmio: give read32/write32 functions more descriptive names
>   mmc: tmio: use BIT() within defines
>   mmc: tmio: use CTL_STATUS consistently
>   mmc: tmio/sdhi: distinguish between SCLKDIVEN and ILL_FUNC
>   mmc: tmio: document CTL_STATUS handling
>
>  drivers/mmc/host/sh_mobile_sdhi.c |  3 ++-
>  drivers/mmc/host/tmio_mmc.h   | 56 
> ---
>  drivers/mmc/host/tmio_mmc_pio.c   | 24 -
>  3 files changed, 43 insertions(+), 40 deletions(-)
>
> --
> 2.7.0
>

Thanks, applied for next!

Kind regards
Uffe

Re: [PATCH 0/4] clk: renesas: Clock Domain fixes

[Ulf Hansson]

On 26 April 2016 at 09:09, Geert Uytterhoeven <geert+rene...@glider.be> wrote:
> Hi Mike, Stephen,
>
> This patch series contains fixes for the Renesas CPG/MSTP and CPG/MSSR
> Clock Domains:
>   1. Postpone calls to pm_genpd_init(), as all local setup of the
>  generic_pm_domain structure should have been completed before
>  calling pm_genpd_init(),
>   2. Use always-on governor for Clock Domains, to prevent their status
>  from being shown as "off-0" in
>  /sys/kernel/debug/pm_genpd/pm_genpd_summary.
>
> I plan to queue these up with other clock updates in
> clk-renesas-for-v4.7, and send a pull request later.
>
> Thanks for your comments!
>
> Geert Uytterhoeven (4):
>   clk: renesas: mstp: Postpone call to pm_genpd_init()
>   clk: renesas: mstp: Use always-on governor for Clock Domain
>   clk: renesas: cpg-mssr: Postpone call to pm_genpd_init()
>   clk: renesas: cpg-mssr: Use always-on governor for Clock Domain
>
>  drivers/clk/renesas/clk-mstp.c | 3 +--
>  drivers/clk/renesas/renesas-cpg-mssr.c | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)

For the series:

Reviewed-by: Ulf Hansson <ulf.hans...@linaro.org>

Kind regards
Uffe

Re: [PATCH 2/4] clk: renesas: mstp: Use always-on governor for Clock Domain

[Ulf Hansson]

On 27 April 2016 at 16:04, Geert Uytterhoeven <ge...@linux-m68k.org> wrote:
> Hi Ulf,
>
> On Wed, Apr 27, 2016 at 3:59 PM, Ulf Hansson <ulf.hans...@linaro.org> wrote:
>> On 26 April 2016 at 09:09, Geert Uytterhoeven <geert+rene...@glider.be> 
>> wrote:
>>> As a pure Clock Domain does not have the concept of powering the domain
>>> itself, the CPG/MSTP driver does not provide power_off() and power_on()
>>> callbacks.
>>> However, the genpd core may still perform a dummy power down, causing
>>> /sys/kernel/debug/pm_genpd/pm_genpd_summary to report the domain's
>>> status being "off-0".
>>>
>>> Use the always-on governor to make sure the domain is never powered
>>> down, and always shows up as "on" in pm_genpd_summary.
>>
>> Hmm.
>>
>> Hypothetically, what if the clock domain would be a subdomain, where
>> its master is able to power down? Using the always on governor would
>> prevent the master from power off as well.
>
> That's correct. However, on R-Mobile/R-Car SoCs, this is not the case, so it
> doesn't matter here.
>
>> I am wondering whether we should introduce some similar as
>> pm_runtime_no_callbacks() but for the generic PM domain instead.
>>
>> What do you think?
>
> In my case, the domain is not managed by its parent, to there's no such
> analogy.

Ok, fair enough.

This looks good to me!

Kind regards
Uffe

>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds

Re: [PATCH v2 0/5] mmc: tmio: make CTL_STATUS handling consistent

[Ulf Hansson]

On 26 April 2016 at 18:53, Wolfram Sang  wrote:
> It took me a little to discover that CTL_STATUS has a different handling than
> the other registers. CTL_STATUS and CTL_STATUS2, both u16, are merged into a
> virtual u32. However, CTL_STATUS2 was also directly accessed.
>
> Clean this up, make this consistent, and document this. Making the driver less
> complex and easier to work with.
>
> Tested on Renesas R-Car Gen2 and Gen3.
>
> Changes since V1:
>
> * rebased
> * added simons review tags
>
>
> Wolfram Sang (5):
>   mmc: tmio: give read32/write32 functions more descriptive names
>   mmc: tmio: use BIT() within defines
>   mmc: tmio: use CTL_STATUS consistently
>   mmc: tmio/sdhi: distinguish between SCLKDIVEN and ILL_FUNC
>   mmc: tmio: document CTL_STATUS handling
>
>  drivers/mmc/host/sh_mobile_sdhi.c |  3 ++-
>  drivers/mmc/host/tmio_mmc.h   | 56 
> ---
>  drivers/mmc/host/tmio_mmc_pio.c   | 24 -
>  3 files changed, 43 insertions(+), 40 deletions(-)
>
> --
> 2.7.0
>

They still don't apply, sorry.

Are you re-basing towards my mmc tree's next branch?

Kind regards
Uffe

Re: [PATCH] mmc: mmcif: remove obsolete support for sh7372

[Ulf Hansson]

On 26 April 2016 at 22:34, Wolfram Sang  wrote:
> From: Wolfram Sang 
>
> There is no support for this platform in the kernel anymore. Make the
> Kconfig text more generic, so it won't get stale anymore.
>
> Reported-by: Geert Uytterhoeven 
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/Kconfig | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 04feea8354cba7..f5be219274329d 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -676,9 +676,9 @@ config MMC_SH_MMCIF
> depends on HAS_DMA
> depends on SUPERH || ARCH_RENESAS || COMPILE_TEST
> help
> - This selects the MMC Host Interface controller (MMCIF).
> + This selects the MMC Host Interface controller (MMCIF) found in 
> various
> + Renesas SoCs for SH and ARM architectures.
>
> - This driver supports MMCIF in sh7724/sh7757/sh7372.
>
>  config MMC_JZ4740
> tristate "JZ4740 SD/Multimedia Card Interface support"
> --
> 2.7.0
>

Re: [PATCH] mmc: sdhi: remove obsolete support for sh7372

[Ulf Hansson]

On 26 April 2016 at 22:29, Wolfram Sang  wrote:
> From: Wolfram Sang 
>
> There is no support for this platform in the kernel anymore.
>
> Reported-by: Geert Uytterhoeven 
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sh_mobile_sdhi.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c 
> b/drivers/mmc/host/sh_mobile_sdhi.c
> index e51d7b01d39a3b..0118d9621bebd0 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -77,7 +77,6 @@ static const struct sh_mobile_sdhi_of_data 
> of_rcar_gen3_compatible = {
>
>  static const struct of_device_id sh_mobile_sdhi_of_match[] = {
> { .compatible = "renesas,sdhi-shmobile" },
> -   { .compatible = "renesas,sdhi-sh7372" },
> { .compatible = "renesas,sdhi-sh73a0", .data = _default_cfg, },
> { .compatible = "renesas,sdhi-r8a73a4", .data = _default_cfg, },
> { .compatible = "renesas,sdhi-r8a7740", .data = _default_cfg, },
> --
> 2.7.0
>

Re: [PATCH V2 0/6] mmc: tmio/sdhi: clean up cruft

[Ulf Hansson]

On 26 April 2016 at 17:55, Wolfram Sang  wrote:
> Largely due to DT unification, some parts of the code became obsolete. Let's
> remove that, the code is complex enough still:
>
> * There are no boards anymore with named interrupt support. Drop support for
>   that (patches 2-4)
> * No need anymore for a public mmc/tmio.h header file. Merge it into the
>   private one (patch 5)
>
> Patches 1 and 6 are small cleanups found on the way :)
>
> Based on latest renesas-drivers which is based on v4.6-rc1 with my sdr50
> patches on top. Tested on a Renesas Lager board and build bot is happy, too
> (after finding some issues initially).
>
> Change since V1:
>
> * Drop a small cleanup touching SH board files, so we stay in the MMC realm.
>   I will do this as a seperate patch so this series won't get delayed.
>
> Please test, comment, apply...
>
>Wolfram
>
>
> Wolfram Sang (6):
>   mmc: sh_mobile_sdhi: don't use array for DT configs
>   mmc: sh_mobile_sdhi: remove obsolete irq_by_name registration
>   mmc: tmio: remove now unneeded seperate irq handlers
>   mmc: tmio: simplify irq handler
>   mmc: tmio: merge distributed include files
>   mmc: sh_mobile_sdhi: simplify code for voltage switching
>
>  drivers/mmc/host/sh_mobile_sdhi.c | 75 
> ---
>  drivers/mmc/host/tmio_mmc.h   | 59 +++---
>  drivers/mmc/host/tmio_mmc_dma.c   |  1 -
>  drivers/mmc/host/tmio_mmc_pio.c   | 55 ++--
>  include/linux/mmc/tmio.h  | 73 -
>  5 files changed, 79 insertions(+), 184 deletions(-)
>  delete mode 100644 include/linux/mmc/tmio.h
>
> --
> 2.7.0
>

Thanks, applied for next!

Kind regards
Uffe

Re: [PATCH v6 05/11] soc: renesas: rcar-sysc: Enable Clock Domain for I/O devices

[Ulf Hansson]

On 22 April 2016 at 09:59, Geert Uytterhoeven <ge...@linux-m68k.org> wrote:
> Hi Ulf,
>
> On Fri, Apr 22, 2016 at 9:28 AM, Ulf Hansson <ulf.hans...@linaro.org> wrote:
>>> @@ -217,6 +218,8 @@ static int rcar_sysc_pd_power_on(struct 
>>> generic_pm_domain *genpd)
>>> return rcar_sysc_power_up(>ch);
>>>  }
>>>
>>> +static bool has_cpg_mstp;
>>> +
>>
>> That's seems lazy. :-)
>>
>> Sure you can provide this as an input parameter to
>> rcar_sysc_pd_setup() instead, no?
>
> This variable is intended to go away, once all R-Car SoCs have been converted
> to use the CPG/MSSR driver, and has_cpg_mstp will always be false.
> Then we can just remove those lines, instead of reworking the parameter
> passing.

Okay, that seems like a reasonable way forward.

Reviewed-by: Ulf Hansson <ulf.hans...@linaro.org>

Kind regards
Uffe

Re: [PATCH v6 03/11] soc: renesas: rcar-sysc: Add DT support for SYSC PM domains

[Ulf Hansson]

On 20 April 2016 at 14:02, Geert Uytterhoeven <geert+rene...@glider.be> wrote:
> Populate the SYSC PM domains from DT, based on the presence of a device
> node for the System Controller. The actual power area hiearchy, and
> features of specific areas are obtained from tables in the C code.
>
> The SYSCIER and SYSCIMR register values are derived from the power areas
> present, which will help to get rid of the hardcoded values in R-Car H1
> and R-Car Gen2 platform code later.
>
> Initialization is done from an early_initcall(), to make sure the PM
> Domains are initialized before secondary CPU bringup.
>
> Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be>

Reviewed-by: Ulf Hansson <ulf.hans...@linaro.org>

Kind regards
Uffe

> ---
> v6:
>   - Drop unneeded genpd->dev_ops.active_wakeup callback,
>   - Move call to pm_genpd_init() after all local initialization,
>
> v5:
>   - Mask SYSC interrupts sources before enabling them (doesn't matter
> much as they're disabled at the GIC level anyway),
>   - Re-add explicit "always-on" power area instead of aliasing the SoC's
> Clock Domain,
>   - Merge the two initialization phases again,
>
> v4:
>   - Make sure not to clear reserved SYSCIMR bits that were set before,
>   - Make the always-on power area implicit and always present, and an
> alias of the existing SoC's Clock Domain. This makes the number of
> power areas a compile-time constant, and allows to drop PD_ALWAYS_ON
> and some checks.
>   - Split initialization in two phases,
>   - Document that ARM cores are controlled by PSCI on R-Car Gen3
> (although the underlying CPG/APMU hardware is the same as on Gen2),
>   - Minor improvements (double evaluation, unused parameter, debug
> message consolidation),
>
> v3:
>   - Drop check for CONFIG_PM_GENERIC_DOMAINS, which is now always
> enabled on R-Car SoCs,
>   - Create PM Domains from hierarchy in C data instead of DT,
>   - Initialize SYSCIER early, as SYSC needs the interrupt sources to be
> enabled to control power,
>   - Mask all SYSC interrupt sources for the CPU,
>   - Add support for an "always-on" domain,
>   - Use early_initcall() instead of core_initcall(),
>   - Do not power up CPU power areas during initialization, as this is
> handled later (directly or indirectly) by the SMP code,
>
> v2:
>   - Add missing definitions for SYSC_PWR_CA15_CPU and SYSC_PWR_CA7_CPU,
>   - Add R-Car H3 (r8a7795) support,
>   - Drop tests for CONFIG_ARCH_SHMOBILE_LEGACY,
>   - Add missing break statements in rcar_sysc_pwr_on_off(),
>   - Add missing calls to of_node_put() in error paths,
>   - Fix build if CONFIG_PM=n,
>   - Update compatible values,
>   - Update copyright.
> ---
>  drivers/soc/renesas/rcar-sysc.c | 202 
> +++-
>  drivers/soc/renesas/rcar-sysc.h |  53 +++
>  2 files changed, 254 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/soc/renesas/rcar-sysc.h
>
> diff --git a/drivers/soc/renesas/rcar-sysc.c b/drivers/soc/renesas/rcar-sysc.c
> index 9ba5fd15c53bf9b9..95f2b59cbd76f777 100644
> --- a/drivers/soc/renesas/rcar-sysc.c
> +++ b/drivers/soc/renesas/rcar-sysc.c
> @@ -2,6 +2,7 @@
>   * R-Car SYSC Power management support
>   *
>   * Copyright (C) 2014  Magnus Damm
> + * Copyright (C) 2015-2016 Glider bvba
>   *
>   * This file is subject to the terms and conditions of the GNU General Public
>   * License.  See the file "COPYING" in the main directory of this archive
> @@ -11,10 +12,15 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
>
> +#include "rcar-sysc.h"
> +
>  /* SYSC Common */
>  #define SYSCSR 0x00/* SYSC Status Register */
>  #define SYSCISR0x04/* Interrupt Status Register 
> */
> @@ -29,7 +35,8 @@
>  /*
>   * Power Control Register Offsets inside the register block for each domain
>   * Note: The "CR" registers for ARM cores exist on H1 only
> - *   Use WFI to power off, CPG/APMU to resume ARM cores on R-Car Gen2
> + *  Use WFI to power off, CPG/APMU to resume ARM cores on R-Car Gen2
> + *  Use PSCI on R-Car Gen3
>   */
>  #define PWRSR_OFFS 0x00/* Power Status Register */
>  #define PWROFFCR_OFFS  0x04/* Power Shutoff Control Register */
> @@ -48,6 +55,8 @@
>  #define SYSCISR_RETRIES1000
>  #define SYSCISR_DELAY_US   1
>
> +#define RCAR_PD_ALWAYS_ON  32  /* Always-on power area */
> +
>  static void __iomem *rcar_sysc_base;
>  static DEFINE_SPINLOCK(

Re: [PATCH v2] PM / Runtime: Only force-resume device if it has been force-suspended

[Ulf Hansson]

On 21 April 2016 at 23:07, Laurent Pinchart
 wrote:
> Hi Rafael,
>
> On Thursday 21 Apr 2016 23:02:06 Rafael J. Wysocki wrote:
>> On Thu, Apr 21, 2016 at 10:57 PM, Laurent Pinchart wrote:
>> > On Thursday 21 Apr 2016 21:52:56 Rafael J. Wysocki wrote:
>> >> On Thursday, April 21, 2016 02:52:55 AM Laurent Pinchart wrote:
>> >>> The pm_runtime_force_suspend() and pm_runtime_force_resume() helpers
>> >>> are designed to help driver being RPM-centric by offering an easy way to
>> >>> manage runtime PM state during system suspend and resume. The first
>> >>> function will force the device into runtime suspend at system suspend
>> >>> time, while the second one will perform the reverse operation at system
>> >>> resume time.
>> >>>
>> >>> However, the pm_runtime_force_resume() really forces resume, regardless
>> >>> of whether the device was running or already suspended before the call
>> >>> to pm_runtime_force_suspend(). This results in devices being runtime
>> >>> resumed at system resume time when they shouldn't.
>> >>>
>> >>> Fix this by recording whether the device has been forcefully suspended
>> >>> in pm_runtime_force_suspend() and condition resume in
>> >>> pm_runtime_force_resume() to that state.
>> >>>
>> >>> All current users of pm_runtime_force_resume() call the function
>> >>> unconditionally in their system resume handler (some actually set it as
>> >>> the resume handler), all after calling pm_runtime_force_suspend() at
>> >>> system suspend time. The change in behaviour should thus be safe.
>> >>>
>> >>> Signed-off-by: Laurent Pinchart
>> >>> 
>> >>> Reviewed-by: Kevin Hilman 
>> >>
>> >> Ulf, any comments?
>> >
>> > Ulf has proposed a different approach in "[PATCH] PM / Runtime: Defer
>> > resuming of the device in pm_runtime_force_resume()". I agree that using
>> > usage_count is better than introducing a new state flag in struct
>> > dev_pm_info, with a caveat: it doesn't work properly :-). We would have
>> > to fix genpd first, as commented in a reply to Ulf's patch.
>>
>> OK, thanks!
>>
>> Since I'd prefer to avoid adding more state flags too, I'll let you
>> guys noodle around this for a while more. :-)
>
> Let's see what we can do in a reasonable time frame. We could decide to merge
> this patch as a temporary fix until the genpd rework is complete.

Subsystems/driver that uses pm_runtime_force_suspend|resume() don't
necessarily need to have their devices attached to a genpd. In such
cases, $subject patch will be an improvement by itself.

Me personally would rather skip the intermediate step you propose, as
I prefer to properly change genpd with what is needed. Moreover, I am
already working on that so it shouldn't take too long before I can
post some patches.

Kind regards
Uffe

Re: [PATCH] PM / Runtime: Only force-resume device if it has been force-suspended

[Ulf Hansson]

On 21 April 2016 at 14:41, Laurent Pinchart
<laurent.pinch...@ideasonboard.com> wrote:
> Hi Ulf,
>
> On Thursday 21 Apr 2016 11:10:19 Ulf Hansson wrote:
>> On 21 April 2016 at 01:30, Laurent Pinchart wrote:
>> > On Monday 07 Mar 2016 11:10:08 Ulf Hansson wrote:
>> >> [...]
>> >>
>> >>>> I agree, that's a better idea. Drivers shouldn't call
>> >>>> pm_runtime_force_resume() if they haven't called
>> >>>> pm_runtime_force_suspend(), so checking the PM use count should be
>> >>>> fine. I'll modify the patch, test it and resubmit.
>> >>>
>> >>> I gave it an unfortunately unsuccessful try. The problem I ran into is
>> >>> that device_prepare() calls pm_runtime_get_noresume() calls
>> >>> pm_runtime_get_noresume(), with the corresponding pm_runtime_put() call
>> >>> being performed in device_complete(). The device power usage_count is
>> >>> thus always non-zero in the system resume handler, so I can't base the
>> >>> decision on that.
>> >>
>> >> As Alan said, let's just check against 1 instead.
>> >
>> > I gave this a try, and unfortunately it won't work.
>> >
>> > pm_genpd_prepare() resumes devices without increasing the usage count,
>> > which
>>
>> It doesn't resume them, it only increases the usage count.
>
> Maybe there's something I don't get, but pm_genpd_prepare() ends with

I realize that I did read *pm_genpd_prepare()* as *device_prepare()*,
which represents the behaviour of the PM core during the prepare phase
(drivers/base/power/main.c). In such cases my earlier reply would make
more sense, I believe.

Apologize for giving you the wrong input by not reading your response
in more detail!

>
> /*
>  * The PM domain must be in the GPD_STATE_ACTIVE state at this point,
>  * so genpd_poweron() will return immediately, but if the device
>  * is suspended (e.g. it's been stopped by genpd_stop_dev()), we need
>  * to make it operational.
>  */
> pm_runtime_resume(dev);
> __pm_runtime_disable(dev, false);
>
> ret = pm_generic_prepare(dev);
> if (ret) {
> ... /* irrelevant error handling */
> }
>
> pm_runtime_put(dev);
> return ret;
>
> The device is thus resumed. Note that the pm_runtime_put() call will decrease
> the usage count that was increased at the beginning of the function (and thus
> makes pm_genpd_prepare() neutral from a usage count point of view) but won't
> resume the device as the disable depth is increased by __pm_runtime_disable().
>
> As far as I understand, the device is thus effectively active when entering
> the driver's system suspend handler, with a usage count equal to 1 or more.
> pm_runtime_force_suspend(), which decides whether to suspend the device based
> on the device pm state and not the usage count, will thus always pm suspend
> the device.
>
> Is the above analysis correct ?

Yes.

Although, genpd is not behaving as it should here. It must *not*
resume all devices, just because the domain is powered.

>
> Now I'm a bit lost as to what happens (and what should happen) at resume time.
> Does genpd and the PM core try to suspend the device after calling the
> driver's resume handler (pm_runtime_force_resume() in this case) under some
> conditions, or does the resume handler have the last word in deciding the PM
> runtime status of the device after system resume ?

The resume callback of genpd decides what will happen, as its a PM
domain and it sits on top of the hierarchy of a devices PM callbacks.

As you have realized, there are issues in genpd regarding the system
PM code, there have even been reports about it.

For example, genpd prevents subsystem/drivers from manage their
devices during system PM - when it start the system PM phase with the
domain in powered off state.

That's not an okay behaviour, as it can't know whether a device needs
to be used to serve a request after that point, causing the device to
be runtime resumed. As genpd prevents it to be suspended via system
PM, it will stay resumed during the system PM suspend phase, at least
that is my understanding and it seems like bad idea.

In general, the system PM code in genpd needs to be modernized. It
somewhat duplicating some code from the PM core and I think the code
can be greatly simplified.

I have started to work on this quite recently, once the first steps
are done I will consider optimizations, such as not runtime resuming
devices unnecessarily.

>
>> > leads to the device always being active in pm_runtime_force_suspend(). The
>> > usage count

Re: [PATCH v5 03/11] soc: renesas: rcar-sysc: Add DT support for SYSC PM domains

[Ulf Hansson]

On 20 April 2016 at 10:24, Geert Uytterhoeven <ge...@linux-m68k.org> wrote:
> Hi Ulf,
>
> On Mon, Apr 18, 2016 at 4:02 PM, Ulf Hansson <ulf.hans...@linaro.org> wrote:
>> On 18 April 2016 at 15:39, Geert Uytterhoeven <ge...@linux-m68k.org> wrote:
>>> On Mon, Apr 18, 2016 at 2:59 PM, Geert Uytterhoeven
>>> <ge...@linux-m68k.org> wrote:
>>>> On Mon, Apr 18, 2016 at 2:21 PM, Ulf Hansson <ulf.hans...@linaro.org> 
>>>> wrote:
>>>>> [...]
>>>>>
>>>>>> +
>>>>>> +static bool rcar_sysc_active_wakeup(struct device *dev)
>>>>>> +{
>>>>>> +   return true;
>>>>>
>>>>> I am interested to know why this is always returning true. Perhaps you
>>>>> can elaborate a bit on that?
>>>>
>>>> Too many copying from old shmobile PM Domain code?
>>>> Honestly, I don't know...
>>>>
>>>> Perhaps Rafael still remembers the original rationale, as git history for
>>>> commit e3e0109138376bb2 ("ARM / shmobile: Support for I/O power domains for
>>>> SH7372 (v9)") doesn't have it.
>>>>
>>>> Google did find: https://lkml.org/lkml/2011/6/30/471
>>>>
>>>> Do we still need this at all? I.e. aren't PM Domains containing wake-up
>>>> devices kept powered automatically during system suspend?
>>>
>>> No they aren't. So for pm-rmobile we do need it.
>>
>> I don't quite understand why genpd should need to treat all devices
>> within the same domain exactly the same, it seems suboptimal.
>>
>> I guess it would be more clever to allow this to be controlled on per
>> device basis instead, so let's say from each driver.
>
> Perhaps this can be handled through device_set_wakeup_enable()?

Yes, the pm_wakeirq API should help with all what is needed.

> Unfortunately this doesn't seem to be called from e.g. gpio-keys.
>

Okay, so it's a matter of deployment for these devices/drivers.

A list of such drivers/devices that needs to be fixed would be great
to have. :-)

Kind regards
Uffe

Re: [PATCH v5 03/11] soc: renesas: rcar-sysc: Add DT support for SYSC PM domains

[Ulf Hansson]

On 18 April 2016 at 15:39, Geert Uytterhoeven <ge...@linux-m68k.org> wrote:
> Hi Ulf,
>
> On Mon, Apr 18, 2016 at 2:59 PM, Geert Uytterhoeven
> <ge...@linux-m68k.org> wrote:
>> On Mon, Apr 18, 2016 at 2:21 PM, Ulf Hansson <ulf.hans...@linaro.org> wrote:
>>> [...]
>>>
>>>> +
>>>> +static bool rcar_sysc_active_wakeup(struct device *dev)
>>>> +{
>>>> +   return true;
>>>
>>> I am interested to know why this is always returning true. Perhaps you
>>> can elaborate a bit on that?
>>
>> Too many copying from old shmobile PM Domain code?
>> Honestly, I don't know...
>>
>> Perhaps Rafael still remembers the original rationale, as git history for
>> commit e3e0109138376bb2 ("ARM / shmobile: Support for I/O power domains for
>> SH7372 (v9)") doesn't have it.
>>
>> Google did find: https://lkml.org/lkml/2011/6/30/471
>>
>> Do we still need this at all? I.e. aren't PM Domains containing wake-up
>> devices kept powered automatically during system suspend?
>
> No they aren't. So for pm-rmobile we do need it.

I don't quite understand why genpd should need to treat all devices
within the same domain exactly the same, it seems suboptimal.

I guess it would be more clever to allow this to be controlled on per
device basis instead, so let's say from each driver.

>
> For rcar-sysc it's different: as no PM Domain contains wake-up devices
> (all I/O devices are in the always-on power area), we don't need the callback.
> Will drop it in v6.

Okay, great!

Kind regards
Uffe

Re: [PATCH v5 03/11] soc: renesas: rcar-sysc: Add DT support for SYSC PM domains

[Ulf Hansson]

[...]

> +
> +static bool rcar_sysc_active_wakeup(struct device *dev)
> +{
> +   return true;

I am interested to know why this is always returning true. Perhaps you
can elaborate a bit on that?

> +}
> +
> +static int rcar_sysc_pd_power_off(struct generic_pm_domain *genpd)
> +{
> +   struct rcar_sysc_pd *pd = to_rcar_pd(genpd);
> +
> +   pr_debug("%s: %s\n", __func__, genpd->name);
> +
> +   if (pd->flags & PD_NO_CR) {
> +   pr_debug("%s: Cannot control %s\n", __func__, genpd->name);
> +   return -EBUSY;
> +   }
> +
> +   if (pd->flags & PD_BUSY) {
> +   pr_debug("%s: %s busy\n", __func__, genpd->name);
> +   return -EBUSY;
> +   }
> +
> +   return rcar_sysc_power_down(>ch);
> +}
> +
> +static int rcar_sysc_pd_power_on(struct generic_pm_domain *genpd)
> +{
> +   struct rcar_sysc_pd *pd = to_rcar_pd(genpd);
> +
> +   pr_debug("%s: %s\n", __func__, genpd->name);
> +
> +   if (pd->flags & PD_NO_CR) {
> +   pr_debug("%s: Cannot control %s\n", __func__, genpd->name);
> +   return 0;
> +   }
> +
> +   return rcar_sysc_power_up(>ch);
> +}
> +
> +static void __init rcar_sysc_pd_setup(struct rcar_sysc_pd *pd)
> +{
> +   struct generic_pm_domain *genpd = >genpd;
> +   const char *name = pd->genpd.name;
> +   struct dev_power_governor *gov = _qos_governor;
> +
> +   if (pd->flags & PD_CPU) {
> +   /*
> +* This domain contains a CPU core and therefore it should
> +* only be turned off if the CPU is not in use.
> +*/
> +   pr_debug("PM domain %s contains %s\n", name, "CPU");
> +   pd->flags |= PD_BUSY;
> +   gov = _domain_always_on_gov;
> +   } else if (pd->flags & PD_SCU) {
> +   /*
> +* This domain contains an SCU and cache-controller, and
> +* therefore it should only be turned off if the CPU cores are
> +* not in use.
> +*/
> +   pr_debug("PM domain %s contains %s\n", name, "SCU");
> +   pd->flags |= PD_BUSY;
> +   gov = _domain_always_on_gov;
> +   } else if (pd->flags & PD_NO_CR) {
> +   /*
> +* This domain cannot be turned off.
> +*/
> +   pd->flags |= PD_BUSY;
> +   gov = _domain_always_on_gov;
> +   }
> +
> +   pm_genpd_init(genpd, gov, false);

This seems weird. I don't think it correct to initialize genpd and
then continue with the below changes (assigning callbacks and power up
the domain).

I would recommend doing this in the reverse order.

> +   genpd->dev_ops.active_wakeup = rcar_sysc_active_wakeup;
> +   genpd->power_off = rcar_sysc_pd_power_off;
> +   genpd->power_on = rcar_sysc_pd_power_on;
> +
> +   if (pd->flags & (PD_CPU | PD_NO_CR)) {
> +   /* Skip CPUs (handled by SMP code) and areas without control 
> */
> +   pr_debug("%s: Not touching %s\n", __func__, genpd->name);
> +   return;
> +   }
> +
> +   if (!rcar_sysc_power_is_off(>ch)) {
> +   pr_debug("%s: %s is already powered\n", __func__, 
> genpd->name);
> +   return;
> +   }
> +
> +   rcar_sysc_power_up(>ch);
> +}

Kind regards
Uffe

Re: [PATCH 2/6] mmc: sh_mobile_sdhi: remove obsolete irq_by_name registration

[Ulf Hansson]

On 6 April 2016 at 11:25, Wolfram Sang  wrote:
> From: Wolfram Sang 
>
> There is no user left in the kernel, so this code can be removed.
> (Legacy, non-DT sh_mobile boards have been removed a while ago.) The
> diff looks more complicated than it is: The if-block for multiplexed isr
> is now the main code path, the rest is removed. A number of sh boards
> included the now deleted include file without needing it. Remove that,
> too.
>
> Signed-off-by: Wolfram Sang 
> ---
>  arch/sh/boards/board-sh7757lcr.c |  1 -
>  arch/sh/boards/mach-ap325rxa/setup.c |  1 -
>  arch/sh/boards/mach-ecovec24/setup.c |  1 -
>  arch/sh/boards/mach-kfr2r09/setup.c  |  1 -
>  arch/sh/boards/mach-migor/setup.c|  1 -
>  arch/sh/boards/mach-se/7724/setup.c  |  1 -
>  drivers/mmc/host/sh_mobile_sdhi.c| 57 
> +---
>  include/linux/mmc/sh_mobile_sdhi.h   | 10 ---
>  8 files changed, 8 insertions(+), 65 deletions(-)
>  delete mode 100644 include/linux/mmc/sh_mobile_sdhi.h

Hi Wolfram,

I need and ack from the Superh arch maintainer to apply this one.

It seems like you didn't include them on "to/cc" list, perhaps repost!?

Kind regards
Uffe

>
> diff --git a/arch/sh/boards/board-sh7757lcr.c 
> b/arch/sh/boards/board-sh7757lcr.c
> index 324599bfad1420..0104c8199c48fe 100644
> --- a/arch/sh/boards/board-sh7757lcr.c
> +++ b/arch/sh/boards/board-sh7757lcr.c
> @@ -20,7 +20,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/sh/boards/mach-ap325rxa/setup.c 
> b/arch/sh/boards/mach-ap325rxa/setup.c
> index 62c3b81300ed28..de8393cb7313bc 100644
> --- a/arch/sh/boards/mach-ap325rxa/setup.c
> +++ b/arch/sh/boards/mach-ap325rxa/setup.c
> @@ -15,7 +15,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/sh/boards/mach-ecovec24/setup.c 
> b/arch/sh/boards/mach-ecovec24/setup.c
> index a9c0c07386fddd..6d612792f6b8ec 100644
> --- a/arch/sh/boards/mach-ecovec24/setup.c
> +++ b/arch/sh/boards/mach-ecovec24/setup.c
> @@ -13,7 +13,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/sh/boards/mach-kfr2r09/setup.c 
> b/arch/sh/boards/mach-kfr2r09/setup.c
> index 6bd9230e64e300..5deb2d82f19f78 100644
> --- a/arch/sh/boards/mach-kfr2r09/setup.c
> +++ b/arch/sh/boards/mach-kfr2r09/setup.c
> @@ -11,7 +11,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/sh/boards/mach-migor/setup.c 
> b/arch/sh/boards/mach-migor/setup.c
> index 7a04da3efce402..5de60a77eaa1ab 100644
> --- a/arch/sh/boards/mach-migor/setup.c
> +++ b/arch/sh/boards/mach-migor/setup.c
> @@ -13,7 +13,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/sh/boards/mach-se/7724/setup.c 
> b/arch/sh/boards/mach-se/7724/setup.c
> index e0e1df136642cd..f1fecd395679ae 100644
> --- a/arch/sh/boards/mach-se/7724/setup.c
> +++ b/arch/sh/boards/mach-se/7724/setup.c
> @@ -15,7 +15,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c 
> b/drivers/mmc/host/sh_mobile_sdhi.c
> index 9beee48d2e280d..51d8dbd5b06a3e 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -28,7 +28,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -315,7 +314,6 @@ static int sh_mobile_sdhi_probe(struct platform_device 
> *pdev)
> struct tmio_mmc_host *host;
> struct resource *res;
> int irq, ret, i = 0;
> -   bool multiplexed_isr = true;
> struct tmio_mmc_dma *dma_priv;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -405,62 +403,23 @@ static int sh_mobile_sdhi_probe(struct platform_device 
> *pdev)
> if (ret < 0)
> goto efree;
>
> -   /*
> -* Allow one or more specific (named) ISRs or
> -* one or more multiplexed (un-named) ISRs.
> -*/
> -
> -   irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> -   if (irq >= 0) {
> -   multiplexed_isr = false;
> -   ret = devm_request_irq(>dev, irq, 
> tmio_mmc_card_detect_irq, 0,
> +   while (1) {
> +   irq = platform_get_irq(pdev, i);
> +   if (irq < 0)
> +   break;
> +   i++;
> +   ret = devm_request_irq(>dev, irq, tmio_mmc_irq, 0,
>   dev_name(>dev), host);
> if (ret)
> goto eirq;
> }
>
> -   irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
> -   if (irq >= 0) {
> -   multiplexed_isr = false;
> -   ret = 

Re: [PATCH v2 0/9] r8a7790: add UHS-I (SDR50) support to Lager

[Ulf Hansson]

On 4 April 2016 at 17:21, Wolfram Sang  wrote:
>
>> The documentation of mmc is quite poor. There are both code and
>> specification that needs to be understand, so some more help from
>> kernel doc could clearly help.
>
> I agree.
>
>> Anyway, I noticed that patch 6 helps to improve some documentation,
>> that's great - thanks!
>
> You're welcome. It wasn't too much work and maybe it helps someone with
> debugging. I found some bug reports on the web which sounded somewhat
> similar.
>
>> I only had one minor comment on patch 7, the rest looks good to me!
>
> Do you want me to resend or could you fix it before applying?

Patch 1->7 is applied for next (amended patch7), thanks!

Kind regards
Uffe

Re: [PATCH v2 7/9] mmc: sh_mobile_sdhi: Add UHS-I mode support

[Ulf Hansson]

On 4 April 2016 at 17:17, Wolfram Sang  wrote:
>
>> > +   usleep_range(5000, 5500);
>>
>> Why do you need this delay/sleep?
>
> My datasheet says to wait at least 5ms before re-enabling the clock
> after a voltage switch...
>
> Ah, found it in the core. It is doing it for 10ms. Maybe my search
> pattern was not fuzzy enough ;)
>
> Okay, so this delay can go.
>

Thanks for the clarification. I will amend the patch, no need for you
to send a new version!

Kind regards
Uffe

Re: [PATCH v2 7/9] mmc: sh_mobile_sdhi: Add UHS-I mode support

[Ulf Hansson]

On 1 April 2016 at 17:44, Wolfram Sang  wrote:
> From: Wolfram Sang 
>
> Implement voltage switch, supporting modes up to SDR-50.
>
> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton.
>
> Signed-off-by: Ben Hutchings 
> Signed-off-by: Wolfram Sang 
> ---
>  Documentation/devicetree/bindings/mmc/tmio_mmc.txt |  3 ++
>  drivers/mmc/host/sh_mobile_sdhi.c  | 50 
> ++
>  2 files changed, 53 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt 
> b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> index 7fb746dd1a68ca..0f610d4b5b005f 100644
> --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> @@ -26,3 +26,6 @@ Required properties:
>
>  Optional properties:
>  - toshiba,mmc-wrprotect-disable: write-protect detection is unavailable
> +- pinctrl-names: should be "default", "state_uhs"
> +- pinctrl-0: should contain default/high speed pin ctrl
> +- pinctrl-1: should contain uhs mode pin ctrl
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c 
> b/drivers/mmc/host/sh_mobile_sdhi.c
> index 8fd1d6b29190b6..89e0c5e9fcc668 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -32,6 +32,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
>
>  #include "tmio_mmc.h"
>
> @@ -97,6 +100,8 @@ struct sh_mobile_sdhi {
> struct clk *clk;
> struct tmio_mmc_data mmc_data;
> struct tmio_mmc_dma dma_priv;
> +   struct pinctrl *pinctrl;
> +   struct pinctrl_state *pins_default, *pins_uhs;
>  };
>
>  static void sh_mobile_sdhi_sdbuf_width(struct tmio_mmc_host *host, int width)
> @@ -205,6 +210,44 @@ static void sh_mobile_sdhi_clk_disable(struct 
> tmio_mmc_host *host)
> clk_disable_unprepare(priv->clk);
>  }
>
> +static int sh_mobile_sdhi_start_signal_voltage_switch(struct mmc_host *mmc,
> + struct mmc_ios *ios)
> +{
> +   struct tmio_mmc_host *host = mmc_priv(mmc);
> +   struct sh_mobile_sdhi *priv = host_to_priv(host);
> +   struct pinctrl_state *pin_state;
> +   int ret;
> +
> +   switch (ios->signal_voltage) {
> +   case MMC_SIGNAL_VOLTAGE_330:
> +   pin_state = priv->pins_default;
> +   break;
> +   case MMC_SIGNAL_VOLTAGE_180:
> +   pin_state = priv->pins_uhs;
> +   break;
> +   default:
> +   return -EINVAL;
> +   }
> +
> +   /*
> +* If anything is missing, assume signal voltage is fixed at
> +* 3.3V and succeed/fail accordingly.
> +*/
> +   if (IS_ERR(priv->pinctrl) || IS_ERR(pin_state))
> +   return ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330 ? 0 : 
> -EINVAL;
> +
> +   ret = mmc_regulator_set_vqmmc(host->mmc, ios);
> +   if (ret)
> +   return ret;
> +
> +   ret = pinctrl_select_state(priv->pinctrl, pin_state);
> +   if (ret)
> +   return ret;
> +
> +   usleep_range(5000, 5500);

Why do you need this delay/sleep?

> +   return 0;
> +}
> +
>  static int sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
>  {
> int timeout = 1000;
> @@ -296,6 +339,12 @@ static int sh_mobile_sdhi_probe(struct platform_device 
> *pdev)
> goto eprobe;
> }
>
> +   priv->pinctrl = devm_pinctrl_get(>dev);
> +   if (!IS_ERR(priv->pinctrl)) {
> +   priv->pins_default = pinctrl_lookup_state(priv->pinctrl, 
> PINCTRL_STATE_DEFAULT);
> +   priv->pins_uhs = pinctrl_lookup_state(priv->pinctrl, 
> "state_uhs");
> +   }
> +
> host = tmio_mmc_host_alloc(pdev);
> if (!host) {
> ret = -ENOMEM;
> @@ -319,6 +368,7 @@ static int sh_mobile_sdhi_probe(struct platform_device 
> *pdev)
> host->clk_update= sh_mobile_sdhi_clk_update;
> host->clk_disable   = sh_mobile_sdhi_clk_disable;
> host->multi_io_quirk= sh_mobile_sdhi_multi_io_quirk;
> +   host->start_signal_voltage_switch = 
> sh_mobile_sdhi_start_signal_voltage_switch;
>
> /* Orginally registers were 16 bit apart, could be 32 or 64 nowadays 
> */
> if (!host->bus_shift && resource_size(res) > 0x100) /* old way to 
> determine the shift */
> --
> 2.7.0
>
> --
> 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


Kind regards
Uffe

Re: [PATCH] PM / Runtime: Only force-resume device if it has been force-suspended

[Ulf Hansson]

[...]

>> I agree, that's a better idea. Drivers shouldn't call
>> pm_runtime_force_resume() if they haven't called pm_runtime_force_suspend(),
>> so checking the PM use count should be fine. I'll modify the patch, test it
>> and resubmit.
>
> I gave it an unfortunately unsuccessful try. The problem I ran into is that
> device_prepare() calls pm_runtime_get_noresume() calls
> pm_runtime_get_noresume(), with the corresponding pm_runtime_put() call being
> performed in device_complete(). The device power usage_count is thus always
> non-zero in the system resume handler, so I can't base the decision on that.

As Alan said, let's just check against 1 instead.

>
> I also noticed that pm_genpd_prepare() runtime-resumes the device (when the
> power domain is in the GPD_STATE_ACTIVE state). I don't know why that is, but
> it means that in practice my device gets runtime-resumed when suspending the
> system while it could stay runtime-suspended in practice.

I am aware of this and it's on my TODO list of improvements of genpd,
The issue is related to an unoptimized behaviour for how genpd deal
with wakeups during system PM.

Kind regards
Uffe

Re: [PATCH] PM / Runtime: Only force-resume device if it has been force-suspended

[Ulf Hansson]

+ Alan

On 3 March 2016 at 21:16, Laurent Pinchart
 wrote:
> The pm_runtime_force_suspend() and pm_runtime_force_resume() helpers are
> designed to help driver being RPM-centric by offering an easy way to
> manager runtime PM state during system suspend and resume. The first
> function will force the device into runtime suspend at system suspend
> time, while the second one will perform the reverse operation at system
> resume time.
>
> However, the pm_runtime_force_resume() really forces resume, regarding
> of whether the device was running or already suspended before the call
> to pm_runtime_force_suspend(). This results in devices being runtime
> resumed at system resume time when they shouldn't.
>
> Fix this by recording whether the device has been forcefully suspended
> in pm_runtime_force_suspend() and condition resume in
> pm_runtime_force_resume() to that state.
>
> All current users of pm_runtime_force_resume() call the function
> uncontionally in their system resume handler (some actually set it as
> the resume handler), all after calling pm_runtime_force_suspend() at
> system suspend time. The change in behaviour should thus be safe.
>
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/base/power/runtime.c | 12 +---
>  include/linux/pm.h   |  1 +
>  2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index 4c7055009bd6..ad2189294c9b 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -1400,6 +1400,7 @@ void pm_runtime_init(struct device *dev)
> pm_suspend_ignore_children(dev, false);
> dev->power.runtime_auto = true;
>
> +   dev->power.is_force_suspended = false;
> dev->power.request_pending = false;
> dev->power.request = RPM_REQ_NONE;
> dev->power.deferred_resume = false;
> @@ -1475,6 +1476,7 @@ int pm_runtime_force_suspend(struct device *dev)
> goto err;
>
> pm_runtime_set_suspended(dev);
> +   dev->power.is_force_suspended = true;
> return 0;
>  err:
> pm_runtime_enable(dev);
> @@ -1483,13 +1485,13 @@ err:
>  EXPORT_SYMBOL_GPL(pm_runtime_force_suspend);
>
>  /**
> - * pm_runtime_force_resume - Force a device into resume state.
> + * pm_runtime_force_resume - Force a device into resume state if needed.
>   * @dev: Device to resume.
>   *
>   * Prior invoking this function we expect the user to have brought the device
>   * into low power state by a call to pm_runtime_force_suspend(). Here we 
> reverse
> - * those actions and brings the device into full power. We update the 
> runtime PM
> - * status and re-enables runtime PM.
> + * those actions and bring the device back to its runtime PM state before 
> forced
> + * suspension. We update the runtime PM status and re-enables runtime PM.
>   *
>   * Typically this function may be invoked from a system resume callback to 
> make
>   * sure the device is put into full power state.
> @@ -1499,6 +1501,9 @@ int pm_runtime_force_resume(struct device *dev)
> int (*callback)(struct device *);
> int ret = 0;
>
> +   if (!dev->power.is_force_suspended)
> +   goto out;
> +
> callback = RPM_GET_CALLBACK(dev, runtime_resume);
>
> if (!callback) {
> @@ -1510,6 +1515,7 @@ int pm_runtime_force_resume(struct device *dev)
> if (ret)
> goto out;
>
> +   dev->power.is_force_suspended = false;
> pm_runtime_set_active(dev);
> pm_runtime_mark_last_busy(dev);
>  out:
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 6a5d654f4447..bec15e0f244e 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -596,6 +596,7 @@ struct dev_pm_info {
> unsigned intuse_autosuspend:1;
> unsigned inttimer_autosuspends:1;
> unsigned intmemalloc_noio:1;
> +   unsigned intis_force_suspended:1;
> enum rpm_requestrequest;
> enum rpm_status runtime_status;
> int runtime_error;
> --

Overall I have no objections to this change, as I think it's improving
the behaviour!

What I was thinking though, but it might be a bit controversial. :-)...
Instead of relying on whether we actually forced runtime suspend
earlier, why couldn't we instead check the runtime PM usage count of
the device?

Only when it's greater than zero, we shall do the forced resume of the
device, otherwise just re-enable runtime PM.

This would have the affect of leaving devices in runtime suspend,
until they really needs to be used again. It would thus decrease the
total system PM resume time.

Do you think this could work?

Kind regards
Uffe

Re: [RFC 5/7] mmc: sh_mobile_sdhi: Add UHS-I mode support

[Ulf Hansson]

On 19 February 2016 at 21:16, Wolfram Sang  wrote:
> From: Ben Hutchings 
>
> Implement voltage switch, supporting modes up to SDR-50.
>
> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton.
>
> Signed-off-by: Ben Hutchings 
> Signed-off-by: Wolfram Sang 
> ---
>  drivers/mmc/host/sh_mobile_sdhi.c | 57 
> +++
>  1 file changed, 57 insertions(+)
>
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c 
> b/drivers/mmc/host/sh_mobile_sdhi.c
> index 4e2ac0e5e5d415..d044fed2736a15 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -32,6 +32,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
>
>  #include "tmio_mmc.h"
>
> @@ -97,6 +100,8 @@ struct sh_mobile_sdhi {
> struct clk *clk;
> struct tmio_mmc_data mmc_data;
> struct tmio_mmc_dma dma_priv;
> +   struct pinctrl *pinctrl;
> +   struct pinctrl_state *pinctrl_3v3, *pinctrl_1v8;

May I suggest the following names for the pin states instead?
pins_default
pins_uhs

The corresponding pinctrl state-names I prefer are these:

PINCTRL_STATE_DEFAULT and "state_uhs". These names are currently used by mtk-sd.

>  };
>
>  static void sh_mobile_sdhi_sdbuf_width(struct tmio_mmc_host *host, int width)
> @@ -205,6 +210,49 @@ static void sh_mobile_sdhi_clk_disable(struct 
> tmio_mmc_host *host)
> clk_disable_unprepare(priv->clk);
>  }
>
> +static int sh_mobile_sdhi_start_signal_voltage_switch(
> +   struct tmio_mmc_host *host, unsigned char signal_voltage)
> +{
> +   struct sh_mobile_sdhi *priv = host_to_priv(host);
> +   struct pinctrl_state *state;
> +   int min_uV, max_uV;
> +   int ret;
> +
> +   switch (signal_voltage) {
> +   case MMC_SIGNAL_VOLTAGE_330:
> +   min_uV = 270;
> +   max_uV = 360;
> +   state = priv->pinctrl_3v3;
> +   break;
> +   case MMC_SIGNAL_VOLTAGE_180:
> +   min_uV = 170;
> +   max_uV = 195;
> +   state = priv->pinctrl_1v8;
> +   break;
> +   default:
> +   return -EINVAL;
> +   }
> +
> +   /*
> +* If anything is missing, assume signal voltage is fixed at
> +* 3.3V and succeed/fail accordingly.
> +*/
> +   if (IS_ERR(host->mmc->supply.vqmmc) ||
> +   IS_ERR(priv->pinctrl) || IS_ERR(state))
> +   return signal_voltage == MMC_SIGNAL_VOLTAGE_330 ? 0 : -EINVAL;
> +
> +   ret = regulator_set_voltage(host->mmc->supply.vqmmc, min_uV, max_uV);
> +   if (ret)
> +   return ret;

You should make use of the mmc_regulator_set_vqmmc() API here instead.

> +
> +   ret = pinctrl_select_state(priv->pinctrl, state);
> +   if (ret)
> +   return ret;
> +
> +   usleep_range(5000, 5500);
> +   return 0;
> +}
> +
>  static int sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
>  {
> int timeout = 1000;
> @@ -296,6 +344,14 @@ static int sh_mobile_sdhi_probe(struct platform_device 
> *pdev)
> goto eprobe;
> }
>
> +   priv->pinctrl = devm_pinctrl_get(>dev);
> +   if (!IS_ERR(priv->pinctrl)) {
> +   priv->pinctrl_3v3 = pinctrl_lookup_state(priv->pinctrl,
> +
> PINCTRL_STATE_DEFAULT);
> +   priv->pinctrl_1v8 = pinctrl_lookup_state(priv->pinctrl,
> +"1v8");

I am missing a separate DT binding documentation patch related to the
above pinctrls.

> +   }
> +
> host = tmio_mmc_host_alloc(pdev);
> if (!host) {
> ret = -ENOMEM;
> @@ -319,6 +375,7 @@ static int sh_mobile_sdhi_probe(struct platform_device 
> *pdev)
> host->clk_update= sh_mobile_sdhi_clk_update;
> host->clk_disable   = sh_mobile_sdhi_clk_disable;
> host->multi_io_quirk= sh_mobile_sdhi_multi_io_quirk;
> +   host->start_signal_voltage_switch = 
> sh_mobile_sdhi_start_signal_voltage_switch;
>
> /* Orginally registers were 16 bit apart, could be 32 or 64 nowadays 
> */
> if (!host->bus_shift && resource_size(res) > 0x100) /* old way to 
> determine the shift */
> --
> 2.7.0
>

Kind regards
Uffe

Re: [PATCH RESEND 1/3] mmc: sdhi: Add r8a7795 support

[Ulf Hansson]

On 15 February 2016 at 16:01, Wolfram Sang  wrote:
> From: Wolfram Sang 
>
> Registers are 64bit apart, so we refactor bus_shift handling a little and set
> it based on the DT compatible. Also, EXT_ACC is different. It has been tested
> on a Salvator-X (Gen3) and, to check for regressions, on a Lager (Gen2).
>
> Signed-off-by: Ai Kyuse 
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

Kind regards
Uffe

> ---
>  Documentation/devicetree/bindings/mmc/tmio_mmc.txt |  1 +
>  drivers/mmc/host/Kconfig   |  2 +-
>  drivers/mmc/host/sh_mobile_sdhi.c  | 47 
> --
>  3 files changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt 
> b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> index 400b640fabc768..7fb746dd1a68ca 100644
> --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> @@ -22,6 +22,7 @@ Required properties:
> "renesas,sdhi-r8a7792" - SDHI IP on R8A7792 SoC
> "renesas,sdhi-r8a7793" - SDHI IP on R8A7793 SoC
> "renesas,sdhi-r8a7794" - SDHI IP on R8A7794 SoC
> +   "renesas,sdhi-r8a7795" - SDHI IP on R8A7795 SoC
>
>  Optional properties:
>  - toshiba,mmc-wrprotect-disable: write-protect detection is unavailable
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 4a35ebf5165d83..d9a9d92fa8379a 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -560,7 +560,7 @@ config MMC_TMIO
>
>  config MMC_SDHI
> tristate "SH-Mobile SDHI SD/SDIO controller support"
> -   depends on SUPERH || ARM
> +   depends on SUPERH || ARM || ARM64
> depends on SUPERH || ARCH_SHMOBILE || COMPILE_TEST
> select MMC_TMIO_CORE
> help
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c 
> b/drivers/mmc/host/sh_mobile_sdhi.c
> index 557e2b9dadeec7..9aa147959276d0 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -1,6 +1,8 @@
>  /*
>   * SuperH Mobile SDHI
>   *
> + * Copyright (C) 2016 Sang Engineering, Wolfram Sang
> + * Copyright (C) 2015-16 Renesas Electronics Corporation
>   * Copyright (C) 2009 Magnus Damm
>   *
>   * This program is free software; you can redistribute it and/or modify
> @@ -43,6 +45,7 @@ struct sh_mobile_sdhi_of_data {
> unsigned long capabilities2;
> enum dma_slave_buswidth dma_buswidth;
> dma_addr_t dma_rx_offset;
> +   unsigned bus_shift;
>  };
>
>  static const struct sh_mobile_sdhi_of_data sh_mobile_sdhi_of_cfg[] = {
> @@ -65,6 +68,13 @@ static const struct sh_mobile_sdhi_of_data 
> of_rcar_gen2_compatible = {
> .dma_rx_offset  = 0x2000,
>  };
>
> +static const struct sh_mobile_sdhi_of_data of_rcar_gen3_compatible = {
> +   .tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_WRPROTECT_DISABLE 
> |
> + TMIO_MMC_CLK_ACTUAL | TMIO_MMC_FAST_CLK_CHG,
> +   .capabilities   = MMC_CAP_SD_HIGHSPEED,
> +   .bus_shift  = 2,
> +};
> +
>  static const struct of_device_id sh_mobile_sdhi_of_match[] = {
> { .compatible = "renesas,sdhi-shmobile" },
> { .compatible = "renesas,sdhi-sh7372" },
> @@ -78,6 +88,7 @@ static const struct of_device_id sh_mobile_sdhi_of_match[] 
> = {
> { .compatible = "renesas,sdhi-r8a7792", .data = 
> _rcar_gen2_compatible, },
> { .compatible = "renesas,sdhi-r8a7793", .data = 
> _rcar_gen2_compatible, },
> { .compatible = "renesas,sdhi-r8a7794", .data = 
> _rcar_gen2_compatible, },
> +   { .compatible = "renesas,sdhi-r8a7795", .data = 
> _rcar_gen3_compatible, },
> {},
>  };
>  MODULE_DEVICE_TABLE(of, sh_mobile_sdhi_of_match);
> @@ -103,6 +114,15 @@ static void sh_mobile_sdhi_sdbuf_width(struct 
> tmio_mmc_host *host, int width)
> case 0xCB0D:
> val = (width == 32) ? 0x : 0x0001;
> break;
> +   case 0xCC10: /* Gen3, SD only */
> +   case 0xCD10: /* Gen3, SD + MMC */
> +   if (width == 64)
> +   val = 0x;
> +   else if (width == 32)
> +   val = 0x0101;
> +   else
> +   val = 0x0001;
> +   break;
> default:
> /* nothing to do */
> return;
> @@ -233,16 +253,26 @@ static int sh_mobile_sdhi_probe(struct platform_device 
> *pdev)
> goto eprobe;
> }
>
> +   if (of_id && of_id->data) {
> +   const struct sh_mobile_sdhi_of_data *of_data = of_id->data;
> +
> +   mmc_data->flags |= of_data->tmio_flags;
> +   mmc_data->capabilities |= of_data->capabilities;
> +   mmc_data->capabilities2 |= of_data->capabilities2;
> +  

Re: [PATCH 0/9] mmc: sdhi: some refactoring and adding basic r8a7795 support

[Ulf Hansson]

On 2 February 2016 at 15:06, Wolfram Sang  wrote:
>
>> Although, I couldn't apply patch1, could you please send a new version
>> rebased onto my next branch.
>
> I rebased to your tree (especially patch 1, rest stayed the same) and
> pushed patches 1-8 here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git mmc/sdhi-refactor
>
> Is pulling okay, or do you want me to resend?

This is fine! I have picked them up from your tree and applied them for next!

Do note that I am re-basing my next branch from time to time, so I
assume the branch you shared with me isn't an immutable branch!?

Thanks and kind regards
Uffe

Re: [PATCH] mmc: sanitize 'bus width' in debug output

[Ulf Hansson]

On 29 January 2016 at 09:27, Wolfram Sang  wrote:
> From: Wolfram Sang 
>
> The bus width is sometimes the actual bus width, and sometimes indices
> to different arrays encoding the bus width. In my debugging case "2"
> could mean 8-bit as well as 4-bit, which was extremly confusing. Let's
> use the human-readable actual bus width in all places.
>
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/core/core.c | 2 +-
>  drivers/mmc/core/mmc.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index f95d41ffc766e5..746de248defced 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1033,7 +1033,7 @@ static inline void mmc_set_ios(struct mmc_host *host)
> "width %u timing %u\n",
>  mmc_hostname(host), ios->clock, ios->bus_mode,
>  ios->power_mode, ios->chip_select, ios->vdd,
> -ios->bus_width, ios->timing);
> +1 << ios->bus_width, ios->timing);
>
> host->ops->set_ios(host, ios);
>  }
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 07a135bc893057..4dbe3df8024b2c 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -945,7 +945,7 @@ static int mmc_select_bus_width(struct mmc_card *card)
> break;
> } else {
> pr_warn("%s: switch to bus width %d failed\n",
> -   mmc_hostname(host), ext_csd_bits[idx]);
> +   mmc_hostname(host), 1 << bus_width);
> }
> }
>
> --
> 2.6.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: make MAN_BKOPS_EN message a debug

[Ulf Hansson]

On 25 January 2016 at 20:18, Wolfram Sang  wrote:
> From: Wolfram Sang 
>
> IMO this info is only useful for developers. Most users won't need this
> information, since there is not much they can do about it.
>
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/core/mmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index bf49e44571f20a..07a135bc893057 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -501,7 +501,7 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 
> *ext_csd)
> card->ext_csd.raw_bkops_status =
> ext_csd[EXT_CSD_BKOPS_STATUS];
> if (!card->ext_csd.man_bkops_en)
> -   pr_info("%s: MAN_BKOPS_EN bit is not set\n",
> +   pr_debug("%s: MAN_BKOPS_EN bit is not set\n",
> mmc_hostname(card->host));
> }
>
> --
> 2.1.4
>
> --
> 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 9/9] mmc: sdhi: Add r8a7795 support

[Ulf Hansson]

On 25 January 2016 at 20:15, Wolfram Sang  wrote:
> From: Wolfram Sang 
>
> Registers are 64bit apart, so we refactor bus_shift handling a little
> and set it based on the DT compatible. Also, EXT_ACC is different.
>
> Signed-off-by: Ai Kyuse 
> Signed-off-by: Wolfram Sang 
> ---
>
> @MMC maintainers: I set the MMC_CAP_WAIT_WHILE_BUSY flage here and it is 
> needed
> to probe the eMMC (implementing RSP_R1 without CRC would also work). However, 
> I
> can't find an explicit statement in the documentation saying that it really
> waits when the bus is busy. Is my "it works, let's use it" approach enough, or
> is there more I can do to verify that setting MMC_CAP_WAIT_WHILE_BUSY is valid
> for this hardware?

In general from the mmc core perspective, when a host announces
MMC_CAP_WAIT_WHILE_BUSY it won't send a CMD13 to poll for busy.

I think you should try without MMC_CAP_WAIT_WHILE_BUSY, and then check
that a following CMD13 command always states that the card isn't busy.
I think the best path to try this is when sending a big write data
request, as in that case you can be quite certain that the card gets
busy between the requests.

So somewhere in the mmc block layer add some debug prints, that should do it.

Kind regards
Uffe

>
>  Documentation/devicetree/bindings/mmc/tmio_mmc.txt |  1 +
>  drivers/mmc/host/Kconfig   |  2 +-
>  drivers/mmc/host/sh_mobile_sdhi.c  | 47 
> --
>  3 files changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt 
> b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> index 400b640fabc768..7fb746dd1a68ca 100644
> --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> @@ -22,6 +22,7 @@ Required properties:
> "renesas,sdhi-r8a7792" - SDHI IP on R8A7792 SoC
> "renesas,sdhi-r8a7793" - SDHI IP on R8A7793 SoC
> "renesas,sdhi-r8a7794" - SDHI IP on R8A7794 SoC
> +   "renesas,sdhi-r8a7795" - SDHI IP on R8A7795 SoC
>
>  Optional properties:
>  - toshiba,mmc-wrprotect-disable: write-protect detection is unavailable
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 1526b8a10b094e..dd1499bd218b16 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -560,7 +560,7 @@ config MMC_TMIO
>
>  config MMC_SDHI
> tristate "SH-Mobile SDHI SD/SDIO controller support"
> -   depends on SUPERH || ARM
> +   depends on SUPERH || ARM || ARM64
> depends on SUPERH || ARCH_SHMOBILE || COMPILE_TEST
> select MMC_TMIO_CORE
> help
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c 
> b/drivers/mmc/host/sh_mobile_sdhi.c
> index 557e2b9dadeec7..f7eff5f53e0013 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -1,6 +1,8 @@
>  /*
>   * SuperH Mobile SDHI
>   *
> + * Copyright (C) 2016 Sang Engineering, Wolfram Sang
> + * Copyright (C) 2015-16 Renesas Electronics Corporation
>   * Copyright (C) 2009 Magnus Damm
>   *
>   * This program is free software; you can redistribute it and/or modify
> @@ -43,6 +45,7 @@ struct sh_mobile_sdhi_of_data {
> unsigned long capabilities2;
> enum dma_slave_buswidth dma_buswidth;
> dma_addr_t dma_rx_offset;
> +   unsigned bus_shift;
>  };
>
>  static const struct sh_mobile_sdhi_of_data sh_mobile_sdhi_of_cfg[] = {
> @@ -65,6 +68,13 @@ static const struct sh_mobile_sdhi_of_data 
> of_rcar_gen2_compatible = {
> .dma_rx_offset  = 0x2000,
>  };
>
> +static const struct sh_mobile_sdhi_of_data of_rcar_gen3_compatible = {
> +   .tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_WRPROTECT_DISABLE 
> |
> + TMIO_MMC_CLK_ACTUAL | TMIO_MMC_FAST_CLK_CHG,
> +   .capabilities   = MMC_CAP_SD_HIGHSPEED | MMC_CAP_WAIT_WHILE_BUSY,
> +   .bus_shift  = 2,
> +};
> +
>  static const struct of_device_id sh_mobile_sdhi_of_match[] = {
> { .compatible = "renesas,sdhi-shmobile" },
> { .compatible = "renesas,sdhi-sh7372" },
> @@ -78,6 +88,7 @@ static const struct of_device_id sh_mobile_sdhi_of_match[] 
> = {
> { .compatible = "renesas,sdhi-r8a7792", .data = 
> _rcar_gen2_compatible, },
> { .compatible = "renesas,sdhi-r8a7793", .data = 
> _rcar_gen2_compatible, },
> { .compatible = "renesas,sdhi-r8a7794", .data = 
> _rcar_gen2_compatible, },
> +   { .compatible = "renesas,sdhi-r8a7795", .data = 
> _rcar_gen3_compatible, },
> {},
>  };
>  MODULE_DEVICE_TABLE(of, sh_mobile_sdhi_of_match);
> @@ -103,6 +114,15 @@ static void sh_mobile_sdhi_sdbuf_width(struct 
> tmio_mmc_host *host, int width)
> case 0xCB0D:
> val = (width == 32) ? 0x : 0x0001;
> break;
> +   case 0xCC10: 

Re: [PATCH 0/9] mmc: sdhi: some refactoring and adding basic r8a7795 support

[Ulf Hansson]

On 25 January 2016 at 20:15, Wolfram Sang  wrote:
> So, here is the series to enable basic SD support on r8a7795; no DMA and UHS-I
> for now. Will be added incrementally. It turns out that the driver needs a
> little love, so some refactoring is also in place before adding the actual
> support.
>
> eMMC works in 4-bit mode, too. 8-bit mode sadly fails currently. I'll post MMC
> support patches once this issue is fixed.
>
> These patches have been tested on Gen3 (Salvator-X) and Gen2 (Lager), doing
> basic operations with SD cards and running the mmc_test driver (its results
> point out some potential corner cases to check later).
>
> A branch can be found here (including clock and DTS patches):
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/v8-sdhi
>
> Changes since RFC (only sent to sh-devel lists):
>
> * patches 1,7,8 are new
> * patch 4 drops 10ms wait also for set_clock
> * patch 9 also sets MMC_CAP_WAIT_WHILE_BUSY (see comment there)
> * some commit message rewording
>
> Please test, comment, apply...
>
> Thanks,
>
>Wolfram
>
>
> Shinobu Uehara (1):
>   mmc: sdhi: Add EXT_ACC register busy check
>
> Wolfram Sang (8):
>   mmc: tmio_dma: remove debug messages with little information
>   mmc: sdhi: error message on ENOMEM is superfluous
>   mmc: tmio: add flag to reduce delay after changing clock status
>   mmc: tmio: remove stale comments
>   mmc: sdhi: use faster clock handling on RCar Gen2
>   mmc: tmio: refactor set_clock a little
>   mmc: tmio: disable clock before changing it
>   mmc: sdhi: Add r8a7795 support
>
>  Documentation/devicetree/bindings/mmc/tmio_mmc.txt |  1 +
>  drivers/mmc/host/Kconfig   |  2 +-
>  drivers/mmc/host/sh_mobile_sdhi.c  | 54 
> +++---
>  drivers/mmc/host/tmio_mmc_dma.c| 12 -
>  drivers/mmc/host/tmio_mmc_pio.c| 27 ++-
>  include/linux/mfd/tmio.h   |  4 ++
>  include/linux/mmc/tmio.h   |  5 ++
>  7 files changed, 63 insertions(+), 42 deletions(-)
>
> --
> 2.1.4
>

Patch 1->8 looks good.

Although, I couldn't apply patch1, could you please send a new version
rebased onto my next branch.

Kind regards
Uffe