All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lee Jones <lee.jones@linaro.org>
To: Jerome Neanne <jneanne@baylibre.com>
Cc: lgirdwood@gmail.com, broonie@kernel.org, robh+dt@kernel.org,
	nm@ti.com, kristo@kernel.org, khilman@baylibre.com,
	narmstrong@baylibre.com, msp@baylibre.com, j-keerthy@ti.com,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, Lee Jones <lee@kernel.org>
Subject: Re: [PATCH v2 07/11] mfd: drivers: Add interrupts support to TI TPS65219 PMIC
Date: Mon, 8 Aug 2022 16:57:53 +0100	[thread overview]
Message-ID: <YvEygQNmLMNV5CiV@google.com> (raw)
In-Reply-To: <20220726103355.17684-8-jneanne@baylibre.com>

On Tue, 26 Jul 2022, Jerome Neanne wrote:

> Add TPS65219 support for interrupts through regmap_irq_chip

Why is this separate to the original patch?

> Signed-off-by: Jerome Neanne <jneanne@baylibre.com>
> ---
>  drivers/mfd/tps65219.c       | 217 ++++++++++++++++++++++++++++++++++-
>  include/linux/mfd/tps65219.h | 113 ++++++++++++++++++
>  2 files changed, 328 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/tps65219.c b/drivers/mfd/tps65219.c
> index c3bf975ea6c7..7366f251c21d 100644
> --- a/drivers/mfd/tps65219.c
> +++ b/drivers/mfd/tps65219.c
> @@ -107,8 +107,73 @@ static void pmic_do_poweroff(void)
>  	tps65219_soft_shutdown(tps);
>  }
>  
> -static const struct mfd_cell tps65219_cells[] = {
> -	{ .name = "tps65219-regulator", },
> +static const struct resource tps65219_pwrbutton_resources[] = {
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_PB_FALLING_EDGE_DETECT, "falling"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_PB_RISING_EDGE_DETECT, "rising"),
> +};
> +
> +static const struct resource tps65219_regulator_resources[] = {
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO3_SCG, "LDO3_SCG"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO3_OC, "LDO3_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO3_UV, "LDO3_UV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO4_SCG, "LDO4_SCG"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO4_OC, "LDO4_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO4_UV, "LDO4_UV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO1_SCG, "LDO1_SCG"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO1_OC, "LDO1_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO1_UV, "LDO1_UV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO2_SCG, "LDO2_SCG"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO2_OC, "LDO2_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO2_UV, "LDO2_UV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK3_SCG, "BUCK3_SCG"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK3_OC, "BUCK3_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK3_NEG_OC, "BUCK3_NEG_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK3_UV, "BUCK3_UV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK1_SCG, "BUCK1_SCG"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK1_OC, "BUCK1_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK1_NEG_OC, "BUCK1_NEG_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK1_UV, "BUCK1_UV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK2_SCG, "BUCK2_SCG"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK2_OC, "BUCK2_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK2_NEG_OC, "BUCK2_NEG_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK2_UV, "BUCK2_UV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK1_RV, "BUCK1_RV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK2_RV, "BUCK2_RV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK3_RV, "BUCK3_RV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO1_RV, "LDO1_RV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO2_RV, "LDO2_RV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO3_RV, "LDO3_RV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO4_RV, "LDO4_RV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK1_RV_SD, "BUCK1_RV_SD"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK2_RV_SD, "BUCK2_RV_SD"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK3_RV_SD, "BUCK3_RV_SD"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO1_RV_SD, "LDO1_RV_SD"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO2_RV_SD, "LDO2_RV_SD"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO3_RV_SD, "LDO3_RV_SD"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO4_RV_SD, "LDO4_RV_SD"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_TIMEOUT, "TIMEOUT"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_SENSOR_3_WARM, "SENSOR_3_WARM"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_SENSOR_2_WARM, "SENSOR_2_WARM"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_SENSOR_1_WARM, "SENSOR_1_WARM"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_SENSOR_0_WARM, "SENSOR_0_WARM"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_SENSOR_3_HOT, "SENSOR_3_HOT"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_SENSOR_2_HOT, "SENSOR_2_HOT"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_SENSOR_1_HOT, "SENSOR_1_HOT"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_SENSOR_0_HOT, "SENSOR_0_HOT"),
> +};
> +
> +#define TPS65219_MAX_CELLS 2

Where is this even used?

> +static const struct mfd_cell tps65219_regulator_cell = {
> +	.name = "tps65219-regulator",
> +	.resources = tps65219_regulator_resources,
> +	.num_resources = ARRAY_SIZE(tps65219_regulator_resources),
> +};
> +
> +static const struct mfd_cell tps65219_pwrbutton_cell = {
> +	.name = "tps65219-pwrbutton",
> +	.resources = tps65219_pwrbutton_resources,
> +	.num_resources = ARRAY_SIZE(tps65219_pwrbutton_resources),
>  };
>  
>  static const struct regmap_config tps65219_regmap_config = {
> @@ -117,6 +182,147 @@ static const struct regmap_config tps65219_regmap_config = {
>  	.max_register = TPS65219_REG_FACTORY_CONFIG_2,
>  };
>  
> +/*
> + * Mapping of main IRQ register bits to sub-IRQ register offsets so that we can
> + * access corect sub-IRQ registers based on bits that are set in main IRQ
> + * register.
> + */
> +/* Timeout Residual Voltage Shutdown */
> +static unsigned int bit0_offsets[] = {TPS65219_TO_RV_POS};
> +static unsigned int bit1_offsets[] = {TPS65219_RV_POS};	/* Residual Voltage */
> +static unsigned int bit2_offsets[] = {TPS65219_SYS_POS};	/* System */
> +static unsigned int bit3_offsets[] = {TPS65219_BUCK_1_2_POS};	/* Buck 1-2 */
> +static unsigned int bit4_offsets[] = {TPS65219_BUCK_3_POS};	/* Buck 3 */
> +static unsigned int bit5_offsets[] = {TPS65219_LDO_1_2_POS};	/* LDO 1-2 */
> +static unsigned int bit6_offsets[] = {TPS65219_LDO_3_4_POS};	/* LDO 3-4 */
> +static unsigned int bit7_offsets[] = {TPS65219_PB_POS};	/* Power Button */

Well this is a pretty miserable interface.

You probably need spaces after { and before }.

> +static struct regmap_irq_sub_irq_map tps65219_sub_irq_offsets[] = {
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bit0_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bit1_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bit2_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bit3_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bit4_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bit5_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bit6_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bit7_offsets),
> +};
> +
> +static struct regmap_irq tps65219_irqs[] = {
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO3_SCG, TPS65219_LDO_3_4_POS,
> +		       TPS65219_INT_LDO3_SCG_MASK),

