2023-10-03 15:29:41

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 0/5] mfd: qcom-spmi-pmic: fix revid implementation

The Qualcomm SPMI PMIC revid implementation is broken in multiple ways
that can lead to resource leaks and crashes. This series reworks the
implementation so that can be used safely.

Included is also a rename of the SPMI device lookup helper which can
hopefully help prevent similar leaks from being reintroduced.

Johan


Johan Hovold (5):
mfd: qcom-spmi-pmic: fix reference leaks in revid helper
mfd: qcom-spmi-pmic: fix revid implementation
mfd: qcom-spmi-pmic: switch to EXPORT_SYMBOL_GPL()
spmi: document spmi_device_from_of() refcounting
spmi: rename spmi device lookup helper

drivers/mfd/qcom-spmi-pmic.c | 103 +++++++++++++++++++++++++----------
drivers/spmi/spmi.c | 9 ++-
include/linux/spmi.h | 2 +-
3 files changed, 80 insertions(+), 34 deletions(-)

--
2.41.0


2023-10-03 15:29:46

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 3/5] mfd: qcom-spmi-pmic: switch to EXPORT_SYMBOL_GPL()

Switch to using EXPORT_SYMBOL_GPL() for the revid helper as there is no
reason not to use it.

Cc: Caleb Connolly <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/mfd/qcom-spmi-pmic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/qcom-spmi-pmic.c b/drivers/mfd/qcom-spmi-pmic.c
index 8e449cff5cec..ee55f09da3ba 100644
--- a/drivers/mfd/qcom-spmi-pmic.c
+++ b/drivers/mfd/qcom-spmi-pmic.c
@@ -241,7 +241,7 @@ const struct qcom_spmi_pmic *qcom_pmic_get(struct device *dev)

return &spmi->pmic;
}
-EXPORT_SYMBOL(qcom_pmic_get);
+EXPORT_SYMBOL_GPL(qcom_pmic_get);

static const struct regmap_config spmi_regmap_config = {
.reg_bits = 16,
--
2.41.0

2023-10-03 15:29:47

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 2/5] mfd: qcom-spmi-pmic: fix revid implementation

The Qualcomm SPMI PMIC revid implementation is broken in multiple ways.

First, it assumes that just because the sibling base device has been
registered that means that it is also bound to a driver, which may not
be the case (e.g. due to probe deferral or asynchronous probe). This
could trigger a NULL-pointer dereference when attempting to access the
driver data of the unbound device.

Second, it accesses driver data of a sibling device directly and without
any locking, which means that the driver data may be freed while it is
being accessed (e.g. on driver unbind).

Third, it leaks a struct device reference to the sibling device which is
looked up using the spmi_device_from_of() every time a function (child)
device is calling the revid function (e.g. on probe).

Fix this mess by reimplementing the revid lookup so that it is done only
at probe of the PMIC device; the base device fetches the revid info from
the hardware, while any secondary SPMI device fetches the information
from the base device and caches it so that it can be accessed safely
from its children. If the base device has not been probed yet then probe
of a secondary device is deferred.

Fixes: e9c11c6e3a0e ("mfd: qcom-spmi-pmic: expose the PMIC revid information to clients")
Cc: [email protected] # 6.0
Cc: Caleb Connolly <[email protected]>
Cc: Dmitry Baryshkov <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/mfd/qcom-spmi-pmic.c | 69 +++++++++++++++++++++++++++---------
1 file changed, 53 insertions(+), 16 deletions(-)

diff --git a/drivers/mfd/qcom-spmi-pmic.c b/drivers/mfd/qcom-spmi-pmic.c
index 47738f7e492c..8e449cff5cec 100644
--- a/drivers/mfd/qcom-spmi-pmic.c
+++ b/drivers/mfd/qcom-spmi-pmic.c
@@ -30,6 +30,8 @@ struct qcom_spmi_dev {
struct qcom_spmi_pmic pmic;
};

+static DEFINE_MUTEX(pmic_spmi_revid_lock);
+
#define N_USIDS(n) ((void *)n)

