* [PATCH v2 10/22] docs: vm: hmm.rst: fix some warnings
From: Mauro Carvalho Chehab @ 2019-06-04 14:17 UTC (permalink / raw)
To: Linux Doc Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, Jérôme Glisse, linux-mm
In-Reply-To: <cover.1559656538.git.mchehab+samsung@kernel.org>
Documentation/vm/hmm.rst:292: WARNING: Unexpected indentation.
Documentation/vm/hmm.rst:300: WARNING: Unexpected indentation.
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
Documentation/vm/hmm.rst | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/vm/hmm.rst b/Documentation/vm/hmm.rst
index 7cdf7282e022..f22bb5fb5eec 100644
--- a/Documentation/vm/hmm.rst
+++ b/Documentation/vm/hmm.rst
@@ -283,7 +283,8 @@ The hmm_range struct has 2 fields default_flags and pfn_flags_mask that allows
to set fault or snapshot policy for a whole range instead of having to set them
for each entries in the range.
-For instance if the device flags for device entries are:
+For instance if the device flags for device entries are::
+
VALID (1 << 63)
WRITE (1 << 62)
--
2.21.0
^ permalink raw reply related
* [PATCH v2 17/22] docs: net: sja1105.rst: fix table format
From: Mauro Carvalho Chehab @ 2019-06-04 14:17 UTC (permalink / raw)
To: Linux Doc Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, David S. Miller, Vladimir Oltean,
Florian Fainelli, netdev
In-Reply-To: <cover.1559656538.git.mchehab+samsung@kernel.org>
There's a table there with produces two warnings when built
with Sphinx:
Documentation/networking/dsa/sja1105.rst:91: WARNING: Block quote ends without a blank line; unexpected unindent.
Documentation/networking/dsa/sja1105.rst:91: WARNING: Block quote ends without a blank line; unexpected unindent.
It will still produce a table, but the html output is wrong, as
it won't interpret the second line as the continuation for the
first ones, because identation doesn't match.
After the change, the output looks a way better and we got rid
of two warnings.
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Acked-by: Vladimir Oltean <olteanv@gmail.com>
---
Documentation/networking/dsa/sja1105.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/networking/dsa/sja1105.rst b/Documentation/networking/dsa/sja1105.rst
index ea7bac438cfd..cb2858dece93 100644
--- a/Documentation/networking/dsa/sja1105.rst
+++ b/Documentation/networking/dsa/sja1105.rst
@@ -86,13 +86,13 @@ functionality.
The following traffic modes are supported over the switch netdevices:
+--------------------+------------+------------------+------------------+
-| | Standalone | Bridged with | Bridged with |
-| | ports | vlan_filtering 0 | vlan_filtering 1 |
+| | Standalone | Bridged with | Bridged with |
+| | ports | vlan_filtering 0 | vlan_filtering 1 |
+====================+============+==================+==================+
| Regular traffic | Yes | Yes | No (use master) |
+--------------------+------------+------------------+------------------+
| Management traffic | Yes | Yes | Yes |
-| (BPDU, PTP) | | | |
+| (BPDU, PTP) | | | |
+--------------------+------------+------------------+------------------+
Switching features
--
2.21.0
^ permalink raw reply related
* [PATCH v2 02/22] isdn: mISDN: remove a bogus reference to a non-existing doc
From: Mauro Carvalho Chehab @ 2019-06-04 14:17 UTC (permalink / raw)
To: Linux Doc Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, Karsten Keil, netdev
In-Reply-To: <cover.1559656538.git.mchehab+samsung@kernel.org>
The mISDN driver was added on those commits:
960366cf8dbb ("Add mISDN DSP")
1b2b03f8e514 ("Add mISDN core files")
04578dd330f1 ("Define AF_ISDN and PF_ISDN")
e4ac9bc1f668 ("Add mISDN driver")
None of them added a Documentation/isdn/mISDN.cert file.
Also, whatever were supposed to be written there on that time,
probably doesn't make any sense nowadays, as I doubt isdn would
have any massive changes.
So, let's just get rid of the broken reference, in order to
shut up a warning produced by ./scripts/documentation-file-ref-check.
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
drivers/isdn/mISDN/dsp_core.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/isdn/mISDN/dsp_core.c b/drivers/isdn/mISDN/dsp_core.c
index cd036e87335a..038e72a84b33 100644
--- a/drivers/isdn/mISDN/dsp_core.c
+++ b/drivers/isdn/mISDN/dsp_core.c
@@ -4,8 +4,6 @@
* Karsten Keil (keil@isdn4linux.de)
*
* This file is (c) under GNU PUBLIC LICENSE
- * For changes and modifications please read
- * ../../../Documentation/isdn/mISDN.cert
*
* Thanks to Karsten Keil (great drivers)
* Cologne Chip (great chips)
--
2.21.0
^ permalink raw reply related
* [PATCH v2 22/22] docs: Kbuild/Makefile: allow check for missing docs at build time
From: Mauro Carvalho Chehab @ 2019-06-04 14:17 UTC (permalink / raw)
To: Linux Doc Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, Masahiro Yamada, Randy Dunlap
In-Reply-To: <cover.1559656538.git.mchehab+samsung@kernel.org>
While this doesn't make sense for production Kernels, in order to
avoid regressions when new documents are added, let's add a
check target at the make file.
For now, the only check it does is to verify if there isn't
any documents with a broken link.
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
Documentation/Kconfig | 13 +++++++++++++
Documentation/Makefile | 5 +++++
Kconfig | 2 ++
scripts/documentation-file-ref-check | 9 +++++++++
4 files changed, 29 insertions(+)
create mode 100644 Documentation/Kconfig
diff --git a/Documentation/Kconfig b/Documentation/Kconfig
new file mode 100644
index 000000000000..66046fa1c341
--- /dev/null
+++ b/Documentation/Kconfig
@@ -0,0 +1,13 @@
+config WARN_MISSING_DOCUMENTS
+
+ bool "Warn if there's a missing documentation file"
+ depends on COMPILE_TEST
+ help
+ It is not uncommon that a document gets renamed.
+ This option makes the Kernel to check for missing dependencies,
+ warning when something is missing. Works only if the Kernel
+ is built from a git tree.
+
+ If unsure, select 'N'.
+
+
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 2edd03b1dad6..89857285a024 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -4,6 +4,11 @@
subdir-y := devicetree/bindings/
+# Check for broken documentation file references
+ifeq ($(CONFIG_WARN_MISSING_DOCUMENTS),y)
+$(shell $(srctree)/scripts/documentation-file-ref-check --warn)
+endif
+
# You can set these variables from the command line.
SPHINXBUILD = sphinx-build
SPHINXOPTS =
diff --git a/Kconfig b/Kconfig
index 48a80beab685..990b0c390dfc 100644
--- a/Kconfig
+++ b/Kconfig
@@ -30,3 +30,5 @@ source "crypto/Kconfig"
source "lib/Kconfig"
source "lib/Kconfig.debug"
+
+source "Documentation/Kconfig"
diff --git a/scripts/documentation-file-ref-check b/scripts/documentation-file-ref-check
index ff16db269079..440227bb55a9 100755
--- a/scripts/documentation-file-ref-check
+++ b/scripts/documentation-file-ref-check
@@ -22,9 +22,16 @@ $scriptname =~ s,.*/([^/]+/),$1,;
# Parse arguments
my $help = 0;
my $fix = 0;
+my $warn = 0;
+
+if (! -d ".git") {
+ printf "Warning: can't check if file exists, as this is not a git tree";
+ exit 0;
+}
GetOptions(
'fix' => \$fix,
+ 'warn' => \$warn,
'h|help|usage' => \$help,
);
@@ -139,6 +146,8 @@ while (<IN>) {
if (!($ref =~ m/(scripts|Kconfig|Kbuild)/)) {
$broken_ref{$ref}++;
}
+ } elsif ($warn) {
+ print STDERR "Warning: $f references a file that doesn't exist: $fulref\n";
} else {
print STDERR "$f: $fulref\n";
}
--
2.21.0
^ permalink raw reply related
* [PATCH v2 15/22] docs: security: core.rst: Fix several warnings
From: Mauro Carvalho Chehab @ 2019-06-04 14:17 UTC (permalink / raw)
To: Linux Doc Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, David Howells, keyrings
In-Reply-To: <cover.1559656538.git.mchehab+samsung@kernel.org>
Multi-line literal markups only work when they're idented at the
same level, with is not the case here:
Documentation/security/keys/core.rst:1597: WARNING: Inline literal start-string without end-string.
Documentation/security/keys/core.rst:1597: WARNING: Inline emphasis start-string without end-string.
Documentation/security/keys/core.rst:1597: WARNING: Inline emphasis start-string without end-string.
Documentation/security/keys/core.rst:1598: WARNING: Inline emphasis start-string without end-string.
Documentation/security/keys/core.rst:1598: WARNING: Inline emphasis start-string without end-string.
Documentation/security/keys/core.rst:1600: WARNING: Inline literal start-string without end-string.
Documentation/security/keys/core.rst:1600: WARNING: Inline emphasis start-string without end-string.
Documentation/security/keys/core.rst:1600: WARNING: Inline emphasis start-string without end-string.
Documentation/security/keys/core.rst:1600: WARNING: Inline emphasis start-string without end-string.
Documentation/security/keys/core.rst:1600: WARNING: Inline emphasis start-string without end-string.
Documentation/security/keys/core.rst:1666: WARNING: Inline literal start-string without end-string.
Documentation/security/keys/core.rst:1666: WARNING: Inline emphasis start-string without end-string.
Documentation/security/keys/core.rst:1666: WARNING: Inline emphasis start-string without end-string.
Documentation/security/keys/core.rst:1666: WARNING: Inline emphasis start-string without end-string.
Fix it by using a code-block instead.
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
Documentation/security/keys/core.rst | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/Documentation/security/keys/core.rst b/Documentation/security/keys/core.rst
index 9521c4207f01..3fd60dcb2dc6 100644
--- a/Documentation/security/keys/core.rst
+++ b/Documentation/security/keys/core.rst
@@ -1594,10 +1594,12 @@ The structure has a number of fields, some of which are mandatory:
attempted key link operation. If there is no match, -EINVAL is returned.
- * ``int (*asym_eds_op)(struct kernel_pkey_params *params,
- const void *in, void *out);``
- ``int (*asym_verify_signature)(struct kernel_pkey_params *params,
- const void *in, const void *in2);``
+ * ``asym_eds_op`` and ``asym_verify_signature``::
+
+ int (*asym_eds_op)(struct kernel_pkey_params *params,
+ const void *in, void *out);
+ int (*asym_verify_signature)(struct kernel_pkey_params *params,
+ const void *in, const void *in2);
These methods are optional. If provided the first allows a key to be
used to encrypt, decrypt or sign a blob of data, and the second allows a
@@ -1662,8 +1664,10 @@ The structure has a number of fields, some of which are mandatory:
required crypto isn't available.
- * ``int (*asym_query)(const struct kernel_pkey_params *params,
- struct kernel_pkey_query *info);``
+ * ``asym_query``::
+
+ int (*asym_query)(const struct kernel_pkey_params *params,
+ struct kernel_pkey_query *info);
This method is optional. If provided it allows information about the
public or asymmetric key held in the key to be determined.
--
2.21.0
^ permalink raw reply related
* Re: [alsa-devel] [PATCH v2 13/22] docs: soundwire: locking: fix tags for a code-block
From: Pierre-Louis Bossart @ 2019-06-04 15:12 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Doc Mailing List
Cc: alsa-devel, Jonathan Corbet, linux-kernel, Mauro Carvalho Chehab,
Vinod Koul, Sanyog Kale
In-Reply-To: <0ea9c284f8db3867985c410d2764a2b68e5b35c1.1559656538.git.mchehab+samsung@kernel.org>
On 6/4/19 9:17 AM, Mauro Carvalho Chehab wrote:
> There's an ascii artwork at Example 1 whose code-block is not properly
> idented, causing those warnings.
>
> Documentation/driver-api/soundwire/locking.rst:50: WARNING: Inconsistent literal block quoting.
> Documentation/driver-api/soundwire/locking.rst:51: WARNING: Line block ends without a blank line.
> Documentation/driver-api/soundwire/locking.rst:55: WARNING: Inline substitution_reference start-string without end-string.
> Documentation/driver-api/soundwire/locking.rst:56: WARNING: Line block ends without a blank line.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Thanks for fixing this.
> ---
> Documentation/driver-api/soundwire/locking.rst | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/driver-api/soundwire/locking.rst b/Documentation/driver-api/soundwire/locking.rst
> index 253f73555255..3a7ffb3d87f3 100644
> --- a/Documentation/driver-api/soundwire/locking.rst
> +++ b/Documentation/driver-api/soundwire/locking.rst
> @@ -44,7 +44,9 @@ Message transfer.
> b. Transfer message (Read/Write) to Slave1 or broadcast message on
> Bus in case of bank switch.
>
> - c. Release Message lock ::
> + c. Release Message lock
> +
> + ::
>
> +----------+ +---------+
> | | | |
>
^ permalink raw reply
* Re: [kbuild:kunit 14/17] htmldocs: include/kunit/kunit-stream.h:58: warning: Function parameter or member '2' not described in '__printf'
From: Brendan Higgins @ 2019-06-04 15:15 UTC (permalink / raw)
To: Randy Dunlap
Cc: kbuild test robot, kbuild-all, linux-kbuild, Masahiro Yamada,
Felix Guo, linux-doc@vger.kernel.org, Jonathan Corbet
In-Reply-To: <94e6e17c-e360-f56d-674c-84cc0032ca4a@infradead.org>
On Mon, May 20, 2019 at 9:49 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 5/10/19 2:03 PM, Brendan Higgins wrote:
> > On Thu, May 9, 2019 at 6:23 PM kbuild test robot <lkp@intel.com> wrote:
> >> tree: https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git kunit
> >> head: c505c0b2e6237c729634327c178f5b0094f1c958
> >> commit: c69e87665049970d1c2d6fe2fa1ae7a7c8655420 [14/17] Documentation: kunit: add documentation for KUnit
> >> reproduce: make htmldocs
> >>
> >> If you fix the issue, kindly add following tag
> >> Reported-by: kbuild test robot <lkp@intel.com>
> >>
> >> All warnings (new ones prefixed by >>):
> > < snip >
> >> drivers/gpu/drm/i915/i915_vma.h:50: warning: cannot understand function prototype: 'struct i915_vma '
> >> drivers/gpu/drm/i915/i915_vma.h:1: warning: no structured comments found
> >> drivers/gpu/drm/i915/intel_guc_fwif.h:536: warning: cannot understand function prototype: 'struct guc_log_buffer_state '
> >> drivers/gpu/drm/i915/i915_trace.h:1: warning: no structured comments found
> >> drivers/gpu/drm/i915/i915_reg.h:156: warning: bad line:
> >> include/linux/interconnect.h:1: warning: no structured comments found
> >>>> include/kunit/kunit-stream.h:58: warning: Function parameter or member '2' not described in '__printf'
> >
> > This looks like a bug in the kernel-doc parser: __printf in this context is:
> >
> >> 8dcda743 Brendan Higgins 2019-05-01 56 void __printf(2, 3) kunit_stream_add(struct kunit_stream *this,
> >> 8dcda743 Brendan Higgins 2019-05-01 57 const char *fmt, ...);
> >
> > which is an attribute to tell the compiler that this is a printf style
> > function with a printf style format string; it doesn't make sense to
> > describe it's parameters.
>
> Yes. I'm a little surprised that this has never come up before, but in my
> quick examination, it seems that most people put the __printf() attribute
> in a header file but not in the .c implementation file, then they document
> the function in the .c file, not in the header file, so it hasn't been needed.
>
> Ignoring __printf() could be added to scripts/kernel-doc. I added
> kernel-doc notation to <linux/kernel.h> panic() and then tested it with
> the patch below. ItWorksForMe. :)
>
>
> >> include/kunit/kunit-stream.h:58: warning: Function parameter or member '3' not described in '__printf'
> >> include/kunit/kunit-stream.h:58: warning: Excess function parameter 'this' description in '__printf'
> >> include/kunit/kunit-stream.h:58: warning: Excess function parameter 'fmt' description in '__printf'
> >> include/linux/skbuff.h:897: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff'
> > < snip >
> >> fs/debugfs/file.c:439: WARNING: Inline literal start-string without end-string.
> >>
> >> vim +58 include/kunit/kunit-stream.h
> >>
> >> 8dcda743 Brendan Higgins 2019-05-01 48
> >> 8dcda743 Brendan Higgins 2019-05-01 49 /**
> >> 8dcda743 Brendan Higgins 2019-05-01 50 * kunit_stream_add(): adds the formatted input to the internal buffer.
> >> 8dcda743 Brendan Higgins 2019-05-01 51 * @this: the stream being operated on.
> >> 8dcda743 Brendan Higgins 2019-05-01 52 * @fmt: printf style format string to append to stream.
> >> 8dcda743 Brendan Higgins 2019-05-01 53 *
> >> 8dcda743 Brendan Higgins 2019-05-01 54 * Appends the formatted string, @fmt, to the internal buffer.
> >> 8dcda743 Brendan Higgins 2019-05-01 55 */
> >> 8dcda743 Brendan Higgins 2019-05-01 56 void __printf(2, 3) kunit_stream_add(struct kunit_stream *this,
> >> 8dcda743 Brendan Higgins 2019-05-01 57 const char *fmt, ...);
> >> 8dcda743 Brendan Higgins 2019-05-01 @58
> >>
> >> :::::: The code at line 58 was first introduced by commit
> >> :::::: 8dcda743c31c1ffc0ac13f3d23f3dd1b85b545f8 kunit: test: add kunit_stream a std::stream like logger
> >
> > Thanks!
>
>
> ---
> From: Randy Dunlap <rdunlap@infradead.org>
>
> Ignore __printf() function attributes just as other __attribute__
> strings are ignored.
>
> Fixes this kernel-doc warning message:
> include/kunit/kunit-stream.h:58: warning: Function parameter or member '2' not described in '__printf'
>
> Reported-by: kbuild test robot <lkp@intel.com>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Brendan Higgins <brendanhiggins@google.com>
> ---
Thanks! I just tried out your patch and it looks like it works.
Tested-by: Brendan Higgins <brendanhiggins@google.com>
^ permalink raw reply
* [PATCH 03/10] mfd / platform: cros_ec: Miscellaneous character device to talk with the EC
From: Enric Balletbo i Serra @ 2019-06-04 15:20 UTC (permalink / raw)
To: linux-kernel
Cc: gwendal, Guenter Roeck, Benson Leung, Lee Jones, kernel, dtor,
Gustavo Pimentel, Randy Dunlap, Lorenzo Pieralisi, linux-doc,
Enno Luebbers, Guido Kiener, Thomas Gleixner,
Kishon Vijay Abraham I, Jonathan Corbet, Wu Hao, Kate Stewart,
Greg Kroah-Hartman, Tycho Andersen, Gerd Hoffmann,
Jilayne Lovejoy
In-Reply-To: <20190604152019.16100-1-enric.balletbo@collabora.com>
That's a driver to talk with the ChromeOS Embedded Controller via a
miscellaneous character device, it creates an entry in /dev for every
instance and implements basic file operations for communicating with the
Embedded Controller with an userspace application. The API is moved to
the uapi folder, which is supposed to contain the user space API of the
kernel.
Note that this will replace current character device interface
implemented in the cros-ec-dev driver in the MFD subsystem. The idea is
to move all the functionality that extends the bounds of what MFD was
designed to platform/chrome subsystem.
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Documentation/ioctl/ioctl-number.txt | 2 +-
drivers/mfd/cros_ec_dev.c | 2 +-
drivers/platform/chrome/Kconfig | 11 +
drivers/platform/chrome/Makefile | 1 +
drivers/platform/chrome/cros_ec_chardev.c | 279 ++++++++++++++++++
.../uapi/linux/cros_ec_chardev.h | 18 +-
6 files changed, 302 insertions(+), 11 deletions(-)
create mode 100644 drivers/platform/chrome/cros_ec_chardev.c
rename drivers/mfd/cros_ec_dev.h => include/uapi/linux/cros_ec_chardev.h (70%)
diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index c9558146ac58..8bd7907ee36d 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -340,7 +340,7 @@ Code Seq#(hex) Include File Comments
0xDD 00-3F ZFCP device driver see drivers/s390/scsi/
<mailto:aherrman@de.ibm.com>
0xE5 00-3F linux/fuse.h
-0xEC 00-01 drivers/platform/chrome/cros_ec_dev.h ChromeOS EC driver
+0xEC 00-01 include/uapi/linux/cros_ec_chardev.h ChromeOS EC driver
0xF3 00-3F drivers/usb/misc/sisusbvga/sisusb.h sisfb (in development)
<mailto:thomas@winischhofer.net>
0xF4 00-1F video/mbxfb.h mbxfb
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 607383b67cf1..11b791c28f84 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -15,7 +15,7 @@
#include <linux/slab.h>
#include <linux/uaccess.h>
-#include "cros_ec_dev.h"
+#include <uapi/linux/cros_ec_chardev.h>
#define DRV_NAME "cros-ec-dev"
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 9417b982ad92..3a9ad001838a 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -147,6 +147,17 @@ config CROS_KBD_LED_BACKLIGHT
To compile this driver as a module, choose M here: the
module will be called cros_kbd_led_backlight.
+config CROS_EC_CHARDEV
+ tristate "ChromeOS EC miscdevice"
+ depends on MFD_CROS_EC_CHARDEV
+ default MFD_CROS_EC_CHARDEV
+ help
+ This driver adds file operations support to talk with the
+ ChromeOS EC from userspace via a character device.
+
+ To compile this driver as a module, choose M here: the
+ module will be called cros_ec_chardev.
+
config CROS_EC_LIGHTBAR
tristate "Chromebook Pixel's lightbar support"
depends on MFD_CROS_EC_CHARDEV
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index ebb57e21923b..d47a7e1097ee 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -16,6 +16,7 @@ cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC) += cros_ec_lpc_mec.o
obj-$(CONFIG_CROS_EC_LPC) += cros_ec_lpcs.o
obj-$(CONFIG_CROS_EC_PROTO) += cros_ec_proto.o cros_ec_trace.o
obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT) += cros_kbd_led_backlight.o
+obj-$(CONFIG_CROS_EC_CHARDEV) += cros_ec_chardev.o
obj-$(CONFIG_CROS_EC_LIGHTBAR) += cros_ec_lightbar.o
obj-$(CONFIG_CROS_EC_VBC) += cros_ec_vbc.o
obj-$(CONFIG_CROS_EC_DEBUGFS) += cros_ec_debugfs.o
diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c
new file mode 100644
index 000000000000..1a0a27080026
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_chardev.c
@@ -0,0 +1,279 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Miscellaneous character driver for ChromeOS Embedded Controller
+ *
+ * Copyright 2019 Google LLC
+ */
+
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/list.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+
+#include <uapi/linux/cros_ec_chardev.h>
+
+#define DRV_NAME "cros-ec-chardev"
+
+static LIST_HEAD(chardev_devices);
+static DEFINE_SPINLOCK(chardev_lock);
+
+struct chardev_data {
+ struct list_head list;
+ struct cros_ec_dev *ec_dev;
+ struct miscdevice misc;
+};
+
+static int ec_get_version(struct cros_ec_dev *ec, char *str, int maxlen)
+{
+ static const char * const current_image_name[] = {
+ "unknown", "read-only", "read-write", "invalid",
+ };
+ struct ec_response_get_version *resp;
+ struct cros_ec_command *msg;
+ int ret;
+
+ msg = kzalloc(sizeof(*msg) + sizeof(*resp), GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ msg->command = EC_CMD_GET_VERSION + ec->cmd_offset;
+ msg->insize = sizeof(*resp);
+
+ ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+ if (ret < 0) {
+ snprintf(str, maxlen,
+ "Unknown EC version, returned error: %d\n",
+ msg->result);
+ goto exit;
+ }
+
+ resp = (struct ec_response_get_version *)msg->data;
+ if (resp->current_image >= ARRAY_SIZE(current_image_name))
+ resp->current_image = 3; /* invalid */
+
+ snprintf(str, maxlen, "%s\n%s\n%s\n",
+ resp->version_string_ro,
+ resp->version_string_rw,
+ current_image_name[resp->current_image]);
+
+ ret = 0;
+exit:
+ kfree(msg);
+ return ret;
+}
+
+/*
+ * Device file ops
+ */
+static int cros_ec_chardev_open(struct inode *inode, struct file *filp)
+{
+ struct miscdevice *mdev = filp->private_data;
+ struct cros_ec_dev *ec_dev = dev_get_drvdata(mdev->parent);
+
+ filp->private_data = ec_dev;
+ nonseekable_open(inode, filp);
+
+ return 0;
+}
+
+static ssize_t cros_ec_chardev_read(struct file *filp, char __user *buffer,
+ size_t length, loff_t *offset)
+{
+ char msg[sizeof(struct ec_response_get_version) +
+ sizeof(CROS_EC_DEV_VERSION)];
+ struct cros_ec_dev *ec = filp->private_data;
+ size_t count;
+ int ret;
+
+ if (*offset != 0)
+ return 0;
+
+ ret = ec_get_version(ec, msg, sizeof(msg));
+ if (ret)
+ return ret;
+
+ count = min(length, strlen(msg));
+
+ if (copy_to_user(buffer, msg, count))
+ return -EFAULT;
+
+ *offset = count;
+ return count;
+}
+
+/*
+ * Ioctls
+ */
+static long cros_ec_chardev_ioctl_xcmd(struct cros_ec_dev *ec, void __user *arg)
+{
+ struct cros_ec_command *s_cmd;
+ struct cros_ec_command u_cmd;
+ long ret;
+
+ if (copy_from_user(&u_cmd, arg, sizeof(u_cmd)))
+ return -EFAULT;
+
+ if (u_cmd.outsize > EC_MAX_MSG_BYTES ||
+ u_cmd.insize > EC_MAX_MSG_BYTES)
+ return -EINVAL;
+
+ s_cmd = kmalloc(sizeof(*s_cmd) + max(u_cmd.outsize, u_cmd.insize),
+ GFP_KERNEL);
+ if (!s_cmd)
+ return -ENOMEM;
+
+ if (copy_from_user(s_cmd, arg, sizeof(*s_cmd) + u_cmd.outsize)) {
+ ret = -EFAULT;
+ goto exit;
+ }
+
+ if (u_cmd.outsize != s_cmd->outsize ||
+ u_cmd.insize != s_cmd->insize) {
+ ret = -EINVAL;
+ goto exit;
+ }
+
+ s_cmd->command += ec->cmd_offset;
+ ret = cros_ec_cmd_xfer(ec->ec_dev, s_cmd);
+ /* Only copy data to userland if data was received. */
+ if (ret < 0)
+ goto exit;
+
+ if (copy_to_user(arg, s_cmd, sizeof(*s_cmd) + s_cmd->insize))
+ ret = -EFAULT;
+exit:
+ kfree(s_cmd);
+ return ret;
+}
+
+static long cros_ec_chardev_ioctl_readmem(struct cros_ec_dev *ec,
+ void __user *arg)
+{
+ struct cros_ec_device *ec_dev = ec->ec_dev;
+ struct cros_ec_readmem s_mem = { };
+ long num;
+
+ /* Not every platform supports direct reads */
+ if (!ec_dev->cmd_readmem)
+ return -ENOTTY;
+
+ if (copy_from_user(&s_mem, arg, sizeof(s_mem)))
+ return -EFAULT;
+
+ num = ec_dev->cmd_readmem(ec_dev, s_mem.offset, s_mem.bytes,
+ s_mem.buffer);
+ if (num <= 0)
+ return num;
+
+ if (copy_to_user((void __user *)arg, &s_mem, sizeof(s_mem)))
+ return -EFAULT;
+
+ return num;
+}
+
+static long cros_ec_chardev_ioctl(struct file *filp, unsigned int cmd,
+ unsigned long arg)
+{
+ struct cros_ec_dev *ec = filp->private_data;
+
+ if (_IOC_TYPE(cmd) != CROS_EC_DEV_IOC)
+ return -ENOTTY;
+
+ switch (cmd) {
+ case CROS_EC_DEV_IOCXCMD:
+ return cros_ec_chardev_ioctl_xcmd(ec, (void __user *)arg);
+ case CROS_EC_DEV_IOCRDMEM:
+ return cros_ec_chardev_ioctl_readmem(ec, (void __user *)arg);
+ }
+
+ return -ENOTTY;
+}
+
+static const struct file_operations chardev_fops = {
+ .open = cros_ec_chardev_open,
+ .read = cros_ec_chardev_read,
+ .unlocked_ioctl = cros_ec_chardev_ioctl,
+#ifdef CONFIG_COMPAT
+ .compat_ioctl = cros_ec_chardev_ioctl,
+#endif
+};
+
+static int cros_ec_chardev_probe(struct platform_device *pdev)
+{
+ struct cros_ec_dev *ec_dev = dev_get_drvdata(pdev->dev.parent);
+ struct cros_ec_platform *ec_platform = dev_get_platdata(ec_dev->dev);
+ struct chardev_data *data;
+ int ret;
+
+ /* Create a char device: we want to create it anew */
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->ec_dev = ec_dev;
+ data->misc.minor = MISC_DYNAMIC_MINOR;
+ data->misc.fops = &chardev_fops;
+ data->misc.name = ec_platform->ec_name;
+ data->misc.parent = pdev->dev.parent;
+
+ ret = misc_register(&data->misc);
+ if (ret)
+ return ret;
+
+ spin_lock(&chardev_lock);
+ list_add(&data->list, &chardev_devices);
+ spin_unlock(&chardev_lock);
+
+ dev_info(&pdev->dev, "Created misc device /dev/%s\n",
+ data->misc.name);
+
+ return 0;
+}
+
+static int cros_ec_chardev_remove(struct platform_device *pdev)
+{
+ struct cros_ec_dev *ec_dev = dev_get_drvdata(pdev->dev.parent);
+ struct chardev_data *data;
+
+ list_for_each_entry(data, &chardev_devices, list)
+ if (data->ec_dev == ec_dev)
+ break;
+
+ if (data->ec_dev != ec_dev) {
+ dev_err(&pdev->dev,
+ "remove called but miscdevice %s not found\n",
+ data->misc.name);
+ return -ENODEV;
+ }
+
+ spin_lock(&chardev_lock);
+ list_del(&data->list);
+ spin_unlock(&chardev_lock);
+ misc_deregister(&data->misc);
+
+ return 0;
+}
+
+static struct platform_driver cros_ec_chardev_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ },
+ .probe = cros_ec_chardev_probe,
+ .remove = cros_ec_chardev_remove,
+};
+
+module_platform_driver(cros_ec_chardev_driver);
+
+MODULE_ALIAS("platform:" DRV_NAME);
+MODULE_AUTHOR("Enric Balletbo i Serra <enric.balletbo@collabora.com>");
+MODULE_DESCRIPTION("ChromeOS EC Miscellaneous Character Driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/cros_ec_dev.h b/include/uapi/linux/cros_ec_chardev.h
similarity index 70%
rename from drivers/mfd/cros_ec_dev.h
rename to include/uapi/linux/cros_ec_chardev.h
index 7a42c3ef50e4..c6dd2549a2a5 100644
--- a/drivers/mfd/cros_ec_dev.h
+++ b/include/uapi/linux/cros_ec_chardev.h
@@ -1,12 +1,12 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
/*
- * cros_ec_dev - expose the Chrome OS Embedded Controller to userspace
+ * ChromeOS EC device interface.
*
* Copyright (C) 2014 Google, Inc.
*/
-#ifndef _CROS_EC_DEV_H_
-#define _CROS_EC_DEV_H_
+#ifndef _UAPI_LINUX_CROS_EC_DEV_H_
+#define _UAPI_LINUX_CROS_EC_DEV_H_
#include <linux/ioctl.h>
#include <linux/types.h>
@@ -20,16 +20,16 @@
* @bytes: Number of bytes to read. Zero means "read a string" (including '\0')
* At most only EC_MEMMAP_SIZE bytes can be read.
* @buffer: Where to store the result. The ioctl returns the number of bytes
- * read or negative on error.
+ * read or negative on error.
*/
struct cros_ec_readmem {
- uint32_t offset;
- uint32_t bytes;
- uint8_t buffer[EC_MEMMAP_SIZE];
+ __u32 offset;
+ __u32 bytes;
+ __u8 buffer[EC_MEMMAP_SIZE];
};
#define CROS_EC_DEV_IOC 0xEC
#define CROS_EC_DEV_IOCXCMD _IOWR(CROS_EC_DEV_IOC, 0, struct cros_ec_command)
#define CROS_EC_DEV_IOCRDMEM _IOWR(CROS_EC_DEV_IOC, 1, struct cros_ec_readmem)
-#endif /* _CROS_EC_DEV_H_ */
+#endif /* _UAPI_LINUX_CROS_EC_DEV_H_ */
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2 13/22] docs: soundwire: locking: fix tags for a code-block
From: Vinod Koul @ 2019-06-04 15:19 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, Sanyog Kale, Pierre-Louis Bossart, alsa-devel
In-Reply-To: <0ea9c284f8db3867985c410d2764a2b68e5b35c1.1559656538.git.mchehab+samsung@kernel.org>
On 04-06-19, 11:17, Mauro Carvalho Chehab wrote:
> There's an ascii artwork at Example 1 whose code-block is not properly
> idented, causing those warnings.
>
> Documentation/driver-api/soundwire/locking.rst:50: WARNING: Inconsistent literal block quoting.
> Documentation/driver-api/soundwire/locking.rst:51: WARNING: Line block ends without a blank line.
> Documentation/driver-api/soundwire/locking.rst:55: WARNING: Inline substitution_reference start-string without end-string.
> Documentation/driver-api/soundwire/locking.rst:56: WARNING: Line block ends without a blank line.
Applied, thanks
--
~Vinod
^ permalink raw reply
* Re: [PATCH 03/10] mfd / platform: cros_ec: Miscellaneous character device to talk with the EC
From: Greg Kroah-Hartman @ 2019-06-04 15:52 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: linux-kernel, gwendal, Guenter Roeck, Benson Leung, Lee Jones,
kernel, dtor, Gustavo Pimentel, Randy Dunlap, Lorenzo Pieralisi,
linux-doc, Enno Luebbers, Guido Kiener, Thomas Gleixner,
Kishon Vijay Abraham I, Jonathan Corbet, Wu Hao, Kate Stewart,
Tycho Andersen, Gerd Hoffmann, Jilayne Lovejoy
In-Reply-To: <20190604152019.16100-4-enric.balletbo@collabora.com>
On Tue, Jun 04, 2019 at 05:20:12PM +0200, Enric Balletbo i Serra wrote:
> That's a driver to talk with the ChromeOS Embedded Controller via a
> miscellaneous character device, it creates an entry in /dev for every
> instance and implements basic file operations for communicating with the
> Embedded Controller with an userspace application. The API is moved to
> the uapi folder, which is supposed to contain the user space API of the
> kernel.
>
> Note that this will replace current character device interface
> implemented in the cros-ec-dev driver in the MFD subsystem. The idea is
> to move all the functionality that extends the bounds of what MFD was
> designed to platform/chrome subsystem.
>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>
> Documentation/ioctl/ioctl-number.txt | 2 +-
> drivers/mfd/cros_ec_dev.c | 2 +-
> drivers/platform/chrome/Kconfig | 11 +
> drivers/platform/chrome/Makefile | 1 +
> drivers/platform/chrome/cros_ec_chardev.c | 279 ++++++++++++++++++
> .../uapi/linux/cros_ec_chardev.h | 18 +-
> 6 files changed, 302 insertions(+), 11 deletions(-)
> create mode 100644 drivers/platform/chrome/cros_ec_chardev.c
> rename drivers/mfd/cros_ec_dev.h => include/uapi/linux/cros_ec_chardev.h (70%)
>
> diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
> index c9558146ac58..8bd7907ee36d 100644
> --- a/Documentation/ioctl/ioctl-number.txt
> +++ b/Documentation/ioctl/ioctl-number.txt
> @@ -340,7 +340,7 @@ Code Seq#(hex) Include File Comments
> 0xDD 00-3F ZFCP device driver see drivers/s390/scsi/
> <mailto:aherrman@de.ibm.com>
> 0xE5 00-3F linux/fuse.h
> -0xEC 00-01 drivers/platform/chrome/cros_ec_dev.h ChromeOS EC driver
> +0xEC 00-01 include/uapi/linux/cros_ec_chardev.h ChromeOS EC driver
> 0xF3 00-3F drivers/usb/misc/sisusbvga/sisusb.h sisfb (in development)
> <mailto:thomas@winischhofer.net>
> 0xF4 00-1F video/mbxfb.h mbxfb
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index 607383b67cf1..11b791c28f84 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -15,7 +15,7 @@
> #include <linux/slab.h>
> #include <linux/uaccess.h>
>
> -#include "cros_ec_dev.h"
> +#include <uapi/linux/cros_ec_chardev.h>
>
> #define DRV_NAME "cros-ec-dev"
>
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index 9417b982ad92..3a9ad001838a 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -147,6 +147,17 @@ config CROS_KBD_LED_BACKLIGHT
> To compile this driver as a module, choose M here: the
> module will be called cros_kbd_led_backlight.
>
> +config CROS_EC_CHARDEV
> + tristate "ChromeOS EC miscdevice"
> + depends on MFD_CROS_EC_CHARDEV
> + default MFD_CROS_EC_CHARDEV
> + help
> + This driver adds file operations support to talk with the
> + ChromeOS EC from userspace via a character device.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called cros_ec_chardev.
> +
> config CROS_EC_LIGHTBAR
> tristate "Chromebook Pixel's lightbar support"
> depends on MFD_CROS_EC_CHARDEV
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index ebb57e21923b..d47a7e1097ee 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -16,6 +16,7 @@ cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC) += cros_ec_lpc_mec.o
> obj-$(CONFIG_CROS_EC_LPC) += cros_ec_lpcs.o
> obj-$(CONFIG_CROS_EC_PROTO) += cros_ec_proto.o cros_ec_trace.o
> obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT) += cros_kbd_led_backlight.o
> +obj-$(CONFIG_CROS_EC_CHARDEV) += cros_ec_chardev.o
> obj-$(CONFIG_CROS_EC_LIGHTBAR) += cros_ec_lightbar.o
> obj-$(CONFIG_CROS_EC_VBC) += cros_ec_vbc.o
> obj-$(CONFIG_CROS_EC_DEBUGFS) += cros_ec_debugfs.o
> diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c
> new file mode 100644
> index 000000000000..1a0a27080026
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_ec_chardev.c
> @@ -0,0 +1,279 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Miscellaneous character driver for ChromeOS Embedded Controller
> + *
> + * Copyright 2019 Google LLC
> + */
> +
> +#include <linux/init.h>
> +#include <linux/device.h>
> +#include <linux/fs.h>
> +#include <linux/list.h>
> +#include <linux/mfd/cros_ec.h>
> +#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +#include <linux/uaccess.h>
> +
> +#include <uapi/linux/cros_ec_chardev.h>
> +
> +#define DRV_NAME "cros-ec-chardev"
> +
> +static LIST_HEAD(chardev_devices);
> +static DEFINE_SPINLOCK(chardev_lock);
> +
> +struct chardev_data {
> + struct list_head list;
> + struct cros_ec_dev *ec_dev;
> + struct miscdevice misc;
> +};
> +
> +static int ec_get_version(struct cros_ec_dev *ec, char *str, int maxlen)
> +{
> + static const char * const current_image_name[] = {
> + "unknown", "read-only", "read-write", "invalid",
> + };
> + struct ec_response_get_version *resp;
> + struct cros_ec_command *msg;
> + int ret;
> +
> + msg = kzalloc(sizeof(*msg) + sizeof(*resp), GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +
> + msg->command = EC_CMD_GET_VERSION + ec->cmd_offset;
> + msg->insize = sizeof(*resp);
> +
> + ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
> + if (ret < 0) {
> + snprintf(str, maxlen,
> + "Unknown EC version, returned error: %d\n",
> + msg->result);
> + goto exit;
> + }
> +
> + resp = (struct ec_response_get_version *)msg->data;
> + if (resp->current_image >= ARRAY_SIZE(current_image_name))
> + resp->current_image = 3; /* invalid */
> +
> + snprintf(str, maxlen, "%s\n%s\n%s\n",
> + resp->version_string_ro,
> + resp->version_string_rw,
> + current_image_name[resp->current_image]);
> +
> + ret = 0;
> +exit:
> + kfree(msg);
> + return ret;
> +}
> +
> +/*
> + * Device file ops
> + */
> +static int cros_ec_chardev_open(struct inode *inode, struct file *filp)
> +{
> + struct miscdevice *mdev = filp->private_data;
> + struct cros_ec_dev *ec_dev = dev_get_drvdata(mdev->parent);
> +
> + filp->private_data = ec_dev;
> + nonseekable_open(inode, filp);
> +
> + return 0;
> +}
> +
> +static ssize_t cros_ec_chardev_read(struct file *filp, char __user *buffer,
> + size_t length, loff_t *offset)
> +{
> + char msg[sizeof(struct ec_response_get_version) +
> + sizeof(CROS_EC_DEV_VERSION)];
> + struct cros_ec_dev *ec = filp->private_data;
> + size_t count;
> + int ret;
> +
> + if (*offset != 0)
> + return 0;
> +
> + ret = ec_get_version(ec, msg, sizeof(msg));
> + if (ret)
> + return ret;
> +
> + count = min(length, strlen(msg));
> +
> + if (copy_to_user(buffer, msg, count))
> + return -EFAULT;
> +
> + *offset = count;
> + return count;
> +}
> +
> +/*
> + * Ioctls
> + */
> +static long cros_ec_chardev_ioctl_xcmd(struct cros_ec_dev *ec, void __user *arg)
> +{
> + struct cros_ec_command *s_cmd;
> + struct cros_ec_command u_cmd;
> + long ret;
> +
> + if (copy_from_user(&u_cmd, arg, sizeof(u_cmd)))
> + return -EFAULT;
> +
> + if (u_cmd.outsize > EC_MAX_MSG_BYTES ||
> + u_cmd.insize > EC_MAX_MSG_BYTES)
> + return -EINVAL;
> +
> + s_cmd = kmalloc(sizeof(*s_cmd) + max(u_cmd.outsize, u_cmd.insize),
> + GFP_KERNEL);
> + if (!s_cmd)
> + return -ENOMEM;
> +
> + if (copy_from_user(s_cmd, arg, sizeof(*s_cmd) + u_cmd.outsize)) {
> + ret = -EFAULT;
> + goto exit;
> + }
> +
> + if (u_cmd.outsize != s_cmd->outsize ||
> + u_cmd.insize != s_cmd->insize) {
> + ret = -EINVAL;
> + goto exit;
> + }
> +
> + s_cmd->command += ec->cmd_offset;
> + ret = cros_ec_cmd_xfer(ec->ec_dev, s_cmd);
> + /* Only copy data to userland if data was received. */
> + if (ret < 0)
> + goto exit;
> +
> + if (copy_to_user(arg, s_cmd, sizeof(*s_cmd) + s_cmd->insize))
> + ret = -EFAULT;
> +exit:
> + kfree(s_cmd);
> + return ret;
> +}
> +
> +static long cros_ec_chardev_ioctl_readmem(struct cros_ec_dev *ec,
> + void __user *arg)
> +{
> + struct cros_ec_device *ec_dev = ec->ec_dev;
> + struct cros_ec_readmem s_mem = { };
> + long num;
> +
> + /* Not every platform supports direct reads */
> + if (!ec_dev->cmd_readmem)
> + return -ENOTTY;
> +
> + if (copy_from_user(&s_mem, arg, sizeof(s_mem)))
> + return -EFAULT;
> +
> + num = ec_dev->cmd_readmem(ec_dev, s_mem.offset, s_mem.bytes,
> + s_mem.buffer);
> + if (num <= 0)
> + return num;
> +
> + if (copy_to_user((void __user *)arg, &s_mem, sizeof(s_mem)))
> + return -EFAULT;
> +
> + return num;
> +}
> +
> +static long cros_ec_chardev_ioctl(struct file *filp, unsigned int cmd,
> + unsigned long arg)
> +{
> + struct cros_ec_dev *ec = filp->private_data;
> +
> + if (_IOC_TYPE(cmd) != CROS_EC_DEV_IOC)
> + return -ENOTTY;
> +
> + switch (cmd) {
> + case CROS_EC_DEV_IOCXCMD:
> + return cros_ec_chardev_ioctl_xcmd(ec, (void __user *)arg);
> + case CROS_EC_DEV_IOCRDMEM:
> + return cros_ec_chardev_ioctl_readmem(ec, (void __user *)arg);
> + }
> +
> + return -ENOTTY;
> +}
> +
> +static const struct file_operations chardev_fops = {
> + .open = cros_ec_chardev_open,
> + .read = cros_ec_chardev_read,
> + .unlocked_ioctl = cros_ec_chardev_ioctl,
> +#ifdef CONFIG_COMPAT
> + .compat_ioctl = cros_ec_chardev_ioctl,
> +#endif
> +};
> +
> +static int cros_ec_chardev_probe(struct platform_device *pdev)
> +{
> + struct cros_ec_dev *ec_dev = dev_get_drvdata(pdev->dev.parent);
> + struct cros_ec_platform *ec_platform = dev_get_platdata(ec_dev->dev);
> + struct chardev_data *data;
> + int ret;
> +
> + /* Create a char device: we want to create it anew */
> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->ec_dev = ec_dev;
> + data->misc.minor = MISC_DYNAMIC_MINOR;
> + data->misc.fops = &chardev_fops;
> + data->misc.name = ec_platform->ec_name;
> + data->misc.parent = pdev->dev.parent;
> +
> + ret = misc_register(&data->misc);
> + if (ret)
> + return ret;
> +
> + spin_lock(&chardev_lock);
> + list_add(&data->list, &chardev_devices);
> + spin_unlock(&chardev_lock);
> +
> + dev_info(&pdev->dev, "Created misc device /dev/%s\n",
> + data->misc.name);
No need to be noisy, if all goes well, your code should be quiet.
> +
> + return 0;
> +}
> +
> +static int cros_ec_chardev_remove(struct platform_device *pdev)
> +{
> + struct cros_ec_dev *ec_dev = dev_get_drvdata(pdev->dev.parent);
> + struct chardev_data *data;
> +
> + list_for_each_entry(data, &chardev_devices, list)
> + if (data->ec_dev == ec_dev)
> + break;
> +
> + if (data->ec_dev != ec_dev) {
> + dev_err(&pdev->dev,
> + "remove called but miscdevice %s not found\n",
> + data->misc.name);
> + return -ENODEV;
> + }
Why do you have this separate list of devices? You don't seem to need
it, you only iterate over it, why is it needed?
> + spin_lock(&chardev_lock);
> + list_del(&data->list);
> + spin_unlock(&chardev_lock);
> + misc_deregister(&data->misc);
> +
> + return 0;
> +}
You also iterate over the list without the lock, so why even have the
lock? Are you sure the list, and the lock, is even needed?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 03/10] mfd / platform: cros_ec: Miscellaneous character device to talk with the EC
From: Ezequiel Garcia @ 2019-06-04 16:58 UTC (permalink / raw)
To: Greg Kroah-Hartman, Enric Balletbo i Serra
Cc: linux-kernel, gwendal, Guenter Roeck, Benson Leung, Lee Jones,
kernel, dtor, Gustavo Pimentel, Randy Dunlap, Lorenzo Pieralisi,
linux-doc, Enno Luebbers, Guido Kiener, Thomas Gleixner,
Kishon Vijay Abraham I, Jonathan Corbet, Wu Hao, Kate Stewart,
Tycho Andersen, Gerd Hoffmann, Jilayne Lovejoy
In-Reply-To: <20190604155228.GB9981@kroah.com>
Hey Greg,
> > + dev_info(&pdev->dev, "Created misc device /dev/%s\n",
> > + data->misc.name);
>
> No need to be noisy, if all goes well, your code should be quiet.
>
I sometimes wonder about this being noise or not, so I will slightly
hijack this thread for this discussion.
From a kernel developer point-of-view, or even from a platform
developer or user with a debugging hat point-of-view, having
a "device created" or "device registered" message is often very useful.
In fact, I wish people would do this more often, so I don't have to
deal with dynamic debug, or hack my way:
diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index 4589631798c9..473549b26bb2 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -603,7 +603,7 @@ static int ov5647_probe(struct i2c_client *client,
if (ret < 0)
goto error;
- dev_dbg(dev, "OmniVision OV5647 camera driver probed\n");
+ dev_info(dev, "OmniVision OV5647 camera driver probed\n");
return 0;
error:
media_entity_cleanup(&sd->entity);
In some subsystems, it's even a behavior I'm more or less relying on:
$ git grep v4l2_info.*registered drivers/media/ | wc -l
26
And on the downsides, I can't find much. It's just one little line,
that is not even noticed unless you have logging turned on.
Thanks,
Ezequiel
^ permalink raw reply related
* Re: [PATCH v2] kbuild: add support for ensuring headers are self-contained
From: Sam Ravnborg @ 2019-06-04 17:25 UTC (permalink / raw)
To: Jani Nikula
Cc: Masahiro Yamada, linux-kbuild, Randy Dunlap, Arnd Bergmann,
David Howells, Chris Wilson, Michal Marek, linux-doc,
linux-kernel, Jonathan Corbet
In-Reply-To: <20190604124248.5564-1-jani.nikula@intel.com>
On Tue, Jun 04, 2019 at 03:42:48PM +0300, Jani Nikula wrote:
> Sometimes it's useful to be able to explicitly ensure certain headers
> remain self-contained, i.e. that they are compilable as standalone
> units, by including and/or forward declaring everything they depend on.
>
> Add special target header-test-y where individual Makefiles can add
> headers to be tested if CONFIG_HEADER_TEST is enabled. This will
> generate a dummy C file per header that gets built as part of extra-y.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Michal Marek <michal.lkml@markovi.net>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Looks good, thanks.
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
^ permalink raw reply
* Re: [PATCH v12] dm: add support to directly boot to a mapped device
From: Helen Koike @ 2019-06-04 17:38 UTC (permalink / raw)
To: Stephen Boyd, dm-devel
Cc: wad, keescook, snitzer, linux-doc, richard.weinberger,
linux-kernel, linux-lvm, enric.balletbo, kernel, agk
In-Reply-To: <5cf5a724.1c69fb81.1e8f0.08fb@mx.google.com>
Hi Stephen,
On 6/3/19 8:02 PM, Stephen Boyd wrote:
> Quoting Helen Koike (2019-02-21 12:33:34)
>> Add a "create" module parameter, which allows device-mapper targets to be
>> configured at boot time. This enables early use of dm targets in the boot
>> process (as the root device or otherwise) without the need of an initramfs.
>>
>> The syntax used in the boot param is based on the concise format from the
>> dmsetup tool to follow the rule of least surprise:
>>
>> sudo dmsetup table --concise /dev/mapper/lroot
>>
>> Which is:
>> dm-mod.create=<name>,<uuid>,<minor>,<flags>,<table>[,<table>+][;<name>,<uuid>,<minor>,<flags>,<table>[,<table>+]+]
>>
>> Where,
>> <name> ::= The device name.
>> <uuid> ::= xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx | ""
>> <minor> ::= The device minor number | ""
>> <flags> ::= "ro" | "rw"
>> <table> ::= <start_sector> <num_sectors> <target_type> <target_args>
>> <target_type> ::= "verity" | "linear" | ...
>>
>> For example, the following could be added in the boot parameters:
>> dm-mod.create="lroot,,,rw, 0 4096 linear 98:16 0, 4096 4096 linear 98:32 0" root=/dev/dm-0
>>
>> Only the targets that were tested are allowed and the ones that doesn't
>> change any block device when the dm is create as read-only. For example,
>> mirror and cache targets are not allowed. The rationale behind this is
>> that if the user makes a mistake, choosing the wrong device to be the
>> mirror or the cache can corrupt data.
>>
>> The only targets allowed are:
>> * crypt
>> * delay
>> * linear
>> * snapshot-origin
>> * striped
>> * verity
>>
>> Co-developed-by: Will Drewry <wad@chromium.org>
>> Co-developed-by: Kees Cook <keescook@chromium.org>
>> Co-developed-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> Signed-off-by: Helen Koike <helen.koike@collabora.com>
>>
>> ---
>>
>
> I'm trying to boot a mainline linux kernel on a chromeos device with dm
> verity and a USB stick but it's not working for me even with this patch.
> I've had to hack around two problems:
>
> 1) rootwait isn't considered
>
> 2) verity doesn't seem to accept UUID for <hash_dev> or <dev>
>
> For the first problem, it happens every boot for me because I'm trying
> to boot off of a USB stick and it's behind a hub that takes a few
> seconds to enumerate. If I hack up the code to call dm_init_init() after
> the 'rootdelay' cmdline parameter is used then I can make this work. It
> would be much nicer if the whole mechanism didn't use a late initcall
> though. If it used a hook from prepare_namespace() and then looped
> waiting for devices to create when rootwait was specified it would work.
The patch was implemented with late initcall partially to be contained
in drivers/md/*, but to support rootwait, adding a hook from
prepare_namespace seems the way to go indeed.
>
> The second problem is that in chromeos we have the bootloader fill out
> the UUID of the kernel partition (%U) and then we have another parameter
> that indicates the offset from that kernel partition to add to the
> kernel partition (typically 1, i.e. PARTNROFF=1) to find the root
> filesystem partition. The way verity seems to work here is that we need
> to specify a path like /dev/sda3 or the major:minor number of the device
> on the commandline to make this work. It would be better if we could add
> in support for the PARTNROFF style that name_to_dev_t() handles so we
> can specify the root partition like we're currently doing. I suspect we
> should be able to add support for this into the device mapper layer so
> that we can specify devices this way.
hmm, I didn't test this yet but at least from what I can see in the
code, verity_ctr() calls dm_get_device() that ends up calling
name_to_dev_t() which should take care of PARTNROFF, this requires a bit
more investigation.
>
> If it helps, an example commandline I've been using to test out a usb
> stick is as follows:
>
> dm-mod.create="vroot,,0,ro, 0 4710400 verity 0 8:19 8:19 4096 4096 588800 588800 sha1 9b0a223aedbf74b06442b0f05fbff33c55edd010 414b21fba60a1901e23aec373e994942e991d6762631e54a39bc42411f244bd2"
Thanks
>
> Also, the documentation (Documentation/device-mapper/dm-init.txt) says
> we can use a way that doesn't specify so many arguments, but dm verity
> complains about not enough arguments (10) when following the example:
>
> vroot,,,ro,
> 0 1740800 verity 254:0 254:0 1740800 sha1
> 76e9be054b15884a9fa85973e9cb274c93afadb6
> 5b3549d54d6c7a3837b9b81ed72e49463a64c03680c47835bef94d768e5646fe;
>
> So the documentation needs an update?
>
Ack, I'll update this.
Thanks
Helen
^ permalink raw reply
* Re: [PATCH 01/15] kbuild: remove headers_{install,check}_all
From: Sam Ravnborg @ 2019-06-04 17:57 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, Randy Dunlap, Arnd Bergmann, David Howells,
Jani Nikula, linux-doc, linux-kernel, Jonathan Corbet,
Michal Marek
In-Reply-To: <20190604101409.2078-2-yamada.masahiro@socionext.com>
On Tue, Jun 04, 2019 at 07:13:55PM +0900, Masahiro Yamada wrote:
> headers_install_all does not make much sense any more because different
> architectures export different set of uapi/linux/ headers. As you see
> in include/uapi/linux/Kbuild, the installation of a.out.h, kvm.h, and
> kvm_para.h is arch-dependent. So, headers_install_all repeats the
> installation/removal of them.
>
> If somebody really thinks it is useful to do headers_install for all
> architectures, it would be possible by small shell-scripting, but the
> top Makefile do not have to provide entry targets just for that purpose.
Agreed, good to see this be dropped.
Acked-by: Sam Ravnborg <sam@ravnborg.org>
^ permalink raw reply
* [PATCH] Documentation/dm-init: fix multi device example
From: Helen Koike @ 2019-06-04 18:27 UTC (permalink / raw)
To: dm-devel, swboyd
Cc: wad, keescook, snitzer, linux-doc, richard.weinberger,
linux-kernel, linux-lvm, enric.balletbo, kernel, agk, Helen Koike,
Jonathan Corbet
The example in the docs regarding multiple device-mappers is invalid (it
has a wrong number of arguments), it's a left over from previous
versions of the patch.
Replace the example with an valid and tested one.
Signed-off-by: Helen Koike <helen.koike@collabora.com>
---
Documentation/device-mapper/dm-init.txt | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/Documentation/device-mapper/dm-init.txt b/Documentation/device-mapper/dm-init.txt
index 8464ee7c01b8..130b3c3679c5 100644
--- a/Documentation/device-mapper/dm-init.txt
+++ b/Documentation/device-mapper/dm-init.txt
@@ -74,13 +74,13 @@ this target to /dev/mapper/lroot (depending on the rules). No uuid was assigned.
An example of multiple device-mappers, with the dm-mod.create="..." contents is shown here
split on multiple lines for readability:
- vroot,,,ro,
- 0 1740800 verity 254:0 254:0 1740800 sha1
- 76e9be054b15884a9fa85973e9cb274c93afadb6
- 5b3549d54d6c7a3837b9b81ed72e49463a64c03680c47835bef94d768e5646fe;
- vram,,,rw,
- 0 32768 linear 1:0 0,
- 32768 32768 linear 1:1 0
+ dm-linear,,1,rw,
+ 0 32768 linear 8:1 0,
+ 32768 1024000 linear 8:2 0;
+ dm-verity,,3,ro,
+ 0 1638400 verity 1 /dev/sdc1 /dev/sdc2 4096 4096 204800 1 sha256
+ ac87db56303c9c1da433d7209b5a6ef3e4779df141200cbd7c157dcb8dd89c42
+ 5ebfe87f7df3235b80a117ebc4078e44f55045487ad4a96581d1adb564615b51
Other examples (per target):
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 03/10] mfd / platform: cros_ec: Miscellaneous character device to talk with the EC
From: Greg Kroah-Hartman @ 2019-06-04 18:35 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: Enric Balletbo i Serra, linux-kernel, gwendal, Guenter Roeck,
Benson Leung, Lee Jones, kernel, dtor, Gustavo Pimentel,
Randy Dunlap, Lorenzo Pieralisi, linux-doc, Enno Luebbers,
Guido Kiener, Thomas Gleixner, Kishon Vijay Abraham I,
Jonathan Corbet, Wu Hao, Kate Stewart, Tycho Andersen,
Gerd Hoffmann, Jilayne Lovejoy
In-Reply-To: <beaf3554bb85974eb118d7722ca55f1823b1850c.camel@collabora.com>
On Tue, Jun 04, 2019 at 01:58:38PM -0300, Ezequiel Garcia wrote:
> Hey Greg,
>
> > > + dev_info(&pdev->dev, "Created misc device /dev/%s\n",
> > > + data->misc.name);
> >
> > No need to be noisy, if all goes well, your code should be quiet.
> >
>
> I sometimes wonder about this being noise or not, so I will slightly
> hijack this thread for this discussion.
>
> >From a kernel developer point-of-view, or even from a platform
> developer or user with a debugging hat point-of-view, having
> a "device created" or "device registered" message is often very useful.
For you, yes. For someone with 30000 devices attached to their system,
it is not, and causes booting to take longer than it should be.
> In fact, I wish people would do this more often, so I don't have to
> deal with dynamic debug, or hack my way:
>
> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> index 4589631798c9..473549b26bb2 100644
> --- a/drivers/media/i2c/ov5647.c
> +++ b/drivers/media/i2c/ov5647.c
> @@ -603,7 +603,7 @@ static int ov5647_probe(struct i2c_client *client,
> if (ret < 0)
> goto error;
>
> - dev_dbg(dev, "OmniVision OV5647 camera driver probed\n");
> + dev_info(dev, "OmniVision OV5647 camera driver probed\n");
> return 0;
> error:
> media_entity_cleanup(&sd->entity);
>
> In some subsystems, it's even a behavior I'm more or less relying on:
>
> $ git grep v4l2_info.*registered drivers/media/ | wc -l
> 26
>
> And on the downsides, I can't find much. It's just one little line,
> that is not even noticed unless you have logging turned on.
Its better to be quiet, which is why the "default driver registration"
macros do not have any printk messages in them. When converting drivers
over to it, we made the boot process much more sane, don't try to go and
add messages for no good reason back in please.
dynamic debugging can be enabled on a module and line-by-line basis,
even from the boot command line. So if you need debugging, you can
always ask someone to just reboot or unload/load the module and get the
message that way.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 03/10] mfd / platform: cros_ec: Miscellaneous character device to talk with the EC
From: Guenter Roeck @ 2019-06-04 18:39 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Ezequiel Garcia, Enric Balletbo i Serra, linux-kernel,
Gwendal Grignou, Guenter Roeck, Benson Leung, Lee Jones, kernel,
Dmitry Torokhov, Gustavo Pimentel, Randy Dunlap,
Lorenzo Pieralisi, linux-doc, Enno Luebbers, Guido Kiener,
Thomas Gleixner, Kishon Vijay Abraham I, Jonathan Corbet, Wu Hao,
Kate Stewart, Tycho Andersen, Gerd Hoffmann, Jilayne Lovejoy
In-Reply-To: <20190604183527.GA20098@kroah.com>
On Tue, Jun 4, 2019 at 11:35 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Jun 04, 2019 at 01:58:38PM -0300, Ezequiel Garcia wrote:
> > Hey Greg,
> >
> > > > + dev_info(&pdev->dev, "Created misc device /dev/%s\n",
> > > > + data->misc.name);
> > >
> > > No need to be noisy, if all goes well, your code should be quiet.
> > >
> >
> > I sometimes wonder about this being noise or not, so I will slightly
> > hijack this thread for this discussion.
> >
> > >From a kernel developer point-of-view, or even from a platform
> > developer or user with a debugging hat point-of-view, having
> > a "device created" or "device registered" message is often very useful.
>
> For you, yes. For someone with 30000 devices attached to their system,
> it is not, and causes booting to take longer than it should be.
>
> > In fact, I wish people would do this more often, so I don't have to
> > deal with dynamic debug, or hack my way:
> >
> > diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> > index 4589631798c9..473549b26bb2 100644
> > --- a/drivers/media/i2c/ov5647.c
> > +++ b/drivers/media/i2c/ov5647.c
> > @@ -603,7 +603,7 @@ static int ov5647_probe(struct i2c_client *client,
> > if (ret < 0)
> > goto error;
> >
> > - dev_dbg(dev, "OmniVision OV5647 camera driver probed\n");
> > + dev_info(dev, "OmniVision OV5647 camera driver probed\n");
> > return 0;
> > error:
> > media_entity_cleanup(&sd->entity);
> >
> > In some subsystems, it's even a behavior I'm more or less relying on:
> >
> > $ git grep v4l2_info.*registered drivers/media/ | wc -l
> > 26
> >
> > And on the downsides, I can't find much. It's just one little line,
> > that is not even noticed unless you have logging turned on.
>
> Its better to be quiet, which is why the "default driver registration"
> macros do not have any printk messages in them. When converting drivers
> over to it, we made the boot process much more sane, don't try to go and
> add messages for no good reason back in please.
>
> dynamic debugging can be enabled on a module and line-by-line basis,
> even from the boot command line. So if you need debugging, you can
> always ask someone to just reboot or unload/load the module and get the
> message that way.
>
Can we by any chance make this an official policy ? I am kind of tired
having to argue about this over and over again.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH 03/10] mfd / platform: cros_ec: Miscellaneous character device to talk with the EC
From: Greg Kroah-Hartman @ 2019-06-04 18:59 UTC (permalink / raw)
To: Guenter Roeck
Cc: Ezequiel Garcia, Enric Balletbo i Serra, linux-kernel,
Gwendal Grignou, Guenter Roeck, Benson Leung, Lee Jones, kernel,
Dmitry Torokhov, Gustavo Pimentel, Randy Dunlap,
Lorenzo Pieralisi, linux-doc, Enno Luebbers, Guido Kiener,
Thomas Gleixner, Kishon Vijay Abraham I, Jonathan Corbet, Wu Hao,
Kate Stewart, Tycho Andersen, Gerd Hoffmann, Jilayne Lovejoy
In-Reply-To: <CABXOdTfU9KaBDhQcwvBGWCmVfnd02_ZFmPGtJsCtGQ-iO9A3Qw@mail.gmail.com>
On Tue, Jun 04, 2019 at 11:39:21AM -0700, Guenter Roeck wrote:
> On Tue, Jun 4, 2019 at 11:35 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Jun 04, 2019 at 01:58:38PM -0300, Ezequiel Garcia wrote:
> > > Hey Greg,
> > >
> > > > > + dev_info(&pdev->dev, "Created misc device /dev/%s\n",
> > > > > + data->misc.name);
> > > >
> > > > No need to be noisy, if all goes well, your code should be quiet.
> > > >
> > >
> > > I sometimes wonder about this being noise or not, so I will slightly
> > > hijack this thread for this discussion.
> > >
> > > >From a kernel developer point-of-view, or even from a platform
> > > developer or user with a debugging hat point-of-view, having
> > > a "device created" or "device registered" message is often very useful.
> >
> > For you, yes. For someone with 30000 devices attached to their system,
> > it is not, and causes booting to take longer than it should be.
> >
> > > In fact, I wish people would do this more often, so I don't have to
> > > deal with dynamic debug, or hack my way:
> > >
> > > diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> > > index 4589631798c9..473549b26bb2 100644
> > > --- a/drivers/media/i2c/ov5647.c
> > > +++ b/drivers/media/i2c/ov5647.c
> > > @@ -603,7 +603,7 @@ static int ov5647_probe(struct i2c_client *client,
> > > if (ret < 0)
> > > goto error;
> > >
> > > - dev_dbg(dev, "OmniVision OV5647 camera driver probed\n");
> > > + dev_info(dev, "OmniVision OV5647 camera driver probed\n");
> > > return 0;
> > > error:
> > > media_entity_cleanup(&sd->entity);
> > >
> > > In some subsystems, it's even a behavior I'm more or less relying on:
> > >
> > > $ git grep v4l2_info.*registered drivers/media/ | wc -l
> > > 26
> > >
> > > And on the downsides, I can't find much. It's just one little line,
> > > that is not even noticed unless you have logging turned on.
> >
> > Its better to be quiet, which is why the "default driver registration"
> > macros do not have any printk messages in them. When converting drivers
> > over to it, we made the boot process much more sane, don't try to go and
> > add messages for no good reason back in please.
> >
> > dynamic debugging can be enabled on a module and line-by-line basis,
> > even from the boot command line. So if you need debugging, you can
> > always ask someone to just reboot or unload/load the module and get the
> > message that way.
> >
>
> Can we by any chance make this an official policy ? I am kind of tired
> having to argue about this over and over again.
Sure, but how does anyone make any "official policy" in the kernel? :)
I could just go through and delete all "look ma, a new driver/device!"
messages, but that might be annoying...
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH RFC] Rough draft document on merging and rebasing
From: Jonathan Corbet @ 2019-06-04 19:08 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Linus Torvalds, LKML, linux-doc
In-Reply-To: <20190601154248.GA17800@mit.edu>
On Sat, 1 Jun 2019 11:42:48 -0400
"Theodore Ts'o" <tytso@mit.edu> wrote:
> Finally, I'm bit concerned about anything which states absolutes,
> because there are people who tend to be real stickler for the rules,
> and if they see something stated in absolute terms, they fail to
> understand that there are exceptions that are well understood, and in
> use for years before the existence of the document which is trying to
> codify best practices.
Hence the "there are exceptions" text at the bottom of the document :)
Anyway, I'll rework it to try to take your comments into account. Maybe
we should consistently say "rebasing" for changing the parent commit of a
patch set, and "history modification" for the other tricks...?
Thanks for taking a look,
jon
^ permalink raw reply
* Re: [PATCH v12] dm: add support to directly boot to a mapped device
From: Ezequiel Garcia @ 2019-06-04 19:21 UTC (permalink / raw)
To: Helen Koike, Stephen Boyd, dm-devel
Cc: wad, keescook, snitzer, linux-doc, richard.weinberger,
linux-kernel, linux-lvm, enric.balletbo, kernel, agk
In-Reply-To: <d6b4fb26-9a1b-0acd-ce4a-e48322a17e7d@collabora.com>
Hi Stephen,
On Tue, 2019-06-04 at 14:38 -0300, Helen Koike wrote:
> Hi Stephen,
>
> On 6/3/19 8:02 PM, Stephen Boyd wrote:
> > Quoting Helen Koike (2019-02-21 12:33:34)
> > > Add a "create" module parameter, which allows device-mapper targets to be
> > > configured at boot time. This enables early use of dm targets in the boot
> > > process (as the root device or otherwise) without the need of an initramfs.
> > >
> > > The syntax used in the boot param is based on the concise format from the
> > > dmsetup tool to follow the rule of least surprise:
> > >
> > > sudo dmsetup table --concise /dev/mapper/lroot
> > >
> > > Which is:
> > > dm-mod.create=<name>,<uuid>,<minor>,<flags>,<table>[,<table>+][;<name>,<uuid>,<minor>,<flags>,<table>[,<table>+]+]
> > >
> > > Where,
> > > <name> ::= The device name.
> > > <uuid> ::= xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx | ""
> > > <minor> ::= The device minor number | ""
> > > <flags> ::= "ro" | "rw"
> > > <table> ::= <start_sector> <num_sectors> <target_type> <target_args>
> > > <target_type> ::= "verity" | "linear" | ...
> > >
> > > For example, the following could be added in the boot parameters:
> > > dm-mod.create="lroot,,,rw, 0 4096 linear 98:16 0, 4096 4096 linear 98:32 0" root=/dev/dm-0
> > >
> > > Only the targets that were tested are allowed and the ones that doesn't
> > > change any block device when the dm is create as read-only. For example,
> > > mirror and cache targets are not allowed. The rationale behind this is
> > > that if the user makes a mistake, choosing the wrong device to be the
> > > mirror or the cache can corrupt data.
> > >
> > > The only targets allowed are:
> > > * crypt
> > > * delay
> > > * linear
> > > * snapshot-origin
> > > * striped
> > > * verity
> > >
> > > Co-developed-by: Will Drewry <wad@chromium.org>
> > > Co-developed-by: Kees Cook <keescook@chromium.org>
> > > Co-developed-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > > Signed-off-by: Helen Koike <helen.koike@collabora.com>
> > >
> > > ---
> > >
> >
> > I'm trying to boot a mainline linux kernel on a chromeos device with dm
> > verity and a USB stick but it's not working for me even with this patch.
> > I've had to hack around two problems:
> >
> > 1) rootwait isn't considered
> >
> > 2) verity doesn't seem to accept UUID for <hash_dev> or <dev>
> >
> > For the first problem, it happens every boot for me because I'm trying
> > to boot off of a USB stick and it's behind a hub that takes a few
> > seconds to enumerate. If I hack up the code to call dm_init_init() after
> > the 'rootdelay' cmdline parameter is used then I can make this work. It
> > would be much nicer if the whole mechanism didn't use a late initcall
> > though. If it used a hook from prepare_namespace() and then looped
> > waiting for devices to create when rootwait was specified it would work.
>
> The patch was implemented with late initcall partially to be contained
> in drivers/md/*, but to support rootwait, adding a hook from
> prepare_namespace seems the way to go indeed.
>
Thanks for bringing this up.
Helen and I have been looking at this code, and we think it's possible
to move things around and add some helpers, so we can implement rootwait
behavior, without actually cluttering init/do_mounts.c.
And along the way, we might get the chance to clean-up this code even
further.
Regards,
Eze
^ permalink raw reply
* Re: [PATCH RFC] Rough draft document on merging and rebasing
From: Geert Uytterhoeven @ 2019-06-04 19:32 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Theodore Ts'o, Linus Torvalds, LKML, open list:DOCUMENTATION
In-Reply-To: <20190604130837.24ea1d7b@lwn.net>
Hi Jon,
On Tue, Jun 4, 2019 at 9:09 PM Jonathan Corbet <corbet@lwn.net> wrote:
> On Sat, 1 Jun 2019 11:42:48 -0400
> "Theodore Ts'o" <tytso@mit.edu> wrote:
> > Finally, I'm bit concerned about anything which states absolutes,
> > because there are people who tend to be real stickler for the rules,
> > and if they see something stated in absolute terms, they fail to
> > understand that there are exceptions that are well understood, and in
> > use for years before the existence of the document which is trying to
> > codify best practices.
>
> Hence the "there are exceptions" text at the bottom of the document :)
>
> Anyway, I'll rework it to try to take your comments into account. Maybe
> we should consistently say "rebasing" for changing the parent commit of a
> patch set, and "history modification" for the other tricks...?
Or just "reworking a branch" for the other tricks?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v12] dm: add support to directly boot to a mapped device
From: Stephen Boyd @ 2019-06-04 19:35 UTC (permalink / raw)
To: Helen Koike, dm-devel
Cc: wad, keescook, snitzer, linux-doc, richard.weinberger,
linux-kernel, linux-lvm, enric.balletbo, kernel, agk
In-Reply-To: <d6b4fb26-9a1b-0acd-ce4a-e48322a17e7d@collabora.com>
Quoting Helen Koike (2019-06-04 10:38:59)
> On 6/3/19 8:02 PM, Stephen Boyd wrote:
> >
> > I'm trying to boot a mainline linux kernel on a chromeos device with dm
> > verity and a USB stick but it's not working for me even with this patch.
> > I've had to hack around two problems:
> >
> > 1) rootwait isn't considered
> >
> > 2) verity doesn't seem to accept UUID for <hash_dev> or <dev>
> >
> > For the first problem, it happens every boot for me because I'm trying
> > to boot off of a USB stick and it's behind a hub that takes a few
> > seconds to enumerate. If I hack up the code to call dm_init_init() after
> > the 'rootdelay' cmdline parameter is used then I can make this work. It
> > would be much nicer if the whole mechanism didn't use a late initcall
> > though. If it used a hook from prepare_namespace() and then looped
> > waiting for devices to create when rootwait was specified it would work.
>
> The patch was implemented with late initcall partially to be contained
> in drivers/md/*, but to support rootwait, adding a hook from
> prepare_namespace seems the way to go indeed.
Alright, great.
>
> >
> > The second problem is that in chromeos we have the bootloader fill out
> > the UUID of the kernel partition (%U) and then we have another parameter
> > that indicates the offset from that kernel partition to add to the
> > kernel partition (typically 1, i.e. PARTNROFF=1) to find the root
> > filesystem partition. The way verity seems to work here is that we need
> > to specify a path like /dev/sda3 or the major:minor number of the device
> > on the commandline to make this work. It would be better if we could add
> > in support for the PARTNROFF style that name_to_dev_t() handles so we
> > can specify the root partition like we're currently doing. I suspect we
> > should be able to add support for this into the device mapper layer so
> > that we can specify devices this way.
>
> hmm, I didn't test this yet but at least from what I can see in the
> code, verity_ctr() calls dm_get_device() that ends up calling
> name_to_dev_t() which should take care of PARTNROFF, this requires a bit
> more investigation.
>
Ok, thanks for pointing that out. Sorry I totally missed this codepath
and I should have investigated more. It works for me with the PARTNROFF
syntax that we've been using, so the problem is the rootwait stuff.
^ permalink raw reply
* Re: [PATCH] Documentation/dm-init: fix multi device example
From: Stephen Boyd @ 2019-06-04 19:37 UTC (permalink / raw)
To: Helen Koike, dm-devel
Cc: wad, keescook, snitzer, linux-doc, richard.weinberger,
linux-kernel, linux-lvm, enric.balletbo, kernel, agk, Helen Koike,
Jonathan Corbet
In-Reply-To: <20190604182719.15944-1-helen.koike@collabora.com>
Quoting Helen Koike (2019-06-04 11:27:19)
> The example in the docs regarding multiple device-mappers is invalid (it
> has a wrong number of arguments), it's a left over from previous
> versions of the patch.
> Replace the example with an valid and tested one.
>
> Signed-off-by: Helen Koike <helen.koike@collabora.com>
>
> ---
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
^ permalink raw reply
* [PATCH v2] Add a document on rebasing and merging
From: Jonathan Corbet @ 2019-06-04 19:48 UTC (permalink / raw)
To: linux-doc
Cc: LKML, Linus Torvalds, Theodore Ts'o, Geert Uytterhoeven,
David Rientjes
Every merge window seems to involve at least one episode where subsystem
maintainers don't manage their trees as Linus would like. Document the
expectations so that at least he has something to point people to.
Acked-by: David Rientjes <rientjes@google.com>
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
Changes in v2:
- Try to clear up "reparenting" v. "history modification"
- Make the "don't rebase public branches" rule into more of a guideline
- Fix typos noted by Geert
- Rename the document to better reflect its contents
Documentation/maintainer/index.rst | 1 +
.../maintainer/rebasing-and-merging.rst | 216 ++++++++++++++++++
2 files changed, 217 insertions(+)
create mode 100644 Documentation/maintainer/rebasing-and-merging.rst
diff --git a/Documentation/maintainer/index.rst
b/Documentation/maintainer/index.rst index 2a14916930cb..56e2c09dfa39
100644 --- a/Documentation/maintainer/index.rst
+++ b/Documentation/maintainer/index.rst
@@ -10,5 +10,6 @@ additions to this manual.
:maxdepth: 2
configure-git
+ rebasing-and-merging
pull-requests
diff --git a/Documentation/maintainer/rebasing-and-merging.rst
b/Documentation/maintainer/rebasing-and-merging.rst new file mode 100644
index 000000000000..2987bd45dfb2
--- /dev/null
+++ b/Documentation/maintainer/rebasing-and-merging.rst
@@ -0,0 +1,216 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+====================
+Rebasing and merging
+====================
+
+Maintaining a subsystem, as a general rule, requires a familiarity with
the +Git source-code management system. Git is a powerful tool with a lot
of +features; as is often the case with such tools, there are right and
wrong +ways to use those features. This document looks in particular at
the use +of rebasing and merging. Maintainers often get in trouble when
they use +those tools incorrectly, but avoiding problems is not actually
all that +hard.
+
+One thing to be aware of in general is that, unlike many other projects,
+the kernel community is not scared by seeing merge commits in its
+development history. Indeed, given the scale of the project, avoiding
+merges would be nearly impossible. Some problems encountered by
+maintainers result from a desire to avoid merges, while others come from
+merging a little too often.
+
+Rebasing
+========
+
+"Rebasing" is the process of changing the history of a series of commits
+within a repository. There are two different types of operations that are
+referred to as rebasing since both are done with the ``git rebase``
+command, but there are significant differences between them:
+
+ - Rebasing can change the parent (starting) commit upon which a series of
+ patches is built. For example, a rebase operation could take a patch
+ set built on the previous kernel release and base it, instead, on the
+ current release. We'll call this operation "reparenting" in the
+ discussion below.
+
+ - Changing the history of a set of patches by fixing (or deleting) broken
+ commits, adding patches, adding tags to commit changelogs, or changing
+ the order in which commits are applied. In the following text, this
+ type of operation will be referred to as "history modification"
+
+The term "rebasing" will be used to refer to both of the above operations.
+Used properly, rebasing can yield a cleaner and clearer development
+history; used improperly, it can obscure that history and introduce bugs.
+
+There are a few rules of thumb that can help developers to avoid the worst
+perils of rebasing:
+
+ - History that has been exposed to the world beyond your private system
+ should usually not be changed. Others may have pulled a copy of your
+ tree and built on it; modifying your tree will create pain for them.
If
+ work is in need of rebasing, that is usually a sign that it is not yet
+ ready to be committed to a public repository.
+
+ That said, there are always exceptions. Some trees (linux-next being
+ a significant example) are frequently rebased by their nature, and
+ developers know not to base work on them. Developers will sometimes
+ expose an unstable branch for others to test with or for automated
+ testing services. If you do expose a branch that may be unstable in
+ this way, be sure that prospective users know not to base work on it.
+
+ - Do not rebase a branch that contains history created by others. If you
+ have pulled changes from another developer's repository, you are now a
+ custodian of their history. You should not change it. With few
+ exceptions, for example, a broken commit in a tree like this should be
+ explicitly reverted rather than disappeared via history modification.
+
+ - Do not reparent a tree without a good reason to do so. Just being on a
+ newer base or avoiding a merge with an upstream repository is not
+ generally a good reason.
+
+ - If you must reparent a repository, do not pick some random kernel
commit
+ as the new base. The kernel is often in a relatively unstable state
+ between release points; basing development on one of those points
+ increases the chances of running into surprising bugs. When a patch
+ series must move to a new base, pick a stable point (such as one of
+ the -rc releases) to move to.
+
+ - Realize that the rebasing a patch series changes the environment in
+ which it was developed and, likely, invalidates much of the testing
that
+ was done. A rebased patch series should, as a general rule, be treated
+ like new code and retested from the beginning.
+
+A frequent cause of merge-window trouble is when Linus is presented with a
+patch series that has clearly been reparented, often to a random commit,
+shortly before the pull request was sent. The chances of such a series
+having been adequately tested are relatively low - as are the chances of
+the pull request being acted upon.
+
+If, instead, rebasing is limited to private trees, commits are based on a
+well-known starting point, and they are well tested, the potential for
+trouble is low.
+
+Merging
+=======
+
+Merging is a common operation in the kernel development process; the 5.1
+development cycle included 1,126 merge commits - nearly 9% of the total.
+Kernel work is accumulated in over 100 different subsystem trees, each of
+which may contain multiple topic branches; each branch is usually
developed +independently of the others. So naturally, at least merge will
be required +before any given branch finds its way into an upstream
repository. +
+Many projects require that branches in pull requests be based on the
+current trunk so that no merge commits appear in the history. The kernel
+is not such a project; any rebasing of branches to avoid merges will, as
+described above, lead to certain trouble.
+
+Subsystem maintainers find themselves having to do two types of merges:
+from lower-level subsystem trees and from others, either sibling trees or
+the mainline. The best practices to follow differ in those two
situations. +
+Merging from lower-level trees
+------------------------------
+
+Larger subsystems tend to have multiple levels of maintainers, with the
+lower-level maintainers sending pull requests to the higher levels.
Acting +on such a pull request will almost certainly generate a merge
commit; that +is as it should be. In fact, subsystem maintainers may want
to use +the --no-ff flag to force the addition of a merge commit in the
rare cases +where one would not normally be created so that the reasons
for the merge +can be recorded. The changelog for the merge should, for
any kind of +merge, say *why* the merge is being done. For a lower-level
tree, "why" is +usually a summary of the changes that will come with that
pull. +
+Maintainers at all levels should be using signed tags on their pull
+requests, and upstream maintainers should verify the tags when pulling
+branches. Failure to do so threatens the security of the development
+process as a whole.
+
+As per the rules outlined above, once you have merged somebody else's
+history into your tree, you cannot rebase that branch, even if you
+otherwise would be able to.
+
+Merging from sibling or upstream trees
+--------------------------------------
+
+While merges from downstream are common and unremarkable, merges from
other +trees tend to be a red flag when it comes time to push a branch
upstream. +Such merges need to be carefully thought about and well
justified, or +there's a good chance that a subsequent pull request will
be rejected. +
+It is natural to want to merge the master branch into a repository; it can
+help to make sure that there are no conflicts with parallel development
and +generally gives a warm, fuzzy feeling of being up-to-date. But this
+temptation should be avoided almost all of the time.
+
+Why is that? Merges with upstream will muddy the development history of
+your own branch. They will significantly increase your chances of
+encountering bugs from elsewhere in the community and make it hard to
+ensure that the work you are managing is stable and ready for upstream.
+Frequent merges can also obscure problems with the development process in
+your tree; they can hide interactions with other trees that should not be
+happening (often) in a well-managed branch.
+
+One of the most frequent causes of merge-related trouble is when a
+maintainer merges with the upstream in order to resolve merge conflicts
+before sending a pull request. Again, this temptation is easy enough to
+understand, but it should absolutely be avoided. This is especially true
+for the final pull request: Linus is adamant that he would much rather see
+merge conflicts than unnecessary back merges. Seeing the conflicts lets
+him know where potential problem areas are. He does a lot of merges (382
+in the 5.1 development cycle) and has gotten quite good at conflict
+resolution - often better than the developers involved.
+
+So what should a maintainer do when there is a conflict between their
+subsystem branch and the mainline? The most important step is to warn
+Linus in the pull request that the conflict will happen; if nothing else,
+that demonstrates an awareness of how your branch fits into the whole.
For +especially difficult conflicts, create and push a *separate* branch
to show +how you would resolve things. Mention that branch in your pull
request, +but the pull request itself should be for the unmerged branch.
+
+Even in the absence of known conflicts, doing a test merge before sending
a +pull request is a good idea. It may alert you to problems that you
somehow +didn't see from linux-next and helps to understand exactly what
you are +asking upstream to do.
+
+Another reason for doing merges of upstream or another subsystem tree is
to +resolve dependencies. These dependency issues do happen at times, and
+sometimes a cross-merge with another tree is the best way to resolve them;
+as always, in such situations, the merge commit should explain why the
+merge has been done. Take a moment to do it right; people will read those
+changelogs.
+
+Often, though, dependency issues indicate that a change of approach is
+needed. Merging another subsystem tree to resolve a dependency risks
+bringing in other bugs. If that subsystem tree fails to be pulled
+upstream, whatever problems it had will block the merging of your tree as
+well. Possible alternatives include agreeing with the maintainer to carry
+both sets of changes in one of the trees or creating a special branch
+dedicated to the dependent commits. If the dependency is related to major
+infrastructural changes, the right solution might be to hold the dependent
+commits for one development cycle so that those changes have time to
+stabilize in the mainline.
+
+Finally
+=======
+
+It is relatively common to merge with the mainline toward the beginning of
+the development cycle in order to pick up changes and fixes done elsewhere
+in the tree. As always, such a merge should pick a well-known release
+point rather than some random spot. If your upstream-bound branch has
+emptied entirely into the mainline during the merge window, you can pull
it +forward with a command like::
+
+ git merge v5.2-rc1^0
+
+The "^0" will cause Git to do a fast-forward merge (which should be
+possible in this situation), thus avoiding the addition of a spurious
merge +commit.
+
+The guidelines laid out above are just that: guidelines. There will
always +be situations that call out for a different solution, and these
guidelines +should not prevent developers from doing the right thing when
the need +arises. But one should always think about whether the need has
truly +arisen and be prepared to explain why something abnormal needs to
be done. --
2.21.0
^ permalink raw reply related
* Re: [PATCH v4 1/2] fTPM: firmware TPM running in TEE
From: Sasha Levin @ 2019-06-04 20:09 UTC (permalink / raw)
To: Sumit Garg
Cc: peterhuewe, Jarkko Sakkinen, jgg, corbet,
Linux Kernel Mailing List, linux-doc, linux-integrity,
Microsoft Linux Kernel List, Thirupathaiah Annapureddy,
Bryan Kelly (CSI), tee-dev
In-Reply-To: <CAFA6WYM1NrghG9qxUhrm76kopvBx9nmCL9XnRs11ysb2Yr0+Qw@mail.gmail.com>
On Tue, Jun 04, 2019 at 11:45:52AM +0530, Sumit Garg wrote:
>On Thu, 30 May 2019 at 20:58, Sasha Levin <sashal@kernel.org> wrote:
>> + /* Open context with TEE driver */
>> + pvt_data->ctx = tee_client_open_context(NULL, ftpm_tee_match, NULL,
>> + NULL);
>> + if (IS_ERR(pvt_data->ctx)) {
>> + dev_err(dev, "%s:tee_client_open_context failed\n", __func__);
>
>Is this well tested? I see this misleading error multiple times as
>follows although TEE driver works pretty well.
Yes, this was all functionally tested.
Why is this error message misleading? I'd be happy to fix it.
>Module built with "CONFIG_TCG_FTPM_TEE=y"
>
>[ 1.436878] ftpm-tee tpm@0: ftpm_tee_probe:tee_client_open_context failed
>[ 1.509471] ftpm-tee tpm@0: ftpm_tee_probe:tee_client_open_context failed
>[ 1.517268] ftpm-tee tpm@0: ftpm_tee_probe:tee_client_open_context failed
>[ 1.525596] ftpm-tee tpm@0: ftpm_tee_probe:tee_client_open_context failed
Does the TEE have the fTPM implementation and such? Could you provide
details about your testing environment (hardware, fTPM verions, etc)?
--
Thanks,
Sasha
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox