Re: [bug, bisected] pfifo_fast causes packet reordering

[Alexander Stein]

On Friday, March 16, 2018, 11:26:47 AM CET Jakob Unterwurzacher wrote:
> On 15.03.18 23:30, John Fastabend wrote:
> >> I have reproduced it using two USB network cards connected to each other. 
> >> The test tool sends UDP packets containing a counter and listens on the 
> >> other interface, it is available at
> >> https://github.com/jakob-tsd/pfifo_stress/blob/master/pfifo_stress.py
> >>
> > 
> > Great thanks, can you also run this with taskset to bind to
> > a single CPU,
> > 
> >   # taskset 0x1 ./pifof_stress.py
> > 
> > And let me know if you still see the OOO.
> 
> Interesting. Looks like it depends on which core it runs on. CPU0 is 
> clean, CPU1 is not.
> 
> Clean:taskset --cpu-list 0 ./pfifo_stress.py
> 
> Broken:   taskset --cpu-list 1 ./pfifo_stress.py
> 
> Maybe related: CPU0 is where USB interrupts are handled:
> 
> > root@rk3399-q7:~# cat /proc/interrupts   
> >CPU0   CPU1   CPU2   CPU3   CPU4   CPU5
> > 217:2175353  0  0  0  0  0 
> > GICv3 142 Level xhci-hcd:usb5

This reminds me somewhat of this thread: 
https://marc.info/?l=linux-can=148007442317274=2

Best regards,
Alexander

[PATCH 1/1] phy/micrel: Change phy_id_mask for KSZ8721

[Alexander Stein]

There are KSZ8721 PHYs with phy_id 0x00221619. In order to detect them
as PHY_ID_KSZ8001 compatible while staying different to PHY_ID_KSZ9021
ignore the last two bits when matching PHY_ID

