Re: [PATCH v3 3/8] staging: rtl8192u: User memset to initialize memory, instead of loop.
> On 25 June 2018 at 13:36 John Whitmore wrote: > > > On Mon, Jun 25, 2018 at 12:06:30PM +0300, Andy Shevchenko wrote: > > On Sun, Jun 24, 2018 at 6:34 PM, John Whitmore > > wrote: > > > Replaced memory initialising loop with memset, as suggested by Andy > > > Shevchenko > > > > > > > Suggested-by ? > > > > Em, not sure how to respond, it certainly wasn't my idea. I was just making > coding style changes, badly. ;) Suggested-by is a tag for patches, to give credit. For example: https://elixir.bootlin.com/linux/v4.18-rc1/source/Documentation/process/submitting-patches.rst See section "13) Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:" Hope that helps, Justin. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging: rtl8192u: Prune the rtl819x_HT.h file of unused definitions.
On 29 June 2018 19:10:07 BST, John Whitmore wrote: >There are two files named "rtl819x_HT.h" > >$ find . -name rtl819x_HT.h -print >./drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h >./drivers/staging/rtl8192e/rtl819x_HT.h > >The two files are very similar but differ slightly. Unsed definitions >have >been removed from "drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h" as >a first >step towards possibly merging the two files into one. Just delete the lines if they are unused. Don't worry about commenting them out. That way it is easier to visually diff the patches for review. (Plus, the previous code is never lost in the git tree, anyway) :-) Justin ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] drivers/speakup: Fix style and coding warnings
> On 03 July 2018 at 08:31 Tamir Suliman wrote:
> +++ b/drivers/staging/speakup/keyhelp.c
> @@ -167,7 +167,7 @@ int spk_handle_help(struct vc_data *vc, u_char type,
> u_char ch, u_short key)
> synth_printf("%s\n", spk_msg_get(MSG_HELP_INFO));
> build_key_data(); /* rebuild each time in case new mapping */
> return 1;
> - } else {
> + } else if {
Interesting construct...
> @@ -787,7 +787,7 @@ static ssize_t message_store_helper(const char *buf,
> size_t count,
> continue;
> }
>
> - index = simple_strtoul(cp, &temp, 10);
> + index = simple_ktrtoul(cp, &temp, 10);
Did you compile this?
Justin.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: lustre: lnet: add static to libcfs_dev declaration
Add a static prefix to the declaration for libcfs_dev. This would fix
the following sparse warning:
drivers/staging/lustre/lnet/libcfs/module.c:317:19: warning: symbol
'libcfs_dev' was not declared. Should it be static?
Signed-off-by: Justin Skists
---
drivers/staging/lustre/lnet/libcfs/module.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/lustre/lnet/libcfs/module.c
b/drivers/staging/lustre/lnet/libcfs/module.c
index ca942f474a55..e021e439f140 100644
--- a/drivers/staging/lustre/lnet/libcfs/module.c
+++ b/drivers/staging/lustre/lnet/libcfs/module.c
@@ -314,7 +314,7 @@ static const struct file_operations libcfs_fops = {
.unlocked_ioctl = libcfs_psdev_ioctl,
};
-struct miscdevice libcfs_dev = {
+static struct miscdevice libcfs_dev = {
.minor = MISC_DYNAMIC_MINOR,
.name = "lnet",
.fops = &libcfs_fops,
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/6] staging: android: Clean up license identifiers
> On 14 May 2018 at 14:29 Dan Carpenter wrote: > > > On Sun, May 06, 2018 at 06:13:24PM -0700, Nathan Chancellor wrote: > > Add the identifiers when missing and fix the ones already present > > according to checkpatch.pl. > > > > Signed-off-by: Nathan Chancellor > > --- > > drivers/staging/android/ashmem.h| 6 +- > > drivers/staging/android/uapi/ashmem.h | 6 +- > > drivers/staging/android/uapi/vsoc_shm.h | 10 +- > > drivers/staging/android/vsoc.c | 11 +-- > > 4 files changed, 4 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/staging/android/ashmem.h > > b/drivers/staging/android/ashmem.h > > index 60d7208f110a..1a478173cd21 100644 > > --- a/drivers/staging/android/ashmem.h > > +++ b/drivers/staging/android/ashmem.h > > @@ -1,13 +1,9 @@ > > -// SPDX-License-Identifier: (GPL-2.0 OR Apache-2.0) > > +/* SPDX-License-Identifier: GPL-2.0 OR Apache-2.0 */ > > > // was correct for SPDX headers. Sorry, header files use the /* ... */ format. :) https://elixir.bootlin.com/linux/v4.17-rc5/source/Documentation/process/license-rules.rst Justin. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: speakup: refactor synths array to use a list
The synths[] array is a collection of synths acting like a list.
There is no need for synths to be an array, so refactor synths[] to use
standard kernel list_head API, instead, and modify the usages to suit.
As a side-effect, the maximum number of synths has also become redundant.
Signed-off-by: Justin Skists
---
drivers/staging/speakup/spk_types.h | 2 ++
drivers/staging/speakup/synth.c | 40 ++---
2 files changed, 15 insertions(+), 27 deletions(-)
diff --git a/drivers/staging/speakup/spk_types.h
b/drivers/staging/speakup/spk_types.h
index 3e082dc3d45c..a2fc72c29894 100644
--- a/drivers/staging/speakup/spk_types.h
+++ b/drivers/staging/speakup/spk_types.h
@@ -160,6 +160,8 @@ struct spk_io_ops {
};
struct spk_synth {
+ struct list_head node;
+
const char *name;
const char *version;
const char *long_name;
diff --git a/drivers/staging/speakup/synth.c b/drivers/staging/speakup/synth.c
index 7deeb7061018..25f259ee4ffc 100644
--- a/drivers/staging/speakup/synth.c
+++ b/drivers/staging/speakup/synth.c
@@ -18,8 +18,7 @@
#include "speakup.h"
#include "serialio.h"
-#define MAXSYNTHS 16 /* Max number of synths in array. */
-static struct spk_synth *synths[MAXSYNTHS + 1];
+static LIST_HEAD(synths);
struct spk_synth *synth;
char spk_pitch_buff[32] = "";
static int module_status;
@@ -355,9 +354,8 @@ struct var_t synth_time_vars[] = {
/* called by: speakup_init() */
int synth_init(char *synth_name)
{
- int i;
int ret = 0;
- struct spk_synth *synth = NULL;
+ struct spk_synth *tmp, *synth = NULL;
if (!synth_name)
return 0;
@@ -371,9 +369,10 @@ int synth_init(char *synth_name)
mutex_lock(&spk_mutex);
/* First, check if we already have it loaded. */
- for (i = 0; i < MAXSYNTHS && synths[i]; i++)
- if (strcmp(synths[i]->name, synth_name) == 0)
- synth = synths[i];
+ list_for_each_entry(tmp, &synths, node) {
+ if (strcmp(tmp->name, synth_name) == 0)
+ synth = tmp;
+ }
/* If we got one, initialize it now. */
if (synth)
@@ -448,29 +447,23 @@ void synth_release(void)
/* called by: all_driver_init() */
int synth_add(struct spk_synth *in_synth)
{
- int i;
int status = 0;
+ struct spk_synth *tmp;
mutex_lock(&spk_mutex);
- for (i = 0; i < MAXSYNTHS && synths[i]; i++)
- /* synth_remove() is responsible for rotating the array down */
- if (in_synth == synths[i]) {
+
+ list_for_each_entry(tmp, &synths, node) {
+ if (tmp == in_synth) {
mutex_unlock(&spk_mutex);
return 0;
}
- if (i == MAXSYNTHS) {
- pr_warn("Error: attempting to add a synth past end of array\n");
- mutex_unlock(&spk_mutex);
- return -1;
}
if (in_synth->startup)
status = do_synth_init(in_synth);
- if (!status) {
- synths[i++] = in_synth;
- synths[i] = NULL;
- }
+ if (!status)
+ list_add_tail(&in_synth->node, &synths);
mutex_unlock(&spk_mutex);
return status;
@@ -479,17 +472,10 @@ EXPORT_SYMBOL_GPL(synth_add);
void synth_remove(struct spk_synth *in_synth)
{
- int i;
-
mutex_lock(&spk_mutex);
if (synth == in_synth)
synth_release();
- for (i = 0; synths[i]; i++) {
- if (in_synth == synths[i])
- break;
- }
- for ( ; synths[i]; i++) /* compress table */
- synths[i] = synths[i + 1];
+ list_del(&in_synth->node);
module_status = 0;
mutex_unlock(&spk_mutex);
}
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: rtl8192e: Series of coding style changes
> On 06 June 2018 at 12:39 John Whitmore wrote: > Again these are just some simple coding style changes to the file, so nothing > of importance. If it keeps grumpy maintainers happy, then it's of great importance! :-) Justin ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: speakup: refactor synths array to use a list
On Wed, Jun 06, 2018 at 03:26:28PM +0200, Samuel Thibault wrote: > Hello, > > Justin Skists, le lun. 04 juin 2018 10:52:12 +0100, a ecrit: > > The synths[] array is a collection of synths acting like a list. > > There is no need for synths to be an array, so refactor synths[] to use > > standard kernel list_head API, instead, and modify the usages to suit. > > As a side-effect, the maximum number of synths has also become redundant. > > This looks good to me, > > Reviewed-by: Samuel Thibault Thank you. > Did you test to e.g. insmod speakup_soft ; insmod speakup_dummy ; rmmod > speakup_soft ; rmmod speakup_dummy > > to make sure it did work correctly? I did. And I swapped synths via the sysfs interface. As always, it's always good to double-check. So, I've scripted the test sequence that I used and attached the output. > I'd also rather see it tested in the real wild before committing. As it should be. :) Justin Kernel info --- # uname -a Linux buildroot 4.17.0-rc7-next-20180601 #1 SMP Mon Jun 4 09:31:05 BST 2018 x86_64 GNU/Linux insert modules -- # modprobe speakup # modprobe speakup_dummy dev=ttyS1 ser=1 start=1 # modprobe speakup_soft # lsmod Module Size Used byTainted: G speakup_soft 16384 0 speakup_dummy 16384 0 speakup 118784 2 speakup_soft,speakup_dummy switching to soft - # echo 'soft' > /sys/accessibility/speakup/synth # cat /sys/accessibility/speakup/synth soft switching to dummy -- # echo 'dummy' > /sys/accessibility/speakup/synth # cat /sys/accessibility/speakup/synth dummy Removing modules # rmmod speakup_dummy # rmmod speakup_soft # lsmod Module Size Used byTainted: G speakup 118784 0 view message log # tail -25 /var/log/messages Jun 6 20:06:57 buildroot kern.notice kernel: random: ssh-keygen: uninitialized urandom read (32 bytes read) Jun 6 20:06:57 buildroot kern.notice kernel: random: sshd: uninitialized urandom read (32 bytes read) Jun 6 20:06:57 buildroot auth.info sshd[105]: Server listening on :: port 22. Jun 6 20:06:57 buildroot auth.info sshd[105]: Server listening on 0.0.0.0 port 22. Jun 6 20:06:57 buildroot daemon.info : starting pid 107, tty '/dev/tty1': '/sbin/getty -L tty1 0 vt100 ' Jun 6 20:07:00 buildroot auth.info login[107]: root login on 'tty1' Jun 6 20:07:08 buildroot kern.notice kernel: random: crng init done Jun 6 20:07:12 buildroot kern.warn kernel: speakup: module is from the staging directory, the quality is unknown, you have been warned. Jun 6 20:07:13 buildroot kern.info kernel: input: Speakup as /devices/virtual/input/input4 Jun 6 20:07:13 buildroot kern.info kernel: initialized device: /dev/synth, node (MAJOR 10, MINOR 25) Jun 6 20:07:13 buildroot kern.info kernel: speakup 3.1.6: initialized Jun 6 20:07:13 buildroot kern.info kernel: synth name on entry is: (null) Jun 6 20:07:13 buildroot kern.warn kernel: speakup_dummy: module is from the staging directory, the quality is unknown, you have been warned. Jun 6 20:07:13 buildroot kern.warn kernel: synth probe Jun 6 20:07:13 buildroot kern.warn kernel: speakup_soft: module is from the staging directory, the quality is unknown, you have been warned. Jun 6 20:07:13 buildroot kern.info kernel: releasing synth dummy Jun 6 20:07:13 buildroot kern.warn kernel: synth probe Jun 6 20:07:13 buildroot kern.info kernel: initialized device: /dev/softsynth, node (MAJOR 10, MINOR 26) Jun 6 20:07:13 buildroot kern.info kernel: initialized device: /dev/softsynthu, node (MAJOR 10, MINOR 27) Jun 6 20:07:13 buildroot kern.warn kernel: soft already in use Jun 6 20:07:13 buildroot kern.info kernel: releasing synth soft Jun 6 20:07:13 buildroot kern.info kernel: unregistered /dev/softsynth Jun 6 20:07:13 buildroot kern.info kernel: unregistered /dev/softsynthu Jun 6 20:07:13 buildroot kern.warn kernel: synth probe Jun 6 20:07:13 buildroot kern.info kernel: releasing synth dummy ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: speakup: refactor synths array to use a list
> On 18 June 2018 at 06:34 Gregory Nowak wrote: > > > On Tue, Jun 12, 2018 at 08:31:06AM +0200, Samuel Thibault wrote: > > The load/unload is about the module itself, i.e. modprobe speakup_bns ; > > modprobe speakup_soft, switch between them, then rmmod speakup_bns ; > > speakup_soft or the converse (to exercise both orders). > > # uname -a > Linux p41box 4.17.1 #1 SMP Sat Jun 16 11:19:57 MST 2018 i686 GNU/Linux > # lsmod |grep "speakup" > speakup_bns16384 0 > speakup_soft 16384 1 > speakup94208 3 speakup_bns,speakup_soft > > With /sys/accessibility/speakup/synth set to bns, I am getting output > alternately from the bns and from soft. It's as if speakup can't make > up its mind which synthesizer is being used. When I echo soft > >/sys/accessibility/speakup/synth, I get no speech at all from either > synthesizer. Is this a known issue, or a regression due to the patch? Justin. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: speakup: refactor synths array to use a list
> On 18 June 2018 at 09:46 Samuel Thibault wrote: > > > Justin Skists, le lun. 18 juin 2018 09:41:44 +0100, a ecrit: > > > On 18 June 2018 at 06:34 Gregory Nowak wrote: > > > With /sys/accessibility/speakup/synth set to bns, I am getting output > > > alternately from the bns and from soft. It's as if speakup can't make > > > up its mind which synthesizer is being used. When I echo soft > > > >/sys/accessibility/speakup/synth, I get no speech at all from either > > > synthesizer. > > > > Is this a known issue, or a regression due to the patch? > > I don't think it was a known issue, but I don't think it can be a > regression due to the patch, since none of that is handled by the > array/list at stake. OK, thanks. That's what I thought, too. When I was going through the driver code, to become familiar with it, there were a few places that I thought needed a closer look. But I need to finish setting up a regression testing system before I do. Justin. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging/speakup: fix checkpatch.pl warning in speak_char()
correct the following warning from checkpatch.pl:-
WARNING: Prefer using '"%s...", __func__' to using 'speak_char', this
function's name, in a string
Signed-off-by: Justin Skists
Acked-by: Samuel Thibault
---
drivers/staging/speakup/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index 67956e24779c..938a0aed7de5 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -447,7 +447,7 @@ static void speak_char(u16 ch)
cp = spk_characters[ch];
if (!cp) {
- pr_info("speak_char: cp == NULL!\n");
+ pr_info("%s: cp == NULL!\n", __func__);
return;
}
if (IS_CHAR(ch, B_CAP)) {
--
2.14.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: lustre: Fix unneeded byte-ordering cast
Fix sparse warning: CHECK drivers/staging//lustre/lnet/lnet/acceptor.c drivers/staging//lustre/lnet/lnet/acceptor.c:243:30: warning: cast to restricted __le32 LNET_PROTO_TCP_MAGIC, as a define, is already CPU byte-ordered when compared to 'magic', so no need for a cast. Signed-off-by: Justin Skists --- drivers/staging/lustre/lnet/lnet/acceptor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lnet/lnet/acceptor.c b/drivers/staging/lustre/lnet/lnet/acceptor.c index fb478e20e204..13e981781b9a 100644 --- a/drivers/staging/lustre/lnet/lnet/acceptor.c +++ b/drivers/staging/lustre/lnet/lnet/acceptor.c @@ -240,7 +240,7 @@ lnet_accept(struct socket *sock, __u32 magic) return -EPROTO; } - if (magic == le32_to_cpu(LNET_PROTO_TCP_MAGIC)) + if (magic == LNET_PROTO_TCP_MAGIC) str = "'old' socknal/tcpnal"; else str = "unrecognised"; -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: Fix unneeded byte-ordering cast
On 20 March 2018 at 01:06, NeilBrown wrote: > On Sat, Mar 17 2018, Justin Skists wrote: > >> Fix sparse warning: >> >> CHECK drivers/staging//lustre/lnet/lnet/acceptor.c >> drivers/staging//lustre/lnet/lnet/acceptor.c:243:30: warning: cast to >> restricted __le32 >> >> LNET_PROTO_TCP_MAGIC, as a define, is already CPU byte-ordered when >> compared to 'magic', so no need for a cast. >> >> Signed-off-by: Justin Skists >> --- >> drivers/staging/lustre/lnet/lnet/acceptor.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/staging/lustre/lnet/lnet/acceptor.c >> b/drivers/staging/lustre/lnet/lnet/acceptor.c >> index fb478e20e204..13e981781b9a 100644 >> --- a/drivers/staging/lustre/lnet/lnet/acceptor.c >> +++ b/drivers/staging/lustre/lnet/lnet/acceptor.c >> @@ -240,7 +240,7 @@ lnet_accept(struct socket *sock, __u32 magic) >> return -EPROTO; >> } >> >> - if (magic == le32_to_cpu(LNET_PROTO_TCP_MAGIC)) >> + if (magic == LNET_PROTO_TCP_MAGIC) >> str = "'old' socknal/tcpnal"; >> else >> str = "unrecognised"; > > This code is almost completely irrelevant (it just choose which error > message to use when failing), but we may as well get it right and I > cannot see why your change is a fix. I admit that the change is trivial, and, in hindsight, the word fix is a little "strong". The rationale was that the if-statement, as it was, probably wouldn't work as intented on big-endian systems. I chose this sparse warning to test the waters, as it was an isolated change, before I thought about proposing a bigger change: There are quite a few sparse warning in regards to struct lnet_hdr with regards to __u32 vs. __le32 (etc.) restricted castings. > I suspect a more correct fix would be to use > lnet_accept_magic(magic, LNET_PROTO_TCP_MAGIC) > as the condition of the if(). This is consistent with other code that > tests magic, and it is consistent with the general understanding that > "magic" should be in host-byte-order for the peer which sent the > message. > > Could you resubmit with that change? I agree that your suggestion is a much better fix. As Greg has already accepted the patch in question into staging-next, would the correct course of action be for me to submit a new patch with a "fixes" tag based on staging-next? Or would Greg prefer to drop the previous one for a fresh v2? Regards, Justin. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: lustre: lnet: use correct 'magic' test
Use the lnet_magic_accept() function to compare 'magic' against
LNET_PROTO_TCP_MAGIC for the appropriate string for an error message.
The original fix removed an unneeded byte-ordering cast because the define
was already CPU byte-ordered and it was assumed that 'magic' was CPU
byte-ordered, too.
Now modify the if-statement to use the appropriate lnet_accept_magic()
function in order to be consistent with similar tests. This will allow
the code to be consistent with the general understanding that 'magic'
should be in host-byte-order for the peer that sent the message.
Fixes: 80782927e3aa ("staging: lustre: Fix unneeded byte-ordering cast")
Cc: NeilBrown
Signed-off-by: Justin Skists
---
drivers/staging/lustre/lnet/lnet/acceptor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/lustre/lnet/lnet/acceptor.c
b/drivers/staging/lustre/lnet/lnet/acceptor.c
index 13e981781b9a..5648f17eddc0 100644
--- a/drivers/staging/lustre/lnet/lnet/acceptor.c
+++ b/drivers/staging/lustre/lnet/lnet/acceptor.c
@@ -240,7 +240,7 @@ lnet_accept(struct socket *sock, __u32 magic)
return -EPROTO;
}
- if (magic == LNET_PROTO_TCP_MAGIC)
+ if (lnet_accept_magic(magic, LNET_PROTO_TCP_MAGIC))
str = "'old' socknal/tcpnal";
else
str = "unrecognised";
--
2.16.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
