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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox