| 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: <aAD6EXrav161J0vS@apocalypse>
Date: Thu, 17 Apr 2025 14:54:41 +0200
From: Andrea della Porta <andrea.porta@...e.com>
To: Dave Stevenson <dave.stevenson@...pberrypi.com>
Cc: Andrea della Porta <andrea.porta@...e.com>,
Stefan Wahren <wahrenst@....net>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Florian Fainelli <florian.fainelli@...adcom.com>,
Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>,
Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczynski <kw@...ux.com>,
Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Linus Walleij <linus.walleij@...aro.org>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>, Bartosz Golaszewski <brgl@...ev.pl>,
Derek Kiernan <derek.kiernan@....com>,
Dragan Cvetic <dragan.cvetic@....com>,
Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Saravana Kannan <saravanak@...gle.com>, linux-clk@...r.kernel.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
linux-gpio@...r.kernel.org, Masahiro Yamada <masahiroy@...nel.org>,
Herve Codina <herve.codina@...tlin.com>,
Luca Ceresoli <luca.ceresoli@...tlin.com>,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
Andrew Lunn <andrew@...n.ch>, Phil Elwell <phil@...pberrypi.com>,
kernel-list@...pberrypi.com
Subject: Re: [PATCH v8 05/13] clk: rp1: Add support for clocks provided by RP1
Hi Dave,
On 11:48 Thu 17 Apr , Dave Stevenson wrote:
> Hi Andrea & Stefan.
>
> On Wed, 16 Apr 2025 at 17:26, Andrea della Porta <andrea.porta@...e.com> wrote:
> >
> > Hi Stefan,
> >
> > On 14:09 Mon 14 Apr , Stefan Wahren wrote:
> > > Hi Andrea,
> > >
> > > Am 19.03.25 um 22:52 schrieb Andrea della Porta:
> > > > RaspberryPi RP1 is an MFD providing, among other peripherals, several
> > > > clock generators and PLLs that drives the sub-peripherals.
> > > > Add the driver to support the clock providers.
> > > >
> > > > Signed-off-by: Andrea della Porta <andrea.porta@...e.com>
> > > > ---
> > > > MAINTAINERS | 5 +
> > > > drivers/clk/Kconfig | 9 +
> > > > drivers/clk/Makefile | 1 +
> > > > drivers/clk/clk-rp1.c | 1512 +++++++++++++++++++++++++++++++++++++++++
> > > > 4 files changed, 1527 insertions(+)
> > > > create mode 100644 drivers/clk/clk-rp1.c
> > > >
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index 896a307fa065..75263700370d 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -19748,6 +19748,11 @@ S: Maintained
> > > > F: Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml
> > > > F: drivers/media/platform/raspberrypi/rp1-cfe/
> > > >
> > > > +RASPBERRY PI RP1 PCI DRIVER
> > > > +M: Andrea della Porta <andrea.porta@...e.com>
> > > > +S: Maintained
> > > > +F: drivers/clk/clk-rp1.c
> > > > +
> > > > RC-CORE / LIRC FRAMEWORK
> > > > M: Sean Young <sean@...s.org>
> > > > L: linux-media@...r.kernel.org
> > > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> > > > index 713573b6c86c..cff90de71409 100644
> > > > --- a/drivers/clk/Kconfig
> > > > +++ b/drivers/clk/Kconfig
> > > > @@ -88,6 +88,15 @@ config COMMON_CLK_RK808
> > > > These multi-function devices have two fixed-rate oscillators, clocked at 32KHz each.
> > > > Clkout1 is always on, Clkout2 can off by control register.
> > > >
> > > > +config COMMON_CLK_RP1
> > > > + tristate "Raspberry Pi RP1-based clock support"
> > > > + depends on MISC_RP1 || COMPILE_TEST
> > > > + default MISC_RP1
> > > > + help
> > > > + Enable common clock framework support for Raspberry Pi RP1.
> > > > + This multi-function device has 3 main PLLs and several clock
> > > > + generators to drive the internal sub-peripherals.
> > > > +
> > > > config COMMON_CLK_HI655X
> > > > tristate "Clock driver for Hi655x" if EXPERT
> > > > depends on (MFD_HI655X_PMIC || COMPILE_TEST)
> > > > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> > > > index bf4bd45adc3a..ff3993ed7e09 100644
> > > > --- a/drivers/clk/Makefile
> > > > +++ b/drivers/clk/Makefile
> > > > @@ -84,6 +84,7 @@ obj-$(CONFIG_CLK_LS1028A_PLLDIG) += clk-plldig.o
> > > > obj-$(CONFIG_COMMON_CLK_PWM) += clk-pwm.o
> > > > obj-$(CONFIG_CLK_QORIQ) += clk-qoriq.o
> > > > obj-$(CONFIG_COMMON_CLK_RK808) += clk-rk808.o
> > > > +obj-$(CONFIG_COMMON_CLK_RP1) += clk-rp1.o
> > > > obj-$(CONFIG_COMMON_CLK_HI655X) += clk-hi655x.o
> > > > obj-$(CONFIG_COMMON_CLK_S2MPS11) += clk-s2mps11.o
> > > > obj-$(CONFIG_COMMON_CLK_SCMI) += clk-scmi.o
> > > > diff --git a/drivers/clk/clk-rp1.c b/drivers/clk/clk-rp1.c
> > > > new file mode 100644
> > > > index 000000000000..72c74e344c1d
> > > > --- /dev/null
> > > > +++ b/drivers/clk/clk-rp1.c
> > > > @@ -0,0 +1,1512 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > ...
> > > > +
> > > > +static int rp1_pll_divider_set_rate(struct clk_hw *hw,
> > > > + unsigned long rate,
> > > > + unsigned long parent_rate)
> > > > +{
> > > > + struct rp1_clk_desc *divider = container_of(hw, struct rp1_clk_desc, div.hw);
> > > > + struct rp1_clockman *clockman = divider->clockman;
> > > > + const struct rp1_pll_data *data = divider->data;
> > > > + u32 div, sec;
> > > > +
> > > > + div = DIV_ROUND_UP_ULL(parent_rate, rate);
> > > > + div = clamp(div, 8u, 19u);
> > > > +
> > > > + spin_lock(&clockman->regs_lock);
> > > > + sec = clockman_read(clockman, data->ctrl_reg);
> > > > + sec &= ~PLL_SEC_DIV_MASK;
> > > > + sec |= FIELD_PREP(PLL_SEC_DIV_MASK, div);
> > > > +
> > > > + /* Must keep the divider in reset to change the value. */
> > > > + sec |= PLL_SEC_RST;
> > > > + clockman_write(clockman, data->ctrl_reg, sec);
> > > > +
> > > > + /* TODO: must sleep 10 pll vco cycles */
> > > Is it possible to implement this with some kind of xsleep or xdelay?
> >
> > I guess so... unless anyone knows a better method such as checking
> > for some undocumented register flag which reveals when the clock is stable
> > so it can be enabled (Phil, Dave, please feel free to step in with advice
> > if you have any), I think this line could solve the issue:
> >
> > ndelay (10 * div * NSEC_PER_SEC / parent_rate);
>
> I've checked with those involved in the hardware side.
> There's no hardware flag that the clock is stable, so the ndelay is
> probably the best option. The VCO can go as low as 600MHz, so the max
> delay would be 166ns.
Perfect, I'll use ndelay then. However, shouldn't this be 16ns max?
I think this formula should give a good estimate:
10ULL * div * NSEC_PER_SEC / parent_rate
and has the advantage of not depending on hard coded values.
>
> Thanks for your continuing work on this.
Thank you for checking.
Regards,
Andrea
>
> Dave
>
> > Many thanks,
> > Andrea
> >
> > > > + sec &= ~PLL_SEC_RST;
> > > > + clockman_write(clockman, data->ctrl_reg, sec);
> > > > + spin_unlock(&clockman->regs_lock);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > >
Powered by blists - more mailing lists