Re: [Intel-wired-lan] [PATCH iwl v1 0/5] igc: TX timestamping fixes

[Vinicius Costa Gomes <vinicius.gomes@intel.com>]

Hi Tony,

Tony Nguyen <anthony.l.nguyen@intel.com> writes:

> On 5/8/2023 3:18 PM, Vinicius Costa Gomes wrote:
>> Tony Nguyen <anthony.l.nguyen@intel.com> writes:
>> 
>>> On 5/4/2023 4:52 PM, Vinicius Costa Gomes wrote:
>>>> Hi,
>>>>
>>>> Changes from the "for-next-queue" version:
>>>>    - As this is intended for the iwl/net-queue tree, removed adding
>>>>      support for adding the "extra" tstamp registers;
>>>>    - Added "Fixes:" tags to the appropriate patches (Vladimir Oltean);
>>>
>>> In most cases, net patches should have Fixes: tags to them. Patches 3
>>> and 5 don't have them and it seems like it would be applicable to them.
>>>
>> 
>> Patch 3 is directly related to patch 1, but I think it deserved a
>> separate commit, as it has a bit of refactor. I can squash it into patch
>> 1, if you think it's better I can do that, no worries, I was only afraid
>> to make the patch harder to follow.
>
> I understand the reasoning and makes sense, however, I want to say I 
> recently read on netdev a comment for keeping it in one patch for ease 
> of backport.
>

Makes sense. Will squash it.

>> Patch 5, as a hardware issue workaround, I didn't know if adding a
>> 'Fixes:' tag made sense, but as a way to direct patches to the right
>> stable trees, that would be a good point in favor, even if it's not
>> fixing a bug in the code. Is this what you had in mind? If so, I can do
>> that.
>
> Yea, I think a hint on how far back to backport would be valuable. I 
> believe even though it's a workaround, from user perspective, it would 
> appear as a bug(?)
>

Will add the 'Fixes:' tag.

>>> Patch 4 seems more like an improvement than a bug fix? If so, -next
>>> would seem a better path for that patch. Based on the 'for-next-queue
>>> version' link, there are still some patches remaining that will go
>>> through -next? Perhaps this can go with them.
>>>
>> 
>> On a very loaded system, for example, time synchronization can fail if
>> something blocks the system workqueue from running, so in a sense, that
>> patches fixes/helps some user visible issues. But I can see it both
>> ways, that this is an improvement. What's your preference?
>
> I think I'd rather err on the side of fixing and it's already here :)
>

Understood. Will keep proposing it here for 'iwl-net'.

Will send the v2 soon.


Thank you,
-- 
Vinicius
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan