> The return value of fiji_populate_smc_boot_level() is needs to be checked.
should?
> An error handling is also needed to phm_find_boot_level() to reset the
> boot level when the function fails. A proper implementation can be found
> in tonga
…
> Fix this by reordering the dereference after the sanity checks.
Can my previous patch review contribution trigger more desirable collateral
evolution
also for this development topic?
https://lore.kernel.org/all/6f01f71b-284b-4841-bda9-a3934cb4e...@web.de/
https://lkml.org/lkml/2025/3/10/696
> When devm_kasprintf() fails, it returns a NULL pointer. However, this return
> value is not properly checked in the function wled_configure.
>
> A NULL check should be added after the devm_kasprintf call to prevent
> potential NULL pointer dereference error.
* Please adhere to word wrapping pr
> devm_kasprintf() return NULL if memory allocation fails. Currently,
…
call? failed?
> Add NULL check after devm_kasprintf() to prevent this issue.
Do you propose to improve this function implementation a bit more?
Regards,
Markus
…
>patch description ;-). I think the original v3 was better worded.
…
Can you find the mentioning of adjustments helpful for better error handling?
Regards,
Markus
> devm_kasprintf() return NULL if memory allocation fails. Currently,
call? failed?
> wled_configure() does not check for this case, leading to a possible NULL
> pointer dereference.
You may omit the word “possible” in such a change description.
(W
…
> can only release 'pfp' field of 'gfx'. The release function of 'me' field
> should be gfx_v12_0_me_fini().
Do you care for an imperative wording in such a change description?
https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rs
…
> Fix this by reordering the dereference after the sanity checks.
Can another wording approach (like the following) be more appropriate?
Thus move the assignment of the variable “dpu_enc” behind …
Would an other summary phrase become nicer?
Regards,
Markus
…
> Fix this by reordering the dereference after the sanity checks.
Another wording suggestion:
Thus move the assignment of the variable “dpu_enc” behind a null pointer check.
Would an other summary phrase be nicer?
Regards,
Markus
…
> drm_edid_alloc() fails, the function will immediately return 0,
…
failed?
…
> the event of failure in these two functions, it is still necessary
> to call the subsequent drm_edid_connector_update() function with
…
You may occasionally put more than 66 characters into text
> No crash or anything can or will happen here.
>
> Markus, maybe you missed the "&" in front of "&fbdev->info" ?
Would you get into the mood to adjust development views
if you would take any feedback and further background information
(by David Svoboda for example) better into account?
https://wik
> Some drivers can use vmap in drm_panic, however, vmap is sleepable and
> takes locks. Since drm_panic will vmap in panic handler, atomic_vmap
> requests pages with GFP_ATOMIC and maps KVA without locks and sleep.
See also:
https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tr
> Anyway, none of that applies here, because this is just pointer math.
Which data processing do you expect to be generally supported at the discussed
source code place (according to the rules of the programming language “C”)?
https://en.cppreference.com/w/c/language/behavior
Regards,
Markus
>> Dereferences of null pointers are treated in special ways.
>
> This not a dereference. It's just pointer math.
Do you prefer the wording “member access through pointer” occasionally?
https://en.cppreference.com/w/c/language/operator_member_access
How do you tend to describe (and detect) null p
>>> The address of a data structure member was determined before
>>> a corresponding null pointer check in the implementation of
>>> the functions “dpu_hw_pp_enable_te” and “dpu_hw_pp_get_vsync_info”.
>>>
>>> Thus avoid the risk for undefined behaviour by removing extra
>>> initialisations for the
From: Markus Elfring
Date: Wed, 5 Apr 2023 18:38:54 +0200
The label “out_prevalid” was used to jump to another pointer check
despite of the detail in the implementation of the function
“nouveau_gem_ioctl_pushbuf” that it was determined already in one case
that the corresponding variable
From: Markus Elfring
Date: Fri, 28 Feb 2025 17:32:45 +0100
Replace nested max() calls by single max3() call in this
function implementation.
This issue was transformed by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/radeon/radeon_uvd.c | 2 +-
1 file
From: Markus Elfring
Date: Mon, 17 Apr 2023 11:26:34 +0200
The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the macro “DECLARE_NEXT_HOLE_ADDR”.
Thus avoid the risk for undefined behaviour by moving the assignment
for the
> struct fb_info *info = &fbdev->info;
>
> if (!fbdev)
> return -EINVAL;
Is such a null pointer check still relevant for the discussed function
implementation?
Regards,
Markus
>>> The address of a data structure member was determined before
>>> a corresponding null pointer check in the implementation of
>>> the functions “dpu_hw_pp_enable_te” and “dpu_hw_pp_get_vsync_info”.
>>>
>>> Thus avoid the risk for undefined behaviour by removing extra
>>> initialisations for the
From: Markus Elfring
Date: Fri, 28 Feb 2025 16:37:00 +0100
Replace nested max() calls by single max3() call in this
function implementation.
This issue was transformed by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/amd/amdgpu/si.c | 2 +-
1 file changed
From: Markus Elfring
Date: Tue, 11 Apr 2023 18:24:24 +0200
The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the functions “dpu_hw_pp_enable_te” and “dpu_hw_pp_get_vsync_info”.
Thus avoid the risk for undefined behaviour by
From: Markus Elfring
Date: Thu, 13 Apr 2023 21:35:36 +0200
The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the function “au1100fb_setmode”.
Thus avoid the risk for undefined behaviour by moving the assignment
for the
From: Markus Elfring
Date: Fri, 28 Feb 2025 18:25:31 +0100
Reduce nested max() calls by a single max3() call in this
function implementation.
This issue was transformed by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 2 +-
1 file
From: Markus Elfring
Date: Fri, 28 Feb 2025 17:18:07 +0100
Replace nested max() calls by single max3() call in this
function implementation.
This issue was transformed by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/drm_ioctl.c | 2 +-
1 file changed, 1
…
> This is converted using scripts/coccinelle/misc/secs_to_jiffies.cocci with
> the following Coccinelle rules:
Is only a single SmPL script rule relevant here?
> @depends on patch@
> expression E;
> @@
>
> -msecs_to_jiffies
> +secs_to_jiffies
> (E
> - * \( 1000 \| MSEC_PER_SEC \)
> )
I would
…
> fence callback add fails.
failed?
> To fix this, we should decrement the reference count of prev when
…
See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.14-rc4#n94
> v2:
Patch version
> Problem: If prev(last_scheduled) was already signaled I encountred a
signalled? encountered?
> memory leak in drm_sched_entity_fini. This is because the
> prev(last_scheduled) fence is not free properly.
freed?
…
> ---
> drivers/gpu/drm/radeon/r420.c | 15 +++
…
How do you think about to improve your version management?
https://lore.kernel.org/all/?q=%22This+looks+like+a+new+version+of+a+previously+submitted+patch%22
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu
> … This patch changes its return type …
See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.14-rc2#n94
> Signed-off-by: Wentao Liang
How good does such an email address fit to the Developer's Certificate of
Origi
…
> ---
> drivers/gpu/drm/xlnx/zynqmp_dp_audio.c | 4
…
See also:
https://lore.kernel.org/all/?q=%22This+looks+like+a+new+version+of+a+previously+submitted+patch%22
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.14-rc2
…
> Add NULL check in zynqmp_audio_init(), to handle kernel NULL
> pointer dereference error.
* Can it be more desirable to prevent such issues?
* Would the message subject have been nicer with a key word like “PATCH”
(besides other refinements)?
…
> ---
> drivers/gpu/drm/xlnx/zynqmp_dp_audi
> devm_kasprintf() can return a NULL pointer on failure,but this
> returned value in zynqmp_audio_init() is not checked.
Another wording suggestion:
devm_kasprintf() calls can return null pointers on failure.
But some return values were not checked in zynqmp_audio_init().
> Add NULL check in zyn
> As it stands, I'll fix up the current rules in v2 following your
> suggestion to keep the multiplication in each line to allow Coccinelle
> to use the commutativity properties and find more instances.
Corresponding software development challenges can eventually be clarified
further.
> I'll re
> Teach the script to suggest conversions for timeout patterns where the
> arguments to msecs_to_jiffies() are expressions as well.
Does anything hinder to benefit any more from a source code analysis approach
(like the following by the extended means of the semantic patch language)?
// SPDX-Lic
>> …
>>> +++ b/scripts/coccinelle/misc/secs_to_jiffies.cocci
>>> @@ -11,12 +11,22 @@
>>>
>>> virtual patch
>> …
>>> -@depends on patch@ constant C; @@
>>> +@depends on patch@
>>> +expression E;
>>> +@@
>>>
>>> -- msecs_to_jiffies(C * MSEC_PER_SEC)
>>> -+ secs_to_jiffies(C)
>>> +-msecs_to_jiffies
>
> Teach the script to suggest conversions for timeout patterns where the
> arguments to msecs_to_jiffies() are expressions as well.
I propose to take another look at implementation details for such a script
variant
according to the semantic patch language.
…
> +++ b/scripts/coccinelle/misc/secs
>> …
>>> ---
>>> Coverity Message:
>>> CID 1601557: (#1 of 1): Uninitialized pointer read (UNINIT)
>>> 3. uninit_use: Using uninitialized value next.
>>
>> May such information become a part for the final change description?
>>
> Ofcourse, it shouldn't be the part of the change description.
I sugg
> Initialize the pointer with NULL as mtk_drm_of_get_ddp_ep_cid
> function might return before assigning value to next pointer
> but yet further dereference to next can lead to some undefined
> behavior as it may point to some invalid location.
* You may occasionally put more than 62 characters in
…
> +++ b/drivers/gpu/drm/clients/drm_log.c
> @@ -0,0 +1,370 @@
…
> +static int drm_log_count_modeset(struct drm_client_dev *client)
> +{
> + struct drm_mode_set *mode_set;
> + int count = 0;
> +
> + mutex_lock(&client->modeset_mutex);
> + drm_client_for_each_modeset(mode_set, clien
> Because the of_dma_configure() will returns '-EPROBE_DEFER' if the probe
return?
…
> Stop pretending that it will always suceess, quit if it fail.
succeed?failed?
How do you think about to add any tags (
Why is a change description missing here?
…
> +++ b/scripts/coccinelle/misc/secs_to_jiffies.cocci
> @@ -0,0 +1,21 @@
…
> +/// Find usages of:
> +/// - msecs_to_jiffies(value*1000)
> +/// - msecs_to_jiffies(value*MSEC_PER_SEC)
I suggest to take another look at corresponding development documentat
…
> +++ b/drivers/media/platform/mediatek/isp/isp_7x/camsys/mtk_cam.c
> @@ -0,0 +1,4168 @@
…
> +void mtk_cam_dev_req_try_queue(struct mtk_cam_device *cam)
> +{
…
> + spin_lock(&cam->running_job_lock);
> + job_count = cam->running_job_count;
> + spin_unlock(&cam->running_job_lock);
…
Un
…
> Although this case may be unrealistic for the current code, it is
> still better to protect against possible bugs.
>
> Bail out also when status is AE_NOT_FOUND.
…
How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?
https://git.kernel.org/pub/scm/linux/kernel/git/torval
…
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -1757,11 +1757,13 @@ int amdgpu_discovery_get_nps_info(struct
> amdgpu_device *adev,
>
> switch (le16_to_cpu(nps_info->v1.header.version_major)) {
> case 1:
> + mem_ranges = kvcalloc(nps_info->v1.count,
> +
>> …
>>> pre-multiplied is supported in the current platform.
>> …
>>
>> format would be?
…
> blend_modes is the driver data that describes the supported blend mode
> in current platform no matter format would be any one.
>
> This sentence is describing mtk_ovl_fmt_convert() will c
> This patch series add support for the monochrome Sharp Memory LCD panels.
…
> ---
> Changes in v10:
…
Is the support for the application of scope-based resource management
still ignored here?
Regards,
Markus
…
> pre-multiplied is supported in the current platform.
…
format would be?
Regards,
Markus
…
> Note that ignore pixel alpha bit is not supported if the SoC is not
> supported the blend_modes.
supporting the blending modes?
> So it will break the original setting of XRGB foramt for the
format?
> Signed-off-by: Jason-JH.Lin
…
> Fix the SoC degradation problem by this sreies.
series?
Regards,
Markus
…
> premultiplied supported in current platform.
…
format would be?
Regards,
Markus
…
> Make the constatnt alpha only enable when having a vliad blend_mode or
…
Another wording suggestion:
Enable the constant alpha channel only when having a valid blending mode?
…
> Signed-off-by: Jason-JH.Lin
Does a dot really belong to this personal name?
https://git.kernel.org/pub/scm/linu
> Although the alpha channel in XRGB formats can be ignored, ALPHA_CON
> must be configured accordingly when using XRGB formats or it will still
> affects CRC generation.
affect?
Can such a change description become a bit nicer with an additional
imperative wording?
https://git.kernel.org/pub/
> This patch series add support for the monochrome Sharp Memory LCD panels.
…
> ---
> Changes in v9:
…
Would you like to benefit from the application of scope-based resource
management
(also for this software component)?
Regards,
Markus
> There is no need to call the dev_err() function directly to print a
> custom message when handling an error from platform_get_irq() function
> as it is going to display an appropriate error message in case of a
> failure.
Can such a change description become a bit nicer with an additional
impera
From: Markus Elfring
Date: Thu, 3 Oct 2024 19:56:29 +0200
An of_node_put(cmm) call was immediately used after a null pointer check
for an of_find_device_by_node() call in this function implementation.
Thus call such a function instead directly before the check.
This issue was transformed by
From: Markus Elfring
Date: Thu, 3 Oct 2024 13:20:05 +0200
Add a label so that a bit of exception handling can be better reused
in a subsequent if branch of this function implementation.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/gpu
…
> +++ b/drivers/gpu/drm/tiny/sharp-memory.c
> @@ -0,0 +1,681 @@
…
> +static int sharp_memory_maintain_display(struct sharp_memory_device *smd)
> +{
…
> + u8 *tx_buffer = smd->tx_buffer;
> +
> + mutex_lock(&smd->tx_mutex);
…
> + mutex_unlock(&smd->tx_mutex);
> +
> + return ret;
> +
From: Markus Elfring
Date: Wed, 25 Sep 2024 21:12:36 +0200
An of_node_put(ep) call was immediately used after a pointer check
for a of_graph_get_remote_port() call in this function implementation.
Thus call such a function only once instead directly before the check.
This issue was transformed
>> Assign return values from copy_to_user() calls to additional local variables
>> so that four kfree() calls and return statements can be omitted accordingly.
…
>> +++ b/drivers/gpu/drm/xe/xe_query.c
>> @@ -220,13 +220,11 @@ static int query_engines(struct xe_device *xe,
>>
>> engines->num_en
From: Markus Elfring
Date: Wed, 18 Sep 2024 09:43:07 +0200
Assign return values from copy_to_user() calls to additional local variables
so that four kfree() calls and return statements can be omitted accordingly.
This issue was transformed by using the Coccinelle software.
Signed-off-by
From: Markus Elfring
Date: Tue, 17 Sep 2024 19:06:23 +0200
Add a jump target so that a bit of exception handling can be better reused
at the end of this function implementation.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/mediatek/mtk_gem.c | 10 ++
1 file changed, 6 insertions
From: Markus Elfring
Date: Tue, 17 Sep 2024 18:50:37 +0200
A sg_free_table(sgt) call was immediately used after a null pointer check
for the data structure member “kvaddr” in this function implementation.
Thus use such a function call only once instead directly before the check.
This issue was
From: Markus Elfring
Date: Tue, 17 Sep 2024 19:16:45 +0200
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (2):
Avoid duplicate sg_free_table(sgt) call
Use common error handling code
drivers/gpu/drm/mediatek/mtk_gem.c | 14
From: Markus Elfring
Date: Tue, 17 Sep 2024 16:40:18 +0200
A of_node_put(remote) call was immediately used after a null pointer check
for the data structure member “next_bridge” in this
function implementation.
Thus use such a function call only once instead directly before the check.
This
…
> +++ b/drivers/accel/rocket/rocket_job.h
> @@ -0,0 +1,49 @@
…
> +#ifndef __ROCKET_JOB_H__
> +#define __ROCKET_JOB_H__
…
I suggest to omit leading underscores from such identifiers.
https://wiki.sei.cmu.edu/confluence/display/c/DCL37-C.+Do+not+declare+or+define+a+reserved+identifier
Regards,
Ma
…
> +++ b/drivers/accel/rocket/rocket_job.c
> @@ -0,0 +1,708 @@
…
> +static int rocket_job_push(struct rocket_job *job)
> +{
…
> + mutex_lock(&rdev->sched_lock);
> + drm_sched_job_arm(&job->base);
…
> + drm_sched_entity_push_job(&job->base);
> +
> + mutex_unlock(&rdev->sched_lock);
…
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp_audio.c
> @@ -0,0 +1,461 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ZynqMP DisplayPort Subsystem Driver - Audio support
> + *
> + * Copyright (C) 2015 - 2023 Xilinx, Inc.
> + *
> + * Authors:
…
Would such information need another adjustment (acco
…
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp_audio.c
> @@ -0,0 +1,461 @@
…
> +static int dp_dai_hw_free(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *socdai)
> +{
…
> + struct zynqmp_dpsub_audio *audio = dpsub->audio;
> +
> + mutex_lock(&audio->enable_lock);
assignment
> for the variable “data” behind the null pointer check.
>
> This issue was detected by using the Coccinelle software.
>
> Fixes: c030f2e4166c3f5597c7e7a70bcd9ab383695de4 ("drm/amdgpu: add
> amdgpu_ras.c to support ras (v2)")
> Signed-off-by: Markus Elfri
> Under some circumstances it might be required to return NULL from the
> create_address_space callback, meaning that the driver should use global
> address space. …
Please avoid a typo in the summary phrase.
Regards,
Markus
…
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> @@ -0,0 +1,708 @@
…
> +static int dw_hdmi_qp_i2c_xfer(struct i2c_adapter *adap,
> +struct i2c_msg *msgs, int num)
> +{
…
> + mutex_lock(&i2c->lock);
…
> + dw_hdmi_qp_mod(hdmi, 0, I2CM_OP_DONE_MASK_N | I2CM_
…
> should be instead of 'return'.
“return” instead?
Can a corresponding imperative wording be preferred for a better change
description?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.11-rc6#n94
Regards,
Ma
Switching to memdup_user(), which combines kmalloc() and copy_from_user(),
and it can simplfy code.
By the way:
Would it have been nicer to avoid a typo anyhow in such a change description?
>>> Applied, thanks!
>>>
>>> [1/1] drm/imagination: Use memdup_user() helper
>>> commit: 2
> > Switching to memdup_user(), which combines kmalloc() and copy_from_user(),
> > and it can simplfy code.
>
> Applied, thanks!
>
> [1/1] drm/imagination: Use memdup_user() helper
> commit: 2872a57c7ad427d428c6d12e95e55b32bdc8e3b8
Do you find any previous contributions still similarly inter
> Nuvoton MA35 SoCs NAND Flash Interface Controller
> supports 2KB, 4KB and 8KB page size, and up to 8-bit,
> 12-bit, and 24-bit hardware ECC calculation circuit
> to protect data communication.
You may occasionally put more than 53 characters into text lines
of such a change description.
https://
If you temporarily find the circumstances too challenging for applications
of scope-based resource management, I suggest to use the following statements
instead
(so that a bit of redundant code can be avoided).
…
> +++ b/drivers/accel/amdxdna/aie2_pci.c
> @@ -0,0 +1,182 @@
…
> +static int aie2_
>> https://lkml.org/lkml/2024/7/19/803
>> https://lore.kernel.org/linux-kernel/010a46ba-9dc4-e3e3-7894-b28b312c6...@amd.com/
>> [01/10] accel/amdxdna: Add a new driver for AMD AI Engine
>> “guard looks cleaner. We will use it.”
> We reconsidered this request
Interesting …
> and searched accel an
…
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
…
> @@ -1418,21 +1416,17 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct
> drm_device *dev, size_t size,
> pages = kcalloc(npages, sizeof(*pages), GFP_KERNEL);
> if (!pages) {
> omap_gem_free_object
…
> Changes since v1:
> - Remove some inline defines
> - Minor changes based code review comments
…
How “good” does such a version description fit to previous
patch review feedback (like the following)?
https://lkml.org/lkml/2024/7/19/803
https://lore.kernel.org/linux-kernel/010a46ba-9dc4-e3e3-78
>> …
>>> +++ b/include/net/devmem.h
>>> @@ -0,0 +1,115 @@
>> …
>>> +#ifndef _NET_DEVMEM_H
>>> +#define _NET_DEVMEM_H
>> …
>>
>> I suggest to omit leading underscores from such identifiers.
>> https://wiki.sei.cmu.edu/confluence/display/c/DCL37-C.+Do+not+declare+or+define+a+reserved+identifier
>>
>
>> …
>>> +++ b/drivers/gpu/drm/loongson/loongson_drv.h
>>> @@ -0,0 +1,108 @@
>> …
>>> +#ifndef __LOONGSON_DRV_H__
>>> +#define __LOONGSON_DRV_H__
>> …
>>
>> I suggest to omit leading underscores from such identifiers.
…
> I suggest add this rules to the checkpatch.pl script,
…
I hope that you woul
…
> +++ b/include/net/devmem.h
> @@ -0,0 +1,115 @@
…
> +#ifndef _NET_DEVMEM_H
> +#define _NET_DEVMEM_H
…
I suggest to omit leading underscores from such identifiers.
https://wiki.sei.cmu.edu/confluence/display/c/DCL37-C.+Do+not+declare+or+define+a+reserved+identifier
Regards,
Markus
…
> +++ b/drivers/gpu/drm/tiny/sharp-memory.c
> @@ -0,0 +1,684 @@
…
> static int sharp_memory_update_display(struct sharp_memory_device *smd,
> +struct drm_framebuffer *fb,
> +struct drm_rect clip,
> +
…
> +++ b/drivers/gpu/drm/loongson/Makefile
> @@ -17,6 +17,9 @@ loongson-y := \
> lsdc_probe.o \
> lsdc_ttm.o
>
> +loongson-y += \
> + loonggpu_pci_drv.o
> +
> loongson-y += loongson_device.o \
…
Why do you propose to adjust the macro contents multiple times here?
(Can it be suffi
…
> +++ b/drivers/gpu/drm/loongson/loongson_drv.c
> @@ -0,0 +1,298 @@
…
> +static int loongson_drm_driver_probe(struct platform_device *pdev)
> +{
…
> + dev_info(&pdev->dev, "probed\n");
…
> +}
…
Do you find such information really relevant?
Regards,
Markus
…
> the driver is loaded, drm/loongson driver still need to wait all of
needs to wait on …?
…
> design. Therefore, add a dummy driver for the GPU, …
Is there a need to reconsider the categorisation and usability descriptions
another bit for such
…
> +++ b/drivers/gpu/drm/loongson/loongson_drv.h
> @@ -0,0 +1,108 @@
…
> +#ifndef __LOONGSON_DRV_H__
> +#define __LOONGSON_DRV_H__
…
I suggest to omit leading underscores from such identifiers.
https://wiki.sei.cmu.edu/confluence/display/c/DCL37-C.+Do+not+declare+or+define+a+reserved+identifier
…
> +++ b/drivers/gpu/drm/loongson/loongson_drv.c
> @@ -0,0 +1,298 @@
…
> +static int loongson_drm_freeze(struct drm_device *ddev)
> +{
…
> + /* unpin all of buffers in the VRAM */
> + mutex_lock(&ldrm->gem.mutex);
…
> + mutex_unlock(&ldrm->gem.mutex);
> +
> + lsdc_bo_evict_vram(dde
>> Is there some reason this whole thing isn't just
>>
>> seq_write(m, drm->vbios.data, drm->vbios.length)
…
> I don't know if my answer is relevant or not here but:
> for () seq_putc(); ==> will fill 'm' with everything that fits in
I find such a discussion approach strange.
> and
>
> In some display subsystems, the functionality of a PCIe device may too
might be?
…
> of the dirver is loaded, …
driver?
…
> its dependencies ready before it can register the service to userspace.
…
> v3 -> v4:
> * Tiny refinement and clean up.
I suggest to reconsider the need for a cover letter according to a single patch.
Regards,
Markus
From: Markus Elfring
Date: Tue, 23 Jul 2024 18:08:15 +0200
Some characters should be put into a sequence.
* Thus print all data by the corresponding function “seq_write” at once.
* Return also the value from this function call.
* Omit a local variable which became redundant with this
>> …
>>> +++ b/drivers/dma-buf/dma-heap.c
>> …
>>> +static void dma_heap_release(struct kref *ref)
>>> +{
>> …
>>> + mutex_lock(&heap_list_lock);
>>> + list_del(&heap->list);
>>> + mutex_unlock(&heap_list_lock);
>> …
>>
>> Under which circumstances would you become interested to apply a
…
> +++ b/drivers/dma-buf/dma-heap.c
…
> +static void dma_heap_release(struct kref *ref)
> +{
…
> + mutex_lock(&heap_list_lock);
> + list_del(&heap->list);
> + mutex_unlock(&heap_list_lock);
…
Under which circumstances would you become interested to apply a statement
like “guard(mutex)
> … The tile columns belong to
…
which …?
…
> +++ b/drivers/accel/amdxdna/aie2_ctx.c
> @@ -0,0 +1,181 @@
…
> +void aie2_hwctx_fini(struct amdxdna_hwctx *hwctx)
> +{
> + struct amdxdna_dev *xdn
> The AI Engine consists of 2D array of tiles arranged as columns. The
> resource solver provides the interfaces to manage allocation of the tile
> columns for a hardware context. The basic column allocation and release
> functions are provided.
Can such a change description be improved with imper
…
> +++ b/drivers/accel/amdxdna/amdxdna_mailbox.c
> @@ -0,0 +1,582 @@
…
> +int xdna_mailbox_send_msg(struct mailbox_channel *mb_chann,
> + const struct xdna_mailbox_msg *msg, u64 tx_timeout)
> +{
> + struct xdna_msg_header *header;
> + struct mailbox_msg *mb_msg;
I pr
…
> +++ b/drivers/accel/amdxdna/aie2_pci.h
> @@ -0,0 +1,144 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2023-2024, Advanced Micro Devices, Inc.
> + */
> +
> +#ifndef _AIE2_PCI_H_
> +#define _AIE2_PCI_H_
…
Please omit leading underscores from such identifiers.
https://wik
…
> +++ b/drivers/accel/amdxdna/aie2_pci.c
> @@ -0,0 +1,182 @@
…
> +static int aie2_init(struct amdxdna_dev *xdna)
> +{
…
> + const struct firmware *fw;
I suggest to take another software design option better into account
also according to the application of scope-based resource management.
*
1 - 100 of 732 matches
Mail list logo
