<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-compose;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="#0563C1" vlink="#954F72">
<div class="WordSection1">
<p class="MsoNormal">> -----Original Message-----<o:p></o:p></p>
<p class="MsoNormal">> From: Kumar Gala [mailto:galak@kernel.crashing.org]<o:p></o:p></p>
<p class="MsoNormal">> <o:p></o:p></p>
<p class="MsoNormal">> On Mar 17, 2015, at 1:58 PM, Madalin Bucur <madalin.bucur@Freescale.com><o:p></o:p></p>
<p class="MsoNormal">> wrote:<o:p></o:p></p>
<p class="MsoNormal">> <o:p></o:p></p>
<p class="MsoNormal">> > This introduces the Freescale Data Path Acceleration Architecture<o:p></o:p></p>
<p class="MsoNormal">> > (DPAA) Ethernet driver (dpaa_eth) that builds upon the DPAA QMan,<o:p></o:p></p>
<p class="MsoNormal">> > BMan, PAMU and FMan drivers to deliver Ethernet connectivity on<o:p></o:p></p>
<p class="MsoNormal">> > the Freescale DPAA QorIQ platforms.<o:p></o:p></p>
<p class="MsoNormal">> ><o:p></o:p></p>
<p class="MsoNormal">> > Signed-off-by: Madalin Bucur <madalin.bucur@freescale.com><o:p></o:p></p>
<p class="MsoNormal">> > ---<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">[snip]<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">> > diff --git a/drivers/net/ethernet/freescale/Makefile<o:p></o:p></p>
<p class="MsoNormal">> b/drivers/net/ethernet/freescale/Makefile<o:p></o:p></p>
<p class="MsoNormal">> > index 71debd1..b6c10ab 100644<o:p></o:p></p>
<p class="MsoNormal">> > --- a/drivers/net/ethernet/freescale/Makefile<o:p></o:p></p>
<p class="MsoNormal">> > +++ b/drivers/net/ethernet/freescale/Makefile<o:p></o:p></p>
<p class="MsoNormal">> > @@ -12,6 +12,7 @@ obj-$(CONFIG_FS_ENET) += fs_enet/<o:p></o:p></p>
<p class="MsoNormal">> > obj-$(CONFIG_FSL_PQ_MDIO) += fsl_pq_mdio.o<o:p></o:p></p>
<p class="MsoNormal">> > obj-$(CONFIG_FSL_XGMAC_MDIO) += xgmac_mdio.o<o:p></o:p></p>
<p class="MsoNormal">> > obj-$(CONFIG_GIANFAR) += gianfar_driver.o<o:p></o:p></p>
<p class="MsoNormal">> > +obj-$(if $(CONFIG_FSL_DPAA_ETH),y) += dpaa/<o:p></o:p></p>
<p class="MsoNormal">> <o:p></o:p></p>
<p class="MsoNormal">> Why isn’t<o:p></o:p></p>
<p class="MsoNormal">> <o:p></o:p></p>
<p class="MsoNormal">> obj-$(CONFIG_FSL_DPAA_ETH) += dpaa/<o:p></o:p></p>
<p class="MsoNormal">> <o:p></o:p></p>
<p class="MsoNormal">> enough?<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">It is, will fix.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">> > diff --git a/drivers/net/ethernet/freescale/dpaa/Kconfig<o:p></o:p></p>
<p class="MsoNormal">> b/drivers/net/ethernet/freescale/dpaa/Kconfig<o:p></o:p></p>
<p class="MsoNormal">> > new file mode 100644<o:p></o:p></p>
<p class="MsoNormal">> > index 0000000..7ef703c<o:p></o:p></p>
<p class="MsoNormal">> > --- /dev/null<o:p></o:p></p>
<p class="MsoNormal">> > +++ b/drivers/net/ethernet/freescale/dpaa/Kconfig<o:p></o:p></p>
<p class="MsoNormal">> > @@ -0,0 +1,49 @@<o:p></o:p></p>
<p class="MsoNormal">> > +menuconfig FSL_DPAA_ETH<o:p></o:p></p>
<p class="MsoNormal">> > + tristate "DPAA Ethernet"<o:p></o:p></p>
<p class="MsoNormal">> > + depends on FSL_SOC && FSL_BMAN && FSL_QMAN && FSL_FMAN<o:p></o:p></p>
<p class="MsoNormal">> > + select PHYLIB<o:p></o:p></p>
<p class="MsoNormal">> > + select FSL_FMAN_MAC<o:p></o:p></p>
<p class="MsoNormal">> > + ---help---<o:p></o:p></p>
<p class="MsoNormal">> > + Data Path Acceleration Architecture Ethernet driver,<o:p></o:p></p>
<p class="MsoNormal">> > + supporting the Freescale QorIQ chips.<o:p></o:p></p>
<p class="MsoNormal">> > + Depends on Freescale Buffer Manager and Queue Manager<o:p></o:p></p>
<p class="MsoNormal">> > + driver and Frame Manager Driver.<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > +if FSL_DPAA_ETH<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > +config FSL_DPAA_CS_THRESHOLD_1G<o:p></o:p></p>
<p class="MsoNormal">> > + hex "Egress congestion threshold on 1G ports"<o:p></o:p></p>
<p class="MsoNormal">> > + depends on FSL_DPAA_ETH<o:p></o:p></p>
<p class="MsoNormal">> > + range 0x1000 0x10000000<o:p></o:p></p>
<p class="MsoNormal">> > + default "0x06000000"<o:p></o:p></p>
<p class="MsoNormal">> > + ---help---<o:p></o:p></p>
<p class="MsoNormal">> > + The size in bytes of the egress Congestion State notification<o:p></o:p></p>
<p class="MsoNormal">> threshold on 1G ports.<o:p></o:p></p>
<p class="MsoNormal">> > + The 1G dTSECs can quite easily be flooded by cores doing Tx in a<o:p></o:p></p>
<p class="MsoNormal">> tight loop<o:p></o:p></p>
<p class="MsoNormal">> > + (e.g. by sending UDP datagrams at "while(1) speed"),<o:p></o:p></p>
<p class="MsoNormal">> > + and the larger the frame size, the more acute the problem.<o:p></o:p></p>
<p class="MsoNormal">> > + So we have to find a balance between these factors:<o:p></o:p></p>
<p class="MsoNormal">> > + - avoiding the device staying congested for a prolonged time<o:p></o:p></p>
<p class="MsoNormal">> (risking<o:p></o:p></p>
<p class="MsoNormal">> > + the netdev watchdog to fire - see also the tx_timeout module<o:p></o:p></p>
<p class="MsoNormal">> param);<o:p></o:p></p>
<p class="MsoNormal">> > + - affecting performance of protocols such as TCP, which otherwise<o:p></o:p></p>
<p class="MsoNormal">> > + behave well under the congestion notification mechanism;<o:p></o:p></p>
<p class="MsoNormal">> > + - preventing the Tx cores from tightly-looping (as if the<o:p></o:p></p>
<p class="MsoNormal">> congestion<o:p></o:p></p>
<p class="MsoNormal">> > + threshold was too low to be effective);<o:p></o:p></p>
<p class="MsoNormal">> > + - running out of memory if the CS threshold is set too high.<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > +config FSL_DPAA_CS_THRESHOLD_10G<o:p></o:p></p>
<p class="MsoNormal">> > + hex "Egress congestion threshold on 10G ports"<o:p></o:p></p>
<p class="MsoNormal">> > + depends on FSL_DPAA_ETH<o:p></o:p></p>
<p class="MsoNormal">> > + range 0x1000 0x20000000<o:p></o:p></p>
<p class="MsoNormal">> > + default "0x10000000"<o:p></o:p></p>
<p class="MsoNormal">> > + ---help ---<o:p></o:p></p>
<p class="MsoNormal">> > + The size in bytes of the egress Congestion State notification<o:p></o:p></p>
<p class="MsoNormal">> threshold on 10G ports.<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > +config FSL_DPAA_INGRESS_CS_THRESHOLD<o:p></o:p></p>
<p class="MsoNormal">> > + hex "Ingress congestion threshold on FMan ports"<o:p></o:p></p>
<p class="MsoNormal">> > + depends on FSL_DPAA_ETH<o:p></o:p></p>
<p class="MsoNormal">> > + default "0x10000000"<o:p></o:p></p>
<p class="MsoNormal">> > + ---help---<o:p></o:p></p>
<p class="MsoNormal">> > + The size in bytes of the ingress tail-drop threshold on FMan ports.<o:p></o:p></p>
<p class="MsoNormal">> > + Traffic piling up above this value will be rejected by QMan and<o:p></o:p></p>
<p class="MsoNormal">> discarded by FMan.<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> <o:p></o:p></p>
<p class="MsoNormal">> Do these thresholds really need to be kconfig options? Are they not<o:p></o:p></p>
<p class="MsoNormal">> changeable at runtime?<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Probably Bogdan thought it was an easy way to provide basic means to tweak them<o:p></o:p></p>
<p class="MsoNormal">and also provide some explanation for them. The current setting is chosen to keep<o:p></o:p></p>
<p class="MsoNormal">them mostly out of the way but still address a corner case a customer reported, if<o:p></o:p></p>
<p class="MsoNormal">I remember correctly. No runtime change is supported.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">> > diff --git a/drivers/net/ethernet/freescale/dpaa/Makefile<o:p></o:p></p>
<p class="MsoNormal">> b/drivers/net/ethernet/freescale/dpaa/Makefile<o:p></o:p></p>
<p class="MsoNormal">> > new file mode 100644<o:p></o:p></p>
<p class="MsoNormal">> > index 0000000..bdeb04e<o:p></o:p></p>
<p class="MsoNormal">> > --- /dev/null<o:p></o:p></p>
<p class="MsoNormal">> > +++ b/drivers/net/ethernet/freescale/dpaa/Makefile<o:p></o:p></p>
<p class="MsoNormal">> > @@ -0,0 +1,14 @@<o:p></o:p></p>
<p class="MsoNormal">> > +#<o:p></o:p></p>
<p class="MsoNormal">> > +# Makefile for the Freescale DPAA Ethernet controllers<o:p></o:p></p>
<p class="MsoNormal">> > +#<o:p></o:p></p>
<p class="MsoNormal">> > +ccflags-y += -DVERSION=\”\"<o:p></o:p></p>
<p class="MsoNormal">> <o:p></o:p></p>
<p class="MsoNormal">> Is this really needed, if so we need to fix that.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Unused, will remove.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">[snip]<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > +#define DPA_DESCRIPTION "FSL DPAA Ethernet driver"<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > +MODULE_LICENSE("Dual BSD/GPL");<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > +MODULE_AUTHOR("Andy Fleming <afleming@freescale.com>");<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > +MODULE_DESCRIPTION(DPA_DESCRIPTION);<o:p></o:p></p>
<p class="MsoNormal">> <o:p></o:p></p>
<p class="MsoNormal">> These are typically at the end of the file, and kept together (ie no blank lines<o:p></o:p></p>
<p class="MsoNormal">> between them)<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Will move them.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">[snip]<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > +static struct platform_device_id dpa_devtype[] = {<o:p></o:p></p>
<p class="MsoNormal">> > + {<o:p></o:p></p>
<p class="MsoNormal">> > + .name = "dpaa-ethernet",<o:p></o:p></p>
<p class="MsoNormal">> > + .driver_data = 0,<o:p></o:p></p>
<p class="MsoNormal">> > + }, {<o:p></o:p></p>
<p class="MsoNormal">> > + }<o:p></o:p></p>
<p class="MsoNormal">> > +};<o:p></o:p></p>
<p class="MsoNormal">> > +MODULE_DEVICE_TABLE(platform, dpa_devtype);<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > +static struct platform_driver dpa_driver = {<o:p></o:p></p>
<p class="MsoNormal">> > + .driver = {<o:p></o:p></p>
<p class="MsoNormal">> > + .name = KBUILD_MODNAME,<o:p></o:p></p>
<p class="MsoNormal">> > + .owner = THIS_MODULE,<o:p></o:p></p>
<p class="MsoNormal">> > + },<o:p></o:p></p>
<p class="MsoNormal">> > + .id_table = dpa_devtype,<o:p></o:p></p>
<p class="MsoNormal">> > + .probe = dpaa_eth_priv_probe,<o:p></o:p></p>
<p class="MsoNormal">> > + .remove = dpa_remove<o:p></o:p></p>
<p class="MsoNormal">> > +};<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > +static int __init __cold dpa_load(void)<o:p></o:p></p>
<p class="MsoNormal">> > +{<o:p></o:p></p>
<p class="MsoNormal">> > + int _errno;<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > + pr_info(DPA_DESCRIPTION " (" VERSION ")\n");<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > + /* initialise dpaa_eth mirror values */<o:p></o:p></p>
<p class="MsoNormal">> > + dpa_rx_extra_headroom = fm_get_rx_extra_headroom();<o:p></o:p></p>
<p class="MsoNormal">> > + dpa_max_frm = fm_get_max_frm();<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > + _errno = platform_driver_register(&dpa_driver);<o:p></o:p></p>
<p class="MsoNormal">> > + if (unlikely(_errno < 0)) {<o:p></o:p></p>
<p class="MsoNormal">> > + pr_err(KBUILD_MODNAME<o:p></o:p></p>
<p class="MsoNormal">> > + ": %s:%hu:%s(): platform_driver_register() = %d\n",<o:p></o:p></p>
<p class="MsoNormal">> > + KBUILD_BASENAME ".c", __LINE__, __func__,<o:p></o:p></p>
<p class="MsoNormal">> _errno);<o:p></o:p></p>
<p class="MsoNormal">> > + }<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > + pr_debug(KBUILD_MODNAME ": %s:%s() ->\n",<o:p></o:p></p>
<p class="MsoNormal">> > + KBUILD_BASENAME ".c", __func__);<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > + return _errno;<o:p></o:p></p>
<p class="MsoNormal">> > +}<o:p></o:p></p>
<p class="MsoNormal">> > +module_init(dpa_load);<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > +static void __exit __cold dpa_unload(void)<o:p></o:p></p>
<p class="MsoNormal">> > +{<o:p></o:p></p>
<p class="MsoNormal">> > + pr_debug(KBUILD_MODNAME ": -> %s:%s()\n",<o:p></o:p></p>
<p class="MsoNormal">> > + KBUILD_BASENAME ".c", __func__);<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > + platform_driver_unregister(&dpa_driver);<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > + /* Only one channel is used and needs to be relased after all<o:p></o:p></p>
<p class="MsoNormal">> > + * interfaces are removed<o:p></o:p></p>
<p class="MsoNormal">> > + */<o:p></o:p></p>
<p class="MsoNormal">> > + dpa_release_channel();<o:p></o:p></p>
<p class="MsoNormal">> > +<o:p></o:p></p>
<p class="MsoNormal">> > + pr_debug(KBUILD_MODNAME ": %s:%s() ->\n",<o:p></o:p></p>
<p class="MsoNormal">> > + KBUILD_BASENAME ".c", __func__);<o:p></o:p></p>
<p class="MsoNormal">> > +}<o:p></o:p></p>
<p class="MsoNormal">> > +module_exit(dpa_unload);<o:p></o:p></p>
<p class="MsoNormal">> <o:p></o:p></p>
<p class="MsoNormal">> What’s the point of this dpa platform driver?<o:p></o:p></p>
<p class="MsoNormal">> <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">It replaces the device tree node probing, based on the feedback [1] received from Scott.<o:p></o:p></p>
<p class="MsoNormal">The FMan MAC driver creates a device for the Ethernet driver to probe against in<o:p></o:p></p>
<p class="MsoNormal">dpaa_eth_add_device(), see [2]. This replaces the rejected model where a configuration<o:p></o:p></p>
<p class="MsoNormal">node for Ethernet was added to the device tree with a reference to the FMan MAC node.<o:p></o:p></p>
<p class="MsoNormal">The platform device provides the Ethernet driver with FMan MAC identification and the<o:p></o:p></p>
<p class="MsoNormal">MAC dev needed back in the adjust_link() call:<o:p></o:p></p>
<p class="MsoNormal">...<o:p></o:p></p>
<p class="MsoNormal">struct dpaa_eth_data {<o:p></o:p></p>
<p class="MsoNormal"> struct device_node *mac_node;<o:p></o:p></p>
<p class="MsoNormal"> struct mac_device *mac_dev;<o:p></o:p></p>
<p class="MsoNormal"> int mac_hw_id;<o:p></o:p></p>
<p class="MsoNormal"> int fman_hw_id;<o:p></o:p></p>
<p class="MsoNormal">};<o:p></o:p></p>
<p class="MsoNormal">...<o:p></o:p></p>
<p class="MsoNormal">static void adjust_link(struct net_device *net_dev)<o:p></o:p></p>
<p class="MsoNormal">{<o:p></o:p></p>
<p class="MsoNormal"> struct device *dev = net_dev->dev.parent;<o:p></o:p></p>
<p class="MsoNormal"> struct dpaa_eth_data *eth_data = dev->platform_data;<o:p></o:p></p>
<p class="MsoNormal"> struct mac_device *mac_dev = eth_data->mac_dev;<o:p></o:p></p>
<p class="MsoNormal">...<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">[snip]<o:p></o:p></p>
<p class="MsoNormal">> > +static inline int __hot dpa_xmit(struct dpa_priv_s *priv,<o:p></o:p></p>
<p class="MsoNormal">> > + struct rtnl_link_stats64 *percpu_stats,<o:p></o:p></p>
<p class="MsoNormal">> > + int queue,<o:p></o:p></p>
<p class="MsoNormal">> > + struct qm_fd *fd)<o:p></o:p></p>
<p class="MsoNormal">> <o:p></o:p></p>
<p class="MsoNormal">> Is there a reason this isnt just in the .c file?<o:p></o:p></p>
<p class="MsoNormal"><o:p></o:p></p>
<p class="MsoNormal">It's also used by the un-submitted advanced drivers that compile against the<o:p></o:p></p>
<p class="MsoNormal">submitted code (there are a few other things used in common - such as the code<o:p></o:p></p>
<p class="MsoNormal">extracted in the dpaa_eth_common.c file).<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">> > +EXPORT_SYMBOL(dpa_mac_dev_get);<o:p></o:p></p>
<p class="MsoNormal">> <o:p></o:p></p>
<p class="MsoNormal">> why is this exported?<o:p></o:p></p>
<p class="MsoNormal">> <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Same reason as above.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">[snip]<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Thank you for your time,<o:p></o:p></p>
<p class="MsoNormal">Madalin<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">[1] https://patchwork.ozlabs.org/patch/415831/<o:p></o:p></p>
<p class="MsoNormal">[2] https://patchwork.ozlabs.org/patch/448874/<o:p></o:p></p>
</div>
</body>
</html>