Netdev List
 help / color / mirror / Atom feed
* [PATCH net] net: phylink: initialize carrier state at creation
From: Klaus Kudielka @ 2023-11-06 18:05 UTC (permalink / raw)
  To: Russell King, Andrew Lunn, Heiner Kallweit
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	netdev, linux-kernel, Klaus Kudielka

Background: Turris Omnia (Armada 385); eth2 (mvneta) connected to SFP bus;
SFP module is present, but no fiber connected, so definitely no carrier.

After booting, eth2 is down, but netdev LED trigger surprisingly reports
link active. Then, after "ip link set eth2 up", the link indicator goes
away - as I would have expected it from the beginning.

It turns out, that the default carrier state after netdev creation is
"carrier ok". Some ethernet drivers explicitly call netif_carrier_off
during probing, others (like mvneta) don't - which explains the current
behaviour: only when the device is brought up, phylink_start calls
netif_carrier_off.

Fix this for all drivers, by calling netif_carrier_off in phylink_create.

Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Klaus Kudielka <klaus.kudielka@gmail.com>
---
 drivers/net/phy/phylink.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 6712883498..a28da80bde 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1616,6 +1616,7 @@ struct phylink *phylink_create(struct phylink_config *config,
 	pl->config = config;
 	if (config->type == PHYLINK_NETDEV) {
 		pl->netdev = to_net_dev(config->dev);
+		netif_carrier_off(pl->netdev);
 	} else if (config->type == PHYLINK_DEV) {
 		pl->dev = config->dev;
 	} else {
-- 
2.42.0


^ permalink raw reply related

* [PATCH] can: netlink: Fix TDCO calculation using the old data bittiming
From: Maxime Jayat @ 2023-11-06 18:01 UTC (permalink / raw)
  To: Wolfgang Grandegger, Marc Kleine-Budde, Vincent Mailhol
  Cc: linux-can, netdev, linux-kernel

The TDCO calculation was done using the currently applied data bittiming,
instead of the newly computed data bittiming, which means that the TDCO
had an invalid value unless setting the same data bittiming twice.

Fixes: d99755f71a80 ("can: netlink: add interface for CAN-FD Transmitter Delay Compensation (TDC)")
Signed-off-by: Maxime Jayat <maxime.jayat@mobile-devices.fr>
---
 drivers/net/can/dev/netlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
index 036d85ef07f5..dfdc039d92a6 100644
--- a/drivers/net/can/dev/netlink.c
+++ b/drivers/net/can/dev/netlink.c
@@ -346,7 +346,7 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
 			/* Neither of TDC parameters nor TDC flags are
 			 * provided: do calculation
 			 */
-			can_calc_tdco(&priv->tdc, priv->tdc_const, &priv->data_bittiming,
+			can_calc_tdco(&priv->tdc, priv->tdc_const, &dbt,
 				      &priv->ctrlmode, priv->ctrlmode_supported);
 		} /* else: both CAN_CTRLMODE_TDC_{AUTO,MANUAL} are explicitly
 		   * turned off. TDC is disabled: do nothing
-- 
2.34.1


^ permalink raw reply related

* Re: [net-next RFC PATCH v5 4/4] dt-bindings: Document bindings for Marvell Aquantia PHY
From: Conor Dooley @ 2023-11-06 17:33 UTC (permalink / raw)
  To: Christian Marangi
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andrew Lunn,
	Heiner Kallweit, Russell King, Robert Marko, Vladimir Oltean,
	netdev, devicetree, linux-kernel
In-Reply-To: <20231106165433.2746-4-ansuelsmth@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 5225 bytes --]

On Mon, Nov 06, 2023 at 05:54:33PM +0100, Christian Marangi wrote:
> Document bindings for Marvell Aquantia PHY.
> 
> The Marvell Aquantia PHY require a firmware to work correctly and there
> at least 3 way to load this firmware.
> 
> Describe all the different way and document the binding "firmware-name"
> to load the PHY firmware from userspace.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
> Changes v5:
> - Drop extra entry not related to HW description
> Changes v3:
> - Make DT description more OS agnostic
> - Use custom select to fix dtbs checks
> Changes v2:
> - Add DT patch
> 
>  .../bindings/net/marvell,aquantia.yaml        | 123 ++++++++++++++++++
>  1 file changed, 123 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> new file mode 100644
> index 000000000000..7106c5bdf73c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> @@ -0,0 +1,123 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell Aquantia Ethernet PHY
> +
> +maintainers:
> +  - Christian Marangi <ansuelsmth@gmail.com>
> +
> +description: |
> +  Marvell Aquantia Ethernet PHY require a firmware to be loaded to actually
> +  work.
> +
> +  This can be done and is implemented by OEM in 3 different way:
> +    - Attached SPI flash directly to the PHY with the firmware. The PHY
> +      will self load the firmware in the presence of this configuration.

> +    - Dedicated partition on system NAND with firmware in it. NVMEM
> +      subsystem will be used and the declared NVMEM cell will load
> +      the firmware to the PHY using the PHY mailbox interface.

I'd probably phrase this one as something more like "Read from a
dedicated partition on system NAND declared in an NVMEM cell, and loaded
to the PHY using its mailbox interface." or something like that - mostly
to get rid of the linux specific "NVMEM subsystem" from the description.

Otherwise,
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

> +    - Manually provided firmware loaded from a file in the filesystem.
> +
> +allOf:
> +  - $ref: ethernet-phy.yaml#
> +
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - ethernet-phy-id03a1.b445
> +          - ethernet-phy-id03a1.b460
> +          - ethernet-phy-id03a1.b4a2
> +          - ethernet-phy-id03a1.b4d0
> +          - ethernet-phy-id03a1.b4e0
> +          - ethernet-phy-id03a1.b5c2
> +          - ethernet-phy-id03a1.b4b0
> +          - ethernet-phy-id03a1.b662
> +          - ethernet-phy-id03a1.b712
> +          - ethernet-phy-id31c3.1c12
> +  required:
> +    - compatible
> +
> +properties:
> +  reg:
> +    maxItems: 1
> +
> +  firmware-name:
> +    description: specify the name of PHY firmware to load
> +
> +  nvmem-cells:
> +    description: phandle to the firmware nvmem cell
> +    maxItems: 1
> +
> +  nvmem-cell-names:
> +    const: firmware
> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    mdio {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ethernet-phy@0 {
> +            /*  Only needed to make DT lint tools work. Do not copy/paste
> +             *  into real DTS files.
> +             */
> +            compatible = "ethernet-phy-id31c3.1c12",
> +                         "ethernet-phy-ieee802.3-c45";
> +
> +            reg = <0>;
> +            firmware-name = "AQR-G4_v5.4.C-AQR_CIG_WF-1945_0x8_ID44776_VER1630.cld";
> +        };
> +
> +        ethernet-phy@1 {
> +            /*  Only needed to make DT lint tools work. Do not copy/paste
> +             *  into real DTS files.
> +             */
> +            compatible = "ethernet-phy-id31c3.1c12",
> +                         "ethernet-phy-ieee802.3-c45";
> +
> +            reg = <0>;
> +            nvmem-cells = <&aqr_fw>;
> +            nvmem-cell-names = "firmware";
> +        };
> +    };
> +
> +    flash {
> +        compatible = "jedec,spi-nor";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        partitions {
> +            compatible = "fixed-partitions";
> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +
> +            /* ... */
> +
> +            partition@650000 {
> +                compatible = "nvmem-cells";
> +                label = "0:ethphyfw";
> +                reg = <0x650000 0x80000>;
> +                read-only;
> +                #address-cells = <1>;
> +                #size-cells = <1>;
> +
> +                aqr_fw: aqr_fw@0 {
> +                    reg = <0x0 0x5f42a>;
> +                };
> +            };
> +
> +            /* ... */
> +
> +        };
> +    };
> -- 
> 2.40.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH bpf 0/6] bpf_redirect_peer fixes
From: Stanislav Fomichev @ 2023-11-06 17:22 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: martin.lau, kuba, netdev, bpf
In-Reply-To: <20231103222748.12551-1-daniel@iogearbox.net>

On 11/03, Daniel Borkmann wrote:
> This fixes bpf_redirect_peer stats accounting for veth and netkit,
> and adds tstats in the first place for the latter. Utilise indirect
> call wrapper for bpf_redirect_peer, and improve test coverage of the
> latter also for netkit devices. Details in the patches, thanks!
> 
> Daniel Borkmann (4):
>   netkit: Add tstats per-CPU traffic counters
>   bpf, netkit: Add indirect call wrapper for fetching peer dev
>   selftests/bpf: De-veth-ize the tc_redirect test case
>   selftests/bpf: Add netkit to tc_redirect selftest
> 
> Peilin Ye (2):
>   veth: Use tstats per-CPU traffic counters
>   bpf: Fix dev's rx stats for bpf_redirect_peer traffic

Acked-by: Stanislav Fomichev <sdf@google.com>

With one optional nit about indirect call.

^ permalink raw reply

* Re: [PATCH bpf 4/6] bpf, netkit: Add indirect call wrapper for fetching peer dev
From: Stanislav Fomichev @ 2023-11-06 17:21 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: martin.lau, kuba, netdev, bpf, Nikolay Aleksandrov
In-Reply-To: <20231103222748.12551-5-daniel@iogearbox.net>

On 11/03, Daniel Borkmann wrote:
> ndo_get_peer_dev is used in tcx BPF fast path, therefore make use of
> indirect call wrapper and therefore optimize the bpf_redirect_peer()
> internal handling a bit. Add a small skb_get_peer_dev() wrapper which
> utilizes the INDIRECT_CALL_1() macro instead of open coding.
> 
> Co-developed-by: Nikolay Aleksandrov <razor@blackwall.org>
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
>  drivers/net/netkit.c |  3 ++-
>  include/net/netkit.h |  6 ++++++
>  net/core/filter.c    | 18 +++++++++++++-----
>  3 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/netkit.c b/drivers/net/netkit.c
> index dc51c23b40f0..934c71a73b5c 100644
> --- a/drivers/net/netkit.c
> +++ b/drivers/net/netkit.c
> @@ -7,6 +7,7 @@
>  #include <linux/filter.h>
>  #include <linux/netfilter_netdev.h>
>  #include <linux/bpf_mprog.h>
> +#include <linux/indirect_call_wrapper.h>
>  
>  #include <net/netkit.h>
>  #include <net/dst.h>
> @@ -177,7 +178,7 @@ static void netkit_set_headroom(struct net_device *dev, int headroom)
>  	rcu_read_unlock();
>  }
>  
> -static struct net_device *netkit_peer_dev(struct net_device *dev)
> +INDIRECT_CALLABLE_SCOPE struct net_device *netkit_peer_dev(struct net_device *dev)
>  {
>  	return rcu_dereference(netkit_priv(dev)->peer);
>  }
> diff --git a/include/net/netkit.h b/include/net/netkit.h
> index 0ba2e6b847ca..9ec0163739f4 100644
> --- a/include/net/netkit.h
> +++ b/include/net/netkit.h
> @@ -10,6 +10,7 @@ int netkit_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog);
>  int netkit_link_attach(const union bpf_attr *attr, struct bpf_prog *prog);
>  int netkit_prog_detach(const union bpf_attr *attr, struct bpf_prog *prog);
>  int netkit_prog_query(const union bpf_attr *attr, union bpf_attr __user *uattr);
> +INDIRECT_CALLABLE_DECLARE(struct net_device *netkit_peer_dev(struct net_device *dev));
>  #else
>  static inline int netkit_prog_attach(const union bpf_attr *attr,
>  				     struct bpf_prog *prog)
> @@ -34,5 +35,10 @@ static inline int netkit_prog_query(const union bpf_attr *attr,
>  {
>  	return -EINVAL;
>  }
> +
> +static inline struct net_device *netkit_peer_dev(struct net_device *dev)
> +{
> +	return NULL;
> +}
>  #endif /* CONFIG_NETKIT */
>  #endif /* __NET_NETKIT_H */
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 7aca28b7d0fd..dbf92b272022 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -81,6 +81,7 @@
>  #include <net/xdp.h>
>  #include <net/mptcp.h>
>  #include <net/netfilter/nf_conntrack_bpf.h>
> +#include <net/netkit.h>
>  #include <linux/un.h>
>  
>  #include "dev.h"
> @@ -2468,6 +2469,16 @@ static const struct bpf_func_proto bpf_clone_redirect_proto = {
>  DEFINE_PER_CPU(struct bpf_redirect_info, bpf_redirect_info);
>  EXPORT_PER_CPU_SYMBOL_GPL(bpf_redirect_info);
>  
> +static struct net_device *skb_get_peer_dev(struct net_device *dev)
> +{
> +	const struct net_device_ops *ops = dev->netdev_ops;
> +
> +	if (likely(ops->ndo_get_peer_dev))
> +		return INDIRECT_CALL_1(ops->ndo_get_peer_dev,
> +				       netkit_peer_dev, dev);

nit: why not put both netkit and veth here under INDIRECT_CALL_2 ?
Presumably should help with the veth deployments as well?

^ permalink raw reply

* Re: [PATCH iproute2-next v2] bridge: mdb: Add get support
From: patchwork-bot+netdevbpf @ 2023-11-06 17:20 UTC (permalink / raw)
  To: Ido Schimmel; +Cc: netdev, dsahern, stephen, razor, mlxsw
In-Reply-To: <20231101074510.1471018-1-idosch@nvidia.com>

Hello:

This patch was applied to iproute2/iproute2-next.git (main)
by David Ahern <dsahern@kernel.org>:

On Wed, 1 Nov 2023 09:45:10 +0200 you wrote:
> Implement MDB get functionality, allowing user space to query a single
> MDB entry from the kernel instead of dumping all the entries. Example
> usage:
> 
>  # bridge mdb add dev br0 port swp1 grp 239.1.1.1 vid 10
>  # bridge mdb add dev br0 port swp2 grp 239.1.1.1 vid 10
>  # bridge mdb add dev br0 port swp2 grp 239.1.1.5 vid 10
>  # bridge mdb get dev br0 grp 239.1.1.1 vid 10
>  dev br0 port swp1 grp 239.1.1.1 temp vid 10
>  dev br0 port swp2 grp 239.1.1.1 temp vid 10
>  # bridge -j -p mdb get dev br0 grp 239.1.1.1 vid 10
>  [ {
>          "index": 10,
>          "dev": "br0",
>          "port": "swp1",
>          "grp": "239.1.1.1",
>          "state": "temp",
>          "flags": [ ],
>          "vid": 10
>      },{
>          "index": 10,
>          "dev": "br0",
>          "port": "swp2",
>          "grp": "239.1.1.1",
>          "state": "temp",
>          "flags": [ ],
>          "vid": 10
>      } ]
>  # bridge mdb get dev br0 grp 239.1.1.1 vid 20
>  Error: bridge: MDB entry not found.
>  # bridge mdb get dev br0 grp 239.1.1.2 vid 10
>  Error: bridge: MDB entry not found.
> 
> [...]

Here is the summary with links:
  - [iproute2-next,v2] bridge: mdb: Add get support
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=77138a2f9477

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply

* Re: [patch net-next v4 1/7] ip/ipnetns: move internals of get_netnsid_from_name() into namespace.c
From: David Ahern @ 2023-11-06 17:11 UTC (permalink / raw)
  To: Jiri Pirko, netdev; +Cc: stephen, daniel.machon
In-Reply-To: <20231027101403.958745-2-jiri@resnulli.us>

On 10/27/23 4:13 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> In order to be able to reuse get_netnsid_from_name() function outside of
> ip code, move the internals to lib/namespace.c to a new function called
> netns_id_from_name().
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
> v3->v4:
> - removed namespace.h include
> v2->v3:
> - s/netns_netnsid_from_name/netns_id_from_name/
> v1->v2:
> - new patch
> ---
>  include/namespace.h |  2 ++
>  ip/ipnetns.c        | 45 +----------------------------------------
>  lib/namespace.c     | 49 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 52 insertions(+), 44 deletions(-)
> 


dcb
    CC       dcb.o
In file included from dcb.c:11:
../include/namespace.h:61:31: warning: ‘struct rtnl_handle’ declared
inside parameter list will not be visible outside of this definition or
declaration
   61 | int netns_id_from_name(struct rtnl_handle *rtnl, const char *name);
      |                               ^~~~~~~~~~~

--
pw-bot: cr

^ permalink raw reply

* [ANNOUNCE] iproute2 6.6 release
From: Stephen Hemminger @ 2023-11-06 17:03 UTC (permalink / raw)
  To: netdev

Update release of iproute2 corresponding to the 6.6 kernel.  Release
is smaller than usual. One notable thing is that support for kernel
components removed in 6.3 kernel was removed from iproute2 (dsmark,
cbq, rsvp).

Download:
    https://www.kernel.org/pub/linux/utils/net/iproute2/iproute2-6.6.0.tar.gz

Repository for current release
    https://github.com/shemminger/iproute2.git
    git://git.kernel.org/pub/scm/network/iproute2/iproute2.git

And future release (net-next):
    git://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git

Contributions:

Allen Hubbe (1):
      vdpa: consume device_features parameter

Amit Cohen (1):
      bridge: fdb: add an error print for unknown command

Andrea Claudi (5):
      ss: make is_selinux_enabled stub work like in SELinux
      ss: make SELinux stub functions conformant to API definitions
      lib: add SELinux include and stub functions
      ip vrf: make ipvrf_exec SELinux-aware
      Makefile: ensure CONF_USR_DIR honours the libdir config

David Ahern (2):
      Update kernel headers
      Update kernel headers

Eric Dumazet (1):
      ss: add support for rcv_wnd and rehash

François Michel (4):
      tc: support the netem seed parameter for loss and corruption events
      man: tc-netem: add section for specifying the netem seed
      tc: fix typo in netem's usage string
      tc: fix several typos in netem's usage string

Ido Schimmel (1):
      bridge: Add backup nexthop ID support

Jiri Pirko (7):
      devlink: accept "name" command line option instead of "trap"/"group"
      devlink: move DL_OPT_SB into required options
      devlink: make parsing of handle non-destructive to argv
      devlink: implement command line args dry parsing
      devlink: return -ENOENT if argument is missing
      mnl_utils: introduce a helper to check if dump policy exists for command
      devlink: implement dump selector for devlink objects show commands

Mathieu Schroeter (4):
      Add get_long utility and adapt get_integer accordingly
      Add utility to convert an unsigned int to string
      ss: change aafilter port from int to long (inode support)
      ss: print unix socket "ports" as unsigned int (inode)

Maxim Petrov (2):
      ip: fix memory leak in 'ip maddr show'
      ss: fix directory leak when -T option is used

Pedro Tammela (1):
      utils: fix get_integer() logic

Ratheesh Kannoth (1):
      tc: Classifier support for SPI field

Stephen Hemminger (16):
      tc: add missing space before else
      uapi: headers update from 6.6-rc2
      fix set-not-used warnings
      bridge: fix potential snprintf overflow
      ila: fix potential snprintf buffer overflow
      Add security policy
      uapi: update headers from 6.6-rc4
      ila: fix array overflow warning
      uapi: update from 6.6-rc5
      tc: remove support for CBQ
      tc: remove support for RSVP classifier
      tc: remove tcindex classifier
      tc: remove dsmark qdisc
      tc: drop support for ATM qdisc
      ssfilter: fix clang warning about conversion
      v6.6.0


^ permalink raw reply

* Re: [RFC Draft net-next] docs: netdev: add section on using lei to manage netdev mail volume
From: David Wei @ 2023-11-06 16:57 UTC (permalink / raw)
  To: Matthieu Baerts
  Cc: netdev, Jakub Kicinski, workflows, linux-doc, pabeni, davem,
	corbet, edumazet
In-Reply-To: <8205a0ba-aeef-4ab6-80cc-87848903f541@kernel.org>

On 2023-11-06 03:24, Matthieu Baerts wrote:
> Hi David,
> 
> On 05/11/2023 19:50, David Wei wrote:
>> As a beginner to netdev I found the volume of mail to be overwhelming. I only
>> want to focus on core netdev changes and ignore most driver changes. I found a
>> way to do this using lei, filtering the mailing list using lore's query
>> language and writing the results into an IMAP server.
> 
> I agree that the volume of mail is too high with a variety of subjects.
> That's why it is very important to CC the right people (as mentioned by
> Patchwork [1] ;) )
> 
> [1]
> https://patchwork.kernel.org/project/netdevbpf/patch/20231105185014.2523447-1-dw@davidwei.uk/

Sorry and noted, I've now CC'd maintainers mentioned by Patchwork.

> 
>> This patch is an RFC draft of updating the maintainer-netdev documentation with
>> this information in the hope of helping out others in the future.
> 
> Note that I'm also using lei to filter emails, e.g. to be notified when
> someone sends a patch modifying this maintainer-netdev.rst file! [2]
> 
> But I don't think this issue of "busy mailing list" is specific to
> netdev. It seems that "lei" is already mentioned in another part of the
> doc [3]. Maybe this part can be improved? Or the netdev doc could add a
> reference to the existing part?

I think "busy mailing list" is especially bad for netdev. There are many
tutorials for setting up lei, but my ideal goal is a copy + paste
command specifically for netdev that outputs into an IMAP server for
beginners to use. As opposed to writing something more generic.

> 
> (Maybe such info should be present elsewhere, e.g. on vger [4] or lore)
> 
> [2]
> https://lore.kernel.org/netdev/?q=%28dfn%3ADocumentation%2Fnetworking%2Fnetdev-FAQ.rst+OR+dfn%3ADocumentation%2Fprocess%2Fmaintainer-netdev.rst%29+AND+rt%3A1.month.ago..
> [3]
> https://docs.kernel.org/maintainer/feature-and-driver-maintainers.html#mailing-list-participation

This document is aimed at kernel maintainers. My concern is that
beginners would not find or read this document.

> [4] http://vger.kernel.org/vger-lists.html

It would be nice to add a link in the netdev list "Info" section. Do you
know how to update it?

How about keeping a netdev specific sample lei query in
maintainer-netdev and refer to it from [4]?

> 
> (Note: regarding the commit message here, each line should be limited to
> max 72 chars ideally)

Apologies, I may not have line wrap set up properly in my editor.

> 
>> Signed-off-by: David Wei <dw@davidwei.uk>
>> ---
>>  Documentation/process/maintainer-netdev.rst | 39 +++++++++++++++++++++
>>  1 file changed, 39 insertions(+)
>>
>> diff --git a/Documentation/process/maintainer-netdev.rst b/Documentation/process/maintainer-netdev.rst
>> index 7feacc20835e..93851783de6f 100644
>> --- a/Documentation/process/maintainer-netdev.rst
>> +++ b/Documentation/process/maintainer-netdev.rst
>> @@ -33,6 +33,45 @@ Aside from subsystems like those mentioned above, all network-related
>>  Linux development (i.e. RFC, review, comments, etc.) takes place on
>>  netdev.
>>  
>> +Managing emails
>> +~~~~~~~~~~~~~~~
>> +
>> +netdev is a busy mailing list with on average over 200 emails received per day,
>> +which can be overwhelming to beginners. Rather than subscribing to the entire
>> +list, considering using ``lei`` to only subscribe to topics that you are
>> +interested in. Konstantin Ryabitsev wrote excellent tutorials on using ``lei``:
>> +
>> + - https://people.kernel.org/monsieuricon/lore-lei-part-1-getting-started
>> + - https://people.kernel.org/monsieuricon/lore-lei-part-2-now-with-imap
>> +
>> +As a netdev beginner, you may want to filter out driver changes and only focus
>> +on core netdev changes. Try using the following query with ``lei q``::
>> +
>> +  lei q -o ~/Mail/netdev \
>> +    -I https://lore.kernel.org/all \
>> +    -t '(b:b/net/* AND tc:netdev@vger.kernel.org AND rt:2.week.ago..'
> 
> Small optimisations:
> 
> - you can remove tc:netdev@vger.kernel.org and modify the '-I' to
> restrict to netdev instead of querying 'all': -I
> https://lore.kernel.org/netdev/

Thank you, this is great.

> 
> - In theory, 'dfn:' should help you to match a filename being modified.
> But in your case, 'net' is too generic, and I don't think we can specify
> "starting with 'net'". You can still omit some results after [5] but the
> syntax doesn't look better :)
> 
>   dfn:net AND NOT dfn:drivers/net AND NOT dfn:selftests/net AND NOT
> dfn:tools/net AND rt:2.week.ago..

I initially went with this as well, but found it tedious to add many AND
NOT statements. My metric was number of emails filtered and matching
using b:b/net/* produced the least number of emails :)

It would be ideal if we could express dfn:^net/*. I contacted the public
inbox folks and they said it is not supported :(

> 
> [5]
> https://lore.kernel.org/netdev/?q=dfn%3Anet+AND+NOT+dfn%3Adrivers%2Fnet+AND+NOT+dfn%3Aselftests%2Fnet+AND+NOT+dfn%3Atools%2Fnet+AND+rt%3A2.week.ago..
> 
>> +This query will only match threads containing messages with patches that modify
>> +files in ``net/*``. For more information on the query language, see:
>> +
>> +  https://lore.kernel.org/linux-btrfs/_/text/help/
> 
> (if this is specific to 'netdev', best to use '/netdev/', not
> '/linux-btrfs/')

Thank you, will fix this.

> 
>> +By default ``lei`` will output to a Maildir, but it also supports Mbox and IMAP
>> +by adding a prefix to the output directory ``-o``. For a list of supported
>> +formats and prefix strings, see:
>> +
>> +  https://www.mankier.com/1/lei-q
> 
> Maybe safer to point to the official doc?
> 
> https://public-inbox.org/lei-q.html
> 
> (or 'man lei-q')

Thanks, official manpages are best.

> 
>> +If you would like to use IMAP, Konstantin’s blog is slightly outdated and you
>> +no longer need to use here strings i.e. ``<<<`` or ``<<EOF``.
> 
> I think we can still use them. In the part 1, they are not used. Maybe
> best to contact Konstantin to update his blog post instead of mentioning
> in the doc that the blog post is outdated?

You're right, I've emailed Konstantin.

> 
>> You can simply
>> +point lei at an IMAP server e.g. ``imaps://imap.gmail.com``::
> 
> In Konstantin's blog post, he mentioned different servers with different
> specificities. Maybe easier to just point to that instead of taking one
> example without more explanations?

Will do!

> 
> Cheers,
> Matt

^ permalink raw reply

* [net-next RFC PATCH v5 3/4] net: phy: aquantia: add firmware load support
From: Christian Marangi @ 2023-11-06 16:54 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andrew Lunn,
	Heiner Kallweit, Russell King, Christian Marangi, Robert Marko,
	Vladimir Oltean, netdev, devicetree, linux-kernel
In-Reply-To: <20231106165433.2746-1-ansuelsmth@gmail.com>

From: Robert Marko <robimarko@gmail.com>

Aquantia PHY-s require firmware to be loaded before they start operating.
It can be automatically loaded in case when there is a SPI-NOR connected
to Aquantia PHY-s or can be loaded from the host via MDIO.

This patch adds support for loading the firmware via MDIO as in most cases
there is no SPI-NOR being used to save on cost.
Firmware loading code itself is ported from mainline U-boot with cleanups.

The firmware has mixed values both in big and little endian.
PHY core itself is big-endian but it expects values to be in little-endian.
The firmware is little-endian but CRC-16 value for it is stored at the end
of firmware in big-endian.

It seems the PHY does the conversion internally from firmware that is
little-endian to the PHY that is big-endian on using the mailbox
but mailbox returns a big-endian CRC-16 to verify the written data
integrity.

Co-developed-by: Christian Marangi <ansuelsmth@gmail.com>
Signed-off-by: Robert Marko <robimarko@gmail.com>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
Changes v4:
- Drop inline
- Drop extra impossible check for negative values
Changes v3:
- Back to RFC due to merge window
- Use unaligned macro instead of memcpy
- Spam sanity check as firmware is evil
- Add print to signal the user the source of the fw (FS or NVMEM)
Changes v2:
- Move out of RFC
- Address sanity check for offsets
- Add additional comments on firmware load check
- Fix some typo
- Capitalize CRC in comments
- Rename load_sysfs to load_fs

 drivers/net/phy/aquantia/Kconfig             |   1 +
 drivers/net/phy/aquantia/Makefile            |   2 +-
 drivers/net/phy/aquantia/aquantia.h          |  32 ++
 drivers/net/phy/aquantia/aquantia_firmware.c | 367 +++++++++++++++++++
 drivers/net/phy/aquantia/aquantia_main.c     |   6 +
 5 files changed, 407 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/phy/aquantia/aquantia_firmware.c

diff --git a/drivers/net/phy/aquantia/Kconfig b/drivers/net/phy/aquantia/Kconfig
index 226146417a6a..a35de4b9b554 100644
--- a/drivers/net/phy/aquantia/Kconfig
+++ b/drivers/net/phy/aquantia/Kconfig
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config AQUANTIA_PHY
 	tristate "Aquantia PHYs"
+	select CRC_CCITT
 	help
 	  Currently supports the Aquantia AQ1202, AQ2104, AQR105, AQR405
diff --git a/drivers/net/phy/aquantia/Makefile b/drivers/net/phy/aquantia/Makefile
index 346f350bc084..aa77fb63c8ec 100644
--- a/drivers/net/phy/aquantia/Makefile
+++ b/drivers/net/phy/aquantia/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
-aquantia-objs			+= aquantia_main.o
+aquantia-objs			+= aquantia_main.o aquantia_firmware.o
 ifdef CONFIG_HWMON
 aquantia-objs			+= aquantia_hwmon.o
 endif
diff --git a/drivers/net/phy/aquantia/aquantia.h b/drivers/net/phy/aquantia/aquantia.h
index f0c767c4fad1..9ed38972abdb 100644
--- a/drivers/net/phy/aquantia/aquantia.h
+++ b/drivers/net/phy/aquantia/aquantia.h
@@ -10,10 +10,35 @@
 #include <linux/phy.h>
 
 /* Vendor specific 1, MDIO_MMD_VEND1 */
+#define VEND1_GLOBAL_SC				0x0
+#define VEND1_GLOBAL_SC_SOFT_RESET		BIT(15)
+#define VEND1_GLOBAL_SC_LOW_POWER		BIT(11)
+
 #define VEND1_GLOBAL_FW_ID			0x0020
 #define VEND1_GLOBAL_FW_ID_MAJOR		GENMASK(15, 8)
 #define VEND1_GLOBAL_FW_ID_MINOR		GENMASK(7, 0)
 
+#define VEND1_GLOBAL_MAILBOX_INTERFACE1			0x0200
+#define VEND1_GLOBAL_MAILBOX_INTERFACE1_EXECUTE		BIT(15)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE1_WRITE		BIT(14)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE1_CRC_RESET	BIT(12)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE1_BUSY		BIT(8)
+
+#define VEND1_GLOBAL_MAILBOX_INTERFACE2			0x0201
+#define VEND1_GLOBAL_MAILBOX_INTERFACE3			0x0202
+#define VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR_MASK	GENMASK(15, 0)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR(x)	FIELD_PREP(VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR_MASK, (u16)((x) >> 16))
+#define VEND1_GLOBAL_MAILBOX_INTERFACE4			0x0203
+#define VEND1_GLOBAL_MAILBOX_INTERFACE4_LSW_ADDR_MASK	GENMASK(15, 2)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE4_LSW_ADDR(x)	FIELD_PREP(VEND1_GLOBAL_MAILBOX_INTERFACE4_LSW_ADDR_MASK, (u16)(x))
+
+#define VEND1_GLOBAL_MAILBOX_INTERFACE5			0x0204
+#define VEND1_GLOBAL_MAILBOX_INTERFACE5_MSW_DATA_MASK	GENMASK(15, 0)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE5_MSW_DATA(x)	FIELD_PREP(VEND1_GLOBAL_MAILBOX_INTERFACE5_MSW_DATA_MASK, (u16)((x) >> 16))
+#define VEND1_GLOBAL_MAILBOX_INTERFACE6			0x0205
+#define VEND1_GLOBAL_MAILBOX_INTERFACE6_LSW_DATA_MASK	GENMASK(15, 0)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE6_LSW_DATA(x)	FIELD_PREP(VEND1_GLOBAL_MAILBOX_INTERFACE6_LSW_DATA_MASK, (u16)(x))
+
 /* The following registers all have similar layouts; first the registers... */
 #define VEND1_GLOBAL_CFG_10M			0x0310
 #define VEND1_GLOBAL_CFG_100M			0x031b
@@ -28,6 +53,11 @@
 #define VEND1_GLOBAL_CFG_RATE_ADAPT_PAUSE	2
 
 /* Vendor specific 1, MDIO_MMD_VEND2 */
+#define VEND1_GLOBAL_CONTROL2			0xc001
+#define VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_RST	BIT(15)
+#define VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_OVD	BIT(6)
+#define VEND1_GLOBAL_CONTROL2_UP_RUN_STALL	BIT(0)
+
 #define VEND1_THERMAL_PROV_HIGH_TEMP_FAIL	0xc421
 #define VEND1_THERMAL_PROV_LOW_TEMP_FAIL	0xc422
 #define VEND1_THERMAL_PROV_HIGH_TEMP_WARN	0xc423
@@ -83,3 +113,5 @@ int aqr_hwmon_probe(struct phy_device *phydev);
 #else
 static inline int aqr_hwmon_probe(struct phy_device *phydev) { return 0; }
 #endif
+
+int aqr_firmware_load(struct phy_device *phydev);
diff --git a/drivers/net/phy/aquantia/aquantia_firmware.c b/drivers/net/phy/aquantia/aquantia_firmware.c
new file mode 100644
index 000000000000..0267ef2a231a
--- /dev/null
+++ b/drivers/net/phy/aquantia/aquantia_firmware.c
@@ -0,0 +1,367 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bitfield.h>
+#include <linux/of.h>
+#include <linux/firmware.h>
+#include <linux/crc-ccitt.h>
+#include <linux/nvmem-consumer.h>
+
+#include <asm/unaligned.h>
+
+#include "aquantia.h"
+
+#define UP_RESET_SLEEP		100
+
+/* addresses of memory segments in the phy */
+#define DRAM_BASE_ADDR		0x3FFE0000
+#define IRAM_BASE_ADDR		0x40000000
+
+/* firmware image format constants */
+#define VERSION_STRING_SIZE		0x40
+#define VERSION_STRING_OFFSET		0x0200
+/* primary offset is written at an offset from the start of the fw blob */
+#define PRIMARY_OFFSET_OFFSET		0x8
+/* primary offset needs to be then added to a base offset */
+#define PRIMARY_OFFSET_SHIFT		12
+#define PRIMARY_OFFSET(x)		((x) << PRIMARY_OFFSET_SHIFT)
+#define HEADER_OFFSET			0x300
+
+struct aqr_fw_header {
+	u32 padding;
+	u8 iram_offset[3];
+	u8 iram_size[3];
+	u8 dram_offset[3];
+	u8 dram_size[3];
+} __packed;
+
+enum aqr_fw_src {
+	AQR_FW_SRC_NVMEM = 0,
+	AQR_FW_SRC_FS,
+};
+
+static const char * const aqr_fw_src_string[] = {
+	[AQR_FW_SRC_NVMEM] = "NVMEM",
+	[AQR_FW_SRC_FS] = "FS",
+};
+
+/* AQR firmware doesn't have fixed offsets for iram and dram section
+ * but instead provide an header with the offset to use on reading
+ * and parsing the firmware.
+ *
+ * AQR firmware can't be trusted and each offset is validated to be
+ * not negative and be in the size of the firmware itself.
+ */
+static bool aqr_fw_validate_get(size_t size, size_t offset, size_t get_size)
+{
+	return offset + get_size <= size;
+}
+
+static int aqr_fw_get_be16(const u8 *data, size_t offset, size_t size, u16 *value)
+{
+	if (!aqr_fw_validate_get(size, offset, sizeof(u16)))
+		return -EINVAL;
+
+	*value = get_unaligned_be16(data + offset);
+
+	return 0;
+}
+
+static int aqr_fw_get_le16(const u8 *data, size_t offset, size_t size, u16 *value)
+{
+	if (!aqr_fw_validate_get(size, offset, sizeof(u16)))
+		return -EINVAL;
+
+	*value = get_unaligned_le16(data + offset);
+
+	return 0;
+}
+
+static int aqr_fw_get_le24(const u8 *data, size_t offset, size_t size, u32 *value)
+{
+	if (!aqr_fw_validate_get(size, offset, sizeof(u8) * 3))
+		return -EINVAL;
+
+	*value = get_unaligned_le24(data + offset);
+
+	return 0;
+}
+
+/* load data into the phy's memory */
+static int aqr_fw_load_memory(struct phy_device *phydev, u32 addr,
+			      const u8 *data, size_t len)
+{
+	u16 crc = 0, up_crc;
+	size_t pos;
+
+	/* PHY expect addr in LE */
+	addr = cpu_to_le32(addr);
+
+	phy_write_mmd(phydev, MDIO_MMD_VEND1,
+		      VEND1_GLOBAL_MAILBOX_INTERFACE1,
+		      VEND1_GLOBAL_MAILBOX_INTERFACE1_CRC_RESET);
+	phy_write_mmd(phydev, MDIO_MMD_VEND1,
+		      VEND1_GLOBAL_MAILBOX_INTERFACE3,
+		      VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR(addr));
+	phy_write_mmd(phydev, MDIO_MMD_VEND1,
+		      VEND1_GLOBAL_MAILBOX_INTERFACE4,
+		      VEND1_GLOBAL_MAILBOX_INTERFACE4_LSW_ADDR(addr));
+
+	/* We assume and enforce the size to be word aligned.
+	 * If a firmware that is not word aligned is found, please report upstream.
+	 */
+	for (pos = 0; pos < len; pos += sizeof(u32)) {
+		u32 word = get_unaligned((const u32 *)(data + pos));
+
+		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE5,
+			      VEND1_GLOBAL_MAILBOX_INTERFACE5_MSW_DATA(word));
+		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE6,
+			      VEND1_GLOBAL_MAILBOX_INTERFACE6_LSW_DATA(word));
+
+		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE1,
+			      VEND1_GLOBAL_MAILBOX_INTERFACE1_EXECUTE |
+			      VEND1_GLOBAL_MAILBOX_INTERFACE1_WRITE);
+
+		/* calculate CRC as we load data to the mailbox.
+		 * We convert word to big-endiang as PHY is BE and mailbox will
+		 * return a BE CRC.
+		 */
+		word = cpu_to_be32(word);
+		crc = crc_ccitt_false(crc, (u8 *)&word, sizeof(word));
+	}
+
+	up_crc = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE2);
+	if (crc != up_crc) {
+		phydev_err(phydev, "CRC mismatch: calculated 0x%04x PHY 0x%04x\n",
+			   crc, up_crc);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int aqr_fw_boot(struct phy_device *phydev, const u8 *data, size_t size,
+		       enum aqr_fw_src fw_src)
+{
+	u16 calculated_crc, read_crc, read_primary_offset;
+	u32 iram_offset = 0, iram_size = 0;
+	u32 dram_offset = 0, dram_size = 0;
+	char version[VERSION_STRING_SIZE];
+	u32 primary_offset = 0;
+	int ret;
+
+	/* extract saved CRC at the end of the fw
+	 * CRC is saved in big-endian as PHY is BE
+	 */
+	ret = aqr_fw_get_be16(data, size - sizeof(u16), size, &read_crc);
+	if (ret) {
+		phydev_err(phydev, "bad firmware CRC in firmware\n");
+		return ret;
+	}
+	calculated_crc = crc_ccitt_false(0, data, size - sizeof(u16));
+	if (read_crc != calculated_crc) {
+		phydev_err(phydev, "bad firmware CRC: file 0x%04x calculated 0x%04x\n",
+			   read_crc, calculated_crc);
+		return -EINVAL;
+	}
+
+	/* Get the primary offset to extract DRAM and IRAM sections. */
+	ret = aqr_fw_get_le16(data, PRIMARY_OFFSET_OFFSET, size, &read_primary_offset);
+	if (ret) {
+		phydev_err(phydev, "bad primary offset in firmware\n");
+		return ret;
+	}
+	primary_offset = PRIMARY_OFFSET(read_primary_offset);
+
+	/* Find the DRAM and IRAM sections within the firmware file.
+	 * Make sure the fw_header is correctly in the firmware.
+	 */
+	if (!aqr_fw_validate_get(size, primary_offset + HEADER_OFFSET,
+				 sizeof(struct aqr_fw_header))) {
+		phydev_err(phydev, "bad fw_header in firmware\n");
+		return -EINVAL;
+	}
+
+	/* offset are in LE and values needs to be converted to cpu endian */
+	ret = aqr_fw_get_le24(data, primary_offset + HEADER_OFFSET +
+			      offsetof(struct aqr_fw_header, iram_offset),
+			      size, &iram_offset);
+	if (ret) {
+		phydev_err(phydev, "bad iram offset in firmware\n");
+		return ret;
+	}
+	ret = aqr_fw_get_le24(data, primary_offset + HEADER_OFFSET +
+			      offsetof(struct aqr_fw_header, iram_size),
+			      size, &iram_size);
+	if (ret) {
+		phydev_err(phydev, "invalid iram size in firmware\n");
+		return ret;
+	}
+	ret = aqr_fw_get_le24(data, primary_offset + HEADER_OFFSET +
+			      offsetof(struct aqr_fw_header, dram_offset),
+			      size, &dram_offset);
+	if (ret) {
+		phydev_err(phydev, "bad dram offset in firmware\n");
+		return ret;
+	}
+	ret = aqr_fw_get_le24(data, primary_offset + HEADER_OFFSET +
+			      offsetof(struct aqr_fw_header, dram_size),
+			      size, &dram_size);
+	if (ret) {
+		phydev_err(phydev, "invalid dram size in firmware\n");
+		return ret;
+	}
+
+	/* Increment the offset with the primary offset.
+	 * Validate iram/dram offset and size.
+	 */
+	iram_offset += primary_offset;
+	if (iram_size % sizeof(u32)) {
+		phydev_err(phydev, "iram size if not aligned to word size. Please report this upstream!\n");
+		return -EINVAL;
+	}
+	if (!aqr_fw_validate_get(size, iram_offset, iram_size)) {
+		phydev_err(phydev, "invalid iram offset for iram size\n");
+		return -EINVAL;
+	}
+
+	dram_offset += primary_offset;
+	if (dram_size % sizeof(u32)) {
+		phydev_err(phydev, "dram size if not aligned to word size. Please report this upstream!\n");
+		return -EINVAL;
+	}
+	if (!aqr_fw_validate_get(size, dram_offset, dram_size)) {
+		phydev_err(phydev, "invalid iram offset for iram size\n");
+		return -EINVAL;
+	}
+
+	phydev_dbg(phydev, "primary %d IRAM offset=%d size=%d DRAM offset=%d size=%d\n",
+		   primary_offset, iram_offset, iram_size, dram_offset, dram_size);
+
+	if (!aqr_fw_validate_get(size, dram_offset + VERSION_STRING_OFFSET,
+				 VERSION_STRING_SIZE)) {
+		phydev_err(phydev, "invalid version in firmware\n");
+		return -EINVAL;
+	}
+	strscpy(version, (char *)data + dram_offset + VERSION_STRING_OFFSET,
+		VERSION_STRING_SIZE);
+	if (version[0] == '\0') {
+		phydev_err(phydev, "invalid version in firmware\n");
+		return -EINVAL;
+	}
+	phydev_info(phydev, "loading firmware version '%s' from '%s'\n", version,
+		    aqr_fw_src_string[fw_src]);
+
+	/* stall the microcprocessor */
+	phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_CONTROL2,
+		      VEND1_GLOBAL_CONTROL2_UP_RUN_STALL | VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_OVD);
+
+	phydev_dbg(phydev, "loading DRAM 0x%08x from offset=%d size=%d\n",
+		   DRAM_BASE_ADDR, dram_offset, dram_size);
+	ret = aqr_fw_load_memory(phydev, DRAM_BASE_ADDR, data + dram_offset,
+				 dram_size);
+	if (ret)
+		return ret;
+
+	phydev_dbg(phydev, "loading IRAM 0x%08x from offset=%d size=%d\n",
+		   IRAM_BASE_ADDR, iram_offset, iram_size);
+	ret = aqr_fw_load_memory(phydev, IRAM_BASE_ADDR, data + iram_offset,
+				 iram_size);
+	if (ret)
+		return ret;
+
+	/* make sure soft reset and low power mode are clear */
+	phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_SC,
+			   VEND1_GLOBAL_SC_SOFT_RESET | VEND1_GLOBAL_SC_LOW_POWER);
+
+	/* Release the microprocessor. UP_RESET must be held for 100 usec. */
+	phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_CONTROL2,
+		      VEND1_GLOBAL_CONTROL2_UP_RUN_STALL |
+		      VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_OVD |
+		      VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_RST);
+	usleep_range(UP_RESET_SLEEP, UP_RESET_SLEEP * 2);
+
+	phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_CONTROL2,
+		      VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_OVD);
+
+	return 0;
+}
+
+static int aqr_firmware_load_nvmem(struct phy_device *phydev)
+{
+	struct nvmem_cell *cell;
+	size_t size;
+	u8 *buf;
+	int ret;
+
+	cell = nvmem_cell_get(&phydev->mdio.dev, "firmware");
+	if (IS_ERR(cell))
+		return PTR_ERR(cell);
+
+	buf = nvmem_cell_read(cell, &size);
+	if (IS_ERR(buf)) {
+		ret = PTR_ERR(buf);
+		goto exit;
+	}
+
+	ret = aqr_fw_boot(phydev, buf, size, AQR_FW_SRC_NVMEM);
+	if (ret)
+		phydev_err(phydev, "firmware loading failed: %d\n", ret);
+
+exit:
+	nvmem_cell_put(cell);
+
+	return ret;
+}
+
+static int aqr_firmware_load_fs(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	const struct firmware *fw;
+	const char *fw_name;
+	int ret;
+
+	ret = of_property_read_string(dev->of_node, "firmware-name",
+				      &fw_name);
+	if (ret)
+		return ret;
+
+	ret = request_firmware(&fw, fw_name, dev);
+	if (ret) {
+		phydev_err(phydev, "failed to find FW file %s (%d)\n",
+			   fw_name, ret);
+		goto exit;
+	}
+
+	ret = aqr_fw_boot(phydev, fw->data, fw->size, AQR_FW_SRC_FS);
+	if (ret)
+		phydev_err(phydev, "firmware loading failed: %d\n", ret);
+
+exit:
+	release_firmware(fw);
+
+	return ret;
+}
+
+int aqr_firmware_load(struct phy_device *phydev)
+{
+	int ret;
+
+	/* Check if the firmware is not already loaded by pooling
+	 * the current version returned by the PHY. If 0 is returned,
+	 * no firmware is loaded.
+	 */
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_FW_ID);
+	if (ret > 0)
+		goto exit;
+
+	ret = aqr_firmware_load_nvmem(phydev);
+	if (!ret)
+		goto exit;
+
+	ret = aqr_firmware_load_fs(phydev);
+	if (ret)
+		return ret;
+
+exit:
+	return 0;
+}
diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index 4498426e9a52..cc4a97741c4a 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -658,11 +658,17 @@ static int aqr107_resume(struct phy_device *phydev)
 
 static int aqr107_probe(struct phy_device *phydev)
 {
+	int ret;
+
 	phydev->priv = devm_kzalloc(&phydev->mdio.dev,
 				    sizeof(struct aqr107_priv), GFP_KERNEL);
 	if (!phydev->priv)
 		return -ENOMEM;
 
+	ret = aqr_firmware_load(phydev);
+	if (ret)
+		return ret;
+
 	return aqr_hwmon_probe(phydev);
 }
 
