linux-kernel - Re: [PATCH v8 11/17] power: pwrseq: add a driver for the PMU module on the QCom WCN chipsets

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: Windows password security audit tool. GUI, reports in PDF.

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

Date: Tue, 4 Jun 2024 12:52:54 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Marcel Holtmann <marcel@...tmann.org>,
	Luiz Augusto von Dentz <luiz.dentz@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Balakrishna Godavarthi <quic_bgodavar@...cinc.com>,
	Rocky Liao <quic_rjliao@...cinc.com>, Kalle Valo <kvalo@...nel.org>,
	Jeff Johnson <jjohnson@...nel.org>,
	Bjorn Andersson <andersson@...nel.org>,
	Konrad Dybcio <konrad.dybcio@...aro.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Srini Kandagatla <srinivas.kandagatla@...aro.org>,
	Elliot Berman <quic_eberman@...cinc.com>,
	Caleb Connolly <caleb.connolly@...aro.org>,
	Neil Armstrong <neil.armstrong@...aro.org>,
	Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
	Alex Elder <elder@...nel.org>, linux-arm-msm@...r.kernel.org,
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
	linux-bluetooth@...r.kernel.org, netdev@...r.kernel.org,
	linux-wireless@...r.kernel.org, ath11k@...ts.infradead.org,
	Jeff Johnson <quic_jjohnson@...cinc.com>,
	ath12k@...ts.infradead.org, linux-pm@...r.kernel.org,
	linux-pci@...r.kernel.org,
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
	kernel@...cinc.com, Amit Pundir <amit.pundir@...aro.org>
Subject: Re: [PATCH v8 11/17] power: pwrseq: add a driver for the PMU module
 on the QCom WCN chipsets

On Tue, May 28, 2024 at 09:03:19PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> 
> This adds the power sequencing driver for the PMU modules present on the
> Qualcomm WCN Bluetooth and Wifi chipsets. It uses the pwrseq subsystem
> and knows how to match the sequencer to the consumer device by verifying
> the relevant properties and DT layout.

> +config POWER_SEQUENCING_QCOM_WCN
> +	tristate "Qualcomm WCN family PMU driver"
> +	default m if ARCH_QCOM
> +	help
> +	  Say Y here to enable the power sequencing driver for Qualcomm
> +	  WCN Bluetooth/WLAN chipsets.
> +
> +	  Typically, a package from the Qualcomm WCN family contains the BT
> +	  and WLAN modules whose power is controlled by the PMU module. As the
> +	  former two share the power-up sequence which is executed by the PMU,
> +	  this driver is needed for correct power control.

"needed for correct power control" suggests that this fixes an
existing problem, and I assume everybody with this kind of device
wants this, and they will see some benefit from enabling it.  But it's
not clear what that user-visible benefit is.  Could be useful both
here and in commit log.

> +struct pwrseq_qcom_wcn_pdata {
> +	const char *const *vregs;
> +	size_t num_vregs;
> +	unsigned int pwup_delay_msec;
> +	unsigned int gpio_enable_delay;

Seems like it'd be nice to have a hint about the units of
gpio_enable_delay (apparently ms) and last_gpio_enable (apparently
jiffies)?  Maybe even use the same units for both, but I'm sure you
have a reason for this.

> +static int pwrseq_qcom_wcn_match(struct pwrseq_device *pwrseq,
> +				 struct device *dev)
> +{
> +	struct pwrseq_qcom_wcn_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> +	struct device_node *dev_node = dev->of_node;
> +
> +	/*
> +	 * The PMU supplies power to the Bluetooth and WLAN modules. both

s/both/Both/