diff mbox series

[3/4] clk: qcom: Add support to LPASS AUDIO_CC Glitch Free Mux clocks

Message ID 20200917132850.7730-4-srinivas.kandagatla@linaro.org (mailing list archive)
State Changes Requested, archived
Headers show
Series clk: qcom : add sm8250 LPASS GFM drivers | expand

Commit Message

Srinivas Kandagatla Sept. 17, 2020, 1:28 p.m. UTC 
GFM Muxes in AUDIO_CC control clocks to LPASS WSA and RX Codec Macros.
This patch adds support to these muxes.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/clk/qcom/Kconfig            |   7 +
 drivers/clk/qcom/Makefile           |   1 +
 drivers/clk/qcom/lpass-gfm-sm8250.c | 235 ++++++++++++++++++++++++++++
 3 files changed, 243 insertions(+)
 create mode 100644 drivers/clk/qcom/lpass-gfm-sm8250.c

Comments

Stephen Boyd Sept. 22, 2020, 6:41 p.m. UTC | #1 
Quoting Srinivas Kandagatla (2020-09-17 06:28:49)
> GFM Muxes in AUDIO_CC control clocks to LPASS WSA and RX Codec Macros.
> This patch adds support to these muxes.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  drivers/clk/qcom/Kconfig            |   7 +
>  drivers/clk/qcom/Makefile           |   1 +
>  drivers/clk/qcom/lpass-gfm-sm8250.c | 235 ++++++++++++++++++++++++++++
>  3 files changed, 243 insertions(+)
>  create mode 100644 drivers/clk/qcom/lpass-gfm-sm8250.c
> 
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 2eda63b7c46c..4e46fd339e62 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -494,4 +494,11 @@ config KRAITCC
>           Support for the Krait CPU clocks on Qualcomm devices.
>           Say Y if you want to support CPU frequency scaling.
>  
> +config CLK_GFM_LPASS_SM8250
> +       tristate "GFM LPASS Clocks"
> +       depends on SND_SOC_QDSP6_COMMON

What is the depends for?

> +       help
> +         Support for the GFM Glitch Free Mux LPASS clock. Say Y
> +         if you want to support GFM Clocks on LPASS for SM8250 SoC.
> +
>  endif
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index 8eb395d02a32..c18e4ead6c9e 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -74,3 +74,4 @@ obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
>  obj-$(CONFIG_KPSS_XCC) += kpss-xcc.o
>  obj-$(CONFIG_QCOM_HFPLL) += hfpll.o
>  obj-$(CONFIG_KRAITCC) += krait-cc.o
> +obj-$(CONFIG_CLK_GFM_LPASS_SM8250) += lpass-gfm-sm8250.o

I don't know but probably this should be sorted by Kconfig instead of
tacked onto the end here.

