Re: [PATCH v1] ptp: add ToD device driver for Intel FPGA cards

On Mon, 20 Mar 2023, Andy Shevchenko wrote:

On Wed, Mar 15, 2023 at 10:37:58AM -0400, Nicolas Pitre wrote:

quoted

On Wed, 15 Mar 2023, Andy Shevchenko wrote:

quoted

On Tue, Mar 14, 2023 at 12:46:48PM -0700, Richard Cochran wrote:

quoted

On Tue, Mar 14, 2023 at 12:47:03PM +0200, Andy Shevchenko wrote:

quoted

The semantics of the above is similar to gpiod_get_optional() and since NULL
is a valid return in such cases, the PTP has to handle this transparently to
the user. Otherwise it's badly designed API which has to be fixed.

Does it now?  Whatever.

quoted

TL;DR: If I'm mistaken, I would like to know why.

git log.  git blame.

Get to know the tools of trade.

So, the culprit seems the commit d1cbfd771ce8 ("ptp_clock: Allow for it
to be optional") which did it half way.

Now I would like to know why the good idea got bad implementation.

Nicolas?

I'd be happy to help but as presented I simply don't know what you're 
talking about. Please give me more context.

When your change introduced the optionality of the above mentioned API,
i.e. ptp_clock_register(), the function started returning NULL, which
is fine. What's not in my opinion is to ask individual drivers to handle it.
That said, if we take a look at gpiod_*_optional() or clk_*_optional()
we may notice that they handle NULL as a valid parameter (object) to their
respective APIs and individual drivers shouldn't take care about that.

Why PTP is so special?

To my knowledge it is not.

The current arrangement has apparently worked well for more than 6 
years. If you see a better way you're welcome to submit patches as 
usual.

Alternatively the above commit can be reverted if no one else 
cares. I personally gave up on the idea of a slimmed down Linux kernel a 
while ago.


Nicolas