-- 
2.40.1


^ permalink raw reply related

* [net-next RFC PATCH v5 4/4] dt-bindings: Document bindings for Marvell Aquantia PHY
From: Christian Marangi @ 2023-11-06 16:54 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andrew Lunn,
	Heiner Kallweit, Russell King, Christian Marangi, Robert Marko,
	Vladimir Oltean, netdev, devicetree, linux-kernel
In-Reply-To: <20231106165433.2746-1-ansuelsmth@gmail.com>

Document bindings for Marvell Aquantia PHY.

The Marvell Aquantia PHY require a firmware to work correctly and there
at least 3 way to load this firmware.

Describe all the different way and document the binding "firmware-name"
to load the PHY firmware from userspace.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
Changes v5:
- Drop extra entry not related to HW description
Changes v3:
- Make DT description more OS agnostic
- Use custom select to fix dtbs checks
Changes v2:
- Add DT patch

 .../bindings/net/marvell,aquantia.yaml        | 123 ++++++++++++++++++
 1 file changed, 123 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/marvell,aquantia.yaml

diff --git a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
new file mode 100644
index 000000000000..7106c5bdf73c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
@@ -0,0 +1,123 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell Aquantia Ethernet PHY
+
+maintainers:
+  - Christian Marangi <ansuelsmth@gmail.com>
+
+description: |
+  Marvell Aquantia Ethernet PHY require a firmware to be loaded to actually
+  work.
+
+  This can be done and is implemented by OEM in 3 different way:
+    - Attached SPI flash directly to the PHY with the firmware. The PHY
+      will self load the firmware in the presence of this configuration.
+    - Dedicated partition on system NAND with firmware in it. NVMEM
+      subsystem will be used and the declared NVMEM cell will load
+      the firmware to the PHY using the PHY mailbox interface.
+    - Manually provided firmware loaded from a file in the filesystem.
+
+allOf:
+  - $ref: ethernet-phy.yaml#
+
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - ethernet-phy-id03a1.b445
+          - ethernet-phy-id03a1.b460
+          - ethernet-phy-id03a1.b4a2
+          - ethernet-phy-id03a1.b4d0
+          - ethernet-phy-id03a1.b4e0
+          - ethernet-phy-id03a1.b5c2
+          - ethernet-phy-id03a1.b4b0
+          - ethernet-phy-id03a1.b662
+          - ethernet-phy-id03a1.b712
+          - ethernet-phy-id31c3.1c12
+  required:
+    - compatible
+
+properties:
+  reg:
+    maxItems: 1
+
+  firmware-name:
+    description: specify the name of PHY firmware to load
+
+  nvmem-cells:
+    description: phandle to the firmware nvmem cell
+    maxItems: 1
+
+  nvmem-cell-names:
+    const: firmware
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ethernet-phy@0 {
+            /*  Only needed to make DT lint tools work. Do not copy/paste
+             *  into real DTS files.
+             */
+            compatible = "ethernet-phy-id31c3.1c12",
+                         "ethernet-phy-ieee802.3-c45";
+
+            reg = <0>;
+            firmware-name = "AQR-G4_v5.4.C-AQR_CIG_WF-1945_0x8_ID44776_VER1630.cld";
+        };
+
+        ethernet-phy@1 {
+            /*  Only needed to make DT lint tools work. Do not copy/paste
+             *  into real DTS files.
+             */
+            compatible = "ethernet-phy-id31c3.1c12",
+                         "ethernet-phy-ieee802.3-c45";
+
+            reg = <0>;
+            nvmem-cells = <&aqr_fw>;
+            nvmem-cell-names = "firmware";
+        };
+    };
+
+    flash {
+        compatible = "jedec,spi-nor";
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        partitions {
+            compatible = "fixed-partitions";
+            #address-cells = <1>;
+            #size-cells = <1>;
+
+            /* ... */
+
+            partition@650000 {
+                compatible = "nvmem-cells";
+                label = "0:ethphyfw";
+                reg = <0x650000 0x80000>;
+                read-only;
+                #address-cells = <1>;
+                #size-cells = <1>;
+
+                aqr_fw: aqr_fw@0 {
+                    reg = <0x0 0x5f42a>;
+                };
+            };
+
+            /* ... */
+
+        };
+    };
-- 
2.40.1


