Re: [RFC PATCH v2] media: i2c: add SCCB helpers

[Wolfram Sang <wsa@the-dreams.de>]

[-- Attachment #1: Type: text/plain, Size: 2135 bytes --]


> > I'd prefer this file to be in the i2c realm. Maybe
> > 'include/linux/i2c-sccb.h" or something. I will come back to this.
> 
> And while at it, I think we also need a .c file, the functions (and especially 
> sccb_read_byte()) should not be static inline.

Before we discuss this, we should make sure that the read-function is
complete and then we can decide further. I found some old notes based on
our previous discussions about SCCB and refactoring its access. You
mentioned there is HW supporting SCCB natively. Also, we found a device
where an SCCB device is connected to an SMBUS controller (yuck!). So,
given that, I'd think the read routine should look like this (in pseudo
code):


bool is_sccb_available(adap)
{
	u32 needed_funcs = I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_WRITE_BYTE_DATA;

	/*
	 * sccb_xfer not needed yet, since there is no driver support currently.
	 * Just showing how it should be done if we ever need it.
	 */
	if (adap->algo->sccb_xfer)
		return true;

	if (i2c_get_functionality(adap) & needed_funcs == needed_funcs)
		return true;

	return false;
}

sccb_get_byte()
{
	if (adap->algo->sccb_xfer)
		adap->algo->sccb_xfer(...)

	/*
	 * Prereq: __i2c_smbus_xfer needs to be created!
	 */
	i2c_lock_adapter(SEGMENT);
	__i2c_smbus_xfer(..., SEND_BYTE, ...)
	__i2c_smbus_xfer(..., GET_BYTE, ...)
	i2c_unlock_adapter(SEGMENT)
}

sccb_write_byte()
{
	return i2c_smbus_write_byte_data(...)
}

If I haven't overlooked something, this should make SCCB possible on I2C
controllers and SMBus controllers. We honor the requirement of not
having repeated start anywhere. As such, we might get even rid of the
I2C_M_STOP flag (for in-kernel users, but we sadly export it to
userspace).

About the IGNORE_NAK flag, I think this still should work where
supported as it is passed along indirectly with I2C_CLIENT_SCCB.
However, this flag handling with SCCB is really a mess and needs an
overhaul. This can be a second step, however. Most devices don't really
need it, do they?

Any further comments? Anything I missed?

Kind regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]