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

On Wed, 25 Oct 2023 at 01:55, Caleb Connolly caleb.connolly@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@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