Linux-mediatek Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] crypto: mediatek - fix format string for 64-bit builds
From: Herbert Xu @ 2017-01-12 16:44 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David S. Miller, Matthias Brugger, Ryder Lee, linux-crypto,
	linux-arm-kernel, linux-mediatek, linux-kernel
In-Reply-To: <20170111135601.4047225-1-arnd@arndb.de>

On Wed, Jan 11, 2017 at 02:55:20PM +0100, Arnd Bergmann wrote:
> After I enabled COMPILE_TEST for non-ARM targets, I ran into these
> warnings:
> 
> crypto/mediatek/mtk-aes.c: In function 'mtk_aes_info_map':
> crypto/mediatek/mtk-aes.c:224:28: error: format '%d' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Werror=format=]
>    dev_err(cryp->dev, "dma %d bytes error\n", sizeof(*info));
> crypto/mediatek/mtk-sha.c:344:28: error: format '%d' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Werror=format=]
> crypto/mediatek/mtk-sha.c:550:21: error: format '%u' expects argument of type 'unsigned int', but argument 4 has type 'size_t {aka long unsigned int}' [-Werror=format=]
> 
> The correct format for size_t is %zu, so use that in all three
> cases.
> 
> Fixes: 785e5c616c84 ("crypto: mediatek - Add crypto driver support for some MediaTek chips")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 1/2] crypto: mediatek - remove ARM dependencies
From: Herbert Xu @ 2017-01-12 16:44 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David S. Miller, Matthias Brugger, Ryder Lee, linux-crypto,
	linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <20170111135104.3961730-1-arnd@arndb.de>

On Wed, Jan 11, 2017 at 02:50:19PM +0100, Arnd Bergmann wrote:
> Building the mediatek driver on an older ARM architecture results in a
> harmless warning:
> 
> warning: (ARCH_OMAP2PLUS_TYPICAL && CRYPTO_DEV_MEDIATEK) selects NEON which has unmet direct dependencies (VFPv3 && CPU_V7)
> 
> We could add an explicit dependency on CPU_V7, but it seems nicer to
> open up the build to additional configurations. This replaces the ARM
> optimized algorithm selection with the normal one that all other drivers
> use, and that in turn lets us relax the dependency on ARM and drop
> a number of the unrelated 'select' statements.
> 
> Obviously a real user would still select those other optimized drivers
> as a fallback, but as there is no strict dependency, we can leave that
> up to the user.
> 
> Fixes: 785e5c616c84 ("crypto: mediatek - Add crypto driver support for some MediaTek chips")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] crypto: mediatek: don't return garbage err on successful return
From: Herbert Xu @ 2017-01-12 16:39 UTC (permalink / raw)
  To: Colin King
  Cc: David S . Miller, Matthias Brugger, Ryder Lee, linux-crypto,
	linux-arm-kernel, linux-mediatek, linux-kernel
In-Reply-To: <20170103132122.26900-1-colin.king@canonical.com>

On Tue, Jan 03, 2017 at 01:21:22PM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> In the case where keylen <= bs mtk_sha_setkey returns an uninitialized
> return value in err.  Fix this by returning 0 instead of err.
> 
> Issue detected by static analysis with cppcheck.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH v2 1/2] mmc: core: Fix CMD6 timeout issue
From: Ulf Hansson @ 2017-01-12 15:21 UTC (permalink / raw)
  To: Yong Mao
  Cc: Chunfeng Yun, Eddie Huang, Adrian Hunter, Shawn Lin,
	Linus Walleij, Chaotian Jing, linux-mmc@vger.kernel.org,
	linux-mediatek
In-Reply-To: <1483433397-11756-2-git-send-email-yong.mao@mediatek.com>

- trimmed cc-list

On 3 January 2017 at 09:49, Yong Mao <yong.mao@mediatek.com> wrote:
> From: yong mao <yong.mao@mediatek.com>
>
> When initializing EMMC, after switch to HS400,
> it will issue CMD6 to change ext_csd,
> if first CMD6 got CRC error,
> the repeat CMD6 may get timeout,
> that's because card is not back to transfer state immediately.
>
> For resolving this issue, it need check if card is busy
> before sending repeat CMD6.

I agree that doing retries in this path might may not be correctly
done, but currently we do it as best effort.

We should probably change this to *not* retry the CMD6 command, in
cases when we have a MMC_RSP_R1B response, because of the reasons you
describe.
I get the feeling that these retry attempts for CMD6, may instead hide
other real issues and making it harder to narrow them down. Don't you
think?

This leads to my following question:
Why do you get a CRC error at the first CMD6 attempt? That shouldn't
happen, right?

Perhaps you can elaborate on what of the CMD6 commands in the HS400
enabling sequence that fails. It may help us to understand, perhaps
there may be something in that sequence that should be changed.

>
> Not only CMD6 here has this issue, but also other R1B CMD has
> the same issue.

Yes, agree!

However, can you please try to point out some other commands than CM6
that you see uses *retries*, has R1B response, and which you believe
may not be properly managed.

Dealing with R1B responses isn't always straight forward. Therefore I
am wondering whether we perhaps should just not allow "automatic
retries" in cases when R1B responses is used.

The reason why I think that is easier, is because of the complexity we
have when dealing with R1B responses.

As for example the timeout may differ depending on the command, so
just guessing that 500 ms might work, isn't good enough. Moreover we
would need to deal with MMC_CAP_WAIT_WHILE_BUSY, etc. Currently I am
not saying that we shouldn't do this, but then I first need to
understand how big of problem this is.

>
> Signed-off-by: Yong Mao <yong.mao@mediatek.com>
> Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
> ---
>  drivers/mmc/core/core.c |   19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 1076b9d..8674dbb 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -566,6 +566,25 @@ void mmc_wait_for_req_done(struct mmc_host *host, struct mmc_request *mrq)
>
>                 mmc_retune_recheck(host);
>
> +               /*
> +                * If a R1B CMD such as CMD6 occur CRC error,
> +                * it will retry 3 times here.
> +                * But before retrying, it must ensure card is in
> +                * transfer state.
> +                * Otherwise, the next retried CMD will got TMO error.
> +                */
> +               if (mmc_resp_type(cmd) == MMC_RSP_R1B && host->ops->card_busy) {
> +                       int tries = 500; /* Wait aprox 500ms at maximum */
> +
> +                       while (host->ops->card_busy(host) && --tries)
> +                               mmc_delay(1);
> +
> +                       if (tries == 0) {
> +                               cmd->error = -EBUSY;
> +                               break;
> +                       }
> +               }
> +
>                 pr_debug("%s: req failed (CMD%u): %d, retrying...\n",
>                          mmc_hostname(host), cmd->opcode, cmd->error);
>                 cmd->retries--;
> --
> 1.7.9.5
>

Kind regards
Uffe

^ permalink raw reply

* [PATCH -next] crypto: mediatek - make symbol of_crypto_id static
From: Wei Yongjun @ 2017-01-12 15:03 UTC (permalink / raw)
  To: Herbert Xu, Matthias Brugger, Ryder Lee
  Cc: linux-mediatek, Wei Yongjun, linux-crypto, linux-arm-kernel

From: Wei Yongjun <weiyongjun1@huawei.com>

Fixes the following sparse warning:

drivers/crypto/mediatek/mtk-platform.c:585:27: warning:
 symbol 'of_crypto_id' was not declared. Should it be static?

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/crypto/mediatek/mtk-platform.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/mediatek/mtk-platform.c b/drivers/crypto/mediatek/mtk-platform.c
index 286296f..a9c713d 100644
--- a/drivers/crypto/mediatek/mtk-platform.c
+++ b/drivers/crypto/mediatek/mtk-platform.c
@@ -582,7 +582,7 @@ static int mtk_crypto_remove(struct platform_device *pdev)
 	return 0;
 }
 
