Re: [PATCH 3/5] thermal: rockchip: fixes invalid temperature case

[Caesar Wang]

å 2016å11æ23æ 10:33, Brian Norris åé:
> On Wed, Nov 23, 2016 at 10:06:15AM +0800, Caesar Wang wrote:
> > å 2016å11æ23æ 05:52, Brian Norris åé:
> > > On Tue, Nov 22, 2016 at 12:57:37PM -0800, Brian Norris wrote:
> > > > > +	if (temp < table->id[low].temp || temp > table->id[high].temp)
> > > > >   		goto exit;
> > > I was revisiting the logic here though, and I don't understand your
> > > error case. You're treating "too low" and "too high" the same, and in
> > > either case, you're choosing a value of ->data_mask. That doesn't make
> > > sense to me, especially for ADC_DECREMENT cases like rk3288. In that
> > > case, you're programming the trip to the lowest possible temperature.
> > I admit that's not perfect, but that should conform to reality.
> >
> > Whichever is the adc value, 12it or 10bit.
> > #define TSADCV2_DATA_MASK            0xfff
> > #define TSADCV3_DATA_MASK            0x3ff
> >
> > The "too low" and "too high" are same, that should indicate that temperature is
> > invalid or over table range.
> >
> > The currect code will return the max analog value to warn it.
> > ---
> >
> > The temperature {-40C, 125C} is for rockchip SoCs, that should be
> > similar with real world's temperature {-INT_MAX, INT_MAX}.
> IIUC, "too high" should not be interpreted as TSADCV2_DATA_MASK on
> rk3288, should it? That corresponds to -40C, which means you'll be
> triggering the alarm temperature at a very *low* temperature, not a very
> high one, no?

The "too high" will correspond to -40C on rk3288, but shouldn't trigger 
the alarm temperature.

Due to the alarm or tshut function will handle it.

e.g.:
static void rk_tsadcv2_alarm_temp(const struct chip_tsadc_table *table,
                  int chn, void __iomem *regs, int temp)
{
    u32 alarm_value, int_en;

    /* Make sure the value is valid */
    alarm_value = rk_tsadcv2_temp_to_code(table, temp);
    if (alarm_value == table->data_mask)
        return;
....
}
or
static void rk_tsadcv2_tshut_temp(const struct chip_tsadc_table *table,
                  int chn, void __iomem *regs, int temp)
{
    u32 tshut_value, val;

    /* Make sure the value is valid */
    tshut_value = rk_tsadcv2_temp_to_code(table, temp);
    if (tshut_value == table->data_mask)
        return;
...
}

> Brian
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-rockchip