* Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix documentation build warnings
From: Mika Kuoppala @ 2019-06-05 12:35 UTC (permalink / raw)
To: Jani Nikula, intel-gfx; +Cc: jani.nikula, linux-doc
In-Reply-To: <20190605095657.23601-2-jani.nikula@intel.com>
Jani Nikula <jani.nikula@intel.com> writes:
> Just a straightforward bag of fixes for a clean htmldocs build.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
^ permalink raw reply
* Re: [PATCH v4 0/2] fTPM: firmware TPM running in TEE
From: Jarkko Sakkinen @ 2019-06-05 14:06 UTC (permalink / raw)
To: Sasha Levin
Cc: peterhuewe, jgg, corbet, linux-kernel, linux-doc, linux-integrity,
linux-kernel, thiruan, bryankel, tee-dev
In-Reply-To: <20190603211648.GV12898@sasha-vm>
On Mon, Jun 03, 2019 at 05:16:48PM -0400, Sasha Levin wrote:
> On Mon, Jun 03, 2019 at 11:28:15PM +0300, Jarkko Sakkinen wrote:
> > On Thu, May 30, 2019 at 11:27:56AM -0400, Sasha Levin wrote:
> > > Changes since v3:
> > >
> > > - Address comments by Jarkko Sakkinen
> > > - Address comments by Igor Opaniuk
> > >
> > > Sasha Levin (2):
> > > fTPM: firmware TPM running in TEE
> > > fTPM: add documentation for ftpm driver
> >
> > I think patches start to look proper but I wonder can anyone test
> > these? I don't think before that I can merge these.
>
> They're all functionally tested by us on actual hardware before being
> sent out.
>
> The reference implementation is open and being kept updated, and an
> interested third party should be able to verify the correctness of these
> patches. However, it doesn't look like there's an interested third party
> given that these patches have been out for a few months now.
So can they be tagged with your tested-by?
/Jarkko
^ permalink raw reply
* [PATCH v10 0/3] watchdog: allow setting deadline for opening /dev/watchdogN
From: Rasmus Villemoes @ 2019-06-05 14:06 UTC (permalink / raw)
To: linux-watchdog@vger.kernel.org, Guenter Roeck, Wim Van Sebroeck,
Jonathan Corbet
Cc: linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
Esben Haabendal, Jerry Hoemann, Rasmus Villemoes
If a watchdog driver tells the framework that the device is running,
the framework takes care of feeding the watchdog until userspace opens
the device. If the userspace application which is supposed to do that
never comes up properly, the watchdog is fed indefinitely by the
kernel. This can be especially problematic for embedded devices.
The existing handle_boot_enabled cmdline parameter/config option
partially solves that, but that is only usable for the subset of
hardware watchdogs that have (or can be configured by the bootloader
to have) a timeout that is sufficient to make it realistic for
userspace to come up. Many devices have timeouts of only a few
seconds, or even less, making handle_boot_enabled insufficient.
These patches allow one to set a maximum time for which the kernel
will feed the watchdog, thus ensuring that either userspace has come
up, or the board gets reset. This allows fallback logic in the
bootloader to attempt some recovery (for example, if an automatic
update is in progress, it could roll back to the previous version).
The patches have been tested on a Raspberry Pi 2 and a Wandboard.
Changes in v10: The open_timeout now only applies to the first open
from userspace. If userspace needs to close and re-open the watchdog
device (e.g. to re-exec itself), and wants the board to reset in case
it doesn't come back quickly enough, the open_timeout can easily be
emulated by combining nowayout with an appropriate WDIOC_SETTIMEOUT.
Changes in v9: Make the unit seconds instead of milliseconds.
Rasmus Villemoes (3):
watchdog: introduce watchdog.open_timeout commandline parameter
watchdog: introduce CONFIG_WATCHDOG_OPEN_TIMEOUT
watchdog: make the device time out at open_deadline when open_timeout
is used
.../watchdog/watchdog-parameters.txt | 8 ++++
drivers/watchdog/Kconfig | 9 ++++
drivers/watchdog/watchdog_dev.c | 48 ++++++++++++++++---
3 files changed, 59 insertions(+), 6 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH v10 1/3] watchdog: introduce watchdog.open_timeout commandline parameter
From: Rasmus Villemoes @ 2019-06-05 14:06 UTC (permalink / raw)
To: linux-watchdog@vger.kernel.org, Guenter Roeck, Wim Van Sebroeck,
Jonathan Corbet
Cc: linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
Esben Haabendal, Jerry Hoemann, Rasmus Villemoes
In-Reply-To: <20190605140628.618-1-rasmus.villemoes@prevas.dk>
The watchdog framework takes care of feeding a hardware watchdog until
userspace opens /dev/watchdogN. If that never happens for some reason
(buggy init script, corrupt root filesystem or whatnot) but the kernel
itself is fine, the machine stays up indefinitely. This patch allows
setting an upper limit for how long the kernel will take care of the
watchdog, thus ensuring that the watchdog will eventually reset the
machine.
A value of 0 (the default) means infinite timeout, preserving the
current behaviour.
This is particularly useful for embedded devices where some fallback
logic is implemented in the bootloader (e.g., use a different root
partition, boot from network, ...).
There is already handle_boot_enabled serving a similar purpose. However,
such a binary choice is unsuitable if the hardware watchdog cannot be
programmed by the bootloader to provide a timeout long enough for
userspace to get up and running. Many of the embedded devices we see use
external (gpio-triggered) watchdogs with a fixed timeout of the order of
1-2 seconds.
The open timeout only applies for the first open from
userspace. Should userspace need to close the watchdog device, with
the intention of re-opening it shortly, the application can emulate
the open timeout feature by combining the nowayout feature with an
appropriate WDIOC_SETTIMEOUT immediately prior to closing the device.
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
.../watchdog/watchdog-parameters.txt | 8 +++++
drivers/watchdog/watchdog_dev.c | 36 ++++++++++++++++++-
2 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
index 0b88e333f9e1..32d3606caa65 100644
--- a/Documentation/watchdog/watchdog-parameters.txt
+++ b/Documentation/watchdog/watchdog-parameters.txt
@@ -8,6 +8,14 @@ See Documentation/admin-guide/kernel-parameters.rst for information on
providing kernel parameters for builtin drivers versus loadable
modules.
+The watchdog core parameter watchdog.open_timeout is the maximum time,
+in seconds, for which the watchdog framework will take care of pinging
+a running hardware watchdog until userspace opens the corresponding
+/dev/watchdogN device. A value of 0 (the default) means an infinite
+timeout. Setting this to a non-zero value can be useful to ensure that
+either userspace comes up properly, or the board gets reset and allows
+fallback logic in the bootloader to try something else.
+
-------------------------------------------------
acquirewdt:
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 252a7c7b6592..e4b51db48f0e 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -69,6 +69,7 @@ struct watchdog_core_data {
struct mutex lock;
ktime_t last_keepalive;
ktime_t last_hw_keepalive;
+ ktime_t open_deadline;
struct hrtimer timer;
struct kthread_work work;
unsigned long status; /* Internal status bits */
@@ -87,6 +88,19 @@ static struct kthread_worker *watchdog_kworker;
static bool handle_boot_enabled =
IS_ENABLED(CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED);
+static unsigned open_timeout;
+
+static bool watchdog_past_open_deadline(struct watchdog_core_data *data)
+{
+ return ktime_after(ktime_get(), data->open_deadline);
+}
+
+static void watchdog_set_open_deadline(struct watchdog_core_data *data)
+{
+ data->open_deadline = open_timeout ?
+ ktime_get() + ktime_set(open_timeout, 0) : KTIME_MAX;
+}
+
static inline bool watchdog_need_worker(struct watchdog_device *wdd)
{
/* All variables in milli-seconds */
@@ -211,7 +225,13 @@ static bool watchdog_worker_should_ping(struct watchdog_core_data *wd_data)
{
struct watchdog_device *wdd = wd_data->wdd;
- return wdd && (watchdog_active(wdd) || watchdog_hw_running(wdd));
+ if (!wdd)
+ return false;
+
+ if (watchdog_active(wdd))
+ return true;
+
+ return watchdog_hw_running(wdd) && !watchdog_past_open_deadline(wd_data);
}
static void watchdog_ping_work(struct kthread_work *work)
@@ -824,6 +844,15 @@ static int watchdog_open(struct inode *inode, struct file *file)
if (!hw_running)
kref_get(&wd_data->kref);
+ /*
+ * open_timeout only applies for the first open from
+ * userspace. Set open_deadline to infinity so that the kernel
+ * will take care of an always-running hardware watchdog in
+ * case the device gets magic-closed or WDIOS_DISABLECARD is
+ * applied.
+ */
+ wd_data->open_deadline = KTIME_MAX;
+
/* dev/watchdog is a virtual (and thus non-seekable) filesystem */
return stream_open(inode, file);
@@ -983,6 +1012,7 @@ static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t devno)
/* Record time of most recent heartbeat as 'just before now'. */
wd_data->last_hw_keepalive = ktime_sub(ktime_get(), 1);
+ watchdog_set_open_deadline(wd_data);
/*
* If the watchdog is running, prevent its driver from being unloaded,
@@ -1181,3 +1211,7 @@ module_param(handle_boot_enabled, bool, 0444);
MODULE_PARM_DESC(handle_boot_enabled,
"Watchdog core auto-updates boot enabled watchdogs before userspace takes over (default="
__MODULE_STRING(IS_ENABLED(CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED)) ")");
+
+module_param(open_timeout, uint, 0644);
+MODULE_PARM_DESC(open_timeout,
+ "Maximum time (in seconds, 0 means infinity) for userspace to take over a running watchdog (default=0)");
--
2.20.1
^ permalink raw reply related
* [PATCH v10 2/3] watchdog: introduce CONFIG_WATCHDOG_OPEN_TIMEOUT
From: Rasmus Villemoes @ 2019-06-05 14:06 UTC (permalink / raw)
To: linux-watchdog@vger.kernel.org, Guenter Roeck, Wim Van Sebroeck,
Jonathan Corbet
Cc: linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
Esben Haabendal, Jerry Hoemann, Rasmus Villemoes
In-Reply-To: <20190605140628.618-1-rasmus.villemoes@prevas.dk>
This allows setting a default value for the watchdog.open_timeout
commandline parameter via Kconfig.
Some BSPs allow remote updating of the kernel image and root file
system, but updating the bootloader requires physical access. Hence, if
one has a firmware update that requires relaxing the
watchdog.open_timeout a little, the value used must be baked into the
kernel image itself and cannot come from the u-boot environment via the
kernel command line.
Being able to set the initial value in .config doesn't change the fact
that the value on the command line, if present, takes precedence, and is
of course immensely useful for development purposes while one has
console acccess, as well as usable in the cases where one can make a
permanent update of the kernel command line.
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
Documentation/watchdog/watchdog-parameters.txt | 8 ++++----
drivers/watchdog/Kconfig | 9 +++++++++
drivers/watchdog/watchdog_dev.c | 5 +++--
3 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
index 32d3606caa65..ec919dc895ff 100644
--- a/Documentation/watchdog/watchdog-parameters.txt
+++ b/Documentation/watchdog/watchdog-parameters.txt
@@ -11,10 +11,10 @@ modules.
The watchdog core parameter watchdog.open_timeout is the maximum time,
in seconds, for which the watchdog framework will take care of pinging
a running hardware watchdog until userspace opens the corresponding
-/dev/watchdogN device. A value of 0 (the default) means an infinite
-timeout. Setting this to a non-zero value can be useful to ensure that
-either userspace comes up properly, or the board gets reset and allows
-fallback logic in the bootloader to try something else.
+/dev/watchdogN device. A value of 0 means an infinite timeout. Setting
+this to a non-zero value can be useful to ensure that either userspace
+comes up properly, or the board gets reset and allows fallback logic
+in the bootloader to try something else.
-------------------------------------------------
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index ffe754539f5a..a8bd621e12f8 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -58,6 +58,15 @@ config WATCHDOG_HANDLE_BOOT_ENABLED
the watchdog on its own. Thus if your userspace does not start fast
enough your device will reboot.
+config WATCHDOG_OPEN_TIMEOUT
+ int "Timeout value for opening watchdog device"
+ default 0
+ help
+ The maximum time, in seconds, for which the watchdog framework takes
+ care of pinging a hardware watchdog. A value of 0 means infinite. The
+ value set here can be overridden by the commandline parameter
+ "watchdog.open_timeout".
+
config WATCHDOG_SYSFS
bool "Read different watchdog information through sysfs"
help
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index e4b51db48f0e..334b810db2cf 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -88,7 +88,7 @@ static struct kthread_worker *watchdog_kworker;
static bool handle_boot_enabled =
IS_ENABLED(CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED);
-static unsigned open_timeout;
+static unsigned open_timeout = CONFIG_WATCHDOG_OPEN_TIMEOUT;
static bool watchdog_past_open_deadline(struct watchdog_core_data *data)
{
@@ -1214,4 +1214,5 @@ MODULE_PARM_DESC(handle_boot_enabled,
module_param(open_timeout, uint, 0644);
MODULE_PARM_DESC(open_timeout,
- "Maximum time (in seconds, 0 means infinity) for userspace to take over a running watchdog (default=0)");
+ "Maximum time (in seconds, 0 means infinity) for userspace to take over a running watchdog (default="
+ __MODULE_STRING(CONFIG_WATCHDOG_OPEN_TIMEOUT) ")");
--
2.20.1
^ permalink raw reply related
* [PATCH v10 3/3] watchdog: make the device time out at open_deadline when open_timeout is used
From: Rasmus Villemoes @ 2019-06-05 14:06 UTC (permalink / raw)
To: linux-watchdog@vger.kernel.org, Guenter Roeck, Wim Van Sebroeck,
Jonathan Corbet
Cc: linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
Esben Haabendal, Jerry Hoemann, Rasmus Villemoes
In-Reply-To: <20190605140628.618-1-rasmus.villemoes@prevas.dk>
When the watchdog device is not open by userspace, the kernel takes
care of pinging it. When the open_timeout feature is in use, we should
ensure that the hardware fires close to open_timeout seconds after the
kernel has assumed responsibility for the device.
To do this, simply reuse the logic that is already in place for
ensuring the same thing when userspace is responsible for regularly
pinging the device:
- When watchdog_active(wdd), this patch doesn't change anything.
- When !watchdoc_active(wdd), the "virtual timeout" should be taken to
be ->open_deadline". When the open_timeout feature is not used or the
device has been opened at least once, ->open_deadline is KTIME_MAX,
and the arithmetic ends up returning keepalive_interval as we used to.
This has been tested on a Wandboard with various combinations of
open_timeout and timeout-sec properties for the on-board watchdog by
booting with 'init=/bin/sh', timestamping the lines on the serial
console, and comparing the timestamp of the 'imx2-wdt 20bc000.wdog:
timeout nnn sec' line with the timestamp of the 'U-Boot SPL ...'
line (which appears just after reset).
Suggested-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
drivers/watchdog/watchdog_dev.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 334b810db2cf..edfb884044e0 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -133,14 +133,15 @@ static ktime_t watchdog_next_keepalive(struct watchdog_device *wdd)
ktime_t virt_timeout;
unsigned int hw_heartbeat_ms;
- virt_timeout = ktime_add(wd_data->last_keepalive,
- ms_to_ktime(timeout_ms));
+ if (watchdog_active(wdd))
+ virt_timeout = ktime_add(wd_data->last_keepalive,
+ ms_to_ktime(timeout_ms));
+ else
+ virt_timeout = wd_data->open_deadline;
+
hw_heartbeat_ms = min_not_zero(timeout_ms, wdd->max_hw_heartbeat_ms);
keepalive_interval = ms_to_ktime(hw_heartbeat_ms / 2);
- if (!watchdog_active(wdd))
- return keepalive_interval;
-
/*
* To ensure that the watchdog times out wdd->timeout seconds
* after the most recent ping from userspace, the last
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v4 1/2] fTPM: firmware TPM running in TEE
From: Jarkko Sakkinen @ 2019-06-05 14:09 UTC (permalink / raw)
To: Sumit Garg
Cc: Sasha Levin, peterhuewe, 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:
> Is this well tested? I see this misleading error multiple times as
> follows although TEE driver works pretty well.
>
> 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
>
> -Sumit
No testing done from my part.
/Jarkko
^ permalink raw reply
* Re: [PATCH 03/10] mfd / platform: cros_ec: Miscellaneous character device to talk with the EC
From: Enric Balletbo Serra @ 2019-06-05 14:33 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: 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: <20190604155228.GB9981@kroah.com>
Hi Greg,
Many thanks for your comments,
Missatge de Greg Kroah-Hartman <gregkh@linuxfoundation.org> del dia
dt., 4 de juny 2019 a les 17:53:
>
> 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.
Ok, I'll remove on next version.
> > +
> > + 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?
>
Right, my bad, I was carrying some code from an old version that is
not needed at all now. I'll remove all the related code.
> > + 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?
>
Right, not needed. Removed on next version.
Thanks,
Enric
> thanks,
>
> greg k-h
^ permalink raw reply
* Re: [v3 PATCH] RISC-V: Add a PE/COFF compliant Image header.
From: Kevin Hilman @ 2019-06-05 16:00 UTC (permalink / raw)
To: Atish Patra, linux-kernel
Cc: Atish Patra, Albert Ou, Anup Patel, Jonathan Corbet, linux-doc,
linux-riscv, Nick Kossifidis, Palmer Dabbelt, Zong Li,
linux-arm-kernel, mark.rutland, marek.vasut, catalin.marinas,
will.deacon, trini, paul.walmsley
In-Reply-To: <20190523183516.583-1-atish.patra@wdc.com>
Atish Patra <atish.patra@wdc.com> writes:
> Currently, last stage boot loaders such as U-Boot can accept only
> uImage which is an unnecessary additional step in automating boot flows.
>
> Add a PE/COFF compliant image header that boot loaders can parse and
> directly load kernel flat Image. The existing booting methods will continue
> to work as it is.
>
> Another goal of this header is to support EFI stub for RISC-V in future.
> EFI specification needs PE/COFF image header in the beginning of the kernel
> image in order to load it as an EFI application. In order to support
> EFI stub, code0 should be replaced with "MZ" magic string and res5(at
> offset 0x3c) should point to the rest of the PE/COFF header (which will
> be added during EFI support).
>
> This patch is based on ARM64 boot image header and provides an opprtunity
> to combine both ARM64 & RISC-V image headers.
>
> Tested on both QEMU and HiFive Unleashed using OpenSBI + U-Boot + Linux.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
Tested booti support on HiFive Unleashed using OpenSBI + U-Boot (master
branch) + Linux.
Tested-by: Kevin Hilman <khilman@baylibre.com>
^ permalink raw reply
* Re: [v3 PATCH] RISC-V: Add a PE/COFF compliant Image header.
From: Mark Rutland @ 2019-06-05 16:26 UTC (permalink / raw)
To: Atish Patra
Cc: linux-kernel, Albert Ou, Anup Patel, Jonathan Corbet, linux-doc,
linux-riscv, Nick Kossifidis, Palmer Dabbelt, Zong Li,
linux-arm-kernel, marek.vasut, catalin.marinas, will.deacon,
trini, paul.walmsley
In-Reply-To: <20190523183516.583-1-atish.patra@wdc.com>
On Thu, May 23, 2019 at 11:35:16AM -0700, Atish Patra wrote:
> Currently, last stage boot loaders such as U-Boot can accept only
> uImage which is an unnecessary additional step in automating boot flows.
>
> Add a PE/COFF compliant image header that boot loaders can parse and
> directly load kernel flat Image. The existing booting methods will continue
> to work as it is.
>
> Another goal of this header is to support EFI stub for RISC-V in future.
> EFI specification needs PE/COFF image header in the beginning of the kernel
> image in order to load it as an EFI application. In order to support
> EFI stub, code0 should be replaced with "MZ" magic string and res5(at
> offset 0x3c) should point to the rest of the PE/COFF header (which will
> be added during EFI support).
>
> This patch is based on ARM64 boot image header and provides an opprtunity
> to combine both ARM64 & RISC-V image headers.
>
> Tested on both QEMU and HiFive Unleashed using OpenSBI + U-Boot + Linux.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
>
> ---
> I have not sent out corresponding U-Boot patch as all the changes are
> compatible with current u-boot support. Once, the kernel header format
> is agreed upon, I will update the U-Boot patch.
>
> Changes from v2->v3
> 1. Modified reserved fields to define a header version.
> 2. Added header documentation.
>
> Changes from v1-v2:
> 1. Added additional reserved elements to make it fully PE compatible.
> ---
> Documentation/riscv/boot-image-header.txt | 50 ++++++++++++++++++
> arch/riscv/include/asm/image.h | 64 +++++++++++++++++++++++
> arch/riscv/kernel/head.S | 32 ++++++++++++
> 3 files changed, 146 insertions(+)
> create mode 100644 Documentation/riscv/boot-image-header.txt
> create mode 100644 arch/riscv/include/asm/image.h
>
> diff --git a/Documentation/riscv/boot-image-header.txt b/Documentation/riscv/boot-image-header.txt
> new file mode 100644
> index 000000000000..68abc2353cec
> --- /dev/null
> +++ b/Documentation/riscv/boot-image-header.txt
> @@ -0,0 +1,50 @@
> + Boot image header in RISC-V Linux
> + =============================================
> +
> +Author: Atish Patra <atish.patra@wdc.com>
> +Date : 20 May 2019
> +
> +This document only describes the boot image header details for RISC-V Linux.
> +The complete booting guide will be available at Documentation/riscv/booting.txt.
> +
> +The following 64-byte header is present in decompressed Linux kernel image.
> +
> + u32 code0; /* Executable code */
> + u32 code1; /* Executable code */
> + u64 text_offset; /* Image load offset, little endian */
> + u64 image_size; /* Effective Image size, little endian */
> + u64 flags; /* kernel flags, little endian */
> + u32 version; /* Version of this header */
> + u32 res1 = 0; /* Reserved */
> + u64 res2 = 0; /* Reserved */
> + u64 magic = 0x5643534952; /* Magic number, little endian, "RISCV" */
> + u32 res3; /* Reserved for additional RISC-V specific header */
> + u32 res4; /* Reserved for PE COFF offset */
> +
> +This header format is compliant with PE/COFF header and largely inspired from
> +ARM64 header. Thus, both ARM64 & RISC-V header can be combined into one common
> +header in future.
> +
> +Notes:
> +- This header can also be reused to support EFI stub for RISC-V in future. EFI
> + specification needs PE/COFF image header in the beginning of the kernel image
> + in order to load it as an EFI application. In order to support EFI stub,
> + code0 should be replaced with "MZ" magic string and res5(at offset 0x3c) should
> + point to the rest of the PE/COFF header.
> +
> +- version field indicate header version number.
> + Bits 0:15 - Minor version
> + Bits 16:31 - Major version
> +
> + This preserves compatibility across newer and older version of the header.
> + The current version is defined as 0.1.
> +
> +- res3 is reserved for offset to any other additional fields. This makes the
> + header extendible in future. One example would be to accommodate ISA
> + extension for RISC-V in future. For current version, it is set to be zero.
> +
> +- In current header, the flag field has only one field.
> + Bit 0: Kernel endianness. 1 if BE, 0 if LE.
> +
> +- Image size is mandatory for boot loader to load kernel image. Booting will
> + fail otherwise.
> diff --git a/arch/riscv/include/asm/image.h b/arch/riscv/include/asm/image.h
> new file mode 100644
> index 000000000000..61c9f20d2f19
> --- /dev/null
> +++ b/arch/riscv/include/asm/image.h
> @@ -0,0 +1,64 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __ASM_IMAGE_H
> +#define __ASM_IMAGE_H
> +
> +#define RISCV_IMAGE_MAGIC "RISCV"
> +
> +
> +#define RISCV_IMAGE_FLAG_BE_SHIFT 0
> +#define RISCV_IMAGE_FLAG_BE_MASK 0x1
> +
> +#define RISCV_IMAGE_FLAG_LE 0
> +#define RISCV_IMAGE_FLAG_BE 1
> +
> +
> +#ifdef CONFIG_CPU_BIG_ENDIAN
> +#define __HEAD_FLAG_BE RISCV_IMAGE_FLAG_BE
> +#else
> +#define __HEAD_FLAG_BE RISCV_IMAGE_FLAG_LE
> +#endif
> +
> +#define __HEAD_FLAG(field) (__HEAD_FLAG_##field << \
> + RISCV_IMAGE_FLAG_##field##_SHIFT)
> +
> +#define __HEAD_FLAGS (__HEAD_FLAG(BE))
If you have a CONFIG_CPU_BIG_ENDIAN kernel, this will not be
little-endian, nor will other fields in your header (e.g. the image
size), so I would recommend dropping this for now.
To manage that for the image_size field you'll probably need to play the
same linker trick games we play on arm64.
It's probably worth having:
#ifdef CONFIG_CPU_BIG_ENDIAN
#error conversion of header fields to LE not yet implemented
#endif
... to catch that later.
Thanks,
Mark,
^ permalink raw reply
* [PATCH v1] docs/core-api: Add string helpers API to the list
From: Andy Shevchenko @ 2019-06-05 16:39 UTC (permalink / raw)
To: Jonathan Corbet, linux-doc, Andrew Morton, Mike Rapoport; +Cc: Andy Shevchenko
Some times string helpers are needed, but there is nothing about them
in the generated documentation.
Fill the gap by adding a reference to string_helpers.c exported functions.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Documentation/core-api/kernel-api.rst | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/core-api/kernel-api.rst b/Documentation/core-api/kernel-api.rst
index a53ec2eb8176..65ae2bf1f86d 100644
--- a/Documentation/core-api/kernel-api.rst
+++ b/Documentation/core-api/kernel-api.rst
@@ -33,6 +33,9 @@ String Conversions
.. kernel-doc:: lib/kstrtox.c
:export:
+.. kernel-doc:: lib/string_helpers.c
+ :export:
+
String Manipulation
-------------------
--
2.20.1
^ permalink raw reply related
* [PATCH v1] docs/core-api: Add integer power functions to the list
From: Andy Shevchenko @ 2019-06-05 16:51 UTC (permalink / raw)
To: Jonathan Corbet, linux-doc, Andrew Morton, Mike Rapoport; +Cc: Andy Shevchenko
Some times integer power functions, such as int_sqrt(), are needed, but
there is nothing about them in the generated documentation.
Fill the gap by adding a reference to the corresponding exported functions.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Documentation/core-api/kernel-api.rst | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/core-api/kernel-api.rst b/Documentation/core-api/kernel-api.rst
index 65ae2bf1f86d..824f24ccf401 100644
--- a/Documentation/core-api/kernel-api.rst
+++ b/Documentation/core-api/kernel-api.rst
@@ -141,6 +141,15 @@ Base 2 log and power Functions
.. kernel-doc:: include/linux/log2.h
:internal:
+Integer power Functions
+-----------------------
+
+.. kernel-doc:: lib/math/int_pow.c
+ :export:
+
+.. kernel-doc:: lib/math/int_sqrt.c
+ :export:
+
Division Functions
------------------
--
2.20.1
^ permalink raw reply related
* Re: Coccinelle: Searching for “when done” in function comments
From: Markus Elfring @ 2019-06-05 18:23 UTC (permalink / raw)
To: Wen Yang, Julia Lawall, linux-doc
Cc: cocci, linux-kernel, Gilles Muller, Masahiro Yamada, Michal Marek,
Nicolas Palix
In-Reply-To: <fa3b24ba-1c57-3115-6a01-ee98fd702087@web.de>
> Does such a source code analysis approach indicate any details
> which should be improved for the affected software documentation?
I have constructed another SmPL script variant.
It can point out that the text “ when done” occurs in comment lines before
48 function implementations which are provided by 20 source files of
the software “Linux next-20190605”.
Will this source code analysis result trigger further development efforts
around detection of unique wordings in these descriptions?
Regards,
Markus
^ permalink raw reply
* [PATCH 2/3] treewide: trivial: fix s/poped/popped/ typo
From: George G. Davis @ 2019-06-05 20:30 UTC (permalink / raw)
To: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
H. Peter Anvin, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
Jiri Kosina, George G. Davis, Masami Hiramatsu, Steven Rostedt,
Andi Kleen, Jann Horn, Nadav Amit, open list:DOCUMENTATION,
open list
In-Reply-To: <1559766612-12178-1-git-send-email-george_davis@mentor.com>
Fix a couple of s/poped/popped/ typos.
Cc: Jiri Kosina <trivial@kernel.org>
Signed-off-by: George G. Davis <george_davis@mentor.com>
---
Documentation/arm/mem_alignment | 2 +-
arch/x86/kernel/kprobes/core.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/arm/mem_alignment b/Documentation/arm/mem_alignment
index 6335fcacbba9..e110e2781039 100644
--- a/Documentation/arm/mem_alignment
+++ b/Documentation/arm/mem_alignment
@@ -1,4 +1,4 @@
-Too many problems poped up because of unnoticed misaligned memory access in
+Too many problems popped up because of unnoticed misaligned memory access in
kernel code lately. Therefore the alignment fixup is now unconditionally
configured in for SA11x0 based targets. According to Alan Cox, this is a
bad idea to configure it out, but Russell King has some good reasons for
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 6afd8061dbae..d3243d93daf4 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -813,7 +813,7 @@ __used __visible void *trampoline_handler(struct pt_regs *regs)
continue;
/*
* Return probes must be pushed on this hash list correct
- * order (same as return order) so that it can be poped
+ * order (same as return order) so that it can be popped
* correctly. However, if we find it is pushed it incorrect
* order, this means we find a function which should not be
* probed, because the wrong order entry is pushed on the
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2] Add a document on rebasing and merging
From: Jonathan Corbet @ 2019-06-05 20:32 UTC (permalink / raw)
To: Theodore Ts'o
Cc: linux-doc, LKML, Linus Torvalds, Geert Uytterhoeven,
David Rientjes
In-Reply-To: <20190605015456.GA2710@mit.edu>
On Tue, 4 Jun 2019 21:54:56 -0400
"Theodore Ts'o" <tytso@mit.edu> wrote:
> FYI, it looks like your patch somehow got hit by your text editor (or
> MUA's) line wrapping...
Weird, I haven't had a problem like that in decades. No idea what
happened here...
> > +
> > + - 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.
>
> Shouldn't "reparenting" be used in this paragraph?
>
> I suppose if a patch is getting dropped or modified that can
> invalidate some of the testing (although it really depends on the
> nature of what's being dropped or modified). And if it's just adding
> a Tested-by tag or a CVE number in the commit description, it's not
> going to invalidate any testing.
I had thought about it and chosen "rebasing", but I can change it.
> > +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 momehnt to do it right; people will read those
> > +changelogs.
>
> It might also be useful to mention it might be useful to put the
> commits which are needed to solve the dependency problem on its own
> separate branch, based off of something like -rc2, and then each of
> the trees which need the prerequisite commits can merge in that
> branch.
That is (I think) in the following paragraph:
> 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.
Perhaps that last line should read "...dedicated to the prerequisite
commits, which can then be merged into both trees" ?
Then perhaps I can finally declare victory on this thing? :)
Thanks,
jon
^ permalink raw reply
* Re: [PATCH v2 1/2] hwmon: pmbus: Add Infineon PXE1610 VR driver
From: Guenter Roeck @ 2019-06-05 20:46 UTC (permalink / raw)
To: Vijay Khemka
Cc: Jean Delvare, Jonathan Corbet, linux-hwmon, linux-doc,
linux-kernel, joel, linux-aspeed, sdasari, Greg Kroah-Hartman
In-Reply-To: <20190530231159.222188-1-vijaykhemka@fb.com>
On Thu, May 30, 2019 at 04:11:56PM -0700, Vijay Khemka wrote:
> Added pmbus driver for the new device Infineon pxe1610
> voltage regulator. It also supports similar family device
> PXE1110 and PXM1310.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
Applied.
Thanks,
Guenter
> ---
> Changes in v2:
> incorporated all the feedback from Guenter Roeck <linux@roeck-us.net>
>
> drivers/hwmon/pmbus/Kconfig | 9 +++
> drivers/hwmon/pmbus/Makefile | 1 +
> drivers/hwmon/pmbus/pxe1610.c | 139 ++++++++++++++++++++++++++++++++++
> 3 files changed, 149 insertions(+)
> create mode 100644 drivers/hwmon/pmbus/pxe1610.c
>
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 30751eb9550a..338ef9b5a395 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -154,6 +154,15 @@ config SENSORS_MAX8688
> This driver can also be built as a module. If so, the module will
> be called max8688.
>
> +config SENSORS_PXE1610
> + tristate "Infineon PXE1610"
> + help
> + If you say yes here you get hardware monitoring support for Infineon
> + PXE1610.
> +
> + This driver can also be built as a module. If so, the module will
> + be called pxe1610.
> +
> config SENSORS_TPS40422
> tristate "TI TPS40422"
> help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index 2219b9300316..b0fbd017a91a 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_SENSORS_MAX20751) += max20751.o
> obj-$(CONFIG_SENSORS_MAX31785) += max31785.o
> obj-$(CONFIG_SENSORS_MAX34440) += max34440.o
> obj-$(CONFIG_SENSORS_MAX8688) += max8688.o
> +obj-$(CONFIG_SENSORS_PXE1610) += pxe1610.o
> obj-$(CONFIG_SENSORS_TPS40422) += tps40422.o
> obj-$(CONFIG_SENSORS_TPS53679) += tps53679.o
> obj-$(CONFIG_SENSORS_UCD9000) += ucd9000.o
> diff --git a/drivers/hwmon/pmbus/pxe1610.c b/drivers/hwmon/pmbus/pxe1610.c
> new file mode 100644
> index 000000000000..ebe3f023f840
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/pxe1610.c
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Hardware monitoring driver for Infineon PXE1610
> + *
> + * Copyright (c) 2019 Facebook Inc
> + *
> + */
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include "pmbus.h"
> +
> +#define PXE1610_NUM_PAGES 3
> +
> +/* Identify chip parameters. */
> +static int pxe1610_identify(struct i2c_client *client,
> + struct pmbus_driver_info *info)
> +{
> + if (pmbus_check_byte_register(client, 0, PMBUS_VOUT_MODE)) {
> + u8 vout_mode;
> + int ret;
> +
> + /* Read the register with VOUT scaling value.*/
> + ret = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
> + if (ret < 0)
> + return ret;
> +
> + vout_mode = ret & GENMASK(4, 0);
> +
> + switch (vout_mode) {
> + case 1:
> + info->vrm_version = vr12;
> + break;
> + case 2:
> + info->vrm_version = vr13;
> + break;
> + default:
> + return -ENODEV;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static struct pmbus_driver_info pxe1610_info = {
> + .pages = PXE1610_NUM_PAGES,
> + .format[PSC_VOLTAGE_IN] = linear,
> + .format[PSC_VOLTAGE_OUT] = vid,
> + .format[PSC_CURRENT_IN] = linear,
> + .format[PSC_CURRENT_OUT] = linear,
> + .format[PSC_TEMPERATURE] = linear,
> + .format[PSC_POWER] = linear,
> + .func[0] = PMBUS_HAVE_VIN
> + | PMBUS_HAVE_VOUT | PMBUS_HAVE_IIN
> + | PMBUS_HAVE_IOUT | PMBUS_HAVE_PIN
> + | PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP
> + | PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT
> + | PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP,
> + .func[1] = PMBUS_HAVE_VIN
> + | PMBUS_HAVE_VOUT | PMBUS_HAVE_IIN
> + | PMBUS_HAVE_IOUT | PMBUS_HAVE_PIN
> + | PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP
> + | PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT
> + | PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP,
> + .func[2] = PMBUS_HAVE_VIN
> + | PMBUS_HAVE_VOUT | PMBUS_HAVE_IIN
> + | PMBUS_HAVE_IOUT | PMBUS_HAVE_PIN
> + | PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP
> + | PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT
> + | PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP,
> + .identify = pxe1610_identify,
> +};
> +
> +static int pxe1610_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct pmbus_driver_info *info;
> + u8 buf[I2C_SMBUS_BLOCK_MAX];
> + int ret;
> +
> + if (!i2c_check_functionality(
> + client->adapter,
> + I2C_FUNC_SMBUS_READ_BYTE_DATA
> + | I2C_FUNC_SMBUS_READ_WORD_DATA
> + | I2C_FUNC_SMBUS_READ_BLOCK_DATA))
> + return -ENODEV;
> +
> + /*
> + * By default this device doesn't boot to page 0, so set page 0
> + * to access all pmbus registers.
> + */
> + i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
> +
> + /* Read Manufacturer id */
> + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to read PMBUS_MFR_ID\n");
> + return ret;
> + }
> + if (ret != 2 || strncmp(buf, "XP", 2)) {
> + dev_err(&client->dev, "MFR_ID unrecognized\n");
> + return -ENODEV;
> + }
> +
> + info = devm_kmemdup(&client->dev, &pxe1610_info,
> + sizeof(struct pmbus_driver_info),
> + GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + return pmbus_do_probe(client, id, info);
> +}
> +
> +static const struct i2c_device_id pxe1610_id[] = {
> + {"pxe1610", 0},
> + {"pxe1110", 0},
> + {"pxm1310", 0},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, pxe1610_id);
> +
> +static struct i2c_driver pxe1610_driver = {
> + .driver = {
> + .name = "pxe1610",
> + },
> + .probe = pxe1610_probe,
> + .remove = pmbus_do_remove,
> + .id_table = pxe1610_id,
> +};
> +
> +module_i2c_driver(pxe1610_driver);
> +
> +MODULE_AUTHOR("Vijay Khemka <vijaykhemka@fb.com>");
> +MODULE_DESCRIPTION("PMBus driver for Infineon PXE1610, PXE1110 and PXM1310");
> +MODULE_LICENSE("GPL");
^ permalink raw reply
* Re: [PATCH v2 2/2] Docs: hwmon: pmbus: Add PXE1610 driver
From: Guenter Roeck @ 2019-06-05 20:48 UTC (permalink / raw)
To: Vijay Khemka
Cc: Jean Delvare, Jonathan Corbet, linux-hwmon, linux-doc,
linux-kernel, joel, linux-aspeed, sdasari, Greg Kroah-Hartman
In-Reply-To: <20190530231159.222188-2-vijaykhemka@fb.com>
On Thu, May 30, 2019 at 04:11:57PM -0700, Vijay Khemka wrote:
> Added support for Infenion PXE1610 driver
>
Applied, after fixing
s/Infenion/Infineon/
s/Infinion/Infineon/
Guenter
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
> Changes in v2:
> incorporated all the feedback from Guenter Roeck <linux@roeck-us.net>
>
> Documentation/hwmon/pxe1610 | 90 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 90 insertions(+)
> create mode 100644 Documentation/hwmon/pxe1610
>
> diff --git a/Documentation/hwmon/pxe1610 b/Documentation/hwmon/pxe1610
> new file mode 100644
> index 000000000000..24825db8736f
> --- /dev/null
> +++ b/Documentation/hwmon/pxe1610
> @@ -0,0 +1,90 @@
> +Kernel driver pxe1610
> +=====================
> +
> +Supported chips:
> + * Infinion PXE1610
> + Prefix: 'pxe1610'
> + Addresses scanned: -
> + Datasheet: Datasheet is not publicly available.
> +
> + * Infinion PXE1110
> + Prefix: 'pxe1110'
> + Addresses scanned: -
> + Datasheet: Datasheet is not publicly available.
> +
> + * Infinion PXM1310
> + Prefix: 'pxm1310'
> + Addresses scanned: -
> + Datasheet: Datasheet is not publicly available.
> +
> +Author: Vijay Khemka <vijaykhemka@fb.com>
> +
> +
> +Description
> +-----------
> +
> +PXE1610/PXE1110 are Multi-rail/Multiphase Digital Controllers
> +and compliant to
> + -- Intel VR13 DC-DC converter specifications.
> + -- Intel SVID protocol.
> +Used for Vcore power regulation for Intel VR13 based microprocessors
> + -- Servers, Workstations, and High-end desktops
> +
> +PXM1310 is a Multi-rail Controllers and it is compliant to
> + -- Intel VR13 DC-DC converter specifications.
> + -- Intel SVID protocol.
> +Used for DDR3/DDR4 Memory power regulation for Intel VR13 and
> +IMVP8 based systems
> +
> +
> +Usage Notes
> +-----------
> +
> +This driver does not probe for PMBus devices. You will have
> +to instantiate devices explicitly.
> +
> +Example: the following commands will load the driver for an PXE1610
> +at address 0x70 on I2C bus #4:
> +
> +# modprobe pxe1610
> +# echo pxe1610 0x70 > /sys/bus/i2c/devices/i2c-4/new_device
> +
> +It can also be instantiated by declaring in device tree
> +
> +
> +Sysfs attributes
> +----------------
> +
> +curr1_label "iin"
> +curr1_input Measured input current
> +curr1_alarm Current high alarm
> +
> +curr[2-4]_label "iout[1-3]"
> +curr[2-4]_input Measured output current
> +curr[2-4]_crit Critical maximum current
> +curr[2-4]_crit_alarm Current critical high alarm
> +
> +in1_label "vin"
> +in1_input Measured input voltage
> +in1_crit Critical maximum input voltage
> +in1_crit_alarm Input voltage critical high alarm
> +
> +in[2-4]_label "vout[1-3]"
> +in[2-4]_input Measured output voltage
> +in[2-4]_lcrit Critical minimum output voltage
> +in[2-4]_lcrit_alarm Output voltage critical low alarm
> +in[2-4]_crit Critical maximum output voltage
> +in[2-4]_crit_alarm Output voltage critical high alarm
> +
> +power1_label "pin"
> +power1_input Measured input power
> +power1_alarm Input power high alarm
> +
> +power[2-4]_label "pout[1-3]"
> +power[2-4]_input Measured output power
> +
> +temp[1-3]_input Measured temperature
> +temp[1-3]_crit Critical high temperature
> +temp[1-3]_crit_alarm Chip temperature critical high alarm
> +temp[1-3]_max Maximum temperature
> +temp[1-3]_max_alarm Chip temperature high alarm
^ permalink raw reply
* Re: [PATCH v1] docs/core-api: Add integer power functions to the list
From: Mike Rapoport @ 2019-06-05 21:39 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Jonathan Corbet, linux-doc, Andrew Morton, Mike Rapoport
In-Reply-To: <20190605165113.50972-1-andriy.shevchenko@linux.intel.com>
On Wed, Jun 05, 2019 at 07:51:13PM +0300, Andy Shevchenko wrote:
> Some times integer power functions, such as int_sqrt(), are needed, but
> there is nothing about them in the generated documentation.
>
> Fill the gap by adding a reference to the corresponding exported functions.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Mike Rapoport <rppt@linux.ibm.com>
> ---
> Documentation/core-api/kernel-api.rst | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/Documentation/core-api/kernel-api.rst b/Documentation/core-api/kernel-api.rst
> index 65ae2bf1f86d..824f24ccf401 100644
> --- a/Documentation/core-api/kernel-api.rst
> +++ b/Documentation/core-api/kernel-api.rst
> @@ -141,6 +141,15 @@ Base 2 log and power Functions
> .. kernel-doc:: include/linux/log2.h
> :internal:
>
> +Integer power Functions
> +-----------------------
> +
> +.. kernel-doc:: lib/math/int_pow.c
> + :export:
> +
> +.. kernel-doc:: lib/math/int_sqrt.c
> + :export:
> +
> Division Functions
> ------------------
>
> --
> 2.20.1
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH v1] docs/core-api: Add string helpers API to the list
From: Mike Rapoport @ 2019-06-05 21:39 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Jonathan Corbet, linux-doc, Andrew Morton, Mike Rapoport
In-Reply-To: <20190605163944.50803-1-andriy.shevchenko@linux.intel.com>
On Wed, Jun 05, 2019 at 07:39:44PM +0300, Andy Shevchenko wrote:
> Some times string helpers are needed, but there is nothing about them
> in the generated documentation.
>
> Fill the gap by adding a reference to string_helpers.c exported functions.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Mike Rapoport <rppt@linux.ibm.com>
> ---
> Documentation/core-api/kernel-api.rst | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/core-api/kernel-api.rst b/Documentation/core-api/kernel-api.rst
> index a53ec2eb8176..65ae2bf1f86d 100644
> --- a/Documentation/core-api/kernel-api.rst
> +++ b/Documentation/core-api/kernel-api.rst
> @@ -33,6 +33,9 @@ String Conversions
> .. kernel-doc:: lib/kstrtox.c
> :export:
>
> +.. kernel-doc:: lib/string_helpers.c
> + :export:
> +
> String Manipulation
> -------------------
>
> --
> 2.20.1
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH v4 17/18] kernel/sysctl-test: Add null pointer test for sysctl.c:proc_dointvec()
From: Iurii Zaikin @ 2019-06-06 1:29 UTC (permalink / raw)
To: Stephen Boyd
Cc: Brendan Higgins, frowand.list, gregkh, jpoimboe, keescook,
kieran.bingham, mcgrof, peterz, robh, shuah, tytso,
yamada.masahiro, devicetree, dri-devel, kunit-dev, linux-doc,
linux-fsdevel, linux-kbuild, linux-kernel, linux-kselftest,
linux-nvdimm, linux-um, Alexander.Levin, Tim.Bird, amir73il,
dan.carpenter, daniel, jdike, joel, julia.lawall, khilman,
knut.omang, logang, mpe, pmladek, rdunlap, richard, rientjes,
rostedt, wfg
In-Reply-To: <20190517182254.548EA20815@mail.kernel.org>
On Fri, May 17, 2019 at 11:22 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Brendan Higgins (2019-05-14 15:17:10)
> > diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
> > new file mode 100644
> > index 0000000000000..fe0f2bae66085
> > --- /dev/null
> > +++ b/kernel/sysctl-test.c
> > @@ -0,0 +1,293 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * KUnit test of proc sysctl.
> > + */
> > +
> > +#include <kunit/test.h>
> > +#include <linux/printk.h>
>
> Is this include used?
Deleted.
>
> > +#include <linux/sysctl.h>
> > +#include <linux/uaccess.h>
>
> Is this include used?
Deleted.
>
> > +
> > +
> > +static void sysctl_test_dointvec_happy_single_negative(struct kunit *test)
> > +{
> > + struct ctl_table table = {
> > + .procname = "foo",
> > + .data = &test_data.int_0001,
> > + .maxlen = sizeof(int),
> > + .mode = 0644,
> > + .proc_handler = proc_dointvec,
> > + .extra1 = &i_zero,
> > + .extra2 = &i_one_hundred,
> > + };
> > + char input[] = "-9";
> > + size_t len = sizeof(input) - 1;
> > + loff_t pos = 0;
> > +
> > + table.data = kunit_kzalloc(test, sizeof(int), GFP_USER);
> > + KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&table, 1, input, &len, &pos));
> > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, len);
> > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, pos);
> > + KUNIT_EXPECT_EQ(test, -9, *(int *)table.data);
>
> Is the casting necessary? Or can the macro do a type coercion of the
> second parameter based on the first type?
Data field is defined as void* so I believe casting is necessary to
dereference it as a pointer to an array of ints. I don't think the
macro should do any type coercion that == operator wouldn't do.
I did change the cast to make it more clear that it's a pointer to an
array of ints being dereferenced.
>
> > +}
> > +
> > +static void sysctl_test_dointvec_single_less_int_min(struct kunit *test)
> > +{
> > + struct ctl_table table = {
> > + .procname = "foo",
> > + .data = &test_data.int_0001,
> > + .maxlen = sizeof(int),
> > + .mode = 0644,
> > + .proc_handler = proc_dointvec,
> > + .extra1 = &i_zero,
> > + .extra2 = &i_one_hundred,
> > + };
> > + char input[32];
> > + size_t len = sizeof(input) - 1;
> > + loff_t pos = 0;
> > + unsigned long abs_of_less_than_min = (unsigned long)INT_MAX
> > + - (INT_MAX + INT_MIN) + 1;
> > +
> > + KUNIT_EXPECT_LT(test,
> > + snprintf(input, sizeof(input), "-%lu",
> > + abs_of_less_than_min),
> > + sizeof(input));
> > +
> > + table.data = kunit_kzalloc(test, sizeof(int), GFP_USER);
> > + KUNIT_EXPECT_EQ(test, -EINVAL,
> > + proc_dointvec(&table, 1, input, &len, &pos));
> > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, len);
> > + KUNIT_EXPECT_EQ(test, 0, *(int *)table.data);
> > +}
> > +
> > +static void sysctl_test_dointvec_single_greater_int_max(struct kunit *test)
> > +{
> > + struct ctl_table table = {
> > + .procname = "foo",
> > + .data = &test_data.int_0001,
> > + .maxlen = sizeof(int),
> > + .mode = 0644,
> > + .proc_handler = proc_dointvec,
> > + .extra1 = &i_zero,
> > + .extra2 = &i_one_hundred,
> > + };
> > + char input[32];
> > + size_t len = sizeof(input) - 1;
> > + loff_t pos = 0;
> > + unsigned long greater_than_max = (unsigned long)INT_MAX + 1;
> > +
> > + KUNIT_EXPECT_GT(test, greater_than_max, INT_MAX);
> > + KUNIT_EXPECT_LT(test, snprintf(input, sizeof(input), "%lu",
> > + greater_than_max),
> > + sizeof(input));
> > + table.data = kunit_kzalloc(test, sizeof(int), GFP_USER);
> > + KUNIT_EXPECT_EQ(test, -EINVAL,
> > + proc_dointvec(&table, 1, input, &len, &pos));
> > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, len);
> > + KUNIT_EXPECT_EQ(test, 0, *(int *)table.data);
> > +}
> > +
> > +static int sysctl_test_init(struct kunit *test)
> > +{
> > + return 0;
> > +}
> > +
> > +/*
> > + * This is run once after each test case, see the comment on example_test_module
> > + * for more information.
> > + */
> > +static void sysctl_test_exit(struct kunit *test)
> > +{
> > +}
> Can the above two be omitted? If they can be empty sometimes it would be
> nice to avoid the extra symbols and code by letting them be assigned to
> NULL in the kunit_module.
Deleted.
>
> > +
> > +/*
> > + * Here we make a list of all the test cases we want to add to the test module
> > + * below.
> > + */
> > +static struct kunit_case sysctl_test_cases[] = {
> > + /*
> > + * This is a helper to create a test case object from a test case
> > + * function; its exact function is not important to understand how to
> > + * use KUnit, just know that this is how you associate test cases with a
> > + * test module.
> > + */
> > + KUNIT_CASE(sysctl_test_dointvec_null_tbl_data),
> > + KUNIT_CASE(sysctl_test_dointvec_table_maxlen_unset),
> > + KUNIT_CASE(sysctl_test_dointvec_table_len_is_zero),
> > + KUNIT_CASE(sysctl_test_dointvec_table_read_but_position_set),
> > + KUNIT_CASE(sysctl_test_dointvec_happy_single_positive),
> > + KUNIT_CASE(sysctl_test_dointvec_happy_single_negative),
> > + KUNIT_CASE(sysctl_test_dointvec_single_less_int_min),
> > + KUNIT_CASE(sysctl_test_dointvec_single_greater_int_max),
> > + {},
> > +};
> > +
> > +/*
> > + * This defines a suite or grouping of tests.
> > + *
> > + * Test cases are defined as belonging to the suite by adding them to
> > + * `test_cases`.
> > + *
> > + * Often it is desirable to run some function which will set up things which
> > + * will be used by every test; this is accomplished with an `init` function
> > + * which runs before each test case is invoked. Similarly, an `exit` function
> > + * may be specified which runs after every test case and can be used to for
> > + * cleanup. For clarity, running tests in a test module would behave as follows:
> > + *
> > + * module.init(test);
> > + * module.test_case[0](test);
> > + * module.exit(test);
> > + * module.init(test);
> > + * module.test_case[1](test);
> > + * module.exit(test);
> > + * ...;
>
> This comment (and the one above for "this is a helper") looks generic
> and should probably only be in some documentation somewhere and not for
> a sysctl test?
>
Deleted.
> > + */
> > +static struct kunit_module sysctl_test_module = {
> > + .name = "sysctl_test",
> > + .init = sysctl_test_init,
> > + .exit = sysctl_test_exit,
> > + .test_cases = sysctl_test_cases,
> > +};
> > +
> > +/*
> > + * This registers the above test module telling KUnit that this is a suite of
> > + * tests that need to be run.
> > + */
>
> Same comment about generic comment.
>
Deleted.
> > +module_test(sysctl_test_module);
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index d5a4a4036d2f8..772af4ec70111 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -1908,6 +1908,12 @@ config TEST_SYSCTL
> >
> > If unsure, say N.
> >
> > +config SYSCTL_KUNIT_TEST
> > + bool "KUnit test for sysctl"
>
> Why not tristate?
>
I don't believe KUnit as a module is currently supported.
> > + depends on KUNIT
> > + help
> > + Enables KUnit sysctl test.
> > +
^ permalink raw reply
* PCI: Correct the resource_alignment parameter example
From: Alexey Kardashevskiy @ 2019-06-06 3:25 UTC (permalink / raw)
To: linux-kernel; +Cc: Alexey Kardashevskiy, linux-doc, Bjorn Helgaas, linux-pci
The option description requires an order and so does the option
parsing code, however the example uses a size, fix this.
Fixes: 8b078c603249 ("PCI: Update "pci=resource_alignment" documentation")
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Documentation/admin-guide/kernel-parameters.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2b8ee90bb644..dcb53d64ad74 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3340,27 +3340,28 @@
resource_alignment=
Format:
[<order of align>@]<pci_dev>[; ...]
Specifies alignment and device to reassign
aligned memory resources. How to
specify the device is described above.
If <order of align> is not specified,
PAGE_SIZE is used as alignment.
PCI-PCI bridge can be specified, if resource
windows need to be expanded.
To specify the alignment for several
instances of a device, the PCI vendor,
device, subvendor, and subdevice may be
- specified, e.g., 4096@pci:8086:9c22:103c:198f
+ specified, e.g., 12@pci:8086:9c22:103c:198f
+ for the 4096 alignment.
ecrc= Enable/disable PCIe ECRC (transaction layer
end-to-end CRC checking).
bios: Use BIOS/firmware settings. This is the
the default.
off: Turn ECRC off
on: Turn ECRC on.
hpiosize=nn[KMG] The fixed amount of bus space which is
reserved for hotplug bridge's IO window.
Default size is 256 bytes.
hpmemsize=nn[KMG] The fixed amount of bus space which is
reserved for hotplug bridge's memory window.
Default size is 2 megabytes.
hpbussize=nn The minimum amount of additional bus numbers
--
2.17.1
^ permalink raw reply related
* Re: PCI: Correct the resource_alignment parameter example
From: Alexey Kardashevskiy @ 2019-06-06 4:38 UTC (permalink / raw)
To: linux-kernel, Bjorn Helgaas; +Cc: linux-doc, linux-pci
In-Reply-To: <20190606032557.107542-1-aik@ozlabs.ru>
Argh, it should have had "PATCH" in the subject, I broke my scripts so
they run git format-patch --subject-prefix="". Sorry. Repost?
On 06/06/2019 13:25, Alexey Kardashevskiy wrote:
> The option description requires an order and so does the option
> parsing code, however the example uses a size, fix this.
>
> Fixes: 8b078c603249 ("PCI: Update "pci=resource_alignment" documentation")
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Documentation/admin-guide/kernel-parameters.txt | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 2b8ee90bb644..dcb53d64ad74 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3340,27 +3340,28 @@
> resource_alignment=
> Format:
> [<order of align>@]<pci_dev>[; ...]
> Specifies alignment and device to reassign
> aligned memory resources. How to
> specify the device is described above.
> If <order of align> is not specified,
> PAGE_SIZE is used as alignment.
> PCI-PCI bridge can be specified, if resource
> windows need to be expanded.
> To specify the alignment for several
> instances of a device, the PCI vendor,
> device, subvendor, and subdevice may be
> - specified, e.g., 4096@pci:8086:9c22:103c:198f
> + specified, e.g., 12@pci:8086:9c22:103c:198f
> + for the 4096 alignment.
> ecrc= Enable/disable PCIe ECRC (transaction layer
> end-to-end CRC checking).
> bios: Use BIOS/firmware settings. This is the
> the default.
> off: Turn ECRC off
> on: Turn ECRC on.
> hpiosize=nn[KMG] The fixed amount of bus space which is
> reserved for hotplug bridge's IO window.
> Default size is 256 bytes.
> hpmemsize=nn[KMG] The fixed amount of bus space which is
> reserved for hotplug bridge's memory window.
> Default size is 2 megabytes.
> hpbussize=nn The minimum amount of additional bus numbers
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 2/3] treewide: trivial: fix s/poped/popped/ typo
From: Steven Rostedt @ 2019-06-06 6:47 UTC (permalink / raw)
To: George G. Davis
Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
H. Peter Anvin, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
Jiri Kosina, Masami Hiramatsu, Andi Kleen, Jann Horn, Nadav Amit,
open list:DOCUMENTATION, open list
In-Reply-To: <1559766612-12178-2-git-send-email-george_davis@mentor.com>
On Wed, 5 Jun 2019 16:30:10 -0400
"George G. Davis" <george_davis@mentor.com> wrote:
> Fix a couple of s/poped/popped/ typos.
>
> Cc: Jiri Kosina <trivial@kernel.org>
> Signed-off-by: George G. Davis <george_davis@mentor.com>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
-- Steve
> ---
> Documentation/arm/mem_alignment | 2 +-
> arch/x86/kernel/kprobes/core.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/arm/mem_alignment b/Documentation/arm/mem_alignment
> index 6335fcacbba9..e110e2781039 100644
> --- a/Documentation/arm/mem_alignment
> +++ b/Documentation/arm/mem_alignment
> @@ -1,4 +1,4 @@
> -Too many problems poped up because of unnoticed misaligned memory access in
> +Too many problems popped up because of unnoticed misaligned memory access in
> kernel code lately. Therefore the alignment fixup is now unconditionally
> configured in for SA11x0 based targets. According to Alan Cox, this is a
> bad idea to configure it out, but Russell King has some good reasons for
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index 6afd8061dbae..d3243d93daf4 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -813,7 +813,7 @@ __used __visible void *trampoline_handler(struct pt_regs *regs)
> continue;
> /*
> * Return probes must be pushed on this hash list correct
> - * order (same as return order) so that it can be poped
> + * order (same as return order) so that it can be popped
> * correctly. However, if we find it is pushed it incorrect
> * order, this means we find a function which should not be
> * probed, because the wrong order entry is pushed on the
^ permalink raw reply
* Re: [PATCH 1/2] Documentation/i915: Fix kernel-doc references to moved gem files
From: Jani Nikula @ 2019-06-06 8:23 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx; +Cc: linux-doc, Chris Wilson
In-Reply-To: <87d0jss4uo.fsf@gaia.fi.intel.com>
On Wed, 05 Jun 2019, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> Jani Nikula <jani.nikula@intel.com> writes:
>
>> The error messages could be more descriptive, but fix these caused by
>> file moves:
>>
>> WARNING: kernel-doc './scripts/kernel-doc -rst -enable-lineno -internal
>> ./drivers/gpu/drm/i915/i915_gem_shrinker.c' failed with return code 2
>> WARNING: kernel-doc './scripts/kernel-doc -rst -enable-lineno -function
>> User command execution ./drivers/gpu/drm/i915/i915_gem_execbuffer.c'
>> failed with return code 1
>> WARNING: kernel-doc './scripts/kernel-doc -rst -enable-lineno -internal
>> ./drivers/gpu/drm/i915/i915_gem_tiling.c' failed with return code 2
>> WARNING: kernel-doc './scripts/kernel-doc -rst -enable-lineno -function
>> buffer object tiling ./drivers/gpu/drm/i915/i915_gem_tiling.c'
>> failed with return code 1
>>
>> Fixes: 10be98a77c55 ("drm/i915: Move more GEM objects under gem/")
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> /o\
>
> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Thanks for the reviews, I took the liberty of pushing these to dinq with
just BAT results, as these are purely documentation/comment changes.
BR,
Jani.
>
>> ---
>> Documentation/gpu/i915.rst | 11 ++++-------
>> 1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
>> index 6c75380b2928..f98ee95da90f 100644
>> --- a/Documentation/gpu/i915.rst
>> +++ b/Documentation/gpu/i915.rst
>> @@ -349,7 +349,7 @@ of buffer object caches. Shrinking is used to make main memory
>> available. Note that this is mostly orthogonal to evicting buffer
>> objects, which has the goal to make space in gpu virtual address spaces.
>>
>> -.. kernel-doc:: drivers/gpu/drm/i915/i915_gem_shrinker.c
>> +.. kernel-doc:: drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
>> :internal:
>>
>> Batchbuffer Parsing
>> @@ -373,7 +373,7 @@ Batchbuffer Pools
>> User Batchbuffer Execution
>> --------------------------
>>
>> -.. kernel-doc:: drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> +.. kernel-doc:: drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>> :doc: User command execution
>>
>> Logical Rings, Logical Ring Contexts and Execlists
>> @@ -382,9 +382,6 @@ Logical Rings, Logical Ring Contexts and Execlists
>> .. kernel-doc:: drivers/gpu/drm/i915/gt/intel_lrc.c
>> :doc: Logical Rings, Logical Ring Contexts and Execlists
>>
>> -.. kernel-doc:: drivers/gpu/drm/i915/gt/intel_lrc.c
>> - :internal:
>> -
>> Global GTT views
>> ----------------
>>
>> @@ -415,10 +412,10 @@ Hardware Tiling and Swizzling Details
>> Object Tiling IOCTLs
>> --------------------
>>
>> -.. kernel-doc:: drivers/gpu/drm/i915/i915_gem_tiling.c
>> +.. kernel-doc:: drivers/gpu/drm/i915/gem/i915_gem_tiling.c
>> :internal:
>>
>> -.. kernel-doc:: drivers/gpu/drm/i915/i915_gem_tiling.c
>> +.. kernel-doc:: drivers/gpu/drm/i915/gem/i915_gem_tiling.c
>> :doc: buffer object tiling
>>
>> WOPCM
>> --
>> 2.20.1
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply
* Re: [PATCH v2] Add a document on rebasing and merging
From: Jani Nikula @ 2019-06-06 9:12 UTC (permalink / raw)
To: Jonathan Corbet, linux-doc
Cc: LKML, Linus Torvalds, Theodore Ts'o, Geert Uytterhoeven,
David Rientjes
In-Reply-To: <20190604134835.16fc6bfa@lwn.net>
On Tue, 04 Jun 2019, Jonathan Corbet <corbet@lwn.net> wrote:
> 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.
Good stuff. Some notes inline.
BR,
Jani.
>
> 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
I think "backmerge" as a term deserves to be highlighted in the heading
or first paragraph of the section.
Occasionally backmerges are required. As a rule of thumb, it might be
worth mentioning you probably shouldn't do such merges across subsystem
hierarchies, i.e. ask the level above you to do a backmerge first, and
then backmerge from them. And that when backmerging from Linus' tree,
the merge point should be a tag.
> +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.
Is it not a common convention to call these special branches "topic
branches"?
FWIW, I don't think I've ever done a cross-merge or a direct merge from
a sibling tree. I've always solved the cases either by topic branches
merged to both trees or by having both trees merged to the first common
upstream tree, and then backmerging. From my POV feels like these
solutions should be presented more prominently than cross-merges.
> +
> +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
>
--
Jani Nikula, Intel Open Source Graphics Center
^ 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