-const struct of_device_id of_crypto_id[] = {
+static const struct of_device_id of_crypto_id[] = {
 	{ .compatible = "mediatek,eip97-crypto" },
 	{},
 };

^ permalink raw reply related

* Re: [PATCH 1/2] mmc: mediatek: Use data tune for CMD line tune
From: Ulf Hansson @ 2017-01-12 10:39 UTC (permalink / raw)
  To: Yong Mao
  Cc: Mark Rutland, devicetree@vger.kernel.org, Nicolas Boichat,
	srv_heupstream, Javier Martinez Canillas, Catalin Marinas,
	Will Deacon, Douglas Anderson, linux-kernel@vger.kernel.org,
	Chunfeng Yun, Rob Herring, linux-mediatek,
	linux-arm-kernel@lists.infradead.org, Philipp Zabel,
	Greg Kroah-Hartman, Matthias Brugger, linux-mmc@vger.kernel.org,
	Eddie Huang, Chaotian Jing
In-Reply-To: <1484215490-7494-2-git-send-email-yong.mao@mediatek.com>

On 12 January 2017 at 11:04, Yong Mao <yong.mao@mediatek.com> wrote:
> From: yong mao <yong.mao@mediatek.com>
>
> CMD response CRC error may cause cannot boot up
> Change to use data tune for CMD line
> Separate cmd internal delay for HS200/HS400 mode

Please try to work a little bit on improving the change log. Moreover
as this is a fix for a regression (it seems like so?), please try to
make that clear.

>
> Signed-off-by: Yong Mao <yong.mao@mediatek.com>
> Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8173-evb.dts |    3 +

Changes to the DTS files should be a separate change. Please split it
into its own patch.

>  drivers/mmc/host/mtk-sd.c                   |  169 +++++++++++++++++++++++----
>  2 files changed, 149 insertions(+), 23 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
> index 0ecaad4..29c3100 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
> @@ -134,6 +134,9 @@
>         bus-width = <8>;
>         max-frequency = <50000000>;
>         cap-mmc-highspeed;
> +       hs200-cmd-int-delay = <26>;
> +       hs400-cmd-int-delay = <14>;
> +       cmd-resp-sel = <0>; /* 0: rising, 1: falling */
>         vmmc-supply = <&mt6397_vemc_3v3_reg>;
>         vqmmc-supply = <&mt6397_vio18_reg>;
>         non-removable;
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index 80ba034..93eb395 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -75,6 +75,7 @@
>  #define MSDC_PATCH_BIT1  0xb4
>  #define MSDC_PAD_TUNE    0xec
>  #define PAD_DS_TUNE      0x188
> +#define PAD_CMD_TUNE     0x18c
>  #define EMMC50_CFG0      0x208
>
>  /*--------------------------------------------------------------------------*/
> @@ -210,12 +211,17 @@
>  #define MSDC_PATCH_BIT_SPCPUSH    (0x1 << 29)  /* RW */
>  #define MSDC_PATCH_BIT_DECRCTMO   (0x1 << 30)  /* RW */
>
> -#define MSDC_PAD_TUNE_DATRRDLY   (0x1f <<  8)  /* RW */
> -#define MSDC_PAD_TUNE_CMDRDLY    (0x1f << 16)  /* RW */
> +#define MSDC_PAD_TUNE_DATWRDLY    (0x1f <<  0)  /* RW */
> +#define MSDC_PAD_TUNE_DATRRDLY    (0x1f <<  8) /* RW */
> +#define MSDC_PAD_TUNE_CMDRDLY     (0x1f << 16)  /* RW */
> +#define MSDC_PAD_TUNE_CMDRRDLY    (0x1f << 22)  /* RW */

Is there a white space change somewhere here? I don't see any changes
to MSDC_PAD_TUNE_DATRRDLY and MSDC_PAD_TUNE_CMDRDLY.

> +#define MSDC_PAD_TUNE_CLKTDLY     (0x1f << 27)  /* RW */
>
> -#define PAD_DS_TUNE_DLY1         (0x1f << 2)   /* RW */
> -#define PAD_DS_TUNE_DLY2         (0x1f << 7)   /* RW */
> -#define PAD_DS_TUNE_DLY3         (0x1f << 12)  /* RW */
> +#define PAD_DS_TUNE_DLY1          (0x1f << 2)   /* RW */
> +#define PAD_DS_TUNE_DLY2          (0x1f << 7)   /* RW */
> +#define PAD_DS_TUNE_DLY3          (0x1f << 12)  /* RW */
> +

Ditto.

> +#define PAD_CMD_TUNE_RX_DLY3      (0x1f << 1)   /* RW */
>
>  #define EMMC50_CFG_PADCMD_LATCHCK (0x1 << 0)   /* RW */
>  #define EMMC50_CFG_CRCSTS_EDGE    (0x1 << 3)   /* RW */
> @@ -236,7 +242,9 @@
>  #define CMD_TIMEOUT         (HZ/10 * 5)        /* 100ms x5 */
>  #define DAT_TIMEOUT         (HZ    * 5)        /* 1000ms x5 */
>
> -#define PAD_DELAY_MAX  32 /* PAD delay cells */
> +#define PAD_DELAY_MAX            32 /* PAD delay cells */

Ditto.

> +#define ENOUGH_MARGIN_MIN        12 /* Enough Margin */
> +#define PREFER_START_POS_MAX     4  /* Prefer start position */
>  /*--------------------------------------------------------------------------*/
>  /* Descriptor Structure                                                     */
>  /*--------------------------------------------------------------------------*/
> @@ -284,12 +292,14 @@ struct msdc_save_para {
>         u32 patch_bit0;
>         u32 patch_bit1;
>         u32 pad_ds_tune;
> +       u32 pad_cmd_tune;
>         u32 emmc50_cfg0;
>  };
>
>  struct msdc_tune_para {
>         u32 iocon;
>         u32 pad_tune;
> +       u32 pad_cmd_tune;
>  };
>
>  struct msdc_delay_phase {
> @@ -331,6 +341,9 @@ struct msdc_host {
>         unsigned char timing;
>         bool vqmmc_enabled;
>         u32 hs400_ds_delay;
> +       u32 hs200_cmd_int_delay; /* cmd internal delay for HS200/SDR104 */
> +       u32 hs400_cmd_int_delay; /* cmd internal delay for HS400 */
> +       u32 hs200_cmd_resp_sel; /* cmd response sample selection */
>         bool hs400_mode;        /* current eMMC will run at hs400 mode */
>         struct msdc_save_para save_para; /* used when gate HCLK */
>         struct msdc_tune_para def_tune_para; /* default tune setting */
> @@ -596,12 +609,21 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
>          */
>         if (host->sclk <= 52000000) {
>                 writel(host->def_tune_para.iocon, host->base + MSDC_IOCON);
> -               writel(host->def_tune_para.pad_tune, host->base + MSDC_PAD_TUNE);
> +               writel(host->def_tune_para.pad_tune,
> +                      host->base + MSDC_PAD_TUNE);

Please don't change code just because you feel like doing it. This is
a completely unessarry change and it makes it harder for me to review.

Can you go thorugh the complete patch and make sure to undo all
similar changes, there are more of them.

>         } else {
> -               writel(host->saved_tune_para.iocon, host->base + MSDC_IOCON);
> -               writel(host->saved_tune_para.pad_tune, host->base + MSDC_PAD_TUNE);
> +               writel(host->saved_tune_para.iocon,
> +                      host->base + MSDC_IOCON);
> +               writel(host->saved_tune_para.pad_tune,
> +                      host->base + MSDC_PAD_TUNE);
> +               writel(host->saved_tune_para.pad_cmd_tune,
> +                      host->base + PAD_CMD_TUNE);
>         }
>
> +       if (timing == MMC_TIMING_MMC_HS400)
> +               sdr_set_field(host->base + PAD_CMD_TUNE,
> +                             MSDC_PAD_TUNE_CMDRRDLY,
> +                             host->hs400_cmd_int_delay);
>         dev_dbg(host->dev, "sclk: %d, timing: %d\n", host->sclk, timing);
>  }
>
> @@ -1302,7 +1324,8 @@ static struct msdc_delay_phase get_best_delay(struct msdc_host *host, u32 delay)
>                         len_final = len;
>                 }
>                 start += len ? len : 1;
> -               if (len >= 8 && start_final < 4)
> +               if (len >= ENOUGH_MARGIN_MIN &&
> +                   start_final < PREFER_START_POS_MAX)
>                         break;
>         }
>
> @@ -1325,48 +1348,128 @@ static int msdc_tune_response(struct mmc_host *mmc, u32 opcode)
>         struct msdc_host *host = mmc_priv(mmc);
>         u32 rise_delay = 0, fall_delay = 0;
>         struct msdc_delay_phase final_rise_delay, final_fall_delay = { 0,};
> +       struct msdc_delay_phase internal_delay_phase;
>         u8 final_delay, final_maxlen;
> +       u32 internal_delay = 0;
>         int cmd_err;
> -       int i;
> +       int i, j;
>
> +       if (mmc->ios.timing == MMC_TIMING_MMC_HS200 ||
> +           mmc->ios.timing == MMC_TIMING_UHS_SDR104)
> +               sdr_set_field(host->base + MSDC_PAD_TUNE,
> +                             MSDC_PAD_TUNE_CMDRRDLY,
> +                             host->hs200_cmd_int_delay);
>         sdr_clr_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
>         for (i = 0 ; i < PAD_DELAY_MAX; i++) {
>                 sdr_set_field(host->base + MSDC_PAD_TUNE,
>                               MSDC_PAD_TUNE_CMDRDLY, i);
> -               mmc_send_tuning(mmc, opcode, &cmd_err);
> -               if (!cmd_err)
> -                       rise_delay |= (1 << i);
> +               for (j = 0; j < 3; j++) {

Any reason to why looping three times makes sense? Maybe add a comment?

> +                       mmc_send_tuning(mmc, opcode, &cmd_err);
> +                       if (!cmd_err) {
> +                               rise_delay |= (1 << i);
> +                       } else {
> +                               rise_delay &= ~(1 << i);
> +                               break;
> +                       }
> +               }
>         }
>         final_rise_delay = get_best_delay(host, rise_delay);
>         /* if rising edge has enough margin, then do not scan falling edge */
> -       if (final_rise_delay.maxlen >= 10 ||
> -           (final_rise_delay.start == 0 && final_rise_delay.maxlen >= 4))
> +       if (final_rise_delay.maxlen >= ENOUGH_MARGIN_MIN &&
> +           final_rise_delay.start < PREFER_START_POS_MAX)

This looks like clean-ups, as you are converting from magic numbers to
defines. Please make this kind of changes separately.

>                 goto skip_fall;
>
>         sdr_set_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
>         for (i = 0; i < PAD_DELAY_MAX; i++) {
>                 sdr_set_field(host->base + MSDC_PAD_TUNE,
>                               MSDC_PAD_TUNE_CMDRDLY, i);
> -               mmc_send_tuning(mmc, opcode, &cmd_err);
> -               if (!cmd_err)
> -                       fall_delay |= (1 << i);
> +               for (j = 0; j < 3; j++) {

3?

> +                       mmc_send_tuning(mmc, opcode, &cmd_err);
> +                       if (!cmd_err) {
> +                               fall_delay |= (1 << i);
> +                       } else {
> +                               fall_delay &= ~(1 << i);
> +                               break;
> +                       };
> +               }
>         }
>         final_fall_delay = get_best_delay(host, fall_delay);
>
>  skip_fall:
>         final_maxlen = max(final_rise_delay.maxlen, final_fall_delay.maxlen);
> +       if (final_fall_delay.maxlen >= ENOUGH_MARGIN_MIN &&
> +           final_fall_delay.start < PREFER_START_POS_MAX)
> +               final_maxlen = final_fall_delay.maxlen;
>         if (final_maxlen == final_rise_delay.maxlen) {
>                 sdr_clr_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
> -               sdr_set_field(host->base + MSDC_PAD_TUNE, MSDC_PAD_TUNE_CMDRDLY,
> +               sdr_set_field(host->base + MSDC_PAD_TUNE,
> +                             MSDC_PAD_TUNE_CMDRDLY,
>                               final_rise_delay.final_phase);
>                 final_delay = final_rise_delay.final_phase;
>         } else {
>                 sdr_set_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
> -               sdr_set_field(host->base + MSDC_PAD_TUNE, MSDC_PAD_TUNE_CMDRDLY,
> +               sdr_set_field(host->base + MSDC_PAD_TUNE,
> +                             MSDC_PAD_TUNE_CMDRDLY,
>                               final_fall_delay.final_phase);
>                 final_delay = final_fall_delay.final_phase;
>         }
> +       if (host->hs200_cmd_int_delay)
> +               goto skip_internal;
>
> +       for (i = 0; i < PAD_DELAY_MAX; i++) {
> +               sdr_set_field(host->base + MSDC_PAD_TUNE,
> +                             MSDC_PAD_TUNE_CMDRRDLY, i);
> +               mmc_send_tuning(mmc, opcode, &cmd_err);
> +               if (!cmd_err)
> +                       internal_delay |= (1 << i);
> +       }
> +       dev_info(host->dev, "Final internal delay: 0x%x\n", internal_delay);

I don't think dev_info() is what you want, right? Perhaps dev_dbg(),
anything at all.

> +       internal_delay_phase = get_best_delay(host, internal_delay);
> +       sdr_set_field(host->base + MSDC_PAD_TUNE, MSDC_PAD_TUNE_CMDRRDLY,
> +                     internal_delay_phase.final_phase);
> +skip_internal:
> +       dev_info(host->dev, "Final cmd pad delay: %x\n", final_delay);

I don't think dev_info() is what you want, right? Perhaps dev_dbg(),
anything at all.

> +       return final_delay == 0xff ? -EIO : 0;
> +}
> +
> +static int hs400_tune_response(struct mmc_host *mmc, u32 opcode)
> +{
> +       struct msdc_host *host = mmc_priv(mmc);
> +       u32 cmd_delay = 0;
> +       struct msdc_delay_phase final_cmd_delay = { 0,};
> +       u8 final_delay;
> +       int cmd_err;
> +       int i, j;
> +
> +       /* select EMMC50 PAD CMD tune */
> +       sdr_set_bits(host->base + PAD_CMD_TUNE, BIT(0));
> +
> +       if (mmc->ios.timing == MMC_TIMING_MMC_HS200 ||
> +           mmc->ios.timing == MMC_TIMING_UHS_SDR104)
> +               sdr_set_field(host->base + MSDC_PAD_TUNE,
> +                             MSDC_PAD_TUNE_CMDRRDLY,
> +                             host->hs200_cmd_int_delay);
> +       sdr_set_field(host->base + MSDC_IOCON, MSDC_IOCON_RSPL,
> +                     host->hs200_cmd_resp_sel);
> +       for (i = 0 ; i < PAD_DELAY_MAX; i++) {
> +               sdr_set_field(host->base + PAD_CMD_TUNE,
> +                             PAD_CMD_TUNE_RX_DLY3, i);
> +               for (j = 0; j < 3; j++) {

3?

> +                       mmc_send_tuning(mmc, opcode, &cmd_err);
> +                       if (!cmd_err) {
> +                               cmd_delay |= (1 << i);
> +                       } else {
> +                               cmd_delay &= ~(1 << i);
> +                               break;
> +                       }
> +               }
> +       }
> +       final_cmd_delay = get_best_delay(host, cmd_delay);
> +       sdr_set_field(host->base + PAD_CMD_TUNE, PAD_CMD_TUNE_RX_DLY3,
> +                     final_cmd_delay.final_phase);
> +       final_delay = final_cmd_delay.final_phase;
> +
> +       dev_info(host->dev, "Final cmd pad delay: %x\n", final_delay);

dev_info() -> dev_dbg() or remove it.

>         return final_delay == 0xff ? -EIO : 0;
>  }
>
> @@ -1389,7 +1492,7 @@ static int msdc_tune_data(struct mmc_host *mmc, u32 opcode)
>         }
>         final_rise_delay = get_best_delay(host, rise_delay);
>         /* if rising edge has enough margin, then do not scan falling edge */
> -       if (final_rise_delay.maxlen >= 10 ||
> +       if (final_rise_delay.maxlen >= ENOUGH_MARGIN_MIN ||

Clean up. Move to separate change.

>             (final_rise_delay.start == 0 && final_rise_delay.maxlen >= 4))
>                 goto skip_fall;
>
> @@ -1422,6 +1525,7 @@ static int msdc_tune_data(struct mmc_host *mmc, u32 opcode)
>                 final_delay = final_fall_delay.final_phase;
>         }
>
> +       dev_info(host->dev, "Final data pad delay: %x\n", final_delay);

