[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3F4725F9F2582F05+20250813092457.GA972027@nic-Precision-5820-Tower>
Date: Wed, 13 Aug 2025 17:24:57 +0800
From: Yibo Dong <dong100@...se.com>
To: MD Danish Anwar <danishanwar@...com>
Cc: andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, horms@...nel.org,
corbet@....net, gur.stavi@...wei.com, maddy@...ux.ibm.com,
mpe@...erman.id.au, lee@...ger.us, gongfan1@...wei.com,
lorenzo@...nel.org, geert+renesas@...der.be,
Parthiban.Veerasooran@...rochip.com, lukas.bulwahn@...hat.com,
alexanderduyck@...com, richardcochran@...il.com,
netdev@...r.kernel.org, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 4/5] net: rnpgbe: Add basic mbx_fw support
On Wed, Aug 13, 2025 at 01:50:08PM +0530, MD Danish Anwar wrote:
> On 12/08/25 3:09 pm, Dong Yibo wrote:
> > +/**
> > + * mucse_fw_get_capability - Get hw abilities from fw
> > + * @hw: pointer to the HW structure
> > + * @abil: pointer to the hw_abilities structure
> > + *
> > + * mucse_fw_get_capability tries to get hw abilities from
> > + * hw.
> > + *
> > + * @return: 0 on success, negative on failure
> > + **/
> > +static int mucse_fw_get_capability(struct mucse_hw *hw,
> > + struct hw_abilities *abil)
> > +{
> > + struct mbx_fw_cmd_reply reply;
> > + struct mbx_fw_cmd_req req;
> > + int err;
> > +
> > + memset(&req, 0, sizeof(req));
> > + memset(&reply, 0, sizeof(reply));
> > + build_phy_abalities_req(&req, &req);
>
> Typo in function name. You probably meant "build_phy_abilities_req".
>
You are right, I will update it.
> > + err = mucse_fw_send_cmd_wait(hw, &req, &reply);
> > + if (!err)
> > + memcpy(abil, &reply.hw_abilities, sizeof(*abil));
> > + return err;
> > +}
> > +
> > +/**
> > + * mucse_mbx_get_capability - Get hw abilities from fw
> > + * @hw: pointer to the HW structure
> > + *
> > + * mucse_mbx_get_capability tries to some capabities from
> > + * hw. Many retrys will do if it is failed.
> > + *
>
> Typo in comment: "tries to some capabities" should be "tries to get
> capabilities"
>
Got it, I will fix it.
> > + * @return: 0 on success, negative on failure
> > + **/
> > +int mucse_mbx_get_capability(struct mucse_hw *hw)
> > +{
> > + struct hw_abilities ability;
> > + int try_cnt = 3;
> > + int err;
> > +
> > + memset(&ability, 0, sizeof(ability));
> > + while (try_cnt--) {
> > + err = mucse_fw_get_capability(hw, &ability);
> > + if (err)
> > + continue;
> > + hw->pfvfnum = le16_to_cpu(ability.pfnum);
> > + hw->fw_version = le32_to_cpu(ability.fw_version);
> > + hw->axi_mhz = le32_to_cpu(ability.axi_mhz);
> > + hw->bd_uid = le32_to_cpu(ability.bd_uid);
> > + return 0;
> > + }
> > + return err;
> > +}
>
>
> Missing initialization of err variable before the last return, which
> could lead to undefined behavior if all attempts fail.
>
Got it, I will init it by 'int err = -EIO'.
> > +
> > +/**
> > + * mbx_cookie_zalloc - Alloc a cookie structure
> > + * @priv_len: private length for this cookie
> > + *
>
>
> --
> Thanks and Regards,
> Danish
>
>
Thanks for your feedback.
Powered by blists - more mailing lists