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

[Simon Goldschmidt]

Marek Vasut <marex at denx.de> schrieb am Fr., 16. Aug. 2019, 13:09:

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

Right, I'll add such a pinctrl as a driver for the scanmgr (which is not
yet present in the devicetree) which will use the sysmgr pins via syscon.

Thanks for sharing your thoughts on this!

Regards,
Simon


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