Am 02.01.2013 22:40, schrieb Antti Palosaari: > On 01/02/2013 11:29 PM, Frank Schäfer wrote: >> 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 ? > > Common code is of course what it should be. Integrating that splitting > logic to I2C subsystem is not common in the meaning as there is other > buses than I2C as well. That splitting logic is something which is not > that much bus specific. Ah ! Yes, maybe it makes sense to place this even on a higher level/layer. Maybe regmap is already what we are looking for (or can be extended). 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
