by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH linux 3/4] drivers/fsi: Add On-Chip Controller (OCC) driver

Hi Edward,

[auto build test ERROR on next-20170619]
[cannot apply to linux/master linus/master robh/for-next v4.12-rc6 v4.12-rc5 v4.12-rc4 v4.12-rc6]
[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/Eddie-James/drivers-fsi-Add-SBEFIFO-and-OCC-drivers/20170623-160949
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 6.3.0-18) 6.3.0 20170516
reproduce:
wget https://raw.githubusercontent.com/01org/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=sh

All error/warnings (new ones prefixed by >>):

drivers//fsi/occ.c: In function 'occ_open':
>> drivers//fsi/occ.c:136:32: error: dereferencing pointer to incomplete type 'struct file'
struct miscdevice *mdev = file->private_data;
^~
>> drivers//fsi/occ.c:147:22: error: 'O_NONBLOCK' undeclared (first use in this function)
if (file->f_flags & O_NONBLOCK)
^~~~~~~~~~
drivers//fsi/occ.c:147:22: note: each undeclared identifier is reported only once for each function it appears in
drivers//fsi/occ.c: At top level:
>> drivers//fsi/occ.c:324:21: error: variable 'occ_fops' has initializer but incomplete type
static const struct file_operations occ_fops = {
^~~~~~~~~~~~~~~
>> drivers//fsi/occ.c:325:2: error: unknown field 'owner' specified in initializer
.owner = THIS_MODULE,
^
In file included from include/linux/linkage.h:6:0,
from include/linux/kernel.h:6,
from include/linux/unaligned/packed_struct.h:4,
from include/linux/unaligned/le_struct.h:4,
from include/asm-generic/unaligned.h:17,
from arch/sh/include/asm/unaligned.h:9,
from drivers//fsi/occ.c:10:
include/linux/export.h:35:21: warning: excess elements in struct initializer
#define THIS_MODULE (&__this_module)
^
>> drivers//fsi/occ.c:325:11: note: in expansion of macro 'THIS_MODULE'
.owner = THIS_MODULE,
^~~~~~~~~~~
include/linux/export.h:35:21: note: (near initialization for 'occ_fops')
#define THIS_MODULE (&__this_module)
^
>> drivers//fsi/occ.c:325:11: note: in expansion of macro 'THIS_MODULE'
.owner = THIS_MODULE,
^~~~~~~~~~~
>> drivers//fsi/occ.c:326:2: error: unknown field 'open' specified in initializer
.open = occ_open,
^
>> drivers//fsi/occ.c:326:10: warning: excess elements in struct initializer
.open = occ_open,
^~~~~~~~
drivers//fsi/occ.c:326:10: note: (near initialization for 'occ_fops')
>> drivers//fsi/occ.c:327:2: error: unknown field 'read' specified in initializer
.read = occ_read,
^
drivers//fsi/occ.c:327:10: warning: excess elements in struct initializer
.read = occ_read,
^~~~~~~~
drivers//fsi/occ.c:327:10: note: (near initialization for 'occ_fops')
>> drivers//fsi/occ.c:328:2: error: unknown field 'write' specified in initializer
.write = occ_write,
^
drivers//fsi/occ.c:328:11: warning: excess elements in struct initializer
.write = occ_write,
^~~~~~~~~
drivers//fsi/occ.c:328:11: note: (near initialization for 'occ_fops')
>> drivers//fsi/occ.c:329:2: error: unknown field 'release' specified in initializer
.release = occ_release,
^
drivers//fsi/occ.c:329:13: warning: excess elements in struct initializer
.release = occ_release,
^~~~~~~~~~~
drivers//fsi/occ.c:329:13: note: (near initialization for 'occ_fops')
>> drivers//fsi/occ.c:324:37: error: storage size of 'occ_fops' isn't known
static const struct file_operations occ_fops = {
^~~~~~~~

vim +/O_NONBLOCK +147 drivers//fsi/occ.c

4 * This program is free software; you can redistribute it and/or modify
5 * it under the terms of the GNU General Public License as published by
6 * the Free Software Foundation; either version 2 of the License, or
7 * (at your option) any later version.
8 */
9
> 10 #include <asm/unaligned.h>
11 #include <linux/device.h>
12 #include <linux/err.h>
13 #include <linux/fsi-sbefifo.h>
14 #include <linux/init.h>
15 #include <linux/kernel.h>
16 #include <linux/miscdevice.h>
17 #include <linux/module.h>
18 #include <linux/mutex.h>
19 #include <linux/of.h>
20 #include <linux/of_platform.h>
21 #include <linux/platform_device.h>
22 #include <linux/slab.h>
23 #include <linux/uaccess.h>
24 #include <linux/wait.h>
25 #include <linux/workqueue.h>
26
27 #define OCC_SRAM_BYTES 4096
28 #define OCC_CMD_DATA_BYTES 4090
29 #define OCC_RESP_DATA_BYTES 4089
30
31 struct occ {
32 struct device *sbefifo;
33 char name[32];
34 int idx;
35 struct miscdevice mdev;
36 struct list_head xfrs;
37 spinlock_t list_lock;
38 struct mutex occ_lock;
39 struct work_struct work;
40 };
41
42 #define to_occ(x) container_of((x), struct occ, mdev)
43
44 struct occ_command {
45 u8 seq_no;
46 u8 cmd_type;
47 u16 data_length;
48 u8 data[OCC_CMD_DATA_BYTES];
49 u16 checksum;
50 } __packed;
51
52 struct occ_response {
53 u8 seq_no;
54 u8 cmd_type;
55 u8 return_status;
56 u16 data_length;
57 u8 data[OCC_RESP_DATA_BYTES];
58 u16 checksum;
59 } __packed;
60
61 /*
62 * transfer flags are NOT mutually exclusive
63 *
64 * Initial flags are none; transfer is created and queued from write(). All
65 * flags are cleared when the transfer is completed by closing the file or
66 * reading all of the available response data.
67 * XFR_IN_PROGRESS is set when a transfer is started from occ_worker_putsram,
68 * and cleared if the transfer fails or occ_worker_getsram completes.
69 * XFR_COMPLETE is set when a transfer fails or finishes occ_worker_getsram.
70 * XFR_CANCELED is set when the transfer's client is released.
71 * XFR_WAITING is set from read() if the transfer isn't complete and
72 * O_NONBLOCK wasn't specified. Cleared in read() when transfer completes or
73 * fails.
74 */
75 enum {
76 XFR_IN_PROGRESS,
77 XFR_COMPLETE,
78 XFR_CANCELED,
79 XFR_WAITING,
80 };
81
82 struct occ_xfr {
83 struct list_head link;
84 int rc;
85 u8 buf[OCC_SRAM_BYTES];
86 size_t cmd_data_length;
87 size_t resp_data_length;
88 unsigned long flags;
89 };
90
91 /*
92 * client flags
93 *
94 * CLIENT_NONBLOCKING is set during open() if the file was opened with the
95 * O_NONBLOCK flag.
96 * CLIENT_XFR_PENDING is set during write() and cleared when all data has been
97 * read.
98 */
99 enum {
100 CLIENT_NONBLOCKING,
101 CLIENT_XFR_PENDING,
102 };
103
104 struct occ_client {
105 struct occ *occ;
106 struct occ_xfr xfr;
107 spinlock_t lock;
108 wait_queue_head_t wait;
109 size_t read_offset;
110 unsigned long flags;
111 };
112
113 #define to_client(x) container_of((x), struct occ_client, xfr)
114
115 static struct workqueue_struct *occ_wq;
116
117 static DEFINE_IDA(occ_ida);
118
119 static void occ_enqueue_xfr(struct occ_xfr *xfr)
120 {
121 int empty;
122 struct occ_client *client = to_client(xfr);
123 struct occ *occ = client->occ;
124
125 spin_lock_irq(&occ->list_lock);
126 empty = list_empty(&occ->xfrs);
127 list_add_tail(&xfr->link, &occ->xfrs);
128 spin_unlock(&occ->list_lock);
129
130 if (empty)
131 queue_work(occ_wq, &occ->work);
132 }
133
134 static int occ_open(struct inode *inode, struct file *file)
135 {
> 136 struct miscdevice *mdev = file->private_data;
137 struct occ *occ = to_occ(mdev);
138 struct occ_client *client = kzalloc(sizeof(*client), GFP_KERNEL);
139
140 if (!client)
141 return -ENOMEM;
142
143 client->occ = occ;
144 spin_lock_init(&client->lock);
145 init_waitqueue_head(&client->wait);
146
> 147 if (file->f_flags & O_NONBLOCK)
148 set_bit(CLIENT_NONBLOCKING, &client->flags);
149
150 file->private_data = client;
151
152 return 0;
153 }
154
155 static ssize_t occ_read(struct file *file, char __user *buf, size_t len,
156 loff_t *offset)
157 {
158 int rc;
159 size_t bytes;
160 struct occ_client *client = file->private_data;
161 struct occ_xfr *xfr = &client->xfr;
162
163 if (len > OCC_SRAM_BYTES)
164 return -EINVAL;
165
166 spin_lock_irq(&client->lock);
167 if (!test_bit(CLIENT_XFR_PENDING, &client->flags)) {
168 /* we just finished reading all data, return 0 */
169 if (client->read_offset) {
170 rc = 0;
171 client->read_offset = 0;
172 } else
173 rc = -ENOMSG;
174
175 goto done;
176 }
177
178 if (!test_bit(XFR_COMPLETE, &xfr->flags)) {
179 if (test_bit(CLIENT_NONBLOCKING, &client->flags)) {
180 rc = -EAGAIN;
181 goto done;
182 }
183
184 set_bit(XFR_WAITING, &xfr->flags);
185 spin_unlock(&client->lock);
186
187 rc = wait_event_interruptible(client->wait,
188 test_bit(XFR_COMPLETE, &xfr->flags) ||
189 test_bit(XFR_CANCELED, &xfr->flags));
190
191 spin_lock_irq(&client->lock);
192 if (test_bit(XFR_CANCELED, &xfr->flags)) {
193 spin_unlock(&client->lock);
194 kfree(client);
195 return -EBADFD;
196 }
197
198 clear_bit(XFR_WAITING, &xfr->flags);
199 if (!test_bit(XFR_COMPLETE, &xfr->flags)) {
200 rc = -EINTR;
201 goto done;
202 }
203 }
204
205 if (xfr->rc) {
206 rc = xfr->rc;
207 goto done;
208 }
209
210 if (copy_to_user(buf, &xfr->buf[client->read_offset], bytes)) {
211 rc = -EFAULT;
212 goto done;
213 }
214
215 client->read_offset += bytes;
216
217 /* xfr done */
218 if (client->read_offset == xfr->resp_data_length)
219 clear_bit(CLIENT_XFR_PENDING, &client->flags);
220
221 rc = bytes;
222
223 done:
224 spin_unlock(&client->lock);
225 return rc;
226 }
227
228 static ssize_t occ_write(struct file *file, const char __user *buf,
229 size_t len, loff_t *offset)
230 {
231 int rc;
232 unsigned int i;
233 u16 data_length, checksum = 0;
234 struct occ_client *client = file->private_data;
235 struct occ_xfr *xfr = &client->xfr;
236
237 if (len > (OCC_CMD_DATA_BYTES + 3) || len < 3)
238 return -EINVAL;
239
240 spin_lock_irq(&client->lock);
241
242 if (test_bit(CLIENT_XFR_PENDING, &client->flags)) {
243 rc = -EBUSY;
244 goto done;
245 }
246
247 memset(xfr, 0, sizeof(*xfr)); /* clear out the transfer */
248 xfr->buf[0] = 1; /* occ sequence number */
249
250 /* Assume user data follows the occ command format.
251 * byte 0: command type
252 * bytes 1-2: data length (msb first)
253 * bytes 3-n: data
254 */
255 if (copy_from_user(&xfr->buf[1], buf, len)) {
256 rc = -EFAULT;
257 goto done;
258 }
259
260 data_length = (xfr->buf[2] << 8) + xfr->buf[3];
261 if (data_length > OCC_CMD_DATA_BYTES) {
262 rc = -EINVAL;
263 goto done;
264 }
265
266 for (i = 0; i < data_length + 4; ++i)
267 checksum += xfr->buf[i];
268
269 xfr->buf[data_length + 4] = checksum >> 8;
270 xfr->buf[data_length + 5] = checksum & 0xFF;
271
272 xfr->cmd_data_length = data_length + 6;
273 client->read_offset = 0;
274 set_bit(CLIENT_XFR_PENDING, &client->flags);
275 occ_enqueue_xfr(xfr);
276
277 rc = len;
278
279 done:
280 spin_unlock(&client->lock);
281 return rc;
282 }
283
284 static int occ_release(struct inode *inode, struct file *file)
285 {
286 struct occ_client *client = file->private_data;
287 struct occ_xfr *xfr = &client->xfr;
288 struct occ *occ = client->occ;
289
290 spin_lock_irq(&client->lock);
291 if (!test_bit(CLIENT_XFR_PENDING, &client->flags)) {
292 spin_unlock(&client->lock);
293 kfree(client);
294 return 0;
295 }
296
297 spin_lock_irq(&occ->list_lock);
298 set_bit(XFR_CANCELED, &xfr->flags);
299 if (!test_bit(XFR_IN_PROGRESS, &xfr->flags)) {
300 /* already deleted from list if complete */
301 if (!test_bit(XFR_COMPLETE, &xfr->flags))
302 list_del(&xfr->link);
303
304 spin_unlock(&occ->list_lock);
305
306 if (test_bit(XFR_WAITING, &xfr->flags)) {
307 /* blocking read; let reader clean up */
308 wake_up_interruptible(&client->wait);
309 spin_unlock(&client->lock);
310 return 0;
311 }
312
313 spin_unlock(&client->lock);
314 kfree(client);
315 return 0;
316 }
317
318 /* operation is in progress; let worker clean up*/
319 spin_unlock(&occ->list_lock);
320 spin_unlock(&client->lock);
321 return 0;
322 }
323
> 324 static const struct file_operations occ_fops = {
> 325 .owner = THIS_MODULE,
> 326 .open = occ_open,
> 327 .read = occ_read,
> 328 .write = occ_write,
> 329 .release = occ_release,
330 };
331
332 static int occ_write_sbefifo(struct sbefifo_client *client, const char *buf,

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

On Wed, Jun 21, 2017 at 02:38:01PM -0500, Eddie James wrote:
> From: "Edward A. James" <[email protected]>
>
> Add an in-kernel read/write API with exported functions. This is
> necessary for other drivers which want to directly interact with the
> OCC. Also parse the OCC device tree node for child nodes and create
> child platform devices accordingly.
>
> Signed-off-by: Edward A. James <[email protected]>
> ---
> .../devicetree/bindings/fsi/ibm,p9-occ.txt | 8 ++
> drivers/fsi/occ.c | 140 +++++++++++++++++----
> include/linux/occ.h | 27 ++++
> 3 files changed, 154 insertions(+), 21 deletions(-)
> create mode 100644 include/linux/occ.h
>
> diff --git a/Documentation/devicetree/bindings/fsi/ibm,p9-occ.txt b/Documentation/devicetree/bindings/fsi/ibm,p9-occ.txt
> index 88002b9..71afba3 100644
> --- a/Documentation/devicetree/bindings/fsi/ibm,p9-occ.txt
> +++ b/Documentation/devicetree/bindings/fsi/ibm,p9-occ.txt
> @@ -6,10 +6,18 @@ Required properties:
>
> Optional properties:
> - reg = <processor index>; : index for the processor this OCC device is on
> + - <child nodes> : Drivers for devices which communicate with
> + this OCC. Child nodes have no required or
> + optional properties that the OCC driver will
> + use.

Please put all the binding in a single, separate patch.

>
> Examples:
>
> occ@1 {
> compatible = "ibm,p9-occ";
> reg = <1>;
> +
> + occ-hwmon@1 {
> + compatible = "ibm,p9-occ-hwmon";
> + };

This looks like you are creating a node just to instantiate a driver as
hwmon is a Linuxism. Just have the parent OCC driver instantiate a hwmon
driver.

Rob