From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 449CFC433DF for ; Mon, 12 Oct 2020 00:59:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DC8D0206CB for ; Mon, 12 Oct 2020 00:59:02 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="ZG/7AljI" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727163AbgJLA7C (ORCPT ); Sun, 11 Oct 2020 20:59:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54690 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726600AbgJLA7C (ORCPT ); Sun, 11 Oct 2020 20:59:02 -0400 Received: from mail-vs1-xe42.google.com (mail-vs1-xe42.google.com [IPv6:2607:f8b0:4864:20::e42]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CC856C0613D0 for ; Sun, 11 Oct 2020 17:59:01 -0700 (PDT) Received: by mail-vs1-xe42.google.com with SMTP id d4so2637115vsk.13 for ; Sun, 11 Oct 2020 17:59:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=pKuO3QKcuUe/F0t0uUwfDC3OUqDsOLuOv/YQ0PPKJlA=; b=ZG/7AljIIxkb5kzG7HUfhx+PenrEEBEfXe2u+72K/l+ASYNVw1bzTK8HkNYSwNlsXK bwSJVF9pjjshsq0EMj7KGo7mcBMXwlnURFfVNlOtH6C34LV6twpBsGEVtA+Eqk+Z/wLn ENLu//cmWsl73fMUH9ggHyTUOLybFWK8/vrMs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=pKuO3QKcuUe/F0t0uUwfDC3OUqDsOLuOv/YQ0PPKJlA=; b=nPfe9vr0imwzYazhs9CleWijqI/a0ZUuSgw3OY0XumvaPr9IyyyQT3pLldt7aA0a/r 1W5RuOersKUmQUZH3gwSr7TqkgNJgOsz8mEwuTNyeDC+JBK4PbVBMYdcy9GHXUr+5gpD ziNrJLL/H8+2LRGkO4FlKxvztnBlu/LRPvKUXkHPn4SQBsv1D2DJrbsyGQ6GujyCE4ZA UKlIwpo1H8/UNWdDIRMPAVarGW8PA8SfUAREIJhPLDwoMtOH0uPWOJe5i7VCFsMdAPpR vrxe84Hi0E+1s7T2B4hryqi2co1nKoYljUSseosYTR0UgIJUpG9EO/hjTZaxNYZFTKbJ UtGg== X-Gm-Message-State: AOAM531nm+E1JHmOYPjNhESwa+6K8ak/bOXiEnzxUX0pQ70LJELOJcR+ MnTynsYDzj02bGQQqX2fat9GwCpKS02H7ppg4tODhw== X-Google-Smtp-Source: ABdhPJzbMq6z0XxcHIW6V+QcP9xfb/3LB2U9g8w72z9oVPeF9M6hCzMfU7wevdjFzc2rwF7rScRjmqmEOTel5s5bRLY= X-Received: by 2002:a67:3344:: with SMTP id z65mr13370348vsz.47.1602464340859; Sun, 11 Oct 2020 17:59:00 -0700 (PDT) MIME-Version: 1.0 References: <20201011091030.28547-1-wenbin.mei@mediatek.com> <20201011091030.28547-5-wenbin.mei@mediatek.com> In-Reply-To: <20201011091030.28547-5-wenbin.mei@mediatek.com> From: Nicolas Boichat Date: Mon, 12 Oct 2020 08:58:49 +0800 Message-ID: Subject: Re: [PATCH v4 4/4] mmc: mediatek: Add subsys clock control for MT8192 msdc To: Wenbin Mei Cc: Ulf Hansson , Rob Herring , Chaotian Jing , Matthias Brugger , linux-mmc@vger.kernel.org, Devicetree List , linux-arm Mailing List , "moderated list:ARM/Mediatek SoC support" , lkml , srv_heupstream Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On Sun, Oct 11, 2020 at 5:10 PM Wenbin Mei wrote: > > MT8192 msdc is an independent sub system, we need control more bus > clocks for it. > Add support for the additional subsys clocks to allow it to be > configured appropriately. > > Signed-off-by: Wenbin Mei > Reviewed-by: Nicolas Boichat Err, you must not add R-by tag unless I explicitly say so (yes, I reviewed v3, but I didn't add my R-by tag). > --- > drivers/mmc/host/mtk-sd.c | 80 ++++++++++++++++++++++++++++++--------- > 1 file changed, 62 insertions(+), 18 deletions(-) > > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c > index a704745e5882..350e45432e21 100644 > --- a/drivers/mmc/host/mtk-sd.c > +++ b/drivers/mmc/host/mtk-sd.c > @@ -425,6 +425,8 @@ struct msdc_host { > struct clk *h_clk; /* msdc h_clk */ > struct clk *bus_clk; /* bus clock which used to access register */ > 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[3]; /* pclk, axi, ahb clock control gate */ > u32 mclk; /* mmc subsystem clock frequency */ > u32 src_clk_freq; /* source clock frequency */ > unsigned char timing; > @@ -784,6 +786,8 @@ static void msdc_set_busy_timeout(struct msdc_host *host, u64 ns, u64 clks) > > static void msdc_gate_clock(struct msdc_host *host) > { > + clk_bulk_disable_unprepare(ARRAY_SIZE(host->bulk_clks), > + host->bulk_clks); > clk_disable_unprepare(host->src_clk_cg); > clk_disable_unprepare(host->src_clk); > clk_disable_unprepare(host->bus_clk); > @@ -792,10 +796,19 @@ static void msdc_gate_clock(struct msdc_host *host) > > static void msdc_ungate_clock(struct msdc_host *host) > { > + int ret; > + > clk_prepare_enable(host->h_clk); > clk_prepare_enable(host->bus_clk); > clk_prepare_enable(host->src_clk); > clk_prepare_enable(host->src_clk_cg); > + ret = clk_bulk_prepare_enable(ARRAY_SIZE(host->bulk_clks), > + host->bulk_clks); > + if (ret) { > + dev_err(host->dev, "enable clks failed!\n"); > + return; > + } > + > while (!(readl(host->base + MSDC_CFG) & MSDC_CFG_CKSTB)) > cpu_relax(); > } > @@ -2366,6 +2379,53 @@ static void msdc_of_property_parse(struct platform_device *pdev, > host->cqhci = false; > } > > +static int msdc_of_clock_parse(struct platform_device *pdev, > + struct msdc_host *host) > +{ > + struct clk *clk; > + > + host->src_clk = devm_clk_get_optional(&pdev->dev, "source"); > + if (IS_ERR(host->src_clk)) > + return PTR_ERR(host->src_clk); > + > + host->h_clk = devm_clk_get_optional(&pdev->dev, "hclk"); > + if (IS_ERR(host->h_clk)) > + return PTR_ERR(host->h_clk); > + > + host->bus_clk = devm_clk_get_optional(&pdev->dev, "bus_clk"); > + if (IS_ERR(host->bus_clk)) > + host->bus_clk = NULL; > + > + /*source clock control gate is optional clock*/ > + host->src_clk_cg = devm_clk_get_optional(&pdev->dev, "source_cg"); > + if (IS_ERR(host->src_clk_cg)) > + host->src_clk_cg = NULL; > + > + host->sys_clk_cg = devm_clk_get_optional(&pdev->dev, "sys_cg"); > + if (IS_ERR(host->sys_clk_cg)) > + host->sys_clk_cg = NULL; > + > + /* If present, always enable for this clock gate */ > + clk_prepare_enable(host->sys_clk_cg); Understand your reply on v3 (this clock gate cannot be disabled -- https://patchwork.kernel.org/patch/11808433/#23678227), but I'm still unsure if this is the right thing to do to enable it at probe time. I'll let others comment. > + > + clk = devm_clk_get_optional(&pdev->dev, "pclk_cg"); > + if (IS_ERR(clk)) > + clk = NULL; > + host->bulk_clks[0].clk = clk; > + > + clk = devm_clk_get_optional(&pdev->dev, "axi_cg"); > + if (IS_ERR(clk)) > + clk = NULL; > + host->bulk_clks[1].clk = clk; > + > + clk = devm_clk_get_optional(&pdev->dev, "ahb_cg"); > + if (IS_ERR(clk)) > + clk = NULL; > + host->bulk_clks[2].clk = clk; Put the clock names in host->bulk_clks[x].id, then call devm_clk_bulk_get_optional. Example here: https://elixir.bootlin.com/linux/latest/source/drivers/gpio/gpio-dwapb.c#L675 > + > + return 0; > +} > + > static int msdc_drv_probe(struct platform_device *pdev) > { > struct mmc_host *mmc; > @@ -2405,25 +2465,9 @@ static int msdc_drv_probe(struct platform_device *pdev) > if (ret) > goto host_free; > > - host->src_clk = devm_clk_get(&pdev->dev, "source"); > - if (IS_ERR(host->src_clk)) { > - ret = PTR_ERR(host->src_clk); > - goto host_free; > - } > - > - host->h_clk = devm_clk_get(&pdev->dev, "hclk"); > - if (IS_ERR(host->h_clk)) { > - ret = PTR_ERR(host->h_clk); > + ret = msdc_of_clock_parse(pdev, host); > + if (ret) > goto host_free; > - } > - > - host->bus_clk = devm_clk_get(&pdev->dev, "bus_clk"); > - if (IS_ERR(host->bus_clk)) > - host->bus_clk = NULL; > - /*source clock control gate is optional clock*/ > - host->src_clk_cg = devm_clk_get(&pdev->dev, "source_cg"); > - if (IS_ERR(host->src_clk_cg)) > - host->src_clk_cg = NULL; > > host->reset = devm_reset_control_get_optional_exclusive(&pdev->dev, > "hrst"); > -- > 2.18.0