[PATCH] drm/vc4: fix ref count leak in vc4_vec_encoder_enable
in vc4_vec_encoder_enable, the call to pm_runtime_get_sync increments
the counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.
Signed-off-by: Navid Emamdoost
---
drivers/gpu/drm/vc4/vc4_vec.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
index bd5b8eb58b18..706fdc7758f9 100644
--- a/drivers/gpu/drm/vc4/vc4_vec.c
+++ b/drivers/gpu/drm/vc4/vc4_vec.c
@@ -406,7 +406,7 @@ static void vc4_vec_encoder_enable(struct drm_encoder
*encoder)
ret = pm_runtime_get_sync(&vec->pdev->dev);
if (ret < 0) {
DRM_ERROR("Failed to retain power domain: %d\n", ret);
- return;
+ goto out;
}
/*
@@ -419,13 +419,13 @@ static void vc4_vec_encoder_enable(struct drm_encoder
*encoder)
ret = clk_set_rate(vec->clock, 10800);
if (ret) {
DRM_ERROR("Failed to set clock rate: %d\n", ret);
- return;
+ goto out;
}
ret = clk_prepare_enable(vec->clock);
if (ret) {
DRM_ERROR("Failed to turn on core clock: %d\n", ret);
- return;
+ goto out;
}
/* Reset the different blocks */
@@ -464,6 +464,8 @@ static void vc4_vec_encoder_enable(struct drm_encoder
*encoder)
VEC_WRITE(VEC_DAC_MISC,
VEC_DAC_MISC_VID_ACT | VEC_DAC_MISC_DAC_RST_N);
VEC_WRITE(VEC_CFG, VEC_CFG_VEC_EN);
+out:
+ pm_runtime_put(&vec->pdev->dev);
}
--
2.17.1
Re: [PATCH] Fix null pointer dereference in vector_user_bpf
On 14/06/2020 02:19, Gaurav Singh wrote:
The bpf_prog is being checked for !NULL after uml_kmalloc
but later its used directly for example:
bpf_prog->filter = bpf and is also later returned upon
success. Fix this, do a NULL check and return right away.
Signed-off-by: Gaurav Singh
---
arch/um/drivers/vector_user.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/um/drivers/vector_user.c b/arch/um/drivers/vector_user.c
index c4a0f26b2824..0e6d6717bf73 100644
--- a/arch/um/drivers/vector_user.c
+++ b/arch/um/drivers/vector_user.c
@@ -789,10 +789,12 @@ void *uml_vector_user_bpf(char *filename)
return false;
}
bpf_prog = uml_kmalloc(sizeof(struct sock_fprog), UM_GFP_KERNEL);
- if (bpf_prog != NULL) {
- bpf_prog->len = statbuf.st_size / sizeof(struct sock_filter);
- bpf_prog->filter = NULL;
+ if (bpf_prog == NULL) {
+ printk(KERN_ERR "Failed to allocate bpf prog buffer");
+ return NULL;
}
+ bpf_prog->len = statbuf.st_size / sizeof(struct sock_filter);
+ bpf_prog->filter = NULL;
ffd = os_open_file(filename, of_read(OPENFLAGS()), 0);
if (ffd < 0) {
printk(KERN_ERR "Error %d opening bpf file", -errno);
Acked-By: Anton Ivanov
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
[PATCH] kernel/trace: Remove function callback casts
In an effort to enable -Wcast-function-type in the top-level Makefile to
support Control Flow Integrity builds, remove all the function callback
casts.
To do this, use the ftrace_ops_list_func function as a wrapper when the
arch not supports ftrace ops instead of the use of a function cast.
Signed-off-by: Oscar Carter
---
kernel/trace/ftrace.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index c163c3531faf..ed1efc0e3a25 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -119,13 +119,12 @@ struct ftrace_ops __rcu *ftrace_ops_list __read_mostly =
&ftrace_list_end;
ftrace_func_t ftrace_trace_function __read_mostly = ftrace_stub;
struct ftrace_ops global_ops;
-#if ARCH_SUPPORTS_FTRACE_OPS
static void ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
struct ftrace_ops *op, struct pt_regs *regs);
-#else
+
+#if !ARCH_SUPPORTS_FTRACE_OPS
/* See comment below, where ftrace_ops_list_func is defined */
static void ftrace_ops_no_ops(unsigned long ip, unsigned long parent_ip);
-#define ftrace_ops_list_func ((ftrace_func_t)ftrace_ops_no_ops)
#endif
static inline void ftrace_ops_init(struct ftrace_ops *ops)
@@ -6860,6 +6859,12 @@ static void ftrace_ops_list_func(unsigned long ip,
unsigned long parent_ip,
}
NOKPROBE_SYMBOL(ftrace_ops_list_func);
#else
+static void ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
+struct ftrace_ops *op, struct pt_regs *regs)
+{
+ ftrace_ops_no_ops(ip, parent_ip);
+}
+
static void ftrace_ops_no_ops(unsigned long ip, unsigned long parent_ip)
{
__ftrace_ops_list_func(ip, parent_ip, NULL, NULL);
--
2.20.1
[PATCH] drm/amdgpu/display: fix ref count leak when pm_runtime_get_sync fails
The call to pm_runtime_get_sync increments the counter even in case of
failure, leading to incorrect ref count.
In case of failure, decrement the ref count before returning.
Signed-off-by: Navid Emamdoost
---
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index f355d9a752d2..a1aec205435d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -716,8 +716,10 @@ amdgpu_connector_lvds_detect(struct drm_connector
*connector, bool force)
if (!drm_kms_helper_is_poll_worker()) {
r = pm_runtime_get_sync(connector->dev->dev);
- if (r < 0)
+ if (r < 0) {
+ pm_runtime_put_autosuspend(connector->dev->dev);
return connector_status_disconnected;
+ }
}
if (encoder) {
@@ -854,8 +856,10 @@ amdgpu_connector_vga_detect(struct drm_connector
*connector, bool force)
if (!drm_kms_helper_is_poll_worker()) {
r = pm_runtime_get_sync(connector->dev->dev);
- if (r < 0)
+ if (r < 0) {
+ pm_runtime_put_autosuspend(connector->dev->dev);
return connector_status_disconnected;
+ }
}
encoder = amdgpu_connector_best_single_encoder(connector);
@@ -977,8 +981,10 @@ amdgpu_connector_dvi_detect(struct drm_connector
*connector, bool force)
if (!drm_kms_helper_is_poll_worker()) {
r = pm_runtime_get_sync(connector->dev->dev);
- if (r < 0)
+ if (r < 0) {
+ pm_runtime_put_autosuspend(connector->dev->dev);
return connector_status_disconnected;
+ }
}
if (!force && amdgpu_connector_check_hpd_status_unchanged(connector)) {
@@ -1328,8 +1334,10 @@ amdgpu_connector_dp_detect(struct drm_connector
*connector, bool force)
if (!drm_kms_helper_is_poll_worker()) {
r = pm_runtime_get_sync(connector->dev->dev);
- if (r < 0)
+ if (r < 0) {
+ pm_runtime_put_autosuspend(connector->dev->dev);
return connector_status_disconnected;
+ }
}
if (!force && amdgpu_connector_check_hpd_status_unchanged(connector)) {
--
2.17.1
[PATCH] drm/amdgpu: fix ref count leak in amdgpu_display_crtc_set_config
in amdgpu_display_crtc_set_config, the call to pm_runtime_get_sync
increments the counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.
Signed-off-by: Navid Emamdoost
---
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index f7143d927b6d..5e51f0acf744 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -282,7 +282,7 @@ int amdgpu_display_crtc_set_config(struct drm_mode_set *set,
ret = pm_runtime_get_sync(dev->dev);
if (ret < 0)
- return ret;
+ goto out;
ret = drm_crtc_helper_set_config(set, ctx);
@@ -297,7 +297,7 @@ int amdgpu_display_crtc_set_config(struct drm_mode_set *set,
take the current one */
if (active && !adev->have_disp_power_ref) {
adev->have_disp_power_ref = true;
- return ret;
+ goto out;
}
/* if we have no active crtcs, then drop the power ref
we got before */
@@ -306,6 +306,7 @@ int amdgpu_display_crtc_set_config(struct drm_mode_set *set,
adev->have_disp_power_ref = false;
}
+out:
/* drop the power reference we got coming in here */
pm_runtime_put_autosuspend(dev->dev);
return ret;
--
2.17.1
[PATCH] smp: Fix a potential usage of stale nr_cpus
The get_option() maybe return 0, it means that the nr_cpus is
not initialized. Then we will use the stale nr_cpus to initialize
the nr_cpu_ids. So fix it.
Signed-off-by: Muchun Song
---
kernel/smp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/smp.c b/kernel/smp.c
index 472c2b274c65..2a9a04acf123 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -634,8 +634,7 @@ static int __init nrcpus(char *str)
{
int nr_cpus;
- get_option(&str, &nr_cpus);
- if (nr_cpus > 0 && nr_cpus < nr_cpu_ids)
+ if (get_option(&str, &nr_cpus) && nr_cpus > 0 && nr_cpus < nr_cpu_ids)
nr_cpu_ids = nr_cpus;
return 0;
--
2.11.0
Re: [PATCH] media: bdisp: fix reference count leaks due to pm_runtime_get_sync
Hi Aditya,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v5.7 next-20200614]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Aditya-Pakki/media-bdisp-fix-reference-count-leaks-due-to-pm_runtime_get_sync/20200614-115114
base: git://linuxtv.org/media_tree.git master
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project
cb5072d1877b38c972f95092db2cedbcddb81da6)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> drivers/media/platform/sti/bdisp/bdisp-v4l2.c:1403:1: warning: unused label
>> 'err_dbg' [-Wunused-label]
err_dbg:
^~~~
1 warning generated.
vim +/err_dbg +1403 drivers/media/platform/sti/bdisp/bdisp-v4l2.c
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1284
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1285 static int bdisp_probe(struct
platform_device *pdev)
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1286 {
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1287 struct bdisp_dev *bdisp;
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1288 struct resource *res;
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1289 struct device *dev =
&pdev->dev;
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1290 int ret;
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1291
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1292 dev_dbg(dev, "%s\n",
__func__);
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1293
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1294 bdisp =
devm_kzalloc(dev, sizeof(struct bdisp_dev), GFP_KERNEL);
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1295 if (!bdisp)
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1296 return -ENOMEM;
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1297
2732bb765f14eb Fabien Dessenne 2018-05-15 1298 ret =
dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
2732bb765f14eb Fabien Dessenne 2018-05-15 1299 if (ret)
2732bb765f14eb Fabien Dessenne 2018-05-15 1300 return ret;
2732bb765f14eb Fabien Dessenne 2018-05-15 1301
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1302 bdisp->pdev = pdev;
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1303 bdisp->dev = dev;
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1304
platform_set_drvdata(pdev, bdisp);
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1305
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1306 if (dev->of_node)
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1307 bdisp->id =
of_alias_get_id(pdev->dev.of_node, BDISP_NAME);
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1308 else
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1309 bdisp->id =
pdev->id;
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1310
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1311
init_waitqueue_head(&bdisp->irq_queue);
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1312
INIT_DELAYED_WORK(&bdisp->timeout_work, bdisp_irq_timeout);
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1313 bdisp->work_queue =
create_workqueue(BDISP_NAME);
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1314
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1315
spin_lock_init(&bdisp->slock);
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1316
mutex_init(&bdisp->lock);
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1317
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1318 /* get resources */
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1319 res =
platform_get_resource(pdev, IORESOURCE_MEM, 0);
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1320 bdisp->regs =
devm_ioremap_resource(dev, res);
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1321 if
(IS_ERR(bdisp->regs)) {
28ffeebbb7bdc0 Fabien Dessenne 2015-05-12 1322 dev_err(dev,
"failed to get regs\n");
8ea1c5af489a76 Chuhong Yuan2019-11-13 1323 ret =
PTR_ERR(bdisp->regs);
8ea1c5af489a76 Chuhong Yuan2019-11-13 1324 goto err_wq;
28ffeebbb7bdc0 Fabien Dessenne 2015-0
[PATCH] drm/amdgpu: fix ref count leak in amdgpu_driver_open_kms
in amdgpu_driver_open_kms the call to pm_runtime_get_sync increments the
counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.
Signed-off-by: Navid Emamdoost
---
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index d7e17e34fee1..bd40aa307462 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -991,7 +991,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct
drm_file *file_priv)
r = pm_runtime_get_sync(dev->dev);
if (r < 0)
- return r;
+ goto pm_put;
fpriv = kzalloc(sizeof(*fpriv), GFP_KERNEL);
if (unlikely(!fpriv)) {
@@ -1042,6 +1042,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct
drm_file *file_priv)
out_suspend:
pm_runtime_mark_last_busy(dev->dev);
+pm_put:
pm_runtime_put_autosuspend(dev->dev);
return r;
--
2.17.1
Re: [PATCH v2] proc/fd: Remove unnecessary variable initialisations in seq_show()
On Fri, Jun 12, 2020 at 06:45:57PM +0200, Markus Elfring wrote:
> > 'files' will be immediately reassigned. 'f_flags' and 'file' will be
> > overwritten in the if{} or seq_show() directly exits with an error.
> > so we don't need to consume CPU resources to initialize them.
>
> I suggest to improve also this change description.
>
> * Should the mentioned identifiers refer to variables?
>
> * Will another imperative wording be preferred?
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=b791d1bdf9212d944d749a5c7ff6febdba241771#n151
>
> * I propose to extend the patch a bit more.
> How do you think about to convert the initialisation for the variable “ret”
> also into a later assignment?
>
> Regards,
> Markus
Hi,
This is the semi-friendly patch-bot of Greg Kroah-Hartman.
Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list. I strongly suggest that you not do this anymore. Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.
Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all. The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback. Please feel free to also ignore emails
from them.
thanks,
greg k-h's patch email bot
Re: [PATCH] Fix code style in css_task_iter_next_css_set()
On Fri, Jun 12, 2020 at 09:56:26PM +0200, Markus Elfring wrote: > > One line similar code before in this function > > I suggest to improve the commit message. > How do you think about a wording variant like the following? > >Combine two assignments for the variable “l” into one statement. > > Regards, > Markus Hi, This is the semi-friendly patch-bot of Greg Kroah-Hartman. Markus, you seem to have sent a nonsensical or otherwise pointless review comment to a patch submission on a Linux kernel developer mailing list. I strongly suggest that you not do this anymore. Please do not bother developers who are actively working to produce patches and features with comments that, in the end, are a waste of time. Patch submitter, please ignore Markus's suggestion; you do not need to follow it at all. The person/bot/AI that sent it is being ignored by almost all Linux kernel maintainers for having a persistent pattern of behavior of producing distracting and pointless commentary, and inability to adapt to feedback. Please feel free to also ignore emails from them. thanks, greg k-h's patch email bot
Re: [PATCH] ethernet: Fix memory leak in ethoc_probe()
On Sat, Jun 13, 2020 at 08:26:12AM +0200, Markus Elfring wrote: > > … The patch fixes this issue. > > I propose to replace this information by the tag “Fixes”. > Please choose another imperative wording for your change description. > > Regards, > Markus Hi, This is the semi-friendly patch-bot of Greg Kroah-Hartman. Markus, you seem to have sent a nonsensical or otherwise pointless review comment to a patch submission on a Linux kernel developer mailing list. I strongly suggest that you not do this anymore. Please do not bother developers who are actively working to produce patches and features with comments that, in the end, are a waste of time. Patch submitter, please ignore Markus's suggestion; you do not need to follow it at all. The person/bot/AI that sent it is being ignored by almost all Linux kernel maintainers for having a persistent pattern of behavior of producing distracting and pointless commentary, and inability to adapt to feedback. Please feel free to also ignore emails from them. thanks, greg k-h's patch email bot
Re: [PATCH] NFC: Fix error handling in rawsock_connect()
On Sat, Jun 13, 2020 at 07:56:36AM +0200, Markus Elfring wrote: > > … The patch fixes this issue. > > I suggest to replace this information by the tag “Fixes”. > Please choose another imperative wording for your change description. > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=df2fbf5bfa0e7fff8b4784507e4d68f200454318#n151 > > Regards, > Markus Hi, This is the semi-friendly patch-bot of Greg Kroah-Hartman. Markus, you seem to have sent a nonsensical or otherwise pointless review comment to a patch submission on a Linux kernel developer mailing list. I strongly suggest that you not do this anymore. Please do not bother developers who are actively working to produce patches and features with comments that, in the end, are a waste of time. Patch submitter, please ignore Markus's suggestion; you do not need to follow it at all. The person/bot/AI that sent it is being ignored by almost all Linux kernel maintainers for having a persistent pattern of behavior of producing distracting and pointless commentary, and inability to adapt to feedback. Please feel free to also ignore emails from them. thanks, greg k-h's patch email bot
Re: [PATCH] rocker: Fix error handling in dma_rings_init()
On Sat, Jun 13, 2020 at 08:14:13AM +0200, Markus Elfring wrote: > > … The patch fixes the > > order consistent with cleanup in rocker_dma_rings_fini(). > > I suggest to choose another imperative wording for your change description. > Will the tag “Fixes” become helpful for the commit message? > > Regards, > Markus Hi, This is the semi-friendly patch-bot of Greg Kroah-Hartman. Markus, you seem to have sent a nonsensical or otherwise pointless review comment to a patch submission on a Linux kernel developer mailing list. I strongly suggest that you not do this anymore. Please do not bother developers who are actively working to produce patches and features with comments that, in the end, are a waste of time. Patch submitter, please ignore Markus's suggestion; you do not need to follow it at all. The person/bot/AI that sent it is being ignored by almost all Linux kernel maintainers for having a persistent pattern of behavior of producing distracting and pointless commentary, and inability to adapt to feedback. Please feel free to also ignore emails from them. thanks, greg k-h's patch email bot
[PATCH] drm/amd/display: fix ref count leak in amdgpu_drm_ioctl
in amdgpu_drm_ioctl the call to pm_runtime_get_sync increments the counter even in case of failure, leading to incorrect ref count. In case of failure, decrement the ref count before returning. Signed-off-by: Navid Emamdoost --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 126e74758a34..d73924e35a57 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -1373,11 +1373,12 @@ long amdgpu_drm_ioctl(struct file *filp, dev = file_priv->minor->dev; ret = pm_runtime_get_sync(dev->dev); if (ret < 0) - return ret; + goto out; ret = drm_ioctl(filp, cmd, arg); pm_runtime_mark_last_busy(dev->dev); +out: pm_runtime_put_autosuspend(dev->dev); return ret; } -- 2.17.1
Re: [PATCH] RDMA/rvt: Improve exception handling in rvt_create_qp()
On Sat, Jun 13, 2020 at 09:15:12AM +0200, Markus Elfring wrote:
> > … The patch fixes this issue by
> > calling rvt_free_rq().
>
> I suggest to choose another imperative wording for your change description.
> Will the tag “Fixes” become helpful for the commit message?
>
> …
> > +++ b/drivers/infiniband/sw/rdmavt/qp.c
> > @@ -1203,6 +1203,7 @@ struct ib_qp *rvt_create_qp(struct ib_pd *ibpd,
> > qp->s_flags = RVT_S_SIGNAL_REQ_WR;
> > err = alloc_ud_wq_attr(qp, rdi->dparms.node);
> > if (err) {
> > + rvt_free_rq(&qp->r_rq);
> > ret = (ERR_PTR(err));
> > goto bail_driver_priv;
> > }
>
> How do you think about the following code variant with the addition
> of a jump target?
>
> err = alloc_ud_wq_attr(qp, rdi->dparms.node);
> if (err) {
> ret = (ERR_PTR(err));
> - goto bail_driver_priv;
> + goto bail_free_rq;
> }
>
> …
>
> bail_rq_wq:
> - rvt_free_rq(&qp->r_rq);
> free_ud_wq_attr(qp);
> +
> +bail_free_rq:
> + rvt_free_rq(&qp->r_rq);
>
> bail_driver_priv:
>
>
> Regards,
> Markus
Hi,
This is the semi-friendly patch-bot of Greg Kroah-Hartman.
Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list. I strongly suggest that you not do this anymore. Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.
Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all. The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback. Please feel free to also ignore emails
from them.
thanks,
greg k-h's patch email bot
Re: [PATCH] test_objagg: Fix memory leak in test_hints_case()
On Sat, Jun 13, 2020 at 08:36:36AM +0200, Markus Elfring wrote: > > … The patch fixes this issue. > > I propose to replace this information by the tag “Fixes”. > Please choose another imperative wording for your change description. > > Regards, > Markus Hi, This is the semi-friendly patch-bot of Greg Kroah-Hartman. Markus, you seem to have sent a nonsensical or otherwise pointless review comment to a patch submission on a Linux kernel developer mailing list. I strongly suggest that you not do this anymore. Please do not bother developers who are actively working to produce patches and features with comments that, in the end, are a waste of time. Patch submitter, please ignore Markus's suggestion; you do not need to follow it at all. The person/bot/AI that sent it is being ignored by almost all Linux kernel maintainers for having a persistent pattern of behavior of producing distracting and pointless commentary, and inability to adapt to feedback. Please feel free to also ignore emails from them. thanks, greg k-h's patch email bot
Re: [PATCH] powerpc/powernv/pci: add ifdef to avoid dead code
Hi,
Le 14/06/2020 à 07:54, Greg Thelen a écrit :
Commit dc3d8f85bb57 ("powerpc/powernv/pci: Re-work bus PE
configuration") removed a couple pnv_ioda_setup_bus_dma() calls. The
only remaining calls are behind CONFIG_IOMMU_API. Thus builds without
CONFIG_IOMMU_API see:
arch/powerpc/platforms/powernv/pci-ioda.c:1888:13: error:
'pnv_ioda_setup_bus_dma' defined but not used
Add CONFIG_IOMMU_API ifdef guard to avoid dead code.
I think you should move the function down into the same ifdef as the
callers instead of adding a new ifdef.
Christophe
Fixes: dc3d8f85bb57 ("powerpc/powernv/pci: Re-work bus PE configuration")
Signed-off-by: Greg Thelen
---
arch/powerpc/platforms/powernv/pci-ioda.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
b/arch/powerpc/platforms/powernv/pci-ioda.c
index 73a63efcf855..f7762052b7c4 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1885,6 +1885,7 @@ static bool pnv_pci_ioda_iommu_bypass_supported(struct
pci_dev *pdev,
return false;
}
+#ifdef CONFIG_IOMMU_API
static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe, struct pci_bus
*bus)
{
struct pci_dev *dev;
@@ -1897,6 +1898,7 @@ static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe
*pe, struct pci_bus *bus)
pnv_ioda_setup_bus_dma(pe, dev->subordinate);
}
}
+#endif
static inline __be64 __iomem *pnv_ioda_get_inval_reg(struct pnv_phb *phb,
bool real_mode)
[PATCH v16 0/1] mt8183 dpi support pin mode swap
Changes since v15: - Fix YAML License to (GPL-2.0-only OR BSD-2-Clause). - "dt-bindings: display: mediatek: control dpi pins mode to avoid leakage" "drm/mediatek: set dpi pin mode to gpio low to avoid leakage current" applied v15. The links are https://patchwork.kernel.org/patch/11489545/ https://patchwork.kernel.org/patch/11489577/ Changes since v14: - add "Acked-by" and "Reviewed-by" - change port@0 to port in yaml Changes since v13: - move dpi dual edge patches to another series because it will have long time to implement the dual edge change base boris patches. https://patchwork.kernel.org/cover/11354279/ Changes since v12: - fix mediatek,dpi.yaml make_dt_binding_check errors. Change since v11: - fine tune mediatek,dpi.yaml. - add Acked-by: Rob Herring . Change since v10: - convert the Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt to yaml format. - read the pclk-sample in endpoint. Changes since v9: - rename pinctrl-names = "gpiomode", "dpimode" to "active", "idle". - fix some typo. Changes since v8: - drop pclk-sample redefine in mediatek,dpi.txt - only get the gpiomode and dpimode when dpi->pinctrl is successful. Changes since v7: - separate dt-bindings to independent patches. - move dpi dual edge to one patch. Changes since v6: - change dual_edge to pclk-sample - remove dpi_pin_mode_swap and Changes since v5: - fine tune the dt-bindings commit message. Changes since v4: - move pin mode control and dual edge control to deveice tree. - update dt-bindings document for pin mode swap and dual edge control. Changes since v3: - add dpi pin mode control when dpi on or off. - update dpi dual edge comment. Changes since v2: - update dt-bindings document for mt8183 dpi. - separate dual edge modfication as independent patch. Jitao Shi (1): dt-bindings: display: mediatek: convert the dpi bindings to yaml .../display/mediatek/mediatek,dpi.txt | 42 .../display/mediatek/mediatek,dpi.yaml| 97 +++ 2 files changed, 97 insertions(+), 42 deletions(-) delete mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml -- 2.25.1
[PATCH v16 1/1] dt-bindings: display: mediatek: convert the dpi bindings to yaml
Convert display/mediatek/mediatek,dpi.txt to display/mediatek/mediatek,dpi.yaml
and remove the old text bindings.
Signed-off-by: Jitao Shi
---
.../display/mediatek/mediatek,dpi.txt | 42
.../display/mediatek/mediatek,dpi.yaml| 97 +++
2 files changed, 97 insertions(+), 42 deletions(-)
delete mode 100644
Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt
create mode 100644
Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
diff --git
a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt
b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt
deleted file mode 100644
index 77def4456706..
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt
+++ /dev/null
@@ -1,42 +0,0 @@
-Mediatek DPI Device
-===
-
-The Mediatek DPI function block is a sink of the display subsystem and
-provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 pixel data on a parallel
-output bus.
-
-Required properties:
-- compatible: "mediatek,-dpi"
- the supported chips are mt2701 , mt8173 and mt8183.
-- reg: Physical base address and length of the controller's registers
-- interrupts: The interrupt signal from the function block.
-- clocks: device clocks
- See Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
-- clock-names: must contain "pixel", "engine", and "pll"
-- port: Output port node with endpoint definitions as described in
- Documentation/devicetree/bindings/graph.txt. This port should be connected
- to the input port of an attached HDMI or LVDS encoder chip.
-
-Optional properties:
-- pinctrl-names: Contain "default" and "sleep".
-
-Example:
-
-dpi0: dpi@1401d000 {
- compatible = "mediatek,mt8173-dpi";
- reg = <0 0x1401d000 0 0x1000>;
- interrupts = ;
- clocks = <&mmsys CLK_MM_DPI_PIXEL>,
-<&mmsys CLK_MM_DPI_ENGINE>,
-<&apmixedsys CLK_APMIXED_TVDPLL>;
- clock-names = "pixel", "engine", "pll";
- pinctrl-names = "default", "sleep";
- pinctrl-0 = <&dpi_pin_func>;
- pinctrl-1 = <&dpi_pin_idle>;
-
- port {
- dpi0_out: endpoint {
- remote-endpoint = <&hdmi0_in>;
- };
- };
-};
diff --git
a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
new file mode 100644
index ..860b719b5dc9
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/mediatek/mediatek,dpi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: mediatek DPI Controller Device Tree Bindings
+
+maintainers:
+ - CK Hu
+ - Jitao shi
+
+description: |
+ The Mediatek DPI function block is a sink of the display subsystem and
+ provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 pixel data on a parallel
+ output bus.
+
+properties:
+ compatible:
+enum:
+ - mediatek,mt2701-dpi
+ - mediatek,mt8173-dpi
+ - mediatek,mt8183-dpi
+
+ reg:
+maxItems: 1
+
+ interrupts:
+maxItems: 1
+
+ clocks:
+items:
+ - description: Pixel Clock
+ - description: Engine Clock
+ - description: DPI PLL
+
+ clock-names:
+items:
+ - const: pixel
+ - const: engine
+ - const: pll
+
+ pinctrl-0: true
+ pinctrl-1: true
+
+ pinctrl-names:
+items:
+ - const: default
+ - const: sleep
+
+ port:
+type: object
+description:
+ Output port node with endpoint definitions as described in
+ Documentation/devicetree/bindings/graph.txt. This port should be
connected
+ to the input port of an attached HDMI or LVDS encoder chip.
+
+properties:
+ endpoint:
+type: object
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - port
+
+additionalProperties: false
+
+examples:
+ - |
+#include
+#include
+#include
+#include
+dpi0: dpi@1401d000 {
+compatible = "mediatek,mt8173-dpi";
+reg = <0 0x1401d000 0 0x1000>;
+interrupts = ;
+clocks = <&mmsys CLK_MM_DPI_PIXEL>,
+ <&mmsys CLK_MM_DPI_ENGINE>,
+ <&apmixedsys CLK_APMIXED_TVDPLL>;
+clock-names = "pixel", "engine", "pll";
+pinctrl-names = "default", "sleep";
+pinctrl-0 = <&dpi_pin_func>;
+pinctrl-1 = <&dpi_pin_idle>;
+
+port {
+dpi0_out: endpoint {
+remote-endpoint = <&hdmi0_in>;
+};
+};
+};
+
+...
--
2.25.1
Re: [Cocci] coccinelle issues
On Sat, 13 Jun 2020, Randy Dunlap wrote: > Hi, > > OK, I've not used Coccinelle and now I am trying to use it. > It seems that I am having a few issues. > The primary one is when I run spatch (via 'make coccicheck' in > the kernel source tree), it tells me: > > Python error: No module named coccilib.elems > > I do see "elems.py" in /usr/local/lib64/coccinelle/python/coccilib. > > I am using coccinelle-master downloaded from github on > Friday June 12, 2020. > > > I have also made the following notes while building/installing it. > > > Note1: The latest coccinelle tarball is not actually available > at the coccinelle home page although the kernel documentation says it is. Yes, I'm aware of this problem. We're not able to update the home page at the moment. This problem is being worked on. > > Note2: https://github.com/coccinelle/coccinelle/blob/master/install.txt > says that 'spatch' is a script, but it seems to be a binary executable > file. Actually, it is a script, and the fact that you say it is a binary may be the reason for your python problem. Normally there is a script (scripts/spatch) that make install puts in place that refers back to where your Coccinelle is installed. > Note3: https://github.com/coccinelle/coccinelle/blob/master/install.txt > probably should say to use 'sudo make install' instead of just > 'make install', just like 'coccinelle.rst' file in the kernel tree says. OK. A lot of documentation for a lot of projects seems to omit the sudo, but I have indeed never understood why. Maybe try again with make distclean, ./autogen, ./configure, sudo make install? julia > > thanks for any help that you can provide. > > -- > ~Randy > > ___ > Cocci mailing list > co...@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci >
Re: [PATCH 1/3] thermal/cpu-cooling, sched/core: Cleanup thermal pressure definition
Hi Valentin,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on tip/auto-latest]
[also build test WARNING on driver-core/driver-core-testing tip/sched/core
arm/for-next arm64/for-next/core soc/for-next linus/master v5.7 next-20200613]
[cannot apply to linux/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Valentin-Schneider/sched-arch_topology-Thermal-pressure-configuration-cleanup/20200614-091051
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
8dc697d75c13ee2901d1a40f1d7d58163048c204
config: arm64-randconfig-r013-20200614 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project
cb5072d1877b38c972f95092db2cedbcddb81da6)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> drivers/base/arch_topology.c:59:6: warning: no previous prototype for
>> function 'arch_set_thermal_pressure' [-Wmissing-prototypes]
void arch_set_thermal_pressure(const struct cpumask *cpus,
^
drivers/base/arch_topology.c:59:1: note: declare 'static' if the function is
not intended to be used outside of this translation unit
void arch_set_thermal_pressure(const struct cpumask *cpus,
^
static
1 warning generated.
vim +/arch_set_thermal_pressure +59 drivers/base/arch_topology.c
58
> 59 void arch_set_thermal_pressure(const struct cpumask *cpus,
60 unsigned long th_pressure)
61 {
62 int cpu;
63
64 for_each_cpu(cpu, cpus)
65 WRITE_ONCE(per_cpu(thermal_pressure, cpu), th_pressure);
66 }
67
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: general protection fault in syscall_return_slowpath
On Tue, Mar 10, 2020 at 9:10 AM Dmitry Vyukov wrote: > > On Tue, Mar 10, 2020 at 7:15 AM Nathan Chancellor > wrote: > > > > On Mon, Mar 09, 2020 at 09:20:58AM +0100, Dmitry Vyukov wrote: > > > On Sun, Mar 8, 2020 at 7:35 PM 'Jann Horn' via syzkaller-bugs > > > wrote: > > > > > > > > On Sun, Mar 8, 2020 at 5:40 PM syzbot > > > > wrote: > > > > > HEAD commit:63623fd4 Merge tag 'for-linus' of > > > > > git://git.kernel.org/pub.. > > > > > git tree: upstream > > > > > console output: > > > > > https://syzkaller.appspot.com/x/log.txt?x=16cfeac3e0 > > > > > kernel config: > > > > > https://syzkaller.appspot.com/x/.config?x=5d2e033af114153f > > > > > dashboard link: > > > > > https://syzkaller.appspot.com/bug?extid=cd66e43794b178bb5cd6 > > > > > compiler: clang version 10.0.0 > > > > > (https://github.com/llvm/llvm-project/ > > > > > c2443155a0fb245c8f17f2c1c72b6ea391e86e81) > > > > > syz repro: > > > > > https://syzkaller.appspot.com/x/repro.syz?x=12a42329e0 > > > > > > > > > > IMPORTANT: if you fix the bug, please add the following tag to the > > > > > commit: > > > > > Reported-by: syzbot+cd66e43794b178bb5...@syzkaller.appspotmail.com > > > > > > > > > > general protection fault, probably for non-canonical address > > > > > 0x11255a6b: [#1] PREEMPT SMP KASAN > > > > > CPU: 0 PID: 8742 Comm: syz-executor.2 Not tainted 5.6.0-rc3-syzkaller > > > > > #0 > > > > > Hardware name: Google Google Compute Engine/Google Compute Engine, > > > > > BIOS Google 01/01/2011 > > > > > RIP: 0010:arch_local_irq_disable arch/x86/include/asm/paravirt.h:757 > > > > > [inline] > > > > > RIP: 0010:syscall_return_slowpath+0xeb/0x4a0 > > > > > arch/x86/entry/common.c:277 > > > > > Code: 00 10 0f 85 de 00 00 00 e8 b2 a3 76 00 48 c7 c0 58 d3 2a 89 48 > > > > > c1 e8 03 80 3c 18 00 74 0c 48 c7 c7 58 d3 2a 89 e8 05 00 00 00 <00> > > > > > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > > > > RSP: 0018:c900020a7ed0 EFLAGS: 00010246 > > > > > RAX: 11255a6b RBX: dc00 RCX: 88808c512380 > > > > > RDX: RSI: RDI: > > > > > RBP: c900020a7f10 R08: 810075bb R09: fbfff14d9182 > > > > > R10: fbfff14d9182 R11: R12: 1110118a2470 > > > > > R13: 4000 R14: 88808c512380 R15: 88808c512380 > > > > > FS: 0154f940() GS:8880aea0() > > > > > knlGS: > > > > > CS: 0010 DS: ES: CR0: 80050033 > > > > > CR2: 0076c000 CR3: a6b05000 CR4: 001406f0 > > > > > DR0: DR1: DR2: > > > > > DR3: DR6: fffe0ff0 DR7: 0400 > > > > > Call Trace: > > > > > do_syscall_64+0x11f/0x1c0 arch/x86/entry/common.c:304 > > > > > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > > > > BUG: kernel NULL pointer dereference, address: > > > > > #PF: supervisor write access in kernel mode > > > > > #PF: error_code(0x0002) - not-present page > > > > > PGD 8fecc067 P4D 8fecc067 PUD 97953067 PMD 0 > > > > > Oops: 0002 [#2] PREEMPT SMP KASAN > > > > > CPU: 0 PID: 8742 Comm: syz-executor.2 Not tainted 5.6.0-rc3-syzkaller > > > > > #0 > > > > > > > > Ugh, why does it build with -Werror... > > > > There are certain warnings that are specifically treated like errors: > > > > In the main Makefile: > > > > KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types) > > > > > Now I am realizing I don't know what's the proper way to turn off > > > warnings entirely... > > > > > > We turn off this CONFIG_ERROR_ON_WARNING historically: > > > https://github.com/google/syzkaller/blob/2e9971bbbfb4df6ba0118353163a7703f3dbd6ec/dashboard/config/bits-syzbot.config#L17 > > > and I thought that's enough. But now I realize it's not even a thing. > > > I see it referenced in some ChromeOS threads and there are some > > > discussions re upstreaming, but apparently it never existed upstream. > > > > > > make has W=n, but it seems that it can only be used to produce more > > > warnings. We don't pass W=3 specifically and there is no W=0. > > > > > > Should we always build with CFLAGS=-w? Is it guaranteed to work? Or is > > > there a better way? > > > > Would passing -Wno-werror via KCFLAGS work? Otherwise, passing > > -Wno-error= should work. > > > > Cheers, > > Nathan > > Filed https://github.com/google/syzkaller/issues/1635 so that this is not > lost. Jann, Getting back to this. Are you sure building without warning will be better? Currently make enables these warnings as errors only: -Werror=strict-prototypes -Werror=implicit-function-declaration -Werror=implicit-int -Werror=date-time -Werror=incompatible-pointer-types -Werror=designated-init So most warnings won't cause build failure. And, say, converting T* to Y* implicitly may be an actual bug in the patch. The other concern is that some people may use s
Re: [PATCH] net: fec: fix ref count leaking when pm_runtime_get_sync fails
> in fec_enet_mdio_read, …
I am curious under which circumstances you would like to improve
such commit messages.
* Will the tag “Fixes” become helpful?
* Which source code analysis tools did trigger to send
update suggestions according to 16 similar issues for today?
…
> +++ b/drivers/net/ethernet/freescale/fec_main.c
…
> @@ -1893,8 +1895,10 @@ static int fec_enet_mdio_write(struct mii_bus *bus,
> int mii_id, int regnum,
> bool is_c45 = !!(regnum & MII_ADDR_C45);
>
> ret = pm_runtime_get_sync(dev);
> - if (ret < 0)
> + if (ret < 0) {
> + pm_runtime_put_autosuspend(dev);
> return ret;
> + }
> else
> ret = 0;
I suggest to adjust also the source code from the else branch.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=96144c58abe7ff767e754b5b80995f7b8846d49b#n196
…
> @@ -2276,6 +2280,7 @@ static void fec_enet_get_regs(struct net_device *ndev,
> }
>
> pm_runtime_mark_last_busy(dev);
> +out:
> pm_runtime_put_autosuspend(dev);
> }
Perhaps use the label “put_runtime” instead?
Regards,
Markus
Re: [PATCH] usb: dwc3: pci: Fix reference count leak in dwc3_pci_resume_work
Hello!
On 14.06.2020 6:15, Aditya Pakki wrote:
dwc3_pci_resume_work() calls pm_runtime_get_sync() that increments
the reference counter. In case of failure, decrement the reference
count and return the error.
In this case you still return nothing.
Signed-off-by: Aditya Pakki
---
drivers/usb/dwc3/dwc3-pci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index b67372737dc9..96c05b121fac 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -206,8 +206,10 @@ static void dwc3_pci_resume_work(struct work_struct *work)
int ret;
ret = pm_runtime_get_sync(&dwc3->dev);
- if (ret)
+ if (ret) {
+ pm_runtime_put_sync_autosuspend(&dwc3->dev);
return;
+ }
pm_runtime_mark_last_busy(&dwc3->dev);
pm_runtime_put_sync_autosuspend(&dwc3->dev);
MBR, Sergei
Re: [PATCH] usb: musb: fix reference count leak in musb_irq_work
On 14.06.2020 6:27, Aditya Pakki wrote:
musb_irq_work() calls pm_runtime_get_sync() that increments
the reference counter. In case of failure, decrement the reference
count and return the error.
Again, *void* function, so no error returned.
Signed-off-by: Aditya Pakki
---
drivers/usb/musb/musb_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 384a8039a7fd..fd36a026bef0 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2070,6 +2070,7 @@ static void musb_irq_work(struct work_struct *data)
error = pm_runtime_get_sync(musb->controller);
if (error < 0) {
dev_err(musb->controller, "Could not enable: %i\n", error);
+ pm_runtime_put_autosuspend(musb->controller);
return;
}
MBR, Sergei
Re: [PATCH] mtd: rawnand: brcmnand: force raw OOB writes
Hi Kamal,
> El 13 jun 2020, a las 17:16, Kamal Dasu escribió:
>
> Alvaro,
>
>
> On Sat, Jun 13, 2020 at 5:01 AM Álvaro Fernández Rojas
> wrote:
>>
>> Hi Kamal,
>>
>>> El 12 jun 2020, a las 20:47, Kamal Dasu escribió:
>>>
>>> On Fri, Jun 5, 2020 at 1:07 PM Álvaro Fernández Rojas
>>> wrote:
MTD_OPS_AUTO_OOB is writting OOB with ECC enabled, which changes all ECC
bytes
from an erased page to 0x00 when JFFS2 cleanmarkers are added with
mtd-utils.
| BBI | JFFS2 | ECC | JFFS2 | Spare |
0800 ff ff 19 85 20 03 00 00 00 00 00 00 08 ff ff ff
However, if OOB is written with ECC disabled, the JFFS2 cleanmarkers are
correctly written without changing the ECC bytes:
| BBI | JFFS2 | ECC | JFFS2 | Spare |
0800 ff ff 19 85 20 03 ff ff ff 00 00 00 08 ff ff ff
>>>
>>> Both brcmand_write_oob_raw() and brcmnand_write_oob() use
>>> brcmnand_write() that uses PROGRAM_PAGE cmd, means also programs data
>>> areas and ECC when enabled is always calculated on DATA+OOB. since
>>
>> Are you sure about that? I think that HW ECC is only calculated for DATA,
>> not for OOB.
>> The fact that we’re not writing any DATA seems to be the problem that is
>> changing all ECC bytes to 0x00.
>>
>
> Only true for 1-bit hamming code on early controller versions. For
> BCH algorithms both data+oob are covered. And the controller driver
> intentionally does not implement a spare area program cmd, even for
> OOB writes. Also BRCMNAND_ACC_CONTROL_PARTIAL_PAGE=0 (when present)
> does not allow for spare areas only writes.
>
>>> in both cases we only want to modify OOB, data is always assumed to be
>>> 0xffs (means erased state). So as far as this api always is used on
>>> the erased page it should be good. Are the sub-pages/oob areas read by
>>> jffs2 also read without ecc enabled?. Just want to be sure that it
>>> does not break any other utilities like nandwrite.
>>
>> No, sub-pages/oob areas read by JFFS2 with ECC enabled.
>> I don’t think this breaks anything at all, since I tested nandwrite with OOB
>> enabled and everything was written perfectly.
>>
>
> Going by the commit message you are assuming 1-bit hamming code, that
> is the only exception on early version controllers where only data was
> covered for ecc calculations when enabled.
> Which version of the controller are you using ?. Did you test with
> BCH-4 or any BCH ?.
All my devices use hamming ECC, even the ones with controller version v4.0
(BCM63268), which should also support BCH.
Therefore I need to stick with hamming ECC if I want the bootloader to be able
to boot the kernel.
However, I should get a new device with BCH in a few days.
I’ll do more tests once I get it, but if BCH also covers OOB, then we could
conditionally force write_oob to RAW only if hamming is configured.
>
>> BTW, I tried another approach that forced MTD_OPS_AUTO_OOB to be written as
>> raw OOB, but it was rejected:
>> http://patchwork.ozlabs.org/project/linux-mtd/patch/20200504094253.2741109-1-nolt...@gmail.com/
>> https://lkml.org/lkml/2020/5/4/215
>>
>
> Are there any other use cases other than jffs2 where only oob needs to
> be modified ?. But surely intentionally disabling ecc instead of
> forcing it is the way, but I am not sure it will still work for other
> BCH algorithms though where both data and oob are covered.
I’ll test this and report back once I get my new device.
>
>>>
Signed-off-by: Álvaro Fernández Rojas
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 8f9ffb46a09f..566281770841 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -2279,13 +2279,6 @@ static int brcmnand_write_page_raw(struct nand_chip
*chip, const uint8_t *buf,
return nand_prog_page_end_op(chip);
}
-static int brcmnand_write_oob(struct nand_chip *chip, int page)
-{
- return brcmnand_write(nand_to_mtd(chip), chip,
- (u64)page << chip->page_shift, NULL,
- chip->oob_poi);
-}
-
static int brcmnand_write_oob_raw(struct nand_chip *chip, int page)
{
struct mtd_info *mtd = nand_to_mtd(chip);
@@ -2642,7 +2635,7 @@ static int brcmnand_init_cs(struct brcmnand_host
*host, struct device_node *dn)
chip->ecc.write_oob_raw = brcmnand_write_oob_raw;
chip->ecc.read_oob_raw = brcmnand_read_oob_raw;
chip->ecc.read_oob = brcmnand_read_oob;
- chip->ecc.write_oob = brcmnand_write_oob;
+ chip->ecc.write_oob = brcmnand_write_oob_raw;
chip->controller = &ctrl->controller;
--
2.26.2
Re: [PATCH] net: macb: fix ref count leaking via pm_runtime_get_sync
> in macb_mdio_write, … * Will a desire evolve to improve also this commit message? * Will the tag “Fixes” become helpful? … > +++ b/drivers/net/ethernet/cadence/macb_main.c … > @@ -3840,11 +3842,14 @@ static int at91ether_open(struct net_device *dev) > > ret = macb_phylink_connect(lp); > if (ret) > - return ret; > + goto out; > > netif_start_queue(dev); > > return 0; > +out: > + pm_runtime_put(&lp->pdev->dev); > + return ret; > } … Perhaps use the label “put_runtime” instead? Regards, Markus
Re: [PATCH 1/3] thermal/cpu-cooling, sched/core: Cleanup thermal pressure definition
Hi Valentin,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on tip/auto-latest]
[also build test WARNING on driver-core/driver-core-testing tip/sched/core
arm/for-next arm64/for-next/core soc/for-next linus/master v5.7 next-20200613]
[cannot apply to linux/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Valentin-Schneider/sched-arch_topology-Thermal-pressure-configuration-cleanup/20200614-091051
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
8dc697d75c13ee2901d1a40f1d7d58163048c204
config: riscv-allyesconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> drivers/base/arch_topology.c:59:6: warning: no previous prototype for
>> 'arch_set_thermal_pressure' [-Wmissing-prototypes]
59 | void arch_set_thermal_pressure(const struct cpumask *cpus,
| ^
vim +/arch_set_thermal_pressure +59 drivers/base/arch_topology.c
58
> 59 void arch_set_thermal_pressure(const struct cpumask *cpus,
60 unsigned long th_pressure)
61 {
62 int cpu;
63
64 for_each_cpu(cpu, cpus)
65 WRITE_ONCE(per_cpu(thermal_pressure, cpu), th_pressure);
66 }
67
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
[PATCH v5 0/9] bmips: add bcm6345 reset controller support
BCM63xx SoCs have a reset controller for certain components. v5: fix kbuild robot error (drop __init). v4: fix device tree bindings documentation. v3: using reset-simple isn't possible since sleeping after performing the reset is also needed. Add BCM63268 and BCM6318 support. v2: add compatibility to reset-simple instead of adding a new driver. Álvaro Fernández Rojas (9): mips: bmips: select ARCH_HAS_RESET_CONTROLLER dt-bindings: reset: add BCM6345 reset controller bindings reset: add BCM6345 reset controller driver mips: bmips: dts: add BCM6328 reset controller support mips: bmips: dts: add BCM6358 reset controller support mips: bmips: dts: add BCM6362 reset controller support mips: bmips: dts: add BCM6368 reset controller support mips: bmips: dts: add BCM63268 reset controller support mips: bmips: add BCM6318 reset controller definitions .../bindings/reset/brcm,bcm6345-reset.yaml| 37 + arch/mips/Kconfig | 1 + arch/mips/boot/dts/brcm/bcm63268.dtsi | 6 + arch/mips/boot/dts/brcm/bcm6328.dtsi | 6 + arch/mips/boot/dts/brcm/bcm6358.dtsi | 6 + arch/mips/boot/dts/brcm/bcm6362.dtsi | 6 + arch/mips/boot/dts/brcm/bcm6368.dtsi | 6 + drivers/reset/Kconfig | 7 + drivers/reset/Makefile| 1 + drivers/reset/reset-bcm6345.c | 135 ++ include/dt-bindings/reset/bcm6318-reset.h | 20 +++ include/dt-bindings/reset/bcm63268-reset.h| 26 include/dt-bindings/reset/bcm6328-reset.h | 18 +++ include/dt-bindings/reset/bcm6358-reset.h | 15 ++ include/dt-bindings/reset/bcm6362-reset.h | 22 +++ include/dt-bindings/reset/bcm6368-reset.h | 16 +++ 16 files changed, 328 insertions(+) create mode 100644 Documentation/devicetree/bindings/reset/brcm,bcm6345-reset.yaml create mode 100644 drivers/reset/reset-bcm6345.c create mode 100644 include/dt-bindings/reset/bcm6318-reset.h create mode 100644 include/dt-bindings/reset/bcm63268-reset.h create mode 100644 include/dt-bindings/reset/bcm6328-reset.h create mode 100644 include/dt-bindings/reset/bcm6358-reset.h create mode 100644 include/dt-bindings/reset/bcm6362-reset.h create mode 100644 include/dt-bindings/reset/bcm6368-reset.h -- 2.27.0
[PATCH v5 3/9] reset: add BCM6345 reset controller driver
Add support for resetting blocks through the Linux reset controller
subsystem for BCM63xx SoCs.
Signed-off-by: Álvaro Fernández Rojas
Reviewed-by: Florian Fainelli
---
v5: fix kbuild robot error (drop __init).
v4: no changes.
v3: using reset-simple isn't possible since sleeping after performing the
reset is also needed.
v2: add compatibility to reset-simple instead of adding a new driver.
drivers/reset/Kconfig | 7 ++
drivers/reset/Makefile| 1 +
drivers/reset/reset-bcm6345.c | 135 ++
3 files changed, 143 insertions(+)
create mode 100644 drivers/reset/reset-bcm6345.c
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index d9efbfd29646..9f1da978cef6 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -41,6 +41,13 @@ config RESET_BERLIN
help
This enables the reset controller driver for Marvell Berlin SoCs.
+config RESET_BCM6345
+ bool "BCM6345 Reset Controller"
+ depends on BMIPS_GENERIC || COMPILE_TEST
+ default BMIPS_GENERIC
+ help
+ This enables the reset controller driver for BCM6345 SoCs.
+
config RESET_BRCMSTB
tristate "Broadcom STB reset controller"
depends on ARCH_BRCMSTB || COMPILE_TEST
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 249ed357c997..e642aae42f0f 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_ARCH_TEGRA) += tegra/
obj-$(CONFIG_RESET_A10SR) += reset-a10sr.o
obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
obj-$(CONFIG_RESET_AXS10X) += reset-axs10x.o
+obj-$(CONFIG_RESET_BCM6345) += reset-bcm6345.o
obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
obj-$(CONFIG_RESET_BRCMSTB) += reset-brcmstb.o
obj-$(CONFIG_RESET_BRCMSTB_RESCAL) += reset-brcmstb-rescal.o
diff --git a/drivers/reset/reset-bcm6345.c b/drivers/reset/reset-bcm6345.c
new file mode 100644
index ..e1acb8d9f661
--- /dev/null
+++ b/drivers/reset/reset-bcm6345.c
@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * BCM6345 Reset Controller Driver
+ *
+ * Copyright (C) 2020 Álvaro Fernández Rojas
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define BCM6345_RESET_NUM 32
+#define BCM6345_RESET_SLEEP_MIN_US 1
+#define BCM6345_RESET_SLEEP_MAX_US 2
+
+struct bcm6345_reset {
+ struct reset_controller_dev rcdev;
+ void __iomem *base;
+ spinlock_t lock;
+};
+
+static void bcm6345_reset_update(struct reset_controller_dev *rcdev,
+unsigned long id, bool assert)
+{
+ struct bcm6345_reset *bcm6345_reset =
+ container_of(rcdev, struct bcm6345_reset, rcdev);
+ unsigned long flags;
+ uint32_t val;
+
+ spin_lock_irqsave(&bcm6345_reset->lock, flags);
+ val = __raw_readl(bcm6345_reset->base);
+ if (assert)
+ val &= ~BIT(id);
+ else
+ val |= BIT(id);
+ __raw_writel(val, bcm6345_reset->base);
+ spin_unlock_irqrestore(&bcm6345_reset->lock, flags);
+}
+
+static int bcm6345_reset_assert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ bcm6345_reset_update(rcdev, id, true);
+
+ return 0;
+}
+
+static int bcm6345_reset_deassert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ bcm6345_reset_update(rcdev, id, false);
+
+ return 0;
+}
+
+static int bcm6345_reset_reset(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ bcm6345_reset_update(rcdev, id, true);
+ usleep_range(BCM6345_RESET_SLEEP_MIN_US,
+BCM6345_RESET_SLEEP_MAX_US);
+
+ bcm6345_reset_update(rcdev, id, false);
+ usleep_range(BCM6345_RESET_SLEEP_MIN_US,
+BCM6345_RESET_SLEEP_MAX_US);
+
+ return 0;
+}
+
+static int bcm6345_reset_status(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct bcm6345_reset *bcm6345_reset =
+ container_of(rcdev, struct bcm6345_reset, rcdev);
+
+ return !(__raw_readl(bcm6345_reset->base) & BIT(id));
+}
+
+static struct reset_control_ops bcm6345_reset_ops = {
+ .assert = bcm6345_reset_assert,
+ .deassert = bcm6345_reset_deassert,
+ .reset = bcm6345_reset_reset,
+ .status = bcm6345_reset_status,
+};
+
+static int bcm6345_reset_probe(struct platform_device *pdev)
+{
+ struct bcm6345_reset *bcm6345_reset;
+ struct resource *res;
+ int err;
+
+ bcm6345_reset = devm_kzalloc(&pdev->dev,
+sizeof(*bcm6345_reset), GFP_KERNEL);
+ if (!bcm6345_reset)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, bcm6345_reset);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ bcm6345_reset->base = devm_ioremap_resource(&pdev->dev, res);
+
[PATCH v5 2/9] dt-bindings: reset: add BCM6345 reset controller bindings
Add device tree binding documentation for BCM6345 reset controller.
Signed-off-by: Álvaro Fernández Rojas
Reviewed-by: Florian Fainelli
---
v5: no changes
v4: change license and fix maxItems.
v3: no changes
v2: no changes
.../bindings/reset/brcm,bcm6345-reset.yaml| 37 +++
1 file changed, 37 insertions(+)
create mode 100644
Documentation/devicetree/bindings/reset/brcm,bcm6345-reset.yaml
diff --git a/Documentation/devicetree/bindings/reset/brcm,bcm6345-reset.yaml
b/Documentation/devicetree/bindings/reset/brcm,bcm6345-reset.yaml
new file mode 100644
index ..560cf6522cb8
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/brcm,bcm6345-reset.yaml
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/reset/brcm,bcm6345-reset.yaml#";
+$schema: "http://devicetree.org/meta-schemas/core.yaml#";
+
+title: BCM6345 reset controller
+
+description: This document describes the BCM6345 reset controller.
+
+maintainers:
+ - Álvaro Fernández Rojas
+
+properties:
+ compatible:
+const: brcm,bcm6345-reset
+
+ reg:
+maxItems: 1
+
+ "#reset-cells":
+const: 1
+
+required:
+ - compatible
+ - reg
+ - "#reset-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+reset-controller@1010 {
+ compatible = "brcm,bcm6345-reset";
+ reg = <0x1010 0x4>;
+ #reset-cells = <1>;
+};
--
2.27.0
[PATCH v5 6/9] mips: bmips: dts: add BCM6362 reset controller support
BCM6362 SoCs have a reset controller for certain components.
Signed-off-by: Álvaro Fernández Rojas
Acked-by: Florian Fainelli
---
v5: no changes.
v4: no changes.
v3: add reset controller definitions header file.
v2: no changes.
arch/mips/boot/dts/brcm/bcm6362.dtsi | 6 ++
include/dt-bindings/reset/bcm6362-reset.h | 22 ++
2 files changed, 28 insertions(+)
create mode 100644 include/dt-bindings/reset/bcm6362-reset.h
diff --git a/arch/mips/boot/dts/brcm/bcm6362.dtsi
b/arch/mips/boot/dts/brcm/bcm6362.dtsi
index 8ae6981735b8..443af6b4c838 100644
--- a/arch/mips/boot/dts/brcm/bcm6362.dtsi
+++ b/arch/mips/boot/dts/brcm/bcm6362.dtsi
@@ -70,6 +70,12 @@ reboot: syscon-reboot@1008 {
mask = <0x1>;
};
+ periph_rst: reset-controller@1010 {
+ compatible = "brcm,bcm6345-reset";
+ reg = <0x1010 0x4>;
+ #reset-cells = <1>;
+ };
+
periph_intc: interrupt-controller@1020 {
compatible = "brcm,bcm6345-l1-intc";
reg = <0x1020 0x10>,
diff --git a/include/dt-bindings/reset/bcm6362-reset.h
b/include/dt-bindings/reset/bcm6362-reset.h
new file mode 100644
index ..7ebb0546e0ab
--- /dev/null
+++ b/include/dt-bindings/reset/bcm6362-reset.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef __DT_BINDINGS_RESET_BCM6362_H
+#define __DT_BINDINGS_RESET_BCM6362_H
+
+#define BCM6362_RST_SPI0
+#define BCM6362_RST_IPSEC 1
+#define BCM6362_RST_EPHY 2
+#define BCM6362_RST_SAR3
+#define BCM6362_RST_ENETSW 4
+#define BCM6362_RST_USBD 5
+#define BCM6362_RST_USBH 6
+#define BCM6362_RST_PCM7
+#define BCM6362_RST_PCIE_CORE 8
+#define BCM6362_RST_PCIE 9
+#define BCM6362_RST_PCIE_EXT 10
+#define BCM6362_RST_WLAN_SHIM 11
+#define BCM6362_RST_DDR_PHY12
+#define BCM6362_RST_FAP13
+#define BCM6362_RST_WLAN_UBUS 14
+
+#endif /* __DT_BINDINGS_RESET_BCM6362_H */
--
2.27.0
[PATCH v5 8/9] mips: bmips: dts: add BCM63268 reset controller support
BCM63268 SoCs have a reset controller for certain components.
Signed-off-by: Álvaro Fernández Rojas
Acked-by: Florian Fainelli
---
v5: no changes.
v4: no changes.
v3: add new path with BCM63268 reset controller support.
arch/mips/boot/dts/brcm/bcm63268.dtsi | 6 +
include/dt-bindings/reset/bcm63268-reset.h | 26 ++
2 files changed, 32 insertions(+)
create mode 100644 include/dt-bindings/reset/bcm63268-reset.h
diff --git a/arch/mips/boot/dts/brcm/bcm63268.dtsi
b/arch/mips/boot/dts/brcm/bcm63268.dtsi
index beec24145af7..0150da7e3905 100644
--- a/arch/mips/boot/dts/brcm/bcm63268.dtsi
+++ b/arch/mips/boot/dts/brcm/bcm63268.dtsi
@@ -70,6 +70,12 @@ reboot: syscon-reboot@1008 {
mask = <0x1>;
};
+ periph_rst: reset-controller@1010 {
+ compatible = "brcm,bcm6345-reset";
+ reg = <0x1010 0x4>;
+ #reset-cells = <1>;
+ };
+
periph_intc: interrupt-controller@1020 {
compatible = "brcm,bcm6345-l1-intc";
reg = <0x1020 0x20>,
diff --git a/include/dt-bindings/reset/bcm63268-reset.h
b/include/dt-bindings/reset/bcm63268-reset.h
new file mode 100644
index ..6a6403a4c2d5
--- /dev/null
+++ b/include/dt-bindings/reset/bcm63268-reset.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef __DT_BINDINGS_RESET_BCM63268_H
+#define __DT_BINDINGS_RESET_BCM63268_H
+
+#define BCM63268_RST_SPI 0
+#define BCM63268_RST_IPSEC 1
+#define BCM63268_RST_EPHY 2
+#define BCM63268_RST_SAR 3
+#define BCM63268_RST_ENETSW4
+#define BCM63268_RST_USBS 5
+#define BCM63268_RST_USBH 6
+#define BCM63268_RST_PCM 7
+#define BCM63268_RST_PCIE_CORE 8
+#define BCM63268_RST_PCIE 9
+#define BCM63268_RST_PCIE_EXT 10
+#define BCM63268_RST_WLAN_SHIM 11
+#define BCM63268_RST_DDR_PHY 12
+#define BCM63268_RST_FAP0 13
+#define BCM63268_RST_WLAN_UBUS 14
+#define BCM63268_RST_DECT 15
+#define BCM63268_RST_FAP1 16
+#define BCM63268_RST_PCIE_HARD 17
+#define BCM63268_RST_GPHY 18
+
+#endif /* __DT_BINDINGS_RESET_BCM63268_H */
--
2.27.0
[PATCH v5 7/9] mips: bmips: dts: add BCM6368 reset controller support
BCM6368 SoCs have a reset controller for certain components.
Signed-off-by: Álvaro Fernández Rojas
Acked-by: Florian Fainelli
---
v5: no changes.
v4: no changes.
v3: add reset controller definitions header file.
v2: no changes.
arch/mips/boot/dts/brcm/bcm6368.dtsi | 6 ++
include/dt-bindings/reset/bcm6368-reset.h | 16
2 files changed, 22 insertions(+)
create mode 100644 include/dt-bindings/reset/bcm6368-reset.h
diff --git a/arch/mips/boot/dts/brcm/bcm6368.dtsi
b/arch/mips/boot/dts/brcm/bcm6368.dtsi
index 449c167dd892..52c19f40b9cc 100644
--- a/arch/mips/boot/dts/brcm/bcm6368.dtsi
+++ b/arch/mips/boot/dts/brcm/bcm6368.dtsi
@@ -70,6 +70,12 @@ reboot: syscon-reboot@1008 {
mask = <0x1>;
};
+ periph_rst: reset-controller@1010 {
+ compatible = "brcm,bcm6345-reset";
+ reg = <0x1010 0x4>;
+ #reset-cells = <1>;
+ };
+
periph_intc: interrupt-controller@1020 {
compatible = "brcm,bcm6345-l1-intc";
reg = <0x1020 0x10>,
diff --git a/include/dt-bindings/reset/bcm6368-reset.h
b/include/dt-bindings/reset/bcm6368-reset.h
new file mode 100644
index ..c81d8eb6d173
--- /dev/null
+++ b/include/dt-bindings/reset/bcm6368-reset.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef __DT_BINDINGS_RESET_BCM6368_H
+#define __DT_BINDINGS_RESET_BCM6368_H
+
+#define BCM6368_RST_SPI0
+#define BCM6368_RST_MPI3
+#define BCM6368_RST_IPSEC 4
+#define BCM6368_RST_EPHY 6
+#define BCM6368_RST_SAR7
+#define BCM6368_RST_SWITCH 10
+#define BCM6368_RST_USBD 11
+#define BCM6368_RST_USBH 12
+#define BCM6368_RST_PCM13
+
+#endif /* __DT_BINDINGS_RESET_BCM6368_H */
--
2.27.0
[PATCH v5 5/9] mips: bmips: dts: add BCM6358 reset controller support
BCM6358 SoCs have a reset controller for certain components.
Signed-off-by: Álvaro Fernández Rojas
Acked-by: Florian Fainelli
---
v5: no changes.
v4: no changes.
v3: add reset controller definitions header file.
v2: no changes.
arch/mips/boot/dts/brcm/bcm6358.dtsi | 6 ++
include/dt-bindings/reset/bcm6358-reset.h | 15 +++
2 files changed, 21 insertions(+)
create mode 100644 include/dt-bindings/reset/bcm6358-reset.h
diff --git a/arch/mips/boot/dts/brcm/bcm6358.dtsi
b/arch/mips/boot/dts/brcm/bcm6358.dtsi
index f21176cac038..9d93e7f5e6fc 100644
--- a/arch/mips/boot/dts/brcm/bcm6358.dtsi
+++ b/arch/mips/boot/dts/brcm/bcm6358.dtsi
@@ -82,6 +82,12 @@ periph_intc: interrupt-controller@fffe000c {
interrupts = <2>, <3>;
};
+ periph_rst: reset-controller@fffe0034 {
+ compatible = "brcm,bcm6345-reset";
+ reg = <0xfffe0034 0x4>;
+ #reset-cells = <1>;
+ };
+
leds0: led-controller@fffe00d0 {
#address-cells = <1>;
#size-cells = <0>;
diff --git a/include/dt-bindings/reset/bcm6358-reset.h
b/include/dt-bindings/reset/bcm6358-reset.h
new file mode 100644
index ..bda62ef84f5a
--- /dev/null
+++ b/include/dt-bindings/reset/bcm6358-reset.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef __DT_BINDINGS_RESET_BCM6358_H
+#define __DT_BINDINGS_RESET_BCM6358_H
+
+#define BCM6358_RST_SPI0
+#define BCM6358_RST_ENET 2
+#define BCM6358_RST_MPI3
+#define BCM6358_RST_EPHY 6
+#define BCM6358_RST_SAR7
+#define BCM6358_RST_USBH 12
+#define BCM6358_RST_PCM13
+#define BCM6358_RST_ADSL 14
+
+#endif /* __DT_BINDINGS_RESET_BCM6358_H */
--
2.27.0
[PATCH v5 1/9] mips: bmips: select ARCH_HAS_RESET_CONTROLLER
This allows to add reset controllers support. Signed-off-by: Álvaro Fernández Rojas Acked-by: Florian Fainelli --- v5: no changes v4: no changes v3: no changes v2: no changes arch/mips/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 6fee1a133e9d..b1840119cb64 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -227,6 +227,7 @@ config ATH79 config BMIPS_GENERIC bool "Broadcom Generic BMIPS kernel" + select ARCH_HAS_RESET_CONTROLLER select ARCH_HAS_SYNC_DMA_FOR_CPU_ALL select ARCH_HAS_PHYS_TO_DMA select BOOT_RAW -- 2.27.0
[PATCH v5 9/9] mips: bmips: add BCM6318 reset controller definitions
BCM6318 SoCs have a reset controller for certain components. Signed-off-by: Álvaro Fernández Rojas Acked-by: Florian Fainelli --- v5: no changes. v4: no changes. v3: add new path with BCM6318 reset controller definitions. include/dt-bindings/reset/bcm6318-reset.h | 20 1 file changed, 20 insertions(+) create mode 100644 include/dt-bindings/reset/bcm6318-reset.h diff --git a/include/dt-bindings/reset/bcm6318-reset.h b/include/dt-bindings/reset/bcm6318-reset.h new file mode 100644 index ..f4fef7bfb06d --- /dev/null +++ b/include/dt-bindings/reset/bcm6318-reset.h @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ + +#ifndef __DT_BINDINGS_RESET_BCM6318_H +#define __DT_BINDINGS_RESET_BCM6318_H + +#define BCM6318_RST_SPI0 +#define BCM6318_RST_EPHY 1 +#define BCM6318_RST_SAR2 +#define BCM6318_RST_ENETSW 3 +#define BCM6318_RST_USBD 4 +#define BCM6318_RST_USBH 5 +#define BCM6318_RST_PCIE_CORE 6 +#define BCM6318_RST_PCIE 7 +#define BCM6318_RST_PCIE_EXT 8 +#define BCM6318_RST_PCIE_HARD 9 +#define BCM6318_RST_ADSL 10 +#define BCM6318_RST_PHYMIPS11 +#define BCM6318_RST_HOSTMIPS 11 + +#endif /* __DT_BINDINGS_RESET_BCM6318_H */ -- 2.27.0
[PATCH v5 4/9] mips: bmips: dts: add BCM6328 reset controller support
BCM6328 SoCs have a reset controller for certain components.
Signed-off-by: Álvaro Fernández Rojas
Acked-by: Florian Fainelli
---
v5: no changes.
v4: no changes.
v3: add reset controller definitions header file.
v2: no changes.
arch/mips/boot/dts/brcm/bcm6328.dtsi | 6 ++
include/dt-bindings/reset/bcm6328-reset.h | 18 ++
2 files changed, 24 insertions(+)
create mode 100644 include/dt-bindings/reset/bcm6328-reset.h
diff --git a/arch/mips/boot/dts/brcm/bcm6328.dtsi
b/arch/mips/boot/dts/brcm/bcm6328.dtsi
index af860d06def6..590118cf5c12 100644
--- a/arch/mips/boot/dts/brcm/bcm6328.dtsi
+++ b/arch/mips/boot/dts/brcm/bcm6328.dtsi
@@ -57,6 +57,12 @@ clkctl: clock-controller@1004 {
#clock-cells = <1>;
};
+ periph_rst: reset-controller@1010 {
+ compatible = "brcm,bcm6345-reset";
+ reg = <0x1010 0x4>;
+ #reset-cells = <1>;
+ };
+
periph_intc: interrupt-controller@1020 {
compatible = "brcm,bcm6345-l1-intc";
reg = <0x1020 0x10>,
diff --git a/include/dt-bindings/reset/bcm6328-reset.h
b/include/dt-bindings/reset/bcm6328-reset.h
new file mode 100644
index ..0f3df87d47af
--- /dev/null
+++ b/include/dt-bindings/reset/bcm6328-reset.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef __DT_BINDINGS_RESET_BCM6328_H
+#define __DT_BINDINGS_RESET_BCM6328_H
+
+#define BCM6328_RST_SPI0
+#define BCM6328_RST_EPHY 1
+#define BCM6328_RST_SAR2
+#define BCM6328_RST_ENETSW 3
+#define BCM6328_RST_USBS 4
+#define BCM6328_RST_USBH 5
+#define BCM6328_RST_PCM6
+#define BCM6328_RST_PCIE_CORE 7
+#define BCM6328_RST_PCIE 8
+#define BCM6328_RST_PCIE_EXT 9
+#define BCM6328_RST_PCIE_HARD 10
+
+#endif /* __DT_BINDINGS_RESET_BCM6328_H */
--
2.27.0
Re: [patch V9 10/39] x86/entry: Provide helpers for execute on irqstack
Qian Cai writes: > On Sat, Jun 13, 2020 at 04:03:02PM +0200, Thomas Gleixner wrote: >> Qian, >> >> Qian Cai writes: >> > On Wed, Jun 10, 2020 at 09:38:56PM +0200, Thomas Gleixner wrote: >> > >> > Thomas, I get ahold of one of the affected systems now if you want some >> > testing there. >> >> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/entry > > Looks good. > > > # cat /sys/kernel/debug/stackdepot/info > Unique stacks: 27481 > Depot index: 285 > Depot offset: 3024 > > > # cat /sys/kernel/debug/stackdepot/info > Unique stacks: 29242 > Depot index: 308 > Depot offset: 6608 > > > # cat /sys/kernel/debug/stackdepot/info > Unique stacks: 49221 > Depot index: 530 > Depot offset: 5888 Thanks for confirming!
Re: [Cocci] [PATCH] coccinelle: api: add kvfree script
On 6/5/20 11:51 PM, Julia Lawall wrote:
> Also, there is no need to exceed 80 characters here. You can put a
> newline in the middle of a \( ... \)
It's required. Looks like it's impossible to break "when" lines.
... when != if (...) { ... E =
\(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\|kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|kvmalloc_array\)(...);
... }
Thanks,
Denis
RFC: a failing pm_runtime_get increases the refcnt?
Hi Linux-PM, both in the I2C subsystem and also for Renesas drivers I maintain, I am starting to get boilerplate patches doing some pm_runtime_put_* variant because a failing pm_runtime_get is supposed to increase the ref counters? Really? This feels wrong and unintuitive to me. I expect there has been a discussion around it but I couldn't find it. I wonder why we don't fix the code where the incremented refcount is expected for some reason. Can I have some pointers please? Thanks, Wolfram signature.asc Description: PGP signature
Re: [PATCH] i2c: xiic: Fix reference count leaks.
On Sat, Jun 13, 2020 at 04:59:23PM -0500, wu000...@umn.edu wrote: > From: Qiushi Wu > > pm_runtime_get_sync() increments the runtime PM usage counter even > when it returns an error code. Thus call pm_runtime_put_noidle() > if pm_runtime_get_sync() fails. Can you point me to a discussion where it was decided that this is a proper fix? I'd think we rather should fix pm_runtime_get_sync() but maybe there are technical reasons against it. signature.asc Description: PGP signature
Re: [PATCH] [v3] i2c: imx-lpi2c: Fix runtime PM imbalance on error
On Mon, Jun 01, 2020 at 02:16:40PM +0800, Dinghao Liu wrote: > pm_runtime_get_sync() increments the runtime PM usage counter even > the call returns an error code. Thus a corresponding decrement is > needed on the error handling path to keep the counter balanced. Can you point me to a discussion where it was decided that this is a proper fix? I'd think we rather should fix pm_runtime_get_sync() but maybe there are technical reasons against it. signature.asc Description: PGP signature
Re: [Cocci] [PATCH] coccinelle: api: add kvfree script
On Sun, 14 Jun 2020, Denis Efremov wrote:
>
>
> On 6/5/20 11:51 PM, Julia Lawall wrote:
> > Also, there is no need to exceed 80 characters here. You can put a
> > newline in the middle of a \( ... \)
>
> It's required. Looks like it's impossible to break "when" lines.
That's true. Sorry for the noise.
julia
>
> ... when != if (...) { ... E =
> \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\|kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|kvmalloc_array\)(...);
> ... }
>
> Thanks,
> Denis
>
Re: [PATCH] Input: bma150: fix ref count leak in bma150_open
> in bma150_open, …
* Can the term “reference count” become relevant also for this commit message
besides other possible adjustments?
* Will the tag “Fixes” become helpful?
…
> +++ b/drivers/input/misc/bma150.c
…
> @@ -357,10 +357,13 @@ static int bma150_open(struct input_dev *input)
> if (bma150->mode != BMA150_MODE_NORMAL) {
> error = bma150_set_mode(bma150, BMA150_MODE_NORMAL);
> if (error)
> - return error;
> + goto out;
> }
>
> return 0;
> +out:
> + pm_runtime_put(&bma150->client->dev);
> + return error;
> }
…
Perhaps use the label “put_runtime” instead?
Regards,
Markus
Re: [PATCH 1/3] thermal/cpu-cooling, sched/core: Cleanup thermal pressure definition
Hi Valentin,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on tip/auto-latest]
[also build test WARNING on driver-core/driver-core-testing tip/sched/core
arm/for-next arm64/for-next/core soc/for-next linus/master v5.7 next-20200613]
[cannot apply to linux/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Valentin-Schneider/sched-arch_topology-Thermal-pressure-configuration-cleanup/20200614-091051
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
8dc697d75c13ee2901d1a40f1d7d58163048c204
config: sparc-allyesconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=sparc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> drivers/thermal/cpufreq_cooling.c:421:1: warning: no previous prototype for
>> 'arch_set_thermal_pressure' [-Wmissing-prototypes]
421 | arch_set_thermal_pressure(const struct cpumask *cpus, unsigned long
th_pressure)
| ^
vim +/arch_set_thermal_pressure +421 drivers/thermal/cpufreq_cooling.c
419
420 __weak void
> 421 arch_set_thermal_pressure(const struct cpumask *cpus, unsigned long
> th_pressure)
422 {
423 }
424
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [Cocci] [PATCH] coccinelle: api: add kvfree script
On 6/14/20 12:17 PM, Julia Lawall wrote: > > > On Sun, 14 Jun 2020, Denis Efremov wrote: > >> >> >> On 6/5/20 11:51 PM, Julia Lawall wrote: >>> Also, there is no need to exceed 80 characters here. You can put a >>> newline in the middle of a \( ... \) >> >> It's required. Looks like it's impossible to break "when" lines. > > That's true. Sorry for the noise. > Anyway, I will send v2 with other lines fixed. Thanks, Denis
Re: [PATCH] Input: bma150: fix ref count leak in bma150_open
On Sun, Jun 14, 2020 at 8:58 AM Navid Emamdoost wrote: > > in bma150_open, pm_runtime_get_sync is called which > increments the counter even in case of failure, leading to incorrect > ref count. In case of failure, decrement the ref count before returning. ... > error = pm_runtime_get_sync(&bma150->client->dev); > if (error < 0 && error != -ENOSYS) > - return error; > + goto out; So, what will happen in case of -ENOSYS? ... > + pm_runtime_put(&bma150->client->dev); Slightly better to use _put_noidle(). (More consistency with error path) -- With Best Regards, Andy Shevchenko
RE: [PATCH v1 2/2] scsi: ufs: Add trace event for UIC commands
Acked-by: Avri Altman
>
>
> Hi Avri,
>
> On Sat, 2020-06-13 at 10:48 +, Avri Altman wrote:
> > > +static void ufshcd_add_uic_command_trace(struct ufs_hba *hba,
> > > +struct uic_command *ucmd,
> > > +const char *str)
> > > +{
> > > + u32 cmd;
> > > +
> > > + if (!trace_ufshcd_uic_command_enabled())
> > > + return;
> > > +
> > > + if (!strcmp(str, "uic_send"))
> > > + cmd = ucmd->command;
> > > + else
> > > + cmd = ufshcd_readl(hba, REG_UIC_COMMAND);
> > Why on complete you can't just use ucmd->command as well?
>
> Reading registers is really helpful for debugging to check if host UIC
> command register really received the command before.
>
> But the original requesting UIC command shall be logged in trace so
> ucmd->command is logged during "send", and the command in register is
> read and logged during "completed". Then we could simply check them to
> know if something wrong while sending the command.
>
> This concept is similar as current UTP command trace events that
> doorbell register is read and dumped in the trace.
>
> >
> > > +
> > > + trace_ufshcd_uic_command(dev_name(hba->dev), str, cmd,
> > > +ucmd->result,
> > > +ufshcd_readl(hba, REG_UIC_COMMAND_ARG_1),
> > > +ufshcd_readl(hba, REG_UIC_COMMAND_ARG_2),
> > > +ufshcd_readl(hba,
> > > REG_UIC_COMMAND_ARG_3));
> > Why can't you just use the ucmd members?
> > Why need to read those?
>
> As above same reason, reading registers can know which arguments are
> exactly sent to the device.
>
> This is very helpful for fast issue breakdown if UIC command is not
> responded under expectation.
>
> Here, we also really want to keep the original requesting arguments from
> "ucmd", just like UIC command. However, arguments in register will be
> changed after UIC command is done so we can not do the same way as UIC
> command. So a compromise is here that we logged the arguments which host
> register exactly received in trace.
>
> >
> > > +}
> >
> >
> > > +
> > > static void ufshcd_add_command_trace(struct ufs_hba *hba,
> > > unsigned int tag, const char *str)
> > > {
> > > @@ -2054,6 +2075,8 @@ ufshcd_dispatch_uic_cmd(struct ufs_hba *hba,
> > > struct uic_command *uic_cmd)
> > > /* Write UIC Cmd */
> > > ufshcd_writel(hba, uic_cmd->command &
> COMMAND_OPCODE_MASK,
> > > REG_UIC_COMMAND);
> > > +
> > > + ufshcd_add_uic_command_trace(hba, uic_cmd, "uic_send");
> > > }
> > >
> > > /**
> > > @@ -2080,6 +2103,9 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba,
> > > struct uic_command *uic_cmd)
> > > hba->active_uic_cmd = NULL;
> > > spin_unlock_irqrestore(hba->host->host_lock, flags);
> > >
> > > + uic_cmd->result = ret;
> > > + ufshcd_add_uic_command_trace(hba, uic_cmd, "uic_complete");
> > > +
> > > return ret;
> > > }
> > Can't you just call the "send" and "complete" from ufshcd_send_uic_cmd?
>
> For "send", we would like to log the time as precise as possible so
> "send" event is logged while UIC command is sent.
>
> Thanks so much! Your question reminds me that "send" trace shall be
> moved before UIC command is sent otherwise register value may be changed
> before logging "send" trace. I will fix this in next version.
>
> For "completed", to make logging time as closed to UIC command
> completion as possible, maybe I need to change the logging timing to
> ufshcd_uic_cmd_compl(), just like UTP command completion trace which is
> logged in __ufshcd_transfer_reg_compl().
>
> If you have no objection, I will try to fix this in next version.
>
> >
> >
> > >
> > > @@ -3760,6 +3786,9 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
> *hba,
> > > struct uic_command *cmd)
> > > ret = (status != PWR_OK) ? status : -1;
> > > }
> > > out:
> > > + cmd->result = ret;
> > > + ufshcd_add_uic_command_trace(hba, cmd, "uic_complete");
> > > +
> > > if (ret) {
> > > ufshcd_print_host_state(hba);
> > > ufshcd_print_pwr_info(hba);
> > > diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h
> > > index 5f300739240d..cf8d568d5a13 100644
> > > --- a/include/trace/events/ufs.h
> > > +++ b/include/trace/events/ufs.h
> > > @@ -249,6 +249,39 @@ TRACE_EVENT(ufshcd_command,
> > > )
> > > );
> > >
> > > +TRACE_EVENT(ufshcd_uic_command,
> > > + TP_PROTO(const char *dev_name, const char *str, u32 cmd, int
> result,
> > > +u32 arg1, u32 arg2, u32 arg3),
> > > +
> > > + TP_ARGS(dev_name, str, cmd, result, arg1, arg2, arg3),
> > > +
> > > + TP_STRUCT__entry(
> > > + __string(dev_name, dev_name)
> > > + __string(str, str)
> > > +
Re: [PATCH] Input: stmfts: fix ref count leak in stmfts_input_open
> in stmfts_input_open, … * Can the term “reference count” become relevant also for this commit message besides other possible adjustments? * Would you like to add the tag “Fixes”? … > +++ b/drivers/input/touchscreen/stmfts.c … > @@ -367,6 +367,9 @@ static int stmfts_input_open(struct input_dev *dev) > } > > return 0; > +out: > + pm_runtime_put(&sdata->client->dev); > + return err; > } … Perhaps use the label “put_runtime” instead? Regards, Markus
Re: RFC: a failing pm_runtime_get increases the refcnt?
On Sun, Jun 14, 2020 at 12:10 PM Wolfram Sang wrote: > both in the I2C subsystem and also for Renesas drivers I maintain, I am > starting to get boilerplate patches doing some pm_runtime_put_* variant > because a failing pm_runtime_get is supposed to increase the ref > counters? Really? This feels wrong and unintuitive to me. Yeah, that is a well known issue with PM (I even have for a long time a coccinelle script, when I realized myself that there are a lot of cases like this, but someone else discovered this recently, like opening a can of worms). > I expect there > has been a discussion around it but I couldn't find it. Rafael explained (again) recently this. I can't find it quickly, unfortunately. > I wonder why we > don't fix the code where the incremented refcount is expected for some > reason. The main idea behind API that a lot of drivers do *not* check error codes from runtime PM, so, we need to keep balance in case of pm_runtime_get(...); ... pm_runtime_put(...); > Can I have some pointers please? -- With Best Regards, Andy Shevchenko
Re: [PATCH] hwmon: (ina3221) Fix reference count leak in ina3221_write_enable
On 6/13/20 8:38 PM, Aditya Pakki wrote:
> ina3221_write_enable() calls pm_runtime_get_sync() that increments
> the reference counter. In case of failure, decrement the reference
> count and return the error.
>
> Signed-off-by: Aditya Pakki
> ---
> drivers/hwmon/ina3221.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c
> index f335d0cb0c77..9a3b46160de1 100644
> --- a/drivers/hwmon/ina3221.c
> +++ b/drivers/hwmon/ina3221.c
> @@ -492,6 +492,7 @@ static int ina3221_write_enable(struct device *dev, int
> channel, bool enable)
> ret = pm_runtime_get_sync(ina->pm_dev);
> if (ret < 0) {
> dev_err(dev, "Failed to get PM runtime\n");
> + pm_runtime_put_sync(ina->pm_dev);
> return ret;
> }
> }
>
Looking into other code handling this error, a call to pm_runtime_put_noidle()
may be more appropriate in this situation.
Guenter
Re: RFC: a failing pm_runtime_get increases the refcnt?
On Sun, Jun 14, 2020 at 12:34 PM Andy Shevchenko wrote: > > On Sun, Jun 14, 2020 at 12:10 PM Wolfram Sang wrote: > > both in the I2C subsystem and also for Renesas drivers I maintain, I am > > starting to get boilerplate patches doing some pm_runtime_put_* variant > > because a failing pm_runtime_get is supposed to increase the ref > > counters? Really? This feels wrong and unintuitive to me. > > Yeah, that is a well known issue with PM (I even have for a long time > a coccinelle script, when I realized myself that there are a lot of > cases like this, but someone else discovered this recently, like > opening a can of worms). > > > I expect there > > has been a discussion around it but I couldn't find it. > > Rafael explained (again) recently this. I can't find it quickly, > unfortunately. I _think_ this discussion, but may be it's simple another tentacle of the same octopus. https://patchwork.ozlabs.org/project/linux-tegra/patch/20200520095148.10995-1-dinghao@zju.edu.cn/ > > > I wonder why we > > don't fix the code where the incremented refcount is expected for some > > reason. > > The main idea behind API that a lot of drivers do *not* check error > codes from runtime PM, so, we need to keep balance in case of > > pm_runtime_get(...); > ... > pm_runtime_put(...); > > > Can I have some pointers please? > > -- > With Best Regards, > Andy Shevchenko -- With Best Regards, Andy Shevchenko
Re: [PATCH] drm/etnaviv: fix ref count leak via pm_runtime_get_sync
On Sun, Jun 14, 2020 at 9:48 AM Navid Emamdoost
wrote:
...
> + if (ret < 0) {
> + pm_runtime_put(gpu->dev);
Please, in all your patches fix this to be _put_noidle(). We wouldn't
bear the flag day of fixing these parts again.
Yes, I know that *now* behaviour is the same, but calling put here is
slightly inconsistent.
...
> + pm_runtime_put(gpu->dev);
--
With Best Regards,
Andy Shevchenko
Re: [PATCH] ALSA: usb-audio: add quirk for Denon DCD-1500RE
On Sat, 13 Jun 2020 13:40:06 +0200, Yick W. Tse wrote: > > fix error "clock source 41 is not valid, cannot use" > > > [] New USB device found, idVendor=154e, idProduct=1002, bcdDevice= 1.00 > [] New USB device strings: Mfr=1, Product=2, SerialNumber=0 > [] Product: DCD-1500RE > [] Manufacturer: D & M Holdings Inc. > [] > [] clock source 41 is not valid, cannot use > [] usbcore: registered new interface driver snd-usb-audio > > > Signed-off-by: Yick W. Tse Thanks, applied with Cc to stable. Takashi
Re: [PATCH] i2c: busses: Fix reference count leaks.
On Sun, Jun 14, 2020 at 1:17 AM wrote:
>
> From: Qiushi Wu
>
> pm_runtime_get_sync() increments the runtime PM usage counter even
> when it returns an error code. Thus call pm_runtime_put_noidle()
> if pm_runtime_get_sync() fails.
>
FWIW,
Reviewed-by: Andy Shevchenko
> Signed-off-by: Qiushi Wu
> ---
> drivers/i2c/busses/i2c-img-scb.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-img-scb.c
> b/drivers/i2c/busses/i2c-img-scb.c
> index 98a89301ed2a..e13ca862fa97 100644
> --- a/drivers/i2c/busses/i2c-img-scb.c
> +++ b/drivers/i2c/busses/i2c-img-scb.c
> @@ -1058,8 +1058,10 @@ static int img_i2c_xfer(struct i2c_adapter *adap,
> struct i2c_msg *msgs,
> }
>
> ret = pm_runtime_get_sync(adap->dev.parent);
> - if (ret < 0)
> + if (ret < 0) {
> + pm_runtime_put_noidle(adap->dev.parent);
> return ret;
> + }
>
> for (i = 0; i < num; i++) {
> struct i2c_msg *msg = &msgs[i];
> @@ -1159,8 +1161,10 @@ static int img_i2c_init(struct img_i2c *i2c)
> int ret;
>
> ret = pm_runtime_get_sync(i2c->adap.dev.parent);
> - if (ret < 0)
> + if (ret < 0) {
> + pm_runtime_put_noidle(i2c->adap.dev.parent);
> return ret;
> + }
>
> rev = img_i2c_readl(i2c, SCB_CORE_REV_REG);
> if ((rev & 0x00ff) < 0x00020200) {
> --
> 2.17.1
>
--
With Best Regards,
Andy Shevchenko
RE: [PATCH 1/1] Documentation:sysfs-ufs: Add WriteBooster documentation
> > Adds sysfs documentation for WriteBooster entries. > > Signed-off-by: Asutosh Das Acked-by: Avri Altman Maybe insert each field following the fields of the same descriptor, attributes, flags etc. Thanks, Avri > --- > Documentation/ABI/testing/sysfs-driver-ufs | 136 > + > 1 file changed, 136 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-driver-ufs > b/Documentation/ABI/testing/sysfs-driver-ufs > index 016724e..d1a3521 100644 > --- a/Documentation/ABI/testing/sysfs-driver-ufs > +++ b/Documentation/ABI/testing/sysfs-driver-ufs > @@ -883,3 +883,139 @@ Contact: Subhash Jadavani > > Description: This entry shows the target state of an UFS UIC link > for the chosen system power management level. > The file is read only. > + > +What: > /sys/bus/platform/drivers/ufshcd/*/device_descriptor/wb_presv_us_en > +Date: June 2020 > +Contact: Asutosh Das > +Description: This entry shows if preserve user-space was configured > + The file is read only. > + > +What: > /sys/bus/platform/drivers/ufshcd/*/device_descriptor/wb_shared_alloc_units > +Date: June 2020 > +Contact: Asutosh Das > +Description: This entry shows the shared allocated units of WB buffer > + The file is read only. > + > +What: /sys/bus/platform/drivers/ufshcd/*/device_descriptor/wb_type > +Date: June 2020 > +Contact: Asutosh Das > +Description: This entry shows the configured WB type. > + 0x1 for shared buffer mode. 0x0 for dedicated buffer mode. > + The file is read only. > + > +What: > /sys/bus/platform/drivers/ufshcd/*/geometry_descriptor/wb_buff_cap_adj > +Date: June 2020 > +Contact: Asutosh Das > +Description: This entry shows the total user-space decrease in shared > + buffer mode. > + The value of this parameter is 3 for TLC NAND when SLC mode > + is used as WriteBooster Buffer. 2 for MLC NAND. > + The file is read only. > + > +What: > /sys/bus/platform/drivers/ufshcd/*/geometry_descriptor/wb_max_alloc_unit > s > +Date: June 2020 > +Contact: Asutosh Das > +Description: This entry shows the Maximum total WriteBooster Buffer size > + which is supported by the entire device. > + The file is read only. > + > +What: > /sys/bus/platform/drivers/ufshcd/*/geometry_descriptor/wb_max_wb_luns > +Date: June 2020 > +Contact: Asutosh Das > +Description: This entry shows the maximum number of luns that can > support > + WriteBooster. > + The file is read only. > + > +What: > /sys/bus/platform/drivers/ufshcd/*/geometry_descriptor/wb_sup_red_type > +Date: June 2020 > +Contact: Asutosh Das > +Description: The supportability of user space reduction mode > + and preserve user space mode. > + 00h: WriteBooster Buffer can be configured only in > + user space reduction type. > + 01h: WriteBooster Buffer can be configured only in > + preserve user space type. > + 02h: Device can be configured in either user space > + reduction type or preserve user space type. > + The file is read only. > + > +What: > /sys/bus/platform/drivers/ufshcd/*/geometry_descriptor/wb_sup_wb_type > +Date: June 2020 > +Contact: Asutosh Das > +Description: The supportability of WriteBooster Buffer type. > + 00h: LU based WriteBooster Buffer configuration > + 01h: Single shared WriteBooster Buffer > + configuration > + 02h: Supporting both LU based WriteBooster > + Buffer and Single shared WriteBooster Buffer > + configuration > + The file is read only. > + > +What: /sys/bus/platform/drivers/ufshcd/*/flags/wb_enable > +Date: June 2020 > +Contact: Asutosh Das > +Description: This entry shows the status of WriteBooster. > + 0: WriteBooster is not enabled. > + 1: WriteBooster is enabled > + The file is read only. > + > +What: /sys/bus/platform/drivers/ufshcd/*/flags/wb_flush_en > +Date: June 2020 > +Contact: Asutosh Das > +Description: This entry shows if flush is enabled. > + 0: Flush operation is not performed. > + 1: Flush operation is performed. > + The file is read only. > + > +What: /sys/bus/platform/drivers/ufshcd/*/flags/wb_flush_during_h8 > +Date: June 2020 > +Contact: Asutosh Das > +Description: Flush WriteBooster Buffer during hibernate state. > + 0: Device is not allowed to flush the > + WriteBooster Buffer during link hibernate > + state. > + 1: Device is allowe
Re: next-0519 on thinkpad x60: sound related? window manager crash
On Sat, 13 Jun 2020 18:25:22 +0200, Alex Xu (Hello71) wrote: > > Excerpts from Takashi Iwai's message of June 11, 2020 1:11 pm: > > Thanks, so something still missing in the mmap handling, I guess. > > > > I've worked on two different branches for potential fixes of your > > problems. Could you test topic/dma-fix and topic/dma-fix2 branches? > > git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git > > Just pull one of them onto Linus' git HEAD. > > > > I guess we'll go with David's new patch, but still it's interesting > > whether my changes do anything good actually. > > > > > > Takashi > > > > On torvalds 623f6dc593, topic/dma-fix causes sound to be output as > alternating half-second bursts of noise and a few seconds of silence. > topic/dma-fix2 appears to work properly. OK, thanks for the feedback! Just to make sure, you're using PulseAudio, right? If so, it was still something wrong about mmap, and the secondary method (the fallback to the continuous page) looks like a safer approach in the end. I suppose that David's fix will be merged sooner or later. Meanwhile I'll work on the change in the sound driver side to make things a bit more robust. They don't conflict and both good applicable. thanks, Takashi
Re: RFC: a failing pm_runtime_get increases the refcnt?
Hi Andy,
On Sun, Jun 14, 2020 at 11:43 AM Andy Shevchenko
wrote:
> On Sun, Jun 14, 2020 at 12:34 PM Andy Shevchenko
> wrote:
> >
> > On Sun, Jun 14, 2020 at 12:10 PM Wolfram Sang wrote:
> > > both in the I2C subsystem and also for Renesas drivers I maintain, I am
> > > starting to get boilerplate patches doing some pm_runtime_put_* variant
> > > because a failing pm_runtime_get is supposed to increase the ref
> > > counters? Really? This feels wrong and unintuitive to me.
> >
> > Yeah, that is a well known issue with PM (I even have for a long time
> > a coccinelle script, when I realized myself that there are a lot of
> > cases like this, but someone else discovered this recently, like
> > opening a can of worms).
> >
> > > I expect there
> > > has been a discussion around it but I couldn't find it.
> >
> > Rafael explained (again) recently this. I can't find it quickly,
> > unfortunately.
>
> I _think_ this discussion, but may be it's simple another tentacle of
> the same octopus.
> https://patchwork.ozlabs.org/project/linux-tegra/patch/20200520095148.10995-1-dinghao@zju.edu.cn/
Thanks, hadn't read that one! (so I was still at -1 from
http://sweng.the-davies.net/Home/rustys-api-design-manifesto ;-)
So "pm_runtime_put_noidle()" is the (definitive?) one to pair with a
pm_runtime_get_sync() failure?
> > > I wonder why we
> > > don't fix the code where the incremented refcount is expected for some
> > > reason.
> >
> > The main idea behind API that a lot of drivers do *not* check error
> > codes from runtime PM, so, we need to keep balance in case of
> >
> > pm_runtime_get(...);
> > ...
> > pm_runtime_put(...);
I've always[*] considered a pm_runtime_get_sync() failure to be fatal
(or: cannot happen), and that there's nothing that can be done to
recover. Hence I never checked the function's return value.
Was that wrong?
[*] at least on Renesas SoCs with Clock and/or Power Domains.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: RFC: a failing pm_runtime_get increases the refcnt?
On Sun, Jun 14, 2020 at 12:00 PM Geert Uytterhoeven
wrote:
> On Sun, Jun 14, 2020 at 11:43 AM Andy Shevchenko
> wrote:
> > On Sun, Jun 14, 2020 at 12:34 PM Andy Shevchenko
> > wrote:
> > >
> > > On Sun, Jun 14, 2020 at 12:10 PM Wolfram Sang wrote:
> > > > both in the I2C subsystem and also for Renesas drivers I maintain, I am
> > > > starting to get boilerplate patches doing some pm_runtime_put_* variant
> > > > because a failing pm_runtime_get is supposed to increase the ref
> > > > counters? Really? This feels wrong and unintuitive to me.
> > >
> > > Yeah, that is a well known issue with PM (I even have for a long time
> > > a coccinelle script, when I realized myself that there are a lot of
> > > cases like this, but someone else discovered this recently, like
> > > opening a can of worms).
> > >
> > > > I expect there
> > > > has been a discussion around it but I couldn't find it.
> > >
> > > Rafael explained (again) recently this. I can't find it quickly,
> > > unfortunately.
> >
> > I _think_ this discussion, but may be it's simple another tentacle of
> > the same octopus.
> > https://patchwork.ozlabs.org/project/linux-tegra/patch/20200520095148.10995-1-dinghao@zju.edu.cn/
>
> Thanks, hadn't read that one! (so I was still at -1 from
> http://sweng.the-davies.net/Home/rustys-api-design-manifesto ;-)
>
> So "pm_runtime_put_noidle()" is the (definitive?) one to pair with a
> pm_runtime_get_sync() failure?
My biggest worry here is all those copycats jumping on the bandwagon,
and sending untested[*] patches that end up calling the wrong function.
[*] Several of them turned out to introduce trivial compile warnings, so
I now consider all patches authored by the same person as untested.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] mm/slab: Add a __GFP_ACCOUNT GFP flag check for slab allocation
Hi Muchun, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on mmotm/master] url: https://github.com/0day-ci/linux/commits/Muchun-Song/mm-slab-Add-a-__GFP_ACCOUNT-GFP-flag-check-for-slab-allocation/20200614-144049 base: git://git.cmpxchg.org/linux-mmotm.git master config: xtensa-allyesconfig (attached as .config) compiler: xtensa-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>, old ones prefixed by <<): In file included from include/linux/kernel.h:11, from include/linux/interrupt.h:6, from mm/kasan/common.c:18: include/linux/scatterlist.h: In function 'sg_set_buf': arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits] 182 | ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr) | ^~ include/linux/compiler.h:78:42: note: in definition of macro 'unlikely' 78 | # define unlikely(x) __builtin_expect(!!(x), 0) | ^ include/linux/scatterlist.h:143:2: note: in expansion of macro 'BUG_ON' 143 | BUG_ON(!virt_addr_valid(buf)); | ^~ arch/xtensa/include/asm/page.h:190:32: note: in expansion of macro 'pfn_valid' 190 | #define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT) |^ include/linux/scatterlist.h:143:10: note: in expansion of macro 'virt_addr_valid' 143 | BUG_ON(!virt_addr_valid(buf)); | ^~~ In file included from include/linux/mm_types_task.h:16, from include/linux/mm_types.h:5, from include/linux/mmzone.h:21, from include/linux/topology.h:33, from include/linux/irq.h:19, from include/asm-generic/hardirq.h:13, from ./arch/xtensa/include/generated/asm/hardirq.h:1, from include/linux/hardirq.h:9, from include/linux/interrupt.h:11, from mm/kasan/common.c:18: mm/kasan/../internal.h: In function 'mem_map_next': arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits] 182 | ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr) | ^~ >> mm/kasan/../internal.h:393:8: note: in expansion of macro 'pfn_valid' 393 | if (!pfn_valid(pfn)) |^ -- In file included from arch/xtensa/include/asm/processor.h:15, from arch/xtensa/include/asm/bitops.h:20, from include/linux/bitops.h:19, from mm/kasan/report.c:17: include/linux/scatterlist.h: In function 'sg_set_buf': arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits] 182 | ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr) | ^~ include/linux/compiler.h:78:42: note: in definition of macro 'unlikely' 78 | # define unlikely(x) __builtin_expect(!!(x), 0) | ^ include/linux/scatterlist.h:143:2: note: in expansion of macro 'BUG_ON' 143 | BUG_ON(!virt_addr_valid(buf)); | ^~ arch/xtensa/include/asm/page.h:190:32: note: in expansion of macro 'pfn_valid' 190 | #define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT) |^ include/linux/scatterlist.h:143:10: note: in expansion of macro 'virt_addr_valid' 143 | BUG_ON(!virt_addr_valid(buf)); | ^~~ In file included from include/linux/mm_types_task.h:16, from include/linux/mm_types.h:5, from include/linux/mmzone.h:21, from include/linux/gfp.h:6, from include/linux/mm.h:10, from include/linux/kallsyms.h:12, from include/linux/ftrace.h:11, from mm/kasan/report.c:18: mm/kasan/../internal.h: In function 'mem_map_next': arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits] 182 | ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr) | ^~ >> mm/kasan/../internal.h:393:8: note: in expansion of macro 'pfn_valid' 393 | if (!pfn_valid(pfn)) |^ mm/kasan/report.c: At top level: mm/kasan/report.c:474:6: warning: no previous prototype for '__kasan_report' [-Wmissing-prototypes] 474 | void __kasan_report(unsigned long addr, size_t size, bool is_write, unsigned long ip) | ^~ -- In file included from arch/xtensa/include/asm/processor.h:15, from arch/xtensa/include/asm/bitops.h:20, from include/linux/bitops.h:19, from mm/kasan/
Re: [RFC] MFD's relationship with Device Tree (OF)
Hi Rob,
Am 2020-06-10 00:03, schrieb Rob Herring:
[..]
Yes, we should use 'reg' whenever possible. If we don't have 'reg',
then you shouldn't have a unit-address either and you can simply match
on the node name (standard DT driver matching is with compatible,
device_type, and node name (w/o unit-address)). We've generally been
doing 'classname-N' when there's no 'reg' to do 'classname@N'.
Matching on 'classname-N' would work with node name matching as only
unit-addresses are stripped.
This still keeps me thinking. Shouldn't we allow the (MFD!) device
driver creator to choose between "classname@N" and "classname-N".
In most cases N might not be made up, but it is arbitrarily chosen;
for example you've chosen the bank for the ab8500 reg. It is not
a defined entity, like an I2C address if your parent is an I2C bus,
or a SPI chip select, or the memory address in case of MMIO. Instead
the device driver creator just chooses some "random" property from
the datasheet; another device creator might have chosen another
property. Wouldn't it make more sense, to just say this MFD provides
N pwm devices and the subnodes are matching based on pwm-{0,1..N-1}?
That would also be the logical consequence of the current MFD sub
device to OF node matching code, which just supports N=1.
-michael
[PATCH v2 2/2] i2c: imx: Fix external abort on interrupt in exit paths
If interrupt comes late, during probe error path or device remove (could
be triggered with CONFIG_DEBUG_SHIRQ), the interrupt handler
i2c_imx_isr() will access registers with the clock being disabled. This
leads to external abort on non-linefetch on Toradex Colibri VF50 module
(with Vybrid VF5xx):
Unhandled fault: external abort on non-linefetch (0x1008) at 0x8882d003
Internal error: : 1008 [#1] ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 5.7.0 #607
Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
(i2c_imx_isr) from [<8017009c>] (free_irq+0x25c/0x3b0)
(free_irq) from [<805844ec>] (release_nodes+0x178/0x284)
(release_nodes) from [<80580030>] (really_probe+0x10c/0x348)
(really_probe) from [<80580380>] (driver_probe_device+0x60/0x170)
(driver_probe_device) from [<80580630>] (device_driver_attach+0x58/0x60)
(device_driver_attach) from [<805806bc>] (__driver_attach+0x84/0xc0)
(__driver_attach) from [<8057e228>] (bus_for_each_dev+0x68/0xb4)
(bus_for_each_dev) from [<8057f3ec>] (bus_add_driver+0x144/0x1ec)
(bus_add_driver) from [<80581320>] (driver_register+0x78/0x110)
(driver_register) from [<8010213c>] (do_one_initcall+0xa8/0x2f4)
(do_one_initcall) from [<80c0100c>] (kernel_init_freeable+0x178/0x1dc)
(kernel_init_freeable) from [<80807048>] (kernel_init+0x8/0x110)
(kernel_init) from [<80100114>] (ret_from_fork+0x14/0x20)
Additionally, the i2c_imx_isr() could wake up the wait queue
(imx_i2c_struct->queue) before its initialization happens.
The resource-managed framework should not be used for interrupt handling,
because the resource will be released too late - after disabling clocks.
The interrupt handler is not prepared for such case.
Fixes: 1c4b6c3bcf30 ("i2c: imx: implement bus recovery")
Cc:
Signed-off-by: Krzysztof Kozlowski
---
Changes since v1:
1. Remove the devm- and use regular methods.
---
drivers/i2c/busses/i2c-imx.c | 25 ++---
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 6e45958565d1..8a0d49419326 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1171,14 +1171,6 @@ static int i2c_imx_probe(struct platform_device *pdev)
return ret;
}
- /* Request IRQ */
- ret = devm_request_irq(&pdev->dev, irq, i2c_imx_isr, IRQF_SHARED,
- pdev->name, i2c_imx);
- if (ret) {
- dev_err(&pdev->dev, "can't claim irq %d\n", irq);
- goto clk_disable;
- }
-
/* Init queue */
init_waitqueue_head(&i2c_imx->queue);
@@ -1197,6 +1189,14 @@ static int i2c_imx_probe(struct platform_device *pdev)
if (ret < 0)
goto rpm_disable;
+ /* Request IRQ */
+ ret = request_threaded_irq(irq, i2c_imx_isr, NULL, IRQF_SHARED,
+ pdev->name, i2c_imx);
+ if (ret) {
+ dev_err(&pdev->dev, "can't claim irq %d\n", irq);
+ goto rpm_put;
+ }
+
/* Set up clock divider */
i2c_imx->bitrate = I2C_MAX_STANDARD_MODE_FREQ;
ret = of_property_read_u32(pdev->dev.of_node,
@@ -1239,13 +1239,13 @@ static int i2c_imx_probe(struct platform_device *pdev)
clk_notifier_unregister:
clk_notifier_unregister(i2c_imx->clk, &i2c_imx->clk_change_nb);
+ free_irq(irq, i2c_imx);
+rpm_put:
pm_runtime_put_noidle(&pdev->dev);
rpm_disable:
pm_runtime_disable(&pdev->dev);
pm_runtime_set_suspended(&pdev->dev);
pm_runtime_dont_use_autosuspend(&pdev->dev);
-
-clk_disable:
clk_disable_unprepare(i2c_imx->clk);
return ret;
}
@@ -1253,7 +1253,7 @@ static int i2c_imx_probe(struct platform_device *pdev)
static int i2c_imx_remove(struct platform_device *pdev)
{
struct imx_i2c_struct *i2c_imx = platform_get_drvdata(pdev);
- int ret;
+ int irq, ret;
ret = pm_runtime_get_sync(&pdev->dev);
if (ret < 0)
@@ -1273,6 +1273,9 @@ static int i2c_imx_remove(struct platform_device *pdev)
imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2SR);
clk_notifier_unregister(i2c_imx->clk, &i2c_imx->clk_change_nb);
+ irq = platform_get_irq(pdev, 0);
+ if (irq >= 0)
+ free_irq(irq, i2c_imx);
clk_disable_unprepare(i2c_imx->clk);
pm_runtime_put_noidle(&pdev->dev);
--
2.7.4
[PATCH v2 1/2] i2c: imx: Fix PM runtime inbalance in probe error path
When pm_runtime_get_sync() fails in probe(), the error path should not
call pm_runtime_put_noidle(). This would lead to inbalance in
usage_count.
Fixes: 588eb93ea49f ("i2c: imx: add runtime pm support to improve the
performance")
Cc:
Signed-off-by: Krzysztof Kozlowski
---
Changes since v1:
1. New patch
---
drivers/i2c/busses/i2c-imx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 0ab5381aa012..6e45958565d1 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1239,8 +1239,8 @@ static int i2c_imx_probe(struct platform_device *pdev)
clk_notifier_unregister:
clk_notifier_unregister(i2c_imx->clk, &i2c_imx->clk_change_nb);
-rpm_disable:
pm_runtime_put_noidle(&pdev->dev);
+rpm_disable:
pm_runtime_disable(&pdev->dev);
pm_runtime_set_suspended(&pdev->dev);
pm_runtime_dont_use_autosuspend(&pdev->dev);
--
2.7.4
Re: [PATCH] scsi: target/sbp: remove firewire SBP target driver
On 14/06/2020 01:03, Finn Thain wrote: > On Sat, 13 Jun 2020, Chris Boot wrote: > >> I no longer have the time to maintain this subsystem nor the hardware to >> test patches with. > > Then why not patch MAINTAINERS, and orphan it, as per usual practice? > > $ git log --oneline MAINTAINERS | grep -i orphan My patch to remove it was in response to: https://lore.kernel.org/lkml/yq1img99d4k@ca-mkp.ca.oracle.com/ >> It also doesn't appear to have any active users so I doubt anyone will >> miss it. >> > > It's not unusual that any Linux driver written more than 5 years ago > "doesn't appear to have any active users". > > If a driver has been orphaned and broken in the past, and no-one stepped > up to fix it within a reasonable period, removal would make sense. But > that's not the case here. > > I haven't used this driver for a long time, but I still own PowerMacs with > firewire, and I know I'm not the only one. I expect that if someone finds this useful it can stick around (but that's not my call). I just don't have the time or inclination or hardware to be able to maintain it anymore, so someone else would have to pick it up. Cheers, Chris -- Chris Boot bo...@boo.tc
Re: RFC: a failing pm_runtime_get increases the refcnt?
On Sun, Jun 14, 2020 at 1:05 PM Geert Uytterhoeven wrote: > On Sun, Jun 14, 2020 at 12:00 PM Geert Uytterhoeven > wrote: > > On Sun, Jun 14, 2020 at 11:43 AM Andy Shevchenko > > wrote: > > > On Sun, Jun 14, 2020 at 12:34 PM Andy Shevchenko > > > wrote: > > > > > > > > On Sun, Jun 14, 2020 at 12:10 PM Wolfram Sang wrote: > > > > > both in the I2C subsystem and also for Renesas drivers I maintain, I > > > > > am > > > > > starting to get boilerplate patches doing some pm_runtime_put_* > > > > > variant > > > > > because a failing pm_runtime_get is supposed to increase the ref > > > > > counters? Really? This feels wrong and unintuitive to me. > > > > > > > > Yeah, that is a well known issue with PM (I even have for a long time > > > > a coccinelle script, when I realized myself that there are a lot of > > > > cases like this, but someone else discovered this recently, like > > > > opening a can of worms). > > > > > > > > > I expect there > > > > > has been a discussion around it but I couldn't find it. > > > > > > > > Rafael explained (again) recently this. I can't find it quickly, > > > > unfortunately. > > > > > > I _think_ this discussion, but may be it's simple another tentacle of > > > the same octopus. > > > https://patchwork.ozlabs.org/project/linux-tegra/patch/20200520095148.10995-1-dinghao@zju.edu.cn/ > > > > Thanks, hadn't read that one! (so I was still at -1 from > > http://sweng.the-davies.net/Home/rustys-api-design-manifesto ;-) > > > > So "pm_runtime_put_noidle()" is the (definitive?) one to pair with a > > pm_runtime_get_sync() failure? > > My biggest worry here is all those copycats jumping on the bandwagon, > and sending untested[*] patches that end up calling the wrong function. > > [*] Several of them turned out to introduce trivial compile warnings, so > I now consider all patches authored by the same person as untested. That's always a problem with janitors like patches... Once I tried to ask them to provide a testing material, but... - some maintainers just accept them without asking questions - some maintainers even defend them that they are doing a good job (and LWN top contributor statistics also motivate some of janitors, though I consider it not the best metrics) - practically almost no contributor answered to my queries, so, I consider all of them are untested independent to the name (if name appears in more than dozen patches, esp. in different subsystems) - and yes, it's a trade-off, some of the patches indeed useful. -- With Best Regards, Andy Shevchenko
[PATCH 1/2] spi: spi-fsl-dspi: Fix external abort on interrupt in exit paths
If interrupt comes late, during probe error path or device remove (could
be triggered with CONFIG_DEBUG_SHIRQ), the interrupt handler
dspi_interrupt() will access registers with the clock being disabled. This
leads to external abort on non-linefetch on Toradex Colibri VF50 module
(with Vybrid VF5xx):
$ echo 4002d000.spi >
/sys/devices/platform/soc/4000.bus/4002d000.spi/driver/unbind
Unhandled fault: external abort on non-linefetch (0x1008) at 0x8887f02c
Internal error: : 1008 [#1] ARM
CPU: 0 PID: 136 Comm: sh Not tainted
5.7.0-next-20200610-9-g5c913fa0f9c5-dirty #74
Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
(regmap_mmio_read32le) from [<8061885c>] (regmap_mmio_read+0x48/0x68)
(regmap_mmio_read) from [<8060e3b8>] (_regmap_bus_reg_read+0x24/0x28)
(_regmap_bus_reg_read) from [<80611c50>] (_regmap_read+0x70/0x1c0)
(_regmap_read) from [<80611dec>] (regmap_read+0x4c/0x6c)
(regmap_read) from [<80678ca0>] (dspi_interrupt+0x3c/0xa8)
(dspi_interrupt) from [<8017acec>] (free_irq+0x26c/0x3cc)
(free_irq) from [<8017dcec>] (devm_irq_release+0x1c/0x20)
(devm_irq_release) from [<805f98ec>] (release_nodes+0x1e4/0x298)
(release_nodes) from [<805f9ac8>] (devres_release_all+0x40/0x60)
(devres_release_all) from [<805f5134>]
(device_release_driver_internal+0x108/0x1ac)
(device_release_driver_internal) from [<805f521c>]
(device_driver_detach+0x20/0x24)
The resource-managed framework should not be used for interrupt handling,
because the resource will be released too late - after disabling clocks.
The interrupt handler is not prepared for such case.
Fixes: 349ad66c0ab0 ("spi:Add Freescale DSPI driver for Vybrid VF610 platform")
Cc:
Signed-off-by: Krzysztof Kozlowski
---
This is an follow up of my other patch for I2C IMX driver [1]. Let's fix the
issues consistently.
[1]
https://lore.kernel.org/lkml/1592130544-19759-2-git-send-email-k...@kernel.org/T/#u
---
drivers/spi/spi-fsl-dspi.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 58190c94561f..57e7a626ba00 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -1385,8 +1385,8 @@ static int dspi_probe(struct platform_device *pdev)
goto poll_mode;
}
- ret = devm_request_irq(&pdev->dev, dspi->irq, dspi_interrupt,
- IRQF_SHARED, pdev->name, dspi);
+ ret = request_threaded_irq(dspi->irq, dspi_interrupt, NULL,
+ IRQF_SHARED, pdev->name, dspi);
if (ret < 0) {
dev_err(&pdev->dev, "Unable to attach DSPI interrupt\n");
goto out_clk_put;
@@ -1400,7 +1400,7 @@ static int dspi_probe(struct platform_device *pdev)
ret = dspi_request_dma(dspi, res->start);
if (ret < 0) {
dev_err(&pdev->dev, "can't get dma channels\n");
- goto out_clk_put;
+ goto out_free_irq;
}
}
@@ -1415,11 +1415,14 @@ static int dspi_probe(struct platform_device *pdev)
ret = spi_register_controller(ctlr);
if (ret != 0) {
dev_err(&pdev->dev, "Problem registering DSPI ctlr\n");
- goto out_clk_put;
+ goto out_free_irq;
}
return ret;
+out_free_irq:
+ if (dspi->irq > 0)
+ free_irq(dspi->irq, dspi);
out_clk_put:
clk_disable_unprepare(dspi->clk);
out_ctlr_put:
@@ -1435,6 +1438,8 @@ static int dspi_remove(struct platform_device *pdev)
/* Disconnect from the SPI framework */
dspi_release_dma(dspi);
+ if (dspi->irq > 0)
+ free_irq(dspi->irq, dspi);
clk_disable_unprepare(dspi->clk);
spi_unregister_controller(dspi->ctlr);
--
2.7.4
[PATCH 2/2] spi: spi-fsl-dspi: Initialize completion before possible interrupt
If interrupt fires early, the dspi_interrupt() could complete
(dspi->xfer_done) before its initialization happens.
Fixes: 4f5ee75ea171 ("spi: spi-fsl-dspi: Replace interruptible wait queue with
a simple completion")
Cc:
Signed-off-by: Krzysztof Kozlowski
---
drivers/spi/spi-fsl-dspi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 57e7a626ba00..efb63ed9fd86 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -1385,6 +1385,8 @@ static int dspi_probe(struct platform_device *pdev)
goto poll_mode;
}
+ init_completion(&dspi->xfer_done);
+
ret = request_threaded_irq(dspi->irq, dspi_interrupt, NULL,
IRQF_SHARED, pdev->name, dspi);
if (ret < 0) {
@@ -1392,8 +1394,6 @@ static int dspi_probe(struct platform_device *pdev)
goto out_clk_put;
}
- init_completion(&dspi->xfer_done);
-
poll_mode:
if (dspi->devtype_data->trans_mode == DSPI_DMA_MODE) {
--
2.7.4
Re: [RESEND PATCH v10 10/10] arm64: dts: Add node for ufs exynos7
On Sat, Jun 13, 2020 at 08:17:06AM +0530, Alim Akhtar wrote: > Adding dt node foe UFS and UFS-PHY for exynos7 SoC. > > Signed-off-by: Alim Akhtar > Tested-by: Paweł Chmiel > --- > .../boot/dts/exynos/exynos7-espresso.dts | 4 ++ > arch/arm64/boot/dts/exynos/exynos7.dtsi | 43 ++- > 2 files changed, 45 insertions(+), 2 deletions(-) > This is already applied and in the linux-next. Don't resend applied patches. Best regards, Krzysztof
Re: [External] Re: [PATCH] mm/slab: Add a __GFP_ACCOUNT GFP flag check for slab allocation
On Sun, Jun 14, 2020 at 6:19 PM kernel test robot wrote: > > Hi Muchun, > > Thank you for the patch! Perhaps something to improve: > > [auto build test WARNING on mmotm/master] > > url: > https://github.com/0day-ci/linux/commits/Muchun-Song/mm-slab-Add-a-__GFP_ACCOUNT-GFP-flag-check-for-slab-allocation/20200614-144049 > base: git://git.cmpxchg.org/linux-mmotm.git master > config: xtensa-allyesconfig (attached as .config) > compiler: xtensa-linux-gcc (GCC) 9.3.0 > reproduce (this is a W=1 build): > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross > ARCH=xtensa > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > > All warnings (new ones prefixed by >>, old ones prefixed by <<): > > In file included from include/linux/kernel.h:11, > from include/linux/interrupt.h:6, > from mm/kasan/common.c:18: > include/linux/scatterlist.h: In function 'sg_set_buf': > arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned > expression >= 0 is always true [-Wtype-limits] > 182 | ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr) > | ^~ > include/linux/compiler.h:78:42: note: in definition of macro 'unlikely' > 78 | # define unlikely(x) __builtin_expect(!!(x), 0) > | ^ > include/linux/scatterlist.h:143:2: note: in expansion of macro 'BUG_ON' > 143 | BUG_ON(!virt_addr_valid(buf)); > | ^~ > arch/xtensa/include/asm/page.h:190:32: note: in expansion of macro 'pfn_valid' > 190 | #define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT) > |^ > include/linux/scatterlist.h:143:10: note: in expansion of macro > 'virt_addr_valid' > 143 | BUG_ON(!virt_addr_valid(buf)); > | ^~~ > In file included from include/linux/mm_types_task.h:16, > from include/linux/mm_types.h:5, > from include/linux/mmzone.h:21, > from include/linux/topology.h:33, > from include/linux/irq.h:19, > from include/asm-generic/hardirq.h:13, > from ./arch/xtensa/include/generated/asm/hardirq.h:1, > from include/linux/hardirq.h:9, > from include/linux/interrupt.h:11, > from mm/kasan/common.c:18: > mm/kasan/../internal.h: In function 'mem_map_next': > arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned > expression >= 0 is always true [-Wtype-limits] > 182 | ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr) > | ^~ > >> mm/kasan/../internal.h:393:8: note: in expansion of macro 'pfn_valid' > 393 | if (!pfn_valid(pfn)) > |^ > -- > In file included from arch/xtensa/include/asm/processor.h:15, > from arch/xtensa/include/asm/bitops.h:20, > from include/linux/bitops.h:19, > from mm/kasan/report.c:17: > include/linux/scatterlist.h: In function 'sg_set_buf': > arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned > expression >= 0 is always true [-Wtype-limits] > 182 | ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr) > | ^~ > include/linux/compiler.h:78:42: note: in definition of macro 'unlikely' > 78 | # define unlikely(x) __builtin_expect(!!(x), 0) > | ^ > include/linux/scatterlist.h:143:2: note: in expansion of macro 'BUG_ON' > 143 | BUG_ON(!virt_addr_valid(buf)); > | ^~ > arch/xtensa/include/asm/page.h:190:32: note: in expansion of macro 'pfn_valid' > 190 | #define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT) > |^ > include/linux/scatterlist.h:143:10: note: in expansion of macro > 'virt_addr_valid' > 143 | BUG_ON(!virt_addr_valid(buf)); > | ^~~ > In file included from include/linux/mm_types_task.h:16, > from include/linux/mm_types.h:5, > from include/linux/mmzone.h:21, > from include/linux/gfp.h:6, > from include/linux/mm.h:10, > from include/linux/kallsyms.h:12, > from include/linux/ftrace.h:11, > from mm/kasan/report.c:18: > mm/kasan/../internal.h: In function 'mem_map_next': > arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned > expression >= 0 is always true [-Wtype-limits] > 182 | ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr) > |
Re: [PATCH 2/2] spi: spi-fsl-dspi: Initialize completion before possible interrupt
On Sun, 14 Jun 2020 at 13:56, Krzysztof Kozlowski wrote:
>
> If interrupt fires early, the dspi_interrupt() could complete
> (dspi->xfer_done) before its initialization happens.
>
> Fixes: 4f5ee75ea171 ("spi: spi-fsl-dspi: Replace interruptible wait queue
> with a simple completion")
> Cc:
> Signed-off-by: Krzysztof Kozlowski
> ---
Why would an interrupt fire before spi_register_controller, therefore
before dspi_transfer_one_message could get called?
Is this master or slave mode?
> drivers/spi/spi-fsl-dspi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
> index 57e7a626ba00..efb63ed9fd86 100644
> --- a/drivers/spi/spi-fsl-dspi.c
> +++ b/drivers/spi/spi-fsl-dspi.c
> @@ -1385,6 +1385,8 @@ static int dspi_probe(struct platform_device *pdev)
> goto poll_mode;
> }
>
> + init_completion(&dspi->xfer_done);
> +
> ret = request_threaded_irq(dspi->irq, dspi_interrupt, NULL,
>IRQF_SHARED, pdev->name, dspi);
> if (ret < 0) {
> @@ -1392,8 +1394,6 @@ static int dspi_probe(struct platform_device *pdev)
> goto out_clk_put;
> }
>
> - init_completion(&dspi->xfer_done);
> -
> poll_mode:
>
> if (dspi->devtype_data->trans_mode == DSPI_DMA_MODE) {
> --
> 2.7.4
>
Re: [PATCH 11/19] perf ftrace: add option '-u/--userstacktrace' to show userspace stacktrace
On Wed, May 20, 2020 at 06:07:41PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sun, May 10, 2020 at 11:06:20PM +0800, Changbin Du escreveu:
> > This adds an option ''-u/--userstacktrace' for function tracer to display
> > userspace back trace.
>
> Probably we should have this as a term, an option to --call-graph?
>
> For --call-graph the way to suppress this is to ask for the event to be
> in the kernel only, i.e. something like:
>
> perf record -e cycles:k --call-graph
>
> So perhaps we should have something like:
>
> perf ftrace --call-graph
>
> With some default, possibly a bit different than the other perf tools,
> just including the kernel, and accepting:
>
> perf ftrace --call-graph
> perf ftrace --call-graph k
> perf ftrace --call-graph u
> perf ftrace --call-graph uk
>
> - Arnaldo
>
I just found userstacktrace is not supported by ftrace yet. It is not
hard to add this feature in kernel and I have done it locally. So I will
drop this option before it is upstreamed.
> > Signed-off-by: Changbin Du
> > ---
> > tools/perf/builtin-ftrace.c | 24
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> > index 2ef5d1c4b23c..ab76ba66bd9e 100644
> > --- a/tools/perf/builtin-ftrace.c
> > +++ b/tools/perf/builtin-ftrace.c
> > @@ -40,6 +40,7 @@ struct perf_ftrace {
> > struct list_headnograph_funcs;
> > int graph_depth;
> > boolfunc_stack_trace;
> > + booluserstacktrace;
> > boolnosleep_time;
> > boolnofuncgraph_irqs;
> > boolfuncgraph_tail;
> > @@ -197,6 +198,8 @@ static void reset_tracing_options(struct perf_ftrace
> > *ftrace __maybe_unused)
> > write_tracing_option_file("funcgraph-proc", "0");
> > write_tracing_option_file("funcgraph-abstime", "0");
> > write_tracing_option_file("irq-info", "0");
> > + write_tracing_option_file("userstacktrace", "0");
> > + write_tracing_option_file("sym-userobj", "0");
> > }
> >
> > static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
> > @@ -287,6 +290,20 @@ static int set_tracing_func_stack_trace(struct
> > perf_ftrace *ftrace)
> > return 0;
> > }
> >
> > +static int set_tracing_userstacktrace(struct perf_ftrace *ftrace)
> > +{
> > + if (!ftrace->userstacktrace)
> > + return 0;
> > +
> > + if (write_tracing_option_file("userstacktrace", "1") < 0)
> > + return -1;
> > +
> > + if (write_tracing_option_file("sym-userobj", "1") < 0)
> > + return -1;
> > +
> > + return 0;
> > +}
> > +
> > static int reset_tracing_cpu(void)
> > {
> > struct perf_cpu_map *cpumap = perf_cpu_map__new(NULL);
> > @@ -482,6 +499,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace,
> > int argc, const char **argv)
> > goto out_reset;
> > }
> >
> > + if (set_tracing_userstacktrace(ftrace) < 0) {
> > + pr_err("failed to set tracing option userstacktrace\n");
> > + goto out_reset;
> > + }
> > +
> > if (set_tracing_filters(ftrace) < 0) {
> > pr_err("failed to set tracing filters\n");
> > goto out_reset;
> > @@ -644,6 +666,8 @@ int cmd_ftrace(int argc, const char **argv)
> > "do not trace given functions", parse_filter_func),
> > OPT_BOOLEAN('s', "func-stack-trace", &ftrace.func_stack_trace,
> > "Show kernel stack trace for function tracer"),
> > + OPT_BOOLEAN('u', "userstacktrace", &ftrace.userstacktrace,
> > + "Show stacktrace of the current user space thread"),
> > OPT_CALLBACK_DEFAULT('G', "graph-funcs", &ftrace.graph_funcs, "func",
> > "Set graph filter on given functions (imply to use
> > function_graph tracer)",
> > parse_filter_func, "*"),
> > --
> > 2.25.1
> >
>
> --
>
> - Arnaldo
--
Cheers,
Changbin Du
Re: [PATCH 2/2] spi: spi-fsl-dspi: Initialize completion before possible interrupt
> > If interrupt fires early, the dspi_interrupt() could complete
> > (dspi->xfer_done) before its initialization happens.
> >
> > Fixes: 4f5ee75ea171 ("spi: spi-fsl-dspi: Replace interruptible wait queue
> > with a simple completion")
> > Cc:
> > Signed-off-by: Krzysztof Kozlowski
> > ---
>
> Why would an interrupt fire before spi_register_controller, therefore
> before dspi_transfer_one_message could get called?
I don't know this HW, but the generic answer usually is: Bootloader used
SPI and didn't clean up properly.
signature.asc
Description: PGP signature
Re: [PATCH 2/2] spi: spi-fsl-dspi: Initialize completion before possible interrupt
On Sun, Jun 14, 2020 at 02:14:15PM +0300, Vladimir Oltean wrote:
> On Sun, 14 Jun 2020 at 13:56, Krzysztof Kozlowski wrote:
> >
> > If interrupt fires early, the dspi_interrupt() could complete
> > (dspi->xfer_done) before its initialization happens.
> >
> > Fixes: 4f5ee75ea171 ("spi: spi-fsl-dspi: Replace interruptible wait queue
> > with a simple completion")
> > Cc:
> > Signed-off-by: Krzysztof Kozlowski
> > ---
>
> Why would an interrupt fire before spi_register_controller, therefore
> before dspi_transfer_one_message could get called?
> Is this master or slave mode?
I guess practically it won't fire. It's more of a matter of logical
order and:
1. Someone might fix the CONFIG_DEBUG_SHIRQ_FIXME one day,
2. The hardware is actually initialized before and someone could attach
to SPI bus some weird device.
Best regards,
Krzysztof
[PATCH v5] ath10k: provide survey info as accumulated data
From: Venkateswara Naralasetty
It is expected that the returned counters by .get_survey are monotonic
increasing. But the data from ath10k gets reset to zero regularly. Channel
active/busy time are then showing incorrect values (less than previous or
sometimes zero) for the currently active channel during successive survey
dump commands.
example:
$ iw dev wlan0 survey dump
Survey data from wlan0
frequency: 5180 MHz [in use]
channel active time:54995 ms
channel busy time: 432 ms
channel receive time: 0 ms
channel transmit time: 59 ms
...
$ iw dev wlan0 survey dump
Survey data from wlan0
frequency: 5180 MHz [in use]
channel active time:32592 ms
channel busy time: 254 ms
channel receive time: 0 ms
channel transmit time: 0 ms
...
The correct way to handle this is to use the non-clearing
WMI_BSS_SURVEY_REQ_TYPE_READ wmi_bss_survey_req_type. The firmware will
then accumulate the survey data and handle wrap arounds.
Tested on:
* QCA9984 hw1.0 firmware 10.4-3.5.3-00057
* QCA988X hw2.0 firmware 10.2.4-1.0-00047
* QCA9888 hw2.0 firmware 10.4-3.9.0.2-00024
* QCA4019 hw1.0 firmware 10.4-3.6-00140
Fixes: fa7937e3d5c2 ("ath10k: update bss channel survey information")
Signed-off-by: Venkateswara Naralasetty
Tested-by: Markus Theil
Tested-by: John Deere <24601dee...@gmail.com>
[s...@narfation.org: adjust commit message]
Signed-off-by: Sven Eckelmann
---
v5:
* add additional tested devices
* restructure commit message
v4:
* updated signed-off-by
v3:
* Rebased on TOT and added Tested-by
Everything expect QCA9984 hw1.0 firmware 10.4-3.5.3-00057 was also tested
by me.
---
drivers/net/wireless/ath/ath10k/mac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c
b/drivers/net/wireless/ath/ath10k/mac.c
index 919d15584d4a..77daca67a8e1 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -7283,7 +7283,7 @@ ath10k_mac_update_bss_chan_survey(struct ath10k *ar,
struct ieee80211_channel *channel)
{
int ret;
- enum wmi_bss_survey_req_type type = WMI_BSS_SURVEY_REQ_TYPE_READ_CLEAR;
+ enum wmi_bss_survey_req_type type = WMI_BSS_SURVEY_REQ_TYPE_READ;
lockdep_assert_held(&ar->conf_mutex);
--
2.20.1
Re: [PATCH] drm/panfrost: fix ref count leak in panfrost_job_hw_submit
> in panfrost_job_hw_submit, … * Can the term “reference count” become relevant also for this commit message besides other possible adjustments? * Would you like to add the tag “Fixes”? … > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c … > @@ -184,6 +183,9 @@ static void panfrost_job_hw_submit(struct panfrost_job > *job, int js) > job, js, jc_head); > > job_write(pfdev, JS_COMMAND_NEXT(js), JS_COMMAND_START); > +out: > + pm_runtime_put_sync_autosuspend(pfdev->dev); > + return; > } … Perhaps use the label “put_sync” instead? Regards, Markus
Re: [PATCH] drm/panfrost: fix ref count leak in panfrost_job_hw_submit
> in panfrost_job_hw_submit, … * Can the term “reference count” become relevant also for this commit message besides other possible adjustments? * Would you like to add the tag “Fixes”? … > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c … > @@ -184,6 +183,9 @@ static void panfrost_job_hw_submit(struct panfrost_job > *job, int js) > job, js, jc_head); > > job_write(pfdev, JS_COMMAND_NEXT(js), JS_COMMAND_START); > +out: > + pm_runtime_put_sync_autosuspend(pfdev->dev); > + return; > } … Perhaps use the label “put_sync” instead? Regards, Markus
Re: [PATCH] drm/panfrost: fix ref count leak in panfrost_job_hw_submit
> in panfrost_job_hw_submit, … * Can the term “reference count” become relevant also for this commit message besides other possible adjustments? * Would you like to add the tag “Fixes”? … > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c … > @@ -184,6 +183,9 @@ static void panfrost_job_hw_submit(struct panfrost_job > *job, int js) > job, js, jc_head); > > job_write(pfdev, JS_COMMAND_NEXT(js), JS_COMMAND_START); > +out: > + pm_runtime_put_sync_autosuspend(pfdev->dev); > + return; > } … Perhaps use the label “put_sync” instead? Regards, Markus
[GIT PULL] Btrfs updates for 5.8, part 2
Hi, this reverts the direct io port to iomap infrastructure of btrfs merged in the first pull request. We found problems in invalidate page that don't seem to be fixable as regressions or without changing iomap code that would not affect other filesystems. There are 4 patches reverted in total, but 3 of them are followup cleanups needed to revert a43a67a2d715540c13 cleanly. The result is the buffer head based implementation of direct io. There's one trivial conflict that git does not auto-resolve, in the address space operations readpages has been replaced by readahead and this change is in the context of the direct io callback diff. Reverts are not great, but under current circumstances I don't see better options. Please pull, thanks. The following changes since commit 2166e5edce9ac1edf3b113d6091ef72fcac2d6c4: btrfs: fix space_info bytes_may_use underflow during space cache writeout (2020-05-28 14:01:53 +0200) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-5.8-part2-tag for you to fetch changes up to 55e20bd12a56e06c38b953177bb162cbbaa96004: Revert "btrfs: switch to iomap_dio_rw() for dio" (2020-06-14 01:19:02 +0200) David Sterba (4): Revert "btrfs: split btrfs_direct_IO to read and write part" Revert "btrfs: remove BTRFS_INODE_READDIO_NEED_LOCK" Revert "fs: remove dio_end_io()" Revert "btrfs: switch to iomap_dio_rw() for dio" fs/btrfs/Kconfig | 1 - fs/btrfs/btrfs_inode.h | 18 +++ fs/btrfs/ctree.h | 4 - fs/btrfs/file.c| 97 + fs/btrfs/inode.c | 379 +++-- fs/direct-io.c | 19 +++ include/linux/fs.h | 2 + 7 files changed, 286 insertions(+), 234 deletions(-)
Re: next-0519 on thinkpad x60: sound related? window manager crash
Excerpts from Takashi Iwai's message of June 14, 2020 5:54 am:
> On Sat, 13 Jun 2020 18:25:22 +0200,
> Alex Xu (Hello71) wrote:
>>
>> Excerpts from Takashi Iwai's message of June 11, 2020 1:11 pm:
>> > Thanks, so something still missing in the mmap handling, I guess.
>> >
>> > I've worked on two different branches for potential fixes of your
>> > problems. Could you test topic/dma-fix and topic/dma-fix2 branches?
>> > git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
>> > Just pull one of them onto Linus' git HEAD.
>> >
>> > I guess we'll go with David's new patch, but still it's interesting
>> > whether my changes do anything good actually.
>> >
>> >
>> > Takashi
>> >
>>
>> On torvalds 623f6dc593, topic/dma-fix causes sound to be output as
>> alternating half-second bursts of noise and a few seconds of silence.
>> topic/dma-fix2 appears to work properly.
>
> OK, thanks for the feedback! Just to make sure, you're using
> PulseAudio, right?
> If so, it was still something wrong about mmap, and the secondary
> method (the fallback to the continuous page) looks like a safer
> approach in the end.
>
> I suppose that David's fix will be merged sooner or later. Meanwhile
> I'll work on the change in the sound driver side to make things a bit
> more robust. They don't conflict and both good applicable.
>
>
> thanks,
>
> Takashi
>
Ah, no, I think that wasn't clear. I use ALSA directly with mostly
default configuration, except an asym sets separate default playback and
record devices.
asound.conf:
defaults.pcm.card 1
defaults.ctl.card 1
pcm.!default {
type asym
playback.pcm
{
type plug
slave.pcm "dmix"
}
capture.pcm
{
type plug
slave.pcm {
type dsnoop
ipc_key 6793
slave.pcm "hw:U0x46d0x81d"
}
}
}
I think I wasn't able to set defaults.pcm.dmix.card and
defaults.pcm.dsnoop.card for some reason, not sure why. I can try that,
but I don't think it will affect this mmap issue.
Thanks,
Alex.
Re: Re: [PATCH] [v3] i2c: imx-lpi2c: Fix runtime PM imbalance on error
> > Can you point me to a discussion where it was decided that this is a > proper fix? I'd think we rather should fix pm_runtime_get_sync() but > maybe there are technical reasons against it. > There is a discussion here: https://lkml.org/lkml/2020/5/20/1100 There are many use cases that suppose pm_runtime_get_sync() will always increment the usage counter and do not check its return value. So I don't think we should adjust this function directly. As for this API, Dan suggested a replacement (wrapper) for later developers. I think this is the best solution. https://lore.kernel.org/patchwork/patch/1245375/ Regards, Dinghao
Re: [PATCH] [v2] iio: magnetometer: ak8974: Fix runtime PM imbalance on error
On Mon, 8 Jun 2020 14:12:18 +0200
Linus Walleij wrote:
> Hi Jonathan,
>
> sorry for missing this :(
>
> On Sun, May 31, 2020 at 4:00 PM Jonathan Cameron wrote:
> > On Tue, 26 May 2020 13:13:56 +0200
> > Linus Walleij wrote:
> >
> > > On Tue, May 26, 2020 at 12:47 PM Dinghao Liu
> > > wrote:
> > >
> > > > When devm_regmap_init_i2c() returns an error code, a pairing
> > > > runtime PM usage counter decrement is needed to keep the
> > > > counter balanced. For error paths after ak8974_set_power(),
> > > > ak8974_detect() and ak8974_reset(), things are the same.
> > > >
> > > > However, When iio_triggered_buffer_setup() returns an error
> > > > code, there will be two PM usgae counter decrements.
> > > >
> > > > Signed-off-by: Dinghao Liu
> > > > ---
> > > >
> > > > Changelog:
> > > >
> > > > v2: - Change 3 goto targets from "power_off" to
> > > > "disabel_pm". Remove unused lable "power_off".
> > > > Move 3 PM runtime related calls to the end of
> > > > the function.
> > >
> > > Thanks for fixing this Dinghao!
> > > Reviewed-by: Linus Walleij
> >
> > Could I have a fixes tag for this one?
>
> It's been there since the beginning so:
> Fixes: 7c94a8b2ee8c ("iio: magn: add a driver for AK8974")
There was a lot of fuzz in this one due to other changes to the driver.
I 'think' it went it cleanly though.
Applied to the fixes-togreg branch of iio.git and marked for
stable.
Thanks,
Jonathan
>
> Yours,
> Linus Walleij
Re: power-off delay/hang due to commit 6d25be57 (mainline)
On Fri, 12 Jun 2020 13:01:22 +0200 Sebastian Andrzej Siewior
wrote:
> + ACPI in case the ACPI folks see something obvious.
[...]
>> The "acpi_os_execute_deferred" messages were repeated many times in the
>> above line, then every 20-30 seconds again for several minutes. Then
>> suddenly a call trace appeared which was similar but not identical to
>> the one I posted upthread, and each line of the trace was followed by
>> the line ", acpi_os_execute_deferred". This went by quite quickly even
>> with the printk_delay I added, and I was unable to photograph the start
>> of it and couldn't get all of the subsequent output, but the screenshots
>> show some of it. After several minutes of this output I again did a
>> hard reboot.
>
> All good. I though that you will have one worker that is blocking but
> you have a lot of them. It appears that one is active and stuck and more
> are waiting.
>
> Could you look at acpi in /proc/interrupts 10 secs apart to see if it
> increments?
>
>grep -E 'acpi|smbus' /proc/interrupts
I tried this several times, 10 seconds apart and longer, but saw no
change, it was always this:
steve [ ~ ]$ grep -E 'acpi|smbus' /proc/interrupts
9: 0 5 0 0 0 0
0 0 0 0 0 0 IO-APIC
9-fasteoi acpi
16: 0 0 0 0 0 0
0 0 0 0 0 0 IO-APIC
16-fasteoi i801_smbus
> You could also do "echo t > /proc/sysrq-trigger" which gives you a lot
> of task state information, but at the end you will also see "howing busy
> workqueues and worker pools:" regarding the workqueue state. I am
> curious to see if you already have worker stuck in kacpid_notify with
> acpi_os_execute_deferred.
What am I supposed to do after "echo t > /proc/sysrq-trigger"? Both
before and after doing that I get an error trying to open the file:
root [ ~ ]# cat /proc/sysrq-trigger
cat: /proc/sysrq-trigger: Input/output error
> Now that we know that know that acpi_os_execute_deferred() is stuck,
> lets shed some light in what it is trying to do. The patch at the end
> will dump this information into the console buffer (The `dmesg' command
> will print the whole kernel buffer). I have no idea if this starts
> printing while the system is running or during shutdown. I would expect
> to see the Start line in acpi_os_execute_deferred() but not the End one.
>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 41168c027a5a4..0e983c558bcb5 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -840,7 +840,9 @@ static void acpi_os_execute_deferred(struct work_struct
> *work)
> {
> struct acpi_os_dpc *dpc = container_of(work, struct acpi_os_dpc, work);
>
> + pr_err("%s(%d) Start %px %pF(%px)\n", __func__, __LINE__, dpc,
> dpc->function, dpc->context);
> dpc->function(dpc->context);
> + pr_err("%s(%d) End %px %pF(%px)\n", __func__, __LINE__, dpc,
> dpc->function, dpc->context);
> kfree(dpc);
> }
>
> @@ -1096,6 +1098,8 @@ acpi_status acpi_os_execute(acpi_execute_type type,
>*/
> if (type == OSL_NOTIFY_HANDLER) {
> queue = kacpi_notify_wq;
> + pr_err("%s(%d) Adding %pS %px\n", __func__, __LINE__, function,
> +context);
> INIT_WORK(&dpc->work, acpi_os_execute_deferred);
> } else if (type == OSL_GPE_HANDLER) {
> queue = kacpid_wq;
I applied this patch to 5.6.4 and recompiled, and on the next boot with
that kernel, the kernel buffer (and kernel and system logs) began to get
flooded with these messages:
Jun 14 10:37:13 strobe-jhalfs kernel: [5.943987]
acpi_os_execute_deferred(843) Start 8fb82c7b6500
3edf1e05(8fb82c492990)
Jun 14 10:37:13 strobe-jhalfs kernel: [5.944102] acpi_os_execute(1101)
Adding acpi_ev_asynch_enable_gpe+0x0/0x2f 8fb82c492990
Jun 14 10:37:13 strobe-jhalfs kernel: [5.944104]
acpi_os_execute_deferred(845) End 8fb82c7b6500
3edf1e05(8fb82c492990)
Jun 14 10:37:13 strobe-jhalfs kernel: [5.944105]
acpi_os_execute_deferred(843) Start 8fb82b844800
2ba560ea(8fb82c492990)
Jun 14 10:37:13 strobe-jhalfs kernel: [5.944124]
acpi_os_execute_deferred(845) End 8fb82b844800
2ba560ea(8fb82c492990)
Jun 14 10:37:13 strobe-jhalfs kernel: [5.944288]
acpi_os_execute_deferred(843) Start 8fb82c7b6540
3edf1e05(8fb82c492990)
Jun 14 10:37:13 strobe-jhalfs kernel: [5.944387] acpi_os_execute(1101)
Adding acpi_ev_asynch_enable_gpe+0x0/0x2f 8fb82c492990
and so on without stopping. I could start X and avoid seeing the
messages, but was afraid the logs would fill up the root partition if I
let it keep going, so I rebooted with another kernel.
Was this message flood because I booted with "ignore_loglevel
initcall_debug"? In the logs there are also
Re: [PATCH v4 1/4] iio: chemical: scd30: add core driver
On Sun, 7 Jun 2020 13:59:48 +0200 Tomasz Duszynski wrote: > On Sat, Jun 06, 2020 at 04:06:52PM +0100, Jonathan Cameron wrote: > > On Wed, 3 Jun 2020 10:44:38 +0200 > > Tomasz Duszynski wrote: > > > > > Add Sensirion SCD30 carbon dioxide core driver. > > > > > > Signed-off-by: Tomasz Duszynski > > > > Hi Tomasz, > > > > One trivial suggestion on loosening a little what counts > > as 'enabled' for the calibration enable. > > > > Otherwise, the ABI for providing the pressure seems odd > > (I missed this previously). It doesn't make sense to provide > > a calibscale for pressure. This is an actual pressure value. > > > > Hi Jonathan, > > Well it does make sense to some extent. If you dig into theory > behind compensation it turns out that you compensate co2 > measurement by scaling it by a reciprocal of a given pressure. Interesting. Kind of makes sense. > > So it seems to match calibscale definition except measured pressure > is not exposed to us directly. > > But if the output channel is preferred then fine. I think that would still be the best plan because a random user won't understand why it would be calibscale. They will think they are writing a 'value' of pressure rather than something to do with scale. Jonathan > > > What we have done for similar cases in the past is to take the > > slightly wide interpretation of output channels. In some sense > > this is about telling the sensor what the value 'should' be > > so I would just use an output pressure channel to provide this > > control. Its a bit odd, but without an explosion in complex > > channel descriptions we haven't come up with a better option yet. > > > > However its also weird enough I'd suggest taking the unusual > > step of documenting it even though it is sort of standard ABI. > > > > Fair enough. > > > Thanks, > > > > Jonathan > > > > > --- > > > Documentation/ABI/testing/sysfs-bus-iio-scd30 | 34 + > > > MAINTAINERS | 6 + > > > drivers/iio/chemical/Kconfig | 11 + > > > drivers/iio/chemical/Makefile | 1 + > > > drivers/iio/chemical/scd30.h | 78 ++ > > > drivers/iio/chemical/scd30_core.c | 761 ++ > > > 6 files changed, 891 insertions(+) > > > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30 > > > create mode 100644 drivers/iio/chemical/scd30.h > > > create mode 100644 drivers/iio/chemical/scd30_core.c > > > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-scd30 > > > b/Documentation/ABI/testing/sysfs-bus-iio-scd30 > > > new file mode 100644 > > > index ..b9712f390bec > > > --- /dev/null > > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-scd30 > > > @@ -0,0 +1,34 @@ > > > +What:/sys/bus/iio/devices/iio:deviceX/calibration_auto_enable > > > +Date:June 2020 > > > +KernelVersion: 5.8 > > > +Contact: linux-...@vger.kernel.org > > > +Description: > > > + Contaminants build-up in the measurement chamber or optical > > > + elements deterioration leads to sensor drift. > > > + > > > + One can compensate for sensor drift by using automatic self > > > + calibration procedure (asc). > > > + > > > + Writing 1 or 0 to this attribute will respectively activate or > > > + deactivate asc. > > > + > > > + Upon reading current asc status is returned. > > > + > > > +What: > > > /sys/bus/iio/devices/iio:deviceX/calibration_forced_value > > > +Date:June 2020 > > > +KernelVersion: 5.8 > > > +Contact: linux-...@vger.kernel.org > > > +Description: > > > + Contaminants build-up in the measurement chamber or optical > > > + elements deterioration leads to sensor drift. > > > + > > > + One can compensate for sensor drift by using forced > > > + recalibration (frc). This is useful in case there's known > > > + co2 reference available nearby the sensor. > > > + > > > + Picking value from the range [400 1 2000] and writing it to the > > > + sensor will set frc. > > > + > > > + Upon reading current frc value is returned. Note that after > > > + power cycling default value (i.e 400) is returned even though > > > + internally sensor had recalibrated itself. > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index 60ed2963efaa..41a509cca6f1 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -15137,6 +15137,12 @@ S: Maintained > > > F: drivers/misc/phantom.c > > > F: include/uapi/linux/phantom.h > > > > > > +SENSIRION SCD30 CARBON DIOXIDE SENSOR DRIVER > > > +M: Tomasz Duszynski > > > +S: Maintained > > > +F: drivers/iio/chemical/scd30.h > > > +F: drivers/iio/chemical/scd30_core.c > > > + > > > SENSIRION SPS30 AIR POLLUTION SENSOR DRIVER > > > M: Tomasz Duszynski > > > S: Maintained > > > diff --git a/drivers/iio/chemical/Kc
Re: [PATCH v2 6/6] iio: remove left-over parent assignments
On Thu, 11 Jun 2020 06:52:00 + "Ardelean, Alexandru" wrote: > On Sat, 2020-06-06 at 17:05 +0100, Jonathan Cameron wrote: > > [External] > > > > On Wed, 3 Jun 2020 14:40:23 +0300 > > Alexandru Ardelean wrote: > > > > > These were found by doing some shell magic: > > > > > > for file in $(git grep -w devm_iio_device_alloc | cut -d: -f1 | sort | > > > uniq) > > > ; do > > > if grep 'parent =' $file | grep -v trig | grep -vq devm_; then > > > echo "$file -> $(grep "parent =" $file)" > > > fi > > > done > > > --- > > > > > > The output is bearable [after the semantic patch is applied]. > > > There is a mix of trigger assignments with some iio device parent > > > assignments that are removed via this patch. > > > > > > Signed-off-by: Alexandru Ardelean > > > > I added a bunch more via a grep of simple parent\ = > > and eyeballing it. Much easier to do after your patches as far > > fewer entries. > > > > vcnl3020 (new) > > ms5611 (hidden via an extra call) > > st_sensors_spi (hidden via an extra call) > > st_sensors_i2c (hidden via an extra call) > > cros_ec_sensors_core (hidden via an extra call) > > I rebased your branch with my work-branch. > That displayes neatly all the stuff you've added. > > drivers/iio/adc/ltc2497-core.c > drivers/iio/chemical/atlas-ezo-sensor.c > drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c > drivers/iio/common/st_sensors/st_sensors_i2c.c > drivers/iio/common/st_sensors/st_sensors_spi.c > drivers/iio/pressure/ms5611_core.c > drivers/iio/proximity/vcnl3020.c > > All look good to me. > So, purely for record purporses, for all the files above: > > Reviewed-by: Alexandru Ardelean Thanks. I should probably have done this as a separate patch for easier reviewing but he thought of left overs to the left overs patch annoyed me ;) Anyhow, end result is the same and the history look sane so I'll leave it as it stands. Thanks, Jonathan > > > > --- > > > drivers/iio/accel/kxcjk-1013.c| 1 - > > > drivers/iio/accel/mma8452.c | 1 - > > > drivers/iio/accel/mma9553.c | 1 - > > > drivers/iio/adc/ad7192.c | 1 - > > > drivers/iio/adc/hx711.c | 1 - > > > drivers/iio/adc/max1363.c | 2 -- > > > drivers/iio/adc/mcp3911.c | 1 - > > > drivers/iio/adc/qcom-spmi-iadc.c | 1 - > > > drivers/iio/amplifiers/ad8366.c | 1 - > > > drivers/iio/chemical/vz89x.c | 1 - > > > drivers/iio/dac/ad5770r.c | 1 - > > > drivers/iio/health/afe4403.c | 1 - > > > drivers/iio/health/afe4404.c | 1 - > > > drivers/iio/humidity/dht11.c | 1 - > > > drivers/iio/humidity/hts221_core.c| 1 - > > > drivers/iio/imu/inv_mpu6050/inv_mpu_core.c| 1 - > > > drivers/iio/light/cm3605.c| 1 - > > > drivers/iio/light/ltr501.c| 1 - > > > drivers/iio/magnetometer/ak8975.c | 1 - > > > drivers/iio/orientation/hid-sensor-rotation.c | 1 - > > > drivers/iio/potentiostat/lmp91000.c | 1 - > > > drivers/iio/proximity/ping.c | 1 - > > > drivers/iio/proximity/pulsedlight-lidar-lite-v2.c | 1 - > > > drivers/iio/proximity/srf04.c | 1 - > > > drivers/iio/proximity/srf08.c | 1 - > > > drivers/iio/temperature/tsys01.c | 1 - > > > drivers/staging/iio/addac/adt7316.c | 1 - > > > 27 files changed, 28 deletions(-) > > > > > > diff --git a/drivers/iio/accel/kxcjk-1013.c > > > b/drivers/iio/accel/kxcjk-1013.c > > > index c9924a65c32a..6b93521c0e17 100644 > > > --- a/drivers/iio/accel/kxcjk-1013.c > > > +++ b/drivers/iio/accel/kxcjk-1013.c > > > @@ -1311,7 +1311,6 @@ static int kxcjk1013_probe(struct i2c_client > > > *client, > > > > > > mutex_init(&data->mutex); > > > > > > - indio_dev->dev.parent = &client->dev; > > > indio_dev->channels = kxcjk1013_channels; > > > indio_dev->num_channels = ARRAY_SIZE(kxcjk1013_channels); > > > indio_dev->available_scan_masks = kxcjk1013_scan_masks; > > > diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c > > > index 00e100fc845a..ef3df402fc3c 100644 > > > --- a/drivers/iio/accel/mma8452.c > > > +++ b/drivers/iio/accel/mma8452.c > > > @@ -1592,7 +1592,6 @@ static int mma8452_probe(struct i2c_client *client, > > > i2c_set_clientdata(client, indio_dev); > > > indio_dev->info = &mma8452_info; > > > indio_dev->name = id->name; > > > - indio_dev->dev.parent = &client->dev; > > > indio_dev->modes = INDIO_DIRECT_MODE; > > > indio_dev->channels = data->chip_info->channels; > > > indio_dev->num_channels = data->chip_info->num_channels; > > > diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma
Re: [PATCH] drm/vc4: fix ref count leak in vc4_dsi_encoder_enable
> in vc4_dsi_encoder_enable, the call to pm_runtime_get_sync increments > the counter even in case of failure, leading to incorrect > ref count. In case of failure, decrement the ref count before returning. * Can the term “reference count” become relevant also for this commit message besides other possible adjustments? * Can it be nicer to combine proposed updates for this software module as a patch series (with a cover letter)? * Would you like to add the tag “Fixes”? … > +++ b/drivers/gpu/drm/vc4/vc4_dsi.c … > @@ -1088,6 +1088,8 @@ static void vc4_dsi_encoder_enable(struct drm_encoder > *encoder) > dev_info(&dsi->pdev->dev, "DSI regs after:\n"); > drm_print_regset32(&p, &dsi->regset); > } > +out: > + pm_runtime_put(dev); > } … * Perhaps use the label “put_runtime” instead? * Do you propose to perform an additional function call always (and not only according to failure cases)? * How do you think about calling the function “pm_runtime_put_noidle”? Regards, Markus
[PATCH] mm: memcontrol: Fix do not put the css reference
We should put the css reference when memory allocation failed.
Fixes: f0a3a24b532d ("mm: memcg/slab: rework non-root kmem_cache lifecycle
management")
Signed-off-by: Muchun Song
---
mm/memcontrol.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 0b38b6ad547d..2323d811ee8e 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2772,8 +2772,10 @@ static void memcg_schedule_kmem_cache_create(struct
mem_cgroup *memcg,
return;
cw = kmalloc(sizeof(*cw), GFP_NOWAIT | __GFP_NOWARN);
- if (!cw)
+ if (!cw) {
+ css_put(&memcg->css);
return;
+ }
cw->memcg = memcg;
cw->cachep = cachep;
--
2.11.0
Re: [PATCH] drm/vc4: fix ref count leak in vc4_dsi_encoder_enable
On Sun, Jun 14, 2020 at 9:55 AM Navid Emamdoost wrote: > > in vc4_dsi_encoder_enable, the call to pm_runtime_get_sync increments > the counter even in case of failure, leading to incorrect > ref count. In case of failure, decrement the ref count before returning. ... > +out: > + pm_runtime_put(dev); Better to use pm_runtime_put_noidle() for error case. And here is a change of semantics, i.e. before your patch there was no put at all. How did you test this? -- With Best Regards, Andy Shevchenko
[PATCH 0/3] mm/slub: Fix slabs_node return value
The slabs_node() always return zero when CONFIG_SLUB_DEBUG is disabled. But some codes determine whether slab is empty by checking the return value of slabs_node(). As you know, the result is not correct. we move the nr_slabs of kmem_cache_node out of the CONFIG_SLUB_DEBUG. So we can get the corrent value returned by the slabs_node(). Muchun Song (3): mm/slub: Fix slabs_node return value when CONFIG_SLUB_DEBUG disabled mm/slub: Use node_nr_slabs() instead of slabs_node() mm/slub: Fix release all resources used by a slab cache mm/slab.h | 2 +- mm/slub.c | 93 +-- 2 files changed, 50 insertions(+), 45 deletions(-) -- 2.11.0
[PATCH 2/3] mm/slub: Use node_nr_slabs() instead of slabs_node()
In the some code, we already get the kmem_cache_node, so we can
use node_nr_slabs() directly instead of slabs_node(). Check the
condition of n->nr_partial can also be removed because we can get
the correct result via node_nr_slabs().
Signed-off-by: Muchun Song
---
mm/slub.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index 1a3e6a5b7287..b73505df3de2 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3829,7 +3829,7 @@ bool __kmem_cache_empty(struct kmem_cache *s)
struct kmem_cache_node *n;
for_each_kmem_cache_node(s, node, n)
- if (n->nr_partial || slabs_node(s, node))
+ if (node_nr_slabs(n))
return false;
return true;
}
@@ -3846,7 +3846,7 @@ int __kmem_cache_shutdown(struct kmem_cache *s)
/* Attempt to free all objects */
for_each_kmem_cache_node(s, node, n) {
free_partial(s, n);
- if (n->nr_partial || slabs_node(s, node))
+ if (node_nr_slabs(n))
return 1;
}
sysfs_slab_remove(s);
@@ -4126,7 +4126,7 @@ int __kmem_cache_shrink(struct kmem_cache *s)
list_for_each_entry_safe(page, t, &discard, slab_list)
discard_slab(s, page);
- if (slabs_node(s, node))
+ if (node_nr_slabs(n))
ret = 1;
}
@@ -4201,7 +4201,7 @@ static void slab_mem_offline_callback(void *arg)
* and offline_pages() function shouldn't call this
* callback. So, we must fail.
*/
- BUG_ON(slabs_node(s, offline_node));
+ BUG_ON(node_nr_slabs(n));
s->node[offline_node] = NULL;
kmem_cache_free(kmem_cache_node, n);
--
2.11.0
[PATCH 1/3] mm/slub: Fix slabs_node return value when CONFIG_SLUB_DEBUG disabled
The slabs_node() always return zero when CONFIG_SLUB_DEBUG is disabled.
But some codes determine whether slab is empty by checking the return
value of slabs_node(). As you know, the result is not correct. This
problem can be reproduce by the follow code(and boot system with the
cmdline of "slub_nomerge"):
void *objs[32];
struct kmem_cache *cache = kmem_cache_create("kmem-test", 128, 0,
0, 0);
if (cache) {
int i;
/* Make a full slab */
for (i = 0; i < ARRAY_SIZE(objs); i++)
objs[i] = kmem_cache_alloc(cache, GFP_KERNEL_ACCOUNT);
/*
* This really should fail because the slab cache still has
* objects. But we did destroy the @cache because of zero
* returned by slabs_node().
*/
kmem_cache_destroy(cache);
}
To fix it, we can move the nr_slabs of kmem_cache_node out of the
CONFIG_SLUB_DEBUG. So we can get the corrent value returned by the
slabs_node().
With this patch applied, we will get a warning message and stack
trace in the dmesg.
Signed-off-by: Muchun Song
---
mm/slab.h | 2 +-
mm/slub.c | 80 +--
2 files changed, 43 insertions(+), 39 deletions(-)
diff --git a/mm/slab.h b/mm/slab.h
index 0b91f2a7b033..062d4542b7e2 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -619,8 +619,8 @@ struct kmem_cache_node {
#ifdef CONFIG_SLUB
unsigned long nr_partial;
struct list_head partial;
-#ifdef CONFIG_SLUB_DEBUG
atomic_long_t nr_slabs;
+#ifdef CONFIG_SLUB_DEBUG
atomic_long_t total_objects;
struct list_head full;
#endif
diff --git a/mm/slub.c b/mm/slub.c
index 49b5cb7da318..1a3e6a5b7287 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1070,39 +1070,14 @@ static void remove_full(struct kmem_cache *s, struct
kmem_cache_node *n, struct
list_del(&page->slab_list);
}
-/* Tracking of the number of slabs for debugging purposes */
-static inline unsigned long slabs_node(struct kmem_cache *s, int node)
+/* Tracking of the number of objects for debugging purposes */
+static inline void inc_objects_node(struct kmem_cache_node *n, int objects)
{
- struct kmem_cache_node *n = get_node(s, node);
-
- return atomic_long_read(&n->nr_slabs);
+ atomic_long_add(objects, &n->total_objects);
}
-static inline unsigned long node_nr_slabs(struct kmem_cache_node *n)
+static inline void dec_objects_node(struct kmem_cache_node *n, int objects)
{
- return atomic_long_read(&n->nr_slabs);
-}
-
-static inline void inc_slabs_node(struct kmem_cache *s, int node, int objects)
-{
- struct kmem_cache_node *n = get_node(s, node);
-
- /*
-* May be called early in order to allocate a slab for the
-* kmem_cache_node structure. Solve the chicken-egg
-* dilemma by deferring the increment of the count during
-* bootstrap (see early_kmem_cache_node_alloc).
-*/
- if (likely(n)) {
- atomic_long_inc(&n->nr_slabs);
- atomic_long_add(objects, &n->total_objects);
- }
-}
-static inline void dec_slabs_node(struct kmem_cache *s, int node, int objects)
-{
- struct kmem_cache_node *n = get_node(s, node);
-
- atomic_long_dec(&n->nr_slabs);
atomic_long_sub(objects, &n->total_objects);
}
@@ -1413,15 +1388,8 @@ slab_flags_t kmem_cache_flags(unsigned int object_size,
#define disable_higher_order_debug 0
-static inline unsigned long slabs_node(struct kmem_cache *s, int node)
- { return 0; }
-static inline unsigned long node_nr_slabs(struct kmem_cache_node *n)
- { return 0; }
-static inline void inc_slabs_node(struct kmem_cache *s, int node,
- int objects) {}
-static inline void dec_slabs_node(struct kmem_cache *s, int node,
- int objects) {}
-
+static inline void inc_objects_node(struct kmem_cache_node *n, int objects) {}
+static inline void dec_objects_node(struct kmem_cache_node *n, int objects) {}
static bool freelist_corrupted(struct kmem_cache *s, struct page *page,
void *freelist, void *nextfree)
{
@@ -1429,6 +1397,42 @@ static bool freelist_corrupted(struct kmem_cache *s,
struct page *page,
}
#endif /* CONFIG_SLUB_DEBUG */
+static inline unsigned long slabs_node(struct kmem_cache *s, int node)
+{
+ struct kmem_cache_node *n = get_node(s, node);
+
+ return atomic_long_read(&n->nr_slabs);
+}
+
+static inline unsigned long node_nr_slabs(struct kmem_cache_node *n)
+{
+ return atomic_long_read(&n->nr_slabs);
+}
+
+static inline void inc_slabs_node(struct kmem_cache *s, int node, int objects)
+{
+ struct kmem_cache_node *n = get_node(s, node);
+
+ /*
+* May be called early in order to allocate a slab for the
+
[PATCH 3/3] mm/slub: Fix release all resources used by a slab cache
The function of __kmem_cache_shutdown() is that release all resources
used by the slab cache, while currently it stop release resources when
the preceding node is not empty.
Signed-off-by: Muchun Song
---
mm/slub.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index b73505df3de2..4e477ef0f2b9 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3839,6 +3839,7 @@ bool __kmem_cache_empty(struct kmem_cache *s)
*/
int __kmem_cache_shutdown(struct kmem_cache *s)
{
+ int ret = 0;
int node;
struct kmem_cache_node *n;
@@ -3846,11 +3847,11 @@ int __kmem_cache_shutdown(struct kmem_cache *s)
/* Attempt to free all objects */
for_each_kmem_cache_node(s, node, n) {
free_partial(s, n);
- if (node_nr_slabs(n))
- return 1;
+ if (!ret && node_nr_slabs(n))
+ ret = 1;
}
sysfs_slab_remove(s);
- return 0;
+ return ret;
}
/
--
2.11.0
Re: RFC: a failing pm_runtime_get increases the refcnt?
On Sun, Jun 14, 2020 at 1:00 PM Geert Uytterhoeven wrote:
>
> Hi Andy,
>
> On Sun, Jun 14, 2020 at 11:43 AM Andy Shevchenko
> wrote:
> > On Sun, Jun 14, 2020 at 12:34 PM Andy Shevchenko
> > wrote:
> > >
> > > On Sun, Jun 14, 2020 at 12:10 PM Wolfram Sang wrote:
> > > > both in the I2C subsystem and also for Renesas drivers I maintain, I am
> > > > starting to get boilerplate patches doing some pm_runtime_put_* variant
> > > > because a failing pm_runtime_get is supposed to increase the ref
> > > > counters? Really? This feels wrong and unintuitive to me.
> > >
> > > Yeah, that is a well known issue with PM (I even have for a long time
> > > a coccinelle script, when I realized myself that there are a lot of
> > > cases like this, but someone else discovered this recently, like
> > > opening a can of worms).
> > >
> > > > I expect there
> > > > has been a discussion around it but I couldn't find it.
> > >
> > > Rafael explained (again) recently this. I can't find it quickly,
> > > unfortunately.
> >
> > I _think_ this discussion, but may be it's simple another tentacle of
> > the same octopus.
> > https://patchwork.ozlabs.org/project/linux-tegra/patch/20200520095148.10995-1-dinghao@zju.edu.cn/
>
> Thanks, hadn't read that one! (so I was still at -1 from
> http://sweng.the-davies.net/Home/rustys-api-design-manifesto ;-)
This one seems the starting point:
https://lkml.org/lkml/2020/5/20/1100
> So "pm_runtime_put_noidle()" is the (definitive?) one to pair with a
> pm_runtime_get_sync() failure?
Depends. If you are using autosuspend, then put_autosuspend() probably
is the right one.
> > > > I wonder why we
> > > > don't fix the code where the incremented refcount is expected for some
> > > > reason.
> > >
> > > The main idea behind API that a lot of drivers do *not* check error
> > > codes from runtime PM, so, we need to keep balance in case of
> > >
> > > pm_runtime_get(...);
> > > ...
> > > pm_runtime_put(...);
>
> I've always[*] considered a pm_runtime_get_sync() failure to be fatal
> (or: cannot happen), and that there's nothing that can be done to
> recover. Hence I never checked the function's return value.
> Was that wrong?
>
> [*] at least on Renesas SoCs with Clock and/or Power Domains.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v4 2/2] ovl: call underlying get_unmapped_area() routine. propogate FMODE_HUGETLBFS
On Fri, Jun 12, 2020 at 3:57 AM Mike Kravetz wrote:
>
> The core routine get_unmapped_area will call a filesystem specific version
> of get_unmapped_area if it exists in file operations. If a file is on a
> union/overlay, overlayfs does not contain a get_unmapped_area f_op and the
> underlying filesystem routine may be ignored. Add an overlayfs f_op to call
> the underlying f_op if it exists.
>
> The routine is_file_hugetlbfs() is used to determine if a file is on
> hugetlbfs. This is determined by f_mode & FMODE_HUGETLBFS. Copy the mode
> to the overlayfs file during open so that is_file_hugetlbfs() will work as
> intended.
>
> These two issues can result in the BUG as shown in [1].
>
> [1] https://lore.kernel.org/linux-mm/b4684e05a2968...@google.com/
>
> Reported-by: syzbot+d6ec23007e951dadf...@syzkaller.appspotmail.com
> Signed-off-by: Miklos Szeredi
> Signed-off-by: Mike Kravetz
> ---
> fs/overlayfs/file.c | 21 +
> 1 file changed, 21 insertions(+)
>
> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> index 87c362f65448..41e5746ba3c6 100644
> --- a/fs/overlayfs/file.c
> +++ b/fs/overlayfs/file.c
> @@ -124,6 +124,8 @@ static int ovl_real_fdget(const struct file *file, struct
> fd *real)
> return ovl_real_fdget_meta(file, real, false);
> }
>
> +#define OVL_F_MODE_TO_UPPER(FMODE_HUGETLBFS)
> +
> static int ovl_open(struct inode *inode, struct file *file)
> {
> struct file *realfile;
> @@ -140,6 +142,9 @@ static int ovl_open(struct inode *inode, struct file
> *file)
> if (IS_ERR(realfile))
> return PTR_ERR(realfile);
>
> + /* Copy modes from underlying file */
> + file->f_mode |= (realfile->f_mode & OVL_F_MODE_TO_UPPER);
> +
The name OVL_F_MODE_TO_UPPER is strange because
ovl_open() may be opening a file from lower or from upper.
Please see attached patches.
They are not enough to solve the syzbot repro, but if you do want to
fix hugetlb/overlay interop I think they will be necessary.
Thanks,
Amir.
From 691915fd8d4bb2c0f944b7d8e504a323b8e53b69 Mon Sep 17 00:00:00 2001
From: Amir Goldstein
Date: Sun, 14 Jun 2020 13:51:30 +0300
Subject: [PATCH 2/2] ovl: warn about unsupported file operations on upper fs
syzbot has reported a crash in a test case where overlayfs uses
hugetlbfs as upper fs.
Since hugetlbfs has no write() nor write_iter() file ops, there seems
to be little value in supporting this configuration, however, we do not
want to regress strange use cases that me be using such configuration.
As a minimum, check for this case and warn about it on mount time as
well as adding this check for the strict requirements set of remote
upper fs.
Signed-off-by: Amir Goldstein
---
fs/overlayfs/super.c | 60
1 file changed, 39 insertions(+), 21 deletions(-)
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 91476bc422f9..5bee70eb8f64 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -6,6 +6,7 @@
#include
#include
+#include
#include
#include
#include
@@ -1131,42 +1132,61 @@ static int ovl_get_upper(struct super_block *sb, struct
ovl_fs *ofs,
}
/*
- * Returns 1 if RENAME_WHITEOUT is supported, 0 if not supported and
+ * Returns 1 if required ops are supported, 0 if not supported and
* negative values if error is encountered.
*/
-static int ovl_check_rename_whiteout(struct dentry *workdir)
+static int ovl_check_supported_ops(struct ovl_fs *ofs)
{
- struct inode *dir = d_inode(workdir);
- struct dentry *temp;
+ struct inode *dir = d_inode(ofs->workdir);
+ struct dentry *temp = NULL;
struct dentry *dest;
struct dentry *whiteout;
struct name_snapshot name;
+ struct path path;
+ struct file *file;
+ unsigned int unsupported;
int err;
inode_lock_nested(dir, I_MUTEX_PARENT);
- temp = ovl_create_temp(workdir, OVL_CATTR(S_IFREG | 0));
- err = PTR_ERR(temp);
- if (IS_ERR(temp))
+ path.mnt = ovl_upper_mnt(ofs);
+ path.dentry = ovl_create_temp(ofs->workdir, OVL_CATTR(S_IFREG | 0));
+ err = PTR_ERR(path.dentry);
+ if (IS_ERR(path.dentry))
goto out_unlock;
- dest = ovl_lookup_temp(workdir);
- err = PTR_ERR(dest);
- if (IS_ERR(dest)) {
- dput(temp);
+ temp = path.dentry;
+ file = dentry_open(&path, O_RDWR, current_cred());
+ err = PTR_ERR(file);
+ if (IS_ERR(file))
+ goto out_unlock;
+
+ unsupported = OVL_UPPER_FMODE_MASK & ~file->f_mode;
+ fput(file);
+ if (unsupported) {
+ err = 0;
+ pr_warn("upper fs does not support required file operations
(%x).\n",
+ unsupported);
goto out_unlock;
}
+ dest = ovl_lookup_temp(ofs->workdir);
+ err = PTR_ERR(dest);
+ if (IS_ERR(dest))
+ goto out_unloc
RE: [PATCH v1 RFC 1/2] spi: introduce fallback to pio
On 2020/06/12 22:16 Mark Brown wrote: > On Fri, Jun 12, 2020 at 01:48:41PM +, Robin Gong wrote: > > On 2020/06/12 18:14 Mark Brown wrote: > > > > Please look at the formatting of your e-mails - they're really hard > > > to read. The line length is over 80 columns and there's no breaks between > paragraphs. > > > Sorry for that, seems my outlook format issue, hope it's ok now this > > time :) > > Yes, looks good thanks! > > > > Client could enable this feature by choosing SPI_MASTER_FALLBACK > > > freely without any impact on others. > > > > SPI_MASTER_FALLBACK. If this works why would any driver not enable > > > the flag? > > > Just make sure little impact if it's not good enough and potential > > issue may come out after it's merged into mainline. TBH, I'm not sure > > if it has taken care all in spi core. Besides, I don't know if other spi > > client need > this feature. > > It's not something that's going to come up a lot for most devices, it'd be a > mapping failure due to running out of memory or something, but your point > about that being possible is valid. > > > > > Any error happen in DMA could fallback to PIO , seems a nice to > > > > have, > > > because it could > > > > give chance to run in PIO which is more reliable. But if there is > > > > also error in > > > > PIO, thus may loop here, it's better adding limit try times here? > > > > An error doesn't mean nothing happened on the bus, an error could > > > for example also be something like a FIFO overrun which corrupts data. > > > Do you mean fallback to PIO may cause FIFO overrun since some latency > > involved so that this patch seems not useful as expected? > > No, I mean that the reason the DMA transfer fails may be something that > happens after we've started putting things on the bus - the bit about FIFOs is > just a random example of an error that could happen. > Sorry Mark for that I can't get your point... The bus error such as data corrupt seems not the spi core's business since it can only be caught in spi controller driver or upper level such as mtd driver (spi-nor) which know what's the failure happen at spi bus HW level or what's the correct data/message. In other words, spi core can't detect such error by transfer_one(). > > > It *could* but only in extreme situations, and again this isn't just > > > handling errors from failure to prepare the hardware but also > > > anything that happens after it. > > > Okay, understood your point. You prefer to some interface provided by > > dma engine before dmaengine_prep_slave_sg so that can_dma() can know > > if this dma channel is ready indeed. But unfortunately, seems there is no > one > > Well, this is free software and everything can be modified! The other option > would be framework changes in SPI that allowed us to indicate from the driver > that an error occured before we started doing anything to the hardware (like > happens here) through something like a special error code or splitting up the > API. Yes, but both assume spi controller driver could detect such dma failure before dmaengine_prep_*(). Let's wait Vinod's comment for that if dmaengine_slave_config could keep direction. But despite of that case, do you think this patch is valid for transfer_one() failue in dma and fallback to pio?
Re: [RFC 1/3] lib: copy_{from,to}_user using gup & kmap_atomic()
Hi, On Sat, Jun 13, 2020 at 02:15:52PM +0100, Russell King - ARM Linux admin wrote: > On Sat, Jun 13, 2020 at 05:34:32PM +0530, afzal mohammed wrote: > > i think C > > library cuts any size read, write to page size (if it exceeds) & > > invokes the system call. > You can't make that assumption about read(2). stdio in the C library > may read a page size of data at a time, but programs are allowed to > call read(2) directly, and the C library will pass such a call straight > through to the kernel. So, if userspace requests a 16k read via > read(2), then read(2) will be invoked covering 16k. > > As an extreme case, for example: > > $ strace -e read dd if=/dev/zero of=/dev/null bs=1048576 count=1 > read(0, > "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., > 1048576) = 1048576 Okay. Yes, observed that dd is passing whatever is the 'bs' to Kernel and from the 'dd' sources (of busybox), it is invoking read system call directly passing 'bs', so it is the tmpfs read that is splitting it to page size as mentioned by Arnd. Regards afzal
[PATCH] MAINTAINERS: move *@vger.kernel.org from M: to L:
Signed-off-by: Alexander A. Klimov --- MAINTAINERS | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 68f21d46614c..86e9276ddb1d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8995,8 +8995,6 @@ F:drivers/net/ethernet/sgi/ioc3-eth.c IOMAP FILESYSTEM LIBRARY M: Christoph Hellwig M: Darrick J. Wong -M: linux-...@vger.kernel.org -M: linux-fsde...@vger.kernel.org L: linux-...@vger.kernel.org L: linux-fsde...@vger.kernel.org S: Supported @@ -13570,7 +13568,7 @@ F: arch/mips/include/asm/mach-pistachio/ F: arch/mips/pistachio/ PKTCDVD DRIVER -M: linux-bl...@vger.kernel.org +L: linux-bl...@vger.kernel.org S: Orphan F: drivers/block/pktcdvd.c F: include/linux/pktcdvd.h @@ -18799,7 +18797,6 @@ F: drivers/xen/*swiotlb* XFS FILESYSTEM M: Darrick J. Wong -M: linux-...@vger.kernel.org L: linux-...@vger.kernel.org S: Supported W: http://xfs.org/ -- 2.27.0
Re: [PATCH] drm/amdgpu: fix ref count leak in amdgpu_display_crtc_set_config
> in amdgpu_display_crtc_set_config, … * Can the term “reference count” become relevant also for this commit message besides other possible adjustments? * Can it be nicer to combine proposed updates for this software module as a patch series (with a cover letter)? * Would you like to add the tag “Fixes”? … > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c … > @@ -306,6 +306,7 @@ int amdgpu_display_crtc_set_config(struct drm_mode_set > *set, > adev->have_disp_power_ref = false; > } > > +out: > /* drop the power reference we got coming in here */ > pm_runtime_put_autosuspend(dev->dev); > return ret; Perhaps use the label “put_runtime” instead? Regards, Markus
Re: [PATCH v5 1/4] iio: chemical: scd30: add core driver
On Tue, 9 Jun 2020 04:18:56 +0800
kernel test robot wrote:
> Hi Tomasz,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on iio/togreg]
> [also build test WARNING on robh/for-next linux/master v5.7]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see
> https://stackoverflow.com/a/37406982]
>
> url:
> https://github.com/0day-ci/linux/commits/Tomasz-Duszynski/Add-support-for-SCD30-sensor/20200608-020304
> base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot
I'll tidy this up whilst applying assuming nothing else comes up.
No comment on it's validity as a warning, but in my view it
is always squashing warnings to keep the noise low for any
more useful ones (as long as it's not too costly).
Jonathan
>
>
> cppcheck warnings: (new ones prefixed by >>)
>
> >> drivers/iio/chemical/scd30_core.c:93:31: warning: Clarify calculation
> >> precedence for '&' and '?'. [clarifyCalculation]
> sign = float32 & BIT(31) ? -1 : 1,
> ^
>
> vim +93 drivers/iio/chemical/scd30_core.c
>
> 87
> 88/* simplified float to fixed point conversion with a scaling
> factor of 0.01 */
> 89static int scd30_float_to_fp(int float32)
> 90{
> 91int fraction, shift,
> 92mantissa = float32 & GENMASK(22, 0),
> > 93sign = float32 & BIT(31) ? -1 : 1,
> 94exp = (float32 & ~BIT(31)) >> 23;
> 95
> 96/* special case 0 */
> 97if (!exp && !mantissa)
> 98return 0;
> 99
>100exp -= 127;
>101if (exp < 0) {
>102exp = -exp;
>103/* return values ranging from 1 to 99 */
>104return sign * BIT(23) + mantissa) * 100) >>
> 23) >> exp);
>105}
>106
>107/* return values starting at 100 */
>108shift = 23 - exp;
>109float32 = BIT(exp) + (mantissa >> shift);
>110fraction = mantissa & GENMASK(shift - 1, 0);
>111
>112return sign * (float32 * 100 + ((fraction * 100) >>
> shift));
>113}
>114
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
Re: [PATCH v5 0/4] Add support for SCD30 sensor
On Sun, 7 Jun 2020 19:58:08 +0200 Tomasz Duszynski wrote: > Following series adds support for Sensirion SCD30 sensor module capable of > measuring carbon dioxide, temperature and relative humidity. CO2 measurements > base on NDIR principle while temperature and relative humidity are measured by > the on board SHT31. As for sensor communication, both I2C and serial > interfaces > are supported. Hi Tomasz, All looks good to me. I'll let it sit on the list a bit longer though to give time for anyone else to review if they wish and ideally pick up a DT review if Rob has time. It isn't unheard of me to somehow loose a set down the back of the sofa, so do poke me if I seem to to have lost this in a few weeks time! (I'll try not to of course!) Thanks, Jonathan > > v5: > * set pressure calibration via output channel > * use kstrtobool() to read value into _enabled attribute > * drop explicit parent asignment as the default one is good enough > (seems 'iio: core: pass parent device as parameter during allocation' >series was accepted) > > v4: > * improve formatting > * improve error handling readability > * fix message validity check on serial write > > v3: > * simplify code by scaling temperature & humidity in _read_meas() > * update realbits in scan types > * s/adjecent/adjacent > * drop IIO_CHAN_INFO_RAW from _write_raw_get_fmt because there's no raw > output channel > * rework locking in _read_raw > * fix endianess problem on BE machine > * align timestamp properly before pushing to buffers > * explain why interrupt gets disabled after registration > * add trigger validation > * drop SCALE for temperature and humidity channel as they are processed > * register action which stops measuring after starting measurements > * spit generic calibration attr into two doing specific things > * add comment explaining why priv in struct scd30_state is for > * rename node in binding example to co2-sensor > > v2: > * move asm/byteorder.h towards the bottom of include list > * make channel address names in enum more specific > * add postfixes to defines and extra comments > * drop unneeded i2c include from scd30 header > * break generic command sending function into specialized options > * expose automatic calibration and forced calibration via the same attr > * use SAMP_FREQ to set frequency instead of meas_interval attr > * use CALISCALE to set pressure compensation instead of pressure_comp attr > * use CALIBBIAS to set temperature offset instead of temp_offset attr > * fix order in MAINTAINERS > * drop attribute allowing one to reset sensor > * as we have dt probing drop board file based probing (i2c_device_id) > * merge patches touching related files > * use fwnode API to retrieve interrupt from dt > * fix interrupt-parent spelling > * change binding license > * drop supply from required property > > Tomasz Duszynski (4): > iio: chemical: scd30: add core driver > iio: chemical: scd30: add I2C interface driver > iio: chemical: scd30: add serial interface driver > dt-bindings: iio: scd30: add device binding file > > Documentation/ABI/testing/sysfs-bus-iio-scd30 | 34 + > .../iio/chemical/sensirion,scd30.yaml | 68 ++ > MAINTAINERS | 9 + > drivers/iio/chemical/Kconfig | 33 + > drivers/iio/chemical/Makefile | 3 + > drivers/iio/chemical/scd30.h | 78 ++ > drivers/iio/chemical/scd30_core.c | 770 ++ > drivers/iio/chemical/scd30_i2c.c | 139 > drivers/iio/chemical/scd30_serial.c | 263 ++ > 9 files changed, 1397 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30 > create mode 100644 > Documentation/devicetree/bindings/iio/chemical/sensirion,scd30.yaml > create mode 100644 drivers/iio/chemical/scd30.h > create mode 100644 drivers/iio/chemical/scd30_core.c > create mode 100644 drivers/iio/chemical/scd30_i2c.c > create mode 100644 drivers/iio/chemical/scd30_serial.c > > -- > 2.27.0 >
