* [PATCH 0/3] vc04_services: Promote bool usage
@ 2022-11-17 16:00 Umang Jain
2022-11-17 16:00 ` [PATCH 1/3] Revert "staging: mmal-vchiq: Avoid use of bool in structures" Umang Jain
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Umang Jain @ 2022-11-17 16:00 UTC (permalink / raw)
To: Florian Fainelli, Broadcom internal kernel review list, Ray Jui,
Dan Carpenter, Greg Kroah-Hartman, Hans Verkuil, Dave Stevenson,
Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel,
linux-staging, linux-kernel
Cc: kieran.bingham, Umang Jain
In commit 7967656ffbfa ("coding-style: Clarify the expectations around
bool") the check to dis-allow bool structure members was removed from
checkpatch.pl. It promotes bool structure members to store boolean
values. This enhances code readability.
Umang Jain (3):
Revert "staging: mmal-vchiq: Avoid use of bool in structures"
vc04_services: mmal-vchiq: Use bool for vchiq_mmal_component.in_use
vc04_services: bcm2835-camera: Use bool values for
mmal_fmt.remove_padding
.../bcm2835-camera/bcm2835-camera.c | 30 +++++++++----------
.../vc04_services/vchiq-mmal/mmal-vchiq.c | 18 +++++------
.../vc04_services/vchiq-mmal/mmal-vchiq.h | 6 ++--
3 files changed, 27 insertions(+), 27 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] 16+ messages in thread* [PATCH 1/3] Revert "staging: mmal-vchiq: Avoid use of bool in structures" 2022-11-17 16:00 [PATCH 0/3] vc04_services: Promote bool usage Umang Jain @ 2022-11-17 16:00 ` Umang Jain 2022-11-17 16:09 ` Kieran Bingham 2022-11-17 16:00 ` [PATCH 2/3] vc04_services: mmal-vchiq: Use bool for vchiq_mmal_component.in_use Umang Jain ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: Umang Jain @ 2022-11-17 16:00 UTC (permalink / raw) To: Florian Fainelli, Broadcom internal kernel review list, Ray Jui, Dan Carpenter, Greg Kroah-Hartman, Hans Verkuil, Dave Stevenson, Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel, linux-staging, linux-kernel Cc: kieran.bingham, Umang Jain This reverts commit 640e77466e69d9c28de227bc76881f5501f532ca. In commit 7967656ffbfa ("coding-style: Clarify the expectations around bool") the check to dis-allow bool structure members was removed from checkpatch.pl. It promotes bool structure members to store boolean values. This enhances code readability. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- .../staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 12 ++++++------ .../staging/vc04_services/vchiq-mmal/mmal-vchiq.h | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c index cb921c94996a..4abb6178cb9f 100644 --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c @@ -863,9 +863,9 @@ static int port_info_get(struct vchiq_mmal_instance *instance, goto release_msg; if (rmsg->u.port_info_get_reply.port.is_enabled == 0) - port->enabled = 0; + port->enabled = false; else - port->enabled = 1; + port->enabled = true; /* copy the values out of the message */ port->handle = rmsg->u.port_info_get_reply.port_handle; @@ -1304,7 +1304,7 @@ static int port_disable(struct vchiq_mmal_instance *instance, if (!port->enabled) return 0; - port->enabled = 0; + port->enabled = false; ret = port_action_port(instance, port, MMAL_MSG_PORT_ACTION_TYPE_DISABLE); @@ -1359,7 +1359,7 @@ static int port_enable(struct vchiq_mmal_instance *instance, if (ret) goto done; - port->enabled = 1; + port->enabled = true; if (port->buffer_cb) { /* send buffer headers to videocore */ @@ -1531,7 +1531,7 @@ int vchiq_mmal_port_connect_tunnel(struct vchiq_mmal_instance *instance, pr_err("failed disconnecting src port\n"); goto release_unlock; } - src->connected->enabled = 0; + src->connected->enabled = false; src->connected = NULL; } @@ -1799,7 +1799,7 @@ int vchiq_mmal_component_disable(struct vchiq_mmal_instance *instance, ret = disable_component(instance, component); if (ret == 0) - component->enabled = 0; + component->enabled = false; mutex_unlock(&instance->vchiq_mutex); diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h index 6006e29232b3..70eda6cac1c6 100644 --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h @@ -48,7 +48,7 @@ typedef void (*vchiq_mmal_buffer_cb)( int status, struct mmal_buffer *buffer); struct vchiq_mmal_port { - u32 enabled:1; + bool enabled:1; u32 handle; u32 type; /* port type, cached to use on port info set */ u32 index; /* port index, cached to use on port info set */ @@ -83,7 +83,7 @@ struct vchiq_mmal_port { struct vchiq_mmal_component { u32 in_use:1; - u32 enabled:1; + bool enabled:1; u32 handle; /* VideoCore handle for component */ u32 inputs; /* Number of input ports */ u32 outputs; /* Number of output ports */ -- 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] 16+ messages in thread
* Re: [PATCH 1/3] Revert "staging: mmal-vchiq: Avoid use of bool in structures" 2022-11-17 16:00 ` [PATCH 1/3] Revert "staging: mmal-vchiq: Avoid use of bool in structures" Umang Jain @ 2022-11-17 16:09 ` Kieran Bingham 2022-11-17 17:55 ` Umang Jain 0 siblings, 1 reply; 16+ messages in thread From: Kieran Bingham @ 2022-11-17 16:09 UTC (permalink / raw) To: Broadcom internal kernel review list, Dan Carpenter, Dave Stevenson, Florian Fainelli, Greg Kroah-Hartman, Hans Verkuil, Mauro Carvalho Chehab, Ray Jui, Umang Jain, linux-arm-kernel, linux-kernel, linux-rpi-kernel, linux-staging Cc: Umang Jain Quoting Umang Jain (2022-11-17 16:00:13) > This reverts commit 640e77466e69d9c28de227bc76881f5501f532ca. > > In commit 7967656ffbfa ("coding-style: Clarify the expectations around > bool") the check to dis-allow bool structure members was removed from > checkpatch.pl. It promotes bool structure members to store boolean > values. This enhances code readability. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > .../staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 12 ++++++------ > .../staging/vc04_services/vchiq-mmal/mmal-vchiq.h | 4 ++-- > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c > index cb921c94996a..4abb6178cb9f 100644 > --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c > +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c > @@ -863,9 +863,9 @@ static int port_info_get(struct vchiq_mmal_instance *instance, > goto release_msg; > > if (rmsg->u.port_info_get_reply.port.is_enabled == 0) > - port->enabled = 0; > + port->enabled = false; > else > - port->enabled = 1; > + port->enabled = true; > > /* copy the values out of the message */ > port->handle = rmsg->u.port_info_get_reply.port_handle; > @@ -1304,7 +1304,7 @@ static int port_disable(struct vchiq_mmal_instance *instance, > if (!port->enabled) > return 0; > > - port->enabled = 0; > + port->enabled = false; > > ret = port_action_port(instance, port, > MMAL_MSG_PORT_ACTION_TYPE_DISABLE); > @@ -1359,7 +1359,7 @@ static int port_enable(struct vchiq_mmal_instance *instance, > if (ret) > goto done; > > - port->enabled = 1; > + port->enabled = true; > > if (port->buffer_cb) { > /* send buffer headers to videocore */ > @@ -1531,7 +1531,7 @@ int vchiq_mmal_port_connect_tunnel(struct vchiq_mmal_instance *instance, > pr_err("failed disconnecting src port\n"); > goto release_unlock; > } > - src->connected->enabled = 0; > + src->connected->enabled = false; > src->connected = NULL; > } > > @@ -1799,7 +1799,7 @@ int vchiq_mmal_component_disable(struct vchiq_mmal_instance *instance, > > ret = disable_component(instance, component); > if (ret == 0) > - component->enabled = 0; > + component->enabled = false; > > mutex_unlock(&instance->vchiq_mutex); > > diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h > index 6006e29232b3..70eda6cac1c6 100644 > --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h > +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h > @@ -48,7 +48,7 @@ typedef void (*vchiq_mmal_buffer_cb)( > int status, struct mmal_buffer *buffer); > > struct vchiq_mmal_port { > - u32 enabled:1; > + bool enabled:1; Is this a direct revert with 'git revert' ? I would expect this to be bool enabled; > u32 handle; > u32 type; /* port type, cached to use on port info set */ > u32 index; /* port index, cached to use on port info set */ > @@ -83,7 +83,7 @@ struct vchiq_mmal_port { > > struct vchiq_mmal_component { > u32 in_use:1; > - u32 enabled:1; > + bool enabled:1; Same here of course. > u32 handle; /* VideoCore handle for component */ > u32 inputs; /* Number of input ports */ > u32 outputs; /* Number of output ports */ > -- > 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] 16+ messages in thread
* Re: [PATCH 1/3] Revert "staging: mmal-vchiq: Avoid use of bool in structures" 2022-11-17 16:09 ` Kieran Bingham @ 2022-11-17 17:55 ` Umang Jain 2022-11-17 18:02 ` Greg Kroah-Hartman 0 siblings, 1 reply; 16+ messages in thread From: Umang Jain @ 2022-11-17 17:55 UTC (permalink / raw) To: Kieran Bingham, Broadcom internal kernel review list, Dan Carpenter, Dave Stevenson, Florian Fainelli, Greg Kroah-Hartman, Hans Verkuil, Mauro Carvalho Chehab, Ray Jui, linux-arm-kernel, linux-kernel, linux-rpi-kernel, linux-staging Hi Kieran, On 11/17/22 9:39 PM, Kieran Bingham wrote: > Quoting Umang Jain (2022-11-17 16:00:13) >> This reverts commit 640e77466e69d9c28de227bc76881f5501f532ca. >> >> In commit 7967656ffbfa ("coding-style: Clarify the expectations around >> bool") the check to dis-allow bool structure members was removed from >> checkpatch.pl. It promotes bool structure members to store boolean >> values. This enhances code readability. >> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >> --- >> .../staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 12 ++++++------ >> .../staging/vc04_services/vchiq-mmal/mmal-vchiq.h | 4 ++-- >> 2 files changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c >> index cb921c94996a..4abb6178cb9f 100644 >> --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c >> +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c >> @@ -863,9 +863,9 @@ static int port_info_get(struct vchiq_mmal_instance *instance, >> goto release_msg; >> >> if (rmsg->u.port_info_get_reply.port.is_enabled == 0) >> - port->enabled = 0; >> + port->enabled = false; >> else >> - port->enabled = 1; >> + port->enabled = true; >> >> /* copy the values out of the message */ >> port->handle = rmsg->u.port_info_get_reply.port_handle; >> @@ -1304,7 +1304,7 @@ static int port_disable(struct vchiq_mmal_instance *instance, >> if (!port->enabled) >> return 0; >> >> - port->enabled = 0; >> + port->enabled = false; >> >> ret = port_action_port(instance, port, >> MMAL_MSG_PORT_ACTION_TYPE_DISABLE); >> @@ -1359,7 +1359,7 @@ static int port_enable(struct vchiq_mmal_instance *instance, >> if (ret) >> goto done; >> >> - port->enabled = 1; >> + port->enabled = true; >> >> if (port->buffer_cb) { >> /* send buffer headers to videocore */ >> @@ -1531,7 +1531,7 @@ int vchiq_mmal_port_connect_tunnel(struct vchiq_mmal_instance *instance, >> pr_err("failed disconnecting src port\n"); >> goto release_unlock; >> } >> - src->connected->enabled = 0; >> + src->connected->enabled = false; >> src->connected = NULL; >> } >> >> @@ -1799,7 +1799,7 @@ int vchiq_mmal_component_disable(struct vchiq_mmal_instance *instance, >> >> ret = disable_component(instance, component); >> if (ret == 0) >> - component->enabled = 0; >> + component->enabled = false; >> >> mutex_unlock(&instance->vchiq_mutex); >> >> diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h >> index 6006e29232b3..70eda6cac1c6 100644 >> --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h >> +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h >> @@ -48,7 +48,7 @@ typedef void (*vchiq_mmal_buffer_cb)( >> int status, struct mmal_buffer *buffer); >> >> struct vchiq_mmal_port { >> - u32 enabled:1; >> + bool enabled:1; > Is this a direct revert with 'git revert' ? No. It had conflicts plus I added the ':1' initialization to keep the logic same (in case 'enabled' gets used directly). Similar pattern come up with: ($) git grep 'bool' -- '*.[h]' | grep '\:1' So it shouldn't be an issue. > > I would expect this to be > bool enabled; True but it won't functionally not be the same in matter of initialization. Should the initialization be split to separate patch? > > >> u32 handle; >> u32 type; /* port type, cached to use on port info set */ >> u32 index; /* port index, cached to use on port info set */ >> @@ -83,7 +83,7 @@ struct vchiq_mmal_port { >> >> struct vchiq_mmal_component { >> u32 in_use:1; >> - u32 enabled:1; >> + bool enabled:1; > Same here of course. > >> u32 handle; /* VideoCore handle for component */ >> u32 inputs; /* Number of input ports */ >> u32 outputs; /* Number of output ports */ >> -- >> 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] 16+ messages in thread
* Re: [PATCH 1/3] Revert "staging: mmal-vchiq: Avoid use of bool in structures" 2022-11-17 17:55 ` Umang Jain @ 2022-11-17 18:02 ` Greg Kroah-Hartman 2022-11-17 18:22 ` Umang Jain 0 siblings, 1 reply; 16+ messages in thread From: Greg Kroah-Hartman @ 2022-11-17 18:02 UTC (permalink / raw) To: Umang Jain Cc: Kieran Bingham, Broadcom internal kernel review list, Dan Carpenter, Dave Stevenson, Florian Fainelli, Hans Verkuil, Mauro Carvalho Chehab, Ray Jui, linux-arm-kernel, linux-kernel, linux-rpi-kernel, linux-staging On Thu, Nov 17, 2022 at 11:25:48PM +0530, Umang Jain wrote: > Hi Kieran, > > On 11/17/22 9:39 PM, Kieran Bingham wrote: > > Quoting Umang Jain (2022-11-17 16:00:13) > > > This reverts commit 640e77466e69d9c28de227bc76881f5501f532ca. > > > > > > In commit 7967656ffbfa ("coding-style: Clarify the expectations around > > > bool") the check to dis-allow bool structure members was removed from > > > checkpatch.pl. It promotes bool structure members to store boolean > > > values. This enhances code readability. > > > > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > > > --- > > > .../staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 12 ++++++------ > > > .../staging/vc04_services/vchiq-mmal/mmal-vchiq.h | 4 ++-- > > > 2 files changed, 8 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c > > > index cb921c94996a..4abb6178cb9f 100644 > > > --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c > > > +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c > > > @@ -863,9 +863,9 @@ static int port_info_get(struct vchiq_mmal_instance *instance, > > > goto release_msg; > > > if (rmsg->u.port_info_get_reply.port.is_enabled == 0) > > > - port->enabled = 0; > > > + port->enabled = false; > > > else > > > - port->enabled = 1; > > > + port->enabled = true; > > > /* copy the values out of the message */ > > > port->handle = rmsg->u.port_info_get_reply.port_handle; > > > @@ -1304,7 +1304,7 @@ static int port_disable(struct vchiq_mmal_instance *instance, > > > if (!port->enabled) > > > return 0; > > > - port->enabled = 0; > > > + port->enabled = false; > > > ret = port_action_port(instance, port, > > > MMAL_MSG_PORT_ACTION_TYPE_DISABLE); > > > @@ -1359,7 +1359,7 @@ static int port_enable(struct vchiq_mmal_instance *instance, > > > if (ret) > > > goto done; > > > - port->enabled = 1; > > > + port->enabled = true; > > > if (port->buffer_cb) { > > > /* send buffer headers to videocore */ > > > @@ -1531,7 +1531,7 @@ int vchiq_mmal_port_connect_tunnel(struct vchiq_mmal_instance *instance, > > > pr_err("failed disconnecting src port\n"); > > > goto release_unlock; > > > } > > > - src->connected->enabled = 0; > > > + src->connected->enabled = false; > > > src->connected = NULL; > > > } > > > @@ -1799,7 +1799,7 @@ int vchiq_mmal_component_disable(struct vchiq_mmal_instance *instance, > > > ret = disable_component(instance, component); > > > if (ret == 0) > > > - component->enabled = 0; > > > + component->enabled = false; > > > mutex_unlock(&instance->vchiq_mutex); > > > diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h > > > index 6006e29232b3..70eda6cac1c6 100644 > > > --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h > > > +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h > > > @@ -48,7 +48,7 @@ typedef void (*vchiq_mmal_buffer_cb)( > > > int status, struct mmal_buffer *buffer); > > > struct vchiq_mmal_port { > > > - u32 enabled:1; > > > + bool enabled:1; > > Is this a direct revert with 'git revert' ? > > > No. It had conflicts plus I added the ':1' initialization to keep the logic > same (in case 'enabled' gets used directly). Similar pattern come up with: > ($) git grep 'bool' -- '*.[h]' | grep '\:1' > > So it shouldn't be an issue. Please don't do that "bool foo:1" makes no sense. Drop the ":1" please. 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] 16+ messages in thread
* Re: [PATCH 1/3] Revert "staging: mmal-vchiq: Avoid use of bool in structures" 2022-11-17 18:02 ` Greg Kroah-Hartman @ 2022-11-17 18:22 ` Umang Jain 2022-11-18 0:12 ` Andrew Lunn 0 siblings, 1 reply; 16+ messages in thread From: Umang Jain @ 2022-11-17 18:22 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Kieran Bingham, Broadcom internal kernel review list, Dan Carpenter, Dave Stevenson, Florian Fainelli, Hans Verkuil, Mauro Carvalho Chehab, Ray Jui, linux-arm-kernel, linux-kernel, linux-rpi-kernel, linux-staging Hi Greg, On 11/17/22 11:32 PM, Greg Kroah-Hartman wrote: > On Thu, Nov 17, 2022 at 11:25:48PM +0530, Umang Jain wrote: >> Hi Kieran, >> >> On 11/17/22 9:39 PM, Kieran Bingham wrote: >>> Quoting Umang Jain (2022-11-17 16:00:13) >>>> This reverts commit 640e77466e69d9c28de227bc76881f5501f532ca. >>>> >>>> In commit 7967656ffbfa ("coding-style: Clarify the expectations around >>>> bool") the check to dis-allow bool structure members was removed from >>>> checkpatch.pl. It promotes bool structure members to store boolean >>>> values. This enhances code readability. >>>> >>>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >>>> --- >>>> .../staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 12 ++++++------ >>>> .../staging/vc04_services/vchiq-mmal/mmal-vchiq.h | 4 ++-- >>>> 2 files changed, 8 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c >>>> index cb921c94996a..4abb6178cb9f 100644 >>>> --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c >>>> +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c >>>> @@ -863,9 +863,9 @@ static int port_info_get(struct vchiq_mmal_instance *instance, >>>> goto release_msg; >>>> if (rmsg->u.port_info_get_reply.port.is_enabled == 0) >>>> - port->enabled = 0; >>>> + port->enabled = false; >>>> else >>>> - port->enabled = 1; >>>> + port->enabled = true; >>>> /* copy the values out of the message */ >>>> port->handle = rmsg->u.port_info_get_reply.port_handle; >>>> @@ -1304,7 +1304,7 @@ static int port_disable(struct vchiq_mmal_instance *instance, >>>> if (!port->enabled) >>>> return 0; >>>> - port->enabled = 0; >>>> + port->enabled = false; >>>> ret = port_action_port(instance, port, >>>> MMAL_MSG_PORT_ACTION_TYPE_DISABLE); >>>> @@ -1359,7 +1359,7 @@ static int port_enable(struct vchiq_mmal_instance *instance, >>>> if (ret) >>>> goto done; >>>> - port->enabled = 1; >>>> + port->enabled = true; >>>> if (port->buffer_cb) { >>>> /* send buffer headers to videocore */ >>>> @@ -1531,7 +1531,7 @@ int vchiq_mmal_port_connect_tunnel(struct vchiq_mmal_instance *instance, >>>> pr_err("failed disconnecting src port\n"); >>>> goto release_unlock; >>>> } >>>> - src->connected->enabled = 0; >>>> + src->connected->enabled = false; >>>> src->connected = NULL; >>>> } >>>> @@ -1799,7 +1799,7 @@ int vchiq_mmal_component_disable(struct vchiq_mmal_instance *instance, >>>> ret = disable_component(instance, component); >>>> if (ret == 0) >>>> - component->enabled = 0; >>>> + component->enabled = false; >>>> mutex_unlock(&instance->vchiq_mutex); >>>> diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h >>>> index 6006e29232b3..70eda6cac1c6 100644 >>>> --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h >>>> +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h >>>> @@ -48,7 +48,7 @@ typedef void (*vchiq_mmal_buffer_cb)( >>>> int status, struct mmal_buffer *buffer); >>>> struct vchiq_mmal_port { >>>> - u32 enabled:1; >>>> + bool enabled:1; >>> Is this a direct revert with 'git revert' ? >> >> No. It had conflicts plus I added the ':1' initialization to keep the logic >> same (in case 'enabled' gets used directly). Similar pattern come up with: >> ($) git grep 'bool' -- '*.[h]' | grep '\:1' >> >> So it shouldn't be an issue. > Please don't do that "bool foo:1" makes no sense. Drop the ":1" > please. It won't affect this patch but if you take a look at 2/3 - you'll see a bool flag 'in_use' that needs to be initialized (as it's getting used directly). I can move the initialization part in the function (_init() or something) and drop the ":1" as you mentioned. That's fine as well but I do find patterns of 'bool foo:1' in the codebase so I assumed it would be safe to use. > > > 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] 16+ messages in thread
* Re: [PATCH 1/3] Revert "staging: mmal-vchiq: Avoid use of bool in structures" 2022-11-17 18:22 ` Umang Jain @ 2022-11-18 0:12 ` Andrew Lunn 2022-11-18 3:58 ` Umang Jain 2022-11-18 8:35 ` Dan Carpenter 0 siblings, 2 replies; 16+ messages in thread From: Andrew Lunn @ 2022-11-18 0:12 UTC (permalink / raw) To: Umang Jain Cc: Greg Kroah-Hartman, Kieran Bingham, Broadcom internal kernel review list, Dan Carpenter, Dave Stevenson, Florian Fainelli, Hans Verkuil, Mauro Carvalho Chehab, Ray Jui, linux-arm-kernel, linux-kernel, linux-rpi-kernel, linux-staging > > > > > struct vchiq_mmal_port { > > > > > - u32 enabled:1; > > > > > + bool enabled:1; > > > > Is this a direct revert with 'git revert' ? > > > > > > No. It had conflicts plus I added the ':1' initialization to keep the logic > > > same (in case 'enabled' gets used directly). Similar pattern come up with: > > > ($) git grep 'bool' -- '*.[h]' | grep '\:1' > > > > > > So it shouldn't be an issue. > > Please don't do that "bool foo:1" makes no sense. Drop the ":1" > > please. > > It won't affect this patch but if you take a look at 2/3 - you'll see a bool > flag 'in_use' that needs to be initialized (as it's getting used directly). > > I can move the initialization part in the function (_init() or something) > and drop the ":1" as you mentioned. That's fine as well but I do find > patterns of 'bool foo:1' in the codebase so I assumed it would be safe to > use. Does :1 really initialise the variable? In "u32 enabled:1" it means this is a 1 bit wide bit field. It seems odd that bool is somehow special and :1 means something else. Andrew _______________________________________________ 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] 16+ messages in thread
* Re: [PATCH 1/3] Revert "staging: mmal-vchiq: Avoid use of bool in structures" 2022-11-18 0:12 ` Andrew Lunn @ 2022-11-18 3:58 ` Umang Jain 2022-11-18 8:35 ` Dan Carpenter 1 sibling, 0 replies; 16+ messages in thread From: Umang Jain @ 2022-11-18 3:58 UTC (permalink / raw) To: Andrew Lunn Cc: Greg Kroah-Hartman, Kieran Bingham, Broadcom internal kernel review list, Dan Carpenter, Dave Stevenson, Florian Fainelli, Hans Verkuil, Mauro Carvalho Chehab, Ray Jui, linux-arm-kernel, linux-kernel, linux-rpi-kernel, linux-staging Hi, On 11/18/22 5:42 AM, Andrew Lunn wrote: >>>>>> struct vchiq_mmal_port { >>>>>> - u32 enabled:1; >>>>>> + bool enabled:1; >>>>> Is this a direct revert with 'git revert' ? >>>> No. It had conflicts plus I added the ':1' initialization to keep the logic >>>> same (in case 'enabled' gets used directly). Similar pattern come up with: >>>> ($) git grep 'bool' -- '*.[h]' | grep '\:1' >>>> >>>> So it shouldn't be an issue. >>> Please don't do that "bool foo:1" makes no sense. Drop the ":1" >>> please. >> It won't affect this patch but if you take a look at 2/3 - you'll see a bool >> flag 'in_use' that needs to be initialized (as it's getting used directly). >> >> I can move the initialization part in the function (_init() or something) >> and drop the ":1" as you mentioned. That's fine as well but I do find >> patterns of 'bool foo:1' in the codebase so I assumed it would be safe to >> use. > Does :1 really initialise the variable? In "u32 enabled:1" it means > this is a 1 bit wide bit field. It seems odd that bool is somehow > special and :1 means something else. Yup you are correct - seems I mis-read :1 as initialization > > Andrew _______________________________________________ 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] 16+ messages in thread
* Re: [PATCH 1/3] Revert "staging: mmal-vchiq: Avoid use of bool in structures" 2022-11-18 0:12 ` Andrew Lunn 2022-11-18 3:58 ` Umang Jain @ 2022-11-18 8:35 ` Dan Carpenter 1 sibling, 0 replies; 16+ messages in thread From: Dan Carpenter @ 2022-11-18 8:35 UTC (permalink / raw) To: Andrew Lunn Cc: Umang Jain, Greg Kroah-Hartman, Kieran Bingham, Broadcom internal kernel review list, Dave Stevenson, Florian Fainelli, Hans Verkuil, Mauro Carvalho Chehab, Ray Jui, linux-arm-kernel, linux-kernel, linux-rpi-kernel, linux-staging On Fri, Nov 18, 2022 at 01:12:32AM +0100, Andrew Lunn wrote: > > Does :1 really initialise the variable? Obviously not. > In "u32 enabled:1" it means > this is a 1 bit wide bit field. It seems odd that bool is somehow > special and :1 means something else. If you have a bunch of consecutive bool a:1; bool b:1; then GCC will squeeze them into the same byte. But if you have bool a; bool b; then they each take a byte with GCC. I have specified GCC because the size of bool types are a bit vague in the C standard. 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] 16+ messages in thread
* [PATCH 2/3] vc04_services: mmal-vchiq: Use bool for vchiq_mmal_component.in_use 2022-11-17 16:00 [PATCH 0/3] vc04_services: Promote bool usage Umang Jain 2022-11-17 16:00 ` [PATCH 1/3] Revert "staging: mmal-vchiq: Avoid use of bool in structures" Umang Jain @ 2022-11-17 16:00 ` Umang Jain 2022-11-18 0:23 ` Andrew Lunn 2022-11-17 16:00 ` [PATCH 3/3] vc04_services: bcm2835-camera: Use bool values for mmal_fmt.remove_padding Umang Jain 2022-11-17 18:11 ` [PATCH 0/3] vc04_services: Promote bool usage Stefan Wahren 3 siblings, 1 reply; 16+ messages in thread From: Umang Jain @ 2022-11-17 16:00 UTC (permalink / raw) To: Florian Fainelli, Broadcom internal kernel review list, Ray Jui, Dan Carpenter, Greg Kroah-Hartman, Hans Verkuil, Dave Stevenson, Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel, linux-staging, linux-kernel Cc: kieran.bingham, Umang Jain In commit 7967656ffbfa ("coding-style: Clarify the expectations around bool") the check to dis-allow bool structure members was removed from checkpatch.pl. It promotes bool structure members to store boolean values. This enhances code readability. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 6 +++--- drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c index 4abb6178cb9f..294b184d4a49 100644 --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c @@ -1648,7 +1648,7 @@ int vchiq_mmal_component_init(struct vchiq_mmal_instance *instance, for (idx = 0; idx < VCHIQ_MMAL_MAX_COMPONENTS; idx++) { if (!instance->component[idx].in_use) { component = &instance->component[idx]; - component->in_use = 1; + component->in_use = true; break; } } @@ -1724,7 +1724,7 @@ int vchiq_mmal_component_init(struct vchiq_mmal_instance *instance, destroy_component(instance, component); unlock: if (component) - component->in_use = 0; + component->in_use = false; mutex_unlock(&instance->vchiq_mutex); return ret; @@ -1747,7 +1747,7 @@ int vchiq_mmal_component_finalise(struct vchiq_mmal_instance *instance, ret = destroy_component(instance, component); - component->in_use = 0; + component->in_use = false; mutex_unlock(&instance->vchiq_mutex); diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h index 70eda6cac1c6..c5be86b0479d 100644 --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h @@ -82,7 +82,7 @@ struct vchiq_mmal_port { }; struct vchiq_mmal_component { - u32 in_use:1; + bool in_use:1; bool enabled:1; u32 handle; /* VideoCore handle for component */ u32 inputs; /* Number of input ports */ -- 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] 16+ messages in thread
* Re: [PATCH 2/3] vc04_services: mmal-vchiq: Use bool for vchiq_mmal_component.in_use 2022-11-17 16:00 ` [PATCH 2/3] vc04_services: mmal-vchiq: Use bool for vchiq_mmal_component.in_use Umang Jain @ 2022-11-18 0:23 ` Andrew Lunn 2022-11-18 9:01 ` Dan Carpenter 0 siblings, 1 reply; 16+ messages in thread From: Andrew Lunn @ 2022-11-18 0:23 UTC (permalink / raw) To: Umang Jain Cc: Florian Fainelli, Broadcom internal kernel review list, Ray Jui, Dan Carpenter, Greg Kroah-Hartman, Hans Verkuil, Dave Stevenson, Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel, linux-staging, linux-kernel, kieran.bingham > struct vchiq_mmal_component { > - u32 in_use:1; > + bool in_use:1; > bool enabled:1; The patch you referenced says: +If a structure has many true/false values, consider consolidating them into a +bitfield with 1 bit members, or using an appropriate fixed width type, such as +u8. The code did exactly this, using two bits fields, in one u32. A bool probably takes up 4 bytes, maybe 8 bytes, so this change probably doubles the storage size for these two fields. Are these fields on the hot path, where an extra AND instruction would make a difference? Andrew _______________________________________________ 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] 16+ messages in thread
* Re: [PATCH 2/3] vc04_services: mmal-vchiq: Use bool for vchiq_mmal_component.in_use 2022-11-18 0:23 ` Andrew Lunn @ 2022-11-18 9:01 ` Dan Carpenter 0 siblings, 0 replies; 16+ messages in thread From: Dan Carpenter @ 2022-11-18 9:01 UTC (permalink / raw) To: Andrew Lunn Cc: Umang Jain, Florian Fainelli, Broadcom internal kernel review list, Ray Jui, Greg Kroah-Hartman, Hans Verkuil, Dave Stevenson, Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel, linux-staging, linux-kernel, kieran.bingham On Fri, Nov 18, 2022 at 01:23:36AM +0100, Andrew Lunn wrote: > > struct vchiq_mmal_component { > > - u32 in_use:1; > > + bool in_use:1; > > bool enabled:1; > > The patch you referenced says: > > +If a structure has many true/false values, consider consolidating them into a > +bitfield with 1 bit members, or using an appropriate fixed width type, such as > +u8. > > The code did exactly this, using two bits fields, in one u32. A bool > probably takes up 4 bytes, maybe 8 bytes, so this change probably > doubles the storage size for these two fields. In GCC and Clang bools take a byte, but the C language is vague and other compilers are free to do it differently. > Are these fields on the > hot path, where an extra AND instruction would make a difference? This patch takes the first u32 for "in_use" and squeezes it into the same byte as "enabled" so it makes the struct four bytes smaller. There is still a 3 byte struct hole between "enabled" and "handle" so we could add more 62 bool bitfields if we wanted. In the v2 patch these become: bool in_use; bool enabled; One byte each and there is a two byte gap before "handle". 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] 16+ messages in thread
* [PATCH 3/3] vc04_services: bcm2835-camera: Use bool values for mmal_fmt.remove_padding 2022-11-17 16:00 [PATCH 0/3] vc04_services: Promote bool usage Umang Jain 2022-11-17 16:00 ` [PATCH 1/3] Revert "staging: mmal-vchiq: Avoid use of bool in structures" Umang Jain 2022-11-17 16:00 ` [PATCH 2/3] vc04_services: mmal-vchiq: Use bool for vchiq_mmal_component.in_use Umang Jain @ 2022-11-17 16:00 ` Umang Jain 2022-11-17 18:11 ` [PATCH 0/3] vc04_services: Promote bool usage Stefan Wahren 3 siblings, 0 replies; 16+ messages in thread From: Umang Jain @ 2022-11-17 16:00 UTC (permalink / raw) To: Florian Fainelli, Broadcom internal kernel review list, Ray Jui, Dan Carpenter, Greg Kroah-Hartman, Hans Verkuil, Dave Stevenson, Mauro Carvalho Chehab, linux-rpi-kernel, linux-arm-kernel, linux-staging, linux-kernel Cc: kieran.bingham, Umang Jain struct mmal_fmt.remove_padding is defined as a boolean type hence, use boolean values for it instead of 0/1 integers. This enhances code readability. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- .../bcm2835-camera/bcm2835-camera.c | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c index fd456d1f7061..797ebe2a973a 100644 --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c @@ -87,21 +87,21 @@ static struct mmal_fmt formats[] = { .depth = 12, .mmal_component = COMP_CAMERA, .ybbp = 1, - .remove_padding = 1, + .remove_padding = true, }, { .fourcc = V4L2_PIX_FMT_YUYV, .mmal = MMAL_ENCODING_YUYV, .depth = 16, .mmal_component = COMP_CAMERA, .ybbp = 2, - .remove_padding = 0, + .remove_padding = false, }, { .fourcc = V4L2_PIX_FMT_RGB24, .mmal = MMAL_ENCODING_RGB24, .depth = 24, .mmal_component = COMP_CAMERA, .ybbp = 3, - .remove_padding = 0, + .remove_padding = false, }, { .fourcc = V4L2_PIX_FMT_JPEG, .flags = V4L2_FMT_FLAG_COMPRESSED, @@ -109,7 +109,7 @@ static struct mmal_fmt formats[] = { .depth = 8, .mmal_component = COMP_IMAGE_ENCODE, .ybbp = 0, - .remove_padding = 0, + .remove_padding = false, }, { .fourcc = V4L2_PIX_FMT_H264, .flags = V4L2_FMT_FLAG_COMPRESSED, @@ -117,7 +117,7 @@ static struct mmal_fmt formats[] = { .depth = 8, .mmal_component = COMP_VIDEO_ENCODE, .ybbp = 0, - .remove_padding = 0, + .remove_padding = false, }, { .fourcc = V4L2_PIX_FMT_MJPEG, .flags = V4L2_FMT_FLAG_COMPRESSED, @@ -125,63 +125,63 @@ static struct mmal_fmt formats[] = { .depth = 8, .mmal_component = COMP_VIDEO_ENCODE, .ybbp = 0, - .remove_padding = 0, + .remove_padding = false, }, { .fourcc = V4L2_PIX_FMT_YVYU, .mmal = MMAL_ENCODING_YVYU, .depth = 16, .mmal_component = COMP_CAMERA, .ybbp = 2, - .remove_padding = 0, + .remove_padding = false, }, { .fourcc = V4L2_PIX_FMT_VYUY, .mmal = MMAL_ENCODING_VYUY, .depth = 16, .mmal_component = COMP_CAMERA, .ybbp = 2, - .remove_padding = 0, + .remove_padding = false, }, { .fourcc = V4L2_PIX_FMT_UYVY, .mmal = MMAL_ENCODING_UYVY, .depth = 16, .mmal_component = COMP_CAMERA, .ybbp = 2, - .remove_padding = 0, + .remove_padding = false, }, { .fourcc = V4L2_PIX_FMT_NV12, .mmal = MMAL_ENCODING_NV12, .depth = 12, .mmal_component = COMP_CAMERA, .ybbp = 1, - .remove_padding = 1, + .remove_padding = true, }, { .fourcc = V4L2_PIX_FMT_BGR24, .mmal = MMAL_ENCODING_BGR24, .depth = 24, .mmal_component = COMP_CAMERA, .ybbp = 3, - .remove_padding = 0, + .remove_padding = false, }, { .fourcc = V4L2_PIX_FMT_YVU420, .mmal = MMAL_ENCODING_YV12, .depth = 12, .mmal_component = COMP_CAMERA, .ybbp = 1, - .remove_padding = 1, + .remove_padding = true, }, { .fourcc = V4L2_PIX_FMT_NV21, .mmal = MMAL_ENCODING_NV21, .depth = 12, .mmal_component = COMP_CAMERA, .ybbp = 1, - .remove_padding = 1, + .remove_padding = true, }, { .fourcc = V4L2_PIX_FMT_BGR32, .mmal = MMAL_ENCODING_BGRA, .depth = 32, .mmal_component = COMP_CAMERA, .ybbp = 4, - .remove_padding = 0, + .remove_padding = false, }, }; @@ -1147,7 +1147,7 @@ static int mmal_setup_components(struct bcm2835_mmal_dev *dev, struct vchiq_mmal_port *port = NULL, *camera_port = NULL; struct vchiq_mmal_component *encode_component = NULL; struct mmal_fmt *mfmt = get_format(f); - u32 remove_padding; + bool remove_padding; if (!mfmt) return -EINVAL; -- 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] 16+ messages in thread
* Re: [PATCH 0/3] vc04_services: Promote bool usage 2022-11-17 16:00 [PATCH 0/3] vc04_services: Promote bool usage Umang Jain ` (2 preceding siblings ...) 2022-11-17 16:00 ` [PATCH 3/3] vc04_services: bcm2835-camera: Use bool values for mmal_fmt.remove_padding Umang Jain @ 2022-11-17 18:11 ` Stefan Wahren 2022-11-18 8:47 ` Dan Carpenter 2022-11-18 17:25 ` Dave Stevenson 3 siblings, 2 replies; 16+ messages in thread From: Stefan Wahren @ 2022-11-17 18:11 UTC (permalink / raw) To: Dave Stevenson Cc: kieran.bingham, Mauro Carvalho Chehab, Hans Verkuil, linux-kernel, linux-rpi-kernel, linux-arm-kernel, Umang Jain, Ray Jui, Broadcom internal kernel review list, Dan Carpenter, Florian Fainelli, Greg Kroah-Hartman, linux-staging Hi Dave, Am 17.11.22 um 17:00 schrieb Umang Jain: > In commit 7967656ffbfa ("coding-style: Clarify the expectations around > bool") the check to dis-allow bool structure members was removed from > checkpatch.pl. It promotes bool structure members to store boolean > values. This enhances code readability. > > Umang Jain (3): > Revert "staging: mmal-vchiq: Avoid use of bool in structures" > vc04_services: mmal-vchiq: Use bool for vchiq_mmal_component.in_use > vc04_services: bcm2835-camera: Use bool values for > mmal_fmt.remove_padding > > .../bcm2835-camera/bcm2835-camera.c | 30 +++++++++---------- > .../vc04_services/vchiq-mmal/mmal-vchiq.c | 18 +++++------ > .../vc04_services/vchiq-mmal/mmal-vchiq.h | 6 ++-- could you please check these changes to be safe? I'm not sure that the affected declarations are really internal. I'm afraid this might affect firmware or userspace. > 3 files changed, 27 insertions(+), 27 deletions(-) > _______________________________________________ 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] 16+ messages in thread
* Re: [PATCH 0/3] vc04_services: Promote bool usage 2022-11-17 18:11 ` [PATCH 0/3] vc04_services: Promote bool usage Stefan Wahren @ 2022-11-18 8:47 ` Dan Carpenter 2022-11-18 17:25 ` Dave Stevenson 1 sibling, 0 replies; 16+ messages in thread From: Dan Carpenter @ 2022-11-18 8:47 UTC (permalink / raw) To: Stefan Wahren Cc: Dave Stevenson, kieran.bingham, Mauro Carvalho Chehab, Hans Verkuil, linux-kernel, linux-rpi-kernel, linux-arm-kernel, Umang Jain, Ray Jui, Broadcom internal kernel review list, Florian Fainelli, Greg Kroah-Hartman, linux-staging On Thu, Nov 17, 2022 at 07:11:41PM +0100, Stefan Wahren wrote: > Hi Dave, > > Am 17.11.22 um 17:00 schrieb Umang Jain: > > In commit 7967656ffbfa ("coding-style: Clarify the expectations around > > bool") the check to dis-allow bool structure members was removed from > > checkpatch.pl. It promotes bool structure members to store boolean > > values. This enhances code readability. > > > > Umang Jain (3): > > Revert "staging: mmal-vchiq: Avoid use of bool in structures" > > vc04_services: mmal-vchiq: Use bool for vchiq_mmal_component.in_use > > vc04_services: bcm2835-camera: Use bool values for > > mmal_fmt.remove_padding > > > > .../bcm2835-camera/bcm2835-camera.c | 30 +++++++++---------- > > .../vc04_services/vchiq-mmal/mmal-vchiq.c | 18 +++++------ > > .../vc04_services/vchiq-mmal/mmal-vchiq.h | 6 ++-- > could you please check these changes to be safe? I'm not sure that the > affected declarations are really internal. I'm afraid this might affect > firmware or userspace. The structs have a bunch of kernel pointers in them so hopefully they are internal. Otherwise we have a different sort of problem. 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] 16+ messages in thread
* Re: [PATCH 0/3] vc04_services: Promote bool usage 2022-11-17 18:11 ` [PATCH 0/3] vc04_services: Promote bool usage Stefan Wahren 2022-11-18 8:47 ` Dan Carpenter @ 2022-11-18 17:25 ` Dave Stevenson 1 sibling, 0 replies; 16+ messages in thread From: Dave Stevenson @ 2022-11-18 17:25 UTC (permalink / raw) To: Stefan Wahren Cc: kieran.bingham, Mauro Carvalho Chehab, Hans Verkuil, linux-kernel, linux-rpi-kernel, linux-arm-kernel, Umang Jain, Ray Jui, Broadcom internal kernel review list, Dan Carpenter, Florian Fainelli, Greg Kroah-Hartman, linux-staging Hi Stefan On Thu, 17 Nov 2022 at 18:11, Stefan Wahren <stefan.wahren@i2se.com> wrote: > > Hi Dave, > > Am 17.11.22 um 17:00 schrieb Umang Jain: > > In commit 7967656ffbfa ("coding-style: Clarify the expectations around > > bool") the check to dis-allow bool structure members was removed from > > checkpatch.pl. It promotes bool structure members to store boolean > > values. This enhances code readability. > > > > Umang Jain (3): > > Revert "staging: mmal-vchiq: Avoid use of bool in structures" > > vc04_services: mmal-vchiq: Use bool for vchiq_mmal_component.in_use > > vc04_services: bcm2835-camera: Use bool values for > > mmal_fmt.remove_padding > > > > .../bcm2835-camera/bcm2835-camera.c | 30 +++++++++---------- > > .../vc04_services/vchiq-mmal/mmal-vchiq.c | 18 +++++------ > > .../vc04_services/vchiq-mmal/mmal-vchiq.h | 6 ++-- > could you please check these changes to be safe? I'm not sure that the > affected declarations are really internal. I'm afraid this might affect > firmware or userspace. No problem. These are totally safe. As I've commented on the v2 patch, if it were in the mmal-msg*.h files then I'd be more concerned as those are matching the firmware structures, but these are just internal state. Dave > > 3 files changed, 27 insertions(+), 27 deletions(-) > > _______________________________________________ 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] 16+ messages in thread
end of thread, other threads:[~2022-11-18 17:27 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-11-17 16:00 [PATCH 0/3] vc04_services: Promote bool usage Umang Jain 2022-11-17 16:00 ` [PATCH 1/3] Revert "staging: mmal-vchiq: Avoid use of bool in structures" Umang Jain 2022-11-17 16:09 ` Kieran Bingham 2022-11-17 17:55 ` Umang Jain 2022-11-17 18:02 ` Greg Kroah-Hartman 2022-11-17 18:22 ` Umang Jain 2022-11-18 0:12 ` Andrew Lunn 2022-11-18 3:58 ` Umang Jain 2022-11-18 8:35 ` Dan Carpenter 2022-11-17 16:00 ` [PATCH 2/3] vc04_services: mmal-vchiq: Use bool for vchiq_mmal_component.in_use Umang Jain 2022-11-18 0:23 ` Andrew Lunn 2022-11-18 9:01 ` Dan Carpenter 2022-11-17 16:00 ` [PATCH 3/3] vc04_services: bcm2835-camera: Use bool values for mmal_fmt.remove_padding Umang Jain 2022-11-17 18:11 ` [PATCH 0/3] vc04_services: Promote bool usage Stefan Wahren 2022-11-18 8:47 ` Dan Carpenter 2022-11-18 17:25 ` Dave Stevenson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).