* [PATCH v2] Staging: vc04_services: Cleanup in ctrl_set_bitrate()
@ 2019-04-20 16:56 Madhumitha Prabakaran
2019-04-20 19:51 ` Greg KH
0 siblings, 1 reply; 3+ messages in thread
From: Madhumitha Prabakaran @ 2019-04-20 16:56 UTC (permalink / raw)
To: eric, stefan.wahren, gregkh, f.fainelli, rjui, sbranden,
bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel,
devel, linux-kernel
Cc: Madhumitha Prabakaran
Remove unnecessary variable and replace return type.
In addition to that align the function parameters with parentheses to
maintain Linux coding style.
Issue suggested by Coccinelle.
Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
---
Changes in v2
1) Fix my own error of removing function call from the variable.
2) Align the function parameters with parentheses.
---
---
.../staging/vc04_services/bcm2835-camera/controls.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c b/drivers/staging/vc04_services/bcm2835-camera/controls.c
index e39ab5fae724..71eb349c3138 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
@@ -607,18 +607,16 @@ static int ctrl_set_bitrate(struct bm2835_mmal_dev *dev,
struct v4l2_ctrl *ctrl,
const struct bm2835_mmal_v4l2_ctrl *mmal_ctrl)
{
- int ret;
struct vchiq_mmal_port *encoder_out;
dev->capture.encode_bitrate = ctrl->val;
encoder_out = &dev->component[MMAL_COMPONENT_VIDEO_ENCODE]->output[0];
- ret = vchiq_mmal_port_parameter_set(dev->instance, encoder_out,
- mmal_ctrl->mmal_id,
- &ctrl->val, sizeof(ctrl->val));
- ret = 0;
- return ret;
+ vchiq_mmal_port_parameter_set(dev->instance, encoder_out,
+ mmal_ctrl->mmal_id, &ctrl->val,
+ sizeof(ctrl->val));
+ return 0;
}
static int ctrl_set_bitrate_mode(struct bm2835_mmal_dev *dev,
--
2.17.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] 3+ messages in thread* Re: [PATCH v2] Staging: vc04_services: Cleanup in ctrl_set_bitrate()
2019-04-20 16:56 [PATCH v2] Staging: vc04_services: Cleanup in ctrl_set_bitrate() Madhumitha Prabakaran
@ 2019-04-20 19:51 ` Greg KH
2019-04-23 13:36 ` Madhumitha Prabakaran
0 siblings, 1 reply; 3+ messages in thread
From: Greg KH @ 2019-04-20 19:51 UTC (permalink / raw)
To: Madhumitha Prabakaran
Cc: stefan.wahren, devel, f.fainelli, sbranden, rjui, linux-kernel,
eric, bcm-kernel-feedback-list, linux-rpi-kernel,
linux-arm-kernel
On Sat, Apr 20, 2019 at 11:56:44AM -0500, Madhumitha Prabakaran wrote:
> Remove unnecessary variable and replace return type.
> In addition to that align the function parameters with parentheses to
> maintain Linux coding style.
>
> Issue suggested by Coccinelle.
>
> Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
>
> ---
> Changes in v2
>
> 1) Fix my own error of removing function call from the variable.
> 2) Align the function parameters with parentheses.
>
> ---
> ---
> .../staging/vc04_services/bcm2835-camera/controls.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> index e39ab5fae724..71eb349c3138 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> @@ -607,18 +607,16 @@ static int ctrl_set_bitrate(struct bm2835_mmal_dev *dev,
> struct v4l2_ctrl *ctrl,
> const struct bm2835_mmal_v4l2_ctrl *mmal_ctrl)
> {
> - int ret;
> struct vchiq_mmal_port *encoder_out;
>
> dev->capture.encode_bitrate = ctrl->val;
>
> encoder_out = &dev->component[MMAL_COMPONENT_VIDEO_ENCODE]->output[0];
>
> - ret = vchiq_mmal_port_parameter_set(dev->instance, encoder_out,
> - mmal_ctrl->mmal_id,
> - &ctrl->val, sizeof(ctrl->val));
> - ret = 0;
> - return ret;
> + vchiq_mmal_port_parameter_set(dev->instance, encoder_out,
> + mmal_ctrl->mmal_id, &ctrl->val,
> + sizeof(ctrl->val));
> + return 0;
> }
Why does this function even return anything at all if it can not fail?
Why not return the value that vchiq_mmal_port_parameter_set() returned?
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] 3+ messages in thread* Re: [PATCH v2] Staging: vc04_services: Cleanup in ctrl_set_bitrate()
2019-04-20 19:51 ` Greg KH
@ 2019-04-23 13:36 ` Madhumitha Prabakaran
0 siblings, 0 replies; 3+ messages in thread
From: Madhumitha Prabakaran @ 2019-04-23 13:36 UTC (permalink / raw)
To: Greg KH
Cc: stefan.wahren, devel, f.fainelli, sbranden, rjui, linux-kernel,
eric, bcm-kernel-feedback-list, linux-rpi-kernel,
linux-arm-kernel
On 04/20 :51, Greg KH wrote:
> On Sat, Apr 20, 2019 at 11:56:44AM -0500, Madhumitha Prabakaran wrote:
> > Remove unnecessary variable and replace return type.
> > In addition to that align the function parameters with parentheses to
> > maintain Linux coding style.
> >
> > Issue suggested by Coccinelle.
> >
> > Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> >
> > ---
> > Changes in v2
> >
> > 1) Fix my own error of removing function call from the variable.
> > 2) Align the function parameters with parentheses.
> >
> > ---
> > ---
> > .../staging/vc04_services/bcm2835-camera/controls.c | 10 ++++------
> > 1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > index e39ab5fae724..71eb349c3138 100644
> > --- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > +++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > @@ -607,18 +607,16 @@ static int ctrl_set_bitrate(struct bm2835_mmal_dev *dev,
> > struct v4l2_ctrl *ctrl,
> > const struct bm2835_mmal_v4l2_ctrl *mmal_ctrl)
> > {
> > - int ret;
> > struct vchiq_mmal_port *encoder_out;
> >
> > dev->capture.encode_bitrate = ctrl->val;
> >
> > encoder_out = &dev->component[MMAL_COMPONENT_VIDEO_ENCODE]->output[0];
> >
> > - ret = vchiq_mmal_port_parameter_set(dev->instance, encoder_out,
> > - mmal_ctrl->mmal_id,
> > - &ctrl->val, sizeof(ctrl->val));
> > - ret = 0;
> > - return ret;
> > + vchiq_mmal_port_parameter_set(dev->instance, encoder_out,
> > + mmal_ctrl->mmal_id, &ctrl->val,
> > + sizeof(ctrl->val));
> > + return 0;
> > }
>
> Why does this function even return anything at all if it can not fail?
I looked at the function vchiq_mmal_port_parameter_set, which calls
another function for setting the port parameters, which inturns sends an
synchronous message. If it is able to send the message successfully, it
returns 0. It can also fail and can return any other value. Also the
function ctrl_set_bitrate is not evaluated anywhere for true or false,
so it is safe to return the value we get from
vchiq_mmal_port_parameter_set instead of returning 0 always.
>
> Why not return the value that vchiq_mmal_port_parameter_set() returned?
>
I will change the code to return the value from
vchiq_mmal_port_parameter_set directly.
> 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] 3+ messages in thread
end of thread, other threads:[~2019-04-23 13:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-20 16:56 [PATCH v2] Staging: vc04_services: Cleanup in ctrl_set_bitrate() Madhumitha Prabakaran
2019-04-20 19:51 ` Greg KH
2019-04-23 13:36 ` Madhumitha Prabakaran
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).