drivers: net: mqnic: Add initial driver support for Corundum (MQNIC) #2849
Conversation
eliza-balas
requested review from
CiprianRegus,
amiclaus,
btogorean,
buha,
chegbeli,
danmois,
dbogdan,
machschmitt,
mhennerich,
nunojsa and
rbolboac
as code owners
eliza-balas
changed the title
|
Hi, This is a lot of stuff to "dump" on a single PR for reviewing... Can't we make this better? Another thing that worries me is that I see this is an out of tree driver maintained externally. Do we have any plan to upstream this? This definitely worries me from a maintainability point of view... So depending on the above plan, I'm no so sure if a deep review makes sense given that we might not want to drastically change the code |
Hi Nuno, We don't know how much time the Corundum drivers will be maintained by the Corundum team externally, but we made some changes to the Corundum Infrastructure IP cores in hdl and this PR also reflects those changes for support with microblaze, and there is an on-going effort to add support for other platforms too. We don't plan to upstream the Corundum driver. But we plan to add more functionality to Corundum Infrastructure in hdl and that functionality will also reflect in the driver. Other then importing the MQNIC sources as they are with the new changes added, I don't see how we can do things differently. |
Given the size of this, it will impose (i suspect) a significant burden to maintain this out of tree which is far from ideal... |
There was a problem hiding this comment.
Notes on the CONFIG_PCI usage, considering the CI log.
Also, after that is resolved, there are a few of modpost issues.
edit: this fixes would go great on corundum repo also.
In disagreement with nuno, if the driver is self contained in the mqnic folder, doing a
git checkout --theirs drivers/net/mqnic ; git add drivers/net/mqnic
for all merge conflicts should do the job, if it ever goes upstream.
But we definitely should at least keep our repo and corundum repo in sync and fix issues on both, since we are using their driver anyway.
eliza-balas
force-pushed
the
staging/corundum_pr
branch
2 times, most recently
from
c480811 to
ea409b9
Compare
eliza-balas
force-pushed
the
staging/corundum_pr
branch
2 times, most recently
from
a04bbd7 to
448e8e6
Compare
gastmaier
left a comment
gastmaier
left a comment
There was a problem hiding this comment.
Hi Eliza, see the comments about the depends and select
Thank you
drivers/net/mqnic/Kconfig
Outdated
|
|
||
| menuconfig CORUNDUM_MQNIC | ||
| tristate "Corundum MQNIC support" | ||
| depends on NET_DEVLINK && I2C_ALGOBIT |
There was a problem hiding this comment.
Hi Eliza,
| depends on NET_DEVLINK && I2C_ALGOBIT | |
| select NET_DEVLINK | |
| select I2C_ALGOBIT | |
| depends on RTC_CLASS |
The driver is not being inferred for compilation because
NET_DEVLINK and I2C_ALGOBIT are "SELECT" symbols, aka, the menuconfig entry looks like a mess.
If I am wrong, the depends needs to match some existing rule that select them instead.
For example, with my suggestion just changing the from depends to select, I still get
mqnic_main.c:(.text+0x85c): undefined reference to `rtc_time64_to_tm'
which could be nodding for an existing rule, but it is provided by RTC_CLASS and belong to neither symbols depends, so I don't think is the case.
And in this case, you also need to add
depends on RTC_CLASS
I used the container locally to try things out, using the exposed methods set_arch, auto_set_kconfig, and compile_kernel
https://analogdevicesinc.github.io/linux/ci.html#setting-up-and-running
Thank you
There was a problem hiding this comment.
Okay, great! I will make the changes, thank you too!
eliza-balas
force-pushed
the
staging/corundum_pr
branch
2 times, most recently
from
63b9dbc to
b215638
Compare
Add MQNIC (Multi-Queue Network Interface Card) source files. Corundum (MQNIC) is an open-source FPGA-based network interface card (NIC) platform. The original Corundum source files are imported from the Corundum repo: https://github.com/ucsdsysnet/corundum/tree/master/modules/mqnic Some changes are added to the original source files: - Port to linux kernel v6.12 - Cleanup code to comply with checkpatch guidelines - Fix incorrect pointer check in mqnic_platform_module_eeprom_put(); - Use cast for dma_addr_t to u64 before right-shifting by 32 bits in BAR address setup to make it compatible to 32-bit platforms; - Use div_s64 for compatibility with microblaze; - Use CORUNDUM_MQNIC_PCI entry to enable PCI support Signed-off-by: Eliza Balas <[email protected]>
…m MQNIC Add YAML-based devicetree binding documentation for the Corundum mqnic FPGA-based multi-queue network interface controller. Signed-off-by: Eliza Balas <[email protected]>
…the list Enable Corundum mqnic and dependencies: RESET_GPIO I2C_ALGOBIT RTC_CLASS Signed-off-by: Eliza Balas <[email protected]>
This patch adds an example device tree for VCU118 / ad9081_fmca_ebz project with Corundum (MQNIC) support. Signed-off-by: Eliza Balas <[email protected]>
eliza-balas
force-pushed
the
staging/corundum_pr
branch
from
b215638 to
bae6c1b
Compare
gastmaier
left a comment
gastmaier
left a comment
There was a problem hiding this comment.
We don't plan to upstream the Corundum driver.
Considering this.
If this changes, I think it makes sense to nuke the driver, then bring it back, to not have to deal with conflicts.
As long as the conflicts are Kconfig/Makefile alphabetic both added", I am ok with merging.
there are a lot of warnings caught by the ci that are useful for corundum source/upstream.
PR Description
PR Type
PR Checklist