From: Mauro Carvalho Chehab <mchehab@redhat.com>
To: Andreas Oberritter <obi@linuxtv.org>
Cc: linux-media@vger.kernel.org, Thierry LELEGARD <tlelegard@logiways.com>
Subject: Re: [PATCH 2/8] DVB: dtv_property_cache_submit shouldn't modifiy the cache
Date: Mon, 09 May 2011 05:58:22 +0200 [thread overview]
Message-ID: <4DC7665E.5000202@redhat.com> (raw)
In-Reply-To: <1304895821-21642-3-git-send-email-obi@linuxtv.org>
Em 09-05-2011 01:03, Andreas Oberritter escreveu:
> - Use const pointers and remove assignments.
That's OK.
> - delivery_system already gets assigned by DTV_DELIVERY_SYSTEM
> and dtv_property_cache_sync.
The logic for those legacy params is too complex. It is easy that
a latter patch to break the implicit set via dtv_property_cache_sync().
Do you actually see a bug caused by the extra set for the delivery system?
If not, I prefer to keep this extra re-assignment.
>
> Signed-off-by: Andreas Oberritter <obi@linuxtv.org>
> ---
> drivers/media/dvb/dvb-core/dvb_frontend.c | 13 +++----------
> 1 files changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c
> index be0f631..1ac7633 100644
> --- a/drivers/media/dvb/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb/dvb-core/dvb_frontend.c
> @@ -1074,7 +1074,7 @@ static void dtv_property_cache_sync(struct dvb_frontend *fe,
> */
> static void dtv_property_legacy_params_sync(struct dvb_frontend *fe)
> {
> - struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> + const struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> struct dvb_frontend_private *fepriv = fe->frontend_priv;
> struct dvb_frontend_parameters *p = &fepriv->parameters;
>
> @@ -1086,14 +1086,12 @@ static void dtv_property_legacy_params_sync(struct dvb_frontend *fe)
> dprintk("%s() Preparing QPSK req\n", __func__);
> p->u.qpsk.symbol_rate = c->symbol_rate;
> p->u.qpsk.fec_inner = c->fec_inner;
> - c->delivery_system = SYS_DVBS;
> break;
> case FE_QAM:
> dprintk("%s() Preparing QAM req\n", __func__);
> p->u.qam.symbol_rate = c->symbol_rate;
> p->u.qam.fec_inner = c->fec_inner;
> p->u.qam.modulation = c->modulation;
> - c->delivery_system = SYS_DVBC_ANNEX_AC;
> break;
> case FE_OFDM:
> dprintk("%s() Preparing OFDM req\n", __func__);
> @@ -1111,15 +1109,10 @@ static void dtv_property_legacy_params_sync(struct dvb_frontend *fe)
> p->u.ofdm.transmission_mode = c->transmission_mode;
> p->u.ofdm.guard_interval = c->guard_interval;
> p->u.ofdm.hierarchy_information = c->hierarchy;
> - c->delivery_system = SYS_DVBT;
> break;
> case FE_ATSC:
> dprintk("%s() Preparing VSB req\n", __func__);
> p->u.vsb.modulation = c->modulation;
> - if ((c->modulation == VSB_8) || (c->modulation == VSB_16))
> - c->delivery_system = SYS_ATSC;
> - else
> - c->delivery_system = SYS_DVBC_ANNEX_B;
> break;
> }
> }
> @@ -1129,7 +1122,7 @@ static void dtv_property_legacy_params_sync(struct dvb_frontend *fe)
> */
> static void dtv_property_adv_params_sync(struct dvb_frontend *fe)
> {
> - struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> + const struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> struct dvb_frontend_private *fepriv = fe->frontend_priv;
> struct dvb_frontend_parameters *p = &fepriv->parameters;
>
> @@ -1170,7 +1163,7 @@ static void dtv_property_adv_params_sync(struct dvb_frontend *fe)
>
> static void dtv_property_cache_submit(struct dvb_frontend *fe)
> {
> - struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> + const struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>
> /* For legacy delivery systems we don't need the delivery_system to
> * be specified, but we populate the older structures from the cache
next prev parent reply other threads:[~2011-05-09 3:58 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-08 23:03 [PATCH 0/8] dvb_frontend cleanup, S2API regression fix Andreas Oberritter
2011-05-08 23:03 ` [PATCH 1/8] DVB: return meaningful error codes in dvb_frontend Andreas Oberritter
2011-05-08 23:03 ` [PATCH 2/8] DVB: dtv_property_cache_submit shouldn't modifiy the cache Andreas Oberritter
2011-05-09 3:58 ` Mauro Carvalho Chehab [this message]
2011-05-09 4:12 ` Mauro Carvalho Chehab
2011-05-09 10:12 ` Andreas Oberritter
2011-05-08 23:03 ` [PATCH 3/8] DVB: call get_property at the end of dtv_property_process_get Andreas Oberritter
2011-05-08 23:03 ` [PATCH 4/8] DVB: dvb_frontend: rename parameters to parameters_in Andreas Oberritter
2011-05-08 23:03 ` [PATCH 5/8] DVB: dvb_frontend: remove unused assignments Andreas Oberritter
2011-05-08 23:03 ` [PATCH 6/8] DVB: dvb_frontend: use shortcut to access fe->dtv_property_cache Andreas Oberritter
2011-05-08 23:03 ` [PATCH 7/8] DVB: dvb_frontend: add parameters_out Andreas Oberritter
2011-05-08 23:03 ` [PATCH 8/8] DVB: allow to read back of detected parameters through S2API Andreas Oberritter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4DC7665E.5000202@redhat.com \
--to=mchehab@redhat.com \
--cc=linux-media@vger.kernel.org \
--cc=obi@linuxtv.org \
--cc=tlelegard@logiways.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.