linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] mfd: intel_soc_pmic_bxtwc: Add Intel BXT WhiskeyCove PMIC ADC thermal channel-zone mapping
@ 2016-06-16 21:58 Bin Gao
  2016-06-17  8:01 ` Lee Jones
  0 siblings, 1 reply; 6+ messages in thread
From: Bin Gao @ 2016-06-16 21:58 UTC (permalink / raw)
  To: Zhang Rui, Lee Jones, Eduardo Valentin, linux-kernel
  Cc: ysiyer, Ajay Thomas, Bin Gao

This patch adds the mapping of PMIC ADC channel to thermal zone.
This mapping is used in the pmic thermal driver to notify the thermal
zone with the pmic adc channel alert interrupts.
This patch also adds three new data structures to
include/linux/mfd/intel_soc_pmic.h: struct trip_config_map{},
struct thermal_irq_map {} and struct pmic_thermal_data {} which are
required by changes we did on intel_soc_pmic_bxtwc.c.

Signed-off-by: Yegnesh S Iyer <yegnesh.s.iyer@intel.com>
Signed-off-by: Bin Gao <bin.gao@intel.com>
---
Changes in v2:
 - Fixed subject line.
 - Combined two patches into one.
 drivers/mfd/intel_soc_pmic_bxtwc.c | 109 +++++++++++++++++++++++++++++++++++++
 include/linux/mfd/intel_soc_pmic.h |  21 +++++++
 2 files changed, 130 insertions(+)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index b942876..ec50788 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -58,6 +58,10 @@
 #define BXTWC_MGPIO1IRQ		0x4E1A
 #define BXTWC_MCRITIRQ		0x4E1B
 
+#define BXTWC_STHRM0IRQ		0x4F19
+#define BXTWC_STHRM1IRQ		0x4F1A
+#define BXTWC_STHRM2IRQ		0x4F1B
+
 /* Whiskey Cove PMIC share same ACPI ID between different platforms */
 #define BROXTON_PMIC_WC_HRV	4
 
@@ -116,6 +120,109 @@ static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
 	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 9, 0x03),
 };
 
+static struct trip_config_map str0_trip_config[] = {
+	{
+		.irq_reg = BXTWC_THRM0IRQ,
+		.irq_mask = 0x01,
+		.irq_en = BXTWC_MTHRM0IRQ,
+		.irq_en_mask = 0x01,
+		.evt_stat = BXTWC_STHRM0IRQ,
+		.evt_mask = 0x01,
+		.trip_num = 0
+	},
+	{
+		.irq_reg = BXTWC_THRM0IRQ,
+		.irq_mask = 0x10,
+		.irq_en = BXTWC_MTHRM0IRQ,
+		.irq_en_mask = 0x10,
+		.evt_stat = BXTWC_STHRM0IRQ,
+		.evt_mask = 0x10,
+		.trip_num = 1
+	}
+};
+
+static struct trip_config_map str1_trip_config[] = {
+	{
+		.irq_reg = BXTWC_THRM0IRQ,
+		.irq_mask = 0x02,
+		.irq_en = BXTWC_MTHRM0IRQ,
+		.irq_en_mask = 0x02,
+		.evt_stat = BXTWC_STHRM0IRQ,
+		.evt_mask = 0x02,
+		.trip_num = 0
+	},
+	{
+		.irq_reg = BXTWC_THRM0IRQ,
+		.irq_mask = 0x20,
+		.irq_en = BXTWC_MTHRM0IRQ,
+		.irq_en_mask = 0x20,
+		.evt_stat = BXTWC_STHRM0IRQ,
+		.evt_mask = 0x20,
+		.trip_num = 1
+	},
+};
+
+static struct trip_config_map str2_trip_config[] = {
+	{
+		.irq_reg = BXTWC_THRM0IRQ,
+		.irq_mask = 0x04,
+		.irq_en = BXTWC_MTHRM0IRQ,
+		.irq_en_mask = 0x04,
+		.evt_stat = BXTWC_STHRM0IRQ,
+		.evt_mask = 0x04,
+		.trip_num = 0
+	},
+	{
+		.irq_reg = BXTWC_THRM0IRQ,
+		.irq_mask = 0x40,
+		.irq_en = BXTWC_MTHRM0IRQ,
+		.irq_en_mask = 0x40,
+		.evt_stat = BXTWC_STHRM0IRQ,
+		.evt_mask = 0x40,
+		.trip_num = 1
+	},
+};
+
+static struct trip_config_map str3_trip_config[] = {
+	{
+		.irq_reg = BXTWC_THRM2IRQ,
+		.irq_mask = 0x10,
+		.irq_en = BXTWC_MTHRM2IRQ,
+		.irq_en_mask = 0x10,
+		.evt_stat = BXTWC_STHRM2IRQ,
+		.evt_mask = 0x10,
+		.trip_num = 0
+	},
+};
+
+static struct thermal_irq_map bxtwc_thermal_irq_map[] = {
+	{
+		.handle = "STR0",
+		.trip_config = str0_trip_config,
+		.num_trips = ARRAY_SIZE(str0_trip_config),
+	},
+	{
+		.handle = "STR1",
+		.trip_config = str1_trip_config,
+		.num_trips = ARRAY_SIZE(str1_trip_config),
+	},
+	{
+		.handle = "STR2",
+		.trip_config = str2_trip_config,
+		.num_trips = ARRAY_SIZE(str2_trip_config),
+	},
+	{
+		.handle = "STR3",
+		.trip_config = str3_trip_config,
+		.num_trips = ARRAY_SIZE(str3_trip_config),
+	},
+};
+
+static struct pmic_thermal_data bxtwc_thermal_data = {
+	.maps = bxtwc_thermal_irq_map,
+	.num_maps = ARRAY_SIZE(bxtwc_thermal_irq_map),
+};
+
 static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
 	.name = "bxtwc_irq_chip",
 	.status_base = BXTWC_IRQLVL1,
@@ -168,6 +275,8 @@ static struct mfd_cell bxt_wc_dev[] = {
 		.name = "bxt_wcove_thermal",
 		.num_resources = ARRAY_SIZE(thermal_resources),
 		.resources = thermal_resources,
+		.platform_data = &bxtwc_thermal_data,
+		.pdata_size = sizeof(bxtwc_thermal_data),
 	},
 	{
 		.name = "bxt_wcove_ext_charger",
diff --git a/include/linux/mfd/intel_soc_pmic.h b/include/linux/mfd/intel_soc_pmic.h
index cf619db..7df4302 100644
--- a/include/linux/mfd/intel_soc_pmic.h
+++ b/include/linux/mfd/intel_soc_pmic.h
@@ -21,6 +21,27 @@
 
 #include <linux/regmap.h>
 
+struct trip_config_map {
+	u16 irq_reg;
+	u16 irq_en;
+	u16 evt_stat;
+	u8 irq_mask;
+	u8 irq_en_mask;
+	u8 evt_mask;
+	u8 trip_num;
+};
+
+struct thermal_irq_map {
+	char handle[20];
+	int num_trips;
+	struct trip_config_map *trip_config;
+};
+
+struct pmic_thermal_data {
+	struct thermal_irq_map *maps;
+	int num_maps;
+};
+
 struct intel_soc_pmic {
 	int irq;
 	struct regmap *regmap;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] mfd: intel_soc_pmic_bxtwc: Add Intel BXT WhiskeyCove PMIC ADC thermal channel-zone mapping
  2016-06-16 21:58 [PATCH v2] mfd: intel_soc_pmic_bxtwc: Add Intel BXT WhiskeyCove PMIC ADC thermal channel-zone mapping Bin Gao
@ 2016-06-17  8:01 ` Lee Jones
  2016-06-20  4:11   ` Bin Gao
  0 siblings, 1 reply; 6+ messages in thread
From: Lee Jones @ 2016-06-17  8:01 UTC (permalink / raw)
  To: Bin Gao
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, ysiyer, Ajay Thomas,
	Bin Gao, broonie

On Thu, 16 Jun 2016, Bin Gao wrote:

> This patch adds the mapping of PMIC ADC channel to thermal zone.
> This mapping is used in the pmic thermal driver to notify the thermal
> zone with the pmic adc channel alert interrupts.
> This patch also adds three new data structures to
> include/linux/mfd/intel_soc_pmic.h: struct trip_config_map{},
> struct thermal_irq_map {} and struct pmic_thermal_data {} which are
> required by changes we did on intel_soc_pmic_bxtwc.c.
> 
> Signed-off-by: Yegnesh S Iyer <yegnesh.s.iyer@intel.com>
> Signed-off-by: Bin Gao <bin.gao@intel.com>
> ---
> Changes in v2:
>  - Fixed subject line.
>  - Combined two patches into one.
>  drivers/mfd/intel_soc_pmic_bxtwc.c | 109 +++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/intel_soc_pmic.h |  21 +++++++
>  2 files changed, 130 insertions(+)
> 
> diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
> index b942876..ec50788 100644
> --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
> +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
> @@ -58,6 +58,10 @@
>  #define BXTWC_MGPIO1IRQ		0x4E1A
>  #define BXTWC_MCRITIRQ		0x4E1B
>  
> +#define BXTWC_STHRM0IRQ		0x4F19
> +#define BXTWC_STHRM1IRQ		0x4F1A
> +#define BXTWC_STHRM2IRQ		0x4F1B
> +
>  /* Whiskey Cove PMIC share same ACPI ID between different platforms */
>  #define BROXTON_PMIC_WC_HRV	4
>  
> @@ -116,6 +120,109 @@ static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
>  	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 9, 0x03),
>  };
>  
> +static struct trip_config_map str0_trip_config[] = {
> +	{
> +		.irq_reg = BXTWC_THRM0IRQ,
> +		.irq_mask = 0x01,
> +		.irq_en = BXTWC_MTHRM0IRQ,
> +		.irq_en_mask = 0x01,
> +		.evt_stat = BXTWC_STHRM0IRQ,
> +		.evt_mask = 0x01,
> +		.trip_num = 0
> +	},
> +	{
> +		.irq_reg = BXTWC_THRM0IRQ,
> +		.irq_mask = 0x10,
> +		.irq_en = BXTWC_MTHRM0IRQ,
> +		.irq_en_mask = 0x10,
> +		.evt_stat = BXTWC_STHRM0IRQ,
> +		.evt_mask = 0x10,
> +		.trip_num = 1
> +	}
> +};
> +
> +static struct trip_config_map str1_trip_config[] = {
> +	{
> +		.irq_reg = BXTWC_THRM0IRQ,
> +		.irq_mask = 0x02,
> +		.irq_en = BXTWC_MTHRM0IRQ,
> +		.irq_en_mask = 0x02,
> +		.evt_stat = BXTWC_STHRM0IRQ,
> +		.evt_mask = 0x02,
> +		.trip_num = 0
> +	},
> +	{
> +		.irq_reg = BXTWC_THRM0IRQ,
> +		.irq_mask = 0x20,
> +		.irq_en = BXTWC_MTHRM0IRQ,
> +		.irq_en_mask = 0x20,
> +		.evt_stat = BXTWC_STHRM0IRQ,
> +		.evt_mask = 0x20,
> +		.trip_num = 1
> +	},
> +};
> +
> +static struct trip_config_map str2_trip_config[] = {
> +	{
> +		.irq_reg = BXTWC_THRM0IRQ,
> +		.irq_mask = 0x04,
> +		.irq_en = BXTWC_MTHRM0IRQ,
> +		.irq_en_mask = 0x04,
> +		.evt_stat = BXTWC_STHRM0IRQ,
> +		.evt_mask = 0x04,
> +		.trip_num = 0
> +	},
> +	{
> +		.irq_reg = BXTWC_THRM0IRQ,
> +		.irq_mask = 0x40,
> +		.irq_en = BXTWC_MTHRM0IRQ,
> +		.irq_en_mask = 0x40,
> +		.evt_stat = BXTWC_STHRM0IRQ,
> +		.evt_mask = 0x40,
> +		.trip_num = 1
> +	},
> +};
> +
> +static struct trip_config_map str3_trip_config[] = {
> +	{
> +		.irq_reg = BXTWC_THRM2IRQ,
> +		.irq_mask = 0x10,
> +		.irq_en = BXTWC_MTHRM2IRQ,
> +		.irq_en_mask = 0x10,
> +		.evt_stat = BXTWC_STHRM2IRQ,
> +		.evt_mask = 0x10,
> +		.trip_num = 0
> +	},
> +};

This looks like a register map to me.

Can you use the regmap framework instead?

> +static struct thermal_irq_map bxtwc_thermal_irq_map[] = {
> +	{
> +		.handle = "STR0",
> +		.trip_config = str0_trip_config,
> +		.num_trips = ARRAY_SIZE(str0_trip_config),
> +	},
> +	{
> +		.handle = "STR1",
> +		.trip_config = str1_trip_config,
> +		.num_trips = ARRAY_SIZE(str1_trip_config),
> +	},
> +	{
> +		.handle = "STR2",
> +		.trip_config = str2_trip_config,
> +		.num_trips = ARRAY_SIZE(str2_trip_config),
> +	},
> +	{
> +		.handle = "STR3",
> +		.trip_config = str3_trip_config,
> +		.num_trips = ARRAY_SIZE(str3_trip_config),
> +	},
> +};
> +
> +static struct pmic_thermal_data bxtwc_thermal_data = {
> +	.maps = bxtwc_thermal_irq_map,
> +	.num_maps = ARRAY_SIZE(bxtwc_thermal_irq_map),
> +};
> +
>  static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
>  	.name = "bxtwc_irq_chip",
>  	.status_base = BXTWC_IRQLVL1,
> @@ -168,6 +275,8 @@ static struct mfd_cell bxt_wc_dev[] = {
>  		.name = "bxt_wcove_thermal",
>  		.num_resources = ARRAY_SIZE(thermal_resources),
>  		.resources = thermal_resources,
> +		.platform_data = &bxtwc_thermal_data,
> +		.pdata_size = sizeof(bxtwc_thermal_data),
>  	},
>  	{
>  		.name = "bxt_wcove_ext_charger",
> diff --git a/include/linux/mfd/intel_soc_pmic.h b/include/linux/mfd/intel_soc_pmic.h
> index cf619db..7df4302 100644
> --- a/include/linux/mfd/intel_soc_pmic.h
> +++ b/include/linux/mfd/intel_soc_pmic.h
> @@ -21,6 +21,27 @@
>  
>  #include <linux/regmap.h>
>  
> +struct trip_config_map {
> +	u16 irq_reg;
> +	u16 irq_en;
> +	u16 evt_stat;
> +	u8 irq_mask;
> +	u8 irq_en_mask;
> +	u8 evt_mask;
> +	u8 trip_num;
> +};
> +
> +struct thermal_irq_map {
> +	char handle[20];
> +	int num_trips;
> +	struct trip_config_map *trip_config;
> +};
> +
> +struct pmic_thermal_data {
> +	struct thermal_irq_map *maps;
> +	int num_maps;
> +};
> +
>  struct intel_soc_pmic {
>  	int irq;
>  	struct regmap *regmap;

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] mfd: intel_soc_pmic_bxtwc: Add Intel BXT WhiskeyCove PMIC ADC thermal channel-zone mapping
  2016-06-17  8:01 ` Lee Jones
@ 2016-06-20  4:11   ` Bin Gao
  2016-06-20  8:49     ` Lee Jones
  0 siblings, 1 reply; 6+ messages in thread
From: Bin Gao @ 2016-06-20  4:11 UTC (permalink / raw)
  To: Lee Jones
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, ysiyer, Ajay Thomas,
	Bin Gao, broonie

On Fri, Jun 17, 2016 at 09:01:59AM +0100, Lee Jones wrote:
> > +static struct trip_config_map str0_trip_config[] = {
> > +	{
> > +		.irq_reg = BXTWC_THRM0IRQ,
> > +		.irq_mask = 0x01,
> > +		.irq_en = BXTWC_MTHRM0IRQ,
> > +		.irq_en_mask = 0x01,
> > +		.evt_stat = BXTWC_STHRM0IRQ,
> > +		.evt_mask = 0x01,
> > +		.trip_num = 0
> > +	},
> > +	{
> > +		.irq_reg = BXTWC_THRM0IRQ,
> > +		.irq_mask = 0x10,
> > +		.irq_en = BXTWC_MTHRM0IRQ,
> > +		.irq_en_mask = 0x10,
> > +		.evt_stat = BXTWC_STHRM0IRQ,
> > +		.evt_mask = 0x10,
> > +		.trip_num = 1
> > +	}
> > +};
> > +
> > +static struct trip_config_map str1_trip_config[] = {
> > +	{
> > +		.irq_reg = BXTWC_THRM0IRQ,
> > +		.irq_mask = 0x02,
> > +		.irq_en = BXTWC_MTHRM0IRQ,
> > +		.irq_en_mask = 0x02,
> > +		.evt_stat = BXTWC_STHRM0IRQ,
> > +		.evt_mask = 0x02,
> > +		.trip_num = 0
> > +	},
> > +	{
> > +		.irq_reg = BXTWC_THRM0IRQ,
> > +		.irq_mask = 0x20,
> > +		.irq_en = BXTWC_MTHRM0IRQ,
> > +		.irq_en_mask = 0x20,
> > +		.evt_stat = BXTWC_STHRM0IRQ,
> > +		.evt_mask = 0x20,
> > +		.trip_num = 1
> > +	},
> > +};
> > +
> > +static struct trip_config_map str2_trip_config[] = {
> > +	{
> > +		.irq_reg = BXTWC_THRM0IRQ,
> > +		.irq_mask = 0x04,
> > +		.irq_en = BXTWC_MTHRM0IRQ,
> > +		.irq_en_mask = 0x04,
> > +		.evt_stat = BXTWC_STHRM0IRQ,
> > +		.evt_mask = 0x04,
> > +		.trip_num = 0
> > +	},
> > +	{
> > +		.irq_reg = BXTWC_THRM0IRQ,
> > +		.irq_mask = 0x40,
> > +		.irq_en = BXTWC_MTHRM0IRQ,
> > +		.irq_en_mask = 0x40,
> > +		.evt_stat = BXTWC_STHRM0IRQ,
> > +		.evt_mask = 0x40,
> > +		.trip_num = 1
> > +	},
> > +};
> > +
> > +static struct trip_config_map str3_trip_config[] = {
> > +	{
> > +		.irq_reg = BXTWC_THRM2IRQ,
> > +		.irq_mask = 0x10,
> > +		.irq_en = BXTWC_MTHRM2IRQ,
> > +		.irq_en_mask = 0x10,
> > +		.evt_stat = BXTWC_STHRM2IRQ,
> > +		.evt_mask = 0x10,
> > +		.trip_num = 0
> > +	},
> > +};
> 
> This looks like a register map to me.
> 
> Can you use the regmap framework instead?

