DieBieEngineering / DieBieSlave-Firmware-Nunchuck

30 stars 24 forks source link

nWrtSpcAvlCount and nReadSpaceAvblCount variables #1

Open Sulymarco opened 6 years ago

Sulymarco commented 6 years ago

Hi Danny, I see that in file driverSWLAN9252.c the variables nWrtSpcAvlCount and nReadSpaceAvblCount are read, decremented but never used. According to the LAN9252 data sheet they must be used to see how many bytes we can read/write from /to the process ram fifo. What do you think?

I know that the same issue is present in the SOES and in the Microchip code, and I already arose the problem to them but so far with no result.

Regards Marco

DieBieEngineering commented 6 years ago

Hi Marco,

Thanks for pointing this out, I have however indeed implemented the examples and did only moderate evaluation of the driver. I have no idea on the consequences this has, all I know is that with all implementations of my code on the slave everything works as expected.

If you have an answer from SOES or Microchip I would love to hear and fix it as well. Did you ask them publicly (if so can you share a linkt)?

Thanks for your help! Danny

Sulymarco commented 6 years ago

Hi Danny, first of all thanks for your reply.

all I know is that with all implementations of my code on the slave everything works as expected

I have no doubt about that, as your code is derived directly from the Microchip example: I think that everybody use it as it is, and, as far as I know, nobody complains.

I have no idea on the consequences this has

I think that the EtherCAT core is faster than the microprocessor and so there are always data to read or space to write available, without checking it. Probably this is the reason why we can get rid of the PRAM_READ_AVAIL_CNT and PRAM_WRITE_AVAIL_CNT fields, without having harmful consequences.

Anyway, even if this is the case, is should be clearly described and specified in the datasheet, but there is no mention about it. Further the code provided by Microchip is odd and this deserve some action from them.

Did you ask them publicly (if so can you share a linkt)?

I asked about this issues in the SOES GitHub repository but so far no answer aside from the developer's amazement. I also raised the question to the Microchip support (no link available) but so far no answer at all. Some years ago Microchip was very responsive to the customer questions, even from "mister nobody ", but it seems that now the company has growed to much and the customer support in no more what it was. Please can you help to put a little bit of pressure on them, asking you too the same question, in the hope to have an answer. You have to register and proceede through this page

Have a nice day Marco

Sulymarco commented 6 years ago

Hi, this is the question I posted to the Microchip support one month ago:

_In the SPIDriver.c file, in functions SPIReadPDRamRegister / SPIWritePDRamRegister, the field of ECAT_PRAM_RD_CMD / ECAT_PRAM_WRCMD, that indicates how many times the process ram data fifo can be read / write without further need to check the status, is never used: it is read, decremented, but nobody uses it. Please can you clarify this issue?

So far no answer so today I added a clarification:

_I expalain better the meaning of my question. The issue is not that there are some line of useless code in your example, but that the PRAM_READ_AVAIL_CNT and the PRAM_WRITE_AVAIL_CNT fields are never evaluated, despite what is written in the LAN9252 datasheet. From the datasheet paragraph 12.11.1 " The PRAM Read Data Available Count (PRAM_READ_AVAIL_CNT) field indicates how many reads can be performed without needing to check the status again. " From the datasheet paragraph 12.11.4 " The PRAM Write Space Available Count (PRAM_WRITE_AVAILCNT) field indicates how many writes can be performed without needing to check the status again. " I think that this deserves some action from you. It would be really nice to have an answer.

I am quite disappointed with the Microchip support ...

Regards Marco

Counterfeiter commented 5 years ago

Is there any Update from Microchip?

Sulymarco commented 5 years ago

Yes, but quite strange: about 2 months ago they ask me if I am still interesting in an answer. To my affirmative answer they said "ok, we forward your question to the businnes unit" (???)

So far no more replies, I am quite confused and perhaps they too.

Marco

Counterfeiter commented 5 years ago

okay...

I see, I have to test the register content by myself.... have a 80 MHz QSPI -> to fast to drop this datasheet information?!

VG

Counterfeiter commented 5 years ago

I test it... it's definitely important with a 80 MHz QSPI... If the FIFO is running out the LAN9252 is returning zeros! Would be great to have datasheet information about this. Because a slower running qspi with DMA makes more sense in my application...

Sulymarco commented 5 years ago

Many thanks for your report.

Marco

Sulymarco commented 5 years ago

Finally I received an answer from Microchip (after more than one year)

Each FIFO will update at about 800nSec. So, we don’t need to poll the register every time since the PIC is running at a slower speed and polling will reduce the speed even further. If we add code to poll the available count, it will further decrease the throughput. If a high-performance MCU is used it is recommended to use a 1uSec delay or poll the available counter.

Marco

Counterfeiter commented 5 years ago

I measure it with about 720 ns update rate. So should be a true value. But in fact it says: A 40 MHz SPI is enough to download and upload the PDO data. A QSPI with 80 MHz seems senseless. 🤭