Re: [PATCH 2/2] power: supply: bq24190_charger: Add support for BQ24296

[Sebastian Reichel]

Hi,

On Sat, Oct 07, 2023 at 10:07:01AM +0800, Hermes Zhang wrote:
> From: Hermes Zhang <chenhuiz@xxxxxxxx>
> 
> 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@xxxxxxxx>
> ---

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

Attachment: signature.asc
Description: PGP signature