| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <MN2PR03MB5008265A724E8E262C3736F793169@MN2PR03MB5008.namprd03.prod.outlook.com>
Date: Mon, 21 Mar 2022 21:49:14 +0000
From: "Fillion, Claude" <Claude.Fillion@...inst.com>
To: Adam Ford <aford173@...il.com>
CC: Luca Ceresoli <luca@...aceresoli.net>,
"linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
"aford@...conembedded.com" <aford@...conembedded.com>,
"cstevens@...conembedded.com" <cstevens@...conembedded.com>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Marek Vasut <marek.vasut@...il.com>
Subject: RE: [EXTERNAL] Re: [PATCH] clk: vc5: Enable VC5_HAS_PFD_FREQ_DBL on
5p49v6965
> -----Original Message-----
> From: Fillion, Claude
> Sent: Monday, March 21, 2022 5:12 PM
> To: Adam Ford <aford173@...il.com>
> Cc: Luca Ceresoli <luca@...aceresoli.net>; linux-clk@...r.kernel.org;
> aford@...conembedded.com; cstevens@...conembedded.com; Michael
> Turquette <mturquette@...libre.com>; Stephen Boyd
> <sboyd@...nel.org>; linux-kernel@...r.kernel.org; Marek Vasut
> <marek.vasut@...il.com>
> Subject: RE: [EXTERNAL] Re: [PATCH] clk: vc5: Enable
> VC5_HAS_PFD_FREQ_DBL on 5p49v6965
>
> Hello Adam and Luca,
>
> > -----Original Message-----
> > From: Adam Ford <aford173@...il.com>
> > Sent: Saturday, March 19, 2022 5:24 PM
> > To: Fillion, Claude <Claude.Fillion@...inst.com>
> > Cc: Luca Ceresoli <luca@...aceresoli.net>; linux-clk@...r.kernel.org;
> > aford@...conembedded.com; cstevens@...conembedded.com;
> Michael
> > Turquette <mturquette@...libre.com>; Stephen Boyd
> <sboyd@...nel.org>;
> > linux-kernel@...r.kernel.org; Marek Vasut <marek.vasut@...il.com>
> > Subject: Re: [EXTERNAL] Re: [PATCH] clk: vc5: Enable
> > VC5_HAS_PFD_FREQ_DBL on 5p49v6965
> >
> > On Thu, Mar 17, 2022 at 1:57 PM Fillion, Claude
> > <Claude.Fillion@...inst.com> wrote:
> > >
> > > Hello Luca,
> > >
> > > > -----Original Message-----
> > > > From: Luca Ceresoli <luca@...aceresoli.net>
> > > > Sent: Tuesday, March 15, 2022 6:53 PM
> > > > To: Fillion, Claude <Claude.Fillion@...inst.com>; Adam Ford
> > > > <aford173@...il.com>; linux-clk@...r.kernel.org
> > > > Cc: aford@...conembedded.com; cstevens@...conembedded.com;
> > Michael
> > > > Turquette <mturquette@...libre.com>; Stephen Boyd
> > > > <sboyd@...nel.org>; linux-kernel@...r.kernel.org; Marek Vasut
> > > > <marek.vasut@...il.com>
> > > > Subject: Re: [EXTERNAL] Re: [PATCH] clk: vc5: Enable
> > > > VC5_HAS_PFD_FREQ_DBL on 5p49v6965
> > > >
> > > > Hi Claude,
> > > >
> > > > [adding Marek in Cc:, the original author of the driver and also
> > > > of the frequency doubler]
> > > >
> > > > On 15/03/22 20:34, Fillion, Claude wrote:
> > > > > Hello Luca,
> > > > >
> > > > > I will defer to Adam, but a few comments:
> > > > >
> > > > >> -----Original Message-----
> > > > >> From: Luca Ceresoli <luca@...aceresoli.net>
> > > > >> Sent: Tuesday, March 15, 2022 4:55 AM
> > > > >> To: Adam Ford <aford173@...il.com>; linux-clk@...r.kernel.org
> > > > >> Cc: aford@...conembedded.com;
> > cstevens@...conembedded.com;
> > > > Fillion,
> > > > >> Claude <Claude.Fillion@...inst.com>; Michael Turquette
> > > > >> <mturquette@...libre.com>; Stephen Boyd <sboyd@...nel.org>;
> > > > >> linux- kernel@...r.kernel.org
> > > > >> 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@...inst.com>
> > > > >>> Signed-off-by: Adam Ford <aford173@...il.com>
> > > > >>>
> > > > >>> 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.
> > > >
> > > > Well, literally speaking what I wrote is correct: the _datasheet_
> > > > does not mention the doubler. Table 25 you mention is on the
> > > > "Register Description and Programming Guide".
> > > >
> > > > Practically speaking I would expect the datasheet to mention the
> > > > hardware blocks including the doubler, but apparently Renesas has
> > > > a different opinion and perhaps they are not alone.
> > > >
> > > > So I think you can forget about my proposal to add a comment.
> > > >
> > > > >> 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.
> > > >
> > > > Coherently with the Register guide. OK.
> > > >
> > > > > 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.
> > > >
> > > > Me neither.
> > > >
> > > > I have no clever idea, only this one that I consider unlikely: by
> > > > enabling the doubler you may have increased some internal
> > > > frequency above its allowed range and thus the chip is not working
> > > > properly anymore. Can you use a lower base frequency or check the
> > > > PLL settings to ensure you are not exceeding some range?
> > > >
> > > > What output frequency are you measuring? OUT0 or another one?
> What
> > > > frequency do you measure with en_ref_doubler = 0?
> > > >
> > > > --
> > > > Luca
> > >
> > > Not sure what I did wrong with my earlier testing, but I am now
> > > seeing both
> > parts respond similarly to the doubler bit being set.
> > >
> > > With doubler bit disabled (register 0x10, value 0xa0), I set the
> > > output
> > frequencies to 1, 10, 100, and 46.8MHz.
> > >
> > > After setting doubler bit (0xa8), I saw frequencies of 1.260, 12.60,
> > > 126.0,
> > and 58.9 Mhz for both 6901 and 6965 parts.
> > >
> > > So from my testing the doubler bit seems to behave similarly for
> > > both
> > parts.
> > >
> > > At this point I will leave my unofficial testing and move on to
> > > writing a
> > consumer driver.
> >
> > I don't have a scope to measure the exact frequencies, but I was able
> > to test it with both USB and Ethernet, which are clock from the
> > versaclock, and I can check the output frequencies against the
> clk_summary in debugfs.
> >
> > Without this patch:
> >
> > clock-controller.mux 1 1 0 25000000
> > 0 0 50000 Y
> > clock-controller.out0_sel_i2cb 0 0 0
> > 25000000 0 0 50000 Y
> > clock-controller.pfd 1 1 0 25000000
> > 0 0 50000 Y
> > clock-controller.pll 1 1 0 2800000000
> > 0 0 50000 Y
> > clock-controller.fod3 1 1 0
> > 24576000 0 0 50000 Y
> > clock-controller.out4 1 1 0
> > 24576000 0 0 50000 Y
> > clock-controller.fod2 0 0 0
> > 24000000 0 0 50000 Y
> > clock-controller.out3 0 0 0
> > 24000000 0 0 50000 Y
> > clock-controller.fod1 0 0 0
> > 24000000 0 0 50000 Y
> > clock-controller.out2 0 0 0
> > 24000000 0 0 50000 Y
> > clock-controller.fod0 0 0 0
> > 24000000 0 0 50000 Y
> > clock-controller.out1 0 0 0
> > 24000000 0 0 50000 Y
> >
> >
> > With this patch:
> >
> > clock-controller.mux 1 1 0 25000000
> > 0 0 50000 Y
> > clock-controller.out0_sel_i2cb 0 0 0
> > 25000000 0 0 50000 Y
> > clock-controller.dbl 1 1 0 25000000
> > 0 0 50000 Y
> > clock-controller.pfd 1 1 0 25000000
> > 0 0 50000 Y
> > clock-controller.pll 1 1 0
> > 2800000000 0 0 50000 Y
> > clock-controller.fod3 1 1 0
> > 24576000 0 0 50000 Y
> > clock-controller.out4 1 1 0
> > 24576000 0 0 50000 Y
> > clock-controller.fod2 0 0 0
> > 24000000 0 0 50000 Y
> > clock-controller.out3 0 0 0
> > 24000000 0 0 50000 Y
> > clock-controller.fod1 0 0 0
> > 24000000 0 0 50000 Y
> > clock-controller.out2 0 0 0
> > 24000000 0 0 50000 Y
> > clock-controller.fod0 0 0 0
> > 24000000 0 0 50000 Y
> > clock-controller.out1 0 0 0
> > 24000000 0 0 50000 Y
> >
> > From what I can tell, the only thing that changes is the introduction
> > of clock- controller.dbl into the clock dump.
> > In my interpretation of reading the programmer's manual, the frequency
> > that is doubled is the reference frequency, but based on looking at
> > the clock dump, it's not obvious what's happening.
> >
> > Having said that, if Claude is measuring incorrect frequencies, I am
> > fine with abandoning this patch.
>
> For what it's worth, I went back and tested output frequencies with patch
> removed.
>
> Here is what I observed(starting with frequencies of 1, 10, 100, 46.8MHz and
> changing register 0x10 from 0xa0 to 0xa8).
>
> 6901: 1.260, 12.60, 126.0, 58.9
> 6965 with patch: 1.260, 12.60, 126.0, 58.9
> 6965 without patch: 1.019, 10.19, 101.9, 47.68
>
> From my limited testing (and knowledge for sure) it seems that in this test
> the patch has some benefit.
>
> -Claude
>
I am really sorry but there appears to be something going on with my setup that is causing spurious results. After sending this email I re-enabled the 6965 patch and am seeing same results I had without the patch (1.019, 10.19, 101.9, 47.68 MHz).
Please disregard my earlier email.
-Claude
> >
> > adam
> >
> > >
> > > Regards,
> > > 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.
======================================================================
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.
Powered by blists - more mailing lists