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 <[email protected]>
---
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
Simplify probe by fetching the regmap and its arguments in one call.
Signed-off-by: Andrew Davis <[email protected]>
---
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
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 <[email protected]>
---
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
Use device_get_match_data() for finding the matching node and fetching
the match data all in one.
Signed-off-by: Andrew Davis <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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 <[email protected]>
--
Best regards,
Dhruva
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 <[email protected]>
> ---
> 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 <[email protected]>
--
Best regards,
Dhruva
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 <[email protected]>
> ---
> 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 <[email protected]>
--
Best regards,
Dhruva
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 <[email protected]>
> ---
> 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 <[email protected]>
--
Best regards,
Dhruva
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 <[email protected]>
> ---
> 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 <[email protected]>
--
Florian
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 <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
--
Florian
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 <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
--
Florian
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 <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
--
Florian
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 <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
--
Florian