Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
On Wednesday 28 February 2018 07:59 PM, Guenter Roeck wrote:
On 02/27/2018 11:03 PM, Mikko Perttunen wrote:
On 02/28/2018 08:12 AM, Rajkumar Rampelli wrote:
On Wednesday 28 February 2018 11:28 AM, Guenter Roeck wrote:
On 02/27/2018 09:38 PM, Rajkumar Rampelli wrote:
On Wednesday 21 February 2018 08:20 PM, Guenter Roeck wrote:
On 02/20/2018 10:58 PM, Rajkumar Rampelli wrote:
Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.
This driver implements a simple interface for monitoring the
speed of
a fan and exposes it in roatations per minute (RPM) to the user
space
by using the hwmon's sysfs interface
Signed-off-by: Rajkumar Rampelli
---
Documentation/hwmon/generic-pwm-tachometer | 17 +
drivers/hwmon/Kconfig | 10 +++
drivers/hwmon/Makefile | 1 +
drivers/hwmon/generic-pwm-tachometer.c | 112
+
4 files changed, 140 insertions(+)
create mode 100644 Documentation/hwmon/generic-pwm-tachometer
create mode 100644 drivers/hwmon/generic-pwm-tachometer.c
diff --git a/Documentation/hwmon/generic-pwm-tachometer
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan.
It uses the
+generic PWM interface and can be used on SoCs as along as the
SoC supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan
speed using
+PWM module and Tachometer controller. It requests period value
through PWM
+capture interface to Tachometer and measures the Rotations per
minute using
+received period value. It exposes the Fan speed in RPM to the
user space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
If you say yes here you get support for the temperature
and power sensors for APM X-Gene SoC.
+config GENERIC_PWM_TACHOMETER
+tristate "Generic PWM based tachometer driver"
+depends on PWM
+help
+ Enables a driver to use PWM signal from motor to use
+ for measuring the motor speed. The RPM is captured by
+ PWM modules which has PWM capture capability and this
+ drivers reads the captured data from PWM IP to convert
+ it to speed in RPM.
+
if ACPI
comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)+=
wm8350-hwmon.o
obj-$(CONFIG_SENSORS_XGENE)+= xgene-hwmon.o
obj-$(CONFIG_PMBUS)+= pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o
ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
diff --git a/drivers/hwmon/generic-pwm-tachometer.c
b/drivers/hwmon/generic-pwm-tachometer.c
new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION. All rights
reserved.
+ *
+ * This program is free software; you can redistribute it
and/or modify it
+ * under the terms and conditions of the GNU General Public
License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful,
but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
Public License for
+ * more details.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct pwm_hwmon_tach {
+struct device*dev;
+struct pwm_device*pwm;
+struct device*hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct
device_attribute *attr,
+char *buf)
+{
+struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+struct pwm_device *pwm = ptt->pwm;
+struct pwm_capture result;
+int err;
+unsigned int rpm = 0;
+
+err = pwm_capture(pwm, &result, 0);
+if (err < 0) {
+dev_err(ptt->dev, "Failed to capture PWM: %d\n", err);
+return err;
+}
+
+if (result.period)
+rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+result.period);
+
+return sprintf(buf, "%u\n", rpm);
+}
+
+static SENSOR_DEVICE_ATTR(rpm, 0444, show_rpm, NULL, 0);
+
+static struct attribute *pwm_tach_attrs[] = {
+&sensor_dev_attr_rpm.dev_attr.attr,
+
Re: [PATCH v16 00/13] support "task_isolation" mode
Hi Chris,
(CC Cavium people)
Thanks for your series.
On Fri, Nov 03, 2017 at 01:04:39PM -0400, Chris Metcalf wrote:
> Here, finally, is a new spin of the task isolation work (v16), with
> changes based on the issues that were raised at last year's Linux
> Plumbers Conference and in the email discussion that followed.
>
> This version of the patch series cleans up a number of areas that were
> a little dodgy in the previous patch series.
>
> - It no longer loops in the final code that prepares to return to
> userspace; instead, it sets things up in the prctl() and then
> validates when preparing to return to userspace, adjusting the
> syscall return value to -EAGAIN at that point if something doesn't
> line up quite correctly.
>
> - We no longer support the NOSIG mode that let you freely call into
> the kernel multiple times while in task isolation. This was always
> a little odd, since you really should be in sufficient control of
> task isolation code that you can explicitly stop isolation with a
> "prctl(PR_TASK_ISOLATION, 0)" before using the kernel for anything
> else. It also made the semantics of migrating the task confusing.
> More importantly, removing that support means that the only path
> that sets up task isolation is the return from prctl(), which allows
> us to make the simplification above.
>
> - We no longer try to signal the task isolation process from a remote
> core when we detect that we are about to violate its isolation.
> Instead, we just print a message there (and optionally dump stack),
> and rely on the eventual interrupt on the core itself to trigger the
> signal. We are always in a safe context to generate a signal when
> we enter the kernel, unlike when we are deep in a call stack sending
> an SMP IPI or whatever.
>
> - We notice the case of an unstable scheduler clock and return
> EINVAL rather than spinning forever with EAGAIN (suggestion from
> Francis Giraldeau).
>
> - The prctl() call requires zeros for arg3/4/5 (suggestion from
> Eugene Syromiatnikov).
>
> The kernel internal isolation API is also now cleaner, and I have
> included kerneldoc APIs for all the interfaces so that it should be
> easier to port it to additional architectures; in fact looking at
> include/linux/isolation.h is a good place to start understanding the
> overall patch set.
>
> I removed Catalin's Reviewed-by for arm64, and Christoph's Tested-by
> for x86, since this version is sufficiently different to merit
> re-review and re-testing.
>
> Note that this is not rebased on top of Frederic's recent housekeeping
> patch series, although it is largely orthogonal right now. After
> Frederic's patch series lands, task isolation is enabled with
> "isolcpus=nohz,domain,CPUS". We could add some shorthand for that
> ("isolcpus=full,CPUS"?) or just use it as-is.
>
> The previous (v15) patch series is here:
>
> https://lkml.kernel.org/r/1471382376-5443-1-git-send-email-cmetc...@mellanox.com
>
> This patch series is available at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/cmetcalf/linux-tile.git
> dataplane
>
> Some folks raised some good points at the LPC discussion and then in
> email discussions that followed. Rather than trying to respond to
> everyone in a flurry of emails, I'll answer some questions here:
>
>
> Why not just instrument user_exit() to raise the isolation-lost signal?
>
> Andy pointed me in this direction. The advantage is that you catch
> *everything*, by definition. There is a hook that can do this in the
> current patch set, but you have to #define DEBUG_TASK_ISOLATION
> manually to take advantage of it, because as written it has two issues:
>
> 1. You can't actually exit the kernel with prctl(PR_TASK_ISOLATION,0)
>because the user_exit hook kills you first.
> 2. You lose the ability to get much better diagnostics by waiting
>until you are further into kernel entry and know what you're doing.
>
> You could work around #2 in several ways, but #1 is harder. I looked
> at x86 for a while, and although you could imagine this, you really
> want to generate a lost-isolation signal on any syscall that isn't
> that exact prctl(), and it's awkward to try to do all of that checking
> before user_exit(). Since in any case we do want to have the more
> specific probes at the various kernel entry points where we generate
> the diagnostics, I felt like it wasn't the right approach to enable
> as a compilation-time default. I'm open to discussion on this though!
>
>
> Can't we do all the exit-to-userspace work with irqs disabled?
>
> In fact, it turns out that you can do lru_add_drain() with irqs
> disabled, so that's what we're doing in the patch series now.
>
> However, it doesn't seem possible to do the synchronous cancellation of
> the vmstat deferred work with irqs disabled, though if there's a way,
> it would be a little cleaner to do that; Christoph? We can certainly
> update the statistics with inte
Re: [PATCH V2 5/7] thermal/drivers/cpu_cooling: Add idle cooling device documentation
On 07/03/2018 00:19, Pavel Machek wrote: > Hi! Hi Pavel, thanks for taking the time to review the documentation. >> --- /dev/null >> +++ b/Documentation/thermal/cpu-idle-cooling.txt >> @@ -0,0 +1,165 @@ >> + >> +Situation: >> +-- >> + > > Can we have some real header here? Also if this is .rst, maybe it > should be marked so? Ok, I will fix it. >> +Under certain circumstances, the SoC reaches a temperature exceeding >> +the allocated power budget or the maximum temperature limit. The > > I don't understand. Power budget is in W, temperature is in > kelvin. Temperature can't exceed power budget AFAICT. Yes, it is badly worded. Is the following better ? " Under certain circumstances a SoC can reach the maximum temperature limit or is unable to stabilize the temperature around a temperature control. When the SoC has to stabilize the temperature, the kernel can act on a cooling device to mitigate the dissipated power. When the maximum temperature is reached and to prevent a catastrophic situation a radical decision must be taken to reduce the temperature under the critical threshold, that impacts the performance. " >> +former must be mitigated to stabilize the SoC temperature around the >> +temperature control using the defined cooling devices, the latter > > later? > >> +catastrophic situation where a radical decision must be taken to >> +reduce the temperature under the critical threshold, that can impact >> +the performances. > > performance. > >> +Another situation is when the silicon reaches a certain temperature >> +which continues to increase even if the dynamic leakage is reduced to >> +its minimum by clock gating the component. The runaway phenomena will >> +continue with the static leakage and only powering down the component, >> +thus dropping the dynamic and static leakage will allow the component >> +to cool down. This situation is critical. > > Critical here, critical there. I have trouble following > it. Theoretically hardware should protect itself, because you don't > want kernel bug to damage your CPU? There are several levels of protection. The first level is mitigating the temperature from the kernel, then in the temperature sensor a reset line will trigger the reboot of the CPUs. Usually it is a register where you write the maximum temperature, from the driver itself. I never tried to write 1000°C in this register and see if I can burn the board. I know some boards have another level of thermal protection in the hardware itself and some other don't. In any case, from a kernel point of view, it is a critical situation as we are about to hard reboot the system and in this case it is preferable to drop drastically the performance but give the opportunity to the system to run in a degraded mode. >> +Last but not least, the system can ask for a specific power budget but >> +because of the OPP density, we can only choose an OPP with a power >> +budget lower than the requested one and underuse the CPU, thus losing >> +performances. In other words, one OPP under uses the CPU with a > > performance. > >> +lesser than the power budget and the next OPP exceed the power budget, >> +an intermediate OPP could have been used if it were present. > > was. > >> +Solutions: >> +-- >> + >> +If we can remove the static and the dynamic leakage for a specific >> +duration in a controlled period, the SoC temperature will >> +decrease. Acting at the idle state duration or the idle cycle > > "should" decrease? If you are in bad environment.. No, it will decrease in any case because of the static leakage drop. The bad environment will impact the speed of this decrease. >> +The Operating Performance Point (OPP) density has a great influence on >> +the control precision of cpufreq, however different vendors have a >> +plethora of OPP density, and some have large power gap between OPPs, >> +that will result in loss of performance during thermal control and >> +loss of power in other scenes. > > scene seems to be wrong word here. yes, 'scenario' will be better :) >> +At a specific OPP, we can assume injecting idle cycle on all CPUs, > > Extra comma? > >> +Idle Injection: >> +--- >> + >> +The base concept of the idle injection is to force the CPU to go to an >> +idle state for a specified time each control cycle, it provides >> +another way to control CPU power and heat in addition to >> +cpufreq. Ideally, if all CPUs of a cluster inject idle synchronously, >> +this cluster can get into the deepest idle state and achieve minimum >> +power consumption, but that will also increase system response latency >> +if we inject less than cpuidle latency. > > I don't understand last sentence. Is it better ? "Ideally, if all CPUs, belonging to the same cluster, inject their idle cycle synchronously, the cluster can reach its power down state with a minimum power consumption and static leakage drop. However, these idle cycles injection will add extra latencies as the CP
Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
On 03/07/2018 01:47 AM, Rajkumar Rampelli wrote:
On Wednesday 28 February 2018 07:59 PM, Guenter Roeck wrote:
On 02/27/2018 11:03 PM, Mikko Perttunen wrote:
On 02/28/2018 08:12 AM, Rajkumar Rampelli wrote:
On Wednesday 28 February 2018 11:28 AM, Guenter Roeck wrote:
On 02/27/2018 09:38 PM, Rajkumar Rampelli wrote:
On Wednesday 21 February 2018 08:20 PM, Guenter Roeck wrote:
On 02/20/2018 10:58 PM, Rajkumar Rampelli wrote:
Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.
This driver implements a simple interface for monitoring the speed of
a fan and exposes it in roatations per minute (RPM) to the user space
by using the hwmon's sysfs interface
Signed-off-by: Rajkumar Rampelli
---
Documentation/hwmon/generic-pwm-tachometer | 17 +
drivers/hwmon/Kconfig | 10 +++
drivers/hwmon/Makefile | 1 +
drivers/hwmon/generic-pwm-tachometer.c | 112 +
4 files changed, 140 insertions(+)
create mode 100644 Documentation/hwmon/generic-pwm-tachometer
create mode 100644 drivers/hwmon/generic-pwm-tachometer.c
diff --git a/Documentation/hwmon/generic-pwm-tachometer
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan. It uses the
+generic PWM interface and can be used on SoCs as along as the SoC supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan speed using
+PWM module and Tachometer controller. It requests period value through PWM
+capture interface to Tachometer and measures the Rotations per minute using
+received period value. It exposes the Fan speed in RPM to the user space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
If you say yes here you get support for the temperature
and power sensors for APM X-Gene SoC.
+config GENERIC_PWM_TACHOMETER
+ tristate "Generic PWM based tachometer driver"
+ depends on PWM
+ help
+ Enables a driver to use PWM signal from motor to use
+ for measuring the motor speed. The RPM is captured by
+ PWM modules which has PWM capture capability and this
+ drivers reads the captured data from PWM IP to convert
+ it to speed in RPM.
+
if ACPI
comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
obj-$(CONFIG_SENSORS_XGENE) += xgene-hwmon.o
obj-$(CONFIG_PMBUS) += pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o
ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
diff --git a/drivers/hwmon/generic-pwm-tachometer.c
b/drivers/hwmon/generic-pwm-tachometer.c
new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct pwm_hwmon_tach {
+ struct device *dev;
+ struct pwm_device *pwm;
+ struct device *hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+ struct pwm_device *pwm = ptt->pwm;
+ struct pwm_capture result;
+ int err;
+ unsigned int rpm = 0;
+
+ err = pwm_capture(pwm, &result, 0);
+ if (err < 0) {
+ dev_err(ptt->dev, "Failed to capture PWM: %d\n", err);
+ return err;
+ }
+
+ if (result.period)
+ rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+ result.period);
+
+ return sprintf(buf, "%u\n", rpm);
+}
+
+static SENSOR_DEVICE_ATTR(rpm, 0444, show_rpm, NULL, 0);
+
+static struct attribute *pwm_tach_attrs[] = {
+ &sensor_dev_attr_r
Re: [PATCH] HID: ntrig: document sysfs interface
On Fri, 2 Mar 2018, Aishwarya Pant wrote: > Add sysfs documentation for N-Trig touchscreens under Documentation/ABI. > Descriptions have been collected from code comments. Applied, thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 15/15] selinux: delay sid population for rootfs till init is complete
On 02/20/2018 12:56 PM, Stephen Smalley wrote: > On Fri, 2018-02-16 at 20:33 +, Taras Kondratiuk wrote: >> From: Victor Kamensky >> >> With initramfs cpio format that supports extended attributes >> we need to skip sid population on sys_lsetxattr call from >> initramfs for rootfs if security server is not initialized yet. >> >> Otherwise callback in selinux_inode_post_setxattr will try to >> translate give security.selinux label into sid context and since >> security server is not available yet inode will receive default >> sid (typically kernel_t). Note that in the same time proper >> label will be stored in inode xattrs. Later, since inode sid >> would be already populated system will never look back at >> actual xattrs. But if we skip sid population for rootfs and >> we have policy that direct use of xattrs for rootfs, proper >> sid will be filled in from extended attributes one node is >> accessed and server is initialized. >> >> Note new DELAYAFTERINIT_MNT super block flag is introduced >> to only mark rootfs for such behavior. For other types of >> tmpfs original logic is still used. > > (cc selinux maintainers) > > Wondering if we shouldn't just do this always, for all filesystem > types. Also, I think this should likely also be done in > selinux_inode_setsecurity() for consistency. I don't understand what selinux thinks it's doing here. Initramfs is special because it's populated early, ideally early enough drivers can load their firmware out of it. This is guaranteed to be before any processes have launched, before any other filesystems have been mounted. I'm surprised selinux is trying to do anything this early because A) what is there for it to do, B) where did it get a ruleset? This isn't really a mount flag, this is a "the selinux subsystem isn't functionally initialized yet" flag. We haven't launched init. In a modular system the module probably isn't loaded. There are no processes, and the only files anywhere are the ones we're in the process of extracting. What's there fore selinux to do? When a filesystem is mounted, none of these cached selinux "we already looked at the xattrs" inode fields are populated yet, correct? It can figure that out when something accesses the file and do it then, so the point is _not_ doing this now and thus not cacheing the wrong info. That's what the mount flag is doing, telling selinux "not yet". So why does selinux not already _know_ "not yet"? Why doesn't load_policy flush the cache of the old default contexts? What happens if you mount an ext2 root and then init reads a dozen files before it gets to the load_policy? Do those doesn't files have bad default contexts forever now? Where does the selinux ruleset come from during the cpio extract? Was it hardwired into the driver? It certainly didn't come out of a file, and it wasn't a process that loaded it. Why is selinux trying to evaluate and cache the security context of files before it has any rules? (It has xattr annotations, but they have no _meaning_ without rules...? Confused, Rob -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] Documentation/sphinx: Fix Directive import error
On Fri, 2 Mar 2018 10:40:14 -0800 Matthew Wilcox wrote: > Sphinx 1.7 removed sphinx.util.compat.Directive so people > who have upgraded cannot build the documentation. Switch to > docutils.parsers.rst.Directive which has been available since > docutils 0.5 released in 2009. > > Bugzilla: https://bugzilla.opensuse.org/show_bug.cgi?id=1083694 > Co-developed-by: Takashi Iwai > Signed-off-by: Matthew Wilcox FWIW, I've *finally* gotten a chance to test this with a few sphinx versions; it works just fine, of course. Will get it upstream shortly. Thanks, jon -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] docs: add Co-Developed-by docs
On Mon, 5 Mar 2018 14:58:21 +1100 "Tobin C. Harding" wrote: > When Co-Developed-by tag was added, docs were only added to > Documention/process/5.Posting.rst and were not added to > Documention/process/submitting-patches.rst > > Add documentation to Documention/process/submitting-patches.rst Someday we should really pull those documents together into a coherent whole so we don't have these weird synchronization issues. But it's much easier to just apply this, so that's what I've done for now, with Willy's comma tweak. I'll leave the checkpatch part to Joe. Thanks, jon -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 15/15] selinux: delay sid population for rootfs till init is complete
On Wed, 7 Mar 2018, Rob Landley wrote: On 02/20/2018 12:56 PM, Stephen Smalley wrote: On Fri, 2018-02-16 at 20:33 +, Taras Kondratiuk wrote: From: Victor Kamensky With initramfs cpio format that supports extended attributes we need to skip sid population on sys_lsetxattr call from initramfs for rootfs if security server is not initialized yet. Otherwise callback in selinux_inode_post_setxattr will try to translate give security.selinux label into sid context and since security server is not available yet inode will receive default sid (typically kernel_t). Note that in the same time proper label will be stored in inode xattrs. Later, since inode sid would be already populated system will never look back at actual xattrs. But if we skip sid population for rootfs and we have policy that direct use of xattrs for rootfs, proper sid will be filled in from extended attributes one node is accessed and server is initialized. Note new DELAYAFTERINIT_MNT super block flag is introduced to only mark rootfs for such behavior. For other types of tmpfs original logic is still used. (cc selinux maintainers) Wondering if we shouldn't just do this always, for all filesystem types. Also, I think this should likely also be done in selinux_inode_setsecurity() for consistency. Sorry, I did not have time to try out Stephen's suggestion, especially given that core initramfs xattrs acceptance and dicussion looks a bit stalled, and for my use case it is dependency before SELinux changes. I will look for both suggestion this week. Hope to see initramfs xattrs patch series review going again. I don't understand what selinux thinks it's doing here. Initramfs is special because it's populated early, ideally early enough drivers can load their firmware out of it. This is guaranteed to be before any processes have launched, before any other filesystems have been mounted. I'm surprised selinux is trying to do anything this early because A) what is there for it to do, B) where did it get a ruleset? This isn't really a mount flag, this is a "the selinux subsystem isn't functionally initialized yet" flag. We haven't launched init. In a modular system the module probably isn't loaded. There are no processes, and the only files anywhere are the ones we're in the process of extracting. What's there fore selinux to do? When a filesystem is mounted, none of these cached selinux "we already looked at the xattrs" inode fields are populated yet, correct? It can figure that out when something accesses the file and do it then, so the point is _not_ doing this now and thus not cacheing the wrong info. That's what the mount flag is doing, telling selinux "not yet". So why does selinux not already _know_ "not yet"? Why doesn't load_policy flush the cache of the old default contexts? What happens if you mount an ext2 root and then init reads a dozen files before it gets to the load_policy? I need to check whether security context caching happens on all file operations, or when setxattr is executed. If latter, setxattr operation before policy load may not be very common use case. Also note there is a second SELinux related patch and corresponding Stephen's comment: if SELinux is enabled in kernel, but policy is not loaded yet, setxattr for security.selinux extended attribute will go for check to SELinux LSM callback, it will be denied. My other patch was relaxing above for "rootfs" only, i.e covering initramfs xattrs case. Stephen's point was that maybe it needs to be relaxed for all cases if policy not loaded yet. I need some time to look at the code and think about what can go wrong, if rule relaxed for all cases. Do those doesn't files have bad default contexts forever now? Where does the selinux ruleset come from during the cpio extract? Yes, in our use case SELinux policy file (/etc/selinux/*/policy/policy.*) comes from cpio initramfs itself. So it is chicken and egg problem. Was it hardwired into the driver? It certainly didn't come out of a file, and it wasn't a process that loaded it. Why is selinux trying to evaluate and cache the security context of files before it has any rules? Note for ext2 case there is no setxattr first as we have in initramfs xattrs case, so extended attributes values are read from backing persitent storage as they were put there before, and there would not be discrepency what is cached in security context inode data scructure and real "security.selinux" extended attribute value in file system. Thanks, Victor (It has xattr annotations, but they have no _meaning_ without rules...? Confused, Rob -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/17] Include linux trace docs to Sphinx TOC tree
On Tue, 27 Feb 2018 17:43:37 -0500 Steven Rostedt wrote: > On Tue, 27 Feb 2018 17:34:22 +0800 > "Du, Changbin" wrote: > > > Ten days past, will you accept this serias? Thank you! > > Currently I'm very overloaded with other code that needs to get done > ASAP, and I need to balance what is critical and what is not. I don't > have time to review this, as this isn't critical, and can wait. > > If Jon can review it to make sure that it doesn't change the > readability of the text, then I'll trust his judgment. So I've spent a while working through the patches. I think it's a well-done RST conversion carried out with a light hand; I do not believe there are any readability issues with the resulting text files. I will note that the series adds some new build warnings: > Documentation/trace/events.rst:45: WARNING: Inline emphasis start-string > without end-string. > Documentation/trace/events.rst:49: WARNING: Inline emphasis start-string > without end-string. > Documentation/trace/events.rst:193: WARNING: Inline emphasis start-string > without end-string. > Documentation/trace/events.rst:114: WARNING: Unknown target name: "common". > Documentation/trace/ftrace.rst:2620: WARNING: Inline emphasis start-string > without end-string. These point to places where the resulting formatted docs are, in fact, incorrect. I had to append the attached patch to the series to make those problems go away. The warnings are there for a purpose! Anyway, with that, the patch series is applied. Thanks for helping to improve the docs, and my apologies for taking so long to get to this. jon >From 6234c7bd8c14508fb76c0a4d6f01eb81c8ce9cbf Mon Sep 17 00:00:00 2001 From: Jonathan Corbet Date: Wed, 7 Mar 2018 10:44:08 -0700 Subject: [PATCH] docs: ftrace: fix a few formatting issues Make sure that literal * characters are not interpreted as emphasis markers. Signed-off-by: Jonathan Corbet --- Documentation/trace/events.rst | 10 +- Documentation/trace/ftrace.rst | 8 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Documentation/trace/events.rst b/Documentation/trace/events.rst index 27bfd06ae29d..bdf1963ba6ba 100644 --- a/Documentation/trace/events.rst +++ b/Documentation/trace/events.rst @@ -42,7 +42,7 @@ To disable all events, echo an empty line to the set_event file:: # echo > /sys/kernel/debug/tracing/set_event -To enable all events, echo '*:*' or '*:' to the set_event file:: +To enable all events, echo ``*:*`` or ``*:`` to the set_event file:: # echo *:* > /sys/kernel/debug/tracing/set_event @@ -50,7 +50,7 @@ The events are organized into subsystems, such as ext4, irq, sched, etc., and a full event name looks like this: :. The subsystem name is optional, but it is displayed in the available_events file. All of the events in a subsystem can be specified via the syntax -":*"; for example, to enable all irq events, you can use the +``:*``; for example, to enable all irq events, you can use the command:: # echo 'irq:*' > /sys/kernel/debug/tracing/set_event @@ -111,8 +111,8 @@ It also displays the format string that will be used to print the event in text mode, along with the event name and ID used for profiling. -Every event has a set of 'common' fields associated with it; these are -the fields prefixed with 'common_'. The other fields vary between +Every event has a set of ``common`` fields associated with it; these are +the fields prefixed with ``common_``. The other fields vary between events and correspond to the fields defined in the TRACE_EVENT definition for that event. @@ -190,7 +190,7 @@ And for string fields they are: ==, !=, ~ -The glob (~) accepts a wild card character (*,?) and character classes +The glob (~) accepts a wild card character (\*,?) and character classes ([). For example:: prev_comm ~ "*sh" diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst index 636aa9bf5674..0bc33ad4a3f9 100644 --- a/Documentation/trace/ftrace.rst +++ b/Documentation/trace/ftrace.rst @@ -2615,13 +2615,13 @@ To see which functions are being traced, you can cat the file: Perhaps this is not enough. The filters also allow glob(7) matching. - * + ``*`` will match functions that begin with - * + ``*`` will match functions that end with - ** + ``**`` will match functions that have in it - * + ``*`` will match functions that begin with and end with .. note:: -- 2.14.3 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] docs: add Co-Developed-by docs
On Wed, Mar 07, 2018 at 10:16:14AM -0700, Jonathan Corbet wrote: > On Mon, 5 Mar 2018 14:58:21 +1100 > "Tobin C. Harding" wrote: > > > When Co-Developed-by tag was added, docs were only added to > > Documention/process/5.Posting.rst and were not added to > > Documention/process/submitting-patches.rst > > > > Add documentation to Documention/process/submitting-patches.rst > > Someday we should really pull those documents together into a coherent > whole so we don't have these weird synchronization issues. But it's much > easier to just apply this, so that's what I've done for now, with Willy's > comma tweak. In case you still have my patch to Documentation/process/5.Posting.rst pending, we'll have a mismatch between Co-developed-by there (which has been used a number of times in the past), and Co-Developed-by in submitting-patches.rst and checkpatch and Co-developed-by (which was used exactly once in Linus' tree). That should be coherent at least... Thanks, Dominik -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core
Hi Julia,
Thanks for sharing your time on reviewing it. Please see my inline answers.
Jae
On 3/6/2018 7:19 PM, Julia Cartwright wrote:
On Wed, Feb 21, 2018 at 08:15:59AM -0800, Jae Hyun Yoo wrote:
This commit adds driver implementation for PECI bus into linux
driver framework.
Signed-off-by: Jae Hyun Yoo
---
[..]
+static int peci_locked_xfer(struct peci_adapter *adapter,
+ struct peci_xfer_msg *msg,
+ bool do_retry,
+ bool has_aw_fcs)
_locked generally means that this function is invoked with some critical
lock held, what lock does the caller need to acquire before invoking
this function?
I intended to show that this function has a mutex locking inside for
serialization of PECI data transactions from multiple callers, but as
you commented out below, the mutex protection scope should be adjusted
to make that covers the peci_scan_cmd_mask() function too. I'll rewrite
the mutex protection scope then this function will be in the locked scope.
+{
+ ktime_t start, end;
+ s64 elapsed_ms;
+ int rc = 0;
+
+ if (!adapter->xfer) {
Is this really an optional feature of an adapter? If this is not
optional, then this check should be in place when the adapter is
registered, not here. (And it should WARN_ON(), because it's a driver
developer error).
I agree with you. I'll move this code into the peci_register_adapter()
function.
+ dev_dbg(&adapter->dev, "PECI level transfers not supported\n");
+ return -ENODEV;
+ }
+
+ if (in_atomic() || irqs_disabled()) {
As Andrew mentioned, this is broken.
You don't even need a might_sleep(). The locking functions you use here
will already include a might_sleep() w/ CONFIG_DEBUG_ATOMIC_SLEEP.
Thanks for letting me know that. I'll drop that checking code and
might_sleep() too.
+ rt_mutex_trylock(&adapter->bus_lock);
+ if (!rc)
+ return -EAGAIN; /* PECI activity is ongoing */
+ } else {
+ rt_mutex_lock(&adapter->bus_lock);
+ }
+
+ if (do_retry)
+ start = ktime_get();
+
+ do {
+ rc = adapter->xfer(adapter, msg);
+
+ if (!do_retry)
+ break;
+
+ /* Per the PECI spec, need to retry commands that return 0x8x */
+ if (!(!rc && ((msg->rx_buf[0] & DEV_PECI_CC_RETRY_ERR_MASK) ==
+ DEV_PECI_CC_TIMEOUT)))
+ break;
This is pretty difficult to parse. Can you split it into two different
conditions?
Sure. I'll split it out.
+
+ /* Set the retry bit to indicate a retry attempt */
+ msg->tx_buf[1] |= DEV_PECI_RETRY_BIT;
Are you sure this bit is to be set in the _second_ byte of tx_buf?
Yes, I'm pretty sure. The first byte contains a PECI command value and
the second byte contains 'HostID[7:1] & Retry[0]' value.
+
+ /* Recalculate the AW FCS if it has one */
+ if (has_aw_fcs)
+ msg->tx_buf[msg->tx_len - 1] = 0x80 ^
+ peci_aw_fcs((u8 *)msg,
+ 2 + msg->tx_len);
+
+ /* Retry for at least 250ms before returning an error */
+ end = ktime_get();
+ elapsed_ms = ktime_to_ms(ktime_sub(end, start));
+ if (elapsed_ms >= DEV_PECI_RETRY_TIME_MS) {
+ dev_dbg(&adapter->dev, "Timeout retrying xfer!\n");
+ break;
+ }
+ } while (true);
+
+ rt_mutex_unlock(&adapter->bus_lock);
+
+ return rc;
+}
+
+static int peci_xfer(struct peci_adapter *adapter, struct peci_xfer_msg *msg)
+{
+ return peci_locked_xfer(adapter, msg, false, false);
+}
+
+static int peci_xfer_with_retries(struct peci_adapter *adapter,
+ struct peci_xfer_msg *msg,
+ bool has_aw_fcs)
+{
+ return peci_locked_xfer(adapter, msg, true, has_aw_fcs);
+}
+
+static int peci_scan_cmd_mask(struct peci_adapter *adapter)
+{
+ struct peci_xfer_msg msg;
+ u32 dib;
+ int rc = 0;
+
+ /* Update command mask just once */
+ if (adapter->cmd_mask & BIT(PECI_CMD_PING))
+ return 0;
+
+ msg.addr = PECI_BASE_ADDR;
+ msg.tx_len= GET_DIB_WR_LEN;
+ msg.rx_len= GET_DIB_RD_LEN;
+ msg.tx_buf[0] = GET_DIB_PECI_CMD;
+
+ rc = peci_xfer(adapter, &msg);
+ if (rc < 0) {
+ dev_dbg(&adapter->dev, "PECI xfer error, rc : %d\n", rc);
+ return rc;
+ }
+
+ dib = msg.rx_buf[0] | (msg.rx_buf[1] << 8) |
+ (msg.rx_buf[2] << 16) | (msg.rx_buf[3] << 24);
+
+ /* Check special case for Get DIB command */
+ if (dib == 0x00) {
+ dev_dbg(&
Re: [v3, 3/4] i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C controller
Hi, On Tue, Feb 27, 2018 at 5:38 PM, Karthikeyan Ramasubramanian wrote: > This bus driver supports the GENI based i2c hardware controller in the > Qualcomm SOCs. The Qualcomm Generic Interface (GENI) is a programmable > module supporting a wide range of serial interfaces including I2C. The > driver supports FIFO mode and DMA mode of transfer and switches modes > dynamically depending on the size of the transfer. > > Signed-off-by: Karthikeyan Ramasubramanian > Signed-off-by: Sagar Dharia > Signed-off-by: Girish Mahadevan > --- > drivers/i2c/busses/Kconfig | 11 + > drivers/i2c/busses/Makefile| 1 + > drivers/i2c/busses/i2c-qcom-geni.c | 626 > + > 3 files changed, 638 insertions(+) I'm not an expert on geni (and, to be honest, I haven't read the main geni patch yet). ...but I figured I could at least add my $0.02 since I've stared at i2c bus drivers a lot in the past. Feel free to tell me if I'm full or crap... > create mode 100644 drivers/i2c/busses/i2c-qcom-geni.c > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index e2954fb..1ddf5cd 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -848,6 +848,17 @@ config I2C_PXA_SLAVE > is necessary for systems where the PXA may be a target on the > I2C bus. > > +config I2C_QCOM_GENI > + tristate "Qualcomm Technologies Inc.'s GENI based I2C controller" > + depends on ARCH_QCOM > + depends on QCOM_GENI_SE > + help > + If you say yes to this option, support will be included for the > + built-in I2C interface on the Qualcomm Technologies Inc.'s SoCs. Kind of a generic description and this driver is only for new SoCs, right? Maybe make it a little more specific? > + > + This driver can also be built as a module. If so, the module > + will be called i2c-qcom-geni. > + > config I2C_QUP > tristate "Qualcomm QUP based I2C controller" > depends on ARCH_QCOM > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile > index 2ce8576..201fce1 100644 > --- a/drivers/i2c/busses/Makefile > +++ b/drivers/i2c/busses/Makefile > @@ -84,6 +84,7 @@ obj-$(CONFIG_I2C_PNX) += i2c-pnx.o > obj-$(CONFIG_I2C_PUV3) += i2c-puv3.o > obj-$(CONFIG_I2C_PXA) += i2c-pxa.o > obj-$(CONFIG_I2C_PXA_PCI) += i2c-pxa-pci.o > +obj-$(CONFIG_I2C_QCOM_GENI)+= i2c-qcom-geni.o > obj-$(CONFIG_I2C_QUP) += i2c-qup.o > obj-$(CONFIG_I2C_RIIC) += i2c-riic.o > obj-$(CONFIG_I2C_RK3X) += i2c-rk3x.o > diff --git a/drivers/i2c/busses/i2c-qcom-geni.c > b/drivers/i2c/busses/i2c-qcom-geni.c > new file mode 100644 > index 000..e1e4268 > --- /dev/null > +++ b/drivers/i2c/busses/i2c-qcom-geni.c > @@ -0,0 +1,626 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// Copyright (c) 2017-2018, The Linux Foundation. All rights reserved. > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define SE_I2C_TX_TRANS_LEN0x26c > +#define SE_I2C_RX_TRANS_LEN0x270 > +#define SE_I2C_SCL_COUNTERS0x278 > + > +#define SE_I2C_ERR (M_CMD_OVERRUN_EN | M_ILLEGAL_CMD_EN | M_CMD_FAILURE_EN > |\ > + M_GP_IRQ_1_EN | M_GP_IRQ_3_EN | M_GP_IRQ_4_EN) > +#define SE_I2C_ABORT BIT(1) > + > +/* M_CMD OP codes for I2C */ > +#define I2C_WRITE 0x1 > +#define I2C_READ 0x2 > +#define I2C_WRITE_READ 0x3 > +#define I2C_ADDR_ONLY 0x4 > +#define I2C_BUS_CLEAR 0x6 > +#define I2C_STOP_ON_BUS0x7 > +/* M_CMD params for I2C */ > +#define PRE_CMD_DELAY BIT(0) > +#define TIMESTAMP_BEFORE BIT(1) > +#define STOP_STRETCH BIT(2) > +#define TIMESTAMP_AFTERBIT(3) > +#define POST_COMMAND_DELAY BIT(4) > +#define IGNORE_ADD_NACKBIT(6) > +#define READ_FINISHED_WITH_ACK BIT(7) > +#define BYPASS_ADDR_PHASE BIT(8) > +#define SLV_ADDR_MSK GENMASK(15, 9) > +#define SLV_ADDR_SHFT 9 > +/* I2C SCL COUNTER fields */ > +#define HIGH_COUNTER_MSK GENMASK(29, 20) > +#define HIGH_COUNTER_SHFT 20 > +#define LOW_COUNTER_MSKGENMASK(19, 10) > +#define LOW_COUNTER_SHFT 10 > +#define CYCLE_COUNTER_MSK GENMASK(9, 0) > + > +#define GP_IRQ00 > +#define GP_IRQ11 > +#define GP_IRQ22 > +#define GP_IRQ33 > +#define GP_IRQ44 > +#define GP_IRQ55 > +#define GENI_OVERRUN 6 > +#define GENI_ILLEGAL_CMD 7 > +#define GENI_ABORT_DONE8 > +#define GENI_TIMEOUT 9 Above should be an enum; then use the enum type as the parameter to geni_i2c_err() so it's obvious that "err" is not a
Re: [PATCH] docs: clarify security-bugs disclosure policy
On Tue, Mar 6, 2018 at 3:31 PM, Dave Hansen wrote: > > I think we need to soften the language a bit. It might scare folks > off, especially the: > > We prefer to fully disclose the bug as soon as possible. > > which is not really the case. Ack. What we do is definitely not full disclosure. In fact, we often actively try to avoid disclosing details and leave that entirely to others. We disclose the *patches*, and the explanation of the patch, but not necessarily anything else (ie no exploit code or even any exploit discussion). We also don't explicitly disclose the discussion of the patches or the report, although part of it mayt obviously become more or less public for other reasons. So we should probably avoid using a term that means something else to a lot of people. And for similar reasons, I don't think the fixed verbiage should use "coordinated disclosure" either, like in your patch. That usually means the kind of embargoes that the security list does not honor. So I think it merits clarification, but maybe just specify the two things relevant to our disclosure: the fact that the patch and explanation for the patch gets made public (but not necessarily other effects), and that the timeframe is very limited. It's not full disclosure, it's not coordinated disclosure, and it's not "no disclosure". It's more like just "timely open fixes". Linus -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/13] Remove metag architecture
On Thu, Feb 22, 2018 at 12:38 AM, James Hogan wrote: > These patches remove the metag architecture and tightly dependent > drivers from the kernel. With the 4.16 kernel the ancient gcc 4.2.4 > based metag toolchain we have been using is hitting compiler bugs, so > now seems a good time to drop it altogether. > > Quoting from patch 1: > > The earliest Meta architecture port of Linux I have a record of was an > import of a Meta port of Linux v2.4.1 in February 2004, which was worked > on significantly over the next few years by Graham Whaley, Will Newton, > Matt Fleming, myself and others. > > Eventually the port was merged into mainline in v3.9 in March 2013, not > long after Imagination Technologies bought MIPS Technologies and shifted > its CPU focus over to the MIPS architecture. > > As a result, though the port was maintained for a while, kept on life > support for a while longer, and useful for testing a few specific > drivers for which I don't have ready access to the equivalent MIPS > hardware, it is now essentially dead with no users. > > It is also stuck using an out-of-tree toolchain based on GCC 4.2.4 which > is no longer maintained, now struggles to build modern kernels due to > toolchain bugs, and doesn't itself build with a modern GCC. The latest > buildroot port is still using an old uClibc snapshot which is no longer > served, and the latest uClibc doesn't build with GCC 4.2.4. > > So lets call it a day and drop the Meta architecture port from the > kernel. RIP Meta. I've pulled it into my asm-generic tree now, which is also part of linux-next, and followed up with patches removing frv, m32r, score, unicore32 and blackfin. I have not removed the device drivers yet, but I'm working on that. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [v2] docs: clarify security-bugs disclosure policy
From: Dave Hansen I think we need to soften the language a bit. It might scare folks off, especially the: We prefer to fully disclose the bug as soon as possible. which is not really the case. Linus says: It's not full disclosure, it's not coordinated disclosure, and it's not "no disclosure". It's more like just "timely open fixes". I changed a bit of the wording in here, but mostly to remove the word "disclosure" since it seems to mean very specific things to people that we do not mean here. Signed-off-by: Dave Hansen Reviewed-by: Dan Williams Cc: Thomas Gleixner Cc: Greg Kroah-Hartman Cc: Linus Torvalds Cc: Alan Cox Cc: Andrea Arcangeli Cc: Andy Lutomirski Cc: Kees Cook Cc: Tim Chen Cc: Alexander Viro Cc: Andrew Morton Cc: linux-doc@vger.kernel.org Cc: Jonathan Corbet Cc: Mark Rutland --- b/Documentation/admin-guide/security-bugs.rst | 24 +--- 1 file changed, 13 insertions(+), 11 deletions(-) diff -puN Documentation/admin-guide/security-bugs.rst~embargo2 Documentation/admin-guide/security-bugs.rst --- a/Documentation/admin-guide/security-bugs.rst~embargo2 2018-03-07 13:23:49.390228208 -0800 +++ b/Documentation/admin-guide/security-bugs.rst 2018-03-07 13:42:37.618225395 -0800 @@ -29,18 +29,20 @@ made public. Disclosure -- -The goal of the Linux kernel security team is to work with the -bug submitter to bug resolution as well as disclosure. We prefer -to fully disclose the bug as soon as possible. It is reasonable to -delay disclosure when the bug or the fix is not yet fully understood, -the solution is not well-tested or for vendor coordination. However, we -expect these delays to be short, measurable in days, not weeks or months. -A disclosure date is negotiated by the security team working with the -bug submitter as well as vendors. However, the kernel security team -holds the final say when setting a disclosure date. The timeframe for -disclosure is from immediate (esp. if it's already publicly known) +The goal of the Linux kernel security team is to work with the bug +submitter to understand and fix the bug. We prefer to publish the fix as +soon as possible, but try to avoid public discussion of the bug itself +and leave that to others. + +Publishing the fix may be delayed when the bug or the fix is not yet +fully understood, the solution is not well-tested or for vendor +coordination. However, we expect these delays to be short, measurable in +days, not weeks or months. A release date is negotiated by the security +team working with the bug submitter as well as vendors. However, the +kernel security team holds the final say when setting a timeframe. The +timeframe varies from immediate (esp. if it's already publicly known bug) to a few weeks. As a basic default policy, we expect report date to -disclosure date to be on the order of 7 days. +release date to be on the order of 7 days. Coordination _ -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [v2] docs: clarify security-bugs disclosure policy
That patch looks fine to me, avoiding any terms that might be misunderstood, and being pretty straightforward. I'm guessing this will go through Jon? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/8] [PATCH 2/8] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs
Hi! > >Are these SoCs x86-based? > > Yes, these are ARM SoCs. Please see Andrew's answer as well. Understood, thanks. > >>+ Read sampling point selection. The whole period of a bit time will be > >>+ divided into 16 time frames. This value will determine which time frame > >>+ this controller will sample PECI signal for data read back. Usually in > >>+ the middle of a bit time is the best. > > > >English? "This value will determine when this controller"? > > > > Could I change it like below?: > > "This value will determine in which time frame this controller samples PECI > signal for data read back" I guess... I'm not native speaker, I guess this could be improved some more. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH] [v2] docs: clarify security-bugs disclosure policy
On Wed, Mar 7, 2018 at 1:46 PM, Dave Hansen wrote: > > From: Dave Hansen > > I think we need to soften the language a bit. It might scare folks > off, especially the: > > We prefer to fully disclose the bug as soon as possible. > > which is not really the case. Linus says: > > It's not full disclosure, it's not coordinated disclosure, > and it's not "no disclosure". It's more like just "timely > open fixes". > > I changed a bit of the wording in here, but mostly to remove the word > "disclosure" since it seems to mean very specific things to people > that we do not mean here. > > Signed-off-by: Dave Hansen > Reviewed-by: Dan Williams > Cc: Thomas Gleixner > Cc: Greg Kroah-Hartman > Cc: Linus Torvalds > Cc: Alan Cox > Cc: Andrea Arcangeli > Cc: Andy Lutomirski > Cc: Kees Cook > Cc: Tim Chen > Cc: Alexander Viro > Cc: Andrew Morton > Cc: linux-doc@vger.kernel.org > Cc: Jonathan Corbet > Cc: Mark Rutland > --- > > b/Documentation/admin-guide/security-bugs.rst | 24 +--- > 1 file changed, 13 insertions(+), 11 deletions(-) > > diff -puN Documentation/admin-guide/security-bugs.rst~embargo2 > Documentation/admin-guide/security-bugs.rst > --- a/Documentation/admin-guide/security-bugs.rst~embargo2 2018-03-07 > 13:23:49.390228208 -0800 > +++ b/Documentation/admin-guide/security-bugs.rst 2018-03-07 > 13:42:37.618225395 -0800 > @@ -29,18 +29,20 @@ made public. > Disclosure > -- > > -The goal of the Linux kernel security team is to work with the > -bug submitter to bug resolution as well as disclosure. We prefer > -to fully disclose the bug as soon as possible. It is reasonable to > -delay disclosure when the bug or the fix is not yet fully understood, > -the solution is not well-tested or for vendor coordination. However, we > -expect these delays to be short, measurable in days, not weeks or months. > -A disclosure date is negotiated by the security team working with the > -bug submitter as well as vendors. However, the kernel security team > -holds the final say when setting a disclosure date. The timeframe for > -disclosure is from immediate (esp. if it's already publicly known) > +The goal of the Linux kernel security team is to work with the bug > +submitter to understand and fix the bug. We prefer to publish the fix as > +soon as possible, but try to avoid public discussion of the bug itself > +and leave that to others. > + > +Publishing the fix may be delayed when the bug or the fix is not yet > +fully understood, the solution is not well-tested or for vendor > +coordination. However, we expect these delays to be short, measurable in > +days, not weeks or months. A release date is negotiated by the security > +team working with the bug submitter as well as vendors. However, the > +kernel security team holds the final say when setting a timeframe. The > +timeframe varies from immediate (esp. if it's already publicly known bug) Nit: I think "a" is missing. I was expecting: "... already a publicly known ... > to a few weeks. As a basic default policy, we expect report date to > -disclosure date to be on the order of 7 days. > +release date to be on the order of 7 days. Otherwise, yeah, looks good. Acked-by: Kees Cook -Kees -- Kees Cook Pixel Security -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] earlycon: Allow specifying a uartclk in options
On Thu, Mar 1, 2018 at 11:43 AM Daniel Kurtz wrote: > Currently when an earlycon is registered, the uartclk is assumed to be > BASE_BAUD * 16 = 1843200. If a baud rate is specified in the earlycon > options, then 8250_early's init_port will program the UART clock divider > registers based on this assumed uartclk. > However, not all uarts have a UART clock of 1843200. For example, the > 8250_dw uart in AMD's CZ/ST uses a fixed 48 MHz clock (as specified in > cz_uart_desc in acpi_apd.c). Thus, specifying a baud when using earlycon > on such a device will result in incorrect divider values and a wrong UART > clock. > Fix this by extending the earlycon options parameter to allow specification > of a uartclk, like so: > earlycon=uart,mmio32,0xfedc6000,115200,4800 > If none is specified, fall-back to prior behavior - 1843200. > Signed-off-by: Daniel Kurtz This general approach is facing resistance, so trying another more targeted approach to work around the "BASE_BAUD=115200" assumption in arch/x86: https://patchwork.kernel.org/patch/10265587/ -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v3, 3/4] i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C controller
Hi Doug,
Thank you for reviewing the patch. I will take a stab at a few comments
below. We will address most of the other comments in next version of I2C
patch.
+
+#define I2C_AUTO_SUSPEND_DELAY 250
Why 250 ms? That seems like an eternity. Is it really that expensive
to turn resources off and on? I would sorta just expect clocks and
stuff to get turned off right after a transaction finished unless
another one was pending right behind it...
The response from RPMh to turn on/off shared resources also take quite a
few msecs. The QUP serial bus block sits quite a few shared-NOCs away
from the memory and runtime-PM is used a bandwidth vote/NOC vote for
these NOCs from QUP to memory. Also the RPC between apps and RPMh can
sometimes take longer depending on other tasks running on apps. This 250
msec avoids thrashing of these RPCs between apps and RPMh.
If you plan to keep these NOCs on forever, then your are right:
runtime-PM will be only used to turn on/off local clocks and we won't
even need autosuspend. that's not true on products where this driver is
currently deployed.
+
+static const struct geni_i2c_clk_fld geni_i2c_clk_map[] = {
+ {KHz(100), 7, 10, 11, 26},
+ {KHz(400), 2, 5, 12, 24},
+ {KHz(1000), 1, 3, 9, 18},
So I guess this is all relying on an input serial clock of 19.2MHz?
Maybe document that?
Assuming I'm understanding the math here, is it really OK for your
100kHz and 1MHz mode to be running slightly fast?
19200. / 2 / 24
400.0
19200. / 7 / 26
105.49450549450549
19200. / 1 / 18
1066.7
It seems like you'd want the fastest clock that you can make that's
_less than_ the spec.
It would also be interesting to know if it's expected that boards
might need to tweak the t_high / t_low depending on their electrical
characteristics. In the past I've had lots of requests from board
makers to tweak things because they've got a long trace, or a stronger
or weaker pull, or ... If so we might later need to add some dts
properties like "i2c-scl-rising-time-ns" and make the math more
dynamic here, unless your hardware somehow automatically adjusts for
this type of thing...
These values are derived by our HW team to comply with the t-high and
t-low specs of I2C. We have confirmed on scope that the frequency of SCL
is indeed less than/equal to the spec. We have not come across slaves
who have needed to tweak these things. We are open to adding these
properties in dts if you have any such slaves not conforming due to
board-layout of other reasons.
+ mode = msg->len > 32 ? GENI_SE_DMA : GENI_SE_FIFO;
DMA is hard and i2c transfers > 32 bytes are rare. Do we really gain
a lot by transferring i2c commands over DMA compared to a FIFO?
Enough to justify the code complexity and the set of bugs that will
show up? I'm sure it will be a controversial assertion given that the
code's already written, but personally I'd be supportive of ripping
DMA mode out to simplify the driver. I'd be curious if anyone else
agrees. To me it seems like premature optimization.
Yes, we have multiple clients (e.g. touch, NFC) using I2C for data
transfers bigger than 32 bytes (some transfers are 100s of bytes). The
fifo size is 32, so we can definitely avoid at least 1 interrupt when
DMA mode is used with data size > 32.
+ geni_se_select_mode(&gi2c->se, mode);
+ writel_relaxed(msg->len, gi2c->se.base + SE_I2C_RX_TRANS_LEN);
+ geni_se_setup_m_cmd(&gi2c->se, I2C_READ, m_param);
+ if (mode == GENI_SE_DMA) {
+ rx_dma = geni_se_rx_dma_prep(&gi2c->se, msg->buf, msg->len);
Randomly I noticed a flag called "I2C_M_DMA_SAFE". Do we need to
check this flag before using msg->buf for DMA? ...or use
i2c_get_dma_safe_msg_buf()?
...btw: the relative lack of people doing this in the kernel is
further evidence of DMA not really being worth it for i2c busses.
I cannot comment about other drivers here using or not using DMA since
they may not be exercised with slaves like NFC?
+ ret = pm_runtime_get_sync(gi2c->se.dev);
+ if (ret < 0) {
+ dev_err(gi2c->se.dev, "error turning SE resources:%d\n", ret);
+ pm_runtime_put_noidle(gi2c->se.dev);
+ /* Set device in suspended since resume failed */
+ pm_runtime_set_suspended(gi2c->se.dev);
+ return ret;
Wow, that's a cluster of arcane calls to handle a call that probably
will never fail (it just enables clocks and sets pinctrl). Sigh.
...but as far as I can tell the whole sequence is right. You
definitely need a "put" after a failed get and it looks like
pm_runtime_set_suspended() has a special exception where it can be
called if you got a runtime error...
We didn't have this in before either. But this condition is somewhat
frequent if I2C transactions are called on cusp of exiting system
suspend. (e.g. PMIC slave getting a wakeup-IRQ and trying to read from
PMIC through I2C to read its status as to wh
Re: [PATCH 00/17] Include linux trace docs to Sphinx TOC tree
On Wed, Mar 07, 2018 at 10:46:49AM -0700, Jonathan Corbet wrote: > On Tue, 27 Feb 2018 17:43:37 -0500 > Steven Rostedt wrote: > > > On Tue, 27 Feb 2018 17:34:22 +0800 > > "Du, Changbin" wrote: > > > > > Ten days past, will you accept this serias? Thank you! > > > > Currently I'm very overloaded with other code that needs to get done > > ASAP, and I need to balance what is critical and what is not. I don't > > have time to review this, as this isn't critical, and can wait. > > > > If Jon can review it to make sure that it doesn't change the > > readability of the text, then I'll trust his judgment. > > So I've spent a while working through the patches. I think it's a > well-done RST conversion carried out with a light hand; I do not believe > there are any readability issues with the resulting text files. > > I will note that the series adds some new build warnings: > > > Documentation/trace/events.rst:45: WARNING: Inline emphasis start-string > > without end-string. > > Documentation/trace/events.rst:49: WARNING: Inline emphasis start-string > > without end-string. > > Documentation/trace/events.rst:193: WARNING: Inline emphasis start-string > > without end-string. > > Documentation/trace/events.rst:114: WARNING: Unknown target name: "common". > > Documentation/trace/ftrace.rst:2620: WARNING: Inline emphasis start-string > > without end-string. > > These point to places where the resulting formatted docs are, in fact, > incorrect. I had to append the attached patch to the series to make those > problems go away. The warnings are there for a purpose! > > Anyway, with that, the patch series is applied. Thanks for helping to > improve the docs, and my apologies for taking so long to get to this. > > jon > I am also appriciated for your review. And I am glade to see these docs can appear in the new beautiful html documentation! Thnak you. - changbin -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/17] Include linux trace docs to Sphinx TOC tree
On Wed, 7 Mar 2018 10:46:49 -0700 Jonathan Corbet wrote: > Anyway, with that, the patch series is applied. Thanks for helping to > improve the docs, and my apologies for taking so long to get to this. Thanks for doing this Jon. I'm going to have to find some time to revisit all the documentation in the tracing directory, because I'm sure they have fallen out of date. -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v3, 3/4] i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C controller
Hi,
On Wed, Mar 7, 2018 at 6:42 PM, Sagar Dharia wrote:
> Hi Doug,
> Thank you for reviewing the patch. I will take a stab at a few comments
> below. We will address most of the other comments in next version of I2C
> patch.
>>
>>
>>
>>> +
>>> +#define I2C_AUTO_SUSPEND_DELAY 250
>>
>>
>> Why 250 ms? That seems like an eternity. Is it really that expensive
>> to turn resources off and on? I would sorta just expect clocks and
>> stuff to get turned off right after a transaction finished unless
>> another one was pending right behind it...
>>
> The response from RPMh to turn on/off shared resources also take quite a few
> msecs. The QUP serial bus block sits quite a few shared-NOCs away from the
> memory and runtime-PM is used a bandwidth vote/NOC vote for these NOCs from
> QUP to memory. Also the RPC between apps and RPMh can sometimes take longer
> depending on other tasks running on apps. This 250 msec avoids thrashing of
> these RPCs between apps and RPMh.
> If you plan to keep these NOCs on forever, then your are right: runtime-PM
> will be only used to turn on/off local clocks and we won't even need
> autosuspend. that's not true on products where this driver is currently
> deployed.
OK, fair enough. I don't know how RPMh works well enough to argue.
It does seem odd that you'd want to design things such that it takes a
few msecs to pull it out of runtime suspend, especially for touch.
>>> +
>>> +static const struct geni_i2c_clk_fld geni_i2c_clk_map[] = {
>>> + {KHz(100), 7, 10, 11, 26},
>>> + {KHz(400), 2, 5, 12, 24},
>>> + {KHz(1000), 1, 3, 9, 18},
>>
>>
>> So I guess this is all relying on an input serial clock of 19.2MHz?
>> Maybe document that?
>>
>> Assuming I'm understanding the math here, is it really OK for your
>> 100kHz and 1MHz mode to be running slightly fast?
>>
>> 19200. / 2 / 24
>
> 400.0
>>
>>
>> 19200. / 7 / 26
>
> 105.49450549450549
>>
>>
>> 19200. / 1 / 18
>
> 1066.7
>>
>>
>> It seems like you'd want the fastest clock that you can make that's
>> _less than_ the spec.
>>
>>
>> It would also be interesting to know if it's expected that boards
>> might need to tweak the t_high / t_low depending on their electrical
>> characteristics. In the past I've had lots of requests from board
>> makers to tweak things because they've got a long trace, or a stronger
>> or weaker pull, or ... If so we might later need to add some dts
>> properties like "i2c-scl-rising-time-ns" and make the math more
>> dynamic here, unless your hardware somehow automatically adjusts for
>> this type of thing...
>> These values are derived by our HW team to comply with the t-high and
>
> t-low specs of I2C. We have confirmed on scope that the frequency of SCL is
> indeed less than/equal to the spec. We have not come across slaves who have
> needed to tweak these things. We are open to adding these properties in dts
> if you have any such slaves not conforming due to board-layout of other
> reasons.
OK, I'm fine with leaving something like this till later if/when it
comes up. Documenting a little bit more about how these timings work
seems like it would be nice, though, at least mentioning what the
source clock is...
>>>
>>> + mode = msg->len > 32 ? GENI_SE_DMA : GENI_SE_FIFO;
>>
>>
>> DMA is hard and i2c transfers > 32 bytes are rare. Do we really gain
>> a lot by transferring i2c commands over DMA compared to a FIFO?
>> Enough to justify the code complexity and the set of bugs that will
>> show up? I'm sure it will be a controversial assertion given that the
>> code's already written, but personally I'd be supportive of ripping
>> DMA mode out to simplify the driver. I'd be curious if anyone else
>> agrees. To me it seems like premature optimization.
>
>
> Yes, we have multiple clients (e.g. touch, NFC) using I2C for data transfers
> bigger than 32 bytes (some transfers are 100s of bytes). The fifo size is
> 32, so we can definitely avoid at least 1 interrupt when DMA mode is used
> with data size > 32.
Does that 1-2 interrupts make any real difference, though? In theory
it really shouldn't affect the transfer rate. We should be able to
service the interrupt plenty fast and if we were concerned we would
tweak the watermark code a little bit. So I guess we're worried about
the extra CPU cycles (and power cost) to service those extra couple
interrupts?
In theory when touch data is coming in or NFC data is coming in then
we're probably not in a super low power state to begin with. If it's
touch data we likely want to have the CPU boosted a bunch to respond
to the user quickly. If we've got 8 cores available all of which can
run at 1GHz or more a few interrupts won't kill us. NFC data is
probably not common enough that we need to optimize power/CPU
utilizatoin for that.
So while i can believe that you do save an interrupt or two, I still
am not convinced that those interrupts are worth a bunch of complex
code (and a whole second co
Re: [PATCH v3 4/4] tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP
On 3/6/2018 2:45 PM, Stephen Boyd wrote:
Quoting Karthik Ramasubramanian (2018-03-05 16:51:23)
On 3/2/2018 3:11 PM, Stephen Boyd wrote:
Quoting Karthikeyan Ramasubramanian (2018-02-27 17:38:09)
+ size_t chars_to_write = 0;
+ size_t avail = DEF_FIFO_DEPTH_WORDS - DEF_TX_WM;
+
+ /*
+* If the WM bit never set, then the Tx state machine is not
+* in a valid state, so break, cancel/abort any existing
+* command. Unfortunately the current data being written is
+* lost.
+*/
+ while (!qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
+ M_TX_FIFO_WATERMARK_EN, true))
Does this ever timeout? So many nested while loops makes it hard to
follow.
Yes. Based on the baud rate of 115200 and the FIFO Depth & Width of (16
* 32), the poll should not take more than 5 ms under the timeout scenario.
Sure, but I'm asking if this has any sort of timeout associated with it.
It looks to be a while loop that could possibly go forever?
I will change it from a while loop to if condition to make it clear.
+static void qcom_geni_serial_console_write(struct console *co, const char *s,
+ unsigned int count)
+{
+ struct uart_port *uport;
+ struct qcom_geni_serial_port *port;
+ bool locked = true;
+ unsigned long flags;
+
+ WARN_ON(co->index < 0 || co->index >= GENI_UART_CONS_PORTS);
+
+ port = get_port_from_line(co->index);
+ if (IS_ERR(port))
+ return;
+
+ uport = &port->uport;
+ if (oops_in_progress)
+ locked = spin_trylock_irqsave(&uport->lock, flags);
+ else
+ spin_lock_irqsave(&uport->lock, flags);
+
+ if (locked) {
+ __qcom_geni_serial_console_write(uport, s, count);
So if oops is in progress, and we didn't lock here, we don't output
data? I'd think we would always want to write to the fifo, just make the
lock grab/release conditional.
If we fail to grab the lock, then there is another active writer. So
trying to write to the fifo will put the hardware in bad state because
writer has programmed the hardware to write 'x' number of words and this
thread will over-write it with 'y' number of words. Also the data that
you might see in the console might be garbled.
I suspect that because this is the serial console, and we want it to
always output stuff even when we're going down in flames, we may want to
ignore that case and just wait for the fifo to free up and then
overwrite the number of words that we want to output and push out more
characters.
I always get confused though because there seem to be lots of different
ways to do things in serial drivers and not too much clear documentation
that I've read describing what to do.
Ok. If the active writer is interrupted due to OOPS handler, then the
interrupted write can be cancelled and the write from OOPS handler can
be performed.
+ spin_unlock_irqrestore(&uport->lock, flags);
+ }
+}
[...]
+
+ if (m_irq_status & (M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN) &&
+ m_irq_en & (M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN))
+ qcom_geni_serial_handle_tx(uport);
+
+ if (s_irq_status & S_GP_IRQ_0_EN || s_irq_status & S_GP_IRQ_1_EN) {
+ if (s_irq_status & S_GP_IRQ_0_EN)
+ uport->icount.parity++;
+ drop_rx = true;
+ } else if (s_irq_status & S_GP_IRQ_2_EN ||
+ s_irq_status & S_GP_IRQ_3_EN) {
+ uport->icount.brk++;
How does break character handling work? I see the accounting here, but
don't see any uart_handle_break() call anywhere.
The reason it is not added is because the hardware does not indicate
when the break character occured. It can be any one of the FIFO words.
The statistics is updated to give an idea that the break happened. We
can add uart_handle_break() but it may not be at an accurate position
for the above mentioned reason.
Sounds like the previous uart design. We would want uart_handle_break()
support for kgdb to work over serial. Do we still get an interrupt
signal that a break character is somewhere in the fifo? If we get that,
then does it also put a NUL (0) character into the fifo that we can
find? I had to do something like that before, where we detect the irq
and then we go through the fifo a character at a time to find the break
character that looks like a NUL, and then let sysrq core handle the
characters after that break character comes in.
I will use the same logic as in blsp2 serial to catch the break
character, same as NULL and push the break character to the framework.
+}
+
+static unsigned long get_clk_div_rate(unsigned int baud, unsigned int *clk_div)
+{
+ unsigned long ser_clk;
+ unsigned long desired_clk;
+
+ desired_clk = bau
Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
On Wednesday 07 March 2018 07:50 PM, Guenter Roeck wrote: On 03/07/2018 01:47 AM, Rajkumar Rampelli wrote: While I am not opposed to ABI changes, the merits of those would need to be discussed on the mailing list. But replacing "fan1_input" with "rpm" is not an acceptable ABI change, even if it may measure something that turns but isn't a fan. If this _is_ in fact supposed to be used for something else but fans, we would have to discuss what that might be, and if hwmon is the appropriate subsystem to measure and report it. This does to some degree lead back to my concern of having the "fan" part of this patch series in the pwm core. I am still not sure if that makes sense. Thanks, Guenter I am planning to add tachometer support in pwm-fan.c driver (drivers/hwmon/) instead of adding new generic-pwm-tachometer.c driver. Measuring RPM value will be done in pwm-fan driver itself using pwm capture feature and will add new sysfs attributes under this driver to report rpm value of fan. There is an existing attribute to report the RPM of fans. It is called fan[1..n]_input. "replacing "fan1_input" with "rpm" is not an acceptable ABI change" Preemptive NACK. The RPM is measured speed via PWM signal capture which is output from fan. So should we have the fan[1..n]_output_rpm? -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 5/7] arm64: kvm: Introduce KVM_ARM_SET_SERROR_ESR ioctl
Hi James,
sorry for my late response due to chines new year.
2018-02-16 1:55 GMT+08:00 James Morse :
> Hi gengdongjiu,
>
> On 12/02/18 10:19, gengdongjiu wrote:
>> On 2018/2/10 1:44, James Morse wrote:
>>> The point? We can't know what a CPU without the RAS extensions puts in
>>> there.
>>>
>>> Why Does this matter? When migrating a pending SError we have to know the
>>> difference between 'use this 64bit value', and 'the CPU will generate it'.
>>> If I make an SError pending with ESR=0 on a CPU with VSESR, I can't
>>> migrated to
>>> a system that generates an impdef SError-ESR, because I can't know it will
>>> be 0.
>
>> For the target system, before taking the SError, no one can know whether its
>> syndrome value
>> is IMPLEMENTATION DEFINED or architecturally defined.
>
> For a virtual-SError, the hypervisor knows what it generated. (do I have
> VSESR_EL2? What did I put in there?).
>
>
>> when the virtual SError is taken, the ESR_ELx.IDS will be updated, then we
>> can know
>> whether the ESR value is impdef or architecturally defined.
>
> True, the guest can't know anything about a pending virtual SError until it
> takes it. Why is this a problem?
>
>
>> It seems migration is only allowed only when target system and source system
>> all support
>> RAS extension, because we do not know whether its syndrome is IMPLEMENTATION
>> DEFINED or
>> architecturally defined.
>
> I don't think Qemu allows migration between hosts with differing guest-ID
> registers. But we shouldn't depend on this, and we may want to hide the v8.2
> RAS
> features from the guest's ID register, but still use them from the host.
>
> The way I imagined it working was we would pack the following information into
> that events struct:
> {
> bool serror_pending;
> bool serror_has_esr;
> u64 serror_esr;
> }
I have used your suggestion struct
>
> The problem I was trying to describe is because there is no value of
> serror_esr
> we can use to mean 'Ignore this, I'm a v8.0 CPU'. VSESR_EL2 is a 64bit
> register,
> any bits we abuse may get a meaning we want to use in the future.
>
> When it comes to migration, v8.{0,1} systems can only GET/SET events where
> serror_has_esr == false, they can't use the serror_esr. On v8.2 systems we
> should require serror_has_esr to be true.
yes, I agreed.
>
> If we need to support migration from v8.{0,1} to v8.2, we can make up an
> impdef
> serror_esr.
For the Qemu migration, I need to check more the QEMU code.
Hi Andrew,
I use KVM_GET/SET_VCPU_EVENTS IOCTL to migrate the Serror
exception status of VM,
The even struct is shown below:
{
bool serror_pending;
bool serror_has_esr;
u64 serror_esr;
}
Only when the target machine is armv8.2, it needs to set the
serror_esr(SError Exception Syndrome Register).
for the armv8.0, software can not set the serror_esr(SError Exception
Syndrome Register).
so when migration from v8.{0,1} to v8.2, QEMU should make up an impdef
serror_esr for the v8.2 target.
can you give me some suggestion how to set that register in the QEMU?
I do not familar with the QEMU migration.
Thanks very much.
>
> We will need to decide what KVM does when SET is called but an SError was
> already pending. 2.5.3 "Multiple SError interrupts" of [0] has something to
> say.
how about KVM set again to the same VCPU?
>
>
> Happy new year,
thanks!
>
> James
>
>
> [0]
> https://static.docs.arm.com/ddi0587/a/RAS%20Extension-release%20candidate_march_29.pdf
> ___
> kvmarm mailing list
> kvm...@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/4] soc: qcom: Add GENI based QUP Wrapper driver
On 3/6/2018 2:56 PM, Stephen Boyd wrote:
Quoting Karthik Ramasubramanian (2018-03-02 16:58:23)
+ return iova;
+}
+EXPORT_SYMBOL(geni_se_tx_dma_prep);
+
+/**
+ * geni_se_rx_dma_prep() - Prepare the Serial Engine for RX DMA transfer
+ * @se:Pointer to the concerned Serial Engine.
+ * @buf: Pointer to the RX buffer.
+ * @len: Length of the RX buffer.
+ *
+ * This function is used to prepare the buffers for DMA RX.
+ *
+ * Return: Mapped DMA Address of the buffer on success, NULL on failure.
+ */
+dma_addr_t geni_se_rx_dma_prep(struct geni_se *se, void *buf, size_t len)
+{
+ dma_addr_t iova;
+ struct geni_wrapper *wrapper = se->wrapper;
+ u32 val;
+
+ iova = dma_map_single(wrapper->dev, buf, len, DMA_FROM_DEVICE);
+ if (dma_mapping_error(wrapper->dev, iova))
+ return (dma_addr_t)NULL;
Can't return a dma_mapping_error address to the caller and have them
figure it out?
Earlier we used to return the DMA_ERROR_CODE which has been removed
recently in arm64 architecture. If we return the dma_mapping_error, then
the caller also needs the device which encountered the mapping error.
The serial interface drivers can use their parent currently to resolve
the mapping error. Once the wrapper starts mapping using IOMMU context
bank, then the serial interface drivers do not know which device to use
to know if there is an error.
Having said that, the dma_ops suggestion might help with handling this
situation. I will look into it further.
Ok, thanks.
+{
+ struct geni_wrapper *wrapper = se->wrapper;
+
+ if (iova)
+ dma_unmap_single(wrapper->dev, iova, len, DMA_FROM_DEVICE);
+}
+EXPORT_SYMBOL(geni_se_rx_dma_unprep);
Instead of having the functions exported, could we set the dma_ops on
all child devices of the wrapper that this driver populates and then
implement the DMA ops for those devices here? I assume that there's
never another DMA master between the wrapper and the serial engine, so I
think it would work.
This suggestion looks like it will work.
It would be a good idea to check with some other people on the dma_ops
suggestion. Maybe add the DMA mapping subsystem folks to help out here
I have added the DMA mapping subsystem folks to help out here.
To present an overview, we have a wrapper controller which is composed
of several serial engines. The serial engines are programmed with UART,
I2C or SPI protocol and support DMA transfer. When the serial engines
perform DMA transfer, the wrapper controller device is used to perform
the mapping. The reason wrapper device is used is because of IOMMU and
there is only one IOMMU context bank to perform the translation for the
entire wrapper controller. So the wrapper controller exports map and
unmap functions to the individual protocol drivers.
There is a suggestion to make the parent wrapper controller implement
the dma_map_ops, instead of exported map/unmap functions and populate
those dma_map_ops on all the children serial engines. Can you please
provide your inputs regarding this suggestion?
DMA MAPPING HELPERS
M: Christoph Hellwig
M: Marek Szyprowski
R: Robin Murphy
L: io...@lists.linux-foundation.org
Regards,
Karthik.
--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Urgent response
Attn: Beneficiary, We have contacted the Federal Ministry of Finance on your Behalf and they have brought a solution to your problem by coordinating your payment in total (10,000,000.00) Ten Million Dollars in an atm card which you can use to withdraw money from any ATM MACHINE CENTER anywhere in the world with a maximum of 1 Dollars daily. You now have the lawful right to claim your fund in an atm card. Since the Federal Bureau of Investigation is involved in this transaction, you have to be rest assured for this is 100% risk free it is our duty to protect the American Citizens, European Citizens, Asian Citizen. All I want you to do is to contact the atm card CENTER Via email or call the office telephone number one of the Consultant will assist you for their requirements to proceed and procure your Approval Slip on your behalf. CONTACT INFORMATION NAME: James Williams EMAIL: paymasterofficed...@gmail.com Do contact us with your details: Full name// Address// Age// Telephone Numbers// Occupation// Your Country// Bank Details// So your files would be updated after which the Delivery of your atm card will be affected to your designated home Address without any further delay and the bank will transfer your funds in total (10,000,000.00) Ten Million Dollars to your Bank account. We will reply you with the secret code (1600 atm card). We advice you get back to the payment office after you have contacted the ATM SWIFT CARD CENTER and we do await your response so we can move on with our Investigation and make sure your ATM SWIFT CARD gets to you. Best Regards James Williams Paymaster General Federal Republic Of Nigeri -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
On 07.03.2018 16:20, Guenter Roeck wrote:
On 03/07/2018 01:47 AM, Rajkumar Rampelli wrote:
On Wednesday 28 February 2018 07:59 PM, Guenter Roeck wrote:
On 02/27/2018 11:03 PM, Mikko Perttunen wrote:
On 02/28/2018 08:12 AM, Rajkumar Rampelli wrote:
On Wednesday 28 February 2018 11:28 AM, Guenter Roeck wrote:
On 02/27/2018 09:38 PM, Rajkumar Rampelli wrote:
On Wednesday 21 February 2018 08:20 PM, Guenter Roeck wrote:
On 02/20/2018 10:58 PM, Rajkumar Rampelli wrote:
Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.
This driver implements a simple interface for monitoring the
speed of
a fan and exposes it in roatations per minute (RPM) to the user
space
by using the hwmon's sysfs interface
Signed-off-by: Rajkumar Rampelli
---
Documentation/hwmon/generic-pwm-tachometer | 17 +
drivers/hwmon/Kconfig | 10 +++
drivers/hwmon/Makefile | 1 +
drivers/hwmon/generic-pwm-tachometer.c | 112
+
4 files changed, 140 insertions(+)
create mode 100644 Documentation/hwmon/generic-pwm-tachometer
create mode 100644 drivers/hwmon/generic-pwm-tachometer.c
diff --git a/Documentation/hwmon/generic-pwm-tachometer
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan.
It uses the
+generic PWM interface and can be used on SoCs as along as the
SoC supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli
+
+Description
+---
+
+The driver implements a simple interface for monitoring the
Fan speed using
+PWM module and Tachometer controller. It requests period value
through PWM
+capture interface to Tachometer and measures the Rotations per
minute using
+received period value. It exposes the Fan speed in RPM to the
user space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
If you say yes here you get support for the temperature
and power sensors for APM X-Gene SoC.
+config GENERIC_PWM_TACHOMETER
+tristate "Generic PWM based tachometer driver"
+depends on PWM
+help
+ Enables a driver to use PWM signal from motor to use
+ for measuring the motor speed. The RPM is captured by
+ PWM modules which has PWM capture capability and this
+ drivers reads the captured data from PWM IP to convert
+ it to speed in RPM.
+
if ACPI
comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)+=
wm8350-hwmon.o
obj-$(CONFIG_SENSORS_XGENE)+= xgene-hwmon.o
obj-$(CONFIG_PMBUS)+= pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o
ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
diff --git a/drivers/hwmon/generic-pwm-tachometer.c
b/drivers/hwmon/generic-pwm-tachometer.c
new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION. All rights
reserved.
+ *
+ * This program is free software; you can redistribute it
and/or modify it
+ * under the terms and conditions of the GNU General Public
License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful,
but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
Public License for
+ * more details.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct pwm_hwmon_tach {
+struct device*dev;
+struct pwm_device*pwm;
+struct device*hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct
device_attribute *attr,
+char *buf)
+{
+struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+struct pwm_device *pwm = ptt->pwm;
+struct pwm_capture result;
+int err;
+unsigned int rpm = 0;
+
+err = pwm_capture(pwm, &result, 0);
+if (err < 0) {
+dev_err(ptt->dev, "Failed to capture PWM: %d\n", err);
+return err;
+}
+
+if (result.period)
+rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+result.period);
+
+return sprintf(buf, "%u\n", rpm);
+}
+
+static SENSOR_DEVICE_ATTR(rpm, 0444, show_rpm, NULL, 0);
+
+static struct attribute *p
