linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steev Klimaszewski <steev@kali.org>
To: Bjorn Andersson <bjorn.andersson@linaro.org>,
	Georgi Djakov <djakov@kernel.org>
Cc: linux-arm-msm@vger.kernel.org, linux-pm@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] interconnect: qcom: osm-l3: Use driver-specific naming
Date: Mon, 26 Jul 2021 11:31:14 -0500	[thread overview]
Message-ID: <5148d281-d5d9-7731-f30a-090af8cd2ea2@kali.org> (raw)
In-Reply-To: <20210725031414.3961227-1-bjorn.andersson@linaro.org>


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>

  reply	other threads:[~2021-07-26 16:39 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-25  3:14 [PATCH] interconnect: qcom: osm-l3: Use driver-specific naming Bjorn Andersson
2021-07-26 16:31 ` Steev Klimaszewski [this message]
2021-07-26 23:02 ` Stephen Boyd

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5148d281-d5d9-7731-f30a-090af8cd2ea2@kali.org \
    --to=steev@kali.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=djakov@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).