netdev - Re: [PATCH net-next 2/2] amd-xgbe: add support for rx-adaptation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c1ddd073-2494-9c8a-0f61-8330d7d6bb0c@amd.com>
Date:   Tue, 31 Jan 2023 15:26:14 +0530
From:   Raju Rangoju <Raju.Rangoju@....com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
        pabeni@...hat.com, Shyam-sundar.S-k@....com
Subject: Re: [PATCH net-next 2/2] amd-xgbe: add support for rx-adaptation



On 1/27/2023 1:13 PM, Raju Rangoju wrote:
> 
> On 1/26/2023 12:34 PM, Jakub Kicinski wrote:
>> On Wed, 25 Jan 2023 12:55:29 +0530 Raju Rangoju wrote:
>>> --- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
>>> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
>>> @@ -387,7 +387,13 @@ struct xgbe_phy_data {
>>>   /* I2C, MDIO and GPIO lines are muxed, so only one device at a time */
>>>   static DEFINE_MUTEX(xgbe_phy_comm_lock);
>>> +static void xgbe_phy_perform_ratechange(struct xgbe_prv_data *pdata,
>>> +                    unsigned int cmd, unsigned int sub_cmd);
>>>   static enum xgbe_an_mode xgbe_phy_an_mode(struct xgbe_prv_data 
>>> *pdata);
>>> +static void xgbe_phy_rrc(struct xgbe_prv_data *pdata);
>>> +static void xgbe_phy_set_mode(struct xgbe_prv_data *pdata, enum 
>>> xgbe_mode mode);
>>> +static void xgbe_phy_kr_mode(struct xgbe_prv_data *pdata);
>>> +static void xgbe_phy_sfi_mode(struct xgbe_prv_data *pdata);
>>
>> Why the forward declarations? It's against the kernel coding style.
> 
> Hi Jakub,
> 
> Sure, I've re-ordered most of the functions correctly to avoid forward 
> declarations. However, there is circular dependency on couple of them. 
> Let us know if its fine to have forward declaration for the functions 
> that have circular-dependency.

Hi Jakub,

Gentle reminder!

Let us know if it is okay to have forward declarations for functions 
that have circular dependency.

Thanks,
Raju

> 
>>
>>>   static int xgbe_phy_i2c_xfer(struct xgbe_prv_data *pdata,
>>>                    struct xgbe_i2c_op *i2c_op)
>>> @@ -2038,6 +2044,87 @@ static void xgbe_phy_set_redrv_mode(struct 
>>> xgbe_prv_data *pdata)
>>>       xgbe_phy_put_comm_ownership(pdata);
>>>   }
>>> +#define MAX_RX_ADAPT_RETRIES    1
>>> +#define XGBE_PMA_RX_VAL_SIG_MASK    (XGBE_PMA_RX_SIG_DET_0_MASK | 
>>> XGBE_PMA_RX_VALID_0_MASK)
>>> +
>>> +static inline void xgbe_set_rx_adap_mode(struct xgbe_prv_data 
>>> *pdata, enum xgbe_mode mode)
>>
>> Don't pointlessly use inline, please. The compiler will know when
>> to inline, and this is not the datapath.
> 
> Sure, I'll take care of this in the next revision.