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: <9f029694-7645-404b-8cd8-00837df64669@amd.com>
Date: Mon, 14 Apr 2025 17:49:58 +0530
From: "Rangoju, Raju" <raju.rangoju@....com>
To: Larysa Zaremba <larysa.zaremba@...el.com>
Cc: andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, Shyam-sundar.S-k@....com
Subject: Re: [PATCH net-next 1/5] amd-xgbe: reorganize the code of XPCS access



On 4/11/2025 2:03 PM, Larysa Zaremba wrote:
> On Tue, Apr 08, 2025 at 11:49:57PM +0530, Raju Rangoju wrote:
>> The xgbe_{read/write}_mmd_regs_v* functions have common code which can
>> be moved to helper functions. Add new helper functions to calculate the
>> mmd_address for v1/v2 of xpcs access.
>>
> 
> Overall seems reasonable, but the new functions are missing the xgbe_ prefix,
> contrary to other in this file.

Thank you for your observation. We have additional patches in 
development that follow this path, and I'll take care of this in the 
future patches that follow.

> 
>> Signed-off-by: Raju Rangoju <Raju.Rangoju@....com>
>> ---
>>   drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 63 ++++++++++--------------
>>   1 file changed, 27 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
>> index b51a3666dddb..ae82dc3ac460 100644
>> --- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
>> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
>> @@ -1041,18 +1041,17 @@ static int xgbe_set_gpio(struct xgbe_prv_data *pdata, unsigned int gpio)
>>   	return 0;
>>   }
>>   
>> -static int xgbe_read_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
>> -				 int mmd_reg)
>> +static unsigned int get_mmd_address(struct xgbe_prv_data *pdata, int mmd_reg)
>>   {
>> -	unsigned long flags;
>> -	unsigned int mmd_address, index, offset;
>> -	int mmd_data;
>> -
>> -	if (mmd_reg & XGBE_ADDR_C45)
>> -		mmd_address = mmd_reg & ~XGBE_ADDR_C45;
>> -	else
>> -		mmd_address = (pdata->mdio_mmd << 16) | (mmd_reg & 0xffff);
>> +	return (mmd_reg & XGBE_ADDR_C45) ?
>> +		mmd_reg & ~XGBE_ADDR_C45 :
>> +		(pdata->mdio_mmd << 16) | (mmd_reg & 0xffff);
>> +}
>>   
>> +static void get_pcs_index_and_offset(struct xgbe_prv_data *pdata,
>> +				     unsigned int mmd_address,
>> +				     unsigned int *index, unsigned int *offset)
>> +{
>>   	/* The PCS registers are accessed using mmio. The underlying
>>   	 * management interface uses indirect addressing to access the MMD
>>   	 * register sets. This requires accessing of the PCS register in two
>> @@ -1063,8 +1062,20 @@ static int xgbe_read_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
>>   	 * offset 1 bit and reading 16 bits of data.
>>   	 */
>>   	mmd_address <<= 1;
>> -	index = mmd_address & ~pdata->xpcs_window_mask;
>> -	offset = pdata->xpcs_window + (mmd_address & pdata->xpcs_window_mask);
>> +	*index = mmd_address & ~pdata->xpcs_window_mask;
>> +	*offset = pdata->xpcs_window + (mmd_address & pdata->xpcs_window_mask);
>> +}
>> +
>> +static int xgbe_read_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
>> +				 int mmd_reg)
>> +{
>> +	unsigned long flags;
>> +	unsigned int mmd_address, index, offset;
>> +	int mmd_data;
>> +
>> +	mmd_address = get_mmd_address(pdata, mmd_reg);
>> +
>> +	get_pcs_index_and_offset(pdata, mmd_address, &index, &offset);
>>   
>>   	spin_lock_irqsave(&pdata->xpcs_lock, flags);
>>   	XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
>> @@ -1080,23 +1091,9 @@ static void xgbe_write_mmd_regs_v2(struct xgbe_prv_data *pdata, int prtad,
>>   	unsigned long flags;
>>   	unsigned int mmd_address, index, offset;
>>   
>> -	if (mmd_reg & XGBE_ADDR_C45)
>> -		mmd_address = mmd_reg & ~XGBE_ADDR_C45;
>> -	else
>> -		mmd_address = (pdata->mdio_mmd << 16) | (mmd_reg & 0xffff);
>> +	mmd_address = get_mmd_address(pdata, mmd_reg);
>>   
>> -	/* The PCS registers are accessed using mmio. The underlying
>> -	 * management interface uses indirect addressing to access the MMD
>> -	 * register sets. This requires accessing of the PCS register in two
>> -	 * phases, an address phase and a data phase.
>> -	 *
>> -	 * The mmio interface is based on 16-bit offsets and values. All
>> -	 * register offsets must therefore be adjusted by left shifting the
>> -	 * offset 1 bit and writing 16 bits of data.
>> -	 */
>> -	mmd_address <<= 1;
>> -	index = mmd_address & ~pdata->xpcs_window_mask;
>> -	offset = pdata->xpcs_window + (mmd_address & pdata->xpcs_window_mask);
>> +	get_pcs_index_and_offset(pdata, mmd_address, &index, &offset);
>>   
>>   	spin_lock_irqsave(&pdata->xpcs_lock, flags);
>>   	XPCS32_IOWRITE(pdata, pdata->xpcs_window_sel_reg, index);
>> @@ -1111,10 +1108,7 @@ static int xgbe_read_mmd_regs_v1(struct xgbe_prv_data *pdata, int prtad,
>>   	unsigned int mmd_address;
>>   	int mmd_data;
>>   
>> -	if (mmd_reg & XGBE_ADDR_C45)
>> -		mmd_address = mmd_reg & ~XGBE_ADDR_C45;
>> -	else
>> -		mmd_address = (pdata->mdio_mmd << 16) | (mmd_reg & 0xffff);
>> +	mmd_address = get_mmd_address(pdata, mmd_reg);
>>   
>>   	/* The PCS registers are accessed using mmio. The underlying APB3
>>   	 * management interface uses indirect addressing to access the MMD
>> @@ -1139,10 +1133,7 @@ static void xgbe_write_mmd_regs_v1(struct xgbe_prv_data *pdata, int prtad,
>>   	unsigned int mmd_address;
>>   	unsigned long flags;
>>   
>> -	if (mmd_reg & XGBE_ADDR_C45)
>> -		mmd_address = mmd_reg & ~XGBE_ADDR_C45;
>> -	else
>> -		mmd_address = (pdata->mdio_mmd << 16) | (mmd_reg & 0xffff);
>> +	mmd_address = get_mmd_address(pdata, mmd_reg);
>>   
>>   	/* The PCS registers are accessed using mmio. The underlying APB3
>>   	 * management interface uses indirect addressing to access the MMD
>> -- 
>> 2.34.1
>>
>>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