Re: [PATCH v3 1/4] interconnect: qcom: icc-rpmh: Add QoS configuration support

[Krzysztof Kozlowski]

On 06/03/2024 08:30, Odelu Kukatla wrote:
> It adds QoS support for QNOC device and includes support for
> configuring priority, priority forward disable, urgency forwarding.
> This helps in priortizing the traffic originating from different
> interconnect masters at NoC(Network On Chip).
> 
> Signed-off-by: Odelu Kukatla <quic_okukatla@xxxxxxxxxxx>
> ---
>  drivers/interconnect/qcom/icc-rpmh.c | 105 +++++++++++++++++++++++++++
>  drivers/interconnect/qcom/icc-rpmh.h |  32 ++++++++
>  2 files changed, 137 insertions(+)

>  
> +	if (desc->config) {
> +		struct resource *res;
> +		void __iomem *base;
> +
> +		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +		if (!res) {
> +			dev_warn(dev, "Skipping QoS, failed to get resource\n");

Why is this a warning? If the MMIO is not required, your driver should
not spawn warnings.


> +			goto skip_qos_config;
> +		}
> +
> +		base = devm_ioremap(dev, res->start, resource_size(res));

Combine these two (there's a helper for that).

> +		if (IS_ERR(base)) {
> +			dev_warn(dev, "Skipping QoS, ioremap failed: %ld\n", PTR_ERR(base));
> +			goto skip_qos_config;
> +		};
> +
> +		qp->regmap = devm_regmap_init_mmio(dev, base, desc->config);
> +

Drop blank line.

> +		if (IS_ERR(qp->regmap)) {
> +			dev_warn(dev, "Skipping QoS, regmap failed; %ld\n", PTR_ERR(qp->regmap));
> +			goto skip_qos_config;
> +		}
> +
> +		qp->num_clks = devm_clk_bulk_get_all(qp->dev, &qp->clks);
> +		if (qp->num_clks < 0) {
> +			dev_warn(dev, "Skipping QoS, failed to get clk: %d\n", qp->num_clks);

dev_info(), because current binding sais clocks are not required and you
are not allowed to affect ABI. While ABI here does not look broken,
spawning new warnings to proper users is not welcomed.

Best regards,
Krzysztof