> diff --git a/drivers/clk/qcom/lpass-gfm-sm8250.c b/drivers/clk/qcom/lpass-gfm-sm8250.c
> new file mode 100644
> index 000000000000..2d5c41ae4969
> --- /dev/null
> +++ b/drivers/clk/qcom/lpass-gfm-sm8250.c
> @@ -0,0 +1,235 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/clk-provider.h>
> +#include <linux/io.h>
> +#include <linux/clk.h>
> +#include <linux/slab.h>
> +#include <linux/err.h>
> +#include <linux/notifier.h>
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_device.h>
> +#include <dt-bindings/clock/qcom,sm8250-lpass-audiocc.h>
> +
> +struct lpass_gfm {
> +       struct device *dev;
> +       void __iomem *base;
> +       struct clk *core_vote;
> +       struct clk *bus_clk;
> +};
> +
> +struct clk_gfm {
> +       unsigned int mux_reg;
> +       unsigned int mux_mask;
> +       struct clk_hw   hw;
> +       struct lpass_gfm *priv;
> +       void __iomem *gfm_mux;
> +};
> +
> +#define GFM_MASK       BIT(1)
> +#define to_clk_gfm(_hw) container_of(_hw, struct clk_gfm, hw)
> +
> +static u8 clk_gfm_get_parent(struct clk_hw *hw)
> +{
> +       struct clk_gfm *clk = to_clk_gfm(hw);
> +
> +       return readl(clk->gfm_mux) & GFM_MASK;
> +}
> +
> +static int clk_gfm_set_parent(struct clk_hw *hw, u8 index)
> +{
> +       struct clk_gfm *clk = to_clk_gfm(hw);
> +       unsigned int val;
> +
> +       val = readl(clk->gfm_mux);
> +
> +       if (index)
> +               val |= GFM_MASK;
> +       else
> +               val &= ~GFM_MASK;
> +
> +       writel(val, clk->gfm_mux);
> +
> +       return 0;
> +}
> +
> +static const struct clk_ops clk_gfm_ops = {
> +       .get_parent = clk_gfm_get_parent,
> +       .set_parent = clk_gfm_set_parent,
> +       .determine_rate = __clk_mux_determine_rate,
> +};
> +
> +static struct clk_gfm lpass_gfm_wsa_mclk = {
> +       .mux_reg = 0x220d8,
> +       .mux_mask = BIT(0),
> +       .hw.init = &(struct clk_init_data) {
> +               .name = "WSA_MCLK",
> +               .ops = &clk_gfm_ops,
> +               .flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
> +               .parent_names = (const char *[]){
> +                       "LPASS_CLK_ID_TX_CORE_MCLK",
> +                       "LPASS_CLK_ID_WSA_CORE_MCLK",
> +               },
> +               .parent_data = (const struct clk_parent_data[]){
> +                               { .index = 0 },
> +                               { .index = 1 },
> +               },
> +               .num_parents = 2,
> +       },
> +};
> +
> +static struct clk_gfm lpass_gfm_wsa_npl = {
> +       .mux_reg = 0x220d8,
> +       .mux_mask = BIT(0),
> +       .hw.init = &(struct clk_init_data) {
> +               .name = "WSA_NPL",
> +               .ops = &clk_gfm_ops,
> +               .flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
> +               .parent_names = (const char *[]){
> +                       "LPASS_CLK_ID_TX_CORE_NPL_MCLK",
> +                       "LPASS_CLK_ID_WSA_CORE_NPL_MCLK",
> +               },
> +               .parent_data = (const struct clk_parent_data[]){
> +                               { .index = 0 },
> +                               { .index = 1 },
> +               },
> +               .num_parents = 2,
> +       },
> +};
> +
> +static struct clk_gfm lpass_gfm_rx_mclk_mclk2 = {
> +       .mux_reg = 0x240d8,
> +       .mux_mask = BIT(0),
> +       .hw.init = &(struct clk_init_data) {
> +               .name = "RX_MCLK_MCLK2",
> +               .ops = &clk_gfm_ops,
> +               .flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
> +               .parent_names = (const char *[]){
> +                       "LPASS_CLK_ID_TX_CORE_MCLK",
> +                       "LPASS_CLK_ID_RX_CORE_MCLK",
> +               },
> +               .parent_data = (const struct clk_parent_data[]){
> +                               { .index = 0 },
> +                               { .index = 1 },
> +               },
> +               .num_parents = 2,
> +       },
> +};
> +
> +static struct clk_gfm lpass_gfm_rx_npl = {
> +       .mux_reg = 0x240d8,
> +       .mux_mask = BIT(0),
> +       .hw.init = &(struct clk_init_data) {
> +               .name = "RX_NPL",
> +               .ops = &clk_gfm_ops,
> +               .flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
> +               .parent_names = (const char *[]){
> +                       "LPASS_CLK_ID_TX_CORE_NPL_MCLK",
> +                       "LPASS_CLK_ID_RX_CORE_NPL_MCLK",
> +               },
> +               .parent_data = (const struct clk_parent_data[]){
> +                               { .index = 0 },
> +                               { .index = 1 },
> +               },

Please use parent_data instead of parent_names. The name can go in the
parent_data struct.

> +               .num_parents = 2,
> +       },
> +};
> +
> +static struct clk_gfm *audiocc_gfm_clks[] = {
> +       [LPASS_CDC_WSA_NPL]             = &lpass_gfm_wsa_npl,
> +       [LPASS_CDC_WSA_MCLK]            = &lpass_gfm_wsa_mclk,
> +       [LPASS_CDC_RX_NPL]              = &lpass_gfm_rx_npl,
> +       [LPASS_CDC_RX_MCLK_MCLK2]       = &lpass_gfm_rx_mclk_mclk2,
> +};
> +
> +static struct clk_hw_onecell_data audiocc_hw_onecell_data = {
> +       .hws = {
> +               [LPASS_CDC_WSA_NPL]     = &lpass_gfm_wsa_npl.hw,
> +               [LPASS_CDC_WSA_MCLK]    = &lpass_gfm_wsa_mclk.hw,
> +               [LPASS_CDC_RX_NPL]      = &lpass_gfm_rx_npl.hw,
> +               [LPASS_CDC_RX_MCLK_MCLK2] = &lpass_gfm_rx_mclk_mclk2.hw,
> +       },
> +       .num = ARRAY_SIZE(audiocc_gfm_clks),
> +};
> +
> +struct lpass_gfm_data {
> +       struct clk_hw_onecell_data *onecell_data;
> +       struct clk_gfm **gfm_clks;
> +};
> +
> +static struct lpass_gfm_data audiocc_data = {
> +       .onecell_data = &audiocc_hw_onecell_data,
> +       .gfm_clks = audiocc_gfm_clks,
> +};
> +
> +static int lpass_gfm_clk_driver_probe(struct platform_device *pdev)
> +{
> +       const struct lpass_gfm_data *data;
> +       struct device *dev = &pdev->dev;
> +       struct resource *res;
> +       struct clk_gfm *gfm;
> +       struct lpass_gfm *cc;
> +       int err, i;
> +
> +       cc = devm_kzalloc(dev, sizeof(*cc), GFP_KERNEL);
> +       if (!cc)
> +               return -ENOMEM;
> +
> +       cc->core_vote = devm_clk_get(&pdev->dev, "core");
> +       if (IS_ERR(cc->core_vote)) {
> +               dev_dbg(dev, "Failed to get lpass core clk\n");
> +               return PTR_ERR(cc->core_vote);
> +       }

Can this use the pm_clk stuff?

> +
> +       data = of_device_get_match_data(dev);

What if data is NULL?

> +       cc->bus_clk = devm_clk_get(&pdev->dev, "bus");
> +       if (IS_ERR(cc->bus_clk)) {
> +               dev_dbg(dev, "Failed to get lpass bus clk\n");
> +               return PTR_ERR(cc->bus_clk);
> +       }
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       cc->base = devm_ioremap_resource(dev, res);
> +       if (IS_ERR(cc->base))
> +               return PTR_ERR(cc->base);
> +
> +       clk_prepare_enable(cc->core_vote);
> +       clk_prepare_enable(cc->bus_clk);

These will always be left enabled if the probe function bails out after here.

> +
> +       for (i = 0; i < data->onecell_data->num; i++) {
> +               if (!data->gfm_clks[i])
> +                       continue;
> +
> +               gfm = data->gfm_clks[i];
> +               gfm->priv = cc;
> +               gfm->gfm_mux = cc->base;
> +               gfm->gfm_mux = gfm->gfm_mux + data->gfm_clks[i]->mux_reg;
> +
> +               err = devm_clk_hw_register(dev, &data->gfm_clks[i]->hw);
> +               if (err)
> +                       return err;
> +
> +       }
> +
> +       return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
> +                                          data->onecell_data);
> +}
> +
> +static const struct of_device_id lpass_gfm_clk_match_table[] = {
> +       {
> +               .compatible = "qcom,sm8250-lpass-audiocc",
> +               .data = &audiocc_data,
> +       },
> +       { }
> +};
> +
> +static struct platform_driver lpass_gfm_clk_driver = {
> +       .probe          = lpass_gfm_clk_driver_probe,
> +       .driver         = {
> +               .name   = "lpass-gfm-clk",
> +               .of_match_table = lpass_gfm_clk_match_table,
> +       },
> +};
> +builtin_platform_driver(lpass_gfm_clk_driver);

