Re: [PATCH v2 3/5] misc: Add w2sg0004 (gps receiver) power control driver

2017-11-15 Thread H. Nikolaus Schaller 
Hi Rob,

> Am 15.11.2017 um 21:17 schrieb Rob Herring :
> 
> On Sun, Nov 12, 2017 at 09:59:56PM +0100, H. Nikolaus Schaller wrote:
>> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
>> 
>> Use serdev API hooks to monitor and forward the UART traffic to /dev/GPSn
>> and turn on/off the module. It also detects if the module is turned on 
>> (sends data)
>> but should be off, e.g. if it was already turned on during boot or 
>> power-on-reset.
>> 
>> Additionally, rfkill block/unblock can be used to control an external LNA
>> (and power down the module if not needed).
>> 
>> The driver concept is based on code developed by NeilBrown 
>> but simplified and adapted to use the new serdev API introduced in 4.11.
>> 
>> Signed-off-by: H. Nikolaus Schaller 
>> ---
>> drivers/misc/Kconfig |  10 +
>> drivers/misc/Makefile|   1 +
>> drivers/misc/w2sg0004.c  | 565 
>> +++
>> include/linux/w2sg0004.h |  27 +++
>> 4 files changed, 603 insertions(+)
>> create mode 100644 drivers/misc/w2sg0004.c
>> create mode 100644 include/linux/w2sg0004.h
>> 
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 8136dc7e863d..09d171d68408 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -518,4 +518,14 @@ source "drivers/misc/mic/Kconfig"
>> source "drivers/misc/genwqe/Kconfig"
>> source "drivers/misc/echo/Kconfig"
>> source "drivers/misc/cxl/Kconfig"
>> +
>> +config W2SG0004
>> +tristate "W2SG00x4 on/off control"
>> +depends on GPIOLIB && SERIAL_DEV_BUS
>> +help
>> +  Enable on/off control of W2SG00x4 GPS moduled connected
>> +  to some SoC UART to allow powering up/down if the /dev/ttyGPSn
>> +  is opened/closed.
>> +  It also provides a rfkill gps name to control the LNA power.
>> +
>> endmenu
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index ad0e64fdba34..abcb667e0ff0 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -51,6 +51,7 @@ obj-$(CONFIG_SRAM_EXEC)+= sram-exec.o
>> obj-y+= mic/
>> obj-$(CONFIG_GENWQE) += genwqe/
>> obj-$(CONFIG_ECHO)   += echo/
>> +obj-$(CONFIG_W2SG0004)  += w2sg0004.o
>> obj-$(CONFIG_VEXPRESS_SYSCFG)+= vexpress-syscfg.o
>> obj-$(CONFIG_CXL_BASE)   += cxl/
>> obj-$(CONFIG_ASPEED_LPC_CTRL)+= aspeed-lpc-ctrl.o
>> diff --git a/drivers/misc/w2sg0004.c b/drivers/misc/w2sg0004.c
>> new file mode 100644
>> index ..55101aa6beeb
>> --- /dev/null
>> +++ b/drivers/misc/w2sg0004.c
>> @@ -0,0 +1,565 @@
>> +/*
>> + * w2sg0004.c
> 
> Listing the filename is pointless IMO.

Ok.

> You need a license too. Use SPDX 
> tag (on first line with C++ style comment).

Well, this is too new to be aware of...
Isn't the MODULE_LICENSE enough any more?

> 
>> + * Driver for power controlling the w2sg0004/w2sg0084 GPS receiver.
>> + *
>> + * This receiver has an ON/OFF pin which must be toggled to
>> + * turn the device 'on' of 'off'.  A high->low->high toggle
>> + * will switch the device on if it is off, and off if it is on.
>> + *
>> + * To enable receiving on/off requests we register with the
>> + * UART power management notifications.
>> + *
>> + * It is not possible to directly detect the state of the device.
>> + * However when it is on it will send characters on a UART line
>> + * regularly.
>> + *
>> + * To detect that the power state is out of sync (e.g. if GPS
>> + * was enabled before a reboot), we register for UART data received
>> + * notifications.
>> + *
>> + * In addition we register as a rfkill client so that we can
>> + * control the LNA power.
>> + *
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/*
>> + * There seems to be restrictions on how quickly we can toggle the
>> + * on/off line.  data sheets says "two rtc ticks", whatever that means.
>> + * If we do it too soon it doesn't work.
>> + * So we have a state machine which uses the common work queue to ensure
>> + * clean transitions.
>> + * When a change is requested we record that request and only act on it
>> + * once the previous change has completed.
>> + * A change involves a 10ms low pulse, and a 990ms raised level, so only
>> + * one change per second.
>> + */
>> +
>> +enum w2sg_state {
>> +W2SG_IDLE,  /* is not changing state */
>> +W2SG_PULSE, /* activate on/off impulse */
>> +W2SG_NOPULSE/* deactivate on/off impulse */
>> +};
>> +
>> +struct w2sg_data {
>> +struct  rfkill *rf_kill;
>> +struct  regulator *lna_regulator;
>> +int lna_blocked;/* rfkill block gps active */
>> +int lna_is_off; /* LNA is currently off

Re: [PATCH v2 3/5] misc: Add w2sg0004 (gps receiver) power control driver

2017-11-15 Thread H. Nikolaus Schaller 
Hi Rob,

> Am 15.11.2017 um 21:17 schrieb Rob Herring :
> 
> On Sun, Nov 12, 2017 at 09:59:56PM +0100, H. Nikolaus Schaller wrote:
>> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
>> 
>> Use serdev API hooks to monitor and forward the UART traffic to /dev/GPSn
>> and turn on/off the module. It also detects if the module is turned on 
>> (sends data)
>> but should be off, e.g. if it was already turned on during boot or 
>> power-on-reset.
>> 
>> Additionally, rfkill block/unblock can be used to control an external LNA
>> (and power down the module if not needed).
>> 
>> The driver concept is based on code developed by NeilBrown 
>> but simplified and adapted to use the new serdev API introduced in 4.11.
>> 
>> Signed-off-by: H. Nikolaus Schaller 
>> ---
>> drivers/misc/Kconfig |  10 +
>> drivers/misc/Makefile|   1 +
>> drivers/misc/w2sg0004.c  | 565 
>> +++
>> include/linux/w2sg0004.h |  27 +++
>> 4 files changed, 603 insertions(+)
>> create mode 100644 drivers/misc/w2sg0004.c
>> create mode 100644 include/linux/w2sg0004.h
>> 
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 8136dc7e863d..09d171d68408 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -518,4 +518,14 @@ source "drivers/misc/mic/Kconfig"
>> source "drivers/misc/genwqe/Kconfig"
>> source "drivers/misc/echo/Kconfig"
>> source "drivers/misc/cxl/Kconfig"
>> +
>> +config W2SG0004
>> +tristate "W2SG00x4 on/off control"
>> +depends on GPIOLIB && SERIAL_DEV_BUS
>> +help
>> +  Enable on/off control of W2SG00x4 GPS moduled connected
>> +  to some SoC UART to allow powering up/down if the /dev/ttyGPSn
>> +  is opened/closed.
>> +  It also provides a rfkill gps name to control the LNA power.
>> +
>> endmenu
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index ad0e64fdba34..abcb667e0ff0 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -51,6 +51,7 @@ obj-$(CONFIG_SRAM_EXEC)+= sram-exec.o
>> obj-y+= mic/
>> obj-$(CONFIG_GENWQE) += genwqe/
>> obj-$(CONFIG_ECHO)   += echo/
>> +obj-$(CONFIG_W2SG0004)  += w2sg0004.o
>> obj-$(CONFIG_VEXPRESS_SYSCFG)+= vexpress-syscfg.o
>> obj-$(CONFIG_CXL_BASE)   += cxl/
>> obj-$(CONFIG_ASPEED_LPC_CTRL)+= aspeed-lpc-ctrl.o
>> diff --git a/drivers/misc/w2sg0004.c b/drivers/misc/w2sg0004.c
>> new file mode 100644
>> index ..55101aa6beeb
>> --- /dev/null
>> +++ b/drivers/misc/w2sg0004.c
>> @@ -0,0 +1,565 @@
>> +/*
>> + * w2sg0004.c
> 
> Listing the filename is pointless IMO.

Ok.

> You need a license too. Use SPDX 
> tag (on first line with C++ style comment).

Well, this is too new to be aware of...
Isn't the MODULE_LICENSE enough any more?

> 
>> + * Driver for power controlling the w2sg0004/w2sg0084 GPS receiver.
>> + *
>> + * This receiver has an ON/OFF pin which must be toggled to
>> + * turn the device 'on' of 'off'.  A high->low->high toggle
>> + * will switch the device on if it is off, and off if it is on.
>> + *
>> + * To enable receiving on/off requests we register with the
>> + * UART power management notifications.
>> + *
>> + * It is not possible to directly detect the state of the device.
>> + * However when it is on it will send characters on a UART line
>> + * regularly.
>> + *
>> + * To detect that the power state is out of sync (e.g. if GPS
>> + * was enabled before a reboot), we register for UART data received
>> + * notifications.
>> + *
>> + * In addition we register as a rfkill client so that we can
>> + * control the LNA power.
>> + *
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/*
>> + * There seems to be restrictions on how quickly we can toggle the
>> + * on/off line.  data sheets says "two rtc ticks", whatever that means.
>> + * If we do it too soon it doesn't work.
>> + * So we have a state machine which uses the common work queue to ensure
>> + * clean transitions.
>> + * When a change is requested we record that request and only act on it
>> + * once the previous change has completed.
>> + * A change involves a 10ms low pulse, and a 990ms raised level, so only
>> + * one change per second.
>> + */
>> +
>> +enum w2sg_state {
>> +W2SG_IDLE,  /* is not changing state */
>> +W2SG_PULSE, /* activate on/off impulse */
>> +W2SG_NOPULSE/* deactivate on/off impulse */
>> +};
>> +
>> +struct w2sg_data {
>> +struct  rfkill *rf_kill;
>> +struct  regulator *lna_regulator;
>> +int lna_blocked;/* rfkill block gps active */
>> +int lna_is_off; /* LNA is currently off */
>> +int is_on;  /* current

Re: [PATCH v2 3/5] misc: Add w2sg0004 (gps receiver) power control driver

2017-11-15 Thread Rob Herring 
On Sun, Nov 12, 2017 at 09:59:56PM +0100, H. Nikolaus Schaller wrote:
> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
> 
> Use serdev API hooks to monitor and forward the UART traffic to /dev/GPSn
> and turn on/off the module. It also detects if the module is turned on (sends 
> data)
> but should be off, e.g. if it was already turned on during boot or 
> power-on-reset.
> 
> Additionally, rfkill block/unblock can be used to control an external LNA
> (and power down the module if not needed).
> 
> The driver concept is based on code developed by NeilBrown 
> but simplified and adapted to use the new serdev API introduced in 4.11.
> 
> Signed-off-by: H. Nikolaus Schaller 
> ---
>  drivers/misc/Kconfig |  10 +
>  drivers/misc/Makefile|   1 +
>  drivers/misc/w2sg0004.c  | 565 
> +++
>  include/linux/w2sg0004.h |  27 +++
>  4 files changed, 603 insertions(+)
>  create mode 100644 drivers/misc/w2sg0004.c
>  create mode 100644 include/linux/w2sg0004.h
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 8136dc7e863d..09d171d68408 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -518,4 +518,14 @@ source "drivers/misc/mic/Kconfig"
>  source "drivers/misc/genwqe/Kconfig"
>  source "drivers/misc/echo/Kconfig"
>  source "drivers/misc/cxl/Kconfig"
> +
> +config W2SG0004
> + tristate "W2SG00x4 on/off control"
> + depends on GPIOLIB && SERIAL_DEV_BUS
> + help
> +  Enable on/off control of W2SG00x4 GPS moduled connected
> +   to some SoC UART to allow powering up/down if the /dev/ttyGPSn
> +   is opened/closed.
> +   It also provides a rfkill gps name to control the LNA power.
> +
>  endmenu
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index ad0e64fdba34..abcb667e0ff0 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -51,6 +51,7 @@ obj-$(CONFIG_SRAM_EXEC) += sram-exec.o
>  obj-y+= mic/
>  obj-$(CONFIG_GENWQE) += genwqe/
>  obj-$(CONFIG_ECHO)   += echo/
> +obj-$(CONFIG_W2SG0004)   += w2sg0004.o
>  obj-$(CONFIG_VEXPRESS_SYSCFG)+= vexpress-syscfg.o
>  obj-$(CONFIG_CXL_BASE)   += cxl/
>  obj-$(CONFIG_ASPEED_LPC_CTRL)+= aspeed-lpc-ctrl.o
> diff --git a/drivers/misc/w2sg0004.c b/drivers/misc/w2sg0004.c
> new file mode 100644
> index ..55101aa6beeb
> --- /dev/null
> +++ b/drivers/misc/w2sg0004.c
> @@ -0,0 +1,565 @@
> +/*
> + * w2sg0004.c

Listing the filename is pointless IMO. You need a license too. Use SPDX 
tag (on first line with C++ style comment).

> + * Driver for power controlling the w2sg0004/w2sg0084 GPS receiver.
> + *
> + * This receiver has an ON/OFF pin which must be toggled to
> + * turn the device 'on' of 'off'.  A high->low->high toggle
> + * will switch the device on if it is off, and off if it is on.
> + *
> + * To enable receiving on/off requests we register with the
> + * UART power management notifications.
> + *
> + * It is not possible to directly detect the state of the device.
> + * However when it is on it will send characters on a UART line
> + * regularly.
> + *
> + * To detect that the power state is out of sync (e.g. if GPS
> + * was enabled before a reboot), we register for UART data received
> + * notifications.
> + *
> + * In addition we register as a rfkill client so that we can
> + * control the LNA power.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * There seems to be restrictions on how quickly we can toggle the
> + * on/off line.  data sheets says "two rtc ticks", whatever that means.
> + * If we do it too soon it doesn't work.
> + * So we have a state machine which uses the common work queue to ensure
> + * clean transitions.
> + * When a change is requested we record that request and only act on it
> + * once the previous change has completed.
> + * A change involves a 10ms low pulse, and a 990ms raised level, so only
> + * one change per second.
> + */
> +
> +enum w2sg_state {
> + W2SG_IDLE,  /* is not changing state */
> + W2SG_PULSE, /* activate on/off impulse */
> + W2SG_NOPULSE/* deactivate on/off impulse */
> +};
> +
> +struct w2sg_data {
> + struct  rfkill *rf_kill;
> + struct  regulator *lna_regulator;
> + int lna_blocked;/* rfkill block gps active */
> + int lna_is_off; /* LNA is currently off */
> + int is_on;  /* current state (0/1) */
> + unsigned long   last_toggle;
> + unsigned long   backoff;/* time to wait since last_toggle */
> + int on_off_gpio;/* the on-off gpio number */
> + struct

Re: [PATCH v2 3/5] misc: Add w2sg0004 (gps receiver) power control driver

2017-11-15 Thread Rob Herring 
On Sun, Nov 12, 2017 at 09:59:56PM +0100, H. Nikolaus Schaller wrote:
> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
> 
> Use serdev API hooks to monitor and forward the UART traffic to /dev/GPSn
> and turn on/off the module. It also detects if the module is turned on (sends 
> data)
> but should be off, e.g. if it was already turned on during boot or 
> power-on-reset.
> 
> Additionally, rfkill block/unblock can be used to control an external LNA
> (and power down the module if not needed).
> 
> The driver concept is based on code developed by NeilBrown 
> but simplified and adapted to use the new serdev API introduced in 4.11.
> 
> Signed-off-by: H. Nikolaus Schaller 
> ---
>  drivers/misc/Kconfig |  10 +
>  drivers/misc/Makefile|   1 +
>  drivers/misc/w2sg0004.c  | 565 
> +++
>  include/linux/w2sg0004.h |  27 +++
>  4 files changed, 603 insertions(+)
>  create mode 100644 drivers/misc/w2sg0004.c
>  create mode 100644 include/linux/w2sg0004.h
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 8136dc7e863d..09d171d68408 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -518,4 +518,14 @@ source "drivers/misc/mic/Kconfig"
>  source "drivers/misc/genwqe/Kconfig"
>  source "drivers/misc/echo/Kconfig"
>  source "drivers/misc/cxl/Kconfig"
> +
> +config W2SG0004
> + tristate "W2SG00x4 on/off control"
> + depends on GPIOLIB && SERIAL_DEV_BUS
> + help
> +  Enable on/off control of W2SG00x4 GPS moduled connected
> +   to some SoC UART to allow powering up/down if the /dev/ttyGPSn
> +   is opened/closed.
> +   It also provides a rfkill gps name to control the LNA power.
> +
>  endmenu
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index ad0e64fdba34..abcb667e0ff0 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -51,6 +51,7 @@ obj-$(CONFIG_SRAM_EXEC) += sram-exec.o
>  obj-y+= mic/
>  obj-$(CONFIG_GENWQE) += genwqe/
>  obj-$(CONFIG_ECHO)   += echo/
> +obj-$(CONFIG_W2SG0004)   += w2sg0004.o
>  obj-$(CONFIG_VEXPRESS_SYSCFG)+= vexpress-syscfg.o
>  obj-$(CONFIG_CXL_BASE)   += cxl/
>  obj-$(CONFIG_ASPEED_LPC_CTRL)+= aspeed-lpc-ctrl.o
> diff --git a/drivers/misc/w2sg0004.c b/drivers/misc/w2sg0004.c
> new file mode 100644
> index ..55101aa6beeb
> --- /dev/null
> +++ b/drivers/misc/w2sg0004.c
> @@ -0,0 +1,565 @@
> +/*
> + * w2sg0004.c

Listing the filename is pointless IMO. You need a license too. Use SPDX 
tag (on first line with C++ style comment).

> + * Driver for power controlling the w2sg0004/w2sg0084 GPS receiver.
> + *
> + * This receiver has an ON/OFF pin which must be toggled to
> + * turn the device 'on' of 'off'.  A high->low->high toggle
> + * will switch the device on if it is off, and off if it is on.
> + *
> + * To enable receiving on/off requests we register with the
> + * UART power management notifications.
> + *
> + * It is not possible to directly detect the state of the device.
> + * However when it is on it will send characters on a UART line
> + * regularly.
> + *
> + * To detect that the power state is out of sync (e.g. if GPS
> + * was enabled before a reboot), we register for UART data received
> + * notifications.
> + *
> + * In addition we register as a rfkill client so that we can
> + * control the LNA power.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * There seems to be restrictions on how quickly we can toggle the
> + * on/off line.  data sheets says "two rtc ticks", whatever that means.
> + * If we do it too soon it doesn't work.
> + * So we have a state machine which uses the common work queue to ensure
> + * clean transitions.
> + * When a change is requested we record that request and only act on it
> + * once the previous change has completed.
> + * A change involves a 10ms low pulse, and a 990ms raised level, so only
> + * one change per second.
> + */
> +
> +enum w2sg_state {
> + W2SG_IDLE,  /* is not changing state */
> + W2SG_PULSE, /* activate on/off impulse */
> + W2SG_NOPULSE/* deactivate on/off impulse */
> +};
> +
> +struct w2sg_data {
> + struct  rfkill *rf_kill;
> + struct  regulator *lna_regulator;
> + int lna_blocked;/* rfkill block gps active */
> + int lna_is_off; /* LNA is currently off */
> + int is_on;  /* current state (0/1) */
> + unsigned long   last_toggle;
> + unsigned long   backoff;/* time to wait since last_toggle */
> + int on_off_gpio;/* the on-off gpio number */
> + struct  serdev_device *uart;/* uart

Re: [PATCH v2 3/5] misc: Add w2sg0004 (gps receiver) power control driver

2017-11-15 Thread kbuild test robot 
Hi Nikolaus,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on v4.14 next-20171115]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/H-Nikolaus-Schaller/misc-new-serdev-based-drivers-for-Wi2Wi-w2sg00x4-GPS-module/20171115-115158
config: tile-allmodconfig (attached as .config)
compiler: tilegx-linux-gcc (GCC) 4.6.2
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=tile 

All warnings (new ones prefixed by >>):

   drivers/misc/w2sg0004.c: In function 'w2sg_uart_receive_buf':
>> drivers/misc/w2sg0004.c:156:3: warning: format '%d' expects argument of type 
>> 'int', but argument 3 has type 'size_t' [-Wformat]
   drivers/misc/w2sg0004.c:161:4: warning: format '%d' expects argument of type 
'int', but argument 2 has type 'size_t' [-Wformat]
   drivers/misc/w2sg0004.c: At top level:
   drivers/misc/w2sg0004.c:487:12: warning: 'w2sg_suspend' defined but not used 
[-Wunused-function]
   drivers/misc/w2sg0004.c:523:12: warning: 'w2sg_resume' defined but not used 
[-Wunused-function]

vim +156 drivers/misc/w2sg0004.c

   125  
   126  static int w2sg_uart_receive_buf(struct serdev_device *serdev,
   127  const unsigned char *rxdata,
   128  size_t count)
   129  {
   130  struct w2sg_data *data =
   131  (struct w2sg_data *) serdev_device_get_drvdata(serdev);
   132  
   133  if (!data->requested && !data->is_on) {
   134  /*
   135   * we have received characters while the w2sg
   136   * should have been be turned off
   137   */
   138  data->discard_count += count;
   139  if ((data->state == W2SG_IDLE) &&
   140  time_after(jiffies,
   141  data->last_toggle + data->backoff)) {
   142  /* Should be off by now, time to toggle again */
   143  pr_debug("w2sg00x4 has sent %d characters data 
although it should be off!\n",
   144  data->discard_count);
   145  
   146  data->discard_count = 0;
   147  
   148  data->is_on = true;
   149  data->backoff *= 2;
   150  if (!data->suspended)
   151  schedule_delayed_work(>work, 0);
   152  }
   153  } else if (data->open_count > 0) {
   154  int n;
   155  
 > 156  pr_debug("w2sg00x4: push %d chars to tty port\n", 
 > count);
   157  
   158  /* pass to user-space */
   159  n = tty_insert_flip_string(>port, rxdata, count);
   160  if (n != count)
   161  pr_err("w2sg00x4: did loose %d characters\n", 
count - n);
   162  tty_flip_buffer_push(>port);
   163  return n;
   164  }
   165  
   166  /* assume we have processed everything */
   167  return count;
   168  }
   169  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v2 3/5] misc: Add w2sg0004 (gps receiver) power control driver

2017-11-15 Thread kbuild test robot 
Hi Nikolaus,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on v4.14 next-20171115]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/H-Nikolaus-Schaller/misc-new-serdev-based-drivers-for-Wi2Wi-w2sg00x4-GPS-module/20171115-115158
config: tile-allmodconfig (attached as .config)
compiler: tilegx-linux-gcc (GCC) 4.6.2
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=tile 

All warnings (new ones prefixed by >>):

   drivers/misc/w2sg0004.c: In function 'w2sg_uart_receive_buf':
>> drivers/misc/w2sg0004.c:156:3: warning: format '%d' expects argument of type 
>> 'int', but argument 3 has type 'size_t' [-Wformat]
   drivers/misc/w2sg0004.c:161:4: warning: format '%d' expects argument of type 
'int', but argument 2 has type 'size_t' [-Wformat]
   drivers/misc/w2sg0004.c: At top level:
   drivers/misc/w2sg0004.c:487:12: warning: 'w2sg_suspend' defined but not used 
[-Wunused-function]
   drivers/misc/w2sg0004.c:523:12: warning: 'w2sg_resume' defined but not used 
[-Wunused-function]

vim +156 drivers/misc/w2sg0004.c

   125  
   126  static int w2sg_uart_receive_buf(struct serdev_device *serdev,
   127  const unsigned char *rxdata,
   128  size_t count)
   129  {
   130  struct w2sg_data *data =
   131  (struct w2sg_data *) serdev_device_get_drvdata(serdev);
   132  
   133  if (!data->requested && !data->is_on) {
   134  /*
   135   * we have received characters while the w2sg
   136   * should have been be turned off
   137   */
   138  data->discard_count += count;
   139  if ((data->state == W2SG_IDLE) &&
   140  time_after(jiffies,
   141  data->last_toggle + data->backoff)) {
   142  /* Should be off by now, time to toggle again */
   143  pr_debug("w2sg00x4 has sent %d characters data 
although it should be off!\n",
   144  data->discard_count);
   145  
   146  data->discard_count = 0;
   147  
   148  data->is_on = true;
   149  data->backoff *= 2;
   150  if (!data->suspended)
   151  schedule_delayed_work(>work, 0);
   152  }
   153  } else if (data->open_count > 0) {
   154  int n;
   155  
 > 156  pr_debug("w2sg00x4: push %d chars to tty port\n", 
 > count);
   157  
   158  /* pass to user-space */
   159  n = tty_insert_flip_string(>port, rxdata, count);
   160  if (n != count)
   161  pr_err("w2sg00x4: did loose %d characters\n", 
count - n);
   162  tty_flip_buffer_push(>port);
   163  return n;
   164  }
   165  
   166  /* assume we have processed everything */
   167  return count;
   168  }
   169  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v2 3/5] misc: Add w2sg0004 (gps receiver) power control driver

