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] [day] [month] [year] [list]
Message-ID: <j3hsfjkdfta5cheavnem5heaszuytm2zfhzm3y3udat3pjxnc5@vzymqkcdau7k>
Date: Sat, 6 Sep 2025 22:57:15 +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 v3 3/7] power: reset: qcom-pon: Migrate to
 devm_spmi_subdevice_alloc_and_add()

Hi,

On Wed, Jul 30, 2025 at 01:26:41PM +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.
> 
> Link: https://lore.kernel.org/r/20250722101317.76729-4-angelogioacchino.delregno@collabora.com
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
> ---

The patch looks good to me, but we need to check how to get it
merged. Probably the best way is an immutable branch. As it looks
like the series needs another respin:

Reviewed-by: Sebastian Reichel <sebastian.reichel@...labora.com>

Greetings,

-- Sebastian

>  drivers/power/reset/qcom-pon.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
> index 7e108982a582..0e075a2e5e48 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,42 @@ 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");
> -		return -ENODEV;
> -	}
> +	sparent = to_spmi_device(dev->parent);
> +	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(pdev->dev.of_node, "reg",
> -				     &pon->baseaddr);
> +	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 (IS_ERR(pon->regmap))
> +		return PTR_ERR(pon->regmap);
> +
>  	reason_shift = (long)of_device_get_match_data(&pdev->dev);
>  
>  	if (reason_shift != NO_REASON_SHIFT) {
> @@ -106,3 +121,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)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