All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] pps-gpio: implement echo pulses
@ 2018-02-06 15:58 Lukas Senger
  2018-02-07  9:09 ` Rodolfo Giometti
  0 siblings, 1 reply; 12+ messages in thread
From: Lukas Senger @ 2018-02-06 15:58 UTC (permalink / raw)
  To: giometti; +Cc: linux-kernel, Lukas Senger

pps-gpio reports as having echo capability via sysfs, which is not
actually the case. This patch implements it.

The output pin is hardcoded as 17. This should probably be configurable
via the dtoverlay in the same way as the input pin.
---
 drivers/pps/clients/pps-gpio.c | 55 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 50 insertions(+), 5 deletions(-)

diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
index 333ad7d5..dd7624f1 100644
--- a/drivers/pps/clients/pps-gpio.c
+++ b/drivers/pps/clients/pps-gpio.c
@@ -35,6 +35,12 @@
 #include <linux/list.h>
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
+#include <linux/delay.h>
+
+/* TODO: this should work like gpio_pin below but I don't know how to work with
+ * devicetree overlays.
+ */
+#define PPS_GPIO_ECHO_PIN 17
 
 /* Info for each registered platform device */
 struct pps_gpio_device_data {
@@ -63,16 +69,41 @@ static irqreturn_t pps_gpio_irq_handler(int irq, void *data)
 
 	rising_edge = gpio_get_value(info->gpio_pin);
 	if ((rising_edge && !info->assert_falling_edge) ||
-			(!rising_edge && info->assert_falling_edge))
+			(!rising_edge && info->assert_falling_edge)) {
+		if (info->pps->params.mode & PPS_ECHOASSERT) {
+			gpio_set_value(PPS_GPIO_ECHO_PIN, 1);
+		}
 		pps_event(info->pps, &ts, PPS_CAPTUREASSERT, NULL);
-	else if (info->capture_clear &&
+	} else if (info->capture_clear &&
 			((rising_edge && info->assert_falling_edge) ||
-			 (!rising_edge && !info->assert_falling_edge)))
+			 (!rising_edge && !info->assert_falling_edge))) {
+		if (info->pps->params.mode & PPS_ECHOCLEAR) {
+			gpio_set_value(PPS_GPIO_ECHO_PIN, 1);
+		}
 		pps_event(info->pps, &ts, PPS_CAPTURECLEAR, NULL);
+	}
 
+	if (info->pps->params.mode & (PPS_ECHOASSERT | PPS_ECHOCLEAR))
+		return IRQ_WAKE_THREAD;
 	return IRQ_HANDLED;
 }
 
+/* If we sent an echo pulse, we set the output pin back to low. This results in
+ * an echo pulse of roughly 100ms length.
+ */
+static irqreturn_t pps_gpio_irq_threaded(int irq, void *data)
+{
+	const struct pps_gpio_device_data *info;
+
+	info = data;
+
+	msleep(100);
+	gpio_set_value(PPS_GPIO_ECHO_PIN, 0);
+
+	return IRQ_HANDLED;
+}
+
+
 static unsigned long
 get_irqf_trigger_flags(const struct pps_gpio_device_data *data)
 {
@@ -131,7 +162,20 @@ static int pps_gpio_probe(struct platform_device *pdev)
 
 	ret = gpio_direction_input(data->gpio_pin);
 	if (ret) {
-		dev_err(&pdev->dev, "failed to set pin direction\n");
+		dev_err(&pdev->dev, "failed to set pin as input\n");
+		return -EINVAL;
+	}
+
+	ret = devm_gpio_request(&pdev->dev, PPS_GPIO_ECHO_PIN, gpio_label);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request GPIO %u\n",
+			PPS_GPIO_ECHO_PIN);
+		return ret;
+	}
+
+	ret = gpio_direction_output(PPS_GPIO_ECHO_PIN, 0);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to set pin as output\n");
 		return -EINVAL;
 	}
 
@@ -165,7 +209,8 @@ static int pps_gpio_probe(struct platform_device *pdev)
 	}
 
 	/* register IRQ interrupt handler */
-	ret = devm_request_irq(&pdev->dev, data->irq, pps_gpio_irq_handler,
+	ret = devm_request_threaded_irq(&pdev->dev, data->irq,
+			pps_gpio_irq_handler, pps_gpio_irq_threaded,
 			get_irqf_trigger_flags(data), data->info.name, data);
 	if (ret) {
 		pps_unregister_source(data->pps);
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] pps-gpio: implement echo pulses
  2018-02-06 15:58 [PATCH] pps-gpio: implement echo pulses Lukas Senger
@ 2018-02-07  9:09 ` Rodolfo Giometti
  2018-02-15 12:59   ` Lukas Senger
  0 siblings, 1 reply; 12+ messages in thread
From: Rodolfo Giometti @ 2018-02-07  9:09 UTC (permalink / raw)
  To: Lukas Senger; +Cc: linux-kernel

On 06/02/18 16:58, Lukas Senger wrote:
> pps-gpio reports as having echo capability via sysfs, which is not
> actually the case. This patch implements it.
> 
> The output pin is hardcoded as 17. This should probably be configurable
> via the dtoverlay in the same way as the input pin.

No hardcoded stuff please! :-D

However thank you for your tutorial code, maybe someone will find it very useful.

Ciao,

Rodolfo

-- 

HCE Engineering                      e-mail: giometti@hce-engineering.it
GNU/Linux Solutions                          giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming                     skype:  rodolfo.giometti
Cosino Project - the quick prototyping embedded system - www.cosino.it
Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] pps-gpio: implement echo pulses
  2018-02-07  9:09 ` Rodolfo Giometti
@ 2018-02-15 12:59   ` Lukas Senger
  2018-02-15 12:59     ` [PATCH 1/2] pps-gpio: Avoid flooding syslog Lukas Senger
  2018-02-15 12:59     ` [PATCH 2/2] pps-gpio: Set echo GPIO pin via devicetree Lukas Senger
  0 siblings, 2 replies; 12+ messages in thread
From: Lukas Senger @ 2018-02-15 12:59 UTC (permalink / raw)
  To: giometti; +Cc: linux-kernel

> No hardcoded stuff please! :-D

I expected as much :) So I looked into it a bit more and came up with
the patch below. It works for me but I'm unsure about two things in
particular:

In the __overrides__ section of the *.dts, why does gpiopin set reg and
should echopin set this as well?

In pps-gpio.c:135 how is pdata in the first part of this if-statement
supposed to know about these values?

Thanks,
Lukas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] pps-gpio: Avoid flooding syslog
  2018-02-15 12:59   ` Lukas Senger
@ 2018-02-15 12:59     ` Lukas Senger
  2018-02-15 13:34       ` Rodolfo Giometti
  2018-02-15 12:59     ` [PATCH 2/2] pps-gpio: Set echo GPIO pin via devicetree Lukas Senger
  1 sibling, 1 reply; 12+ messages in thread
From: Lukas Senger @ 2018-02-15 12:59 UTC (permalink / raw)
  To: giometti; +Cc: linux-kernel, Lukas Senger

---
 drivers/pps/clients/pps-gpio.c | 1 +
 include/linux/pps-gpio.h       | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
index dd7624f1d23f..35c3b14fc9b9 100644
--- a/drivers/pps/clients/pps-gpio.c
+++ b/drivers/pps/clients/pps-gpio.c
@@ -196,6 +196,7 @@ static int pps_gpio_probe(struct platform_device *pdev)
 	data->info.owner = THIS_MODULE;
 	snprintf(data->info.name, PPS_MAX_NAME_LEN - 1, "%s.%d",
 		 pdev->name, pdev->id);
+	data->info.echo = pps_gpio_echo;
 
 	/* register PPS source */
 	pps_default_params = PPS_CAPTUREASSERT | PPS_OFFSETASSERT;
diff --git a/include/linux/pps-gpio.h b/include/linux/pps-gpio.h
index 0035abe41b9a..67f50e8dcd11 100644
--- a/include/linux/pps-gpio.h
+++ b/include/linux/pps-gpio.h
@@ -29,4 +29,6 @@ struct pps_gpio_platform_data {
 	const char *gpio_label;
 };
 
+static void pps_gpio_echo(struct pps_device *pps, int event, void *data){}
+
 #endif
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] pps-gpio: Set echo GPIO pin via devicetree
  2018-02-15 12:59   ` Lukas Senger
  2018-02-15 12:59     ` [PATCH 1/2] pps-gpio: Avoid flooding syslog Lukas Senger
@ 2018-02-15 12:59     ` Lukas Senger
  2018-02-15 13:43       ` Rodolfo Giometti
  1 sibling, 1 reply; 12+ messages in thread
From: Lukas Senger @ 2018-02-15 12:59 UTC (permalink / raw)
  To: giometti; +Cc: linux-kernel, Lukas Senger

---
 arch/arm/boot/dts/overlays/pps-gpio-overlay.dts | 13 ++++++++-----
 drivers/pps/clients/pps-gpio.c                  | 26 ++++++++++++++-----------
 include/linux/pps-gpio.h                        |  1 +
 3 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/arch/arm/boot/dts/overlays/pps-gpio-overlay.dts b/arch/arm/boot/dts/overlays/pps-gpio-overlay.dts
index 9ee4bdfa6167..06e6cf5fc6ea 100644
--- a/arch/arm/boot/dts/overlays/pps-gpio-overlay.dts
+++ b/arch/arm/boot/dts/overlays/pps-gpio-overlay.dts
@@ -10,7 +10,8 @@
 				compatible = "pps-gpio";
 				pinctrl-names = "default";
 				pinctrl-0 = <&pps_pins>;
-				gpios = <&gpio 18 0>;
+				in-gpios = <&gpio 18 0>;
+				out-gpios = <&gpio 17 0>;
 				status = "okay";
 			};
 		};
@@ -20,18 +21,20 @@
 		target = <&gpio>;
 		__overlay__ {
 			pps_pins: pps_pins@12 {
-				brcm,pins =     <18>;
-				brcm,function = <0>;    // in
-				brcm,pull =     <0>;    // off
+				brcm,pins =     <18 17>;
+				brcm,function = <0 1>;    // in  out
+				brcm,pull =     <0 0>;    // off off
 			};
 		};
 	};
 
 	__overrides__ {
-		gpiopin = <&pps>,"gpios:4",
+		gpiopin = <&pps>,"in-gpios:4",
 			  <&pps>,"reg:0",
 			  <&pps_pins>,"brcm,pins:0",
 			  <&pps_pins>,"reg:0";
+		echopin = <&pps>,"out-gpios:4",
+			  <&pps_pins>,"brcm,pins:4";
 		assert_falling_edge = <&pps>,"assert-falling-edge?";
 	};
 };
diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
index 35c3b14fc9b9..ce3065889a7e 100644
--- a/drivers/pps/clients/pps-gpio.c
+++ b/drivers/pps/clients/pps-gpio.c
@@ -37,10 +37,6 @@
 #include <linux/of_gpio.h>
 #include <linux/delay.h>
 
-/* TODO: this should work like gpio_pin below but I don't know how to work with
- * devicetree overlays.
- */
-#define PPS_GPIO_ECHO_PIN 17
 
 /* Info for each registered platform device */
 struct pps_gpio_device_data {
@@ -50,6 +46,7 @@ struct pps_gpio_device_data {
 	bool assert_falling_edge;
 	bool capture_clear;
 	unsigned int gpio_pin;
+	unsigned int echo_pin;
 };
 
 /*
@@ -71,14 +68,14 @@ static irqreturn_t pps_gpio_irq_handler(int irq, void *data)
 	if ((rising_edge && !info->assert_falling_edge) ||
 			(!rising_edge && info->assert_falling_edge)) {
 		if (info->pps->params.mode & PPS_ECHOASSERT) {
-			gpio_set_value(PPS_GPIO_ECHO_PIN, 1);
+			gpio_set_value(info->echo_pin, 1);
 		}
 		pps_event(info->pps, &ts, PPS_CAPTUREASSERT, NULL);
 	} else if (info->capture_clear &&
 			((rising_edge && info->assert_falling_edge) ||
 			 (!rising_edge && !info->assert_falling_edge))) {
 		if (info->pps->params.mode & PPS_ECHOCLEAR) {
-			gpio_set_value(PPS_GPIO_ECHO_PIN, 1);
+			gpio_set_value(info->echo_pin, 1);
 		}
 		pps_event(info->pps, &ts, PPS_CAPTURECLEAR, NULL);
 	}
@@ -98,7 +95,7 @@ static irqreturn_t pps_gpio_irq_threaded(int irq, void *data)
 	info = data;
 
 	msleep(100);
-	gpio_set_value(PPS_GPIO_ECHO_PIN, 0);
+	gpio_set_value(info->echo_pin, 0);
 
 	return IRQ_HANDLED;
 }
@@ -135,17 +132,24 @@ static int pps_gpio_probe(struct platform_device *pdev)
 
 	if (pdata) {
 		data->gpio_pin = pdata->gpio_pin;
+		data->echo_pin = pdata->echo_pin;
 		gpio_label = pdata->gpio_label;
 
 		data->assert_falling_edge = pdata->assert_falling_edge;
 		data->capture_clear = pdata->capture_clear;
 	} else {
-		ret = of_get_gpio(np, 0);
+		ret = of_get_named_gpio(np, "in-gpios", 0);
 		if (ret < 0) {
 			dev_err(&pdev->dev, "failed to get GPIO from device tree\n");
 			return ret;
 		}
 		data->gpio_pin = ret;
+		ret = of_get_named_gpio(np, "out-gpios", 0);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "failed to get second GPIO from device tree\n");
+			return ret;
+		}
+		data->echo_pin = ret;
 		gpio_label = PPS_GPIO_NAME;
 
 		if (of_get_property(np, "assert-falling-edge", NULL))
@@ -166,14 +170,14 @@ static int pps_gpio_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	ret = devm_gpio_request(&pdev->dev, PPS_GPIO_ECHO_PIN, gpio_label);
+	ret = devm_gpio_request(&pdev->dev, data->echo_pin, gpio_label);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to request GPIO %u\n",
-			PPS_GPIO_ECHO_PIN);
+			data->echo_pin);
 		return ret;
 	}
 
-	ret = gpio_direction_output(PPS_GPIO_ECHO_PIN, 0);
+	ret = gpio_direction_output(data->echo_pin, 0);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to set pin as output\n");
 		return -EINVAL;
diff --git a/include/linux/pps-gpio.h b/include/linux/pps-gpio.h
index 67f50e8dcd11..de1701ae1c6a 100644
--- a/include/linux/pps-gpio.h
+++ b/include/linux/pps-gpio.h
@@ -26,6 +26,7 @@ struct pps_gpio_platform_data {
 	bool assert_falling_edge;
 	bool capture_clear;
 	unsigned int gpio_pin;
+	unsigned int echo_pin;
 	const char *gpio_label;
 };
 
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] pps-gpio: Avoid flooding syslog
  2018-02-15 12:59     ` [PATCH 1/2] pps-gpio: Avoid flooding syslog Lukas Senger
@ 2018-02-15 13:34       ` Rodolfo Giometti
  2018-02-15 15:12         ` Lukas Senger
  0 siblings, 1 reply; 12+ messages in thread
From: Rodolfo Giometti @ 2018-02-15 13:34 UTC (permalink / raw)
  To: Lukas Senger; +Cc: linux-kernel

On 15/02/18 13:59, Lukas Senger wrote:

^^^^^^^^^^^^^^^^^^^
Missing description and signatures...

> ---
>   drivers/pps/clients/pps-gpio.c | 1 +
>   include/linux/pps-gpio.h       | 2 ++
>   2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
> index dd7624f1d23f..35c3b14fc9b9 100644
> --- a/drivers/pps/clients/pps-gpio.c
> +++ b/drivers/pps/clients/pps-gpio.c
> @@ -196,6 +196,7 @@ static int pps_gpio_probe(struct platform_device *pdev)
>   	data->info.owner = THIS_MODULE;
>   	snprintf(data->info.name, PPS_MAX_NAME_LEN - 1, "%s.%d",
>   		 pdev->name, pdev->id);
> +	data->info.echo = pps_gpio_echo;
>   
>   	/* register PPS source */
>   	pps_default_params = PPS_CAPTUREASSERT | PPS_OFFSETASSERT;
> diff --git a/include/linux/pps-gpio.h b/include/linux/pps-gpio.h
> index 0035abe41b9a..67f50e8dcd11 100644
> --- a/include/linux/pps-gpio.h
> +++ b/include/linux/pps-gpio.h
> @@ -29,4 +29,6 @@ struct pps_gpio_platform_data {
>   	const char *gpio_label;
>   };
>   
> +static void pps_gpio_echo(struct pps_device *pps, int event, void *data){}
> +
>   #endif

Why a void function? You should use it to toggle echoing GPIO... =8-o

Ciao,

Rodolfo

-- 

HCE Engineering                      e-mail: giometti@hce-engineering.it
GNU/Linux Solutions                          giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming                     skype:  rodolfo.giometti
Cosino Project - the quick prototyping embedded system - www.cosino.it
Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] pps-gpio: Set echo GPIO pin via devicetree
  2018-02-15 12:59     ` [PATCH 2/2] pps-gpio: Set echo GPIO pin via devicetree Lukas Senger
@ 2018-02-15 13:43       ` Rodolfo Giometti
  2018-02-15 15:08         ` Lukas Senger
  0 siblings, 1 reply; 12+ messages in thread
From: Rodolfo Giometti @ 2018-02-15 13:43 UTC (permalink / raw)
  To: Lukas Senger; +Cc: linux-kernel

On 15/02/18 13:59, Lukas Senger wrote:
> ---
>   arch/arm/boot/dts/overlays/pps-gpio-overlay.dts | 13 ++++++++-----
>   drivers/pps/clients/pps-gpio.c                  | 26 ++++++++++++++-----------
>   include/linux/pps-gpio.h                        |  1 +
>   3 files changed, 24 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/overlays/pps-gpio-overlay.dts b/arch/arm/boot/dts/overlays/pps-gpio-overlay.dts
> index 9ee4bdfa6167..06e6cf5fc6ea 100644
> --- a/arch/arm/boot/dts/overlays/pps-gpio-overlay.dts
> +++ b/arch/arm/boot/dts/overlays/pps-gpio-overlay.dts
> @@ -10,7 +10,8 @@
>   				compatible = "pps-gpio";
>   				pinctrl-names = "default";
>   				pinctrl-0 = <&pps_pins>;
> -				gpios = <&gpio 18 0>;
> +				in-gpios = <&gpio 18 0>;

Please, don't break backward compatibility! You can leave "gpios" as is and 
using, for instance, "echo-gpios" for echoing purposes. -+
                                                          |
                                     +--------------------+
                                     |
                                 vvvvvvvvv
> +				out-gpios = <&gpio 17 0>;
>   				status = "okay";
>   			};
>   		};
> @@ -20,18 +21,20 @@
>   		target = <&gpio>;
>   		__overlay__ {
>   			pps_pins: pps_pins@12 {
> -				brcm,pins =     <18>;
> -				brcm,function = <0>;    // in
> -				brcm,pull =     <0>;    // off
> +				brcm,pins =     <18 17>;
> +				brcm,function = <0 1>;    // in  out
> +				brcm,pull =     <0 0>;    // off off

These modifications are not PPS related.

>   			};
>   		};
>   	};
>   
>   	__overrides__ {
> -		gpiopin = <&pps>,"gpios:4",
> +		gpiopin = <&pps>,"in-gpios:4",
>   			  <&pps>,"reg:0",
>   			  <&pps_pins>,"brcm,pins:0",
>   			  <&pps_pins>,"reg:0";
> +		echopin = <&pps>,"out-gpios:4",
> +			  <&pps_pins>,"brcm,pins:4";

Ditto.

>   		assert_falling_edge = <&pps>,"assert-falling-edge?";
>   	};
>   };
> diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
> index 35c3b14fc9b9..ce3065889a7e 100644
> --- a/drivers/pps/clients/pps-gpio.c
> +++ b/drivers/pps/clients/pps-gpio.c
> @@ -37,10 +37,6 @@
>   #include <linux/of_gpio.h>
>   #include <linux/delay.h>
>   
> -/* TODO: this should work like gpio_pin below but I don't know how to work with
> - * devicetree overlays.
> - */
> -#define PPS_GPIO_ECHO_PIN 17

Please provide patches against current kernel code and not against your old patches.

I stop reviewing here since following modifications are similar to just reviewed 
and not acceptable. I'm sorry.

Ciao,

Rodolfo

-- 

HCE Engineering                      e-mail: giometti@hce-engineering.it
GNU/Linux Solutions                          giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming                     skype:  rodolfo.giometti
Cosino Project - the quick prototyping embedded system - www.cosino.it
Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] pps-gpio: Set echo GPIO pin via devicetree
  2018-02-15 13:43       ` Rodolfo Giometti
