From: Mathieu Poirier <[email protected]>
Multi-cluster remoteproc designs typically have the following DT
declaration:
remoteproc-cluster {
compatible = "soc,remoteproc-cluster";
core0: core0 {
compatible = "soc,remoteproc-core"
memory-region;
sram;
};
core1: core1 {
compatible = "soc,remoteproc-core"
memory-region;
sram;
}
};
A driver exists for the cluster rather than the individual cores
themselves so that operation mode and HW specific configurations
applicable to the cluster can be made.
Because the driver exists at the cluster level and not the individual
core level, function rproc_get_by_phandle() fails to return the
remoteproc associated with the phandled it is called for.
This patch enhances rproc_get_by_phandle() by looking for the cluster's
driver when the driver for the immediate remoteproc's parent is not
found.
Reported-by: Ben Levinsky <[email protected]>
Signed-off-by: Mathieu Poirier <[email protected]>
Co-developed-by: Tarak Reddy <[email protected]>
Signed-off-by: Tarak Reddy <[email protected]>
Co-developed-by: Tanmay Shah <[email protected]>
Signed-off-by: Tanmay Shah <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 29 ++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 695cce218e8c..f276956f2c5c 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -33,6 +33,7 @@
#include <linux/idr.h>
#include <linux/elf.h>
#include <linux/crc32.h>
+#include <linux/of_platform.h>
#include <linux/of_reserved_mem.h>
#include <linux/virtio_ids.h>
#include <linux/virtio_ring.h>
@@ -2112,6 +2113,7 @@ EXPORT_SYMBOL(rproc_detach);
struct rproc *rproc_get_by_phandle(phandle phandle)
{
struct rproc *rproc = NULL, *r;
+ struct device_driver *driver;
struct device_node *np;
np = of_find_node_by_phandle(phandle);
@@ -2122,7 +2124,26 @@ struct rproc *rproc_get_by_phandle(phandle phandle)
list_for_each_entry_rcu(r, &rproc_list, node) {
if (r->dev.parent && device_match_of_node(r->dev.parent, np)) {
/* prevent underlying implementation from being removed */
- if (!try_module_get(r->dev.parent->driver->owner)) {
+
+ /*
+ * If the remoteproc's parent has a driver, the
+ * remoteproc is not part of a cluster and we can use
+ * that driver.
+ */
+ driver = r->dev.parent->driver;
+
+ /*
+ * If the remoteproc's parent does not have a driver,
+ * look for the driver associated with the cluster.
+ */
+ if (!driver) {
+ if (r->dev.parent->parent)
+ driver = r->dev.parent->parent->driver;
+ if (!driver)
+ break;
+ }
+
+ if (!try_module_get(driver->owner)) {
dev_err(&r->dev, "can't get owner\n");
break;
}
@@ -2533,7 +2554,11 @@ EXPORT_SYMBOL(rproc_free);
*/
void rproc_put(struct rproc *rproc)
{
- module_put(rproc->dev.parent->driver->owner);
+ if (rproc->dev.parent->driver)
+ module_put(rproc->dev.parent->driver->owner);
+ else
+ module_put(rproc->dev.parent->parent->driver->owner);
+
put_device(&rproc->dev);
}
EXPORT_SYMBOL(rproc_put);
base-commit: 99f59b148871dadb9104366e3d25b120a97f897b
--
2.25.1
Rejected-by: Tanmay Shah <[email protected]>
I will send new v5 with change long included.
On 1/30/24 9:48 AM, Tanmay Shah wrote:
> From: Mathieu Poirier <[email protected]>
>
> Multi-cluster remoteproc designs typically have the following DT
> declaration:
>
> remoteproc-cluster {
> compatible = "soc,remoteproc-cluster";
>
> core0: core0 {
> compatible = "soc,remoteproc-core"
> memory-region;
> sram;
> };
>
> core1: core1 {
> compatible = "soc,remoteproc-core"
> memory-region;
> sram;
> }
> };
>
> A driver exists for the cluster rather than the individual cores
> themselves so that operation mode and HW specific configurations
> applicable to the cluster can be made.
>
> Because the driver exists at the cluster level and not the individual
> core level, function rproc_get_by_phandle() fails to return the
> remoteproc associated with the phandled it is called for.
>
> This patch enhances rproc_get_by_phandle() by looking for the cluster's
> driver when the driver for the immediate remoteproc's parent is not
> found.
>
> Reported-by: Ben Levinsky <[email protected]>
> Signed-off-by: Mathieu Poirier <[email protected]>
> Co-developed-by: Tarak Reddy <[email protected]>
> Signed-off-by: Tarak Reddy <[email protected]>
> Co-developed-by: Tanmay Shah <[email protected]>
> Signed-off-by: Tanmay Shah <[email protected]>
> ---
> drivers/remoteproc/remoteproc_core.c | 29 ++++++++++++++++++++++++++--
> 1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 695cce218e8c..f276956f2c5c 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -33,6 +33,7 @@
> #include <linux/idr.h>
> #include <linux/elf.h>
> #include <linux/crc32.h>
> +#include <linux/of_platform.h>
> #include <linux/of_reserved_mem.h>
> #include <linux/virtio_ids.h>
> #include <linux/virtio_ring.h>
> @@ -2112,6 +2113,7 @@ EXPORT_SYMBOL(rproc_detach);
> struct rproc *rproc_get_by_phandle(phandle phandle)
> {
> struct rproc *rproc = NULL, *r;
> + struct device_driver *driver;
> struct device_node *np;
>
> np = of_find_node_by_phandle(phandle);
> @@ -2122,7 +2124,26 @@ struct rproc *rproc_get_by_phandle(phandle phandle)
> list_for_each_entry_rcu(r, &rproc_list, node) {
> if (r->dev.parent && device_match_of_node(r->dev.parent, np)) {
> /* prevent underlying implementation from being removed */
> - if (!try_module_get(r->dev.parent->driver->owner)) {
> +
> + /*
> + * If the remoteproc's parent has a driver, the
> + * remoteproc is not part of a cluster and we can use
> + * that driver.
> + */
> + driver = r->dev.parent->driver;
> +
> + /*
> + * If the remoteproc's parent does not have a driver,
> + * look for the driver associated with the cluster.
> + */
> + if (!driver) {
> + if (r->dev.parent->parent)
> + driver = r->dev.parent->parent->driver;
> + if (!driver)
> + break;
> + }
> +
> + if (!try_module_get(driver->owner)) {
> dev_err(&r->dev, "can't get owner\n");
> break;
> }
> @@ -2533,7 +2554,11 @@ EXPORT_SYMBOL(rproc_free);
> */
> void rproc_put(struct rproc *rproc)
> {
> - module_put(rproc->dev.parent->driver->owner);
> + if (rproc->dev.parent->driver)
> + module_put(rproc->dev.parent->driver->owner);
> + else
> + module_put(rproc->dev.parent->parent->driver->owner);
> +
> put_device(&rproc->dev);
> }
> EXPORT_SYMBOL(rproc_put);
>
> base-commit: 99f59b148871dadb9104366e3d25b120a97f897b
I am sorry, I missed the fact that this patch was picked up and available on for-next branch.
Won't be sending new one now.
Thanks,
Tanmay
On 2/7/24 4:18 PM, Tanmay Shah wrote:
> Rejected-by: Tanmay Shah <[email protected]>
>
> I will send new v5 with change long included.
>
> On 1/30/24 9:48 AM, Tanmay Shah wrote:
> > From: Mathieu Poirier <[email protected]>
> >
> > Multi-cluster remoteproc designs typically have the following DT
> > declaration:
> >
> > remoteproc-cluster {
> > compatible = "soc,remoteproc-cluster";
> >
> > core0: core0 {
> > compatible = "soc,remoteproc-core"
> > memory-region;
> > sram;
> > };
> >
> > core1: core1 {
> > compatible = "soc,remoteproc-core"
> > memory-region;
> > sram;
> > }
> > };
> >
> > A driver exists for the cluster rather than the individual cores
> > themselves so that operation mode and HW specific configurations
> > applicable to the cluster can be made.
> >
> > Because the driver exists at the cluster level and not the individual
> > core level, function rproc_get_by_phandle() fails to return the
> > remoteproc associated with the phandled it is called for.
> >
> > This patch enhances rproc_get_by_phandle() by looking for the cluster's
> > driver when the driver for the immediate remoteproc's parent is not
> > found.
> >
> > Reported-by: Ben Levinsky <[email protected]>
> > Signed-off-by: Mathieu Poirier <[email protected]>
> > Co-developed-by: Tarak Reddy <[email protected]>
> > Signed-off-by: Tarak Reddy <[email protected]>
> > Co-developed-by: Tanmay Shah <[email protected]>
> > Signed-off-by: Tanmay Shah <[email protected]>
> > ---
> > drivers/remoteproc/remoteproc_core.c | 29 ++++++++++++++++++++++++++--
> > 1 file changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> > index 695cce218e8c..f276956f2c5c 100644
> > --- a/drivers/remoteproc/remoteproc_core.c
> > +++ b/drivers/remoteproc/remoteproc_core.c
> > @@ -33,6 +33,7 @@
> > #include <linux/idr.h>
> > #include <linux/elf.h>
> > #include <linux/crc32.h>
> > +#include <linux/of_platform.h>
> > #include <linux/of_reserved_mem.h>
> > #include <linux/virtio_ids.h>
> > #include <linux/virtio_ring.h>
> > @@ -2112,6 +2113,7 @@ EXPORT_SYMBOL(rproc_detach);
> > struct rproc *rproc_get_by_phandle(phandle phandle)
> > {
> > struct rproc *rproc = NULL, *r;
> > + struct device_driver *driver;
> > struct device_node *np;
> >
> > np = of_find_node_by_phandle(phandle);
> > @@ -2122,7 +2124,26 @@ struct rproc *rproc_get_by_phandle(phandle phandle)
> > list_for_each_entry_rcu(r, &rproc_list, node) {
> > if (r->dev.parent && device_match_of_node(r->dev.parent, np)) {
> > /* prevent underlying implementation from being removed */
> > - if (!try_module_get(r->dev.parent->driver->owner)) {
> > +
> > + /*
> > + * If the remoteproc's parent has a driver, the
> > + * remoteproc is not part of a cluster and we can use
> > + * that driver.
> > + */
> > + driver = r->dev.parent->driver;
> > +
> > + /*
> > + * If the remoteproc's parent does not have a driver,
> > + * look for the driver associated with the cluster.
> > + */
> > + if (!driver) {
> > + if (r->dev.parent->parent)
> > + driver = r->dev.parent->parent->driver;
> > + if (!driver)
> > + break;
> > + }
> > +
> > + if (!try_module_get(driver->owner)) {
> > dev_err(&r->dev, "can't get owner\n");
> > break;
> > }
> > @@ -2533,7 +2554,11 @@ EXPORT_SYMBOL(rproc_free);
> > */
> > void rproc_put(struct rproc *rproc)
> > {
> > - module_put(rproc->dev.parent->driver->owner);
> > + if (rproc->dev.parent->driver)
> > + module_put(rproc->dev.parent->driver->owner);
> > + else
> > + module_put(rproc->dev.parent->parent->driver->owner);
> > +
> > put_device(&rproc->dev);
> > }
> > EXPORT_SYMBOL(rproc_put);
> >
> > base-commit: 99f59b148871dadb9104366e3d25b120a97f897b