| Message ID | 20240712-x1e80100-battmgr-v1-1-a253d767f493@linaro.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | power: supply: qcom_battmgr: Ignore extra __le32 in info payload | expand |
On Fri, Jul 12, 2024 at 12:00:03PM +0200, Stephan Gerhold wrote: > Some newer ADSP firmware versions on X1E80100 report an extra __le32 at the > end of the battery information request payload, causing qcom_battmgr to > fail to initialize. Adjust the check to ignore the extra field in the info > payload so we can support both old and newer firmware versions. > > Tested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org> Can confirm that the old fw still works (didn't really look at the patch): Tested-by: Johan Hovold <johan+linaro@kernel.org> Johan
On Fri, Jul 12, 2024 at 12:00:03PM GMT, Stephan Gerhold wrote: > Some newer ADSP firmware versions on X1E80100 report an extra __le32 at the > end of the battery information request payload, causing qcom_battmgr to > fail to initialize. Adjust the check to ignore the extra field in the info > payload so we can support both old and newer firmware versions. > > Tested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org> > --- > drivers/power/supply/qcom_battmgr.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c > index 46f36dcb185c..a5b5f1251af1 100644 > --- a/drivers/power/supply/qcom_battmgr.c > +++ b/drivers/power/supply/qcom_battmgr.c > @@ -1007,7 +1007,9 @@ static void qcom_battmgr_sc8280xp_callback(struct qcom_battmgr *battmgr, > battmgr->error = 0; > break; > case BATTMGR_BAT_INFO: > - if (payload_len != sizeof(resp->info)) { > + /* some firmware versions report an extra __le32 at the end of the payload */ Any useful information in that extra? > + if (payload_len != sizeof(resp->info) && > + payload_len != (sizeof(resp->info) + sizeof(__le32))) { > dev_warn(battmgr->dev, > "invalid payload length for battery information request: %zd\n", > payload_len); > > --- > base-commit: 3fe121b622825ff8cc995a1e6b026181c48188db > change-id: 20240711-x1e80100-battmgr-1eaab3b8f024 > > Best regards, > -- > Stephan Gerhold <stephan.gerhold@linaro.org> >
On Sat, Jul 13, 2024 at 07:17:51PM +0300, Dmitry Baryshkov wrote: > On Fri, Jul 12, 2024 at 12:00:03PM GMT, Stephan Gerhold wrote: > > Some newer ADSP firmware versions on X1E80100 report an extra __le32 at the > > end of the battery information request payload, causing qcom_battmgr to > > fail to initialize. Adjust the check to ignore the extra field in the info > > payload so we can support both old and newer firmware versions. > > > > Tested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org> > > --- > > drivers/power/supply/qcom_battmgr.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c > > index 46f36dcb185c..a5b5f1251af1 100644 > > --- a/drivers/power/supply/qcom_battmgr.c > > +++ b/drivers/power/supply/qcom_battmgr.c > > @@ -1007,7 +1007,9 @@ static void qcom_battmgr_sc8280xp_callback(struct qcom_battmgr *battmgr, > > battmgr->error = 0; > > break; > > case BATTMGR_BAT_INFO: > > - if (payload_len != sizeof(resp->info)) { > > + /* some firmware versions report an extra __le32 at the end of the payload */ > > Any useful information in that extra? > No, I don't think so. I think we can just ignore it. Thanks, Stephan
On Mon, Jul 15, 2024 at 11:15:16AM GMT, Stephan Gerhold wrote: > On Sat, Jul 13, 2024 at 07:17:51PM +0300, Dmitry Baryshkov wrote: > > On Fri, Jul 12, 2024 at 12:00:03PM GMT, Stephan Gerhold wrote: > > > Some newer ADSP firmware versions on X1E80100 report an extra __le32 at the > > > end of the battery information request payload, causing qcom_battmgr to > > > fail to initialize. Adjust the check to ignore the extra field in the info > > > payload so we can support both old and newer firmware versions. > > > > > > Tested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > > Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org> > > > --- > > > drivers/power/supply/qcom_battmgr.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c > > > index 46f36dcb185c..a5b5f1251af1 100644 > > > --- a/drivers/power/supply/qcom_battmgr.c > > > +++ b/drivers/power/supply/qcom_battmgr.c > > > @@ -1007,7 +1007,9 @@ static void qcom_battmgr_sc8280xp_callback(struct qcom_battmgr *battmgr, > > > battmgr->error = 0; > > > break; > > > case BATTMGR_BAT_INFO: > > > - if (payload_len != sizeof(resp->info)) { > > > + /* some firmware versions report an extra __le32 at the end of the payload */ > > > > Any useful information in that extra? > > > > No, I don't think so. I think we can just ignore it. If that's the case, Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On Fri, 12 Jul 2024 12:00:03 +0200, Stephan Gerhold wrote: > Some newer ADSP firmware versions on X1E80100 report an extra __le32 at the > end of the battery information request payload, causing qcom_battmgr to > fail to initialize. Adjust the check to ignore the extra field in the info > payload so we can support both old and newer firmware versions. > > Applied, thanks! [1/1] power: supply: qcom_battmgr: Ignore extra __le32 in info payload commit: d6cca7631a4b54a8995e3bc53e5afb11d3b0c8ff Best regards,
diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c index 46f36dcb185c..a5b5f1251af1 100644 --- a/drivers/power/supply/qcom_battmgr.c +++ b/drivers/power/supply/qcom_battmgr.c @@ -1007,7 +1007,9 @@ static void qcom_battmgr_sc8280xp_callback(struct qcom_battmgr *battmgr, battmgr->error = 0; break; case BATTMGR_BAT_INFO: - if (payload_len != sizeof(resp->info)) { + /* some firmware versions report an extra __le32 at the end of the payload */ + if (payload_len != sizeof(resp->info) && + payload_len != (sizeof(resp->info) + sizeof(__le32))) { dev_warn(battmgr->dev, "invalid payload length for battery information request: %zd\n", payload_len);