All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND] clk: vc5: fix output disabling when enabling a FOD
@ 2021-05-27 21:16 Luca Ceresoli
  2021-06-02  8:00 ` Stephen Boyd
  2021-06-09  0:53 ` Stephen Boyd
  0 siblings, 2 replies; 6+ messages in thread
From: Luca Ceresoli @ 2021-05-27 21:16 UTC (permalink / raw)
  To: linux-clk
  Cc: Luca Ceresoli, Michael Turquette, Stephen Boyd, linux-kernel, Adam Ford

On 5P49V6965, when an output is enabled we enable the corresponding
FOD. When this happens for the first time, and specifically when writing
register VC5_OUT_DIV_CONTROL in vc5_clk_out_prepare(), all other outputs
are stopped for a short time and then restarted.

According to Renesas support this is intended: "The reason for that is VC6E
has synced up all output function".

This behaviour can be disabled at least on VersaClock 6E devices, of which
only the 5P49V6965 is currently implemented by this driver. This requires
writing bit 7 (bypass_sync{1..4}) in register 0x20..0x50.  Those registers
are named "Unused Factory Reserved Register", and the bits are documented
as "Skip VDDO<N> verification", which does not clearly explain the relation
to FOD sync. However according to Renesas support as well as my testing
setting this bit does prevent disabling of all clock outputs when enabling
a FOD.

See "VersaClock ® 6E Family Register Descriptions and Programming Guide"
(August 30, 2018), Table 116 "Power Up VDD check", page 58:
https://www.renesas.com/us/en/document/mau/versaclock-6e-family-register-descriptions-and-programming-guide

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Reviewed-by: Adam Ford <aford173@gmail.com>

---

Changes v1 -> v2:
 - add Adam's Reviewed-by tag
---
 drivers/clk/clk-versaclock5.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-versaclock5.c b/drivers/clk/clk-versaclock5.c
index 344cd6c61188..3c737742c2a9 100644
--- a/drivers/clk/clk-versaclock5.c
+++ b/drivers/clk/clk-versaclock5.c
@@ -69,7 +69,10 @@
 #define VC5_FEEDBACK_FRAC_DIV(n)		(0x19 + (n))
 #define VC5_RC_CONTROL0				0x1e
 #define VC5_RC_CONTROL1				0x1f
-/* Register 0x20 is factory reserved */
+
+/* These registers are named "Unused Factory Reserved Registers" */
+#define VC5_RESERVED_X0(idx)		(0x20 + ((idx) * 0x10))
+#define VC5_RESERVED_X0_BYPASS_SYNC	BIT(7) /* bypass_sync<idx> bit */
 
 /* Output divider control for divider 1,2,3,4 */
 #define VC5_OUT_DIV_CONTROL(idx)	(0x21 + ((idx) * 0x10))
@@ -87,7 +90,6 @@
 #define VC5_OUT_DIV_SKEW_INT(idx, n)	(0x2b + ((idx) * 0x10) + (n))
 #define VC5_OUT_DIV_INT(idx, n)		(0x2d + ((idx) * 0x10) + (n))
 #define VC5_OUT_DIV_SKEW_FRAC(idx)	(0x2f + ((idx) * 0x10))
-/* Registers 0x30, 0x40, 0x50 are factory reserved */
 
 /* Clock control register for clock 1,2 */
 #define VC5_CLK_OUTPUT_CFG(idx, n)	(0x60 + ((idx) * 0x2) + (n))
@@ -140,6 +142,8 @@
 #define VC5_HAS_INTERNAL_XTAL	BIT(0)
 /* chip has PFD requency doubler */
 #define VC5_HAS_PFD_FREQ_DBL	BIT(1)
+/* chip has bits to disable FOD sync */
+#define VC5_HAS_BYPASS_SYNC_BIT	BIT(2)
 
 /* Supported IDT VC5 models. */
 enum vc5_model {
@@ -581,6 +585,23 @@ static int vc5_clk_out_prepare(struct clk_hw *hw)
 	unsigned int src;
 	int ret;
 
+	/*
+	 * When enabling a FOD, all currently enabled FODs are briefly
+	 * stopped in order to synchronize all of them. This causes a clock
+	 * disruption to any unrelated chips that might be already using
+	 * other clock outputs. Bypass the sync feature to avoid the issue,
+	 * which is possible on the VersaClock 6E family via reserved
+	 * registers.
+	 */
+	if (vc5->chip_info->flags & VC5_HAS_BYPASS_SYNC_BIT) {
+		ret = regmap_update_bits(vc5->regmap,
+					 VC5_RESERVED_X0(hwdata->num),
+					 VC5_RESERVED_X0_BYPASS_SYNC,
+					 VC5_RESERVED_X0_BYPASS_SYNC);
+		if (ret)
+			return ret;
+	}
+
 	/*
 	 * If the input mux is disabled, enable it first and
 	 * select source from matching FOD.
@@ -1166,7 +1187,7 @@ static const struct vc5_chip_info idt_5p49v6965_info = {
 	.model = IDT_VC6_5P49V6965,
 	.clk_fod_cnt = 4,
 	.clk_out_cnt = 5,
-	.flags = 0,
+	.flags = VC5_HAS_BYPASS_SYNC_BIT,
 };
 
 static const struct i2c_device_id vc5_id[] = {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RESEND] clk: vc5: fix output disabling when enabling a FOD
  2021-05-27 21:16 [PATCH RESEND] clk: vc5: fix output disabling when enabling a FOD Luca Ceresoli
@ 2021-06-02  8:00 ` Stephen Boyd
  2021-06-03  8:44   ` Luca Ceresoli
  2021-06-09  0:53 ` Stephen Boyd
  1 sibling, 1 reply; 6+ messages in thread
From: Stephen Boyd @ 2021-06-02  8:00 UTC (permalink / raw)
  To: Luca Ceresoli, linux-clk
  Cc: Luca Ceresoli, Michael Turquette, linux-kernel, Adam Ford

Quoting Luca Ceresoli (2021-05-27 14:16:47)
> On 5P49V6965, when an output is enabled we enable the corresponding
> FOD. When this happens for the first time, and specifically when writing
> register VC5_OUT_DIV_CONTROL in vc5_clk_out_prepare(), all other outputs
> are stopped for a short time and then restarted.
> 
> According to Renesas support this is intended: "The reason for that is VC6E
> has synced up all output function".
> 
> This behaviour can be disabled at least on VersaClock 6E devices, of which
> only the 5P49V6965 is currently implemented by this driver. This requires
> writing bit 7 (bypass_sync{1..4}) in register 0x20..0x50.  Those registers
> are named "Unused Factory Reserved Register", and the bits are documented
> as "Skip VDDO<N> verification", which does not clearly explain the relation
> to FOD sync. However according to Renesas support as well as my testing
> setting this bit does prevent disabling of all clock outputs when enabling
> a FOD.
> 
> See "VersaClock ® 6E Family Register Descriptions and Programming Guide"
> (August 30, 2018), Table 116 "Power Up VDD check", page 58:
> https://www.renesas.com/us/en/document/mau/versaclock-6e-family-register-descriptions-and-programming-guide
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> Reviewed-by: Adam Ford <aford173@gmail.com>
> 
> ---

Any Fixes tag for this patch?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RESEND] clk: vc5: fix output disabling when enabling a FOD
  2021-06-02  8:00 ` Stephen Boyd
@ 2021-06-03  8:44   ` Luca Ceresoli
  2021-06-03 20:07     ` Stephen Boyd
  0 siblings, 1 reply; 6+ messages in thread
From: Luca Ceresoli @ 2021-06-03  8:44 UTC (permalink / raw)
  To: Stephen Boyd, linux-clk; +Cc: Michael Turquette, linux-kernel, Adam Ford

Hi Stephen,

On 02/06/21 10:00, Stephen Boyd wrote:
> Quoting Luca Ceresoli (2021-05-27 14:16:47)
>> On 5P49V6965, when an output is enabled we enable the corresponding
>> FOD. When this happens for the first time, and specifically when writing
>> register VC5_OUT_DIV_CONTROL in vc5_clk_out_prepare(), all other outputs
>> are stopped for a short time and then restarted.
>>
>> According to Renesas support this is intended: "The reason for that is VC6E
>> has synced up all output function".
>>
>> This behaviour can be disabled at least on VersaClock 6E devices, of which
>> only the 5P49V6965 is currently implemented by this driver. This requires
>> writing bit 7 (bypass_sync{1..4}) in register 0x20..0x50.  Those registers
>> are named "Unused Factory Reserved Register", and the bits are documented
>> as "Skip VDDO<N> verification", which does not clearly explain the relation
>> to FOD sync. However according to Renesas support as well as my testing
>> setting this bit does prevent disabling of all clock outputs when enabling
>> a FOD.
>>
>> See "VersaClock ® 6E Family Register Descriptions and Programming Guide"
>> (August 30, 2018), Table 116 "Power Up VDD check", page 58:
>> https://www.renesas.com/us/en/document/mau/versaclock-6e-family-register-descriptions-and-programming-guide
>>
>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>> Reviewed-by: Adam Ford <aford173@gmail.com>
>>
>> ---
> 
> Any Fixes tag for this patch?

I didn't add any as there is no commit that is clearly introducing the
problem. This patch fixes a behavior of the chip, which is there by
design by causes problems in some use cases.

If a Fixes tag is required than I guess it should be the commit adding
support for the 5P49V6965, which is the only supported variant of VC[56]
having having the problematic behavior _and_ the reserved register bits
to prevent it. However I hardly could blame the author of that code for
such a "peculiar" chip behaviour. Do you still want me to add such a tag?

-- 
Luca


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RESEND] clk: vc5: fix output disabling when enabling a FOD
  2021-06-03  8:44   ` Luca Ceresoli
@ 2021-06-03 20:07     ` Stephen Boyd
  2021-06-03 21:06       ` Luca Ceresoli
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Boyd @ 2021-06-03 20:07 UTC (permalink / raw)
  To: Luca Ceresoli, linux-clk; +Cc: Michael Turquette, linux-kernel, Adam Ford

Quoting Luca Ceresoli (2021-06-03 01:44:57)
> Hi Stephen,
> 
> On 02/06/21 10:00, Stephen Boyd wrote:
> > Quoting Luca Ceresoli (2021-05-27 14:16:47)
> >> On 5P49V6965, when an output is enabled we enable the corresponding
> >> FOD. When this happens for the first time, and specifically when writing
> >> register VC5_OUT_DIV_CONTROL in vc5_clk_out_prepare(), all other outputs
> >> are stopped for a short time and then restarted.
> >>
> >> According to Renesas support this is intended: "The reason for that is VC6E
> >> has synced up all output function".
> >>
> >> This behaviour can be disabled at least on VersaClock 6E devices, of which
> >> only the 5P49V6965 is currently implemented by this driver. This requires
> >> writing bit 7 (bypass_sync{1..4}) in register 0x20..0x50.  Those registers
> >> are named "Unused Factory Reserved Register", and the bits are documented
> >> as "Skip VDDO<N> verification", which does not clearly explain the relation
> >> to FOD sync. However according to Renesas support as well as my testing
> >> setting this bit does prevent disabling of all clock outputs when enabling
> >> a FOD.
> >>
> >> See "VersaClock ® 6E Family Register Descriptions and Programming Guide"
> >> (August 30, 2018), Table 116 "Power Up VDD check", page 58:
> >> https://www.renesas.com/us/en/document/mau/versaclock-6e-family-register-descriptions-and-programming-guide
> >>
> >> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> >> Reviewed-by: Adam Ford <aford173@gmail.com>
> >>
> >> ---
> > 
> > Any Fixes tag for this patch?
> 
> I didn't add any as there is no commit that is clearly introducing the
> problem. This patch fixes a behavior of the chip, which is there by
> design by causes problems in some use cases.
> 
> If a Fixes tag is required than I guess it should be the commit adding
> support for the 5P49V6965, which is the only supported variant of VC[56]
> having having the problematic behavior _and_ the reserved register bits
> to prevent it. However I hardly could blame the author of that code for
> such a "peculiar" chip behaviour. Do you still want me to add such a tag?

I tend to liberally apply the Fixes tag if something is being fixed. It
helps understand that the patch is not introducing a new feature and
when the incorrect code was introduced. I can slap on a Fixes tag
myself, just not sure what to do.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RESEND] clk: vc5: fix output disabling when enabling a FOD
  2021-06-03 20:07     ` Stephen Boyd
@ 2021-06-03 21:06       ` Luca Ceresoli
  0 siblings, 0 replies; 6+ messages in thread
From: Luca Ceresoli @ 2021-06-03 21:06 UTC (permalink / raw)
  To: Stephen Boyd, linux-clk; +Cc: Michael Turquette, linux-kernel, Adam Ford

Hi,

On 03/06/21 22:07, Stephen Boyd wrote:
> Quoting Luca Ceresoli (2021-06-03 01:44:57)
>> Hi Stephen,
>>
>> On 02/06/21 10:00, Stephen Boyd wrote:
>>> Quoting Luca Ceresoli (2021-05-27 14:16:47)
>>>> On 5P49V6965, when an output is enabled we enable the corresponding
>>>> FOD. When this happens for the first time, and specifically when writing
>>>> register VC5_OUT_DIV_CONTROL in vc5_clk_out_prepare(), all other outputs
>>>> are stopped for a short time and then restarted.
>>>>
>>>> According to Renesas support this is intended: "The reason for that is VC6E
>>>> has synced up all output function".
>>>>
>>>> This behaviour can be disabled at least on VersaClock 6E devices, of which
>>>> only the 5P49V6965 is currently implemented by this driver. This requires
>>>> writing bit 7 (bypass_sync{1..4}) in register 0x20..0x50.  Those registers
>>>> are named "Unused Factory Reserved Register", and the bits are documented
>>>> as "Skip VDDO<N> verification", which does not clearly explain the relation
>>>> to FOD sync. However according to Renesas support as well as my testing
>>>> setting this bit does prevent disabling of all clock outputs when enabling
>>>> a FOD.
>>>>
>>>> See "VersaClock ® 6E Family Register Descriptions and Programming Guide"
>>>> (August 30, 2018), Table 116 "Power Up VDD check", page 58:
>>>> https://www.renesas.com/us/en/document/mau/versaclock-6e-family-register-descriptions-and-programming-guide
>>>>
>>>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>>>> Reviewed-by: Adam Ford <aford173@gmail.com>
>>>>
>>>> ---
>>>
>>> Any Fixes tag for this patch?
>>
>> I didn't add any as there is no commit that is clearly introducing the
>> problem. This patch fixes a behavior of the chip, which is there by
>> design by causes problems in some use cases.
>>
>> If a Fixes tag is required than I guess it should be the commit adding
>> support for the 5P49V6965, which is the only supported variant of VC[56]
>> having having the problematic behavior _and_ the reserved register bits
>> to prevent it. However I hardly could blame the author of that code for
>> such a "peculiar" chip behaviour. Do you still want me to add such a tag?
> 
> I tend to liberally apply the Fixes tag if something is being fixed. It
> helps understand that the patch is not introducing a new feature and
> when the incorrect code was introduced. I can slap on a Fixes tag
> myself, just not sure what to do.
> 

If you're OK in adding it while applying, here it is:

Fixes: 2bda748e6ad8 ("clk: vc5: Add support for IDT VersaClock 5P49V6965")

Thanks.
-- 
Luca


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RESEND] clk: vc5: fix output disabling when enabling a FOD
  2021-05-27 21:16 [PATCH RESEND] clk: vc5: fix output disabling when enabling a FOD Luca Ceresoli
  2021-06-02  8:00 ` Stephen Boyd
@ 2021-06-09  0:53 ` Stephen Boyd
  1 sibling, 0 replies; 6+ messages in thread
From: Stephen Boyd @ 2021-06-09  0:53 UTC (permalink / raw)
  To: Luca Ceresoli, linux-clk
  Cc: Luca Ceresoli, Michael Turquette, linux-kernel, Adam Ford

Quoting Luca Ceresoli (2021-05-27 14:16:47)
> On 5P49V6965, when an output is enabled we enable the corresponding
> FOD. When this happens for the first time, and specifically when writing
> register VC5_OUT_DIV_CONTROL in vc5_clk_out_prepare(), all other outputs
> are stopped for a short time and then restarted.
> 
> According to Renesas support this is intended: "The reason for that is VC6E
> has synced up all output function".
> 
> This behaviour can be disabled at least on VersaClock 6E devices, of which
> only the 5P49V6965 is currently implemented by this driver. This requires
> writing bit 7 (bypass_sync{1..4}) in register 0x20..0x50.  Those registers
> are named "Unused Factory Reserved Register", and the bits are documented
> as "Skip VDDO<N> verification", which does not clearly explain the relation
> to FOD sync. However according to Renesas support as well as my testing
> setting this bit does prevent disabling of all clock outputs when enabling
> a FOD.
> 
> See "VersaClock ® 6E Family Register Descriptions and Programming Guide"
> (August 30, 2018), Table 116 "Power Up VDD check", page 58:
> https://www.renesas.com/us/en/document/mau/versaclock-6e-family-register-descriptions-and-programming-guide
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> Reviewed-by: Adam Ford <aford173@gmail.com>
> 
> ---

Applied to clk-next

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-06-09  0:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-27 21:16 [PATCH RESEND] clk: vc5: fix output disabling when enabling a FOD Luca Ceresoli
2021-06-02  8:00 ` Stephen Boyd
2021-06-03  8:44   ` Luca Ceresoli
2021-06-03 20:07     ` Stephen Boyd
2021-06-03 21:06       ` Luca Ceresoli
2021-06-09  0:53 ` Stephen Boyd

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.