Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 5612738

Browse files
Tech-TXdevyte
Tech-TX
authored and
devyte
committedDec 15, 2019
change to make inline helpers truly inline [issue 6875] (#6898)
* change to make inline helpers truly inline [issue 6875] * pulled the inline helpers out of the TWI class [issue 6875] * removed some inlines causing issues [issue 6875] * removed 2 more inlines from slave timeout section [issue 6875] * removed 2 more inline attributes on public functions, moved twi_scl_valley up into the master section [issue 6875]
1 parent 1d05283 commit 5612738

File tree

1 file changed

+94
-89
lines changed

1 file changed

+94
-89
lines changed
 

‎cores/esp8266/core_esp8266_si2c.cpp

Lines changed: 94 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,32 @@ extern "C" {
3131
#include "ets_sys.h"
3232
};
3333

34+
// Inline helpers
35+
static inline __attribute__((always_inline)) void SDA_LOW(const int twi_sda)
36+
{
37+
GPES = (1 << twi_sda);
38+
}
39+
static inline __attribute__((always_inline)) void SDA_HIGH(const int twi_sda)
40+
{
41+
GPEC = (1 << twi_sda);
42+
}
43+
static inline __attribute__((always_inline)) bool SDA_READ(const int twi_sda)
44+
{
45+
return (GPI & (1 << twi_sda)) != 0;
46+
}
47+
static inline __attribute__((always_inline)) void SCL_LOW(const int twi_scl)
48+
{
49+
GPES = (1 << twi_scl);
50+
}
51+
static inline __attribute__((always_inline)) void SCL_HIGH(const int twi_scl)
52+
{
53+
GPEC = (1 << twi_scl);
54+
}
55+
static inline __attribute__((always_inline)) bool SCL_READ(const int twi_scl)
56+
{
57+
return (GPI & (1 << twi_scl)) != 0;
58+
}
59+
3460

3561
// Implement as a class to reduce code size by allowing access to many global variables with a single base pointer
3662
class Twi
@@ -95,37 +121,12 @@ class Twi
95121
unsigned char read_byte(bool nack);
96122
void ICACHE_RAM_ATTR onTwipEvent(uint8_t status);
97123

98-
// Inline helpers
99-
inline void SDA_LOW()
100-
{
101-
GPES = (1 << twi_sda);
102-
}
103-
inline void SDA_HIGH()
104-
{
105-
GPEC = (1 << twi_sda);
106-
}
107-
inline bool SDA_READ()
108-
{
109-
return (GPI & (1 << twi_sda)) != 0;
110-
}
111-
inline void SCL_LOW()
112-
{
113-
GPES = (1 << twi_scl);
114-
}
115-
inline void SCL_HIGH()
116-
{
117-
GPEC = (1 << twi_scl);
118-
}
119-
inline bool SCL_READ()
120-
{
121-
return (GPI & (1 << twi_scl)) != 0;
122-
}
123124
// Handle the case where a slave needs to stretch the clock with a time-limited busy wait
124125
inline void WAIT_CLOCK_STRETCH()
125126
{
126127
esp8266::polledTimeout::oneShotFastUs timeout(twi_clockStretchLimit);
127128
esp8266::polledTimeout::periodicFastUs yieldTimeout(5000);
128-
while(!timeout && !SCL_READ()) // outer loop is stretch duration up to stretch limit
129+
while(!timeout && !SCL_READ(twi_scl)) // outer loop is stretch duration up to stretch limit
129130
{
130131
if (yieldTimeout) // inner loop yields every 5ms
131132
yield();
@@ -146,9 +147,9 @@ class Twi
146147
uint8_t transmit(const uint8_t* data, uint8_t length);
147148
void attachSlaveRxEvent(void (*function)(uint8_t*, size_t));
148149
void attachSlaveTxEvent(void (*function)(void));
149-
inline void ICACHE_RAM_ATTR reply(uint8_t ack);
150-
inline void ICACHE_RAM_ATTR stop(void);
151-
inline void ICACHE_RAM_ATTR releaseBus(void);
150+
void ICACHE_RAM_ATTR reply(uint8_t ack);
151+
void ICACHE_RAM_ATTR stop(void);
152+
void ICACHE_RAM_ATTR releaseBus(void);
152153
void enableSlave();
153154
};
154155

@@ -277,57 +278,57 @@ void ICACHE_RAM_ATTR Twi::busywait(unsigned char v)
277278

278279
bool Twi::write_start(void)
279280
{
280-
SCL_HIGH();
281-
SDA_HIGH();
282-
if (!SDA_READ())
281+
SCL_HIGH(twi_scl);
282+
SDA_HIGH(twi_sda);
283+
if (!SDA_READ(twi_sda))
283284
{
284285
return false;
285286
}
286287
busywait(twi_dcount);
287-
SDA_LOW();
288+
SDA_LOW(twi_sda);
288289
busywait(twi_dcount);
289290
return true;
290291
}
291292

292293
bool Twi::write_stop(void)
293294
{
294-
SCL_LOW();
295-
SDA_LOW();
295+
SCL_LOW(twi_scl);
296+
SDA_LOW(twi_sda);
296297
busywait(twi_dcount);
297-
SCL_HIGH();
298+
SCL_HIGH(twi_scl);
298299
WAIT_CLOCK_STRETCH();
299300
busywait(twi_dcount);
300-
SDA_HIGH();
301+
SDA_HIGH(twi_sda);
301302
busywait(twi_dcount);
302303
return true;
303304
}
304305

305306
bool Twi::write_bit(bool bit)
306307
{
307-
SCL_LOW();
308+
SCL_LOW(twi_scl);
308309
if (bit)
309310
{
310-
SDA_HIGH();
311+
SDA_HIGH(twi_sda);
311312
}
312313
else
313314
{
314-
SDA_LOW();
315+
SDA_LOW(twi_sda);
315316
}
316317
busywait(twi_dcount + 1);
317-
SCL_HIGH();
318+
SCL_HIGH(twi_scl);
318319
WAIT_CLOCK_STRETCH();
319320
busywait(twi_dcount);
320321
return true;
321322
}
322323

323324
bool Twi::read_bit(void)
324325
{
325-
SCL_LOW();
326-
SDA_HIGH();
326+
SCL_LOW(twi_scl);
327+
SDA_HIGH(twi_sda);
327328
busywait(twi_dcount + 2);
328-
SCL_HIGH();
329+
SCL_HIGH(twi_scl);
329330
WAIT_CLOCK_STRETCH();
330-
bool bit = SDA_READ();
331+
bool bit = SDA_READ(twi_sda);
331332
busywait(twi_dcount);
332333
return bit;
333334
}
@@ -392,7 +393,7 @@ unsigned char Twi::writeTo(unsigned char address, unsigned char * buf, unsigned
392393
// busywait(twi_dcount);
393394
}
394395
i = 0;
395-
while (!SDA_READ() && (i++) < 10)
396+
while (!SDA_READ(twi_sda) && (i++) < 10)
396397
{
397398
twi_scl_valley();
398399
busywait(twi_dcount);
@@ -431,32 +432,40 @@ unsigned char Twi::readFrom(unsigned char address, unsigned char* buf, unsigned
431432
// busywait(twi_dcount);
432433
}
433434
i = 0;
434-
while (!SDA_READ() && (i++) < 10)
435+
while (!SDA_READ(twi_sda) && (i++) < 10)
435436
{
436437
twi_scl_valley();
437438
busywait(twi_dcount);
438439
}
439440
return 0;
440441
}
441442

443+
void Twi::twi_scl_valley(void)
444+
{
445+
SCL_LOW(twi_scl);
446+
busywait(twi_dcount);
447+
SCL_HIGH(twi_scl);
448+
WAIT_CLOCK_STRETCH();
449+
}
450+
442451
uint8_t Twi::status()
443452
{
444453
WAIT_CLOCK_STRETCH(); // wait for a slow slave to finish
445-
if (!SCL_READ())
454+
if (!SCL_READ(twi_scl))
446455
{
447456
return I2C_SCL_HELD_LOW; // SCL held low by another device, no procedure available to recover
448457
}
449458

450459
int clockCount = 20;
451-
while (!SDA_READ() && clockCount-- > 0) // if SDA low, read the bits slaves have to sent to a max
460+
while (!SDA_READ(twi_sda) && clockCount-- > 0) // if SDA low, read the bits slaves have to sent to a max
452461
{
453462
read_bit();
454-
if (!SCL_READ())
463+
if (!SCL_READ(twi_scl))
455464
{
456465
return I2C_SCL_HELD_LOW_AFTER_READ; // I2C bus error. SCL held low beyond slave clock stretch time
457466
}
458467
}
459-
if (!SDA_READ())
468+
if (!SDA_READ(twi_sda))
460469
{
461470
return I2C_SDA_HELD_LOW; // I2C bus error. SDA line held low by slave/another_master after n bits.
462471
}
@@ -500,42 +509,45 @@ void Twi::attachSlaveTxEvent(void (*function)(void))
500509
twi_onSlaveTransmit = function;
501510
}
502511

503-
inline void ICACHE_RAM_ATTR Twi::reply(uint8_t ack)
512+
// DO NOT INLINE, inlining reply() in combination with compiler optimizations causes function breakup into
513+
// parts and the ICACHE_RAM_ATTR isn't propagated correctly to all parts, which of course causes crashes.
514+
// TODO: test with gcc 9.x and if it still fails, disable optimization with -fdisable-ipa-fnsplit
515+
void ICACHE_RAM_ATTR Twi::reply(uint8_t ack)
504516
{
505517
// transmit master read ready signal, with or without ack
506518
if (ack)
507519
{
508520
//TWCR = _BV(TWEN) | _BV(TWIE) | _BV(TWINT) | _BV(TWEA);
509-
SCL_HIGH(); // _BV(TWINT)
521+
SCL_HIGH(twi.twi_scl); // _BV(TWINT)
510522
twi_ack = 1; // _BV(TWEA)
511523
}
512524
else
513525
{
514526
//TWCR = _BV(TWEN) | _BV(TWIE) | _BV(TWINT);
515-
SCL_HIGH(); // _BV(TWINT)
527+
SCL_HIGH(twi.twi_scl); // _BV(TWINT)
516528
twi_ack = 0; // ~_BV(TWEA)
517529
}
518530
}
519531

520-
inline void ICACHE_RAM_ATTR Twi::stop(void)
532+
void ICACHE_RAM_ATTR Twi::stop(void)
521533
{
522534
// send stop condition
523535
//TWCR = _BV(TWEN) | _BV(TWIE) | _BV(TWEA) | _BV(TWINT) | _BV(TWSTO);
524-
SCL_HIGH(); // _BV(TWINT)
536+
SCL_HIGH(twi.twi_scl); // _BV(TWINT)
525537
twi_ack = 1; // _BV(TWEA)
526538
busywait(5); // Maybe this should be here
527-
SDA_HIGH(); // _BV(TWSTO)
539+
SDA_HIGH(twi.twi_sda); // _BV(TWSTO)
528540
// update twi state
529541
twi_state = TWI_READY;
530542
}
531543

532-
inline void ICACHE_RAM_ATTR Twi::releaseBus(void)
544+
void ICACHE_RAM_ATTR Twi::releaseBus(void)
533545
{
534546
// release bus
535547
//TWCR = _BV(TWEN) | _BV(TWIE) | _BV(TWEA) | _BV(TWINT);
536-
SCL_HIGH(); // _BV(TWINT)
548+
SCL_HIGH(twi.twi_scl); // _BV(TWINT)
537549
twi_ack = 1; // _BV(TWEA)
538-
SDA_HIGH();
550+
SDA_HIGH(twi.twi_sda);
539551

540552
// update twi state
541553
twi_state = TWI_READY;
@@ -616,11 +628,11 @@ void ICACHE_RAM_ATTR Twi::onTwipEvent(uint8_t status)
616628
bitCount--;
617629
if (twi_data & 0x80)
618630
{
619-
SDA_HIGH();
631+
SDA_HIGH(twi.twi_sda);
620632
}
621633
else
622634
{
623-
SDA_LOW();
635+
SDA_LOW(twi.twi_sda);
624636
}
625637
twi_data <<= 1;
626638

@@ -650,14 +662,6 @@ void ICACHE_RAM_ATTR Twi::onTwipEvent(uint8_t status)
650662
}
651663
}
652664

653-
void Twi::twi_scl_valley(void)
654-
{
655-
SCL_LOW();
656-
busywait(twi_dcount);
657-
SCL_HIGH();
658-
WAIT_CLOCK_STRETCH();
659-
}
660-
661665
void ICACHE_RAM_ATTR Twi::onTimer(void *unused)
662666
{
663667
(void)unused;
@@ -714,8 +718,9 @@ void ICACHE_RAM_ATTR Twi::onSclChange(void)
714718
unsigned int scl;
715719

716720
// Store bool return in int to reduce final code size.
717-
sda = twi.SDA_READ();
718-
scl = twi.SCL_READ();
721+
722+
sda = SDA_READ(twi.twi_sda);
723+
scl = SCL_READ(twi.twi_scl);
719724

720725
twi.twip_status = 0xF8; // reset TWI status
721726

@@ -758,7 +763,7 @@ void ICACHE_RAM_ATTR Twi::onSclChange(void)
758763
}
759764
else
760765
{
761-
twi.SDA_LOW();
766+
SDA_LOW(twi.twi_sda);
762767
}
763768
}
764769
else
@@ -769,7 +774,7 @@ void ICACHE_RAM_ATTR Twi::onSclChange(void)
769774
}
770775
else
771776
{
772-
twi.SDA_LOW();
777+
SDA_LOW(twi.twi_sda);
773778
}
774779
}
775780
twi.twip_state = TWIP_WAIT_ACK;
@@ -787,13 +792,13 @@ void ICACHE_RAM_ATTR Twi::onSclChange(void)
787792
{
788793
if ((twi.twi_data & 0xFE) != twi.twi_addr)
789794
{
790-
twi.SDA_HIGH();
795+
SDA_HIGH(twi.twi_sda);
791796
twi.twip_state = TWIP_WAIT_STOP;
792797
}
793798
else
794799
{
795-
twi.SCL_LOW(); // clock stretching
796-
twi.SDA_HIGH();
800+
SCL_LOW(twi.twi_scl); // clock stretching
801+
SDA_HIGH(twi.twi_sda);
797802
twi.twip_mode = TWIPM_ADDRESSED;
798803
if (!(twi.twi_data & 0x01))
799804
{
@@ -810,8 +815,8 @@ void ICACHE_RAM_ATTR Twi::onSclChange(void)
810815
}
811816
else
812817
{
813-
twi.SCL_LOW(); // clock stretching
814-
twi.SDA_HIGH();
818+
SCL_LOW(twi.twi_scl); // clock stretching
819+
SDA_HIGH(twi.twi_sda);
815820
if (!twi.twi_ack)
816821
{
817822
twi.onTwipEvent(TW_SR_DATA_NACK);
@@ -838,11 +843,11 @@ void ICACHE_RAM_ATTR Twi::onSclChange(void)
838843
twi.bitCount--;
839844
if (twi.twi_data & 0x80)
840845
{
841-
twi.SDA_HIGH();
846+
SDA_HIGH(twi.twi_sda);
842847
}
843848
else
844849
{
845-
twi.SDA_LOW();
850+
SDA_LOW(twi.twi_sda);
846851
}
847852
twi.twi_data <<= 1;
848853

@@ -864,7 +869,7 @@ void ICACHE_RAM_ATTR Twi::onSclChange(void)
864869
}
865870
else
866871
{
867-
twi.SDA_HIGH();
872+
SDA_HIGH(twi.twi_sda);
868873
twi.twip_state = TWIP_READ_ACK;
869874
}
870875
}
@@ -888,7 +893,7 @@ void ICACHE_RAM_ATTR Twi::onSclChange(void)
888893
}
889894
else
890895
{
891-
twi.SCL_LOW(); // clock stretching
896+
SCL_LOW(twi.twi_scl); // clock stretching
892897
if (twi.twi_ack && twi.twi_ack_rec)
893898
{
894899
twi.onTwipEvent(TW_ST_DATA_ACK);
@@ -911,8 +916,8 @@ void ICACHE_RAM_ATTR Twi::onSdaChange(void)
911916
unsigned int scl;
912917

913918
// Store bool return in int to reduce final code size.
914-
sda = twi.SDA_READ();
915-
scl = twi.SCL_READ();
919+
sda = SDA_READ(twi.twi_sda);
920+
scl = SCL_READ(twi.twi_scl);
916921

917922
int twip_state_mask = S2M(twi.twip_state);
918923
if (scl) /* !DATA */
@@ -934,7 +939,7 @@ void ICACHE_RAM_ATTR Twi::onSdaChange(void)
934939
else IFSTATE(S2M(TWIP_START) | S2M(TWIP_REP_START) | S2M(TWIP_SEND_ACK) | S2M(TWIP_WAIT_ACK) | S2M(TWIP_SLA_R) | S2M(TWIP_REC_ACK) | S2M(TWIP_READ_ACK) | S2M(TWIP_RWAIT_ACK) | S2M(TWIP_WRITE))
935940
{
936941
// START or STOP
937-
twi.SDA_HIGH(); // Should not be necessary
942+
SDA_HIGH(twi.twi_sda); // Should not be necessary
938943
twi.onTwipEvent(TW_BUS_ERROR);
939944
twi.twip_mode = TWIPM_WAIT;
940945
twi.twip_state = TWIP_BUS_ERR;
@@ -944,11 +949,11 @@ void ICACHE_RAM_ATTR Twi::onSdaChange(void)
944949
if (sda)
945950
{
946951
// STOP
947-
twi.SCL_LOW(); // clock stretching
952+
SCL_LOW(twi.twi_scl); // clock stretching
948953
ets_timer_disarm(&twi.timer);
949954
twi.twip_state = TWIP_IDLE;
950955
twi.twip_mode = TWIPM_IDLE;
951-
twi.SCL_HIGH();
956+
SCL_HIGH(twi.twi_scl);
952957
}
953958
else
954959
{
@@ -978,7 +983,7 @@ void ICACHE_RAM_ATTR Twi::onSdaChange(void)
978983
else
979984
{
980985
// during first bit in byte transfer - ok
981-
twi.SCL_LOW(); // clock stretching
986+
SCL_LOW(twi.twi_scl); // clock stretching
982987
twi.onTwipEvent(TW_SR_STOP);
983988
if (sda)
984989
{

0 commit comments

Comments
 (0)
Please sign in to comment.