| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20241227130236.755794-1-quic_msavaliy@quicinc.com>
Date: Fri, 27 Dec 2024 18:32:36 +0530
From: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
To: andi.shyti@...nel.org, linux-arm-msm@...r.kernel.org,
linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org,
quic_bjorande@...cinc.com
Cc: konrad.dybcio@...aro.org, quic_vdadhani@...cinc.com, vkoul@...nel.org,
Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
Subject: [PATCH v5] i2c: i2c-qcom-geni: Serve transfer during early resume stage
Fix the issue where pm_runtime_get_sync() fails during the PM early resume
phase, returning -EACCES because runtime PM for the device is disabled at
this stage. This failure causes I2C transfers to fail. To resolve this,
serve transfers with a forced resume.
Allow certain I2C clients, such as PCI or Touch devices, to request I2C
transfers during the early resume stage. Enable any I2C client to initiate
a transfer request very early in the resume stage, such as during the
noirq phase of PM. Register an interrupt with the IRQF_EARLY_RESUME and
IRQF_NO_SUSPEND flags to handle these transfers and avoid timeouts when
IRQs are not enabled during the early stage.
A Potential usecase: PCIe client -> PCIe Driver -> I2C driver. It involves
a PCIe client driver communicating with the PCIe driver, which in turn
interfaces with the I2C driver. Upon powering on the PCIe device, send
certain configurations over I2C. During the suspend phase, use the
suspend_noirq() routine to turn off the PCIe device, as some PCIe clients
continue transfers until this phase. During the resume_noirq() phase,
restore power to the PCIe device and reconfigure it via I2C. This ensures
that the PCIe device is properly configured and operational after resuming
from a suspended state.
Use pm_runtime_force_resume() to address the failure of
pm_runtime_get_sync() returning a negative value when runtime PM is
disabled.
Co-developed-by: Viken Dadhaniya <quic_vdadhani@...cinc.com>
Signed-off-by: Viken Dadhaniya <quic_vdadhani@...cinc.com>
Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
---
Link to V4: https://lore.kernel.org/lkml/bd699719-4958-445a-a685-4b5f6a8ad81f@quicinc.com/
v4->v5:
- Commit log enhanced considering Bjorn's comments by explaining PCIe usecase.
- Enhanced comment with reason when using pm_runtime_force_resume().
- Corrected IS_ENABLED(CONFIG_PM) condition inside geni_i2c_xfer().
- Improved debug log as per Bjorn's suggestions during suspend, resume.
- Reverted back comment before devm_request_irq().
---
Link to V3: https://lore.kernel.org/all/20241119143031.3331753-1-quic_msavaliy@quicinc.com/T/
v3->v4 :
- Enhanced commit log by explaining client usecase scenario during early resume.
- Covered 'usage_count' of 'struct dev_pm_info' under CONFIG_PM to compile non PM CONFIG.
---
Link to V2: https://lore.kernel.org/lkml/202410132233.P25W2vKq-lkp@intel.com/T/
v2 -> v3:
- Updated exact usecase and scenario in the commit log description.
- Removed bulleted points from technical description, added details in free flow.
- Used pm_runtime_force_resume/suspend() instead customized local implementation.
- Added debug log after pm_runtime_force_suspend().
---
v1 -> v2:
- Changed gi2c->se.dev to dev during dev_dbg() calls.
- Addressed review comments from Andi and Bjorn.
- Returned 0 instead garbage inside geni_i2c_force_resume().
- Added comments explaining forced resume transfer when runtime PM
remains disabled.
---
V1 link: https://patches.linaro.org/project/linux-i2c/patch/20240328123743.1713696-1-quic_msavaliy@quicinc.com/
---
---
drivers/i2c/busses/i2c-qcom-geni.c | 47 +++++++++++++++++++++++-------
1 file changed, 37 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 7a22e1f46e60..1885e1ceb11c 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -695,17 +695,31 @@ static int geni_i2c_xfer(struct i2c_adapter *adap,
int num)
{
struct geni_i2c_dev *gi2c = i2c_get_adapdata(adap);
+ struct device *dev = gi2c->se.dev;
int ret;
gi2c->err = 0;
reinit_completion(&gi2c->done);
- ret = pm_runtime_get_sync(gi2c->se.dev);
- if (ret < 0) {
- dev_err(gi2c->se.dev, "error turning SE resources:%d\n", ret);
- pm_runtime_put_noidle(gi2c->se.dev);
- /* Set device in suspended since resume failed */
- pm_runtime_set_suspended(gi2c->se.dev);
- return ret;
+ /* During early resume stage, runtime PM is disabled and pm_runtime_get_sync()
+ * returns error Hence use force_resume() and serve transfer.
+ */
+ if (!pm_runtime_enabled(dev) && gi2c->suspended) {
+ #if (!IS_ENABLED(CONFIG_PM))
+ dev_dbg(dev, "Forced power ON, pm_usage_count: %d\n",
+ atomic_read(&dev->power.usage_count));
+ #endif
+ ret = pm_runtime_force_resume(dev);
+ if (ret)
+ return ret;
+ } else {
+ ret = pm_runtime_get_sync(gi2c->se.dev);
+ if (ret < 0) {
+ dev_err(gi2c->se.dev, "Error turning resources: %d\n", ret);
+ pm_runtime_put_noidle(gi2c->se.dev);
+ /* Set device in suspended since resume failed */
+ pm_runtime_set_suspended(gi2c->se.dev);
+ return ret;
+ }
}
qcom_geni_i2c_conf(gi2c);
@@ -715,8 +729,20 @@ static int geni_i2c_xfer(struct i2c_adapter *adap,
else
ret = geni_i2c_fifo_xfer(gi2c, msgs, num);
- pm_runtime_mark_last_busy(gi2c->se.dev);
- pm_runtime_put_autosuspend(gi2c->se.dev);
+ /* if Runtime PM is disabled, do force_suspend() else autosuspend the driver */
+ if (!pm_runtime_enabled(dev) && !gi2c->suspended) {
+ ret = pm_runtime_force_suspend(dev);
+ #if (!IS_ENABLED(CONFIG_PM))
+ dev_dbg(dev, "Forced power OFF, pm_usage_count: %d\n",
+ atomic_read(&dev->power.usage_count));
+ #endif
+ if (ret)
+ return ret;
+ } else {
+ pm_runtime_mark_last_busy(gi2c->se.dev);
+ pm_runtime_put_autosuspend(gi2c->se.dev);
+ }
+
gi2c->cur = NULL;
gi2c->err = 0;
return ret;
@@ -835,7 +861,8 @@ static int geni_i2c_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, gi2c);
/* Keep interrupts disabled initially to allow for low-power modes */
- ret = devm_request_irq(dev, gi2c->irq, geni_i2c_irq, IRQF_NO_AUTOEN,
+ ret = devm_request_irq(dev, gi2c->irq, geni_i2c_irq,
+ IRQF_NO_AUTOEN | IRQF_EARLY_RESUME | IRQF_NO_SUSPEND,
dev_name(dev), gi2c);
if (ret) {
dev_err(dev, "Request_irq failed:%d: err:%d\n",
--
2.25.1
Powered by blists - more mailing lists