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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Sent: Tuesday, March 21, 2023 10:40 PM
> To: Zhang, Tianfei <tianfei.zhang@xxxxxxxxx>
> Cc: Nicolas Pitre <nico@xxxxxxxxxxx>; Richard Cochran
> <richardcochran@xxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux-
> fpga@xxxxxxxxxxxxxxx; ilpo.jarvinen@xxxxxxxxxxxxxxx; Weight, Russell H
> <russell.h.weight@xxxxxxxxx>; matthew.gerlach@xxxxxxxxxxxxxxx; pierre-
> louis.bossart@xxxxxxxxxxxxxxx; Gomes, Vinicius <vinicius.gomes@xxxxxxxxx>;
> Khadatare, RaghavendraX Anand <raghavendrax.anand.khadatare@xxxxxxxxx>
> Subject: Re: [PATCH v1] ptp: add ToD device driver for Intel FPGA cards
> 
> On Tue, Mar 21, 2023 at 02:28:15PM +0000, Zhang, Tianfei wrote:
> > > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > > Sent: Tuesday, March 21, 2023 9:03 PM
> > > To: Nicolas Pitre <nico@xxxxxxxxxxx> On Mon, Mar 20, 2023 at
> > > 04:53:07PM -0400, Nicolas Pitre wrote:
> > > > On Mon, 20 Mar 2023, Richard Cochran wrote:
> > > > > On Mon, Mar 20, 2023 at 09:43:30AM -0400, Nicolas Pitre wrote:
> > > > >
> > > > > > 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.
> > > > >
> > > > > Does this mean I can restore the posix clocks back into the core
> > > > > unconditionally?
> > > >
> > > > This only means _I_ no longer care. I'm not speaking for others (e.g.
> > > > OpenWRT or the like) who might still rely on splitting it out.
> > > > Maybe Andy wants to "fix" it?
> > >
> > > I would be a good choice, if I have an access to the hardware at hand to test.
> > > That said, I think Richard himself can try to finish that feature
> > > (optional PTP) and on my side I can help with reviewing the code (just Cc me
> when needed).
> >
> > Handle NULL as a valid parameter (object) to their respective APIs
> > looks a good idea, but this will be a big change and need fully test
> > for all devices,
> 
> Since it's core change, so a few devices that cover 100% APIs that should handle
> optional PTP. I don't think it requires enormous work.
> 
> > we can add it on the TODO list.
> 
> It would be a good idea.
> 
> > So for this patch, are you agree we continue use the existing
> > ptp_clock_register() API, for example, change my driver like below:
> 
> The problem is that it will increase the technical debt.
> What about sending with NULL handled variant together with an RFC/RFT that
> finishes the optional PTP support?

I think sending the other patchset to fix this NULL handled issue of PTP core will be better?

> 
> >       dt->ptp_clock = ptp_clock_register(&dt->ptp_clock_ops, dev);
> 
> 	ret = PTR_ERR_OR_ZERO(...);
> 	if (ret)
> 		return ...
> 
> ?
> 
> >       if (IS_ERR_OR_NULL(dt->ptp_clock))
> >               return dev_err_probe(dt->dev, IS_ERR_OR_NULL(dt->ptp_clock),
> >                                    "Unable to register PTP clock\n");


Would you like below:

        dt->ptp_clock = ptp_clock_register(&dt->ptp_clock_ops, dev);
       ret = PTR_ERR_OR_ZERO(dt->ptp_clock);
        return dev_err_probe(dt->dev, ret, "Unable to register PTP clock\n");





[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux