AnsweredHot!Sharing code

Author
bblessing
Super Member
  • Total Posts : 441
  • Reward points : 0
  • Joined: 2008/12/04 06:44:21
  • Location: Cincinnati, OH
  • Status: offline
2017/08/28 08:28:04 (permalink)
5 (1)

Sharing code

    So some things didn't make it into Harmony 2.04 that I thought were good fixes, ones that I use regularly. As it stands, I have to haul these fixes over from one version to another. I'm ok with that, though I should probably add these drivers to subversion so that I can merge Microchip's changes with mine and make life much easier. I've tried to post code, zipped up and under the limit, but have recently been getting denied. This wasn't a problem before, so I can only conclude that we have limits overall on the size and number of attachments we make. This is perfectly understandable, as a forum full of software submissions would get unwieldy.
 
    My question is what would be the proper channel for submitting software? I know that this is something that Microchip is probably hesitant to get involved with as it's difficult to keep track of what's worthwhile and what's not, massive testing notwithstanding. Should I start posting my software on GitHub? It's based on Harmony code, and I've worked hard to make it appear seamless. If I can, would Microchip block the links?
 
    A while back, somebody mentioned that Microchip should accept software and treat Harmony as an open source project. This is fine and dandy, but it might be easier just to have some place where we can easily post software, link to it via these fora, and allow Microchip to poach the really good fixes if they want. I'm assuming GitHub is the way to go, but I've never used it.
#1
MikeinAZ
Administrator
  • Total Posts : 263
  • Reward points : 0
  • Joined: 2014/06/03 07:25:32
  • Location: Chandler, AZ
  • Status: offline
Re: Sharing code 2017/08/28 10:07:59 (permalink) ☄ Helpfulby bblessing 2017/08/28 10:42:25
4.8 (5)
Hello,
 
I cannot comment on why the forum may have blocked some of your links.  Generally it does not have a limit or problem with this, but obviously there are many others that would attest to these sort of problems.  There is a forum for problems on the forum (if that even makes sense) that may bring some clarity to that.
 
We do always try to improve the code.  I am not sure which specific changes you have made, or if we had previously tried to incorporate them and somehow ignored/drop/cut them.  If you have specific suggestions on this feel free to PM me.  We can correspond by email.
 
For listing your own changes, I first recommend doing so as code snips right here in the forum.  This is done regularly.  If it is an entire driver or other larger (middleware, app) structure, then you may want to use embedded code source (www.embeddedcodesource.com).  This is intended to be an archive of support from contributors like you that may help others.  
 
I really would prefer you not set up your own Github with a parallel Harmony archive.  It is not that it is a bad idea, but it would be difficult for others to see what works, what is official, and what is new / changes.  With each release there are many changes, and with this parallel structure not being updated with all we are doing internally there will be obvious conflicts.  I can see the amount of support on something like this increasing instead of decreasing.  In any case, support issues aside, we will not block a link to an outside code repository if it is professionally done and adds values to the customer base.
 
In the future, we are considering ways to add this kind of open archive and the ability to accept user contributions to it.  We agree this will generally make it more active, although there are licensing and support elements that are more difficult to manage.  
#2
bblessing
Super Member
  • Total Posts : 441
  • Reward points : 0
  • Joined: 2008/12/04 06:44:21
  • Location: Cincinnati, OH
  • Status: offline
Re: Sharing code 2017/08/28 10:42:19 (permalink)
4 (2)
There are three things that I wanted to contribute, though two depend on the third. The first is an updated SPI library, specifically that which pertains to the client configure part. As it stands now, there are two callbacks (operationStarted and operationEnded) and baudRate. In probably >90% of the projects that I've worked on there have been chips with different clock settings on the same SPI bus. E.g., I have had Analog Devices A/Ds (clock idle high) and Microchip EEPROMs (clock idle low) on the same bus. I added clockMode and inputSamplePhase to the fields available in DRV_SPI_CLIENT_DATA. This allows chips with different settings to play nicely on the same bus. Yes, I could use a separate SPI port for each chip, but this doesn't help with legacy products that could benefit from Harmony or those with limited SPI ports. The code changes were quite straightforward: look for all instances of baudRate and add in the new fields AND, this is the biggie, ensure that the fields are modified BEFORE the call to operationStarted. This ensures that everything is set up properly.
 
