netdev - Re: [net-next.git 2/9] stmmac: add IEEE 1588-2002 PTP support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 10 Mar 2013 13:25:54 +0100
From:	Richard Cochran <richardcochran@...il.com>
To:	Giuseppe CAVALLARO <peppe.cavallaro@...com>
Cc:	netdev@...r.kernel.org, bh74.an@...sung.com,
	Rayagond K <rayagond@...avyalabs.com>
Subject: Re: [net-next.git 2/9] stmmac: add IEEE 1588-2002 PTP support

On Fri, Mar 08, 2013 at 08:02:21AM +0100, Giuseppe CAVALLARO wrote:
> On 3/8/2013 7:34 AM, Richard Cochran wrote:

> >>+
> >>+static int enh_desc_get_rx_timestamp_status(struct dma_desc *p)
> >>+{
> >>+	/* FIXME if Enhance descriptor with 8 DWORDS is enabled */
> >
> >Why not fix these FIXMEs for the next respin?
> 
> This is fixed in the patch #5 where we use the extended descriptors
> for PTP2.

If possible, it is nice for the reviewers and for the logic of the
patch series to order the changes so that these FIXMEs go away.
 
> >>+	struct hwtstamp_config config;
> >>+	struct timespec now;
> >>+	u64 temp = 0;
> >
> >You add this new code here, but you change it all around again a few
> >patches later. Please just submit the final, combined version.
> 
> we kept these separately because the patch #5 (for example) depends on
> another one that adds the extended descriptor support. Also If I add
> all the code in a single patch this will be very big. I had some
> problems to review all separately. So I suspect that if we merge all
> in a single patch this will not help (especially myself). At any rate,
> tell me if you prefer to have a single patch. I can do that.

I am not asking for bigger patches. It is good to arrange your patches
in small steps, since that makes both reviewing and bisecting easier.

However, for brand new code, I find it quite annoying to read one
patch, and then to have it all re-written in the next one. (If a new
function *only* grows during a patch series, that is easy to follow.)

So what I would like to see is a logical, understandable series of
small steps, but when new code appears, it is the real, final form.

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html