RE: [PATCH net-next v2 5/6] dpaa_eth: change DMA device
> -Original Message-
> From: Jakub Kicinski
> Sent: Wednesday, October 23, 2019 12:15 AM
> To: Madalin-cristian Bucur
> Cc: da...@davemloft.net; netdev@vger.kernel.org; Roy Pledge
> ; Laurentiu Tudor
> Subject: Re: [PATCH net-next v2 5/6] dpaa_eth: change DMA device
>
> On Tue, 22 Oct 2019 14:15:00 +0300, Madalin Bucur wrote:
> > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > index 8d5686d88d30..639cafaa59b8 100644
> > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > @@ -1335,15 +1335,15 @@ static void dpaa_fd_release(const struct
> net_device *net_dev,
> > vaddr = phys_to_virt(qm_fd_addr(fd));
> > sgt = vaddr + qm_fd_get_offset(fd);
> >
> > - dma_unmap_single(dpaa_bp->dev, qm_fd_addr(fd), dpaa_bp->size,
> > -DMA_FROM_DEVICE);
> > + dma_unmap_single(dpaa_bp->priv->rx_dma_dev, qm_fd_addr(fd),
> > +dpaa_bp->size, DMA_FROM_DEVICE);
> >
> > dpaa_release_sgt_members(sgt);
> >
> > - addr = dma_map_single(dpaa_bp->dev, vaddr, dpaa_bp->size,
> > - DMA_FROM_DEVICE);
> > - if (dma_mapping_error(dpaa_bp->dev, addr)) {
> > - dev_err(dpaa_bp->dev, "DMA mapping failed");
> > + addr = dma_map_single(dpaa_bp->priv->rx_dma_dev, vaddr,
> > + dpaa_bp->size, DMA_FROM_DEVICE);
> > + if (dma_mapping_error(dpaa_bp->priv->rx_dma_dev, addr)) {
> > + netdev_err(net_dev, "DMA mapping failed");
>
> You seem to be missing new line chars at the end of the "DMA mapping
> failed" messages :( Could you please fix all of them and repost?
>
> > return;
> > }
> > bm_buffer_set64(&bmb, addr);
Sure
RE: [PATCH net-next 5/6] dpaa_eth: change DMA device
> -Original Message-
> From: Laurentiu Tudor
> Sent: Tuesday, October 22, 2019 1:35 PM
> To: Madalin-cristian Bucur ; da...@davemloft.net;
> netdev@vger.kernel.org
> Cc: Roy Pledge
> Subject: Re: [PATCH net-next 5/6] dpaa_eth: change DMA device
>
>
>
> On 22.10.2019 12:10, Madalin-cristian Bucur wrote:
> >> -Original Message-
> >> From: Laurentiu Tudor
> >> Sent: Tuesday, October 22, 2019 11:50 AM
> >> To: Madalin-cristian Bucur ;
> da...@davemloft.net;
> >> netdev@vger.kernel.org
> >> Cc: Roy Pledge
> >> Subject: Re: [PATCH net-next 5/6] dpaa_eth: change DMA device
> >>
> >> Hello,
> >>
> >> On 21.10.2019 15:28, Madalin-cristian Bucur wrote:
> >>> The DPAA Ethernet driver is using the FMan MAC as the device for DMA
> >>> mapping. This is not actually correct, as the real DMA device is the
> >>> FMan port (the FMan Rx port for reception and the FMan Tx port for
> >>> transmission). Changing the device used for DMA mapping to the Fman
> >>> Rx and Tx port devices.
> >>>
> >>> Signed-off-by: Madalin Bucur
> >>> Signed-off-by: Laurentiu Tudor
> >>> ---
> >>>drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 105 +-
> ---
> >>
> >>>drivers/net/ethernet/freescale/dpaa/dpaa_eth.h | 8 +-
> >>>2 files changed, 62 insertions(+), 51 deletions(-)
> >>>
> >>> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> >> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> >>> index 8d5686d88d30..639cafaa59b8 100644
> >>> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> >>> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> >>> @@ -1335,15 +1335,15 @@ static void dpaa_fd_release(const struct
> >> net_device *net_dev,
> >>> vaddr = phys_to_virt(qm_fd_addr(fd));
> >>> sgt = vaddr + qm_fd_get_offset(fd);
> >>>
> >>> - dma_unmap_single(dpaa_bp->dev, qm_fd_addr(fd), dpaa_bp->size,
> >>> - DMA_FROM_DEVICE);
> >>> + dma_unmap_single(dpaa_bp->priv->rx_dma_dev, qm_fd_addr(fd),
> >>> + dpaa_bp->size, DMA_FROM_DEVICE);
> >>>
> >>> dpaa_release_sgt_members(sgt);
> >>>
> >>> - addr = dma_map_single(dpaa_bp->dev, vaddr, dpaa_bp->size,
> >>> - DMA_FROM_DEVICE);
> >>> - if (dma_mapping_error(dpaa_bp->dev, addr)) {
> >>> - dev_err(dpaa_bp->dev, "DMA mapping failed");
> >>> + addr = dma_map_single(dpaa_bp->priv->rx_dma_dev, vaddr,
> >>> + dpaa_bp->size, DMA_FROM_DEVICE);
> >>> + if (dma_mapping_error(dpaa_bp->priv->rx_dma_dev, addr)) {
> >>> + netdev_err(net_dev, "DMA mapping failed");
> >>> return;
> >>> }
> >>> bm_buffer_set64(&bmb, addr);
> >>> @@ -1488,7 +1488,7 @@ static int dpaa_enable_tx_csum(struct dpaa_priv
> >> *priv,
> >>>
> >>>static int dpaa_bp_add_8_bufs(const struct dpaa_bp *dpaa_bp)
> >>>{
> >>> - struct device *dev = dpaa_bp->dev;
> >>> + struct net_device *net_dev = dpaa_bp->priv->net_dev;
> >>> struct bm_buffer bmb[8];
> >>> dma_addr_t addr;
> >>> void *new_buf;
> >>> @@ -1497,16 +1497,18 @@ static int dpaa_bp_add_8_bufs(const struct
> >> dpaa_bp *dpaa_bp)
> >>> for (i = 0; i < 8; i++) {
> >>> new_buf = netdev_alloc_frag(dpaa_bp->raw_size);
> >>> if (unlikely(!new_buf)) {
> >>> - dev_err(dev, "netdev_alloc_frag() failed, size %zu\n",
> >>> - dpaa_bp->raw_size);
> >>> + netdev_err(net_dev,
> >>> +"netdev_alloc_frag() failed, size %zu\n",
> >>> +dpaa_bp->raw_size);
> >>> goto release_previous_buffs;
> >>> }
> >>> new_buf = PTR_ALIGN(new_buf, SMP_CACHE_BYTES);
> >>>
> >>&
RE: [PATCH net-next 5/6] dpaa_eth: change DMA device
> -Original Message-
> From: Laurentiu Tudor
> Sent: Tuesday, October 22, 2019 11:50 AM
> To: Madalin-cristian Bucur ; da...@davemloft.net;
> netdev@vger.kernel.org
> Cc: Roy Pledge
> Subject: Re: [PATCH net-next 5/6] dpaa_eth: change DMA device
>
> Hello,
>
> On 21.10.2019 15:28, Madalin-cristian Bucur wrote:
> > The DPAA Ethernet driver is using the FMan MAC as the device for DMA
> > mapping. This is not actually correct, as the real DMA device is the
> > FMan port (the FMan Rx port for reception and the FMan Tx port for
> > transmission). Changing the device used for DMA mapping to the Fman
> > Rx and Tx port devices.
> >
> > Signed-off-by: Madalin Bucur
> > Signed-off-by: Laurentiu Tudor
> > ---
> > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 105 +
>
> > drivers/net/ethernet/freescale/dpaa/dpaa_eth.h | 8 +-
> > 2 files changed, 62 insertions(+), 51 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > index 8d5686d88d30..639cafaa59b8 100644
> > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > @@ -1335,15 +1335,15 @@ static void dpaa_fd_release(const struct
> net_device *net_dev,
> > vaddr = phys_to_virt(qm_fd_addr(fd));
> > sgt = vaddr + qm_fd_get_offset(fd);
> >
> > - dma_unmap_single(dpaa_bp->dev, qm_fd_addr(fd), dpaa_bp->size,
> > -DMA_FROM_DEVICE);
> > + dma_unmap_single(dpaa_bp->priv->rx_dma_dev, qm_fd_addr(fd),
> > +dpaa_bp->size, DMA_FROM_DEVICE);
> >
> > dpaa_release_sgt_members(sgt);
> >
> > - addr = dma_map_single(dpaa_bp->dev, vaddr, dpaa_bp->size,
> > - DMA_FROM_DEVICE);
> > - if (dma_mapping_error(dpaa_bp->dev, addr)) {
> > - dev_err(dpaa_bp->dev, "DMA mapping failed");
> > + addr = dma_map_single(dpaa_bp->priv->rx_dma_dev, vaddr,
> > + dpaa_bp->size, DMA_FROM_DEVICE);
> > + if (dma_mapping_error(dpaa_bp->priv->rx_dma_dev, addr)) {
> > + netdev_err(net_dev, "DMA mapping failed");
> > return;
> > }
> > bm_buffer_set64(&bmb, addr);
> > @@ -1488,7 +1488,7 @@ static int dpaa_enable_tx_csum(struct dpaa_priv
> *priv,
> >
> > static int dpaa_bp_add_8_bufs(const struct dpaa_bp *dpaa_bp)
> > {
> > - struct device *dev = dpaa_bp->dev;
> > + struct net_device *net_dev = dpaa_bp->priv->net_dev;
> > struct bm_buffer bmb[8];
> > dma_addr_t addr;
> > void *new_buf;
> > @@ -1497,16 +1497,18 @@ static int dpaa_bp_add_8_bufs(const struct
> dpaa_bp *dpaa_bp)
> > for (i = 0; i < 8; i++) {
> > new_buf = netdev_alloc_frag(dpaa_bp->raw_size);
> > if (unlikely(!new_buf)) {
> > - dev_err(dev, "netdev_alloc_frag() failed, size %zu\n",
> > - dpaa_bp->raw_size);
> > + netdev_err(net_dev,
> > + "netdev_alloc_frag() failed, size %zu\n",
> > + dpaa_bp->raw_size);
> > goto release_previous_buffs;
> > }
> > new_buf = PTR_ALIGN(new_buf, SMP_CACHE_BYTES);
> >
> > - addr = dma_map_single(dev, new_buf,
> > + addr = dma_map_single(dpaa_bp->priv->rx_dma_dev, new_buf,
> > dpaa_bp->size, DMA_FROM_DEVICE);
> > - if (unlikely(dma_mapping_error(dev, addr))) {
> > - dev_err(dpaa_bp->dev, "DMA map failed");
> > + if (unlikely(dma_mapping_error(dpaa_bp->priv->rx_dma_dev,
> > + addr))) {
> > + netdev_err(net_dev, "DMA map failed");
> > goto release_previous_buffs;
> > }
> >
> > @@ -1634,7 +1636,7 @@ static struct sk_buff *dpaa_cleanup_tx_fd(const
> struct dpaa_priv *priv,
> >
> > if (unlikely(qm_fd_get_format(fd) == qm_fd_sg)) {
> > nr_frags = skb_shinfo(skb)->nr_frags;
> > - dma_unmap_single(dev, addr,
> > + dma_unmap_single(priv->tx_dma_dev, addr,
> >
RE: [PATCH net-next 5/6] dpaa_eth: change DMA device
> -Original Message- > From: Jakub Kicinski > Sent: Tuesday, October 22, 2019 7:23 AM > To: Madalin-cristian Bucur > Cc: da...@davemloft.net; netdev@vger.kernel.org; Roy Pledge > ; Laurentiu Tudor > Subject: Re: [PATCH net-next 5/6] dpaa_eth: change DMA device > > On Mon, 21 Oct 2019 12:28:02 +0000, Madalin-cristian Bucur wrote: > > The DPAA Ethernet driver is using the FMan MAC as the device for DMA > > mapping. This is not actually correct, as the real DMA device is the > > FMan port (the FMan Rx port for reception and the FMan Tx port for > > transmission). Changing the device used for DMA mapping to the Fman > > Rx and Tx port devices. > > > > Signed-off-by: Madalin Bucur > > Signed-off-by: Laurentiu Tudor > > Curious, we also have a patch for fixing this for IXP400. > Is there something in recent kernels that uncovers this bug? Hi Jakub, it's related to the IOMMU on ARM64, this change is just one of the many required to get IOMMU working on DPAA 1 platforms. The device used for DMA mapping was the MAC but in the DPAA it's another HW block that is actually doing the DMA transfers, the port (be it Rx or Tx). This fix just makes the code correct, to actually enable IOMMU there are some more changes and some are under discussion on other threads [1]. I'm pushing these changes so I can make other modifications to the DPAA driver while the IOMMU related open items are solved. Regards, Madalin 1. Laurentiu's IOMMU related changes: https://lkml.org/lkml/2019/4/22/357 https://lore.kernel.org/patchwork/cover/997994/ https://lore.kernel.org/patchwork/project/lkml/list/?series=396215
RE: [PATCH net-next 0/6] DPAA Ethernet changes
> -Original Message- > From: Jakub Kicinski > Sent: Tuesday, October 22, 2019 7:26 AM > To: Madalin-cristian Bucur > Cc: da...@davemloft.net; netdev@vger.kernel.org; Roy Pledge > ; Laurentiu Tudor > Subject: Re: [PATCH net-next 0/6] DPAA Ethernet changes > > On Mon, 21 Oct 2019 12:27:51 +0000, Madalin-cristian Bucur wrote: > > Here's a series of changes for the DPAA Ethernet, addressing minor > > or unapparent issues in the codebase, adding probe ordering based on > > a recently added DPAA QMan API, removing some redundant code. > > Hi Madalin! > > Patch 2 looks like it may be a bug fix but I gather it has a dependency > in net-next so it can't go to net? It's a fix for a theoretical issue that is not reproducing with the current code base. Future changes related to the IOMMU support may make this issue visible. > More importantly - I think your From: line on this posting is > > Madalin-cristian Bucur > > While the sign-off on the patches you wrote is: > > Madalin Bucur > > I think these gotta be identical otherwise the bots which ensure the > author added his sign-off may scream at us. The formatted patches look like this: >From 55a524a41099fa9b2f5fbbb9f3a87108437942bb Mon Sep 17 00:00:00 2001 From: Madalin Bucur Date: Mon, 21 Oct 2019 15:21:26 +0300 Subject: [PATCH net-next 0/6] DPAA Ethernet changes Content-Type: text/plain; charset="us-ascii" Reply-to: madalin.bu...@nxp.com but then there are some MS servers trying to be helpful and the message ends up like this: From: Madalin-cristian Bucur To: "da...@davemloft.net" , "netdev@vger.kernel.org" CC: Roy Pledge , Laurentiu Tudor , Madalin-cristian Bucur Subject: [PATCH net-next 0/6] DPAA Ethernet changes Thread-Topic: [PATCH net-next 0/6] DPAA Ethernet changes Thread-Index: AQHViAr0ehww7MPYPUqedXDa00qudg== X-MS-Exchange-MessageSentRepresentingType: 1 Date: Mon, 21 Oct 2019 12:27:51 + Message-ID: <1571660862-18313-1-git-send-email-madalin.bu...@nxp.com> Reply-To: Madalin-cristian Bucur Return-Path: madalin.bu...@nxp.com It's probably a good time to think about pull requests...
[PATCH net-next 3/6] dpaa_eth: remove redundant code
Condition was previously checked, removing duplicate code. Signed-off-by: Madalin Bucur --- drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c index 75eeb2ef409f..8d5686d88d30 100644 --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c @@ -2304,10 +2304,6 @@ static enum qman_cb_dqrr_result rx_default_dqrr(struct qman_portal *portal, return qman_cb_dqrr_consume; } - dpaa_bp = dpaa_bpid2pool(fd->bpid); - if (!dpaa_bp) - return qman_cb_dqrr_consume; - dma_unmap_single(dpaa_bp->dev, addr, dpaa_bp->size, DMA_FROM_DEVICE); /* prefetch the first 64 bytes of the frame or the SGT start */ -- 2.1.0
[PATCH net-next 1/6] fsl/fman: don't touch liodn base regs reserved on non-PAMU SoCs
From: Laurentiu Tudor
The liodn base registers are specific to PAMU based NXP systems and are
reserved on SMMU based ones. Don't access them unless PAMU is compiled in.
Signed-off-by: Laurentiu Tudor
Signed-off-by: Madalin Bucur
---
drivers/net/ethernet/freescale/fman/fman.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/fman/fman.c
b/drivers/net/ethernet/freescale/fman/fman.c
index 210749bf1eac..934111def0be 100644
--- a/drivers/net/ethernet/freescale/fman/fman.c
+++ b/drivers/net/ethernet/freescale/fman/fman.c
@@ -634,6 +634,9 @@ static void set_port_liodn(struct fman *fman, u8 port_id,
{
u32 tmp;
+ iowrite32be(liodn_ofst, &fman->bmi_regs->fmbm_spliodn[port_id - 1]);
+ if (!IS_ENABLED(CONFIG_FSL_PAMU))
+ return;
/* set LIODN base for this port */
tmp = ioread32be(&fman->dma_regs->fmdmplr[port_id / 2]);
if (port_id % 2) {
@@ -644,7 +647,6 @@ static void set_port_liodn(struct fman *fman, u8 port_id,
tmp |= liodn_base << DMA_LIODN_SHIFT;
}
iowrite32be(tmp, &fman->dma_regs->fmdmplr[port_id / 2]);
- iowrite32be(liodn_ofst, &fman->bmi_regs->fmbm_spliodn[port_id - 1]);
}
static void enable_rams_ecc(struct fman_fpm_regs __iomem *fpm_rg)
@@ -1942,6 +1944,8 @@ static int fman_init(struct fman *fman)
fman->liodn_offset[i] =
ioread32be(&fman->bmi_regs->fmbm_spliodn[i - 1]);
+ if (!IS_ENABLED(CONFIG_FSL_PAMU))
+ continue;
liodn_base = ioread32be(&fman->dma_regs->fmdmplr[i / 2]);
if (i % 2) {
/* FMDM_PLR LSB holds LIODN base for odd ports */
--
2.1.0
[PATCH net-next 0/6] DPAA Ethernet changes
Here's a series of changes for the DPAA Ethernet, addressing minor or unapparent issues in the codebase, adding probe ordering based on a recently added DPAA QMan API, removing some redundant code. Laurentiu Tudor (3): fsl/fman: don't touch liodn base regs reserved on non-PAMU SoCs dpaa_eth: defer probing after qbman fsl/fman: add API to get the device behind a fman port Madalin Bucur (3): dpaa_eth: remove redundant code dpaa_eth: change DMA device fsl/fman: remove unused struct member drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 128 +++- drivers/net/ethernet/freescale/dpaa/dpaa_eth.h | 8 +- drivers/net/ethernet/freescale/fman/fman.c | 6 +- drivers/net/ethernet/freescale/fman/fman_port.c | 17 +++- drivers/net/ethernet/freescale/fman/fman_port.h | 2 + 5 files changed, 108 insertions(+), 53 deletions(-) -- 2.1.0
[PATCH net-next 2/6] dpaa_eth: defer probing after qbman
From: Laurentiu Tudor
If the DPAA 1 Ethernet driver gets probed before the QBMan driver it will
cause a boot crash. Add predictability in the probing order by deferring
the Ethernet driver probe after QBMan and portals by using the recently
introduced QBMan APIs.
Signed-off-by: Laurentiu Tudor
Signed-off-by: Madalin Bucur
---
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 31 ++
1 file changed, 31 insertions(+)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index b4b82b9c5cd6..75eeb2ef409f 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -2773,6 +2773,37 @@ static int dpaa_eth_probe(struct platform_device *pdev)
int err = 0, i, channel;
struct device *dev;
+ err = bman_is_probed();
+ if (!err)
+ return -EPROBE_DEFER;
+ if (err < 0) {
+ dev_err(&pdev->dev, "failing probe due to bman probe error\n");
+ return -ENODEV;
+ }
+ err = qman_is_probed();
+ if (!err)
+ return -EPROBE_DEFER;
+ if (err < 0) {
+ dev_err(&pdev->dev, "failing probe due to qman probe error\n");
+ return -ENODEV;
+ }
+ err = bman_portals_probed();
+ if (!err)
+ return -EPROBE_DEFER;
+ if (err < 0) {
+ dev_err(&pdev->dev,
+ "failing probe due to bman portals probe error\n");
+ return -ENODEV;
+ }
+ err = qman_portals_probed();
+ if (!err)
+ return -EPROBE_DEFER;
+ if (err < 0) {
+ dev_err(&pdev->dev,
+ "failing probe due to qman portals probe error\n");
+ return -ENODEV;
+ }
+
/* device used for DMA mapping */
dev = pdev->dev.parent;
err = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(40));
--
2.1.0
[PATCH net-next 5/6] dpaa_eth: change DMA device
The DPAA Ethernet driver is using the FMan MAC as the device for DMA
mapping. This is not actually correct, as the real DMA device is the
FMan port (the FMan Rx port for reception and the FMan Tx port for
transmission). Changing the device used for DMA mapping to the Fman
Rx and Tx port devices.
Signed-off-by: Madalin Bucur
Signed-off-by: Laurentiu Tudor
---
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 105 +
drivers/net/ethernet/freescale/dpaa/dpaa_eth.h | 8 +-
2 files changed, 62 insertions(+), 51 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 8d5686d88d30..639cafaa59b8 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -1335,15 +1335,15 @@ static void dpaa_fd_release(const struct net_device
*net_dev,
vaddr = phys_to_virt(qm_fd_addr(fd));
sgt = vaddr + qm_fd_get_offset(fd);
- dma_unmap_single(dpaa_bp->dev, qm_fd_addr(fd), dpaa_bp->size,
-DMA_FROM_DEVICE);
+ dma_unmap_single(dpaa_bp->priv->rx_dma_dev, qm_fd_addr(fd),
+dpaa_bp->size, DMA_FROM_DEVICE);
dpaa_release_sgt_members(sgt);
- addr = dma_map_single(dpaa_bp->dev, vaddr, dpaa_bp->size,
- DMA_FROM_DEVICE);
- if (dma_mapping_error(dpaa_bp->dev, addr)) {
- dev_err(dpaa_bp->dev, "DMA mapping failed");
+ addr = dma_map_single(dpaa_bp->priv->rx_dma_dev, vaddr,
+ dpaa_bp->size, DMA_FROM_DEVICE);
+ if (dma_mapping_error(dpaa_bp->priv->rx_dma_dev, addr)) {
+ netdev_err(net_dev, "DMA mapping failed");
return;
}
bm_buffer_set64(&bmb, addr);
@@ -1488,7 +1488,7 @@ static int dpaa_enable_tx_csum(struct dpaa_priv *priv,
static int dpaa_bp_add_8_bufs(const struct dpaa_bp *dpaa_bp)
{
- struct device *dev = dpaa_bp->dev;
+ struct net_device *net_dev = dpaa_bp->priv->net_dev;
struct bm_buffer bmb[8];
dma_addr_t addr;
void *new_buf;
@@ -1497,16 +1497,18 @@ static int dpaa_bp_add_8_bufs(const struct dpaa_bp
*dpaa_bp)
for (i = 0; i < 8; i++) {
new_buf = netdev_alloc_frag(dpaa_bp->raw_size);
if (unlikely(!new_buf)) {
- dev_err(dev, "netdev_alloc_frag() failed, size %zu\n",
- dpaa_bp->raw_size);
+ netdev_err(net_dev,
+ "netdev_alloc_frag() failed, size %zu\n",
+ dpaa_bp->raw_size);
goto release_previous_buffs;
}
new_buf = PTR_ALIGN(new_buf, SMP_CACHE_BYTES);
- addr = dma_map_single(dev, new_buf,
+ addr = dma_map_single(dpaa_bp->priv->rx_dma_dev, new_buf,
dpaa_bp->size, DMA_FROM_DEVICE);
- if (unlikely(dma_mapping_error(dev, addr))) {
- dev_err(dpaa_bp->dev, "DMA map failed");
+ if (unlikely(dma_mapping_error(dpaa_bp->priv->rx_dma_dev,
+ addr))) {
+ netdev_err(net_dev, "DMA map failed");
goto release_previous_buffs;
}
@@ -1634,7 +1636,7 @@ static struct sk_buff *dpaa_cleanup_tx_fd(const struct
dpaa_priv *priv,
if (unlikely(qm_fd_get_format(fd) == qm_fd_sg)) {
nr_frags = skb_shinfo(skb)->nr_frags;
- dma_unmap_single(dev, addr,
+ dma_unmap_single(priv->tx_dma_dev, addr,
qm_fd_get_offset(fd) + DPAA_SGT_SIZE,
dma_dir);
@@ -1644,21 +1646,21 @@ static struct sk_buff *dpaa_cleanup_tx_fd(const struct
dpaa_priv *priv,
sgt = phys_to_virt(addr + qm_fd_get_offset(fd));
/* sgt[0] is from lowmem, was dma_map_single()-ed */
- dma_unmap_single(dev, qm_sg_addr(&sgt[0]),
+ dma_unmap_single(priv->tx_dma_dev, qm_sg_addr(&sgt[0]),
qm_sg_entry_get_len(&sgt[0]), dma_dir);
/* remaining pages were mapped with skb_frag_dma_map() */
for (i = 1; i <= nr_frags; i++) {
WARN_ON(qm_sg_entry_is_ext(&sgt[i]));
- dma_unmap_page(dev, qm_sg_addr(&sgt[i]),
+ dma_unmap_page(priv->tx_dma_dev, qm_sg_addr(&sgt[i]),
qm_sg_entry_get_len(&sgt[i]), dma_dir);
}
/* Free the page frag that we allocated on Tx */
skb_free_frag(phys_to_virt(addr));
} else {
-
[PATCH net-next 4/6] fsl/fman: add API to get the device behind a fman port
From: Laurentiu Tudor
Add an API that retrieves the 'struct device' that the specified FMan
port probed against. The new API will be used in a subsequent patch
that corrects the DMA devices used by the dpaa_eth driver.
Signed-off-by: Laurentiu Tudor
Signed-off-by: Madalin Bucur
---
drivers/net/ethernet/freescale/fman/fman_port.c | 14 ++
drivers/net/ethernet/freescale/fman/fman_port.h | 2 ++
2 files changed, 16 insertions(+)
diff --git a/drivers/net/ethernet/freescale/fman/fman_port.c
b/drivers/net/ethernet/freescale/fman/fman_port.c
index ee82ee1384eb..bd76c9730692 100644
--- a/drivers/net/ethernet/freescale/fman/fman_port.c
+++ b/drivers/net/ethernet/freescale/fman/fman_port.c
@@ -1728,6 +1728,20 @@ u32 fman_port_get_qman_channel_id(struct fman_port *port)
}
EXPORT_SYMBOL(fman_port_get_qman_channel_id);
+/**
+ * fman_port_get_device
+ * port: Pointer to the FMan port device
+ *
+ * Get the 'struct device' associated to the specified FMan port device
+ *
+ * Return: pointer to associated 'struct device'
+ */
+struct device *fman_port_get_device(struct fman_port *port)
+{
+ return port->dev;
+}
+EXPORT_SYMBOL(fman_port_get_device);
+
int fman_port_get_hash_result_offset(struct fman_port *port, u32 *offset)
{
if (port->buffer_offsets.hash_result_offset == ILLEGAL_BASE)
diff --git a/drivers/net/ethernet/freescale/fman/fman_port.h
b/drivers/net/ethernet/freescale/fman/fman_port.h
index 9dbb69f40121..82f12661a46d 100644
--- a/drivers/net/ethernet/freescale/fman/fman_port.h
+++ b/drivers/net/ethernet/freescale/fman/fman_port.h
@@ -157,4 +157,6 @@ int fman_port_get_tstamp(struct fman_port *port, const void
*data, u64 *tstamp);
struct fman_port *fman_port_bind(struct device *dev);
+struct device *fman_port_get_device(struct fman_port *port);
+
#endif /* __FMAN_PORT_H */
--
2.1.0
[PATCH net-next 6/6] fsl/fman: remove unused struct member
Remove unused struct member second_largest_buf_size. Also, an out of
bounds access would have occurred in the removed code if there was only
one buffer pool in use.
Signed-off-by: Madalin Bucur
---
drivers/net/ethernet/freescale/fman/fman_port.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fman/fman_port.c
b/drivers/net/ethernet/freescale/fman/fman_port.c
index bd76c9730692..87b26f063cc8 100644
--- a/drivers/net/ethernet/freescale/fman/fman_port.c
+++ b/drivers/net/ethernet/freescale/fman/fman_port.c
@@ -435,7 +435,6 @@ struct fman_port_cfg {
struct fman_port_rx_pools_params {
u8 num_of_pools;
- u16 second_largest_buf_size;
u16 largest_buf_size;
};
@@ -946,8 +945,6 @@ static int set_ext_buffer_pools(struct fman_port *port)
port->rx_pools_params.num_of_pools = ext_buf_pools->num_of_pools_used;
port->rx_pools_params.largest_buf_size =
sizes_array[ordered_array[ext_buf_pools->num_of_pools_used - 1]];
- port->rx_pools_params.second_largest_buf_size =
- sizes_array[ordered_array[ext_buf_pools->num_of_pools_used - 2]];
/* FMBM_RMPD reg. - pool depletion */
if (buf_pool_depletion->pools_grp_mode_enable) {
--
2.1.0
RE: Fw: [Bug 205215] New: Amiga X5000 Cyrus board DPAA network driver issue
> -Original Message- > From: Maxim Uvarov > Sent: Friday, October 18, 2019 11:42 AM > To: Stephen Hemminger > Cc: Madalin-cristian Bucur ; netdev > > Subject: Re: Fw: [Bug 205215] New: Amiga X5000 Cyrus board DPAA network > driver issue > > чт, 17 окт. 2019 г. в 20:27, Stephen Hemminger > : > > So what is happening, > > > > First of all the hardware is functional. When using AmigaOS4.1FE the > Ethernet > > adapter is fine. When using the adapter within U-boot (ping for example) > it is > > also working as expected. > > > > When booted into linux the ethernet adapter does not get up. > > The output of DMESG is: > > > > skateman@X5000LNX:~$ dmesg | grep eth > > [ 4.740603] fsl_dpaa_mac ffe4e6000.ethernet: FMan dTSEC version: > 0x08240101 > > [ 4.741026] fsl_dpaa_mac ffe4e6000.ethernet: FMan MAC address: > > 00:04:a3:6b:41:7c > > [ 4.741387] fsl_dpaa_mac ffe4e8000.ethernet: FMan dTSEC version: > 0x08240101 > > [ 4.741740] fsl_dpaa_mac ffe4e8000.ethernet: FMan MAC address: > > 00:1e:c0:f8:31:b5 > > [ 4.742001] fsl_dpaa_mac ffe4e.ethernet: > > of_get_mac_address(/soc@ffe00/fman@40/ethernet@e) failed > > sound like mac is missing in dt. > Indeed, if you do not set the required u-boot env entries, the MAC address fixup of the device tree will not be performed and the driver will not probe those MACs. The question is - are the interfaces that do probe functional? Madalin
patch inclusion in lts trees
Hi Florian, Steffen,
the fix below, addressing a problem from kernel v4.9, did not get picked
up in the lts trees, is there a reason for this? Are there more such fixes
that were left out?
Thank you,
Madalin
commit 7a474c36586f4277f930ab7e6865c97e44dfc3bc
Author: Florian Westphal
Date: Fri Jan 4 14:17:01 2019 +0100
xfrm: policy: increment xfrm_hash_generation on hash rebuild
Hash rebuild will re-set all the inexact entries, then re-insert them.
Lookups that can occur in parallel will therefore not find any policies.
This was safe when lookups were still guarded by rwlock.
After rcu-ification, lookups check the hash_generation seqcount to detect
when a hash resize takes place. Hash rebuild missed the needed increment.
Hash resizes and hash rebuilds cannot occur in parallel (both acquire
hash_resize_mutex), so just increment xfrm_hash_generation, like resize.
Fixes: a7c44247f704e3 ("xfrm: policy: make xfrm_policy_lookup_bytype lockless")
Signed-off-by: Florian Westphal
Signed-off-by: Steffen Klassert
[PATCH v2] net: phy: aquantia: readd XGMII support for AQR107
XGMII interface mode no longer works on AQR107 after the recent changes,
adding back support.
Fixes: 570c8a7d5303 ("net: phy: aquantia: check for supported interface modes
in config_init")
Signed-off-by: Madalin Bucur
---
drivers/net/phy/aquantia_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c
index eed4fe3d871f..0fedd28fdb6e 100644
--- a/drivers/net/phy/aquantia_main.c
+++ b/drivers/net/phy/aquantia_main.c
@@ -487,6 +487,7 @@ static int aqr107_config_init(struct phy_device *phydev)
/* Check that the PHY interface type is compatible */
if (phydev->interface != PHY_INTERFACE_MODE_SGMII &&
phydev->interface != PHY_INTERFACE_MODE_2500BASEX &&
+ phydev->interface != PHY_INTERFACE_MODE_XGMII &&
phydev->interface != PHY_INTERFACE_MODE_10GKR)
return -ENODEV;
--
2.1.0
FW: [PATCH] net: phy: aquantia: readd XGMII support for AQR107
XGMII interface mode no longer works on AQR107 after the recent changes, adding back support. Signed-off-by: Madalin Bucur --- drivers/net/phy/aquantia_main.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c index eed4fe3d871f..0fedd28fdb6e 100644 --- a/drivers/net/phy/aquantia_main.c +++ b/drivers/net/phy/aquantia_main.c @@ -487,6 +487,7 @@ static int aqr107_config_init(struct phy_device *phydev) /* Check that the PHY interface type is compatible */ if (phydev->interface != PHY_INTERFACE_MODE_SGMII && phydev->interface != PHY_INTERFACE_MODE_2500BASEX && + phydev->interface != PHY_INTERFACE_MODE_XGMII && phydev->interface != PHY_INTERFACE_MODE_10GKR) return -ENODEV; -- 2.1.0
RE: Explaining the XDP page-requirement (Was: [PATCH v2 net-next 0/8] dpaa2-eth: Introduce XDP support)
> -Original Message- > From: netdev-ow...@vger.kernel.org On > Behalf Of Jesper Dangaard Brouer > Sent: Friday, December 21, 2018 5:31 PM > To: Ioana Ciocoi Radulescu > Cc: Ilias Apalodimas ; > netdev@vger.kernel.org; da...@davemloft.net; Ioana Ciornei > ; dsah...@gmail.com; Camelia Alexandra Groza > ; bro...@redhat.com > Subject: Explaining the XDP page-requirement (Was: [PATCH v2 net-next 0/8] > dpaa2-eth: Introduce XDP support) > > On Fri, 7 Dec 2018 18:07:49 + > Ioana Ciocoi Radulescu wrote: > > > Thanks a lot for the info, will look into this. Do you have any > > pointers as to why the full page restriction exists in the first > > place? Sorry if it's a dumb question, but I haven't found details on > > this and I'd really like to understand it. > > Hi Ioana, > > I promised (offlist) that I would get back to you explaining the XDP > page-requirement... > > There are several reasons for XDP to require frames are backed by a > page. It started out with a focus on gaining speed via simplicity. > > The overall requirement is: XDP frame in physical contigious memory > - which is a requirement from BPF Direct-Access, for validating > correcness. > - Implying you cannot split packet data over several pages. > > An important part of the page-requirement is to allow creating SKB's > outside the driver code. This happen today in both cpumap and veth > (when doing XDP_REDIRECT). And we need to control and limit the > variations, to avoid having to handle all kind of SKB schemes. > Specifically we need enough tailroom for the skb-shared-info. > > In the beginning we had the requirement of: 1-page per XDP frame. > - Gave us a simplified memory model > - Allow us to not touch atomic refcnt on page (always 1) > - Fixed 256 bytes headroom > - This gave us a lot of tailroom, expanding tail was trivial. > > Eventually ixgbe+i40e force us to use a split-page model, allowing two > frames per page. > - This started to complicate memory model > - This unfortunately gave issue of unknown tailroom, which killed the >tailroom expand option. > - Changes XDP headroom to be variable (192 or 256 bytes) Hi Jesper, is the split page memory model supported now (with two frames per page)? Thanks, Madalin > E.g. I really want to allow bpf_xdp_adjust_tail() to *expand* the > frame size, but after allowing the split-page model, we couldn't allow > this easily. And SKB alloc in cpumap/veth was also complicated by not > knowing (implicit) xdp_frame "hard-end". (We might have to extend > xdp_buff with "data_hard_end"). > > -- > Best regards, > Jesper Dangaard Brouer > MSc.CS, Principal Kernel Engineer at Red Hat > LinkedIn: > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.link > edin.com%2Fin%2Fbrouer&data=02%7C01%7Cmadalin.bucur%40nxp.com%7C44c593 > 0f8a224fdd063208d66759613b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63 > 6810030928918215&sdata=PIdwIEvOAPlyWPScMjOdWiauOp2wAI7QXu9FNJ0SHzs%3D& > amp;reserved=0
RE: [PATCH] fsl/fman: avoid sleeping in atomic context while adding an address
Hi,
this is a duplicate of this other patch addressing the issue:
commit 0d9c9a238faf925823bde866182c663b6d734f2e
Author: Scott Wood
Date: Thu Dec 27 18:29:09 2018 -0600
fsl/fman: Use GFP_ATOMIC in {memac,tgec}_add_hash_mac_address()
Thank you,
Madalin
> -Original Message-
> From: Yi Wang
> Sent: Friday, January 4, 2019 7:50 AM
> To: Madalin-cristian Bucur
> Cc: da...@davemloft.net; netdev@vger.kernel.org; linux-
> ker...@vger.kernel.org; xue.zhih...@zte.com.cn; wang.y...@zte.com.cn;
> huang.jun...@zte.com.cn; Junhua Huang
> Subject: [PATCH] fsl/fman: avoid sleeping in atomic context while adding
> an address
>
> From: Junhua Huang
>
> dev_set_rx_mode will call function pointer mac_dev->add_hash_mac_addr
> while holding spin_lock_bh. The function pointer points to
> memac_add_hash_mac_address when ethernet type is fman-memac,
> which will kmalloc use GFP_KERNEL flag.
>
> / # ifconfig eth2 192.168.1.168
> [ 576.604544] BUG: sleeping function called from invalid context at
> mm/slab.h:393
> [ 576.610587] in_atomic(): 1, irqs_disabled(): 0, pid: 2751, name:
> ifconfig
> [ 576.616105] 2 locks held by ifconfig/2751:
> [ 576.618916] #0:(rtnl_mutex)at: []
> .rtnl_lock+0x1c/0x30
> [ 576.625523] #1:(_xmit_ETHER)at: []
> .dev_set_rx_mode+0x24/0x54
> [ 576.632745] CPU: 5 PID: 2751 Comm: ifconfig Tainted: G W 4.9.115-
> rt93-EMBSYS-@332 #3
> [ 576.642942] Call Trace:
> [ 576.644085] [c0007499b440] [c0a09eb4]
> .dump_stack+0xe0/0x14c (unreliable)
> [ 576.650642] [c0007499b4d0] [c0076f3c]
> .___might_sleep+0x1ac/0x278
> [ 576.656493] [c0007499b560] [c01aad6c]
> .kmem_cache_alloc+0x144/0x28c
> [ 576.662518] [c0007499b620] [c0634c18]
> .memac_add_hash_mac_address+0x100/0x194
> [ 576.669416] [c0007499b6b0] [c0630b54]
> .set_multi+0x1bc/0x20c
> [ 576.674829] [c0007499b760] [c063718c]
> .dpaa_set_rx_mode+0x84/0x104
> [ 576.680765] [c0007499b7e0] [c07f9394]
> .__dev_set_rx_mode+0x64/0xdc
> [ 576.686701] [c0007499b870] [c07f943c]
> .dev_set_rx_mode+0x30/0x54
> [ 576.692464] [c0007499b8f0] [c07f98f4]
> .__dev_change_flags+0x98/0x1c4
> [ 576.698573] [c0007499b980] [c07f9a4c]
> .dev_change_flags+0x2c/0x80
> [ 576.704429] [c0007499ba10] [c08cde28]
> .devinet_ioctl+0x624/0x8e0
> [ 576.710191] [c0007499bb00] [c08d1678]
> .inet_ioctl+0x1f4/0x250
> [ 576.715697] [c0007499bb70] [c07c89d8]
> .sock_do_ioctl+0x50/0xa8
> [ 576.721284] [c0007499bc00] [c07c94e8]
> .sock_ioctl+0x2b8/0x39c
> [ 576.726786] [c0007499bca0] [c01e2500]
> .do_vfs_ioctl+0xc8/0x8b4
> [ 576.732373] [c0007499bd90] [c01e2d44] .SyS_ioctl+0x58/0xa4
> [ 576.737612] [c0007499be30] [c7a4]
> system_call+0x38/0x108
>
> Signed-off-by: Junhua Huang
> ---
> drivers/net/ethernet/freescale/fman/fman_memac.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c
> b/drivers/net/ethernet/freescale/fman/fman_memac.c
> index 71a5ded..21dd557 100644
> --- a/drivers/net/ethernet/freescale/fman/fman_memac.c
> +++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
> @@ -923,7 +923,7 @@ int memac_add_hash_mac_address(struct fman_mac *memac,
> enet_addr_t *eth_addr)
> hash = get_mac_addr_hash_code(addr) & HASH_CTRL_ADDR_MASK;
>
> /* Create element to be added to the driver hash table */
> - hash_entry = kmalloc(sizeof(*hash_entry), GFP_KERNEL);
> + hash_entry = kmalloc(sizeof(*hash_entry), GFP_ATOMIC);
> if (!hash_entry)
> return -ENOMEM;
> hash_entry->addr = addr;
> --
> 2.15.2
RE: [PATCH] fsl/fman: Use GFP_ATOMIC in {memac,tgec}_add_hash_mac_address()
> -Original Message-
> From: Scott Wood
> Sent: Friday, December 28, 2018 2:29 AM
> To: Madalin-cristian Bucur
> Cc: netdev@vger.kernel.org; linuxppc-...@lists.ozlabs.org
> Subject: [PATCH] fsl/fman: Use GFP_ATOMIC in
> {memac,tgec}_add_hash_mac_address()
>
> These functions are called from atomic context:
>
> [9.150239] BUG: sleeping function called from invalid context at
> /home/scott/git/linux/mm/slab.h:421
> [9.158159] in_atomic(): 1, irqs_disabled(): 0, pid: 4432, name: ip
> [9.163128] CPU: 8 PID: 4432 Comm: ip Not tainted 4.20.0-rc2-00169-
> g63d86876f324 #29
> [9.163130] Call Trace:
> [9.170701] [c002e899a980] [c09c1068] .dump_stack+0xa8/0xec
> (unreliable)
> [9.177140] [c002e899aa10] [c007a7b4]
> .___might_sleep+0x138/0x164
> [9.184440] [c002e899aa80] [c01d5bac]
> .kmem_cache_alloc_trace+0x238/0x30c
> [9.191216] [c002e899ab40] [c065ea1c]
> .memac_add_hash_mac_address+0x104/0x198
> [9.199464] [c002e899abd0] [c065a788]
> .set_multi+0x1c8/0x218
> [9.206242] [c002e899ac80] [c06615ec]
> .dpaa_set_rx_mode+0xdc/0x17c
> [9.213544] [c002e899ad00] [c083d2b0]
> .__dev_set_rx_mode+0x80/0xd4
> [9.219535] [c002e899ad90] [c083d334]
> .dev_set_rx_mode+0x30/0x54
> [9.225271] [c002e899ae10] [c083d4a0]
> .__dev_open+0x148/0x1c8
> [9.230751] [c002e899aeb0] [c083d934]
> .__dev_change_flags+0x19c/0x1e0
> [9.230755] [c002e899af60] [c083d9a4]
> .dev_change_flags+0x2c/0x80
> [9.242752] [c002e899aff0] [c08554ec]
> .do_setlink+0x350/0xf08
> [9.248228] [c002e899b170] [c0857ad0]
> .rtnl_newlink+0x588/0x7e0
> [9.253965] [c002e899b740] [c0852424]
> .rtnetlink_rcv_msg+0x3e0/0x498
> [9.261440] [c002e899b820] [c0884790]
> .netlink_rcv_skb+0x134/0x14c
> [9.267607] [c002e899b8e0] [c0851840]
> .rtnetlink_rcv+0x18/0x2c
> [9.274558] [c002e899b950] [c0883c8c]
> .netlink_unicast+0x214/0x318
> [9.281163] [c002e899ba00] [c0884220]
> .netlink_sendmsg+0x348/0x444
> [9.287076] [c002e899bae0] [c080d13c]
> .sock_sendmsg+0x2c/0x54
> [9.287080] [c002e899bb50] [c08106c0]
> .___sys_sendmsg+0x2d0/0x2d8
> [9.298375] [c002e899bd30] [c0811a80]
> .__sys_sendmsg+0x5c/0xb0
> [9.303939] [c002e899be20] [c6b0] system_call+0x60/0x6c
>
> Signed-off-by: Scott Wood
> ---
> drivers/net/ethernet/freescale/fman/fman_memac.c | 2 +-
> drivers/net/ethernet/freescale/fman/fman_tgec.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c
> b/drivers/net/ethernet/freescale/fman/fman_memac.c
> index bc6eb30aa20f..41c6fa200e74 100644
> --- a/drivers/net/ethernet/freescale/fman/fman_memac.c
> +++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
> @@ -928,7 +928,7 @@ int memac_add_hash_mac_address(struct fman_mac *memac,
> enet_addr_t *eth_addr)
> hash = get_mac_addr_hash_code(addr) & HASH_CTRL_ADDR_MASK;
>
> /* Create element to be added to the driver hash table */
> - hash_entry = kmalloc(sizeof(*hash_entry), GFP_KERNEL);
> + hash_entry = kmalloc(sizeof(*hash_entry), GFP_ATOMIC);
> if (!hash_entry)
> return -ENOMEM;
> hash_entry->addr = addr;
> diff --git a/drivers/net/ethernet/freescale/fman/fman_tgec.c
> b/drivers/net/ethernet/freescale/fman/fman_tgec.c
> index 40705938eecc..f75b9c11b2d2 100644
> --- a/drivers/net/ethernet/freescale/fman/fman_tgec.c
> +++ b/drivers/net/ethernet/freescale/fman/fman_tgec.c
> @@ -553,7 +553,7 @@ int tgec_add_hash_mac_address(struct fman_mac *tgec,
> enet_addr_t *eth_addr)
> hash = (crc >> TGEC_HASH_MCAST_SHIFT) & TGEC_HASH_ADR_MSK;
>
> /* Create element to be added to the driver hash table */
> - hash_entry = kmalloc(sizeof(*hash_entry), GFP_KERNEL);
> + hash_entry = kmalloc(sizeof(*hash_entry), GFP_ATOMIC);
> if (!hash_entry)
> return -ENOMEM;
> hash_entry->addr = addr;
> --
> 2.17.1
Thank you
RE: [PATCH] dpaa_eth: Add dpaa_change_carrier()
> -Original Message-
> From: Joakim Tjernlund
> Sent: Thursday, December 6, 2018 5:32 PM
> To: netdev @ vger . kernel . org ; Madalin-
> cristian Bucur
> Cc: jo...@infinera.com
> Subject: [PATCH] dpaa_eth: Add dpaa_change_carrier()
>
> This allows to control carrier from /sys/class/net/ethX/carrier
Hi,
can you please explain why it's needed?
Thanks,
Madalin
> Signed-off-by: Joakim Tjernlund
> ---
> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> index a202c50d6fc7..0e93dfa6df0a 100644
> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> @@ -2502,12 +2502,23 @@ static int dpaa_ioctl(struct net_device *net_dev,
> struct ifreq *rq, int cmd)
> return phy_mii_ioctl(net_dev->phydev, rq, cmd);
> }
>
> +static int dpaa_change_carrier(struct net_device *dev, bool new_carrier)
> +{
> + struct phy_device *phydev = dev->phydev;
> +
> + if (phydev && phydev->phy_link_change)
> + phydev->phy_link_change(phydev, new_carrier, 1);
> +
> + return 0;
> +}
> +
> static const struct net_device_ops dpaa_ops = {
> .ndo_open = dpaa_open,
> .ndo_start_xmit = dpaa_start_xmit,
> .ndo_stop = dpaa_eth_stop,
> .ndo_tx_timeout = dpaa_tx_timeout,
> .ndo_get_stats64 = dpaa_get_stats64,
> + .ndo_change_carrier = dpaa_change_carrier,
> .ndo_set_mac_address = dpaa_set_mac_address,
> .ndo_validate_addr = eth_validate_addr,
> .ndo_set_rx_mode = dpaa_set_rx_mode,
> --
> 2.18.1
RE: [PATCH v2 2/2] dpaa_eth: add ethtool coalesce control
> -Original Message-
> From: David Miller
> Sent: Saturday, November 17, 2018 5:42 AM
> To: Madalin-cristian Bucur
> Subject: Re: [PATCH v2 2/2] dpaa_eth: add ethtool coalesce control
>
> From: Madalin Bucur
> Date: Tue, 13 Nov 2018 18:29:51 +0200
>
> > + for_each_cpu(cpu, cpus) {
> > + portal = qman_get_affine_portal(cpu);
> > + res = qman_portal_set_iperiod(portal, period);
> > + if (res)
> > + return res;
> > + res = qman_dqrr_set_ithresh(portal, thresh);
> > + if (res)
> > + return res;
>
> Nope, you can't do it like this.
>
> If any intermediate change fails, you have to unwind all of the
> changes made up until that point.
>
> Which means you'll have to store the previous setting somewhere
> and reinstall those saved values in the error path.
Thank you, I'll come back with a v3.
Madalin
RE: [PATCH v2 00/22] SMMU enablement for NXP LS1043A and LS1046A
> -Original Message- > From: laurentiu.tu...@nxp.com [mailto:laurentiu.tu...@nxp.com] > Sent: Wednesday, September 26, 2018 4:22 PM > To: devicet...@vger.kernel.org; netdev@vger.kernel.org; linux- > ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org > Cc: Roy Pledge ; Madalin-cristian Bucur > ; da...@davemloft.net; shawn...@kernel.org; Leo Li > ; robin.mur...@arm.com; Bharat Bhushan > ; Laurentiu Tudor > Subject: [PATCH v2 00/22] SMMU enablement for NXP LS1043A and LS1046A > > From: Laurentiu Tudor > > This patch series adds SMMU support for NXP LS1043A and LS1046A chips > and consists mostly in important driver fixes and the required device > tree updates. It touches several subsystems and consists of three main > parts: > - changes in soc/drivers/fsl/qbman drivers adding iommu mapping of >reserved memory areas, fixes and defered probe support > - changes in drivers/net/ethernet/freescale/dpaa_eth drivers >consisting in misc dma mapping related fixes and probe ordering > - addition of the actual arm smmu device tree node together with >various adjustments to the device trees > > Performance impact > > Running iperf benchmarks in a back-to-back setup (both sides > having smmu enabled) on a 10GBps port show an important > networking performance degradation of around 40% (9.48Gbps > linerate vs 5.45Gbps) and around 30%-35% with iommu.strict=1. > If you need performance but without SMMU support you can use > "iommu.passthrough=1" to disable SMMU. > > The patch set is based on net-next so, if generally agreed, I'd suggest > to get the patches through the netdev tree after getting all the Acks. > > Changes in v2: > - dropped confusing comments in smmu interrupt property (Robin) > - add an intermediate simple-bus for usb to fix dma size issue (Robin) > - use defines instead of numbers in smmu interrupt definition > - drop redundant double iommu_get_domain_for_dev() call in few qbman >patches > > Laurentiu Tudor (22): > soc/fsl/qman: fixup liodns only on ppc targets > soc/fsl/bman: map FBPR area in the iommu > soc/fsl/qman: map FQD and PFDR areas in the iommu > soc/fsl/qman-portal: map CENA area in the iommu > soc/fsl/qbman: add APIs to retrieve the probing status > soc/fsl/qman_portals: defer probe after qman's probe > soc/fsl/bman_portals: defer probe after bman's probe > soc/fsl/qbman_portals: add APIs to retrieve the probing status > fsl/fman: backup and restore ICID registers > fsl/fman: add API to get the device behind a fman port > dpaa_eth: defer probing after qbman > dpaa_eth: base dma mappings on the fman rx port > dpaa_eth: fix iova handling for contiguous frames > dpaa_eth: fix iova handling for sg frames > dpaa_eth: fix SG frame cleanup > arm64: dts: ls1046a: add smmu node > arm64: dts: ls1043a: add smmu node > arm64: dts: ls104xa: set mask to drop TBU ID from StreamID > arm64: dts: ls104x: add missing dma ranges property > arm64: dts: ls104x: add iommu-map to pci controllers > arm64: dts: ls104x: make dma-coherent global to the SoC > arm64: dts: ls104x: use a pseudo-bus to constrain usb dma size > > .../arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 106 ++ > .../arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 102 ++--- > .../net/ethernet/freescale/dpaa/dpaa_eth.c| 136 -- > drivers/net/ethernet/freescale/fman/fman.c| 35 - > drivers/net/ethernet/freescale/fman/fman.h| 4 + > .../net/ethernet/freescale/fman/fman_port.c | 14 ++ > .../net/ethernet/freescale/fman/fman_port.h | 2 + > drivers/soc/fsl/qbman/bman_ccsr.c | 22 +++ > drivers/soc/fsl/qbman/bman_portal.c | 20 ++- > drivers/soc/fsl/qbman/qman_ccsr.c | 28 > drivers/soc/fsl/qbman/qman_portal.c | 35 + > include/soc/fsl/bman.h| 16 +++ > include/soc/fsl/qman.h| 17 +++ > 13 files changed, 438 insertions(+), 99 deletions(-) > > -- > 2.17.1 For the fsl/fman and dpaa_eth: Acked-by: Madalin Bucur Thank you
RE: [PATCH] fsl/fman: remove unnecessary set_dma_ops() call and HAS_DMA dependency
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Sent: Thursday, March 22, 2018 8:32 PM > To: Madalin-cristian Bucur > Cc: geert.uytterhoe...@gmail.com; netdev@vger.kernel.org; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH] fsl/fman: remove unnecessary set_dma_ops() call and > HAS_DMA dependency > > From: Madalin Bucur > Date: Wed, 21 Mar 2018 03:58:19 -0500 > > > The platform device is no longer used for DMA mapping so the > > (questionable) setting of the DMA ops done here is no longer > > needed. Removing it together with the HAS_DMA dependency that > > it required. > > > > Signed-off-by: Madalin Bucur > > This doesn't apply to any of my trees. Sorry, it's caused by a patch in my tree that adds ARM 32b support, resulting in differences in the context: - depends on FSL_SOC || ARCH_LAYERSCAPE || COMPILE_TEST + depends on ARM || ARCH_LAYERSCAPE || FSL_SOC || COMPILE_TEST I'll send a v2 based on a clean tree. Madalin
RE: [PATCH v2 10/21] lightnvm: Remove depends on HAS_DMA in case of platform dependency
> -Original Message- > From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] > On Behalf Of Geert Uytterhoeven > Sent: Friday, March 16, 2018 3:52 PM > To: Christoph Hellwig ; Marek Szyprowski > ; Robin Murphy ; > Felipe Balbi ; Greg Kroah-Hartman > ; James E . J . Bottomley > ; Martin K . Petersen > ; Andrew Morton foundation.org>; Mark Brown ; Liam Girdwood > ; Tejun Heo ; Herbert Xu > ; David S . Miller ; > Bartlomiej Zolnierkiewicz ; Stefan Richter > ; Alan Tull ; Moritz Fischer > ; Wolfram Sang ; Jonathan Cameron > ; Joerg Roedel ; Matias Bjorling > ; Jassi Brar ; Mauro Carvalho > Chehab ; Ulf Hansson ; David > Woodhouse ; Brian Norris > ; Marek Vasut ; > Cyrille Pitchen ; Boris Brezillon > ; Richard Weinberger ; > Kalle Valo ; Ohad Ben-Cohen ; > Bjorn Andersson ; Eric Anholt ; > Stefan Wahren > Cc: io...@lists.linux-foundation.org; linux-...@vger.kernel.org; linux- > s...@vger.kernel.org; alsa-de...@alsa-project.org; linux-...@vger.kernel.org; > linux-cry...@vger.kernel.org; linux-fb...@vger.kernel.org; linux1394- > de...@lists.sourceforge.net; linux-f...@vger.kernel.org; linux- > i...@vger.kernel.org; linux-...@vger.kernel.org; linux-bl...@vger.kernel.org; > linux-me...@vger.kernel.org; linux-...@vger.kernel.org; linux- > m...@lists.infradead.org; netdev@vger.kernel.org; linux- > remotep...@vger.kernel.org; linux-ser...@vger.kernel.org; linux- > s...@vger.kernel.org; de...@driverdev.osuosl.org; linux- > ker...@vger.kernel.org; Geert Uytterhoeven > Subject: [PATCH v2 10/21] lightnvm: Remove depends on HAS_DMA in case of > platform dependency > > Remove dependencies on HAS_DMA where a Kconfig symbol depends on > another > symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST". > In most cases this other symbol is an architecture or platform specific > symbol, or PCI. > > Generic symbols and drivers without platform dependencies keep their > dependencies on HAS_DMA, to prevent compiling subsystems or drivers that > cannot work anyway. > > This simplifies the dependencies, and allows to improve compile-testing. > > Notes: > - FSL_FMAN keeps its dependency on HAS_DMA, as it calls set_dma_ops(), > which does not exist if HAS_DMA=n (Do we need a dummy? The use of > set_dma_ops() in this driver is questionable), Hi, The set_dma_ops() is no longer required in the fsl/fman, I'll send a patch to remove it. Thanks > - SND_SOC_LPASS_IPQ806X and SND_SOC_LPASS_PLATFORM loose their > dependency on HAS_DMA, as they are selected from > SND_SOC_APQ8016_SBC. > > Signed-off-by: Geert Uytterhoeven > Reviewed-by: Mark Brown > Acked-by: Robin Murphy > --- > v2: > - Add Reviewed-by, Acked-by, > - Drop RFC state, > - Split per subsystem. > --- > drivers/lightnvm/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/lightnvm/Kconfig b/drivers/lightnvm/Kconfig > index 10c08982185a572f..9c03f35d9df113c6 100644 > --- a/drivers/lightnvm/Kconfig > +++ b/drivers/lightnvm/Kconfig > @@ -4,7 +4,7 @@ > > menuconfig NVM > bool "Open-Channel SSD target support" > - depends on BLOCK && HAS_DMA && PCI > + depends on BLOCK && PCI > select BLK_DEV_NVME > help > Say Y here to get to enable Open-channel SSDs. > -- > 2.7.4
RE: [PATCH 0/5] DPAA Ethernet fixes
> -Original Message- > From: Joakim Tjernlund [mailto:joakim.tjernl...@infinera.com] > Sent: Wednesday, March 14, 2018 8:43 PM > To: da...@davemloft.net; Madalin-cristian Bucur > > > On Wed, 2018-03-14 at 08:37 -0500, Madalin Bucur wrote: > > Hi, > > > > This patch set is addressing several issues in the DPAA Ethernet > > driver suite: > > > > - module unload crash caused by wrong reference to device being left > >in the cleanup code after the DSA related changes > > - scheduling wile atomic bug in QMan code revealed during dpaa_eth > >module unload > > - a couple of error counter fixes, a duplicated init in dpaa_eth. > > hmm, some of these(all?) bugs are in stable too, CC: stable perhaps? Hi Jocke, I did not check that, they should be added. Thanks, Madalin > > > > Madalin > > > > Camelia Groza (3): > > dpaa_eth: remove duplicate initialization > > dpaa_eth: increment the RX dropped counter when needed > > dpaa_eth: remove duplicate increment of the tx_errors counter > > > > Madalin Bucur (2): > > soc/fsl/qbman: fix issue in qman_delete_cgr_safe() > > dpaa_eth: fix error in dpaa_remove() > > > > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 8 > > drivers/soc/fsl/qbman/qman.c | 28 > > +- > > 2 files changed, 9 insertions(+), 27 deletions(-) > > > > -- > > 2.1.0 > >
RE: [PATCH] fsl/fman: avoid sleeping in atomic context while adding an address
> -Original Message-
> From: Denis Kirjanov [mailto:k...@linux-powerpc.org]
> Sent: Sunday, March 4, 2018 8:48 PM
> To: Madalin-cristian Bucur
> Cc: netdev@vger.kernel.org; Denis Kirjanov
> Subject: [PATCH] fsl/fman: avoid sleeping in atomic context while adding an
> address
>
> __dev_mc_add grabs an adress spinlock so use
> atomic context in kmalloc.
>
> / # ifconfig eth0 inet 192.168.0.111
> [ 89.331622] BUG: sleeping function called from invalid context at
> mm/slab.h:420
> [ 89.339002] in_atomic(): 1, irqs_disabled(): 0, pid: 1035, name: ifconfig
> [ 89.345799] 2 locks held by ifconfig/1035:
> [ 89.349908] #0: (rtnl_mutex){+.+.}, at: [<(ptrval)>]
> devinet_ioctl+0xc0/0x8a0
> [ 89.357258] #1: (_xmit_ETHER){+...}, at: [<(ptrval)>]
> __dev_mc_add+0x28/0x80
> [ 89.364520] CPU: 1 PID: 1035 Comm: ifconfig Not tainted 4.16.0-rc3-dirty
> #8
> [ 89.371464] Call Trace:
> [ 89.373908] [e959db60] [c066f948] dump_stack+0xa4/0xfc (unreliable)
> [ 89.380177] [e959db80] [c00671d8] ___might_sleep+0x248/0x280
> [ 89.385833] [e959dba0] [c01aec34]
> kmem_cache_alloc_trace+0x174/0x320
> [ 89.392179] [e959dbd0] [c04ab920]
> dtsec_add_hash_mac_address+0x130/0x240
> [ 89.398874] [e959dc00] [c04a9d74] set_multi+0x174/0x1b0
> [ 89.404093] [e959dc30] [c04afb68] dpaa_set_rx_mode+0x68/0xe0
> [ 89.409745] [e959dc40] [c057baf8] __dev_mc_add+0x58/0x80
> [ 89.415052] [e959dc60] [c060fd64] igmp_group_added+0x164/0x190
> [ 89.420878] [e959dca0] [c060ffa8] ip_mc_inc_group+0x218/0x460
> [ 89.426617] [e959dce0] [c06120fc] ip_mc_up+0x3c/0x190
> [ 89.431662] [e959dd10] [c0607270] inetdev_event+0x250/0x620
> [ 89.437227] [e959dd50] [c005f190] notifier_call_chain+0x80/0xf0
> [ 89.443138] [e959dd80] [c0573a74] __dev_notify_flags+0x54/0xf0
> [ 89.448964] [e959dda0] [c05743f8] dev_change_flags+0x48/0x60
> [ 89.454615] [e959ddc0] [c0606744] devinet_ioctl+0x544/0x8a0
> [ 89.460180] [e959de10] [c060987c] inet_ioctl+0x9c/0x1f0
> [ 89.465400] [e959de80] [c05479a8] sock_ioctl+0x168/0x460
> [ 89.470708] [e959ded0] [c01cf3ec] do_vfs_ioctl+0xac/0x8c0
> [ 89.476099] [e959df20] [c01cfc40] SyS_ioctl+0x40/0xc0
> [ 89.481147] [e959df40] [c0011318] ret_from_syscall+0x0/0x3c
> [ 89.486715] --- interrupt: c01 at 0x1006943c
> [ 89.486715] LR = 0x100c45ec
>
> Signed-off-by: Denis Kirjanov
> ---
> drivers/net/ethernet/freescale/fman/fman_dtsec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/freescale/fman/fman_dtsec.c
> b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
> index ea43b4974149..7af31ddd093f 100644
> --- a/drivers/net/ethernet/freescale/fman/fman_dtsec.c
> +++ b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
> @@ -1100,7 +1100,7 @@ int dtsec_add_hash_mac_address(struct
> fman_mac *dtsec, enet_addr_t *eth_addr)
> set_bucket(dtsec->regs, bucket, true);
>
> /* Create element to be added to the driver hash table */
> - hash_entry = kmalloc(sizeof(*hash_entry), GFP_KERNEL);
> + hash_entry = kmalloc(sizeof(*hash_entry), GFP_ATOMIC);
> if (!hash_entry)
> return -ENOMEM;
> hash_entry->addr = addr;
> --
> 2.13.6
Acked-by: Madalin Bucur
RE: [PATCH net-next] dpaa_eth: fix pause capability advertisement logic
> -Original Message- > From: Jake Moroni [mailto:m...@jakemoroni.com] > Sent: Sunday, February 18, 2018 10:26 PM > To: Madalin-cristian Bucur > Cc: netdev@vger.kernel.org; Jake Moroni > Subject: [PATCH net-next] dpaa_eth: fix pause capability advertisement > logic > > The ADVERTISED_Asym_Pause bit was being improperly set when both > rx and tx pause were enabled. When rx and tx are both enabled, only > the ADVERTISED_Pause bit is supposed to be set. > > Signed-off-by: Jake Moroni > --- > drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c > b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c > index faea674..85306d1 100644 > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c > @@ -211,7 +211,7 @@ static int dpaa_set_pauseparam(struct net_device > *net_dev, > if (epause->rx_pause) > newadv = ADVERTISED_Pause | ADVERTISED_Asym_Pause; > if (epause->tx_pause) > - newadv |= ADVERTISED_Asym_Pause; > + newadv ^= ADVERTISED_Asym_Pause; > > oldadv = phydev->advertising & > (ADVERTISED_Pause | ADVERTISED_Asym_Pause); > -- > 2.7.4 Acked-by: Madalin Bucur
RE: DPAA Ethernet traffice troubles with Linux kernel
> -Original Message- > From: Madalin-cristian Bucur > Sent: Wednesday, January 17, 2018 4:25 PM > To: David S . Miller > Cc: linuxppc-...@lists.ozlabs.org; netdev@vger.kernel.org; > madskate...@gmail.com; 'Madalin-cristian Bucur' ; > Andrew Lunn ; Joakim Tjernlund > > Subject: RE: DPAA Ethernet traffice troubles with Linux kernel > > > -Original Message- > > From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] > > On Behalf Of Madalin-cristian Bucur > > Sent: Wednesday, January 17, 2018 4:16 PM > > To: Andrew Lunn ; Joakim Tjernlund > > > > Cc: linuxppc-...@lists.ozlabs.org; netdev@vger.kernel.org; > > madskate...@gmail.com; David S . Miller > > Subject: RE: DPAA Ethernet traffice troubles with Linux kernel > > > > > -Original Message- > > > From: Andrew Lunn [mailto:and...@lunn.ch] > > > Sent: Wednesday, January 17, 2018 3:44 PM > > > To: Joakim Tjernlund > > > Subject: Re: DPAA Ethernet traffice troubles with Linux kernel > > > > > > > That doesn't work really, having users to hit the bug, debug it, fix > > it > > > and then > > > > find it fixed already in upstream, then specifically request it to > be > > > backported to stable. > > > > I don't need this fix to be backported, already got it. Someone else > > > might though. > > > > > > The "someone else might though" is a big point of asking for it to > > > added to stable. The other reason is it means one less patch you need > > > to maintain in your build. > > > > I've sent that patch [1] for net but I guess the timing was wrong and > > it was merged to net-next. > > > > > > I would be interested in bug fixes upstream which fixes: > > > > > > Did you try upstream? Does it give the same errors? > > > > > > Andrew > > > > [1] https://patchwork.kernel.org/patch/10146119/ > > > > Madalin > > Hi Dave, > > Can you please add the fix [1] to stable? > > Thank you, > Madalin Sorry, I've provided the wrong link towards the patch (v1 instead of v3), here's the correct one: https://patchwork.kernel.org/patch/10151969/ Madalin
RE: DPAA Ethernet traffice troubles with Linux kernel
> -Original Message- > From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] > On Behalf Of Madalin-cristian Bucur > Sent: Wednesday, January 17, 2018 4:16 PM > To: Andrew Lunn ; Joakim Tjernlund > > Cc: linuxppc-...@lists.ozlabs.org; netdev@vger.kernel.org; > madskate...@gmail.com; David S . Miller > Subject: RE: DPAA Ethernet traffice troubles with Linux kernel > > > -Original Message- > > From: Andrew Lunn [mailto:and...@lunn.ch] > > Sent: Wednesday, January 17, 2018 3:44 PM > > To: Joakim Tjernlund > > Subject: Re: DPAA Ethernet traffice troubles with Linux kernel > > > > > That doesn't work really, having users to hit the bug, debug it, fix > it > > and then > > > find it fixed already in upstream, then specifically request it to be > > backported to stable. > > > I don't need this fix to be backported, already got it. Someone else > > might though. > > > > The "someone else might though" is a big point of asking for it to > > added to stable. The other reason is it means one less patch you need > > to maintain in your build. > > I've sent that patch [1] for net but I guess the timing was wrong and > it was merged to net-next. > > > > I would be interested in bug fixes upstream which fixes: > > > > Did you try upstream? Does it give the same errors? > > > > Andrew > > [1] https://patchwork.kernel.org/patch/10146119/ > > Madalin Hi Dave, Can you please add the fix [1] to stable? Thank you, Madalin
RE: DPAA Ethernet traffice troubles with Linux kernel
> -Original Message- > From: Andrew Lunn [mailto:and...@lunn.ch] > Sent: Wednesday, January 17, 2018 3:44 PM > To: Joakim Tjernlund > Subject: Re: DPAA Ethernet traffice troubles with Linux kernel > > > That doesn't work really, having users to hit the bug, debug it, fix it > and then > > find it fixed already in upstream, then specifically request it to be > backported to stable. > > I don't need this fix to be backported, already got it. Someone else > might though. > > The "someone else might though" is a big point of asking for it to > added to stable. The other reason is it means one less patch you need > to maintain in your build. I've sent that patch [1] for net but I guess the timing was wrong and it was merged to net-next. > > I would be interested in bug fixes upstream which fixes: > > Did you try upstream? Does it give the same errors? > > Andrew [1] https://patchwork.kernel.org/patch/10146119/ Madalin
RE: DPAA Ethernet traffice troubles with Linux kernel
> -Original Message- > From: Joakim Tjernlund [mailto:joakim.tjernl...@infinera.com] > Sent: Tuesday, January 16, 2018 7:58 PM > To: and...@lunn.ch > Subject: Re: DPAA Ethernet traffice troubles with Linux kernel > > On Thu, 1970-01-01 at 00:00 +, Andrew Lunn wrote: > > > > Hi Joakim > > > > You appear to be using an old kernel. Take a look at: > > Not really, I am using 4.14.x and I don't think that is old. Seems like > this > patch hasn't been sent to 4.14.x. > > I wonder if I might be missing something else, we just moved to 4.14 and > notic that all > our fixed PHYs are non functioning: > fsl_mac ffe4e2000.ethernet: FMan MEMAC > fsl_mac ffe4e2000.ethernet: FMan MAC address: 00:06:9c:0b:06:20 > fsl_mac dpaa-ethernet.0: __devm_request_mem_region(mac) failed > fsl_mac: probe of dpaa-ethernet.0 failed with error -16 > fsl_mac ffe4e4000.ethernet: FMan MEMAC > fsl_mac ffe4e4000.ethernet: FMan MAC address: 00:06:9c:0b:06:21 > fsl_mac dpaa-ethernet.1: __devm_request_mem_region(mac) failed > fsl_mac: probe of dpaa-ethernet.1 failed with error -16 > fsl_mac ffe4e6000.ethernet: FMan MEMAC > fsl_mac ffe4e6000.ethernet: FMan MAC address: 00:06:9c:0b:06:22 > fsl_mac dpaa-ethernet.2: __devm_request_mem_region(mac) failed > fsl_mac: probe of dpaa-ethernet.2 failed with error -16 > fsl_mac ffe4e8000.ethernet: FMan MEMAC > fsl_mac ffe4e8000.ethernet: FMan MAC address: 00:06:9c:0b:06:23 > fsl_mac dpaa-ethernet.3: __devm_request_mem_region(mac) failed > fsl_mac: probe of dpaa-ethernet.3 failed with error -16 > > Feels like FMAN still think there are real PHYs there ? Hi Joakim, These errors are issued when trying to probe the second time the same MAC node. The issue was introduced by this commit: commit 4d8ee1935bcd666360311dfdadeee235d682d69a Author: Florian Fainelli Date: Tue Aug 22 15:24:47 2017 -0700 fsl/man: Inherit parent device and of_node and was later addressed by this patch set: http://patchwork.ozlabs.org/project/netdev/list/?series=8462&state=* Even with these errors printed, all is working fine, it's just the second probing that fails. Adding the latter patches or reverting the one above makes the errors prints dissapear. Madalin
RE: DPAA Ethernet problems with mainstream Linux kernels
> -Original Message- > From: Darren Stevens [mailto:dar...@stevens-zone.net] > Sent: Tuesday, January 16, 2018 12:40 AM > To: Madalin-cristian Bucur > Cc: Jamie Krueger ; linuxppc- > d...@lists.ozlabs.org; netdev@vger.kernel.org > Subject: Re: DPAA Ethernet problems with mainstream Linux kernels > > Hello Madalin-cristian > > On 15/01/2018, Madalin-cristian Bucur wrote: > >> > The device tree that you mention, cyrus_p5020.eth.dts is not found in > >> > the Linux kernel sources. The cyrus_p5020.dts file from the fsl ppc > >> > device tree folder does not include the PHY information for the DPAA > >> > interfaces. The problems that you experience may be caused by some > >> > issues with the PHY configuration (i.e. internal delay). > >> The cyrus_p5020.eth.dts is a modified version of the cyrus_p5020.dts, > >> which of course was based off the original p5020ds.dts file. As you > >> noted, the current cyrus_p5020.dts file is incomplete, and does not > >> map the Ethernet connections properly. > > This is because the current linux kernel version of cyrus_p5020.dts > includes 'p5020si-pre.dtsi' and 'p5020si-post.dtsi' include files, which > orginally gave us working ethernet (when we used the SDK kernel) However > at some point you moved the ethernet aliases from the board dts file to > the p5020si-pre.dtsi file breaking the linkages for our board. > > cyrus-pre.dtsi is simply p5020si-pre.dtsi with only the correct aliases > in. > > >> ** I have attached both the cyrus_p5020.eth.dts and cyrus-pre.dtsi > >> files with this email for comparison. Please let me know if you > see > >> any corrections that should be made to either file. > > > > At a first glance they look fine to me. > > That's good to know. > > >> I have started testing along that line, using Wireshark to view the > >> traffic on the X5000/20 itself, and from another machine connected > >> on the same subnet. So far (as indicated by some details of in my > >> initial email), I can see outgoing broadcast requests (for DHCP) > >> being sent out from the X5000/20, and these requests are correctly > >> constructed and visible outside the X5000/20. > >> > >> However, no responses to the DHCP broadcasts appear to reach > >> to X5000/20's DPAA Ethernet. I will need to setup some further > >> tests to determine if the DHCP server saw the requests and responded > >> to them. (I assume the DHCP server is getting them, and responding, > >> as I can always get a successful DHCP response to the X5000/20 > >> when using an add-on Ethernet PICe card on the same subnet). > > This matches what I see, the interface I have connected to the network > shows an increasing number of transmitted packets, but no received ones. > > Jamie also noticed the following error in dmesg (right after the ethernet > port becomes active) > > [4.112165] fsl_dpa dpaa-ethernet.0 eth0: Probed interface eth0 > [4.116616] fsl_dpa dpaa-ethernet.1 eth1: Probed interface eth1 > [ ... ] > [ 106.501055] IPv6: ADDRCONF(NETDEV_UP): eth1: link is not ready > [ 106.570944] IPv6: ADDRCONF(NETDEV_UP): eth1: link is not ready > [ 106.605044] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready > [ 106.674918] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready > [ 108.702771] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready > [ 109.032798] fsl-pamu: pamu_av_isr: access violation interrupt > [ 109.032806] fsl-pamu: pamu_av_isr: POES1= > [ 109.032808] fsl-pamu: pamu_av_isr: POES2= > [ 109.032809] fsl-pamu: pamu_av_isr: AVS1=002d0081 > [ 109.032811] fsl-pamu: pamu_av_isr: AVS2=0081 > [ 109.032813] fsl-pamu: pamu_av_isr: AVA=0001f1328000 > [ 109.032815] fsl-pamu: pamu_av_isr: UDAD=002d0001 > [ 109.032817] fsl-pamu: pamu_av_isr: POEA= > > I haven't seen this anywhere else, and wondered if it is relevant. > > Regards > Darren The PAMU related errors may be relevant to the issue, if you have incorrect settings you may have no traffic passing through. The PAMU configuration should be made by the bootloader. Can you try to disable CONFIG_FSL_PAMU? Madalin
RE: DPAA Ethernet traffice troubles with Linux kernel
> -Original Message- > From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] > On Behalf Of Andrew Lunn > Sent: Tuesday, January 16, 2018 5:04 PM > To: mad skateman > Cc: Christian Zigotzky ; Joakim Tjernlund > ; linuxppc-...@lists.ozlabs.org; Madalin- > cristian Bucur ; netdev@vger.kernel.org > Subject: Re: DPAA Ethernet traffice troubles with Linux kernel > > > When i use mii-tool too Kick the tranceiver... it comes alive.. i can > > ping the eth0 itself > > > > root@X5000LNX:/home/skateman# mii-tool -R eth0 > > resetting the transceiver... > > root@X5000LNX:/home/skateman# ping 192.168.22.44 > > PING 192.168.22.44 (192.168.22.44) 56(84) bytes of data. > > 64 bytes from 192.168.22.44: icmp_seq=1 ttl=64 time=0.045 ms > > 64 bytes from 192.168.22.44: icmp_seq=2 ttl=64 time=0.046 ms > > 64 bytes from 192.168.22.44: icmp_seq=3 ttl=64 time=0.047 ms > > 64 bytes from 192.168.22.44: icmp_seq=4 ttl=64 time=0.048 ms > > What PHY driver are you using? > > This smells a bit like an RGMII-ID problem. > > Andrew Hi Andrew, >From another thread[1] on the same topic: > I am not sure what PHY hardware/configuration you are using on the > DS and RDB platforms, but I can confirm that AmigaONE X5000/20 > (Cyrus Motherboard with p5020 SoC), has dTSEC 4 and dTSEC 5 > wired to two Micrel KSZ9021RN Gigabit Ethernet PHYs, using the > RGMII protocol. [1] https://www.spinics.net/lists/netdev/msg478062.html
RE: DPAA Ethernet traffice troubles with Linux kernel
> -Original Message- > From: Linuxppc-dev [mailto:linuxppc-dev- > bounces+madalin.bucur=nxp@lists.ozlabs.org] On Behalf Of mad skateman > Sent: Wednesday, January 10, 2018 10:39 PM > To: linuxppc-...@lists.ozlabs.org > Subject: DPAA Ethernet traffice troubles with Linux kernel > > Hi linux devs, > > Like mentioned in this thread > https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-January/167630.html > i also experience the exact same issues. > I am also trying to find out why the network traffic is not flowing > the way it should (out for example ). > > My linux knowledge is very basic but i hope i can contribute anyway. > > I am using the AmigaOne X5000 with a P5020 > Most detailed technical information regarding this issue can be found > in the Thread by Jamie Krueger mentioned above. > > In this screenshot, the ETH0 and ETH1 seem up and running (probed) .. > even due to the FSL_DPAA_MAC error messages that DMESG shows. > http://www.skateman.nl/wp-content/uploads/2018/01/Screenshot-at-2018-01-08-21_22_06_ETH_NIC_ERROR.png > > http://www.skateman.nl/wp-content/uploads/2018/01/Screenshot-at-2018-01-08-22_16_28.png > > I was able to use some tooling like ETHTOOL to adjust some settings > and check if the interface responded. This all seems fine. > > Hope that someone can find a fix, so the Ethernet adapter can be used. > > Thanks!! Hi, Please use text logs instead of pictures next time, it's easier to read. The errors you see are related to missing MAC addresses for the unused interfaces, you can ignore these are they are not relevant for the issue you encounter. Normally the unused interfaces should have status disabled in the device tree but there is not a big deal if they fail like that. As I've advised Jamie on the other thread, please try to connect the device back 2 back to a known good machine and determine what is broken - Rx/Tx? Is there another software version that does work on these machines? Madalin
RE: DPAA Ethernet problems with mainstream Linux kernels
> -Original Message- > From: Jamie Krueger [mailto:ja...@bitbybitsoftwaregroup.com] > Sent: Friday, January 12, 2018 6:36 PM > To: Madalin-cristian Bucur ; linuxppc- > d...@lists.ozlabs.org > Cc: netdev@vger.kernel.org > Subject: Re: DPAA Ethernet problems with mainstream Linux kernels > > On 01/12/2018 08:22 AM, Madalin-cristian Bucur wrote: > >> -Original Message- > >> From: Linuxppc-dev [mailto:linuxppc-dev- > >> bounces+madalin.bucur=nxp@lists.ozlabs.org] On Behalf Of Jamie > Krueger > >> Sent: Wednesday, January 10, 2018 5:57 PM > >> To: linuxppc-...@lists.ozlabs.org > >> Subject: DPAA Ethernet problems with mainstream Linux kernels > >> > >> Hello all @ linuxppc-dev, > >> > >> I have been working with a team of people maintaining PowerPC > >> Linux for the new AmigaONE X5000/20 (a Freescale p5020 SoC based > >> machine). > >> > >> We are trying to determine why the submitted Data Path Acceleration > >> Architecture (DPAA) Ethernet Driver is not fully functional with > >> the mainstream Linux kernels. > > Hi Jamie, > Hi Madalin, > > We are testing the DPAA driver on several DS and RDB platforms and it > > is working properly. The issues you encounter with it on the X5000/20 > > are likely caused by some issues specific to that particular platform. > It is good to hear that the DPAA driver is functioning correctly > on the reference platforms. I am positive you are correct that > the issue is the difference in implementation on the X5000/20 > (Cyrus) motherboard, as compared to the reference boards. > > Can you verify which Linux Kernel sources your tests are being > performed on? We have been testing using the mainstream > Linux sources up to linux-4.15-rc6 thus far. Latest run is on 4.15.0-rc7-00200-gc92a9a4. > > The device tree that you mention, cyrus_p5020.eth.dts is not found in > > the Linux kernel sources. The cyrus_p5020.dts file from the fsl ppc > > device tree folder does not include the PHY information for the DPAA > > interfaces. The problems that you experience may be caused by some > > issues with the PHY configuration (i.e. internal delay). > The cyrus_p5020.eth.dts is a modified version of the cyrus_p5020.dts, > which of course was based off the original p5020ds.dts file. As you > noted, the current cyrus_p5020.dts file is incomplete, and does not > map the Ethernet connections properly. > > The cyrus_p5020.eth.dts file, along with it's cyrus-pre.dtsi dependent > file, are an attempt to correctly define the Ethernet hardware, as it is > implemented on the X5000/20. > > ** I have attached both the cyrus_p5020.eth.dts and cyrus-pre.dtsi > files with this email for comparison. Please let me know if you see > any corrections that should be made to either file. At a first glance they look fine to me. > I am not sure what PHY hardware/configuration you are using on the > DS and RDB platforms, but I can confirm that AmigaONE X5000/20 > (Cyrus Motherboard with p5020 SoC), has dTSEC 4 and dTSEC 5 > wired to two Micrel KSZ9021RN Gigabit Ethernet PHYs, using the > RGMII protocol. Since it's RGMII, I think you should look into RGMII internal delay requirements for this board. > > I suggest > > that you connect the DPAA interface to a traffic analyzer or directly > > to another device on which you can capture the incoming traffic and > > check that the received frames are correct. > I have started testing along that line, using Wireshark to view the > traffic on the X5000/20 itself, and from another machine connected > on the same subnet. So far (as indicated by some details of in my > initial email), I can see outgoing broadcast requests (for DHCP) > being sent out from the X5000/20, and these requests are correctly > constructed and visible outside the X5000/20. > > However, no responses to the DHCP broadcasts appear to reach > to X5000/20's DPAA Ethernet. I will need to setup some further > tests to determine if the DHCP server saw the requests and responded > to them. (I assume the DHCP server is getting them, and responding, > as I can always get a successful DHCP response to the X5000/20 > when using an add-on Ethernet PICe card on the same subnet). > > I will setup some more direct machine-to-machine testing to > see what else I can glean from the network traffic. That will provide more clarity to the actual issue. > Please have a look at the attached dts files, maybe there is something > obvious there we are not seeing. > > Also, given that the X5000/20 uses Micrel KSZ9021RN PHYs in RGMII > mode, what changes to the DPAA hardware con
RE: [PATCH net-next] net: dpaa: change while() to if() in dpaa_fq_setup()
> -Original Message-
> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
> On Behalf Of yuan linyu
> Sent: Saturday, January 13, 2018 11:25 AM
> To: netdev@vger.kernel.org
> Cc: David S . Miller ; yuan linyu
>
> Subject: Re: [PATCH net-next] net: dpaa: change while() to if() in
> dpaa_fq_setup()
>
> i am wrong, ignore it.
>
Yes, the wile is required.
Madalin
> On 六, 2018-01-13 at 17:15 +0800, yuan linyu wrote:
> > From: yuan linyu
> >
> > while loop is not needed, because list_for_each_entry() already check
> all fq.
> >
> > Signed-off-by: yuan linyu
> > ---
> > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > index 7caa8da..fd0e411 100644
> > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > @@ -931,7 +931,7 @@ static void dpaa_fq_setup(struct dpaa_priv *priv,
> > }
> >
> > /* Make sure all CPUs receive a corresponding Tx queue. */
> > - while (egress_cnt < DPAA_ETH_TXQ_NUM) {
> > + if (egress_cnt < DPAA_ETH_TXQ_NUM) {
> > list_for_each_entry(fq, &priv->dpaa_fq_list, list) {
> > if (fq->fq_type != FQ_TYPE_TX)
> > continue;
RE: DPAA Ethernet problems with mainstream Linux kernels
> -Original Message- > From: Linuxppc-dev [mailto:linuxppc-dev- > bounces+madalin.bucur=nxp@lists.ozlabs.org] On Behalf Of Jamie Krueger > Sent: Wednesday, January 10, 2018 5:57 PM > To: linuxppc-...@lists.ozlabs.org > Subject: DPAA Ethernet problems with mainstream Linux kernels > > Hello all @ linuxppc-dev, > > I have been working with a team of people maintaining PowerPC > Linux for the new AmigaONE X5000/20 (a Freescale p5020 SoC based > machine). > > We are trying to determine why the submitted Data Path Acceleration > Architecture (DPAA) Ethernet Driver is not fully functional with > the mainstream Linux kernels. Hi Jamie, We are testing the DPAA driver on several DS and RDB platforms and it is working properly. The issues you encounter with it on the X5000/20 are likely caused by some issues specific to that particular platform. The device tree that you mention, cyrus_p5020.eth.dts is not found in the Linux kernel sources. The cyrus_p5020.dts file from the fsl ppc device tree folder does not include the PHY information for the DPAA interfaces. The problems that you experience may be caused by some issues with the PHY configuration (i.e. internal delay). I suggest that you connect the DPAA interface to a traffic analyzer or directly to another device on which you can capture the incoming traffic and check that the received frames are correct. Madalin > Here is the results from my latest tests. They were performed using > the linux-4.10.17 ppc64, since that represents when the DPAA Ethernet > code was introduced. > > Similar tests, with similar results, were also performed > using the latest Linux kernels: > > linux-4.15-rc5 > linux-4.15-rc6 > linux-4.15-rc7 > > (Hence the reason for falling back to test the kernel right > after the introduction of the DPAA Ethernet driver sources) > > --- > > All Kernel builds had the DPAA Ethernet enabled in the kernel, > and are using the correct cyrus_p5020.eth.dtb device tree file > (for use on the X5000/20). > > The results are quite similar for all kernels in regards to the DPAA > Ethernet. > > All tested kernels setup the two Ethernet interfaces correctly > as eth0 and eth1, and pull the correct MAC addresses from U-Boot > environment variables ethaddr and eth1addr respectively. > > So at this point Linux has what it believes is fully configured > hardware, waiting to have an IP Address/Netmask/Gateway > to be set and to bring the interface online. > > However, all attempts to communicate with the outside world > do not make it out the physical (PHY) hardware - or do they? > > ** The following results were captured under linux-4.10.17 ** > > When I bring the interface up using a static address, in this case > 192.168.1.21, I see the following (NOTE TX bytes says 154.0 KB, > while RX bytes says 0.0 B): > > jamie@X5000-Linux:$ ifconfig > eth0 Link encap:Ethernet HWaddr 00:80:10:11:11:11 > inet addr:192.168.1.21 Bcast:192.168.1.255 Mask:255.255.255.0 > inet6 addr: fe80::280:10ff:fe11:/64 Scope:Link > UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1 > RX packets:0 errors:0 dropped:0 overruns:0 frame:0 > TX packets:1428 errors:0 dropped:0 overruns:0 carrier:0 > collisions:0 txqueuelen:1000 > RX bytes:0 (0.0 B) TX bytes:154066 (154.0 KB) > Memory:fe4e6000-fe4e6fff > > eth1 Link encap:Ethernet HWaddr 00:80:10:22:22:22 > UP BROADCAST MULTICAST MTU:1500 Metric:1 > RX packets:0 errors:0 dropped:0 overruns:0 frame:0 > TX packets:0 errors:0 dropped:0 overruns:0 carrier:0 > collisions:0 txqueuelen:1000 > RX bytes:0 (0.0 B) TX bytes:0 (0.0 B) > Memory:fe4e8000-fe4e8fff > > lo Link encap:Local Loopback > inet addr:127.0.0.1 Mask:255.0.0.0 > inet6 addr: ::1/128 Scope:Host > UP LOOPBACK RUNNING MTU:65536 Metric:1 > RX packets:1869 errors:0 dropped:0 overruns:0 frame:0 > TX packets:1869 errors:0 dropped:0 overruns:0 carrier:0 > collisions:0 txqueuelen:1000 > RX bytes:156932 (156.9 KB) TX bytes:156932 (156.9 KB) > > Checking the routing table, everything looks fine there: > > jamie@X5000-Linux:$ netstat -r > Kernel IP routing table > Destination Gateway Genmask Flags MSS Window irtt > Iface > default 192.168.1.1 0.0.0.0 UG 0 0 0 > eth0 > link-local * 255.255.0.0 U 0 0 0 > eth0 > 192.168.1.0 * 255.255.255.0 U 0 0 0 > eth0 > > Attempting to PING the interface itself works: > > jamie@X5000-Linux:$ ping 192.168.1.21 > PING 192.168.1.21 (192.168.1.21) 56(84) bytes of data. > 64 bytes from 192.168.1.21: icmp_seq=1 ttl=64 time=0.037 ms > 64 bytes from 192.168.1.21: icmp_seq=2 ttl=64 time=0.045 ms > 64 bytes from 192.168.1.21: icmp_seq=3 ttl=64 time=0.033
RE: [PATCH net] of_mdio: avoid MDIO bus removal when a PHY is missing
> -Original Message- > From: Andrew Lunn [mailto:and...@lunn.ch] > Sent: Friday, January 05, 2018 5:13 PM > To: Madalin-cristian Bucur > Subject: Re: [PATCH net] of_mdio: avoid MDIO bus removal when a PHY is > missing > > On Fri, Jan 05, 2018 at 11:36:14AM +0200, Madalin Bucur wrote: > > If one of the child devices is missing the of_mdiobus_register_phy() > > call will return -ENODEV. When a missing device is encountered the > > registration of the remaining PHYs is stopped and the MDIO bus will > > fail to register. Propagate all errors except ENODEV to avoid it. > > Hi Madalin > > This is is not clear cut. If the PHY is in device tree, the PHY should > exist. So returning ENODEV is justified. The device tree blob is > broken. But i can also see the value for continuing. There is a chance > some of your other interfaces come up, allowing you to get the correct > device tree blob for the hardware. > > Please add > > dev_err(&mdio->dev, "MDIO device at address %d is missing.\n"); > > Andrew This appears on boards that include in the device tree the description for the PHYs found on an optional riser card. When the riser card is removed, this issue is triggered. I'll send a v2 with the dev_err() included. Thanks, Madalin
RE: [RFC] Support for SGMII 2500
> -Original Message- > From: Andrew Lunn [mailto:and...@lunn.ch] > Sent: Tuesday, November 28, 2017 4:13 PM > To: Madalin-cristian Bucur > Subject: Re: [RFC] Support for SGMII 2500 > > > Hi Andrew, > > > > Bhaskar is working on enabling a PFE [1] MAC connected to an Aquantia > AQR107 > > PHY [2] on a LS1012AQDS board. Initially I've indicated 2500Base-X too, > but it > > seems the HW actually works in SGMII mode. The QDS boards are lower > volume, > > higher spec boards than the RDBs [3], they exercise most of the HW > capabilities. > > The webpage for the AQR107 lists 2500Base-X, so i assume the issue is > with the MAC? Ideally you want to use 2500Base-X, since this is wider > known. > > Anyway, you seem to have a legitimate need for it. > > However, i would prefer a different name. The convention is to put the > number first. So PHY_INTERFACE_MODE_2500SGMII. > >Andrew OK, I just wanted to make sure 2.5G SGMII is to be added separately from the "normal" SGMII, as it was done in u-boot. Thanks also for the naming hint. Madalin
RE: [RFC] Support for SGMII 2500
> -Original Message- > From: Andrew Lunn [mailto:and...@lunn.ch] > Sent: Tuesday, November 28, 2017 3:30 PM > Subject: Re: [RFC] Support for SGMII 2500 > > On Tue, Nov 28, 2017 at 07:25:48AM +, Madalin-cristian Bucur wrote: > > Hi, > > > > There is a disconnect between the SGMII 2500 support in u-boot and > Linux. > > Bhaskar is trying to add support for a SGMII interface working at > 2.5Gbps > > by using the PHY connection type "sgmii-2500" in the device tree: > > Hi Madalin > > What MAC and PHY are you using? > > I did a quick search for SGMII 2.5, and all i keep coming across is > 2500BASE-X. I just want to make sure you really do need SGMII at 2500, > and not 2500BASE-X, which is already supported. > > Thanks > Andrew Hi Andrew, Bhaskar is working on enabling a PFE [1] MAC connected to an Aquantia AQR107 PHY [2] on a LS1012AQDS board. Initially I've indicated 2500Base-X too, but it seems the HW actually works in SGMII mode. The QDS boards are lower volume, higher spec boards than the RDBs [3], they exercise most of the HW capabilities. Regards, Madalin [1] https://www.nxp.com/products/processors-and-microcontrollers/arm-based-processors-and-mcus/qoriq-layerscape-arm-processors/qoriq-layerscape-1012a-low-power-communication-processor:LS1012A [2] https://www.aquantia.com/products/enterprise/aqr107/ [3] https://www.nxp.com/support/developer-resources/software-development-tools/qoriq-developer-resources/qoriq-ls1012a-reference-design-board:LS1012A-RDB
[RFC] Support for SGMII 2500
Hi,
There is a disconnect between the SGMII 2500 support in u-boot and Linux.
Bhaskar is trying to add support for a SGMII interface working at 2.5Gbps
by using the PHY connection type "sgmii-2500" in the device tree:
phy-connection-type = "sgmii-2500";
This is supported by u-boot, in include/phy.h:
typedef enum {
PHY_INTERFACE_MODE_MII,
PHY_INTERFACE_MODE_GMII,
PHY_INTERFACE_MODE_SGMII,
PHY_INTERFACE_MODE_SGMII_2500,
...
static const char *phy_interface_strings[] = {
[PHY_INTERFACE_MODE_MII]= "mii",
[PHY_INTERFACE_MODE_GMII] = "gmii",
[PHY_INTERFACE_MODE_SGMII] = "sgmii",
[PHY_INTERFACE_MODE_SGMII_2500] = "sgmii-2500",
...
since this commit:
commit c35f8693942d8284c635592f263a0fe11abe1d1d
Author: Shengzhou Liu
Date: Thu Oct 23 17:20:57 2014 +0800
net/fm: add 2.5G SGMII support
As auto-negotiation is not supported for 2.5G SGMII, we need
to add a new type PHY_INTERFACE_MODE_SGMII_2500 to differentiate
SGMII-1G and SGMII-2.5G with different setting for auto-negotiation.
Signed-off-by: Shaohui Xie
Signed-off-by: Shengzhou Liu
Reviewed-by: York Sun
In the Linux kernel we do not have a separate define for SGMII_2500, should we
add
something like the change below?
Thanks,
Madalin
---
diff --git a/include/linux/phy.h b/include/linux/phy.h
index dc82a07..086f7a3 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -68,6 +68,7 @@ typedef enum {
PHY_INTERFACE_MODE_MII,
PHY_INTERFACE_MODE_GMII,
PHY_INTERFACE_MODE_SGMII,
+ PHY_INTERFACE_MODE_SGMII_2500,
PHY_INTERFACE_MODE_TBI,
PHY_INTERFACE_MODE_REVMII,
PHY_INTERFACE_MODE_RMII,
@@ -123,6 +124,8 @@ static inline const char *phy_modes(phy_interface_t
interface)
return "gmii";
case PHY_INTERFACE_MODE_SGMII:
return "sgmii";
+ case PHY_INTERFACE_MODE_SGMII_2500:
+ return "sgmii-2500";
case PHY_INTERFACE_MODE_TBI:
return "tbi";
case PHY_INTERFACE_MODE_REVMII:
RE: [PATCH 0/4] fsl/fman: Fix some error handling code in mac_probe
Hi Christophe,
I'll review and test your fixes.
Thank you!
Madalin
> -Original Message-
> From: Christophe JAILLET [mailto:christophe.jail...@wanadoo.fr]
> Sent: Monday, November 06, 2017 11:53 PM
> To: Madalin-cristian Bucur
> Cc: netdev@vger.kernel.org; linux-ker...@vger.kernel.org; kernel-
> janit...@vger.kernel.org; Christophe JAILLET
>
> Subject: [PATCH 0/4] fsl/fman: Fix some error handling code in mac_probe
>
> Commit c6e26ea8c893 ("dpaa_eth: change device used") generated some
> conflicts in my patches waiting for submission. So I took a closer look at
> it.
>
>
> So here is a serie of 4 patches.
>
> The 1st one is just about a spurious call to 'dev_set_drvdata()', which is
> done in only 1 error handling path in the function.
>
> The 2nd one removes some devm_iounmap/release/kfree functions which look
> useless to me.
>
> The 3rd one fixes a missing of_node_put.
>
> The 4th one is just cosmetic and removes a useless message.
>
>
> Christophe JAILLET (4):
> fsl/fman: Remove a useless call to 'dev_set_drvdata()'
> fsl/fman: Remove some useless code
> fsl/fman: Add a missing 'of_node_put()' call in an error handling path
> fsl/fman: Remove a useless 'dev_err()' call
>
> drivers/net/ethernet/freescale/fman/mac.c | 10 +++---
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> --
> 2.14.1
RE: [PATCH net-next] net: dpaa: remove init which already done in per-cpu allocation
> +Eric > > > -Original Message- > > From: Yuan, Linyu (NSB - CN/Shanghai) [mailto:linyu.yuan@nokia- > sbell.com] > > > > I just saw below accepted commit, it said "per cpu allocations are > already > > zeroed, no need to clear them again." > > > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.ker > nel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fdavem%2Fnet- > next.git%2Fcommit%2F%3Fid%3Dbfd8e5a407133e58a92a38ccf3d0ba6db81f22d8&data= > 02%7C01%7Cmadalin.bucur%40nxp.com%7C4116c18371684740c41a08d51f65c6e9%7C686 > ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636449444348895856&sdata=PBvP4J0ch > 8o%2Bvif3C8D77dVxGW4vjUlCYwJzJcIzhFk%3D&reserved=0 > > > > I will not touch memory sub-system, so I will not change this function > > description. > > > > Your particular change removes redundancy, it's fine. Having the memset > documented > somewhere would prevent similar redundancy from being added in the future. > > > > -Original Message- > > > From: Madalin-cristian Bucur [mailto:madalin.bu...@nxp.com] > > > Sent: Monday, October 30, 2017 1:56 PM > > > To: Yuan, Linyu (NSB - CN/Shanghai); yuan linyu; > netdev@vger.kernel.org; > > > gre...@linuxfoundation.org > > > Cc: David S . Miller > > > Subject: RE: [PATCH net-next] net: dpaa: remove init which already > done > > in > > > per-cpu allocation > > > > > > Also here: > > > > > > > > > > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Felixir.f > ree- > electrons.com%2Flinux%2Flatest%2Fsource%2Fmm%2Fpercpu.c%23L717&data=02%7C0 > 1%7Cmadalin.bucur%40nxp.com%7C4116c18371684740c41a08d51f65c6e9%7C686ea1d3b > c2b4c6fa92cd99c5c301635%7C0%7C0%7C636449444348895856&sdata=G9AEcoAPE7yWH%2 > FHi56DP3zns8LaIixJ4xkuQCTJDI5E%3D&reserved=0 > > > > > > Should the fact that the memory is zeroed be included in the function > > > description? > > > > > > /** > > > * pcpu_alloc - the percpu allocator > > > * @size: size of area to allocate in bytes > > > * @align: alignment of area (max PAGE_SIZE) > > > * @reserved: allocate from the reserved chunk if available > > > * @gfp: allocation flags > > > * > > > * Allocate percpu area of @size bytes aligned at @align. If @gfp > > doesn't > > > * contain %GFP_KERNEL, the allocation is atomic. > > > * > > > * RETURNS: > > > * Percpu pointer to the allocated area on success, NULL on failure. > > > */ > > > > > > Now it seems to be an implementation detail rather than a guarantee. > > > > > > Looking at Documentation/driver-model/devres.txt, the memset is not > > > mentioned there either. > > > > > > > -Original Message- > > > > From: Yuan, Linyu (NSB - CN/Shanghai) [mailto:linyu.yuan@nokia- > > sbell.com] > > > > Sent: Monday, October 30, 2017 7:21 AM > > > > To: Madalin-cristian Bucur ; yuan linyu > > > > ; netdev@vger.kernel.org > > > > Cc: David S . Miller > > > > Subject: RE: [PATCH net-next] net: dpaa: remove init which already > > > > done in per-cpu allocation > > > > > > > > > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Felixir.f > ree- > electrons.com%2Flinux%2Flatest%2Fsource%2Fmm%2Fpercpu.c%23L1018&data=02%7C > 01%7Cmadalin.bucur%40nxp.com%7C4116c18371684740c41a08d51f65c6e9%7C686ea1d3 > bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636449444348895856&sdata=%2FOCOMao4jJti > ptOqUUXBbZMt5TNRXdFfyrqa1zIekAI%3D&reserved=0 > > > > > > > > > -Original Message- > > > > > From: Madalin-cristian Bucur [mailto:madalin.bu...@nxp.com] > > > > > Sent: Monday, October 30, 2017 1:15 PM > > > > > To: Yuan, Linyu (NSB - CN/Shanghai); yuan linyu; > > netdev@vger.kernel.org > > > > > Cc: David S . Miller > > > > > Subject: RE: [PATCH net-next] net: dpaa: remove init which already > > > > > done in per-cpu allocation > > > > > > > > > > Is the memset part documented? > > > > > Can you point to the specific comment & code that does it? > > > > > > > > > > Thanks > > > > > > > > > > > -Original Message- > > > > > > From: Yuan, Linyu (NSB - CN/Shanghai) [mailto:linyu.yuan@nokia- > > > > sbell.com] > > > > > > Sent:
RE: [PATCH net-next] net: dpaa: remove init which already done in per-cpu allocation
+Eric > -Original Message- > From: Yuan, Linyu (NSB - CN/Shanghai) [mailto:linyu.y...@nokia-sbell.com] > > I just saw below accepted commit, it said "per cpu allocations are already > zeroed, no need to clear them again." > > https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=bfd8e5a407133e58a92a38ccf3d0ba6db81f22d8 > > I will not touch memory sub-system, so I will not change this function > description. > Your particular change removes redundancy, it's fine. Having the memset documented somewhere would prevent similar redundancy from being added in the future. > > -Original Message- > > From: Madalin-cristian Bucur [mailto:madalin.bu...@nxp.com] > > Sent: Monday, October 30, 2017 1:56 PM > > To: Yuan, Linyu (NSB - CN/Shanghai); yuan linyu; netdev@vger.kernel.org; > > gre...@linuxfoundation.org > > Cc: David S . Miller > > Subject: RE: [PATCH net-next] net: dpaa: remove init which already done > in > > per-cpu allocation > > > > Also here: > > > > > > http://elixir.free-electrons.com/linux/latest/source/mm/percpu.c#L717 > > > > Should the fact that the memory is zeroed be included in the function > > description? > > > > /** > > * pcpu_alloc - the percpu allocator > > * @size: size of area to allocate in bytes > > * @align: alignment of area (max PAGE_SIZE) > > * @reserved: allocate from the reserved chunk if available > > * @gfp: allocation flags > > * > > * Allocate percpu area of @size bytes aligned at @align. If @gfp > doesn't > > * contain %GFP_KERNEL, the allocation is atomic. > > * > > * RETURNS: > > * Percpu pointer to the allocated area on success, NULL on failure. > > */ > > > > Now it seems to be an implementation detail rather than a guarantee. > > > > Looking at Documentation/driver-model/devres.txt, the memset is not > > mentioned there either. > > > > > -Original Message- > > > From: Yuan, Linyu (NSB - CN/Shanghai) [mailto:linyu.yuan@nokia- > sbell.com] > > > Sent: Monday, October 30, 2017 7:21 AM > > > To: Madalin-cristian Bucur ; yuan linyu > > > ; netdev@vger.kernel.org > > > Cc: David S . Miller > > > Subject: RE: [PATCH net-next] net: dpaa: remove init which already > > > done in per-cpu allocation > > > > > > http://elixir.free-electrons.com/linux/latest/source/mm/percpu.c#L1018 > > > > > > > -Original Message- > > > > From: Madalin-cristian Bucur [mailto:madalin.bu...@nxp.com] > > > > Sent: Monday, October 30, 2017 1:15 PM > > > > To: Yuan, Linyu (NSB - CN/Shanghai); yuan linyu; > netdev@vger.kernel.org > > > > Cc: David S . Miller > > > > Subject: RE: [PATCH net-next] net: dpaa: remove init which already > > > > done in per-cpu allocation > > > > > > > > Is the memset part documented? > > > > Can you point to the specific comment & code that does it? > > > > > > > > Thanks > > > > > > > > > -Original Message- > > > > > From: Yuan, Linyu (NSB - CN/Shanghai) [mailto:linyu.yuan@nokia- > > > sbell.com] > > > > > Sent: Monday, October 30, 2017 7:12 AM > > > > > To: Madalin-cristian Bucur ; yuan linyu > > > > > ; netdev@vger.kernel.org > > > > > Cc: David S . Miller > > > > > Subject: RE: [PATCH net-next] net: dpaa: remove init which already > > > done in > > > > > per-cpu allocation > > > > > > > > > > Hi, > > > > > > > > > > devm_alloc_percpu() will allocate per-cpu memory and memset > allocated > > > > > block content to 0. > > > > > > > > > > > -Original Message- > > > > > > From: Madalin-cristian Bucur [mailto:madalin.bu...@nxp.com] > > > > > > Sent: Monday, October 30, 2017 1:08 PM > > > > > > To: yuan linyu; netdev@vger.kernel.org > > > > > > Cc: David S . Miller; Yuan, Linyu (NSB - CN/Shanghai) > > > > > > Subject: RE: [PATCH net-next] net: dpaa: remove init which > > > > > > already done in per-cpu allocation > > > > > > > > > > > > Hi Yuan, > > > > > > > > > > > > Can you please give more details about this change you are > > > proposing? > > > > > > > > > > > > Regards, > > >
RE: [PATCH net-next] net: dpaa: remove init which already done in per-cpu allocation
Also here:
http://elixir.free-electrons.com/linux/latest/source/mm/percpu.c#L717
Should the fact that the memory is zeroed be included in the function
description?
/**
* pcpu_alloc - the percpu allocator
* @size: size of area to allocate in bytes
* @align: alignment of area (max PAGE_SIZE)
* @reserved: allocate from the reserved chunk if available
* @gfp: allocation flags
*
* Allocate percpu area of @size bytes aligned at @align. If @gfp doesn't
* contain %GFP_KERNEL, the allocation is atomic.
*
* RETURNS:
* Percpu pointer to the allocated area on success, NULL on failure.
*/
Now it seems to be an implementation detail rather than a guarantee.
Looking at Documentation/driver-model/devres.txt, the memset is not mentioned
there either.
> -Original Message-
> From: Yuan, Linyu (NSB - CN/Shanghai) [mailto:linyu.y...@nokia-sbell.com]
> Sent: Monday, October 30, 2017 7:21 AM
> To: Madalin-cristian Bucur ; yuan linyu
> ; netdev@vger.kernel.org
> Cc: David S . Miller
> Subject: RE: [PATCH net-next] net: dpaa: remove init which already done in
> per-cpu allocation
>
> http://elixir.free-electrons.com/linux/latest/source/mm/percpu.c#L1018
>
> > -Original Message-
> > From: Madalin-cristian Bucur [mailto:madalin.bu...@nxp.com]
> > Sent: Monday, October 30, 2017 1:15 PM
> > To: Yuan, Linyu (NSB - CN/Shanghai); yuan linyu; netdev@vger.kernel.org
> > Cc: David S . Miller
> > Subject: RE: [PATCH net-next] net: dpaa: remove init which already done
> in
> > per-cpu allocation
> >
> > Is the memset part documented?
> > Can you point to the specific comment & code that does it?
> >
> > Thanks
> >
> > > -Original Message-
> > > From: Yuan, Linyu (NSB - CN/Shanghai) [mailto:linyu.yuan@nokia-
> sbell.com]
> > > Sent: Monday, October 30, 2017 7:12 AM
> > > To: Madalin-cristian Bucur ; yuan linyu
> > > ; netdev@vger.kernel.org
> > > Cc: David S . Miller
> > > Subject: RE: [PATCH net-next] net: dpaa: remove init which already
> done in
> > > per-cpu allocation
> > >
> > > Hi,
> > >
> > > devm_alloc_percpu() will allocate per-cpu memory and memset allocated
> > > block content to 0.
> > >
> > > > -Original Message-
> > > > From: Madalin-cristian Bucur [mailto:madalin.bu...@nxp.com]
> > > > Sent: Monday, October 30, 2017 1:08 PM
> > > > To: yuan linyu; netdev@vger.kernel.org
> > > > Cc: David S . Miller; Yuan, Linyu (NSB - CN/Shanghai)
> > > > Subject: RE: [PATCH net-next] net: dpaa: remove init which already
> done
> > > in
> > > > per-cpu allocation
> > > >
> > > > Hi Yuan,
> > > >
> > > > Can you please give more details about this change you are
> proposing?
> > > >
> > > > Regards,
> > > > Madalin
> > > >
> > > > > -Original Message-
> > > > > From: netdev-ow...@vger.kernel.org
> > > > [mailto:netdev-ow...@vger.kernel.org]
> > > > > On Behalf Of yuan linyu
> > > > > Sent: Sunday, October 29, 2017 3:49 AM
> > > > > To: netdev@vger.kernel.org
> > > > > Cc: David S . Miller ; yuan linyu
> > > > >
> > > > > Subject: [PATCH net-next] net: dpaa: remove init which already
> done in
> > > > > per-cpu allocation
> > > > >
> > > > > From: yuan linyu
> > > > >
> > > > > Signed-off-by: yuan linyu
> > > > > ---
> > > > > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 4
> > > > > 1 file changed, 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > > > b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > > > index a8d0be8..1ccc316 100644
> > > > > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > > > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > > > @@ -2814,10 +2814,6 @@ static int dpaa_eth_probe(struct
> > > > platform_device
> > > > > *pdev)
> > > > > err = -ENOMEM;
> > > > > goto free_dpaa_fqs;
> > > > > }
> > > > > - for_each_possible_cpu(i) {
> > > > > - percpu_priv = per_cpu_ptr(priv->percpu_priv, i);
> > > > > - memset(percpu_priv, 0, sizeof(*percpu_priv));
> > > > > - }
> > > > >
> > > > > priv->num_tc = 1;
> > > > > netif_set_real_num_tx_queues(net_dev, priv->num_tc *
> > > > > DPAA_TC_TXQ_NUM);
> > > > > --
> > > > > 2.7.4
> > > > >
RE: [PATCH net-next] net: dpaa: remove init which already done in per-cpu allocation
Is the memset part documented?
Can you point to the specific comment & code that does it?
Thanks
> -Original Message-
> From: Yuan, Linyu (NSB - CN/Shanghai) [mailto:linyu.y...@nokia-sbell.com]
> Sent: Monday, October 30, 2017 7:12 AM
> To: Madalin-cristian Bucur ; yuan linyu
> ; netdev@vger.kernel.org
> Cc: David S . Miller
> Subject: RE: [PATCH net-next] net: dpaa: remove init which already done in
> per-cpu allocation
>
> Hi,
>
> devm_alloc_percpu() will allocate per-cpu memory and memset allocated
> block content to 0.
>
> > -----Original Message-
> > From: Madalin-cristian Bucur [mailto:madalin.bu...@nxp.com]
> > Sent: Monday, October 30, 2017 1:08 PM
> > To: yuan linyu; netdev@vger.kernel.org
> > Cc: David S . Miller; Yuan, Linyu (NSB - CN/Shanghai)
> > Subject: RE: [PATCH net-next] net: dpaa: remove init which already done
> in
> > per-cpu allocation
> >
> > Hi Yuan,
> >
> > Can you please give more details about this change you are proposing?
> >
> > Regards,
> > Madalin
> >
> > > -Original Message-
> > > From: netdev-ow...@vger.kernel.org
> > [mailto:netdev-ow...@vger.kernel.org]
> > > On Behalf Of yuan linyu
> > > Sent: Sunday, October 29, 2017 3:49 AM
> > > To: netdev@vger.kernel.org
> > > Cc: David S . Miller ; yuan linyu
> > >
> > > Subject: [PATCH net-next] net: dpaa: remove init which already done in
> > > per-cpu allocation
> > >
> > > From: yuan linyu
> > >
> > > Signed-off-by: yuan linyu
> > > ---
> > > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 4
> > > 1 file changed, 4 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > index a8d0be8..1ccc316 100644
> > > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > > @@ -2814,10 +2814,6 @@ static int dpaa_eth_probe(struct
> > platform_device
> > > *pdev)
> > > err = -ENOMEM;
> > > goto free_dpaa_fqs;
> > > }
> > > - for_each_possible_cpu(i) {
> > > - percpu_priv = per_cpu_ptr(priv->percpu_priv, i);
> > > - memset(percpu_priv, 0, sizeof(*percpu_priv));
> > > - }
> > >
> > > priv->num_tc = 1;
> > > netif_set_real_num_tx_queues(net_dev, priv->num_tc *
> > > DPAA_TC_TXQ_NUM);
> > > --
> > > 2.7.4
> > >
RE: [PATCH net-next] net: dpaa: remove init which already done in per-cpu allocation
Hi Yuan,
Can you please give more details about this change you are proposing?
Regards,
Madalin
> -Original Message-
> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
> On Behalf Of yuan linyu
> Sent: Sunday, October 29, 2017 3:49 AM
> To: netdev@vger.kernel.org
> Cc: David S . Miller ; yuan linyu
>
> Subject: [PATCH net-next] net: dpaa: remove init which already done in
> per-cpu allocation
>
> From: yuan linyu
>
> Signed-off-by: yuan linyu
> ---
> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> index a8d0be8..1ccc316 100644
> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> @@ -2814,10 +2814,6 @@ static int dpaa_eth_probe(struct platform_device
> *pdev)
> err = -ENOMEM;
> goto free_dpaa_fqs;
> }
> - for_each_possible_cpu(i) {
> - percpu_priv = per_cpu_ptr(priv->percpu_priv, i);
> - memset(percpu_priv, 0, sizeof(*percpu_priv));
> - }
>
> priv->num_tc = 1;
> netif_set_real_num_tx_queues(net_dev, priv->num_tc *
> DPAA_TC_TXQ_NUM);
> --
> 2.7.4
>
RE: [PATCH v2 2/5] dpaa_eth: move of_phy_connect() to the eth driver
> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Sunday, October 15, 2017 9:34 PM
> To: Madalin-cristian Bucur
> Cc: netdev@vger.kernel.org; da...@davemloft.net; f.faine...@gmail.com;
> vivien.dide...@savoirfairelinux.com; jun...@outlook.com; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH v2 2/5] dpaa_eth: move of_phy_connect() to the eth
> driver
>
> On Fri, Oct 13, 2017 at 05:50:09PM +0300, Madalin Bucur wrote:
> > Signed-off-by: Madalin Bucur
> > ---
> > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 48 +++--
> > drivers/net/ethernet/freescale/fman/mac.c | 97 ++-
> ---
> > drivers/net/ethernet/freescale/fman/mac.h | 5 +-
> > 3 files changed, 66 insertions(+), 84 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > index 4225806..7cf61d6 100644
> > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > @@ -2435,6 +2435,48 @@ static void dpaa_eth_napi_disable(struct
> dpaa_priv *priv)
> > }
> > }
> >
> > +static void dpaa_adjust_link(struct net_device *net_dev)
> > +{
> > + struct mac_device *mac_dev;
> > + struct dpaa_priv *priv;
> > +
> > + priv = netdev_priv(net_dev);
> > + mac_dev = priv->mac_dev;
> > + mac_dev->adjust_link(mac_dev);
> > +}
> > +
> > +static int dpaa_phy_init(struct net_device *net_dev)
> > +{
> > + struct mac_device *mac_dev;
> > + struct phy_device *phy_dev;
> > + struct dpaa_priv *priv;
> > +
> > + priv = netdev_priv(net_dev);
> > + mac_dev = priv->mac_dev;
> > +
> > + phy_dev = of_phy_connect(net_dev, mac_dev->phy_node,
> > +&dpaa_adjust_link, 0,
> > +mac_dev->phy_if);
> > + if (!phy_dev) {
> > + netif_err(priv, ifup, net_dev, "init_phy() failed\n");
> > + return -ENODEV;
> > + }
> > +
> > + /* Remove any features not supported by the controller */
> > + phy_dev->supported &= mac_dev->if_support;
> > +
> > + /* Enable the symmetric and asymmetric PAUSE frame advertisements,
> > +* as most of the PHY drivers do not enable them by default.
> > +*/
>
> Hi Madalin
>
> This is just moving code around, so the patch is O.K. However, it
> would be nice to have a followup patch. This comment is wrong. The phy
> driver should never enable symmetric and asymmetric PAUSE frames. The
> MAC needs to, because only the MAC knows if the MAC supports pause
> frames.
>
> Andrew
Hi Andrew,
This is obsolete and it will be removed, I'll send a v3. It remained there from
a time it used to be valid (the original DPAA Ethernet driver was developed
and maintained out of tree since about 9 years ago). I see this thread on the
subject which is relatively recent:
https://www.spinics.net/lists/netdev/msg404288.html
Thanks,
Madalin
RE: [PATCH v2 5/5] fsl/fman: add dpaa in module names
> -Original Message- > From: Florian Fainelli [mailto:f.faine...@gmail.com] > Sent: Friday, October 13, 2017 8:39 PM > To: Madalin-cristian Bucur ; > netdev@vger.kernel.org; da...@davemloft.net > Cc: and...@lunn.ch; vivien.dide...@savoirfairelinux.com; > jun...@outlook.com; linux-ker...@vger.kernel.org > Subject: Re: [PATCH v2 5/5] fsl/fman: add dpaa in module names > > On 10/13/2017 07:50 AM, Madalin Bucur wrote: > > Signed-off-by: Madalin Bucur > > You should provide a line or two to explain why are you making this > change, it is to resolve modular build configurations? This change just renames the FMan driver modules, using a common prefix for the DPAA FMan and DPAA Ethernet drivers. Besides making the names more aligned, this allows writing udev rules that match on either driver name, if needed, using the fsl_dpaa_* prefix. The change of netdev dev required for the DSA probing makes the previous rules written using this prefix fail, this change makes them work again. > > --- > > drivers/net/ethernet/freescale/fman/Makefile | 12 ++-- > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/net/ethernet/freescale/fman/Makefile > b/drivers/net/ethernet/freescale/fman/Makefile > > index 2c38119..4ae524a 100644 > > --- a/drivers/net/ethernet/freescale/fman/Makefile > > +++ b/drivers/net/ethernet/freescale/fman/Makefile > > @@ -1,9 +1,9 @@ > > subdir-ccflags-y += -I$(srctree)/drivers/net/ethernet/freescale/fman > > > > -obj-$(CONFIG_FSL_FMAN) += fsl_fman.o > > -obj-$(CONFIG_FSL_FMAN) += fsl_fman_port.o > > -obj-$(CONFIG_FSL_FMAN) += fsl_mac.o > > +obj-$(CONFIG_FSL_FMAN) += fsl_dpaa_fman.o > > +obj-$(CONFIG_FSL_FMAN) += fsl_dpaa_fman_port.o > > +obj-$(CONFIG_FSL_FMAN) += fsl_dpaa_mac.o > > > > -fsl_fman-objs := fman_muram.o fman.o fman_sp.o fman_keygen.o > > -fsl_fman_port-objs := fman_port.o > > -fsl_mac-objs:= mac.o fman_dtsec.o fman_memac.o fman_tgec.o > > +fsl_dpaa_fman-objs := fman_muram.o fman.o fman_sp.o fman_keygen.o > > +fsl_dpaa_fman_port-objs := fman_port.o > > +fsl_dpaa_mac-objs:= mac.o fman_dtsec.o fman_memac.o fman_tgec.o > > > > > -- > Florian
RE: [PATCH 3/4] dpaa_eth: change device used
> -Original Message-
> From: David Miller [mailto:da...@davemloft.net]
> Sent: Thursday, October 12, 2017 12:02 AM
> To: Madalin-cristian Bucur
> Subject: Re: [PATCH 3/4] dpaa_eth: change device used
>
> From: Madalin Bucur
> Date: Tue, 10 Oct 2017 17:10:17 +0300
>
> > @@ -2696,7 +2681,13 @@ static int dpaa_eth_probe(struct platform_device
> *pdev)
> > int err = 0, i, channel;
> > struct device *dev;
> >
> > - dev = &pdev->dev;
> > + /* device used for DMA mapping */
> > + dev = pdev->dev.parent;
> > + err = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(40));
> > + if (err) {
> > + dev_err(dev, "dma_coerce_mask_and_coherent() failed\n");
> > + goto dev_mask_failed;
> > + }
> >
> > /* Allocate this early, so we can store relevant information in
> > * the private area
>
> Since you are moving this code up before the netdev allocation, you must
> adjust the failure path goto label used.
>
> Your change as-is will cause an OOPS because we'll pass a NULL pointer
> to free_netdev().
Thank you, besides this new issue I was introducing I see there other problems,
I'll include a cleanup of these error paths in v2.
Madalin
RE: [PATCH] fsl/fman: remove of_node
> -Original Message- > From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] > On Behalf Of Madalin-cristian Bucur > Sent: Wednesday, October 04, 2017 12:54 PM > To: David Miller > Subject: RE: [PATCH] fsl/fman: remove of_node > > > -Original Message- > > From: David Miller [mailto:da...@davemloft.net] > > Sent: Wednesday, October 04, 2017 7:44 AM > > To: Madalin-cristian Bucur > > Cc: netdev@vger.kernel.org; and...@lunn.ch; f.faine...@gmail.com; linux- > > ker...@vger.kernel.org > > Subject: Re: [PATCH] fsl/fman: remove of_node > > > > From: Madalin-cristian Bucur > > Date: Tue, 3 Oct 2017 08:49:31 + > > > > > My patch removes the of_node that was set to a device that was not an > > > of_device, preventing duplicated probing of both the real of_device > > > and the "fake" one created through this assignment. > > > > > > I understand that the DSA issue that triggered the initial change > > > was related to DSA finding the network devices using > > > of_find_net_device_by_node(), something that will not work for the > > > DPAA case where the netdevice does not have an of_node. I do not know > > > enough about DSA to come up with a solution for this problem now. > > > Andrew, Florian, can you please comment on this? > > > > It sounds like you're knowingly breaking DSA. > > It never worked, even with the change I'm reverting. I'll resend this change as part of a patch set that changes the device used as net_dev dev to ensure DSA will find a of_device there. To make that work some changes to adjust link (that also make it cleaner) were needed. Also, to keep the old udev rules happy, I've changed the names of the FMan kernel modules from fsl_fman_* to fsl_dpaa_fman*. I do not have a DSA setup to test so I just tested the part related to of_find_net_device_by_node() being able to determine the net_device based on a device tree handle using an artificial device tree and code construct. I hope that will help the initial reporter of the DSA issue on DPAA (Junote Cai). Madalin
RE: [PATCH] fsl/fman: remove of_node
> -Original Message- > From: Andrew Lunn [mailto:and...@lunn.ch] > Sent: Tuesday, October 03, 2017 4:01 PM > To: Madalin-cristian Bucur > Cc: David Miller ; netdev@vger.kernel.org; > f.faine...@gmail.com; linux-ker...@vger.kernel.org > Subject: Re: [PATCH] fsl/fman: remove of_node > > On Tue, Oct 03, 2017 at 08:49:31AM +0000, Madalin-cristian Bucur wrote: > > > -Original Message- > > > From: David Miller [mailto:da...@davemloft.net] > > > Sent: Tuesday, October 03, 2017 2:05 AM > > > To: Madalin-cristian Bucur > > > Subject: Re: [PATCH] fsl/fman: remove of_node > > > > > > From: Madalin Bucur > > > Date: Mon, 2 Oct 2017 13:31:37 +0300 > > > > > > > The FMan MAC driver allocates a platform device for the Ethernet > > > > driver to probe on. Setting pdev->dev.of_node with the MAC node > > > > triggers the MAC driver probing of the new platform device. While > > > > this fails quickly and does not affect the functionality of the > > > > drivers, it is incorrect and must be removed. This was added to > > > > address a report that DSA code using of_find_net_device_by_node() > > > > is unable to use the DPAA interfaces. Error message seen before > > > > this fix: > > > > > > > > fsl_mac dpaa-ethernet.0: __devm_request_mem_region(mac) failed > > > > fsl_mac: probe of dpaa-ethernet.0 failed with error -16 > > > > > > > > Signed-off-by: Madalin Bucur > > > > > > Is the DSA issue no longer something we need to be concerned > > > about? If not, why? You have to explain this. > > > > My patch removes the of_node that was set to a device that was not an > > of_device, preventing duplicated probing of both the real of_device > > and the "fake" one created through this assignment. > > > > I understand that the DSA issue that triggered the initial change > > was related to DSA finding the network devices using > > of_find_net_device_by_node(), something that will not work for the > > DPAA case where the netdevice does not have an of_node. I do not know > > enough about DSA to come up with a solution for this problem now. > > Andrew, Florian, can you please comment on this? > > > > Thanks, > > Hi Madalin > > I guess the real fix is to throw away the platform device. But that is > a big change. > > I've not looked at the code in detail. Why is the platform device > needed? > > Andrew There are multiple components that are aggregated by the DPAA Ethernet netdevice, the FMan MAC is one of them. There is no entry in the device tree for the Ethernet device as this is just a software construct that binds together multiple pieces from the DPAA FMan, QMan, BMan. The probing of the Ethernet driver is performed against a platform device that is created in the FMan MAC. Setting the of_node in the new platform device makes it look like an of_device without being one, thus the errors at the MAC driver probing against the platform device. Does DSA work for systems that do not use a device tree to boot, i.e. ACPI? Madalin
RE: [PATCH] fsl/fman: remove of_node
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Sent: Wednesday, October 04, 2017 7:44 AM > To: Madalin-cristian Bucur > Cc: netdev@vger.kernel.org; and...@lunn.ch; f.faine...@gmail.com; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH] fsl/fman: remove of_node > > From: Madalin-cristian Bucur > Date: Tue, 3 Oct 2017 08:49:31 + > > > My patch removes the of_node that was set to a device that was not an > > of_device, preventing duplicated probing of both the real of_device > > and the "fake" one created through this assignment. > > > > I understand that the DSA issue that triggered the initial change > > was related to DSA finding the network devices using > > of_find_net_device_by_node(), something that will not work for the > > DPAA case where the netdevice does not have an of_node. I do not know > > enough about DSA to come up with a solution for this problem now. > > Andrew, Florian, can you please comment on this? > > It sounds like you're knowingly breaking DSA. It never worked, even with the change I'm reverting.
RE: [PATCH] fsl/fman: remove of_node
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Sent: Tuesday, October 03, 2017 2:05 AM > To: Madalin-cristian Bucur > Subject: Re: [PATCH] fsl/fman: remove of_node > > From: Madalin Bucur > Date: Mon, 2 Oct 2017 13:31:37 +0300 > > > The FMan MAC driver allocates a platform device for the Ethernet > > driver to probe on. Setting pdev->dev.of_node with the MAC node > > triggers the MAC driver probing of the new platform device. While > > this fails quickly and does not affect the functionality of the > > drivers, it is incorrect and must be removed. This was added to > > address a report that DSA code using of_find_net_device_by_node() > > is unable to use the DPAA interfaces. Error message seen before > > this fix: > > > > fsl_mac dpaa-ethernet.0: __devm_request_mem_region(mac) failed > > fsl_mac: probe of dpaa-ethernet.0 failed with error -16 > > > > Signed-off-by: Madalin Bucur > > Is the DSA issue no longer something we need to be concerned > about? If not, why? You have to explain this. My patch removes the of_node that was set to a device that was not an of_device, preventing duplicated probing of both the real of_device and the "fake" one created through this assignment. I understand that the DSA issue that triggered the initial change was related to DSA finding the network devices using of_find_net_device_by_node(), something that will not work for the DPAA case where the netdevice does not have an of_node. I do not know enough about DSA to come up with a solution for this problem now. Andrew, Florian, can you please comment on this? Thanks, Madalin
RE: [PATCH] fsl/fman: make arrays port_ids static, reduces object code size
> -Original Message- > From: Colin King [mailto:colin.k...@canonical.com] > Sent: Thursday, August 31, 2017 4:25 PM > Subject: [PATCH] fsl/fman: make arrays port_ids static, reduces object > code size > > From: Colin Ian King > > Don't populate the arrays port_ids on the stack, instead make them static. > Makes the object code smaller by over 700 bytes: > > Before: >text data bss dec hex filename > 28785 5832 192 3480987f9 fman.o > > After: >text data bss dec hex filename > 27921 5992 192 341058539 fman.o > > Signed-off-by: Colin Ian King Thanks, Madalin
RE: [PATCH v3 0/7] Add RSS to DPAA 1.x Ethernet driver
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Subject: Re: [PATCH v3 0/7] Add RSS to DPAA 1.x Ethernet driver > > From: David Miller > Date: Thu, 24 Aug 2017 09:42:20 -0700 (PDT) > > > From: Madalin Bucur > > Date: Thu, 24 Aug 2017 10:28:21 +0300 > > > >> This patch set introduces Receive Side Scaling for the DPAA Ethernet > >> driver. Documentation is updated with details related to the new > >> feature and limitations that apply. > >> Added also a small fix. > >> > >> v2: removed a C++ style comment > >> v3: move struct fman to header file to avoid exporting a function > > > > Series applied, thanks. > > Actually I'm reverting, this doesn't even compile. Hi, Sorry for this blunder, I've only tested on PPC, where it works. Will come back with a proper patch set. Madalin
RE: [PATCH v2 1/6] fsl/fman: enable FMan Keygen
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Sent: Wednesday, August 23, 2017 7:47 AM > To: Madalin-cristian Bucur > Cc: netdev@vger.kernel.org; linuxppc-...@lists.ozlabs.org; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH v2 1/6] fsl/fman: enable FMan Keygen > > From: Madalin-cristian Bucur > Date: Wed, 23 Aug 2017 04:36:56 + > > > The struct fman is only visible in the fman file, the fman port > > module uses struct fman as an opaque pointer, thus this export. > > Don't use that programming model. > > Export the datastructure properly to it's users. > > This abstraction scheme is so wasteful and costly. Normally does not come with this cost, it's this case where one of the sub-modules needs to call into another that gets things complicated. I'll move struct fman to the header file. Thanks, Madalin
RE: [PATCH v2 1/6] fsl/fman: enable FMan Keygen
> -Original Message-
> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
> On Behalf Of David Miller
> Sent: Wednesday, August 23, 2017 12:35 AM
> Subject: Re: [PATCH v2 1/6] fsl/fman: enable FMan Keygen
>
> From: Madalin Bucur
> Date: Tue, 22 Aug 2017 20:31:01 +0300
>
> > /**
> > + * fman_get_keygen
> > + *
> > + * @fman: A Pointer to FMan device
> > + *
> > + * Get the handle to KeyGen module part of FM driver
> > + *
> > + * Return: Handle to KeyGen
> > + */
> > +struct fman_keygen *fman_get_keygen(struct fman *fman)
> > +{
> > + return fman->keygen;
> > +}
> > +EXPORT_SYMBOL(fman_get_keygen);
>
> Please don't do this.
>
> Just directly derefence the pointer in the source code to
> get the keygen.
>
> Thank you.
Hi,
The struct fman is only visible in the fman file, the fman port module uses
struct
fman as an opaque pointer, thus this export.
Madalin
RE: [PATCH net-next] fsl/fman: implement several errata workarounds
> -Original Message- > From: Florinel Iordache [mailto:florinel.iorda...@nxp.com] > Subject: [PATCH net-next] fsl/fman: implement several errata workarounds > > Implemented workarounds for the following dTSEC Erratum: > A002, A004, A0012, A0014, A004839 on several operations > that involve MAC CFG register changes: adjust link, > rx pause frames, modify MAC address. > > Signed-off-by: Florinel Iordache Acked-by: Madalin Bucur
RE: [PATCH] dpaa_eth: use correct device for DMA mapping API
> -Original Message-
> From: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] On Behalf Of
> Arnd Bergmann
> Subject: Re: [PATCH] dpaa_eth: use correct device for DMA mapping API
>
> On Tue, Jul 11, 2017 at 10:50 AM, Madalin-cristian Bucur
> wrote:
>
> > Hi Arnd,
> >
> > Thanks for looking into this, I've tested your fix, it seems to need
> more work:
> >
> > [0.894968] platform: DMA map failed
> > [0.898627] platform: DMA map failed
> > [0.902288] platform: DMA map failed
> > [0.905947] platform: DMA map failed
> > [0.909606] platform: DMA map failed
> > [0.913265] platform: DMA map failed
>
> I see: the assignment ended up after the first use, so ->dev was still
> NULL here.
>
> This should fix the problem you saw here:
>
> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> index f7b0b928cd53..988c0212ce7e 100644
> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> @@ -2650,6 +2650,8 @@ static int dpaa_eth_probe(struct platform_device
> *pdev)
> for (i = 0; i < DPAA_BPS_NUM; i++) {
> int err;
>
> + /* DMA operations are done on the platform-provided device */
> + dpaa_bps[i]->dev = dev->parent;
> dpaa_bps[i] = dpaa_bp_alloc(dev);
Your new change de-references dpaa_bps[i] before it is set, this won't work
either.
> if (IS_ERR(dpaa_bps[i]))
> return PTR_ERR(dpaa_bps[i]);
> @@ -2657,8 +2659,6 @@ static int dpaa_eth_probe(struct platform_device
> *pdev)
> dpaa_bps[i]->raw_size = bpool_buffer_raw_size(i, DPAA_BPS_NUM);
> /* avoid runtime computations by keeping the usable size here */
> dpaa_bps[i]->size = dpaa_bp_size(dpaa_bps[i]->raw_size);
> - /* DMA operations are done on the platform-provided device */
> - dpaa_bps[i]->dev = dev->parent;
>
> err = dpaa_bp_alloc_pool(dpaa_bps[i]);
> if (err < 0) {
>
> > @@ -2806,7 +2799,6 @@ static int dpaa_eth_probe(struct platform_device
> *pdev)
> > dpaa_bps_free(priv);
> > bp_create_failed:
> > fq_probe_failed:
> > -dev_mask_failed:
> > mac_probe_failed:
> > dev_set_drvdata(dev, NULL);
> > free_netdev(net_dev);
> >
> > I'll try to address your concern about performing the DMA mapping on a
> different
> > device than the one set up by the platform code.
>
> Thanks!
>
> Arnd
RE: [PATCH] dpaa_eth: use correct device for DMA mapping API
> -Original Message-
> From: Arnd Bergmann [mailto:a...@arndb.de]
> Sent: Monday, July 10, 2017 6:14 PM
> Subject: [PATCH] dpaa_eth: use correct device for DMA mapping API
>
> Geert Uytterhoeven ran into a build error without CONFIG_HAS_DMA,
> as a result of the driver calling set_dma_ops(). While we can
> fix the build error in the dma-mapping implementation, there is
> another problem in this driver:
>
> The configuration for the DMA is done by the platform code,
> looking up information about the system from the device tree.
> This copies the information only in an incomplete way, setting
> the dma_map_ops and forcing a specific mask, but ignoring all
> settings regarding IOMMU, coherence etc.
>
> A better way to avoid the problem is to only ever pass a device
> into the dma_mapping implementation that has been setup by the
> platform code. In this case, that is the parent device, so we
> can get that pointer at probe time. Fortunately, we already have
> a pointer in the device specific structure for that, so we only
> need to modify that.
>
> Fixes: fb52728a9294 ("dpaa_eth: reuse the dma_ops provided by the FMan MAC
> device")
> Signed-off-by: Arnd Bergmann
> ---
> Not tested, please see if this works before applying!
> ---
> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 11 ++-
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> index 757b873735a5..f7b0b928cd53 100644
> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> @@ -2646,14 +2646,6 @@ static int dpaa_eth_probe(struct platform_device
> *pdev)
> priv->buf_layout[RX].priv_data_size = DPAA_RX_PRIV_DATA_SIZE; /* Rx
> */
> priv->buf_layout[TX].priv_data_size = DPAA_TX_PRIV_DATA_SIZE; /* Tx
> */
>
> - /* device used for DMA mapping */
> - set_dma_ops(dev, get_dma_ops(&pdev->dev));
> - err = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(40));
> - if (err) {
> - dev_err(dev, "dma_coerce_mask_and_coherent() failed\n");
> - goto dev_mask_failed;
> - }
> -
> /* bp init */
> for (i = 0; i < DPAA_BPS_NUM; i++) {
> int err;
> @@ -2665,7 +2657,8 @@ static int dpaa_eth_probe(struct platform_device
> *pdev)
> dpaa_bps[i]->raw_size = bpool_buffer_raw_size(i,
> DPAA_BPS_NUM);
> /* avoid runtime computations by keeping the usable size here
> */
> dpaa_bps[i]->size = dpaa_bp_size(dpaa_bps[i]->raw_size);
> - dpaa_bps[i]->dev = dev;
> + /* DMA operations are done on the platform-provided device */
> + dpaa_bps[i]->dev = dev->parent;
>
> err = dpaa_bp_alloc_pool(dpaa_bps[i]);
> if (err < 0) {
> --
> 2.9.0
Hi Arnd,
Thanks for looking into this, I've tested your fix, it seems to need more work:
[0.894968] platform: DMA map failed
[0.898627] platform: DMA map failed
[0.902288] platform: DMA map failed
[0.905947] platform: DMA map failed
[0.909606] platform: DMA map failed
[0.913265] platform: DMA map failed
You also missed this related change:
@@ -2806,7 +2799,6 @@ static int dpaa_eth_probe(struct platform_device *pdev)
dpaa_bps_free(priv);
bp_create_failed:
fq_probe_failed:
-dev_mask_failed:
mac_probe_failed:
dev_set_drvdata(dev, NULL);
free_netdev(net_dev);
I'll try to address your concern about performing the DMA mapping on a different
device than the one set up by the platform code.
Regards,
Madalin
RE: [PATCH NET V6 1/2] net: phy: Add phy loopback support in net phy framework
> -Original Message-
> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
> On Behalf Of Lin Yun Sheng
> Sent: Tuesday, June 27, 2017 2:01 PM
> To: da...@davemloft.net; and...@lunn.ch; f.faine...@gmail.com
> Cc: huangda...@hisilicon.com; xuw...@hisilicon.com;
> liguo...@hisilicon.com; yisen.zhu...@huawei.com;
> gabriele.paol...@huawei.com; john.ga...@huawei.com; linux...@huawei.com;
> yisen.zhu...@huawei.com; salil.me...@huawei.com; lipeng...@huawei.com;
> trem...@gmail.com; netdev@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: [PATCH NET V6 1/2] net: phy: Add phy loopback support in net phy
> framework
>
> This patch add set_loopback in phy_driver, which is used by Mac
> driver to enable or disable a phy. it also add a generic
> genphy_loopback function, which use BMCR loopback bit to enable
> or disable a phy.
"disable a phy" or disable the PHY loopback function?
>
> Signed-off-by: Lin Yun Sheng
> ---
> drivers/net/phy/marvell.c| 1 +
> drivers/net/phy/phy_device.c | 51
>
> include/linux/phy.h | 5 +
> 3 files changed, 57 insertions(+)
>
> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> index 57297ba..01a1586 100644
> --- a/drivers/net/phy/marvell.c
> +++ b/drivers/net/phy/marvell.c
> @@ -2094,6 +2094,7 @@ static int m88e1510_probe(struct phy_device *phydev)
> .get_sset_count = marvell_get_sset_count,
> .get_strings = marvell_get_strings,
> .get_stats = marvell_get_stats,
> + .set_loopback = genphy_loopback,
> },
> {
> .phy_id = MARVELL_PHY_ID_88E1540,
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 1219eea..1e08d62 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1123,6 +1123,39 @@ int phy_resume(struct phy_device *phydev)
> }
> EXPORT_SYMBOL(phy_resume);
>
> +int phy_loopback(struct phy_device *phydev, bool enable)
> +{
> + struct phy_driver *phydrv = to_phy_driver(phydev->mdio.dev.driver);
> + int ret = 0;
> +
> + mutex_lock(&phydev->lock);
> +
> + if (enable && phydev->loopback_enabled) {
> + ret = -EBUSY;
> + goto out;
> + }
> +
> + if (!enable && !phydev->loopback_enabled) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
if (enable == phydev->loopback_enabled)
> + if (phydev->drv && phydrv->set_loopback)
> + ret = phydrv->set_loopback(phydev, enable);
> + else
> + ret = -EOPNOTSUPP;
> +
> + if (ret)
> + goto out;
> +
> + phydev->loopback_enabled = enable;
> +
> +out:
> + mutex_unlock(&phydev->lock);
> + return ret;
> +}
> +EXPORT_SYMBOL(phy_loopback);
> +
> /* Generic PHY support and helper functions */
>
> /**
> @@ -1628,6 +1661,23 @@ static int gen10g_resume(struct phy_device *phydev)
> return 0;
> }
>
> +int genphy_loopback(struct phy_device *phydev, bool enable)
> +{
> + int value;
> +
> + value = phy_read(phydev, MII_BMCR);
> + if (value < 0)
> + return value;
> +
> + if (enable)
> + value |= BMCR_LOOPBACK;
> + else
> + value &= ~BMCR_LOOPBACK;
> +
> + return phy_write(phydev, MII_BMCR, value);
> +}
> +EXPORT_SYMBOL(genphy_loopback);
> +
> static int __set_phy_supported(struct phy_device *phydev, u32 max_speed)
> {
> /* The default values for phydev->supported are provided by the PHY
> @@ -1874,6 +1924,7 @@ void phy_drivers_unregister(struct phy_driver *drv,
> int n)
> .read_status= genphy_read_status,
> .suspend= genphy_suspend,
> .resume = genphy_resume,
> + .set_loopback = genphy_loopback,
> }, {
> .phy_id = 0x,
> .phy_id_mask= 0x,
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index e76e4ad..49c903dc 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -364,6 +364,7 @@ struct phy_c45_device_ids {
> * is_pseudo_fixed_link: Set to true if this phy is an Ethernet switch,
> etc.
> * has_fixups: Set to true if this phy has fixups/quirks.
> * suspended: Set to true if this phy has been suspended successfully.
> + * loopback_enabled: Set true if this phy has been loopbacked
> successfully.
> * state: state of the PHY for management purposes
> * dev_flags: Device-specific flags used by the PHY driver.
> * link_timeout: The number of timer firings to wait before the
> @@ -400,6 +401,7 @@ struct phy_device {
> bool is_pseudo_fixed_link;
> bool has_fixups;
> bool suspended;
> + bool loopback_enabled;
>
> enum phy_state state;
>
> @@ -639,6 +641,7 @@ struct phy_driver {
> int (*set_tunable)(struct phy_device *dev,
> struct ethtool_tunable *tuna,
> const void *data);
> + int (*set_loopback)(struct
RE: [PATCH 1/2] fsl/fman: propagate dma_ops
> -Original Message-
> From: geert.uytterhoe...@gmail.com [mailto:geert.uytterhoe...@gmail.com]
> On Behalf Of Geert Uytterhoeven
> Sent: Monday, June 26, 2017 7:24 PM
> To: Madalin-cristian Bucur
> Cc: netdev@vger.kernel.org; David S. Miller ;
> linuxppc-...@lists.ozlabs.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH 1/2] fsl/fman: propagate dma_ops
>
> Hi Madalin,
>
> On Mon, Jun 26, 2017 at 4:55 PM, Madalin-cristian Bucur
> wrote:
> >> -Original Message-
> >> From: geert.uytterhoe...@gmail.com
> [mailto:geert.uytterhoe...@gmail.com]
> >> On Behalf Of Geert Uytterhoeven
> >> Sent: Monday, June 26, 2017 10:49 AM
> >> To: Madalin-cristian Bucur
> >> Cc: netdev@vger.kernel.org; David S. Miller ;
> >> linuxppc-...@lists.ozlabs.org; linux-ker...@vger.kernel.org
> >> Subject: Re: [PATCH 1/2] fsl/fman: propagate dma_ops
> >>
> > On Mon, Jun 19, 2017 at 5:04 PM, Madalin Bucur
> >> wrote:
> >> > Make sure dma_ops are set, to be later used by the Ethernet driver.
> >> >
> >> > Signed-off-by: Madalin Bucur
> >> > ---
> >> > drivers/net/ethernet/freescale/fman/mac.c | 2 ++
> >> > 1 file changed, 2 insertions(+)
> >> >
> >> > diff --git a/drivers/net/ethernet/freescale/fman/mac.c
> >> b/drivers/net/ethernet/freescale/fman/mac.c
> >> > index 0b31f85..6e67d22f 100644
> >> > --- a/drivers/net/ethernet/freescale/fman/mac.c
> >> > +++ b/drivers/net/ethernet/freescale/fman/mac.c
> >> > @@ -623,6 +623,8 @@ static struct platform_device
> >> *dpaa_eth_add_device(int fman_id,
> >> > goto no_mem;
> >> > }
> >> >
> >> > + set_dma_ops(&pdev->dev, get_dma_ops(priv->dev));
> >> > +
> >>
> >> When compile-testing with f NO_DMA=y:
> >>
> >> drivers/net/ethernet/freescale/fman/mac.c: In function
> >> ‘dpaa_eth_add_device’:
> >> drivers/net/ethernet/freescale/fman/mac.c:626: error: implicit
> >> declaration of function ‘set_dma_ops’
> >>
> >> Reverting commit 5567e989198b5a8d fixes this regression in v4.12-rc7.
> >>
> >> Why is this change needed?
> >> There's no single other call to the DMA API in this file?
> >
> > We're setting here the dma_ops that are later used in the other
> driver/patch.
> > The problem is we now depend upon DMA but do not explicitly declare it:
> >
> > < HAS_DMA'
> > in its Kconfig>>
>
> Sure. But only if the driver really uses DMA.
> I can stick a set_dma_ops() call in whatever driver, but that doesn't
> mean it will
> suddenly use DMA.
> Why does the FMan driver suddenly has a dependency on DMA, if it doesn't
> use DMA?
>
> > I'll need to add this to the FMan driver Kconfig.
>
> Why does the FMan driver need this?
> Why can't his call be done in the driver that uses the DMA APIO?
The DPAA Ethernet driver makes use of DMA ops. It used to get them from
an API call (arch_setup_dma_ops) that was not exported. The DPAA Ethernet
that makes use of the FMan devices does not get the dma_ops as it does not
probe neither as an OF platform device nor thorough ACPI. It probes as a
platform device based on information prepared by the FMan driver. What the
FMan change [1] does is supplement the information shared with the Ethernet
driver with the dma_ops that the FMan driver gets during OF probing. There
are no scenarios one can use the DPAA drivers with NO_DMA, as far as I know.
For general info on the DPAA drivers please refer to the documentation
found in Documentation/networking/dpaa.txt. For the probing of the Ethernet
driver see change [2] and dpaa_eth_add_device() in fsl/fman, dpaa_eth_probe()
in dpaa_eth.
[1] 5567e989198b5a8d fsl/fman: propagate dma_ops
[2] fb52728a9294d97d dpaa_eth: reuse the dma_ops provided by the FMan MAC device
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
>
> In personal conversations with technical people, I call myself a hacker.
> But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
Thanks,
Madalin
RE: [PATCH] fsl/fman: add dependency on HAS_DMA
> -Original Message-
> From: geert.uytterhoe...@gmail.com [mailto:geert.uytterhoe...@gmail.com]
> On Behalf Of Geert Uytterhoeven
> Sent: Monday, June 26, 2017 7:17 PM
> To: Fabio Estevam
> Cc: Madalin-cristian Bucur ;
> netdev@vger.kernel.org; David S. Miller ; linuxppc-
> d...@lists.ozlabs.org; linux-kernel
> Subject: Re: [PATCH] fsl/fman: add dependency on HAS_DMA
>
> On Mon, Jun 26, 2017 at 5:20 PM, Fabio Estevam wrote:
> > On Mon, Jun 26, 2017 at 12:12 PM, Madalin Bucur
> wrote:
> >> A previous commit inserted a dependency on DMA API that requires
> >> HAS_DMA to be added in Kconfig.
> >
> > It would be nice to specify the commit that caused this.
>
> That would be commit 5567e989198b5a8d ("fsl/fman: propagate dma_ops").
>
> However, none of the fman code uses any DMA API calls, so IMHO
> the set_dma_ops() should be done somewhere else.
The Ethernet driver is making use of the DMA ops set here.
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
>
> In personal conversations with technical people, I call myself a hacker.
> But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
RE: [PATCH] fsl/fman: add dependency on HAS_DMA
> -Original Message- > From: Fabio Estevam [mailto:feste...@gmail.com] > Sent: Monday, June 26, 2017 6:21 PM > To: Madalin-cristian Bucur > Cc: netdev@vger.kernel.org; David S. Miller ; Geert > Uytterhoeven ; linuxppc-...@lists.ozlabs.org; linux- > kernel > Subject: Re: [PATCH] fsl/fman: add dependency on HAS_DMA > > On Mon, Jun 26, 2017 at 12:12 PM, Madalin Bucur > wrote: > > A previous commit inserted a dependency on DMA API that requires > > HAS_DMA to be added in Kconfig. > > It would be nice to specify the commit that caused this. Sent v2, thanks. Madalin
RE: [PATCH 1/2] fsl/fman: propagate dma_ops
> -Original Message-
> From: geert.uytterhoe...@gmail.com [mailto:geert.uytterhoe...@gmail.com]
> On Behalf Of Geert Uytterhoeven
> Sent: Monday, June 26, 2017 10:49 AM
> To: Madalin-cristian Bucur
> Cc: netdev@vger.kernel.org; David S. Miller ;
> linuxppc-...@lists.ozlabs.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH 1/2] fsl/fman: propagate dma_ops
>
> Hi Madalin,
>
> On Mon, Jun 19, 2017 at 5:04 PM, Madalin Bucur
> wrote:
> > Make sure dma_ops are set, to be later used by the Ethernet driver.
> >
> > Signed-off-by: Madalin Bucur
> > ---
> > drivers/net/ethernet/freescale/fman/mac.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/freescale/fman/mac.c
> b/drivers/net/ethernet/freescale/fman/mac.c
> > index 0b31f85..6e67d22f 100644
> > --- a/drivers/net/ethernet/freescale/fman/mac.c
> > +++ b/drivers/net/ethernet/freescale/fman/mac.c
> > @@ -623,6 +623,8 @@ static struct platform_device
> *dpaa_eth_add_device(int fman_id,
> > goto no_mem;
> > }
> >
> > + set_dma_ops(&pdev->dev, get_dma_ops(priv->dev));
> > +
>
> When compile-testing with f NO_DMA=y:
>
> drivers/net/ethernet/freescale/fman/mac.c: In function
> ‘dpaa_eth_add_device’:
> drivers/net/ethernet/freescale/fman/mac.c:626: error: implicit
> declaration of function ‘set_dma_ops’
>
> Reverting commit 5567e989198b5a8d fixes this regression in v4.12-rc7.
>
> Why is this change needed?
> There's no single other call to the DMA API in this file?
We're setting here the dma_ops that are later used in the other driver/patch.
The problem is we now depend upon DMA but do not explicitly declare it:
<>
I'll need to add this to the FMan driver Kconfig.
> If it's really needed, can't set_dma_ops() be called from the driver that
> needs it, cfr. your other patch "[PATCH 2/2] dpaa_eth: reuse the dma_ops
> provided by the FMan MAC device"?
>
> Thanks!
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
>
> In personal conversations with technical people, I call myself a hacker.
> But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
Thanks,
Madalin
RE: [PATCH] dt-bindings: net: move FMan binding
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Sent: Monday, May 15, 2017 5:31 PM > Subject: Re: [PATCH] dt-bindings: net: move FMan binding > > From: Madalin Bucur > Date: Mon, 15 May 2017 16:36:38 +0300 > > > Besides the PPC SoCs, the QorIQ DPAA FMan is also present on ARM SoCs, > > moving the device tree binding document into the bindings/net folder. > > > > Signed-off-by: Madalin Bucur > > What tree is this targetted at for merging? I hope Rob Herring will take this into his tree, it's a device tree binding change. Thanks, Madalin
RE: [PATCH v3 1/2] net: phy: Fix PHY AN done state machine for interrupt driven PHYs
On March 27, 2017 2:59 PM, Roger Quadros wrote:
> The Ethernet link on an interrupt driven PHY was not coming up if the
> Ethernet cable was plugged before the Ethernet interface was brought up.
>
> The PHY state machine seems to be stuck from RUNNING to AN state
> with no new interrupts from the PHY. So it doesn't know when the
> PHY Auto-negotiation has been completed and doesn't transition to RUNNING
> state with ANEG done thus netif_carrier_on() is never called.
>
> NOTE: genphy_config_aneg() will not restart PHY Auto-negotiation of
> advertisement parameters didn't change.
>
> Fix this by scheduling the PHY state machine in phy_start_aneg().
> There is no way of knowing in phy.c whether auto-negotiation was
> restarted or not by the PHY driver so we just wait for the next
> poll/interrupt to update the PHY state machine.
>
> Fixes: 3c293f4e08b5 ("net: phy: Trigger state machine on state change and
> not polling.")
> Cc: stable # v4.9+
> Signed-off-by: Roger Quadros
> ---
> v3: Fix typo in commit message
>
> drivers/net/phy/phy.c | 4
> 1 file changed, 4 insertions(+)
Tested-by: Madalin Bucur
RE: [PATCH 0/9] QorIQ DPAA 1 updates
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Sent: Tuesday, February 21, 2017 8:20 PM > > From: Madalin Bucur > Date: Tue, 21 Feb 2017 13:52:45 +0200 > > > This patch set introduces a series of fixes and features to the DPAA 1 > > drivers. Besides activating hardware Rx checksum offloading, four > traffic > > classes are added for Tx traffic prioritisation. > > The net-next tree is closed, please resubmit this after the merge window > when > the net-next tree opens back up. > > Thanks. Sorry, I'll resend. Meanwhile I should start baking some cakes...
Fixed link for 10G
Hi Florian,
I'm trying to add a fixed-link property that declares 10G speed
for a XGMII PHY and I'm encountering some issues as the fixed
link infrastructure does not seem to support this speed.
I'm using this device tree snippet (using the legacy format, but it
should not matter):
ethernet@f2000 { /* 10GEC2 */
fixed-link = <0 1 1 0 0>;
phy-connection-type = "xgmii";
};
and I get this error:
[0.464238] swphy: unknown speed
[0.467464] fsl_mac: probe of 1af2000.ethernet failed with error -22
Looking at the code, fixed_phy_register() seems to check for speeds up
to 1G and swphy only caters 1G and lower speeds, the swphy_decode_speed()
returning -EINVAL for 10G, triggering the error printed above in
swphy_validate_state().
What would be the proper way to add support for the 10G fixed link speed?
Thank you,
Madalin
RE: [PATCH net] tcp: add a missing barrier in tcp_tasklet_func()
> -Original Message-
> From: Eric Dumazet [mailto:eric.duma...@gmail.com]
> Sent: Wednesday, December 21, 2016 3:43 PM
>
> Madalin reported crashes happening in tcp_tasklet_func() on powerpc64
>
> Before TSQ_QUEUED bit is cleared, we must ensure the changes done
> by list_del(&tp->tsq_node); are committed to memory, otherwise
> corruption might happen, as an other cpu could catch TSQ_QUEUED
> clearance too soon.
>
> We can notice that old kernels were immune to this bug, because
> TSQ_QUEUED was cleared after a bh_lock_sock(sk)/bh_unlock_sock(sk)
> section, but they could have missed a kick to write additional bytes,
> when NIC interrupts for a given flow are spread to multiple cpus.
>
> Affected TCP flows would need an incoming ACK or RTO timer to add more
> packets to the pipe. So overall situation should be better now.
>
> Fixes: b223feb9de2a ("tcp: tsq: add shortcut in tcp_tasklet_func()")
> Signed-off-by: Eric Dumazet
> Reported-by: Madalin Bucur
> Tested-by: Madalin Bucur
It's actually tested by Xing Lei:
Tested-by: Xing Lei
Thank you for the fast resolution.
> ---
> net/ipv4/tcp_output.c |1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index
> b45101f3d2bd2e0f0077305a061add4f7ea0de27..31a255b555ad86a3537c077862e3ea38
> f9b44284 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -769,6 +769,7 @@ static void tcp_tasklet_func(unsigned long data)
> list_del(&tp->tsq_node);
>
> sk = (struct sock *)tp;
> + smp_mb__before_atomic();
> clear_bit(TSQ_QUEUED, &sk->sk_tsq_flags);
>
> if (!sk->sk_lock.owned &&
>
RE: [upstream-release] [PATCH net v3 2/4] powerpc: fsl/fman: remove fsl, fman from of_device_ids[]
> From: Scott Wood [mailto:o...@buserror.net]
> Sent: Monday, December 19, 2016 9:46 PM
>
> On Mon, 2016-12-19 at 18:13 +0200, Madalin Bucur wrote:
> > The fsl/fman drivers will use of_platform_populate() on all
> > supported platforms. Call of_platform_populate() to probe the
> > FMan sub-nodes.
> >
> > Signed-off-by: Igal Liberman
> > Signed-off-by: Madalin Bucur
> > ---
> > arch/powerpc/platforms/85xx/corenet_generic.c | 3 ---
> > drivers/net/ethernet/freescale/fman/fman.c| 8
> > 2 files changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c
> > b/arch/powerpc/platforms/85xx/corenet_generic.c
> > index 1179115..824b7f1 100644
> > --- a/arch/powerpc/platforms/85xx/corenet_generic.c
> > +++ b/arch/powerpc/platforms/85xx/corenet_generic.c
> > @@ -117,9 +117,6 @@ static const struct of_device_id of_device_ids[] = {
> > {
> > .compatible = "fsl,qe",
> > },
> > - {
> > - .compatible= "fsl,fman",
> > - },
> > /* The following two are for the Freescale hypervisor */
> > {
> > .name = "hypervisor",
>
> For this part:
>
> Acked-by: Scott Wood
Thank you, added to v4.
> > diff --git a/drivers/net/ethernet/freescale/fman/fman.c
> > b/drivers/net/ethernet/freescale/fman/fman.c
> > index dafd9e1..0b7f711 100644
> > --- a/drivers/net/ethernet/freescale/fman/fman.c
> > +++ b/drivers/net/ethernet/freescale/fman/fman.c
> > @@ -2868,6 +2868,14 @@ static struct fman *read_dts_node(struct
> > platform_device *of_dev)
> >
> > fman->dev = &of_dev->dev;
> >
> > + /* call of_platform_populate in order to probe sub-nodes on arm64
> > */
> > + err = of_platform_populate(fm_node, NULL, NULL, &of_dev->dev);
> > + if (err) {
> > + dev_err(&of_dev->dev, "%s: of_platform_populate()
> > failed\n",
> > + __func__);
> > + goto fman_free;
> > + }
>
> The "on arm64" comment is no longer accurate (and the rest of the comment
> seems unnecessary).
>
> -Scott
Removed in v4.
RE: [PATCH net v2 2/5] powerpc: remove fsl,fman from of_device_ids[]
> From: David Miller [mailto:da...@davemloft.net] > Sent: Monday, December 19, 2016 5:37 PM > > From: Madalin Bucur > Date: Mon, 19 Dec 2016 11:22:20 +0200 > > > The fsl/fman drivers will use of_platform_populate() on all > > supported platforms. > > > > Signed-off-by: Madalin Bucur > > It seems that this creates a failure point between patches #2 and > #3. If the cases handled by this "fsl,fman" entry are only afterwards > covered by the of_platform_populate() code added in patch #3 then you > cannot split these two changes up like this. > > The two changes must be done at the same time, otherwise probing will > fail for some people between patches #2 and #3. You are right, that will happen. I was not convinced these need to be merged due to the different places they touch. I'll send a v3.
RE: [upstream-release] [PATCH net 2/4] fsl/fman: arm: call of_platform_populate() for arm64 platfrom
> From: Scott Wood
> Sent: Thursday, December 15, 2016 8:42 PM
>
> On 12/15/2016 07:11 AM, Madalin Bucur wrote:
> > From: Igal Liberman
> >
> > Signed-off-by: Igal Liberman
> > ---
> > drivers/net/ethernet/freescale/fman/fman.c | 10 ++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/freescale/fman/fman.c
> b/drivers/net/ethernet/freescale/fman/fman.c
> > index dafd9e1..f36b4eb 100644
> > --- a/drivers/net/ethernet/freescale/fman/fman.c
> > +++ b/drivers/net/ethernet/freescale/fman/fman.c
> > @@ -2868,6 +2868,16 @@ static struct fman *read_dts_node(struct
> platform_device *of_dev)
> >
> > fman->dev = &of_dev->dev;
> >
> > +#ifdef CONFIG_ARM64
> > + /* call of_platform_populate in order to probe sub-nodes on arm64 */
> > + err = of_platform_populate(fm_node, NULL, NULL, &of_dev->dev);
> > + if (err) {
> > + dev_err(&of_dev->dev, "%s: of_platform_populate() failed\n",
> > + __func__);
> > + goto fman_free;
> > + }
> > +#endif
>
> Should we remove fsl,fman from the PPC of_device_ids[], so this doesn't
> need an ifdef?
>
> Why is it #ifdef CONFIG_ARM64 rather than #ifndef CONFIG_PPC?
>
> -Scott
Igal was working on adding ARM64 support when this patch was created, thus the
choice of #ifdef CONFIG_ARM64. Unifying this for PPC and ARM64 by always calling
of_platform_populate() sounds like the best approach. I would need to
synchronize
the introduction of this code with the removal of the fsl,fman entry from the
of_device_ids[] array.
Dave, Michael, Scott, is it ok to add to v2 of this patch set the patch that
removes
the compatible "fsl,fman" from arch/powerpc/platforms/85xx/corenet_generic.c?
Thanks,
Madalin
RE: [PATCH net] phy: Don't increment MDIO bus refcount unless it's a different owner
> From: netdev-ow...@vger.kernel.org On Behalf Of Florian Fainelli
> Sent: Thursday, December 08, 2016 7:54 PM
> To: Johan Hovold
> On 12/08/2016 09:01 AM, Johan Hovold wrote:
> > On Thu, Dec 08, 2016 at 08:47:54AM -0800, Florian Fainelli wrote:
> >> On 12/08/2016 08:27 AM, Johan Hovold wrote:
> >>> On Tue, Dec 06, 2016 at 08:54:43PM -0800, Florian Fainelli wrote:
> Commit 3e3aaf649416 ("phy: fix mdiobus module safety") fixed the way
> we
> dealt with MDIO bus module reference count, but sort of introduced a
> regression in that, if an Ethernet driver registers its own MDIO bus
> driver, as is common, we will end up with the Ethernet driver's
> module->refnct set to 1, thus preventing this driver from any
> removal.
>
> Fix this by comparing the network device's device driver owner
> against
> the MDIO bus driver owner, and only if they are different, increment
> the
> MDIO bus module refcount.
>
> Fixes: 3e3aaf649416 ("phy: fix mdiobus module safety")
> Signed-off-by: Florian Fainelli
> ---
> Russell,
>
> I verified this against the ethoc driver primarily (on a TS7300
> board)
> and bcmgenet.
>
> Thanks!
>
> drivers/net/phy/phy_device.c | 16 +---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/phy/phy_device.c
> b/drivers/net/phy/phy_device.c
> index 1a4bf8acad78..c4ceb082e970 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -857,11 +857,17 @@ EXPORT_SYMBOL(phy_attached_print);
> int phy_attach_direct(struct net_device *dev, struct phy_device
> *phydev,
> u32 flags, phy_interface_t interface)
> {
> +struct module *ndev_owner = dev->dev.parent->driver->owner;
> >>>
> >>> Is this really safe? A driver does not need to set a parent device,
> and
> >>> in that case you get a NULL-deref here (I tried using cpsw).
> >>
> >> Humm, cpsw does call SET_NETDEV_DEV() which should take care of that,
> is
> >> the call made too late? Do you have an example oops?
> >
> > Sorry if I was being unclear, cpsw does set a parent device, but there
> > are network driver that do not. Perhaps such drivers will never hit this
> > code path, but I can't say for sure and everything appear to work for
> > cpsw if you comment out that SET_NETDEV_DEV (well, at least before this
> > patch).
>
> You were clear, I did not understand that you exercised this with cpsw
> to see whether this was safe in all conditions.
>
> >
> >> I don't mind safeguarding this with a check against dev->dev.parent,
> but
> >> I would like to fix the drivers where relevant too, since
> >> SET_NETDEV_DEV() should really be called, otherwise a number of things
> >> just don't work
> >
> > I grepped for for register_netdev and think I saw a number of drivers
> > which do not call SET_NETDEV_DEV.
> >
> > Again, perhaps they will never hit this path, but thought I should ask.
>
> You are absolutely right, this is a potential problem, so far I found
> two legitimate drivers that do not call SET_NETDEV_DEV (lantiq_etop.c
> and cpmac.c, both fixed), and Freescale's FMAN driver, which I have a
> hard time understanding what it does with mac_dev->net_dev...
>
> Thanks!
> --
> Florian
Hi Florian,
The Freescale DPAA Ethernet driver is in drivers/net/ethernet/freescale/dpaa:
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c:2501:SET_NETDEV_DEV(net_dev,
dev);
and it is making use of the MAC and ports driver in the FMan driver (and of the
QBMan drivers in drivers/soc/fsl/qbman but that's off topic). You need to look
at the net-next tree for this, the drivers were gradually added.
Madalin
RE: [PATCH net v2 4/5] net: fsl/fman: fix fixed-link-phydev reference leak
> -Original Message-
> From: Johan Hovold [mailto:jhov...@gmail.com] On Behalf Of Johan Hovold
> Sent: Thursday, November 24, 2016 8:22 PM
>
> Make sure to drop the reference taken by of_phy_find_device() when
> looking up a fixed-link phydev during probe.
>
> Fixes: 57ba4c9b56d8 ("fsl/fman: Add FMan MAC support")
> Signed-off-by: Johan Hovold
> ---
> drivers/net/ethernet/freescale/fman/mac.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ethernet/freescale/fman/mac.c
> b/drivers/net/ethernet/freescale/fman/mac.c
> index 8fe6b3e253fa..736db9d9b0ad 100644
> --- a/drivers/net/ethernet/freescale/fman/mac.c
> +++ b/drivers/net/ethernet/freescale/fman/mac.c
> @@ -892,6 +892,8 @@ static int mac_probe(struct platform_device *_of_dev)
> priv->fixed_link->duplex = phy->duplex;
> priv->fixed_link->pause = phy->pause;
> priv->fixed_link->asym_pause = phy->asym_pause;
> +
> + put_device(&phy->mdio.dev);
> }
>
> err = mac_dev->init(mac_dev);
> --
> 2.7.3
Thank you,
Madalin
RE: [patch] fsl/fman: fix a leak in tgec_free()
> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Thursday, November 24, 2016 1:21 PM
> To: Madalin-Cristian Bucur ; Igal Liberman
>
> Cc: netdev@vger.kernel.org; kernel-janit...@vger.kernel.org
> Subject: [patch] fsl/fman: fix a leak in tgec_free()
>
> We set "tgec->cfg" to NULL before passing it to kfree(). There is no
> need to set it to NULL at all. Let's just delete it.
Agree, thanks.
> Fixes: 57ba4c9b56d8 ("fsl/fman: Add FMan MAC support")
> Signed-off-by: Dan Carpenter
> ---
> I haven't tested this. It occurs to me that this code might be
> something to paper over a use after free bug by changing it to a leak
> instead.
>
> It applies to net-master.
>
> diff --git a/drivers/net/ethernet/freescale/fman/fman_tgec.c
> b/drivers/net/ethernet/freescale/fman/fman_tgec.c
> index efabb04..4b0f3a5 100644
> --- a/drivers/net/ethernet/freescale/fman/fman_tgec.c
> +++ b/drivers/net/ethernet/freescale/fman/fman_tgec.c
> @@ -722,9 +722,6 @@ int tgec_free(struct fman_mac *tgec)
> {
> free_init_resources(tgec);
>
> - if (tgec->cfg)
> - tgec->cfg = NULL;
> -
> kfree(tgec->cfg);
> kfree(tgec);
>
RE: [PATCH net-next v7 03/10] dpaa_eth: add option to use one buffer pool set
> From: David Miller [mailto:da...@davemloft.net] > Sent: Sunday, November 13, 2016 7:46 PM > > From: Madalin Bucur > Date: Fri, 11 Nov 2016 10:20:00 +0200 > > > @@ -8,3 +8,12 @@ menuconfig FSL_DPAA_ETH > > 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_ETH_COMMON_BPOOL > > + bool "Use a common buffer pool set for all the interfaces" > > + ---help--- > > + The DPAA Ethernet netdevices require buffer pools for storing the > buffers > > + used by the FMan hardware for reception. One can use a single > buffer pool > > + set for all interfaces or a dedicated buffer pool set for each > interface. > > +endif # FSL_DPAA_ETH > > This in no way belongs in Kconfig. If you want to support this, > support it wit a run time configuration choice via ethtool flags > or similar. Do not use debugfs, do not use sysfs, do not use > module options. > > If you put it in Kconfig, distributions will have to pick one way or > another which means that users who want the other choice lose. This > never works. I've introduced this Kconfig option as a backwards compatible option, to be able to run comparative tests between the independent buffer pool setup and the previous common buffer pool setup. There are not so many reasons to use the same buffer pool besides "having the old setup", the memory saving is marginal, in all other aspects the separate buffer pools setup fares better. I'll remove this patch from the next submission. Should anyone care for this I can add an entry to the feature backlog to add runtime support but it will be quite low in priority. Thank you for your review. Madalin
RE: [PATCH net-next v6 02/10] dpaa_eth: add support for DPAA Ethernet
> From: Madalin-Cristian Bucur > Sent: Monday, November 07, 2016 5:43 PM > > > From: David Miller [mailto:da...@davemloft.net] > > Sent: Thursday, November 03, 2016 9:58 PM > > > > From: Madalin Bucur > > Date: Wed, 2 Nov 2016 22:17:26 +0200 > > > > > This introduces the Freescale Data Path Acceleration Architecture > > > + int numstats = sizeof(struct rtnl_link_stats64) / sizeof(u64); > > ... > > > + cpustats = (u64 *)&percpu_priv->stats; > > > + > > > + for (j = 0; j < numstats; j++) > > > + netstats[j] += cpustats[j]; > > > > This is a memcpy() on well-typed datastructures which requires no > > casting or special handling whatsoever, so use memcpy instead of > > needlessly open coding the operation. > > Will fix. Took a second look at this, it's not copying but adding the percpu statistics into consolidated results. Madalin
RE: [PATCH net-next v6 02/10] dpaa_eth: add support for DPAA Ethernet
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Sent: Monday, November 07, 2016 5:55 PM > > From: Madalin-Cristian Bucur > Date: Mon, 7 Nov 2016 15:43:26 + > > >> From: David Miller [mailto:da...@davemloft.net] > >> Sent: Thursday, November 03, 2016 9:58 PM > >> > >> Why? By clearing this, you disallow an important fundamental way to do > >> performane testing, via pktgen. > > > > The Tx path in DPAA requires one to insert a back-pointer to the skb > into > > the Tx buffer. On the Tx confirmation path the back-pointer in the > buffer > > is used to release the skb. If Tx buffer is shared we'd alter the back- > pointer > > and leak/double free skbs. See also > > Then have your software state store an array of SKB pointers, one for each > TX ring entry, just like every other driver does. There is no Tx ring in DPAA. Frames are send out on QMan HW queues towards the FMan for Tx and then received back on Tx confirmation queues for cleanup. Array traversal would for sure cost more than using the back-pointer. Also, we can now process confirmations on a different core than the one doing Tx, we'd have to keep the arrays percpu and force the Tx conf on the same core. Or add locks. Madalin
RE: [PATCH net-next v6 02/10] dpaa_eth: add support for DPAA Ethernet
> From: David Miller [mailto:da...@davemloft.net] > Sent: Monday, November 07, 2016 6:40 PM > > From: Madalin-Cristian Bucur > Date: Mon, 7 Nov 2016 16:32:16 + > > >> From: David Miller [mailto:da...@davemloft.net] > >> Sent: Monday, November 07, 2016 5:55 PM > >> > >> From: Madalin-Cristian Bucur > >> Date: Mon, 7 Nov 2016 15:43:26 + > >> > >> >> From: David Miller [mailto:da...@davemloft.net] > >> >> Sent: Thursday, November 03, 2016 9:58 PM > >> >> > >> >> Why? By clearing this, you disallow an important fundamental way to > >> >> do performane testing, via pktgen. > >> > > >> > The Tx path in DPAA requires one to insert a back-pointer to the skb > >> > into > >> > the Tx buffer. On the Tx confirmation path the back-pointer in the > >> > buffer > >> > is used to release the skb. If Tx buffer is shared we'd alter the > >> > back-pointer > >> > and leak/double free skbs. See also > >> > >> Then have your software state store an array of SKB pointers, one for > each > >> TX ring entry, just like every other driver does. > > > > There is no Tx ring in DPAA. Frames are send out on QMan HW queues > > towards the FMan for Tx and then received back on Tx confirmation queues > > for cleanup. > > Array traversal would for sure cost more than using the back-pointer. > > Also, we can now process confirmations on a different core than the one > > doing Tx, > > we'd have to keep the arrays percpu and force the Tx conf on the same > > core. Or add locks. > > Report back an integer index, like every scsi driver out there which > completes tagged queued block I/O operations asynchronously. You can > associate the array with a specific TX confirmation queue. >From HW? It only gives you back the buffer start address (plus length, etc). "buff_2_skb()" needs to be solved in SW, expensively using array (lists? As the number of frames in flight can be large/variable) or cheaply with the back pointer. The back-pointer approach has its tradeoffs: no shared skbs, imposed non-zero needed_headroom.
RE: [PATCH net-next v6 02/10] dpaa_eth: add support for DPAA Ethernet
> From: David Miller [mailto:da...@davemloft.net]
> Sent: Thursday, November 03, 2016 9:58 PM
>
> From: Madalin Bucur
> Date: Wed, 2 Nov 2016 22:17:26 +0200
>
> > This introduces the Freescale Data Path Acceleration Architecture
> > +static inline size_t bpool_buffer_raw_size(u8 index, u8 cnt)
> > +{
> > + u8 i;
> > + size_t res = DPAA_BP_RAW_SIZE / 2;
>
> Always order local variable declarations from longest to shortest line,
> also know as Reverse Christmas Tree Format.
>
> Please audit your entire submission for this problem, it occurs
> everywhere.
Thank you, I'll resolve this.
> > + /* we do not want shared skbs on TX */
> > + net_dev->priv_flags &= ~IFF_TX_SKB_SHARING;
>
> Why? By clearing this, you disallow an important fundamental way to do
> performane testing, via pktgen.
The Tx path in DPAA requires one to insert a back-pointer to the skb into
the Tx buffer. On the Tx confirmation path the back-pointer in the buffer
is used to release the skb. If Tx buffer is shared we'd alter the back-pointer
and leak/double free skbs. See also
static int dpaa_start_xmit(struct sk_buff *skb, struct net_device
*net_dev)
{
...
if (!nonlinear) {
/* We're going to store the skb backpointer at the
beginning
* of the data buffer, so we need a privately owned skb
*
* We've made sure skb is not shared in dev->priv_flags,
* we need to verify the skb head is not cloned
*/
if (skb_cow_head(skb, priv->tx_headroom))
goto enomem;
WARN_ON(skb_is_nonlinear(skb));
}
...
> > + int numstats = sizeof(struct rtnl_link_stats64) / sizeof(u64);
> ...
> > + cpustats = (u64 *)&percpu_priv->stats;
> > +
> > + for (j = 0; j < numstats; j++)
> > + netstats[j] += cpustats[j];
>
> This is a memcpy() on well-typed datastructures which requires no
> casting or special handling whatsoever, so use memcpy instead of
> needlessly open coding the operation.
Will fix.
> > +static int dpaa_change_mtu(struct net_device *net_dev, int new_mtu)
> > +{
> > + const int max_mtu = dpaa_get_max_mtu();
> > +
> > + /* Make sure we don't exceed the Ethernet controller's MAXFRM */
> > + if (new_mtu < 68 || new_mtu > max_mtu) {
> > + netdev_err(net_dev, "Invalid L3 mtu %d (must be between %d and
> %d).\n",
> > + new_mtu, 68, max_mtu);
> > + return -EINVAL;
> > + }
> > + net_dev->mtu = new_mtu;
> > +
> > + return 0;
> > +}
>
> MTU restrictions are handled in the net-next tree via net_dev->min_mtu and
> net_dev->max_mtu. Use that and do not define this NDO operation as you do
> not need it.
OK
> > +static int dpaa_set_features(struct net_device *dev, netdev_features_t
> features)
> > +{
> > + /* Not much to do here for now */
> > + dev->features = features;
> > + return 0;
> > +}
>
> Do not define unnecessary NDO operations, let the defaults do their job.
>
> > +static netdev_features_t dpaa_fix_features(struct net_device *dev,
> > + netdev_features_t features)
> > +{
> > + netdev_features_t unsupported_features = 0;
> > +
> > + /* In theory we should never be requested to enable features that
> > +* we didn't set in netdev->features and netdev->hw_features at
> probe
> > +* time, but double check just to be on the safe side.
> > +*/
> > + unsupported_features |= NETIF_F_RXCSUM;
> > +
> > + features &= ~unsupported_features;
> > +
> > + return features;
> > +}
>
> Unless you can show that your need this, do not "guess" by implement this
> NDO operation. You don't need it.
Will remove it.
> > +#ifdef CONFIG_FSL_DPAA_ETH_FRIENDLY_IF_NAME
> > +static int dpaa_mac_hw_index_get(struct platform_device *pdev)
> > +{
> > + struct device *dpaa_dev;
> > + struct dpaa_eth_data *eth_data;
> > +
> > + dpaa_dev = &pdev->dev;
> > + eth_data = dpaa_dev->platform_data;
> > +
> > + return eth_data->mac_hw_id;
> > +}
> > +
> > +static int dpaa_mac_fman_index_get(struct platform_device *pdev)
> > +{
> > + struct device *dpaa_dev;
> > + struct dpaa_eth_data *eth_data;
> > +
> > + dpaa_dev = &pdev->dev;
> > + eth_data = dpaa_dev->platform_data;
> > +
> > + return eth_data->fman_hw_id;
> > +}
> > +#endif
>
> Do not play network device naming games like this, use the standard name
> assignment done by the kernel and have userspace entities do geographic or
> device type specific naming.
>
> I want to see this code completely removed.
I'll remove the option, udev rules like these can achieve the same effect:
SUBSYSTEM=="net", DRIVERS=="fsl_dpa*", ATTR{device_addr}=="ffe4e",
NAME="fm1-mac1"
SUBSYSTEM=="net", DRIVERS=="fsl_dpa*", ATTR{device_addr}=="ffe4e2000",
NAME="fm1-mac2"
> > +static int dpaa_set_mac_address(s
RE: [net-next 08/13] fsl/fman: check pcsphy pointer before use
> -Original Message-
> From: David Laight [mailto:david.lai...@aculab.com]
> Sent: Tuesday, October 04, 2016 5:44 PM
> To: Madalin-Cristian Bucur ;
> netdev@vger.kernel.org
> Cc: linuxdev.baldr...@gmail.com; linuxppc-...@lists.ozlabs.org;
> da...@davemloft.net; linux-ker...@vger.kernel.org
> Subject: RE: [net-next 08/13] fsl/fman: check pcsphy pointer before use
>
> From: Madalin Bucur
> > Sent: 04 October 2016 08:33
> > Subject: [net-next 08/13] fsl/fman: check pcsphy pointer before use
> ..
> > --- a/drivers/net/ethernet/freescale/fman/fman_memac.c
> > +++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
> > @@ -507,6 +507,9 @@ static void setup_sgmii_internal_phy(struct
> fman_mac *memac,
> > {
> > u16 tmp_reg16;
> >
> > + if (WARN_ON(!memac->pcsphy))
> > + return;
> > +
>
> Why?
>
> Either it can validly be NULL in which case you don't want the message.
> Or it shouldn't be NULL in which case you need to find and fix the bug.
> The later kernel OOPS will make the bug much easier to find.
>
> David
You can get into that situation if you have a broken device tree, state into
which someone may get while trying to create the device tree for a new
board. Avoiding a crash here allows the user to look at the device trees as
seen by the kernel / add some debug code if needed.
Madalin
RE: [v9, 6/6] fsl/fman: Add FMan MAC driver
> -Original Message-
> From: David Miller [mailto:da...@davemloft.net]
>
> From: Scott Wood
> Date: Tue, 8 Dec 2015 16:44:38 -0600
>
> > On Tue, 2015-12-08 at 14:18 -0600, Andy Fleming wrote:
> >> On Thu, Dec 3, 2015 at 1:19 AM, wrote:
> >> > From: Igal Liberman
> >> >
> >> > This patch adds the Ethernet MAC driver supporting the three
> >> > different types of MACs: dTSEC, tGEC and mEMAC.
> >> >
> >> > Signed-off-by: Igal Liberman
> >>
> >> [...]
> >>
> >> > +
> >> > +MODULE_LICENSE("Dual BSD/GPL");
> >> > +
> >> > +MODULE_AUTHOR("Emil Medve ");
> >>
> >> I imagine this email address doesn't exist anymore, or won't soon.
> >> This is also an issue in the ethernet driver (with my old address).
> >> I'm not sure what the right approach is, but we shouldn't be putting
> >> obsolete email addresses in the kernel.
> >
> > I don't think a MODULE_AUTHOR tag makes sense for drivers like this that
> had a
> > lot of people work on them. git history is better for giving credit in
> such
> > cases.
>
> Agreed.
I've already removed the MODULE_AUTHOR tag from the dpaa_eth code, this will
be removed as well. We are required to keep the Signed-off by: tags of the
original authors for the patches and also were instructed to keep the original
Freescale email addresses even if the patch authors have different email
addresses now. To complicate things further, after the Freescale - NXP merger
the freescale.com employee emails will be replaced by nxp.com email addresses
so our future submissions will have a mix of legacy freescale.com emails for
the former employees and nxp.com for the current ones. Given that the Freescale
brand is replaced with NXP, we may need to reflect this in the folder structure,
prefixes used in the code, etc. Copyright will also need to reflect this change,
any further changes will be attributed to NXP. Is there a norm/custom followed
in such situations?
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: [net-next v5 0/8] dpaa_eth: Add the Freescale DPAA Ethernet driver
Hi Timur, I've managed somehow to make got send-email to move the From: line in the body instead of the header, probably typed something wrong when asked to confirm the sender. I've resent the series. Regards, Madalin From: Timur Tabi Sent: Saturday, December 5, 2015 6:40:11 AM To: Bucur Madalin-Cristian-B32716 Cc: netdev@vger.kernel.org; linuxppc-...@lists.ozlabs.org; lkml; David Miller; Wood Scott-B07421; Liberman Igal-B31950; p...@mindchasers.com; Joe Perches; pebo...@tiscali.nl; Joakim Tjernlund; Greg Kroah-Hartman Subject: Re: [net-next v5 0/8] dpaa_eth: Add the Freescale DPAA Ethernet driver On Thu, Dec 3, 2015 at 6:08 AM, <> wrote: > From: Madalin Bucur > > This patch series adds the Ethernet driver for the Freescale > QorIQ Data Path Acceleration Architecture (DPAA). Please fix your git-send-email configuration, so that your emails are formatted properly. This is the From: header: From: <> -- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. -- 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: [net-next v4 4/8] dpaa_eth: add driver's Tx queue selection
> -Original Message-
> From: Wood Scott-B07421
> Sent: Wednesday, December 02, 2015 11:41 PM
>
> On Mon, 2015-11-02 at 19:31 +0200, Madalin Bucur wrote:
> > Allow the selection of the transmission queue based on the CPU id.
>
> Explain why.
I'll add more details in the commit log. This is a customer generated
feature. Bypassing the standard XPS can increase performance by making use
of the DPAA HW particularities.
> >
> > Signed-off-by: Madalin Bucur
> > ---
> > drivers/net/ethernet/freescale/dpaa/Kconfig | 10 ++
> > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c| 3 +++
> > drivers/net/ethernet/freescale/dpaa/dpaa_eth.h| 6 ++
> > drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c | 8
> > drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h | 4
> > 5 files changed, 31 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa/Kconfig
> > b/drivers/net/ethernet/freescale/dpaa/Kconfig
> > index 022d5aa..2577aac 100644
> > --- a/drivers/net/ethernet/freescale/dpaa/Kconfig
> > +++ b/drivers/net/ethernet/freescale/dpaa/Kconfig
> > @@ -11,6 +11,16 @@ menuconfig FSL_DPAA_ETH
> >
> > if FSL_DPAA_ETH
> >
> > +config FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE
> > + bool "Use driver's Tx queue selection mechanism"
> > + default y
> > + ---help---
> > + The DPAA Ethernet driver defines a ndo_select_queue() callback
> > for optimal selection
> > + of the egress FQ. That will override the XPS support for this
> > netdevice.
> > + If for whatever reason you want to be in control of the egress FQ
> > -to-CPU selection and mapping,
> > + or simply don't want to use the driver's ndo_select_queue()
> > callback, then unselect this
> > + and use the standard XPS support instead.
>
> Is there a use case for needing this to be configurable?
If the standard XPS is desired, the Kconfig option allows the driver user to
select that.
> > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > index 31d55b4..894f1a7 100644
> > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > @@ -390,6 +390,9 @@ static const struct net_device_ops dpa_private_ops =
> {
> > .ndo_get_stats64 = dpa_get_stats64,
> > .ndo_set_mac_address = dpa_set_mac_address,
> > .ndo_validate_addr = eth_validate_addr,
> > +#ifdef CONFIG_FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE
> > + .ndo_select_queue = dpa_select_queue,
> > +#endif
> > .ndo_change_mtu = dpa_change_mtu,
> > .ndo_set_rx_mode = dpa_set_rx_mode,
> > .ndo_init = dpa_ndo_init,
> > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
> > b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
> > index 1ba6617..87577cf 100644
> > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
> > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
> > @@ -420,9 +420,15 @@ static inline void _dpa_assign_wq(struct dpa_fq
> *fq)
> > }
> > }
> >
> > +#ifdef CONFIG_FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE
> > +/* Use in lieu of skb_get_queue_mapping() */
> > +#define dpa_get_queue_mapping(skb) \
> > + raw_smp_processor_id()
> > +#else
> > /* Use the queue selected by XPS */
> > #define dpa_get_queue_mapping(skb) \
> > skb_get_queue_mapping(skb)
> > +#endif
>
> Why is this necessary? Shouldn't providing a custom .ndo_select_queue()
> be
> sufficient to ensure that skb_get_queue_mapping() returns the same thing?
dpa_get_queue_mapping() is used in more than one place, the ndo function cannot
be used directly in all places, the current setup is justified.
> And if this goes away, it's just a matter of a function pointer, so if it
> does
> need to be configurable it could be a runtime option.
>
> -Scott
It's used on the hot path, adding an extra indirection layer to make it
selectable
at runtime would defeat the purpose...
Madalin
RE: [net-next v4 2/8] dpaa_eth: add support for DPAA Ethernet
> -Original Message-
> From: Joakim Tjernlund [mailto:joakim.tjernl...@transmode.se]
>
> On Tue, 2015-11-03 at 09:37 +0000, Madalin-Cristian Bucur wrote:
> > > -Original Message-
> > > From: Joakim Tjernlund [mailto:joakim.tjernl...@transmode.se]
> > >
> > > On Mon, 2015-11-02 at 19:31 +0200, Madalin Bucur wrote:
> > > > + if (unlikely(fd_status & FM_FD_STAT_RX_ERRORS) != 0) {
> > > > + if (net_ratelimit())
> > > > + netif_warn(priv, hw, net_dev, "FD status =
> > > 0x%08x\n",
> > > > + fd_status &
> FM_FD_STAT_RX_ERRORS);
> > > > +
> > > > + percpu_stats->rx_errors++;
> > > > + goto _release_frame;
> > > > + }
> > >
> > > I cannot find any detailed error accounting(maybe I am not looking
> hard
> > > enough) but I
> > > would appreciate if both TX and RX errors where better
> > > accounted(rx_length_errors, rx_frame_errors,
> > > rx_crc_errors, rx_fifo_errors etc.). This has helped me many times in
> the
> > > past diagnosing
> > > board HW problems.
> > >
> > > Jocke
> >
> > Hi Jocke,
> >
> > There are some error counters exported through ethtool (used to be
> debugfs).
> > FMan HW provides more debug information than we currently export, that
> will be
> > improved in the future but given the current priority of having a
> codebase as
> > small and reviewable as possible we had to drop some things from the
> initial
> > submission.
>
> I know, but ethtool is not always available.
> Even the old fec_main.c has it:
> if (status & (BD_ENET_RX_LG | BD_ENET_RX_SH | BD_ENET_RX_NO |
> BD_ENET_RX_CR | BD_ENET_RX_OV)) {
> ndev->stats.rx_errors++;
> if (status & (BD_ENET_RX_LG | BD_ENET_RX_SH)) {
> /* Frame too long or too short. */
> ndev->stats.rx_length_errors++;
> }
> if (status & BD_ENET_RX_NO) /* Frame alignment */
> ndev->stats.rx_frame_errors++;
> if (status & BD_ENET_RX_CR) /* CRC Error */
> ndev->stats.rx_crc_errors++;
> if (status & BD_ENET_RX_OV) /* FIFO overrun */
> ndev->stats.rx_fifo_errors++;
> }
> so it is just a few more lines ... Pretty please ? :)
>
> Jocke
It may be more that just a few lines to add complete debug details.
Your request is noted and will be among the first features to work on
after the driver is accepted upstream.
Thanks,
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: [net-next v4 3/8] dpaa_eth: add support for S/G frames
> -Original Message-
> From: Joe Perches [mailto:j...@perches.com]
>
> On Mon, 2015-11-02 at 19:31 +0200, Madalin Bucur wrote:
> > Add support for Scater/Gather (S/G) frames. The FMan can place
> > the frame content into multiple buffers and provide a S/G Table
> > (SGT) into one first buffer with references to the others.
>
> trivia: scatter
>
> > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c
> []
> > @@ -1177,10 +1177,42 @@ void dpaa_eth_init_ports(struct mac_device
> *mac_dev,
> > port_fqs->rx_defq, &buf_layout[RX]);
> > }
> >
> > +void dpa_release_sgt(struct qm_sg_entry *sgt)
> > +{
> > + struct dpa_bp *dpa_bp;
> > + struct bm_buffer bmb[DPA_BUFF_RELEASE_MAX];
>
> Where is "struct bm_buffer" defined?
>
Thank you, I'll address this and the other observations you have sent.
The struct bm_buffer is defined in the Buffer Manager driver header file,
in include/soc/fsl/bman.h.
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: [net-next v4 2/8] dpaa_eth: add support for DPAA Ethernet
> -Original Message-
> From: Joakim Tjernlund [mailto:joakim.tjernl...@transmode.se]
>
> On Mon, 2015-11-02 at 19:31 +0200, Madalin Bucur wrote:
> > + if (unlikely(fd_status & FM_FD_STAT_RX_ERRORS) != 0) {
> > + if (net_ratelimit())
> > + netif_warn(priv, hw, net_dev, "FD status =
> 0x%08x\n",
> > + fd_status & FM_FD_STAT_RX_ERRORS);
> > +
> > + percpu_stats->rx_errors++;
> > + goto _release_frame;
> > + }
>
> I cannot find any detailed error accounting(maybe I am not looking hard
> enough) but I
> would appreciate if both TX and RX errors where better
> accounted(rx_length_errors, rx_frame_errors,
> rx_crc_errors, rx_fifo_errors etc.). This has helped me many times in the
> past diagnosing
> board HW problems.
>
> Jocke
Hi Jocke,
There are some error counters exported through ethtool (used to be debugfs).
FMan HW provides more debug information than we currently export, that will be
improved in the future but given the current priority of having a codebase as
small and reviewable as possible we had to drop some things from the initial
submission.
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: [v3 1/8] devres: add devm_alloc_percpu()
> -Original Message- > From: Wood Scott-B07421 > Sent: Friday, October 02, 2015 4:01 AM > > On Thu, Sep 24, 2015 at 06:00:12PM +0300, Madalin Bucur wrote: > > Introduce managed counterparts for alloc_percpu() and free_percpu(). > > Add devm_alloc_percpu() and devm_free_percpu() into the managed > > interfaces list. > > > > Signed-off-by: Madalin Bucur > > Tested-by: Madalin-Cristian Bucur > > --- > > Documentation/driver-model/devres.txt | 4 +++ > > drivers/base/devres.c | 64 > +++ > > include/linux/device.h| 19 +++ > > 3 files changed, 87 insertions(+) > > Greg KH needs to be CCed on any drivers/base changes. > > -Scott Thank you, I will add him. 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: [net-next:master 1512/1524] net/ipv4/af_inet.c:1486:26: error: 'offt' undeclared
> -Original Message-
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> Subject: Re: [net-next:master 1512/1524] net/ipv4/af_inet.c:1486:26: error:
> 'offt' undeclared
>
> From: kbuild test robot
> Date: Mon, 31 Aug 2015 13:06:10 +0800
>
> >net/ipv4/af_inet.c: In function 'snmp_get_cpu_field64':
> >>> net/ipv4/af_inet.c:1486:26: error: 'offt' undeclared (first use in this
> function)
> > v = *(((u64 *)bhptr) + offt);
> > ^
> >net/ipv4/af_inet.c:1486:26: note: each undeclared identifier is reported
> only once for each function it appears in
> >net/ipv4/af_inet.c: In function 'snmp_fold_field64':
> >>> net/ipv4/af_inet.c:1499:39: error: 'offct' undeclared (first use in this
> function)
> > res += snmp_get_cpu_field(mib, cpu, offct, syncp_offset);
> > ^
> >>> net/ipv4/af_inet.c:1499:10: error: too many arguments to function
> 'snmp_get_cpu_field'
> > res += snmp_get_cpu_field(mib, cpu, offct, syncp_offset);
> > ^
> >net/ipv4/af_inet.c:1455:5: note: declared here
> > u64 snmp_get_cpu_field(void __percpu *mib, int cpu, int offt)
> > ^
> >net/ipv4/af_inet.c:1499: confused by earlier errors, bailing out
>
> Thanks, this should fix it:
>
>
> [PATCH] ipv4: Fix 32-bit build.
>
>net/ipv4/af_inet.c: In function 'snmp_get_cpu_field64':
> >> net/ipv4/af_inet.c:1486:26: error: 'offt' undeclared (first use in this
> function)
> v = *(((u64 *)bhptr) + offt);
> ^
>net/ipv4/af_inet.c:1486:26: note: each undeclared identifier is reported
> only once for each function it appears in
>net/ipv4/af_inet.c: In function 'snmp_fold_field64':
> >> net/ipv4/af_inet.c:1499:39: error: 'offct' undeclared (first use in this
> function)
> res += snmp_get_cpu_field(mib, cpu, offct, syncp_offset);
> ^
> >> net/ipv4/af_inet.c:1499:10: error: too many arguments to function
> 'snmp_get_cpu_field'
> res += snmp_get_cpu_field(mib, cpu, offct, syncp_offset);
> ^
>net/ipv4/af_inet.c:1455:5: note: declared here
> u64 snmp_get_cpu_field(void __percpu *mib, int cpu, int offt)
> ^
>
> Reported-by: kbuild test robot
> Signed-off-by: David S. Miller
> ---
> net/ipv4/af_inet.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 0c69c0b..c2d0ebc 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1471,7 +1471,7 @@ EXPORT_SYMBOL_GPL(snmp_fold_field);
>
> #if BITS_PER_LONG==32
>
> -u64 snmp_get_cpu_field64(void __percpu *mib, int cpu, int offct,
> +u64 snmp_get_cpu_field64(void __percpu *mib, int cpu, int offt,
>size_t syncp_offset)
> {
> void *bhptr;
> @@ -1496,7 +1496,7 @@ u64 snmp_fold_field64(void __percpu *mib, int
> offt, size_t syncp_offset)
> int cpu;
>
> for_each_possible_cpu(cpu) {
> - res += snmp_get_cpu_field(mib, cpu, offct, syncp_offset);
> + res += snmp_get_cpu_field(mib, cpu, offt, syncp_offset);
> }
> return res;
> }
> --
> 2.1.0
>
> --
Hi,
shouldn't that be snmp_get_cpu_field64() ?
- res += snmp_get_cpu_field(mib, cpu, offct, syncp_offset);
+ res += snmp_get_cpu_field64(mib, cpu, offt, syncp_offset);
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: [v2] net: phy: fixed: propagate fixed link values to struct
> -Original Message- > From: Stas Sergeev [mailto:s...@list.ru] > Sent: Wednesday, August 26, 2015 6:51 PM > To: Bucur Madalin-Cristian-B32716 ; > f.faine...@gmail.com > Cc: netdev@vger.kernel.org; linux-ker...@vger.kernel.org; Liberman Igal- > B31950 > Subject: Re: [v2] net: phy: fixed: propagate fixed link values to struct > > 26.08.2015 17:58, Madalin Bucur пишет: > > The fixed link values parsed from the device tree are stored in > > the struct fixed_phy member status. The struct phy_device members > > speed, duplex were not updated. > > ACK, but IMHO it will make more sense if you include that > into your upcoming patch set rather than sending separately, > as otherwise there is simply no in-kernel users of that new > functionality (all the current users likely do not access > these fields as early as you want to, so they don't care). > In any case, the patch looks good to me and the policy is > up to others. Given that it's more of a fix than a feature, I think it can be picked up separate from a certain driver that accesses those fields early but I guess Florian, David will decide this. Thanks, Madalin
RE: [PATCH 0/2] of: fsl/fman: reuse the fixed node parsing code
> -Original Message- > From: Stas Sergeev [mailto:s...@list.ru] > But have you looked into the patch I pointed previously? > https://lkml.org/lkml/2015/7/20/711 > You code will likely clash with it because my patch extends > of_phy_register_fixed_link(). > I admin I failed to grasp the details of your change - the lack of ample context Lines makes it a bit difficult. I'm sure your change could be merged then the of parsing could be separated from the actual fixed_phy_register() call if anyone cares about that. > > Then I could call only of_* functions but the end result would be the same > > and of_phy_register_fixed_phy() would not justify its existence that > > much... > You didn't say you wanted to obsolete the of_phy_register_fixed_phy(). > Since it is there (and even changed by me in a way your > patch will likely clash), IMHO it would be better if it is used, > rather than copy/pasted into the driver. Please note I was referring to a fictional new function that would embed the call to fixed_phy_register(). I was not talking about some existing API, just about a new of_call named of_phy_register_fixed_phy() that would in the end be called by of_phy_register_fixed_link() and by some drivers that want to get in the middle of things and get a hold on status. Maybe the fact we're reviewing two patches in one thread is what makes the discussion less than optimal. > > The getter for status you suggest would be fine, but not sure how one > > would retrieve > > it from the mac_node unless we change of_phy_register_fixed_link() to > > return the > > pointer to phy (and all the drivers that use it...). > If you look for instance to mvneta.c, you'll find the following: > --- > err = of_phy_register_fixed_link(dn); > /* In the case of a fixed PHY, the DT node associated > * to the PHY is the Ethernet MAC DT node. > */ > phy_node = of_node_get(dn); > ... > phy = of_phy_find_device(dn); > --- > > So the answer is: just use the same mac_node for both. I understand, I'll use this approach although is suboptimal imho to scan the device tree again to get a phy pointer that you need just to get some of info that was parsed in a call you just made. > >> Also I meant the description should have been in the patch, > >> not in the e-mail. :) You only wrote _what_ the patch does > >> (which is of course obvious from the code itself), but not > >> _why_ and _what was fixed_ (what didn't work). > >> > > If you refer to the first, separation patch, I thought the description was > enough: > > > > of: separate fixed link parsing from registration > > > > Some drivers may need > "may need"? I don't understand. > If it is a fix, then they _do need_, and in this case it should > be specified what was broken and what is fixed. > If it is just a clean-up, then "may need" may suffice, but it > was not mentioned it is a clean-up. So I still don't know what > this patch is all about. > "Some drivers" - which ones? The ones that are limited to > the purely fixed links, and never support AN or MDIO? > Or some other drivers too? > So really, the description sounds very cryptic to me. Mine, when there is a fixed link node, maybe others. When there isn't any fixed link node, the internal PHY config defaults to 1G full duplex AN enabled and adjust link takes care of things. > > > to parse the fixed link values before registering > > the fixed link phy. Separate the parsing from the actual registration > > and provide an export for the added parsing function. > > > > Signed-off-by: Madalin Bucur > > > > For this one it was a bit brief, I admit - the longer version would be that > before it > > we were not using from fixed link anything else but the fact the link was > fixed > > (ignored actual speed, duplex values there) > And what didn't work as the result? > > > and this patch tries to fix that. > What started to work after that patch that didn't without it? 10M half duplex for instance I'd close this thread for now and use in my driver of_phy_find_device(mac_node). Thank you, Madalin
RE: [PATCH 0/2] of: fsl/fman: reuse the fixed node parsing code
> -Original Message-
> From: Stas Sergeev [mailto:s...@list.ru]
>
> 12.08.2015 16:26, Madalin-Cristian Bucur пишет:
> >>> I've tried to make the smallest changes that allow me to retrieve those
> >>> without modifying existing API.
> >>> Why is it important to hide the default values from the MAC driver?
> >> My worry is that the fixed values are not really fixed, and
> >> therefore are not always useful to access directly. It is likely
> >> not a problem for your use-case, as, as you say, the AN is
> >> disabled, but this is probably not the best to do in general.
> > Yes, not a problem in my case.
> >
> >> And also you do:
> >> ---
> >>
> >> - err = of_phy_register_fixed_link(mac_node);
> >> - if (err)
> >> + struct phy_device *phy;
> >> +
> >> + mac_dev->fixed_link = kzalloc(sizeof(*mac_dev-
> >>> fixed_link),
> >> +GFP_KERNEL);
> >> + if (of_phy_parse_fixed_link(mac_node, mac_dev-
> >>> fixed_link))
> >> + goto _return_dev_set_drvdata;
> >> +
> >> + phy = fixed_phy_register(PHY_POLL, mac_dev->fixed_link,
> >> + mac_node);
> >>
> >> ---
> >>
> >> which means you really want to circumvent the current OF
> >> api quite a lot, without saying why in the patch description.
> > I circumvent the API because I din not want to change existing API.
> > If I could get a reference to the status struct without changing any code
> > or without being required to call by myself fixed_phy_register(), I
> > would of done that. Given the existing code in
> of_phy_register_fixed_link(),
> > this was my only option. I could have broken of_phy_register_fixed_link()
> > in two functions:
> >
> > of_phy_parse_fixed_link() and of_phy_register_fixed_link(), the latter
> doing only
> > the call to fixed_phy_register()
> >
> > that would allow to keep of_phy_register_fixed_link() as it is, broken in
> two stages:
> >
> > - parsing
> > - registering
> >
> > than can be used by other drivers in order to get the status but I think
> > it's
> overkill.
> What I referred to as "circumventing an API" is that you do
> phy = fixed_phy_register(PHY_POLL, mac_dev->fixed_link, + mac_node);
> by hands, instead of letting the of_phy_register_fixed_link() doing so.
>
> How about a smaller circumvention, like this for instance:
> ---
> err = of_phy_register_fixed_link(mac_node);
> phy = of_phy_find_device(dn);
> status = fixed_phy_get_link_status(phy);// no such func, to be coded up
> ---
>
> Or even like this:
> ---
> err = of_phy_register_fixed_link(mac_node);
> phy = of_phy_find_device(dn);
> set_speed_and_duplex(phy->speed, phy->duplex);// not sure if these
> values are available that early
> ---
After my patch, all that of_phy_register_fixed_link() does is to call
the new parsing function I introduced then register the fixed PHY.
I could have done this (pseudocode):
- add of_phy_parse_fixed_link() as seen in the patch
- add of_phy_register_fixed_phy() that just calls fixed_phy_register():
int of_phy_register_fixed_phy(node)
{
phy = fixed_phy_register(PHY_POLL, mac_dev->fixed_link,
mac_node);
return (!phy);
}
- change of_phy_register_fixed_link() to contain only this:
int of_phy_register_fixed_link(node)
{
of_phy_parse_fixed_link(node, &status);
return of_phy_register_fixed_phy(node);
}
Then I could call only of_* functions but the end result would be the same and
of_phy_register_fixed_phy() would not justify its existence that much...
The getter for status you suggest would be fine, but not sure how one would
retrieve
it from the mac_node unless we change of_phy_register_fixed_link() to return the
pointer to phy (and all the drivers that use it...).
>
> Also I meant the description should have been in the patch,
> not in the e-mail. :) You only wrote _what_ the patch does
> (which is of course obvious from the code itself), but not
> _why_ and _what was fixed_ (what didn't work).
>
If you refer to the first, separation patch, I thought the description was
enough:
of: separate fixed link parsing from registration
Some drivers may need to parse the fixed link values before registering
the fixed link phy. Separate the parsing from the actual registration
and provide an export for the added parsing function.
Signed-off-by: Madalin Bucur
For this one it w
RE: [PATCH 0/2] of: fsl/fman: reuse the fixed node parsing code
> -Original Message- > From: Stas Sergeev [mailto:s...@list.ru] > > 11.08.2015 19:33, Madalin-Cristian Bucur пишет: > > + Joakim, Shaohui > > > >> -Original Message- > >> From: Stas Sergeev [mailto:s...@list.ru] > >> > >> 08.08.2015 20:32, Florian Fainelli пишет: > >>> CC'ing Stas, > >> Hi. > >> > >>> Le 08/05/15 07:42, Madalin Bucur a écrit : > >>>> The FMan MAC configuration code needs the speed and duplex > >> information > >>>> for fixed-link interfaces that is parsed now by the of function > >>>> of_phy_register_fixed_link(). This parses the fixed-link parameters but > >>>> does not expose to the caller neither the phy_device pointer nor the > >>>> status struct where it loads the fixed-link params. > > I've tried to make the smallest changes that allow me to retrieve those > > without modifying existing API. > > Why is it important to hide the default values from the MAC driver? > My worry is that the fixed values are not really fixed, and > therefore are not always useful to access directly. It is likely > not a problem for your use-case, as, as you say, the AN is > disabled, but this is probably not the best to do in general. Yes, not a problem in my case. > And also you do: > --- > > - err = of_phy_register_fixed_link(mac_node); > - if (err) > + struct phy_device *phy; > + > + mac_dev->fixed_link = kzalloc(sizeof(*mac_dev- > >fixed_link), > + GFP_KERNEL); > + if (of_phy_parse_fixed_link(mac_node, mac_dev- > >fixed_link)) > + goto _return_dev_set_drvdata; > + > + phy = fixed_phy_register(PHY_POLL, mac_dev->fixed_link, > + mac_node); > > --- > > which means you really want to circumvent the current OF > api quite a lot, without saying why in the patch description. I circumvent the API because I din not want to change existing API. If I could get a reference to the status struct without changing any code or without being required to call by myself fixed_phy_register(), I would of done that. Given the existing code in of_phy_register_fixed_link(), this was my only option. I could have broken of_phy_register_fixed_link() in two functions: of_phy_parse_fixed_link() and of_phy_register_fixed_link(), the latter doing only the call to fixed_phy_register() that would allow to keep of_phy_register_fixed_link() as it is, broken in two stages: - parsing - registering than can be used by other drivers in order to get the status but I think it's overkill. > As such, it may be difficult to review. Could you please write > a more complete description to the patch? To better understand this patch, think of it as just a refactoring of the of_phy_register_fixed_link() that does two things inside: - parsing of fixed link node (2 bindings supported) - register phy by calling fixed_phy_register() in the same way, in the same codebase I've extracted the parsing in a separate function ( following the "one function should do one thing" rule). Then I've exported this function to make status available to callers. > As to your problem: would it be possible to set speed & duplex > after you do of_phy_connect()? It returns the phy_device > pointer, and perhaps you can look into phydev->speed and > phydev->duplex at that point? It would be possible but un-natural as I'd have probing information only available at runtime. That would just complicate matters for my particular case ans I suspect there will be other drivers that get into this situation. You are concerned about people abusing this API to read fixed link status when the link is not really fixed, I'm concerned about declaring the link as fixed-link when it's not. Maybe the naming/binding needs to be revised to cover the case when all is fixed but the link.
RE: [PATCH 0/2] of: fsl/fman: reuse the fixed node parsing code
+ Joakim, Shaohui > -Original Message- > From: Stas Sergeev [mailto:s...@list.ru] > > 08.08.2015 20:32, Florian Fainelli пишет: > > CC'ing Stas, > Hi. > > > Le 08/05/15 07:42, Madalin Bucur a écrit : > >> The FMan MAC configuration code needs the speed and duplex > information > >> for fixed-link interfaces that is parsed now by the of function > >> of_phy_register_fixed_link(). This parses the fixed-link parameters but > >> does not expose to the caller neither the phy_device pointer nor the > >> status struct where it loads the fixed-link params. > I have only barely touched that code, but IMO both things > are by design. There are some API deficiencies, and so, many > drivers still use of_phy_find_device() to circumvent the encapsulation > and get the phy_device pointer, but this is unlikely a good thing > to do. I even proposed some API extensions, but there was no > interest. > > >> By extracting the > >> fixed-link parsing code from of_phy_register_fixed_link() into a > >> separate function the parsed values are made available without changing > >> the existing API. This change also removes a small redundancy in the > >> previous code calling fixed_phy_register(). > Today, the fixed_link is not always fixed. > See for example this patch (already mainlined): > https://lkml.org/lkml/2015/7/20/711 > of_phy_is_fixed_link() returns 'true' if you have > managed="in-band-status", and so the SGMII in-band status > can update fixed-link params. > > So my question is: why do you even need to know whether > the link is fixed or not? IIRC you can check the phy_device > pointer in the adjust_link callback of of_phy_connect() to get > the current link status values. Why is this not enough for your > task? Maybe the patch description should be updated to include > why the current technique is bad, what is actually fixed by the > change. > I think using the fixed-link DT values directly is not something > to be done. The encapsulation is there for a reason, so maybe > instead we can see what API additions do we need to avoid the > current limitations that force people to use of_phy_find_device() > and other work-arounds. I need to be able to determine the imposed speed and duplex for fixed link external PHYs because I need to configure the internal PHY with matching values. If I do not set the same speed, given the fact that AN needs to be off, there will be no link and no adjust link to fix things later (and the internal PHY is not updated by adjust link anyway). I do not have access at the phy pointer at the time I need the speed and duplex, to retrieve the defaults from there and I've tried to make the smallest changes that allow me to retrieve those without modifying existing API. Why is it important to hide the default values from the MAC driver? Thank you, Madalin
RE: [v2 8/9] dpaa_eth: add debugfs entries
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > > From: Madalin Bucur > Date: Wed, 5 Aug 2015 18:41:28 +0300 > > > Export per CPU counters through debugfs. > > > > Signed-off-by: Madalin Bucur > > This is absolutely inappropriate. > > You can export these just fine via ethtool statistics. There is zero reason > to add ugly debugfs crap for something like this. If you have such a strong adversity about the debugfs, then I'll export these debug counters through ethtool statistics in the next version. 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 03/10] dpaa_eth: add configurable bpool thresholds
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Sent: Monday, July 27, 2015 2:35 AM > To: Bucur Madalin-Cristian-B32716 > Cc: j...@perches.com; netdev@vger.kernel.org; linuxppc- > d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; Wood Scott-B07421; > Liberman Igal-B31950; p...@mindchasers.com; pebo...@tiscali.nl; > joakim.tjernl...@transmode.se > Subject: Re: [PATCH 03/10] dpaa_eth: add configurable bpool thresholds > > From: Madalin-Cristian Bucur > Date: Fri, 24 Jul 2015 15:49:39 + > > >> -Original Message- > >> From: Joe Perches [mailto:j...@perches.com] > >> On Wed, 2015-07-22 at 19:16 +0300, Madalin Bucur wrote: > >> > Allow the user to tweak the refill threshold and the total number > >> > of buffers in the buffer pool. The provided values are for one CPU. > >> > >> Any value in making these module parameters instead? > > > > I expect one would (hardly ever) change these to improve some corner > > cases then use them with the new values. It may help in the tuning process > > but afterwards the bloat to the bootcmd would probably be a nuisance. > > I think these should be controlled by the existing ethtool infrastructure. > > Neither the Kconfig mechanism nor module parameters are appropriate, at > all. The existing ethtool options are for ring based drivers (ethtool -g / -G). I would not use those as we are not using rings (they do not map well anyway). We could introduce special options for our non-ring devices but for these parameters in particular I'd just resort to defines in the code as it's improbable one would want to change them. 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 03/10] dpaa_eth: add configurable bpool thresholds
> -Original Message- > From: Joe Perches [mailto:j...@perches.com] > On Wed, 2015-07-22 at 19:16 +0300, Madalin Bucur wrote: > > Allow the user to tweak the refill threshold and the total number > > of buffers in the buffer pool. The provided values are for one CPU. > > Any value in making these module parameters instead? I expect one would (hardly ever) change these to improve some corner cases then use them with the new values. It may help in the tuning process but afterwards the bloat to the bootcmd would probably be a nuisance. > > +config FSL_DPAA_ETH_MAX_BUF_COUNT > > + int "Maximum number of buffers in private bpool" > > + range 64 2048 > > + default "128" > > + ---help--- > > + The maximum number of buffers to be by default allocated in the > DPAA-Ethernet private port's > > + buffer pool. One needn't normally modify this, as it has probably > been tuned for performance > > + already. This cannot be lower than DPAA_ETH_REFILL_THRESHOLD. > > + > > +config FSL_DPAA_ETH_REFILL_THRESHOLD > > + int "Private bpool refill threshold" > > + range 32 FSL_DPAA_ETH_MAX_BUF_COUNT > > + default "80" > > + ---help--- > > + The DPAA-Ethernet driver will start replenishing buffer pools whose > count > > + falls below this threshold. This must be related to > DPAA_ETH_MAX_BUF_COUNT. One needn't normally > > + modify this value unless one has very specific performance reasons. > > + > > config FSL_DPAA_CS_THRESHOLD_1G > > hex "Egress congestion threshold on 1G ports" > > range 0x1000 0x1000 > > -- 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
