netdev - Re: [PATCH v3 net-next 2/3] amd-xgbe: add support for Crater ethernet device

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: <f23d6b09-0682-4f10-a5a7-1e2c41801429@amd.com>
Date: Wed, 13 Dec 2023 10:42:07 -0600
From: Tom Lendacky <thomas.lendacky@....com>
To: Raju Rangoju <Raju.Rangoju@....com>, netdev@...r.kernel.org
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, Shyam-sundar.S-k@....com,
 Sudheesh Mavila <sudheesh.mavila@....com>
Subject: Re: [PATCH v3 net-next 2/3] amd-xgbe: add support for Crater ethernet
 device

On 12/13/23 10:25, Raju Rangoju wrote:
> On 12/12/2023 9:02 PM, Tom Lendacky wrote:
>> On 12/11/23 23:37, Raju Rangoju wrote:
>>> Add the necessary support to enable Crater ethernet device. Since the
>>> BAR1 address cannot be used to access the XPCS registers on Crater, use
>>> the pci_{read/write}_config_dword calls. Also, include the new pci device
>>> id 0x1641 to register Crater device with PCIe.
>>>
>>> Co-developed-by: Sudheesh Mavila <sudheesh.mavila@....com>
>>> Signed-off-by: Sudheesh Mavila <sudheesh.mavila@....com>
>>> Signed-off-by: Raju Rangoju <Raju.Rangoju@....com>
>>> ---
>>>   drivers/net/ethernet/amd/xgbe/xgbe-common.h |  5 ++
>>>   drivers/net/ethernet/amd/xgbe/xgbe-dev.c    | 93 +++++++++++++++++++++
>>>   drivers/net/ethernet/amd/xgbe/xgbe-pci.c    | 35 +++++++-
>>>   drivers/net/ethernet/amd/xgbe/xgbe.h        |  6 ++
>>>   4 files changed, 137 insertions(+), 2 deletions(-)
>>>

>>> +
>>> +    spin_lock_irqsave(&pdata->xpcs_lock, flags);
>>> +    pci_write_config_dword(rdev, 0x60, (pdata->xphy_base + 
>>> pdata->xpcs_window_sel_reg));
>>> +    pci_write_config_dword(rdev, 0x64, index);
>>> +    pci_write_config_dword(rdev, 0x60, pdata->xphy_base + offset);
>>> +    pci_read_config_dword(rdev, 0x64, &ctr_mmd_data);
>>> +    if (offset % 4) {
>>> +        ctr_mmd_data = FIELD_PREP(XGBE_GEN_HI_MASK, mmd_data) |
>>> +                   FIELD_GET(XGBE_GEN_LO_MASK, ctr_mmd_data);
>>> +    } else {
>>> +        ctr_mmd_data = FIELD_PREP(XGBE_GEN_HI_MASK,
>>> +                      FIELD_GET(XGBE_GEN_HI_MASK, ctr_mmd_data)) |
>>> +                   FIELD_GET(XGBE_GEN_LO_MASK, mmd_data);
>>> +    }
>>
>> Braces aren't necessary.
>>
>> Also, just curious, what is the "ctr" prefix meant to imply here?
> 
> ctr = crater. Will use the full name instead to avoid confusion.

You should probably make it generic and use pci_ or tmp_ (or smn_ if you 
end up squashing patches 2 and 3), since this may not be "crater" specific 
if this is used by another device ID in the future.

> 
>>
>>

>>> @@ -295,15 +295,28 @@ static int xgbe_pci_probe(struct pci_dev *pdev, 
>>> const struct pci_device_id *id)
>>>           /* Yellow Carp devices do not need rrc */
>>>           pdata->vdata->enable_rrc = 0;
>>>           break;
>>> +    case 0x1630:
>>
>> What PCI ID is this, it doesn't match the 0x1641 added below?
> 
> 0x1630 is root hub

Ah, right, sorry for the noise.

Thanks,
Tom

>