linux-kernel - Re: [PATCH 2/2] power: supply: bq24190

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: <20231021010730.zk2hlbin75fn3cy6@mercury.elektranox.org>
Date:   Sat, 21 Oct 2023 03:07:30 +0200
From:   Sebastian Reichel <sebastian.reichel@...labora.com>
To:     Hermes Zhang <Hermes.Zhang@...s.com>
Cc:     krzysztof.kozlowski+dt@...aro.org, robh+dt@...nel.org,
        linux-kernel@...r.kernel.org, kernel@...s.com,
        Hermes Zhang <chenhuiz@...s.com>, linux-pm@...r.kernel.org
Subject: Re: [PATCH 2/2] power: supply: bq24190_charger: Add support for
 BQ24296

Hi,

On Sat, Oct 07, 2023 at 10:07:01AM +0800, Hermes Zhang wrote:
> From: Hermes Zhang <chenhuiz@...s.com>
> 
> The BQ24296 is most similar to the BQ24196, but the:
> 1. OTG config is split from CHG config (REG01)
> 2. ICHG (Fast Charge Current limit) range is smaller (<=3008mA)
> 3. NTC fault is simplified to 2 bits
> 
> Signed-off-by: Hermes Zhang <chenhuiz@...s.com>
> ---

Thanks for your patch. I did not look into tiny details, because I
think this needs to be restructured first. Please follow the style
of bq256xx_charger.c, bq24257_charger.c and bq2515x_charger.c and
use a struct together with i2c_get_match_data().

Also put device specific functions into the struct to avoid a switch
case structure like the following:

> [...]
> +static int bq24190_hw_init(struct bq24190_dev_info *bdi)
> +{
> +	int ret;
> +
> +	/* First check that the device really is what its supposed to be */
> +	switch (bdi->chip) {
> +	case BQ24190:
> +	case BQ24192:
> +	case BQ24192i:
> +	case BQ24196:
> +		ret = bq24190_check_chip(bdi);
> +		break;
> +	case BQ24296:
> +		ret = bq24296_check_chip(bdi);
> +		break;
> +	default:
> +		dev_err(bdi->dev, "Error unsupported model: %d\n", bdi->chip);
> +		return -EINVAL;
> +	}
> [...]

Instead it should look like this:

struct info {
    ...
    int (*check_chip)(struct bq24190_dev_info *bdi);
    ...
};

static int bq24190_hw_init(struct bq24190_dev_info *bdi)
{
    return bdi->info->check_chip(bdi);
}

Thanks,

-- Sebastian

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)