Message-ID: <1428653720.7346.70.camel@pengutronix.de>
Subject: Re: [PATCH] PPS: Restore lost capture-clear option to pps-gpio
 module.
From: Jan =?ISO-8859-1?Q?L=FCbbe?= <jlu@pengutronix.de>
To: "Gary E. Miller" <gem@rellim.com>
Cc: linux-kernel@vger.kernel.org, Rodolfo Giometti <giometti@enneenne.com>,
        Ricardo Martin s <rasm@fe.up.pt>,
        James Nus s <jamesnuss@nanometrics.ca>
Date: Fri, 10 Apr 2015 10:15:20 +0200
In-Reply-To: <20150409124945.2743383e@rellim.com>
References: <201504022004.t32K4xbW013508@spidey.rellim.com>
	 <1428584709.7346.25.camel@pengutronix.de>
	 <20150409124945.2743383e@rellim.com>
Organization: Pengutronix
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Mime-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3230
Lines: 80

On Do, 2015-04-09 at 12:49 -0700, Gary E. Miller wrote:
> Yo Jan!
> 
> On Thu, 09 Apr 2015 15:05:09 +0200
> Jan Lübbe <jlu@pengutronix.de> wrote:
> 
> > Hi Gary,
> > 
> > On Do, 2015-04-02 at 12:21 -0700, Gary E. Miller wrote:
> > > In the conversion from platform to device tree the capture-clear
> > > option was lost.
> > > 
> > > capture-clear is needed so that time_pps_fetch() will report both
> > > edges of each PPS pulse.  Both edges are needed so that userland
> > > programs, like gpsd, can autodetect the leading and trailing PPS
> > > edges.
> > 
> > This is intentional. The userspace program can configure which edges
> > it wants to capture at runtime.
> 
> Yes, on pps-ldisc, but no on pps-gpio.
> 
> If you think I am wrong then please show me how I can do that in my
> userspace program. I have talked to several experts that could not
> figure out how to do so.  They all started with the same assertion as
> yours.
> 
> The time_pps_setcap() function call will not work to do so because
> the PPS_CAPTUREASSERT is locked out before time_pps_getcap().

Yes, you are correct. Looking at pps_cdev_ioctl(PPS_SETPARAMS), it will
check the requested mode against pps->info.mode and then store the
configured mode in pps->params.mode.

> Notice that info->catpure_clear can ONLY be set one place in the entire
> driver, and that place is during platform probing.  pps-gpio.c line 110.
> No other place.
> 
> Without info->capture_clear it is not possible to capture the clear
> edge.   Check pps-gpio.c lines 68 and 82.

Yes. The IRQ capabilities (as stored in info.mode) only depend on the
actual capabilities of the GPIO, so it shouldn't be necessary to declare
them again in the pps-gpio node.

> > Only the polarity
> > (assert-falling-edge) is actually determined by the hardware and must
> > be described in the device tree.
> 
> Now, but when it was a platform driver the capture assert could also be
> selected in the platform hardware description.  I'm just trying to
> restore lost functionality.
> 
> If instead you would be in favor of always allowing capture assert that
> would be good as well.

I'm strongly against adding "configuration" (as opposed to HW
description) to DT. To me, the correct way is to allow configuration of
capture clear from userspace if the HW supports it.

To do that, the pps-gpio driver would need to check (probably in
pps_gpio_probe) if the GPIO supports falling and/or rising edges. Then
it can set the info.mode field correctly. From
pps_cdev_ioctl(PPS_SETPARAMS) we'd then need to cause the driver to
reconfigure its IRQ. This way we won't cause unnecessary interrupts for
edges not needed by userspace.

Regards,
Jan
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/