-
Notifications
You must be signed in to change notification settings - Fork 7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
drivers: sensor: ltc4162: Add ltc4162 step-down charger support #35317
drivers: sensor: ltc4162: Add ltc4162 step-down charger support #35317
Conversation
d543d86
to
76ce4a0
Compare
f312456
to
7ac9592
Compare
@mbolivar-nordic @avisconti @MaureenHelm requesting for review. |
@carlescufi as mentioned in this issue, consumed sensor driver API to handle the battery charging attributes. |
The PR has been opened for more than a one & half month now, can I get some update? |
@MaureenHelm @carlescufi @mbolivar-nordic Can this be tagged for 2.7? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, I found an issue with the binding. I saw a couple of other things too, but I did not review the driver in detail.
bat_short_fault = 1 | ||
}; | ||
|
||
/* Individual Bits are mutually exclusive */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here; just use BIT().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, one more thing, sorry.
7ac9592
to
483e74d
Compare
@mbolivar-nordic could you please re-review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done a quick review, please check comments.
483e74d
to
7bae949
Compare
include/drivers/sensor.h
Outdated
enum charger_status { | ||
CHARGER_STATUS_CHARGING, | ||
CHARGER_STATUS_DISCHARGING, | ||
CHARGER_STATUS_NOT_CHARGING, | ||
CHARGER_STATUS_UNKNOWN, | ||
CHARGER_STATUS_FULL, | ||
}; | ||
|
||
/** | ||
* @brief What algorithm is charger using | ||
*/ | ||
enum charge_type { | ||
CHARGER_CHARGE_TYPE_NONE, | ||
CHARGER_CHARGE_TYPE_TRICKLE, | ||
CHARGER_CHARGE_TYPE_FAST, | ||
}; | ||
|
||
/** | ||
* @brief Charger health | ||
*/ | ||
enum charger_health { | ||
CHARGER_HEALTH_UNKNOWN, | ||
CHARGER_HEALTH_GOOD, | ||
CHARGER_HEALTH_DEAD, | ||
CHARGER_HEALTH_UNSPEC_FAILURE, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if these should be driver specific (private), @MaureenHelm ?
samples/sensor/ltc4162/src/main.c
Outdated
const struct device *dev = DEVICE_DT_GET_ANY(adi_ltc4162); | ||
|
||
if (dev == NULL) { | ||
printk("Failed to get device\n"); | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DEVICE_DT_GET_ONE
can be used to obtain a build failure. cc: @mbolivar-nordic we should likely settle on DEVICE_DT_GET_ONE
:-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mbolivar-nordic we should likely settle on
DEVICE_DT_GET_ONE
:-)
agree to disagree I guess :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree to disagree I guess :)
To amplify on my comment for the sake of this particular PR:
As I have already discussed with @gmarull elsewhere, the DEVICE_DT_GET_ONE build error message is very confusing and doesn't really indicate the problem.
Deferring the error to runtime with DEVICE_DT_GET_ANY isn't as good as getting a build error in some ways, but it's also better in others, because you get an error message you can understand.
Different people will value these two tradeoffs differently, and that's fine. I don't think there is a one-size-fits-all answer to which macro to use, and think the author of a sample -- in this case @Navin-Sankar -- should be allowed to decide what works for them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using DEVICE_DT_GET_ANY
macro to get the device
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
Add sensor channels useful for battery charger. Signed-off-by: Navin Sankar Velliangiri <navin@linumiz.com>
Implementation of LTC4162 step-down battery charger support. Signed-off-by: Navin Sankar Velliangiri <navin@linumiz.com>
b14c161
to
434a9ea
Compare
@MaureenHelm Rebased to main, please revisit. |
@mbolivar-nordic @gmarull please revisit |
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
Add supports for LTC4162 step-down battery charger driver support with sample application
Signed-off-by: Navin Sankar Velliangiri navin@linumiz.com