Extend MAC HAL API by macClearInterrupts() Topic is solved

Use this forum for requesting small changes in ChibiOS. Large changes should be discussed in the development forum. This forum is NOT for support.
hal
Posts: 8
Joined: Fri Nov 02, 2012 3:07 pm
Has thanked: 2 times
Been thanked: 1 time

Extend MAC HAL API by macClearInterrupts()

Postby hal » Sat Oct 28, 2023 6:01 pm

Hello Giovanny.

I wrote two drivers for W5500 Wiznet SPI Ethernet Controller with integrated TCP/IP stack. The first driver is implementing "plain" ethernet using hal_mac_lld and therefore needs LWIP to work; the second one is based on IoLibrary from Wiznet. I'm going to contact folks from Chibios-Contrib and commit both drivers and bindings for IoLibrary in there.

The thing with the first "plain" ethernet driver is that W5500 is a SPI based Ethernet Controller and I can't clear interrupt registers in the ISR just by using SPISend()/SPIReceive(). So I need a function in the MAC HAL to clear interrupts in the LwIP task. Furthermore, hal_mac_lld needs to know about SPIDriver and SPIConfig. That is why I need to bind them somehow to MACConfig and feed that MACConfig to lwipInit().

To shorten it all up, I have attached a patch. ;)

One little thing about low_level_output(), though. I mean it's odd using macWaitTransmitDescriptor() once, call macWriteTransmitDescriptor() with the same descriptor multiple times and release it only after that, just like this: wait/use/use/use.../release. Shouldn't it be more "symmetrical": wait/use/release, wait/use/release? I ran into it because I have been experimenting with mbedtls and DTLS (which is UDP based), where DTLS wants UDP packets being sent in one piece rather than divided in multiple small packets. So I've changed the code to wait/use/release and used pbuf_clone() in order to rebuild a full packet from pieces. It will consume more RAM, but it's worth it.

Speaking of mbedtls, where would you prefer to have it? In ChibiOS/ext or better in ChibiOS-Contrib/ext?

What do you think all about?

Thanks in advance for taking time!
Attachments
mac_hal.zip
(2.95 KiB) Downloaded 187 times

User avatar
Giovanni
Site Admin
Posts: 14463
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1080 times
Been thanked: 922 times
Contact:

Re: Extend MAC HAL API by macClearInterrupts()

Postby Giovanni » Sat Oct 28, 2023 6:22 pm

Hi,

Exposing workarounds and interrupt-related code at HAL API level is against "rules", drivers are meant to hide inner details to the user code. You should try to hide interrupt clearing within the driver, there are SPI functions that can be called from withing ISRs, see spiStartSendI() for example, it is asynchronous and gives you a callback when the operation is done.

About descriptors, those are meant to be written multiple times then released, it is not an error.

Note that I don't handle code under /community, you need to make a pull request or talk with the maintainers on the ChibiOS Discord server.

Giovanni

hal
Posts: 8
Joined: Fri Nov 02, 2012 3:07 pm
Has thanked: 2 times
Been thanked: 1 time

Re: Extend MAC HAL API by macClearInterrupts()

Postby hal » Sun Oct 29, 2023 12:00 am

Hello Giovanni, thanks for response!

Ok, I understand about HAL API; driver's interrupt handling has been rewritten now.

One issue is still present what I'm struggling with. That one is around the lines with for(q = p; ...) macWriteTransmitDescriptor() in low_level_output(). I have said before that I'm using mbedtls and DTLS... mbedtls allocates buffer and uses it to encrypt data into one UDP packet in there and then pass the buffer to LWIP. LWIP has not enough place in buffer, can't prepend data in the buffer with UDP header, therefore splits the buffer into two and pass it to low_level_output() as linked list struct pbuf *p. So far there is nothing suspicious about. The problem is now that putting parts of the UDP packet one by one on the ethernet in the for-loop results in malformed packets, see dtls-corrupted.png.

The only one solution, I have found, is to rebuild the packet with pbuf_clone() and send it out in one piece. See mac_hal2.zip. Replacing the loop with pbuf_clone() results in awaited behaivour as can be seen from the packet No. 5812.

What's your opinion?
Attachments
mac_hal2.zip
(908 Bytes) Downloaded 181 times
dtls-corrupted.png

User avatar
Giovanni
Site Admin
Posts: 14463
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1080 times
Been thanked: 922 times
Contact:

Re: Extend MAC HAL API by macClearInterrupts()  Topic is solved

Postby Giovanni » Sun Oct 29, 2023 6:30 pm

Note that multiple writes into a descriptor still result in a single frame sent, the limit is the maximum frame site.

Writes are just a way to "assemble" frame composed of multiple parts.

Giovanni

hal
Posts: 8
Joined: Fri Nov 02, 2012 3:07 pm
Has thanked: 2 times
Been thanked: 1 time

Re: Extend MAC HAL API by macClearInterrupts()

Postby hal » Mon Oct 30, 2023 1:06 pm

Thanks Giovanni!

This was the hint you gave: "Writes are just a way to "assemble" frame composed of multiple parts."

I did not realise that by macWriteTransmitDescriptor() the driver should have only written to device's internal buffer and put that buffer on the ethernet only after macReleaseTransmitDescriptorX() has been called. That's it.

Thanks again!

hal


Return to “Small Change Requests”

Who is online

Users browsing this forum: No registered users and 3 guests