dev_info() -> dev_dbg() or remove it.

>         return final_delay == 0xff ? -EIO : 0;
>  }
>
> @@ -1430,10 +1534,13 @@ static int msdc_execute_tuning(struct mmc_host *mmc, u32 opcode)
>         struct msdc_host *host = mmc_priv(mmc);
>         int ret;
>
> +       if (host->hs400_mode)
> +               ret = hs400_tune_response(mmc, opcode);
> +       else
>         ret = msdc_tune_response(mmc, opcode);

Because of the new else clause, seems like above needs an intendation.

>         if (ret == -EIO) {
>                 dev_err(host->dev, "Tune response fail!\n");
> -               return ret;
> +               goto out;

Not needed, remove the label and this change.

>         }
>         if (host->hs400_mode == false) {
>                 ret = msdc_tune_data(mmc, opcode);
> @@ -1443,6 +1550,8 @@ static int msdc_execute_tuning(struct mmc_host *mmc, u32 opcode)
>
>         host->saved_tune_para.iocon = readl(host->base + MSDC_IOCON);
>         host->saved_tune_para.pad_tune = readl(host->base + MSDC_PAD_TUNE);
> +       host->saved_tune_para.pad_cmd_tune = readl(host->base + PAD_CMD_TUNE);
> +out:
>         return ret;
>  }
>
> @@ -1553,6 +1662,18 @@ static int msdc_drv_probe(struct platform_device *pdev)
>                 dev_dbg(&pdev->dev, "hs400-ds-delay: %x\n",
>                         host->hs400_ds_delay);
>
> +       if (!of_property_read_u32(pdev->dev.of_node, "hs200-cmd-int-delay",
> +                                 &host->hs200_cmd_int_delay))
> +               dev_dbg(&pdev->dev, "host->hs200-cmd-int-delay: %x\n",
> +                       host->hs200_cmd_int_delay);
> +       if (!of_property_read_u32(pdev->dev.of_node, "hs400-cmd-int-delay",
> +                                 &host->hs400_cmd_int_delay))
> +               dev_dbg(&pdev->dev, "host->hs400-cmd-int-delay: %x\n",
> +                       host->hs400_cmd_int_delay);
> +       if (!of_property_read_u32(pdev->dev.of_node, "cmd-resp-sel",
> +                                 &host->hs200_cmd_resp_sel))
> +               dev_dbg(&pdev->dev, "host->hs200_cmd-resp-sel: %x\n",
> +                       host->hs200_cmd_resp_sel);

I suggest you take the oppotunity to move the MTK DTS parsing into its
own function, include the existing parsing of the "hs400-ds-delay".
This improve the readablitity of the code.

>         host->dev = &pdev->dev;
>         host->mmc = mmc;
>         host->src_clk_freq = clk_get_rate(host->src_clk);
> @@ -1663,6 +1784,7 @@ static void msdc_save_reg(struct msdc_host *host)
>         host->save_para.patch_bit0 = readl(host->base + MSDC_PATCH_BIT);
>         host->save_para.patch_bit1 = readl(host->base + MSDC_PATCH_BIT1);
>         host->save_para.pad_ds_tune = readl(host->base + PAD_DS_TUNE);
> +       host->save_para.pad_cmd_tune = readl(host->base + PAD_CMD_TUNE);
>         host->save_para.emmc50_cfg0 = readl(host->base + EMMC50_CFG0);
>  }
>
> @@ -1675,6 +1797,7 @@ static void msdc_restore_reg(struct msdc_host *host)
>         writel(host->save_para.patch_bit0, host->base + MSDC_PATCH_BIT);
>         writel(host->save_para.patch_bit1, host->base + MSDC_PATCH_BIT1);
>         writel(host->save_para.pad_ds_tune, host->base + PAD_DS_TUNE);
> +       writel(host->save_para.pad_cmd_tune, host->base + PAD_CMD_TUNE);
>         writel(host->save_para.emmc50_cfg0, host->base + EMMC50_CFG0);
>  }
>
> --
> 1.7.9.5
>

Kind regards
Uffe

^ permalink raw reply

* Re: [PATCH 2/2] mmc: dt-bindings: update Mediatek MMC bindings
From: Ulf Hansson @ 2017-01-12 10:15 UTC (permalink / raw)
  To: Yong Mao
  Cc: Rob Herring, Mark Rutland, Chaotian Jing, Eddie Huang,
	Chunfeng Yun, linux-mmc@vger.kernel.org,
	devicetree@vger.kernel.org, linux-mediatek
In-Reply-To: <1484215490-7494-3-git-send-email-yong.mao@mediatek.com>

-trimmed cc list (future wise, I suggest you be more careful whom to
keep on cc/to)

On 12 January 2017 at 11:04, Yong Mao <yong.mao@mediatek.com> wrote:
> From: yong mao <yong.mao@mediatek.com>
>
> Add description for hs200-cmd-int-delay
> Add description for hs400-cmd-int-delay
> Add description for cmd-resp-sel
>
> Signed-off-by: Yong Mao <yong.mao@mediatek.com>

The changes to DTS bindings should come prior the actual code change
using the bindings. Please make this patch 1 instead.

Otherwise this looks good to me.

Kind regards
Uffe

> ---
>  Documentation/devicetree/bindings/mmc/mtk-sd.txt |    6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> index 0120c7f..2dbb3b0 100644
> --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> @@ -21,6 +21,9 @@ Optional properties:
>  - assigned-clocks: PLL of the source clock
>  - assigned-clock-parents: parent of source clock, used for HS400 mode to get 400Mhz source clock
>  - hs400-ds-delay: HS400 DS delay setting
> +- hs200-cmd-int-delay: HS200 command internal delay setting
> +- hs400-cmd-int-delay: HS400 command internal delay setting
> +- cmd-resp-sel: command response sample selection
>
>  Examples:
>  mmc0: mmc@11230000 {
> @@ -38,4 +41,7 @@ mmc0: mmc@11230000 {
>         assigned-clocks = <&topckgen CLK_TOP_MSDC50_0_SEL>;
>         assigned-clock-parents = <&topckgen CLK_TOP_MSDCPLL_D2>;
>         hs400-ds-delay = <0x14015>;
> +       hs200-cmd-int-delay = <26>;
> +       hs400-cmd-int-delay = <14>;
> +       cmd-resp-sel = <0>; /* 0: rising, 1: falling */
>  };
> --
> 1.7.9.5
>

^ permalink raw reply

* [PATCH 2/2] mmc: dt-bindings: update Mediatek MMC bindings
From: Yong Mao @ 2017-01-12 10:04 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Nicolas Boichat,
	srv_heupstream-NuS5LvNUpcJWk0Htik3J/w, Javier Martinez Canillas,
	Catalin Marinas, Will Deacon, Douglas Anderson, yong mao,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Chunfeng Yun, Rob Herring,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Philipp Zabel,
	Greg Kroah-Hartman, Matthias Brugger,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA, Eddie Huang, Chaotian Jing
In-Reply-To: <1484215490-7494-1-git-send-email-yong.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

From: yong mao <yong.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

Add description for hs200-cmd-int-delay
Add description for hs400-cmd-int-delay
Add description for cmd-resp-sel

Signed-off-by: Yong Mao <yong.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 Documentation/devicetree/bindings/mmc/mtk-sd.txt |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
index 0120c7f..2dbb3b0 100644
--- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
+++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
@@ -21,6 +21,9 @@ Optional properties:
 - assigned-clocks: PLL of the source clock
 - assigned-clock-parents: parent of source clock, used for HS400 mode to get 400Mhz source clock
 - hs400-ds-delay: HS400 DS delay setting
+- hs200-cmd-int-delay: HS200 command internal delay setting
+- hs400-cmd-int-delay: HS400 command internal delay setting
+- cmd-resp-sel: command response sample selection
 
 Examples:
 mmc0: mmc@11230000 {
@@ -38,4 +41,7 @@ mmc0: mmc@11230000 {
 	assigned-clocks = <&topckgen CLK_TOP_MSDC50_0_SEL>;
 	assigned-clock-parents = <&topckgen CLK_TOP_MSDCPLL_D2>;
 	hs400-ds-delay = <0x14015>;
+	hs200-cmd-int-delay = <26>;
+	hs400-cmd-int-delay = <14>;
+	cmd-resp-sel = <0>; /* 0: rising, 1: falling */
 };
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 1/2] mmc: mediatek: Use data tune for CMD line tune
From: Yong Mao @ 2017-01-12 10:04 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Mark Rutland, devicetree, Nicolas Boichat, srv_heupstream,
	Javier Martinez Canillas, Catalin Marinas, Will Deacon,
	Douglas Anderson, yong mao, linux-kernel, Chunfeng Yun,
	Rob Herring, linux-mediatek, linux-arm-kernel, Philipp Zabel,
	Greg Kroah-Hartman, Matthias Brugger, linux-mmc, Eddie Huang,
	Chaotian Jing
In-Reply-To: <1484215490-7494-1-git-send-email-yong.mao@mediatek.com>

From: yong mao <yong.mao@mediatek.com>

CMD response CRC error may cause cannot boot up
Change to use data tune for CMD line
Separate cmd internal delay for HS200/HS400 mode

Signed-off-by: Yong Mao <yong.mao@mediatek.com>
Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8173-evb.dts |    3 +
 drivers/mmc/host/mtk-sd.c                   |  169 +++++++++++++++++++++++----
 2 files changed, 149 insertions(+), 23 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
index 0ecaad4..29c3100 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
@@ -134,6 +134,9 @@
 	bus-width = <8>;
 	max-frequency = <50000000>;
 	cap-mmc-highspeed;
+	hs200-cmd-int-delay = <26>;
+	hs400-cmd-int-delay = <14>;
+	cmd-resp-sel = <0>; /* 0: rising, 1: falling */
 	vmmc-supply = <&mt6397_vemc_3v3_reg>;
 	vqmmc-supply = <&mt6397_vio18_reg>;
 	non-removable;
diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 80ba034..93eb395 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -75,6 +75,7 @@
 #define MSDC_PATCH_BIT1  0xb4
 #define MSDC_PAD_TUNE    0xec
 #define PAD_DS_TUNE      0x188
+#define PAD_CMD_TUNE     0x18c
 #define EMMC50_CFG0      0x208
 
 /*--------------------------------------------------------------------------*/
@@ -210,12 +211,17 @@
 #define MSDC_PATCH_BIT_SPCPUSH    (0x1 << 29)	/* RW */
 #define MSDC_PATCH_BIT_DECRCTMO   (0x1 << 30)	/* RW */
 
-#define MSDC_PAD_TUNE_DATRRDLY	  (0x1f <<  8)	/* RW */
-#define MSDC_PAD_TUNE_CMDRDLY	  (0x1f << 16)  /* RW */
+#define MSDC_PAD_TUNE_DATWRDLY    (0x1f <<  0)  /* RW */
+#define MSDC_PAD_TUNE_DATRRDLY    (0x1f <<  8)	/* RW */
+#define MSDC_PAD_TUNE_CMDRDLY     (0x1f << 16)  /* RW */
+#define MSDC_PAD_TUNE_CMDRRDLY    (0x1f << 22)  /* RW */
+#define MSDC_PAD_TUNE_CLKTDLY     (0x1f << 27)  /* RW */
 
-#define PAD_DS_TUNE_DLY1	  (0x1f << 2)   /* RW */
-#define PAD_DS_TUNE_DLY2	  (0x1f << 7)   /* RW */
-#define PAD_DS_TUNE_DLY3	  (0x1f << 12)  /* RW */
+#define PAD_DS_TUNE_DLY1          (0x1f << 2)   /* RW */
+#define PAD_DS_TUNE_DLY2          (0x1f << 7)   /* RW */
+#define PAD_DS_TUNE_DLY3          (0x1f << 12)  /* RW */
+
+#define PAD_CMD_TUNE_RX_DLY3      (0x1f << 1)   /* RW */
 
 #define EMMC50_CFG_PADCMD_LATCHCK (0x1 << 0)   /* RW */
 #define EMMC50_CFG_CRCSTS_EDGE    (0x1 << 3)   /* RW */
@@ -236,7 +242,9 @@
 #define CMD_TIMEOUT         (HZ/10 * 5)	/* 100ms x5 */
 #define DAT_TIMEOUT         (HZ    * 5)	/* 1000ms x5 */
 
-#define PAD_DELAY_MAX	32 /* PAD delay cells */
+#define PAD_DELAY_MAX            32 /* PAD delay cells */
+#define ENOUGH_MARGIN_MIN        12 /* Enough Margin */
+#define PREFER_START_POS_MAX     4  /* Prefer start position */
 /*--------------------------------------------------------------------------*/
 /* Descriptor Structure                                                     */
 /*--------------------------------------------------------------------------*/
@@ -284,12 +292,14 @@ struct msdc_save_para {
 	u32 patch_bit0;
 	u32 patch_bit1;
 	u32 pad_ds_tune;
+	u32 pad_cmd_tune;
 	u32 emmc50_cfg0;
 };
 
 struct msdc_tune_para {
 	u32 iocon;
 	u32 pad_tune;
+	u32 pad_cmd_tune;
 };
 
 struct msdc_delay_phase {
@@ -331,6 +341,9 @@ struct msdc_host {
 	unsigned char timing;
 	bool vqmmc_enabled;
 	u32 hs400_ds_delay;
+	u32 hs200_cmd_int_delay; /* cmd internal delay for HS200/SDR104 */
+	u32 hs400_cmd_int_delay; /* cmd internal delay for HS400 */
+	u32 hs200_cmd_resp_sel; /* cmd response sample selection */
 	bool hs400_mode;	/* current eMMC will run at hs400 mode */
 	struct msdc_save_para save_para; /* used when gate HCLK */
 	struct msdc_tune_para def_tune_para; /* default tune setting */
@@ -596,12 +609,21 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
 	 */
 	if (host->sclk <= 52000000) {
 		writel(host->def_tune_para.iocon, host->base + MSDC_IOCON);
-		writel(host->def_tune_para.pad_tune, host->base + MSDC_PAD_TUNE);
+		writel(host->def_tune_para.pad_tune,
+		       host->base + MSDC_PAD_TUNE);
 	} else {
-		writel(host->saved_tune_para.iocon, host->base + MSDC_IOCON);
-		writel(host->saved_tune_para.pad_tune, host->base + MSDC_PAD_TUNE);
+		writel(host->saved_tune_para.iocon,
+		       host->base + MSDC_IOCON);
+		writel(host->saved_tune_para.pad_tune,
+		       host->base + MSDC_PAD_TUNE);
+		writel(host->saved_tune_para.pad_cmd_tune,
+		       host->base + PAD_CMD_TUNE);
 	}
 
+	if (timing == MMC_TIMING_MMC_HS400)
+		sdr_set_field(host->base + PAD_CMD_TUNE,
+			      MSDC_PAD_TUNE_CMDRRDLY,
+			      host->hs400_cmd_int_delay);
 	dev_dbg(host->dev, "sclk: %d, timing: %d\n", host->sclk, timing);
 }
 
@@ -1302,7 +1324,8 @@ static struct msdc_delay_phase get_best_delay(struct msdc_host *host, u32 delay)
 			len_final = len;
 		}
 		start += len ? len : 1;
-		if (len >= 8 && start_final < 4)
+		if (len >= ENOUGH_MARGIN_MIN &&
+		    start_final < PREFER_START_POS_MAX)
 			break;
 	}
 
