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 for Android: free password hash cracker in your pocket

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

Message-ID: <f4a2cd5d-7d36-6f4d-6012-7e4e87d44d7c@amd.com>
Date:   Fri, 27 Jan 2023 13:13:21 +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/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.

> 
>>   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.