| Message ID | 20201012024345.8361-5-wenbin.mei@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add mmc support for MT8192 SoC | expand |
Thanks for the quick turnaround. And sorry, I should have noticed these issues in my previous pass. On Mon, Oct 12, 2020 at 10:44 AM Wenbin Mei <wenbin.mei@mediatek.com> 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 <wenbin.mei@mediatek.com> > --- > drivers/mmc/host/mtk-sd.c | 74 +++++++++++++++++++++++++++++---------- > 1 file changed, 56 insertions(+), 18 deletions(-) > > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c > index a704745e5882..41703e6d6b17 100644 > --- a/drivers/mmc/host/mtk-sd.c > +++ b/drivers/mmc/host/mtk-sd.c > @@ -35,6 +35,7 @@ > #include "cqhci.h" > > #define MAX_BD_NUM 1024 > +#define MSDC_NR_CLOCKS 3 > > /*--------------------------------------------------------------------------*/ > /* Common Definition */ > @@ -425,6 +426,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[MSDC_NR_CLOCKS]; > u32 mclk; /* mmc subsystem clock frequency */ > u32 src_clk_freq; /* source clock frequency */ > unsigned char timing; > @@ -784,6 +787,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_bulk_disable_unprepare(MSDC_NR_CLOCKS, 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,18 @@ 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(MSDC_NR_CLOCKS, host->bulk_clks); > + if (ret) { > + dev_err(host->dev, "Cannot enable pclk/axi/ahb clock gates\n"); > + return; > + } It's a bit odd that we only care about the last 3 clocks... Should we return early if any of the clocks can't be enabled? Changing the behaviour for the other clocks should be in another patch though. > + > while (!(readl(host->base + MSDC_CFG) & MSDC_CFG_CKSTB)) > cpu_relax(); > } > @@ -2366,6 +2378,48 @@ 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) > +{ > + int ret; > + > + host->src_clk = devm_clk_get_optional(&pdev->dev, "source"); I think you want devm_clk_get, as the previous version of the code does not make this clock optional. > + if (IS_ERR(host->src_clk)) > + return PTR_ERR(host->src_clk); > + > + host->h_clk = devm_clk_get_optional(&pdev->dev, "hclk"); ditto, devm_clk_get > + 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; This is consistent with previous behaviour, but this looks wrong. If the clock exists (!= NULL return value), but you get an error, you should return that error. This belongs in another patch though. > + > + /*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); > + > + host->bulk_clks[0].id = "pclk_cg"; > + host->bulk_clks[1].id = "axi_cg"; > + host->bulk_clks[2].id = "ahb_cg"; > + ret = devm_clk_bulk_get_optional(&pdev->dev, MSDC_NR_CLOCKS, > + host->bulk_clks); > + if (ret) { > + dev_err(&pdev->dev, "Cannot get pclk/axi/ahb clock gates\n"); > + return ret; > + } > + > + return 0; > +} > + > static int msdc_drv_probe(struct platform_device *pdev) > { > struct mmc_host *mmc; > @@ -2405,25 +2459,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
diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c index a704745e5882..41703e6d6b17 100644 --- a/drivers/mmc/host/mtk-sd.c +++ b/drivers/mmc/host/mtk-sd.c @@ -35,6 +35,7 @@ #include "cqhci.h" #define MAX_BD_NUM 1024 +#define MSDC_NR_CLOCKS 3 /*--------------------------------------------------------------------------*/ /* Common Definition */ @@ -425,6 +426,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[MSDC_NR_CLOCKS]; u32 mclk; /* mmc subsystem clock frequency */ u32 src_clk_freq; /* source clock frequency */ unsigned char timing; @@ -784,6 +787,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_bulk_disable_unprepare(MSDC_NR_CLOCKS, 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,18 @@ 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(MSDC_NR_CLOCKS, host->bulk_clks); + if (ret) { + dev_err(host->dev, "Cannot enable pclk/axi/ahb clock gates\n"); + return; + } + while (!(readl(host->base + MSDC_CFG) & MSDC_CFG_CKSTB)) cpu_relax(); } @@ -2366,6 +2378,48 @@ 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) +{ + int ret; + + 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); + + host->bulk_clks[0].id = "pclk_cg"; + host->bulk_clks[1].id = "axi_cg"; + host->bulk_clks[2].id = "ahb_cg"; + ret = devm_clk_bulk_get_optional(&pdev->dev, MSDC_NR_CLOCKS, + host->bulk_clks); + if (ret) { + dev_err(&pdev->dev, "Cannot get pclk/axi/ahb clock gates\n"); + return ret; + } + + return 0; +} + static int msdc_drv_probe(struct platform_device *pdev) { struct mmc_host *mmc; @@ -2405,25 +2459,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");
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 <wenbin.mei@mediatek.com> --- drivers/mmc/host/mtk-sd.c | 74 +++++++++++++++++++++++++++++---------- 1 file changed, 56 insertions(+), 18 deletions(-)