These are platform data used by another driver(thermal driver) which
uses regmap framework to access some of the fields of the structure(
irq_reg, irq_en and evt_stat).

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] mfd: intel_soc_pmic_bxtwc: Add Intel BXT WhiskeyCove PMIC ADC thermal channel-zone mapping
  2016-06-20  4:11   ` Bin Gao
@ 2016-06-20  8:49     ` Lee Jones
  2016-06-20  8:52       ` Lee Jones
  0 siblings, 1 reply; 6+ messages in thread
From: Lee Jones @ 2016-06-20  8:49 UTC (permalink / raw)
  To: Bin Gao
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, ysiyer, Ajay Thomas,
	Bin Gao, broonie

On Sun, 19 Jun 2016, Bin Gao wrote:

> On Fri, Jun 17, 2016 at 09:01:59AM +0100, Lee Jones wrote:
> > > +static struct trip_config_map str0_trip_config[] = {
> > > +	{
> > > +		.irq_reg = BXTWC_THRM0IRQ,
> > > +		.irq_mask = 0x01,
> > > +		.irq_en = BXTWC_MTHRM0IRQ,
> > > +		.irq_en_mask = 0x01,
> > > +		.evt_stat = BXTWC_STHRM0IRQ,
> > > +		.evt_mask = 0x01,
> > > +		.trip_num = 0
> > > +	},
> > > +	{
> > > +		.irq_reg = BXTWC_THRM0IRQ,
> > > +		.irq_mask = 0x10,
> > > +		.irq_en = BXTWC_MTHRM0IRQ,
> > > +		.irq_en_mask = 0x10,
> > > +		.evt_stat = BXTWC_STHRM0IRQ,
> > > +		.evt_mask = 0x10,
> > > +		.trip_num = 1
> > > +	}
> > > +};
> > > +
> > > +static struct trip_config_map str1_trip_config[] = {
> > > +	{
> > > +		.irq_reg = BXTWC_THRM0IRQ,
> > > +		.irq_mask = 0x02,
> > > +		.irq_en = BXTWC_MTHRM0IRQ,
> > > +		.irq_en_mask = 0x02,
> > > +		.evt_stat = BXTWC_STHRM0IRQ,
> > > +		.evt_mask = 0x02,
> > > +		.trip_num = 0
> > > +	},
> > > +	{
> > > +		.irq_reg = BXTWC_THRM0IRQ,
> > > +		.irq_mask = 0x20,
> > > +		.irq_en = BXTWC_MTHRM0IRQ,
> > > +		.irq_en_mask = 0x20,
> > > +		.evt_stat = BXTWC_STHRM0IRQ,
> > > +		.evt_mask = 0x20,
> > > +		.trip_num = 1
> > > +	},
> > > +};
> > > +
> > > +static struct trip_config_map str2_trip_config[] = {
> > > +	{
> > > +		.irq_reg = BXTWC_THRM0IRQ,
> > > +		.irq_mask = 0x04,
> > > +		.irq_en = BXTWC_MTHRM0IRQ,
> > > +		.irq_en_mask = 0x04,
> > > +		.evt_stat = BXTWC_STHRM0IRQ,
> > > +		.evt_mask = 0x04,
> > > +		.trip_num = 0
> > > +	},
> > > +	{
> > > +		.irq_reg = BXTWC_THRM0IRQ,
> > > +		.irq_mask = 0x40,
> > > +		.irq_en = BXTWC_MTHRM0IRQ,
> > > +		.irq_en_mask = 0x40,
> > > +		.evt_stat = BXTWC_STHRM0IRQ,
> > > +		.evt_mask = 0x40,
> > > +		.trip_num = 1
> > > +	},
> > > +};
> > > +
> > > +static struct trip_config_map str3_trip_config[] = {
> > > +	{
> > > +		.irq_reg = BXTWC_THRM2IRQ,
> > > +		.irq_mask = 0x10,
> > > +		.irq_en = BXTWC_MTHRM2IRQ,
> > > +		.irq_en_mask = 0x10,
> > > +		.evt_stat = BXTWC_STHRM2IRQ,
> > > +		.evt_mask = 0x10,
> > > +		.trip_num = 0
> > > +	},
> > > +};
> > 
> > This looks like a register map to me.
> > 
> > Can you use the regmap framework instead?
> 
> These are platform data used by another driver(thermal driver) which
> uses regmap framework to access some of the fields of the structure(
> irq_reg, irq_en and evt_stat).

I suggest you create the regmap here and pass it to the thermal driver
instead.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] mfd: intel_soc_pmic_bxtwc: Add Intel BXT WhiskeyCove PMIC ADC thermal channel-zone mapping
  2016-06-20  8:49     ` Lee Jones
@ 2016-06-20  8:52       ` Lee Jones
  2016-06-20 18:11         ` Bin Gao
  0 siblings, 1 reply; 6+ messages in thread
From: Lee Jones @ 2016-06-20  8:52 UTC (permalink / raw)
  To: Bin Gao
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, ysiyer, Ajay Thomas,
	Bin Gao, broonie

On Mon, 20 Jun 2016, Lee Jones wrote:
> On Sun, 19 Jun 2016, Bin Gao wrote:

> > On Fri, Jun 17, 2016 at 09:01:59AM +0100, Lee Jones wrote:
> > > > +static struct trip_config_map str0_trip_config[] = {
> > > > +	{
> > > > +		.irq_reg = BXTWC_THRM0IRQ,
> > > > +		.irq_mask = 0x01,
> > > > +		.irq_en = BXTWC_MTHRM0IRQ,
> > > > +		.irq_en_mask = 0x01,
> > > > +		.evt_stat = BXTWC_STHRM0IRQ,
> > > > +		.evt_mask = 0x01,
> > > > +		.trip_num = 0
> > > > +	},
> > > > +	{
> > > > +		.irq_reg = BXTWC_THRM0IRQ,
> > > > +		.irq_mask = 0x10,
> > > > +		.irq_en = BXTWC_MTHRM0IRQ,
> > > > +		.irq_en_mask = 0x10,
> > > > +		.evt_stat = BXTWC_STHRM0IRQ,
> > > > +		.evt_mask = 0x10,
> > > > +		.trip_num = 1
> > > > +	}
> > > > +};
> > > > +
> > > > +static struct trip_config_map str1_trip_config[] = {
> > > > +	{
> > > > +		.irq_reg = BXTWC_THRM0IRQ,
> > > > +		.irq_mask = 0x02,
> > > > +		.irq_en = BXTWC_MTHRM0IRQ,
> > > > +		.irq_en_mask = 0x02,
> > > > +		.evt_stat = BXTWC_STHRM0IRQ,
> > > > +		.evt_mask = 0x02,
> > > > +		.trip_num = 0
> > > > +	},
> > > > +	{
> > > > +		.irq_reg = BXTWC_THRM0IRQ,
> > > > +		.irq_mask = 0x20,
> > > > +		.irq_en = BXTWC_MTHRM0IRQ,
> > > > +		.irq_en_mask = 0x20,
> > > > +		.evt_stat = BXTWC_STHRM0IRQ,
> > > > +		.evt_mask = 0x20,
> > > > +		.trip_num = 1
> > > > +	},
> > > > +};
> > > > +
> > > > +static struct trip_config_map str2_trip_config[] = {
> > > > +	{
> > > > +		.irq_reg = BXTWC_THRM0IRQ,
> > > > +		.irq_mask = 0x04,
> > > > +		.irq_en = BXTWC_MTHRM0IRQ,
> > > > +		.irq_en_mask = 0x04,
> > > > +		.evt_stat = BXTWC_STHRM0IRQ,
> > > > +		.evt_mask = 0x04,
> > > > +		.trip_num = 0
> > > > +	},
> > > > +	{
> > > > +		.irq_reg = BXTWC_THRM0IRQ,
> > > > +		.irq_mask = 0x40,
> > > > +		.irq_en = BXTWC_MTHRM0IRQ,
> > > > +		.irq_en_mask = 0x40,
> > > > +		.evt_stat = BXTWC_STHRM0IRQ,
> > > > +		.evt_mask = 0x40,
> > > > +		.trip_num = 1
> > > > +	},
> > > > +};
> > > > +
> > > > +static struct trip_config_map str3_trip_config[] = {
> > > > +	{
> > > > +		.irq_reg = BXTWC_THRM2IRQ,
> > > > +		.irq_mask = 0x10,
> > > > +		.irq_en = BXTWC_MTHRM2IRQ,
> > > > +		.irq_en_mask = 0x10,
> > > > +		.evt_stat = BXTWC_STHRM2IRQ,
> > > > +		.evt_mask = 0x10,
> > > > +		.trip_num = 0
> > > > +	},
> > > > +};
> > > 
> > > This looks like a register map to me.
> > > 
> > > Can you use the regmap framework instead?
> > 
> > These are platform data used by another driver(thermal driver) which
> > uses regmap framework to access some of the fields of the structure(
> > irq_reg, irq_en and evt_stat).
> 
> I suggest you create the regmap here and pass it to the thermal driver
> instead.

Better yet, why don't you just create the regmap in the thermal
driver?  There is no need (in fact, it's not even allowed) to pass
register addresses though platform data.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] mfd: intel_soc_pmic_bxtwc: Add Intel BXT WhiskeyCove PMIC ADC thermal channel-zone mapping
  2016-06-20  8:52       ` Lee Jones
@ 2016-06-20 18:11         ` Bin Gao
  0 siblings, 0 replies; 6+ messages in thread
From: Bin Gao @ 2016-06-20 18:11 UTC (permalink / raw)
  To: Lee Jones
  Cc: Zhang Rui, Eduardo Valentin, linux-kernel, ysiyer, Ajay Thomas,
	Bin Gao, broonie

On Mon, Jun 20, 2016 at 09:52:00AM +0100, Lee Jones wrote:
> > > > > +static struct trip_config_map str3_trip_config[] = {
> > > > > +	{
> > > > > +		.irq_reg = BXTWC_THRM2IRQ,
> > > > > +		.irq_mask = 0x10,
> > > > > +		.irq_en = BXTWC_MTHRM2IRQ,
> > > > > +		.irq_en_mask = 0x10,
> > > > > +		.evt_stat = BXTWC_STHRM2IRQ,
> > > > > +		.evt_mask = 0x10,
> > > > > +		.trip_num = 0
> > > > > +	},
> > > > > +};
> > > > 
> > > > This looks like a register map to me.
> > > > 
> > > > Can you use the regmap framework instead?
> > > 
> > > These are platform data used by another driver(thermal driver) which
> > > uses regmap framework to access some of the fields of the structure(
> > > irq_reg, irq_en and evt_stat).
> > 
> > I suggest you create the regmap here and pass it to the thermal driver
> > instead.
> 
> Better yet, why don't you just create the regmap in the thermal
> driver?  There is no need (in fact, it's not even allowed) to pass
> register addresses though platform data.

We did implement regmap in the acpi opregion driver(see drivers/acpi/pmic
for details) for the PMIC device. Each individual driver, e.g. gpio,
thermal, etc. will only access registers that belong to its logic unit.
Since the shared regmap implementation is already there, a single individual
driver doesn't need to implement again.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-06-20 18:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-16 21:58 [PATCH v2] mfd: intel_soc_pmic_bxtwc: Add Intel BXT WhiskeyCove PMIC ADC thermal channel-zone mapping Bin Gao
2016-06-17  8:01 ` Lee Jones
2016-06-20  4:11   ` Bin Gao
2016-06-20  8:49     ` Lee Jones
2016-06-20  8:52       ` Lee Jones
2016-06-20 18:11         ` Bin Gao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).