from:"Philipp Rossak"

[linux-sunxi] Re: Allwinner SID THS calibration data cell representation?

2019-02-25 Thread Philipp Rossak





On 19.02.19 08:54, Chen-Yu Tsai wrote:

Sorry for resurrecting an old discussion, but since someone posted patches
for H5 and H6, I thought we should resolve this. I'm working on patches to
fix / replace the big-endian issue.

On Thu, Sep 6, 2018 at 7:51 PM Maxime Ripard  wrote:


On Thu, Sep 06, 2018 at 01:47:47PM +0200, Philipp Rossak wrote:

On 04.09.2018 18:46, Emmanuel Vadot wrote:

+ /* Data cells */
+ thermal_calibration: calib@234 {
+ reg = <0x234 0x8>;
+ };

   You are declaring 8 bytes of calibration data but to my knowledge it's
only 2 bytes per sensor, so 2 bytes for H3.
   Am I missing something ?

   Thanks,


Emmanuel you are right, it is 2 bytes per Sensor and should be 2 bytes for
H3, but the thermal calibration data field is on all chips 64 bit wide - so
8 bytes. So I'm reading here the complete calibration data field.


Having one cell per channel would make more sense I guess.


Would it? The 2 32-bit words directly map onto the registers 0x74 / 0x78 in
the THS. As far as the SID is concerned, their is just one consumer for this
data, the thermal sensor. How the thermal sensor uses that data is really not
its concern. And the thermal sensor is really just copying the data from the
e-fuses into its registers. Nothing more.


Using 2 32-bit words for the THS would be also ok (from my perspective).


Furthermore, with the register access interface, the e-fuses are read/write
32 bits at a time. Seems to me it would make more sense to enforce a 32-bit
word size, so cells should be multiples of 32 bits.


For THS I'm ok with that.


Regards
ChenYu


Regards,
Philipp

--
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH v3 27/30] ARM: dts: sun8i: h3: use calibration for ths

2018-09-06 Thread Philipp Rossak





On 06.09.2018 14:04, Icenowy Zheng wrote:



于 2018年9月6日 GMT+08:00 下午7:51:15, Maxime Ripard  写到:

On Thu, Sep 06, 2018 at 01:47:47PM +0200, Philipp Rossak wrote:

On 04.09.2018 18:46, Emmanuel Vadot wrote:

+   /* Data cells */
+   thermal_calibration: calib@234 {
+   reg = <0x234 0x8>;
+   };

   You are declaring 8 bytes of calibration data but to my knowledge

it's

only 2 bytes per sensor, so 2 bytes for H3.
   Am I missing something ?

   Thanks,


Emmanuel you are right, it is 2 bytes per Sensor and should be 2

bytes for

H3, but the thermal calibration data field is on all chips 64 bit

wide - so

8 bytes. So I'm reading here the complete calibration data field.


Having one cell per channel would make more sense I guess.

Ok I will change this.


I have mentioned that this is impossible because of wrong
addressing caused by LE2BE in SID driver.

I know! But I would like to prepare patches for it, that they can be 
merged when this is fixed.


Philipp

--
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 27/30] ARM: dts: sun8i: h3: use calibration for ths

2018-09-06 Thread Philipp Rossak


On 04.09.2018 18:46, Emmanuel Vadot wrote:

+   /* Data cells */
+   thermal_calibration: calib@234 {
+   reg = <0x234 0x8>;
+   };

  You are declaring 8 bytes of calibration data but to my knowledge it's
only 2 bytes per sensor, so 2 bytes for H3.
  Am I missing something ?

  Thanks,


Emmanuel you are right, it is 2 bytes per Sensor and should be 2 bytes 
for H3, but the thermal calibration data field is on all chips 64 bit 
wide - so 8 bytes. So I'm reading here the complete calibration data field.


Philipp

--
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 30/30] ARM: sun8i: a83t: full range OPP tables and CPUfreq

2018-09-06 Thread Philipp Rossak

On 06.09.2018 09:24, Quentin Schulz wrote:

Hi Philipp,

On Thu, Aug 30, 2018 at 05:45:18PM +0200, Philipp Rossak wrote:

Since we have now thermal trotteling enabeled we can now add the full
range of the OPP table.

That's not the reason why they were not added.

Please see commit 2db639d8c1663d7543c9ab5323383d94c8a76c63[1].

Basically, you only want the OPPs which can work below or at the default
voltage of the CPU supply, because the CPU supply is specific to each
board.

If you set your CPU to work at a given frequency and the voltage isn't
updated (saying opp-microvolt = ; in DT isn't enough, you need
cpu-supply to be provided and functional), the CPU might just crash.

Without cpu-supply property, underclocking isn't effective in term of
thermal cooling or power saving. Overclocking is very, very, very likely
to make the CPU crash.

It's not a very difficult thing to do to test if a given frequency work
well but it needs a specific test environment and it's a lengthy test,
you can have a look at those tools here[3] if you like. It's not because
it works in a given test case that'll work on the long term under heavy
load and constant frequency changes.

For A83T, I already did it and the outcome is the patch in [1]. Same for
A33.

So, if you want to use these three higher OPPs, you need to define them
in your board DTS and add the cpu-supply property. See what's done for
the A33 and more specifically the Sinlinx SinA33[2] as an example.

[1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2db639d8c1663d7543c9ab5323383d94c8a76c63
[2]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts
[3]http://linux-sunxi.org/Hardware_Reliability_Tests#CPU

Quentin

Hey Quentin,

thanks for your feedback!

Sounds like we will never be able to run the A83T on its maximum
frequency in mainline.

I will do some testing, during the next weeks/months when I have time.
With the old Allwinner kernel I was able to run the A83T with its
maximum frequency without any problems since my board is very good cooled.

For now I will drop this patch.

Philipp

--
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 05/30] iio: adc: move SUN4I_GPADC_CHANNEL define to header file

2018-09-03 Thread Philipp Rossak


On 02.09.2018 22:01, Jonathan Cameron wrote:

On Thu, 30 Aug 2018 17:44:53 +0200
Philipp Rossak  wrote:


We are moving the SUN4I_GPADC_CHANNEL define to the header file.

Maxime has raised this point in other patches...

Why?  Obvious what but I have no idea why you are doing this.

Thanks,

Jonathan

There are two reasons:
1. Personal taste: I like to have the #define stuff in the header file.
2. When I started the rework I had to get some better overview, so I 
moved it...


Since those two reasons are no good reasons to submit a patch I will 
drop it and keep it in the *.c file.


Philipp



Signed-off-by: Philipp Rossak 
---
  drivers/iio/adc/sun4i-gpadc-iio.c | 9 -
  include/linux/mfd/sun4i-gpadc.h   | 9 +
  2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c 
b/drivers/iio/adc/sun4i-gpadc-iio.c
index d95dd0fde2a6..666329940e1e 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -109,15 +109,6 @@ struct sun4i_gpadc_iio {
struct device   *sensor_device;
  };
  
-#define SUN4I_GPADC_ADC_CHANNEL(_channel, _name) {		\

-   .type = IIO_VOLTAGE,\
-   .indexed = 1,   \
-   .channel = _channel,\
-   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
-   .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),   \
-   .datasheet_name = _name,\
-}
-
  static struct iio_map sun4i_gpadc_hwmon_maps[] = {
{
.adc_channel_label = "temp_adc",
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
index 139872c2e0fe..54c7c9375c1b 100644
--- a/include/linux/mfd/sun4i-gpadc.h
+++ b/include/linux/mfd/sun4i-gpadc.h
@@ -90,6 +90,15 @@
  /* 10s delay before suspending the IP */
  #define SUN4I_GPADC_AUTOSUSPEND_DELAY 1
  
+#define SUN4I_GPADC_ADC_CHANNEL(_channel, _name) {		\

+   .type = IIO_VOLTAGE,\
+   .indexed = 1,   \
+   .channel = _channel,\
+   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
+   .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),   \
+   .datasheet_name = _name,\
+}
+
  struct sun4i_gpadc_dev {
struct device   *dev;
struct regmap   *regmap;




--
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 18/30] iio: adc: sun4i-gpadc-iio: rework: support multiple sensors

2018-09-03 Thread Philipp Rossak


On 02.09.2018 22:11, Jonathan Cameron wrote:

This feels like a good place to factor out the code into a utility
function that just does one of them.  That should hopefully
reduce the indenting etc enough to make the code easier to read.


+   info->tzds[i].info = info;
+   info->tzds[i].sensor_id = i;
+
+   info->tzds[i].tzd = thermal_zone_of_sensor_register(
+   info->sensor_device,
+   i, >tzds[i], _ts_tz_ops);
+   /*
+* Do not fail driver probing when failing to register in
+* thermal because no thermal DT node is found.
+*/
+   if (IS_ERR(info->tzds[i].tzd) && \
+   PTR_ERR(info->tzds[i].tzd) != -ENODEV) {
+   dev_err(>dev,
+   "could not register thermal sensor: %ld\n",
+   PTR_ERR(info->tzds[i].tzd));
+   return PTR_ERR(info->tzds[i].tzd);
+   }


So this code above should be placed in a separate function and called by 
the for loop?

Did I understand that right?

Philipp

--
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 20/30] iio: adc: sun4i-gpadc-iio: rework: device specific suspend & resume

2018-08-31 Thread Philipp Rossak





On 31.08.2018 11:09, Maxime Ripard wrote:

+static int sun4i_ths_suspend(struct sun4i_gpadc_iio *info)

suspend is already a hook in the kernel, which hasn't the same meaning
than runtime_suspend (and the same applies to resume), so we'd rather
pick a better name. And all the functions (and the driver) use gpadc,
please continue to use that prefix.


I agree.
For the newer sensors (from H3) the Sensor is referenced in the 
datasheets as Thermal Sensor short THS. So I would like to use for the 
newer sensors that prefix. Is that ok?


Philipp

--
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 21/30] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor

2018-08-31 Thread Philipp Rossak


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, );
+
+   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

--
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH v3 21/30] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor

2018-08-31 Thread Philipp Rossak





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

--
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

1 2 >

1 - 100 of 191 matches

Mail list logo