@ 2018-02-15 15:08         ` Lukas Senger
  2018-02-15 15:33           ` Rodolfo Giometti
  0 siblings, 1 reply; 12+ messages in thread
From: Lukas Senger @ 2018-02-15 15:08 UTC (permalink / raw)
  To: Rodolfo Giometti; +Cc: linux-kernel

> > @@ -20,18 +21,20 @@
> >   		target = <&gpio>;
> >   		__overlay__ {
> >   			pps_pins: pps_pins@12 {
> > -				brcm,pins =     <18>;
> > -				brcm,function = <0>;    // in
> > -				brcm,pull =     <0>;    // off
> > +				brcm,pins =     <18 17>;
> > +				brcm,function = <0 1>;    // in
> > out
> > +				brcm,pull =     <0 0>;    // off
> > off  
> 
> These modifications are not PPS related.
> 
> >   			};
> >   		};
> >   	};
> >   
> >   	__overrides__ {
> > -		gpiopin = <&pps>,"gpios:4",
> > +		gpiopin = <&pps>,"in-gpios:4",
> >   			  <&pps>,"reg:0",
> >   			  <&pps_pins>,"brcm,pins:0",
> >   			  <&pps_pins>,"reg:0";
> > +		echopin = <&pps>,"out-gpios:4",
> > +			  <&pps_pins>,"brcm,pins:4";  
> 
> Ditto.

I don't understand why these modifications are unrelated. Especially
the echopin-option should exist, shouldn't it?

Cheers,
Lukas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] pps-gpio: Avoid flooding syslog
  2018-02-15 13:34       ` Rodolfo Giometti
@ 2018-02-15 15:12         ` Lukas Senger
  2018-02-15 15:36           ` Rodolfo Giometti
  0 siblings, 1 reply; 12+ messages in thread
From: Lukas Senger @ 2018-02-15 15:12 UTC (permalink / raw)
  To: Rodolfo Giometti; +Cc: linux-kernel


> Why a void function? You should use it to toggle echoing GPIO... =8-o

RFC 2783 says to generate the echo pulse "as rapidly as possible" which
is why I do the toggling in the irq handler. However this leaves the
default echo function installed which just floods syslog.

Another problem with this patch is that it also is against my old one
and not against current kernel code.

Cheers,
Lukas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] pps-gpio: Set echo GPIO pin via devicetree
  2018-02-15 15:08         ` Lukas Senger
