Re: [PATCH net-next v02 01/14] hinic3: HW initialization

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 28/08/2025 13:10, Fan Gong wrote:
> Add the hardware resource data structures, functions for HW initialization,
> configuration and releasement.
> 
> Co-developed-by: Xin Guo <guoxin09@huawei.com>
> Signed-off-by: Xin Guo <guoxin09@huawei.com>
> Co-developed-by: Zhu Yikai <zhuyikai1@h-partners.com>
> Signed-off-by: Zhu Yikai <zhuyikai1@h-partners.com>
> Signed-off-by: Fan Gong <gongfan1@huawei.com>
> ---
>   .../net/ethernet/huawei/hinic3/hinic3_hwdev.c |  53 +++-
>   .../net/ethernet/huawei/hinic3/hinic3_hwif.c  | 227 ++++++++++++++++++
>   .../net/ethernet/huawei/hinic3/hinic3_hwif.h  |  13 +
>   .../net/ethernet/huawei/hinic3/hinic3_lld.c   |   3 +-
>   .../huawei/hinic3/hinic3_pci_id_tbl.h         |   9 +
>   5 files changed, 301 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_hwdev.c b/drivers/net/ethernet/huawei/hinic3/hinic3_hwdev.c
> index 6e8788a64925..5bd5745f4b96 100644
> --- a/drivers/net/ethernet/huawei/hinic3/hinic3_hwdev.c
> +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_hwdev.c

[...]

> +/* Init attr struct from HW attr values. */
> +static void init_hwif_attr(struct hinic3_func_attr *attr, u32 attr0, u32 attr1,
> +			   u32 attr2, u32 attr3, u32 attr6)
> +{
> +	attr->func_global_idx = HINIC3_AF0_GET(attr0, FUNC_GLOBAL_IDX);
> +	attr->port_to_port_idx = HINIC3_AF0_GET(attr0, P2P_IDX);
> +	attr->pci_intf_idx = HINIC3_AF0_GET(attr0, PCI_INTF_IDX);
> +	attr->func_type = HINIC3_AF0_GET(attr0, FUNC_TYPE);
> +
> +	attr->num_aeqs = BIT(HINIC3_AF1_GET(attr1, AEQS_PER_FUNC));
> +	attr->num_ceqs = HINIC3_AF2_GET(attr2, CEQS_PER_FUNC);
> +	attr->num_irqs = HINIC3_AF2_GET(attr2, IRQS_PER_FUNC);
> +	if (attr->num_irqs > HINIC3_MAX_MSIX_ENTRY)
> +		attr->num_irqs = HINIC3_MAX_MSIX_ENTRY;
> +
> +	attr->num_sq = HINIC3_AF6_GET(attr6, FUNC_MAX_SQ);
> +	attr->msix_flex_en = HINIC3_AF6_GET(attr6, MSIX_FLEX_EN);
> +}
> +
> +/* Get device attributes from HW. */
> +static int get_hwif_attr(struct hinic3_hwdev *hwdev)
> +{
> +	u32 attr0, attr1, attr2, attr3, attr6;
> +	struct hinic3_hwif *hwif;
> +
> +	hwif = hwdev->hwif;
> +	attr0  = hinic3_hwif_read_reg(hwif, HINIC3_CSR_FUNC_ATTR0_ADDR);
> +	attr1  = hinic3_hwif_read_reg(hwif, HINIC3_CSR_FUNC_ATTR1_ADDR);
> +	attr2  = hinic3_hwif_read_reg(hwif, HINIC3_CSR_FUNC_ATTR2_ADDR);
> +	attr3  = hinic3_hwif_read_reg(hwif, HINIC3_CSR_FUNC_ATTR3_ADDR);
> +	attr6  = hinic3_hwif_read_reg(hwif, HINIC3_CSR_FUNC_ATTR6_ADDR);
> +	init_hwif_attr(&hwif->attr, attr0, attr1, attr2, attr3, attr6);

well, get_hwif_attr() name is misleading here, as the function doesn't
only read values, it also sets some of them. if there is no other users
of init function, it might be better to merge them.

> +
> +	return 0;

there is no way the function can return error - what's the reason to
have return value?