@@ -1325,48 +1348,128 @@ static int msdc_tune_response(struct mmc_host *mmc, u32 opcode)
 	struct msdc_host *host = mmc_priv(mmc);
 	u32 rise_delay = 0, fall_delay = 0;
 	struct msdc_delay_phase final_rise_delay, final_fall_delay = { 0,};
+	struct msdc_delay_phase internal_delay_phase;
 	u8 final_delay, final_maxlen;
+	u32 internal_delay = 0;
 	int cmd_err;
-	int i;
+	int i, j;
 
+	if (mmc->ios.timing == MMC_TIMING_MMC_HS200 ||
+	    mmc->ios.timing == MMC_TIMING_UHS_SDR104)
+		sdr_set_field(host->base + MSDC_PAD_TUNE,
+			      MSDC_PAD_TUNE_CMDRRDLY,
+			      host->hs200_cmd_int_delay);
 	sdr_clr_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
 	for (i = 0 ; i < PAD_DELAY_MAX; i++) {
 		sdr_set_field(host->base + MSDC_PAD_TUNE,
 			      MSDC_PAD_TUNE_CMDRDLY, i);
-		mmc_send_tuning(mmc, opcode, &cmd_err);
-		if (!cmd_err)
-			rise_delay |= (1 << i);
+		for (j = 0; j < 3; j++) {
+			mmc_send_tuning(mmc, opcode, &cmd_err);
+			if (!cmd_err) {
+				rise_delay |= (1 << i);
+			} else {
+				rise_delay &= ~(1 << i);
+				break;
+			}
+		}
 	}
 	final_rise_delay = get_best_delay(host, rise_delay);
 	/* if rising edge has enough margin, then do not scan falling edge */
-	if (final_rise_delay.maxlen >= 10 ||
-	    (final_rise_delay.start == 0 && final_rise_delay.maxlen >= 4))
+	if (final_rise_delay.maxlen >= ENOUGH_MARGIN_MIN &&
+	    final_rise_delay.start < PREFER_START_POS_MAX)
 		goto skip_fall;
 
 	sdr_set_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
 	for (i = 0; i < PAD_DELAY_MAX; i++) {
 		sdr_set_field(host->base + MSDC_PAD_TUNE,
 			      MSDC_PAD_TUNE_CMDRDLY, i);
-		mmc_send_tuning(mmc, opcode, &cmd_err);
-		if (!cmd_err)
-			fall_delay |= (1 << i);
+		for (j = 0; j < 3; j++) {
+			mmc_send_tuning(mmc, opcode, &cmd_err);
+			if (!cmd_err) {
+				fall_delay |= (1 << i);
+			} else {
+				fall_delay &= ~(1 << i);
+				break;
+			};
+		}
 	}
 	final_fall_delay = get_best_delay(host, fall_delay);
 
 skip_fall:
 	final_maxlen = max(final_rise_delay.maxlen, final_fall_delay.maxlen);
+	if (final_fall_delay.maxlen >= ENOUGH_MARGIN_MIN &&
+	    final_fall_delay.start < PREFER_START_POS_MAX)
+		final_maxlen = final_fall_delay.maxlen;
 	if (final_maxlen == final_rise_delay.maxlen) {
 		sdr_clr_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
-		sdr_set_field(host->base + MSDC_PAD_TUNE, MSDC_PAD_TUNE_CMDRDLY,
+		sdr_set_field(host->base + MSDC_PAD_TUNE,
+			      MSDC_PAD_TUNE_CMDRDLY,
 			      final_rise_delay.final_phase);
 		final_delay = final_rise_delay.final_phase;
 	} else {
 		sdr_set_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
-		sdr_set_field(host->base + MSDC_PAD_TUNE, MSDC_PAD_TUNE_CMDRDLY,
+		sdr_set_field(host->base + MSDC_PAD_TUNE,
+			      MSDC_PAD_TUNE_CMDRDLY,
 			      final_fall_delay.final_phase);
 		final_delay = final_fall_delay.final_phase;
 	}
+	if (host->hs200_cmd_int_delay)
+		goto skip_internal;
 
+	for (i = 0; i < PAD_DELAY_MAX; i++) {
+		sdr_set_field(host->base + MSDC_PAD_TUNE,
+			      MSDC_PAD_TUNE_CMDRRDLY, i);
+		mmc_send_tuning(mmc, opcode, &cmd_err);
+		if (!cmd_err)
+			internal_delay |= (1 << i);
+	}
+	dev_info(host->dev, "Final internal delay: 0x%x\n", internal_delay);
+	internal_delay_phase = get_best_delay(host, internal_delay);
+	sdr_set_field(host->base + MSDC_PAD_TUNE, MSDC_PAD_TUNE_CMDRRDLY,
+		      internal_delay_phase.final_phase);
+skip_internal:
+	dev_info(host->dev, "Final cmd pad delay: %x\n", final_delay);
+	return final_delay == 0xff ? -EIO : 0;
+}
+
+static int hs400_tune_response(struct mmc_host *mmc, u32 opcode)
+{
+	struct msdc_host *host = mmc_priv(mmc);
+	u32 cmd_delay = 0;
+	struct msdc_delay_phase final_cmd_delay = { 0,};
+	u8 final_delay;
+	int cmd_err;
+	int i, j;
+
+	/* select EMMC50 PAD CMD tune */
+	sdr_set_bits(host->base + PAD_CMD_TUNE, BIT(0));
+
+	if (mmc->ios.timing == MMC_TIMING_MMC_HS200 ||
+	    mmc->ios.timing == MMC_TIMING_UHS_SDR104)
+		sdr_set_field(host->base + MSDC_PAD_TUNE,
+			      MSDC_PAD_TUNE_CMDRRDLY,
+			      host->hs200_cmd_int_delay);
+	sdr_set_field(host->base + MSDC_IOCON, MSDC_IOCON_RSPL,
+		      host->hs200_cmd_resp_sel);
+	for (i = 0 ; i < PAD_DELAY_MAX; i++) {
+		sdr_set_field(host->base + PAD_CMD_TUNE,
+			      PAD_CMD_TUNE_RX_DLY3, i);
+		for (j = 0; j < 3; j++) {
+			mmc_send_tuning(mmc, opcode, &cmd_err);
+			if (!cmd_err) {
+				cmd_delay |= (1 << i);
+			} else {
+				cmd_delay &= ~(1 << i);
+				break;
+			}
+		}
+	}
+	final_cmd_delay = get_best_delay(host, cmd_delay);
+	sdr_set_field(host->base + PAD_CMD_TUNE, PAD_CMD_TUNE_RX_DLY3,
+		      final_cmd_delay.final_phase);
+	final_delay = final_cmd_delay.final_phase;
+
+	dev_info(host->dev, "Final cmd pad delay: %x\n", final_delay);
 	return final_delay == 0xff ? -EIO : 0;
 }
 
@@ -1389,7 +1492,7 @@ static int msdc_tune_data(struct mmc_host *mmc, u32 opcode)
 	}
 	final_rise_delay = get_best_delay(host, rise_delay);
 	/* if rising edge has enough margin, then do not scan falling edge */
-	if (final_rise_delay.maxlen >= 10 ||
+	if (final_rise_delay.maxlen >= ENOUGH_MARGIN_MIN ||
 	    (final_rise_delay.start == 0 && final_rise_delay.maxlen >= 4))
 		goto skip_fall;
 
@@ -1422,6 +1525,7 @@ static int msdc_tune_data(struct mmc_host *mmc, u32 opcode)
 		final_delay = final_fall_delay.final_phase;
 	}
 
+	dev_info(host->dev, "Final data pad delay: %x\n", final_delay);
 	return final_delay == 0xff ? -EIO : 0;
 }
 
@@ -1430,10 +1534,13 @@ static int msdc_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	struct msdc_host *host = mmc_priv(mmc);
 	int ret;
 
+	if (host->hs400_mode)
+		ret = hs400_tune_response(mmc, opcode);
+	else
 	ret = msdc_tune_response(mmc, opcode);
 	if (ret == -EIO) {
 		dev_err(host->dev, "Tune response fail!\n");
-		return ret;
+		goto out;
 	}
 	if (host->hs400_mode == false) {
 		ret = msdc_tune_data(mmc, opcode);
@@ -1443,6 +1550,8 @@ static int msdc_execute_tuning(struct mmc_host *mmc, u32 opcode)
 
 	host->saved_tune_para.iocon = readl(host->base + MSDC_IOCON);
 	host->saved_tune_para.pad_tune = readl(host->base + MSDC_PAD_TUNE);
+	host->saved_tune_para.pad_cmd_tune = readl(host->base + PAD_CMD_TUNE);
+out:
 	return ret;
 }
 
@@ -1553,6 +1662,18 @@ static int msdc_drv_probe(struct platform_device *pdev)
 		dev_dbg(&pdev->dev, "hs400-ds-delay: %x\n",
 			host->hs400_ds_delay);
 
