On 11/10/19 4:05 PM, Wolfram Sang wrote:
> Hi Hans,
>
> I know it is 13 years since you wrote that code:
>
> ba8fc39954bf ("V4L/DVB (4270): Add tda9887-specific tuner configuration")
>
> There, you added i2c_clients_command() which looks today like this:
>
> i2c_clients_command(priv->i2c_props.adap, TUNER_SET_CONFIG,
> &tda9887_cfg);
>
> Now, I would like to get rid of the i2c_clients_command() API, and this
> is the one remaining user. While trying to convert it into a local
> function, I started wondering why i2c_clients_command() is used at all.
> Why do we need to try all devices on the bus? A few lines later we have:
>
> tuner_i2c_xfer_send(&priv->i2c_props, buffer, 4);
>
> so i2c_props.addr seems to have a valid address? Or are there multiple
> tuners on that bus? Can you kindly shed some light on this?
Oh boy... The tuner code is awful for many reasons.
One reason is that for a given video capture card there can be multiple
tuner devices: Analog TV, Digital TV, Analog demodulator and/or radio.
The TUNER_SET_CONFIG command is (AFAICT) only used to configure the
tda9887 (demodulator), and the problem is that when this code was written
the tuner instance that is calling TUNER_SET_CONFIG does not know on which
i2c address the demod is.
TUNER_SET_CONFIG is called whenever the TV or radio frequency is
changed, and when that happens the demod needs to update its configuration
as well. So the tuner_i2c_xfer_send(&priv->i2c_props, buffer, 4); is for the
tuner itself (whose i2c address is known), but i2c_clients_command is
basically broadcasting to anyone on the bus, and only the tda9987 will
actually act on the TUNER_SET_CONFIG command.
However, it looks like this code is only used by hybrid tuners (i.e.
supporting both digital and analog TV), and that the available tuners
are now added to a global list via hybrid_tuner_request_state().
This should make it possible for tuner-simple.c to actually lookup the
i2c address of the demod from that global list and use a direct call.
I've CC-ed a bunch of people who know more about DVB than I do, so
I hope they can double-check that I am on the right track.
Wolfram, do you just want to get rid of i2c_clients_command or the i2c
command() callback as well?
Regardless, it would be nice if we can get rid of the command() callback
in the media subsystem. If I read the code right, then you can get the
tuner struct of an i2c_client using 'struct tuner *t = to_tuner(i2c_get_clientdata(c));'
and from there you can get the analog_demod_ops that you need to
configure the tda9887.
So I think this would work:
1) walk hybrid_tuner_instance_list to find a tuner with name "tda9887" for
the given i2c_adapter.
2) get the i2c_client from the adapter with address state->i2c_props.addr.
3) struct tuner *t = to_tuner(i2c_get_clientdata(c));
4) if (t->fe.ops.analog_ops.set_config)
t->fe.ops.analog_ops.set_config(&t->fe, &config);
Regards,
Hans
