espressif / esp-hosted

Hosted Solution (Linux/MCU) with ESP32 (Wi-Fi + BT + BLE)
Other
675 stars 158 forks source link

esp_hosted_ng: fix build failure on Linux 6.9.0 #358

Closed giuliobenetti closed 5 months ago

giuliobenetti commented 5 months ago

With Linux commit: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=620d269f29a569ba37419cc03cf1da2d55f6252a spi_master compatibility has gone, so let's redefine missing needed macros spi_master and spi_master_put() locally if Linux version is >= 6.9.0.

yann-morin-1998 commented 5 months ago

Since the initial conversion to 'controller' was started in 2017 with kernel commit 8caab75fd2c2, first released with kernel 4.13, would it rather make sense to actually stop using the old 'master' symbols, and directly use the 'controller' ones now, and discontinue relying on that compatibility layer at all?

giuliobenetti commented 5 months ago

Since the initial conversion to 'controller' was started in 2017 with kernel commit 8caab75fd2c2, first released with kernel 4.13, would it rather make sense to actually stop using the old 'master' symbols, and directly use the 'controller' ones now, and discontinue relying on that compatibility layer at all?

It's a good idea, but I see a pending PR for Linux 4.5.0 that has never been merged: https://github.com/espressif/esp-hosted/pull/157

But oldest Linux maintained is 4.19: https://www.kernel.org/category/releases.html and on December 2024 it will cease.

If @mantriyogesh is ok I can rework the patch to substitute spi_master with spi_controller

mantriyogesh commented 5 months ago

@giuliobenetti I had added some comments in #157 for your attention.

If @mantriyogesh is ok I can rework the patch to substitute spi_master with spi_controller

Sure. We have not yet supporting 6.9.X in master. So, Unless it is not breaking backward compatibility, we are more than happy to get your contributions. Please make a note, as much as possible, to try to add kernel version dependent changes into esp_kernel_port.h. This helps to keep the code readability and separate kernel dependent changes from main driver code.

Although 4.X is technically an ancient kernel, as long as people use it, we need to keep supporting. Unfortunately, even till date, we get requests for even 3.x kernels.

giuliobenetti commented 5 months ago

@giuliobenetti I had added some comments in #157 for your attention.

If @mantriyogesh is ok I can rework the patch to substitute spi_master with spi_controller

Sure. We have not yet supporting 6.9.X in master. So, Unless it is not breaking backward compatibility, we are more than happy to get your contributions. Please make a note, as much as possible, to try to add kernel version dependent changes into esp_kernel_port.h.

Good idea, just reworked placing macro into this ^^^ file.

This helps to keep the code readability and separate kernel dependent changes from main driver code.

Although 4.X is technically an ancient kernel, as long as people use it, we need to keep supporting. Unfortunately, even till date, we get requests for even 3.x kernels.

kapilkedawat commented 5 months ago

Hi @giuliobenetti , thank you for your contribution. Closed by https://github.com/espressif/esp-hosted/commit/30865423a25c565664bff4363d96b7f96791f80a @mantriyogesh