linux-kernel - Re: [PATCH v1 4/4] serial: qcom-geni: Enable Serial on SA8255p Qualcomm platforms

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aSb7XhbTTtF3Wd-3@black.igk.intel.com>
Date: Wed, 26 Nov 2025 14:06:38 +0100
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Praveen Talari <praveen.talari@....qualcomm.com>
Cc: Bjorn Andersson <andersson@...nel.org>,
	Konrad Dybcio <konradybcio@...nel.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jirislaby@...nel.org>,
	Dmitry Baryshkov <lumag@...nel.org>, linux-arm-msm@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-gpio@...r.kernel.org, linux-serial@...r.kernel.org,
	alexey.klimov@...aro.org, krzk@...nel.org,
	bryan.odonoghue@...aro.org, jorge.ramirez@....qualcomm.com,
	dmitry.baryshkov@....qualcomm.com,
	Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
	psodagud@...cinc.com, djaggi@...cinc.com, quic_msavaliy@...cinc.com,
	quic_vtanuku@...cinc.com, quic_arandive@...cinc.com,
	quic_shazhuss@...cinc.com, quic_cchiluve@...cinc.com
Subject: Re: [PATCH v1 4/4] serial: qcom-geni: Enable Serial on SA8255p
 Qualcomm platforms

On Mon, Nov 10, 2025 at 03:40:43PM +0530, Praveen Talari wrote:
> The Qualcomm automotive SA8255p SoC relies on firmware to configure
> platform resources, including clocks, interconnects and TLMM.
> The driver requests resources operations over SCMI using power
> and performance protocols.
> 
> The SCMI power protocol enables or disables resources like clocks,
> interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
> such as resume/suspend, to control power states(on/off).
> 
> The SCMI performance protocol manages UART baud rates, with each baud
> rate represented by a performance level. The driver uses the
> dev_pm_opp_set_level() API to request the desired baud rate by
> specifying the performance level.

...

> +static int geni_serial_pwr_init(struct uart_port *uport)
> +{
> +	struct qcom_geni_serial_port *port = to_dev_port(uport);
> +	int ret;
> +
> +	ret = dev_pm_domain_attach_list(port->se.dev,
> +					&port->dev_data->pd_data, &port->pd_list);
> +	if (ret <= 0)
> +		return -EINVAL;
> +
> +	return 0;

Why shadowing an error code?

This should be

	ret = dev_pm_domain_attach_list(port->se.dev,
					&port->dev_data->pd_data, &port->pd_list);
	if (ret < 0)
		return ret; // assuming it returns a Linux err code.
	/* Some comment, perhaps? */
	if (ret == 0)
		return -EINVAL;

	return 0;

> +}

...

>  		port->rx_buf = devm_kzalloc(uport->dev,
>  					    DMA_RX_BUF_SIZE, GFP_KERNEL);
> -		if (!port->rx_buf)
> -			return -ENOMEM;
> +		if (!port->rx_buf) {
> +			ret = -ENOMEM;
> +			goto error;

This is wrong. After devm_*() calls should not be goto:s. It should be very
exceptional cases otherwise.

> +		}

...

>  	port->name = devm_kasprintf(uport->dev, GFP_KERNEL,
>  			"qcom_geni_serial_%s%d",
>  			uart_console(uport) ? "console" : "uart", uport->line);
> -	if (!port->name)
> -		return -ENOMEM;
> +	if (!port->name) {
> +		ret = -ENOMEM;
> +		goto error;
> +	}

Ditto.

...

> +	if (irq < 0) {
> +		ret = irq;
> +		goto error;

Ditto. And so on...

> +	}

...

Hint: use devm_add_action_or_reset() above and drop most of the changes in this
patch.

-- 
With Best Regards,
Andy Shevchenko