Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 26 Feb 2020 16:44:35 +0100
From:   Jiri Pirko <jiri@resnulli.us>
To:     Vadym Kochan <vadym.kochan@plvision.eu>
Cc:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "David S . Miller" <davem@davemloft.net>,
        Oleksandr Mazur <oleksandr.mazur@plvision.eu>,
        Taras Chornyi <taras.chornyi@plvision.eu>,
        Serhiy Boiko <serhiy.boiko@plvision.eu>,
        Andrii Savka <andrii.savka@plvision.eu>,
        Volodymyr Mytnyk <volodymyr.mytnyk@plvision.eu>
Subject: Re: [RFC net-next 0/3] net: marvell: prestera: Add Switchdev driver
 for Prestera family ASIC device 98DX326x (AC3x)
Message-ID: <20200226154435.GB26061@nanopsycho>
References: <20200225163025.9430-1-vadym.kochan@plvision.eu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200225163025.9430-1-vadym.kochan@plvision.eu>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Tue, Feb 25, 2020 at 05:30:52PM CET, vadym.kochan@plvision.eu wrote:
>Marvell Prestera 98DX326x integrates up to 24 ports of 1GbE with 8
>ports of 10GbE uplinks or 2 ports of 40Gbps stacking for a largely
>wireless SMB deployment.
>
>Prestera Switchdev is a firmware based driver which operates via PCI
>bus. The driver is split into 2 modules:
>
>    - prestera_sw.ko - main generic Switchdev Prestera ASIC related logic.
>
>    - prestera_pci.ko - bus specific code which also implements firmware

It is unusual to see ".ko" in patchset cover letter...


>                        loading and low-level messaging protocol between
>                        firmware and the switchdev driver.
>
>This driver implementation includes only L1 & basic L2 support.
>
>The core Prestera switching logic is implemented in prestera.c, there is
>an intermediate hw layer between core logic and firmware. It is
>implemented in prestera_hw.c, the purpose of it is to encapsulate hw
>related logic, in future there is a plan to support more devices with
>different HW related configurations.
>
>The firmware has to be loaded each time device is reset. The driver is
>loading it from:
>
>    /lib/firmware/marvell/prestera_fw_img.bin
>
>The firmware image version is located within internal header and consists
>of 3 numbers - MAJOR.MINOR.PATCH. Additionally, driver has hard-coded
>minimum supported firmware version which it can work with:
>
>    MAJOR - reflects the support on ABI level between driver and loaded
>            firmware, this number should be the same for driver and
>            loaded firmware.
>
>    MINOR - this is the minimal supported version between driver and the
>            firmware.
>
>    PATCH - indicates only fixes, firmware ABI is not changed.
>

It is usual that the file name contains a version. I think it is
good to make sure you are loading the version your driver is compatible
with. There could be multiple versions for multiple kernels.


>The firmware image will be submitted to the linux-firmware after the
>driver is accepted.

Hmm, not sure how this works, shouldn't it be submitted there first?


>
>The following Switchdev features are supported:

You don't need to mention "Switchdev". It is just a offloading layer for
bridge. Does not mean anything else now...


>
>    - VLAN-aware bridge offloading
>    - VLAN-unaware bridge offloading
>    - FDB offloading (learning, ageing)
>    - Switchport configuration
>
>CPU RX/TX support will be provided in the next contribution.
>
>Vadym Kochan (3):
>  net: marvell: prestera: Add Switchdev driver for Prestera family ASIC
>    device 98DX325x (AC3x)
>  net: marvell: prestera: Add PCI interface support
>  dt-bindings: marvell,prestera: Add address mapping for Prestera
>    Switchdev PCIe driver
>
> .../bindings/net/marvell,prestera.txt         |   13 +
> drivers/net/ethernet/marvell/Kconfig          |    1 +
> drivers/net/ethernet/marvell/Makefile         |    1 +
> drivers/net/ethernet/marvell/prestera/Kconfig |   24 +
> .../net/ethernet/marvell/prestera/Makefile    |    5 +
> .../net/ethernet/marvell/prestera/prestera.c  | 1502 +++++++++++++++++
> .../net/ethernet/marvell/prestera/prestera.h  |  244 +++
> .../marvell/prestera/prestera_drv_ver.h       |   23 +
> .../ethernet/marvell/prestera/prestera_hw.c   | 1094 ++++++++++++
> .../ethernet/marvell/prestera/prestera_hw.h   |  159 ++
> .../ethernet/marvell/prestera/prestera_pci.c  |  840 +++++++++
> .../marvell/prestera/prestera_switchdev.c     | 1217 +++++++++++++
> 12 files changed, 5123 insertions(+)
> create mode 100644 drivers/net/ethernet/marvell/prestera/Kconfig
> create mode 100644 drivers/net/ethernet/marvell/prestera/Makefile
> create mode 100644 drivers/net/ethernet/marvell/prestera/prestera.c
> create mode 100644 drivers/net/ethernet/marvell/prestera/prestera.h
> create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_drv_ver.h
> create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_hw.c
> create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_hw.h
> create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_pci.c
> create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
>
>-- 
>2.17.1
>