I really don't think this is a good idea - perhaps for those other watches we could instead have an #ifndef DISABLE_SPIFLASH_CS_OPTIMISE or something and remove the spiFlashLastAddress?

Or we could just make the check itself behind an #ifdef?

Or.. it sounds like if you're adding SPIFLASH storage to the watch you're building your own firmware, so you could just build your Inline C code directly into the firmware and poke spiFlashLastAddress?

The problem is, this code ends up getting absolutely thrashed when executing JS from Flash - it's called every 16(or 32? I forget) characters of JS executed. This patch adds a read from the IO bus (which I think only runs at 16Mhz?), not to mention the shift and so on.

I'd have thought it would slow down JS execution when on other devices there's no need as the SPI bus isn't shared. I feel like there's really no point 'adding robustness' in those cases when actually it's perfectly robust anyway.

it sounds like if you're adding SPIFLASH storage to the watch you're building your own firmware, so you could just build your Inline C code directly into the firmware and poke spiFlashLastAddress?

Yes I addressed that in first comment 'even if it was fully native in libs/graphics it would still be not so nice to access jshardware.c internal variable from other unrelated code.' As you say I am building custom firmware so I can do it in any way.

I was not expecting this would be an issue since in best case this calls spiFlashRead
which does similar access 1+24*length times (3*8*len+setting mosi) so we are talking about 1/(24*(16 or 32)) slowdown of I/o bus operations (in addition to all other code it does + subroutine call).

BTW, this is reading OUT register (not IN) so do you think it really goes down to 16Mhz to read back value previously written there by CPU? Are all GPIO registers slower too i.e. all the CNF ones? I would guess 16Mhz relates only how the changes propagate between real wires and those registers so e.g. reading IN at 64Mhz would give you same value 4 times due to slower input but I wouldn't guess it would slow down the actual ARM read from 0x50000000+0x504. But I see I am maybe wrong on this. So any access to 0x50000000 area waits for some 16MHz bus(?)

So then you may possibly also remove clearing MOSI pin in spiFlashRead to save one such cycle as the flash ignores data on read anyway(?).

Configuring out this optimization would make no sense even for my case with shared display as this would be performance hit all the time.

So I guess if you really see it as performance issue then hiding this behind something like #ifdef SPIFLASH_SHARED_SPI is still worth the effort since this could be moved to board files instead of local modification. Apart from DaFit watches this is also issue on another watch - ID107HR. So there are at least two persons that would possibly build with that flag. So I'll add the ifdef around it.

Thanks! I was pretty sure IO accesses to the GPIO were slower - at least that was always the case on STM32, and I'd imagined nRF52 would be similar.

However I stand corrected:

var c = E.compiledC(`
// int testa()
// int testb()
typedef volatile unsigned int uint32_t;
int testa(){
  uint32_t tStart = *((uint32_t*)0xE000E018); // systick
  volatile uint32_t z = 0;
  for (int i=0;i<100000;i++)
    z+=*((uint32_t*)0x20000000);
  uint32_t tEnd = *((uint32_t*)0xE000E018); // systick
  return (z&1)+(tStart-tEnd)&0xFFFFFF;
}

int testb(){
  uint32_t tStart = *((uint32_t*)0xE000E018); // systick
  volatile uint32_t z = 0;
  for (int i=0;i<100000;i++)
    z+=*((uint32_t*)0x50000504);
  uint32_t tEnd = *((uint32_t*)0xE000E018); // systick
  return (z&1)+(tStart-tEnd)&0xFFFFFF;
}

`);

print(c.testa()) // 1240839 RAM (flash is similar)
print(c.testb()) // 1057747 GPIO

GPIO is actually faster!

However I still think the original code will be faster because spiFlashLastAddress is already in a register. I know the hit is pretty minor and it seems petty, but I feel like if I'm integrating stuff that is done to make life easier for a few folks using Espruino on their own devices, it really shouldn't negatively impact users of official devices at all.

So then you may possibly also remove clearing MOSI pin in spiFlashRead to save one such cycle as the flash ignores data on read anyway(?)

I'd be up for that - I know it's minor, but everything helps.