static const struct of_device_id pmic_spmi_id_table[] = {
@@ -76,24 +78,21 @@ static const struct of_device_id pmic_spmi_id_table[] = {
*
* This only supports PMICs with 1 or 2 USIDs.
*/
-static struct spmi_device *qcom_pmic_get_base_usid(struct device *dev)
+static struct spmi_device *qcom_pmic_get_base_usid(struct spmi_device *sdev, struct qcom_spmi_dev *ctx)
{
- struct spmi_device *sdev;
- struct qcom_spmi_dev *ctx;
struct device_node *spmi_bus;
struct device_node *child;
int function_parent_usid, ret;
u32 pmic_addr;

- sdev = to_spmi_device(dev);
- ctx = dev_get_drvdata(&sdev->dev);
-
/*
* Quick return if the function device is already in the base
* USID. This will always be hit for PMICs with only 1 USID.
*/
- if (sdev->usid % ctx->num_usids == 0)
+ if (sdev->usid % ctx->num_usids == 0) {
+ get_device(&sdev->dev);
return sdev;
+ }

function_parent_usid = sdev->usid;

@@ -118,10 +117,8 @@ static struct spmi_device *qcom_pmic_get_base_usid(struct device *dev)
sdev = spmi_device_from_of(child);
if (!sdev) {
/*
- * If the base USID for this PMIC hasn't probed yet
- * but the secondary USID has, then we need to defer
- * the function driver so that it will attempt to
- * probe again when the base USID is ready.
+ * If the base USID for this PMIC hasn't been
+ * registered yet then we need to defer.
*/
sdev = ERR_PTR(-EPROBE_DEFER);
}
@@ -135,6 +132,35 @@ static struct spmi_device *qcom_pmic_get_base_usid(struct device *dev)
return sdev;
}

+static int pmic_spmi_get_base_revid(struct spmi_device *sdev, struct qcom_spmi_dev *ctx)
+{
+ struct qcom_spmi_dev *base_ctx;
+ struct spmi_device *base;
+ int ret = 0;
+
+ base = qcom_pmic_get_base_usid(sdev, ctx);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ /*
+ * Copy revid info from base device if it has probed and is still
+ * bound to its driver.
+ */
+ mutex_lock(&pmic_spmi_revid_lock);
+ base_ctx = spmi_device_get_drvdata(base);
+ if (!base_ctx) {
+ ret = -EPROBE_DEFER;
+ goto out_unlock;
+ }
+ memcpy(&ctx->pmic, &base_ctx->pmic, sizeof(ctx->pmic));
+out_unlock:
+ mutex_unlock(&pmic_spmi_revid_lock);
+
+ put_device(&base->dev);
+
+ return ret;
+}
+
static int pmic_spmi_load_revid(struct regmap *map, struct device *dev,
struct qcom_spmi_pmic *pmic)
{
@@ -210,11 +236,7 @@ const struct qcom_spmi_pmic *qcom_pmic_get(struct device *dev)
if (!of_match_device(pmic_spmi_id_table, dev->parent))
return ERR_PTR(-EINVAL);

- sdev = qcom_pmic_get_base_usid(dev->parent);
-
- if (IS_ERR(sdev))
- return ERR_CAST(sdev);
-
+ sdev = to_spmi_device(dev->parent);
spmi = dev_get_drvdata(&sdev->dev);

return &spmi->pmic;
@@ -249,16 +271,31 @@ static int pmic_spmi_probe(struct spmi_device *sdev)
ret = pmic_spmi_load_revid(regmap, &sdev->dev, &ctx->pmic);
if (ret < 0)
return ret;
+ } else {
+ ret = pmic_spmi_get_base_revid(sdev, ctx);
+ if (ret)
+ return ret;
}
+
+ mutex_lock(&pmic_spmi_revid_lock);
spmi_device_set_drvdata(sdev, ctx);
+ mutex_unlock(&pmic_spmi_revid_lock);

return devm_of_platform_populate(&sdev->dev);
}

+static void pmic_spmi_remove(struct spmi_device *sdev)
+{
+ mutex_lock(&pmic_spmi_revid_lock);
+ spmi_device_set_drvdata(sdev, NULL);
+ mutex_unlock(&pmic_spmi_revid_lock);
+}
+
MODULE_DEVICE_TABLE(of, pmic_spmi_id_table);

static struct spmi_driver pmic_spmi_driver = {
.probe = pmic_spmi_probe,
+ .remove = pmic_spmi_remove,
.driver = {
.name = "pmic-spmi",
.of_match_table = pmic_spmi_id_table,
--
2.41.0

2023-10-03 15:29:50

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 5/5] spmi: rename spmi device lookup helper

Rename the SPMI device helper which is used to lookup a device from its
OF node as spmi_find_device_by_of_node() so that it reflects the
implementation and matches how other helpers like this are named.

This will specifically make it more clear that this is a lookup function
which returns a reference counted structure.

Signed-off-by: Johan Hovold <[email protected]>
---
drivers/mfd/qcom-spmi-pmic.c | 2 +-
drivers/spmi/spmi.c | 6 +++---
include/linux/spmi.h | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/qcom-spmi-pmic.c b/drivers/mfd/qcom-spmi-pmic.c
index ee55f09da3ba..1c17adeb7a6d 100644
--- a/drivers/mfd/qcom-spmi-pmic.c
+++ b/drivers/mfd/qcom-spmi-pmic.c
@@ -114,7 +114,7 @@ static struct spmi_device *qcom_pmic_get_base_usid(struct spmi_device *sdev, str
}

if (pmic_addr == function_parent_usid - (ctx->num_usids - 1)) {
- sdev = spmi_device_from_of(child);
+ sdev = spmi_find_device_by_of_node(child);
if (!sdev) {
/*
* If the base USID for this PMIC hasn't been
diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
index ca2fd4d72fa6..93cd4a34debc 100644
--- a/drivers/spmi/spmi.c
+++ b/drivers/spmi/spmi.c
@@ -388,7 +388,7 @@ static struct bus_type spmi_bus_type = {
};

/**
- * spmi_device_from_of() - get the associated SPMI device from a device node
+ * spmi_find_device_by_of_node() - look up an SPMI device from a device node
*
* @np: device node
*
@@ -397,7 +397,7 @@ static struct bus_type spmi_bus_type = {
*
* Returns the struct spmi_device associated with a device node or NULL.
*/
-struct spmi_device *spmi_device_from_of(struct device_node *np)
+struct spmi_device *spmi_find_device_by_of_node(struct device_node *np)
{
struct device *dev = bus_find_device_by_of_node(&spmi_bus_type, np);

@@ -405,7 +405,7 @@ struct spmi_device *spmi_device_from_of(struct device_node *np)
return to_spmi_device(dev);
return NULL;
}
-EXPORT_SYMBOL_GPL(spmi_device_from_of);
+EXPORT_SYMBOL_GPL(spmi_find_device_by_of_node);

/**
* spmi_device_alloc() - Allocate a new SPMI device
diff --git a/include/linux/spmi.h b/include/linux/spmi.h
index eac1956a8727..2a4ce4144f9f 100644
--- a/include/linux/spmi.h
+++ b/include/linux/spmi.h
@@ -166,7 +166,7 @@ static inline void spmi_driver_unregister(struct spmi_driver *sdrv)

struct device_node;

-struct spmi_device *spmi_device_from_of(struct device_node *np);
+struct spmi_device *spmi_find_device_by_of_node(struct device_node *np);
int spmi_register_read(struct spmi_device *sdev, u8 addr, u8 *buf);
int spmi_ext_register_read(struct spmi_device *sdev, u8 addr, u8 *buf,
size_t len);
--
2.41.0

2023-10-03 15:29:54

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 4/5] spmi: document spmi_device_from_of() refcounting

Add a comment documenting that the spmi_device_from_of() takes a
reference to the embedded struct device that needs to be dropped after
use.

Signed-off-by: Johan Hovold <[email protected]>
---
drivers/spmi/spmi.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
index 7313d4c18a04..ca2fd4d72fa6 100644
--- a/drivers/spmi/spmi.c
+++ b/drivers/spmi/spmi.c
@@ -392,6 +392,9 @@ static struct bus_type spmi_bus_type = {
*
* @np: device node
*
+ * Takes a reference to the embedded struct device which needs to be dropped
+ * after use.
+ *
* Returns the struct spmi_device associated with a device node or NULL.
*/
struct spmi_device *spmi_device_from_of(struct device_node *np)
--
2.41.0

2023-10-03 15:30:47

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 1/5] mfd: qcom-spmi-pmic: fix reference leaks in revid helper

The Qualcomm SPMI PMIC revid implementation is broken in multiple ways.

First, it totally ignores struct device_node reference counting and
leaks references to the parent bus node as well as each child it
iterates over using an open-coded for_each_child_of_node().

Second, it leaks references to each spmi device on the bus that it
iterates over by failing to drop the reference taken by the
spmi_device_from_of() helper.

Fix the struct device_node leaks by reimplementing the lookup using
for_each_child_of_node() and adding the missing reference count
decrements. Fix the sibling struct device leaks by dropping the
unnecessary lookups of devices with the wrong USID.

Note that this still leaves one struct device reference leak in case a
base device is found but it is not the parent of the device used for the
lookup. This will be addressed in a follow-on patch.

Fixes: e9c11c6e3a0e ("mfd: qcom-spmi-pmic: expose the PMIC revid information to clients")
Cc: [email protected] # 6.0
Cc: Caleb Connolly <[email protected]>
Cc: Dmitry Baryshkov <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/mfd/qcom-spmi-pmic.c | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/mfd/qcom-spmi-pmic.c b/drivers/mfd/qcom-spmi-pmic.c
index 7e2cd79d17eb..47738f7e492c 100644
--- a/drivers/mfd/qcom-spmi-pmic.c
+++ b/drivers/mfd/qcom-spmi-pmic.c
@@ -81,7 +81,7 @@ static struct spmi_device *qcom_pmic_get_base_usid(struct device *dev)
struct spmi_device *sdev;
struct qcom_spmi_dev *ctx;
struct device_node *spmi_bus;
- struct device_node *other_usid = NULL;
+ struct device_node *child;
int function_parent_usid, ret;
u32 pmic_addr;

@@ -105,28 +105,34 @@ static struct spmi_device *qcom_pmic_get_base_usid(struct device *dev)
* device for USID 2.
*/
spmi_bus = of_get_parent(sdev->dev.of_node);
- do {
- other_usid = of_get_next_child(spmi_bus, other_usid);
-
- ret = of_property_read_u32_index(other_usid, "reg", 0, &pmic_addr);
- if (ret)
- return ERR_PTR(ret);
+ sdev = ERR_PTR(-ENODATA);
+ for_each_child_of_node(spmi_bus, child) {
+ ret = of_property_read_u32_index(child, "reg", 0, &pmic_addr);
+ if (ret) {
+ of_node_put(child);
+ sdev = ERR_PTR(ret);
+ break;
+ }

- sdev = spmi_device_from_of(other_usid);
if (pmic_addr == function_parent_usid - (ctx->num_usids - 1)) {
- if (!sdev)
+ sdev = spmi_device_from_of(child);
+ if (!sdev) {
/*
* If the base USID for this PMIC hasn't probed yet
* but the secondary USID has, then we need to defer
* the function driver so that it will attempt to
* probe again when the base USID is ready.
*/
- return ERR_PTR(-EPROBE_DEFER);
- return sdev;
+ sdev = ERR_PTR(-EPROBE_DEFER);
+ }
+ of_node_put(child);
+ break;
}
- } while (other_usid->sibling);
+ }
+
+ of_node_put(spmi_bus);

- return ERR_PTR(-ENODATA);
+ return sdev;
}

static int pmic_spmi_load_revid(struct regmap *map, struct device *dev,
--
2.41.0

2023-10-03 15:57:23

by Caleb Connolly

[permalink] [raw]
Subject: Re: [PATCH 0/5] mfd: qcom-spmi-pmic: fix revid implementation



On 03/10/2023 16:29, Johan Hovold wrote:
> The Qualcomm SPMI PMIC revid implementation is broken in multiple ways
> that can lead to resource leaks and crashes. This series reworks the
> implementation so that can be used safely.
>
> Included is also a rename of the SPMI device lookup helper which can
> hopefully help prevent similar leaks from being reintroduced.
>
> Johan

This is.. definitely a major improvement. Thanks for cleaning up my mess

fwiw
Acked-by: Caleb Connolly <[email protected]>
>
>
> Johan Hovold (5):
> mfd: qcom-spmi-pmic: fix reference leaks in revid helper
> mfd: qcom-spmi-pmic: fix revid implementation
> mfd: qcom-spmi-pmic: switch to EXPORT_SYMBOL_GPL()
> spmi: document spmi_device_from_of() refcounting
> spmi: rename spmi device lookup helper
>
> drivers/mfd/qcom-spmi-pmic.c | 103 +++++++++++++++++++++++++----------
> drivers/spmi/spmi.c | 9 ++-
> include/linux/spmi.h | 2 +-
> 3 files changed, 80 insertions(+), 34 deletions(-)
>

--
// Caleb (they/them)

2023-10-06 23:53:21

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 3/5] mfd: qcom-spmi-pmic: switch to EXPORT_SYMBOL_GPL()

On 3.10.2023 17:29, Johan Hovold wrote:
> Switch to using EXPORT_SYMBOL_GPL() for the revid helper as there is no
> reason not to use it.
>
> Cc: Caleb Connolly <[email protected]>
> Signed-off-by: Johan Hovold <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>

Konrad

2023-10-12 10:21:31

by Lee Jones

[permalink] [raw]
Subject: Re: (subset) [PATCH 0/5] mfd: qcom-spmi-pmic: fix revid implementation

On Tue, 03 Oct 2023 17:29:22 +0200, Johan Hovold wrote:
> The Qualcomm SPMI PMIC revid implementation is broken in multiple ways
> that can lead to resource leaks and crashes. This series reworks the
> implementation so that can be used safely.
>
> Included is also a rename of the SPMI device lookup helper which can
> hopefully help prevent similar leaks from being reintroduced.
>
> [...]

Applied, thanks!

[1/5] mfd: qcom-spmi-pmic: fix reference leaks in revid helper
commit: 365cf70ff33fe20e749227346d7245f7f0dccb58
[2/5] mfd: qcom-spmi-pmic: fix revid implementation
commit: 7370f9de463b1d21dcdf9480a0a15babecd14ac7
[3/5] mfd: qcom-spmi-pmic: switch to EXPORT_SYMBOL_GPL()
commit: b5cd5e72a71e9d368e20271d3a772dd045ae220e

--
Lee Jones [李琼斯]

2023-10-13 09:55:23

by Johan Hovold

[permalink] [raw]
Subject: Re: (subset) [PATCH 0/5] mfd: qcom-spmi-pmic: fix revid implementation

On Thu, Oct 12, 2023 at 11:20:57AM +0100, Lee Jones wrote:
> On Tue, 03 Oct 2023 17:29:22 +0200, Johan Hovold wrote:
> > The Qualcomm SPMI PMIC revid implementation is broken in multiple ways
> > that can lead to resource leaks and crashes. This series reworks the
> > implementation so that can be used safely.
> >
> > Included is also a rename of the SPMI device lookup helper which can
> > hopefully help prevent similar leaks from being reintroduced.
> >
> > [...]
>
> Applied, thanks!
>
> [1/5] mfd: qcom-spmi-pmic: fix reference leaks in revid helper
> commit: 365cf70ff33fe20e749227346d7245f7f0dccb58
> [2/5] mfd: qcom-spmi-pmic: fix revid implementation
> commit: 7370f9de463b1d21dcdf9480a0a15babecd14ac7
> [3/5] mfd: qcom-spmi-pmic: switch to EXPORT_SYMBOL_GPL()
> commit: b5cd5e72a71e9d368e20271d3a772dd045ae220e

Thanks for picking these up, Lee. You don't seem to have pushed these
out yet so I'm not sure if you intend to send them on for 6.6-rc or
6.7-rc1 yet.

Either way, would it be possible to include also the related and
dependant spmi cleanups?

Stephen, could you ack those changes so Lee can take them as well?

Johan

2023-10-13 10:15:31

by Lee Jones

[permalink] [raw]
Subject: Re: (subset) [PATCH 0/5] mfd: qcom-spmi-pmic: fix revid implementation

On Fri, 13 Oct 2023, Johan Hovold wrote:

> On Thu, Oct 12, 2023 at 11:20:57AM +0100, Lee Jones wrote:
> > On Tue, 03 Oct 2023 17:29:22 +0200, Johan Hovold wrote:
> > > The Qualcomm SPMI PMIC revid implementation is broken in multiple ways
> > > that can lead to resource leaks and crashes. This series reworks the
> > > implementation so that can be used safely.
> > >
> > > Included is also a rename of the SPMI device lookup helper which can
> > > hopefully help prevent similar leaks from being reintroduced.
> > >
> > > [...]
> >
> > Applied, thanks!
> >
> > [1/5] mfd: qcom-spmi-pmic: fix reference leaks in revid helper
> > commit: 365cf70ff33fe20e749227346d7245f7f0dccb58
> > [2/5] mfd: qcom-spmi-pmic: fix revid implementation
> > commit: 7370f9de463b1d21dcdf9480a0a15babecd14ac7
> > [3/5] mfd: qcom-spmi-pmic: switch to EXPORT_SYMBOL_GPL()
> > commit: b5cd5e72a71e9d368e20271d3a772dd045ae220e
>
> Thanks for picking these up, Lee. You don't seem to have pushed these
> out yet so I'm not sure if you intend to send them on for 6.6-rc or
> 6.7-rc1 yet.

Just pushed. Should be in -next tomorrow.

These do not fix commits from this release and I'm not planning on
sending another PR for the current rcs, so these are due for v6.7-rc1.

> Either way, would it be possible to include also the related and
> dependant spmi cleanups?

With Acks, sure.

> Stephen, could you ack those changes so Lee can take them as well?


--
Lee Jones [李琼斯]

2023-10-23 07:06:32

by Johan Hovold

[permalink] [raw]
Subject: Need SPMI ack (was: Re: (subset) [PATCH 0/5] mfd: qcom-spmi-pmic: fix revid implementation)

Hi Stephen,

On Fri, Oct 13, 2023 at 11:55:27AM +0200, Johan Hovold wrote:
> On Thu, Oct 12, 2023 at 11:20:57AM +0100, Lee Jones wrote:
> > On Tue, 03 Oct 2023 17:29:22 +0200, Johan Hovold wrote:
> > > The Qualcomm SPMI PMIC revid implementation is broken in multiple ways
> > > that can lead to resource leaks and crashes. This series reworks the
> > > implementation so that can be used safely.
> > >
> > > Included is also a rename of the SPMI device lookup helper which can
> > > hopefully help prevent similar leaks from being reintroduced.
> > >
> > > [...]
> >
> > Applied, thanks!
> >
> > [1/5] mfd: qcom-spmi-pmic: fix reference leaks in revid helper
> > commit: 365cf70ff33fe20e749227346d7245f7f0dccb58
> > [2/5] mfd: qcom-spmi-pmic: fix revid implementation
> > commit: 7370f9de463b1d21dcdf9480a0a15babecd14ac7
> > [3/5] mfd: qcom-spmi-pmic: switch to EXPORT_SYMBOL_GPL()
> > commit: b5cd5e72a71e9d368e20271d3a772dd045ae220e
>
> Thanks for picking these up, Lee. You don't seem to have pushed these
> out yet so I'm not sure if you intend to send them on for 6.6-rc or
> 6.7-rc1 yet.
>
> Either way, would it be possible to include also the related and
> dependant spmi cleanups?
>
> Stephen, could you ack those changes so Lee can take them as well?

Could you please consider acking the two SPMI patches so that they can
go in through Lee's tree along with the rest of the series for 6.7-rc1
(e.g. to avoid dealing with inter-tree dependencies)?

Johan

2023-10-24 01:58:35

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH 5/5] spmi: rename spmi device lookup helper

Quoting Johan Hovold (2023-10-03 08:29:27)
> Rename the SPMI device helper which is used to lookup a device from its
> OF node as spmi_find_device_by_of_node() so that it reflects the
> implementation and matches how other helpers like this are named.
>
> This will specifically make it more clear that this is a lookup function
> which returns a reference counted structure.
>
> Signed-off-by: Johan Hovold <[email protected]>
> ---

Acked-by: Stephen Boyd <[email protected]>

2023-10-24 01:58:55

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH 4/5] spmi: document spmi_device_from_of() refcounting

