All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: "David Lanzendörfer" <david.lanzendoerfer@o2s.ch>,
	"Ulf Hansson" <ulf.hansson@linaro.org>,
	"Tomeu Vizoso" <tomeu.vizoso@collabora.com>,
	"Arnd Bergmann" <arnd@arndb.de>,
	linux-mmc@vger.kernel.org, "Chris Ball" <chris@printf.net>,
	linux-kernel@vger.kernel.org,
	"Peter Griffin" <peter.griffin@linaro.org>,
	"Chen-Yu Tsai" <wens@csie.org>, 李想 <lixiang@allwinnertech.com>,
	"Maxime Ripard" <maxime.ripard@free-electrons.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 3/4] mmc: sunxi: Reset behavior fix
Date: Tue, 16 Dec 2014 10:57:10 +0100	[thread overview]
Message-ID: <549001F6.5080202@redhat.com> (raw)
In-Reply-To: <20141216003756.15488.34861.stgit@dizzy-6.o2s.ch>

Hi,

On 16-12-14 01:37, David Lanzendörfer wrote:
> When there is only one DES available the DMA performs a FIFO reset and waits until the reinitialization has been completed.
> Disabling the SDXC_IDMAC_DES0_DIC bit prevents the DMA from sending an interrupt after it has finished this reinitialization.
>
> The flags SDXC_IDMAC_DES0_FD and SDXC_IDMAC_DES0_LD are both required in order to use the controller with more than one DES.
>
> Signed-off-by: David Lanzendörfer <david.lanzendoerfer@o2s.ch>
> Reported-by: 李想 <lixiang@allwinnertech.com>
> ---
>   drivers/mmc/host/sunxi-mmc.c |   12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> index 50bd3d2..5331c88 100644
> --- a/drivers/mmc/host/sunxi-mmc.c
> +++ b/drivers/mmc/host/sunxi-mmc.c
> @@ -310,7 +310,17 @@ static void sunxi_mmc_init_idma_des(struct sunxi_mmc_host *host,
>   	}
>
>   	pdes[0].config |= SDXC_IDMAC_DES0_FD;
> -	pdes[i - 1].config = SDXC_IDMAC_DES0_OWN | SDXC_IDMAC_DES0_LD;
> +
> +	/*
> +	 * When there is only one DES available the DMA performs a FIFO reset and waits
> +	 * until the reinitialization has been completed.
> +	 * Disabling the SDXC_IDMAC_DES0_DIC bit prevents the DMA from sending an interrupt
> +	 * after it has finished this reinitialization.
> +	 */
> +	pdes[i - 1].config = SDXC_IDMAC_DES0_OWN;
> +	pdes[i - 1].config |= SDXC_IDMAC_DES0_FD;
> +	pdes[i - 1].config |= SDXC_IDMAC_DES0_LD;
> +	pdes[i - 1].config &= ~SDXC_IDMAC_DES0_DIC;
>
>   	/*
>   	 * Avoid the io-store starting the idmac hitting io-mem before the

This is wrong. For one you are starting with an assignment, so you could just as well
set all the flags you want with a single line / assignment. Besides that you're setting
the FD flag, which should only be set for the first descriptor of a transfer, so for
multi descriptor transfers this is wrong.

Please see the patch which I send you which gets this right.

Regards,

Hans


WARNING: multiple messages have this Message-ID (diff)
From: hdegoede@redhat.com (Hans de Goede)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/4] mmc: sunxi: Reset behavior fix
Date: Tue, 16 Dec 2014 10:57:10 +0100	[thread overview]
Message-ID: <549001F6.5080202@redhat.com> (raw)
In-Reply-To: <20141216003756.15488.34861.stgit@dizzy-6.o2s.ch>

Hi,

On 16-12-14 01:37, David Lanzend?rfer wrote:
> When there is only one DES available the DMA performs a FIFO reset and waits until the reinitialization has been completed.
> Disabling the SDXC_IDMAC_DES0_DIC bit prevents the DMA from sending an interrupt after it has finished this reinitialization.
>
> The flags SDXC_IDMAC_DES0_FD and SDXC_IDMAC_DES0_LD are both required in order to use the controller with more than one DES.
>
> Signed-off-by: David Lanzend?rfer <david.lanzendoerfer@o2s.ch>
> Reported-by: ?? <lixiang@allwinnertech.com>
> ---
>   drivers/mmc/host/sunxi-mmc.c |   12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> index 50bd3d2..5331c88 100644
> --- a/drivers/mmc/host/sunxi-mmc.c
> +++ b/drivers/mmc/host/sunxi-mmc.c
> @@ -310,7 +310,17 @@ static void sunxi_mmc_init_idma_des(struct sunxi_mmc_host *host,
>   	}
>
>   	pdes[0].config |= SDXC_IDMAC_DES0_FD;
> -	pdes[i - 1].config = SDXC_IDMAC_DES0_OWN | SDXC_IDMAC_DES0_LD;
> +
> +	/*
> +	 * When there is only one DES available the DMA performs a FIFO reset and waits
> +	 * until the reinitialization has been completed.
> +	 * Disabling the SDXC_IDMAC_DES0_DIC bit prevents the DMA from sending an interrupt
> +	 * after it has finished this reinitialization.
> +	 */
> +	pdes[i - 1].config = SDXC_IDMAC_DES0_OWN;
> +	pdes[i - 1].config |= SDXC_IDMAC_DES0_FD;
> +	pdes[i - 1].config |= SDXC_IDMAC_DES0_LD;
> +	pdes[i - 1].config &= ~SDXC_IDMAC_DES0_DIC;
>
>   	/*
>   	 * Avoid the io-store starting the idmac hitting io-mem before the

This is wrong. For one you are starting with an assignment, so you could just as well
set all the flags you want with a single line / assignment. Besides that you're setting
the FD flag, which should only be set for the first descriptor of a transfer, so for
multi descriptor transfers this is wrong.

Please see the patch which I send you which gets this right.

Regards,

Hans

  reply	other threads:[~2014-12-16  9:58 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-16  0:37 [PATCH 0/4] mmc: sunxi: General fixup David Lanzendörfer
2014-12-16  0:37 ` David Lanzendörfer
2014-12-16  0:37 ` [PATCH 1/4] mmc: sunxi: Lock fix David Lanzendörfer
2014-12-16  0:37   ` David Lanzendörfer
2014-12-16  0:37 ` [PATCH 2/4] mmc: sunxi: Correcting SDXC_HARDWARE_RESET bit David Lanzendörfer
2014-12-16  0:37   ` David Lanzendörfer
2014-12-16  0:37 ` [PATCH 3/4] mmc: sunxi: Reset behavior fix David Lanzendörfer
2014-12-16  0:37   ` David Lanzendörfer
2014-12-16  9:57   ` Hans de Goede [this message]
2014-12-16  9:57     ` Hans de Goede
2014-12-28 19:32     ` sunxi-project-meetup@31c3 David Lanzendörfer
2014-12-29 10:13       ` sunxi-project-meetup@31c3 Hans de Goede
2014-12-29 10:13         ` sunxi-project-meetup@31c3 Hans de Goede
2014-12-16  0:38 ` [PATCH 4/4] mmc: sunxi: Removing unused code David Lanzendörfer
2014-12-16  0:38   ` David Lanzendörfer
2014-12-16  0:38   ` David Lanzendörfer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=549001F6.5080202@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=arnd@arndb.de \
    --cc=chris@printf.net \
    --cc=david.lanzendoerfer@o2s.ch \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=lixiang@allwinnertech.com \
    --cc=maxime.ripard@free-electrons.com \
    --cc=peter.griffin@linaro.org \
    --cc=tomeu.vizoso@collabora.com \
    --cc=ulf.hansson@linaro.org \
    --cc=wens@csie.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.