[PATCH V4 0/7] clk: bcm2835: add clocks and add MASH support

[PATCH V4 0/7] clk: bcm2835: add clocks and add MASH support

[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.

There is also an issue when the clock divider is < 2 - in that
case no clock-output is generate. The clamping code has been
enhanced to handle this as well.

Similarly there is also the clamping of the highest divider now
limited to the highest possible integer divider instead of the
highest possible fractional diviver.

Finally we add all the 23 different HW clocks that have not been
configured in the driver.

Changelog:
	V1 -> V2: split the asoc/sound patches from the clock patches
	      	  enable frac/mash support
	V2 -> V3: clamp clock divider to be >= 2
		  clamp max clock divider to be integer (not fractional)
		  added additional limit checks for divider selection
		    allowing fallback to lower mash levels.
		  use a newer probing mechanism based on a single array
	V3 -> V4: fixed bad NULL pointer check in init
	      	  fixed (mash) limit checks
		  separated basic mash support from limits checks
		  really tested with I2S device (unfortunately V3 was
		  not well tested in this respect)

Martin Sperl (7):
  clk: bcm2835: the minimum clock divider is 2
  clk: bcm2835: clamp clock divider to highest integer only
  clk: bcm2835: remove use of BCM2835_CLOCK_COUNT in driver
  clk: bcm2835: enable management of PCM clock
  clk: bcm2835: add missing 22 HW-clocks.
  clk: bcm2835: enable fractional and mash support
  clk: bcm2835: apply limits on dividers to MASH mode.

 drivers/clk/bcm/clk-bcm2835.c       |  583 ++++++++++++++++++++++++++++++-----
 include/dt-bindings/clock/bcm2835.h |   25 +-
 2 files changed, 521 insertions(+), 87 deletions(-)

--
1.7.10.4

[PATCH V4 1/7] clk: bcm2835: the minimum clock divider is 2

[kernel]

From: Martin Sperl <kernel@martin.sperl.org>

Testing with different clock divider values has shown
that (at least for the PCM clock) the clock divider
has to be at least 2, otherwise the clock will not
output a signal.

So the clamping has changed from 1 to 2 and comments
about the kind of clamping applied have been added.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 drivers/clk/bcm/clk-bcm2835.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 015e687..10e97b7 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -1178,7 +1178,11 @@ static u32 bcm2835_clock_choose_div(struct clk_hw *hw,
 	div &= ~unused_frac_mask;
 
 	/* Clamp to the limits. */
-	div = max(div, unused_frac_mask + 1);
+
+	/* divider must be >= 2 */
+	div = max_t(u32, div, (2 << CM_DIV_FRAC_BITS));
+
+	/* clamp to max divider allowed */
 	div = min_t(u32, div, GENMASK(data->int_bits + CM_DIV_FRAC_BITS - 1,
 				      CM_DIV_FRAC_BITS - data->frac_bits));
 
-- 
1.7.10.4

Re: [PATCH V4 1/7] clk: bcm2835: the minimum clock divider is 2

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 413 bytes --]

kernel@martin.sperl.org writes:

> From: Martin Sperl <kernel@martin.sperl.org>
>
> Testing with different clock divider values has shown
> that (at least for the PCM clock) the clock divider
> has to be at least 2, otherwise the clock will not
> output a signal.

For a MASH clock (PWM, PCM, SLIMBUS, but not the others), the minimum
integer component of the divider is:

mash 0: 1
mash 1: 2
mash 2: 3
mash 3: 5

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH V4 1/7] clk: bcm2835: the minimum clock divider is 2

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 535 bytes --]

Eric Anholt <eric@anholt.net> writes:

> kernel@martin.sperl.org writes:
>
>> From: Martin Sperl <kernel@martin.sperl.org>
>>
>> Testing with different clock divider values has shown
>> that (at least for the PCM clock) the clock divider
>> has to be at least 2, otherwise the clock will not
>> output a signal.
>
> For a MASH clock (PWM, PCM, SLIMBUS, but not the others), the minimum
> integer component of the divider is:
>
> mash 0: 1
> mash 1: 2
> mash 2: 3
> mash 3: 5

More specific MASH list:

GP0
GP1
(*not* gp2)
PCM
PWM
SLIM

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH V4 1/7] clk: bcm2835: the minimum clock divider is 2

[Martin Sperl]

On 02.02.2016 02:52, Eric Anholt wrote:
> Eric Anholt <eric@anholt.net> writes:
>
>> kernel@martin.sperl.org writes:
>>
>>> From: Martin Sperl <kernel@martin.sperl.org>
>>>
>>> Testing with different clock divider values has shown
>>> that (at least for the PCM clock) the clock divider
>>> has to be at least 2, otherwise the clock will not
>>> output a signal.
>>
>> For a MASH clock (PWM, PCM, SLIMBUS, but not the others), the minimum
>> integer component of the divider is:
>>
>> mash 0: 1
>> mash 1: 2
>> mash 2: 3
>> mash 3: 5
>
> More specific MASH list:
>
> GP0
> GP1
> (*not* gp2)
> PCM
> PWM
> SLIM
>
I got the list from the broadcom provided headers for the VC4.
and where CM_*_DIV range does not start at 12 we have a fractional
divider (also requires if CM_*_FRAC or CM_*_MASH is set)

And if CM_*_MASH is set we got a mash enabled clock:
CM_GNRICCTL_MASH
CM_GP0CTL_MASH
CM_GP1CTL_MASH
CM_PCMCTL_MASH
CM_PWMCTL_MASH
CM_SLIMCTL_MASH

And this is what can get implemented by configuring mash
in the DT - otherwise only frac (a.k.a. MASH = 1) is
used for any clock that has .frac_bits > 0.

Re: [PATCH V4 1/7] clk: bcm2835: the minimum clock divider is 2

[Martin Sperl]

On 02.02.2016 00:15, Eric Anholt wrote:
> kernel@martin.sperl.org writes:
>
>> From: Martin Sperl <kernel@martin.sperl.org>
>>
>> Testing with different clock divider values has shown
>> that (at least for the PCM clock) the clock divider
>> has to be at least 2, otherwise the clock will not
>> output a signal.
>
> For a MASH clock (PWM, PCM, SLIMBUS, but not the others), the minimum
> integer component of the divider is:
>
> mash 0: 1
> mash 1: 2
> mash 2: 3
> mash 3: 5
>

I know that that is what the datasheet says - see also the errata:
http://elinux.org/BCM2835_datasheet_errata#p105_table.

Experimentation has show that a divider between 1 and 1 + 4095 / 4096
does not provide any output PCM clock - only 2 and above does work for
mash = 0, 1, 2 or 3.

Example: Requesting 12288000Hz (=192kHz at 64bit) with the 19.2Mhz
oscillator results in a divider of: 1 + 2304 / 4096 and this does not
give a clock output.

See the report by hiassoft here:
https://github.com/raspberrypi/linux/issues/1231#issuecomment-171003182
(note that it also applies to mash = 0, but there may be no comment on
this fact in this thread)

Note that there is patch 7 that implements the above "mash" limits
for a divider and downgrades to a lower mash level if the divider
does not qualify.

Martin

Re: [PATCH V4 1/7] clk: bcm2835: the minimum clock divider is 2

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 1960 bytes --]

Martin Sperl <kernel@martin.sperl.org> writes:

> On 02.02.2016 00:15, Eric Anholt wrote:
>> kernel@martin.sperl.org writes:
>>
>>> From: Martin Sperl <kernel@martin.sperl.org>
>>>
>>> Testing with different clock divider values has shown
>>> that (at least for the PCM clock) the clock divider
>>> has to be at least 2, otherwise the clock will not
>>> output a signal.
>>
>> For a MASH clock (PWM, PCM, SLIMBUS, but not the others), the minimum
>> integer component of the divider is:
>>
>> mash 0: 1
>> mash 1: 2
>> mash 2: 3
>> mash 3: 5
>>
>
> I know that that is what the datasheet says - see also the errata:
> http://elinux.org/BCM2835_datasheet_errata#p105_table.
>
> Experimentation has show that a divider between 1 and 1 + 4095 / 4096
> does not provide any output PCM clock - only 2 and above does work for
> mash = 0, 1, 2 or 3.
>
> Example: Requesting 12288000Hz (=192kHz at 64bit) with the 19.2Mhz
> oscillator results in a divider of: 1 + 2304 / 4096 and this does not
> give a clock output.
>
> See the report by hiassoft here:
> https://github.com/raspberrypi/linux/issues/1231#issuecomment-171003182
> (note that it also applies to mash = 0, but there may be no comment on
> this fact in this thread)
>
> Note that there is patch 7 that implements the above "mash" limits
> for a divider and downgrades to a lower mash level if the divider
> does not qualify.

I do see "2, 2, 3, 5" being used as the minimum dividers used for mash
clocks in the firmware's clock setup.  So, as long as you limit this to
the MASH clocks, I think you're right.

What I'd expect of this series as a respin:

1) Fix setup of existing MASH clocks
- Mark the few existing MASH clocks as being MASH.
- Apply MASH integer divider limits to MASH clocks.
- Set the MASH field on MASH clocks.

2) Add PCM clock

