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.
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.
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.
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.
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
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
[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
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.
[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
[PATCH] Documentation/process/posting: wrap text at 80 cols
Trivial patch to adjust the text formatting to wrap at 80 columns. No actual content has changed. Signed-off-by: Justin Skists --- Documentation/process/5.Posting.rst | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Documentation/process/5.Posting.rst b/Documentation/process/5.Posting.rst index c209d70da66f..c418c5d6cae4 100644 --- a/Documentation/process/5.Posting.rst +++ b/Documentation/process/5.Posting.rst @@ -10,8 +10,8 @@ of conventions and procedures which are used in the posting of patches; following them will make life much easier for everybody involved. This document will attempt to cover these expectations in reasonable detail; more information can also be found in the files process/submitting-patches.rst, -process/submitting-drivers.rst, and process/submit-checklist.rst in the kernel documentation -directory. +process/submitting-drivers.rst, and process/submit-checklist.rst in the kernel +documentation directory. When to post @@ -198,8 +198,8 @@ pass it to diff with the "-X" option. The tags mentioned above are used to describe how various developers have been associated with the development of this patch. They are described in -detail in the process/submitting-patches.rst document; what follows here is a brief -summary. Each of these lines has the format: +detail in the process/submitting-patches.rst document; what follows here is a +brief summary. Each of these lines has the format: :: @@ -210,8 +210,8 @@ The tags in common use are: - Signed-off-by: this is a developer's certification that he or she has the right to submit the patch for inclusion into the kernel. It is an agreement to the Developer's Certificate of Origin, the full text of - which can be found in Documentation/process/submitting-patches.rst. Code without a - proper signoff cannot be merged into the mainline. + which can be found in Documentation/process/submitting-patches.rst. Code + without a proper signoff cannot be merged into the mainline. - Co-developed-by: states that the patch was also created by another developer along with the original author. This is useful at times when multiple @@ -226,8 +226,8 @@ The tags in common use are: it to work. - Reviewed-by: the named developer has reviewed the patch for correctness; - see the reviewer's statement in Documentation/process/submitting-patches.rst for more - detail. + see the reviewer's statement in Documentation/process/submitting-patches.rst + for more detail. - Reported-by: names a user who reported a problem which is fixed by this patch; this tag is used to give credit to the (often underappreciated) -- 2.17.0
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.
[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
Where did my symbols go?
Hi. Sorry if this is an idiotic question, especially if it's one worthy of slapping! I've been trying to set my system up to have multiple linux "test-bed" setups on one drive - That was done easily, I thought. However, my Caldera (freebie eDesktop 2.4) distribution won't insmod my driver, saying that a load of unresolved symbols exist. The strange thing, is that all the symbols, I thought, are supposed to be in the kernel itself. Symbols like kmalloc, iounmap, jiffies, printk... I've never come across this problem before. I'm pretty sure that my driver isn't the only thing on my Caldera partition that need these symbols! Any lights? Does the System.map file have any part in the boot (or module-loading) process? I thought it was just a text file that gets produced when the kernel is compiled showing humans (or debuggers) where everything is... Justin. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
RE: Linux kernel modules development in C++
But but but.. wasn't the very first C++ compilers really just a preprocessor into standard C? I went to a advanced C course, a year or so ago, and the guy showed us how to write OO code in C that went a little beyond everyday encapsulation. At the end, he told us that that was the type of code that the old C++ preprocessors used to output. Justin. > -Original Message- > > > > Nothing. What I was saying if you want some OO language in the kernel, > C++ > > > is the only option I guess. Mixing languages is a pain.. > > > > There is no such thing as "OO language". There is OO style and there are > > ways to use it when you are writing in almost any language. And OO in C > is > > not harder than OO in C++, provided that you are not trying to write in > > C++ and translate line-by-line into C. > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
RE: cPCI development
Am I right in assumming that 2.2.14 (as from RH6.2) supports cPCI? Or do I need to start developing on 2.4? I really do need to do some research into this, if I knew where to start. I need some docs! (either paper, URL, or the straight-jacket kind) Justin. > -Original Message- > From: Russell King [SMTP:[EMAIL PROTECTED]] > > [EMAIL PROTECTED] writes: > > I'm about to embark on some compact-PCI driver development for Linux and > I > > was wondering where I can find some info. Is there any difference > between > > PCI and cPCI development on Linux? > > > > URLs would be great! Or, if this is the wrong list for driver > development > > issues, a pointer to the correct mailing list would be lovely. > > I believe that cPCI is the same as normal PCI from the software point of > view. > However, cPCI has different connectors. > > I'm currently working on an ARM development board which can be used either > as a ATX motherboard with PCI slots, or plugged into a cPCI backplane, and > the only thing between the two is a standard DEC PCI to PCI bridge. > > Disclaimer: I haven't done any in-depth investigations yet, so I could be > talking complete and utter nonsense here. > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
cPCI development
Hi. I'm about to embark on some compact-PCI driver development for Linux and I was wondering where I can find some info. Is there any difference between PCI and cPCI development on Linux? URLs would be great! Or, if this is the wrong list for driver development issues, a pointer to the correct mailing list would be lovely. Cheers, Justin. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
RE: PCI-DMA
Standard book: Try "Linux Device Drivers" by Alessandro Rubini (O'Reilly)... Justin > -Original Message- > From: [EMAIL PROTECTED] [SMTP:[EMAIL PROTECTED]] > > hi, > can anybody tell, where to find information about dma usage by pci > -devices in linux > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