Quoting Johan Hovold (2023-10-03 08:29:26)
> Add a comment documenting that the spmi_device_from_of() takes a
> reference to the embedded struct device that needs to be dropped after
> use.
>
> Signed-off-by: Johan Hovold <[email protected]>
> ---

Acked-by: Stephen Boyd <[email protected]>

2023-10-25 12:17:22

by Lee Jones

[permalink] [raw]
Subject: Re: (subset) [PATCH 0/5] mfd: qcom-spmi-pmic: fix revid implementation

On Tue, 03 Oct 2023 17:29:22 +0200, Johan Hovold wrote:
> The Qualcomm SPMI PMIC revid implementation is broken in multiple ways
> that can lead to resource leaks and crashes. This series reworks the
> implementation so that can be used safely.
>
> Included is also a rename of the SPMI device lookup helper which can
> hopefully help prevent similar leaks from being reintroduced.
>
> [...]

Applied, thanks!

[4/5] spmi: document spmi_device_from_of() refcounting
commit: 7db72c01ae2359dbab29f4a60cda49757cf84516
[5/5] spmi: rename spmi device lookup helper
(no commit info)

--
Lee Jones [李琼斯]

2023-10-25 12:17:27

by Lee Jones

[permalink] [raw]
Subject: Re: (subset) [PATCH 4/5] spmi: document spmi_device_from_of() refcounting

On Tue, 03 Oct 2023 17:29:26 +0200, Johan Hovold wrote:
> Add a comment documenting that the spmi_device_from_of() takes a
> reference to the embedded struct device that needs to be dropped after
> use.
>
>

Applied, thanks!

[4/5] spmi: document spmi_device_from_of() refcounting
commit: 7db72c01ae2359dbab29f4a60cda49757cf84516

--
Lee Jones [李琼斯]

2023-10-25 12:17:57

by Lee Jones

[permalink] [raw]
Subject: Re: (subset) [PATCH 5/5] spmi: rename spmi device lookup helper

On Tue, 03 Oct 2023 17:29:27 +0200, Johan Hovold wrote:
> Rename the SPMI device helper which is used to lookup a device from its
> OF node as spmi_find_device_by_of_node() so that it reflects the
> implementation and matches how other helpers like this are named.
>
> This will specifically make it more clear that this is a lookup function
> which returns a reference counted structure.
>
> [...]

Applied, thanks!

[5/5] spmi: rename spmi device lookup helper
commit: 2a2aaed224d93093f4c94234c796ae3396bdceee

--
Lee Jones [李琼斯]

2023-10-25 12:18:46

by Lee Jones

[permalink] [raw]
Subject: Re: (subset) [PATCH 0/5] mfd: qcom-spmi-pmic: fix revid implementation

