[PATCH 6/8] clk/qcom: use function pointers for enable and set_rate

Sumit Garg sumit.garg at linaro.org
Fri Oct 27 13:06:25 CEST 2023


On Wed, 25 Oct 2023 at 01:55, Caleb Connolly <caleb.connolly at linaro.org> wrote:
>
> Currently, it isn't possible to build clock drivers for more than one
> platform due to how the msm_enable() and msm_set_rate() callbacks are
> implemented.
>
> Extend qcom_cc_data to include function pointers for these and convert
> all platforms to use them.
>
> Previously, clock drivers relied on common.h to include the board

No it's not included via common.h but rather via target specific
include file like: include/configs/sdm845.h.

> specific sysmap header, include those explicitly to further reduce the
> dependency between the clock driver and a particular target
> configuration.

If you really need to remove that dependency then the clock specific
macros need to move out of the system map header as a separate header
in drivers/clk/qcom/.

>
> Signed-off-by: Caleb Connolly <caleb.connolly at linaro.org>
> ---
>  drivers/clk/qcom/clock-apq8016.c | 12 ++++++------
>  drivers/clk/qcom/clock-apq8096.c | 12 ++++++------
>  drivers/clk/qcom/clock-ipq4019.c |  6 ++++--
>  drivers/clk/qcom/clock-qcom.c    | 17 ++++++++++++-----
>  drivers/clk/qcom/clock-qcom.h    |  5 +++++
>  drivers/clk/qcom/clock-qcs404.c  |  5 ++++-
>  drivers/clk/qcom/clock-sdm845.c  | 12 ++++++++----
>  7 files changed, 45 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/clk/qcom/clock-apq8016.c b/drivers/clk/qcom/clock-apq8016.c
> index e3b9b8c1b91b..f74f7a0f5ad9 100644
> --- a/drivers/clk/qcom/clock-apq8016.c
> +++ b/drivers/clk/qcom/clock-apq8016.c
> @@ -13,6 +13,7 @@
>  #include <errno.h>
>  #include <asm/io.h>
>  #include <linux/bitops.h>
> +#include <mach/sysmap-apq8016.h>
>
>  #include "clock-qcom.h"
>
> @@ -94,7 +95,7 @@ static int clk_init_uart(struct qcom_cc_priv *priv)
>         return 0;
>  }
>
> -ulong msm_set_rate(struct clk *clk, ulong rate)
> +static ulong apq8016_set_rate(struct clk *clk, ulong rate)

I would rather prefer it to be renamed as: <soc>_clk_set_rate() which
is self descriptive.

>  {
>         struct qcom_cc_priv *priv = dev_get_priv(clk->dev);
>
> @@ -113,15 +114,14 @@ ulong msm_set_rate(struct clk *clk, ulong rate)
>         }
>  }
>
> -int msm_enable(struct clk *clk)
> -{
> -       return 0;
> -}
> +static struct qcom_cc_data apq8016_data = {
> +       .set_rate = apq8016_set_rate,
> +};
>
>  static const struct udevice_id gcc_apq8016_of_match[] = {
>         {
>                 .compatible = "qcom,gcc-apq8016",
> -               /* TODO: add reset map */
> +               .data = (ulong)&apq8016_data,
>         },
>         { }
>  };
> diff --git a/drivers/clk/qcom/clock-apq8096.c b/drivers/clk/qcom/clock-apq8096.c
> index dc64d11ca979..1efb6e2313a5 100644
> --- a/drivers/clk/qcom/clock-apq8096.c
> +++ b/drivers/clk/qcom/clock-apq8096.c
> @@ -13,6 +13,7 @@
>  #include <errno.h>
>  #include <asm/io.h>
>  #include <linux/bitops.h>
> +#include <mach/sysmap-apq8096.h>
>
>  #include "clock-qcom.h"
>
> @@ -80,7 +81,7 @@ static int clk_init_uart(struct qcom_cc_priv *priv)
>         return 0;
>  }
>
> -ulong msm_set_rate(struct clk *clk, ulong rate)
> +static ulong apq8096_set_rate(struct clk *clk, ulong rate)
>  {
>         struct qcom_cc_priv *priv = dev_get_priv(clk->dev);
>
> @@ -95,15 +96,14 @@ ulong msm_set_rate(struct clk *clk, ulong rate)
>         }
>  }
>
> -int msm_enable(struct clk *clk)
> -{
> -       return 0;
> -}
> +static struct qcom_cc_data apq8096_data = {
> +       .set_rate = apq8096_set_rate,
> +};
>
>  static const struct udevice_id gcc_apq8096_of_match[] = {
>         {
>                 .compatible = "qcom,gcc-apq8096",
> -               /* TODO: add reset map */
> +               .data = (ulong)&apq8096_data,
>         },
>         { }
>  };
> diff --git a/drivers/clk/qcom/clock-ipq4019.c b/drivers/clk/qcom/clock-ipq4019.c
> index 6636af98132d..d42b32c3afd3 100644
> --- a/drivers/clk/qcom/clock-ipq4019.c
> +++ b/drivers/clk/qcom/clock-ipq4019.c
> @@ -17,7 +17,7 @@
>
>  #include "clock-qcom.h"
>
> -ulong msm_set_rate(struct clk *clk, ulong rate)
> +static ulong ipq4019_set_rate(struct clk *clk, ulong rate)
>  {
>         switch (clk->id) {
>         case GCC_BLSP1_UART1_APPS_CLK: /*UART1*/
> @@ -28,7 +28,7 @@ ulong msm_set_rate(struct clk *clk, ulong rate)
>         }
>  }
>
> -int msm_enable(struct clk *clk)
> +static int ipq4019_enable(struct clk *clk)

Ditto, <soc>_clk_enable().

-Sumit

>  {
>         switch (clk->id) {
>         case GCC_BLSP1_QUP1_SPI_APPS_CLK: /*SPI1*/
> @@ -125,6 +125,8 @@ static const struct qcom_reset_map gcc_ipq4019_resets[] = {
>  };
>
>  static struct qcom_cc_data ipq4019_data = {
> +       .enable = ipq4019_enable,
> +       .set_rate = ipq4019_set_rate,
>         .resets = gcc_ipq4019_resets,
>         .num_resets = ARRAY_SIZE(gcc_ipq4019_resets),
>  };
> diff --git a/drivers/clk/qcom/clock-qcom.c b/drivers/clk/qcom/clock-qcom.c
> index b0416a05789d..a9602d0c9ca6 100644
> --- a/drivers/clk/qcom/clock-qcom.c
> +++ b/drivers/clk/qcom/clock-qcom.c
> @@ -28,9 +28,6 @@
>  #define CBCR_BRANCH_ENABLE_BIT  BIT(0)
>  #define CBCR_BRANCH_OFF_BIT     BIT(31)
>
> -extern ulong msm_set_rate(struct clk *clk, ulong rate);
> -extern int msm_enable(struct clk *clk);
> -
>  /* Enable clock controlled by CBC soft macro */
>  void clk_enable_cbc(phys_addr_t cbcr)
>  {
> @@ -160,12 +157,22 @@ static int msm_clk_probe(struct udevice *dev)
>
>  static ulong msm_clk_set_rate(struct clk *clk, ulong rate)
>  {
> -       return msm_set_rate(clk, rate);
> +       struct qcom_cc_data *data = (struct qcom_cc_data *)dev_get_driver_data(clk->dev);
> +
> +       if (data->set_rate)
> +               return data->set_rate(clk, rate);
> +
> +       return 0;
>  }
>
>  static int msm_clk_enable(struct clk *clk)
>  {
> -       return msm_enable(clk);
> +       struct qcom_cc_data *data = (struct qcom_cc_data *)dev_get_driver_data(clk->dev);
> +
> +       if (data->enable)
> +               return data->enable(clk);
> +
> +       return 0;
>  }
>
>  static struct clk_ops msm_clk_ops = {
> diff --git a/drivers/clk/qcom/clock-qcom.h b/drivers/clk/qcom/clock-qcom.h
> index 7b3bcf41f421..4c5959b60038 100644
> --- a/drivers/clk/qcom/clock-qcom.h
> +++ b/drivers/clk/qcom/clock-qcom.h
> @@ -47,11 +47,16 @@ struct qcom_reset_map {
>         u8 bit;
>  };
>
> +struct clk;
> +
>  struct qcom_cc_data {
>         const struct qcom_reset_map     *resets;
>         unsigned long                   num_resets;
>         const struct gate_clk           *clks;
>         unsigned long                   num_clks;
> +
> +       int (*enable)(struct clk *clk);
> +       unsigned long (*set_rate)(struct clk *clk, unsigned long rate);
>  };
>
>  struct qcom_cc_priv {
> diff --git a/drivers/clk/qcom/clock-qcs404.c b/drivers/clk/qcom/clock-qcs404.c
> index e622309b6747..ce83ec741278 100644
> --- a/drivers/clk/qcom/clock-qcs404.c
> +++ b/drivers/clk/qcom/clock-qcs404.c
> @@ -12,6 +12,7 @@
>  #include <asm/io.h>
>  #include <linux/bitops.h>
>  #include <dt-bindings/clock/qcom,gcc-qcs404.h>
> +#include <mach/sysmap-qcs404.h>
>
>  #include "clock-qcom.h"
>
> @@ -157,7 +158,7 @@ ulong msm_set_rate(struct clk *clk, ulong rate)
>         return 0;
>  }
>
> -int msm_enable(struct clk *clk)
> +static int qcs404_enable(struct clk *clk)
>  {
>         struct qcom_cc_priv *priv = dev_get_priv(clk->dev);
>
> @@ -266,6 +267,8 @@ static const struct qcom_reset_map qcs404_gcc_resets[] = {
>  static const struct qcom_cc_data qcs404_gcc_data = {
>         .resets = qcs404_gcc_resets,
>         .num_resets = ARRAY_SIZE(qcs404_gcc_resets),
> +       .enable = qcs404_enable,
> +       .set_rate = qcs404_set_rate,
>  };
>
>  static const struct udevice_id gcc_qcs404_of_match[] = {
> diff --git a/drivers/clk/qcom/clock-sdm845.c b/drivers/clk/qcom/clock-sdm845.c
> index ccad73b6ff15..7b614f9661f0 100644
> --- a/drivers/clk/qcom/clock-sdm845.c
> +++ b/drivers/clk/qcom/clock-sdm845.c
> @@ -16,6 +16,7 @@
>  #include <asm/io.h>
>  #include <linux/bitops.h>
>  #include <dt-bindings/clock/qcom,gcc-sdm845.h>
> +#include <mach/sysmap-sdm845.h>
>
>  #include "clock-qcom.h"
>
> @@ -72,7 +73,7 @@ const struct freq_tbl *qcom_find_freq(const struct freq_tbl *f, uint rate)
>         return f - 1;
>  }
>
> -ulong msm_set_rate(struct clk *clk, ulong rate)
> +static ulong sdm845_set_rate(struct clk *clk, ulong rate)
>  {
>         struct qcom_cc_priv *priv = dev_get_priv(clk->dev);
>         const struct freq_tbl *freq;
> @@ -224,7 +225,7 @@ static const struct gate_clk sdm845_clks[] = {
>         GATE_CLK(GCC_CPUSS_DVM_BUS_CLK,                 0x48190, 0x00000001),
>  };
>
> -int msm_enable(struct clk *clk)
> +static int sdm845_enable(struct clk *clk)
>  {
>         struct qcom_cc_priv *priv = dev_get_priv(clk->dev);
>
> @@ -264,17 +265,20 @@ static const struct qcom_reset_map sdm845_gcc_resets[] = {
>         [GCC_PCIE_1_PHY_BCR] = { 0x8e01c },
>  };
>
> -static const struct qcom_cc_data qcs404_gcc_data = {
> +static struct qcom_cc_data sdm845_gcc_data = {
>         .resets = sdm845_gcc_resets,
>         .num_resets = ARRAY_SIZE(sdm845_gcc_resets),
>         .clks = sdm845_clks,
>         .num_clks = ARRAY_SIZE(sdm845_clks),
> +
> +       .enable = sdm845_enable,
> +       .set_rate = sdm845_set_rate,
>  };
>
>  static const struct udevice_id gcc_sdm845_of_match[] = {
>         {
>                 .compatible = "qcom,gcc-sdm845",
> -               .data = (ulong)&qcs404_gcc_data,
> +               .data = (ulong)&sdm845_gcc_data,
>         },
>         { }
>  };
>
> --
> 2.42.0
>


More information about the U-Boot mailing list