Re: [v4,net-next,03/11] net/nebula-matrix: add chip related definitions

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email says "This is an automated AI code reviewer, please treat what
it says with a grain of salt."

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net/nebula-matrix: add chip related definitions

This commit adds hardware register definitions for the Leonis chip,
including PPE, DP, and interface module base addresses, as well as
P4 configuration data used during chip initialization.

> diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw.h b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw.h
> new file mode 100644
> index 000000000000..f657c1e6a5ad
> --- /dev/null
> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw.h
> @@ -0,0 +1,128 @@
> +/* SPDX-License-Identifier: GPL-2.0*/

This isn't a bug, but the SPDX license identifier is missing a space
before the closing comment. The kernel standard format is:

    /* SPDX-License-Identifier: GPL-2.0 */

[ ... ]

> diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_hw_leonis.h b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_hw_leonis.h
> index 240d356f18b0..9000650256ca 100644
> --- a/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_hw_leonis.h
> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_hw_leonis.h

[ ... ]

> +#define NBL_DP_UL4S_BASE			(0x00204000)

[ ... ]

> +#define NBL_DP_DL4S_BASE			(0x00614000)

[ ... ]

> +#define NBL_DP_DL4S_BASE			(0x00614000)
> +#define NBL_DP_UL4S_BASE			(0x00204000)

NBL_DP_DL4S_BASE and NBL_DP_UL4S_BASE are defined twice in this file.
While the values are identical, would it make sense to remove the
duplicate definitions?

[ ... ]

> +/*  ----------  DSCH  ----------  */
> +/* DSCH vn_host_qid_max */
> +#define NBL_DSCH_NOTIFY_BITMAP_ARR(i) \
> +	(NBL_DP_DSCH_BASE + 0x00003000 + (i) * BYTES_PER_DWORD)
> +#define NBL_DSCH_FLY_BITMAP_ARR(i) \
> +	(NBL_DP_DSCH_BASE + 0x00004000 + (i) * BYTES_PER_DWORD)

BYTES_PER_DWORD is not defined in this driver. Will this cause
compilation errors when these macros are expanded? Consider defining
BYTES_PER_DWORD or using the already-defined NBL_BYTES_IN_REG.

> +#define NBL_DSCH_PORT_MAP_REG_ARR(i) \
> +	(NBL_DP_DSCH_BASE + 0x00005000 + (i) * sizeof(struct nbl_port_map))

struct nbl_port_map is referenced but not defined anywhere in the
nebula-matrix driver. Is this type defined elsewhere, or is a definition
missing?

[ ... ]

> +/* epro epro_kgen_ft table */
> +#define NBL_EPRO_KGEN_FT_TABLE(i)         \
> +	(NBL_PPE_EPRO_BASE + 0x00001980 + \
> +	 (i) * sizeof(struct nbl_epro_kgen_ft_tbl))

struct nbl_epro_kgen_ft_tbl is referenced but not defined in this
driver. Is a definition missing for this type?

[ ... ]

> diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_hw_leonis_regs.h b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_hw_leonis_regs.h
> new file mode 100644
> index 000000000000..75b7dd783354
> --- /dev/null
> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_hw_leonis_regs.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0*/

This isn't a bug, but the SPDX license identifier is missing a space
before the closing comment. The kernel standard format is:

    /* SPDX-License-Identifier: GPL-2.0 */