On Wed, Jul 18, 2018 at 08:04:08PM +0000, [email protected] wrote:
> Thank you for your detailed feedback. I'm working on it now, but I feel it
> will take a little extra time to complete. Therefor I'm planning to remove
> PTP support from this patch series, and resubmit it in a new patch later.
Ok.
> > > + if (cleanup) {
> > > + lan743x_ptp_unrequest_tx_timestamp(tx->adapter);
> > > + dev_kfree_skb(buffer_info->skb);
> > > + } else {
> > > + lan743x_ptp_tx_timestamp_skb(tx->adapter,
> > > + buffer_info->skb,
> > > + (buffer_info->flags &
> > > +
> > TX_BUFFER_INFO_FLAG_IGNORE_SYNC)
> > > + != 0);
> >
> > This is poor coding style. Please find a better way.
>
> Can you clarify what is poor and what would be better?
> For example, should I change "X != 0" to "X ? true : false".
Look at this:
lan743x_ptp_tx_timestamp_skb(tx->adapter,
buffer_info->skb,
(buffer_info->flags &
TX_BUFFER_INFO_FLAG_IGNORE_SYNC)
!= 0);
Can't you reduce
(buffer_info->flags & TX_BUFFER_INFO_FLAG_IGNORE_SYNC) != 0
into a local variable:
lan743x_ptp_tx_timestamp_skb(tx->adapter, buffer_info->skb,
xyz);
?
> So you mean PPS is not intended to generate a physical signal?
Yes.
> It is only intended to call ptp_clock_event?
Yes.
> I can configure the hardware to generate an interrupt each second and then
> call
> ptp_clock_event. Would that satisfy the pps requirements?
Yes.
> Regarding PTP_CLK_REQ_PEROUT. Is that intended for physical signals?
Yes.
Thanks,
Richard
