Messages in this thread |  | | From | Sergey Organov <> | Subject | Re: [PATCH v2 net] net: fec: fix hardware time stamping by external devices | Date | Tue, 14 Jul 2020 15:39:04 +0300 |
| |
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
|  |