Re: [Ksummit-discuss] [PATCH] media: add a subsystem profile documentation

[Joe Perches]

On Thu, 2019-09-26 at 08:53 -0700, Kees Cook wrote:
> On Thu, Sep 26, 2019 at 08:14:03AM -0700, Joe Perches wrote:
> > On Wed, 2019-09-25 at 11:40 -0700, Kees Cook wrote:
> > > Is "6" a safe lower bound here? I thought 12 was the way to go?
> > []
> > > $ git log | egrep 'Fixes: [a-f0-9]{1,40}' | col2 | awk '{print length }' 
> > > | sort | uniq -c | sort -n | tail
> > > 238 8
> > > 300 7
> > > 330 14
> > > 344 6
> > > 352 11
> > > 408 40
> > > 425 10
> > > 735 16
> > >1866 13
> > >   31446 12
> > > 
> > > Hmpf, 6 is pretty high up there...
> > 
> > Yes, but your grep then col2 isn't right.
> > You are counting all the 'Fixes: commit ' output
> > as 6 because that's the length of 'commit'.
> 
> the [a-f0-9]{1,40} already excludes "commit".

No it doesn't as commit starts with c which matches [a-f0-9]{1,40}

Try your original egrep command line yourself.

Maybe use:

$ git log | egrep 'Fixes: [a-f0-9]{1,40}' | awk '{ if (length($2) == 6) { print 
$0;} }'

The first few matches are

