Re: linux-next: Signed-off-by missing for commit in the net-next tree
On Thu, 2020-09-17 at 08:31 +1000, Stephen Rothwell wrote:
> Hi all,
>
> Commits
>
> 0d2ffdc8d400 ("net/mlx5: Don't call timecounter cyc2time directly
> from 1PPS flow")
> 87f3495cbe8d ("net/mlx5: Release clock lock before scheduling a PPS
> work")
> aac2df7f022e ("net/mlx5: Rename ptp clock info")
> fb609b5112bd ("net/mlx5: Always use container_of to find mdev
> pointer from clock struct")
> ec529b44abfe ("net/mlx5: remove erroneous fallthrough")
>
> are missing a Signed-off-by from their committer.
>
Sorry for the mix-up, i overwrote the old mlnx email address with the
new nvidia one but didn't consider the committer email :S.
Re: More filesystem need this fix (xfs: use MMAPLOCK around filemap_map_pages())
On 17.09.20 г. 4:44 ч., Dave Chinner wrote: > On Wed, Sep 16, 2020 at 05:58:51PM +0200, Jan Kara wrote: >> On Sat 12-09-20 09:19:11, Amir Goldstein wrote: >>> On Tue, Jun 23, 2020 at 8:21 AM Dave Chinner wrote: > > So > > P0p1 > > hole punch starts > takes XFS_MMAPLOCK_EXCL > truncate_pagecache_range() > unmap_mapping_range(start, end) > > > do_fault_around() > ->map_pages > filemap_map_pages() > page mapping valid, > page is up to date > maps PTEs > > truncate_inode_pages_range() > truncate_cleanup_page(page) > invalidates page > delete_from_page_cache_batch(page) > frees page > > > That doesn't seem good to me. > > Sure, maybe the page hasn't been freed back to the free lists > because of elevated refcounts. But it's been released by the > filesystem and not longer in the page cache so nothing good can come > of this situation... > > AFAICT, this race condition exists for the truncate case as well > as filemap_map_pages() doesn't have a "page beyond inode size" check > in it. (It's not relevant to the discussion at hand but for the sake of completeness): It does have a check: max_idx = DIV_ROUND_UP(i_size_read(mapping->host), PAGE_SIZE); if (page->index >= max_idx) goto unlock;
[PATCH V3 3/4] ARM: imx_v6_v7_defconfig: Build in CONFIG_GPIO_MXC by default
i.MX SoC GPIO driver provides the basic functions of GPIO pin operations and IRQ operations, it is now changed from "def_bool y" to "tristate", so it should be explicitly enabled to make sure all consumers work normally. Signed-off-by: Anson Huang --- changes since V2: - improve commit message to explain why CONFIG_GPIO_MXC needs to be enabled. --- arch/arm/configs/imx_v6_v7_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig index 0fa79bd..221f5c3 100644 --- a/arch/arm/configs/imx_v6_v7_defconfig +++ b/arch/arm/configs/imx_v6_v7_defconfig @@ -217,6 +217,7 @@ CONFIG_GPIO_PCA953X=y CONFIG_GPIO_PCF857X=y CONFIG_GPIO_STMPE=y CONFIG_GPIO_74X164=y +CONFIG_GPIO_MXC=y CONFIG_POWER_RESET=y CONFIG_POWER_RESET_SYSCON=y CONFIG_POWER_RESET_SYSCON_POWEROFF=y -- 2.7.4
[PATCH V3 2/4] arm64: defconfig: Build in CONFIG_GPIO_MXC by default
i.MX SoC GPIO driver provides the basic functions of GPIO pin operations and IRQ operations, it is now changed from "def_bool y" to "tristate", so it should be explicitly enabled to make sure all consumers work normally. Signed-off-by: Anson Huang --- changes since V2: - improve commit message to explain why CONFIG_GPIO_MXC needs to be enabled. --- arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 63003ec..c8fca1a 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -510,6 +510,7 @@ CONFIG_GPIO_PCA953X=y CONFIG_GPIO_PCA953X_IRQ=y CONFIG_GPIO_BD9571MWV=m CONFIG_GPIO_MAX77620=y +CONFIG_GPIO_MXC=y CONFIG_POWER_AVS=y CONFIG_QCOM_CPR=y CONFIG_ROCKCHIP_IODOMAIN=y -- 2.7.4
[PATCH V3 1/4] gpio: mxc: Support module build
Change config to tristate, add module device table, module author,
description and license to support module build for i.MX GPIO driver.
As this is a SoC GPIO module, it provides common functions for most
of the peripheral devices, such as GPIO pins control, secondary
interrupt controller for GPIO pins IRQ etc., without GPIO driver, most
of the peripheral devices will NOT work properly, so GPIO module is
similar with clock, pinctrl driver that should be loaded ONCE and
never unloaded.
Since MXC GPIO driver needs to have init function to register syscore
ops once, here still use subsys_initcall(), NOT module_platform_driver().
Signed-off-by: Anson Huang
---
no change.
---
drivers/gpio/Kconfig| 2 +-
drivers/gpio/gpio-mxc.c | 6 ++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 5cfdaf3..c7292a5 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -397,7 +397,7 @@ config GPIO_MVEBU
select REGMAP_MMIO
config GPIO_MXC
- def_bool y
+ tristate "i.MX GPIO support"
depends on ARCH_MXC || COMPILE_TEST
select GPIO_GENERIC
select GENERIC_IRQ_CHIP
diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 64278a4..643f4c55 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -158,6 +159,7 @@ static const struct of_device_id mxc_gpio_dt_ids[] = {
{ .compatible = "fsl,imx7d-gpio", .data =
_gpio_devtype[IMX35_GPIO], },
{ /* sentinel */ }
};
+MODULE_DEVICE_TABLE(of, mxc_gpio_dt_ids);
/*
* MX2 has one interrupt *for all* gpio ports. The list is used
@@ -604,3 +606,7 @@ static int __init gpio_mxc_init(void)
return platform_driver_register(_gpio_driver);
}
subsys_initcall(gpio_mxc_init);
+
+MODULE_AUTHOR("Shawn Guo ");
+MODULE_DESCRIPTION("i.MX GPIO Driver");
+MODULE_LICENSE("GPL");
--
2.7.4
[PATCH V3 4/4] ARM: multi_v7_defconfig: Build in CONFIG_GPIO_MXC by default
i.MX SoC GPIO driver provides the basic functions of GPIO pin operations and IRQ operations, it is now changed from "def_bool y" to "tristate", so it should be explicitly enabled to make sure all consumers work normally. Signed-off-by: Anson Huang --- changes since V2: - improve commit message to explain why CONFIG_GPIO_MXC needs to be enabled. --- arch/arm/configs/multi_v7_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig index bfaa38c..d2744ff 100644 --- a/arch/arm/configs/multi_v7_defconfig +++ b/arch/arm/configs/multi_v7_defconfig @@ -465,6 +465,7 @@ CONFIG_GPIO_PALMAS=y CONFIG_GPIO_TPS6586X=y CONFIG_GPIO_TPS65910=y CONFIG_GPIO_TWL4030=y +CONFIG_GPIO_MXC=y CONFIG_POWER_AVS=y CONFIG_ROCKCHIP_IODOMAIN=y CONFIG_POWER_RESET_AS3722=y -- 2.7.4
Re: [PATCH v5 0/5] mm: introduce memfd_secret system call to create "secret" memory areas
On Thu, 17 Sep 2020 at 01:20, Andrew Morton wrote: > > On Wed, 16 Sep 2020 10:35:34 +0300 Mike Rapoport wrote: > > > This is an implementation of "secret" mappings backed by a file descriptor. > > I've dropped the boot time reservation patch for now as it is not strictly > > required for the basic usage and can be easily added later either with or > > without CMA. > > It seems early days for this, especially as regards reviewer buyin. > But I'll toss it in there to get it some additional testing. > > A test suite in tools/testging/selftests/ would be helpful, especially > for arch maintainers. > > I assume that user-facing manpage alterations are planned? I was just about to write a mail into this thread when I saw this :-). So far, I don't think I saw a manual page patch. Mike, how about it? Thanks, Michael -- Michael Kerrisk Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ Linux/UNIX System Programming Training: http://man7.org/training/
Re: [PATCH -next] btrfs: Make btrfs_sysfs_add_fs_devices static
On 16/9/20 10:26 pm, YueHaibing wrote:
Fix sparse warning:
fs/btrfs/sysfs.c:1386:5: warning:
symbol 'btrfs_sysfs_add_fs_devices' was not declared. Should it be static?
misc-next branch has it declared static. It was fixed later.
Thanks, Anand
Signed-off-by: YueHaibing
---
fs/btrfs/sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index e7b0e10685d9..279d9262b676 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -1383,7 +1383,7 @@ int btrfs_sysfs_add_device(struct btrfs_device *device)
return ret;
}
-int btrfs_sysfs_add_fs_devices(struct btrfs_fs_devices *fs_devices)
+static int btrfs_sysfs_add_fs_devices(struct btrfs_fs_devices *fs_devices)
{
int ret;
struct btrfs_device *device;
Re: [PATCH v1] atomisp:pci/runtime/queue: modify the return error value
LGTM.
Reviewed-by: Tianjia Zhang
On 9/17/20 11:44 AM, Xiaoliang Pang wrote:
modify the return error value is -EDOM
Fixes: 2cac05dee6e30("drm/amd/powerplay: add the hw manager for vega12 (v4)")
Cc: Evan Quan
Signed-off-by: Xiaoliang Pang
---
.../staging/media/atomisp/pci/runtime/queue/src/queue_access.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/runtime/queue/src/queue_access.c
b/drivers/staging/media/atomisp/pci/runtime/queue/src/queue_access.c
index fdca743c4ab7..424e7a15a389 100644
--- a/drivers/staging/media/atomisp/pci/runtime/queue/src/queue_access.c
+++ b/drivers/staging/media/atomisp/pci/runtime/queue/src/queue_access.c
@@ -44,7 +44,7 @@ int ia_css_queue_load(
the value as zero. This causes division by 0
exception as the size is used in a modular
division operation. */
- return EDOM;
+ return -EDOM;
}
}
[BlueZ PATCH 4/6] Bluetooth: Handle system suspend resume case
This patch adds code to handle the system suspension during interleave
scan. The interleave scan will be canceled when the system is going to
sleep, and will be restarted after waking up.
Signed-off-by: Howard Chung
Reviewed-by: Alain Michaud
Reviewed-by: Manish Mandlik
Reviewed-by: Abhishek Pandit-Subedi
Reviewed-by: Miao-chen Chou
---
net/bluetooth/hci_request.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 89443b48d90ce..d9082019b6386 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1081,6 +1081,9 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
filter_policy |= 0x02;
if (hdev->suspended) {
+ /* Block suspend notifier on response */
+ set_bit(SUSPEND_SCAN_ENABLE, hdev->suspend_tasks);
+
window = hdev->le_scan_window_suspend;
interval = hdev->le_scan_int_suspend;
} else if (hci_is_le_conn_scanning(hdev)) {
@@ -1167,10 +1170,8 @@ static void hci_req_config_le_suspend_scan(struct
hci_request *req)
hci_req_add_le_scan_disable(req, false);
/* Configure params and enable scanning */
- hci_req_add_le_passive_scan(req);
+ __hci_update_background_scan(req);
- /* Block suspend notifier on response */
- set_bit(SUSPEND_SCAN_ENABLE, req->hdev->suspend_tasks);
}
static void cancel_adv_timeout(struct hci_dev *hdev)
@@ -1282,8 +1283,10 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum
suspended_state next)
hci_req_add(, HCI_OP_WRITE_SCAN_ENABLE, 1, _scan);
/* Disable LE passive scan if enabled */
- if (hci_dev_test_flag(hdev, HCI_LE_SCAN))
+ if (hci_dev_test_flag(hdev, HCI_LE_SCAN)) {
+ cancel_interleave_scan(hdev);
hci_req_add_le_scan_disable(, false);
+ }
/* Mark task needing completion */
set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);
--
2.28.0.618.gf4bc123cb7-goog
[BlueZ PATCH 6/6] Bluetooth: Add toggle to switch off interleave scan
This patch add a configurable parameter to switch off the interleave
scan feature.
Signed-off-by: Howard Chung
Reviewed-by: Alain Michaud
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_core.c | 1 +
net/bluetooth/hci_request.c | 3 ++-
net/bluetooth/mgmt_config.c | 6 ++
4 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 179350f869fdb..c3253f1cac0c2 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -363,6 +363,7 @@ struct hci_dev {
__u32 clock;
__u16 advmon_allowlist_duration;
__u16 advmon_no_filter_duration;
+ __u16 enable_advmon_interleave_scan;
__u16 devid_source;
__u16 devid_vendor;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 6c8850149265a..4608715860cce 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3595,6 +3595,7 @@ struct hci_dev *hci_alloc_dev(void)
/* The default values will be chosen in the future */
hdev->advmon_allowlist_duration = 300;
hdev->advmon_no_filter_duration = 500;
+ hdev->enable_advmon_interleave_scan = 0x0001; /* Default to enable */
hdev->sniff_max_interval = 800;
hdev->sniff_min_interval = 80;
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 1fcf6736811e4..bb38e1dead68f 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -500,7 +500,8 @@ static void __hci_update_background_scan(struct hci_request
*req)
if (hci_dev_test_flag(hdev, HCI_LE_SCAN))
hci_req_add_le_scan_disable(req, false);
- if (!update_adv_monitor_scan_state(hdev)) {
+ if (!hdev->enable_advmon_interleave_scan ||
+ !update_adv_monitor_scan_state(hdev)) {
hci_req_add_le_passive_scan(req);
bt_dev_dbg(hdev, "%s starting background scanning",
hdev->name);
diff --git a/net/bluetooth/mgmt_config.c b/net/bluetooth/mgmt_config.c
index 6dc3e43dcaa9f..4df6de44ee438 100644
--- a/net/bluetooth/mgmt_config.c
+++ b/net/bluetooth/mgmt_config.c
@@ -69,6 +69,7 @@ int read_def_system_config(struct sock *sk, struct hci_dev
*hdev, void *data,
def_le_autoconnect_timeout),
HDEV_PARAM_U16(0x001d, advmon_allowlist_duration),
HDEV_PARAM_U16(0x001e, advmon_no_filter_duration),
+ HDEV_PARAM_U16(0x001f, enable_advmon_interleave_scan),
};
struct mgmt_rp_read_def_system_config *rp = (void *)params;
@@ -143,6 +144,7 @@ int set_def_system_config(struct sock *sk, struct hci_dev
*hdev, void *data,
case 0x001c:
case 0x001d:
case 0x001e:
+ case 0x001f:
if (len != sizeof(u16)) {
bt_dev_warn(hdev, "invalid length %d, exp %zu
for type %d",
len, sizeof(u16), type);
@@ -264,6 +266,10 @@ int set_def_system_config(struct sock *sk, struct hci_dev
*hdev, void *data,
hdev->advmon_no_filter_duration =
TLV_GET_LE16(buffer);
break;
+ case 0x0001f:
+ hdev->enable_advmon_interleave_scan =
+ TLV_GET_LE16(buffer);
+ break;
default:
bt_dev_warn(hdev, "unsupported parameter %u", type);
break;
--
2.28.0.618.gf4bc123cb7-goog
[BlueZ PATCH 2/6] Bluetooth: Set scan parameters for ADV Monitor
Set scan parameters when there is at least one Advertisement monitor.
Signed-off-by: Howard Chung
Reviewed-by: Alain Michaud
Reviewed-by: Manish Mandlik
Reviewed-by: Abhishek Pandit-Subedi
Reviewed-by: Miao-chen Chou
---
net/bluetooth/hci_request.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 413e3a5aabf54..d2b06f5c93804 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1027,6 +1027,9 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
} else if (hci_is_le_conn_scanning(hdev)) {
window = hdev->le_scan_window_connect;
interval = hdev->le_scan_int_connect;
+ } else if (hci_is_adv_monitoring(hdev)) {
+ window = hdev->le_scan_window_adv_monitor;
+ interval = hdev->le_scan_int_adv_monitor;
} else {
window = hdev->le_scan_window;
interval = hdev->le_scan_interval;
--
2.28.0.618.gf4bc123cb7-goog
Re: [PATCH] bitfield.h: annotate type_replace_bits functions with __must_check
On Thu, Sep 17, 2020 at 09:34:59AM +0530, Vinod Koul wrote:
> On 16-09-20, 16:33, Srinivas Kandagatla wrote:
> >
> >
> > On 16/09/2020 16:20, Greg KH wrote:
> > > On Wed, Sep 16, 2020 at 04:03:33PM +0100, Srinivas Kandagatla wrote:
> > > > usage of apis like u32_replace_bits() without actually catching the
> > > > return
> > > > value could hide problems without any warning!
> > > >
> > > > Found this with recent usage of this api in SoundWire!
> > > > Having __must_check annotation would really catch this issues in future!
> > > >
> > > > Signed-off-by: Srinivas Kandagatla
> > > > ---
> > > > include/linux/bitfield.h | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
> > > > index 4e035aca6f7e..eb4f69253946 100644
> > > > --- a/include/linux/bitfield.h
> > > > +++ b/include/linux/bitfield.h
> > > > @@ -131,7 +131,7 @@ static __always_inline __##type
> > > > type##_encode_bits(base v, base field) \
> > > > __field_overflow();
> > > > \
> > > > return to((v & field_mask(field)) * field_multiplier(field));
> > > > \
> > > > }
> > > > \
> > > > -static __always_inline __##type type##_replace_bits(__##type old,
> > > > \
> > > > +static __always_inline __must_check __##type
> > > > type##_replace_bits(__##type old, \
> > > > base val, base field)
> > > > \
> > > > {
> > > > \
> > > > return (old & ~to(field)) | type##_encode_bits(val, field);
> > > > \
> > > > --
> > > > 2.21.0
> > > >
> > >
> > > Don't add __must_check to things that if merged will instantly cause
> > > build warnings to the system, that's just rude :(
> > Currently there are not many users for this api, found only one instance in
> > drivers/net/ipa/ipa_table.c which is also fixed with
> > https://lkml.org/lkml/2020/9/10/1062
> >
> > >
> > > Fix up everything first, and then try to make this type of change.
> > >
> > > But why does this function have to be checked?
> > As this function would return updated value, this check is more to with
> > using the return value!
> >
> > It is easy for someone to ignore this return value assuming that the new
> > value is already updated! So this check should help!
> >
> > TBH, This is what happened when we(vkoul and me) tried use this api :-)
>
> So the only user of this has been moved to *p_replace_bits(), looking
> back I think we should remove *_replace_bits (no users atm) and
> duplicate of *p_replace_bits(). If not adding this patch would be
> sensible thing to do
>
> Somehow I feel former is a better idea ;-)
Yes, please remove it if there is no users.
thanks,
greg k-h
[BlueZ PATCH 3/6] Bluetooth: Interleave with allowlist scan
This patch implements the interleaving between allowlist scan and
no-filter scan. It'll be used to save power when at least one monitor is
registered and at least one pending connection or one device to be
scanned for.
The durations of the allowlist scan and the no-filter scan are
controlled by MGMT command: Set Default System Configuration. The
default values are set randomly for now.
Signed-off-by: Howard Chung
Reviewed-by: Alain Michaud
Reviewed-by: Manish Mandlik
---
include/net/bluetooth/hci_core.h | 10 +++
net/bluetooth/hci_core.c | 4 +
net/bluetooth/hci_request.c | 137 +--
net/bluetooth/mgmt_config.c | 13 +++
4 files changed, 156 insertions(+), 8 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 9873e1c8cd163..179350f869fdb 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -361,6 +361,8 @@ struct hci_dev {
__u8ssp_debug_mode;
__u8hw_error_code;
__u32 clock;
+ __u16 advmon_allowlist_duration;
+ __u16 advmon_no_filter_duration;
__u16 devid_source;
__u16 devid_vendor;
@@ -542,6 +544,14 @@ struct hci_dev {
struct delayed_work rpa_expired;
bdaddr_trpa;
+ enum {
+ ADV_MONITOR_SCAN_NONE,
+ ADV_MONITOR_SCAN_NO_FILTER,
+ ADV_MONITOR_SCAN_ALLOWLIST
+ } adv_monitor_scan_state;
+
+ struct delayed_work interleave_adv_monitor_scan;
+
#if IS_ENABLED(CONFIG_BT_LEDS)
struct led_trigger *power_led;
#endif
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index f30a1f5950e15..6c8850149265a 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3592,6 +3592,10 @@ struct hci_dev *hci_alloc_dev(void)
hdev->cur_adv_instance = 0x00;
hdev->adv_instance_timeout = 0;
+ /* The default values will be chosen in the future */
+ hdev->advmon_allowlist_duration = 300;
+ hdev->advmon_no_filter_duration = 500;
+
hdev->sniff_max_interval = 800;
hdev->sniff_min_interval = 80;
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index d2b06f5c93804..89443b48d90ce 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -378,6 +378,57 @@ void __hci_req_write_fast_connectable(struct hci_request
*req, bool enable)
hci_req_add(req, HCI_OP_WRITE_PAGE_SCAN_TYPE, 1, );
}
+static void start_interleave_scan(struct hci_dev *hdev)
+{
+ hdev->adv_monitor_scan_state = ADV_MONITOR_SCAN_NO_FILTER;
+ queue_delayed_work(hdev->req_workqueue,
+ >interleave_adv_monitor_scan, 0);
+}
+
+static bool is_interleave_scanning(struct hci_dev *hdev)
+{
+ return hdev->adv_monitor_scan_state != ADV_MONITOR_SCAN_NONE;
+}
+
+static void cancel_interleave_scan(struct hci_dev *hdev)
+{
+ bt_dev_dbg(hdev, "%s cancelling interleave scan", hdev->name);
+
+ cancel_delayed_work_sync(>interleave_adv_monitor_scan);
+
+ hdev->adv_monitor_scan_state = ADV_MONITOR_SCAN_NONE;
+}
+
+/* Return true if interleave_scan is running after exiting this function,
+ * otherwise, return false
+ */
+static bool update_adv_monitor_scan_state(struct hci_dev *hdev)
+{
+ if (!hci_is_adv_monitoring(hdev) ||
+ (list_empty(>pend_le_conns) &&
+list_empty(>pend_le_reports))) {
+ if (is_interleave_scanning(hdev)) {
+ /* If the interleave condition no longer holds, cancel
+* the existed interleave scan.
+*/
+ cancel_interleave_scan(hdev);
+ }
+ return false;
+ }
+
+ if (!is_interleave_scanning(hdev)) {
+ /* If there is at least one ADV monitors and one pending LE
+* connection or one device to be scanned for, we should
+* alternate between allowlist scan and one without any filters
+* to save power.
+*/
+ start_interleave_scan(hdev);
+ bt_dev_dbg(hdev, "%s starting interleave scan", hdev->name);
+ }
+
+ return true;
+}
+
/* This function controls the background scanning based on hdev->pend_le_conns
* list. If there are pending LE connection we start the background scanning,
* otherwise we stop it.
@@ -449,9 +500,11 @@ static void __hci_update_background_scan(struct
hci_request *req)
if (hci_dev_test_flag(hdev, HCI_LE_SCAN))
hci_req_add_le_scan_disable(req, false);
- hci_req_add_le_passive_scan(req);
-
- BT_DBG("%s starting background scanning", hdev->name);
+ if (!update_adv_monitor_scan_state(hdev)) {
+
[BlueZ PATCH 5/6] Bluetooth: Handle active scan case
This patch adds code to handle the active scan during interleave
scan. The interleave scan will be canceled when users start active scan,
and it will be restarted after active scan stopped.
Signed-off-by: Howard Chung
Reviewed-by: Alain Michaud
Reviewed-by: Manish Mandlik
---
net/bluetooth/hci_request.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index d9082019b6386..1fcf6736811e4 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -3085,8 +3085,10 @@ static int active_scan(struct hci_request *req, unsigned
long opt)
* running. Thus, we should temporarily stop it in order to set the
* discovery scanning parameters.
*/
- if (hci_dev_test_flag(hdev, HCI_LE_SCAN))
+ if (hci_dev_test_flag(hdev, HCI_LE_SCAN)) {
hci_req_add_le_scan_disable(req, false);
+ cancel_interleave_scan(hdev);
+ }
/* All active scans will be done with either a resolvable private
* address (when privacy feature has been enabled) or non-resolvable
--
2.28.0.618.gf4bc123cb7-goog
[BlueZ PATCH 1/6] Bluetooth: Update Adv monitor count upon removal
From: Miao-chen Chou
This fixes the count of Adv monitor upon monitor removal.
The following test was performed.
- Start two btmgmt consoles, issue a btmgmt advmon-remove command on one
console and observe a MGMT_EV_ADV_MONITOR_REMOVED event on the other.
Signed-off-by: Howard Chung
Signed-off-by: Miao-chen Chou
Reviewed-by: Alain Michaud
---
net/bluetooth/hci_core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8a2645a833013..f30a1f5950e15 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3061,6 +3061,7 @@ static int free_adv_monitor(int id, void *ptr, void *data)
idr_remove(>adv_monitors_idr, monitor->handle);
hci_free_adv_monitor(monitor);
+ hdev->adv_monitors_cnt--;
return 0;
}
@@ -3077,6 +3078,7 @@ int hci_remove_adv_monitor(struct hci_dev *hdev, u16
handle)
idr_remove(>adv_monitors_idr, monitor->handle);
hci_free_adv_monitor(monitor);
+ hdev->adv_monitors_cnt--;
} else {
/* Remove all monitors if handle is 0. */
idr_for_each(>adv_monitors_idr, _adv_monitor, hdev);
--
2.28.0.618.gf4bc123cb7-goog
Re: [PATCH] Revert "perf report: Treat an argument as a symbol filter"
On Thu, Sep 17, 2020 at 12:15 PM Wei Li wrote:
>
> Since commit 6db6127c4dad ("perf report: Treat an argument as a symbol
> filter"), the only one unrecognized argument for perf-report is treated
> as a symbol filter. This is not described in man page nor help info,
> and the result is really confusing, especially when it's misspecified by
> the user (e.g. missing -i for perf.data).
How about adding documentation then?
Thanks
Namhyung
>
> As we can use "--symbol-filter=" if we really want to filter a symbol,
> it may be better to revert this misfeature.
>
> Signed-off-by: Wei Li
> ---
> tools/perf/builtin-report.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 3c74c9c0f3c3..f57ebc1bcd20 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -1317,13 +1317,9 @@ int cmd_report(int argc, const char **argv)
> argc = parse_options(argc, argv, options, report_usage, 0);
> if (argc) {
> /*
> -* Special case: if there's an argument left then assume that
> -* it's a symbol filter:
> +* Any (unrecognized) arguments left?
> */
> - if (argc > 1)
> - usage_with_options(report_usage, options);
> -
> - report.symbol_filter_str = argv[0];
> + usage_with_options(report_usage, options);
> }
>
> if (annotate_check_args(_opts) < 0)
> --
> 2.17.1
>
[PATCH v2 3/4] i2c: designware: Allow SMBus block reads up to 255 bytes in length
From: Sultan Alsawaf
According to the SMBus 3.0 protocol specification, block transfer limits
were increased from 32 bytes to 255 bytes. Remove the obsolete 32-byte
limitation.
Signed-off-by: Sultan Alsawaf
---
drivers/i2c/busses/i2c-designware-master.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-designware-master.c
b/drivers/i2c/busses/i2c-designware-master.c
index 22f28516bca7..5bd64bd17d94 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -433,7 +433,7 @@ i2c_dw_read(struct dw_i2c_dev *dev)
regmap_read(dev->map, DW_IC_DATA_CMD, );
if (flags & I2C_M_RECV_LEN) {
/* Ensure length byte is a valid value */
- if (tmp <= I2C_SMBUS_BLOCK_MAX && tmp > 0)
+ if (tmp > 0)
len = i2c_dw_recv_len(dev, tmp);
else
len = i2c_dw_recv_len(dev, len);
--
2.28.0
[PATCH v2 2/4] i2c: designware: Ensure tx_buf_len is nonzero for SMBus block reads
From: Sultan Alsawaf
The point of adding a byte to len in i2c_dw_recv_len() is to make sure
that tx_buf_len is nonzero, so that i2c_dw_xfer_msg() can let the i2c
controller know that the i2c transaction can end. Otherwise, the i2c
controller will think that the transaction can never end for block
reads, which results in the stop-detection bit never being set and thus
the transaction timing out.
Adding a byte to len is not a reliable way to do this though; sometimes
it lets tx_buf_len become zero, which results in the scenario described
above. Therefore, just directly ensure tx_buf_len cannot be zero to fix
the issue.
Fixes: c3ae106050b9 ("i2c: designware: Implement support for SMBus block read
and write")
Signed-off-by: Sultan Alsawaf
---
drivers/i2c/busses/i2c-designware-master.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-master.c
b/drivers/i2c/busses/i2c-designware-master.c
index d78f48ca4886..22f28516bca7 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -395,8 +395,9 @@ i2c_dw_recv_len(struct dw_i2c_dev *dev, u8 len)
* Adjust the buffer length and mask the flag
* after receiving the first byte.
*/
- len += (flags & I2C_CLIENT_PEC) ? 2 : 1;
- dev->tx_buf_len = len - min_t(u8, len, dev->rx_outstanding);
+ if (flags & I2C_CLIENT_PEC)
+ len++;
+ dev->tx_buf_len = len - min_t(u8, len - 1, dev->rx_outstanding);
msgs[dev->msg_read_idx].len = len;
msgs[dev->msg_read_idx].flags &= ~I2C_M_RECV_LEN;
--
2.28.0
[PATCH v2 1/4] i2c: designware: Fix transfer failures for invalid SMBus block reads
From: Sultan Alsawaf
SMBus block reads can be broken because the read function will just skip
over bytes it doesn't like until reaching a byte that conforms to the
length restrictions for block reads. This is problematic when it isn't
known if the incoming payload is indeed a conforming block read.
According to the SMBus specification, block reads will only send the
payload length in the first byte, so we can fix this by only considering
the first byte in a sequence for block read length purposes.
In addition, when the length byte is invalid, the original transfer
length still needs to be adjusted to avoid a controller timeout.
Fixes: c3ae106050b9 ("i2c: designware: Implement support for SMBus block read
and write")
Signed-off-by: Sultan Alsawaf
---
drivers/i2c/busses/i2c-designware-master.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-master.c
b/drivers/i2c/busses/i2c-designware-master.c
index d6425ad6e6a3..d78f48ca4886 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -430,10 +430,12 @@ i2c_dw_read(struct dw_i2c_dev *dev)
u32 flags = msgs[dev->msg_read_idx].flags;
regmap_read(dev->map, DW_IC_DATA_CMD, );
- /* Ensure length byte is a valid value */
- if (flags & I2C_M_RECV_LEN &&
- tmp <= I2C_SMBUS_BLOCK_MAX && tmp > 0) {
- len = i2c_dw_recv_len(dev, tmp);
+ if (flags & I2C_M_RECV_LEN) {
+ /* Ensure length byte is a valid value */
+ if (tmp <= I2C_SMBUS_BLOCK_MAX && tmp > 0)
+ len = i2c_dw_recv_len(dev, tmp);
+ else
+ len = i2c_dw_recv_len(dev, len);
}
*buf++ = tmp;
dev->rx_outstanding--;
--
2.28.0
[PATCH v2 0/4] i2c-hid: Save power by reducing i2c xfers with block reads
From: Sultan Alsawaf This is a fixed resubmission of "[PATCH 0/2] i2c-hid: Save power by reducing i2c xfers with block reads". That original patchset did not have enough fixes for the designware i2c adapter's I2C_M_RECV_LEN feature, which is documented extensively in the original email thread. Here is the original cover letter, which still applies: "I noticed on my Dell Precision 15 5540 with an i9-9880H that simply putting my finger on the touchpad would increase my system's power consumption by 4W, which is quite considerable. Resting my finger on the touchpad would generate roughly 4000 i2c irqs per second, or roughly 20 i2c irqs per touchpad irq. Upon closer inspection, I noticed that the i2c-hid driver would always transfer the maximum report size over i2c (which is 60 bytes for my touchpad), but all of my touchpad's normal touch events are only 32 bytes long according to the length byte contained in the buffer sequence. Therefore, I was able to save about 2W of power by passing the I2C_M_RECV_LEN flag in i2c-hid, which says to look for the payload length in the first byte of the transfer buffer and adjust the i2c transaction accordingly. The only problem though is that my i2c controller's driver allows bytes other than the first one to be used to retrieve the payload length, which is incorrect according to the SMBus spec, and would break my i2c-hid change since not *all* of the reports from my touchpad are conforming SMBus block reads. This patchset fixes the I2C_M_RECV_LEN behavior in the designware i2c driver and modifies i2c-hid to use I2C_M_RECV_LEN to save quite a bit of power. Even if the peripheral controlled by i2c-hid doesn't support block reads, the i2c controller drivers should cope with this and proceed with the i2c transfer using the original requested length." Sultan Sultan Alsawaf (4): i2c: designware: Fix transfer failures for invalid SMBus block reads i2c: designware: Ensure tx_buf_len is nonzero for SMBus block reads i2c: designware: Allow SMBus block reads up to 255 bytes in length HID: i2c-hid: Use block reads when possible to save power drivers/hid/i2c-hid/i2c-hid-core.c | 5 - drivers/i2c/busses/i2c-designware-master.c | 15 +-- 2 files changed, 13 insertions(+), 7 deletions(-) -- 2.28.0
[PATCH v2 4/4] HID: i2c-hid: Use block reads when possible to save power
From: Sultan Alsawaf
We have no way of knowing how large an incoming payload is going to be,
so the only strategy available up until now has been to always retrieve
the maximum possible report length over i2c, which can be quite
inefficient. For devices that send reports in block read format, the i2c
controller driver can read the payload length on the fly and terminate
the i2c transaction early, resulting in considerable power savings.
On a Dell Precision 15 5540 with an i9-9880H, resting my finger on the
touchpad causes psys power readings to go up by about 4W and hover there
until I remove my finger. With this patch, my psys readings go from 4.7W
down to 3.1W, yielding about 1.6W in savings. This is because my
touchpad's max report length is 60 bytes, but all of the regular reports
it sends for touch events are only 32 bytes, so the i2c transfer is
roughly halved for the common case.
Acked-by: Jiri Kosina
Signed-off-by: Sultan Alsawaf
---
drivers/hid/i2c-hid/i2c-hid-core.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c
b/drivers/hid/i2c-hid/i2c-hid-core.c
index dbd04492825d..66950f472122 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -476,11 +476,14 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
int ret;
u32 ret_size;
int size = le16_to_cpu(ihid->hdesc.wMaxInputLength);
+ u16 flags;
if (size > ihid->bufsize)
size = ihid->bufsize;
- ret = i2c_master_recv(ihid->client, ihid->inbuf, size);
+ /* Try to do a block read if the size fits in one byte */
+ flags = size > 255 ? I2C_M_RD : I2C_M_RD | I2C_M_RECV_LEN;
+ ret = i2c_transfer_buffer_flags(ihid->client, ihid->inbuf, size, flags);
if (ret != size) {
if (ret < 0)
return;
--
2.28.0
Re: [PATCH] perf metric: Code cleanup with map_for_each_event()
Hello,
On Thu, Sep 17, 2020 at 11:45 AM Wei Li wrote:
>
> Since we have introduced map_for_each_event() to walk the 'pmu_events_map',
> clean up metricgroup__print() and metricgroup__has_metric() with it.
>
> Signed-off-by: Wei Li
Acked-by: Namhyung Kim
A nit-pick below:
> ---
> tools/perf/util/metricgroup.c | 33 +
> 1 file changed, 13 insertions(+), 20 deletions(-)
>
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 8831b964288f..3734cbb2c456 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -26,6 +26,17 @@
> #include "util.h"
> #include
>
> +#define map_for_each_event(__pe, __idx, __map) \
> + for (__idx = 0, __pe = &__map->table[__idx];\
> +__pe->name || __pe->metric_group || __pe->metric_name; \
> +__pe = &__map->table[++__idx])
> +
> +#define map_for_each_metric(__pe, __idx, __map, __metric) \
> + map_for_each_event(__pe, __idx, __map) \
> + if (__pe->metric_expr &&\
> + (match_metric(__pe->metric_group, __metric) || \
> +match_metric(__pe->metric_name, __metric)))
> +
You may consider adding a declaration of match_metric() here.
Right now, all users are below the function so it's ok but
having the macro here can enable future addition above IMHO.
Thanks
Namhyung
> struct metric_event *metricgroup__lookup(struct rblist *metric_events,
> struct evsel *evsel,
> bool create)
> @@ -475,12 +486,9 @@ void metricgroup__print(bool metrics, bool metricgroups,
> char *filter,
> groups.node_new = mep_new;
> groups.node_cmp = mep_cmp;
> groups.node_delete = mep_delete;
> - for (i = 0; ; i++) {
> + map_for_each_event(pe, i, map) {
> const char *g;
> - pe = >table[i];
>
> - if (!pe->name && !pe->metric_group && !pe->metric_name)
> - break;
> if (!pe->metric_expr)
> continue;
> g = pe->metric_group;
> @@ -745,17 +753,6 @@ static int __add_metric(struct list_head *metric_list,
> return 0;
> }
>
> -#define map_for_each_event(__pe, __idx, __map) \
> - for (__idx = 0, __pe = &__map->table[__idx];\
> -__pe->name || __pe->metric_group || __pe->metric_name; \
> -__pe = &__map->table[++__idx])
> -
> -#define map_for_each_metric(__pe, __idx, __map, __metric) \
> - map_for_each_event(__pe, __idx, __map) \
> - if (__pe->metric_expr &&\
> - (match_metric(__pe->metric_group, __metric) || \
> -match_metric(__pe->metric_name, __metric)))
> -
> static struct pmu_event *find_metric(const char *metric, struct
> pmu_events_map *map)
> {
> struct pmu_event *pe;
> @@ -1092,11 +1089,7 @@ bool metricgroup__has_metric(const char *metric)
> if (!map)
> return false;
>
> - for (i = 0; ; i++) {
> - pe = >table[i];
> -
> - if (!pe->name && !pe->metric_group && !pe->metric_name)
> - break;
> + map_for_each_event(pe, i, map) {
> if (!pe->metric_expr)
> continue;
> if (match_metric(pe->metric_name, metric))
> --
> 2.17.1
>
linux-next: build failure after merge of the rcu tree
Hi all,
After merging the rcu tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:
In file included from kernel/rcu/update.c:578:
kernel/rcu/tasks.h:601:20: error: static declaration of
'show_rcu_tasks_classic_gp_kthread' follows non-static declaration
601 | static inline void show_rcu_tasks_classic_gp_kthread(void) { }
|^
In file included from kernel/rcu/update.c:49:
kernel/rcu/rcu.h:537:6: note: previous declaration of
'show_rcu_tasks_classic_gp_kthread' was here
537 | void show_rcu_tasks_classic_gp_kthread(void);
| ^
Caused by commit
675d3ca52626 ("rcutorture: Make grace-period kthread report match RCU flavor
being tested")
I have used the rcu tree from next-20200916 for today.
--
Cheers,
Stephen Rothwell
pgpmzEy0j4Tet.pgp
Description: OpenPGP digital signature
Re: [RFC PATCH V3 12/21] mmc: sdhci: UHS-II support, add hooks for additional operations
On Thu, Sep 17, 2020 at 07:52:03AM +0300, Adrian Hunter wrote:
> On 17/09/20 5:31 am, AKASHI Takahiro wrote:
> > Adrian,
> >
> > On Wed, Sep 16, 2020 at 01:00:35PM +0300, Adrian Hunter wrote:
> >> On 16/09/20 11:05 am, AKASHI Takahiro wrote:
> >>> Adrian,
> >>>
> >>> Your comments are scattered over various functions, and so
> >>> I would like to address them in separate replies.
> >>>
> >>> First, I'd like to discuss sdhci_[add|remove]_host().
> >>>
> >>> On Fri, Aug 21, 2020 at 05:08:32PM +0300, Adrian Hunter wrote:
> On 10/07/20 2:10 pm, Ben Chuang wrote:
> > From: Ben Chuang
> >
> > In this commit, UHS-II related operations will be called via a function
> > pointer array, sdhci_uhs2_ops, in order to make UHS-II support as
> > a kernel module.
> > This array will be initialized only if CONFIG_MMC_SDHCI_UHS2 is enabled
> > and when the UHS-II module is loaded. Otherwise, all the functions
> > stay void.
> >
> > Signed-off-by: Ben Chuang
> > Signed-off-by: AKASHI Takahiro
> > ---
> >>>
> >>> (snip)
> >>>
> > if (intmask & (SDHCI_INT_CARD_INSERT |
> > SDHCI_INT_CARD_REMOVE)) {
> > u32 present = sdhci_readl(host,
> > SDHCI_PRESENT_STATE) &
> > SDHCI_CARD_PRESENT;
> > @@ -4717,6 +4812,14 @@ int sdhci_setup_host(struct sdhci_host *host)
> > /* This may alter mmc->*_blk_* parameters */
> > sdhci_allocate_bounce_buffer(host);
> >
> > + if (IS_ENABLED(CONFIG_MMC_SDHCI_UHS2) &&
> > + host->version >= SDHCI_SPEC_400 &&
> > + sdhci_uhs2_ops.add_host) {
> > + ret = sdhci_uhs2_ops.add_host(host, host->caps1);
> > + if (ret)
> > + goto unreg;
> > + }
> > +
>
> I think you should look at creating uhs2_add_host() instead
>
> > return 0;
> >
> > unreg:
> > @@ -4738,6 +4841,8 @@ void sdhci_cleanup_host(struct sdhci_host *host)
> > {
> > struct mmc_host *mmc = host->mmc;
> >
> > + /* FIXME: Do we have to do some cleanup for UHS2 here? */
> > +
> > if (!IS_ERR(mmc->supply.vqmmc))
> > regulator_disable(mmc->supply.vqmmc);
> >
> > @@ -4766,6 +4871,14 @@ int __sdhci_add_host(struct sdhci_host *host)
> > mmc->cqe_ops = NULL;
> > }
> >
> > + if ((mmc->caps & MMC_CAP_UHS2) && !host->v4_mode) {
> > + /* host doesn't want to enable UHS2 support */
> > + mmc->caps &= ~MMC_CAP_UHS2;
> > + mmc->flags &= ~MMC_UHS2_SUPPORT;
> > +
> > + /* FIXME: Do we have to do some cleanup here? */
> > + }
> > +
> > host->complete_wq = alloc_workqueue("sdhci", flags, 0);
> > if (!host->complete_wq)
> > return -ENOMEM;
> > @@ -4812,6 +4925,9 @@ int __sdhci_add_host(struct sdhci_host *host)
> > unled:
> > sdhci_led_unregister(host);
> > unirq:
> > + if (IS_ENABLED(CONFIG_MMC_SDHCI_UHS2) &&
> > + sdhci_uhs2_ops.remove_host)
> > + sdhci_uhs2_ops.remove_host(host, 0);
> > sdhci_do_reset(host, SDHCI_RESET_ALL);
> > sdhci_writel(host, 0, SDHCI_INT_ENABLE);
> > sdhci_writel(host, 0, SDHCI_SIGNAL_ENABLE);
> > @@ -4869,6 +4985,10 @@ void sdhci_remove_host(struct sdhci_host *host,
> > int dead)
> >
> > sdhci_led_unregister(host);
> >
> > + if (IS_ENABLED(CONFIG_MMC_SDHCI_UHS2) &&
> > + sdhci_uhs2_ops.remove_host)
> > + sdhci_uhs2_ops.remove_host(host, dead);
> > +
>
> I think you should look at creating uhs2_remove_host() instead
> >>>
> >>> You suggest that we will have separate sdhci_uhs2_[add|remove]_host(),
> >>> but I don't think it's always convenient.
> >>>
> >>> UHS-II capable host will be set to call sdhci_uhs2_add_host() explicitly,
> >>> but we can't do that in case of pci and pltfm based drivers as they
> >>> utilize
> >>> common helper functions, sdhci_pci_probe() and sdhci_pltfm_register(),
> >>> respectively.
> >>
> >> sdhci-pci has an add_host op
> >>
> >> sdhci_pltfm_init can be used instead of sdhci_pltfm_register
> >>
> >>
> >>> Therefore, we inevitably have to call sdhci_uhs2_add_host() there.
> >>>
> >>> If so, why should we distinguish sdhci_uhs2_add_host from
> >>> sdhci_uhs_add_host?
> >>> I don't see any good reason.
> >>> Moreover, as a result, there exists a mixed usage of sdhci_ interfaces
> >>> and sdhci_uhs2_ interfaces in sdhci-pci-core.c and sdhci-pltfm.c.
> >>>
> >>> It sounds odd to me.
> >>
> >> It is already done that way for cqhci.
> >
> > Okay, if it is your policy, I will follow that.
> > Then, I'm going to add
> > -
Re: [RFC PATCH V3 12/21] mmc: sdhci: UHS-II support, add hooks for additional operations
Adrian, Ben,
Regarding _reset() function,
On Fri, Aug 21, 2020 at 05:08:32PM +0300, Adrian Hunter wrote:
> On 10/07/20 2:10 pm, Ben Chuang wrote:
> > From: Ben Chuang
> >
> > In this commit, UHS-II related operations will be called via a function
> > pointer array, sdhci_uhs2_ops, in order to make UHS-II support as
> > a kernel module.
> > This array will be initialized only if CONFIG_MMC_SDHCI_UHS2 is enabled
> > and when the UHS-II module is loaded. Otherwise, all the functions
> > stay void.
> >
> > Signed-off-by: Ben Chuang
> > Signed-off-by: AKASHI Takahiro
> > ---
> > drivers/mmc/host/sdhci.c | 152 ++-
> > 1 file changed, 136 insertions(+), 16 deletions(-)
> >
(snip)
> > if (host->ops->platform_send_init_74_clocks)
> > host->ops->platform_send_init_74_clocks(host, ios->power_mode);
> >
> > @@ -2331,7 +2411,7 @@ void sdhci_set_ios(struct mmc_host *mmc, struct
> > mmc_ios *ios)
> > }
> >
> > if (host->version >= SDHCI_SPEC_300) {
> > - u16 clk, ctrl_2;
> > + u16 clk;
> >
> > if (!host->preset_enabled) {
> > sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
> > @@ -3173,11 +3253,19 @@ static bool sdhci_request_done(struct sdhci_host
> > *host)
> > /* This is to force an update */
> > host->ops->set_clock(host, host->clock);
> >
> > - /* Spec says we should do both at the same time, but Ricoh
> > - controllers do not like that. */
> > - sdhci_do_reset(host, SDHCI_RESET_CMD);
> > - sdhci_do_reset(host, SDHCI_RESET_DATA);
> > -
> > + if (IS_ENABLED(CONFIG_MMC_SDHCI_UHS2) &&
> > + host->mmc->flags & MMC_UHS2_INITIALIZED) {
> > + if (sdhci_uhs2_ops.reset)
> > + sdhci_uhs2_ops.reset(host,
> > +SDHCI_UHS2_SW_RESET_SD);
> > + } else {
> > + /*
> > +* Spec says we should do both at the same time, but
> > +* Ricoh controllers do not like that.
> > +*/
> > + sdhci_do_reset(host, SDHCI_RESET_CMD);
> > + sdhci_do_reset(host, SDHCI_RESET_DATA);
> > + }
>
> Please look at using the existing ->reset() sdhci host op instead.
Well, the second argument to those reset functions is a bit-wise value
to different "reset" registers, SDHCI_SOFTWARE_RESET and SDHCI_UHS2_SW_RESET,
respectively.
This fact raises a couple of questions to me:
1) Does it make sense to merge two functionality into one, i.e.
sdhci_do_reset(), which is set to call ->reset hook?
-> Adrian
2) UHS2_SW_RESET_SD is done only at this place while there are many callsites
of reset(RESET_CMD|RESET_DATA) in sdhci.c.
Why does the current code work?
I found, in sdhci-pci-gli.c,
sdhci_gl9755_reset()
/* reset sd-tran on UHS2 mode if need to reset cmd/data */
if ((mask & SDHCI_RESET_CMD) | (mask & SDHCI_RESET_DATA))
gl9755_uhs2_reset_sd_tran(host);
Is this the trick to avoid the issue?
(It looks redundant in terms of the hack above in sdhci_request_done()
and even quite dirty to me. Moreover, no corresponding code for gl9750
and gl9763.)
-> Ben
3) (More or less SD specification issue)
In UHS-II mode, do we have to call reset(SHCI_RESET_ALL) along with
reset(UHS2_SW_RESET_FULL)?
Under the current implementation, both will be called at the end.
-> Adrian, Ben
4) (Not directly linked to UHS-II support)
In some places, we see the sequence:
sdhci_do_reset(host, SDHCI_RESET_CMD);
sdhci_do_reset(host, SDHCI_RESET_DATA);
while in other places,
sdhci_do_reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
If the statement below is true,
> > - /* Spec says we should do both at the same time, but Ricoh
> > - controllers do not like that. */
the latter should be wrong.
-> Adrian
-Takahiro Akashi
> > host->pending_reset = false;
> > }
> >
> > @@ -3532,6 +3620,13 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
> > SDHCI_INT_BUS_POWER);
> > sdhci_writel(host, mask, SDHCI_INT_STATUS);
> >
> > + if (IS_ENABLED(CONFIG_MMC_SDHCI_UHS2) &&
> > + intmask & SDHCI_INT_ERROR &&
> > + host->mmc->flags & MMC_UHS2_SUPPORT) {
> > + if (sdhci_uhs2_ops.irq)
> > + sdhci_uhs2_ops.irq(host);
> > + }
> > +
>
> Please look at using the existing ->irq() sdhci host op instead
>
> > if (intmask & (SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE)) {
> > u32 present = sdhci_readl(host, SDHCI_PRESENT_STATE) &
> > SDHCI_CARD_PRESENT;
> > @@ -4717,6 +4812,14 @@ int
Re: [PATCH 1/5] dt-bindings: phy: qcom,qmp: Document SM8250 PCIe PHY bindings
On Wed 16 Sep 23:32 CDT 2020, Vinod Koul wrote: > On 16-09-20, 17:45, Bjorn Andersson wrote: > > On Wed 16 Sep 08:19 CDT 2020, Manivannan Sadhasivam wrote: > > > > > Document the DT bindings of below PCIe PHY versions used on SM8250: > > > > > > QMP GEN3x1 PHY - 1 lane > > > QMP GEN3x2 PHY - 2 lanes > > > QMP Modem PHY - 2 lanes > > > > How about something like "Add the three PCIe PHYs found in SM8250 to the > > QMP binding"? > > Or add just one compatible sm8250-qmp-pcie and then use number of lanes > as dt property? > If we have the same initialization sequence then that sounds reasonable. Perhaps we can derive the number of lanes from the child node? A bigger question is how we deal with this going forward, if there are more crazy setups like on sc8180x where the lanes might be grouped differently based on board... Regards, Bjorn > > > > > > > > Signed-off-by: Manivannan Sadhasivam > > > --- > > > Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml | 5 + > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml > > > b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml > > > index 185cdea9cf81..69b67f79075c 100644 > > > --- a/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml > > > +++ b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml > > > @@ -31,6 +31,9 @@ properties: > > >- qcom,sdm845-qmp-usb3-uni-phy > > >- qcom,sm8150-qmp-ufs-phy > > >- qcom,sm8250-qmp-ufs-phy > > > + - qcom,qcom,sm8250-qmp-gen3x1-pcie-phy > > > + - qcom,qcom,sm8250-qmp-gen3x2-pcie-phy > > > + - qcom,qcom,sm8250-qmp-modem-pcie-phy > > > > One "qcom," should be enough. > > > > > > > >reg: > > > items: > > > @@ -259,6 +262,8 @@ allOf: > > > enum: > > >- qcom,sdm845-qhp-pcie-phy > > >- qcom,sdm845-qmp-pcie-phy > > > + - qcom,sm8250-qhp-pcie-phy > > > + - qcom,sm8250-qmp-pcie-phy > > > > Adjust these. > > > > Regards, > > Bjorn > > > > > then: > > >properties: > > > clocks: > > > -- > > > 2.17.1 > > > > > -- > ~Vinod
[PATCH net-next] net: stmmac: introduce rtnl_lock|unlock() on configuring real_num_rx|tx_queues
From: "Tan, Tee Min"
For driver open(), rtnl_lock is acquired by network stack but not in the
resume(). Therefore, we introduce lock_acquired boolean to control when
to use rtnl_lock|unlock() within stmmac_hw_setup().
Fixes: 686cff3d7022 ("net: stmmac: Fix incorrect location to set
real_num_rx|tx_queues")
Signed-off-by: Tan, Tee Min
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index df2c74bbfcff..22e6a3defa78 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2607,7 +2607,8 @@ static void stmmac_safety_feat_configuration(struct
stmmac_priv *priv)
* 0 on success and an appropriate (-)ve integer as defined in errno.h
* file on failure.
*/
-static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
+static int stmmac_hw_setup(struct net_device *dev, bool init_ptp,
+ bool lock_acquired)
{
struct stmmac_priv *priv = netdev_priv(dev);
u32 rx_cnt = priv->plat->rx_queues_to_use;
@@ -2715,9 +2716,15 @@ static int stmmac_hw_setup(struct net_device *dev, bool
init_ptp)
}
/* Configure real RX and TX queues */
+ if (!lock_acquired)
+ rtnl_lock();
+
netif_set_real_num_rx_queues(dev, priv->plat->rx_queues_to_use);
netif_set_real_num_tx_queues(dev, priv->plat->tx_queues_to_use);
+ if (!lock_acquired)
+ rtnl_unlock();
+
/* Start the ball rolling... */
stmmac_start_all_dma(priv);
@@ -2804,7 +2811,7 @@ static int stmmac_open(struct net_device *dev)
goto init_error;
}
- ret = stmmac_hw_setup(dev, true);
+ ret = stmmac_hw_setup(dev, true, true);
if (ret < 0) {
netdev_err(priv->dev, "%s: Hw setup failed\n", __func__);
goto init_error;
@@ -5238,7 +5245,7 @@ int stmmac_resume(struct device *dev)
stmmac_clear_descriptors(priv);
- stmmac_hw_setup(ndev, false);
+ stmmac_hw_setup(ndev, false, false);
stmmac_init_coalesce(priv);
stmmac_set_rx_mode(ndev);
--
2.17.0
Re: [RFC PATCH V3 12/21] mmc: sdhci: UHS-II support, add hooks for additional operations
On 17/09/20 5:31 am, AKASHI Takahiro wrote:
> Adrian,
>
> On Wed, Sep 16, 2020 at 01:00:35PM +0300, Adrian Hunter wrote:
>> On 16/09/20 11:05 am, AKASHI Takahiro wrote:
>>> Adrian,
>>>
>>> Your comments are scattered over various functions, and so
>>> I would like to address them in separate replies.
>>>
>>> First, I'd like to discuss sdhci_[add|remove]_host().
>>>
>>> On Fri, Aug 21, 2020 at 05:08:32PM +0300, Adrian Hunter wrote:
On 10/07/20 2:10 pm, Ben Chuang wrote:
> From: Ben Chuang
>
> In this commit, UHS-II related operations will be called via a function
> pointer array, sdhci_uhs2_ops, in order to make UHS-II support as
> a kernel module.
> This array will be initialized only if CONFIG_MMC_SDHCI_UHS2 is enabled
> and when the UHS-II module is loaded. Otherwise, all the functions
> stay void.
>
> Signed-off-by: Ben Chuang
> Signed-off-by: AKASHI Takahiro
> ---
>>>
>>> (snip)
>>>
> if (intmask & (SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE)) {
> u32 present = sdhci_readl(host, SDHCI_PRESENT_STATE) &
> SDHCI_CARD_PRESENT;
> @@ -4717,6 +4812,14 @@ int sdhci_setup_host(struct sdhci_host *host)
> /* This may alter mmc->*_blk_* parameters */
> sdhci_allocate_bounce_buffer(host);
>
> + if (IS_ENABLED(CONFIG_MMC_SDHCI_UHS2) &&
> + host->version >= SDHCI_SPEC_400 &&
> + sdhci_uhs2_ops.add_host) {
> + ret = sdhci_uhs2_ops.add_host(host, host->caps1);
> + if (ret)
> + goto unreg;
> + }
> +
I think you should look at creating uhs2_add_host() instead
> return 0;
>
> unreg:
> @@ -4738,6 +4841,8 @@ void sdhci_cleanup_host(struct sdhci_host *host)
> {
> struct mmc_host *mmc = host->mmc;
>
> + /* FIXME: Do we have to do some cleanup for UHS2 here? */
> +
> if (!IS_ERR(mmc->supply.vqmmc))
> regulator_disable(mmc->supply.vqmmc);
>
> @@ -4766,6 +4871,14 @@ int __sdhci_add_host(struct sdhci_host *host)
> mmc->cqe_ops = NULL;
> }
>
> + if ((mmc->caps & MMC_CAP_UHS2) && !host->v4_mode) {
> + /* host doesn't want to enable UHS2 support */
> + mmc->caps &= ~MMC_CAP_UHS2;
> + mmc->flags &= ~MMC_UHS2_SUPPORT;
> +
> + /* FIXME: Do we have to do some cleanup here? */
> + }
> +
> host->complete_wq = alloc_workqueue("sdhci", flags, 0);
> if (!host->complete_wq)
> return -ENOMEM;
> @@ -4812,6 +4925,9 @@ int __sdhci_add_host(struct sdhci_host *host)
> unled:
> sdhci_led_unregister(host);
> unirq:
> + if (IS_ENABLED(CONFIG_MMC_SDHCI_UHS2) &&
> + sdhci_uhs2_ops.remove_host)
> + sdhci_uhs2_ops.remove_host(host, 0);
> sdhci_do_reset(host, SDHCI_RESET_ALL);
> sdhci_writel(host, 0, SDHCI_INT_ENABLE);
> sdhci_writel(host, 0, SDHCI_SIGNAL_ENABLE);
> @@ -4869,6 +4985,10 @@ void sdhci_remove_host(struct sdhci_host *host,
> int dead)
>
> sdhci_led_unregister(host);
>
> + if (IS_ENABLED(CONFIG_MMC_SDHCI_UHS2) &&
> + sdhci_uhs2_ops.remove_host)
> + sdhci_uhs2_ops.remove_host(host, dead);
> +
I think you should look at creating uhs2_remove_host() instead
>>>
>>> You suggest that we will have separate sdhci_uhs2_[add|remove]_host(),
>>> but I don't think it's always convenient.
>>>
>>> UHS-II capable host will be set to call sdhci_uhs2_add_host() explicitly,
>>> but we can't do that in case of pci and pltfm based drivers as they utilize
>>> common helper functions, sdhci_pci_probe() and sdhci_pltfm_register(),
>>> respectively.
>>
>> sdhci-pci has an add_host op
>>
>> sdhci_pltfm_init can be used instead of sdhci_pltfm_register
>>
>>
>>> Therefore, we inevitably have to call sdhci_uhs2_add_host() there.
>>>
>>> If so, why should we distinguish sdhci_uhs2_add_host from
>>> sdhci_uhs_add_host?
>>> I don't see any good reason.
>>> Moreover, as a result, there exists a mixed usage of sdhci_ interfaces
>>> and sdhci_uhs2_ interfaces in sdhci-pci-core.c and sdhci-pltfm.c.
>>>
>>> It sounds odd to me.
>>
>> It is already done that way for cqhci.
>
> Okay, if it is your policy, I will follow that.
> Then, I'm going to add
> - remove_host field to struct sdhci_pci_fixes
> - a controller specific helper function to each driver (only pci-gli for now)
> even though it looks quite generic.
If they seem generic then consider naming them
sdhci_pci_uhs2_[add|remove]_host and putting them in sdhci-pci-core.c
>
> sdhci_gli_[add|remove]_host(struct sdhci_pci_slot *slot)
> {
> return sdhci_uhs2_[add|remove]_host(slot->host);
> }
>
> # Or do you want to create a file like sdhci-uhs2-pci.c for those functions?
No
>
> -Takahiro Akashi
>
>>>
>>> -Takahiro Akashi
>>>
>>>
Re: [PATCH] selftests/harness: Flush stdout before forking
On Wed, Sep 16, 2020 at 9:16 PM Michael Ellerman wrote: > > The test harness forks() a child to run each test. Both the parent and > the child print to stdout using libc functions. That can lead to > duplicated (or more) output if the libc buffers are not flushed before > forking. > > It's generally not seen when running programs directly, because stdout > will usually be line buffered when it's pointing to a terminal. > > This was noticed when running the seccomp_bpf test, eg: > > $ ./seccomp_bpf | tee test.log > $ grep -c "TAP version 13" test.log > 2 > > But we only expect the TAP header to appear once. > > It can be exacerbated using stdbuf to increase the buffer size: > > $ stdbuf -o 1MB ./seccomp_bpf > test.log > $ grep -c "TAP version 13" test.log > 13 > > The fix is simple, we just flush stdout & stderr before fork. Usually > stderr is unbuffered, but that can be changed, so flush it as well > just to be safe. > > Signed-off-by: Michael Ellerman > --- > tools/testing/selftests/kselftest_harness.h | 5 + > 1 file changed, 5 insertions(+) Tested-by: Max Filippov -- Thanks. -- Max
Re: [PATCH 2/5] phy: qualcomm: phy-qcom-qmp: Add PCIe PHY support for SM8250 SoC
On 16-09-20, 18:49, Manivannan Sadhasivam wrote:
> SM8250 has multiple different PHY versions:
> QMP GEN3x1 PHY - 1 lane
> QMP GEN3x2 PHY - 2 lanes
> QMP Modem PHY - 2 lanes
>
> Add support for these with relevant init sequence.
>
> Signed-off-by: Manivannan Sadhasivam
> ---
> drivers/phy/qualcomm/phy-qcom-qmp.c | 297
> drivers/phy/qualcomm/phy-qcom-qmp.h | 18 ++
> 2 files changed, 315 insertions(+)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c
> b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index 562053ce9455..746f49ef2542 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> @@ -217,6 +217,13 @@ static const unsigned int
> sdm845_ufsphy_regs_layout[QPHY_LAYOUT_SIZE] = {
> [QPHY_PCS_READY_STATUS] = 0x160,
> };
>
> +static const unsigned int sm8250_pcie_regs_layout[QPHY_LAYOUT_SIZE] = {
> + [QPHY_SW_RESET] = 0x00,
> + [QPHY_START_CTRL] = 0x44,
> + [QPHY_PCS_STATUS] = 0x14,
> + [QPHY_PCS_POWER_DOWN_CONTROL] = 0x40,
> +};
> +
> static const unsigned int sm8150_ufsphy_regs_layout[QPHY_LAYOUT_SIZE] = {
> [QPHY_START_CTRL] = QPHY_V4_PCS_UFS_PHY_START,
> [QPHY_PCS_READY_STATUS] = QPHY_V4_PCS_UFS_READY_STATUS,
> @@ -1743,6 +1750,226 @@ static const struct qmp_phy_init_tbl
> sm8250_usb3_uniphy_pcs_tbl[] = {
> QMP_PHY_INIT_CFG(QPHY_V4_PCS_REFGEN_REQ_CONFIG1, 0x21),
> };
>
> +static const struct qmp_phy_init_tbl sm8250_qmp_gen3x1_pcie_serdes_tbl[] = {
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_SYSCLK_EN_SEL, 0x08),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_CLK_SELECT, 0x34),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_CORECLK_DIV_MODE1, 0x08),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_PLL_IVCO, 0x0f),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_LOCK_CMP_EN, 0x42),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_VCO_TUNE1_MODE0, 0x24),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_VCO_TUNE2_MODE1, 0x03),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_VCO_TUNE1_MODE1, 0xb4),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_VCO_TUNE_MAP, 0x02),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_BIN_VCOCAL_HSCLK_SEL, 0x11),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DEC_START_MODE0, 0x82),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DIV_FRAC_START3_MODE0, 0x03),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DIV_FRAC_START2_MODE0, 0x55),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DIV_FRAC_START1_MODE0, 0x55),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_LOCK_CMP2_MODE0, 0x1a),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_LOCK_CMP1_MODE0, 0x0a),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DEC_START_MODE1, 0x68),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DIV_FRAC_START3_MODE1, 0x02),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DIV_FRAC_START2_MODE1, 0xaa),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DIV_FRAC_START1_MODE1, 0xab),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_LOCK_CMP2_MODE1, 0x34),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_LOCK_CMP1_MODE1, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_HSCLK_SEL, 0x01),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_CP_CTRL_MODE0, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_PLL_RCTRL_MODE0, 0x16),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_PLL_CCTRL_MODE0, 0x36),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_CP_CTRL_MODE1, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_PLL_RCTRL_MODE1, 0x16),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_PLL_CCTRL_MODE1, 0x36),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE2_MODE0, 0x1e),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE1_MODE0, 0xca),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE2_MODE1, 0x18),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_BIN_VCOCAL_CMP_CODE1_MODE1, 0xa2),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_SYSCLK_BUF_ENABLE, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_EN_CENTER, 0x01),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_PER1, 0x31),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_PER2, 0x01),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_STEP_SIZE1_MODE0, 0xde),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_STEP_SIZE2_MODE0, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_STEP_SIZE1_MODE1, 0x4c),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_STEP_SIZE2_MODE1, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_CLK_ENABLE1, 0x90),
> +};
> +
> +static const struct qmp_phy_init_tbl sm8250_qmp_gen3x1_pcie_tx_tbl[] = {
> + QMP_PHY_INIT_CFG(QSERDES_V4_TX_RCV_DETECT_LVL_2, 0x12),
> + QMP_PHY_INIT_CFG(QSERDES_V4_TX_LANE_MODE_1, 0x35),
> + QMP_PHY_INIT_CFG(QSERDES_V4_TX_RES_CODE_LANE_OFFSET_TX, 0x11),
> +};
> +
> +static const struct qmp_phy_init_tbl sm8250_qmp_gen3x1_pcie_rx_tbl[] = {
> + QMP_PHY_INIT_CFG(QSERDES_V4_RX_UCDR_FO_GAIN, 0x0c),
> + QMP_PHY_INIT_CFG(QSERDES_V4_RX_UCDR_SO_GAIN, 0x03),
> + QMP_PHY_INIT_CFG(QSERDES_V4_RX_GM_CAL, 0x1b),
> + QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_IDAC_TSETTLE_HIGH, 0x00),
> + QMP_PHY_INIT_CFG(QSERDES_V4_RX_RX_IDAC_TSETTLE_LOW, 0xc0),
> +
Re: [PATCH v8 00/18] blk-mq/scsi: Provide hostwide shared tags for SCSI HBAs
John, > Have you had a chance to check these outstanding SCSI patches? > > scsi: megaraid_sas: Added support for shared host tagset for cpuhotplug > scsi: scsi_debug: Support host tagset > scsi: hisi_sas: Switch v3 hw to MQ > scsi: core: Show nr_hw_queues in sysfs > scsi: Add host and host template flag 'host_tagset' These look good to me. Jens, feel free to merge. Acked-by: Martin K. Petersen -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH 2/2] arm64/mm: Enable color zero pages
Hi Robin, On 9/16/20 8:46 PM, Robin Murphy wrote: On 2020-09-16 09:28, Will Deacon wrote: On Wed, Sep 16, 2020 at 01:25:23PM +1000, Gavin Shan wrote: This enables color zero pages by allocating contigous page frames for it. The number of pages for this is determined by L1 dCache (or iCache) size, which is probbed from the hardware. * Add cache_total_size() to return L1 dCache (or iCache) size * Implement setup_zero_pages(), which is called after the page allocator begins to work, to allocate the contigous pages needed by color zero page. * Reworked ZERO_PAGE() and define __HAVE_COLOR_ZERO_PAGE. Signed-off-by: Gavin Shan --- arch/arm64/include/asm/cache.h | 22 arch/arm64/include/asm/pgtable.h | 9 ++-- arch/arm64/kernel/cacheinfo.c | 34 +++ arch/arm64/mm/init.c | 35 arch/arm64/mm/mmu.c | 7 --- 5 files changed, 98 insertions(+), 9 deletions(-) diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h index a4d1b5f771f6..420e9dde2c51 100644 --- a/arch/arm64/include/asm/cache.h +++ b/arch/arm64/include/asm/cache.h @@ -39,6 +39,27 @@ #define CLIDR_LOC(clidr) (((clidr) >> CLIDR_LOC_SHIFT) & 0x7) #define CLIDR_LOUIS(clidr) (((clidr) >> CLIDR_LOUIS_SHIFT) & 0x7) +#define CSSELR_TND_SHIFT 4 +#define CSSELR_TND_MASK (UL(1) << CSSELR_TND_SHIFT) +#define CSSELR_LEVEL_SHIFT 1 +#define CSSELR_LEVEL_MASK (UL(7) << CSSELR_LEVEL_SHIFT) +#define CSSELR_IND_SHIFT 0 +#define CSSERL_IND_MASK (UL(1) << CSSELR_IND_SHIFT) + +#define CCSIDR_64_LS_SHIFT 0 +#define CCSIDR_64_LS_MASK (UL(7) << CCSIDR_64_LS_SHIFT) +#define CCSIDR_64_ASSOC_SHIFT 3 +#define CCSIDR_64_ASSOC_MASK (UL(0x1F) << CCSIDR_64_ASSOC_SHIFT) +#define CCSIDR_64_SET_SHIFT 32 +#define CCSIDR_64_SET_MASK (UL(0xFF) << CCSIDR_64_SET_SHIFT) + +#define CCSIDR_32_LS_SHIFT 0 +#define CCSIDR_32_LS_MASK (UL(7) << CCSIDR_32_LS_SHIFT) +#define CCSIDR_32_ASSOC_SHIFT 3 +#define CCSIDR_32_ASSOC_MASK (UL(0x3FF) << CCSIDR_32_ASSOC_SHIFT) +#define CCSIDR_32_SET_SHIFT 13 +#define CCSIDR_32_SET_MASK (UL(0x7FFF) << CCSIDR_32_SET_SHIFT) I don't think we should be inferring cache structure from these register values. The Arm ARM helpfully says: | You cannot make any inference about the actual sizes of caches based | on these parameters. so we need to take the topology information from elsewhere. Yes, these represent parameters for the low-level cache maintenance by set/way instructions, and nothing more. There are definitely cases where they do not reflect the underlying cache structure (commit 793acf870ea3 is an obvious first call). Thanks for your confirming. Yeah, the system registers aren't reliable since the cache way/set are hard coded to 1/1. As I suggested in another thread, ACPI (PPTT) table would be correct place to get such kind of information. [...] Cheers, Gavin
Re: [PATCH -next] soc: use devm_platform_ioremap_resource_byname
On Wed 16 Sep 11:15 UTC 2020, Qilong Zhang wrote:
> Use the devm_platform_ioremap_resource_byname() helper instead of
> calling platform_get_resource_byname() and devm_ioremap_resource()
> separately.
>
> Signed-off-by: Zhang Qilong
I fixed up the subject line to match the previous commits in this driver
and applied the patch.
Thank you,
Bjorn
> ---
> drivers/soc/qcom/llcc-qcom.c | 7 +--
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
> index 429b5a60a1ba..70fbe70c6213 100644
> --- a/drivers/soc/qcom/llcc-qcom.c
> +++ b/drivers/soc/qcom/llcc-qcom.c
> @@ -387,7 +387,6 @@ static int qcom_llcc_remove(struct platform_device *pdev)
> static struct regmap *qcom_llcc_init_mmio(struct platform_device *pdev,
> const char *name)
> {
> - struct resource *res;
> void __iomem *base;
> struct regmap_config llcc_regmap_config = {
> .reg_bits = 32,
> @@ -396,11 +395,7 @@ static struct regmap *qcom_llcc_init_mmio(struct
> platform_device *pdev,
> .fast_io = true,
> };
>
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
> - if (!res)
> - return ERR_PTR(-ENODEV);
> -
> - base = devm_ioremap_resource(>dev, res);
> + base = devm_platform_ioremap_resource_byname(pdev, name);
> if (IS_ERR(base))
> return ERR_CAST(base);
>
> --
> 2.17.1
>
Re: [PATCH v2] nfs: remove incorrect fallthrough label
On Wed, Sep 16, 2020 at 01:02:55PM -0700, Nick Desaulniers wrote:
> There is no case after the default from which to fallthrough to. Clang
> will error in this case (unhelpfully without context, see link below)
> and GCC will with -Wswitch-unreachable.
>
> The previous commit should have just replaced the comment with a break
> statement.
>
> If we consider implicit fallthrough to be a design mistake of C, then
> all case statements should be terminated with one of the following
> statements:
> * break
> * continue
> * return
> * __attribute__(__fallthrough__)
> * goto (plz no)
> * (call of function with __attribute__(__noreturn__))
>
> Fixes: 2a1390c95a69 ("nfs: Convert to use the preferred fallthrough macro")
> Link: https://bugs.llvm.org/show_bug.cgi?id=47539
> Suggested-by: Joe Perches
> Signed-off-by: Nick Desaulniers
Reviewed-by: Nathan Chancellor
> ---
> Changes v2:
> * add break rather than no terminating statement as per Joe.
> * add Joe's suggested by tag.
> * add blurb about acceptable terminal statements.
>
> fs/nfs/super.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index d20326ee0475..eb2401079b04 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -889,7 +889,7 @@ static struct nfs_server *nfs_try_mount_request(struct
> fs_context *fc)
> default:
> if (rpcauth_get_gssinfo(flavor, ) != 0)
> continue;
> - fallthrough;
> + break;
> }
> dfprintk(MOUNT, "NFS: attempting to use auth flavor %u\n",
> flavor);
> ctx->selected_flavor = flavor;
> --
> 2.28.0.618.gf4bc123cb7-goog
>
Re: [PATCH 1/5] dt-bindings: phy: qcom,qmp: Document SM8250 PCIe PHY bindings
On 16-09-20, 17:45, Bjorn Andersson wrote: > On Wed 16 Sep 08:19 CDT 2020, Manivannan Sadhasivam wrote: > > > Document the DT bindings of below PCIe PHY versions used on SM8250: > > > > QMP GEN3x1 PHY - 1 lane > > QMP GEN3x2 PHY - 2 lanes > > QMP Modem PHY - 2 lanes > > How about something like "Add the three PCIe PHYs found in SM8250 to the > QMP binding"? Or add just one compatible sm8250-qmp-pcie and then use number of lanes as dt property? > > > > > Signed-off-by: Manivannan Sadhasivam > > --- > > Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml > > b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml > > index 185cdea9cf81..69b67f79075c 100644 > > --- a/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml > > +++ b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml > > @@ -31,6 +31,9 @@ properties: > >- qcom,sdm845-qmp-usb3-uni-phy > >- qcom,sm8150-qmp-ufs-phy > >- qcom,sm8250-qmp-ufs-phy > > + - qcom,qcom,sm8250-qmp-gen3x1-pcie-phy > > + - qcom,qcom,sm8250-qmp-gen3x2-pcie-phy > > + - qcom,qcom,sm8250-qmp-modem-pcie-phy > > One "qcom," should be enough. > > > > >reg: > > items: > > @@ -259,6 +262,8 @@ allOf: > > enum: > >- qcom,sdm845-qhp-pcie-phy > >- qcom,sdm845-qmp-pcie-phy > > + - qcom,sm8250-qhp-pcie-phy > > + - qcom,sm8250-qmp-pcie-phy > > Adjust these. > > Regards, > Bjorn > > > then: > >properties: > > clocks: > > -- > > 2.17.1 > > -- ~Vinod
RE: [PATCH v1] powerplay:hwmgr - modify the return value
[AMD Official Use Only - Internal Distribution Only]
Thanks. Reviewed-by: Evan Quan
-Original Message-
From: Xiaoliang Pang
Sent: Thursday, September 17, 2020 11:46 AM
To: Quan, Evan ; Deucher, Alexander
; Koenig, Christian ;
airl...@linux.ie; dan...@ffwll.ch; Feng, Kenneth ;
zhengbi...@huawei.com; pe...@vangils.xyz; yt...@amd.com
Cc: Das, Nirmoy ; Huang, JinHuiEric
; amd-...@lists.freedesktop.org;
dri-de...@lists.freedesktop.org; linux-kernel@vger.kernel.org;
tianjia.zh...@linux.alibaba.com; dawning.p...@gmail.com
Subject: [PATCH v1] powerplay:hwmgr - modify the return value
modify the return value is -EINVAL
Fixes: f83a9991648bb("drm/amd/powerplay: add Vega10 powerplay support (v5)")
Fixes: 2cac05dee6e30("drm/amd/powerplay: add the hw manager for vega12 (v4)")
Cc: Eric Huang
Cc: Evan Quan
Signed-off-by: Xiaoliang Pang
---
drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 2 +-
drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
index c378a000c934..7eada3098ffc 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
@@ -4659,7 +4659,7 @@ static int
vega10_display_configuration_changed_task(struct pp_hwmgr *hwmgr)
if ((data->water_marks_bitmap & WaterMarksExist) &&
!(data->water_marks_bitmap & WaterMarksLoaded)) {
result = smum_smc_table_manager(hwmgr, (uint8_t *)wm_table, WMTABLE, false);
-PP_ASSERT_WITH_CODE(result, "Failed to update WMTABLE!", return EINVAL);
+PP_ASSERT_WITH_CODE(result, "Failed to update WMTABLE!", return -EINVAL);
data->water_marks_bitmap |= WaterMarksLoaded;
}
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
index a678a67f1c0d..04da52cea824 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
@@ -2390,7 +2390,7 @@ static int
vega12_display_configuration_changed_task(struct pp_hwmgr *hwmgr)
!(data->water_marks_bitmap & WaterMarksLoaded)) {
result = smum_smc_table_manager(hwmgr,
(uint8_t *)wm_table, TABLE_WATERMARKS, false);
-PP_ASSERT_WITH_CODE(result, "Failed to update WMTABLE!", return EINVAL);
+PP_ASSERT_WITH_CODE(result, "Failed to update WMTABLE!", return -EINVAL);
data->water_marks_bitmap |= WaterMarksLoaded;
}
--
2.17.1
[PATCH] selftests/harness: Flush stdout before forking
The test harness forks() a child to run each test. Both the parent and
the child print to stdout using libc functions. That can lead to
duplicated (or more) output if the libc buffers are not flushed before
forking.
It's generally not seen when running programs directly, because stdout
will usually be line buffered when it's pointing to a terminal.
This was noticed when running the seccomp_bpf test, eg:
$ ./seccomp_bpf | tee test.log
$ grep -c "TAP version 13" test.log
2
But we only expect the TAP header to appear once.
It can be exacerbated using stdbuf to increase the buffer size:
$ stdbuf -o 1MB ./seccomp_bpf > test.log
$ grep -c "TAP version 13" test.log
13
The fix is simple, we just flush stdout & stderr before fork. Usually
stderr is unbuffered, but that can be changed, so flush it as well
just to be safe.
Signed-off-by: Michael Ellerman
---
tools/testing/selftests/kselftest_harness.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/kselftest_harness.h
b/tools/testing/selftests/kselftest_harness.h
index 4f78e4805633..f19804df244c 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -971,6 +971,11 @@ void __run_test(struct __fixture_metadata *f,
ksft_print_msg(" RUN %s%s%s.%s ...\n",
f->name, variant->name[0] ? "." : "", variant->name, t->name);
+
+ /* Make sure output buffers are flushed before fork */
+ fflush(stdout);
+ fflush(stderr);
+
t->pid = fork();
if (t->pid < 0) {
ksft_print_msg("ERROR SPAWNING TEST CHILD\n");
--
2.25.1
Re: [v3 2/2] mm: khugepaged: avoid overriding min_free_kbytes set by user
Please ignore this patch. I forgot to run scripts/checkpatch.pl and see
CHECK: Alignment should match open parenthesis
#30: FILE: mm/khugepaged.c:2287:
+ if (recommended_min > min_free_kbytes ||
+ recommended_min > user_min_free_kbytes) {
ERROR: do not initialise globals to 0
#43: FILE: mm/page_alloc.c:318:
+int user_min_free_kbytes = 0;
Sorry for trouble, I will send a new version.
Vijay
On 9/16/2020 6:21 PM, Vijay Balakrishna wrote:
set_recommended_min_free_kbytes need to honor min_free_kbytes set by the
user. Post start-of-day THP enable or memory hotplug operations can
lose user specified min_free_kbytes, in particular when it is higher than
calculated recommended value. user_min_free_kbytes initialized to 0
to avoid undesired result when comparing with "unsigned long" type.
Signed-off-by: Vijay Balakrishna
Cc: sta...@vger.kernel.org
Reviewed-by: Pavel Tatashin
---
mm/khugepaged.c | 3 ++-
mm/page_alloc.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 4f7107476a6f..3c1147816d12 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -2283,7 +2283,8 @@ static void set_recommended_min_free_kbytes(void)
(unsigned long) nr_free_buffer_pages() / 20);
recommended_min <<= (PAGE_SHIFT-10);
- if (recommended_min > min_free_kbytes) {
+ if (recommended_min > min_free_kbytes ||
+ recommended_min > user_min_free_kbytes) {
if (user_min_free_kbytes >= 0)
pr_info("raising min_free_kbytes from %d to %lu to help
transparent hugepage allocations\n",
min_free_kbytes, recommended_min);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index fab5e97dc9ca..7b81fb139034 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -315,7 +315,7 @@ compound_page_dtor * const
compound_page_dtors[NR_COMPOUND_DTORS] = {
};
int min_free_kbytes = 1024;
-int user_min_free_kbytes = -1;
+int user_min_free_kbytes = 0;
#ifdef CONFIG_DISCONTIGMEM
/*
* DiscontigMem defines memory ranges as separate pg_data_t even if the ranges
Re: [tip:x86/seves] BUILD SUCCESS WITH WARNING e6eb15c9ba3165698488ae5c34920eea20eaa38e
On 2020-09-16, 'Marco Elver' via Clang Built Linux wrote:
On Wed, 16 Sep 2020 at 20:22, 'Nick Desaulniers' via kasan-dev
wrote:
On Wed, Sep 16, 2020 at 1:46 AM Marco Elver wrote:
>
> On Wed, 16 Sep 2020 at 10:30, wrote:
> > On Tue, Sep 15, 2020 at 08:09:16PM +0200, Marco Elver wrote:
> > > On Tue, 15 Sep 2020 at 19:40, Nick Desaulniers
wrote:
> > > > On Tue, Sep 15, 2020 at 10:21 AM Borislav Petkov wrote:
> >
> > > > > init/calibrate.o: warning: objtool: asan.module_ctor()+0xc: call
without frame pointer save/setup
> > > > > init/calibrate.o: warning: objtool: asan.module_dtor()+0xc: call
without frame pointer save/setup
> > > > > init/version.o: warning: objtool: asan.module_ctor()+0xc: call
without frame pointer save/setup
> > > > > init/version.o: warning: objtool: asan.module_dtor()+0xc: call
without frame pointer save/setup
> > > > > certs/system_keyring.o: warning: objtool: asan.module_ctor()+0xc:
call without frame pointer save/setup
> > > > > certs/system_keyring.o: warning: objtool: asan.module_dtor()+0xc:
call without frame pointer save/setup
> > >
> > > This one also appears with Clang 11. This is new I think because we
> > > started emitting ASAN ctors for globals redzone initialization.
> > >
> > > I think we really do not care about precise stack frames in these
> > > compiler-generated functions. So, would it be reasonable to make
> > > objtool ignore all *san.module_ctor and *san.module_dtor functions (we
> > > have them for ASAN, TSAN, MSAN)?
> >
> > The thing is, if objtool cannot follow, it cannot generate ORC data and
> > our unwinder cannot unwind through the instrumentation, and that is a
> > fail.
> >
> > Or am I missing something here?
>
> They aren't about the actual instrumentation. The warnings are about
> module_ctor/module_dtor functions which are compiler-generated, and
> these are only called on initialization/destruction (dtors only for
> modules I guess).
>
> E.g. for KASAN it's the calls to __asan_register_globals that are
> called from asan.module_ctor. For KCSAN the tsan.module_ctor is
> effectively a noop (because __tsan_init() is a noop), so it really
> doesn't matter much.
>
> Is my assumption correct that the only effect would be if something
> called by them fails, we just don't see the full stack trace? I think
> we can live with that, there are only few central places that deal
> with ctors/dtors (do_ctors(), ...?).
>
> The "real" fix would be to teach the compilers about "frame pointer
> save/setup" for generated functions, but I don't think that's
> realistic.
So this has come up before, specifically in the context of gcov:
https://github.com/ClangBuiltLinux/linux/issues/955.
I looked into this a bit, and IIRC, the issue was that compiler
generated functions aren't very good about keeping track of whether
they should or should not emit framepointer setup/teardown
prolog/epilogs. In LLVM's IR, -fno-omit-frame-pointer gets attached
to every function as a function level attribute.
https://godbolt.org/z/fcn9c6 ("frame-pointer"="all").
There were some recent LLVM patches for BTI (arm64) that made some BTI
related command line flags module level attributes, which I thought
was interesting; I was wondering last night if -fno-omit-frame-pointer
and maybe even the level of stack protector should be? I guess LTO
would complicate things; not sure it would be good to merge modules
with different attributes; I'm not sure how that's handled today in
LLVM.
Basically, when the compiler is synthesizing a new function
definition, it should check whether a frame pointer should be emitted
or not. We could do that today by maybe scanning all other function
definitions for the presence of "frame-pointer"="all" fn attr,
breaking early if we find one, and emitting the frame pointer setup in
that case. Though I guess it's "frame-pointer"="none" otherwise, so
maybe checking any other fn def would be fine; I don't see any C fn
attr's that allow you to keep frame pointers or not. What's tricky is
that the front end flag was resolved much earlier than where this code
gets generated, so it would need to look for traces that the flag ever
existed, which sounds brittle on paper to me.
Thanks for the summary -- yeah, that was my suspicion, that some
attribute was being lost somewhere. And I think if we generalize this,
and don't just try to attach "frame-pointer" attr to the function, we
probably also solve the BTI issue that Mark still pointed out with
these module_ctor/dtors.
I was trying to see if there was a generic way to attach all the
common attributes to the function generated here:
https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Utils/ModuleUtils.cpp#L122
-- but we probably can't attach all attributes, and need to remove a
bunch of them again like the sanitizers (or alternatively just select
the ones we need). But, I'm still digging for the function that
attaches all the common attributes...
Thanks,
-- Marco
Speaking of gcov, do
Re: [PATCH v2 1/2] vfs: block chmod of symlinks
On Thu, Sep 17, 2020 at 05:07:15AM +0100, Al Viro wrote:
> On Wed, Sep 16, 2020 at 07:25:53AM +0100, Christoph Hellwig wrote:
> > On Tue, Sep 15, 2020 at 08:22:54PM -0400, Rich Felker wrote:
> > > It was discovered while implementing userspace emulation of fchmodat
> > > AT_SYMLINK_NOFOLLOW (using O_PATH and procfs magic symlinks; otherwise
> > > it's not possible to target symlinks with chmod operations) that some
> > > filesystems erroneously allow access mode of symlinks to be changed,
> > > but return failure with EOPNOTSUPP (see glibc issue #14578 and commit
> > > a492b1e5ef). This inconsistency is non-conforming and wrong, and the
> > > consensus seems to be that it was unintentional to allow link modes to
> > > be changed in the first place.
> > >
> > > Signed-off-by: Rich Felker
> > > ---
> > > fs/open.c | 6 ++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/fs/open.c b/fs/open.c
> > > index 9af548fb841b..cdb7964aaa6e 100644
> > > --- a/fs/open.c
> > > +++ b/fs/open.c
> > > @@ -570,6 +570,12 @@ int chmod_common(const struct path *path, umode_t
> > > mode)
> > > struct iattr newattrs;
> > > int error;
> > >
> > > + /* Block chmod from getting to fs layer. Ideally the fs would either
> > > + * allow it or fail with EOPNOTSUPP, but some are buggy and return
> > > + * an error but change the mode, which is non-conforming and wrong. */
> > > + if (S_ISLNK(inode->i_mode))
> > > + return -EOPNOTSUPP;
> >
> > Our usualy place for this would be setattr_prepare. Also the comment
> > style is off, and I don't think we should talk about buggy file systems
> > here, but a policy to not allow the chmod. I also suspect the right
> > error value is EINVAL - EOPNOTSUPP isn't really used in normal posix
> > file system interfaces.
>
> Er... Wasn't that an ACL-related crap? XFS calling posix_acl_chmod()
> after it has committed to i_mode change, propagating the error to
> caller of ->notify_change(), IIRC...
>
> Put it another way, why do we want
> if (!inode->i_op->set_acl)
> return -EOPNOTSUPP;
> in posix_acl_chmod(), when we have
> if (!IS_POSIXACL(inode))
> return 0;
> right next to it? If nothing else, make that
> if (!IS_POSIXACL(inode) || !inode->i_op->get_acl)
> return 0; // piss off - nothing to adjust here
Arrgh... That'd break shmem and similar filesystems... Still, it
feels like we should _not_ bother in cases when there's no ACL
for that sucker; after all, if get_acl() returns NULL, we quietly
return 0 and that's it.
How about something like this instead?
diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index 95882b3f5f62..2339160fabab 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -559,8 +559,6 @@ posix_acl_chmod(struct inode *inode, umode_t mode)
if (!IS_POSIXACL(inode))
return 0;
- if (!inode->i_op->set_acl)
- return -EOPNOTSUPP;
acl = get_acl(inode, ACL_TYPE_ACCESS);
if (IS_ERR_OR_NULL(acl)) {
@@ -569,6 +567,10 @@ posix_acl_chmod(struct inode *inode, umode_t mode)
return PTR_ERR(acl);
}
+ if (!inode->i_op->set_acl) {
+ posix_acl_release(acl);
+ return -EOPNOTSUPP;
+ }
ret = __posix_acl_chmod(, GFP_KERNEL, mode);
if (ret)
return ret;
RE: [PATCH V2 4/4] ARM: imx: cpuidle-imx7ulp: Stop mode disallowed when HSRUN
> From: Peng Fan
> Sent: Wednesday, September 16, 2020 10:49 AM
>
> When cpu runs in HSRUN mode, cpuidle is not allowed to run into Stop mode.
> So add imx7ulp_get_mode to get thr cpu run mode, and use WAIT mode
> instead, when cpu in HSRUN mode.
>
> Signed-off-by: Peng Fan
> ---
> arch/arm/mach-imx/common.h | 1 +
> arch/arm/mach-imx/cpuidle-imx7ulp.c | 14 +++---
> arch/arm/mach-imx/pm-imx7ulp.c | 10 ++
> 3 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
> index 72c3fcc32910..707ac650f1c2 100644
> --- a/arch/arm/mach-imx/common.h
> +++ b/arch/arm/mach-imx/common.h
> @@ -103,6 +103,7 @@ void imx6_set_int_mem_clk_lpm(bool enable); void
> imx6sl_set_wait_clk(bool enter); int imx_mmdc_get_ddr_type(void); int
> imx7ulp_set_lpm(enum ulp_cpu_pwr_mode mode);
> +u32 imx7ulp_get_mode(void);
>
> void imx_cpu_die(unsigned int cpu);
> int imx_cpu_kill(unsigned int cpu);
> diff --git a/arch/arm/mach-imx/cpuidle-imx7ulp.c
> b/arch/arm/mach-imx/cpuidle-imx7ulp.c
> index ca86c967d19e..e7009d10b331 100644
> --- a/arch/arm/mach-imx/cpuidle-imx7ulp.c
> +++ b/arch/arm/mach-imx/cpuidle-imx7ulp.c
> @@ -15,10 +15,18 @@
> static int imx7ulp_enter_wait(struct cpuidle_device *dev,
> struct cpuidle_driver *drv, int index) {
> - if (index == 1)
> + u32 mode;
> +
> + if (index == 1) {
> imx7ulp_set_lpm(ULP_PM_WAIT);
> - else
> - imx7ulp_set_lpm(ULP_PM_STOP);
> + } else {
> + mode = imx7ulp_get_mode();
> +
> + if (mode == 3)
Can we also put a comment above to ease the code reading?
Otherwise:
Reviewed-by: Dong Aisheng
Regards
Aisheng
> + imx7ulp_set_lpm(ULP_PM_WAIT);
> + else
> + imx7ulp_set_lpm(ULP_PM_STOP);
> + }
>
> cpu_do_idle();
>
> diff --git a/arch/arm/mach-imx/pm-imx7ulp.c
> b/arch/arm/mach-imx/pm-imx7ulp.c index 393faf1e8382..1410ccfc71bd
> 100644
> --- a/arch/arm/mach-imx/pm-imx7ulp.c
> +++ b/arch/arm/mach-imx/pm-imx7ulp.c
> @@ -63,6 +63,16 @@ int imx7ulp_set_lpm(enum ulp_cpu_pwr_mode mode)
> return 0;
> }
>
> +u32 imx7ulp_get_mode(void)
> +{
> + u32 mode;
> +
> + mode = readl_relaxed(smc1_base + SMC_PMCTRL) & BM_PMCTRL_RUNM;
> + mode >>= BP_PMCTRL_RUNM;
> +
> + return mode;
> +}
> +
> void __init imx7ulp_pm_init(void)
> {
> struct device_node *np;
> --
> 2.28.0
Re: [PATCH] bitfield.h: annotate type_replace_bits functions with __must_check
On 16-09-20, 16:33, Srinivas Kandagatla wrote:
>
>
> On 16/09/2020 16:20, Greg KH wrote:
> > On Wed, Sep 16, 2020 at 04:03:33PM +0100, Srinivas Kandagatla wrote:
> > > usage of apis like u32_replace_bits() without actually catching the return
> > > value could hide problems without any warning!
> > >
> > > Found this with recent usage of this api in SoundWire!
> > > Having __must_check annotation would really catch this issues in future!
> > >
> > > Signed-off-by: Srinivas Kandagatla
> > > ---
> > > include/linux/bitfield.h | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
> > > index 4e035aca6f7e..eb4f69253946 100644
> > > --- a/include/linux/bitfield.h
> > > +++ b/include/linux/bitfield.h
> > > @@ -131,7 +131,7 @@ static __always_inline __##type
> > > type##_encode_bits(base v, base field)\
> > > __field_overflow();
> > > \
> > > return to((v & field_mask(field)) * field_multiplier(field));
> > > \
> > > }
> > > \
> > > -static __always_inline __##type type##_replace_bits(__##type old,
> > > \
> > > +static __always_inline __must_check __##type
> > > type##_replace_bits(__##type old, \
> > > base val, base field)
> > > \
> > > {
> > > \
> > > return (old & ~to(field)) | type##_encode_bits(val, field);
> > > \
> > > --
> > > 2.21.0
> > >
> >
> > Don't add __must_check to things that if merged will instantly cause
> > build warnings to the system, that's just rude :(
> Currently there are not many users for this api, found only one instance in
> drivers/net/ipa/ipa_table.c which is also fixed with
> https://lkml.org/lkml/2020/9/10/1062
>
> >
> > Fix up everything first, and then try to make this type of change.
> >
> > But why does this function have to be checked?
> As this function would return updated value, this check is more to with
> using the return value!
>
> It is easy for someone to ignore this return value assuming that the new
> value is already updated! So this check should help!
>
> TBH, This is what happened when we(vkoul and me) tried use this api :-)
So the only user of this has been moved to *p_replace_bits(), looking
back I think we should remove *_replace_bits (no users atm) and
duplicate of *p_replace_bits(). If not adding this patch would be
sensible thing to do
Somehow I feel former is a better idea ;-)
Thanks
--
~Vinod
Re: [PATCH v2 1/2] vfs: block chmod of symlinks
On Wed, Sep 16, 2020 at 07:25:53AM +0100, Christoph Hellwig wrote: > On Tue, Sep 15, 2020 at 08:22:54PM -0400, Rich Felker wrote: > > It was discovered while implementing userspace emulation of fchmodat > > AT_SYMLINK_NOFOLLOW (using O_PATH and procfs magic symlinks; otherwise > > it's not possible to target symlinks with chmod operations) that some > > filesystems erroneously allow access mode of symlinks to be changed, > > but return failure with EOPNOTSUPP (see glibc issue #14578 and commit > > a492b1e5ef). This inconsistency is non-conforming and wrong, and the > > consensus seems to be that it was unintentional to allow link modes to > > be changed in the first place. > > > > Signed-off-by: Rich Felker > > --- > > fs/open.c | 6 ++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/fs/open.c b/fs/open.c > > index 9af548fb841b..cdb7964aaa6e 100644 > > --- a/fs/open.c > > +++ b/fs/open.c > > @@ -570,6 +570,12 @@ int chmod_common(const struct path *path, umode_t mode) > > struct iattr newattrs; > > int error; > > > > + /* Block chmod from getting to fs layer. Ideally the fs would either > > +* allow it or fail with EOPNOTSUPP, but some are buggy and return > > +* an error but change the mode, which is non-conforming and wrong. */ > > + if (S_ISLNK(inode->i_mode)) > > + return -EOPNOTSUPP; > > Our usualy place for this would be setattr_prepare. Also the comment > style is off, and I don't think we should talk about buggy file systems > here, but a policy to not allow the chmod. I also suspect the right > error value is EINVAL - EOPNOTSUPP isn't really used in normal posix > file system interfaces. Er... Wasn't that an ACL-related crap? XFS calling posix_acl_chmod() after it has committed to i_mode change, propagating the error to caller of ->notify_change(), IIRC... Put it another way, why do we want if (!inode->i_op->set_acl) return -EOPNOTSUPP; in posix_acl_chmod(), when we have if (!IS_POSIXACL(inode)) return 0; right next to it? If nothing else, make that if (!IS_POSIXACL(inode) || !inode->i_op->get_acl) return 0; // piss off - nothing to adjust here
RE: [PATCH V2 3/4] ARM: imx: imx7ulp: support HSRUN mode
> From: Peng Fan
> Sent: Wednesday, September 16, 2020 10:49 AM
>
> Configure PMPROT to let ARM core could run into HSRUN mode.
> In LDO-enabled mode, HSRUN mode is not allowed, so add a check before
> configure PMPROT.
>
> Signed-off-by: Peng Fan
> ---
> arch/arm/mach-imx/pm-imx7ulp.c | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/arch/arm/mach-imx/pm-imx7ulp.c
> b/arch/arm/mach-imx/pm-imx7ulp.c index 2e756d8191fa..393faf1e8382
> 100644
> --- a/arch/arm/mach-imx/pm-imx7ulp.c
> +++ b/arch/arm/mach-imx/pm-imx7ulp.c
> @@ -11,6 +11,10 @@
>
> #include "common.h"
>
> +#define PMC0_CTRL0x28
> +#define BM_CTRL_LDOENBIT(31)
> +
> +#define SMC_PMPROT 0x8
> #define SMC_PMCTRL 0x10
> #define BP_PMCTRL_PSTOPO16
> #define PSTOPO_PSTOP30x3
> @@ -25,7 +29,10 @@
> #define BM_PMCTRL_RUNM (3 << BP_PMCTRL_RUNM)
> #define BM_PMCTRL_STOPM (7 << BP_PMCTRL_STOPM)
>
> +#define BM_PMPROT_AHSRUN BIT(7)
> +
> static void __iomem *smc1_base;
> +static void __iomem *pmc0_base;
>
> int imx7ulp_set_lpm(enum ulp_cpu_pwr_mode mode) { @@ -65,5 +72,13
> @@ void __init imx7ulp_pm_init(void)
> of_node_put(np);
> WARN_ON(!smc1_base);
>
> + np = of_find_compatible_node(NULL, NULL, "fsl,imx7ulp-pmc0");
> + pmc0_base = of_iomap(np, 0);
> + WARN_ON(!pmc0_base);
> + of_node_put(np);
> +
> + if (!(readl_relaxed(pmc0_base + PMC0_CTRL) & BM_CTRL_LDOEN))
> + writel_relaxed(BM_PMPROT_AHSRUN, smc1_base + SMC_PMPROT);
When will HSRUN mode be enabled? E.g. RUNM=HSRUN
It seems RUNM will be cleared in the following imx7ulp_set_lpm().
Regards
Aisheng
> +
> imx7ulp_set_lpm(ULP_PM_RUN);
> }
> --
> 2.28.0
Re: [PATCH] hinic: fix potential resource leak
On 2020/9/17 11:03, Wei Li wrote: > + err = irq_set_affinity_hint(rq->irq, >affinity_mask); > + if (err) > + goto err_irq; > + > + return 0; > + > +err_irq: > + rx_del_napi(rxq); > + return err; If irq_set_affinity_hint fails, irq should be freed as well.
Re: [PATCH] hinic: fix potential resource leak
Hi luobin, On 2020/9/17 11:44, luobin (L) wrote: > On 2020/9/17 11:03, Wei Li wrote: >> +err = irq_set_affinity_hint(rq->irq, >affinity_mask); >> +if (err) >> +goto err_irq; >> + >> +return 0; >> + >> +err_irq: >> +rx_del_napi(rxq); >> +return err; > If irq_set_affinity_hint fails, irq should be freed as well. > Yes, indeed. I will fix that in the next spin. Thanks, Wei
[PATCH v1] atomisp:pci/runtime/queue: modify the return error value
modify the return error value is -EDOM
Fixes: 2cac05dee6e30("drm/amd/powerplay: add the hw manager for vega12 (v4)")
Cc: Evan Quan
Signed-off-by: Xiaoliang Pang
---
.../staging/media/atomisp/pci/runtime/queue/src/queue_access.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/runtime/queue/src/queue_access.c
b/drivers/staging/media/atomisp/pci/runtime/queue/src/queue_access.c
index fdca743c4ab7..424e7a15a389 100644
--- a/drivers/staging/media/atomisp/pci/runtime/queue/src/queue_access.c
+++ b/drivers/staging/media/atomisp/pci/runtime/queue/src/queue_access.c
@@ -44,7 +44,7 @@ int ia_css_queue_load(
the value as zero. This causes division by 0
exception as the size is used in a modular
division operation. */
- return EDOM;
+ return -EDOM;
}
}
--
2.17.1
[PATCH v1] powerplay:hwmgr - modify the return value
modify the return value is -EINVAL
Fixes: f83a9991648bb("drm/amd/powerplay: add Vega10 powerplay support (v5)")
Fixes: 2cac05dee6e30("drm/amd/powerplay: add the hw manager for vega12 (v4)")
Cc: Eric Huang
Cc: Evan Quan
Signed-off-by: Xiaoliang Pang
---
drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 2 +-
drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
index c378a000c934..7eada3098ffc 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
@@ -4659,7 +4659,7 @@ static int
vega10_display_configuration_changed_task(struct pp_hwmgr *hwmgr)
if ((data->water_marks_bitmap & WaterMarksExist) &&
!(data->water_marks_bitmap & WaterMarksLoaded)) {
result = smum_smc_table_manager(hwmgr, (uint8_t *)wm_table,
WMTABLE, false);
- PP_ASSERT_WITH_CODE(result, "Failed to update WMTABLE!", return
EINVAL);
+ PP_ASSERT_WITH_CODE(result, "Failed to update WMTABLE!", return
-EINVAL);
data->water_marks_bitmap |= WaterMarksLoaded;
}
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
index a678a67f1c0d..04da52cea824 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
@@ -2390,7 +2390,7 @@ static int
vega12_display_configuration_changed_task(struct pp_hwmgr *hwmgr)
!(data->water_marks_bitmap & WaterMarksLoaded)) {
result = smum_smc_table_manager(hwmgr,
(uint8_t *)wm_table,
TABLE_WATERMARKS, false);
- PP_ASSERT_WITH_CODE(result, "Failed to update WMTABLE!", return
EINVAL);
+ PP_ASSERT_WITH_CODE(result, "Failed to update WMTABLE!", return
-EINVAL);
data->water_marks_bitmap |= WaterMarksLoaded;
}
--
2.17.1
[PATCH 2/2] vfio/pci: Remove bardirty from vfio_pci_device
It isn't clear what purpose the @bardirty serves. It might be used to avoid
the unnecessary vfio_bar_fixup() invoking on a user-space BAR read, which
is not required when bardirty is unset.
The variable was introduced in commit 89e1f7d4c66d ("vfio: Add PCI device
driver") but never actually used, so it shouldn't be that important. Remove
it.
Signed-off-by: Zenghui Yu
---
drivers/vfio/pci/vfio_pci_config.c | 7 ---
drivers/vfio/pci/vfio_pci_private.h | 1 -
2 files changed, 8 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_config.c
b/drivers/vfio/pci/vfio_pci_config.c
index d98843feddce..e93b287fea02 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -507,8 +507,6 @@ static void vfio_bar_fixup(struct vfio_pci_device *vdev)
*vbar &= cpu_to_le32((u32)mask);
} else
*vbar = 0;
-
- vdev->bardirty = false;
}
static int vfio_basic_config_read(struct vfio_pci_device *vdev, int pos,
@@ -633,9 +631,6 @@ static int vfio_basic_config_write(struct vfio_pci_device
*vdev, int pos,
}
}
- if (is_bar(offset))
- vdev->bardirty = true;
-
return count;
}
@@ -1697,8 +1692,6 @@ int vfio_config_init(struct vfio_pci_device *vdev)
if (ret)
goto out;
- vdev->bardirty = true;
-
/*
* XXX can we just pci_load_saved_state/pci_restore_state?
* may need to rebuild vconfig after that
diff --git a/drivers/vfio/pci/vfio_pci_private.h
b/drivers/vfio/pci/vfio_pci_private.h
index 61ca8ab165dc..dc96a0fda548 100644
--- a/drivers/vfio/pci/vfio_pci_private.h
+++ b/drivers/vfio/pci/vfio_pci_private.h
@@ -122,7 +122,6 @@ struct vfio_pci_device {
boolvirq_disabled;
boolreset_works;
boolextended_caps;
- boolbardirty;
boolhas_vga;
boolneeds_reset;
boolnointx;
--
2.19.1
[PATCH 1/2] vfio/pci: Remove redundant declaration of vfio_pci_driver
It was added by commit 137e5531351d ("vfio/pci: Add sriov_configure
support") and actually unnecessary. Remove it.
Signed-off-by: Zenghui Yu
---
drivers/vfio/pci/vfio_pci.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 1ab1f5cda4ac..da68e2f86622 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -1862,7 +1862,6 @@ static const struct vfio_device_ops vfio_pci_ops = {
static int vfio_pci_reflck_attach(struct vfio_pci_device *vdev);
static void vfio_pci_reflck_put(struct vfio_pci_reflck *reflck);
-static struct pci_driver vfio_pci_driver;
static int vfio_pci_bus_notifier(struct notifier_block *nb,
unsigned long action, void *data)
--
2.19.1
Re: [PATCH v4 1/4] genirq: define an empty function set_handle_irq() if !GENERIC_IRQ_MULTI_HANDLER
On 2020/9/15 16:43, Zhen Lei wrote:
> To avoid compilation error if an irqchip driver references the function
> set_handle_irq() but may not select GENERIC_IRQ_MULTI_HANDLER on some
> systems.
Hi, Marc:
Do you agree with this method?
Otherwise, I should use "#ifdef CONFIG_GENERIC_IRQ_MULTI_HANDLER ... #endif"
to perform the compilation isolation. This may make the code less beautiful.
>
> For example, the Synopsys DesignWare APB interrupt controller
> (dw_apb_ictl) is used as the secondary interrupt controller on arc, csky,
> arm64, and most arm32 SoCs, and it's also used as the primary interrupt
> controller on Hisilicon SD5203 (an arm32 SoC). The latter need to use
> set_handle_irq() to register the top-level IRQ handler, but this multi
> irq handler registration mechanism is not implemented on arc system.
>
> The input parameter "handle_irq" maybe defined as static and only
> set_handle_irq() references it. This will trigger "defined but not used"
> warning. So add "(void)handle_irq" to suppress it.
>
> Signed-off-by: Zhen Lei
> ---
> include/linux/irq.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 1b7f4dfee35b397..0848a2aaa9b40b1 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -1252,6 +1252,8 @@ void irq_matrix_free(struct irq_matrix *m, unsigned int
> cpu,
> * top-level IRQ handler.
> */
> extern void (*handle_arch_irq)(struct pt_regs *) __ro_after_init;
> +#else
> +#define set_handle_irq(handle_irq) do { (void)handle_irq; } while (0)
> #endif
>
> #endif /* _LINUX_IRQ_H */
>
Re: [PATCH 2/2] arm64/mm: Enable color zero pages
Hi Will, On 9/16/20 6:28 PM, Will Deacon wrote: On Wed, Sep 16, 2020 at 01:25:23PM +1000, Gavin Shan wrote: This enables color zero pages by allocating contigous page frames for it. The number of pages for this is determined by L1 dCache (or iCache) size, which is probbed from the hardware. * Add cache_total_size() to return L1 dCache (or iCache) size * Implement setup_zero_pages(), which is called after the page allocator begins to work, to allocate the contigous pages needed by color zero page. * Reworked ZERO_PAGE() and define __HAVE_COLOR_ZERO_PAGE. Signed-off-by: Gavin Shan --- arch/arm64/include/asm/cache.h | 22 arch/arm64/include/asm/pgtable.h | 9 ++-- arch/arm64/kernel/cacheinfo.c| 34 +++ arch/arm64/mm/init.c | 35 arch/arm64/mm/mmu.c | 7 --- 5 files changed, 98 insertions(+), 9 deletions(-) diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h index a4d1b5f771f6..420e9dde2c51 100644 --- a/arch/arm64/include/asm/cache.h +++ b/arch/arm64/include/asm/cache.h @@ -39,6 +39,27 @@ #define CLIDR_LOC(clidr) (((clidr) >> CLIDR_LOC_SHIFT) & 0x7) #define CLIDR_LOUIS(clidr)(((clidr) >> CLIDR_LOUIS_SHIFT) & 0x7) +#define CSSELR_TND_SHIFT 4 +#define CSSELR_TND_MASK(UL(1) << CSSELR_TND_SHIFT) +#define CSSELR_LEVEL_SHIFT 1 +#define CSSELR_LEVEL_MASK (UL(7) << CSSELR_LEVEL_SHIFT) +#define CSSELR_IND_SHIFT 0 +#define CSSERL_IND_MASK(UL(1) << CSSELR_IND_SHIFT) + +#define CCSIDR_64_LS_SHIFT 0 +#define CCSIDR_64_LS_MASK (UL(7) << CCSIDR_64_LS_SHIFT) +#define CCSIDR_64_ASSOC_SHIFT 3 +#define CCSIDR_64_ASSOC_MASK (UL(0x1F) << CCSIDR_64_ASSOC_SHIFT) +#define CCSIDR_64_SET_SHIFT32 +#define CCSIDR_64_SET_MASK (UL(0xFF) << CCSIDR_64_SET_SHIFT) + +#define CCSIDR_32_LS_SHIFT 0 +#define CCSIDR_32_LS_MASK (UL(7) << CCSIDR_32_LS_SHIFT) +#define CCSIDR_32_ASSOC_SHIFT 3 +#define CCSIDR_32_ASSOC_MASK (UL(0x3FF) << CCSIDR_32_ASSOC_SHIFT) +#define CCSIDR_32_SET_SHIFT13 +#define CCSIDR_32_SET_MASK (UL(0x7FFF) << CCSIDR_32_SET_SHIFT) I don't think we should be inferring cache structure from these register values. The Arm ARM helpfully says: | You cannot make any inference about the actual sizes of caches based | on these parameters. so we need to take the topology information from elsewhere. Yeah, I also noticed the statement in the spec. However, the L1 cache size figured out from above registers are matching with "lscpu" on the machine where I did my tests. Note "lscpu" depends on sysfs entries whose information is retrieved from ACPI (PPTT) table. The number of cache levels are partially retrieved from system register (clidr_el1). It's doable to retrieve the L1 cache size from ACPI (PPTT) table. I'll change accordingly in v2 if this enablement is really needed. More clarify is provided below. But before we get into that, can you justify why we need to do this at all, please? Do you have data to show the benefit of adding this complexity? Initially, I found it's the missed feature which has been enabled on mips/s390. Currently, all read-only anonymous VMAs are backed up by same zero page. It means all reads to these VMAs are cached by same set of cache, but still multiple ways if supported. So it would be nice to have multiple zero pages to back up these read-only anonymous VMAs, so that the reads on them can be cached by multiple sets (multiple ways still if supported). It's overall beneficial to the performance. Unfortunately, I didn't find a machine where the size of cache set is larger than page size. So I had one experiment as indication how L1 data cache miss affects the overall performance: L1 data cache size: 32KB L1 data cache line size: 64 Number of L1 data cache set: 64 Number of L1 data cache ways: 8 -- size = (cache_line_size) * (num_of_sets) * (num_of_ways) Kernel configuration: VA_BITS: 48 PAGE_SIZE: 4KB PMD HugeTLB Page Size: 2MB Experiment: I have a program to do the following things and check the consumed time and L1-data-cache-misses by perf. (1) Allocate (mmap) a PMD HugeTLB Page, which is 2MB. (2) Read on the mmap'd region in step of page size (4KB) for 8 or 9 times. Note 8 is the number of data cache ways. (3) Repeat (2) for 100 times. Result: (a) when we have 8 for the steps in (2): 37,103 L1-dcache-load-misses 0.217522515 seconds time elapsed 0.217564000 seconds user 0.0 seconds sys (b) when we have 9 for the
LUCRATIVE PROJECT VENTURE
I am an Independent Financial Consultant, I have a reputable client who is seeking for an experienced individual or company that can profitably invest $51,000.000 United State Dollars privately for TPI. Your swift response is highly needed. The modalities is 100% risk free Best Regard Nicola Samir Wahiba
[v3 PATCH] drm/mediatek: dsi: fix scrolling of panel with small hfp or hbp
Replace horizontal_backporch_byte with vm->hback_porch * bpp to aovid
flowing judgement negative number.
if ((vm->hfront_porch * dsi_tmp_buf_bpp + horizontal_backporch_byte) >
data_phy_cycles * dsi->lanes + delta)
Signed-off-by: Jitao Shi
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 54 ++
1 file changed, 19 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 16fd99dcdacf..f69ebeaf 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -445,6 +445,7 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
u32 horizontal_backporch_byte;
u32 horizontal_frontporch_byte;
u32 dsi_tmp_buf_bpp, data_phy_cycles;
+ u32 delta;
struct mtk_phy_timing *timing = >phy_timing;
struct videomode *vm = >vm;
@@ -475,42 +476,25 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
data_phy_cycles = timing->lpx + timing->da_hs_prepare +
timing->da_hs_zero + timing->da_hs_exit + 3;
- if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
- if ((vm->hfront_porch + vm->hback_porch) * dsi_tmp_buf_bpp >
- data_phy_cycles * dsi->lanes + 18) {
- horizontal_frontporch_byte =
- vm->hfront_porch * dsi_tmp_buf_bpp -
- (data_phy_cycles * dsi->lanes + 18) *
- vm->hfront_porch /
- (vm->hfront_porch + vm->hback_porch);
-
- horizontal_backporch_byte =
- horizontal_backporch_byte -
- (data_phy_cycles * dsi->lanes + 18) *
- vm->hback_porch /
- (vm->hfront_porch + vm->hback_porch);
- } else {
- DRM_WARN("HFP less than d-phy, FPS will under 60Hz\n");
- horizontal_frontporch_byte = vm->hfront_porch *
-dsi_tmp_buf_bpp;
- }
+ delta = (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) ? 18 : 12;
+
+ if ((vm->hfront_porch * dsi_tmp_buf_bpp + horizontal_backporch_byte) >
+ data_phy_cycles * dsi->lanes + delta) {
+ horizontal_frontporch_byte =
+ vm->hfront_porch * dsi_tmp_buf_bpp -
+ (data_phy_cycles * dsi->lanes + delta) *
+ vm->hfront_porch /
+ (vm->hfront_porch + vm->hback_porch);
+
+ horizontal_backporch_byte =
+ horizontal_backporch_byte -
+ (data_phy_cycles * dsi->lanes + delta) *
+ vm->hback_porch /
+ (vm->hfront_porch + vm->hback_porch);
} else {
- if ((vm->hfront_porch + vm->hback_porch) * dsi_tmp_buf_bpp >
- data_phy_cycles * dsi->lanes + 12) {
- horizontal_frontporch_byte =
- vm->hfront_porch * dsi_tmp_buf_bpp -
- (data_phy_cycles * dsi->lanes + 12) *
- vm->hfront_porch /
- (vm->hfront_porch + vm->hback_porch);
- horizontal_backporch_byte = horizontal_backporch_byte -
- (data_phy_cycles * dsi->lanes + 12) *
- vm->hback_porch /
- (vm->hfront_porch + vm->hback_porch);
- } else {
- DRM_WARN("HFP less than d-phy, FPS will under 60Hz\n");
- horizontal_frontporch_byte = vm->hfront_porch *
-dsi_tmp_buf_bpp;
- }
+ DRM_WARN("HFP + HBP less than d-phy, FPS will under 60Hz\n");
+ horizontal_frontporch_byte = vm->hfront_porch *
+dsi_tmp_buf_bpp;
}
writel(horizontal_sync_active_byte, dsi->regs + DSI_HSA_WC);
--
2.12.5
Re: [PATCH 1/2] dt-bindings: interrupt-controller: add Hisilicon SD5203 vector interrupt controller
On 2020/9/15 14:12, Leizhen (ThunderTown) wrote:
>
>
> On 2020/9/15 4:31, Rob Herring wrote:
>> On Thu, Sep 03, 2020 at 08:05:03PM +0800, Zhen Lei wrote:
>>> Add DT bindings for the Hisilicon SD5203 vector interrupt controller.
>>>
>>> Signed-off-by: Zhen Lei
>>> ---
>>> .../hisilicon,sd5203-vic.txt | 27 +++
>>
>> Bindings should be in DT schema format now.
Do I need to change the existing "snps,dw-apb-ictl.txt" to DT schema format?
>
> Hi, Rob Herring:
>
> As Marc Zyngier's suggestion, I discarded adding an independent SD5203-VIC
> driver, but make the dw-apb-ictl irqchip driver to support hierarchy irq
> domain.
> So this new file was also dropped. Now, I updated the descriptions in the
> existing
> file "snps,dw-apb-ictl.txt" in the following versions.
>
>>
>>> 1 file changed, 27 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt
>>>
>>> b/Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt
>>> new file mode 100644
>>> index ..a08292e868b0
>>> --- /dev/null
>>> +++
>>> b/Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt
>>> @@ -0,0 +1,27 @@
>>> +Hisilicon SD5203 vector interrupt controller (VIC)
>>> +
>>> +Hisilicon SD5203 VIC based on Synopsys DesignWare APB interrupt
>>> controller, but
>>> +there's something special:
>>> +1. The maximum number of irqs supported is 32. The registers ENABLE, MASK
>>> and
>>> + FINALSTATUS are 32 bits.
>>> +2. There is only one VIC, it's used as primary interrupt controller.
>>> +
>>> +Required properties:
>>> +- compatible: shall be "hisilicon,sd5203-vic"
>>> +- reg: physical base address of the controller and length of memory mapped
>>> + region starting with ENABLE_LOW register
>>> +- interrupt-controller: identifies the node as an interrupt controller
>>> +- #interrupt-cells: number of cells to encode an interrupt-specifier,
>>> shall be 1
>>> +
>>> +The interrupt sources map to the corresponding bits in the interrupt
>>> +registers, i.e.
>>> +- 0 maps to bit 0 of low interrupts,
>>> +- 1 maps to bit 1 of low interrupts,
>>> +
>>> +Example:
>>> + vic: interrupt-controller@1013 {
>>> + compatible = "hisilicon,sd5203-vic";
>>> + reg = <0x1013 0x1000>;
>>> + interrupt-controller;
>>> + #interrupt-cells = <1>;
>>> + };
>>> --
>>> 2.26.0.106.g9fadedd
>>>
>>>
>>
>> .
>>
[PATCH RFC 6/8] thermal: Modify thermal governors to do nothing for trip points being monitored for falling temperature
For now, thermal governors other than step wise governorr do not support
monitoring of falling temperature. Hence, in case of calls to the governor
for trip points marked as THERMAL_TRIP_MONITOR_FALLING, return doing
nothing.
Signed-off-by: Thara Gopinath
---
drivers/thermal/gov_bang_bang.c | 12
drivers/thermal/gov_fair_share.c | 12
drivers/thermal/gov_power_allocator.c | 12
3 files changed, 36 insertions(+)
diff --git a/drivers/thermal/gov_bang_bang.c b/drivers/thermal/gov_bang_bang.c
index 991a1c54296d..a662047e5961 100644
--- a/drivers/thermal/gov_bang_bang.c
+++ b/drivers/thermal/gov_bang_bang.c
@@ -99,6 +99,18 @@ static void thermal_zone_trip_update(struct
thermal_zone_device *tz, int trip)
static int bang_bang_control(struct thermal_zone_device *tz, int trip)
{
struct thermal_instance *instance;
+ enum thermal_trip_monitor_type monitor_type =
+ THERMAL_TRIP_MONITOR_RISING;
+
+ /*
+* Return doing nothing if the trip point is monitored for
+* falling temperature
+*/
+ if (tz->ops->get_trip_mon_type) {
+ tz->ops->get_trip_mon_type(tz, trip, _type);
+ if (monitor_type == THERMAL_TRIP_MONITOR_FALLING)
+ return 0;
+ }
thermal_zone_trip_update(tz, trip);
diff --git a/drivers/thermal/gov_fair_share.c b/drivers/thermal/gov_fair_share.c
index aaa07180ab48..064ad6ed67ad 100644
--- a/drivers/thermal/gov_fair_share.c
+++ b/drivers/thermal/gov_fair_share.c
@@ -81,6 +81,18 @@ static int fair_share_throttle(struct thermal_zone_device
*tz, int trip)
int total_weight = 0;
int total_instance = 0;
int cur_trip_level = get_trip_level(tz);
+ enum thermal_trip_monitor_type monitor_type =
+ THERMAL_TRIP_MONITOR_RISING;
+
+ /*
+* Return doing nothing if the trip point is monitored for
+* falling temperature
+*/
+ if (tz->ops->get_trip_mon_type) {
+ tz->ops->get_trip_mon_type(tz, trip, _type);
+ if (monitor_type == THERMAL_TRIP_MONITOR_FALLING)
+ return 0;
+ }
list_for_each_entry(instance, >thermal_instances, tz_node) {
if (instance->trip != trip)
diff --git a/drivers/thermal/gov_power_allocator.c
b/drivers/thermal/gov_power_allocator.c
index 5cb518d8f156..0f674cd1b9b8 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -606,6 +606,8 @@ static int power_allocator_throttle(struct
thermal_zone_device *tz, int trip)
{
int ret;
int switch_on_temp, control_temp;
+ enum thermal_trip_monitor_type monitor_type =
+ THERMAL_TRIP_MONITOR_RISING;
struct power_allocator_params *params = tz->governor_data;
/*
@@ -615,6 +617,16 @@ static int power_allocator_throttle(struct
thermal_zone_device *tz, int trip)
if (trip != params->trip_max_desired_temperature)
return 0;
+ /*
+* Return doing nothing if the trip point is monitored for
+* falling temperature
+*/
+ if (tz->ops->get_trip_mon_type) {
+ tz->ops->get_trip_mon_type(tz, trip, _type);
+ if (monitor_type == THERMAL_TRIP_MONITOR_FALLING)
+ return 0;
+ }
+
ret = tz->ops->get_trip_temp(tz, params->trip_switch_on,
_on_temp);
if (!ret && (tz->temperature < switch_on_temp)) {
--
2.25.1
[PATCH RFC 2/8] thermal: Introduce new property monitor_type for trip point.
Thermal trip points can be defined to indicate whether a
temperature rise or a temperature fall is to be monitored. This
property can now be defined in the DT bindings for a trip point.
To support this following three changes are introduced to thermal
core and sysfs code.
1. Define a new variable in thermal_trip to capture the monitor
rising/falling information from trip point DT bindings.
2. Define a new ops in thermal_zone_device_ops that can be populated
to indicate whether a trip is being monitored for rising or falling
temperature. If the ops is not populated or if the binding is missing
in the DT, it is assumed that the trip is being monitored for rising
temperature. (default behavior today)
Signed-off-by: Thara Gopinath
---
drivers/thermal/thermal_core.h | 2 ++
include/linux/thermal.h| 2 ++
include/uapi/linux/thermal.h | 5 +
3 files changed, 9 insertions(+)
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index e00fc5585ea8..c56addfe2284 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -77,12 +77,14 @@ int power_actor_set_power(struct thermal_cooling_device
*cdev,
* @temperature: temperature value in miliCelsius
* @hysteresis: relative hysteresis in miliCelsius
* @type: trip point type
+ * @monitor_type: trip point monitor type
*/
struct thermal_trip {
struct device_node *np;
int temperature;
int hysteresis;
enum thermal_trip_type type;
+ enum thermal_trip_monitor_type monitor_type;
};
int get_tz_trend(struct thermal_zone_device *tz, int trip);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 42ef807e5d84..a50ed958d0bd 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -72,6 +72,8 @@ struct thermal_zone_device_ops {
int (*set_trip_temp) (struct thermal_zone_device *, int, int);
int (*get_trip_hyst) (struct thermal_zone_device *, int, int *);
int (*set_trip_hyst) (struct thermal_zone_device *, int, int);
+ int (*get_trip_mon_type)(struct thermal_zone_device *, int,
+enum thermal_trip_monitor_type *);
int (*get_crit_temp) (struct thermal_zone_device *, int *);
int (*set_emul_temp) (struct thermal_zone_device *, int);
int (*get_trend) (struct thermal_zone_device *, int,
diff --git a/include/uapi/linux/thermal.h b/include/uapi/linux/thermal.h
index c105054cbb57..d3bb4e4fad69 100644
--- a/include/uapi/linux/thermal.h
+++ b/include/uapi/linux/thermal.h
@@ -16,6 +16,11 @@ enum thermal_trip_type {
THERMAL_TRIP_CRITICAL,
};
+enum thermal_trip_monitor_type {
+ THERMAL_TRIP_MONITOR_RISING = 0,
+ THERMAL_TRIP_MONITOR_FALLING
+};
+
/* Adding event notification support elements */
#define THERMAL_GENL_FAMILY_NAME "thermal"
#define THERMAL_GENL_VERSION 0x01
--
2.25.1
[PATCH RFC 7/8] thermal:core: Add is_warming_dev and supporting warming device api's to the cooling dev framework.
Introduce a variable is_warming_dev to indicate if a "cooling" device is
actually a warming device or not. Also introduce api's to register and
unregister warming device.
This is a temporary patch. If we agree to replace the term "cooling" with
"mitigating" (or any other appropriate term) in the thermal framework, this
patch can be dropped. Also I have not added warming_device_register api for
all the versions of cooling_device_register because
devm_thermal_of_warming_device_register is the only api needed in the
kernel today as is evident by the next patch in this series.
Signed-off-by: Thara Gopinath
---
drivers/thermal/thermal_core.c | 88 +-
include/linux/thermal.h| 7 +++
2 files changed, 83 insertions(+), 12 deletions(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index bfd436379408..4aae48a80e00 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1101,7 +1101,8 @@ static void bind_cdev(struct thermal_cooling_device *cdev)
*/
static struct thermal_cooling_device *
__thermal_cooling_device_register(struct device_node *np,
- const char *type, void *devdata,
+ const char *type, bool is_warming_dev,
+ void *devdata,
const struct thermal_cooling_device_ops *ops)
{
struct thermal_cooling_device *cdev;
@@ -1134,6 +1135,7 @@ __thermal_cooling_device_register(struct device_node *np,
cdev->updated = false;
cdev->device.class = _class;
cdev->devdata = devdata;
+ cdev->is_warming_dev = is_warming_dev;
thermal_cooling_device_setup_sysfs(cdev);
dev_set_name(>device, "cooling_device%d", cdev->id);
result = device_register(>device);
@@ -1178,7 +1180,7 @@ struct thermal_cooling_device *
thermal_cooling_device_register(const char *type, void *devdata,
const struct thermal_cooling_device_ops *ops)
{
- return __thermal_cooling_device_register(NULL, type, devdata, ops);
+ return __thermal_cooling_device_register(NULL, type, false, devdata,
ops);
}
EXPORT_SYMBOL_GPL(thermal_cooling_device_register);
@@ -1202,7 +1204,7 @@ thermal_of_cooling_device_register(struct device_node *np,
const char *type, void *devdata,
const struct thermal_cooling_device_ops *ops)
{
- return __thermal_cooling_device_register(np, type, devdata, ops);
+ return __thermal_cooling_device_register(np, type, false, devdata, ops);
}
EXPORT_SYMBOL_GPL(thermal_of_cooling_device_register);
@@ -1242,7 +1244,7 @@ devm_thermal_of_cooling_device_register(struct device
*dev,
if (!ptr)
return ERR_PTR(-ENOMEM);
- tcd = __thermal_cooling_device_register(np, type, devdata, ops);
+ tcd = __thermal_cooling_device_register(np, type, false, devdata, ops);
if (IS_ERR(tcd)) {
devres_free(ptr);
return tcd;
@@ -1255,6 +1257,49 @@ devm_thermal_of_cooling_device_register(struct device
*dev,
}
EXPORT_SYMBOL_GPL(devm_thermal_of_cooling_device_register);
+/**
+ * devm_thermal_of_warming_device_register() - register an OF thermal warming
+ *device
+ * @dev: a valid struct device pointer of a sensor device.
+ * @np:a pointer to a device tree node.
+ * @type: the thermal cooling device type.
+ * @devdata: device private data.
+ * @ops: standard thermal cooling devices callbacks.
+ *
+ * This function will register a warming device with device tree node
reference.
+ * This interface function adds a new thermal warming device
(fan/processor/...)
+ * to /sys/class/thermal/ folder as cooling_device[0-*]. It tries to bind
itself
+ * to all the thermal zone devices registered at the same time.
+ *
+ * Return: a pointer to the created struct thermal_cooling_device or an
+ * ERR_PTR. Caller must check return value with IS_ERR*() helpers.
+ */
+struct thermal_cooling_device *
+devm_thermal_of_warming_device_register(struct device *dev,
+ struct device_node *np,
+ char *type, void *devdata,
+ const struct thermal_cooling_device_ops
*ops)
+{
+ struct thermal_cooling_device **ptr, *tcd;
+
+ ptr = devres_alloc(thermal_cooling_device_release, sizeof(*ptr),
+ GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ tcd = __thermal_cooling_device_register(np, type, true, devdata, ops);
+ if (IS_ERR(tcd)) {
+ devres_free(ptr);
+ return tcd;
+ }
+
+ *ptr = tcd;
+ devres_add(dev, ptr);
+
+ return tcd;
+}
+EXPORT_SYMBOL_GPL(devm_thermal_of_warming_device_register);
+
[PATCH RFC 5/8] thermal: gov_step_wise: Extend thermal step-wise governor to monitor falling temperature.
>From the step wise governor point of view, the policy decisions
that has to taken on a thermal trip point that is defined to be monitored
for falling temperature is the mirror opposite of the decisions it has
to take on a trip point that is monitored for rising temperature.
Signed-off-by: Thara Gopinath
---
drivers/thermal/gov_step_wise.c | 62 +
1 file changed, 47 insertions(+), 15 deletions(-)
diff --git a/drivers/thermal/gov_step_wise.c b/drivers/thermal/gov_step_wise.c
index 2ae7198d3067..c036ff7b4fb2 100644
--- a/drivers/thermal/gov_step_wise.c
+++ b/drivers/thermal/gov_step_wise.c
@@ -35,7 +35,8 @@
* deactivate the thermal instance
*/
static unsigned long get_target_state(struct thermal_instance *instance,
- enum thermal_trend trend, bool throttle)
+ enum thermal_trend trend, bool throttle,
+ enum thermal_trip_monitor_type type)
{
struct thermal_cooling_device *cdev = instance->cdev;
unsigned long cur_state;
@@ -65,11 +66,24 @@ static unsigned long get_target_state(struct
thermal_instance *instance,
switch (trend) {
case THERMAL_TREND_RAISING:
- if (throttle) {
- next_target = cur_state < instance->upper ?
- (cur_state + 1) : instance->upper;
- if (next_target < instance->lower)
- next_target = instance->lower;
+ if (type == THERMAL_TRIP_MONITOR_FALLING) {
+ if (cur_state <= instance->lower) {
+ if (!throttle)
+ next_target = THERMAL_NO_TARGET;
+ } else {
+ if (!throttle) {
+ next_target = cur_state - 1;
+ if (next_target > instance->upper)
+ next_target = instance->upper;
+ }
+ }
+ } else {
+ if (throttle) {
+ next_target = cur_state < instance->upper ?
+ (cur_state + 1) : instance->upper;
+ if (next_target < instance->lower)
+ next_target = instance->lower;
+ }
}
break;
case THERMAL_TREND_RAISE_FULL:
@@ -77,14 +91,23 @@ static unsigned long get_target_state(struct
thermal_instance *instance,
next_target = instance->upper;
break;
case THERMAL_TREND_DROPPING:
- if (cur_state <= instance->lower) {
- if (!throttle)
- next_target = THERMAL_NO_TARGET;
+ if (type == THERMAL_TRIP_MONITOR_FALLING) {
+ if (throttle) {
+ next_target = cur_state < instance->upper ?
+ (cur_state + 1) : instance->upper;
+ if (next_target < instance->lower)
+ next_target = instance->lower;
+ }
} else {
- if (!throttle) {
- next_target = cur_state - 1;
- if (next_target > instance->upper)
- next_target = instance->upper;
+ if (cur_state <= instance->lower) {
+ if (!throttle)
+ next_target = THERMAL_NO_TARGET;
+ } else {
+ if (!throttle) {
+ next_target = cur_state - 1;
+ if (next_target > instance->upper)
+ next_target = instance->upper;
+ }
}
}
break;
@@ -117,6 +140,8 @@ static void thermal_zone_trip_update(struct
thermal_zone_device *tz, int trip)
{
int trip_temp;
enum thermal_trip_type trip_type;
+ enum thermal_trip_monitor_type monitor_type =
+ THERMAL_TRIP_MONITOR_RISING;
enum thermal_trend trend;
struct thermal_instance *instance;
bool throttle = false;
@@ -130,9 +155,15 @@ static void thermal_zone_trip_update(struct
thermal_zone_device *tz, int trip)
tz->ops->get_trip_type(tz, trip, _type);
}
+ if (tz->ops->get_trip_mon_type)
+ tz->ops->get_trip_mon_type(tz, trip, _type);
+
trend = get_tz_trend(tz, trip);
- if (tz->temperature >= trip_temp) {
+
[PATCH RFC 4/8] thermal:core:Add genetlink notifications for monitoring falling temperature
Add notification calls for trip points that are being monitored for
falling temperatures.
Signed-off-by: Thara Gopinath
---
drivers/thermal/thermal_core.c | 25 +++--
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 72bf159bcecc..bfd436379408 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -417,6 +417,7 @@ static void handle_critical_trips(struct
thermal_zone_device *tz,
static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
{
enum thermal_trip_type type;
+ enum thermal_trip_monitor_type mon_type;
int trip_temp, hyst = 0;
/* Ignore disabled trip points */
@@ -428,13 +429,25 @@ static void handle_thermal_trip(struct
thermal_zone_device *tz, int trip)
if (tz->ops->get_trip_hyst)
tz->ops->get_trip_hyst(tz, trip, );
+ if (tz->ops->get_trip_mon_type)
+ tz->ops->get_trip_mon_type(tz, trip, _type);
+
if (tz->last_temperature != THERMAL_TEMP_INVALID) {
- if (tz->last_temperature < trip_temp &&
- tz->temperature >= trip_temp)
- thermal_notify_tz_trip_up(tz->id, trip);
- if (tz->last_temperature >= trip_temp &&
- tz->temperature < (trip_temp - hyst))
- thermal_notify_tz_trip_down(tz->id, trip);
+ if (mon_type == THERMAL_TRIP_MONITOR_FALLING) {
+ if (tz->last_temperature > trip_temp &&
+ tz->temperature <= trip_temp)
+ thermal_notify_tz_trip_down(tz->id, trip);
+ if (tz->last_temperature <= trip_temp &&
+ tz->temperature > (trip_temp + hyst))
+ thermal_notify_tz_trip_up(tz->id, trip);
+ } else {
+ if (tz->last_temperature < trip_temp &&
+ tz->temperature >= trip_temp)
+ thermal_notify_tz_trip_up(tz->id, trip);
+ if (tz->last_temperature >= trip_temp &&
+ tz->temperature < (trip_temp - hyst))
+ thermal_notify_tz_trip_down(tz->id, trip);
+ }
}
if (type == THERMAL_TRIP_CRITICAL || type == THERMAL_TRIP_HOT)
--
2.25.1
[PATCH RFC 8/8] soc:qcom:qcom_aoss: Change cooling_device_register to warming_device_register
Always on subsystem host resources cx and ebi that are used as warming devices. Use the newly introduce _warming_device_register to register these devices with the thermal framework. Signed-off-by: Thara Gopinath --- drivers/soc/qcom/qcom_aoss.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c index ed2c687c16b3..4f65c03a5def 100644 --- a/drivers/soc/qcom/qcom_aoss.c +++ b/drivers/soc/qcom/qcom_aoss.c @@ -461,7 +461,7 @@ static int qmp_cooling_device_add(struct qmp *qmp, qmp_cdev->qmp = qmp; qmp_cdev->state = !qmp_cdev_max_state; qmp_cdev->name = cdev_name; - qmp_cdev->cdev = devm_thermal_of_cooling_device_register + qmp_cdev->cdev = devm_thermal_of_warming_device_register (qmp->dev, node, cdev_name, qmp_cdev, _cooling_device_ops); @@ -501,7 +501,7 @@ static int qmp_cooling_devices_register(struct qmp *qmp) unroll: while (--count >= 0) - thermal_cooling_device_unregister + thermal_warming_device_unregister (qmp->cooling_devs[count].cdev); return ret; @@ -512,7 +512,7 @@ static void qmp_cooling_devices_remove(struct qmp *qmp) int i; for (i = 0; i < QMP_NUM_COOLING_RESOURCES; i++) - thermal_cooling_device_unregister(qmp->cooling_devs[i].cdev); + thermal_warming_device_unregister(qmp->cooling_devs[i].cdev); } static int qmp_probe(struct platform_device *pdev) -- 2.25.1
[PATCH RFC 1/8] dt-bindings: thermal: Introduce monitor-falling parameter to thermal trip point binding
Introduce a new binding parameter to thermal trip point description to indicate whether the temperature level specified by the trip point is monitored for a rise or fall in temperature. Signed-off-by: Thara Gopinath --- .../devicetree/bindings/thermal/thermal-zones.yaml | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml index 3ec9cc87ec50..cc1332ad6c16 100644 --- a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml +++ b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml @@ -161,6 +161,13 @@ patternProperties: The active trip type can be used to control other HW to help in cooling e.g. fans can be sped up or slowed down + monitor-falling: +description: | + boolean, If true, the trip point is being monitored for + falling temperature. If false/absent/default, the trip + point is being monitored for rising temperature. +type: boolean + required: - temperature - hysteresis -- 2.25.1
[PATCH RFC 3/8] thermal: thermal_of: Extend thermal dt driver to support bi-directional monitoring of a thermal trip point.
Introduce of_thermal_get_trip_monitor_type to return the direction
of monitoring of a thermal trip point. Also translate the DT information
regarding trip point monitor direction into the thermal framework.
Signed-off-by: Thara Gopinath
---
drivers/thermal/thermal_of.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index 69ef12f852b7..5bc7f5bab772 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -328,6 +328,20 @@ static int of_thermal_get_trip_hyst(struct
thermal_zone_device *tz, int trip,
return 0;
}
+static int of_thermal_get_trip_monitor_type
+ (struct thermal_zone_device *tz, int trip,
+enum thermal_trip_monitor_type *type)
+{
+ struct __thermal_zone *data = tz->devdata;
+
+ if (trip >= data->ntrips || trip < 0)
+ return -EDOM;
+
+ *type = data->trips[trip].monitor_type;
+
+ return 0;
+}
+
static int of_thermal_set_trip_hyst(struct thermal_zone_device *tz, int trip,
int hyst)
{
@@ -363,6 +377,7 @@ static struct thermal_zone_device_ops of_thermal_ops = {
.set_trip_temp = of_thermal_set_trip_temp,
.get_trip_hyst = of_thermal_get_trip_hyst,
.set_trip_hyst = of_thermal_set_trip_hyst,
+ .get_trip_mon_type = of_thermal_get_trip_monitor_type,
.get_crit_temp = of_thermal_get_crit_temp,
.bind = of_thermal_bind,
@@ -801,6 +816,7 @@ static int thermal_of_populate_trip(struct device_node *np,
{
int prop;
int ret;
+ bool is_monitor_falling;
ret = of_property_read_u32(np, "temperature", );
if (ret < 0) {
@@ -822,6 +838,12 @@ static int thermal_of_populate_trip(struct device_node *np,
return ret;
}
+ ret = of_property_read_bool(np, "monitor-falling");
+ if (is_monitor_falling)
+ trip->monitor_type = THERMAL_TRIP_MONITOR_FALLING;
+ else
+ trip->monitor_type = THERMAL_TRIP_MONITOR_RISING;
+
/* Required for cooling map matching */
trip->np = np;
of_node_get(np);
--
2.25.1
[PATCH RFC 0/8] Introduce warming in thermal framework
Thermal framework today supports monitoring for rising temperatures and subsequently initiating cooling action in case of a thermal trip point being crossed. There are scenarios where a SoC need warming mitigating action to be activated if the temperature falls below a cetain permissible limit. Since warming action can be considered mirror opposite of cooling action, most of the thermal framework can be re-used to achieve this. The key assumption in this patch series is that a device can act either as a warming device or a cooling device and not as both. In order to support warming three extensions are needed in the thermal framework. 1. Indication that a trip point is being monitored for falling temperature and not rising temperature. We discussed two different ways to achieve this during LPC. First option is to introduce a new trip type to indicate that a trip is a cold trip(THERMAL_TRIP_COLD). The second option is to introduce a new property for trip point that will indicate whether a trip point is being monitored for rising temperature or falling temperature. The patch series(patches 1-4) chooses the second approach since it allows trip points of any type to be monitored for rising or falling temperature.Also this was the preferred approach when discussed during LPC. The approach that introduces a new cold trip type was posted on the list earlier as a RFC and can be found at [1]. 2. Extend the exisitng governors to handle monitoring of falling temperature. The patch series(patches 5 & 6) extends the step wise governor to monitor the falling temperature.Other governors return doing nothing if the trip point they are being called for is being monitored for falling temperature. The governors' mitigate function is called "throttle" in the thermal framework and with this patch series it is a misnomer as the function is called for both throttling and warming up. Ideally "throttle" should be renamed to "mitigate" to improve readability of code. The renaming is not part of this series. 3. Finally, the cooling device framework itself can be reused for a warming device. As stated before a device can act either as a warming device or a cooling device and not as both. With this the cooling state in the framework can be considered as mitigating state with 0 as the state with no thermal mitigation and higher the number higher the thermal mitigation. Again what affects the code readability and comprehension is the term "cooling" which is a misnomer here. Ideally the term "cooling" should be renamed to "mitigating" and hence thermal_cooling_device will become thermal_mitgating_device. The renaming is not part of the patch series as even though the renaming is a simple search-replace, it will change a lot of files. The patch series(patches 7 & 8) instead introduces a minimal set of _warming_device_ apis to register and unregister warming devices which internally is identical to the _cooling_device_ counterpart. 1. https://lkml.org/lkml/2020/7/10/639 Thara Gopinath (8): dt-bindings: thermal: Introduce monitor-falling parameter to thermal trip point binding thermal: Introduce new property monitor_type for trip point. thermal: thermal_of: Extend thermal dt driver to support bi-directional monitoring of a thermal trip point. thermal:core:Add genetlink notifications for monitoring falling temperature thermal: gov_step_wise: Extend thermal step-wise governor to monitor falling temperature. thermal: Modify thermal governors to do nothing for trip points being monitored for falling temperature thermal:core: Add is_warming_dev and supporting warming device api's to the cooling dev framework. soc:qcom:qcom_aoss: Change cooling_device_register to warming_device_register .../bindings/thermal/thermal-zones.yaml | 7 ++ drivers/soc/qcom/qcom_aoss.c | 6 +- drivers/thermal/gov_bang_bang.c | 12 ++ drivers/thermal/gov_fair_share.c | 12 ++ drivers/thermal/gov_power_allocator.c | 12 ++ drivers/thermal/gov_step_wise.c | 62 +++--- drivers/thermal/thermal_core.c| 113 +++--- drivers/thermal/thermal_core.h| 2 + drivers/thermal/thermal_of.c | 22 include/linux/thermal.h | 9 ++ include/uapi/linux/thermal.h | 5 + 11 files changed, 226 insertions(+), 36 deletions(-) -- 2.25.1
Re: [PATCH v2 2/2] ARM: support PHYS_OFFSET minimum aligned at 64KiB boundary
On 2020/9/16 15:57, Russell King - ARM Linux admin wrote: > On Wed, Sep 16, 2020 at 09:57:15AM +0800, Leizhen (ThunderTown) wrote: >> On 2020/9/16 3:01, Russell King - ARM Linux admin wrote: >>> On Tue, Sep 15, 2020 at 09:16:15PM +0800, Zhen Lei wrote: Currently, only support the kernels where the base of physical memory is at a 16MiB boundary. Because the add/sub instructions only contains 8bits unrotated value. But we can use one more "add/sub" instructions to handle bits 23-16. The performance will be slightly affected. Since most boards meet 16 MiB alignment, so add a new configuration option ARM_PATCH_PHYS_VIRT_RADICAL (default n) to control it. Say Y if anyone really needs it. All r0-r7 (r1 = machine no, r2 = atags or dtb, in the start-up phase) are used in __fixup_a_pv_table() now, but the callee saved r11 is not used in the whole head.S file. So choose it. Because the calculation of "y = x + __pv_offset[63:24]" have been done, so we only need to calculate "y = y + __pv_offset[23:16]", that's why the parameters "to" and "from" of __pv_stub() and __pv_add_carry_stub() in the scope of CONFIG_ARM_PATCH_PHYS_VIRT_RADICAL are all passed "t" (above y). Signed-off-by: Zhen Lei --- arch/arm/Kconfig | 18 +- arch/arm/include/asm/memory.h | 16 +--- arch/arm/kernel/head.S| 25 +++-- 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index e00d94b16658765..19fc2c746e2ce29 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -240,12 +240,28 @@ config ARM_PATCH_PHYS_VIRT kernel in system memory. This can only be used with non-XIP MMU kernels where the base -of physical memory is at a 16MB boundary. +of physical memory is at a 16MiB boundary. Only disable this option if you know that you do not require this feature (eg, building a kernel for a single machine) and you need to shrink the kernel to the minimal size. +config ARM_PATCH_PHYS_VIRT_RADICAL + bool "Support PHYS_OFFSET minimum aligned at 64KiB boundary" + default n >>> >>> Please drop the "default n" - this is the default anyway. >> >> OK, I will remove it. >> >>> @@ -236,6 +243,9 @@ static inline unsigned long __phys_to_virt(phys_addr_t x) * in place where 'r' 32 bit operand is expected. */ __pv_stub((unsigned long) x, t, "sub", __PV_BITS_31_24); +#ifdef CONFIG_ARM_PATCH_PHYS_VIRT_RADICAL + __pv_stub((unsigned long) t, t, "sub", __PV_BITS_23_16); >>> >>> t is already unsigned long, so this cast is not necessary. >> >> Oh, yes, yes, I copied from the above statement, but forgot to remove it. >> >>> >>> I've been debating whether it would be better to use "movw" for this >>> for ARMv7. In other words: >>> >>> movwtmp, #16-bit >>> adds%Q0, %1, tmp, lsl #16 >>> adc %R0, %R0, #0 >>> >>> It would certainly be less instructions, but at the cost of an >>> additional register - and we'd have to change the fixup code to >>> know about movw. >> >> It's one less instruction for 64KiB boundary && (sizeof(phys_addr_t) == 8), >> and no increase or decrease for 64KiB boundary && (sizeof(phys_addr_t) == 4), >> but one more instruction for 16MiB boundary. >> >> And maybe: 16MiB is widely used, but 64KiB is rarely used. >> >> So I'm inclined to the current revision. > > Multiplatform kernels (which will be what distros build) will have to > enable this option if they wish to support this platform. So, in that > case it doesn't just impacting a single platform, but all platforms. I will try movw. But it may take a few days, because I feel that the changes will be a little big. >
linux-next: Signed-off-by missing for commit in the rcu tree
Hi all,
Commit
903c5302fa2d ("sched/core: Allow try_invoke_on_locked_down_task() with irqs
disabled")
is missing a Signed-off-by from its author and committer.
I didn't complain about this when it was first present because I figured
it was just a debugging commit that would be removed quickly. However,
there are now quite a few follow up commits ...
--
Cheers,
Stephen Rothwell
pgphV45UEgdpW.pgp
Description: OpenPGP digital signature
[PATCH] w1: Use kobj_to_dev() API
Use kobj_to_dev() instead of container_of()
Signed-off-by: Wang Qing
---
include/linux/w1.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/w1.h b/include/linux/w1.h
index cebf346..7f45174
--- a/include/linux/w1.h
+++ b/include/linux/w1.h
@@ -311,7 +311,7 @@ static inline struct w1_slave* dev_to_w1_slave(struct
device *dev)
static inline struct w1_slave* kobj_to_w1_slave(struct kobject *kobj)
{
- return dev_to_w1_slave(container_of(kobj, struct device, kobj));
+ return dev_to_w1_slave(kobj_to_dev(kobj););
}
static inline struct w1_master* dev_to_w1_master(struct device *dev)
--
2.7.4
Re: [PATCH 0/2] perf probe: Support debuginfod client
On Wed, 16 Sep 2020 16:17:53 -0400 "Frank Ch. Eigler" wrote: > Hi - > > > > Nice, even uses the source code fetching part of the webapi! > > > > So, can I take that as an Acked-by or Reviewed-by? > > Sure. Thanks Frank and Arnaldo! > > > I need to support this in pahole... > > pahole/dwarves use elfutils, so it already has automatic support. > > https://sourceware.org/elfutils/Debuginfod.html I'm still not sure that which interface of elfutils I should use for this "automatic" debuginfod support. Are there good documentation about it? Since this series just for the kernel binary, I have to check we can do something on user-space binaries. Thank you, -- Masami Hiramatsu
[PATCH] Revert "perf report: Treat an argument as a symbol filter"
Since commit 6db6127c4dad ("perf report: Treat an argument as a symbol
filter"), the only one unrecognized argument for perf-report is treated
as a symbol filter. This is not described in man page nor help info,
and the result is really confusing, especially when it's misspecified by
the user (e.g. missing -i for perf.data).
As we can use "--symbol-filter=" if we really want to filter a symbol,
it may be better to revert this misfeature.
Signed-off-by: Wei Li
---
tools/perf/builtin-report.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 3c74c9c0f3c3..f57ebc1bcd20 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -1317,13 +1317,9 @@ int cmd_report(int argc, const char **argv)
argc = parse_options(argc, argv, options, report_usage, 0);
if (argc) {
/*
-* Special case: if there's an argument left then assume that
-* it's a symbol filter:
+* Any (unrecognized) arguments left?
*/
- if (argc > 1)
- usage_with_options(report_usage, options);
-
- report.symbol_filter_str = argv[0];
+ usage_with_options(report_usage, options);
}
if (annotate_check_args(_opts) < 0)
--
2.17.1
Re: [PATCH v2 3/3] PCI: iproc: Display PCIe Link information
On Thu, Sep 17, 2020 at 7:22 AM Rob Herring wrote:
>
Hi Rob,
Thanks for review.
> On Tue, Sep 15, 2020 at 07:15:41PM +0530, Srinath Mannam wrote:
> > After successful linkup more comprehensive information about PCIe link
> > speed and link width will be displayed to the console.
> >
> > Signed-off-by: Srinath Mannam
> > ---
> > drivers/pci/controller/pcie-iproc.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/pci/controller/pcie-iproc.c
> > b/drivers/pci/controller/pcie-iproc.c
> > index cc5b7823edeb..8ef2d1fe392c 100644
> > --- a/drivers/pci/controller/pcie-iproc.c
> > +++ b/drivers/pci/controller/pcie-iproc.c
> > @@ -1479,6 +1479,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct
> > list_head *res)
> > {
> > struct device *dev;
> > int ret;
> > + struct pci_dev *pdev;
> > struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
> >
> > dev = pcie->dev;
> > @@ -1542,6 +1543,11 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct
> > list_head *res)
> > goto err_power_off_phy;
> > }
> >
> > + for_each_pci_bridge(pdev, host->bus) {
> > + if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)
> > + pcie_print_link_status(pdev);
> > + }
>
> If this information is useful for 1 host implementation, why not all of
> them and put this in a common spot.
In common, pcie_print_link_status() is called during pci device caps
initialization, if the available link bandwidth is less than capabilities
of devices. Few EP drivers also used this function to print link
bandwidth info. This host can be configured for different link
speeds and link widths on different platforms so we thought
displaying link bandwidth after successful linkup is helpful to
know link details.
Thanks & Regards,
Srinath.
>
> Rob
Re: [RFC] autonuma: Migrate on fault among multiple bound nodes
On Wed, Sep 16, 2020 at 05:29:41PM +0200, David Hildenbrand wrote: > On 16.09.20 15:39, Qian Cai wrote: > > On Wed, 2020-09-16 at 08:59 +0800, Huang Ying wrote: > >> static int apply_policy_zone(struct mempolicy *policy, enum zone_type > >> zone) > >> @@ -2474,11 +2481,13 @@ int mpol_misplaced(struct page *page, struct > >> vm_area_struct *vma, unsigned long > >>int thisnid = cpu_to_node(thiscpu); > >>int polnid = NUMA_NO_NODE; > >>int ret = -1; > >> + bool moron; > > > > Are you really going to use that name those days? > > > > > > include/uapi/linux/mempolicy.h:#define MPOL_F_MORON (1 << 4) /* > Migrate On protnone Reference On Node */ > > Not commenting the decision for that name. It's uapi ... and naming the > variable like the uapi flag seems to be a sane thing to do ... hmmm ... Perhaps we could migrate to mopron / MPOL_F_MOPRON?
[PATCH] i2c: Use kobj_to_dev() API
Use kobj_to_dev() instead of container_of()
Signed-off-by: Wang Qing
---
include/linux/i2c.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index fc55ea4..5662265
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -344,7 +344,7 @@ const struct i2c_device_id *i2c_match_id(const struct
i2c_device_id *id,
static inline struct i2c_client *kobj_to_i2c_client(struct kobject *kobj)
{
- struct device * const dev = container_of(kobj, struct device, kobj);
+ struct device * const dev = kobj_to_dev(kobj);
return to_i2c_client(dev);
}
--
2.7.4
Re: More filesystem need this fix (xfs: use MMAPLOCK around filemap_map_pages())
On Thu, Sep 17, 2020 at 11:44:54AM +1000, Dave Chinner wrote: > So > > P0p1 > > hole punch starts > takes XFS_MMAPLOCK_EXCL > truncate_pagecache_range() ... locks page ... > unmap_mapping_range(start, end) > > > do_fault_around() > ->map_pages > filemap_map_pages() ... trylock page fails ... > page mapping valid, > page is up to date > maps PTEs > > truncate_inode_pages_range() > truncate_cleanup_page(page) > invalidates page > delete_from_page_cache_batch(page) > frees page > > > That doesn't seem good to me. > > Sure, maybe the page hasn't been freed back to the free lists > because of elevated refcounts. But it's been released by the > filesystem and not longer in the page cache so nothing good can come > of this situation... > > AFAICT, this race condition exists for the truncate case as well > as filemap_map_pages() doesn't have a "page beyond inode size" check > in it. However, exposure here is very limited in the truncate case > because truncate_setsize()->truncate_pagecache() zaps the PTEs > again after invalidating the page cache. > > Either way, adding the XFS_MMAPLOCK_SHARED around > filemap_map_pages() avoids the race condition for fallocate and > truncate operations for XFS... > > > As such it is a rather > > different beast compared to the fault handler from fs POV and does not need > > protection from hole punching (current serialization on page lock and > > checking of page->mapping is enough). > > That being said I agree this is subtle and the moment someone adds e.g. a > > readahead call into filemap_map_pages() we have a real problem. I'm not > > sure how to prevent this risk... > > Subtle, yes. So subtle, in fact, I fail to see any reason why the > above race cannot occur as there's no obvious serialisation in the > page cache between PTE zapping and page invalidation to prevent a > fault from coming in an re-establishing the PTEs before invalidation > occurs. > > Cheers, > > Dave. > -- > Dave Chinner > da...@fromorbit.com >
[PATCH] hinic: fix potential resource leak
In rx_request_irq(), it will just return what irq_set_affinity_hint()
returns. If it is failed, the napi added is not deleted properly.
Add a common exit for failures to do this.
Signed-off-by: Wei Li
---
drivers/net/ethernet/huawei/hinic/hinic_rx.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_rx.c
b/drivers/net/ethernet/huawei/hinic/hinic_rx.c
index 5bee951fe9d4..63da9cc8ca51 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_rx.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_rx.c
@@ -543,18 +543,23 @@ static int rx_request_irq(struct hinic_rxq *rxq)
if (err) {
netif_err(nic_dev, drv, rxq->netdev,
"Failed to set RX interrupt coalescing attribute\n");
- rx_del_napi(rxq);
- return err;
+ goto err_irq;
}
err = request_irq(rq->irq, rx_irq, 0, rxq->irq_name, rxq);
- if (err) {
- rx_del_napi(rxq);
- return err;
- }
+ if (err)
+ goto err_irq;
cpumask_set_cpu(qp->q_id % num_online_cpus(), >affinity_mask);
- return irq_set_affinity_hint(rq->irq, >affinity_mask);
+ err = irq_set_affinity_hint(rq->irq, >affinity_mask);
+ if (err)
+ goto err_irq;
+
+ return 0;
+
+err_irq:
+ rx_del_napi(rxq);
+ return err;
}
static void rx_free_irq(struct hinic_rxq *rxq)
--
2.17.1
RE: [PATCH V2 2/4] ARM: dts: imx7ulp: add pmc node
> From: Peng Fan
> Sent: Wednesday, September 16, 2020 10:49 AM
>
> Add i.MX7ULP pmc node for m4 and a7.
>
> Signed-off-by: Peng Fan
> ---
> arch/arm/boot/dts/imx7ulp.dtsi | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx7ulp.dtsi b/arch/arm/boot/dts/imx7ulp.dtsi
> index b7ea37ad4e55..b02dc4c97fb8 100644
> --- a/arch/arm/boot/dts/imx7ulp.dtsi
> +++ b/arch/arm/boot/dts/imx7ulp.dtsi
> @@ -286,6 +286,11 @@ pcc2: clock-controller@403f {
> assigned-clock-parents = <
> IMX7ULP_CLK_SOSC_BUS_CLK>;
> };
>
> + pmc1: pmc1@4040 {
s/pmc1/pmc
> + compatible = "fsl,imx7ulp-pmc1";
> + reg = <0x4040 0x1000>;
> + };
> +
> smc1: clock-controller@4041 {
> compatible = "fsl,imx7ulp-smc1";
> reg = <0x4041 0x1000>;
> @@ -447,6 +452,11 @@ m4aips1: bus@4108 {
> reg = <0x4108 0x8>;
> ranges;
>
> + pmc0: pmc0@410a1000 {
s/pmc0/pmc
> + compatible = "fsl,imx7ulp-pmc0";
> + reg = <0x410a1000 0x1000>;
> + };
> +
> sim: sim@410a3000 {
> compatible = "fsl,imx7ulp-sim", "syscon";
> reg = <0x410a3000 0x1000>;
> --
> 2.28.0
RE: [PATCH V2 1/4] dt-bindings: fsl: add i.MX7ULP PMC
> From: Peng Fan > Sent: Wednesday, September 16, 2020 10:49 AM > > Update fsl,imx7ulp-pm.yaml to include pmc > > Signed-off-by: Peng Fan > --- > .../devicetree/bindings/arm/freescale/fsl,imx7ulp-pm.yaml | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git > a/Documentation/devicetree/bindings/arm/freescale/fsl,imx7ulp-pm.yaml > b/Documentation/devicetree/bindings/arm/freescale/fsl,imx7ulp-pm.yaml > index 3b26040f8f18..28ebaa8b1d1e 100644 > --- a/Documentation/devicetree/bindings/arm/freescale/fsl,imx7ulp-pm.yaml > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,imx7ulp-pm.yaml > @@ -23,7 +23,11 @@ description: | > > properties: >compatible: > -const: fsl,imx7ulp-smc1 > +items: > + - enum: > + - fsl,imx7ulp-smc1 > + - fsl,imx7ulp-pmc0 > + - fsl,imx7ulp-pmc1 Could you add Description for each of them? Regards Aisheng > >reg: > maxItems: 1 > -- > 2.28.0
[PATCH] perf metric: Code cleanup with map_for_each_event()
Since we have introduced map_for_each_event() to walk the 'pmu_events_map',
clean up metricgroup__print() and metricgroup__has_metric() with it.
Signed-off-by: Wei Li
---
tools/perf/util/metricgroup.c | 33 +
1 file changed, 13 insertions(+), 20 deletions(-)
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 8831b964288f..3734cbb2c456 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -26,6 +26,17 @@
#include "util.h"
#include
+#define map_for_each_event(__pe, __idx, __map) \
+ for (__idx = 0, __pe = &__map->table[__idx];\
+__pe->name || __pe->metric_group || __pe->metric_name; \
+__pe = &__map->table[++__idx])
+
+#define map_for_each_metric(__pe, __idx, __map, __metric) \
+ map_for_each_event(__pe, __idx, __map) \
+ if (__pe->metric_expr &&\
+ (match_metric(__pe->metric_group, __metric) || \
+match_metric(__pe->metric_name, __metric)))
+
struct metric_event *metricgroup__lookup(struct rblist *metric_events,
struct evsel *evsel,
bool create)
@@ -475,12 +486,9 @@ void metricgroup__print(bool metrics, bool metricgroups,
char *filter,
groups.node_new = mep_new;
groups.node_cmp = mep_cmp;
groups.node_delete = mep_delete;
- for (i = 0; ; i++) {
+ map_for_each_event(pe, i, map) {
const char *g;
- pe = >table[i];
- if (!pe->name && !pe->metric_group && !pe->metric_name)
- break;
if (!pe->metric_expr)
continue;
g = pe->metric_group;
@@ -745,17 +753,6 @@ static int __add_metric(struct list_head *metric_list,
return 0;
}
-#define map_for_each_event(__pe, __idx, __map) \
- for (__idx = 0, __pe = &__map->table[__idx];\
-__pe->name || __pe->metric_group || __pe->metric_name; \
-__pe = &__map->table[++__idx])
-
-#define map_for_each_metric(__pe, __idx, __map, __metric) \
- map_for_each_event(__pe, __idx, __map) \
- if (__pe->metric_expr &&\
- (match_metric(__pe->metric_group, __metric) || \
-match_metric(__pe->metric_name, __metric)))
-
static struct pmu_event *find_metric(const char *metric, struct pmu_events_map
*map)
{
struct pmu_event *pe;
@@ -1092,11 +1089,7 @@ bool metricgroup__has_metric(const char *metric)
if (!map)
return false;
- for (i = 0; ; i++) {
- pe = >table[i];
-
- if (!pe->name && !pe->metric_group && !pe->metric_name)
- break;
+ map_for_each_event(pe, i, map) {
if (!pe->metric_expr)
continue;
if (match_metric(pe->metric_name, metric))
--
2.17.1
RE: [PATCH] time: Avoid undefined behaviour in timespec64_to_ns
> -Original Message-
> From: Arnd Bergmann [mailto:a...@arndb.de]
> Sent: Tuesday, September 15, 2020 8:45 PM
> To: Zengtao (B)
> Cc: Thomas Gleixner; Vincenzo Frascino; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] time: Avoid undefined behaviour in
> timespec64_to_ns
>
> On Tue, Sep 15, 2020 at 2:20 PM Zengtao (B)
> wrote:
>
> > > > Fixes: bd40a175769d ("y2038: itimer: change implementation to
> > > timespec64")
> > >
> > > This one caused the regression, but if we add the check here, it
> > > may be best to also add it in prior kernels that may have the same
> > > bug in other callers of the same function. Maybe backport all the
> > > way to stable kernels that first added timespec64?
> > >
> >
> > I think we need to do the backport, but not sure about the start
> point
> > Thanks for your review.
>
> I would suggest
>
> Cc: # v3.17+
> Fixes: 361a3bf00582 ("time64: Add time64.h header and define
> struct timespec64")
Yes, make sense, commit 361a3bf00582 introduce a potential issue and
commit bd40a175769d trigger the issue.
Regards
Zengtao
>
>Arnd
Re: [PATCH v2 0/3] PCI: iproc: Add fixes to pcie iproc
On Thu, Sep 17, 2020 at 3:38 AM Bjorn Helgaas wrote: > Hi Bjorn, Thanks for review. > On Tue, Sep 15, 2020 at 07:15:38PM +0530, Srinath Mannam wrote: > > This patch series contains fixes and improvements to pcie iproc driver. > > > > This patch set is based on Linux-5.9.0-rc2. > > > > Changes from v1: > > - Addressed Bjorn's review comments > > - pcie_print_link_status is used to print Link information. > > - Added IARR1/IMAP1 window map definition. > > > > Bharat Gooty (1): > > PCI: iproc: fix out of bound array access > > > > Roman Bacik (1): > > PCI: iproc: fix invalidating PAXB address mapping > > You didn't update thest subject lines so they match. > https://lore.kernel.org/r/20200326194810.ga11...@google.com Yes this patchset is the latest version to the patches in the link. My apologies that I missed to address your review comment about the commit message of "PCI: iproc: fix out of bound array access" in this patchset. Thanks & Regards, Srinath. > > > Srinath Mannam (1): > > PCI: iproc: Display PCIe Link information > > > > drivers/pci/controller/pcie-iproc.c | 29 ++--- > > 1 file changed, 22 insertions(+), 7 deletions(-) > > > > -- > > 2.17.1 > >
linux-next: manual merge of the net-next tree with the net tree
Hi all,
Today's linux-next merge of the net-next tree got a conflict in:
net/ipv4/route.c
between commit:
2fbc6e89b2f1 ("ipv4: Update exception handling for multipath routes via same
device")
from the net tree and commit:
8b4510d76cde ("net: gain ipv4 mtu when mtu is not locked")
from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc net/ipv4/route.c
index 58642b29a499,2c05b863ae43..
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@@ -1015,10 -1013,9 +1015,10 @@@ out: kfree_skb(skb)
static void __ip_rt_update_pmtu(struct rtable *rt, struct flowi4 *fl4, u32
mtu)
{
struct dst_entry *dst = >dst;
+ struct net *net = dev_net(dst->dev);
- u32 old_mtu = ipv4_mtu(dst);
struct fib_result res;
bool lock = false;
+ u32 old_mtu;
if (ip_mtu_locked(dst))
return;
pgpZojHF_yccX.pgp
Description: OpenPGP digital signature
Re: [PATCH v18 00/32] per memcg lru_lock: reviews
在 2020/9/16 上午12:58, Daniel Jordan 写道: > On Tue, Sep 15, 2020 at 01:21:56AM -0700, Hugh Dickins wrote: >> On Sun, 13 Sep 2020, Alex Shi wrote: >>> Uh, I updated the testing with some new results here: >>> https://lkml.org/lkml/2020/8/26/212 >> Right, I missed that, that's better, thanks. Any other test results? > Alex, you were doing some will-it-scale runs earlier. Are you planning to do > more of those? Otherwise I can add them in. Hi Daniel, Does compaction perf scalable, like thpscale, I except they could get some benefit. Thanks Alex
Re: [RFC PATCH 0/2] Remove shrinker's nr_deferred
On Wed, Sep 16, 2020 at 11:58:21AM -0700, Yang Shi wrote: > > Recently huge amount one-off slab drop was seen on some vfs metadata heavy > workloads, > it turned out there were huge amount accumulated nr_deferred objects seen by > the > shrinker. > > I managed to reproduce this problem with kernel build workload plus negative > dentry > generator. > > First step, run the below kernel build test script: > > NR_CPUS=`cat /proc/cpuinfo | grep -e processor | wc -l` > > cd /root/Buildarea/linux-stable > > for i in `seq 1500`; do > cgcreate -g memory:kern_build > echo 4G > /sys/fs/cgroup/memory/kern_build/memory.limit_in_bytes > > echo 3 > /proc/sys/vm/drop_caches > cgexec -g memory:kern_build make clean > /dev/null 2>&1 > cgexec -g memory:kern_build make -j$NR_CPUS > /dev/null 2>&1 > > cgdelete -g memory:kern_build > done > > That would generate huge amount deferred objects due to __GFP_NOFS > allocations. > > Then run the below negative dentry generator script: > > NR_CPUS=`cat /proc/cpuinfo | grep -e processor | wc -l` > > mkdir /sys/fs/cgroup/memory/test > echo $$ > /sys/fs/cgroup/memory/test/tasks > > for i in `seq $NR_CPUS`; do > while true; do > FILE=`head /dev/urandom | tr -dc A-Za-z0-9 | head -c 64` > cat $FILE 2>/dev/null > done & > done > > Then kswapd will shrink half of dentry cache in just one loop as the below > tracing result > showed: > > kswapd0-475 [028] 305968.252561: mm_shrink_slab_start: > super_cache_scan+0x0/0x190 24acf00c: nid: 0 > objects to shrink 4994376020 gfp_flags GFP_KERNEL cache items 93689873 delta > 45746 total_scan 46844936 priority 12 > kswapd0-475 [021] 306013.099399: mm_shrink_slab_end: > super_cache_scan+0x0/0x190 24acf00c: nid: 0 unused > scan count 4994376020 new scan count 4947576838 total_scan 8 last shrinker > return val 46844928 You have 93M dentries and inodes in the cache, and the reclaim delta is 45746, which is totally sane for a priority 12 reclaim priority. SO you've basically had to do a couple of million GFP_NOFS direct reclaim passes that were unable to reclaim anything to get to a point where the deferred reclaim would up to 4.9 -billion- objects. Basically, you would up the deferred work so far that it got out of control before a GFP_KERNEL reclaim context could do anything to bring it under control. However, removing defered work is not the solution. If we don't defer some of this reclaim work, then filesystem intensive workloads -cannot reclaim memory from their own caches- when they need memory. And when those caches largely dominate the used memory in the machine, this will grind the filesystem workload to a halt.. Hence this deferral mechanism is actually critical to keeping the filesystem caches balanced with the rest of the system. The behaviour you see is the windup clamping code triggering: /* * We need to avoid excessive windup on filesystem shrinkers * due to large numbers of GFP_NOFS allocations causing the * shrinkers to return -1 all the time. This results in a large * nr being built up so when a shrink that can do some work * comes along it empties the entire cache due to nr >>> * freeable. This is bad for sustaining a working set in * memory. * * Hence only allow the shrinker to scan the entire cache when * a large delta change is calculated directly. */ if (delta < freeable / 4) total_scan = min(total_scan, freeable / 2); It clamps the worst case freeing to half the cache, and that is exactly what you are seeing. This, unfortunately, won't be enough to fix the windup problem once it's spiralled out of control. It's fairly rare for this to happen - it takes effort to find an adverse workload that will cause windup like this. So, with all that said, a year ago I actually fixed this problem as part of some work I did to provide non-blocking inode reclaim infrastructure in the shrinker for XFS inode reclaim. See this patch: https://lore.kernel.org/linux-xfs/20191031234618.15403-13-da...@fromorbit.com/ It did two things. First it ensured all the deferred work was done by kswapd so that some poor direct reclaim victim didn't hit a massive reclaim latency spike because of windup. Secondly, it clamped the maximum windup to the maximum single pass reclaim scan limit, which is (freeable * 2) objects. Finally it also changed the amount of deferred work a single kswapd pass did to be directly proportional to the reclaim priority. Hence as we get closer to OOM, kswapd tries much harder to get the deferred work backlog down to zero. This means that a single, low priority reclaim pass will never reclaim half the cache - only sustained memory pressure and _reclaim priority windup_ will do that. You probably want to look at all the shrinker infrastructure
RE: [EXT] Re: [PATCH v6 3/3] net: dsa: ocelot: Add support for QinQ Operation
> On Wed, Sep 16, 2020 at 10:28:38AM +, Hongbo Wang wrote: > > Hi Vladimir, > > > > if swp0 connects with customer, and swp1 connects with ISP, According > > to the VSC99599_1_00_TS.pdf, swp0 and swp1 will have different > > VLAN_POP_CNT && VLAN_AWARE_ENA, > > > > swp0 should set VLAN_CFG.VLAN_POP_CNT=0 && > VLAN_CFG.VLAN_AWARE_ENA=0 > > swp1 should set VLAN_CFG.VLAN_POP_CNT=1 && > VLAN_CFG.VLAN_AWARE_ENA=1 > > > > but when set vlan_filter=1, current code will set same value for both > > swp0 and swp1, for compatibility with existing code(802.1Q mode), so > > add devlink to set swp0 and swp1 into different modes. > > But if you make VLAN_CFG.VLAN_AWARE_ENA=0, does that mean the switch > will accept any 802.1ad VLAN, not only those configured in the VLAN database > of the bridge? Otherwise said, after running the commands above, and I send a > packet to swp0 having tpid:88A8 vid:101, then the bridge should not accept it. > > I might be wrong, but I thought that an 802.1ad bridge with > vlan_filtering=1 behaves the same as an 802.1q bridge, except that it should > filter VLANs using a different TPID (0x88a8 instead of 0x8100). > I don't think the driver, in the way you're configuring it, does that, does > it? hi Vladimir, you can refer to "4.3.3.0.1 MAN Access Switch Example" in VSC99599_1_00_TS.pdf, By testing the case, if don't set VLAN_AWARE_ENA=0 for customer's port swp0, the Q-in-Q feature can't work well. In order to distinguish the port for customer and for ISP, I add devlink command, Actually, I can modify the driver config directly, not using devlink, but it will be not compatible with current code and user guide. Thanks, hongbo
Re: [PATCH v7] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver
Hi, Rob sir: Sorry to bother you, may I have your review comment for the binding part? Appreciated. On Wed, 2020-09-16 at 19:39 +0800, Hector Yuan wrote: > Hi, Rob sir: > > Sorry to bother you, may I have your review comment for the binding > part? > Appreciated. > > On Thu, 2020-09-10 at 11:04 +0530, Viresh Kumar wrote: > > On 10-09-20, 13:30, Hector Yuan wrote: > > > On Thu, 2020-09-10 at 10:33 +0530, Viresh Kumar wrote: > > > > On 10-09-20, 12:31, Hector Yuan wrote: > > > > > The CPUfreq HW present in some Mediatek chipsets offloads the steps > > > > > necessary for changing the frequency of CPUs. > > > > > The driver implements the cpufreq driver interface for this hardware > > > > > engine. > > > > > > > > > > This patch depends on the MT6779 DTS patch submitted by Hanks Chen > > > > > https://lkml.org/lkml/2020/8/4/1094 > > > > > > > > Thanks for hanging there. Looks good to me. I will apply it once Rob > > > > Ack's the binding patch. > > > > > > > > > > Many thanks for your help. May I know if you can add Reviewed-by tag to > > > this patch set. > > > > Since this patchset is going to get merged via my tree (ARM cpufreq > > tree), a reviewed-by isn't required here. I will queue it up for > > 5.10-rc1 after I receive an Ack from Rob. > > > > > I would like to prepare some patches for more features > > > based on this. Is that okay to you? Thanks again. > > > > That should be fine. > > > >
[PATCH] scsi: qedf: remove redundant assignment to variable 'rc'
This assignment is meaningless, so remove it.
Signed-off-by: Jing Xiangfeng
---
drivers/scsi/qedf/qedf_io.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/scsi/qedf/qedf_io.c b/drivers/scsi/qedf/qedf_io.c
index acd9774a9387..c04078283121 100644
--- a/drivers/scsi/qedf/qedf_io.c
+++ b/drivers/scsi/qedf/qedf_io.c
@@ -2159,7 +2159,6 @@ int qedf_initiate_cleanup(struct qedf_ioreq *io_req,
/* Sanity check qedf_rport before dereferencing any pointers */
if (!test_bit(QEDF_RPORT_SESSION_READY, >flags)) {
QEDF_ERR(NULL, "tgt not offloaded\n");
- rc = 1;
return SUCCESS;
}
--
2.17.1
[PATCH RESEND v3 4/5] media: uvcvideo: Protect uvc queue file operations against disconnect
uvc queue file operations have no mutex protection against USB
disconnect. This is questionable at least for the poll operation,
which has to wait for the uvc queue mutex. By the time that mutex
has been acquired, is it possible that the video device has been
unregistered.
Protect all file operations by using the queue mutex to avoid
possible race conditions. After acquiring the mutex, check if
the video device is still registered, and bail out if not.
Cc: Laurent Pinchart
Cc: Alan Stern
Cc: Hans Verkuil
Signed-off-by: Guenter Roeck
---
drivers/media/usb/uvc/uvc_queue.c | 32 +--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_queue.c
b/drivers/media/usb/uvc/uvc_queue.c
index cd60c6c1749e..b2c59ce38008 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -358,24 +358,52 @@ int uvc_queue_streamoff(struct uvc_video_queue *queue,
enum v4l2_buf_type type)
int uvc_queue_mmap(struct uvc_video_queue *queue, struct vm_area_struct *vma)
{
- return vb2_mmap(>queue, vma);
+ struct uvc_streaming *stream = uvc_queue_to_stream(queue);
+ int ret;
+
+ mutex_lock(>mutex);
+ if (!video_is_registered(>vdev)) {
+ ret = -ENODEV;
+ goto unlock;
+ }
+ ret = vb2_mmap(>queue, vma);
+unlock:
+ mutex_unlock(>mutex);
+ return ret;
}
#ifndef CONFIG_MMU
unsigned long uvc_queue_get_unmapped_area(struct uvc_video_queue *queue,
unsigned long pgoff)
{
- return vb2_get_unmapped_area(>queue, 0, 0, pgoff, 0);
+ struct uvc_streaming *stream = uvc_queue_to_stream(queue);
+ unsigned long ret;
+
+ mutex_lock(>mutex);
+ if (!video_is_registered(>vdev)) {
+ ret = -ENODEV;
+ goto unlock;
+ }
+ ret = vb2_get_unmapped_area(>queue, 0, 0, pgoff, 0);
+unlock:
+ mutex_unlock(>mutex);
+ return ret;
}
#endif
__poll_t uvc_queue_poll(struct uvc_video_queue *queue, struct file *file,
poll_table *wait)
{
+ struct uvc_streaming *stream = uvc_queue_to_stream(queue);
__poll_t ret;
mutex_lock(>mutex);
+ if (!video_is_registered(>vdev)) {
+ ret = EPOLLERR;
+ goto unlock;
+ }
ret = vb2_poll(>queue, file, wait);
+unlock:
mutex_unlock(>mutex);
return ret;
--
2.17.1
[PATCH RESEND v3 0/5] media: uvcvideo: Fix race conditions
Something seems to have gone wrong with v3 of this patch series. I am sure I sent it out, but I don't find it anywhere. Resending. Sorry for any duplicates. The uvcvideo code has no lock protection against USB disconnects while video operations are ongoing. This has resulted in random error reports, typically pointing to a crash in usb_ifnum_to_if(), called from usb_hcd_alloc_bandwidth(). A typical traceback is as follows. usb 1-4: USB disconnect, device number 3 BUG: unable to handle kernel NULL pointer dereference at PGD 0 P4D 0 Oops: [#1] PREEMPT SMP PTI CPU: 0 PID: 5633 Comm: V4L2CaptureThre Not tainted 4.19.113-08536-g5d29ca36db06 #1 Hardware name: GOOGLE Edgar, BIOS Google_Edgar.7287.167.156 03/25/2019 RIP: 0010:usb_ifnum_to_if+0x29/0x40 Code: <...> RSP: 0018:a46f42a47a80 EFLAGS: 00010246 RAX: RBX: RCX: 904a396c9000 RDX: 904a39641320 RSI: 0001 RDI: RBP: a46f42a47a80 R08: 0002 R09: R10: 9975 R11: 0009 R12: R13: 904a396b3800 R14: 904a39e88000 R15: FS: 7f396448e700() GS:904a3ba0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: CR3: 00016cb46000 CR4: 001006f0 Call Trace: usb_hcd_alloc_bandwidth+0x1ee/0x30f usb_set_interface+0x1a3/0x2b7 uvc_video_start_transfer+0x29b/0x4b8 [uvcvideo] uvc_video_start_streaming+0x91/0xdd [uvcvideo] uvc_start_streaming+0x28/0x5d [uvcvideo] vb2_start_streaming+0x61/0x143 [videobuf2_common] vb2_core_streamon+0xf7/0x10f [videobuf2_common] uvc_queue_streamon+0x2e/0x41 [uvcvideo] uvc_ioctl_streamon+0x42/0x5c [uvcvideo] __video_do_ioctl+0x33d/0x42a video_usercopy+0x34e/0x5ff ? video_ioctl2+0x16/0x16 v4l2_ioctl+0x46/0x53 do_vfs_ioctl+0x50a/0x76f ksys_ioctl+0x58/0x83 __x64_sys_ioctl+0x1a/0x1e do_syscall_64+0x54/0xde While there are not many references to this problem on mailing lists, it is reported on a regular basis on various Chromebooks (roughly 300 reports per month). The problem is relatively easy to reproduce by adding msleep() calls into the code. I tried to reproduce the problem with non-uvcvideo webcams, but was unsuccessful. I was unable to get Philips (pwc) webcams to work. gspca based webcams don't experience the problem, or at least I was unable to reproduce it (The gspa driver does not trigger sending USB messages in the open function, and otherwise uses the locking mechanism provided by the v4l2/vb2 core). I don't presume to claim that I found every issue, but this patch series should fix at least the major problems. The patch series was tested exensively on a Chromebook running chromeos-4.19 and on a Linux system running a v5.8.y based kernel. v3: - In patch 5/5, add missing calls to usb_autopm_put_interface() and kfree() to failure code path v2: - Added details about problem frequency and testing with non-uvc webcams to summary - In patch 4/5, return EPOLLERR instead of -ENODEV on poll errors - Fix description in patch 5/5 Guenter Roeck (5): media: uvcvideo: Cancel async worker earlier media: uvcvideo: Lock video streams and queues while unregistering media: uvcvideo: Release stream queue when unregistering video device media: uvcvideo: Protect uvc queue file operations against disconnect media: uvcvideo: Abort uvc_v4l2_open if video device is unregistered drivers/media/usb/uvc/uvc_ctrl.c | 11 ++ drivers/media/usb/uvc/uvc_driver.c | 12 ++ drivers/media/usb/uvc/uvc_queue.c | 32 +-- drivers/media/usb/uvc/uvc_v4l2.c | 45 -- drivers/media/usb/uvc/uvcvideo.h | 1 + 5 files changed, 93 insertions(+), 8 deletions(-)
Re: [PATCH v3 2/2] ARM: support PHYS_OFFSET minimum aligned at 64KiB boundary
On 2020/9/16 19:15, Ard Biesheuvel wrote:
> (+ Arnd, Nico)
>
> On Wed, 16 Sep 2020 at 05:51, Zhen Lei wrote:
>>
>> Currently, only support the kernels where the base of physical memory is
>> at a 16MiB boundary. Because the add/sub instructions only contains 8bits
>> unrotated value. But we can use one more "add/sub" instructions to handle
>> bits 23-16. The performance will be slightly affected.
>>
>> Since most boards meet 16 MiB alignment, so add a new configuration
>> option ARM_PATCH_PHYS_VIRT_RADICAL (default n) to control it. Say Y if
>> anyone really needs it.
>>
>> All r0-r7 (r1 = machine no, r2 = atags or dtb, in the start-up phase) are
>> used in __fixup_a_pv_table() now, but the callee saved r11 is not used in
>> the whole head.S file. So choose it.
>>
>> Because the calculation of "y = x + __pv_offset[63:24]" have been done,
>> so we only need to calculate "y = y + __pv_offset[23:16]", that's why
>> the parameters "to" and "from" of __pv_stub() and __pv_add_carry_stub()
>> in the scope of CONFIG_ARM_PATCH_PHYS_VIRT_RADICAL are all passed "t"
>> (above y).
>>
>> Signed-off-by: Zhen Lei
>> ---
>> arch/arm/Kconfig | 17 -
>> arch/arm/include/asm/memory.h | 16 +---
>> arch/arm/kernel/head.S| 25 +++--
>> 3 files changed, 48 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index e00d94b16658765..073dafa428f3c87 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -240,12 +240,27 @@ config ARM_PATCH_PHYS_VIRT
>> kernel in system memory.
>>
>> This can only be used with non-XIP MMU kernels where the base
>> - of physical memory is at a 16MB boundary.
>> + of physical memory is at a 16MiB boundary.
>>
>> Only disable this option if you know that you do not require
>> this feature (eg, building a kernel for a single machine) and
>> you need to shrink the kernel to the minimal size.
>>
>> +config ARM_PATCH_PHYS_VIRT_RADICAL
>> + bool "Support PHYS_OFFSET minimum aligned at 64KiB boundary"
>> + depends on ARM_PATCH_PHYS_VIRT
>> + depends on !THUMB2_KERNEL
>
> Why is this not implemented for Thumb2 too?
No Thumb2 boards.
>
> Also, as Russell points out as well, this may end up being enabled for
> all multiarch kernels, so it makes sense to explore whether we can
> enable this unconditionally.
Yes, In fact, I think we can consider enabling this unconditionally after
the THUMB2 branch is implemented. Performance and code size should not be
a problem.
> Do you have any numbers wrt the impact on
> text size? I would assume it is negligible, but numbers help.
The text size increased a bit more than 2 KB (2164 Bytes), about 0.0146%.
make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- distclean defconfig
Before:
$ size vmlinux
textdata bss dec hex filename
14781964 7508366 420080 2271041015a888a vmlinux
After:
$ size vmlinux
textdata bss dec hex filename
14784128 7508366 420080 2271257415a90fe vmlinux
>
> Being able to decompress the image to any 2MiB aligned base address is
> also quite useful for EFI boot, and it may also help to get rid of the
> TEXT_OFFSET hacks we have for some platforms in the future.>
>
>> + help
>> + This can only be used with non-XIP MMU kernels where the base
>> + of physical memory is at a 64KiB boundary.
>> +
>> + Compared with ARM_PATCH_PHYS_VIRT, one or two more instructions
>> + need to be added to implement the conversion of bits 23-16 of
>> + the VA/PA in phys-to-virt and virt-to-phys. The performance is
>> + slightly affected.
>> +
>
> Does it affect performance in other ways beyond code size/Icache density?
I just want to say it will slightly slower than !ARM_PATCH_PHYS_VIRT_RADICAL,
because one or two more instructions. It certainly cannot affect system
performance.
Because of your doubts, I think I should remove the statement: "The performance
is
slightly affected."
>
>> + If unsure say N here.
>> +
>> config NEED_MACH_IO_H
>> bool
>> help
>> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
>> index 99035b5891ef442..f97b37303a00f60 100644
>> --- a/arch/arm/include/asm/memory.h
>> +++ b/arch/arm/include/asm/memory.h
>> @@ -173,6 +173,7 @@
>> * so that all we need to do is modify the 8-bit constant field.
>> */
>> #define __PV_BITS_31_240x8100
>> +#define __PV_BITS_23_160x0081
>> #define __PV_BITS_7_0 0x81
>>
>> extern unsigned long __pv_phys_pfn_offset;
>> @@ -201,7 +202,7 @@
>> : "=r" (t) \
>> : "I" (__PV_BITS_7_0))
>>
>> -#define __pv_add_carry_stub(x, y) \
>> +#define __pv_add_carry_stub(x, y, type)\
>> __asm__ volatile("@ __pv_add_carry_stub\n"
[PATCH RESEND v3 2/5] media: uvcvideo: Lock video streams and queues while unregistering
The call to uvc_disconnect() is not protected by any mutex.
This means it can and will be called while other accesses to the video
device are in progress. This can cause all kinds of race conditions,
including crashes such as the following.
usb 1-4: USB disconnect, device number 3
BUG: unable to handle kernel NULL pointer dereference at
PGD 0 P4D 0
Oops: [#1] PREEMPT SMP PTI
CPU: 0 PID: 5633 Comm: V4L2CaptureThre Not tainted 4.19.113-08536-g5d29ca36db06
#1
Hardware name: GOOGLE Edgar, BIOS Google_Edgar.7287.167.156 03/25/2019
RIP: 0010:usb_ifnum_to_if+0x29/0x40
Code: <...>
RSP: 0018:a46f42a47a80 EFLAGS: 00010246
RAX: RBX: RCX: 904a396c9000
RDX: 904a39641320 RSI: 0001 RDI:
RBP: a46f42a47a80 R08: 0002 R09:
R10: 9975 R11: 0009 R12:
R13: 904a396b3800 R14: 904a39e88000 R15:
FS: 7f396448e700() GS:904a3ba0() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: CR3: 00016cb46000 CR4: 001006f0
Call Trace:
usb_hcd_alloc_bandwidth+0x1ee/0x30f
usb_set_interface+0x1a3/0x2b7
uvc_video_start_transfer+0x29b/0x4b8 [uvcvideo]
uvc_video_start_streaming+0x91/0xdd [uvcvideo]
uvc_start_streaming+0x28/0x5d [uvcvideo]
vb2_start_streaming+0x61/0x143 [videobuf2_common]
vb2_core_streamon+0xf7/0x10f [videobuf2_common]
uvc_queue_streamon+0x2e/0x41 [uvcvideo]
uvc_ioctl_streamon+0x42/0x5c [uvcvideo]
__video_do_ioctl+0x33d/0x42a
video_usercopy+0x34e/0x5ff
? video_ioctl2+0x16/0x16
v4l2_ioctl+0x46/0x53
do_vfs_ioctl+0x50a/0x76f
ksys_ioctl+0x58/0x83
__x64_sys_ioctl+0x1a/0x1e
do_syscall_64+0x54/0xde
usb_set_interface() should not be called after the USB device has been
unregistered. However, in the above case the disconnect happened after
v4l2_ioctl() was called, but before the call to usb_ifnum_to_if().
Acquire various mutexes in uvc_unregister_video() to fix the majority
(maybe all) of the observed race conditions.
The uvc_device lock prevents races against suspend and resume calls
and the poll function.
The uvc_streaming lock prevents races against stream related functions;
for the most part, those are ioctls. This lock also requires other
functions using this lock to check if a video device is still registered
after acquiring it. For example, it was observed that the video device
was already unregistered by the time the stream lock was acquired in
uvc_ioctl_streamon().
The uvc_queue lock prevents races against queue functions, Most of
those are already protected by the uvc_streaming lock, but some
are called directly. This is done as added protection; an actual race
was not (yet) observed.
Cc: Laurent Pinchart
Cc: Alan Stern
Cc: Hans Verkuil
Signed-off-by: Guenter Roeck
---
drivers/media/usb/uvc/uvc_driver.c | 9 +++
drivers/media/usb/uvc/uvc_v4l2.c | 39 +-
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/drivers/media/usb/uvc/uvc_driver.c
b/drivers/media/usb/uvc/uvc_driver.c
index bfba67a69185..a5808305f1e3 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1941,14 +1941,22 @@ static void uvc_unregister_video(struct uvc_device *dev)
{
struct uvc_streaming *stream;
+ mutex_lock(>lock);
+
list_for_each_entry(stream, >streams, list) {
if (!video_is_registered(>vdev))
continue;
+ mutex_lock(>mutex);
+ mutex_lock(>queue.mutex);
+
video_unregister_device(>vdev);
video_unregister_device(>meta.vdev);
uvc_debugfs_cleanup_stream(stream);
+
+ mutex_unlock(>queue.mutex);
+ mutex_unlock(>mutex);
}
uvc_status_unregister(dev);
@@ -1960,6 +1968,7 @@ static void uvc_unregister_video(struct uvc_device *dev)
if (media_devnode_is_registered(dev->mdev.devnode))
media_device_unregister(>mdev);
#endif
+ mutex_unlock(>lock);
}
int uvc_register_video_device(struct uvc_device *dev,
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 0335e69b70ab..7e5e583dae5e 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -237,6 +237,12 @@ static int uvc_v4l2_try_format(struct uvc_streaming
*stream,
* the Windows driver).
*/
mutex_lock(>mutex);
+ if (!video_is_registered(>vdev)) {
+ mutex_unlock(>mutex);
+ ret = -ENODEV;
+ goto done;
+ }
+
if (stream->dev->quirks & UVC_QUIRK_PROBE_EXTRAFIELDS)
probe->dwMaxVideoFrameSize =
stream->ctrl.dwMaxVideoFrameSize;
@@ -274,6 +280,12 @@ static int uvc_v4l2_get_format(struct uvc_streaming
*stream,
return -EINVAL;
[PATCH RESEND v3 3/5] media: uvcvideo: Release stream queue when unregistering video device
The following traceback was observed when stress testing the uvcvideo driver. config->interface[0] is NULL WARNING: CPU: 0 PID: 2757 at drivers/usb/core/usb.c:285 usb_ifnum_to_if+0x81/0x85 ^^^ added log message and WARN() to prevent crash Modules linked in: <...> CPU: 0 PID: 2757 Comm: inotify_reader Not tainted 4.19.139 #20 Hardware name: Google Phaser/Phaser, BIOS Google_Phaser.10952.0.0 08/09/2018 RIP: 0010:usb_ifnum_to_if+0x81/0x85 Code: <...> RSP: 0018:9ee20141fa58 EFLAGS: 00010246 RAX: 438a0e5a525f1800 RBX: RCX: RDX: 975477a1de90 RSI: 975477a153d0 RDI: 975477a153d0 RBP: 9ee20141fa70 R08: 002c R09: 002daec189138d78 R10: 0010 R11: a7da42e6 R12: 975459594800 R13: 0001 R14: 0001 R15: 975465376400 FS: 7ddebffd6700() GS:975477a0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 12c83000 CR3: 0001bbaf8000 CR4: 00340ef0 Call Trace: usb_set_interface+0x3b/0x2f3 uvc_video_stop_streaming+0x38/0x81 [uvcvideo] uvc_stop_streaming+0x21/0x49 [uvcvideo] __vb2_queue_cancel+0x33/0x249 [videobuf2_common] vb2_core_queue_release+0x1c/0x45 [videobuf2_common] uvc_queue_release+0x26/0x32 [uvcvideo] uvc_v4l2_release+0x41/0xdd [uvcvideo] v4l2_release+0x99/0xed __fput+0xf0/0x1ea task_work_run+0x7f/0xa7 do_exit+0x1d1/0x6eb do_group_exit+0x9d/0xac get_signal+0x135/0x482 do_signal+0x4a/0x63c exit_to_usermode_loop+0x86/0xa5 do_syscall_64+0x171/0x269 ? __do_page_fault+0x272/0x474 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x7ddec156dc26 Code: Bad RIP value. RSP: 002b:7ddebffd5c10 EFLAGS: 0293 ORIG_RAX: 0017 RAX: fdfe RBX: 000a RCX: 7ddec156dc26 RDX: RSI: 7ddebffd5d28 RDI: 000a RBP: 7ddebffd5c50 R08: R09: R10: R11: 0293 R12: 7ddebffd5d28 R13: R14: R15: When this was observed, a USB disconnect was in progress, uvc_disconnect() had returned, but usb_disable_device() was still running. Drivers are not supposed to call any USB functions after the driver disconnect function has been called. The uvcvideo driver does not follow that convention and tries to write to the disconnected USB interface anyway. This results in a variety of race conditions. To solve this specific problem, release the uvc queue from uvc_unregister_video() after the associated video devices have been unregistered. Since the function already holds the uvc queue mutex, bypass uvc_queue_release() and call vb2_queue_release() directly. Cc: Laurent Pinchart Cc: Alan Stern Cc: Hans Verkuil Signed-off-by: Guenter Roeck --- drivers/media/usb/uvc/uvc_driver.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index a5808305f1e3..27b72806b9b9 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -1955,6 +1955,8 @@ static void uvc_unregister_video(struct uvc_device *dev) uvc_debugfs_cleanup_stream(stream); + vb2_queue_release(>queue.queue); + mutex_unlock(>queue.mutex); mutex_unlock(>mutex); } -- 2.17.1
[PATCH] ptp: mark symbols static where possible
We get 1 warning when building kernel with W=1:
drivers/ptp/ptp_pch.c:182:5: warning: no previous prototype for
‘pch_ch_control_read’ [-Wmissing-prototypes]
u32 pch_ch_control_read(struct pci_dev *pdev)
drivers/ptp/ptp_pch.c:193:6: warning: no previous prototype for
‘pch_ch_control_write’ [-Wmissing-prototypes]
void pch_ch_control_write(struct pci_dev *pdev, u32 val)
drivers/ptp/ptp_pch.c:201:5: warning: no previous prototype for
‘pch_ch_event_read’ [-Wmissing-prototypes]
u32 pch_ch_event_read(struct pci_dev *pdev)
drivers/ptp/ptp_pch.c:212:6: warning: no previous prototype for
‘pch_ch_event_write’ [-Wmissing-prototypes]
void pch_ch_event_write(struct pci_dev *pdev, u32 val)
drivers/ptp/ptp_pch.c:220:5: warning: no previous prototype for
‘pch_src_uuid_lo_read’ [-Wmissing-prototypes]
u32 pch_src_uuid_lo_read(struct pci_dev *pdev)
drivers/ptp/ptp_pch.c:231:5: warning: no previous prototype for
‘pch_src_uuid_hi_read’ [-Wmissing-prototypes]
u32 pch_src_uuid_hi_read(struct pci_dev *pdev)
drivers/ptp/ptp_pch.c:242:5: warning: no previous prototype for
‘pch_rx_snap_read’ [-Wmissing-prototypes]
u64 pch_rx_snap_read(struct pci_dev *pdev)
drivers/ptp/ptp_pch.c:259:5: warning: no previous prototype for
‘pch_tx_snap_read’ [-Wmissing-prototypes]
u64 pch_tx_snap_read(struct pci_dev *pdev)
drivers/ptp/ptp_pch.c:300:5: warning: no previous prototype for
‘pch_set_station_address’ [-Wmissing-prototypes]
int pch_set_station_address(u8 *addr, struct pci_dev *pdev)
Signed-off-by: Herrington
---
drivers/ptp/ptp_pch.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/ptp/ptp_pch.c b/drivers/ptp/ptp_pch.c
index ce10ecd41ba0..8db2d1893577 100644
--- a/drivers/ptp/ptp_pch.c
+++ b/drivers/ptp/ptp_pch.c
@@ -179,7 +179,7 @@ static inline void pch_block_reset(struct pch_dev *chip)
iowrite32(val, (>regs->control));
}
-u32 pch_ch_control_read(struct pci_dev *pdev)
+static u32 pch_ch_control_read(struct pci_dev *pdev)
{
struct pch_dev *chip = pci_get_drvdata(pdev);
u32 val;
@@ -190,7 +190,7 @@ u32 pch_ch_control_read(struct pci_dev *pdev)
}
EXPORT_SYMBOL(pch_ch_control_read);
-void pch_ch_control_write(struct pci_dev *pdev, u32 val)
+static void pch_ch_control_write(struct pci_dev *pdev, u32 val)
{
struct pch_dev *chip = pci_get_drvdata(pdev);
@@ -198,7 +198,7 @@ void pch_ch_control_write(struct pci_dev *pdev, u32 val)
}
EXPORT_SYMBOL(pch_ch_control_write);
-u32 pch_ch_event_read(struct pci_dev *pdev)
+static u32 pch_ch_event_read(struct pci_dev *pdev)
{
struct pch_dev *chip = pci_get_drvdata(pdev);
u32 val;
@@ -209,7 +209,7 @@ u32 pch_ch_event_read(struct pci_dev *pdev)
}
EXPORT_SYMBOL(pch_ch_event_read);
-void pch_ch_event_write(struct pci_dev *pdev, u32 val)
+static void pch_ch_event_write(struct pci_dev *pdev, u32 val)
{
struct pch_dev *chip = pci_get_drvdata(pdev);
@@ -217,7 +217,7 @@ void pch_ch_event_write(struct pci_dev *pdev, u32 val)
}
EXPORT_SYMBOL(pch_ch_event_write);
-u32 pch_src_uuid_lo_read(struct pci_dev *pdev)
+static u32 pch_src_uuid_lo_read(struct pci_dev *pdev)
{
struct pch_dev *chip = pci_get_drvdata(pdev);
u32 val;
@@ -228,7 +228,7 @@ u32 pch_src_uuid_lo_read(struct pci_dev *pdev)
}
EXPORT_SYMBOL(pch_src_uuid_lo_read);
-u32 pch_src_uuid_hi_read(struct pci_dev *pdev)
+static u32 pch_src_uuid_hi_read(struct pci_dev *pdev)
{
struct pch_dev *chip = pci_get_drvdata(pdev);
u32 val;
@@ -239,7 +239,7 @@ u32 pch_src_uuid_hi_read(struct pci_dev *pdev)
}
EXPORT_SYMBOL(pch_src_uuid_hi_read);
-u64 pch_rx_snap_read(struct pci_dev *pdev)
+static u64 pch_rx_snap_read(struct pci_dev *pdev)
{
struct pch_dev *chip = pci_get_drvdata(pdev);
u64 ns;
@@ -256,7 +256,7 @@ u64 pch_rx_snap_read(struct pci_dev *pdev)
}
EXPORT_SYMBOL(pch_rx_snap_read);
-u64 pch_tx_snap_read(struct pci_dev *pdev)
+static u64 pch_tx_snap_read(struct pci_dev *pdev)
{
struct pch_dev *chip = pci_get_drvdata(pdev);
u64 ns;
@@ -297,7 +297,7 @@ static void pch_reset(struct pch_dev *chip)
* traffic on the ethernet interface
* @addr: dress which contain the column separated address to be used.
*/
-int pch_set_station_address(u8 *addr, struct pci_dev *pdev)
+static int pch_set_station_address(u8 *addr, struct pci_dev *pdev)
{
s32 i;
struct pch_dev *chip = pci_get_drvdata(pdev);
--
2.18.1
[PATCH RESEND v3 5/5] media: uvcvideo: Abort uvc_v4l2_open if video device is unregistered
uvc_v4l2_open() acquires the uvc device mutex. After doing so,
it does not check if the video device is still registered. This may
result in race conditions and can result in an attempt to submit an urb
to a disconnected USB interface (from uvc_status_start).
The problem was observed after adding a call to msleep() just before
acquiring the mutex and disconnecting the camera during the sleep.
Check if the video device is still registered after acquiring the mutex
to avoid the problem. In the release function, only call uvc_status_stop()
if the video device is still registered. If the video device has been
unregistered, the urb associated with uvc status has already been killed
in uvc_status_unregister(). Trying to kill it again won't do any good
and might have unexpected side effects.
Cc: Laurent Pinchart
Cc: Alan Stern
Cc: Hans Verkuil
Signed-off-by: Guenter Roeck
---
drivers/media/usb/uvc/uvc_v4l2.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 7e5e583dae5e..8073eae5d879 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -548,6 +548,12 @@ static int uvc_v4l2_open(struct file *file)
}
mutex_lock(>dev->lock);
+ if (!video_is_registered(>vdev)) {
+ mutex_unlock(>dev->lock);
+ usb_autopm_put_interface(stream->dev->intf);
+ kfree(handle);
+ return -ENODEV;
+ }
if (stream->dev->users == 0) {
ret = uvc_status_start(stream->dev, GFP_KERNEL);
if (ret < 0) {
@@ -590,7 +596,7 @@ static int uvc_v4l2_release(struct file *file)
file->private_data = NULL;
mutex_lock(>dev->lock);
- if (--stream->dev->users == 0)
+ if (--stream->dev->users == 0 && video_is_registered(>vdev))
uvc_status_stop(stream->dev);
mutex_unlock(>dev->lock);
--
2.17.1
[PATCH RESEND v3 1/5] media: uvcvideo: Cancel async worker earlier
So far the asynchronous control worker was canceled only in
uvc_ctrl_cleanup_device. This is much later than the call to
uvc_disconnect. However, after the call to uvc_disconnect returns,
there must be no more USB activity. This can result in all kinds
of problems in the USB code. One observed example:
URB 993e83d0bc00 submitted while active
WARNING: CPU: 0 PID: 4046 at drivers/usb/core/urb.c:364
usb_submit_urb+0x4ba/0x55e
Modules linked in: <...>
CPU: 0 PID: 4046 Comm: kworker/0:35 Not tainted 4.19.139 #18
Hardware name: Google Phaser/Phaser, BIOS Google_Phaser.10952.0.0 08/09/2018
Workqueue: events uvc_ctrl_status_event_work [uvcvideo]
RIP: 0010:usb_submit_urb+0x4ba/0x55e
Code: <...>
RSP: 0018:b08d471ebde8 EFLAGS: 00010246
RAX: a6da85d923ea5d00 RBX: 993e71985928 RCX:
RDX: 993f37a1de90 RSI: 993f37a153d0 RDI: 993f37a153d0
RBP: b08d471ebe28 R08: 003b R09: 001424bf85822e96
R10: 0010 R11: 975a4398 R12: 993e83d0b000
R13: 993e83d0bc00 R14: R15: fff0
FS: () GS:993f37a0() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: ec9c CR3: 00025b16 CR4: 00340ef0
Call Trace:
uvc_ctrl_status_event_work+0xd6/0x107 [uvcvideo]
process_one_work+0x19b/0x4c5
worker_thread+0x10d/0x286
kthread+0x138/0x140
? process_one_work+0x4c5/0x4c5
? kthread_associate_blkcg+0xc1/0xc1
ret_from_fork+0x1f/0x40
Introduce new function uvc_ctrl_stop_device() to cancel the worker
and call it from uvc_unregister_video() to solve the problem.
Cc: Laurent Pinchart
Cc: Alan Stern
Cc: Hans Verkuil
Signed-off-by: Guenter Roeck
---
drivers/media/usb/uvc/uvc_ctrl.c | 11 +++
drivers/media/usb/uvc/uvc_driver.c | 1 +
drivers/media/usb/uvc/uvcvideo.h | 1 +
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index e399b9fad757..130c56e0063d 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2340,14 +2340,17 @@ static void uvc_ctrl_cleanup_mappings(struct uvc_device
*dev,
}
}
-void uvc_ctrl_cleanup_device(struct uvc_device *dev)
+void uvc_ctrl_stop_device(struct uvc_device *dev)
{
- struct uvc_entity *entity;
- unsigned int i;
-
/* Can be uninitialized if we are aborting on probe error. */
if (dev->async_ctrl.work.func)
cancel_work_sync(>async_ctrl.work);
+}
+
+void uvc_ctrl_cleanup_device(struct uvc_device *dev)
+{
+ struct uvc_entity *entity;
+ unsigned int i;
/* Free controls and control mappings for all entities. */
list_for_each_entry(entity, >entities, list) {
diff --git a/drivers/media/usb/uvc/uvc_driver.c
b/drivers/media/usb/uvc/uvc_driver.c
index 431d86e1c94b..bfba67a69185 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1952,6 +1952,7 @@ static void uvc_unregister_video(struct uvc_device *dev)
}
uvc_status_unregister(dev);
+ uvc_ctrl_stop_device(dev);
if (dev->vdev.dev)
v4l2_device_unregister(>vdev);
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 6ab972c643e3..543afcd9fd26 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -830,6 +830,7 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
int uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
const struct uvc_control_mapping *mapping);
int uvc_ctrl_init_device(struct uvc_device *dev);
+void uvc_ctrl_stop_device(struct uvc_device *dev);
void uvc_ctrl_cleanup_device(struct uvc_device *dev);
int uvc_ctrl_restore_values(struct uvc_device *dev);
bool uvc_ctrl_status_event(struct urb *urb, struct uvc_video_chain *chain,
--
2.17.1
[PATCH RFC] KVM: x86: emulate wait-for-SIPI and SIPI-VMExit
From: Yadong Qi
Background: We have a lightweight HV, it needs INIT-VMExit and
SIPI-VMExit to wake-up APs for guests since it do not monitoring
the Local APIC. But currently virtual wait-for-SIPI(WFS) state
is not supported in KVM, so when running on top of KVM, the L1
HV cannot receive the INIT-VMExit and SIPI-VMExit which cause
the L2 guest cannot wake up the APs.
This patch is incomplete, it emulated wait-for-SIPI state by halt
the vCPU and emulated SIPI-VMExit to L1 when trapped SIPI signal
from L2. I am posting it RFC to gauge whether or not upstream
KVM is interested in emulating wait-for-SIPI state before
investing the time to finish the full support.
According to Intel SDM Chapter 25.2 Other Causes of VM Exits,
SIPIs cause VM exits when a logical processor is in
wait-for-SIPI state.
In this patch:
1. introduce SIPI exit reason,
2. introduce wait-for-SIPI state for nVMX,
3. advertise wait-for-SIPI support to guest.
When L1 hypervisor is not monitoring Local APIC, L0 need to emulate
INIT-VMExit and SIPI-VMExit to L1 to emulate INIT-SIPI-SIPI for
L2. L2 LAPIC write would be traped by L0 Hypervisor(KVM), L0 should
emulate the INIT/SIPI vmexit to L1 hypervisor to set proper state
for L2's vcpu state.
Handle procdure:
Source vCPU:
L2 write LAPIC.ICR(INIT).
L0 trap LAPIC.ICR write(INIT): inject a latched INIT event to target
vCPU.
Target vCPU:
L0 emulate an INIT VMExit to L1 if is guest mode.
L1 set guest VMCS, guest_activity_state=WAIT_SIPI, vmresume.
L0 halt vCPU if (vmcs12.guest_activity_state == WAIT_SIPI).
Source vCPU:
L2 write LAPIC.ICR(SIPI).
L0 trap LAPIC.ICR write(INIT): inject a latched SIPI event to traget
vCPU.
Target vCPU:
L0 emulate an SIPI VMExit to L1 if (vmcs12.guest_activity_state ==
WAIT_SIPI).
L1 set CS:IP, guest_activity_state=ACTIVE, vmresume
L0 resume to L2
L2 start-up
Signed-off-by: Yadong Qi
---
arch/x86/include/asm/vmx.h | 1 +
arch/x86/include/uapi/asm/vmx.h | 2 ++
arch/x86/kvm/lapic.c| 5 +++--
arch/x86/kvm/vmx/nested.c | 25 +
4 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index cd7de4b401fe..bff06dc64c52 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -113,6 +113,7 @@
#define VMX_MISC_PREEMPTION_TIMER_RATE_MASK0x001f
#define VMX_MISC_SAVE_EFER_LMA 0x0020
#define VMX_MISC_ACTIVITY_HLT 0x0040
+#define VMX_MISC_ACTIVITY_WAIT_SIPI0x0100
#define VMX_MISC_ZERO_LEN_INS 0x4000
#define VMX_MISC_MSR_LIST_MULTIPLIER 512
diff --git a/arch/x86/include/uapi/asm/vmx.h b/arch/x86/include/uapi/asm/vmx.h
index b8ff9e8ac0d5..ada955c5ebb6 100644
--- a/arch/x86/include/uapi/asm/vmx.h
+++ b/arch/x86/include/uapi/asm/vmx.h
@@ -32,6 +32,7 @@
#define EXIT_REASON_EXTERNAL_INTERRUPT 1
#define EXIT_REASON_TRIPLE_FAULT2
#define EXIT_REASON_INIT_SIGNAL3
+#define EXIT_REASON_SIPI_SIGNAL 4
#define EXIT_REASON_INTERRUPT_WINDOW7
#define EXIT_REASON_NMI_WINDOW 8
@@ -94,6 +95,7 @@
{ EXIT_REASON_EXTERNAL_INTERRUPT,"EXTERNAL_INTERRUPT" }, \
{ EXIT_REASON_TRIPLE_FAULT, "TRIPLE_FAULT" }, \
{ EXIT_REASON_INIT_SIGNAL, "INIT_SIGNAL" }, \
+ { EXIT_REASON_SIPI_SIGNAL, "SIPI_SIGNAL" }, \
{ EXIT_REASON_INTERRUPT_WINDOW, "INTERRUPT_WINDOW" }, \
{ EXIT_REASON_NMI_WINDOW,"NMI_WINDOW" }, \
{ EXIT_REASON_TASK_SWITCH, "TASK_SWITCH" }, \
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 5ccbee7165a2..d04ac7dc6adf 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2839,7 +2839,7 @@ void kvm_apic_accept_events(struct kvm_vcpu *vcpu)
/*
* INITs are latched while CPU is in specific states
-* (SMM, VMX non-root mode, SVM with GIF=0).
+* (SMM, SVM with GIF=0).
* Because a CPU cannot be in these states immediately
* after it has processed an INIT signal (and thus in
* KVM_MP_STATE_INIT_RECEIVED state), just eat SIPIs
@@ -2847,7 +2847,8 @@ void kvm_apic_accept_events(struct kvm_vcpu *vcpu)
*/
if (kvm_vcpu_latch_init(vcpu)) {
WARN_ON_ONCE(vcpu->arch.mp_state == KVM_MP_STATE_INIT_RECEIVED);
- if (test_bit(KVM_APIC_SIPI, >pending_events))
+ if (test_bit(KVM_APIC_SIPI, >pending_events) &&
+ !is_guest_mode(vcpu))
clear_bit(KVM_APIC_SIPI, >pending_events);
return;
}
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 1bb6b31eb646..399933b8ac3a 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -2946,7 +2946,8 @@ static int nested_vmx_check_vmcs_link_ptr(struct kvm_vcpu
*vcpu,
Re: [PATCH v2 0/5] seqlock: Introduce PREEMPT_RT support
Hi all, On Wed, 16 Sep 2020 15:02:33 +0200 pet...@infradead.org wrote: > > On Wed, Sep 16, 2020 at 09:00:59AM -0400, Qian Cai wrote: > > > > > > - Original Message - > > > On Wed, Sep 16, 2020 at 08:52:07AM -0400, Qian Cai wrote: > > > > On Tue, 2020-09-15 at 16:30 +0200, pet...@infradead.org wrote: > > > > > On Tue, Sep 15, 2020 at 08:48:17PM +0800, Boqun Feng wrote: > > > > > > I think this happened because seqcount_##lockname##_init() is > > > > > > defined > > > > > > at > > > > > > function rather than macro, so when the seqcount_init() gets expand > > > > > > in > > > > > > > > > > Bah! I hate all this :/ > > > > > > > > > > I suspect the below, while more verbose than I'd like is the best > > > > > option. > > > > > > > > Stephen, can you add this patch for now until Peter beats you to it? > > > > > > Did you verify it works? I only wrote it.. > > > > Yes, I did. > > Excellent, I'll stick a Tested-by from you on then. I'll add this into the tip tree merge today (unless the tip tree is updated in the mean time). -- Cheers, Stephen Rothwell pgpl8Iukb0goQ.pgp Description: OpenPGP digital signature
Re: [RFC PATCH V3 12/21] mmc: sdhci: UHS-II support, add hooks for additional operations
Adrian,
On Wed, Sep 16, 2020 at 01:00:35PM +0300, Adrian Hunter wrote:
> On 16/09/20 11:05 am, AKASHI Takahiro wrote:
> > Adrian,
> >
> > Your comments are scattered over various functions, and so
> > I would like to address them in separate replies.
> >
> > First, I'd like to discuss sdhci_[add|remove]_host().
> >
> > On Fri, Aug 21, 2020 at 05:08:32PM +0300, Adrian Hunter wrote:
> >> On 10/07/20 2:10 pm, Ben Chuang wrote:
> >>> From: Ben Chuang
> >>>
> >>> In this commit, UHS-II related operations will be called via a function
> >>> pointer array, sdhci_uhs2_ops, in order to make UHS-II support as
> >>> a kernel module.
> >>> This array will be initialized only if CONFIG_MMC_SDHCI_UHS2 is enabled
> >>> and when the UHS-II module is loaded. Otherwise, all the functions
> >>> stay void.
> >>>
> >>> Signed-off-by: Ben Chuang
> >>> Signed-off-by: AKASHI Takahiro
> >>> ---
> >
> > (snip)
> >
> >>> if (intmask & (SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE)) {
> >>> u32 present = sdhci_readl(host, SDHCI_PRESENT_STATE) &
> >>> SDHCI_CARD_PRESENT;
> >>> @@ -4717,6 +4812,14 @@ int sdhci_setup_host(struct sdhci_host *host)
> >>> /* This may alter mmc->*_blk_* parameters */
> >>> sdhci_allocate_bounce_buffer(host);
> >>>
> >>> + if (IS_ENABLED(CONFIG_MMC_SDHCI_UHS2) &&
> >>> + host->version >= SDHCI_SPEC_400 &&
> >>> + sdhci_uhs2_ops.add_host) {
> >>> + ret = sdhci_uhs2_ops.add_host(host, host->caps1);
> >>> + if (ret)
> >>> + goto unreg;
> >>> + }
> >>> +
> >>
> >> I think you should look at creating uhs2_add_host() instead
> >>
> >>> return 0;
> >>>
> >>> unreg:
> >>> @@ -4738,6 +4841,8 @@ void sdhci_cleanup_host(struct sdhci_host *host)
> >>> {
> >>> struct mmc_host *mmc = host->mmc;
> >>>
> >>> + /* FIXME: Do we have to do some cleanup for UHS2 here? */
> >>> +
> >>> if (!IS_ERR(mmc->supply.vqmmc))
> >>> regulator_disable(mmc->supply.vqmmc);
> >>>
> >>> @@ -4766,6 +4871,14 @@ int __sdhci_add_host(struct sdhci_host *host)
> >>> mmc->cqe_ops = NULL;
> >>> }
> >>>
> >>> + if ((mmc->caps & MMC_CAP_UHS2) && !host->v4_mode) {
> >>> + /* host doesn't want to enable UHS2 support */
> >>> + mmc->caps &= ~MMC_CAP_UHS2;
> >>> + mmc->flags &= ~MMC_UHS2_SUPPORT;
> >>> +
> >>> + /* FIXME: Do we have to do some cleanup here? */
> >>> + }
> >>> +
> >>> host->complete_wq = alloc_workqueue("sdhci", flags, 0);
> >>> if (!host->complete_wq)
> >>> return -ENOMEM;
> >>> @@ -4812,6 +4925,9 @@ int __sdhci_add_host(struct sdhci_host *host)
> >>> unled:
> >>> sdhci_led_unregister(host);
> >>> unirq:
> >>> + if (IS_ENABLED(CONFIG_MMC_SDHCI_UHS2) &&
> >>> + sdhci_uhs2_ops.remove_host)
> >>> + sdhci_uhs2_ops.remove_host(host, 0);
> >>> sdhci_do_reset(host, SDHCI_RESET_ALL);
> >>> sdhci_writel(host, 0, SDHCI_INT_ENABLE);
> >>> sdhci_writel(host, 0, SDHCI_SIGNAL_ENABLE);
> >>> @@ -4869,6 +4985,10 @@ void sdhci_remove_host(struct sdhci_host *host,
> >>> int dead)
> >>>
> >>> sdhci_led_unregister(host);
> >>>
> >>> + if (IS_ENABLED(CONFIG_MMC_SDHCI_UHS2) &&
> >>> + sdhci_uhs2_ops.remove_host)
> >>> + sdhci_uhs2_ops.remove_host(host, dead);
> >>> +
> >>
> >> I think you should look at creating uhs2_remove_host() instead
> >
> > You suggest that we will have separate sdhci_uhs2_[add|remove]_host(),
> > but I don't think it's always convenient.
> >
> > UHS-II capable host will be set to call sdhci_uhs2_add_host() explicitly,
> > but we can't do that in case of pci and pltfm based drivers as they utilize
> > common helper functions, sdhci_pci_probe() and sdhci_pltfm_register(),
> > respectively.
>
> sdhci-pci has an add_host op
>
> sdhci_pltfm_init can be used instead of sdhci_pltfm_register
>
>
> > Therefore, we inevitably have to call sdhci_uhs2_add_host() there.
> >
> > If so, why should we distinguish sdhci_uhs2_add_host from
> > sdhci_uhs_add_host?
> > I don't see any good reason.
> > Moreover, as a result, there exists a mixed usage of sdhci_ interfaces
> > and sdhci_uhs2_ interfaces in sdhci-pci-core.c and sdhci-pltfm.c.
> >
> > It sounds odd to me.
>
> It is already done that way for cqhci.
Okay, if it is your policy, I will follow that.
Then, I'm going to add
- remove_host field to struct sdhci_pci_fixes
- a controller specific helper function to each driver (only pci-gli for now)
even though it looks quite generic.
sdhci_gli_[add|remove]_host(struct sdhci_pci_slot *slot)
{
return sdhci_uhs2_[add|remove]_host(slot->host);
}
# Or do you want to create a file like sdhci-uhs2-pci.c for those functions?
-Takahiro Akashi
> >
> > -Takahiro Akashi
> >
> >
> >>
> >>> if (!dead)
> >>> sdhci_do_reset(host, SDHCI_RESET_ALL);
> >>>
> >>>
> >>
>
Re: [PATCH v1 1/1] scsi: ufshcd: Properly set the device Icc Level
On Wed 16 Sep 19:53 CDT 2020, nguy...@codeaurora.org wrote:
> On 2020-09-15 06:37, Bjorn Andersson wrote:
> > On Tue 15 Sep 03:49 CDT 2020, nguy...@codeaurora.org wrote:
> >
> > > On 2020-09-14 19:54, Bjorn Andersson wrote:
> > > > On Tue 01 Sep 01:19 UTC 2020, Bao D. Nguyen wrote:
> > > >
> > > > > UFS version 3.0 and later devices require Vcc and Vccq power supplies
> > > > > with Vccq2 being optional. While earlier UFS version 2.0 and 2.1
> > > > > devices, the Vcc and Vccq2 are required with Vccq being optional.
> > > > > Check the required power supplies used by the device
> > > > > and set the device's supported Icc level properly.
> > > > >
> > > > > Signed-off-by: Can Guo
> > > > > Signed-off-by: Asutosh Das
> > > > > Signed-off-by: Bao D. Nguyen
> > > > > ---
> > > > > drivers/scsi/ufs/ufshcd.c | 5 +++--
> > > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > > > > index 06e2439..fdd1d3e 100644
> > > > > --- a/drivers/scsi/ufs/ufshcd.c
> > > > > +++ b/drivers/scsi/ufs/ufshcd.c
> > > > > @@ -6845,8 +6845,9 @@ static u32
> > > > > ufshcd_find_max_sup_active_icc_level(struct ufs_hba *hba,
> > > > > {
> > > > > u32 icc_level = 0;
> > > > >
> > > > > - if (!hba->vreg_info.vcc || !hba->vreg_info.vccq ||
> > > > > - !hba->vreg_info.vccq2) {
> > > > > + if (!hba->vreg_info.vcc ||
> > > >
> > > > How did you test this?
> > > >
> > > > devm_regulator_get() never returns NULL, so afaict this conditional will
> > > > never be taken with either the old or new version of the code.
> > > Thanks for your comment. The call flow is as follows:
> > > ufshcd_pltfrm_init->ufshcd_parse_regulator_info->ufshcd_populate_vreg
> > > In the ufshcd_populate_vreg() function, it looks for DT entries
> > > "%s-supply"
> > > For UFS3.0+ devices, "vccq2-supply" is optional, so the vendor may
> > > choose
> > > not to provide vccq2-supply in the DT.
> > > As a result, a NULL is returned to hba->vreg_info.vccq2.
> > > Same for UFS2.0 and UFS2.1 devices, a NULL may be returned to
> > > hba->vreg_info.vccq if vccq-supply is not provided in the DT.
> > > The current code only checks for !hba->vreg_info.vccq OR
> > > !hba->vreg_info.vccq2. It will skip the setting for icc_level
> > > if either vccq or vccq2 is not provided in the DT.
> > > >
> >
> > Thanks for the pointers, I now see that the there will only be struct
> > ufs_vreg objects allocated for the items that has an associated
> > %s-supply.
> >
> > FYI, the idiomatic way to handle optional regulators is to use
> > regulator_get_optional(), which will return -ENODEV for regulators not
> > specified.
> Thanks for the regulator_get_optional() suggestion. Do you have a strong
> opinion about
> using regulator_get_optional() or would my proposal be ok? With
> regulator_get_optional(),
> we need to make 3 calls and check each result while the current
> implementation is also reliable
> simple quick check for NULL without any potential problem.
>
I think the changes to the conditional that you're proposing in this
patch is reasonable.
Regards,
Bjorn
> Thanks,
> Bao
> >
> > Regards,
> > Bjorn
> >
> > > > Regards,
> > > > Bjorn
> > > >
> > > > > + (!hba->vreg_info.vccq && hba->dev_info.wspecversion >=
> > > > > 0x300) ||
> > > > > + (!hba->vreg_info.vccq2 && hba->dev_info.wspecversion <
> > > > > 0x300)) {
> > > > > dev_err(hba->dev,
> > > > > "%s: Regulator capability was not set,
> > > > > actvIccLevel=%d",
> > > > > __func__,
> > > > > icc_level);
> > > > > --
> > > > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> > > > > Forum,
> > > > > a Linux Foundation Collaborative Project
> > > > >
RE: canfdtest on flexcan loopback
> -Original Message- > From: Marc Kleine-Budde > Sent: 2020年9月16日 20:01 > To: Vladimir Oltean > Cc: w...@grandegger.com; Pankaj Bansal ; Pankaj > Bansal (OSS) ; linux-...@vger.kernel.org; > Joakim Zhang ; linux-kernel@vger.kernel.org; > Vladimir Oltean > Subject: Re: canfdtest on flexcan loopback > > On 9/16/20 1:45 PM, Vladimir Oltean wrote: > > On Wed, Sep 16, 2020 at 01:32:49PM +0200, Marc Kleine-Budde wrote: > >> Which driver are you using? The mainline driver only uses one TX buffer. > > > > Are there multiple flexcan drivers in circulation? Yes, the mainline > > driver with a single priv->tx_mb. > > I assume nxp has several patches on their kernels. Are you using the mainline > kernel or the one that's provided by nxp? Hi Marc, Vladimir, Yes, Vladimir uses kernel provided by NXP, I also help try to look into this issue, but it can't be reproduced on i.MX platforms. Our local flexcan driver is almost cherry-picked from linux-can-next/flexcan branch to implement CAN FD mode, which is a version that cleaned up by you before. I confirm that we still use single TX mailbox to send frames, per my understanding, reorder should not occur here. According to Vladimir's description, exactly it happens: "I have added trace points to the end of the flexcan_start_xmit function, which print the entire skb, and the frames appear to be written to the TX message buffer in the correct order. They are seen, however, in the incorrect order on the wire." Since Vladimir only test Classic mode, he can turn to 5.4 upstream kernel to see if this reorder issue also can be reproduced. @Vladimir Oltean, could you please have a try? To easy the test, I think you only need replace below several files at local side, then use "fsl,ls1021ar2-flexcan" compatible string in dts. https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/net/can/flexcan.c?h=v5.4.65 https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/net/can/rx-offload.c?h=v5.4.65 https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/linux/can/rx-offload.h?h=v5.4.65 If it can't work, suggest to use 5.4 upstream kernel to reproduce this issue. Best Regards, Joakim Zhang > Marc > > -- > Pengutronix e.K. | Marc Kleine-Budde | > Embedded Linux | https://www.pengutronix.de | > Vertretung West/Dortmund | Phone: +49-231-2826-924 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: REGRESSION: 37f4a24c2469: blk-mq: centralise related handling into blk_mq_get_driver_tag
On Wed, Sep 16, 2020 at 04:20:26PM -0400, Theodore Y. Ts'o wrote: > On Wed, Sep 16, 2020 at 07:09:41AM +0800, Ming Lei wrote: > > > The problem is it's a bit tricky to revert 568f27006577, since there > > > is a merge conflict in blk_kick_flush(). I attempted to do the bisect > > > manually here, but it's clearly not right since the kernel is not > > > booting after the revert: > > > > > > https://github.com/tytso/ext4/commit/1e67516382a33da2c9d483b860ac4ec2ad390870 > > > > > > branch: > > > > > > https://github.com/tytso/ext4/tree/manual-revert-of-568f27006577 > > > > > > Can you send me a patch which correctly reverts 568f27006577? I can > > > try it against -rc1 .. -rc4, whichever is most convenient. > > > > Please test the following revert patch against -rc4. > > Unfortunately the results of the revert is... wierd. > > With -rc4, *all* of the VM's are failing --- reliably. With rc4 with > the revert, *some* of the VM's are able to complete the tests, but > over half are still locking up or failing with some kind of oops. So > that seems to imply that there is some kind of timing issue going on, > or maybe there are multiple bugs in play? Obviously there is other more serious issue, since 568f27006577 is completely reverted in your test, and you still see list corruption issue. So I'd suggest to find the big issue first. Once it is fixed, maybe everything becomes fine. .. > > v5.9-rc4 with the revert of 568f27006577: we're seeing a similar > number of VM failures, but the failure signature is different. > The most common failure is: > > [ 390.023691] [ cut here ] > [ 390.028614] list_del corruption, e1c241b00408->next is > LIST_POISON1 (dead0100) > [ 390.037040] WARNING: CPU: 1 PID: 5948 at lib/list_debug.c:47 > __list_del_entry_valid+0x4e/0x90 > [ 390.045684] CPU: 1 PID: 5948 Comm: umount Not tainted > 5.9.0-rc4-xfstests-1-g6fdef015feba #11 > [ 390.054581] Hardware name: Google Google Compute Engine/Google > Compute Engine, BIOS Google 01/01/2011 > [ 390.063943] RIP: 0010:__list_del_entry_valid+0x4e/0x90 > [ 390.069731] Code: 2e 48 8b 32 48 39 fe 75 3a 48 8b 50 08 48 39 f2 75 > 48 b8 01 00 00 00 c3 48 89 fe 48 89 c2 48 c7 c7 10 13 12 9b e8 30 2f 8c ff > <0f> 0b 31 c0 c3 48 89 fe 48 c7 c7 48 13 12 9b e8 1c 2f 8c ff 0f 0b > [ 390.088615] RSP: 0018:ae95c6ddba28 EFLAGS: 00010082 > [ 390.094079] RAX: RBX: ce95bfc007d0 RCX: > 0027 > [ 390.101338] RDX: 0027 RSI: a0c9d93d7dc0 RDI: > a0c9d93d7dc8 > [ 390.108659] RBP: e1c241b00408 R08: 006ba6bff7dc R09: > > [ 390.115925] R10: a0c9d3f444c0 R11: 0046 R12: > a0c9d8041180 > [ 390.123186] R13: a0c86c010e00 R14: e1c241b00400 R15: > a0c9d8042240 > [ 390.130637] FS: 7fb227580080() GS:a0c9d920() > knlGS: > [ 390.138860] CS: 0010 DS: ES: CR0: 80050033 > [ 390.144721] CR2: 7ff72d2dfe74 CR3: 0001cdbb8002 CR4: > 003706e0 > [ 390.152022] DR0: DR1: DR2: > > [ 390.159314] DR3: DR6: fffe0ff0 DR7: > 0400 > [ 390.166569] Call Trace: > [ 390.169148] free_block+0xec/0x270 > [ 390.173100] ___cache_free+0x488/0x6b0 > [ 390.177062] kfree+0xc9/0x1d0 > [ 390.181202] kmem_freepages+0xa0/0xf0 > [ 390.185009] slab_destroy+0x19/0x50 > [ 390.188653] slabs_destroy+0x6d/0x90 > [ 390.192339] ___cache_free+0x4a3/0x6b0 > [ 390.196477] kfree+0xc9/0x1d0 > [ 390.199651] kmem_freepages+0xa0/0xf0 > [ 390.203459] slab_destroy+0x19/0x50 > [ 390.207060] slabs_destroy+0x6d/0x90 > [ 390.210784] ___cache_free+0x4a3/0x6b0 > [ 390.214672] ? lockdep_hardirqs_on_prepare+0xe7/0x180 > [ 390.219845] kfree+0xc9/0x1d0 > [ 390.222928] put_crypt_info+0xe3/0x100 > [ 390.226801] fscrypt_put_encryption_info+0x15/0x30 > [ 390.231721] ext4_clear_inode+0x80/0x90 > [ 390.235774] ext4_evict_inode+0x6d/0x630 > [ 390.239960] evict+0xd0/0x1a0 > [ 390.243049] dispose_list+0x51/0x80 > [ 390.246659] evict_inodes+0x15b/0x1b0 > [ 390.250526] generic_shutdown_super+0x37/0x100 > [ 390.255094] kill_block_super+0x21/0x50 > [ 390.259066] deactivate_locked_super+0x2f/0x70 > [ 390.263638] cleanup_mnt+0xb8/0x140 > [ 390.267248] task_work_run+0x73/0xc0 > [ 390.270953] exit_to_user_mode_prepare+0x197/0x1a0 > [ 390.277333] syscall_exit_to_user_mode+0x3c/0x210 > [ 390.282171] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > [ 390.287348] RIP: 0033:0x7fb2279a6507 > [ 390.291128] Code: 19 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66