@ 2018-02-15 15:33           ` Rodolfo Giometti
  0 siblings, 0 replies; 12+ messages in thread
From: Rodolfo Giometti @ 2018-02-15 15:33 UTC (permalink / raw)
  To: Lukas Senger; +Cc: linux-kernel

On 15/02/18 16:08, Lukas Senger wrote:
>>> @@ -20,18 +21,20 @@
>>>    		target = <&gpio>;
>>>    		__overlay__ {
>>>    			pps_pins: pps_pins@12 {
>>> -				brcm,pins =     <18>;
>>> -				brcm,function = <0>;    // in
>>> -				brcm,pull =     <0>;    // off
>>> +				brcm,pins =     <18 17>;
>>> +				brcm,function = <0 1>;    // in
>>> out
>>> +				brcm,pull =     <0 0>;    // off
>>> off
>>
>> These modifications are not PPS related.
>>
>>>    			};
>>>    		};
>>>    	};
>>>    
>>>    	__overrides__ {
>>> -		gpiopin = <&pps>,"gpios:4",
>>> +		gpiopin = <&pps>,"in-gpios:4",
>>>    			  <&pps>,"reg:0",
>>>    			  <&pps_pins>,"brcm,pins:0",
>>>    			  <&pps_pins>,"reg:0";
>>> +		echopin = <&pps>,"out-gpios:4",
>>> +			  <&pps_pins>,"brcm,pins:4";
>>
>> Ditto.
> 
> I don't understand why these modifications are unrelated. Especially
> the echopin-option should exist, shouldn't it?

These modifications are needed to define a custom instance of a PPS device which 
is not part of the PPS subtree, that's why they should be put into another patch.

Ciao,

Rodolfo

-- 

HCE Engineering                      e-mail: giometti@hce-engineering.it
GNU/Linux Solutions                          giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming                     skype:  rodolfo.giometti
Cosino Project - the quick prototyping embedded system - www.cosino.it
Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] pps-gpio: Avoid flooding syslog
  2018-02-15 15:12         ` Lukas Senger
