Message-ID: <de980a49-b802-417a-a57e-2c47f67b08e4@leemhuis.info>
Date: Mon, 3 Jun 2024 09:09:12 +0200
From: Thorsten Leemhuis <linux@...mhuis.info>
To: MD Danish Anwar <danishanwar@...com>,
 Dan Carpenter <dan.carpenter@...aro.org>, Jan Kiszka
 <jan.kiszka@...mens.com>, Simon Horman <horms@...nel.org>,
 Andrew Lunn <andrew@...n.ch>, Vladimir Oltean <vladimir.oltean@....com>,
 Wolfram Sang <wsa+renesas@...g-engineering.com>,
 Niklas Schnelle <schnelle@...ux.ibm.com>, Arnd Bergmann <arnd@...db.de>,
 Diogo Ivo <diogo.ivo@...mens.com>, Roger Quadros <rogerq@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Jakub Kicinski <kuba@...nel.org>,
 Eric Dumazet <edumazet@...gle.com>, "David S. Miller" <davem@...emloft.net>
Cc: linux-arm-kernel@...ts.infradead.org, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, srk@...com,
 Vignesh Raghavendra <vigneshr@...com>
Subject: "ERROR: modpost: "icssg_queue_pop" [...] undefined" on arm64 (was:
 [PATCH net-next v6 1/3] net: ti: icssg-prueth: Add helper functions to
 configure FDB)

On 28.05.24 13:37, MD Danish Anwar wrote:
> Introduce helper functions to configure firmware FDB tables, VLAN tables
> and Port VLAN ID settings to aid adding Switch mode support.
> 
> Signed-off-by: MD Danish Anwar <danishanwar@...com>

Hi! Since Friday I get a compile error in my -next builds for Fedora:

ERROR: modpost: "icssg_queue_push"
[drivers/net/ethernet/ti/icssg-prueth-sr1.ko] undefined!
ERROR: modpost: "icssg_queue_pop"
[drivers/net/ethernet/ti/icssg-prueth-sr1.ko] undefined!

Looks like this problem was found and reported mid May by the kernel
test robot already, which identified a earlier version of the patch I'm
replying to to be the cause:
https://lore.kernel.org/all/202405182038.ncf1mL7Z-lkp@intel.com/

That and the fact that the patch showed up in -next on Friday makes me
assume that my problem is caused by this change as well as well. A build
log can be found here:
https://download.copr.fedorainfracloud.org/results/@kernel-vanilla/next/fedora-39-aarch64/07523690-next-next-all/builder-live.log.gz

I don't have the .config at hand, but can provide it when needed.

Ciao, Thorsten

