| 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: <Z2H_7Xry3R2fWpMZ@intel.com>
Date: Tue, 17 Dec 2024 17:49:17 -0500
From: Rodrigo Vivi <rodrigo.vivi@...el.com>
To: Alexander Usyskin <alexander.usyskin@...el.com>
CC: Miquel Raynal <miquel.raynal@...tlin.com>, Richard Weinberger
<richard@....at>, Vignesh Raghavendra <vigneshr@...com>, Lucas De Marchi
<lucas.demarchi@...el.com>, Thomas Hellström
<thomas.hellstrom@...ux.intel.com>, Maarten Lankhorst
<maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, Jani Nikula <jani.nikula@...ux.intel.com>,
Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>, Tvrtko Ursulin
<tursulin@...ulin.net>, Oren Weil <oren.jer.weil@...el.com>,
<linux-mtd@...ts.infradead.org>, <dri-devel@...ts.freedesktop.org>,
<intel-gfx@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 06/10] mtd: intel-dg: wake card on operations
On Tue, Nov 19, 2024 at 04:01:08PM +0200, Alexander Usyskin wrote:
> Enable runtime PM in mtd driver to notify graphics driver that
> whole card should be kept awake while nvm operations are
> performed through this driver.
>
> CC: Lucas De Marchi <lucas.demarchi@...el.com>
> Acked-by: Miquel Raynal <miquel.raynal@...tlin.com>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@...el.com>
> ---
> drivers/mtd/devices/mtd-intel-dg.c | 70 +++++++++++++++++++++++++-----
> 1 file changed, 58 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mtd/devices/mtd-intel-dg.c b/drivers/mtd/devices/mtd-intel-dg.c
> index 230bf444b7fe..9dd23b11ee95 100644
> --- a/drivers/mtd/devices/mtd-intel-dg.c
> +++ b/drivers/mtd/devices/mtd-intel-dg.c
> @@ -15,11 +15,14 @@
> #include <linux/module.h>
> #include <linux/mtd/mtd.h>
> #include <linux/mtd/partitions.h>
> +#include <linux/pm_runtime.h>
> #include <linux/string.h>
> #include <linux/slab.h>
> #include <linux/sizes.h>
> #include <linux/types.h>
>
> +#define INTEL_DG_NVM_RPM_TIMEOUT 500
> +
> struct intel_dg_nvm {
> struct kref refcnt;
> struct mtd_info mtd;
> @@ -460,6 +463,7 @@ static int intel_dg_mtd_erase(struct mtd_info *mtd, struct erase_info *info)
> loff_t from;
> size_t len;
> size_t total_len;
> + int ret = 0;
>
> if (WARN_ON(!nvm))
> return -EINVAL;
> @@ -474,20 +478,28 @@ static int intel_dg_mtd_erase(struct mtd_info *mtd, struct erase_info *info)
> total_len = info->len;
> addr = info->addr;
>
> + ret = pm_runtime_resume_and_get(mtd->dev.parent);
on this, I really don't believe this is right and we should use
the parent child relation ship in our favor and only have the mtd
device to handle their own runtime pm...
> + if (ret < 0) {
> + dev_err(&mtd->dev, "rpm: get failed %d\n", ret);
> + return ret;
> + }
> +
> guard(mutex)(&nvm->lock);
>
> while (total_len > 0) {
> if (!IS_ALIGNED(addr, SZ_4K) || !IS_ALIGNED(total_len, SZ_4K)) {
> dev_err(&mtd->dev, "unaligned erase %llx %zx\n", addr, total_len);
> info->fail_addr = addr;
> - return -ERANGE;
> + ret = -ERANGE;
> + goto out;
> }
>
> idx = idg_nvm_get_region(nvm, addr);
> if (idx >= nvm->nregions) {
> dev_err(&mtd->dev, "out of range");
> info->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
> - return -ERANGE;
> + ret = -ERANGE;
> + goto out;
> }
>
> from = addr - nvm->regions[idx].offset;
> @@ -503,14 +515,18 @@ static int intel_dg_mtd_erase(struct mtd_info *mtd, struct erase_info *info)
> if (bytes < 0) {
> dev_dbg(&mtd->dev, "erase failed with %zd\n", bytes);
> info->fail_addr += nvm->regions[idx].offset;
> - return bytes;
> + ret = bytes;
> + goto out;
> }
>
> addr += len;
> total_len -= len;
> }
>
> - return 0;
> +out:
> + pm_runtime_mark_last_busy(mtd->dev.parent);
> + pm_runtime_put_autosuspend(mtd->dev.parent);
> + return ret;
> }
>
> static int intel_dg_mtd_read(struct mtd_info *mtd, loff_t from, size_t len,
> @@ -539,17 +555,25 @@ static int intel_dg_mtd_read(struct mtd_info *mtd, loff_t from, size_t len,
> if (len > nvm->regions[idx].size - from)
> len = nvm->regions[idx].size - from;
>
> + ret = pm_runtime_resume_and_get(mtd->dev.parent);
> + if (ret < 0) {
> + dev_err(&mtd->dev, "rpm: get failed %zd\n", ret);
> + return ret;
> + }
> +
> guard(mutex)(&nvm->lock);
>
> ret = idg_read(nvm, region, from, len, buf);
> if (ret < 0) {
> dev_dbg(&mtd->dev, "read failed with %zd\n", ret);
> - return ret;
> + } else {
> + *retlen = ret;
> + ret = 0;
> }
>
> - *retlen = ret;
> -
> - return 0;
> + pm_runtime_mark_last_busy(mtd->dev.parent);
> + pm_runtime_put_autosuspend(mtd->dev.parent);
> + return ret;
> }
>
> static int intel_dg_mtd_write(struct mtd_info *mtd, loff_t to, size_t len,
> @@ -578,17 +602,25 @@ static int intel_dg_mtd_write(struct mtd_info *mtd, loff_t to, size_t len,
> if (len > nvm->regions[idx].size - to)
> len = nvm->regions[idx].size - to;
>
> + ret = pm_runtime_resume_and_get(mtd->dev.parent);
> + if (ret < 0) {
> + dev_err(&mtd->dev, "rpm: get failed %zd\n", ret);
> + return ret;
> + }
> +
> guard(mutex)(&nvm->lock);
>
> ret = idg_write(nvm, region, to, len, buf);
> if (ret < 0) {
> dev_dbg(&mtd->dev, "write failed with %zd\n", ret);
> - return ret;
> + } else {
> + *retlen = ret;
> + ret = 0;
> }
>
> - *retlen = ret;
> -
> - return 0;
> + pm_runtime_mark_last_busy(mtd->dev.parent);
> + pm_runtime_put_autosuspend(mtd->dev.parent);
> + return ret;
> }
>
> static void intel_dg_nvm_release(struct kref *kref)
> @@ -720,6 +752,17 @@ static int intel_dg_mtd_probe(struct auxiliary_device *aux_dev,
> n++;
> }
>
> + devm_pm_runtime_enable(device);
> +
> + pm_runtime_set_autosuspend_delay(device, INTEL_DG_NVM_RPM_TIMEOUT);
> + pm_runtime_use_autosuspend(device);
> +
> + ret = pm_runtime_resume_and_get(device);
> + if (ret < 0) {
> + dev_err(device, "rpm: get failed %d\n", ret);
> + goto err_norpm;
> + }
> +
> nvm->base = devm_ioremap_resource(device, &invm->bar);
> if (IS_ERR(nvm->base)) {
> dev_err(device, "mmio not mapped\n");
> @@ -742,9 +785,12 @@ static int intel_dg_mtd_probe(struct auxiliary_device *aux_dev,
>
> dev_set_drvdata(&aux_dev->dev, nvm);
>
> + pm_runtime_put(device);
> return 0;
>
> err:
> + pm_runtime_put(device);
> +err_norpm:
> kref_put(&nvm->refcnt, intel_dg_nvm_release);
> return ret;
> }
> --
> 2.43.0
>
Powered by blists - more mailing lists