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

2018-09-30 Thread 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

2018-03-21 Thread 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 <j...@perches.com>
Acked-by: Andy Shevchenko <andy.shevche...@gmail.com>
Acked-by: Paul Moore <p...@paul-moore.com>
Acked-by: Alex Deucher <alexander.deuc...@amd.com>
Acked-by: Dave Chinner <dchin...@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.w...@oracle.com>
Acked-by: Alexandre Belloni <alexandre.bell...@free-electrons.com>
Acked-by: Martin K. Petersen <martin.peter...@oracle.com>
Acked-by: Takashi Iwai <ti...@suse.de>
Acked-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
---

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, "=" (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 @@ mp

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

2018-03-16 Thread 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

2018-03-01 Thread 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

2018-03-01 Thread 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 <j...@perches.com>
---
 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, , );
-   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(>dev, "Field type %d invalid.\n", mf->field);
+   dev_err(>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(>dev,
-   "Product ID error %x:%x\n",
+   dev_err(>dev, "Product ID error %x:%x\n",
id, priv->revision);
ret = -ENODEV;
goto done;
}
 
-   dev_info(>dev,
-"tw9910 Product ID %0x:%0x\n", id, priv->revision);
+   dev_info(>dev, "tw9910 Product ID %0x:%0x\n",
+id, priv->revision);
 
priv->norm = V4L2_STD_NTSC;
priv->scale = _ntsc_scales[0];
 
 done:
tw9910_s_power(>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

2018-03-01 Thread Joe Perches
Update multiline statements to open parenthesis.
Update a ?: to a single line.

Signed-off-by: Joe Perches <j...@perches.com>
---
 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;
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;
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

2018-02-12 Thread 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

2018-01-09 Thread 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,
> -   >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_

2017-12-19 Thread 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

2017-12-19 Thread 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 <j...@perches.com>
---
 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_

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

2017-12-17 Thread 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 <j...@perches.com>
---
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, "=" (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(>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 @@ static s

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

2017-12-14 Thread 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

2017-12-14 Thread 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

2017-12-14 Thread 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

2017-12-12 Thread 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
> <mche...@kernel.org> wrote:
> > Em Tue, 12 Dec 2017 03:42:32 -0800
> > Joe Perches <j...@perches.com> 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 = 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

2017-12-12 Thread 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
> <mkru...@linuxtv.org> wrote:
> > On Mon, Dec 11, 2017 at 2:34 PM, Joe Perches <j...@perches.com> 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, , 1);
> 
> return (ret == 1) ? len : ret;
> @@ -54,11 +54,11 @@ static inline int tuner_i2c_xfer_recv(struct
> tuner_i2c_props *props,
>  }
> 
>  static inline int t

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

2017-12-11 Thread 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

2017-11-16 Thread 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

2017-11-16 Thread Joe Perches
Line continuations with excess spacing causes unexpected output.

Signed-off-by: Joe Perches <j...@perches.com>
---
 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

2017-11-16 Thread 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

2017-11-04 Thread 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 <j...@perches.com>
---
 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

2017-11-01 Thread 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()

2017-09-24 Thread 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(>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()

2017-09-22 Thread 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

2017-09-22 Thread 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 <j...@perches.com>
---
 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 {
-  

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

2017-09-15 Thread 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

2017-08-29 Thread 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

2017-08-02 Thread 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

2017-07-16 Thread 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

2017-07-14 Thread 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

2017-07-14 Thread 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

2017-07-09 Thread 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()

2017-07-07 Thread 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

2017-06-29 Thread 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(>dev, "error in i2c_read_reg\n");
> + dev_err(>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(>dev, "error in i2c_read\n");
> + dev_err(>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(>dev, "%s: i2c_transfer error\n", __func__);



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

2017-06-09 Thread 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 <j...@perches.com>
---
 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

2017-06-09 Thread 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 <j...@perches.com>
---
 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, )
|| stk_sensor_inb(dev, REG_MIDL, )) {
-   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, )
|| stk_sensor_inb(dev, REG_VER, )) {
-   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:
case MODE_CI

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

2017-06-07 Thread 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 <j...@perches.com> 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

2017-06-07 Thread 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

2017-05-30 Thread 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

2017-05-30 Thread 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

2017-05-30 Thread 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

2017-04-29 Thread Joe Perches
__printf validation adds format and argument validation.

Fix the various broken format/argument mismatches.

Signed-off-by: Joe Perches <j...@perches.com>
---

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/c

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

2017-04-29 Thread Joe Perches
__printf validation adds format and argument validation.

Fix the various broken format/argument mismatches.

Signed-off-by: Joe Perches <j...@perches.com>
---

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, &q

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

2017-03-30 Thread Joe Perches
Add these misspellings to scripts/spelling.txt too

Signed-off-by: Joe Perches <j...@perches.com>
---
 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(, 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 = >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
index 1beb1ec2

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

2017-03-05 Thread 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

2017-03-02 Thread 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 = 
 
-   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, );
+  raw_smp_processor_id(), __builtin_return_address(0),
+  prefix, );
 
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

2017-03-02 Thread 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 <j...@perches.com> 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

2017-03-02 Thread 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 <j...@perches.com>
> Fixes: 7c2bd2f930ae ("ocfs2: reduce object size of mlog uses")
> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> ---
>  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

2017-02-23 Thread Joe Perches
On Thu, 2017-02-23 at 17:41 +, Emil Velikov wrote:
> On 23 February 2017 at 17:18, Joe Perches <j...@perches.com> wrote:
> > On Thu, 2017-02-23 at 09:28 -0600, Rob Herring wrote:
> > > On Fri, Feb 17, 2017 at 1:11 AM, Joe Perches <j...@perches.com> 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

2017-02-23 Thread 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 <j...@perches.com> 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

2017-02-16 Thread 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_trapped.c

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

2017-02-16 Thread 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 <j...@perches.com>
---
 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

2017-02-10 Thread 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.

2017-01-31 Thread Joe Perches
On Tue, 2017-01-31 at 10:30 -0800, Eric Anholt wrote:
> Joe Perches <j...@perches.com> writes:
> 
> > On Mon, 2017-01-30 at 12:05 -0800, Eric Anholt wrote:
> > > Joe Perches <j...@perches.com> 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.

2017-01-30 Thread Joe Perches
On Mon, 2017-01-30 at 12:05 -0800, Eric Anholt wrote:
> Joe Perches <j...@perches.com> 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.

2017-01-27 Thread 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

2016-10-13 Thread 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

2016-10-01 Thread 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

2016-09-11 Thread 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

2016-08-06 Thread 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

2016-07-12 Thread 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

2016-07-12 Thread 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

2016-07-12 Thread 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

2016-07-11 Thread 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(_dev->dev, "%s", artifacts_warning);
> + dev_warn(_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(_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(_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_*

2016-06-29 Thread 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

2016-06-12 Thread 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

2016-03-26 Thread 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__, >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

2016-03-13 Thread 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

2016-03-04 Thread 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 <j...@perches.com>
---
 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(>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",
 >saddr, >daddr,
 lli->ctrla, lli->ctrlb, >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", >res->start);
+   cal_info(dev, "CAL Registers @ %pa:\n", >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",
>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", >phys_addr);
+   rmcd_debug(MMAP, "%pad", >phys_addr);
kref_get(>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", >phys_addr);
+   rmcd_debug(MMAP, "%pad", >phys_addr);
mutex_lock(>md->buf_mutex);
kref_put(>ref, mport_release_mapping);
mutex_unlock(>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, >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, , 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, >pdev->dev,
-   "Translated (RIO_0x%llx -> PCIe_0x%pad), size=0x%x",
+   "Translated (RIO_0x%llx -> PCIe_%pad), size=0x%x",
rstart, , 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, >pdev->dev,
-   "Configured IBWIN%d (RIO_0x%llx -> PCIe_0x%pad), size=0x%llx",
+   "Configured IBWIN%d (RIO_0x%llx -> PCIe_%pad), size=0x%llx",
i, ibw_start, _start, ibw_size);
 

Re: [RFC PATCH v0] Add tw5864 driver

2016-01-02 Thread 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(>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(>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 = _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(>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 "unsubscribe linux-media" in
the 

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

2015-08-26 Thread 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.

2015-08-08 Thread 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 abbo...@mev.co.uk


--
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?

2015-08-03 Thread 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:
k.deb...@samsung.com: 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?

2015-08-02 Thread Joe Perches
On Sun, 2015-08-02 at 20:31 +, Mail Delivery System wrote:
 k.deb...@samsung.com: 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 kyungmin.p...@samsung.com
-M: Kamil Debski k.deb...@samsung.com
 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 kyungmin.p...@samsung.com
-M: Kamil Debski k.deb...@samsung.com
 M: Jeongtae Park jtp.p...@samsung.com
 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 k.deb...@samsung.com
 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 k.deb...@samsung.com
 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 %0xdecimal

2015-08-02 Thread Joe Perches
Correct misuse of 0x%d in logging message.

Signed-off-by: Joe Perches j...@perches.com
---
 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.

2015-07-30 Thread 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 j...@perches.com 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 j...@perches.com

--
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.

2015-06-24 Thread 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 = {
 /* ALPS TDHU2
  * 

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

2015-06-14 Thread Joe Perches
Format mac addresses with the normal kernel extension.

Signed-off-by: Joe Perches j...@perches.com
---
 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

2015-06-09 Thread 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 j...@perches.com 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

2015-06-08 Thread 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

2015-05-28 Thread Joe Perches
On Thu, 2015-05-28 at 23:02 +0200, Julia Lawall wrote:
 From: Julia Lawall julia.law...@lip6.fr
 
 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

2015-05-25 Thread 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 j...@perches.com
---
 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 linux/usb.h
 #include linux/usb/input.h
 #include media/rc-core.h
@@ -84,25 +86,22 @@
 #include ts2020.h
 
 
-#define LME2510_C_S7395dvb-usb-lme2510c-s7395.fw;
-#define LME2510_C_LG   dvb-usb-lme2510c-lg.fw;
-#define LME2510_C_S0194dvb-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_S7395dvb-usb-lme2510c-s7395.fw
+#define LME2510_C_LG   dvb-usb-lme2510c-lg.fw
+#define LME2510_C_S0194dvb-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,
   ibuf[5]);
-   deb_info(1, INT Key =%08x, key);
+   deb_info(1, INT Key =%08x\n, key);
if (adap_to_d(adap)-rc_dev != NULL)
rc_keydown(adap_to_d(adap)-rc_dev

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

2015-05-17 Thread 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

2015-02-24 Thread 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 shua...@osg.samsung.com 
  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_level 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

2015-02-23 Thread 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 tapaswenipat...@gmail.com
 ---
  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 0/3] dvb_net: general cleaning

2015-01-28 Thread 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


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

2015-01-28 Thread Joe Perches
No need for more macros, so remove them and use the kernel extension.

Signed-off-by: Joe Perches j...@perches.com
---
 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 3/3] dvb_net: Convert local hex dump to print_hex_dump_debug

2015-01-28 Thread Joe Perches
Use the generic facility instead of a home-grown one.

Signed-off-by: Joe Perches j...@perches.com
---
 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 2/3] dvb_net: Use standard debugging facilities

2015-01-28 Thread 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 j...@perches.com
---
 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 (ret0) {
@@ -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);
}
break;
case RX_MODE_ALL_MULTI:
priv-multi_num=1;
-   dprintk(%s: set multi_secfilter[0]\n, __func__);
+   netdev_dbg(dev, set multi_secfilter[0]\n);
dvb_net_filter_sec_set(dev, priv-multi_secfilter[0],
   mac_allmulti, mask_allmulti

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

2015-01-03 Thread 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

2014-12-10 Thread 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 v4 1/2] staging: media: lirc: lirc_zilog.c: fix quoted strings split across lines

2014-12-10 Thread 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] staging: media: lirc: lirc_zilog.c: fix quoted strings split across lines

2014-11-26 Thread Joe Perches
On Wed, 2014-11-26 at 15:42 +, Luis de Bethencourt wrote:
 On 26 November 2014 at 01:49, Joe Perches j...@perches.com 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


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

2014-11-25 Thread Joe Perches
On Tue, 2014-11-25 at 20:19 +, 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,8 +792,7 @@ 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,
 + dev_err(tx-ir-l.dev, unsupported code set file version (%u, 
 expected1) -- please upgrade to a newer driver,
   version);

Hello Luis.

Please look at the strings being coalesced before
submitting patches.

It's a fairly common defect to have either a missing
space between the coalesced fragments or too many
spaces.

It's almost certain that there should be a space
between the expected and 1 here.


--
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

2014-11-25 Thread Joe Perches
On Tue, 2014-11-25 at 20:40 +, Luis de Bethencourt wrote:
 On Tue, Nov 25, 2014 at 12:27:24PM -0800, Joe Perches wrote:
  On Tue, 2014-11-25 at 20:19 +, 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,8 +792,7 @@ 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,
   + dev_err(tx-ir-l.dev, unsupported code set file version (%u, 
   expected1) -- please upgrade to a newer driver,
 version);
  
  Hello Luis.
  
  Please look at the strings being coalesced before
  submitting patches.
  
  It's a fairly common defect to have either a missing
  space between the coalesced fragments or too mano
  spaces.
  
  It's almost certain that there should be a space
  between the expected and 1 here.
  
  
 
 Hello Joe,
 
 Thanks for taking the time to review this. I sent a new
 version fixing the missing space. 

Thanks.

In the future, you might consider being more
comprehensive with your patches.

This code could be neatened a bit by:

o using another set of logging macros
o removing the unnecessary ftrace like logging
o realigning arguments

Something like:

---
 drivers/staging/media/lirc/lirc_zilog.c | 151 +++-
 1 file changed, 73 insertions(+), 78 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index bebb9f1..523af12 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -158,6 +158,17 @@ static bool debug; /* debug output */
 static bool tx_only;   /* only handle the IR Tx function */
 static int minor = -1; /* minor number */
 
+/* logging macros */
+#define ir_err(ir, fmt, ...)   \
+   dev_err((ir)-l.dev, fmt, ##__VA_ARGS__)
+#define ir_warn(ir, fmt, ...)  \
+   dev_warn((ir)-l.dev, fmt, ##__VA_ARGS__)
+#define ir_notice(ir, fmt, ...)\
+   dev_notice((ir)-l.dev, fmt, ##__VA_ARGS__)
+#define ir_info(ir, fmt, ...)  \
+   dev_info((ir)-l.dev, fmt, ##__VA_ARGS__)
+#define ir_dbg(ir, fmt, ...)   \
+   dev_dbg((ir)-l.dev, fmt, ##__VA_ARGS__)
 
 /* struct IR reference counting */
 static struct IR *get_ir_device(struct IR *ir, bool ir_devices_lock_held)
@@ -322,7 +333,7 @@ static int add_to_buf(struct IR *ir)
struct IR_tx *tx;
 
if (lirc_buffer_full(rbuf)) {
-   dev_dbg(ir-l.dev, buffer overflow\n);
+   ir_dbg(ir, buffer overflow\n);
return -EOVERFLOW;
}
 
@@ -368,18 +379,15 @@ static int add_to_buf(struct IR *ir)
 */
ret = i2c_master_send(rx-c, sendbuf, 1);
if (ret != 1) {
-   dev_err(ir-l.dev, i2c_master_send failed with %d\n,
-  ret);
+   ir_err(ir, i2c_master_send failed with %d\n, ret);
if (failures = 3) {
mutex_unlock(ir-ir_lock);
-   dev_err(ir-l.dev, unable to read from the IR 
chip 
-   after 3 resets, giving up\n);
+   ir_err(ir, unable to read from the IR chip 
after 3 resets, giving up\n);
break;
}
 
/* Looks like the chip crashed, reset it */
-   dev_err(ir-l.dev, polling the IR receiver chip 
failed, 
-   trying reset\n);
+   ir_err(ir, polling the IR receiver chip failed, trying 
reset\n);
 
set_current_state(TASK_UNINTERRUPTIBLE);
if (kthread_should_stop()) {
@@ -405,14 +413,13 @@ static int add_to_buf(struct IR *ir)
ret = i2c_master_recv(rx-c, keybuf, sizeof(keybuf));
mutex_unlock(ir-ir_lock);
if (ret != sizeof(keybuf)) {
-   dev_err(ir-l.dev, i2c_master_recv failed with %d -- 
-   keeping last read buffer\n, ret);
+   ir_err(ir, i2c_master_recv failed with %d -- keeping 
last read buffer\n,
+  ret);
} else {
rx-b[0] = keybuf[3];
rx-b[1] = keybuf[4

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

2014-11-25 Thread Joe Perches
On Tue, 2014-11-25 at 21:14 +, Luis de Bethencourt wrote:
 On Tue, Nov 25, 2014 at 01:00:07PM -0800, Joe Perches wrote:
  In the future, you might consider being more
  comprehensive with your patches.
 
 Wasn't sure about the scope of the style fixing
 patches. I've been reading Kernel Newbies and
 this looked like a good way to start
 contributing. Good to know more exhaustive
 changes are welcome.
  
  This code could be neatened a bit by:
  
  o using another set of logging macros
  o removing the unnecessary ftrace like logging
  o realigning arguments
 
 Great ideas.
 Should this have been all included in one patch,
 or each as part of a series with the previous
 one?
 Want to take the opportunity to learn about the
 process.

Hello again Luis.

I think the suggestion I posted here is suitable
for a single change.

Ideally, you'd make individual patches each with
a single type of change.

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

But don't just use checkpatch as the sole
decider of what's appropriate to fix or neaten.

checkpatch is a stupid, brainless little script.
So is the automation script that uses checkpatch.

For instance, checkpatch would not have suggested
creating and using another logging macro.

Please use your own taste to best figure out what
to fix and how.

Using checkpatch to get familiar with kernel
development is fine and all, but fixing actual
defects and submitting new code is way more
useful.

cheers, welcome, Joe

--
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 05/11] cx25840/cx18: Use standard ordering of mask and shift

2014-10-26 Thread Joe Perches
Precedence of  and  is not the same and is not left to right.
shift has higher precedence and should be done after the mask.

This use has a mask then shift which is not the normal style.

Move the shift before the mask to match nearly all the other
uses in kernel.

Signed-off-by: Joe Perches j...@perches.com
---
 drivers/media/i2c/cx25840/cx25840-core.c | 12 ++--
 drivers/media/pci/cx18/cx18-av-core.c| 16 
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/media/i2c/cx25840/cx25840-core.c 
b/drivers/media/i2c/cx25840/cx25840-core.c
index e453a3f..0327032 100644
--- a/drivers/media/i2c/cx25840/cx25840-core.c
+++ b/drivers/media/i2c/cx25840/cx25840-core.c
@@ -879,7 +879,7 @@ void cx25840_std_setup(struct i2c_client *client)
/* Sets horizontal blanking delay and active lines */
cx25840_write(client, 0x470, hblank);
cx25840_write(client, 0x471,
-   0xff  (((hblank  8)  0x3) | (hactive  4)));
+ (((hblank  8)  0x3) | (hactive  4))  0xff);
cx25840_write(client, 0x472, hactive  4);
 
/* Sets burst gate delay */
@@ -888,13 +888,13 @@ void cx25840_std_setup(struct i2c_client *client)
/* Sets vertical blanking delay and active duration */
cx25840_write(client, 0x474, vblank);
cx25840_write(client, 0x475,
-   0xff  (((vblank  8)  0x3) | (vactive  4)));
+ (((vblank  8)  0x3) | (vactive  4))  0xff);
cx25840_write(client, 0x476, vactive  4);
cx25840_write(client, 0x477, vblank656);
 
/* Sets src decimation rate */
-   cx25840_write(client, 0x478, 0xff  src_decimation);
-   cx25840_write(client, 0x479, 0xff  (src_decimation  8));
+   cx25840_write(client, 0x478, src_decimation  0xff);
+   cx25840_write(client, 0x479, (src_decimation  8)  0xff);
 
/* Sets Luma and UV Low pass filters */
cx25840_write(client, 0x47a, luma_lpf  6 | ((uv_lpf  4)  0x30));
@@ -904,8 +904,8 @@ void cx25840_std_setup(struct i2c_client *client)
 
/* Sets SC Step*/
cx25840_write(client, 0x47c, sc);
-   cx25840_write(client, 0x47d, 0xff  sc  8);
-   cx25840_write(client, 0x47e, 0xff  sc  16);
+   cx25840_write(client, 0x47d, (sc  8)  0xff);
+   cx25840_write(client, 0x47e, (sc  16)  0xff);
 
/* Sets VBI parameters */
if (std  V4L2_STD_625_50) {
diff --git a/drivers/media/pci/cx18/cx18-av-core.c 
b/drivers/media/pci/cx18/cx18-av-core.c
index 2d3afe0..45be26c 100644
--- a/drivers/media/pci/cx18/cx18-av-core.c
+++ b/drivers/media/pci/cx18/cx18-av-core.c
@@ -490,8 +490,8 @@ void cx18_av_std_setup(struct cx18 *cx)
 
/* Sets horizontal blanking delay and active lines */
cx18_av_write(cx, 0x470, hblank);
-   cx18_av_write(cx, 0x471, 0xff  (((hblank  8)  0x3) |
-   (hactive  4)));
+   cx18_av_write(cx, 0x471,
+ (((hblank  8)  0x3) | (hactive  4))  0xff);
cx18_av_write(cx, 0x472, hactive  4);
 
/* Sets burst gate delay */
@@ -499,14 +499,14 @@ void cx18_av_std_setup(struct cx18 *cx)
 
/* Sets vertical blanking delay and active duration */
cx18_av_write(cx, 0x474, vblank);
-   cx18_av_write(cx, 0x475, 0xff  (((vblank  8)  0x3) |
-   (vactive  4)));
+   cx18_av_write(cx, 0x475,
+ (((vblank  8)  0x3) | (vactive  4))  0xff);
cx18_av_write(cx, 0x476, vactive  4);
cx18_av_write(cx, 0x477, vblank656);
 
/* Sets src decimation rate */
-   cx18_av_write(cx, 0x478, 0xff  src_decimation);
-   cx18_av_write(cx, 0x479, 0xff  (src_decimation  8));
+   cx18_av_write(cx, 0x478, src_decimation  0xff);
+   cx18_av_write(cx, 0x479, (src_decimation  8)  0xff);
 
/* Sets Luma and UV Low pass filters */
cx18_av_write(cx, 0x47a, luma_lpf  6 | ((uv_lpf  4)  0x30));
@@ -516,8 +516,8 @@ void cx18_av_std_setup(struct cx18 *cx)
 
/* Sets SC Step*/
cx18_av_write(cx, 0x47c, sc);
-   cx18_av_write(cx, 0x47d, 0xff  sc  8);
-   cx18_av_write(cx, 0x47e, 0xff  sc  16);
+   cx18_av_write(cx, 0x47d, (sc  8)  0xff);
+   cx18_av_write(cx, 0x47e, (sc  16)  0xff);
 
if (std  V4L2_STD_625_50) {
state-slicer_line_delay = 1;
-- 
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 04/11] dvb-net: Fix probable mask then right shift defects

2014-10-26 Thread Joe Perches
Precedence of  and  is not the same and is not left to right.
shift has higher precedence and should be done after the mask.

Add parentheses around the mask.

Signed-off-by: Joe Perches j...@perches.com
---
 drivers/media/dvb-core/dvb_net.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c
index 059e611..441814b 100644
--- a/drivers/media/dvb-core/dvb_net.c
+++ b/drivers/media/dvb-core/dvb_net.c
@@ -379,7 +379,9 @@ static void dvb_net_ule( struct net_device *dev, const u8 
*buf, size_t buf_len )
/* Check TS error conditions: sync_byte, 
transport_error_indicator, scrambling_control . */
if ((ts[0] != TS_SYNC) || (ts[1]  TS_TEI) || ((ts[3]  
TS_SC) != 0)) {
printk(KERN_WARNING %lu: Invalid TS cell: SYNC 
%#x, TEI %u, SC %#x.\n,
-  priv-ts_count, ts[0], ts[1]  TS_TEI  
7, ts[3]  0xC0  6);
+  priv-ts_count, ts[0],
+  (ts[1]  TS_TEI)  7,
+  (ts[3]  0xC0)  6);
 
/* Drop partly decoded SNDU, reset state, 
resync on PUSI. */
if (priv-ule_skb) {
-- 
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 00/11] treewide: mask then shift defects and style updates

2014-10-26 Thread Joe Perches
logical mask has lower precedence than shift but should be
done before the shift so parentheses are generally required.

And when masking with a fixed value after a shift, normal kernel
style has the shift on the left, then the shift on the right so
convert a few non-conforming uses.

Joe Perches (11):
  block: nvme-scsi: Fix probable mask then right shift defects
  radeon: evergreen: Fix probable mask then right shift defects
  aiptek: Fix probable mask then right shift defects
  dvb-net: Fix probable mask then right shift defects
  cx25840/cx18: Use standard ordering of mask and shift
  wm8350-core: Fix probable mask then right shift defect
  iwlwifi: dvm: Fix probable mask then right shift defect
  ssb: driver_chip_comon_pmu: Fix probable mask then right shift defect
  tty: ipwireless: Fix probable mask then right shift defects
  hwa-hc: Fix probable mask then right shift defect
  sound: ad1889: Fix probable mask then right shift defects

 drivers/block/nvme-scsi.c| 12 ++--
 drivers/gpu/drm/radeon/evergreen.c   |  3 ++-
 drivers/input/tablet/aiptek.c|  6 +++---
 drivers/media/dvb-core/dvb_net.c |  4 +++-
 drivers/media/i2c/cx25840/cx25840-core.c | 12 ++--
 drivers/media/pci/cx18/cx18-av-core.c| 16 
 drivers/mfd/wm8350-core.c|  2 +-
 drivers/net/wireless/iwlwifi/dvm/lib.c   |  4 ++--
 drivers/ssb/driver_chipcommon_pmu.c  |  4 ++--
 drivers/tty/ipwireless/hardware.c| 12 ++--
 drivers/usb/host/hwa-hc.c|  2 +-
 sound/pci/ad1889.c   |  8 
 12 files changed, 44 insertions(+), 41 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


[PATCH] MAINTAINERS: Update ivtv mailing lists as subscriber-only

2014-10-26 Thread Joe Perches
Mark these as subscriber-only mailing lists.

Signed-off-by: Joe Perches j...@perches.com
---
I got rejects not moderation emails for patches to these lists...

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

diff --git a/MAINTAINERS b/MAINTAINERS
index 1b063fc..2e353c7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2684,7 +2684,7 @@ F:drivers/net/wireless/cw1200/
 
 CX18 VIDEO4LINUX DRIVER
 M: Andy Walls awa...@md.metrocast.net
-L: ivtv-de...@ivtvdriver.org (moderated for non-subscribers)
+L: ivtv-de...@ivtvdriver.org (subscribers-only)
 L: linux-media@vger.kernel.org
 T: git git://linuxtv.org/media_tree.git
 W: http://linuxtv.org
@@ -5152,7 +5152,7 @@ F:drivers/media/tuners/it913x*
 
 IVTV VIDEO4LINUX DRIVER
 M: Andy Walls awa...@md.metrocast.net
-L: ivtv-de...@ivtvdriver.org (moderated for non-subscribers)
+L: ivtv-de...@ivtvdriver.org (subscribers-only)
 L: linux-media@vger.kernel.org
 T: git git://linuxtv.org/media_tree.git
 W: http://www.ivtvdriver.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


  1   2   3   >