netdev - [PATCH RFC net-next 0/3] mv88e6xxx: setting 2500base-x mode for CPU/DSA port in dts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20190816150834.26939-1-marek.behun@nic.cz>
Date:   Fri, 16 Aug 2019 17:08:31 +0200
From:   Marek Behún <marek.behun@....cz>
To:     netdev@...r.kernel.org
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Marek Behún <marek.behun@....cz>
Subject: [PATCH RFC net-next 0/3] mv88e6xxx: setting 2500base-x mode for CPU/DSA port in dts

Hello,

I am preparing device tree for Turris Mox, and am encountering the
question of how to properly tell in DTS that a specific port is
connected to the cpu via 2500base-x mode.

CPU port is connected to eth1. In eth1, I simply have
  &eth1 {
    phy-mode = "2500base-x";
    managed = "in-band-status";
  };

This does not work for the CPU/DSA ports though, because of how phylink
and mv88e6xxx operate. CPU/DSA ports SERDES is enabled from
mv88e6xxx_setup(). SERDES irq is not enabled for there ports. Enabling
SERDES irq for there ports cannot be done from mv88e6xxx_setup(),
because the IRQ may fire immediately after enablement, and the phylink
structures do not exist yet for there ports (they are create by DSA only
after the .setup() method is called).

One way to make it work is to use fixed-link for there ports, with
speed = <2500>. But looking at the mv88e6xxx driver I discovered that
this works only because we are lucky (or because of my commit
65b034cf5c176, whatever you prefer. But when I was sending that
patch, fixed-link was not needed for the switch to work):
  - when first the mv88e6xxx_port_setup_mac is called from
    mv88e6xxx_setup_port, it is called with SPEED_MAX. The
    ->port_max_speed_mode() method is called to determine cmode for
    this port, which is 2500basex
  - afterwards, mv88e6xxx_port_setup_mac is called with parameters
    speed=2500 and mode=PHY_INTERFACE_MODE_NA. Because of commit
    65b034cf5c176, cmode is not set to something else

I think that the correct way to do this would be for CPU/DSA port nodes
to have the same setting in dts as the eth node (eg 2500base-x/inband).

For this to work, the mv88e6390_serdes_irq_link_sgmii has to be able
to determine between 2500base-x vs 1000base-x modes. This is done by
the first patch.

The second patch adds two new methods into the DSA operations structure,
.port_setup() and .port_teardown(). The .port_setup is called from
dsa_port_setup() after the port is registered and phylink structure
already exists, and .port_teardown() is called from dsa_port_teardown()
before the port is unregistered.

The third patch then utilizes these new methods to enable/disable
SERDESes and ther IRQs for CPU/DSA ports.

Please comment on this new code, whether it is acceptable to have these
new methods, if they should be called differently, and so on.

Thank you.

Marek

Marek Behún (3):
  net: dsa: mv88e6xxx: support 2500base-x in SGMII IRQ handler
  net: dsa: add port_setup/port_teardown methods to DSA ops
  net: dsa: mv88e6xxx: setup SERDES irq also for CPU/DSA ports

 drivers/net/dsa/mv88e6xxx/chip.c   | 54 ++++++++++++++++++++++++------
 drivers/net/dsa/mv88e6xxx/serdes.c |  6 +++-
 include/net/dsa.h                  |  2 ++
 net/dsa/dsa2.c                     | 10 +++++-
 4 files changed, 60 insertions(+), 12 deletions(-)

-- 
2.21.0