Signed-off-by: Alexander Stein <alexander...@web.de>
---
 drivers/net/phy/micrel.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 5a8fefc25157..469247b9bdc7 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -839,7 +839,7 @@ static struct phy_driver ksphy_driver[] = {
 }, {
.phy_id = PHY_ID_KSZ8001,
.name   = "Micrel KSZ8001 or KS8721",
-   .phy_id_mask= 0x00ff,
+   .phy_id_mask= 0x00fc,
.features   = (PHY_BASIC_FEATURES | SUPPORTED_Pause),
.flags  = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
.driver_data= _type,
@@ -963,7 +963,7 @@ MODULE_LICENSE("GPL");
 static struct mdio_device_id __maybe_unused micrel_tbl[] = {
{ PHY_ID_KSZ9021, 0x000e },
{ PHY_ID_KSZ9031, MICREL_PHY_ID_MASK },
-   { PHY_ID_KSZ8001, 0x00ff },
+   { PHY_ID_KSZ8001, 0x00fc },
{ PHY_ID_KS8737, MICREL_PHY_ID_MASK },
{ PHY_ID_KSZ8021, 0x00ff },
{ PHY_ID_KSZ8031, 0x00ff },
-- 
2.9.2

Re: [PATCH 1/2] NET: PHY: Add PHY LED control binding.

[Alexander Stein]

On Wednesday 08 June 2016 14:30:08, Rob Herring wrote:
> > diff --git a/Documentation/devicetree/bindings/phy/phy-leds.txt
> > b/Documentation/devicetree/bindings/phy/phy-leds.txt new file mode 100644
> > index 000..1a35e3d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/phy-leds.txt
> > @@ -0,0 +1,52 @@
> > +LED configuration for Ethernet phys
> > +
> > +All these properties are optional, not all properties are supported by
> > +all PHYs. When more then one property name is define for one LED the
> > +order they get applied is device depended.
> > +Property names:
> > +   led-const-on: conditions the LED should be constant on
> > +   led-pulse: condition the LED should be pulsed on
> > +   led-blink-slow: condition the LED should slowly blink
> 
> How slow is slow?

This depends on the MMD.INTERNAL.LEDCH.SBF setting which is 2 Hz by default.

> > +   led-blink-fast: condition the LED should fast blink
> 
> How fast is fast?

This depends on the MMD.INTERNAL.LEDCH.FBF setting which is 16 Hz by default.

Both can be set independently to 2, 4, 8 or 16 Hz.

Best regards,
Alexander

Re: [PATCH 1/1 RFC] net/phy: Add Lantiq PHY driver

[Alexander Stein]

Hi Hauke,

On Monday 23 May 2016 09:12:54, Mehrtens, Hauke wrote:
> > On Thursday 19 May 2016 12:03:10, Mathias Kresin wrote:
> > > 2016-05-19 9:03 GMT+02:00 John Crispin <j...@phrozen.org>:
> > > > On 19/05/2016 08:57, Alexander Stein wrote:
> > > >> Thanks for the link, I wasn't aware of that patch. I like it in
> > > >> general, but there are some things I'd like to get addressed first:
> > > >> * vr9_gphy_of_reg_init() writes uncoditionally to led3h and led3l
> > > >> even on
> > > >> 
> > > >>   PEf7071 which does not have this register at all
> > > > 
> > > > we use this driver mainly on the 11g and 22f version. mathias
> > > > recently added the led3 handling.
> > > > 
> > > > @Mathias, can you have a look at this and fix it inside the lede tree
> > > > ?
> > > 
> > > Well, I haven't added the led3 handling, I've only changed the initial
> > > value (function) of led3.
> > > 
> > > Maybe it's cleaner to not use a default value for the led function and
> > > completely rely on the device tree bindings. But by adjusting the
> > > initial values, I had to change only the led function of one board in
> > > the openwrt xrx200 subtarget instead of touching all dts files.
> > 
> > I think setting default values is good.
> 
> The registers are set to some reset values after the chip is coming out of
> reset, but we should set  them all to the same value, Mathias said that all
> except for one board he knows are using only one LED per port, but they are
> often using different LED pins, I will change my patch.

One LED per port? I would think of using one RJ45 socket per port which 
usually have 2 LEDs.

> > > I know that the LTQ Datasheet for the PEF 7071 Version 1.5 mentions
> > > the led3 control register albeit there is no pin for a forth led. So I
> > > guess it's safe to write to the led3 register even for the PEF 7071.
> > 
> > Mh, my PEF 7071 User Manual (Version 2.0, 2012-10-17) doesn't mention
> > LED3x registers. There is LED3DA and LED3EN in PHY_LED but was removed in
> > 1.6 manual.
> 
> LED3x is only available in PEF 7072 which is a different package with more
> pins for the LED3 and some other interfaces.
> > I think, some flag if the PHY supports LED3 and depend on that is just
> > fine.
> I do not know how to distinguish between PEF 7071 and PEF 7072.

I expected that PEF 7072 would have a different PHY ID, but apparently this is 
not the case, though I don't have a datasheet for 7072. Is there really no way 
to distinguish those two?

Alexander

Re: [PATCH 1/1 RFC] net/phy: Add Lantiq PHY driver

[Alexander Stein]

On Thursday 19 May 2016 12:03:10, Mathias Kresin wrote:
> 2016-05-19 9:03 GMT+02:00 John Crispin <j...@phrozen.org>:
> > On 19/05/2016 08:57, Alexander Stein wrote:
> >> Thanks for the link, I wasn't aware of that patch. I like it in general,
> >> but there are some things I'd like to get addressed first:
> >> * vr9_gphy_of_reg_init() writes uncoditionally to led3h and led3l even on
> >> 
> >>   PEf7071 which does not have this register at all
> > 
> > we use this driver mainly on the 11g and 22f version. mathias recently
> > added the led3 handling.
> > 
> > @Mathias, can you have a look at this and fix it inside the lede tree ?
> 
> Well, I haven't added the led3 handling, I've only changed the initial
> value (function) of led3.
> 
> Maybe it's cleaner to not use a default value for the led function and
> completely rely on the device tree bindings. But by adjusting the
> initial values, I had to change only the led function of one board in
> the openwrt xrx200 subtarget instead of touching all dts files.

I think setting default values is good.

> I know that the LTQ Datasheet for the PEF 7071 Version 1.5 mentions
> the led3 control register albeit there is no pin for a forth led. So I
> guess it's safe to write to the led3 register even for the PEF 7071.

Mh, my PEF 7071 User Manual (Version 2.0, 2012-10-17) doesn't mention LED3x 
registers. There is LED3DA and LED3EN in PHY_LED but was removed in 1.6 
manual.
I think, some flag if the PHY supports LED3 and depend on that is just fine.

Best regards,
Alexander

Re: [PATCH 1/1 RFC] net/phy: Add Lantiq PHY driver

[Alexander Stein]

On Thursday 19 May 2016 09:03:26, John Crispin wrote:
> [ changing haukes mail addr to the intel one ]
> 
> On 19/05/2016 08:57, Alexander Stein wrote:
> > Hi John,
> > 
> > On Thursday 19 May 2016 06:50:56, John Crispin wrote:
> >> On 18/05/2016 18:24, Florian Fainelli wrote:
> >>> CC'ing Andrew, John,
> >> 
> >> also CC'ing Matthias and Hauke. we have had a driver in OpenWrt/LEDE for
> >> several years that seems a little more complete than this one.
> >> 
> >> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/lantiq/p
> >> atc
> >> hes-4.4/0023-NET-PHY-adds-driver-for-lantiq-PHY11G.patch;h=93bb4275ec1d2
> >> 61f3 98afb8fdc879c1dd973f997;hb=HEAD
> > 
> > Thanks for the link, I wasn't aware of that patch. I like it in general,
> > but there are some things I'd like to get addressed first:
> > * vr9_gphy_of_reg_init() writes uncoditionally to led3h and led3l even on
> > 
> >   PEf7071 which does not have this register at all
> 
> we use this driver mainly on the 11g and 22f version. mathias recently
> added the led3 handling.
> 
> @Mathias, can you have a look at this and fix it inside the lede tree ?
> 
> > * Why is PHY_HAS_INTERRUPT commented out everywhere?
> 
> legacy code, the old mips silicon had a bug and the internal phys irq
> lines worked unreliably so we used polling instead. rather than remove
> the code i just disabled that part. code is not cleaned up yet for
> upstream submission as you can tell :-)

Would you or Mathias mind dropping a cleaned up patch to netdev ml, cc'ing me? 
I can try it on our hardware using the 11g. Maybe I can even test the IRQ 
feature.

Regards,
Alexander

Re: [PATCH 1/1 RFC] net/phy: Add Lantiq PHY driver

[Alexander Stein]

On Wednesday 18 May 2016 19:01:09, Andrew Lunn wrote:
> > For LEDs, we had a patch series floating around adding LED triggers [1],
> > and it seems to me like the LEDs class subsystem would be a good fit for
> > controlling PHY LEDs, possibly with the help of PHYLIB when it comes to
> > doing the low-level work of registering LEDs and their names with the
> > LEDS subsystem.
> > 
> > [1]: http://lists.openwall.net/netdev/2016/03/23/61
> 
> That patch fizzled out. I got the feeling it was pushing the
> capabilities of the coder. I do however think it is a reasonable path
> to follow for PHY LEDs.
> 
> I took a quick look at the datasheet and the controlling of the LEDs
> is very flexible. It should not be a problem to expose some of that
> functionality via LED triggers.

To be honest I don't know how the PHY LEDs could be set by LED triggers. 
Wouldn't that require to create triggers for each value which can be written 
to LEDxH and LEDxL? In my case the hardware requires some specific setting due 
to LED connections.
Of course making the LED configuration changeable at runtime would be awesome.

Best regards,
Alexander

Re: [PATCH 1/1 RFC] net/phy: Add Lantiq PHY driver

[Alexander Stein]

Hi John,

On Thursday 19 May 2016 06:50:56, John Crispin wrote:
> On 18/05/2016 18:24, Florian Fainelli wrote:
> > CC'ing Andrew, John,
> 
> also CC'ing Matthias and Hauke. we have had a driver in OpenWrt/LEDE for
> several years that seems a little more complete than this one.
> 
> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/lantiq/patc
> hes-4.4/0023-NET-PHY-adds-driver-for-lantiq-PHY11G.patch;h=93bb4275ec1d261f3
> 98afb8fdc879c1dd973f997;hb=HEAD

Thanks for the link, I wasn't aware of that patch. I like it in general, but 
there are some things I'd like to get addressed first:
* vr9_gphy_of_reg_init() writes uncoditionally to led3h and led3l even on
  PEf7071 which does not have this register at all
* Why is PHY_HAS_INTERRUPT commented out everywhere?
* ltq_phy_init and ltq_phy_exit can be simplified using phy_drivers_register
  and phy_drivers_unregister
* A mdio_device_id table is missing

Best regards,
Alexander

[PATCH 1/1 RFC] net/phy: Add Lantiq PHY driver

[Alexander Stein]

This currently only supports PEF7071 and allows to specify max-speed and
is able to read the LED configuration from device-tree.

Signed-off-by: Alexander Stein <alexander.st...@systec-electronic.com>
---
The main purpose for now is to set a LED configuration from device tree and
to limit the maximum speed. The latter one in my case hardware limited.
As MAC and it's link partner support 1000MBit/s they would try to use that
but will eventually fail due to magnetics only supporting 100MBit/s. So
limit the maximum link speed supported directly from the start.

As this is a RFC I skipped the device tree binding doc.

 drivers/net/phy/Kconfig  |   5 ++
 drivers/net/phy/Makefile |   1 +
 drivers/net/phy/lantiq.c | 167 +++
 3 files changed, 173 insertions(+)
 create mode 100644 drivers/net/phy/lantiq.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 3e28f7a..c004885 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -119,6 +119,11 @@ config STE10XP
---help---
  This is the driver for the STe100p and STe101p PHYs.
 
+config LANTIQ_PHY
+   tristate "Driver for Lantiq PHYs"
+   ---help---
+ Supports the PEF7071 PHYs.
+
 config LSI_ET1011C_PHY
tristate "Driver for LSI ET1011C PHY"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 8ad4ac6..e886549 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -38,3 +38,4 @@ obj-$(CONFIG_MDIO_SUN4I)  += mdio-sun4i.o
 obj-$(CONFIG_MDIO_MOXART)  += mdio-moxart.o
 obj-$(CONFIG_AMD_XGBE_PHY) += amd-xgbe-phy.o
 obj-$(CONFIG_MDIO_BCM_UNIMAC)  += mdio-bcm-unimac.o
+obj-$(CONFIG_LANTIQ_PHY)   += lantiq.o
diff --git a/drivers/net/phy/lantiq.c b/drivers/net/phy/lantiq.c
new file mode 100644
index 000..876a7d1
--- /dev/null
+++ b/drivers/net/phy/lantiq.c
@@ -0,0 +1,167 @@
+/*
+ * Driver for Lantiq PHYs
+ *
+ * Author: Alexander Stein <alexander.st...@systec-electronic.com>
+ *
+ * Copyright (c) 2015-2016 SYS TEC electronic GmbH
+ *
+ * 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.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#define PHY_ID_PEF7071 0xd565a401
+
+#define MII_LANTIQ_MMD_CTRL_REG0x0d
+#define MII_LANTIQ_MMD_REGDATA_REG 0x0e
+#define OP_DATA1
+
+struct lantiqphy_led_ctrl {
+   const char *property;
+   u32 regnum;
+};
+
+static int lantiq_extended_write(struct phy_device *phydev,
+u8 mode, u32 dev_addr, u32 regnum, u16 val)
+{
+   phy_write(phydev, MII_LANTIQ_MMD_CTRL_REG, dev_addr);
+   phy_write(phydev, MII_LANTIQ_MMD_REGDATA_REG, regnum);
+   phy_write(phydev, MII_LANTIQ_MMD_CTRL_REG, (mode << 14) | dev_addr);
+   return phy_write(phydev, MII_LANTIQ_MMD_REGDATA_REG, val);
+}
+
+static int lantiq_of_load_led_config(struct phy_device *phydev,
+struct device_node *of_node,
+const struct lantiqphy_led_ctrl *leds,
+u8 entries)
+{
+   u16 val;
+   int i;
+   int ret = 0;
+
+   for (i = 0; i < entries; i++) {
+   if (!of_property_read_u16(of_node, leds[i].property, )) {
+   ret = lantiq_extended_write(phydev, OP_DATA, 0x1f,
+   leds[i].regnum, val);
+   if (ret) {
+   dev_err(>dev, "Error writing register 
0x1f.%04x (%d)\n",
+   leds[i].regnum, ret);
+   break;
+   }
+   }
+   }
+
+   return ret;
+}
+
+static const struct lantiqphy_led_ctrl leds[] = {
+   {
+   .property = "led0h",
+   .regnum = 0x01e2,
+   },
+   {
+   .property = "led0l",
+   .regnum = 0x01e3,
+   },
+   {
+   .property = "led1h",
+   .regnum = 0x01e4,
+   },
+   {
+   .property = "led1l",
+   .regnum = 0x01e5,
+   },
+   {
+   .property = "led2h",
+   .regnum = 0x01e6,
+   },
+   {
+   .property = "led2l",
+   .regnum = 0x01e7,
+   },
+};
+
+static int lantiqphy_config_init(struct phy_device *phydev)
+{
+   struct device *dev = >dev;
+   struct device_node *of_node = dev->of_node;
+   u32 max_speed;
+
+   if (!of_node && dev->parent->of_node)
+   of_node = dev->parent->of_node;
+

Re: [PATCH 0/3] net: macb: Fix coding style issues

[Alexander Stein]

Hi,

On Monday 07 March 2016 08:17:36, Moritz Fischer wrote:
> this series deals with most of the checkpatch warnings
> generated for macb. There are two BUG_ON()'s that I didn't touch, yet,
> that were suggested by checkpatch, that I can address in a follow up
> commit if needed.

I think addressing those BUG_ON() warnings would be nice as they can affect a 
running system pretty bad.

Best regards,
Alexander

Re: [PATCH] rtlwifi: pass struct rtl_stats by reference as it is more efficient

[Alexander Stein]

On Monday 22 February 2016 10:16:20, Colin Ian King wrote:
> On 22/02/16 06:51, Alexander Stein wrote:
> > On Saturday 20 February 2016 22:10:27, Colin King wrote:
> >> From: Colin Ian King <colin.k...@canonical.com>
> >>
> >> passing rtl_stats by value is inefficient; the structure is over 300
> >> bytes in size and generally just one field (packet_report_type)
> >> is being accessed, so the pass by value is a relatively large overhead.
> >> This change just affects just the rx_command_packet calls.
> > 
> > Why not using a const pointer?
> 
> const struct rtl_stats *const status?

I think "const structl rtl_stats* status" is enough, no need to make that 
pointer const itself. Dunno if that would make any difference for compilers in 
that case.
The idea is that you cannot change the passed struct, same as before because of 
copy by value.

Best regards,
Alexander

Re: [PATCH] rtlwifi: pass struct rtl_stats by reference as it is more efficient

[Alexander Stein]

On Saturday 20 February 2016 22:10:27, Colin King wrote:
> From: Colin Ian King 
> 
> passing rtl_stats by value is inefficient; the structure is over 300
> bytes in size and generally just one field (packet_report_type)
> is being accessed, so the pass by value is a relatively large overhead.
> This change just affects just the rx_command_packet calls.

Why not using a const pointer?

Best regards,
Alexander

[PATCH 1/1] net sysfs: Print link speed as signed integer

[Alexander Stein]

Otherwise 4294967295 (MBit/s) (-1) will be printed when there is no link.
Documentation/ABI/testing/sysfs-class-net does not state if this shall be
signed or unsigned.

Signed-off-by: Alexander Stein <alexander.st...@systec-electronic.com>
---
 net/core/net-sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 805a95a..e3f7eea 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -31,7 +31,7 @@
 static const char fmt_hex[] = "%#x\n";
 static const char fmt_long_hex[] = "%#lx\n";
 static const char fmt_dec[] = "%d\n";
-static const char fmt_udec[] = "%u\n";
+static const char fmt_udec[] = "%d\n";
 static const char fmt_ulong[] = "%lu\n";
 static const char fmt_u64[] = "%llu\n";
 
-- 
2.4.9

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/1] net sysfs: Print link speed as signed integer

[Alexander Stein]

Otherwise 4294967295 (MBit/s) (-1) will be printed when there is no link.
Documentation/ABI/testing/sysfs-class-net does not state if this shall be
signed or unsigned.
Also remove the now unused variable fmt_udec.

Signed-off-by: Alexander Stein <alexander.st...@systec-electronic.com>
---
Changes in v2:
* Switch the format specifier instead of changing it
* Remove the now unused variable

 net/core/net-sysfs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 805a95a..830f8a7 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -31,7 +31,6 @@
 static const char fmt_hex[] = "%#x\n";
 static const char fmt_long_hex[] = "%#lx\n";
 static const char fmt_dec[] = "%d\n";
-static const char fmt_udec[] = "%u\n";
 static const char fmt_ulong[] = "%lu\n";
 static const char fmt_u64[] = "%llu\n";
 
@@ -202,7 +201,7 @@ static ssize_t speed_show(struct device *dev,
if (netif_running(netdev)) {
struct ethtool_cmd cmd;
if (!__ethtool_get_settings(netdev, ))
-   ret = sprintf(buf, fmt_udec, ethtool_cmd_speed());
+   ret = sprintf(buf, fmt_dec, ethtool_cmd_speed());
}
rtnl_unlock();
return ret;
-- 
2.4.9

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html