drivers: power: ltc2992: Add LTC2992 driver #2165
Conversation
cencarna
requested review from
buha,
CiprianRegus,
dbogdan,
amiclaus,
rbolboac and
danmois
as code owners
|
|
drivers/power/ltc2992/ltc2992.h
Outdated
| struct ltc2992_init_param { | ||
| struct no_os_i2c_init_param i2c_init; | ||
|
|
||
| #if LTC2992_USE_GPIO |
There was a problem hiding this comment.
This macro is redundant. The user already has the option to use or not the gpio via no_os_gpio_get_optional
There was a problem hiding this comment.
will remove LTC2992_USE_GPIO macro in struct ltc2992_dev, struct ltc2992_init_param, ltc2992_init, and ltc2992_remove. Should I also remove the same macro at the beginning of gpio-specific functions and variables? I made this macro so that they will not be included at compile-time if the user opt not to use the gpios.
|
|
||
| i2c_err : | ||
| no_os_free(dev); | ||
| return ret; |
There was a problem hiding this comment.
revert the order of i2c_err and dev_err. Then you would only have 1 return ret
There was a problem hiding this comment.
Removing the first return ret may result to double frees. Will this be okay? Alternatively, I can just call no_os_gpio_remove and no_os_i2c_remove directly in dev_err
dev_err:
no_os_gpio_remove(dev->alert_gpio_desc);
no_os_i2c_remove(dev->i2c_desc);
i2c_err:
no_os_free(dev);
return ret;
Will this be a better implementation?
There was a problem hiding this comment.
Sorry for the delay, yes, that's what I've meant
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
cencarna
force-pushed
the
staging/ltc2992
branch
from
7ed8d59
to
5ee2932
Compare
|
Commenter does not have sufficient privileges for PR 2165 in repo analogdevicesinc/no-OS |
|
Changelog: |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
cencarna
force-pushed
the
staging/ltc2992
branch
2 times, most recently
from
d14a005
to
d2902b4
Compare
|
Changelog: |
The LTC2992 is a rail-to-rail system monitor that measures current, voltage, and power of two supplies. It features an operating range of 2.7V to 100V and includes a shunt regulator for supplies above 100V. Minimum and maximum values are stored and an overrange alert with programmable thresholds minimizes the need for software polling. Data is reported via a standard I2C interface. Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
Add README.rst file containing documentation of the LTC2992 driver. Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@analog.com>
|
Change log: |
| * @note The default is @p TRUE. | ||
| */ | ||
| #ifndef LTC2992_USE_GPIO | ||
| #define LTC2992_USE_GPIO 1 |
There was a problem hiding this comment.
I still don't quite like this approach of guarding part functionalities using macro definitions. Moreover you enable all features by default here. I'd suggest dropping it completely, but let's see what others have to say.
Pull Request Description
The LTC2992 is a rail-to-rail system monitor that measures current,
voltage, and power of two supplies. It features an operating range of
2.7V to 100V and includes a shunt regulator for supplies above 100V.
Minimum and maximum values are stored and an overrange alert with
programmable thresholds minimizes the need for software polling. Data is
reported via a standard I2C interface.
PR Type
PR Checklist