One line please.

> +	REGMAP_IRQ_REG(TPS65219_INT_LDO3_OC,
> +		       TPS65219_LDO_3_4_POS, TPS65219_INT_LDO3_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO3_UV, TPS65219_LDO_3_4_POS,
> +		       TPS65219_INT_LDO3_UV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO4_SCG, TPS65219_LDO_3_4_POS,
> +		       TPS65219_INT_LDO4_SCG_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO4_OC, TPS65219_LDO_3_4_POS,
> +		       TPS65219_INT_LDO4_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO4_UV, TPS65219_LDO_3_4_POS,
> +		       TPS65219_INT_LDO4_UV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO1_SCG,
> +		       TPS65219_LDO_1_2_POS, TPS65219_INT_LDO1_SCG_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO1_OC, TPS65219_LDO_1_2_POS,
> +		       TPS65219_INT_LDO1_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO1_UV, TPS65219_LDO_1_2_POS,
> +		       TPS65219_INT_LDO1_UV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO2_SCG, TPS65219_LDO_1_2_POS,
> +		       TPS65219_INT_LDO2_SCG_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO2_OC, TPS65219_LDO_1_2_POS,
> +		       TPS65219_INT_LDO2_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO2_UV, TPS65219_LDO_1_2_POS,
> +		       TPS65219_INT_LDO2_UV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK3_SCG, TPS65219_BUCK_3_POS,
> +		       TPS65219_INT_BUCK3_SCG_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK3_OC, TPS65219_BUCK_3_POS,
> +		       TPS65219_INT_BUCK3_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK3_NEG_OC, TPS65219_BUCK_3_POS,
> +		       TPS65219_INT_BUCK3_NEG_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK3_UV, TPS65219_BUCK_3_POS,
> +		       TPS65219_INT_BUCK3_UV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK2_SCG, TPS65219_BUCK_1_2_POS,
> +		       TPS65219_INT_BUCK2_SCG_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK2_OC, TPS65219_BUCK_1_2_POS,
> +		       TPS65219_INT_BUCK2_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK2_NEG_OC, TPS65219_BUCK_1_2_POS,
> +		       TPS65219_INT_BUCK2_NEG_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK2_UV, TPS65219_BUCK_1_2_POS,
> +		       TPS65219_INT_BUCK2_UV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK1_SCG, TPS65219_BUCK_1_2_POS,
> +		       TPS65219_INT_BUCK1_SCG_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK1_OC, TPS65219_BUCK_1_2_POS,
> +		       TPS65219_INT_BUCK1_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK1_NEG_OC, TPS65219_BUCK_1_2_POS,
> +		       TPS65219_INT_BUCK1_NEG_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK1_UV, TPS65219_BUCK_1_2_POS,
> +		       TPS65219_INT_BUCK1_UV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_SENSOR_3_WARM,
> +		       TPS65219_SYS_POS, TPS65219_INT_SENSOR_3_WARM_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_SENSOR_2_WARM, TPS65219_SYS_POS,
> +		       TPS65219_INT_SENSOR_2_WARM_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_SENSOR_1_WARM, TPS65219_SYS_POS,
> +		       TPS65219_INT_SENSOR_1_WARM_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_SENSOR_0_WARM, TPS65219_SYS_POS,
> +		       TPS65219_INT_SENSOR_0_WARM_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_SENSOR_3_HOT, TPS65219_SYS_POS,
> +		       TPS65219_INT_SENSOR_3_HOT_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_SENSOR_2_HOT, TPS65219_SYS_POS,
> +		       TPS65219_INT_SENSOR_2_HOT_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_SENSOR_1_HOT, TPS65219_SYS_POS,
> +		       TPS65219_INT_SENSOR_1_HOT_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_SENSOR_0_HOT, TPS65219_SYS_POS,
> +		       TPS65219_INT_SENSOR_0_HOT_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK1_RV, TPS65219_RV_POS,
> +		       TPS65219_INT_BUCK1_RV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK2_RV, TPS65219_RV_POS,
> +		       TPS65219_INT_BUCK2_RV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK3_RV, TPS65219_RV_POS,
> +		       TPS65219_INT_BUCK3_RV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO1_RV, TPS65219_RV_POS,
> +		       TPS65219_INT_LDO1_RV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO2_RV, TPS65219_RV_POS,
> +		       TPS65219_INT_LDO2_RV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO3_RV, TPS65219_RV_POS,
> +		       TPS65219_INT_LDO3_RV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO4_RV, TPS65219_RV_POS,
> +		       TPS65219_INT_LDO4_RV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK1_RV_SD,
> +		       TPS65219_TO_RV_POS, TPS65219_INT_BUCK1_RV_SD_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK2_RV_SD,
> +		       TPS65219_TO_RV_POS, TPS65219_INT_BUCK2_RV_SD_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK3_RV_SD, TPS65219_TO_RV_POS,
> +		       TPS65219_INT_BUCK3_RV_SD_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO1_RV_SD, TPS65219_TO_RV_POS,
> +		       TPS65219_INT_LDO1_RV_SD_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO2_RV_SD, TPS65219_TO_RV_POS,
> +		       TPS65219_INT_LDO2_RV_SD_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO3_RV_SD,
> +		       TPS65219_TO_RV_POS, TPS65219_INT_LDO3_RV_SD_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO4_RV_SD, TPS65219_TO_RV_POS,
> +		       TPS65219_INT_LDO4_RV_SD_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_TIMEOUT, TPS65219_TO_RV_POS,
> +		       TPS65219_INT_TIMEOUT_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_PB_FALLING_EDGE_DETECT,
> +		       TPS65219_PB_POS, TPS65219_INT_PB_FALLING_EDGE_DET_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_PB_RISING_EDGE_DETECT, TPS65219_PB_POS,
> +		       TPS65219_INT_PB_RISING_EDGE_DET_MASK),
> +};
> +
> +static struct regmap_irq_chip tps65219_irq_chip = {
> +	.name = "tps65219_irq",
> +	.main_status = TPS65219_REG_INT_SOURCE,
> +	.num_main_regs = 1,
> +	.num_main_status_bits = 8,
> +	.irqs = tps65219_irqs,
> +	.num_irqs = ARRAY_SIZE(tps65219_irqs),
> +	.status_base = TPS65219_REG_INT_LDO_3_4,
> +	.ack_base = TPS65219_REG_INT_LDO_3_4,
> +	.clear_ack = 1,
> +	.num_regs = 8,
> +	.sub_reg_offsets = &tps65219_sub_irq_offsets[0],
> +};
> +
>  static const struct of_device_id of_tps65219_match_table[] = {
>  	{ .compatible = "ti,tps65219", },
>  	{}
> @@ -137,6 +343,7 @@ static int tps65219_probe(struct i2c_client *client,
>  
>  	i2c_set_clientdata(client, tps);
>  	tps->dev = &client->dev;
> +	tps->irq = client->irq;

Is this used again beyond devm_regmap_add_irq_chip()?

>  	tps->regmap = devm_regmap_init_i2c(client, &tps65219_regmap_config);
>  	if (IS_ERR(tps->regmap)) {
>  		ret = PTR_ERR(tps->regmap);
> @@ -145,6 +352,12 @@ static int tps65219_probe(struct i2c_client *client,
>  		return ret;
>  	}
>  
> +	ret = devm_regmap_add_irq_chip(&client->dev, tps->regmap, tps->irq,
> +				       IRQF_ONESHOT, 0, &tps65219_irq_chip,
> +				       &tps->irq_data);

2 lines.

> +	if (ret)
> +		return ret;
> +
>  	ret = regmap_read(tps->regmap, TPS65219_REG_TI_DEV_ID, &chipid);
>  	if (ret) {
>  		dev_err(tps->dev, "Failed to read device ID: %d\n", ret);
> diff --git a/include/linux/mfd/tps65219.h b/include/linux/mfd/tps65219.h
> index e9197ab8bc75..8cecaf9bc682 100644
> --- a/include/linux/mfd/tps65219.h
> +++ b/include/linux/mfd/tps65219.h
> @@ -214,6 +214,104 @@
>  #define TPS65219_INT_PB_RISING_EDGE_DET_MASK		BIT(1)
>  #define TPS65219_INT_PB_REAL_TIME_STATUS_MASK		BIT(2)
>  
> +/* Masks for main IRQ register bits */
> +enum {
> +	TPS65219_INT_TIMEOUT_RV_SD,
> +#define TPS65219_INT_TIMEOUT_RV_SD_MASK BIT(TPS65219_INT_TIMEOUT_RV_SD)
> +	TPS65219_INT_RV,
> +#define TPS65219_INT_RV_MASK BIT(TPS65219_INT_RV)
> +	TPS65219_INT_SYSTEM,
> +#define TPS65219_INT_SYSTEM_MASK BIT(TPS65219_INT_SYSTEM)
> +	TPS65219_INT_BUCK_1_2,
> +#define TPS65219_INT_BUCK_1_2_MASK BIT(TPS65219_INT_BUCK_1_2)
> +	TPS65219_INT_BUCK_3,
> +#define TPS65219_INT_BUCK_3_MASK BIT(TPS65219_INT_BUCK_3)
> +	TPS65219_INT_LDO_1_2,
> +#define TPS65219_INT_LDO_1_2_MASK BIT(TPS65219_INT_LDO_1_2)
> +	TPS65219_INT_LDO_3_4,
> +#define TPS65219_INT_LDO_3_4_MASK BIT(TPS65219_LDO_3_4)
> +	TPS65219_INT_PB,
> +#define TPS65219_INT_PB_MASK BIT(TPS65219_INT_PB)

Where are these horrid defines used?

Can't you just use the BIT() macros.  We know what they mean.

> +};
> +
> +/* Timeout Residual Voltage Shutdown */

If you used nice nomenclature, you wouldn't need the comments.

> +#define TPS65219_TO_RV_POS 6
> +/* Residual Voltage */
> +#define TPS65219_RV_POS 5
> +/* System */
> +#define TPS65219_SYS_POS 4
> +/* Buck 1-2 */
> +#define TPS65219_BUCK_1_2_POS 3
> +/* Buck 3 */
> +#define TPS65219_BUCK_3_POS 2
> +/* LDO 1-2 */
> +#define TPS65219_LDO_1_2_POS 1
> +/* LDO 3-4 */
> +#define TPS65219_LDO_3_4_POS 0
> +/* Power Button */
> +#define TPS65219_PB_POS 7
> +
> +/* IRQs */
> +enum {
> +	/* LDO3-4 register IRQs */
> +	TPS65219_INT_LDO3_SCG,
> +	TPS65219_INT_LDO3_OC,
> +	TPS65219_INT_LDO3_UV,
> +	TPS65219_INT_LDO4_SCG,
> +	TPS65219_INT_LDO4_OC,
> +	TPS65219_INT_LDO4_UV,
> +	/* LDO1-2 */
> +	TPS65219_INT_LDO1_SCG,
> +	TPS65219_INT_LDO1_OC,
> +	TPS65219_INT_LDO1_UV,
> +	TPS65219_INT_LDO2_SCG,
> +	TPS65219_INT_LDO2_OC,
> +	TPS65219_INT_LDO2_UV,
> +	/* BUCK3 */
> +	TPS65219_INT_BUCK3_SCG,
> +	TPS65219_INT_BUCK3_OC,
> +	TPS65219_INT_BUCK3_NEG_OC,
> +	TPS65219_INT_BUCK3_UV,
> +	/* BUCK1-2 */
> +	TPS65219_INT_BUCK1_SCG,
> +	TPS65219_INT_BUCK1_OC,
> +	TPS65219_INT_BUCK1_NEG_OC,
> +	TPS65219_INT_BUCK1_UV,
> +	TPS65219_INT_BUCK2_SCG,
> +	TPS65219_INT_BUCK2_OC,
> +	TPS65219_INT_BUCK2_NEG_OC,
> +	TPS65219_INT_BUCK2_UV,
> +	/* Thermal Sensor  */
> +	TPS65219_INT_SENSOR_3_WARM,
> +	TPS65219_INT_SENSOR_2_WARM,
> +	TPS65219_INT_SENSOR_1_WARM,
> +	TPS65219_INT_SENSOR_0_WARM,
> +	TPS65219_INT_SENSOR_3_HOT,
> +	TPS65219_INT_SENSOR_2_HOT,
> +	TPS65219_INT_SENSOR_1_HOT,
> +	TPS65219_INT_SENSOR_0_HOT,
> +	/* Residual Voltage */
> +	TPS65219_INT_BUCK1_RV,
> +	TPS65219_INT_BUCK2_RV,
> +	TPS65219_INT_BUCK3_RV,
> +	TPS65219_INT_LDO1_RV,
> +	TPS65219_INT_LDO2_RV,
> +	TPS65219_INT_LDO3_RV,
> +	TPS65219_INT_LDO4_RV,
> +	/* Residual Voltage ShutDown */
> +	TPS65219_INT_BUCK1_RV_SD,
> +	TPS65219_INT_BUCK2_RV_SD,
> +	TPS65219_INT_BUCK3_RV_SD,
> +	TPS65219_INT_LDO1_RV_SD,
> +	TPS65219_INT_LDO2_RV_SD,
> +	TPS65219_INT_LDO3_RV_SD,
> +	TPS65219_INT_LDO4_RV_SD,
> +	TPS65219_INT_TIMEOUT,
> +	/* Power Button */
> +	TPS65219_INT_PB_FALLING_EDGE_DETECT,
> +	TPS65219_INT_PB_RISING_EDGE_DETECT,
> +};
> +
>  enum tps65219_regulator_id {
>  	/* DCDC's */
>  	TPS65219_BUCK_1,
> @@ -235,6 +333,19 @@ enum tps65219_regulator_id {
>  /* Number of total regulators available */
>  #define TPS65219_NUM_REGULATOR		(TPS65219_NUM_DCDC + TPS65219_NUM_LDO)
>  
> +/* Define the TPS65219 IRQ numbers */
> +enum tps65219_irqs {
> +	/* INT source registers */
> +	TPS65219_TO_RV_SD_SET_IRQ,
> +	TPS65219_RV_SET_IRQ,
> +	TPS65219_SYS_SET_IRQ,
> +	TPS65219_BUCK_1_2_SET_IRQ,
> +	TPS65219_BUCK_3_SET_IRQ,
> +	TPS65219_LDO_1_2_SET_IRQ,
> +	TPS65219_LDO_3_4_SET_IRQ,
> +	TPS65219_PB_SET_IRQ,
> +};
> +
>  /**
>   * struct tps65219 - tps65219 sub-driver chip access routines
>   *
> @@ -243,6 +354,8 @@ enum tps65219_regulator_id {
>  struct tps65219 {
>  	struct device *dev;
>  	unsigned int id;
> +	int irq;
> +	struct regmap_irq_chip_data *irq_data;
>  	struct regulator_desc desc[TPS65219_NUM_REGULATOR];
>  	struct regmap *regmap;
>  	struct notifier_block nb;

-- 
DEPRECATED: Please use lee@kernel.org

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Lee Jones <lee.jones@linaro.org>
To: Jerome Neanne <jneanne@baylibre.com>
Cc: lgirdwood@gmail.com, broonie@kernel.org, robh+dt@kernel.org,
	nm@ti.com, kristo@kernel.org, khilman@baylibre.com,
	narmstrong@baylibre.com, msp@baylibre.com, j-keerthy@ti.com,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, Lee Jones <lee@kernel.org>
Subject: Re: [PATCH v2 07/11] mfd: drivers: Add interrupts support to TI TPS65219 PMIC
Date: Mon, 8 Aug 2022 16:57:53 +0100	[thread overview]
Message-ID: <YvEygQNmLMNV5CiV@google.com> (raw)
In-Reply-To: <20220726103355.17684-8-jneanne@baylibre.com>

On Tue, 26 Jul 2022, Jerome Neanne wrote:

> Add TPS65219 support for interrupts through regmap_irq_chip

Why is this separate to the original patch?

> Signed-off-by: Jerome Neanne <jneanne@baylibre.com>
> ---
>  drivers/mfd/tps65219.c       | 217 ++++++++++++++++++++++++++++++++++-
>  include/linux/mfd/tps65219.h | 113 ++++++++++++++++++
>  2 files changed, 328 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/tps65219.c b/drivers/mfd/tps65219.c
> index c3bf975ea6c7..7366f251c21d 100644
> --- a/drivers/mfd/tps65219.c
> +++ b/drivers/mfd/tps65219.c
> @@ -107,8 +107,73 @@ static void pmic_do_poweroff(void)
>  	tps65219_soft_shutdown(tps);
>  }
>  
> -static const struct mfd_cell tps65219_cells[] = {
> -	{ .name = "tps65219-regulator", },
> +static const struct resource tps65219_pwrbutton_resources[] = {
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_PB_FALLING_EDGE_DETECT, "falling"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_PB_RISING_EDGE_DETECT, "rising"),
> +};
> +
> +static const struct resource tps65219_regulator_resources[] = {
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO3_SCG, "LDO3_SCG"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO3_OC, "LDO3_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO3_UV, "LDO3_UV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO4_SCG, "LDO4_SCG"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO4_OC, "LDO4_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO4_UV, "LDO4_UV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO1_SCG, "LDO1_SCG"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO1_OC, "LDO1_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO1_UV, "LDO1_UV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO2_SCG, "LDO2_SCG"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO2_OC, "LDO2_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO2_UV, "LDO2_UV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK3_SCG, "BUCK3_SCG"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK3_OC, "BUCK3_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK3_NEG_OC, "BUCK3_NEG_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK3_UV, "BUCK3_UV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK1_SCG, "BUCK1_SCG"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK1_OC, "BUCK1_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK1_NEG_OC, "BUCK1_NEG_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK1_UV, "BUCK1_UV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK2_SCG, "BUCK2_SCG"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK2_OC, "BUCK2_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK2_NEG_OC, "BUCK2_NEG_OC"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK2_UV, "BUCK2_UV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK1_RV, "BUCK1_RV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK2_RV, "BUCK2_RV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK3_RV, "BUCK3_RV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO1_RV, "LDO1_RV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO2_RV, "LDO2_RV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO3_RV, "LDO3_RV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO4_RV, "LDO4_RV"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK1_RV_SD, "BUCK1_RV_SD"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK2_RV_SD, "BUCK2_RV_SD"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_BUCK3_RV_SD, "BUCK3_RV_SD"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO1_RV_SD, "LDO1_RV_SD"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO2_RV_SD, "LDO2_RV_SD"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO3_RV_SD, "LDO3_RV_SD"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_LDO4_RV_SD, "LDO4_RV_SD"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_TIMEOUT, "TIMEOUT"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_SENSOR_3_WARM, "SENSOR_3_WARM"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_SENSOR_2_WARM, "SENSOR_2_WARM"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_SENSOR_1_WARM, "SENSOR_1_WARM"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_SENSOR_0_WARM, "SENSOR_0_WARM"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_SENSOR_3_HOT, "SENSOR_3_HOT"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_SENSOR_2_HOT, "SENSOR_2_HOT"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_SENSOR_1_HOT, "SENSOR_1_HOT"),
> +	DEFINE_RES_IRQ_NAMED(TPS65219_INT_SENSOR_0_HOT, "SENSOR_0_HOT"),
> +};
> +
> +#define TPS65219_MAX_CELLS 2

Where is this even used?

> +static const struct mfd_cell tps65219_regulator_cell = {
> +	.name = "tps65219-regulator",
> +	.resources = tps65219_regulator_resources,
> +	.num_resources = ARRAY_SIZE(tps65219_regulator_resources),
> +};
> +
> +static const struct mfd_cell tps65219_pwrbutton_cell = {
> +	.name = "tps65219-pwrbutton",
> +	.resources = tps65219_pwrbutton_resources,
> +	.num_resources = ARRAY_SIZE(tps65219_pwrbutton_resources),
>  };
>  
>  static const struct regmap_config tps65219_regmap_config = {
> @@ -117,6 +182,147 @@ static const struct regmap_config tps65219_regmap_config = {
>  	.max_register = TPS65219_REG_FACTORY_CONFIG_2,
>  };
>  
> +/*
> + * Mapping of main IRQ register bits to sub-IRQ register offsets so that we can
> + * access corect sub-IRQ registers based on bits that are set in main IRQ
> + * register.
> + */
> +/* Timeout Residual Voltage Shutdown */
> +static unsigned int bit0_offsets[] = {TPS65219_TO_RV_POS};
> +static unsigned int bit1_offsets[] = {TPS65219_RV_POS};	/* Residual Voltage */
> +static unsigned int bit2_offsets[] = {TPS65219_SYS_POS};	/* System */
> +static unsigned int bit3_offsets[] = {TPS65219_BUCK_1_2_POS};	/* Buck 1-2 */
> +static unsigned int bit4_offsets[] = {TPS65219_BUCK_3_POS};	/* Buck 3 */
> +static unsigned int bit5_offsets[] = {TPS65219_LDO_1_2_POS};	/* LDO 1-2 */
> +static unsigned int bit6_offsets[] = {TPS65219_LDO_3_4_POS};	/* LDO 3-4 */
> +static unsigned int bit7_offsets[] = {TPS65219_PB_POS};	/* Power Button */

Well this is a pretty miserable interface.

You probably need spaces after { and before }.

> +static struct regmap_irq_sub_irq_map tps65219_sub_irq_offsets[] = {
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bit0_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bit1_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bit2_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bit3_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bit4_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bit5_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bit6_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bit7_offsets),
> +};
> +
> +static struct regmap_irq tps65219_irqs[] = {
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO3_SCG, TPS65219_LDO_3_4_POS,
> +		       TPS65219_INT_LDO3_SCG_MASK),

One line please.

> +	REGMAP_IRQ_REG(TPS65219_INT_LDO3_OC,
> +		       TPS65219_LDO_3_4_POS, TPS65219_INT_LDO3_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO3_UV, TPS65219_LDO_3_4_POS,
> +		       TPS65219_INT_LDO3_UV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO4_SCG, TPS65219_LDO_3_4_POS,
> +		       TPS65219_INT_LDO4_SCG_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO4_OC, TPS65219_LDO_3_4_POS,
> +		       TPS65219_INT_LDO4_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO4_UV, TPS65219_LDO_3_4_POS,
> +		       TPS65219_INT_LDO4_UV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO1_SCG,
> +		       TPS65219_LDO_1_2_POS, TPS65219_INT_LDO1_SCG_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO1_OC, TPS65219_LDO_1_2_POS,
> +		       TPS65219_INT_LDO1_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO1_UV, TPS65219_LDO_1_2_POS,
> +		       TPS65219_INT_LDO1_UV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO2_SCG, TPS65219_LDO_1_2_POS,
> +		       TPS65219_INT_LDO2_SCG_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO2_OC, TPS65219_LDO_1_2_POS,
> +		       TPS65219_INT_LDO2_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO2_UV, TPS65219_LDO_1_2_POS,
> +		       TPS65219_INT_LDO2_UV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK3_SCG, TPS65219_BUCK_3_POS,
> +		       TPS65219_INT_BUCK3_SCG_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK3_OC, TPS65219_BUCK_3_POS,
> +		       TPS65219_INT_BUCK3_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK3_NEG_OC, TPS65219_BUCK_3_POS,
> +		       TPS65219_INT_BUCK3_NEG_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK3_UV, TPS65219_BUCK_3_POS,
> +		       TPS65219_INT_BUCK3_UV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK2_SCG, TPS65219_BUCK_1_2_POS,
> +		       TPS65219_INT_BUCK2_SCG_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK2_OC, TPS65219_BUCK_1_2_POS,
> +		       TPS65219_INT_BUCK2_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK2_NEG_OC, TPS65219_BUCK_1_2_POS,
> +		       TPS65219_INT_BUCK2_NEG_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK2_UV, TPS65219_BUCK_1_2_POS,
> +		       TPS65219_INT_BUCK2_UV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK1_SCG, TPS65219_BUCK_1_2_POS,
> +		       TPS65219_INT_BUCK1_SCG_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK1_OC, TPS65219_BUCK_1_2_POS,
> +		       TPS65219_INT_BUCK1_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK1_NEG_OC, TPS65219_BUCK_1_2_POS,
> +		       TPS65219_INT_BUCK1_NEG_OC_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK1_UV, TPS65219_BUCK_1_2_POS,
> +		       TPS65219_INT_BUCK1_UV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_SENSOR_3_WARM,
> +		       TPS65219_SYS_POS, TPS65219_INT_SENSOR_3_WARM_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_SENSOR_2_WARM, TPS65219_SYS_POS,
> +		       TPS65219_INT_SENSOR_2_WARM_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_SENSOR_1_WARM, TPS65219_SYS_POS,
> +		       TPS65219_INT_SENSOR_1_WARM_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_SENSOR_0_WARM, TPS65219_SYS_POS,
> +		       TPS65219_INT_SENSOR_0_WARM_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_SENSOR_3_HOT, TPS65219_SYS_POS,
> +		       TPS65219_INT_SENSOR_3_HOT_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_SENSOR_2_HOT, TPS65219_SYS_POS,
> +		       TPS65219_INT_SENSOR_2_HOT_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_SENSOR_1_HOT, TPS65219_SYS_POS,
> +		       TPS65219_INT_SENSOR_1_HOT_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_SENSOR_0_HOT, TPS65219_SYS_POS,
> +		       TPS65219_INT_SENSOR_0_HOT_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK1_RV, TPS65219_RV_POS,
> +		       TPS65219_INT_BUCK1_RV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK2_RV, TPS65219_RV_POS,
> +		       TPS65219_INT_BUCK2_RV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK3_RV, TPS65219_RV_POS,
> +		       TPS65219_INT_BUCK3_RV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO1_RV, TPS65219_RV_POS,
> +		       TPS65219_INT_LDO1_RV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO2_RV, TPS65219_RV_POS,
> +		       TPS65219_INT_LDO2_RV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO3_RV, TPS65219_RV_POS,
> +		       TPS65219_INT_LDO3_RV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO4_RV, TPS65219_RV_POS,
> +		       TPS65219_INT_LDO4_RV_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK1_RV_SD,
> +		       TPS65219_TO_RV_POS, TPS65219_INT_BUCK1_RV_SD_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK2_RV_SD,
> +		       TPS65219_TO_RV_POS, TPS65219_INT_BUCK2_RV_SD_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_BUCK3_RV_SD, TPS65219_TO_RV_POS,
> +		       TPS65219_INT_BUCK3_RV_SD_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO1_RV_SD, TPS65219_TO_RV_POS,
> +		       TPS65219_INT_LDO1_RV_SD_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO2_RV_SD, TPS65219_TO_RV_POS,
> +		       TPS65219_INT_LDO2_RV_SD_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO3_RV_SD,
> +		       TPS65219_TO_RV_POS, TPS65219_INT_LDO3_RV_SD_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_LDO4_RV_SD, TPS65219_TO_RV_POS,
> +		       TPS65219_INT_LDO4_RV_SD_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_TIMEOUT, TPS65219_TO_RV_POS,
> +		       TPS65219_INT_TIMEOUT_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_PB_FALLING_EDGE_DETECT,
> +		       TPS65219_PB_POS, TPS65219_INT_PB_FALLING_EDGE_DET_MASK),
> +	REGMAP_IRQ_REG(TPS65219_INT_PB_RISING_EDGE_DETECT, TPS65219_PB_POS,
> +		       TPS65219_INT_PB_RISING_EDGE_DET_MASK),
> +};
> +
> +static struct regmap_irq_chip tps65219_irq_chip = {
> +	.name = "tps65219_irq",
> +	.main_status = TPS65219_REG_INT_SOURCE,
> +	.num_main_regs = 1,
> +	.num_main_status_bits = 8,
> +	.irqs = tps65219_irqs,
> +	.num_irqs = ARRAY_SIZE(tps65219_irqs),
> +	.status_base = TPS65219_REG_INT_LDO_3_4,
> +	.ack_base = TPS65219_REG_INT_LDO_3_4,
> +	.clear_ack = 1,
> +	.num_regs = 8,
> +	.sub_reg_offsets = &tps65219_sub_irq_offsets[0],
> +};
> +
>  static const struct of_device_id of_tps65219_match_table[] = {
>  	{ .compatible = "ti,tps65219", },
>  	{}
> @@ -137,6 +343,7 @@ static int tps65219_probe(struct i2c_client *client,
>  
>  	i2c_set_clientdata(client, tps);
>  	tps->dev = &client->dev;
> +	tps->irq = client->irq;

Is this used again beyond devm_regmap_add_irq_chip()?

>  	tps->regmap = devm_regmap_init_i2c(client, &tps65219_regmap_config);
>  	if (IS_ERR(tps->regmap)) {
>  		ret = PTR_ERR(tps->regmap);
> @@ -145,6 +352,12 @@ static int tps65219_probe(struct i2c_client *client,
>  		return ret;
>  	}
>  
> +	ret = devm_regmap_add_irq_chip(&client->dev, tps->regmap, tps->irq,
> +				       IRQF_ONESHOT, 0, &tps65219_irq_chip,
> +				       &tps->irq_data);

2 lines.

> +	if (ret)
> +		return ret;
> +
>  	ret = regmap_read(tps->regmap, TPS65219_REG_TI_DEV_ID, &chipid);
>  	if (ret) {
>  		dev_err(tps->dev, "Failed to read device ID: %d\n", ret);
> diff --git a/include/linux/mfd/tps65219.h b/include/linux/mfd/tps65219.h
> index e9197ab8bc75..8cecaf9bc682 100644
> --- a/include/linux/mfd/tps65219.h
> +++ b/include/linux/mfd/tps65219.h
> @@ -214,6 +214,104 @@
>  #define TPS65219_INT_PB_RISING_EDGE_DET_MASK		BIT(1)
>  #define TPS65219_INT_PB_REAL_TIME_STATUS_MASK		BIT(2)
>  
> +/* Masks for main IRQ register bits */
> +enum {
> +	TPS65219_INT_TIMEOUT_RV_SD,
> +#define TPS65219_INT_TIMEOUT_RV_SD_MASK BIT(TPS65219_INT_TIMEOUT_RV_SD)
> +	TPS65219_INT_RV,
> +#define TPS65219_INT_RV_MASK BIT(TPS65219_INT_RV)
> +	TPS65219_INT_SYSTEM,
> +#define TPS65219_INT_SYSTEM_MASK BIT(TPS65219_INT_SYSTEM)
> +	TPS65219_INT_BUCK_1_2,
> +#define TPS65219_INT_BUCK_1_2_MASK BIT(TPS65219_INT_BUCK_1_2)
> +	TPS65219_INT_BUCK_3,
> +#define TPS65219_INT_BUCK_3_MASK BIT(TPS65219_INT_BUCK_3)
> +	TPS65219_INT_LDO_1_2,
> +#define TPS65219_INT_LDO_1_2_MASK BIT(TPS65219_INT_LDO_1_2)
> +	TPS65219_INT_LDO_3_4,
> +#define TPS65219_INT_LDO_3_4_MASK BIT(TPS65219_LDO_3_4)
> +	TPS65219_INT_PB,
> +#define TPS65219_INT_PB_MASK BIT(TPS65219_INT_PB)

Where are these horrid defines used?

Can't you just use the BIT() macros.  We know what they mean.

> +};
> +
> +/* Timeout Residual Voltage Shutdown */

If you used nice nomenclature, you wouldn't need the comments.

> +#define TPS65219_TO_RV_POS 6
> +/* Residual Voltage */
> +#define TPS65219_RV_POS 5
> +/* System */
> +#define TPS65219_SYS_POS 4
> +/* Buck 1-2 */
> +#define TPS65219_BUCK_1_2_POS 3
> +/* Buck 3 */
> +#define TPS65219_BUCK_3_POS 2
> +/* LDO 1-2 */
> +#define TPS65219_LDO_1_2_POS 1
> +/* LDO 3-4 */
> +#define TPS65219_LDO_3_4_POS 0
> +/* Power Button */
> +#define TPS65219_PB_POS 7
> +
> +/* IRQs */
> +enum {
> +	/* LDO3-4 register IRQs */
> +	TPS65219_INT_LDO3_SCG,
> +	TPS65219_INT_LDO3_OC,
> +	TPS65219_INT_LDO3_UV,
> +	TPS65219_INT_LDO4_SCG,
> +	TPS65219_INT_LDO4_OC,
> +	TPS65219_INT_LDO4_UV,
> +	/* LDO1-2 */
> +	TPS65219_INT_LDO1_SCG,
> +	TPS65219_INT_LDO1_OC,
> +	TPS65219_INT_LDO1_UV,
> +	TPS65219_INT_LDO2_SCG,
> +	TPS65219_INT_LDO2_OC,
> +	TPS65219_INT_LDO2_UV,
> +	/* BUCK3 */
> +	TPS65219_INT_BUCK3_SCG,
> +	TPS65219_INT_BUCK3_OC,
> +	TPS65219_INT_BUCK3_NEG_OC,
> +	TPS65219_INT_BUCK3_UV,
> +	/* BUCK1-2 */
> +	TPS65219_INT_BUCK1_SCG,
> +	TPS65219_INT_BUCK1_OC,
> +	TPS65219_INT_BUCK1_NEG_OC,
> +	TPS65219_INT_BUCK1_UV,
> +	TPS65219_INT_BUCK2_SCG,
> +	TPS65219_INT_BUCK2_OC,
> +	TPS65219_INT_BUCK2_NEG_OC,
> +	TPS65219_INT_BUCK2_UV,
> +	/* Thermal Sensor  */
> +	TPS65219_INT_SENSOR_3_WARM,
> +	TPS65219_INT_SENSOR_2_WARM,
> +	TPS65219_INT_SENSOR_1_WARM,
> +	TPS65219_INT_SENSOR_0_WARM,
> +	TPS65219_INT_SENSOR_3_HOT,
> +	TPS65219_INT_SENSOR_2_HOT,
> +	TPS65219_INT_SENSOR_1_HOT,
> +	TPS65219_INT_SENSOR_0_HOT,
> +	/* Residual Voltage */
> +	TPS65219_INT_BUCK1_RV,
> +	TPS65219_INT_BUCK2_RV,
> +	TPS65219_INT_BUCK3_RV,
> +	TPS65219_INT_LDO1_RV,
> +	TPS65219_INT_LDO2_RV,
> +	TPS65219_INT_LDO3_RV,
> +	TPS65219_INT_LDO4_RV,
> +	/* Residual Voltage ShutDown */
> +	TPS65219_INT_BUCK1_RV_SD,
> +	TPS65219_INT_BUCK2_RV_SD,
> +	TPS65219_INT_BUCK3_RV_SD,
> +	TPS65219_INT_LDO1_RV_SD,
> +	TPS65219_INT_LDO2_RV_SD,
> +	TPS65219_INT_LDO3_RV_SD,
> +	TPS65219_INT_LDO4_RV_SD,
> +	TPS65219_INT_TIMEOUT,
> +	/* Power Button */
> +	TPS65219_INT_PB_FALLING_EDGE_DETECT,
> +	TPS65219_INT_PB_RISING_EDGE_DETECT,
> +};
> +
>  enum tps65219_regulator_id {
>  	/* DCDC's */
>  	TPS65219_BUCK_1,
> @@ -235,6 +333,19 @@ enum tps65219_regulator_id {
>  /* Number of total regulators available */
>  #define TPS65219_NUM_REGULATOR		(TPS65219_NUM_DCDC + TPS65219_NUM_LDO)
>  
> +/* Define the TPS65219 IRQ numbers */
> +enum tps65219_irqs {
> +	/* INT source registers */
> +	TPS65219_TO_RV_SD_SET_IRQ,
> +	TPS65219_RV_SET_IRQ,
> +	TPS65219_SYS_SET_IRQ,
> +	TPS65219_BUCK_1_2_SET_IRQ,
> +	TPS65219_BUCK_3_SET_IRQ,
> +	TPS65219_LDO_1_2_SET_IRQ,
> +	TPS65219_LDO_3_4_SET_IRQ,
> +	TPS65219_PB_SET_IRQ,
> +};
> +
>  /**
>   * struct tps65219 - tps65219 sub-driver chip access routines
>   *
> @@ -243,6 +354,8 @@ enum tps65219_regulator_id {
>  struct tps65219 {
>  	struct device *dev;
>  	unsigned int id;
> +	int irq;
> +	struct regmap_irq_chip_data *irq_data;
>  	struct regulator_desc desc[TPS65219_NUM_REGULATOR];
>  	struct regmap *regmap;
>  	struct notifier_block nb;

-- 
DEPRECATED: Please use lee@kernel.org

  reply	other threads:[~2022-08-08 15:59 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-26 10:33 [PATCH v2 00/11] Add support for TI TPS65219 PMIC Jerome Neanne
2022-07-26 10:33 ` Jerome Neanne
2022-07-26 10:33 ` [PATCH v2 01/11] DONOTMERGE: arm64: dts: ti: Add TI TPS65219 PMIC support for AM642 SK board Jerome Neanne
2022-07-26 10:33   ` Jerome Neanne
2022-07-26 10:33 ` [PATCH v2 02/11] DONOTMERGE: arm64: dts: ti: Add pinmux and irq mapping for TPS65219 external interrupts Jerome Neanne
2022-07-26 10:33   ` Jerome Neanne
2022-07-26 10:33 ` [PATCH v2 03/11] DONOTMERGE: arm64: dts: ti: k3-am642-sk: Enable tps65219 power-button Jerome Neanne
2022-07-26 10:33   ` Jerome Neanne
2022-07-26 10:33 ` [PATCH v2 04/11] MAINTAINERS: OMAP2+ support, add tps65218-pwrbutton Jerome Neanne
2022-07-26 10:33   ` Jerome Neanne
2022-07-26 14:30   ` Markus Schneider-Pargmann
2022-07-26 14:30     ` Markus Schneider-Pargmann
2022-07-26 10:33 ` [PATCH v2 05/11] regulator: dt-bindings: Add TI TPS65219 PMIC bindings Jerome Neanne
2022-07-26 10:33   ` Jerome Neanne
2022-07-26 12:56   ` Markus Schneider-Pargmann
2022-07-26 12:56     ` Markus Schneider-Pargmann
2022-07-26 10:33 ` [PATCH v2 06/11] mfd: drivers: Add TI TPS65219 PMIC support Jerome Neanne
2022-07-26 10:33   ` Jerome Neanne
2022-08-08 15:49   ` Lee Jones
2022-08-08 15:49     ` Lee Jones
2022-08-25  9:17     ` Markus Schneider-Pargmann
2022-08-25  9:17       ` Markus Schneider-Pargmann
2022-07-26 10:33 ` [PATCH v2 07/11] mfd: drivers: Add interrupts support to TI TPS65219 PMIC Jerome Neanne
2022-07-26 10:33   ` Jerome Neanne
2022-08-08 15:57   ` Lee Jones [this message]
2022-08-08 15:57     ` Lee Jones
2022-07-26 10:33 ` [PATCH v2 08/11] mfd: tps65219: Add power-button support Jerome Neanne
2022-07-26 10:33   ` Jerome Neanne
2022-08-08 16:05   ` Lee Jones
2022-08-08 16:05     ` Lee Jones
2022-07-26 10:33 ` [PATCH v2 09/11] regulator: drivers: Add TI TPS65219 PMIC regulators support Jerome Neanne
2022-07-26 10:33   ` Jerome Neanne
2022-07-26 10:33 ` [PATCH v2 10/11] Input: Add tps65219 interrupt driven powerbutton Jerome Neanne
2022-07-26 10:33   ` Jerome Neanne
2022-07-26 10:33 ` [PATCH v2 11/11] arm64: defconfig: Add tps65219 as modules Jerome Neanne
2022-07-26 10:33   ` Jerome Neanne

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YvEygQNmLMNV5CiV@google.com \
    --to=lee.jones@linaro.org \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=j-keerthy@ti.com \
    --cc=jneanne@baylibre.com \
    --cc=khilman@baylibre.com \
    --cc=kristo@kernel.org \
    --cc=lee@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=msp@baylibre.com \
    --cc=narmstrong@baylibre.com \
    --cc=nm@ti.com \
    --cc=robh+dt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.