* [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.