| Message ID | 20180830154518.29507-22-embed3d@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IIO-based thermal sensor driver for Allwinner H3 and A83T SoC | expand |
Hello, On Thu, Aug 30, 2018 at 05:45:09PM +0200, Philipp Rossak wrote: > This patch adds support for the H3 ths sensor. > > The H3 supports interrupts. The interrupt is configured to update the > the sensor values every second. The calibration data is writen at the > begin of the init process. > > Signed-off-by: Philipp Rossak <embed3d@gmail.com> > --- > drivers/iio/adc/sun4i-gpadc-iio.c | 91 +++++++++++++++++++++++++++++++++++++ > include/linux/iio/adc/sun4i-gpadc.h | 18 ++++++++ > 2 files changed, 109 insertions(+) > > diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c > index c7b46c82e3e5..d5c7971b2558 100644 > --- a/drivers/iio/adc/sun4i-gpadc-iio.c > +++ b/drivers/iio/adc/sun4i-gpadc-iio.c > @@ -72,6 +72,7 @@ struct gpadc_data { > u32 temp_data_base; > int sensor_count; > bool supports_nvmem; > + u32 ths_irq_clear; > }; > > static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id); > @@ -79,6 +80,10 @@ static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id); > static int sun4i_ths_resume(struct sun4i_gpadc_iio *info); > static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info); > > +static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info); > +static int sun8i_h3_ths_suspend(struct sun4i_gpadc_iio *info); > +static irqreturn_t sunx8i_h3_irq_thread(int irq, void *data); > + > static const struct gpadc_data sun4i_gpadc_data = { > .temp_offset = -1932, > .temp_scale = 133, > @@ -137,6 +142,22 @@ static const struct gpadc_data sun8i_a33_gpadc_data = { > .sensor_count = 1, > }; > > +static const struct gpadc_data sun8i_h3_ths_data = { > + .temp_offset = -1791, > + .temp_scale = -121, > + .temp_data_base = SUN8I_H3_THS_TDATA0, > + .ths_irq_thread = sunx8i_h3_irq_thread, > + .support_irq = true, > + .has_bus_clk = true, > + .has_bus_rst = true, > + .has_mod_clk = true, > + .sensor_count = 1, > + .supports_nvmem = true, > + .ths_resume = sun8i_h3_ths_resume, > + .ths_suspend = sun8i_h3_ths_suspend, > + .ths_irq_clear = SUN8I_H3_THS_INTS_TDATA_IRQ_0, > +}; > + > struct sun4i_sensor_tzd { > struct sun4i_gpadc_iio *info; > struct thermal_zone_device *tzd; > @@ -409,6 +430,31 @@ static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id) > return IRQ_HANDLED; > } > > +static irqreturn_t sunx8i_h3_irq_thread(int irq, void *data) > +{ > + struct sun4i_gpadc_iio *info = data; > + int i; > + > + regmap_write(info->regmap, SUN8I_H3_THS_STAT, > + info->data->ths_irq_clear); > + > + for (i = 0; i < info->data->sensor_count; i++) > + thermal_zone_device_update(info->tzds[i].tzd, > + THERMAL_EVENT_TEMP_SAMPLE); > + > + return IRQ_HANDLED; > +} > + > +static int sun8i_h3_calibrate(struct sun4i_gpadc_iio *info) > +{ > +// regmap_write(info->regmap, SUNXI_THS_CDATA_0_1, > +// info->calibration_data[0]); > +// regmap_write(info->regmap, SUNXI_THS_CDATA_2_3, > +// info->calibration_data[1]); This should probably be implemented, or left out completely. regards, o. > + return 0; > +} > + > static int sun4i_gpadc_runtime_suspend(struct device *dev) > { > struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev)); > @@ -428,6 +474,16 @@ static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info) > return 0; > } > > +static int sun8i_h3_ths_suspend(struct sun4i_gpadc_iio *info) > +{ > + /* Disable ths interrupt */ > + regmap_write(info->regmap, SUN8I_H3_THS_INTC, 0x0); > + /* Disable temperature sensor */ > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, 0x0); > + > + return 0; > +} > + > static int sun4i_gpadc_runtime_resume(struct device *dev) > { > struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev)); > @@ -454,6 +510,37 @@ static int sun4i_ths_resume(struct sun4i_gpadc_iio *info) > return 0; > } > > +static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info) > +{ > + u32 value; > + > + sun8i_h3_calibrate(info); > + > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL0, > + SUN4I_GPADC_CTRL0_T_ACQ(0xff)); > + > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, > + SUN8I_H3_THS_ACQ1(0x3f)); > + > + regmap_write(info->regmap, SUN8I_H3_THS_STAT, > + SUN8I_H3_THS_INTS_TDATA_IRQ_0); > + > + regmap_write(info->regmap, SUN8I_H3_THS_FILTER, > + SUN4I_GPADC_CTRL3_FILTER_EN | > + SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2)); > + > + regmap_write(info->regmap, SUN8I_H3_THS_INTC, > + SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 | > + SUN8I_H3_THS_TEMP_PERIOD(0x55)); > + > + regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value); > + > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, > + SUN8I_H3_THS_TEMP_SENSE_EN0 | value); > + > + return 0; > +} > + > static int sun4i_gpadc_get_temp(void *data, int *temp) > { > struct sun4i_sensor_tzd *tzd = data; > @@ -497,6 +584,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = { > .compatible = "allwinner,sun6i-a31-gpadc", > .data = &sun6i_gpadc_data > }, > + { > + .compatible = "allwinner,sun8i-h3-ths", > + .data = &sun8i_h3_ths_data, > + }, > { /* sentinel */ } > }; > > diff --git a/include/linux/iio/adc/sun4i-gpadc.h b/include/linux/iio/adc/sun4i-gpadc.h > index 99feeba28105..169b4de9a34d 100644 > --- a/include/linux/iio/adc/sun4i-gpadc.h > +++ b/include/linux/iio/adc/sun4i-gpadc.h > @@ -100,6 +100,24 @@ > } > > /* SUNXI_THS COMMON REGISTERS + DEFINES */ > +#define SUN8I_H3_THS_CTRL0 0x00 > + > +#define SUN8I_H3_THS_CTRL2 0x40 > +#define SUN8I_H3_THS_ACQ1(x) (GENMASK(31, 16) & ((x) << 16)) > +#define SUN8I_H3_THS_TEMP_SENSE_EN0 BIT(0) > + > +#define SUN8I_H3_THS_INTC 0x44 > +#define SUN8I_H3_THS_TEMP_PERIOD(x) (GENMASK(31, 12) & ((x) << 12)) > +#define SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 BIT(8) > + > +#define SUN8I_H3_THS_STAT 0x48 > +#define SUN8I_H3_THS_INTS_TDATA_IRQ_0 BIT(8) > + > +#define SUN8I_H3_THS_FILTER 0x70 > +#define SUNXI_THS_CDATA_0_1 0x74 > +#define SUNXI_THS_CDATA_2_3 0x78 > +#define SUN8I_H3_THS_TDATA0 0x80 > + > #define MAX_SENSOR_COUNT 4 > > #endif > -- > 2.11.0 > > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout.
On 30.08.2018 18:27, Ondřej Jirman wrote: >> +static int sun8i_h3_calibrate(struct sun4i_gpadc_iio *info) >> +{ >> +// regmap_write(info->regmap, SUNXI_THS_CDATA_0_1, >> +// info->calibration_data[0]); >> +// regmap_write(info->regmap, SUNXI_THS_CDATA_2_3, >> +// info->calibration_data[1]); > This should probably be implemented, or left out completely. > > regards, > o. > Thanks you are right! This should be implemented! I will fix this in the next version! Thanks, Philipp
On 30.08.2018 22:00, Philipp Rossak wrote: > On 30.08.2018 18:27, Ondřej Jirman wrote: >>> +static int sun8i_h3_calibrate(struct sun4i_gpadc_iio *info) >>> +{ >>> +// regmap_write(info->regmap, SUNXI_THS_CDATA_0_1, >>> +// info->calibration_data[0]); >>> +// regmap_write(info->regmap, SUNXI_THS_CDATA_2_3, >>> +// info->calibration_data[1]); >> This should probably be implemented, or left out completely. >> >> regards, >> o. >> > Thanks you are right! > This should be implemented! I will fix this in the next version! > > Thanks, > Philipp I just realized this function need to check if calibration datas are available. Writing zeros to the calibration data regs "breaks" the thermal sensor. Philipp
On Thu, Aug 30, 2018 at 05:45:09PM +0200, Philipp Rossak wrote: > This patch adds support for the H3 ths sensor. > > The H3 supports interrupts. The interrupt is configured to update the > the sensor values every second. The calibration data is writen at the > begin of the init process. > > Signed-off-by: Philipp Rossak <embed3d@gmail.com> > --- > drivers/iio/adc/sun4i-gpadc-iio.c | 91 +++++++++++++++++++++++++++++++++++++ > include/linux/iio/adc/sun4i-gpadc.h | 18 ++++++++ > 2 files changed, 109 insertions(+) > > diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c > index c7b46c82e3e5..d5c7971b2558 100644 > --- a/drivers/iio/adc/sun4i-gpadc-iio.c > +++ b/drivers/iio/adc/sun4i-gpadc-iio.c > @@ -72,6 +72,7 @@ struct gpadc_data { > u32 temp_data_base; > int sensor_count; > bool supports_nvmem; > + u32 ths_irq_clear; > }; > > static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id); > @@ -79,6 +80,10 @@ static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id); > static int sun4i_ths_resume(struct sun4i_gpadc_iio *info); > static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info); > > +static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info); > +static int sun8i_h3_ths_suspend(struct sun4i_gpadc_iio *info); > +static irqreturn_t sunx8i_h3_irq_thread(int irq, void *data); > + > static const struct gpadc_data sun4i_gpadc_data = { > .temp_offset = -1932, > .temp_scale = 133, > @@ -137,6 +142,22 @@ static const struct gpadc_data sun8i_a33_gpadc_data = { > .sensor_count = 1, > }; > > +static const struct gpadc_data sun8i_h3_ths_data = { > + .temp_offset = -1791, > + .temp_scale = -121, > + .temp_data_base = SUN8I_H3_THS_TDATA0, > + .ths_irq_thread = sunx8i_h3_irq_thread, > + .support_irq = true, > + .has_bus_clk = true, > + .has_bus_rst = true, > + .has_mod_clk = true, > + .sensor_count = 1, > + .supports_nvmem = true, > + .ths_resume = sun8i_h3_ths_resume, > + .ths_suspend = sun8i_h3_ths_suspend, > + .ths_irq_clear = SUN8I_H3_THS_INTS_TDATA_IRQ_0, > +}; > + > struct sun4i_sensor_tzd { > struct sun4i_gpadc_iio *info; > struct thermal_zone_device *tzd; > @@ -409,6 +430,31 @@ static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id) > return IRQ_HANDLED; > } > > +static irqreturn_t sunx8i_h3_irq_thread(int irq, void *data) > +{ > + struct sun4i_gpadc_iio *info = data; > + int i; > + > + regmap_write(info->regmap, SUN8I_H3_THS_STAT, > + info->data->ths_irq_clear); > + > + for (i = 0; i < info->data->sensor_count; i++) > + thermal_zone_device_update(info->tzds[i].tzd, > + THERMAL_EVENT_TEMP_SAMPLE); > + > + return IRQ_HANDLED; > +} > + > +static int sun8i_h3_calibrate(struct sun4i_gpadc_iio *info) > +{ > +// regmap_write(info->regmap, SUNXI_THS_CDATA_0_1, > +// info->calibration_data[0]); > +// regmap_write(info->regmap, SUNXI_THS_CDATA_2_3, > +// info->calibration_data[1]); Don't put commented code. > + > + return 0; > +} > + > static int sun4i_gpadc_runtime_suspend(struct device *dev) > { > struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev)); > @@ -428,6 +474,16 @@ static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info) > return 0; > } > > +static int sun8i_h3_ths_suspend(struct sun4i_gpadc_iio *info) > +{ > + /* Disable ths interrupt */ > + regmap_write(info->regmap, SUN8I_H3_THS_INTC, 0x0); > + /* Disable temperature sensor */ > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, 0x0); > + > + return 0; > +} > + > static int sun4i_gpadc_runtime_resume(struct device *dev) > { > struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev)); > @@ -454,6 +510,37 @@ static int sun4i_ths_resume(struct sun4i_gpadc_iio *info) > return 0; > } > > +static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info) > +{ > + u32 value; > + > + sun8i_h3_calibrate(info); > + > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL0, > + SUN4I_GPADC_CTRL0_T_ACQ(0xff)); > + > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, > + SUN8I_H3_THS_ACQ1(0x3f)); > + > + regmap_write(info->regmap, SUN8I_H3_THS_STAT, > + SUN8I_H3_THS_INTS_TDATA_IRQ_0); > + > + regmap_write(info->regmap, SUN8I_H3_THS_FILTER, > + SUN4I_GPADC_CTRL3_FILTER_EN | > + SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2)); > + > + regmap_write(info->regmap, SUN8I_H3_THS_INTC, > + SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 | > + SUN8I_H3_THS_TEMP_PERIOD(0x55)); > + > + regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value); > + > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, > + SUN8I_H3_THS_TEMP_SENSE_EN0 | value); Ideally, all these values should have a comment explaining what they are. And we really start to have a lot of registers defines. We'd be better off using regmap_fields.
在 2018-08-31五的 11:11 +0200，Maxime Ripard写道： > On Thu, Aug 30, 2018 at 05:45:09PM +0200, Philipp Rossak wrote: > > This patch adds support for the H3 ths sensor. > > > > The H3 supports interrupts. The interrupt is configured to update > > the > > the sensor values every second. The calibration data is writen at > > the > > begin of the init process. > > > > Signed-off-by: Philipp Rossak <embed3d@gmail.com> > > --- > > drivers/iio/adc/sun4i-gpadc-iio.c | 91 > > +++++++++++++++++++++++++++++++++++++ > > include/linux/iio/adc/sun4i-gpadc.h | 18 ++++++++ > > 2 files changed, 109 insertions(+) > > > > diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c > > b/drivers/iio/adc/sun4i-gpadc-iio.c > > index c7b46c82e3e5..d5c7971b2558 100644 > > --- a/drivers/iio/adc/sun4i-gpadc-iio.c > > +++ b/drivers/iio/adc/sun4i-gpadc-iio.c > > @@ -72,6 +72,7 @@ struct gpadc_data { > > u32 temp_data_base; > > int sensor_count; > > bool supports_nvmem; > > + u32 ths_irq_clear; > > }; > > > > static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void > > *dev_id); > > @@ -79,6 +80,10 @@ static irqreturn_t > > sun4i_gpadc_data_irq_handler(int irq, void *dev_id); > > static int sun4i_ths_resume(struct sun4i_gpadc_iio *info); > > static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info); > > > > +static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info); > > +static int sun8i_h3_ths_suspend(struct sun4i_gpadc_iio *info); > > +static irqreturn_t sunx8i_h3_irq_thread(int irq, void *data); > > + > > static const struct gpadc_data sun4i_gpadc_data = { > > .temp_offset = -1932, > > .temp_scale = 133, > > @@ -137,6 +142,22 @@ static const struct gpadc_data > > sun8i_a33_gpadc_data = { > > .sensor_count = 1, > > }; > > > > +static const struct gpadc_data sun8i_h3_ths_data = { > > + .temp_offset = -1791, > > + .temp_scale = -121, > > + .temp_data_base = SUN8I_H3_THS_TDATA0, > > + .ths_irq_thread = sunx8i_h3_irq_thread, > > + .support_irq = true, > > + .has_bus_clk = true, > > + .has_bus_rst = true, > > + .has_mod_clk = true, > > + .sensor_count = 1, > > + .supports_nvmem = true, > > + .ths_resume = sun8i_h3_ths_resume, > > + .ths_suspend = sun8i_h3_ths_suspend, > > + .ths_irq_clear = SUN8I_H3_THS_INTS_TDATA_IRQ_0, > > +}; > > + > > struct sun4i_sensor_tzd { > > struct sun4i_gpadc_iio *info; > > struct thermal_zone_device *tzd; > > @@ -409,6 +430,31 @@ static irqreturn_t > > sun4i_gpadc_data_irq_handler(int irq, void *dev_id) > > return IRQ_HANDLED; > > } > > > > +static irqreturn_t sunx8i_h3_irq_thread(int irq, void *data) > > +{ > > + struct sun4i_gpadc_iio *info = data; > > + int i; > > + > > + regmap_write(info->regmap, SUN8I_H3_THS_STAT, > > + info->data->ths_irq_clear); > > + > > + for (i = 0; i < info->data->sensor_count; i++) > > + thermal_zone_device_update(info->tzds[i].tzd, > > + THERMAL_EVENT_TEMP_SAMP > > LE); > > + > > + return IRQ_HANDLED; > > +} > > + > > +static int sun8i_h3_calibrate(struct sun4i_gpadc_iio *info) > > +{ > > +// regmap_write(info->regmap, SUNXI_THS_CDATA_0_1, > > +// info->calibration_data[0]); > > +// regmap_write(info->regmap, SUNXI_THS_CDATA_2_3, > > +// info->calibration_data[1]); > > Don't put commented code. Personally I suggest to leave out all SID or calibration related patches here. Currently we seems to be wrongly converting SID to big endian, however, the orgnization of the THS calibration data on H6 shows that it's surely little endian: It consists a temperature value in 1/10 celsuis as unit, and some thermal register readout values, which are the values read out at the given temperature, and every value here (the temperature and the readout) are all half word length. Let the temperature value be AABB, the two readout values be XXYY and ZZWW, the oragnization is: BB AA YY XX WW ZZ ** ** . When converting the SID to big endian, it becomes: XX YY AA BB ** ** ZZ WW , which is non-sense, and not able to do sub-word cell addressing. Maxime, should I drop the LE2BE conversion in SID driver? (I doubt whether it will break compatibility.) Philipp, could you delay to send any code that uses SID? > > > + > > + return 0; > > +} > > + > > static int sun4i_gpadc_runtime_suspend(struct device *dev) > > { > > struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev)); > > @@ -428,6 +474,16 @@ static int sun4i_ths_suspend(struct > > sun4i_gpadc_iio *info) > > return 0; > > } > > > > +static int sun8i_h3_ths_suspend(struct sun4i_gpadc_iio *info) > > +{ > > + /* Disable ths interrupt */ > > + regmap_write(info->regmap, SUN8I_H3_THS_INTC, 0x0); > > + /* Disable temperature sensor */ > > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, 0x0); > > + > > + return 0; > > +} > > + > > static int sun4i_gpadc_runtime_resume(struct device *dev) > > { > > struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev)); > > @@ -454,6 +510,37 @@ static int sun4i_ths_resume(struct > > sun4i_gpadc_iio *info) > > return 0; > > } > > > > +static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info) > > +{ > > + u32 value; > > + > > + sun8i_h3_calibrate(info); > > + > > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL0, > > + SUN4I_GPADC_CTRL0_T_ACQ(0xff)); > > + > > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, > > + SUN8I_H3_THS_ACQ1(0x3f)); > > + > > + regmap_write(info->regmap, SUN8I_H3_THS_STAT, > > + SUN8I_H3_THS_INTS_TDATA_IRQ_0); > > + > > + regmap_write(info->regmap, SUN8I_H3_THS_FILTER, > > + SUN4I_GPADC_CTRL3_FILTER_EN | > > + SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2)); > > + > > + regmap_write(info->regmap, SUN8I_H3_THS_INTC, > > + SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 | > > + SUN8I_H3_THS_TEMP_PERIOD(0x55)); > > + > > + regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value); > > + > > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, > > + SUN8I_H3_THS_TEMP_SENSE_EN0 | value); > > Ideally, all these values should have a comment explaining what they > are. > > And we really start to have a lot of registers defines. We'd be > better > off using regmap_fields. > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 31.08.2018 11:51, Icenowy Zheng wrote: > Personally I suggest to leave out all SID or calibration related > patches here. > > Currently we seems to be wrongly converting SID to big endian, however, > the orgnization of the THS calibration data on H6 shows that it's > surely little endian: > > It consists a temperature value in 1/10 celsuis as unit, and some > thermal register readout values, which are the values read out at the > given temperature, and every value here (the temperature and the > readout) are all half word length. > > Let the temperature value be AABB, the two readout values be XXYY and > ZZWW, the oragnization is: > BB AA YY XX WW ZZ ** ** . > > When converting the SID to big endian, it becomes: > XX YY AA BB ** ** ZZ WW , > which is non-sense, and not able to do sub-word cell addressing. > > Maxime, should I drop the LE2BE conversion in SID driver? (I doubt > whether it will break compatibility.) > > Philipp, could you delay to send any code that uses SID? Yes for sure! I will move the related patches to the end of the series and add a DO-NOT-MERGE flag in the title. So I can get those also ready/reviewed but not merged. Icenowy, do you know more about the A83T SID? From the general specs it could be the same like on the A64 or the H3. Thanks, Philipp
On 31.08.2018 11:11, Maxime Ripard wrote: >> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL0, >> + SUN4I_GPADC_CTRL0_T_ACQ(0xff)); >> + >> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, >> + SUN8I_H3_THS_ACQ1(0x3f)); >> + >> + regmap_write(info->regmap, SUN8I_H3_THS_STAT, >> + SUN8I_H3_THS_INTS_TDATA_IRQ_0); >> + >> + regmap_write(info->regmap, SUN8I_H3_THS_FILTER, >> + SUN4I_GPADC_CTRL3_FILTER_EN | >> + SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2)); >> + >> + regmap_write(info->regmap, SUN8I_H3_THS_INTC, >> + SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 | >> + SUN8I_H3_THS_TEMP_PERIOD(0x55)); >> + >> + regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value); >> + >> + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, >> + SUN8I_H3_THS_TEMP_SENSE_EN0 | value); > Ideally, all these values should have a comment explaining what they > are. > > And we really start to have a lot of registers defines. We'd be better > off using regmap_fields. I will rework this in the next version. Thanks, Philipp
On Fri, Aug 31, 2018 at 05:51:41PM +0800, Icenowy Zheng wrote: > Personally I suggest to leave out all SID or calibration related > patches here. > > Currently we seems to be wrongly converting SID to big endian, however, > the orgnization of the THS calibration data on H6 shows that it's > surely little endian: > > It consists a temperature value in 1/10 celsuis as unit, and some > thermal register readout values, which are the values read out at the > given temperature, and every value here (the temperature and the > readout) are all half word length. > > Let the temperature value be AABB, the two readout values be XXYY and > ZZWW, the oragnization is: > BB AA YY XX WW ZZ ** ** . > > When converting the SID to big endian, it becomes: > XX YY AA BB ** ** ZZ WW , > which is non-sense, and not able to do sub-word cell addressing. > > Maxime, should I drop the LE2BE conversion in SID driver? (I doubt > whether it will break compatibility.) This is exposed to the userspace, so no. Maxime >
于 2018年9月3日 GMT+08:00 下午6:20:22, Maxime Ripard <maxime.ripard@bootlin.com> 写到: >On Fri, Aug 31, 2018 at 05:51:41PM +0800, Icenowy Zheng wrote: >> Personally I suggest to leave out all SID or calibration related >> patches here. >> >> Currently we seems to be wrongly converting SID to big endian, >however, >> the orgnization of the THS calibration data on H6 shows that it's >> surely little endian: >> >> It consists a temperature value in 1/10 celsuis as unit, and some >> thermal register readout values, which are the values read out at the >> given temperature, and every value here (the temperature and the >> readout) are all half word length. >> >> Let the temperature value be AABB, the two readout values be XXYY and >> ZZWW, the oragnization is: >> BB AA YY XX WW ZZ ** ** . >> >> When converting the SID to big endian, it becomes: >> XX YY AA BB ** ** ZZ WW , >> which is non-sense, and not able to do sub-word cell addressing. >> >> Maxime, should I drop the LE2BE conversion in SID driver? (I doubt >> whether it will break compatibility.) > >This is exposed to the userspace, so no. Please note the LE2BE totally breaks the SID addressing. Without it dropped all cells must be referenced with 4 byte word as unit, and half word addressing of SID is thus not possible. The driver will also need then to split the half words if needed. I think this is kind of hardware misbehavior, and not a simple UAPI change, so the UAPI stability shouldn't affect this change. > >Maxime >>
On Mon, Sep 03, 2018 at 07:01:47PM +0800, Icenowy Zheng wrote: > 于 2018年9月3日 GMT+08:00 下午6:20:22, Maxime Ripard <maxime.ripard@bootlin.com> 写到: > >On Fri, Aug 31, 2018 at 05:51:41PM +0800, Icenowy Zheng wrote: > >> Personally I suggest to leave out all SID or calibration related > >> patches here. > >> > >> Currently we seems to be wrongly converting SID to big endian, > >however, > >> the orgnization of the THS calibration data on H6 shows that it's > >> surely little endian: > >> > >> It consists a temperature value in 1/10 celsuis as unit, and some > >> thermal register readout values, which are the values read out at the > >> given temperature, and every value here (the temperature and the > >> readout) are all half word length. > >> > >> Let the temperature value be AABB, the two readout values be XXYY and > >> ZZWW, the oragnization is: > >> BB AA YY XX WW ZZ ** ** . > >> > >> When converting the SID to big endian, it becomes: > >> XX YY AA BB ** ** ZZ WW , > >> which is non-sense, and not able to do sub-word cell addressing. > >> > >> Maxime, should I drop the LE2BE conversion in SID driver? (I doubt > >> whether it will break compatibility.) > > > >This is exposed to the userspace, so no. > > Please note the LE2BE totally breaks the SID addressing. On which SoCs? > Without it dropped all cells must be referenced with 4 byte word as > unit, and half word addressing of SID is thus not possible. The > driver will also need then to split the half words if needed. > > I think this is kind of hardware misbehavior, and not a simple > UAPI change, so the UAPI stability shouldn't affect this change. How would you feel having your files on your filesystem suddenly not in the same endianness and having all the bytes reordered? This is definitely about the userspace ABI. Maxime
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c index c7b46c82e3e5..d5c7971b2558 100644 --- a/drivers/iio/adc/sun4i-gpadc-iio.c +++ b/drivers/iio/adc/sun4i-gpadc-iio.c @@ -72,6 +72,7 @@ struct gpadc_data { u32 temp_data_base; int sensor_count; bool supports_nvmem; + u32 ths_irq_clear; }; static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id); @@ -79,6 +80,10 @@ static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id); static int sun4i_ths_resume(struct sun4i_gpadc_iio *info); static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info); +static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info); +static int sun8i_h3_ths_suspend(struct sun4i_gpadc_iio *info); +static irqreturn_t sunx8i_h3_irq_thread(int irq, void *data); + static const struct gpadc_data sun4i_gpadc_data = { .temp_offset = -1932, .temp_scale = 133, @@ -137,6 +142,22 @@ static const struct gpadc_data sun8i_a33_gpadc_data = { .sensor_count = 1, }; +static const struct gpadc_data sun8i_h3_ths_data = { + .temp_offset = -1791, + .temp_scale = -121, + .temp_data_base = SUN8I_H3_THS_TDATA0, + .ths_irq_thread = sunx8i_h3_irq_thread, + .support_irq = true, + .has_bus_clk = true, + .has_bus_rst = true, + .has_mod_clk = true, + .sensor_count = 1, + .supports_nvmem = true, + .ths_resume = sun8i_h3_ths_resume, + .ths_suspend = sun8i_h3_ths_suspend, + .ths_irq_clear = SUN8I_H3_THS_INTS_TDATA_IRQ_0, +}; + struct sun4i_sensor_tzd { struct sun4i_gpadc_iio *info; struct thermal_zone_device *tzd; @@ -409,6 +430,31 @@ static irqreturn_t sun4i_gpadc_data_irq_handler(int irq, void *dev_id) return IRQ_HANDLED; } +static irqreturn_t sunx8i_h3_irq_thread(int irq, void *data) +{ + struct sun4i_gpadc_iio *info = data; + int i; + + regmap_write(info->regmap, SUN8I_H3_THS_STAT, + info->data->ths_irq_clear); + + for (i = 0; i < info->data->sensor_count; i++) + thermal_zone_device_update(info->tzds[i].tzd, + THERMAL_EVENT_TEMP_SAMPLE); + + return IRQ_HANDLED; +} + +static int sun8i_h3_calibrate(struct sun4i_gpadc_iio *info) +{ +// regmap_write(info->regmap, SUNXI_THS_CDATA_0_1, +// info->calibration_data[0]); +// regmap_write(info->regmap, SUNXI_THS_CDATA_2_3, +// info->calibration_data[1]); + + return 0; +} + static int sun4i_gpadc_runtime_suspend(struct device *dev) { struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev)); @@ -428,6 +474,16 @@ static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info) return 0; } +static int sun8i_h3_ths_suspend(struct sun4i_gpadc_iio *info) +{ + /* Disable ths interrupt */ + regmap_write(info->regmap, SUN8I_H3_THS_INTC, 0x0); + /* Disable temperature sensor */ + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, 0x0); + + return 0; +} + static int sun4i_gpadc_runtime_resume(struct device *dev) { struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev)); @@ -454,6 +510,37 @@ static int sun4i_ths_resume(struct sun4i_gpadc_iio *info) return 0; } +static int sun8i_h3_ths_resume(struct sun4i_gpadc_iio *info) +{ + u32 value; + + sun8i_h3_calibrate(info); + + regmap_write(info->regmap, SUN8I_H3_THS_CTRL0, + SUN4I_GPADC_CTRL0_T_ACQ(0xff)); + + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, + SUN8I_H3_THS_ACQ1(0x3f)); + + regmap_write(info->regmap, SUN8I_H3_THS_STAT, + SUN8I_H3_THS_INTS_TDATA_IRQ_0); + + regmap_write(info->regmap, SUN8I_H3_THS_FILTER, + SUN4I_GPADC_CTRL3_FILTER_EN | + SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2)); + + regmap_write(info->regmap, SUN8I_H3_THS_INTC, + SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 | + SUN8I_H3_THS_TEMP_PERIOD(0x55)); + + regmap_read(info->regmap, SUN8I_H3_THS_CTRL2, &value); + + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, + SUN8I_H3_THS_TEMP_SENSE_EN0 | value); + + return 0; +} + static int sun4i_gpadc_get_temp(void *data, int *temp) { struct sun4i_sensor_tzd *tzd = data; @@ -497,6 +584,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = { .compatible = "allwinner,sun6i-a31-gpadc", .data = &sun6i_gpadc_data }, + { + .compatible = "allwinner,sun8i-h3-ths", + .data = &sun8i_h3_ths_data, + }, { /* sentinel */ } }; diff --git a/include/linux/iio/adc/sun4i-gpadc.h b/include/linux/iio/adc/sun4i-gpadc.h index 99feeba28105..169b4de9a34d 100644 --- a/include/linux/iio/adc/sun4i-gpadc.h +++ b/include/linux/iio/adc/sun4i-gpadc.h @@ -100,6 +100,24 @@ } /* SUNXI_THS COMMON REGISTERS + DEFINES */ +#define SUN8I_H3_THS_CTRL0 0x00 + +#define SUN8I_H3_THS_CTRL2 0x40 +#define SUN8I_H3_THS_ACQ1(x) (GENMASK(31, 16) & ((x) << 16)) +#define SUN8I_H3_THS_TEMP_SENSE_EN0 BIT(0) + +#define SUN8I_H3_THS_INTC 0x44 +#define SUN8I_H3_THS_TEMP_PERIOD(x) (GENMASK(31, 12) & ((x) << 12)) +#define SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 BIT(8) + +#define SUN8I_H3_THS_STAT 0x48 +#define SUN8I_H3_THS_INTS_TDATA_IRQ_0 BIT(8) + +#define SUN8I_H3_THS_FILTER 0x70 +#define SUNXI_THS_CDATA_0_1 0x74 +#define SUNXI_THS_CDATA_2_3 0x78 +#define SUN8I_H3_THS_TDATA0 0x80 + #define MAX_SENSOR_COUNT 4 #endif
This patch adds support for the H3 ths sensor. The H3 supports interrupts. The interrupt is configured to update the the sensor values every second. The calibration data is writen at the begin of the init process. Signed-off-by: Philipp Rossak <embed3d@gmail.com> --- drivers/iio/adc/sun4i-gpadc-iio.c | 91 +++++++++++++++++++++++++++++++++++++ include/linux/iio/adc/sun4i-gpadc.h | 18 ++++++++ 2 files changed, 109 insertions(+)