Re: [PATCH 1/6] mfd: syscon: Add syscon_register() function

[David Lechner]

On 01/22/2018 05:39 AM, Sekhar Nori wrote:
> On Monday 22 January 2018 04:38 PM, Arnd Bergmann wrote:
> > On Mon, Jan 22, 2018 at 11:53 AM, Sekhar Nori <nsekhar@xxxxxx> wrote:
> > > On Saturday 20 January 2018 08:50 AM, David Lechner wrote:
> > > > This adds a new syscon_register() function that creates a new syscon
> > > > regmap and adds it to the lookup list.
> > >
> > > /*
> > >   * registers a "global" syscon "device", usually not backed by a real
> > >   * device. To be used only in cases where the syscon is not
> > >   * related to any actual device, like system clocks, for example.
> > >   *
> > >   * name passed here must be globally unique.
> > >   */
> > > struct regmap *syscon_register_by_name(resource_size_t start, size_t
> > >                                         size, const char *name);
> > >
> > > and
> > >
> > > struct regmap *syscon_regmap_lookup_by_name(const char *name);
> > >
> > > I understand, with this the driver becomes little more complicated and
> > > DT and non-DT cases still need to be handled differently. But I think
> > > thats not the main issue you are trying to solve.
> >
> > I think the easiest way to handle this in traditional board files is to use
> > a platform_data structure: when the platform registers the syscon,
> > it holds a pointer to the regmap and can simply add pass it to any
> > device using the syscon through platform_data. The driver using it
> > then does:
> >
> > if (dev->of_node) {
> >        priv->regmap = syscon_regmap_lookup_by_phandle(...);
> > else
> >        priv->regmap = pdata->regmap;
>
> I think this works as well and does not need any new API. For clocks,
> there is no platform data AFAICT, so it will have to be whatever
> structure is being used to pass clock info to CCF driver (like perhaps
> an addition parameter to da850_pll_clk_init() in David's under-review
> DaVinci CCF conversion series).
>
> > At some point, we had a syscon_regmap_lookup_by_pdevname()
> > that was introduced for a similar case, but we should just kill that
> > now, after the platform that needed it is DT-only.
>
> Yeah, I just noticed that too. syscon_regmap_lookup_by_pdevname has only
> two callers in kernel. The call from clps711x.c UART driver does not
> seem to be needed anymore after clps711x was converted to DT-only in v4.8
>
> The other user is phy-da8xx-usb.c which this series is trying to get rid
> of. So, perhaps syscon_regmap_lookup_by_pdevname() should be dropped by
> v2 of this series.

Sounds like a plan.

Sekhar, would you please have a look at [PATCH 4/6] "ARM: davinci: move
davinci_clk_init() to init_time" I've sent this 5 times already as part
of the common clock series. If it looks good to you, it would be nice if
you could pick it up so I don't have to keep resending it. :-)