On 2/16/21 9:48 AM, Kumar Kartikeya Dwivedi wrote:
On Tue, Feb 16, 2021 at 08:24:59PM IST, Alex Elder wrote:
This is a good change. But while you're at it, I would
appreciate if you would convert a few spots to use
sizeof(dest) rather than a fixed constant. I will
point them out below
On 2/12/21 4:50 PM, Manikantan Ravichandran wrote:
checkpatch warning fix for string split across lines
Signed-off-by: Manikantan Ravichandran
I think what you're doing here *looks* reasonable. But
the GB_AUDIO_MANAGER_MODULE_NAME_LEN_SSCANF symbol is
a (string) numeric value that is associa
On 1/31/21 11:28 AM, Kumar Kartikeya Dwivedi wrote:
strlcpy is marked as deprecated in Documentation/process/deprecated.rst,
and there is no functional difference when the caller expects truncation
(when not checking the return value). strscpy is relatively better as it
also avoids scanning the w
On 1/3/21 4:35 PM, Arnd Bergmann wrote:
From: Arnd Bergmann
clang warns about excessive stack usage in this driver when
UBSAN is enabled:
drivers/staging/greybus/audio_topology.c:977:12: error: stack frame size of
1836 bytes in function 'gbaudio_tplg_create_widget'
[-Werror,-Wframe-larger-th
On 12/29/20 7:37 PM, Zheng Yongjun wrote:
Use kzalloc rather than kcalloc(1,...)
The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)
//
@@
@@
- kcalloc(1,
+ kzalloc(
...)
//
Signed-off-by: Zheng Yongjun
Looks good.
Reviewed-by: Alex Elder
On 12/29/20 7:50 AM, Zheng Yongjun wrote:
Use kzalloc rather than kcalloc(1,...)
The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)
//
@@
@@
- kcalloc(1,
+ kzalloc(
...)
//
Signed-off-by: Zheng Yongjun
---
drivers/staging/greybus/light.c | 2
On 9/25/20 9:11 AM, Coiby Xu wrote:
On Thu, Sep 24, 2020 at 10:54:50AM +, David Laight wrote:
From: Coiby Xu
Sent: 24 September 2020 11:21
Use __8 to replace int and remove the unnecessary __bitwise type
attribute.
Found by sparse,
...
diff --git a/include/uapi/sound/asound.h b/include
On 9/25/20 9:07 AM, Coiby Xu wrote:
On Thu, Sep 24, 2020 at 01:00:57PM +0200, Greg Kroah-Hartman wrote:
On Thu, Sep 24, 2020 at 06:20:39PM +0800, Coiby Xu wrote:
Use __8 to replace int and remove the unnecessary __bitwise type
attribute.
Found by sparse,
. . .
diff --git a/include/uapi/so
On 9/24/20 5:20 AM, Coiby Xu wrote:
> Use __8 to replace int and remove the unnecessary __bitwise type attribute.
>
> Found by sparse,
Greg's right, don't change the public header.
You could try this in the Greybus code to eliminate the warning,
but I haven't tried it (and for all I know it's no
or format bitmask").
>
> Found by sparse,
Looks good.
Reviewed-by: Alex Elder
>
> $ make C=2 drivers/staging/greybus/
> drivers/staging/greybus/audio_codec.c:691:36: warning: incorrect type in
> initializer (different base types)
> drivers/staging/greybus/audio_cod
. Either way, you can add this:
Reviewed-by: Alex Elder
> $ make C=2 drivers/staging/greybus/
> drivers/staging/greybus/audio_module.c:222:25: warning: incorrect type in
> assignment (different base types)
> drivers/staging/greybus/audio_module.c:222:25:expected restricted __le16
On 7/30/20 11:02 AM, Colin Ian King wrote:
> Hi,
>
> Static analysis with Coverity has detected an uninitialized value being
> used in a comparison. The error was detected on a recent change to
> drivers/staging/greybus/audio_topology.c however the issue actually
> dates back to the original comm
On 7/9/20 5:27 AM, Vaibhav Agarwal wrote:
> snd_soc_jack APIs are modified in recent kernel versions. This patch
> updates the codec driver to resolve the compilation errors related to
> jack framework.
Greg has already accepted this series so I won't review this now. But
I still wanted to provid
udio modules")
> Signed-off-by: Dan Carpenter
This is a good fix, thanks.
Reviewed-by: Alex Elder
> ---
> drivers/staging/greybus/audio_helper.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/greybus/audio_helper.c
>
On 5/25/20 1:10 AM, Till Varoquaux wrote:
Successed -> succeeded.
Signed-off-by: Till Varoquaux
Looks good.
Reviewed-by: Alex Elder
---
drivers/staging/greybus/loopback.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/greybus/loopback.c
b/driv
On 4/29/20 2:00 PM, Arnd Bergmann wrote:
> gcc-10 points out an uninitialized variable use:
Wow, nice, checking individual uninitialized fields within
the structure.
The structure should really be zero-initialized anyway; it's
passed as a structure in a message elsewhere. With your
change, all f
or the patch.
Reviewed-by: Alex Elder
> ---
> drivers/staging/greybus/tools/loopback_test.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/greybus/tools/loopback_test.c
> b/drivers/staging/greybus/tools/loopback_test.c
> index
On 2/11/20 4:47 PM, Gustavo A. R. Silva wrote:
>
>
> On 2/11/20 16:15, Alex Elder wrote:
>> On 2/11/20 3:12 PM, Gustavo A. R. Silva wrote:
>>> The current codebase makes use of the zero-length array language
>>> extension to the C90 standard, but the preferred
On 2/11/20 3:12 PM, Gustavo A. R. Silva wrote:
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
>
> struct foo
On 1/25/20 6:14 AM, SAURAV GIREPUNJE wrote:
> On 25/01/20 11:00 +0100, Johan Hovold wrote:
>> On Sat, Jan 25, 2020 at 02:14:03PM +0530, Saurav Girepunje wrote:
>>> fix uninitialized variables issue found using static code analysis tool
>>
>> Which tool is that?
>>
>>> (error) Uninitialized variable
at I've done here serves as a real review, so:
Acked-by: Alex Elder
-Alex
>
> ./include/linux/greybus/hd.h:23:50: error: unknown type name 'u16'
> int (*cport_disable)(struct gb_host_device *hd, u16 cport_id); ^~~
> ./includ
is is the movement of the Documentation entries and a
> number of the module drivers that are stable.
They all look good to me. (I don't always agree with checkpatch,
but standardization is good.) Thanks Greg.
Acked-by: Alex Elder
> Greg Kroah-Hartman (9):
> staging: greybus: fi
On 4/16/19 5:13 PM, Madhumitha Prabakaran wrote:
> Fix a blank line after structure declarations. Also, convert
> macros into inline functions in order to maintain Linux kernel
> coding style based on which the inline function is
> preferable over the macro.
Madhumitha, here is my explanation for
On 4/17/19 1:25 AM, Greg KH wrote:
> On Tue, Apr 16, 2019 at 05:13:18PM -0500, Madhumitha Prabakaran wrote:
>> Fix a blank line after structure declarations. Also, convert
>> macros into inline functions in order to maintain Linux kernel
>> coding style based on which the inline function is
>> pref
On 4/5/19 3:53 PM, Dan Carpenter wrote:
> On Fri, Apr 05, 2019 at 03:00:46PM -0500, Madhumitha Prabakaran
> wrote:
>> Fix spinlock_t definition without comment.
>>
>> Signed-off-by: Madhumitha Prabakaran
Madhumitha, the reason one would want a comment associated with
a lock field in a structure
On 4/1/19 9:22 AM, Will Cunningham wrote:
> Line was >80 characters.
This looks fine, but "tmp" is not a meaningful name.
That argument to gb_connection_create() is a cport id,
so "cport_id" would be a much better name for the variable.
It seems picky, but details like this make the code much
mor
On 3/31/19 1:40 AM, Joe Perches wrote:
> On Sun, 2019-03-31 at 01:20 -0500, Alex Elder wrote:
>> On 3/31/19 1:04 AM, Joe Perches wrote:
>>> Blind adherence to 80 column limits leads to poor looking
>>> code. Especially with longish identifier lengths.
>> I agree
On 3/31/19 1:04 AM, Joe Perches wrote:
> On Sun, 2019-03-31 at 01:30 -0400, Will Cunningham wrote:
>> Line was >80 characters.
> []
>> diff --git a/drivers/staging/greybus/usb.c b/drivers/staging/greybus/usb.c
> []
>> @@ -169,8 +169,8 @@ static int gb_usb_probe(struct gbphy_device *gbphy_dev,
>>
I suspect you found this in the
program output though, and also noticed it in the README file.
So... Looks good to me.
Reviewed-by: Alex Elder
>
> Signed-off-by: Colin Ian King
> ---
> drivers/staging/greybus/tools/README.loopback | 2 +-
> drivers/staging/greybus/tools/loop
On 06/05/2018 04:00 AM, Dan Carpenter wrote:
> On Tue, Jun 05, 2018 at 11:02:36AM +0530, Viresh Kumar wrote:
>> On 03-06-18, 08:52, Janani Sankara Babu wrote:
>>> This patch replaces comparison of var to NULL with !var
>>>
>>> Signed-off-by: Janani Sankara Babu
Wow, such deep discussion for such
On 04/27/2018 11:35 PM, Arvind Yadav wrote:
> Replace the manual validity checks for the GPIO with the
> gpio_is_valid().
>
> Signed-off-by: Arvind Yadav
Looks good.
Reviewed-by: Alex Elder
> ---
> chnage in v2 :
> Returning invalid gpio as err
On 04/27/2018 07:50 AM, Arvind Yadav wrote:
>
>
> On Friday 27 April 2018 05:47 PM, Alex Elder wrote:
>> On 04/27/2018 05:52 AM, Arvind Yadav wrote:
>>> Replace the manual validity checks for the GPIO with the
>>> gpio_is_valid().
>> I haven't looke
On 04/27/2018 05:52 AM, Arvind Yadav wrote:
> Replace the manual validity checks for the GPIO with the
> gpio_is_valid().
I haven't looked through the code paths very closely, but I
think that get_named_gpio() might return -EPROBE_DEFER, which
would be something we want to pass to the caller.
So
t that's probably not a big deal.
Reviewed-by: Alex Elder
> ---
> v2:
> Updated the patch title and description.
>
> drivers/staging/greybus/camera.c | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/greybus/camera.c
On 11/07/2017 08:47 AM, Greg Kroah-Hartman wrote:
> On Tue, Nov 07, 2017 at 08:42:07AM -0600, Alex Elder wrote:
>> On 11/07/2017 07:58 AM, Greg Kroah-Hartman wrote:
>>> It's good to have SPDX identifiers in all files to make it easier to
>>> audit the
t to remove the 700+ different ways that files in
> the kernel describe the GPL license text. And there's unneeded stuff
> like the address (sometimes incorrect) for the FSF which is never
> needed.
>
> No copyright headers or other non-license-description text was removed.
C++ style comment? (Use /* */ not //?)
Otherwise:
Reviewed-by: Alex Elder
>
> Cc: Johan Hovold
> Cc: Alex Elder
> Cc: Greg Kroah-Hartman
> Cc: Vaibhav Hiremath
> Cc: Vaibhav Agarwal
> Cc: Mark Greer
> Cc: Viresh Kumar
> Cc: Rui Miguel Silva
> Cc: David
On 04/12/2017 08:36 PM, Darryl T. Agostinelli wrote:
> $ make includecheck | grep staging
> ./drivers/staging/greybus/uart.c: linux/serial.h is included more than once.
>
> Signed-off-by: Darryl T. Agostinelli
Looks good.
Reviewed-by: Alex Elder
> ---
> drivers/staging/
On 04/12/2017 08:36 PM, Darryl T. Agostinelli wrote:
> Fixes:
> $ make versioncheck | grep staging
> ./drivers/staging/greybus/light.c: 15 linux/version.h not needed.
>
> Signed-off-by: Darryl T. Agostinelli
Looks good.
Reviewed-by: Alex Elder
> ---
> drivers/stagin
On 02/21/2017 07:44 AM, Johan Hovold wrote:
> On Tue, Feb 21, 2017 at 02:20:50PM +0100, Julia Lawall wrote:
>>
>>
>> On Tue, 21 Feb 2017, simran singhal wrote:
>>
>>> This patch removes braces for single statement blocks. The warning
>>> was detected using checkpatch.pl.
>>> Coccinelle was used to
Signed-off-by: Greg Kroah-Hartman
This consists entirely of deletions. I haven't applied the
patch to test it, so I trust it compiles as it should. And if
that is true, I'm sure it causes no harm, since as you said,
nobody uses the code right now.
Reviewed-by: Alex Elder
> ---
&g
On 01/05/2017 12:25 PM, Bryan O'Donoghue wrote:
> On 05/01/17 17:39, Greg Kroah-Hartman wrote:
>> From: Greg Kroah-Hartman
>>
>> While the timesync protocol was a great idea, it never ended up getting
>> implemented by any known hardware devices. It's also a bit
>> "interesting" in how it ties in
On 11/30/2016 12:32 PM, Dan Carpenter wrote:
> On Wed, Nov 30, 2016 at 05:21:40PM +0100, Andrea Ghittino wrote:
>> Honestly, for a kernel newbie it is difficult to manage. I received 2
>> email that suggest
>> to use S_IRUGO. My original patch was with octal, based on checkpatch.pl
>> advice.
Go
On 11/26/2016 03:50 PM, Andrea Ghittino wrote:
> Fixes greybus user/groups permission style warnings
> found by checkpatch.pl tool
>
> Signed-off-by: Andrea Ghittino
I don't understand why using 0444 would be preferred
over S_IRUGO (for example). Do you know? Maybe the
checkpatch.pl output sa
On 11/29/2016 03:11 PM, Andrea Ghittino wrote:
> Fixes greybus "line over 80 characters" style warnings
> found by checkpatch.pl tool
I have a few comments. Please update your patch and send
a new version.
> Signed-off-by: Andrea Ghittino
You state that this is v2; how does this version diffe
On 10/31/2016 08:50 AM, Alex Elder wrote:
> The Greybus kernel code, developed as part of Google's Project Ara,
> is in the upstream Linux kernel tree (under drivers/staging). The
> cancellation of that project makes the future for Greybus a bit less
> certain. There is intere
On 11/02/2016 11:29 AM, Alexandre Bailon wrote:
> On 10/31/2016 02:50 PM, Alex Elder wrote:
>> Git repositories. Public git repositories related to Project Ara
>> are all hosted here:
>> https://github.com/projectara
>> At this time I see no reason to move away fro
The Greybus kernel code, developed as part of Google's Project Ara,
is in the upstream Linux kernel tree (under drivers/staging). The
cancellation of that project makes the future for Greybus a bit less
certain. There is interest among the core developers of Greybus
(and others) to do what we can
On 09/21/2016 12:05 PM, Richard Groux wrote:
> Minor error spotted by checkpatch.pl in greybus
> code indent should use tabs where possible
>
> Signed-off-by: Richard Groux
Looks good.
Reviewed-by: Alex Elder
> ---
> drivers/staging/greybus/audio_codec.c | 2 +-
&g
On 09/21/2016 12:05 PM, Richard Groux wrote:
> Minor error spotted by checkpatch.pl in greybus
> space required before the open brace '{'
>
> Signed-off-by: Richard Groux
Sure, looks good. Greg will have to apply these patches.
Reviewed-by: Alex Elder
> ---
&
50 matches
Mail list logo
