Re: [PATCH v2 57/60] staging: ced1401: usb1401.c fix checkpatch errors
On Thu, Jul 10, 2014 at 11:04:13AM +0200, Luca Ellero wrote: Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com Which checkpatch warnings? Please be specific. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 59/60] staging: ced1401: ced_ioc.c fix checkpatch warnings
On Thu, Jul 10, 2014 at 11:04:15AM +0200, Luca Ellero wrote: Signed-off-by: Luca Ellero luca.ell...@brickedbrain.com --- drivers/staging/ced1401/ced_ioc.c | 212 +++-- 1 file changed, 131 insertions(+), 81 deletions(-) That's a lot of changes all in one patch. Please take these last 4 patches and rework them to be more specific, and to only do one thing per patch (logical thing, like fixing comments in a file, etc.) I've applied all of the other ones in the series, so this should give you an easier base to work off of. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 4/4] staging: rtl8192u: Fixed too long line
On Thu, Jul 10, 2014 at 05:54:57PM +0530, sanjeev sharma wrote:
From: sanjeev sharma sanjeev_sha...@mentor.com
This patch will fix too long lines warning
reported by checkpatch.pl.
Signed-off-by: Sanjeev Sharma sanjeev_sha...@mentor.com
---
drivers/staging/rtl8192u/r819xU_phy.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/rtl8192u/r819xU_phy.c
b/drivers/staging/rtl8192u/r819xU_phy.c
index 3155616..b2d81e9 100644
--- a/drivers/staging/rtl8192u/r819xU_phy.c
+++ b/drivers/staging/rtl8192u/r819xU_phy.c
@@ -1236,7 +1236,8 @@ static u8 rtl8192_phy_SetSwChnlCmdArray(SwChnlCmd
*CmdTable, u32 CmdTableIdx,
return false;
}
if (CmdTableIdx = CmdTableSz) {
- RT_TRACE(COMP_ERR, %s(): Access invalid index, please check
size of the table, CmdTableIdx:%d, CmdTableSz:%d\n,
+ RT_TRACE(COMP_ERR,
+ %s():invalid index,CmdTableIdx:%d, CmdTableSz:%d\n,
Why did you change the string? That's not ok, you can ignore this
warning, it's a string that you can not break up, or change.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: unisys: added virtpci info entry
On Fri, Jul 11, 2014 at 07:11:45PM -0400, Erik Arfvidson wrote:
This patch adds the virtpci debugfs directory and the info entry
inside of it.
Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com
Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com
---
v3: Fixed formating and comments. Also added debufs_remove_recursive() and
simple simple_read_from_buffer()
v2: fixed comments and applied Dan Carpenter suggestions
drivers/staging/unisys/virtpci/virtpci.c | 108
++-
1 file changed, 105 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/unisys/virtpci/virtpci.c
b/drivers/staging/unisys/virtpci/virtpci.c
index 7d840b0..6ea29f5 100644
--- a/drivers/staging/unisys/virtpci/virtpci.c
+++ b/drivers/staging/unisys/virtpci/virtpci.c
@@ -36,6 +36,7 @@
#include linux/mod_devicetable.h
#include linux/if_ether.h
#include linux/version.h
+#include linux/debugfs.h
#include version.h
#include guestlinuxdebug.h
#include timskmodutils.h
@@ -100,6 +101,12 @@ static int virtpci_device_resume(struct device *dev);
static int virtpci_device_probe(struct device *dev);
static int virtpci_device_remove(struct device *dev);
+static ssize_t info_debugfs_read(struct file *file, char __user *buf,
+ size_t len, loff_t *offset);
+
+static const struct file_operations debugfs_info_fops = {
+ .read = info_debugfs_read,
+};
/*/
/* Globals */
@@ -139,7 +146,19 @@ static DEFINE_RWLOCK(VpcidevListLock);
/* filled in with info about this driver, wrt it servicing client busses */
static ULTRA_VBUS_DEVICEINFO Bus_DriverInfo;
-
+/*/
+/* DebugFS entries */
+/*/
+/* dentry is used to create the debugfs entry directory
+ * for virtpci
+ */
+static struct dentry *virtpci_debugfs_dir;
+static struct dentry *info_debugfs_entry;
+/* info_debugfs_entry is used to tell virtpci to display current info
+ * kept in the driver
+ */
You don't need this variable as all you do is set it, and then never do
anything with it. Worse case, make it a local variable. As you never
check it, just don't even do that...
+#define DIR_DEBUGFS_ENTRY virtpci
+#define INFO_DEBUGFS_ENTRY_FN info
You only reference these #defines once, no need for them at all.
struct virtpci_busdev {
struct device virtpci_bus_device;
@@ -1375,6 +1394,85 @@ void virtpci_unregister_driver(struct virtpci_driver
*drv)
DBGINF(Leaving\n);
}
EXPORT_SYMBOL_GPL(virtpci_unregister_driver);
+/*/
+/* debugfs filesystem functions */
+/*/
+struct print_vbus_info {
+ int *length;
+ char *buf;
+};
+
+static int print_vbus(struct device *vbus, void *data)
+{
+ struct print_vbus_info *p = (struct print_vbus_info *)data;
+
+ *p-length += sprintf(p-buf + *p-length, bus_id:%s\n,
+ dev_name(vbus));
+ return 0;
+}
+
+static ssize_t info_debugfs_read(struct file *file, char __user *buf,
+ size_t len, loff_t *offset)
+{
+ ssize_t bytes_read = 0;
+ int str_pos = 0;
+ struct virtpci_dev *tmpvpcidev;
+ unsigned long flags;
+ struct print_vbus_info printparam;
+ char *vbuf;
+
+ vbuf = kzalloc(len, GFP_KERNEL);
Sorry I missed this last time, but what happens if userspace asks for
10Gb? Nasty problems will happen if you try to allocate that. Or
100Mb. Set a reasonable upper bound here please.
Remember:
All input is evil.
+ if (!vbuf)
+ return -ENOMEM;
+
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ Virtual PCI Bus devices\n);
Why the leading ' '?
+ printparam.length = str_pos;
+ printparam.buf = vbuf;
+ if (bus_for_each_dev(virtpci_bus_type, NULL,
+ (void *) printparam, print_vbus))
+ LOGERR(Failed to find bus\n);
+
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ \n Virtual PCI devices\n);
+ read_lock_irqsave(VpcidevListLock, flags);
+ tmpvpcidev = VpcidevListHead;
+ while (tmpvpcidev) {
+ if (tmpvpcidev-devtype == VIRTHBA_TYPE) {
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ [%d:%d] VHba:%08x:%08x
max-config:%d-%d-%d-%d,
+ tmpvpcidev-busNo, tmpvpcidev-deviceNo,
+ tmpvpcidev-scsi.wwnn.wwnn1,
+ tmpvpcidev-scsi.wwnn.wwnn2,
+ tmpvpcidev-scsi.max.max_channel,
+
Re: [PATCH v3] drivers/staging/comedi/drivers/ni_atmio16d.c: remove pointless condition
On Fri, Jul 11, 2014 at 03:39:48PM +0100, Ian Abbott wrote: On 2014-07-11 15:38, Ian Abbott wrote: From: Andrey Utkin andrey.krieger.ut...@gmail.com From: Andrey Utkin andrey.krieger.ut...@gmail.com Dammit! Greg, do you want to sort that out or should I have another go? Heh, no worries, I can fix it up, thanks. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote: A simple script to run checkpatch --fix for various types of of cleanups. This script is useful primarily for staging. This reformats code to a more CodingStyle conforming style, compiles it, verifies that the object code hasn't changed, and git commits it too. And 'git commits' it? Heh, I should just run this myself to clean up staging and beat everyone else to it... I know some people already have private versions of these things, might as well make it public for all to abuse :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote:
A simple script to run checkpatch --fix for various types of
of cleanups.
This script is useful primarily for staging.
This reformats code to a more CodingStyle conforming style,
compiles it, verifies that the object code hasn't changed,
and git commits it too.
You must have the necessary development tools, git, and a
recent git tree. Ideally use Greg KH's staging-next, which
can be retrieved via these commands:
git clone git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
git checkout staging-next
To use this script try a sequence of commands like:
cd linux_repository
git checkout -b your_branch
make allyesconfig
mkdir patches
./scripts/reformat_with_checkpatch.sh drivers/staging/dir/*.[ch]
git format-patch --cover-letter -o patches/your_branch staging-next
git send-email patches/your_branch
Signed-off-by: Joe Perches j...@perches.com
---
scripts/reformat_with_checkpatch.sh | 141
1 file changed, 141 insertions(+)
create mode 100755 scripts/reformat_with_checkpatch.sh
No --help option?
How do I run this thing?
I ran it on a file that had no problems and got a mess, all I think due
to something odd in checkpatch itself right now:
$ ./scripts/checkpatch.pl --file
drivers/staging/lustre/include/linux/lnet/api.h
Useless use of greediness modifier '+' in regex; marked by -- HERE in
m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217.
total: 0 errors, 0 warnings, 220 lines checked
What's with that 'Useless use... error? perl 5.20 in use, is that the issue?
Anyway, running this script on the file gave me this:
$ ./reformat_with_checkpatch drivers/staging/lustre/include/linux/lnet/api.h
file: drivers/staging/lustre/include/linux/lnet/api.h description:
whitespace neatening
types:spacing,space_before_tab,pointer_location,trailing_whitespace,bracket_space
Useless use of greediness modifier '+' in regex; marked by -- HERE in
m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217.
total: 0 errors, 0 warnings, 0 checks, 220 lines checked
NOTE: Used message types: BRACKET_SPACE POINTER_LOCATION SPACE_BEFORE_TAB
SPACING TRAILING_WHITESPACE
drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems
and is ready for submission.
file: drivers/staging/lustre/include/linux/lnet/api.h description: remove
spaces before tabs types:space_before_tab
Useless use of greediness modifier '+' in regex; marked by -- HERE in
m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217.
total: 0 errors, 0 warnings, 0 checks, 220 lines checked
NOTE: Used message types: SPACE_BEFORE_TAB
drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems
and is ready for submission.
file: drivers/staging/lustre/include/linux/lnet/api.h description: fix label
positions types:indented_label
Useless use of greediness modifier '+' in regex; marked by -- HERE in
m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217.
total: 0 errors, 0 warnings, 0 checks, 220 lines checked
NOTE: Used message types: INDENTED_LABEL
drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems
and is ready for submission.
file: drivers/staging/lustre/include/linux/lnet/api.h description: align
arguments to parenthesis types:parenthesis_alignment
Useless use of greediness modifier '+' in regex; marked by -- HERE in
m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217.
total: 0 errors, 0 warnings, 0 checks, 220 lines checked
NOTE: Used message types: PARENTHESIS_ALIGNMENT
drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems
and is ready for submission.
file: drivers/staging/lustre/include/linux/lnet/api.h description: fix brace
positions types:open_brace,braces,else_after_brace,while_after_brace
Useless use of greediness modifier '+' in regex; marked by -- HERE in
m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217.
total: 0 errors, 0 warnings, 0 checks, 220 lines checked
NOTE: Used message types: BRACES ELSE_AFTER_BRACE OPEN_BRACE WHILE_AFTER_BRACE
drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems
and is ready for submission.
file: drivers/staging/lustre/include/linux/lnet/api.h description: fix blank
lines types:line_spacing
Useless use of greediness modifier '+' in regex; marked by -- HERE in
m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217.
total: 0 errors, 0 warnings, 0 checks, 220 lines checked
NOTE: Used message types: LINE_SPACING
drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems
and is ready for submission.
file: drivers/staging/lustre/include/linux/lnet/api.h description: use
standard attributes types:prefer_packed,prefer_aligned
Useless use of greediness modifier '+' in regex; marked by -- HERE in
m/(^\+.*)
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote:
A simple script to run checkpatch --fix for various types of
of cleanups.
This script is useful primarily for staging.
This reformats code to a more CodingStyle conforming style,
compiles it, verifies that the object code hasn't changed,
and git commits it too.
You must have the necessary development tools, git, and a
recent git tree. Ideally use Greg KH's staging-next, which
can be retrieved via these commands:
git clone git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
git checkout staging-next
To use this script try a sequence of commands like:
cd linux_repository
git checkout -b your_branch
make allyesconfig
mkdir patches
./scripts/reformat_with_checkpatch.sh drivers/staging/dir/*.[ch]
git format-patch --cover-letter -o patches/your_branch staging-next
git send-email patches/your_branch
Signed-off-by: Joe Perches j...@perches.com
---
scripts/reformat_with_checkpatch.sh | 141
1 file changed, 141 insertions(+)
create mode 100755 scripts/reformat_with_checkpatch.sh
diff --git a/scripts/reformat_with_checkpatch.sh
b/scripts/reformat_with_checkpatch.sh
new file mode 100755
index 000..5415a8e
--- /dev/null
+++ b/scripts/reformat_with_checkpatch.sh
@@ -0,0 +1,141 @@
+#!/bin/bash
+# (c) 2014, Joe Perches j...@perches.com
+#
+# Automate checkpatch modifications and git commits to
+# neaten code that doesn't conform to CodingStyle.
+# Useful primarily for files in drivers/staging
+#
+# Licensed under the terms of the GNU GPL License version 2
+
+declare -ar whitespace_types=( \
+
'whitespace_neatening:spacing,space_before_tab,pointer_location,trailing_whitespace,bracket_space'
\
+'remove_spaces_before_tabs:space_before_tab' \
+'fix_label_positions:indented_label' \
+'align_arguments_to_parenthesis:parenthesis_alignment' \
+)
+
+declare -ar changecode_types=( \
+
'fix_brace_positions:open_brace,braces,else_after_brace,while_after_brace'
\
+'fix_blank_lines:line_spacing' \
+'use_standard_attributes:prefer_packed,prefer_aligned' \
+'remove_unnecessary_externs:avoid_externs' \
+'update_c90_comment_style:c99_comments' \
+)
+
+checkpatch_update ()
+{
+file=$1
+
+desc=$(echo $2 | cut -f1 -d: | sed 's/_/ /g')
+types=$(echo $2 | cut -f2- -d:)
+
+echo file: $file description: $desc types:$types
+
+./scripts/checkpatch.pl --file --fix-inplace --strict --types=$types
$file
+
+checkpatch_fixes=$file.diff
+git diff --stat -p --exit-code $file $checkpatch_fixes
+if [ $? == 0 ] ; then
+ rm -f $checkpatch_fixes
+ return
+fi
+
+basename=$(basename $file)
+if [ ${basename##*.} == c ] ; then
+
+ git checkout $file
+ obj=$(echo $file | sed 's/\.c$/\.o/')
+ if [ -e $obj ] ; then
+ rm -f $obj
+ fi
+
+ echo Compiling original version...
+
+ ${CROSS_COMPILE}make $obj
+
+ ${CROSS_COMPILE}objdump -D $obj | \
+ sed s/^[[:space:]]\+[0-9a-f]\+// $obj.old
+
+ patch -p1 $checkpatch_fixes
+
+ echo Compiling modified version...
+
+ ${CROSS_COMPILE}make $obj
+
+ ${CROSS_COMPILE}objdump -D $obj | \
+ sed s/^[[:space:]]\+[0-9a-f]\+// $obj.new
+
+ echo Comparing objects...
+ diff -Nurd $obj.new $obj.old
+ if [ $? -ne 0 ] ; then
+ echo Object differences exist! - Verify changes before commit!
+ read -s -p Press the 'enter' key to continue:
+ else
+ echo No object differences found
+ fi
+ rm -f $obj.old
+ rm -f $obj.new
+fi
+
+echo running checkpatch on possible checkpatch fixes...
+
+./scripts/checkpatch.pl --no-summary --no-signoff $checkpatch_fixes
+rm -f $checkpatch_fixes
+
+echo Verify checkpatch output and make any necessary changes
+echo Edit '$file' if appropriate
+read -s -p Press the 'enter' key to continue:
+
+commit_log_file=$(mktemp git_commit.XX)
+
+if [ -e $commit_log_file ] ; then
+ rm -f $commit_log_file
+fi
+
+if [[ $file =~ ^drivers/staging/ ]] ; then
+ echo -n staging: $commit_log_file
+fi
+echo $(basename $(dirname $file)): checkpatch cleanup: $desc
$commit_log_file
If I pick drivers/staging/lustre/include/linux/lnet/types.h, then I get:
staging: lnet: checkpatch cleanup: whitespace neatening
and no 'types.h' here, is that intentional? If so, why?
And this is fun, I'm going to let this rip on the lustre code...
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote:
A simple script to run checkpatch --fix for various types of
of cleanups.
This script is useful primarily for staging.
This reformats code to a more CodingStyle conforming style,
compiles it, verifies that the object code hasn't changed,
and git commits it too.
You must have the necessary development tools, git, and a
recent git tree. Ideally use Greg KH's staging-next, which
can be retrieved via these commands:
git clone git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
git checkout staging-next
To use this script try a sequence of commands like:
cd linux_repository
git checkout -b your_branch
make allyesconfig
mkdir patches
./scripts/reformat_with_checkpatch.sh drivers/staging/dir/*.[ch]
git format-patch --cover-letter -o patches/your_branch staging-next
git send-email patches/your_branch
Signed-off-by: Joe Perches j...@perches.com
---
scripts/reformat_with_checkpatch.sh | 141
1 file changed, 141 insertions(+)
create mode 100755 scripts/reformat_with_checkpatch.sh
diff --git a/scripts/reformat_with_checkpatch.sh
b/scripts/reformat_with_checkpatch.sh
new file mode 100755
index 000..5415a8e
--- /dev/null
+++ b/scripts/reformat_with_checkpatch.sh
@@ -0,0 +1,141 @@
+#!/bin/bash
+# (c) 2014, Joe Perches j...@perches.com
+#
+# Automate checkpatch modifications and git commits to
+# neaten code that doesn't conform to CodingStyle.
+# Useful primarily for files in drivers/staging
+#
+# Licensed under the terms of the GNU GPL License version 2
+
+declare -ar whitespace_types=( \
+
'whitespace_neatening:spacing,space_before_tab,pointer_location,trailing_whitespace,bracket_space'
\
+'remove_spaces_before_tabs:space_before_tab' \
+'fix_label_positions:indented_label' \
+'align_arguments_to_parenthesis:parenthesis_alignment' \
+)
+
+declare -ar changecode_types=( \
+
'fix_brace_positions:open_brace,braces,else_after_brace,while_after_brace'
\
+'fix_blank_lines:line_spacing' \
+'use_standard_attributes:prefer_packed,prefer_aligned' \
+'remove_unnecessary_externs:avoid_externs' \
+'update_c90_comment_style:c99_comments' \
+)
+
+checkpatch_update ()
+{
+file=$1
+
+desc=$(echo $2 | cut -f1 -d: | sed 's/_/ /g')
+types=$(echo $2 | cut -f2- -d:)
+
+echo file: $file description: $desc types:$types
+
+./scripts/checkpatch.pl --file --fix-inplace --strict --types=$types
$file
+
+checkpatch_fixes=$file.diff
+git diff --stat -p --exit-code $file $checkpatch_fixes
+if [ $? == 0 ] ; then
+ rm -f $checkpatch_fixes
+ return
+fi
+
+basename=$(basename $file)
+if [ ${basename##*.} == c ] ; then
+
+ git checkout $file
+ obj=$(echo $file | sed 's/\.c$/\.o/')
+ if [ -e $obj ] ; then
+ rm -f $obj
+ fi
+
+ echo Compiling original version...
+
+ ${CROSS_COMPILE}make $obj
This fails for when a cflags option is set (like an include path). Now,
I could argue that having an include path override in a kernel Makefile
is a bug, but well, it's staging...
Anyway, try running this script on
drivers/staging/lustre/lnet/lnet/acceptor.c to see how this build fails.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 06:46:52PM -0700, Joe Perches wrote:
On Fri, 2014-07-11 at 18:39 -0700, Greg KH wrote:
On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote:
A simple script to run checkpatch --fix for various types of
of cleanups.
]
drivers/staging/lustre/include/linux/lnet/api.h has no obvious style
problems and is ready for submission.
file: drivers/staging/lustre/include/linux/lnet/api.h description:
update c90 comment style types:c99_comments
Useless use of greediness modifier '+' in regex; marked by -- HERE in
m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217.
[]
drivers/staging/lustre/include/linux/lnet/api.h has no obvious style
problems and is ready for submission.
[]
Is that expected?
No, I haven't seen that.
Can you tell me what git tree you're working on?
My staging-next branch of staging.git on git.kernel.org
Also, can you use the scripts/checkpatch from -next
tag next-20140711
that will take a bit to checkout, I'll do that afterward.
My system has:
$ perl --version
This is perl 5, version 18, subversion 2 (v5.18.2) built for
i686-linux-gnu-thread-multi-64int
(with 41 registered patches, see perl -V for more detail)
I think this started showing up for me for perl 5.20. Let me go
checkout linux-next and see if that fixes anything or not...
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 07:01:14PM -0700, Greg KH wrote:
On Fri, Jul 11, 2014 at 06:46:52PM -0700, Joe Perches wrote:
On Fri, 2014-07-11 at 18:39 -0700, Greg KH wrote:
On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote:
A simple script to run checkpatch --fix for various types of
of cleanups.
]
drivers/staging/lustre/include/linux/lnet/api.h has no obvious style
problems and is ready for submission.
file: drivers/staging/lustre/include/linux/lnet/api.h description:
update c90 comment style types:c99_comments
Useless use of greediness modifier '+' in regex; marked by -- HERE in
m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2217.
[]
drivers/staging/lustre/include/linux/lnet/api.h has no obvious style
problems and is ready for submission.
[]
Is that expected?
No, I haven't seen that.
Can you tell me what git tree you're working on?
My staging-next branch of staging.git on git.kernel.org
Also, can you use the scripts/checkpatch from -next
tag next-20140711
that will take a bit to checkout, I'll do that afterward.
My system has:
$ perl --version
This is perl 5, version 18, subversion 2 (v5.18.2) built for
i686-linux-gnu-thread-multi-64int
(with 41 registered patches, see perl -V for more detail)
I think this started showing up for me for perl 5.20. Let me go
checkout linux-next and see if that fixes anything or not...
Ok, with linux-next I get the same thing:
~/linux/tmp/linux-next $ ./scripts/checkpatch.pl -f --strict
drivers/staging/lustre/include/linux/lnet/api.h --types=c99_comments
Useless use of greediness modifier '+' in regex; marked by -- HERE in
m/(^\+.*) {8,8}+ -- HERE \t/ at ./scripts/checkpatch.pl line 2358.
total: 0 errors, 0 warnings, 0 checks, 220 lines checked
NOTE: Used message types: C99_COMMENTS
drivers/staging/lustre/include/linux/lnet/api.h has no obvious style problems
and is ready for submission.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 06:57:24PM -0700, Joe Perches wrote: On Fri, 2014-07-11 at 18:53 -0700, Greg KH wrote: On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote: A simple script to run checkpatch --fix for various types of of cleanups. [] Anyway, try running this script on drivers/staging/lustre/lnet/lnet/acceptor.c to see how this build fails. lustre doesn't use normal kernel makefiles. Well, it does, it just has a: subdir-ccflags-y := -I$(src)/include/ line in drivers/staging/lustre/Makefile that messes up making .o files in subdirectories. I'll go fix that include file mess up now to be able to remove that line so the the .o file building will work. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] checkpatch: Remove unnecessary + after {8,8}
On Fri, Jul 11, 2014 at 07:09:30PM -0700, Joe Perches wrote: There's a useless + use that needs to be removed as perl 5.20 emits a Useless use of greediness modifier '+' message each time it's hit. Reported-by: Greg KH gre...@linuxfoundation.org Signed-off-by: Joe Perches j...@perches.com --- On Fri, 2014-07-11 at 19:05 -0700, Greg KH wrote: Ok, with linux-next I get the same thing: Thanks Greg. Very nice, thanks for this: Tested-by: Greg Kroah-Hartman gre...@linuxfoundation.org ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 06:40:16PM -0700, Joe Perches wrote: On Fri, 2014-07-11 at 18:34 -0700, Greg KH wrote: On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote: A simple script to run checkpatch --fix for various types of of cleanups. This script is useful primarily for staging. This reformats code to a more CodingStyle conforming style, compiles it, verifies that the object code hasn't changed, and git commits it too. And 'git commits' it? The thought I had was to made it easier to submit my first kernel patch correctly. like that tuxradar article you wrote a few years ago. http://www.tuxradar.com/content/newbies-guide-hacking-linux-kernel Yeah, but it's good for people to actually understand the change they are making, and edit it themselves. Or at least I think so, but I know others don't, so I don't mind this script. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 06:57:24PM -0700, Joe Perches wrote: On Fri, 2014-07-11 at 18:53 -0700, Greg KH wrote: On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote: A simple script to run checkpatch --fix for various types of of cleanups. [] Anyway, try running this script on drivers/staging/lustre/lnet/lnet/acceptor.c to see how this build fails. lustre doesn't use normal kernel makefiles. Ok, I've fixed all of the lustre Makefile crud up and checked it into my staging-next branch. In doing so, it exposed just what a horrid mess the lustre include files are, which is good, hopefully people will notice and start fixing it up. And with the fixes, this script now properly can build and test lustre files. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote:
A simple script to run checkpatch --fix for various types of
of cleanups.
This script is useful primarily for staging.
This reformats code to a more CodingStyle conforming style,
compiles it, verifies that the object code hasn't changed,
and git commits it too.
You must have the necessary development tools, git, and a
recent git tree. Ideally use Greg KH's staging-next, which
can be retrieved via these commands:
git clone git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
git checkout staging-next
To use this script try a sequence of commands like:
cd linux_repository
git checkout -b your_branch
make allyesconfig
mkdir patches
./scripts/reformat_with_checkpatch.sh drivers/staging/dir/*.[ch]
git format-patch --cover-letter -o patches/your_branch staging-next
git send-email patches/your_branch
When running this on drivers/base/bus.c, it says that the .o files are
different, when the diffstat for what makes them different is only
whitespace.
I did the following:
$ scripts/reformat_with_checkpatch.sh drivers/base/bus.c
Ignore the first set of things it tries to commit by answering N to the
Would you like to commit these changes.
Then the second thing it tries to change in the file says that there is
a .o file difference.
Yet the diff is below, I don't see how this happens. Is this due to
there being some old temp file around because I did not accept the first
set of changes?
thanks,
greg k-h
-
diff:
drivers/base/bus.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 83e910a57563..3546d02b46f0 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -32,7 +32,6 @@ static struct kset *system_kset;
#define to_drv_attr(_attr) container_of(_attr, struct driver_attribute, attr)
-
static int __must_check bus_rescan_devices_helper(struct device *dev,
void *data);
@@ -128,6 +127,7 @@ static const struct sysfs_ops bus_sysfs_ops = {
int bus_create_file(struct bus_type *bus, struct bus_attribute *attr)
{
int error;
+
if (bus_get(bus)) {
error = sysfs_create_file(bus-p-subsys.kobj, attr-attr);
bus_put(bus);
@@ -817,6 +817,7 @@ EXPORT_SYMBOL_GPL(device_reprobe);
struct bus_type *find_bus(char *name)
{
struct kobject *k = kset_find_obj(bus_kset, name);
+
return k ? to_bus(k) : NULL;
}
#endif /* 0 */
--
What the script complained about:
Comparing objects...
--- drivers/base/bus.o.new 2014-07-12 01:16:32.984755945 -0700
+++ drivers/base/bus.o.old 2014-07-12 01:16:31.924755967 -0700
@@ -2449,13 +2449,13 @@
descriptor.17493:
...
-: bf 03 00 00 00 mov$0x3,%edi
+: be 03 00 00 00 mov$0x3,%esi
: 00 00 add%al,(%rax)
...
0028 descriptor.17483:
...
-: a2 03 00 00 00 00 00movabs %al,0x3
+: a1 03 00 00 00 00 00movabs 0x3,%eax
: 00 00
0050 descriptor.17406:
@@ -2468,7 +2468,7 @@
0078 descriptor.17073:
...
-: 56 push %rsi
+: 57 push %rdi
: 00 00 add%al,(%rax)
: 00 00 add%al,(%rax)
: 00 00 add%al,(%rax)
Object differences exist! - Verify changes before commit!
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Sat, Jul 12, 2014 at 12:30:43PM +0300, Dan Carpenter wrote: On Fri, Jul 11, 2014 at 06:40:16PM -0700, Joe Perches wrote: On Fri, 2014-07-11 at 18:34 -0700, Greg KH wrote: On Fri, Jul 11, 2014 at 06:21:27PM -0700, Joe Perches wrote: A simple script to run checkpatch --fix for various types of of cleanups. This script is useful primarily for staging. This reformats code to a more CodingStyle conforming style, compiles it, verifies that the object code hasn't changed, and git commits it too. And 'git commits' it? The thought I had was to made it easier to submit my first kernel patch correctly. like that tuxradar article you wrote a few years ago. http://www.tuxradar.com/content/newbies-guide-hacking-linux-kernel Heh, I should just run this myself to clean up staging and beat everyone else to it... At least for the whitespace noise, you could but I hope it'll encourage a few more people to try kernel patching instead. I have always hate the idea of automated patches from random people because I don't trust them to not be malicious so I have to review them manually. There is a story that back in the day the US government paid someone to send tons of checkpatch style patches to BSD. The guy thought he was cleaning up the code but actually he was part of a larger scheme to flood the maintainers with patches so another person could slip in some malicious code. Based on some of the patches that I get sent, I wouldn't be surprised if that's already happening :( It's better if someone just ran this on all new staging code before adding it to the tree. I spent some time messing with this script today, and while it is fun to run, I don't think it's all that useful. One example, before running the script: ~/linux/work/staging $ ./scripts/checkpatch.pl --terse --file drivers/staging/android/binder.c | tail -n 1 total: 0 errors, 103 warnings, 3670 lines checked After running it: ~/linux/work/staging $ ./scripts/checkpatch.pl --terse --file drivers/staging/android/binder.c | tail -n 1 total: 0 errors, 125 warnings, 3670 lines checked And that was after the script created two patches, with the resulting diffstat of: drivers/staging/android/binder.c | 124 +++ 1 file changed, 62 insertions(+), 62 deletions(-) So 2 patches, 60+ lines to review, and 22 more warnings from checkpatch as the end result? Yes, the warnings are all due to line-length, but Joe, you shouldn't add a patch that causes more checkpatch warnings than before :) I know people have scripts like this of their own, and while it might be nice to standardize them, I am really reluctant to put this script in the kernel tree itself. There's a barrier of entry to write your own type of script here that honestly, I like. We already have the example of someone who obviously does not know the C language at all, running through the kernel tree at the moment, creating horrible patches that are flat-out wrong and annoying maintainers with their result. I've had to kill-file them for now, as it was just too annoying and maintainer time is what we have the least of. While I always want to see more developers get involved with kernel development, there should be a minimum barrier to entry. And that barrier is the knowledge of the C language, and knowledge of how to edit a text file, and use git. This script takes that barrier away, for whitespace cleanups, with not much real use overall. So, I'll keep my local copy of this script now, just to have fun with at times when I'm bored. But I don't think it should be merged, as-is. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] reformat_with_checkpatch: Add automation to checkpatch
On Sat, Jul 12, 2014 at 11:29:37AM -0700, Joe Perches wrote: On Sat, 2014-07-12 at 10:55 -0700, Greg KH wrote: Yes, the warnings are all due to line-length, but Joe, you shouldn't add a patch that causes more checkpatch warnings than before :) Yeah, that was intentional though. This script does a series of relatively discrete changes. Lindent would more or less work, but it's _horrible_ at wrapping overlong lines and merges all types of changes together. Oh I agree, I don't want to see Lindent, but maybe, if the patch adds checkpatch warnings, it should be at least flagged as maybe a problem? While I always want to see more developers get involved with kernel development, there should be a minimum barrier to entry. And that barrier is the knowledge of the C language, and knowledge of how to edit a text file, and use git. This script takes that barrier away, for whitespace cleanups, with not much real use overall. So, I'll keep my local copy of this script now, just to have fun with at times when I'm bored. But I don't think it should be merged, as-is. Dunno, I still think it's useful. For you, and me, but the world? Would you want to be on the receiving end of this patch script? I don't, and I'm willing to take almost any patch cleanup for staging code. I think that says something :) Maybe when you get new code, you might run it through a script like this before committing it. I will keep it for me, like I said. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: Cleanup style issues
On Sat, Jul 12, 2014 at 09:55:56PM +0200, Peter Senna Tschudin wrote: This patch fixes the following checkpatch warnings: - Remove else after return - Add space after declaration Tested by compilation only. Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com --- drivers/staging/android/sync_debug.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) This file is no longer in my kernel tree, what tree did you make it against? Please create staging patches against the staging-next branch of the staging.git tree on git.kernel.org so that I can apply them. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: Cleanup style issues
On Sat, Jul 12, 2014 at 01:23:10PM -0700, Greg KH wrote: On Sat, Jul 12, 2014 at 09:55:56PM +0200, Peter Senna Tschudin wrote: This patch fixes the following checkpatch warnings: - Remove else after return - Add space after declaration Tested by compilation only. Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com --- drivers/staging/android/sync_debug.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) This file is no longer in my kernel tree, what tree did you make it against? Please create staging patches against the staging-next branch of the staging.git tree on git.kernel.org so that I can apply them. Ah, nevermind, things moved around here, I figured this out, sorry for the noise... greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[GIT PULL] Staging driver fixes for 3.16-rc5
The following changes since commit cd3de83f147601356395b57a8673e9c5ff1e59d1: Linux 3.16-rc4 (2014-07-06 12:37:51 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/ tags/staging-3.16-rc5 for you to fetch changes up to bdac8ca90e07edde8bbef5396784f7011e6e5b8d: Merge tag 'iio-fixes-for-3.16c' of git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio into staging-linus (2014-07-08 15:43:45 -0700) Staging / IIO fixes for 3.16-rc5 Here are some IIO driver fixes for 3.16-rc5. Nothing major, just resolves some minor issues that have been reported. Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org Greg Kroah-Hartman (1): Merge tag 'iio-fixes-for-3.16c' of git://git.kernel.org/.../jic23/iio into staging-linus Jan Kardell (1): iio: ti_am335x_adc: Fix: Use same step id at FIFOs both ends Peter Meerwald (1): iio:tcs3472: Check for buffer enabled and locking Sachin Kamat (6): iio: hid-sensor-press: Fix return values iio: hid-sensor-accel-3d: Fix return values iio: hid-sensor-magn-3d: Fix return values iio: hid-sensor-als: Fix return values iio: hid-sensor-gyro-3d: Fix return values iio: hid-sensor-prox: Fix return values drivers/iio/accel/hid-sensor-accel-3d.c | 7 ++- drivers/iio/adc/ti_am335x_adc.c | 2 +- drivers/iio/gyro/hid-sensor-gyro-3d.c | 7 ++- drivers/iio/light/hid-sensor-als.c| 7 ++- drivers/iio/light/hid-sensor-prox.c | 7 ++- drivers/iio/light/tcs3472.c | 11 ++- drivers/iio/magnetometer/hid-sensor-magn-3d.c | 7 ++- drivers/iio/pressure/hid-sensor-press.c | 7 ++- 8 files changed, 23 insertions(+), 32 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Anybody working on ced1401?
On Fri, Jun 27, 2014 at 05:11:45PM +0200, Luca Ellero wrote: Hi Greg, On 27/06/2014 16:55, Greg KH wrote: On Fri, Jun 27, 2014 at 03:04:43PM +0200, Luca Ellero wrote: Il 26/06/2014 21:23, Greg KH ha scritto: On Thu, Jun 26, 2014 at 09:36:17AM +0200, Alois Schloegl wrote: On 2014-06-18 13:33, Kristina Martšenko wrote: Hi Alois, I'm helping Greg do a bit of cleanup in the staging tree. I noticed that nobody seems to have worked towards moving ced1401 out of staging in over a year. Are there any plans to clean it up and move it out soon? Because otherwise we're going to have to delete the driver, as we don't want staging to become a permanent place for unfinished code. Thanks, Kristina Hi Kristina, thanks for the notice. Please, give me some time for checking here how we want to go forward with this. What do you mean by this? You all have had a lot of time, with no real progress at all? How about we delete the driver and if you decide to continue to work on it, we can revert that deletion and go from there? thanks, greg k-h Hi, I have a hundred patches for this driver ready to send. Why have you not sent them previously? What is preventing you to send them? Because it's only for a few weeks that I've been working on them. They basically convert all camelCase and Hungarian notation names to Linux convention and fix some checkpatch errors (no logic is modified). Do you have this hardware to test the code with? Do you want to work on getting this driver merged out of the staging tree? I used to have one CED1401, but unfortunately I don't have it any more. Anyway if Alois (or someone else) can test the patches it would be great. I can do a bit more work to get this driver out of staging. As you don't have the hardware anymore, does it make much sense to keep working on the driver? I'd recommend just deleting it, if someone shows up with the hardware later, we can always revert it. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: vt6556: Cleanup coding style issues
On Sun, Jul 13, 2014 at 05:39:55PM +0200, Peter Senna Tschudin wrote: This patch cleanup coding style issues reported by checkpatch. Tested by compilation only. Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com --- Based on latest staging-next. You just sent me 4 patches, all with the same subject (but at least 2 of them had the order in which to apply them in, which is nice.) Please redo these such that they have a unique subject, and I can tell which order to apply them in, as I really have no idea about the second two. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2 1/4] staging: vt6556: Cleanup coding style issues
On Sun, Jul 13, 2014 at 09:11:18PM +0200, Peter Senna Tschudin wrote: This patch cleanup coding style issues reported by checkpatch. Tested by compilation only. Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com --- Cahnges from V1: - Sent all patches in a series Why did you forget the other thing that I asked you to change? I can't take these as-is, sorry, please go back and re-read what I wrote... greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: rtl8821ae: fix coding style issues in cam.c Fix all coding style error and warnings in cam.c reported by checkpatch.pl
On Sun, Jul 13, 2014 at 11:47:57PM +0200, Joerg C. Meyer wrote: Signed-off-by: Joerg C. Meyer jo...@meyer.homedns.org Your changelog body ended up in the Subject: line (that happens if you don't put a blank line after the first line in your git commit. Also, you don't say _what_ issues you fixed here, please be specific. And if you fix more than one type of thing, please break that up into multiple patches, as you are only allowed to do one-thing-per-patch. Care to try this again? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2 1/4] staging: vt6556: Cleanup coding style issues
On Mon, Jul 14, 2014 at 10:59:32AM +0200, Peter Senna Tschudin wrote:
On Sun, Jul 13, 2014 at 12:36:47PM -0700, Greg KH wrote:
On Sun, Jul 13, 2014 at 09:11:18PM +0200, Peter Senna Tschudin wrote:
This patch cleanup coding style issues reported by checkpatch.
Tested by compilation only.
Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com
---
Cahnges from V1:
- Sent all patches in a series
Why did you forget the other thing that I asked you to change?
I can't take these as-is, sorry, please go back and re-read what I
wrote...
Actually I was trying to do what you asked me on this E-mail:
-- // --
You just sent me 4 patches, all with the same subject (but at least 2 of
them had the order in which to apply them in, which is nice.)
Please redo these such that they have a unique subject, and I can tell
which order to apply them in, as I really have no idea about the second
two.
-- // --
I'm sending V3 as a single patch.
That's going to get rejected as well :(
You did put them in a nice order, but you had the same subject text for
each patch, which I told you not to do, right?
{sigh}
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: rtl8821ae: fix coding style issue in cam.c
On Mon, Jul 14, 2014 at 11:42:33AM +0200, Joerg C. Meyer wrote: This is a patch to the cam.c file that fix a coding style error (do not use C99 // comments) Signed-off-by: Joerg C. Meyer jo...@meyer.homedns.org Minor nit, you need a blank line between these two lines... --- drivers/staging/rtl8821ae/cam.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/staging/rtl8821ae/cam.c b/drivers/staging/rtl8821ae/cam.c index 3bc6b3d..9c0cd1e 100644 --- a/drivers/staging/rtl8821ae/cam.c +++ b/drivers/staging/rtl8821ae/cam.c @@ -152,7 +152,7 @@ u8 rtl_cam_add_one_entry(struct ieee80211_hw *hw, u8 *mac_addr, return 1; } -//EXPORT_SYMBOL(rtl_cam_add_one_entry); +/* EXPORT_SYMBOL(rtl_cam_add_one_entry); */ Your patch is correct, but how about just deleting these lines instead? They aren't needed, so please remove them. Can you do that and resend? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: rtl8821ae: fix comment coding style issue in cam.c
On Mon, Jul 14, 2014 at 04:09:26PM +0200, Joerg C. Meyer wrote: This is a patch to the cam.c file that removes some obsolete C99 style comments Signed-off-by: Joerg C. Meyer jo...@meyer.homedns.org 4 copies of the same patch? Anyway, this text doesn't match what the patch actually does (deleted unneeded lines). 3 time's a charm? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2 1/4] staging: vt6556: Cleanup coding style issues
On Mon, Jul 14, 2014 at 07:01:37PM +0200, Peter Senna Tschudin wrote: note I'm not trying to push my changes over the rules. I'm trying to understand the problem, to avoid creating similar noise in the future. /note Now I understand that the problem with the series of 4 patches is that the subject is the same on the 4 patches. Having the same subject in 4 patches is not good. I got this one. But I have no clue why joining 4 cleanup patches into 1 is bad. The patches are all for the same driver, are all silencing checkpatch warnings, and even the typedef stuff was reported by checkpatch. The commit message of the single patch describes it all. If the subject of the series is the problem, why not make a single patch instead of a series of similar patches? It made sense from my perspective. So what is the problem in re-submit 4 similar patches as a single patch? Because it is _much_ harder to review a patch that way. I get a few hundred patches a week to review. If you only do one thing per patch, it is trivial to review, and you don't have to pick through a patch to determine if all of it is correct based on a larger patch, that does multiple things. Also, the rule for a kernel patch is do only one thing. If you do: - remove typedef - fix space layout for a single file, that really is 2 different things, yet you could claim they are both coding style cleanups. Reviewing both of these at the same time, together, makes it much harder to do. Remember, for the kernel, we waste individual developer's time, at the expense of reviewer's time, as we have far more developers than reviewers. Hope this helps explain things more. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 91/93] Staging: comedi: 8253.h fixed by removing 'return' from generic func
On Tue, Jul 15, 2014 at 06:13:19AM +0300, sam-the-6 wrote: From: Sam Asadi asadi.sam...@gmail.com Signed-off-by: Sam Asadi asadi.sam...@gmail.com Where are the other 90 patches in this series, I only received 91, 92, 93, and 94. Wait, 94 out of 93 What is going on here? modified: drivers/staging/comedi/drivers/8253.h What is this line for? Also, please look at your email client's From: line up above, it isn't matching your From: and Signed-off-by: line in the body of the patch, why? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 92/93] Staging: comedi: 8255: fixed by adding an empthy line
On Tue, Jul 15, 2014 at 06:15:44AM +0300, sam-the-6 wrote: From: Sam Asadi asadi.sam...@gmail.com fixed a coding style issue. Your Subject: does not match the body of the patch at all, why not? Please be descriptive as to what you are doing. Also, only do one thing per patch, and one thing is NOT fix all coding style issues. Please redo this whole series, starting with the numbering of them, and break them up into individual changes, not large patches. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 94/94] Staging: comedi: 3 files revised fixed style issues
On Tue, Jul 15, 2014 at 04:19:22PM +0300, sam-the-6 wrote: From: Sam Asadi asadi.sam...@gmail.com 3 files in 'staging/comedi/drivers/' revised again and style issues fixed Signed-off-by: Sam Asadi asadi.sam...@gmail.com modified: drivers/staging/comedi/drivers/8253.h modified: drivers/staging/comedi/drivers/8255.c modified: drivers/staging/comedi/drivers/adl_pci9118.c --- drivers/staging/comedi/drivers/8253.h| 34 +++--- drivers/staging/comedi/drivers/8255.c| 154 +- drivers/staging/comedi/drivers/adl_pci9118.c | 140 +++ 3 files changed, 167 insertions(+), 161 deletions(-) As said before, this isn't an ok way to do this, please break it up into individual changes per patch. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: unisys: added virtpci info entry
On Tue, Jul 15, 2014 at 10:36:43AM -0400, Erik Arfvidson wrote:
This patch adds the virtpci debugfs directory and the info entry
inside of it.
Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com
Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com
---
v4: Fixed comments, upper bound buffer, removed #define for virtpci and info,
modified printvbus to work with scnprintf and added and extra element to
print_vbus_info struct
v3: Fixed formating and comments. Also added debufs_remove_recursive() and
simple simple_read_from_buffer()
v2: Fixed comments and applied Dan Carpenter suggestions
drivers/staging/unisys/virtpci/virtpci.c | 111
++-
1 file changed, 108 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/unisys/virtpci/virtpci.c
b/drivers/staging/unisys/virtpci/virtpci.c
index 7d840b0..1929137 100644
--- a/drivers/staging/unisys/virtpci/virtpci.c
+++ b/drivers/staging/unisys/virtpci/virtpci.c
@@ -36,6 +36,7 @@
#include linux/mod_devicetable.h
#include linux/if_ether.h
#include linux/version.h
+#include linux/debugfs.h
#include version.h
#include guestlinuxdebug.h
#include timskmodutils.h
@@ -57,6 +58,7 @@ struct driver_private {
#endif
#define BUS_ID(x) dev_name(x)
+#define MAX_BUF 16384
Lovely magic number, care to explain why this is this size?
#include virtpci.h
@@ -100,6 +102,12 @@ static int virtpci_device_resume(struct device *dev);
static int virtpci_device_probe(struct device *dev);
static int virtpci_device_remove(struct device *dev);
+static ssize_t info_debugfs_read(struct file *file, char __user *buf,
+ size_t len, loff_t *offset);
+
+static const struct file_operations debugfs_info_fops = {
+ .read = info_debugfs_read,
+};
/*/
/* Globals */
@@ -139,7 +147,17 @@ static DEFINE_RWLOCK(VpcidevListLock);
/* filled in with info about this driver, wrt it servicing client busses */
static ULTRA_VBUS_DEVICEINFO Bus_DriverInfo;
-
+/*/
+/* debugfs entries */
+/*/
+/* dentry is used to create the debugfs entry directory
+ * for virtpci
+ */
+static struct dentry *virtpci_debugfs_dir;
+static struct dentry *info_debugfs_entry;
+/* info_debugfs_entry is used to tell virtpci to display current info
+ * kept in the driver
+ */
Odd to comment both above and below the variables, pick one style and
stick to it.
Also, no empty line ater the Bus_DriverInfo variable and the comment?
And again, you don't need the info_debugfs_entry variable at all. You
set it once and then never touch it again, seems like a waste to have
it, right?
struct virtpci_busdev {
struct device virtpci_bus_device;
@@ -1375,6 +1393,89 @@ void virtpci_unregister_driver(struct virtpci_driver
*drv)
DBGINF(Leaving\n);
}
EXPORT_SYMBOL_GPL(virtpci_unregister_driver);
+/*/
+/* debugfs filesystem functions */
+/*/
Again, no blank line before the comment?
+struct print_vbus_info {
+ int *str_pos;
+ char *buf;
+ size_t *len;
+};
+
+static int print_vbus(struct device *vbus, void *data)
+{
+ struct print_vbus_info *p = (struct print_vbus_info *)data;
+
+ *p-str_pos += scnprintf(p-buf + *p-str_pos, *p-len - *p-str_pos,
+ bus_id:%s\n, dev_name(vbus));
+ return 0;
+}
+
+static ssize_t info_debugfs_read(struct file *file, char __user *buf,
+ size_t len, loff_t *offset)
+{
+ ssize_t bytes_read = 0;
+ int str_pos = 0;
+ struct virtpci_dev *tmpvpcidev;
+ unsigned long flags;
+ struct print_vbus_info printparam;
+ char *vbuf;
+
+ if (len MAX_BUF)
+ len = MAX_BUF;
+ vbuf = kzalloc(len, GFP_KERNEL);
+ if (!vbuf)
+ return -ENOMEM;
+
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ Virtual PCI Bus devices\n);
+ printparam.str_pos = str_pos;
+ printparam.buf = vbuf;
+ printparam.len = len;
+ if (bus_for_each_dev(virtpci_bus_type, NULL,
+ (void *) printparam, print_vbus))
+ LOGERR(Failed to find bus\n);
+
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ \n Virtual PCI devices\n);
+ read_lock_irqsave(VpcidevListLock, flags);
+ tmpvpcidev = VpcidevListHead;
+ while (tmpvpcidev) {
+ if (tmpvpcidev-devtype == VIRTHBA_TYPE) {
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ [%d:%d] VHba:%08x:%08x
Re: [PATCH V4 1/6] staging: vt6556: Cleanup trivial coding style issues
On Mon, Jul 14, 2014 at 09:15:28PM +0200, Peter Senna Tschudin wrote:
This patch cleans up the following checkpatch issues:
- tabs instead of spaces on the beginning of a line
- use correct /* */ comment style
- put { and } on the correct places
- line over 80 chars
- indentation style for multi-line calls / comments
- space after semicolon ,
- new line after declaration
Tested by compilation only.
Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com
---
Cahnges from V3:
- Splitted the patches by change type
That's a lot of things to be doing all in one single patch, why didn't
you split this up further?
Please do so.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V4 4/6] staging: vt6556: Remove typedefs
On Mon, Jul 14, 2014 at 09:15:31PM +0200, Peter Senna Tschudin wrote:
This patch removes uneeded typedefs reported by chackpatch and removes
one enum. The removed enum from card.h:
typedef enum _CARD_PHY_TYPE {
PHY_TYPE_AUTO = 0,
PHY_TYPE_11B,
PHY_TYPE_11G,
PHY_TYPE_11A
} CARD_PHY_TYPE, *PCARD_PHY_TYPE;
Why did you remove this?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V4 6/6] staging: vt6556: Replace printk by pr_warn
On Mon, Jul 14, 2014 at 09:15:33PM +0200, Peter Senna Tschudin wrote: This patch fixes a checkpatch warning by replacing printk by pr_warn. Tested by compilation only. Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com --- Cahnges from V3: - Splitted the patches by change type drivers/staging/vt6656/main_usb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c index cc0281a..64c25e2 100644 --- a/drivers/staging/vt6656/main_usb.c +++ b/drivers/staging/vt6656/main_usb.c @@ -527,7 +527,7 @@ static void usb_device_reset(struct vnt_private *pDevice) status = usb_reset_device(pDevice-usb); if (status) -printk(usb_device_reset fail status=%d\n,status); + pr_warn(usb_device_reset fail status=%d\n, status); Please use dev_warn() instead. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/8 V2] staging: dgap: remove unneeded dgap_err()
On Tue, Jul 15, 2014 at 06:11:44PM +0900, Daeseok Youn wrote: The dgap_err() is printing a message with pr_err(), so all those are replaced. Use definition pr_fmt and then all of dgap: in the beginning of print messages are removed. And also removed out of memory message because the kernel has own message for that. Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- V2: use pr_fmt dgap: prefix on print message on dgap. remove out of memory message. Adds Mark to TO list and CC list for checking send this email properly to him. drivers/staging/dgap/dgap.c | 306 +++ 1 files changed, 133 insertions(+), 173 deletions(-) diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c index 06c55cb..9e750fb 100644 --- a/drivers/staging/dgap/dgap.c +++ b/drivers/staging/dgap/dgap.c @@ -41,6 +41,8 @@ */ #undef DIGI_CONCENTRATORS_SUPPORTED +#define pr_fmt(fmt) dgap: fmt + #include linux/kernel.h #include linux/module.h #include linux/pci.h @@ -153,7 +155,6 @@ static void dgap_firmware_reset_port(struct channel_t *ch); static int dgap_gettok(char **in); static char *dgap_getword(char **in); static int dgap_checknode(struct cnode *p); -static void dgap_err(char *s); /* * Function prototypes from dgap_sysfs.h @@ -815,7 +816,7 @@ static struct board_t *dgap_found_board(struct pci_dev *pdev, int id, if (ret) goto free_brd; - pr_info(dgap: board %d: %s (rev %d), irq %ld\n, + pr_info(board %d: %s (rev %d), irq %ld\n, boardnum, brd-name, brd-rev, brd-irq); Almost all of the pr_*() calls in this driver should be converted over to use dev_*() calls instead. And some of them, like this one, should be removed entirely (no need for a driver to be noisy when a device for it is found, it should be quiet if at all possible, unless something went wrong.) So can you do that here instead? I've applied the earlier patches in this series, and stopped here. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 7/8 RESEND] staging: dgap: introduce dgap_cleanup_nodes()
On Tue, Jul 15, 2014 at 06:14:25PM +0900, Daeseok Youn wrote: When a configration file is parsed with dgap_parsefile(), makes nodes for saving configrations for board. configuration files should not be parsed in the kernel at all. That logic should be removed as it should not be needed. Mark, can you verify that this is not needed with your hardware anymore? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: unisys: added virtpci info entry
On Tue, Jul 15, 2014 at 11:31:03AM -0400, Erik Arfvidson wrote: On 07/15/2014 10:45 AM, Greg KH wrote: [SNIP] +#define MAX_BUF 16384 Lovely magic number, care to explain why this is this size? Assuming we have the maximum possible configuration: 4 busses, 32 devices per bus, and we assume max of 80 characters per linegives us 10,560 bytes which rounds up to 2^14. Then _DOCUMENT IT_. So in 5 years, when you look at the code again, you will remember this is how you came up with that number. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: unisys: added virtpci info entry
On Tue, Jul 15, 2014 at 11:38:16AM -0400, Erik Arfvidson wrote: This patch adds the virtpci debugfs directory and the info entry inside of it. Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com --- v5: Adjusted comments and formatting, remove unused function What unused function? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/94] ARM: shmobile: Add DT and defconfigs to MAINTAINERS
On Tue, Jul 15, 2014 at 07:53:07PM +0300, Sam Asadi wrote: From: Simon Horman horms+rene...@verge.net.au There are a number of DT and defconfig files which are maintained as part of shmobile but have not been listed as such in the MAINTAINERS file. This creates confusion from time to time. Signed-off-by: Simon Horman horms+rene...@verge.net.au Signed-off-by: sam-the-6 asadi.sam...@gmail.com Really? We need a real name here. And why are you passing on Simon's patches, why would I care about this patch to the MAINTAINERS file? confused, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/94] ARM: shmobile: Add DT and defconfigs to MAINTAINERS
On Tue, Jul 15, 2014 at 10:05:30AM -0700, Greg KH wrote:
On Tue, Jul 15, 2014 at 07:53:07PM +0300, Sam Asadi wrote:
From: Simon Horman horms+rene...@verge.net.au
There are a number of DT and defconfig files which
are maintained as part of shmobile but have not been
listed as such in the MAINTAINERS file. This creates
confusion from time to time.
Signed-off-by: Simon Horman horms+rene...@verge.net.au
Signed-off-by: sam-the-6 asadi.sam...@gmail.com
Really? We need a real name here.
And why are you passing on Simon's patches, why would I care about this
patch to the MAINTAINERS file?
Ok, what are you doing here? You just sent 94 patches that are already
committed in the kernel tree. Why in the name of ${DIETY} would you do
that?
What are you trying to do?
confused,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 95/95] Staging: commedi: 8253.h: style issue fixed
Patch 95/95? Really? Where are the other 94 patches in this series? :( Please look at what you are doing here, does it make sense? On Tue, Jul 15, 2014 at 08:04:35PM +0300, Sam Asadi wrote: a revision to the file that previously had several style issues. It's clean now. What does this mean? What did you do in this changeset? Describe _why_ you made the change, not the result of what happened _after_ this patch is applied. Please step back, take a day or two to regroup, and revisit what you are trying to do here. You are not sending out patches correctly at all. Please read some tutorials on how to do this properly (try kernelnewbies.org) and then do a dry-run by sending the patches to yourself. After that looks correct, then consider sending them to the kernel developers. But again, wait, don't rush, there's no deadline here. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192ee: Fix setting highest n rate
On Mon, Jul 14, 2014 at 10:56:32PM -0500, Larry Finger wrote:
Commit 4fb6a37c3f94c1cb4b828bfcc4347771e1628f88 by Andrey Utkin
andrey.krieger.ut...@gmail.com and entitled staging: rtl8192ee:
Correct bitmask in comparsion fixed what appeared to be a typo. After
consultation with the Realtek engineers, merely testing for a 2T2R device
is sufficient to ensure that the TX MCS map will equal 0x0c, thus the second
test can be ignored.
Signed-off-by: Larry Finger larry.fin...@lwfinger.net
---
drivers/staging/rtl8192ee/base.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8192ee/base.c
b/drivers/staging/rtl8192ee/base.c
index 71ed12e..b8b897a 100644
--- a/drivers/staging/rtl8192ee/base.c
+++ b/drivers/staging/rtl8192ee/base.c
@@ -826,8 +826,7 @@ static u8 _rtl_get_vht_highest_n_rate(struct ieee80211_hw
*hw,
u8 hw_rate;
u16 map = le16_to_cpu(sta-vht_cap.vht_mcs.tx_mcs_map);
- if ((get_rf_type(rtlphy) == RF_2T2R)
- (map 0x000c) != 0x000c) {
+ if ((get_rf_type(rtlphy) == RF_2T2R) {
I don't think you test built this change :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/8 V2] staging: dgap: remove unneeded dgap_err()
On Wed, Jul 16, 2014 at 08:21:30AM +0900, DaeSeok Youn wrote: Hi, 2014-07-16 0:29 GMT+09:00 Greg KH gre...@linuxfoundation.org: On Tue, Jul 15, 2014 at 06:11:44PM +0900, Daeseok Youn wrote: The dgap_err() is printing a message with pr_err(), so all those are replaced. Use definition pr_fmt and then all of dgap: in the beginning of print messages are removed. And also removed out of memory message because the kernel has own message for that. Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- V2: use pr_fmt dgap: prefix on print message on dgap. remove out of memory message. Adds Mark to TO list and CC list for checking send this email properly to him. drivers/staging/dgap/dgap.c | 306 +++ 1 files changed, 133 insertions(+), 173 deletions(-) diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c index 06c55cb..9e750fb 100644 --- a/drivers/staging/dgap/dgap.c +++ b/drivers/staging/dgap/dgap.c @@ -41,6 +41,8 @@ */ #undef DIGI_CONCENTRATORS_SUPPORTED +#define pr_fmt(fmt) dgap: fmt + #include linux/kernel.h #include linux/module.h #include linux/pci.h @@ -153,7 +155,6 @@ static void dgap_firmware_reset_port(struct channel_t *ch); static int dgap_gettok(char **in); static char *dgap_getword(char **in); static int dgap_checknode(struct cnode *p); -static void dgap_err(char *s); /* * Function prototypes from dgap_sysfs.h @@ -815,7 +816,7 @@ static struct board_t *dgap_found_board(struct pci_dev *pdev, int id, if (ret) goto free_brd; - pr_info(dgap: board %d: %s (rev %d), irq %ld\n, + pr_info(board %d: %s (rev %d), irq %ld\n, boardnum, brd-name, brd-rev, brd-irq); Almost all of the pr_*() calls in this driver should be converted over to use dev_*() calls instead. And some of them, like this one, should be removed entirely (no need for a driver to be noisy when a device for it is found, it should be quiet if at all possible, unless something went wrong.) So can you do that here instead? I've applied the earlier patches in this series, and stopped here. OK. I can. pr_*() calls are replaced with dev_*() calls. And also removes some of print message which are useless like out of memory Yes, please do that, that would be great. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V4 4/6] staging: vt6556: Remove typedefs
On Tue, Jul 15, 2014 at 10:05:54PM +0200, Peter Senna Tschudin wrote:
On Tue, Jul 15, 2014 at 5:06 PM, Greg KH gre...@linuxfoundation.org wrote:
On Mon, Jul 14, 2014 at 09:15:31PM +0200, Peter Senna Tschudin wrote:
This patch removes uneeded typedefs reported by chackpatch and removes
one enum. The removed enum from card.h:
typedef enum _CARD_PHY_TYPE {
PHY_TYPE_AUTO = 0,
PHY_TYPE_11B,
PHY_TYPE_11G,
PHY_TYPE_11A
} CARD_PHY_TYPE, *PCARD_PHY_TYPE;
Why did you remove this?
Unlike the other two enums this patch change, this one is not in use.
As checkpatch complained about the typedef and it is not currently in
use, I removed it.
Then say that, we want to know _why_ you do something, not _what_ you
do, as it's obvious from the code as to _what_ happens.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 02/10] staging: unisys: add toolaction to sysfs
On Tue, Jul 15, 2014 at 01:30:42PM -0400, Benjamin Romer wrote:
Move the proc entry for controlling the toolaction field to sysfs. The field
appears in /sys/devices/platform/visorchipset/install/toolaction.
This field is used to tell s-Par which type of recovery tool action to perform
on the next guest boot-up. The meaning of the value is dependent on the type
of installation software used to commission the guest.
Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com
---
.../unisys/visorchipset/visorchipset_main.c| 157
-
1 file changed, 60 insertions(+), 97 deletions(-)
All sysfs files need a Documentation/ABI/ entry. As this isn't in the
real part of the kernel yet, just create the entries in the unisys/
subdir and then when it moves out, we can move them to the right place
in the tree.
Can you redo this patch with that entry?
And of course, there are comments below:
--- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
+++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
@@ -150,11 +150,6 @@ static ssize_t proc_read_installer(struct file *file,
char __user *buf,
static ssize_t proc_write_installer(struct file *file,
const char __user *buffer,
size_t count, loff_t *ppos);
-static ssize_t proc_read_toolaction(struct file *file, char __user *buf,
- size_t len, loff_t *offset);
-static ssize_t proc_write_toolaction(struct file *file,
- const char __user *buffer,
- size_t count, loff_t *ppos);
static ssize_t proc_read_bootToTool(struct file *file, char __user *buf,
size_t len, loff_t *offset);
static ssize_t proc_write_bootToTool(struct file *file,
@@ -165,11 +160,6 @@ static const struct file_operations proc_installer_fops
= {
.write = proc_write_installer,
};
-static const struct file_operations proc_toolaction_fops = {
- .read = proc_read_toolaction,
- .write = proc_write_toolaction,
-};
-
static const struct file_operations proc_bootToTool_fops = {
.read = proc_read_bootToTool,
.write = proc_write_bootToTool,
@@ -322,10 +312,36 @@ static VISORCHIPSET_BUSDEV_RESPONDERS BusDev_Responders
= {
/* info for /dev/visorchipset */
static dev_t MajorDev = -1; /** indicates major num for device */
+/* prototypes for attributes */
+static ssize_t show_toolaction(struct device *dev,
+ struct device_attribute *attr, char *buf);
+
+static ssize_t store_toolaction(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count);
+
+static DEVICE_ATTR(toolaction, S_IRUSR | S_IWUSR, show_toolaction,
+ store_toolaction);
+
+static struct attribute *visorchipset_install_attrs[] = {
+ dev_attr_toolaction.attr,
+ NULL
+};
+
+static struct attribute_group visorchipset_install_group = {
+ .name = install,
+ .attrs = visorchipset_install_attrs
+};
+
+static const struct attribute_group *visorchipset_dev_groups[] = {
+ visorchipset_install_group,
+ NULL
+};
+
/* /sys/devices/platform/visorchipset */
static struct platform_device Visorchipset_platform_device = {
.name = visorchipset,
.id = -1,
+ .dev.groups = visorchipset_dev_groups,
};
Why is this a platform device in the first place? Isn't it a real
device in the system on some type of bus?
/* Function prototypes */
@@ -337,6 +353,40 @@ static void
controlvm_respond_physdev_changestate(CONTROLVM_MESSAGE_HEADER *
msgHdr, int response,
ULTRA_SEGMENT_STATE state);
+ssize_t show_toolaction(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ if (ControlVm_channel) {
How can this not be true? I tried to follow the code here, but it's
horrid, why is this assigned in a work queue and not when the module
starts up?
+ U8 toolAction;
+
+ visorchannel_read(ControlVm_channel,
+ offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
+ToolAction), toolAction, sizeof(U8));
+ return scnprintf(buf, PAGE_SIZE, %u\n, toolAction);
+ } else
+ return -ENODEV;
+}
Why would this sysfs file be created if there is not a device? That's
not how sysfs files work, it should only be present if the file has a
value, and if not, it shouldn't be there.
Same goes for your other sysfs files in this patchset, you should never
be return -ENODEV, you just shouldn't have created the file in the first
place. That makes your kernel code simpler, and hopefully, your
userspace code easier too.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
Re: [PATCH V5 6/6] staging: vt6556: Replace printk by dev_warn
On Tue, Jul 15, 2014 at 10:46:49PM +0200, Peter Senna Tschudin wrote: This patch fixes a checkpatch warning by replacing printk by dev_warn. Tested by compilation only. Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com --- Changes from V4: - use dev_warn instead of pr_warn drivers/staging/vt6656/main_usb.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c index 7567646..6708e98 100644 --- a/drivers/staging/vt6656/main_usb.c +++ b/drivers/staging/vt6656/main_usb.c @@ -527,7 +527,8 @@ static void usb_device_reset(struct vnt_private *pDevice) status = usb_reset_device(pDevice-usb); if (status) -printk(usb_device_reset fail status=%d\n,status); + dev_warn(pDevice-usb-dev, + usb_device_reset fail status=%d\n, status); } static void device_free_int_bufs(struct vnt_private *priv) This doesn't apply to my latest tree, can you please refresh it and resend? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/8 V2] staging: dgap: remove unneeded dgap_err()
On Wed, Jul 16, 2014 at 06:26:17PM +0900, DaeSeok Youn wrote: 2014-07-16 8:50 GMT+09:00 Greg KH gre...@linuxfoundation.org: On Wed, Jul 16, 2014 at 08:21:30AM +0900, DaeSeok Youn wrote: Hi, 2014-07-16 0:29 GMT+09:00 Greg KH gre...@linuxfoundation.org: On Tue, Jul 15, 2014 at 06:11:44PM +0900, Daeseok Youn wrote: The dgap_err() is printing a message with pr_err(), so all those are replaced. Use definition pr_fmt and then all of dgap: in the beginning of print messages are removed. And also removed out of memory message because the kernel has own message for that. Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- V2: use pr_fmt dgap: prefix on print message on dgap. remove out of memory message. Adds Mark to TO list and CC list for checking send this email properly to him. drivers/staging/dgap/dgap.c | 306 +++ 1 files changed, 133 insertions(+), 173 deletions(-) diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c index 06c55cb..9e750fb 100644 --- a/drivers/staging/dgap/dgap.c +++ b/drivers/staging/dgap/dgap.c @@ -41,6 +41,8 @@ */ #undef DIGI_CONCENTRATORS_SUPPORTED +#define pr_fmt(fmt) dgap: fmt + #include linux/kernel.h #include linux/module.h #include linux/pci.h @@ -153,7 +155,6 @@ static void dgap_firmware_reset_port(struct channel_t *ch); static int dgap_gettok(char **in); static char *dgap_getword(char **in); static int dgap_checknode(struct cnode *p); -static void dgap_err(char *s); /* * Function prototypes from dgap_sysfs.h @@ -815,7 +816,7 @@ static struct board_t *dgap_found_board(struct pci_dev *pdev, int id, if (ret) goto free_brd; - pr_info(dgap: board %d: %s (rev %d), irq %ld\n, + pr_info(board %d: %s (rev %d), irq %ld\n, boardnum, brd-name, brd-rev, brd-irq); Almost all of the pr_*() calls in this driver should be converted over to use dev_*() calls instead. And some of them, like this one, should be removed entirely (no need for a driver to be noisy when a device for it is found, it should be quiet if at all possible, unless something went wrong.) So can you do that here instead? I've applied the earlier patches in this series, and stopped here. OK. I can. pr_*() calls are replaced with dev_*() calls. And also removes some of print message which are useless like out of memory Yes, please do that, that would be great. I have been working to change pr_*() to dev_*(), but dgap_parse() has no struct device for using dev_*(). If dgap_parse still need for this driver, it need to take a parameter for using dev_*(), it may be pdev but configuration file doesn't need to parse in kernel at all, dgap_parse() will be removed. For now keep the parsing code, and find a device to use, there should be one somewhere, as it is a driver :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: comedi: adl_pci9118: a style issue fixed
On Wed, Jul 16, 2014 at 03:24:12PM +0300, Sam Asadi wrote:
'quoted string split across lines' warning in checkpatching fixed
by group whole string in one line.
Signed-off-by: Sam Asadi asadi.sam...@gmail.com
---
drivers/staging/comedi/drivers/adl_pci9118.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adl_pci9118.c
b/drivers/staging/comedi/drivers/adl_pci9118.c
index 59a65cb..b2d25f5 100644
--- a/drivers/staging/comedi/drivers/adl_pci9118.c
+++ b/drivers/staging/comedi/drivers/adl_pci9118.c
@@ -412,8 +412,7 @@ static int check_channel_list(struct comedi_device *dev,
if ((CR_AREF(chanlist[i]) == AREF_DIFF) !=
(differencial)) {
comedi_error(dev,
- Differencial and single ended
- inputs can't be mixtured!);
+ Differencial and single ended
inputs can't be mixtured!);
return 0;
}
if ((CR_RANGE(chanlist[i]) PCI9118_BIPOLAR_RANGES) !=
--
I get a very odd error when trying to apply this patch with git:
$ git am -s ../s1
Applying: Staging: comedi: adl_pci9118: a style issue fixed
fatal: corrupt patch at line 10
Patch failed at 0001 Staging: comedi: adl_pci9118: a style issue fixed
The copy of the patch that failed is found in:
/home/gregkh/linux/work/staging/.git/rebase-apply/patch
When you have resolved this problem, run git am --continue.
If you prefer to skip this patch, run git am --skip instead.
To restore the original branch and stop patching, run git am --abort.
Can you please fix this up and resend it in a format I can apply it in?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: gdm724x: fix missing blank line after variable declaration
On Wed, Jul 16, 2014 at 04:55:23PM +0530, Kiran Padwal wrote: From: Kiran Padwal kiran.pad...@gmail.com Checkpatch fix - Add missing blank line after variable declaration Signed-off-by:Kiran Padwal kiran.padwa...@gmail.com Minor nit, you need a ' ' after the ':'. Also, you sent me two different patches, with the same Subject: line. So I can't apply either of them, sorry. Please resend both of them with better subject lines (hint, put the filename in the subject: that would fix this...) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/2] Staging: Comedi: adl_pci9118: 2 style issues fixed
On Wed, Jul 16, 2014 at 06:22:58PM +0300, Sam Asadi wrote: 2 style issues fixed: one misspelling a quoted string split across lines. Sam Asadi (2): Staging: comedi: adl_pci9118: a style issue fixed Staging: comedi: adl_pci9118: fractured spelling fixed drivers/staging/comedi/drivers/adl_pci9118.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) These worked, thanks. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 03/32] staging: comedi: comedidev.h: checkpatch.pl cleanup (else after return)
On Wed, Jul 16, 2014 at 10:43:14AM -0700, H Hartley Sweeten wrote: Fix the checkpatch.pl warning in this file: WARNING: else is not generally useful after a break or return Also, for aesthetics, rename the comedi_subdevice parameter from 'subd' to 's' since this is the norm for comedi source files. Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com Cc: Ian Abbott abbo...@mev.co.uk --- drivers/staging/comedi/comedidev.h | 8 1 file changed, 4 insertions(+), 4 deletions(-) Doesn't apply... ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 02/32] staging: comedi: comedi.h: checkpatch.pl cleanup (else after return)
On Wed, Jul 16, 2014 at 10:43:13AM -0700, H Hartley Sweeten wrote: Fix the two checkpatch.pl warnings in this file: WARNING: else is not generally useful after a break or return Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com Cc: Ian Abbott abbo...@mev.co.uk --- drivers/staging/comedi/comedi.h | 8 1 file changed, 4 insertions(+), 4 deletions(-) Also already in my tree... ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/32] staging: comedi: comedi_fops: checkpatch.pl cleanup (else after return)
On Wed, Jul 16, 2014 at 10:43:12AM -0700, H Hartley Sweeten wrote: Fix the two checkpatch.pl warnings in this file: WARNING: else is not generally useful after a break or return Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com Cc: Ian Abbott abbo...@mev.co.uk --- drivers/staging/comedi/comedi_fops.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) This is already in my tree :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: How to replace control code in gdm72xx?
On Wed, Jul 16, 2014 at 09:24:28PM +0100, Michalis Pappas wrote: Hi, I'm currently working on bringing the gdm72xx WiMAX driver out of staging. The driver currently uses two control channels: 1. The SIOCDEVPRIVATE ioctl to send and receive state messages 2. A customly defined netlink protocol for passing messages verbatim to the device controller AFAIK both of the above are deprecated, so I considered switching to the interface defined in wimax.h, which defines a communication protocol over generic netlink that replaces (2) nicely. However it is not compatible with (1), as: * Except from the device status, the gdm72xx driver uses two more types of messages (connection and OMA status), which is not supported by wimax.h. * The gdm driver needs to be able to receive status messages from userspace, which is not supported by wimax.h either. What type of status messages are needed to be sent to the driver? I therefore consider using the wimax stack as defined in wimax.h for the netlink part, but replacing the ioctl with a file under /sys/class/net/wm0/ Is anyone still working on wimax to even object to add new functions like this? :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: gdm72xx: conditionally compile debug code
On Wed, Jul 16, 2014 at 09:40:18PM +0100, Michalis Pappas wrote: On 07/09/2014 07:51 PM, Greg KH wrote: diff --git a/drivers/staging/gdm72xx/gdm_sdio.c b/drivers/staging/gdm72xx/gdm_sdio.c index 9d2de6f..914fd75 100644 --- a/drivers/staging/gdm72xx/gdm_sdio.c +++ b/drivers/staging/gdm72xx/gdm_sdio.c @@ -280,9 +280,11 @@ static void send_sdu(struct sdio_func *func, struct tx_cxt *tx) spin_unlock_irqrestore(tx-lock, flags); + #if defined(GDM72xx_DEBUG) print_hex_dump_debug(sdio_send: , DUMP_PREFIX_NONE, 16, 1, tx-sdu_buf + TYPE_A_HEADER_SIZE, aggr_len - TYPE_A_HEADER_SIZE, false); + #endif This should be moved to use dev_dbg(), along with the other calls to this function in this file. But dev_dbg() gets eventually to be printk(), which cannot print the buffer, so using print_hex_dump_debug() seems to be correct for this case, no? No, you don't want to print the message unless debugging is enabled, and dev_dbg() uses the proper in-kernel dynamic debug infrastructure. There should never be a separate config option for debugging a driver, that ensures that no user will ever be able to help you out with it. So delete the ifdef stuff, and the config option, and just use the proper in-kernel infrastructure for this. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 03/32] staging: comedi: comedidev.h: checkpatch.pl cleanup (else after return)
On Wed, Jul 16, 2014 at 09:27:02PM +, Hartley Sweeten wrote: On Wednesday, July 16, 2014 1:34 PM, Greg KH wrote: On Wed, Jul 16, 2014 at 10:43:14AM -0700, H Hartley Sweeten wrote: Fix the checkpatch.pl warning in this file: WARNING: else is not generally useful after a break or return Also, for aesthetics, rename the comedi_subdevice parameter from 'subd' to 's' since this is the norm for comedi source files. Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com Cc: Ian Abbott abbo...@mev.co.uk --- drivers/staging/comedi/comedidev.h | 8 1 file changed, 4 insertions(+), 4 deletions(-) Doesn't apply... Not a problem. You must have had some other patches that already fixed this and the first 2 in the series. Yes, I think I got them from someone else the other day. I'll make sure to check linux-next to verify that the issues are resolved. Thanks, that would be good. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: gdm72xx: conditionally compile debug code
On Wed, Jul 16, 2014 at 11:03:06PM +0100, Michalis Pappas wrote: On 07/16/2014 09:50 PM, Greg KH wrote: On Wed, Jul 16, 2014 at 09:40:18PM +0100, Michalis Pappas wrote: On 07/09/2014 07:51 PM, Greg KH wrote: diff --git a/drivers/staging/gdm72xx/gdm_sdio.c b/drivers/staging/gdm72xx/gdm_sdio.c index 9d2de6f..914fd75 100644 --- a/drivers/staging/gdm72xx/gdm_sdio.c +++ b/drivers/staging/gdm72xx/gdm_sdio.c @@ -280,9 +280,11 @@ static void send_sdu(struct sdio_func *func, struct tx_cxt *tx) spin_unlock_irqrestore(tx-lock, flags); +#if defined(GDM72xx_DEBUG) print_hex_dump_debug(sdio_send: , DUMP_PREFIX_NONE, 16, 1, tx-sdu_buf + TYPE_A_HEADER_SIZE, aggr_len - TYPE_A_HEADER_SIZE, false); +#endif This should be moved to use dev_dbg(), along with the other calls to this function in this file. But dev_dbg() gets eventually to be printk(), which cannot print the buffer, so using print_hex_dump_debug() seems to be correct for this case, no? No, you don't want to print the message unless debugging is enabled, and dev_dbg() uses the proper in-kernel dynamic debug infrastructure. There should never be a separate config option for debugging a driver, that ensures that no user will ever be able to help you out with it. So delete the ifdef stuff, and the config option, and just use the proper in-kernel infrastructure for this. thanks, greg k-h Ok, I agree on the ifdef stuff. My question was regarding your suggestion above to replace print_hex_debug() with dev_dbg() You want the device name/id/label to show up as well, that is why you should use the dev_dbg() version, print_hex_dump() does not take a struct device *, so the user has no idea what device this data was coming from. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: gdm72xx: conditionally compile debug code
On Wed, Jul 16, 2014 at 03:46:09PM -0700, Joe Perches wrote: On Wed, 2014-07-16 at 15:10 -0700, Greg KH wrote: On Wed, Jul 16, 2014 at 11:03:06PM +0100, Michalis Pappas wrote: [] Ok, I agree on the ifdef stuff. My question was regarding your suggestion above to replace print_hex_debug() with dev_dbg() You want the device name/id/label to show up as well, that is why you should use the dev_dbg() version, print_hex_dump() does not take a struct device *, so the user has no idea what device this data was coming from. But Michalis could alway add something like: dev_hex_dump() and dev_dbg_hex_dump() With the built-in hex dump primitive in printk(), why would you want to do that? You shouldn't be putting more than 64 bytes in a single printk message in a hex dump, if you want to do more, use debugfs. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/5] staging: comedi: cleanup pr_{level} messages
On Thu, Jul 17, 2014 at 12:27:27PM -0700, H Hartley Sweeten wrote:
Where possible, convert all the pr_{level} messages to dev_{level}.
H Hartley Sweeten (5):
staging: comedi: usbduxfast: convert pr_err() to dev_err()
staging: comedi: usbduxfast: convert pr_warn() to dev_warn()
staging: comedi: cb_pcidas64: remove unused pr_fmt() macro
staging: comedi: ni_labpc: tidy up labpc_ai_scan_mode()
staging: comedi: comedi_fops: use pr_fmt()
Thanks for doing this cleanup, and the previous one as well, now
applied.
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] Staging: adl_pci9118: a style issue fixed
On Thu, Jul 17, 2014 at 01:26:23AM +0300, Sam Asadi wrote:
a 'quoted string split across lines' fixed while better use of
English applied to the text.
Signed-off-by: Sam Asadi asadi.sam...@gmail.com
---
drivers/staging/comedi/drivers/adl_pci9118.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adl_pci9118.c
b/drivers/staging/comedi/drivers/adl_pci9118.c
index 93bd9ee..7365f31 100644
--- a/drivers/staging/comedi/drivers/adl_pci9118.c
+++ b/drivers/staging/comedi/drivers/adl_pci9118.c
@@ -418,8 +418,7 @@ static int check_channel_list(struct comedi_device *dev,
if ((CR_RANGE(chanlist[i]) PCI9118_BIPOLAR_RANGES) !=
(bipolar)) {
comedi_error(dev,
- Bipolar and unipolar ranges
- can't be mixtured!);
+ Bipolar and unipolar ranges can't
be mixed!);
return 0;
}
if (!devpriv-usemux differencial
--
1.7.10.4
This patch doesn't apply to my tree due to other changes in this same
file. Can you please refresh it and resend?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: gdm724x: gdm_lte.c: fix missing blank line after variable declaration
On Thu, Jul 17, 2014 at 09:30:38AM +0530, Kiran Padwal wrote:
Checkpatch fix - Add missing blank line after variable declaration
Signed-off-by: Kiran Padwal kiran.padwa...@gmail.com
---
drivers/staging/gdm724x/gdm_lte.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/gdm724x/gdm_lte.c
b/drivers/staging/gdm724x/gdm_lte.c
index 64c55b9..bc6d574 100644
--- a/drivers/staging/gdm724x/gdm_lte.c
+++ b/drivers/staging/gdm724x/gdm_lte.c
@@ -447,6 +447,7 @@ static int gdm_lte_tx(struct sk_buff *skb, struct
net_device *dev)
*/
if (nic_type NIC_TYPE_F_VLAN) {
struct vlan_ethhdr *vlan_eth = (struct vlan_ethhdr *)skb-data;
+
nic-vlan_id = ntohs(vlan_eth-h_vlan_TCI) VLAN_VID_MASK;
data_buf = skb-data + (VLAN_ETH_HLEN - ETH_HLEN);
data_len = skb-len - (VLAN_ETH_HLEN - ETH_HLEN);
@@ -505,6 +506,7 @@ static int gdm_lte_tx(struct sk_buff *skb, struct
net_device *dev)
static struct net_device_stats *gdm_lte_stats(struct net_device *dev)
{
struct nic *nic = netdev_priv(dev);
+
return nic-stats;
}
Someone else sent me this same patch back in May, and it's in my tree.
Please always work against linux-next, or my staging-next branch of my
staging.git tree if you want to send me patches like this.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/25] Replace DEFINE_PCI_DEVICE_TABLE macro use
On Fri, Jul 18, 2014 at 12:22:13PM -0400, John W. Linville wrote: On Fri, Jul 18, 2014 at 05:26:47PM +0200, Benoit Taine wrote: We should prefer `const struct pci_device_id` over `DEFINE_PCI_DEVICE_TABLE` to meet kernel coding style guidelines. This issue was reported by checkpatch. Honestly, I prefer the macro -- it stands-out more. Maybe the style guidelines and/or checkpatch should change instead? The macro is horrid, no other bus has this type of thing just to save a few characters in typing, so why should PCI be special in this regard anymore? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/25] Replace DEFINE_PCI_DEVICE_TABLE macro use
On Fri, Jul 18, 2014 at 09:54:32AM -0700, James Bottomley wrote: On Fri, 2014-07-18 at 09:43 -0700, Greg KH wrote: On Fri, Jul 18, 2014 at 12:22:13PM -0400, John W. Linville wrote: On Fri, Jul 18, 2014 at 05:26:47PM +0200, Benoit Taine wrote: We should prefer `const struct pci_device_id` over `DEFINE_PCI_DEVICE_TABLE` to meet kernel coding style guidelines. This issue was reported by checkpatch. Honestly, I prefer the macro -- it stands-out more. Maybe the style guidelines and/or checkpatch should change instead? The macro is horrid, no other bus has this type of thing just to save a few characters in typing OK, so this is the macro: #define DEFINE_PCI_DEVICE_TABLE(_table) \ const struct pci_device_id _table[] Could you explain what's so horrible? The reason it's useful today is that people forget the const (and sometimes the [] making it a true table instead of a pointer). If you use the DEFINE_PCI_DEVICE_TABLE macro, the compile breaks if you use it wrongly (good) and you automatically get the correct annotations. We have almost 1000 more uses of the non-macro version than the macro version in the kernel today: $ git grep -w DEFINE_PCI_DEVICE_TABLE | wc -l 262 $ git grep const struct pci_device_id | wc -l 1254 My big complaint is that we need to be consistant, either pick one or the other and stick to it. As the macro is the least used, it's easiest to fix up, and it also is more consistant with all other kernel subsystems which do not have such a macro. As there is no need for the __init macro mess anymore, there's no real need for the DEFINE_PCI_DEVICE_TABLE macro either. I think checkpatch will catch the use of non-const users for the id table already today, it catches lots of other uses like this already. , so why should PCI be special in this regard anymore? I think the PCI usage dwarfs most other bus types now, so you could turn the question around. However, I don't think majority voting is a good guide to best practise; lets debate the merits for their own sake. Not really dwarf, USB is close with over 700 such structures: $ git grep const struct usb_device_id | wc -l 725 And i2c is almost just as big as PCI: $ git grep const struct i2c_device_id | wc -l 1223 So again, this macro is not consistent with the majority of PCI drivers, nor with any other type of device id declaration in the kernel, which is why I feel it should be removed. And finally, the PCI documentation itself says to not use this macro, so this isn't a new thing. From Documentation/PCI/pci.txt: The ID table is an array of struct pci_device_id entries ending with an all-zero entry. Definitions with static const are generally preferred. Use of the deprecated macro DEFINE_PCI_DEVICE_TABLE should be avoided. That wording went into the file last December, when we last talked about this and everyone in that discussion agreed to remove the macro for the above reasons. Consistency matters. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/25] Replace DEFINE_PCI_DEVICE_TABLE macro use
On Sat, Jul 19, 2014 at 07:14:12AM +1000, Dave Airlie wrote: We have almost 1000 more uses of the non-macro version than the macro version in the kernel today: $ git grep -w DEFINE_PCI_DEVICE_TABLE | wc -l 262 $ git grep const struct pci_device_id | wc -l 1254 did you check for non-const ones? just to see if we have any of the broken case in the tree :-) I didn't :) as for consistency, pci_dev vs usb_device :-P Read farther down the email... ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/7] staging: comedi: core: checkpatch.pl --strict cleanup
On Fri, Jul 18, 2014 at 02:28:09PM -0700, H Hartley Sweeten wrote: This series fixes most of the checkpatch.pl --strict issues in the comedi core files. comedidev.h still has a couple: CHECK: spinlock_t definition without comment CHECK: struct mutex definition without comment CHECK: Avoid CamelCase: RANGE_mA CHECK: Avoid CamelCase: UNIT_mA CHECK: Avoid CamelCase: range_0_20mA CHECK: Avoid CamelCase: range_4_20mA CHECK: Avoid CamelCase: range_0_32mA I'm hoping Ian Abbot can add some comments about the spinlock_t and mutex definitions. The CamelCase ones are easy to fix but I think 'mA' is easier to read than 'MA' or 'ma'. Yeah, don't worry about that, it's fine. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] Staging: comedi: adl_pci9118: commenting style issue fixed
On Fri, Jul 18, 2014 at 11:13:07PM +0300, Sam Asadi wrote:
A 'quoted string split across lines' issue fixed, while a better use of
language applied to the comment.
Signed-off-by: Sam Asadi asadi.sam...@gmail.com
---
drivers/staging/comedi/drivers/adl_pci9118.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adl_pci9118.c
b/drivers/staging/comedi/drivers/adl_pci9118.c
index 93bd9ee..7365f31 100644
--- a/drivers/staging/comedi/drivers/adl_pci9118.c
+++ b/drivers/staging/comedi/drivers/adl_pci9118.c
@@ -418,8 +418,7 @@ static int check_channel_list(struct comedi_device *dev,
if ((CR_RANGE(chanlist[i]) PCI9118_BIPOLAR_RANGES) !=
(bipolar)) {
comedi_error(dev,
- Bipolar and unipolar ranges
- can't be mixtured!);
+ Bipolar and unipolar ranges can't
be mixed!);
What tree did you refresh this against? My staging-next branch of
staging.git on git.kernel.org does not have comedi_error() anymore in it
:(
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: Fixed missing blank line
On Fri, Jul 18, 2014 at 10:17:54AM +0530, Sanjeev Sharma wrote:
This patch will add an blank line after
declaration reported by checkpatch.pl script.
Signed-off-by: Sanjeev Sharma sanjeev_sha...@mentor.com
---
drivers/staging/android/sw_sync.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/android/sw_sync.c
b/drivers/staging/android/sw_sync.c
index a76db3f..863d4b1 100644
--- a/drivers/staging/android/sw_sync.c
+++ b/drivers/staging/android/sw_sync.c
@@ -97,6 +97,7 @@ static void sw_sync_pt_value_str(struct sync_pt *sync_pt,
char *str, int size)
{
struct sw_sync_pt *pt = (struct sw_sync_pt *)sync_pt;
+
snprintf(str, size, %d, pt-value);
}
@@ -156,6 +157,7 @@ static int sw_sync_open(struct inode *inode, struct file
*file)
static int sw_sync_release(struct inode *inode, struct file *file)
{
struct sw_sync_timeline *obj = file-private_data;
+
sync_timeline_destroy(obj-obj);
return 0;
}
I already applied a previous version of this patch, with your gmail
address :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: vt6655: apply kernel coding style to wCommandTimerWait function
On Fri, Jul 18, 2014 at 09:35:13PM +0300, Igor Bezukh wrote: Since there is a lot of stuff that need to be changed in order to meet the kernel coding style in wcmd.c file, I've decided to fix function-per-patch. The following changes were made in vCommandTimerWait function: - Camel case change: - MSecond --- msec - hDdeviceContext --- private - pDevice --- priv - Removed redundant return - Removed redndant comment In future patches, I will also change the function name itself. A better way to do this, that is easier to review, is to do, on a per-file basis, one thing, like remove redundant return, or remove unneeded comments. Having to review all of these changes at once, even for something as small as a single function, is much harder than making sure you only do one thing, and do it all at once. So, can you break it up in this way instead? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/8] staging: unisys: add toolaction to sysfs
On Sat, Jul 19, 2014 at 10:26:54AM -0400, Benjamin Romer wrote:
Move the proc entry for controlling the toolaction field to sysfs. The field
appears in /sys/devices/platform/visorchipset/install/toolaction.
This field is used to tell s-Par which type of recovery tool action to perform
on the next guest boot-up. The meaning of the value is dependent on the type
of installation software used to commission the guest.
Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com
---
.../unisys/visorchipset/visorchipset_main.c| 151
-
1 file changed, 60 insertions(+), 91 deletions(-)
diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c
b/drivers/staging/unisys/visorchipset/visorchipset_main.c
index a16d67e..f45e352 100644
--- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
+++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
@@ -149,11 +149,6 @@ static ssize_t proc_read_installer(struct file *file,
char __user *buf,
static ssize_t proc_write_installer(struct file *file,
const char __user *buffer,
size_t count, loff_t *ppos);
-static ssize_t proc_read_toolaction(struct file *file, char __user *buf,
- size_t len, loff_t *offset);
-static ssize_t proc_write_toolaction(struct file *file,
- const char __user *buffer,
- size_t count, loff_t *ppos);
static ssize_t proc_read_bootToTool(struct file *file, char __user *buf,
size_t len, loff_t *offset);
static ssize_t proc_write_bootToTool(struct file *file,
@@ -164,11 +159,6 @@ static const struct file_operations proc_installer_fops
= {
.write = proc_write_installer,
};
-static const struct file_operations proc_toolaction_fops = {
- .read = proc_read_toolaction,
- .write = proc_write_toolaction,
-};
-
static const struct file_operations proc_bootToTool_fops = {
.read = proc_read_bootToTool,
.write = proc_write_bootToTool,
@@ -321,10 +311,36 @@ static VISORCHIPSET_BUSDEV_RESPONDERS BusDev_Responders
= {
/* info for /dev/visorchipset */
static dev_t MajorDev = -1; /** indicates major num for device */
+/* prototypes for attributes */
+static ssize_t show_toolaction(struct device *dev,
+ struct device_attribute *attr, char *buf);
+
+static ssize_t store_toolaction(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count);
+
+static DEVICE_ATTR(toolaction, S_IRUSR | S_IWUSR, show_toolaction,
+ store_toolaction);
DEVICE_ATTR_RW() please. Never use the raw DEVICE_ATTR() if at all
possible because it's harder to audit that you really got the
permissions properly.
+
+static struct attribute *visorchipset_install_attrs[] = {
+ dev_attr_toolaction.attr,
+ NULL
+};
+
+static struct attribute_group visorchipset_install_group = {
+ .name = install,
+ .attrs = visorchipset_install_attrs
+};
+
+static const struct attribute_group *visorchipset_dev_groups[] = {
+ visorchipset_install_group,
+ NULL
+};
+
/* /sys/devices/platform/visorchipset */
static struct platform_device Visorchipset_platform_device = {
.name = visorchipset,
.id = -1,
+ .dev.groups = visorchipset_dev_groups,
Only create this device when ControlVm_channel is present in the system,
that should take out your check for it in the show/store function.
Same goes for the rest of these patches.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/8] staging: unisys: visorchipset proc fixes
On Sat, Jul 19, 2014 at 02:26:48PM -0400, Ben Romer wrote: Greg KH gre...@linuxfoundation.org wrote: What happened to the idea of only creating the sysfs files _if_ it is needed? You are always creating these files, and then can return -ENODEV if the device really isn't there, that's not what you should do for a sysfs file. If the file is present, it should return data, not return an error. If the device isn't there, just don't create the file. Greg, I submitted a set of patches before this set that does just that. I moved the controlvm channel function into visorchipset_main.c and removed the old files, and made it so that if the channel is not present the module wouldn't load. I also removed all the code that returns ENODEV, except for the module init function, where it gets returned if there's no controlvm channel present. I could change that to some other error, or let the module load and then not create files if the channel isn't present, if you'd prefer that? But if the module doesn't load, the files in sys don't get created, so I thought that would be a good solution. The commit numbers were 524b0b6 for the controlvm channel function, and 8a1182e for the extraneous checks and ENODEV errors being removed. See my comments in your patch 1/8 for the specifics as to why I didn't know that you have done this work (remember, I deal with hundreds of patches a week and have no short term memory...) greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/8] staging: unisys: add toolaction to sysfs
On Sat, Jul 19, 2014 at 10:26:54AM -0400, Benjamin Romer wrote:
+ssize_t show_toolaction(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ if (ControlVm_channel) {
+ U8 toolAction;
+
+ visorchannel_read(ControlVm_channel,
+ offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
+ToolAction), toolAction, sizeof(U8));
+ return scnprintf(buf, PAGE_SIZE, %u\n, toolAction);
+ } else
+ return -ENODEV;
+}
+
+ssize_t store_toolaction(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ if (ControlVm_channel) {
+ U8 toolAction;
+
+ if (sscanf(buf, %hhu\n, toolAction) == 1) {
+ if (visorchannel_write(ControlVm_channel,
+ offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
+ ToolAction),
+ toolAction, sizeof(U8)) 0)
+ return -EFAULT;
+ else
+ return count;
+ } else
+ return -EIO;
+ } else
+ return -ENODEV;
+}
How could ControlVm_channel ever be NULL? Don't check for this, as the
file shouldn't even be here if ControlVm_channel is null.
Same goes for the other sysfs files in this patch series.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] Staging: comedi: adv_pci1710.c line over 80 fixed
On Sun, Jul 20, 2014 at 04:15:50AM +0300, Sam Asadi wrote: A line over 80 issue fixed, which is a comment. Signed-off-by: Sam Asadi asadi.sam...@gmail.com --- drivers/staging/comedi/drivers/adv_pci1710.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c b/drivers/staging/comedi/drivers/adv_pci1710.c index 602b7a1..13ff78c 100644 --- a/drivers/staging/comedi/drivers/adv_pci1710.c +++ b/drivers/staging/comedi/drivers/adv_pci1710.c @@ -723,7 +723,8 @@ static int pci171x_ai_cancel(struct comedi_device *dev, devpriv-CntrlReg = Control_CNT0; devpriv-CntrlReg |= Control_SW; - outw(devpriv-CntrlReg, dev-iobase + PCI171x_CONTROL); /* reset any operations */ + outw(devpriv-CntrlReg, dev-iobase + PCI171x_CONTROL); + /* reset any operations */ That really doesn't make any sense now, does it? Please put it _before_ the outw() call. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[GIT PULL] Staging driver fixes for 3.16-rc6
The following changes since commit 1795cd9b3a91d4b5473c97f491d63892442212ab: Linux 3.16-rc5 (2014-07-13 14:04:33 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/ tags/staging-3.16-rc6 for you to fetch changes up to 9359003385a2faffa502d201771d45624037a4cd: Merge tag 'iio-fixes-for-3.16d' of git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio into staging-linus (2014-07-13 15:47:26 -0700) Staging fixes for 3.16-rc6 Here are 2 IIO driver fixes for 3.16-rc6 that resolve some reported issues. Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org Greg Kroah-Hartman (1): Merge tag 'iio-fixes-for-3.16d' of git://git.kernel.org/.../jic23/iio into staging-linus Martin Fuzzey (1): iio: mma8452: Use correct acceleration units. Srinivas Pandruvada (1): iio:core: Handle error when mask type is not separate drivers/iio/accel/mma8452.c | 8 +++- drivers/iio/industrialio-event.c | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/8] staging: unisys: add toolaction to sysfs
On Mon, Jul 21, 2014 at 08:36:02AM -0500, Romer, Benjamin M wrote:
On Sat, 2014-07-19 at 09:36 -0700, Greg KH wrote:
/* /sys/devices/platform/visorchipset */
static struct platform_device Visorchipset_platform_device = {
.name = visorchipset,
.id = -1,
+ .dev.groups = visorchipset_dev_groups,
Only create this device when ControlVm_channel is present in the system,
that should take out your check for it in the show/store function.
Same goes for the rest of these patches.
thanks,
greg k-h
I'm not sure I understand. Do you mean that we should only create sysfs
devices dynamically, and not use a static here?
Ideally yes. But that's not what I was referring to.
Only create this sysfs device when ControlVm_channel is created, that
way you will not have to check for it in your sysfs show/store
functions.
I assume that any other sysfs devices in our driver set created this way
would have the same issue?
yes.
thanks
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/29] staging: rtl8723au: Remove bad layer API
On Mon, Jul 21, 2014 at 11:24:40AM +0200, jes.soren...@redhat.com wrote: From: Jes Sorensen jes.soren...@redhat.com Greg, Another set of rtl8723au changes - this was meant to be a 'I'll just fix this little issue' but snowballed into a fair bit more, as I realized just how bad the original code was (patch 10 onwards). The bulk of this set removes a bad layered API. The original driver is trying to moduralize different components (RF handling, baseband handling, 80211 handling etc). They try to create layer independant types they can pass around (which is bad enough by itself), but in this particular case they registered pointers to the original values and derefenced those, losing all type checking in the process and kept multiple defines of the same constants to use in each layer (patch 17). In addition there are a few other remove empty function fixes, and patch 29/29 fixes a locking issue that could be relevant for 3.16. The set is clean against staging-next as of this morning. All applied, thanks. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] staging: sep: Solves some problems reported by checkpatch
On Sat, Jul 19, 2014 at 07:34:39PM +0200, LABBE Corentin wrote: Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com --- drivers/staging/sep/sep_main.c | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) _which_ specific problems are fixed here? Please be specific and provide a valid changelog entry in the body of the email, changelogs without any information other than the subject: are generally frowned apon. Can you please fix this up and resend? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: vt6655: remove redundant comments
On Sun, Jul 20, 2014 at 08:41:50AM +0300, Igor Bezukh wrote: Clean redundant comments in the code. You also messed with some strings :( Which would have been fine, in a separate patch. Please break this up into at least 2 different patches, each only doing one type of thing. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] staging: unisys: remove virthba proc
On Mon, Jul 21, 2014 at 02:47:42PM -0400, Erik Arfvidson wrote: This patch removes all proc entries, directories, and functions A better Subject would be: staging: unisys: remove virthba proc files right? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] staging: unisys: added virthba debugfs dir and info entry
On Mon, Jul 21, 2014 at 02:47:43PM -0400, Erik Arfvidson wrote:
This patch adds virthba debugfs directory and info entry
Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com
Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com
---
drivers/staging/unisys/virthba/virthba.c | 79
1 file changed, 79 insertions(+)
diff --git a/drivers/staging/unisys/virthba/virthba.c
b/drivers/staging/unisys/virthba/virthba.c
index b9cbcf2..121cc05 100644
--- a/drivers/staging/unisys/virthba/virthba.c
+++ b/drivers/staging/unisys/virthba/virthba.c
@@ -50,6 +50,7 @@
#include scsi/scsi_cmnd.h
#include scsi/scsi_device.h
#include asm/param.h
+#include linux/debugfs.h
#include linux/types.h
#include virthba.h
@@ -66,6 +67,11 @@
/* NOTE: L1_CACHE_BYTES =128 */
#define DEVICE_ATTRIBUTE struct device_attribute
+ /* MAX_BUF = 6 lines x 10 MAXVHBA x 80 characters
+ * = 4800 bytes ~ 2^13 = 8192 bytes
+ */
+#define MAX_BUF 8192
+
/*/
/* Forward declarations */
/*/
@@ -104,6 +110,8 @@ static int virthba_serverup(struct virtpci_dev
*virtpcidev);
static int virthba_serverdown(struct virtpci_dev *virtpcidev, u32 state);
static void doDiskAddRemove(struct work_struct *work);
static void virthba_serverdown_complete(struct work_struct *work);
+static ssize_t info_debugfs_read(struct file *file, char __user *buf,
+ size_t len, loff_t *offset);
/*/
/* Globals */
@@ -221,9 +229,18 @@ struct virthba_devices_open {
struct virthba_info *virthbainfo;
};
+static const struct file_operations debugfs_info_fops = {
+ .read = info_debugfs_read,
+};
+
+/*/
+/* Structs */
+/*/
+
#define VIRTHBASOPENMAX 1
/* array of open devices maintained by open() and close(); */
static struct virthba_devices_open VirtHbasOpen[VIRTHBASOPENMAX];
+static struct dentry *virthba_debugfs_dir;
/*/
/* Local Functions*/
@@ -1342,6 +1359,62 @@ process_incoming_rsps(void *v)
complete_and_exit(dc-threadinfo.has_stopped, 0);
}
+/*/
+/* Debugfs filesystem functions */
+/*/
+
+static ssize_t info_debugfs_read(struct file *file,
+ char __user *buf, size_t len, loff_t *offset)
+{
+ ssize_t bytes_read = 0;
+ int str_pos = 0;
+ U64 phys_flags_addr;
+ int i;
+ struct virthba_info *virthbainfo;
+ char *vbuf;
+
+ if (len MAX_BUF)
+ len = MAX_BUF;
+ vbuf = kzalloc(len, GFP_KERNEL);
+ if (!vbuf)
+ return -ENOMEM;
+
+ for (i = 0; i VIRTHBASOPENMAX; i++) {
+ if (VirtHbasOpen[i].virthbainfo == NULL)
+ continue;
+
+ virthbainfo = VirtHbasOpen[i].virthbainfo;
+
+ str_pos += scnprintf(vbuf + str_pos,
+ len - str_pos, MaxBuffLen:%u\n, MaxBuffLen);
+
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ \nvirthba result queue poll wait:%d usecs.\n,
+ rsltq_wait_usecs);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ \ninterrupts_rcvd = %llu, interrupts_disabled
= %llu\n,
+ virthbainfo-interrupts_rcvd,
+ virthbainfo-interrupts_disabled);
+ str_pos += scnprintf(vbuf + str_pos,
+ len - str_pos, \ninterrupts_notme = %llu,\n,
+ virthbainfo-interrupts_notme);
+ phys_flags_addr = virt_to_phys((__force void *)
+virthbainfo-flags_addr);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ flags_addr = %p, phys_flags_addr=0x%016llx,
FeatureFlags=%llu\n,
+ virthbainfo-flags_addr, phys_flags_addr,
+ (__le64)readq(virthbainfo-flags_addr));
+ str_pos += scnprintf(vbuf + str_pos,
+ len - str_pos, acquire_failed_cnt:%llu\n,
+ virthbainfo-acquire_failed_cnt);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos, \n);
+ }
+
+ bytes_read = simple_read_from_buffer(buf, len, offset, vbuf, str_pos);
+ kfree(vbuf);
+ return bytes_read;
+}
+
/* As per VirtpciFunc returns 1
Re: [PATCH 4/4] staging: unisys: added virthba enable_ints entry
On Mon, Jul 21, 2014 at 02:47:45PM -0400, Erik Arfvidson wrote:
This patch adds enable_ints entry to virthba directory
Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com
Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com
---
drivers/staging/unisys/virthba/virthba.c | 63
1 file changed, 63 insertions(+)
diff --git a/drivers/staging/unisys/virthba/virthba.c
b/drivers/staging/unisys/virthba/virthba.c
index 857de11..65a75c8 100644
--- a/drivers/staging/unisys/virthba/virthba.c
+++ b/drivers/staging/unisys/virthba/virthba.c
@@ -112,6 +112,8 @@ static void doDiskAddRemove(struct work_struct *work);
static void virthba_serverdown_complete(struct work_struct *work);
static ssize_t info_debugfs_read(struct file *file, char __user *buf,
size_t len, loff_t *offset);
+static ssize_t enable_ints_write(struct file *file,
+ const char __user *buffer, size_t count, loff_t *ppos);
/*/
/* Globals */
@@ -233,6 +235,10 @@ static const struct file_operations debugfs_info_fops = {
.read = info_debugfs_read,
};
+static const struct file_operations debugfs_enable_ints_fops = {
+ .write = enable_ints_write,
+};
+
/*/
/* Structs */
/*/
@@ -1415,6 +1421,60 @@ static ssize_t info_debugfs_read(struct file *file,
return bytes_read;
}
+static ssize_t enable_ints_write(struct file *file,
+ const char __user *buffer, size_t count, loff_t *ppos)
+{
+ char buf[4];
+ int i, new_value;
+ struct virthba_info *virthbainfo;
+
+ U64 __iomem *Features_addr;
+ U64 mask;
+
+ if (count = ARRAY_SIZE(buf))
+ return -EINVAL;
+
+ buf[count] = '\0';
+ if (copy_from_user(buf, buffer, count)) {
+ LOGERR(copy_from_user failed. buf%.*s count%lu\n,
+(int) count, buf, count);
+ return -EFAULT;
+ }
+
+ i = kstrtoint(buf, 10 , new_value);
+
+ if (i != 0) {
+ LOGERR(Failed to scan value for enable_ints, buf%.*s,
+(int) count, buf);
+ return -EFAULT;
Careful with allowing userspace to flood the log :(
+ }
+
+ /*set all counts to new_value usually 0*/
We don't have a lack of ' ' characters available to us, please use
them...
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: llite: rw.c: use gfp_t to store GFP flags
On Tue, Jul 22, 2014 at 02:57:57AM +0200, Guillaume Morin wrote: Use gfp_t to store GPF_* flags instead of unsigned int. This was reported by sparse. Signed-off-by: Guillaume Morin guilla...@morinfr.org --- drivers/staging/lustre/lustre/llite/rw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/llite/rw.c b/drivers/staging/lustre/lustre/llite/rw.c index 5616210..3554272 100644 --- a/drivers/staging/lustre/lustre/llite/rw.c +++ b/drivers/staging/lustre/lustre/llite/rw.c @@ -496,7 +496,7 @@ static int ll_read_ahead_page(const struct lu_env *env, struct cl_io *io, struct cl_object *clob = ll_i2info(mapping-host)-lli_clob; struct cl_page *page; enum ra_stat which = _NR_RA_STAT; /* keep gcc happy */ - unsigned int gfp_mask; + gfp_t gfp_mask; Odd thing is, that function doesn't seem to even use that variable anywhere... Can you just remove it entirely? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Realtek RTL8821ae driver
On Mon, Jul 21, 2014 at 08:50:26PM -0400, Chris Murphy wrote: Problem: On Asus TX201LA running 15.6 kernel x86_64, driver repeatedly disconnects and reconnects. Dual boot same machine on Win 8.1 is very stable. Behavior starts as soon as Linux is booted and logged in. Saw same behavior on 13.32 and earlier kernel versions. Using the latest bios upgrade. Keywords: RTL8821AE, TX201LA, Asus Trio In addition to the environment info below, I include a copy of dmesg which shows the behavior. Linux version 3.15.6-031506-generic (apw@gomeisa) (gcc version 4.6.3 (Ubuntu/ Linaro 4.6.3-1ubuntu5) ) #201407172034 SMP Fri Jul 18 00:36:12 UTC 2014 This is a horrid driver, people are working on making it better for later kernels by rewriting it entirely. I think that driver will be merged in the 3.17 kernel release, can you look in the linux-next release to see if it is there? Larry, what's the status of your rewrite for this code? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: gdm724x: gdm_lte.c: Fix warning of prefer ether_addr_copy()
On Tue, Jul 22, 2014 at 12:26:42PM +0530, Kiran Padwal wrote: This patch fixes the following checkpatch.pl warnings: WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2). Is that true here? Have you tested this on the hardware to ensure it works properly? I hate that checkpatch message... thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: cxt1e1: Fix static symbol sparse warnings for global vars in linux.c
On Tue, Jul 22, 2014 at 04:07:51PM -0400, Jeff Oczek wrote: Put extern declarations in cxt1e1_common.h to reduce sparse warnings for linux.c: drivers/staging/cxt1e1/linux.c:86:13: warning: symbol 'error_flag' was not declared. Should it be static? drivers/staging/cxt1e1/linux.c:91:13: warning: symbol 'cxt1e1_max_mru' was not declared. Should it be static? drivers/staging/cxt1e1/linux.c:95:13: warning: symbol 'cxt1e1_max_mtu' was not declared. Should it be static? drivers/staging/cxt1e1/linux.c:96:13: warning: symbol 'max_mtu_default' was not declared. Should it be static? drivers/staging/cxt1e1/linux.c:99:13: warning: symbol 'max_txdesc_used' was not declared. Should it be static? drivers/staging/cxt1e1/linux.c:100:13: warning: symbol 'max_txd:esc_default' was not declared. Should it be static? drivers/staging/cxt1e1/linux.c:103:13: warning: symbol 'max_rxdesc_used' was not declared. Should it be static? drivers/staging/cxt1e1/linux.c:104:13: warning: symbol 'max_rxdesc_default' was not declared. Should it be static? drivers/staging/cxt1e1/linux.c:153:1: warning: symbol 'c4_wk_chan_restart' was not declared. Should it be static? drivers/staging/cxt1e1/linux.c:171:1: warning: symbol 'c4_wk_chan_init' was not declared. Should it be static? drivers/staging/cxt1e1/linux.c:186:1: warning: symbol 'c4_wq_port_init' was not declared. Should it be static? drivers/staging/cxt1e1/linux.c:208:1: warning: symbol 'c4_wq_port_cleanup' was not declared. Should it be static? Signed-off-by: Jeff Oczek jeffoc...@gmail.com --- drivers/staging/cxt1e1/cxt1e1_common.h | 15 +++ drivers/staging/cxt1e1/hwprobe.c | 2 +- drivers/staging/cxt1e1/linux.c | 2 +- drivers/staging/cxt1e1/musycc.c| 7 +-- drivers/staging/cxt1e1/pmcc4_drv.c | 5 + 5 files changed, 19 insertions(+), 12 deletions(-) create mode 100644 drivers/staging/cxt1e1/cxt1e1_common.h diff --git a/drivers/staging/cxt1e1/cxt1e1_common.h b/drivers/staging/cxt1e1/cxt1e1_common.h new file mode 100644 index 000..ac6b974 --- /dev/null +++ b/drivers/staging/cxt1e1/cxt1e1_common.h @@ -0,0 +1,15 @@ +#ifndef __CXT1E1_COMMON_H +#define __CXT1E1_COMMON_H + +#include pmcc4.h + +extern int error_flag; I know you didn't name this variable, but wow, that's a horrid name for a global variable :) Any way you could change this to first fix up the name of the variable to something a bit more device-specific first, before this patch? Perhaps cxt1e1_error_flag? +extern int cxt1e1_max_mru; +extern int cxt1e1_max_mtu; These are fine. +extern int max_mtu_default; +extern int max_txdesc_used; +extern int max_txdesc_default; +extern int max_rxdesc_used; +extern int max_rxdesc_default; Again, these are bad names, can you do the same thing here? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: cxt1e1: Fix static symbol sparse warnings for global vars in linux.c
On Tue, Jul 22, 2014 at 04:56:59PM -0400, Jeff Oczek wrote: On Tue, Jul 22, 2014 at 01:17:22PM -0700, Greg KH wrote: On Tue, Jul 22, 2014 at 04:07:51PM -0400, Jeff Oczek wrote: Put extern declarations in cxt1e1_common.h to reduce sparse warnings for linux.c: I know you didn't name this variable, but wow, that's a horrid name for a global variable :) Any way you could change this to first fix up the name of the variable to something a bit more device-specific first, before this patch? Perhaps cxt1e1_error_flag? +extern int cxt1e1_max_mru; +extern int cxt1e1_max_mtu; These are fine. +extern int max_mtu_default; +extern int max_txdesc_used; +extern int max_txdesc_default; +extern int max_rxdesc_used; +extern int max_rxdesc_default; Again, these are bad names, can you do the same thing here? thanks, greg k-h Hi Greg, max_txdesc_used and max_rxdesc_used are module parameters, Ugh :( is it ok to change them? I'm quite new to this -- I don't know if that would count as breaking userspace or not. Yeah, we can't change them, good catch. If not allowed, I could go the route of changing these less descriptive ones to static in the main file and then make an assignment to the global vars during the module init. That would be the best thing to do. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 8/8 v2] staging: unisys: ABI documentation for new sysfs entries
On Tue, Jul 22, 2014 at 09:56:32AM -0400, Benjamin Romer wrote: This patch adds a documentation file for all of the interfaces that were moved to sysfs by the other patches in this set. Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com --- v2: whitespace errors were corrected. .../Documentation/ABI/sysfs-platform-visorchipset | 74 ++ 1 file changed, 74 insertions(+) create mode 100644 drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset diff --git a/drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset b/drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset new file mode 100644 index 000..b6cad24 --- /dev/null +++ b/drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset @@ -0,0 +1,74 @@ +What:install/error +Date:7/18/2014 +KernelVersion: TBD +Contact: sparmaintai...@unisys.com +Description: used to send the ID of a string that should be displayed on + s-Par's automatic installation progress screen when an error + is encountered during installation. This field has no effect + if not in installation mode. +Users: sparmaintai...@unisys.com + +What:install/remainingsteps +Date:7/18/2014 +KernelVersion: TBD +Contact: sparmaintai...@unisys.com +Description: used to set the value of the progress bar on the s-Par automatic + installation progress screen. This field has no effect if not in + installation mode. +Users: sparmaintai...@unisys.com + +What:install/textid +Date:7/18/2014 +KernelVersion: TBD +Contact: sparmaintai...@unisys.com +Description: used to send the ID of a string that should be displayed on + s-Par's automatic installation progress screen. Setting this + field when not in installation mode (boottotool was set on + the previous guest boot) has no effect. +Users: sparmaintai...@unisys.com + +What:install/boottotool +Date:7/18/2014 +KernelVersion: TBD +Contact: sparmaintai...@unisys.com +Description: The boottotool flag controls s-Par behavior on the next boot of + this guest. Setting the flag will cause the guest to boot from + the utility and installation image, which will use the value in + the toolaction field to determine what operation is being + requested. +Users: sparmaintai...@unisys.com + +What:install/toolaction +Date:7/18/2014 +KernelVersion: TBD +Contact: sparmaintai...@unisys.com +Description: This field is used to tell s-Par which type of recovery tool + action to perform on the next guest boot-up. The meaning of the + value is dependent on the type of installation software used to + commission the guest. +Users: sparmaintai...@unisys.com + +What:guest/chipsetready +Date:7/18/2014 +KernelVersion: TBD +Contact: sparmaintai...@unisys.com +Description: This entry is used by Unisys application software on the guest + to acknowledge completion of specific events for integration + purposes, but these acknowledgements are not required for the + guest to operate correctly. +Users: sparmaintai...@unisys.com What is the format of the chipsetready file? It looks like you want to write 2 values to it? Please describe it better, for all of these. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/8 v2] staging: unisys: move chipsetready to sysfs
On Tue, Jul 22, 2014 at 09:56:28AM -0400, Benjamin Romer wrote:
Move the chipsetready proc entry to sysfs under a new directory guest. This
entry is used by Unisys application software on the guest to acknowledge
completion of specific events for integration purposes, but these
acknowledgements are not required for the guest to operate correctly.
The store function is simplified as well, to use scanf() instead of copying
the buffer and using strsep().
Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com
---
v2: attribute creation was fixed and checks for controlvm_channel pointer were
removed. The off-by-one error in the sscanf() was fixed. Error -1 that was
being returned was changed to -EINVAL.
.../unisys/visorchipset/visorchipset_main.c| 88
--
1 file changed, 33 insertions(+), 55 deletions(-)
diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c
b/drivers/staging/unisys/visorchipset/visorchipset_main.c
index a20e21b..74ab15b 100644
--- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
+++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
@@ -129,9 +129,6 @@ static MYPROCTYPE *PartitionType;
#define VISORCHIPSET_DIAG_PROC_ENTRY_FN diagdump
static struct proc_dir_entry *diag_proc_dir;
-#define VISORCHIPSET_CHIPSET_PROC_ENTRY_FN chipsetready
-static struct proc_dir_entry *chipset_proc_dir;
-
#define VISORCHIPSET_PARAHOTPLUG_PROC_ENTRY_FN parahotplug
static struct proc_dir_entry *parahotplug_proc_dir;
@@ -323,6 +320,10 @@ static ssize_t remaining_steps_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count);
static DEVICE_ATTR_RW(remaining_steps);
+static ssize_t chipsetready_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count);
+static DEVICE_ATTR_WO(chipsetready);
+
static struct attribute *visorchipset_install_attrs[] = {
dev_attr_toolaction.attr,
dev_attr_boottotool.attr,
@@ -337,8 +338,19 @@ static struct attribute_group visorchipset_install_group
= {
.attrs = visorchipset_install_attrs
};
+static struct attribute *visorchipset_guest_attrs[] = {
+ dev_attr_chipsetready.attr,
+ NULL
+};
+
+static struct attribute_group visorchipset_guest_group = {
+ .name = guest,
+ .attrs = visorchipset_guest_attrs
+};
+
static const struct attribute_group *visorchipset_dev_groups[] = {
visorchipset_install_group,
+ visorchipset_guest_group,
NULL
};
@@ -2369,49 +2381,27 @@ visorchipset_cache_free(struct kmem_cache *pool, void
*p, char *fn, int ln)
kmem_cache_free(pool, p);
}
-#define gettoken(bufp) strsep(bufp, -\t\n)
-
-static ssize_t
-chipset_proc_write(struct file *file, const char __user *buffer,
-size_t count, loff_t *ppos)
+ssize_t chipsetready_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
Shouldn't this, and your other sysfs file attribute functions, be
static? You said they were above, but not here, so, which does the
compiler decide to use? I think the last one, but please be consistant.
Same goes for all of the patches in this series...
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/8 v2] staging: unisys: move parahotplug to sysfs
On Tue, Jul 22, 2014 at 09:56:29AM -0400, Benjamin Romer wrote:
Move the /proc/visorchipset/parahotplug interface to sysfs under
/sys/devices/platform/visorchipset/guest/parahotplug.
The parahotplug interface is used to deal with recovery situations on s-Par
guest partitions. The command service partition will send a message to a guest
when a device that guest is using needs to be temporarily removed. The message
triggers a udev event that will cause a recovery script to run. When that
script has completed its work, it will write to the parahotplug interface to
send a message back to Command indicating that it is safe to remove the
device.
Moving this interface to sysfs orphans the visorchipset_proc_read_writeonly()
function, so it is also removed.
Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com
---
v2: attribute creation was fixed and checks for controlvm_channel pointer were
removed.
.../unisys/visorchipset/visorchipset_main.c| 59
--
1 file changed, 11 insertions(+), 48 deletions(-)
diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c
b/drivers/staging/unisys/visorchipset/visorchipset_main.c
index 74ab15b..c88f95f 100644
--- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
+++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
@@ -129,19 +129,12 @@ static MYPROCTYPE *PartitionType;
#define VISORCHIPSET_DIAG_PROC_ENTRY_FN diagdump
static struct proc_dir_entry *diag_proc_dir;
-#define VISORCHIPSET_PARAHOTPLUG_PROC_ENTRY_FN parahotplug
-static struct proc_dir_entry *parahotplug_proc_dir;
-
static LIST_HEAD(BusInfoList);
static LIST_HEAD(DevInfoList);
static struct proc_dir_entry *ProcDir;
static VISORCHANNEL *ControlVm_channel;
-static ssize_t visorchipset_proc_read_writeonly(struct file *file,
- char __user *buf,
- size_t len, loff_t *offset);
-
typedef struct {
U8 __iomem *ptr;/* pointer to base address of payload pool */
U64 offset; /* offset from beginning of controlvm
@@ -324,6 +317,10 @@ static ssize_t chipsetready_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count);
static DEVICE_ATTR_WO(chipsetready);
+static ssize_t parahotplug_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count);
+static DEVICE_ATTR_WO(parahotplug);
+
static struct attribute *visorchipset_install_attrs[] = {
dev_attr_toolaction.attr,
dev_attr_boottotool.attr,
@@ -340,6 +337,7 @@ static struct attribute_group visorchipset_install_group
= {
static struct attribute *visorchipset_guest_attrs[] = {
dev_attr_chipsetready.attr,
+ dev_attr_parahotplug.attr,
NULL
};
@@ -1812,30 +1810,17 @@ parahotplug_process_message(CONTROLVM_MESSAGE *inmsg)
/*
* Gets called when the udev script writes to
- * /proc/visorchipset/parahotplug. Expects input in the form of id
- * active where id is the identifier passed to the script that
- * matches a request on the request list, and active is 0 or 1
- * indicating whether the device is now enabled or not.
+ * /sys/devices/platform/visorchipset/guest/parahotplug.
+ * Expects input in the form of id active where id is the identifier
+ * passed to the script that matches a request on the request list, and
active
+ * is 0 or 1 indicating whether the device is now enabled or not.
Why isn't this information in the ABI file?
Also, 2 values for one sysfs file? Not acceptable, sorry.
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: Fixed missing blank line
On Wed, Jul 23, 2014 at 10:08:44AM +, Sharma, Sanjeev wrote: Thanks, so this is also available in next kernel release version. I don't understand the question. You should have gotten an automated email when the patch was applied that explained where the patch was now located, and when it would be merged into Linus's tree. Did you not get that? greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] Staging: comedi: amplc_pc236: a blank line inserted
On Wed, Jul 23, 2014 at 05:06:28AM +0300, Sam Asadi wrote:
A 'Missing a blank line after declarations' warning fixed by inserting
a blank line after struct pointer declaration.
Signed-off-by: Sam Asadi asadi.sam...@gmail.com
---
drivers/staging/comedi/drivers/amplc_pc236.c |1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/comedi/drivers/amplc_pc236.c
b/drivers/staging/comedi/drivers/amplc_pc236.c
index c9a96ad..d0f81f8 100644
--- a/drivers/staging/comedi/drivers/amplc_pc236.c
+++ b/drivers/staging/comedi/drivers/amplc_pc236.c
@@ -515,6 +515,7 @@ static void pc236_detach(struct comedi_device *dev)
comedi_legacy_detach(dev);
} else if (is_pci_board(thisboard)) {
struct pci_dev *pcidev = comedi_to_pci_dev(dev);
+
if (dev-irq)
free_irq(dev-irq, dev);
comedi_pci_disable(dev);
This is already in my tree, sorry :(
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/2] staging: cxt1e1: Prefix ambiguous variable names with 'cxt1e1_' for clarity
On Tue, Jul 22, 2014 at 08:34:55PM -0400, Jeff Oczek wrote: Changed names of ambiguous sounding variable names as follows error_flag - cxt1e1_error_flag max_mtu_default - cxt1e1_max_mtu_default max_txdesc_used - cxt1e1_max_txdesc_used max_txdesc_default - cxt1e1_max_txdesc_default max_rxdesc_used - cxt1e1_max_rxdesc_used max_rxdesc_default - cxt1e1_max_rxdesc_default Since max_txdesc_used, max_rxdesc_used are module parameters, these were changed from global to static and the module init function assigns the values to the newly named global variables Signed-off-by: Jeff Oczek jeffoc...@gmail.com --- drivers/staging/cxt1e1/hwprobe.c | 7 ++--- drivers/staging/cxt1e1/linux.c | 53 +- drivers/staging/cxt1e1/musycc.c| 4 +-- drivers/staging/cxt1e1/pmcc4_drv.c | 22 +--- drivers/staging/cxt1e1/sbeproc.c | 6 ++--- 5 files changed, 51 insertions(+), 41 deletions(-) This driver isn't even in my kernel tree anymore, so how can I apply it? What kernel branch/version did you make it against? Please always work against linux-next, or my staging-next of my staging.git kernel tree when sending patches. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging:bcm:DDRinit.c:coding style:line over 80 char
On Thu, Jul 24, 2014 at 06:19:46PM +0530, Sudip Mukherjee wrote: --- drivers/staging/bcm/DDRInit.c | 59 --- 1 file changed, 39 insertions(+), 20 deletions(-) Hi, This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree. You are receiving this message because of the following common error(s) as indicated below: - Your patch does not have a Signed-off-by: line. Please read the kernel file, Documentation/SubmittingPatches and resend it after adding that line. Note, the line needs to be in the body of the email, before the patch, not at the bottom of the patch or in the email signature. - You did not specify a description of why the patch is needed, or possibly, any description at all, in the email body. Please read the section entitled The canonical patch format in the kernel file, Documentation/SubmittingPatches for what is needed in order to properly describe the change. - You did not write a descriptive Subject: for the patch, allowing Greg, and everyone else, to know what this patch is all about. Please read the section entitled The canonical patch format in the kernel file, Documentation/SubmittingPatches for what a proper Subject: line should look like. If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers. thanks, greg k-h's patch email bot ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/6 v3] staging: unisys: remove partition information from proc
On Thu, Jul 24, 2014 at 02:08:45PM -0400, Benjamin Romer wrote: Debugging information for the guest's channels was being exposed in proc. Remove the code that creates these entries, which are no longer needed. Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com --- v3: patch location changed due to prior patches being revised. v2: patch location changed due to prior patches being revised. Makefile | 2 +- .../unisys/visorchipset/visorchipset_main.c| 99 -- 2 files changed, 1 insertion(+), 100 deletions(-) diff --git a/Makefile b/Makefile index f3c543d..a1c224f 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ VERSION = 3 PATCHLEVEL = 16 SUBLEVEL = 0 -EXTRAVERSION = -rc5 +EXTRAVERSION = -bmr4 NAME = Shuffling Zombie Juror # *DOCUMENTATION* Really ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: vt6655: remove redundant comments from card.h
On Thu, Jul 24, 2014 at 09:50:14PM +0300, Igor Bezukh wrote: Removed redundant comments from card.h header file. Signed-off-by: Igor Bezukh igb...@gmail.com --- drivers/staging/vt6655/80211hdr.h |7 --- drivers/staging/vt6655/80211mgr.h | 38 - drivers/staging/vt6655/aes_ccmp.h |9 - drivers/staging/vt6655/baseband.h | 12 drivers/staging/vt6655/bssdb.h| 25 ++-- drivers/staging/vt6655/card.h | 10 +- 6 files changed, 3 insertions(+), 98 deletions(-) Your subject does not match up with this diffstat :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Anybody working on sep?
On Fri, Jul 25, 2014 at 10:54:47PM +0100, Alan Cox wrote: On Fri, 2014-07-25 at 20:17 +0300, Kristina Martšenko wrote: On 23/06/14 23:32, Kristina Martšenko wrote: Hi Mark, I'm helping Greg do a bit of cleanup in the staging tree. I noticed that nobody seems to have worked towards moving sep out of staging in over a year. Are there any plans to clean it up and move it out soon? No response from Mark here. Alan, Jayant, either of you know what the status of the sep staging driver is? I believe works last time anyone checked. I don't think we have anyone officially working on the early Intel phone cpus for upstream any more. So can we just drop it then? If no one is working on moving it out of staging, it should go. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: unisys: Fix code style
On Fri, Jul 25, 2014 at 03:37:41PM +0530, Arjun AK wrote: From 8e7748dd81cf63c62dbef8f102e97da1d4d5d90a Mon Sep 17 00:00:00 2001 From: Arjun AK arjunak...@gmail.com Date: Fri, 25 Jul 2014 15:09:11 +0530 Subject: [PATCH] Staging: unisys: Fix code style Why is all of this here in this message? Fixes code style in multiple files Please break this up into one logical change per patch, and be specific as to what you change. Signed-off-by: Arjun AK arjunak...@gmail.com I need a full name here, sorry. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: usb driver
On Wed, Mar 05, 2014 at 10:36:53PM -0800, Kalyan Kodamagula wrote: Hi, Iam new to this ML,I need info regarding Usb device drivers.I had learned Lnx device drivers and having knowledge on USB Host driver(highlevel). I had seen probe/read/write functions of a driver.As there is no need to write drivers from scratch,we can gain knowledge by fixing some bugs in the driver. Can some one please give me links/doc to see the bugs and related fixes of USB drivers. Have you looked at drivers/usb/ in the kernel source tree? If you read the git history, there are lots of links and information there about what has been fixed over the years. Good luck, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: usbip: userspace: increase version to 2.0
On Wed, Mar 05, 2014 at 11:30:06PM -0800, Andrew Grover wrote: On Wed, Mar 5, 2014 at 10:14 PM, Valentina Manea valentina.mane...@gmail.com wrote: On Tue, Mar 4, 2014 at 9:20 PM, Greg KH gre...@linuxfoundation.org wrote: On Tue, Mar 04, 2014 at 09:16:39PM +0200, Valentina Manea wrote: -AC_INIT([usbip-utils], [1.1.1], [linux-...@vger.kernel.org]) +AC_INIT([usbip-utils], [2.0], [linux-...@vger.kernel.org]) Why? What does this mean? What warrents the version change? Why have a version at all? This was part of an effort to refresh USB/IP by moving userspace out of kernel.git. Since some major changes have been made (libudev migration), Andy (cc'ed) and me thought it was worth to be promoted to version 2.0. (sorry, resending in plain text mode so vger doesn't bounce (curse you gmail :-)) Valentina did considerable work in moving usbip-utils from using the defunct libsysfs to libudev (well, part of systemd now it seems.) so some version bump seems appropriate, why not to 2.0? esp. as a heads-up to pkg maintainers - btw usbip-utils is already packaged for Debian, and I could probably see it in Fedora too, why not. As to why have a version at all, this is of course tied to whether usbip-utils will ever emerge from the belly of the whale and return to its own home. I think it should someday, if the concerns about long-term maintenance and interface stability can be addressed to your satisfaction. It would be considerable work to integrate it into the kernel build, and would need to be undone if it ever left kernel.git. One big confusion this patch caused, is that it showed up on its own before the libudev rewrite in my inbox. So all I see is this patch incrementing the version for no real reason. Valentina, please always order all of your patches so I know which order in which to apply them in. I see a bunch of follow-on patches after your larger series, what order should they be applied in? Can you just resend _all_ of these, and number them in the correct order in which they need to be applied in, so I know exactly what to do here? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