I also added a DRV_SPI_BufferAddWriteRead3 function that is the same as WriteRead2, but contains a read offset. This is necessary as it gets rid of the problem that the SPI flash driver has: two messages are queued for a read operation between the CS line assert/deassert. This leads to the problem of other messages being queued in between and renders the SPI flash driver unable to play nicely on a shared SPI port. In previous versions of Harmony, it made for an ugly situation where the callbacks weren't used and CS asserts/deasserts where scattered all over the driver. I've written about this extensively on these fora. I have gotten this to work with non-DMA modes, though I'm testing that now.
 
From there, I wrote a generic SPI EEPROM driver that, in theory, will allow one to select a SPI based Microchip EEPROM from a drop-down box and have it work out of the gate. It works well with the 25AA256 and 25AA1024 chips.
 
Finally, I have a driver for the SPI-based RTCC chips (e.g. MCP79520, thought it should work with all of them). In addition to exposing the RTCC registers it gives access to the SRAM and EEPROM as well.
 
In any case, I won't mess with GitHub. I'll do whatever you ask of me, including emailing the drivers to you and/or submitting them to embedded code source. At a minimum, the learning has been invaluable and I've poached quite a few good ideas from the Harmony framework. Thanks again for your help and hearing me out!
#3
bblessing
Super Member
  • Total Posts : 441
  • Reward points : 0
  • Joined: 2008/12/04 06:44:21
  • Location: Cincinnati, OH
  • Status: offline
Re: Sharing code 2017/08/29 09:24:36 (permalink)
0
Man, getting into the guts of the DMA logic for the SPI driver is pretty intense, though fortunately the ability to add in an offset is there. What is the likelihood of the dynamic SPI driver being reworked? I'm seeing a lot of very complex conditions inside if statements and the ftl files have #if/else statements galore. From an outsider's standpoint it looks unwieldy, especially considering that EBM, standard, and DMA modes all have the same combined logic in drv_spi_dynamic_tasks.c.ftl. Perhaps it might be worth splitting those up? It's more to test, but you get rid of all of that ftl #if/else logic. I thought a major driver for getting away from the old MLA was the #ifdef preprocessor hell. This is better, but still problematic.
#4
MikeinAZ
Administrator
  • Total Posts : 263
  • Reward points : 0
  • Joined: 2014/06/03 07:25:32
  • Location: Chandler, AZ
  • Status: offline
Re: Sharing code 2017/08/29 09:27:43 (permalink)
3 (1)
I will send the SPI guy in your direction.  Yes, some of that code is problematic.
#5
mrpackethead
Super Member
  • Total Posts : 654
  • Reward points : 0
  • Joined: 2007/04/01 23:33:39
  • Location: 0
  • Status: online
Re: Sharing code 2017/08/29 22:04:07 (permalink)
4.5 (2)
Having a well constructed GIT repo, is not a bad thing at all.   You only need to accept what you want to merge into the master code branch.. Git has lots of helpful tools for submitting bugs/ making notes..   Its a really widely used way of having lots of people contributing.. 



but it would be difficult for others to see what works, what is official, and what is new / changes

Every commit provides all that info!!    

yes, Microchip you'd need to contribute to this, but this would push development and bug fix / features ahead at a pace that you will never be able to do by yourself!     Because you'll have the resources of a whole bunch of free smart people who want to acehive the same thign as you..   Functional, bug free code, that is feature rich.



 
#6
Totem
Super Member
  • Total Posts : 263
  • Reward points : 0
  • Joined: 2014/12/04 02:18:11
  • Location: Mars
  • Status: offline
Re: Sharing code 2017/08/30 19:36:53 (permalink)
3 (1)
I think you can submit the SPI driver suggestions and code through microchip ticketing system.
 
1. I agree with first point: Clock mode selection is needed as part of client configuration
2. Can you give more details on second point?
3. Having device drivers for microchip parts is a good idea, but it may increase the installer/library size. 
post edited by Totem - 2017/08/30 21:44:57

Everything is Relative!
#7
bblessing
Super Member
  • Total Posts : 441
  • Reward points : 0
  • Joined: 2008/12/04 06:44:21
  • Location: Cincinnati, OH
  • Status: offline
Re: Sharing code 2017/08/31 02:45:09 (permalink) ☼ Best Answerby MikeinAZ 2017/09/01 15:31:27
0
Regarding point 2: look at how reads are accomplished in ANY SPI flash driver. The CS line is brought low, the instruction and address are then written in a buffered write, all of the bytes are read in a buffered read, and then CS is brought high. The fact that there are TWO buffered operations means that there is a chance that something else may be queued up between the write and the read, which causes corruption. Yes, the driver can gain exclusive use of the SPI bus, but blowing a SPI bus per flash chip is not ideal.

The read offset is the best that I can come up with that will combine the write and read into a SINGLE buffered operation. You cannot do a write and read operation as the first few bytes of the read buffer will be incorrect. Basically, a read offset means the first few bytes of a write and read operation will not be read and treated as dummy values.

An alternative would be to provide a function that would allow the ability to determine if two slots are open in the SPI queue so that the write and read may be queued one after the other without leaving the function. If not, don't queue up either.
 
Edit: Come to think of it, the alternative may not even need to worry about the SPI library. If you add a DRV_SPI_BufferAddWrite2 and then a DRV_SPI_BufferAddWrite2 in succession then this leaves you with the following situations: a) the write is not added to the queue b) the read is not added c) both are added. In the case of a), you'll just go back and try again later. With c), you're good to go. With b), just pull the CS line high when it fails to add the read operation to the queue. If the write has or has not finished is immaterial: it's just sending the instruction and address, so pulling the CS line high at any point here will result in the overall operation being cancelled so far as the flash is concerned. This is because any time this technique is used, it is sending out a read opcode (read, read ID, or read status) with its address (not with a status read). The overhead would be low as no more than like 5 bytes would be sent.
 
Some variables will need to be added. The DRV_SST25_OBJ will need to have another DRV_SPI_BUFFER_HANDLE type added as well as another generic buffer like cmdParams and its length cmdParamsLen. Still, this is not adding a ton of memory to the driver.
 
I will try and get the alternative driver up and going. It is kind of ugly, but I'm pretty sure this will solve the problem without getting into the guts of a very complicated SPI driver.
 
Edit 2: How about that: it works! I can now use the SPI bus in DMA mode with no issues, or standard and EBM mode like before. Better yet, it seems to be playing ball quite well with my other devices as the items in the queue are guaranteed to be one right after the other.
post edited by bblessing - 2017/08/31 11:26:17
#8
Emcy
Super Member
  • Total Posts : 443
  • Reward points : 0
  • Joined: 2008/01/09 03:37:06
  • Location: 0
  • Status: offline
Re: Sharing code 2017/09/08 02:56:43 (permalink)
0
MikeinAZ
 
In the future, we are considering ways to add this kind of open archive and the ability to accept user contributions to it.  We agree this will generally make it more active, although there are licensing and support elements that are more difficult to manage.  


Very good idea, I hope this feature will be implemented soon :)
 
#9
arpananand
Super Member
  • Total Posts : 342
  • Reward points : 0
  • Joined: 2009/11/18 04:35:42
  • Location: 0
  • Status: offline
Re: Sharing code 2017/09/11 08:30:36 (permalink)
0
bblessing
The read offset is the best that I can come up with that will combine the write and read into a SINGLE buffered operation. You cannot do a write and read operation as the first few bytes of the read buffer will be incorrect. Basically, a read offset means the first few bytes of a write and read operation will not be read and treated as dummy values.

do you see any problem with this method? i think it is good that currently API is designed in this way so that both the use case scenario can be handled. if some one needs parallel read while writing, or if someone needs read followed by write, both can be handled with the current API.
 
so i think there is no need of any new API, is there?
 
#10
bblessing
Super Member
  • Total Posts : 441
  • Reward points : 0
  • Joined: 2008/12/04 06:44:21
  • Location: Cincinnati, OH
  • Status: offline
Re: Sharing code 2017/09/11 13:01:57 (permalink)
0
arpananand
bblessing
The read offset is the best that I can come up with that will combine the write and read into a SINGLE buffered operation. You cannot do a write and read operation as the first few bytes of the read buffer will be incorrect. Basically, a read offset means the first few bytes of a write and read operation will not be read and treated as dummy values.

do you see any problem with this method? i think it is good that currently API is designed in this way so that both the use case scenario can be handled. if some one needs parallel read while writing, or if someone needs read followed by write, both can be handled with the current API.
 
so i think there is no need of any new API, is there?
 


After working through another approach, the offset is not necessary. However the API does need to change regarding the DRV_SPI_CLIENT_DATA structure as noted above. I have the code finished, but I have been unable to post it. I'm waiting for things to be officially ready with www.embeddedcodesource.com and my account there before I post it.
#11
Tez
Moderator
  • Total Posts : 462
  • Reward points : 0
  • Joined: 2006/10/04 11:09:05
  • Location: 0
  • Status: offline
Re: Sharing code 2017/09/13 07:29:49 (permalink)
0
Please check your personal message.
#12
Jump to:
© 2017 APG vNext Commercial Version 4.5