[linux-dvb] Re: Proposal for new frontend architecture

[Gerd Knorr <kraxel@bytesex.org>]

> There is a parallel, non-dvb discussion about adding 
> i2c_adapter->command() on LKML now named "[PATCH][2.6] Add command 
> function to struct i2c_adapter".

I've seen it ...

> It's a big question, if keeping a kernel i2c demod driver is really a 
> good thing.

Yes, it is IMHO.

> There is the i2c_client->command() interface that can be 
> used to give commands from the adapter to the client. As we have seen, 
> there is sometimes the need to have a client->adapter communication as 
> well, so we proposed i2c_adapter->command(). Greg K-H and others dislike 
> this idea, because it's not type safe. I now agree with that.

Yep, theier complain is that i2c_client->command() is a ioctl()-like,
not type-save thingy.  At least v4l2 actually uses that to pass down
ioctls to the i2c drivers, so it must work that way.  For the
kernel-internal stuff (TUNER_SET_TYPE + friends) it might be useful to
switch over to something type safe through.

> Similar to the results of the LKML discussion, it's perhaps time to say 
> good-bye to generic i2c busses.

I don't think so.

> The adapter registers this special bus within the system and requests 
> other modules to join in. The dvb i2c adapters provide functions for dvb 
> i2c clients to query h/w dependent informations. If an dvb i2c client 
> joins the bus, they provide functions for the dvb i2c adapter to call 
> certain actions in a type-safe manner.

I'd design it the other way around:  Don't let the i2c clients query the
config information, but require the adapter to pass it when creating the
devices.  Leave the busses as-is, and extend the client structs.  A
client usually has some specific function, that isn't nessesarely true
for a bus (bttv can have both analog and digital stuff, the embedded
devs maybe have hw monitoring and dvb chips on the same bus ...).

> It's the question whether enhancing the i2c-core in that way is going to 
> be accepted. Do other drivers really benefit from the possibility of 
> having a well-defined interface as well or it this simply bloat?

I'd go for enhancing kernel i2c.  I even think we don't have to actually
touch the i2c-core to do that.  Asking kernel-i2c not to probe is just a
matter of clearing the I2C_DF_NOTIFY flag in i2c_driver.  Then do
something like this ...

struct dvb_fe_i2c_client {
	struct i2c_client core;
	int (dvb_fe_unregister)(struct dvb_fe_i2c_client *c);
	int (dvb_do_foobar)(struct dvb_fe_i2c_client *c, /* args */);
	/* whatever else we'll need */
};

struct dvb_fe_i2c_client* <fe-name>_create(
	struct i2c_adapter *adap,
	struct dvb_adapter *dvb,
	/* some more parameters with config info */,
	/* maybe also callbacks for stuff which should't
           be in the fe driver, i.e. (*set_pll)(), so we
           can get started with moving that out of the
           fe code and still rougthly keep the current
           fe driver structure */
)

> For example my tea6415c or tea6420c drivers, these device are matrix 
> switches and really dumb. The adapter currently use the ioctl interface 
> to control the configuration of the matrix.
> 
> IMO having a "well-defined interface" in that case doesn't really help, 
> because there is only one function ("set input a to output b") these 
> chipsets have.

For these it might be not worth designing a specific interface but just
have some functions like that:

	struct i2c_client* tea6415c_create(...);
	tea6415c_matrix(struct i2c_client *c, int in, int out);

> We have to keep in mind the problems on set-top-box systems. There, we 
> only have one i2c bus that can have it all: dvb "frontend", thermal 
> sensors, audio/video multiplexer, analog/digital converters and 
> digital/analog converters.

Exactly thats why I think it would be wise *not* to extend the i2c
busses but the clients.

  Gerd

-- 
return -ENOSIG;