2017-11-15 Thread kbuild test robot 
Hi Nikolaus,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on v4.14]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/H-Nikolaus-Schaller/misc-new-serdev-based-drivers-for-Wi2Wi-w2sg00x4-GPS-module/20171115-115158
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:13:0,
from include/linux/delay.h:21,
from drivers/misc/w2sg0004.c:25:
   drivers/misc/w2sg0004.c: In function 'w2sg_uart_receive_buf':
>> drivers/misc/w2sg0004.c:156:12: warning: format '%d' expects argument of 
>> type 'int', but argument 3 has type 'size_t {aka long unsigned int}' 
>> [-Wformat=]
  pr_debug("w2sg00x4: push %d chars to tty port\n", count);
   ^
   include/linux/printk.h:285:21: note: in definition of macro 'pr_fmt'
#define pr_fmt(fmt) fmt
^~~
   include/linux/printk.h:333:2: note: in expansion of macro 'dynamic_pr_debug'
 dynamic_pr_debug(fmt, ##__VA_ARGS__)
 ^~~~
>> drivers/misc/w2sg0004.c:156:3: note: in expansion of macro 'pr_debug'
  pr_debug("w2sg00x4: push %d chars to tty port\n", count);
  ^~~~
   In file included from include/linux/printk.h:6:0,
from include/linux/kernel.h:13,
from include/linux/delay.h:21,
from drivers/misc/w2sg0004.c:25:
   include/linux/kern_levels.h:4:18: warning: format '%d' expects argument of 
type 'int', but argument 2 has type 'size_t {aka long unsigned int}' [-Wformat=]
#define KERN_SOH "\001"  /* ASCII Start Of Header */
 ^
   include/linux/kern_levels.h:10:18: note: in expansion of macro 'KERN_SOH'
#define KERN_ERR KERN_SOH "3" /* error conditions */
 ^~~~
   include/linux/printk.h:301:9: note: in expansion of macro 'KERN_ERR'
 printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~
>> drivers/misc/w2sg0004.c:161:4: note: in expansion of macro 'pr_err'
   pr_err("w2sg00x4: did loose %d characters\n", count - n);
   ^~

vim +156 drivers/misc/w2sg0004.c

  > 25  #include 
26  #include 
27  #include 
28  #include 
29  #include 
30  #include 
31  #include 
32  #include 
33  #include 
34  #include 
35  #include 
36  #include 
37  #include 
38  #include 
39  #include 
40  #include 
41  #include 
42  
43  /*
44   * There seems to be restrictions on how quickly we can toggle the
45   * on/off line.  data sheets says "two rtc ticks", whatever that means.
46   * If we do it too soon it doesn't work.
47   * So we have a state machine which uses the common work queue to ensure
48   * clean transitions.
49   * When a change is requested we record that request and only act on it
50   * once the previous change has completed.
51   * A change involves a 10ms low pulse, and a 990ms raised level, so only
52   * one change per second.
53   */
54  
55  enum w2sg_state {
56  W2SG_IDLE,  /* is not changing state */
57  W2SG_PULSE, /* activate on/off impulse */
58  W2SG_NOPULSE/* deactivate on/off impulse */
59  };
60  
61  struct w2sg_data {
62  struct  rfkill *rf_kill;
63  struct  regulator *lna_regulator;
64  int lna_blocked;/* rfkill block gps active */
65  int lna_is_off; /* LNA is currently off */
66  int is_on;  /* current state (0/1) */
67  unsigned long   last_toggle;
68  unsigned long   backoff;/* time to wait since 
last_toggle */
69  int on_off_gpio;/* the on-off gpio number */
70  struct  serdev_device *uart;/* uart connected to 
the chip */
71  struct  tty_driver *tty_drv;/* this is the user 
space tty */
72  struct  device *dev;/* from 
tty_port_register_device() */
73  struct  tty_port port;
74  int open_count; /* how often we were opened */
75  enumw2sg_state state;
76  int requested;  /* requested state (0/1) */
77  int suspended;
78  struct delayed_work work;
79  int discard_count;
80  };
81  
82  static struct w2sg_data *w2sg_by_minor[1];
83  
84  static int w2sg_set_lna_power(struct w2sg_data *data)
85  {
86

Re: [PATCH v2 3/5] misc: Add w2sg0004 (gps receiver) power control driver

2017-11-15 Thread kbuild test robot 
Hi Nikolaus,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on v4.14]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/H-Nikolaus-Schaller/misc-new-serdev-based-drivers-for-Wi2Wi-w2sg00x4-GPS-module/20171115-115158
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:13:0,
from include/linux/delay.h:21,
from drivers/misc/w2sg0004.c:25:
   drivers/misc/w2sg0004.c: In function 'w2sg_uart_receive_buf':
>> drivers/misc/w2sg0004.c:156:12: warning: format '%d' expects argument of 
>> type 'int', but argument 3 has type 'size_t {aka long unsigned int}' 
>> [-Wformat=]
  pr_debug("w2sg00x4: push %d chars to tty port\n", count);
   ^
   include/linux/printk.h:285:21: note: in definition of macro 'pr_fmt'
#define pr_fmt(fmt) fmt
^~~
   include/linux/printk.h:333:2: note: in expansion of macro 'dynamic_pr_debug'
 dynamic_pr_debug(fmt, ##__VA_ARGS__)
 ^~~~
>> drivers/misc/w2sg0004.c:156:3: note: in expansion of macro 'pr_debug'
  pr_debug("w2sg00x4: push %d chars to tty port\n", count);
  ^~~~
   In file included from include/linux/printk.h:6:0,
from include/linux/kernel.h:13,
from include/linux/delay.h:21,
from drivers/misc/w2sg0004.c:25:
   include/linux/kern_levels.h:4:18: warning: format '%d' expects argument of 
type 'int', but argument 2 has type 'size_t {aka long unsigned int}' [-Wformat=]
#define KERN_SOH "\001"  /* ASCII Start Of Header */
 ^
   include/linux/kern_levels.h:10:18: note: in expansion of macro 'KERN_SOH'
#define KERN_ERR KERN_SOH "3" /* error conditions */
 ^~~~
   include/linux/printk.h:301:9: note: in expansion of macro 'KERN_ERR'
 printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~
>> drivers/misc/w2sg0004.c:161:4: note: in expansion of macro 'pr_err'
   pr_err("w2sg00x4: did loose %d characters\n", count - n);
   ^~

vim +156 drivers/misc/w2sg0004.c

  > 25  #include 
26  #include 
27  #include 
28  #include 
29  #include 
30  #include 
31  #include 
32  #include 
33  #include 
34  #include 
35  #include 
36  #include 
37  #include 
38  #include 
39  #include 
40  #include 
41  #include 
42  
43  /*
44   * There seems to be restrictions on how quickly we can toggle the
45   * on/off line.  data sheets says "two rtc ticks", whatever that means.
46   * If we do it too soon it doesn't work.
47   * So we have a state machine which uses the common work queue to ensure
48   * clean transitions.
49   * When a change is requested we record that request and only act on it
50   * once the previous change has completed.
51   * A change involves a 10ms low pulse, and a 990ms raised level, so only
52   * one change per second.
53   */
54  
55  enum w2sg_state {
56  W2SG_IDLE,  /* is not changing state */
57  W2SG_PULSE, /* activate on/off impulse */
58  W2SG_NOPULSE/* deactivate on/off impulse */
59  };
60  
61  struct w2sg_data {
62  struct  rfkill *rf_kill;
63  struct  regulator *lna_regulator;
64  int lna_blocked;/* rfkill block gps active */
65  int lna_is_off; /* LNA is currently off */
66  int is_on;  /* current state (0/1) */
67  unsigned long   last_toggle;
68  unsigned long   backoff;/* time to wait since 
last_toggle */
69  int on_off_gpio;/* the on-off gpio number */
70  struct  serdev_device *uart;/* uart connected to 
the chip */
71  struct  tty_driver *tty_drv;/* this is the user 
space tty */
72  struct  device *dev;/* from 
tty_port_register_device() */
73  struct  tty_port port;
74  int open_count; /* how often we were opened */
75  enumw2sg_state state;
76  int requested;  /* requested state (0/1) */
77  int suspended;
78  struct delayed_work work;
79  int discard_count;
80  };
81  
82  static struct w2sg_data *w2sg_by_minor[1];
83  
84  static int w2sg_set_lna_power(struct w2sg_data *data)
85  {
86