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

[David Lechner <david@lechnology.com>]

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@ti.com> 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. :-)