| Message ID | 20240819-pmic-glink-v6-11-races-v2-3-88fe3ab1f0e2@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | soc: qcom: pmic_glink: v6.11-rc bug fixes | expand |
On Mon, Aug 19, 2024 at 01:07:47PM -0700, Bjorn Andersson wrote: > When the pmic_glink state is UP and we either receive a protection- > domain (PD) notification indicating that the PD is going down, or that > the whole remoteproc is going down, it's expected that the pmic_glink > client instances are notified that their function has gone DOWN. > > This is not what the code does, which results in the client state either > not updating, or being wrong in many cases. So let's fix the conditions. > @@ -191,7 +191,7 @@ static void pmic_glink_state_notify_clients(struct pmic_glink *pg) > if (pg->pdr_state == SERVREG_SERVICE_STATE_UP && pg->ept) > new_state = SERVREG_SERVICE_STATE_UP; > } else { > - if (pg->pdr_state == SERVREG_SERVICE_STATE_UP && pg->ept) > + if (pg->pdr_state == SERVREG_SERVICE_STATE_DOWN || !pg->ept) > new_state = SERVREG_SERVICE_STATE_DOWN; > } I guess you could drop the outer conditional if (pg->client_state != SERVREG_SERVICE_STATE_UP) { } else { } here to make this a bit more readable, but that's for a separate patch. Reviewed-by: Johan Hovold <johan+linaro@kernel.org> Johan
On Tue, Aug 20, 2024 at 09:07:10AM +0200, Johan Hovold wrote: > On Mon, Aug 19, 2024 at 01:07:47PM -0700, Bjorn Andersson wrote: > > When the pmic_glink state is UP and we either receive a protection- > > domain (PD) notification indicating that the PD is going down, or that > > the whole remoteproc is going down, it's expected that the pmic_glink > > client instances are notified that their function has gone DOWN. > > > > This is not what the code does, which results in the client state either > > not updating, or being wrong in many cases. So let's fix the conditions. And I believe you meant s/with/when/ in the patch Subject. Johan
diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c index e4747f1d3da5..cb202a37e8ab 100644 --- a/drivers/soc/qcom/pmic_glink.c +++ b/drivers/soc/qcom/pmic_glink.c @@ -191,7 +191,7 @@ static void pmic_glink_state_notify_clients(struct pmic_glink *pg) if (pg->pdr_state == SERVREG_SERVICE_STATE_UP && pg->ept) new_state = SERVREG_SERVICE_STATE_UP; } else { - if (pg->pdr_state == SERVREG_SERVICE_STATE_UP && pg->ept) + if (pg->pdr_state == SERVREG_SERVICE_STATE_DOWN || !pg->ept) new_state = SERVREG_SERVICE_STATE_DOWN; }
When the pmic_glink state is UP and we either receive a protection- domain (PD) notification indicating that the PD is going down, or that the whole remoteproc is going down, it's expected that the pmic_glink client instances are notified that their function has gone DOWN. This is not what the code does, which results in the client state either not updating, or being wrong in many cases. So let's fix the conditions. Fixes: 58ef4ece1e41 ("soc: qcom: pmic_glink: Introduce base PMIC GLINK driver") Cc: stable@vger.kernel.org Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Tested-by: Amit Pundir <amit.pundir@linaro.org> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> --- drivers/soc/qcom/pmic_glink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)