* [PATCH] interconnect: qcom: osm-l3: Use driver-specific naming
@ 2021-07-25 3:14 Bjorn Andersson
2021-07-26 16:31 ` Steev Klimaszewski
2021-07-26 23:02 ` Stephen Boyd
0 siblings, 2 replies; 3+ messages in thread
From: Bjorn Andersson @ 2021-07-25 3:14 UTC (permalink / raw)
To: Georgi Djakov; +Cc: linux-arm-msm, linux-pm, linux-kernel
In situations were the developer screws up by e.g. not giving the OSM
nodes unique identifiers the interconnect framework might mix up nodes
between the OSM L3 provider and e.g. the RPMh provider.
The resulting callstack containts "qcom_icc_set", which is not unique to
the OSM L3 provider driver. Once the faulting qcom_icc_set() is
identified it's further confusing that "qcom_icc_node" is different
between the different drivers.
To avoid this confusion, rename the node struct and the setter in the
OSM L3 driver to include "osm_l3" in their names.
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
This was written after the sc8180x patch and as such applies ontop of:
https://lore.kernel.org/linux-arm-msm/20210725025834.3941777-2-bjorn.andersson@linaro.org/
drivers/interconnect/qcom/osm-l3.c | 46 +++++++++++++++---------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c
index 13e41b932567..c7af143980de 100644
--- a/drivers/interconnect/qcom/osm-l3.c
+++ b/drivers/interconnect/qcom/osm-l3.c
@@ -38,7 +38,7 @@
#define OSM_L3_MAX_LINKS 1
-#define to_qcom_provider(_provider) \
+#define to_osm_l3_provider(_provider) \
container_of(_provider, struct qcom_osm_l3_icc_provider, provider)
struct qcom_osm_l3_icc_provider {
@@ -50,14 +50,14 @@ struct qcom_osm_l3_icc_provider {
};
/**
- * struct qcom_icc_node - Qualcomm specific interconnect nodes
+ * struct qcom_osm_l3_node - Qualcomm specific interconnect nodes
* @name: the node name used in debugfs
* @links: an array of nodes where we can go next while traversing
* @id: a unique node identifier
* @num_links: the total number of @links
* @buswidth: width of the interconnect between a node and the bus
*/
-struct qcom_icc_node {
+struct qcom_osm_l3_node {
const char *name;
u16 links[OSM_L3_MAX_LINKS];
u16 id;
@@ -65,8 +65,8 @@ struct qcom_icc_node {
u16 buswidth;
};
-struct qcom_icc_desc {
- const struct qcom_icc_node **nodes;
+struct qcom_osm_l3_desc {
+ const struct qcom_osm_l3_node **nodes;
size_t num_nodes;
unsigned int lut_row_size;
unsigned int reg_freq_lut;
@@ -74,7 +74,7 @@ struct qcom_icc_desc {
};
#define DEFINE_QNODE(_name, _id, _buswidth, ...) \
- static const struct qcom_icc_node _name = { \
+ static const struct qcom_osm_l3_node _name = { \
.name = #_name, \
.id = _id, \
.buswidth = _buswidth, \
@@ -85,12 +85,12 @@ struct qcom_icc_desc {
DEFINE_QNODE(sdm845_osm_apps_l3, SDM845_MASTER_OSM_L3_APPS, 16, SDM845_SLAVE_OSM_L3);
DEFINE_QNODE(sdm845_osm_l3, SDM845_SLAVE_OSM_L3, 16);
-static const struct qcom_icc_node *sdm845_osm_l3_nodes[] = {
+static const struct qcom_osm_l3_node *sdm845_osm_l3_nodes[] = {
[MASTER_OSM_L3_APPS] = &sdm845_osm_apps_l3,
[SLAVE_OSM_L3] = &sdm845_osm_l3,
};
-static const struct qcom_icc_desc sdm845_icc_osm_l3 = {
+static const struct qcom_osm_l3_desc sdm845_icc_osm_l3 = {
.nodes = sdm845_osm_l3_nodes,
.num_nodes = ARRAY_SIZE(sdm845_osm_l3_nodes),
.lut_row_size = OSM_LUT_ROW_SIZE,
@@ -101,12 +101,12 @@ static const struct qcom_icc_desc sdm845_icc_osm_l3 = {
DEFINE_QNODE(sc7180_osm_apps_l3, SC7180_MASTER_OSM_L3_APPS, 16, SC7180_SLAVE_OSM_L3);
DEFINE_QNODE(sc7180_osm_l3, SC7180_SLAVE_OSM_L3, 16);
-static const struct qcom_icc_node *sc7180_osm_l3_nodes[] = {
+static const struct qcom_osm_l3_node *sc7180_osm_l3_nodes[] = {
[MASTER_OSM_L3_APPS] = &sc7180_osm_apps_l3,
[SLAVE_OSM_L3] = &sc7180_osm_l3,
};
-static const struct qcom_icc_desc sc7180_icc_osm_l3 = {
+static const struct qcom_osm_l3_desc sc7180_icc_osm_l3 = {
.nodes = sc7180_osm_l3_nodes,
.num_nodes = ARRAY_SIZE(sc7180_osm_l3_nodes),
.lut_row_size = OSM_LUT_ROW_SIZE,
@@ -117,12 +117,12 @@ static const struct qcom_icc_desc sc7180_icc_osm_l3 = {
DEFINE_QNODE(sc8180x_osm_apps_l3, SC8180X_MASTER_OSM_L3_APPS, 32, SC8180X_SLAVE_OSM_L3);
DEFINE_QNODE(sc8180x_osm_l3, SC8180X_SLAVE_OSM_L3, 32);
-static const struct qcom_icc_node *sc8180x_osm_l3_nodes[] = {
+static const struct qcom_osm_l3_node *sc8180x_osm_l3_nodes[] = {
[MASTER_OSM_L3_APPS] = &sc8180x_osm_apps_l3,
[SLAVE_OSM_L3] = &sc8180x_osm_l3,
};
-static const struct qcom_icc_desc sc8180x_icc_osm_l3 = {
+static const struct qcom_osm_l3_desc sc8180x_icc_osm_l3 = {
.nodes = sc8180x_osm_l3_nodes,
.num_nodes = ARRAY_SIZE(sc8180x_osm_l3_nodes),
.lut_row_size = OSM_LUT_ROW_SIZE,
@@ -133,12 +133,12 @@ static const struct qcom_icc_desc sc8180x_icc_osm_l3 = {
DEFINE_QNODE(sm8150_osm_apps_l3, SM8150_MASTER_OSM_L3_APPS, 32, SM8150_SLAVE_OSM_L3);
DEFINE_QNODE(sm8150_osm_l3, SM8150_SLAVE_OSM_L3, 32);
-static const struct qcom_icc_node *sm8150_osm_l3_nodes[] = {
+static const struct qcom_osm_l3_node *sm8150_osm_l3_nodes[] = {
[MASTER_OSM_L3_APPS] = &sm8150_osm_apps_l3,
[SLAVE_OSM_L3] = &sm8150_osm_l3,
};
-static const struct qcom_icc_desc sm8150_icc_osm_l3 = {
+static const struct qcom_osm_l3_desc sm8150_icc_osm_l3 = {
.nodes = sm8150_osm_l3_nodes,
.num_nodes = ARRAY_SIZE(sm8150_osm_l3_nodes),
.lut_row_size = OSM_LUT_ROW_SIZE,
@@ -149,12 +149,12 @@ static const struct qcom_icc_desc sm8150_icc_osm_l3 = {
DEFINE_QNODE(sm8250_epss_apps_l3, SM8250_MASTER_EPSS_L3_APPS, 32, SM8250_SLAVE_EPSS_L3);
DEFINE_QNODE(sm8250_epss_l3, SM8250_SLAVE_EPSS_L3, 32);
-static const struct qcom_icc_node *sm8250_epss_l3_nodes[] = {
+static const struct qcom_osm_l3_node *sm8250_epss_l3_nodes[] = {
[MASTER_EPSS_L3_APPS] = &sm8250_epss_apps_l3,
[SLAVE_EPSS_L3_SHARED] = &sm8250_epss_l3,
};
-static const struct qcom_icc_desc sm8250_icc_epss_l3 = {
+static const struct qcom_osm_l3_desc sm8250_icc_epss_l3 = {
.nodes = sm8250_epss_l3_nodes,
.num_nodes = ARRAY_SIZE(sm8250_epss_l3_nodes),
.lut_row_size = EPSS_LUT_ROW_SIZE,
@@ -162,11 +162,11 @@ static const struct qcom_icc_desc sm8250_icc_epss_l3 = {
.reg_perf_state = EPSS_REG_PERF_STATE,
};
-static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
+static int qcom_osm_l3_set(struct icc_node *src, struct icc_node *dst)
{
struct qcom_osm_l3_icc_provider *qp;
struct icc_provider *provider;
- const struct qcom_icc_node *qn;
+ const struct qcom_osm_l3_node *qn;
struct icc_node *n;
unsigned int index;
u32 agg_peak = 0;
@@ -175,7 +175,7 @@ static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
qn = src->data;
provider = src->provider;
- qp = to_qcom_provider(provider);
+ qp = to_osm_l3_provider(provider);
list_for_each_entry(n, &provider->nodes, node_list)
provider->aggregate(n, 0, n->avg_bw, n->peak_bw,
@@ -208,10 +208,10 @@ static int qcom_osm_l3_probe(struct platform_device *pdev)
u32 info, src, lval, i, prev_freq = 0, freq;
static unsigned long hw_rate, xo_rate;
struct qcom_osm_l3_icc_provider *qp;
- const struct qcom_icc_desc *desc;
+ const struct qcom_osm_l3_desc *desc;
struct icc_onecell_data *data;
struct icc_provider *provider;
- const struct qcom_icc_node **qnodes;
+ const struct qcom_osm_l3_node **qnodes;
struct icc_node *node;
size_t num_nodes;
struct clk *clk;
@@ -281,7 +281,7 @@ static int qcom_osm_l3_probe(struct platform_device *pdev)
provider = &qp->provider;
provider->dev = &pdev->dev;
- provider->set = qcom_icc_set;
+ provider->set = qcom_osm_l3_set;
provider->aggregate = icc_std_aggregate;
provider->xlate = of_icc_xlate_onecell;
INIT_LIST_HEAD(&provider->nodes);
@@ -303,7 +303,7 @@ static int qcom_osm_l3_probe(struct platform_device *pdev)
}
node->name = qnodes[i]->name;
- /* Cast away const and add it back in qcom_icc_set() */
+ /* Cast away const and add it back in qcom_osm_l3_set() */
node->data = (void *)qnodes[i];
icc_node_add(node, provider);
--
2.29.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] interconnect: qcom: osm-l3: Use driver-specific naming
2021-07-25 3:14 [PATCH] interconnect: qcom: osm-l3: Use driver-specific naming Bjorn Andersson
@ 2021-07-26 16:31 ` Steev Klimaszewski
2021-07-26 23:02 ` Stephen Boyd
1 sibling, 0 replies; 3+ messages in thread
From: Steev Klimaszewski @ 2021-07-26 16:31 UTC (permalink / raw)
To: Bjorn Andersson, Georgi Djakov; +Cc: linux-arm-msm, linux-pm, linux-kernel
On 7/24/21 10:14 PM, Bjorn Andersson wrote:
> In situations were the developer screws up by e.g. not giving the OSM
> nodes unique identifiers the interconnect framework might mix up nodes
> between the OSM L3 provider and e.g. the RPMh provider.
>
> The resulting callstack containts "qcom_icc_set", which is not unique to
> the OSM L3 provider driver. Once the faulting qcom_icc_set() is
> identified it's further confusing that "qcom_icc_node" is different
> between the different drivers.
>
> To avoid this confusion, rename the node struct and the setter in the
> OSM L3 driver to include "osm_l3" in their names.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> This was written after the sc8180x patch and as such applies ontop of:
> https://lore.kernel.org/linux-arm-msm/20210725025834.3941777-2-bjorn.andersson@linaro.org/
>
> drivers/interconnect/qcom/osm-l3.c | 46 +++++++++++++++---------------
> 1 file changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c
> index 13e41b932567..c7af143980de 100644
> --- a/drivers/interconnect/qcom/osm-l3.c
> +++ b/drivers/interconnect/qcom/osm-l3.c
> @@ -38,7 +38,7 @@
>
> #define OSM_L3_MAX_LINKS 1
>
> -#define to_qcom_provider(_provider) \
> +#define to_osm_l3_provider(_provider) \
> container_of(_provider, struct qcom_osm_l3_icc_provider, provider)
>
> struct qcom_osm_l3_icc_provider {
> @@ -50,14 +50,14 @@ struct qcom_osm_l3_icc_provider {
> };
>
> /**
> - * struct qcom_icc_node - Qualcomm specific interconnect nodes
> + * struct qcom_osm_l3_node - Qualcomm specific interconnect nodes
> * @name: the node name used in debugfs
> * @links: an array of nodes where we can go next while traversing
> * @id: a unique node identifier
> * @num_links: the total number of @links
> * @buswidth: width of the interconnect between a node and the bus
> */
> -struct qcom_icc_node {
> +struct qcom_osm_l3_node {
> const char *name;
> u16 links[OSM_L3_MAX_LINKS];
> u16 id;
> @@ -65,8 +65,8 @@ struct qcom_icc_node {
> u16 buswidth;
> };
>
> -struct qcom_icc_desc {
> - const struct qcom_icc_node **nodes;
> +struct qcom_osm_l3_desc {
> + const struct qcom_osm_l3_node **nodes;
> size_t num_nodes;
> unsigned int lut_row_size;
> unsigned int reg_freq_lut;
> @@ -74,7 +74,7 @@ struct qcom_icc_desc {
> };
>
> #define DEFINE_QNODE(_name, _id, _buswidth, ...) \
> - static const struct qcom_icc_node _name = { \
> + static const struct qcom_osm_l3_node _name = { \
> .name = #_name, \
> .id = _id, \
> .buswidth = _buswidth, \
> @@ -85,12 +85,12 @@ struct qcom_icc_desc {
> DEFINE_QNODE(sdm845_osm_apps_l3, SDM845_MASTER_OSM_L3_APPS, 16, SDM845_SLAVE_OSM_L3);
> DEFINE_QNODE(sdm845_osm_l3, SDM845_SLAVE_OSM_L3, 16);
>
> -static const struct qcom_icc_node *sdm845_osm_l3_nodes[] = {
> +static const struct qcom_osm_l3_node *sdm845_osm_l3_nodes[] = {
> [MASTER_OSM_L3_APPS] = &sdm845_osm_apps_l3,
> [SLAVE_OSM_L3] = &sdm845_osm_l3,
> };
>
> -static const struct qcom_icc_desc sdm845_icc_osm_l3 = {
> +static const struct qcom_osm_l3_desc sdm845_icc_osm_l3 = {
> .nodes = sdm845_osm_l3_nodes,
> .num_nodes = ARRAY_SIZE(sdm845_osm_l3_nodes),
> .lut_row_size = OSM_LUT_ROW_SIZE,
> @@ -101,12 +101,12 @@ static const struct qcom_icc_desc sdm845_icc_osm_l3 = {
> DEFINE_QNODE(sc7180_osm_apps_l3, SC7180_MASTER_OSM_L3_APPS, 16, SC7180_SLAVE_OSM_L3);
> DEFINE_QNODE(sc7180_osm_l3, SC7180_SLAVE_OSM_L3, 16);
>
> -static const struct qcom_icc_node *sc7180_osm_l3_nodes[] = {
> +static const struct qcom_osm_l3_node *sc7180_osm_l3_nodes[] = {
> [MASTER_OSM_L3_APPS] = &sc7180_osm_apps_l3,
> [SLAVE_OSM_L3] = &sc7180_osm_l3,
> };
>
> -static const struct qcom_icc_desc sc7180_icc_osm_l3 = {
> +static const struct qcom_osm_l3_desc sc7180_icc_osm_l3 = {
> .nodes = sc7180_osm_l3_nodes,
> .num_nodes = ARRAY_SIZE(sc7180_osm_l3_nodes),
> .lut_row_size = OSM_LUT_ROW_SIZE,
> @@ -117,12 +117,12 @@ static const struct qcom_icc_desc sc7180_icc_osm_l3 = {
> DEFINE_QNODE(sc8180x_osm_apps_l3, SC8180X_MASTER_OSM_L3_APPS, 32, SC8180X_SLAVE_OSM_L3);
> DEFINE_QNODE(sc8180x_osm_l3, SC8180X_SLAVE_OSM_L3, 32);
>
> -static const struct qcom_icc_node *sc8180x_osm_l3_nodes[] = {
> +static const struct qcom_osm_l3_node *sc8180x_osm_l3_nodes[] = {
> [MASTER_OSM_L3_APPS] = &sc8180x_osm_apps_l3,
> [SLAVE_OSM_L3] = &sc8180x_osm_l3,
> };
>
> -static const struct qcom_icc_desc sc8180x_icc_osm_l3 = {
> +static const struct qcom_osm_l3_desc sc8180x_icc_osm_l3 = {
> .nodes = sc8180x_osm_l3_nodes,
> .num_nodes = ARRAY_SIZE(sc8180x_osm_l3_nodes),
> .lut_row_size = OSM_LUT_ROW_SIZE,
> @@ -133,12 +133,12 @@ static const struct qcom_icc_desc sc8180x_icc_osm_l3 = {
> DEFINE_QNODE(sm8150_osm_apps_l3, SM8150_MASTER_OSM_L3_APPS, 32, SM8150_SLAVE_OSM_L3);
> DEFINE_QNODE(sm8150_osm_l3, SM8150_SLAVE_OSM_L3, 32);
>
> -static const struct qcom_icc_node *sm8150_osm_l3_nodes[] = {
> +static const struct qcom_osm_l3_node *sm8150_osm_l3_nodes[] = {
> [MASTER_OSM_L3_APPS] = &sm8150_osm_apps_l3,
> [SLAVE_OSM_L3] = &sm8150_osm_l3,
> };
>
> -static const struct qcom_icc_desc sm8150_icc_osm_l3 = {
> +static const struct qcom_osm_l3_desc sm8150_icc_osm_l3 = {
> .nodes = sm8150_osm_l3_nodes,
> .num_nodes = ARRAY_SIZE(sm8150_osm_l3_nodes),
> .lut_row_size = OSM_LUT_ROW_SIZE,
> @@ -149,12 +149,12 @@ static const struct qcom_icc_desc sm8150_icc_osm_l3 = {
> DEFINE_QNODE(sm8250_epss_apps_l3, SM8250_MASTER_EPSS_L3_APPS, 32, SM8250_SLAVE_EPSS_L3);
> DEFINE_QNODE(sm8250_epss_l3, SM8250_SLAVE_EPSS_L3, 32);
>
> -static const struct qcom_icc_node *sm8250_epss_l3_nodes[] = {
> +static const struct qcom_osm_l3_node *sm8250_epss_l3_nodes[] = {
> [MASTER_EPSS_L3_APPS] = &sm8250_epss_apps_l3,
> [SLAVE_EPSS_L3_SHARED] = &sm8250_epss_l3,
> };
>
> -static const struct qcom_icc_desc sm8250_icc_epss_l3 = {
> +static const struct qcom_osm_l3_desc sm8250_icc_epss_l3 = {
> .nodes = sm8250_epss_l3_nodes,
> .num_nodes = ARRAY_SIZE(sm8250_epss_l3_nodes),
> .lut_row_size = EPSS_LUT_ROW_SIZE,
> @@ -162,11 +162,11 @@ static const struct qcom_icc_desc sm8250_icc_epss_l3 = {
> .reg_perf_state = EPSS_REG_PERF_STATE,
> };
>
> -static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
> +static int qcom_osm_l3_set(struct icc_node *src, struct icc_node *dst)
> {
> struct qcom_osm_l3_icc_provider *qp;
> struct icc_provider *provider;
> - const struct qcom_icc_node *qn;
> + const struct qcom_osm_l3_node *qn;
> struct icc_node *n;
> unsigned int index;
> u32 agg_peak = 0;
> @@ -175,7 +175,7 @@ static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
>
> qn = src->data;
> provider = src->provider;
> - qp = to_qcom_provider(provider);
> + qp = to_osm_l3_provider(provider);
>
> list_for_each_entry(n, &provider->nodes, node_list)
> provider->aggregate(n, 0, n->avg_bw, n->peak_bw,
> @@ -208,10 +208,10 @@ static int qcom_osm_l3_probe(struct platform_device *pdev)
> u32 info, src, lval, i, prev_freq = 0, freq;
> static unsigned long hw_rate, xo_rate;
> struct qcom_osm_l3_icc_provider *qp;
> - const struct qcom_icc_desc *desc;
> + const struct qcom_osm_l3_desc *desc;
> struct icc_onecell_data *data;
> struct icc_provider *provider;
> - const struct qcom_icc_node **qnodes;
> + const struct qcom_osm_l3_node **qnodes;
> struct icc_node *node;
> size_t num_nodes;
> struct clk *clk;
> @@ -281,7 +281,7 @@ static int qcom_osm_l3_probe(struct platform_device *pdev)
>
> provider = &qp->provider;
> provider->dev = &pdev->dev;
> - provider->set = qcom_icc_set;
> + provider->set = qcom_osm_l3_set;
> provider->aggregate = icc_std_aggregate;
> provider->xlate = of_icc_xlate_onecell;
> INIT_LIST_HEAD(&provider->nodes);
> @@ -303,7 +303,7 @@ static int qcom_osm_l3_probe(struct platform_device *pdev)
> }
>
> node->name = qnodes[i]->name;
> - /* Cast away const and add it back in qcom_icc_set() */
> + /* Cast away const and add it back in qcom_osm_l3_set() */
> node->data = (void *)qnodes[i];
> icc_node_add(node, provider);
>
Hi Bjorn,
Tested on both C630 and Flex 5G
Tested-by: Steev Klimaszewski <steev@kali.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] interconnect: qcom: osm-l3: Use driver-specific naming
2021-07-25 3:14 [PATCH] interconnect: qcom: osm-l3: Use driver-specific naming Bjorn Andersson
2021-07-26 16:31 ` Steev Klimaszewski
@ 2021-07-26 23:02 ` Stephen Boyd
1 sibling, 0 replies; 3+ messages in thread
From: Stephen Boyd @ 2021-07-26 23:02 UTC (permalink / raw)
To: Bjorn Andersson, Georgi Djakov; +Cc: linux-arm-msm, linux-pm, linux-kernel
Quoting Bjorn Andersson (2021-07-24 20:14:14)
> In situations were the developer screws up by e.g. not giving the OSM
> nodes unique identifiers the interconnect framework might mix up nodes
> between the OSM L3 provider and e.g. the RPMh provider.
>
> The resulting callstack containts "qcom_icc_set", which is not unique to
> the OSM L3 provider driver. Once the faulting qcom_icc_set() is
> identified it's further confusing that "qcom_icc_node" is different
> between the different drivers.
>
> To avoid this confusion, rename the node struct and the setter in the
> OSM L3 driver to include "osm_l3" in their names.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
Thanks
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-07-26 23:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-25 3:14 [PATCH] interconnect: qcom: osm-l3: Use driver-specific naming Bjorn Andersson
2021-07-26 16:31 ` Steev Klimaszewski
2021-07-26 23:02 ` Stephen Boyd
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).