rk_tsadc breaks (my) Rock64

[Ganbold Tsagaankhuu]

On Tue, Dec 3, 2019 at 6:14 PM Peter Jeremy <peter at rulingia.com> wrote:

> On 2019-Dec-02 15:45:48 +0100, Emmanuel Vadot <manu at bidouilliste.com>
> wrote:
> >> >> 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.
>
> Yes.  Looking more closely, I was wrong.  There are 2 GRF devices in
> the RK3328 that probe/attach separately.
>
> > 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).
>
> I don't understand how the SYSCON_WRITE_4() operations can work when
> the offsets are outside the memory range allocated to the device.
>
> The other oddity I have is that Rockchip_RK3328TRM_V1.1-Part1-20170321.pdf
> Table 9-1 (the Temperature-to-code mapping table) is identical to
> rk3288_calib_data and looks completely different to rk3328_calib_data.
> (Looking more closely, the values in the two tables sum to 4096).
>
> I've tried the following, slightly more extensive patch and now get
> temperature readings that aren't insane (~40°C idle and ~52°C under
> heavy FPU load, at 1200MHz with a small heatsink and active airflow).
> I haven't worked through the TRM in detail to see if there's anything
> else missing.
>

I confirm this patch works for me on NanoPI R2S board. Without this patch
it panics in same way.

Ganbold



>
> Index: rk_tsadc.c
> ===================================================================
> --- rk_tsadc.c  (revision 355286)
> +++ rk_tsadc.c  (working copy)
> @@ -98,11 +98,6 @@
>         int                     channel;
>  };
>
> -enum tsadc_type {
> -       RK_TSADC_V2,
> -       RK_TSADC_V3
> -};
> -
>  struct rk_calib_entry {
>         uint32_t        raw;
>         int             temp;
> @@ -109,13 +104,14 @@
>  };
>
>  struct tsadc_calib_info {
> -       bool decrement_mode;
> +    //bool decrement_mode;
>         struct rk_calib_entry   *table;
>         int                     nentries;
>  };
>
>  struct tsadc_conf {
> -       enum tsadc_type         type;
> +       int                     use_syscon;
> +       int                     q_sel_ntc;
>         int                     shutdown_temp;
>         int                     shutdown_mode;
>         int                     shutdown_pol;
> @@ -188,7 +184,8 @@
>  };
>
>  struct tsadc_conf rk3288_tsadc_conf = {
> -       .type =                 RK_TSADC_V2,
> +       .use_syscon=            0,
> +       .q_sel_ntc=             0,
>         .shutdown_temp =        95000,
>         .shutdown_mode =        1, /* GPIO */
>         .shutdown_pol =         0, /* Low  */
> @@ -241,7 +238,8 @@
>  };
>
>  static struct tsadc_conf rk3328_tsadc_conf = {
> -       .type =                 RK_TSADC_V3,
> +       .use_syscon=            0,
> +       .q_sel_ntc=             1,
>         .shutdown_temp =        95000,
>         .shutdown_mode =        0, /* CRU */
>         .shutdown_pol =         0, /* Low  */
> @@ -296,7 +294,8 @@
>  };
>
>  static struct tsadc_conf rk3399_tsadc_conf = {
> -       .type =                 RK_TSADC_V3,
> +       .use_syscon=            1,
> +       .q_sel_ntc=             1,
>         .shutdown_temp =        95000,
>         .shutdown_mode =        1, /* GPIO */
>         .shutdown_pol =         0, /* Low  */
> @@ -444,11 +443,11 @@
>                 val |= TSADC_AUTO_CON_POL_HI;
>         else
>                 val &= ~TSADC_AUTO_CON_POL_HI;
> -       if (sc->conf->type == RK_TSADC_V3)
> +       if (sc->conf->q_sel_ntc)
>                 val |= TSADC_AUTO_Q_SEL;
>         WR4(sc, TSADC_AUTO_CON, val);
>
> -       if (sc->conf->type == RK_TSADC_V2) {
> +       if (!sc->conf->use_syscon) {
>                 /* V2 init */
>                 WR4(sc, TSADC_AUTO_PERIOD, 250);        /* 250 ms */
>                 WR4(sc, TSADC_AUTO_PERIOD_HT, 50);      /*  50 ms */
>
> --
> Peter Jeremy
>