Re: [GIT PULL FOR 3.3] HDIC HD29L2 DMB-TH demodulator driver

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

 



On 12/21/2011 12:35 AM, Michael Krufky wrote:
On Tue, Dec 20, 2011 at 10:26 AM, Mauro Carvalho Chehab
<mchehab@xxxxxxxxxx>  wrote:
On 20-12-2011 12:52, Antti Palosaari wrote:
+    /* reset demod */
+    /* it is recommended to HW reset chip using RST_N pin */
+    if (fe->callback) {
+        ret = fe->callback(fe, 0, 0, 0);

This looks weird on my eyes. The fe->callback is tuner-dependent.
So, the command you should use there requires a test for the tuner
type.

In other words, if you're needing to use it, the code should be doing
something similar to:

          if (fe->callback&&    priv->tuner_type == TUNER_XC2028)
         ret = fe->callback(fe, 0, XC2028_TUNER_RESET, 0);

(the actual coding will depend on how do you store the tuner type, and
what are the commands for the tuner you're using)

That's said, it probably makes sense to deprecate fe->callback in favor
or a more generic set of callbacks, like fe->tuner_reset().

Yes it is wrong because there was no DEMOD defined. But, the callback
itself is correctly. IIRC there was only TUNER defined and no DEMOD.
Look callback definition from the API. It is very simply to fix. But at
the time left it like that, because I wanted to avoid touching one file
more. I will fix it properly later (2 line change).

And it was not a bug, it was just my known decision. I just forget to comment it as FIXME or TODO.

Feel free to touch on other files, provided that you fix that. There's
no default behavior for fe->callback, as it were written in order to
provide ways for the tuner to call the bridge driver for some device-specific
reasons. So, the commands are defined with macros, and the callback code
should be device-specific.

This generic callback was written for the BRIDGE driver to be called
by any frontend COMPONENT, not specifically the tuner, perhaps a demod
or LNB, but at the time of writing, we only needed it from the tuner
so the DVB_FRONTEND_COMPONENT_TUNER(0) was the only #define created at
the time.  This was written with forward-compatibility in mind, so
lets please not deprecate it unless we actually have to -- I see
additional uses for this coming in the future.

In order to use fe callback properly, please add "#define
DVB_FRONTEND_COMPONENT_DEMOD 1" to dvb-core/dvb_frontend.h , and
simply call your callback as fe->callback(adap_state,
DVB_FRONTEND_COMPONENT_DEMOD, CMD, ARG) ... This way, the callback
knows that it is being called by the demod and not the tuner, it is
receiving command CMD with argument ARG.

I can imagine a need one day to perhaps convert the "int arg" into a
"void* arg", but such a need doesn't exist today.  I don't think it
gets any more generic than this.

         int (*callback)(void *adapter_priv, int component, int cmd, int arg);

Cheers,

Mike

+1 for that. It was just what I also was thinking :) I will add that demod component define to the internal API and it is fixed properly.

regards
Antti


--
http://palosaari.fi/
--
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