On Fri, 16 Jan 2009, Jean Delvare wrote:
> Hi Mauro, Trent,
> On Fri, 16 Jan 2009 00:02:52 -0200, Mauro Carvalho Chehab wrote:
> > On Thu, 15 Jan 2009 10:33:15 -0800 (PST)
> > Trent Piepho <xy...@speakeasy.org> wrote:
> >
> > > On Thu, 15 Jan 2009, Mauro Carvalho Chehab wrote:
> > For now, we should finish the Hans approach, since it also helps to stop 
> > using
> > the legacy i2c methods. After having all drivers using it, we can do some
> > cleanup at the drivers.

How will this work for drivers like bttv, where the i2c address of the
tuner chips isn't know for every supported card?

> > However, I like the idea of providing a better support for those buses that
> > have an i2c switch inside (I don't like to call it "virtual" - it is a real 
> > i2c
> > bus, where part of the bus is controlled by a switch).
>
> I second this, the term "virtual" to qualify these buses is improper.
> Better speak in terms of I2C segments, multiplexers and gates. Gates
> can be seen as a degenerated form of multiplexers, but this is not
> necessarily the best approach.

Maybe I should say virtual i2c adapter.  The driver registers another i2c
adapter that does not represent a new i2c master, but instead a new i2c bus
segment on an existing i2c master.

> > >  When a device has some kind of i2c gate, it creates a new
> > > I2C adapter for the devices behind the gate.  The code for this virtual 
> > > i2c
> > > adapter can just open the gate, pass of the request to the main i2c
> > > adapter, then close the gate.  Creating a new i2c adapter should trigger
> > > the i2c drivers that scan to do so and find new devices behind the gate.
> > >
> > > It seems like this would solve the scan order problem, since the bus the
> > > tuner/demod/whatever is on won't exist until the gate it is behind can be
> > > properly controlled.
>
> Well, the scanning order problem is IMHO better resolved by
> instantiating the devices explicitly instead of letting the i2c
> subsystem probe for them. Everything is already in place to do this,
> and a number of drivers are already being converted in this direction.

The problem is that then one needs to know where the i2c devices are, and
this is not known for all cards.  When the bttv driver was converted to use
the new i2c subsystem a decade ago and used probing as the only way to find
devices, I said it was a bad idea.  But now we have this database of
cards with no information about what chips and what address.

> > > There are a number of additional benefits too.  There are many devices 
> > > that
> > > can be behind many different kinds of gates.  So we have all these gate
> > > control functions that must be called manually from all over the place.
> > > This adds bloat and developers are always forgetting to call them, which
> > > doesn't cause any problem they notice because their card doesn't have a
> > > gate.
> > >
> > > With manual gate control, we must remember to close the gate when done 
> > > with
> > > the device.  But this isn't always done and the gate is left open.  These
> > > gates are there for a reason, to shield RF devices from noise created by
> > > the I2C bus, and so leaving them opens impairs RF performance.
> > >
> > > And when the gate is only controlled by the driver in the kernel, it is
> > > hard to manually debug/test i2c devices from userspace with i2c-dev.
> >
> > I'm not sure if we should implement it inside v4l core, or at i2c-core.
>
> This is an excellent question, I don't know for sure myself.

Why does either need support?

static int cx88_gate_i2c_xfer(struct i2c_adapter *i2c_adap,
                              struct i2c_msg *msgs, int num)
{
        struct cx88_gate_i2c *gate= i2c_get_adapdata(i2c_adap);
        int ret;

        cx88_dvb_gate_ctrl(gate->core, 1);
        ret = i2c_transfer(&gate->core->i2c_adap, msgs, num);
        cx88_dvb_gate_ctrl(gate->core, 0);
        return ret;
}

...
        static struct i2c_algorithm cx88_gate = {
                .master_xfer = cx88_gate_i2c_xfer,
                ...
        };
        static struct i2c_adapter adap = {
                .algo = &cx88_gate,
                ...
        };
        struct cx88_gate_i2c *gate = kzalloc(sizeof(typeof(*gate)), GFP_KERNEL);
        gate->core = core;
        i2c_set_adapdata(&adap, gate);
        i2c_add_adapter(&adap);

What part needs to go somewhere else?

> of 2 other:
>
> * At I2C bus driver level. We can have a pre-transfer handler and a
> post-transfer handler, which does what needs to be done (like opening
> and closing a gate) based on the address of the device that is being
> accessed. I had discussed this approach with Michael Krufky long ago.

This won't work when muxes are used to put multiple i2c devices with the
same address behind a single master.

It still has the problem of probe order when one wants to scan for chips.
The i2c core does scanning when a new adapter is created.  But, suppose the
mux is an i2c device on the adapter?  In order for the scanner to find
anything behind the mux, it needs to be detected and working before the bus
is scanned.  But this may not happen.  Say one calls:
i2c_add_adapter(adap); i2c_new_device(adap, &the_mux);

If the client driver for the device behind the mux uses probing, and is
already loaded when i2c_add_adapter() is called, then it will be probed for
before the i2c_new_device() call and won't be found because the mux isn't
working.

If instead the mux driver created a new i2c adapter for the segment(s)
behind it, which would happen when i2c_new_device() is called, then those
segments would be probed at the correct time.

> * At the chip driver level, using a custom API. I think this is what is
> done right now? You lose transparency, but performance may be better:
> if you have many commands to send to a device behind the gate, you can
> open the gate, batch them, then close the gate. With the transparent
> models, the gate is opened and closed again for every I2C transfer. I
> don't know if this is a big problem in practice, as presumably this
> only happens once at initialization time?

No, the devices behind the gate are usually used throughout device
operation.  Typically there is only one transfer at a time and so nothing
to batch together.

But this isn't insolvable.  For a real mux, the driver could only switch
the mux when the selected segment is different from the last one.  For a
gate, one could reset a timer on each transfer that will close the gate
when it gets triggered.  That way when multiple commands arrive close
enough together the gate can be left open for all of them.  This has an
advantage over manually batching commands in that in many cases the code
that sends one command does not know that another command will be sent
immediately afterwards and so they are not batched.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to