| Message ID | 20210118020848.11721-3-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: sunxi: Initial Allwinner H616 SoC support | expand |
Hi, On Mon, Jan 18, 2021 at 02:08:29AM +0000, Andre Przywara wrote: > From: Yangtao Li <frank@allwinnertech.com> > > This patch adds support for A100 MMC controller, which use word address > for internal dma. > > Signed-off-by: Yangtao Li <frank@allwinnertech.com> > Signed-off-by: Andre Przywara <andre.przywara@arm.com> We should also disable the timings setup in probe to derive them from the DT. This is causing issues on some SoCs already, so it would be best to not make the situation worse Maxime
On Mon, 18 Jan 2021 14:28:54 +0100 Maxime Ripard <maxime@cerno.tech> wrote: Hi Maxime, > On Mon, Jan 18, 2021 at 02:08:29AM +0000, Andre Przywara wrote: > > From: Yangtao Li <frank@allwinnertech.com> > > > > This patch adds support for A100 MMC controller, which use word > > address for internal dma. > > > > Signed-off-by: Yangtao Li <frank@allwinnertech.com> > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > We should also disable the timings setup in probe to derive them from > the DT. This is causing issues on some SoCs already, so it would be > best to not make the situation worse But only for those new SoCs, where we have the speed modes in the DT in every case (so only new ones)? And this disabling would be SoC/compatible string dependent? Happy to send a patch later if that is what you were thinking about. Also I was wondering about the voltage dependent speed modes: At the moment the driver declares both MMC_CAP_1_8V_DDR and MMC_CAP_3_3V_DDR, so I mimic this in the .dtsi. However in the eventual DTB this looks somewhat dodgy, since most boards only support one of those voltages. Do we ignore this, and rely on the vqmmc-supply to limit this choice? Cheers, Andre Btw: This patch is already in Ulf's -next tree, I just included it here for the sake of completeness. Is that a problem? I don't think it affects the build, so we don't care too much?
On Mon, Jan 18, 2021 at 11:53 PM Andre Przywara <andre.przywara@arm.com> wrote: > > On Mon, 18 Jan 2021 14:28:54 +0100 > Maxime Ripard <maxime@cerno.tech> wrote: > > Hi Maxime, > > > On Mon, Jan 18, 2021 at 02:08:29AM +0000, Andre Przywara wrote: > > > From: Yangtao Li <frank@allwinnertech.com> > > > > > > This patch adds support for A100 MMC controller, which use word > > > address for internal dma. > > > > > > Signed-off-by: Yangtao Li <frank@allwinnertech.com> > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > > > We should also disable the timings setup in probe to derive them from > > the DT. This is causing issues on some SoCs already, so it would be > > best to not make the situation worse > > But only for those new SoCs, where we have the speed modes in the DT > in every case (so only new ones)? And this disabling would be > SoC/compatible string dependent? Happy to send a patch later if that is > what you were thinking about. > > Also I was wondering about the voltage dependent speed modes: At the > moment the driver declares both MMC_CAP_1_8V_DDR and MMC_CAP_3_3V_DDR, > so I mimic this in the .dtsi. However in the eventual DTB this looks > somewhat dodgy, since most boards only support one of those voltages. Do > we ignore this, and rely on the vqmmc-supply to limit this choice? IIRC the DDR flags for separate voltages was added after we (I) added MMC DDR support to our driver. And yes, the core also checks vqmmc-supply before actually selecting the mode. ChenYu > Cheers, > Andre > > Btw: This patch is already in Ulf's -next tree, I just included it here > for the sake of completeness. Is that a problem? I don't think it > affects the build, so we don't care too much? > > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20210118155228.3bd0e909%40slackpad.fritz.box.
Hi Andre, On Mon, Jan 18, 2021 at 03:52:28PM +0000, Andre Przywara wrote: > On Mon, 18 Jan 2021 14:28:54 +0100 > Maxime Ripard <maxime@cerno.tech> wrote: > > Hi Maxime, > > > On Mon, Jan 18, 2021 at 02:08:29AM +0000, Andre Przywara wrote: > > > From: Yangtao Li <frank@allwinnertech.com> > > > > > > This patch adds support for A100 MMC controller, which use word > > > address for internal dma. > > > > > > Signed-off-by: Yangtao Li <frank@allwinnertech.com> > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > > > We should also disable the timings setup in probe to derive them from > > the DT. This is causing issues on some SoCs already, so it would be > > best to not make the situation worse > > But only for those new SoCs, where we have the speed modes in the DT > in every case (so only new ones)? And this disabling would be > SoC/compatible string dependent? Happy to send a patch later if that is > what you were thinking about. Yeah, we should only do it for new SoCs at the moment, based on the compatible. I guess at some point we'll have to remove it for the older SoCs as well since we have reports of it failing for SoCs as old as the A20, but we'll probably want to make it as smooth as possible. Maxime
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c index 6310693f2ac0..e46bb4e404a8 100644 --- a/drivers/mmc/host/sunxi-mmc.c +++ b/drivers/mmc/host/sunxi-mmc.c @@ -245,6 +245,7 @@ struct sunxi_idma_des { struct sunxi_mmc_cfg { u32 idma_des_size_bits; + u32 idma_des_shift; const struct sunxi_mmc_clk_delay *clk_delays; /* does the IP block support autocalibration? */ @@ -344,7 +345,7 @@ static int sunxi_mmc_init_host(struct sunxi_mmc_host *host) /* Enable CEATA support */ mmc_writel(host, REG_FUNS, SDXC_CEATA_ON); /* Set DMA descriptor list base address */ - mmc_writel(host, REG_DLBA, host->sg_dma); + mmc_writel(host, REG_DLBA, host->sg_dma >> host->cfg->idma_des_shift); rval = mmc_readl(host, REG_GCTRL); rval |= SDXC_INTERRUPT_ENABLE_BIT; @@ -374,8 +375,10 @@ static void sunxi_mmc_init_idma_des(struct sunxi_mmc_host *host, next_desc += sizeof(struct sunxi_idma_des); pdes[i].buf_addr_ptr1 = - cpu_to_le32(sg_dma_address(&data->sg[i])); - pdes[i].buf_addr_ptr2 = cpu_to_le32((u32)next_desc); + cpu_to_le32(sg_dma_address(&data->sg[i]) >> + host->cfg->idma_des_shift); + pdes[i].buf_addr_ptr2 = cpu_to_le32((u32)next_desc >> + host->cfg->idma_des_shift); } pdes[0].config |= cpu_to_le32(SDXC_IDMAC_DES0_FD); @@ -1179,6 +1182,23 @@ static const struct sunxi_mmc_cfg sun50i_a64_emmc_cfg = { .needs_new_timings = true, }; +static const struct sunxi_mmc_cfg sun50i_a100_cfg = { + .idma_des_size_bits = 16, + .idma_des_shift = 2, + .clk_delays = NULL, + .can_calibrate = true, + .mask_data0 = true, + .needs_new_timings = true, +}; + +static const struct sunxi_mmc_cfg sun50i_a100_emmc_cfg = { + .idma_des_size_bits = 13, + .idma_des_shift = 2, + .clk_delays = NULL, + .can_calibrate = true, + .needs_new_timings = true, +}; + static const struct of_device_id sunxi_mmc_of_match[] = { { .compatible = "allwinner,sun4i-a10-mmc", .data = &sun4i_a10_cfg }, { .compatible = "allwinner,sun5i-a13-mmc", .data = &sun5i_a13_cfg }, @@ -1187,6 +1207,8 @@ static const struct of_device_id sunxi_mmc_of_match[] = { { .compatible = "allwinner,sun9i-a80-mmc", .data = &sun9i_a80_cfg }, { .compatible = "allwinner,sun50i-a64-mmc", .data = &sun50i_a64_cfg }, { .compatible = "allwinner,sun50i-a64-emmc", .data = &sun50i_a64_emmc_cfg }, + { .compatible = "allwinner,sun50i-a100-mmc", .data = &sun50i_a100_cfg }, + { .compatible = "allwinner,sun50i-a100-emmc", .data = &sun50i_a100_emmc_cfg }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, sunxi_mmc_of_match);