Messages in this thread | | | Date | Thu, 30 Jan 2020 07:01:53 -0800 | From | Richard Cochran <> | Subject | Re: [PATCH 2/2] ptp: Add a ptp clock driver for IDT 82P33 SMU. |
| |
The net-next tree is closed for new submissions until after the mainline merge window closes. So you will have to re-submit this patch then.
This patch and the first one should have [net-next] in the subject line. Please address both messages (1/2 and 2/2) to the netdev list, with the others on CC.
But for, here are a few coding style issues...
On Wed, Jan 29, 2020 at 02:37:18PM -0500, min.li.xe@renesas.com wrote: > @@ -12,4 +12,5 @@ obj-$(CONFIG_PTP_1588_CLOCK_KVM) += ptp_kvm.o > obj-$(CONFIG_PTP_1588_CLOCK_QORIQ) += ptp-qoriq.o > ptp-qoriq-y += ptp_qoriq.o > ptp-qoriq-$(CONFIG_DEBUG_FS) += ptp_qoriq_debugfs.o > -obj-$(CONFIG_PTP_1588_CLOCK_IDTCM) += ptp_clockmatrix.o > \ No newline at end of file
Please fix white space here
> +obj-$(CONFIG_PTP_1588_CLOCK_IDTCM) += ptp_clockmatrix.o > +obj-$(CONFIG_PTP_1588_CLOCK_IDT82P33) += ptp_idt82p33.o > \ No newline at end of file
and here.
> +static void _idt82p33_caps_init(struct ptp_clock_info *caps) > +{
No need for leading _ on private methods.
> + (caps)->owner = THIS_MODULE; > + (caps)->max_adj = 92000; > + (caps)->adjfreq = idt82p33_adjfreq; > + (caps)->adjtime = idt82p33_adjtime; > + (caps)->gettime64 = idt82p33_gettime; > + (caps)->settime64 = idt82p33_settime; > + (caps)->enable = idt82p33_enable; > +} > + > +static int _mask_bit_count(int mask) > +{
There is a GCC built in for this.
> + int ret = 0; > + > + while (mask != 0) { > + mask &= (mask-1); > + ret++; > + } > + > + return ret; > +} > + > +static void _byte_array_to_timespec(struct timespec64 *ts, > + u8 buf[TOD_BYTE_COUNT]) > +{
Prefix all your functions with idt82p33_ please.
> + u8 i; > + s32 nsec; > + time64_t sec; > + > + nsec = buf[3]; > + for (i = 0; i < 3; i++) { > + nsec <<= 8; > + nsec |= buf[2 - i]; > + } > + > + sec = buf[9]; > + for (i = 0; i < 5; i++) { > + sec <<= 8; > + sec |= buf[8 - i]; > + } > + > + ts->tv_sec = sec; > + ts->tv_nsec = nsec; > +}
Thanks, Richard
| |