diff mbox series

[J/OEM-5.17/U,1/1] UBUNTU: SAUCE: igc: wait for the MAC copy when enabled MAC passthrough

Message ID 20220803075553.884839-2-aaron.ma@canonical.com
State New
Headers show
Series Fix invalid MAC address after hotplug tbt dock | expand

Commit Message

Aaron Ma Aug. 3, 2022, 7:55 a.m. UTC
BugLink: https://bugs.launchpad.net/bugs/1942999

Such as dock hot plug event when runtime, for hardware implementation,
the MAC copy takes less than one second when BIOS enabled MAC passthrough.
After test on Lenovo TBT4 dock, 600ms is enough to update the
MAC address.
Otherwise ethernet fails to work.

Link: https://lore.kernel.org/lkml/20210702045120.22855-2-aaron.ma@canonical.com/
Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Andrea Righi Aug. 4, 2022, 6:37 a.m. UTC | #1
On Wed, Aug 03, 2022 at 03:55:53PM +0800, Aaron Ma wrote:
> BugLink: https://bugs.launchpad.net/bugs/1942999
> 
> Such as dock hot plug event when runtime, for hardware implementation,
> the MAC copy takes less than one second when BIOS enabled MAC passthrough.
> After test on Lenovo TBT4 dock, 600ms is enough to update the
> MAC address.
> Otherwise ethernet fails to work.
> 
> Link: https://lore.kernel.org/lkml/20210702045120.22855-2-aaron.ma@canonical.com/
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
>  drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index f99819fc559d9..634cfdc203203 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -6323,6 +6323,9 @@ static int igc_probe(struct pci_dev *pdev,
>  	memcpy(&hw->mac.ops, ei->mac_ops, sizeof(hw->mac.ops));
>  	memcpy(&hw->phy.ops, ei->phy_ops, sizeof(hw->phy.ops));
>  
> +	if (pci_is_thunderbolt_attached(pdev))
> +		msleep(600);
> +

Unconditional sleeps are always unpredictable and a bit scary... isn't
there a more reliable way to detect when the MAC address is updated?
Like an interrupt, reading back from a register or somthing similar?

Thanks,
-Andrea

>  	/* Initialize skew-specific constants */
>  	err = ei->get_invariants(hw);
>  	if (err)
> -- 
> 2.37.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Aaron Ma Aug. 4, 2022, 7:04 a.m. UTC | #2
On 8/4/22 14:37, Andrea Righi wrote:
> On Wed, Aug 03, 2022 at 03:55:53PM +0800, Aaron Ma wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1942999
>>
>> Such as dock hot plug event when runtime, for hardware implementation,
>> the MAC copy takes less than one second when BIOS enabled MAC passthrough.
>> After test on Lenovo TBT4 dock, 600ms is enough to update the
>> MAC address.
>> Otherwise ethernet fails to work.
>>
>> Link: https://lore.kernel.org/lkml/20210702045120.22855-2-aaron.ma@canonical.com/
>> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
>> ---
>>   drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
>> index f99819fc559d9..634cfdc203203 100644
>> --- a/drivers/net/ethernet/intel/igc/igc_main.c
>> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
>> @@ -6323,6 +6323,9 @@ static int igc_probe(struct pci_dev *pdev,
>>   	memcpy(&hw->mac.ops, ei->mac_ops, sizeof(hw->mac.ops));
>>   	memcpy(&hw->phy.ops, ei->phy_ops, sizeof(hw->phy.ops));
>>   
>> +	if (pci_is_thunderbolt_attached(pdev))
>> +		msleep(600);
>> +
> Unconditional sleeps are always unpredictable and a bit scary... isn't
> there a more reliable way to detect when the MAC address is updated?
> Like an interrupt, reading back from a register or somthing similar?

That's the ideal way, but I discussed with Intel, there is no such interrupts.
In the datasheet, the I225 Windows driver implementation is to wait for 1 second.

Regards,
Aaron

> Thanks,
> -Andrea
>
>>   	/* Initialize skew-specific constants */
>>   	err = ei->get_invariants(hw);
>>   	if (err)
>> -- 
>> 2.37.1
>>
>>
>> -- 
>> kernel-team mailing list
>> kernel-team@lists.ubuntu.com
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Stefan Bader Aug. 5, 2022, 2:53 p.m. UTC | #3
On 03.08.22 09:55, Aaron Ma wrote:
> BugLink: https://bugs.launchpad.net/bugs/1942999
> 
> Such as dock hot plug event when runtime, for hardware implementation,
> the MAC copy takes less than one second when BIOS enabled MAC passthrough.
> After test on Lenovo TBT4 dock, 600ms is enough to update the
> MAC address.
> Otherwise ethernet fails to work.
> 
> Link: https://lore.kernel.org/lkml/20210702045120.22855-2-aaron.ma@canonical.com/
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---

Applied to jammy:linux/master-next. Thanks.

-Stefan

>   drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index f99819fc559d9..634cfdc203203 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -6323,6 +6323,9 @@ static int igc_probe(struct pci_dev *pdev,
>   	memcpy(&hw->mac.ops, ei->mac_ops, sizeof(hw->mac.ops));
>   	memcpy(&hw->phy.ops, ei->phy_ops, sizeof(hw->phy.ops));
>   
> +	if (pci_is_thunderbolt_attached(pdev))
> +		msleep(600);
> +
>   	/* Initialize skew-specific constants */
>   	err = ei->get_invariants(hw);
>   	if (err)
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index f99819fc559d9..634cfdc203203 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -6323,6 +6323,9 @@  static int igc_probe(struct pci_dev *pdev,
 	memcpy(&hw->mac.ops, ei->mac_ops, sizeof(hw->mac.ops));
 	memcpy(&hw->phy.ops, ei->phy_ops, sizeof(hw->phy.ops));
 
+	if (pci_is_thunderbolt_attached(pdev))
+		msleep(600);
+
 	/* Initialize skew-specific constants */
 	err = ei->get_invariants(hw);
 	if (err)