[PATCH 1/5] power: reset: brcmstb: Use normal driver register function

[PATCH 1/5] power: reset: brcmstb: Use normal driver register function

[Andrew Davis]

The platform_driver_probe() helper is useful when the probe function
is in the _init section, that is not the case here. Use the normal
platform_driver_register() function.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/power/reset/brcmstb-reboot.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/power/reset/brcmstb-reboot.c b/drivers/power/reset/brcmstb-reboot.c
index 0f2944dc93551..797f0079bb590 100644
--- a/drivers/power/reset/brcmstb-reboot.c
+++ b/drivers/power/reset/brcmstb-reboot.c
@@ -140,7 +140,6 @@ static struct platform_driver brcmstb_reboot_driver = {
 
 static int __init brcmstb_reboot_init(void)
 {
-	return platform_driver_probe(&brcmstb_reboot_driver,
-					brcmstb_reboot_probe);
+	return platform_driver_register(&brcmstb_reboot_driver);
 }
 subsys_initcall(brcmstb_reboot_init);
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/5] power: reset: brcmstb: Use device_get_match_data() for matching

[Andrew Davis]

Use device_get_match_data() for finding the matching node and fetching
the match data all in one.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/power/reset/brcmstb-reboot.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/power/reset/brcmstb-reboot.c b/drivers/power/reset/brcmstb-reboot.c
index 797f0079bb590..db5b7120eadd0 100644
--- a/drivers/power/reset/brcmstb-reboot.c
+++ b/drivers/power/reset/brcmstb-reboot.c
@@ -83,24 +83,16 @@ static const struct reset_reg_mask reset_bits_65nm = {
 	.sw_mstr_rst_mask = BIT(31),
 };
 
-static const struct of_device_id of_match[] = {
-	{ .compatible = "brcm,brcmstb-reboot", .data = &reset_bits_40nm },
-	{ .compatible = "brcm,bcm7038-reboot", .data = &reset_bits_65nm },
-	{},
-};
-
 static int brcmstb_reboot_probe(struct platform_device *pdev)
 {
 	int rc;
 	struct device_node *np = pdev->dev.of_node;
-	const struct of_device_id *of_id;
 
-	of_id = of_match_node(of_match, np);
-	if (!of_id) {
-		pr_err("failed to look up compatible string\n");
+	reset_masks = device_get_match_data(&pdev->dev);
+	if (!reset_masks) {
+		pr_err("failed to get match data\n");
 		return -EINVAL;
 	}
-	reset_masks = of_id->data;
 
 	regmap = syscon_regmap_lookup_by_phandle(np, "syscon");
 	if (IS_ERR(regmap)) {
@@ -130,6 +122,12 @@ static int brcmstb_reboot_probe(struct platform_device *pdev)
 	return rc;
 }
 
+static const struct of_device_id of_match[] = {
+	{ .compatible = "brcm,brcmstb-reboot", .data = &reset_bits_40nm },
+	{ .compatible = "brcm,bcm7038-reboot", .data = &reset_bits_65nm },
+	{},
+};
+
 static struct platform_driver brcmstb_reboot_driver = {
 	.probe = brcmstb_reboot_probe,
 	.driver = {
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/5] power: reset: brcmstb: Use device_get_match_data() for matching

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 264 bytes --]



On 6/10/2024 3:28 PM, Andrew Davis wrote:
> Use device_get_match_data() for finding the matching node and fetching
> the match data all in one.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>

Acked-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

[PATCH 3/5] power: reset: brcmstb: Use syscon_regmap_lookup_by_phandle_args() helper

[Andrew Davis]

Simplify probe by fetching the regmap and its arguments in one call.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/power/reset/brcmstb-reboot.c | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/drivers/power/reset/brcmstb-reboot.c b/drivers/power/reset/brcmstb-reboot.c
index db5b7120eadd0..94ea317f61ef4 100644
--- a/drivers/power/reset/brcmstb-reboot.c
+++ b/drivers/power/reset/brcmstb-reboot.c
@@ -18,9 +18,6 @@
 #include <linux/smp.h>
 #include <linux/mfd/syscon.h>
 
-#define RESET_SOURCE_ENABLE_REG 1
-#define SW_MASTER_RESET_REG 2
-
 static struct regmap *regmap;
 static u32 rst_src_en;
 static u32 sw_mstr_rst;
@@ -87,6 +84,7 @@ static int brcmstb_reboot_probe(struct platform_device *pdev)
 {
 	int rc;
 	struct device_node *np = pdev->dev.of_node;
+	unsigned int args[2];
 
 	reset_masks = device_get_match_data(&pdev->dev);
 	if (!reset_masks) {
@@ -94,25 +92,13 @@ static int brcmstb_reboot_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	regmap = syscon_regmap_lookup_by_phandle(np, "syscon");
+	regmap = syscon_regmap_lookup_by_phandle_args(np, "syscon", 2, args);
 	if (IS_ERR(regmap)) {
 		pr_err("failed to get syscon phandle\n");
 		return -EINVAL;
 	}
-
-	rc = of_property_read_u32_index(np, "syscon", RESET_SOURCE_ENABLE_REG,
-					&rst_src_en);
-	if (rc) {
-		pr_err("can't get rst_src_en offset (%d)\n", rc);
-		return -EINVAL;
-	}
-
-	rc = of_property_read_u32_index(np, "syscon", SW_MASTER_RESET_REG,
-					&sw_mstr_rst);
-	if (rc) {
-		pr_err("can't get sw_mstr_rst offset (%d)\n", rc);
-		return -EINVAL;
-	}
+	rst_src_en = args[0];
+	sw_mstr_rst = args[1];
 
 	rc = register_restart_handler(&brcmstb_restart_nb);
 	if (rc)
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] power: reset: brcmstb: Use syscon_regmap_lookup_by_phandle_args() helper

[Dhruva Gole]

Hi,

On Jun 10, 2024 at 09:28:34 -0500, Andrew Davis wrote:
> Simplify probe by fetching the regmap and its arguments in one call.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  drivers/power/reset/brcmstb-reboot.c | 22 ++++------------------
>  1 file changed, 4 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/power/reset/brcmstb-reboot.c b/drivers/power/reset/brcmstb-reboot.c
> index db5b7120eadd0..94ea317f61ef4 100644
> --- a/drivers/power/reset/brcmstb-reboot.c
> +++ b/drivers/power/reset/brcmstb-reboot.c
> @@ -18,9 +18,6 @@
>  #include <linux/smp.h>
>  #include <linux/mfd/syscon.h>
>  
> -#define RESET_SOURCE_ENABLE_REG 1
> -#define SW_MASTER_RESET_REG 2
> -
>  static struct regmap *regmap;
>  static u32 rst_src_en;
>  static u32 sw_mstr_rst;
> @@ -87,6 +84,7 @@ static int brcmstb_reboot_probe(struct platform_device *pdev)
>  {
>  	int rc;
>  	struct device_node *np = pdev->dev.of_node;
> +	unsigned int args[2];

You can also call it syscon_args, but I'm fine either way.

>  
>  	reset_masks = device_get_match_data(&pdev->dev);
>  	if (!reset_masks) {
> @@ -94,25 +92,13 @@ static int brcmstb_reboot_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> -	regmap = syscon_regmap_lookup_by_phandle(np, "syscon");
> +	regmap = syscon_regmap_lookup_by_phandle_args(np, "syscon", 2, args);
>  	if (IS_ERR(regmap)) {
>  		pr_err("failed to get syscon phandle\n");
>  		return -EINVAL;
>  	}
> -
> -	rc = of_property_read_u32_index(np, "syscon", RESET_SOURCE_ENABLE_REG,
> -					&rst_src_en);
> -	if (rc) {
> -		pr_err("can't get rst_src_en offset (%d)\n", rc);
> -		return -EINVAL;
> -	}
> -
> -	rc = of_property_read_u32_index(np, "syscon", SW_MASTER_RESET_REG,
> -					&sw_mstr_rst);
> -	if (rc) {
> -		pr_err("can't get sw_mstr_rst offset (%d)\n", rc);
> -		return -EINVAL;
> -	}
> +	rst_src_en = args[0];
> +	sw_mstr_rst = args[1];

Reviewed-by: Dhruva Gole <d-gole@ti.com>


-- 
Best regards,
Dhruva

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] power: reset: brcmstb: Use syscon_regmap_lookup_by_phandle_args() helper

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 1437 bytes --]



On 6/10/2024 3:28 PM, Andrew Davis wrote:
> Simplify probe by fetching the regmap and its arguments in one call.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>   drivers/power/reset/brcmstb-reboot.c | 22 ++++------------------
>   1 file changed, 4 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/power/reset/brcmstb-reboot.c b/drivers/power/reset/brcmstb-reboot.c
> index db5b7120eadd0..94ea317f61ef4 100644
> --- a/drivers/power/reset/brcmstb-reboot.c
> +++ b/drivers/power/reset/brcmstb-reboot.c
> @@ -18,9 +18,6 @@
>   #include <linux/smp.h>
>   #include <linux/mfd/syscon.h>
>   
> -#define RESET_SOURCE_ENABLE_REG 1
> -#define SW_MASTER_RESET_REG 2
> -
>   static struct regmap *regmap;
>   static u32 rst_src_en;
>   static u32 sw_mstr_rst;
> @@ -87,6 +84,7 @@ static int brcmstb_reboot_probe(struct platform_device *pdev)
>   {
>   	int rc;
>   	struct device_node *np = pdev->dev.of_node;
> +	unsigned int args[2];
>   
>   	reset_masks = device_get_match_data(&pdev->dev);
>   	if (!reset_masks) {
> @@ -94,25 +92,13 @@ static int brcmstb_reboot_probe(struct platform_device *pdev)
>   		return -EINVAL;
>   	}
>   
> -	regmap = syscon_regmap_lookup_by_phandle(np, "syscon");
> +	regmap = syscon_regmap_lookup_by_phandle_args(np, "syscon", 2, args);

Not that this is likely to change, but I would use ARRAY_SIZE(args) 
here. With that:

Acked-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

[PATCH 4/5] power: reset: brcmstb: Use devm_register_sys_off_handler()

[Andrew Davis]

Function register_restart_handler() is deprecated. Using this new API
removes our need to keep and manage a struct notifier_block.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/power/reset/brcmstb-reboot.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/power/reset/brcmstb-reboot.c b/drivers/power/reset/brcmstb-reboot.c
index 94ea317f61ef4..59ed1513cfb30 100644
--- a/drivers/power/reset/brcmstb-reboot.c
+++ b/drivers/power/reset/brcmstb-reboot.c
@@ -29,8 +29,7 @@ struct reset_reg_mask {
 
 static const struct reset_reg_mask *reset_masks;
 
-static int brcmstb_restart_handler(struct notifier_block *this,
-				   unsigned long mode, void *cmd)
+static int brcmstb_restart_handler(struct sys_off_data *data)
 {
 	int rc;
 	u32 tmp;
@@ -65,11 +64,6 @@ static int brcmstb_restart_handler(struct notifier_block *this,
 	return NOTIFY_DONE;
 }
 
-static struct notifier_block brcmstb_restart_nb = {
-	.notifier_call = brcmstb_restart_handler,
-	.priority = 128,
-};
-
 static const struct reset_reg_mask reset_bits_40nm = {
 	.rst_src_en_mask = BIT(0),
 	.sw_mstr_rst_mask = BIT(0),
@@ -100,7 +94,8 @@ static int brcmstb_reboot_probe(struct platform_device *pdev)
 	rst_src_en = args[0];
 	sw_mstr_rst = args[1];
 
-	rc = register_restart_handler(&brcmstb_restart_nb);
+	rc = devm_register_sys_off_handler(&pdev->dev, SYS_OFF_MODE_RESTART,
+					   128, brcmstb_restart_handler, NULL);
 	if (rc)
 		dev_err(&pdev->dev,
 			"cannot register restart handler (err=%d)\n", rc);
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/5] power: reset: brcmstb: Use devm_register_sys_off_handler()

[Dhruva Gole]

On Jun 10, 2024 at 09:28:35 -0500, Andrew Davis wrote:
> Function register_restart_handler() is deprecated. Using this new API
> removes our need to keep and manage a struct notifier_block.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  drivers/power/reset/brcmstb-reboot.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/power/reset/brcmstb-reboot.c b/drivers/power/reset/brcmstb-reboot.c
> index 94ea317f61ef4..59ed1513cfb30 100644
> --- a/drivers/power/reset/brcmstb-reboot.c
> +++ b/drivers/power/reset/brcmstb-reboot.c
> @@ -29,8 +29,7 @@ struct reset_reg_mask {
>  
>  static const struct reset_reg_mask *reset_masks;
>  
> -static int brcmstb_restart_handler(struct notifier_block *this,
> -				   unsigned long mode, void *cmd)
> +static int brcmstb_restart_handler(struct sys_off_data *data)
>  {
>  	int rc;
>  	u32 tmp;
> @@ -65,11 +64,6 @@ static int brcmstb_restart_handler(struct notifier_block *this,
>  	return NOTIFY_DONE;
>  }
>  
> -static struct notifier_block brcmstb_restart_nb = {
> -	.notifier_call = brcmstb_restart_handler,
> -	.priority = 128,
> -};
> -
>  static const struct reset_reg_mask reset_bits_40nm = {
>  	.rst_src_en_mask = BIT(0),
>  	.sw_mstr_rst_mask = BIT(0),
> @@ -100,7 +94,8 @@ static int brcmstb_reboot_probe(struct platform_device *pdev)
>  	rst_src_en = args[0];
>  	sw_mstr_rst = args[1];
>  
> -	rc = register_restart_handler(&brcmstb_restart_nb);
> +	rc = devm_register_sys_off_handler(&pdev->dev, SYS_OFF_MODE_RESTART,
> +					   128, brcmstb_restart_handler, NULL);

Reviewed-by: Dhruva Gole <d-gole@ti.com>


-- 
Best regards,
Dhruva

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/5] power: reset: brcmstb: Use devm_register_sys_off_handler()

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 297 bytes --]



On 6/10/2024 3:28 PM, Andrew Davis wrote:
> Function register_restart_handler() is deprecated. Using this new API
> removes our need to keep and manage a struct notifier_block.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>

Acked-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

[PATCH 5/5] power: reset: brcmstb: Do not go into infinite loop if reset fails

[Andrew Davis]

There may be other backup reset methods available, do not halt
here so that other reset methods can be tried.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/power/reset/brcmstb-reboot.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/power/reset/brcmstb-reboot.c b/drivers/power/reset/brcmstb-reboot.c
index 59ed1513cfb30..441b44e0a9f29 100644
--- a/drivers/power/reset/brcmstb-reboot.c
+++ b/drivers/power/reset/brcmstb-reboot.c
@@ -58,9 +58,6 @@ static int brcmstb_restart_handler(struct sys_off_data *data)
 		return NOTIFY_DONE;
 	}
 
-	while (1)
-		;
-
 	return NOTIFY_DONE;
 }
 
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/5] power: reset: brcmstb: Do not go into infinite loop if reset fails

[Dhruva Gole]

On Jun 10, 2024 at 09:28:36 -0500, Andrew Davis wrote:
> There may be other backup reset methods available, do not halt
> here so that other reset methods can be tried.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  drivers/power/reset/brcmstb-reboot.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/power/reset/brcmstb-reboot.c b/drivers/power/reset/brcmstb-reboot.c
> index 59ed1513cfb30..441b44e0a9f29 100644
> --- a/drivers/power/reset/brcmstb-reboot.c
> +++ b/drivers/power/reset/brcmstb-reboot.c
> @@ -58,9 +58,6 @@ static int brcmstb_restart_handler(struct sys_off_data *data)
>  		return NOTIFY_DONE;
>  	}
>  
> -	while (1)
> -		;
> -

I agree, while (1) may not be the best thing to do here.
Reviewed-by: Dhruva Gole <d-gole@ti.com>

-- 
Best regards,
Dhruva

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/5] power: reset: brcmstb: Do not go into infinite loop if reset fails

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 276 bytes --]



On 6/10/2024 3:28 PM, Andrew Davis wrote:
> There may be other backup reset methods available, do not halt
> here so that other reset methods can be tried.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>

Acked-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH 1/5] power: reset: brcmstb: Use normal driver register function

[Dhruva Gole]

Hi,

On Jun 10, 2024 at 09:28:32 -0500, Andrew Davis wrote:
> The platform_driver_probe() helper is useful when the probe function
> is in the _init section, that is not the case here. Use the normal
> platform_driver_register() function.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  drivers/power/reset/brcmstb-reboot.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/power/reset/brcmstb-reboot.c b/drivers/power/reset/brcmstb-reboot.c
> index 0f2944dc93551..797f0079bb590 100644
> --- a/drivers/power/reset/brcmstb-reboot.c
> +++ b/drivers/power/reset/brcmstb-reboot.c
> @@ -140,7 +140,6 @@ static struct platform_driver brcmstb_reboot_driver = {
>  
>  static int __init brcmstb_reboot_init(void)
>  {
> -	return platform_driver_probe(&brcmstb_reboot_driver,
> -					brcmstb_reboot_probe);
> +	return platform_driver_register(&brcmstb_reboot_driver);

Reviewed-by: Dhruva Gole <d-gole@ti.com>


-- 
Best regards,
Dhruva

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/5] power: reset: brcmstb: Use normal driver register function

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 341 bytes --]



On 6/10/2024 3:28 PM, Andrew Davis wrote:
> The platform_driver_probe() helper is useful when the probe function
> is in the _init section, that is not the case here. Use the normal
> platform_driver_register() function.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>

Acked-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH 1/5] power: reset: brcmstb: Use normal driver register function

[Sebastian Reichel]

On Mon, 10 Jun 2024 09:28:32 -0500, Andrew Davis wrote:
> The platform_driver_probe() helper is useful when the probe function
> is in the _init section, that is not the case here. Use the normal
> platform_driver_register() function.
> 
> 

Applied, thanks!

[1/5] power: reset: brcmstb: Use normal driver register function
      commit: 0174d12f9b7ebc83f1f2b6c25f05304104de5a91
[2/5] power: reset: brcmstb: Use device_get_match_data() for matching
      commit: cf37f16a60f332fad21fe0a45893ac4c6a825d9d
[3/5] power: reset: brcmstb: Use syscon_regmap_lookup_by_phandle_args() helper
      commit: a4ceaab660cabdc9ac2d2514a809174443209b3b
[4/5] power: reset: brcmstb: Use devm_register_sys_off_handler()
      commit: ad87aee5cba821066be99c76efd818368ff5bb4a
[5/5] power: reset: brcmstb: Do not go into infinite loop if reset fails
      commit: cf8c39b00e982fa506b16f9d76657838c09150cb

Best regards,
-- 
Sebastian Reichel <sebastian.reichel@collabora.com>