If you want to add some other clocks that are definitely the same kinds
of clock generators and where we have known parents for them, that's
fine with me.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

[PATCH V4 2/7] clk: bcm2835: clamp clock divider to highest integer only

[kernel]

From: Martin Sperl <kernel@martin.sperl.org>

The clock divider calculation right now clamps the
divider to the highest possible fractional divider.
So typically (BIT(div_int_bits) - 1) + 4095 / 4096.

As the fractional clock divider is alterating between
(Fosc / div_int) and (Fosc / (div_int + 1))
the divider will overflow for the (div_int + 1) case.
As with the "underflow" case we have seen for (div < 2),
we can assume that the same applies on the upper limit
as well.

So this patch will instead clamp to the divider to the
max of (BIT(div_int_bits) - 1)

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 drivers/clk/bcm/clk-bcm2835.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 10e97b7..3d6490f 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -1182,9 +1182,9 @@ static u32 bcm2835_clock_choose_div(struct clk_hw *hw,
 	/* divider must be >= 2 */
 	div = max_t(u32, div, (2 << CM_DIV_FRAC_BITS));
 
-	/* clamp to max divider allowed */
+	/* clamp to max divider allowed - max is integer divider */
 	div = min_t(u32, div, GENMASK(data->int_bits + CM_DIV_FRAC_BITS - 1,
-				      CM_DIV_FRAC_BITS - data->frac_bits));
+				      CM_DIV_FRAC_BITS));
 
 	return div;
 }
-- 
1.7.10.4

Re: [PATCH V4 2/7] clk: bcm2835: clamp clock divider to highest integer only

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 877 bytes --]

kernel@martin.sperl.org writes:

> From: Martin Sperl <kernel@martin.sperl.org>
>
> The clock divider calculation right now clamps the
> divider to the highest possible fractional divider.
> So typically (BIT(div_int_bits) - 1) + 4095 / 4096.
>
> As the fractional clock divider is alterating between
> (Fosc / div_int) and (Fosc / (div_int + 1))
> the divider will overflow for the (div_int + 1) case.
> As with the "underflow" case we have seen for (div < 2),
> we can assume that the same applies on the upper limit
> as well.
>
> So this patch will instead clamp to the divider to the
> max of (BIT(div_int_bits) - 1)

Looks like this restriction might be necessary, though it's not quite
spelled out in the docs.

This also needs to be conditional on it being a MASH clock, and the
limit on the divider changes in the same way based on the mash number as
for the minimum.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH V4 2/7] clk: bcm2835: clamp clock divider to highest integer only

[Martin Sperl]

On 02.02.2016 00:19, Eric Anholt wrote:
> kernel@martin.sperl.org writes:
>
>> From: Martin Sperl <kernel@martin.sperl.org>
>>
>> The clock divider calculation right now clamps the
>> divider to the highest possible fractional divider.
>> So typically (BIT(div_int_bits) - 1) + 4095 / 4096.
>>
>> As the fractional clock divider is alterating between
>> (Fosc / div_int) and (Fosc / (div_int + 1))
>> the divider will overflow for the (div_int + 1) case.
>> As with the "underflow" case we have seen for (div < 2),
>> we can assume that the same applies on the upper limit
>> as well.
>>
>> So this patch will instead clamp to the divider to the
>> max of (BIT(div_int_bits) - 1)
>
> Looks like this restriction might be necessary, though it's not quite
> spelled out in the docs.
>
> This also needs to be conditional on it being a MASH clock, and the
> limit on the divider changes in the same way based on the mash number as
> for the minimum.
>
See:
[PATCH V4 7/7] clk: bcm2835: apply limits on dividers to MASH mode.
That "downgrades" to lower mash values in case we are out of range.
Also note that it applies to MASH=1 as well.

If the other (non mash-supported) clocks support this "better" (if it 
works), then we can make that change only apply to mash enabled clocks.
But so far I doubt it works correctly so I have applied those limits to
all clocks.

[PATCH V4 3/7] clk: bcm2835: remove use of BCM2835_CLOCK_COUNT in driver

[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.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 drivers/clk/bcm/clk-bcm2835.c       |  167 ++++++++++++++++++++---------------
 include/dt-bindings/clock/bcm2835.h |    2 -
 2 files changed, 94 insertions(+), 75 deletions(-)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 3d6490f..567c265 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)
@@ -817,6 +817,25 @@ static const struct bcm2835_clock_data bcm2835_clock_pwm_data = {
 	.frac_bits = 12,
 };
 
+struct bcm2835_gate_data {
+	const char *name;
+	const char *parent;
+
+	u32 ctl_reg;
+};
+
+/*
+ * CM_PERIICTL (and CM_PERIACTL, CM_SYSCTL and CM_VPUCTL if
+ * you have the debug bit set in the power manager, which we
+ * don't bother exposing) are individual gates off of the
+ * non-stop vpu clock.
+ */
+static const struct bcm2835_gate_data bcm2835_clock_peri_image_data = {
+	.name = "peri_image",
+	.parent = "vpu",
+	.ctl_reg = CM_PERIICTL,
+};
+
 struct bcm2835_pll {
 	struct clk_hw hw;
 	struct bcm2835_cprman *cprman;
@@ -1502,14 +1521,81 @@ static struct clk *bcm2835_register_clock(struct bcm2835_cprman *cprman,
 	return devm_clk_register(cprman->dev, &clock->hw);
 }
 
+static struct clk *bcm2835_register_gate(struct bcm2835_cprman *cprman,
+					 const struct bcm2835_gate_data *data)
+{
+	return clk_register_gate(cprman->dev, data->name, data->parent,
+				 CLK_IGNORE_UNUSED | CLK_SET_RATE_GATE,
+				 cprman->regs + data->ctl_reg,
+				 CM_GATE_BIT, 0, &cprman->regs_lock);
+}
+
+typedef struct clk *(*bcm2835_clk_register)(struct bcm2835_cprman *cprman,
+					    const void *data);
+struct bcm2835_clk_desc {
+	bcm2835_clk_register clk_register;
+	const void *data;
+};
+
+#define _REGISTER(f, d) { .clk_register = (bcm2835_clk_register)f, \
+			  .data = d }
+#define REGISTER_PLL(d)		_REGISTER(&bcm2835_register_pll, d)
+#define REGISTER_PLL_DIV(d)	_REGISTER(&bcm2835_register_pll_divider, d)
+#define REGISTER_CLK(d)		_REGISTER(&bcm2835_register_clock, d)
+#define REGISTER_GATE(d)	_REGISTER(&bcm2835_register_gate, d)
+
+static const struct bcm2835_clk_desc clk_desc_array[] = {
+	/* register PLL */
+	[BCM2835_PLLA]		= REGISTER_PLL(&bcm2835_plla_data),
+	[BCM2835_PLLB]		= REGISTER_PLL(&bcm2835_pllb_data),
+	[BCM2835_PLLC]		= REGISTER_PLL(&bcm2835_pllc_data),
+	[BCM2835_PLLD]		= REGISTER_PLL(&bcm2835_plld_data),
+	[BCM2835_PLLH]		= REGISTER_PLL(&bcm2835_pllh_data),
+	/* the PLL dividers */
+	[BCM2835_PLLA_CORE]	= REGISTER_PLL_DIV(&bcm2835_plla_core_data),
+	[BCM2835_PLLA_PER]	= REGISTER_PLL_DIV(&bcm2835_plla_per_data),
+	[BCM2835_PLLC_CORE0]	= REGISTER_PLL_DIV(&bcm2835_pllc_core0_data),
+	[BCM2835_PLLC_CORE1]	= REGISTER_PLL_DIV(&bcm2835_pllc_core1_data),
+	[BCM2835_PLLC_CORE2]	= REGISTER_PLL_DIV(&bcm2835_pllc_core2_data),
+	[BCM2835_PLLC_PER]	= REGISTER_PLL_DIV(&bcm2835_pllc_per_data),
+	[BCM2835_PLLD_CORE]	= REGISTER_PLL_DIV(&bcm2835_plld_core_data),
+	[BCM2835_PLLD_PER]	= REGISTER_PLL_DIV(&bcm2835_plld_per_data),
+	[BCM2835_PLLH_RCAL]	= REGISTER_PLL_DIV(&bcm2835_pllh_rcal_data),
+	[BCM2835_PLLH_AUX]	= REGISTER_PLL_DIV(&bcm2835_pllh_aux_data),
+	[BCM2835_PLLH_PIX]	= REGISTER_PLL_DIV(&bcm2835_pllh_pix_data),
+	/* the clocks */
+	[BCM2835_CLOCK_TIMER]	= REGISTER_CLK(&bcm2835_clock_timer_data),
+	[BCM2835_CLOCK_OTP]	= REGISTER_CLK(&bcm2835_clock_otp_data),
+	[BCM2835_CLOCK_TSENS]	= REGISTER_CLK(&bcm2835_clock_tsens_data),
+	[BCM2835_CLOCK_VPU]	= REGISTER_CLK(&bcm2835_clock_vpu_data),
+	[BCM2835_CLOCK_V3D]	= REGISTER_CLK(&bcm2835_clock_v3d_data),
+	[BCM2835_CLOCK_ISP]	= REGISTER_CLK(&bcm2835_clock_isp_data),
+	[BCM2835_CLOCK_H264]	= REGISTER_CLK(&bcm2835_clock_h264_data),
+	[BCM2835_CLOCK_V3D]	= REGISTER_CLK(&bcm2835_clock_v3d_data),
+	[BCM2835_CLOCK_SDRAM]	= REGISTER_CLK(&bcm2835_clock_sdram_data),
+	[BCM2835_CLOCK_UART]	= REGISTER_CLK(&bcm2835_clock_uart_data),
+	[BCM2835_CLOCK_VEC]	= REGISTER_CLK(&bcm2835_clock_vec_data),
+	[BCM2835_CLOCK_HSM]	= REGISTER_CLK(&bcm2835_clock_hsm_data),
+	[BCM2835_CLOCK_EMMC]	= REGISTER_CLK(&bcm2835_clock_emmc_data),
+	[BCM2835_CLOCK_PWM]	= REGISTER_CLK(&bcm2835_clock_pwm_data),
+	/* the gates */
+	[BCM2835_CLOCK_PERI_IMAGE] = REGISTER_GATE(
+		&bcm2835_clock_peri_image_data),
+};
+
 static int bcm2835_clk_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct clk **clks;
 	struct bcm2835_cprman *cprman;
 	struct resource *res;
+	const struct bcm2835_clk_desc *desc;
+	const size_t asize = ARRAY_SIZE(clk_desc_array);
+	size_t i;
 
-	cprman = devm_kzalloc(dev, sizeof(*cprman), GFP_KERNEL);
+	cprman = devm_kzalloc(dev,
+			      sizeof(*cprman) + asize * sizeof(*clks),
+			      GFP_KERNEL);
 	if (!cprman)
 		return -ENOMEM;
 
@@ -1526,80 +1612,15 @@ 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 = asize;
 	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);
-
-	/*
-	 * CM_PERIICTL (and CM_PERIACTL, CM_SYSCTL and CM_VPUCTL if
-	 * you have the debug bit set in the power manager, which we
-	 * don't bother exposing) are individual gates off of the
-	 * non-stop vpu clock.
-	 */
-	clks[BCM2835_CLOCK_PERI_IMAGE] =
-		clk_register_gate(dev, "peri_image", "vpu",
-				  CLK_IGNORE_UNUSED | CLK_SET_RATE_GATE,
-				  cprman->regs + CM_PERIICTL, CM_GATE_BIT,
-				  0, &cprman->regs_lock);
-
-	clks[BCM2835_CLOCK_PWM] =
-		bcm2835_register_clock(cprman, &bcm2835_clock_pwm_data);
+	for (i = 0; i < asize; i++) {
+		desc = &clk_desc_array[i];
+		if (desc->clk_register && desc->data)
+			clks[i] = desc->clk_register(cprman, desc->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

[PATCH V4 4/7] clk: bcm2835: enable management of PCM clock

[kernel-TqfNSX0MhmxHKSADF0wUEw]

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>
---
 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 567c265..8bc0a76 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
@@ -817,6 +819,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_gate_data {
 	const char *name;
 	const char *parent;
@@ -1578,6 +1590,7 @@ static const struct bcm2835_clk_desc clk_desc_array[] = {
 	[BCM2835_CLOCK_HSM]	= REGISTER_CLK(&bcm2835_clock_hsm_data),
 	[BCM2835_CLOCK_EMMC]	= REGISTER_CLK(&bcm2835_clock_emmc_data),
 	[BCM2835_CLOCK_PWM]	= REGISTER_CLK(&bcm2835_clock_pwm_data),
+	[BCM2835_CLOCK_PCM]	= REGISTER_CLK(&bcm2835_clock_pcm_data),
 	/* the gates */
 	[BCM2835_CLOCK_PERI_IMAGE] = REGISTER_GATE(
 		&bcm2835_clock_peri_image_data),
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

[PATCH V4 5/7] clk: bcm2835: add missing 22 HW-clocks.

[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.

There are a 3 more clocks that possibly could get implemented
as gates instead of clocks:


There are comments to indicate those.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 drivers/clk/bcm/clk-bcm2835.c       |  260 ++++++++++++++++++++++++++++++++++-
 include/dt-bindings/clock/bcm2835.h |   22 +++
 2 files changed, 276 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 8bc0a76..1640288 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

@@ -829,6 +842,219 @@ 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 = {
+	/* this is possibly a gate */
+	.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 = {
+	/* this is possibly a gate */
+	.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 = {
+	/* this is possibly a gate */
+	.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_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_gate_data {
 	const char *name;
 	const char *parent;
@@ -836,18 +1062,18 @@ struct bcm2835_gate_data {
 	u32 ctl_reg;
 };

-/*
- * CM_PERIICTL (and CM_PERIACTL, CM_SYSCTL and CM_VPUCTL if
- * you have the debug bit set in the power manager, which we
- * don't bother exposing) are individual gates off of the
- * non-stop vpu clock.
- */
 static const struct bcm2835_gate_data bcm2835_clock_peri_image_data = {
 	.name = "peri_image",
 	.parent = "vpu",
 	.ctl_reg = CM_PERIICTL,
 };

+static const struct bcm2835_gate_data bcm2835_clock_sys_data = {
+	.name = "sys",
+	.parent = "vpu",
+	.ctl_reg = CM_SYSCTL,
+};
+
 struct bcm2835_pll {
 	struct clk_hw hw;
 	struct bcm2835_cprman *cprman;
@@ -1591,9 +1817,31 @@ static const struct bcm2835_clk_desc clk_desc_array[] = {
 	[BCM2835_CLOCK_EMMC]	= REGISTER_CLK(&bcm2835_clock_emmc_data),
 	[BCM2835_CLOCK_PWM]	= REGISTER_CLK(&bcm2835_clock_pwm_data),
 	[BCM2835_CLOCK_PCM]	= REGISTER_CLK(&bcm2835_clock_pcm_data),
+	[BCM2835_CLOCK_AVEO]	= REGISTER_CLK(&bcm2835_clock_aveo_data),
+	[BCM2835_CLOCK_CAM0]	= REGISTER_CLK(&bcm2835_clock_cam0_data),
+	[BCM2835_CLOCK_CAM1]	= REGISTER_CLK(&bcm2835_clock_cam1_data),
+	[BCM2835_CLOCK_CCP2]	= REGISTER_CLK(&bcm2835_clock_ccp2_data),
+	[BCM2835_CLOCK_DFT]	= REGISTER_CLK(&bcm2835_clock_dft_data),
+	[BCM2835_CLOCK_DPI]	= REGISTER_CLK(&bcm2835_clock_dpi_data),
+	[BCM2835_CLOCK_DSI0E]	= REGISTER_CLK(&bcm2835_clock_dsi0e_data),
+	[BCM2835_CLOCK_DSI0P]	= REGISTER_CLK(&bcm2835_clock_dsi0p_data),
+	[BCM2835_CLOCK_DSI1E]	= REGISTER_CLK(&bcm2835_clock_dsi1e_data),
+	[BCM2835_CLOCK_DSI1P]	= REGISTER_CLK(&bcm2835_clock_dsi1p_data),
+	[BCM2835_CLOCK_GNRIC]	= REGISTER_CLK(&bcm2835_clock_gnric_data),
+	[BCM2835_CLOCK_GP0]	= REGISTER_CLK(&bcm2835_clock_gp0_data),
+	[BCM2835_CLOCK_GP1]	= REGISTER_CLK(&bcm2835_clock_gp1_data),
+	[BCM2835_CLOCK_GP2]	= REGISTER_CLK(&bcm2835_clock_gp2_data),
+	[BCM2835_CLOCK_PERIA]	= REGISTER_CLK(&bcm2835_clock_peria_data),
+	[BCM2835_CLOCK_PULSE]	= REGISTER_CLK(&bcm2835_clock_pulse_data),
+	[BCM2835_CLOCK_SLIM]	= REGISTER_CLK(&bcm2835_clock_slim_data),
+	[BCM2835_CLOCK_SMI]	= REGISTER_CLK(&bcm2835_clock_smi_data),
+	[BCM2835_CLOCK_TD0]	= REGISTER_CLK(&bcm2835_clock_td0_data),
+	[BCM2835_CLOCK_TD1]	= REGISTER_CLK(&bcm2835_clock_td1_data),
+	[BCM2835_CLOCK_TEC]	= REGISTER_CLK(&bcm2835_clock_tec_data),
 	/* the gates */
 	[BCM2835_CLOCK_PERI_IMAGE] = REGISTER_GATE(
 		&bcm2835_clock_peri_image_data),
+	[BCM2835_CLOCK_SYS]	= REGISTER_GATE(&bcm2835_clock_sys_data),
 };

 static int bcm2835_clk_probe(struct platform_device *pdev)
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

Re: [PATCH V4 5/7] clk: bcm2835: add missing 22 HW-clocks.

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 9921 bytes --]

kernel@martin.sperl.org writes:

> 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.

We need the parents to be correct if we're going to land the patch.
I'll try to update them.

I'm not a fan of this "let's just shove everything we can find in some
header file into the .c and hope for the best."  Most of these clocks
were left out intentionally.

> There are a 3 more clocks that possibly could get implemented
> as gates instead of clocks:
>
>
> There are comments to indicate those.
>
> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> ---
>  drivers/clk/bcm/clk-bcm2835.c       |  260 ++++++++++++++++++++++++++++++++++-
>  include/dt-bindings/clock/bcm2835.h |   22 +++
>  2 files changed, 276 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index 8bc0a76..1640288 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
>
> @@ -829,6 +842,219 @@ 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,
> +};

AVEO has 0 fractional bits

> +static const struct bcm2835_clock_data bcm2835_clock_ccp2_data = {
> +	/* this is possibly a gate */
> +	.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,
> +};

CCP2 is a gate from a different clock source, so this won't work.

> +static const struct bcm2835_clock_data bcm2835_clock_dsi0p_data = {
> +	/* this is possibly a gate */
> +	.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_dsi1p_data = {
> +	/* this is possibly a gate */
> +	.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,
> +};

DSI0/1 pixel clocks take different clock sources and are gates off of
them, so these definitions don't work.

> +
> +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,
> +};

GNRIC isn't an actual clock, it's just what's used for describing the
overall structure of clocks.

> +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,
> +};

This register doesn't do anything, because the debug bit in the power
manager is not set.  We don't think we should expose a clock gate if it
doesn't work, I think.

> +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,
> +};

There's some other divider involved in this clock, won't give correct results.

> +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,
> +};

These are some other clock generator, not the generic or mash ones used
elsewhere.  I wouldn't enable them without testing.

> +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,
> +};

TEC should be osc parents.

> -/*
> - * CM_PERIICTL (and CM_PERIACTL, CM_SYSCTL and CM_VPUCTL if
> - * you have the debug bit set in the power manager, which we
> - * don't bother exposing) are individual gates off of the
> - * non-stop vpu clock.
> - */
>  static const struct bcm2835_gate_data bcm2835_clock_peri_image_data = {
>  	.name = "peri_image",
>  	.parent = "vpu",
>  	.ctl_reg = CM_PERIICTL,
>  };
>
> +static const struct bcm2835_gate_data bcm2835_clock_sys_data = {
> +	.name = "sys",
> +	.parent = "vpu",
> +	.ctl_reg = CM_SYSCTL,
> +};

same concern as peria.

> +
>  struct bcm2835_pll {
>  	struct clk_hw hw;
>  	struct bcm2835_cprman *cprman;
> @@ -1591,9 +1817,31 @@ static const struct bcm2835_clk_desc clk_desc_array[] = {
>  	[BCM2835_CLOCK_EMMC]	= REGISTER_CLK(&bcm2835_clock_emmc_data),
>  	[BCM2835_CLOCK_PWM]	= REGISTER_CLK(&bcm2835_clock_pwm_data),
>  	[BCM2835_CLOCK_PCM]	= REGISTER_CLK(&bcm2835_clock_pcm_data),
> +	[BCM2835_CLOCK_AVEO]	= REGISTER_CLK(&bcm2835_clock_aveo_data),
> +	[BCM2835_CLOCK_CAM0]	= REGISTER_CLK(&bcm2835_clock_cam0_data),
> +	[BCM2835_CLOCK_CAM1]	= REGISTER_CLK(&bcm2835_clock_cam1_data),
> +	[BCM2835_CLOCK_CCP2]	= REGISTER_CLK(&bcm2835_clock_ccp2_data),
> +	[BCM2835_CLOCK_DFT]	= REGISTER_CLK(&bcm2835_clock_dft_data),
> +	[BCM2835_CLOCK_DPI]	= REGISTER_CLK(&bcm2835_clock_dpi_data),
> +	[BCM2835_CLOCK_DSI0E]	= REGISTER_CLK(&bcm2835_clock_dsi0e_data),
> +	[BCM2835_CLOCK_DSI0P]	= REGISTER_CLK(&bcm2835_clock_dsi0p_data),
> +	[BCM2835_CLOCK_DSI1E]	= REGISTER_CLK(&bcm2835_clock_dsi1e_data),
> +	[BCM2835_CLOCK_DSI1P]	= REGISTER_CLK(&bcm2835_clock_dsi1p_data),
> +	[BCM2835_CLOCK_GNRIC]	= REGISTER_CLK(&bcm2835_clock_gnric_data),
> +	[BCM2835_CLOCK_GP0]	= REGISTER_CLK(&bcm2835_clock_gp0_data),
> +	[BCM2835_CLOCK_GP1]	= REGISTER_CLK(&bcm2835_clock_gp1_data),
> +	[BCM2835_CLOCK_GP2]	= REGISTER_CLK(&bcm2835_clock_gp2_data),
> +	[BCM2835_CLOCK_PERIA]	= REGISTER_CLK(&bcm2835_clock_peria_data),
> +	[BCM2835_CLOCK_PULSE]	= REGISTER_CLK(&bcm2835_clock_pulse_data),
> +	[BCM2835_CLOCK_SLIM]	= REGISTER_CLK(&bcm2835_clock_slim_data),
> +	[BCM2835_CLOCK_SMI]	= REGISTER_CLK(&bcm2835_clock_smi_data),
> +	[BCM2835_CLOCK_TD0]	= REGISTER_CLK(&bcm2835_clock_td0_data),
> +	[BCM2835_CLOCK_TD1]	= REGISTER_CLK(&bcm2835_clock_td1_data),
> +	[BCM2835_CLOCK_TEC]	= REGISTER_CLK(&bcm2835_clock_tec_data),
>  	/* the gates */
>  	[BCM2835_CLOCK_PERI_IMAGE] = REGISTER_GATE(
>  		&bcm2835_clock_peri_image_data),
> +	[BCM2835_CLOCK_SYS]	= REGISTER_GATE(&bcm2835_clock_sys_data),
>  };
>
>  static int bcm2835_clk_probe(struct platform_device *pdev)
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH V4 5/7] clk: bcm2835: add missing 22 HW-clocks.

[Martin Sperl]

On 02.02.2016 02:51, Eric Anholt wrote:
> kernel@martin.sperl.org writes:
>
>> 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.
>
> We need the parents to be correct if we're going to land the patch.
> I'll try to update them.
>
> I'm not a fan of this "let's just shove everything we can find in some
> header file into the .c and hope for the best."  Most of these clocks
> were left out intentionally.

Well - I wanted to get them in just in case we need them later.

If you have got access to documentation which states the correct
parent mux, then please share them so that we can implement them
correctly.

Also listing all allows us then to expose the values of the registers
via debugfs in case we need it - see separate RFC-patch 9 -  where
we expose the raw register values as well.

>> +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,
>> +};
>
> AVEO has 0 fractional bits

Correct - my issue (copy paste - I assume).

>
>> +static const struct bcm2835_clock_data bcm2835_clock_ccp2_data = {
>> +	/* this is possibly a gate */
>> +	.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,
>> +};
>
> CCP2 is a gate from a different clock source, so this won't work.
See comment above: please provide parent clock mux.
See also my comment about 3 clock that may be gates - this applies.

>
>> +static const struct bcm2835_clock_data bcm2835_clock_dsi0p_data = {
>> +	/* this is possibly a gate */
>> +	.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_dsi1p_data = {
>> +	/* this is possibly a gate */
>> +	.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,
>> +};
>
> DSI0/1 pixel clocks take different clock sources and are gates off of
> them, so these definitions don't work.
See comment above: please provide parent clock.
See also my comment about 3 clock that may be gates.

>
>> +
>> +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,
>> +};
>
> GNRIC isn't an actual clock, it's just what's used for describing the
> overall structure of clocks.

Well - there is the corresponding register at 0x7e101000 which reads as:
ctl = 0x0000636d
div = 0x0000636d

So we could remove this one if this is really the case of a dummy -
even though I wonder why there would be space in io-space reserved for
this "description" only.

>
>> +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,
>> +};
>
> This register doesn't do anything, because the debug bit in the power
> manager is not set.  We don't think we should expose a clock gate if it
> doesn't work, I think.
maybe we allow setting debug in the PM at a later point in time?

>
>> +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,
>> +};
>
> There's some other divider involved in this clock, won't give correct results.
See comment above: please provide parent clock.

>
>> +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,
>> +};
>
> These are some other clock generator, not the generic or mash ones used
> elsewhere.  I wouldn't enable them without testing.