+	if (!of_property_read_u32(pdev->dev.of_node, "hs200-cmd-int-delay",
+				  &host->hs200_cmd_int_delay))
+		dev_dbg(&pdev->dev, "host->hs200-cmd-int-delay: %x\n",
+			host->hs200_cmd_int_delay);
+	if (!of_property_read_u32(pdev->dev.of_node, "hs400-cmd-int-delay",
+				  &host->hs400_cmd_int_delay))
+		dev_dbg(&pdev->dev, "host->hs400-cmd-int-delay: %x\n",
+			host->hs400_cmd_int_delay);
+	if (!of_property_read_u32(pdev->dev.of_node, "cmd-resp-sel",
+				  &host->hs200_cmd_resp_sel))
+		dev_dbg(&pdev->dev, "host->hs200_cmd-resp-sel: %x\n",
+			host->hs200_cmd_resp_sel);
 	host->dev = &pdev->dev;
 	host->mmc = mmc;
 	host->src_clk_freq = clk_get_rate(host->src_clk);
@@ -1663,6 +1784,7 @@ static void msdc_save_reg(struct msdc_host *host)
 	host->save_para.patch_bit0 = readl(host->base + MSDC_PATCH_BIT);
 	host->save_para.patch_bit1 = readl(host->base + MSDC_PATCH_BIT1);
 	host->save_para.pad_ds_tune = readl(host->base + PAD_DS_TUNE);
+	host->save_para.pad_cmd_tune = readl(host->base + PAD_CMD_TUNE);
 	host->save_para.emmc50_cfg0 = readl(host->base + EMMC50_CFG0);
 }
 
@@ -1675,6 +1797,7 @@ static void msdc_restore_reg(struct msdc_host *host)
 	writel(host->save_para.patch_bit0, host->base + MSDC_PATCH_BIT);
 	writel(host->save_para.patch_bit1, host->base + MSDC_PATCH_BIT1);
 	writel(host->save_para.pad_ds_tune, host->base + PAD_DS_TUNE);
+	writel(host->save_para.pad_cmd_tune, host->base + PAD_CMD_TUNE);
 	writel(host->save_para.emmc50_cfg0, host->base + EMMC50_CFG0);
 }
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 0/2] Use data tune for CMD line tune
From: Yong Mao @ 2017-01-12 10:04 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Mark Rutland, devicetree, Nicolas Boichat, srv_heupstream,
	Javier Martinez Canillas, Catalin Marinas, Will Deacon,
	Douglas Anderson, yong mao, linux-kernel, Chunfeng Yun,
	Rob Herring, linux-mediatek, linux-arm-kernel, Philipp Zabel,
	Greg Kroah-Hartman, Matthias Brugger, linux-mmc, Eddie Huang,
	Chaotian Jing

CMD response CRC error may cause cannot boot up
Change to use data tune for CMD line
Separate cmd internal delay for HS200/HS400 mode

yong mao (2):
  mmc: mediatek: Use data tune for CMD line tune
  mmc: dt-bindings: update Mediatek MMC bindings

 Documentation/devicetree/bindings/mmc/mtk-sd.txt |   6 +
 arch/arm64/boot/dts/mediatek/mt8173-evb.dts      |   3 +
 drivers/mmc/host/mtk-sd.c                        | 169 ++++++++++++++++++++---
 3 files changed, 155 insertions(+), 23 deletions(-)

-- 
1.8.1.1

^ permalink raw reply

* Re: [PATCH] clk: mediatek: Fix MT2701 dependencies
From: Jean Delvare @ 2017-01-12  8:40 UTC (permalink / raw)
  To: James Liao
  Cc: Erin Lo, Stephen Boyd, linux-clk-u79uwXL29TY76Z2rM5mHXA,
	Michael Turquette,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Shunli Wang,
	Matthias Brugger, Andreas Färber
In-Reply-To: <1484192392.26901.12.camel@mtksdaap41>

Hi James,

On Thu, 12 Jan 2017 11:39:52 +0800, James Liao wrote:
> On Wed, 2017-01-11 at 13:41 +0100, Jean Delvare wrote:
> >  config COMMON_CLK_MT8173
> >  	bool "Clock driver for Mediatek MT8173"
> > -	depends on ARCH_MEDIATEK || COMPILE_TEST
> > +	depends on (ARCH_MEDIATEK && ARM64) || COMPILE_TEST
> >  	select COMMON_CLK_MEDIATEK
> > -	default ARCH_MEDIATEK
> > +	default y
> >  	---help---
> >  	  This driver supports Mediatek MT8173 clocks.
> 
> Although MT8173 is a 64-bit SoC, but 32-bit kernel can run on it. So I
> think it no need to limit COMMON_CLK_MT8173 only be enabled on ARM64
> platform.

OK, I'll leave that one alone then, thanks for the information.

-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply

* Re: [PATCH 0/4] ARM: dts: mt7623: Add initial Geek Force support
From: John Crispin @ 2017-01-12  8:23 UTC (permalink / raw)
  To: Andreas Färber,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Matthias Brugger, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Paul Lai,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <3fecb422-8185-7ee0-c203-2bfdc4fd1393-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org>

Hi Andreas,

had a look last night why the ethernet dtsi was not added and it
obviously was not added as we were waiting for the clk-mt2701 to be
merged. the ethernet dtsi will have phandles pointing at the clk nodes
which did not exist at the time. same is true for the PWM code.

i sat down last night and worked out what pending patches i still have
for mt7623 and out of the ~80 required to get v4.4 working i only need
around 10 for v4.10-rc1.

i started to rebase these patches last night and will have time to test
them tomorrow or early next week. as the pwrap node alone is around 200
lines of devicetree we need to figure out a way to add this to the dts
files without duplicating it. i'll try to post a series early next week
that we can then discuss and rebase your geekboard patches on.

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

^ permalink raw reply

* Re: [PATCH v5] arm64: dts: mt8173: add mmsel clocks for 4K support
From: Daniel Kurtz @ 2017-01-12  4:50 UTC (permalink / raw)
  To: Bibby Hsieh
  Cc: Mark Rutland, open list:OPEN FIRMWARE AND...,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	moderated list:ARM/Mediatek SoC support, Rob Herring, Pawel Moll,
	Ian Campbell, Kumar Gala, Catalin Marinas, Will Deacon,
	Matthias Brugger, Yingjoe Chen, Sascha Hauer, James Liao,
	Lorenzo Pieralisi
In-Reply-To: <1470279438-60372-1-git-send-email-bibby.hsieh@mediatek.com>

Hi Matthias,

(Trying again to send plain text email)...

On Thu, Aug 4, 2016 at 10:57 AM, Bibby Hsieh <bibby.hsieh@mediatek.com> wrote:
> To support HDMI 4K resolution, mmsys need clcok
> mm_sel to be 400MHz.
>
> The board .dts file should override the clock rate
> property with the higher VENCPLL frequency the board
> supports HDMI 4K resolution.
>
> Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>

Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>


It looks like this patch was lost.  It is actually a prerequisite for
MTK 4k HDMI support, which already landed in v4.9.

See the email thread entitled:
[PATCH v5 0/3] MT8173 HDMI 4K support <https://lkml.org/lkml/2016/9/28/893>

Or these three:

0d2200794f0a drm/mediatek: modify the factor to make the pll_rate set
in the 1G-2G range
968253bd7caa drm/mediatek: enhance the HDMI driving current
d542b7c473f0 drm/mediatek: do mtk_hdmi_send_infoframe after HDMI clock enable

-Dan

> ---
>  arch/arm64/boot/dts/mediatek/mt8173.dtsi |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> index 78529e4..c3f32f3 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> @@ -690,6 +690,8 @@
>                         compatible = "mediatek,mt8173-mmsys", "syscon";
>                         reg = <0 0x14000000 0 0x1000>;
>                         power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
> +                       assigned-clocks = <&topckgen CLK_TOP_MM_SEL>;
> +                       assigned-clock-rates = <400000000>;
>                         #clock-cells = <1>;
>                 };
>
> --
> 1.7.9.5
>

^ permalink raw reply

* Re: [PATCH] clk: mediatek: Fix MT2701 dependencies
From: James Liao @ 2017-01-12  3:39 UTC (permalink / raw)
  To: Jean Delvare
  Cc: Andreas Färber, linux-clk, linux-mediatek, Matthias Brugger,
	Erin Lo, Stephen Boyd, Shunli Wang, Michael Turquette
In-Reply-To: <20170111134146.36878e01@endymion>

Hi Jean,

On Wed, 2017-01-11 at 13:41 +0100, Jean Delvare wrote:
> Hi James,
> 
> On Wed, 11 Jan 2017 09:56:28 +0800, James Liao wrote:
> > On Tue, 2017-01-10 at 13:03 +0100, Jean Delvare wrote:
> > > What about MT8135 and MT8173, are they 32-bit SoCs as well?
> > 
> > MT8135 is a 32-bit SoC and MT8173 is a 64-bit SoC.
> 
> OK, so would the following change make sense?
> 
> ---
>  drivers/clk/mediatek/Kconfig |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> --- linux-4.10-rc2.orig/drivers/clk/mediatek/Kconfig	2017-01-09 11:17:37.542344083 +0100
> +++ linux-4.10-rc2/drivers/clk/mediatek/Kconfig	2017-01-11 13:40:21.965890913 +0100
> @@ -52,16 +52,16 @@ config COMMON_CLK_MT2701_BDPSYS
>  
>  config COMMON_CLK_MT8135
>  	bool "Clock driver for Mediatek MT8135"
> -	depends on ARCH_MEDIATEK || COMPILE_TEST
> +	depends on (ARCH_MEDIATEK && !ARM64) || COMPILE_TEST
>  	select COMMON_CLK_MEDIATEK
> -	default ARCH_MEDIATEK
> +	default y
>  	---help---
>  	  This driver supports Mediatek MT8135 clocks.
>  
>  config COMMON_CLK_MT8173
>  	bool "Clock driver for Mediatek MT8173"
> -	depends on ARCH_MEDIATEK || COMPILE_TEST
> +	depends on (ARCH_MEDIATEK && ARM64) || COMPILE_TEST
>  	select COMMON_CLK_MEDIATEK
> -	default ARCH_MEDIATEK
> +	default y
>  	---help---
>  	  This driver supports Mediatek MT8173 clocks.
> 
> Thanks,

Although MT8173 is a 64-bit SoC, but 32-bit kernel can run on it. So I
think it no need to limit COMMON_CLK_MT8173 only be enabled on ARM64
platform.


Best regards,

James



^ permalink raw reply

* Re: [PATCH 1/2] Documentation: devicetree: Add document bindings for mtk-cir
From: Rob Herring @ 2017-01-11 21:27 UTC (permalink / raw)
  To: Sean Wang
  Cc: Mauro Carvalho Chehab, Hans de Goede, Heiner Kallweit,
	Mark Rutland, Matthias Brugger, Andi Shyti, Hans Verkuil,
	Sean Young, Ivaylo Dimitrov, linux-media@vger.kernel.org,
	devicetree@vger.kernel.org, linux-mediatek,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, sean wang
In-Reply-To: <1484015754.4057.4.camel@mtkswgap22>

On Mon, Jan 9, 2017 at 8:35 PM, Sean Wang <sean.wang@mediatek.com> wrote:
> Hi Rob,
>
> thanks for your effort for reviewing. I added comments inline.
>
> On Mon, 2017-01-09 at 12:32 -0600, Rob Herring wrote:
>> On Fri, Jan 06, 2017 at 12:06:23AM +0800, sean.wang@mediatek.com wrote:
>> > From: Sean Wang <sean.wang@mediatek.com>
>> >
>> > This patch adds documentation for devicetree bindings for
>> > Mediatek IR controller.
>> >
>> > Signed-off-by: Sean Wang <sean.wang@mediatek.com>
>> > ---
>> >  .../devicetree/bindings/media/mtk-cir.txt          | 23 ++++++++++++++++++++++
>> >  1 file changed, 23 insertions(+)
>> >  create mode 100644 linux-4.8.rc1_p0/Documentation/devicetree/bindings/media/mtk-cir.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/media/mtk-cir.txt b/Documentation/devicetree/bindings/media/mtk-cir.txt
>> > new file mode 100644
>> > index 0000000..bbedd71
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/media/mtk-cir.txt
>> > @@ -0,0 +1,23 @@
>> > +Device-Tree bindings for Mediatek IR controller found in Mediatek SoC family
>> > +
>> > +Required properties:
>> > +- compatible           : "mediatek,mt7623-ir"
>> > +- clocks       : list of clock specifiers, corresponding to
>> > +                 entries in clock-names property;
>> > +- clock-names          : should contain "clk" entries;
>> > +- interrupts           : should contain IR IRQ number;
>> > +- reg                  : should contain IO map address for IR.
>> > +
>> > +Optional properties:
>> > +- linux,rc-map-name : Remote control map name.
>>
>> Would 'label' be appropriate here instead? If not, this needs to be
>> documented in a common location and explained better.
>>
> I checked with how the way applied in other IR drivers is and found that
> most IR driver also use the same label to identify the scan/key table
> they prefer to use such as gpio-ir-recv, ir-hix5hd2, meson-ir and
> sunxi-cir or use hard coding inside the driver. So I thought it should
> be appropriate here currently.