Should be module_platform_driver()?
Srinivas Kandagatla Sept. 22, 2020, 9:45 p.m. UTC | #2 
Thanks Stephen for review,

On 22/09/2020 19:41, Stephen Boyd wrote:
> Quoting Srinivas Kandagatla (2020-09-17 06:28:49)
>> GFM Muxes in AUDIO_CC control clocks to LPASS WSA and RX Codec Macros.
>> This patch adds support to these muxes.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>>   drivers/clk/qcom/Kconfig            |   7 +
>>   drivers/clk/qcom/Makefile           |   1 +
>>   drivers/clk/qcom/lpass-gfm-sm8250.c | 235 ++++++++++++++++++++++++++++
>>   3 files changed, 243 insertions(+)
>>   create mode 100644 drivers/clk/qcom/lpass-gfm-sm8250.c
>>
>> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
>> index 2eda63b7c46c..4e46fd339e62 100644
>> --- a/drivers/clk/qcom/Kconfig
>> +++ b/drivers/clk/qcom/Kconfig
>> @@ -494,4 +494,11 @@ config KRAITCC
>>            Support for the Krait CPU clocks on Qualcomm devices.
>>            Say Y if you want to support CPU frequency scaling.
>>   
>> +config CLK_GFM_LPASS_SM8250
>> +       tristate "GFM LPASS Clocks"
>> +       depends on SND_SOC_QDSP6_COMMON
> 
> What is the depends for?
I think I forgot to remove this!

> 
>> +       help
>> +         Support for the GFM Glitch Free Mux LPASS clock. Say Y
>> +         if you want to support GFM Clocks on LPASS for SM8250 SoC.
>> +
>>   endif
>> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
>> index 8eb395d02a32..c18e4ead6c9e 100644
>> --- a/drivers/clk/qcom/Makefile
>> +++ b/drivers/clk/qcom/Makefile
>> @@ -74,3 +74,4 @@ obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
>>   obj-$(CONFIG_KPSS_XCC) += kpss-xcc.o
>>   obj-$(CONFIG_QCOM_HFPLL) += hfpll.o
>>   obj-$(CONFIG_KRAITCC) += krait-cc.o
>> +obj-$(CONFIG_CLK_GFM_LPASS_SM8250) += lpass-gfm-sm8250.o
> 
> I don't know but probably this should be sorted by Kconfig instead of
> tacked onto the end here.
> 
>> diff --git a/drivers/clk/qcom/lpass-gfm-sm8250.c b/drivers/clk/qcom/lpass-gfm-sm8250.c
>> new file mode 100644
>> index 000000000000..2d5c41ae4969
>> --- /dev/null
>> +++ b/drivers/clk/qcom/lpass-gfm-sm8250.c
>> @@ -0,0 +1,235 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/io.h>
>> +#include <linux/clk.h>
>> +#include <linux/slab.h>
>> +#include <linux/err.h>
>> +#include <linux/notifier.h>
>> +#include <linux/device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/of_device.h>
>> +#include <dt-bindings/clock/qcom,sm8250-lpass-audiocc.h>
>> +
>> +struct lpass_gfm {
>> +       struct device *dev;
>> +       void __iomem *base;
>> +       struct clk *core_vote;
>> +       struct clk *bus_clk;
>> +};
>> +
>> +struct clk_gfm {
>> +       unsigned int mux_reg;
>> +       unsigned int mux_mask;
>> +       struct clk_hw   hw;
>> +       struct lpass_gfm *priv;
>> +       void __iomem *gfm_mux;
>> +};
>> +
...
>> +static struct clk_gfm lpass_gfm_rx_npl = {
>> +       .mux_reg = 0x240d8,
>> +       .mux_mask = BIT(0),
>> +       .hw.init = &(struct clk_init_data) {
>> +               .name = "RX_NPL",
>> +               .ops = &clk_gfm_ops,
>> +               .flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
>> +               .parent_names = (const char *[]){
>> +                       "LPASS_CLK_ID_TX_CORE_NPL_MCLK",
>> +                       "LPASS_CLK_ID_RX_CORE_NPL_MCLK",
>> +               },
>> +               .parent_data = (const struct clk_parent_data[]){
>> +                               { .index = 0 },
>> +                               { .index = 1 },
>> +               },
> 
> Please use parent_data instead of parent_names. The name can go in the
> parent_data struct.

Ah.. I missed that, will fix that in next version!

> 
>> +               .num_parents = 2,
>> +       },
>> +};
>> +
>> +static struct clk_gfm *audiocc_gfm_clks[] = {
>> +       [LPASS_CDC_WSA_NPL]             = &lpass_gfm_wsa_npl,
>> +       [LPASS_CDC_WSA_MCLK]            = &lpass_gfm_wsa_mclk,
>> +       [LPASS_CDC_RX_NPL]              = &lpass_gfm_rx_npl,
>> +       [LPASS_CDC_RX_MCLK_MCLK2]       = &lpass_gfm_rx_mclk_mclk2,
>> +};
>> +
>> +static struct clk_hw_onecell_data audiocc_hw_onecell_data = {
>> +       .hws = {
>> +               [LPASS_CDC_WSA_NPL]     = &lpass_gfm_wsa_npl.hw,
>> +               [LPASS_CDC_WSA_MCLK]    = &lpass_gfm_wsa_mclk.hw,
>> +               [LPASS_CDC_RX_NPL]      = &lpass_gfm_rx_npl.hw,
>> +               [LPASS_CDC_RX_MCLK_MCLK2] = &lpass_gfm_rx_mclk_mclk2.hw,
>> +       },
>> +       .num = ARRAY_SIZE(audiocc_gfm_clks),
>> +};
>> +
>> +struct lpass_gfm_data {
>> +       struct clk_hw_onecell_data *onecell_data;
>> +       struct clk_gfm **gfm_clks;
>> +};
>> +
>> +static struct lpass_gfm_data audiocc_data = {
>> +       .onecell_data = &audiocc_hw_onecell_data,
>> +       .gfm_clks = audiocc_gfm_clks,
>> +};
>> +
>> +static int lpass_gfm_clk_driver_probe(struct platform_device *pdev)
>> +{
>> +       const struct lpass_gfm_data *data;
>> +       struct device *dev = &pdev->dev;
>> +       struct resource *res;
>> +       struct clk_gfm *gfm;
>> +       struct lpass_gfm *cc;
>> +       int err, i;
>> +
>> +       cc = devm_kzalloc(dev, sizeof(*cc), GFP_KERNEL);
>> +       if (!cc)
>> +               return -ENOMEM;
>> +
>> +       cc->core_vote = devm_clk_get(&pdev->dev, "core");
>> +       if (IS_ERR(cc->core_vote)) {
>> +               dev_dbg(dev, "Failed to get lpass core clk\n");
>> +               return PTR_ERR(cc->core_vote);
>> +       }
> 
> Can this use the pm_clk stuff?

you mean add runtime pm support or something else?

I can give it a go and see!


> 
>> +
>> +       data = of_device_get_match_data(dev);
> 
> What if data is NULL?
It should not be here if there is no match of compatible string, so data 
should not be NULL!

> 
>> +       cc->bus_clk = devm_clk_get(&pdev->dev, "bus");
>> +       if (IS_ERR(cc->bus_clk)) {
>> +               dev_dbg(dev, "Failed to get lpass bus clk\n");
>> +               return PTR_ERR(cc->bus_clk);
>> +       }
>> +
>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +       cc->base = devm_ioremap_resource(dev, res);
>> +       if (IS_ERR(cc->base))
>> +               return PTR_ERR(cc->base);
>> +
>> +       clk_prepare_enable(cc->core_vote);
>> +       clk_prepare_enable(cc->bus_clk);
> 
> These will always be left enabled if the probe function bails out after here.
> 
I agree, Will add some error handling
>> +
>> +       for (i = 0; i < data->onecell_data->num; i++) {
>> +               if (!data->gfm_clks[i])
>> +                       continue;
>> +
>> +               gfm = data->gfm_clks[i];
>> +               gfm->priv = cc;
>> +               gfm->gfm_mux = cc->base;
>> +               gfm->gfm_mux = gfm->gfm_mux + data->gfm_clks[i]->mux_reg;
>> +
>> +               err = devm_clk_hw_register(dev, &data->gfm_clks[i]->hw);
>> +               if (err)
>> +                       return err;
>> +
>> +       }
>> +
>> +       return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
>> +                                          data->onecell_data);
>> +}
>> +
>> +static const struct of_device_id lpass_gfm_clk_match_table[] = {
>> +       {
>> +               .compatible = "qcom,sm8250-lpass-audiocc",
>> +               .data = &audiocc_data,
>> +       },
>> +       { }
>> +};
>> +
>> +static struct platform_driver lpass_gfm_clk_driver = {
>> +       .probe          = lpass_gfm_clk_driver_probe,
>> +       .driver         = {
>> +               .name   = "lpass-gfm-clk",
>> +               .of_match_table = lpass_gfm_clk_match_table,
>> +       },
>> +};
>> +builtin_platform_driver(lpass_gfm_clk_driver);
> 
> Should be module_platform_driver()?
sure, will do that!

>
Dmitry Baryshkov Sept. 22, 2020, 9:49 p.m. UTC | #3 
On 17/09/2020 16:28, Srinivas Kandagatla wrote:
> GFM Muxes in AUDIO_CC control clocks to LPASS WSA and RX Codec Macros.
> This patch adds support to these muxes.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>   drivers/clk/qcom/Kconfig            |   7 +
>   drivers/clk/qcom/Makefile           |   1 +
>   drivers/clk/qcom/lpass-gfm-sm8250.c | 235 ++++++++++++++++++++++++++++
>   3 files changed, 243 insertions(+)
>   create mode 100644 drivers/clk/qcom/lpass-gfm-sm8250.c
> 
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 2eda63b7c46c..4e46fd339e62 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -494,4 +494,11 @@ config KRAITCC
>   	  Support for the Krait CPU clocks on Qualcomm devices.
>   	  Say Y if you want to support CPU frequency scaling.
>   
> +config CLK_GFM_LPASS_SM8250
> +	tristate "GFM LPASS Clocks"

Since this driver can be built as module, few things are missing (see 
below).

> +	depends on SND_SOC_QDSP6_COMMON
> +	help
> +	  Support for the GFM Glitch Free Mux LPASS clock. Say Y
> +	  if you want to support GFM Clocks on LPASS for SM8250 SoC.
> +

[skipped]

> +
> +static const struct of_device_id lpass_gfm_clk_match_table[] = {
> +	{
> +		.compatible = "qcom,sm8250-lpass-audiocc",
> +		.data = &audiocc_data,
> +	},
> +	{ }
> +};

MODULE_DEVICE_TABLE(of, lpass_gfm_clk_match_table);

> +
> +static struct platform_driver lpass_gfm_clk_driver = {
> +	.probe		= lpass_gfm_clk_driver_probe,
> +	.driver		= {
> +		.name	= "lpass-gfm-clk",
> +		.of_match_table = lpass_gfm_clk_match_table,
> +	},
> +};
> +builtin_platform_driver(lpass_gfm_clk_driver);


Wouldn't you like to use module_platform_driver() here, like other LPASS 
CC drivers do?

Also MODULE_LICENSE is missing.
Stephen Boyd Sept. 23, 2020, 11:28 p.m. UTC | #4 
Quoting Srinivas Kandagatla (2020-09-22 14:45:07)
> Thanks Stephen for review,
> 
> On 22/09/2020 19:41, Stephen Boyd wrote:
> > Quoting Srinivas Kandagatla (2020-09-17 06:28:49)
> >> diff --git a/drivers/clk/qcom/lpass-gfm-sm8250.c b/drivers/clk/qcom/lpass-gfm-sm8250.c
> >> new file mode 100644
> >> index 000000000000..2d5c41ae4969
> >> --- /dev/null
> >> +++ b/drivers/clk/qcom/lpass-gfm-sm8250.c
> >> @@ -0,0 +1,235 @@
[...]
> >> +};
> >> +
> >> +static int lpass_gfm_clk_driver_probe(struct platform_device *pdev)
> >> +{
> >> +       const struct lpass_gfm_data *data;
> >> +       struct device *dev = &pdev->dev;
> >> +       struct resource *res;
> >> +       struct clk_gfm *gfm;
> >> +       struct lpass_gfm *cc;
> >> +       int err, i;
> >> +
> >> +       cc = devm_kzalloc(dev, sizeof(*cc), GFP_KERNEL);
> >> +       if (!cc)
> >> +               return -ENOMEM;
> >> +
> >> +       cc->core_vote = devm_clk_get(&pdev->dev, "core");
> >> +       if (IS_ERR(cc->core_vote)) {
> >> +               dev_dbg(dev, "Failed to get lpass core clk\n");
> >> +               return PTR_ERR(cc->core_vote);
> >> +       }
> > 
> > Can this use the pm_clk stuff?
> 
> you mean add runtime pm support or something else?
> 
> I can give it a go and see!
> 

Yes use the runtime PM support and pm_clk APIs. There are some examples
in this directory already.

> 
> > 
> >> +
> >> +       data = of_device_get_match_data(dev);
> > 
> > What if data is NULL?
> It should not be here if there is no match of compatible string, so data 
> should not be NULL!

Ok sure but it always makes me feel better if we check for a valid
pointer with this API.
diff mbox series

Patch

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 2eda63b7c46c..4e46fd339e62 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -494,4 +494,11 @@  config KRAITCC
 	  Support for the Krait CPU clocks on Qualcomm devices.
 	  Say Y if you want to support CPU frequency scaling.
 
+config CLK_GFM_LPASS_SM8250
+	tristate "GFM LPASS Clocks"
+	depends on SND_SOC_QDSP6_COMMON
+	help
+	  Support for the GFM Glitch Free Mux LPASS clock. Say Y
+	  if you want to support GFM Clocks on LPASS for SM8250 SoC.
+
 endif
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 8eb395d02a32..c18e4ead6c9e 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -74,3 +74,4 @@  obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
 obj-$(CONFIG_KPSS_XCC) += kpss-xcc.o
 obj-$(CONFIG_QCOM_HFPLL) += hfpll.o
 obj-$(CONFIG_KRAITCC) += krait-cc.o
+obj-$(CONFIG_CLK_GFM_LPASS_SM8250) += lpass-gfm-sm8250.o
diff --git a/drivers/clk/qcom/lpass-gfm-sm8250.c b/drivers/clk/qcom/lpass-gfm-sm8250.c
new file mode 100644
index 000000000000..2d5c41ae4969
--- /dev/null
+++ b/drivers/clk/qcom/lpass-gfm-sm8250.c
@@ -0,0 +1,235 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/notifier.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/of_device.h>
+#include <dt-bindings/clock/qcom,sm8250-lpass-audiocc.h>
+
+struct lpass_gfm {
+	struct device *dev;
+	void __iomem *base;
+	struct clk *core_vote;
+	struct clk *bus_clk;
+};
+
+struct clk_gfm {
+	unsigned int mux_reg;
+	unsigned int mux_mask;
+	struct clk_hw	hw;
+	struct lpass_gfm *priv;
+	void __iomem *gfm_mux;
+};
+
+#define GFM_MASK	BIT(1)
+#define to_clk_gfm(_hw) container_of(_hw, struct clk_gfm, hw)
+
+static u8 clk_gfm_get_parent(struct clk_hw *hw)
+{
+	struct clk_gfm *clk = to_clk_gfm(hw);
+
+	return readl(clk->gfm_mux) & GFM_MASK;
+}
+
+static int clk_gfm_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct clk_gfm *clk = to_clk_gfm(hw);
+	unsigned int val;
+
+	val = readl(clk->gfm_mux);
+
+	if (index)
+		val |= GFM_MASK;
+	else
+		val &= ~GFM_MASK;
+
+	writel(val, clk->gfm_mux);
+
+	return 0;
+}
+
+static const struct clk_ops clk_gfm_ops = {
+	.get_parent = clk_gfm_get_parent,
+	.set_parent = clk_gfm_set_parent,
+	.determine_rate = __clk_mux_determine_rate,
+};
+
+static struct clk_gfm lpass_gfm_wsa_mclk = {
+	.mux_reg = 0x220d8,
+	.mux_mask = BIT(0),
+	.hw.init = &(struct clk_init_data) {
+		.name = "WSA_MCLK",
+		.ops = &clk_gfm_ops,
+		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
+		.parent_names = (const char *[]){
+			"LPASS_CLK_ID_TX_CORE_MCLK",
+			"LPASS_CLK_ID_WSA_CORE_MCLK",
+		},
+		.parent_data = (const struct clk_parent_data[]){
+				{ .index = 0 },
+				{ .index = 1 },
+		},
+		.num_parents = 2,
+	},
+};
+
+static struct clk_gfm lpass_gfm_wsa_npl = {
+	.mux_reg = 0x220d8,
+	.mux_mask = BIT(0),
+	.hw.init = &(struct clk_init_data) {
+		.name = "WSA_NPL",
+		.ops = &clk_gfm_ops,
+		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
+		.parent_names = (const char *[]){
+			"LPASS_CLK_ID_TX_CORE_NPL_MCLK",
+			"LPASS_CLK_ID_WSA_CORE_NPL_MCLK",
+		},
+		.parent_data = (const struct clk_parent_data[]){
+				{ .index = 0 },
+				{ .index = 1 },
+		},
+		.num_parents = 2,
+	},
+};
+
+static struct clk_gfm lpass_gfm_rx_mclk_mclk2 = {
+	.mux_reg = 0x240d8,
+	.mux_mask = BIT(0),
+	.hw.init = &(struct clk_init_data) {
+		.name = "RX_MCLK_MCLK2",
+		.ops = &clk_gfm_ops,
+		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
+		.parent_names = (const char *[]){
+			"LPASS_CLK_ID_TX_CORE_MCLK",
+			"LPASS_CLK_ID_RX_CORE_MCLK",
+		},
+		.parent_data = (const struct clk_parent_data[]){
+				{ .index = 0 },
+				{ .index = 1 },
+		},
+		.num_parents = 2,
+	},
+};
+
+static struct clk_gfm lpass_gfm_rx_npl = {
+	.mux_reg = 0x240d8,
+	.mux_mask = BIT(0),
+	.hw.init = &(struct clk_init_data) {
+		.name = "RX_NPL",
+		.ops = &clk_gfm_ops,
+		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
+		.parent_names = (const char *[]){
+			"LPASS_CLK_ID_TX_CORE_NPL_MCLK",
+			"LPASS_CLK_ID_RX_CORE_NPL_MCLK",
+		},
+		.parent_data = (const struct clk_parent_data[]){
+				{ .index = 0 },
+				{ .index = 1 },
+		},
+		.num_parents = 2,
+	},
+};
+
+static struct clk_gfm *audiocc_gfm_clks[] = {
+	[LPASS_CDC_WSA_NPL]		= &lpass_gfm_wsa_npl,
+	[LPASS_CDC_WSA_MCLK]		= &lpass_gfm_wsa_mclk,
+	[LPASS_CDC_RX_NPL]		= &lpass_gfm_rx_npl,
+	[LPASS_CDC_RX_MCLK_MCLK2]	= &lpass_gfm_rx_mclk_mclk2,
+};
+
+static struct clk_hw_onecell_data audiocc_hw_onecell_data = {
+	.hws = {
+		[LPASS_CDC_WSA_NPL]	= &lpass_gfm_wsa_npl.hw,
+		[LPASS_CDC_WSA_MCLK]	= &lpass_gfm_wsa_mclk.hw,
+		[LPASS_CDC_RX_NPL]	= &lpass_gfm_rx_npl.hw,
+		[LPASS_CDC_RX_MCLK_MCLK2] = &lpass_gfm_rx_mclk_mclk2.hw,
+	},
+	.num = ARRAY_SIZE(audiocc_gfm_clks),
+};
+
+struct lpass_gfm_data {
+	struct clk_hw_onecell_data *onecell_data;
+	struct clk_gfm **gfm_clks;
+};
+
+static struct lpass_gfm_data audiocc_data = {
+	.onecell_data = &audiocc_hw_onecell_data,
+	.gfm_clks = audiocc_gfm_clks,
+};
+
+static int lpass_gfm_clk_driver_probe(struct platform_device *pdev)
+{
+	const struct lpass_gfm_data *data;
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	struct clk_gfm *gfm;
+	struct lpass_gfm *cc;
+	int err, i;
+
+	cc = devm_kzalloc(dev, sizeof(*cc), GFP_KERNEL);
+	if (!cc)
+		return -ENOMEM;
+
+	cc->core_vote = devm_clk_get(&pdev->dev, "core");
+	if (IS_ERR(cc->core_vote)) {
+		dev_dbg(dev, "Failed to get lpass core clk\n");
+		return PTR_ERR(cc->core_vote);
+	}
+
+	data = of_device_get_match_data(dev);
+	cc->bus_clk = devm_clk_get(&pdev->dev, "bus");
+	if (IS_ERR(cc->bus_clk)) {
+		dev_dbg(dev, "Failed to get lpass bus clk\n");
+		return PTR_ERR(cc->bus_clk);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	cc->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(cc->base))
+		return PTR_ERR(cc->base);
+
+	clk_prepare_enable(cc->core_vote);
+	clk_prepare_enable(cc->bus_clk);
+
+	for (i = 0; i < data->onecell_data->num; i++) {
+		if (!data->gfm_clks[i])
+			continue;
+
+		gfm = data->gfm_clks[i];
+		gfm->priv = cc;
+		gfm->gfm_mux = cc->base;
+		gfm->gfm_mux = gfm->gfm_mux + data->gfm_clks[i]->mux_reg;
+
+		err = devm_clk_hw_register(dev, &data->gfm_clks[i]->hw);
+		if (err)
+			return err;
+
+	}
+
+	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
+					   data->onecell_data);
+}
+
+static const struct of_device_id lpass_gfm_clk_match_table[] = {
+	{
+		.compatible = "qcom,sm8250-lpass-audiocc",
+		.data = &audiocc_data,
+	},
+	{ }
+};
+
+static struct platform_driver lpass_gfm_clk_driver = {
+	.probe		= lpass_gfm_clk_driver_probe,
+	.driver		= {
+		.name	= "lpass-gfm-clk",
+		.of_match_table = lpass_gfm_clk_match_table,
+	},
+};
+builtin_platform_driver(lpass_gfm_clk_driver);