| Message ID | 20241230101528.3836531-9-tianx@yunsilicon.com (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net-next/yunsilicon: ADD Yunsilicon XSC Ethernet Driver | expand |
> +static void *xsc_eth_add(struct xsc_core_device *xdev) > +{ > + adapter->dev = &adapter->pdev->dev; > + adapter->xdev = (void *)xdev; > + xdev->netdev = (void *)netdev; Why have casts to void *? There are clear type here, rather than it being a cookie passed via an abstract interface. Andrew
Andrew, In another module(xsc_pci), we check xdev_netdev is NULL or not to see whether network module(xsc_eth) is loaded. we do not care about the real type,and we do not want to include the related header files in other modules. so we use the void type. 在 2024/12/30 下午11:29, Andrew Lunn 写道: >> +static void *xsc_eth_add(struct xsc_core_device *xdev) >> +{ >> + adapter->dev = &adapter->pdev->dev; >> + adapter->xdev = (void *)xdev; >> + xdev->netdev = (void *)netdev; > Why have casts to void *? There are clear type here, rather than it > being a cookie passed via an abstract interface. > > Andrew
On Tue, Dec 31, 2024 at 12:13:23AM +0800, weihonggang wrote: > Andrew, In another module(xsc_pci), we check xdev_netdev is NULL or not > to see whether network module(xsc_eth) is loaded. we do not care about > the real type,and we do not want to include the related header files in > other modules. so we use the void type. Please don't top post. If all you care about is if the module is loaded, turn it into a bool, and set it true. Andrew
在 2024/12/31 下午1:12, Andrew Lunn 写道: > On Tue, Dec 31, 2024 at 12:13:23AM +0800, weihonggang wrote: >> Andrew, In another module(xsc_pci), we check xdev_netdev is NULL or not >> to see whether network module(xsc_eth) is loaded. we do not care about >> the real type,and we do not want to include the related header files in >> other modules. so we use the void type. > Please don't top post. > > If all you care about is if the module is loaded, turn it into a bool, > and set it true. > > Andrew Sorry, I checked the code,rdma module(not in current patch) will use the veriable and turn it to real type。so it can not turn into bool type instead。Any other suggestions?
On 2024/12/31 13:12, Andrew Lunn wrote: > On Tue, Dec 31, 2024 at 12:13:23AM +0800, weihonggang wrote: >> Andrew, In another module(xsc_pci), we check xdev_netdev is NULL or not >> to see whether network module(xsc_eth) is loaded. we do not care about >> the real type,and we do not want to include the related header files in >> other modules. so we use the void type. > Please don't top post. > > If all you care about is if the module is loaded, turn it into a bool, > and set it true. > > Andrew Hi, Andrew Not only the PCI module, but our later RDMA module also needs the netdev structure in xsc_core_device to access network information. To simplify the review, we haven't submitted the RDMA module, but keeping the netdev helps avoid repeated changes when submitting later. Best regards, Xin
On Tue, Dec 31, 2024 at 05:40:15PM +0800, tianx wrote: > On 2024/12/31 13:12, Andrew Lunn wrote: > > On Tue, Dec 31, 2024 at 12:13:23AM +0800, weihonggang wrote: > >> Andrew, In another module(xsc_pci), we check xdev_netdev is NULL or not > >> to see whether network module(xsc_eth) is loaded. we do not care about > >> the real type,and we do not want to include the related header files in > >> other modules. so we use the void type. > > Please don't top post. > > > > If all you care about is if the module is loaded, turn it into a bool, > > and set it true. > > > > Andrew > > Hi, Andrew > > Not only the PCI module, but our later RDMA module also needs the netdev > structure in xsc_core_device to access network information. To simplify > the review, we haven't submitted the RDMA module, but keeping the netdev > helps avoid repeated changes when submitting later. Don't worry about RDMA at this point, your driver structure doesn't fit current multi-subsystem design. You will need to completely rewrite your "net-next/yunsilicon: Device and interface management" patch anyway when you will send us RDMA part. Please use auxiliary bus infrastructure to split your driver to separate it separate modules, instead of reinventing it. Thanks > > Best regards, > > Xin >
On Tue, Dec 31, 2024 at 05:40:15PM +0800, tianx wrote: > On 2024/12/31 13:12, Andrew Lunn wrote: > > On Tue, Dec 31, 2024 at 12:13:23AM +0800, weihonggang wrote: > >> Andrew, In another module(xsc_pci), we check xdev_netdev is NULL or not > >> to see whether network module(xsc_eth) is loaded. we do not care about > >> the real type,and we do not want to include the related header files in > >> other modules. so we use the void type. > > Please don't top post. > > > > If all you care about is if the module is loaded, turn it into a bool, > > and set it true. > > > > Andrew > > Hi, Andrew > > Not only the PCI module, but our later RDMA module also needs the netdev > structure in xsc_core_device to access network information. To simplify > the review, we haven't submitted the RDMA module, but keeping the netdev > helps avoid repeated changes when submitting later. So you might be rewriting this all in order to get the RDMA code merged.... But if not, just drop the void * cast. Make xdev_netdev a struct net_device *, etc. Andrew
diff --git a/drivers/net/ethernet/yunsilicon/Makefile b/drivers/net/ethernet/yunsilicon/Makefile index 6fc8259a7..65b9a6265 100644 --- a/drivers/net/ethernet/yunsilicon/Makefile +++ b/drivers/net/ethernet/yunsilicon/Makefile @@ -4,5 +4,5 @@ # Makefile for the Yunsilicon device drivers. # -# obj-$(CONFIG_YUNSILICON_XSC_ETH) += xsc/net/ +obj-$(CONFIG_YUNSILICON_XSC_ETH) += xsc/net/ obj-$(CONFIG_YUNSILICON_XSC_PCI) += xsc/pci/ \ No newline at end of file diff --git a/drivers/net/ethernet/yunsilicon/xsc/common/xsc_core.h b/drivers/net/ethernet/yunsilicon/xsc/common/xsc_core.h index 2eb9c3c80..ee43b5f47 100644 --- a/drivers/net/ethernet/yunsilicon/xsc/common/xsc_core.h +++ b/drivers/net/ethernet/yunsilicon/xsc/common/xsc_core.h @@ -429,6 +429,7 @@ struct xsc_core_device { struct pci_dev *pdev; struct device *device; struct xsc_priv priv; + void *netdev; void *eth_priv; struct xsc_dev_resource *dev_res; diff --git a/drivers/net/ethernet/yunsilicon/xsc/net/main.c b/drivers/net/ethernet/yunsilicon/xsc/net/main.c new file mode 100644 index 000000000..8d29e88cf --- /dev/null +++ b/drivers/net/ethernet/yunsilicon/xsc/net/main.c @@ -0,0 +1,116 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2021-2025, Shanghai Yunsilicon Technology Co., Ltd. + * All rights reserved. + */ + +#include <linux/netdevice.h> +#include <linux/etherdevice.h> +#include "common/xsc_core.h" +#include "xsc_eth_common.h" +#include "xsc_eth.h" + +static int xsc_get_max_num_channels(struct xsc_core_device *xdev) +{ + return min_t(int, xdev->dev_res->eq_table.num_comp_vectors, + XSC_ETH_MAX_NUM_CHANNELS); +} + +static void *xsc_eth_add(struct xsc_core_device *xdev) +{ + struct xsc_adapter *adapter; + struct net_device *netdev; + int num_chl, num_tc; + int err; + + num_chl = xsc_get_max_num_channels(xdev); + num_tc = xdev->caps.max_tc; + + netdev = alloc_etherdev_mqs(sizeof(struct xsc_adapter), + num_chl * num_tc, num_chl); + if (!netdev) { + pr_err("alloc_etherdev_mqs failed, txq=%d, rxq=%d\n", + (num_chl * num_tc), num_chl); + return NULL; + } + + netdev->dev.parent = &xdev->pdev->dev; + adapter = netdev_priv(netdev); + adapter->netdev = netdev; + adapter->pdev = xdev->pdev; + adapter->dev = &adapter->pdev->dev; + adapter->xdev = (void *)xdev; + xdev->eth_priv = adapter; + + err = register_netdev(netdev); + if (err) { + netdev_err(netdev, "register_netdev failed, err=%d\n", err); + goto err_free_netdev; + } + + xdev->netdev = (void *)netdev; + + return adapter; + +err_free_netdev: + free_netdev(netdev); + + return NULL; +} + +static void xsc_eth_remove(struct xsc_core_device *xdev, void *context) +{ + struct xsc_adapter *adapter; + + if (!xdev) + return; + + adapter = xdev->eth_priv; + if (!adapter) { + netdev_err(adapter->netdev, "failed! adapter is null\n"); + return; + } + + unregister_netdev(adapter->netdev); + + free_netdev(adapter->netdev); + + xdev->netdev = NULL; + xdev->eth_priv = NULL; +} + +static struct xsc_interface xsc_interface = { + .add = xsc_eth_add, + .remove = xsc_eth_remove, + .event = NULL, + .protocol = XSC_INTERFACE_PROTOCOL_ETH, +}; + +static void xsc_remove_eth_driver(void) +{ + xsc_unregister_interface(&xsc_interface); +} + +static __init int xsc_net_driver_init(void) +{ + int ret; + + ret = xsc_register_interface(&xsc_interface); + if (ret != 0) { + pr_err("failed to register interface\n"); + goto out; + } + return 0; +out: + return -1; +} + +static __exit void xsc_net_driver_exit(void) +{ + xsc_remove_eth_driver(); +} + +module_init(xsc_net_driver_init); +module_exit(xsc_net_driver_exit); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("Yunsilicon XSC ethernet driver"); diff --git a/drivers/net/ethernet/yunsilicon/xsc/net/xsc_eth.h b/drivers/net/ethernet/yunsilicon/xsc/net/xsc_eth.h new file mode 100644 index 000000000..0c70c0d59 --- /dev/null +++ b/drivers/net/ethernet/yunsilicon/xsc/net/xsc_eth.h @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright (C) 2021-2025, Shanghai Yunsilicon Technology Co., Ltd. + * All rights reserved. + */ + +#ifndef __XSC_ETH_H +#define __XSC_ETH_H + +struct xsc_adapter { + struct net_device *netdev; + struct pci_dev *pdev; + struct device *dev; + struct xsc_core_device *xdev; +}; + +#endif /* __XSC_ETH_H */ diff --git a/drivers/net/ethernet/yunsilicon/xsc/net/xsc_eth_common.h b/drivers/net/ethernet/yunsilicon/xsc/net/xsc_eth_common.h new file mode 100644 index 000000000..b5640f05d --- /dev/null +++ b/drivers/net/ethernet/yunsilicon/xsc/net/xsc_eth_common.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright (C) 2021-2025, Shanghai Yunsilicon Technology Co., Ltd. + * All rights reserved. + */ + +#ifndef __XSC_ETH_COMMON_H +#define __XSC_ETH_COMMON_H + +#define XSC_LOG_INDIR_RQT_SIZE 0x8 + +#define XSC_INDIR_RQT_SIZE BIT(XSC_LOG_INDIR_RQT_SIZE) +#define XSC_ETH_MIN_NUM_CHANNELS 2 +#define XSC_ETH_MAX_NUM_CHANNELS XSC_INDIR_RQT_SIZE + +#endif