All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] vc04_services: vchiq-mmal: Drop bool usage
@ 2022-11-17 12:59 ` Umang Jain
  0 siblings, 0 replies; 18+ messages in thread
From: Umang Jain @ 2022-11-17 12:59 UTC (permalink / raw)
  To: Florian Fainelli, Broadcom internal kernel review list,
	Greg Kroah-Hartman, Stefan Wahren, Hans Verkuil, Dave Stevenson,
	Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, linux-kernel
  Cc: kieran.bingham, Umang Jain

Simple fixes to drop bool usage from vchiq-mmal.
Individual patches contains details.

Dave Stevenson (1):
  staging: vc04_services: mmal-vchiq: Do not assign bool to u32

Umang Jain (1):
  staging: vc04_services: mmal-common: Do not use bool in structures

 drivers/staging/vc04_services/vchiq-mmal/mmal-common.h | 6 +++---
 drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c  | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.38.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 0/2] vc04_services: vchiq-mmal: Drop bool usage
@ 2022-11-17 12:59 ` Umang Jain
  0 siblings, 0 replies; 18+ messages in thread
From: Umang Jain @ 2022-11-17 12:59 UTC (permalink / raw)
  To: Florian Fainelli, Broadcom internal kernel review list,
	Greg Kroah-Hartman, Stefan Wahren, Hans Verkuil, Dave Stevenson,
	Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, linux-kernel
  Cc: kieran.bingham, Umang Jain

Simple fixes to drop bool usage from vchiq-mmal.
Individual patches contains details.

Dave Stevenson (1):
  staging: vc04_services: mmal-vchiq: Do not assign bool to u32

Umang Jain (1):
  staging: vc04_services: mmal-common: Do not use bool in structures

 drivers/staging/vc04_services/vchiq-mmal/mmal-common.h | 6 +++---
 drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c  | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.38.1


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/2] staging: vc04_services: mmal-vchiq: Do not assign bool to u32
  2022-11-17 12:59 ` Umang Jain
@ 2022-11-17 12:59   ` Umang Jain
  -1 siblings, 0 replies; 18+ messages in thread
From: Umang Jain @ 2022-11-17 12:59 UTC (permalink / raw)
  To: Florian Fainelli, Broadcom internal kernel review list,
	Greg Kroah-Hartman, Stefan Wahren, Hans Verkuil, Dave Stevenson,
	Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, linux-kernel
  Cc: kieran.bingham, Umang Jain

From: Dave Stevenson <dave.stevenson@raspberrypi.com>

struct vchiq_mmal_component.enabled is a u32 type. Do not assign
it a bool.

Fixes: 640e77466e69 ("staging: mmal-vchiq: Avoid use of bool in structures")
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
index cb921c94996a..17f8ceda87ca 100644
--- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
@@ -1773,7 +1773,7 @@ int vchiq_mmal_component_enable(struct vchiq_mmal_instance *instance,
 
 	ret = enable_component(instance, component);
 	if (ret == 0)
-		component->enabled = true;
+		component->enabled = 1;
 
 	mutex_unlock(&instance->vchiq_mutex);
 
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 1/2] staging: vc04_services: mmal-vchiq: Do not assign bool to u32
@ 2022-11-17 12:59   ` Umang Jain
  0 siblings, 0 replies; 18+ messages in thread
From: Umang Jain @ 2022-11-17 12:59 UTC (permalink / raw)
  To: Florian Fainelli, Broadcom internal kernel review list,
	Greg Kroah-Hartman, Stefan Wahren, Hans Verkuil, Dave Stevenson,
	Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, linux-kernel
  Cc: kieran.bingham, Umang Jain

From: Dave Stevenson <dave.stevenson@raspberrypi.com>

struct vchiq_mmal_component.enabled is a u32 type. Do not assign
it a bool.

Fixes: 640e77466e69 ("staging: mmal-vchiq: Avoid use of bool in structures")
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
index cb921c94996a..17f8ceda87ca 100644
--- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
@@ -1773,7 +1773,7 @@ int vchiq_mmal_component_enable(struct vchiq_mmal_instance *instance,
 
 	ret = enable_component(instance, component);
 	if (ret == 0)
-		component->enabled = true;
+		component->enabled = 1;
 
 	mutex_unlock(&instance->vchiq_mutex);
 
-- 
2.38.1


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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/2] staging: vc04_services: mmal-common: Do not use bool in structures
  2022-11-17 12:59 ` Umang Jain
@ 2022-11-17 12:59   ` Umang Jain
  -1 siblings, 0 replies; 18+ messages in thread
From: Umang Jain @ 2022-11-17 12:59 UTC (permalink / raw)
  To: Florian Fainelli, Broadcom internal kernel review list,
	Greg Kroah-Hartman, Stefan Wahren, Hans Verkuil, Dave Stevenson,
	Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, linux-kernel
  Cc: kieran.bingham, Umang Jain

Do not use bool in structures, it already gets flagged by checkpatch:

"Avoid using bool structure members because of possible alignment issues"

Hence, modify struct mmal_fmt.remove_padding to use u32. No change in
assignments as 0/1 are already being used with mmal_fmt.remove_padding.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 drivers/staging/vc04_services/vchiq-mmal/mmal-common.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-common.h b/drivers/staging/vc04_services/vchiq-mmal/mmal-common.h
index b33129403a30..fd02440f41b2 100644
--- a/drivers/staging/vc04_services/vchiq-mmal/mmal-common.h
+++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-common.h
@@ -32,9 +32,9 @@ struct mmal_fmt {
 	int depth;
 	u32 mmal_component;  /* MMAL component index to be used to encode */
 	u32 ybbp;            /* depth of first Y plane for planar formats */
-	bool remove_padding;   /* Does the GPU have to remove padding,
-				* or can we do hide padding via bytesperline.
-				*/
+	u32 remove_padding;  /* Does the GPU have to remove padding,
+			      * or can we do hide padding via bytesperline.
+			      */
 };
 
 /* buffer for one video frame */
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/2] staging: vc04_services: mmal-common: Do not use bool in structures
@ 2022-11-17 12:59   ` Umang Jain
  0 siblings, 0 replies; 18+ messages in thread
From: Umang Jain @ 2022-11-17 12:59 UTC (permalink / raw)
  To: Florian Fainelli, Broadcom internal kernel review list,
	Greg Kroah-Hartman, Stefan Wahren, Hans Verkuil, Dave Stevenson,
	Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, linux-kernel
  Cc: kieran.bingham, Umang Jain

Do not use bool in structures, it already gets flagged by checkpatch:

"Avoid using bool structure members because of possible alignment issues"

Hence, modify struct mmal_fmt.remove_padding to use u32. No change in
assignments as 0/1 are already being used with mmal_fmt.remove_padding.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 drivers/staging/vc04_services/vchiq-mmal/mmal-common.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-common.h b/drivers/staging/vc04_services/vchiq-mmal/mmal-common.h
index b33129403a30..fd02440f41b2 100644
--- a/drivers/staging/vc04_services/vchiq-mmal/mmal-common.h
+++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-common.h
@@ -32,9 +32,9 @@ struct mmal_fmt {
 	int depth;
 	u32 mmal_component;  /* MMAL component index to be used to encode */
 	u32 ybbp;            /* depth of first Y plane for planar formats */
-	bool remove_padding;   /* Does the GPU have to remove padding,
-				* or can we do hide padding via bytesperline.
-				*/
+	u32 remove_padding;  /* Does the GPU have to remove padding,
+			      * or can we do hide padding via bytesperline.
+			      */
 };
 
 /* buffer for one video frame */
-- 
2.38.1


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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] staging: vc04_services: mmal-common: Do not use bool in structures
  2022-11-17 12:59   ` Umang Jain
@ 2022-11-17 13:13     ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 18+ messages in thread
From: Greg Kroah-Hartman @ 2022-11-17 13:13 UTC (permalink / raw)
  To: Umang Jain
  Cc: Florian Fainelli, Broadcom internal kernel review list,
	Stefan Wahren, Hans Verkuil, Dave Stevenson,
	Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, linux-kernel, kieran.bingham

On Thu, Nov 17, 2022 at 06:29:53PM +0530, Umang Jain wrote:
> Do not use bool in structures, it already gets flagged by checkpatch:
> 
> "Avoid using bool structure members because of possible alignment issues"
> 
> Hence, modify struct mmal_fmt.remove_padding to use u32. No change in
> assignments as 0/1 are already being used with mmal_fmt.remove_padding.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  drivers/staging/vc04_services/vchiq-mmal/mmal-common.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-common.h b/drivers/staging/vc04_services/vchiq-mmal/mmal-common.h
> index b33129403a30..fd02440f41b2 100644
> --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-common.h
> +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-common.h
> @@ -32,9 +32,9 @@ struct mmal_fmt {
>  	int depth;
>  	u32 mmal_component;  /* MMAL component index to be used to encode */
>  	u32 ybbp;            /* depth of first Y plane for planar formats */
> -	bool remove_padding;   /* Does the GPU have to remove padding,
> -				* or can we do hide padding via bytesperline.
> -				*/
> +	u32 remove_padding;  /* Does the GPU have to remove padding,
> +			      * or can we do hide padding via bytesperline.
> +			      */

checkpatch is wrong here, bool is correct to use and is just fine.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] staging: vc04_services: mmal-common: Do not use bool in structures
@ 2022-11-17 13:13     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 18+ messages in thread
From: Greg Kroah-Hartman @ 2022-11-17 13:13 UTC (permalink / raw)
  To: Umang Jain
  Cc: Florian Fainelli, Broadcom internal kernel review list,
	Stefan Wahren, Hans Verkuil, Dave Stevenson,
	Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, linux-kernel, kieran.bingham

On Thu, Nov 17, 2022 at 06:29:53PM +0530, Umang Jain wrote:
> Do not use bool in structures, it already gets flagged by checkpatch:
> 
> "Avoid using bool structure members because of possible alignment issues"
> 
> Hence, modify struct mmal_fmt.remove_padding to use u32. No change in
> assignments as 0/1 are already being used with mmal_fmt.remove_padding.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  drivers/staging/vc04_services/vchiq-mmal/mmal-common.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-common.h b/drivers/staging/vc04_services/vchiq-mmal/mmal-common.h
> index b33129403a30..fd02440f41b2 100644
> --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-common.h
> +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-common.h
> @@ -32,9 +32,9 @@ struct mmal_fmt {
>  	int depth;
>  	u32 mmal_component;  /* MMAL component index to be used to encode */
>  	u32 ybbp;            /* depth of first Y plane for planar formats */
> -	bool remove_padding;   /* Does the GPU have to remove padding,
> -				* or can we do hide padding via bytesperline.
> -				*/
> +	u32 remove_padding;  /* Does the GPU have to remove padding,
> +			      * or can we do hide padding via bytesperline.
> +			      */

checkpatch is wrong here, bool is correct to use and is just fine.

thanks,

greg k-h

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] staging: vc04_services: mmal-vchiq: Do not assign bool to u32
  2022-11-17 12:59   ` Umang Jain
@ 2022-11-17 13:13     ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 18+ messages in thread
From: Greg Kroah-Hartman @ 2022-11-17 13:13 UTC (permalink / raw)
  To: Umang Jain
  Cc: Florian Fainelli, Broadcom internal kernel review list,
	Stefan Wahren, Hans Verkuil, Dave Stevenson,
	Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, linux-kernel, kieran.bingham

On Thu, Nov 17, 2022 at 06:29:52PM +0530, Umang Jain wrote:
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> 
> struct vchiq_mmal_component.enabled is a u32 type. Do not assign
> it a bool.
> 
> Fixes: 640e77466e69 ("staging: mmal-vchiq: Avoid use of bool in structures")
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> index cb921c94996a..17f8ceda87ca 100644
> --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> @@ -1773,7 +1773,7 @@ int vchiq_mmal_component_enable(struct vchiq_mmal_instance *instance,
>  
>  	ret = enable_component(instance, component);
>  	if (ret == 0)
> -		component->enabled = true;
> +		component->enabled = 1;

Why not make enabled a bool instead?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] staging: vc04_services: mmal-vchiq: Do not assign bool to u32
@ 2022-11-17 13:13     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 18+ messages in thread
From: Greg Kroah-Hartman @ 2022-11-17 13:13 UTC (permalink / raw)
  To: Umang Jain
  Cc: Florian Fainelli, Broadcom internal kernel review list,
	Stefan Wahren, Hans Verkuil, Dave Stevenson,
	Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, linux-kernel, kieran.bingham

On Thu, Nov 17, 2022 at 06:29:52PM +0530, Umang Jain wrote:
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> 
> struct vchiq_mmal_component.enabled is a u32 type. Do not assign
> it a bool.
> 
> Fixes: 640e77466e69 ("staging: mmal-vchiq: Avoid use of bool in structures")
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> index cb921c94996a..17f8ceda87ca 100644
> --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> @@ -1773,7 +1773,7 @@ int vchiq_mmal_component_enable(struct vchiq_mmal_instance *instance,
>  
>  	ret = enable_component(instance, component);
>  	if (ret == 0)
> -		component->enabled = true;
> +		component->enabled = 1;

Why not make enabled a bool instead?

thanks,

greg k-h

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] staging: vc04_services: mmal-vchiq: Do not assign bool to u32
  2022-11-17 13:13     ` Greg Kroah-Hartman
@ 2022-11-17 13:27       ` Umang Jain
  -1 siblings, 0 replies; 18+ messages in thread
From: Umang Jain @ 2022-11-17 13:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Florian Fainelli, Broadcom internal kernel review list,
	Stefan Wahren, Hans Verkuil, Dave Stevenson,
	Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, linux-kernel, kieran.bingham

Hi Greg,

Thanks for the comment,

On 11/17/22 6:43 PM, Greg Kroah-Hartman wrote:
> On Thu, Nov 17, 2022 at 06:29:52PM +0530, Umang Jain wrote:
>> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
>>
>> struct vchiq_mmal_component.enabled is a u32 type. Do not assign
>> it a bool.
>>
>> Fixes: 640e77466e69 ("staging: mmal-vchiq: Avoid use of bool in structures")
>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
>> ---
>>   drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
>> index cb921c94996a..17f8ceda87ca 100644
>> --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
>> +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
>> @@ -1773,7 +1773,7 @@ int vchiq_mmal_component_enable(struct vchiq_mmal_instance *instance,
>>   
>>   	ret = enable_component(instance, component);
>>   	if (ret == 0)
>> -		component->enabled = true;
>> +		component->enabled = 1;
> Why not make enabled a bool instead?

Makes sense. It would probably require reverting the 640e77466e69 
("staging: mmal-vchiq: Avoid use of bool in structures")

I'll also find other occurances in vchiq-mmal (if any). Also for other 
reviewers, I found the context at:

7967656ffbfa ("coding-style: Clarify the expectations around bool")

Thanks,

uajain

>
> thanks,
>
> greg k-h


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] staging: vc04_services: mmal-vchiq: Do not assign bool to u32
@ 2022-11-17 13:27       ` Umang Jain
  0 siblings, 0 replies; 18+ messages in thread
From: Umang Jain @ 2022-11-17 13:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Florian Fainelli, Broadcom internal kernel review list,
	Stefan Wahren, Hans Verkuil, Dave Stevenson,
	Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, linux-kernel, kieran.bingham

Hi Greg,

Thanks for the comment,

On 11/17/22 6:43 PM, Greg Kroah-Hartman wrote:
> On Thu, Nov 17, 2022 at 06:29:52PM +0530, Umang Jain wrote:
>> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
>>
>> struct vchiq_mmal_component.enabled is a u32 type. Do not assign
>> it a bool.
>>
>> Fixes: 640e77466e69 ("staging: mmal-vchiq: Avoid use of bool in structures")
>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
>> ---
>>   drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
>> index cb921c94996a..17f8ceda87ca 100644
>> --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
>> +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
>> @@ -1773,7 +1773,7 @@ int vchiq_mmal_component_enable(struct vchiq_mmal_instance *instance,
>>   
>>   	ret = enable_component(instance, component);
>>   	if (ret == 0)
>> -		component->enabled = true;
>> +		component->enabled = 1;
> Why not make enabled a bool instead?

Makes sense. It would probably require reverting the 640e77466e69 
("staging: mmal-vchiq: Avoid use of bool in structures")

I'll also find other occurances in vchiq-mmal (if any). Also for other 
reviewers, I found the context at:

7967656ffbfa ("coding-style: Clarify the expectations around bool")

Thanks,

uajain

>
> thanks,
>
> greg k-h


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] staging: vc04_services: mmal-vchiq: Do not assign bool to u32
  2022-11-17 12:59   ` Umang Jain
@ 2022-11-17 13:43     ` Dan Carpenter
  -1 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2022-11-17 13:43 UTC (permalink / raw)
  To: Umang Jain
  Cc: Florian Fainelli, Broadcom internal kernel review list,
	Greg Kroah-Hartman, Stefan Wahren, Hans Verkuil, Dave Stevenson,
	Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, linux-kernel, kieran.bingham

On Thu, Nov 17, 2022 at 06:29:52PM +0530, Umang Jain wrote:
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> 
> struct vchiq_mmal_component.enabled is a u32 type. Do not assign
> it a bool.

It's not a u32 type so this is wrong.

	u32 enabled:1;

But also "true" is better than "1" in terms of a human reading the code.

Perhaps this is from a static checker?  I am also the author of a checker
tool so I know how stupid they can be.  When the checker says something
dumb, then the correct response is to be be briefly amused and not to
slavishly obey it.

regards,
dan


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] staging: vc04_services: mmal-vchiq: Do not assign bool to u32
@ 2022-11-17 13:43     ` Dan Carpenter
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2022-11-17 13:43 UTC (permalink / raw)
  To: Umang Jain
  Cc: Florian Fainelli, Broadcom internal kernel review list,
	Greg Kroah-Hartman, Stefan Wahren, Hans Verkuil, Dave Stevenson,
	Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, linux-kernel, kieran.bingham

On Thu, Nov 17, 2022 at 06:29:52PM +0530, Umang Jain wrote:
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> 
> struct vchiq_mmal_component.enabled is a u32 type. Do not assign
> it a bool.

It's not a u32 type so this is wrong.

	u32 enabled:1;

But also "true" is better than "1" in terms of a human reading the code.

Perhaps this is from a static checker?  I am also the author of a checker
tool so I know how stupid they can be.  When the checker says something
dumb, then the correct response is to be be briefly amused and not to
slavishly obey it.

regards,
dan


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] staging: vc04_services: mmal-common: Do not use bool in structures
  2022-11-17 12:59   ` Umang Jain
@ 2022-11-17 13:48     ` Dan Carpenter
  -1 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2022-11-17 13:48 UTC (permalink / raw)
  To: Umang Jain
  Cc: Florian Fainelli, Broadcom internal kernel review list,
	Greg Kroah-Hartman, Stefan Wahren, Hans Verkuil, Dave Stevenson,
	Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, linux-kernel, kieran.bingham

On Thu, Nov 17, 2022 at 06:29:53PM +0530, Umang Jain wrote:
> Do not use bool in structures, it already gets flagged by checkpatch:
> 
> "Avoid using bool structure members because of possible alignment issues"
> 

This checkpatch warning was removed almost 4 years ago.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] staging: vc04_services: mmal-common: Do not use bool in structures
@ 2022-11-17 13:48     ` Dan Carpenter
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2022-11-17 13:48 UTC (permalink / raw)
  To: Umang Jain
  Cc: Florian Fainelli, Broadcom internal kernel review list,
	Greg Kroah-Hartman, Stefan Wahren, Hans Verkuil, Dave Stevenson,
	Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, linux-kernel, kieran.bingham

On Thu, Nov 17, 2022 at 06:29:53PM +0530, Umang Jain wrote:
> Do not use bool in structures, it already gets flagged by checkpatch:
> 
> "Avoid using bool structure members because of possible alignment issues"
> 

This checkpatch warning was removed almost 4 years ago.

regards,
dan carpenter

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] staging: vc04_services: mmal-vchiq: Do not assign bool to u32
  2022-11-17 13:27       ` Umang Jain
@ 2022-11-17 13:50         ` Dan Carpenter
  -1 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2022-11-17 13:50 UTC (permalink / raw)
  To: Umang Jain
  Cc: Greg Kroah-Hartman, Florian Fainelli,
	Broadcom internal kernel review list, Stefan Wahren,
	Hans Verkuil, Dave Stevenson, Mauro Carvalho Chehab,
	linux-rpi-kernel, linux-arm-kernel, linux-staging, linux-kernel,
	kieran.bingham

On Thu, Nov 17, 2022 at 06:57:07PM +0530, Umang Jain wrote:
> Hi Greg,
> 
> Thanks for the comment,
> 
> On 11/17/22 6:43 PM, Greg Kroah-Hartman wrote:
> > On Thu, Nov 17, 2022 at 06:29:52PM +0530, Umang Jain wrote:
> > > From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > 
> > > struct vchiq_mmal_component.enabled is a u32 type. Do not assign
> > > it a bool.
> > > 
> > > Fixes: 640e77466e69 ("staging: mmal-vchiq: Avoid use of bool in structures")
> > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> > > ---
> > >   drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > > index cb921c94996a..17f8ceda87ca 100644
> > > --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > > +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > > @@ -1773,7 +1773,7 @@ int vchiq_mmal_component_enable(struct vchiq_mmal_instance *instance,
> > >   	ret = enable_component(instance, component);
> > >   	if (ret == 0)
> > > -		component->enabled = true;
> > > +		component->enabled = 1;
> > Why not make enabled a bool instead?
> 
> Makes sense. It would probably require reverting the 640e77466e69 ("staging:
> mmal-vchiq: Avoid use of bool in structures")
> 

Reverting that patch seems like a good idea.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] staging: vc04_services: mmal-vchiq: Do not assign bool to u32
@ 2022-11-17 13:50         ` Dan Carpenter
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2022-11-17 13:50 UTC (permalink / raw)
  To: Umang Jain
  Cc: Greg Kroah-Hartman, Florian Fainelli,
	Broadcom internal kernel review list, Stefan Wahren,
	Hans Verkuil, Dave Stevenson, Mauro Carvalho Chehab,
	linux-rpi-kernel, linux-arm-kernel, linux-staging, linux-kernel,
	kieran.bingham

On Thu, Nov 17, 2022 at 06:57:07PM +0530, Umang Jain wrote:
> Hi Greg,
> 
> Thanks for the comment,
> 
> On 11/17/22 6:43 PM, Greg Kroah-Hartman wrote:
> > On Thu, Nov 17, 2022 at 06:29:52PM +0530, Umang Jain wrote:
> > > From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > 
> > > struct vchiq_mmal_component.enabled is a u32 type. Do not assign
> > > it a bool.
> > > 
> > > Fixes: 640e77466e69 ("staging: mmal-vchiq: Avoid use of bool in structures")
> > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> > > ---
> > >   drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > > index cb921c94996a..17f8ceda87ca 100644
> > > --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > > +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > > @@ -1773,7 +1773,7 @@ int vchiq_mmal_component_enable(struct vchiq_mmal_instance *instance,
> > >   	ret = enable_component(instance, component);
> > >   	if (ret == 0)
> > > -		component->enabled = true;
> > > +		component->enabled = 1;
> > Why not make enabled a bool instead?
> 
> Makes sense. It would probably require reverting the 640e77466e69 ("staging:
> mmal-vchiq: Avoid use of bool in structures")
> 

Reverting that patch seems like a good idea.

regards,
dan carpenter


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-11-17 13:51 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-17 12:59 [PATCH 0/2] vc04_services: vchiq-mmal: Drop bool usage Umang Jain
2022-11-17 12:59 ` Umang Jain
2022-11-17 12:59 ` [PATCH 1/2] staging: vc04_services: mmal-vchiq: Do not assign bool to u32 Umang Jain
2022-11-17 12:59   ` Umang Jain
2022-11-17 13:13   ` Greg Kroah-Hartman
2022-11-17 13:13     ` Greg Kroah-Hartman
2022-11-17 13:27     ` Umang Jain
2022-11-17 13:27       ` Umang Jain
2022-11-17 13:50       ` Dan Carpenter
2022-11-17 13:50         ` Dan Carpenter
2022-11-17 13:43   ` Dan Carpenter
2022-11-17 13:43     ` Dan Carpenter
2022-11-17 12:59 ` [PATCH 2/2] staging: vc04_services: mmal-common: Do not use bool in structures Umang Jain
2022-11-17 12:59   ` Umang Jain
2022-11-17 13:13   ` Greg Kroah-Hartman
2022-11-17 13:13     ` Greg Kroah-Hartman
2022-11-17 13:48   ` Dan Carpenter
2022-11-17 13:48     ` Dan Carpenter

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.