[U-Boot] [PATCH 5/6] arm: socfpga: gen5: add readonly clk driver

[Simon Goldschmidt]

Am 15.08.2019 um 19:07 schrieb Marek Vasut:
> On 8/15/19 6:57 PM, Simon Goldschmidt wrote:
>> Am 15.08.2019 um 18:34 schrieb Marek Vasut:
>>> On 8/15/19 6:22 PM, Simon Goldschmidt wrote:
>>>> Hi Marek,
>>>>
>>>> Am 24.07.2019 um 09:45 schrieb Simon Goldschmidt:
>>>>> On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut <marex at denx.de> wrote:
>>>>>>
>>>>>> On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
>>>>>>> This adds clk-gen5 as a readonly DM_CLK driver that can return
>>>>>>> clocks for
>>>>>>> the peripherals.
>>>>>>>
>>>>>>> Further changes are:
>>>>>>> - select DM_CLK for gen5 U-Boot and SPL
>>>>>>> - add SPL tags to clock nodes in socfpga-common-u-boot.dtsi
>>>>>>> - adjust socfpga.dtsi to provide missing src selection registers
>>>>>>> - start 'handoff.dtsi' file for socrates (conatains clock speeds for
>>>>>>> now)
>>>>>>
>>>>>> These should likely be separate patches then ?
>>>>>
>>>>> Well, in the end, yes. Especially the handoff.dtsi is required for
>>>>> *all*
>>>>> socfpga_gen5 boards, so I'll need to adapt the 'qts-filter.sh'
>>>>> script to
>>>>> generate it.
>>>>>
>>>>> I'll change that script and separate these patches in v2.
>>>>
>>>> I'm in the process of moving all of the 'qts' header files to devicetree
>>>> handoff.dtsi files. CLK and DDR are already working (pinmux/iocsr not
>>>> yet) - but I need to work a bit on DM memory consumption.
>>>>
>>>> So now I'm faced with a question: in which driver do I implement the
>>>> pinmux control? From a DM point of view, it could be a UCLASS_PINCTRL
>>>> driver in 'drivers/pinctrl', but since it's more or less read-only
>>>> unless we'd get more details about the hardware, I'm a bit hesistant to
>>>> do it that way.
>>>>
>>>> Also, the registers are in 'sysmgr', which is handled by the standard
>>>> "syscon" driver right now, so it could well get a UCLASS_SYSCON driver?
>>>
>>> What do we need read-only pinmux driver for ? I would expect pinmux to
>>> be more write-only, from the hardware perspective that is.
>>
>> Well, I don't know. I just need a driver that can parse its dts subtree
>> for the config instead of loading from the qts wrapper file. Then this
>> driver needs to be probed at the appropriate place in SPL so that the
>> pins are initialized.
> 
> Sounds more like misc driver or something along those lines.

Hmm, probing UCLASS_MISC in SPL to get the pinmux initializes sounds a 
bit strange, but that's probably OK.

> 
>> My future plan is then that such a driver could be re-probed after
>> loading some kind of dts overlay matching an FPGA image to be programmed
>> (as that FPGA image can contain different pin settings, e.g. when using
>> loan IO).
> 
> But then it becomes a PINMUX driver.

Well, what I'm writing *is* a writeonly driver controlling the pins. 
However, since we don't know anything about the iocsr part, we can't 
implement all the functions in 'struct pinctrl_ops' (just write the 
given scan chain and that's it). I think PINMUX would fit best, but see 
below...

> 
>> I'm just not familiar enough with pinctrl drivers or syscon drivers and
>> could need some input on which direction to take this...
>>
>> Does a syscon driver for that purpose sound better?
> 
> Isn't syscon driver for system controllers, which provide e.g. regmaps
> to subdevices ? I think the altera pinmux shouldn't be syscon.

The thing is that 'sysmgr' already *is* a syscon driver: it provides 
access to various control bits (e.g. used by the ethernet driver in 
Linux) but also pinmux registers.

Of course, I could add "scanmgr at 0xfff02000" as a new node in the 
devicetree that would be the PINMUX driver which accesses the pinmux 
registers in sysmgr via sysycon... That would keep sysmgr's syscon 
behaviour working (for other drivers).

Regards,
Simon