Maybe so, but anything with linux prefix gets extra scrutiny and I'm
not sure that happened on the previous cases. If label has the same
meaning, then we should start using that and deprecate this property.
In any case, a property used by multiple bindings needs to be
documented in a common place. The explanation of the property is bad
too. It just spells out RC with no explanation. I'm sure you just
copy-n-pasted it from the others, but that doesn't make it okay.

Rob

^ permalink raw reply

* Re: [PATCH] clk: mediatek: Fix MT2701 dependencies
From: Jean Delvare @ 2017-01-11 14:42 UTC (permalink / raw)
  To: Yingjoe Chen
  Cc: James Liao, Erin Lo, Stephen Boyd, linux-clk, Michael Turquette,
	linux-mediatek, Shunli Wang, Matthias Brugger,
	Andreas Färber
In-Reply-To: <1484143526.8614.0.camel@mtksdaap41>

Hi Joe,

On Wed, 11 Jan 2017 22:05:26 +0800, Yingjoe Chen wrote:
> On Wed, 2017-01-11 at 13:41 +0100, Jean Delvare wrote:
> > @@ -52,16 +52,16 @@ config COMMON_CLK_MT2701_BDPSYS
> >  
> >  config COMMON_CLK_MT8135
> >  	bool "Clock driver for Mediatek MT8135"
> > -	depends on ARCH_MEDIATEK || COMPILE_TEST
> > +	depends on (ARCH_MEDIATEK && !ARM64) || COMPILE_TEST
> 
> 
> Why not just check for ARM?
> 
> +	depends on (ARCH_MEDIATEK && ARM) || COMPILE_TEST

I don't know :D Andreas suggested it for MT2701, so I assumed ARM was
set also for 64-bit kernels (like X86 is set for 64-bit kernels) and
ARM64 had to be used to differentiate. But it seems ARM isn't set for
64-bit kernels, so you are right, checking for ARM should work and is
more straightforward. I'll fix it before submitting the patch.

Thanks for the review,
-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply

* Re: [PATCH] clk: mediatek: Fix MT2701 dependencies
From: Yingjoe Chen @ 2017-01-11 14:05 UTC (permalink / raw)
  To: Jean Delvare
  Cc: James Liao, Erin Lo, Stephen Boyd, linux-clk, Michael Turquette,
	linux-mediatek, Shunli Wang, Matthias Brugger,
	Andreas Färber
In-Reply-To: <20170111134146.36878e01@endymion>

On Wed, 2017-01-11 at 13:41 +0100, Jean Delvare wrote:
> Hi James,
> 
> On Wed, 11 Jan 2017 09:56:28 +0800, James Liao wrote:
> > On Tue, 2017-01-10 at 13:03 +0100, Jean Delvare wrote:
> > > What about MT8135 and MT8173, are they 32-bit SoCs as well?
> > 
> > MT8135 is a 32-bit SoC and MT8173 is a 64-bit SoC.
> 
> OK, so would the following change make sense?
> 
> ---
>  drivers/clk/mediatek/Kconfig |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> --- linux-4.10-rc2.orig/drivers/clk/mediatek/Kconfig	2017-01-09 11:17:37.542344083 +0100
> +++ linux-4.10-rc2/drivers/clk/mediatek/Kconfig	2017-01-11 13:40:21.965890913 +0100
> @@ -52,16 +52,16 @@ config COMMON_CLK_MT2701_BDPSYS
>  
>  config COMMON_CLK_MT8135
>  	bool "Clock driver for Mediatek MT8135"
> -	depends on ARCH_MEDIATEK || COMPILE_TEST
> +	depends on (ARCH_MEDIATEK && !ARM64) || COMPILE_TEST


Why not just check for ARM?

+	depends on (ARCH_MEDIATEK && ARM) || COMPILE_TEST

Joe.C


^ permalink raw reply

* [PATCH 2/2] crypto: mediatek - fix format string for 64-bit builds
From: Arnd Bergmann @ 2017-01-11 13:55 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Ryder Lee, Arnd Bergmann, linux-kernel, linux-mediatek,
	linux-crypto, Matthias Brugger, David S. Miller, linux-arm-kernel
In-Reply-To: <20170111135104.3961730-1-arnd@arndb.de>

After I enabled COMPILE_TEST for non-ARM targets, I ran into these
warnings:

crypto/mediatek/mtk-aes.c: In function 'mtk_aes_info_map':
crypto/mediatek/mtk-aes.c:224:28: error: format '%d' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Werror=format=]
   dev_err(cryp->dev, "dma %d bytes error\n", sizeof(*info));
crypto/mediatek/mtk-sha.c:344:28: error: format '%d' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Werror=format=]
crypto/mediatek/mtk-sha.c:550:21: error: format '%u' expects argument of type 'unsigned int', but argument 4 has type 'size_t {aka long unsigned int}' [-Werror=format=]

The correct format for size_t is %zu, so use that in all three
cases.

Fixes: 785e5c616c84 ("crypto: mediatek - Add crypto driver support for some MediaTek chips")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/crypto/mediatek/mtk-aes.c | 2 +-
 drivers/crypto/mediatek/mtk-sha.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/mediatek/mtk-aes.c b/drivers/crypto/mediatek/mtk-aes.c
index 3271471060d9..1370cabeeb5b 100644
--- a/drivers/crypto/mediatek/mtk-aes.c
+++ b/drivers/crypto/mediatek/mtk-aes.c
@@ -221,7 +221,7 @@ static int mtk_aes_info_map(struct mtk_cryp *cryp,
 	aes->ct_dma = dma_map_single(cryp->dev, info, sizeof(*info),
 					DMA_TO_DEVICE);
 	if (unlikely(dma_mapping_error(cryp->dev, aes->ct_dma))) {
-		dev_err(cryp->dev, "dma %d bytes error\n", sizeof(*info));
+		dev_err(cryp->dev, "dma %zu bytes error\n", sizeof(*info));
 		return -EINVAL;
 	}
 	aes->tfm_dma = aes->ct_dma + sizeof(*ct);
diff --git a/drivers/crypto/mediatek/mtk-sha.c b/drivers/crypto/mediatek/mtk-sha.c
index 89513632c8ed..98b3d74ae23d 100644
--- a/drivers/crypto/mediatek/mtk-sha.c
+++ b/drivers/crypto/mediatek/mtk-sha.c
@@ -341,7 +341,7 @@ static int mtk_sha_info_map(struct mtk_cryp *cryp,
 	sha->ct_dma = dma_map_single(cryp->dev, info, sizeof(*info),
 				      DMA_BIDIRECTIONAL);
 	if (unlikely(dma_mapping_error(cryp->dev, sha->ct_dma))) {
-		dev_err(cryp->dev, "dma %d bytes error\n", sizeof(*info));
+		dev_err(cryp->dev, "dma %zu bytes error\n", sizeof(*info));
 		return -EINVAL;
 	}
 	sha->tfm_dma = sha->ct_dma + sizeof(*ct);
@@ -547,7 +547,7 @@ static int mtk_sha_update_slow(struct mtk_cryp *cryp,
 
 	final = (ctx->flags & SHA_FLAGS_FINUP) && !ctx->total;
 
-	dev_dbg(cryp->dev, "slow: bufcnt: %u\n", ctx->bufcnt);
+	dev_dbg(cryp->dev, "slow: bufcnt: %zu\n", ctx->bufcnt);
 
 	if (final) {
 		sha->flags |= SHA_FLAGS_FINAL;
-- 
2.9.0

^ permalink raw reply related

* [PATCH 1/2] crypto: mediatek - remove ARM dependencies
From: Arnd Bergmann @ 2017-01-11 13:50 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Arnd Bergmann, David S. Miller, Matthias Brugger, Ryder Lee,
	linux-crypto, linux-kernel, linux-arm-kernel, linux-mediatek

Building the mediatek driver on an older ARM architecture results in a
harmless warning:

warning: (ARCH_OMAP2PLUS_TYPICAL && CRYPTO_DEV_MEDIATEK) selects NEON which has unmet direct dependencies (VFPv3 && CPU_V7)

We could add an explicit dependency on CPU_V7, but it seems nicer to
open up the build to additional configurations. This replaces the ARM
optimized algorithm selection with the normal one that all other drivers
use, and that in turn lets us relax the dependency on ARM and drop
a number of the unrelated 'select' statements.

Obviously a real user would still select those other optimized drivers
as a fallback, but as there is no strict dependency, we can leave that
up to the user.

Fixes: 785e5c616c84 ("crypto: mediatek - Add crypto driver support for some MediaTek chips")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/crypto/Kconfig | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 8ded3af88b16..9d37ae07b4ce 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -555,15 +555,12 @@ config CRYPTO_DEV_ROCKCHIP
 
 config CRYPTO_DEV_MEDIATEK
 	tristate "MediaTek's EIP97 Cryptographic Engine driver"
-	depends on ARM && (ARCH_MEDIATEK || COMPILE_TEST)
-	select NEON
-	select KERNEL_MODE_NEON
-	select ARM_CRYPTO
+	depends on (ARM && ARCH_MEDIATEK) || COMPILE_TEST
 	select CRYPTO_AES
 	select CRYPTO_BLKCIPHER
-	select CRYPTO_SHA1_ARM_NEON
-	select CRYPTO_SHA256_ARM
-	select CRYPTO_SHA512_ARM
+	select CRYPTO_SHA1
+	select CRYPTO_SHA256
+	select CRYPTO_SHA512
 	select CRYPTO_HMAC
 	help
 	  This driver allows you to utilize the hardware crypto accelerator
-- 
2.9.0

^ permalink raw reply related

* Re: [PATCH] clk: mediatek: Fix MT2701 dependencies
From: Jean Delvare @ 2017-01-11 12:41 UTC (permalink / raw)
  To: James Liao
  Cc: Erin Lo, Stephen Boyd, linux-clk-u79uwXL29TY76Z2rM5mHXA,
	Michael Turquette,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Shunli Wang,
	Matthias Brugger, Andreas Färber
In-Reply-To: <1484099788.26901.8.camel@mtksdaap41>

Hi James,

On Wed, 11 Jan 2017 09:56:28 +0800, James Liao wrote:
> On Tue, 2017-01-10 at 13:03 +0100, Jean Delvare wrote:
> > What about MT8135 and MT8173, are they 32-bit SoCs as well?
> 
> MT8135 is a 32-bit SoC and MT8173 is a 64-bit SoC.

OK, so would the following change make sense?

---
 drivers/clk/mediatek/Kconfig |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

--- linux-4.10-rc2.orig/drivers/clk/mediatek/Kconfig	2017-01-09 11:17:37.542344083 +0100
+++ linux-4.10-rc2/drivers/clk/mediatek/Kconfig	2017-01-11 13:40:21.965890913 +0100
@@ -52,16 +52,16 @@ config COMMON_CLK_MT2701_BDPSYS
 
 config COMMON_CLK_MT8135
 	bool "Clock driver for Mediatek MT8135"
-	depends on ARCH_MEDIATEK || COMPILE_TEST
+	depends on (ARCH_MEDIATEK && !ARM64) || COMPILE_TEST
 	select COMMON_CLK_MEDIATEK
-	default ARCH_MEDIATEK
+	default y
 	---help---
 	  This driver supports Mediatek MT8135 clocks.
 
 config COMMON_CLK_MT8173
 	bool "Clock driver for Mediatek MT8173"
-	depends on ARCH_MEDIATEK || COMPILE_TEST
+	depends on (ARCH_MEDIATEK && ARM64) || COMPILE_TEST
 	select COMMON_CLK_MEDIATEK
-	default ARCH_MEDIATEK
+	default y
 	---help---
 	  This driver supports Mediatek MT8173 clocks.

Thanks,
-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply

* Re: [PATCH v2 2/2] media: rc: add driver for IR remote receiver on MT7623 SoC
From: Sean Wang @ 2017-01-11  9:27 UTC (permalink / raw)
  To: Andi Shyti
  Cc: kbuild test robot, kbuild-all, mchehab, hdegoede, hkallweit1,
	robh+dt, mark.rutland, matthias.bgg, hverkuil, sean,
	ivo.g.dimitrov.75, linux-media, devicetree, linux-mediatek,
	linux-arm-kernel, linux-kernel, keyhaede
In-Reply-To: <20170110224543.uuoa7ofkvolz6inp@gangnam.samsung>

okay, I will continue to work based on your changes unless someone else
has concerns

On Wed, 2017-01-11 at 07:45 +0900, Andi Shyti wrote:
> Hi Sean,
> 
> >    include/linux/compiler.h:253:8: sparse: attribute 'no_sanitize_address': unknown attribute
> > >> drivers/media/rc/mtk-cir.c:215:41: sparse: too many arguments for function devm_rc_allocate_device
> >    drivers/media/rc/mtk-cir.c: In function 'mtk_ir_probe':
> >    drivers/media/rc/mtk-cir.c:215:11: error: too many arguments to function 'devm_rc_allocate_device'
> >      ir->rc = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW);
> >               ^~~~~~~~~~~~~~~~~~~~~~~
> >    In file included from drivers/media/rc/mtk-cir.c:22:0:
> >    include/media/rc-core.h:213:16: note: declared here
> >     struct rc_dev *devm_rc_allocate_device(struct device *dev);
> >                    ^~~~~~~~~~~~~~~~~~~~~~~
> > 
> > vim +/devm_rc_allocate_device +215 drivers/media/rc/mtk-cir.c
> > 
> >    209		ir->base = devm_ioremap_resource(dev, res);
> >    210		if (IS_ERR(ir->base)) {
> >    211			dev_err(dev, "failed to map registers\n");
> >    212			return PTR_ERR(ir->base);
> >    213		}
> >    214	
> >  > 215		ir->rc = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW);
> 
> this error comes because the patches I pointed out have not been
> applied yet. I guess you can ignore them as long as you tested
> yours on top those patches.
> 
> Andi

^ permalink raw reply

* Re: [PATCH v2 2/2] media: rc: add driver for IR remote receiver on MT7623 SoC
From: Sean Wang @ 2017-01-11  9:19 UTC (permalink / raw)
  To: Sean Young
  Cc: mchehab-JPH+aEBZ4P+UEJcrhfAQsw, hdegoede-H+wXaHxf7aLQT0dZR+AlfA,
	hkallweit1-Re5JQEeQqe8AvxtiuMwx3w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w,
	andi.shyti-Sze3O3UU22JBDgjK7y7TUQ,
	hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
	ivo.g.dimitrov.75-Re5JQEeQqe8AvxtiuMwx3w,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	keyhaede-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20170110172355.GA27008-3XSxi2G4b3iXFJAUJl40Xg@public.gmane.org>

On Tue, 2017-01-10 at 17:23 +0000, Sean Young wrote:
> Hi Sean,
> 

> > > 
> > > The kernel guarantees that calls to the interrupt handler are serialised,
> > > no need to disable the interrupt in the handler.
> > 
> > agreed. I will save the mtk irq disable/enable and retest again.
> > 
> > 
> > > > +
> > > > +	/* Reset decoder state machine */
> > > > +	ir_raw_event_reset(ir->rc);
> > > 
> > > Not needed.
> > 
> > 
> > two reasons I added the line here
> > 
> > 1) 
> > I thought it is possible the decoder goes to the
> > middle state when getting the data not belonged
> > to the protocol. If so, that would cause the decoding
> > fails in the next time receiving the valid protocol data.
> 
> The last IR event submitted will always be a long space, that's enough
> to reset the decoders. Adding a ir_raw_event_reset() will do this
> more explicitly, rather than their state machines resetting themselves
> through the trailing space.

thanks for the detailed explanation :) i got it

