lkml.org 
[lkml]   [2020]   [Oct]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v4 5/7] clk: axi-clkgen: Respect ZYNQMP PFD/VCO frequency limits
On Thu, Oct 1, 2020 at 8:18 AM Alexandru Ardelean
<ardeleanalex@gmail.com> wrote:
>
> On Wed, Sep 30, 2020 at 8:16 PM Moritz Fischer <mdf@kernel.org> wrote:
> >
> > On Wed, Sep 30, 2020 at 08:22:23AM +0300, Alexandru Ardelean wrote:
> > > On Tue, Sep 29, 2020 at 6:30 PM Moritz Fischer <mdf@kernel.org> wrote:
> > > >
> > > > Hi Alexandru,
> > > >
> > > > On Tue, Sep 29, 2020 at 05:44:15PM +0300, Alexandru Ardelean wrote:
> > > > > From: Mathias Tausen <mta@gomspace.com>
> > > > >
> > > > > Since axi-clkgen is now supported on ZYNQMP, make sure the max/min
> > > > > frequencies of the PFD and VCO are respected.
> > > > >
> > > > > Signed-off-by: Mathias Tausen <mta@gomspace.com>
> > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > >
> > > > This patch still does not cover the PCIe Zynq plugged into ZynqMP linux
> > > > machine case.
> > > >
> > > > > ---
> > > > > drivers/clk/clk-axi-clkgen.c | 9 +++++++++
> > > > > 1 file changed, 9 insertions(+)
> > > > >
> > > > > diff --git a/drivers/clk/clk-axi-clkgen.c b/drivers/clk/clk-axi-clkgen.c
> > > > > index 4342b7735590..2319bb1c5c08 100644
> > > > > --- a/drivers/clk/clk-axi-clkgen.c
> > > > > +++ b/drivers/clk/clk-axi-clkgen.c
> > > > > @@ -108,12 +108,21 @@ static uint32_t axi_clkgen_lookup_lock(unsigned int m)
> > > > > return 0x1f1f00fa;
> > > > > }
> > > > >
> > > > > +#ifdef ARCH_ZYNQMP
> > > > > +static const struct axi_clkgen_limits axi_clkgen_default_limits = {
> > > > > + .fpfd_min = 10000,
> > > > > + .fpfd_max = 450000,
> > > > > + .fvco_min = 800000,
> > > > > + .fvco_max = 1600000,
> > > > > +};
> > > > > +#else
> > > > > static const struct axi_clkgen_limits axi_clkgen_default_limits = {
> > > > > .fpfd_min = 10000,
> > > > > .fpfd_max = 300000,
> > > > > .fvco_min = 600000,
> > > > > .fvco_max = 1200000,
> > > > > };
> > > > > +#endif
> > > >
> > > > I still don't understand this. You have a way to determine which fabric
> > > > you are looking at with the FPGA info. Why not:
> > > >
> > > > [..] axi_clkgen_zynqmp_default_limits = {
> > > > };
> > > >
> > > > [..] axi_clkgen_default_limits = {
> > > > };
> > > >
> > > > Set them based on what you read back, i.e. determine which fabric you
> > > > are looking at *per clock gen* and use that info, rather than making a
> > > > compile time decision to support only one of them.
> > > >
> > > > Generally speaking #ifdef $ARCH should be a last resort solution.
> > >
> > > The support for reading back the fabric parameters is implemented in
> > > the AXI CLKGEN PCORE version starting with 5.0.a
> > > Links:
> > > https://github.com/analogdevicesinc/hdl/commits/master/library/common/up_clkgen.v
> > > https://github.com/analogdevicesinc/hdl/commit/66823682b63c1037abdc3fc1dd4d4e63d3cfbc1a
> > > https://github.com/analogdevicesinc/hdl/commit/7dcb2050c7946fab5ea5a273eda7c53ea7b969a6
> > >
> > > Before that version, these details aren't there, so the best you can
> > > do is assume compile-time ARCH defaults.
> >
> > This is a property of the instance and not of the driver. If you can't
> > query the hardware to figure out what you're looking at, but have
> > different behaviours, please use different compatible strings and make
> > the default limits platform data.
> >
> > Something like this:
> >
> > static const struct of_device_id axi_clkgen_ids[] = {
> > {
> > .compatible = "foo-zynqmp",
> > .data = &zynqmp_default_limits,
> > },
> > {
> > .compatible = "bar-zynq",
> > .data = &zynq_default_limits,
> > },
> >
> > { },
> > };
> >
>
> oh, apologies for not thinking about this;
> i'll spin this up

at this point, i also need to do a device-tree binding conversion to yaml;
[which i finished just now];
so, this series has grown a bit more;

i'm thinking of splitting this series now;
i'll send the DT binding conversion first/separately;
then i'll re-send Lars' patches [for fractional dividers] separately;
they don't touch on this ZynqMP discussion;
after that, i'll resume on this part;

mind you: it's not that i am trying to avoid anyone [with this split],
but it will require Rob's review for the binding thing, and that adds
another speed to the review;
so, splitting things, will help with things going at each reviewer's rate;

>
> thanks
> Alex
>
> > And pull the info out in your probe function, then you can have both
> > configurations and select via device-tree.
> >
> > Thanks,
> > Moritz

\
 
 \ /
  Last update: 2020-10-01 10:37    [W:0.090 / U:0.228 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site