Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SPI: Implemented MISO, MOSI, SCK and SS pins handling using avr_bitba… #249

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hovercraft-github
Copy link
Collaborator

This allows to simulate a 'real' signal time patterns on SPI interface related MCU pins and communicate with diverse devices over it.
Should also fix #222.

@buserror
Copy link
Owner

Does the 'old' mode works -- ie use one IRQ per words? As much as I like simulating more, I'm worried that bit banging will slow down simulation considerably.. Is there a way to make it 'optional'?

I have simavr project where I'm not interested at all in the bit patterns generated, I'm more instered in what's passing on, and slowing down the simulation isn't terribly useful -- in fact that's why I never implemented it! :-)

@hovercraft-github
Copy link
Collaborator Author

hovercraft-github commented Sep 14, 2017

I tried to do all my best to preserve the 'old' mode. At least board_ssd1306 example works without any noticeable slow-down. Have you some additional tests or examples I can verify?
I agree, bit-banging can slow down the simulation, but this would happen only during very intensive, constantly running communication, where LARGE amounts of data are sent/received over SPI. Example may be programming a big EEPROM chip. For me, this is certainly a rare case! So I'd better make the 'old' mode an 'option' :)
Anyway, how to implement a control for this optional mode the right way? I will add AVR_IOCTL_SPI_BITBANG with boolean parameter to give the user facility to turn bit-bang mode on or off - @buserror is this Ok?

@buserror
Copy link
Owner

Yes that would be great actually. An alternative would possibly be to be able to use a AVR_MCU macro and entirely disable the bitbang module? I suppose the module could be used to simulate modulation for the uart/i2c etc as well at that rate (eventually).

The bitbang was never something I envisioned for simavr, as I'm more interested in the 'logical' simulation than in the pin-accurate simulation...

@buserror
Copy link
Owner

@hovercraft-github any progress on this? I do like the idea of the AVR_MCU macro you add to your own firmware, kinda like the VCD, clock speed, voltages etc...
Feel free to make it 'global' to the whole bitbang module too, I think it'd be handy. Otherwise, perhaps we could use a bitmask for the peripherals to enable/disable?

@hovercraft-github
Copy link
Collaborator Author

hovercraft-github commented Oct 19, 2017

Sorry, I was quite busy last month :)
Now I suppose to finalize this patch within next week.
I'll do it both way: via AVR_MCU and also via AVR_IOCTL_SPI_BITBANG,
because it's requirement for simulator GUI projects (simutron, simulide..) - this gives the user a convenience to choose mode of operation via settings menu, avoiding recompiling the firmware project.

@buserror
Copy link
Owner

Sill quite keen on this patch @hovercraft-github -- no hurries but it'd be sad to drop it...

@hovercraft-github
Copy link
Collaborator Author

@buserror Sorry, some circumstances prevented me from completing this task for several months.
If you still interested, I will try to revive it now.

@buserror
Copy link
Owner

Hmm GH doesn't let me comment on the first commit somehow... so I'll paste this there:

What about a static const char clock_div = { 4,16,64,128]; clock_divider = clock_div[clock_divider_ix) ?

@hovercraft-github
Copy link
Collaborator Author

@buserror Majority of code complete, now testing. Thanks for the clock_divider note.

@buserror
Copy link
Owner

It's looking good -- just tell me 'when' -- feel free to 'colapse' your commits if you feel like it..

@hovercraft-github
Copy link
Collaborator Author

I found discrepancy in how .name member of different peripherals initialized:

  • USART instances initialized with '0' symbol (=0x30) +index, i.e. 0x31 for uart1 and so on
  • standalone SPI and TWI instances initialized with plain 0x00

AVR_IOCTL_XX macros in examples and tests follow the same scheme, i.e. AVR_IOCTL_UART_GETIRQ('3') for uart3, but AVR_IOCTL_SPI_GETIRQ(0) for the standalone SPI.
I would like to leave it as is, but reserve this feature to differentiate in the future named SPI instances (those originating from USART in SPI mode) from standalone one.
E.g. mega2560 has 4 USARTs, hence four SPI instances which we will refer as AVR_IOCTL_SPI_GETIRQ('0'), AVR_IOCTL_SPI_GETIRQ('1') and so on,
plus single standalone SPI, which we still refer as AVR_IOCTL_SPI_GETIRQ(0)

I still fill doubt on this however.

@buserror
Copy link
Owner

buserror commented May 1, 2018

It's possible that I didn't think of AVRS with multiple SPIs/TWI at the time I wrote the macro... I think I was planning all along to have it as a character tho...

@hovercraft-github
Copy link
Collaborator Author

I believe it will become obvious how to deal with instance names when we start to implement SPI/USART. I want to ask you to leave it as is for awhile.
In the current state generic tests pass, but I nead yet some test for the slave mode and for AVR_MCU.

@hovercraft-github
Copy link
Collaborator Author

@buserror Please review my last commit 2ec8a9a. I'm not sure you would like a new function avr_io_findinstance, but I find it very useful in any "generic" (i.e. not bound to some single processor model)
simulation to gather out some peripheral details. Also without it I can't fix an issue with inconsistent state of the output port, when this port is shared between bitbang and some other function.

Now all tests pass, board_timer_64led and board_ssd1306 examples pass, so I'm ready to 'squeeze' the commits..

Michel?

@buserror
Copy link
Owner

I just did @hovercraft-github -- thanks for your work on this too... it's shaping up! :-)

@hovercraft-github
Copy link
Collaborator Author

@buserror I will try to add a variant of avr_io_findinstance according to your suggestion in separate patch later.

@hovercraft-github
Copy link
Collaborator Author

@buserror Is there something stopping you from merging this patch now? Is it current avr_io_findinstance implementation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SPI problems with 2 avr's connected
2 participants