linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] mfd: intel_soc_pmic_bxtwc: Add Intel BXT WhiskeyCove PMIC ADC thermal channel mapping and USB type-C resources
@ 2016-06-28  0:44 Bin Gao
  2016-06-28 14:58 ` Lee Jones
  0 siblings, 1 reply; 3+ messages in thread
From: Bin Gao @ 2016-06-28  0:44 UTC (permalink / raw)
  To: Zhang Rui, Qipeng Zha, Lee Jones, Eduardo Valentin, linux-kernel
  Cc: ysiyer, Ajay Thomas, Bin Gao

This patch adds the mapping of PMIC ADC channel to thermal zone and
USB type-C resources. 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: Rohit S Kenchanpura <rohit.s.kenchanpura@intel.com>
Signed-off-by: Bin Gao <bin.gao@intel.com>
---
Changes in v4:
 - Extended existing regmap reg instead of defining a new one:
   REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 5, 0x3f)
Changes in v3:
 - Added USB type-C resources.
Changes in v2:
 - Fixed subject line.
 - Combined two patches into one.
 drivers/mfd/intel_soc_pmic_bxtwc.c | 126 ++++++++++++++++++++++++++++++++++++-
 include/linux/mfd/intel_soc_pmic.h |  21 +++++++
 2 files changed, 146 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index b942876..e69eb86 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -47,6 +47,8 @@
 #define BXTWC_MIRQLVL1		0x4E0E
 #define BXTWC_MPWRTNIRQ		0x4E0F
 
+#define BXTWC_MIRQLVL1_MCHGR	BIT(5)
+
 #define BXTWC_MTHRM0IRQ		0x4E12
 #define BXTWC_MTHRM1IRQ		0x4E13
 #define BXTWC_MTHRM2IRQ		0x4E14
@@ -58,6 +60,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
 
@@ -109,13 +115,116 @@ static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
 	REGMAP_IRQ_REG(BXTWC_THRM2_IRQ, 2, 0xff),
 	REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 3, 0x1f),
 	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 4, 0xff),
-	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 5, 0x1f),
+	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 5, 0x3f),
 	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 6, 0x1f),
 	REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 7, 0xff),
 	REGMAP_IRQ_REG(BXTWC_GPIO1_IRQ, 8, 0x3f),
 	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,
@@ -143,6 +252,10 @@ static struct resource adc_resources[] = {
 	DEFINE_RES_IRQ_NAMED(BXTWC_ADC_IRQ, "ADC"),
 };
 
+static struct resource usbc_resources[] = {
+	DEFINE_RES_IRQ_NAMED(BXTWC_CHGR0_IRQ, "USBC"),
+};
+
 static struct resource charger_resources[] = {
 	DEFINE_RES_IRQ_NAMED(BXTWC_CHGR0_IRQ, "CHARGER"),
 	DEFINE_RES_IRQ_NAMED(BXTWC_CHGR1_IRQ, "CHARGER1"),
@@ -168,6 +281,13 @@ 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_usbc",
+		.num_resources = ARRAY_SIZE(usbc_resources),
+		.resources = usbc_resources,
 	},
 	{
 		.name = "bxt_wcove_ext_charger",
@@ -403,6 +523,10 @@ static int bxtwc_probe(struct platform_device *pdev)
 		goto err_sysfs;
 	}
 
+	/* Unmask CHRG first level irq */
+	regmap_update_bits(pmic->regmap, BXTWC_MIRQLVL1,
+				BXTWC_MIRQLVL1_MCHGR, 0);
+
 	return 0;
 
 err_sysfs:
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] 3+ messages in thread

* Re: [PATCH v4] mfd: intel_soc_pmic_bxtwc: Add Intel BXT WhiskeyCove PMIC ADC thermal channel mapping and USB type-C resources
  2016-06-28  0:44 [PATCH v4] mfd: intel_soc_pmic_bxtwc: Add Intel BXT WhiskeyCove PMIC ADC thermal channel mapping and USB type-C resources Bin Gao
@ 2016-06-28 14:58 ` Lee Jones
  2016-07-21  0:08   ` Bin Gao
  0 siblings, 1 reply; 3+ messages in thread
From: Lee Jones @ 2016-06-28 14:58 UTC (permalink / raw)
  To: Bin Gao
  Cc: Zhang Rui, Qipeng Zha, Eduardo Valentin, linux-kernel, ysiyer,
	Ajay Thomas, Bin Gao

On Mon, 27 Jun 2016, Bin Gao wrote:

> This patch adds the mapping of PMIC ADC channel to thermal zone and
> USB type-C resources. 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.

No need for this section.

> Signed-off-by: Yegnesh S Iyer <yegnesh.s.iyer@intel.com>
> Signed-off-by: Rohit S Kenchanpura <rohit.s.kenchanpura@intel.com>
> Signed-off-by: Bin Gao <bin.gao@intel.com>
> ---
> Changes in v4:
>  - Extended existing regmap reg instead of defining a new one:
>    REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 5, 0x3f)
> Changes in v3:
>  - Added USB type-C resources.
> Changes in v2:
>  - Fixed subject line.
>  - Combined two patches into one.
>  drivers/mfd/intel_soc_pmic_bxtwc.c | 126 ++++++++++++++++++++++++++++++++++++-
>  include/linux/mfd/intel_soc_pmic.h |  21 +++++++
>  2 files changed, 146 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
> index b942876..e69eb86 100644
> --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
> +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
> @@ -47,6 +47,8 @@
>  #define BXTWC_MIRQLVL1		0x4E0E
>  #define BXTWC_MPWRTNIRQ		0x4E0F
>  
> +#define BXTWC_MIRQLVL1_MCHGR	BIT(5)
> +

This line looks very much out of place here.

>  #define BXTWC_MTHRM0IRQ		0x4E12
>  #define BXTWC_MTHRM1IRQ		0x4E13
>  #define BXTWC_MTHRM2IRQ		0x4E14
> @@ -58,6 +60,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
>  
> @@ -109,13 +115,116 @@ static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
>  	REGMAP_IRQ_REG(BXTWC_THRM2_IRQ, 2, 0xff),
>  	REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 3, 0x1f),
>  	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 4, 0xff),
> -	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 5, 0x1f),
> +	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 5, 0x3f),

Is the original code wrong?

If so, it's a fix and needs it's own patch
.
>  	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 6, 0x1f),
>  	REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 7, 0xff),
>  	REGMAP_IRQ_REG(BXTWC_GPIO1_IRQ, 8, 0x3f),
>  	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
> +	},
> +};

I thought I'd told you about this already?  I don't want you to be
passing though register maps using your own hand-rolled solution.

Either use Regmap and pass through the pointer, or pass some kind of
device identifier and choose the correct mapping from within the
Thermal driver.

> +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,
> @@ -143,6 +252,10 @@ static struct resource adc_resources[] = {
>  	DEFINE_RES_IRQ_NAMED(BXTWC_ADC_IRQ, "ADC"),
>  };
>  
> +static struct resource usbc_resources[] = {
> +	DEFINE_RES_IRQ_NAMED(BXTWC_CHGR0_IRQ, "USBC"),
> +};
> +
>  static struct resource charger_resources[] = {
>  	DEFINE_RES_IRQ_NAMED(BXTWC_CHGR0_IRQ, "CHARGER"),
>  	DEFINE_RES_IRQ_NAMED(BXTWC_CHGR1_IRQ, "CHARGER1"),
> @@ -168,6 +281,13 @@ 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_usbc",
> +		.num_resources = ARRAY_SIZE(usbc_resources),
> +		.resources = usbc_resources,
>  	},
>  	{
>  		.name = "bxt_wcove_ext_charger",
> @@ -403,6 +523,10 @@ static int bxtwc_probe(struct platform_device *pdev)
>  		goto err_sysfs;
>  	}
>  
> +	/* Unmask CHRG first level irq */
> +	regmap_update_bits(pmic->regmap, BXTWC_MIRQLVL1,
> +				BXTWC_MIRQLVL1_MCHGR, 0);
> +
>  	return 0;
>  
>  err_sysfs:
> 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] 3+ messages in thread

* Re: [PATCH v4] mfd: intel_soc_pmic_bxtwc: Add Intel BXT WhiskeyCove PMIC ADC thermal channel mapping and USB type-C resources
  2016-06-28 14:58 ` Lee Jones
@ 2016-07-21  0:08   ` Bin Gao
  0 siblings, 0 replies; 3+ messages in thread
From: Bin Gao @ 2016-07-21  0:08 UTC (permalink / raw)
  To: Lee Jones
  Cc: Zhang Rui, Qipeng Zha, Eduardo Valentin, linux-kernel, ysiyer,
	Ajay Thomas, Bin Gao

On Tue, Jun 28, 2016 at 03:58:49PM +0100, Lee Jones wrote:
> On Mon, 27 Jun 2016, Bin Gao wrote:
> 
> > This patch adds the mapping of PMIC ADC channel to thermal zone and
> > USB type-C resources. 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.
> 
> No need for this section.
> 
> > Signed-off-by: Yegnesh S Iyer <yegnesh.s.iyer@intel.com>
> > Signed-off-by: Rohit S Kenchanpura <rohit.s.kenchanpura@intel.com>
> > Signed-off-by: Bin Gao <bin.gao@intel.com>
> > ---
> > Changes in v4:
> >  - Extended existing regmap reg instead of defining a new one:
> >    REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 5, 0x3f)
> > Changes in v3:
> >  - Added USB type-C resources.
> > Changes in v2:
> >  - Fixed subject line.
> >  - Combined two patches into one.
> >  drivers/mfd/intel_soc_pmic_bxtwc.c | 126 ++++++++++++++++++++++++++++++++++++-
> >  include/linux/mfd/intel_soc_pmic.h |  21 +++++++
> >  2 files changed, 146 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
> > index b942876..e69eb86 100644
> > --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
> > +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
> > @@ -47,6 +47,8 @@
> >  #define BXTWC_MIRQLVL1		0x4E0E
> >  #define BXTWC_MPWRTNIRQ		0x4E0F
> >  
> > +#define BXTWC_MIRQLVL1_MCHGR	BIT(5)
> > +
> 
> This line looks very much out of place here.
> 
> >  #define BXTWC_MTHRM0IRQ		0x4E12
> >  #define BXTWC_MTHRM1IRQ		0x4E13
> >  #define BXTWC_MTHRM2IRQ		0x4E14
> > @@ -58,6 +60,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
> >  
> > @@ -109,13 +115,116 @@ static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
> >  	REGMAP_IRQ_REG(BXTWC_THRM2_IRQ, 2, 0xff),
> >  	REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 3, 0x1f),
> >  	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 4, 0xff),
> > -	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 5, 0x1f),
> > +	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 5, 0x3f),
> 
> Is the original code wrong?
> 
> If so, it's a fix and needs it's own patch
> .
> >  	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 6, 0x1f),
> >  	REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 7, 0xff),
> >  	REGMAP_IRQ_REG(BXTWC_GPIO1_IRQ, 8, 0x3f),
> >  	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
> > +	},
> > +};
> 
> I thought I'd told you about this already?  I don't want you to be
> passing though register maps using your own hand-rolled solution.
> 
> Either use Regmap and pass through the pointer, or pass some kind of
> device identifier and choose the correct mapping from within the
> Thermal driver.
> 

I have moved all the register maps to the thermal driver.
Please ignore this patch. Thanks for your review.

-Bin

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

end of thread, other threads:[~2016-07-21  0:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-28  0:44 [PATCH v4] mfd: intel_soc_pmic_bxtwc: Add Intel BXT WhiskeyCove PMIC ADC thermal channel mapping and USB type-C resources Bin Gao
2016-06-28 14:58 ` Lee Jones
2016-07-21  0:08   ` 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).