@ 2018-02-15 15:36           ` Rodolfo Giometti
  2018-02-15 19:48             ` Lukas Senger
  0 siblings, 1 reply; 12+ messages in thread
From: Rodolfo Giometti @ 2018-02-15 15:36 UTC (permalink / raw)
  To: Lukas Senger; +Cc: linux-kernel

On 15/02/18 16:12, Lukas Senger wrote:
> 
>> Why a void function? You should use it to toggle echoing GPIO... =8-o
> 
> RFC 2783 says to generate the echo pulse "as rapidly as possible" which
> is why I do the toggling in the irq handler. However this leaves the
> default echo function installed which just floods syslog.

The pps_event() function does it for you in a beauty-and-clear way by using the 
info.echo() function. :-D

> Another problem with this patch is that it also is against my old one
> and not against current kernel code.

Yes, correct it please.

Ciao,

Rodolfo

-- 

HCE Engineering                      e-mail: giometti@hce-engineering.it
GNU/Linux Solutions                          giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming                     skype:  rodolfo.giometti
Cosino Project - the quick prototyping embedded system - www.cosino.it
Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] pps-gpio: Avoid flooding syslog
  2018-02-15 15:36           ` Rodolfo Giometti
@ 2018-02-15 19:48             ` Lukas Senger
  0 siblings, 0 replies; 12+ messages in thread
From: Lukas Senger @ 2018-02-15 19:48 UTC (permalink / raw)
  To: Rodolfo Giometti; +Cc: linux-kernel

> The pps_event() function does it for you in a beauty-and-clear way by
> using the info.echo() function. :-D

I thought it would increase latency to use info.echo() but I just
measured it and there is no significant difference. I guess it was a
case of premature optimization :)

Cheers,
Lukas

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-02-15 19:48 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-06 15:58 [PATCH] pps-gpio: implement echo pulses Lukas Senger
2018-02-07  9:09 ` Rodolfo Giometti
2018-02-15 12:59   ` Lukas Senger
2018-02-15 12:59     ` [PATCH 1/2] pps-gpio: Avoid flooding syslog Lukas Senger
2018-02-15 13:34       ` Rodolfo Giometti
2018-02-15 15:12         ` Lukas Senger
2018-02-15 15:36           ` Rodolfo Giometti
2018-02-15 19:48             ` Lukas Senger
2018-02-15 12:59     ` [PATCH 2/2] pps-gpio: Set echo GPIO pin via devicetree Lukas Senger
2018-02-15 13:43       ` Rodolfo Giometti
2018-02-15 15:08         ` Lukas Senger
2018-02-15 15:33           ` Rodolfo Giometti

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.