As long as they are not referenced in the DT these only are read only
and can get read via debugfs - these are disabled anyway:

root@raspcm:/build/linux# head /sys/kernel/debug/clk/td*/clk_rate
==> /sys/kernel/debug/clk/td0/clk_rate <==
0

==> /sys/kernel/debug/clk/td1/clk_rate <==
0

>
>> +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,
>> +};
>
> TEC should be osc parents.
I will change that.

>
>> -/*
>> - * CM_PERIICTL (and CM_PERIACTL, CM_SYSCTL and CM_VPUCTL if
>> - * you have the debug bit set in the power manager, which we
>> - * don't bother exposing) are individual gates off of the
>> - * non-stop vpu clock.
>> - */
>>   static const struct bcm2835_gate_data bcm2835_clock_peri_image_data = {
>>   	.name = "peri_image",
>>   	.parent = "vpu",
>>   	.ctl_reg = CM_PERIICTL,
>>   };
>>
>> +static const struct bcm2835_gate_data bcm2835_clock_sys_data = {
>> +	.name = "sys",
>> +	.parent = "vpu",
>> +	.ctl_reg = CM_SYSCTL,
>> +};
>
> same concern as peria.
maybe we allow setting debug in the PM at a later point in time?

Please propose how to continue to get the clocks in.

