Received-SPF: pass (google.com: domain of linux-kernel+bounces-108742-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
IronPort-SDR: oQrzJEPhxgP8uSFlXa+okPvyeBYCfJjsBKwDc4F+mjozrIdneuUGtuGjWkX3CLcm3b42JOf9Tz
 bIziGp9kJiug==
From: Bhargav Raviprakash <bhargav.r@ltts.com>
To: jpanis@baylibre.com
Cc: arnd@arndb.de,
	bhargav.r@ltts.com,
	broonie@kernel.org,
	conor+dt@kernel.org,
	devicetree@vger.kernel.org,
	eblanc@baylibre.com,
	gregkh@linuxfoundation.org,
	kristo@kernel.org,
	krzysztof.kozlowski+dt@linaro.org,
	lee@kernel.org,
	lgirdwood@gmail.com,
	linus.walleij@linaro.org,
	linux-arm-kernel@lists.infradead.org,
	linux-gpio@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	m.nirmaladevi@ltts.com,
	nm@ti.com,
	robh+dt@kernel.org,
	vigneshr@ti.com
Subject: Re: [PATCH v3 01/11] mfd: tps6594: Add register definitions for TI TPS65224 PMIC
Date: Wed, 20 Mar 2024 15:43:54 +0530
Message-Id: <20240320101354.463936-1-bhargav.r@ltts.com>
In-Reply-To: <44295772-4635-42c2-b7b5-cdc37505715e@baylibre.com>
References: <44295772-4635-42c2-b7b5-cdc37505715e@baylibre.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

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 <m.nirmaladevi@ltts.com>
> >
> > 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 <m.nirmaladevi@ltts.com>
> > Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
> > ---
> >   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