On Tue, May 28, 2024 at 09:03:19PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> 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/