Quoting Sergio Paracuellos (2020-12-17 01:54:18)
>
> On Thu, Dec 17, 2020 at 10:09 AM Stephen Boyd <sboyd@xxxxxxxxxx> wrote:
> >
> > Quoting Sergio Paracuellos (2020-11-22 01:55:53)
> > > diff --git a/drivers/clk/ralink/Makefile b/drivers/clk/ralink/Makefile
> > > new file mode 100644
> > > index 000000000000..cf6f9216379d
> > > --- /dev/null
> > > +++ b/drivers/clk/ralink/Makefile
> > > @@ -0,0 +1,2 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +obj-$(CONFIG_CLK_MT7621) += clk-mt7621.o
> > > diff --git a/drivers/clk/ralink/clk-mt7621.c b/drivers/clk/ralink/clk-mt7621.c
> > > new file mode 100644
> > > index 000000000000..4e929f13fe7c
> > > --- /dev/null
> > > +++ b/drivers/clk/ralink/clk-mt7621.c
> > > @@ -0,0 +1,435 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Mediatek MT7621 Clock Driver
> > > + * Author: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
> > > + */
> > > +
> > > +#include <linux/bitops.h>
> > > +#include <linux/clk-provider.h>
> > > +#include <linux/mfd/syscon.h>
> > > +#include <linux/module.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/regmap.h>
> > > +#include <asm/mach-ralink/ralink_regs.h>
> >
> > Is it possible to drop this include? Doing so would make this portable
> > and compilable on more architectures so us cross compilers can check
> > build stuff and make changes easily.
>
> No, this is not possible. This old arch makes some global functions
> there to properly access different registers in the palmbus. It is not
> also well documented so it is really difficult to make something
> better with this.
> This is needed to use 'rt_memc_r32'
> (arch/mips/include/asm/mach-ralink/ralink_regs.h) for reading
> MEMC_REG_CPU_PLL.
>
> This is a not documented register and is not in the syscon related
> part and we need it to derive the clock frequency for the XTAL clock.
Ok.
> > > +static int mt7621_gate_ops_init(struct device_node *np,
> > > + struct mt7621_gate *sclk)
> > > +{
> > > + struct clk_init_data init = {
> > > + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> >
> > Why ignore unused? Are they CLK_IS_CRITICAL? Can they be enabled at
> > driver probe instead of here? Or left out of the kernel entirely if they
> > shouldn't be turned off?
>
> Because all the platform drivers are not changed to use this gates yet
> and all gates are enabled by default (related registers are set to all
> ones), kernel disables all the stuff because they are not being
> referenced, but yes, you are right, I think I can call
> clk_prepare_enable for all of them at init time and avoid this
> 'CLK_IGNORE_UNUSED' flag to don't break anything of the current other
> upstream code.
Does something crash if they're turned off? We have CLK_IS_CRITICAL for
that. The CLK_IGNORE_UNUSED flag is sort of deprecated now.
> > > +
> > > +#define CLK_BASE(_name, _parent, _recalc) { \
> > > + .init = &(struct clk_init_data) { \
> > > + .name = _name, \
> > > + .ops = &(const struct clk_ops) { \
> > > + .recalc_rate = _recalc, \
> > > + }, \
> > > + .parent_names = (const char *const[]) { _parent }, \
> >
> > Please use clk_parent_data instead
>
> parent can also be NULL here and num_parents zero, but I will search
> what do you really mean with this 'clk_parent_data' :).
Heh, 'git grep clk_parent_data -- drivers/clk/' should give some clues.
> > > +free_clk_prov:
> > > + kfree(clk_prov);
> > > +}
> > > +
> > > +CLK_OF_DECLARE(mt7621_clk, "mediatek,mt7621-clk", mt7621_clk_init);
> >
> > Any reason to use this vs. a platform driver?
>
> We need clocks available in 'plat_time_init' before setting up the
> timer for the GIC, so to maintain all the clock driver in a simple
> file and using only one device tree node and no separate the gates
> into another platform driver, I think this is the only way to go, but
> please correct me if I am wrong.
We can register the few clks like that early with
CLK_OF_DECLARE_DRIVER() and then have a platform driver register the
rest of the clks that aren't required early. This lets us hook into the
driver framework better while still getting those few clks to turn on
early enough for the timers.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
