Re: RFC on STM32 Ethernet driver


Erwin Rol
 

Hey Neil,

Thanks, somehow just didn't think about that. Will rewrite the eth
driver to use the CONTAINER_OF() construct.

BTW is that i2c driver going to be included in mainline? :-)

- Erwin

On 9-6-2017 13:25, Neil Armstrong wrote:
Hi Erwin,

You can use CONTAINER_OF() to get the parent structure like
https://github.com/superna9999/zephyr/blob/stm32f4_i2c/drivers/i2c/i2c_stm32.c#L93

Neil

On 06/09/2017 12:22 PM, Erwin Rol wrote:
Hey Erwan,

like you say some things are subjective when it comes to HAL's, but
somethings really bother me with the STM32 HAL.

The most important is how to deal with IRQ's. In Zephyr I have to setup
the ISR like this;

IRQ_CONNECT(ETH_IRQn, CONFIG_ETH_STM32_IRQ_PRI, eth_isr,
DEVICE_GET(eth0_stm32), 0);
irq_enable(ETH_IRQn);

Now the eth_isr will be called on IRQ. But to use everything from the
HAL we must call the;

void HAL_ETH_IRQHandler(ETH_HandleTypeDef *heth)

That function will than call;

__weak void HAL_ETH_RxCpltCallback(ETH_HandleTypeDef *heth)

But as you can see we register a ISR to take a "struct eth0_stm32*" but
all the HAL functions only have "ETH_HandleTypeDef *heth" arguments.

So our driver ISR somehow needs something to go from ETH_HandleTypeDef
*heth to struct eth0_stm32*. The other way around is simple, the
eth0_stm32 struct just has a ETH_HandleTypeDef member.

I solved this to copy the HAL_ETH_IRQHandler handler to my driver and
change it to take my eth0_stm32 struct as argument.

But I really don't like the idea to use half the HAL and copy the other
half.

Would there be a better solution?

BTW: this applies to pretty much everything the uses STM32 IRQ's. I
already noticed the the UART does not use the HAL, but that would have
had the same problem.

It would have been nice if those ETH_HandleTypeDef structures had a
void* user_data member.

That directly brings me to the next point, how are things managed with
upstream (ST) if Zephyr finds problems/bugs in the HAL? Bugs they
probably would not mind to fix, but how to deal with things that are
Zephyr specific, will that mean the Zephyr STM32 HAL will diverge from
the official ST HAL ?

- Erwin

On 6-6-2017 11:19, Erwan Gouriou wrote:
Hi Erwin,


Thanks for proposing this driver, I think it will interest a lot of people.

About your last point, let me mention ST view on this:

This is a question of trade-off. Using HAL has some drawbacks (some might
be objectives like footprint or more subjective like CamelCase), but it's
definitely possible in Zephyr.
Main interest is that it will help having a reliable driver working on
all STM32
SoCs supporting ethernet (today in zephyr: stm32f407, stm32f429, stm32f469,
stm32f7xx under review) faster. More supported SoCs also means more
interested people, which will help in getting the driver mature.
In the end, it should save time to work on end applications rather than
porting.

A native driver, specific to the IP (which might also be used on other
SoCs),
might be more optimized, but this might also take you some time before
having it mature for complex IPs such as ethernet (or USB..).

In any case, I think it would be great to have your driver upstreamed in
a first time. Then, time will tell if a native driver is nice to have or
required.

Erwan



On 3 June 2017 at 15:17, Erwin Rol <mailinglists@...
<mailto:mailinglists@...>> wrote:

Hello,

I made an initial STM32F4 Ethernet driver. It is based on the HAL
version, and doesn't try to be smart with DMA and packet buffer memory,
it just does memcpy to/from the DMA buffers. But it works.

It can be found at github;

https://github.com/lowlander/zephyr/commit/6a49b0e0c435f0529b97255c675fb57e54d23a54
<https://github.com/lowlander/zephyr/commit/6a49b0e0c435f0529b97255c675fb57e54d23a54>
Since it is for the olimex_stm32_e407 board (which is still pending to
be merged) I didn't want to create a pull request yet.

The driver use the STM32 HAL (or at least parts of it), and as I
mentioned "it works" but I am very uncertain about if I like the HAL
thing or not.

For example the ISR uses this __weak callbacks, but you will not have
access to your device structure anymore, and it is very unclear what is
happening. So I pretty much copied the whole ISR to only have the
possibility to access my network device structure passed to the ISR.

With the use of macros like "ETH" naming collisions are bound to happen
sooner or later.

The CamelCase naming convention of the HAL just gives me a headache, but
that's a matter of taste I guess.

I know I am opening a can of worms, but I really could use some guidance
on if using the HAL for more complex drivers like Ethernet is the way to
go or if just making a clean implementation is better.

- Erwin


_______________________________________________
Zephyr-devel mailing list
Zephyr-devel@...
<mailto:Zephyr-devel@...>
https://lists.zephyrproject.org/mailman/listinfo/zephyr-devel
<https://lists.zephyrproject.org/mailman/listinfo/zephyr-devel>

_______________________________________________
Zephyr-devel mailing list
Zephyr-devel@...
https://lists.zephyrproject.org/mailman/listinfo/zephyr-devel

Join devel@lists.zephyrproject.org to automatically receive all group messages.