Re: [PATCH 02/10] dpaa_eth: add support for DPAA Ethernet
On Wed, 2015-07-22 at 19:16 +0300, Madalin Bucur wrote: This introduces the Freescale Data Path Acceleration Architecture (DPAA) Ethernet driver (dpaa_eth) that builds upon the DPAA QMan, BMan, PAMU and FMan drivers to deliver Ethernet connectivity on the Freescale DPAA QorIQ platforms. Signed-off-by: Madalin Bucur madalin.bu...@freescale.com --- drivers/net/ethernet/freescale/Kconfig |2 + drivers/net/ethernet/freescale/Makefile|1 + drivers/net/ethernet/freescale/dpaa/Kconfig| 46 + drivers/net/ethernet/freescale/dpaa/Makefile | 13 + drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 827 + drivers/net/ethernet/freescale/dpaa/dpaa_eth.h | 447 +++ .../net/ethernet/freescale/dpaa/dpaa_eth_common.c | 1254 .../net/ethernet/freescale/dpaa/dpaa_eth_common.h | 119 ++ drivers/net/ethernet/freescale/dpaa/dpaa_eth_sg.c | 406 +++ 9 files changed, 3115 insertions(+) create mode 100644 drivers/net/ethernet/freescale/dpaa/Kconfig create mode 100644 drivers/net/ethernet/freescale/dpaa/Makefile create mode 100644 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c create mode 100644 drivers/net/ethernet/freescale/dpaa/dpaa_eth.h create mode 100644 drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c create mode 100644 drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h create mode 100644 drivers/net/ethernet/freescale/dpaa/dpaa_eth_sg.c diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig index f3f89cc..92198be 100644 --- a/drivers/net/ethernet/freescale/Kconfig +++ b/drivers/net/ethernet/freescale/Kconfig @@ -92,4 +92,6 @@ config GIANFAR and MPC86xx family of chips, the eTSEC on LS1021A and the FEC on the 8540. +source drivers/net/ethernet/freescale/dpaa/Kconfig + endif # NET_VENDOR_FREESCALE diff --git a/drivers/net/ethernet/freescale/Makefile b/drivers/net/ethernet/freescale/Makefile index 4097c58..ae13dc5 100644 --- a/drivers/net/ethernet/freescale/Makefile +++ b/drivers/net/ethernet/freescale/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_FS_ENET) += fs_enet/ obj-$(CONFIG_FSL_PQ_MDIO) += fsl_pq_mdio.o obj-$(CONFIG_FSL_XGMAC_MDIO) += xgmac_mdio.o obj-$(CONFIG_GIANFAR) += gianfar_driver.o +obj-$(CONFIG_FSL_DPAA_ETH) += dpaa/ obj-$(CONFIG_PTP_1588_CLOCK_GIANFAR) += gianfar_ptp.o gianfar_driver-objs := gianfar.o \ gianfar_ethtool.o diff --git a/drivers/net/ethernet/freescale/dpaa/Kconfig b/drivers/net/ethernet/freescale/dpaa/Kconfig new file mode 100644 index 000..1f3a203 --- /dev/null +++ b/drivers/net/ethernet/freescale/dpaa/Kconfig @@ -0,0 +1,46 @@ +menuconfig FSL_DPAA_ETH + tristate DPAA Ethernet + depends on FSL_SOC FSL_BMAN FSL_QMAN FSL_FMAN + select PHYLIB + select FSL_FMAN_MAC + ---help--- + Data Path Acceleration Architecture Ethernet driver, + supporting the Freescale QorIQ chips. + Depends on Freescale Buffer Manager and Queue Manager + driver and Frame Manager Driver. + +if FSL_DPAA_ETH + +config FSL_DPAA_CS_THRESHOLD_1G + hex Egress congestion threshold on 1G ports + range 0x1000 0x1000 + default 0x0600 + ---help--- + The size in bytes of the egress Congestion State notification threshold on 1G ports. + The 1G dTSECs can quite easily be flooded by cores doing Tx in a tight loop + (e.g. by sending UDP datagrams at while(1) speed), + and the larger the frame size, the more acute the problem. + So we have to find a balance between these factors: +- avoiding the device staying congested for a prolonged time (risking + the netdev watchdog to fire - see also the tx_timeout module param); + - affecting performance of protocols such as TCP, which otherwise + behave well under the congestion notification mechanism; +- preventing the Tx cores from tightly-looping (as if the congestion + threshold was too low to be effective); +- running out of memory if the CS threshold is set too high. + +config FSL_DPAA_CS_THRESHOLD_10G + hex Egress congestion threshold on 10G ports + range 0x1000 0x2000 + default 0x1000 + ---help --- + The size in bytes of the egress Congestion State notification threshold on 10G ports. + +config FSL_DPAA_INGRESS_CS_THRESHOLD + hex Ingress congestion threshold on FMan ports + default 0x1000 + ---help--- + The size in bytes of the ingress tail-drop threshold on FMan ports. + Traffic piling up above this value will be rejected by QMan and discarded by FMan. + +endif # FSL_DPAA_ETH diff --git a/drivers/net/ethernet/freescale/dpaa/Makefile b/drivers/net/ethernet/freescale/dpaa/Makefile new file mode 100644 index 000..cf126dd ---
Re: [PATCH 02/10] dpaa_eth: add support for DPAA Ethernet
On Fri, 2015-07-24 at 10:45 -0500, Bucur Madalin-Cristian-B32716 wrote:
-Original Message-
From: Joe Perches [mailto:j...@perches.com]
On Wed, 2015-07-22 at 19:16 +0300, Madalin Bucur wrote:
+static int __init dpa_load(void)
+{
[]
+ err = platform_driver_register(dpa_driver);
+ if (unlikely(err 0)) {
+ pr_err(KBUILD_MODNAME
+ : %s:%hu:%s(): platform_driver_register() = %d\n,
+ KBUILD_BASENAME .c, __LINE__, __func__, err);
+ }
+
+ pr_debug(KBUILD_MODNAME : %s:%s() -\n,
+ KBUILD_BASENAME .c, __func__);
Perhaps these should use pr_fmt
Agree.
How about dropping all that complexity, and just using pr_debug(%s\n,
__func__), or dev_dbg where possible?
+static void __exit dpa_unload(void)
+{
+ pr_debug(KBUILD_MODNAME : - %s:%s()\n,
+ KBUILD_BASENAME .c, __func__);
dynamic debug has __func__ available and perhaps
the function tracer might be used instead.
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
[]
+#define __hot
curious.
Maybe it'd be good to add a real __hot to compiler.h
They're mostly there to make readers aware the code is critical, any
changes could mess performance.
Mostly or entirely? Why not just use a comment, which could also point out
specific things that were done for performance?
-Scott
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 02/10] dpaa_eth: add support for DPAA Ethernet
-Original Message-
From: Joe Perches [mailto:j...@perches.com]
On Wed, 2015-07-22 at 19:16 +0300, Madalin Bucur wrote:
This introduces the Freescale Data Path Acceleration Architecture
(DPAA) Ethernet driver (dpaa_eth) that builds upon the DPAA QMan,
BMan, PAMU and FMan drivers to deliver Ethernet connectivity on
the Freescale DPAA QorIQ platforms.
trivia:
+static void __hot _dpa_tx_conf(struct net_device *net_dev,
+ const struct dpa_priv_s *priv,
+ struct dpa_percpu_priv_s *percpu_priv,
+ const struct qm_fd *fd,
+ u32 fqid)
+{
[]
+static struct dpa_bp * __cold
+dpa_priv_bp_probe(struct device *dev)
Do the __hot and __cold markings really matter?
Some of them may be questionable.
Some may be, yes. I need to go through all of them.
+static int __init dpa_load(void)
+{
[]
+ err = platform_driver_register(dpa_driver);
+ if (unlikely(err 0)) {
+ pr_err(KBUILD_MODNAME
+ : %s:%hu:%s(): platform_driver_register() = %d\n,
+ KBUILD_BASENAME .c, __LINE__, __func__, err);
+ }
+
+ pr_debug(KBUILD_MODNAME : %s:%s() -\n,
+KBUILD_BASENAME .c, __func__);
Perhaps these should use pr_fmt
Agree.
+static void __exit dpa_unload(void)
+{
+ pr_debug(KBUILD_MODNAME : - %s:%s()\n,
+KBUILD_BASENAME .c, __func__);
dynamic debug has __func__ available and perhaps
the function tracer might be used instead.
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
[]
+#define __hot
curious.
Maybe it'd be good to add a real __hot to compiler.h
They're mostly there to make readers aware the code is critical, any
changes could mess performance.
+struct dpa_buffer_layout_s {
+ u16 priv_data_size;
+ boolparse_results;
+ booltime_stamp;
+ boolhash_results;
+ u16 data_align;
+};
+struct dpa_fq {
+ struct qman_fq fq_base;
+ struct list_head list;
+ struct net_device *net_dev;
some inconsistent indentation here and there
Yes, I've tried to align the style but given the many editors along the time
the code existed
there still are areas out of sync.
+struct dpa_bp {
+ struct bman_pool*pool;
+ u8 bpid;
+ struct device *dev;
+ union {
+ /* The buffer pools used for the private ports are initialized
+* with target_count buffers for each CPU; at runtime the
+* number of buffers per CPU is constantly brought back to
this
+* level
+*/
+ int target_count;
+ /* The configured value for the number of buffers in the
pool,
+* used for shared port buffer pools
+*/
+ int config_count;
+ };
Anonymous unions are relatively rare
We liked the direct access to members...
In this particular case the use is a bit excessive, we can do without it.
+ struct {
+ /**
Maybe the /** style should be avoided
Will fix.
+* All egress queues to a given net device belong to one
+* (and the same) congestion group.
+*/
+ struct qman_cgr cgr;
+ } cgr_data;
[]
+int dpa_stop(struct net_device *net_dev)
+{
[]
+ err = mac_dev-stop(mac_dev);
+ if (unlikely(err 0))
+ netif_err(priv, ifdown, net_dev, mac_dev-stop() = %d\n,
+ err);
Some of the likely/unlikely uses may not
be useful/necessary.
In this particular case it's gratuitous, I'll go through all of them.
+
+ for_each_port_device(i, mac_dev-port_dev) {
+ error = fm_port_disable(
+ fm_port_drv_handle(mac_dev-
port_dev[i]));
+ err = error ? error : err;
if (error)
err = error;
is more obvious to me.
Yes, it's more readable.
Thank you,
Madalin
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/10] dpaa_eth: add support for DPAA Ethernet
On Wed, 2015-07-22 at 19:16 +0300, Madalin Bucur wrote:
This introduces the Freescale Data Path Acceleration Architecture
(DPAA) Ethernet driver (dpaa_eth) that builds upon the DPAA QMan,
BMan, PAMU and FMan drivers to deliver Ethernet connectivity on
the Freescale DPAA QorIQ platforms.
trivia:
+static void __hot _dpa_tx_conf(struct net_device *net_dev,
+const struct dpa_priv_s *priv,
+struct dpa_percpu_priv_s *percpu_priv,
+const struct qm_fd *fd,
+u32 fqid)
+{
[]
+static struct dpa_bp * __cold
+dpa_priv_bp_probe(struct device *dev)
Do the __hot and __cold markings really matter?
Some of them may be questionable.
+static int __init dpa_load(void)
+{
[]
+ err = platform_driver_register(dpa_driver);
+ if (unlikely(err 0)) {
+ pr_err(KBUILD_MODNAME
+ : %s:%hu:%s(): platform_driver_register() = %d\n,
+ KBUILD_BASENAME .c, __LINE__, __func__, err);
+ }
+
+ pr_debug(KBUILD_MODNAME : %s:%s() -\n,
+ KBUILD_BASENAME .c, __func__);
Perhaps these should use pr_fmt
+static void __exit dpa_unload(void)
+{
+ pr_debug(KBUILD_MODNAME : - %s:%s()\n,
+ KBUILD_BASENAME .c, __func__);
dynamic debug has __func__ available and perhaps
the function tracer might be used instead.
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
[]
+#define __hot
curious.
Maybe it'd be good to add a real __hot to compiler.h
+struct dpa_buffer_layout_s {
+ u16 priv_data_size;
+ boolparse_results;
+ booltime_stamp;
+ boolhash_results;
+ u16 data_align;
+};
+struct dpa_fq {
+ struct qman_fq fq_base;
+ struct list_head list;
+ struct net_device *net_dev;
some inconsistent indentation here and there
+struct dpa_bp {
+ struct bman_pool*pool;
+ u8 bpid;
+ struct device *dev;
+ union {
+ /* The buffer pools used for the private ports are initialized
+ * with target_count buffers for each CPU; at runtime the
+ * number of buffers per CPU is constantly brought back to this
+ * level
+ */
+ int target_count;
+ /* The configured value for the number of buffers in the pool,
+ * used for shared port buffer pools
+ */
+ int config_count;
+ };
Anonymous unions are relatively rare
+ struct {
+ /**
Maybe the /** style should be avoided
+ * All egress queues to a given net device belong to one
+ * (and the same) congestion group.
+ */
+ struct qman_cgr cgr;
+ } cgr_data;
[]
+int dpa_stop(struct net_device *net_dev)
+{
[]
+ err = mac_dev-stop(mac_dev);
+ if (unlikely(err 0))
+ netif_err(priv, ifdown, net_dev, mac_dev-stop() = %d\n,
+ err);
Some of the likely/unlikely uses may not
be useful/necessary.
+
+ for_each_port_device(i, mac_dev-port_dev) {
+ error = fm_port_disable(
+ fm_port_drv_handle(mac_dev-port_dev[i]));
+ err = error ? error : err;
if (error)
err = error;
is more obvious to me.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
