On Fri, 6 Aug 2021 at 05:16, Peng.Zhou <peng.zhou@xxxxxxxxxxxx> wrote: > > Hi Ulf, > > Thank you for your advise, I have a question about genpd provider, is it > a mandatory DT node for eMMC? Why I confuse it due to I found we only do > enable/disable clock during used eMMC in our platforms without any power > releated actions....it seems that there is no any "pwner-domain" used > for eMMC in our platforms. A power-domain is optional for an eMMC controller to use. It then sounds like you should add it, in cases when you have mmc crypto support. Hope this helps! Kind regards Uffe > > On Mon, 2021-05-10 at 19:57 +0800, Ulf Hansson wrote: > > On Wed, 14 Apr 2021 at 11:12, Peng Zhou <peng.zhou@xxxxxxxxxxxx> wrote: > > > > > > From: Peng Zhou <Peng.Zhou@xxxxxxxxxxxx> > > > > > > 1. add crypto clock control and ungate it before CQHCI init > > > > > > 2. set MMC_CAP2_CRYPTO property of eMMC > > > > > > Signed-off-by: Peng Zhou <Peng.Zhou@xxxxxxxxxxxx> > > > > As discussed [1], I think the ARM SMC call should be managed together > > with the crypto clock, from a PM domain (a genpd provider). > > > > These are the steps I think should help to get this supported: > > > > 1) > > Add a new generic mmc DT property for crypto support. Make it being > > parsed from mmc_of_parse() and let it set MMC_CAP2_CRYPTO. > > > > 2) > > Start by adding a genpd provider in DT and implement support for it in > > drivers/soc/*. See > > Documentation/devicetree/bindings/power/power-domain.yaml, for generic > > DT bindings. > > > > If you already have a genpd provider for the device, you may either > > extend it to cope with the below or implement a new and model the it > > as a child (subdomain) of the existing one. > > > > 3) > > To fetch the crypto clock during probe, the genpd provider should > > implement the ->attach|detach_dev() callbacks. The ->attach_dev() > > callback is invoked from genpd_add_device(), which is called while > > probing the device. Vice verse for the ->detach_dev() callback. > > > > 4) > > Ungating/gating the clock should be implemented from the genpd > > provider's ->start|stop() callbacks. These callbacks are called from > > genpd_runtime_suspend|resume(), thus when the device gets runtime > > suspended/resumed. As the mmc host driver already supports runtime PM, > > only a minor update in the driver's ->probe() is needed, which is to > > call dev_pm_domain_start(). This turns on the crypto clock and allows > > its registers to be accessed. > > > > 5) > > Add also the ARM SMC calls to the genpd provider's ->start|stop() > > callbacks. This means those gets called from SoC specific code, which > > helps to keep the driver portable. > > > > 6) > > Add the crypto clock DT binding for the mmc host. > > > > 7) > > Wire up the mmc host device node in DT to be attached to the genpd > > provider (see power-domains and power-domain-names properties). Make > > sure to also specify the crypto clock and the new mmc crypto property > > for it. > > > > Please, just ask if there is anything unclear. I will do my best to help! > > > > Kind regards > > Uffe > > > > [1] > > https://patchwork.kernel.org/project/linux-mmc/patch/20210309015750.6283-1-peng.zhou@xxxxxxxxxxxx/ > > > > > --- > > > drivers/mmc/host/mtk-sd.c | 15 ++++++++++++++- > > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c > > > index 898ed1b023df..1c90360d6cf2 100644 > > > --- a/drivers/mmc/host/mtk-sd.c > > > +++ b/drivers/mmc/host/mtk-sd.c > > > @@ -442,6 +442,7 @@ struct msdc_host { > > > struct clk *src_clk_cg; /* msdc source clock control gate */ > > > struct clk *sys_clk_cg; /* msdc subsys clock control gate */ > > > struct clk_bulk_data bulk_clks[MSDC_NR_CLOCKS]; > > > + struct clk *crypto_clk; /* msdc crypto clock */ > > > u32 mclk; /* mmc subsystem clock frequency */ > > > u32 src_clk_freq; /* source clock frequency */ > > > unsigned char timing; > > > @@ -802,6 +803,7 @@ static void msdc_set_busy_timeout(struct msdc_host *host, u64 ns, u64 clks) > > > > > > static void msdc_gate_clock(struct msdc_host *host) > > > { > > > + clk_disable_unprepare(host->crypto_clk); > > > clk_bulk_disable_unprepare(MSDC_NR_CLOCKS, host->bulk_clks); > > > clk_disable_unprepare(host->src_clk_cg); > > > clk_disable_unprepare(host->src_clk); > > > @@ -822,6 +824,7 @@ static void msdc_ungate_clock(struct msdc_host *host) > > > dev_err(host->dev, "Cannot enable pclk/axi/ahb clock gates\n"); > > > return; > > > } > > > + clk_prepare_enable(host->crypto_clk); > > > > > > while (!(readl(host->base + MSDC_CFG) & MSDC_CFG_CKSTB)) > > > cpu_relax(); > > > @@ -2512,6 +2515,15 @@ static int msdc_drv_probe(struct platform_device *pdev) > > > goto host_free; > > > } > > > > > > + /* only eMMC has crypto property */ > > > + if ((mmc->caps2 & MMC_CAP2_NO_SD) && (mmc->caps2 & MMC_CAP2_NO_SDIO)) { > > > + host->crypto_clk = devm_clk_get(&pdev->dev, "crypto"); > > > + if (IS_ERR(host->crypto_clk)) > > > + host->crypto_clk = NULL; > > > + else > > > + mmc->caps2 |= MMC_CAP2_CRYPTO; > > > + } > > > + > > > host->irq = platform_get_irq(pdev, 0); > > > if (host->irq < 0) { > > > ret = -EINVAL; > > > @@ -2582,6 +2594,8 @@ static int msdc_drv_probe(struct platform_device *pdev) > > > host->dma_mask = DMA_BIT_MASK(32); > > > mmc_dev(mmc)->dma_mask = &host->dma_mask; > > > > > > + /* here ungate due to cqhci init will access registers */ > > > + msdc_ungate_clock(host); > > > if (mmc->caps2 & MMC_CAP2_CQE) { > > > host->cq_host = devm_kzalloc(mmc->parent, > > > sizeof(*host->cq_host), > > > @@ -2618,7 +2632,6 @@ static int msdc_drv_probe(struct platform_device *pdev) > > > spin_lock_init(&host->lock); > > > > > > platform_set_drvdata(pdev, mmc); > > > - msdc_ungate_clock(host); > > > msdc_init_hw(host); > > > > > > ret = devm_request_irq(&pdev->dev, host->irq, msdc_irq, > > > -- > > > 2.18.0 >