On Wed, 25 Oct 2023, Lee Jones wrote:

> On Tue, 03 Oct 2023 17:29:22 +0200, Johan Hovold wrote:
> > The Qualcomm SPMI PMIC revid implementation is broken in multiple ways
> > that can lead to resource leaks and crashes. This series reworks the
> > implementation so that can be used safely.
> >
> > Included is also a rename of the SPMI device lookup helper which can
> > hopefully help prevent similar leaks from being reintroduced.
> >
> > [...]
>
> Applied, thanks!
>
> [4/5] spmi: document spmi_device_from_of() refcounting
> commit: 7db72c01ae2359dbab29f4a60cda49757cf84516
> [5/5] spmi: rename spmi device lookup helper
> (no commit info)

Not entirely sure why B4 is sending out these separately!

Still, both applied, thanks.

--
Lee Jones [李琼斯]

2023-10-25 12:25:06

by Johan Hovold

[permalink] [raw]
Subject: Re: (subset) [PATCH 0/5] mfd: qcom-spmi-pmic: fix revid implementation

On Wed, Oct 25, 2023 at 01:18:27PM +0100, Lee Jones wrote:
> On Wed, 25 Oct 2023, Lee Jones wrote:
> > On Tue, 03 Oct 2023 17:29:22 +0200, Johan Hovold wrote:

> > > Included is also a rename of the SPMI device lookup helper which can
> > > hopefully help prevent similar leaks from being reintroduced.

> > Applied, thanks!
> >
> > [4/5] spmi: document spmi_device_from_of() refcounting
> > commit: 7db72c01ae2359dbab29f4a60cda49757cf84516
> > [5/5] spmi: rename spmi device lookup helper
> > (no commit info)
>
> Not entirely sure why B4 is sending out these separately!
>
> Still, both applied, thanks.

Thanks for taking these, and thanks for the acks Stephen!

Johan