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

[Marek Vasut]

On 8/15/19 7:42 PM, Simon Goldschmidt wrote:
> 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.

Well it's kinda pinmux, but not really. It's not like you can specify,
in DT, how to program a pin or a range of pins either. But maybe PINCTRL
uclass with some really rudimentary driver is OK.

I am not sure myself to be honest.

>>> 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...

Go for it then :)

>>> 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.

Maybe you can have a pinctrl driver that is a consumer of the syscon
interface ?

> 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
> 


-- 
Best regards,
Marek Vasut