From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752894AbcGUAEf (ORCPT ); Wed, 20 Jul 2016 20:04:35 -0400 Received: from mga14.intel.com ([192.55.52.115]:18594 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752610AbcGUAEe (ORCPT ); Wed, 20 Jul 2016 20:04:34 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,396,1464678000"; d="scan'208";a="1010892407" Date: Wed, 20 Jul 2016 17:08:55 -0700 From: Bin Gao To: Lee Jones 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: <20160721000855.GA211765@worksta> References: <20160628004406.GB9303@worksta> <20160628145849.GG24982@dell> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160628145849.GG24982@dell> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > 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. > I have moved all the register maps to the thermal driver. Please ignore this patch. Thanks for your review. -Bin