From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752321AbcF1O6H (ORCPT ); Tue, 28 Jun 2016 10:58:07 -0400 Received: from mail-wm0-f49.google.com ([74.125.82.49]:36957 "EHLO mail-wm0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751952AbcF1O6F (ORCPT ); Tue, 28 Jun 2016 10:58:05 -0400 Date: Tue, 28 Jun 2016 15:58:49 +0100 From: Lee Jones To: Bin Gao Cc: Zhang Rui , Qipeng Zha , Eduardo Valentin , linux-kernel@vger.kernel.org, ysiyer , Ajay Thomas , Bin Gao Subject: Re: [PATCH v4] mfd: intel_soc_pmic_bxtwc: Add Intel BXT WhiskeyCove PMIC ADC thermal channel mapping and USB type-C resources Message-ID: <20160628145849.GG24982@dell> References: <20160628004406.GB9303@worksta> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20160628004406.GB9303@worksta> User-Agent: Mutt/1.6.1 (2016-04-27) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > Signed-off-by: Rohit S Kenchanpura > Signed-off-by: Bin Gao > --- > 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 > > +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