[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160628145849.GG24982@dell>
Date: Tue, 28 Jun 2016 15:58:49 +0100
From: Lee Jones <lee.jones@...aro.org>
To: Bin Gao <bin.gao@...ux.intel.com>
Cc: Zhang Rui <rui.zhang@...el.com>, Qipeng Zha <qipeng.zha@...el.com>,
Eduardo Valentin <edubezval@...il.com>,
linux-kernel@...r.kernel.org, ysiyer <yegnesh.s.iyer@...el.com>,
Ajay Thomas <ajay.thomas.david.rajamanickam@...el.com>,
Bin Gao <bin.gao@...el.com>
Subject: Re: [PATCH v4] mfd: intel_soc_pmic_bxtwc: Add Intel BXT WhiskeyCove
PMIC ADC thermal channel mapping and USB type-C resources
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@...el.com>
> Signed-off-by: Rohit S Kenchanpura <rohit.s.kenchanpura@...el.com>
> Signed-off-by: Bin Gao <bin.gao@...el.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
Powered by blists - more mailing lists