If you want some clocks left out, then that is fine and we
can accommodate that and just leave the defines in the bindings
header for later use...

Just list them.

Martin

Re: [PATCH V4 5/7] clk: bcm2835: add missing 22 HW-clocks.

[Martin Sperl]

> On 08.02.2016, at 12:12, Martin Sperl <kernel@martin.sperl.org> wrote:
> 
> 
> 
> On 02.02.2016 02:51, Eric Anholt wrote:
>> kernel@martin.sperl.org writes:
>> 
>>> 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.
>> 
>> We need the parents to be correct if we're going to land the patch.
>> I'll try to update them.
>> 
>> I'm not a fan of this "let's just shove everything we can find in some
>> header file into the .c and hope for the best."  Most of these clocks
>> were left out intentionally.
> 
> Well - I wanted to get them in just in case we need them later.
> 
> If you have got access to documentation which states the correct
> parent mux, then please share them so that we can implement them
> correctly.
> 
> Also listing all allows us then to expose the values of the registers
> via debugfs in case we need it - see separate RFC-patch 9 -  where
> we expose the raw register values as well.
> 
>>> +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,
>>> +};
>> 
>> AVEO has 0 fractional bits
> 
> Correct - my issue (copy paste - I assume).
> 
>> 
>>> +static const struct bcm2835_clock_data bcm2835_clock_ccp2_data = {
>>> +	/* this is possibly a gate */
>>> +	.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,
>>> +};
>> 
>> CCP2 is a gate from a different clock source, so this won't work.
> See comment above: please provide parent clock mux.
> See also my comment about 3 clock that may be gates - this applies.
> 
>> 
>>> +static const struct bcm2835_clock_data bcm2835_clock_dsi0p_data = {
>>> +	/* this is possibly a gate */
>>> +	.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_dsi1p_data = {
>>> +	/* this is possibly a gate */
>>> +	.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,
>>> +};
>> 
>> DSI0/1 pixel clocks take different clock sources and are gates off of
>> them, so these definitions don't work.
> See comment above: please provide parent clock.
> See also my comment about 3 clock that may be gates.
> 
>> 
>>> +
>>> +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,
>>> +};
>> 
>> GNRIC isn't an actual clock, it's just what's used for describing the
>> overall structure of clocks.
> 
> Well - there is the corresponding register at 0x7e101000 which reads as:
> ctl = 0x0000636d
> div = 0x0000636d
> 
> So we could remove this one if this is really the case of a dummy -
> even though I wonder why there would be space in io-space reserved for
> this "description" only.
> 
>> 
>>> +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,
>>> +};
>> 
>> This register doesn't do anything, because the debug bit in the power
>> manager is not set.  We don't think we should expose a clock gate if it
>> doesn't work, I think.
> maybe we allow setting debug in the PM at a later point in time?
> 
>> 
>>> +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,
>>> +};
>> 
>> There's some other divider involved in this clock, won't give correct results.
> See comment above: please provide parent clock.
> 
>> 
>>> +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,
>>> +};
>> 
>> These are some other clock generator, not the generic or mash ones used
>> elsewhere.  I wouldn't enable them without testing.
> 
> As long as they are not referenced in the DT these only are read only
> and can get read via debugfs - these are disabled anyway:
> 
> root@raspcm:/build/linux# head /sys/kernel/debug/clk/td*/clk_rate
> ==> /sys/kernel/debug/clk/td0/clk_rate <==
> 0
> 
> ==> /sys/kernel/debug/clk/td1/clk_rate <==
> 0
> 
>> 
>>> +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,
>>> +};
>> 
>> TEC should be osc parents.
> I will change that.
> 
>> 
>>> -/*
>>> - * CM_PERIICTL (and CM_PERIACTL, CM_SYSCTL and CM_VPUCTL if
>>> - * you have the debug bit set in the power manager, which we
>>> - * don't bother exposing) are individual gates off of the
>>> - * non-stop vpu clock.
>>> - */
>>>  static const struct bcm2835_gate_data bcm2835_clock_peri_image_data = {
>>>  	.name = "peri_image",
>>>  	.parent = "vpu",
>>>  	.ctl_reg = CM_PERIICTL,
>>>  };
>>> 
>>> +static const struct bcm2835_gate_data bcm2835_clock_sys_data = {
>>> +	.name = "sys",
>>> +	.parent = "vpu",
>>> +	.ctl_reg = CM_SYSCTL,
>>> +};
>> 
>> same concern as peria.
> maybe we allow setting debug in the PM at a later point in time?
> 
> Please propose how to continue to get the clocks in.
> 
> If you want some clocks left out, then that is fine and we
> can accommodate that and just leave the defines in the bindings
> header for later use...
> 
> Just list them.
> 


