netdev - Re: [PATCH V2 0/7] support the cpts found on am335x devices

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] [day] [month] [year] [list]

Message-ID: <20121022110240.GA2285@netboy.at.omicron.at>
Date:	Mon, 22 Oct 2012 13:02:40 +0200
From:	Richard Cochran <richardcochran@...il.com>
To:	"N, Mugunthan V" <mugunthanvnm@...com>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	David Miller <davem@...emloft.net>,
	"Chemparathy, Cyril" <cyril@...com>,
	"Govindarajan, Sriramakrishnan" <srk@...com>
Subject: Re: [PATCH V2 0/7] support the cpts found on am335x devices

On Mon, Oct 22, 2012 at 10:39:40AM +0000, N, Mugunthan V wrote:
> 
> I think the timecounter_init initializes only the software. But still I am
> not clear how the time given in cpts_ptp_settime will be projected to
> hardware. Can you correct me if I am wrong.

Look at the settime function:

+static int cpts_ptp_settime(struct ptp_clock_info *ptp,
+			    const struct timespec *ts)
+{
+	u64 ns;
+	unsigned long flags;
+	struct cpts *cpts = container_of(ptp, struct cpts, info);
+
+	ns = ts->tv_sec * 1000000000ULL;
+	ns += ts->tv_nsec;
+
+	spin_lock_irqsave(&cpts->lock, flags);
+	timecounter_init(&cpts->tc, &cpts->cc, ns);
+	spin_unlock_irqrestore(&cpts->lock, flags);
+
+	return 0;
+}

Then, look at the implementation of timecounter_init().

> > > Since we poll for the 32bit over flow for every HZ * 8 cycle, won't
> > > there be a system overhead. If the CPTS ref clock is changed
> > according
> > > to the ptp freq adjust api, how will the timecounter take care of
> > change
> > > in frequency
> > 
> > There is nothing to do here. What are you asking?
> 
> I think instead of fixing the driver to AM335X, let's make the driver
> generic as the same driver can be used to TI814X, TI813X and other
> upcoming platforms TI811X where the CPTS ref clock can be changed as
> per usage requirement.

Now I see what your are worried about, but it really isn't a big
problem. The solution in your driver of doing a 64 bit division in the
hot path is unattractive and unnecessary. We can accomodate variable
reference clock frequencies using a board specific DT, just as we do
for gianfar today.

Let's just get the am335x working, shall we? Until we actually have
the other boards working, all this talk about TI81xx is really just
talk. I mean, ATM the CPSW does not work on *any* board at all.

> > I hope that you meant, "better if we can configure _either_ slave."
> > Considering how SO_TIMESTAMPING works, you can't use both at once.
> > 
> 
> Since CPSW as a Ethernet switch, the PTP packet can be delivered to
> any downstream port. So if we fix the PTP time stamping to one port
> then the driver won't receive time stamping information when the packet
> is delivered to the other port.

And just how is this supposed to work with SO_TIMESTAMPING?

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