Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <ae4b19a2-0315-a748-d048-6eb3ed2cc65a@kernel.org>
Date:   Tue, 28 Jun 2022 20:57:36 +0300
MIME-Version: 1.0
Subject: Re: [PATCH V2 8/9] interconnect: imx: configure NoC
 mode/prioriry/ext_control
Content-Language: en-US
To:     "Peng Fan (OSS)" <peng.fan@oss.nxp.com>, shawnguo@kernel.org,
        s.hauer@pengutronix.de, festevam@gmail.com, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, abel.vesa@nxp.com,
        abailon@baylibre.com, l.stach@pengutronix.de,
        laurent.pinchart@ideasonboard.com, marex@denx.de,
        paul.elder@ideasonboard.com, Markus.Niebel@ew.tq-group.com,
        aford173@gmail.com
Cc:     kernel@pengutronix.de, linux-pm@vger.kernel.org,
        devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org, linux-imx@nxp.com,
        abelvesa@kernel.org, Peng Fan <peng.fan@nxp.com>
References: <20220616073320.2203000-1-peng.fan@oss.nxp.com>
 <20220616073320.2203000-9-peng.fan@oss.nxp.com>
From:   Georgi Djakov <djakov@kernel.org>
In-Reply-To: <20220616073320.2203000-9-peng.fan@oss.nxp.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Peng,

On 16.06.22 10:33, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Introduce imx_icc_noc_setting structure to describe a master port setting
> Pass imx_icc_noc_setting as a parameter from specific driver
> Set priority level, mode, ext control in imx_icc_node_set
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>   drivers/interconnect/imx/imx.c    | 43 ++++++++++++++++++++++++++----
>   drivers/interconnect/imx/imx.h    | 44 ++++++++++++++++++++++++++++++-
>   drivers/interconnect/imx/imx8mm.c |  2 +-
>   drivers/interconnect/imx/imx8mn.c |  2 +-
>   drivers/interconnect/imx/imx8mq.c |  2 +-
>   5 files changed, 84 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
> index 78557fe6da2c..bd728caf2b85 100644

[..]

> @@ -37,8 +40,24 @@ static int imx_icc_node_set(struct icc_node *node)
>   {
>   	struct device *dev = node->provider->dev;
>   	struct imx_icc_node *node_data = node->data;
> +	void __iomem *base;
> +	u32 prio;
>   	u64 freq;
>   
> +	if (node_data->setting && !node_data->setting->ignore && node->peak_bw) {
> +		base = node_data->setting->reg + node_data->imx_provider->noc_base;
> +		if (node_data->setting->mode == IMX_NOC_MODE_FIXED) {
> +			prio = node_data->setting->prio_level;
> +			prio = PRIORITY_COMP_MARK | (prio << 8) | prio;
> +			writel(prio, base + IMX_NOC_PRIO_REG);
> +			writel(node_data->setting->mode, base + IMX_NOC_MODE_REG);
> +			writel(node_data->setting->ext_control, base + IMX_NOC_EXT_CTL_REG);
> +		} else {
> +			dev_info(dev, "mode: %d not supported\n", node_data->setting->mode);
> +			return -ENOTSUPP;

Nit: I believe that -EOPNOTSUPP is the preferred error code.

> +		}
> +	}
> +
>   	if (!node_data->qos_dev)
>   		return 0;
>   

[..]

> @@ -237,7 +263,8 @@ static int get_max_node_id(struct imx_icc_node_desc *nodes, int nodes_count)
>   }
>   
>   int imx_icc_register(struct platform_device *pdev,
> -		     struct imx_icc_node_desc *nodes, int nodes_count)
> +		     struct imx_icc_node_desc *nodes, int nodes_count,
> +		     struct imx_icc_noc_setting *settings)
>   {
>   	struct device *dev = &pdev->dev;
>   	struct icc_onecell_data *data;
> @@ -267,13 +294,19 @@ int imx_icc_register(struct platform_device *pdev,
>   	provider->dev->of_node = dev->parent->of_node;
>   	platform_set_drvdata(pdev, imx_provider);
>   
> +	if (settings) {
> +		imx_provider->noc_base = devm_of_iomap(dev, provider->dev->of_node, 0, NULL);
> +		if (!imx_provider->noc_base)

devm_of_iomap() returns ERR_PTR(). So we should check it with IS_ERR().

Thanks,
Georgi

> +			return PTR_ERR(imx_provider->noc_base);
> +	}
> +
>   	ret = icc_provider_add(provider);
>   	if (ret) {
>   		dev_err(dev, "error adding interconnect provider: %d\n", ret);
>   		return ret;
>   	}
>   
> -	ret = imx_icc_register_nodes(imx_provider, nodes, nodes_count);
> +	ret = imx_icc_register_nodes(imx_provider, nodes, nodes_count, settings);
>   	if (ret)
>   		goto provider_del;
>