RE: [PATCH v4 net-next 04/15] ptp: netc: add NETC V4 Timer PTP driver support

[Wei Fang]

> > ---
> what's change in v4?

Sorry, I should put the latest changes first.

> 
> > v2 changes:
> > 1. Rename netc_timer_get_source_clk() to
> >    netc_timer_get_reference_clk_source() and refactor it 2. Remove the
> > scaled_ppm check in netc_timer_adjfine() 3. Add a comment in
> > netc_timer_cur_time_read() 4. Add linux/bitfield.h to fix the build
> > errors
> > v3 changes:
> > 1. Refactor netc_timer_adjtime() and remove netc_timer_cnt_read() 2.
> > Remove the check of dma_set_mask_and_coherent() 3. Use devm_kzalloc()
> > and pci_ioremap_bar() 4. Move alarm related logic including irq
> > handler to the next patch 5. Improve the commit message 6. Refactor
> > netc_timer_get_reference_clk_source() and remove
> >    clk_prepare_enable()
> > 7. Use FIELD_PREP() helper
> > 8. Rename PTP_1588_CLOCK_NETC to PTP_NETC_V4_TIMER and improve the
> >    help text.
> > 9. Refine netc_timer_adjtime(), change tmr_off to s64 type as we
> >    confirmed TMR_OFF is a signed register.
> > v4 changes:
> > 1. Remove NETC_TMR_PCI_DEVID
> > 2. Fix build warning: "NSEC_PER_SEC << 32" --> "(u64)NSEC_PER_SEC << 32"
> > 3. Remove netc_timer_get_phc_index()
> > 4. Remove phc_index from struct netc_timer 5. Change PTP_NETC_V4_TIMER
> > from bool to tristate 6. Move devm_kzalloc() at the begining of
> > netc_timer_pci_probe() 7. Remove the err log when
> > netc_timer_parse_dt() returns error, instead,
> >    add the err log to netc_timer_get_reference_clk_source()
> > ---

> > +static void netc_timer_cnt_write(struct netc_timer *priv, u64 ns) {
> > +	u32 tmr_cnt_h = upper_32_bits(ns);
> > +	u32 tmr_cnt_l = lower_32_bits(ns);
> > +
> > +	/* Writes to the TMR_CNT_L register copies the written value
> > +	 * into the shadow TMR_CNT_L register. Writes to the TMR_CNT_H
> > +	 * register copies the values written into the shadow TMR_CNT_H
> > +	 * register. Contents of the shadow registers are copied into
> > +	 * the TMR_CNT_L and TMR_CNT_H registers following a write into
> > +	 * the TMR_CNT_H register. So the user must writes to TMR_CNT_L
> > +	 * register first.
> 
> You have not answer my question, does other _L _H have the same behavior, like
> below OFF_L and OFF_H?
> 

Sorry, I missed the question in v3.
The answer should be 'yes', TMR_ALARM_L/H also has the same behavior.
Regarding to TMROFF_L/H, this behavior is not explicitly stated in the block
guide, but the behavior of registers of the same type should be consistent
within the same IP. I looked at the driver of the QorIQ platform, which uses
the same IP (but different version), it also writes TMROFF_L first, and then
writes TMROFF_H.

I will add this to comment, thanks.

> > +	 */
> > +	netc_timer_wr(priv, NETC_TMR_CNT_L, tmr_cnt_l);
> > +	netc_timer_wr(priv, NETC_TMR_CNT_H, tmr_cnt_h); }
> > +
> > +static u64 netc_timer_offset_read(struct netc_timer *priv) {
> > +	u32 tmr_off_l, tmr_off_h;
> > +	u64 offset;
> > +
> > +	tmr_off_l = netc_timer_rd(priv, NETC_TMR_OFF_L);
> > +	tmr_off_h = netc_timer_rd(priv, NETC_TMR_OFF_H);
> > +	offset = (((u64)tmr_off_h) << 32) | tmr_off_l;
> > +
> > +	return offset;
> > +}
> > +
> > +static void netc_timer_offset_write(struct netc_timer *priv, u64
> > +offset) {
> > +	u32 tmr_off_h = upper_32_bits(offset);
> > +	u32 tmr_off_l = lower_32_bits(offset);
> > +
> > +	netc_timer_wr(priv, NETC_TMR_OFF_L, tmr_off_l);
> > +	netc_timer_wr(priv, NETC_TMR_OFF_H, tmr_off_h); }
> > +
> > +static u64 netc_timer_cur_time_read(struct netc_timer *priv) {
> > +	u32 time_h, time_l;
> > +	u64 ns;
> > +
> > +	/* The user should read NETC_TMR_CUR_TIME_L first to
> > +	 * get correct current time.
> > +	 */
> > +	time_l = netc_timer_rd(priv, NETC_TMR_CUR_TIME_L);
> > +	time_h = netc_timer_rd(priv, NETC_TMR_CUR_TIME_H);
> > +	ns = (u64)time_h << 32 | time_l;
> > +
> > +	return ns;
> > +}