* [PATCH V2 0/4] clk: bcm2835: add additinal clocks and add frac support
@ 2016-01-11 19:55 ` kernel at martin.sperl.org
0 siblings, 0 replies; 33+ messages in thread
From: kernel @ 2016-01-11 19:55 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Stephen Warren, Lee Jones,
Eric Anholt, Remi Pommarel, linux-clk, linux-rpi-kernel,
linux-arm-kernel, devicetree
Cc: Martin Sperl
From: Martin Sperl <kernel@martin.sperl.org>
The clk-bcm2835 driver right now relies on BCM2835_CLOCK_COUNT defined
in include/dt-binding/clocks/bcm2835.h
With every new clock introduced this value needs to increase,
which is not what should happen for bindings.
So we reorganize the driver so that it is no longer necessary
to define BCM2835_CLOCK_COUNT.
Also the driver calculates fractional clock dividers correctly,
but it does not enable the bit to enable support in the register.
As a minimal extension we now can also define higher order MASH
support when defining the clocks.
Finally we add all the 23 different HW clocks that have not been
configured in the driver.
Martin Sperl (4):
clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
clk: bcm2835: enable fractional and mash support
clk: bcm2835: enable management of PCM clock
clk: bcm2835: add missing 22 HW-clocks.
drivers/clk/bcm/clk-bcm2835.c | 527 +++++++++++++++++++++++++++++++----
include/dt-bindings/clock/bcm2835.h | 25 +-
2 files changed, 489 insertions(+), 63 deletions(-)
--
1.7.10.4
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V2 0/4] clk: bcm2835: add additinal clocks and add frac support
@ 2016-01-11 19:55 ` kernel at martin.sperl.org
0 siblings, 0 replies; 33+ messages in thread
From: kernel at martin.sperl.org @ 2016-01-11 19:55 UTC (permalink / raw)
To: linux-arm-kernel
From: Martin Sperl <kernel@martin.sperl.org>
The clk-bcm2835 driver right now relies on BCM2835_CLOCK_COUNT defined
in include/dt-binding/clocks/bcm2835.h
With every new clock introduced this value needs to increase,
which is not what should happen for bindings.
So we reorganize the driver so that it is no longer necessary
to define BCM2835_CLOCK_COUNT.
Also the driver calculates fractional clock dividers correctly,
but it does not enable the bit to enable support in the register.
As a minimal extension we now can also define higher order MASH
support when defining the clocks.
Finally we add all the 23 different HW clocks that have not been
configured in the driver.
Martin Sperl (4):
clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
clk: bcm2835: enable fractional and mash support
clk: bcm2835: enable management of PCM clock
clk: bcm2835: add missing 22 HW-clocks.
drivers/clk/bcm/clk-bcm2835.c | 527 +++++++++++++++++++++++++++++++----
include/dt-bindings/clock/bcm2835.h | 25 +-
2 files changed, 489 insertions(+), 63 deletions(-)
--
1.7.10.4
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V2 1/4] clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
2016-01-11 19:55 ` kernel at martin.sperl.org
@ 2016-01-11 19:55 ` kernel at martin.sperl.org
-1 siblings, 0 replies; 33+ messages in thread
From: kernel @ 2016-01-11 19:55 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Stephen Warren, Lee Jones,
Eric Anholt, Remi Pommarel, linux-clk, linux-rpi-kernel,
linux-arm-kernel, devicetree
Cc: Martin Sperl
From: Martin Sperl <kernel@martin.sperl.org>
As the use of BCM2835_CLOCK_COUNT in
include/dt-bindings/clock/bcm2835.h is frowned upon as
it needs to get modified every time a new clock gets introduced
this patch changes the clk-bcm2835 driver to use a different
scheme for registration of clocks and pll, so that there
is no more need for BCM2835_CLOCK_COUNT to be defined.
The way the patch is designed also makes sure to allow control
the order in which the clocks are defined.
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
drivers/clk/bcm/clk-bcm2835.c | 208 +++++++++++++++++++++++++----------
include/dt-bindings/clock/bcm2835.h | 2 -
2 files changed, 147 insertions(+), 63 deletions(-)
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 015e687..759202a 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -288,7 +288,7 @@ struct bcm2835_cprman {
const char *osc_name;
struct clk_onecell_data onecell;
- struct clk *clks[BCM2835_CLOCK_COUNT];
+ struct clk *clks[];
};
static inline void cprman_write(struct bcm2835_cprman *cprman, u32 reg, u32 val)
@@ -1498,14 +1498,146 @@ static struct clk *bcm2835_register_clock(struct bcm2835_cprman *cprman,
return devm_clk_register(cprman->dev, &clock->hw);
}
+enum bcm2835_clk_register {
+ bcm2835_clk_register_pll,
+ bcm2835_clk_register_pll_div,
+ bcm2835_clk_register_clock,
+};
+
+/*the list of clocks and plls to register */
+enum bcm2835_register_clock_type {
+ CLK_TYPE_UNSET,
+ CLK_TYPE_PLL,
+ CLK_TYPE_PLL_DIV,
+ CLK_TYPE_CLOCK,
+};
+
+struct bcm2835_register_clock {
+ size_t index;
+ enum bcm2835_register_clock_type clock_type;
+ const void *data;
+};
+
+#define _REGISTER(i, t, d) { .index = i, .clock_type = t, .data = d }
+
+#define REGISTER_PLL(i, d) _REGISTER(i, CLK_TYPE_PLL, d)
+#define REGISTER_PLL_DIV(i, d) _REGISTER(i, CLK_TYPE_PLL_DIV, d)
+#define REGISTER_CLOCK(i, d) _REGISTER(i, CLK_TYPE_CLOCK, d)
+
+/*
+ * note that this array is processed first to last,
+ * so that we can define inititalization order.
+ * with the order right now: pll, pll_div and then clock
+ * this allows to retain the existing id- mapping in the device tree.
+ * ths also means we have to have some more explicit coding
+ * and can not use sparse arrays or similar.
+ */
+static const struct bcm2835_register_clock bcm2835_register_clocks[] = {
+ /* the PLL clocks */
+ REGISTER_PLL(BCM2835_PLLA, &bcm2835_plla_data),
+ REGISTER_PLL(BCM2835_PLLB, &bcm2835_pllb_data),
+ REGISTER_PLL(BCM2835_PLLC, &bcm2835_pllc_data),
+ REGISTER_PLL(BCM2835_PLLD, &bcm2835_plld_data),
+ REGISTER_PLL(BCM2835_PLLH, &bcm2835_pllh_data),
+ /* the PLL dividers */
+ REGISTER_PLL_DIV(BCM2835_PLLA_CORE, &bcm2835_plla_core_data),
+ REGISTER_PLL_DIV(BCM2835_PLLA_PER, &bcm2835_plla_per_data),
+ REGISTER_PLL_DIV(BCM2835_PLLC_CORE0, &bcm2835_pllc_core0_data),
+ REGISTER_PLL_DIV(BCM2835_PLLC_CORE1, &bcm2835_pllc_core1_data),
+ REGISTER_PLL_DIV(BCM2835_PLLC_CORE2, &bcm2835_pllc_core2_data),
+ REGISTER_PLL_DIV(BCM2835_PLLC_PER, &bcm2835_pllc_per_data),
+ REGISTER_PLL_DIV(BCM2835_PLLD_CORE, &bcm2835_plld_core_data),
+ REGISTER_PLL_DIV(BCM2835_PLLD_PER, &bcm2835_plld_per_data),
+ REGISTER_PLL_DIV(BCM2835_PLLH_RCAL, &bcm2835_pllh_rcal_data),
+ REGISTER_PLL_DIV(BCM2835_PLLH_AUX, &bcm2835_pllh_aux_data),
+ REGISTER_PLL_DIV(BCM2835_PLLH_PIX, &bcm2835_pllh_pix_data),
+ /* the clocks */
+ REGISTER_CLOCK(BCM2835_CLOCK_TIMER, &bcm2835_clock_timer_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_OTP, &bcm2835_clock_otp_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_TSENS, &bcm2835_clock_tsens_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_VPU, &bcm2835_clock_vpu_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_V3D, &bcm2835_clock_v3d_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_ISP, &bcm2835_clock_isp_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_H264, &bcm2835_clock_h264_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_V3D, &bcm2835_clock_v3d_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_SDRAM, &bcm2835_clock_sdram_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_UART, &bcm2835_clock_uart_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_VEC, &bcm2835_clock_vec_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_HSM, &bcm2835_clock_hsm_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_EMMC, &bcm2835_clock_emmc_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_PWM, &bcm2835_clock_pwm_data),
+};
+
+void bcm2835_register_duplicate_index(
+ struct device *dev, const struct bcm2835_register_clock *reg,
+ struct clk *clk)
+{
+ const char *name, *type;
+
+ switch (reg->clock_type) {
+ case CLK_TYPE_PLL:
+ name = ((struct bcm2835_pll_data *)reg->data)->name;
+ type = "pll";
+ break;
+ case CLK_TYPE_PLL_DIV:
+ name = ((struct bcm2835_pll_divider_data *)reg->data)->name;
+ type = "pll divider";
+ break;
+ case CLK_TYPE_CLOCK:
+ name = ((struct bcm2835_clock_data *)reg->data)->name;
+ type = "clock";
+ break;
+ default:
+ dev_err(dev, "Unsupported clock type %d for index %d\n",
+ reg->clock_type, reg->index);
+ return;
+ }
+
+ dev_err(dev,
+ "Could not register %s \"%s\" because index %i already defined as clock: %pC\n",
+ type, name, reg->index, clk);
+}
+
+struct clk *bcm2835_register_pllclock(
+ struct device *dev, struct bcm2835_cprman *cprman,
+ const struct bcm2835_register_clock *reg)
+{
+ switch (reg->clock_type) {
+ case CLK_TYPE_PLL:
+ return bcm2835_register_pll(
+ cprman, reg->data);
+ case CLK_TYPE_PLL_DIV:
+ return bcm2835_register_pll_divider(
+ cprman, reg->data);
+ case CLK_TYPE_CLOCK:
+ return bcm2835_register_clock(
+ cprman, reg->data);
+ default:
+ dev_err(dev, "Unsupported clock type %d for index %d\n",
+ reg->clock_type, reg->index);
+ }
+
+ return NULL;
+}
+
static int bcm2835_clk_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct clk **clks;
+ size_t clk_cnt;
struct bcm2835_cprman *cprman;
struct resource *res;
+ size_t i;
+
+ /* find the max clock index */
+ clk_cnt = BCM2835_CLOCK_PERI_IMAGE; /* see below */
+ for (i = 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++)
+ clk_cnt = max(clk_cnt, bcm2835_register_clocks[i].index);
+ clk_cnt += 1;
- cprman = devm_kzalloc(dev, sizeof(*cprman), GFP_KERNEL);
+ cprman = devm_kzalloc(dev,
+ sizeof(*cprman) + clk_cnt * sizeof(*clks),
+ GFP_KERNEL);
if (!cprman)
return -ENOMEM;
@@ -1522,65 +1654,22 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, cprman);
- cprman->onecell.clk_num = BCM2835_CLOCK_COUNT;
+ cprman->onecell.clk_num = clk_cnt;
cprman->onecell.clks = cprman->clks;
clks = cprman->clks;
- clks[BCM2835_PLLA] = bcm2835_register_pll(cprman, &bcm2835_plla_data);
- clks[BCM2835_PLLB] = bcm2835_register_pll(cprman, &bcm2835_pllb_data);
- clks[BCM2835_PLLC] = bcm2835_register_pll(cprman, &bcm2835_pllc_data);
- clks[BCM2835_PLLD] = bcm2835_register_pll(cprman, &bcm2835_plld_data);
- clks[BCM2835_PLLH] = bcm2835_register_pll(cprman, &bcm2835_pllh_data);
-
- clks[BCM2835_PLLA_CORE] =
- bcm2835_register_pll_divider(cprman, &bcm2835_plla_core_data);
- clks[BCM2835_PLLA_PER] =
- bcm2835_register_pll_divider(cprman, &bcm2835_plla_per_data);
- clks[BCM2835_PLLC_CORE0] =
- bcm2835_register_pll_divider(cprman, &bcm2835_pllc_core0_data);
- clks[BCM2835_PLLC_CORE1] =
- bcm2835_register_pll_divider(cprman, &bcm2835_pllc_core1_data);
- clks[BCM2835_PLLC_CORE2] =
- bcm2835_register_pll_divider(cprman, &bcm2835_pllc_core2_data);
- clks[BCM2835_PLLC_PER] =
- bcm2835_register_pll_divider(cprman, &bcm2835_pllc_per_data);
- clks[BCM2835_PLLD_CORE] =
- bcm2835_register_pll_divider(cprman, &bcm2835_plld_core_data);
- clks[BCM2835_PLLD_PER] =
- bcm2835_register_pll_divider(cprman, &bcm2835_plld_per_data);
- clks[BCM2835_PLLH_RCAL] =
- bcm2835_register_pll_divider(cprman, &bcm2835_pllh_rcal_data);
- clks[BCM2835_PLLH_AUX] =
- bcm2835_register_pll_divider(cprman, &bcm2835_pllh_aux_data);
- clks[BCM2835_PLLH_PIX] =
- bcm2835_register_pll_divider(cprman, &bcm2835_pllh_pix_data);
-
- clks[BCM2835_CLOCK_TIMER] =
- bcm2835_register_clock(cprman, &bcm2835_clock_timer_data);
- clks[BCM2835_CLOCK_OTP] =
- bcm2835_register_clock(cprman, &bcm2835_clock_otp_data);
- clks[BCM2835_CLOCK_TSENS] =
- bcm2835_register_clock(cprman, &bcm2835_clock_tsens_data);
- clks[BCM2835_CLOCK_VPU] =
- bcm2835_register_clock(cprman, &bcm2835_clock_vpu_data);
- clks[BCM2835_CLOCK_V3D] =
- bcm2835_register_clock(cprman, &bcm2835_clock_v3d_data);
- clks[BCM2835_CLOCK_ISP] =
- bcm2835_register_clock(cprman, &bcm2835_clock_isp_data);
- clks[BCM2835_CLOCK_H264] =
- bcm2835_register_clock(cprman, &bcm2835_clock_h264_data);
- clks[BCM2835_CLOCK_V3D] =
- bcm2835_register_clock(cprman, &bcm2835_clock_v3d_data);
- clks[BCM2835_CLOCK_SDRAM] =
- bcm2835_register_clock(cprman, &bcm2835_clock_sdram_data);
- clks[BCM2835_CLOCK_UART] =
- bcm2835_register_clock(cprman, &bcm2835_clock_uart_data);
- clks[BCM2835_CLOCK_VEC] =
- bcm2835_register_clock(cprman, &bcm2835_clock_vec_data);
- clks[BCM2835_CLOCK_HSM] =
- bcm2835_register_clock(cprman, &bcm2835_clock_hsm_data);
- clks[BCM2835_CLOCK_EMMC] =
- bcm2835_register_clock(cprman, &bcm2835_clock_emmc_data);
+ /* now register */
+ for (i = 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++) {
+ if (clks[bcm2835_register_clocks[i].index])
+ bcm2835_register_duplicate_index(
+ dev, &bcm2835_register_clocks[i],
+ clks[bcm2835_register_clocks[i].index]);
+ else
+ clks[bcm2835_register_clocks[i].index] =
+ bcm2835_register_pllclock(
+ dev, cprman,
+ &bcm2835_register_clocks[i]);
+ }
/*
* CM_PERIICTL (and CM_PERIACTL, CM_SYSCTL and CM_VPUCTL if
@@ -1594,9 +1683,6 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
cprman->regs + CM_PERIICTL, CM_GATE_BIT,
0, &cprman->regs_lock);
- clks[BCM2835_CLOCK_PWM] =
- bcm2835_register_clock(cprman, &bcm2835_clock_pwm_data);
-
return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
&cprman->onecell);
}
diff --git a/include/dt-bindings/clock/bcm2835.h b/include/dt-bindings/clock/bcm2835.h
index 61f1d20..87235ac 100644
--- a/include/dt-bindings/clock/bcm2835.h
+++ b/include/dt-bindings/clock/bcm2835.h
@@ -44,5 +44,3 @@
#define BCM2835_CLOCK_EMMC 28
#define BCM2835_CLOCK_PERI_IMAGE 29
#define BCM2835_CLOCK_PWM 30
-
-#define BCM2835_CLOCK_COUNT 31
--
1.7.10.4
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH V2 1/4] clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
@ 2016-01-11 19:55 ` kernel at martin.sperl.org
0 siblings, 0 replies; 33+ messages in thread
From: kernel at martin.sperl.org @ 2016-01-11 19:55 UTC (permalink / raw)
To: linux-arm-kernel
From: Martin Sperl <kernel@martin.sperl.org>
As the use of BCM2835_CLOCK_COUNT in
include/dt-bindings/clock/bcm2835.h is frowned upon as
it needs to get modified every time a new clock gets introduced
this patch changes the clk-bcm2835 driver to use a different
scheme for registration of clocks and pll, so that there
is no more need for BCM2835_CLOCK_COUNT to be defined.
The way the patch is designed also makes sure to allow control
the order in which the clocks are defined.
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
drivers/clk/bcm/clk-bcm2835.c | 208 +++++++++++++++++++++++++----------
include/dt-bindings/clock/bcm2835.h | 2 -
2 files changed, 147 insertions(+), 63 deletions(-)
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 015e687..759202a 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -288,7 +288,7 @@ struct bcm2835_cprman {
const char *osc_name;
struct clk_onecell_data onecell;
- struct clk *clks[BCM2835_CLOCK_COUNT];
+ struct clk *clks[];
};
static inline void cprman_write(struct bcm2835_cprman *cprman, u32 reg, u32 val)
@@ -1498,14 +1498,146 @@ static struct clk *bcm2835_register_clock(struct bcm2835_cprman *cprman,
return devm_clk_register(cprman->dev, &clock->hw);
}
+enum bcm2835_clk_register {
+ bcm2835_clk_register_pll,
+ bcm2835_clk_register_pll_div,
+ bcm2835_clk_register_clock,
+};
+
+/*the list of clocks and plls to register */
+enum bcm2835_register_clock_type {
+ CLK_TYPE_UNSET,
+ CLK_TYPE_PLL,
+ CLK_TYPE_PLL_DIV,
+ CLK_TYPE_CLOCK,
+};
+
+struct bcm2835_register_clock {
+ size_t index;
+ enum bcm2835_register_clock_type clock_type;
+ const void *data;
+};
+
+#define _REGISTER(i, t, d) { .index = i, .clock_type = t, .data = d }
+
+#define REGISTER_PLL(i, d) _REGISTER(i, CLK_TYPE_PLL, d)
+#define REGISTER_PLL_DIV(i, d) _REGISTER(i, CLK_TYPE_PLL_DIV, d)
+#define REGISTER_CLOCK(i, d) _REGISTER(i, CLK_TYPE_CLOCK, d)
+
+/*
+ * note that this array is processed first to last,
+ * so that we can define inititalization order.
+ * with the order right now: pll, pll_div and then clock
+ * this allows to retain the existing id- mapping in the device tree.
+ * ths also means we have to have some more explicit coding
+ * and can not use sparse arrays or similar.
+ */
+static const struct bcm2835_register_clock bcm2835_register_clocks[] = {
+ /* the PLL clocks */
+ REGISTER_PLL(BCM2835_PLLA, &bcm2835_plla_data),
+ REGISTER_PLL(BCM2835_PLLB, &bcm2835_pllb_data),
+ REGISTER_PLL(BCM2835_PLLC, &bcm2835_pllc_data),
+ REGISTER_PLL(BCM2835_PLLD, &bcm2835_plld_data),
+ REGISTER_PLL(BCM2835_PLLH, &bcm2835_pllh_data),
+ /* the PLL dividers */
+ REGISTER_PLL_DIV(BCM2835_PLLA_CORE, &bcm2835_plla_core_data),
+ REGISTER_PLL_DIV(BCM2835_PLLA_PER, &bcm2835_plla_per_data),
+ REGISTER_PLL_DIV(BCM2835_PLLC_CORE0, &bcm2835_pllc_core0_data),
+ REGISTER_PLL_DIV(BCM2835_PLLC_CORE1, &bcm2835_pllc_core1_data),
+ REGISTER_PLL_DIV(BCM2835_PLLC_CORE2, &bcm2835_pllc_core2_data),
+ REGISTER_PLL_DIV(BCM2835_PLLC_PER, &bcm2835_pllc_per_data),
+ REGISTER_PLL_DIV(BCM2835_PLLD_CORE, &bcm2835_plld_core_data),
+ REGISTER_PLL_DIV(BCM2835_PLLD_PER, &bcm2835_plld_per_data),
+ REGISTER_PLL_DIV(BCM2835_PLLH_RCAL, &bcm2835_pllh_rcal_data),
+ REGISTER_PLL_DIV(BCM2835_PLLH_AUX, &bcm2835_pllh_aux_data),
+ REGISTER_PLL_DIV(BCM2835_PLLH_PIX, &bcm2835_pllh_pix_data),
+ /* the clocks */
+ REGISTER_CLOCK(BCM2835_CLOCK_TIMER, &bcm2835_clock_timer_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_OTP, &bcm2835_clock_otp_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_TSENS, &bcm2835_clock_tsens_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_VPU, &bcm2835_clock_vpu_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_V3D, &bcm2835_clock_v3d_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_ISP, &bcm2835_clock_isp_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_H264, &bcm2835_clock_h264_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_V3D, &bcm2835_clock_v3d_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_SDRAM, &bcm2835_clock_sdram_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_UART, &bcm2835_clock_uart_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_VEC, &bcm2835_clock_vec_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_HSM, &bcm2835_clock_hsm_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_EMMC, &bcm2835_clock_emmc_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_PWM, &bcm2835_clock_pwm_data),
+};
+
+void bcm2835_register_duplicate_index(
+ struct device *dev, const struct bcm2835_register_clock *reg,
+ struct clk *clk)
+{
+ const char *name, *type;
+
+ switch (reg->clock_type) {
+ case CLK_TYPE_PLL:
+ name = ((struct bcm2835_pll_data *)reg->data)->name;
+ type = "pll";
+ break;
+ case CLK_TYPE_PLL_DIV:
+ name = ((struct bcm2835_pll_divider_data *)reg->data)->name;
+ type = "pll divider";
+ break;
+ case CLK_TYPE_CLOCK:
+ name = ((struct bcm2835_clock_data *)reg->data)->name;
+ type = "clock";
+ break;
+ default:
+ dev_err(dev, "Unsupported clock type %d for index %d\n",
+ reg->clock_type, reg->index);
+ return;
+ }
+
+ dev_err(dev,
+ "Could not register %s \"%s\" because index %i already defined as clock: %pC\n",
+ type, name, reg->index, clk);
+}
+
+struct clk *bcm2835_register_pllclock(
+ struct device *dev, struct bcm2835_cprman *cprman,
+ const struct bcm2835_register_clock *reg)
+{
+ switch (reg->clock_type) {
+ case CLK_TYPE_PLL:
+ return bcm2835_register_pll(
+ cprman, reg->data);
+ case CLK_TYPE_PLL_DIV:
+ return bcm2835_register_pll_divider(
+ cprman, reg->data);
+ case CLK_TYPE_CLOCK:
+ return bcm2835_register_clock(
+ cprman, reg->data);
+ default:
+ dev_err(dev, "Unsupported clock type %d for index %d\n",
+ reg->clock_type, reg->index);
+ }
+
+ return NULL;
+}
+
static int bcm2835_clk_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct clk **clks;
+ size_t clk_cnt;
struct bcm2835_cprman *cprman;
struct resource *res;
+ size_t i;
+
+ /* find the max clock index */
+ clk_cnt = BCM2835_CLOCK_PERI_IMAGE; /* see below */
+ for (i = 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++)
+ clk_cnt = max(clk_cnt, bcm2835_register_clocks[i].index);
+ clk_cnt += 1;
- cprman = devm_kzalloc(dev, sizeof(*cprman), GFP_KERNEL);
+ cprman = devm_kzalloc(dev,
+ sizeof(*cprman) + clk_cnt * sizeof(*clks),
+ GFP_KERNEL);
if (!cprman)
return -ENOMEM;
@@ -1522,65 +1654,22 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, cprman);
- cprman->onecell.clk_num = BCM2835_CLOCK_COUNT;
+ cprman->onecell.clk_num = clk_cnt;
cprman->onecell.clks = cprman->clks;
clks = cprman->clks;
- clks[BCM2835_PLLA] = bcm2835_register_pll(cprman, &bcm2835_plla_data);
- clks[BCM2835_PLLB] = bcm2835_register_pll(cprman, &bcm2835_pllb_data);
- clks[BCM2835_PLLC] = bcm2835_register_pll(cprman, &bcm2835_pllc_data);
- clks[BCM2835_PLLD] = bcm2835_register_pll(cprman, &bcm2835_plld_data);
- clks[BCM2835_PLLH] = bcm2835_register_pll(cprman, &bcm2835_pllh_data);
-
- clks[BCM2835_PLLA_CORE] =
- bcm2835_register_pll_divider(cprman, &bcm2835_plla_core_data);
- clks[BCM2835_PLLA_PER] =
- bcm2835_register_pll_divider(cprman, &bcm2835_plla_per_data);
- clks[BCM2835_PLLC_CORE0] =
- bcm2835_register_pll_divider(cprman, &bcm2835_pllc_core0_data);
- clks[BCM2835_PLLC_CORE1] =
- bcm2835_register_pll_divider(cprman, &bcm2835_pllc_core1_data);
- clks[BCM2835_PLLC_CORE2] =
- bcm2835_register_pll_divider(cprman, &bcm2835_pllc_core2_data);
- clks[BCM2835_PLLC_PER] =
- bcm2835_register_pll_divider(cprman, &bcm2835_pllc_per_data);
- clks[BCM2835_PLLD_CORE] =
- bcm2835_register_pll_divider(cprman, &bcm2835_plld_core_data);
- clks[BCM2835_PLLD_PER] =
- bcm2835_register_pll_divider(cprman, &bcm2835_plld_per_data);
- clks[BCM2835_PLLH_RCAL] =
- bcm2835_register_pll_divider(cprman, &bcm2835_pllh_rcal_data);
- clks[BCM2835_PLLH_AUX] =
- bcm2835_register_pll_divider(cprman, &bcm2835_pllh_aux_data);
- clks[BCM2835_PLLH_PIX] =
- bcm2835_register_pll_divider(cprman, &bcm2835_pllh_pix_data);
-
- clks[BCM2835_CLOCK_TIMER] =
- bcm2835_register_clock(cprman, &bcm2835_clock_timer_data);
- clks[BCM2835_CLOCK_OTP] =
- bcm2835_register_clock(cprman, &bcm2835_clock_otp_data);
- clks[BCM2835_CLOCK_TSENS] =
- bcm2835_register_clock(cprman, &bcm2835_clock_tsens_data);
- clks[BCM2835_CLOCK_VPU] =
- bcm2835_register_clock(cprman, &bcm2835_clock_vpu_data);
- clks[BCM2835_CLOCK_V3D] =
- bcm2835_register_clock(cprman, &bcm2835_clock_v3d_data);
- clks[BCM2835_CLOCK_ISP] =
- bcm2835_register_clock(cprman, &bcm2835_clock_isp_data);
- clks[BCM2835_CLOCK_H264] =
- bcm2835_register_clock(cprman, &bcm2835_clock_h264_data);
- clks[BCM2835_CLOCK_V3D] =
- bcm2835_register_clock(cprman, &bcm2835_clock_v3d_data);
- clks[BCM2835_CLOCK_SDRAM] =
- bcm2835_register_clock(cprman, &bcm2835_clock_sdram_data);
- clks[BCM2835_CLOCK_UART] =
- bcm2835_register_clock(cprman, &bcm2835_clock_uart_data);
- clks[BCM2835_CLOCK_VEC] =
- bcm2835_register_clock(cprman, &bcm2835_clock_vec_data);
- clks[BCM2835_CLOCK_HSM] =
- bcm2835_register_clock(cprman, &bcm2835_clock_hsm_data);
- clks[BCM2835_CLOCK_EMMC] =
- bcm2835_register_clock(cprman, &bcm2835_clock_emmc_data);
+ /* now register */
+ for (i = 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++) {
+ if (clks[bcm2835_register_clocks[i].index])
+ bcm2835_register_duplicate_index(
+ dev, &bcm2835_register_clocks[i],
+ clks[bcm2835_register_clocks[i].index]);
+ else
+ clks[bcm2835_register_clocks[i].index] =
+ bcm2835_register_pllclock(
+ dev, cprman,
+ &bcm2835_register_clocks[i]);
+ }
/*
* CM_PERIICTL (and CM_PERIACTL, CM_SYSCTL and CM_VPUCTL if
@@ -1594,9 +1683,6 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
cprman->regs + CM_PERIICTL, CM_GATE_BIT,
0, &cprman->regs_lock);
- clks[BCM2835_CLOCK_PWM] =
- bcm2835_register_clock(cprman, &bcm2835_clock_pwm_data);
-
return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
&cprman->onecell);
}
diff --git a/include/dt-bindings/clock/bcm2835.h b/include/dt-bindings/clock/bcm2835.h
index 61f1d20..87235ac 100644
--- a/include/dt-bindings/clock/bcm2835.h
+++ b/include/dt-bindings/clock/bcm2835.h
@@ -44,5 +44,3 @@
#define BCM2835_CLOCK_EMMC 28
#define BCM2835_CLOCK_PERI_IMAGE 29
#define BCM2835_CLOCK_PWM 30
-
-#define BCM2835_CLOCK_COUNT 31
--
1.7.10.4
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH V2 2/4] clk: bcm2835: enable fractional and mash support
2016-01-11 19:55 ` kernel at martin.sperl.org
@ 2016-01-11 19:55 ` kernel at martin.sperl.org
-1 siblings, 0 replies; 33+ messages in thread
From: kernel @ 2016-01-11 19:55 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Stephen Warren, Lee Jones,
Eric Anholt, Remi Pommarel, linux-clk, linux-rpi-kernel,
linux-arm-kernel, devicetree
Cc: Martin Sperl
From: Martin Sperl <kernel@martin.sperl.org>
The clk-bcm2835 driver right now does the correct calculation
of the fractional clock divider, but it does not set the FRAC
bit to enable the fractual clock divider in HW
This patch enables FRAC for all clocks with frac_bits > 0
but allows to define the selection of a higher-order MASH
support instead of just FRAC.
Right now there are no limits imposed on maximum frequencies
when using MASH/FRAC is enabled.
There is a documented limit of 25MHz for MASH, but it is not
stated if this also applies to clock dividers that only support
FRAC and not MASH.
As for how higher order MASH works the following may give
some insight: http://www.aholme.co.uk/Frac2/Mash.htm
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
drivers/clk/bcm/clk-bcm2835.c | 50 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 759202a..7c782d3 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -115,6 +115,13 @@
# define CM_GATE BIT(CM_GATE_BIT)
# define CM_BUSY BIT(7)
# define CM_BUSYD BIT(8)
+# define CM_MASH_BITS 2
+# define CM_MASH_SHIFT 9
+# define CM_MASH_MASK GENMASK(10, 9)
+# define CM_MASH(v) ((v << CM_MASH_SHIFT) & CM_MASH_MASK)
+# define CM_MASH_FRAC CM_MASH(1)
+# define CM_MASH_2ND_ORDER CM_MASH(2)
+# define CM_MASH_3RD_ORDER CM_MASH(3)
# define CM_SRC_SHIFT 0
# define CM_SRC_BITS 4
# define CM_SRC_MASK 0xf
@@ -632,6 +639,8 @@ struct bcm2835_clock_data {
u32 int_bits;
/* Number of fractional bits in the divider */
u32 frac_bits;
+ /* the mash value to use - see CM_MASH and CM_MASH_FRAC/ORDER */
+ u32 mash;
bool is_vpu_clock;
};
@@ -1274,9 +1283,50 @@ static int bcm2835_clock_set_rate(struct clk_hw *hw,
struct bcm2835_cprman *cprman = clock->cprman;
const struct bcm2835_clock_data *data = clock->data;
u32 div = bcm2835_clock_choose_div(hw, rate, parent_rate, false);
+ u32 ctl, mash;
+ bool enabled;
+ spin_lock(&cprman->regs_lock);
+ /* check if divider is identical, then return */
+ if (div == cprman_read(cprman, data->div_reg))
+ goto unlock;
+
+ /* it is recommended to only set clock registers when disabled */
+ ctl = cprman_read(cprman, data->ctl_reg);
+ enabled = ctl & CM_ENABLE;
+ if (enabled) {
+ /* disable clock */
+ cprman_write(cprman, data->ctl_reg, ctl);
+
+ /* release lock while busy waiting */
+ spin_unlock(&cprman->regs_lock);
+ bcm2835_clock_wait_busy(clock);
+ spin_lock(&cprman->regs_lock);
+
+ /* read the register again */
+ ctl = cprman_read(cprman, data->ctl_reg);
+ }
+
+ /* set the divider */
cprman_write(cprman, data->div_reg, div);
+ /* set frac/mash if necessarry */
+ mash = 0;
+ if ((data->frac_bits) && (div & GENMASK(CM_DIV_FRAC_BITS, 0)))
+ mash = (data->mash) ? data->mash : CM_MASH_FRAC;
+
+ /* set mash to the selected value */
+ ctl &= ~CM_MASH_MASK;
+ ctl |= mash & CM_MASH_MASK;
+ cprman_write(cprman, data->ctl_reg, ctl);
+
+ /* re-enable the clock */
+ if (enabled)
+ cprman_write(cprman, data->ctl_reg, ctl | CM_ENABLE);
+
+unlock:
+ spin_unlock(&cprman->regs_lock);
+
return 0;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH V2 2/4] clk: bcm2835: enable fractional and mash support
@ 2016-01-11 19:55 ` kernel at martin.sperl.org
0 siblings, 0 replies; 33+ messages in thread
From: kernel at martin.sperl.org @ 2016-01-11 19:55 UTC (permalink / raw)
To: linux-arm-kernel
From: Martin Sperl <kernel@martin.sperl.org>
The clk-bcm2835 driver right now does the correct calculation
of the fractional clock divider, but it does not set the FRAC
bit to enable the fractual clock divider in HW
This patch enables FRAC for all clocks with frac_bits > 0
but allows to define the selection of a higher-order MASH
support instead of just FRAC.
Right now there are no limits imposed on maximum frequencies
when using MASH/FRAC is enabled.
There is a documented limit of 25MHz for MASH, but it is not
stated if this also applies to clock dividers that only support
FRAC and not MASH.
As for how higher order MASH works the following may give
some insight: http://www.aholme.co.uk/Frac2/Mash.htm
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
drivers/clk/bcm/clk-bcm2835.c | 50 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 759202a..7c782d3 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -115,6 +115,13 @@
# define CM_GATE BIT(CM_GATE_BIT)
# define CM_BUSY BIT(7)
# define CM_BUSYD BIT(8)
+# define CM_MASH_BITS 2
+# define CM_MASH_SHIFT 9
+# define CM_MASH_MASK GENMASK(10, 9)
+# define CM_MASH(v) ((v << CM_MASH_SHIFT) & CM_MASH_MASK)
+# define CM_MASH_FRAC CM_MASH(1)
+# define CM_MASH_2ND_ORDER CM_MASH(2)
+# define CM_MASH_3RD_ORDER CM_MASH(3)
# define CM_SRC_SHIFT 0
# define CM_SRC_BITS 4
# define CM_SRC_MASK 0xf
@@ -632,6 +639,8 @@ struct bcm2835_clock_data {
u32 int_bits;
/* Number of fractional bits in the divider */
u32 frac_bits;
+ /* the mash value to use - see CM_MASH and CM_MASH_FRAC/ORDER */
+ u32 mash;
bool is_vpu_clock;
};
@@ -1274,9 +1283,50 @@ static int bcm2835_clock_set_rate(struct clk_hw *hw,
struct bcm2835_cprman *cprman = clock->cprman;
const struct bcm2835_clock_data *data = clock->data;
u32 div = bcm2835_clock_choose_div(hw, rate, parent_rate, false);
+ u32 ctl, mash;
+ bool enabled;
+ spin_lock(&cprman->regs_lock);
+ /* check if divider is identical, then return */
+ if (div == cprman_read(cprman, data->div_reg))
+ goto unlock;
+
+ /* it is recommended to only set clock registers when disabled */
+ ctl = cprman_read(cprman, data->ctl_reg);
+ enabled = ctl & CM_ENABLE;
+ if (enabled) {
+ /* disable clock */
+ cprman_write(cprman, data->ctl_reg, ctl);
+
+ /* release lock while busy waiting */
+ spin_unlock(&cprman->regs_lock);
+ bcm2835_clock_wait_busy(clock);
+ spin_lock(&cprman->regs_lock);
+
+ /* read the register again */
+ ctl = cprman_read(cprman, data->ctl_reg);
+ }
+
+ /* set the divider */
cprman_write(cprman, data->div_reg, div);
+ /* set frac/mash if necessarry */
+ mash = 0;
+ if ((data->frac_bits) && (div & GENMASK(CM_DIV_FRAC_BITS, 0)))
+ mash = (data->mash) ? data->mash : CM_MASH_FRAC;
+
+ /* set mash to the selected value */
+ ctl &= ~CM_MASH_MASK;
+ ctl |= mash & CM_MASH_MASK;
+ cprman_write(cprman, data->ctl_reg, ctl);
+
+ /* re-enable the clock */
+ if (enabled)
+ cprman_write(cprman, data->ctl_reg, ctl | CM_ENABLE);
+
+unlock:
+ spin_unlock(&cprman->regs_lock);
+
return 0;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH V2 3/4] clk: bcm2835: enable management of PCM clock
2016-01-11 19:55 ` kernel at martin.sperl.org
@ 2016-01-11 19:55 ` kernel at martin.sperl.org
-1 siblings, 0 replies; 33+ messages in thread
From: kernel @ 2016-01-11 19:55 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Stephen Warren, Lee Jones,
Eric Anholt, Remi Pommarel, linux-clk, linux-rpi-kernel,
linux-arm-kernel, devicetree
Cc: Martin Sperl
From: Martin Sperl <kernel@martin.sperl.org>
Enable the PCM clock in the SOC, which is used by the
bcm2835-i2s driver.
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
drivers/clk/bcm/clk-bcm2835.c | 13 +++++++++++++
include/dt-bindings/clock/bcm2835.h | 1 +
2 files changed, 14 insertions(+)
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 7c782d3..573b5b1 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -88,6 +88,8 @@
#define CM_HSMDIV 0x08c
#define CM_OTPCTL 0x090
#define CM_OTPDIV 0x094
+#define CM_PCMCTL 0x098
+#define CM_PCMDIV 0x09c
#define CM_PWMCTL 0x0a0
#define CM_PWMDIV 0x0a4
#define CM_SMICTL 0x0b0
@@ -826,6 +828,16 @@ static const struct bcm2835_clock_data bcm2835_clock_pwm_data = {
.frac_bits = 12,
};
+static const struct bcm2835_clock_data bcm2835_clock_pcm_data = {
+ .name = "pcm",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_PCMCTL,
+ .div_reg = CM_PCMDIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
struct bcm2835_pll {
struct clk_hw hw;
struct bcm2835_cprman *cprman;
@@ -1616,6 +1628,7 @@ static const struct bcm2835_register_clock bcm2835_register_clocks[] = {
REGISTER_CLOCK(BCM2835_CLOCK_HSM, &bcm2835_clock_hsm_data),
REGISTER_CLOCK(BCM2835_CLOCK_EMMC, &bcm2835_clock_emmc_data),
REGISTER_CLOCK(BCM2835_CLOCK_PWM, &bcm2835_clock_pwm_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_PCM, &bcm2835_clock_pcm_data),
};
void bcm2835_register_duplicate_index(
diff --git a/include/dt-bindings/clock/bcm2835.h b/include/dt-bindings/clock/bcm2835.h
index 87235ac..9a7b4a5 100644
--- a/include/dt-bindings/clock/bcm2835.h
+++ b/include/dt-bindings/clock/bcm2835.h
@@ -44,3 +44,4 @@
#define BCM2835_CLOCK_EMMC 28
#define BCM2835_CLOCK_PERI_IMAGE 29
#define BCM2835_CLOCK_PWM 30
+#define BCM2835_CLOCK_PCM 31
--
1.7.10.4
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH V2 3/4] clk: bcm2835: enable management of PCM clock
@ 2016-01-11 19:55 ` kernel at martin.sperl.org
0 siblings, 0 replies; 33+ messages in thread
From: kernel at martin.sperl.org @ 2016-01-11 19:55 UTC (permalink / raw)
To: linux-arm-kernel
From: Martin Sperl <kernel@martin.sperl.org>
Enable the PCM clock in the SOC, which is used by the
bcm2835-i2s driver.
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
drivers/clk/bcm/clk-bcm2835.c | 13 +++++++++++++
include/dt-bindings/clock/bcm2835.h | 1 +
2 files changed, 14 insertions(+)
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 7c782d3..573b5b1 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -88,6 +88,8 @@
#define CM_HSMDIV 0x08c
#define CM_OTPCTL 0x090
#define CM_OTPDIV 0x094
+#define CM_PCMCTL 0x098
+#define CM_PCMDIV 0x09c
#define CM_PWMCTL 0x0a0
#define CM_PWMDIV 0x0a4
#define CM_SMICTL 0x0b0
@@ -826,6 +828,16 @@ static const struct bcm2835_clock_data bcm2835_clock_pwm_data = {
.frac_bits = 12,
};
+static const struct bcm2835_clock_data bcm2835_clock_pcm_data = {
+ .name = "pcm",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_PCMCTL,
+ .div_reg = CM_PCMDIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
struct bcm2835_pll {
struct clk_hw hw;
struct bcm2835_cprman *cprman;
@@ -1616,6 +1628,7 @@ static const struct bcm2835_register_clock bcm2835_register_clocks[] = {
REGISTER_CLOCK(BCM2835_CLOCK_HSM, &bcm2835_clock_hsm_data),
REGISTER_CLOCK(BCM2835_CLOCK_EMMC, &bcm2835_clock_emmc_data),
REGISTER_CLOCK(BCM2835_CLOCK_PWM, &bcm2835_clock_pwm_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_PCM, &bcm2835_clock_pcm_data),
};
void bcm2835_register_duplicate_index(
diff --git a/include/dt-bindings/clock/bcm2835.h b/include/dt-bindings/clock/bcm2835.h
index 87235ac..9a7b4a5 100644
--- a/include/dt-bindings/clock/bcm2835.h
+++ b/include/dt-bindings/clock/bcm2835.h
@@ -44,3 +44,4 @@
#define BCM2835_CLOCK_EMMC 28
#define BCM2835_CLOCK_PERI_IMAGE 29
#define BCM2835_CLOCK_PWM 30
+#define BCM2835_CLOCK_PCM 31
--
1.7.10.4
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH V2 4/4] clk: bcm2835: add missing 22 HW-clocks.
2016-01-11 19:55 ` kernel at martin.sperl.org
@ 2016-01-11 19:55 ` kernel at martin.sperl.org
-1 siblings, 0 replies; 33+ messages in thread
From: kernel @ 2016-01-11 19:55 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Stephen Warren, Lee Jones,
Eric Anholt, Remi Pommarel, linux-clk, linux-rpi-kernel,
linux-arm-kernel, devicetree
Cc: Martin Sperl
From: Martin Sperl <kernel@martin.sperl.org>
There were 22 HW clocks missing from the clock driver.
These have been included and int_bits and frac_bits
have been set correctly based on information extracted
from the broadcom videocore headers
(http://www.broadcom.com/docs/support/videocore/Brcm_Android_ICS_Graphics_Stack.tar.gz)
For an extracted view of the registers please see:
https://github.com/msperl/rpi-registers/blob/master/md/Region_CM.md
bcm2835_clock_per_parents has been assigned as the parent
clock for all new clocks, but this may not be correct
in all cases - documentation on this is not publicly
available, so some modifications may be needed in the
future.
Note that some of those clocks may be owned by the firmware,
so we may need an additional flag to set "ownership".
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
drivers/clk/bcm/clk-bcm2835.c | 256 +++++++++++++++++++++++++++++++++++
include/dt-bindings/clock/bcm2835.h | 22 +++
2 files changed, 278 insertions(+)
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 573b5b1..99cc756 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -92,8 +92,18 @@
#define CM_PCMDIV 0x09c
#define CM_PWMCTL 0x0a0
#define CM_PWMDIV 0x0a4
+#define CM_SLIMCTL 0x0a8
+#define CM_SLIMDIV 0x0ac
#define CM_SMICTL 0x0b0
#define CM_SMIDIV 0x0b4
+#define CM_TCNTCTL 0x0c0
+#define CM_TCNTDIV 0x0c4
+#define CM_TECCTL 0x0c8
+#define CM_TECDIV 0x0cc
+#define CM_TD0CTL 0x0d0
+#define CM_TD0DIV 0x0d4
+#define CM_TD1CTL 0x0d8
+#define CM_TD1DIV 0x0dc
#define CM_TSENSCTL 0x0e0
#define CM_TSENSDIV 0x0e4
#define CM_TIMERCTL 0x0e8
@@ -107,6 +117,9 @@
#define CM_SDCCTL 0x1a8
#define CM_SDCDIV 0x1ac
#define CM_ARMCTL 0x1b0
+#define CM_ARMDIV 0x1b4
+#define CM_AVEOCTL 0x1b8
+#define CM_AVEODIV 0x1bc
#define CM_EMMCCTL 0x1c0
#define CM_EMMCDIV 0x1c4
@@ -838,6 +851,226 @@ static const struct bcm2835_clock_data bcm2835_clock_pcm_data = {
.frac_bits = 12,
};
+static const struct bcm2835_clock_data bcm2835_clock_aveo_data = {
+ .name = "aveo",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_AVEOCTL,
+ .div_reg = CM_AVEODIV,
+ .int_bits = 4,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_cam0_data = {
+ .name = "cam0",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_CAM0CTL,
+ .div_reg = CM_CAM0DIV,
+ .int_bits = 4,
+ .frac_bits = 8,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_cam1_data = {
+ .name = "cam1",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_CAM1CTL,
+ .div_reg = CM_CAM1DIV,
+ .int_bits = 4,
+ .frac_bits = 8,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_ccp2_data = {
+ .name = "ccp2",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_CCP2CTL,
+ .div_reg = CM_CCP2DIV,
+ .int_bits = 1,
+ .frac_bits = 0,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_dft_data = {
+ .name = "dft",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_DFTCTL,
+ .div_reg = CM_DFTDIV,
+ .int_bits = 5,
+ .frac_bits = 0,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_dpi_data = {
+ .name = "dpi",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_DPICTL,
+ .div_reg = CM_DPIDIV,
+ .int_bits = 4,
+ .frac_bits = 8,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_dsi0e_data = {
+ .name = "dsi0e",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_DSI0ECTL,
+ .div_reg = CM_DSI0EDIV,
+ .int_bits = 4,
+ .frac_bits = 8,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_dsi0p_data = {
+ .name = "dsi0p",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_DSI0PCTL,
+ .div_reg = CM_DSI0PDIV,
+ .int_bits = 1,
+ .frac_bits = 0,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_dsi1e_data = {
+ .name = "dsi1e",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_DSI1ECTL,
+ .div_reg = CM_DSI1EDIV,
+ .int_bits = 4,
+ .frac_bits = 8,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_dsi1p_data = {
+ .name = "dsi1p",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_DSI1PCTL,
+ .div_reg = CM_DSI1PDIV,
+ .int_bits = 1,
+ .frac_bits = 0,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_gnric_data = {
+ .name = "gnric",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_GNRICCTL,
+ .div_reg = CM_GNRICDIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_gp0_data = {
+ .name = "gp0",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_GP0CTL,
+ .div_reg = CM_GP0DIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_gp1_data = {
+ .name = "gp1",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_GP1CTL,
+ .div_reg = CM_GP1DIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_gp2_data = {
+ .name = "gp2",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_GP2CTL,
+ .div_reg = CM_GP2DIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_peria_data = {
+ .name = "peria",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_PERIACTL,
+ .div_reg = CM_PERIADIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_pulse_data = {
+ .name = "pulse",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_PULSECTL,
+ .div_reg = CM_PULSEDIV,
+ .int_bits = 12,
+ .frac_bits = 0,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_slim_data = {
+ .name = "slim",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_SLIMCTL,
+ .div_reg = CM_SLIMDIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_smi_data = {
+ .name = "smi",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_SMICTL,
+ .div_reg = CM_SMIDIV,
+ .int_bits = 4,
+ .frac_bits = 8,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_sys_data = {
+ .name = "sys",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_SYSCTL,
+ .div_reg = CM_SYSDIV,
+ .int_bits = 1,
+ .frac_bits = 0,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_td0_data = {
+ .name = "td0",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_TD0CTL,
+ .div_reg = CM_TD0DIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_td1_data = {
+ .name = "td1",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_TD1CTL,
+ .div_reg = CM_TD1DIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_tec_data = {
+ .name = "tec",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_TECCTL,
+ .div_reg = CM_TECDIV,
+ .int_bits = 6,
+ .frac_bits = 0,
+};
+
struct bcm2835_pll {
struct clk_hw hw;
struct bcm2835_cprman *cprman;
@@ -1629,6 +1862,29 @@ static const struct bcm2835_register_clock bcm2835_register_clocks[] = {
REGISTER_CLOCK(BCM2835_CLOCK_EMMC, &bcm2835_clock_emmc_data),
REGISTER_CLOCK(BCM2835_CLOCK_PWM, &bcm2835_clock_pwm_data),
REGISTER_CLOCK(BCM2835_CLOCK_PCM, &bcm2835_clock_pcm_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_AVEO, &bcm2835_clock_aveo_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_CAM0, &bcm2835_clock_cam0_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_CAM1, &bcm2835_clock_cam1_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_CCP2, &bcm2835_clock_ccp2_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_DFT, &bcm2835_clock_dft_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_DPI, &bcm2835_clock_dpi_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_DSI0E, &bcm2835_clock_dsi0e_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_DSI0P, &bcm2835_clock_dsi0p_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_DSI1E, &bcm2835_clock_dsi1e_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_DSI1P, &bcm2835_clock_dsi1p_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_GNRIC, &bcm2835_clock_gnric_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_GP0, &bcm2835_clock_gp0_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_GP1, &bcm2835_clock_gp1_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_GP2, &bcm2835_clock_gp2_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_PERIA, &bcm2835_clock_peria_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_PULSE, &bcm2835_clock_pulse_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_SLIM, &bcm2835_clock_slim_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_SMI, &bcm2835_clock_smi_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_SYS, &bcm2835_clock_sys_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_TD0, &bcm2835_clock_td0_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_TD1, &bcm2835_clock_td1_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_TEC, &bcm2835_clock_tec_data),
+
};
void bcm2835_register_duplicate_index(
diff --git a/include/dt-bindings/clock/bcm2835.h b/include/dt-bindings/clock/bcm2835.h
index 9a7b4a5..d29f181 100644
--- a/include/dt-bindings/clock/bcm2835.h
+++ b/include/dt-bindings/clock/bcm2835.h
@@ -45,3 +45,25 @@
#define BCM2835_CLOCK_PERI_IMAGE 29
#define BCM2835_CLOCK_PWM 30
#define BCM2835_CLOCK_PCM 31
+#define BCM2835_CLOCK_AVEO 32
+#define BCM2835_CLOCK_CAM0 33
+#define BCM2835_CLOCK_CAM1 34
+#define BCM2835_CLOCK_CCP2 35
+#define BCM2835_CLOCK_DFT 36
+#define BCM2835_CLOCK_DPI 37
+#define BCM2835_CLOCK_DSI0E 38
+#define BCM2835_CLOCK_DSI0P 39
+#define BCM2835_CLOCK_DSI1E 40
+#define BCM2835_CLOCK_DSI1P 41
+#define BCM2835_CLOCK_GNRIC 42
+#define BCM2835_CLOCK_GP0 43
+#define BCM2835_CLOCK_GP1 44
+#define BCM2835_CLOCK_GP2 45
+#define BCM2835_CLOCK_PERIA 46
+#define BCM2835_CLOCK_PULSE 47
+#define BCM2835_CLOCK_SLIM 48
+#define BCM2835_CLOCK_SMI 49
+#define BCM2835_CLOCK_SYS 50
+#define BCM2835_CLOCK_TD0 51
+#define BCM2835_CLOCK_TD1 52
+#define BCM2835_CLOCK_TEC 53
--
1.7.10.4
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH V2 4/4] clk: bcm2835: add missing 22 HW-clocks.
@ 2016-01-11 19:55 ` kernel at martin.sperl.org
0 siblings, 0 replies; 33+ messages in thread
From: kernel at martin.sperl.org @ 2016-01-11 19:55 UTC (permalink / raw)
To: linux-arm-kernel
From: Martin Sperl <kernel@martin.sperl.org>
There were 22 HW clocks missing from the clock driver.
These have been included and int_bits and frac_bits
have been set correctly based on information extracted
from the broadcom videocore headers
(http://www.broadcom.com/docs/support/videocore/Brcm_Android_ICS_Graphics_Stack.tar.gz)
For an extracted view of the registers please see:
https://github.com/msperl/rpi-registers/blob/master/md/Region_CM.md
bcm2835_clock_per_parents has been assigned as the parent
clock for all new clocks, but this may not be correct
in all cases - documentation on this is not publicly
available, so some modifications may be needed in the
future.
Note that some of those clocks may be owned by the firmware,
so we may need an additional flag to set "ownership".
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
drivers/clk/bcm/clk-bcm2835.c | 256 +++++++++++++++++++++++++++++++++++
include/dt-bindings/clock/bcm2835.h | 22 +++
2 files changed, 278 insertions(+)
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 573b5b1..99cc756 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -92,8 +92,18 @@
#define CM_PCMDIV 0x09c
#define CM_PWMCTL 0x0a0
#define CM_PWMDIV 0x0a4
+#define CM_SLIMCTL 0x0a8
+#define CM_SLIMDIV 0x0ac
#define CM_SMICTL 0x0b0
#define CM_SMIDIV 0x0b4
+#define CM_TCNTCTL 0x0c0
+#define CM_TCNTDIV 0x0c4
+#define CM_TECCTL 0x0c8
+#define CM_TECDIV 0x0cc
+#define CM_TD0CTL 0x0d0
+#define CM_TD0DIV 0x0d4
+#define CM_TD1CTL 0x0d8
+#define CM_TD1DIV 0x0dc
#define CM_TSENSCTL 0x0e0
#define CM_TSENSDIV 0x0e4
#define CM_TIMERCTL 0x0e8
@@ -107,6 +117,9 @@
#define CM_SDCCTL 0x1a8
#define CM_SDCDIV 0x1ac
#define CM_ARMCTL 0x1b0
+#define CM_ARMDIV 0x1b4
+#define CM_AVEOCTL 0x1b8
+#define CM_AVEODIV 0x1bc
#define CM_EMMCCTL 0x1c0
#define CM_EMMCDIV 0x1c4
@@ -838,6 +851,226 @@ static const struct bcm2835_clock_data bcm2835_clock_pcm_data = {
.frac_bits = 12,
};
+static const struct bcm2835_clock_data bcm2835_clock_aveo_data = {
+ .name = "aveo",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_AVEOCTL,
+ .div_reg = CM_AVEODIV,
+ .int_bits = 4,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_cam0_data = {
+ .name = "cam0",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_CAM0CTL,
+ .div_reg = CM_CAM0DIV,
+ .int_bits = 4,
+ .frac_bits = 8,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_cam1_data = {
+ .name = "cam1",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_CAM1CTL,
+ .div_reg = CM_CAM1DIV,
+ .int_bits = 4,
+ .frac_bits = 8,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_ccp2_data = {
+ .name = "ccp2",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_CCP2CTL,
+ .div_reg = CM_CCP2DIV,
+ .int_bits = 1,
+ .frac_bits = 0,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_dft_data = {
+ .name = "dft",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_DFTCTL,
+ .div_reg = CM_DFTDIV,
+ .int_bits = 5,
+ .frac_bits = 0,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_dpi_data = {
+ .name = "dpi",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_DPICTL,
+ .div_reg = CM_DPIDIV,
+ .int_bits = 4,
+ .frac_bits = 8,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_dsi0e_data = {
+ .name = "dsi0e",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_DSI0ECTL,
+ .div_reg = CM_DSI0EDIV,
+ .int_bits = 4,
+ .frac_bits = 8,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_dsi0p_data = {
+ .name = "dsi0p",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_DSI0PCTL,
+ .div_reg = CM_DSI0PDIV,
+ .int_bits = 1,
+ .frac_bits = 0,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_dsi1e_data = {
+ .name = "dsi1e",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_DSI1ECTL,
+ .div_reg = CM_DSI1EDIV,
+ .int_bits = 4,
+ .frac_bits = 8,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_dsi1p_data = {
+ .name = "dsi1p",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_DSI1PCTL,
+ .div_reg = CM_DSI1PDIV,
+ .int_bits = 1,
+ .frac_bits = 0,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_gnric_data = {
+ .name = "gnric",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_GNRICCTL,
+ .div_reg = CM_GNRICDIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_gp0_data = {
+ .name = "gp0",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_GP0CTL,
+ .div_reg = CM_GP0DIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_gp1_data = {
+ .name = "gp1",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_GP1CTL,
+ .div_reg = CM_GP1DIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_gp2_data = {
+ .name = "gp2",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_GP2CTL,
+ .div_reg = CM_GP2DIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_peria_data = {
+ .name = "peria",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_PERIACTL,
+ .div_reg = CM_PERIADIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_pulse_data = {
+ .name = "pulse",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_PULSECTL,
+ .div_reg = CM_PULSEDIV,
+ .int_bits = 12,
+ .frac_bits = 0,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_slim_data = {
+ .name = "slim",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_SLIMCTL,
+ .div_reg = CM_SLIMDIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_smi_data = {
+ .name = "smi",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_SMICTL,
+ .div_reg = CM_SMIDIV,
+ .int_bits = 4,
+ .frac_bits = 8,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_sys_data = {
+ .name = "sys",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_SYSCTL,
+ .div_reg = CM_SYSDIV,
+ .int_bits = 1,
+ .frac_bits = 0,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_td0_data = {
+ .name = "td0",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_TD0CTL,
+ .div_reg = CM_TD0DIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_td1_data = {
+ .name = "td1",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_TD1CTL,
+ .div_reg = CM_TD1DIV,
+ .int_bits = 12,
+ .frac_bits = 12,
+};
+
+static const struct bcm2835_clock_data bcm2835_clock_tec_data = {
+ .name = "tec",
+ .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
+ .parents = bcm2835_clock_per_parents,
+ .ctl_reg = CM_TECCTL,
+ .div_reg = CM_TECDIV,
+ .int_bits = 6,
+ .frac_bits = 0,
+};
+
struct bcm2835_pll {
struct clk_hw hw;
struct bcm2835_cprman *cprman;
@@ -1629,6 +1862,29 @@ static const struct bcm2835_register_clock bcm2835_register_clocks[] = {
REGISTER_CLOCK(BCM2835_CLOCK_EMMC, &bcm2835_clock_emmc_data),
REGISTER_CLOCK(BCM2835_CLOCK_PWM, &bcm2835_clock_pwm_data),
REGISTER_CLOCK(BCM2835_CLOCK_PCM, &bcm2835_clock_pcm_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_AVEO, &bcm2835_clock_aveo_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_CAM0, &bcm2835_clock_cam0_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_CAM1, &bcm2835_clock_cam1_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_CCP2, &bcm2835_clock_ccp2_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_DFT, &bcm2835_clock_dft_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_DPI, &bcm2835_clock_dpi_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_DSI0E, &bcm2835_clock_dsi0e_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_DSI0P, &bcm2835_clock_dsi0p_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_DSI1E, &bcm2835_clock_dsi1e_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_DSI1P, &bcm2835_clock_dsi1p_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_GNRIC, &bcm2835_clock_gnric_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_GP0, &bcm2835_clock_gp0_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_GP1, &bcm2835_clock_gp1_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_GP2, &bcm2835_clock_gp2_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_PERIA, &bcm2835_clock_peria_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_PULSE, &bcm2835_clock_pulse_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_SLIM, &bcm2835_clock_slim_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_SMI, &bcm2835_clock_smi_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_SYS, &bcm2835_clock_sys_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_TD0, &bcm2835_clock_td0_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_TD1, &bcm2835_clock_td1_data),
+ REGISTER_CLOCK(BCM2835_CLOCK_TEC, &bcm2835_clock_tec_data),
+
};
void bcm2835_register_duplicate_index(
diff --git a/include/dt-bindings/clock/bcm2835.h b/include/dt-bindings/clock/bcm2835.h
index 9a7b4a5..d29f181 100644
--- a/include/dt-bindings/clock/bcm2835.h
+++ b/include/dt-bindings/clock/bcm2835.h
@@ -45,3 +45,25 @@
#define BCM2835_CLOCK_PERI_IMAGE 29
#define BCM2835_CLOCK_PWM 30
#define BCM2835_CLOCK_PCM 31
+#define BCM2835_CLOCK_AVEO 32
+#define BCM2835_CLOCK_CAM0 33
+#define BCM2835_CLOCK_CAM1 34
+#define BCM2835_CLOCK_CCP2 35
+#define BCM2835_CLOCK_DFT 36
+#define BCM2835_CLOCK_DPI 37
+#define BCM2835_CLOCK_DSI0E 38
+#define BCM2835_CLOCK_DSI0P 39
+#define BCM2835_CLOCK_DSI1E 40
+#define BCM2835_CLOCK_DSI1P 41
+#define BCM2835_CLOCK_GNRIC 42
+#define BCM2835_CLOCK_GP0 43
+#define BCM2835_CLOCK_GP1 44
+#define BCM2835_CLOCK_GP2 45
+#define BCM2835_CLOCK_PERIA 46
+#define BCM2835_CLOCK_PULSE 47
+#define BCM2835_CLOCK_SLIM 48
+#define BCM2835_CLOCK_SMI 49
+#define BCM2835_CLOCK_SYS 50
+#define BCM2835_CLOCK_TD0 51
+#define BCM2835_CLOCK_TD1 52
+#define BCM2835_CLOCK_TEC 53
--
1.7.10.4
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH V2 0/4] clk: bcm2835: add additinal clocks and add frac support
2016-01-11 19:55 ` kernel at martin.sperl.org
(?)
@ 2016-01-11 21:01 ` Arnd Bergmann
-1 siblings, 0 replies; 33+ messages in thread
From: Arnd Bergmann @ 2016-01-11 21:01 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: kernel-TqfNSX0MhmxHKSADF0wUEw, Michael Turquette, Stephen Boyd,
Stephen Warren, Lee Jones, Eric Anholt, Remi Pommarel,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
On Monday 11 January 2016 19:55:52 kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>
> The clk-bcm2835 driver right now relies on BCM2835_CLOCK_COUNT defined
> in include/dt-binding/clocks/bcm2835.h
> With every new clock introduced this value needs to increase,
> which is not what should happen for bindings.
>
> So we reorganize the driver so that it is no longer necessary
> to define BCM2835_CLOCK_COUNT.
>
> Also the driver calculates fractional clock dividers correctly,
> but it does not enable the bit to enable support in the register.
> As a minimal extension we now can also define higher order MASH
> support when defining the clocks.
>
> Finally we add all the 23 different HW clocks that have not been
> configured in the driver.
>
>
Looks good to me, thanks for the update!
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V2 0/4] clk: bcm2835: add additinal clocks and add frac support
@ 2016-01-11 21:01 ` Arnd Bergmann
0 siblings, 0 replies; 33+ messages in thread
From: Arnd Bergmann @ 2016-01-11 21:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kernel, Michael Turquette, Stephen Boyd, Stephen Warren,
Lee Jones, Eric Anholt, Remi Pommarel, linux-clk,
linux-rpi-kernel, devicetree
On Monday 11 January 2016 19:55:52 kernel@martin.sperl.org wrote:
> From: Martin Sperl <kernel@martin.sperl.org>
>
> The clk-bcm2835 driver right now relies on BCM2835_CLOCK_COUNT defined
> in include/dt-binding/clocks/bcm2835.h
> With every new clock introduced this value needs to increase,
> which is not what should happen for bindings.
>
> So we reorganize the driver so that it is no longer necessary
> to define BCM2835_CLOCK_COUNT.
>
> Also the driver calculates fractional clock dividers correctly,
> but it does not enable the bit to enable support in the register.
> As a minimal extension we now can also define higher order MASH
> support when defining the clocks.
>
> Finally we add all the 23 different HW clocks that have not been
> configured in the driver.
>
>
Looks good to me, thanks for the update!
Arnd
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V2 0/4] clk: bcm2835: add additinal clocks and add frac support
@ 2016-01-11 21:01 ` Arnd Bergmann
0 siblings, 0 replies; 33+ messages in thread
From: Arnd Bergmann @ 2016-01-11 21:01 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 11 January 2016 19:55:52 kernel at martin.sperl.org wrote:
> From: Martin Sperl <kernel@martin.sperl.org>
>
> The clk-bcm2835 driver right now relies on BCM2835_CLOCK_COUNT defined
> in include/dt-binding/clocks/bcm2835.h
> With every new clock introduced this value needs to increase,
> which is not what should happen for bindings.
>
> So we reorganize the driver so that it is no longer necessary
> to define BCM2835_CLOCK_COUNT.
>
> Also the driver calculates fractional clock dividers correctly,
> but it does not enable the bit to enable support in the register.
> As a minimal extension we now can also define higher order MASH
> support when defining the clocks.
>
> Finally we add all the 23 different HW clocks that have not been
> configured in the driver.
>
>
Looks good to me, thanks for the update!
Arnd
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V2 1/4] clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
2016-01-11 19:55 ` kernel at martin.sperl.org
(?)
@ 2016-01-13 20:00 ` Michael Turquette
-1 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-01-13 20:00 UTC (permalink / raw)
To: Stephen Boyd, Stephen Warren, Lee Jones, Eric Anholt,
Remi Pommarel, linux-clk, linux-rpi-kernel, linux-arm-kernel,
devicetree
Cc: Martin Sperl
Hi Martin,
Quoting kernel@martin.sperl.org (2016-01-11 11:55:53)
> From: Martin Sperl <kernel@martin.sperl.org>
>
> As the use of BCM2835_CLOCK_COUNT in
> include/dt-bindings/clock/bcm2835.h is frowned upon as
> it needs to get modified every time a new clock gets introduced
> this patch changes the clk-bcm2835 driver to use a different
> scheme for registration of clocks and pll, so that there
> is no more need for BCM2835_CLOCK_COUNT to be defined.
>
> The way the patch is designed also makes sure to allow control
> the order in which the clocks are defined.
>
> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> ---
> drivers/clk/bcm/clk-bcm2835.c | 208 +++++++++++++++++++++++++----------
> include/dt-bindings/clock/bcm2835.h | 2 -
> 2 files changed, 147 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index 015e687..759202a 100644
> --- a/drivers/clk/bcm/clk-bcm2835.c
> +++ b/drivers/clk/bcm/clk-bcm2835.c
> @@ -288,7 +288,7 @@ struct bcm2835_cprman {
> const char *osc_name;
>
> struct clk_onecell_data onecell;
> - struct clk *clks[BCM2835_CLOCK_COUNT];
> + struct clk *clks[];
> };
>
> static inline void cprman_write(struct bcm2835_cprman *cprman, u32 reg, u32 val)
> @@ -1498,14 +1498,146 @@ static struct clk *bcm2835_register_clock(struct bcm2835_cprman *cprman,
> return devm_clk_register(cprman->dev, &clock->hw);
> }
>
> +enum bcm2835_clk_register {
> + bcm2835_clk_register_pll,
> + bcm2835_clk_register_pll_div,
> + bcm2835_clk_register_clock,
> +};
> +
> +/*the list of clocks and plls to register */
> +enum bcm2835_register_clock_type {
Nitpick: s/clock/clk/
It helps with grepping later on. Typically "clock" is used for OF/DT
stuff and clk is used for Linux-specific stuff. This isn't a rule, but
seems to be the de facto standard.
> + CLK_TYPE_UNSET,
> + CLK_TYPE_PLL,
> + CLK_TYPE_PLL_DIV,
> + CLK_TYPE_CLOCK,
> +};
> +
> +struct bcm2835_register_clock {
Nitpick: how about bcm2835_clk_desc ?
> + size_t index;
> + enum bcm2835_register_clock_type clock_type;
> + const void *data;
> +};
> +
> +#define _REGISTER(i, t, d) { .index = i, .clock_type = t, .data = d }
> +
> +#define REGISTER_PLL(i, d) _REGISTER(i, CLK_TYPE_PLL, d)
> +#define REGISTER_PLL_DIV(i, d) _REGISTER(i, CLK_TYPE_PLL_DIV, d)
> +#define REGISTER_CLOCK(i, d) _REGISTER(i, CLK_TYPE_CLOCK, d)
> +
> +/*
> + * note that this array is processed first to last,
> + * so that we can define inititalization order.
> + * with the order right now: pll, pll_div and then clock
> + * this allows to retain the existing id- mapping in the device tree.
> + * ths also means we have to have some more explicit coding
> + * and can not use sparse arrays or similar.
> + */
> +static const struct bcm2835_register_clock bcm2835_register_clocks[] = {
> + /* the PLL clocks */
> + REGISTER_PLL(BCM2835_PLLA, &bcm2835_plla_data),
> + REGISTER_PLL(BCM2835_PLLB, &bcm2835_pllb_data),
> + REGISTER_PLL(BCM2835_PLLC, &bcm2835_pllc_data),
> + REGISTER_PLL(BCM2835_PLLD, &bcm2835_plld_data),
> + REGISTER_PLL(BCM2835_PLLH, &bcm2835_pllh_data),
> + /* the PLL dividers */
> + REGISTER_PLL_DIV(BCM2835_PLLA_CORE, &bcm2835_plla_core_data),
> + REGISTER_PLL_DIV(BCM2835_PLLA_PER, &bcm2835_plla_per_data),
> + REGISTER_PLL_DIV(BCM2835_PLLC_CORE0, &bcm2835_pllc_core0_data),
> + REGISTER_PLL_DIV(BCM2835_PLLC_CORE1, &bcm2835_pllc_core1_data),
> + REGISTER_PLL_DIV(BCM2835_PLLC_CORE2, &bcm2835_pllc_core2_data),
> + REGISTER_PLL_DIV(BCM2835_PLLC_PER, &bcm2835_pllc_per_data),
> + REGISTER_PLL_DIV(BCM2835_PLLD_CORE, &bcm2835_plld_core_data),
> + REGISTER_PLL_DIV(BCM2835_PLLD_PER, &bcm2835_plld_per_data),
> + REGISTER_PLL_DIV(BCM2835_PLLH_RCAL, &bcm2835_pllh_rcal_data),
> + REGISTER_PLL_DIV(BCM2835_PLLH_AUX, &bcm2835_pllh_aux_data),
> + REGISTER_PLL_DIV(BCM2835_PLLH_PIX, &bcm2835_pllh_pix_data),
> + /* the clocks */
> + REGISTER_CLOCK(BCM2835_CLOCK_TIMER, &bcm2835_clock_timer_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_OTP, &bcm2835_clock_otp_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_TSENS, &bcm2835_clock_tsens_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_VPU, &bcm2835_clock_vpu_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_V3D, &bcm2835_clock_v3d_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_ISP, &bcm2835_clock_isp_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_H264, &bcm2835_clock_h264_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_V3D, &bcm2835_clock_v3d_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_SDRAM, &bcm2835_clock_sdram_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_UART, &bcm2835_clock_uart_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_VEC, &bcm2835_clock_vec_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_HSM, &bcm2835_clock_hsm_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_EMMC, &bcm2835_clock_emmc_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_PWM, &bcm2835_clock_pwm_data),
> +};
> +
> +void bcm2835_register_duplicate_index(
> + struct device *dev, const struct bcm2835_register_clock *reg,
> + struct clk *clk)
> +{
> + const char *name, *type;
> +
> + switch (reg->clock_type) {
> + case CLK_TYPE_PLL:
> + name = ((struct bcm2835_pll_data *)reg->data)->name;
> + type = "pll";
> + break;
> + case CLK_TYPE_PLL_DIV:
> + name = ((struct bcm2835_pll_divider_data *)reg->data)->name;
> + type = "pll divider";
> + break;
> + case CLK_TYPE_CLOCK:
> + name = ((struct bcm2835_clock_data *)reg->data)->name;
> + type = "clock";
> + break;
> + default:
> + dev_err(dev, "Unsupported clock type %d for index %d\n",
> + reg->clock_type, reg->index);
> + return;
> + }
> +
> + dev_err(dev,
> + "Could not register %s \"%s\" because index %i already defined as clock: %pC\n",
> + type, name, reg->index, clk);
> +}
> +
> +struct clk *bcm2835_register_pllclock(
> + struct device *dev, struct bcm2835_cprman *cprman,
> + const struct bcm2835_register_clock *reg)
> +{
> + switch (reg->clock_type) {
> + case CLK_TYPE_PLL:
> + return bcm2835_register_pll(
> + cprman, reg->data);
> + case CLK_TYPE_PLL_DIV:
> + return bcm2835_register_pll_divider(
> + cprman, reg->data);
> + case CLK_TYPE_CLOCK:
> + return bcm2835_register_clock(
> + cprman, reg->data);
> + default:
> + dev_err(dev, "Unsupported clock type %d for index %d\n",
> + reg->clock_type, reg->index);
> + }
> +
> + return NULL;
> +}
> +
> static int bcm2835_clk_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct clk **clks;
> + size_t clk_cnt;
> struct bcm2835_cprman *cprman;
> struct resource *res;
> + size_t i;
> +
> + /* find the max clock index */
> + clk_cnt = BCM2835_CLOCK_PERI_IMAGE; /* see below */
> + for (i = 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++)
> + clk_cnt = max(clk_cnt, bcm2835_register_clocks[i].index);
> + clk_cnt += 1;
I'm not sure how this solution is better than using CLOCK_COUNT. Some
other bindings use a max value, NR_CLKS or other sentinel.
Why did you not choose to set clk_cnt equal to BCM2835_CLOCK_PWM? Why
not initialize it to zero?
>
> - cprman = devm_kzalloc(dev, sizeof(*cprman), GFP_KERNEL);
> + cprman = devm_kzalloc(dev,
> + sizeof(*cprman) + clk_cnt * sizeof(*clks),
> + GFP_KERNEL);
> if (!cprman)
> return -ENOMEM;
>
> @@ -1522,65 +1654,22 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, cprman);
>
> - cprman->onecell.clk_num = BCM2835_CLOCK_COUNT;
> + cprman->onecell.clk_num = clk_cnt;
> cprman->onecell.clks = cprman->clks;
> clks = cprman->clks;
>
> - clks[BCM2835_PLLA] = bcm2835_register_pll(cprman, &bcm2835_plla_data);
> - clks[BCM2835_PLLB] = bcm2835_register_pll(cprman, &bcm2835_pllb_data);
> - clks[BCM2835_PLLC] = bcm2835_register_pll(cprman, &bcm2835_pllc_data);
> - clks[BCM2835_PLLD] = bcm2835_register_pll(cprman, &bcm2835_plld_data);
> - clks[BCM2835_PLLH] = bcm2835_register_pll(cprman, &bcm2835_pllh_data);
> -
> - clks[BCM2835_PLLA_CORE] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_plla_core_data);
> - clks[BCM2835_PLLA_PER] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_plla_per_data);
> - clks[BCM2835_PLLC_CORE0] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllc_core0_data);
> - clks[BCM2835_PLLC_CORE1] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllc_core1_data);
> - clks[BCM2835_PLLC_CORE2] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllc_core2_data);
> - clks[BCM2835_PLLC_PER] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllc_per_data);
> - clks[BCM2835_PLLD_CORE] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_plld_core_data);
> - clks[BCM2835_PLLD_PER] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_plld_per_data);
> - clks[BCM2835_PLLH_RCAL] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllh_rcal_data);
> - clks[BCM2835_PLLH_AUX] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllh_aux_data);
> - clks[BCM2835_PLLH_PIX] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllh_pix_data);
> -
> - clks[BCM2835_CLOCK_TIMER] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_timer_data);
> - clks[BCM2835_CLOCK_OTP] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_otp_data);
> - clks[BCM2835_CLOCK_TSENS] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_tsens_data);
> - clks[BCM2835_CLOCK_VPU] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_vpu_data);
> - clks[BCM2835_CLOCK_V3D] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_v3d_data);
> - clks[BCM2835_CLOCK_ISP] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_isp_data);
> - clks[BCM2835_CLOCK_H264] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_h264_data);
> - clks[BCM2835_CLOCK_V3D] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_v3d_data);
> - clks[BCM2835_CLOCK_SDRAM] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_sdram_data);
> - clks[BCM2835_CLOCK_UART] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_uart_data);
> - clks[BCM2835_CLOCK_VEC] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_vec_data);
> - clks[BCM2835_CLOCK_HSM] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_hsm_data);
> - clks[BCM2835_CLOCK_EMMC] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_emmc_data);
> + /* now register */
> + for (i = 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++) {
> + if (clks[bcm2835_register_clocks[i].index])
> + bcm2835_register_duplicate_index(
> + dev, &bcm2835_register_clocks[i],
> + clks[bcm2835_register_clocks[i].index]);
Why is this necessary? When do you have duplicate indices?
Regards,
Mike
> + else
> + clks[bcm2835_register_clocks[i].index] =
> + bcm2835_register_pllclock(
> + dev, cprman,
> + &bcm2835_register_clocks[i]);
> + }
>
> /*
> * CM_PERIICTL (and CM_PERIACTL, CM_SYSCTL and CM_VPUCTL if
> @@ -1594,9 +1683,6 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
> cprman->regs + CM_PERIICTL, CM_GATE_BIT,
> 0, &cprman->regs_lock);
>
> - clks[BCM2835_CLOCK_PWM] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_pwm_data);
> -
> return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
> &cprman->onecell);
> }
> diff --git a/include/dt-bindings/clock/bcm2835.h b/include/dt-bindings/clock/bcm2835.h
> index 61f1d20..87235ac 100644
> --- a/include/dt-bindings/clock/bcm2835.h
> +++ b/include/dt-bindings/clock/bcm2835.h
> @@ -44,5 +44,3 @@
> #define BCM2835_CLOCK_EMMC 28
> #define BCM2835_CLOCK_PERI_IMAGE 29
> #define BCM2835_CLOCK_PWM 30
> -
> -#define BCM2835_CLOCK_COUNT 31
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V2 1/4] clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
@ 2016-01-13 20:00 ` Michael Turquette
0 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-01-13 20:00 UTC (permalink / raw)
To: kernel, Stephen Boyd, Stephen Warren, Lee Jones, Eric Anholt,
Remi Pommarel, linux-clk, linux-rpi-kernel, linux-arm-kernel,
devicetree
Cc: Martin Sperl
Hi Martin,
Quoting kernel@martin.sperl.org (2016-01-11 11:55:53)
> From: Martin Sperl <kernel@martin.sperl.org>
> =
> As the use of BCM2835_CLOCK_COUNT in
> include/dt-bindings/clock/bcm2835.h is frowned upon as
> it needs to get modified every time a new clock gets introduced
> this patch changes the clk-bcm2835 driver to use a different
> scheme for registration of clocks and pll, so that there
> is no more need for BCM2835_CLOCK_COUNT to be defined.
> =
> The way the patch is designed also makes sure to allow control
> the order in which the clocks are defined.
> =
> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> ---
> drivers/clk/bcm/clk-bcm2835.c | 208 +++++++++++++++++++++++++----=
------
> include/dt-bindings/clock/bcm2835.h | 2 -
> 2 files changed, 147 insertions(+), 63 deletions(-)
> =
> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index 015e687..759202a 100644
> --- a/drivers/clk/bcm/clk-bcm2835.c
> +++ b/drivers/clk/bcm/clk-bcm2835.c
> @@ -288,7 +288,7 @@ struct bcm2835_cprman {
> const char *osc_name;
> =
> struct clk_onecell_data onecell;
> - struct clk *clks[BCM2835_CLOCK_COUNT];
> + struct clk *clks[];
> };
> =
> static inline void cprman_write(struct bcm2835_cprman *cprman, u32 reg, =
u32 val)
> @@ -1498,14 +1498,146 @@ static struct clk *bcm2835_register_clock(struct=
bcm2835_cprman *cprman,
> return devm_clk_register(cprman->dev, &clock->hw);
> }
> =
> +enum bcm2835_clk_register {
> + bcm2835_clk_register_pll,
> + bcm2835_clk_register_pll_div,
> + bcm2835_clk_register_clock,
> +};
> +
> +/*the list of clocks and plls to register */
> +enum bcm2835_register_clock_type {
Nitpick: s/clock/clk/
It helps with grepping later on. Typically "clock" is used for OF/DT
stuff and clk is used for Linux-specific stuff. This isn't a rule, but
seems to be the de facto standard.
> + CLK_TYPE_UNSET,
> + CLK_TYPE_PLL,
> + CLK_TYPE_PLL_DIV,
> + CLK_TYPE_CLOCK,
> +};
> +
> +struct bcm2835_register_clock {
Nitpick: how about bcm2835_clk_desc ?
> + size_t index;
> + enum bcm2835_register_clock_type clock_type;
> + const void *data;
> +};
> +
> +#define _REGISTER(i, t, d) { .index =3D i, .clock_type =3D t, .data =3D =
d }
> +
> +#define REGISTER_PLL(i, d) _REGISTER(i, CLK_TYPE_PLL, d)
> +#define REGISTER_PLL_DIV(i, d) _REGISTER(i, CLK_TYPE_PLL_DIV, d)
> +#define REGISTER_CLOCK(i, d) _REGISTER(i, CLK_TYPE_CLOCK, d)
> +
> +/*
> + * note that this array is processed first to last,
> + * so that we can define inititalization order.
> + * with the order right now: pll, pll_div and then clock
> + * this allows to retain the existing id- mapping in the device tree.
> + * ths also means we have to have some more explicit coding
> + * and can not use sparse arrays or similar.
> + */
> +static const struct bcm2835_register_clock bcm2835_register_clocks[] =3D=
{
> + /* the PLL clocks */
> + REGISTER_PLL(BCM2835_PLLA, &bcm2835_plla_data),
> + REGISTER_PLL(BCM2835_PLLB, &bcm2835_pllb_data),
> + REGISTER_PLL(BCM2835_PLLC, &bcm2835_pllc_data),
> + REGISTER_PLL(BCM2835_PLLD, &bcm2835_plld_data),
> + REGISTER_PLL(BCM2835_PLLH, &bcm2835_pllh_data),
> + /* the PLL dividers */
> + REGISTER_PLL_DIV(BCM2835_PLLA_CORE, &bcm2835_plla_core_data),
> + REGISTER_PLL_DIV(BCM2835_PLLA_PER, &bcm2835_plla_per_data),
> + REGISTER_PLL_DIV(BCM2835_PLLC_CORE0, &bcm2835_pllc_core0_data),
> + REGISTER_PLL_DIV(BCM2835_PLLC_CORE1, &bcm2835_pllc_core1_data),
> + REGISTER_PLL_DIV(BCM2835_PLLC_CORE2, &bcm2835_pllc_core2_data),
> + REGISTER_PLL_DIV(BCM2835_PLLC_PER, &bcm2835_pllc_per_data),
> + REGISTER_PLL_DIV(BCM2835_PLLD_CORE, &bcm2835_plld_core_data),
> + REGISTER_PLL_DIV(BCM2835_PLLD_PER, &bcm2835_plld_per_data),
> + REGISTER_PLL_DIV(BCM2835_PLLH_RCAL, &bcm2835_pllh_rcal_data),
> + REGISTER_PLL_DIV(BCM2835_PLLH_AUX, &bcm2835_pllh_aux_data),
> + REGISTER_PLL_DIV(BCM2835_PLLH_PIX, &bcm2835_pllh_pix_data),
> + /* the clocks */
> + REGISTER_CLOCK(BCM2835_CLOCK_TIMER, &bcm2835_clock_timer_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_OTP, &bcm2835_clock_otp_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_TSENS, &bcm2835_clock_tsens_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_VPU, &bcm2835_clock_vpu_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_V3D, &bcm2835_clock_v3d_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_ISP, &bcm2835_clock_isp_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_H264, &bcm2835_clock_h264_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_V3D, &bcm2835_clock_v3d_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_SDRAM, &bcm2835_clock_sdram_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_UART, &bcm2835_clock_uart_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_VEC, &bcm2835_clock_vec_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_HSM, &bcm2835_clock_hsm_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_EMMC, &bcm2835_clock_emmc_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_PWM, &bcm2835_clock_pwm_data),
> +};
> +
> +void bcm2835_register_duplicate_index(
> + struct device *dev, const struct bcm2835_register_clock *reg,
> + struct clk *clk)
> +{
> + const char *name, *type;
> +
> + switch (reg->clock_type) {
> + case CLK_TYPE_PLL:
> + name =3D ((struct bcm2835_pll_data *)reg->data)->name;
> + type =3D "pll";
> + break;
> + case CLK_TYPE_PLL_DIV:
> + name =3D ((struct bcm2835_pll_divider_data *)reg->data)->=
name;
> + type =3D "pll divider";
> + break;
> + case CLK_TYPE_CLOCK:
> + name =3D ((struct bcm2835_clock_data *)reg->data)->name;
> + type =3D "clock";
> + break;
> + default:
> + dev_err(dev, "Unsupported clock type %d for index %d\n",
> + reg->clock_type, reg->index);
> + return;
> + }
> +
> + dev_err(dev,
> + "Could not register %s \"%s\" because index %i already de=
fined as clock: %pC\n",
> + type, name, reg->index, clk);
> +}
> +
> +struct clk *bcm2835_register_pllclock(
> + struct device *dev, struct bcm2835_cprman *cprman,
> + const struct bcm2835_register_clock *reg)
> +{
> + switch (reg->clock_type) {
> + case CLK_TYPE_PLL:
> + return bcm2835_register_pll(
> + cprman, reg->data);
> + case CLK_TYPE_PLL_DIV:
> + return bcm2835_register_pll_divider(
> + cprman, reg->data);
> + case CLK_TYPE_CLOCK:
> + return bcm2835_register_clock(
> + cprman, reg->data);
> + default:
> + dev_err(dev, "Unsupported clock type %d for index %d\n",
> + reg->clock_type, reg->index);
> + }
> +
> + return NULL;
> +}
> +
> static int bcm2835_clk_probe(struct platform_device *pdev)
> {
> struct device *dev =3D &pdev->dev;
> struct clk **clks;
> + size_t clk_cnt;
> struct bcm2835_cprman *cprman;
> struct resource *res;
> + size_t i;
> +
> + /* find the max clock index */
> + clk_cnt =3D BCM2835_CLOCK_PERI_IMAGE; /* see below */
> + for (i =3D 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++)
> + clk_cnt =3D max(clk_cnt, bcm2835_register_clocks[i].index=
);
> + clk_cnt +=3D 1;
I'm not sure how this solution is better than using CLOCK_COUNT. Some
other bindings use a max value, NR_CLKS or other sentinel.
Why did you not choose to set clk_cnt equal to BCM2835_CLOCK_PWM? Why
not initialize it to zero?
> =
> - cprman =3D devm_kzalloc(dev, sizeof(*cprman), GFP_KERNEL);
> + cprman =3D devm_kzalloc(dev,
> + sizeof(*cprman) + clk_cnt * sizeof(*clks),
> + GFP_KERNEL);
> if (!cprman)
> return -ENOMEM;
> =
> @@ -1522,65 +1654,22 @@ static int bcm2835_clk_probe(struct platform_devi=
ce *pdev)
> =
> platform_set_drvdata(pdev, cprman);
> =
> - cprman->onecell.clk_num =3D BCM2835_CLOCK_COUNT;
> + cprman->onecell.clk_num =3D clk_cnt;
> cprman->onecell.clks =3D cprman->clks;
> clks =3D cprman->clks;
> =
> - clks[BCM2835_PLLA] =3D bcm2835_register_pll(cprman, &bcm2835_plla=
_data);
> - clks[BCM2835_PLLB] =3D bcm2835_register_pll(cprman, &bcm2835_pllb=
_data);
> - clks[BCM2835_PLLC] =3D bcm2835_register_pll(cprman, &bcm2835_pllc=
_data);
> - clks[BCM2835_PLLD] =3D bcm2835_register_pll(cprman, &bcm2835_plld=
_data);
> - clks[BCM2835_PLLH] =3D bcm2835_register_pll(cprman, &bcm2835_pllh=
_data);
> -
> - clks[BCM2835_PLLA_CORE] =3D
> - bcm2835_register_pll_divider(cprman, &bcm2835_plla_core_d=
ata);
> - clks[BCM2835_PLLA_PER] =3D
> - bcm2835_register_pll_divider(cprman, &bcm2835_plla_per_da=
ta);
> - clks[BCM2835_PLLC_CORE0] =3D
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllc_core0_=
data);
> - clks[BCM2835_PLLC_CORE1] =3D
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllc_core1_=
data);
> - clks[BCM2835_PLLC_CORE2] =3D
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllc_core2_=
data);
> - clks[BCM2835_PLLC_PER] =3D
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllc_per_da=
ta);
> - clks[BCM2835_PLLD_CORE] =3D
> - bcm2835_register_pll_divider(cprman, &bcm2835_plld_core_d=
ata);
> - clks[BCM2835_PLLD_PER] =3D
> - bcm2835_register_pll_divider(cprman, &bcm2835_plld_per_da=
ta);
> - clks[BCM2835_PLLH_RCAL] =3D
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllh_rcal_d=
ata);
> - clks[BCM2835_PLLH_AUX] =3D
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllh_aux_da=
ta);
> - clks[BCM2835_PLLH_PIX] =3D
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllh_pix_da=
ta);
> -
> - clks[BCM2835_CLOCK_TIMER] =3D
> - bcm2835_register_clock(cprman, &bcm2835_clock_timer_data);
> - clks[BCM2835_CLOCK_OTP] =3D
> - bcm2835_register_clock(cprman, &bcm2835_clock_otp_data);
> - clks[BCM2835_CLOCK_TSENS] =3D
> - bcm2835_register_clock(cprman, &bcm2835_clock_tsens_data);
> - clks[BCM2835_CLOCK_VPU] =3D
> - bcm2835_register_clock(cprman, &bcm2835_clock_vpu_data);
> - clks[BCM2835_CLOCK_V3D] =3D
> - bcm2835_register_clock(cprman, &bcm2835_clock_v3d_data);
> - clks[BCM2835_CLOCK_ISP] =3D
> - bcm2835_register_clock(cprman, &bcm2835_clock_isp_data);
> - clks[BCM2835_CLOCK_H264] =3D
> - bcm2835_register_clock(cprman, &bcm2835_clock_h264_data);
> - clks[BCM2835_CLOCK_V3D] =3D
> - bcm2835_register_clock(cprman, &bcm2835_clock_v3d_data);
> - clks[BCM2835_CLOCK_SDRAM] =3D
> - bcm2835_register_clock(cprman, &bcm2835_clock_sdram_data);
> - clks[BCM2835_CLOCK_UART] =3D
> - bcm2835_register_clock(cprman, &bcm2835_clock_uart_data);
> - clks[BCM2835_CLOCK_VEC] =3D
> - bcm2835_register_clock(cprman, &bcm2835_clock_vec_data);
> - clks[BCM2835_CLOCK_HSM] =3D
> - bcm2835_register_clock(cprman, &bcm2835_clock_hsm_data);
> - clks[BCM2835_CLOCK_EMMC] =3D
> - bcm2835_register_clock(cprman, &bcm2835_clock_emmc_data);
> + /* now register */
> + for (i =3D 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++) {
> + if (clks[bcm2835_register_clocks[i].index])
> + bcm2835_register_duplicate_index(
> + dev, &bcm2835_register_clocks[i],
> + clks[bcm2835_register_clocks[i].index]);
Why is this necessary? When do you have duplicate indices?
Regards,
Mike
> + else
> + clks[bcm2835_register_clocks[i].index] =3D
> + bcm2835_register_pllclock(
> + dev, cprman,
> + &bcm2835_register_clocks[i]);
> + }
> =
> /*
> * CM_PERIICTL (and CM_PERIACTL, CM_SYSCTL and CM_VPUCTL if
> @@ -1594,9 +1683,6 @@ static int bcm2835_clk_probe(struct platform_device=
*pdev)
> cprman->regs + CM_PERIICTL, CM_GATE_BIT,
> 0, &cprman->regs_lock);
> =
> - clks[BCM2835_CLOCK_PWM] =3D
> - bcm2835_register_clock(cprman, &bcm2835_clock_pwm_data);
> -
> return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
> &cprman->onecell);
> }
> diff --git a/include/dt-bindings/clock/bcm2835.h b/include/dt-bindings/cl=
ock/bcm2835.h
> index 61f1d20..87235ac 100644
> --- a/include/dt-bindings/clock/bcm2835.h
> +++ b/include/dt-bindings/clock/bcm2835.h
> @@ -44,5 +44,3 @@
> #define BCM2835_CLOCK_EMMC 28
> #define BCM2835_CLOCK_PERI_IMAGE 29
> #define BCM2835_CLOCK_PWM 30
> -
> -#define BCM2835_CLOCK_COUNT 31
> --
> 1.7.10.4
>=20
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V2 1/4] clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
@ 2016-01-13 20:00 ` Michael Turquette
0 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-01-13 20:00 UTC (permalink / raw)
To: linux-arm-kernel
Hi Martin,
Quoting kernel at martin.sperl.org (2016-01-11 11:55:53)
> From: Martin Sperl <kernel@martin.sperl.org>
>
> As the use of BCM2835_CLOCK_COUNT in
> include/dt-bindings/clock/bcm2835.h is frowned upon as
> it needs to get modified every time a new clock gets introduced
> this patch changes the clk-bcm2835 driver to use a different
> scheme for registration of clocks and pll, so that there
> is no more need for BCM2835_CLOCK_COUNT to be defined.
>
> The way the patch is designed also makes sure to allow control
> the order in which the clocks are defined.
>
> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> ---
> drivers/clk/bcm/clk-bcm2835.c | 208 +++++++++++++++++++++++++----------
> include/dt-bindings/clock/bcm2835.h | 2 -
> 2 files changed, 147 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index 015e687..759202a 100644
> --- a/drivers/clk/bcm/clk-bcm2835.c
> +++ b/drivers/clk/bcm/clk-bcm2835.c
> @@ -288,7 +288,7 @@ struct bcm2835_cprman {
> const char *osc_name;
>
> struct clk_onecell_data onecell;
> - struct clk *clks[BCM2835_CLOCK_COUNT];
> + struct clk *clks[];
> };
>
> static inline void cprman_write(struct bcm2835_cprman *cprman, u32 reg, u32 val)
> @@ -1498,14 +1498,146 @@ static struct clk *bcm2835_register_clock(struct bcm2835_cprman *cprman,
> return devm_clk_register(cprman->dev, &clock->hw);
> }
>
> +enum bcm2835_clk_register {
> + bcm2835_clk_register_pll,
> + bcm2835_clk_register_pll_div,
> + bcm2835_clk_register_clock,
> +};
> +
> +/*the list of clocks and plls to register */
> +enum bcm2835_register_clock_type {
Nitpick: s/clock/clk/
It helps with grepping later on. Typically "clock" is used for OF/DT
stuff and clk is used for Linux-specific stuff. This isn't a rule, but
seems to be the de facto standard.
> + CLK_TYPE_UNSET,
> + CLK_TYPE_PLL,
> + CLK_TYPE_PLL_DIV,
> + CLK_TYPE_CLOCK,
> +};
> +
> +struct bcm2835_register_clock {
Nitpick: how about bcm2835_clk_desc ?
> + size_t index;
> + enum bcm2835_register_clock_type clock_type;
> + const void *data;
> +};
> +
> +#define _REGISTER(i, t, d) { .index = i, .clock_type = t, .data = d }
> +
> +#define REGISTER_PLL(i, d) _REGISTER(i, CLK_TYPE_PLL, d)
> +#define REGISTER_PLL_DIV(i, d) _REGISTER(i, CLK_TYPE_PLL_DIV, d)
> +#define REGISTER_CLOCK(i, d) _REGISTER(i, CLK_TYPE_CLOCK, d)
> +
> +/*
> + * note that this array is processed first to last,
> + * so that we can define inititalization order.
> + * with the order right now: pll, pll_div and then clock
> + * this allows to retain the existing id- mapping in the device tree.
> + * ths also means we have to have some more explicit coding
> + * and can not use sparse arrays or similar.
> + */
> +static const struct bcm2835_register_clock bcm2835_register_clocks[] = {
> + /* the PLL clocks */
> + REGISTER_PLL(BCM2835_PLLA, &bcm2835_plla_data),
> + REGISTER_PLL(BCM2835_PLLB, &bcm2835_pllb_data),
> + REGISTER_PLL(BCM2835_PLLC, &bcm2835_pllc_data),
> + REGISTER_PLL(BCM2835_PLLD, &bcm2835_plld_data),
> + REGISTER_PLL(BCM2835_PLLH, &bcm2835_pllh_data),
> + /* the PLL dividers */
> + REGISTER_PLL_DIV(BCM2835_PLLA_CORE, &bcm2835_plla_core_data),
> + REGISTER_PLL_DIV(BCM2835_PLLA_PER, &bcm2835_plla_per_data),
> + REGISTER_PLL_DIV(BCM2835_PLLC_CORE0, &bcm2835_pllc_core0_data),
> + REGISTER_PLL_DIV(BCM2835_PLLC_CORE1, &bcm2835_pllc_core1_data),
> + REGISTER_PLL_DIV(BCM2835_PLLC_CORE2, &bcm2835_pllc_core2_data),
> + REGISTER_PLL_DIV(BCM2835_PLLC_PER, &bcm2835_pllc_per_data),
> + REGISTER_PLL_DIV(BCM2835_PLLD_CORE, &bcm2835_plld_core_data),
> + REGISTER_PLL_DIV(BCM2835_PLLD_PER, &bcm2835_plld_per_data),
> + REGISTER_PLL_DIV(BCM2835_PLLH_RCAL, &bcm2835_pllh_rcal_data),
> + REGISTER_PLL_DIV(BCM2835_PLLH_AUX, &bcm2835_pllh_aux_data),
> + REGISTER_PLL_DIV(BCM2835_PLLH_PIX, &bcm2835_pllh_pix_data),
> + /* the clocks */
> + REGISTER_CLOCK(BCM2835_CLOCK_TIMER, &bcm2835_clock_timer_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_OTP, &bcm2835_clock_otp_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_TSENS, &bcm2835_clock_tsens_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_VPU, &bcm2835_clock_vpu_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_V3D, &bcm2835_clock_v3d_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_ISP, &bcm2835_clock_isp_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_H264, &bcm2835_clock_h264_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_V3D, &bcm2835_clock_v3d_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_SDRAM, &bcm2835_clock_sdram_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_UART, &bcm2835_clock_uart_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_VEC, &bcm2835_clock_vec_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_HSM, &bcm2835_clock_hsm_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_EMMC, &bcm2835_clock_emmc_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_PWM, &bcm2835_clock_pwm_data),
> +};
> +
> +void bcm2835_register_duplicate_index(
> + struct device *dev, const struct bcm2835_register_clock *reg,
> + struct clk *clk)
> +{
> + const char *name, *type;
> +
> + switch (reg->clock_type) {
> + case CLK_TYPE_PLL:
> + name = ((struct bcm2835_pll_data *)reg->data)->name;
> + type = "pll";
> + break;
> + case CLK_TYPE_PLL_DIV:
> + name = ((struct bcm2835_pll_divider_data *)reg->data)->name;
> + type = "pll divider";
> + break;
> + case CLK_TYPE_CLOCK:
> + name = ((struct bcm2835_clock_data *)reg->data)->name;
> + type = "clock";
> + break;
> + default:
> + dev_err(dev, "Unsupported clock type %d for index %d\n",
> + reg->clock_type, reg->index);
> + return;
> + }
> +
> + dev_err(dev,
> + "Could not register %s \"%s\" because index %i already defined as clock: %pC\n",
> + type, name, reg->index, clk);
> +}
> +
> +struct clk *bcm2835_register_pllclock(
> + struct device *dev, struct bcm2835_cprman *cprman,
> + const struct bcm2835_register_clock *reg)
> +{
> + switch (reg->clock_type) {
> + case CLK_TYPE_PLL:
> + return bcm2835_register_pll(
> + cprman, reg->data);
> + case CLK_TYPE_PLL_DIV:
> + return bcm2835_register_pll_divider(
> + cprman, reg->data);
> + case CLK_TYPE_CLOCK:
> + return bcm2835_register_clock(
> + cprman, reg->data);
> + default:
> + dev_err(dev, "Unsupported clock type %d for index %d\n",
> + reg->clock_type, reg->index);
> + }
> +
> + return NULL;
> +}
> +
> static int bcm2835_clk_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct clk **clks;
> + size_t clk_cnt;
> struct bcm2835_cprman *cprman;
> struct resource *res;
> + size_t i;
> +
> + /* find the max clock index */
> + clk_cnt = BCM2835_CLOCK_PERI_IMAGE; /* see below */
> + for (i = 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++)
> + clk_cnt = max(clk_cnt, bcm2835_register_clocks[i].index);
> + clk_cnt += 1;
I'm not sure how this solution is better than using CLOCK_COUNT. Some
other bindings use a max value, NR_CLKS or other sentinel.
Why did you not choose to set clk_cnt equal to BCM2835_CLOCK_PWM? Why
not initialize it to zero?
>
> - cprman = devm_kzalloc(dev, sizeof(*cprman), GFP_KERNEL);
> + cprman = devm_kzalloc(dev,
> + sizeof(*cprman) + clk_cnt * sizeof(*clks),
> + GFP_KERNEL);
> if (!cprman)
> return -ENOMEM;
>
> @@ -1522,65 +1654,22 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, cprman);
>
> - cprman->onecell.clk_num = BCM2835_CLOCK_COUNT;
> + cprman->onecell.clk_num = clk_cnt;
> cprman->onecell.clks = cprman->clks;
> clks = cprman->clks;
>
> - clks[BCM2835_PLLA] = bcm2835_register_pll(cprman, &bcm2835_plla_data);
> - clks[BCM2835_PLLB] = bcm2835_register_pll(cprman, &bcm2835_pllb_data);
> - clks[BCM2835_PLLC] = bcm2835_register_pll(cprman, &bcm2835_pllc_data);
> - clks[BCM2835_PLLD] = bcm2835_register_pll(cprman, &bcm2835_plld_data);
> - clks[BCM2835_PLLH] = bcm2835_register_pll(cprman, &bcm2835_pllh_data);
> -
> - clks[BCM2835_PLLA_CORE] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_plla_core_data);
> - clks[BCM2835_PLLA_PER] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_plla_per_data);
> - clks[BCM2835_PLLC_CORE0] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllc_core0_data);
> - clks[BCM2835_PLLC_CORE1] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllc_core1_data);
> - clks[BCM2835_PLLC_CORE2] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllc_core2_data);
> - clks[BCM2835_PLLC_PER] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllc_per_data);
> - clks[BCM2835_PLLD_CORE] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_plld_core_data);
> - clks[BCM2835_PLLD_PER] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_plld_per_data);
> - clks[BCM2835_PLLH_RCAL] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllh_rcal_data);
> - clks[BCM2835_PLLH_AUX] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllh_aux_data);
> - clks[BCM2835_PLLH_PIX] =
> - bcm2835_register_pll_divider(cprman, &bcm2835_pllh_pix_data);
> -
> - clks[BCM2835_CLOCK_TIMER] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_timer_data);
> - clks[BCM2835_CLOCK_OTP] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_otp_data);
> - clks[BCM2835_CLOCK_TSENS] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_tsens_data);
> - clks[BCM2835_CLOCK_VPU] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_vpu_data);
> - clks[BCM2835_CLOCK_V3D] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_v3d_data);
> - clks[BCM2835_CLOCK_ISP] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_isp_data);
> - clks[BCM2835_CLOCK_H264] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_h264_data);
> - clks[BCM2835_CLOCK_V3D] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_v3d_data);
> - clks[BCM2835_CLOCK_SDRAM] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_sdram_data);
> - clks[BCM2835_CLOCK_UART] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_uart_data);
> - clks[BCM2835_CLOCK_VEC] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_vec_data);
> - clks[BCM2835_CLOCK_HSM] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_hsm_data);
> - clks[BCM2835_CLOCK_EMMC] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_emmc_data);
> + /* now register */
> + for (i = 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++) {
> + if (clks[bcm2835_register_clocks[i].index])
> + bcm2835_register_duplicate_index(
> + dev, &bcm2835_register_clocks[i],
> + clks[bcm2835_register_clocks[i].index]);
Why is this necessary? When do you have duplicate indices?
Regards,
Mike
> + else
> + clks[bcm2835_register_clocks[i].index] =
> + bcm2835_register_pllclock(
> + dev, cprman,
> + &bcm2835_register_clocks[i]);
> + }
>
> /*
> * CM_PERIICTL (and CM_PERIACTL, CM_SYSCTL and CM_VPUCTL if
> @@ -1594,9 +1683,6 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
> cprman->regs + CM_PERIICTL, CM_GATE_BIT,
> 0, &cprman->regs_lock);
>
> - clks[BCM2835_CLOCK_PWM] =
> - bcm2835_register_clock(cprman, &bcm2835_clock_pwm_data);
> -
> return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
> &cprman->onecell);
> }
> diff --git a/include/dt-bindings/clock/bcm2835.h b/include/dt-bindings/clock/bcm2835.h
> index 61f1d20..87235ac 100644
> --- a/include/dt-bindings/clock/bcm2835.h
> +++ b/include/dt-bindings/clock/bcm2835.h
> @@ -44,5 +44,3 @@
> #define BCM2835_CLOCK_EMMC 28
> #define BCM2835_CLOCK_PERI_IMAGE 29
> #define BCM2835_CLOCK_PWM 30
> -
> -#define BCM2835_CLOCK_COUNT 31
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V2 2/4] clk: bcm2835: enable fractional and mash support
2016-01-11 19:55 ` kernel at martin.sperl.org
(?)
@ 2016-01-13 20:07 ` Michael Turquette
-1 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-01-13 20:07 UTC (permalink / raw)
To: Stephen Boyd, Stephen Warren, Lee Jones, Eric Anholt,
Remi Pommarel, linux-clk, linux-rpi-kernel, linux-arm-kernel,
devicetree
Cc: Martin Sperl
Hi Martin,
Quoting kernel@martin.sperl.org (2016-01-11 11:55:54)
> @@ -1274,9 +1283,50 @@ static int bcm2835_clock_set_rate(struct clk_hw *hw,
> struct bcm2835_cprman *cprman = clock->cprman;
> const struct bcm2835_clock_data *data = clock->data;
> u32 div = bcm2835_clock_choose_div(hw, rate, parent_rate, false);
> + u32 ctl, mash;
> + bool enabled;
>
> + spin_lock(&cprman->regs_lock);
> + /* check if divider is identical, then return */
> + if (div == cprman_read(cprman, data->div_reg))
> + goto unlock;
> +
> + /* it is recommended to only set clock registers when disabled */
> + ctl = cprman_read(cprman, data->ctl_reg);
> + enabled = ctl & CM_ENABLE;
> + if (enabled) {
> + /* disable clock */
> + cprman_write(cprman, data->ctl_reg, ctl);
This seems unsafe to me. Any IP block consuming this signal will have
its clock cut without warning!
If you need to enforce this behavior we provide the CLK_SET_RATE_GATE
flag to handle it at the framework level.
Regards,
Mike
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V2 2/4] clk: bcm2835: enable fractional and mash support
@ 2016-01-13 20:07 ` Michael Turquette
0 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-01-13 20:07 UTC (permalink / raw)
To: kernel, Stephen Boyd, Stephen Warren, Lee Jones, Eric Anholt,
Remi Pommarel, linux-clk, linux-rpi-kernel, linux-arm-kernel,
devicetree
Cc: Martin Sperl
Hi Martin,
Quoting kernel@martin.sperl.org (2016-01-11 11:55:54)
> @@ -1274,9 +1283,50 @@ static int bcm2835_clock_set_rate(struct clk_hw *h=
w,
> struct bcm2835_cprman *cprman =3D clock->cprman;
> const struct bcm2835_clock_data *data =3D clock->data;
> u32 div =3D bcm2835_clock_choose_div(hw, rate, parent_rate, false=
);
> + u32 ctl, mash;
> + bool enabled;
> =
> + spin_lock(&cprman->regs_lock);
> + /* check if divider is identical, then return */
> + if (div =3D=3D cprman_read(cprman, data->div_reg))
> + goto unlock;
> +
> + /* it is recommended to only set clock registers when disabled */
> + ctl =3D cprman_read(cprman, data->ctl_reg);
> + enabled =3D ctl & CM_ENABLE;
> + if (enabled) {
> + /* disable clock */
> + cprman_write(cprman, data->ctl_reg, ctl);
This seems unsafe to me. Any IP block consuming this signal will have
its clock cut without warning!
If you need to enforce this behavior we provide the CLK_SET_RATE_GATE
flag to handle it at the framework level.
Regards,
Mike
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V2 2/4] clk: bcm2835: enable fractional and mash support
@ 2016-01-13 20:07 ` Michael Turquette
0 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-01-13 20:07 UTC (permalink / raw)
To: linux-arm-kernel
Hi Martin,
Quoting kernel at martin.sperl.org (2016-01-11 11:55:54)
> @@ -1274,9 +1283,50 @@ static int bcm2835_clock_set_rate(struct clk_hw *hw,
> struct bcm2835_cprman *cprman = clock->cprman;
> const struct bcm2835_clock_data *data = clock->data;
> u32 div = bcm2835_clock_choose_div(hw, rate, parent_rate, false);
> + u32 ctl, mash;
> + bool enabled;
>
> + spin_lock(&cprman->regs_lock);
> + /* check if divider is identical, then return */
> + if (div == cprman_read(cprman, data->div_reg))
> + goto unlock;
> +
> + /* it is recommended to only set clock registers when disabled */
> + ctl = cprman_read(cprman, data->ctl_reg);
> + enabled = ctl & CM_ENABLE;
> + if (enabled) {
> + /* disable clock */
> + cprman_write(cprman, data->ctl_reg, ctl);
This seems unsafe to me. Any IP block consuming this signal will have
its clock cut without warning!
If you need to enforce this behavior we provide the CLK_SET_RATE_GATE
flag to handle it at the framework level.
Regards,
Mike
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V2 3/4] clk: bcm2835: enable management of PCM clock
2016-01-11 19:55 ` kernel at martin.sperl.org
(?)
@ 2016-01-13 20:11 ` Michael Turquette
-1 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-01-13 20:11 UTC (permalink / raw)
To: Stephen Boyd, Stephen Warren, Lee Jones, Eric Anholt,
Remi Pommarel, linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Martin Sperl
Quoting kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org (2016-01-11 11:55:55)
> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>
> Enable the PCM clock in the SOC, which is used by the
> bcm2835-i2s driver.
>
> Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
Looks OK to me.
Regards,
Mike
> ---
> drivers/clk/bcm/clk-bcm2835.c | 13 +++++++++++++
> include/dt-bindings/clock/bcm2835.h | 1 +
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index 7c782d3..573b5b1 100644
> --- a/drivers/clk/bcm/clk-bcm2835.c
> +++ b/drivers/clk/bcm/clk-bcm2835.c
> @@ -88,6 +88,8 @@
> #define CM_HSMDIV 0x08c
> #define CM_OTPCTL 0x090
> #define CM_OTPDIV 0x094
> +#define CM_PCMCTL 0x098
> +#define CM_PCMDIV 0x09c
> #define CM_PWMCTL 0x0a0
> #define CM_PWMDIV 0x0a4
> #define CM_SMICTL 0x0b0
> @@ -826,6 +828,16 @@ static const struct bcm2835_clock_data bcm2835_clock_pwm_data = {
> .frac_bits = 12,
> };
>
> +static const struct bcm2835_clock_data bcm2835_clock_pcm_data = {
> + .name = "pcm",
> + .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
> + .parents = bcm2835_clock_per_parents,
> + .ctl_reg = CM_PCMCTL,
> + .div_reg = CM_PCMDIV,
> + .int_bits = 12,
> + .frac_bits = 12,
> +};
> +
> struct bcm2835_pll {
> struct clk_hw hw;
> struct bcm2835_cprman *cprman;
> @@ -1616,6 +1628,7 @@ static const struct bcm2835_register_clock bcm2835_register_clocks[] = {
> REGISTER_CLOCK(BCM2835_CLOCK_HSM, &bcm2835_clock_hsm_data),
> REGISTER_CLOCK(BCM2835_CLOCK_EMMC, &bcm2835_clock_emmc_data),
> REGISTER_CLOCK(BCM2835_CLOCK_PWM, &bcm2835_clock_pwm_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_PCM, &bcm2835_clock_pcm_data),
> };
>
> void bcm2835_register_duplicate_index(
> diff --git a/include/dt-bindings/clock/bcm2835.h b/include/dt-bindings/clock/bcm2835.h
> index 87235ac..9a7b4a5 100644
> --- a/include/dt-bindings/clock/bcm2835.h
> +++ b/include/dt-bindings/clock/bcm2835.h
> @@ -44,3 +44,4 @@
> #define BCM2835_CLOCK_EMMC 28
> #define BCM2835_CLOCK_PERI_IMAGE 29
> #define BCM2835_CLOCK_PWM 30
> +#define BCM2835_CLOCK_PCM 31
> --
> 1.7.10.4
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V2 3/4] clk: bcm2835: enable management of PCM clock
@ 2016-01-13 20:11 ` Michael Turquette
0 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-01-13 20:11 UTC (permalink / raw)
To: kernel, Stephen Boyd, Stephen Warren, Lee Jones, Eric Anholt,
Remi Pommarel, linux-clk, linux-rpi-kernel, linux-arm-kernel,
devicetree
Cc: Martin Sperl
Quoting kernel@martin.sperl.org (2016-01-11 11:55:55)
> From: Martin Sperl <kernel@martin.sperl.org>
> =
> Enable the PCM clock in the SOC, which is used by the
> bcm2835-i2s driver.
> =
> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Looks OK to me.
Regards,
Mike
> ---
> drivers/clk/bcm/clk-bcm2835.c | 13 +++++++++++++
> include/dt-bindings/clock/bcm2835.h | 1 +
> 2 files changed, 14 insertions(+)
> =
> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index 7c782d3..573b5b1 100644
> --- a/drivers/clk/bcm/clk-bcm2835.c
> +++ b/drivers/clk/bcm/clk-bcm2835.c
> @@ -88,6 +88,8 @@
> #define CM_HSMDIV 0x08c
> #define CM_OTPCTL 0x090
> #define CM_OTPDIV 0x094
> +#define CM_PCMCTL 0x098
> +#define CM_PCMDIV 0x09c
> #define CM_PWMCTL 0x0a0
> #define CM_PWMDIV 0x0a4
> #define CM_SMICTL 0x0b0
> @@ -826,6 +828,16 @@ static const struct bcm2835_clock_data bcm2835_clock=
_pwm_data =3D {
> .frac_bits =3D 12,
> };
> =
> +static const struct bcm2835_clock_data bcm2835_clock_pcm_data =3D {
> + .name =3D "pcm",
> + .num_mux_parents =3D ARRAY_SIZE(bcm2835_clock_per_parents),
> + .parents =3D bcm2835_clock_per_parents,
> + .ctl_reg =3D CM_PCMCTL,
> + .div_reg =3D CM_PCMDIV,
> + .int_bits =3D 12,
> + .frac_bits =3D 12,
> +};
> +
> struct bcm2835_pll {
> struct clk_hw hw;
> struct bcm2835_cprman *cprman;
> @@ -1616,6 +1628,7 @@ static const struct bcm2835_register_clock bcm2835_=
register_clocks[] =3D {
> REGISTER_CLOCK(BCM2835_CLOCK_HSM, &bcm2835_clock_hsm_data),
> REGISTER_CLOCK(BCM2835_CLOCK_EMMC, &bcm2835_clock_emmc_data),
> REGISTER_CLOCK(BCM2835_CLOCK_PWM, &bcm2835_clock_pwm_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_PCM, &bcm2835_clock_pcm_data),
> };
> =
> void bcm2835_register_duplicate_index(
> diff --git a/include/dt-bindings/clock/bcm2835.h b/include/dt-bindings/cl=
ock/bcm2835.h
> index 87235ac..9a7b4a5 100644
> --- a/include/dt-bindings/clock/bcm2835.h
> +++ b/include/dt-bindings/clock/bcm2835.h
> @@ -44,3 +44,4 @@
> #define BCM2835_CLOCK_EMMC 28
> #define BCM2835_CLOCK_PERI_IMAGE 29
> #define BCM2835_CLOCK_PWM 30
> +#define BCM2835_CLOCK_PCM 31
> -- =
> 1.7.10.4
>=20
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V2 3/4] clk: bcm2835: enable management of PCM clock
@ 2016-01-13 20:11 ` Michael Turquette
0 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-01-13 20:11 UTC (permalink / raw)
To: linux-arm-kernel
Quoting kernel at martin.sperl.org (2016-01-11 11:55:55)
> From: Martin Sperl <kernel@martin.sperl.org>
>
> Enable the PCM clock in the SOC, which is used by the
> bcm2835-i2s driver.
>
> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Looks OK to me.
Regards,
Mike
> ---
> drivers/clk/bcm/clk-bcm2835.c | 13 +++++++++++++
> include/dt-bindings/clock/bcm2835.h | 1 +
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index 7c782d3..573b5b1 100644
> --- a/drivers/clk/bcm/clk-bcm2835.c
> +++ b/drivers/clk/bcm/clk-bcm2835.c
> @@ -88,6 +88,8 @@
> #define CM_HSMDIV 0x08c
> #define CM_OTPCTL 0x090
> #define CM_OTPDIV 0x094
> +#define CM_PCMCTL 0x098
> +#define CM_PCMDIV 0x09c
> #define CM_PWMCTL 0x0a0
> #define CM_PWMDIV 0x0a4
> #define CM_SMICTL 0x0b0
> @@ -826,6 +828,16 @@ static const struct bcm2835_clock_data bcm2835_clock_pwm_data = {
> .frac_bits = 12,
> };
>
> +static const struct bcm2835_clock_data bcm2835_clock_pcm_data = {
> + .name = "pcm",
> + .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents),
> + .parents = bcm2835_clock_per_parents,
> + .ctl_reg = CM_PCMCTL,
> + .div_reg = CM_PCMDIV,
> + .int_bits = 12,
> + .frac_bits = 12,
> +};
> +
> struct bcm2835_pll {
> struct clk_hw hw;
> struct bcm2835_cprman *cprman;
> @@ -1616,6 +1628,7 @@ static const struct bcm2835_register_clock bcm2835_register_clocks[] = {
> REGISTER_CLOCK(BCM2835_CLOCK_HSM, &bcm2835_clock_hsm_data),
> REGISTER_CLOCK(BCM2835_CLOCK_EMMC, &bcm2835_clock_emmc_data),
> REGISTER_CLOCK(BCM2835_CLOCK_PWM, &bcm2835_clock_pwm_data),
> + REGISTER_CLOCK(BCM2835_CLOCK_PCM, &bcm2835_clock_pcm_data),
> };
>
> void bcm2835_register_duplicate_index(
> diff --git a/include/dt-bindings/clock/bcm2835.h b/include/dt-bindings/clock/bcm2835.h
> index 87235ac..9a7b4a5 100644
> --- a/include/dt-bindings/clock/bcm2835.h
> +++ b/include/dt-bindings/clock/bcm2835.h
> @@ -44,3 +44,4 @@
> #define BCM2835_CLOCK_EMMC 28
> #define BCM2835_CLOCK_PERI_IMAGE 29
> #define BCM2835_CLOCK_PWM 30
> +#define BCM2835_CLOCK_PCM 31
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V2 1/4] clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
2016-01-13 20:00 ` Michael Turquette
(?)
@ 2016-01-14 0:13 ` Michael Turquette
-1 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-01-14 0:13 UTC (permalink / raw)
To: Stephen Boyd, Stephen Warren, Lee Jones, Eric Anholt,
Remi Pommarel, linux-clk, linux-rpi-kernel, linux-arm-kernel,
devicetree
Cc: Martin Sperl
Quoting Michael Turquette (2016-01-13 12:00:12)
> Hi Martin,
>
> Quoting kernel@martin.sperl.org (2016-01-11 11:55:53)
> > static int bcm2835_clk_probe(struct platform_device *pdev)
> > {
> > struct device *dev = &pdev->dev;
> > struct clk **clks;
> > + size_t clk_cnt;
> > struct bcm2835_cprman *cprman;
> > struct resource *res;
> > + size_t i;
> > +
> > + /* find the max clock index */
> > + clk_cnt = BCM2835_CLOCK_PERI_IMAGE; /* see below */
> > + for (i = 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++)
> > + clk_cnt = max(clk_cnt, bcm2835_register_clocks[i].index);
> > + clk_cnt += 1;
>
> I'm not sure how this solution is better than using CLOCK_COUNT. Some
> other bindings use a max value, NR_CLKS or other sentinel.
>
> Why did you not choose to set clk_cnt equal to BCM2835_CLOCK_PWM? Why
> not initialize it to zero?
OK, I just caught up on the asoc/bcm2835 thread.
Really the best solution would be to have an array of all of the clks in
the driver and just use ARRAY_SIZE on it.
For your driver, could you make an array of clk_hw pointers and call
devm_clk_register on all of them in a loop? This gets rid of the big
pile of explicit calls in bcm2835_clk_probe.
You can also get rid of stuff like bcm2835_plla_core_data by just
stuffing that data into a single struct initialization. Here is a
snippet for how the qcom clk drivers do it:
static struct clk_pll gpll0 = {
.l_reg = 0x0004,
.m_reg = 0x0008,
.n_reg = 0x000c,
.config_reg = 0x0014,
.mode_reg = 0x0000,
.status_reg = 0x001c,
.status_bit = 17,
.clkr.hw.init = &(struct clk_init_data){
.name = "gpll0",
.parent_names = (const char *[]){ "xo" },
.num_parents = 1,
.ops = &clk_pll_ops,
},
};
Then a single array of clks per driver references this:
static struct clk_regmap *gcc_apq8084_clocks[] = {
[GPLL0] = &gpll0.clkr,
...
(in your case you won't reference struct clk_regmap, but struct clk_hw)
The actual registration happens in your probe like so:
static int bcm2835_clk_probe(struct platform_device *pdev)
{
...
for (i = 0; i < num_clks; i++) {
clk = devm_clk_register(dev, array_of_clks[i].hw)
...
}
Regards,
Mike
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V2 1/4] clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
@ 2016-01-14 0:13 ` Michael Turquette
0 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-01-14 0:13 UTC (permalink / raw)
To: kernel, Stephen Boyd, Stephen Warren, Lee Jones, Eric Anholt,
Remi Pommarel, linux-clk, linux-rpi-kernel, linux-arm-kernel,
devicetree
Cc: Martin Sperl
Quoting Michael Turquette (2016-01-13 12:00:12)
> Hi Martin,
> =
> Quoting kernel@martin.sperl.org (2016-01-11 11:55:53)
> > static int bcm2835_clk_probe(struct platform_device *pdev)
> > {
> > struct device *dev =3D &pdev->dev;
> > struct clk **clks;
> > + size_t clk_cnt;
> > struct bcm2835_cprman *cprman;
> > struct resource *res;
> > + size_t i;
> > +
> > + /* find the max clock index */
> > + clk_cnt =3D BCM2835_CLOCK_PERI_IMAGE; /* see below */
> > + for (i =3D 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++)
> > + clk_cnt =3D max(clk_cnt, bcm2835_register_clocks[i].ind=
ex);
> > + clk_cnt +=3D 1;
> =
> I'm not sure how this solution is better than using CLOCK_COUNT. Some
> other bindings use a max value, NR_CLKS or other sentinel.
> =
> Why did you not choose to set clk_cnt equal to BCM2835_CLOCK_PWM? Why
> not initialize it to zero?
OK, I just caught up on the asoc/bcm2835 thread.
Really the best solution would be to have an array of all of the clks in
the driver and just use ARRAY_SIZE on it.
For your driver, could you make an array of clk_hw pointers and call
devm_clk_register on all of them in a loop? This gets rid of the big
pile of explicit calls in bcm2835_clk_probe.
You can also get rid of stuff like bcm2835_plla_core_data by just
stuffing that data into a single struct initialization. Here is a
snippet for how the qcom clk drivers do it:
static struct clk_pll gpll0 =3D {
.l_reg =3D 0x0004,
.m_reg =3D 0x0008,
.n_reg =3D 0x000c,
.config_reg =3D 0x0014,
.mode_reg =3D 0x0000,
.status_reg =3D 0x001c,
.status_bit =3D 17,
.clkr.hw.init =3D &(struct clk_init_data){
.name =3D "gpll0",
.parent_names =3D (const char *[]){ "xo" },
.num_parents =3D 1,
.ops =3D &clk_pll_ops,
},
};
Then a single array of clks per driver references this:
static struct clk_regmap *gcc_apq8084_clocks[] =3D {
[GPLL0] =3D &gpll0.clkr,
...
(in your case you won't reference struct clk_regmap, but struct clk_hw)
The actual registration happens in your probe like so:
static int bcm2835_clk_probe(struct platform_device *pdev)
{
...
for (i =3D 0; i < num_clks; i++) {
clk =3D devm_clk_register(dev, array_of_clks[i].hw)
...
}
Regards,
Mike
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V2 1/4] clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
@ 2016-01-14 0:13 ` Michael Turquette
0 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-01-14 0:13 UTC (permalink / raw)
To: linux-arm-kernel
Quoting Michael Turquette (2016-01-13 12:00:12)
> Hi Martin,
>
> Quoting kernel at martin.sperl.org (2016-01-11 11:55:53)
> > static int bcm2835_clk_probe(struct platform_device *pdev)
> > {
> > struct device *dev = &pdev->dev;
> > struct clk **clks;
> > + size_t clk_cnt;
> > struct bcm2835_cprman *cprman;
> > struct resource *res;
> > + size_t i;
> > +
> > + /* find the max clock index */
> > + clk_cnt = BCM2835_CLOCK_PERI_IMAGE; /* see below */
> > + for (i = 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++)
> > + clk_cnt = max(clk_cnt, bcm2835_register_clocks[i].index);
> > + clk_cnt += 1;
>
> I'm not sure how this solution is better than using CLOCK_COUNT. Some
> other bindings use a max value, NR_CLKS or other sentinel.
>
> Why did you not choose to set clk_cnt equal to BCM2835_CLOCK_PWM? Why
> not initialize it to zero?
OK, I just caught up on the asoc/bcm2835 thread.
Really the best solution would be to have an array of all of the clks in
the driver and just use ARRAY_SIZE on it.
For your driver, could you make an array of clk_hw pointers and call
devm_clk_register on all of them in a loop? This gets rid of the big
pile of explicit calls in bcm2835_clk_probe.
You can also get rid of stuff like bcm2835_plla_core_data by just
stuffing that data into a single struct initialization. Here is a
snippet for how the qcom clk drivers do it:
static struct clk_pll gpll0 = {
.l_reg = 0x0004,
.m_reg = 0x0008,
.n_reg = 0x000c,
.config_reg = 0x0014,
.mode_reg = 0x0000,
.status_reg = 0x001c,
.status_bit = 17,
.clkr.hw.init = &(struct clk_init_data){
.name = "gpll0",
.parent_names = (const char *[]){ "xo" },
.num_parents = 1,
.ops = &clk_pll_ops,
},
};
Then a single array of clks per driver references this:
static struct clk_regmap *gcc_apq8084_clocks[] = {
[GPLL0] = &gpll0.clkr,
...
(in your case you won't reference struct clk_regmap, but struct clk_hw)
The actual registration happens in your probe like so:
static int bcm2835_clk_probe(struct platform_device *pdev)
{
...
for (i = 0; i < num_clks; i++) {
clk = devm_clk_register(dev, array_of_clks[i].hw)
...
}
Regards,
Mike
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V2 1/4] clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
2016-01-14 0:13 ` Michael Turquette
@ 2016-01-14 12:11 ` Martin Sperl
-1 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-01-14 12:11 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Stephen Warren, Lee Jones,
Eric Anholt, Remi Pommarel, linux-clk, linux-rpi-kernel,
linux-arm-kernel, devicetree
On 14.01.2016 01:13, Michael Turquette wrote:
> Quoting Michael Turquette (2016-01-13 12:00:12)
>> Hi Martin,
>>
>> Quoting kernel@martin.sperl.org (2016-01-11 11:55:53)
>>> static int bcm2835_clk_probe(struct platform_device *pdev)
>>> {
>>> struct device *dev = &pdev->dev;
>>> struct clk **clks;
>>> + size_t clk_cnt;
>>> struct bcm2835_cprman *cprman;
>>> struct resource *res;
>>> + size_t i;
>>> +
>>> + /* find the max clock index */
>>> + clk_cnt = BCM2835_CLOCK_PERI_IMAGE; /* see below */
>>> + for (i = 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++)
>>> + clk_cnt = max(clk_cnt, bcm2835_register_clocks[i].index);
>>> + clk_cnt += 1;
>>
>> I'm not sure how this solution is better than using CLOCK_COUNT. Some
>> other bindings use a max value, NR_CLKS or other sentinel.
>>
>> Why did you not choose to set clk_cnt equal to BCM2835_CLOCK_PWM? Why
>> not initialize it to zero?
>
> OK, I just caught up on the asoc/bcm2835 thread.
>
> Really the best solution would be to have an array of all of the clks in
> the driver and just use ARRAY_SIZE on it.
>
> For your driver, could you make an array of clk_hw pointers and call
> devm_clk_register on all of them in a loop? This gets rid of the big
> pile of explicit calls in bcm2835_clk_probe.
>
> You can also get rid of stuff like bcm2835_plla_core_data by just
> stuffing that data into a single struct initialization. Here is a
> snippet for how the qcom clk drivers do it:
>
> static struct clk_pll gpll0 = {
> .l_reg = 0x0004,
> .m_reg = 0x0008,
> .n_reg = 0x000c,
> .config_reg = 0x0014,
> .mode_reg = 0x0000,
> .status_reg = 0x001c,
> .status_bit = 17,
> .clkr.hw.init = &(struct clk_init_data){
> .name = "gpll0",
> .parent_names = (const char *[]){ "xo" },
> .num_parents = 1,
> .ops = &clk_pll_ops,
> },
> };
I did not know you could do that - that could make life easier...
But a quick look shows that this approach probably would require a
major rewrite of all the methods.
> static int bcm2835_clk_probe(struct platform_device *pdev)
> {
> ...
> for (i = 0; i < num_clks; i++) {
> clk = devm_clk_register(dev, array_of_clks[i].hw)
> ...
> }
I guess I can use a slightly different approach that does not
require as many changes, that looks like this:
static const struct bcm2835_clk_desc clk_desc_array[] = {
/* register PLL */
[BCM2835_PLLA] = REGISTER_PLL(&bcm2835_plla_data),
...
};
...
static int bcm2835_clk_probe(struct platform_device *pdev)
{
const size_t asize = ARRAY_SIZE(clk_desc_array);
...
for (i = 0; i < asize; i++) {
desc = &clk_desc_array[i];
if (desc)
clks[i] = desc->clk_register(cprman,
desc->data);
}
...
}
If we need to order the initialization then we can add some
priority field to clk_desc_array and iterate over all the priority
values.
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V2 1/4] clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
@ 2016-01-14 12:11 ` Martin Sperl
0 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-01-14 12:11 UTC (permalink / raw)
To: linux-arm-kernel
On 14.01.2016 01:13, Michael Turquette wrote:
> Quoting Michael Turquette (2016-01-13 12:00:12)
>> Hi Martin,
>>
>> Quoting kernel at martin.sperl.org (2016-01-11 11:55:53)
>>> static int bcm2835_clk_probe(struct platform_device *pdev)
>>> {
>>> struct device *dev = &pdev->dev;
>>> struct clk **clks;
>>> + size_t clk_cnt;
>>> struct bcm2835_cprman *cprman;
>>> struct resource *res;
>>> + size_t i;
>>> +
>>> + /* find the max clock index */
>>> + clk_cnt = BCM2835_CLOCK_PERI_IMAGE; /* see below */
>>> + for (i = 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++)
>>> + clk_cnt = max(clk_cnt, bcm2835_register_clocks[i].index);
>>> + clk_cnt += 1;
>>
>> I'm not sure how this solution is better than using CLOCK_COUNT. Some
>> other bindings use a max value, NR_CLKS or other sentinel.
>>
>> Why did you not choose to set clk_cnt equal to BCM2835_CLOCK_PWM? Why
>> not initialize it to zero?
>
> OK, I just caught up on the asoc/bcm2835 thread.
>
> Really the best solution would be to have an array of all of the clks in
> the driver and just use ARRAY_SIZE on it.
>
> For your driver, could you make an array of clk_hw pointers and call
> devm_clk_register on all of them in a loop? This gets rid of the big
> pile of explicit calls in bcm2835_clk_probe.
>
> You can also get rid of stuff like bcm2835_plla_core_data by just
> stuffing that data into a single struct initialization. Here is a
> snippet for how the qcom clk drivers do it:
>
> static struct clk_pll gpll0 = {
> .l_reg = 0x0004,
> .m_reg = 0x0008,
> .n_reg = 0x000c,
> .config_reg = 0x0014,
> .mode_reg = 0x0000,
> .status_reg = 0x001c,
> .status_bit = 17,
> .clkr.hw.init = &(struct clk_init_data){
> .name = "gpll0",
> .parent_names = (const char *[]){ "xo" },
> .num_parents = 1,
> .ops = &clk_pll_ops,
> },
> };
I did not know you could do that - that could make life easier...
But a quick look shows that this approach probably would require a
major rewrite of all the methods.
> static int bcm2835_clk_probe(struct platform_device *pdev)
> {
> ...
> for (i = 0; i < num_clks; i++) {
> clk = devm_clk_register(dev, array_of_clks[i].hw)
> ...
> }
I guess I can use a slightly different approach that does not
require as many changes, that looks like this:
static const struct bcm2835_clk_desc clk_desc_array[] = {
/* register PLL */
[BCM2835_PLLA] = REGISTER_PLL(&bcm2835_plla_data),
...
};
...
static int bcm2835_clk_probe(struct platform_device *pdev)
{
const size_t asize = ARRAY_SIZE(clk_desc_array);
...
for (i = 0; i < asize; i++) {
desc = &clk_desc_array[i];
if (desc)
clks[i] = desc->clk_register(cprman,
desc->data);
}
...
}
If we need to order the initialization then we can add some
priority field to clk_desc_array and iterate over all the priority
values.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V2 1/4] clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
2016-01-14 12:11 ` Martin Sperl
(?)
@ 2016-01-14 20:23 ` Michael Turquette
-1 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-01-14 20:23 UTC (permalink / raw)
To: Martin Sperl, Stephen Boyd, Stephen Warren, Lee Jones,
Eric Anholt, Remi Pommarel, linux-clk, linux-rpi-kernel,
linux-arm-kernel, devicetree
Quoting Martin Sperl (2016-01-14 04:11:02)
>
>
> On 14.01.2016 01:13, Michael Turquette wrote:
> > Quoting Michael Turquette (2016-01-13 12:00:12)
> >> Hi Martin,
> >>
> >> Quoting kernel@martin.sperl.org (2016-01-11 11:55:53)
> >>> static int bcm2835_clk_probe(struct platform_device *pdev)
> >>> {
> >>> struct device *dev = &pdev->dev;
> >>> struct clk **clks;
> >>> + size_t clk_cnt;
> >>> struct bcm2835_cprman *cprman;
> >>> struct resource *res;
> >>> + size_t i;
> >>> +
> >>> + /* find the max clock index */
> >>> + clk_cnt = BCM2835_CLOCK_PERI_IMAGE; /* see below */
> >>> + for (i = 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++)
> >>> + clk_cnt = max(clk_cnt, bcm2835_register_clocks[i].index);
> >>> + clk_cnt += 1;
> >>
> >> I'm not sure how this solution is better than using CLOCK_COUNT. Some
> >> other bindings use a max value, NR_CLKS or other sentinel.
> >>
> >> Why did you not choose to set clk_cnt equal to BCM2835_CLOCK_PWM? Why
> >> not initialize it to zero?
> >
> > OK, I just caught up on the asoc/bcm2835 thread.
> >
> > Really the best solution would be to have an array of all of the clks in
> > the driver and just use ARRAY_SIZE on it.
> >
> > For your driver, could you make an array of clk_hw pointers and call
> > devm_clk_register on all of them in a loop? This gets rid of the big
> > pile of explicit calls in bcm2835_clk_probe.
> >
> > You can also get rid of stuff like bcm2835_plla_core_data by just
> > stuffing that data into a single struct initialization. Here is a
> > snippet for how the qcom clk drivers do it:
> >
> > static struct clk_pll gpll0 = {
> > .l_reg = 0x0004,
> > .m_reg = 0x0008,
> > .n_reg = 0x000c,
> > .config_reg = 0x0014,
> > .mode_reg = 0x0000,
> > .status_reg = 0x001c,
> > .status_bit = 17,
> > .clkr.hw.init = &(struct clk_init_data){
> > .name = "gpll0",
> > .parent_names = (const char *[]){ "xo" },
> > .num_parents = 1,
> > .ops = &clk_pll_ops,
> > },
> > };
>
> I did not know you could do that - that could make life easier...
>
> But a quick look shows that this approach probably would require a
> major rewrite of all the methods.
>
> > static int bcm2835_clk_probe(struct platform_device *pdev)
> > {
> > ...
> > for (i = 0; i < num_clks; i++) {
> > clk = devm_clk_register(dev, array_of_clks[i].hw)
> > ...
> > }
>
> I guess I can use a slightly different approach that does not
> require as many changes, that looks like this:
>
> static const struct bcm2835_clk_desc clk_desc_array[] = {
> /* register PLL */
> [BCM2835_PLLA] = REGISTER_PLL(&bcm2835_plla_data),
> ...
> };
> ...
> static int bcm2835_clk_probe(struct platform_device *pdev)
> {
> const size_t asize = ARRAY_SIZE(clk_desc_array);
> ...
> for (i = 0; i < asize; i++) {
> desc = &clk_desc_array[i];
> if (desc)
> clks[i] = desc->clk_register(cprman,
> desc->data);
Interesting. I have been playing with the idea of putting a .register()
callback into struct clk_init_data. Then it would be called by a new
clk_register_desc() top-level registration function.
You've done that here, but you've put the registration function in your
machine-specific struct. Nothing wrong with that.
Do you actually need a machine-specific registration function? Many clk
drivers only use their machine-specific registration functions to
allocate a stuct clk_hw and populate the contents of struct
clk_init_data, which can be done by the framework and reduce duplicate
code in drivers.
(they do this because the basic clk types like clk-divider, clk-gate,
etc do it, and everyone loves to copy/paste that code)
bcm2835 seems to register two clks in the PLL registration function (one
of which is a fixed factor divider), but you could just add those
post-PLL dividers to your array of clk data?
> }
> ...
> }
>
> If we need to order the initialization then we can add some
> priority field to clk_desc_array and iterate over all the priority
> values.
I guess that you can order them in your array? Just start with the root
clocks at the beginning of the array (regardless of "clk type") and walk
through the array registering them.
Regards,
Mike
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V2 1/4] clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
@ 2016-01-14 20:23 ` Michael Turquette
0 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-01-14 20:23 UTC (permalink / raw)
To: Martin Sperl, Stephen Boyd, Stephen Warren, Lee Jones,
Eric Anholt, Remi Pommarel, linux-clk, linux-rpi-kernel,
linux-arm-kernel, devicetree
Quoting Martin Sperl (2016-01-14 04:11:02)
> =
> =
> On 14.01.2016 01:13, Michael Turquette wrote:
> > Quoting Michael Turquette (2016-01-13 12:00:12)
> >> Hi Martin,
> >>
> >> Quoting kernel@martin.sperl.org (2016-01-11 11:55:53)
> >>> static int bcm2835_clk_probe(struct platform_device *pdev)
> >>> {
> >>> struct device *dev =3D &pdev->dev;
> >>> struct clk **clks;
> >>> + size_t clk_cnt;
> >>> struct bcm2835_cprman *cprman;
> >>> struct resource *res;
> >>> + size_t i;
> >>> +
> >>> + /* find the max clock index */
> >>> + clk_cnt =3D BCM2835_CLOCK_PERI_IMAGE; /* see below */
> >>> + for (i =3D 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++)
> >>> + clk_cnt =3D max(clk_cnt, bcm2835_register_clocks[i].i=
ndex);
> >>> + clk_cnt +=3D 1;
> >>
> >> I'm not sure how this solution is better than using CLOCK_COUNT. Some
> >> other bindings use a max value, NR_CLKS or other sentinel.
> >>
> >> Why did you not choose to set clk_cnt equal to BCM2835_CLOCK_PWM? Why
> >> not initialize it to zero?
> >
> > OK, I just caught up on the asoc/bcm2835 thread.
> >
> > Really the best solution would be to have an array of all of the clks in
> > the driver and just use ARRAY_SIZE on it.
> >
> > For your driver, could you make an array of clk_hw pointers and call
> > devm_clk_register on all of them in a loop? This gets rid of the big
> > pile of explicit calls in bcm2835_clk_probe.
> >
> > You can also get rid of stuff like bcm2835_plla_core_data by just
> > stuffing that data into a single struct initialization. Here is a
> > snippet for how the qcom clk drivers do it:
> >
> > static struct clk_pll gpll0 =3D {
> > .l_reg =3D 0x0004,
> > .m_reg =3D 0x0008,
> > .n_reg =3D 0x000c,
> > .config_reg =3D 0x0014,
> > .mode_reg =3D 0x0000,
> > .status_reg =3D 0x001c,
> > .status_bit =3D 17,
> > .clkr.hw.init =3D &(struct clk_init_data){
> > .name =3D "gpll0",
> > .parent_names =3D (const char *[]){ "xo" },
> > .num_parents =3D 1,
> > .ops =3D &clk_pll_ops,
> > },
> > };
> =
> I did not know you could do that - that could make life easier...
> =
> But a quick look shows that this approach probably would require a
> major rewrite of all the methods.
> =
> > static int bcm2835_clk_probe(struct platform_device *pdev)
> > {
> > ...
> > for (i =3D 0; i < num_clks; i++) {
> > clk =3D devm_clk_register(dev, array_of_clks[i].hw)
> > ...
> > }
> =
> I guess I can use a slightly different approach that does not
> require as many changes, that looks like this:
> =
> static const struct bcm2835_clk_desc clk_desc_array[] =3D {
> /* register PLL */
> [BCM2835_PLLA] =3D REGISTER_PLL(&bcm2835_plla_data),
> ...
> };
> ...
> static int bcm2835_clk_probe(struct platform_device *pdev)
> {
> const size_t asize =3D ARRAY_SIZE(clk_desc_array);
> ...
> for (i =3D 0; i < asize; i++) {
> desc =3D &clk_desc_array[i];
> if (desc)
> clks[i] =3D desc->clk_register(cprman,
> desc->data);
Interesting. I have been playing with the idea of putting a .register()
callback into struct clk_init_data. Then it would be called by a new
clk_register_desc() top-level registration function.
You've done that here, but you've put the registration function in your
machine-specific struct. Nothing wrong with that.
Do you actually need a machine-specific registration function? Many clk
drivers only use their machine-specific registration functions to
allocate a stuct clk_hw and populate the contents of struct
clk_init_data, which can be done by the framework and reduce duplicate
code in drivers.
(they do this because the basic clk types like clk-divider, clk-gate,
etc do it, and everyone loves to copy/paste that code)
bcm2835 seems to register two clks in the PLL registration function (one
of which is a fixed factor divider), but you could just add those
post-PLL dividers to your array of clk data?
> }
> ...
> }
> =
> If we need to order the initialization then we can add some
> priority field to clk_desc_array and iterate over all the priority
> values.
I guess that you can order them in your array? Just start with the root
clocks at the beginning of the array (regardless of "clk type") and walk
through the array registering them.
Regards,
Mike
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V2 1/4] clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
@ 2016-01-14 20:23 ` Michael Turquette
0 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-01-14 20:23 UTC (permalink / raw)
To: linux-arm-kernel
Quoting Martin Sperl (2016-01-14 04:11:02)
>
>
> On 14.01.2016 01:13, Michael Turquette wrote:
> > Quoting Michael Turquette (2016-01-13 12:00:12)
> >> Hi Martin,
> >>
> >> Quoting kernel at martin.sperl.org (2016-01-11 11:55:53)
> >>> static int bcm2835_clk_probe(struct platform_device *pdev)
> >>> {
> >>> struct device *dev = &pdev->dev;
> >>> struct clk **clks;
> >>> + size_t clk_cnt;
> >>> struct bcm2835_cprman *cprman;
> >>> struct resource *res;
> >>> + size_t i;
> >>> +
> >>> + /* find the max clock index */
> >>> + clk_cnt = BCM2835_CLOCK_PERI_IMAGE; /* see below */
> >>> + for (i = 0; i < ARRAY_SIZE(bcm2835_register_clocks); i++)
> >>> + clk_cnt = max(clk_cnt, bcm2835_register_clocks[i].index);
> >>> + clk_cnt += 1;
> >>
> >> I'm not sure how this solution is better than using CLOCK_COUNT. Some
> >> other bindings use a max value, NR_CLKS or other sentinel.
> >>
> >> Why did you not choose to set clk_cnt equal to BCM2835_CLOCK_PWM? Why
> >> not initialize it to zero?
> >
> > OK, I just caught up on the asoc/bcm2835 thread.
> >
> > Really the best solution would be to have an array of all of the clks in
> > the driver and just use ARRAY_SIZE on it.
> >
> > For your driver, could you make an array of clk_hw pointers and call
> > devm_clk_register on all of them in a loop? This gets rid of the big
> > pile of explicit calls in bcm2835_clk_probe.
> >
> > You can also get rid of stuff like bcm2835_plla_core_data by just
> > stuffing that data into a single struct initialization. Here is a
> > snippet for how the qcom clk drivers do it:
> >
> > static struct clk_pll gpll0 = {
> > .l_reg = 0x0004,
> > .m_reg = 0x0008,
> > .n_reg = 0x000c,
> > .config_reg = 0x0014,
> > .mode_reg = 0x0000,
> > .status_reg = 0x001c,
> > .status_bit = 17,
> > .clkr.hw.init = &(struct clk_init_data){
> > .name = "gpll0",
> > .parent_names = (const char *[]){ "xo" },
> > .num_parents = 1,
> > .ops = &clk_pll_ops,
> > },
> > };
>
> I did not know you could do that - that could make life easier...
>
> But a quick look shows that this approach probably would require a
> major rewrite of all the methods.
>
> > static int bcm2835_clk_probe(struct platform_device *pdev)
> > {
> > ...
> > for (i = 0; i < num_clks; i++) {
> > clk = devm_clk_register(dev, array_of_clks[i].hw)
> > ...
> > }
>
> I guess I can use a slightly different approach that does not
> require as many changes, that looks like this:
>
> static const struct bcm2835_clk_desc clk_desc_array[] = {
> /* register PLL */
> [BCM2835_PLLA] = REGISTER_PLL(&bcm2835_plla_data),
> ...
> };
> ...
> static int bcm2835_clk_probe(struct platform_device *pdev)
> {
> const size_t asize = ARRAY_SIZE(clk_desc_array);
> ...
> for (i = 0; i < asize; i++) {
> desc = &clk_desc_array[i];
> if (desc)
> clks[i] = desc->clk_register(cprman,
> desc->data);
Interesting. I have been playing with the idea of putting a .register()
callback into struct clk_init_data. Then it would be called by a new
clk_register_desc() top-level registration function.
You've done that here, but you've put the registration function in your
machine-specific struct. Nothing wrong with that.
Do you actually need a machine-specific registration function? Many clk
drivers only use their machine-specific registration functions to
allocate a stuct clk_hw and populate the contents of struct
clk_init_data, which can be done by the framework and reduce duplicate
code in drivers.
(they do this because the basic clk types like clk-divider, clk-gate,
etc do it, and everyone loves to copy/paste that code)
bcm2835 seems to register two clks in the PLL registration function (one
of which is a fixed factor divider), but you could just add those
post-PLL dividers to your array of clk data?
> }
> ...
> }
>
> If we need to order the initialization then we can add some
> priority field to clk_desc_array and iterate over all the priority
> values.
I guess that you can order them in your array? Just start with the root
clocks at the beginning of the array (regardless of "clk type") and walk
through the array registering them.
Regards,
Mike
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V2 1/4] clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
2016-01-14 20:23 ` Michael Turquette
(?)
@ 2016-01-14 21:24 ` Martin Sperl
-1 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-01-14 21:24 UTC (permalink / raw)
To: Michael Turquette
Cc: Stephen Boyd, Stephen Warren, Lee Jones, Eric Anholt,
Remi Pommarel, linux-clk, linux-rpi-kernel, linux-arm-kernel,
devicetree
>
> On 14.01.2016, at 21:23, Michael Turquette <mturquette@baylibre.com> wrote:
>> static int bcm2835_clk_probe(struct platform_device *pdev)
>> {
>> const size_t asize = ARRAY_SIZE(clk_desc_array);
>> ...
>> for (i = 0; i < asize; i++) {
>> desc = &clk_desc_array[i];
>> if (desc)
>> clks[i] = desc->clk_register(cprman,
>> desc->data);
>
> Interesting. I have been playing with the idea of putting a .register()
> callback into struct clk_init_data. Then it would be called by a new
> clk_register_desc() top-level registration function.
>
> You've done that here, but you've put the registration function in your
> machine-specific struct. Nothing wrong with that.
>
> Do you actually need a machine-specific registration function? Many clk
> drivers only use their machine-specific registration functions to
> allocate a stuct clk_hw and populate the contents of struct
> clk_init_data, which can be done by the framework and reduce duplicate
> code in drivers.
>
> (they do this because the basic clk types like clk-divider, clk-gate,
> etc do it, and everyone loves to copy/paste that code)
>
> bcm2835 seems to register two clks in the PLL registration function (one
> of which is a fixed factor divider), but you could just add those
> post-PLL dividers to your array of clk data?
>
My goal was to limit the changes of the patch as much as possible.
Also that way I know it will continue to work as there real
registration code has not changed - it was just wrapped (and I
have to admit: I did not want to go into the details of understanding
the existing registration code.
If there is ever a change in the framework, then we may make use of it.
>>
>> If we need to order the initialization then we can add some
>> priority field to clk_desc_array and iterate over all the priority
>> values.
>
> I guess that you can order them in your array? Just start with the root
> clocks at the beginning of the array (regardless of "clk type") and walk
> through the array registering them.
That was what my earlier patch was doing, but right now we have a sparse
array wo we do not have to check for duplicates.
Anyway: my interpretation is that the clocks only become really
available to the device-tree with of_clk_add_provider
so that means there should not be any race… (but this may not be
the absolute truth…
So I hope this is acceptable (besides the bug I have mentioned
earlier for which I will send a version 4)
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V2 1/4] clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
@ 2016-01-14 21:24 ` Martin Sperl
0 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-01-14 21:24 UTC (permalink / raw)
To: Michael Turquette
Cc: Stephen Boyd, Stephen Warren, Lee Jones, Eric Anholt,
Remi Pommarel, linux-clk, linux-rpi-kernel, linux-arm-kernel,
devicetree
>=20
> On 14.01.2016, at 21:23, Michael Turquette <mturquette@baylibre.com> =
wrote:
>> static int bcm2835_clk_probe(struct platform_device *pdev)
>> {
>> const size_t asize =3D ARRAY_SIZE(clk_desc_array);
>> ...
>> for (i =3D 0; i < asize; i++) {
>> desc =3D &clk_desc_array[i];
>> if (desc)
>> clks[i] =3D desc->clk_register(cprman,
>> desc->data);
>=20
> Interesting. I have been playing with the idea of putting a =
.register()
> callback into struct clk_init_data. Then it would be called by a new
> clk_register_desc() top-level registration function.
>=20
> You've done that here, but you've put the registration function in =
your
> machine-specific struct. Nothing wrong with that.
>=20
> Do you actually need a machine-specific registration function? Many =
clk
> drivers only use their machine-specific registration functions to
> allocate a stuct clk_hw and populate the contents of struct
> clk_init_data, which can be done by the framework and reduce duplicate
> code in drivers.
>=20
> (they do this because the basic clk types like clk-divider, clk-gate,
> etc do it, and everyone loves to copy/paste that code)
>=20
> bcm2835 seems to register two clks in the PLL registration function =
(one
> of which is a fixed factor divider), but you could just add those
> post-PLL dividers to your array of clk data?
>=20
My goal was to limit the changes of the patch as much as possible.
Also that way I know it will continue to work as there real
registration code has not changed - it was just wrapped (and I
have to admit: I did not want to go into the details of understanding
the existing registration code.
If there is ever a change in the framework, then we may make use of it.
>>=20
>> If we need to order the initialization then we can add some
>> priority field to clk_desc_array and iterate over all the priority
>> values.
>=20
> I guess that you can order them in your array? Just start with the =
root
> clocks at the beginning of the array (regardless of "clk type") and =
walk
> through the array registering them.
That was what my earlier patch was doing, but right now we have a sparse
array wo we do not have to check for duplicates.
Anyway: my interpretation is that the clocks only become really
available to the device-tree with of_clk_add_provider
so that means there should not be any race=E2=80=A6 (but this may not be
the absolute truth=E2=80=A6
So I hope this is acceptable (besides the bug I have mentioned
earlier for which I will send a version 4)
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V2 1/4] clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835
@ 2016-01-14 21:24 ` Martin Sperl
0 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-01-14 21:24 UTC (permalink / raw)
To: linux-arm-kernel
>
> On 14.01.2016, at 21:23, Michael Turquette <mturquette@baylibre.com> wrote:
>> static int bcm2835_clk_probe(struct platform_device *pdev)
>> {
>> const size_t asize = ARRAY_SIZE(clk_desc_array);
>> ...
>> for (i = 0; i < asize; i++) {
>> desc = &clk_desc_array[i];
>> if (desc)
>> clks[i] = desc->clk_register(cprman,
>> desc->data);
>
> Interesting. I have been playing with the idea of putting a .register()
> callback into struct clk_init_data. Then it would be called by a new
> clk_register_desc() top-level registration function.
>
> You've done that here, but you've put the registration function in your
> machine-specific struct. Nothing wrong with that.
>
> Do you actually need a machine-specific registration function? Many clk
> drivers only use their machine-specific registration functions to
> allocate a stuct clk_hw and populate the contents of struct
> clk_init_data, which can be done by the framework and reduce duplicate
> code in drivers.
>
> (they do this because the basic clk types like clk-divider, clk-gate,
> etc do it, and everyone loves to copy/paste that code)
>
> bcm2835 seems to register two clks in the PLL registration function (one
> of which is a fixed factor divider), but you could just add those
> post-PLL dividers to your array of clk data?
>
My goal was to limit the changes of the patch as much as possible.
Also that way I know it will continue to work as there real
registration code has not changed - it was just wrapped (and I
have to admit: I did not want to go into the details of understanding
the existing registration code.
If there is ever a change in the framework, then we may make use of it.
>>
>> If we need to order the initialization then we can add some
>> priority field to clk_desc_array and iterate over all the priority
>> values.
>
> I guess that you can order them in your array? Just start with the root
> clocks at the beginning of the array (regardless of "clk type") and walk
> through the array registering them.
That was what my earlier patch was doing, but right now we have a sparse
array wo we do not have to check for duplicates.
Anyway: my interpretation is that the clocks only become really
available to the device-tree with of_clk_add_provider
so that means there should not be any race? (but this may not be
the absolute truth?
So I hope this is acceptable (besides the bug I have mentioned
earlier for which I will send a version 4)
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2016-01-14 21:24 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-11 19:55 [PATCH V2 0/4] clk: bcm2835: add additinal clocks and add frac support kernel
2016-01-11 19:55 ` kernel at martin.sperl.org
2016-01-11 19:55 ` [PATCH V2 1/4] clk: bcm2835: avoid the use of BCM2835_CLOCK_COUNT in clk-bcm2835 kernel
2016-01-11 19:55 ` kernel at martin.sperl.org
2016-01-13 20:00 ` Michael Turquette
2016-01-13 20:00 ` Michael Turquette
2016-01-13 20:00 ` Michael Turquette
2016-01-14 0:13 ` Michael Turquette
2016-01-14 0:13 ` Michael Turquette
2016-01-14 0:13 ` Michael Turquette
2016-01-14 12:11 ` Martin Sperl
2016-01-14 12:11 ` Martin Sperl
2016-01-14 20:23 ` Michael Turquette
2016-01-14 20:23 ` Michael Turquette
2016-01-14 20:23 ` Michael Turquette
2016-01-14 21:24 ` Martin Sperl
2016-01-14 21:24 ` Martin Sperl
2016-01-14 21:24 ` Martin Sperl
2016-01-11 19:55 ` [PATCH V2 2/4] clk: bcm2835: enable fractional and mash support kernel
2016-01-11 19:55 ` kernel at martin.sperl.org
2016-01-13 20:07 ` Michael Turquette
2016-01-13 20:07 ` Michael Turquette
2016-01-13 20:07 ` Michael Turquette
2016-01-11 19:55 ` [PATCH V2 3/4] clk: bcm2835: enable management of PCM clock kernel
2016-01-11 19:55 ` kernel at martin.sperl.org
[not found] ` <1452542157-2387-4-git-send-email-kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
2016-01-13 20:11 ` Michael Turquette
2016-01-13 20:11 ` Michael Turquette
2016-01-13 20:11 ` Michael Turquette
2016-01-11 19:55 ` [PATCH V2 4/4] clk: bcm2835: add missing 22 HW-clocks kernel
2016-01-11 19:55 ` kernel at martin.sperl.org
[not found] ` <1452542157-2387-1-git-send-email-kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
2016-01-11 21:01 ` [PATCH V2 0/4] clk: bcm2835: add additinal clocks and add frac support Arnd Bergmann
2016-01-11 21:01 ` Arnd Bergmann
2016-01-11 21:01 ` Arnd Bergmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.