2024-03-08 11:26:01

by Julien Panis

[permalink] [raw]
Subject: Re: [PATCH v3 01/11] mfd: tps6594: Add register definitions for TI TPS65224 PMIC

On 3/8/24 11:34, Bhargav Raviprakash wrote:
> From: Nirmala Devi Mal Nadar <[email protected]>
>
> Extend TPS6594 PMIC register and field definitions to support TPS65224
> power management IC.
>
> TPS65224 is software compatible to TPS6594 and can re-use many of the
> same definitions, new definitions are added to support additional
> controls available on TPS65224.
>
> Signed-off-by: Nirmala Devi Mal Nadar <[email protected]>
> Signed-off-by: Bhargav Raviprakash <[email protected]>
> ---
> include/linux/mfd/tps6594.h | 354 ++++++++++++++++++++++++++++++++++--
> 1 file changed, 342 insertions(+), 12 deletions(-)

[...]

> +/* IRQs */
> +enum tps65224_irqs {
> + /* INT_BUCK register */
> + TPS65224_IRQ_BUCK1_UVOV,
> + TPS65224_IRQ_BUCK2_UVOV,
> + TPS65224_IRQ_BUCK3_UVOV,
> + TPS65224_IRQ_BUCK4_UVOV,
> + /* INT_LDO_VMON register */
> + TPS65224_IRQ_LDO1_UVOV,
> + TPS65224_IRQ_LDO2_UVOV,
> + TPS65224_IRQ_LDO3_UVOV,
> + TPS65224_IRQ_VCCA_UVOV,
> + TPS65224_IRQ_VMON1_UVOV,
> + TPS65224_IRQ_VMON2_UVOV,
> + /* INT_GPIO register */
> + TPS65224_IRQ_GPIO1,
> + TPS65224_IRQ_GPIO2,
> + TPS65224_IRQ_GPIO3,
> + TPS65224_IRQ_GPIO4,
> + TPS65224_IRQ_GPIO5,
> + TPS65224_IRQ_GPIO6,
> + /* INT_STARTUP register */
> + TPS65224_IRQ_VSENSE,
> + TPS65224_IRQ_ENABLE,
> + TPS65224_IRQ_PB_SHORT,
> + TPS65224_IRQ_FSD,
> + TPS65224_IRQ_SOFT_REBOOT,
> + /* INT_MISC register */
> + TPS65224_IRQ_BIST_PASS,
> + TPS65224_IRQ_EXT_CLK,
> + TPS65224_IRQ_REG_UNLOCK,
> + TPS65224_IRQ_TWARN,
> + TPS65224_IRQ_PB_LONG,
> + TPS65224_IRQ_PB_FALL,
> + TPS65224_IRQ_PB_RISE,
> + TPS65224_IRQ_ADC_CONV_READY,
> + /* INT_MODERATE_ERR register */
> + TPS65224_IRQ_TSD_ORD,
> + TPS65224_IRQ_BIST_FAIL,
> + TPS65224_IRQ_REG_CRC_ERR,
> + TPS65224_IRQ_RECOV_CNT,
> + /* INT_SEVERE_ERR register */
> + TPS65224_IRQ_TSD_IMM,
> + TPS65224_IRQ_VCCA_OVP,
> + TPS65224_IRQ_PFSM_ERR,
> + TPS65224_IRQ_BG_XMON,
> + /* INT_FSM_ERR register */
> + TPS65224_IRQ_IMM_SHUTDOWN,
> + TPS65224_IRQ_ORD_SHUTDOWN,
> + TPS65224_IRQ_MCU_PWR_ERR,
> + TPS65224_IRQ_SOC_PWR_ERR,
> + TPS65224_IRQ_COMM_ERR,
> + TPS65224_IRQ_I2C2_ERR,
> + /* INT_ESM register */
> + TPS65224_IRQ_ESM_MCU_PIN,
> + TPS65224_IRQ_ESM_MCU_FAIL,
> + TPS65224_IRQ_ESM_MCU_RST,

You should remove the 3 lines above for ESM_MCU, since there is none
linux driver for ESM_MCU.

> +};
> +
> +#define TPS65224_IRQ_NAME_BUCK1_UVOV "buck1_uvov"
> +#define TPS65224_IRQ_NAME_BUCK2_UVOV "buck2_uvov"
> +#define TPS65224_IRQ_NAME_BUCK3_UVOV "buck3_uvov"
> +#define TPS65224_IRQ_NAME_BUCK4_UVOV "buck4_uvov"
> +#define TPS65224_IRQ_NAME_LDO1_UVOV "ldo1_uvov"
> +#define TPS65224_IRQ_NAME_LDO2_UVOV "ldo2_uvov"
> +#define TPS65224_IRQ_NAME_LDO3_UVOV "ldo3_uvov"
> +#define TPS65224_IRQ_NAME_VCCA_UVOV "vcca_uvov"
> +#define TPS65224_IRQ_NAME_VMON1_UVOV "vmon1_uvov"
> +#define TPS65224_IRQ_NAME_VMON2_UVOV "vmon2_uvov"
> +#define TPS65224_IRQ_NAME_GPIO1 "gpio1"
> +#define TPS65224_IRQ_NAME_GPIO2 "gpio2"
> +#define TPS65224_IRQ_NAME_GPIO3 "gpio3"
> +#define TPS65224_IRQ_NAME_GPIO4 "gpio4"
> +#define TPS65224_IRQ_NAME_GPIO5 "gpio5"
> +#define TPS65224_IRQ_NAME_GPIO6 "gpio6"
> +#define TPS65224_IRQ_NAME_VSENSE "vsense"
> +#define TPS65224_IRQ_NAME_ENABLE "enable"
> +#define TPS65224_IRQ_NAME_PB_SHORT "pb_short"
> +#define TPS65224_IRQ_NAME_FSD "fsd"
> +#define TPS65224_IRQ_NAME_SOFT_REBOOT "soft_reboot"
> +#define TPS65224_IRQ_NAME_BIST_PASS "bist_pass"
> +#define TPS65224_IRQ_NAME_EXT_CLK "ext_clk"
> +#define TPS65224_IRQ_NAME_REG_UNLOCK "reg_unlock"
> +#define TPS65224_IRQ_NAME_TWARN "twarn"
> +#define TPS65224_IRQ_NAME_PB_LONG "pb_long"
> +#define TPS65224_IRQ_NAME_PB_FALL "pb_fall"
> +#define TPS65224_IRQ_NAME_PB_RISE "pb_rise"
> +#define TPS65224_IRQ_NAME_ADC_CONV_READY "adc_conv_ready"
> +#define TPS65224_IRQ_NAME_TSD_ORD "tsd_ord"
> +#define TPS65224_IRQ_NAME_BIST_FAIL "bist_fail"
> +#define TPS65224_IRQ_NAME_REG_CRC_ERR "reg_crc_err"
> +#define TPS65224_IRQ_NAME_RECOV_CNT "recov_cnt"
> +#define TPS65224_IRQ_NAME_TSD_IMM "tsd_imm"
> +#define TPS65224_IRQ_NAME_VCCA_OVP "vcca_ovp"
> +#define TPS65224_IRQ_NAME_PFSM_ERR "pfsm_err"
> +#define TPS65224_IRQ_NAME_BG_XMON "bg_xmon"
> +#define TPS65224_IRQ_NAME_IMM_SHUTDOWN "imm_shutdown"
> +#define TPS65224_IRQ_NAME_ORD_SHUTDOWN "ord_shutdown"
> +#define TPS65224_IRQ_NAME_MCU_PWR_ERR "mcu_pwr_err"
> +#define TPS65224_IRQ_NAME_SOC_PWR_ERR "soc_pwr_err"
> +#define TPS65224_IRQ_NAME_COMM_ERR "comm_err"
> +#define TPS65224_IRQ_NAME_I2C2_ERR "i2c2_err"
> +#define TPS65224_IRQ_NAME_ESM_MCU_PIN "esm_mcu_pin"
> +#define TPS65224_IRQ_NAME_ESM_MCU_FAIL "esm_mcu_fail"
> +#define TPS65224_IRQ_NAME_ESM_MCU_RST "esm_mcu_rst"

You should remove the 3 lines above for ESM_MCU.

Julien



2024-03-20 10:15:35

by Bhargav Raviprakash

[permalink] [raw]
Subject: Re: [PATCH v3 01/11] mfd: tps6594: Add register definitions for TI TPS65224 PMIC

Hello,

On Fri, 8 Mar 2024 12:24:40 +0100, Julien wrote:
> On 3/8/24 11:34, Bhargav Raviprakash wrote:
> > From: Nirmala Devi Mal Nadar <[email protected]>
> >
> > Extend TPS6594 PMIC register and field definitions to support TPS65224
> > power management IC.
> >
> > TPS65224 is software compatible to TPS6594 and can re-use many of the
> > same definitions, new definitions are added to support additional
> > controls available on TPS65224.
> >
> > Signed-off-by: Nirmala Devi Mal Nadar <[email protected]>
> > Signed-off-by: Bhargav Raviprakash <[email protected]>
> > ---
> > include/linux/mfd/tps6594.h | 354 ++++++++++++++++++++++++++++++++++--
> > 1 file changed, 342 insertions(+), 12 deletions(-)
>
> [...]
>
> > +/* IRQs */
> > +enum tps65224_irqs {
> > + /* INT_BUCK register */
> > + TPS65224_IRQ_BUCK1_UVOV,
> > + TPS65224_IRQ_BUCK2_UVOV,
> > + TPS65224_IRQ_BUCK3_UVOV,
> > + TPS65224_IRQ_BUCK4_UVOV,
> > + /* INT_LDO_VMON register */
> > + TPS65224_IRQ_LDO1_UVOV,
> > + TPS65224_IRQ_LDO2_UVOV,
> > + TPS65224_IRQ_LDO3_UVOV,
> > + TPS65224_IRQ_VCCA_UVOV,
> > + TPS65224_IRQ_VMON1_UVOV,
> > + TPS65224_IRQ_VMON2_UVOV,
> > + /* INT_GPIO register */
> > + TPS65224_IRQ_GPIO1,
> > + TPS65224_IRQ_GPIO2,
> > + TPS65224_IRQ_GPIO3,
> > + TPS65224_IRQ_GPIO4,
> > + TPS65224_IRQ_GPIO5,
> > + TPS65224_IRQ_GPIO6,
> > + /* INT_STARTUP register */
> > + TPS65224_IRQ_VSENSE,
> > + TPS65224_IRQ_ENABLE,
> > + TPS65224_IRQ_PB_SHORT,
> > + TPS65224_IRQ_FSD,
> > + TPS65224_IRQ_SOFT_REBOOT,
> > + /* INT_MISC register */
> > + TPS65224_IRQ_BIST_PASS,
> > + TPS65224_IRQ_EXT_CLK,
> > + TPS65224_IRQ_REG_UNLOCK,
> > + TPS65224_IRQ_TWARN,
> > + TPS65224_IRQ_PB_LONG,
> > + TPS65224_IRQ_PB_FALL,
> > + TPS65224_IRQ_PB_RISE,
> > + TPS65224_IRQ_ADC_CONV_READY,
> > + /* INT_MODERATE_ERR register */
> > + TPS65224_IRQ_TSD_ORD,
> > + TPS65224_IRQ_BIST_FAIL,
> > + TPS65224_IRQ_REG_CRC_ERR,
> > + TPS65224_IRQ_RECOV_CNT,
> > + /* INT_SEVERE_ERR register */
> > + TPS65224_IRQ_TSD_IMM,
> > + TPS65224_IRQ_VCCA_OVP,
> > + TPS65224_IRQ_PFSM_ERR,
> > + TPS65224_IRQ_BG_XMON,
> > + /* INT_FSM_ERR register */
> > + TPS65224_IRQ_IMM_SHUTDOWN,
> > + TPS65224_IRQ_ORD_SHUTDOWN,
> > + TPS65224_IRQ_MCU_PWR_ERR,
> > + TPS65224_IRQ_SOC_PWR_ERR,
> > + TPS65224_IRQ_COMM_ERR,
> > + TPS65224_IRQ_I2C2_ERR,
> > + /* INT_ESM register */
> > + TPS65224_IRQ_ESM_MCU_PIN,
> > + TPS65224_IRQ_ESM_MCU_FAIL,
> > + TPS65224_IRQ_ESM_MCU_RST,
>
> You should remove the 3 lines above for ESM_MCU, since there is none
> linux driver for ESM_MCU.
>
> > +};
> > +
> > +#define TPS65224_IRQ_NAME_BUCK1_UVOV "buck1_uvov"
> > +#define TPS65224_IRQ_NAME_BUCK2_UVOV "buck2_uvov"
> > +#define TPS65224_IRQ_NAME_BUCK3_UVOV "buck3_uvov"
> > +#define TPS65224_IRQ_NAME_BUCK4_UVOV "buck4_uvov"
> > +#define TPS65224_IRQ_NAME_LDO1_UVOV "ldo1_uvov"
> > +#define TPS65224_IRQ_NAME_LDO2_UVOV "ldo2_uvov"
> > +#define TPS65224_IRQ_NAME_LDO3_UVOV "ldo3_uvov"
> > +#define TPS65224_IRQ_NAME_VCCA_UVOV "vcca_uvov"
> > +#define TPS65224_IRQ_NAME_VMON1_UVOV "vmon1_uvov"
> > +#define TPS65224_IRQ_NAME_VMON2_UVOV "vmon2_uvov"
> > +#define TPS65224_IRQ_NAME_GPIO1 "gpio1"
> > +#define TPS65224_IRQ_NAME_GPIO2 "gpio2"
> > +#define TPS65224_IRQ_NAME_GPIO3 "gpio3"
> > +#define TPS65224_IRQ_NAME_GPIO4 "gpio4"
> > +#define TPS65224_IRQ_NAME_GPIO5 "gpio5"
> > +#define TPS65224_IRQ_NAME_GPIO6 "gpio6"
> > +#define TPS65224_IRQ_NAME_VSENSE "vsense"
> > +#define TPS65224_IRQ_NAME_ENABLE "enable"
> > +#define TPS65224_IRQ_NAME_PB_SHORT "pb_short"
> > +#define TPS65224_IRQ_NAME_FSD "fsd"
> > +#define TPS65224_IRQ_NAME_SOFT_REBOOT "soft_reboot"
> > +#define TPS65224_IRQ_NAME_BIST_PASS "bist_pass"
> > +#define TPS65224_IRQ_NAME_EXT_CLK "ext_clk"
> > +#define TPS65224_IRQ_NAME_REG_UNLOCK "reg_unlock"
> > +#define TPS65224_IRQ_NAME_TWARN "twarn"
> > +#define TPS65224_IRQ_NAME_PB_LONG "pb_long"
> > +#define TPS65224_IRQ_NAME_PB_FALL "pb_fall"
> > +#define TPS65224_IRQ_NAME_PB_RISE "pb_rise"
> > +#define TPS65224_IRQ_NAME_ADC_CONV_READY "adc_conv_ready"
> > +#define TPS65224_IRQ_NAME_TSD_ORD "tsd_ord"
> > +#define TPS65224_IRQ_NAME_BIST_FAIL "bist_fail"
> > +#define TPS65224_IRQ_NAME_REG_CRC_ERR "reg_crc_err"
> > +#define TPS65224_IRQ_NAME_RECOV_CNT "recov_cnt"
> > +#define TPS65224_IRQ_NAME_TSD_IMM "tsd_imm"
> > +#define TPS65224_IRQ_NAME_VCCA_OVP "vcca_ovp"
> > +#define TPS65224_IRQ_NAME_PFSM_ERR "pfsm_err"
> > +#define TPS65224_IRQ_NAME_BG_XMON "bg_xmon"
> > +#define TPS65224_IRQ_NAME_IMM_SHUTDOWN "imm_shutdown"
> > +#define TPS65224_IRQ_NAME_ORD_SHUTDOWN "ord_shutdown"
> > +#define TPS65224_IRQ_NAME_MCU_PWR_ERR "mcu_pwr_err"
> > +#define TPS65224_IRQ_NAME_SOC_PWR_ERR "soc_pwr_err"
> > +#define TPS65224_IRQ_NAME_COMM_ERR "comm_err"
> > +#define TPS65224_IRQ_NAME_I2C2_ERR "i2c2_err"
> > +#define TPS65224_IRQ_NAME_ESM_MCU_PIN "esm_mcu_pin"
> > +#define TPS65224_IRQ_NAME_ESM_MCU_FAIL "esm_mcu_fail"
> > +#define TPS65224_IRQ_NAME_ESM_MCU_RST "esm_mcu_rst"
>
> You should remove the 3 lines above for ESM_MCU.
>
> Julien

Thanks! will get it done in the next version.

Regards,
Bhargav