RE: [PATCH] powerpc/kmcent2: update the ethernet devices' phy properties

[Madalin-cristian Bucur]

> -Original Message-
> From: Scott Wood 
> Sent: Wednesday, August 28, 2019 7:19 AM
> To: Valentin Longchamp ; Madalin-cristian Bucur
> 
> Cc: linuxppc-dev@lists.ozlabs.org; ga...@kernel.crashing.org;
> net...@vger.kernel.org
> Subject: Re: [PATCH] powerpc/kmcent2: update the ethernet devices' phy
> properties
> 
> On Thu, 2019-08-08 at 23:09 +0200, Valentin Longchamp wrote:
> > Le mar. 30 juil. 2019 à 11:44, Madalin-cristian Bucur
> >  a écrit :
> > >
> > > > -Original Message-
> > > >
> > > > > Le dim. 14 juil. 2019 à 22:05, Valentin Longchamp
> > > > >  a écrit :
> > > > > >
> > > > > > Change all phy-connection-type properties to phy-mode that are
> > > > > > better
> > > > > > supported by the fman driver.
> > > > > >
> > > > > > Use the more readable fixed-link node for the 2 sgmii links.
> > > > > >
> > > > > > Change the RGMII link to rgmii-id as the clock delays are added
> by
> > > > > > the
> > > > > > phy.
> > > > > >
> > > > > > Signed-off-by: Valentin Longchamp 
> > > >
> > > > I don't see any other uses of phy-mode in arch/powerpc/boot/dts/fsl,
> and
> > > > I see
> > > > lots of phy-connection-type with fman.  Madalin, does this patch
> look
> > > > OK?
> > > >
> > > > -Scott
> > >
> > > Hi,
> > >
> > > we are using "phy-connection-type" not "phy-mode" for the NXP (former
> > > Freescale)
> > > DPAA platforms. While the two seem to be interchangeable ("phy-mode"
> seems
> > > to be
> > > more recent, looking at the device tree bindings), the driver code in
> > > Linux seems
> > > to use one or the other, not both so one should stick with the variant
> the
> > > driver
> > > is using. To make things more complex, there may be dependencies in
> > > bootloaders,
> > > I see code in u-boot using only "phy-connection-type" or only "phy-
> mode".
> > >
> > > I'd leave "phy-connection-type" as is.
> >
> > So I have finally had time to have a look and now I understand what
> > happens. You are right, there are bootloader dependencies: u-boot
> > calls fdt_fixup_phy_connection() that somehow in our case adds (or
> > changes if already in the device tree) the phy-connection-type
> > property to a wrong value ! By having a phy-mode in the device tree,
> > that is not changed by u-boot and by chance picked up by the kernel
> > fman driver (of_get_phy_mode() ) over phy-connection-mode, the below
> > patch fixes it for us.
> >
> > I agree with you, it's not correct to have both phy-connection-type
> > and phy-mode. Ideally, u-boot on the board should be reworked so that
> > it does not perform the above wrong fixup. However, in an "unfixed"
> > .dtb (I have disabled fdt_fixup_phy_connection), the device tree in
> > the end only has either phy-connection-type or phy-mode, according to
> > what was chosen in the .dts file. And the fman driver works well with
> > both (thanks to the call to of_get_phy_mode() ). I would therefore
> > argue that even if all other DPAA platforms use phy-connection-type,
> > phy-mode is valid as well. (Furthermore we already have hundreds of
> > such boards in the field and we don't really support "remote" u-boot
> > update, so the u-boot fix is going to be difficult for us to pull).
> >
> > Valentin
> 
> Madalin, are you OK with the patch given this explanation?
> 
> -Scott
> 

Yes, I understand that it's the only option they have, given the inability
to upgrade u-boot (this may prove to be an issue in the future, in other
situations).

Acked-by: Madalin Bucur 

RE: [PATCH] powerpc/kmcent2: update the ethernet devices' phy properties

[Madalin-cristian Bucur]

> -Original Message-
> From: Scott Wood 
> Sent: Sunday, July 28, 2019 10:27 PM
> To: Valentin Longchamp ; linuxppc-
> d...@lists.ozlabs.org; ga...@kernel.crashing.org
> Cc: Madalin-cristian Bucur 
> Subject: Re: [PATCH] powerpc/kmcent2: update the ethernet devices' phy
> properties
> 
> On Sun, 2019-07-28 at 18:01 +0200, Valentin Longchamp wrote:
> > Hi Scott, Kumar,
> >
> > Looking at this patch I have realised that I had already submitted it
> > to the mailing list nearly 2 years ago:
> >
> > https://patchwork.ozlabs.org/patch/842944/
> >
> > Could you please make sure that this one gets merged in the next
> > window, so that I avoid forgetting such a patch a 2nd time ?
> >
> > Thanks a lot
> 
> I added it to my patchwork todo list; thanks for the reminder.
> 
> > Le dim. 14 juil. 2019 à 22:05, Valentin Longchamp
> >  a écrit :
> > >
> > > Change all phy-connection-type properties to phy-mode that are better
> > > supported by the fman driver.
> > >
> > > Use the more readable fixed-link node for the 2 sgmii links.
> > >
> > > Change the RGMII link to rgmii-id as the clock delays are added by the
> > > phy.
> > >
> > > Signed-off-by: Valentin Longchamp 
> 
> I don't see any other uses of phy-mode in arch/powerpc/boot/dts/fsl, and I see
> lots of phy-connection-type with fman.  Madalin, does this patch look OK?
> 
> -Scott

Hi,

we are using "phy-connection-type" not "phy-mode" for the NXP (former Freescale)
DPAA platforms. While the two seem to be interchangeable ("phy-mode" seems to be
more recent, looking at the device tree bindings), the driver code in Linux 
seems
to use one or the other, not both so one should stick with the variant the 
driver
is using. To make things more complex, there may be dependencies in bootloaders,
I see code in u-boot using only "phy-connection-type" or only "phy-mode".

I'd leave "phy-connection-type" as is.

Madalin

