linux-kernel - Re: [PATCH v2 3/7] power: reset: qcom-pon: Migrate to devm_spmi_subdevice_alloc_and

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]

Message-ID: <z4xuesfnqc7wmenbgvu7k3odphxb7p6pyqdihx6cchojtrprpu@ihxk7tdrtrgz>
Date: Tue, 22 Jul 2025 18:05:39 +0200
From: Sebastian Reichel <sebastian.reichel@...labora.com>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: sboyd@...nel.org, jic23@...nel.org, dlechner@...libre.com, 
	nuno.sa@...log.com, andy@...nel.org, arnd@...db.de, gregkh@...uxfoundation.org, 
	srini@...nel.org, vkoul@...nel.org, kishon@...nel.org, 
	krzysztof.kozlowski@...aro.org, u.kleine-koenig@...libre.com, linux-arm-msm@...r.kernel.org, 
	linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org, linux-phy@...ts.infradead.org, 
	linux-pm@...r.kernel.org, kernel@...labora.com, wenst@...omium.org, 
	casey.connolly@...aro.org
Subject: Re: [PATCH v2 3/7] power: reset: qcom-pon: Migrate to
 devm_spmi_subdevice_alloc_and_add()

Hi,

On Tue, Jul 22, 2025 at 12:13:13PM +0200, AngeloGioacchino Del Regno wrote:
> Some Qualcomm PMICs integrates a Power On device supporting pwrkey
> and resin along with the Android reboot reason action identifier.
> 
> Instead of using the parent SPMI device (the main PMIC) as a kind
> of syscon in this driver, register a new SPMI sub-device for PON
> and initialize its own regmap with this sub-device's specific base
> address, retrieved from the devicetree.
> 
> This allows to stop manually adding the register base address to
> every R/W call in this driver, as this can be, and is now, handled
> by the regmap API instead.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
> ---
>  drivers/power/reset/qcom-pon.c | 37 ++++++++++++++++++++++++++--------
>  1 file changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
> index 7e108982a582..3558494aea36 100644
> --- a/drivers/power/reset/qcom-pon.c
> +++ b/drivers/power/reset/qcom-pon.c
> @@ -11,6 +11,7 @@
>  #include <linux/reboot.h>
>  #include <linux/reboot-mode.h>
>  #include <linux/regmap.h>
> +#include <linux/spmi.h>
>  
>  #define PON_SOFT_RB_SPARE		0x8f
>  
> @@ -22,7 +23,6 @@
>  struct qcom_pon {
>  	struct device *dev;
>  	struct regmap *regmap;
> -	u32 baseaddr;
>  	struct reboot_mode_driver reboot_mode;
>  	long reason_shift;
>  };
> @@ -35,7 +35,7 @@ static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
>  	int ret;
>  
>  	ret = regmap_update_bits(pon->regmap,
> -				 pon->baseaddr + PON_SOFT_RB_SPARE,
> +				 PON_SOFT_RB_SPARE,
>  				 GENMASK(7, pon->reason_shift),
>  				 magic << pon->reason_shift);
>  	if (ret < 0)
> @@ -46,27 +46,47 @@ static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
>  
>  static int qcom_pon_probe(struct platform_device *pdev)
>  {
> +	struct regmap_config qcom_pon_regmap_config = {
> +		.reg_bits = 16,
> +		.val_bits = 8,
> +		.max_register = 0x100,
> +		.fast_io = true,
> +	};
> +	struct device *dev = &pdev->dev;
> +	struct spmi_subdevice *sub_sdev;
> +	struct spmi_device *sparent;
>  	struct qcom_pon *pon;
>  	long reason_shift;
>  	int error;
>  
> +	if (!dev->parent)
> +		return -ENODEV;
> +
>  	pon = devm_kzalloc(&pdev->dev, sizeof(*pon), GFP_KERNEL);
>  	if (!pon)
>  		return -ENOMEM;
>  
>  	pon->dev = &pdev->dev;
>  
> -	pon->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> -	if (!pon->regmap) {
> -		dev_err(&pdev->dev, "failed to locate regmap\n");
> +	sparent = to_spmi_device(dev->parent);
> +	if (!sparent)
>  		return -ENODEV;

This cannot happen, since to_spmi_device() is using container_of(),
which is effectively just a cast.

> -	}
>  
> -	error = of_property_read_u32(pdev->dev.of_node, "reg",
> -				     &pon->baseaddr);
> +	sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
> +	if (IS_ERR(sub_sdev))
> +		return PTR_ERR(sub_sdev);
> +
> +	error = of_property_read_u32(dev->of_node, "reg",
> +				     &qcom_pon_regmap_config.reg_base);
>  	if (error)
>  		return error;
>  
> +	pon->regmap = devm_regmap_init_spmi_ext(&sub_sdev->sdev, &qcom_pon_regmap_config);
> +	if (!pon->regmap) {
> +		dev_err(&pdev->dev, "failed to locate regmap\n");
> +		return -ENODEV;
> +	}

The error message no longer fits and the function returns error
pointers instead of NULL, so:

if (IS_ERR(pon->regmap))
    return PTR_ERR(pon->regmap);

Greetings,

-- Sebastian

>  	reason_shift = (long)of_device_get_match_data(&pdev->dev);
>  
>  	if (reason_shift != NO_REASON_SHIFT) {
> @@ -106,3 +126,4 @@ module_platform_driver(qcom_pon_driver);
>  
>  MODULE_DESCRIPTION("Qualcomm Power On driver");
>  MODULE_LICENSE("GPL v2");
> +MODULE_IMPORT_NS("SPMI");
> -- 
> 2.50.1
> 

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)