Em 09-05-2011 05:58, Mauro Carvalho Chehab escreveu:
> 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.
Hmm... after applying all patches the logic change, and patch 2 may actually
make sense. I'll need to re-examine the patch series.
On a quick look, if applied as-is, I suspect that git bisect
will break dvb in the middle of the patch series.
Anyway, patch 1/8 is OK. For now, I'll apply only this patch. I'll delay the
others until I have more time. I'm currently traveling abroad, due to Linaro
Development Summit, so, I don't have much time for review (and I'm also suffering
for a 5 hours jet-leg).
>>
>> Signed-off-by: Andreas Oberritter <obi@xxxxxxxxxxx>
>> ---
>> 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