^ permalink raw reply related

* [net-next RFC PATCH v5 2/4] net: phy: aquantia: move MMD_VEND define to header
From: Christian Marangi @ 2023-11-06 16:54 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andrew Lunn,
	Heiner Kallweit, Russell King, Christian Marangi, Robert Marko,
	Vladimir Oltean, netdev, devicetree, linux-kernel
In-Reply-To: <20231106165433.2746-1-ansuelsmth@gmail.com>

Move MMD_VEND define to header to clean things up and in preparation for
firmware loading support that require some define placed in
aquantia_main.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
Changes v4:
- Add Reviewed-by tag
Changes v3:
- Add this patch

 drivers/net/phy/aquantia/aquantia.h       | 69 +++++++++++++++++++++++
 drivers/net/phy/aquantia/aquantia_hwmon.c | 14 -----
 drivers/net/phy/aquantia/aquantia_main.c  | 55 ------------------
 3 files changed, 69 insertions(+), 69 deletions(-)

diff --git a/drivers/net/phy/aquantia/aquantia.h b/drivers/net/phy/aquantia/aquantia.h
index c684b65c642c..f0c767c4fad1 100644
--- a/drivers/net/phy/aquantia/aquantia.h
+++ b/drivers/net/phy/aquantia/aquantia.h
@@ -9,6 +9,75 @@
 #include <linux/device.h>
 #include <linux/phy.h>
 
+/* Vendor specific 1, MDIO_MMD_VEND1 */
+#define VEND1_GLOBAL_FW_ID			0x0020
+#define VEND1_GLOBAL_FW_ID_MAJOR		GENMASK(15, 8)
+#define VEND1_GLOBAL_FW_ID_MINOR		GENMASK(7, 0)
+
+/* The following registers all have similar layouts; first the registers... */
+#define VEND1_GLOBAL_CFG_10M			0x0310
+#define VEND1_GLOBAL_CFG_100M			0x031b
+#define VEND1_GLOBAL_CFG_1G			0x031c
+#define VEND1_GLOBAL_CFG_2_5G			0x031d
+#define VEND1_GLOBAL_CFG_5G			0x031e
+#define VEND1_GLOBAL_CFG_10G			0x031f
+/* ...and now the fields */
+#define VEND1_GLOBAL_CFG_RATE_ADAPT		GENMASK(8, 7)
+#define VEND1_GLOBAL_CFG_RATE_ADAPT_NONE	0
+#define VEND1_GLOBAL_CFG_RATE_ADAPT_USX		1
+#define VEND1_GLOBAL_CFG_RATE_ADAPT_PAUSE	2
+
+/* Vendor specific 1, MDIO_MMD_VEND2 */
+#define VEND1_THERMAL_PROV_HIGH_TEMP_FAIL	0xc421
+#define VEND1_THERMAL_PROV_LOW_TEMP_FAIL	0xc422
+#define VEND1_THERMAL_PROV_HIGH_TEMP_WARN	0xc423
+#define VEND1_THERMAL_PROV_LOW_TEMP_WARN	0xc424
+#define VEND1_THERMAL_STAT1			0xc820
+#define VEND1_THERMAL_STAT2			0xc821
+#define VEND1_THERMAL_STAT2_VALID		BIT(0)
+#define VEND1_GENERAL_STAT1			0xc830
+#define VEND1_GENERAL_STAT1_HIGH_TEMP_FAIL	BIT(14)
+#define VEND1_GENERAL_STAT1_LOW_TEMP_FAIL	BIT(13)
+#define VEND1_GENERAL_STAT1_HIGH_TEMP_WARN	BIT(12)
+#define VEND1_GENERAL_STAT1_LOW_TEMP_WARN	BIT(11)
+
+#define VEND1_GLOBAL_GEN_STAT2			0xc831
+#define VEND1_GLOBAL_GEN_STAT2_OP_IN_PROG	BIT(15)
+
+#define VEND1_GLOBAL_RSVD_STAT1			0xc885
+#define VEND1_GLOBAL_RSVD_STAT1_FW_BUILD_ID	GENMASK(7, 4)
+#define VEND1_GLOBAL_RSVD_STAT1_PROV_ID		GENMASK(3, 0)
+
+#define VEND1_GLOBAL_RSVD_STAT9			0xc88d
+#define VEND1_GLOBAL_RSVD_STAT9_MODE		GENMASK(7, 0)
+#define VEND1_GLOBAL_RSVD_STAT9_1000BT2		0x23
+
+#define VEND1_GLOBAL_INT_STD_STATUS		0xfc00
+#define VEND1_GLOBAL_INT_VEND_STATUS		0xfc01
+
+#define VEND1_GLOBAL_INT_STD_MASK		0xff00
+#define VEND1_GLOBAL_INT_STD_MASK_PMA1		BIT(15)
+#define VEND1_GLOBAL_INT_STD_MASK_PMA2		BIT(14)
+#define VEND1_GLOBAL_INT_STD_MASK_PCS1		BIT(13)
+#define VEND1_GLOBAL_INT_STD_MASK_PCS2		BIT(12)
+#define VEND1_GLOBAL_INT_STD_MASK_PCS3		BIT(11)
+#define VEND1_GLOBAL_INT_STD_MASK_PHY_XS1	BIT(10)
+#define VEND1_GLOBAL_INT_STD_MASK_PHY_XS2	BIT(9)
+#define VEND1_GLOBAL_INT_STD_MASK_AN1		BIT(8)
+#define VEND1_GLOBAL_INT_STD_MASK_AN2		BIT(7)
+#define VEND1_GLOBAL_INT_STD_MASK_GBE		BIT(6)
+#define VEND1_GLOBAL_INT_STD_MASK_ALL		BIT(0)
+
+#define VEND1_GLOBAL_INT_VEND_MASK		0xff01
+#define VEND1_GLOBAL_INT_VEND_MASK_PMA		BIT(15)
+#define VEND1_GLOBAL_INT_VEND_MASK_PCS		BIT(14)
+#define VEND1_GLOBAL_INT_VEND_MASK_PHY_XS	BIT(13)
+#define VEND1_GLOBAL_INT_VEND_MASK_AN		BIT(12)
+#define VEND1_GLOBAL_INT_VEND_MASK_GBE		BIT(11)
+#define VEND1_GLOBAL_INT_VEND_MASK_GLOBAL1	BIT(2)
+#define VEND1_GLOBAL_INT_VEND_MASK_GLOBAL2	BIT(1)
+#define VEND1_GLOBAL_INT_VEND_MASK_GLOBAL3	BIT(0)
+
 #if IS_REACHABLE(CONFIG_HWMON)
 int aqr_hwmon_probe(struct phy_device *phydev);
 #else
diff --git a/drivers/net/phy/aquantia/aquantia_hwmon.c b/drivers/net/phy/aquantia/aquantia_hwmon.c
index 0da451e46f69..7b3c49c3bf49 100644
--- a/drivers/net/phy/aquantia/aquantia_hwmon.c
+++ b/drivers/net/phy/aquantia/aquantia_hwmon.c
@@ -13,20 +13,6 @@
 
 #include "aquantia.h"
 
-/* Vendor specific 1, MDIO_MMD_VEND2 */
-#define VEND1_THERMAL_PROV_HIGH_TEMP_FAIL	0xc421
-#define VEND1_THERMAL_PROV_LOW_TEMP_FAIL	0xc422
-#define VEND1_THERMAL_PROV_HIGH_TEMP_WARN	0xc423
-#define VEND1_THERMAL_PROV_LOW_TEMP_WARN	0xc424
-#define VEND1_THERMAL_STAT1			0xc820
-#define VEND1_THERMAL_STAT2			0xc821
-#define VEND1_THERMAL_STAT2_VALID		BIT(0)
-#define VEND1_GENERAL_STAT1			0xc830
-#define VEND1_GENERAL_STAT1_HIGH_TEMP_FAIL	BIT(14)
-#define VEND1_GENERAL_STAT1_LOW_TEMP_FAIL	BIT(13)
-#define VEND1_GENERAL_STAT1_HIGH_TEMP_WARN	BIT(12)
-#define VEND1_GENERAL_STAT1_LOW_TEMP_WARN	BIT(11)
-
 #if IS_REACHABLE(CONFIG_HWMON)
 
 static umode_t aqr_hwmon_is_visible(const void *data,
diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index 334a6904ca5a..4498426e9a52 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -91,61 +91,6 @@
 #define MDIO_C22EXT_STAT_SGMII_TX_FRAME_ALIGN_ERR	0xd31a
 #define MDIO_C22EXT_STAT_SGMII_TX_RUNT_FRAMES		0xd31b
 
-/* Vendor specific 1, MDIO_MMD_VEND1 */
-#define VEND1_GLOBAL_FW_ID			0x0020
-#define VEND1_GLOBAL_FW_ID_MAJOR		GENMASK(15, 8)
-#define VEND1_GLOBAL_FW_ID_MINOR		GENMASK(7, 0)
-
-#define VEND1_GLOBAL_GEN_STAT2			0xc831
-#define VEND1_GLOBAL_GEN_STAT2_OP_IN_PROG	BIT(15)
-
-/* The following registers all have similar layouts; first the registers... */
-#define VEND1_GLOBAL_CFG_10M			0x0310
-#define VEND1_GLOBAL_CFG_100M			0x031b
-#define VEND1_GLOBAL_CFG_1G			0x031c
-#define VEND1_GLOBAL_CFG_2_5G			0x031d
-#define VEND1_GLOBAL_CFG_5G			0x031e
-#define VEND1_GLOBAL_CFG_10G			0x031f
-/* ...and now the fields */
-#define VEND1_GLOBAL_CFG_RATE_ADAPT		GENMASK(8, 7)
-#define VEND1_GLOBAL_CFG_RATE_ADAPT_NONE	0
-#define VEND1_GLOBAL_CFG_RATE_ADAPT_USX		1
-#define VEND1_GLOBAL_CFG_RATE_ADAPT_PAUSE	2
-
-#define VEND1_GLOBAL_RSVD_STAT1			0xc885
-#define VEND1_GLOBAL_RSVD_STAT1_FW_BUILD_ID	GENMASK(7, 4)
-#define VEND1_GLOBAL_RSVD_STAT1_PROV_ID		GENMASK(3, 0)
-
-#define VEND1_GLOBAL_RSVD_STAT9			0xc88d
-#define VEND1_GLOBAL_RSVD_STAT9_MODE		GENMASK(7, 0)
-#define VEND1_GLOBAL_RSVD_STAT9_1000BT2		0x23
-
-#define VEND1_GLOBAL_INT_STD_STATUS		0xfc00
-#define VEND1_GLOBAL_INT_VEND_STATUS		0xfc01
-
-#define VEND1_GLOBAL_INT_STD_MASK		0xff00
-#define VEND1_GLOBAL_INT_STD_MASK_PMA1		BIT(15)
-#define VEND1_GLOBAL_INT_STD_MASK_PMA2		BIT(14)
-#define VEND1_GLOBAL_INT_STD_MASK_PCS1		BIT(13)
-#define VEND1_GLOBAL_INT_STD_MASK_PCS2		BIT(12)
-#define VEND1_GLOBAL_INT_STD_MASK_PCS3		BIT(11)
-#define VEND1_GLOBAL_INT_STD_MASK_PHY_XS1	BIT(10)
-#define VEND1_GLOBAL_INT_STD_MASK_PHY_XS2	BIT(9)
-#define VEND1_GLOBAL_INT_STD_MASK_AN1		BIT(8)
-#define VEND1_GLOBAL_INT_STD_MASK_AN2		BIT(7)
-#define VEND1_GLOBAL_INT_STD_MASK_GBE		BIT(6)
-#define VEND1_GLOBAL_INT_STD_MASK_ALL		BIT(0)
-
-#define VEND1_GLOBAL_INT_VEND_MASK		0xff01
-#define VEND1_GLOBAL_INT_VEND_MASK_PMA		BIT(15)
-#define VEND1_GLOBAL_INT_VEND_MASK_PCS		BIT(14)
-#define VEND1_GLOBAL_INT_VEND_MASK_PHY_XS	BIT(13)
-#define VEND1_GLOBAL_INT_VEND_MASK_AN		BIT(12)
-#define VEND1_GLOBAL_INT_VEND_MASK_GBE		BIT(11)
-#define VEND1_GLOBAL_INT_VEND_MASK_GLOBAL1	BIT(2)
-#define VEND1_GLOBAL_INT_VEND_MASK_GLOBAL2	BIT(1)
-#define VEND1_GLOBAL_INT_VEND_MASK_GLOBAL3	BIT(0)
-
 /* Sleep and timeout for checking if the Processor-Intensive
  * MDIO operation is finished
  */
-- 
2.40.1


^ permalink raw reply related

* [net-next RFC PATCH v5 1/4] net: phy: aquantia: move to separate directory
From: Christian Marangi @ 2023-11-06 16:54 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andrew Lunn,
	Heiner Kallweit, Russell King, Christian Marangi, Robert Marko,
	Vladimir Oltean, netdev, devicetree, linux-kernel

Move aquantia PHY driver to separate driectory in preparation for
firmware loading support to keep things tidy.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
---
Changes v4:
- Keep order for kconfig config
Changes v3:
- Add this patch

 drivers/net/phy/Kconfig                         | 5 +----
 drivers/net/phy/Makefile                        | 6 +-----
 drivers/net/phy/aquantia/Kconfig                | 5 +++++
 drivers/net/phy/aquantia/Makefile               | 6 ++++++
 drivers/net/phy/{ => aquantia}/aquantia.h       | 0
 drivers/net/phy/{ => aquantia}/aquantia_hwmon.c | 0
 drivers/net/phy/{ => aquantia}/aquantia_main.c  | 0
 7 files changed, 13 insertions(+), 9 deletions(-)
 create mode 100644 drivers/net/phy/aquantia/Kconfig
 create mode 100644 drivers/net/phy/aquantia/Makefile
 rename drivers/net/phy/{ => aquantia}/aquantia.h (100%)
 rename drivers/net/phy/{ => aquantia}/aquantia_hwmon.c (100%)
 rename drivers/net/phy/{ => aquantia}/aquantia_main.c (100%)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 421d2b62918f..25cfc5ded1da 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -96,10 +96,7 @@ config ADIN1100_PHY
 	  Currently supports the:
 	  - ADIN1100 - Robust,Industrial, Low Power 10BASE-T1L Ethernet PHY
 
-config AQUANTIA_PHY
-	tristate "Aquantia PHYs"
-	help
-	  Currently supports the Aquantia AQ1202, AQ2104, AQR105, AQR405
+source "drivers/net/phy/aquantia/Kconfig"
 
 config AX88796B_PHY
 	tristate "Asix PHYs"
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index c945ed9bd14b..f65e85c91fc1 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -35,11 +35,7 @@ obj-y				+= $(sfp-obj-y) $(sfp-obj-m)
 obj-$(CONFIG_ADIN_PHY)		+= adin.o
 obj-$(CONFIG_ADIN1100_PHY)	+= adin1100.o
 obj-$(CONFIG_AMD_PHY)		+= amd.o
-aquantia-objs			+= aquantia_main.o
-ifdef CONFIG_HWMON
-aquantia-objs			+= aquantia_hwmon.o
-endif
-obj-$(CONFIG_AQUANTIA_PHY)	+= aquantia.o
+obj-$(CONFIG_AQUANTIA_PHY)	+= aquantia/
 obj-$(CONFIG_AT803X_PHY)	+= at803x.o
 obj-$(CONFIG_AX88796B_PHY)	+= ax88796b.o
 obj-$(CONFIG_BCM54140_PHY)	+= bcm54140.o
diff --git a/drivers/net/phy/aquantia/Kconfig b/drivers/net/phy/aquantia/Kconfig
new file mode 100644
index 000000000000..226146417a6a
--- /dev/null
+++ b/drivers/net/phy/aquantia/Kconfig
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config AQUANTIA_PHY
+	tristate "Aquantia PHYs"
+	help
+	  Currently supports the Aquantia AQ1202, AQ2104, AQR105, AQR405
diff --git a/drivers/net/phy/aquantia/Makefile b/drivers/net/phy/aquantia/Makefile
new file mode 100644
index 000000000000..346f350bc084
--- /dev/null
+++ b/drivers/net/phy/aquantia/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+aquantia-objs			+= aquantia_main.o
+ifdef CONFIG_HWMON
+aquantia-objs			+= aquantia_hwmon.o
+endif
+obj-$(CONFIG_AQUANTIA_PHY)	+= aquantia.o
diff --git a/drivers/net/phy/aquantia.h b/drivers/net/phy/aquantia/aquantia.h
similarity index 100%
rename from drivers/net/phy/aquantia.h
rename to drivers/net/phy/aquantia/aquantia.h
diff --git a/drivers/net/phy/aquantia_hwmon.c b/drivers/net/phy/aquantia/aquantia_hwmon.c
similarity index 100%
rename from drivers/net/phy/aquantia_hwmon.c
rename to drivers/net/phy/aquantia/aquantia_hwmon.c
diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
similarity index 100%
rename from drivers/net/phy/aquantia_main.c
rename to drivers/net/phy/aquantia/aquantia_main.c
-- 
2.40.1


^ permalink raw reply related

* Re: Bypass qdiscs?
From: David Ahern @ 2023-11-06 16:51 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: Stephen Hemminger, John Ousterhout, Andrew Lunn, netdev
In-Reply-To: <CAM0EoM=nzobHqxD45wf+DR-sAGSaxE2m-kUf__40-rekdkhhoA@mail.gmail.com>

On 11/6/23 9:17 AM, Jamal Hadi Salim wrote:
> BTW, Homa in-kernel instead of bypass is a better approach because you
> get the advantages of all other infra that the kernel offers..

yes. The memcpy that Homa currently needs (based on the netdevconf talk)
should be avoidable using the recent page pool work (RFCs).

^ permalink raw reply

* Re: [PATCH bpf-next v5 06/13] xsk: Document tx_metadata_len layout
From: Stanislav Fomichev @ 2023-11-06 16:47 UTC (permalink / raw)
  To: Simon Horman
  Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
	kpsingh, haoluo, jolsa, kuba, toke, willemb, dsahern,
	magnus.karlsson, bjorn, maciej.fijalkowski, hawk,
	yoong.siang.song, netdev, xdp-hints
In-Reply-To: <20231105124514.GD3579@kernel.org>

On Sun, Nov 5, 2023 at 4:45 AM Simon Horman <horms@kernel.org> wrote:
>
> On Thu, Nov 02, 2023 at 03:58:30PM -0700, Stanislav Fomichev wrote:
> > - how to use
> > - how to query features
> > - pointers to the examples
> >
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
>
> ...
>
> > diff --git a/Documentation/networking/xsk-tx-metadata.rst b/Documentation/networking/xsk-tx-metadata.rst
> > new file mode 100644
> > index 000000000000..4f376560b23f
> > --- /dev/null
> > +++ b/Documentation/networking/xsk-tx-metadata.rst
> > @@ -0,0 +1,70 @@
>
> Hi Stan,
>
> a minor nit from my side: an SPDX licence identifier tag should probably go
> here.

Ugh, thanks, not sure how I missed it :-(

^ permalink raw reply

* Re: [net-next RFC PATCH v3 4/4] dt-bindings: Document bindings for Marvell Aquantia PHY
From: Christian Marangi @ 2023-11-06 16:47 UTC (permalink / raw)
  To: Conor Dooley
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andrew Lunn,
	Heiner Kallweit, Russell King, Robert Marko, Vladimir Oltean,
	netdev, devicetree, linux-kernel
In-Reply-To: <20231103-outboard-murkiness-e3256874c9a7@spud>

On Fri, Nov 03, 2023 at 01:08:45PM +0000, Conor Dooley wrote:
> Yo,
> 
> On Thu, Nov 02, 2023 at 04:00:32PM +0100, Christian Marangi wrote:
> > Document bindings for Marvell Aquantia PHY.
> > 
> > The Marvell Aquantia PHY require a firmware to work correctly and there
> > at least 3 way to load this firmware.
> > 
> > Describe all the different way and document the binding "firmware-name"
> > to load the PHY firmware from userspace.
> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> > Changes v3:
> > - Make DT description more OS agnostic
> > - Use custom select to fix dtbs checks
> > Changes v2:
> > - Add DT patch
> > 
> >  .../bindings/net/marvell,aquantia.yaml        | 126 ++++++++++++++++++
> >  1 file changed, 126 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> > new file mode 100644
> > index 000000000000..d43cf28a4d61
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> > @@ -0,0 +1,126 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Marvell Aquantia Ethernet PHY
> > +
> > +maintainers:
> > +  - Christian Marangi <ansuelsmth@gmail.com>
> > +
> > +description: |
> > +  Marvell Aquantia Ethernet PHY require a firmware to be loaded to actually
> > +  work.
> > +
> > +  This can be done and is implemented by OEM in 3 different way:
> > +    - Attached SPI directly to the PHY with the firmware. The PHY will
> 
> You a word here? Should that not be "SPI flash"?
>

Added!

> > +      self load the firmware in the presence of this configuration.
> 
> > +    - Dedicated partition on system NAND with firmware in it. NVMEM
> > +      subsystem will be used and the declared NVMEM cell will load
> > +      the firmware to the PHY using the PHY mailbox interface.
> > +    - Manually provided firmware loaded from a file in the filesystem.
> > +
> > +  If declared, NVMEM will always take priority over filesystem provided
> > +  firmware.
> 
> This section here reads entirely like "software policy". The first
> bullet in your list is fine - as that is what the PHY will do itself.
> The second and third bullets here seem like two different ways that
> someone could integrate their system, and I am not objecting to either
> of those ways of doing things.
> The priority system that you mention however I don't think is suitable
> for a description of the hardware - the PHY itself doesn't require that
> an external-to-it flash device take priority over something in the
> filesystem, right?
> 

Yes the priority system is just something in software and nothing about
hardware. I dropped in v5. Thanks for the review!

-- 
	Ansuel

^ permalink raw reply

* Re: Bypass qdiscs?
From: Jamal Hadi Salim @ 2023-11-06 16:17 UTC (permalink / raw)
  To: David Ahern; +Cc: Stephen Hemminger, John Ousterhout, Andrew Lunn, netdev
In-Reply-To: <CAM0EoMm+x2eOVbn_NMDYVu4tEjccvvHObt0OSPvCibMAfiNs5w@mail.gmail.com>

On Mon, Nov 6, 2023 at 11:12 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On Sun, Nov 5, 2023 at 11:27 PM David Ahern <dsahern@kernel.org> wrote:
> >
> > On 11/5/23 8:23 PM, Stephen Hemminger wrote:
> > > On Sat, 4 Nov 2023 19:47:30 -0700
> > > John Ousterhout <ouster@cs.stanford.edu> wrote:
> > >
> > >> I haven't tried creating a "pass through" qdisc, but that seems like a
> > >> reasonable approach if (as it seems) there isn't something already
> > >> built-in that provides equivalent functionality.
> > >>
> > >> -John-
> > >>
> > >> P.S. If hardware starts supporting Homa, I hope that it will be
> > >> possible to move the entire transport to the NIC, so that applications
> > >> can bypass the kernel entirely, as with RDMA.
> > >
> > > One old trick was setting netdev queue length to 0 to avoid qdisc.
> > >
> >
> > tc qdisc replace dev <name> root noqueue
> >
> > should work
>
> John,
> IIUC,  Homa transmit is done by  a pacer that ensures the packets are
> scheduled without forming the queues in the NIC. So what David said
> above should be sufficient setup.


BTW, Homa in-kernel instead of bypass is a better approach because you
get the advantages of all other infra that the kernel offers..

cheers,
jamal

> cheers,
> jamal
> >

^ permalink raw reply

* Re: Bypass qdiscs?
From: Jamal Hadi Salim @ 2023-11-06 16:12 UTC (permalink / raw)
  To: David Ahern; +Cc: Stephen Hemminger, John Ousterhout, Andrew Lunn, netdev
In-Reply-To: <b80374c7-3f5a-4f47-8955-c16d14e7549a@kernel.org>

On Sun, Nov 5, 2023 at 11:27 PM David Ahern <dsahern@kernel.org> wrote:
>
> On 11/5/23 8:23 PM, Stephen Hemminger wrote:
> > On Sat, 4 Nov 2023 19:47:30 -0700
> > John Ousterhout <ouster@cs.stanford.edu> wrote:
> >
> >> I haven't tried creating a "pass through" qdisc, but that seems like a
> >> reasonable approach if (as it seems) there isn't something already
> >> built-in that provides equivalent functionality.
> >>
> >> -John-
> >>
> >> P.S. If hardware starts supporting Homa, I hope that it will be
> >> possible to move the entire transport to the NIC, so that applications
> >> can bypass the kernel entirely, as with RDMA.
> >
> > One old trick was setting netdev queue length to 0 to avoid qdisc.
> >
>
> tc qdisc replace dev <name> root noqueue
>
> should work

John,
IIUC,  Homa transmit is done by  a pacer that ensures the packets are
scheduled without forming the queues in the NIC. So what David said
above should be sufficient setup.

cheers,
jamal
>

^ permalink raw reply

* [PATCH net] net: enetc: shorten enetc_setup_xdp_prog() error message to fit NETLINK_MAX_FMTMSG_LEN
From: Vladimir Oltean @ 2023-11-06 16:03 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	oe-kbuild-all, linux-kernel, Simon Horman, Claudiu Manoil,
	kernel test robot

NETLINK_MAX_FMTMSG_LEN is currently hardcoded to 80, and we provide an
error printf-formatted string having 96 characters including the
terminating \0. Assuming each %d (representing a queue) gets replaced by
a number having at most 2 digits (a reasonable assumption), the final
string is also 96 characters wide, which is too much.

Reduce the verbiage a bit by removing some (partially) redundant words,
which makes the new printf-formatted string be 73 characters wide with
the trailing newline.

Fixes: 800db2d125c2 ("net: enetc: ensure we always have a minimum number of TXQs for stack")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/lkml/202311061336.4dsWMT1h-lkp@intel.com/
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index 30bec47bc665..cffbf27c4656 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -2769,7 +2769,7 @@ static int enetc_setup_xdp_prog(struct net_device *ndev, struct bpf_prog *prog,
 	if (priv->min_num_stack_tx_queues + num_xdp_tx_queues >
 	    priv->num_tx_rings) {
 		NL_SET_ERR_MSG_FMT_MOD(extack,
-				       "Reserving %d XDP TXQs does not leave a minimum of %d TXQs for network stack (total %d available)",
+				       "Reserving %d XDP TXQs does not leave a minimum of %d for stack (total %d)",
 				       num_xdp_tx_queues,
 				       priv->min_num_stack_tx_queues,
 				       priv->num_tx_rings);
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH net v2] tcp: Fix -Wc23-extensions in tcp_options_write()
From: Nathan Chancellor @ 2023-11-06 15:58 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: davem, dsahern, kuba, pabeni, ndesaulniers, trix, 0x7f454c46,
	noureddine, hch, netdev, linux-kernel, llvm, patches
In-Reply-To: <CANn89i+GF=4QuVMevE7Ur2Zi0nDjBujMHWJayURR9fbcr+McnA@mail.gmail.com>

On Mon, Nov 06, 2023 at 04:52:52PM +0100, Eric Dumazet wrote:
> On Mon, Nov 6, 2023 at 4:36 PM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > Clang warns (or errors with CONFIG_WERROR=y) when CONFIG_TCP_AO is set:
> >
> >   net/ipv4/tcp_output.c:663:2: error: label at end of compound statement is a C23 extension [-Werror,-Wc23-extensions]
> >     663 |         }
> >         |         ^
> >   1 error generated.
> >
> > On earlier releases (such as clang-11, the current minimum supported
> > version for building the kernel) that do not support C23, this was a
> > hard error unconditionally:
> >
> >   net/ipv4/tcp_output.c:663:2: error: expected statement
> >           }
> >           ^
> >   1 error generated.
> >
> > While adding a semicolon after the label would resolve this, it is more
> > in line with the kernel as a whole to refactor this block into a
> > standalone function, which means the goto a label construct can just be
> > replaced with a simple return. Do so to resolve the warning.
> >
> > Closes: https://github.com/ClangBuiltLinux/linux/issues/1953
> > Fixes: 1e03d32bea8e ("net/tcp: Add TCP-AO sign to outgoing packets")
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> > Please let me know if this function should have a different name. I
> > think I got all the changes of the function shuffle correct but some
> > testing would be appreciated.
> >
> > Changes in v2:
> > - Break out problematic block into its own function so that goto can be
> >   replaced with a simple return, instead of the simple semicolon
> >   approach of v1 (Christoph)
> > - Link to v1: https://lore.kernel.org/r/20231031-tcp-ao-fix-label-in-compound-statement-warning-v1-1-c9731d115f17@kernel.org
> > ---
> >  net/ipv4/tcp_output.c | 69 ++++++++++++++++++++++++++++-----------------------
> >  1 file changed, 38 insertions(+), 31 deletions(-)
> >
> > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> > index 0d8dd5b7e2e5..3f8dc74fbf40 100644
> > --- a/net/ipv4/tcp_output.c
> > +++ b/net/ipv4/tcp_output.c
> > @@ -601,6 +601,43 @@ static void bpf_skops_write_hdr_opt(struct sock *sk, struct sk_buff *skb,
> >  }
> >  #endif
> >
> > +static void process_tcp_ao_options(struct tcp_sock *tp,
> > +                                  const struct tcp_request_sock *tcprsk,
> > +                                  struct tcp_out_options *opts,
> > +                                  struct tcp_out_options *opts,
> 
> ptr has a different type than in the caller, this is a bit confusing
> 
> I would use
> 
> static __be32 * process_tcp_ao_options(struct tcp_sock *tp, const
> struct tcp_request_sock *tcprsk,
>                   struct tcp_out_options *opts,struct tcp_key *key, __be32 *ptr)

Ah, this suggestion is much better, thanks. I'll make this adjustment
and send a v3 later today in case others have any suggested changes (I
know netdev prefers waiting 24 hours for another revision but I'd like
to get this warning cleared up by -rc1 so it does not proliferate into
other trees and I sent v1 almost a week ago).

> > +{
> > +#ifdef CONFIG_TCP_AO
> > +       u8 maclen = tcp_ao_maclen(key->ao_key);
> > +
> > +       if (tcprsk) {
> > +               u8 aolen = maclen + sizeof(struct tcp_ao_hdr);
> > +
> > +               *(*ptr)++ = htonl((TCPOPT_AO << 24) | (aolen << 16) |
> > +                                 (tcprsk->ao_keyid << 8) |
> > +                                 (tcprsk->ao_rcv_next));
> 
> 
>                  *ptr++ = ...
> 
> (and in all other *ptr uses in this helper)
> 
> > +       } else {
> > +               struct tcp_ao_key *rnext_key;
> > +               struct tcp_ao_info *ao_info;
> > +
> > +               ao_info = rcu_dereference_check(tp->ao_info,
> > +                       lockdep_sock_is_held(&tp->inet_conn.icsk_inet.sk));
> > +               rnext_key = READ_ONCE(ao_info->rnext_key);
> > +               if (WARN_ON_ONCE(!rnext_key))
> > +                       return;
> > +               *(*ptr)++ = htonl((TCPOPT_AO << 24) |
> > +                                 (tcp_ao_len(key->ao_key) << 16) |
> > +                                 (key->ao_key->sndid << 8) |
> > +                                 (rnext_key->rcvid));
> > +       }
> > +       opts->hash_location = (__u8 *)(*ptr);
> > +       *ptr += maclen / sizeof(**ptr);
> > +       if (unlikely(maclen % sizeof(**ptr))) {
> > +               memset(*ptr, TCPOPT_NOP, sizeof(**ptr));
> > +               (*ptr)++;
> > +       }
> > +#endif
> 
>     return ptr;
> +}
> > +
> >  /* Write previously computed TCP options to the packet.
> >   *
> >   * Beware: Something in the Internet is very sensitive to the ordering of
> > @@ -629,37 +666,7 @@ static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp,
> >                 opts->hash_location = (__u8 *)ptr;
> >                 ptr += 4;
> >         } else if (tcp_key_is_ao(key)) {
> > -#ifdef CONFIG_TCP_AO
> > -               u8 maclen = tcp_ao_maclen(key->ao_key);
> > -
> > -               if (tcprsk) {
> > -                       u8 aolen = maclen + sizeof(struct tcp_ao_hdr);
> > -
> > -                       *ptr++ = htonl((TCPOPT_AO << 24) | (aolen << 16) |
> > -                                      (tcprsk->ao_keyid << 8) |
> > -                                      (tcprsk->ao_rcv_next));
> > -               } else {
> > -                       struct tcp_ao_key *rnext_key;
> > -                       struct tcp_ao_info *ao_info;
> > -
> > -                       ao_info = rcu_dereference_check(tp->ao_info,
> > -                               lockdep_sock_is_held(&tp->inet_conn.icsk_inet.sk));
> > -                       rnext_key = READ_ONCE(ao_info->rnext_key);
> > -                       if (WARN_ON_ONCE(!rnext_key))
> > -                               goto out_ao;
> > -                       *ptr++ = htonl((TCPOPT_AO << 24) |
> > -                                      (tcp_ao_len(key->ao_key) << 16) |
> > -                                      (key->ao_key->sndid << 8) |
> > -                                      (rnext_key->rcvid));
> > -               }
> > -               opts->hash_location = (__u8 *)ptr;
> > -               ptr += maclen / sizeof(*ptr);
> > -               if (unlikely(maclen % sizeof(*ptr))) {
> > -                       memset(ptr, TCPOPT_NOP, sizeof(*ptr));
> > -                       ptr++;
> > -               }
> > -out_ao:
> > -#endif
> > +               process_tcp_ao_options(tp, tcprsk, opts, key, &ptr);
> 
> ptr = process_tcp_ao_options(tp, tcprsk, opts, key, ptr);
> 
> 
> >         }
> >         if (unlikely(opts->mss)) {
> >                 *ptr++ = htonl((TCPOPT_MSS << 24) |
> >
> > ---
> > base-commit: c1ed833e0b3b7b9edc82b97b73b2a8a10ceab241
> > change-id: 20231031-tcp-ao-fix-label-in-compound-statement-warning-ebd6c9978498
> >
> > Best regards,
> > --
> > Nathan Chancellor <nathan@kernel.org>
> >

^ permalink raw reply

* Re: [PATCH net v2] tcp: Fix -Wc23-extensions in tcp_options_write()
From: Eric Dumazet @ 2023-11-06 15:52 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: davem, dsahern, kuba, pabeni, ndesaulniers, trix, 0x7f454c46,
	noureddine, hch, netdev, linux-kernel, llvm, patches
In-Reply-To: <20231106-tcp-ao-fix-label-in-compound-statement-warning-v2-1-91eff6e1648c@kernel.org>

On Mon, Nov 6, 2023 at 4:36 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> Clang warns (or errors with CONFIG_WERROR=y) when CONFIG_TCP_AO is set:
>
>   net/ipv4/tcp_output.c:663:2: error: label at end of compound statement is a C23 extension [-Werror,-Wc23-extensions]
>     663 |         }
>         |         ^
>   1 error generated.
>
> On earlier releases (such as clang-11, the current minimum supported
> version for building the kernel) that do not support C23, this was a
> hard error unconditionally:
>
>   net/ipv4/tcp_output.c:663:2: error: expected statement
>           }
>           ^
>   1 error generated.
>
> While adding a semicolon after the label would resolve this, it is more
> in line with the kernel as a whole to refactor this block into a
> standalone function, which means the goto a label construct can just be
> replaced with a simple return. Do so to resolve the warning.
>
> Closes: https://github.com/ClangBuiltLinux/linux/issues/1953
> Fixes: 1e03d32bea8e ("net/tcp: Add TCP-AO sign to outgoing packets")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
> Please let me know if this function should have a different name. I
> think I got all the changes of the function shuffle correct but some
> testing would be appreciated.
>
> Changes in v2:
> - Break out problematic block into its own function so that goto can be
>   replaced with a simple return, instead of the simple semicolon
>   approach of v1 (Christoph)
> - Link to v1: https://lore.kernel.org/r/20231031-tcp-ao-fix-label-in-compound-statement-warning-v1-1-c9731d115f17@kernel.org
> ---
>  net/ipv4/tcp_output.c | 69 ++++++++++++++++++++++++++++-----------------------
>  1 file changed, 38 insertions(+), 31 deletions(-)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 0d8dd5b7e2e5..3f8dc74fbf40 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -601,6 +601,43 @@ static void bpf_skops_write_hdr_opt(struct sock *sk, struct sk_buff *skb,
>  }
>  #endif
>
> +static void process_tcp_ao_options(struct tcp_sock *tp,
> +                                  const struct tcp_request_sock *tcprsk,
> +                                  struct tcp_out_options *opts,
> +                                  struct tcp_out_options *opts,

ptr has a different type than in the caller, this is a bit confusing

I would use

static __be32 * process_tcp_ao_options(struct tcp_sock *tp, const
struct tcp_request_sock *tcprsk,
                  struct tcp_out_options *opts,struct tcp_key *key, __be32 *ptr)

> +{
> +#ifdef CONFIG_TCP_AO
> +       u8 maclen = tcp_ao_maclen(key->ao_key);
> +
> +       if (tcprsk) {
> +               u8 aolen = maclen + sizeof(struct tcp_ao_hdr);
> +
> +               *(*ptr)++ = htonl((TCPOPT_AO << 24) | (aolen << 16) |
> +                                 (tcprsk->ao_keyid << 8) |
> +                                 (tcprsk->ao_rcv_next));


                 *ptr++ = ...

(and in all other *ptr uses in this helper)

> +       } else {
> +               struct tcp_ao_key *rnext_key;
> +               struct tcp_ao_info *ao_info;
> +
> +               ao_info = rcu_dereference_check(tp->ao_info,
> +                       lockdep_sock_is_held(&tp->inet_conn.icsk_inet.sk));
> +               rnext_key = READ_ONCE(ao_info->rnext_key);
> +               if (WARN_ON_ONCE(!rnext_key))
> +                       return;
> +               *(*ptr)++ = htonl((TCPOPT_AO << 24) |
> +                                 (tcp_ao_len(key->ao_key) << 16) |
> +                                 (key->ao_key->sndid << 8) |
> +                                 (rnext_key->rcvid));
> +       }
> +       opts->hash_location = (__u8 *)(*ptr);
> +       *ptr += maclen / sizeof(**ptr);
> +       if (unlikely(maclen % sizeof(**ptr))) {
> +               memset(*ptr, TCPOPT_NOP, sizeof(**ptr));
> +               (*ptr)++;
> +       }
> +#endif

    return ptr;
+}
> +
>  /* Write previously computed TCP options to the packet.
>   *
>   * Beware: Something in the Internet is very sensitive to the ordering of
> @@ -629,37 +666,7 @@ static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp,
>                 opts->hash_location = (__u8 *)ptr;
>                 ptr += 4;
>         } else if (tcp_key_is_ao(key)) {
> -#ifdef CONFIG_TCP_AO
> -               u8 maclen = tcp_ao_maclen(key->ao_key);
> -
> -               if (tcprsk) {
> -                       u8 aolen = maclen + sizeof(struct tcp_ao_hdr);
> -
> -                       *ptr++ = htonl((TCPOPT_AO << 24) | (aolen << 16) |
> -                                      (tcprsk->ao_keyid << 8) |
> -                                      (tcprsk->ao_rcv_next));
> -               } else {
> -                       struct tcp_ao_key *rnext_key;
> -                       struct tcp_ao_info *ao_info;
> -
> -                       ao_info = rcu_dereference_check(tp->ao_info,
> -                               lockdep_sock_is_held(&tp->inet_conn.icsk_inet.sk));
> -                       rnext_key = READ_ONCE(ao_info->rnext_key);
> -                       if (WARN_ON_ONCE(!rnext_key))
> -                               goto out_ao;
> -                       *ptr++ = htonl((TCPOPT_AO << 24) |
> -                                      (tcp_ao_len(key->ao_key) << 16) |
> -                                      (key->ao_key->sndid << 8) |
> -                                      (rnext_key->rcvid));
> -               }
> -               opts->hash_location = (__u8 *)ptr;
> -               ptr += maclen / sizeof(*ptr);
> -               if (unlikely(maclen % sizeof(*ptr))) {
> -                       memset(ptr, TCPOPT_NOP, sizeof(*ptr));
> -                       ptr++;
> -               }
> -out_ao:
> -#endif
> +               process_tcp_ao_options(tp, tcprsk, opts, key, &ptr);

ptr = process_tcp_ao_options(tp, tcprsk, opts, key, ptr);


>         }
>         if (unlikely(opts->mss)) {
>                 *ptr++ = htonl((TCPOPT_MSS << 24) |
>
> ---
> base-commit: c1ed833e0b3b7b9edc82b97b73b2a8a10ceab241
> change-id: 20231031-tcp-ao-fix-label-in-compound-statement-warning-ebd6c9978498
>
> Best regards,
> --
> Nathan Chancellor <nathan@kernel.org>
>

^ permalink raw reply

* Re: [PATCH iwl-net v2] ice: fix DDP package download for packages without signature segment
From: Maciej Fijalkowski @ 2023-11-06 15:40 UTC (permalink / raw)
  To: Paul Greenwalt
  Cc: intel-wired-lan, netdev, jesse.brandeburg, anthony.l.nguyen,
	davem, kuba, horms, tony.brelinski, Dan Nowlin
In-Reply-To: <20231106151808.421280-2-paul.greenwalt@intel.com>

On Mon, Nov 06, 2023 at 10:18:08AM -0500, Paul Greenwalt wrote:
> From: Dan Nowlin <dan.nowlin@intel.com>
> 
> Commit 3cbdb0343022 ("ice: Add support for E830 DDP package segment")
> incorrectly removed support for package download for packages without a
> signature segment. These packages include the signature buffer inline
> in the configurations buffers, and not in a signature segment.
> 
> Fix package download by providing download support for both packages
> with (ice_download_pkg_with_sig_seg()) and without signature segment
> (ice_download_pkg_without_sig_seg()).
> 
> Fixes: 3cbdb0343022 ("ice: Add support for E830 DDP package segment")
> Reported-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Signed-off-by: Dan Nowlin <dan.nowlin@intel.com>
> Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com>
> ---
> Changelog
> v2->v3:

so is that a v3 or v2? subject says v2.

You should include tested-by and reviewed-by tags provided by me and
Wojciech.

> - correct Reported-by email address.
> ___
> 
>  drivers/net/ethernet/intel/ice/ice_ddp.c | 106 ++++++++++++++++++++++-
>  1 file changed, 103 insertions(+), 3 deletions(-)
> 

[...]

^ permalink raw reply

* [PATCH net v2] tcp: Fix -Wc23-extensions in tcp_options_write()
From: Nathan Chancellor @ 2023-11-06 15:36 UTC (permalink / raw)
  To: edumazet, davem, dsahern, kuba, pabeni
  Cc: ndesaulniers, trix, 0x7f454c46, noureddine, hch, netdev,
	linux-kernel, llvm, patches, Nathan Chancellor

Clang warns (or errors with CONFIG_WERROR=y) when CONFIG_TCP_AO is set:

  net/ipv4/tcp_output.c:663:2: error: label at end of compound statement is a C23 extension [-Werror,-Wc23-extensions]
    663 |         }
        |         ^
  1 error generated.

On earlier releases (such as clang-11, the current minimum supported
version for building the kernel) that do not support C23, this was a
hard error unconditionally:

  net/ipv4/tcp_output.c:663:2: error: expected statement
          }
          ^
  1 error generated.

While adding a semicolon after the label would resolve this, it is more
in line with the kernel as a whole to refactor this block into a
standalone function, which means the goto a label construct can just be
replaced with a simple return. Do so to resolve the warning.

Closes: https://github.com/ClangBuiltLinux/linux/issues/1953
Fixes: 1e03d32bea8e ("net/tcp: Add TCP-AO sign to outgoing packets")
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
Please let me know if this function should have a different name. I
think I got all the changes of the function shuffle correct but some
testing would be appreciated.

Changes in v2:
- Break out problematic block into its own function so that goto can be
  replaced with a simple return, instead of the simple semicolon
  approach of v1 (Christoph)
- Link to v1: https://lore.kernel.org/r/20231031-tcp-ao-fix-label-in-compound-statement-warning-v1-1-c9731d115f17@kernel.org
---
 net/ipv4/tcp_output.c | 69 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 38 insertions(+), 31 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 0d8dd5b7e2e5..3f8dc74fbf40 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -601,6 +601,43 @@ static void bpf_skops_write_hdr_opt(struct sock *sk, struct sk_buff *skb,
 }
 #endif
 
+static void process_tcp_ao_options(struct tcp_sock *tp,
+				   const struct tcp_request_sock *tcprsk,
+				   struct tcp_out_options *opts,
+				   struct tcp_key *key, __be32 **ptr)
+{
+#ifdef CONFIG_TCP_AO
+	u8 maclen = tcp_ao_maclen(key->ao_key);
+
+	if (tcprsk) {
+		u8 aolen = maclen + sizeof(struct tcp_ao_hdr);
+
+		*(*ptr)++ = htonl((TCPOPT_AO << 24) | (aolen << 16) |
+				  (tcprsk->ao_keyid << 8) |
+				  (tcprsk->ao_rcv_next));
+	} else {
+		struct tcp_ao_key *rnext_key;
+		struct tcp_ao_info *ao_info;
+
+		ao_info = rcu_dereference_check(tp->ao_info,
+			lockdep_sock_is_held(&tp->inet_conn.icsk_inet.sk));
+		rnext_key = READ_ONCE(ao_info->rnext_key);
+		if (WARN_ON_ONCE(!rnext_key))
+			return;
+		*(*ptr)++ = htonl((TCPOPT_AO << 24) |
+				  (tcp_ao_len(key->ao_key) << 16) |
+				  (key->ao_key->sndid << 8) |
+				  (rnext_key->rcvid));
+	}
+	opts->hash_location = (__u8 *)(*ptr);
+	*ptr += maclen / sizeof(**ptr);
+	if (unlikely(maclen % sizeof(**ptr))) {
+		memset(*ptr, TCPOPT_NOP, sizeof(**ptr));
+		(*ptr)++;
+	}
+#endif
+}
+
 /* Write previously computed TCP options to the packet.
  *
  * Beware: Something in the Internet is very sensitive to the ordering of
@@ -629,37 +666,7 @@ static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp,
 		opts->hash_location = (__u8 *)ptr;
 		ptr += 4;
 	} else if (tcp_key_is_ao(key)) {
-#ifdef CONFIG_TCP_AO
-		u8 maclen = tcp_ao_maclen(key->ao_key);
-
-		if (tcprsk) {
-			u8 aolen = maclen + sizeof(struct tcp_ao_hdr);
-
-			*ptr++ = htonl((TCPOPT_AO << 24) | (aolen << 16) |
-				       (tcprsk->ao_keyid << 8) |
-				       (tcprsk->ao_rcv_next));
-		} else {
-			struct tcp_ao_key *rnext_key;
-			struct tcp_ao_info *ao_info;
-
-			ao_info = rcu_dereference_check(tp->ao_info,
-				lockdep_sock_is_held(&tp->inet_conn.icsk_inet.sk));
-			rnext_key = READ_ONCE(ao_info->rnext_key);
-			if (WARN_ON_ONCE(!rnext_key))
-				goto out_ao;
-			*ptr++ = htonl((TCPOPT_AO << 24) |
-				       (tcp_ao_len(key->ao_key) << 16) |
-				       (key->ao_key->sndid << 8) |
-				       (rnext_key->rcvid));
-		}
-		opts->hash_location = (__u8 *)ptr;
-		ptr += maclen / sizeof(*ptr);
-		if (unlikely(maclen % sizeof(*ptr))) {
-			memset(ptr, TCPOPT_NOP, sizeof(*ptr));
-			ptr++;
-		}
-out_ao:
-#endif
+		process_tcp_ao_options(tp, tcprsk, opts, key, &ptr);
 	}
 	if (unlikely(opts->mss)) {
 		*ptr++ = htonl((TCPOPT_MSS << 24) |

---
base-commit: c1ed833e0b3b7b9edc82b97b73b2a8a10ceab241
change-id: 20231031-tcp-ao-fix-label-in-compound-statement-warning-ebd6c9978498

Best regards,
-- 
Nathan Chancellor <nathan@kernel.org>


^ permalink raw reply related

* [PATCH iwl-net v2] ice: fix DDP package download for packages without signature segment
From: Paul Greenwalt @ 2023-11-06 15:18 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, jesse.brandeburg, anthony.l.nguyen, davem, kuba, horms,
	tony.brelinski, Dan Nowlin, Maciej Fijalkowski, Paul Greenwalt
In-Reply-To: <20231106151808.421280-1-paul.greenwalt@intel.com>

From: Dan Nowlin <dan.nowlin@intel.com>

Commit 3cbdb0343022 ("ice: Add support for E830 DDP package segment")
incorrectly removed support for package download for packages without a
signature segment. These packages include the signature buffer inline
in the configurations buffers, and not in a signature segment.

Fix package download by providing download support for both packages
with (ice_download_pkg_with_sig_seg()) and without signature segment
(ice_download_pkg_without_sig_seg()).

Fixes: 3cbdb0343022 ("ice: Add support for E830 DDP package segment")
Reported-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Dan Nowlin <dan.nowlin@intel.com>
Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com>
---
Changelog
v2->v3:
- correct Reported-by email address.
___

 drivers/net/ethernet/intel/ice/ice_ddp.c | 106 ++++++++++++++++++++++-
 1 file changed, 103 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ethernet/intel/ice/ice_ddp.c
index cfb1580f5850..3f1a11d0252c 100644
--- a/drivers/net/ethernet/intel/ice/ice_ddp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ddp.c
@@ -1479,14 +1479,14 @@ ice_post_dwnld_pkg_actions(struct ice_hw *hw)
 }
 
 /**
- * ice_download_pkg
+ * ice_download_pkg_with_sig_seg
  * @hw: pointer to the hardware structure
  * @pkg_hdr: pointer to package header
  *
  * Handles the download of a complete package.
  */
 static enum ice_ddp_state
-ice_download_pkg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr)
+ice_download_pkg_with_sig_seg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr)
 {
 	enum ice_aq_err aq_err = hw->adminq.sq_last_status;
 	enum ice_ddp_state state = ICE_DDP_PKG_ERR;
@@ -1519,6 +1519,106 @@ ice_download_pkg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr)
 		state = ice_post_dwnld_pkg_actions(hw);
 
 	ice_release_global_cfg_lock(hw);
