On Tue, May 24, 2016 at 11:43:16AM +0900, Simon Horman wrote:
> From: Ai Kyuse <ai.kyuse.uw@xxxxxxxxxxx>
I wonder if you shouldn't take over ownership of this and the previous
patch? You changed quite a lot.
> +static inline u32 sd_scc_read32(struct tmio_mmc_host *host, int addr)
> +{
> + return readl(host_to_priv(host)->scc_ctl + (addr << host->bus_shift));
> +}
What about passing 'priv' to these functions? Then we can save the
host_to_priv for each access.
> +
> +static inline void sd_scc_write32(struct tmio_mmc_host *host, int addr, u32 val)
> +{
> + writel(val, host_to_priv(host)->scc_ctl + (addr << host->bus_shift));
> +}
Ditto.
> +
> +static unsigned int sh_mobile_sdhi_init_tuning(struct tmio_mmc_host *host)
> +{
> + if (!(host->mmc->caps & MMC_CAP_UHS_SDR104))
> + return 0;
Will the core call us if MMC_CAP_UHS_SDR104 was not set?
> +
> + /* set sampling clock selection range */
> + sd_scc_write32(host, SH_MOBILE_SDHI_SCC_DTCNTL,
> + 0x8 << SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT);
> +
> + /* Initialize SCC */
> + sd_ctrl_write32_as_16_and_16(host, CTL_STATUS, 0x00000000);
..., CTL_STATUS, 0);
?
> +
> + sd_scc_write32(host, SH_MOBILE_SDHI_SCC_DTCNTL,
> + SH_MOBILE_SDHI_SCC_DTCNTL_TAPEN |
> + sd_scc_read32(host, SH_MOBILE_SDHI_SCC_DTCNTL));
> +
> + sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~0x0100 &
> + sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
'CLK_CTL_SCLKEN' instead of 0x100?
> +
> + sd_scc_write32(host, SH_MOBILE_SDHI_SCC_CKSEL,
> + SH_MOBILE_SDHI_SCC_CKSEL_DTSEL |
> + sd_scc_read32(host, SH_MOBILE_SDHI_SCC_CKSEL));
> +
> + sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, 0x0100 |
> + sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
> +
> + sd_scc_write32(host, SH_MOBILE_SDHI_SCC_RVSCNTL,
> + ~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
> + sd_scc_read32(host, SH_MOBILE_SDHI_SCC_RVSCNTL));
> +
> + sd_scc_write32(host, SH_MOBILE_SDHI_SCC_DT2FF, host->scc_tappos);
> +
> + /* Read TAPNUM */
> + return (sd_scc_read32(host, SH_MOBILE_SDHI_SCC_DTCNTL) >>
> + SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT) &
> + SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_MASK;
> +}
> +
> +static void sh_mobile_sdhi_prepare_tuning(struct tmio_mmc_host *host,
> + unsigned long tap)
> +{
> + /* Set sampling clock position */
> + sd_scc_write32(host, SH_MOBILE_SDHI_SCC_TAPSET, tap);
> +}
> +
> +#define SH_MOBILE_SDHI_MAX_TAP 3
unused
> +
> +static int sh_mobile_sdhi_select_tuning(struct tmio_mmc_host *host,
> + bool *tap, int tap_size)
> +{
> + unsigned long tap_num, i;
> + int ok_count;
> +
> + /* Clear SCC_RVSREQ */
> + sd_scc_write32(host, SH_MOBILE_SDHI_SCC_RVSREQ, 0);
> +
> + /* Select SCC */
> + tap_num = (sd_scc_read32(host, SH_MOBILE_SDHI_SCC_DTCNTL) >>
> + SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT) &
> + SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_MASK;
> +
> + if (tap_num * 2 != tap_size)
> + return -EINVAL;
> +
> + /*
> + * Select clock where three consecutive bock reads succeeded.
> + *
> + * There may be multiple occurrences of three successive reads
> + * and selecting any of them is correct. Here the first one is
> + * selected.
> + */
> + ok_count = 0;
> + for (i = 0; i < tap_size; i++) {
> + if (tap[i])
> + ok_count++;
> + else
> + ok_count = 0;
ok_count = tap[i] ? ok_count + 1 : 0;
? Yes, I do like the ternary operator :D
...
> + if (host->mmc->caps & MMC_CAP_UHS_SDR104) {
> + /* Reset SCC */
> + sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~0x0100 &
> + sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
'CLK_CTL_SCLKEN' instead of 0x100?
> +
> + sd_scc_write32(host, SH_MOBILE_SDHI_SCC_CKSEL,
> + ~SH_MOBILE_SDHI_SCC_CKSEL_DTSEL &
> + sd_scc_read32(host, SH_MOBILE_SDHI_SCC_CKSEL));
> +
> + sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, 0x0100 |
> + sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
Ditto.
...
> + if (!hit)
> + dev_warn(&host->pdev->dev, "Unknown clock rate for SDR104 and HS200\n");
HS200 will come later, I think (although the path should be easy now).
Thanks, I think we are quite close. Maybe Ulf does have some high level
comments?
Attachment:
signature.asc
Description: PGP signature
