linux-kernel - Re: [PATCH v5 09/11] regulator: tps6594-regulator: Add TI TPS65224 PMIC regulators

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <29d6a737-0bfc-4440-ac57-54c94c6d7ad4@moroto.mountain>
Date: Tue, 2 Apr 2024 13:06:46 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Bhargav Raviprakash <bhargav.r@...s.com>
Cc: linux-kernel@...r.kernel.org, m.nirmaladevi@...s.com, lee@...nel.org,
	robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
	conor+dt@...nel.org, jpanis@...libre.com,
	devicetree@...r.kernel.org, arnd@...db.de,
	gregkh@...uxfoundation.org, lgirdwood@...il.com, broonie@...nel.org,
	linus.walleij@...aro.org, linux-gpio@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, nm@...com, vigneshr@...com,
	kristo@...nel.org, eblanc@...libre.com
Subject: Re: [PATCH v5 09/11] regulator: tps6594-regulator: Add TI TPS65224
 PMIC regulators

On Thu, Mar 28, 2024 at 06:10:14PM +0530, Bhargav Raviprakash wrote:
>  static irqreturn_t tps6594_regulator_irq_handler(int irq, void *data)
>  {
>  	struct tps6594_regulator_irq_data *irq_data = data;
> @@ -369,17 +513,23 @@ static irqreturn_t tps6594_regulator_irq_handler(int irq, void *data)
>  static int tps6594_request_reg_irqs(struct platform_device *pdev,
              ^^^^^^^
This function is not beautiful.  I think since you're changing it from
being tps6594 specific, maybe you want to rename a bunch of stuff.

>  				    struct regulator_dev *rdev,
>  				    struct tps6594_regulator_irq_data *irq_data,
> -				    struct tps6594_regulator_irq_type *tps6594_regs_irq_types,
> +				    struct tps6594_regulator_irq_type *regs_irq_types,
>  				    int *irq_idx)
>  {
>  	struct tps6594_regulator_irq_type *irq_type;
>  	struct tps6594 *tps = dev_get_drvdata(pdev->dev.parent);
> -	int j;
> +	size_t j;
>  	int irq;
>  	int error;
> +	size_t interrupt_cnt;
> +
> +	if (tps->chip_id == TPS6594)
> +		interrupt_cnt = ARRAY_SIZE(tps6594_buck1_irq_types);
> +	else
> +		interrupt_cnt = ARRAY_SIZE(tps65224_buck1_irq_types);

Either 1) pass both the array and the size or 2) just use tps->chip_id
to determine both the array and the arrays_size.  Passing the array and
then determining which array was passed by looking at the type is ugly.

regards,
dan carpenter


>  
> -	for (j = 0; j < REGS_INT_NB; j++) {
> -		irq_type = &tps6594_regs_irq_types[j];
> +	for (j = 0; j < interrupt_cnt; j++) {
> +		irq_type = &regs_irq_types[j];
>  		irq = platform_get_irq_byname(pdev, irq_type->irq_name);
>  		if (irq < 0)
>  			return -EINVAL;