Messages in this thread | | | From | Martin Blumenstingl <> | Date | Tue, 1 Oct 2019 20:53:59 +0200 | Subject | Re: [PATCH 2/6] clk: meson: add a driver for the Meson8/8b/8m2 DDR clock controller |
| |
Hi Jerome,
thank you for taking the time to go through this!
On Tue, Oct 1, 2019 at 3:29 PM Jerome Brunet <jbrunet@baylibre.com> wrote: [...] > > +#include <linux/platform_device.h> > > +#include <linux/mfd/syscon.h> > > syscon is not used in the driver oops, good catch - thank you
[...] > > +static struct clk_hw_onecell_data meson8_ddr_clk_hw_onecell_data = { > > + .hws = { > > + [DDR_CLKID_DDR_PLL_DCO] = &meson8_ddr_pll_dco.hw, > > + [DDR_CLKID_DDR_PLL] = &meson8_ddr_pll.hw, > > I wonder if onecell is not overkill for this driver. We won't expose the > DCO, so only the post divider remains > > Do you expect this provider to have more than one leaf clock ? > If not, maybe you could use of_clk_hw_simple_get() instead ? there's some more clock bits in DDR_CLK_CNTL - the public A311D datasheet has a description for these bits but I'm not sure they do the same on Meson8/Meson8b/Meson8m2 all I know is that some magic bytes are written to DDR_CLK_CNTL in the old u-boot code
that's why I don't want to make any assumptions and play safe here (by using a onecell clock provider)
> > + }, > > + .num = 2, > > +}; > > + > > +static struct clk_regmap *const meson8_ddr_clk_regmaps[] = { > > + &meson8_ddr_pll_dco, > > + &meson8_ddr_pll, > > +}; > > + > > +static const struct regmap_config meson8_ddr_clkc_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 32, > > + .reg_stride = 4, > > + .fast_io = true, > > I think fast_io will default to true with memory based register. > Setting the max_register would be nice good point - I'll take care of this when sending v2
Martin
| |