Re: PATCH 04/13: 0004-TDA18271-Allow-frontend-to-set-DELSYS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 21, 2011 at 4:28 PM, Manu Abraham <abraham.manu@xxxxxxxxx> wrote:
> On 11/22/11, Michael Krufky <mkrufky@xxxxxxxxxxx> wrote:
>> Thank you, Manu... After the Linux Kernel Summit in Prague, I had
>> intentions of solving this exact problem, but you did it first -- good
>> job!
>>
>> I have reviewed the patch to the tda18271 driver, and the changes make
>> good sense to me.  I have one question, however:
>>
>> Perhaps my eyes have overlooked something -- I fail to see any code
>> that defines the new "set_state" callback or any code that calls this
>> new callback within dvb-core (assuming dvb_frontend.c)  I also can't
>> find the structure declaration of the "tuner_state" struct... ... is
>> this patch missing from your series, or did I just overlook it?
>
> I guess more like that. The data structure existed for quite a long
> while in dvb_frontend.h and hence you don't find any new changes. Only
> delivery and modulation added to it.
>
>>
>> That missing patch is what interests me most.  Once I can see that
>> missing code, I'd like to begin discussion on whether we actually need
>> the additional callback, or if it can simply be handled by the
>> set_params call.  Likewise, I'm not exactly sure why we need this
>> affional "struct tuner_state" ...  Perhaps the answer will be
>> self-explanatory once I see the code - maybe no discussion is
>> necessary :-P
>>
>> But this does look good to me so far.  I'd be happy to provide my
>> "reviewed-by" tag once I can see the missing code mentioned above.
>
> The callback is used from within a demodulator context as usual and hence.
> eg:
>
>        /* program tuner */
> -       if (fe->ops.tuner_ops.set_params)
> -               fe->ops.tuner_ops.set_params(fe, params);
> +       tstate.delsys = SYS_DVBC_ANNEX_AC;
> +       tstate.frequency = c->frequency;
> +
> +       if (fe->ops.tuner_ops.set_state) {
> +               fe->ops.tuner_ops.set_state(fe,
> +                                           DVBFE_TUNER_DELSYS    |
> +                                           DVBFE_TUNER_FREQUENCY,
> +                                           &tstate);
> +       } else {
> +               if (fe->ops.tuner_ops.set_params)
> +                       fe->ops.tuner_ops.set_params(fe, params);
> +       }
>
>
> Best Regards,
> Manu
>

Manu,

Thank you for explaining -- I found that structure in dvb_frontend.h,
now that you've pointed that out.

I am on board with this change -- it is a positive move in the right
direction.  I believe that after this is merged, we may be able to
obsolete and remove the set_params callback.  In fact, we can even
obsolete the set_analog_params callback as well, using set_state as
the single entry point for setting the tuner.  Of course, one step at
a time -- this is great for now.  We should consider the other
optimizations after this has been merged and tested. :-)

Reviewed-by: Michael Krufky <mkrufky@xxxxxxxxxxx>
--
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


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux