Re: [PATCH v3 07/11] iio: adc: sun4i-gpadc-iio: add support for A33 thermal sensor

2017-03-25 Thread Jonathan Cameron
On 21/03/17 15:36, Quentin Schulz wrote:
> This adds support for the Allwinner A33 thermal sensor.
> 
> Unlike the A10, A13 and A31, the Allwinner A33 only has one channel
> which is dedicated to the thermal sensor. Moreover, its thermal sensor
> does not generate interruptions, thus we only need to directly read the
> register storing the temperature value.
> 
> The MFD used by the A10, A13 and A31, was created to avoid breaking the
> DT binding, but since the nodes for the ADC weren't there for the A33,
> it is not needed.
> 
> Though the A33 does not have an internal ADC, it has a thermal sensor
> which shares the same registers with GPADC of the already supported SoCs
> and almost the same bits, for the same purpose (thermal sensor).
> 
> The thermal sensor behaves exactly the same (except the presence of
> interrupts or not) on the different SoCs.
> 
> Signed-off-by: Quentin Schulz 
> Acked-by: Lee Jones 
Acked-by: Jonathan Cameron 

I'm assuming this is going through your tree Lee, let me know if you want
to do it differently.

Jonathan
> ---
> 
> v3:
>   - switched compatible from allwinner,sun8i-a33-gpadc-iio to
>   allwinner,sun8i-a33-ths to better reflect the datasheet's name,
>   - fixed the non-working if (!IS_ENABLED(THERMAL_OF)) by prefixing it with
>   CONFIG,
> 
> v2:
>   - removed added comments in Kconfig,
>   - simplified Kconfig depends on condition,
>   - removed THERMAL_OF requirement for sun8i,
>   - renamed sun8i_gpadc_channels to sun8i_a33_gpadc_channels,
>   - renamed use_dt boolean in no_irq as it reflects better why we need it,
>   - removed spurious/unneeded modifications done in v1,
> 
>  drivers/iio/adc/Kconfig   |   2 +-
>  drivers/iio/adc/sun4i-gpadc-iio.c | 100 
> --
>  include/linux/mfd/sun4i-gpadc.h   |   4 ++
>  3 files changed, 102 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 9f8b4b1..8c8ead6 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -562,7 +562,7 @@ config STX104
>  config SUN4I_GPADC
>   tristate "Support for the Allwinner SoCs GPADC"
>   depends on IIO
> - depends on MFD_SUN4I_GPADC
> + depends on MFD_SUN4I_GPADC || MACH_SUN8I
>   help
> Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
> GPADC. This ADC provides 4 channels which can be used as an ADC or as
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c 
> b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 7cb997a..74705aa 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -85,6 +85,12 @@ static const struct gpadc_data sun6i_gpadc_data = {
>   .adc_chan_mask = SUN6I_GPADC_CTRL1_ADC_CHAN_MASK,
>  };
>  
> +static const struct gpadc_data sun8i_a33_gpadc_data = {
> + .temp_offset = -1662,
> + .temp_scale = 162,
> + .tp_mode_en = SUN8I_GPADC_CTRL1_CHOP_TEMP_EN,
> +};
> +
>  struct sun4i_gpadc_iio {
>   struct iio_dev  *indio_dev;
>   struct completion   completion;
> @@ -96,6 +102,7 @@ struct sun4i_gpadc_iio {
>   unsigned inttemp_data_irq;
>   atomic_tignore_temp_data_irq;
>   const struct gpadc_data *data;
> + boolno_irq;
>   /* prevents concurrent reads of temperature and ADC */
>   struct mutexmutex;
>  };
> @@ -138,6 +145,23 @@ static const struct iio_chan_spec 
> sun4i_gpadc_channels_no_temp[] = {
>   SUN4I_GPADC_ADC_CHANNEL(3, "adc_chan3"),
>  };
>  
> +static const struct iio_chan_spec sun8i_a33_gpadc_channels[] = {
> + {
> + .type = IIO_TEMP,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +   BIT(IIO_CHAN_INFO_SCALE) |
> +   BIT(IIO_CHAN_INFO_OFFSET),
> + .datasheet_name = "temp_adc",
> + },
> +};
> +
> +static const struct regmap_config sun4i_gpadc_regmap_config = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .fast_io = true,
> +};
> +
>  static int sun4i_prepare_for_irq(struct iio_dev *indio_dev, int channel,
>unsigned int irq)
>  {
> @@ -247,6 +271,17 @@ static int sun4i_gpadc_temp_read(struct iio_dev 
> *indio_dev, int *val)
>  {
>   struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
>  
> + if (info->no_irq) {
> + pm_runtime_get_sync(indio_dev->dev.parent);
> +
> + regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
> +
> + pm_runtime_mark_last_busy(indio_dev->dev.parent);
> + pm_runtime_put_autosuspend(indio_dev->dev.parent);
> +
> + return 0;
> + }
> +
>   return sun4i_gpadc_read(indio_dev, 0, val, info->temp_data_irq);
>  }
>  
> @@ -454,6 +489,58 @@ static int 

Re: [PATCH v3 07/11] iio: adc: sun4i-gpadc-iio: add support for A33 thermal sensor

2017-03-25 Thread Jonathan Cameron
On 21/03/17 15:36, Quentin Schulz wrote:
> This adds support for the Allwinner A33 thermal sensor.
> 
> Unlike the A10, A13 and A31, the Allwinner A33 only has one channel
> which is dedicated to the thermal sensor. Moreover, its thermal sensor
> does not generate interruptions, thus we only need to directly read the
> register storing the temperature value.
> 
> The MFD used by the A10, A13 and A31, was created to avoid breaking the
> DT binding, but since the nodes for the ADC weren't there for the A33,
> it is not needed.
> 
> Though the A33 does not have an internal ADC, it has a thermal sensor
> which shares the same registers with GPADC of the already supported SoCs
> and almost the same bits, for the same purpose (thermal sensor).
> 
> The thermal sensor behaves exactly the same (except the presence of
> interrupts or not) on the different SoCs.
> 
> Signed-off-by: Quentin Schulz 
> Acked-by: Lee Jones 
Acked-by: Jonathan Cameron 

I'm assuming this is going through your tree Lee, let me know if you want
to do it differently.

Jonathan
> ---
> 
> v3:
>   - switched compatible from allwinner,sun8i-a33-gpadc-iio to
>   allwinner,sun8i-a33-ths to better reflect the datasheet's name,
>   - fixed the non-working if (!IS_ENABLED(THERMAL_OF)) by prefixing it with
>   CONFIG,
> 
> v2:
>   - removed added comments in Kconfig,
>   - simplified Kconfig depends on condition,
>   - removed THERMAL_OF requirement for sun8i,
>   - renamed sun8i_gpadc_channels to sun8i_a33_gpadc_channels,
>   - renamed use_dt boolean in no_irq as it reflects better why we need it,
>   - removed spurious/unneeded modifications done in v1,
> 
>  drivers/iio/adc/Kconfig   |   2 +-
>  drivers/iio/adc/sun4i-gpadc-iio.c | 100 
> --
>  include/linux/mfd/sun4i-gpadc.h   |   4 ++
>  3 files changed, 102 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 9f8b4b1..8c8ead6 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -562,7 +562,7 @@ config STX104
>  config SUN4I_GPADC
>   tristate "Support for the Allwinner SoCs GPADC"
>   depends on IIO
> - depends on MFD_SUN4I_GPADC
> + depends on MFD_SUN4I_GPADC || MACH_SUN8I
>   help
> Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
> GPADC. This ADC provides 4 channels which can be used as an ADC or as
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c 
> b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 7cb997a..74705aa 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -85,6 +85,12 @@ static const struct gpadc_data sun6i_gpadc_data = {
>   .adc_chan_mask = SUN6I_GPADC_CTRL1_ADC_CHAN_MASK,
>  };
>  
> +static const struct gpadc_data sun8i_a33_gpadc_data = {
> + .temp_offset = -1662,
> + .temp_scale = 162,
> + .tp_mode_en = SUN8I_GPADC_CTRL1_CHOP_TEMP_EN,
> +};
> +
>  struct sun4i_gpadc_iio {
>   struct iio_dev  *indio_dev;
>   struct completion   completion;
> @@ -96,6 +102,7 @@ struct sun4i_gpadc_iio {
>   unsigned inttemp_data_irq;
>   atomic_tignore_temp_data_irq;
>   const struct gpadc_data *data;
> + boolno_irq;
>   /* prevents concurrent reads of temperature and ADC */
>   struct mutexmutex;
>  };
> @@ -138,6 +145,23 @@ static const struct iio_chan_spec 
> sun4i_gpadc_channels_no_temp[] = {
>   SUN4I_GPADC_ADC_CHANNEL(3, "adc_chan3"),
>  };
>  
> +static const struct iio_chan_spec sun8i_a33_gpadc_channels[] = {
> + {
> + .type = IIO_TEMP,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +   BIT(IIO_CHAN_INFO_SCALE) |
> +   BIT(IIO_CHAN_INFO_OFFSET),
> + .datasheet_name = "temp_adc",
> + },
> +};
> +
> +static const struct regmap_config sun4i_gpadc_regmap_config = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .fast_io = true,
> +};
> +
>  static int sun4i_prepare_for_irq(struct iio_dev *indio_dev, int channel,
>unsigned int irq)
>  {
> @@ -247,6 +271,17 @@ static int sun4i_gpadc_temp_read(struct iio_dev 
> *indio_dev, int *val)
>  {
>   struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
>  
> + if (info->no_irq) {
> + pm_runtime_get_sync(indio_dev->dev.parent);
> +
> + regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
> +
> + pm_runtime_mark_last_busy(indio_dev->dev.parent);
> + pm_runtime_put_autosuspend(indio_dev->dev.parent);
> +
> + return 0;
> + }
> +
>   return sun4i_gpadc_read(indio_dev, 0, val, info->temp_data_irq);
>  }
>  
> @@ -454,6 +489,58 @@ static int sun4i_irq_init(struct platform_device *pdev, 
> const char *name,
>   

[PATCH v3 07/11] iio: adc: sun4i-gpadc-iio: add support for A33 thermal sensor

2017-03-21 Thread Quentin Schulz
This adds support for the Allwinner A33 thermal sensor.

Unlike the A10, A13 and A31, the Allwinner A33 only has one channel
which is dedicated to the thermal sensor. Moreover, its thermal sensor
does not generate interruptions, thus we only need to directly read the
register storing the temperature value.

The MFD used by the A10, A13 and A31, was created to avoid breaking the
DT binding, but since the nodes for the ADC weren't there for the A33,
it is not needed.

Though the A33 does not have an internal ADC, it has a thermal sensor
which shares the same registers with GPADC of the already supported SoCs
and almost the same bits, for the same purpose (thermal sensor).

The thermal sensor behaves exactly the same (except the presence of
interrupts or not) on the different SoCs.

Signed-off-by: Quentin Schulz 
Acked-by: Lee Jones 
---

v3:
  - switched compatible from allwinner,sun8i-a33-gpadc-iio to
  allwinner,sun8i-a33-ths to better reflect the datasheet's name,
  - fixed the non-working if (!IS_ENABLED(THERMAL_OF)) by prefixing it with
  CONFIG,

v2:
  - removed added comments in Kconfig,
  - simplified Kconfig depends on condition,
  - removed THERMAL_OF requirement for sun8i,
  - renamed sun8i_gpadc_channels to sun8i_a33_gpadc_channels,
  - renamed use_dt boolean in no_irq as it reflects better why we need it,
  - removed spurious/unneeded modifications done in v1,

 drivers/iio/adc/Kconfig   |   2 +-
 drivers/iio/adc/sun4i-gpadc-iio.c | 100 --
 include/linux/mfd/sun4i-gpadc.h   |   4 ++
 3 files changed, 102 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 9f8b4b1..8c8ead6 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -562,7 +562,7 @@ config STX104
 config SUN4I_GPADC
tristate "Support for the Allwinner SoCs GPADC"
depends on IIO
-   depends on MFD_SUN4I_GPADC
+   depends on MFD_SUN4I_GPADC || MACH_SUN8I
help
  Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
  GPADC. This ADC provides 4 channels which can be used as an ADC or as
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c 
b/drivers/iio/adc/sun4i-gpadc-iio.c
index 7cb997a..74705aa 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -85,6 +85,12 @@ static const struct gpadc_data sun6i_gpadc_data = {
.adc_chan_mask = SUN6I_GPADC_CTRL1_ADC_CHAN_MASK,
 };
 
+static const struct gpadc_data sun8i_a33_gpadc_data = {
+   .temp_offset = -1662,
+   .temp_scale = 162,
+   .tp_mode_en = SUN8I_GPADC_CTRL1_CHOP_TEMP_EN,
+};
+
 struct sun4i_gpadc_iio {
struct iio_dev  *indio_dev;
struct completion   completion;
@@ -96,6 +102,7 @@ struct sun4i_gpadc_iio {
unsigned inttemp_data_irq;
atomic_tignore_temp_data_irq;
const struct gpadc_data *data;
+   boolno_irq;
/* prevents concurrent reads of temperature and ADC */
struct mutexmutex;
 };
@@ -138,6 +145,23 @@ static const struct iio_chan_spec 
sun4i_gpadc_channels_no_temp[] = {
SUN4I_GPADC_ADC_CHANNEL(3, "adc_chan3"),
 };
 
+static const struct iio_chan_spec sun8i_a33_gpadc_channels[] = {
+   {
+   .type = IIO_TEMP,
+   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_OFFSET),
+   .datasheet_name = "temp_adc",
+   },
+};
+
+static const struct regmap_config sun4i_gpadc_regmap_config = {
+   .reg_bits = 32,
+   .val_bits = 32,
+   .reg_stride = 4,
+   .fast_io = true,
+};
+
 static int sun4i_prepare_for_irq(struct iio_dev *indio_dev, int channel,
 unsigned int irq)
 {
@@ -247,6 +271,17 @@ static int sun4i_gpadc_temp_read(struct iio_dev 
*indio_dev, int *val)
 {
struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
 
+   if (info->no_irq) {
+   pm_runtime_get_sync(indio_dev->dev.parent);
+
+   regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
+
+   pm_runtime_mark_last_busy(indio_dev->dev.parent);
+   pm_runtime_put_autosuspend(indio_dev->dev.parent);
+
+   return 0;
+   }
+
return sun4i_gpadc_read(indio_dev, 0, val, info->temp_data_irq);
 }
 
@@ -454,6 +489,58 @@ static int sun4i_irq_init(struct platform_device *pdev, 
const char *name,
return 0;
 }
 
+static const struct of_device_id sun4i_gpadc_of_id[] = {
+   {
+   .compatible = "allwinner,sun8i-a33-ths",
+   .data = _a33_gpadc_data,
+   },
+   { /* sentinel */ }
+};
+
+static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
+ 

[PATCH v3 07/11] iio: adc: sun4i-gpadc-iio: add support for A33 thermal sensor

2017-03-21 Thread Quentin Schulz
This adds support for the Allwinner A33 thermal sensor.

Unlike the A10, A13 and A31, the Allwinner A33 only has one channel
which is dedicated to the thermal sensor. Moreover, its thermal sensor
does not generate interruptions, thus we only need to directly read the
register storing the temperature value.

The MFD used by the A10, A13 and A31, was created to avoid breaking the
DT binding, but since the nodes for the ADC weren't there for the A33,
it is not needed.

Though the A33 does not have an internal ADC, it has a thermal sensor
which shares the same registers with GPADC of the already supported SoCs
and almost the same bits, for the same purpose (thermal sensor).

The thermal sensor behaves exactly the same (except the presence of
interrupts or not) on the different SoCs.

Signed-off-by: Quentin Schulz 
Acked-by: Lee Jones 
---

v3:
  - switched compatible from allwinner,sun8i-a33-gpadc-iio to
  allwinner,sun8i-a33-ths to better reflect the datasheet's name,
  - fixed the non-working if (!IS_ENABLED(THERMAL_OF)) by prefixing it with
  CONFIG,

v2:
  - removed added comments in Kconfig,
  - simplified Kconfig depends on condition,
  - removed THERMAL_OF requirement for sun8i,
  - renamed sun8i_gpadc_channels to sun8i_a33_gpadc_channels,
  - renamed use_dt boolean in no_irq as it reflects better why we need it,
  - removed spurious/unneeded modifications done in v1,

 drivers/iio/adc/Kconfig   |   2 +-
 drivers/iio/adc/sun4i-gpadc-iio.c | 100 --
 include/linux/mfd/sun4i-gpadc.h   |   4 ++
 3 files changed, 102 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 9f8b4b1..8c8ead6 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -562,7 +562,7 @@ config STX104
 config SUN4I_GPADC
tristate "Support for the Allwinner SoCs GPADC"
depends on IIO
-   depends on MFD_SUN4I_GPADC
+   depends on MFD_SUN4I_GPADC || MACH_SUN8I
help
  Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
  GPADC. This ADC provides 4 channels which can be used as an ADC or as
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c 
b/drivers/iio/adc/sun4i-gpadc-iio.c
index 7cb997a..74705aa 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -85,6 +85,12 @@ static const struct gpadc_data sun6i_gpadc_data = {
.adc_chan_mask = SUN6I_GPADC_CTRL1_ADC_CHAN_MASK,
 };
 
+static const struct gpadc_data sun8i_a33_gpadc_data = {
+   .temp_offset = -1662,
+   .temp_scale = 162,
+   .tp_mode_en = SUN8I_GPADC_CTRL1_CHOP_TEMP_EN,
+};
+
 struct sun4i_gpadc_iio {
struct iio_dev  *indio_dev;
struct completion   completion;
@@ -96,6 +102,7 @@ struct sun4i_gpadc_iio {
unsigned inttemp_data_irq;
atomic_tignore_temp_data_irq;
const struct gpadc_data *data;
+   boolno_irq;
/* prevents concurrent reads of temperature and ADC */
struct mutexmutex;
 };
@@ -138,6 +145,23 @@ static const struct iio_chan_spec 
sun4i_gpadc_channels_no_temp[] = {
SUN4I_GPADC_ADC_CHANNEL(3, "adc_chan3"),
 };
 
+static const struct iio_chan_spec sun8i_a33_gpadc_channels[] = {
+   {
+   .type = IIO_TEMP,
+   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_OFFSET),
+   .datasheet_name = "temp_adc",
+   },
+};
+
+static const struct regmap_config sun4i_gpadc_regmap_config = {
+   .reg_bits = 32,
+   .val_bits = 32,
+   .reg_stride = 4,
+   .fast_io = true,
+};
+
 static int sun4i_prepare_for_irq(struct iio_dev *indio_dev, int channel,
 unsigned int irq)
 {
@@ -247,6 +271,17 @@ static int sun4i_gpadc_temp_read(struct iio_dev 
*indio_dev, int *val)
 {
struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
 
+   if (info->no_irq) {
+   pm_runtime_get_sync(indio_dev->dev.parent);
+
+   regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
+
+   pm_runtime_mark_last_busy(indio_dev->dev.parent);
+   pm_runtime_put_autosuspend(indio_dev->dev.parent);
+
+   return 0;
+   }
+
return sun4i_gpadc_read(indio_dev, 0, val, info->temp_data_irq);
 }
 
@@ -454,6 +489,58 @@ static int sun4i_irq_init(struct platform_device *pdev, 
const char *name,
return 0;
 }
 
+static const struct of_device_id sun4i_gpadc_of_id[] = {
+   {
+   .compatible = "allwinner,sun8i-a33-ths",
+   .data = _a33_gpadc_data,
+   },
+   { /* sentinel */ }
+};
+
+static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
+   struct iio_dev *indio_dev)
+{
+