You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mynewt.apache.org by Sterling Hughes <st...@apache.org> on 2016/08/24 23:37:05 UTC
HAL & I2C redux
Hi,
While we’re doing the HAL changes, I’d like folks to check out the
I2C APIs, and provide any comments they have:
https://github.com/apache/incubator-mynewt-core/blob/sterly_refactor/hw/hal/include/hal/hal_i2c.h
I was relatively happy with these APIs, except for maybe the
hal_i2c_master_data: I think write/read should just take these three as
arguments: but I didn’t see this as significant enough to change.
However, the one difference between SPI (new SPI HAL APIs) and UART is
that I2C does not have a non-blocking mode. I didn’t think this was a
huge issue, because I assume that I2C is likely going to be mostly slow
communication in a low-priority task context. However, I’m interested
to know if folks think an interrupt based/non-blocking API is desirable
for I2C, and worth the implementation overhead for people developing the
HAL.
Also, folks should feel free to review the new HAL which is here:
https://github.com/apache/incubator-mynewt-core/tree/sterly_refactor/hw/hal/include/hal
With the caveat that Will’s new SPI HAL interface isn’t committed,
and we are missing a watchdog HAL (coming soon.)
Sterling
Re: HAL & I2C redux
Posted by marko kiiskila <ma...@runtime.io>.
Hi,
I’d like to have an option to timeout the blocking operations.
> On Aug 25, 2016, at 9:38 AM, will sanfilippo <wi...@runtime.io> wrote:
>
> Hello:
>
> Comments:
>
> +1 on no non-blocking interface. Adding a non-blocking interface later (for those desiring one) should not be terribly difficult.
>
> The API seem reasonable although I am no i2c expert by any means.
>
>
>> On Aug 24, 2016, at 4:37 PM, Sterling Hughes <st...@apache.org> wrote:
>>
>> Hi,
>>
>> While we’re doing the HAL changes, I’d like folks to check out the I2C APIs, and provide any comments they have:
>>
>> https://github.com/apache/incubator-mynewt-core/blob/sterly_refactor/hw/hal/include/hal/hal_i2c.h
>>
>> I was relatively happy with these APIs, except for maybe the hal_i2c_master_data: I think write/read should just take these three as arguments: but I didn’t see this as significant enough to change.
>>
>> However, the one difference between SPI (new SPI HAL APIs) and UART is that I2C does not have a non-blocking mode. I didn’t think this was a huge issue, because I assume that I2C is likely going to be mostly slow communication in a low-priority task context. However, I’m interested to know if folks think an interrupt based/non-blocking API is desirable for I2C, and worth the implementation overhead for people developing the HAL.
>>
>> Also, folks should feel free to review the new HAL which is here:
>>
>> https://github.com/apache/incubator-mynewt-core/tree/sterly_refactor/hw/hal/include/hal
>>
>> With the caveat that Will’s new SPI HAL interface isn’t committed, and we are missing a watchdog HAL (coming soon.)
>>
>>
>>
>> Sterling
>
Re: HAL & I2C redux
Posted by will sanfilippo <wi...@runtime.io>.
Hello:
Comments:
+1 on no non-blocking interface. Adding a non-blocking interface later (for those desiring one) should not be terribly difficult.
The API seem reasonable although I am no i2c expert by any means.
> On Aug 24, 2016, at 4:37 PM, Sterling Hughes <st...@apache.org> wrote:
>
> Hi,
>
> While we’re doing the HAL changes, I’d like folks to check out the I2C APIs, and provide any comments they have:
>
> https://github.com/apache/incubator-mynewt-core/blob/sterly_refactor/hw/hal/include/hal/hal_i2c.h
>
> I was relatively happy with these APIs, except for maybe the hal_i2c_master_data: I think write/read should just take these three as arguments: but I didn’t see this as significant enough to change.
>
> However, the one difference between SPI (new SPI HAL APIs) and UART is that I2C does not have a non-blocking mode. I didn’t think this was a huge issue, because I assume that I2C is likely going to be mostly slow communication in a low-priority task context. However, I’m interested to know if folks think an interrupt based/non-blocking API is desirable for I2C, and worth the implementation overhead for people developing the HAL.
>
> Also, folks should feel free to review the new HAL which is here:
>
> https://github.com/apache/incubator-mynewt-core/tree/sterly_refactor/hw/hal/include/hal
>
> With the caveat that Will’s new SPI HAL interface isn’t committed, and we are missing a watchdog HAL (coming soon.)
>
>
>
> Sterling
Re: HAL & I2C redux
Posted by Kevin Townsend <ke...@adafruit.com>.
I'd have to look at all the datasheets for the support MCUs, but some
MCUs definitely don't play well with clock stretching, I just wish I
could remember some examples quickly. We've run into problems with
popular SoCs or ICs like the BNO055, which use CS.
See this for example for the ESP (though this is just from a quick
search on Google, I haven't tried):
http://www.esp8266.com/viewtopic.php?p=41401
K.
On 25/08/16 20:14, marko kiiskila wrote:
> I was wondering about clock stretching as well, and if it all targets
> do it properly. If not, maybe there should be a way of controlling
> the data clocking speed. At the moment I read the API such that the
> MCU would try to clock out data as fast as it can?
>
>> On Aug 25, 2016, at 10:45 AM, Kevin Townsend <ke...@adafruit.com> wrote:
>>
>> Hi Sterling (et al),
>>
>> start() and stop() and probably clearer to anyone familiar with I2C, and it removes any ambiguity about if cached data is also being sent. Or the single control() command with a parameter seems fine as well, though that's just my opinion.
>>
>> Something concerning blocking or non blocking calls as well ... I think having a user-defined timeout makes sense (as Marko mentioned) with an appropriate return code on timeout, but the API should also keep clock stretching in mind (http://www.i2c-bus.org/clock-stretching/). I'm not sure what the solution would be, though. If the I2C slave uses clock stretching and is holding the clock low, that's still in a valid state for the transaction, but what takes priority if the timeout expires first? I suppose the explicit timeout should take precedence and the transaction should be aborted, but it's worth considering. We've also had issues with clock stretching on some HW since not every I2C peripheral on every MCU handles it properly either ... it might be worth having a flag and the MCU abstraction level to indicate of the I2C peripheral supports clock stretching, just to know when working with drivers that require it.
>>
>> K.
>>
>>
>> On 25/08/16 19:36, Sterling Hughes wrote:
>>> Hi Kevin,
>>>
>>> On 25 Aug 2016, at 9:49, Kevin Townsend wrote:
>>>
>>>> Although, this will also depend on how things are implemented in the .c code ... I only see the header at present. But from experience some sensors require the stop to be handled differently between multiple read or writes, so it's worth considering how to keep the STOP condition flexible and under user control since there isn't a one size fits all approach for every I2C sensor out there.
>>>>
>>>>
>>> Yeah, most of the vendor APIs I see have the stop bit as an option (or by default) after a write() command, whereas our HAL APIs use begin() and end() to generate stop conditions. Frankly, I think it might make more sense to label these start() and stop(), because a lot of the I2C protocols can have multiple start conditions and multiple stop conditions. Or maybe a control() command that takes either START or STOP, to make it clear that it\u2019s not modifying transaction state in anyway.
>>>
>>> Sterling
Re: HAL & I2C redux
Posted by marko kiiskila <ma...@runtime.io>.
I was wondering about clock stretching as well, and if it all targets
do it properly. If not, maybe there should be a way of controlling
the data clocking speed. At the moment I read the API such that the
MCU would try to clock out data as fast as it can?
> On Aug 25, 2016, at 10:45 AM, Kevin Townsend <ke...@adafruit.com> wrote:
>
> Hi Sterling (et al),
>
> start() and stop() and probably clearer to anyone familiar with I2C, and it removes any ambiguity about if cached data is also being sent. Or the single control() command with a parameter seems fine as well, though that's just my opinion.
>
> Something concerning blocking or non blocking calls as well ... I think having a user-defined timeout makes sense (as Marko mentioned) with an appropriate return code on timeout, but the API should also keep clock stretching in mind (http://www.i2c-bus.org/clock-stretching/). I'm not sure what the solution would be, though. If the I2C slave uses clock stretching and is holding the clock low, that's still in a valid state for the transaction, but what takes priority if the timeout expires first? I suppose the explicit timeout should take precedence and the transaction should be aborted, but it's worth considering. We've also had issues with clock stretching on some HW since not every I2C peripheral on every MCU handles it properly either ... it might be worth having a flag and the MCU abstraction level to indicate of the I2C peripheral supports clock stretching, just to know when working with drivers that require it.
>
> K.
>
>
> On 25/08/16 19:36, Sterling Hughes wrote:
>> Hi Kevin,
>>
>> On 25 Aug 2016, at 9:49, Kevin Townsend wrote:
>>
>>> Although, this will also depend on how things are implemented in the .c code ... I only see the header at present. But from experience some sensors require the stop to be handled differently between multiple read or writes, so it's worth considering how to keep the STOP condition flexible and under user control since there isn't a one size fits all approach for every I2C sensor out there.
>>>
>>>
>>
>> Yeah, most of the vendor APIs I see have the stop bit as an option (or by default) after a write() command, whereas our HAL APIs use begin() and end() to generate stop conditions. Frankly, I think it might make more sense to label these start() and stop(), because a lot of the I2C protocols can have multiple start conditions and multiple stop conditions. Or maybe a control() command that takes either START or STOP, to make it clear that it’s not modifying transaction state in anyway.
>>
>> Sterling
>
Re: HAL & I2C redux
Posted by Kevin Townsend <ke...@adafruit.com>.
Hi Sterling (et al),
start() and stop() and probably clearer to anyone familiar with I2C, and
it removes any ambiguity about if cached data is also being sent. Or the
single control() command with a parameter seems fine as well, though
that's just my opinion.
Something concerning blocking or non blocking calls as well ... I think
having a user-defined timeout makes sense (as Marko mentioned) with an
appropriate return code on timeout, but the API should also keep clock
stretching in mind (http://www.i2c-bus.org/clock-stretching/). I'm not
sure what the solution would be, though. If the I2C slave uses clock
stretching and is holding the clock low, that's still in a valid state
for the transaction, but what takes priority if the timeout expires
first? I suppose the explicit timeout should take precedence and the
transaction should be aborted, but it's worth considering. We've also
had issues with clock stretching on some HW since not every I2C
peripheral on every MCU handles it properly either ... it might be worth
having a flag and the MCU abstraction level to indicate of the I2C
peripheral supports clock stretching, just to know when working with
drivers that require it.
K.
On 25/08/16 19:36, Sterling Hughes wrote:
> Hi Kevin,
>
> On 25 Aug 2016, at 9:49, Kevin Townsend wrote:
>
>> Although, this will also depend on how things are implemented in the
>> .c code ... I only see the header at present. But from experience
>> some sensors require the stop to be handled differently between
>> multiple read or writes, so it's worth considering how to keep the
>> STOP condition flexible and under user control since there isn't a
>> one size fits all approach for every I2C sensor out there.
>>
>>
>
> Yeah, most of the vendor APIs I see have the stop bit as an option
> (or by default) after a write() command, whereas our HAL APIs use
> begin() and end() to generate stop conditions. Frankly, I think it
> might make more sense to label these start() and stop(), because a lot
> of the I2C protocols can have multiple start conditions and multiple
> stop conditions. Or maybe a control() command that takes either START
> or STOP, to make it clear that it\u2019s not modifying transaction state in
> anyway.
>
> Sterling
Re: HAL & I2C redux
Posted by "David G. Simmons" <sa...@mac.com>.
> On Aug 25, 2016, at 1:36 PM, Sterling Hughes <st...@apache.org> wrote:
>
> Hi Kevin,
>
> On 25 Aug 2016, at 9:49, Kevin Townsend wrote:
>
>> Although, this will also depend on how things are implemented in the .c code ... I only see the header at present. But from experience some sensors require the stop to be handled differently between multiple read or writes, so it's worth considering how to keep the STOP condition flexible and under user control since there isn't a one size fits all approach for every I2C sensor out there.
>>
>>
>
> Yeah, most of the vendor APIs I see have the stop bit as an option (or by default) after a write() command, whereas our HAL APIs use begin() and end() to generate stop conditions. Frankly, I think it might make more sense to label these start() and stop(), because a lot of the I2C protocols can have multiple start conditions and multiple stop conditions. Or maybe a control() command that takes either START or STOP, to make it clear that it’s not modifying transaction state in anyway.
>
> Sterling
+1 on calling them start() and stop().
dg
--
David G. Simmons
(919) 534-5099
Web • Blog • Linkedin • Twitter • GitHub
/** Message digitally signed for security and authenticity.
* If you cannot read the PGP.sig attachment, please go to
* http://www.gnupg.com/ Secure your email!!!
* Public key available at keyserver.pgp.com
**/
♺ This email uses 100% recycled electrons. Don't blow it by printing!
There are only 2 hard things in computer science: Cache invalidation, naming things, and off-by-one errors.
Re: HAL & I2C redux
Posted by Sterling Hughes <st...@apache.org>.
Hi Kevin,
On 25 Aug 2016, at 9:49, Kevin Townsend wrote:
> Although, this will also depend on how things are implemented in the
> .c code ... I only see the header at present. But from experience some
> sensors require the stop to be handled differently between multiple
> read or writes, so it's worth considering how to keep the STOP
> condition flexible and under user control since there isn't a one size
> fits all approach for every I2C sensor out there.
>
>
Yeah, most of the vendor APIs I see have the stop bit as an option (or
by default) after a write() command, whereas our HAL APIs use begin()
and end() to generate stop conditions. Frankly, I think it might make
more sense to label these start() and stop(), because a lot of the I2C
protocols can have multiple start conditions and multiple stop
conditions. Or maybe a control() command that takes either START or
STOP, to make it clear that it\u2019s not modifying transaction state in
anyway.
Sterling
Re: HAL & I2C redux
Posted by Kevin Townsend <ke...@adafruit.com>.
Although, this will also depend on how things are implemented in the .c
code ... I only see the header at present. But from experience some
sensors require the stop to be handled differently between multiple read
or writes, so it's worth considering how to keep the STOP condition
flexible and under user control since there isn't a one size fits all
approach for every I2C sensor out there.
On 25/08/16 18:46, Kevin Townsend wrote:
> You might want to consider how to handle the STOP condition ... some
> sensors have different behaviour about when the stop condition should
> or shouldn't be set. See the Arduino Wire library for example, who
> make the stop condition optional when you end the transaction:
> https://www.arduino.cc/en/Reference/WireEndTransmission
>
> Kevin
>
>
> On 25/08/16 01:37, Sterling Hughes wrote:
>> Hi,
>>
>> While we\u2019re doing the HAL changes, I\u2019d like folks to check out the
>> I2C APIs, and provide any comments they have:
>>
>> https://github.com/apache/incubator-mynewt-core/blob/sterly_refactor/hw/hal/include/hal/hal_i2c.h
>>
>>
>> I was relatively happy with these APIs, except for maybe the
>> hal_i2c_master_data: I think write/read should just take these three
>> as arguments: but I didn\u2019t see this as significant enough to change.
>>
>> However, the one difference between SPI (new SPI HAL APIs) and UART
>> is that I2C does not have a non-blocking mode. I didn\u2019t think this
>> was a huge issue, because I assume that I2C is likely going to be
>> mostly slow communication in a low-priority task context. However,
>> I\u2019m interested to know if folks think an interrupt based/non-blocking
>> API is desirable for I2C, and worth the implementation overhead for
>> people developing the HAL.
>>
>> Also, folks should feel free to review the new HAL which is here:
>>
>> https://github.com/apache/incubator-mynewt-core/tree/sterly_refactor/hw/hal/include/hal
>>
>>
>> With the caveat that Will\u2019s new SPI HAL interface isn\u2019t committed,
>> and we are missing a watchdog HAL (coming soon.)
>>
>>
>>
>> Sterling
>
Re: HAL & I2C redux
Posted by Kevin Townsend <ke...@adafruit.com>.
You might want to consider how to handle the STOP condition ... some
sensors have different behaviour about when the stop condition should or
shouldn't be set. See the Arduino Wire library for example, who make
the stop condition optional when you end the transaction:
https://www.arduino.cc/en/Reference/WireEndTransmission
Kevin
On 25/08/16 01:37, Sterling Hughes wrote:
> Hi,
>
> While we\u2019re doing the HAL changes, I\u2019d like folks to check out the I2C
> APIs, and provide any comments they have:
>
> https://github.com/apache/incubator-mynewt-core/blob/sterly_refactor/hw/hal/include/hal/hal_i2c.h
>
>
> I was relatively happy with these APIs, except for maybe the
> hal_i2c_master_data: I think write/read should just take these three
> as arguments: but I didn\u2019t see this as significant enough to change.
>
> However, the one difference between SPI (new SPI HAL APIs) and UART is
> that I2C does not have a non-blocking mode. I didn\u2019t think this was a
> huge issue, because I assume that I2C is likely going to be mostly
> slow communication in a low-priority task context. However, I\u2019m
> interested to know if folks think an interrupt based/non-blocking API
> is desirable for I2C, and worth the implementation overhead for people
> developing the HAL.
>
> Also, folks should feel free to review the new HAL which is here:
>
> https://github.com/apache/incubator-mynewt-core/tree/sterly_refactor/hw/hal/include/hal
>
>
> With the caveat that Will\u2019s new SPI HAL interface isn\u2019t committed, and
> we are missing a watchdog HAL (coming soon.)
>
>
>
> Sterling
Re: HAL & I2C redux
Posted by Sterling Hughes <st...@apache.org>.
On 25 Aug 2016, at 14:00, Sterling Hughes wrote:
> Hi,
>
> On 25 Aug 2016, at 9:48, St�phane D'Alu wrote:
>
>> Hi,
>>
>> You could want to take a look at others.
>> ChibiOS has a nice and clean API.
>>
>>
>
> I have taken a look at quite a few (Zephyr, mbed-hal). I just took a
> look at ChibiOS: I\u2019ll refer to them more in the past. I haven\u2019t
> paid much attention to them, because I thought they were GPL: but it
^^^^^^^^
future
:)
Re: HAL & I2C redux
Posted by Sterling Hughes <st...@apache.org>.
Hi,
On 25 Aug 2016, at 9:48, St�phane D'Alu wrote:
> Hi,
>
> You could want to take a look at others.
> ChibiOS has a nice and clean API.
>
>
I have taken a look at quite a few (Zephyr, mbed-hal). I just took a
look at ChibiOS: I\u2019ll refer to them more in the past. I haven\u2019t
paid much attention to them, because I thought they were GPL: but it
looks like all their HAL stuff is Apache licensed, which is interesting
to see! I generally like the design choices I saw there.
Sterling
Re: HAL & I2C redux
Posted by Stéphane D'Alu <sd...@sdalu.com>.
Hi,
You could want to take a look at others.
ChibiOS has a nice and clean API.
On 25 August 2016 01:37:05 CEST, Sterling Hughes <st...@apache.org> wrote:
>Hi,
>
>While we\u2019re doing the HAL changes, I\u2019d like folks to check out the
>I2C APIs, and provide any comments they have:
>
>https://github.com/apache/incubator-mynewt-core/blob/sterly_refactor/hw/hal/include/hal/hal_i2c.h
>
>I was relatively happy with these APIs, except for maybe the
>hal_i2c_master_data: I think write/read should just take these three as
>
>arguments: but I didn\u2019t see this as significant enough to change.
>
>However, the one difference between SPI (new SPI HAL APIs) and UART is
>that I2C does not have a non-blocking mode. I didn\u2019t think this was a
>huge issue, because I assume that I2C is likely going to be mostly slow
>
>communication in a low-priority task context. However, I\u2019m interested
>to know if folks think an interrupt based/non-blocking API is desirable
>
>for I2C, and worth the implementation overhead for people developing
>the
>HAL.
>
>Also, folks should feel free to review the new HAL which is here:
>
>https://github.com/apache/incubator-mynewt-core/tree/sterly_refactor/hw/hal/include/hal
>
>With the caveat that Will\u2019s new SPI HAL interface isn\u2019t committed,
>and we are missing a watchdog HAL (coming soon.)
>
>
>
>Sterling
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
Re: HAL & I2C redux
Posted by will sanfilippo <wi...@runtime.io>.
+1 on HAL timer and cputime being part of the OS and moved to the OS.
> On Aug 24, 2016, at 4:43 PM, Sterling Hughes <st...@apache.org> wrote:
>
> Oh, I should add to this:
>
> I think we’re missing a timer interface HAL and a cputime itself should move from being a HAL to OS service. cputime should use the timer HAL IMO.
>
> sterling
>
> On 24 Aug 2016, at 16:37, Sterling Hughes wrote:
>
>> Hi,
>>
>> While we’re doing the HAL changes, I’d like folks to check out the I2C APIs, and provide any comments they have:
>>
>> https://github.com/apache/incubator-mynewt-core/blob/sterly_refactor/hw/hal/include/hal/hal_i2c.h
>>
>> I was relatively happy with these APIs, except for maybe the hal_i2c_master_data: I think write/read should just take these three as arguments: but I didn’t see this as significant enough to change.
>>
>> However, the one difference between SPI (new SPI HAL APIs) and UART is that I2C does not have a non-blocking mode. I didn’t think this was a huge issue, because I assume that I2C is likely going to be mostly slow communication in a low-priority task context. However, I’m interested to know if folks think an interrupt based/non-blocking API is desirable for I2C, and worth the implementation overhead for people developing the HAL.
>>
>> Also, folks should feel free to review the new HAL which is here:
>>
>> https://github.com/apache/incubator-mynewt-core/tree/sterly_refactor/hw/hal/include/hal
>>
>> With the caveat that Will’s new SPI HAL interface isn’t committed, and we are missing a watchdog HAL (coming soon.)
>>
>>
>>
>> Sterling
Re: HAL & I2C redux
Posted by Sterling Hughes <st...@apache.org>.
Oh, I should add to this:
I think we’re missing a timer interface HAL and a cputime itself
should move from being a HAL to OS service. cputime should use the
timer HAL IMO.
sterling
On 24 Aug 2016, at 16:37, Sterling Hughes wrote:
> Hi,
>
> While we’re doing the HAL changes, I’d like folks to check out the
> I2C APIs, and provide any comments they have:
>
> https://github.com/apache/incubator-mynewt-core/blob/sterly_refactor/hw/hal/include/hal/hal_i2c.h
>
> I was relatively happy with these APIs, except for maybe the
> hal_i2c_master_data: I think write/read should just take these three
> as arguments: but I didn’t see this as significant enough to change.
>
> However, the one difference between SPI (new SPI HAL APIs) and UART is
> that I2C does not have a non-blocking mode. I didn’t think this was
> a huge issue, because I assume that I2C is likely going to be mostly
> slow communication in a low-priority task context. However, I’m
> interested to know if folks think an interrupt based/non-blocking API
> is desirable for I2C, and worth the implementation overhead for people
> developing the HAL.
>
> Also, folks should feel free to review the new HAL which is here:
>
> https://github.com/apache/incubator-mynewt-core/tree/sterly_refactor/hw/hal/include/hal
>
> With the caveat that Will’s new SPI HAL interface isn’t committed,
> and we are missing a watchdog HAL (coming soon.)
>
>
>
> Sterling