Add support for MAX1720x Maxim ModelGauge m5 Fuel Gauge #2774
Conversation
The MAX1720x is a stand-alone fuel gauge that communicates via I2C and provides battery voltage, current. The driver registers as a power supply class device and supports basic property reads via the sysfs interface. The binding document for the Device Tree has been added at: Documentation/devicetree/bindings/power/supply/maxim,max1720x.yaml Signed-off-by: Joan Na <joan.na@analog.com>
joan-na-good
requested review from
nunojsa,
mhennerich,
dbogdan,
btogorean,
chegbeli,
buha,
amiclaus,
rbolboac,
danmois,
CiprianRegus and
machschmitt
as code owners
| title: Maxim ModelGauge m5 Fuel Gauge family (MAX17201/5, MAX1733x series) | ||
|
|
||
| maintainers: | ||
| - "Joan Na <joan.na@analog.com>" |
There was a problem hiding this comment.
Don't mix bindings with driver code. Bindings should be in it's own patch...
| - "maxim,max17335" | ||
|
|
||
| reg: | ||
| description: I2C address of the device |
| maxItems: 1 | ||
|
|
||
| maxim,rsense: | ||
| description: Rsense register value in milliohms |
| $ref: /schemas/types.yaml#/definitions/int32 | ||
|
|
||
| status: | ||
| enum: [ "okay", "disabled" ] |
There was a problem hiding this comment.
what is the above property? You're not including any other bindings... If it's the DT node status, drop it.
|
|
||
| maxim,ialrt-max: | ||
| description: Upper current limit (in mA) to trigger ALRT1 | ||
| $ref: /schemas/types.yaml#/definitions/int32 |
There was a problem hiding this comment.
almost all of the above feel like hwmon stuff. It seems the power supply subsystem already has some ind of bridge with hwmon. Did you looked at this:
https://elixir.bootlin.com/linux/v6.14.5/source/drivers/power/supply/power_supply_core.c#L1600
Make sure you have no way of controlling the above at runtime before considering dt properties
| [ID_MAX17320] = max17320_get_battery_health, | ||
| [ID_MAX17330] = max17330_get_battery_health, | ||
| [ID_MAX17332] = max17330_get_battery_health, | ||
| [ID_MAX17335] = max17330_get_battery_health, |
There was a problem hiding this comment.
put this in a chip info structure
| [ID_MAX17335] = max17330_get_battery_health, | ||
| }; | ||
|
|
||
| static u32 register_to_read; |
There was a problem hiding this comment.
global variable like this is very questionable
| static irqreturn_t max17330_irq_handler(int id, void *dev); | ||
|
|
||
| static int max1720x_regmap_write(struct max1720x_priv *priv, unsigned int reg, unsigned int val); | ||
| static int max1720x_regmap_read(struct max1720x_priv *priv, unsigned int reg, unsigned int *val); |
| [ID_MAX17330] = max17330_irq_handler, | ||
| [ID_MAX17332] = max17330_irq_handler, | ||
| [ID_MAX17335] = max17330_irq_handler, | ||
| }; |
| [ID_MAX17330] = max17330_regs, | ||
| [ID_MAX17332] = max17332_regs, | ||
| [ID_MAX17335] = max17335_regs, | ||
| }; |
There was a problem hiding this comment.
Dunno the above is really needed
| @@ -414,7 +414,18 @@ config BATTERY_MAX1721X | |||
|
|
|||
| Say Y here to enable support for the MAX17211/MAX17215 standalone | |||
| battery gas-gauge. | |||
|
|
|||
| config BATTERY_MAX1721X | |||
There was a problem hiding this comment.
should be BATTERY_MAX1720X and before the previous entry, always in alphabetical order
I did a run with the experimental ci at https://github.com/gastmaier/adi-linux/actions/runs/14999793574
But since the driver never gets compiled (and a error is raised for it), it is only linting checking
The MAX1720x is a stand-alone fuel gauge that communicates via I2C and provides battery voltage, current.
The driver registers as a power supply class device and supports basic property reads via the sysfs interface.
The binding document for the Device Tree has been added at: Documentation/devicetree/bindings/power/supply/maxim,max1720x.yaml
PR Description
necessary to understand them. List any dependencies required for this change.
any space), or simply check them after publishing the PR.
description and try to push all related PRs simultaneously.
PR Type
PR Checklist