Re: i2c bit banging timeout for SCL
Better late, than never: https://reviews.freebsd.org/D22109 This implements the configurable SCL low timeout with a new default and most of the discussed changes plus a little bit more. If you are interested, please feel free to join the review if you are not a reviewer already. -- Andriy Gapon ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: i2c bit banging timeout for SCL
On Mon, 2019-07-01 at 11:25 -0600, Warner Losh wrote: > On Mon, Jul 1, 2019 at 11:14 AM Poul-Henning Kamp > > wrote: > > > > > In message < > > canczdfofbvmxptnel4goqxtvp6zd-xrtja4rmuo1racy0jd...@mail.gmail.com> > > , > > Warner Losh writes: > > > > > The only issue, really, is that this timeout is a busy loop and > > > there may > > > be I/O bus contention introduced on these systems. > > > > Does it have to be a busy loop for the entire duration ? > > > > Spin for the median, timeout+poll for the rest of the time ? > > > > That's a good suggestion. I'd be inclined to spin for 1 tick or so, > then do > a timeout per tick after that (eg, shift from DELAY to pause(1)). It > won't > be super accurate or high performance, but when the devices are slow, > that > would add only a little extra time. > > Ideally, that's what we'd do. In the short term, bumping the timeout > wouldn't be horrible. > > Warner Most of the DELAY() in i2c bitbang is just the idle time before toggling the clock line to achieve the 100khz bus rate. That's a 10us delay, and on modern hardware those delays should be pause() calls because that's enough time to get useful work done. When polling for ack at the end of a byte, using a DELAY(1) loop makes more sense (actually, just polling without delay may make even more sense, since DELAY() is generally just polling a clock register). Hmm, actually, it looks like iicbb hardcodes the bus frequency delay as 10us and delays after every toggle, so I guess it's really running the bus at 50khz. -- Ian ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: i2c bit banging timeout for SCL
On Mon, Jul 1, 2019 at 11:14 AM Poul-Henning Kamp wrote: > > In message < > canczdfofbvmxptnel4goqxtvp6zd-xrtja4rmuo1racy0jd...@mail.gmail.com>, > Warner Losh writes: > > >The only issue, really, is that this timeout is a busy loop and there may > >be I/O bus contention introduced on these systems. > > Does it have to be a busy loop for the entire duration ? > > Spin for the median, timeout+poll for the rest of the time ? > That's a good suggestion. I'd be inclined to spin for 1 tick or so, then do a timeout per tick after that (eg, shift from DELAY to pause(1)). It won't be super accurate or high performance, but when the devices are slow, that would add only a little extra time. Ideally, that's what we'd do. In the short term, bumping the timeout wouldn't be horrible. Warner ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: i2c bit banging timeout for SCL
In message , Warner Losh writes: >The only issue, really, is that this timeout is a busy loop and there may >be I/O bus contention introduced on these systems. Does it have to be a busy loop for the entire duration ? Spin for the median, timeout+poll for the rest of the time ? -- Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 p...@freebsd.org | TCP/IP since RFC 956 FreeBSD committer | BSD since 4.3-tahoe Never attribute to malice what can adequately be explained by incompetence. ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: i2c bit banging timeout for SCL
On Mon, Jul 1, 2019 at 10:30 AM Ian Lepore wrote: > On Mon, 2019-07-01 at 19:03 +0300, Andriy Gapon wrote: > > iicbb driver has a hardcoded timeout that defines how long the driver > waits for > > SCL line to go high after the driver releases it to float. Sometimes > slaves > > hold the line low until they are ready to continue with the > communication. > > As a side note, the timeout means that the driver just goes on as if the > line > > became high. Maybe it should produce an error instead. > > > > Anyway, I would like to increase the current timeout of 100 x 10us to > 1000 x > > 10us. The rationale is that there are many slave devices, like sensors, > that > > take about 10 ms to return a result. So, I think that it makes sense to > play > > nice with such devices by default. > > > > Probably I'll add a sysctl for that parameter while I'll be there. > > > > Any objections? > > Many (perhaps most?) modern i2c slave devices are both i2c and smbus > compatible, and the smbus slave timeout is 35ms, so that wouldn't be a > bad default value. > I'd concur here. Someone else will have a device that takes 20ms to reply and wonder why we're broken... The only issue, really, is that this timeout is a busy loop and there may be I/O bus contention introduced on these systems. My gut tells me that we should look out for that, but bump the default timeout to the limit of the spec we implement. There's no I2C limit, and there is a smbus one, so let's go with the latter. Most of the time, most of the devices will react within a millisecond anyway, which isn't terrible for a bit-banged situation. > I don't think ignoring the error and forging ahead is a good idea. It > should return an error, and perhaps it should use the bitbang bus- > recovery sequence (from iic_recover_bus.c) to unwedge the slave device. > Also agreed. Warner ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: i2c bit banging timeout for SCL
On Mon, 2019-07-01 at 19:03 +0300, Andriy Gapon wrote: > iicbb driver has a hardcoded timeout that defines how long the driver waits > for > SCL line to go high after the driver releases it to float. Sometimes slaves > hold the line low until they are ready to continue with the communication. > As a side note, the timeout means that the driver just goes on as if the line > became high. Maybe it should produce an error instead. > > Anyway, I would like to increase the current timeout of 100 x 10us to 1000 x > 10us. The rationale is that there are many slave devices, like sensors, that > take about 10 ms to return a result. So, I think that it makes sense to play > nice with such devices by default. > > Probably I'll add a sysctl for that parameter while I'll be there. > > Any objections? Many (perhaps most?) modern i2c slave devices are both i2c and smbus compatible, and the smbus slave timeout is 35ms, so that wouldn't be a bad default value. I don't think ignoring the error and forging ahead is a good idea. It should return an error, and perhaps it should use the bitbang bus- recovery sequence (from iic_recover_bus.c) to unwedge the slave device. -- Ian ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: i2c bit banging timeout for SCL
On 01/07/2019 19:16, Poul-Henning Kamp wrote: > > In message , Andriy Gapon > writes: > >> iicbb driver has a hardcoded timeout that defines how long the driver waits >> for >> SCL line to go high after the driver releases it to float. Sometimes slaves >> hold the line low until they are ready to continue with the communication. >> As a side note, the timeout means that the driver just goes on as if the line >> became high. Maybe it should produce an error instead. >> >> Anyway, I would like to increase the current timeout of 100 x 10us to 1000 x >> 10us. The rationale is that there are many slave devices, like sensors, that >> take about 10 ms to return a result. So, I think that it makes sense to play >> nice with such devices by default. >> >> Probably I'll add a sysctl for that parameter while I'll be there. > > sysctl or ioctl ? An ioctl that could set the value per slave sounds like a very good idea. But it's a little bit more work than I planned for now, so it's a sysctl to set the default global value. -- Andriy Gapon ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: i2c bit banging timeout for SCL
In message , Andriy Gapon writes: >iicbb driver has a hardcoded timeout that defines how long the driver waits for > SCL line to go high after the driver releases it to float. Sometimes slaves >hold the line low until they are ready to continue with the communication. >As a side note, the timeout means that the driver just goes on as if the line >became high. Maybe it should produce an error instead. > >Anyway, I would like to increase the current timeout of 100 x 10us to 1000 x >10us. The rationale is that there are many slave devices, like sensors, that >take about 10 ms to return a result. So, I think that it makes sense to play >nice with such devices by default. > >Probably I'll add a sysctl for that parameter while I'll be there. sysctl or ioctl ? -- Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 p...@freebsd.org | TCP/IP since RFC 956 FreeBSD committer | BSD since 4.3-tahoe Never attribute to malice what can adequately be explained by incompetence. ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
i2c bit banging timeout for SCL
iicbb driver has a hardcoded timeout that defines how long the driver waits for SCL line to go high after the driver releases it to float. Sometimes slaves hold the line low until they are ready to continue with the communication. As a side note, the timeout means that the driver just goes on as if the line became high. Maybe it should produce an error instead. Anyway, I would like to increase the current timeout of 100 x 10us to 1000 x 10us. The rationale is that there are many slave devices, like sensors, that take about 10 ms to return a result. So, I think that it makes sense to play nice with such devices by default. Probably I'll add a sysctl for that parameter while I'll be there. Any objections? -- Andriy Gapon ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
