Re: [PATCH 2/2] Add driver for MAX17211/MAX17215 fuel gauge

From: Sebastian Reichel
Date: Sat Apr 29 2017 - 12:41:25 EST


Hi,

Please make sure you Cc the relevant people / mailing lists. You
can use ./scripts/get_maintainer.pl to find out who should receive
the patches.

On Sat, Apr 29, 2017 at 05:34:29PM +0300, Alex A. Mihaylov wrote:
> Maxim Semiconductor MAX17211/MAX17215 single/multi-cell fuel gauge
> monitor with M5 Fuel Gauge algorithm
>
> This driver provide userspace access to MAX17211/MAX17215 data with
> power_supply class drivers.

> ---
> drivers/power/supply/Kconfig | 8 +
> drivers/power/supply/Makefile | 1 +

Please move the entries in both files before CONFIG_BATTERY_MAX17040.

> [...]
>
> + * FixMe:
> + * Device without no_thermal = true not register (err -22)
> + * Len of platform device name "max17211-battery.X.auto"
> + * more than 20 chars limit in THERMAL_NAME_LENGTH from
> + * include/uapi/linux/thermal.h
> + */

IIRC we already had problems with small THERMAL_NAME_LENGTH before.
I suggest to add another patch, that increases THERMAL_NAME_LENGTH
(don't forget to Cc/To the thermal subsystem people).

> + info->bat_desc.no_thermal = true;
> + psy_cfg.drv_data = info;
> +
> + if (w1_max1721x_reg_get(info->w1_dev,
> + MAX1721X_REG_NRSENSE, &info->rsense))
> + return -ENODEV;
> +
> + if (!info->rsense) {
> + dev_warn(info->dev, "RSenese not calibrated, set 10 mOhms!\n");
> + info->rsense = 1000; /* in regs in 10^-5 */
> + }
> + dev_dbg(info->dev, "RSense: %d mOhms.\n", info->rsense / 100);
> +
> + if (get_string(info->w1_dev, MAX1721X_REG_MFG_STR,
> + MAX1721X_REG_MFG_NUMB, info->ManufacturerName)) {
> + dev_err(info->dev, "Can't read manufacturer. Hardware error.\n");
> + return -ENODEV;
> + }
> +
> + if (!info->ManufacturerName[0])
> + strncpy(info->ManufacturerName, DEF_MFG_NAME,
> + 2 * MAX1721X_REG_MFG_NUMB);
> +
> + if (get_string(info->w1_dev, MAX1721X_REG_DEV_STR,
> + MAX1721X_REG_DEV_NUMB, info->DeviceName)) {
> + dev_err(info->dev, "Can't read device. Hardware error.\n");
> + return -ENODEV;
> + }
> + if (!info->DeviceName[0]) {
> + uint16_t dev_name;
> +
> + if (w1_max1721x_reg_get(info->w1_dev,
> + MAX172XX_REG_DEVNAME, &dev_name)) {
> + dev_err(info->w1_dev, "Can't read device name reg.\n");
> + return -ENODEV;
> + }
> +
> + switch (dev_name & MAX172XX_DEV_MASK) {
> + case MAX172X1_DEV:
> + strncpy(info->DeviceName, DEF_DEV_NAME_MAX17211,
> + 2 * MAX1721X_REG_DEV_NUMB);
> + break;
> + case MAX172X5_DEV:
> + strncpy(info->DeviceName, DEF_DEV_NAME_MAX17215,
> + 2 * MAX1721X_REG_DEV_NUMB);
> + break;
> + default:
> + strncpy(info->DeviceName, DEF_DEV_NAME_UNKNOWN,
> + 2 * MAX1721X_REG_DEV_NUMB);
> + }
> + }
> +
> + if (get_sn_string(info->w1_dev, info->SerialNumber)) {
> + dev_err(info->dev, "Can't read serial. Hardware error.\n");
> + return -ENODEV;
> + }
> +
> + info->bat = power_supply_register(&pdev->dev, &info->bat_desc,
> + &psy_cfg);
> + if (IS_ERR(info->bat)) {
> + dev_err(info->dev, "failed to register battery\n");
> + return PTR_ERR(info->bat);
> + }

Please use devm_power_supply_register() and drop the remove
function.

> + return 0;
> +}
> +
> +static int max1721x_battery_remove(struct platform_device *pdev)
> +{
> + struct max17211_device_info *info = platform_get_drvdata(pdev);
> +
> + power_supply_unregister(info->bat);
> +
> + return 0;
> +}
> +
> +static struct platform_driver max1721x_battery_driver = {
> + .driver = {
> + .name = "max1721x-battery",
> + },
> + .probe = max1721x_battery_probe,
> + .remove = max1721x_battery_remove,
> +};
> +module_platform_driver(max1721x_battery_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Alex A. Mihaylov <minimumlaw@xxxxxxxxxx>");
> +MODULE_DESCRIPTION("Maxim MAX17211/MAX17215 Fuel Gauage IC driver");
> +MODULE_ALIAS("platform:max1721x-battery");

-- Sebastian

Attachment: signature.asc
Description: PGP signature