Hi Greg,
These are the patches I have for the next merge window. They are mostly
fixes, one new PCI ID and a "feature" for the Trace Hub's memory buffer
trace collection. Andy Shevchenko again lent his keen reviewer's eye,
and aiaiai static checkers approve.
A signed tag can be found below, individual patches follow. Please,
consider pulling or applying. Thanks!
The following changes since commit bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9:
Linux 5.6-rc1 (2020-02-09 16:08:48 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/ash/stm.git tags/intel_th-stm-for-greg-20200316
for you to fetch changes up to dfc01b7430dc478ce5728539af6f961b7a3ce908:
intel_th: pci: Add Elkhart Lake CPU support (2020-03-16 20:06:04 +0200)
----------------------------------------------------------------
intel_th/stm class: Updates for v5.7
These are:
* An option to (not) stop the trace on buffer-full condition
* Various small fixes
* A new PCI ID for Intel TH
----------------------------------------------------------------
Alexander Shishkin (6):
intel_th: Disallow multi mode on devices where it's broken
stm class: sys-t: Fix the use of time_after()
intel_th: msu: Make stopping the trace optional
intel_th: msu: Fix the unexpected state warning
intel_th: Fix user-visible error codes
intel_th: pci: Add Elkhart Lake CPU support
.../ABI/testing/sysfs-bus-intel_th-devices-msc | 8 +++
drivers/hwtracing/intel_th/intel_th.h | 2 +
drivers/hwtracing/intel_th/msu.c | 61 ++++++++++++++++++----
drivers/hwtracing/intel_th/pci.c | 13 ++++-
drivers/hwtracing/stm/p_sys-t.c | 6 +--
5 files changed, 76 insertions(+), 14 deletions(-)
--
2.25.1
Some versions of Intel TH have an issue that prevents the multi mode of
MSU from working correctly, resulting in no trace data and potentially
stuck MSU pipeline.
Disable multi mode on such devices.
Signed-off-by: Alexander Shishkin <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
drivers/hwtracing/intel_th/intel_th.h | 2 ++
drivers/hwtracing/intel_th/msu.c | 11 +++++++++--
drivers/hwtracing/intel_th/pci.c | 8 ++++++--
3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/hwtracing/intel_th/intel_th.h b/drivers/hwtracing/intel_th/intel_th.h
index 6f4f5486fe6d..5fe694708b7a 100644
--- a/drivers/hwtracing/intel_th/intel_th.h
+++ b/drivers/hwtracing/intel_th/intel_th.h
@@ -47,11 +47,13 @@ struct intel_th_output {
/**
* struct intel_th_drvdata - describes hardware capabilities and quirks
* @tscu_enable: device needs SW to enable time stamping unit
+ * @multi_is_broken: device has multiblock mode is broken
* @has_mintctl: device has interrupt control (MINTCTL) register
* @host_mode_only: device can only operate in 'host debugger' mode
*/
struct intel_th_drvdata {
unsigned int tscu_enable : 1,
+ multi_is_broken : 1,
has_mintctl : 1,
host_mode_only : 1;
};
diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index 8e48c7458aa3..6e118b790d83 100644
--- a/drivers/hwtracing/intel_th/msu.c
+++ b/drivers/hwtracing/intel_th/msu.c
@@ -157,7 +157,8 @@ struct msc {
/* config */
unsigned int enabled : 1,
wrap : 1,
- do_irq : 1;
+ do_irq : 1,
+ multi_is_broken : 1;
unsigned int mode;
unsigned int burst_len;
unsigned int index;
@@ -1664,7 +1665,7 @@ static int intel_th_msc_init(struct msc *msc)
{
atomic_set(&msc->user_count, -1);
- msc->mode = MSC_MODE_MULTI;
+ msc->mode = msc->multi_is_broken ? MSC_MODE_SINGLE : MSC_MODE_MULTI;
mutex_init(&msc->buf_mutex);
INIT_LIST_HEAD(&msc->win_list);
INIT_LIST_HEAD(&msc->iter_list);
@@ -1876,6 +1877,9 @@ mode_store(struct device *dev, struct device_attribute *attr, const char *buf,
return -EINVAL;
found:
+ if (i == MSC_MODE_MULTI && msc->multi_is_broken)
+ return -EOPNOTSUPP;
+
mutex_lock(&msc->buf_mutex);
ret = 0;
@@ -2082,6 +2086,9 @@ static int intel_th_msc_probe(struct intel_th_device *thdev)
if (!res)
msc->do_irq = 1;
+ if (INTEL_TH_CAP(to_intel_th(thdev), multi_is_broken))
+ msc->multi_is_broken = 1;
+
msc->index = thdev->id;
msc->thdev = thdev;
diff --git a/drivers/hwtracing/intel_th/pci.c b/drivers/hwtracing/intel_th/pci.c
index e9d90b53bbc4..ad7e51ebe49e 100644
--- a/drivers/hwtracing/intel_th/pci.c
+++ b/drivers/hwtracing/intel_th/pci.c
@@ -120,6 +120,10 @@ static void intel_th_pci_remove(struct pci_dev *pdev)
pci_free_irq_vectors(pdev);
}
+static const struct intel_th_drvdata intel_th_1x_multi_is_broken = {
+ .multi_is_broken = 1,
+};
+
static const struct intel_th_drvdata intel_th_2x = {
.tscu_enable = 1,
.has_mintctl = 1,
@@ -152,7 +156,7 @@ static const struct pci_device_id intel_th_pci_id_table[] = {
{
/* Kaby Lake PCH-H */
PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0xa2a6),
- .driver_data = (kernel_ulong_t)0,
+ .driver_data = (kernel_ulong_t)&intel_th_1x_multi_is_broken,
},
{
/* Denverton */
@@ -207,7 +211,7 @@ static const struct pci_device_id intel_th_pci_id_table[] = {
{
/* Comet Lake PCH-V */
PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0xa3a6),
- .driver_data = (kernel_ulong_t)&intel_th_2x,
+ .driver_data = (kernel_ulong_t)&intel_th_1x_multi_is_broken,
},
{
/* Ice Lake NNPI */
--
2.25.1
Some use cases prefer to keep collecting the trace data into the last
available window while the other windows are being offloaded instead of
stopping the trace. In this scenario, the window switch happens
automatically when the next window becomes available again.
Add an option to allow this and a sysfs attribute to enable it.
Signed-off-by: Alexander Shishkin <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
.../testing/sysfs-bus-intel_th-devices-msc | 8 ++++
drivers/hwtracing/intel_th/msu.c | 37 ++++++++++++++++++-
2 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-intel_th-devices-msc b/Documentation/ABI/testing/sysfs-bus-intel_th-devices-msc
index 456cb62b384c..7fd2601c2831 100644
--- a/Documentation/ABI/testing/sysfs-bus-intel_th-devices-msc
+++ b/Documentation/ABI/testing/sysfs-bus-intel_th-devices-msc
@@ -40,3 +40,11 @@ Description: (RW) Trigger window switch for the MSC's buffer, in
triggering a window switch for the buffer. Returns an error in any
other operating mode or attempts to write something other than "1".
+What: /sys/bus/intel_th/devices/<intel_th_id>-msc<msc-id>/stop_on_full
+Date: March 2020
+KernelVersion: 5.7
+Contact: Alexander Shishkin <[email protected]>
+Description: (RW) Configure whether trace stops when the last available window
+ becomes full (1/y/Y) or wraps around and continues until the next
+ window becomes available again (0/n/N).
+
diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index 6e118b790d83..45916b48bcf0 100644
--- a/drivers/hwtracing/intel_th/msu.c
+++ b/drivers/hwtracing/intel_th/msu.c
@@ -138,6 +138,7 @@ struct msc {
struct list_head win_list;
struct sg_table single_sgt;
struct msc_window *cur_win;
+ struct msc_window *switch_on_unlock;
unsigned long nr_pages;
unsigned long single_sz;
unsigned int single_wrap : 1;
@@ -154,6 +155,8 @@ struct msc {
struct list_head iter_list;
+ bool stop_on_full;
+
/* config */
unsigned int enabled : 1,
wrap : 1,
@@ -1717,6 +1720,10 @@ void intel_th_msc_window_unlock(struct device *dev, struct sg_table *sgt)
return;
msc_win_set_lockout(win, WIN_LOCKED, WIN_READY);
+ if (msc->switch_on_unlock == win) {
+ msc->switch_on_unlock = NULL;
+ msc_win_switch(msc);
+ }
}
EXPORT_SYMBOL_GPL(intel_th_msc_window_unlock);
@@ -1757,7 +1764,11 @@ static irqreturn_t intel_th_msc_interrupt(struct intel_th_device *thdev)
/* next window: if READY, proceed, if LOCKED, stop the trace */
if (msc_win_set_lockout(next_win, WIN_READY, WIN_INUSE)) {
- schedule_work(&msc->work);
+ if (msc->stop_on_full)
+ schedule_work(&msc->work);
+ else
+ msc->switch_on_unlock = next_win;
+
return IRQ_HANDLED;
}
@@ -2050,11 +2061,35 @@ win_switch_store(struct device *dev, struct device_attribute *attr,
static DEVICE_ATTR_WO(win_switch);
+static ssize_t stop_on_full_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct msc *msc = dev_get_drvdata(dev);
+
+ return scnprintf(buf, PAGE_SIZE, "%d\n", msc->stop_on_full);
+}
+
+static ssize_t stop_on_full_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ struct msc *msc = dev_get_drvdata(dev);
+ unsigned long val;
+ int ret;
+
+ ret = kstrtobool(buf, &msc->stop_on_full);
+
+ return ret ? ret : size;
+}
+
+static DEVICE_ATTR_RW(stop_on_full);
+
static struct attribute *msc_output_attrs[] = {
&dev_attr_wrap.attr,
&dev_attr_mode.attr,
&dev_attr_nr_pages.attr,
&dev_attr_win_switch.attr,
+ &dev_attr_stop_on_full.attr,
NULL,
};
--
2.25.1
There are a few places in the driver that end up returning ENOTSUPP to
the user, replace those with EINVAL.
Signed-off-by: Alexander Shishkin <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Fixes: ba82664c134ef ("intel_th: Add Memory Storage Unit driver")
Cc: [email protected] # v4.4+
---
drivers/hwtracing/intel_th/msu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index 7ac7dd4d3b1c..f08e9e883710 100644
--- a/drivers/hwtracing/intel_th/msu.c
+++ b/drivers/hwtracing/intel_th/msu.c
@@ -765,7 +765,7 @@ static int msc_configure(struct msc *msc)
lockdep_assert_held(&msc->buf_mutex);
if (msc->mode > MSC_MODE_MULTI)
- return -ENOTSUPP;
+ return -EINVAL;
if (msc->mode == MSC_MODE_MULTI) {
if (msc_win_set_lockout(msc->cur_win, WIN_READY, WIN_INUSE))
@@ -1299,7 +1299,7 @@ static int msc_buffer_alloc(struct msc *msc, unsigned long *nr_pages,
} else if (msc->mode == MSC_MODE_MULTI) {
ret = msc_buffer_multi_alloc(msc, nr_pages, nr_wins);
} else {
- ret = -ENOTSUPP;
+ ret = -EINVAL;
}
if (!ret) {
@@ -1535,7 +1535,7 @@ static ssize_t intel_th_msc_read(struct file *file, char __user *buf,
if (ret >= 0)
*ppos = iter->offset;
} else {
- ret = -ENOTSUPP;
+ ret = -EINVAL;
}
put_count:
--
2.25.1
The operands of time_after() are in a wrong order in both instances in
the sys-t driver. Fix that.
Signed-off-by: Alexander Shishkin <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Fixes: 39f10239df75 ("stm class: p_sys-t: Add support for CLOCKSYNC packets")
Fixes: d69d5e83110f ("stm class: Add MIPI SyS-T protocol support")
Cc: [email protected] # v4.20+
---
drivers/hwtracing/stm/p_sys-t.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/stm/p_sys-t.c b/drivers/hwtracing/stm/p_sys-t.c
index b178a5495b67..360b5c03df95 100644
--- a/drivers/hwtracing/stm/p_sys-t.c
+++ b/drivers/hwtracing/stm/p_sys-t.c
@@ -238,7 +238,7 @@ static struct configfs_attribute *sys_t_policy_attrs[] = {
static inline bool sys_t_need_ts(struct sys_t_output *op)
{
if (op->node.ts_interval &&
- time_after(op->ts_jiffies + op->node.ts_interval, jiffies)) {
+ time_after(jiffies, op->ts_jiffies + op->node.ts_interval)) {
op->ts_jiffies = jiffies;
return true;
@@ -250,8 +250,8 @@ static inline bool sys_t_need_ts(struct sys_t_output *op)
static bool sys_t_need_clock_sync(struct sys_t_output *op)
{
if (op->node.clocksync_interval &&
- time_after(op->clocksync_jiffies + op->node.clocksync_interval,
- jiffies)) {
+ time_after(jiffies,
+ op->clocksync_jiffies + op->node.clocksync_interval)) {
op->clocksync_jiffies = jiffies;
return true;
--
2.25.1
The unexpected state warning should only warn on illegal state
transitions. Fix that.
Signed-off-by: Alexander Shishkin <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Fixes: 615c164da0eb4 ("intel_th: msu: Introduce buffer interface")
Cc: [email protected] # v5.4+
---
drivers/hwtracing/intel_th/msu.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index 45916b48bcf0..7ac7dd4d3b1c 100644
--- a/drivers/hwtracing/intel_th/msu.c
+++ b/drivers/hwtracing/intel_th/msu.c
@@ -722,9 +722,6 @@ static int msc_win_set_lockout(struct msc_window *win,
if (old != expect) {
ret = -EINVAL;
- dev_warn_ratelimited(msc_dev(win->msc),
- "expected lockout state %d, got %d\n",
- expect, old);
goto unlock;
}
@@ -745,6 +742,10 @@ static int msc_win_set_lockout(struct msc_window *win,
/* from intel_th_msc_window_unlock(), don't warn if not locked */
if (expect == WIN_LOCKED && old == new)
return 0;
+
+ dev_warn_ratelimited(msc_dev(win->msc),
+ "expected lockout state %d, got %d\n",
+ expect, old);
}
return ret;
--
2.25.1
This adds support for the Trace Hub in Elkhart Lake CPU.
Signed-off-by: Alexander Shishkin <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Cc: [email protected]
---
drivers/hwtracing/intel_th/pci.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/hwtracing/intel_th/pci.c b/drivers/hwtracing/intel_th/pci.c
index ad7e51ebe49e..7ccac74553a6 100644
--- a/drivers/hwtracing/intel_th/pci.c
+++ b/drivers/hwtracing/intel_th/pci.c
@@ -238,6 +238,11 @@ static const struct pci_device_id intel_th_pci_id_table[] = {
PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x4da6),
.driver_data = (kernel_ulong_t)&intel_th_2x,
},
+ {
+ /* Elkhart Lake CPU */
+ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x4529),
+ .driver_data = (kernel_ulong_t)&intel_th_2x,
+ },
{
/* Elkhart Lake */
PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x4b26),
--
2.25.1
Hi
[This is an automated email]
This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v5.5.9, v5.4.25, v4.19.109, v4.14.173, v4.9.216, v4.4.216.
v5.5.9: Build OK!
v5.4.25: Build OK!
v4.19.109: Build OK!
v4.14.173: Build OK!
v4.9.216: Failed to apply! Possible dependencies:
4aa5aed2b6f2 ("intel_th: pci: Add Ice Lake NNPI support")
59d08d00d43c ("intel_th: pci: Add Ice Lake PCH support")
88385866bab8 ("intel_th: pci: Add Elkhart Lake SOC support")
920ce7c33db2 ("intel_th: pci: Add Cedar Fork PCH support")
9c78255fdde4 ("intel_th: pci: Add Tiger Lake support")
9d55499d8da4 ("intel_th: pci: Add Jasper Lake PCH support")
e60e9a4b231a ("intel_th: pci: Add Comet Lake support")
v4.4.216: Failed to apply! Possible dependencies:
4aa5aed2b6f2 ("intel_th: pci: Add Ice Lake NNPI support")
59d08d00d43c ("intel_th: pci: Add Ice Lake PCH support")
88385866bab8 ("intel_th: pci: Add Elkhart Lake SOC support")
920ce7c33db2 ("intel_th: pci: Add Cedar Fork PCH support")
9c78255fdde4 ("intel_th: pci: Add Tiger Lake support")
9d55499d8da4 ("intel_th: pci: Add Jasper Lake PCH support")
e60e9a4b231a ("intel_th: pci: Add Comet Lake support")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
--
Thanks
Sasha
On Tue, Mar 17, 2020 at 08:22:09AM +0200, Alexander Shishkin wrote:
> Hi Greg,
>
> These are the patches I have for the next merge window. They are mostly
> fixes, one new PCI ID and a "feature" for the Trace Hub's memory buffer
> trace collection. Andy Shevchenko again lent his keen reviewer's eye,
> and aiaiai static checkers approve.
>
> A signed tag can be found below, individual patches follow. Please,
> consider pulling or applying. Thanks!
>
> The following changes since commit bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9:
>
> Linux 5.6-rc1 (2020-02-09 16:08:48 -0800)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/ash/stm.git tags/intel_th-stm-for-greg-20200316
This adds a build warning:
drivers/hwtracing/intel_th/msu.c: In function ‘stop_on_full_store’:
drivers/hwtracing/intel_th/msu.c:2078:16: warning: unused variable ‘val’ [-Wunused-variable]
2078 | unsigned long val;
| ^~~
So I'll just pick and choose the patches to take here to try to avoid
that...
Can you fix the offending patch up and resend it?
thanks,
greg k-h
On Tue, Mar 17, 2020 at 08:22:11AM +0200, Alexander Shishkin wrote:
> The operands of time_after() are in a wrong order in both instances in
> the sys-t driver. Fix that.
>
> Signed-off-by: Alexander Shishkin <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Fixes: 39f10239df75 ("stm class: p_sys-t: Add support for CLOCKSYNC packets")
> Fixes: d69d5e83110f ("stm class: Add MIPI SyS-T protocol support")
> Cc: [email protected] # v4.20+
> ---
> drivers/hwtracing/stm/p_sys-t.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
This should go to 5.6-final, right?
Don't mix and match patches in a series if possible...
greg k-h
On Tue, Mar 17, 2020 at 08:22:12AM +0200, Alexander Shishkin wrote:
> Some use cases prefer to keep collecting the trace data into the last
> available window while the other windows are being offloaded instead of
> stopping the trace. In this scenario, the window switch happens
> automatically when the next window becomes available again.
>
> Add an option to allow this and a sysfs attribute to enable it.
>
> Signed-off-by: Alexander Shishkin <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> ---
> .../testing/sysfs-bus-intel_th-devices-msc | 8 ++++
> drivers/hwtracing/intel_th/msu.c | 37 ++++++++++++++++++-
> 2 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-intel_th-devices-msc b/Documentation/ABI/testing/sysfs-bus-intel_th-devices-msc
> index 456cb62b384c..7fd2601c2831 100644
> --- a/Documentation/ABI/testing/sysfs-bus-intel_th-devices-msc
> +++ b/Documentation/ABI/testing/sysfs-bus-intel_th-devices-msc
> @@ -40,3 +40,11 @@ Description: (RW) Trigger window switch for the MSC's buffer, in
> triggering a window switch for the buffer. Returns an error in any
> other operating mode or attempts to write something other than "1".
>
> +What: /sys/bus/intel_th/devices/<intel_th_id>-msc<msc-id>/stop_on_full
> +Date: March 2020
> +KernelVersion: 5.7
> +Contact: Alexander Shishkin <[email protected]>
> +Description: (RW) Configure whether trace stops when the last available window
> + becomes full (1/y/Y) or wraps around and continues until the next
> + window becomes available again (0/n/N).
> +
> diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
> index 6e118b790d83..45916b48bcf0 100644
> --- a/drivers/hwtracing/intel_th/msu.c
> +++ b/drivers/hwtracing/intel_th/msu.c
> @@ -138,6 +138,7 @@ struct msc {
> struct list_head win_list;
> struct sg_table single_sgt;
> struct msc_window *cur_win;
> + struct msc_window *switch_on_unlock;
> unsigned long nr_pages;
> unsigned long single_sz;
> unsigned int single_wrap : 1;
> @@ -154,6 +155,8 @@ struct msc {
>
> struct list_head iter_list;
>
> + bool stop_on_full;
> +
> /* config */
> unsigned int enabled : 1,
> wrap : 1,
> @@ -1717,6 +1720,10 @@ void intel_th_msc_window_unlock(struct device *dev, struct sg_table *sgt)
> return;
>
> msc_win_set_lockout(win, WIN_LOCKED, WIN_READY);
> + if (msc->switch_on_unlock == win) {
> + msc->switch_on_unlock = NULL;
> + msc_win_switch(msc);
> + }
> }
> EXPORT_SYMBOL_GPL(intel_th_msc_window_unlock);
>
> @@ -1757,7 +1764,11 @@ static irqreturn_t intel_th_msc_interrupt(struct intel_th_device *thdev)
>
> /* next window: if READY, proceed, if LOCKED, stop the trace */
> if (msc_win_set_lockout(next_win, WIN_READY, WIN_INUSE)) {
> - schedule_work(&msc->work);
> + if (msc->stop_on_full)
> + schedule_work(&msc->work);
> + else
> + msc->switch_on_unlock = next_win;
> +
> return IRQ_HANDLED;
> }
>
> @@ -2050,11 +2061,35 @@ win_switch_store(struct device *dev, struct device_attribute *attr,
>
> static DEVICE_ATTR_WO(win_switch);
>
> +static ssize_t stop_on_full_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct msc *msc = dev_get_drvdata(dev);
> +
> + return scnprintf(buf, PAGE_SIZE, "%d\n", msc->stop_on_full);
No need for the scnprinf() crazyness for a single boolean value. Just
use sprintf() and keep it simple.
> +static ssize_t stop_on_full_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + struct msc *msc = dev_get_drvdata(dev);
> + unsigned long val;
> + int ret;
> +
> + ret = kstrtobool(buf, &msc->stop_on_full);
> +
> + return ret ? ret : size;
> +}
Here's the problem, you don't use val.
And spell out the ? : crazyness:
if (ret)
return ret;
return size;
much simpler for people to read and understand.
thanks,
greg k-h
On Tue, Mar 17, 2020 at 08:22:13AM +0200, Alexander Shishkin wrote:
> The unexpected state warning should only warn on illegal state
> transitions. Fix that.
>
> Signed-off-by: Alexander Shishkin <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Fixes: 615c164da0eb4 ("intel_th: msu: Introduce buffer interface")
> Cc: [email protected] # v5.4+
> ---
> drivers/hwtracing/intel_th/msu.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
I'll add this to the 5.6-final pile...
On Tue, Mar 17, 2020 at 08:22:14AM +0200, Alexander Shishkin wrote:
> There are a few places in the driver that end up returning ENOTSUPP to
> the user, replace those with EINVAL.
>
> Signed-off-by: Alexander Shishkin <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Fixes: ba82664c134ef ("intel_th: Add Memory Storage Unit driver")
> Cc: [email protected] # v4.4+
> ---
> drivers/hwtracing/intel_th/msu.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Same here, for 5.6
On Tue, Mar 17, 2020 at 08:22:15AM +0200, Alexander Shishkin wrote:
> This adds support for the Trace Hub in Elkhart Lake CPU.
>
> Signed-off-by: Alexander Shishkin <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Cc: [email protected]
> ---
> drivers/hwtracing/intel_th/pci.c | 5 +++++
> 1 file changed, 5 insertions(+)
And again, for 5.6.
Wow this pull request was messed up. Let's go back to just normal patch
series submissions please. If I hadn't seen that build warning, this
would have been a mess.
greg k-h
Some use cases prefer to keep collecting the trace data into the last
available window while the other windows are being offloaded instead of
stopping the trace. In this scenario, the window switch happens
automatically when the next window becomes available again.
Add an option to allow this and a sysfs attribute to enable it.
Signed-off-by: Alexander Shishkin <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
.../testing/sysfs-bus-intel_th-devices-msc | 8 ++++
drivers/hwtracing/intel_th/msu.c | 38 ++++++++++++++++++-
2 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-intel_th-devices-msc b/Documentation/ABI/testing/sysfs-bus-intel_th-devices-msc
index 456cb62b384c..7fd2601c2831 100644
--- a/Documentation/ABI/testing/sysfs-bus-intel_th-devices-msc
+++ b/Documentation/ABI/testing/sysfs-bus-intel_th-devices-msc
@@ -40,3 +40,11 @@ Description: (RW) Trigger window switch for the MSC's buffer, in
triggering a window switch for the buffer. Returns an error in any
other operating mode or attempts to write something other than "1".
+What: /sys/bus/intel_th/devices/<intel_th_id>-msc<msc-id>/stop_on_full
+Date: March 2020
+KernelVersion: 5.7
+Contact: Alexander Shishkin <[email protected]>
+Description: (RW) Configure whether trace stops when the last available window
+ becomes full (1/y/Y) or wraps around and continues until the next
+ window becomes available again (0/n/N).
+
diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index 6e118b790d83..db04086122dc 100644
--- a/drivers/hwtracing/intel_th/msu.c
+++ b/drivers/hwtracing/intel_th/msu.c
@@ -138,6 +138,7 @@ struct msc {
struct list_head win_list;
struct sg_table single_sgt;
struct msc_window *cur_win;
+ struct msc_window *switch_on_unlock;
unsigned long nr_pages;
unsigned long single_sz;
unsigned int single_wrap : 1;
@@ -154,6 +155,8 @@ struct msc {
struct list_head iter_list;
+ bool stop_on_full;
+
/* config */
unsigned int enabled : 1,
wrap : 1,
@@ -1717,6 +1720,10 @@ void intel_th_msc_window_unlock(struct device *dev, struct sg_table *sgt)
return;
msc_win_set_lockout(win, WIN_LOCKED, WIN_READY);
+ if (msc->switch_on_unlock == win) {
+ msc->switch_on_unlock = NULL;
+ msc_win_switch(msc);
+ }
}
EXPORT_SYMBOL_GPL(intel_th_msc_window_unlock);
@@ -1757,7 +1764,11 @@ static irqreturn_t intel_th_msc_interrupt(struct intel_th_device *thdev)
/* next window: if READY, proceed, if LOCKED, stop the trace */
if (msc_win_set_lockout(next_win, WIN_READY, WIN_INUSE)) {
- schedule_work(&msc->work);
+ if (msc->stop_on_full)
+ schedule_work(&msc->work);
+ else
+ msc->switch_on_unlock = next_win;
+
return IRQ_HANDLED;
}
@@ -2050,11 +2061,36 @@ win_switch_store(struct device *dev, struct device_attribute *attr,
static DEVICE_ATTR_WO(win_switch);
+static ssize_t stop_on_full_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct msc *msc = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%d\n", msc->stop_on_full);
+}
+
+static ssize_t stop_on_full_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ struct msc *msc = dev_get_drvdata(dev);
+ int ret;
+
+ ret = kstrtobool(buf, &msc->stop_on_full);
+ if (ret)
+ return ret;
+
+ return size;
+}
+
+static DEVICE_ATTR_RW(stop_on_full);
+
static struct attribute *msc_output_attrs[] = {
&dev_attr_wrap.attr,
&dev_attr_mode.attr,
&dev_attr_nr_pages.attr,
&dev_attr_win_switch.attr,
+ &dev_attr_stop_on_full.attr,
NULL,
};
--
2.25.1