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

[kbuild test robot <lkp@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 7687 bytes --]

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 <linux/delay.h>
    26	#include <linux/err.h>
    27	#include <linux/interrupt.h>
    28	#include <linux/irq.h>
    29	#include <linux/module.h>
    30	#include <linux/of.h>
    31	#include <linux/of_irq.h>
    32	#include <linux/of_gpio.h>
    33	#include <linux/platform_device.h>
    34	#include <linux/rfkill.h>
    35	#include <linux/serdev.h>
    36	#include <linux/sched.h>
    37	#include <linux/slab.h>
    38	#include <linux/tty.h>
    39	#include <linux/tty_flip.h>
    40	#include <linux/w2sg0004.h>
    41	#include <linux/workqueue.h>
    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		enum		w2sg_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		int ret = 0;
    87		int off = data->suspended || !data->requested || data->lna_blocked;
    88	
    89		pr_debug("%s: %s\n", __func__, off ? "off" : "on");
    90	
    91		if (off != data->lna_is_off) {
    92			data->lna_is_off = off;
    93			if (!IS_ERR_OR_NULL(data->lna_regulator)) {
    94				if (off)
    95					regulator_disable(data->lna_regulator);
    96				else
    97					ret = regulator_enable(data->lna_regulator);
    98			}
    99		}
   100	
   101		return ret;
   102	}
   103	
   104	static void w2sg_set_power(void *pdata, int val)
   105	{
   106		struct w2sg_data *data = (struct w2sg_data *) pdata;
   107	
   108		pr_debug("%s to state=%d (requested=%d)\n", __func__, val, data->requested);
   109	
   110		if (val && !data->requested) {
   111			data->requested = true;
   112		} else if (!val && data->requested) {
   113			data->backoff = HZ;
   114			data->requested = false;
   115		} else
   116			return;
   117	
   118		pr_debug("w2sg00x4 scheduled for %d\n", data->requested);
   119	
   120		if (!data->suspended)
   121			schedule_delayed_work(&data->work, 0);
   122	}
   123	
   124	/* called each time data is received by the UART (i.e. sent by the w2sg0004) */
   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(&data->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(&data->port, rxdata, count);
   160			if (n != count)
 > 161				pr_err("w2sg00x4: did loose %d characters\n", count - n);
   162			tty_flip_buffer_push(&data->port);
   163			return n;
   164		}
   165	
   166		/* assume we have processed everything */
   167		return count;
   168	}
   169	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 61726 bytes --]