| Message ID | 20220621192034.3332546-21-sudeep.holla@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arch_topology: Updates to add socket support and fix cluster ids | expand |
Hi, I just noticed this in a quick test. On Tuesday 21 Jun 2022 at 20:20:34 (+0100), Sudeep Holla wrote: > We don't support the topology for clusters of CPU clusters while the > DT and ACPI bindings theoritcally support the same. Just warn about the > same so that it is clear to the users of arch_topology that the nested > clusters are not yet supported. > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > --- > drivers/base/arch_topology.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > index ed1cb64a95aa..1c5fa7bbbd00 100644 > --- a/drivers/base/arch_topology.c > +++ b/drivers/base/arch_topology.c > @@ -567,6 +567,8 @@ static int __init parse_cluster(struct device_node *cluster, int package_id, > if (c) { > leaf = false; > ret = parse_cluster(c, package_id, i, depth + 1); > + if (depth > 1) > + pr_warn("Topology for clusters of clusters not yet supported\n"); I think the check should be for (depth > 0) or (depth >= 1). We end up having depth = 2 when we have cluster 0 { //depth is 0 cluster 0 { //depth is 1 cluster0 { //depth is 2 ... I suppose we should warn about nested clusters from depth 1, right? Thanks, Ionela. > of_node_put(c); > if (ret != 0) > return ret; > -- > 2.36.1 >
On Wed, Jun 22, 2022 at 04:06:29PM +0100, Ionela Voinescu wrote: > Hi, > > I just noticed this in a quick test. > > On Tuesday 21 Jun 2022 at 20:20:34 (+0100), Sudeep Holla wrote: > > We don't support the topology for clusters of CPU clusters while the > > DT and ACPI bindings theoritcally support the same. Just warn about the > > same so that it is clear to the users of arch_topology that the nested > > clusters are not yet supported. > > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > > --- > > drivers/base/arch_topology.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > > index ed1cb64a95aa..1c5fa7bbbd00 100644 > > --- a/drivers/base/arch_topology.c > > +++ b/drivers/base/arch_topology.c > > @@ -567,6 +567,8 @@ static int __init parse_cluster(struct device_node *cluster, int package_id, > > if (c) { > > leaf = false; > > ret = parse_cluster(c, package_id, i, depth + 1); > > + if (depth > 1) > > + pr_warn("Topology for clusters of clusters not yet supported\n"); > > I think the check should be for (depth > 0) or (depth >= 1). > > We end up having depth = 2 when we have > > cluster 0 { > //depth is 0 > cluster 0 { > //depth is 1 > cluster0 { > //depth is 2 > ... > > I suppose we should warn about nested clusters from depth 1, right? > You are absolutely correct. For some reason when I wrote this patch I read the line above as depth++ instead of depth + 1. I was searching for that now reading your reply just to realise that I misread it. Thanks for catching this.
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c index ed1cb64a95aa..1c5fa7bbbd00 100644 --- a/drivers/base/arch_topology.c +++ b/drivers/base/arch_topology.c @@ -567,6 +567,8 @@ static int __init parse_cluster(struct device_node *cluster, int package_id, if (c) { leaf = false; ret = parse_cluster(c, package_id, i, depth + 1); + if (depth > 1) + pr_warn("Topology for clusters of clusters not yet supported\n"); of_node_put(c); if (ret != 0) return ret;
We don't support the topology for clusters of CPU clusters while the DT and ACPI bindings theoritcally support the same. Just warn about the same so that it is clear to the users of arch_topology that the nested clusters are not yet supported. Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/base/arch_topology.c | 2 ++ 1 file changed, 2 insertions(+)