| Message ID | 1582694833-9407-4-git-send-email-mkshah@codeaurora.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v7,1/3] arm64: dts: qcom: sc7180: Add cpuidle low power states | expand |
Quoting Maulik Shah (2020-02-25 21:27:13) > Add changes to invoke rpmh flush when the data in cache is dirty. > > This is done only if OSI is not supported in PSCI. If OSI is supported > rpmh_flush can get invoked when the last cpu going to power collapse Please write rpmh_flush() so we know it's a function and not a variable. > deepest low power mode. > > Signed-off-by: Maulik Shah <mkshah@codeaurora.org> > Reviewed-by: Srinivas Rao L <lsrao@codeaurora.org> > --- > drivers/soc/qcom/rpmh.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c > index 83ba4e0..839af8d 100644 > --- a/drivers/soc/qcom/rpmh.c > +++ b/drivers/soc/qcom/rpmh.c > @@ -12,6 +12,7 @@ > #include <linux/module.h> > #include <linux/of.h> > #include <linux/platform_device.h> > +#include <linux/psci.h> > #include <linux/slab.h> > #include <linux/spinlock.h> > #include <linux/types.h> > @@ -163,6 +164,9 @@ static struct cache_req *cache_rpm_request(struct rpmh_ctrlr *ctrlr, > unlock: > spin_unlock_irqrestore(&ctrlr->cache_lock, flags); > > + if (ctrlr->dirty && !psci_has_osi_support()) Can we introduce a stub function for psci_has_osi_support() when CONFIG_ARM_PSCI_FW=n? This driver currently has: config QCOM_RPMH bool "Qualcomm RPM-Hardened (RPMH) Communication" depends on ARCH_QCOM && ARM64 || COMPILE_TEST which implies that this will break build testing once built on something that isn't arm64. > + return rpmh_flush(ctrlr) ? ERR_PTR(-EINVAL) : req; > + > return req; > } > > @@ -391,6 +395,8 @@ int rpmh_write_batch(const struct device *dev, enum rpmh_state state, > > if (state != RPMH_ACTIVE_ONLY_STATE) { > cache_batch(ctrlr, req); > + if (!psci_has_osi_support()) > + return rpmh_flush(ctrlr); While the diff is small it is also sad that we turn around after adding it to a list and immediately take it off the list and send it. Can't we do this without having to do the list add/remove dance? > return 0; > } >
On 2/27/2020 4:19 AM, Stephen Boyd wrote: > Quoting Maulik Shah (2020-02-25 21:27:13) >> Add changes to invoke rpmh flush when the data in cache is dirty. >> >> This is done only if OSI is not supported in PSCI. If OSI is supported >> rpmh_flush can get invoked when the last cpu going to power collapse > Please write rpmh_flush() so we know it's a function and not a variable. Done. Will update in v8. >> deepest low power mode. >> >> Signed-off-by: Maulik Shah <mkshah@codeaurora.org> >> Reviewed-by: Srinivas Rao L <lsrao@codeaurora.org> >> --- >> drivers/soc/qcom/rpmh.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c >> index 83ba4e0..839af8d 100644 >> --- a/drivers/soc/qcom/rpmh.c >> +++ b/drivers/soc/qcom/rpmh.c >> @@ -12,6 +12,7 @@ >> #include <linux/module.h> >> #include <linux/of.h> >> #include <linux/platform_device.h> >> +#include <linux/psci.h> >> #include <linux/slab.h> >> #include <linux/spinlock.h> >> #include <linux/types.h> >> @@ -163,6 +164,9 @@ static struct cache_req *cache_rpm_request(struct rpmh_ctrlr *ctrlr, >> unlock: >> spin_unlock_irqrestore(&ctrlr->cache_lock, flags); >> >> + if (ctrlr->dirty && !psci_has_osi_support()) > Can we introduce a stub function for psci_has_osi_support() when > CONFIG_ARM_PSCI_FW=n? This driver currently has: > > config QCOM_RPMH > bool "Qualcomm RPM-Hardened (RPMH) Communication" > depends on ARCH_QCOM && ARM64 || COMPILE_TEST > > > which implies that this will break build testing once built on something > that isn't arm64. > Thanks for pointing this, i think its better to remove COMPILE_TEST so driver only gets build for arm64. >> + return rpmh_flush(ctrlr) ? ERR_PTR(-EINVAL) : req; >> + >> return req; >> } >> >> @@ -391,6 +395,8 @@ int rpmh_write_batch(const struct device *dev, enum rpmh_state state, >> >> if (state != RPMH_ACTIVE_ONLY_STATE) { >> cache_batch(ctrlr, req); >> + if (!psci_has_osi_support()) >> + return rpmh_flush(ctrlr); > While the diff is small it is also sad that we turn around after adding > it to a list and immediately take it off the list and send it. Can't we > do this without having to do the list add/remove dance? No, we need to keep it in list, the target supporting OSI will get it off list only when last cpu enters deepest idle mode. > >> return 0; >> } >>
diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c index 83ba4e0..839af8d 100644 --- a/drivers/soc/qcom/rpmh.c +++ b/drivers/soc/qcom/rpmh.c @@ -12,6 +12,7 @@ #include <linux/module.h> #include <linux/of.h> #include <linux/platform_device.h> +#include <linux/psci.h> #include <linux/slab.h> #include <linux/spinlock.h> #include <linux/types.h> @@ -163,6 +164,9 @@ static struct cache_req *cache_rpm_request(struct rpmh_ctrlr *ctrlr, unlock: spin_unlock_irqrestore(&ctrlr->cache_lock, flags); + if (ctrlr->dirty && !psci_has_osi_support()) + return rpmh_flush(ctrlr) ? ERR_PTR(-EINVAL) : req; + return req; } @@ -391,6 +395,8 @@ int rpmh_write_batch(const struct device *dev, enum rpmh_state state, if (state != RPMH_ACTIVE_ONLY_STATE) { cache_batch(ctrlr, req); + if (!psci_has_osi_support()) + return rpmh_flush(ctrlr); return 0; }