but some hardware limitation let me can't do it in implicit way :(

reset decoder state machine explicitly is needed because 
1) the longest duration for space MTK IR hardware can record is not
safely long. e.g  12ms if rx resolution is 46us by default. There is
still the risk to satisfying every decoder to reset themselves through
long enough trailing spaces
 
2) the IRQ handler called guarantees that start of IR message is always
contained in and starting from register MTK_CHKDATA_REG(0). 

I will add these words for hardware limitation into comments in the
driver

> > 2) 
> > the mtk hardware register always contains the start of 
> > IR message. So force to sync the state between 
> > HW and ir-core.
> > 
> > 
> > 
> > > > +
> > > > +	/* First message must be pulse */
> > > > +	rawir.pulse = false;
> > > 
> > > pulse = true?
> > 
> > becasue of rawir.pulse = !rawir.pulse does as below
> > so the initial value is set as false.
> 
> Ah, sorry, of course. :)
> 
> > > > +
> > > > +	/* Handle all pulse and space IR controller captures */
> > > > +	for (i = 0 ; i < MTK_CHKDATA_SZ ; i++) {
> > > > +		val = mtk_r32(ir, MTK_CHKDATA_REG(i));
> > > > +		dev_dbg(ir->dev, "@reg%d=0x%08x\n", i, val);
> > > > +
> > > > +		for (j = 0 ; j < 4 ; j++) {
> > > > +			wid = (val & (MTK_WIDTH_MASK << j * 8)) >> j * 8;
> > > > +			rawir.pulse = !rawir.pulse;
> > > > +			rawir.duration = wid * (MTK_IR_SAMPLE + 1);
> > > > +			ir_raw_event_store_with_filter(ir->rc, &rawir);
> > > > +		}
> > > 
> > > In v1 you would break out of the loop if the ir message was shorter, but
> > > now you are always passing on 68 pulses and spaces. Is that right?
> > 
> > as I asked in the previous mail list as below i copied from it, so i
> > made some changes ...
> > 
> > """""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
> > > I had another question. I found multiple and same IR messages being
> > > received when using SONY remote controller. Should driver needs to
> > > report each message or only one of these to the upper layer ?
> > 
> > In general the driver shouldn't try to change any IR message, this
> > should be done in rc-core if necessary.
> > 
> > rc-core should handle this correctly. If the same key is received twice
> > within IR_KEYPRESS_TIMEOUT (250ms) then it not reported to the input
> > layer.
> > """"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
> > 
> > for example:
> > the 68 pulse/spaces might contains 2.x IR messages when I
> > pressed one key on SONY remote control. 
> > 
> > the v1 proposed is passing only one IR message into ir-core ; 
> > the v2 done is passing all IR messages even including the last
> > incomplete message into ir-core. 
> 
> Yes, agreed. Sorry if I wasn't clear. I just wanted to make sure you've
> thought about what happens when the IR message is short (e.g. rc-5 with
> 23 pulse-spaces). Are the remaining registers 0 or do we get stale data
> from a previous transmit?

Before exit from this IRQ handler , mtk_w32_mask(ir, 0x1, MTK_IRCLR,
MTK_IRCLR_REG) would be done that causes all registers used to store 
pulses and spaces to be cleared, so no stale data appears in the next 
transmit.


> > But I was still afraid the state machine can't  go back to initial state
> > after receiving these incomplete data. 
> > 
> > So the ir_raw_event_reset() call in the beginning of ISR seems becoming
> > more important.
> > 
> > > > +	}
> > > > +
> > > > +	/* The maximum number of edges the IR controller can
> > > > +	 * hold is MTK_CHKDATA_SZ * 4. So if received IR messages
> > > > +	 * is over the limit, the last incomplete IR message would
> > > > +	 * be appended trailing space and still would be sent into
> > > > +	 * ir-rc-raw to decode. That helps it is possible that it
> > > > +	 * has enough information to decode a scancode even if the
> > > > +	 * trailing end of the message is missing.
> > > > +	 */
> > > > +	if (!MTK_IR_END(wid, rawir.pulse)) {
> > > > +		rawir.pulse = false;
> > > > +		rawir.duration = MTK_MAX_SAMPLES * (MTK_IR_SAMPLE + 1);
> > > > +		ir_raw_event_store_with_filter(ir->rc, &rawir);
> > > > +	}
> 
> See here you add a long space if one was not added already.
> 
> > > > +
> > > > +	ir_raw_event_handle(ir->rc);
> > > > +
> > > > +	/* Restart controller for the next receive */
> > > > +	mtk_w32_mask(ir, 0x1, MTK_IRCLR, MTK_IRCLR_REG);
> > > > +
> > > > +	/* Clear interrupt status */
> > > > +	mtk_w32_mask(ir, 0x1, MTK_IRINT_CLR, MTK_IRINT_CLR_REG);
> > > > +
> > > > +	/* Enable interrupt */
> > > > +	mtk_irq_enable(ir, MTK_IRINT_EN);
> > > > +
> > > > +	return IRQ_HANDLED;
> > > > +}
> > > > +
> > > > +static int mtk_ir_probe(struct platform_device *pdev)
> > > > +{
> > > > +	struct device *dev = &pdev->dev;
> > > > +	struct device_node *dn = dev->of_node;
> > > > +	struct resource *res;
> > > > +	struct mtk_ir *ir;
> > > > +	u32 val;
> > > > +	int ret = 0;
> > > > +	const char *map_name;
> > > > +
> > > > +	ir = devm_kzalloc(dev, sizeof(struct mtk_ir), GFP_KERNEL);
> > > > +	if (!ir)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	ir->dev = dev;
> > > > +
> > > > +	if (!of_device_is_compatible(dn, "mediatek,mt7623-cir"))
> > > > +		return -ENODEV;
> > > > +
> > > > +	ir->clk = devm_clk_get(dev, "clk");
> > > > +	if (IS_ERR(ir->clk)) {
> > > > +		dev_err(dev, "failed to get a ir clock.\n");
> > > > +		return PTR_ERR(ir->clk);
> > > > +	}
> > > > +
> > > > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > > +	ir->base = devm_ioremap_resource(dev, res);
> > > > +	if (IS_ERR(ir->base)) {
> > > > +		dev_err(dev, "failed to map registers\n");
> > > > +		return PTR_ERR(ir->base);
> > > > +	}
> > > > +
> > > > +	ir->rc = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW);
> > > > +	if (!ir->rc) {
> > > > +		dev_err(dev, "failed to allocate device\n");
> > > > +		return -ENOMEM;
> > > > +	}
> > > > +
> > > > +	ir->rc->priv = ir;
> > > > +	ir->rc->input_name = MTK_IR_DEV;
> > > > +	ir->rc->input_phys = MTK_IR_DEV "/input0";
> > > > +	ir->rc->input_id.bustype = BUS_HOST;
> > > > +	ir->rc->input_id.vendor = 0x0001;
> > > > +	ir->rc->input_id.product = 0x0001;
> > > > +	ir->rc->input_id.version = 0x0001;
> > > > +	map_name = of_get_property(dn, "linux,rc-map-name", NULL);
> > > > +	ir->rc->map_name = map_name ?: RC_MAP_EMPTY;
> > > > +	ir->rc->dev.parent = dev;
> > > > +	ir->rc->driver_name = MTK_IR_DEV;
> > > > +	ir->rc->allowed_protocols = RC_BIT_ALL;
> > > > +	ir->rc->rx_resolution = MTK_IR_SAMPLE;
> > > > +	ir->rc->timeout = MTK_MAX_SAMPLES * (MTK_IR_SAMPLE + 1);
> > > > +
> > > > +	ret = devm_rc_register_device(dev, ir->rc);
> > > 
> > > Here you do devm_rc_register_device()
> > 
> > does it have problem ?
> 
> Sorry, no. I just wanted to highlight wrt a comment below.
> > 
> > 
> > > > +	if (ret) {
> > > > +		dev_err(dev, "failed to register rc device\n");
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	platform_set_drvdata(pdev, ir);
> > > > +
> > > > +	ir->irq = platform_get_irq(pdev, 0);
> > > > +	if (ir->irq < 0) {
> > > > +		dev_err(dev, "no irq resource\n");
> > > > +		return -ENODEV;
> > > > +	}
> > > > +
> > > > +	/* Enable interrupt after proper hardware
> > > > +	 * setup and IRQ handler registration
> > > > +	 */
> > > > +	if (clk_prepare_enable(ir->clk)) {
> > > > +		dev_err(dev, "try to enable ir_clk failed\n");
> > > > +		ret = -EINVAL;
> > > > +		goto exit_clkdisable_clk;
> > > > +	}
> > > > +
> > > > +	mtk_irq_disable(ir, MTK_IRINT_EN);
> > > > +
> > > > +	ret = devm_request_irq(dev, ir->irq, mtk_ir_irq, 0, MTK_IR_DEV, ir);
> > > > +	if (ret) {
> > > > +		dev_err(dev, "failed request irq\n");
> > > > +		goto exit_clkdisable_clk;
> > > > +	}
> > > > +
> > > > +	/* Enable IR and PWM */
> > > > +	val = mtk_r32(ir, MTK_CONFIG_HIGH_REG);
> > > > +	val |= MTK_PWM_EN | MTK_IR_EN;
> > > > +	mtk_w32(ir, val, MTK_CONFIG_HIGH_REG);
> > > > +
> > > > +	/* Setting sample period */
> > > > +	mtk_w32_mask(ir, MTK_CHK_PERIOD, MTK_CHK_PERIOD_MASK,
> > > > +		     MTK_CONFIG_LOW_REG);
> > > > +
> > > > +	mtk_irq_enable(ir, MTK_IRINT_EN);
> > > > +
> > > > +	dev_info(dev, "Initialized MT7623 IR driver, sample period = %luus\n",
> > > > +		 DIV_ROUND_CLOSEST(MTK_IR_SAMPLE, 1000));
> > > > +
> > > > +	return 0;
> > > > +
> > > > +exit_clkdisable_clk:
> > > > +	clk_disable_unprepare(ir->clk);
> > > > +
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +static int mtk_ir_remove(struct platform_device *pdev)
> > > > +{
> > > > +	struct mtk_ir *ir = platform_get_drvdata(pdev);
> > > > +
> > > > +	/* Avoid contention between remove handler and
> > > > +	 * IRQ handler so that disabling IR interrupt and
> > > > +	 * waiting for pending IRQ handler to complete
> > > > +	 */
> > > > +	mtk_irq_disable(ir, MTK_IRINT_EN);
> > > > +	synchronize_irq(ir->irq);
> > > > +
> > > > +	clk_disable_unprepare(ir->clk);
> > > > +
> > > > +	rc_unregister_device(ir->rc);
> > > 
> > > Yet here you explicitly call rc_unregister_device(). Since it was registered
> > > with the devm call, this call is not needed and will lead to double frees etc
> > 
> > bug :( .  I will fix it ..
> > 
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static const struct of_device_id mtk_ir_match[] = {
> > > > +	{ .compatible = "mediatek,mt7623-cir" },
> > > > +	{},
> > > > +};
> > > > +MODULE_DEVICE_TABLE(of, mtk_ir_match);
> > > > +
> > > > +static struct platform_driver mtk_ir_driver = {
> > > > +	.probe          = mtk_ir_probe,
> > > > +	.remove         = mtk_ir_remove,
> > > > +	.driver = {
> > > > +		.name = MTK_IR_DEV,
> > > > +		.of_match_table = mtk_ir_match,
> > > > +	},
> > > > +};
> > > > +
> > > > +module_platform_driver(mtk_ir_driver);
> > > > +
> > > > +MODULE_DESCRIPTION("Mediatek IR Receiver Controller Driver");
> > > > +MODULE_AUTHOR("Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>");
> > > > +MODULE_LICENSE("GPL");
> > > > -- 
> > > > 2.7.4
> > > > 
> > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-media" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

^ permalink raw reply

* [PATCH 4/4] arm: dts: mt2701: Add auxadc device node.
From: Erin Lo @ 2017-01-11  8:38 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: devicetree, linux-arm-kernel, linux-kernel, linux-mediatek,
	srv_heupstream, Zhiyong Tao, Erin Lo
In-Reply-To: <1484123924-8946-1-git-send-email-erin.lo@mediatek.com>

From: Zhiyong Tao <zhiyong.tao@mediatek.com>

Add auxadc device node for MT2701.

Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com>
Signed-off-by: Erin Lo <erin.lo@mediatek.com>
---
 arch/arm/boot/dts/mt2701.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/mt2701.dtsi b/arch/arm/boot/dts/mt2701.dtsi
index 1182c43..4f52019 100644
--- a/arch/arm/boot/dts/mt2701.dtsi
+++ b/arch/arm/boot/dts/mt2701.dtsi
@@ -208,6 +208,15 @@
 		      <0 0x10216000 0 0x2000>;
 	};
 
+	auxadc: adc@11001000 {
+		compatible = "mediatek,mt2701-auxadc";
+		reg = <0 0x11001000 0 0x1000>;
+		clocks = <&pericfg CLK_PERI_AUXADC>;
+		clock-names = "main";
+		#io-channel-cells = <1>;
+		status = "disabled";
+	};
+
 	uart0: serial@11002000 {
 		compatible = "mediatek,mt2701-uart",
 			     "mediatek,mt6577-uart";
-- 
1.9.1

^ permalink raw reply related

* [PATCH 3/4] arm: dts: mt2701: Add nand device node
From: Erin Lo @ 2017-01-11  8:38 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: devicetree, linux-arm-kernel, linux-kernel, linux-mediatek,
	srv_heupstream, Xiaolei Li, Erin Lo
In-Reply-To: <1484123924-8946-1-git-send-email-erin.lo@mediatek.com>

From: Xiaolei Li <xiaolei.li@mediatek.com>

Add mt2701 nand device node, include nfi and bch ecc.

Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
Signed-off-by: Erin Lo <erin.lo@mediatek.com>
---
 arch/arm/boot/dts/mt2701.dtsi | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm/boot/dts/mt2701.dtsi b/arch/arm/boot/dts/mt2701.dtsi
index 87be52c..1182c43 100644
--- a/arch/arm/boot/dts/mt2701.dtsi
+++ b/arch/arm/boot/dts/mt2701.dtsi
@@ -261,6 +261,28 @@
 		status = "disabled";
 	};
 
+	nandc: nfi@1100d000 {
+		compatible = "mediatek,mt2701-nfc";
+		reg = <0 0x1100d000 0 0x1000>;
+		interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_LOW>;
+		clocks = <&pericfg CLK_PERI_NFI>,
+			 <&pericfg CLK_PERI_NFI_PAD>;
+		clock-names = "nfi_clk", "pad_clk";
+		status = "disabled";
+		ecc-engine = <&bch>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+
+	bch: ecc@1100e000 {
+		compatible = "mediatek,mt2701-ecc";
+		reg = <0 0x1100e000 0 0x1000>;
+		interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_LOW>;
+		clocks = <&pericfg CLK_PERI_NFI_ECC>;
+		clock-names = "nfiecc_clk";
+		status = "disabled";
+	};
+
 	spi1: spi@11016000 {
 		compatible = "mediatek,mt2701-spi";
 		#address-cells = <1>;
-- 
1.9.1

^ permalink raw reply related

* [PATCH 2/4] arm: dts: mt2701: Add iommu/smi device node
From: Erin Lo @ 2017-01-11  8:38 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: devicetree, linux-arm-kernel, linux-kernel, linux-mediatek,
	srv_heupstream, Honghui Zhang, Erin Lo
In-Reply-To: <1484123924-8946-1-git-send-email-erin.lo@mediatek.com>

From: Honghui Zhang <honghui.zhang@mediatek.com>

Add the device node of iommu and smi for MT2701.

Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
Signed-off-by: Erin Lo <erin.lo@mediatek.com>
---
 arch/arm/boot/dts/mt2701.dtsi | 54 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/arch/arm/boot/dts/mt2701.dtsi b/arch/arm/boot/dts/mt2701.dtsi
index eb4c6fd..87be52c 100644
--- a/arch/arm/boot/dts/mt2701.dtsi
+++ b/arch/arm/boot/dts/mt2701.dtsi
@@ -17,6 +17,7 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/reset/mt2701-resets.h>
+#include <dt-bindings/memory/mt2701-larb-port.h>
 #include "skeleton64.dtsi"
 #include "mt2701-pinfunc.h"
 
@@ -161,6 +162,16 @@
 		clock-names = "system-clk", "rtc-clk";
 	};
 
+	smi_common: smi@1000c000 {
+		compatible = "mediatek,mt2701-smi-common";
+		reg = <0 0x1000c000 0 0x1000>;
+		clocks = <&infracfg CLK_INFRA_SMI>,
+			 <&mmsys CLK_MM_SMI_COMMON>,
+			 <&infracfg CLK_INFRA_SMI>;
+		clock-names = "apb", "smi", "async";
+		power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
+	};
+
 	sysirq: interrupt-controller@10200100 {
 		compatible = "mediatek,mt2701-sysirq",
 			     "mediatek,mt6577-sysirq";
@@ -170,6 +181,16 @@
 		reg = <0 0x10200100 0 0x1c>;
 	};
 
+	iommu: mmsys_iommu@10205000 {
+		compatible = "mediatek,mt2701-m4u";
+		reg = <0 0x10205000 0 0x1000>;
+		interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_LOW>;
+		clocks = <&infracfg CLK_INFRA_M4U>;
+		clock-names = "bclk";
+		mediatek,larbs = <&larb0 &larb1 &larb2>;
+		#iommu-cells = <1>;
+	};
+
 	apmixedsys: syscon@10209000 {
 		compatible = "mediatek,mt2701-apmixedsys", "syscon";
 		reg = <0 0x10209000 0 0x1000>;
@@ -272,18 +293,51 @@
 		#clock-cells = <1>;
 	};
 
+	larb0: larb@14010000 {
+		compatible = "mediatek,mt2701-smi-larb";
+		reg = <0 0x14010000 0 0x1000>;
+		mediatek,smi = <&smi_common>;
+		mediatek,larbidx = <0>;
+		clocks = <&mmsys CLK_MM_SMI_LARB0>,
+			 <&mmsys CLK_MM_SMI_LARB0>;
+		clock-names = "apb", "smi";
+		power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
+	};
+
 	imgsys: syscon@15000000 {
 		compatible = "mediatek,mt2701-imgsys", "syscon";
 		reg = <0 0x15000000 0 0x1000>;
 		#clock-cells = <1>;
 	};
 
+	larb2: larb@15001000 {
+		compatible = "mediatek,mt2701-smi-larb";
+		reg = <0 0x15001000 0 0x1000>;
+		mediatek,smi = <&smi_common>;
+		mediatek,larbidx = <2>;
+		clocks = <&imgsys CLK_IMG_SMI_COMM>,
+			 <&imgsys CLK_IMG_SMI_COMM>;
+		clock-names = "apb", "smi";
+		power-domains = <&scpsys MT2701_POWER_DOMAIN_ISP>;
+	};
+
 	vdecsys: syscon@16000000 {
 		compatible = "mediatek,mt2701-vdecsys", "syscon";
 		reg = <0 0x16000000 0 0x1000>;
 		#clock-cells = <1>;
 	};
 
+	larb1: larb@16010000 {
+		compatible = "mediatek,mt2701-smi-larb";
+		reg = <0 0x16010000 0 0x1000>;
+		mediatek,smi = <&smi_common>;
+		mediatek,larbidx = <1>;
+		clocks = <&vdecsys CLK_VDEC_CKGEN>,
+			 <&vdecsys CLK_VDEC_LARB>;
+		clock-names = "apb", "smi";
+		power-domains = <&scpsys MT2701_POWER_DOMAIN_VDEC>;
+	};
+
 	hifsys: syscon@1a000000 {
 		compatible = "mediatek,mt2701-hifsys", "syscon";
 		reg = <0 0x1a000000 0 0x1000>;
-- 
1.9.1

^ permalink raw reply related


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