As I am trying to complete the list of clocks here my “best” summary
so far of the clock tree for the bcm2835 taken from various sources:
* Datasheet
* broadcom provided VC4 headers (via https://github.com/msperl/rpi-registers)
* mailing list

And I have come up with the following description that
you can put in graphviz or http://www.webgraphviz.com/

--- cut here ---
digraph clocks {
	graph [ordering="out"];

	subgraph cluster_osc {
		label = "Oscillators";

                "GND";
                "OSC";
                "testdebug0";
                "testdebug1";
	}

	subgraph cluster_pll {
		label = "PLLs";

		subgraph cluster_plla {
			label = "PLLA";

			OSC -> PLLA;

			PLLA -> PLLA_core;
			PLLA -> PLLA_per;
			PLLA -> PLLA_dsi0;
			PLLA -> PLLA_ccp2;
		}

		subgraph cluster_pllb {
			label = "PLLB";

			OSC -> PLLB;

			PLLB -> PLLB_ARM;
			PLLB -> PLLB_SP0;
			PLLB -> PLLB_SP1;
			PLLB -> PLLB_SP2;
		}

		subgraph cluster_pllc {
			label = "PLLC";

			OSC -> PLLC;

			PLLC -> PLLC_core0;
			PLLC -> PLLC_core1;
			PLLC -> PLLC_core2;
			PLLC -> PLLC_per;
		}

		subgraph cluster_plld {
			label = "PLLD";

			OSC -> PLLD;

			PLLD -> PLLD_core;
			PLLD -> PLLD_per;
			PLLD -> PLLD_dsi0;
			PLLD -> PLLD_dsi1;
		}

		subgraph cluster_pllh {
			label = "PLLH";

			OSC -> PLLH;

			PLLH -> PLLH_aux;
			PLLH -> PLLH_pix;
			PLLH -> PLLH_rcal;
		}
	}

	subgraph cluster_mux {
		label = "clocks";

		subgraph cluster_vpu_clocks {
			label = "VPU-clocks";

			subgraph cluster_vpu_mux {
				label = "VPU-mux";

				vGND        [label="0: GND"];
				vOSC        [label="1: OSC"];
				vtestdebug0 [label="2: testdebug0"];
				vtestdebug1 [label="3: testdebug1"];
				vPLLA_core  [label="4: PLLA_core"];
				vPLLC_core0 [label="5: PLLC_core0"];
				vPLLD_core  [label="6: PLLD_core"];
				vPLLH_aux   [label="7: PLLH_aux"];
				vPLLC_core1 [label="8: PLLC_core1"];
				vPLLC_core2 [label="9: PLLC_core2"];

				GND        -> vGND        -> vpu_mux;
				OSC        -> vOSC        -> vpu_mux;
				testdebug0 -> vtestdebug0 -> vpu_mux;
				testdebug1 -> vtestdebug1 -> vpu_mux;
				PLLA_core  -> vPLLA_core  -> vpu_mux;
				PLLC_core0 -> vPLLC_core1 -> vpu_mux;
				PLLD_core  -> vPLLD_core  -> vpu_mux;
				PLLH_aux   -> vPLLH_aux   -> vpu_mux;
				PLLC_core1 -> vPLLC_core1 -> vpu_mux;
				PLLC_core2 -> vPLLC_core2 -> vpu_mux;
			}

			vpu_mux -> vpu;
			vpu_mux -> v3d;
			vpu_mux -> isp;
			vpu_mux -> h264;
			vpu_mux -> sdram;
		}

		subgraph cluster_per_clocks {
			label = "Periperial-clocks";

			subgraph cluster_per_mux {
				label = "Periperal-mux";

				pGND        [label="0: GND"];
				pOSC        [label="1: OSC"];
				ptestdebug0 [label="2: testdebug0"];
				ptestdebug1 [label="3: testdebug1"];
				pPLLA_per   [label="4: PLLA_per"];
				pPLLC_per   [label="5: PLLC_per"];
				pPLLD_per   [label="5: PLLD_per"];
				pPLLH_aux   [label="5: PLLH_aux"];

				GND        -> pGND        -> per_mux;
				OSC        -> pOSC        -> per_mux;
				testdebug0 -> ptestdebug0 -> per_mux;
				testdebug1 -> ptestdebug1 -> per_mux;
				PLLA_per   -> pPLLA_per   -> per_mux;
				PLLC_per   -> pPLLC_per   -> per_mux;
				PLLD_per   -> pPLLD_per   -> per_mux;
				PLLH_aux   -> pPLLH_aux   -> per_mux;
			}

			per_mux -> vec;
			per_mux -> uart;
			per_mux -> hsm;
			per_mux -> emmc;
			per_mux -> pwm;
			per_mux -> pcm;
			per_mux -> aveo;
			per_mux -> cam0;
			per_mux -> cam1;
			per_mux -> dft;
			per_mux -> dpi;
			per_mux -> dsi0e;
			per_mux -> dsi1e;
			per_mux -> gp0;
			per_mux -> gp1;
			per_mux -> gp2;
			per_mux -> slim;
			per_mux -> smi;
		}

		subgraph cluster_osc_clocks {
			label = "osc-clocks";

			subgraph cluster_osc_mux {
				label = "osc-mux";

				oGND        [label="0: GND"];
				oOSC        [label="1: OSC"];
				otestdebug0 [label="2: testdebug0"];
				otestdebug1 [label="3: testdebug1"];

				GND        -> oGND        -> osc_mux;
				OSC        -> oOSC        -> osc_mux;
				testdebug0 -> otestdebug0 -> osc_mux;
				testdebug1 -> otestdebug1 -> osc_mux;
			}

			osc_mux -> tsens;
			osc_mux -> tec;
			osc_mux -> otp;
		}

		subgraph cluster_unknown_mux_clocks {
			label = "unknown-parent-mux-clocks";

			ukn_mux -> ccp2;
			ukn_mux -> dsi0pix;
			ukn_mux -> dsi1pix;
			ukn_mux -> pulse;
			ukn_mux -> td0;
			ukn_mux -> td1;
		}

		subgraph cluster_debug_clocks {
			label = "debug-clocks";

			debug_mux -> peria;
			debug_mux -> sys;
		}
	}

	subgraph cluster_aux {
		label = “auxiliar-clocks";

		vpu -> spi1;
		vpu -> spi2;
		vpu -> uart1;
	}
}
--- cut here ---

I have no idea where we could put this information
in the kernel tree - maybe this would help.

From this you can see that we have:
* PLL that is not configured now:
  * PLLB
* a few more PLL-dividers that are not configured:
  * PLLB_ARM
  * PLLB_SP0
  * PLLB_SP1
  * PLLB_SP2
  * PLLD_dsi0
  * PLLD_dsi1
  * PLLH_rcal
* a few clocks which Eric has said are wrong and where
  the corresponding parents need to get found:
  * ccp2 (gate)
  * dsi0pix (gate - maybe PLLD_dis0?)
  * dis1pix (gate - maybe PLLD_dis1)
  * pulse (gate)
  * td0
  * td1
* debug clocks that require a running debug power domain:
  * peria
  * sys
* missing clocks in the current driver:
  * aveo
  * cam0
  * cam1
  * ccp2 (see note above)
  * dtf
  * dpi
  * dsi0e   (maybe this also uses a different parent clock mux?)
  * dsi0pix (maybe this also uses a different parent clock mux?)
  * dsi1e   (maybe this also uses a different parent clock mux?)
  * dsi1pix (maybe this also uses a different parent clock mux?)
  * gp0
  * gp1
  * gp2
  * peria (see note above)
  * pulse (see note above)
  * slim
  * smi
  * td0 (see note above)
  * td1 (see note above)
  * td2 (see note above)
  * tec
  * sys (see note above)
* generic clock that is supposedly not in use

I can create a new patch that will include all those
missing clocks that are undisputed in their settings
(hence without any comments/notes) - I hope this is acceptable.

If someone has any ideas about those clocks where we 
miss the correct parents/mux, then please add your
wisdom.

Re: [PATCH V4 5/7] clk: bcm2835: add missing 22 HW-clocks.

[Stefan Wahren]

> Martin Sperl <kernel@martin.sperl.org> hat am 17. Februar 2016 um 19:25
> geschrieben:
>
> As I am trying to complete the list of clocks here my “best” summary
> so far of the clock tree for the bcm2835 taken from various sources:
> * Datasheet
> * broadcom provided VC4 headers (via https://github.com/msperl/rpi-registers)
> * mailing list
>
> And I have come up with the following description that
> you can put in graphviz or http://www.webgraphviz.com/
>
> --- cut here ---
> digraph clocks {
> graph [ordering="out"];
>
> subgraph cluster_osc {
> label = "Oscillators";
>
> "GND";
> "OSC";
> "testdebug0";
> "testdebug1";
> }
>
> subgraph cluster_pll {
> label = "PLLs";
>
> subgraph cluster_plla {
> label = "PLLA";
>
> OSC -> PLLA;
>
> PLLA -> PLLA_core;
> PLLA -> PLLA_per;
> PLLA -> PLLA_dsi0;
> PLLA -> PLLA_ccp2;
> }
>
> subgraph cluster_pllb {
> label = "PLLB";
>
> OSC -> PLLB;
>
> PLLB -> PLLB_ARM;
> PLLB -> PLLB_SP0;
> PLLB -> PLLB_SP1;
> PLLB -> PLLB_SP2;
> }
>
> subgraph cluster_pllc {
> label = "PLLC";
>
> OSC -> PLLC;
>
> PLLC -> PLLC_core0;
> PLLC -> PLLC_core1;
> PLLC -> PLLC_core2;
> PLLC -> PLLC_per;
> }
>
> subgraph cluster_plld {
> label = "PLLD";
>
> OSC -> PLLD;
>
> PLLD -> PLLD_core;
> PLLD -> PLLD_per;
> PLLD -> PLLD_dsi0;
> PLLD -> PLLD_dsi1;
> }
>
> subgraph cluster_pllh {
> label = "PLLH";
>
> OSC -> PLLH;
>
> PLLH -> PLLH_aux;
> PLLH -> PLLH_pix;
> PLLH -> PLLH_rcal;
> }
> }
>
> subgraph cluster_mux {
> label = "clocks";
>
> subgraph cluster_vpu_clocks {
> label = "VPU-clocks";
>
> subgraph cluster_vpu_mux {
> label = "VPU-mux";
>
> vGND [label="0: GND"];
> vOSC [label="1: OSC"];
> vtestdebug0 [label="2: testdebug0"];
> vtestdebug1 [label="3: testdebug1"];
> vPLLA_core [label="4: PLLA_core"];
> vPLLC_core0 [label="5: PLLC_core0"];
> vPLLD_core [label="6: PLLD_core"];
> vPLLH_aux [label="7: PLLH_aux"];
> vPLLC_core1 [label="8: PLLC_core1"];
> vPLLC_core2 [label="9: PLLC_core2"];
>
> GND -> vGND -> vpu_mux;
> OSC -> vOSC -> vpu_mux;
> testdebug0 -> vtestdebug0 -> vpu_mux;
> testdebug1 -> vtestdebug1 -> vpu_mux;
> PLLA_core -> vPLLA_core -> vpu_mux;
> PLLC_core0 -> vPLLC_core1 -> vpu_mux;
> PLLD_core -> vPLLD_core -> vpu_mux;
> PLLH_aux -> vPLLH_aux -> vpu_mux;
> PLLC_core1 -> vPLLC_core1 -> vpu_mux;
> PLLC_core2 -> vPLLC_core2 -> vpu_mux;
> }
>
> vpu_mux -> vpu;
> vpu_mux -> v3d;
> vpu_mux -> isp;
> vpu_mux -> h264;
> vpu_mux -> sdram;
> }
>
> subgraph cluster_per_clocks {
> label = "Periperial-clocks";
>
> subgraph cluster_per_mux {
> label = "Periperal-mux";
>
> pGND [label="0: GND"];
> pOSC [label="1: OSC"];
> ptestdebug0 [label="2: testdebug0"];
> ptestdebug1 [label="3: testdebug1"];
> pPLLA_per [label="4: PLLA_per"];
> pPLLC_per [label="5: PLLC_per"];
> pPLLD_per [label="5: PLLD_per"];
> pPLLH_aux [label="5: PLLH_aux"];
>
> GND -> pGND -> per_mux;
> OSC -> pOSC -> per_mux;
> testdebug0 -> ptestdebug0 -> per_mux;
> testdebug1 -> ptestdebug1 -> per_mux;
> PLLA_per -> pPLLA_per -> per_mux;
> PLLC_per -> pPLLC_per -> per_mux;
> PLLD_per -> pPLLD_per -> per_mux;
> PLLH_aux -> pPLLH_aux -> per_mux;
> }
>
> per_mux -> vec;
> per_mux -> uart;
> per_mux -> hsm;
> per_mux -> emmc;
> per_mux -> pwm;
> per_mux -> pcm;
> per_mux -> aveo;
> per_mux -> cam0;
> per_mux -> cam1;
> per_mux -> dft;
> per_mux -> dpi;
> per_mux -> dsi0e;
> per_mux -> dsi1e;
> per_mux -> gp0;
> per_mux -> gp1;
> per_mux -> gp2;
> per_mux -> slim;
> per_mux -> smi;
> }
>
> subgraph cluster_osc_clocks {
> label = "osc-clocks";
>
> subgraph cluster_osc_mux {
> label = "osc-mux";
>
> oGND [label="0: GND"];
> oOSC [label="1: OSC"];
> otestdebug0 [label="2: testdebug0"];
> otestdebug1 [label="3: testdebug1"];
>
> GND -> oGND -> osc_mux;
> OSC -> oOSC -> osc_mux;
> testdebug0 -> otestdebug0 -> osc_mux;
> testdebug1 -> otestdebug1 -> osc_mux;
> }
>
> osc_mux -> tsens;
> osc_mux -> tec;
> osc_mux -> otp;
> }
>
> subgraph cluster_unknown_mux_clocks {
> label = "unknown-parent-mux-clocks";
>
> ukn_mux -> ccp2;
> ukn_mux -> dsi0pix;
> ukn_mux -> dsi1pix;
> ukn_mux -> pulse;
> ukn_mux -> td0;
> ukn_mux -> td1;
> }
>
> subgraph cluster_debug_clocks {
> label = "debug-clocks";
>
> debug_mux -> peria;
> debug_mux -> sys;
> }
> }
>
> subgraph cluster_aux {
> label = “auxiliar-clocks";
>
> vpu -> spi1;
> vpu -> spi2;
> vpu -> uart1;
> }
> }
> --- cut here ---
>
> I have no idea where we could put this information
> in the kernel tree - maybe this would help.
>

Documentation/devicetree/bindings/clock/brcm,bcm2835-cprman.txt should be a good
place.

[PATCH V4 6/7] clk: bcm2835: enable fractional and mash support

[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.

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 by assigning .mash in the
clock description structure.

Right now there are no limits imposed on maximum frequencies
or additional restrictions on min/max divider when MASH/FRAC
is enabled and used.

The way that implementation is done the top 8 bits of the divider
are used to signify the mash level used for this divider.
This has been wrapped in a new type mashtype with some helper
functions that allows extraction of the divider information.
typedef is used to minimize the amount of change.
(note also that the top 8 bit are not used by the clock registers
besides requireing some magic when setting the register)

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 drivers/clk/bcm/clk-bcm2835.c |  111 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 104 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 1640288..545c21a 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -51,6 +51,7 @@
 #define CM_GNRICCTL		0x000
 #define CM_GNRICDIV		0x004
 # define CM_DIV_FRAC_BITS	12
+# define CM_DIV_INT_BITS	12

 #define CM_VPUCTL		0x008
 #define CM_VPUDIV		0x00c
@@ -130,6 +131,10 @@
 # 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_SRC_SHIFT			0
 # define CM_SRC_BITS			4
 # define CM_SRC_MASK			0xf
@@ -296,6 +301,63 @@
 #define LOCK_TIMEOUT_NS		100000000
 #define BCM2835_MAX_FB_RATE	1750000000u

+enum bcm2835_clock_mash_type {
+	MASH_NONE = 0,
+	MASH_FRAC = 1,
+	MASH_2ND_ORDER = 2,
+	MASH_3RD_ORDER = 3
+};
+
+/*
+ * mashdiv - Helper type packed in u32 that contains
+ *   integer divider (bits 12-23)
+ *   integer divider (bits 11-0)
+ *   mash type       (bits 25-24)
+ * this also includes some helperfunctions to calc the compound u32
+ * as well as extracting the individual data from the compound u32.
+ */
+typedef u32 divmash;
+
+/* define the lowes/highest bits for a field and masks */
+#define DIVMASH_DIVF_LO_BIT 0
+#define DIVMASH_DIVF_HI_BIT (CM_DIV_FRAC_BITS - 1)
+#define DIVMASH_DIVF_MASK GENMASK(DIVMASH_DIVF_HI_BIT, DIVMASH_DIVF_LO_BIT)
+#define DIVMASH_DIVI_LO_BIT (DIVMASH_DIVF_HI_BIT + 1)
+#define DIVMASH_DIVI_HI_BIT (CM_DIV_INT_BITS + CM_DIV_FRAC_BITS - 1)
+#define DIVMASH_DIVI_MASK GENMASK(DIVMASH_DIVI_HI_BIT, DIVMASH_DIVI_LO_BIT)
+#define DIVMASH_DIV_LO_BIT DIVMASH_DIVF_LO_BIT
+#define DIVMASH_DIV_MASK (DIVMASH_DIVI_MASK | DIVMASH_DIVF_MASK)
+#define DIVMASH_MASH_LO_BIT (DIVMASH_DIVI_HI_BIT + 1)
+#define DIVMASH_MASH_HI_BIT (DIVMASH_MASH_LO_BIT + 1)
+#define DIVMASH_MASH_MASK GENMASK(DIVMASH_MASH_HI_BIT, DIVMASH_MASH_LO_BIT)
+
+static inline divmash divmash_calc(enum bcm2835_clock_mash_type mash,
+				   u32 div)
+{
+	return ((div << DIVMASH_DIV_LO_BIT)   & DIVMASH_DIV_MASK) |
+	       ((mash << DIVMASH_MASH_LO_BIT) & DIVMASH_MASH_MASK);
+}
+
+static inline enum bcm2835_clock_mash_type divmash_get_mash(divmash dm)
+{
+	return (dm & DIVMASH_MASH_MASK) >> DIVMASH_MASH_LO_BIT;
+}
+
+static inline u32 divmash_get_div(divmash dm)
+{
+	return (dm & DIVMASH_DIV_MASK) >> DIVMASH_DIV_LO_BIT;
+}
+
+static inline u32 divmash_get_divi(divmash dm)
+{
+	return (dm & DIVMASH_DIVI_MASK) >> DIVMASH_DIVI_LO_BIT;
+}
+
+static inline u32 divmash_get_divf(divmash dm)
+{
+	return (dm & DIVMASH_DIVF_MASK) >> DIVMASH_DIVF_LO_BIT;
+}
+
 struct bcm2835_cprman {
 	struct device *dev;
 	void __iomem *regs;
@@ -647,6 +709,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 */
+	enum bcm2835_clock_mash_type mash;

 	bool is_vpu_clock;
 };
@@ -1413,16 +1477,17 @@ static int bcm2835_clock_is_on(struct clk_hw *hw)
 	return (cprman_read(cprman, data->ctl_reg) & CM_ENABLE) != 0;
 }

-static u32 bcm2835_clock_choose_div(struct clk_hw *hw,
-				    unsigned long rate,
-				    unsigned long parent_rate,
-				    bool round_up)
+static divmash bcm2835_clock_choose_div(struct clk_hw *hw,
+					unsigned long rate,
+					unsigned long parent_rate,
+					bool round_up)
 {
 	struct bcm2835_clock *clock = bcm2835_clock_from_hw(hw);
 	const struct bcm2835_clock_data *data = clock->data;
 	u32 unused_frac_mask =
 		GENMASK(CM_DIV_FRAC_BITS - data->frac_bits, 0) >> 1;
 	u64 temp = (u64)parent_rate << CM_DIV_FRAC_BITS;
+	enum bcm2835_clock_mash_type mash = MASH_NONE;
 	u64 rem;
 	u32 div;

@@ -1443,7 +1508,11 @@ static u32 bcm2835_clock_choose_div(struct clk_hw *hw,
 	div = min_t(u32, div, GENMASK(data->int_bits + CM_DIV_FRAC_BITS - 1,
 				      CM_DIV_FRAC_BITS));

-	return div;
+	/* set mash if necessary */
+	if (data->frac_bits && (div & GENMASK(CM_DIV_FRAC_BITS - 1, 0)))
+		mash = data->mash ? data->mash : MASH_FRAC;
+
+	return divmash_calc(mash, div);
 }

 static long bcm2835_clock_rate_from_divisor(struct bcm2835_clock *clock,
@@ -1534,10 +1603,36 @@ static int bcm2835_clock_set_rate(struct clk_hw *hw,
 	struct bcm2835_clock *clock = bcm2835_clock_from_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);
+	divmash dm = bcm2835_clock_choose_div(hw, rate, parent_rate, false);
+	u32 div = divmash_get_div(dm);
+	enum bcm2835_clock_mash_type mash = divmash_get_mash(dm);
+	u32 ctl;
+
+	spin_lock(&cprman->regs_lock);

+	/* if div and mash are identical, then there is nothing to do */
+	ctl = cprman_read(cprman, data->ctl_reg);
+	if ((div == cprman_read(cprman, data->div_reg)) &&
+	    (CM_MASH(mash) == (ctl & CM_MASH_MASK)))
+		goto unlock_exit;
+
+	/*
+	 * Setting up mash type
+	 *
+	 * In principle it is recommended to stop/start the clock first,
+	 * but as we set CLK_SET_RATE_GATE during registration of the
+	 * clock this requirement should be take care of by the
+	 * clk-framework.
+	 */
+	cprman_write(cprman, data->ctl_reg,
+		     (ctl & ~CM_MASH_MASK) | CM_MASH(mash));
+
+	/* and set div */
 	cprman_write(cprman, data->div_reg, div);

+unlock_exit:
+	spin_unlock(&cprman->regs_lock);
+
 	return 0;
 }

@@ -1549,6 +1644,7 @@ static int bcm2835_clock_determine_rate(struct clk_hw *hw,
 	unsigned long rate, best_rate = 0;
 	unsigned long prate, best_prate = 0;
 	size_t i;
+	divmash dm;
 	u32 div;

 	/*
@@ -1559,7 +1655,8 @@ static int bcm2835_clock_determine_rate(struct clk_hw *hw,
 		if (!parent)
 			continue;
 		prate = clk_hw_get_rate(parent);
-		div = bcm2835_clock_choose_div(hw, req->rate, prate, true);
+		dm = bcm2835_clock_choose_div(hw, req->rate, prate, true);
+		div = divmash_get_div(dm);
 		rate = bcm2835_clock_rate_from_divisor(clock, prate, div);
 		if (rate > best_rate && rate <= req->rate) {
 			best_parent = parent;
--
1.7.10.4

[PATCH V4 7/7] clk: bcm2835: apply limits on dividers to MASH mode.

[kernel]

From: Martin Sperl <kernel@martin.sperl.org>

There are several limits on the divider as well as effective frequency
that apply when a fractional divider with higher order MASH is used.

This patch applies all the information about limits that is available
at this time and all of which has been tested empirically by
Mathias Reichl using primarily the pcm clock.

The patch tries to use the "highest" order of MASH support and when it
fails it reduces MASH order and retries the test - fall back all the way
to integer divide if necessary, where "normal" clamping of limits
happens.

Note that http://www.aholme.co.uk/Frac2/Mash.htm
contains a description of MASH, the author - allegedly - was
working for Broadcom at that time.

Suggested-by: Mathias Reichl <hias@horus.com>
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 drivers/clk/bcm/clk-bcm2835.c |   52 ++++++++++++++++++++++++++++++++---------
 1 file changed, 41 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 545c21a..637f8ae 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -300,6 +300,7 @@

 #define LOCK_TIMEOUT_NS		100000000
 #define BCM2835_MAX_FB_RATE	1750000000u
+#define BCM2835_MASH_MAX_FREQ	25000000u

 enum bcm2835_clock_mash_type {
 	MASH_NONE = 0,
@@ -1489,7 +1490,9 @@ static divmash bcm2835_clock_choose_div(struct clk_hw *hw,
 	u64 temp = (u64)parent_rate << CM_DIV_FRAC_BITS;
 	enum bcm2835_clock_mash_type mash = MASH_NONE;
 	u64 rem;
-	u32 div;
+	u32 div, divi, divf;
+	const u32 divi_max = BIT(data->int_bits) - 1;
+	const u32 divi_min = 2;

 	rem = do_div(temp, rate);
 	div = temp;
@@ -1499,20 +1502,47 @@ static divmash bcm2835_clock_choose_div(struct clk_hw *hw,
 		div += unused_frac_mask + 1;
 	div &= ~unused_frac_mask;

-	/* Clamp to the limits. */
+	divi = div >> CM_DIV_FRAC_BITS;
+	divf = div & GENMASK(CM_DIV_FRAC_BITS - 1, 0);

-	/* divider must be >= 2 */
-	div = max_t(u32, div, (2 << CM_DIV_FRAC_BITS));
+	/* select mash mode */
+	if (data->frac_bits && divf)
+		mash = data->mash ? data->mash : MASH_FRAC;

-	/* clamp to max divider allowed - max is integer divider */
-	div = min_t(u32, div, GENMASK(data->int_bits + CM_DIV_FRAC_BITS - 1,
-				      CM_DIV_FRAC_BITS));
+	/*
+	 * handle possible limits for different mash levels with fall-tru
+	 * For offset values see page 105 table 6-32 in
+	 * BCM2835-ARM-Peripherials as well as the errata at:
+	 *   http://elinux.org/BCM2835_datasheet_errata#p105_table
+	 */
+	switch (mash) {
+	case MASH_3RD_ORDER:
+		if ((divi >= divi_min + 3) &&
+		    (divi + 4 <= divi_max) &&
+		    (parent_rate / (divi - 3) <= BCM2835_MASH_MAX_FREQ))
+			return divmash_calc(MASH_3RD_ORDER, div);
+		/* fall tru if not in bounds */
+	case MASH_2ND_ORDER:
+		if ((divi >= divi_min + 1) &&
+		    (divi + 2 <= divi_max) &&
+		    (parent_rate / (divi - 1) <= BCM2835_MASH_MAX_FREQ))
+			return divmash_calc(MASH_2ND_ORDER, div);
+		/* fall tru if not in bounds */
+	case MASH_FRAC:
+		if ((divi >= divi_min) &&
+		    (divi + 1 <= divi_max))
+			return divmash_calc(MASH_FRAC, div);
+		/* fall tru if not in bounds */
+	case MASH_NONE:
+	default:
+		break;
+	}

-	/* set mash if necessary */
-	if (data->frac_bits && (div & GENMASK(CM_DIV_FRAC_BITS - 1, 0)))
-		mash = data->mash ? data->mash : MASH_FRAC;
+	/* we apply standard clamping based on divi alone */
+	divi = max(divi, divi_min);
+	divi = min(divi, divi_max);

-	return divmash_calc(mash, div);
+	return divmash_calc(MASH_NONE, divi << CM_DIV_FRAC_BITS);
 }

 static long bcm2835_clock_rate_from_divisor(struct bcm2835_clock *clock,
--
1.7.10.4

Re: [PATCH V4 0/7] clk: bcm2835: add clocks and add MASH support

[Martin Sperl]

> On 15.01.2016, at 15:20, 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.
> 
> There is also an issue when the clock divider is < 2 - in that
> case no clock-output is generate. The clamping code has been
> enhanced to handle this as well.
> 
> Similarly there is also the clamping of the highest divider now
> limited to the highest possible integer divider instead of the
> highest possible fractional diviver.
> 
> Finally we add all the 23 different HW clocks that have not been
> configured in the driver.
> 
> Changelog:
> 	V1 -> V2: split the asoc/sound patches from the clock patches
> 	      	  enable frac/mash support
> 	V2 -> V3: clamp clock divider to be >= 2
> 		  clamp max clock divider to be integer (not fractional)
> 		  added additional limit checks for divider selection
> 		    allowing fallback to lower mash levels.
> 		  use a newer probing mechanism based on a single array
> 	V3 -> V4: fixed bad NULL pointer check in init
> 	      	  fixed (mash) limit checks
> 		  separated basic mash support from limits checks
> 		  really tested with I2S device (unfortunately V3 was
> 		  not well tested in this respect)
> 
> Martin Sperl (7):
>  clk: bcm2835: the minimum clock divider is 2
>  clk: bcm2835: clamp clock divider to highest integer only
>  clk: bcm2835: remove use of BCM2835_CLOCK_COUNT in driver
>  clk: bcm2835: enable management of PCM clock
Please note that Mark Brown has already merged the patch to
bcm2835-i2s that makes use of the PCM-clock added with this patch.

>  clk: bcm2835: add missing 22 HW-clocks.
>  clk: bcm2835: enable fractional and mash support
>  clk: bcm2835: apply limits on dividers to MASH mode.
> 
> drivers/clk/bcm/clk-bcm2835.c       |  583 ++++++++++++++++++++++++++++++-----
> include/dt-bindings/clock/bcm2835.h |   25 +-
> 2 files changed, 521 insertions(+), 87 deletions(-)
> 
> --
> 1.7.10.4
> 

There is still some discussion around the best clock selection
method (especially low jitter for PCM and possibly PWM), but as I guess
not all “use-cases” for clocks are identical it would require some
parametrization.

As such parameters typically would go into device-tree here a
proposal for how we could implement the device-tree options:

	clocks: cprman@7e101000 {
		compatible = "brcm,bcm2835-cprman";
		#clock-cells = <1>;
		reg = <0x7e101000 0x2000>;

		/* CPRMAN derives everything from the platform's
		 * oscillator.
		 */
		clocks = <&clk_osc>;

		/* non default override for some clocks */
		clock@BCM2835_CLOCK_PCM {
			reg = <BCM2835_CLOCK_PCM>;

			/* all of the below properties are optional */

			/* only use the following as parents - but must
			 * be a member of all the permissible plls for this
			 * clock.
			 * note that PLLC is used for overclocking
			 * the ARM by downstream kernels, so it should not
			 * get used for PCM.
			 */
			parent-clock-names = “xosc”,
					     “plla_per”,
					     “plld_per”,
					     “pllh_aux”;
			/* alternate clock/divider selector */
			bcrm,clock-selector = <BCM2835_CLOCK_SELECTOR_LOWJITTER>;
			/* minimum divider used for fractional support */
			bcrm,min-div-f = <20>;
			/* alternatively defined explicitly as jitter */
			max-jitter-ns = <500>;
			/* mash_support defining the max MASH order */
			bcrm,max-mash-order = <BCM2835_MASH_ORDER_3>;
			/* possibly other parameters needed by the chosen
			 * clock selector.
			 */
		};
	};

Would something like this be acceptable?

I would then create a separate patchset based on this patchset to
introduce those features when I get back.

Thanks,
	Martin