[{"id":1776862,"web_url":"http://patchwork.ozlabs.org/comment/1776862/","msgid":"<CAMuHMdWfoYzWPJYaGOhF8qshKhvngwFKs=kvuH62PYif9q9+0g@mail.gmail.com>","list_archive_url":null,"date":"2017-09-28T08:33:18","subject":"Re: [patch v9 0/4] JTAG driver introduction","submitter":{"id":703,"url":"http://patchwork.ozlabs.org/api/people/703/","name":"Geert Uytterhoeven","email":"geert@linux-m68k.org"},"content":"Hi Oleksandr,\n\n[My attention was drawn by https://lwn.net/Articles/734440/]\n[CC linux-spi, which was never included, while linux-serial was]\n\nOn Thu, Sep 21, 2017 at 11:25 AM, Oleksandr Shamray\n<oleksandrs@mellanox.com> wrote:\n> When a need raise up to use JTAG interface for system's devices\n> programming or CPU debugging, usually the user layer\n> application implements jtag protocol by bit-bang or using a\n> proprietary connection to vendor hardware.\n> This method can be slow and not generic.\n>\n> We propose to implement general JTAG interface and infrastructure\n> to communicate with user layer application. In such way, we can\n> have the standard JTAG interface core part and separation from\n> specific HW implementation.\n> This allow new capability to debug the CPU or program system's\n> device via BMC without additional devices nor cost.\n>\n> This patch purpose is to add JTAG master core infrastructure by\n> defining new JTAG class and provide generic JTAG interface\n> to allow hardware specific drivers to connect this interface.\n> This will enable all JTAG drivers to use the common interface\n> part and will have separate for hardware implementation.\n>\n> The JTAG (Joint Test Action Group) core driver provides minimal generic\n> JTAG interface, which can be used by hardware specific JTAG master\n> controllers. By providing common interface for the JTAG controllers,\n> user space device programing is hardware independent.\n>\n> Modern SoC which in use for embedded system' equipped with\n> internal JTAG master interface.\n> This interface is used for programming and debugging system's\n> hardware components, like CPLD, FPGA, CPU, voltage and\n> industrial controllers.\n> Firmware for such devices can be upgraded through JTAG interface during\n> Runtime. The JTAG standard support for multiple devices programming,\n> is in case their lines are daisy-chained together.\n>\n> For example, systems which equipped with host CPU, BMC SoC or/and\n> number of programmable devices are capable to connect a pin and\n> select system components dynamically for programming and debugging,\n> This is using by the BMC which is equipped with internal SoC master\n> controller.\n> For example:\n>\n> BMC JTAG master --> pin selected to CPLDs chain for programming (filed\n> upgrade, production)\n> BMC JTAG master --> pin selected to voltage monitors for programming\n> (field upgrade, production)\n> BMC JTAG master --> pin selected to host CPU (on-site debugging\n> and developers debugging)\n>\n> For example, we can have application in user space which using calls\n> to JTAG driver executes CPLD programming directly from SVF file\n>\n> The JTAG standard (IEEE 1149.1) defines the next connector pins:\n> - TDI (Test Data In);\n> - TDO (Test Data Out);\n> - TCK (Test Clock);\n> - TMS (Test Mode Select);\n> - TRST (Test Reset) (Optional);\n>\n> The SoC equipped with JTAG master controller, performs\n> device programming on command or vector level. For example\n> a file in a standard SVF (Serial Vector Format) that contains\n> boundary scan vectors, can be used by sending each vector\n> to the JTAG interface and the JTAG controller will execute\n> the programming.\n>\n> Initial version provides the system calls set for:\n> - SIR (Scan Instruction Register, IEEE 1149.1 Data Register scan);\n> - SDR (Scan Data Register, IEEE 1149.1 Instruction Register scan);\n> - RUNTEST (Forces the IEEE 1149.1 bus to a run state for a specified\n>   number of clocks.\n>\n> SoC which are not equipped with JTAG master interface, can be built\n> on top of JTAG core driver infrastructure, by applying bit-banging of\n> TDI, TDO, TCK and TMS pins within the hardware specific driver.\n\nOr by using an SPI master?\n\nGr{oetje,eeting}s,\n\n                        Geert\n\n--\nGeert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org\n\nIn personal conversations with technical people, I call myself a hacker. But\nwhen I'm talking to journalists I just say \"programmer\" or something like that.\n                                -- Linus Torvalds\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"X8N9zfEz\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2nzh3zwBz9rxl\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tThu, 28 Sep 2017 18:34:24 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752476AbdI1Idi (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tThu, 28 Sep 2017 04:33:38 -0400","from mail-wm0-f67.google.com ([74.125.82.67]:52822 \"EHLO\n\tmail-wm0-f67.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751947AbdI1IdU (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Thu, 28 Sep 2017 04:33:20 -0400","by mail-wm0-f67.google.com with SMTP id m127so644311wmm.1;\n\tThu, 28 Sep 2017 01:33:19 -0700 (PDT)","by 10.223.129.226 with HTTP; Thu, 28 Sep 2017 01:33:18 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=FJL9dm/7ZlsaQE893hGebXwGqiac4C2PcNh9ircWQ8Q=;\n\tb=X8N9zfEzucoeiJT9aT6Jdm1AKp/2v/1f8MMmQSzWCdtRSr87LGrfF4u6yfFUZBDPbh\n\t2+fgmxW1oNUBHEmylXINrnbSYMeo5pD1Bs1ZKNaIilJKlTC81lr/Hlum4Eel5TYzp8uE\n\tLXmVOQkWFGEmDm6RkMr7jAfWulivayrONt/ARnk16v5dT189D8v9kL7LTJKXltgV7WUs\n\tdD9vjpQpFlrVaZhWeof+7zlqhXrvWZcYgiYlNUwiqJPKy/HmCgGPi9OYkphm+tvWgjx/\n\tfM4b91e0ZUN2oHM2adV7vQewJ5FTJzkku4gTENnhXRPE8mb78/4Ciru/b8isan1frQpr\n\tcTAQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:sender:in-reply-to:references:from\n\t:date:message-id:subject:to:cc;\n\tbh=FJL9dm/7ZlsaQE893hGebXwGqiac4C2PcNh9ircWQ8Q=;\n\tb=npGI5vc2YVgpoYrKbKtqxnlEIdhdg4wGTX8jeezi9aSk+p7LBi6okmkZzOeOsQsZa3\n\tojd6aeKhsuoa5MuuzxkOcQ4b5ce1qa8gxDCRFQ0ZGgoztff99JSyc20etEqA8QJziJI4\n\tbstkJ5zT9xNIVL6UGNDnXRDKb0oB1nOiqFwqmW8GK+t9ljuxVtZXfSs0saTSfECPwizy\n\tfO04F9MZGpTpVMXAoH1ALoYMNO0FHhV+XOl/OTT1NCLOh9JzlwC1mADnfgopL/5j/Ao7\n\tVBxofwQ5Bt6+uPc5pyKhVnXB7RR3/uSz8fHY9r4h2yno8o9qpZ7Tu5eWp9vSIkoDeZgs\n\tfsaA==","X-Gm-Message-State":"AHPjjUi9ULrOoC6yvTiaghVFo4PxZsnFy0oChUe7c0UMkRTW0iBlhxJR\n\tk2cgpfdbUq8Pkbb3ZZDkk06QiNJRDbup1Rdzv8cVNg==","X-Google-Smtp-Source":"AOwi7QDbfPUN4h0CanHxpn5JAospxR2VGMF9nOelwXbcDVr20BjteBp50x7wGAKon9NoeJFWWRYEWNvLVNyxXhC0Uu8=","X-Received":"by 10.28.36.212 with SMTP id k203mr260094wmk.127.1506587598760; \n\tThu, 28 Sep 2017 01:33:18 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<1505985932-27568-1-git-send-email-oleksandrs@mellanox.com>","References":"<1505985932-27568-1-git-send-email-oleksandrs@mellanox.com>","From":"Geert Uytterhoeven <geert@linux-m68k.org>","Date":"Thu, 28 Sep 2017 10:33:18 +0200","X-Google-Sender-Auth":"ErALBLwzm6Pxb4jxp6dk4SBrX4o","Message-ID":"<CAMuHMdWfoYzWPJYaGOhF8qshKhvngwFKs=kvuH62PYif9q9+0g@mail.gmail.com>","Subject":"Re: [patch v9 0/4] JTAG driver introduction","To":"Oleksandr Shamray <oleksandrs@mellanox.com>","Cc":"Greg KH <gregkh@linuxfoundation.org>, Arnd Bergmann <arnd@arndb.de>,\n\t\"linux-kernel@vger.kernel.org\" <linux-kernel@vger.kernel.org>,\n\t\"linux-arm-kernel@lists.infradead.org\" \n\t<linux-arm-kernel@lists.infradead.org>,\n\t\"devicetree@vger.kernel.org\" <devicetree@vger.kernel.org>,\n\topenbmc@lists.ozlabs.org, Joel Stanley <joel@jms.id.au>,\n\tJiri Pirko <jiri@resnulli.us>, Tobias Klauser <tklauser@distanz.ch>, \n\t\"linux-serial@vger.kernel.org\" <linux-serial@vger.kernel.org>,\n\tmec@shout.net, Vadim Pasternak <vadimp@mellanox.com>,\n\tsystem-sw-low-level@mellanox.com, Rob Herring <robh+dt@kernel.org>,\n\topenocd-devel-owner@lists.sourceforge.net,\n\t\"linux-api@vger.kernel.org\" <linux-api@vger.kernel.org>,\n\t\"David S. Miller\" <davem@davemloft.net>,\n\tMauro Carvalho Chehab <mchehab@kernel.org>,\n\tlinux-spi <linux-spi@vger.kernel.org>, Mark Brown <broonie@kernel.org>","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1776882,"web_url":"http://patchwork.ozlabs.org/comment/1776882/","msgid":"<AM4PR0501MB2194D67623C640E20B3A7FABB1790@AM4PR0501MB2194.eurprd05.prod.outlook.com>","list_archive_url":null,"date":"2017-09-28T08:53:12","subject":"RE: [patch v9 0/4] JTAG driver introduction","submitter":{"id":72099,"url":"http://patchwork.ozlabs.org/api/people/72099/","name":"Oleksandr Shamray","email":"oleksandrs@mellanox.com"},"content":"> -----Original Message-----\r\n> From: geert.uytterhoeven@gmail.com [mailto:geert.uytterhoeven@gmail.com]\r\n> On Behalf Of Geert Uytterhoeven\r\n> Sent: Thursday, September 28, 2017 11:33 AM\r\n> To: Oleksandr Shamray <oleksandrs@mellanox.com>\r\n> Cc: Greg KH <gregkh@linuxfoundation.org>; Arnd Bergmann\r\n> <arnd@arndb.de>; linux-kernel@vger.kernel.org; linux-arm-\r\n> kernel@lists.infradead.org; devicetree@vger.kernel.org;\r\n> openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>; Jiri Pirko\r\n> <jiri@resnulli.us>; Tobias Klauser <tklauser@distanz.ch>; linux-\r\n> serial@vger.kernel.org; mec@shout.net; Vadim Pasternak\r\n> <vadimp@mellanox.com>; system-sw-low-level <system-sw-low-\r\n> level@mellanox.com>; Rob Herring <robh+dt@kernel.org>; openocd-devel-\r\n> owner@lists.sourceforge.net; linux-api@vger.kernel.org; David S. Miller\r\n> <davem@davemloft.net>; Mauro Carvalho Chehab <mchehab@kernel.org>;\r\n> linux-spi <linux-spi@vger.kernel.org>; Mark Brown <broonie@kernel.org>\r\n> Subject: Re: [patch v9 0/4] JTAG driver introduction\r\n> \r\n> Hi Oleksandr,\r\n> \r\n> [My attention was drawn by\r\n> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flwn.net\r\n> %2FArticles%2F734440%2F&data=02%7C01%7Coleksandrs%40mellanox.com%7\r\n> C97b8ba88686a42daaace08d5064b92eb%7Ca652971c7d2e4d9ba6a4d149256f\r\n> 461b%7C0%7C0%7C636421844026854216&sdata=TeHD4a3%2FBN6a5XG3Jizf5\r\n> pmsyJHJjzkEzkpnqsXC6S0%3D&reserved=0]\r\n> [CC linux-spi, which was never included, while linux-serial was]\r\n> \r\n> On Thu, Sep 21, 2017 at 11:25 AM, Oleksandr Shamray\r\n> <oleksandrs@mellanox.com> wrote:\r\n> > When a need raise up to use JTAG interface for system's devices\r\n> > programming or CPU debugging, usually the user layer application\r\n> > implements jtag protocol by bit-bang or using a proprietary connection\r\n> > to vendor hardware.\r\n> > This method can be slow and not generic.\r\n> >\r\n\r\n\r\n[..]\r\n\r\n> >\r\n> > Initial version provides the system calls set for:\r\n> > - SIR (Scan Instruction Register, IEEE 1149.1 Data Register scan);\r\n> > - SDR (Scan Data Register, IEEE 1149.1 Instruction Register scan);\r\n> > - RUNTEST (Forces the IEEE 1149.1 bus to a run state for a specified\r\n> >   number of clocks.\r\n> >\r\n> > SoC which are not equipped with JTAG master interface, can be built on\r\n> > top of JTAG core driver infrastructure, by applying bit-banging of\r\n> > TDI, TDO, TCK and TMS pins within the hardware specific driver.\r\n> \r\n> Or by using an SPI master?\r\n> \r\n\r\nI think it depends on how flexible the SPI interface is. If you can set it to transfer \r\nfrom 1 to n bits at a time, and you control the TMS line in software, you should \r\nbe able to use it. If the SPI interface can only transfer a multiple of 8 bits \r\nat a time, then in general it would not be suitable for JTAG.\r\n\r\n> Gr{oetje,eeting}s,\r\n> \r\n>                         Geert\r\n> \r\n> --\r\n> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-\r\n> m68k.org\r\n> \r\n> In personal conversations with technical people, I call myself a hacker. But\r\n> when I'm talking to journalists I just say \"programmer\" or something like that.\r\n>                                 -- Linus Torvalds","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=Mellanox.com header.i=@Mellanox.com\n\theader.b=\"YhB68n3o\"; dkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=oleksandrs@mellanox.com; "],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2pPb3J8Sz9t2l\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tThu, 28 Sep 2017 18:53:23 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751920AbdI1IxV (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tThu, 28 Sep 2017 04:53:21 -0400","from mail-eopbgr10084.outbound.protection.outlook.com\n\t([40.107.1.84]:27520\n\t\"EHLO EUR02-HE1-obe.outbound.protection.outlook.com\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1751592AbdI1IxT (ORCPT <rfc822;devicetree@vger.kernel.org>);\n\tThu, 28 Sep 2017 04:53:19 -0400","from AM4PR0501MB2194.eurprd05.prod.outlook.com (10.165.82.13) by\n\tHE1PR05MB3339.eurprd05.prod.outlook.com (10.170.243.145) with\n\tMicrosoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id\n\t15.20.77.7; Thu, 28 Sep 2017 08:53:12 +0000","from AM4PR0501MB2194.eurprd05.prod.outlook.com\n\t([fe80::c1dc:8bea:a5cb:bcb6]) by\n\tAM4PR0501MB2194.eurprd05.prod.outlook.com\n\t([fe80::c1dc:8bea:a5cb:bcb6%18]) with mapi id 15.20.0077.016;\n\tThu, 28 Sep 2017 08:53:12 +0000"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com;\n\ts=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version;\n\tbh=ZSzKvaLqOB+8JYL84xiGjioh1+Ge986pOVACqDcoF7A=;\n\tb=YhB68n3oRQmGg9w303lbAsZU7We49vO7GEDff5oF+YcfIRCjA8P7wZNfmSoffUVNHVfPrF6PoClrOTIfrv5ilez84C/1+QRGcWk1UCFiNgq+tJqWObEG/HLeXSBv8bgYpoY3UVk+4Xg08JW+NCkrjGbcfi9LmzY82oeW197SEXk=","From":"Oleksandr Shamray <oleksandrs@mellanox.com>","To":"Geert Uytterhoeven <geert@linux-m68k.org>","CC":"Greg KH <gregkh@linuxfoundation.org>, Arnd Bergmann <arnd@arndb.de>,\n\t\"linux-kernel@vger.kernel.org\" <linux-kernel@vger.kernel.org>,\n\t\"linux-arm-kernel@lists.infradead.org\" \n\t<linux-arm-kernel@lists.infradead.org>,\n\t\"devicetree@vger.kernel.org\" <devicetree@vger.kernel.org>,\n\t\"openbmc@lists.ozlabs.org\" <openbmc@lists.ozlabs.org>,\n\tJoel Stanley <joel@jms.id.au>, Jiri Pirko <jiri@resnulli.us>,\n\tTobias Klauser <tklauser@distanz.ch>,\n\t\"linux-serial@vger.kernel.org\" <linux-serial@vger.kernel.org>,\n\t\"mec@shout.net\" <mec@shout.net>, Vadim Pasternak <vadimp@mellanox.com>,\n\tsystem-sw-low-level <system-sw-low-level@mellanox.com>,\n\tRob Herring <robh+dt@kernel.org>,\n\t\"openocd-devel-owner@lists.sourceforge.net\" \n\t<openocd-devel-owner@lists.sourceforge.net>,\n\t\"linux-api@vger.kernel.org\" <linux-api@vger.kernel.org>,\n\t\"David S. Miller\" <davem@davemloft.net>,\n\t\"Mauro Carvalho Chehab\" <mchehab@kernel.org>,\n\tlinux-spi <linux-spi@vger.kernel.org>, Mark Brown <broonie@kernel.org>","Subject":"RE: [patch v9 0/4] JTAG driver introduction","Thread-Topic":"[patch v9 0/4] JTAG driver introduction","Thread-Index":"AQHTMru4M5azt7XbqEuLkdSxxS/sN6LKA2cAgAAE/MA=","Date":"Thu, 28 Sep 2017 08:53:12 +0000","Message-ID":"<AM4PR0501MB2194D67623C640E20B3A7FABB1790@AM4PR0501MB2194.eurprd05.prod.outlook.com>","References":"<1505985932-27568-1-git-send-email-oleksandrs@mellanox.com>\n\t<CAMuHMdWfoYzWPJYaGOhF8qshKhvngwFKs=kvuH62PYif9q9+0g@mail.gmail.com>","In-Reply-To":"<CAMuHMdWfoYzWPJYaGOhF8qshKhvngwFKs=kvuH62PYif9q9+0g@mail.gmail.com>","Accept-Language":"en-US, uk-UA","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","authentication-results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=Mellanox.com header.i=@Mellanox.com\n\theader.b=\"YhB68n3o\"; dkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=oleksandrs@mellanox.com; "],"x-originating-ip":"[80.90.224.13]","x-ms-publictraffictype":"Email","x-microsoft-exchange-diagnostics":"1; HE1PR05MB3339;\n\t6:q+k/swSyvnXMeIADVfddUZ4/QfGRNAJI6qPB0CJ0I1OjU6p82DZm7/vOSHUzBJsUsXxtcNCmKWNUigc977ldy4l1rC0Hou/rok49BXphXP0LbtcYZNpLaNoLxT4+NaHdSCDZcMM1B2BLbRYQQigSepww3O2kCSSgX5HgkTXEu5kEjK+R4I0J4j1VEHCdp9cZjry+fyBkUV7AZZUdDVLgrdqkb4ALvLybuR5ptQ69j33AuCBAsHHNxA9buf2/j2C+9G0VI7+UGUErYnMZ6JegrYyJTLjbMJ2iB1XrBQKaGZMJoCVesukXsDZ5e7QZrziifssW1RFz2Fg1iYbzWWcvyQ==;\n\t5:ClwoVo+Fk+rAlj2RvFhpZbaZsuDNiVbgp7fgs6CnzzkV/Oh//r39zG6QNMsNgfk0LoE+sqvySpMdYt9K2b0xwnL7qCPatl8LyRCNH07PZMAbddU/oBM8fqsYsYLC7hI2Uj/hRv/vIhrbiNytJX7DGw==;\n\t24:6LRVj4bt+W3GL5mjMPHnEpIIrfwBHwVxoEJ3nQ8cLhfXrjGlZEyo/XQJZvouI+tQgmIHbaKKrRb658/7w8wM0Di0qyT8MoqIvp2epQwSwjI=;\n\t7:HuJvL1dSsp07gbzfpDyapcaFWIOPplqDMGWThQvVCREjAd7HK/CWtGnnxH23kGAkDVlz3RUnf9N/BFf0pOZ7sm2YN6W+LtrLMHX6qxitS5BkKxw+pZy/WAEV64Il07X0GUSEG8vQhgH3B6BAoHkUiTDu9PC9wiy63L2IudefHLwnyRWLpAnWa5nMrTeED0pKO9Lm0PogzB+nR+aVHVufPTN0Hmai0ZhDuwCmEeRqKxg=","x-ms-office365-filtering-correlation-id":"dee184af-35ab-403f-a408-08d5064e59c3","x-ms-office365-filtering-ht":"Tenant","x-microsoft-antispam":"UriScan:; BCL:0; PCL:0;\n\tRULEID:(22001)(2017030254152)(48565401081)(2017052603199)(201703131423075)(201703031133081)(201702281549075);\n\tSRVR:HE1PR05MB3339; ","x-ms-traffictypediagnostic":"HE1PR05MB3339:","x-exchange-antispam-report-test":"UriScan:(143289334528602)(9452136761055)(189930954265078)(65623756079841)(258649278758335)(42262312472803)(45079756050767);","x-microsoft-antispam-prvs":"<HE1PR05MB333982ADD76CFBB82476A360B1790@HE1PR05MB3339.eurprd05.prod.outlook.com>","x-exchange-antispam-report-cfa-test":"BCL:0; PCL:0;\n\tRULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(8121501046)(5005006)(93006095)(93001095)(10201501046)(3002001)(100000703101)(100105400095)(6055026)(6041248)(20161123558100)(20161123555025)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123564025)(20161123562025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);\n\tSRVR:HE1PR05MB3339; BCL:0; PCL:0;\n\tRULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);\n\tSRVR:HE1PR05MB3339; ","x-forefront-prvs":"0444EB1997","x-forefront-antispam-report":"SFV:NSPM;\n\tSFS:(10009020)(6009001)(376002)(346002)(39860400002)(13464003)(24454002)(199003)(189002)(377454003)(55016002)(101416001)(14454004)(7736002)(6246003)(105586002)(7696004)(305945005)(97736004)(106356001)(5250100002)(966005)(3660700001)(6306002)(86362001)(45080400002)(9686003)(3280700002)(2950100002)(6506006)(53546010)(99286003)(189998001)(5660300001)(478600001)(2900100001)(53936002)(8676002)(81156014)(316002)(25786009)(54356999)(50986999)(76176999)(8936002)(81166006)(7416002)(33656002)(68736007)(54906003)(66066001)(6916009)(74316002)(3846002)(6436002)(2906002)(229853002)(4326008)(102836003)(6116002);\n\tDIR:OUT; SFP:1101; SCL:1; SRVR:HE1PR05MB3339;\n\tH:AM4PR0501MB2194.eurprd05.prod.outlook.com; FPR:; SPF:None;\n\tPTR:InfoNoRecords; A:1; MX:1; LANG:en; ","received-spf":"None (protection.outlook.com: mellanox.com does not designate\n\tpermitted sender hosts)","spamdiagnosticoutput":"1:99","spamdiagnosticmetadata":"NSPM","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","MIME-Version":"1.0","X-OriginatorOrg":"Mellanox.com","X-MS-Exchange-CrossTenant-originalarrivaltime":"28 Sep 2017 08:53:12.4284\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"a652971c-7d2e-4d9b-a6a4-d149256f461b","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"HE1PR05MB3339","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1776889,"web_url":"http://patchwork.ozlabs.org/comment/1776889/","msgid":"<CAMuHMdX1HNym4n67y2VbZqFDLHFGunfRp-LPn0g2K4qhya_w9g@mail.gmail.com>","list_archive_url":null,"date":"2017-09-28T09:02:10","subject":"Re: [patch v9 0/4] JTAG driver introduction","submitter":{"id":703,"url":"http://patchwork.ozlabs.org/api/people/703/","name":"Geert Uytterhoeven","email":"geert@linux-m68k.org"},"content":"Hi Oleksandr,\n\nOn Thu, Sep 28, 2017 at 10:53 AM, Oleksandr Shamray\n<oleksandrs@mellanox.com> wrote:\n>> [My attention was drawn by\n>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flwn.net\n>> %2FArticles%2F734440%2F&data=02%7C01%7Coleksandrs%40mellanox.com%7\n>> C97b8ba88686a42daaace08d5064b92eb%7Ca652971c7d2e4d9ba6a4d149256f\n>> 461b%7C0%7C0%7C636421844026854216&sdata=TeHD4a3%2FBN6a5XG3Jizf5\n>> pmsyJHJjzkEzkpnqsXC6S0%3D&reserved=0]\n>> [CC linux-spi, which was never included, while linux-serial was]\n>>\n>> On Thu, Sep 21, 2017 at 11:25 AM, Oleksandr Shamray\n>> <oleksandrs@mellanox.com> wrote:\n>> > When a need raise up to use JTAG interface for system's devices\n>> > programming or CPU debugging, usually the user layer application\n>> > implements jtag protocol by bit-bang or using a proprietary connection\n>> > to vendor hardware.\n>> > This method can be slow and not generic.\n>> >\n>\n>\n> [..]\n>\n>> >\n>> > Initial version provides the system calls set for:\n>> > - SIR (Scan Instruction Register, IEEE 1149.1 Data Register scan);\n>> > - SDR (Scan Data Register, IEEE 1149.1 Instruction Register scan);\n>> > - RUNTEST (Forces the IEEE 1149.1 bus to a run state for a specified\n>> >   number of clocks.\n>> >\n>> > SoC which are not equipped with JTAG master interface, can be built on\n>> > top of JTAG core driver infrastructure, by applying bit-banging of\n>> > TDI, TDO, TCK and TMS pins within the hardware specific driver.\n>>\n>> Or by using an SPI master?\n>>\n>\n> I think it depends on how flexible the SPI interface is. If you can set it to transfer\n> from 1 to n bits at a time, and you control the TMS line in software, you should\n> be able to use it. If the SPI interface can only transfer a multiple of 8 bits\n> at a time, then in general it would not be suitable for JTAG.\n\nSure, that depends on the actual SPI master interface.\nBut I guess you can write a generic JTAG-over-SPI driver, and use a tms-gpios\nproperty in DT to specify how to control TMS.\n\nGr{oetje,eeting}s,\n\n                        Geert\n\n--\nGeert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org\n\nIn personal conversations with technical people, I call myself a hacker. But\nwhen I'm talking to journalists I just say \"programmer\" or something like that.\n                                -- Linus Torvalds\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"FV2jeYPi\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2pbt5Lp7z9t2l\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tThu, 28 Sep 2017 19:02:18 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752453AbdI1JCQ (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tThu, 28 Sep 2017 05:02:16 -0400","from mail-wm0-f66.google.com ([74.125.82.66]:45286 \"EHLO\n\tmail-wm0-f66.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752392AbdI1JCN (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Thu, 28 Sep 2017 05:02:13 -0400","by mail-wm0-f66.google.com with SMTP id q124so818364wmb.0;\n\tThu, 28 Sep 2017 02:02:11 -0700 (PDT)","by 10.223.129.226 with HTTP; Thu, 28 Sep 2017 02:02:10 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=B8OHUWY4g6nbqeNhcZNkWYkQiSc3AHzgIHIMyPyG6to=;\n\tb=FV2jeYPizZAIGwZynPVykmTDWB7/TkZdTf5MK9bZBw6AckuhoWpitCCjVe4iNAmHbQ\n\t1IErqwjvtwnSmDJN9vNG0zP6haCbqj3XoTTllboN1MGCQAe62PnUmvZHAe7ILfqsDW3E\n\tlXUaHHoWUQS7VEea4o7AAhB4BB8er0nFi2eaL/+hfatZoSxk+ZKHcJ/nEDKEPpXmrnfZ\n\tMkfeLArpcD4tHa2A/zbgxBlVlq0QdsQ74NsYlVLdCTDGb2LjcM9oWtDvbCE9Jv69jI2+\n\tG3xH6LgANEV7j6Ig/zny6eGVj7gc+1kwqjDI8PjV3cMybw/0aUHYssACM6hpWcUO4KHw\n\tX5Ig==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:sender:in-reply-to:references:from\n\t:date:message-id:subject:to:cc;\n\tbh=B8OHUWY4g6nbqeNhcZNkWYkQiSc3AHzgIHIMyPyG6to=;\n\tb=m7/auqP3sqIRh2YBKm4rQIY5+mtXY1pYJQdh6emDnLzj8I+C8GaISNynzTuEAgHeKj\n\tiTKA0zUWaMQGOdQ5T9Ut3jJqnmMJ7Mr4cZqpZ8qC6G3Hz1UxgBqKphMw7ZYhV/9/cVP+\n\tnBX+OggW79T9N5HppYJFYiPc0jICVb5B5M4rL6oRE9g2wIuPiqbzMVquZRsrHLl1DHqA\n\tbcNhTJa2zQGthIoCkJ4cEL8Jq/i7/DBQpLdgesCvv2PU9xeXecZjw2zqk4oyp4uLZwxQ\n\t+NpU3nmY5AVa0QXkZ8zTQJCGVTo9hacaBSiJvaaqPsyU/wvUHdq35WGHLd99CiYAuo37\n\tBAGw==","X-Gm-Message-State":"AHPjjUjxIJsYFuVUEuuIfH3QVxH3eqXiQgBiE+OUJf4/+IlA85W1bdp4\n\tRhKVZ1AlDLegLbXPH2D4thrarhlwODNc8MnaCR8=","X-Google-Smtp-Source":"AOwi7QABqUCPPd4e9SYSYljs3C62h4SmYVMqoXc6U86sfd7wiHmnNS1NiPlDSCQPLDGDDfFE5cVKdzUO+gnuEm7FGwA=","X-Received":"by 10.28.71.25 with SMTP id u25mr373899wma.22.1506589331224; Thu,\n\t28 Sep 2017 02:02:11 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<AM4PR0501MB2194D67623C640E20B3A7FABB1790@AM4PR0501MB2194.eurprd05.prod.outlook.com>","References":"<1505985932-27568-1-git-send-email-oleksandrs@mellanox.com>\n\t<CAMuHMdWfoYzWPJYaGOhF8qshKhvngwFKs=kvuH62PYif9q9+0g@mail.gmail.com>\n\t<AM4PR0501MB2194D67623C640E20B3A7FABB1790@AM4PR0501MB2194.eurprd05.prod.outlook.com>","From":"Geert Uytterhoeven <geert@linux-m68k.org>","Date":"Thu, 28 Sep 2017 11:02:10 +0200","X-Google-Sender-Auth":"yqjJszmxOFubJPBq44r3ySY7E7Y","Message-ID":"<CAMuHMdX1HNym4n67y2VbZqFDLHFGunfRp-LPn0g2K4qhya_w9g@mail.gmail.com>","Subject":"Re: [patch v9 0/4] JTAG driver introduction","To":"Oleksandr Shamray <oleksandrs@mellanox.com>","Cc":"Greg KH <gregkh@linuxfoundation.org>, Arnd Bergmann <arnd@arndb.de>,\n\t\"linux-kernel@vger.kernel.org\" <linux-kernel@vger.kernel.org>,\n\t\"linux-arm-kernel@lists.infradead.org\" \n\t<linux-arm-kernel@lists.infradead.org>,\n\t\"devicetree@vger.kernel.org\" <devicetree@vger.kernel.org>,\n\t\"openbmc@lists.ozlabs.org\" <openbmc@lists.ozlabs.org>,\n\tJoel Stanley <joel@jms.id.au>, Jiri Pirko <jiri@resnulli.us>,\n\tTobias Klauser <tklauser@distanz.ch>,\n\t\"linux-serial@vger.kernel.org\" <linux-serial@vger.kernel.org>,\n\t\"mec@shout.net\" <mec@shout.net>, Vadim Pasternak <vadimp@mellanox.com>,\n\tsystem-sw-low-level <system-sw-low-level@mellanox.com>,\n\tRob Herring <robh+dt@kernel.org>,\n\t\"openocd-devel-owner@lists.sourceforge.net\" \n\t<openocd-devel-owner@lists.sourceforge.net>,\n\t\"linux-api@vger.kernel.org\" <linux-api@vger.kernel.org>,\n\t\"David S. Miller\" <davem@davemloft.net>,\n\tMauro Carvalho Chehab <mchehab@kernel.org>,\n\tlinux-spi <linux-spi@vger.kernel.org>, Mark Brown <broonie@kernel.org>","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1776964,"web_url":"http://patchwork.ozlabs.org/comment/1776964/","msgid":"<AM4PR0501MB219466B15EB1314E3AF8413FB1790@AM4PR0501MB2194.eurprd05.prod.outlook.com>","list_archive_url":null,"date":"2017-09-28T11:11:42","subject":"RE: [patch v9 0/4] JTAG driver introduction","submitter":{"id":72099,"url":"http://patchwork.ozlabs.org/api/people/72099/","name":"Oleksandr Shamray","email":"oleksandrs@mellanox.com"},"content":"> -----Original Message-----\r\n> From: geert.uytterhoeven@gmail.com [mailto:geert.uytterhoeven@gmail.com]\r\n> On Behalf Of Geert Uytterhoeven\r\n> Sent: Thursday, September 28, 2017 12:02 PM\r\n> To: Oleksandr Shamray <oleksandrs@mellanox.com>\r\n> Cc: Greg KH <gregkh@linuxfoundation.org>; Arnd Bergmann\r\n> <arnd@arndb.de>; linux-kernel@vger.kernel.org; linux-arm-\r\n> kernel@lists.infradead.org; devicetree@vger.kernel.org;\r\n> openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>; Jiri Pirko\r\n> <jiri@resnulli.us>; Tobias Klauser <tklauser@distanz.ch>; linux-\r\n> serial@vger.kernel.org; mec@shout.net; Vadim Pasternak\r\n> <vadimp@mellanox.com>; system-sw-low-level <system-sw-low-\r\n> level@mellanox.com>; Rob Herring <robh+dt@kernel.org>; openocd-devel-\r\n> owner@lists.sourceforge.net; linux-api@vger.kernel.org; David S. Miller\r\n> <davem@davemloft.net>; Mauro Carvalho Chehab <mchehab@kernel.org>;\r\n> linux-spi <linux-spi@vger.kernel.org>; Mark Brown <broonie@kernel.org>\r\n> Subject: Re: [patch v9 0/4] JTAG driver introduction\r\n> \r\n> Hi Oleksandr,\r\n> \r\n> On Thu, Sep 28, 2017 at 10:53 AM, Oleksandr Shamray\r\n> <oleksandrs@mellanox.com> wrote:\r\n> >> [My attention was drawn by\r\n> >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flw\r\n> >> n.net\r\n> >>\r\n> %2FArticles%2F734440%2F&data=02%7C01%7Coleksandrs%40mellanox.com%7\r\n> >>\r\n> C97b8ba88686a42daaace08d5064b92eb%7Ca652971c7d2e4d9ba6a4d149256f\r\n> >>\r\n> 461b%7C0%7C0%7C636421844026854216&sdata=TeHD4a3%2FBN6a5XG3Jizf5\r\n> >> pmsyJHJjzkEzkpnqsXC6S0%3D&reserved=0]\r\n> >> [CC linux-spi, which was never included, while linux-serial was]\r\n> >>\r\n> >> On Thu, Sep 21, 2017 at 11:25 AM, Oleksandr Shamray\r\n> >> <oleksandrs@mellanox.com> wrote:\r\n> >> > When a need raise up to use JTAG interface for system's devices\r\n> >> > programming or CPU debugging, usually the user layer application\r\n> >> > implements jtag protocol by bit-bang or using a proprietary\r\n> >> > connection to vendor hardware.\r\n> >> > This method can be slow and not generic.\r\n> >> >\r\n> >\r\n> >\r\n> > [..]\r\n> >\r\n> >> >\r\n> >> > Initial version provides the system calls set for:\r\n> >> > - SIR (Scan Instruction Register, IEEE 1149.1 Data Register scan);\r\n> >> > - SDR (Scan Data Register, IEEE 1149.1 Instruction Register scan);\r\n> >> > - RUNTEST (Forces the IEEE 1149.1 bus to a run state for a specified\r\n> >> >   number of clocks.\r\n> >> >\r\n> >> > SoC which are not equipped with JTAG master interface, can be built\r\n> >> > on top of JTAG core driver infrastructure, by applying bit-banging\r\n> >> > of TDI, TDO, TCK and TMS pins within the hardware specific driver.\r\n> >>\r\n> >> Or by using an SPI master?\r\n> >>\r\n> >\r\n> > I think it depends on how flexible the SPI interface is. If you can\r\n> > set it to transfer from 1 to n bits at a time, and you control the TMS\r\n> > line in software, you should be able to use it. If the SPI interface\r\n> > can only transfer a multiple of 8 bits at a time, then in general it would not be\r\n> suitable for JTAG.\r\n> \r\n> Sure, that depends on the actual SPI master interface.\r\n> But I guess you can write a generic JTAG-over-SPI driver, and use a tms-gpios\r\n> property in DT to specify how to control TMS.\r\n> \r\n\r\nYes, this is a good idea for SoC which are not equipped with JTAG hardware.\r\n\r\n> Gr{oetje,eeting}s,\r\n> \r\n>                         Geert\r\n> \r\n> --\r\n> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-\r\n> m68k.org\r\n> \r\n> In personal conversations with technical people, I call myself a hacker. But\r\n> when I'm talking to journalists I just say \"programmer\" or something like that.\r\n>                                 -- Linus Torvalds","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=Mellanox.com header.i=@Mellanox.com\n\theader.b=\"RkJXnhJu\"; dkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=oleksandrs@mellanox.com; "],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2sTd4r9Fz9tXp\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tThu, 28 Sep 2017 21:12:05 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752013AbdI1LMD (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tThu, 28 Sep 2017 07:12:03 -0400","from mail-ve1eur01on0067.outbound.protection.outlook.com\n\t([104.47.1.67]:29472\n\t\"EHLO EUR01-VE1-obe.outbound.protection.outlook.com\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1751816AbdI1LLs (ORCPT <rfc822;devicetree@vger.kernel.org>);\n\tThu, 28 Sep 2017 07:11:48 -0400","from AM4PR0501MB2194.eurprd05.prod.outlook.com (10.165.82.13) by\n\tVI1PR05MB3343.eurprd05.prod.outlook.com (10.170.238.144) with\n\tMicrosoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id\n\t15.20.77.7; Thu, 28 Sep 2017 11:11:42 +0000","from AM4PR0501MB2194.eurprd05.prod.outlook.com\n\t([fe80::c1dc:8bea:a5cb:bcb6]) by\n\tAM4PR0501MB2194.eurprd05.prod.outlook.com\n\t([fe80::c1dc:8bea:a5cb:bcb6%18]) with mapi id 15.20.0077.016;\n\tThu, 28 Sep 2017 11:11:42 +0000"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com;\n\ts=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version;\n\tbh=x0nmrw6G39mJ5AFWSCC0hEmoEzScp7LObaJLgCsRl8I=;\n\tb=RkJXnhJu0FiNvzi9Df5KoPKI7hOsrGjfCYjh+MczJG7UrnW6laB/3dLobLhMjNzMsq3F+eqf1OMBhYbfW8e1lT29WsRjaioKwTuy1VoetoGhENUmFVPuodM5OnQahzbSTMEqgAT3VALpYm3/vXTX+MMzlN3OQJNSrMB+lKW4WHI=","From":"Oleksandr Shamray <oleksandrs@mellanox.com>","To":"Geert Uytterhoeven <geert@linux-m68k.org>","CC":"Greg KH <gregkh@linuxfoundation.org>, Arnd Bergmann <arnd@arndb.de>,\n\t\"linux-kernel@vger.kernel.org\" <linux-kernel@vger.kernel.org>,\n\t\"linux-arm-kernel@lists.infradead.org\" \n\t<linux-arm-kernel@lists.infradead.org>,\n\t\"devicetree@vger.kernel.org\" <devicetree@vger.kernel.org>,\n\t\"openbmc@lists.ozlabs.org\" <openbmc@lists.ozlabs.org>,\n\tJoel Stanley <joel@jms.id.au>, Jiri Pirko <jiri@resnulli.us>,\n\tTobias Klauser <tklauser@distanz.ch>,\n\t\"linux-serial@vger.kernel.org\" <linux-serial@vger.kernel.org>,\n\t\"mec@shout.net\" <mec@shout.net>, Vadim Pasternak <vadimp@mellanox.com>,\n\tsystem-sw-low-level <system-sw-low-level@mellanox.com>,\n\tRob Herring <robh+dt@kernel.org>,\n\t\"openocd-devel-owner@lists.sourceforge.net\" \n\t<openocd-devel-owner@lists.sourceforge.net>,\n\t\"linux-api@vger.kernel.org\" <linux-api@vger.kernel.org>,\n\t\"David S. Miller\" <davem@davemloft.net>,\n\t\"Mauro Carvalho Chehab\" <mchehab@kernel.org>,\n\tlinux-spi <linux-spi@vger.kernel.org>, Mark Brown <broonie@kernel.org>","Subject":"RE: [patch v9 0/4] JTAG driver introduction","Thread-Topic":"[patch v9 0/4] JTAG driver introduction","Thread-Index":"AQHTMru4M5azt7XbqEuLkdSxxS/sN6LKA2cAgAAE/MCAAAMUAIAAIqNQ","Date":"Thu, 28 Sep 2017 11:11:42 +0000","Message-ID":"<AM4PR0501MB219466B15EB1314E3AF8413FB1790@AM4PR0501MB2194.eurprd05.prod.outlook.com>","References":"<1505985932-27568-1-git-send-email-oleksandrs@mellanox.com>\n\t<CAMuHMdWfoYzWPJYaGOhF8qshKhvngwFKs=kvuH62PYif9q9+0g@mail.gmail.com>\n\t<AM4PR0501MB2194D67623C640E20B3A7FABB1790@AM4PR0501MB2194.eurprd05.prod.outlook.com>\n\t<CAMuHMdX1HNym4n67y2VbZqFDLHFGunfRp-LPn0g2K4qhya_w9g@mail.gmail.com>","In-Reply-To":"<CAMuHMdX1HNym4n67y2VbZqFDLHFGunfRp-LPn0g2K4qhya_w9g@mail.gmail.com>","Accept-Language":"en-US, uk-UA","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","authentication-results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=Mellanox.com header.i=@Mellanox.com\n\theader.b=\"RkJXnhJu\"; dkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=oleksandrs@mellanox.com; "],"x-originating-ip":"[80.90.224.13]","x-ms-publictraffictype":"Email","x-microsoft-exchange-diagnostics":"1; VI1PR05MB3343;\n\t6:no5mxZ/+rBgQR4VUQ9EIsDKHlGLze6N9DQ9neeKWeCAjkm1GQK44xulUH2kOr9pQG5ZAXWQTERvbGV0gQ9eUftrmm7PvEF3fwGw4bHqar0QvDWtG9Tst9SrLlM5D5WNaDweBzyr46n55RAoY/g0CBjwxWhqBRCgJ9HUiRJnr/T8iTsE9MZQExViCwPqQ4c4YviMJ/Coz5oz9Cl03OpwO/m/GIqmkqc55PIC+uDdLHw3OjA52p3sLWc3kph0Gn/d9TyqWcAJq1QeDSXTXXMBmqkpUXhi+L1gFviSo+TcqfFJF93NWpZRW/xUQPnVd0nFrCDShDpj4EDirliny18ZjfA==;\n\t5:bSeEanLLST/NB8olvOlpyz3Jz2kAklRfiU5bmNiIJi0aA9Ma9Auf/XqNqlY31gJMMkf96R5tJD1X4DMu4xhe3tGntJieYkHJbXit+q8xbaZa7NwPyf9J7fmhqOsiUsOsoCfjrw/pJc3uWvs2/ZRA/w==;\n\t24:S8LdIrYUnF/LZ4jm2BIzpHAe2Ev2Cs9AbEwTzy1BawX7MN8sGwaH6Po5NtVLRBOwqTSFvfHLl18ujGuojsF2JBwRZFSoXVN6pb/M13A4+vo=;\n\t7:ObfCEA2lL3SNjvnn1TEJtf0N6nE+DTSV+SH+IXGflSnkgVLG8xUBn1MFD2kF1CCpVNT7AH8uUXuXasBHBFC1YnS3M9uBnO+FikzzoVgSsvfElxjGoEf1Ib/Bw7XdRTMLUNUdXiWG4K2n/dd4CDCHpK96dvKOujROZq00omPZVWpMsJrKUpeg9Q1lOOd39N4lf2D3C8WxdlCseL79DdTpkJ4h91Q/QnPBuhaaM8ueBGk=","x-ms-office365-filtering-correlation-id":"79a4c72b-661c-4866-8de8-08d50661b2ed","x-ms-office365-filtering-ht":"Tenant","x-microsoft-antispam":"UriScan:; BCL:0; PCL:0;\n\tRULEID:(22001)(2017030254152)(48565401081)(2017052603199)(201703131423075)(201703031133081)(201702281549075);\n\tSRVR:VI1PR05MB3343; ","x-ms-traffictypediagnostic":"VI1PR05MB3343:","x-exchange-antispam-report-test":"UriScan:(143289334528602)(9452136761055)(189930954265078)(65623756079841)(258649278758335)(42262312472803)(45079756050767);","x-microsoft-antispam-prvs":"<VI1PR05MB3343A04C54FF9E810D1D50A5B1790@VI1PR05MB3343.eurprd05.prod.outlook.com>","x-exchange-antispam-report-cfa-test":"BCL:0; PCL:0;\n\tRULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(93006095)(93001095)(10201501046)(100000703101)(100105400095)(3002001)(6055026)(6041248)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123564025)(20161123562025)(20161123555025)(20161123558100)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);\n\tSRVR:VI1PR05MB3343; BCL:0; PCL:0;\n\tRULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);\n\tSRVR:VI1PR05MB3343; ","x-forefront-prvs":"0444EB1997","x-forefront-antispam-report":"SFV:NSPM;\n\tSFS:(10009020)(6009001)(376002)(346002)(39860400002)(189002)(13464003)(377454003)(199003)(24454002)(6306002)(2906002)(3660700001)(86362001)(14454004)(93886005)(55016002)(8676002)(8936002)(33656002)(3280700002)(25786009)(99286003)(81166006)(966005)(68736007)(81156014)(478600001)(229853002)(189998001)(4326008)(45080400002)(54906003)(5250100002)(53936002)(316002)(6506006)(97736004)(6436002)(3846002)(7736002)(53546010)(50986999)(5660300001)(7696004)(2950100002)(106356001)(105586002)(2900100001)(6246003)(9686003)(74316002)(101416001)(76176999)(6916009)(7416002)(305945005)(66066001)(54356999)(6116002)(102836003);\n\tDIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR05MB3343;\n\tH:AM4PR0501MB2194.eurprd05.prod.outlook.com; FPR:; SPF:None;\n\tPTR:InfoNoRecords; MX:1; A:1; LANG:en; ","received-spf":"None (protection.outlook.com: mellanox.com does not designate\n\tpermitted sender hosts)","spamdiagnosticoutput":"1:99","spamdiagnosticmetadata":"NSPM","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","MIME-Version":"1.0","X-OriginatorOrg":"Mellanox.com","X-MS-Exchange-CrossTenant-originalarrivaltime":"28 Sep 2017 11:11:42.4581\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"a652971c-7d2e-4d9b-a6a4-d149256f461b","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"VI1PR05MB3343","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1791391,"web_url":"http://patchwork.ozlabs.org/comment/1791391/","msgid":"<20171020115512.GA2073@kroah.com>","list_archive_url":null,"date":"2017-10-20T11:55:12","subject":"Re: [patch v9 1/4] drivers: jtag: Add JTAG core driver","submitter":{"id":11800,"url":"http://patchwork.ozlabs.org/api/people/11800/","name":"Greg Kroah-Hartman","email":"gregkh@linuxfoundation.org"},"content":"On Thu, Sep 21, 2017 at 12:25:29PM +0300, Oleksandr Shamray wrote:\n> +struct jtag {\n> +\tstruct device *dev;\n> +\tstruct cdev cdev;\n\nWhy are you using a cdev here and not just a normal misc device?  I\nforgot if this is what you were doing before, sorry...\n\n> +\tint id;\n> +\tatomic_t open;\n\nWhy do you need this?\n\n> +\tconst struct jtag_ops *ops;\n> +\tunsigned long priv[0] __aligned(ARCH_DMA_MINALIGN);\n\nHuh?  Why is this needed to be dma aligned?  Why not just use the\nprivate pointer in struct device?\n\n> +};\n> +\n> +static dev_t jtag_devt;\n> +static DEFINE_IDA(jtag_ida);\n> +\n> +void *jtag_priv(struct jtag *jtag)\n> +{\n> +\treturn jtag->priv;\n> +}\n> +EXPORT_SYMBOL_GPL(jtag_priv);\n> +\n> +static u8 *jtag_copy_from_user(__u64 udata, unsigned long bit_size)\n> +{\n> +\tunsigned long size;\n> +\tvoid *kdata;\n> +\n> +\tsize = DIV_ROUND_UP(bit_size, BITS_PER_BYTE);\n> +\tkdata = memdup_user(u64_to_user_ptr(udata), size);\n\nYou only use this once, why not just open-code it?\n\n> +\n> +\treturn kdata;\n> +}\n> +\n> +static unsigned long jtag_copy_to_user(__u64 udata, u8 *kdata,\n> +\t\t\t\t       unsigned long bit_size)\n> +{\n> +\tunsigned long size;\n> +\n> +\tsize = DIV_ROUND_UP(bit_size, BITS_PER_BYTE);\n> +\n> +\treturn copy_to_user(u64_to_user_ptr(udata), (void *)(kdata), size);\n\nSame here, making this a separate function seems odd.\n\n> +}\n> +\n> +static struct class jtag_class = {\n> +\t.name = \"jtag\",\n> +\t.owner = THIS_MODULE,\n> +};\n> +\n> +static int jtag_run_test_idle_op(struct jtag *jtag,\n> +\t\t\t\t struct jtag_run_test_idle *idle)\n> +{\n> +\tif (jtag->ops->idle)\n> +\t\treturn jtag->ops->idle(jtag, idle);\n> +\telse\n> +\t\treturn -EOPNOTSUPP;\n\nWhy is this a function?  Why not just do this work in the ioctl?\n\n> +}\n> +\n> +static int jtag_xfer_op(struct jtag *jtag, struct jtag_xfer *xfer, u8 *data)\n> +{\n> +\tif (jtag->ops->xfer)\n> +\t\treturn jtag->ops->xfer(jtag, xfer, data);\n> +\telse\n> +\t\treturn -EOPNOTSUPP;\n\nSame here, doesn't need to be a function.\n\n> +}\n> +\n> +static long jtag_ioctl(struct file *file, unsigned int cmd, unsigned long arg)\n> +{\n> +\tstruct jtag *jtag = file->private_data;\n> +\tstruct jtag_run_test_idle idle;\n> +\tstruct jtag_xfer xfer;\n> +\tu8 *xfer_data;\n> +\tu32 value;\n> +\tint err;\n> +\n> +\tswitch (cmd) {\n> +\tcase JTAG_GIOCFREQ:\n> +\t\tif (jtag->ops->freq_get)\n> +\t\t\terr = jtag->ops->freq_get(jtag, &value);\n> +\t\telse\n> +\t\t\terr = -EOPNOTSUPP;\n> +\t\tif (err)\n> +\t\t\tbreak;\n> +\n> +\t\terr = put_user(value, (__u32 *)arg);\n> +\t\tbreak;\n\nAre you sure the return value of put_user is correct here?  Shouldn't\nyou return -EFAULT if err is not 0?\n\n> +\n> +\tcase JTAG_SIOCFREQ:\n> +\t\terr = get_user(value, (__u32 *)arg);\n> +\n\nwhy a blank line?\n\n> +\t\tif (value == 0)\n> +\t\t\terr = -EINVAL;\n\nCheck err first.\n\n> +\t\tif (err)\n> +\t\t\tbreak;\n\nAnd set it properly, again err should be -EFAULT, right?\n\n> +\n> +\t\tif (jtag->ops->freq_set)\n> +\t\t\terr = jtag->ops->freq_set(jtag, value);\n> +\t\telse\n> +\t\t\terr = -EOPNOTSUPP;\n> +\t\tbreak;\n> +\n> +\tcase JTAG_IOCRUNTEST:\n> +\t\tif (copy_from_user(&idle, (void *)arg,\n> +\t\t\t\t   sizeof(struct jtag_run_test_idle)))\n> +\t\t\treturn -ENOMEM;\n> +\t\terr = jtag_run_test_idle_op(jtag, &idle);\n\nWho validates the structure fields?  Is that up to the individual jtag\ndriver?  Why not do it in the core correctly so that it only has to be\ndone in one place and you do not have to audit every individual driver?\n\n> +\t\tbreak;\n> +\n> +\tcase JTAG_IOCXFER:\n> +\t\tif (copy_from_user(&xfer, (void *)arg,\n> +\t\t\t\t   sizeof(struct jtag_xfer)))\n> +\t\t\treturn -EFAULT;\n> +\n> +\t\tif (xfer.length >= JTAG_MAX_XFER_DATA_LEN)\n> +\t\t\treturn -EFAULT;\n> +\n> +\t\txfer_data = jtag_copy_from_user(xfer.tdio, xfer.length);\n> +\t\tif (!xfer_data)\n> +\t\t\treturn -ENOMEM;\n\nAre you sure that's the correct error value?\n\n> +\n> +\t\terr = jtag_xfer_op(jtag, &xfer, xfer_data);\n> +\t\tif (jtag_copy_to_user(xfer.tdio, xfer_data, xfer.length)) {\n> +\t\t\tkfree(xfer_data);\n> +\t\t\treturn -EFAULT;\n> +\t\t}\n> +\n> +\t\tkfree(xfer_data);\n> +\t\tif (copy_to_user((void *)arg, &xfer, sizeof(struct jtag_xfer)))\n> +\t\t\treturn -EFAULT;\n> +\t\tbreak;\n> +\n> +\tcase JTAG_GIOCSTATUS:\n> +\t\tif (jtag->ops->status_get)\n> +\t\t\terr = jtag->ops->status_get(jtag, &value);\n> +\t\telse\n> +\t\t\terr = -EOPNOTSUPP;\n> +\t\tif (err)\n> +\t\t\tbreak;\n> +\n> +\t\terr = put_user(value, (__u32 *)arg);\n\nSame put_user err issue here.\n\n> +\t\tbreak;\n> +\n> +\tdefault:\n> +\t\treturn -EINVAL;\n> +\t}\n> +\treturn err;\n> +}\n> +\n> +#ifdef CONFIG_COMPAT\n> +static long jtag_ioctl_compat(struct file *file, unsigned int cmd,\n> +\t\t\t      unsigned long arg)\n> +{\n> +\treturn jtag_ioctl(file, cmd, (unsigned long)compat_ptr(arg));\n> +}\n> +#endif\n\nWhy do you need a compat callback for a new ioctl?  Create your\nstructures properly and this should not be needed, right?\n\n> +\n> +static int jtag_open(struct inode *inode, struct file *file)\n> +{\n> +\tstruct jtag *jtag = container_of(inode->i_cdev, struct jtag, cdev);\n> +\n> +\tif (atomic_read(&jtag->open)) {\n> +\t\tdev_info(NULL, \"jtag already opened\\n\");\n> +\t\treturn -EBUSY;\n\nWhy do you care if multiple opens can happen?\n\n> +\t}\n> +\n> +\tatomic_inc(&jtag->open);\n\nOh look, you just raced with two different people opening at the same\ntime :(\n\nAn atomic value is never a replacement for a lock, sorry.\n\n> +\tfile->private_data = jtag;\n> +\treturn 0;\n> +}\n> +\n> +static int jtag_release(struct inode *inode, struct file *file)\n> +{\n> +\tstruct jtag *jtag = file->private_data;\n> +\n> +\tatomic_dec(&jtag->open);\n\nAgain, not needed.\n\n> +\treturn 0;\n> +}\n> +\n> +static const struct file_operations jtag_fops = {\n> +\t.owner\t\t= THIS_MODULE,\n> +\t.open\t\t= jtag_open,\n> +\t.release\t= jtag_release,\n> +\t.llseek\t\t= noop_llseek,\n> +\t.unlocked_ioctl = jtag_ioctl,\n> +#ifdef CONFIG_COMPAT\n> +\t.compat_ioctl\t= jtag_ioctl_compat,\n> +#endif\n> +};\n> +\n> +struct jtag *jtag_alloc(size_t priv_size, const struct jtag_ops *ops)\n> +{\n> +\tstruct jtag *jtag;\n> +\n> +\tjtag = kzalloc(sizeof(*jtag) + round_up(priv_size, ARCH_DMA_MINALIGN),\n> +\t\t       GFP_KERNEL);\n> +\tif (!jtag)\n> +\t\treturn NULL;\n> +\n> +\tjtag->ops = ops;\n> +\treturn jtag;\n> +}\n> +EXPORT_SYMBOL_GPL(jtag_alloc);\n\nregister should allocate and register the device, why pass a structure\nback that the caller then has to do something else with to use it?\n\n> +\n> +void jtag_free(struct jtag *jtag)\n> +{\n> +\tkfree(jtag);\n> +}\n> +EXPORT_SYMBOL_GPL(jtag_free);\n> +\n> +int jtag_register(struct jtag *jtag)\n> +{\n> +\tint id;\n> +\tint err;\n> +\n> +\tid = ida_simple_get(&jtag_ida, 0, 0, GFP_KERNEL);\n> +\tif (id < 0)\n> +\t\treturn id;\n> +\n> +\tjtag->id = id;\n> +\tcdev_init(&jtag->cdev, &jtag_fops);\n> +\tjtag->cdev.owner = THIS_MODULE;\n> +\terr = cdev_add(&jtag->cdev, MKDEV(MAJOR(jtag_devt), jtag->id), 1);\n> +\tif (err)\n> +\t\tgoto err_cdev;\n\nmisc device would be so much simpler and easier here :(\n\n\n> +\n> +\t/* Register this jtag device with the driver core */\n> +\tjtag->dev = device_create(&jtag_class, NULL, MKDEV(MAJOR(jtag_devt),\n> +\t\t\t\t\t\t\t   jtag->id),\n> +\t\t\t\t  NULL, \"jtag%d\", jtag->id);\n> +\tif (!jtag->dev)\n> +\t\tgoto err_device_create;\n> +\n> +\tatomic_set(&jtag->open, 0);\n> +\tdev_set_drvdata(jtag->dev, jtag);\n> +\treturn err;\n> +\n> +err_device_create:\n> +\tcdev_del(&jtag->cdev);\n> +err_cdev:\n> +\tida_simple_remove(&jtag_ida, id);\n> +\treturn err;\n> +}\n> +EXPORT_SYMBOL_GPL(jtag_register);\n> +\n> +void jtag_unregister(struct jtag *jtag)\n> +{\n> +\tstruct device *dev = jtag->dev;\n> +\n> +\tcdev_del(&jtag->cdev);\n> +\tdevice_unregister(dev);\n> +\tida_simple_remove(&jtag_ida, jtag->id);\n> +}\n> +EXPORT_SYMBOL_GPL(jtag_unregister);\n> +\n> +static int __init jtag_init(void)\n> +{\n> +\tint err;\n> +\n> +\terr = alloc_chrdev_region(&jtag_devt, JTAG_FIRST_MINOR,\n> +\t\t\t\t  JTAG_MAX_DEVICES, \"jtag\");\n> +\tif (err)\n> +\t\treturn err;\n> +\n> +\terr = class_register(&jtag_class);\n> +\tif (err)\n> +\t\tunregister_chrdev_region(jtag_devt, JTAG_MAX_DEVICES);\n> +\n> +\treturn err;\n> +}\n> +\n> +static void __exit jtag_exit(void)\n> +{\n> +\tclass_unregister(&jtag_class);\n> +\tunregister_chrdev_region(jtag_devt, JTAG_MAX_DEVICES);\n\nYour idr leaked memory :(\n\n\n\n> +}\n> +\n> +module_init(jtag_init);\n> +module_exit(jtag_exit);\n> +\n> +MODULE_AUTHOR(\"Oleksandr Shamray <oleksandrs@mellanox.com>\");\n> +MODULE_DESCRIPTION(\"Generic jtag support\");\n> +MODULE_LICENSE(\"GPL v2\");\n> diff --git a/include/linux/jtag.h b/include/linux/jtag.h\n> new file mode 100644\n> index 0000000..c016fed\n> --- /dev/null\n> +++ b/include/linux/jtag.h\n> @@ -0,0 +1,48 @@\n> +/*\n> + * drivers/jtag/jtag.c\n> + *\n> + * Copyright (c) 2017 Mellanox Technologies. All rights reserved.\n> + * Copyright (c) 2017 Oleksandr Shamray <oleksandrs@mellanox.com>\n> + *\n> + * Released under the GPLv2 only.\n> + * SPDX-License-Identifier: GPL-2.0\n> + */\n> +\n> +#ifndef __JTAG_H\n> +#define __JTAG_H\n> +\n> +#include <uapi/linux/jtag.h>\n> +\n> +#ifndef ARCH_DMA_MINALIGN\n> +#define ARCH_DMA_MINALIGN 1\n> +#endif\n> +\n> +#define jtag_u64_to_ptr(arg) ((void *)(uintptr_t)arg)\n> +\n> +#define JTAG_MAX_XFER_DATA_LEN 65535\n> +\n> +struct jtag;\n> +/**\n> + * struct jtag_ops - callbacks for jtag control functions:\n> + *\n> + * @freq_get: get frequency function. Filled by device driver\n> + * @freq_set: set frequency function. Filled by device driver\n> + * @status_get: set status function. Filled by device driver\n> + * @idle: set JTAG to idle state function. Filled by device driver\n> + * @xfer: send JTAG xfer function. Filled by device driver\n> + */\n> +struct jtag_ops {\n> +\tint (*freq_get)(struct jtag *jtag, u32 *freq);\n> +\tint (*freq_set)(struct jtag *jtag, u32 freq);\n> +\tint (*status_get)(struct jtag *jtag, u32 *state);\n> +\tint (*idle)(struct jtag *jtag, struct jtag_run_test_idle *idle);\n> +\tint (*xfer)(struct jtag *jtag, struct jtag_xfer *xfer);\n> +};\n> +\n> +void *jtag_priv(struct jtag *jtag);\n> +int jtag_register(struct jtag *jtag);\n> +void jtag_unregister(struct jtag *jtag);\n> +struct jtag *jtag_alloc(size_t priv_size, const struct jtag_ops *ops);\n> +void jtag_free(struct jtag *jtag);\n> +\n> +#endif /* __JTAG_H */\n> diff --git a/include/uapi/linux/jtag.h b/include/uapi/linux/jtag.h\n> new file mode 100644\n> index 0000000..180bf22\n> --- /dev/null\n> +++ b/include/uapi/linux/jtag.h\n> @@ -0,0 +1,115 @@\n> +/*\n> + * JTAG class driver\n> + *\n> + * Copyright (c) 2017 Mellanox Technologies. All rights reserved.\n> + * Copyright (c) 2017 Oleksandr Shamray <oleksandrs@mellanox.com>\n> + *\n> + * Released under the GPLv2/BSD.\n> + * SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause\n> + */\n> +\n> +#ifndef __UAPI_LINUX_JTAG_H\n> +#define __UAPI_LINUX_JTAG_H\n> +\n> +/**\n> + * enum jtag_xfer_mode:\n> + *\n> + * @JTAG_XFER_HW_MODE: hardware mode transfer\n> + * @JTAG_XFER_SW_MODE: software mode transfer\n> + */\n> +enum jtag_xfer_mode {\n> +\tJTAG_XFER_HW_MODE,\n> +\tJTAG_XFER_SW_MODE,\n> +};\n> +\n> +/**\n> + * enum jtag_endstate:\n> + *\n> + * @JTAG_STATE_IDLE: JTAG state machine IDLE state\n> + * @JTAG_STATE_PAUSEIR: JTAG state machine PAUSE_IR state\n> + * @JTAG_STATE_PAUSEDR: JTAG state machine PAUSE_DR state\n> + */\n> +enum jtag_endstate {\n> +\tJTAG_STATE_IDLE,\n> +\tJTAG_STATE_PAUSEIR,\n> +\tJTAG_STATE_PAUSEDR,\n> +};\n> +\n> +/**\n> + * enum jtag_xfer_type:\n> + *\n> + * @JTAG_SIR_XFER: SIR transfer\n> + * @JTAG_SDR_XFER: SDR transfer\n> + */\n> +enum jtag_xfer_type {\n> +\tJTAG_SIR_XFER,\n> +\tJTAG_SDR_XFER,\n> +};\n> +\n> +/**\n> + * enum jtag_xfer_direction:\n> + *\n> + * @JTAG_READ_XFER: read transfer\n> + * @JTAG_WRITE_XFER: write transfer\n> + */\n> +enum jtag_xfer_direction {\n> +\tJTAG_READ_XFER,\n> +\tJTAG_WRITE_XFER,\n> +};\n> +\n> +/**\n> + * struct jtag_run_test_idle - forces JTAG state machine to\n> + * RUN_TEST/IDLE state\n> + *\n> + * @mode: access mode\n> + * @reset: 0 - run IDLE/PAUSE from current state\n> + *         1 - go through TEST_LOGIC/RESET state before  IDLE/PAUSE\n> + * @end: completion flag\n> + * @tck: clock counter\n> + *\n> + * Structure represents interface to JTAG device for jtag idle\n> + * execution.\n> + */\n> +struct jtag_run_test_idle {\n> +\t__u8\tmode;\n> +\t__u8\treset;\n> +\t__u8\tendstate;\n> +\t__u8\ttck;\n> +};\n> +\n> +/**\n> + * struct jtag_xfer - jtag xfer:\n> + *\n> + * @mode: access mode\n> + * @type: transfer type\n> + * @direction: xfer direction\n> + * @length: xfer bits len\n> + * @tdio : xfer data array\n> + * @endir: xfer end state\n> + *\n> + * Structure represents interface to Aspeed JTAG device for jtag sdr xfer\n> + * execution.\n> + */\n> +struct jtag_xfer {\n> +\t__u8\tmode;\n> +\t__u8\ttype;\n> +\t__u8\tdirection;\n> +\t__u8\tendstate;\n> +\t__u32\tlength;\n> +\t__u64\ttdio;\n\nI don't see anything odd here that requires a compat ioctl callback, do\nyou?\n\nthanks,\n\ngreg k-h\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yJPP83nRjz9t5l\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tFri, 20 Oct 2017 22:55:08 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753132AbdJTLzH (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tFri, 20 Oct 2017 07:55:07 -0400","from mail.linuxfoundation.org ([140.211.169.12]:51514 \"EHLO\n\tmail.linuxfoundation.org\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1753125AbdJTLzF (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Fri, 20 Oct 2017 07:55:05 -0400","from localhost (LFbn-1-12253-150.w90-92.abo.wanadoo.fr\n\t[90.92.67.150])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPSA id C24B6BD9;\n\tFri, 20 Oct 2017 11:55:04 +0000 (UTC)"],"Date":"Fri, 20 Oct 2017 13:55:12 +0200","From":"Greg KH <gregkh@linuxfoundation.org>","To":"Oleksandr Shamray <oleksandrs@mellanox.com>","Cc":"arnd@arndb.de, linux-kernel@vger.kernel.org,\n\tlinux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,\n\topenbmc@lists.ozlabs.org, joel@jms.id.au, jiri@resnulli.us,\n\ttklauser@distanz.ch, linux-serial@vger.kernel.org, mec@shout.net,\n\tvadimp@mellanox.com, system-sw-low-level@mellanox.com,\n\trobh+dt@kernel.org, openocd-devel-owner@lists.sourceforge.net,\n\tlinux-api@vger.kernel.org, davem@davemloft.net, mchehab@kernel.org,\n\tJiri Pirko <jiri@mellanox.com>","Subject":"Re: [patch v9 1/4] drivers: jtag: Add JTAG core driver","Message-ID":"<20171020115512.GA2073@kroah.com>","References":"<1505985932-27568-1-git-send-email-oleksandrs@mellanox.com>\n\t<1505985932-27568-2-git-send-email-oleksandrs@mellanox.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<1505985932-27568-2-git-send-email-oleksandrs@mellanox.com>","User-Agent":"Mutt/1.9.1 (2017-09-22)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1791525,"web_url":"http://patchwork.ozlabs.org/comment/1791525/","msgid":"<20171020145417.GD8965@kroah.com>","list_archive_url":null,"date":"2017-10-20T14:54:17","subject":"Re: [patch v9 1/4] drivers: jtag: Add JTAG core driver","submitter":{"id":11800,"url":"http://patchwork.ozlabs.org/api/people/11800/","name":"Greg Kroah-Hartman","email":"gregkh@linuxfoundation.org"},"content":"On Fri, Oct 20, 2017 at 02:34:00PM +0000, Oleksandr Shamray wrote:\n> Hi Greg.\n> \n> > -----Original Message-----\n> > From: Greg KH [mailto:gregkh@linuxfoundation.org]\n> > Sent: Friday, October 20, 2017 2:55 PM\n> > To: Oleksandr Shamray <oleksandrs@mellanox.com>\n> > Cc: arnd@arndb.de; linux-kernel@vger.kernel.org; linux-arm-\n> > kernel@lists.infradead.org; devicetree@vger.kernel.org;\n> > openbmc@lists.ozlabs.org; joel@jms.id.au; jiri@resnulli.us;\n> > tklauser@distanz.ch; linux-serial@vger.kernel.org; mec@shout.net; Vadim\n> > Pasternak <vadimp@mellanox.com>; system-sw-low-level <system-sw-low-\n> > level@mellanox.com>; robh+dt@kernel.org; openocd-devel-\n> > owner@lists.sourceforge.net; linux-api@vger.kernel.org;\n> > davem@davemloft.net; mchehab@kernel.org; Jiri Pirko <jiri@mellanox.com>\n> > Subject: Re: [patch v9 1/4] drivers: jtag: Add JTAG core driver\n> > \n> > On Thu, Sep 21, 2017 at 12:25:29PM +0300, Oleksandr Shamray wrote:\n> > > +struct jtag {\n> > > +\tstruct device *dev;\n> > > +\tstruct cdev cdev;\n> > \n> > Why are you using a cdev here and not just a normal misc device? \n> \n> What the benefits to use misc instead of cdev?\n\nLess code, simpler logic, easier to review and understand, etc.\n\nLet me ask you, why use a cdev instead of a misc?\n\n> > I forgot if this is what you were doing before, sorry...\n> > \n> > > +\tint id;\n> > > +\tatomic_t open;\n> > \n> > Why do you need this?\n> \n> This counter used to avoid open at the same time by 2 or more users.\n\nBut it isn't working :)\n\nAnd why do you care?\n\n> > > +\tconst struct jtag_ops *ops;\n> > > +\tunsigned long priv[0] __aligned(ARCH_DMA_MINALIGN);\n> > \n> > Huh?  Why is this needed to be dma aligned?  Why not just use the private\n> > pointer in struct device?\n> > \n> \n> It is critical?\n\nYou are saying it is, so you have to justify it.  There is a pointer for\nyou to use, don't make new ones for no reason, right?\n\n> > > +};\n> > > +\n> > > +static dev_t jtag_devt;\n> > > +static DEFINE_IDA(jtag_ida);\n> > > +\n> > > +void *jtag_priv(struct jtag *jtag)\n> > > +{\n> > > +\treturn jtag->priv;\n> > > +}\n> > > +EXPORT_SYMBOL_GPL(jtag_priv);\n> > > +\n> > > +static u8 *jtag_copy_from_user(__u64 udata, unsigned long bit_size) {\n> > > +\tunsigned long size;\n> > > +\tvoid *kdata;\n> > > +\n> > > +\tsize = DIV_ROUND_UP(bit_size, BITS_PER_BYTE);\n> > > +\tkdata = memdup_user(u64_to_user_ptr(udata), size);\n> > \n> > You only use this once, why not just open-code it?\n> \n> I think it makes code more understandable.\n\nAs a reviewer, I don't :)\n\n> > > +\n> > > +\treturn kdata;\n> > > +}\n> > > +\n> > > +static unsigned long jtag_copy_to_user(__u64 udata, u8 *kdata,\n> > > +\t\t\t\t       unsigned long bit_size)\n> > > +{\n> > > +\tunsigned long size;\n> > > +\n> > > +\tsize = DIV_ROUND_UP(bit_size, BITS_PER_BYTE);\n> > > +\n> > > +\treturn copy_to_user(u64_to_user_ptr(udata), (void *)(kdata), size);\n> > \n> > Same here, making this a separate function seems odd.\n> \n> Same, I think it makes code more understandable.\n\nBut it doesn't.\n\n> > > +\n> > > +\t\tif (jtag->ops->freq_set)\n> > > +\t\t\terr = jtag->ops->freq_set(jtag, value);\n> > > +\t\telse\n> > > +\t\t\terr = -EOPNOTSUPP;\n> > > +\t\tbreak;\n> > > +\n> > > +\tcase JTAG_IOCRUNTEST:\n> > > +\t\tif (copy_from_user(&idle, (void *)arg,\n> > > +\t\t\t\t   sizeof(struct jtag_run_test_idle)))\n> > > +\t\t\treturn -ENOMEM;\n> > > +\t\terr = jtag_run_test_idle_op(jtag, &idle);\n> > \n> > Who validates the structure fields?  Is that up to the individual jtag driver?  Why\n> > not do it in the core correctly so that it only has to be done in one place and you\n> > do not have to audit every individual driver?\n> \n> Input parameters validated by jtag  platform driver. I think it not critical.\n\nNot true at all.  It is very critical.  Remmeber, \"All Input Is Evil!\"\n\nYou have to validate this.  I as a reviewer have to find where you are\nvalidating this data to ensure bad things do not happen.  I can't review\nthat here, now I have to go and review all of the individual drivers,\nwhich is a major pain, don't you agree?\n\n> > > +\t\tbreak;\n> > > +\n> > > +\tcase JTAG_IOCXFER:\n> > > +\t\tif (copy_from_user(&xfer, (void *)arg,\n> > > +\t\t\t\t   sizeof(struct jtag_xfer)))\n> > > +\t\t\treturn -EFAULT;\n> > > +\n> > > +\t\tif (xfer.length >= JTAG_MAX_XFER_DATA_LEN)\n> > > +\t\t\treturn -EFAULT;\n> > > +\n> > > +\t\txfer_data = jtag_copy_from_user(xfer.tdio, xfer.length);\n> > > +\t\tif (!xfer_data)\n> > > +\t\t\treturn -ENOMEM;\n> > \n> > Are you sure that's the correct error value?\n> \n> I think yes, but what you suggest?\n\nA fault happened, so -EFAULT, right?\n\n> [..]\n> > +       .unlocked_ioctl = jtag_ioctl,\n> > +       .open           = jtag_open,\n> > +       .release        = jtag_release,\n> > +};\n> \n> add a compat_ioctl pointer here, after ensuring that all ioctl\n> commands are compatible between 32-bit and 64-bit user space.\n> [..]\n\nAnd if you do not, what happens?  You shouldn't need it as there is no\nfixups necessary, or am I mistaken about that?\n\n> > > +static int jtag_open(struct inode *inode, struct file *file) {\n> > > +\tstruct jtag *jtag = container_of(inode->i_cdev, struct jtag, cdev);\n> > > +\n> > > +\tif (atomic_read(&jtag->open)) {\n> > > +\t\tdev_info(NULL, \"jtag already opened\\n\");\n> > > +\t\treturn -EBUSY;\n> > \n> > Why do you care if multiple opens can happen?\n> \n> Jtag HW not support to using with multiple requests from different users. So we prohibit this.\n\nWhy does the kernel care?\n\nAnd again, your implementation is broken, it's not actually doing this\nprotection.  I recommend just not doing it at all, but if you really are\ninsisting on it, you have to get it correct :)\n\nthanks,\n\ngreg k-h\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yJTN21lfhz9t5x\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tSat, 21 Oct 2017 01:54:26 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751664AbdJTOyL (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tFri, 20 Oct 2017 10:54:11 -0400","from mail.linuxfoundation.org ([140.211.169.12]:40476 \"EHLO\n\tmail.linuxfoundation.org\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751468AbdJTOyK (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Fri, 20 Oct 2017 10:54:10 -0400","from localhost (LFbn-1-12253-150.w90-92.abo.wanadoo.fr\n\t[90.92.67.150])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPSA id C4D9C88A;\n\tFri, 20 Oct 2017 14:54:08 +0000 (UTC)"],"Date":"Fri, 20 Oct 2017 16:54:17 +0200","From":"Greg KH <gregkh@linuxfoundation.org>","To":"Oleksandr Shamray <oleksandrs@mellanox.com>","Cc":"\"arnd@arndb.de\" <arnd@arndb.de>,\n\t\"linux-kernel@vger.kernel.org\" <linux-kernel@vger.kernel.org>,\n\t\"linux-arm-kernel@lists.infradead.org\" \n\t<linux-arm-kernel@lists.infradead.org>,\n\t\"devicetree@vger.kernel.org\" <devicetree@vger.kernel.org>,\n\t\"openbmc@lists.ozlabs.org\" <openbmc@lists.ozlabs.org>,\n\t\"joel@jms.id.au\" <joel@jms.id.au>,\n\t\"jiri@resnulli.us\" <jiri@resnulli.us>, \n\t\"tklauser@distanz.ch\" <tklauser@distanz.ch>,\n\t\"linux-serial@vger.kernel.org\" <linux-serial@vger.kernel.org>,\n\t\"mec@shout.net\" <mec@shout.net>, Vadim Pasternak <vadimp@mellanox.com>,\n\tsystem-sw-low-level <system-sw-low-level@mellanox.com>,\n\t\"robh+dt@kernel.org\" <robh+dt@kernel.org>,\n\t\"openocd-devel-owner@lists.sourceforge.net\" \n\t<openocd-devel-owner@lists.sourceforge.net>,\n\t\"linux-api@vger.kernel.org\" <linux-api@vger.kernel.org>,\n\t\"davem@davemloft.net\" <davem@davemloft.net>,\n\t\"mchehab@kernel.org\" <mchehab@kernel.org>, Jiri Pirko <jiri@mellanox.com>","Subject":"Re: [patch v9 1/4] drivers: jtag: Add JTAG core driver","Message-ID":"<20171020145417.GD8965@kroah.com>","References":"<1505985932-27568-1-git-send-email-oleksandrs@mellanox.com>\n\t<1505985932-27568-2-git-send-email-oleksandrs@mellanox.com>\n\t<20171020115512.GA2073@kroah.com>\n\t<AM4PR0501MB21940531802FC14EDE8C3F75B1430@AM4PR0501MB2194.eurprd05.prod.outlook.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<AM4PR0501MB21940531802FC14EDE8C3F75B1430@AM4PR0501MB2194.eurprd05.prod.outlook.com>","User-Agent":"Mutt/1.9.1 (2017-09-22)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1793826,"web_url":"http://patchwork.ozlabs.org/comment/1793826/","msgid":"<AM4PR0501MB21942F65C3F28F8AE9B22504B1440@AM4PR0501MB2194.eurprd05.prod.outlook.com>","list_archive_url":null,"date":"2017-10-25T15:58:42","subject":"RE: [patch v9 1/4] drivers: jtag: Add JTAG core driver","submitter":{"id":72099,"url":"http://patchwork.ozlabs.org/api/people/72099/","name":"Oleksandr Shamray","email":"oleksandrs@mellanox.com"},"content":"> -----Original Message-----\n> From: Greg KH [mailto:gregkh@linuxfoundation.org]\n> Sent: Friday, October 20, 2017 5:54 PM\n> To: Oleksandr Shamray <oleksandrs@mellanox.com>\n> Cc: arnd@arndb.de; linux-kernel@vger.kernel.org; linux-arm-\n> kernel@lists.infradead.org; devicetree@vger.kernel.org;\n> openbmc@lists.ozlabs.org; joel@jms.id.au; jiri@resnulli.us;\n> tklauser@distanz.ch; linux-serial@vger.kernel.org; mec@shout.net; Vadim\n> Pasternak <vadimp@mellanox.com>; system-sw-low-level <system-sw-low-\n> level@mellanox.com>; robh+dt@kernel.org; openocd-devel-\n> owner@lists.sourceforge.net; linux-api@vger.kernel.org;\n> davem@davemloft.net; mchehab@kernel.org; Jiri Pirko <jiri@mellanox.com>\n> Subject: Re: [patch v9 1/4] drivers: jtag: Add JTAG core driver\n> \n> On Fri, Oct 20, 2017 at 02:34:00PM +0000, Oleksandr Shamray wrote:\n> > Hi Greg.\n> >\n> > > -----Original Message-----\n> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]\n> > > Sent: Friday, October 20, 2017 2:55 PM\n> > > To: Oleksandr Shamray <oleksandrs@mellanox.com>\n> > > Cc: arnd@arndb.de; linux-kernel@vger.kernel.org; linux-arm-\n> > > kernel@lists.infradead.org; devicetree@vger.kernel.org;\n> > > openbmc@lists.ozlabs.org; joel@jms.id.au; jiri@resnulli.us;\n> > > tklauser@distanz.ch; linux-serial@vger.kernel.org; mec@shout.net;\n> > > Vadim Pasternak <vadimp@mellanox.com>; system-sw-low-level\n> > > <system-sw-low- level@mellanox.com>; robh+dt@kernel.org;\n> > > openocd-devel- owner@lists.sourceforge.net;\n> > > linux-api@vger.kernel.org; davem@davemloft.net; mchehab@kernel.org;\n> > > Jiri Pirko <jiri@mellanox.com>\n> > > Subject: Re: [patch v9 1/4] drivers: jtag: Add JTAG core driver\n> > >\n> > > On Thu, Sep 21, 2017 at 12:25:29PM +0300, Oleksandr Shamray wrote:\n> > > > +struct jtag {\n> > > > +\tstruct device *dev;\n> > > > +\tstruct cdev cdev;\n> > >\n> > > Why are you using a cdev here and not just a normal misc device?\n> >\n> > What the benefits to use misc instead of cdev?\n> \n> Less code, simpler logic, easier to review and understand, etc.\n> \n> Let me ask you, why use a cdev instead of a misc?\n\nAs I know misc device more applicable if we want to create one device f.e.  /dev/jtag. \nBut in current case we can have more than one jtag device /dev/jtag0 ... /dev/jtagN.  \nSo I decided to use cdev.\n\n> \n> > > I forgot if this is what you were doing before, sorry...\n> > >\n> > > > +\tint id;\n> > > > +\tatomic_t open;\n> > >\n> > > Why do you need this?\n> >\n> > This counter used to avoid open at the same time by 2 or more users.\n> \n> But it isn't working :)\n> \n> And why do you care?\n> \n> > > > +\tconst struct jtag_ops *ops;\n> > > > +\tunsigned long priv[0] __aligned(ARCH_DMA_MINALIGN);\n> > >\n> > > Huh?  Why is this needed to be dma aligned?  Why not just use the\n> > > private pointer in struct device?\n> > >\n> >\n> > It is critical?\n> \n> You are saying it is, so you have to justify it.  There is a pointer for you to use,\n> don't make new ones for no reason, right?\n> \n\nYou are right. Will remove.\n\n> > > > +};\n> > > > +\n> > > > +static dev_t jtag_devt;\n> > > > +static DEFINE_IDA(jtag_ida);\n> > > > +\n> > > > +void *jtag_priv(struct jtag *jtag) {\n> > > > +\treturn jtag->priv;\n> > > > +}\n> > > > +EXPORT_SYMBOL_GPL(jtag_priv);\n> > > > +\n> > > > +static u8 *jtag_copy_from_user(__u64 udata, unsigned long bit_size) {\n> > > > +\tunsigned long size;\n> > > > +\tvoid *kdata;\n> > > > +\n> > > > +\tsize = DIV_ROUND_UP(bit_size, BITS_PER_BYTE);\n> > > > +\tkdata = memdup_user(u64_to_user_ptr(udata), size);\n> > >\n> > > You only use this once, why not just open-code it?\n> >\n> > I think it makes code more understandable.\n> \n> As a reviewer, I don't :)\n\nOk, I will fix :)\n\n> \n> > > > +\n> > > > +\treturn kdata;\n> > > > +}\n> > > > +\n> > > > +static unsigned long jtag_copy_to_user(__u64 udata, u8 *kdata,\n> > > > +\t\t\t\t       unsigned long bit_size) {\n> > > > +\tunsigned long size;\n> > > > +\n> > > > +\tsize = DIV_ROUND_UP(bit_size, BITS_PER_BYTE);\n> > > > +\n> > > > +\treturn copy_to_user(u64_to_user_ptr(udata), (void *)(kdata),\n> > > > +size);\n> > >\n> > > Same here, making this a separate function seems odd.\n> >\n> > Same, I think it makes code more understandable.\n> \n> But it doesn't.\n> \n\nOk, I will fix :)\n\n> > > > +\n> > > > +\t\tif (jtag->ops->freq_set)\n> > > > +\t\t\terr = jtag->ops->freq_set(jtag, value);\n> > > > +\t\telse\n> > > > +\t\t\terr = -EOPNOTSUPP;\n> > > > +\t\tbreak;\n> > > > +\n> > > > +\tcase JTAG_IOCRUNTEST:\n> > > > +\t\tif (copy_from_user(&idle, (void *)arg,\n> > > > +\t\t\t\t   sizeof(struct jtag_run_test_idle)))\n> > > > +\t\t\treturn -ENOMEM;\n> > > > +\t\terr = jtag_run_test_idle_op(jtag, &idle);\n> > >\n> > > Who validates the structure fields?  Is that up to the individual\n> > > jtag driver?  Why not do it in the core correctly so that it only\n> > > has to be done in one place and you do not have to audit every individual\n> driver?\n> >\n> > Input parameters validated by jtag  platform driver. I think it not critical.\n> \n> Not true at all.  It is very critical.  Remmeber, \"All Input Is Evil!\"\n> \n> You have to validate this.  I as a reviewer have to find where you are validating\n> this data to ensure bad things do not happen.  I can't review that here, now I\n> have to go and review all of the individual drivers, which is a major pain, don't\n> you agree?\n\nAgree.  I will add input parameter checking here before call device driver.\n\n> \n> > > > +\t\tbreak;\n> > > > +\n> > > > +\tcase JTAG_IOCXFER:\n> > > > +\t\tif (copy_from_user(&xfer, (void *)arg,\n> > > > +\t\t\t\t   sizeof(struct jtag_xfer)))\n> > > > +\t\t\treturn -EFAULT;\n> > > > +\n> > > > +\t\tif (xfer.length >= JTAG_MAX_XFER_DATA_LEN)\n> > > > +\t\t\treturn -EFAULT;\n> > > > +\n> > > > +\t\txfer_data = jtag_copy_from_user(xfer.tdio, xfer.length);\n> > > > +\t\tif (!xfer_data)\n> > > > +\t\t\treturn -ENOMEM;\n> > >\n> > > Are you sure that's the correct error value?\n> >\n> > I think yes, but what you suggest?\n> \n> A fault happened, so -EFAULT, right?\n> \n\nRight.\n\n\n> > [..]\n> > > +       .unlocked_ioctl = jtag_ioctl,\n> > > +       .open           = jtag_open,\n> > > +       .release        = jtag_release,\n> > > +};\n> >\n> > add a compat_ioctl pointer here, after ensuring that all ioctl\n> > commands are compatible between 32-bit and 64-bit user space.\n> > [..]\n> \n> And if you do not, what happens?  You shouldn't need it as there is no fixups\n> necessary, or am I mistaken about that?\n\nYes, you are right. In code compat_ioctl called same function as in unlocked_ioctl. \nSo I can remove compat and system will always call unlocked_ioctl.\n\n> \n> > > > +static int jtag_open(struct inode *inode, struct file *file) {\n> > > > +\tstruct jtag *jtag = container_of(inode->i_cdev, struct jtag,\n> > > > +cdev);\n> > > > +\n> > > > +\tif (atomic_read(&jtag->open)) {\n> > > > +\t\tdev_info(NULL, \"jtag already opened\\n\");\n> > > > +\t\treturn -EBUSY;\n> > >\n> > > Why do you care if multiple opens can happen?\n> >\n> > Jtag HW not support to using with multiple requests from different users. So\n> we prohibit this.\n> \n> Why does the kernel care?\n> \n> And again, your implementation is broken, it's not actually doing this\n> protection.  I recommend just not doing it at all, but if you really are insisting\n> on it, you have to get it correct :)\n\nI will follow your recommendations and remove it. \n\n> \n> thanks,\n> \n> greg k-h\n\nThanks. \nOleksandr S\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=Mellanox.com header.i=@Mellanox.com\n\theader.b=\"FqOiz4gd\"; dkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=oleksandrs@mellanox.com; "],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yMZZQ1t85z9t3C\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tThu, 26 Oct 2017 02:59:10 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751915AbdJYP6x (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tWed, 25 Oct 2017 11:58:53 -0400","from mail-db5eur01on0052.outbound.protection.outlook.com\n\t([104.47.2.52]:34997\n\t\"EHLO EUR01-DB5-obe.outbound.protection.outlook.com\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1751723AbdJYP6v (ORCPT <rfc822;devicetree@vger.kernel.org>);\n\tWed, 25 Oct 2017 11:58:51 -0400","from AM4PR0501MB2194.eurprd05.prod.outlook.com (10.165.82.13) by\n\tHE1PR05MB1756.eurprd05.prod.outlook.com (10.169.120.26) with\n\tMicrosoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id\n\t15.20.156.4; Wed, 25 Oct 2017 15:58:42 +0000","from AM4PR0501MB2194.eurprd05.prod.outlook.com\n\t([fe80::b02a:703f:24d5:1193]) by\n\tAM4PR0501MB2194.eurprd05.prod.outlook.com\n\t([fe80::b02a:703f:24d5:1193%18]) with mapi id 15.20.0156.007;\n\tWed, 25 Oct 2017 15:58:42 +0000"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com;\n\ts=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version;\n\tbh=eNHgtBicbgOFkkBPNPWxWm+dfkXDdjiQxBKBrkoKKrY=;\n\tb=FqOiz4gd+GaM+X4AaoQ3041mwtEYGEaDEjkQvepZBrd8Nk2a+gr8LLD3UT8RYapFVKR1tjwTODKuDfkUcQuBk6YxyP6N6BhzlZuKVUAcHLzUOrx0S5XSX/8fg5t/m4F4Z9YF93pMTBQkzZesq16sSxMr1cGYX3xKbAxnb7wpx+U=","From":"Oleksandr Shamray <oleksandrs@mellanox.com>","To":"Greg KH <gregkh@linuxfoundation.org>","CC":"\"arnd@arndb.de\" <arnd@arndb.de>,\n\t\"linux-kernel@vger.kernel.org\" <linux-kernel@vger.kernel.org>,\n\t\"linux-arm-kernel@lists.infradead.org\" \n\t<linux-arm-kernel@lists.infradead.org>,\n\t\"devicetree@vger.kernel.org\" <devicetree@vger.kernel.org>,\n\t\"openbmc@lists.ozlabs.org\" <openbmc@lists.ozlabs.org>,\n\t\"joel@jms.id.au\" <joel@jms.id.au>,\n\t\"jiri@resnulli.us\" <jiri@resnulli.us>, \n\t\"tklauser@distanz.ch\" <tklauser@distanz.ch>,\n\t\"linux-serial@vger.kernel.org\" <linux-serial@vger.kernel.org>,\n\t\"mec@shout.net\" <mec@shout.net>,\n\t\"Vadim Pasternak\" <vadimp@mellanox.com>, \n\tsystem-sw-low-level <system-sw-low-level@mellanox.com>,\n\t\"robh+dt@kernel.org\" <robh+dt@kernel.org>,\n\t\"openocd-devel-owner@lists.sourceforge.net\" \n\t<openocd-devel-owner@lists.sourceforge.net>,\n\t\"linux-api@vger.kernel.org\" <linux-api@vger.kernel.org>,\n\t\"davem@davemloft.net\" <davem@davemloft.net>,\n\t\"mchehab@kernel.org\" <mchehab@kernel.org>, Jiri Pirko <jiri@mellanox.com>","Subject":"RE: [patch v9 1/4] drivers: jtag: Add JTAG core driver","Thread-Topic":"[patch v9 1/4] drivers: jtag: Add JTAG core driver","Thread-Index":"AQHTMru5yoecjoOKkEeYBkPR/vITOKLszxoAgAAGBCCAACwFgIAH2fow","Date":"Wed, 25 Oct 2017 15:58:42 +0000","Message-ID":"<AM4PR0501MB21942F65C3F28F8AE9B22504B1440@AM4PR0501MB2194.eurprd05.prod.outlook.com>","References":"<1505985932-27568-1-git-send-email-oleksandrs@mellanox.com>\n\t<1505985932-27568-2-git-send-email-oleksandrs@mellanox.com>\n\t<20171020115512.GA2073@kroah.com>\n\t<AM4PR0501MB21940531802FC14EDE8C3F75B1430@AM4PR0501MB2194.eurprd05.prod.outlook.com>\n\t<20171020145417.GD8965@kroah.com>","In-Reply-To":"<20171020145417.GD8965@kroah.com>","Accept-Language":"en-US, uk-UA","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","authentication-results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=Mellanox.com header.i=@Mellanox.com\n\theader.b=\"FqOiz4gd\"; dkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=oleksandrs@mellanox.com; "],"x-originating-ip":"[80.90.224.13]","x-ms-publictraffictype":"Email","x-microsoft-exchange-diagnostics":"1; HE1PR05MB1756;\n\t6:6pH5gR+V5H6qUhYXMqJCd23d09OATvqlQjV2KB8WC8s4D54I0g3GmQAfO6fhP00lIdKZ2XfdmnKaGI8saaLJqdH7hzUJaNd61OCvjRusXofBIp81EnbOgZuM1A0derVxE8A6CQ1eq5YeRJW6K+ABrzEAGC1bZhmikU0cT8VClyEVFSQsGehKYjh4Q64cz7QabHavq+eWpcrMsRJfnoWlZGrLxLlVxvFKIGdtOr209Pf1Ho4IAw1oqa4/1swXb5S6PeJ8G/cA4D5bSgoWyI4No+Xy9fK7WjDgXrc578uyCb8cQ1jAVI8GBMf7Vp9JcgfIO6cZTmjpqp/pVQL8jt0+Kg==;\n\t5:I9Va1Ux3fo8b8NGHFmQhXUphjWkXwlvQ5dhT0VzBRsYbyXrx0WTSbIwZU4RI4T5NGbRlDCyttpewxsTKM/SUIaEmvzUN/Ckdj/pP7OZjdy+Pdp5qzoaGRtzUuI7f1N2aeDYH3uwLEkD0waxUjV0y6w==;\n\t24:ku3mxYI4JWZF0nR7+5Fin93gXi9RCgLx+AMTGIJVo+X2eFzzMV0wUYTTne/NeMP+OhsLZfiOYWKgqswXO7AJZr1jB2Yt56syBrnRVpgAcZs=;\n\t7:vrafflJEzH2R7GkQQdsi0pdy6xuWqomTSEDU5TV8JNDaAkjjftptTqSVh8gslTlbG/p5BMp9WI+Sx83rFVI7TZemnri6PYcUdkTZS93XTiDzZwA5xawFFS8+Khj+5P4oqD7oQQhvBdzHzewpEgPoi6NdnRRcOFa0MDFI5J059hkaR5jocXjn0SKUkLgHmPzaQcYsAFS7dgEBGUPGT8JiWmoqpHmRerkH0YBZXYF+UqU=","x-ms-exchange-antispam-srfa-diagnostics":"SSOS;SSOR;","x-ms-office365-filtering-correlation-id":"a93cf6bd-d920-4b6d-a2cb-08d51bc143e1","x-ms-office365-filtering-ht":"Tenant","x-microsoft-antispam":"UriScan:; BCL:0; PCL:0;\n\tRULEID:(22001)(48565401081)(4534020)(4602075)(4627075)(201703031133081)(201702281549075)(2017052603199);\n\tSRVR:HE1PR05MB1756; ","x-ms-traffictypediagnostic":"HE1PR05MB1756:","x-exchange-antispam-report-test":"UriScan:(143289334528602)(9452136761055)(65623756079841)(258649278758335)(42262312472803);","x-microsoft-antispam-prvs":"<HE1PR05MB1756674151E477C3DB2E8B4BB1440@HE1PR05MB1756.eurprd05.prod.outlook.com>","x-exchange-antispam-report-cfa-test":"BCL:0; PCL:0;\n\tRULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(8121501046)(5005006)(93006095)(93001095)(100000703101)(100105400095)(3002001)(10201501046)(3231020)(6055026)(6041248)(20161123558100)(20161123555025)(20161123564025)(20161123562025)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);\n\tSRVR:HE1PR05MB1756; BCL:0; PCL:0;\n\tRULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);\n\tSRVR:HE1PR05MB1756; ","x-forefront-prvs":"0471B73328","x-forefront-antispam-report":"SFV:NSPM;\n\tSFS:(10009020)(6009001)(39860400002)(376002)(346002)(199003)(189002)(24454002)(13464003)(105586002)(6436002)(74316002)(68736007)(2950100002)(6506006)(66066001)(54356999)(6916009)(14454004)(76176999)(25786009)(305945005)(229853002)(97736004)(3280700002)(50986999)(101416001)(2906002)(3660700001)(33656002)(86362001)(106356001)(7696004)(53546010)(5660300001)(7416002)(3846002)(102836003)(93886005)(2900100001)(7736002)(6116002)(55016002)(8676002)(9686003)(54906003)(107886003)(478600001)(53936002)(99286003)(4326008)(8936002)(81156014)(81166006)(5250100002)(6246003)(189998001)(316002);\n\tDIR:OUT; SFP:1101; SCL:1; SRVR:HE1PR05MB1756;\n\tH:AM4PR0501MB2194.eurprd05.prod.outlook.com; FPR:; SPF:None;\n\tPTR:InfoNoRecords; A:1; MX:1; LANG:en; ","received-spf":"None (protection.outlook.com: mellanox.com does not designate\n\tpermitted sender hosts)","spamdiagnosticoutput":"1:99","spamdiagnosticmetadata":"NSPM","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"quoted-printable","MIME-Version":"1.0","X-OriginatorOrg":"Mellanox.com","X-MS-Exchange-CrossTenant-Network-Message-Id":"a93cf6bd-d920-4b6d-a2cb-08d51bc143e1","X-MS-Exchange-CrossTenant-originalarrivaltime":"25 Oct 2017 15:58:42.2729\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"a652971c-7d2e-4d9b-a6a4-d149256f461b","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"HE1PR05MB1756","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}}]