the commit referenced in Fixes: below replaced the call to
Fixes: commit 18a992787896 ("ARM: ux500: move soc_id driver to drivers/soc")
Fixes: commit 0580dde59438 ("ASoC: simple-card-utils: add 
asoc_simple_debug_info()")
Since Fixes: 8c5421c016a4 ("perf pmu: Display pmu name when printing
Fixes: commit 961fb3c206dc ("ASoC: rockchip: rk3399_gru_sound: don't select 
unnecessary Platform")

> > I also think the length of the hex commit value doesn't
> > matter much as it's got to be a specific single commit
> > SHA1 anyway, otherwise the commit id lookup will fail.
> 
> Fail enough. We do already have 6-digit SHA1 collisions, so it seemed
> like using more than 6 would be nicer? *shrug* I don't have a strong
> opinion. :)
> 
> > > > > @@ -1031,6 +1040,7 @@ MAINTAINER field selection options:
> > > >  --roles => show roles (status:subsystem, git-signer, list, etc...)
> > > >  --rolestats => show roles and statistics (commits/total_commits, %)
> > > >  --file-emails => add email addresses found in -f file (default: 0 
> > > > (off))
> > > > +--fixes => for patches, add signatures of commits with 'Fixes: 
> > > > ' (default: 1 (on))
> > > 
> > > Should "Tested-by" and "Co-developed-by" get added to @signature_tags ?
> > 
> > All "-by:" signatures are added.
> 
> Ah, I'd missed where that happened. I do note that's only when
> git-all-signature-types is set, which is default 0. (/me goes to add
> this to his invocations...)
> 
> my $email_git_all_signature_types = 0;
> ...
> if ($email_git_all_signature_types) {
> $signature_pattern = "(.+?)[Bb][Yy]:";
> } else {
> $signature_pattern = "\(" . join("|", @signature_tags) . "\)";
> }
> 
> > > @commit_authors is unused?
> > 
> > Yes, authors are already required to sign-off so
> > it's just duplicating already existing signatures.
> 
> Sure, it just seemed odd to populate it if it wasn't going to be used.

It's a generic function.

Re: [Ksummit-discuss] [PATCH] media: add a subsystem profile documentation

[Joe Perches]

On Wed, 2019-09-25 at 11:40 -0700, Kees Cook wrote:
> On Wed, Sep 25, 2019 at 10:13:37AM -0700, Joe Perches wrote:
> > On Thu, 2019-09-19 at 09:56 +0300, Dan Carpenter wrote:
> > > When I sent a patch, I use get_maintainer.pl then I add whoever the
> > > wrote the commit from the Fixes tag.  Then I remove Colin King and Kees
> > > Cook from the CC list because they worked all over the tree and I know
> > > them.  I also normally remove LKML if there is another mailing list but
> > > at least one subsystem uses LKML for patchwork so this isn't safe.
> > > 
> > > So the safest instructions are "Use get_matainer.pl and add the person
> > > who wrote the commit in the Fixes tag".
> > 
> > Maybe add this:
> > 
> > Add the signers of any commit referenced in a "Fixes: " line
> > of a patch description.
> 
> Oh yes please! I've always done this manually, so that's a nice bit of
> automation. :)
> 
> Is "6" a safe lower bound here? I thought 12 was the way to go?
[]
> $ git log | egrep 'Fixes: [a-f0-9]{1,40}' | col2 | awk '{print length }' | 
> sort | uniq -c | sort -n | tail
> 238 8
> 300 7
> 330 14
> 344 6
> 352 11
> 408 40
> 425 10
> 735 16
>1866 13
>   31446 12
> 
> Hmpf, 6 is pretty high up there...

Yes, but your grep then col2 isn't right.
You are counting all the 'Fixes: commit ' output
as 6 because that's the length of 'commit'.

I also think the length of the hex commit value doesn't
matter much as it's got to be a specific single commit
SHA1 anyway, otherwise the commit id lookup will fail.

> > > @@ -1031,6 +1040,7 @@ MAINTAINER field selection options:
> >  --roles => show roles (status:subsystem, git-signer, list, etc...)
> >  --rolestats => show roles and statistics (commits/total_commits, %)
> >  --file-emails => add email addresses found in -f file (default: 0 
> > (off))
> > +--fixes => for patches, add signatures of commits with 'Fixes: 
> > ' (default: 1 (on))
> 
> Should "Tested-by" and "Co-developed-by" get added to @signature_tags ?

All "-by:" signatures are added.

> @commit_authors is unused?

Yes, authors are already required to sign-off so
it's just duplicating already existing signatures.

Re: [Ksummit-discuss] [PATCH] media: add a subsystem profile documentation

[Joe Perches]

On Thu, 2019-09-19 at 09:56 +0300, Dan Carpenter wrote:
> When I sent a patch, I use get_maintainer.pl then I add whoever the
> wrote the commit from the Fixes tag.  Then I remove Colin King and Kees
> Cook from the CC list because they worked all over the tree and I know
> them.  I also normally remove LKML if there is another mailing list but
> at least one subsystem uses LKML for patchwork so this isn't safe.
> 
> So the safest instructions are "Use get_matainer.pl and add the person
> who wrote the commit in the Fixes tag".

Maybe add this:

Add the signers of any commit referenced in a "Fixes: " line
of a patch description.

---
 scripts/get_maintainer.pl | 38 +-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 5ef59214c555..34085d146fa2 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -26,6 +26,7 @@ my $email = 1;
 my $email_usename = 1;
 my $email_maintainer = 1;
 my $email_reviewer = 1;
+my $email_fixes = 1;
 my $email_list = 1;
 my $email_moderated_list = 1;
 my $email_subscriber_list = 0;
@@ -249,6 +250,7 @@ if (!GetOptions(
'r!' => \$email_reviewer,
'n!' => \$email_usename,
'l!' => \$email_list,
+   'fixes!' => \$email_fixes,
'moderated!' => \$email_moderated_list,
's!' => \$email_subscriber_list,
'multiline!' => \$output_multiline,
@@ -503,6 +505,7 @@ sub read_mailmap {
 ## use the filenames on the command line or find the filenames in the 
patchfiles
 
 my @files = ();
+my @fixes = ();# If a patch description includes 
Fixes: lines
 my @range = ();
 my @keyword_tvi = ();
 my @file_emails = ();
@@ -568,6 +571,8 @@ foreach my $file (@ARGV) {
my $filename2 = $2;
push(@files, $filename1);
push(@files, $filename2);
+   } elsif (m/^Fixes:\s+([0-9a-fA-F]{6,40})/) {
+   push(@fixes, $1) if ($email_fixes);
} elsif (m/^\+\+\+\s+(\S+)/ or m/^---\s+(\S+)/) {
my $filename = $1;
$filename =~ s@^[^/]*/@@;
@@ -598,6 +603,7 @@ foreach my $file (@ARGV) {
 }
 
 @file_emails = uniq(@file_emails);
+@fixes = uniq(@fixes);
 
 my %email_hash_name;
 my %email_hash_address;
@@ -612,7 +618,6 @@ my %deduplicate_name_hash = ();
 my %deduplicate_address_hash = ();
 
 my @maintainers = get_maintainers();
-
 if (@maintainers) {
 @maintainers = merge_email(@maintainers);
 output(@maintainers);
@@ -927,6 +932,10 @@ sub get_maintainers {
}
 }
 
+foreach my $fix (@fixes) {
+   vcs_add_commit_signers($fix, "blamed_fixes");
+}
+
 foreach my $email (@email_to, @list_to) {
$email->[0] = deduplicate_email($email->[0]);
 }
@@ -1031,6 +1040,7 @@ MAINTAINER field selection options:
 --roles => show roles (status:subsystem, git-signer, list, etc...)
 --rolestats => show roles and statistics (commits/total_commits, %)
 --file-emails => add email addresses found in -f file (default: 0 (off))
+--fixes => for patches, add signatures of commits with 'Fixes: ' 
(default: 1 (on))
   --scm => print SCM tree(s) if any
   --status => print status if any
   --subsystem => print subsystem name if any
@@ -1730,6 +1740,32 @@ sub vcs_is_hg {
 return $vcs_used == 2;
 }
 
+sub vcs_add_commit_signers {
+return if (!vcs_exists());
+
+my ($commit, $desc) = @_;
+my $commit_count = 0;
+my $commit_authors_ref;
+my $commit_signers_ref;
+my $stats_ref;
+my @commit_authors = ();
+my @commit_signers = ();
+my $cmd;
+
+$cmd = $VCS_cmds{"find_commit_signers_cmd"};
+$cmd =~ s/(\$\w+)/$1/eeg;  #substitute variables in $cmd
+
+($commit_count, $commit_signers_ref, $commit_authors_ref, $stats_ref) = 
vcs_find_signers($cmd, "");
+@commit_authors = @{$commit_authors_ref} if defined $commit_authors_ref;
+@commit_signers = @{$commit_signers_ref} if defined $commit_signers_ref;
+
+foreach my $signer (@commit_signers) {
+   $signer = deduplicate_email($signer);
+}
+
+vcs_assign($desc, 1, @commit_signers);
+}
+
 sub interactive_get_maintainers {
 my ($list_ref) = @_;
 my @list = @$list_ref;

Re: [Ksummit-discuss] [PATCH] media: add a subsystem profile documentation

[Joe Perches]

On Thu, 2019-09-19 at 10:08 +0300, Dan Carpenter wrote:
> On Wed, Sep 18, 2019 at 03:48:31PM -0300, Mauro Carvalho Chehab wrote:
> > Em Wed, 18 Sep 2019 20:27:05 +0300
> > Laurent Pinchart  escreveu:
> > 
> > > > Anyway, not sure if the other sub-maintainers see the same way. From my 
> > > > side,
> > > > I prefer not to be c/c, as this is just more noise, as I just rely on
> > > > patchwork for media patches. What about changing this to:
> > > > 
> > > > Patches for the media subsystem should be sent to the media 
> > > > mailing list
> > > > at linux-media@vger.kernel.org as plain text only e-mail. 
> > > > Emails with
> > > > HTML will be automatically rejected by the mail server. It 
> > > > could be wise 
> > > > to also copy the sub-maintainer(s).  
> > > 
> > > That works for me. As this is really a personal preference, is there a
> > > way it could be encoded in MAINTAINERS in a per-person fashion ?
> > > Something that would allow you to opt-out from CC from linux-media (but
> > > possibly opt-in for other parts of the kernel), and allow me to opt-in
> > > for the drivers I maintain ?
> > 
> > I don't think so. Perhaps we could add, instead, something like that at the
> > sub-maintainers section of the profile.
> 
> Of course there is a way to add yourself as a maintainer for a specific
> .c file...  Maybe people feel like MAINTAINERS is too crowded?
> 
> We could update get_maintainer.pl to grep the .c files for a specific
> tag instead of putting everything in a centralized MAINTAINERS file.

Another option is to split the MAINTAINERS file into multiple
distributed files.  get_maintainer.pl already supports that.

https://lwn.net/Articles/730509/
https://lore.kernel.org/lkml/1501350403.5368.65.ca...@perches.com/

> But it doesn't make sense to try store that information MY BRAIN!  I
> can't remember anything from one minute to the next so I have no idea
> who maintains media submodules...

Nor I.  Nor should I have to.

Re: [PATCH v2 2/6] staging: video: rockchip: add v4l2 decoder

[Joe Perches]

On Thu, 2019-03-07 at 18:03 +0800, Randy Li wrote:
> It is based on the vendor driver sent to mail list before.

trivial notes:

> diff --git a/drivers/staging/rockchip-mpp/mpp_debug.h 
> b/drivers/staging/rockchip-mpp/mpp_debug.h
[]
> +#define mpp_debug_func(type, fmt, args...)   \
> + do {\
> + if (unlikely(debug & type)) {   \
> + pr_info("%s:%d: " fmt,  \
> +  __func__, __LINE__, ##args);   \
> + }   \
> + } while (0)
> +#define mpp_debug(type, fmt, args...)\
> + do {\
> + if (unlikely(debug & type)) {   \
> + pr_info(fmt, ##args);   \
> + }   \
> + } while (0)
> +

It's generally better to emit debug messages at KERN_DEBUG

> +#define mpp_debug_enter()\
> + do {\
> + if (unlikely(debug & DEBUG_FUNCTION)) { \
> + pr_info("%s:%d: enter\n",   \
> +  __func__, __LINE__);   \
> + }   \
> + } while (0)
> +
> +#define mpp_debug_leave()\
> + do {\
> + if (unlikely(debug & DEBUG_FUNCTION)) { \
> + pr_info("%s:%d: leave\n",   \
> +  __func__, __LINE__);   \
> + }   \
> + } while (0)

I suggest removal of these macros and uses.

There's not much value in enter/leave markings as
the generic ftrace facility does this already.

> +
> +#define mpp_err(fmt, args...)\
> + pr_err("%s:%d: " fmt, __func__, __LINE__, ##args)

__func__, __LINE__ markings generally have little value.

Re: [PATCH 3/5] media: sunxi: Add A10 CSI driver

[Joe Perches]

On Tue, 2018-11-13 at 13:24 +0100, Hans Verkuil wrote:
> On 11/13/18 09:24, Maxime Ripard wrote:
> > The older CSI drivers have camera capture interface different from the one
> > in the newer ones.
[]
> > diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h 
> > b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
[]
> > +   // Videobuf2
> 
> Doesn't checkpatch.pl --strict complain about the use of '//'?

No, not since

commit dadf680de3c2eb4cba9840619991eda0cfe98778
Author: Joe Perches 
Date:   Tue Aug 2 14:04:33 2016 -0700

checkpatch: allow c99 style // comments

Sanitise the lines that contain c99 comments so that the error doesn't
get emitted.

Re: [PATCH v11 1/5] venus: firmware: add routine to reset ARM9

[Joe Perches]

On Wed, 2018-10-17 at 11:49 +0300, Stanimir Varbanov wrote:
> On 10/08/2018 04:32 PM, Vikash Garodia wrote:
> > Add routine to reset the ARM9 and brings it out of reset. Also
> > abstract the Venus CPU state handling with a new function. This
> > is in preparation to add PIL functionality in venus driver.
[]
> > diff --git a/drivers/media/platform/qcom/venus/core.h 
> > b/drivers/media/platform/qcom/venus/core.h
[]
> > @@ -129,6 +130,7 @@ struct venus_core {
> > struct device *dev;
> > struct device *dev_dec;
> > struct device *dev_enc;
> > +   bool use_tz;
> 
> could you make it unsigned? For more info please run checkpatch --strict.
> 
> I know that we have structure members of type bool already - that should
> be fixed with follow-up patches, I guess.

That's probably not necessary.

I personally have no issue with bool struct members that
are only used on a transitory basis and not used by hardware
or shared between multiple cpus with different hardware
alignment requirements.

Nothing in this struct is saved or shared.

Perhaps the checkpatch message should be expanded to
enumerate when bool use in a struct is acceptable.

Re: [PATCH] MAINTAINERS: Remove stale file entry for the Atmel ISI driver

[Joe Perches]

On Sun, 2018-09-30 at 06:30 -0300, Mauro Carvalho Chehab wrote:
> Em Sun, 30 Sep 2018 09:54:48 +0300
> Laurent Pinchart  escreveu:
> 
> > include/media/atmel-isi got removed three years ago without the
> > MAINTAINERS file being updated. Remove the stale entry.
> > 
> > Fixes: 40a78f36fc92 ("[media] v4l: atmel-isi: Remove support for platform 
> > data")
> > Reported-by: Joe Perches 
> > Signed-off-by: Laurent Pinchart 
> > ---
> >  MAINTAINERS | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
[]
> > @@ -2497,7 +2497,6 @@ M:Ludovic Desroches 
> > 
> >  L: linux-media@vger.kernel.org
> >  S: Supported
> >  F: drivers/media/platform/atmel/atmel-isi.c
> > -F: include/media/atmel-isi.h
> 
> I guess the right fix would be to replace it by:
> 
> F: drivers/media/platform/atmel/atmel-isi.h

Or replace both F entries with:

F:  drivers/media/platform/atmel/atmel-isi.*

Or combine the 2 MICROCHIP sections into one

MICROCHIP ISC DRIVER
M:  Eugen Hristev 
L:  linux-media@vger.kernel.org
S:  Supported
F:  drivers/media/platform/atmel/atmel-isc.c
F:  drivers/media/platform/atmel/atmel-isc-regs.h
F:  devicetree/bindings/media/atmel-isc.txt

MICROCHIP ISI DRIVER
M:  Eugen Hristev 
L:  linux-media@vger.kernel.org
S:  Supported
F:  drivers/media/platform/atmel/atmel-isi.c
F:  include/media/atmel-isi.h

and maybe use something like:

MICROCHIP MEDIA DRIVERS
M:  Eugen Hristev 
L:  
linux-media@vger.kernel.org
S:  Supported
F:  drivers/media/platform/atmel/
F:  devicetree/bindings/media/atmel-isc.txt

[trivial PATCH V2] treewide: Align function definition open/close braces

[Joe Perches]

Some functions definitions have either the initial open brace and/or
the closing brace outside of column 1.

Move those braces to column 1.

This allows various function analyzers like gnu complexity to work
properly for these modified functions.

Signed-off-by: Joe Perches 
Acked-by: Andy Shevchenko 
Acked-by: Paul Moore 
Acked-by: Alex Deucher 
Acked-by: Dave Chinner 
Reviewed-by: Darrick J. Wong 
Acked-by: Alexandre Belloni 
Acked-by: Martin K. Petersen 
Acked-by: Takashi Iwai 
Acked-by: Mauro Carvalho Chehab 
---

git diff -w still shows no difference.

This patch was sent but December and not applied.

As the trivial maintainer seems not active, it'd be nice if
Andrew Morton picks this up.

V2: Remove fs/xfs/libxfs/xfs_alloc.c as it's updated and remerge the rest

 arch/x86/include/asm/atomic64_32.h   |  2 +-
 drivers/acpi/custom_method.c |  2 +-
 drivers/acpi/fan.c   |  2 +-
 drivers/gpu/drm/amd/display/dc/core/dc.c |  2 +-
 drivers/media/i2c/msp3400-kthreads.c |  2 +-
 drivers/message/fusion/mptsas.c  |  2 +-
 drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c |  2 +-
 drivers/net/wireless/ath/ath9k/xmit.c|  2 +-
 drivers/platform/x86/eeepc-laptop.c  |  2 +-
 drivers/rtc/rtc-ab-b5ze-s3.c |  2 +-
 drivers/scsi/dpt_i2o.c   |  2 +-
 drivers/scsi/sym53c8xx_2/sym_glue.c  |  2 +-
 fs/locks.c   |  2 +-
 fs/ocfs2/stack_user.c|  2 +-
 fs/xfs/xfs_export.c  |  2 +-
 kernel/audit.c   |  6 +++---
 kernel/trace/trace_printk.c  |  4 ++--
 lib/raid6/sse2.c | 14 +++---
 sound/soc/fsl/fsl_dma.c  |  2 +-
 19 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/arch/x86/include/asm/atomic64_32.h 
b/arch/x86/include/asm/atomic64_32.h
index 46e1ef17d92d..92212bf0484f 100644
--- a/arch/x86/include/asm/atomic64_32.h
+++ b/arch/x86/include/asm/atomic64_32.h
@@ -123,7 +123,7 @@ static inline long long arch_atomic64_read(const atomic64_t 
*v)
long long r;
alternative_atomic64(read, "=&A" (r), "c" (v) : "memory");
return r;
- }
+}
 
 /**
  * arch_atomic64_add_return - add and return
diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index b33fba70ec51..a07fbe999eb6 100644
--- a/drivers/acpi/custom_method.c
+++ b/drivers/acpi/custom_method.c
@@ -97,7 +97,7 @@ static void __exit acpi_custom_method_exit(void)
 {
if (cm_dentry)
debugfs_remove(cm_dentry);
- }
+}
 
 module_init(acpi_custom_method_init);
 module_exit(acpi_custom_method_exit);
diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
index 6cf4988206f2..3563103590c6 100644
--- a/drivers/acpi/fan.c
+++ b/drivers/acpi/fan.c
@@ -219,7 +219,7 @@ fan_set_cur_state(struct thermal_cooling_device *cdev, 
unsigned long state)
return fan_set_state_acpi4(device, state);
else
return fan_set_state(device, state);
- }
+}
 
 static const struct thermal_cooling_device_ops fan_cooling_ops = {
.get_max_state = fan_get_max_state,
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 8394d69b963f..e934326a95d3 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -588,7 +588,7 @@ static void disable_dangling_plane(struct dc *dc, struct 
dc_state *context)
  
**/
 
 struct dc *dc_create(const struct dc_init_data *init_params)
- {
+{
struct dc *dc = kzalloc(sizeof(*dc), GFP_KERNEL);
unsigned int full_pipe_count;
 
diff --git a/drivers/media/i2c/msp3400-kthreads.c 
b/drivers/media/i2c/msp3400-kthreads.c
index 4dd01e9f553b..dc6cb8d475b3 100644
--- a/drivers/media/i2c/msp3400-kthreads.c
+++ b/drivers/media/i2c/msp3400-kthreads.c
@@ -885,7 +885,7 @@ static int msp34xxg_modus(struct i2c_client *client)
 }
 
 static void msp34xxg_set_source(struct i2c_client *client, u16 reg, int in)
- {
+{
struct msp_state *state = to_state(i2c_get_clientdata(client));
int source, matrix;
 
diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 439ee9c5f535..231f3a1e27bf 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -2967,7 +2967,7 @@ mptsas_exp_repmanufacture_info(MPT_ADAPTER *ioc,
mutex_unlock(&ioc->sas_mgmt.mutex);
 out:
return ret;
- }
+}
 
 static void
 mptsas_parse_device_info(struct sas_identify *identify,
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c 
b/drivers/net/ethernet

Re: [PATCH][RFC] kernel.h: provide array iterator

[Joe Perches]

On Fri, 2018-03-16 at 16:27 +0100, Rasmus Villemoes wrote:
> On 2018-03-15 11:00, Kieran Bingham wrote:
> > Simplify array iteration with a helper to iterate each entry in an array.
> > Utilise the existing ARRAY_SIZE macro to identify the length of the array
> > and pointer arithmetic to process each item as a for loop.

I recall getting negative feedback on a similar proposal
a decade ago:

https://lkml.org/lkml/2007/2/13/25

Not sure this is different.

Re: [PATCH] media: tw9910: Whitespace alignment

[Joe Perches]

On Thu, 2018-03-01 at 20:02 +0100, jacopo mondi wrote:
> Hi Joe,

Hello Jacopo

> On Thu, Mar 01, 2018 at 03:50:22AM -0800, Joe Perches wrote:
> > Update multiline statements to open parenthesis.
> > Update a ?: to a single line.
> 
> Thanks for the cleanup.
> You may want to rebase this on my series from a few days ago
> 
> https://patchwork.linuxtv.org/patch/47475/

My patch is completely trivial.

I didn't see your patch but presumably Mauro, if he
cares to apply this patch, can handle whatever conflicts
that might exist.

cheers, Joe

[PATCH] media: tw9910: Miscellaneous neatening

[Joe Perches]

Yet more whitespace and style neatening

o Add blank lines before returns
o Reverse a logic test and return early on error
o Move formats to same line as dev_ calls
o Remove an unnecessary period from a logging message

Signed-off-by: Joe Perches 
---
 drivers/media/i2c/tw9910.c | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index ab32cd81ebd0..cc648deb8123 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -752,6 +752,7 @@ static int tw9910_get_selection(struct v4l2_subdev *sd,
sel->r.width= 768;
sel->r.height   = 576;
}
+
return 0;
 }
 
@@ -799,11 +800,13 @@ static int tw9910_s_fmt(struct v4l2_subdev *sd,
mf->colorspace = V4L2_COLORSPACE_SMPTE170M;
 
ret = tw9910_set_frame(sd, &width, &height);
-   if (!ret) {
-   mf->width   = width;
-   mf->height  = height;
-   }
-   return ret;
+   if (ret)
+   return ret;
+
+   mf->width   = width;
+   mf->height  = height;
+
+   return 0;
 }
 
 static int tw9910_set_fmt(struct v4l2_subdev *sd,
@@ -821,7 +824,7 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd,
if (mf->field == V4L2_FIELD_ANY) {
mf->field = V4L2_FIELD_INTERLACED_BT;
} else if (mf->field != V4L2_FIELD_INTERLACED_BT) {
-   dev_err(&client->dev, "Field type %d invalid.\n", mf->field);
+   dev_err(&client->dev, "Field type %d invalid\n", mf->field);
return -EINVAL;
}
 
@@ -840,7 +843,9 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd,
 
if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
return tw9910_s_fmt(sd, mf);
+
cfg->try_fmt = *mf;
+
return 0;
 }
 
@@ -871,21 +876,21 @@ static int tw9910_video_probe(struct i2c_client *client)
id = GET_ID(id);
 
if (id != 0x0b || priv->revision > 0x01) {
-   dev_err(&client->dev,
-   "Product ID error %x:%x\n",
+   dev_err(&client->dev, "Product ID error %x:%x\n",
id, priv->revision);
ret = -ENODEV;
goto done;
}
 
-   dev_info(&client->dev,
-"tw9910 Product ID %0x:%0x\n", id, priv->revision);
+   dev_info(&client->dev, "tw9910 Product ID %0x:%0x\n",
+id, priv->revision);
 
priv->norm = V4L2_STD_NTSC;
priv->scale = &tw9910_ntsc_scales[0];
 
 done:
tw9910_s_power(&priv->subdev, 0);
+
return ret;
 }
 
@@ -905,12 +910,14 @@ static int tw9910_enum_mbus_code(struct v4l2_subdev *sd,
return -EINVAL;
 
code->code = MEDIA_BUS_FMT_UYVY8_2X8;
+
return 0;
 }
 
 static int tw9910_g_tvnorms(struct v4l2_subdev *sd, v4l2_std_id *norm)
 {
*norm = V4L2_STD_NTSC | V4L2_STD_PAL;
+
return 0;
 }
 
-- 
2.15.0

[PATCH] media: tw9910: Whitespace alignment

[Joe Perches]

Update multiline statements to open parenthesis.
Update a ?: to a single line.

Signed-off-by: Joe Perches 
---
 drivers/media/i2c/tw9910.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index cc5d383fc6b8..ab32cd81ebd0 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -445,7 +445,7 @@ static const struct tw9910_scale_ctrl 
*tw9910_select_norm(v4l2_std_id norm,
 
for (i = 0; i < size; i++) {
tmp = abs(width - scale[i].width) +
-   abs(height - scale[i].height);
+ abs(height - scale[i].height);
if (tmp < diff) {
diff = tmp;
ret = scale + i;
@@ -534,9 +534,9 @@ static int tw9910_s_std(struct v4l2_subdev *sd, v4l2_std_id 
norm)
if (!ret)
ret = i2c_smbus_write_byte_data(client, CROP_HI,
((vdelay >> 2) & 0xc0) |
-   ((vact >> 4) & 0x30) |
-   ((hdelay >> 6) & 0x0c) |
-   ((hact >> 8) & 0x03));
+   ((vact >> 4) & 0x30) |
+   ((hdelay >> 6) & 0x0c) |
+   ((hact >> 8) & 0x03));
if (!ret)
ret = i2c_smbus_write_byte_data(client, VDELAY_LO,
vdelay & 0xff);
@@ -642,8 +642,7 @@ static int tw9910_s_power(struct v4l2_subdev *sd, int on)
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct tw9910_priv *priv = to_tw9910(client);
 
-   return on ? tw9910_power_on(priv) :
-   tw9910_power_off(priv);
+   return on ? tw9910_power_on(priv) : tw9910_power_off(priv);
 }
 
 static int tw9910_set_frame(struct v4l2_subdev *sd, u32 *width, u32 *height)
@@ -733,7 +732,7 @@ static int tw9910_set_frame(struct v4l2_subdev *sd, u32 
*width, u32 *height)
 
 static int tw9910_get_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
-   struct v4l2_subdev_selection *sel)
+   struct v4l2_subdev_selection *sel)
 {
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct tw9910_priv *priv = to_tw9910(client);
@@ -758,7 +757,7 @@ static int tw9910_get_selection(struct v4l2_subdev *sd,
 
 static int tw9910_get_fmt(struct v4l2_subdev *sd,
  struct v4l2_subdev_pad_config *cfg,
-   struct v4l2_subdev_format *format)
+ struct v4l2_subdev_format *format)
 {
struct v4l2_mbus_framefmt *mf = &format->format;
struct i2c_client *client = v4l2_get_subdevdata(sd);
@@ -809,7 +808,7 @@ static int tw9910_s_fmt(struct v4l2_subdev *sd,
 
 static int tw9910_set_fmt(struct v4l2_subdev *sd,
  struct v4l2_subdev_pad_config *cfg,
-   struct v4l2_subdev_format *format)
+ struct v4l2_subdev_format *format)
 {
struct v4l2_mbus_framefmt *mf = &format->format;
struct i2c_client *client = v4l2_get_subdevdata(sd);
@@ -900,7 +899,7 @@ static const struct v4l2_subdev_core_ops 
tw9910_subdev_core_ops = {
 
 static int tw9910_enum_mbus_code(struct v4l2_subdev *sd,
 struct v4l2_subdev_pad_config *cfg,
-   struct v4l2_subdev_mbus_code_enum *code)
+struct v4l2_subdev_mbus_code_enum *code)
 {
if (code->pad || code->index)
return -EINVAL;
-- 
2.15.0

usleep_range without a range

[Joe Perches]

scheduling can generally be better when these values are
not identical.  Perhaps these ranges should be expanded.

$ git grep -P -n "usleep_range\s*\(\s*([\w\.\>\-]+)\s*,\s*\1\s*\)"
drivers/clk/ux500/clk-sysctrl.c:45: 
usleep_range(clk->enable_delay_us, clk->enable_delay_us);
drivers/cpufreq/pmac64-cpufreq.c:140:   usleep_range(1000, 1000);
drivers/cpufreq/pmac64-cpufreq.c:239:   usleep_range(1, 1); /* should 
be faster , to fix */
drivers/cpufreq/pmac64-cpufreq.c:284:   usleep_range(500, 500);
drivers/media/i2c/smiapp/smiapp-core.c:1228:usleep_range(1000, 1000);
drivers/media/i2c/smiapp/smiapp-core.c:1235:usleep_range(1000, 1000);
drivers/media/i2c/smiapp/smiapp-core.c:1240:usleep_range(sleep, sleep);
drivers/media/i2c/smiapp/smiapp-core.c:1387:usleep_range(5000, 5000);
drivers/media/i2c/smiapp/smiapp-quirk.c:205:usleep_range(2000, 2000);
drivers/media/i2c/smiapp/smiapp-regs.c:279: usleep_range(2000, 
2000);
drivers/power/supply/ab8500_fg.c:643:   usleep_range(100, 100);
drivers/staging/rtl8192u/r819xU_phy.c:180:  usleep_range(1000, 1000);
drivers/staging/rtl8192u/r819xU_phy.c:736:  
usleep_range(1000, 1000);
drivers/staging/rtl8192u/r819xU_phy.c:740:  
usleep_range(1000, 1000);
sound/soc/codecs/ab8500-codec.c:1065:   
usleep_range(AB8500_ANC_SM_DELAY, AB8500_ANC_SM_DELAY);
sound/soc/codecs/ab8500-codec.c:1068:   
usleep_range(AB8500_ANC_SM_DELAY, AB8500_ANC_SM_DELAY);

Re: [PATCH 1/3] media/ttusb-budget: remove pci_zalloc_coherent abuse

[Joe Perches]

On Tue, 2018-01-09 at 21:39 +0100, Christoph Hellwig wrote:
> Switch to a plain kzalloc instea of pci_zalloc_coherent to allocate
> memory for the USB DMA.
[]
> diff --git a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c 
> b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
[]
> @@ -792,21 +791,15 @@ static void ttusb_free_iso_urbs(struct ttusb *ttusb)
> []
>  static int ttusb_alloc_iso_urbs(struct ttusb *ttusb)
>  {
>   int i;
>  
> - ttusb->iso_buffer = pci_zalloc_consistent(NULL,
> -   ISO_FRAME_SIZE * 
> FRAMES_PER_ISO_BUF * ISO_BUF_COUNT,
> -   &ttusb->iso_dma_handle);
> -
> + ttusb->iso_buffer = kzalloc(ISO_FRAME_SIZE * FRAMES_PER_ISO_BUF *
> + ISO_BUF_COUNT, GFP_KERNEL);
>   if (!ttusb->iso_buffer) {
>   dprintk("%s: pci_alloc_consistent - not enough memory\n",
>   __func__);

This message doesn't make sense anymore and it might as well
be deleted.

And it might be better to use kcalloc

ttusb->iso_buffer = kcalloc(FRAMES_PER_ISO_BUF * ISO_BUF_COUNT,
ISO_FRAME_SIZE, GFP_KERNEL);

[-next PATCH 0/4] sysfs and DEVICE_ATTR_

[Joe Perches]

Joe Perches (4):
  sysfs.h: Use octal permissions
  treewide: Use DEVICE_ATTR_RW
  treewide: Use DEVICE_ATTR_RO
  treewide: Use DEVICE_ATTR_WO

 arch/arm/mach-pxa/sharpsl_pm.c |  4 +-
 arch/s390/kernel/smp.c |  2 +-
 arch/s390/kernel/topology.c|  3 +-
 arch/sh/drivers/push-switch.c  |  2 +-
 arch/tile/kernel/sysfs.c   | 12 ++--
 arch/x86/kernel/cpu/microcode/core.c   |  2 +-
 drivers/acpi/device_sysfs.c|  6 +-
 drivers/char/ipmi/ipmi_msghandler.c| 17 +++---
 drivers/gpu/drm/i915/i915_sysfs.c  | 12 ++--
 drivers/input/touchscreen/elants_i2c.c |  2 +-
 drivers/net/ethernet/ibm/ibmvnic.c |  2 +-
 drivers/net/wimax/i2400m/sysfs.c   |  3 +-
 drivers/nvme/host/core.c   | 10 ++--
 drivers/platform/x86/compal-laptop.c   | 18 ++
 drivers/s390/cio/css.c |  8 +--
 drivers/s390/cio/device.c  | 10 ++--
 drivers/s390/crypto/ap_card.c  |  2 +-
 drivers/scsi/hpsa.c| 10 ++--
 drivers/scsi/lpfc/lpfc_attr.c  | 64 --
 .../staging/media/atomisp/pci/atomisp2/hmm/hmm.c   |  8 +--
 drivers/thermal/thermal_sysfs.c| 17 +++---
 drivers/tty/serial/sh-sci.c|  2 +-
 drivers/usb/host/xhci-dbgcap.c |  2 +-
 drivers/usb/phy/phy-tahvo.c|  2 +-
 drivers/video/fbdev/auo_k190x.c|  4 +-
 drivers/video/fbdev/w100fb.c   |  4 +-
 include/linux/sysfs.h  | 14 ++---
 lib/test_firmware.c| 14 ++---
 lib/test_kmod.c| 14 ++---
 sound/soc/omap/mcbsp.c |  4 +-
 sound/soc/soc-core.c   |  2 +-
 sound/soc/soc-dapm.c   |  2 +-
 32 files changed, 120 insertions(+), 158 deletions(-)

-- 
2.15.0

[-next PATCH 3/4] treewide: Use DEVICE_ATTR_RO

[Joe Perches]

Convert DEVICE_ATTR uses to DEVICE_ATTR_RO where possible.

Done with perl script:

$ git grep -w --name-only DEVICE_ATTR | \
  xargs perl -i -e 'local $/; while (<>) { 
s/\bDEVICE_ATTR\s*\(\s*(\w+)\s*,\s*\(?(?:\s*S_IRUGO\s*|\s*0444\s*)\)?\s*,\s*\1_show\s*,\s*NULL\s*\)/DEVICE_ATTR_RO(\1)/g;
 print;}'

Signed-off-by: Joe Perches 
---
 arch/arm/mach-pxa/sharpsl_pm.c   |  4 ++--
 arch/sh/drivers/push-switch.c|  2 +-
 arch/tile/kernel/sysfs.c | 10 +-
 drivers/acpi/device_sysfs.c  |  6 +++---
 drivers/char/ipmi/ipmi_msghandler.c  | 17 -
 drivers/gpu/drm/i915/i915_sysfs.c|  6 +++---
 drivers/nvme/host/core.c | 10 +-
 drivers/s390/cio/css.c   |  8 
 drivers/s390/cio/device.c|  8 
 drivers/s390/crypto/ap_card.c|  2 +-
 drivers/scsi/hpsa.c  | 10 +-
 drivers/scsi/lpfc/lpfc_attr.c| 18 --
 drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c |  8 
 drivers/thermal/thermal_sysfs.c  |  6 +++---
 sound/soc/soc-core.c |  2 +-
 sound/soc/soc-dapm.c |  2 +-
 16 files changed, 58 insertions(+), 61 deletions(-)

diff --git a/arch/arm/mach-pxa/sharpsl_pm.c b/arch/arm/mach-pxa/sharpsl_pm.c
index 398ba9ba2632..ef9fd9b759cb 100644
--- a/arch/arm/mach-pxa/sharpsl_pm.c
+++ b/arch/arm/mach-pxa/sharpsl_pm.c
@@ -802,8 +802,8 @@ static ssize_t battery_voltage_show(struct device *dev, 
struct device_attribute
return sprintf(buf, "%d\n", sharpsl_pm.battstat.mainbat_voltage);
 }
 
-static DEVICE_ATTR(battery_percentage, 0444, battery_percentage_show, NULL);
-static DEVICE_ATTR(battery_voltage, 0444, battery_voltage_show, NULL);
+static DEVICE_ATTR_RO(battery_percentage);
+static DEVICE_ATTR_RO(battery_voltage);
 
 extern void (*apm_get_power_status)(struct apm_power_info *);
 
diff --git a/arch/sh/drivers/push-switch.c b/arch/sh/drivers/push-switch.c
index a17181160233..762bc5619910 100644
--- a/arch/sh/drivers/push-switch.c
+++ b/arch/sh/drivers/push-switch.c
@@ -24,7 +24,7 @@ static ssize_t switch_show(struct device *dev,
struct push_switch_platform_info *psw_info = dev->platform_data;
return sprintf(buf, "%s\n", psw_info->name);
 }
-static DEVICE_ATTR(switch, S_IRUGO, switch_show, NULL);
+static DEVICE_ATTR_RO(switch);
 
 static void switch_timer(struct timer_list *t)
 {
diff --git a/arch/tile/kernel/sysfs.c b/arch/tile/kernel/sysfs.c
index af5024f0fb5a..b09456a3d77a 100644
--- a/arch/tile/kernel/sysfs.c
+++ b/arch/tile/kernel/sysfs.c
@@ -38,7 +38,7 @@ static ssize_t chip_width_show(struct device *dev,
 {
return sprintf(page, "%u\n", smp_width);
 }
-static DEVICE_ATTR(chip_width, 0444, chip_width_show, NULL);
+static DEVICE_ATTR_RO(chip_width);
 
 static ssize_t chip_height_show(struct device *dev,
struct device_attribute *attr,
@@ -46,7 +46,7 @@ static ssize_t chip_height_show(struct device *dev,
 {
return sprintf(page, "%u\n", smp_height);
 }
-static DEVICE_ATTR(chip_height, 0444, chip_height_show, NULL);
+static DEVICE_ATTR_RO(chip_height);
 
 static ssize_t chip_serial_show(struct device *dev,
struct device_attribute *attr,
@@ -54,7 +54,7 @@ static ssize_t chip_serial_show(struct device *dev,
 {
return get_hv_confstr(page, HV_CONFSTR_CHIP_SERIAL_NUM);
 }
-static DEVICE_ATTR(chip_serial, 0444, chip_serial_show, NULL);
+static DEVICE_ATTR_RO(chip_serial);
 
 static ssize_t chip_revision_show(struct device *dev,
  struct device_attribute *attr,
@@ -62,7 +62,7 @@ static ssize_t chip_revision_show(struct device *dev,
 {
return get_hv_confstr(page, HV_CONFSTR_CHIP_REV);
 }
-static DEVICE_ATTR(chip_revision, 0444, chip_revision_show, NULL);
+static DEVICE_ATTR_RO(chip_revision);
 
 
 static ssize_t type_show(struct device *dev,
@@ -71,7 +71,7 @@ static ssize_t type_show(struct device *dev,
 {
return sprintf(page, "tilera\n");
 }
-static DEVICE_ATTR(type, 0444, type_show, NULL);
+static DEVICE_ATTR_RO(type);
 
 #define HV_CONF_ATTR(name, conf)   \
static ssize_t name ## _show(struct device *dev,\
diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index a041689e5701..545e91420cde 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -357,7 +357,7 @@ static ssize_t real_power_state_show(struct device *dev,
return sprintf(buf, "%s\n", acpi_power_state_string(state));
 }
 
-static DEVICE_ATTR(real_power_state, 0444, real_power_state_show, N

[trivial PATCH] treewide: Align function definition open/close braces

[Joe Perches]

Some functions definitions have either the initial open brace and/or
the closing brace outside of column 1.

Move those braces to column 1.

This allows various function analyzers like gnu complexity to work
properly for these modified functions.

Miscellanea:

o Remove extra trailing ; and blank line from xfs_agf_verify

Signed-off-by: Joe Perches 
---
git diff -w shows no difference other than the above 'Miscellanea'

(this is against -next, but it applies against Linus' tree
 with a couple offsets)

 arch/x86/include/asm/atomic64_32.h   |  2 +-
 drivers/acpi/custom_method.c |  2 +-
 drivers/acpi/fan.c   |  2 +-
 drivers/gpu/drm/amd/display/dc/core/dc.c |  2 +-
 drivers/media/i2c/msp3400-kthreads.c |  2 +-
 drivers/message/fusion/mptsas.c  |  2 +-
 drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c |  2 +-
 drivers/net/wireless/ath/ath9k/xmit.c|  2 +-
 drivers/platform/x86/eeepc-laptop.c  |  2 +-
 drivers/rtc/rtc-ab-b5ze-s3.c |  2 +-
 drivers/scsi/dpt_i2o.c   |  2 +-
 drivers/scsi/sym53c8xx_2/sym_glue.c  |  2 +-
 fs/locks.c   |  2 +-
 fs/ocfs2/stack_user.c|  2 +-
 fs/xfs/libxfs/xfs_alloc.c|  5 ++---
 fs/xfs/xfs_export.c  |  2 +-
 kernel/audit.c   |  6 +++---
 kernel/trace/trace_printk.c  |  4 ++--
 lib/raid6/sse2.c | 14 +++---
 sound/soc/fsl/fsl_dma.c  |  2 +-
 20 files changed, 30 insertions(+), 31 deletions(-)

diff --git a/arch/x86/include/asm/atomic64_32.h 
b/arch/x86/include/asm/atomic64_32.h
index 97c46b8169b7..d4d4883080fa 100644
--- a/arch/x86/include/asm/atomic64_32.h
+++ b/arch/x86/include/asm/atomic64_32.h
@@ -122,7 +122,7 @@ static inline long long atomic64_read(const atomic64_t *v)
long long r;
alternative_atomic64(read, "=&A" (r), "c" (v) : "memory");
return r;
- }
+}
 
 /**
  * atomic64_add_return - add and return
diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index c68e72414a67..e967c1173ba3 100644
--- a/drivers/acpi/custom_method.c
+++ b/drivers/acpi/custom_method.c
@@ -94,7 +94,7 @@ static void __exit acpi_custom_method_exit(void)
 {
if (cm_dentry)
debugfs_remove(cm_dentry);
- }
+}
 
 module_init(acpi_custom_method_init);
 module_exit(acpi_custom_method_exit);
diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
index 6cf4988206f2..3563103590c6 100644
--- a/drivers/acpi/fan.c
+++ b/drivers/acpi/fan.c
@@ -219,7 +219,7 @@ fan_set_cur_state(struct thermal_cooling_device *cdev, 
unsigned long state)
return fan_set_state_acpi4(device, state);
else
return fan_set_state(device, state);
- }
+}
 
 static const struct thermal_cooling_device_ops fan_cooling_ops = {
.get_max_state = fan_get_max_state,
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index d1488d5ee028..1e0d1e7c5324 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -461,7 +461,7 @@ static void disable_dangling_plane(struct dc *dc, struct 
dc_state *context)
  
**/
 
 struct dc *dc_create(const struct dc_init_data *init_params)
- {
+{
struct dc *dc = kzalloc(sizeof(*dc), GFP_KERNEL);
unsigned int full_pipe_count;
 
diff --git a/drivers/media/i2c/msp3400-kthreads.c 
b/drivers/media/i2c/msp3400-kthreads.c
index 4dd01e9f553b..dc6cb8d475b3 100644
--- a/drivers/media/i2c/msp3400-kthreads.c
+++ b/drivers/media/i2c/msp3400-kthreads.c
@@ -885,7 +885,7 @@ static int msp34xxg_modus(struct i2c_client *client)
 }
 
 static void msp34xxg_set_source(struct i2c_client *client, u16 reg, int in)
- {
+{
struct msp_state *state = to_state(i2c_get_clientdata(client));
int source, matrix;
 
diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 345f6035599e..69a62d23514b 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -2968,7 +2968,7 @@ mptsas_exp_repmanufacture_info(MPT_ADAPTER *ioc,
mutex_unlock(&ioc->sas_mgmt.mutex);
 out:
return ret;
- }
+}
 
 static void
 mptsas_parse_device_info(struct sas_identify *identify,
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c 
b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
index 3dd973475125..0ea141ece19e 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
@@ -603,7 +603,7 @@ 

Re: [PATCH] media: v4l: xilinx: Use SPDX-License-Identifier

[Joe Perches]

On Thu, 2017-12-14 at 20:37 +0200, Laurent Pinchart wrote:
> Hi Joe,

Hi Laurent.

> On Thursday, 14 December 2017 20:32:20 EET Joe Perches wrote:
> > Adding a comment line that describes an implicit or
> > explicit license is different than removing the license
> > text itself.
> 
> The SPDX license header is meant to be equivalent to the license text.

I understand that.
At a minimum, removing BSD license text is undesirable
as that license states:

 ** Redistributions of source code must retain the above copyright
 *  notice, this list of conditions and the following disclaimer.
etc...

> The only reason why the large SPDX patch didn't touch the whole kernel in one 
> go 
> was that it was easier to split in in multiple chunks.

Not really, it was scripted.

> This is no different 
> than not including the full GPL license in every header file but only 
> pointing 
> to it through its name and reference, as every kernel source file does.

Not every kernel source file had a license text
or a reference to another license file.

Re: [PATCH] media: v4l: xilinx: Use SPDX-License-Identifier

[Joe Perches]

On Thu, 2017-12-14 at 20:28 +0200, Laurent Pinchart wrote:
> Hi Mauro,
> 
> On Thursday, 14 December 2017 19:05:27 EET Mauro Carvalho Chehab wrote:
> > Em Fri,  8 Dec 2017 18:05:37 +0530 Dhaval Shah escreveu:
> > > SPDX-License-Identifier is used for the Xilinx Video IP and
> > > related drivers.
> > > 
> > > Signed-off-by: Dhaval Shah 
> > 
> > Hi Dhaval,
> > 
> > You're not listed as one of the Xilinx driver maintainers. I'm afraid that,
> > without their explicit acks, sent to the ML, I can't accept a patch
> > touching at the driver's license tags.
> 
> The patch doesn't change the license, I don't see why it would cause any 
> issue. Greg isn't listed as the maintainer or copyright holder of any of the 
> 10k+ files to which he added an SPDX license header in the last kernel 
> release.

Adding a comment line that describes an implicit or
explicit license is different than removing the license
text itself.

Re: [PATCH] media: v4l: xilinx: Use SPDX-License-Identifier

[Joe Perches]

On Thu, 2017-12-14 at 15:05 -0200, Mauro Carvalho Chehab wrote:
> Em Fri,  8 Dec 2017 18:05:37 +0530
> Dhaval Shah  escreveu:
> 
> > SPDX-License-Identifier is used for the Xilinx Video IP and
> > related drivers.
> > 
> > Signed-off-by: Dhaval Shah 
> 
> Hi Dhaval,
> 
> You're not listed as one of the Xilinx driver maintainers. I'm afraid that,
> without their explicit acks, sent to the ML, I can't accept a patch
> touching at the driver's license tags.

And even a maintainer may not have the sole right
to modify a license.

Re: [PATCH] tuners: tda8290: reduce stack usage with kasan

[Joe Perches]

On Tue, 2017-12-12 at 15:21 +0100, Arnd Bergmann wrote:
> On Tue, Dec 12, 2017 at 1:45 PM, Mauro Carvalho Chehab
>  wrote:
> > Em Tue, 12 Dec 2017 03:42:32 -0800
> > Joe Perches  escreveu:
> > 
> > > > I actually thought about marking them 'const' here before sending
> > > > (without noticing the changelog text) and then ran into what must
> > > > have led me to drop the 'const' originally: tuner_i2c_xfer_send()
> > > > takes a non-const pointer. This can be fixed but it requires
> > > > an ugly cast:
> > > 
> > > Casting away const is always a horrible hack.
> > > 
> > > Until it could be changed, my preference would
> > > be to update the changelog and perhaps add to
> > > the changelog the reason why it can not be const
> > > as detailed below.
> > > 
> > > ie: xfer_send and xfer_xend_recv both take a
> > > non-const unsigned char *
> 
> Ok.
> 
> > Perhaps, on a separate changeset, we could change I2C routines to
> > accept const unsigned char pointers. This is unrelated to tda8290
> > KASAN fixes. So, it should go via I2C tree, and, once accepted
> > there, we can change V4L2 drivers (and other drivers) accordingly.
> 
> I don't see how that would work unfortunately. i2c_msg contains
> a pointer to the data, and that is used for both input and output,
> including arrays like
> 
> struct i2c_msg msgs[] = {
> {
> .addr = dvo->slave_addr,
> .flags = 0,
> .len = 1,
> .buf = &addr,
> },
> {
> .addr = dvo->slave_addr,
> .flags = I2C_M_RD,
> .len = 1,
> .buf = val,
> }
> };
> 
> that have one constant output pointer and one non-constant
> input pointer. We could add an anonymous union for 'buf'
> to make that two separate pointers, but that's barely any
> better than the cast, and it would break the named initializers
> in the example above, at least on older compilers. Adding
> a second pointer to i2c_msg would add a bit of bloat and
> also require tree-wide changes or ugly hacks.

Perhaps add something like

struct i2c_msg_set {
__u16 addr; /* slave address*/
__u16 flags;
__u16 len;  /* msg length   */
const __u8 *buf;/* pointer to read-only msg data*/
};

struct i2c_msg_get {
__u16 addr; /* slave address*/
__u16 flags;
__u16 len;  /* msg length   */
__u8 *buf;  /* pointer to writeable msg data*/
};

to the uapi include and use that where appropriate
but where a write then read is done via a single
i2c_msg array, it's not really feasible either.

Probably better to avoid any churn and just mark
all these as static rather than static const.

Re: [PATCH] tuners: tda8290: reduce stack usage with kasan

[Joe Perches]

On Tue, 2017-12-12 at 11:24 +0100, Arnd Bergmann wrote:
> On Mon, Dec 11, 2017 at 10:17 PM, Michael Ira Krufky
>  wrote:
> > On Mon, Dec 11, 2017 at 2:34 PM, Joe Perches  wrote:
> > > On Mon, 2017-12-11 at 13:06 +0100, Arnd Bergmann wrote:
> > > > With CONFIG_KASAN enabled, we get a relatively large stack frame in one 
> > > > function
> > > > 
> > > > drivers/media/tuners/tda8290.c: In function 'tda8290_set_params':
> > > > drivers/media/tuners/tda8290.c:310:1: warning: the frame size of 1520 
> > > > bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > > > 
> > > > With CONFIG_KASAN_EXTRA this goes up to
> > > > 
> > > > drivers/media/tuners/tda8290.c: In function 'tda8290_set_params':
> > > > drivers/media/tuners/tda8290.c:310:1: error: the frame size of 3200 
> > > > bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
> > > > 
> > > > We can significantly reduce this by marking local arrays as 'static 
> > > > const', and
> > > > this should result in better compiled code for everyone.
> > > 
> > > []
> > > > diff --git a/drivers/media/tuners/tda8290.c 
> > > > b/drivers/media/tuners/tda8290.c
> > > 
> > > []
> > > > @@ -63,8 +63,8 @@ static int tda8290_i2c_bridge(struct dvb_frontend 
> > > > *fe, int close)
> > > >  {
> > > >   struct tda8290_priv *priv = fe->analog_demod_priv;
> > > > 
> > > > - unsigned char  enable[2] = { 0x21, 0xC0 };
> > > > - unsigned char disable[2] = { 0x21, 0x00 };
> > > > + static unsigned char  enable[2] = { 0x21, 0xC0 };
> > > > + static unsigned char disable[2] = { 0x21, 0x00 };
> > > 
> > > Doesn't match commit message.
> > > 
> > > static const or just static?
> > > 
> > > > @@ -84,9 +84,9 @@ static int tda8295_i2c_bridge(struct dvb_frontend 
> > > > *fe, int close)
> > > >  {
> > > >   struct tda8290_priv *priv = fe->analog_demod_priv;
> > > > 
> > > > - unsigned char  enable[2] = { 0x45, 0xc1 };
> > > > - unsigned char disable[2] = { 0x46, 0x00 };
> > > > - unsigned char buf[3] = { 0x45, 0x01, 0x00 };
> > > > + static unsigned char  enable[2] = { 0x45, 0xc1 };
> > > > + static unsigned char disable[2] = { 0x46, 0x00 };
> > > 
> > > etc.
> > > 
> > > 
> > 
> > 
> > Joe is correct - they can be CONSTified. My bad -- a lot of the code I
> > wrote many years ago has this problem -- I wasn't so stack-conscious
> > back then.
> > 
> > The bytes in `enable` / `disable` don't get changed, but they may be
> > copied to another byte array that does get changed.  If would be best
> > to make these `static const`
> 
> Right. This was an older patch of mine that I picked up again
> after running into a warning that I had been ignoring for a while,
> and I didn't double-check the message.
> 
> I actually thought about marking them 'const' here before sending
> (without noticing the changelog text) and then ran into what must
> have led me to drop the 'const' originally: tuner_i2c_xfer_send()
> takes a non-const pointer. This can be fixed but it requires
> an ugly cast:

Casting away const is always a horrible hack.

Until it could be changed, my preference would
be to update the changelog and perhaps add to
the changelog the reason why it can not be const
as detailed below.

ie: xfer_send and xfer_xend_recv both take a
non-const unsigned char *

> diff --git a/drivers/media/tuners/tuner-i2c.h 
> b/drivers/media/tuners/tuner-i2c.h
> index bda67a5a76f2..809466eec780 100644
> --- a/drivers/media/tuners/tuner-i2c.h
> +++ b/drivers/media/tuners/tuner-i2c.h
> @@ -34,10 +34,10 @@ struct tuner_i2c_props {
>  };
> 
>  static inline int tuner_i2c_xfer_send(struct tuner_i2c_props *props,
> - unsigned char *buf, int len)
> + const unsigned char *buf, int len)
>  {
> struct i2c_msg msg = { .addr = props->addr, .flags = 0,
> -  .buf = buf, .len = len };
> +  .buf = (unsigned char *)buf, .len = len };
> int ret = i2c_transfer(props->adap, &msg, 1);
> 
> return (ret == 1) ? len : ret;
> @@ -54,11 +54,11 @@ static inline int tuner_i2c_xfer_recv(struct
> tuner_i2c_props *props,
>  }

Re: [PATCH] tuners: tda8290: reduce stack usage with kasan

[Joe Perches]

On Mon, 2017-12-11 at 13:06 +0100, Arnd Bergmann wrote:
> With CONFIG_KASAN enabled, we get a relatively large stack frame in one 
> function
> 
> drivers/media/tuners/tda8290.c: In function 'tda8290_set_params':
> drivers/media/tuners/tda8290.c:310:1: warning: the frame size of 1520 bytes 
> is larger than 1024 bytes [-Wframe-larger-than=]
> 
> With CONFIG_KASAN_EXTRA this goes up to
> 
> drivers/media/tuners/tda8290.c: In function 'tda8290_set_params':
> drivers/media/tuners/tda8290.c:310:1: error: the frame size of 3200 bytes is 
> larger than 3072 bytes [-Werror=frame-larger-than=]
> 
> We can significantly reduce this by marking local arrays as 'static const', 
> and
> this should result in better compiled code for everyone.
[]
> diff --git a/drivers/media/tuners/tda8290.c b/drivers/media/tuners/tda8290.c
[]
> @@ -63,8 +63,8 @@ static int tda8290_i2c_bridge(struct dvb_frontend *fe, int 
> close)
>  {
>   struct tda8290_priv *priv = fe->analog_demod_priv;
>  
> - unsigned char  enable[2] = { 0x21, 0xC0 };
> - unsigned char disable[2] = { 0x21, 0x00 };
> + static unsigned char  enable[2] = { 0x21, 0xC0 };
> + static unsigned char disable[2] = { 0x21, 0x00 };

Doesn't match commit message.

static const or just static?

> @@ -84,9 +84,9 @@ static int tda8295_i2c_bridge(struct dvb_frontend *fe, int 
> close)
>  {
>   struct tda8290_priv *priv = fe->analog_demod_priv;
>  
> - unsigned char  enable[2] = { 0x45, 0xc1 };
> - unsigned char disable[2] = { 0x46, 0x00 };
> - unsigned char buf[3] = { 0x45, 0x01, 0x00 };
> + static unsigned char  enable[2] = { 0x45, 0xc1 };
> + static unsigned char disable[2] = { 0x46, 0x00 };

etc.

Re: [PATCH 0/4] treewide: Fix line continuation formats

[Joe Perches]

On Thu, 2017-11-16 at 12:11 -0500, Mimi Zohar wrote:
> On Thu, 2017-11-16 at 07:27 -0800, Joe Perches wrote:
> > Avoid using line continations in formats as that causes unexpected
> > output.
> 
> Is having lines greater than 80 characters the preferred method?

yes.

>  Could you add quotes before the backlash and before the first word on
> the next line instead?

coalesced formats are preferred.

[PATCH 3/4] [media] dibx000_common: Fix line continuation format

[Joe Perches]

Line continuations with excess spacing causes unexpected output.

Signed-off-by: Joe Perches 
---
 drivers/media/dvb-frontends/dibx000_common.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/dvb-frontends/dibx000_common.c 
b/drivers/media/dvb-frontends/dibx000_common.c
index bc28184c7fb0..d981233e458f 100644
--- a/drivers/media/dvb-frontends/dibx000_common.c
+++ b/drivers/media/dvb-frontends/dibx000_common.c
@@ -288,8 +288,8 @@ static int dibx000_i2c_gated_gpio67_xfer(struct i2c_adapter 
*i2c_adap,
int ret;
 
if (num > 32) {
-   dprintk("%s: too much I2C message to be transmitted (%i).\
-   Maximum is 32", __func__, num);
+   dprintk("%s: too much I2C message to be transmitted (%i). 
Maximum is 32",
+   __func__, num);
return -ENOMEM;
}
 
@@ -335,8 +335,8 @@ static int dibx000_i2c_gated_tuner_xfer(struct i2c_adapter 
*i2c_adap,
int ret;
 
if (num > 32) {
-   dprintk("%s: too much I2C message to be transmitted (%i).\
-   Maximum is 32", __func__, num);
+   dprintk("%s: too much I2C message to be transmitted (%i). 
Maximum is 32",
+   __func__, num);
return -ENOMEM;
}
 
-- 
2.15.0

[PATCH 0/4] treewide: Fix line continuation formats

[Joe Perches]

Avoid using line continations in formats as that causes unexpected
output.

Joe Perches (4):
  rk3399_dmc: Fix line continuation format
  drm: amd: Fix line continuation formats
  [media] dibx000_common: Fix line continuation format
  ima: Fix line continuation format

 drivers/devfreq/rk3399_dmc.c   |  4 ++--
 drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c   | 11 -
 .../amd/powerplay/hwmgr/process_pptables_v1_0.c|  6 ++---
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 27 --
 drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c   |  6 ++---
 .../gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c  |  9 +++-
 .../gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c   |  6 ++---
 drivers/media/dvb-frontends/dibx000_common.c   |  8 +++
 security/integrity/ima/ima_template.c  | 11 -
 9 files changed, 33 insertions(+), 55 deletions(-)

-- 
2.15.0

[PATCH] media: uvcvideo: Make some structs const

[Joe Perches]

Move some data to text

$ size drivers/media/usb/uvc/uvc_ctrl.o*
   textdata bss dec hex filename
  343232364   0   366878f4f drivers/media/usb/uvc/uvc_ctrl.o.new
  286598028   0   366878f4f drivers/media/usb/uvc/uvc_ctrl.o.old

Signed-off-by: Joe Perches 
---
 drivers/media/usb/uvc/uvc_ctrl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 20397aba6849..44a0554bf62d 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -37,7 +37,7 @@
  * Controls
  */
 
-static struct uvc_control_info uvc_ctrls[] = {
+static const struct uvc_control_info uvc_ctrls[] = {
{
.entity = UVC_GUID_UVC_PROCESSING,
.selector   = UVC_PU_BRIGHTNESS_CONTROL,
@@ -420,7 +420,7 @@ static void uvc_ctrl_set_rel_speed(struct 
uvc_control_mapping *mapping,
data[first+1] = min_t(int, abs(value), 0xff);
 }
 
-static struct uvc_control_mapping uvc_ctrl_mappings[] = {
+static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
{
.id = V4L2_CID_BRIGHTNESS,
.name   = "Brightness",
-- 
2.15.0

MAINTAINERS has a AS3645A LED FLASH duplicated section in -next

[Joe Perches]

MAINTAINERS is not supposed to have duplicated sections.
Can you both please resolve this?

AS3645A LED FLASH CONTROLLER DRIVER
M:  Sakari Ailus 
L:  linux-l...@vger.kernel.org
S:  Maintained
F:  drivers/leds/leds-as3645a.c

AS3645A LED FLASH CONTROLLER DRIVER
M:  Laurent Pinchart 
L:  linux-media@vger.kernel.org
T:  git git://linuxtv.org/media_tree.git
S:  Maintained
F:  drivers/media/i2c/as3645a.c
F:  include/media/i2c/as3645a.h

Re: [PATCH 1/6] [media] omap_vout: Delete an error message for a failed memory allocation in omap_vout_create_video_devices()

[Joe Perches]

On Sun, 2017-09-24 at 12:22 +0200, SF Markus Elfring wrote:
> Omit an extra message for a memory allocation failure in this function.
[]
> diff --git a/drivers/media/platform/omap/omap_vout.c 
> b/drivers/media/platform/omap/omap_vout.c
[]
> @@ -1948,7 +1948,5 @@ static int __init omap_vout_create_video_devices(struct 
> platform_device *pdev)
> - if (!vout) {
> - dev_err(&pdev->dev, ": could not allocate memory\n");
> + if (!vout)
>   return -ENOMEM;
> - }
>  
>   vout->vid = k;
>   vid_dev->vouts[k] = vout;

Use normal patch styles.
Fix your tools before you send any more patches.

Re: [media] spca500: Use common error handling code in spca500_synch310()

[Joe Perches]

On Fri, 2017-09-22 at 19:46 +0200, SF Markus Elfring wrote:
> > > > They are both equally uninformative.
> > > 
> > > Which identifier would you find appropriate there?
> > 
> > error was fine.
> 
> How do the different views fit together?

Markus, please respect what others tell you because
your coding style "taste" could be improved.

[PATCH] gspca: Convert PERR to gspca_err

[Joe Perches]

Use a more typical kernel logging style.

The current macro hides the gspca_dev argument so add it to the
macro uses instead.

Miscellanea:

o Add missing '\n' terminations to formats
o Realign arguments to open parenthesis

Signed-off-by: Joe Perches 
---
 drivers/media/usb/gspca/benq.c |  6 +--
 drivers/media/usb/gspca/conex.c|  6 +--
 drivers/media/usb/gspca/cpia1.c| 24 ++-
 drivers/media/usb/gspca/dtcs033.c  | 12 +++---
 drivers/media/usb/gspca/etoms.c|  4 +-
 drivers/media/usb/gspca/gl860/gl860.c  |  2 +-
 drivers/media/usb/gspca/gspca.c| 26 +++-
 drivers/media/usb/gspca/gspca.h|  4 +-
 drivers/media/usb/gspca/jeilinj.c  |  2 +-
 drivers/media/usb/gspca/konica.c   | 26 ++--
 drivers/media/usb/gspca/m5602/m5602_core.c |  2 +-
 drivers/media/usb/gspca/mr97310a.c |  6 +--
 drivers/media/usb/gspca/ov519.c| 65 --
 drivers/media/usb/gspca/ov534.c|  4 +-
 drivers/media/usb/gspca/pac7302.c  |  2 +-
 drivers/media/usb/gspca/pac7311.c  |  2 +-
 drivers/media/usb/gspca/sn9c2028.c |  4 +-
 drivers/media/usb/gspca/sonixj.c   |  4 +-
 drivers/media/usb/gspca/spca1528.c |  4 +-
 drivers/media/usb/gspca/spca500.c  | 36 -
 drivers/media/usb/gspca/spca501.c  |  4 +-
 drivers/media/usb/gspca/spca505.c  |  2 +-
 drivers/media/usb/gspca/spca508.c  |  3 +-
 drivers/media/usb/gspca/spca561.c  |  2 +-
 drivers/media/usb/gspca/sq905.c|  2 +-
 drivers/media/usb/gspca/sq905c.c   |  6 +--
 drivers/media/usb/gspca/sq930x.c   |  2 +-
 drivers/media/usb/gspca/stv0680.c  | 16 
 drivers/media/usb/gspca/stv06xx/stv06xx.c  | 10 ++---
 drivers/media/usb/gspca/sunplus.c  |  2 +-
 drivers/media/usb/gspca/touptek.c  | 38 +
 drivers/media/usb/gspca/w996Xcf.c  |  2 +-
 32 files changed, 174 insertions(+), 156 deletions(-)

diff --git a/drivers/media/usb/gspca/benq.c b/drivers/media/usb/gspca/benq.c
index 60a728203b3b..b5955bf0d0fc 100644
--- a/drivers/media/usb/gspca/benq.c
+++ b/drivers/media/usb/gspca/benq.c
@@ -180,9 +180,9 @@ static void sd_isoc_irq(struct urb *urb)
/* check the packet status and length */
if (urb0->iso_frame_desc[i].actual_length != SD_PKT_SZ
|| urb->iso_frame_desc[i].actual_length != SD_PKT_SZ) {
-   PERR("ISOC bad lengths %d / %d",
-   urb0->iso_frame_desc[i].actual_length,
-   urb->iso_frame_desc[i].actual_length);
+   gspca_err(gspca_dev, "ISOC bad lengths %d / %d\n",
+ urb0->iso_frame_desc[i].actual_length,
+ urb->iso_frame_desc[i].actual_length);
gspca_dev->last_packet_type = DISCARD_PACKET;
continue;
}
diff --git a/drivers/media/usb/gspca/conex.c b/drivers/media/usb/gspca/conex.c
index bdcdf7999c56..0223b33156dd 100644
--- a/drivers/media/usb/gspca/conex.c
+++ b/drivers/media/usb/gspca/conex.c
@@ -70,7 +70,7 @@ static void reg_r(struct gspca_dev *gspca_dev,
struct usb_device *dev = gspca_dev->dev;
 
if (len > USB_BUF_SZ) {
-   PERR("reg_r: buffer overflow\n");
+   gspca_err(gspca_dev, "reg_r: buffer overflow\n");
return;
}
 
@@ -109,7 +109,7 @@ static void reg_w(struct gspca_dev *gspca_dev,
struct usb_device *dev = gspca_dev->dev;
 
if (len > USB_BUF_SZ) {
-   PERR("reg_w: buffer overflow\n");
+   gspca_err(gspca_dev, "reg_w: buffer overflow\n");
return;
}
PDEBUG(D_USBO, "reg write [%02x] = %02x..", index, *buffer);
@@ -683,7 +683,7 @@ static void cx11646_jpeg(struct gspca_dev*gspca_dev)
reg_w_val(gspca_dev, 0x0053, 0x00);
} while (--retry);
if (retry == 0)
-   PERR("Damned Errors sending jpeg Table");
+   gspca_err(gspca_dev, "Damned Errors sending jpeg Table\n");
/* send the qtable now */
reg_r(gspca_dev, 0x0001, 1);/* -> 0x18 */
length = 8;
diff --git a/drivers/media/usb/gspca/cpia1.c b/drivers/media/usb/gspca/cpia1.c
index e91d00762e94..99b456d64729 100644
--- a/drivers/media/usb/gspca/cpia1.c
+++ b/drivers/media/usb/gspca/cpia1.c
@@ -419,7 +419,8 @@ static int cpia_usb_transferCmd(struct gspca_dev 
*gspca_dev, u8 *command)
pipe = usb_sndctrlpipe(gspca_dev->dev, 0);
requesttype = USB_TYPE_VENDOR | USB_RECIP_DEVICE;
} else {
-   PERR("Unexpected first byte 

Re: [PATCH 2/2] [media] stm32-dcmi: Improve four size determinations

[Joe Perches]

On Fri, 2017-09-15 at 19:29 +0200, SF Markus Elfring wrote:
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
> b/drivers/media/platform/stm32/stm32-dcmi.c
[]
> @@ -1372,9 +1372,8 @@ static int dcmi_formats_init(struct stm32_dcmi *dcmi)
>   dcmi->sd_formats = devm_kcalloc(dcmi->dev,
> - num_fmts, sizeof(struct dcmi_format *),
> + num_fmts, sizeof(*dcmi->sd_formats),
>   GFP_KERNEL);
>   if (!dcmi->sd_formats)
>   return -ENOMEM;
>  
> - memcpy(dcmi->sd_formats, sd_fmts,
> -num_fmts * sizeof(struct dcmi_format *));
> + memcpy(dcmi->sd_formats, sd_fmts, num_fmts * sizeof(*dcmi->sd_formats));

devm_kmemdup

Re: [PATCH 4/4] [media] zr364xx: Fix a typo in a comment line of the file header

[Joe Perches]

On Tue, 2017-08-29 at 07:35 +0200, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Mon, 28 Aug 2017 22:46:30 +0200
> 
> Fix a word in this description.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/media/usb/zr364xx/zr364xx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/zr364xx/zr364xx.c 
> b/drivers/media/usb/zr364xx/zr364xx.c
> index 4cc6d2a9d91f..4ccf71d8b608 100644
> --- a/drivers/media/usb/zr364xx/zr364xx.c
> +++ b/drivers/media/usb/zr364xx/zr364xx.c
> @@ -2,7 +2,7 @@
>   * Zoran 364xx based USB webcam module version 0.73
>   *
>   * Allows you to use your USB webcam with V4L2 applications
> - * This is still in heavy developpement !
> + * This is still in heavy development!

There is almost no development being done here.
Just delete the line.

Re: [PATCH] media: staging: atomisp: sh_css_calloc shall return a pointer to the allocated space

[Joe Perches]

On Wed, 2017-08-02 at 18:00 +1000, Sergei A. Trusov wrote:
> The calloc function returns either a null pointer or a pointer to the
> allocated space. Add the second case that is missed.

gads.

Bug added by commit da22013f7df4 ("atomisp: remove indirection from
sh_css_malloc")

These wrappers should really be deleted.

Re: [PATCH 2/2] [media] staging/atomisp: fixed trivial coding style issue

[Joe Perches]

On Sun, 2017-07-16 at 16:38 -0700, Shy More wrote:
> Below was the trival error flagged by checkpatch.pl:
> ERROR: space prohibited after that open parenthesis '('
[]
> diff --git 
> a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c
>  
> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c
[]
> @@ -131,7 +131,7 @@ void ia_css_isys_ibuf_rmgr_release(
>   for (i = 0; i < ibuf_rsrc.num_allocated; i++) {
>   handle = getHandle(i);
>   if ((handle->start_addr == *start_addr)
> - && ( true == handle->active)) {
> + && (true == handle->active)) {
>   handle->active = false;
>   ibuf_rsrc.num_active--;
>   break;

Better would have been to remove the comparison to true

if (handle->start_addr == *start_addr && handle->active)

but this would probably read better and perhaps be
marginally faster on some processors if written like:

if (handle->active && handle->start_addr == *start_addr)

Re: [PATCH 05/14] isdn: isdnloop: suppress a gcc-7 warning

[Joe Perches]

On Fri, 2017-07-14 at 11:25 +0200, Arnd Bergmann wrote:
> We test whether a bit is set in a mask here, which is correct
> but gcc warns about it as it thinks it might be confusing:
> 
> drivers/isdn/isdnloop/isdnloop.c:412:37: error: ?: using integer constants in 
> boolean context, the expression will always evaluate to 'true' 
> [-Werror=int-in-bool-context]
> 
> This replaces the negation of an integer with an equivalent
> comparison to zero, which gets rid of the warning.
[]
> diff --git a/drivers/isdn/isdnloop/isdnloop.c 
> b/drivers/isdn/isdnloop/isdnloop.c
[]
> @@ -409,7 +409,7 @@ isdnloop_sendbuf(int channel, struct sk_buff *skb, 
> isdnloop_card *card)
>   return -EINVAL;
>   }
>   if (len) {
> - if (!(card->flags & (channel) ? ISDNLOOP_FLAGS_B2ACTIVE : 
> ISDNLOOP_FLAGS_B1ACTIVE))
> + if ((card->flags & (channel) ? ISDNLOOP_FLAGS_B2ACTIVE : 
> ISDNLOOP_FLAGS_B1ACTIVE) == 0)
>   return 0;
>   if (card->sndcount[channel] > ISDNLOOP_MAX_SQUEUE)
>   return 0;

The if as written can not be zero.

drivers/isdn/isdnloop/isdnloop.h:#define ISDNLOOP_FLAGS_B1ACTIVE 1  /* 
B-Channel-1 is open   */
drivers/isdn/isdnloop/isdnloop.h:#define ISDNLOOP_FLAGS_B2ACTIVE 2  /* 
B-Channel-2 is open   */

Perhaps this is a logic defect and should be:

if (!(card->flags & ((channel) ? ISDNLOOP_FLAGS_B2ACTIVE : 
ISDNLOOP_FLAGS_B1ACTIVE)))

Re: [PATCH 13/14] iopoll: avoid -Wint-in-bool-context warning

[Joe Perches]

On Fri, 2017-07-14 at 11:31 +0200, Arnd Bergmann wrote:
> When we pass the result of a multiplication as the timeout, we
> can get a warning:
> 
> drivers/mmc/host/bcm2835.c:596:149: error: '*' in boolean context, suggest 
> '&&' instead [-Werror=int-in-bool-context]
> drivers/mfd/arizona-core.c:247:195: error: '*' in boolean context, suggest 
> '&&' instead [-Werror=int-in-bool-context]
> 
> This is easy to avoid by comparing the timeout to zero instead,
> making it a boolean expression.

Perhaps this is better as != 0 if the multiply is signed.

> diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
[]
> @@ -48,7 +48,8 @@
>   (val) = op(addr); \
>   if (cond) \
>   break; \
> - if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
> + if ((timeout_us) > 0 && \
> + ktime_compare(ktime_get(), timeout) > 0) { \
>   (val) = op(addr); \
>   break; \
>   } \

etc...

Re: [PATCH] checkpatch: fixed alignment and comment style

[Joe Perches]

On Sun, 2017-07-09 at 19:39 +0200, Philipp Guendisch wrote:
> This patch fixed alignment, comment style and one appearance of
> misordered constant in an if comparison.
> Semantic should not be affected by this patch.

Your email subject is wrong.  This is not a checkpatch patch.

Your subject line should be something like:

[PATCH] staging: atomisp2: hmm: Alignment code to open parenthesis

And it's probably more likely to be applied if you separate out
the two different types of changes you are making into 2 patches.

Re: [PATCH 1/2] staging: media: atomisp2: css2400: Replace kfree()/vfree() with kvfree()

[Joe Perches]

On Fri, 2017-07-07 at 20:40 -0400, Amitoj Kaur Chawla wrote:
> Conditionally calling kfree()/vfree() can be replaced by a call to 
> kvfree() which handles both kmalloced memory and vmalloced memory.
[]
> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c 
> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
[]
> @@ -2029,10 +2029,7 @@ void *sh_css_calloc(size_t N, size_t size)
> 
>  void sh_css_free(void *ptr)
>  {
> - if (is_vmalloc_addr(ptr))
> - vfree(ptr);
> - else
> - kfree(ptr);
> + kvfree(ptr);
>  }

Why not just get rid of sh_css_free and use kvfree directly?

Why not get rid of all the sh_css_ functions?

Re: [PATCH] drivers/staging/media: Prefer using __func__ instead

[Joe Perches]

On Thu, 2017-06-29 at 16:59 +0530, Pushkar Jambhlekar wrote:
> Function name is hardcoded. replacing with __func__

Please run your proposed patches through checkpatch
before you send them.

> diff --git a/drivers/staging/media/cxd2099/cxd2099.c 
> b/drivers/staging/media/cxd2099/cxd2099.c
[]
> @@ -100,7 +100,7 @@ static int i2c_read_reg(struct i2c_adapter *adapter, u8 
> adr,
>  .buf = val, .len = 1} };
>  
>   if (i2c_transfer(adapter, msgs, 2) != 2) {
> - dev_err(&adapter->dev, "error in i2c_read_reg\n");
> + dev_err(&adapter->dev, "error in %s\n", __func__);
>   return -1;
>   }
>   return 0;
> @@ -115,7 +115,7 @@ static int i2c_read(struct i2c_adapter *adapter, u8 adr,
>  .buf = data, .len = n} };
>  
>   if (i2c_transfer(adapter, msgs, 2) != 2) {
> - dev_err(&adapter->dev, "error in i2c_read\n");
> + dev_err(&adapter->dev, "error in %s\n",__func__);

There is a missing space before __func__.

As well, the form for listing a function name
is generally:

print("%s: \n", __func__);

so ideally, these messages would be something like:

dev_err(&adapter->dev, "%s: i2c_transfer error\n", __func__);

[PATCH] [media] tuner-core: Remove unused #define PREFIX

[Joe Perches]

Commit 680d87c0a9e3 ("[media] tuner-core: use pr_foo, instead of
internal printk macros") removed the use of PREFIX, remove the #define

Signed-off-by: Joe Perches 
---
 drivers/media/v4l2-core/tuner-core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/v4l2-core/tuner-core.c 
b/drivers/media/v4l2-core/tuner-core.c
index e48b7c032c95..8db45dfc271b 100644
--- a/drivers/media/v4l2-core/tuner-core.c
+++ b/drivers/media/v4l2-core/tuner-core.c
@@ -43,8 +43,6 @@
 
 #define UNSET (-1U)
 
-#define PREFIX (t->i2c->dev.driver->name)
-
 /*
  * Driver modprobe parameters
  */
-- 
2.10.0.rc2.1.g053435c

[PATCH] stkwebcam: Use more common logging styles

[Joe Perches]

Convert STK_ to pr_ to use the typical kernel logging.
Add a define for pr_fmt.  No change in logging output.

Miscellanea:

o Remove now unused PREFIX and STK_ macros
o Realign arguments
o Use pr__ratelimited
o Add a few missing newlines to formats

Signed-off-by: Joe Perches 
---
 drivers/media/usb/stkwebcam/stk-sensor.c | 32 ---
 drivers/media/usb/stkwebcam/stk-webcam.c | 70 
 drivers/media/usb/stkwebcam/stk-webcam.h |  6 ---
 3 files changed, 52 insertions(+), 56 deletions(-)

diff --git a/drivers/media/usb/stkwebcam/stk-sensor.c 
b/drivers/media/usb/stkwebcam/stk-sensor.c
index 985af9933c7e..c1d4505f84ea 100644
--- a/drivers/media/usb/stkwebcam/stk-sensor.c
+++ b/drivers/media/usb/stkwebcam/stk-sensor.c
@@ -41,6 +41,8 @@
 
 /* It seems the i2c bus is controlled with these registers */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include "stk-webcam.h"
 
 #define STK_IIC_BASE   (0x0200)
@@ -239,8 +241,8 @@ static int stk_sensor_outb(struct stk_camera *dev, u8 reg, 
u8 val)
} while (tmpval == 0 && i < MAX_RETRIES);
if (tmpval != STK_IIC_STAT_TX_OK) {
if (tmpval)
-   STK_ERROR("stk_sensor_outb failed, status=0x%02x\n",
-   tmpval);
+   pr_err("stk_sensor_outb failed, status=0x%02x\n",
+  tmpval);
return 1;
} else
return 0;
@@ -262,8 +264,8 @@ static int stk_sensor_inb(struct stk_camera *dev, u8 reg, 
u8 *val)
} while (tmpval == 0 && i < MAX_RETRIES);
if (tmpval != STK_IIC_STAT_RX_OK) {
if (tmpval)
-   STK_ERROR("stk_sensor_inb failed, status=0x%02x\n",
-   tmpval);
+   pr_err("stk_sensor_inb failed, status=0x%02x\n",
+  tmpval);
return 1;
}
 
@@ -366,29 +368,29 @@ int stk_sensor_init(struct stk_camera *dev)
if (stk_camera_write_reg(dev, STK_IIC_ENABLE, STK_IIC_ENABLE_YES)
|| stk_camera_write_reg(dev, STK_IIC_ADDR, SENSOR_ADDRESS)
|| stk_sensor_outb(dev, REG_COM7, COM7_RESET)) {
-   STK_ERROR("Sensor resetting failed\n");
+   pr_err("Sensor resetting failed\n");
return -ENODEV;
}
msleep(10);
/* Read the manufacturer ID: ov = 0x7FA2 */
if (stk_sensor_inb(dev, REG_MIDH, &idh)
|| stk_sensor_inb(dev, REG_MIDL, &idl)) {
-   STK_ERROR("Strange error reading sensor ID\n");
+   pr_err("Strange error reading sensor ID\n");
return -ENODEV;
}
if (idh != 0x7f || idl != 0xa2) {
-   STK_ERROR("Huh? you don't have a sensor from ovt\n");
+   pr_err("Huh? you don't have a sensor from ovt\n");
return -ENODEV;
}
if (stk_sensor_inb(dev, REG_PID, &idh)
|| stk_sensor_inb(dev, REG_VER, &idl)) {
-   STK_ERROR("Could not read sensor model\n");
+   pr_err("Could not read sensor model\n");
return -ENODEV;
}
stk_sensor_write_regvals(dev, ov_initvals);
msleep(10);
-   STK_INFO("OmniVision sensor detected, id %02X%02X at address %x\n",
-idh, idl, SENSOR_ADDRESS);
+   pr_info("OmniVision sensor detected, id %02X%02X at address %x\n",
+   idh, idl, SENSOR_ADDRESS);
return 0;
 }
 
@@ -520,7 +522,8 @@ int stk_sensor_configure(struct stk_camera *dev)
case MODE_SXGA: com7 = COM7_FMT_SXGA;
dummylines = 0;
break;
-   default: STK_ERROR("Unsupported mode %d\n", dev->vsettings.mode);
+   default:
+   pr_err("Unsupported mode %d\n", dev->vsettings.mode);
return -EFAULT;
}
switch (dev->vsettings.palette) {
@@ -544,7 +547,8 @@ int stk_sensor_configure(struct stk_camera *dev)
com7 |= COM7_PBAYER;
rv = ov_fmt_bayer;
break;
-   default: STK_ERROR("Unsupported colorspace\n");
+   default:
+   pr_err("Unsupported colorspace\n");
return -EFAULT;
}
/*FIXME sometimes the sensor go to a bad state
@@ -564,7 +568,7 @@ int stk_sensor_configure(struct stk_camera *dev)
switch (dev->vsettings.mode) {
case MODE_VGA:
if (stk_sensor_set_hw(dev, 302, 1582, 6, 486))
-   STK_ERROR("stk_sensor_set_hw failed (VGA)\n");
+   pr_err("stk_sensor_set_hw failed (VGA)\n");
break;
case MODE_SXGA:

Re: [PATCH v2] [media] vb2: core: Lower the log level of debug outputs

[Joe Perches]

On Thu, 2017-06-08 at 14:24 +0900, Tomasz Figa wrote:
> On Thu, Jun 8, 2017 at 2:16 PM, Joe Perches  wrote:
[]
> > If there automated systems that rely on specific levels, then
> > changing the levels of individual messages could also cause
> > those automated systems to fail.
> 
> Well, that might be true for some of them indeed. I was thinking about
> our use case, which relies on particular numbers to get expected
> verbosity levels not caring about particular messages. I guess the
> break all or none rule is going to apply here, so we should do the
> bitmap conversion indeed. :)
> 
> On the other hand, I think it would be still preferable to do the
> conversion in a separate patch.

Right.  No worries.

Re: [PATCH v2] [media] vb2: core: Lower the log level of debug outputs

[Joe Perches]

On Thu, 2017-06-08 at 13:39 +0900, Tomasz Figa wrote:
> On Thu, Jun 8, 2017 at 12:24 PM, Hirokazu Honda  wrote:
> > Hi,
> > 
> > I completely understand bitmask method now.
> > I agree to the idea, but it is necessary to change the specification of
> > a debug parameter.
> >  (We probably need to change a document about that?)
> > For example, there is maybe a user who set a debug parameter 3.
> > The user assume that logs whose levels are less than 4 are shown.
> > However, after the bitmask method is adopted, someday the logs whose
> > level is 1 or 2 are only shown, not 3 level logs are not shown.
> > This will be confusing to users.
> 
> I think I have to agree with Hirokazu here. Even though it's only
> about debugging, there might be some automatic testing systems that
> actually rely on certain values here.

I think it's a non-argument.

If there automated systems that rely on specific levels, then
changing the levels of individual messages could also cause
those automated systems to fail.

Re: [PATCH v2] [media] vb2: core: Lower the log level of debug outputs

[Joe Perches]

On Wed, 2017-05-31 at 12:28 +0900, Hirokazu Honda wrote:
> If I understand a bitmap correctly, it is necessary to change the log level
> for each message.
> I didn't mean a bitmap will take a long CPU time.
> I mean the work to change so takes a long time.

No, none of the messages or levels need change,
only the >= test changes to & so that for instance,
level 1 and level 3 messages could be emitted
without also emitting level 2 messages.

The patch suggested is all that would be required.

Re: [PATCH v2] [media] vb2: core: Lower the log level of debug outputs

[Joe Perches]

On Wed, 2017-05-31 at 11:05 +0900, Hirokazu Honda wrote:
> Although bitmap is useful, there is need to change the log level for each
> log.
> Because it will take a longer time, it should be done in another patch.

I have no idea what you mean.

A bit & comparison is typically an identical instruction
cycle count to a >= comparison.

Re: [PATCH v2] [media] vb2: core: Lower the log level of debug outputs

[Joe Perches]

On Tue, 2017-05-30 at 18:49 +0900, Hirokazu Honda wrote:
> Some debug output whose log level is set 1 flooded the log.
> Their log level is lowered to find the important log easily.

Maybe use pr_debug instead?

Perhaps it would be better to change the level to a bitmap
so these can be more individually controlled.

Maybe add MODULE_PARM_DESC too.

Perhaps something like below (without the pr_debug conversion)

---
 drivers/media/v4l2-core/videobuf2-core.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 94afbbf92807..88ae2b238115 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -31,12 +31,13 @@
 
 static int debug;
 module_param(debug, int, 0644);
+MODULE_PARM_DESC(debug, "debugging output control bitmap (values from 0-31)")
 
-#define dprintk(level, fmt, arg...)  \
-   do {  \
-   if (debug >= level)   \
-   pr_info("vb2-core: %s: " fmt, __func__, ## arg); \
-   } while (0)
+#define dprintk(level, fmt, ...)   \
+do {   \
+   if (debug & BIT(level)) \
+   pr_info("vb2-core: %s: " fmt, __func__, ##__VA_ARGS__); \
+} while (0)
 
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 

[PATCH V2] staging: atomisp: Add __printf validation and fix fallout

[Joe Perches]

__printf validation adds format and argument validation.

Fix the various broken format/argument mismatches.

Signed-off-by: Joe Perches 
---

v2: bah, now without unrelated changes to other staging files...

I'm not at all sure all the modifications are appropriate.

Some maybe should use the original format types like
%x instead of %p with *pointer instead of just pointer

 .../isp/kernels/sdis/sdis_1.0/ia_css_sdis.host.c  |  6 +++---
 .../isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c   |  2 +-
 .../css2400/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c |  2 +-
 .../css2400/runtime/debug/interface/ia_css_debug.h|  1 +
 .../atomisp2/css2400/runtime/debug/src/ia_css_debug.c |  6 +++---
 .../media/atomisp/pci/atomisp2/css2400/sh_css.c   | 19 ++-
 .../media/atomisp/pci/atomisp2/css2400/sh_css_mipi.c  |  2 +-
 .../atomisp/pci/atomisp2/css2400/sh_css_params.c  | 10 +-
 8 files changed, 25 insertions(+), 23 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/sdis_1.0/ia_css_sdis.host.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/sdis_1.0/ia_css_sdis.host.c
index 0dde8425c67d..4c77e1463aaa 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/sdis_1.0/ia_css_sdis.host.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/sdis_1.0/ia_css_sdis.host.c
@@ -265,9 +265,9 @@ ia_css_translate_dvs_statistics(
assert(isp_stats->hor_proj != NULL);
assert(isp_stats->ver_proj != NULL);
 
-   IA_CSS_ENTER("hproj=%p, vproj=%p, haddr=%x, vaddr=%x",
-   host_stats->hor_proj, host_stats->ver_proj,
-   isp_stats->hor_proj, isp_stats->ver_proj);
+   IA_CSS_ENTER("hproj=%p, vproj=%p, haddr=%p, vaddr=%p",
+host_stats->hor_proj, host_stats->ver_proj,
+isp_stats->hor_proj, isp_stats->ver_proj);
 
hor_num_isp = host_stats->grid.aligned_height;
ver_num_isp = host_stats->grid.aligned_width;
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c
index 930061d48df7..5ac81f87bfa3 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c
@@ -213,7 +213,7 @@ ia_css_translate_dvs2_statistics(
 "hor_coefs.even_real=%p, hor_coefs.even_imag=%p, "
 "ver_coefs.odd_real=%p, ver_coefs.odd_imag=%p, "
 "ver_coefs.even_real=%p, ver_coefs.even_imag=%p, "
-"haddr=%x, vaddr=%x",
+"haddr=%p, vaddr=%p",
host_stats->hor_prod.odd_real, host_stats->hor_prod.odd_imag,
host_stats->hor_prod.even_real, host_stats->hor_prod.even_imag,
host_stats->ver_prod.odd_real, host_stats->ver_prod.odd_imag,
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c
index 804c19ab4485..222a7bd7f176 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c
@@ -55,7 +55,7 @@ ia_css_tnr_dump(
"tnr_coef", tnr->coef);
ia_css_debug_dtrace(level, "\t%-32s = %d\n",
"tnr_threshold_Y", tnr->threshold_Y);
-   ia_css_debug_dtrace(level, "\t%-32s = %d\n"
+   ia_css_debug_dtrace(level, "\t%-32s = %d\n",
"tnr_threshold_C", tnr->threshold_C);
 }
 
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/interface/ia_css_debug.h
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/interface/ia_css_debug.h
index be7df3a30c21..91c105cc6204 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/interface/ia_css_debug.h
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/interface/ia_css_debug.h
@@ -137,6 +137,7 @@ ia_css_debug_vdtrace(unsigned int level, const char *fmt, 
va_list args)
sh_css_vprint(fmt, args);
 }
 
+__printf(2, 3)
 extern void ia_css_debug_dtrace(unsigned int level, const char *fmt, ...);
 
 /*! @brief Dump sp thread's stack contents
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/r

[PATCH] staging: atomisp: Add __printf validation and fix fallout

[Joe Perches]

__printf validation adds format and argument validation.

Fix the various broken format/argument mismatches.

Signed-off-by: Joe Perches 
---

I'm not at all sure all the modifications are appropriate.

Some maybe should use the original format types like
%x instead of %p with *pointer instead of just pointer

 drivers/staging/ks7010/ks_wlan.h  | 13 +++--
 .../isp/kernels/sdis/sdis_1.0/ia_css_sdis.host.c  |  6 +++---
 .../isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c   |  2 +-
 .../css2400/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c |  2 +-
 .../css2400/runtime/debug/interface/ia_css_debug.h|  1 +
 .../atomisp2/css2400/runtime/debug/src/ia_css_debug.c |  6 +++---
 .../media/atomisp/pci/atomisp2/css2400/sh_css.c   | 19 ++-
 .../media/atomisp/pci/atomisp2/css2400/sh_css_mipi.c  |  2 +-
 .../atomisp/pci/atomisp2/css2400/sh_css_params.c  | 10 +-
 9 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan.h b/drivers/staging/ks7010/ks_wlan.h
index eb15db90733b..7df01703d861 100644
--- a/drivers/staging/ks7010/ks_wlan.h
+++ b/drivers/staging/ks7010/ks_wlan.h
@@ -35,13 +35,14 @@
 #include "ks7010_sdio.h"
 
 #ifdef KS_WLAN_DEBUG
-#define DPRINTK(n, fmt, args...) \
-   do { \
-   if (KS_WLAN_DEBUG > (n)) \
-   pr_notice("%s: "fmt, __func__, ## args); \
-   } while (0)
+#define DPRINTK(n, fmt, ...)\
+do {\
+   if (KS_WLAN_DEBUG > (n)) \
+   pr_notice("%s: "fmt, __func__, ##__VA_ARGS__);   \
+} while (0)
 #else
-#define DPRINTK(n, fmt, args...)
+#define DPRINTK(n, fmt, ...)   \
+   no_printk(fmt, ##__VA_ARGS__)
 #endif
 
 struct ks_wlan_parameter {
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/sdis_1.0/ia_css_sdis.host.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/sdis_1.0/ia_css_sdis.host.c
index 0dde8425c67d..4c77e1463aaa 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/sdis_1.0/ia_css_sdis.host.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/sdis_1.0/ia_css_sdis.host.c
@@ -265,9 +265,9 @@ ia_css_translate_dvs_statistics(
assert(isp_stats->hor_proj != NULL);
assert(isp_stats->ver_proj != NULL);
 
-   IA_CSS_ENTER("hproj=%p, vproj=%p, haddr=%x, vaddr=%x",
-   host_stats->hor_proj, host_stats->ver_proj,
-   isp_stats->hor_proj, isp_stats->ver_proj);
+   IA_CSS_ENTER("hproj=%p, vproj=%p, haddr=%p, vaddr=%p",
+host_stats->hor_proj, host_stats->ver_proj,
+isp_stats->hor_proj, isp_stats->ver_proj);
 
hor_num_isp = host_stats->grid.aligned_height;
ver_num_isp = host_stats->grid.aligned_width;
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c
index 930061d48df7..5ac81f87bfa3 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c
@@ -213,7 +213,7 @@ ia_css_translate_dvs2_statistics(
 "hor_coefs.even_real=%p, hor_coefs.even_imag=%p, "
 "ver_coefs.odd_real=%p, ver_coefs.odd_imag=%p, "
 "ver_coefs.even_real=%p, ver_coefs.even_imag=%p, "
-"haddr=%x, vaddr=%x",
+"haddr=%p, vaddr=%p",
host_stats->hor_prod.odd_real, host_stats->hor_prod.odd_imag,
host_stats->hor_prod.even_real, host_stats->hor_prod.even_imag,
host_stats->ver_prod.odd_real, host_stats->ver_prod.odd_imag,
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c
index 804c19ab4485..222a7bd7f176 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c
@@ -55,7 +55,7 @@ ia_css_tnr_dump(
"tnr_coef", tnr->coef);
ia_css_debug_dtrace(level, "\t%-32s = %d\n",
"tnr_threshold_Y", tnr->threshold_Y);
-   ia_css_debug_dtrace(level, "\t%-32s = %d\n"
+   ia_css_debug_dtrace(level, "\t%-32s = %d\n&quo

[PATCH] treewide: Correct diffrent[iate] and banlance typos

[Joe Perches]

Add these misspellings to scripts/spelling.txt too

Signed-off-by: Joe Perches 
---
 drivers/media/dvb-frontends/drx39xyj/drx_dap_fasi.h | 2 +-
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c  | 2 +-
 drivers/net/ethernet/hisilicon/hns/hns_enet.c   | 2 +-
 drivers/net/ethernet/qlogic/qed/qed_int.c   | 2 +-
 drivers/net/ethernet/qlogic/qed/qed_main.c  | 2 +-
 drivers/net/ethernet/qlogic/qed/qed_sriov.c | 2 +-
 include/linux/mlx4/device.h | 2 +-
 scripts/spelling.txt| 3 +++
 8 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/media/dvb-frontends/drx39xyj/drx_dap_fasi.h 
b/drivers/media/dvb-frontends/drx39xyj/drx_dap_fasi.h
index 354ec07eae87..23ae72468025 100644
--- a/drivers/media/dvb-frontends/drx39xyj/drx_dap_fasi.h
+++ b/drivers/media/dvb-frontends/drx39xyj/drx_dap_fasi.h
@@ -70,7 +70,7 @@
 * (3) both long and short but short preferred and long only when necesarry
 *
 * These modes must be selected compile time via compile switches.
-* Compile switch settings for the diffrent modes:
+* Compile switch settings for the different modes:
 * (1) DRXDAPFASI_LONG_ADDR_ALLOWED=0, DRXDAPFASI_SHORT_ADDR_ALLOWED=1
 * (2) DRXDAPFASI_LONG_ADDR_ALLOWED=1, DRXDAPFASI_SHORT_ADDR_ALLOWED=0
 * (3) DRXDAPFASI_LONG_ADDR_ALLOWED=1, DRXDAPFASI_SHORT_ADDR_ALLOWED=1
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c 
b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c
index cea6bdcde33f..8baf9d3eb4b1 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c
@@ -1591,7 +1591,7 @@ static int __bnx2x_vlan_mac_execute_step(struct bnx2x *bp,
if (rc != 0) {
__bnx2x_vlan_mac_h_pend(bp, o, *ramrod_flags);
 
-   /* Calling function should not diffrentiate between this case
+   /* Calling function should not differentiate between this case
 * and the case in which there is already a pending ramrod
 */
rc = 1;
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c 
b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index fca37e2c7f01..e70324f4fe84 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -1207,7 +1207,7 @@ static void hns_set_irq_affinity(struct hns_nic_priv 
*priv)
if (!alloc_cpumask_var(&mask, GFP_KERNEL))
return;
 
-   /*diffrent irq banlance for 16core and 32core*/
+   /* different irq balance for 16core and 32core */
if (h->q_num == num_possible_cpus()) {
for (i = 0; i < h->q_num * 2; i++) {
rd = &priv->ring_data[i];
diff --git a/drivers/net/ethernet/qlogic/qed/qed_int.c 
b/drivers/net/ethernet/qlogic/qed/qed_int.c
index 84310b60849b..c6b348f00e7b 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_int.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_int.c
@@ -3057,7 +3057,7 @@ int qed_int_igu_read_cam(struct qed_hwfn *p_hwfn, struct 
qed_ptt *p_ptt)
 
/* There's a possibility the igu_sb_cnt_iov doesn't properly reflect
 * the number of VF SBs [especially for first VF on engine, as we can't
-* diffrentiate between empty entries and its entries].
+* differentiate between empty entries and its entries].
 * Since we don't really support more SBs than VFs today, prevent any
 * such configuration by sanitizing the number of SBs to equal the
 * number of VFs.
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c 
b/drivers/net/ethernet/qlogic/qed/qed_main.c
index d4edb993b1b0..b595f7dd4a58 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -951,7 +951,7 @@ static int qed_slowpath_start(struct qed_dev *cdev,
if (rc)
goto err2;
 
-   /* First Dword used to diffrentiate between various sources */
+   /* First Dword used to differentiate between various sources */
data = cdev->firmware->data + sizeof(u32);
 
qed_dbg_pf_init(cdev);
diff --git a/drivers/net/ethernet/qlogic/qed/qed_sriov.c 
b/drivers/net/ethernet/qlogic/qed/qed_sriov.c
index 18fc6e62ca41..a69774b19712 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_sriov.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_sriov.c
@@ -625,7 +625,7 @@ int qed_iov_hw_info(struct qed_hwfn *p_hwfn)
 *  - If !ARI, VFs would start on next device.
 *so offset - (256 - pf_id) would provide the number.
 * Utilize the fact that (256 - pf_id) is achieved only by later
-* to diffrentiate between the two.
+* to differentiate between the two.
 */
 
if (p_hwfn->cdev->p_iov_info->offset < (256 - p_hwfn->abs_pf_id)) {
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
ind

Re: [Outreachy kernel] [PATCH] staging: media: Remove unnecessary function and its call

[Joe Perches]

On Sun, 2017-03-05 at 10:14 -0800, Alison Schofield wrote:
> On Sun, Mar 05, 2017 at 12:17:21PM +0530, simran singhal wrote:
> > The function atomisp_set_stop_timeout on being called, simply returns
> > back. The function hasn't been mentioned in the TODO and doesn't have
> > FIXME code around. Hence, atomisp_set_stop_timeout and its calls have been
> > removed.
> > 
> > Signed-off-by: simran singhal 
> > ---
> 
> Hi Simran,
> 
> It's helpful to state right in the subject line what you removed.
> ie.  remove unused function atomisp_set_stop_timeout()
> 
> If you do that, scan's or grep'ing the git log pretty oneline's can 
> easily see this without having to dig into the log.
> 
> (gitpretty='git log --pretty=oneline --abbrev-commit')
> 
> Can you share to Outreachy group how you found this?  By inspection
> or otherwise??

At least for the rtl8192u patch submitted:

It's also helpful to read the comment you remove
and determine if what you are doing is the correct
thing to do and explain why it's OK in the commit
message. (fractured english below notwithstanding)

 /* These function were added to load crypte module autoly */
-   ieee80211_tkip_null();
 

Re: [PATCH 24/26] ocfs2: reduce stack size with KASAN

[Joe Perches]

On Thu, 2017-03-02 at 23:59 +0100, Arnd Bergmann wrote:
> KASAN decides that passing a pointer to _m into an extern function
> (_mlog_printk) is potentially dangerous, as that function might
> keep a reference to that pointer after it goes out of scope,
> or it might not know the correct length of the stack object pointed to.
> 
> We can see from looking at the __mlog_printk() function definition
> that it's actually safe, but the compiler cannot see that when looking
> at another source file.

OK, thanks.

btw:

changing __mlog_printk can save ~11% (90+KB) of object text size
by removing __func__ and __LINE__ and using vsprintf pointer extension
%pS, __builtin_return_address(0) as it is already used in dlmmaster.

(defconfig x86-64, with ocfs2)

$ size fs/ocfs2/built-in.o*
   textdata bss dec hex filename
 759791  111373  105688  976852   ee7d4 fs/ocfs2/built-in.o.new
 852959  111373  105688 1070020  1053c4 fs/ocfs2/built-in.o.old

It's nearly the same output.

---

 fs/ocfs2/cluster/masklog.c | 8 
 fs/ocfs2/cluster/masklog.h | 8 +++-
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/fs/ocfs2/cluster/masklog.c b/fs/ocfs2/cluster/masklog.c
index d331c2386b94..a3f080f37108 100644
--- a/fs/ocfs2/cluster/masklog.c
+++ b/fs/ocfs2/cluster/masklog.c
@@ -64,8 +64,7 @@ static ssize_t mlog_mask_store(u64 mask, const char *buf, 
size_t count)
return count;
 }
 
-void __mlog_printk(const u64 *mask, const char *func, int line,
-  const char *fmt, ...)
+void __mlog_printk(const u64 *mask, const char *fmt, ...)
 {
struct va_format vaf;
va_list args;
@@ -90,9 +89,10 @@ void __mlog_printk(const u64 *mask, const char *func, int 
line,
vaf.fmt = fmt;
vaf.va = &args;
 
-   printk("%s(%s,%u,%u):%s:%d %s%pV",
+   printk("%s(%s,%u,%u):%pS %s%pV",
   level, current->comm, task_pid_nr(current),
-  raw_smp_processor_id(), func, line, prefix, &vaf);
+  raw_smp_processor_id(), __builtin_return_address(0),
+  prefix, &vaf);
 
va_end(args);
 }
diff --git a/fs/ocfs2/cluster/masklog.h b/fs/ocfs2/cluster/masklog.h
index 3c16da69605d..56ba5baf625b 100644
--- a/fs/ocfs2/cluster/masklog.h
+++ b/fs/ocfs2/cluster/masklog.h
@@ -162,9 +162,8 @@ extern struct mlog_bits mlog_and_bits, mlog_not_bits;
 
 #endif
 
-__printf(4, 5) __nocapture(2)
-void __mlog_printk(const u64 *m, const char *func, int line,
-  const char *fmt, ...);
+__printf(2, 3) __nocapture(2)
+void __mlog_printk(const u64 *m, const char *fmt, ...);
 
 /*
  * Testing before the __mlog_printk call lets the compiler eliminate the
@@ -174,8 +173,7 @@ void __mlog_printk(const u64 *m, const char *func, int line,
 do {   \
u64 _m = MLOG_MASK_PREFIX | (mask); \
if (_m & ML_ALLOWED_BITS)   \
-   __mlog_printk(&_m, __func__, __LINE__, fmt, \
- ##__VA_ARGS__);   \
+   __mlog_printk(&_m, fmt, ##__VA_ARGS__); \
 } while (0)
 
 #define mlog_errno(st) ({  \

Re: [PATCH 24/26] ocfs2: reduce stack size with KASAN

[Joe Perches]

On Thu, 2017-03-02 at 23:22 +0100, Arnd Bergmann wrote:
> On Thu, Mar 2, 2017 at 6:46 PM, Joe Perches  wrote:
> > On Thu, 2017-03-02 at 17:38 +0100, Arnd Bergmann wrote:
> > > The internal logging infrastructure in ocfs2 causes special warning code 
> > > to be
> > > used with KASAN, which produces rather large stack frames:
> > > fs/ocfs2/super.c: In function 'ocfs2_fill_super':
> > > fs/ocfs2/super.c:1219:1: error: the frame size of 3264 bytes is larger 
> > > than 3072 bytes [-Werror=frame-larger-than=]
> > 
> > At least by default it doesn't seem to.
> > 
> > gcc 6.2 allyesconfig, CONFIG_KASAN=y
> > with either CONFIG_KASAN_INLINE or CONFIG_KASAN_OUTLINE
> > 
> > gcc doesn't emit a stack warning
> 
> The warning is disabled until patch 26/26. which picks the 3072 default.
> The 3264 number was with gcc-7, which is worse than gcc-6 since it enables
> an extra check.
> 
> > > By simply passing the mask by value instead of reference, we can avoid the
> > > problem completely.
> > 
> > Any idea why that's so?
> 
> With KASAN, every time we inline the function, the compiler has to allocate
> space for another copy of the variable plus a redzone to detect whether
> passing it by reference into another function causes an overflow at runtime.

These logging functions aren't inlined.
You're referring to the stack frame?

> > >  On 64-bit architectures, this is also more efficient,
> > 
> > Efficient true, but the same overall stack no?
> 
> Here is what I see with CONFIG_FRAME_WARN=300 and x86_64-linux-gcc-6.3.1:
> 
> before:
[]
> fs/ocfs2/super.c:1219:1: error: the frame size of 552 bytes is larger
> than 300 bytes [-Werror=frame-larger-than=]
> 
> after:
> fs/ocfs2/super.c: In function 'ocfs2_fill_super':
> fs/ocfs2/super.c:1219:1: error: the frame size of 472 bytes is larger
> than 300 bytes [-Werror=frame-larger-than=]
> 
> and with gcc-7.0.1 (including -fsanitize-address-use-after-scope), before:
[]
> fs/ocfs2/super.c:1219:1: error: the frame size of 3264 bytes is larger
> than 300 bytes [-Werror=frame-larger-than=]
> 
> after:
> fs/ocfs2/super.c: In function 'ocfs2_fill_super':
> fs/ocfs2/super.c:1219:1: error: the frame size of 704 bytes is larger
> than 300 bytes [-Werror=frame-larger-than=]

Still doesn't make sense to me.

None of the logging functions are inlined as they are all
EXPORT_SYMBOL.

This just changes a pointer to a u64, which is the same
size on x86-64 (and is of course larger on x86-32).

Perhaps KASAN has the odd behavior and working around
KASAN's behavior may not be the proper thing to do.

Maybe if CONFIG_KASAN is set, the minimum stack should
be increased via THREAD_SIZE_ORDER or some such.

Re: [PATCH 24/26] ocfs2: reduce stack size with KASAN

[Joe Perches]

On Thu, 2017-03-02 at 17:38 +0100, Arnd Bergmann wrote:
> The internal logging infrastructure in ocfs2 causes special warning code to be
> used with KASAN, which produces rather large stack frames:

> fs/ocfs2/super.c: In function 'ocfs2_fill_super':
> fs/ocfs2/super.c:1219:1: error: the frame size of 3264 bytes is larger than 
> 3072 bytes [-Werror=frame-larger-than=]

At least by default it doesn't seem to.

gcc 6.2 allyesconfig, CONFIG_KASAN=y
with either CONFIG_KASAN_INLINE or CONFIG_KASAN_OUTLINE

gcc doesn't emit a stack warning

> By simply passing the mask by value instead of reference, we can avoid the
> problem completely.

Any idea why that's so?
 
>  On 64-bit architectures, this is also more efficient,

Efficient true, but the same overall stack no?

> while on the less common (at least among ocfs2 users) 32-bit architectures,
> I'm guessing that the resulting code is comparable to what it was before.
> 
> The current version was introduced by Joe Perches as an optimization, maybe
> he can see if my change regresses compared to his.

I don't see it.

> Cc: Joe Perches 
> Fixes: 7c2bd2f930ae ("ocfs2: reduce object size of mlog uses")
> Signed-off-by: Arnd Bergmann 
> ---
>  fs/ocfs2/cluster/masklog.c | 10 +-
>  fs/o cfs2/cluster/masklog.h |  4 ++--
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/ocfs2/cluster/masklog.c b/fs/ocfs2/cluster/masklog.c
> index d331c2386b94..9720c5443e4d 100644
> --- a/fs/ocfs2/cluster/masklog.c
> +++ b/fs/ocfs2/cluster/masklog.c
> @@ -64,7 +64,7 @@ static ssize_t mlog_mask_store(u64 mask, const char *buf, 
> size_t count)
>   return count;
>  }
>  
> -void __mlog_printk(const u64 *mask, const char *func, int line,
> +void __mlog_printk(const u64 mask, const char *func, int line,
>  const char *fmt, ...)
>  {
>   struct va_format vaf;
> @@ -72,14 +72,14 @@ void __mlog_printk(const u64 *mask, const char *func, int 
> line,
>   const char *level;
>   const char *prefix = "";
>  
> - if (!__mlog_test_u64(*mask, mlog_and_bits) ||
> - __mlog_test_u64(*mask, mlog_not_bits))
> + if (!__mlog_test_u64(mask, mlog_and_bits) ||
> + __mlog_test_u64(mask, mlog_not_bits))
>   return;
>  
> - if (*mask & ML_ERROR) {
> + if (mask & ML_ERROR) {
>   level = KERN_ERR;
>   prefix = "ERROR: ";
> - } else if (*mask & ML_NOTICE) {
> + } else if (mask & ML_NOTICE) {
>   level = KERN_NOTICE;
>   } else {
>   level = KERN_INFO;
> diff --git a/fs/ocfs2/cluster/masklog.h b/fs/ocfs2/cluster/masklog.h
> index 308ea0eb35fd..0d0f4bf2c3d8 100644
> --- a/fs/ocfs2/cluster/masklog.h
> +++ b/fs/ocfs2/cluster/masklog.h
> @@ -163,7 +163,7 @@ extern struct mlog_bits mlog_and_bits, mlog_not_bits;
>  #endif
>  
>  __printf(4, 5)
> -void __mlog_printk(const u64 *m, const char *func, int line,
> +void __mlog_printk(const u64 m, const char *func, int line,
>  const char *fmt, ...);
>  
>  /*
> @@ -174,7 +174,7 @@ void __mlog_printk(const u64 *m, const char *func, int 
> line,
>  do { \
>   u64 _m = MLOG_MASK_PREFIX | (mask); \
>   if (_m & ML_ALLOWED_BITS)   \
> - __mlog_printk(&_m, __func__, __LINE__, fmt, \
> + __mlog_printk(_m, __func__, __LINE__, fmt,  \
> ##__VA_ARGS__);   \
>  } while (0)
>  

Re: [PATCH 00/35] treewide trivial patches converting pr_warning to pr_warn

[Joe Perches]

On Thu, 2017-02-23 at 17:41 +, Emil Velikov wrote:
> On 23 February 2017 at 17:18, Joe Perches  wrote:
> > On Thu, 2017-02-23 at 09:28 -0600, Rob Herring wrote:
> > > On Fri, Feb 17, 2017 at 1:11 AM, Joe Perches  wrote:
> > > > There are ~4300 uses of pr_warn and ~250 uses of the older
> > > > pr_warning in the kernel source tree.
> > > > 
> > > > Make the use of pr_warn consistent across all kernel files.
> > > > 
> > > > This excludes all files in tools/ as there is a separate
> > > > define pr_warning for that directory tree and pr_warn is
> > > > not used in tools/.
> > > > 
> > > > Done with 'sed s/\bpr_warning\b/pr_warn/' and some emacsing.
> > 
> > []
> > > Where's the removal of pr_warning so we don't have more sneak in?
> > 
> > After all of these actually get applied,
> > and maybe a cycle or two later, one would
> > get sent.
> > 
> 
> By which point you'll get a few reincarnation of it. So you'll have to
> do the same exercise again :-(

Maybe to one or two files.  Not a big deal.

> I guess the question is - are you expecting to get the series merged
> all together/via one tree ?

No.  The only person that could do that effectively is Linus.

> If not, your plan is perfectly reasonable.

Re: [PATCH 00/35] treewide trivial patches converting pr_warning to pr_warn

[Joe Perches]

On Thu, 2017-02-23 at 09:28 -0600, Rob Herring wrote:
> On Fri, Feb 17, 2017 at 1:11 AM, Joe Perches  wrote:
> > There are ~4300 uses of pr_warn and ~250 uses of the older
> > pr_warning in the kernel source tree.
> > 
> > Make the use of pr_warn consistent across all kernel files.
> > 
> > This excludes all files in tools/ as there is a separate
> > define pr_warning for that directory tree and pr_warn is
> > not used in tools/.
> > 
> > Done with 'sed s/\bpr_warning\b/pr_warn/' and some emacsing.
[]
> Where's the removal of pr_warning so we don't have more sneak in?

After all of these actually get applied,
and maybe a cycle or two later, one would
get sent.

[PATCH 00/35] treewide trivial patches converting pr_warning to pr_warn

[Joe Perches]

There are ~4300 uses of pr_warn and ~250 uses of the older
pr_warning in the kernel source tree.

Make the use of pr_warn consistent across all kernel files.

This excludes all files in tools/ as there is a separate
define pr_warning for that directory tree and pr_warn is
not used in tools/.

Done with 'sed s/\bpr_warning\b/pr_warn/' and some emacsing.

Miscellanea:

o Coalesce formats and realign arguments

Some files not compiled - no cross-compilers

Joe Perches (35):
  alpha: Convert remaining uses of pr_warning to pr_warn
  ARM: ep93xx: Convert remaining uses of pr_warning to pr_warn
  arm64: Convert remaining uses of pr_warning to pr_warn
  arch/blackfin: Convert remaining uses of pr_warning to pr_warn
  ia64: Convert remaining use of pr_warning to pr_warn
  powerpc: Convert remaining uses of pr_warning to pr_warn
  sh: Convert remaining uses of pr_warning to pr_warn
  sparc: Convert remaining use of pr_warning to pr_warn
  x86: Convert remaining uses of pr_warning to pr_warn
  drivers/acpi: Convert remaining uses of pr_warning to pr_warn
  block/drbd: Convert remaining uses of pr_warning to pr_warn
  gdrom: Convert remaining uses of pr_warning to pr_warn
  drivers/char: Convert remaining use of pr_warning to pr_warn
  clocksource: Convert remaining use of pr_warning to pr_warn
  drivers/crypto: Convert remaining uses of pr_warning to pr_warn
  fmc: Convert remaining use of pr_warning to pr_warn
  drivers/gpu: Convert remaining uses of pr_warning to pr_warn
  drivers/ide: Convert remaining uses of pr_warning to pr_warn
  drivers/input: Convert remaining uses of pr_warning to pr_warn
  drivers/isdn: Convert remaining uses of pr_warning to pr_warn
  drivers/macintosh: Convert remaining uses of pr_warning to pr_warn
  drivers/media: Convert remaining use of pr_warning to pr_warn
  drivers/mfd: Convert remaining uses of pr_warning to pr_warn
  drivers/mtd: Convert remaining uses of pr_warning to pr_warn
  drivers/of: Convert remaining uses of pr_warning to pr_warn
  drivers/oprofile: Convert remaining uses of pr_warning to pr_warn
  drivers/platform: Convert remaining uses of pr_warning to pr_warn
  drivers/rapidio: Convert remaining use of pr_warning to pr_warn
  drivers/scsi: Convert remaining use of pr_warning to pr_warn
  drivers/sh: Convert remaining use of pr_warning to pr_warn
  drivers/tty: Convert remaining uses of pr_warning to pr_warn
  drivers/video: Convert remaining uses of pr_warning to pr_warn
  kernel/trace: Convert remaining uses of pr_warning to pr_warn
  lib: Convert remaining uses of pr_warning to pr_warn
  sound/soc: Convert remaining uses of pr_warning to pr_warn

 arch/alpha/kernel/perf_event.c |  4 +-
 arch/arm/mach-ep93xx/core.c|  4 +-
 arch/arm64/include/asm/syscall.h   |  8 ++--
 arch/arm64/kernel/hw_breakpoint.c  |  8 ++--
 arch/arm64/kernel/smp.c|  4 +-
 arch/blackfin/kernel/nmi.c |  2 +-
 arch/blackfin/kernel/ptrace.c  |  2 +-
 arch/blackfin/mach-bf533/boards/stamp.c|  2 +-
 arch/blackfin/mach-bf537/boards/cm_bf537e.c|  2 +-
 arch/blackfin/mach-bf537/boards/cm_bf537u.c|  2 +-
 arch/blackfin/mach-bf537/boards/stamp.c|  2 +-
 arch/blackfin/mach-bf537/boards/tcm_bf537.c|  2 +-
 arch/blackfin/mach-bf561/boards/cm_bf561.c |  2 +-
 arch/blackfin/mach-bf561/boards/ezkit.c|  2 +-
 arch/blackfin/mm/isram-driver.c|  4 +-
 arch/ia64/kernel/setup.c   |  6 +--
 arch/powerpc/kernel/pci-common.c   |  4 +-
 arch/powerpc/mm/init_64.c  |  5 +--
 arch/powerpc/mm/mem.c  |  3 +-
 arch/powerpc/platforms/512x/mpc512x_shared.c   |  4 +-
 arch/powerpc/platforms/85xx/socrates_fpga_pic.c|  7 ++--
 arch/powerpc/platforms/86xx/mpc86xx_hpcn.c |  2 +-
 arch/powerpc/platforms/pasemi/dma_lib.c|  4 +-
 arch/powerpc/platforms/powernv/opal.c  |  8 ++--
 arch/powerpc/platforms/powernv/pci-ioda.c  | 10 ++---
 arch/powerpc/platforms/ps3/device-init.c   | 14 +++
 arch/powerpc/platforms/ps3/mm.c|  4 +-
 arch/powerpc/platforms/ps3/os-area.c   |  2 +-
 arch/powerpc/platforms/pseries/iommu.c |  8 ++--
 arch/powerpc/platforms/pseries/setup.c |  4 +-
 arch/powerpc/sysdev/fsl_pci.c  |  9 ++---
 arch/powerpc/sysdev/mpic.c | 10 ++---
 arch/powerpc/sysdev/xics/icp-native.c  | 10 ++---
 arch/powerpc/sysdev/xics/ics-opal.c|  4 +-
 arch/powerpc/sysdev/xics/ics-rtas.c|  4 +-
 arch/powerpc/sysdev/xics/xics-common.c |  8 ++--
 arch/sh/boards/mach-sdk7786/nmi.c  |  2 +-
 arch/sh/drivers/pci/fixups-sdk7786.c   |  2 +-
 arch/sh/kernel/io

[PATCH 22/35] drivers/media: Convert remaining use of pr_warning to pr_warn

[Joe Perches]

To enable eventual removal of pr_warning

This makes pr_warn use consistent for drivers/media

Prior to this patch, there was 1 use of pr_warning and
310 uses of pr_warn in drivers/media

Signed-off-by: Joe Perches 
---
 drivers/media/platform/sh_vou.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/sh_vou.c b/drivers/media/platform/sh_vou.c
index ef2a519bcd4c..992d61a8b961 100644
--- a/drivers/media/platform/sh_vou.c
+++ b/drivers/media/platform/sh_vou.c
@@ -813,8 +813,8 @@ static u32 sh_vou_ntsc_mode(enum sh_vou_bus_fmt bus_fmt)
 {
switch (bus_fmt) {
default:
-   pr_warning("%s(): Invalid bus-format code %d, using default 
8-bit\n",
-  __func__, bus_fmt);
+   pr_warn("%s(): Invalid bus-format code %d, using default 
8-bit\n",
+   __func__, bus_fmt);
case SH_VOU_BUS_8BIT:
return 1;
case SH_VOU_BUS_16BIT:
-- 
2.10.0.rc2.1.g053435c

Re: [PATCH v2] media: s5p_mfc print buf pointer in hex constistently

[Joe Perches]

On Fri, 2017-02-10 at 08:40 -0700, Shuah Khan wrote:
> Fix s5p_mfc_set_dec_frame_buffer_v6() to print buffer pointer in hex to be
> consistent with the rest of the messages in the routine.

More curiously, why is buf_addr a size_t and not
a dma_addr_t?

> Signed-off-by: Shuah Khan 
> ---
> 
> Fixed commit log. No code changes. Thanks for the catch.
> 
>  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c 
> b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> index d6f207e..fc45980 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> @@ -497,7 +497,7 @@ static int s5p_mfc_set_dec_frame_buffer_v6(struct 
> s5p_mfc_ctx *ctx)
>   }
>   }
>  
> - mfc_debug(2, "Buf1: %zu, buf_size1: %d (frames %d)\n",
> + mfc_debug(2, "Buf1: %zx, buf_size1: %d (frames %d)\n",
>   buf_addr1, buf_size1, ctx->total_dpb_count);
>   if (buf_size1 < 0) {
>   mfc_debug(2, "Not enough memory has been allocated.\n");
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] staging: bcm2835-v4l2: Apply spelling fixes from checkpatch.

[Joe Perches]

On Tue, 2017-01-31 at 10:30 -0800, Eric Anholt wrote:
> Joe Perches  writes:
> 
> > On Mon, 2017-01-30 at 12:05 -0800, Eric Anholt wrote:
> > > Joe Perches  writes:
> > > 
> > > > On Fri, 2017-01-27 at 13:55 -0800, Eric Anholt wrote:
> > > > > Generated with checkpatch.pl --fix-inplace and git add -p out of the
> > > > > results.
> > > > 
> > > > Maybe another.
> > > > 
> > > > > diff --git a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c 
> > > > > b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
> > > > 
> > > > []
> > > > > @@ -239,7 +239,7 @@ static int bulk_receive(struct 
> > > > > vchiq_mmal_instance *instance,
> > > > >   pr_err("buffer list empty trying to submit bulk 
> > > > > receive\n");
> > > > >  
> > > > >   /* todo: this is a serious error, we should never have
> > > > > -  * commited a buffer_to_host operation to the mmal
> > > > > +  * committed a buffer_to_host operation to the mmal
> > > > >* port without the buffer to back it up (underflow
> > > > >* handling) and there is no obvious way to deal with
> > > > >* this - how is the mmal servie going to react when
> > > > 
> > > > Perhaps s/servie/service/ ?
> > > 
> > > I was trying to restrict this patch to just the fixes from checkpatch.
> > 
> > That's the wrong thing to do if you're fixing
> > spelling defects.  checkpatch is just one mechanism
> > to identify some, and definitely not all, typos and
> > spelling defects.
> > 
> > If you fixing, fix.  Don't just rely on the brainless
> > tools, use your decidedly non-mechanical brain.
> 
> "if you touch anything, you must fix everything."  If that's how things
> work, I would just retract the patch.

I didn't say that,and I don't mean that.

If you notice a similar defect when you are fixing
any arbitrary defect, please try to fix all of similar
defects.

As is, a patch that fixes just servie would cause a
patch conflict with your patch.

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] staging: bcm2835-v4l2: Apply spelling fixes from checkpatch.

[Joe Perches]

On Mon, 2017-01-30 at 12:05 -0800, Eric Anholt wrote:
> Joe Perches  writes:
> 
> > On Fri, 2017-01-27 at 13:55 -0800, Eric Anholt wrote:
> > > Generated with checkpatch.pl --fix-inplace and git add -p out of the
> > > results.
> > 
> > Maybe another.
> > 
> > > diff --git a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c 
> > > b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
> > 
> > []
> > > @@ -239,7 +239,7 @@ static int bulk_receive(struct vchiq_mmal_instance 
> > > *instance,
> > >   pr_err("buffer list empty trying to submit bulk receive\n");
> > >  
> > >   /* todo: this is a serious error, we should never have
> > > -  * commited a buffer_to_host operation to the mmal
> > > +  * committed a buffer_to_host operation to the mmal
> > >* port without the buffer to back it up (underflow
> > >* handling) and there is no obvious way to deal with
> > >* this - how is the mmal servie going to react when
> > 
> > Perhaps s/servie/service/ ?
> 
> I was trying to restrict this patch to just the fixes from checkpatch.

That's the wrong thing to do if you're fixing
spelling defects.  checkpatch is just one mechanism
to identify some, and definitely not all, typos and
spelling defects.

If you fixing, fix.  Don't just rely on the brainless
tools, use your decidedly non-mechanical brain.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] staging: bcm2835-v4l2: Apply spelling fixes from checkpatch.

[Joe Perches]

On Fri, 2017-01-27 at 13:55 -0800, Eric Anholt wrote:
> Generated with checkpatch.pl --fix-inplace and git add -p out of the
> results.

Maybe another.

> diff --git a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c 
> b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
[]
> @@ -239,7 +239,7 @@ static int bulk_receive(struct vchiq_mmal_instance 
> *instance,
>   pr_err("buffer list empty trying to submit bulk receive\n");
>  
>   /* todo: this is a serious error, we should never have
> -  * commited a buffer_to_host operation to the mmal
> +  * committed a buffer_to_host operation to the mmal
>* port without the buffer to back it up (underflow
>* handling) and there is no obvious way to deal with
>* this - how is the mmal servie going to react when

Perhaps s/servie/service/ ?


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/18] [media] RedRat3: Use kcalloc() in two functions

[Joe Perches]

On Thu, 2016-10-13 at 18:18 +0200, SF Markus Elfring wrote:
> diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c
[]
> @@ -549,7 +549,7 @@ static void redrat3_get_firmware_rev(struct redrat3_dev 
> *rr3)
>   int rc = 0;
>   char *buffer;
>  
> - buffer = kzalloc(sizeof(char) * (RR3_FW_VERSION_LEN + 1), GFP_KERNEL);
> + buffer = kcalloc(RR3_FW_VERSION_LEN + 1, sizeof(*buffer), GFP_KERNEL);
>   if (!buffer) {
>   dev_err(rr3->dev, "Memory allocation failure\n");
>   return;,

Markus, please stop being _so_ mechanical and use your
brain a little too.  By definition, sizeof(char) == 1.

This _really_ should be kzalloc(RR3_FW_VERSION_LEN + 1,...)

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/15] improve function-level documentation

[Joe Perches]

On Sat, 2016-10-01 at 21:46 +0200, Julia Lawall wrote:
> These patches fix cases where the documentation above a function definition
> is not consistent with the function header.  Issues are detected using the
> semantic patch below (http://coccinelle.lip6.fr/).  Basically, the semantic
> patch parses a file to find comments, then matches each function header,
> and checks that the name and parameter list in the function header are
> compatible with the comment that preceeds it most closely.

Hi Julia.

Would it be possible for a semantic patch to scan for
function definitions where the types do not have
identifiers and update the definitions to match the
declarations?

For instance, given:


int foo(int);


int foo(int bar)
{
return baz;
}

Could coccinelle output:

diff a/some.h b/some.h
[]
-int foo(int);
+int foo(int bar);
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/26] constify local structures

[Joe Perches]

On Sun, 2016-09-11 at 15:05 +0200, Julia Lawall wrote:
> Constify local structures.

Thanks Julia.

A few suggestions & questions:

Perhaps the script should go into scripts/coccinelle/
so that future cases could be caught by the robot
and commit message referenced by the patch instances.

Can you please compile the files modified using the
appropriate defconfig/allyesconfig and show the
movement from data to const by using
$ size .new/old
and include that in the changelogs (maybe next time)?

Is it possible for a rule to trace the instances where
an address of a struct or struct member is taken by
locally defined and declared function call where the
callee does not modify any dereferenced object?

ie:

struct foo {
int bar;
char *baz;
};

struct foo qux[] = {
{ 1, "description 1" },
{ 2, "dewcription 2" },
[ n, "etc" ]...,
};

void message(struct foo *msg)
{
printk("%d %s\n", msg->bar, msg->baz);
}

where some code uses

message(qux[index]);

So could a coccinelle script change:

struct foo qux[] = { to const struct foo quz[] = {

and

void message(struct foo *msg) to void message(const struct foo *msg)

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL for v4.8-rc1] mailcap fixup for two entries

[Joe Perches]

On Sat, 2016-08-06 at 07:35 -0300, Mauro Carvalho Chehab wrote:
> Hi Linus,
> 
> Please pull from my tree for a small fixup on my entry and Shuah's entry at
> .mailcap.
> 
> Basically, those entries were with a syntax that makes get_maintainer.pl to
> do the wrong thing.
> 
> Thanks!
> Mauro

.mailmap

The old entries were simply improper.

git shortlog wasn't doing the right thing either.

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: s5p-mfc Fix misspelled error message and checkpatch errors

[Joe Perches]

On Tue, 2016-07-12 at 10:08 -0600, Shuah Khan wrote:
> On 07/12/2016 09:51 AM, Joe Perches wrote:
> > On Tue, 2016-07-12 at 08:42 -0600, Shuah Khan wrote:
> > > On 07/12/2016 08:06 AM, Joe Perches wrote:
> > > > On Mon, 2016-07-11 at 16:39 -0600, Shuah Khan wrote:
> > > > > 
> > > > > Fix misspelled error message and existing checkpatch errors in the
> > > > > error message conditional.
> > > > []
> > > > > 
> > > > > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c 
> > > > > b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> > > > []
> > > > > 
> > > > > @@ -775,11 +775,11 @@ static int vidioc_g_crop(struct file *file, 
> > > > > void *priv,
> > > > >   u32 left, right, top, bottom;
> > > > >  
> > > > >   if (ctx->state != MFCINST_HEAD_PARSED &&
> > > > > - ctx->state != MFCINST_RUNNING && ctx->state != MFCINST_FINISHING
> > > > > - && ctx->state != 
> > > > > MFCINST_FINISHED) {
> > > > > - mfc_err("Cannont set crop\n");
> > > > > - return -EINVAL;
> > > > > - }
> > > > > + ctx->state != MFCINST_RUNNING && ctx->state != 
> > > > > MFCINST_FINISHING
> > > > > + && ctx->state != MFCINST_FINISHED) {
> > > > > + mfc_err("Can not get crop information\n");
> > > > > + return -EINVAL;
> > > > > + }
> > > > is it a set or a get?
> > > vidioc_g_crop is a get routine.
> > > > 
> > > > 
> > > > It'd be nicer for humans to read if the alignment was consistent
> > > Are you okay with this alignment change or would you like it
> > > changed?
> > Well, if you're resubmitting, I'd prefer it changed.
> > Thanks.
> > 
> chekcpatch stopped complaining. Are you looking for the entire file
> alignments changed? I am not clear on what needs to be changed?

I think doing just this spelling and get/set correction and
fixing the alignment in this single case in a single patch
would be fine here.

Foxing the alignment for the entire file would be a more
significant change and isn't necessary in this patch.

Another thing possible for the file would be to change the
mfc_debug and mfc_err/mfc_info macros to use pr_
without the generally unnecessary __func__ and __LINE__
uses.

This could both enable dynamic_debug uses for the KERN_DEBUG
cases and reduce overall object size.

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: s5p-mfc Fix misspelled error message and checkpatch errors

[Joe Perches]

On Tue, 2016-07-12 at 08:42 -0600, Shuah Khan wrote:
> On 07/12/2016 08:06 AM, Joe Perches wrote:
> > On Mon, 2016-07-11 at 16:39 -0600, Shuah Khan wrote:
> > > Fix misspelled error message and existing checkpatch errors in the
> > > error message conditional.
> > []
> > > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c 
> > > b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> > []
> > > @@ -775,11 +775,11 @@ static int vidioc_g_crop(struct file *file, void 
> > > *priv,
> > >   u32 left, right, top, bottom;
> > >  
> > >   if (ctx->state != MFCINST_HEAD_PARSED &&
> > > - ctx->state != MFCINST_RUNNING && ctx->state != MFCINST_FINISHING
> > > - && ctx->state != MFCINST_FINISHED) {
> > > - mfc_err("Cannont set crop\n");
> > > - return -EINVAL;
> > > - }
> > > + ctx->state != MFCINST_RUNNING && ctx->state != MFCINST_FINISHING
> > > + && ctx->state != MFCINST_FINISHED) {
> > > + mfc_err("Can not get crop information\n");
> > > + return -EINVAL;
> > > + }
> > is it a set or a get?
> vidioc_g_crop is a get routine.
> > 
> > It'd be nicer for humans to read if the alignment was consistent
> Are you okay with this alignment change or would you like it
> changed?

Well, if you're resubmitting, I'd prefer it changed.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: s5p-mfc Fix misspelled error message and checkpatch errors

[Joe Perches]

On Mon, 2016-07-11 at 16:39 -0600, Shuah Khan wrote:
> Fix misspelled error message and existing checkpatch errors in the
> error message conditional.
[]
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c 
> b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
[]
> @@ -775,11 +775,11 @@ static int vidioc_g_crop(struct file *file, void *priv,
>   u32 left, right, top, bottom;
>  
>   if (ctx->state != MFCINST_HEAD_PARSED &&
> - ctx->state != MFCINST_RUNNING && ctx->state != MFCINST_FINISHING
> - && ctx->state != MFCINST_FINISHED) {
> - mfc_err("Cannont set crop\n");
> - return -EINVAL;
> - }
> + ctx->state != MFCINST_RUNNING && ctx->state != MFCINST_FINISHING
> + && ctx->state != MFCINST_FINISHED) {
> + mfc_err("Can not get crop information\n");
> + return -EINVAL;
> + }

is it a set or a get?

It'd be nicer for humans to read if the alignment was consistent

if (ctx->state != MFCINST_HEAD_PARSED &&
    ctx->state != MFCINST_RUNNING &&
    ctx->state != MFCINST_FINISHING &&
    ctx->state != MFCINST_FINISHED) {
mfc_err("Can not get crop information\n");
return -EINVAL;
}


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] [media] pci: Add tw5864 driver

[Joe Perches]

On Mon, 2016-07-11 at 18:17 +0300, Andrey Utkin wrote:
[]
> diff --git a/drivers/media/pci/tw5864/tw5864-core.c 
> b/drivers/media/pci/tw5864/tw5864-core.c
[]
> +static const char * const artifacts_warning =
> +"BEWARE OF KNOWN ISSUES WITH VIDEO QUALITY\n"
> +"\n"
> +"This driver was developed by Bluecherry LLC by deducing behaviour of\n"
> +"original manufacturer's driver, from both source code and execution 
> traces.\n"
> +"It is known that there are some artifacts on output video with this 
> driver:\n"
> +" - on all known hardware samples: random pixels of wrong color (mostly\n"
> +"   white, red or blue) appearing and disappearing on sequences of 
> P-frames;\n"
> +" - on some hardware samples (known with H.264 core version e006:2800):\n"
> +"   total madness on P-frames: blocks of wrong luminance; blocks of wrong\n"
> +"   colors \"creeping\" across the picture.\n"
> +"There is a workaround for both issues: avoid P-frames by setting GOP size\n"
> +"to 1. To do that, run this command on device files created by this 
> driver:\n"
> +"\n"
> +"v4l2-ctl --device /dev/videoX --set-ctrl=video_gop_size=1\n"
> +"\n";
> +
> +static char *artifacts_warning_continued =
> +"These issues are not decoding errors; all produced H.264 streams are 
> decoded\n"
> +"properly. Streams without P-frames don't have these artifacts so it's not\n"
> +"analog-to-digital conversion issues nor internal memory errors; we 
> conclude\n"
> +"it's internal H.264 encoder issues.\n"
> +"We cannot even check the original driver's behaviour because it has never\n"
> +"worked properly at all in our development environment. So these issues 
> may\n"
> +"be actually related to firmware or hardware. However it may be that 
> there's\n"
> +"just some more register settings missing in the driver which would please\n"
> +"the hardware.\n"
> +"Manufacturer didn't help much on our inquiries, but feel free to disturb\n"
> +"again the support of Intersil (owner of former Techwell).\n"
> +"\n";
[]
> +static int tw5864_initdev(struct pci_dev *pci_dev,
> +   const struct pci_device_id *pci_id)
> +{
[]
> + dev_warn(&pci_dev->dev, "%s", artifacts_warning);
> + dev_warn(&pci_dev->dev, "%s", artifacts_warning_continued);

Is all that verbosity useful?

And trivially:

Each of these blocks will start with the dev_ prefix
and the subsequent lines will not have the same prefix

Perhaps it'd be better to write this something like:

static const char * const artifacts_warning[] = {
"BEWARE OF KNOWN ISSUES WITH VIDEO QUALITY",
"",
"This driver was developed by Bluecherry LLC by deducing behaviour of",
"original manufacturer's driver, from both source code and execution 
traces.",
"It is known that there are some artifacts on output video with this 
driver:",
" - on all known hardware samples: random pixels of wrong color 
(mostly",
"   white, red or blue) appearing and disappearing on sequences of 
P-frames;",
" - on some hardware samples (known with H.264 core version 
e006:2800):",
"   total madness on P-frames: blocks of wrong luminance; blocks of 
wrong",
"   colors \"creeping\" across the picture.",
"There is a workaround for both issues: avoid P-frames by setting GOP 
size",
"to 1. To do that, run this command on device files created by this 
driver:",
"",
"v4l2-ctl --device /dev/videoX --set-ctrl=video_gop_size=1",
"",
"These issues are not decoding errors; all produced H.264 streams are 
decoded",
"properly. Streams without P-frames don't have these artifacts so it's 
not",
"analog-to-digital conversion issues nor internal memory errors; we 
conclude",
"it's internal H.264 encoder issues.",
"We cannot even check the original driver's behaviour because it has 
never",
"worked properly at all in our development environment. So these issues 
may",
"be actually related to firmware or hardware. However it may be that 
there's",
"just some more register settings missing in the driver which would 
please",
"the hardware.",
"Manufacturer didn't help much on our inquiries, but feel free to 
disturb",
"again the support of Intersil (owner of former Techwell).\n"
};

and use

for (i = 0; i < ARRAY_SIZE(artifacts_warning), i++)
dev_warn(&pci_dev->dev, %s\n", artifacts_warning[i]);

so that each line is prefixed.

It also might be better to issue something like a single
line dev_warn referring to the driver code and just leave
this comment in the driver sources.

Something like:

dev_warn(&pci_dev->dev,
"This driver has known defects in video quality\n");

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/15] lirc_dev: replace printk with pr_* or dev_*

[Joe Perches]

On Wed, 2016-06-29 at 22:20 +0900, Andi Shyti wrote:
> This patch mutes also all the checkpatch warnings related to
> printk.
> 
> Reword all the printouts so that the string doesn't need to be
> split, which fixes the following checkpatch warning:

Adding

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

before any #include would allow these to be prefixed
automatically and allow the embedded prefixes to be removed.
> diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
[]
> @@ -240,59 +240,51 @@ static int lirc_allocate_driver(struct lirc_driver *d)
>   int err;
>  
>   if (!d) {
> - printk(KERN_ERR "lirc_dev: lirc_register_driver: "
> -    "driver pointer must be not NULL!\n");
> + pr_err("lirc_dev: driver pointer must be not NULL!\n");
>   err = -EBADRQC;
>   goto out;
>   }

pr_err("driver pointer must not be NULL!\n");

And typical multiple line statement alignment is to
the open parenthesis.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [very-RFC 5/8] Add TSN machinery to drive the traffic from a shim over the network

[Joe Perches]

On Sun, 2016-06-12 at 00:22 +0200, Henrik Austad wrote:
> From: Henrik Austad 
> 
> In short summary:
> 
> * tsn_core.c is the main driver of tsn, all new links go through
>   here and all data to/form the shims are handled here
>   core also manages the shim-interface.
[]
> diff --git a/net/tsn/tsn_configfs.c b/net/tsn/tsn_configfs.c
[]
> +static inline struct tsn_link *to_tsn_link(struct config_item *item)
> +{
> + /* this line causes checkpatch to WARN. making checkpatch happy,
> +  * makes code messy..
> +  */
> + return item ? container_of(to_config_group(item), struct tsn_link, 
> group) : NULL;
> +}

How about

static inline struct tsn_link *to_tsn_link(struct config_item *item)
{
if (!item)
return NULL;
return container_of(to_config_group(item), struct tsn_link, group);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/4] media: Add Media Device Allocator API

[Joe Perches]

On Fri, 2016-03-25 at 22:38 -0600, Shuah Khan wrote:
> Add Media Device Allocator API to manage Media Device life time problems.
> There are known problems with media device life time management. When media
> device is released while an media ioctl is in progress, ioctls fail with
> use-after-free errors and kernel hangs in some cases.

Seems reasonable, thanks.

trivial:

> diff --git a/drivers/media/media-dev-allocator.c 
> b/drivers/media/media-dev-allocator.c
[]
> +static struct media_device *__media_device_get(struct device *dev,
> +    bool alloc, bool kref)
> +{
[]
> + pr_info("%s: mdev=%p\n", __func__, &mdi->mdev);

All of the pr_info uses here seem like debugging
and should likely be pr_debug instead.
> +struct media_device *media_device_find(struct device *dev)
> +{
> + pr_info("%s\n", __func__);

These seem like function tracing and maybe could/should
use ftrace instead.
+/* don't allocate - increment kref if one is found */
> +struct media_device *media_device_get_ref(struct device *dev)
> +{
> + pr_info("%s\n", __func__);

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add tw5864 driver

[Joe Perches]

On Mon, 2016-03-14 at 03:59 +0200, Andrey Utkin wrote:
> Support for boards based on Techwell TW5864 chip which provides
> multichannel video & audio grabbing and encoding (H.264, MJPEG,
> ADPCM G.726).

trivia:

Perhaps all the __used arrays could be const

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[TRIVIAL PATCH] treewide: Remove unnecessary 0x prefixes before %pa extension uses

[Joe Perches]

Since commit 3cab1e711297 ("lib/vsprintf: refactor duplicate code
to special_hex_number()") %pa uses have been ouput with a 0x prefix.

These 0x prefixes in the formats are unnecessary.

Signed-off-by: Joe Perches 
---
 drivers/dma/at_hdmac_regs.h  | 2 +-
 drivers/media/platform/ti-vpe/cal.c  | 2 +-
 drivers/nvdimm/pmem.c| 2 +-
 drivers/rapidio/devices/rio_mport_cdev.c | 4 ++--
 drivers/rapidio/devices/tsi721.c | 8 
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/at_hdmac_regs.h b/drivers/dma/at_hdmac_regs.h
index 7f58f06..0474e4a 100644
--- a/drivers/dma/at_hdmac_regs.h
+++ b/drivers/dma/at_hdmac_regs.h
@@ -385,7 +385,7 @@ static void vdbg_dump_regs(struct at_dma_chan *atchan) {}
 static void atc_dump_lli(struct at_dma_chan *atchan, struct at_lli *lli)
 {
dev_crit(chan2dev(&atchan->chan_common),
-"  desc: s%pad d%pad ctrl0x%x:0x%x l0x%pad\n",
+"  desc: s%pad d%pad ctrl0x%x:0x%x l%pad\n",
 &lli->saddr, &lli->daddr,
 lli->ctrla, lli->ctrlb, &lli->dscr);
 }
diff --git a/drivers/media/platform/ti-vpe/cal.c 
b/drivers/media/platform/ti-vpe/cal.c
index 82001e6..7dce489 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -498,7 +498,7 @@ static inline void cal_runtime_put(struct cal_dev *dev)
 
 static void cal_quickdump_regs(struct cal_dev *dev)
 {
-   cal_info(dev, "CAL Registers @ 0x%pa:\n", &dev->res->start);
+   cal_info(dev, "CAL Registers @ %pa:\n", &dev->res->start);
print_hex_dump(KERN_INFO, "", DUMP_PREFIX_OFFSET, 16, 4,
   (__force const void *)dev->base,
   resource_size(dev->res), false);
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 8d0b546..eb619d1 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -172,7 +172,7 @@ static struct pmem_device *pmem_alloc(struct device *dev,
 
if (!devm_request_mem_region(dev, pmem->phys_addr, pmem->size,
dev_name(dev))) {
-   dev_warn(dev, "could not reserve region [0x%pa:0x%zx]\n",
+   dev_warn(dev, "could not reserve region [%pa:0x%zx]\n",
&pmem->phys_addr, pmem->size);
return ERR_PTR(-EBUSY);
}
diff --git a/drivers/rapidio/devices/rio_mport_cdev.c 
b/drivers/rapidio/devices/rio_mport_cdev.c
index a3369d1..211a67d 100644
--- a/drivers/rapidio/devices/rio_mport_cdev.c
+++ b/drivers/rapidio/devices/rio_mport_cdev.c
@@ -2223,7 +2223,7 @@ static void mport_mm_open(struct vm_area_struct *vma)
 {
struct rio_mport_mapping *map = vma->vm_private_data;
 
-rmcd_debug(MMAP, "0x%pad", &map->phys_addr);
+   rmcd_debug(MMAP, "%pad", &map->phys_addr);
kref_get(&map->ref);
 }
 
@@ -2231,7 +2231,7 @@ static void mport_mm_close(struct vm_area_struct *vma)
 {
struct rio_mport_mapping *map = vma->vm_private_data;
 
-rmcd_debug(MMAP, "0x%pad", &map->phys_addr);
+   rmcd_debug(MMAP, "%pad", &map->phys_addr);
mutex_lock(&map->md->buf_mutex);
kref_put(&map->ref, mport_release_mapping);
mutex_unlock(&map->md->buf_mutex);
diff --git a/drivers/rapidio/devices/tsi721.c b/drivers/rapidio/devices/tsi721.c
index b5b4556..4c20e99 100644
--- a/drivers/rapidio/devices/tsi721.c
+++ b/drivers/rapidio/devices/tsi721.c
@@ -1101,7 +1101,7 @@ static int tsi721_rio_map_inb_mem(struct rio_mport 
*mport, dma_addr_t lstart,
ibw_start = lstart & ~(ibw_size - 1);
 
tsi_debug(IBW, &priv->pdev->dev,
-   "Direct (RIO_0x%llx -> PCIe_0x%pad), size=0x%x, 
ibw_start = 0x%llx",
+   "Direct (RIO_0x%llx -> PCIe_%pad), size=0x%x, ibw_start 
= 0x%llx",
rstart, &lstart, size, ibw_start);
 
while ((lstart + size) > (ibw_start + ibw_size)) {
@@ -1120,7 +1120,7 @@ static int tsi721_rio_map_inb_mem(struct rio_mport 
*mport, dma_addr_t lstart,
 
} else {
tsi_debug(IBW, &priv->pdev->dev,
-   "Translated (RIO_0x%llx -> PCIe_0x%pad), size=0x%x",
+   "Translated (RIO_0x%llx -> PCIe_%pad), size=0x%x",
rstart, &lstart, size);
 
if (!is_power_of_2(size) || size < 0x1000 ||
@@ -1215,7 +1215,7 @@ static int tsi721_rio_map_inb_mem(struct rio_mport 
*mport, dma_addr_t lstart,
priv->ibwin_cnt--;
 
tsi_debug(IBW, &priv->pdev->dev,
-   "Configured IBWIN%d (RIO_0x%llx -> PCIe

Re: [RFC PATCH v0] Add tw5864 driver

[Joe Perches]

On Sun, 2016-01-03 at 03:41 +0200, Andrey Utkin wrote:
> (Disclaimer up to scissors mark)
> 
> Please be so kind to take a look at a new driver.

trivial comments only:
> diff --git a/drivers/staging/media/tw5864/tw5864-bs.h 
> b/drivers/staging/media/tw5864/tw5864-bs.h
[]
> +static inline int bs_pos(struct bs *s)
> +{
> + return (8 * (s->p - s->p_start) + 8 - s->i_left);
> +}

several of these have unnecessary parentheses

> +static inline int bs_eof(struct bs *s)
> +{
> + return (s->p >= s->p_end ? 1 : 0);
> +}

Maybe use bool a bit more
> +/* golomb functions */
> +static inline void bs_write_ue(struct bs *s, u32 val)
> +{
> + int i_size = 0;
> + static const int i_size0_255[256] = {

Maybe use s8 instead of int to reduce object size

> + 1, 1, 2, 2, 3, 3, 3, 3, 4, 4, 4, 4, 4, 4, 4, 4, 5, 5, 5,
> + 5, 5,
> + 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5,
> + 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6,

Perhaps it'd be clearer to use gcc's ranged initializer syntax

[  0 ...   1] = 1,
[  2 ...   3] = 2,
[  4 ...   7] = 3,
[  8 ...  15] = 4,
[ 16 ...  31] = 5,
[ 32 ...  63] = 6,
etc...

or maybe just use fls

> +static inline int bs_size_ue(unsigned int val)
> +{
> + int i_size = 0;
> + static const int i_size0_254[255] = {

Same sort of thing

> diff --git a/drivers/staging/media/tw5864/tw5864-config.c 
> b/drivers/staging/media/tw5864/tw5864-config.c
[]
> +u8 tw_indir_readb(struct tw5864_dev *dev, u16 addr)
> +{
> + int timeout = 3;

misleading name, retries would be more proper,
or maybe use real timed loops.

> + u32 data = 0;
> +
> + addr <<= 2;
> +
> + while ((tw_readl(TW5864_IND_CTL) >> 31) && (timeout--))
> + ;
> + if (!timeout)
> + dev_err(&dev->pci->dev,
> + "tw_indir_writel() timeout before reading\n");
> +
> + tw_writel(TW5864_IND_CTL, addr | TW5864_ENABLE);
> +
> + timeout = 3;
> + while ((tw_readl(TW5864_IND_CTL) >> 31) && (timeout--))
> + ;
> + if (!timeout)
> + dev_err(&dev->pci->dev,
> + "tw_indir_writel() timeout at reading\n");
> +
> + data = tw_readl(TW5864_IND_DATA);
> + return data & 0xff;
> +}
[]
> +static size_t regs_dump(struct tw5864_dev *dev, char *buf, size_t size)
> +{
> + size_t count = 0;
> +
> + u32 reg_addr;
> + u32 value;
> +
> + for (reg_addr = 0x; (count < size) && (reg_addr <= 0x2FFC);
> +  reg_addr += 4) {
> + value = tw_readl(reg_addr);
> + count += scnprintf(buf + count, size - count,
> +    "[0x%05x] = 0x%08x\n", reg_addr, value);
> + }
> +
> + for (reg_addr = 0x4000; (count < size) && (reg_addr <= 0x4FFC);
> +  reg_addr += 4) {
> + value = tw_readl(reg_addr);
> + count += scnprintf(buf + count, size - count,
> +    "[0x%05x] = 0x%08x\n", reg_addr, value);
> + }
> +
> + for (reg_addr = 0x8000; (count < size) && (reg_addr <= 0x180DC);
> +  reg_addr += 4) {
> + value = tw_readl(reg_addr);
> + count += scnprintf(buf + count, size - count,
> +    "[0x%05x] = 0x%08x\n", reg_addr, value);
> + }
> +
> + for (reg_addr = 0x18100; (count < size) && (reg_addr <= 0x1817C);
> +  reg_addr += 4) {
> + value = tw_readl(reg_addr);
> + count += scnprintf(buf + count, size - count,
> +    "[0x%05x] = 0x%08x\n", reg_addr, value);
> + }
> +
> + for (reg_addr = 0x8; (count < size) && (reg_addr <= 0x87FFF);
> +  reg_addr += 4) {
> + value = tw_readl(reg_addr);
> + count += scnprintf(buf + count, size - count,
> +    "[0x%05x] = 0x%08x\n", reg_addr, value);
> + }

This seems a little repetitive.

> diff --git a/drivers/staging/media/tw5864/tw5864-tables.h 
> b/drivers/staging/media/tw5864/tw5864-tables.h
[]
> +static const u32 forward_quantization_table[QUANTIZATION_TABLE_LEN] = {

u16?


> + static const struct v4l2_ctrl_config tw5864_md_thresholds = {
> + .ops = &tw5864_ctrl_ops,
> + .id = V4L2_CID_DETECT_MD_THRESHOLD_GRID,
> + .dims = {MD_CELLS_HOR, MD_CELLS_VERT},
> + .def = 14,
> + /* See tw5864_md_metric_from_mvd() */
> + .max = 2 * 0x0f,
> + .step = 1,
> + };

odd indentation

> +#ifdef DEBUG
> + dev_dbg(&input->root->pci->dev,
> + "input %d, frame md stats: min %u, max %u, avg %u, cells above 
> threshold: %u\n",
> + input->input_number, min, max, sum / md_cells,
> + cnt_above_thresh);
> +#endif

unnecessary #ifdef

--
To unsubscribe from this list: send the line "unsubscri

Re: [PATCH] media/pci/cobalt: Use %*ph to print small buffers

[Joe Perches]

On Thu, 2015-08-27 at 00:51 +0600, Alexander Kuleshov wrote:
> printk() supports %*ph format specifier for printing a small buffers,
> let's use it intead of %02x %02x...

Having just suffered this myself...

> diff --git a/drivers/media/pci/cobalt/cobalt-cpld.c 
> b/drivers/media/pci/cobalt/cobalt-cpld.c
[]
> @@ -330,9 +330,7 @@ bool cobalt_cpld_set_freq(struct cobalt *cobalt, unsigned 
> f_out)
>  
>   if (!memcmp(read_regs, regs, sizeof(read_regs)))
>   break;
> - cobalt_dbg(1, "retry: %02x %02x %02x %02x %02x %02x\n",
> - read_regs[0], read_regs[1], read_regs[2],
> - read_regs[3], read_regs[4], read_regs[5]);
> + cobalt_dbg(1, "retry: %6ph\n");

Aren't you missing something like compile testing?


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] Staging: media: davinci_vpfe: fix over 80 characters coding style issue.

[Joe Perches]

On Sat, 2015-08-08 at 08:42 -0700, Greg KH wrote:
> Greg does not accept drivers/staging/media/* patches,

You could change
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 111a6b2..089c458 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9742,6 +9742,7 @@ T:git 
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
 L: de...@driverdev.osuosl.org
 S: Supported
 F: drivers/staging/
+X: drivers/staging/media/
 
 STAGING - COMEDI
 M: Ian Abbott 


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [lm-sensors] MAINTAINERS/s5p: Kamil Debski no longer with Samsung?

[Joe Perches]

On Mon, 2015-08-03 at 19:06 +0200, Jean Delvare wrote:
> Hi Bartlomiej,
> 
> Le Monday 03 August 2015 à 17:33 +0200, Bartlomiej Zolnierkiewicz a
> écrit :
> > Hi,
> > 
> > On Sunday, August 02, 2015 01:40:40 PM Joe Perches wrote:
> > > On Sun, 2015-08-02 at 20:31 +, Mail Delivery System wrote:
> > > > : host mailin.samsung.com[203.254.224.12] 
> > > > said: 550 5.1.1
> > > > Recipient address rejected: User unknown (in reply to RCPT TO 
> > > > command)
> > > 
> > > His email address bounces.
> > > 
> > > Should MAINTAINERS be updated?
> > 
> > Please wait with these changes, the situation should be clarified soon
> > (I've added Kamil to Cc).
> 
> Already clarified behind the scenes ;-) The patch should be 
> discarded.

It wasn't a patch, it was a question with a list of
sections that might be modified, and it wasn't signed.

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

MAINTAINERS/s5p: Kamil Debski no longer with Samsung?

[Joe Perches]

On Sun, 2015-08-02 at 20:31 +, Mail Delivery System wrote:
> : host mailin.samsung.com[203.254.224.12] 
> said: 550 5.1.1
> Recipient address rejected: User unknown (in reply to RCPT TO 
> command)

His email address bounces.

Should MAINTAINERS be updated?

---
 MAINTAINERS | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 826affa..b5197c7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1442,7 +1442,6 @@ F:arch/arm/mach-s5pv210/
 
 ARM/SAMSUNG S5P SERIES 2D GRAPHICS ACCELERATION (G2D) SUPPORT
 M: Kyungmin Park 
-M: Kamil Debski 
 L: linux-arm-ker...@lists.infradead.org
 L: linux-media@vger.kernel.org
 S: Maintained
@@ -1450,7 +1449,6 @@ F:drivers/media/platform/s5p-g2d/
 
 ARM/SAMSUNG S5P SERIES Multi Format Codec (MFC) SUPPORT
 M: Kyungmin Park 
-M: Kamil Debski 
 M: Jeongtae Park 
 L: linux-arm-ker...@lists.infradead.org
 L: linux-media@vger.kernel.org
@@ -8248,9 +8246,8 @@ S:Maintained
 F: drivers/media/usb/pwc/*
 
 PWM FAN DRIVER
-M: Kamil Debski 
 L: lm-sens...@lm-sensors.org
-S: Supported
+S: Orphan
 F: Documentation/devicetree/bindings/hwmon/pwm-fan.txt
 F: Documentation/hwmon/pwm-fan
 F: drivers/hwmon/pwm-fan.c
@@ -8906,9 +8903,8 @@ T:
https://github.com/lmajewski/linux-samsung-thermal.git
 F: drivers/thermal/samsung/
 
 SAMSUNG USB2 PHY DRIVER
-M: Kamil Debski 
 L: linux-ker...@vger.kernel.org
-S: Supported
+S: Orphan
 F: Documentation/devicetree/bindings/phy/samsung-phy.txt
 F: Documentation/phy/samsung-usb2.txt
 F: drivers/phy/phy-exynos4210-usb2.c
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[TRIVIAL PATCH] [media] s5p-mfc: Correct misuse of %0x

[Joe Perches]

Correct misuse of 0x%d in logging message.

Signed-off-by: Joe Perches 
---
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c 
b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
index 12497f5..c10ad57 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
@@ -520,7 +520,7 @@ static int s5p_mfc_set_enc_stream_buffer_v6(struct 
s5p_mfc_ctx *ctx,
writel(addr, mfc_regs->e_stream_buffer_addr); /* 16B align */
writel(size, mfc_regs->e_stream_buffer_size);
 
-   mfc_debug(2, "stream buf addr: 0x%08lx, size: 0x%d\n",
+   mfc_debug(2, "stream buf addr: 0x%08lx, size: 0x%x\n",
  addr, size);
 
return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/12] [media] dvb-pll: Add support for THOMSON DTT7546X tuner.

[Joe Perches]

On Thu, 2015-07-30 at 10:47 +0100, Peter Griffin wrote:
> Hi Mauro / Joe,
> 
> On Wed, 22 Jul 2015, Mauro Carvalho Chehab wrote:
> 
> > Em Wed, 24 Jun 2015 18:17:37 -0700
> > Joe Perches  escreveu:
> > 
> > > On Wed, 2015-06-24 at 16:11 +0100, Peter Griffin wrote:
> > > > This is used in conjunction with the STV0367 demodulator on
> > > > the STV0367-NIM-V1.0 NIM card which can be used with the STi
> > > > STB SoC's.
> > > 
> > > Barely associated to this specific patch, but for
> > > dvb-pll.c, another thing that seems possible is to
> > > convert the struct dvb_pll_desc uses to const and
> > > change the "entries" fixed array size from 12 to []
> > > 
> > > It'd save a couple KB overall and remove ~5KB of data.
> > > 
> > > $ size drivers/media/dvb-frontends/dvb-pll.o*
> > >text  data bss dec hex filename
> > >8520  15522120   121922fa0 
> > > drivers/media/dvb-frontends/dvb-pll.o.new
> > >5624  63632120   14107371b 
> > > drivers/media/dvb-frontends/dvb-pll.o.old
> > 
> > Peter,
> > 
> > Please add this patch on the next patch series you submit.
> 
> Ok will do, I've added this patch with a slightly updated commit message
> to my series.
> 
> Joe - Can I add your signed-off-by?

Signed-off-by: Joe Perches 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/12] [media] dvb-pll: Add support for THOMSON DTT7546X tuner.

[Joe Perches]

On Wed, 2015-06-24 at 16:11 +0100, Peter Griffin wrote:
> This is used in conjunction with the STV0367 demodulator on
> the STV0367-NIM-V1.0 NIM card which can be used with the STi
> STB SoC's.

Barely associated to this specific patch, but for
dvb-pll.c, another thing that seems possible is to
convert the struct dvb_pll_desc uses to const and
change the "entries" fixed array size from 12 to []

It'd save a couple KB overall and remove ~5KB of data.

$ size drivers/media/dvb-frontends/dvb-pll.o*
   textdata bss dec hex filename
   852015522120   121922fa0 
drivers/media/dvb-frontends/dvb-pll.o.new
   562463632120   14107371b 
drivers/media/dvb-frontends/dvb-pll.o.old
---
 drivers/media/dvb-frontends/dvb-pll.c | 50 +--
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/media/dvb-frontends/dvb-pll.c 
b/drivers/media/dvb-frontends/dvb-pll.c
index 6d8fe88..53089e1 100644
--- a/drivers/media/dvb-frontends/dvb-pll.c
+++ b/drivers/media/dvb-frontends/dvb-pll.c
@@ -34,7 +34,7 @@ struct dvb_pll_priv {
struct i2c_adapter *i2c;
 
/* the PLL descriptor */
-   struct dvb_pll_desc *pll_desc;
+   const struct dvb_pll_desc *pll_desc;
 
/* cached frequency/bandwidth */
u32 frequency;
@@ -57,7 +57,7 @@ MODULE_PARM_DESC(id, "force pll id to use (DEBUG ONLY)");
 /* --- */
 
 struct dvb_pll_desc {
-   char *name;
+   const char *name;
u32  min;
u32  max;
u32  iffreq;
@@ -71,13 +71,13 @@ struct dvb_pll_desc {
u32 stepsize;
u8  config;
u8  cb;
-   } entries[12];
+   } entries[];
 };
 
 /* --- */
 /* descriptions*/
 
-static struct dvb_pll_desc dvb_pll_thomson_dtt7579 = {
+static const struct dvb_pll_desc dvb_pll_thomson_dtt7579 = {
.name  = "Thomson dtt7579",
.min   = 17700,
.max   = 85800,
@@ -99,7 +99,7 @@ static void thomson_dtt759x_bw(struct dvb_frontend *fe, u8 
*buf)
buf[3] |= 0x10;
 }
 
-static struct dvb_pll_desc dvb_pll_thomson_dtt759x = {
+static const struct dvb_pll_desc dvb_pll_thomson_dtt759x = {
.name  = "Thomson dtt759x",
.min   = 17700,
.max   = 89600,
@@ -123,7 +123,7 @@ static void thomson_dtt7520x_bw(struct dvb_frontend *fe, u8 
*buf)
buf[3] ^= 0x10;
 }
 
-static struct dvb_pll_desc dvb_pll_thomson_dtt7520x = {
+static const struct dvb_pll_desc dvb_pll_thomson_dtt7520x = {
.name  = "Thomson dtt7520x",
.min   = 18500,
.max   = 9,
@@ -141,7 +141,7 @@ static struct dvb_pll_desc dvb_pll_thomson_dtt7520x = {
},
 };
 
-static struct dvb_pll_desc dvb_pll_lg_z201 = {
+static const struct dvb_pll_desc dvb_pll_lg_z201 = {
.name  = "LG z201",
.min   = 17400,
.max   = 86200,
@@ -157,7 +157,7 @@ static struct dvb_pll_desc dvb_pll_lg_z201 = {
},
 };
 
-static struct dvb_pll_desc dvb_pll_unknown_1 = {
+static const struct dvb_pll_desc dvb_pll_unknown_1 = {
.name  = "unknown 1", /* used by dntv live dvb-t */
.min   = 17400,
.max   = 86200,
@@ -179,7 +179,7 @@ static struct dvb_pll_desc dvb_pll_unknown_1 = {
 /* Infineon TUA6010XS
  * used in Thomson Cable Tuner
  */
-static struct dvb_pll_desc dvb_pll_tua6010xs = {
+static const struct dvb_pll_desc dvb_pll_tua6010xs = {
.name  = "Infineon TUA6010XS",
.min   =  4425,
.max   = 85800,
@@ -193,7 +193,7 @@ static struct dvb_pll_desc dvb_pll_tua6010xs = {
 };
 
 /* Panasonic env57h1xd5 (some Philips PLL ?) */
-static struct dvb_pll_desc dvb_pll_env57h1xd5 = {
+static const struct dvb_pll_desc dvb_pll_env57h1xd5 = {
.name  = "Panasonic ENV57H1XD5",
.min   =  4425,
.max   = 85800,
@@ -217,7 +217,7 @@ static void tda665x_bw(struct dvb_frontend *fe, u8 *buf)
buf[3] |= 0x08;
 }
 
-static struct dvb_pll_desc dvb_pll_tda665x = {
+static const struct dvb_pll_desc dvb_pll_tda665x = {
.name  = "Philips TDA6650/TDA6651",
.min   =  4425,
.max   = 85800,
@@ -251,7 +251,7 @@ static void tua6034_bw(struct dvb_frontend *fe, u8 *buf)
buf[3] |= 0x08;
 }
 
-static struct dvb_pll_desc dvb_pll_tua6034 = {
+static const struct dvb_pll_desc dvb_pll_tua6034 = {
.name  = "Infineon TUA6034",
.min   =  4425,
.max   = 85800,
@@ -275,7 +275,7 @@ static void tded4_bw(struct dvb_frontend *fe, u8 *buf)
buf[3] |= 0x04;
 }
 
-static struct dvb_pll_desc dvb_pll_tded4 = {
+static const struct dvb_pll_desc dvb_pll_tded4 = {
.name = "ALPS TDED4",
.min = 4700,
.max = 86300,
@@ -293,7 +293,7 @@ static struct dvb_pll_desc dvb_pll_tded4

[PATCH] media: ttpci: Use vsprintf %pM extension

[Joe Perches]

Format mac addresses with the normal kernel extension.

Signed-off-by: Joe Perches 
---
 drivers/media/pci/ttpci/ttpci-eeprom.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/pci/ttpci/ttpci-eeprom.c 
b/drivers/media/pci/ttpci/ttpci-eeprom.c
index 32d4315..c6f31f2 100644
--- a/drivers/media/pci/ttpci/ttpci-eeprom.c
+++ b/drivers/media/pci/ttpci/ttpci-eeprom.c
@@ -162,9 +162,7 @@ int ttpci_eeprom_parse_mac(struct i2c_adapter *adapter, u8 
*proposed_mac)
}
 
memcpy(proposed_mac, decodedMAC, 6);
-   dprintk("adapter has MAC addr = %.2x:%.2x:%.2x:%.2x:%.2x:%.2x\n",
-   decodedMAC[0], decodedMAC[1], decodedMAC[2],
-   decodedMAC[3], decodedMAC[4], decodedMAC[5]);
+   dprintk("adapter has MAC addr = %pM\n", decodedMAC);
return 0;
 }
 


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [media] lmedm04: Neaten logging

[Joe Perches]

On Tue, 2015-06-09 at 19:07 -0300, Mauro Carvalho Chehab wrote:
> Hi Joe,

Marhaba Mauro.

> Em Mon, 25 May 2015 01:29:07 -0700
> Joe Perches  escreveu:
> 
> > Use a more current logging style.
> > 
> > o Use pr_fmt
> > o Add missing newlines to formats
> > o Remove used-once lme_debug macro incorporating it into dbg_info
> > o Remove unnecessary allocation error messages
> > o Remove unnecessary semicolons from #defines
> > o Remove info macro and convert uses to pr_info
> > o Fix spelling of snippet
> > o Use %phN extension
> 
> There are a few checkpatch warnings:

I hardly use checkpatch tbh

The long lines I don't care about, I presume all the others are
existing, but I'll run --fix on it and resubmit if you want.

> > diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c 
> > b/drivers/media/usb/dvb-usb-v2/lmedm04.c
[]
> >  /* debug */
> >  static int dvb_usb_lme2510_debug;
> > -#define lme_debug(var, level, args...) do { \
> > -   if ((var >= level)) \
> > -   pr_debug(DVB_USB_LOG_PREFIX": " args); \
> > +#define deb_info(level, fmt, ...)  \
> > +do {   
> > \
> > +   if (dvb_usb_lme2510_debug >= level) \
> > +   pr_debug(fmt, ##__VA_ARGS__);   \
> >  } while (0)
> 
> 
> The usage of both a debug level and pr_debug() is not nice, as,
> if CONFIG_DYNAMIC_DEBUG is enabled (with is the case on most distros),
> one needing to debug would need to both pass a debug level AND to enable
> the debug line via sysfs, with is not nice.

> We might of course remove debug levels as a hole and just use 
> pr_debug(), but the end result is generally worse (didn't chec
> the specifics on this file).
> 
> So, the better here would be to use printk like:
> 
> #define deb_info(level, fmt, ...)\
>   do { if (dvb_usb_lme2510_debug >= level)\
>   printk(KERN_DEBUG pr_fmt(fmt), ## arg);\
>   } while (0)
> 
> Ok, this issue were already present on the old code, but IMHO the
> best is to either use the above definition of deb_info() or to just
> call pr_debug() and get rid of dvb_usb_lme2510_debug.

Alternately, you could #define DEBUG and these would be enabled
by default for CONFIG_DYNAMIC_DEBUG and act the same otherwise.


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 18/26] [media] dvb: Get rid of typedev usage for enums

[Joe Perches]

On Mon, 2015-06-08 at 16:54 -0300, Mauro Carvalho Chehab wrote:
> The DVB API was originally defined using typedefs. This is against
> Kernel CodingStyle, and there's no good usage here. While we can't
> remove its usage on userspace, we can avoid its usage in Kernelspace.
> 
> So, let's do it.
> 
> This patch was generated by this shell script:
> 
>   for j in $(grep typedef include/uapi/linux/dvb/frontend.h |cut -d' ' -f 
> 3); do for i in $(find drivers/media -name '*.[ch]' -type f) $(find 
> drivers/staging/media -name '*.[ch]' -type f); do sed "s,${j}_t,enum $j," <$i 
> >a && mv a $i; done; done

Seems sensible, thanks.

> While here, make CodingStyle fixes on the affected lines.

Trivial note examples:

> diff --git a/drivers/media/common/b2c2/flexcop-fe-tuner.c 
> b/drivers/media/common/b2c2/flexcop-fe-tuner.c
[]
> @@ -39,7 +39,8 @@ static int flexcop_fe_request_firmware(struct dvb_frontend 
> *fe,
>  
>  /* lnb control */
>  #if FE_SUPPORTED(MT312) || FE_SUPPORTED(STV0299)
> -static int flexcop_set_voltage(struct dvb_frontend *fe, fe_sec_voltage_t 
> voltage)
> +static int flexcop_set_voltage(struct dvb_frontend *fe,
> +enum fe_sec_voltage voltage)

Aligned to open paren.

> @@ -157,7 +158,7 @@ static int flexcop_diseqc_send_master_cmd(struct 
> dvb_frontend *fe,
>  }
>  
>  static int flexcop_diseqc_send_burst(struct dvb_frontend *fe,
> - fe_sec_mini_cmd_t minicmd)
> + enum fe_sec_mini_cmd minicmd)

Not aligned to open paren.

Why some and not all?

> diff --git a/drivers/media/common/siano/smsdvb.h 
> b/drivers/media/common/siano/smsdvb.h
[]
> @@ -40,7 +40,7 @@ struct smsdvb_client_t {
>   struct dmxdev   dmxdev;
>   struct dvb_frontend frontend;
>  
> - fe_status_t fe_status;
> + enum fe_status fe_status;

Maybe realign these too.


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/9] drivers/media/usb/airspy/airspy.c: drop unneeded goto

[Joe Perches]

On Thu, 2015-05-28 at 23:02 +0200, Julia Lawall wrote:
> From: Julia Lawall 
> 
> Delete jump to a label on the next line, when that label is not
> used elsewhere.

Seems sensible but:

> diff --git a/drivers/media/usb/airspy/airspy.c 
> b/drivers/media/usb/airspy/airspy.c
[]
> @@ -937,9 +937,6 @@ static int airspy_set_if_gain(struct airspy *s)
>   ret = airspy_ctrl_msg(s, CMD_SET_VGA_GAIN, 0, s->if_gain->val,
>   &u8tmp, 1);
>   if (ret)
> - goto err;
> -err:
> - if (ret)
>   dev_dbg(s->dev, "failed=%d\n", ret);
>  
>   return ret;

Ideally the function above this should also be modified
do drop the unnecessary double test of ret

static int airspy_set_mixer_gain(struct airspy *s)
{
int ret;
u8 u8tmp;

dev_dbg(s->dev, "mixer auto=%d->%d val=%d->%d\n",
s->mixer_gain_auto->cur.val, s->mixer_gain_auto->val,
s->mixer_gain->cur.val, s->mixer_gain->val);

ret = airspy_ctrl_msg(s, CMD_SET_MIXER_AGC, 0, s->mixer_gain_auto->val,
&u8tmp, 1);
if (ret)
goto err;

if (s->mixer_gain_auto->val == false) {
ret = airspy_ctrl_msg(s, CMD_SET_MIXER_GAIN, 0,
s->mixer_gain->val, &u8tmp, 1);
if (ret)
goto err;
}
err:
if (ret)
dev_dbg(s->dev, "failed=%d\n", ret);

return ret;
}

These could become something like:

static int airspy_set_mixer_gain(struct airspy *s)
{
int ret;
u8 u8tmp;

dev_dbg(s->dev, "mixer auto=%d->%d val=%d->%d\n",
s->mixer_gain_auto->cur.val, s->mixer_gain_auto->val,
s->mixer_gain->cur.val, s->mixer_gain->val);

ret = airspy_ctrl_msg(s, CMD_SET_MIXER_AGC, 0, s->mixer_gain_auto->val,
&u8tmp, 1);
if (ret)
goto err;

if (s->mixer_gain_auto->val == false) {
ret = airspy_ctrl_msg(s, CMD_SET_MIXER_GAIN, 0,
s->mixer_gain->val, &u8tmp, 1);
if (ret)
goto err;
}

return 0;

err:
dev_dbg(s->dev, "failed=%d\n", ret);
return ret;
}


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] [media] lmedm04: Neaten logging

[Joe Perches]

Use a more current logging style.

o Use pr_fmt
o Add missing newlines to formats
o Remove used-once lme_debug macro incorporating it into dbg_info
o Remove unnecessary allocation error messages
o Remove unnecessary semicolons from #defines
o Remove info macro and convert uses to pr_info
o Fix spelling of snippet
o Use %phN extension

Signed-off-by: Joe Perches 
---
 drivers/media/usb/dvb-usb-v2/lmedm04.c | 105 +++--
 1 file changed, 49 insertions(+), 56 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c 
b/drivers/media/usb/dvb-usb-v2/lmedm04.c
index 5de6f7c..7e8e58b 100644
--- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
+++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
@@ -67,6 +67,8 @@
  * M88RS2000 suffers from loss of lock.
  */
 #define DVB_USB_LOG_PREFIX "LME2510(C)"
+#define pr_fmt(fmt) DVB_USB_LOG_PREFIX ": " fmt
+
 #include 
 #include 
 #include 
@@ -84,25 +86,22 @@
 #include "ts2020.h"
 
 
-#define LME2510_C_S7395"dvb-usb-lme2510c-s7395.fw";
-#define LME2510_C_LG   "dvb-usb-lme2510c-lg.fw";
-#define LME2510_C_S0194"dvb-usb-lme2510c-s0194.fw";
-#define LME2510_C_RS2000 "dvb-usb-lme2510c-rs2000.fw";
-#define LME2510_LG "dvb-usb-lme2510-lg.fw";
-#define LME2510_S0194  "dvb-usb-lme2510-s0194.fw";
+#define LME2510_C_S7395"dvb-usb-lme2510c-s7395.fw"
+#define LME2510_C_LG   "dvb-usb-lme2510c-lg.fw"
+#define LME2510_C_S0194"dvb-usb-lme2510c-s0194.fw"
+#define LME2510_C_RS2000 "dvb-usb-lme2510c-rs2000.fw"
+#define LME2510_LG "dvb-usb-lme2510-lg.fw"
+#define LME2510_S0194  "dvb-usb-lme2510-s0194.fw"
 
 /* debug */
 static int dvb_usb_lme2510_debug;
-#define lme_debug(var, level, args...) do { \
-   if ((var >= level)) \
-   pr_debug(DVB_USB_LOG_PREFIX": " args); \
+#define deb_info(level, fmt, ...)  \
+do {   \
+   if (dvb_usb_lme2510_debug >= level) \
+   pr_debug(fmt, ##__VA_ARGS__);   \
 } while (0)
-#define deb_info(level, args...) lme_debug(dvb_usb_lme2510_debug, level, args)
-#define debug_data_snipet(level, name, p) \
-deb_info(level, name" (%02x%02x%02x%02x%02x%02x%02x%02x)", \
-   *p, *(p+1), *(p+2), *(p+3), *(p+4), \
-   *(p+5), *(p+6), *(p+7));
-#define info(args...) pr_info(DVB_USB_LOG_PREFIX": "args)
+#define debug_data_snippet(level, name, p) \
+   deb_info(level, name " (%*phN)\n", 8, p)
 
 module_param_named(debug, dvb_usb_lme2510_debug, int, 0644);
 MODULE_PARM_DESC(debug, "set debugging level (1=info (or-able)).");
@@ -182,10 +181,8 @@ static int lme2510_usb_talk(struct dvb_usb_device *d,
 
if (st->usb_buffer == NULL) {
st->usb_buffer = kmalloc(64, GFP_KERNEL);
-   if (st->usb_buffer == NULL) {
-   info("MEM Error no memory");
+   if (st->usb_buffer == NULL)
return -ENOMEM;
-   }
}
buff = st->usb_buffer;
 
@@ -234,7 +231,7 @@ static int lme2510_enable_pid(struct dvb_usb_device *d, u8 
index, u16 pid_out)
u8 pid_no = index * 2;
u8 pid_len = pid_no + 2;
int ret = 0;
-   deb_info(1, "PID Setting Pid %04x", pid_out);
+   deb_info(1, "PID Setting Pid %04x\n", pid_out);
 
if (st->pid_size == 0)
ret |= lme2510_stream_restart(d);
@@ -275,7 +272,7 @@ static void lme2510_int_response(struct urb *lme_urb)
case -ESHUTDOWN:
return;
default:
-   info("Error %x", lme_urb->status);
+   pr_info("Error %x\n", lme_urb->status);
break;
}
 
@@ -286,17 +283,17 @@ static void lme2510_int_response(struct urb *lme_urb)
 
for (i = 0; i < offset; ++i) {
ibuf = (u8 *)&rbuf[i*8];
-   deb_info(5, "INT O/S C =%02x C/O=%02x Type =%02x%02x",
-   offset, i, ibuf[0], ibuf[1]);
+   deb_info(5, "INT O/S C =%02x C/O=%02x Type =%02x%02x\n",
+offset, i, ibuf[0], ibuf[1]);
 
switch (ibuf[0]) {
case 0xaa:
-   debug_data_snipet(1, "INT Remote data snipet", ibuf);
+   debug_data_snippet(1, "INT Remote data snippet", ibuf);
if ((ibuf[4] + ibuf[5]) == 0xff) {
key = RC_SCANCODE_NECX((ibuf[2] ^ 0xff) << 8 |
   (ibuf[3] > 0) ? (ibuf[3] 
^ 0xff) : 0,

Re: [PATCH] Clarify expression which uses both multiplication and pointer dereference

[Joe Perches]

On Sun, 2015-05-17 at 19:18 +0200, Alex Dowad wrote:
> This fixes a checkpatch style error in vpfe_buffer_queue_setup.

There is no checkpatch message for this style.

Nor should there be.

> diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c 
> b/drivers/staging/media/davinci_vpfe/vpfe_video.c
[]
> @@ -1095,7 +1095,7 @@ vpfe_buffer_queue_setup(struct vb2_queue *vq, const 
> struct v4l2_format *fmt,
> - while (size * *nbuffers > vpfe_dev->video_limit)
> + while (size * (*nbuffers) > vpfe_dev->video_limit)


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: em28xx replace printk in dprintk macros

[Joe Perches]

On Tue, 2015-02-24 at 16:41 -0700, Shuah Khan wrote:
> On 02/24/2015 03:03 PM, Mauro Carvalho Chehab wrote:
> > Em Tue, 24 Feb 2015 11:53:47 -0700 Shuah Khan  
> > escreveu:
> >> Replace printk macro in dprintk macros in em28xx audio, dvb,
> >> and input files with pr_* equivalent routines.
[]
> >> diff --git a/drivers/media/usb/em28xx/em28xx-input.c 
> >> b/drivers/media/usb/em28xx/em28xx-input.c
[]
> >>  #define dprintk(fmt, arg...) \
> >>if (ir_debug) { \
> >> -  printk(KERN_DEBUG "%s/ir: " fmt, ir->name , ## arg); \
> >> +  pr_debug("%s/ir: " fmt, ir->name, ## arg); \
> > 
> > NACK.
> > 
> > This is the worse of two words, as it would require both to enable
> > each debug line via dynamic printk setting and to enable ir_debug.
> Ah. I missed that. Sorry for the noise.

It's
At some point, I'm going to propose a standard mechanism
similar to netif_ that does bitmap matching for
dynamic_debug and generic debugging.



--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers: media: i2c : s5c73m3: Replace dev_err with pr_err

[Joe Perches]

On Tue, 2015-02-24 at 10:17 +0530, Tapasweni Pathak wrote:
> Replace dev_err statement with pr_err to fix null dereference.
> 
> Found by Coccinelle.
> 
> Signed-off-by: Tapasweni Pathak 
> ---
>  drivers/media/i2c/s5c73m3/s5c73m3-spi.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-spi.c 
> b/drivers/media/i2c/s5c73m3/s5c73m3-spi.c
> index f60b265..63eb190 100644
> --- a/drivers/media/i2c/s5c73m3/s5c73m3-spi.c
> +++ b/drivers/media/i2c/s5c73m3/s5c73m3-spi.c
> @@ -52,7 +52,7 @@ static int spi_xmit(struct spi_device *spi_dev, void *addr, 
> const int len,
>   xfer.rx_buf = addr;
> 
>   if (spi_dev == NULL) {
> - dev_err(&spi_dev->dev, "SPI device is uninitialized\n");
> + pr_err("SPI device is uninitialized\n");
>   return -ENODEV;
>   }

It'd be better to move this above the if (dir...) block
and use ratelimit/once it too

static int spi_xmit(struct spi_device *spi_dev, void *addr, const int len,
enum spi_direction dir)
{
struct spi_message msg;
int r;
struct spi_transfer xfer = {
.len= len,
};

if (!spi_dev) {
pr_err_once("SPI device is uninitialized\n");
return -ENODEV;
}

if (dir == SPI_DIR_TX)
xfer.tx_buf = addr;
else
xfer.rx_buf = addr;

...

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] dvb_net: Use standard debugging facilities

[Joe Perches]

Convert dprintk to netdev_dbg where appropriate.
Remove dvb_net_debug module_param.
Remove __func__ from output as that can be added by dynamic_debug.

Signed-off-by: Joe Perches 
---
 drivers/media/dvb-core/dvb_net.c | 57 +---
 1 file changed, 24 insertions(+), 33 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c
index ff79b0b..0b0f97a 100644
--- a/drivers/media/dvb-core/dvb_net.c
+++ b/drivers/media/dvb-core/dvb_net.c
@@ -68,13 +68,6 @@
 #include "dvb_demux.h"
 #include "dvb_net.h"
 
-static int dvb_net_debug;
-module_param(dvb_net_debug, int, 0444);
-MODULE_PARM_DESC(dvb_net_debug, "enable debug messages");
-
-#define dprintk(x...) do { if (dvb_net_debug) printk(x); } while (0)
-
-
 static inline __u32 iov_crc32( __u32 c, struct kvec *iov, unsigned int cnt )
 {
unsigned int j;
@@ -312,9 +305,9 @@ static int handle_ule_extensions( struct dvb_net_priv *p )
return l;   /* Stop extension header processing and 
discard SNDU. */
total_ext_len += l;
 #ifdef ULE_DEBUG
-   dprintk("handle_ule_extensions: ule_next_hdr=%p, 
ule_sndu_type=%i, "
-   "l=%i, total_ext_len=%i\n", p->ule_next_hdr,
-   (int) p->ule_sndu_type, l, total_ext_len);
+   pr_debug("ule_next_hdr=%p, ule_sndu_type=%i, l=%i, 
total_ext_len=%i\n",
+p->ule_next_hdr, (int)p->ule_sndu_type,
+l, total_ext_len);
 #endif
 
} while (p->ule_sndu_type < ETH_P_802_3_MIN);
@@ -697,8 +690,8 @@ static void dvb_net_ule( struct net_device *dev, const u8 
*buf, size_t buf_len )
 
if (drop) {
 #ifdef ULE_DEBUG
-   dprintk("Dropping SNDU: MAC 
destination address does not match: dest addr: %pM, dev addr: %pM\n",
-   priv->ule_skb->data, 
dev->dev_addr);
+   netdev_dbg(dev, "Dropping SNDU: 
MAC destination address does not match: dest addr: %pM, dev addr: %pM\n",
+  priv->ule_skb->data, 
dev->dev_addr);
 #endif
dev_kfree_skb(priv->ule_skb);
goto sndu_done;
@@ -961,8 +954,7 @@ static int dvb_net_filter_sec_set(struct net_device *dev,
(*secfilter)->filter_mask[10] = mac_mask[1];
(*secfilter)->filter_mask[11]=mac_mask[0];
 
-   dprintk("%s: filter mac=%pM\n", dev->name, mac);
-   dprintk("%s: filter mask=%pM\n", dev->name, mac_mask);
+   netdev_dbg(dev, "filter mac=%pM mask=%pM\n", mac, mac_mask);
 
return 0;
 }
@@ -974,7 +966,7 @@ static int dvb_net_feed_start(struct net_device *dev)
struct dmx_demux *demux = priv->demux;
unsigned char *mac = (unsigned char *) dev->dev_addr;
 
-   dprintk("%s: rx_mode %i\n", __func__, priv->rx_mode);
+   netdev_dbg(dev, "rx_mode %i\n", priv->rx_mode);
mutex_lock(&priv->mutex);
if (priv->tsfeed || priv->secfeed || priv->secfilter || 
priv->multi_secfilter[0])
printk("%s: BUG %d\n", __func__, __LINE__);
@@ -984,7 +976,7 @@ static int dvb_net_feed_start(struct net_device *dev)
priv->tsfeed = NULL;
 
if (priv->feedtype == DVB_NET_FEEDTYPE_MPE) {
-   dprintk("%s: alloc secfeed\n", __func__);
+   netdev_dbg(dev, "alloc secfeed\n");
ret=demux->allocate_section_feed(demux, &priv->secfeed,
 dvb_net_sec_callback);
if (ret<0) {
@@ -1002,38 +994,38 @@ static int dvb_net_feed_start(struct net_device *dev)
}
 
if (priv->rx_mode != RX_MODE_PROMISC) {
-   dprintk("%s: set secfilter\n", __func__);
+   netdev_dbg(dev, "set secfilter\n");
dvb_net_filter_sec_set(dev, &priv->secfilter, mac, 
mask_normal);
}
 
switch (priv->rx_mode) {
case RX_MODE_MULTI:
for (i = 0; i < priv->multi_num; i++) {
-   dprintk("%s: set multi_secfilter[%d]\n", 
__func__, i);
+   netdev_dbg(dev, "set multi_secfilter[%d]\n", i);
dvb_net_filter_sec_set(dev, 
&priv->multi_secfilter[i],
   priv->multi_macs[i], 
mask_normal);
}

[PATCH 3/3] dvb_net: Convert local hex dump to print_hex_dump_debug

[Joe Perches]

Use the generic facility instead of a home-grown one.

Signed-off-by: Joe Perches 
---
 drivers/media/dvb-core/dvb_net.c | 28 ++--
 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c
index 0b0f97a..686d327 100644
--- a/drivers/media/dvb-core/dvb_net.c
+++ b/drivers/media/dvb-core/dvb_net.c
@@ -83,33 +83,9 @@ static inline __u32 iov_crc32( __u32 c, struct kvec *iov, 
unsigned int cnt )
 
 #ifdef ULE_DEBUG
 
-#define isprint(c) ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c 
>= '0' && c <= '9'))
-
-static void hexdump( const unsigned char *buf, unsigned short len )
+static void hexdump(const unsigned char *buf, unsigned short len)
 {
-   char str[80], octet[10];
-   int ofs, i, l;
-
-   for (ofs = 0; ofs < len; ofs += 16) {
-   sprintf( str, "%03d: ", ofs );
-
-   for (i = 0; i < 16; i++) {
-   if ((i + ofs) < len)
-   sprintf( octet, "%02x ", buf[ofs + i] );
-   else
-   strcpy( octet, "   " );
-
-   strcat( str, octet );
-   }
-   strcat( str, "  " );
-   l = strlen( str );
-
-   for (i = 0; (i < 16) && ((i + ofs) < len); i++)
-   str[l++] = isprint( buf[ofs + i] ) ? buf[ofs + i] : '.';
-
-   str[l] = '\0';
-   printk( KERN_WARNING "%s\n", str );
-   }
+   print_hex_dump_debug("", DUMP_PREFIX_OFFSET, 16, 1, buf, len, true);
 }
 
 #endif
-- 
2.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] dvb_net: Use vsprintf %pM extension to print Ethernet addresses

[Joe Perches]

No need for more macros, so remove them and use the kernel extension.

Signed-off-by: Joe Perches 
---
 drivers/media/dvb-core/dvb_net.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c
index e4041f0..ff79b0b 100644
--- a/drivers/media/dvb-core/dvb_net.c
+++ b/drivers/media/dvb-core/dvb_net.c
@@ -90,9 +90,6 @@ static inline __u32 iov_crc32( __u32 c, struct kvec *iov, 
unsigned int cnt )
 
 #ifdef ULE_DEBUG
 
-#define MAC_ADDR_PRINTFMT "%.2x:%.2x:%.2x:%.2x:%.2x:%.2x"
-#define MAX_ADDR_PRINTFMT_ARGS(macap) 
(macap)[0],(macap)[1],(macap)[2],(macap)[3],(macap)[4],(macap)[5]
-
 #define isprint(c) ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c 
>= '0' && c <= '9'))
 
 static void hexdump( const unsigned char *buf, unsigned short len )
@@ -700,8 +697,8 @@ static void dvb_net_ule( struct net_device *dev, const u8 
*buf, size_t buf_len )
 
if (drop) {
 #ifdef ULE_DEBUG
-   dprintk("Dropping SNDU: MAC 
destination address does not match: dest addr: "MAC_ADDR_PRINTFMT", dev addr: 
"MAC_ADDR_PRINTFMT"\n",
-   
MAX_ADDR_PRINTFMT_ARGS(priv->ule_skb->data), 
MAX_ADDR_PRINTFMT_ARGS(dev->dev_addr));
+   dprintk("Dropping SNDU: MAC 
destination address does not match: dest addr: %pM, dev addr: %pM\n",
+   priv->ule_skb->data, 
dev->dev_addr);
 #endif
dev_kfree_skb(priv->ule_skb);
goto sndu_done;
-- 
2.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/3] dvb_net: general cleaning

[Joe Perches]

Use more common kernel mechanisms

Joe Perches (3):
  dvb_net: Use vsprintf %pM extension to print Ethernet addresses
  dvb_net: Use standard debugging facilities
  dvb_net: Convert local hex dump to print_hex_dump_debug

 drivers/media/dvb-core/dvb_net.c | 88 
 1 file changed, 26 insertions(+), 62 deletions(-)

-- 
2.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] cx25840: fill the media controller entity

[Joe Perches]

On Sat, 2015-01-03 at 18:09 -0200, Mauro Carvalho Chehab wrote:
> Instead of keeping the media controller entity not initialized,
> fill it and create the pads for cx25840.

Won't you get an unused variable for ret
without CONFIG_MEDIA_CONTROLLER?

> diff --git a/drivers/media/i2c/cx25840/cx25840-core.c 
> b/drivers/media/i2c/cx25840/cx25840-core.c
[]
> @@ -5134,7 +5134,7 @@ static int cx25840_probe(struct i2c_client *client,
>  {
>   struct cx25840_state *state;
>   struct v4l2_subdev *sd;
> - int default_volume;
> + int default_volume, ret;
>   u32 id;
>   u16 device_id;
>  
> @@ -5178,6 +5178,18 @@ static int cx25840_probe(struct i2c_client *client,
>  
>   sd = &state->sd;
>   v4l2_i2c_subdev_init(sd, client, &cx25840_ops);
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> + /* TODO: need to represent analog inputs too */
> + state->pads[0].flags = MEDIA_PAD_FL_SINK;   /* Tuner or input */
> + state->pads[1].flags = MEDIA_PAD_FL_SOURCE; /* Video */
> + state->pads[2].flags = MEDIA_PAD_FL_SOURCE; /* VBI */
> + sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
> +
> + ret = media_entity_init(&sd->entity, ARRAY_SIZE(state->pads),
> + state->pads, 0);
> + if (ret < 0)
> + v4l_info(client, "failed to initialize media entity!\n");
> +#endif
>  
>   switch (id) {
>   case CX23885_AV:

Maybe write this without ret at all.

if (media_entry_init(...) < 0)
v4l_info(...)


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/2] staging: media: lirc: lirc_zilog.c: fix quoted strings split across lines

[Joe Perches]

On Wed, 2014-12-10 at 23:57 +, Luis de Bethencourt wrote:
> On Wed, Dec 10, 2014 at 03:39:09PM -0800, Joe Perches wrote:
> > On Wed, 2014-12-10 at 22:33 +, Luis de Bethencourt wrote:
> > > diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
> > > b/drivers/staging/media/lirc/lirc_zilog.c
[]
> > > + dev_err(tx->ir->l.dev,
> > > + "unsupported code set file version (%u, expected 1) -- 
> > > please upgrade to a newer driver",
> > > + version);
> > 
> > Unrelated but this one should have a '\n' termination
> > at the end of the format.
> I can add that change, no problem. As part of this patch or a third one?

Up to you.


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/2] staging: media: lirc: lirc_zilog.c: fix quoted strings split across lines

[Joe Perches]

On Wed, 2014-12-10 at 22:33 +, Luis de Bethencourt wrote:
> checkpatch makes an exception to the 80-colum rule for quotes strings, and
> Documentation/CodingStyle recommends not splitting quotes strings across lines
> because it breaks the ability to grep for the string. Fixing these.
[]
> diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
> b/drivers/staging/media/lirc/lirc_zilog.c
[]
> @@ -794,9 +796,9 @@ static int fw_load(struct IR_tx *tx)
>   if (!read_uint8(&data, tx_data->endp, &version))
>   goto corrupt;
>   if (version != 1) {
> - dev_err(tx->ir->l.dev, "unsupported code set file version (%u, 
> expected"
> - "1) -- please upgrade to a newer driver",
> - version);
> + dev_err(tx->ir->l.dev,
> + "unsupported code set file version (%u, expected 1) -- 
> please upgrade to a newer driver",
> + version);

Unrelated but this one should have a '\n' termination
at the end of the format.

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: media: lirc: lirc_zilog.c: fix quoted strings split across lines

[Joe Perches]

On Wed, 2014-11-26 at 15:42 +, Luis de Bethencourt wrote:
> On 26 November 2014 at 01:49, Joe Perches  wrote:
[]
> > There is a script I posted a while back that
> > groups various checkpatch "types" together and
> > makes it a bit easier to do cleanup style
> > patches.
> >
> > https://lkml.org/lkml/2014/7/11/794
> That is useful! I just run it on staging/octeon/ and it wrote two patches.
> Will submit them in a minute.

Please make sure and write better commit messages
than the script produces.

> > Using checkpatch to get familiar with kernel
> > development is fine and all, but fixing actual
> > defects and submitting new code is way more
> > useful.
[]
> I agree. I was just using checkpatch to learn about the development process.
> How to create patches, submit patches, follow review, and such. Better to
> do it
> with small changes like this first.

That's a good way to start.

> Which makes me wonder. Is my patch accepted? Will it be merged? I can do the
> proposed logging macro additions in a few days. Not sure yet how the final
> step of the process when patches get accepted and merged works.

You will generally get an email from a maintainer
when patches are accepted/rejected or you get
feedback asking for various changes.

Greg KH does that for drivers/staging but not for
drivers/staging/media.  Mauro Carvalho Chehab does.

These emails are not immediate.  It can take 2 or 3
weeks for a response.  Sometimes longer, sometimes
shorter, sometimes no response ever comes.

After a month or so, if you get no response, maybe
the maintainer never saw it.  You should maybe
expand the cc: list for the email.

When the patch is more than a trivial style cleanup,
Andrew Morton generally picks up orphan patches.

For some subsystems, there are "tracking" mechanisms
like patchwork:

For instance, netdev (net/ and drivers/net/) uses:
http://patchwork.ozlabs.org/project/netdev/list/
and David Miller, the primary networking maintainer
is very prompt about updating it.

There's this list of patchwork entries, but maintainer
activity of these lists vary:

https://patchwork.kernel.org/

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html