> > > ---
> > >  arch/powerpc/boot/dts/fsl/kmcent2.dts | 16 +++-
> > >  1 file changed, 11 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/arch/powerpc/boot/dts/fsl/kmcent2.dts
> > > b/arch/powerpc/boot/dts/fsl/kmcent2.dts
> > > index 48b7f9797124..c3e0741cafb1 100644
> > > --- a/arch/powerpc/boot/dts/fsl/kmcent2.dts
> > > +++ b/arch/powerpc/boot/dts/fsl/kmcent2.dts
> > > @@ -210,13 +210,19 @@
> > >
> > > fman@40 {
> > > ethernet@e {
> > > -   fixed-link = <0 1 1000 0 0>;
> > > -   phy-connection-type = "sgmii";
> > > +   phy-mode = "sgmii";
> > > +   fixed-link {
> > > +   speed = <1000>;
> > > +   full-duplex;
> > > +   };
> > > };
> > >
> > > ethernet@e2000 {
> > > -   fixed-link = <1 1 1000 0 0>;
> > > -   phy-connection-type = "sgmii";
> > > +   phy-mode = "sgmii";
> > > +   fixed-link {
> > > +   speed = <1000>;
> > > +   full-duplex;
> > > +   };
> > > };
> > >
> > > ethernet@e4000 {
> > > @@ -229,7 +235,7 @@
> > >
> > > ethernet@e8000 {
> > > phy-handle = <_phy>;
> > > -   phy-connection-type = "rgmii";
> > > +   phy-mode = "rgmii-id";
> > > };
> > >
> > > mdio0: mdio@fc000 {
> > > --
> > > 2.17.1
> > >
> >
> >

RE: [PATCH] fsl/fman: Use GFP_ATOMIC in {memac,tgec}_add_hash_mac_address()

[Madalin-cristian Bucur]

> -Original Message-
> From: Scott Wood 
> Sent: Friday, December 28, 2018 2:29 AM
> To: Madalin-cristian Bucur 
> Cc: net...@vger.kernel.org; linuxppc-dev@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 v2 2/2] dpaa_eth: add ethtool coalesce control

[Madalin-cristian Bucur]

> -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 0/5] soc/fsl/qbman: DPAA QBMan fixes and additions

[Madalin-cristian Bucur]

> -Original Message-
> From: Li Yang [mailto:leoyang...@nxp.com]
> Sent: Tuesday, October 2, 2018 1:30 AM
> To: Madalin-cristian Bucur 
> Cc: Roy Pledge ; Claudiu Manoil
> ; Catalin Marinas ; Scott
> Wood ; moderated list:ARM/FREESCALE IMX / MXC ARM
> ARCHITECTURE ; linuxppc-dev
> ; lkml 
> Subject: Re: [PATCH v2 0/5] soc/fsl/qbman: DPAA QBMan fixes and additions
> 
> On Fri, Sep 28, 2018 at 3:44 AM Madalin Bucur 
> wrote:
> >
> 
> Applied 1-4 to for-next while waiting for clarification on 5/5.   And
> updated the prefix to "soc: fsl:" style to be aligned with arm-soc
> convention.  Please try to use that style in the future for soc/fsl
> patches.

Thank you, I've sent an email about the APIs.
I'm not sure we need to align the prefix to arm-soc as the soc/fsl does not
service only ARM but also PPC based SoCs and historically we've been using
the soc/* format.

Regards,
Madalin

RE: [PATCH v2 5/5] soc/fsl_qbman: export coalesce change API

[Madalin-cristian Bucur]

> -Original Message-
> From: Li Yang [mailto:leoyang...@nxp.com]
> Sent: Tuesday, October 2, 2018 12:50 AM
> To: Madalin-cristian Bucur 
> Cc: Roy Pledge ; Claudiu Manoil
> ; Catalin Marinas ; Scott
> Wood ; moderated list:ARM/FREESCALE IMX / MXC ARM
> ARCHITECTURE ; linuxppc-dev
> ; lkml 
> Subject: Re: [PATCH v2 5/5] soc/fsl_qbman: export coalesce change API
> 
> On Fri, Sep 28, 2018 at 3:45 AM Madalin Bucur 
> wrote:
> >
> > Export the API required to control the QMan portal interrupt coalescing
> > settings.
> 
> These are new APIs not just old APIs being exported.  What is the user
> of these APIs?  Is the user being submitted?  We cannot have APIs in
> kernel that has no users.

Hi,

These are new APIs that will be used in the DPAA Ethernet driver.
Changes for the DPAA QBMan and DPAA Ethernet follow different paths upstream
so the Ethernet driver patch that makes use of these APIs will be sent when
these changes reach the net-next/master tree.

Regards,
Madalin

RE: [PATCH 1/4] soc/fsl/qbman: Check if CPU is offline when initializing portals

[Madalin-cristian Bucur]

> -Original Message-
> From: Li Yang [mailto:leoyang...@nxp.com]
> Sent: Saturday, September 22, 2018 1:15 AM
> To: Madalin-cristian Bucur 
> Subject: Re: [PATCH 1/4] soc/fsl/qbman: Check if CPU is offline when
> initializing portals
> 
> On Thu, Sep 20, 2018 at 10:09 AM Madalin Bucur 
> wrote:
> >
> > From: Roy Pledge 
> >
> > If the affine portal for a specific CPU is offline at boot time
> > affine its interrupt to CPU 0. If the CPU is later brought online
> > the hotplug handler will correctly adjust the affinity.
> 
> Although this does provide better compatibility with cpu hotplug, we
> still have a problem.  You are assuming the CPU0 is always online
> which is not the case for arm64.  How about not setting the affinity
> and let the system decide if the dedicated CPU is offline?

I can change the hardcoding of CPU 0 with raw_smp_processor_id().
While we're at it, I'd move the introduced code into a helper function
to avoid duplication:

static inline void dpaa_set_portal_irq_affinity(struct device* dev,
int cpu, int 
irq)

> >
> > Signed-off-by: Roy Pledge 
> > Signed-off-by: Madalin Bucur 
> > ---
> >  drivers/soc/fsl/qbman/bman.c | 17 +
> >  drivers/soc/fsl/qbman/qman.c | 18 +-
> >  2 files changed, 26 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/soc/fsl/qbman/bman.c b/drivers/soc/fsl/qbman/bman.c
> > index f9485cedc648..2e6e682bf16b 100644
> > --- a/drivers/soc/fsl/qbman/bman.c
> > +++ b/drivers/soc/fsl/qbman/bman.c
> > @@ -562,10 +562,19 @@ static int bman_create_portal(struct bman_portal
> *portal,
> > dev_err(c->dev, "request_irq() failed\n");
> > goto fail_irq;
> > }
> > -   if (c->cpu != -1 && irq_can_set_affinity(c->irq) &&
> > -   irq_set_affinity(c->irq, cpumask_of(c->cpu))) {
> > -   dev_err(c->dev, "irq_set_affinity() failed\n");
> > -   goto fail_affinity;
> > +   if (cpu_online(c->cpu) && c->cpu != -1 &&
> > +   irq_can_set_affinity(c->irq)) {
> > +   if (irq_set_affinity(c->irq, cpumask_of(c->cpu))) {
> > +   dev_err(c->dev, "irq_set_affinity() failed
> %d\n",
> > +   c->cpu);
> > +   goto fail_affinity;
> > +   }
> > +   } else {
> > +   /* CPU is offline, direct IRQ to CPU 0 */
> > +   if (irq_set_affinity(c->irq, cpumask_of(0))) {
> > +   dev_err(c->dev, "irq_set_affinity() cpu 0
> failed\n");
> > +   goto fail_affinity;
> > +   }
> > }
> >
> > /* Need RCR to be empty before continuing */
> > diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
> > index ecb22749df0b..7dbcb475a59c 100644
> > --- a/drivers/soc/fsl/qbman/qman.c
> > +++ b/drivers/soc/fsl/qbman/qman.c
> > @@ -935,7 +935,6 @@ static inline int qm_mc_result_timeout(struct
> qm_portal *portal,
> > break;
> > udelay(1);
> > } while (--timeout);
> > -
> > return timeout;
> >  }
> >
> > @@ -1210,10 +1209,19 @@ static int qman_create_portal(struct qman_portal
> *portal,
> > dev_err(c->dev, "request_irq() failed\n");
> > goto fail_irq;
> > }
> > -   if (c->cpu != -1 && irq_can_set_affinity(c->irq) &&
> > -   irq_set_affinity(c->irq, cpumask_of(c->cpu))) {
> > -   dev_err(c->dev, "irq_set_affinity() failed\n");
> > -   goto fail_affinity;
> > +   if (cpu_online(c->cpu) && c->cpu != -1 &&
> > +   irq_can_set_affinity(c->irq)) {
> > +   if (irq_set_affinity(c->irq, cpumask_of(c->cpu))) {
> > +   dev_err(c->dev, "irq_set_affinity() failed
> %d\n",
> > +   c->cpu);
> > +   goto fail_affinity;
> > +   }
> > +   } else {
> > +   /* CPU is offline, direct IRQ to CPU 0 */
> > +   if (irq_set_affinity(c->irq, cpumask_of(0))) {
> > +   dev_err(c->dev, "irq_set_affinity() cpu 0
> failed\n");
> > +   goto fail_affinity;
> > +   }
> > }
> >
> > /* Need EQCR to be empty before continuing */
> > --
> > 2.1.0
> >

RE: [v3, 00/10] Support DPAA PTP clock and timestamping

[Madalin-cristian Bucur]

> -Original Message-
> From: Yangbo Lu [mailto:yangbo...@nxp.com]
> Sent: Thursday, June 7, 2018 12:21 PM
> To: net...@vger.kernel.org; Madalin-cristian Bucur
> ; Richard Cochran ;
> Rob Herring ; Shawn Guo ;
> David S . Miller 
> Cc: devicet...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux-arm-
> ker...@lists.infradead.org; linux-ker...@vger.kernel.org; Y.b. Lu
> 
> Subject: [v3, 00/10] Support DPAA PTP clock and timestamping
> 
> This patchset is to support DPAA FMAN PTP clock and HW timestamping.
> It had been verified on both ARM platform and PPC platform.
> - The patch #1 to patch #5 are to support DPAA FMAN 1588 timer in
>   ptp_qoriq driver.
> - The patch #6 to patch #10 are to add HW timestamping support in
>   DPAA ethernet driver.
> 
> Yangbo Lu (10):
>   fsl/fman: share the event interrupt
>   ptp: support DPAA FMan 1588 timer in ptp_qoriq
>   dt-binding: ptp_qoriq: add DPAA FMan support
>   powerpc/mpc85xx: move ptp timer out of fman in dts
>   arm64: dts: fsl: move ptp timer out of fman
>   fsl/fman: add set_tstamp interface
>   fsl/fman_port: support getting timestamp
>   fsl/fman: define frame description command UPD
>   dpaa_eth: add support for hardware timestamping
>   dpaa_eth: add the get_ts_info interface for ethtool
> 
>  Documentation/devicetree/bindings/net/fsl-fman.txt |   25 +-
>  .../devicetree/bindings/ptp/ptp-qoriq.txt  |   15 +++-
>  arch/arm64/boot/dts/freescale/qoriq-fman3-0.dtsi   |   14 ++-
>  arch/powerpc/boot/dts/fsl/qoriq-fman-0.dtsi|   14 ++-
>  arch/powerpc/boot/dts/fsl/qoriq-fman-1.dtsi|   14 ++-
>  arch/powerpc/boot/dts/fsl/qoriq-fman3-0.dtsi   |   14 ++-
>  arch/powerpc/boot/dts/fsl/qoriq-fman3-1.dtsi   |   14 ++-
>  arch/powerpc/boot/dts/fsl/qoriq-fman3l-0.dtsi  |   14 ++-
>  drivers/net/ethernet/freescale/dpaa/dpaa_eth.c |   88
> -
>  drivers/net/ethernet/freescale/dpaa/dpaa_eth.h |3 +
>  drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c |   39 
>  drivers/net/ethernet/freescale/fman/fman.c |3 +-
>  drivers/net/ethernet/freescale/fman/fman.h |1 +
>  drivers/net/ethernet/freescale/fman/fman_dtsec.c   |   27 +
>  drivers/net/ethernet/freescale/fman/fman_dtsec.h   |1 +
>  drivers/net/ethernet/freescale/fman/fman_memac.c   |5 +
>  drivers/net/ethernet/freescale/fman/fman_memac.h   |1 +
>  drivers/net/ethernet/freescale/fman/fman_port.c|   12 +++
>  drivers/net/ethernet/freescale/fman/fman_port.h|2 +
>  drivers/net/ethernet/freescale/fman/fman_tgec.c|   21 
>  drivers/net/ethernet/freescale/fman/fman_tgec.h|1 +
>  drivers/net/ethernet/freescale/fman/mac.c  |3 +
>  drivers/net/ethernet/freescale/fman/mac.h  |1 +
>  drivers/ptp/Kconfig|2 +-
>  drivers/ptp/ptp_qoriq.c|  104 ---
>  include/linux/fsl/ptp_qoriq.h  |   38 ++--
>  26 files changed, 361 insertions(+), 115 deletions(-)

Acked-by: Madalin Bucur 

RE: [v2, 09/10] dpaa_eth: add support for hardware timestamping

[Madalin-cristian Bucur]

> -Original Message-
> From: Yangbo Lu [mailto:yangbo...@nxp.com]
> Sent: Thursday, June 7, 2018 6:23 AM
> Subject: [v2, 09/10] dpaa_eth: add support for hardware timestamping
> 
> This patch is to add hardware timestamping support
> for dpaa_eth. On Rx, timestamping is enabled for
> all frames. On Tx, we only instruct the hardware
> to timestamp the frames marked accordingly by the
> stack.
> 
> Signed-off-by: Yangbo Lu 
> ---
> Changes for v2:
>   - Removed ifdef for timestamp code.
>   - Minor fixes for code style.
> ---
>  drivers/net/ethernet/freescale/dpaa/dpaa_eth.c |  101
> ++-
>  drivers/net/ethernet/freescale/dpaa/dpaa_eth.h |3 +
>  2 files changed, 99 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> index fd43f98..bd589ac 100644
> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> @@ -1168,7 +1168,7 @@ static int dpaa_eth_init_tx_port(struct fman_port
> *port, struct dpaa_fq *errq,
>   buf_prefix_content.priv_data_size = buf_layout->priv_data_size;
>   buf_prefix_content.pass_prs_result = true;
>   buf_prefix_content.pass_hash_result = true;
> - buf_prefix_content.pass_time_stamp = false;
> + buf_prefix_content.pass_time_stamp = true;
>   buf_prefix_content.data_align = DPAA_FD_DATA_ALIGNMENT;
> 
>   params.specific_params.non_rx_params.err_fqid = errq->fqid;
> @@ -1210,7 +1210,7 @@ static int dpaa_eth_init_rx_port(struct fman_port
> *port, struct dpaa_bp **bps,
>   buf_prefix_content.priv_data_size = buf_layout->priv_data_size;
>   buf_prefix_content.pass_prs_result = true;
>   buf_prefix_content.pass_hash_result = true;
> - buf_prefix_content.pass_time_stamp = false;
> + buf_prefix_content.pass_time_stamp = true;
>   buf_prefix_content.data_align = DPAA_FD_DATA_ALIGNMENT;
> 
>   rx_p = _params.rx_params;
> @@ -1592,6 +1592,16 @@ static int dpaa_eth_refill_bpools(struct dpaa_priv
> *priv)
>   return 0;
>  }
> 
> +static int dpaa_get_tstamp_ns(struct net_device *net_dev, u64 *ns,
> +   struct fman_port *port, const void *data)
> +{
> + if (!fman_port_get_tstamp_field(port, data, ns)) {
> + be64_to_cpus(ns);

Please move this endianness conversion in the fman API.

> + return 0;
> + }
> + return -EINVAL;
> +}
> +
>  /* Cleanup function for outgoing frame descriptors that were built on Tx
> path,
>   * either contiguous frames or scatter/gather ones.
>   * Skb freeing is not handled here.
> @@ -1607,14 +1617,29 @@ static int dpaa_eth_refill_bpools(struct dpaa_priv
> *priv)
>  {
>   const enum dma_data_direction dma_dir = DMA_TO_DEVICE;
>   struct device *dev = priv->net_dev->dev.parent;
> + struct skb_shared_hwtstamps shhwtstamps;
>   dma_addr_t addr = qm_fd_addr(fd);
>   const struct qm_sg_entry *sgt;
>   struct sk_buff **skbh, *skb;
>   int nr_frags, i;
> + u64 ns;
> 
>   skbh = (struct sk_buff **)phys_to_virt(addr);
>   skb = *skbh;
> 
> + if (priv->tx_tstamp && skb_shinfo(skb)->tx_flags &
> SKBTX_HW_TSTAMP) {
> + memset(, 0, sizeof(shhwtstamps));
> +
> + if (!dpaa_get_tstamp_ns(priv->net_dev, ,
> + priv->mac_dev->port[TX],
> + (void *)skbh)) {
> + shhwtstamps.hwtstamp = ns_to_ktime(ns);
> + skb_tstamp_tx(skb, );
> + } else {
> + dev_warn(dev, "dpaa_get_tstamp_ns failed!\n");
> + }
> + }
> +
>   if (unlikely(qm_fd_get_format(fd) == qm_fd_sg)) {
>   nr_frags = skb_shinfo(skb)->nr_frags;
>   dma_unmap_single(dev, addr, qm_fd_get_offset(fd) +
> @@ -2086,6 +2111,11 @@ static int dpaa_start_xmit(struct sk_buff *skb,
> struct net_device *net_dev)
>   if (unlikely(err < 0))
>   goto skb_to_fd_failed;
> 
> + if (priv->tx_tstamp && skb_shinfo(skb)->tx_flags &
> SKBTX_HW_TSTAMP) {
> + fd.cmd |= FM_FD_CMD_UPD;

The fd.cmd field is big endian, please use this:

+   fd.cmd |= cpu_to_be32(FM_FD_CMD_UPD);

> + skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> + }
> +
>   if (likely(dpaa_xmit(priv, percpu_stats, queue_mapping, ) == 0))
>   return NETDEV_TX_OK;
> 
> @@ -2227,6 +2257,7 @@ static enum qman_cb_dqrr_result
> rx_default_dqrr(struct qman_portal *portal,
>   struct qman_fq *fq,
>   const struct qm_dqrr_entry
> *dq)
>  {
> + struct skb_shared_hwtstamps *shhwtstamps;
>   struct rtnl_link_stats64 *percpu_stats;
>   struct dpaa_percpu_priv *percpu_priv;
>   const struct qm_fd *fd = >fd;
> @@ -2240,6 +2271,7 @@ static enum qman_cb_dqrr_result
> 

RE: [PATCH 0/5] DPAA Ethernet fixes

[Madalin-cristian Bucur]

> -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
> <madalin.bu...@nxp.com>
> 
> 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: DPAA Ethernet traffice troubles with Linux kernel

[Madalin-cristian Bucur]

> -Original Message-
> From: Madalin-cristian Bucur
> Sent: Wednesday, January 17, 2018 4:25 PM
> To: David S . Miller <da...@davemloft.net>
> Cc: linuxppc-dev@lists.ozlabs.org; net...@vger.kernel.org;
> madskate...@gmail.com; 'Madalin-cristian Bucur' <madalin.bu...@nxp.com>;
> Andrew Lunn <and...@lunn.ch>; Joakim Tjernlund
> <joakim.tjernl...@infinera.com>
> 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 <and...@lunn.ch>; Joakim Tjernlund
> > <joakim.tjernl...@infinera.com>
> > Cc: linuxppc-dev@lists.ozlabs.org; net...@vger.kernel.org;
> > madskate...@gmail.com; David S . Miller <da...@davemloft.net>
> > 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 <joakim.tjernl...@infinera.com>
> > > 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

[Madalin-cristian Bucur]

> -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 <and...@lunn.ch>; Joakim Tjernlund
> <joakim.tjernl...@infinera.com>
> Cc: linuxppc-dev@lists.ozlabs.org; net...@vger.kernel.org;
> madskate...@gmail.com; David S . Miller <da...@davemloft.net>
> 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 <joakim.tjernl...@infinera.com>
> > 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

[Madalin-cristian Bucur]

> -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

[Madalin-cristian Bucur]

> -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=*

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

[Madalin-cristian Bucur]

> -Original Message-
> From: Darren Stevens [mailto:dar...@stevens-zone.net]
> Sent: Tuesday, January 16, 2018 12:40 AM
> To: Madalin-cristian Bucur <madalin.bu...@nxp.com>
> Cc: Jamie Krueger <ja...@bitbybitsoftwaregroup.com>; linuxppc-
> d...@lists.ozlabs.org; net...@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

[Madalin-cristian Bucur]

> -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 <madskate...@gmail.com>
> Cc: Christian Zigotzky <chzigot...@xenosoft.de>; Joakim Tjernlund
> <joakim.tjernl...@infinera.com>; linuxppc-dev@lists.ozlabs.org; Madalin-
> cristian Bucur <madalin.bu...@nxp.com>; net...@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

[Madalin-cristian Bucur]

> -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-dev@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

[Madalin-cristian Bucur]

> -Original Message-
> From: Jamie Krueger [mailto:ja...@bitbybitsoftwaregroup.com]
> Sent: Friday, January 12, 2018 6:36 PM
> To: Madalin-cristian Bucur <madalin.bu...@nxp.com>; linuxppc-
> d...@lists.ozlabs.org
> Cc: net...@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-dev@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 har

RE: DPAA Ethernet problems with mainstream Linux kernels

[Madalin-cristian Bucur]

> -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-dev@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 

RE: [PATCH v3 0/7] Add RSS to DPAA 1.x Ethernet driver

[Madalin-cristian Bucur]

> -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

[Madalin-cristian Bucur]

> -Original Message-
> From: David Miller [mailto:da...@davemloft.net]
> Sent: Wednesday, August 23, 2017 7:47 AM
> To: Madalin-cristian Bucur <madalin.bu...@nxp.com>
> Cc: net...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH v2 1/6] fsl/fman: enable FMan Keygen
> 
> From: Madalin-cristian Bucur <madalin.bu...@nxp.com>
> 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

[Madalin-cristian Bucur]

> -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

[Madalin-cristian Bucur]

> -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 1/2] fsl/fman: propagate dma_ops

[Madalin-cristian Bucur]

> -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 <madalin.bu...@nxp.com>
> Cc: net...@vger.kernel.org; David S. Miller <da...@davemloft.net>;
> linuxppc-dev@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
> <madalin.bu...@nxp.com> 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 <madalin.bu...@nxp.com>
> >> Cc: net...@vger.kernel.org; David S. Miller <da...@davemloft.net>;
> >> linuxppc-dev@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 <madalin.bu...@nxp.com>
> >> wrote:
> >> > Make sure dma_ops are set, to be later used by the Ethernet driver.
> >> >
> >> > Signed-off-by: Madalin Bucur <madalin.bu...@nxp.com>
> >> > ---
> >> >  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(>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

[Madalin-cristian Bucur]

> -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 <feste...@gmail.com>
> Cc: Madalin-cristian Bucur <madalin.bu...@nxp.com>;
> net...@vger.kernel.org; David S. Miller <da...@davemloft.net>; linuxppc-
> d...@lists.ozlabs.org; linux-kernel <linux-ker...@vger.kernel.org>
> Subject: Re: [PATCH] fsl/fman: add dependency on HAS_DMA
> 
> On Mon, Jun 26, 2017 at 5:20 PM, Fabio Estevam <feste...@gmail.com> wrote:
> > On Mon, Jun 26, 2017 at 12:12 PM, Madalin Bucur <madalin.bu...@nxp.com>
> 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

[Madalin-cristian Bucur]

> -Original Message-
> From: Fabio Estevam [mailto:feste...@gmail.com]
> Sent: Monday, June 26, 2017 6:21 PM
> To: Madalin-cristian Bucur <madalin.bu...@nxp.com>
> Cc: net...@vger.kernel.org; David S. Miller <da...@davemloft.net>; Geert
> Uytterhoeven <ge...@linux-m68k.org>; linuxppc-dev@lists.ozlabs.org; linux-
> kernel <linux-ker...@vger.kernel.org>
> Subject: Re: [PATCH] fsl/fman: add dependency on HAS_DMA
> 
> On Mon, Jun 26, 2017 at 12:12 PM, Madalin Bucur <madalin.bu...@nxp.com>
> 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

[Madalin-cristian Bucur]

> -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 <madalin.bu...@nxp.com>
> Cc: net...@vger.kernel.org; David S. Miller <da...@davemloft.net>;
> linuxppc-dev@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 <madalin.bu...@nxp.com>
> wrote:
> > Make sure dma_ops are set, to be later used by the Ethernet driver.
> >
> > Signed-off-by: Madalin Bucur <madalin.bu...@nxp.com>
> > ---
> >  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(>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: DPAA: Software Portal selection for network interface

[Madalin-cristian Bucur]

Hi Sebastian,

> -Original Message-
> From: Sebastian Huber [mailto:sebastian.hu...@embedded-brains.de]
> Sent: Thursday, June 22, 2017 3:42 PM
> To: linuxppc-dev@lists.ozlabs.org
> Cc: Madalin-cristian Bucur <madalin.bu...@nxp.com>
> Subject: DPAA: Software Portal selection for network interface
> 
> Hello,
> 
> as far as I understand the software portal selection for a network
> interface is done in
> 
> static int dpaa_eth_probe(struct platform_device *pdev)
> {
>  [...]
>  channel = dpaa_get_channel();
>  if (channel < 0) {
>  dev_err(dev, "dpaa_get_channel() failed\n");
>  err = channel;
>  goto get_channel_failed;
>  }
> 
>  priv->channel = (u16)channel;
> 
>  /* Start a thread that will walk the CPUs with affine portals
>   * and add this pool channel to each's dequeue mask.
>   */
>  dpaa_eth_add_channel(priv->channel);
> 
> with
> 
> static int dpaa_get_channel(void)
> {
>  spin_lock(_pool_channel_init);
>  if (!rx_pool_channel) {
>  u32 pool;
>  int ret;
> 
>  ret = qman_alloc_pool();
> 
>  if (!ret)
>  rx_pool_channel = pool;
>  }
>  spin_unlock(_pool_channel_init);
>  if (!rx_pool_channel)
>  return -ENOMEM;
>  return rx_pool_channel;
> }
> 
> which always returns the same pool channel (e.g. 0x401) if successful.

That's correct.

> This means all the QMan portal_isr() are distributed round-robin to all
> affine portals. Is there some way to configure the software portal for a
> specific network interface, e.g. use processors 0, 1, 2, 3 for one
> interface,and 4, 5, 6, 7 for another?

We've stripped all configurability and advanced features from the initial
DPAA submission. We don't have options to configure this. On the other hand,
the ingress queues are held active in the portal, resulting in one CPU doing
dequeues while there are frames available. This is done to avoid frame 
reordering, improving termination performance (and forwarded TCP performance).

The downside is that we're not saturating all CPUs with traffic. We're
currently working on adding Rx hashing, to be able to maintain intra-flow
frame order while spreading processing on all CPUs.

Meanwhile, would RPS (Receive Packet Steering) be a solution for you?

> --
> Sebastian Huber, embedded brains GmbH
> 
> Address : Dornierstr. 4, D-82178 Puchheim, Germany
> Phone   : +49 89 189 47 41-16
> Fax : +49 89 189 47 41-09
> E-Mail  : sebastian.hu...@embedded-brains.de
> PGP : Public key available on request.
> 
> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.


Regards,
Madalin

RE: [PATCH] dt-bindings: net: move FMan binding

[Madalin-Cristian Bucur]

> -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 0/9] QorIQ DPAA 1 updates

[Madalin-Cristian Bucur]

> -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...

RE: [upstream-release] [PATCH net v3 2/4] powerpc: fsl/fman: remove fsl, fman from of_device_ids[]

[Madalin-Cristian Bucur]

> 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 = _dev->dev;
> >
> > +   /* call of_platform_populate in order to probe sub-nodes on arm64
> > */
> > +   err = of_platform_populate(fm_node, NULL, NULL, _dev->dev);
> > +   if (err) {
> > +   dev_err(_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[]

[Madalin-Cristian Bucur]

> 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

[Madalin-Cristian Bucur]

> 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 = _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, _dev->dev);
> > +   if (err) {
> > +   dev_err(_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: [v2, 2/3] powerpc/fsl/dts: add QMan and BMan portal nodes on t1024

[Madalin-Cristian Bucur]

> From: Scott Wood [mailto:o...@buserror.net]
> Sent: Wednesday, December 07, 2016 2:59 AM
> 
> On Tue, Dec 06, 2016 at 03:13:38PM +0200, Madalin Bucur wrote:
> > Signed-off-by: Madalin Bucur 
> > ---
> >  arch/powerpc/boot/dts/fsl/t1024qds.dts | 29
> +
> >  arch/powerpc/boot/dts/fsl/t1024rdb.dts | 33
> +
> >  2 files changed, 62 insertions(+)
> 
> So, in patch 1/3 you add qman and bman nodes to t1023si-post.dtsi and
> call it "add QMan and BMan portal nodes on t1023rdb" as if it were
> board-specific (the only board-specific part is the reserved-memory
> nodes, not the portals).
> 
> Then, in this patch you only touch board-specific files, and label it
> "add QMan and BMan portal nodes on t1024"...

That needs fixing...

> > diff --git a/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> b/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> > index 302cdd2..73a6453 100644
> > --- a/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> > +++ b/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> > @@ -41,6 +41,31 @@
> > #size-cells = <2>;
> > interrupt-parent = <>;
> >
> > +   aliases {
> > +   sg_2500_aqr105_phy4 = _2500_aqr105_phy4;
> > +   };
> 
> What does this have to do with the qman and bman portal nodes?  Why is
> this alias needed?
> 
> -Scott

It's needed by u-boot, should be a separate patch.
I'll resend.

Madalin

RE: [PATCH 3/3] powerpc/fsl/dts: add FMan node for t1042d4rdb

[Madalin-Cristian Bucur]

> From: Scott Wood [mailto:o...@buserror.net]
> Sent: Tuesday, November 15, 2016 8:19 AM
> 
> On Fri, 2016-11-11 at 17:53 +0200, Madalin Bucur wrote:
> > Signed-off-by: Madalin Bucur 
> > ---
> >  arch/powerpc/boot/dts/fsl/t1042d4rdb.dts | 47
> > 
> >  1 file changed, 47 insertions(+)
> >
> > diff --git a/arch/powerpc/boot/dts/fsl/t1042d4rdb.dts
> > b/arch/powerpc/boot/dts/fsl/t1042d4rdb.dts
> > index 2a5a90d..8c0c318 100644
> > --- a/arch/powerpc/boot/dts/fsl/t1042d4rdb.dts
> > +++ b/arch/powerpc/boot/dts/fsl/t1042d4rdb.dts
> > @@ -48,6 +48,53 @@
> >     "fsl,deepsleep-cpld";
> >     };
> >     };
> > +   soc: soc@ffe00 {
> 
> Please leave a blank line between nodes, especially here at the top level.
> 
> -Scott

I missed your feedback, will send a v2.

Thanks,
Madalin

RE: [PATCH net-next v7 03/10] dpaa_eth: add option to use one buffer pool set

[Madalin-Cristian Bucur]

> 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

[Madalin-Cristian Bucur]

> 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 <madalin.bu...@nxp.com>
> > 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 *)_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

[Madalin-Cristian Bucur]

> From: David Miller [mailto:da...@davemloft.net]
> Sent: Monday, November 07, 2016 6:40 PM
> 
> From: Madalin-Cristian Bucur <madalin.bu...@nxp.com>
> 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 <madalin.bu...@nxp.com>
> >> 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

[Madalin-Cristian Bucur]

> -Original Message-
> From: David Miller [mailto:da...@davemloft.net]
> Sent: Monday, November 07, 2016 5:55 PM
> 
> From: Madalin-Cristian Bucur <madalin.bu...@nxp.com>
> 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

[Madalin-Cristian Bucur]

> 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 *)_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 = >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 = >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 

RE: [net-next 08/13] fsl/fman: check pcsphy pointer before use

[Madalin-Cristian Bucur]

> -Original Message-
> From: David Laight [mailto:david.lai...@aculab.com]
> Sent: Tuesday, October 04, 2016 5:44 PM
> To: Madalin-Cristian Bucur <madalin.bu...@nxp.com>;
> net...@vger.kernel.org
> Cc: linuxdev.baldr...@gmail.com; linuxppc-dev@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: [v2] powerpc: Fix incorrect PPC32 PAMU dependency

[Madalin-Cristian Bucur]

> -Original Message-
> From: Andy Fleming 
> To: j...@8bytes.org
> Cc: io...@lists.linux-foundation.org, linuxppc-dev@lists.ozlabs.org
> Subject: [v2] powerpc: Fix incorrect PPC32 PAMU dependency
>
> The Freescale PAMU can be enabled on both 32 and 64-bit Power
> chips. Commit 477ab7a19cec8409e4e2dd10e7348e4cac3c06e5
> (iommu: Make more drivers depend on COMPILE_TEST)
> restricted PAMU to PPC32. PPC covers both.
>
> Signed-off-by: Andy Fleming 

Tested-by: Madalin Bucur 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [v9, 6/6] fsl/fman: Add FMan MAC driver

[Madalin-Cristian Bucur]

> -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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [net-next v5 0/8] dpaa_eth: Add the Freescale DPAA Ethernet driver

[Madalin-Cristian Bucur]

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: net...@vger.kernel.org; linuxppc-dev@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.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [net-next v4 4/8] dpaa_eth: add driver's Tx queue selection

[Madalin-Cristian Bucur]

> -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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [net-next v4 2/8] dpaa_eth: add support for DPAA Ethernet

[Madalin-Cristian Bucur]

> -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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [net-next v4 3/8] dpaa_eth: add support for S/G frames

[Madalin-Cristian Bucur]

> -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, _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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [net-next v4 2/8] dpaa_eth: add support for DPAA Ethernet

[Madalin-Cristian Bucur]

> -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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [v3 1/8] devres: add devm_alloc_percpu()

[Madalin-Cristian Bucur]

> -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 <madalin.bu...@freescale.com>
> > Tested-by: Madalin-Cristian Bucur <madalin.bu...@freescale.com>
> > ---
> >  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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [v2 8/9] dpaa_eth: add debugfs entries

[Madalin-Cristian Bucur]

 -Original Message-
 From: David Miller [mailto:da...@davemloft.net]
 
 From: Madalin Bucur madalin.bu...@freescale.com
 Date: Wed, 5 Aug 2015 18:41:28 +0300
 
  Export per CPU counters through debugfs.
 
  Signed-off-by: Madalin Bucur madalin.bu...@freescale.com
 
 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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

SDK/ethernet git repository branches

[Madalin-Cristian Bucur]

Here is a short description of the SDK/ethernet repository branches used for 
the DPAA
drivers and Gianfar Ethernet driver.

:: DPAA Upstreaming branches:

:. ldup
 - development branch for the DPAA upstreamable drivers (QBMan, FMan, dpaa_eth)
 - changes are staged here for submission to the upstream mailing list for 
review 
acceptance
 - automated tests are run in 3 stages on this branch when new changes are 
pushed
 - periodically rebased over the latest vanilla Linux kernel tag

:. ldup-sdk
 - development branch for the DPAA upstreamable drivers second wave content
 - changes staged here will not be sent in the first patch set or are meant for 
long
term integration of SDK code with upstreamable drivers
 - automated tests are run in 3 stages on this branch when new changes are 
pushed
 - periodically rebased over ldup; currently is left behind, working on getting 
back on
top of ldup

:: DPAA SDK branches:

:. dpaa-next
 - default development branch for the SDK DPAA drivers
 - changes are staged here for inclusion in the SDK Linux kernel tree
 - automated tests are run in 3 stages on this branch when new changes are 
pushed
 - periodically rebased over the SDK Linux kernel tree

:: DPAA LS1043 support branches:

:. dpaa-arm-next
 - development branch for the LS1043 support in the dpaa_eth drivers
 - changes were staged here for inclusion in the LS1043 Linux kernel tree held 
by Mingkai
 - this will disappear when LS1043 support is included in the SDK, its role 
will be fulfilled
by dpaa-next
 - was periodically rebased over dpaa-next
 - needs to be brought to date with changes from linux-v3.19-dpaa

:. linux-v3.19-dpaa
 - when LS1043 development continued on a different kernel version, dpaa-next
no longer was able to fulfill its role while still remaining based on 
dpaa-next
 - here the hombrew LS1043 support branch prepared by Mingkai was updated to
contain the SDK1.8 codebase plus several patches that were required by the 
kernel
upgrade and some ARM specific patches to the Ethernet drivers that were not 
included
in SDK1.8
 - the branch was used as a temporary staging area for the LS1043 DPAA Ethernet 
drivers
code until it was pulled in the official LS1043 Linux tree 
ls1043-linux/linux-v3.19

:: Gianfar branch:

:. gfar-next
 - branch based on the vanilla kernel
 - currently based on v4.1 Linux kernel tag
 - automated testing setup in progress

:: Branches to add

:. dpaa-next-2.0
 - branch does not exist, it will be created for SDK1.9 development
 - will be based on the SDK 1.9 official tree when that will be communicated
 - changes will be pulled from dpaa-next

:. dpaa-next-2.0
 - branch does not exist, it will be created for SDK2.0 development
 - will be based on the SDK 2.0 official tree when that will be communicated
 - changes will be pulled from dpaa-next and ldup

:: Branches to remove
 - temporary branches used as temporary vehicles for the DPAA teams 
collaboration may be added here
 - these temporary branches need to be removed when their purpose was met

:. qmanv2_squashed
 - changes to ldup QBMan patches were pushed here by Roy for review  inclusion 
in ldup

:. qman_v2
 - changes to ldup QBMan patches were pushed here by Roy for review  inclusion 
in ldup

Regards,
Madalin

PS: this information will be updated in Sharepoint at this address:

https://freescale.sharepoint.com/sites/dngsst/nrt/sdktech/ethconn/EthernetConnectivity/SDK%20ethernet%20git%20repository%20branches.aspx

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [PATCH 03/10] dpaa_eth: add configurable bpool thresholds

[Madalin-Cristian Bucur]

 -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; net...@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 madalin.bu...@freescale.com
 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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [PATCH 02/10] dpaa_eth: add support for DPAA Ethernet

[Madalin-Cristian Bucur]

 -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

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [PATCH 03/10] dpaa_eth: add configurable bpool thresholds

[Madalin-Cristian Bucur]

 -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
 
 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [PATCH RFC 02/10] dpaa_eth: add support for DPAA Ethernet

[Madalin-Cristian Bucur]

Hi Joakim

 -Original Message-
 From: Joakim Tjernlund [mailto:joakim.tjernl...@transmode.se]
 Sent: Monday, July 20, 2015 10:57 AM
 To: linuxppc-dev@lists.ozlabs.org; net...@vger.kernel.org; Bucur Madalin-
 Cristian-B32716
 Cc: linux-ker...@vger.kernel.org
 Subject: Re: [PATCH RFC 02/10] dpaa_eth: add support for DPAA Ethernet
 
 On Mon, 2015-07-20 at 09:54 +0200, Joakim Tjernlund wrote:
  On Wed, 2015-04-01 at 19:19 +0300, Madalin Bucur wrote:
   This introduces the Freescale Data Path Acceleration Architecture
   (DPAA) Ethernet driver (dpaa_eth) that builds upon the DPAA QMan,
   BMan, PAMU and FMan drivers to deliver Ethernet connectivity on
   the Freescale DPAA QorIQ platforms.
  
   Signed-off-by: Madalin Bucur madalin.bu...@freescale.com
   ---
  
   + snprintf(net_dev-name, IFNAMSIZ, fm%d-mac%d,
   +  dpa_mac_fman_index_get(pdev),
   +  dpa_mac_hw_index_get(pdev));
 
  Should ethernet drivers dictate interface name in user space nowadays?
  I would prefer if you didn't.

The preformatted interface name was thought as a helper for quick interface
identification. It also ensures constant naming of the interfaces, i.e. if you
add/remove PCI network cards. One can make use of udev rules to override
default interface names (eth%d) in userspace.

Another reason for using this is that the interface name was also used for the
debugfs file name and when compiling dpaa_eth as a module there was a
problem with udev concurrently renaming interfaces from eth0 to something
like fmx-macy, making the next probed DPAA interface temporarily get the
eth0 name (before being renamed fmx-macw). Subsequently,
the debugfs_create_file(net_dev-name,...) call failed because of duplicated
names.

If this is considered more of a bug than a feature, I can remove it and only 
change
the naming of the debugfs entries to avoid the udev issue.

  I am trying these patches on a custom T1042 board using Linux 4.1 but
  I cannot get Fixed PHY to work:
  libphy: PHY fixed-0:00 not found
  fsl_dpa dpaa-ethernet.2 eth2: Could not connect to PHY fixed-0:00
  fsl_dpa dpaa-ethernet.2 eth2: init_phy() = -19
 
  Not sure what I have missed here, any ideas?
 
 I meant I am using
 http://git.freescale.com/git/cgit.cgi/ppc/upstream/linux.git/
 on top of 4.1
 
  Jocke

Please make sure you have CONFIG_FIXED_PHY=y in your .config.
Can you please share the device tree part where you've added the fixed-link 
entry?

Madalin
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [RFC,v3,12/12] fsl/fman: Add FMan MAC driver

[Madalin-Cristian Bucur]

Hi Joakim,

It seems we just need to align to the API introduced by Thomas Petazzoni
in 3be2a49e.

Madalin

 -Original Message-
 From: Joakim Tjernlund [mailto:joakim.tjernl...@transmode.se]
 Sent: Monday, July 20, 2015 3:16 PM
 To: net...@vger.kernel.org; Liberman Igal-B31950
 Cc: linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org; Bucur
 Madalin-Cristian-B32716
 Subject: Re: [RFC,v3,12/12] fsl/fman: Add FMan MAC driver
 
 On Wed, 2015-04-29 at 12:29 +0300, Igal.Liberman wrote:
  From: Igal Liberman igal.liber...@freescale.com
 
  This patch adds the Ethernet MAC driver support.
 
  Signed-off-by: Igal Liberman igal.liber...@freescale.com
  ---
   drivers/net/ethernet/freescale/fman/inc/mac.h |  125 +
   drivers/net/ethernet/freescale/fman/mac/Makefile  |3 +-
   drivers/net/ethernet/freescale/fman/mac/mac-api.c |  605
 +
   drivers/net/ethernet/freescale/fman/mac/mac.c |  527
 ++
   4 files changed, 1259 insertions(+), 1 deletion(-)
   create mode 100644 drivers/net/ethernet/freescale/fman/inc/mac.h
   create mode 100644 drivers/net/ethernet/freescale/fman/mac/mac-api.c
   create mode 100644 drivers/net/ethernet/freescale/fman/mac/mac.c
 
  diff --git a/drivers/net/ethernet/freescale/fman/inc/mac.h
 b/drivers/net/ethernet/freescale/fman/inc/mac.h
  new file mode 100644
  index 000..2d27331
  --- /dev/null
  +++ b/drivers/net/ethernet/freescale/fman/inc/mac.h
 .
  +   /* Get the rest of the PHY information */
  +   mac_dev-phy_node = of_parse_phandle(mac_node, phy-handle,
 0);
  +   if (!mac_dev-phy_node) {
  +   int sz;
  +   const u32 *phy_id = of_get_property(mac_node, fixed-
 link,
  +   sz);
  +   if (!phy_id || sz  sizeof(*phy_id)) {
  +   dev_err(dev, No PHY (or fixed link) found\n);
  +   _errno = -EINVAL;
  +   goto _return_dev_set_drvdata;
  +   }
  +
  +   sprintf(mac_dev-fixed_bus_id, PHY_ID_FMT, fixed-0,
  +   phy_id[0]);
  +   }
 
 The above for fixed PHY does not work for me, changing it to does:
 
 diff --git a/drivers/net/ethernet/freescale/fman/mac/mac.c
 b/drivers/net/ethernet/freescale/fman/mac/mac.c
 index 4eb8f7c..a8be96a 100644
 --- a/drivers/net/ethernet/freescale/fman/mac/mac.c
 +++ b/drivers/net/ethernet/freescale/fman/mac/mac.c
 @@ -42,6 +42,7 @@
  #include linux/module.h
  #include linux/of_address.h
  #include linux/of_platform.h
 +#include linux/of_mdio.h
  #include linux/of_net.h
  #include linux/device.h
  #include linux/phy.h
 @@ -399,7 +400,7 @@ static int __cold mac_probe(struct platform_device
 *_of_dev)
 
 /* Get the rest of the PHY information */
 mac_dev-phy_node = of_parse_phandle(mac_node, phy-handle, 0);
 -   if (!mac_dev-phy_node) {
 +   if (0  !mac_dev-phy_node) {
 int sz;
 const u32 *phy_id = of_get_property(mac_node, fixed-link,
 sz);
 @@ -412,6 +413,16 @@ static int __cold mac_probe(struct platform_device
 *_of_dev)
 sprintf(mac_dev-fixed_bus_id, PHY_ID_FMT, fixed-0,
 phy_id[0]);
 }
 +   if (!mac_dev-phy_node  of_phy_is_fixed_link(mac_node)) {
 +   /*
 +* In the case of a fixed PHY, the DT node associated
 +* to the PHY is the Ethernet MAC DT node.
 +*/
 +   _errno = of_phy_register_fixed_link(mac_node);
 +   if (_errno)
 +   return _errno;
 +   mac_dev-phy_node = of_node_get(mac_node);
 +   }
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [RFC,v3,12/12] fsl/fman: Add FMan MAC driver

[Madalin-Cristian Bucur]

 -Original Message-
 From: Joakim Tjernlund [mailto:joakim.tjernl...@transmode.se]
 Sent: Monday, July 20, 2015 3:57 PM
 To: net...@vger.kernel.org; Liberman Igal-B31950; Bucur Madalin-Cristian-
 B32716
 Cc: linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org
 Subject: Re: [RFC,v3,12/12] fsl/fman: Add FMan MAC driver
 
 On Mon, 2015-07-20 at 12:28 +, Madalin-Cristian Bucur wrote:
  Hi Joakim,
 
  It seems we just need to align to the API introduced by Thomas Petazzoni
  in 3be2a49e.
 
  Madalin
 
 So it seems, any idea when the next spin will be ready?
 Could you also push it onto
   http://git.freescale.com/git/cgit.cgi/ppc/upstream/linux.git/
 ?
 
  Jocke

We're working on addressing all the feedback received to date (you've just added
a bit more) then we'll re-submit the FMan driver together with the DPAA Ethernet
driver. A push in the public git is also going to take place after the patches 
are sent
for review.

Madalin
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [RFC,v3 02/10] dpaa_eth: add support for DPAA Ethernet

[Madalin-Cristian Bucur]

 -Original Message-
 From: Eric Dumazet [mailto:eric.duma...@gmail.com]
 
 On Wed, 2015-06-10 at 07:38 +, Madalin-Cristian Bucur wrote:
  Hi Eric,
 
  Can you please tell us if this change would be for the better?
 
  I was about to say yes to this request but checked and no other Ethernet
 driver seems to use the queue trans_start.
  I was able to find your patch net: tx scalability works : trans_start [
 http://patchwork.ozlabs.org/patch/27104/ ] but did not find more about this
 topic.
 
 
 Drivers no longer have to worry about updating trans_start themselves.
 Core networking stack handles this.
 
 Check txq_trans_update() done from netdev_start_xmit()

I'll remove the updates, thank you.

Madalin
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [RFC,v3 02/10] dpaa_eth: add support for DPAA Ethernet

[Madalin-Cristian Bucur]

 -Original Message-
 From: Eric Dumazet [mailto:eric.duma...@gmail.com]
 
 On Wed, 2015-04-29 at 17:56 +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.
 ...
 
  +   /* We're going to store the skb backpointer at the beginning
  +* of the data buffer, so we need a privately owned skb
  +*/
  +
  +   /* Code borrowed from skb_unshare(). */
  +   if (skb_cloned(skb)) {
  +   struct sk_buff *nskb = skb_copy(skb, GFP_ATOMIC);
  +
  +   /* Finally, create a contig FD from this skb */
  +   skb_to_contig_fd(priv, skb, fd, countptr, offset);
  +
  +   kfree_skb(skb);
  +   skb = nskb;
  +   /* skb_copy() has now linearized the skbuff. */
  +   }
  +
 
 You know that TCP packets are clones, right ?
 This code is killing performance.
 
 TCP allows the headers part being modified by a driver if needed.
 
 You should use skb_header_cloned() instead.

Thank you, I'll address this. I plan to do something like this:

+   if (!nonlinear) {
+   /* We're going to store the skb backpointer at the beginning
+* of the data buffer, so we need a privately owned skb
+*/
+
+   /* make sure skb is not shared, skb_cow_head() assumes it's not 
*/
+   skb = skb_share_check(skb, GFP_ATOMIC);
+   if (!skb)
+   goto enomem;
+
+   /* verify the skb head is not cloned */
+   if (skb_cow_head(skb, priv-tx_headroom))
+   goto enomem;
+
+   nonlinear = skb_is_nonlinear(skb);
+   }

but I'm not sure the skb_share_check() is required on tx.
I'm also a bit puzzled by the aliasing between shared and cloned terms
(i.e. skb_unshare() could be named something like skb_unclone();
 the skb_share_check() not only checks but also unshares an skb so
 the already taken skb_unshare() name would probably fit too).

Madalin
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [RFC,v3 02/10] dpaa_eth: add support for DPAA Ethernet

[Madalin-Cristian Bucur]

Hi Eric,

Can you please tell us if this change would be for the better?

I was about to say yes to this request but checked and no other Ethernet driver 
seems to use the queue trans_start.
I was able to find your patch net: tx scalability works : trans_start [ 
http://patchwork.ozlabs.org/patch/27104/ ] but did not find more about this 
topic.
 
Thank you,
Madalin

 -Original Message-
 From: Xie Jianhua-B29408
 Sent: Wednesday, June 10, 2015 9:00 AM
 To: Bucur Madalin-Cristian-B32716; net...@vger.kernel.org; linuxppc-
 d...@lists.ozlabs.org
 Cc: linux-ker...@vger.kernel.org
 Subject: RE: [RFC,v3 02/10] dpaa_eth: add support for DPAA Ethernet
 
 
 
  -Original Message-
  From: Linuxppc-dev [mailto:linuxppc-dev-
  bounces+jianhua.xie=freescale@lists.ozlabs.org] On Behalf Of
 Madalin
  Bucur
  Sent: Wednesday, April 29, 2015 10:57 PM
  To: net...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
  Cc: linux-ker...@vger.kernel.org; Bucur Madalin-Cristian-B32716
  Subject: [RFC,v3 02/10] dpaa_eth: add support for DPAA Ethernet
 
  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.
 
 Snip..
 
  +
  +   if (unlikely(dpa_xmit(priv, percpu_stats, queue_mapping, fd)  0))
  +   goto xmit_failed;
  +
  +   net_dev-trans_start = jiffies;
 
 It is probably better to use netdev_queue-trans_start to instead of
 net_dev-trans_start on SMP.
 
 Best Regards,
 Jianhua
 
  +   return NETDEV_TX_OK;
  +
  +xmit_failed:
  +   if (fd.cmd  FM_FD_CMD_FCO) {
  +   (*countptr)--;
  +   dpa_fd_release(net_dev, fd);
  +   percpu_stats-tx_errors++;
  +   return NETDEV_TX_OK;
  +   }
  +   _dpa_cleanup_tx_fd(priv, fd);
  +   percpu_stats-tx_errors++;
  +   dev_kfree_skb(skb);
  +   return NETDEV_TX_OK;
  +}
  --
  1.7.11.7
 
  ___
  Linuxppc-dev mailing list
  Linuxppc-dev@lists.ozlabs.org
  https://lists.ozlabs.org/listinfo/linuxppc-dev
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [PATCH] powerpc/fsl: Add FMan best effort port compatible

[Madalin-Cristian Bucur]

 -Original Message-
 From: Wood Scott-B07421
 
 The compatible string should describe what programming interface is
 present.  Other information should be in other properties.  Having the
 same compatible for tx and rx definitely seems wrong.
 
 -Scott
 

Hi Scott, what we tried to accomplish here is to avoid updating u-boot for this 
new compatible and have minimal additions to the binding document.

We currently know that:

fsl,fman-v3-port-rx  is an Rx port that can work in 1G mode
fsl,fman-v3-port-tx  is an Tx port that can work in 1G mode

We proposed adding a new optional compatible fsl,fman-v3-best-effort-port  
such that:

fsl,fman-v3-port-rx  is an Rx port that can work in 1G mode
fsl,fman-v3-port-tx  is an Tx port that can work in 1G mode

fsl,fman-v3-port-rx, fsl,fman-v3-best-effort-port  is an Rx port that can 
work in 1G mode and also in limited resourced 10G mode
fsl,fman-v3-port-tx, fsl,fman-v3-best-effort-port  is an Tx port that can 
work in 1G mode and also in limited resourced 10G mode

The only code addition required is in the Linux driver, code that checks for 
the optional, supplemental parameter fsl,fman-v3-best-effort-port to change 
some resource allocations.

Madalin
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH RFC 02/10] dpaa_eth: add support for DPAA Ethernet

[Madalin-Cristian Bucur]

 -Original Message-
 From: Joe Perches [mailto:j...@perches.com]
 On Wed, 2015-04-01 at 19:19 +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.
 
 trivial notes:
 
  diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
 b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
 []
  @@ -0,0 +1,835 @@
  +/* Copyright 2008 - 2015 Freescale Semiconductor Inc.
  + *
  + * Redistribution and use in source and binary forms, with or without
  + * modification, are permitted provided that the following conditions are
 met:
  + * * Redistributions of source code must retain the above copyright
  + *  notice, this list of conditions and the following disclaimer.
  + * * Redistributions in binary form must reproduce the above copyright
  + *  notice, this list of conditions and the following disclaimer in the
  + *  documentation and/or other materials provided with the
 distribution.
  + * * Neither the name of Freescale Semiconductor nor the
  + *  names of its contributors may be used to endorse or promote
 products
  + *  derived from this software without specific prior written permission.
  + *
  + * ALTERNATIVELY, this software may be distributed under the terms of
 the
  + * GNU General Public License (GPL) as published by the Free Software
  + * Foundation, either version 2 of that License or (at your option) any
  + * later version.
 
 Given this is GPLed here, does the first block need to exist?

I'll replace that with this license text:

/* Freescale(R) DPAA Ethernet Linux driver
 * Copyright 2008 - 2015 Freescale Semiconductor Inc.
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License
 * as published by the Free Software Foundation; either version 2
 * of the License, or (at your option) any later version.
 * 
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 * 
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, see http://www.gnu.org/licenses/.
 */

  +#define pr_fmt(fmt) \
  +   KBUILD_MODNAME :  fmt
 
 single line please

I will address this.

  +#include linux/if_arp.h  /* arp_hdr_len() */
  +#include linux/if_vlan.h /* VLAN_HLEN */
  +#include linux/icmp.h/* struct icmphdr */
  +#include linux/ip.h  /* struct iphdr */
  +#include linux/ipv6.h/* struct ipv6hdr */
  +#include linux/udp.h /* struct udphdr */
  +#include linux/tcp.h /* struct tcphdr */
  +#include linux/net.h /* net_ratelimit() */
  +#include linux/if_ether.h/* ETH_P_IP and ETH_P_IPV6 */
 
 These comments are pretty unusual and likely incomplete
 so they're probably not useful.

I will clean-up all files.

  +static u8 debug = -1;
  +module_param(debug, byte, S_IRUGO);
  +MODULE_PARM_DESC(debug, Module/Driver verbosity level);
 
 default on?  Likely default off would be better.

Found several drivers that keep it by default enabled, should I change it to 
off?

  +static void _dpa_rx_error(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)
  +{
  +   /* limit common, possibly innocuous Rx FIFO Overflow errors'
  +* interference with zero-loss convergence benchmark results.
  +*/
  +   if (likely(fd-status  FM_FD_STAT_ERR_PHYSICAL))
  +   pr_warn_once(non-zero error counters in fman statistics
 (sysfs)\n);
  +   else
  +   if (netif_msg_hw(priv)  net_ratelimit())
  +   netdev_err(net_dev, Err FD status = 0x%08x\n,
  +  fd-status  FM_FD_STAT_RX_ERRORS);
 
   if (netif_msg_foo(priv))
   netdev_level(netdev, ...)
 
 uses can be written like:
 
   netif_level(priv, foo, netdev, ...);
 
 So this is perhaps better as
 
   if (likely(fd-status  FM_FD_STAT_ERR_PHYSICAL))
   pr_warn_once(non-zero error counters in fman statistics
 (sysfs)\n);
   else if (net_ratelimit())
   netif_err(priv, hw, net_dev, Err FD status = 0x%08x\n,
 fd-status  FM_FD_STAT_RX_ERRORS);

Thank you, will try to address all the occurrences.

  +
  +   percpu_priv-stats.rx_errors++;
  +
  +   dpa_fd_release(net_dev, fd);,
  +}
  +
  +static void _dpa_tx_error(struct net_device*net_dev,
  + const struct dpa_priv_s   *priv,
  + struct dpa_percpu_priv_s  

Re: [PATCH RFC 06/10] dpaa_eth: add ethtool functionality

[Madalin-Cristian Bucur]

 -Original Message-
 From: Joe Perches [mailto:j...@perches.com]
 On Wed, 2015-04-01 at 19:19 +0300, Madalin Bucur wrote:
  Add support for basic ethtool operations.
 
  diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
 b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
 []
  +static int __cold dpa_get_settings(struct net_device *net_dev,
  +  struct ethtool_cmd *et_cmd)
  +{
  +   int  _errno;
 
 Using a variable name of _errno is misleading at best,
 (btw: the only return value for phy_ethtool_gset is 0)

I'll rename all miss-used occurrences of _errno.
 
  +static void __cold dpa_get_drvinfo(struct net_device *net_dev,
  +  struct ethtool_drvinfo *drvinfo)
  +{
  +   int  _errno;
  +
  +   strncpy(drvinfo-driver, KBUILD_MODNAME,
  +   sizeof(drvinfo-driver) - 1)[sizeof(drvinfo-driver) - 1] = 0;
 
 That's a really odd and unusual construct
 more commonly written as strlcpy

I will change this.

  +   _errno = snprintf(drvinfo-version, sizeof(drvinfo-version),
 
 Using errno here is especially misleading as that's
 not the return value of an snprintf

I will take care of this one too.

Thank you,
Madalin
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [PATCH RFC 02/10] dpaa_eth: add support for DPAA Ethernet

[Madalin-Cristian Bucur]

 -Original Message-
 From: Paul Bolle [mailto:pebo...@tiscali.nl]
 
 Just a few nits.
 
 This series is posted as an RFC, so this might not be what you're
 expecting right now. But as these messages got tangled up in my mail
 filter anyhow, I thought I might as well bother you with these nits now.
 
 On Wed, 2015-04-01 at 19:19 +0300, Madalin Bucur wrote:
  --- /dev/null
  +++ b/drivers/net/ethernet/freescale/dpaa/Kconfig
  @@ -0,0 +1,49 @@
  +menuconfig FSL_DPAA_ETH
  +   tristate DPAA Ethernet
  +   depends on FSL_SOC  FSL_BMAN  FSL_QMAN  FSL_FMAN
  +   select PHYLIB
  +   select FSL_FMAN_MAC
  +   ---help---
  + Data Path Acceleration Architecture Ethernet driver,
  + supporting the Freescale QorIQ chips.
  + Depends on Freescale Buffer Manager and Queue Manager
  + driver and Frame Manager Driver.
  +
  +if FSL_DPAA_ETH
  +
  +config FSL_DPAA_CS_THRESHOLD_1G
  +   hex Egress congestion threshold on 1G ports
  +   depends on FSL_DPAA_ETH
 
 This entry is inside the if FSL_DPAA_ETH block. So this line should be
 superfluous.

Will address this and the other occurrences.

  +   range 0x1000 0x1000
  +   default 0x0600
  +   ---help---
  + The size in bytes of the egress Congestion State notification
 threshold on 1G ports.
  + The 1G dTSECs can quite easily be flooded by cores doing Tx in a
 tight loop
  + (e.g. by sending UDP datagrams at while(1) speed),
  + and the larger the frame size, the more acute the problem.
  + So we have to find a balance between these factors:
  +  - avoiding the device staying congested for a prolonged time
 (risking
  + the netdev watchdog to fire - see also the tx_timeout 
  module
 param);
  +   - affecting performance of protocols such as TCP, which 
  otherwise
  +behave well under the congestion notification mechanism;
  +  - preventing the Tx cores from tightly-looping (as if the
 congestion
  +threshold was too low to be effective);
  +  - running out of memory if the CS threshold is set too high.
  +
  +config FSL_DPAA_CS_THRESHOLD_10G
  +   hex Egress congestion threshold on 10G ports
  +   depends on FSL_DPAA_ETH
 
 Ditto.
 
  +   range 0x1000 0x2000
  +   default 0x1000
  +   ---help ---
  + The size in bytes of the egress Congestion State notification
 threshold on 10G ports.
  +
  +config FSL_DPAA_INGRESS_CS_THRESHOLD
  +   hex Ingress congestion threshold on FMan ports
  +   depends on FSL_DPAA_ETH
 
 Ditto.
 
  +   default 0x1000
  +   ---help---
  + The size in bytes of the ingress tail-drop threshold on FMan ports.
  + Traffic piling up above this value will be rejected by QMan and
 discarded by FMan.
  +
  +endif # FSL_DPAA_ETH
 
 A similar comment can be made for the entries added in 03/10, 05/10, and
 09/10.
 
 Thanks,
 
 
 Paul Bolle

Thank you,
Madalin
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [PATCH RFC 10/10] dpaa_eth: add trace points

[Madalin-Cristian Bucur]

 -Original Message-
 From: Joe Perches [mailto:j...@perches.com]

 On Wed, 2015-04-01 at 19:19 +0300, Madalin Bucur wrote:
  Add trace points on the hot processing path.
 
 more trivia:
 
  diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h
 b/drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h
 []
  +#define fd_format_name(format) { qm_fd_##format, #format }
  +#define fd_format_list \
  +   fd_format_name(contig), \
  +   fd_format_name(sg)
 
 Are these used anywhere?

Yes, by the Frame Descriptor print:

/* This is what gets printed when the trace event is triggered */
TP_printk(TR_FMT,
  __get_str(name), __entry-fqid, __entry-fd_addr,
  __print_symbolic(__entry-fd_format, fd_format_list), 
// -- here
  __entry-fd_offset, __entry-fd_length, __entry-fd_status)


  +#define TR_FMT [%s] fqid=%d, fd: addr=0x%llx, format=%s, off=%u,
 len=%u, \
  +status=0x%08x
 
 It's nicer to coalesce string fragments.
 
 Unless this is intended to be used more than once,
 perhaps it's better to remove it and use the string
 directly instead.
 
  +   /* This is what gets printed when the trace event is triggered */
  +   TP_printk(TR_FMT,
  + __get_str(name), __entry-fqid, __entry-fd_addr,
  + __print_symbolic(__entry-fd_format, fd_format_list),
  + __entry-fd_offset, __entry-fd_length, __entry-
 fd_status)
 

Checkpatch seems to be less forgiving if the long string is not in a printk:

WARNING: line over 80 characters
#22: FILE: drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h:47:
+#define TR_FMT [%s] fqid=%d, fd: addr=0x%llx, format=%s, off=%u, len=%u, 
status=0x%08x

Madalin
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: RE: [PATCH RFC 02/11] dpaa_eth: add support for DPAA Ethernet

[Madalin-Cristian Bucur]

 -Original Message-
 From: Kumar Gala [mailto:ga...@kernel.crashing.org]
 
 On Mar 17, 2015, at 1:58 PM, Madalin Bucur madalin.bu...@freescale.com
 wrote:
 
  This introduces the Freescale Data Path Acceleration Architecture
  (DPAA) Ethernet driver (dpaa_eth) that builds upon the DPAA QMan,
  BMan, PAMU and FMan drivers to deliver Ethernet connectivity on
  the Freescale DPAA QorIQ platforms.
 
  Signed-off-by: Madalin Bucur madalin.bu...@freescale.com
  ---

[snip]

  diff --git a/drivers/net/ethernet/freescale/Makefile
 b/drivers/net/ethernet/freescale/Makefile
  index 71debd1..b6c10ab 100644
  --- a/drivers/net/ethernet/freescale/Makefile
  +++ b/drivers/net/ethernet/freescale/Makefile
  @@ -12,6 +12,7 @@ obj-$(CONFIG_FS_ENET) += fs_enet/
  obj-$(CONFIG_FSL_PQ_MDIO) += fsl_pq_mdio.o
  obj-$(CONFIG_FSL_XGMAC_MDIO) += xgmac_mdio.o
  obj-$(CONFIG_GIANFAR) += gianfar_driver.o
  +obj-$(if $(CONFIG_FSL_DPAA_ETH),y) += dpaa/
 
 Why isn't
 
 obj-$(CONFIG_FSL_DPAA_ETH)    += dpaa/
 
 enough?

It is, will fix.

  diff --git a/drivers/net/ethernet/freescale/dpaa/Kconfig
 b/drivers/net/ethernet/freescale/dpaa/Kconfig
  new file mode 100644
  index 000..7ef703c
  --- /dev/null
  +++ b/drivers/net/ethernet/freescale/dpaa/Kconfig
  @@ -0,0 +1,49 @@
  +menuconfig FSL_DPAA_ETH
  +  tristate DPAA Ethernet
  +  depends on FSL_SOC  FSL_BMAN  FSL_QMAN  FSL_FMAN
  +  select PHYLIB
  +  select FSL_FMAN_MAC
  +  ---help---
  +    Data Path Acceleration Architecture Ethernet driver,
  +    supporting the Freescale QorIQ chips.
  +    Depends on Freescale Buffer Manager and Queue Manager
  +    driver and Frame Manager Driver.
  +
  +if FSL_DPAA_ETH
  +
  +config FSL_DPAA_CS_THRESHOLD_1G
  +  hex Egress congestion threshold on 1G ports
  +  depends on FSL_DPAA_ETH
  +  range 0x1000 0x1000
  +  default 0x0600
  +  ---help---
  +    The size in bytes of the egress Congestion State notification
 threshold on 1G ports.
  +    The 1G dTSECs can quite easily be flooded by cores doing Tx in a
 tight loop
  +    (e.g. by sending UDP datagrams at while(1) speed),
  +    and the larger the frame size, the more acute the problem.
  +    So we have to find a balance between these factors:
  +     - avoiding the device staying congested for a prolonged time
 (risking
  + the netdev watchdog to fire - see also the tx_timeout 
  module
 param);
  +   - affecting performance of protocols such as TCP, which 
  otherwise
  +   behave well under the congestion notification mechanism;
  +     - preventing the Tx cores from tightly-looping (as if the
 congestion
  +   threshold was too low to be effective);
  +     - running out of memory if the CS threshold is set too high.
  +
  +config FSL_DPAA_CS_THRESHOLD_10G
  +  hex Egress congestion threshold on 10G ports
  +  depends on FSL_DPAA_ETH
  +  range 0x1000 0x2000
  +  default 0x1000
  +  ---help ---
  +    The size in bytes of the egress Congestion State notification
 threshold on 10G ports.
  +
  +config FSL_DPAA_INGRESS_CS_THRESHOLD
  +  hex Ingress congestion threshold on FMan ports
  +  depends on FSL_DPAA_ETH
  +  default 0x1000
  +  ---help---
  +    The size in bytes of the ingress tail-drop threshold on FMan ports.
  +    Traffic piling up above this value will be rejected by QMan and
 discarded by FMan.
  +
 
 Do these thresholds really need to be kconfig options?  Are they not
 changeable at runtime?

Probably Bogdan thought it was an easy way to provide basic means to tweak them
and also provide some explanation for them. The current setting is chosen to 
keep
them mostly out of the way but still address a corner case a customer reported, 
if
I remember correctly. No runtime change is supported.

  diff --git a/drivers/net/ethernet/freescale/dpaa/Makefile
 b/drivers/net/ethernet/freescale/dpaa/Makefile
  new file mode 100644
  index 000..bdeb04e
  --- /dev/null
  +++ b/drivers/net/ethernet/freescale/dpaa/Makefile
  @@ -0,0 +1,14 @@
  +#
  +# Makefile for the Freescale DPAA Ethernet controllers
  +#
  +ccflags-y += -DVERSION=\\
 
 Is this really needed, if so we need to fix that.

Unused, will remove.

[snip]
  +
  +#define DPA_DESCRIPTION FSL DPAA Ethernet driver
  +
  +MODULE_LICENSE(Dual BSD/GPL);
  +
  +MODULE_AUTHOR(Andy Fleming aflem...@freescale.com);
  +
  +MODULE_DESCRIPTION(DPA_DESCRIPTION);
 
 These are typically at the end of the file, and kept together (ie no blank 
 lines
 between them)

Will move them.

[snip]
  +
  +static struct platform_device_id dpa_devtype[] = {
  +  {
  +  .name = dpaa-ethernet,
  +  .driver_data = 0,
  +  }, {
  +  }
  +};
  +MODULE_DEVICE_TABLE(platform, dpa_devtype);
  +
  +static struct platform_driver dpa_driver = {
  +  .driver = {
  + 

RE: [PATCH] Documentation: bindings: net: DPAA corenet binding document

[Madalin-Cristian Bucur]

 -Original Message-
 From: Wood Scott-B07421
 Sent: Tuesday, December 02, 2014 6:40 AM
 On Fri, 2014-11-28 at 12:10 +0200, Madalin Bucur wrote:
  Add the device tree binding document for the DPAA corenet node
  and DPAA Ethernet nodes.
 
  Signed-off-by: Madalin Bucur madalin.bu...@freescale.com
  ---
   Documentation/devicetree/bindings/net/fsl-dpaa.txt | 31
 ++
   1 file changed, 31 insertions(+)
   create mode 100644 Documentation/devicetree/bindings/net/fsl-dpaa.txt
 
  diff --git a/Documentation/devicetree/bindings/net/fsl-dpaa.txt
 b/Documentation/devicetree/bindings/net/fsl-dpaa.txt
  new file mode 100644
  index 000..822c668
  --- /dev/null
  +++ b/Documentation/devicetree/bindings/net/fsl-dpaa.txt
  @@ -0,0 +1,31 @@
  +*DPAA corenet
  +
  +The corenet bus containing all DPAA Ethernet nodes.
 
 What does this have to do with corenet?
 
The corenet-generic platform code uses this compatible. Here are some excerpts
from the platform code found in SDK 
arch/powerpc/platforms/85xx/corenet_generic.c
...
 * Corenet based SoC DS Setup
 *
 * Maintained by Kumar Gala (see MAINTAINERS for contact information)
 *
 * Copyright 2009-2011 Freescale Semiconductor Inc.
...
static const struct of_device_id of_device_ids[] = {
{
.compatible = simple-bus
},
{
.compatible = fsl,dpaa
},
...
int __init corenet_gen_publish_devices(void)
{
return of_platform_bus_probe(NULL, of_device_ids, NULL);
}
...
  +Required property
  + - compatible: string property.  Must include fsl,dpaa. Can include
  +   also fsl,SoC-dpaa.
 
 No need for the SoC part.  As we previously discussed, the only
 purpose of this node is backwards compatibility with the U-Boot MAC
 address fixup -- if U-Boot doesn't look for the SoC version, then
 don't complicate things.
 
 Though, I can't find where U-Boot references this node.  Are you sure
 it's not using the ethernet%d aliases like everything else, in which
 case why do we need this node at all?
 
 -Scott
 

The initial (Freescale SDK) binding document contained those compatibles,
not sure what the initial intent was for the SoC variants.

The fsl,dpaa node is of interest to the DPAA Ethernet because it is
the parent of the fsl,dpa-ethernet nodes.

Madalin
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev