lkml.org 
[lkml]   [2020]   [Jul]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2 net] net: fec: fix hardware time stamping by external devices
Date
Vladimir Oltean <olteanv@gmail.com> writes:

> On Mon, Jul 13, 2020 at 01:32:09AM +0300, Sergey Organov wrote:

[...]

>> > From the perspective of the mainline kernel, that can never happen.
>>
>> Yet in happened to me, and in some way because of the UAPI deficiencies
>> I've mentioned, as ethtool has entirely separate code path, that happens
>> to be correct for a long time already.
>>
>
> Yup, you are right:
>

[...]

> Very bad design choice indeed...
> Given the fact that the PHY timestamping needs massaging from MAC driver
> for plenty of other reasons, now of all things, ethtool just decided
> it's not going to consult the MAC driver about the PHC it intends to
> expose to user space, and just say "here's the PHY, deal with it". This
> is a structural bug, I would say.
>
>> > From your perspective as a developer, in your private work tree, where
>> > _you_ added the necessary wiring for PHY timestamping, I fully
>> > understand that this is exactly what happened _to_you_.
>> > I am not saying that PHY timestamping doesn't need this issue fixed. It
>> > does, and if it weren't for DSA, it would have simply been a "new
>> > feature", and it would have been ok to have everything in the same
>> > patch.
>>
>> Except that it's not a "new feature", but a bug-fix of an existing one,
>> as I see it.
>>
>
> See above. It's clear that the intention of the PHY timestamping support
> is for MAC drivers to opt-in, otherwise some mechanism would have been
> devised such that not every single one of them would need to check for
> phy_has_hwtstamp() in .ndo_do_ioctl(). That simply doesn't scale. Also,
> it seems that automatically calling phy_ts_info from
> __ethtool_get_ts_info is not coherent with that intention.
>
> I need to think more about this. Anyway, if your aim is to "reduce
> confusion" for others walking in your foot steps, I think this is much
> worthier of your time: avoiding the inconsistent situation where the MAC
> driver is obviously not ready for PHY timestamping, however not all
> parts of the kernel are in agreement with that, and tell the user
> something else.

You see, I have a problem on kernel 4.9.146. After I apply this patch,
the problem goes away, at least for FEC/PHY combo that I care about, and
chances are high that for DSA as well, according to your own expertise.
Why should I care what is or is not ready for what to get a bug-fix
patch into the kernel? Why should I guess some vague "intentions" or
spend my time elsewhere?

Also please notice that if, as you suggest, I will propose only half of
the patch that will fix DSA only, then I will create confusion for
FEC/PHY users that will have no way to figure they need another part of
the fix to get their setup to work.

Could we please finally agree that, as what I suggest is indeed a simple
bug-fix, we could safely let it into the kernel?

Thanks,
-- Sergey

\
 
 \ /
  Last update: 2020-07-14 14:40    [W:0.087 / U:0.324 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site