+
+	return state;
+}
+
+/**
+ * ice_dwnld_cfg_bufs
+ * @hw: pointer to the hardware structure
+ * @bufs: pointer to an array of buffers
+ * @count: the number of buffers in the array
+ *
+ * Obtains global config lock and downloads the package configuration buffers
+ * to the firmware.
+ */
+static enum ice_ddp_state
+ice_dwnld_cfg_bufs(struct ice_hw *hw, struct ice_buf *bufs, u32 count)
+{
+	enum ice_ddp_state state = ICE_DDP_PKG_SUCCESS;
+	struct ice_buf_hdr *bh;
+	int status;
+
+	if (!bufs || !count)
+		return ICE_DDP_PKG_ERR;
+
+	/* If the first buffer's first section has its metadata bit set
+	 * then there are no buffers to be downloaded, and the operation is
+	 * considered a success.
+	 */
+	bh = (struct ice_buf_hdr *)bufs;
+	if (le32_to_cpu(bh->section_entry[0].type) & ICE_METADATA_BUF)
+		return ICE_DDP_PKG_SUCCESS;
+
+	status = ice_acquire_global_cfg_lock(hw, ICE_RES_WRITE);
+	if (status) {
+		if (status == -EALREADY)
+			return ICE_DDP_PKG_ALREADY_LOADED;
+		return ice_map_aq_err_to_ddp_state(hw->adminq.sq_last_status);
+	}
+
+	state = ice_dwnld_cfg_bufs_no_lock(hw, bufs, 0, count, true);
+	if (!state)
+		state = ice_post_dwnld_pkg_actions(hw);
+
+	ice_release_global_cfg_lock(hw);
+
+	return state;
+}
+
+/**
+ * ice_download_pkg_without_sig_seg
+ * @hw: pointer to the hardware structure
+ * @ice_seg: pointer to the segment of the package to be downloaded
+ *
+ * Handles the download of a complete package without signature segment.
+ */
+static enum ice_ddp_state
+ice_download_pkg_without_sig_seg(struct ice_hw *hw, struct ice_seg *ice_seg)
+{
+	struct ice_buf_table *ice_buf_tbl;
+	enum ice_ddp_state state;
+
+	ice_debug(hw, ICE_DBG_PKG, "Segment format version: %d.%d.%d.%d\n",
+		  ice_seg->hdr.seg_format_ver.major,
+		  ice_seg->hdr.seg_format_ver.minor,
+		  ice_seg->hdr.seg_format_ver.update,
+		  ice_seg->hdr.seg_format_ver.draft);
+
+	ice_debug(hw, ICE_DBG_PKG, "Seg: type 0x%X, size %d, name %s\n",
+		  le32_to_cpu(ice_seg->hdr.seg_type),
+		  le32_to_cpu(ice_seg->hdr.seg_size), ice_seg->hdr.seg_id);
+
+	ice_buf_tbl = ice_find_buf_table(ice_seg);
+
+	ice_debug(hw, ICE_DBG_PKG, "Seg buf count: %d\n",
+		  le32_to_cpu(ice_buf_tbl->buf_count));
+
+	state = ice_dwnld_cfg_bufs(hw, ice_buf_tbl->buf_array,
+				   le32_to_cpu(ice_buf_tbl->buf_count));
+
+	return state;
+}
+
+/**
+ * ice_download_pkg
+ * @hw: pointer to the hardware structure
+ * @pkg_hdr: pointer to package header
+ * @ice_seg: pointer to the segment of the package to be downloaded
+ *
+ * Handles the download of a complete package.
+ */
+static enum ice_ddp_state
+ice_download_pkg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr,
+		 struct ice_seg *ice_seg)
+{
+	enum ice_ddp_state state;
+
+	if (hw->pkg_has_signing_seg)
+		state = ice_download_pkg_with_sig_seg(hw, pkg_hdr);
+	else
+		state = ice_download_pkg_without_sig_seg(hw, ice_seg);
+
 	ice_post_pkg_dwnld_vlan_mode_cfg(hw);
 
 	return state;
@@ -2083,7 +2183,7 @@ enum ice_ddp_state ice_init_pkg(struct ice_hw *hw, u8 *buf, u32 len)
 
 	/* initialize package hints and then download package */
 	ice_init_pkg_hints(hw, seg);
-	state = ice_download_pkg(hw, pkg);
+	state = ice_download_pkg(hw, pkg, seg);
 	if (state == ICE_DDP_PKG_ALREADY_LOADED) {
 		ice_debug(hw, ICE_DBG_INIT,
 			  "package previously loaded - no work.\n");

base-commit: 016b9332a3346e97a6cacffea0f9dc10e1235a75
-- 
2.41.0


^ permalink raw reply related

* [PATCH iwl-net] ice: fix DDP package download for packages without signature segment
From: Paul Greenwalt @ 2023-11-06 15:18 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, jesse.brandeburg, anthony.l.nguyen, davem, kuba, horms,
	tony.brelinski, Dan Nowlin, Maciej Fijalkowski, Paul Greenwalt

From: Dan Nowlin <dan.nowlin@intel.com>

Commit 3cbdb0343022 ("ice: Add support for E830 DDP package segment")
incorrectly removed support for package download for packages without a
signature segment. These packages include the signature buffer inline
in the configurations buffers, and do not in a signature segment.

Fix package download by providing download support for both packages
with (ice_download_pkg_with_sig_seg()) and without signature segment
(ice_download_pkg_without_sig_seg()).

Fixes: 3cbdb0343022 ("ice: Add support for E830 DDP package segment")
Reported-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Dan Nowlin <dan.nowlin@intel.com>
Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com>
---
Changelog
v1->v2:
Fix Reported-by email.
---

 drivers/net/ethernet/intel/ice/ice_ddp.c | 106 ++++++++++++++++++++++-
 1 file changed, 103 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ethernet/intel/ice/ice_ddp.c
index cfb1580f5850..3f1a11d0252c 100644
--- a/drivers/net/ethernet/intel/ice/ice_ddp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ddp.c
@@ -1479,14 +1479,14 @@ ice_post_dwnld_pkg_actions(struct ice_hw *hw)
 }
 
 /**
- * ice_download_pkg
+ * ice_download_pkg_with_sig_seg
  * @hw: pointer to the hardware structure
  * @pkg_hdr: pointer to package header
  *
  * Handles the download of a complete package.
  */
 static enum ice_ddp_state
-ice_download_pkg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr)
+ice_download_pkg_with_sig_seg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr)
 {
 	enum ice_aq_err aq_err = hw->adminq.sq_last_status;
 	enum ice_ddp_state state = ICE_DDP_PKG_ERR;
@@ -1519,6 +1519,106 @@ ice_download_pkg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr)
 		state = ice_post_dwnld_pkg_actions(hw);
 
 	ice_release_global_cfg_lock(hw);
