RE: [EXTERNAL] Re: [PATCH] clk: vc5: Enable VC5_HAS_PFD_FREQ_DBL on 5p49v6965

[Fillion, Claude]

Hello Luca,

I will defer to Adam, but a few comments:

> -----Original Message-----
> From: Luca Ceresoli <luca@xxxxxxxxxxxxxxxx>
> Sent: Tuesday, March 15, 2022 4:55 AM
> To: Adam Ford <aford173@xxxxxxxxx>; linux-clk@xxxxxxxxxxxxxxx
> Cc: aford@xxxxxxxxxxxxxxxxxx; cstevens@xxxxxxxxxxxxxxxxxx;
> Fillion, Claude <Claude.Fillion@xxxxxxxxxxx>; Michael Turquette
> <mturquette@xxxxxxxxxxxx>; Stephen Boyd <sboyd@xxxxxxxxxx>; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: [EXTERNAL] Re: [PATCH] clk: vc5: Enable VC5_HAS_PFD_FREQ_DBL
> on 5p49v6965
> 
> This email originated outside of MKS.  Use caution when sharing information
> or opening attachments and links.
> 
> ----------------------------------------------------------------------------------------------
> ----------------------------------------------
> Hi Adam, Claude,
> 
> thanks for your patch.
> 
> On 13/03/22 12:57, Adam Ford wrote:
> > The 5p49v6965 has a reference clock frequency doubler.
> > Enabling it adds versaclock_som.dbl to the clock tree, but the output
> > frequency remains correct.
> >
> > Suggested-by: Claude Fillion <Claude.Fillion@xxxxxxxxxxx>
> > Signed-off-by: Adam Ford <aford173@xxxxxxxxx>
> >
> > diff --git a/drivers/clk/clk-versaclock5.c
> > b/drivers/clk/clk-versaclock5.c index e7be3e54b9be..4d190579e874
> > 100644
> > --- a/drivers/clk/clk-versaclock5.c
> > +++ b/drivers/clk/clk-versaclock5.c
> > @@ -1211,7 +1211,7 @@ static const struct vc5_chip_info
> idt_5p49v6965_info = {
> >  	.model = IDT_VC6_5P49V6965,
> >  	.clk_fod_cnt = 4,
> >  	.clk_out_cnt = 5,
> > -	.flags = VC5_HAS_BYPASS_SYNC_BIT,
> > +	.flags = VC5_HAS_BYPASS_SYNC_BIT | VC5_HAS_PFD_FREQ_DBL,
> 
> 
> If my understanding is correct, the doubler is not mentioned by the
> datasheet, but it exists. Maybe it's worth a line of comment to help future
> readers not waste their time in finding out:
>   /* Frequency doubler not mentioned on datasheet */
> 

I see the doubler bit mentioned in Table 25 of both v6 and v6e specs.  It is named differently, but appears to have the same purpose. 


> Can you confirm that:
>  - the en_ref_doubler bit value defaults to zero when reading it, as the
>    register guide says?
>  - if set to 1 the frequencies double?
> 
> With that confirmed, the patch looks good.
> 
> Thanks,
> --
> Luca

I played around a bit with the programming board today and did not see what I expected to see.

Using i2cget I see that the register in question (0x10) has a default value of 0xA0 for both 6901 and 6965.  Thus it seems disabled by default for both parts.

Starting at my base frequency of 46.8MHz, setting the bit to 1 (i2cset)  changes the output  frequency to 59.04MHz for the 6901 part, and to 47.7MHz for the 6965 part.  So setting the 'doubler' bit changes output frequency for both parts, but not the same amount.

Not sure of the meaning, just want to pass the information along.

-Claude


======================================================================
This message and any attachments are intended only for the designated recipient(s) and may contain confidential or proprietary information and be subject to the attorney-client privilege or other confidentiality protections.  If you are not a designated recipient, you may not review, use, copy or distribute this message or any attachments.  If you received this email in error, please notify the sender by reply e-mail and permanently delete the original and any copies of this message and any attachments thereto.  Thank you.