rk_tsadc breaks (my) Rock64

[Emmanuel Vadot]

On Mon, 2 Dec 2019 14:53:07 +0100
Michal Meloun <meloun.michal at gmail.com> wrote:

> 
> 
> On 02.12.2019 14:04, Emmanuel Vadot wrote:
> > On Mon, 2 Dec 2019 22:13:22 +1100
> > Peter Jeremy <peter at rulingia.com> wrote:
> > 
> >> On 2019-Dec-01 22:07:16 +1100, Peter Jeremy <peter at rulingia.com> wrote:
> >>> r355173 added code to read the Rockchip temperature sensors.  Unfortunately,
> >>> this breaks on my Rock64.  I've tried to understand what's going wrong but
> >>> haven't managed to make much headway.  It looks like there some configuration
> >>> missing from syscon that tsadc needs but I'm not sure what (and I don't really
> >>> understand what syscon is doing).  I'd appreciate any insights.
> >>
> >> I've added a pile of printf's and done some more digging and have made some
> >> progress.
> >>
> >> Firstly, I've found that the syscon at ff100000 FDT entry attaches as two
> >> distinct devices:
> >> rk_grf0: <RockChip General Register Files> mem 0xff100000-0xff100fff on ofwbus0
> >>   (via compatible = "rockchip,rk3328-grf")
> >> simple_mfd0: <Simple MFD (Multi-Functions Device)> mem 0xff450000-0xff45ffff on ofwbus0
> >>   (via compatible = "simple-mfd")
> > 
> >  ??? those aren't the same devices.
> > 
> >> Based on the traceback going via simple_mfd_syscon_write_4(), I had assumed
> >> that tsadc_attach() was using the latter device but when I added code to
> >> print structure addresses, I discovered it was the former.
> >>
> >> This makes the problem clearer: rk_grf0 requests and is allocated 4KiB
> >> memory ("reg = <0x0 0xff100000 0x0 0x1000>;" in the FDT and
> >> "mem 0xff100000-0xff100fff" in the device attach message above) but the
> >> tsadc_init() code is doing:
> >>                         SYSCON_WRITE_4(sc->grf, GRF_TSADC_TESTBIT_L,
> >>                             GRF_TSADC_VCM_EN_L);
> >> with
> >> #define GRF_TSADC_TESTBIT_L                     0x0e648
> >> and that offset is well outside the 4KiB allocated to the device.
> >> (On the positive side, a panic makes the problem a lot clearer than
> >> writing to a random device location would have been).
> >>
> >> Ganbold's followup shows that the RK3399 allocates 64KiB to the syscon
> >> device so the equivalent write is valid on a RK3399.
> >>
> >> I suspect the problem is that the following defines are only valid for
> >> the RK3399 since I can't find any matches to either the names or offsets
> >> in the following:
> >> #define GRF_SARADC_TESTBIT                      0x0e644
> >> #define GRF_TSADC_TESTBIT_L                     0x0e648
> >> #define GRF_TSADC_TESTBIT_H                     0x0e64c
> >>
> >> Unfortunately, that also means I currently have no idea what the RK3328
> >> equivalents to those offsets are.
> >>
> >> -- 
> >> Peter Jeremy
> > 
> > root at rock64:~ # uname -a
> > FreeBSD rock64.home.blih.net 13.0-CURRENT FreeBSD 13.0-CURRENT #13 r355188:355190M: Thu Nov 28 21:43:00 CET 2019     manu at skull.home.blih.net:/usr/home/manu/Work/freebsd/obj/usr/home/manu/Work/freebsd/freebsd-svn/base/head/arm64.aarch64/sys/GENERIC  arm64
> > root at rock64:~ # sysctl hw.temperature
> > hw.temperature.CPU: 50.7C
> > 
> >  Everything is working for me after I've added the clocks, so I don't
> > know what's happening for you. But after looking again at the docs it
> > seems that rk3328 is a mix between v2 and v3 so the GRF write are wrong.
> >  I'll have a look today or tomorow and fix this.
> > 
> 
> Peter,
> can you please try following trivial patch?
> Michal
> 
 
 This isn't enough, RK3328 initialization is V2 but needs the
AUTO_Q_SEL bit too (like v3). There might be other stuff that I
haven't found yet.
 That still doesn't explain why it's working for me (both DEBUG and
NODEBUG kernels).

-- 
Emmanuel Vadot <manu at bidouilliste.com>