[PATCH v2 3/7] power: reset: qcom-pon: Migrate to devm_spmi_subdevice_alloc_and_add()

AngeloGioacchino Del Regno posted 7 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH v2 3/7] power: reset: qcom-pon: Migrate to devm_spmi_subdevice_alloc_and_add()
Posted by AngeloGioacchino Del Regno 2 months, 2 weeks ago
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@collabora.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;
-	}
 
-	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;
+	}
+
 	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
Re: [PATCH v2 3/7] power: reset: qcom-pon: Migrate to devm_spmi_subdevice_alloc_and_add()
Posted by Sebastian Reichel 2 months, 2 weeks ago
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@collabora.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
>