> ---
>  drivers/net/ethernet/ti/icssg/icssg_config.c | 170 +++++++++++++++++++
>  drivers/net/ethernet/ti/icssg/icssg_config.h |  19 +++
>  drivers/net/ethernet/ti/icssg/icssg_prueth.h |  12 ++
>  3 files changed, 201 insertions(+)
> 
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c
> index 15f2235bf90f..2213374d4d45 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_config.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c
> @@ -477,3 +477,173 @@ void icssg_config_set_speed(struct prueth_emac *emac)
>  
>  	writeb(fw_speed, emac->dram.va + PORT_LINK_SPEED_OFFSET);
>  }
> +
> +int icssg_send_fdb_msg(struct prueth_emac *emac, struct mgmt_cmd *cmd,
> +		       struct mgmt_cmd_rsp *rsp)
> +{
> +	struct prueth *prueth = emac->prueth;
> +	int slice = prueth_emac_slice(emac);
> +	int addr, ret;
> +
> +	addr = icssg_queue_pop(prueth, slice == 0 ?
> +			       ICSSG_CMD_POP_SLICE0 : ICSSG_CMD_POP_SLICE1);
> +	if (addr < 0)
> +		return addr;
> +
> +	/* First 4 bytes have FW owned buffer linking info which should
> +	 * not be touched
> +	 */
> +	memcpy_toio(prueth->shram.va + addr + 4, cmd, sizeof(*cmd));
> +	icssg_queue_push(prueth, slice == 0 ?
> +			 ICSSG_CMD_PUSH_SLICE0 : ICSSG_CMD_PUSH_SLICE1, addr);
> +	ret = read_poll_timeout(icssg_queue_pop, addr, addr >= 0,
> +				2000, 20000000, false, prueth, slice == 0 ?
> +				ICSSG_RSP_POP_SLICE0 : ICSSG_RSP_POP_SLICE1);
> +	if (ret) {
> +		netdev_err(emac->ndev, "Timedout sending HWQ message\n");
> +		return ret;
> +	}
> +
> +	memcpy_fromio(rsp, prueth->shram.va + addr, sizeof(*rsp));
> +	/* Return buffer back for to pool */
> +	icssg_queue_push(prueth, slice == 0 ?
> +			 ICSSG_RSP_PUSH_SLICE0 : ICSSG_RSP_PUSH_SLICE1, addr);
> +
> +	return 0;
> +}
> +
> +static void icssg_fdb_setup(struct prueth_emac *emac, struct mgmt_cmd *fdb_cmd,
> +			    const unsigned char *addr, u8 fid, int cmd)
> +{
> +	int slice = prueth_emac_slice(emac);
> +	u8 mac_fid[ETH_ALEN + 2];
> +	u16 fdb_slot;
> +
> +	ether_addr_copy(mac_fid, addr);
> +
> +	/* 1-1 VID-FID mapping is already setup */
> +	mac_fid[ETH_ALEN] = fid;
> +	mac_fid[ETH_ALEN + 1] = 0;
> +
> +	fdb_slot = bitrev32(crc32_le(0, mac_fid, 8)) & PRUETH_SWITCH_FDB_MASK;
> +
> +	fdb_cmd->header = ICSSG_FW_MGMT_CMD_HEADER;
> +	fdb_cmd->type   = ICSSG_FW_MGMT_FDB_CMD_TYPE;
> +	fdb_cmd->seqnum = ++(emac->prueth->icssg_hwcmdseq);
> +	fdb_cmd->param  = cmd;
> +	fdb_cmd->param |= (slice << 4);
> +
> +	memcpy(&fdb_cmd->cmd_args[0], addr, 4);
> +	memcpy(&fdb_cmd->cmd_args[1], &addr[4], 2);
> +	fdb_cmd->cmd_args[2] = fdb_slot;
> +
> +	netdev_dbg(emac->ndev, "MAC %pM slot %X FID %X\n", addr, fdb_slot, fid);
> +}
> +
> +int icssg_fdb_add_del(struct prueth_emac *emac, const unsigned char *addr,
> +		      u8 vid, u8 fid_c2, bool add)
> +{
> +	struct mgmt_cmd_rsp fdb_cmd_rsp = { 0 };
> +	struct mgmt_cmd fdb_cmd = { 0 };
> +	u8 fid = vid;
> +	int ret;
> +
> +	icssg_fdb_setup(emac, &fdb_cmd, addr, fid, add ? ICSS_CMD_ADD_FDB : ICSS_CMD_DEL_FDB);
> +
> +	fid_c2 |= ICSSG_FDB_ENTRY_VALID;
> +	fdb_cmd.cmd_args[1] |= ((fid << 16) | (fid_c2 << 24));
> +
> +	ret = icssg_send_fdb_msg(emac, &fdb_cmd, &fdb_cmd_rsp);
> +	if (ret)
> +		return ret;
> +
> +	WARN_ON(fdb_cmd.seqnum != fdb_cmd_rsp.seqnum);
> +	if (fdb_cmd_rsp.status == 1)
> +		return 0;
> +
> +	return -EINVAL;
> +}
> +
> +int icssg_fdb_lookup(struct prueth_emac *emac, const unsigned char *addr,
> +		     u8 vid)
> +{
> +	struct mgmt_cmd_rsp fdb_cmd_rsp = { 0 };
> +	struct mgmt_cmd fdb_cmd = { 0 };
> +	struct prueth_fdb_slot *slot;
> +	u8 fid = vid;
> +	int ret, i;
> +
> +	icssg_fdb_setup(emac, &fdb_cmd, addr, fid, ICSS_CMD_GET_FDB_SLOT);
> +
> +	fdb_cmd.cmd_args[1] |= fid << 16;
> +
> +	ret = icssg_send_fdb_msg(emac, &fdb_cmd, &fdb_cmd_rsp);
> +	if (ret)
> +		return ret;
> +
> +	WARN_ON(fdb_cmd.seqnum != fdb_cmd_rsp.seqnum);
> +
> +	slot = (struct prueth_fdb_slot __force *)(emac->dram.va + FDB_CMD_BUFFER);
> +	for (i = 0; i < 4; i++) {
> +		if (ether_addr_equal(addr, slot->mac) && vid == slot->fid)
> +			return (slot->fid_c2 & ~ICSSG_FDB_ENTRY_VALID);
> +		slot++;
> +	}
> +
> +	return 0;
> +}
> +
> +void icssg_vtbl_modify(struct prueth_emac *emac, u8 vid, u8 port_mask,
> +		       u8 untag_mask, bool add)
> +{
> +	struct prueth *prueth = emac->prueth;
> +	struct prueth_vlan_tbl *tbl;
> +	u8 fid_c1;
> +
> +	tbl = prueth->vlan_tbl;
> +	fid_c1 = tbl[vid].fid_c1;
> +
> +	/* FID_C1: bit0..2 port membership mask,
> +	 * bit3..5 tagging mask for each port
> +	 * bit6 Stream VID (not handled currently)
> +	 * bit7 MC flood (not handled currently)
> +	 */
> +	if (add) {
> +		fid_c1 |= (port_mask | port_mask << 3);
> +		fid_c1 &= ~(untag_mask << 3);
> +	} else {
> +		fid_c1 &= ~(port_mask | port_mask << 3);
> +	}
> +
> +	tbl[vid].fid_c1 = fid_c1;
> +}
> +
> +u16 icssg_get_pvid(struct prueth_emac *emac)
> +{
> +	struct prueth *prueth = emac->prueth;
> +	u32 pvid;
> +
> +	if (emac->port_id == PRUETH_PORT_MII0)
> +		pvid = readl(prueth->shram.va + EMAC_ICSSG_SWITCH_PORT1_DEFAULT_VLAN_OFFSET);
> +	else
> +		pvid = readl(prueth->shram.va + EMAC_ICSSG_SWITCH_PORT2_DEFAULT_VLAN_OFFSET);
> +
> +	pvid = pvid >> 24;
> +
> +	return pvid;
> +}
> +
> +void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port)
> +{
> +	u32 pvid;
> +
> +	/* only 256 VLANs are supported */
> +	pvid = (u32 __force)cpu_to_be32((ETH_P_8021Q << 16) | (vid & 0xff));
> +
> +	if (port == PRUETH_PORT_MII0)
> +		writel(pvid, prueth->shram.va + EMAC_ICSSG_SWITCH_PORT1_DEFAULT_VLAN_OFFSET);
> +	else if (port == PRUETH_PORT_MII1)
> +		writel(pvid, prueth->shram.va + EMAC_ICSSG_SWITCH_PORT2_DEFAULT_VLAN_OFFSET);
> +	else
> +		writel(pvid, prueth->shram.va + EMAC_ICSSG_SWITCH_PORT0_DEFAULT_VLAN_OFFSET);
> +}
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
> index cf2ea4bd22a2..4a9721aa6057 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_config.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
> @@ -35,6 +35,8 @@ struct icssg_flow_cfg {
>  	(2 * (PRUETH_EMAC_BUF_POOL_SIZE * PRUETH_NUM_BUF_POOLS + \
>  	 PRUETH_EMAC_RX_CTX_BUF_SIZE * 2))
>  
> +#define PRUETH_SWITCH_FDB_MASK ((SIZE_OF_FDB / NUMBER_OF_FDB_BUCKET_ENTRIES) - 1)
> +
>  struct icssg_rxq_ctx {
>  	__le32 start[3];
>  	__le32 end;
> @@ -202,6 +204,23 @@ struct icssg_setclock_desc {
>  #define ICSSG_TS_PUSH_SLICE0	40
>  #define ICSSG_TS_PUSH_SLICE1	41
>  
> +struct mgmt_cmd {
> +	u8 param;
> +	u8 seqnum;
> +	u8 type;
> +	u8 header;
> +	u32 cmd_args[3];
> +};
> +
> +struct mgmt_cmd_rsp {
> +	u32 reserved;
> +	u8 status;
> +	u8 seqnum;
> +	u8 type;
> +	u8 header;
> +	u32 cmd_args[3];
> +};
> +
>  /* FDB FID_C2 flag definitions */
>  /* Indicates host port membership.*/
>  #define ICSSG_FDB_ENTRY_P0_MEMBERSHIP         BIT(0)
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> index a78c5eb75fb8..82bdad9702c3 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> @@ -232,6 +232,7 @@ struct icssg_firmwares {
>   * @emacs_initialized: num of EMACs/ext ports that are up/running
>   * @iep0: pointer to IEP0 device
>   * @iep1: pointer to IEP1 device
> + * @vlan_tbl: VLAN-FID table pointer
>   */
>  struct prueth {
>  	struct device *dev;
> @@ -256,6 +257,7 @@ struct prueth {
>  	int emacs_initialized;
>  	struct icss_iep *iep0;
>  	struct icss_iep *iep1;
> +	struct prueth_vlan_tbl *vlan_tbl;
>  };
>  
>  struct emac_tx_ts_response {
> @@ -313,6 +315,16 @@ int icssg_queue_pop(struct prueth *prueth, u8 queue);
>  void icssg_queue_push(struct prueth *prueth, int queue, u16 addr);
>  u32 icssg_queue_level(struct prueth *prueth, int queue);
>  
> +int icssg_send_fdb_msg(struct prueth_emac *emac, struct mgmt_cmd *cmd,
> +		       struct mgmt_cmd_rsp *rsp);
> +int icssg_fdb_add_del(struct prueth_emac *emac,  const unsigned char *addr,
> +		      u8 vid, u8 fid_c2, bool add);
> +int icssg_fdb_lookup(struct prueth_emac *emac, const unsigned char *addr,
> +		     u8 vid);
> +void icssg_vtbl_modify(struct prueth_emac *emac, u8 vid, u8 port_mask,
> +		       u8 untag_mask, bool add);
> +u16 icssg_get_pvid(struct prueth_emac *emac);
> +void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port);
>  #define prueth_napi_to_tx_chn(pnapi) \
>  	container_of(pnapi, struct prueth_tx_chn, napi_tx)
>  

Powered by blists - more mailing lists