From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965473AbbEMTjh (ORCPT ); Wed, 13 May 2015 15:39:37 -0400 Received: from mail-gw1-out.broadcom.com ([216.31.210.62]:4833 "EHLO mail-gw1-out.broadcom.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965423AbbEMTjc (ORCPT ); Wed, 13 May 2015 15:39:32 -0400 X-IronPort-AV: E=Sophos;i="5.13,422,1427785200"; d="scan'208";a="64946703" Message-ID: <5553A873.1010803@broadcom.com> Date: Wed, 13 May 2015 12:39:31 -0700 From: Jonathan Richardson User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: Richard Cochran CC: Arnd Bergmann , Darren Edamura , Mark Rutland , Scott Branden , Pawel Moll , Ian Campbell , Ray Jui , "Greg Kroah-Hartman" , , , Rob Herring , , Kumar Gala , Subject: Re: [PATCH 2/2] misc: Add initial Digital Timing Engine (DTE) driver for cygnus References: <1429744923-2007-1-git-send-email-jonathar@broadcom.com> <1429744923-2007-3-git-send-email-jonathar@broadcom.com> <6346218.3x3hbSlxMs@wuerfel> <5543CD73.2030902@broadcom.com> <20150513152130.GB2065@localhost.localdomain> In-Reply-To: <20150513152130.GB2065@localhost.localdomain> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Richard, glad you retrieved our driver. Comments below. On 15-05-13 08:21 AM, Richard Cochran wrote: > On Fri, May 01, 2015 at 12:01:07PM -0700, Jonathan Richardson wrote: >> The DTE creates timestamps of hardware based events such as GPIO, I2S >> signals for audio, etc. It was also intended to provide 802.1AS / PTP >> timestamps for network packets. The h/w has up to 32 "clients" -- the >> hardware inputs into a timestamping engine. These clients are specific >> to the chip the DTE is used on. For Cygnus you can see what they are in >> our 'enum dte_client' from bcm_cygnus_dte.h. > > These 32 channels should either go to kernel consumers (i2s audio) or, > if there aren't any, to the generic PTP ioctl. > >> The DTE timestamper creates timestamps based on the current clock wall >> time. > > What do you mean by "wall time"? CLOCK_REALTIME? It's just the incrementing local NCO time. Whatever the value of it is is the timestamp time. When we add the timestamp to the s/w FIFO we adjust the time to a reference time. > >> When an event occurs it stores the timestamp in a h/w FIFO. Each >> client also has a divider that can be set to control the rate that >> timestamps are generated at by the timestamper and these are adjustable >> at run time. > > This does not make sense. If you time stamp events, then the rate is > determined by the events themselves. The input events may occur at a rate greater than we want to generate timestamps at. Maybe we need a better term than divider. The h/w sees x input signals over x period of time and then samples the input signal at a divided down period of time to limit the timestamp rate. Timestamp input sample rate? > >> It's a bit more than a PTP hardware clock on a NIC. It's a clock for PTP >> plus timestamping 32 other hardware inputs that can be enabled at any >> time with timestamps being generated at varying frequencies. As clients >> are enabled that generate timestamps at higher frequencies, the >> isochronous interrupt frequency needs to be increased so that overflows >> in the the h/w and s/w FIFO's don't occur (this frequency could possibly >> be automated instead of changing it manually as we currently do). > > Yes, the driver should configure an appropriate rate automatically. I was wrong. It can't. The DTE doesn't know anything about the inputs hooked up to it nor the frequency of the input events. It needs to be set by user space and the value can change at run time whenever we want to change it. > >> It looks like the driver could almost be a PTP driver instead of a char >> driver controlled with ioctls. PTP does this already using >> clock_gettime(), clock_settime(), clock_adjtime(). But we want to set >> the frequency as well as adjust the clock and I don't see how that is >> possible with the stripped down timex data passed to the driver from >> ptp_clock_adjtime(). > > You can convert ppb into your time base using simple math. Yes, this isn't a problem. > >> We have the additional requirement of controlling multiple clients and >> retrieving the timestamps, etc. The PTP driver allows for some control >> of external time stamp channels already using 'n_ext_ts' in struct >> ptp_clock_info. We could use that to enable clients and get timestamps, >> but we also need to be able to change dividers for the clients at run >> time if desired. It doesn't look like additional ioctls could be passed >> to a PTP driver because ptp_ioctl() is the ioctl handler. > > I don't see any need for use space to set dividers. Let the driver do > that. See above. Thanks. > > Thanks, > Richard > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jonathan Richardson Subject: Re: [PATCH 2/2] misc: Add initial Digital Timing Engine (DTE) driver for cygnus Date: Wed, 13 May 2015 12:39:31 -0700 Message-ID: <5553A873.1010803@broadcom.com> References: <1429744923-2007-1-git-send-email-jonathar@broadcom.com> <1429744923-2007-3-git-send-email-jonathar@broadcom.com> <6346218.3x3hbSlxMs@wuerfel> <5543CD73.2030902@broadcom.com> <20150513152130.GB2065@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20150513152130.GB2065-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Richard Cochran Cc: Arnd Bergmann , Darren Edamura , Mark Rutland , Scott Branden , Pawel Moll , Ian Campbell , Ray Jui , Greg Kroah-Hartman , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring , bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org, Kumar Gala , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org List-Id: devicetree@vger.kernel.org Richard, glad you retrieved our driver. Comments below. On 15-05-13 08:21 AM, Richard Cochran wrote: > On Fri, May 01, 2015 at 12:01:07PM -0700, Jonathan Richardson wrote: >> The DTE creates timestamps of hardware based events such as GPIO, I2S >> signals for audio, etc. It was also intended to provide 802.1AS / PTP >> timestamps for network packets. The h/w has up to 32 "clients" -- the >> hardware inputs into a timestamping engine. These clients are specific >> to the chip the DTE is used on. For Cygnus you can see what they are in >> our 'enum dte_client' from bcm_cygnus_dte.h. > > These 32 channels should either go to kernel consumers (i2s audio) or, > if there aren't any, to the generic PTP ioctl. > >> The DTE timestamper creates timestamps based on the current clock wall >> time. > > What do you mean by "wall time"? CLOCK_REALTIME? It's just the incrementing local NCO time. Whatever the value of it is is the timestamp time. When we add the timestamp to the s/w FIFO we adjust the time to a reference time. > >> When an event occurs it stores the timestamp in a h/w FIFO. Each >> client also has a divider that can be set to control the rate that >> timestamps are generated at by the timestamper and these are adjustable >> at run time. > > This does not make sense. If you time stamp events, then the rate is > determined by the events themselves. The input events may occur at a rate greater than we want to generate timestamps at. Maybe we need a better term than divider. The h/w sees x input signals over x period of time and then samples the input signal at a divided down period of time to limit the timestamp rate. Timestamp input sample rate? > >> It's a bit more than a PTP hardware clock on a NIC. It's a clock for PTP >> plus timestamping 32 other hardware inputs that can be enabled at any >> time with timestamps being generated at varying frequencies. As clients >> are enabled that generate timestamps at higher frequencies, the >> isochronous interrupt frequency needs to be increased so that overflows >> in the the h/w and s/w FIFO's don't occur (this frequency could possibly >> be automated instead of changing it manually as we currently do). > > Yes, the driver should configure an appropriate rate automatically. I was wrong. It can't. The DTE doesn't know anything about the inputs hooked up to it nor the frequency of the input events. It needs to be set by user space and the value can change at run time whenever we want to change it. > >> It looks like the driver could almost be a PTP driver instead of a char >> driver controlled with ioctls. PTP does this already using >> clock_gettime(), clock_settime(), clock_adjtime(). But we want to set >> the frequency as well as adjust the clock and I don't see how that is >> possible with the stripped down timex data passed to the driver from >> ptp_clock_adjtime(). > > You can convert ppb into your time base using simple math. Yes, this isn't a problem. > >> We have the additional requirement of controlling multiple clients and >> retrieving the timestamps, etc. The PTP driver allows for some control >> of external time stamp channels already using 'n_ext_ts' in struct >> ptp_clock_info. We could use that to enable clients and get timestamps, >> but we also need to be able to change dividers for the clients at run >> time if desired. It doesn't look like additional ioctls could be passed >> to a PTP driver because ptp_ioctl() is the ioctl handler. > > I don't see any need for use space to set dividers. Let the driver do > that. See above. Thanks. > > Thanks, > Richard > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: jonathar@broadcom.com (Jonathan Richardson) Date: Wed, 13 May 2015 12:39:31 -0700 Subject: [PATCH 2/2] misc: Add initial Digital Timing Engine (DTE) driver for cygnus In-Reply-To: <20150513152130.GB2065@localhost.localdomain> References: <1429744923-2007-1-git-send-email-jonathar@broadcom.com> <1429744923-2007-3-git-send-email-jonathar@broadcom.com> <6346218.3x3hbSlxMs@wuerfel> <5543CD73.2030902@broadcom.com> <20150513152130.GB2065@localhost.localdomain> Message-ID: <5553A873.1010803@broadcom.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Richard, glad you retrieved our driver. Comments below. On 15-05-13 08:21 AM, Richard Cochran wrote: > On Fri, May 01, 2015 at 12:01:07PM -0700, Jonathan Richardson wrote: >> The DTE creates timestamps of hardware based events such as GPIO, I2S >> signals for audio, etc. It was also intended to provide 802.1AS / PTP >> timestamps for network packets. The h/w has up to 32 "clients" -- the >> hardware inputs into a timestamping engine. These clients are specific >> to the chip the DTE is used on. For Cygnus you can see what they are in >> our 'enum dte_client' from bcm_cygnus_dte.h. > > These 32 channels should either go to kernel consumers (i2s audio) or, > if there aren't any, to the generic PTP ioctl. > >> The DTE timestamper creates timestamps based on the current clock wall >> time. > > What do you mean by "wall time"? CLOCK_REALTIME? It's just the incrementing local NCO time. Whatever the value of it is is the timestamp time. When we add the timestamp to the s/w FIFO we adjust the time to a reference time. > >> When an event occurs it stores the timestamp in a h/w FIFO. Each >> client also has a divider that can be set to control the rate that >> timestamps are generated at by the timestamper and these are adjustable >> at run time. > > This does not make sense. If you time stamp events, then the rate is > determined by the events themselves. The input events may occur at a rate greater than we want to generate timestamps at. Maybe we need a better term than divider. The h/w sees x input signals over x period of time and then samples the input signal at a divided down period of time to limit the timestamp rate. Timestamp input sample rate? > >> It's a bit more than a PTP hardware clock on a NIC. It's a clock for PTP >> plus timestamping 32 other hardware inputs that can be enabled at any >> time with timestamps being generated at varying frequencies. As clients >> are enabled that generate timestamps at higher frequencies, the >> isochronous interrupt frequency needs to be increased so that overflows >> in the the h/w and s/w FIFO's don't occur (this frequency could possibly >> be automated instead of changing it manually as we currently do). > > Yes, the driver should configure an appropriate rate automatically. I was wrong. It can't. The DTE doesn't know anything about the inputs hooked up to it nor the frequency of the input events. It needs to be set by user space and the value can change at run time whenever we want to change it. > >> It looks like the driver could almost be a PTP driver instead of a char >> driver controlled with ioctls. PTP does this already using >> clock_gettime(), clock_settime(), clock_adjtime(). But we want to set >> the frequency as well as adjust the clock and I don't see how that is >> possible with the stripped down timex data passed to the driver from >> ptp_clock_adjtime(). > > You can convert ppb into your time base using simple math. Yes, this isn't a problem. > >> We have the additional requirement of controlling multiple clients and >> retrieving the timestamps, etc. The PTP driver allows for some control >> of external time stamp channels already using 'n_ext_ts' in struct >> ptp_clock_info. We could use that to enable clients and get timestamps, >> but we also need to be able to change dividers for the clients at run >> time if desired. It doesn't look like additional ioctls could be passed >> to a PTP driver because ptp_ioctl() is the ioctl handler. > > I don't see any need for use space to set dividers. Let the driver do > that. See above. Thanks. > > Thanks, > Richard >