Re: [greybus-dev] [PATCH v2] Staging: greybus: light: Prefer kcalloc over kzalloc

[Viresh Kumar]

On 08-05-17, 18:05, kart...@techveda.org wrote:
> From: Karthik Tummala 
> 
> Fixed following checkpatch.pl warning:
>  * WARNING: Prefer kcalloc over kzalloc with multiply
> 
> Instead of specifying no.of bytes * size as argument
> in kzalloc, prefer kcalloc.
> 
> Signed-off-by: Karthik Tummala 
> Reviewed-by: Rui Miguel Silva 
> ---
> Changes for v2:
> - Changed subject line & fixed typo as suggested by
>   Rui Miguel Silva
> ---
>  drivers/staging/greybus/light.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
> index 1681362..861a249 100644
> --- a/drivers/staging/greybus/light.c
> +++ b/drivers/staging/greybus/light.c
> @@ -1030,7 +1030,7 @@ static int gb_lights_light_config(struct gb_lights 
> *glights, u8 id)
>   light->channels_count = conf.channel_count;
>   light->name = kstrndup(conf.name, NAMES_MAX, GFP_KERNEL);
>  
> - light->channels = kzalloc(light->channels_count *
> + light->channels = kcalloc(light->channels_count,
> sizeof(struct gb_channel), GFP_KERNEL);
>   if (!light->channels)
>   return -ENOMEM;
> @@ -1167,7 +1167,7 @@ static int gb_lights_create_all(struct gb_lights 
> *glights)
>   if (ret < 0)
>   goto out;
>  
> - glights->lights = kzalloc(glights->lights_count *
> + glights->lights = kcalloc(glights->lights_count,
> sizeof(struct gb_light), GFP_KERNEL);
>   if (!glights->lights) {
>   ret = -ENOMEM;

Acked-by: Viresh Kumar 

-- 
viresh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 40/40] media: imx: set and propagate empty field, colorimetry params

[Steve Longerbeam]

On 05/08/2017 02:41 AM, Philipp Zabel wrote:

Hi Steve,

On Wed, 2017-04-12 at 17:45 -0700, Steve Longerbeam wrote:

This patch adds a call to imx_media_fill_empty_mbus_fields() in the
*_try_fmt() functions at the sink pads, to set empty field order and
colorimetry parameters.

If the field order is set to ANY, choose the currently set field order
at the sink pad. If the colorspace is set to DEFAULT, choose the
current colorspace at the sink pad.  If any of xfer_func, ycbcr_enc
or quantization are set to DEFAULT, either choose the current sink pad
setting, or the default setting for the new colorspace, if non-DEFAULT
colorspace was given.

Colorimetry is also propagated from sink to source pads anywhere
this has not already been done. The exception is ic-prpencvf at the
source pad, since the Image Converter outputs fixed quantization and
Y`CbCr encoding.

Signed-off-by: Steve Longerbeam 
---
 drivers/staging/media/imx/imx-ic-prp.c  |  5 ++-
 drivers/staging/media/imx/imx-ic-prpencvf.c | 25 +++---
 drivers/staging/media/imx/imx-media-csi.c   | 12 +--
 drivers/staging/media/imx/imx-media-utils.c | 53 +
 drivers/staging/media/imx/imx-media-vdic.c  |  7 ++--
 drivers/staging/media/imx/imx-media.h   |  3 +-
 6 files changed, 95 insertions(+), 10 deletions(-)


[...]

diff --git a/drivers/staging/media/imx/imx-media-utils.c 
b/drivers/staging/media/imx/imx-media-utils.c
index 7b2f92d..b07d0ae 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -464,6 +464,59 @@ int imx_media_init_mbus_fmt(struct v4l2_mbus_framefmt 
*mbus,
 }
 EXPORT_SYMBOL_GPL(imx_media_init_mbus_fmt);

+/*
+ * Check whether the field or colorimetry params in tryfmt are
+ * uninitialized, and if so fill them with the values from fmt.
+ * The exception is when tryfmt->colorspace has been initialized,
+ * if so all the further default colorimetry params can be derived
+ * from tryfmt->colorspace.
+ */
+void imx_media_fill_empty_mbus_fields(struct v4l2_mbus_framefmt *tryfmt,
+ struct v4l2_mbus_framefmt *fmt)
+{
+   /* fill field if necessary */
+   if (tryfmt->field == V4L2_FIELD_ANY)
+   tryfmt->field = fmt->field;
+
+   /* fill colorimetry if necessary */
+   if (tryfmt->colorspace == V4L2_COLORSPACE_DEFAULT) {
+   tryfmt->colorspace = fmt->colorspace;
+   if (tryfmt->xfer_func == V4L2_XFER_FUNC_DEFAULT)
+   tryfmt->xfer_func = fmt->xfer_func;
+   if (tryfmt->ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT)
+   tryfmt->ycbcr_enc = fmt->ycbcr_enc;
+   if (tryfmt->quantization == V4L2_QUANTIZATION_DEFAULT)
+   tryfmt->quantization = fmt->quantization;


According to Hans' latest comments, this could be changed to:

--8<--
From cca3cda9effcaca0891eb8044a79137023fed1c2 Mon Sep 17 00:00:00 2001
From: Philipp Zabel 
Date: Mon, 8 May 2017 11:38:05 +0200
Subject: [PATCH] fixup! media: imx: set and propagate default field,
 colorimetry

Signed-off-by: Philipp Zabel 
---
 drivers/staging/media/imx/imx-media-utils.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-utils.c 
b/drivers/staging/media/imx/imx-media-utils.c
index a8766489e8a18..ec2abd618cc44 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -497,12 +497,9 @@ void imx_media_fill_default_mbus_fields(struct 
v4l2_mbus_framefmt *tryfmt,
/* fill colorimetry if necessary */
if (tryfmt->colorspace == V4L2_COLORSPACE_DEFAULT) {
tryfmt->colorspace = fmt->colorspace;
-   if (tryfmt->xfer_func == V4L2_XFER_FUNC_DEFAULT)
-   tryfmt->xfer_func = fmt->xfer_func;
-   if (tryfmt->ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT)
-   tryfmt->ycbcr_enc = fmt->ycbcr_enc;
-   if (tryfmt->quantization == V4L2_QUANTIZATION_DEFAULT)
-   tryfmt->quantization = fmt->quantization;
+   tryfmt->xfer_func = fmt->xfer_func;
+   tryfmt->ycbcr_enc = fmt->ycbcr_enc;
+   tryfmt->quantization = fmt->quantization;
} else {
if (tryfmt->xfer_func == V4L2_XFER_FUNC_DEFAULT) {
tryfmt->xfer_func =



Hi Philipp, makes sense to me, I'll make the change in next revision.

Steve
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/3] staging: iio: meter: Fix the identations for proper alignments

[Harinath Nampally]

This patch fixes below checkpatch.pl kind of warnings:
CHECK: Alignment should match open parenthesis

Signed-off-by: Harinath Nampally 
---
 drivers/staging/iio/meter/ade7753.c | 55 ++---
 1 file changed, 27 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7753.c 
b/drivers/staging/iio/meter/ade7753.c
index cffe6bf..5d45a68 100644
--- a/drivers/staging/iio/meter/ade7753.c
+++ b/drivers/staging/iio/meter/ade7753.c
@@ -108,9 +108,8 @@ static int ade7753_spi_write_reg_8(struct device *dev,
return ret;
 }
 
-static int ade7753_spi_write_reg_16(struct device *dev,
-   u8 reg_address,
-   u16 value)
+static int ade7753_spi_write_reg_16(struct device *dev, u8 reg_address,
+   u16 value)
 {
int ret;
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
@@ -127,8 +126,8 @@ static int ade7753_spi_write_reg_16(struct device *dev,
 }
 
 static int ade7753_spi_read_reg_8(struct device *dev,
-   u8 reg_address,
-   u8 *val)
+ u8 reg_address,
+ u8 *val)
 {
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct ade7753_state *st = iio_priv(indio_dev);
@@ -137,7 +136,7 @@ static int ade7753_spi_read_reg_8(struct device *dev,
ret = spi_w8r8(st->us, ADE7753_READ_REG(reg_address));
if (ret < 0) {
dev_err(>us->dev, "problem when reading 8 bit register 
0x%02X",
-   reg_address);
+   reg_address);
return ret;
}
*val = ret;
@@ -146,8 +145,8 @@ static int ade7753_spi_read_reg_8(struct device *dev,
 }
 
 static int ade7753_spi_read_reg_16(struct device *dev,
-   u8 reg_address,
-   u16 *val)
+  u8 reg_address,
+  u16 *val)
 {
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct ade7753_state *st = iio_priv(indio_dev);
@@ -166,8 +165,8 @@ static int ade7753_spi_read_reg_16(struct device *dev,
 }
 
 static int ade7753_spi_read_reg_24(struct device *dev,
-   u8 reg_address,
-   u32 *val)
+  u8 reg_address,
+  u32 *val)
 {
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct ade7753_state *st = iio_priv(indio_dev);
@@ -190,7 +189,7 @@ static int ade7753_spi_read_reg_24(struct device *dev,
ret = spi_sync_transfer(st->us, xfers, ARRAY_SIZE(xfers));
if (ret) {
dev_err(>us->dev, "problem when reading 24 bit register 
0x%02X",
-   reg_address);
+   reg_address);
goto error_ret;
}
*val = (st->rx[0] << 16) | (st->rx[1] << 8) | st->rx[2];
@@ -201,8 +200,8 @@ static int ade7753_spi_read_reg_24(struct device *dev,
 }
 
 static ssize_t ade7753_read_8bit(struct device *dev,
-   struct device_attribute *attr,
-   char *buf)
+struct device_attribute *attr,
+char *buf)
 {
int ret;
u8 val;
@@ -216,8 +215,8 @@ static ssize_t ade7753_read_8bit(struct device *dev,
 }
 
 static ssize_t ade7753_read_16bit(struct device *dev,
-   struct device_attribute *attr,
-   char *buf)
+ struct device_attribute *attr,
+ char *buf)
 {
int ret;
u16 val;
@@ -231,8 +230,8 @@ static ssize_t ade7753_read_16bit(struct device *dev,
 }
 
 static ssize_t ade7753_read_24bit(struct device *dev,
-   struct device_attribute *attr,
-   char *buf)
+ struct device_attribute *attr,
+ char *buf)
 {
int ret;
u32 val;
@@ -246,9 +245,9 @@ static ssize_t ade7753_read_24bit(struct device *dev,
 }
 
 static ssize_t ade7753_write_8bit(struct device *dev,
-   struct device_attribute *attr,
-   const char *buf,
-   size_t len)
+ struct device_attribute *attr,
+ const char *buf,
+ size_t len)
 {
struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
int ret;
@@ -264,9 +263,9 @@ static ssize_t ade7753_write_8bit(struct device *dev,
 }
 
 static ssize_t ade7753_write_16bit(struct device *dev,
-   struct device_attribute *attr,
-   const char *buf,
-   size_t len)
+  struct device_attribute *attr,
+  const char *buf,
+  size_t len)
 {
struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
int ret;
@@ -451,8 +450,8 

[PATCH 3/3] staging: iio: meter: Replace symbolic permissions with octal permissions

[Harinath Nampally]

This patch fixes below kind of warnings:
WARNING: Symbolic permissions 'S_IXXX | S_IXXX' are not preferred.

Below errors are false positives:
ade7753.c:382: ERROR: Use 4 digit octal (0777) not decimal permissions
ade7753.c:386: ERROR: Use 4 digit octal (0777) not decimal permissions

Signed-off-by: Harinath Nampally 
---
 drivers/staging/iio/meter/ade7753.c | 46 ++---
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7753.c 
b/drivers/staging/iio/meter/ade7753.c
index 5d45a68..2534bd0 100644
--- a/drivers/staging/iio/meter/ade7753.c
+++ b/drivers/staging/iio/meter/ade7753.c
@@ -298,92 +298,92 @@ static IIO_DEV_ATTR_AENERGY(ade7753_read_24bit, 
ADE7753_AENERGY);
 static IIO_DEV_ATTR_LAENERGY(ade7753_read_24bit, ADE7753_LAENERGY);
 static IIO_DEV_ATTR_VAENERGY(ade7753_read_24bit, ADE7753_VAENERGY);
 static IIO_DEV_ATTR_LVAENERGY(ade7753_read_24bit, ADE7753_LVAENERGY);
-static IIO_DEV_ATTR_CFDEN(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_CFDEN(0644,
ade7753_read_16bit,
ade7753_write_16bit,
ADE7753_CFDEN);
-static IIO_DEV_ATTR_CFNUM(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_CFNUM(0644,
ade7753_read_8bit,
ade7753_write_8bit,
ADE7753_CFNUM);
 static IIO_DEV_ATTR_CHKSUM(ade7753_read_8bit, ADE7753_CHKSUM);
-static IIO_DEV_ATTR_PHCAL(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_PHCAL(0644,
ade7753_read_16bit,
ade7753_write_16bit,
ADE7753_PHCAL);
-static IIO_DEV_ATTR_APOS(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_APOS(0644,
ade7753_read_16bit,
ade7753_write_16bit,
ADE7753_APOS);
-static IIO_DEV_ATTR_SAGCYC(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_SAGCYC(0644,
ade7753_read_8bit,
ade7753_write_8bit,
ADE7753_SAGCYC);
-static IIO_DEV_ATTR_SAGLVL(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_SAGLVL(0644,
ade7753_read_8bit,
ade7753_write_8bit,
ADE7753_SAGLVL);
-static IIO_DEV_ATTR_LINECYC(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_LINECYC(0644,
ade7753_read_8bit,
ade7753_write_8bit,
ADE7753_LINECYC);
-static IIO_DEV_ATTR_WDIV(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_WDIV(0644,
ade7753_read_8bit,
ade7753_write_8bit,
ADE7753_WDIV);
-static IIO_DEV_ATTR_IRMS(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_IRMS(0644,
ade7753_read_24bit,
NULL,
ADE7753_IRMS);
-static IIO_DEV_ATTR_VRMS(S_IRUGO,
+static IIO_DEV_ATTR_VRMS(0444,
ade7753_read_24bit,
NULL,
ADE7753_VRMS);
-static IIO_DEV_ATTR_IRMSOS(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_IRMSOS(0644,
ade7753_read_16bit,
ade7753_write_16bit,
ADE7753_IRMSOS);
-static IIO_DEV_ATTR_VRMSOS(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_VRMSOS(0644,
ade7753_read_16bit,
ade7753_write_16bit,
ADE7753_VRMSOS);
-static IIO_DEV_ATTR_WGAIN(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_WGAIN(0644,
ade7753_read_16bit,
ade7753_write_16bit,
ADE7753_WGAIN);
-static IIO_DEV_ATTR_VAGAIN(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_VAGAIN(0644,
ade7753_read_16bit,
ade7753_write_16bit,
ADE7753_VAGAIN);
-static IIO_DEV_ATTR_PGA_GAIN(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_PGA_GAIN(0644,
ade7753_read_16bit,
ade7753_write_16bit,
ADE7753_GAIN);
-static IIO_DEV_ATTR_IPKLVL(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_IPKLVL(0644,
ade7753_read_8bit,
ade7753_write_8bit,
ADE7753_IPKLVL);
-static IIO_DEV_ATTR_VPKLVL(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_VPKLVL(0644,
ade7753_read_8bit,
ade7753_write_8bit,
ADE7753_VPKLVL);
-static IIO_DEV_ATTR_IPEAK(S_IRUGO,
+static IIO_DEV_ATTR_IPEAK(0444,
ade7753_read_24bit,
NULL,
ADE7753_IPEAK);
-static IIO_DEV_ATTR_VPEAK(S_IRUGO,
+static IIO_DEV_ATTR_VPEAK(0444,
ade7753_read_24bit,
NULL,
ADE7753_VPEAK);
-static IIO_DEV_ATTR_VPERIOD(S_IRUGO,
+static IIO_DEV_ATTR_VPERIOD(0444,
ade7753_read_16bit,
NULL,
ADE7753_PERIOD);
-static IIO_DEV_ATTR_CH_OFF(1, S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_CH_OFF(1, 0644,
ade7753_read_8bit,
ade7753_write_8bit,
ADE7753_CH1OS);
-static IIO_DEV_ATTR_CH_OFF(2, S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_CH_OFF(2, 0644,
ade7753_read_8bit,
ade7753_write_8bit,
ADE7753_CH2OS);

[PATCH 0/3] coding style warnings fixes

[Harinath Nampally]

This patchset is for fixes reported by checkpatch.pl

Please find the following related patches:
[PATCH 1/3] staging: iio: meter: Add the comment for mutex definition
[PATCH 2/3] staging: iio: meter: Fix the identations for proper alignments
[PATCH 3/3] staging: iio: meter: Replace symbolic permissions with octal 
permissions
-
Harinath Nampally (3):
  staging: iio: meter: Add the comment for mutex definition
  staging: iio: meter: Fix the identations for proper alignments
  staging: iio: meter: Replace symbolic permissions with octal permissions 

 drivers/staging/iio/meter/ade7753.c | 104 ++--
 1 file changed, 52 insertions(+), 52 deletions(-)

-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/3] staging: iio: meter: Add the comment for mutex definition

[Harinath Nampally]

This patch fixes below checkpatch.pl warning:
CHECK: struct mutex definition without comment

Signed-off-by: Harinath Nampally 
---
 drivers/staging/iio/meter/ade7753.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/iio/meter/ade7753.c 
b/drivers/staging/iio/meter/ade7753.c
index b71fbd3..cffe6bf 100644
--- a/drivers/staging/iio/meter/ade7753.c
+++ b/drivers/staging/iio/meter/ade7753.c
@@ -78,12 +78,13 @@
 /**
  * struct ade7753_state - device instance specific data
  * @us: actual spi_device
+ * @buf_lock:   mutex to protect tx and rx
  * @tx: transmit buffer
  * @rx: receive buffer
- * @buf_lock:   mutex to protect tx and rx
  **/
 struct ade7753_state {
struct spi_device   *us;
+ /* mutex to protect tx and rx */
struct mutexbuf_lock;
u8  tx[ADE7753_MAX_TX] cacheline_aligned;
u8  rx[ADE7753_MAX_RX];
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: octeon-usb: use correct function for hcd cleanup

[Anton Bondarenko]

Use usb_put_hdc to release hdc allocated by usb_create_hcd.
This is needed to handle sub-allocations and HCD sharing correctly.

Signed-off-by: Anton Bondarenko 
---
 drivers/staging/octeon-usb/octeon-hcd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/octeon-usb/octeon-hcd.c 
b/drivers/staging/octeon-usb/octeon-hcd.c
index 9a7858a300fd..068aece25d37 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -3659,14 +3659,14 @@ static int octeon_usb_probe(struct platform_device 
*pdev)
status = cvmx_usb_initialize(dev, usb);
if (status) {
dev_dbg(dev, "USB initialization failed with %d\n", status);
-   kfree(hcd);
+   usb_put_hcd(hcd);
return -1;
}
 
status = usb_add_hcd(hcd, irq, 0);
if (status) {
dev_dbg(dev, "USB add HCD failed with %d\n", status);
-   kfree(hcd);
+   usb_put_hcd(hcd);
return -1;
}
device_wakeup_enable(hcd->self.controller);
@@ -3691,7 +3691,7 @@ static int octeon_usb_remove(struct platform_device *pdev)
if (status)
dev_dbg(dev, "USB shutdown failed with %d\n", status);
 
-   kfree(hcd);
+   usb_put_hcd(hcd);
 
return 0;
 }
-- 
2.11.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] ATOMISP: Tidies up code warnings and errors in file

[Mark Railton]

Cleared up some errors and warnings in
drivers/staging/media/atomisp/i2c/ap1302.c

Signed-off-by: Mark Railton 
---
 drivers/staging/media/atomisp/i2c/ap1302.c | 83 ++
 1 file changed, 50 insertions(+), 33 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/ap1302.c 
b/drivers/staging/media/atomisp/i2c/ap1302.c
index bacffbe..8c33a35 100644
--- a/drivers/staging/media/atomisp/i2c/ap1302.c
+++ b/drivers/staging/media/atomisp/i2c/ap1302.c
@@ -11,11 +11,6 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
- * 02110-1301, USA.
- *
  */
 
 #include "../include/linux/atomisp.h"
@@ -162,9 +157,11 @@ static struct ap1302_context_info context_info[] = {
{CNTX_HINF_CTRL, AP1302_REG16, "hinf_ctrl"},
 };
 
-/* This array stores the description list for metadata.
-   The metadata contains exposure settings and face
-   detection results. */
+/*
+ *  This array stores the description list for metadata.
+ *  The metadata contains exposure settings and face
+ *  detection results.
+ */
 static u16 ap1302_ss_list[] = {
0xb01c, /* From 0x0186 with size 0x1C are exposure settings. */
0x0186,
@@ -213,6 +210,7 @@ static int ap1302_i2c_write_reg(struct v4l2_subdev *sd,
struct ap1302_device *dev = to_ap1302_device(sd);
struct i2c_client *client = v4l2_get_subdevdata(sd);
int ret;
+
if (len == AP1302_REG16)
ret = regmap_write(dev->regmap16, reg, val);
else if (len == AP1302_REG32)
@@ -236,11 +234,13 @@ static u16
 ap1302_calculate_context_reg_addr(enum ap1302_contexts context, u16 offset)
 {
u16 reg_addr;
-   /* The register offset is defined according to preview/video registers.
-  Preview and video context have the same register definition.
-  But snapshot context does not have register S1_SENSOR_MODE.
-  When setting snapshot registers, if the offset exceeds
-  S1_SENSOR_MODE, the actual offset needs to minus 2. */
+   /*
+*  The register offset is defined according to preview/video registers.
+*  Preview and video context have the same register definition.
+*  But snapshot context does not have register S1_SENSOR_MODE.
+*  When setting snapshot registers, if the offset exceeds
+*  S1_SENSOR_MODE, the actual offset needs to minus 2.
+*/
if (context == CONTEXT_SNAPSHOT) {
if (offset == CNTX_S1_SENSOR_MODE)
return 0;
@@ -261,6 +261,7 @@ static int ap1302_read_context_reg(struct v4l2_subdev *sd,
 {
struct ap1302_device *dev = to_ap1302_device(sd);
u16 reg_addr = ap1302_calculate_context_reg_addr(context, offset);
+
if (reg_addr == 0)
return -EINVAL;
return ap1302_i2c_read_reg(sd, reg_addr, len,
@@ -272,6 +273,7 @@ static int ap1302_write_context_reg(struct v4l2_subdev *sd,
 {
struct ap1302_device *dev = to_ap1302_device(sd);
u16 reg_addr = ap1302_calculate_context_reg_addr(context, offset);
+
if (reg_addr == 0)
return -EINVAL;
return ap1302_i2c_write_reg(sd, reg_addr, len,
@@ -284,7 +286,9 @@ static int ap1302_dump_context_reg(struct v4l2_subdev *sd,
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ap1302_device *dev = to_ap1302_device(sd);
int i;
+
dev_dbg(>dev, "Dump registers for context[%d]:\n", context);
+
for (i = 0; i < ARRAY_SIZE(context_info); i++) {
struct ap1302_context_info *info = _info[i];
u8 *var = (u8 *)>cntx_config[context] + info->offset;
@@ -308,6 +312,7 @@ static int ap1302_request_firmware(struct v4l2_subdev *sd)
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ap1302_device *dev = to_ap1302_device(sd);
int ret;
+
ret = request_firmware(>fw, "ap1302_fw.bin", >dev);
if (ret)
dev_err(>dev,
@@ -315,12 +320,14 @@ static int ap1302_request_firmware(struct v4l2_subdev *sd)
return ret;
 }
 
-/* When loading firmware, host writes firmware data from address 0x8000.
-   When the address reaches 0x9FFF, the next address should return to 0x8000.
-   This function handles this address window and load firmware data to AP1302.
-   win_pos indicates the offset within this window. Firmware loading procedure
-   may call this function several times. win_pos records the current position
-   that has been written to.*/
+/*
+ *  When loading firmware, host writes firmware data from address 0x8000.
+ *  When the address reaches 0x9FFF, the next address should return to 0x8000.
+ *  This function handles this address window and load 

Re: [PATCH] staging : rtl8188eu : remove unnecessary else

[Joe Perches]

On Mon, 2017-05-08 at 14:13 +0530, Surender Polsani wrote:
> according to coding style else is not generally
> useful after a break or return
> 
> Signed-off-by: Surender Polsani 
> ---
>  drivers/staging/rtl8188eu/hal/rtl8188e_dm.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c 
> b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
> index d04b7fb..7420f55 100644
> --- a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
> +++ b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
> @@ -212,8 +212,7 @@ u8 rtw_hal_antdiv_before_linked(struct adapter *Adapter)
>  
>   rtw_antenna_select_cmd(Adapter, dm_swat_tbl->CurAntenna, false);
>   return true;
> - } else {
> + }
>   dm_swat_tbl->SWAS_NoLink_State = 0;
>   return false;
> - }
>  }

It's just fine how else after return is used here.

In the future you should reflow indentation after
removing braces though.

The function might be restructured a bit to return
false before ever returning true.

Something like:
---
 drivers/staging/rtl8188eu/hal/rtl8188e_dm.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c 
b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
index d04b7fbb71e1..1de9bcaa58b8 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
@@ -198,22 +198,22 @@ u8 rtw_hal_antdiv_before_linked(struct adapter *Adapter)
struct sw_ant_switch *dm_swat_tbl = _odm->DM_SWAT_Table;
struct mlme_priv *pmlmepriv = &(Adapter->mlmepriv);
 
-   /*  Condition that does not need to use antenna diversity. */
+   /* Condition that does not need to use antenna diversity */
if (Adapter->HalData->AntDivCfg == 0)
return false;
 
if (check_fwstate(pmlmepriv, _FW_LINKED))
return false;
 
-   if (dm_swat_tbl->SWAS_NoLink_State == 0) {
-   /* switch channel */
-   dm_swat_tbl->SWAS_NoLink_State = 1;
-   dm_swat_tbl->CurAntenna = (dm_swat_tbl->CurAntenna == 
Antenna_A) ? Antenna_B : Antenna_A;
-
-   rtw_antenna_select_cmd(Adapter, dm_swat_tbl->CurAntenna, false);
-   return true;
-   } else {
+   if (dm_swat_tbl->SWAS_NoLink_State != 0) {
dm_swat_tbl->SWAS_NoLink_State = 0;
return false;
}
+
+   /* switch channel */
+   dm_swat_tbl->SWAS_NoLink_State = 1;
+   dm_swat_tbl->CurAntenna = dm_swat_tbl->CurAntenna == Antenna_A ? 
Antenna_B : Antenna_A;
+   rtw_antenna_select_cmd(Adapter, dm_swat_tbl->CurAntenna, false);
+
+   return true;
 }

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] android: binder: check result of binder_get_thread() in binder_poll()

[Doug Anderson]

Dmitry,

On Mon, May 8, 2017 at 1:46 PM, John Stultz  wrote:
> On Mon, May 8, 2017 at 1:43 PM, Dmitry Torokhov
>  wrote:
>> If binder_get_thread() fails to give us a thread data, we should avoid
>> dereferencing a NULL pointer and return POLLERR instead.
>>
>> Signed-off-by: Dmitry Torokhov 
>
> Pulling Todd Kjos in on this too.
> -john
>
>> ---
>>  drivers/android/binder.c | 12 
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
>> index aae4d8d4be36..66ed714fedd5 100644
>> --- a/drivers/android/binder.c
>> +++ b/drivers/android/binder.c
>> @@ -3103,18 +3103,22 @@ static unsigned int binder_poll(struct file *filp,
>> struct poll_table_struct *wait)
>>  {
>> struct binder_proc *proc = filp->private_data;
>> -   struct binder_thread *thread = NULL;
>> +   struct binder_thread *thread;
>> int wait_for_proc_work;
>>
>> binder_lock(__func__);
>>
>> thread = binder_get_thread(proc);
>> -
>> -   wait_for_proc_work = thread->transaction_stack == NULL &&
>> -   list_empty(>todo) && thread->return_error == BR_OK;
>> +   if (thread)
>> +   wait_for_proc_work = thread->transaction_stack == NULL &&
>> +list_empty(>todo) &&
>> +thread->return_error == BR_OK;
>>
>> binder_unlock(__func__);
>>
>> +   if (!thread)
>> +   return POLLERR;
>> +
>> if (wait_for_proc_work) {
>> if (binder_has_proc_work(proc, thread))
>> return POLLIN;
>> --

I'm no expert on the poll function, but I agree that it's wise to
check the result of binder_get_thread() since it can return NULL.
FWIW:

Reviewed-by: Douglas Anderson 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] android: binder: check result of binder_get_thread() in binder_poll()

[John Stultz]

On Mon, May 8, 2017 at 1:43 PM, Dmitry Torokhov
 wrote:
> If binder_get_thread() fails to give us a thread data, we should avoid
> dereferencing a NULL pointer and return POLLERR instead.
>
> Signed-off-by: Dmitry Torokhov 

Pulling Todd Kjos in on this too.
-john

> ---
>  drivers/android/binder.c | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index aae4d8d4be36..66ed714fedd5 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -3103,18 +3103,22 @@ static unsigned int binder_poll(struct file *filp,
> struct poll_table_struct *wait)
>  {
> struct binder_proc *proc = filp->private_data;
> -   struct binder_thread *thread = NULL;
> +   struct binder_thread *thread;
> int wait_for_proc_work;
>
> binder_lock(__func__);
>
> thread = binder_get_thread(proc);
> -
> -   wait_for_proc_work = thread->transaction_stack == NULL &&
> -   list_empty(>todo) && thread->return_error == BR_OK;
> +   if (thread)
> +   wait_for_proc_work = thread->transaction_stack == NULL &&
> +list_empty(>todo) &&
> +thread->return_error == BR_OK;
>
> binder_unlock(__func__);
>
> +   if (!thread)
> +   return POLLERR;
> +
> if (wait_for_proc_work) {
> if (binder_has_proc_work(proc, thread))
> return POLLIN;
> --
> 2.13.0.rc1.294.g07d810a77f-goog
>
>
> --
> Dmitry
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: iio: meter: ade7854: Fix symbolic permissions to octal permissions

[eddi1983]

From: Christoph Fanelsa 

Fixed the checkpatch warnings (task #10 of eudyptula challenge) of prefered 
octal permissions over symbolic ones

Signed-off-by: Christoph Fanelsa 
Cc: Michael Hennerich 
Cc: Jonathan Cameron 
Cc: Hartmut Knaack 
Cc: Peter Meerwald-Stadler 
Cc: Greg Kroah-Hartman 
Cc: linux-...@vger.kernel.org
Cc: de...@driverdev.osuosl.org 
Cc: linux-ker...@vger.kernel.org
---
 drivers/staging/iio/meter/ade7854.c | 88 ++---
 1 file changed, 44 insertions(+), 44 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7854.c 
b/drivers/staging/iio/meter/ade7854.c
index c6cffc11b0ba..70612da64a8b 100644
--- a/drivers/staging/iio/meter/ade7854.c
+++ b/drivers/staging/iio/meter/ade7854.c
@@ -186,127 +186,127 @@ static int ade7854_reset(struct device *dev)
return st->write_reg_16(dev, ADE7854_CONFIG, val);
 }
 
-static IIO_DEV_ATTR_AIGAIN(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_AIGAIN(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_AIGAIN);
-static IIO_DEV_ATTR_BIGAIN(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_BIGAIN(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_BIGAIN);
-static IIO_DEV_ATTR_CIGAIN(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_CIGAIN(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_CIGAIN);
-static IIO_DEV_ATTR_NIGAIN(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_NIGAIN(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_NIGAIN);
-static IIO_DEV_ATTR_AVGAIN(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_AVGAIN(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_AVGAIN);
-static IIO_DEV_ATTR_BVGAIN(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_BVGAIN(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_BVGAIN);
-static IIO_DEV_ATTR_CVGAIN(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_CVGAIN(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_CVGAIN);
-static IIO_DEV_ATTR_APPARENT_POWER_A_GAIN(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_APPARENT_POWER_A_GAIN(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_AVAGAIN);
-static IIO_DEV_ATTR_APPARENT_POWER_B_GAIN(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_APPARENT_POWER_B_GAIN(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_BVAGAIN);
-static IIO_DEV_ATTR_APPARENT_POWER_C_GAIN(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_APPARENT_POWER_C_GAIN(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_CVAGAIN);
-static IIO_DEV_ATTR_ACTIVE_POWER_A_OFFSET(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_ACTIVE_POWER_A_OFFSET(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_AWATTOS);
-static IIO_DEV_ATTR_ACTIVE_POWER_B_OFFSET(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_ACTIVE_POWER_B_OFFSET(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_BWATTOS);
-static IIO_DEV_ATTR_ACTIVE_POWER_C_OFFSET(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_ACTIVE_POWER_C_OFFSET(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_CWATTOS);
-static IIO_DEV_ATTR_REACTIVE_POWER_A_GAIN(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_REACTIVE_POWER_A_GAIN(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_AVARGAIN);
-static IIO_DEV_ATTR_REACTIVE_POWER_B_GAIN(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_REACTIVE_POWER_B_GAIN(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_BVARGAIN);
-static IIO_DEV_ATTR_REACTIVE_POWER_C_GAIN(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_REACTIVE_POWER_C_GAIN(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_CVARGAIN);
-static IIO_DEV_ATTR_REACTIVE_POWER_A_OFFSET(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_REACTIVE_POWER_A_OFFSET(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_AVAROS);
-static IIO_DEV_ATTR_REACTIVE_POWER_B_OFFSET(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_REACTIVE_POWER_B_OFFSET(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_BVAROS);
-static IIO_DEV_ATTR_REACTIVE_POWER_C_OFFSET(S_IWUSR | S_IRUGO,
+static IIO_DEV_ATTR_REACTIVE_POWER_C_OFFSET(0644,
ade7854_read_24bit,
ade7854_write_24bit,
ADE7854_CVAROS);
-static IIO_DEV_ATTR_VPEAK(S_IWUSR | S_IRUGO,

[PATCH 3/4] Staging: rtl8712: ieee80211: fixed camelcase coding style

[Jaya Durga]

Fixed coding style issue

The following are the renames  done to avoid camecase 
#Configuration-> configuration
#BeaconPeriod -> beacon_period
#ATIMWindow   -> atim_window
#DSConfig -> ds_config
#FHConfig -> fh_config
#HopPattern   -> hop_pattern
#HopSet   -> hop_set
#DwellTime -> dwell_time

Signed-off-by: Jaya Durga 
---
 drivers/staging/rtl8712/ieee80211.c   |  7 ---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 25 +
 drivers/staging/rtl8712/rtl871x_ioctl_rtl.c   |  9 -
 drivers/staging/rtl8712/rtl871x_mlme.c| 20 ++--
 drivers/staging/rtl8712/wlan_bssdef.h | 16 
 5 files changed, 39 insertions(+), 38 deletions(-)

diff --git a/drivers/staging/rtl8712/ieee80211.c 
b/drivers/staging/rtl8712/ieee80211.c
index 66d9daf..42cb46c 100644
--- a/drivers/staging/rtl8712/ieee80211.c
+++ b/drivers/staging/rtl8712/ieee80211.c
@@ -174,7 +174,8 @@ int r8712_generate_ie(struct registry_priv *pregistrypriv)
sz += 8;
ie += sz;
/*beacon interval : 2bytes*/
-   *(__le16 *)ie = 
cpu_to_le16((u16)pdev_network->Configuration.BeaconPeriod);
+   *(__le16 *)ie = cpu_to_le16(
+   (u16)pdev_network->configuration.beacon_period);
sz += 2;
ie += 2;
/*capability info*/
@@ -203,10 +204,10 @@ int r8712_generate_ie(struct registry_priv *pregistrypriv)
}
/*DS parameter set*/
ie = r8712_set_ie(ie, _DSSET_IE_, 1,
- (u8 *)_network->Configuration.DSConfig, );
+ (u8 *)_network->configuration.ds_config, );
/*IBSS Parameter Set*/
ie = r8712_set_ie(ie, _IBSS_PARA_IE_, 2,
- (u8 *)_network->Configuration.ATIMWindow, );
+ (u8 *)_network->configuration.atim_window, );
return sz;
 }
 
diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c 
b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index f4167f1..6adb4e5 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -150,12 +150,12 @@ static noinline_for_stack char *translate_scan(struct 
_adapter *padapter,
u16 cap, ht_cap = false, mcs_rate;
u8 rssi;
 
-   if ((pnetwork->network.Configuration.DSConfig < 1) ||
-   (pnetwork->network.Configuration.DSConfig > 14)) {
-   if (pnetwork->network.Configuration.DSConfig < 1)
-   pnetwork->network.Configuration.DSConfig = 1;
+   if ((pnetwork->network.configuration.ds_config < 1) ||
+   (pnetwork->network.configuration.ds_config > 14)) {
+   if (pnetwork->network.configuration.ds_config < 1)
+   pnetwork->network.configuration.ds_config = 1;
else
-   pnetwork->network.Configuration.DSConfig = 14;
+   pnetwork->network.configuration.ds_config = 14;
}
/* AP MAC address */
iwe.cmd = SIOCGIWAP;
@@ -212,18 +212,19 @@ static noinline_for_stack char *translate_scan(struct 
_adapter *padapter,
iwe.cmd = SIOCGIWFREQ;
{
/*  check legal index */
-   u8 dsconfig = pnetwork->network.Configuration.DSConfig;
+   u8 dsconfig = pnetwork->network.configuration.ds_config;
 
if (dsconfig >= 1 && dsconfig <= sizeof(
ieee80211_wlan_frequencies) / sizeof(long))
-   iwe.u.freq.m = (s32)(ieee80211_wlan_frequencies[
-  pnetwork->network.Configuration.
-  DSConfig - 1] * 10);
+   iwe.u.freq.m = (s32)
+   (ieee80211_wlan_frequencies
+   [pnetwork->network.configuration.ds_config
+   - 1] * 10);
else
iwe.u.freq.m = 0;
}
iwe.u.freq.e = (s16)1;
-   iwe.u.freq.i = (u8)pnetwork->network.Configuration.DSConfig;
+   iwe.u.freq.i = (u8)pnetwork->network.configuration.ds_config;
start = iwe_stream_add_event(info, start, stop, ,
IW_EV_FREQ_LEN);
/* Add encryption capability */
@@ -702,9 +703,9 @@ static int r8711_wx_get_freq(struct net_device *dev,
 
if (check_fwstate(pmlmepriv, _FW_LINKED)) {
wrqu->freq.m = ieee80211_wlan_frequencies[
-  pcur_bss->Configuration.DSConfig - 1] * 10;
+  pcur_bss->configuration.ds_config - 1] * 10;
wrqu->freq.e = 1;
-   wrqu->freq.i = pcur_bss->Configuration.DSConfig;
+   wrqu->freq.i = pcur_bss->configuration.ds_config;
} else {
return 

[PATCH 4/4] Staging: rtl8712: ieee80211:fixed warning line over 80 characters coding style issue

[Jaya Durga]

fixed warning line over 80 characters coding style issue

Signed-off-by: Jaya Durga 
---
 drivers/staging/rtl8712/ieee80211.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8712/ieee80211.c 
b/drivers/staging/rtl8712/ieee80211.c
index 42cb46c..a7152df 100644
--- a/drivers/staging/rtl8712/ieee80211.c
+++ b/drivers/staging/rtl8712/ieee80211.c
@@ -222,7 +222,9 @@ unsigned char *r8712_get_wpa_ie(unsigned char *pie, int 
*wpa_ie_len, int limit)
pbuf = r8712_get_ie(pbuf, _WPA_IE_ID_, , limit);
if (pbuf) {
/*check if oui matches...*/
-   if (memcmp((pbuf + 2), wpa_oui_type, 
sizeof(wpa_oui_type)))
+   if (memcmp(
+   (pbuf + 2), wpa_oui_type,
+   sizeof(wpa_oui_type)))
goto check_next_ie;
/*check version...*/
memcpy((u8 *), (pbuf + 6), sizeof(val16));
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: media: fix one code style problem

[Remco Verhoef]

this patch will fix one code style problem (ctx:WxE), space
prohibited before that

Signed-off-by: Remco Verhoef 
---
 .../staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c| 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c 
b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
index 5b4506a..b0f9188 100644
--- a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
@@ -51,7 +51,7 @@ struct gmin_subdev {
 
 static struct gmin_subdev gmin_subdevs[MAX_SUBDEVS];
 
-static enum { PMIC_UNSET = 0, PMIC_REGULATOR, PMIC_AXP, PMIC_TI ,
+static enum { PMIC_UNSET = 0, PMIC_REGULATOR, PMIC_AXP, PMIC_TI,
PMIC_CRYSTALCOVE } pmic_id;
 
 /* The atomisp uses type==0 for the end-of-list marker, so leave space. */
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3] staging: media: atomisp: Fix indentation to tabs

[Gideon Sheril]

Fixing following checkpatch warnnings/errors:
ERROR: code indent should use tabs where possible
WARNING: please, no spaces at the start of a line

Signed-off-by: Gideon Sheril 
---
 Changed from previuos version:
  - Fixed alignment issues
  - Verified no new warning or errors appear after patch

 .../platform/intel-mid/atomisp_gmin_platform.c | 150 ++---
 1 file changed, 75 insertions(+), 75 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c 
b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
index 5b4506a..4eb0c91 100644
--- a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
@@ -152,13 +152,13 @@ const struct camera_af_platform_data 
*camera_get_af_platform_data(void)
 EXPORT_SYMBOL_GPL(camera_get_af_platform_data);
 
 int atomisp_register_i2c_module(struct v4l2_subdev *subdev,
-struct camera_sensor_platform_data *plat_data,
-enum intel_v4l2_subdev_type type)
+   struct camera_sensor_platform_data *plat_data,
+   enum intel_v4l2_subdev_type type)
 {
int i;
struct i2c_board_info *bi;
struct gmin_subdev *gs;
-struct i2c_client *client = v4l2_get_subdevdata(subdev);
+   struct i2c_client *client = v4l2_get_subdevdata(subdev);
struct acpi_device *adev;
 
dev_info(>dev, "register atomisp i2c module type %d\n", type);
@@ -270,45 +270,45 @@ static const struct gmin_cfg_var t100_vars[] = {
 };
 
 static const struct gmin_cfg_var mrd7_vars[] = {
-{"INT33F8:00_CamType", "1"},
-{"INT33F8:00_CsiPort", "1"},
-{"INT33F8:00_CsiLanes","2"},
-{"INT33F8:00_CsiFmt","13"},
-{"INT33F8:00_CsiBayer", "0"},
-{"INT33F8:00_CamClk", "0"},
-{"INT33F9:00_CamType", "1"},
-{"INT33F9:00_CsiPort", "0"},
-{"INT33F9:00_CsiLanes","1"},
-{"INT33F9:00_CsiFmt","13"},
-{"INT33F9:00_CsiBayer", "0"},
-{"INT33F9:00_CamClk", "1"},
-{},
+   {"INT33F8:00_CamType", "1"},
+   {"INT33F8:00_CsiPort", "1"},
+   {"INT33F8:00_CsiLanes", "2"},
+   {"INT33F8:00_CsiFmt", "13"},
+   {"INT33F8:00_CsiBayer", "0"},
+   {"INT33F8:00_CamClk", "0"},
+   {"INT33F9:00_CamType", "1"},
+   {"INT33F9:00_CsiPort", "0"},
+   {"INT33F9:00_CsiLanes", "1"},
+   {"INT33F9:00_CsiFmt", "13"},
+   {"INT33F9:00_CsiBayer", "0"},
+   {"INT33F9:00_CamClk", "1"},
+   {},
 };
 
 static const struct gmin_cfg_var ecs7_vars[] = {
-{"INT33BE:00_CsiPort", "1"},
-{"INT33BE:00_CsiLanes","2"},
-{"INT33BE:00_CsiFmt","13"},
-{"INT33BE:00_CsiBayer", "2"},
-{"INT33BE:00_CamClk", "0"},
-{"INT33F0:00_CsiPort", "0"},
-{"INT33F0:00_CsiLanes","1"},
-{"INT33F0:00_CsiFmt","13"},
-{"INT33F0:00_CsiBayer", "0"},
-{"INT33F0:00_CamClk", "1"},
-{"gmin_V2P8GPIO","402"},
-{},
+   {"INT33BE:00_CsiPort", "1"},
+   {"INT33BE:00_CsiLanes", "2"},
+   {"INT33BE:00_CsiFmt", "13"},
+   {"INT33BE:00_CsiBayer", "2"},
+   {"INT33BE:00_CamClk", "0"},
+   {"INT33F0:00_CsiPort", "0"},
+   {"INT33F0:00_CsiLanes", "1"},
+   {"INT33F0:00_CsiFmt", "13"},
+   {"INT33F0:00_CsiBayer", "0"},
+   {"INT33F0:00_CamClk", "1"},
+   {"gmin_V2P8GPIO", "402"},
+   {},
 };
 
 
 static const struct gmin_cfg_var i8880_vars[] = {
-{"XXOV2680:00_CsiPort", "1"},
-{"XXOV2680:00_CsiLanes","1"},
-{"XXOV2680:00_CamClk","0"},
-{"XXGC0310:00_CsiPort", "0"},
-{"XXGC0310:00_CsiLanes", "1"},
-{"XXGC0310:00_CamClk", "1"},
-{},
+   {"XXOV2680:00_CsiPort", "1"},
+   {"XXOV2680:00_CsiLanes", "1"},
+   {"XXOV2680:00_CamClk", "0"},
+   {"XXGC0310:00_CsiPort", "0"},
+   {"XXGC0310:00_CsiLanes", "1"},
+   {"XXGC0310:00_CamClk", "1"},
+   {},
 };
 
 static const struct {
@@ -317,9 +317,9 @@ static const struct {
 } hard_vars[] = {
{ "BYT-T FFD8", ffrd8_vars },
{ "T100TA", t100_vars },
-{ "MRD7", mrd7_vars },
-{ "ST70408", ecs7_vars },
-{ "VTA0803", i8880_vars },
+   { "MRD7", mrd7_vars },
+   { "ST70408", ecs7_vars },
+   { "VTA0803", i8880_vars },
 };
 
 
@@ -343,7 +343,7 @@ static struct gmin_subdev *gmin_subdev_add(struct 
v4l2_subdev *subdev)
 {
int i, ret;
struct device *dev;
-struct i2c_client *client = v4l2_get_subdevdata(subdev);
+   struct i2c_client *client = v4l2_get_subdevdata(subdev);
 
if (!pmic_id) {
 
@@ -361,8 +361,8 @@ static struct gmin_subdev *gmin_subdev_add(struct 
v4l2_subdev *subdev)
return NULL;
 
dev_info(dev,
-   "gmin: initializing atomisp module subdev 

Re: [PATCH v5] staging: rtl8723bs: remove re-positioned call to kfree in os_dep/ioctl_cfg80211.c

[Ian W MORRISON]

On 9 May 2017 at 00:42, Andy Shevchenko  wrote:
> On Sun, May 7, 2017 at 1:04 PM, Ian W MORRISON  wrote:



>
> What are you trying to achieve by posting this patch over and over so often?
>
> Version 5 look pretty good to me. Just stop posting it again and wait
> when Greg takes it.
>

I made a series of errors in the patch submission due to inexperience
in submitting my very first patch. The visibility of my minor mistakes
in certainly not what I would have liked and I can only apologize for
the annoyance caused. I believe I have finally corrected the patch to
meet the exacting standards required and having learnt from the
experience I hope never to repeat them.

> FWIW (version 5!):
> Reviewed-by: Andy Shevchenko 
>
> --
> With Best Regards,
> Andy Shevchenko

Thank you for your review,
Ian
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v5] staging: rtl8723bs: remove re-positioned call to kfree in os_dep/ioctl_cfg80211.c

[Andy Shevchenko]

On Sun, May 7, 2017 at 1:04 PM, Ian W MORRISON  wrote:
> A kernel cloned from
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git and
> built with the latest RTL8723BS driver included (CONFIG_RTL8723BS=m)
> fails when booting on an Intel Atom device with the RTL8723BS wifi
> chipset due to an error in
> drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c.
>
> The kernel when booted with Ubuntu 17.04 results in an unusable system
> however with the following patch booting is successful and the system
> is usable. The patch and kernel builds have been tested against on an
> Intel Compute Stick (STCK1A32WFC model).
>
> This version of the patch (version 5) has tabs rather than spaces, no
> wrap around, a changelog and a sign-off email.

What are you trying to achieve by posting this patch over and over so often?

Version 5 look pretty good to me. Just stop posting it again and wait
when Greg takes it.

FWIW (version 5!):
Reviewed-by: Andy Shevchenko 

-- 
With Best Regards,
Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH -next] staging/android/ion: remove useless document file

[Laura Abbott]

On 05/06/2017 02:49 AM, Yisheng Xie wrote:
> From: Yisheng Xie 
> 
> After commit 9828282e33a0 ("staging: android: ion: Remove old platform
> support"), the document about devicetree of ion is no need anymore, so
> just remove it.
> 

Acked-by: Laura Abbott 

> Signed-off-by: Yisheng Xie 
> ---
>  .../devicetree/bindings/staging/ion/hi6220-ion.txt | 31 -
>  MAINTAINERS|  1 -
>  drivers/staging/android/ion/devicetree.txt | 51 
> --
>  3 files changed, 83 deletions(-)
>  delete mode 100644 
> Documentation/devicetree/bindings/staging/ion/hi6220-ion.txt
>  delete mode 100644 drivers/staging/android/ion/devicetree.txt
> 
> diff --git a/Documentation/devicetree/bindings/staging/ion/hi6220-ion.txt 
> b/Documentation/devicetree/bindings/staging/ion/hi6220-ion.txt
> deleted file mode 100644
> index c59e27c..000
> --- a/Documentation/devicetree/bindings/staging/ion/hi6220-ion.txt
> +++ /dev/null
> @@ -1,31 +0,0 @@
> -Hi6220 SoC ION
> -===
> -Required properties:
> -- compatible : "hisilicon,hi6220-ion"
> -- list of the ION heaps
> - - heap name : maybe heap_sys_user@0
> - - heap id   : id should be unique in the system.
> - - heap base : base ddr address of the heap,0 means that
> - it is dynamic.
> - - heap size : memory size and 0 means it is dynamic.
> - - heap type : the heap type of the heap, please also
> - see the define in ion.h(drivers/staging/android/uapi/ion.h)
> 
> -Example:
> - hi6220-ion {
> - compatible = "hisilicon,hi6220-ion";
> - heap_sys_user@0 {
> - heap-name = "sys_user";
> - heap-id   = <0x0>;
> - heap-base = <0x0>;
> - heap-size = <0x0>;
> - heap-type = "ion_system";
> - };
> - heap_sys_contig@0 {
> - heap-name = "sys_contig";
> - heap-id   = <0x1>;
> - heap-base = <0x0>;
> - heap-size = <0x0>;
> - heap-type = "ion_system_contig";
> - };
> - };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 58590cf..9ec008c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -846,7 +846,6 @@ M:Laura Abbott 
>  M:   Sumit Semwal 
>  L:   de...@driverdev.osuosl.org
>  S:   Supported
> -F:   Documentation/devicetree/bindings/staging/ion/
>  F:   drivers/staging/android/ion
>  F:   drivers/staging/android/uapi/ion.h
>  F:   drivers/staging/android/uapi/ion_test.h
> diff --git a/drivers/staging/android/ion/devicetree.txt 
> b/drivers/staging/android/ion/devicetree.txt
> deleted file mode 100644
> index 16871527..000
> --- a/drivers/staging/android/ion/devicetree.txt
> +++ /dev/null
> @@ -1,51 +0,0 @@
> -Ion Memory Manager
> -
> -Ion is a memory manager that allows for sharing of buffers via dma-buf.
> -Ion allows for different types of allocation via an abstraction called
> -a 'heap'. A heap represents a specific type of memory. Each heap has
> -a different type. There can be multiple instances of the same heap
> -type.
> -
> -Specific heap instances are tied to heap IDs. Heap IDs are not to be 
> specified
> -in the devicetree.
> -
> -Required properties for Ion
> -
> -- compatible: "linux,ion" PLUS a compatible property for the device
> -
> -All child nodes of a linux,ion node are interpreted as heaps
> -
> -required properties for heaps
> -
> -- compatible: compatible string for a heap type PLUS a compatible property
> -for the specific instance of the heap. Current heap types
> --- linux,ion-heap-system
> --- linux,ion-heap-system-contig
> --- linux,ion-heap-carveout
> --- linux,ion-heap-chunk
> --- linux,ion-heap-dma
> --- linux,ion-heap-custom
> -
> -Optional properties
> -- memory-region: A phandle to a memory region. Required for DMA heap type
> -(see reserved-memory.txt for details on the reservation)
> -
> -Example:
> -
> - ion {
> - compatbile = "hisilicon,ion", "linux,ion";
> -
> - ion-system-heap {
> - compatbile = "hisilicon,system-heap", 
> "linux,ion-heap-system"
> - };
> -
> - ion-camera-region {
> - compatible = "hisilicon,camera-heap", 
> "linux,ion-heap-dma"
> - memory-region = <_region>;
> - };
> -
> - ion-fb-region {
> - compatbile = "hisilicon,fb-heap", "linux,ion-heap-dma"
> - memory-region = <_region>;
> - };
> - }
> 

___
devel mailing list
de...@linuxdriverproject.org

[PATCH v6] staging: rtl8723bs: remove re-positioned call to kfree in os_dep/ioctl_cfg80211.c

[Ian W MORRISON]

A re-positioned call to kfree() in 
drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
causes a segmentation error. This patch removed the kfree() call.

Fixes 6557ddfec348 ("staging: rtl8723bs: Fix various errors in 
os_dep/ioctl_cfg80211.c")
Signed-off-by: Ian W Morrison 
Reviewed-by: Hans de Goede 
---
This version of the patch (version 6) fixes errors in the patch submission.
 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c 
b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
index f17f4fb..2ee9df5 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -3527,7 +3527,6 @@ int rtw_wdev_alloc(struct adapter *padapter, struct 
device *dev)
pwdev_priv->power_mgmt = true;
else
pwdev_priv->power_mgmt = false;
-   kfree((u8 *)wdev);
 
return ret;
 
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] added pca9570 driver

[Dan Carpenter]

This should just go to the gpio people and not staging.
The address is linux-g...@vger.kernel.org  CC the Device Tree people for
answers the device tree questions: devicet...@vger.kernel.org
The subject should be:  [PATCH] gpio: pca9570: add pca9570 driver

The patch has a line wrapping problem so it doesn't apply properly.
Read Documentation/process/email-clients.rst

On Mon, May 08, 2017 at 12:31:52PM +0100, Chris Dew wrote:
> First, apologies, this is my first Linux kernel patch in at least 15 years.
> While I have spent a few hours reading various pieces of documentation about
> the Linux kernel development processes, I have probably missed some critical
> files entirely.
> 
> Also, there are probably a few coding issues in this patch.

There are some, yes.  Run scripts/checkpatch.pl --strict on your patch.

> 
> This driver has only been tested to work on our custom ARM board, where we
> give it the following device tree info:
> 
> arch/arm/boot/dts/imx6qdl-smarcfimx6.dtsi:
> 
> ...
>  {
> ...
> pca9570: pca9570@24 {
> compatible = "pca9570";
> reg = <0x24>;
> gpio-controller;
> };
> };
> 
> I have some questions:
> 
> - Is the "value", with which struct gpio_chip.set is called, guaranteed to
> be 0 or 1?
> 
> - Do I need to implement any form of locking around this driver, or is that
> done for me in layers above?
> 
> - The pca9571 is an 8-bit version of this chip.  Is leaving pca9571 support
> to a later patch the best idea?  (I do not have one to test, nor any way to
> test one, at the moment.)

My view is that you can't really hurt anything by adding broken support
for something that currently has no support.  But note the untested
parts in your commit message.

> 
> - This code was written and tested against a vendor-modified 4.1.15 kernel
> tree.  This patch is made against the stable 4.9.9.  To make it compile
> against the newer kernel I had to replace references to struct gpio_chip.dev
> with struct gpio_chip.parent, as that struct member had been renamed between
> kernel versions.
> 

This suggests that you didn't write the original?  Please give
authorship credit.  Use From: Some Name  as the
first line of the email.  Otherwise, just write everything against
linux-next because we don't care about the history of it.

> - Should this driver be in the staging directory?
> 

Nah.

>   - If so, how to do this, as staging does not seem to have either a
> staging/gpio nor a staging/include/linux/i2c directory to put the driver's
> files in.
> 
> Please let me know what else I need to read and do, to make this patch into
> something that stands a chance of being accepted upstream.
> 
> All the best,
> 
> Chris.
> 

You need a Signed-Off-By on the real patch.

I've never reviewed GPIO driver, and don't know much about ARM or device
tree so take my review with a grain of salt.

> 
> ---
>  drivers/gpio/Kconfig|   7 ++
>  drivers/gpio/Makefile   |   1 +
>  drivers/gpio/gpio-pca9570.c | 253
> 
>  include/linux/i2c/pca9570.h |  22 
>  4 files changed, 283 insertions(+)
>  create mode 100644 drivers/gpio/gpio-pca9570.c
>  create mode 100644 include/linux/i2c/pca9570.h
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 0504307..a068cdb 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -762,6 +762,13 @@ config GPIO_PCA953X
> 
> 40 bits:  pca9505, pca9698
> 
> +config GPIO_PCA9570
> + tristate "PCA9570 GPO expander"
> + depends on I2C
> + help
> +   Say yes here to provide access to the PCA9570 SMBus GPO expander,
> +   made by NXP.
> +
>  config GPIO_PCA953X_IRQ
>   bool "Interrupt controller support for PCA953x"
>   depends on GPIO_PCA953X=y
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index becb96c..ec7946f 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -90,6 +90,7 @@ obj-$(CONFIG_GPIO_MXS)  += gpio-mxs.o
>  obj-$(CONFIG_GPIO_OCTEON)+= gpio-octeon.o
>  obj-$(CONFIG_GPIO_OMAP)  += gpio-omap.o
>  obj-$(CONFIG_GPIO_PCA953X)   += gpio-pca953x.o
> +obj-$(CONFIG_GPIO_PCA9570)   += gpio-pca9570.o
>  obj-$(CONFIG_GPIO_PCF857X)   += gpio-pcf857x.o
>  obj-$(CONFIG_GPIO_PCH)   += gpio-pch.o
>  obj-$(CONFIG_GPIO_PCI_IDIO_16)   += gpio-pci-idio-16.o
> diff --git a/drivers/gpio/gpio-pca9570.c b/drivers/gpio/gpio-pca9570.c
> new file mode 100644
> index 000..b3dc0e3
> --- /dev/null
> +++ b/drivers/gpio/gpio-pca9570.c
> @@ -0,0 +1,253 @@
> +/*
> + * Driver for pca9570 I2C GPO expanders
> + * Note: 9570 is at 0x24 - these value must be set
> + * in device tree.

This can fit on the line before.

> + *
> + * Copyright (C) 2017 Chris Dew, Thorcom Systems Ltd.
> + *
> + * Derived from drivers/gpio/gpio-max732x.c
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it 

[PATCH v2] Staging: greybus: light: Prefer kcalloc over kzalloc

[karthik]

From: Karthik Tummala 

Fixed following checkpatch.pl warning:
 * WARNING: Prefer kcalloc over kzalloc with multiply

Instead of specifying no.of bytes * size as argument
in kzalloc, prefer kcalloc.

Signed-off-by: Karthik Tummala 
Reviewed-by: Rui Miguel Silva 
---
Changes for v2:
- Changed subject line & fixed typo as suggested by
  Rui Miguel Silva
---
 drivers/staging/greybus/light.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
index 1681362..861a249 100644
--- a/drivers/staging/greybus/light.c
+++ b/drivers/staging/greybus/light.c
@@ -1030,7 +1030,7 @@ static int gb_lights_light_config(struct gb_lights 
*glights, u8 id)
light->channels_count = conf.channel_count;
light->name = kstrndup(conf.name, NAMES_MAX, GFP_KERNEL);
 
-   light->channels = kzalloc(light->channels_count *
+   light->channels = kcalloc(light->channels_count,
  sizeof(struct gb_channel), GFP_KERNEL);
if (!light->channels)
return -ENOMEM;
@@ -1167,7 +1167,7 @@ static int gb_lights_create_all(struct gb_lights *glights)
if (ret < 0)
goto out;
 
-   glights->lights = kzalloc(glights->lights_count *
+   glights->lights = kcalloc(glights->lights_count,
  sizeof(struct gb_light), GFP_KERNEL);
if (!glights->lights) {
ret = -ENOMEM;
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] added pca9570 driver

[Greg KH]

On Mon, May 08, 2017 at 12:31:52PM +0100, Chris Dew wrote:
> First, apologies, this is my first Linux kernel patch in at least 15 years.
> While I have spent a few hours reading various pieces of documentation about
> the Linux kernel development processes, I have probably missed some critical
> files entirely.
> 
> Also, there are probably a few coding issues in this patch.
> 
> This driver has only been tested to work on our custom ARM board, where we
> give it the following device tree info:
> 
> arch/arm/boot/dts/imx6qdl-smarcfimx6.dtsi:
> 
> ...
>  {
> ...
> pca9570: pca9570@24 {
> compatible = "pca9570";
> reg = <0x24>;
> gpio-controller;
> };
> };
> 
> I have some questions:
> 
> - Is the "value", with which struct gpio_chip.set is called, guaranteed to
> be 0 or 1?
> 
> - Do I need to implement any form of locking around this driver, or is that
> done for me in layers above?
> 
> - The pca9571 is an 8-bit version of this chip.  Is leaving pca9571 support
> to a later patch the best idea?  (I do not have one to test, nor any way to
> test one, at the moment.)

These are all good questions to ask on the gpio mailing list.  Use
scripts/get_maintainer.pl to determine what people, and lists, to cc:
for a patch.  That will get you to the right people.

> - This code was written and tested against a vendor-modified 4.1.15 kernel
> tree.  This patch is made against the stable 4.9.9.  To make it compile
> against the newer kernel I had to replace references to struct gpio_chip.dev
> with struct gpio_chip.parent, as that struct member had been renamed between
> kernel versions.

4.9.9 is really old as well, you need to make a patch against Linus's
tree at the least, and linux-next is usually much better.  We can't go
back in time and add new drivers to old kernels.

> - Should this driver be in the staging directory?

Why would it belong there?  What needs to be done to it to get it out of
staging?  Hint, you need a TODO file for a staging driver listing that
type of thing...

Minor comments on your patch:


You need a proper changelog and signed-off-by: line, please read
Documentation/SubmittingPatches for all of the info you need.

> 
> 
> ---
>  drivers/gpio/Kconfig|   7 ++
>  drivers/gpio/Makefile   |   1 +
>  drivers/gpio/gpio-pca9570.c | 253
> 

Your patch is line-wapped :(

>  include/linux/i2c/pca9570.h |  22 
>  4 files changed, 283 insertions(+)
>  create mode 100644 drivers/gpio/gpio-pca9570.c
>  create mode 100644 include/linux/i2c/pca9570.h

Why do you need a .h file?

> +/*
> + * Driver for pca9570 I2C GPO expanders
> + * Note: 9570 is at 0x24 - these value must be set
> + * in device tree.
> + *
> + * Copyright (C) 2017 Chris Dew, Thorcom Systems Ltd.
> + *
> + * Derived from drivers/gpio/gpio-max732x.c
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.

Do you really mean "any later version"?

> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.

Always run scripts/checkpatch.pl on your patch, so you don't get grumpyu
maintainers telling you to run scripts/checkpatch.pl on your patch...

> +out_failed:
> + if (chip->client)
> + i2c_unregister_device(chip->client);
> +
> + return -1;

Don't make up random error values, use proper -E codes instead.


thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] added pca9570 driver

[Chris Dew]

First, apologies, this is my first Linux kernel patch in at least 15 
years.  While I have spent a few hours reading various pieces of 
documentation about the Linux kernel development processes, I have 
probably missed some critical files entirely.


Also, there are probably a few coding issues in this patch.

This driver has only been tested to work on our custom ARM board, where 
we give it the following device tree info:


arch/arm/boot/dts/imx6qdl-smarcfimx6.dtsi:

...
 {
...
pca9570: pca9570@24 {
compatible = "pca9570";
reg = <0x24>;
gpio-controller;
};
};

I have some questions:

- Is the "value", with which struct gpio_chip.set is called, guaranteed 
to be 0 or 1?


- Do I need to implement any form of locking around this driver, or is 
that done for me in layers above?


- The pca9571 is an 8-bit version of this chip.  Is leaving pca9571 
support to a later patch the best idea?  (I do not have one to test, nor 
any way to test one, at the moment.)


- This code was written and tested against a vendor-modified 4.1.15 
kernel tree.  This patch is made against the stable 4.9.9.  To make it 
compile against the newer kernel I had to replace references to struct 
gpio_chip.dev with struct gpio_chip.parent, as that struct member had 
been renamed between kernel versions.


- Should this driver be in the staging directory?

  - If so, how to do this, as staging does not seem to have either a 
staging/gpio nor a staging/include/linux/i2c directory to put the 
driver's files in.


Please let me know what else I need to read and do, to make this patch 
into something that stands a chance of being accepted upstream.


All the best,

Chris.


---
 drivers/gpio/Kconfig|   7 ++
 drivers/gpio/Makefile   |   1 +
 drivers/gpio/gpio-pca9570.c | 253 


 include/linux/i2c/pca9570.h |  22 
 4 files changed, 283 insertions(+)
 create mode 100644 drivers/gpio/gpio-pca9570.c
 create mode 100644 include/linux/i2c/pca9570.h

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 0504307..a068cdb 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -762,6 +762,13 @@ config GPIO_PCA953X

  40 bits:  pca9505, pca9698

+config GPIO_PCA9570
+   tristate "PCA9570 GPO expander"
+   depends on I2C
+   help
+ Say yes here to provide access to the PCA9570 SMBus GPO expander,
+ made by NXP.
+
 config GPIO_PCA953X_IRQ
bool "Interrupt controller support for PCA953x"
depends on GPIO_PCA953X=y
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index becb96c..ec7946f 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -90,6 +90,7 @@ obj-$(CONFIG_GPIO_MXS)+= gpio-mxs.o
 obj-$(CONFIG_GPIO_OCTEON)  += gpio-octeon.o
 obj-$(CONFIG_GPIO_OMAP)+= gpio-omap.o
 obj-$(CONFIG_GPIO_PCA953X) += gpio-pca953x.o
+obj-$(CONFIG_GPIO_PCA9570) += gpio-pca9570.o
 obj-$(CONFIG_GPIO_PCF857X) += gpio-pcf857x.o
 obj-$(CONFIG_GPIO_PCH) += gpio-pch.o
 obj-$(CONFIG_GPIO_PCI_IDIO_16) += gpio-pci-idio-16.o
diff --git a/drivers/gpio/gpio-pca9570.c b/drivers/gpio/gpio-pca9570.c
new file mode 100644
index 000..b3dc0e3
--- /dev/null
+++ b/drivers/gpio/gpio-pca9570.c
@@ -0,0 +1,253 @@
+/*
+ * Driver for pca9570 I2C GPO expanders
+ * Note: 9570 is at 0x24 - these value must be set
+ * in device tree.
+ *
+ * Copyright (C) 2017 Chris Dew, Thorcom Systems Ltd.
+ *
+ * Derived from drivers/gpio/gpio-max732x.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define PCA9570_GPIO_COUNT 4
+
+enum {
+   PCA9570,
+};
+
+static const struct i2c_device_id pca9570_id[] = {
+   { "pca9570", PCA9570 },
+   { },
+};
+MODULE_DEVICE_TABLE(i2c, pca9570_id);
+
+#ifdef CONFIG_OF
+static const struct of_device_id pca9570_of_table[] = {
+	{ .compatible = "nxp,pca9570" }, /* FIXME: I just put "nxp" in here as 
other drivers had a manufacturer name */

+   { }
+};
+MODULE_DEVICE_TABLE(of, pca9570_of_table);
+#endif
+
+
+struct pca9570_chip {
+   struct gpio_chip gpio_chip;
+   struct i2c_client *client;
+   struct mutex lock; 

Re: [PATCH] Staging: greybus: Prefer kcalloc over kzalloc

[Rui Miguel Silva]

Hi Karthik,
Thanks for the patch.

On Sat, May 06, 2017 at 07:44:10PM +0530, kart...@techveda.org wrote:

From: Karthik Tummala 

Fixed following checkpatch.pl warning:
* WARNING: Prefer kcalloc over kzalloc with multiply

Instead of specifying no.of bytes * size as arugment
in kzalloc, prefer kcalloc.

Signed-off-by: Karthik Tummala 


Can you send a v2 with a subject similar to the other commits in
the file. i.e, starting with staging: greybus: light: ...

And you have a typo in the change log s/arugment/argument/.

If this two are fixed, you can add my:
Reviewed-by: Rui Miguel Silva 

Cheers,
  Rui

---
drivers/staging/greybus/light.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
index 1681362..861a249 100644
--- a/drivers/staging/greybus/light.c
+++ b/drivers/staging/greybus/light.c
@@ -1030,7 +1030,7 @@ static int gb_lights_light_config(struct gb_lights 
*glights, u8 id)
light->channels_count = conf.channel_count;
light->name = kstrndup(conf.name, NAMES_MAX, GFP_KERNEL);

-   light->channels = kzalloc(light->channels_count *
+   light->channels = kcalloc(light->channels_count,
  sizeof(struct gb_channel), GFP_KERNEL);
if (!light->channels)
return -ENOMEM;
@@ -1167,7 +1167,7 @@ static int gb_lights_create_all(struct gb_lights *glights)
if (ret < 0)
goto out;

-   glights->lights = kzalloc(glights->lights_count *
+   glights->lights = kcalloc(glights->lights_count,
  sizeof(struct gb_light), GFP_KERNEL);
if (!glights->lights) {
ret = -ENOMEM;
--
1.9.1


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default

[Hans Verkuil]

On 05/08/2017 11:36 AM, Philipp Zabel wrote:
> Hi Hans,
> 
> On Mon, 2017-05-08 at 10:27 +0200, Hans Verkuil wrote:
>> Hi Philipp,
>>
>> Sorry for the very long delay, but I finally had some time to think about 
>> this.
> 
> Thank you for your thoughts.
> 
>> On 04/06/2017 03:55 PM, Philipp Zabel wrote:
>>> If the the field order is set to ANY in set_fmt, choose the currently
>>> set field order. If the colorspace is set to DEFAULT, choose the current
>>> colorspace.  If any of xfer_func, ycbcr_enc or quantization are set to
>>> DEFAULT, either choose the current setting, or the default setting for the
>>> new colorspace, if non-DEFAULT colorspace was given.
>>>
>>> This allows to let field order and colorimetry settings be propagated
>>> from upstream by calling media-ctl on the upstream entity source pad,
>>> and then call media-ctl on the sink pad to manually set the input frame
>>> interval, without changing the already set field order and colorimetry
>>> information.
>>>
>>> Signed-off-by: Philipp Zabel 
>>> ---
>>> This is based on imx-media-staging-md-v14, and it is supposed to allow
>>> configuring the pipeline with media-ctl like this:
>>>
>>> 1) media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY8_1X16/1920x1080]"
>>> 2) media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY8_1X16/1920x108]"
>>> 3) media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY8_1X16/1920x1080]"
>>> 4) media-ctl --set-v4l2 "'ipu1_csi0':0[fmt:UYVY8_1X16/1920x1080@1/60]"
>>> 5) media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/1920x1080@1/30]"
>>>
>>> Without having step 4) overwrite the colorspace and field order set on
>>> 'ipu1_csi0':0 by the propagation in step 3).
>>> ---
>>>  drivers/staging/media/imx/imx-media-csi.c | 34 
>>> +++
>>>  1 file changed, 34 insertions(+)
>>>
>>> diff --git a/drivers/staging/media/imx/imx-media-csi.c 
>>> b/drivers/staging/media/imx/imx-media-csi.c
>>> index 64dc454f6b371..d94ce1de2bf05 100644
>>> --- a/drivers/staging/media/imx/imx-media-csi.c
>>> +++ b/drivers/staging/media/imx/imx-media-csi.c
>>> @@ -1325,6 +1325,40 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
>>> csi_try_fmt(priv, sensor, cfg, sdformat, crop, compose, );
>>>  
>>> fmt = __csi_get_fmt(priv, cfg, sdformat->pad, sdformat->which);
>>> +
>>> +   /* Retain current field setting as default */
>>> +   if (sdformat->format.field == V4L2_FIELD_ANY)
>>> +   sdformat->format.field = fmt->field;
>>
>> This is OK.
> 
> Would this be a "may" or a "should"? As in,
> "If SUBDEV_S_FMT is called with field == ANY, the driver may/should set
> field to the previously configured interlacing field order".

To quote the FIELD_ANY documentation:

file:///home/hans/work/src/v4l/media-git/Documentation/output/html/uapi/v4l/field-order.html#field-order

"Drivers must never return V4L2_FIELD_ANY."

How a driver replaces FIELD_ANY is something that I am not sure should be 
explicitly
stated in the documentation. In many cases there is only one choice (usually 
FIELD_NONE).

In cases like this I think your proposed rule is a good recommendation. I would 
probably
phrase it like that.

> What would be the correct place to document this behaviour?
> Documentation/media/uapi/v4l/vidioc-subdev-g-fmt.rst?

Yes.

> 
>>> +   /* Retain current colorspace setting as default */
>>> +   if (sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT) {
>>> +   sdformat->format.colorspace = fmt->colorspace;
>>> +   if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT)
>>> +   sdformat->format.xfer_func = fmt->xfer_func;
>>> +   if (sdformat->format.ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT)
>>> +   sdformat->format.ycbcr_enc = fmt->ycbcr_enc;
>>> +   if (sdformat->format.quantization == V4L2_QUANTIZATION_DEFAULT)
>>> +   sdformat->format.quantization = fmt->quantization;
>>
>> If sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT, then you can just 
>> copy
>> all four fields from fmt to sdformat->format. The other three fields are 
>> meaningless
>> when colorspace == V4L2_COLORSPACE_DEFAULT.
> 
> Ok, good. Ignoring the transfer function / YCbCr encoding / quantization
> range fields when colorspace is DEFAULT would simplify this part to:
> 
>   if (sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT) {
>   sdformat->format.colorspace = fmt->colorspace;
>   sdformat->format.xfer_func = fmt->xfer_func;
>   sdformat->format.ycbcr_enc = fmt->ycbcr_enc;
>   sdformat->format.quantization = fmt->quantization;
>   }
> 
> Is that expectation already written down somewhere? If not, should we
> add it to Documentation/media/uapi/v4l/pixfmt-006.rst?

I don't think it is written down. It would be a good idea to make this
explicit.

> 
>>> +   } else {
>>> +   if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT) {
>>> +   sdformat->format.xfer_func =

Re: [PATCH 40/40] media: imx: set and propagate empty field, colorimetry params

[Philipp Zabel]

Hi Steve,

On Wed, 2017-04-12 at 17:45 -0700, Steve Longerbeam wrote:
> This patch adds a call to imx_media_fill_empty_mbus_fields() in the
> *_try_fmt() functions at the sink pads, to set empty field order and
> colorimetry parameters.
> 
> If the field order is set to ANY, choose the currently set field order
> at the sink pad. If the colorspace is set to DEFAULT, choose the
> current colorspace at the sink pad.  If any of xfer_func, ycbcr_enc
> or quantization are set to DEFAULT, either choose the current sink pad
> setting, or the default setting for the new colorspace, if non-DEFAULT
> colorspace was given.
> 
> Colorimetry is also propagated from sink to source pads anywhere
> this has not already been done. The exception is ic-prpencvf at the
> source pad, since the Image Converter outputs fixed quantization and
> Y`CbCr encoding.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/staging/media/imx/imx-ic-prp.c  |  5 ++-
>  drivers/staging/media/imx/imx-ic-prpencvf.c | 25 +++---
>  drivers/staging/media/imx/imx-media-csi.c   | 12 +--
>  drivers/staging/media/imx/imx-media-utils.c | 53 
> +
>  drivers/staging/media/imx/imx-media-vdic.c  |  7 ++--
>  drivers/staging/media/imx/imx-media.h   |  3 +-
>  6 files changed, 95 insertions(+), 10 deletions(-)
> 
[...]
> diff --git a/drivers/staging/media/imx/imx-media-utils.c 
> b/drivers/staging/media/imx/imx-media-utils.c
> index 7b2f92d..b07d0ae 100644
> --- a/drivers/staging/media/imx/imx-media-utils.c
> +++ b/drivers/staging/media/imx/imx-media-utils.c
> @@ -464,6 +464,59 @@ int imx_media_init_mbus_fmt(struct v4l2_mbus_framefmt 
> *mbus,
>  }
>  EXPORT_SYMBOL_GPL(imx_media_init_mbus_fmt);
>  
> +/*
> + * Check whether the field or colorimetry params in tryfmt are
> + * uninitialized, and if so fill them with the values from fmt.
> + * The exception is when tryfmt->colorspace has been initialized,
> + * if so all the further default colorimetry params can be derived
> + * from tryfmt->colorspace.
> + */
> +void imx_media_fill_empty_mbus_fields(struct v4l2_mbus_framefmt *tryfmt,
> +   struct v4l2_mbus_framefmt *fmt)
> +{
> + /* fill field if necessary */
> + if (tryfmt->field == V4L2_FIELD_ANY)
> + tryfmt->field = fmt->field;
> +
> + /* fill colorimetry if necessary */
> + if (tryfmt->colorspace == V4L2_COLORSPACE_DEFAULT) {
> + tryfmt->colorspace = fmt->colorspace;
> + if (tryfmt->xfer_func == V4L2_XFER_FUNC_DEFAULT)
> + tryfmt->xfer_func = fmt->xfer_func;
> + if (tryfmt->ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT)
> + tryfmt->ycbcr_enc = fmt->ycbcr_enc;
> + if (tryfmt->quantization == V4L2_QUANTIZATION_DEFAULT)
> + tryfmt->quantization = fmt->quantization;

According to Hans' latest comments, this could be changed to:

--8<--
>From cca3cda9effcaca0891eb8044a79137023fed1c2 Mon Sep 17 00:00:00 2001
From: Philipp Zabel 
Date: Mon, 8 May 2017 11:38:05 +0200
Subject: [PATCH] fixup! media: imx: set and propagate default field,
 colorimetry

Signed-off-by: Philipp Zabel 
---
 drivers/staging/media/imx/imx-media-utils.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-utils.c 
b/drivers/staging/media/imx/imx-media-utils.c
index a8766489e8a18..ec2abd618cc44 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -497,12 +497,9 @@ void imx_media_fill_default_mbus_fields(struct 
v4l2_mbus_framefmt *tryfmt,
/* fill colorimetry if necessary */
if (tryfmt->colorspace == V4L2_COLORSPACE_DEFAULT) {
tryfmt->colorspace = fmt->colorspace;
-   if (tryfmt->xfer_func == V4L2_XFER_FUNC_DEFAULT)
-   tryfmt->xfer_func = fmt->xfer_func;
-   if (tryfmt->ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT)
-   tryfmt->ycbcr_enc = fmt->ycbcr_enc;
-   if (tryfmt->quantization == V4L2_QUANTIZATION_DEFAULT)
-   tryfmt->quantization = fmt->quantization;
+   tryfmt->xfer_func = fmt->xfer_func;
+   tryfmt->ycbcr_enc = fmt->ycbcr_enc;
+   tryfmt->quantization = fmt->quantization;
} else {
if (tryfmt->xfer_func == V4L2_XFER_FUNC_DEFAULT) {
tryfmt->xfer_func =
-- 
2.11.0
-->8--

> + } else {
> + const struct imx_media_pixfmt *cc;
> + bool is_rgb = false;
> +
> + cc = imx_media_find_mbus_format(tryfmt->code,
> + CS_SEL_ANY, false);
> + if (!cc)
> + cc = imx_media_find_ipu_format(tryfmt->code,
> +

Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default

[Philipp Zabel]

Hi Hans,

On Mon, 2017-05-08 at 10:27 +0200, Hans Verkuil wrote:
> Hi Philipp,
> 
> Sorry for the very long delay, but I finally had some time to think about 
> this.

Thank you for your thoughts.

> On 04/06/2017 03:55 PM, Philipp Zabel wrote:
> > If the the field order is set to ANY in set_fmt, choose the currently
> > set field order. If the colorspace is set to DEFAULT, choose the current
> > colorspace.  If any of xfer_func, ycbcr_enc or quantization are set to
> > DEFAULT, either choose the current setting, or the default setting for the
> > new colorspace, if non-DEFAULT colorspace was given.
> > 
> > This allows to let field order and colorimetry settings be propagated
> > from upstream by calling media-ctl on the upstream entity source pad,
> > and then call media-ctl on the sink pad to manually set the input frame
> > interval, without changing the already set field order and colorimetry
> > information.
> > 
> > Signed-off-by: Philipp Zabel 
> > ---
> > This is based on imx-media-staging-md-v14, and it is supposed to allow
> > configuring the pipeline with media-ctl like this:
> > 
> > 1) media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY8_1X16/1920x1080]"
> > 2) media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY8_1X16/1920x108]"
> > 3) media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY8_1X16/1920x1080]"
> > 4) media-ctl --set-v4l2 "'ipu1_csi0':0[fmt:UYVY8_1X16/1920x1080@1/60]"
> > 5) media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/1920x1080@1/30]"
> > 
> > Without having step 4) overwrite the colorspace and field order set on
> > 'ipu1_csi0':0 by the propagation in step 3).
> > ---
> >  drivers/staging/media/imx/imx-media-csi.c | 34 
> > +++
> >  1 file changed, 34 insertions(+)
> > 
> > diff --git a/drivers/staging/media/imx/imx-media-csi.c 
> > b/drivers/staging/media/imx/imx-media-csi.c
> > index 64dc454f6b371..d94ce1de2bf05 100644
> > --- a/drivers/staging/media/imx/imx-media-csi.c
> > +++ b/drivers/staging/media/imx/imx-media-csi.c
> > @@ -1325,6 +1325,40 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
> > csi_try_fmt(priv, sensor, cfg, sdformat, crop, compose, );
> >  
> > fmt = __csi_get_fmt(priv, cfg, sdformat->pad, sdformat->which);
> > +
> > +   /* Retain current field setting as default */
> > +   if (sdformat->format.field == V4L2_FIELD_ANY)
> > +   sdformat->format.field = fmt->field;
> 
> This is OK.

Would this be a "may" or a "should"? As in,
"If SUBDEV_S_FMT is called with field == ANY, the driver may/should set
field to the previously configured interlacing field order".

What would be the correct place to document this behaviour?
Documentation/media/uapi/v4l/vidioc-subdev-g-fmt.rst?

> > +   /* Retain current colorspace setting as default */
> > +   if (sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT) {
> > +   sdformat->format.colorspace = fmt->colorspace;
> > +   if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT)
> > +   sdformat->format.xfer_func = fmt->xfer_func;
> > +   if (sdformat->format.ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT)
> > +   sdformat->format.ycbcr_enc = fmt->ycbcr_enc;
> > +   if (sdformat->format.quantization == V4L2_QUANTIZATION_DEFAULT)
> > +   sdformat->format.quantization = fmt->quantization;
> 
> If sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT, then you can just 
> copy
> all four fields from fmt to sdformat->format. The other three fields are 
> meaningless
> when colorspace == V4L2_COLORSPACE_DEFAULT.

Ok, good. Ignoring the transfer function / YCbCr encoding / quantization
range fields when colorspace is DEFAULT would simplify this part to:

if (sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT) {
sdformat->format.colorspace = fmt->colorspace;
sdformat->format.xfer_func = fmt->xfer_func;
sdformat->format.ycbcr_enc = fmt->ycbcr_enc;
sdformat->format.quantization = fmt->quantization;
}

Is that expectation already written down somewhere? If not, should we
add it to Documentation/media/uapi/v4l/pixfmt-006.rst?

> > +   } else {
> > +   if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT) {
> > +   sdformat->format.xfer_func =
> > +   V4L2_MAP_XFER_FUNC_DEFAULT(
> > +   sdformat->format.colorspace);
> > +   }
> > +   if (sdformat->format.ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT) {
> > +   sdformat->format.ycbcr_enc =
> > +   V4L2_MAP_YCBCR_ENC_DEFAULT(
> > +   sdformat->format.colorspace);
> > +   }
> > +   if (sdformat->format.quantization == V4L2_QUANTIZATION_DEFAULT) 
> > {
> > +   sdformat->format.quantization =
> > +   V4L2_MAP_QUANTIZATION_DEFAULT(
> > +   

[PATCH] staging : rtl8188eu : remove unnecessary else

[Surender Polsani]

according to coding style else is not generally
useful after a break or return

Signed-off-by: Surender Polsani 
---
 drivers/staging/rtl8188eu/hal/rtl8188e_dm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c 
b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
index d04b7fb..7420f55 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
@@ -212,8 +212,7 @@ u8 rtw_hal_antdiv_before_linked(struct adapter *Adapter)
 
rtw_antenna_select_cmd(Adapter, dm_swat_tbl->CurAntenna, false);
return true;
-   } else {
+   }
dm_swat_tbl->SWAS_NoLink_State = 0;
return false;
-   }
 }
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default

[Hans Verkuil]

Hi Philipp,

Sorry for the very long delay, but I finally had some time to think about this.

On 04/06/2017 03:55 PM, Philipp Zabel wrote:
> If the the field order is set to ANY in set_fmt, choose the currently
> set field order. If the colorspace is set to DEFAULT, choose the current
> colorspace.  If any of xfer_func, ycbcr_enc or quantization are set to
> DEFAULT, either choose the current setting, or the default setting for the
> new colorspace, if non-DEFAULT colorspace was given.
> 
> This allows to let field order and colorimetry settings be propagated
> from upstream by calling media-ctl on the upstream entity source pad,
> and then call media-ctl on the sink pad to manually set the input frame
> interval, without changing the already set field order and colorimetry
> information.
> 
> Signed-off-by: Philipp Zabel 
> ---
> This is based on imx-media-staging-md-v14, and it is supposed to allow
> configuring the pipeline with media-ctl like this:
> 
> 1) media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY8_1X16/1920x1080]"
> 2) media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY8_1X16/1920x108]"
> 3) media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY8_1X16/1920x1080]"
> 4) media-ctl --set-v4l2 "'ipu1_csi0':0[fmt:UYVY8_1X16/1920x1080@1/60]"
> 5) media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/1920x1080@1/30]"
> 
> Without having step 4) overwrite the colorspace and field order set on
> 'ipu1_csi0':0 by the propagation in step 3).
> ---
>  drivers/staging/media/imx/imx-media-csi.c | 34 
> +++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/drivers/staging/media/imx/imx-media-csi.c 
> b/drivers/staging/media/imx/imx-media-csi.c
> index 64dc454f6b371..d94ce1de2bf05 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -1325,6 +1325,40 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
>   csi_try_fmt(priv, sensor, cfg, sdformat, crop, compose, );
>  
>   fmt = __csi_get_fmt(priv, cfg, sdformat->pad, sdformat->which);
> +
> + /* Retain current field setting as default */
> + if (sdformat->format.field == V4L2_FIELD_ANY)
> + sdformat->format.field = fmt->field;

This is OK.

> +
> + /* Retain current colorspace setting as default */
> + if (sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT) {
> + sdformat->format.colorspace = fmt->colorspace;
> + if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT)
> + sdformat->format.xfer_func = fmt->xfer_func;
> + if (sdformat->format.ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT)
> + sdformat->format.ycbcr_enc = fmt->ycbcr_enc;
> + if (sdformat->format.quantization == V4L2_QUANTIZATION_DEFAULT)
> + sdformat->format.quantization = fmt->quantization;

If sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT, then you can just 
copy
all four fields from fmt to sdformat->format. The other three fields are 
meaningless
when colorspace == V4L2_COLORSPACE_DEFAULT.

> + } else {
> + if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT) {
> + sdformat->format.xfer_func =
> + V4L2_MAP_XFER_FUNC_DEFAULT(
> + sdformat->format.colorspace);
> + }
> + if (sdformat->format.ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT) {
> + sdformat->format.ycbcr_enc =
> + V4L2_MAP_YCBCR_ENC_DEFAULT(
> + sdformat->format.colorspace);
> + }
> + if (sdformat->format.quantization == V4L2_QUANTIZATION_DEFAULT) 
> {
> + sdformat->format.quantization =
> + V4L2_MAP_QUANTIZATION_DEFAULT(
> + cc->cs != IPUV3_COLORSPACE_YUV,
> + sdformat->format.colorspace,
> + sdformat->format.ycbcr_enc);
> + }

Is this needed for validation? Currently these fields play no role in the
default link validation. Which I think is actually the right thing to do,
unless the subdev can do actual colorspace conversion.

I would just drop the whole 'else' here.

Actually, wouldn't it be better to always just copy this information from
fmt? This subdev doesn't do any colorspace conversion, it just passes on
this information. I.e., you can't set it in any meaningful way.

Regards,

Hans

> + }
> +
>   *fmt = sdformat->format;
>  
>   if (sdformat->pad == CSI_SINK_PAD) {
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] staging: ccree: Fix initialization of anonymous unions

[Arnd Bergmann]

On Sun, May 7, 2017 at 9:53 PM, Geert Uytterhoeven  wrote:
> With gcc 4.1.2:
>
> drivers/staging/ccree/ssi_hash.c:1990: error: unknown field 
> ‘template_ahash’ specified in initializer
> drivers/staging/ccree/ssi_hash.c:1991: error: unknown field ‘init’ 
> specified in initializer
> drivers/staging/ccree/ssi_hash.c:1991: warning: missing braces around 
> initializer
> drivers/staging/ccree/ssi_hash.c:1991: warning: (near initialization for 
> ‘driver_hash[0]..template_ahash’)
> drivers/staging/ccree/ssi_hash.c:1992: error: unknown field ‘update’ 
> specified in initializer
> drivers/staging/ccree/ssi_hash.c:1992: warning: excess elements in union 
> initializer
> ...
>
> Add missing braces to fix this.
> After this it compiles without warnings with gcc 4.1.2 and 4.9.0.

Acked-by: Arnd Bergmann 

For reference, we need this up to gcc-4.5, the anonymous initializers were added
on gcc-4.6.

 Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [V4] staging : rtl8188eu : remove void function return

[Joe Perches]

On Mon, 2017-05-08 at 07:59 +0200, Greg KH wrote:
> On Mon, May 08, 2017 at 11:22:35AM +0530, Surender Polsani wrote:
> > kernel coding style doesn't allow the return statement
> > in void function.
> > 
> > Signed-off-by: Surender Polsani 
> > ---
> > Changes for v2:
> > corrected subject line as suggested
> > Changes for v3:
> > modified from line as suggested by Greg KH
> > placed a semicolon in label for fixing build error
> > Changes for v4:
> > changes made as suggested by Dan Carpenter

No, Dan correctly suggested that the goto
and the label be removed, not what has been
done in this patch.

> > diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c 
> > b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
[]
> > @@ -146,7 +146,6 @@ void rtw_hal_dm_watchdog(struct adapter *Adapter)
> >  
> > if (!hw_init_completed)
> > goto skip_dm;
> > -
> 
> Why did you remove this line?

I presume because of carelessness.

> > /* ODM */
> > pmlmepriv = >mlmepriv;
> >  
> > @@ -165,7 +164,7 @@ void rtw_hal_dm_watchdog(struct adapter *Adapter)
> >  skip_dm:
> > /*  Check GPIO to determine current RF on/off and Pbc status. */
> > /*  Check Hardware Radio ON/OFF or not */
> > -   return;
> > +   ;
> 
> Why is the ';' still here?

A label immediately before a function termination
requires a statement after the label and before the
last closing brace.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [V4] staging : rtl8188eu : remove void function return

[Greg KH]

On Mon, May 08, 2017 at 11:22:35AM +0530, Surender Polsani wrote:
> kernel coding style doesn't allow the return statement
> in void function.
> 
> Signed-off-by: Surender Polsani 
> ---
> Changes for v2:
> corrected subject line as suggested
> Changes for v3:
> modified from line as suggested by Greg KH
> placed a semicolon in label for fixing build error
> Changes for v4:
> changes made as suggested by Dan Carpenter
> ---
>  drivers/staging/rtl8188eu/hal/rtl8188e_dm.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c 
> b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
> index d04b7fb..bf9f10f 100644
> --- a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
> +++ b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
> @@ -146,7 +146,6 @@ void rtw_hal_dm_watchdog(struct adapter *Adapter)
>  
>   if (!hw_init_completed)
>   goto skip_dm;
> -

Why did you remove this line?

>   /* ODM */
>   pmlmepriv = >mlmepriv;
>  
> @@ -165,7 +164,7 @@ void rtw_hal_dm_watchdog(struct adapter *Adapter)
>  skip_dm:
>   /*  Check GPIO to determine current RF on/off and Pbc status. */
>   /*  Check Hardware Radio ON/OFF or not */
> - return;
> + ;

Why is the ';' still here?

You do understand the C language, right?

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel