Message ID | 20201006104414.67984-2-ezequiel@collabora.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | CODA timeout fix & assorted changes | expand |
Hi Ezequiel, On Tue, 2020-10-06 at 07:44 -0300, Ezequiel Garcia wrote: > The ctx->initialized flag is set in __coda_decoder_seq_init, > so it's redundant to set it in coda_dec_seq_init_work. > Remove the redundant set, which allows to simplify the > implementation quite a bit. > > This change shouldn't affect functionality as it's just > cosmetics. > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > --- > drivers/media/platform/coda/coda-bit.c | 12 ++---------- > 1 file changed, 2 insertions(+), 10 deletions(-) > > diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c > index bf75927bac4e..aa0a47c34413 100644 > --- a/drivers/media/platform/coda/coda-bit.c > +++ b/drivers/media/platform/coda/coda-bit.c > @@ -2005,21 +2005,13 @@ static void coda_dec_seq_init_work(struct work_struct *work) > struct coda_ctx *ctx = container_of(work, > struct coda_ctx, seq_init_work); > struct coda_dev *dev = ctx->dev; > - int ret; > > mutex_lock(&ctx->buffer_mutex); > mutex_lock(&dev->coda_mutex); > > - if (ctx->initialized == 1) > - goto out; > - > - ret = __coda_decoder_seq_init(ctx); > - if (ret < 0) > - goto out; > - > - ctx->initialized = 1; > + if (!ctx->initialized) > + __coda_decoder_seq_init(ctx); > > -out: > mutex_unlock(&dev->coda_mutex); > mutex_unlock(&ctx->buffer_mutex); > } Thank you for the cleanup! In general, I'd put the fixes before the cosmetics unless they somehow simplify the following steps, but in this case it doesn't matter much. Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> regards Philipp
diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c index bf75927bac4e..aa0a47c34413 100644 --- a/drivers/media/platform/coda/coda-bit.c +++ b/drivers/media/platform/coda/coda-bit.c @@ -2005,21 +2005,13 @@ static void coda_dec_seq_init_work(struct work_struct *work) struct coda_ctx *ctx = container_of(work, struct coda_ctx, seq_init_work); struct coda_dev *dev = ctx->dev; - int ret; mutex_lock(&ctx->buffer_mutex); mutex_lock(&dev->coda_mutex); - if (ctx->initialized == 1) - goto out; - - ret = __coda_decoder_seq_init(ctx); - if (ret < 0) - goto out; - - ctx->initialized = 1; + if (!ctx->initialized) + __coda_decoder_seq_init(ctx); -out: mutex_unlock(&dev->coda_mutex); mutex_unlock(&ctx->buffer_mutex); }
The ctx->initialized flag is set in __coda_decoder_seq_init, so it's redundant to set it in coda_dec_seq_init_work. Remove the redundant set, which allows to simplify the implementation quite a bit. This change shouldn't affect functionality as it's just cosmetics. Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> --- drivers/media/platform/coda/coda-bit.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-)