Re: [PATCH 6/8] clk/qcom: use function pointers for enable and set_rate
On Wed, 25 Oct 2023 at 01:55, Caleb Connolly 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
> ---
> 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
> #include
> #include
> +#include
>
> #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: _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
> #include
> #include
> +#include
>
> #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, _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_enabl
[PATCH 6/8] clk/qcom: use function pointers for enable and set_rate
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
specific sysmap header, include those explicitly to further reduce the
dependency between the clock driver and a particular target
configuration.
Signed-off-by: Caleb Connolly
---
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
#include
#include
+#include
#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)
{
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
#include
#include
+#include
#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)
{
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(