+
+	return state;
+}
+
+/**
+ * ice_dwnld_cfg_bufs
+ * @hw: pointer to the hardware structure
+ * @bufs: pointer to an array of buffers
+ * @count: the number of buffers in the array
+ *
+ * Obtains global config lock and downloads the package configuration buffers
+ * to the firmware.
+ */
+static enum ice_ddp_state
+ice_dwnld_cfg_bufs(struct ice_hw *hw, struct ice_buf *bufs, u32 count)
+{
+	enum ice_ddp_state state = ICE_DDP_PKG_SUCCESS;
+	struct ice_buf_hdr *bh;
+	int status;
+
+	if (!bufs || !count)
+		return ICE_DDP_PKG_ERR;
+
+	/* If the first buffer's first section has its metadata bit set
+	 * then there are no buffers to be downloaded, and the operation is
+	 * considered a success.
+	 */
+	bh = (struct ice_buf_hdr *)bufs;
+	if (le32_to_cpu(bh->section_entry[0].type) & ICE_METADATA_BUF)
+		return ICE_DDP_PKG_SUCCESS;
+
+	status = ice_acquire_global_cfg_lock(hw, ICE_RES_WRITE);
+	if (status) {
+		if (status == -EALREADY)
+			return ICE_DDP_PKG_ALREADY_LOADED;
+		return ice_map_aq_err_to_ddp_state(hw->adminq.sq_last_status);
+	}
+
+	state = ice_dwnld_cfg_bufs_no_lock(hw, bufs, 0, count, true);
+	if (!state)
+		state = ice_post_dwnld_pkg_actions(hw);
+
+	ice_release_global_cfg_lock(hw);
+
+	return state;
+}
+
+/**
+ * ice_download_pkg_without_sig_seg
+ * @hw: pointer to the hardware structure
+ * @ice_seg: pointer to the segment of the package to be downloaded
+ *
+ * Handles the download of a complete package without signature segment.
+ */
+static enum ice_ddp_state
+ice_download_pkg_without_sig_seg(struct ice_hw *hw, struct ice_seg *ice_seg)
+{
+	struct ice_buf_table *ice_buf_tbl;
+	enum ice_ddp_state state;
+
+	ice_debug(hw, ICE_DBG_PKG, "Segment format version: %d.%d.%d.%d\n",
+		  ice_seg->hdr.seg_format_ver.major,
+		  ice_seg->hdr.seg_format_ver.minor,
+		  ice_seg->hdr.seg_format_ver.update,
+		  ice_seg->hdr.seg_format_ver.draft);
+
+	ice_debug(hw, ICE_DBG_PKG, "Seg: type 0x%X, size %d, name %s\n",
+		  le32_to_cpu(ice_seg->hdr.seg_type),
+		  le32_to_cpu(ice_seg->hdr.seg_size), ice_seg->hdr.seg_id);
+
+	ice_buf_tbl = ice_find_buf_table(ice_seg);
+
+	ice_debug(hw, ICE_DBG_PKG, "Seg buf count: %d\n",
+		  le32_to_cpu(ice_buf_tbl->buf_count));
+
+	state = ice_dwnld_cfg_bufs(hw, ice_buf_tbl->buf_array,
+				   le32_to_cpu(ice_buf_tbl->buf_count));
+
+	return state;
+}
+
+/**
+ * ice_download_pkg
+ * @hw: pointer to the hardware structure
+ * @pkg_hdr: pointer to package header
+ * @ice_seg: pointer to the segment of the package to be downloaded
+ *
+ * Handles the download of a complete package.
+ */
+static enum ice_ddp_state
+ice_download_pkg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr,
+		 struct ice_seg *ice_seg)
+{
+	enum ice_ddp_state state;
+
+	if (hw->pkg_has_signing_seg)
+		state = ice_download_pkg_with_sig_seg(hw, pkg_hdr);
+	else
+		state = ice_download_pkg_without_sig_seg(hw, ice_seg);
+
 	ice_post_pkg_dwnld_vlan_mode_cfg(hw);
 
 	return state;
@@ -2083,7 +2183,7 @@ enum ice_ddp_state ice_init_pkg(struct ice_hw *hw, u8 *buf, u32 len)
 
 	/* initialize package hints and then download package */
 	ice_init_pkg_hints(hw, seg);
-	state = ice_download_pkg(hw, pkg);
+	state = ice_download_pkg(hw, pkg, seg);
 	if (state == ICE_DDP_PKG_ALREADY_LOADED) {
 		ice_debug(hw, ICE_DBG_INIT,
 			  "package previously loaded - no work.\n");

base-commit: 016b9332a3346e97a6cacffea0f9dc10e1235a75
-- 
2.41.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox