[PATCH 1/3] powerpc/platforms/83xx/suspend: Reorder to get rid of a forward declaration

[PATCH 1/3] powerpc/platforms/83xx/suspend: Reorder to get rid of a forward declaration

[Uwe Kleine-König]

By moving up pmc_types and pmc_match, the forward declaration for pmc_match
can be dropped.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 arch/powerpc/platforms/83xx/suspend.c | 43 +++++++++++++--------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
index 6d47a5b81485..30b7700a2c98 100644
--- a/arch/powerpc/platforms/83xx/suspend.c
+++ b/arch/powerpc/platforms/83xx/suspend.c
@@ -319,7 +319,27 @@ static const struct platform_suspend_ops mpc83xx_suspend_ops = {
 	.end = mpc83xx_suspend_end,
 };
 
-static const struct of_device_id pmc_match[];
+static struct pmc_type pmc_types[] = {
+	{
+		.has_deep_sleep = 1,
+	},
+	{
+		.has_deep_sleep = 0,
+	}
+};
+
+static const struct of_device_id pmc_match[] = {
+	{
+		.compatible = "fsl,mpc8313-pmc",
+		.data = &pmc_types[0],
+	},
+	{
+		.compatible = "fsl,mpc8349-pmc",
+		.data = &pmc_types[1],
+	},
+	{}
+};
+
 static int pmc_probe(struct platform_device *ofdev)
 {
 	struct device_node *np = ofdev->dev.of_node;
@@ -406,27 +426,6 @@ static int pmc_remove(struct platform_device *ofdev)
 	return -EPERM;
 };
 
-static struct pmc_type pmc_types[] = {
-	{
-		.has_deep_sleep = 1,
-	},
-	{
-		.has_deep_sleep = 0,
-	}
-};
-
-static const struct of_device_id pmc_match[] = {
-	{
-		.compatible = "fsl,mpc8313-pmc",
-		.data = &pmc_types[0],
-	},
-	{
-		.compatible = "fsl,mpc8349-pmc",
-		.data = &pmc_types[1],
-	},
-	{}
-};
-
 static struct platform_driver pmc_driver = {
 	.driver = {
 		.name = "mpc83xx-pmc",

base-commit: f2906aa863381afb0015a9eb7fefad885d4e5a56
-- 
2.36.1

[PATCH 2/3] powerpc/platforms/83xx/suspend: Prevent unloading the driver

[Uwe Kleine-König]

Returning an error in .remove() doesn't prevent a driver from being
unloaded. On unbind this only results in an error message, but the
device is remove anyhow.

I guess the author's idea of just returning -EPERM in .remove() was to
prevent unbinding a device. To achieve that set the suppress_bind_attrs
driver property and drop the useless .remove callback.

This is a preparation for making platform remove callbacks return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 arch/powerpc/platforms/83xx/suspend.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
index 30b7700a2c98..309f42ab63d4 100644
--- a/arch/powerpc/platforms/83xx/suspend.c
+++ b/arch/powerpc/platforms/83xx/suspend.c
@@ -421,18 +421,13 @@ static int pmc_probe(struct platform_device *ofdev)
 	return ret;
 }
 
-static int pmc_remove(struct platform_device *ofdev)
-{
-	return -EPERM;
-};
-
 static struct platform_driver pmc_driver = {
 	.driver = {
 		.name = "mpc83xx-pmc",
 		.of_match_table = pmc_match,
+		.suppress_bind_attrs = true,
 	},
 	.probe = pmc_probe,
-	.remove = pmc_remove
 };
 
 builtin_platform_driver(pmc_driver);
-- 
2.36.1

Re: [PATCH 2/3] powerpc/platforms/83xx/suspend: Prevent unloading the driver

[Christophe Leroy]

Le 07/07/2022 à 08:14, Uwe Kleine-König a écrit :
> Returning an error in .remove() doesn't prevent a driver from being
> unloaded. On unbind this only results in an error message, but the
> device is remove anyhow.
> 
> I guess the author's idea of just returning -EPERM in .remove() was to
> prevent unbinding a device. To achieve that set the suppress_bind_attrs
> driver property and drop the useless .remove callback.
> 
> This is a preparation for making platform remove callbacks return void.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>


> ---
>   arch/powerpc/platforms/83xx/suspend.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
> index 30b7700a2c98..309f42ab63d4 100644
> --- a/arch/powerpc/platforms/83xx/suspend.c
> +++ b/arch/powerpc/platforms/83xx/suspend.c
> @@ -421,18 +421,13 @@ static int pmc_probe(struct platform_device *ofdev)
>   	return ret;
>   }
>   
> -static int pmc_remove(struct platform_device *ofdev)
> -{
> -	return -EPERM;
> -};
> -
>   static struct platform_driver pmc_driver = {
>   	.driver = {
>   		.name = "mpc83xx-pmc",
>   		.of_match_table = pmc_match,
> +		.suppress_bind_attrs = true,
>   	},
>   	.probe = pmc_probe,
> -	.remove = pmc_remove
>   };
>   
>   builtin_platform_driver(pmc_driver);

Re: [PATCH 2/3] powerpc/platforms/83xx/suspend: Prevent unloading the driver

[Scott Wood]

On Thu, 2022-07-07 at 08:14 +0200, Uwe Kleine-König wrote:
> Returning an error in .remove() doesn't prevent a driver from being
> unloaded. On unbind this only results in an error message, but the
> device is remove anyhow.
> 
> I guess the author's idea of just returning -EPERM in .remove() was to
> prevent unbinding a device. To achieve that set the suppress_bind_attrs
> driver property and drop the useless .remove callback.

I don't remember if I thought it would prevent removal, or if it was just the
only thing I could do to signal that removing it would be a bad idea (albeit
of relatively little consequence since it can't be built as a module).  

suppress_bind_attrs didn't exist back then. :-)

In any case,

Acked-by: Scott Wood <oss@buserror.net>

-Scott

[PATCH 3/3] powerpc/platforms/83xx/suspend: Remove write-only global variable

[Uwe Kleine-König]

pmc_dev is only assigned in .probe(), otherwise the variable is unused.
So drop this pointer that serves no purpose.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 arch/powerpc/platforms/83xx/suspend.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
index 309f42ab63d4..3fa8979ac8a6 100644
--- a/arch/powerpc/platforms/83xx/suspend.c
+++ b/arch/powerpc/platforms/83xx/suspend.c
@@ -100,7 +100,6 @@ struct pmc_type {
 	int has_deep_sleep;
 };
 
-static struct platform_device *pmc_dev;
 static int has_deep_sleep, deep_sleeping;
 static int pmc_irq;
 static struct mpc83xx_pmc __iomem *pmc_regs;
@@ -356,7 +355,6 @@ static int pmc_probe(struct platform_device *ofdev)
 
 	has_deep_sleep = type->has_deep_sleep;
 	immrbase = get_immrbase();
-	pmc_dev = ofdev;
 
 	is_pci_agent = mpc83xx_is_pci_agent();
 	if (is_pci_agent < 0)
-- 
2.36.1

Re: [PATCH 3/3] powerpc/platforms/83xx/suspend: Remove write-only global variable

[Christophe Leroy]

Le 07/07/2022 à 08:14, Uwe Kleine-König a écrit :
> pmc_dev is only assigned in .probe(), otherwise the variable is unused.
> So drop this pointer that serves no purpose.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
>   arch/powerpc/platforms/83xx/suspend.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
> index 309f42ab63d4..3fa8979ac8a6 100644
> --- a/arch/powerpc/platforms/83xx/suspend.c
> +++ b/arch/powerpc/platforms/83xx/suspend.c
> @@ -100,7 +100,6 @@ struct pmc_type {
>   	int has_deep_sleep;
>   };
>   
> -static struct platform_device *pmc_dev;
>   static int has_deep_sleep, deep_sleeping;
>   static int pmc_irq;
>   static struct mpc83xx_pmc __iomem *pmc_regs;
> @@ -356,7 +355,6 @@ static int pmc_probe(struct platform_device *ofdev)
>   
>   	has_deep_sleep = type->has_deep_sleep;
>   	immrbase = get_immrbase();
> -	pmc_dev = ofdev;
>   
>   	is_pci_agent = mpc83xx_is_pci_agent();
>   	if (is_pci_agent < 0)

Re: [PATCH 1/3] powerpc/platforms/83xx/suspend: Reorder to get rid of a forward declaration

[Christophe Leroy]

Le 07/07/2022 à 08:14, Uwe Kleine-König a écrit :
> By moving up pmc_types and pmc_match, the forward declaration for pmc_match
> can be dropped.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
>   arch/powerpc/platforms/83xx/suspend.c | 43 +++++++++++++--------------
>   1 file changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
> index 6d47a5b81485..30b7700a2c98 100644
> --- a/arch/powerpc/platforms/83xx/suspend.c
> +++ b/arch/powerpc/platforms/83xx/suspend.c
> @@ -319,7 +319,27 @@ static const struct platform_suspend_ops mpc83xx_suspend_ops = {
>   	.end = mpc83xx_suspend_end,
>   };
>   
> -static const struct of_device_id pmc_match[];
> +static struct pmc_type pmc_types[] = {
> +	{
> +		.has_deep_sleep = 1,
> +	},
> +	{
> +		.has_deep_sleep = 0,
> +	}
> +};
> +
> +static const struct of_device_id pmc_match[] = {
> +	{
> +		.compatible = "fsl,mpc8313-pmc",
> +		.data = &pmc_types[0],
> +	},
> +	{
> +		.compatible = "fsl,mpc8349-pmc",
> +		.data = &pmc_types[1],
> +	},
> +	{}
> +};
> +
>   static int pmc_probe(struct platform_device *ofdev)
>   {
>   	struct device_node *np = ofdev->dev.of_node;
> @@ -406,27 +426,6 @@ static int pmc_remove(struct platform_device *ofdev)
>   	return -EPERM;
>   };
>   
> -static struct pmc_type pmc_types[] = {
> -	{
> -		.has_deep_sleep = 1,
> -	},
> -	{
> -		.has_deep_sleep = 0,
> -	}
> -};
> -
> -static const struct of_device_id pmc_match[] = {
> -	{
> -		.compatible = "fsl,mpc8313-pmc",
> -		.data = &pmc_types[0],
> -	},
> -	{
> -		.compatible = "fsl,mpc8349-pmc",
> -		.data = &pmc_types[1],
> -	},
> -	{}
> -};
> -
>   static struct platform_driver pmc_driver = {
>   	.driver = {
>   		.name = "mpc83xx-pmc",
> 
> base-commit: f2906aa863381afb0015a9eb7fefad885d4e5a56

Re: [PATCH 1/3] powerpc/platforms/83xx/suspend: Reorder to get rid of a forward declaration

[Michael Ellerman]

On Thu, 7 Jul 2022 08:14:39 +0200, Uwe Kleine-König wrote:
> By moving up pmc_types and pmc_match, the forward declaration for pmc_match
> can be dropped.
> 
> 

Applied to powerpc/next.

[1/3] powerpc/platforms/83xx/suspend: Reorder to get rid of a forward declaration
      https://git.kernel.org/powerpc/c/fde345e4d39a4f16697a8060564fff1dbac05035
[2/3] powerpc/platforms/83xx/suspend: Prevent unloading the driver
      https://git.kernel.org/powerpc/c/ccc1439b924bca5d5a5d81cf6b0d4b10b321282e
[3/3] powerpc/platforms/83xx/suspend: Remove write-only global variable
      https://git.kernel.org/powerpc/c/95b002e4e47a36d88deec70808ef36674fb33cf5

cheers