All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Pankaj Dubey <pankaj.dubey-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	arnd-r2nGTMty4D4@public.gmane.org,
	krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	kgene-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org,
	thomas.ab-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org
Subject: Re: [RESEND-PATCH] ARM: EXYNOS: remove smp hook from machine descriptor
Date: Thu, 8 Dec 2016 19:33:49 +0200	[thread overview]
Message-ID: <20161208173349.GA8522@kozik-lap> (raw)
In-Reply-To: <1481166135-1588-1-git-send-email-pankaj.dubey-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

On Thu, Dec 08, 2016 at 08:32:15AM +0530, Pankaj Dubey wrote:
> Use CPU_METHOD_OF_DECLARE() for smp_ops instead of using it
> via machine descriptor.
> 
> Signed-off-by: Pankaj Dubey <pankaj.dubey-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> 
> Resending as I missed to include samsung mailing list.
> 
>  arch/arm/boot/dts/exynos3250.dtsi      | 1 +
>  arch/arm/boot/dts/exynos4210.dtsi      | 1 +
>  arch/arm/boot/dts/exynos4212.dtsi      | 1 +
>  arch/arm/boot/dts/exynos4412.dtsi      | 1 +
>  arch/arm/boot/dts/exynos5250.dtsi      | 1 +
>  arch/arm/boot/dts/exynos5260.dtsi      | 1 +
>  arch/arm/boot/dts/exynos5410.dtsi      | 1 +
>  arch/arm/boot/dts/exynos5420-cpus.dtsi | 1 +
>  arch/arm/boot/dts/exynos5422-cpus.dtsi | 1 +
>  arch/arm/boot/dts/exynos5440.dtsi      | 1 +
>  arch/arm/mach-exynos/common.h          | 2 --
>  arch/arm/mach-exynos/exynos.c          | 1 -
>  arch/arm/mach-exynos/platsmp.c         | 2 ++
>  13 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
> index ba17ee1..f28f669 100644
> --- a/arch/arm/boot/dts/exynos3250.dtsi
> +++ b/arch/arm/boot/dts/exynos3250.dtsi
> @@ -53,6 +53,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu0: cpu@0 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
> index 7f3a18c..6dfd98d 100644
> --- a/arch/arm/boot/dts/exynos4210.dtsi
> +++ b/arch/arm/boot/dts/exynos4210.dtsi
> @@ -35,6 +35,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu0: cpu@900 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
> index 5389011..3e8982e 100644
> --- a/arch/arm/boot/dts/exynos4212.dtsi
> +++ b/arch/arm/boot/dts/exynos4212.dtsi
> @@ -25,6 +25,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu0: cpu@A00 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
> index 40beede..faf2fb8 100644
> --- a/arch/arm/boot/dts/exynos4412.dtsi
> +++ b/arch/arm/boot/dts/exynos4412.dtsi
> @@ -25,6 +25,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu0: cpu@A00 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> index b6d7444..580897c 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -52,6 +52,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu0: cpu@0 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos5260.dtsi b/arch/arm/boot/dts/exynos5260.dtsi
> index 5818718..1af6e76 100644
> --- a/arch/arm/boot/dts/exynos5260.dtsi
> +++ b/arch/arm/boot/dts/exynos5260.dtsi
> @@ -32,6 +32,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu@0 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos5410.dtsi b/arch/arm/boot/dts/exynos5410.dtsi
> index 2b6adaf..b092cdc 100644
> --- a/arch/arm/boot/dts/exynos5410.dtsi
> +++ b/arch/arm/boot/dts/exynos5410.dtsi
> @@ -33,6 +33,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu0: cpu@0 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos5420-cpus.dtsi b/arch/arm/boot/dts/exynos5420-cpus.dtsi
> index 5c052d7..a587704 100644
> --- a/arch/arm/boot/dts/exynos5420-cpus.dtsi
> +++ b/arch/arm/boot/dts/exynos5420-cpus.dtsi
> @@ -24,6 +24,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu0: cpu@0 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos5422-cpus.dtsi b/arch/arm/boot/dts/exynos5422-cpus.dtsi
> index bf3c6f1..7fcdfd0 100644
> --- a/arch/arm/boot/dts/exynos5422-cpus.dtsi
> +++ b/arch/arm/boot/dts/exynos5422-cpus.dtsi
> @@ -23,6 +23,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu0: cpu@100 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos5440.dtsi b/arch/arm/boot/dts/exynos5440.dtsi
> index 2a2e570..0a958e8 100644
> --- a/arch/arm/boot/dts/exynos5440.dtsi
> +++ b/arch/arm/boot/dts/exynos5440.dtsi
> @@ -50,6 +50,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu@0 {
>  			device_type = "cpu";
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index fb12d11..051e1ab 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -143,8 +143,6 @@ static inline void exynos_pm_init(void) {}
>  extern void exynos_cpu_resume(void);
>  extern void exynos_cpu_resume_ns(void);
>  
> -extern const struct smp_operations exynos_smp_ops;
> -
>  extern void exynos_cpu_power_down(int cpu);
>  extern void exynos_cpu_power_up(int cpu);
>  extern int  exynos_cpu_power_state(int cpu);
> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> index fa08ef9..f0a766e 100644
> --- a/arch/arm/mach-exynos/exynos.c
> +++ b/arch/arm/mach-exynos/exynos.c
> @@ -211,7 +211,6 @@ DT_MACHINE_START(EXYNOS_DT, "SAMSUNG EXYNOS (Flattened Device Tree)")
>  	/* Maintainer: Kukjin Kim <kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> */
>  	.l2c_aux_val	= 0x3c400001,
>  	.l2c_aux_mask	= 0xc20fffff,
> -	.smp		= smp_ops(exynos_smp_ops),
>  	.map_io		= exynos_init_io,
>  	.init_early	= exynos_firmware_init,
>  	.init_irq	= exynos_init_irq,
> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index 94405c7..43eec10 100644
> --- a/arch/arm/mach-exynos/platsmp.c
> +++ b/arch/arm/mach-exynos/platsmp.c
> @@ -474,3 +474,5 @@ const struct smp_operations exynos_smp_ops __initconst = {
>  	.cpu_die		= exynos_cpu_die,
>  #endif
>  };
> +
> +CPU_METHOD_OF_DECLARE(exynos_smp, "samsung,exynos-smp", &exynos_smp_ops);

Three issues:
1. This has to be documented. Checkpatch did not complain about it?
2. I think this breaks the ABI with existing DTBs because now the
   enable-method of cpus becomes mandatory. But the
   Documentation/devicetree/bindings/arm/cpus.txt is saying that:
   "On ARM 32-bit systems this property is optional and can be one of"

3. Please split DTS changes to separate patches. This is, by the way,
   connected with #2 above: if there was no ABI break, then the DTS
   could go to separate branch easily.

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: krzk@kernel.org (Krzysztof Kozlowski)
To: linux-arm-kernel@lists.infradead.org
Subject: [RESEND-PATCH] ARM: EXYNOS: remove smp hook from machine descriptor
Date: Thu, 8 Dec 2016 19:33:49 +0200	[thread overview]
Message-ID: <20161208173349.GA8522@kozik-lap> (raw)
In-Reply-To: <1481166135-1588-1-git-send-email-pankaj.dubey@samsung.com>

On Thu, Dec 08, 2016 at 08:32:15AM +0530, Pankaj Dubey wrote:
> Use CPU_METHOD_OF_DECLARE() for smp_ops instead of using it
> via machine descriptor.
> 
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> 
> Resending as I missed to include samsung mailing list.
> 
>  arch/arm/boot/dts/exynos3250.dtsi      | 1 +
>  arch/arm/boot/dts/exynos4210.dtsi      | 1 +
>  arch/arm/boot/dts/exynos4212.dtsi      | 1 +
>  arch/arm/boot/dts/exynos4412.dtsi      | 1 +
>  arch/arm/boot/dts/exynos5250.dtsi      | 1 +
>  arch/arm/boot/dts/exynos5260.dtsi      | 1 +
>  arch/arm/boot/dts/exynos5410.dtsi      | 1 +
>  arch/arm/boot/dts/exynos5420-cpus.dtsi | 1 +
>  arch/arm/boot/dts/exynos5422-cpus.dtsi | 1 +
>  arch/arm/boot/dts/exynos5440.dtsi      | 1 +
>  arch/arm/mach-exynos/common.h          | 2 --
>  arch/arm/mach-exynos/exynos.c          | 1 -
>  arch/arm/mach-exynos/platsmp.c         | 2 ++
>  13 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
> index ba17ee1..f28f669 100644
> --- a/arch/arm/boot/dts/exynos3250.dtsi
> +++ b/arch/arm/boot/dts/exynos3250.dtsi
> @@ -53,6 +53,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu0: cpu at 0 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
> index 7f3a18c..6dfd98d 100644
> --- a/arch/arm/boot/dts/exynos4210.dtsi
> +++ b/arch/arm/boot/dts/exynos4210.dtsi
> @@ -35,6 +35,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu0: cpu at 900 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
> index 5389011..3e8982e 100644
> --- a/arch/arm/boot/dts/exynos4212.dtsi
> +++ b/arch/arm/boot/dts/exynos4212.dtsi
> @@ -25,6 +25,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu0: cpu at A00 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
> index 40beede..faf2fb8 100644
> --- a/arch/arm/boot/dts/exynos4412.dtsi
> +++ b/arch/arm/boot/dts/exynos4412.dtsi
> @@ -25,6 +25,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu0: cpu at A00 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> index b6d7444..580897c 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -52,6 +52,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu0: cpu at 0 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos5260.dtsi b/arch/arm/boot/dts/exynos5260.dtsi
> index 5818718..1af6e76 100644
> --- a/arch/arm/boot/dts/exynos5260.dtsi
> +++ b/arch/arm/boot/dts/exynos5260.dtsi
> @@ -32,6 +32,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu at 0 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos5410.dtsi b/arch/arm/boot/dts/exynos5410.dtsi
> index 2b6adaf..b092cdc 100644
> --- a/arch/arm/boot/dts/exynos5410.dtsi
> +++ b/arch/arm/boot/dts/exynos5410.dtsi
> @@ -33,6 +33,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu0: cpu at 0 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos5420-cpus.dtsi b/arch/arm/boot/dts/exynos5420-cpus.dtsi
> index 5c052d7..a587704 100644
> --- a/arch/arm/boot/dts/exynos5420-cpus.dtsi
> +++ b/arch/arm/boot/dts/exynos5420-cpus.dtsi
> @@ -24,6 +24,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu0: cpu at 0 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos5422-cpus.dtsi b/arch/arm/boot/dts/exynos5422-cpus.dtsi
> index bf3c6f1..7fcdfd0 100644
> --- a/arch/arm/boot/dts/exynos5422-cpus.dtsi
> +++ b/arch/arm/boot/dts/exynos5422-cpus.dtsi
> @@ -23,6 +23,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu0: cpu at 100 {
>  			device_type = "cpu";
> diff --git a/arch/arm/boot/dts/exynos5440.dtsi b/arch/arm/boot/dts/exynos5440.dtsi
> index 2a2e570..0a958e8 100644
> --- a/arch/arm/boot/dts/exynos5440.dtsi
> +++ b/arch/arm/boot/dts/exynos5440.dtsi
> @@ -50,6 +50,7 @@
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> +		enable-method = "samsung,exynos-smp";
>  
>  		cpu at 0 {
>  			device_type = "cpu";
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index fb12d11..051e1ab 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -143,8 +143,6 @@ static inline void exynos_pm_init(void) {}
>  extern void exynos_cpu_resume(void);
>  extern void exynos_cpu_resume_ns(void);
>  
> -extern const struct smp_operations exynos_smp_ops;
> -
>  extern void exynos_cpu_power_down(int cpu);
>  extern void exynos_cpu_power_up(int cpu);
>  extern int  exynos_cpu_power_state(int cpu);
> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> index fa08ef9..f0a766e 100644
> --- a/arch/arm/mach-exynos/exynos.c
> +++ b/arch/arm/mach-exynos/exynos.c
> @@ -211,7 +211,6 @@ DT_MACHINE_START(EXYNOS_DT, "SAMSUNG EXYNOS (Flattened Device Tree)")
>  	/* Maintainer: Kukjin Kim <kgene.kim@samsung.com> */
>  	.l2c_aux_val	= 0x3c400001,
>  	.l2c_aux_mask	= 0xc20fffff,
> -	.smp		= smp_ops(exynos_smp_ops),
>  	.map_io		= exynos_init_io,
>  	.init_early	= exynos_firmware_init,
>  	.init_irq	= exynos_init_irq,
> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index 94405c7..43eec10 100644
> --- a/arch/arm/mach-exynos/platsmp.c
> +++ b/arch/arm/mach-exynos/platsmp.c
> @@ -474,3 +474,5 @@ const struct smp_operations exynos_smp_ops __initconst = {
>  	.cpu_die		= exynos_cpu_die,
>  #endif
>  };
> +
> +CPU_METHOD_OF_DECLARE(exynos_smp, "samsung,exynos-smp", &exynos_smp_ops);

Three issues:
1. This has to be documented. Checkpatch did not complain about it?
2. I think this breaks the ABI with existing DTBs because now the
   enable-method of cpus becomes mandatory. But the
   Documentation/devicetree/bindings/arm/cpus.txt is saying that:
   "On ARM 32-bit systems this property is optional and can be one of"

3. Please split DTS changes to separate patches. This is, by the way,
   connected with #2 above: if there was no ABI break, then the DTS
   could go to separate branch easily.

Best regards,
Krzysztof

  parent reply	other threads:[~2016-12-08 17:33 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-08  3:02 [RESEND-PATCH] ARM: EXYNOS: remove smp hook from machine descriptor Pankaj Dubey
2016-12-08  3:02 ` Pankaj Dubey
     [not found] ` <1481166135-1588-1-git-send-email-pankaj.dubey-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2016-12-08 17:33   ` Krzysztof Kozlowski [this message]
2016-12-08 17:33     ` Krzysztof Kozlowski
2016-12-09  9:12     ` pankaj.dubey
2016-12-09  9:12       ` pankaj.dubey
2016-12-09 14:24       ` Krzysztof Kozlowski
2016-12-09 14:24         ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161208173349.GA8522@kozik-lap \
    --to=krzk-dgejt+ai2ygdnm+yrofe0a@public.gmane.org \
    --cc=arnd-r2nGTMty4D4@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org \
    --cc=kgene-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=pankaj.dubey-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org \
    --cc=thomas.ab-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.