Am 02.01.2013 22:15, schrieb Antti Palosaari: > On 01/02/2013 11:12 PM, Frank Schäfer wrote: >> Hi Antti, >> >> Am 02.01.2013 20:29, schrieb Antti Palosaari: >>> On 12/24/2012 01:09 PM, Frank Schäfer wrote: >>>> Am 23.12.2012 15:46, schrieb Mauro Carvalho Chehab: >>>>> Em Sun, 23 Dec 2012 14:58:12 +0100 >>>>> Frank Schäfer <fschaefer.oss@xxxxxxxxxxxxxx> escreveu: >>>>> >>>>>> Am 23.12.2012 01:07, schrieb Mauro Carvalho Chehab: >>>>>>> Em Sun, 16 Dec 2012 19:23:28 +0100 >>>>>>> Frank Schäfer <fschaefer.oss@xxxxxxxxxxxxxx> escreveu: >>>> >>>>>>> Those devices are limited, and just like other devices (cx231xx >>>>>>> for example), >>>>>>> the I2C bus need to split long messages, otherwise the I2C devices >>>>>>> will >>>>>>> fail. >>>>>> I2C adapters are supposed to fail with -EOPNOTSUPP if the message >>>>>> length >>>>>> exceeds their capabilities. >>>>>> Drivers must be able to handle this error, otherwise they have to >>>>>> be fixed. >>>>> Currently, afaikt, no V4L2 I2C client knows how to handle it. >>>> >>>> Maybe. Fortunately, it seems to cause no trouble. >>>> >>>>> Ok, returning >>>>> -EOPNOTSUPP if the I2C data came from userspace makes sense. >>>>> >>>>>>> Btw, there was already a long discussion with regards to splitting >>>>>>> long >>>>>>> I2C messages at the I2C bus or at the I2C adapters. The decision >>>>>>> was >>>>>>> to do it at the I2C bus logic, as it is simpler than making a code >>>>>>> at each I2C client for them to properly handle -EOPNOTSUPP and >>>>>>> implement >>>>>>> a fallback logic to reduce the transfer window until reach what's >>>>>>> supported by the device. >>>>>> While letting the i2c bus layer split messages sounds like the right >>>>>> thing to do, it is hard to realize that in practice. >>>>>> The reason is, that the needed algorithm depends on the >>>>>> capabilities and >>>>>> behavior of the i2c adapter _and_ the connected i2c client. >>>>>> The three main parameters are: >>>>>> - message size limits >>>>>> - client register width >>>>>> - automatic register index incrementation >>>>>> >>>>>> I don't know what has been discussed in past, >>>>> You'll need to dig into the ML archives. This is a recurrent theme, >>>>> and, >>>>> we have implementations doing I2C split at bus (most cases) and a few >>>>> ones doing it at the client side. >>>> >>>> Yeah, I also have a working implementation of i2c block read/write >>>> emulation in my experimental code. ;) >>>> >>>>>> but I talked to Jean >>>>>> Delvare about the message size constraints a few weeks ago. >>>>>> He told me that it doesn't make sense to try to handle this at >>>>>> the i2c >>>>>> subsystem level. The parameters can be different for reading and >>>>>> writing, adapter and client and things are getting complicated >>>>>> quickly. >>>>> Jean's opinion is to push it to I2C clients (and we actually do it >>>>> on a >>>>> few cases), but as I explained before, there are several drivers >>>>> where >>>>> this is better done at the I2C bus driver, as the I2C implementation >>>>> allows doing it easily at bus level by playing with I2C STOP bits/I2C >>>>> start bits. >>>>> >>>>> We simply have too much I2C clients, and -EOPNOTSUPP error code >>>>> doesn't >>>>> tell the max size of the I2C messages. Adding a complex split logic >>>>> for every driver is not a common practice, as just a few I2C bus >>>>> bridge >>>>> drivers suffer from very strict limits. >>>> >>>> Yes, and even with those who have such a strict limit, it is >>>> usually not >>>> exceeded because the clients are too 'simple'. ;) >>>> >>>>> Also, clients that split I2C messages don't actually handle >>>>> -EOPNOTSUPP. >>>>> Instead, they have an init parameter telling the maximum size of the >>>>> I2C messages accepted by the bus. >>>>> >>>>> The logic there is complex, and may require an additional logic at >>>>> the >>>>> bus side, in order to warrant that no I2C stop/start bits will be >>>>> sent >>>>> in the middle of a message, or otherwise the device will fail[1]. >>>>> >>>>> So, it is generally simpler and more effective to just do it at >>>>> the bus >>>>> side. >>>> >>>> Maybe. I have no opinion yet. >>>> My feeling is, that this should be handled by the i2c subsystem as >>>> much >>>> as possible, but >>>> a) it's complex due to the described reasons >>>> b) I have no complete concept yet >>>> c) the i2c people seem to be not very interested >>>> d) there is lots of other stuff with a higher priority on my TODO list >>> >>> Maybe you already have seen, but I did some initial stuff year or two >>> ago for implementing that but left it unimplemented as there was so >>> much stuff to check and discuss in order to agree correct solution. >>> >>> http://www.mail-archive.com/linux-media@xxxxxxxxxxxxxxx/msg38840.html >>> >>> There is regmap which maybe could do stuff like that, I am not sure as >>> I never tested it. At least it could do some stuff top of I2C bus. >> >> Yes, I've read this discussion, but didn't have time to take a deeper >> look into the regmap stuff yet. >> >> For the em28xx driver itself, there is no real need for i2c block >> read/write emulation at the moment. We could save only a few lines. >> I'm also burried with lots of other stuff at the moment which has a >> higher priority for me. >> >> Please note that the whole discussion has nothing to do with this patch. >> It just removes code which isn't and has never been working. >> >>> >>> Also I heavily disagree you what goes to I2C subsystem integration. >>> That is clearly stuff which resides top of I2C bus and it is *not bus >>> dependent*. There is many other buses too having similar splitting >>> logic like SPI? >>> >> >> I don't understand you. In which points do we disagree ?? > > "My feeling is, that this should be handled by the i2c subsystem as > much as possible" Maybe I should have said "as much as makes sense" ;) To be more precise: I think it's always good to have as much common code as possible. And of course the complexity of the code must be justified by it's benefits. Do you agree ? Frank > > Antti > > -- 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
