Eric Anholt offically announces support of VC4 without access to expander on the RPI 3

[Michael Zoran]

On Mon, 2017-03-20 at 10:22 -0700, Eric Anholt wrote:
> Michael Zoran  writes:
> 
> > > > Since the API is completely documented, I see no reason we or
> > > > anybody
> > > > couldn't essentially rewrite the driver while it's in
> > > > staging.  I
> > > > just
> > > > think it would be best for everyone if the new version was a
> > > > drop
> > > > in
> > > > replacement for the original version.  Essential an enhancement
> > > > rather
> > > > then a competitor.
> > > 
> > > I think my comments weren't fundamental changes, but you surely
> > > mean
> > > the devicetree ABI? I like to see this driver ASAP out of staging
> > > and
> > > i'm not interested to maintain 2 functional identical driver only
> > > to
> > > keep compability with the Foundation tree. Currently i'm afraid
> > > that
> > > we build up many drivers in staging, which need a complete
> > > rewrite
> > > later if they should come out of staging. It would be nice if we
> > > could avoid the situation we have with the thermal driver.
> > > 
> > > Stefan
> > 
> > The API I'm talking about here is the mailbox API that is used to
> > talk
> > to the firmware.  The numbers and structures to pass are
> > documented. 
> > Nothing prevents anybody from rewriting this driver and submitting
> > it
> > to the appropriate subsystems.  It's certainly small enough.
> > 
> > If you really want working thermal or cpu speed drivers today,
> > nothing
> > stops anybody from submitting the downstream drivers after doing
> > some
> > minor touchups and submitting them to staging.  That would at least
> > get
> > things working while people argue about what the correct DT nodes
> > should be.
> > 
> > I would also like to point out that the RPI 3 has been out for over
> > a
> > year and nobody has been able to get working video out of it
> > through
> > VC4 on a mainline tree.  At least until now.  So I'm not sure the
> > best
> > way to go is for the expander driver to go under the GPIO subtree.
> 
> Excuse me?  Display works fine on my Pi3.  VC4 uses DDC to probe for
> connection when the GPIO line isn't present in the DT.

Just a FYI, Eric Anholt has offically announced support for VC4 for
HDMI on mainline Linus build without any support from the expander on
the RPI 3.  

Sounds like this particular driver isn't needed then, correct?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Steve Longerbeam]

On 03/20/2017 10:23 AM, Philipp Zabel wrote:

Hi Steve, Russell,

On Mon, 2017-03-20 at 14:17 +, Russell King - ARM Linux wrote:

On Mon, Mar 20, 2017 at 03:00:51PM +0100, Philipp Zabel wrote:

On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote:

The same document says:

  Scaling support is optional. When supported by a subdev, the crop
  rectangle on the subdev's sink pad is scaled to the size configured
  using the
  :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL
  using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the
  subdev supports scaling but not composing, the top and left values are
  not used and must always be set to zero.


Right, this sentence does imply that when scaling is supported, there
must be a sink compose rectangle, even when composing is not.

I have previously set up scaling like this:

media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

Does this mean, it should work like this instead?

media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
media-ctl --set-v4l2 
"'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

I suppose setting the source pad format should not be allowed to modify
the sink compose rectangle.


That is what I believe having read these documents several times, but
we need v4l2 people to confirm.


What do you think of this:

--8<--
From 2830aebc404bdfc9d7fc1ec94e5282d0b668e8f6 Mon Sep 17 00:00:00 2001
From: Philipp Zabel 
Date: Mon, 20 Mar 2017 17:10:21 +0100
Subject: [PATCH] media: imx: csi: add sink selection rectangles

Move the crop rectangle to the sink pad and add a sink compose rectangle
to configure scaling. Also propagate rectangles from sink pad to crop
rectangle, to compose rectangle, and to the source pads both in ACTIVE
and TRY variants of set_fmt/selection, and initialize the default crop
and compose rectangles.



I haven't had a chance to look at this patch in detail yet, but on
first glance it looks good to me. I'll try to find the time tomorrow
to incorporate it and then fixup Russell's subsequent patches for
the enum frame sizes/intervals.

Steve


Signed-off-by: Philipp Zabel 
---
 drivers/staging/media/imx/imx-media-csi.c | 216 +-
 1 file changed, 152 insertions(+), 64 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 560da3abdd70b..b026a5d602ddf 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -79,6 +79,7 @@ struct csi_priv {
const struct imx_media_pixfmt *cc[CSI_NUM_PADS];
struct v4l2_fract frame_interval;
struct v4l2_rect crop;
+   struct v4l2_rect compose;
const struct csi_skip_desc *skip[CSI_NUM_PADS - 1];

/* active vb2 buffers to send to video dev sink */
@@ -574,8 +575,8 @@ static int csi_setup(struct csi_priv *priv)
ipu_csi_set_window(priv->csi, >crop);

ipu_csi_set_downsize(priv->csi,
-priv->crop.width == 2 * outfmt->width,
-priv->crop.height == 2 * outfmt->height);
+priv->crop.width == 2 * priv->compose.width,
+priv->crop.height == 2 * priv->compose.height);

ipu_csi_init_interface(priv->csi, _mbus_cfg, _fmt);

@@ -1001,6 +1002,27 @@ __csi_get_fmt(struct csi_priv *priv, struct 
v4l2_subdev_pad_config *cfg,
return >format_mbus[pad];
 }

+static struct v4l2_rect *
+__csi_get_crop(struct csi_priv *priv, struct v4l2_subdev_pad_config *cfg,
+  enum v4l2_subdev_format_whence which)
+{
+   if (which == V4L2_SUBDEV_FORMAT_TRY)
+   return v4l2_subdev_get_try_crop(>sd, cfg, CSI_SINK_PAD);
+   else
+   return >crop;
+}
+
+static struct v4l2_rect *
+__csi_get_compose(struct csi_priv *priv, struct v4l2_subdev_pad_config *cfg,
+ enum v4l2_subdev_format_whence which)
+{
+   if (which == V4L2_SUBDEV_FORMAT_TRY)
+   return v4l2_subdev_get_try_compose(>sd, cfg,
+  CSI_SINK_PAD);
+   else
+   return >compose;
+}
+
 static void csi_try_crop(struct csi_priv *priv,
 struct v4l2_rect *crop,
 struct v4l2_subdev_pad_config *cfg,
@@ -1159,6 +1181,7 @@ static void csi_try_fmt(struct csi_priv *priv,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *sdformat,
struct v4l2_rect *crop,
+   struct v4l2_rect *compose,
const struct imx_media_pixfmt **cc)
 {
const struct imx_media_pixfmt *incc;
@@ -1173,15 +1196,8 @@ static void 

Re: [PATCH] staging: radio-bcm2048: Fix checkpatch warnings: WARNING: Prefer 'unsigned int' to bare use of 'unsigned'

[kbuild test robot]

Hi unknown,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.11-rc3 next-20170320]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/unknown/staging-radio-bcm2048-Fix-checkpatch-warnings-WARNING-Prefer-unsigned-int-to-bare-use-of-unsigned/20170321-105721
base:   git://linuxtv.org/media_tree.git master
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa 

All errors (new ones prefixed by >>):

   drivers/staging/media/bcm2048/radio-bcm2048.c: In function 
'bcm2048_power_state_write':
>> drivers/staging/media/bcm2048/radio-bcm2048.c:2031:50: error: two or more 
>> data types in declaration specifiers
DEFINE_SYSFS_PROPERTY(power_state, unsigned int, int, "%u", 0)
 ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:1951:2: note: in definition of 
macro 'property_write'
 type value;   \
 ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:2031:1: note: in expansion of 
macro 'DEFINE_SYSFS_PROPERTY'
DEFINE_SYSFS_PROPERTY(power_state, unsigned int, int, "%u", 0)
^
   drivers/staging/media/bcm2048/radio-bcm2048.c: In function 
'bcm2048_mute_write':
   drivers/staging/media/bcm2048/radio-bcm2048.c:2032:43: error: two or more 
data types in declaration specifiers
DEFINE_SYSFS_PROPERTY(mute, unsigned int, int, "%u", 0)
  ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:1951:2: note: in definition of 
macro 'property_write'
 type value;   \
 ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:2032:1: note: in expansion of 
macro 'DEFINE_SYSFS_PROPERTY'
DEFINE_SYSFS_PROPERTY(mute, unsigned int, int, "%u", 0)
^
   drivers/staging/media/bcm2048/radio-bcm2048.c: In function 
'bcm2048_audio_route_write':
   drivers/staging/media/bcm2048/radio-bcm2048.c:2033:50: error: two or more 
data types in declaration specifiers
DEFINE_SYSFS_PROPERTY(audio_route, unsigned int, int, "%u", 0)
 ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:1951:2: note: in definition of 
macro 'property_write'
 type value;   \
 ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:2033:1: note: in expansion of 
macro 'DEFINE_SYSFS_PROPERTY'
DEFINE_SYSFS_PROPERTY(audio_route, unsigned int, int, "%u", 0)
^
   drivers/staging/media/bcm2048/radio-bcm2048.c: In function 
'bcm2048_dac_output_write':
   drivers/staging/media/bcm2048/radio-bcm2048.c:2034:49: error: two or more 
data types in declaration specifiers
DEFINE_SYSFS_PROPERTY(dac_output, unsigned int, int, "%u", 0)
^
   drivers/staging/media/bcm2048/radio-bcm2048.c:1951:2: note: in definition of 
macro 'property_write'
 type value;   \
 ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:2034:1: note: in expansion of 
macro 'DEFINE_SYSFS_PROPERTY'
DEFINE_SYSFS_PROPERTY(dac_output, unsigned int, int, "%u", 0)
^
   drivers/staging/media/bcm2048/radio-bcm2048.c: In function 
'bcm2048_fm_hi_lo_injection_write':
   drivers/staging/media/bcm2048/radio-bcm2048.c:2036:57: error: two or more 
data types in declaration specifiers
DEFINE_SYSFS_PROPERTY(fm_hi_lo_injection, unsigned int, int, "%u", 0)
^
   drivers/staging/media/bcm2048/radio-bcm2048.c:1951:2: note: in definition of 
macro 'property_write'
 type value;   \
 ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:2036:1: note: in expansion of 
macro 'DEFINE_SYSFS_PROPERTY'
DEFINE_SYSFS_PROPERTY(fm_hi_lo_injection, unsigned int, int, "%u", 0)
^
   drivers/staging/media/bcm2048/radio-bcm2048.c: In function 
'bcm2048_fm_frequency_write':
   drivers/staging/media/bcm2048/radio-bcm2048.c:2037:51: error: two or more 
data types in declaration specifiers
DEFINE_SYSFS_PROPERTY(fm_frequency, unsigned int, int, "%u", 0)
  ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:1951:2: note: in definition of 
macro 'property_write'
 type value;   \
 ^
   drivers/staging/media/bcm2048/radio-bcm2048.c:2037:1: note: in expansion of 
macro 'DEFINE_SYSFS_PROPERTY'
DEFINE_SYSFS_PROPERTY(fm_frequency, unsigned int, int, "%u", 0)
^
   drivers/staging/media/bcm2048/radio-bcm2048.c: In function 
'bcm2048_fm_af_frequency_write':
   drivers/staging/media/bcm2048/radio-bcm2048

[PATCH 2/2] staging: vt6656: rf.c: spaces preferred around that '-'

[Matthew Giassa]

Resolving 2 checkpatch warnings generated due to:
CHECK: spaces preferred around that '-'

Signed-off-by: Matthew Giassa 
---
 drivers/staging/vt6656/rf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/vt6656/rf.c b/drivers/staging/vt6656/rf.c
index 0e3a62a..fe09627 100644
--- a/drivers/staging/vt6656/rf.c
+++ b/drivers/staging/vt6656/rf.c
@@ -611,7 +611,7 @@ int vnt_rf_write_embedded(struct vnt_private *priv, u32 
data)
reg_data[3] = (u8)(data >> 24);
 
vnt_control_out(priv, MESSAGE_TYPE_WRITE_IFRF,
-   0, 0, ARRAY_SIZE(reg_data), reg_data);
+   0, 0, ARRAY_SIZE(reg_data), reg_data);
 
return true;
 }
@@ -643,9 +643,9 @@ int vnt_rf_setpower(struct vnt_private *priv, u32 rate, u32 
channel)
case RATE_48M:
case RATE_54M:
if (channel > CB_MAX_CHANNEL_24G)
-   power = priv->ofdm_a_pwr_tbl[channel-15];
+   power = priv->ofdm_a_pwr_tbl[channel - 15];
else
-   power = priv->ofdm_pwr_tbl[channel-1];
+   power = priv->ofdm_pwr_tbl[channel - 1];
break;
}
 
-- 
2.7.4

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

[PATCH] staging: vt6656: rf.c: resolve all checkpatch issues.

[Matthew Giassa]

These patches address a few outstanding checkpatch warnings/errors in
rf.c, mainly due to space/indentation (style) issues.

---
 drivers/staging/vt6656/rf.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)
---

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

[PATCH 1/2] staging: vt6656: rf.c: alignment should match open parentheses

[Matthew Giassa]

Resolving 5 instances of checkpatch error/warning statements generated
due to:
ERROR: code indent should use tabs where possible
CHECK: Alignment should match open parenthesis

Signed-off-by: Matthew Giassa 
---
 drivers/staging/vt6656/rf.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/vt6656/rf.c b/drivers/staging/vt6656/rf.c
index 068c1c8..0e3a62a 100644
--- a/drivers/staging/vt6656/rf.c
+++ b/drivers/staging/vt6656/rf.c
@@ -876,7 +876,7 @@ void vnt_rf_table_download(struct vnt_private *priv)
memcpy(array, addr1, length1);
 
vnt_control_out(priv, MESSAGE_TYPE_WRITE, 0,
-   MESSAGE_REQUEST_RF_INIT, length1, array);
+   MESSAGE_REQUEST_RF_INIT, length1, array);
 
/* Channel Table 0 */
value = 0;
@@ -889,7 +889,7 @@ void vnt_rf_table_download(struct vnt_private *priv)
memcpy(array, addr2, length);
 
vnt_control_out(priv, MESSAGE_TYPE_WRITE,
-   value, MESSAGE_REQUEST_RF_CH0, length, array);
+   value, MESSAGE_REQUEST_RF_CH0, length, array);
 
length2 -= length;
value += length;
@@ -907,7 +907,7 @@ void vnt_rf_table_download(struct vnt_private *priv)
memcpy(array, addr3, length);
 
vnt_control_out(priv, MESSAGE_TYPE_WRITE,
-   value, MESSAGE_REQUEST_RF_CH1, length, array);
+   value, MESSAGE_REQUEST_RF_CH1, length, array);
 
length3 -= length;
value += length;
@@ -924,7 +924,7 @@ void vnt_rf_table_download(struct vnt_private *priv)
 
/* Init Table 2 */
vnt_control_out(priv, MESSAGE_TYPE_WRITE,
-   0, MESSAGE_REQUEST_RF_INIT2, length1, array);
+   0, MESSAGE_REQUEST_RF_INIT2, length1, array);
 
/* Channel Table 0 */
value = 0;
@@ -937,7 +937,8 @@ void vnt_rf_table_download(struct vnt_private *priv)
memcpy(array, addr2, length);
 
vnt_control_out(priv, MESSAGE_TYPE_WRITE,
-   value, MESSAGE_REQUEST_RF_CH2, length, 
array);
+   value, MESSAGE_REQUEST_RF_CH2, length,
+   array);
 
length2 -= length;
value += length;
-- 
2.7.4

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

[PATCH v2 4/6] staging: media: atomisp: fix build errors when PM is disabled

[Jérémy Lefaure]

The function atomisp_restore_iumit_reg is unused when PM is disabled.
Adding __maybe_unused to the function definition avoids a warning.

The function atomisp_mrfld_power_down is defined only when PM is
enabled. So in atomisp_pci_probe, it should be called only when PM is
enabled.

Signed-off-by: Jérémy Lefaure 
---
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
index 20b409789de2..626d2f114d8d 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
@@ -264,7 +264,7 @@ static int atomisp_save_iunit_reg(struct atomisp_device 
*isp)
return 0;
 }
 
-static int atomisp_restore_iunit_reg(struct atomisp_device *isp)
+static int __maybe_unused atomisp_restore_iunit_reg(struct atomisp_device *isp)
 {
struct pci_dev *dev = isp->pdev;
 
@@ -1526,7 +1526,7 @@ static int atomisp_pci_probe(struct pci_dev *dev,
atomisp_ospm_dphy_down(isp);
 
/* Address later when we worry about the ...field chips */
-   if (atomisp_mrfld_power_down(isp))
+   if (IS_ENABLED(CONFIG_PM) && atomisp_mrfld_power_down(isp))
dev_err(>dev, "Failed to switch off ISP\n");
pci_dev_put(isp->pci_root);
return err;
-- 
2.12.0

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

[PATCH 10/10] staging: ks7010: rename return value identifier

[Tobin C. Harding]

Driver uses multiple identifier names for the same task (retval, ret,
rc). It would be easier to read the code if a single task is
identified with a single name. 'ret' is the most common return value
identifier name found in the kernel tree, following the principle of
least surprise using 'ret' is a decent choice.

Rename rc -> ret
Rename retval -> ret

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks7010_sdio.c | 142 +--
 drivers/staging/ks7010/ks_hostif.c   |  16 ++--
 drivers/staging/ks7010/ks_wlan_net.c |  10 +--
 3 files changed, 84 insertions(+), 84 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
b/drivers/staging/ks7010/ks7010_sdio.c
index 8823e93..28b91be 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -52,44 +52,48 @@ static int ks7010_sdio_read(struct ks_wlan_private *priv, 
unsigned int address,
unsigned char *buffer, int length)
 {
struct ks_sdio_card *card;
-   int rc;
+   int ret;
 
card = priv->ks_wlan_hw.sdio_card;
 
if (length == 1)/* CMD52 */
-   *buffer = sdio_readb(card->func, address, );
+   *buffer = sdio_readb(card->func, address, );
else/* CMD53 multi-block transfer */
-   rc = sdio_memcpy_fromio(card->func, buffer, address, length);
+   ret = sdio_memcpy_fromio(card->func, buffer, address, length);
 
-   if (rc)
-   DPRINTK(1, "sdio error=%d size=%d\n", rc, length);
+   if (ret) {
+   DPRINTK(1, "sdio error=%d size=%d\n", ret, length);
+   return ret;
+   }
 
-   return rc;
+   return 0;
 }
 
 static int ks7010_sdio_write(struct ks_wlan_private *priv, unsigned int 
address,
 unsigned char *buffer, int length)
 {
struct ks_sdio_card *card;
-   int rc;
+   int ret;
 
card = priv->ks_wlan_hw.sdio_card;
 
if (length == 1)/* CMD52 */
-   sdio_writeb(card->func, *buffer, address, );
+   sdio_writeb(card->func, *buffer, address, );
else/* CMD53 */
-   rc = sdio_memcpy_toio(card->func, address, buffer, length);
+   ret = sdio_memcpy_toio(card->func, address, buffer, length);
 
-   if (rc)
-   DPRINTK(1, "sdio error=%d size=%d\n", rc, length);
+   if (ret) {
+   DPRINTK(1, "sdio error=%d size=%d\n", ret, length);
+   return ret;
+   }
 
-   return rc;
+   return 0;
 }
 
 static void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv)
 {
unsigned char rw_data;
-   int retval;
+   int ret;
 
DPRINTK(4, "\n");
 
@@ -98,9 +102,8 @@ static void ks_wlan_hw_sleep_doze_request(struct 
ks_wlan_private *priv)
 
if (atomic_read(>sleepstatus.status) == 0) {
rw_data = GCR_B_DOZE;
-   retval =
-   ks7010_sdio_write(priv, GCR_B, _data, sizeof(rw_data));
-   if (retval) {
+   ret = ks7010_sdio_write(priv, GCR_B, _data, sizeof(rw_data));
+   if (ret) {
DPRINTK(1, " error : GCR_B=%02X\n", rw_data);
goto set_sleep_mode;
}
@@ -119,7 +122,7 @@ static void ks_wlan_hw_sleep_doze_request(struct 
ks_wlan_private *priv)
 static void ks_wlan_hw_sleep_wakeup_request(struct ks_wlan_private *priv)
 {
unsigned char rw_data;
-   int retval;
+   int ret;
 
DPRINTK(4, "\n");
 
@@ -128,9 +131,8 @@ static void ks_wlan_hw_sleep_wakeup_request(struct 
ks_wlan_private *priv)
 
if (atomic_read(>sleepstatus.status) == 1) {
rw_data = WAKEUP_REQ;
-   retval =
-   ks7010_sdio_write(priv, WAKEUP, _data, sizeof(rw_data));
-   if (retval) {
+   ret = ks7010_sdio_write(priv, WAKEUP, _data, 
sizeof(rw_data));
+   if (ret) {
DPRINTK(1, " error : WAKEUP=%02X\n", rw_data);
goto set_sleep_mode;
}
@@ -149,14 +151,13 @@ static void ks_wlan_hw_sleep_wakeup_request(struct 
ks_wlan_private *priv)
 void ks_wlan_hw_wakeup_request(struct ks_wlan_private *priv)
 {
unsigned char rw_data;
-   int retval;
+   int ret;
 
DPRINTK(4, "\n");
if (atomic_read(>psstatus.status) == PS_SNOOZE) {
rw_data = WAKEUP_REQ;
-   retval =
-   ks7010_sdio_write(priv, WAKEUP, _data, sizeof(rw_data));
-   if (retval)
+   ret = ks7010_sdio_write(priv, WAKEUP, _data, 
sizeof(rw_data));
+   if (ret)
DPRINTK(1, " error : WAKEUP=%02X\n", rw_data);
 
DPRINTK(4, "wake up : WAKEUP=%02X\n", rw_data);
@@ -241,17 +242,17 @@ static int enqueue_txdev(struct ks_wlan_private *priv, 
unsigned char *p,
 

[PATCH 07/10] staging: ks7010: make goto labels uniform

[Tobin C. Harding]

Driver uses different label forms for similar purposes. It would be
more clear if single use case has uniform label. 'out' is generic and
adds no meaning to label.

Documentation/process/coding-style.rst:
Choose label names which say what the goto does or why the goto
exists.

Rename labels so as to better describe what they do. If an execution
path only exists for the label on an error, prefix the label with
'err_'. If a non-error execution path exist do not use prefix.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks7010_sdio.c | 70 ++--
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
b/drivers/staging/ks7010/ks7010_sdio.c
index 3dab700..40ec028 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -102,7 +102,7 @@ static void ks_wlan_hw_sleep_doze_request(struct 
ks_wlan_private *priv)
ks7010_sdio_write(priv, GCR_B, _data, sizeof(rw_data));
if (retval) {
DPRINTK(1, " error : GCR_B=%02X\n", rw_data);
-   goto out;
+   goto set_sleep_mode;
}
DPRINTK(4, "PMG SET!! : GCR_B=%02X\n", rw_data);
DPRINTK(3, "sleep_mode=SLP_SLEEP\n");
@@ -112,7 +112,7 @@ static void ks_wlan_hw_sleep_doze_request(struct 
ks_wlan_private *priv)
DPRINTK(1, "sleep_mode=%d\n", priv->sleep_mode);
}
 
- out:
+set_sleep_mode:
priv->sleep_mode = atomic_read(>sleepstatus.status);
 }
 
@@ -132,7 +132,7 @@ static void ks_wlan_hw_sleep_wakeup_request(struct 
ks_wlan_private *priv)
ks7010_sdio_write(priv, WAKEUP, _data, sizeof(rw_data));
if (retval) {
DPRINTK(1, " error : WAKEUP=%02X\n", rw_data);
-   goto out;
+   goto set_sleep_mode;
}
DPRINTK(4, "wake up : WAKEUP=%02X\n", rw_data);
atomic_set(>sleepstatus.status, 0);
@@ -142,7 +142,7 @@ static void ks_wlan_hw_sleep_wakeup_request(struct 
ks_wlan_private *priv)
DPRINTK(1, "sleep_mode=%d\n", priv->sleep_mode);
}
 
- out:
+set_sleep_mode:
priv->sleep_mode = atomic_read(>sleepstatus.status);
 }
 
@@ -495,18 +495,18 @@ static void ks7010_rw_function(struct work_struct *work)
queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
   >ks_wlan_hw.rw_wq, 1);
}
-   goto err_out;
+   goto err_release_host;
}
 
/* sleep mode doze */
if (atomic_read(>sleepstatus.doze_request) == 1) {
ks_wlan_hw_sleep_doze_request(priv);
-   goto err_out;
+   goto err_release_host;
}
/* sleep mode wakeup */
if (atomic_read(>sleepstatus.wakeup_request) == 1) {
ks_wlan_hw_sleep_wakeup_request(priv);
-   goto err_out;
+   goto err_release_host;
}
 
/* read (WriteStatus/ReadDataSize FN1:00_0014) */
@@ -515,7 +515,7 @@ static void ks7010_rw_function(struct work_struct *work)
if (retval) {
DPRINTK(1, " error : WSTATUS_RSIZE=%02X psstatus=%d\n", rw_data,
atomic_read(>psstatus.status));
-   goto err_out;
+   goto err_release_host;
}
DPRINTK(4, "WSTATUS_RSIZE=%02X\n", rw_data);
 
@@ -528,7 +528,7 @@ static void ks7010_rw_function(struct work_struct *work)
 
_ks_wlan_hw_power_save(priv);
 
- err_out:
+err_release_host:
sdio_release_host(priv->ks_wlan_hw.sdio_card->func);
 }
 
@@ -549,7 +549,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
 sizeof(status));
if (retval) {
DPRINTK(1, "read INT_PENDING Failed!!(%d)\n", retval);
-   goto intr_out;
+   goto queue_delayed_work;
}
DPRINTK(4, "INT_PENDING=%02X\n", rw_data);
 
@@ -565,7 +565,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
 sizeof(rw_data));
if (retval) {
DPRINTK(1, " error : GCR_B=%02X\n", rw_data);
-   goto intr_out;
+   goto queue_delayed_work;
}
/* DPRINTK(1, "GCR_B=%02X\n", rw_data); */
if (rw_data == GCR_B_ACTIVE) {
@@ -587,7 +587,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
if (retval) {
DPRINTK(1, " error : WSTATUS_RSIZE=%02X\n",
rw_data);
-   goto intr_out;
+   

[PATCH 06/10] staging: ks7010: return directly on error

[Tobin C. Harding]

Function uses goto label with no clean up code. In this case we
should just return directly.

Remove goto statement, return directly on error.

Signed-off-by: Tobin C. Harding 
---

'/* length check fail */' comment added because preceding line is a
standard function return value check followed by single (un-braced)
statement. Following this with a return makes it look like there is a
bug (missing braces). Adding the comment will save the next (and
subsequent) developers from having to think through this.

 drivers/staging/ks7010/ks7010_sdio.c | 10 --
 drivers/staging/ks7010/ks_wlan_net.c |  7 +++
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
b/drivers/staging/ks7010/ks7010_sdio.c
index 1d3a15f..3dab700 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -400,7 +400,7 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size)
if (cnt_rxqbody(priv) >= (RX_DEVICE_BUFF_SIZE - 1)) {
/* in case of buffer overflow */
DPRINTK(1, "rx buffer overflow\n");
-   goto error_out;
+   return;
}
rx_buffer = >rx_dev.rx_dev_buff[priv->rx_dev.qtail];
 
@@ -408,7 +408,7 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size)
ks7010_sdio_read(priv, DATA_WINDOW, _buffer->data[0],
 hif_align_size(size));
if (retval)
-   goto error_out;
+   return;
 
/* length check */
if (size > 2046 || size == 0) {
@@ -426,7 +426,8 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size)
if (retval)
DPRINTK(1, " error : READ_STATUS=%02X\n", read_status);
 
-   goto error_out;
+   /* length check fail */
+   return;
}
 
hdr = (struct hostif_hdr *)_buffer->data[0];
@@ -453,9 +454,6 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size)
 
/* rx_event_task((void *)priv); */
tasklet_schedule(>ks_wlan_hw.rx_bh_task);
-
- error_out:
-   return;
 }
 
 static void ks7010_rw_function(struct work_struct *work)
diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
b/drivers/staging/ks7010/ks_wlan_net.c
index f18ff56..d96eed6 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -202,7 +202,6 @@ static int ks_wlan_set_freq(struct net_device *dev,
 {
struct ks_wlan_private *priv =
(struct ks_wlan_private *)netdev_priv(dev);
-   int rc = -EINPROGRESS;  /* Call commit handler */
 
if (priv->sleep_mode == SLP_SLEEP)
return -EPERM;
@@ -222,7 +221,7 @@ static int ks_wlan_set_freq(struct net_device *dev,
}
/* Setting by channel number */
if ((fwrq->m > 1000) || (fwrq->e > 0)) {
-   rc = -EOPNOTSUPP;
+   return -EOPNOTSUPP;
} else {
int channel = fwrq->m;
/* We should do a better check than that,
@@ -232,7 +231,7 @@ static int ks_wlan_set_freq(struct net_device *dev,
netdev_dbg(dev,
   "%s: New channel value of %d is invalid!\n",
   dev->name, fwrq->m);
-   rc = -EINVAL;
+   return -EINVAL;
} else {
/* Yes ! We can set it !!! */
priv->reg.channel = (u8)(channel);
@@ -240,7 +239,7 @@ static int ks_wlan_set_freq(struct net_device *dev,
}
}
 
-   return rc;
+   return -EINPROGRESS;/* Call commit handler */
 }
 
 static int ks_wlan_get_freq(struct net_device *dev,
-- 
2.7.4

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

[PATCH 08/10] staging: ks7010: remove non-zero comparison

[Tobin C. Harding]

Comparison, does not equal zero, is redundant

'if (foo != 0)'  is equal to  'if (foo)'

Typical kernel coding style is to use the shorter form.

Remove unnecessary non-zero comparison.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks7010_sdio.c | 4 ++--
 drivers/staging/ks7010/ks_hostif.c   | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
b/drivers/staging/ks7010/ks7010_sdio.c
index 40ec028..8823e93 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -61,7 +61,7 @@ static int ks7010_sdio_read(struct ks_wlan_private *priv, 
unsigned int address,
else/* CMD53 multi-block transfer */
rc = sdio_memcpy_fromio(card->func, buffer, address, length);
 
-   if (rc != 0)
+   if (rc)
DPRINTK(1, "sdio error=%d size=%d\n", rc, length);
 
return rc;
@@ -80,7 +80,7 @@ static int ks7010_sdio_write(struct ks_wlan_private *priv, 
unsigned int address,
else/* CMD53 */
rc = sdio_memcpy_toio(card->func, address, buffer, length);
 
-   if (rc != 0)
+   if (rc)
DPRINTK(1, "sdio error=%d size=%d\n", rc, length);
 
return rc;
diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index 3eb0f2e..83cda1f 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -528,7 +528,7 @@ void hostif_mib_get_confirm(struct ks_wlan_private *priv)
mib_val_size = get_WORD(priv);  /* MIB value size */
mib_val_type = get_WORD(priv);  /* MIB value type */
 
-   if (mib_status != 0) {
+   if (mib_status) {
/* in case of error */
DPRINTK(1, "attribute=%08X, status=%08X\n", mib_attribute,
mib_status);
@@ -604,7 +604,7 @@ void hostif_mib_set_confirm(struct ks_wlan_private *priv)
mib_status = get_DWORD(priv);   /* MIB Status */
mib_attribute = get_DWORD(priv);/* MIB attribute */
 
-   if (mib_status != 0) {
+   if (mib_status) {
/* in case of error */
DPRINTK(1, "error :: attribute=%08X, status=%08X\n",
mib_attribute, mib_status);
@@ -834,7 +834,7 @@ void hostif_scan_indication(struct ks_wlan_private *priv)
DPRINTK(3, "scan_ind_count = %d\n", priv->scan_ind_count);
ap_info = (struct ap_info_t *)(priv->rxp);
 
-   if (priv->scan_ind_count != 0) {
+   if (priv->scan_ind_count) {
for (i = 0; i < priv->aplist.size; i++) {   /* bssid check 
*/
if (!memcmp
(ap_info->bssid,
-- 
2.7.4

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

[PATCH 09/10] staging: ks7010: remove zero comparison

[Tobin C. Harding]

Comparison, equal to zero, is redundant

'if (foo == 0)'  is equal to  'if (!foo)'

Typical kernel coding style is to use the shorter form.

Remove unnecessary zero comparison.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_wlan_net.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
b/drivers/staging/ks7010/ks_wlan_net.c
index d96eed6..7d4c7f3 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -279,7 +279,7 @@ static int ks_wlan_set_essid(struct net_device *dev,
 
/* for SLEEP MODE */
/* Check if we asked for `any' */
-   if (dwrq->flags == 0) {
+   if (!dwrq->flags) {
/* Just send an empty SSID list */
memset(priv->reg.ssid.body, 0, sizeof(priv->reg.ssid.body));
priv->reg.ssid.size = 0;
@@ -1531,7 +1531,7 @@ static int ks_wlan_get_scan(struct net_device *dev,
return -EAGAIN;
}
 
-   if (priv->aplist.size == 0) {
+   if (!priv->aplist.size) {
/* Client error, no scan results...
 * The caller need to restart the scan.
 */
-- 
2.7.4

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

[PATCH 04/10] staging: ks7010: fix checkpatch BRACES

[Tobin C. Harding]

Checkpatch emits CHECK: Unbalanced braces around else
statement. Statements in question are single statements so we do not
need braces. Checkpatch also warns about multiple line dereference for
this code.

Fix if/else/else if statement use of braces. Fix function argument layout
at the same time since it is the same statement.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_hostif.c | 22 +-
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index db10e16..68e26f4 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -2456,19 +2456,15 @@ void hostif_sme_execute(struct ks_wlan_private *priv, 
int event)
hostif_phy_information_request(priv);
break;
case SME_MIC_FAILURE_REQUEST:
-   if (priv->wpa.mic_failure.failure == 1) {
-   hostif_mic_failure_request(priv,
-  priv->wpa.mic_failure.
-  failure - 1, 0);
-   } else if (priv->wpa.mic_failure.failure == 2) {
-   hostif_mic_failure_request(priv,
-  priv->wpa.mic_failure.
-  failure - 1,
-  priv->wpa.mic_failure.
-  counter);
-   } else
-   DPRINTK(4,
-   "SME_MIC_FAILURE_REQUEST: failure count=%u 
error?\n",
+   if (priv->wpa.mic_failure.failure == 1)
+   hostif_mic_failure_request(
+   priv, priv->wpa.mic_failure.failure - 1, 0);
+   else if (priv->wpa.mic_failure.failure == 2)
+   hostif_mic_failure_request(
+   priv, priv->wpa.mic_failure.failure - 1,
+   priv->wpa.mic_failure.counter);
+   else
+   DPRINTK(4, "SME_MIC_FAILURE_REQUEST: failure count=%u 
error?\n",
priv->wpa.mic_failure.failure);
break;
case SME_MIC_FAILURE_CONFIRM:
-- 
2.7.4

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

[PATCH 00/10] staging: ks7010: audit return statements

[Tobin C. Harding]

Driver return statement code exhibits a degree of uncleanliness.

Driver return statement code is non-uniform in its use of
identifiers, both variables and goto labels.

goto statements occasionally are used with out clean up code instead
of returning directly.

Variables are defined (at declaration time) to zero unnecessarily.

Code could be cleaned up by an audit of all return statement code.

Initial patches do general checkpatch cleanups.

Patch 01, patch 02, and patch 03 are white space fixes, separated by
checkpatch type.

Patch 04 fixes unbalanced braces.

Patch 05 removes multiple line assignments.

Patch 06 removes usage of goto statements with out clean up code.

Patch 07 renames goto labels to be more informative and uniform.

Patch 08 removes not-zero comparison i.e if (foo != 0).

Patch 09 removes comparison to zero i.e if (foo == 0) if it does not
add information to the statement.

Patch 10 renames identifiers 'rc' and 'retval' to 'ret' in order to be
uniform across the driver.

Tobin C. Harding (10):
  staging: ks7010: fix checkpatch LINE_SPACING
  staging: ks7010: fix checkpatch SPACING
  staging: ks7010: fix checkpatch PARENTHESIS_ALIGNMENT
  staging: ks7010: fix checkpatch BRACES
  staging: ks7010: fix checkpatch MULTIPLE_ASSIGNMENTS
  staging: ks7010: return directly on error
  staging: ks7010: make goto labels uniform
  staging: ks7010: remove non-zero comparison
  staging: ks7010: remove zero comparison
  staging: ks7010: rename return value identifier

 drivers/staging/ks7010/ks7010_sdio.c | 226 +--
 drivers/staging/ks7010/ks_hostif.c   |  47 
 drivers/staging/ks7010/ks_wlan_net.c |  38 +++---
 drivers/staging/ks7010/michael_mic.c |   4 +-
 4 files changed, 155 insertions(+), 160 deletions(-)

-- 
2.7.4

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

[PATCH 03/10] staging: ks7010: fix checkpatch PARENTHESIS_ALIGNMENT

[Tobin C. Harding]

Checkpatch emits CHECK: Alignment should match open parenthesis.

Fix alignment to open parenthesis in line with kernel coding style.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks7010_sdio.c |  4 ++--
 drivers/staging/ks7010/ks_wlan_net.c | 16 
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
b/drivers/staging/ks7010/ks7010_sdio.c
index 0fa13cd..1d3a15f 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -193,8 +193,8 @@ static int _ks_wlan_hw_power_save(struct ks_wlan_private 
*priv)
cnt_txqbody(priv));
 
if (!atomic_read(>psstatus.confirm_wait) &&
-   !atomic_read(>psstatus.snooze_guard) &&
-   !cnt_txqbody(priv)) {
+   !atomic_read(>psstatus.snooze_guard) &&
+   !cnt_txqbody(priv)) {
retval = ks7010_sdio_read(priv, INT_PENDING, _data,
  sizeof(rw_data));
if (retval) {
diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
b/drivers/staging/ks7010/ks_wlan_net.c
index f6f7ffd..f18ff56 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1806,11 +1806,11 @@ static int ks_wlan_set_encode_ext(struct net_device 
*dev,
commit |= SME_WEP_INDEX;
} else if (enc->ext_flags & IW_ENCODE_EXT_RX_SEQ_VALID) {
memcpy(>wpa.key[index].rx_seq[0],
-   enc->rx_seq, IW_ENCODE_SEQ_MAX_SIZE);
+  enc->rx_seq, IW_ENCODE_SEQ_MAX_SIZE);
}
 
memcpy(>wpa.key[index].addr.sa_data[0],
-   >addr.sa_data[0], ETH_ALEN);
+  >addr.sa_data[0], ETH_ALEN);
 
switch (enc->alg) {
case IW_ENCODE_ALG_NONE:
@@ -1829,7 +1829,7 @@ static int ks_wlan_set_encode_ext(struct net_device *dev,
}
if (enc->key_len) {
memcpy(>wpa.key[index].key_val[0],
-   >key[0], enc->key_len);
+  >key[0], enc->key_len);
priv->wpa.key[index].key_len = enc->key_len;
commit |= (SME_WEP_VAL1 << index);
}
@@ -1841,19 +1841,19 @@ static int ks_wlan_set_encode_ext(struct net_device 
*dev,
}
if (enc->key_len == 32) {
memcpy(>wpa.key[index].key_val[0],
-   >key[0], enc->key_len - 16);
+  >key[0], enc->key_len - 16);
priv->wpa.key[index].key_len =
enc->key_len - 16;
if (priv->wpa.key_mgmt_suite == 4) {/* WPA_NONE */
memcpy(>wpa.key[index].
-   tx_mic_key[0], >key[16], 8);
+  tx_mic_key[0], >key[16], 8);
memcpy(>wpa.key[index].
-   rx_mic_key[0], >key[16], 8);
+  rx_mic_key[0], >key[16], 8);
} else {
memcpy(>wpa.key[index].
-   tx_mic_key[0], >key[16], 8);
+  tx_mic_key[0], >key[16], 8);
memcpy(>wpa.key[index].
-   rx_mic_key[0], >key[24], 8);
+  rx_mic_key[0], >key[24], 8);
}
commit |= (SME_WEP_VAL1 << index);
}
-- 
2.7.4

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

[PATCH 01/10] staging: ks7010: fix checkpatch LINE_SPACING

[Tobin C. Harding]

Checkpatch emits CHECK: Please use a blank line after
function/struct/union/enum declarations.

Add blank line after function definition.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_wlan_net.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
b/drivers/staging/ks7010/ks_wlan_net.c
index 1b7027c..f6f7ffd 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -2360,6 +2360,7 @@ static int ks_wlan_get_sleep_mode(struct net_device *dev,
 
return 0;
 }
+
 #ifdef WPS
 
 static int ks_wlan_set_wps_enable(struct net_device *dev,
-- 
2.7.4

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

[PATCH 02/10] staging: ks7010: fix checkpatch SPACING

[Tobin C. Harding]

Checkpatch emits CHECK: No space is necessary after a cast.

Remove unnecessary space after cast.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/michael_mic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ks7010/michael_mic.c 
b/drivers/staging/ks7010/michael_mic.c
index de5e724..80497ef 100644
--- a/drivers/staging/ks7010/michael_mic.c
+++ b/drivers/staging/ks7010/michael_mic.c
@@ -141,8 +141,8 @@ void MichaelMICFunction(struct michael_mic_t *Mic, u8 *Key,
 * +--+--++--++--+--+--+--+--+--+--+--+
 */
MichaelInitializeFunction(Mic, Key);
-   MichaelAppend(Mic, (uint8_t *) Data, 12);   /* |DA|SA| */
+   MichaelAppend(Mic, (uint8_t *)Data, 12);/* |DA|SA| */
MichaelAppend(Mic, pad_data, 4);/* |Priority|0|0|0| */
-   MichaelAppend(Mic, (uint8_t *) (Data + 12), Len - 12);  /* |Data| */
+   MichaelAppend(Mic, (uint8_t *)(Data + 12), Len - 12);   /* |Data| */
MichaelGetMIC(Mic, Result);
 }
-- 
2.7.4

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

[PATCH 05/10] staging: ks7010: fix checkpatch MULTIPLE_ASSIGNMENTS

[Tobin C. Harding]

Checkpatch emits CHECK: multiple assignments should be avoided.

Move multiple line assignment to individual lines.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_hostif.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index 68e26f4..3eb0f2e 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -2682,7 +2682,8 @@ int hostif_init(struct ks_wlan_private *priv)
INIT_LIST_HEAD(>pmklist.pmk[i].list);
 
priv->sme_i.sme_status = SME_IDLE;
-   priv->sme_i.qhead = priv->sme_i.qtail = 0;
+   priv->sme_i.qhead = 0;
+   priv->sme_i.qtail = 0;
 #ifdef KS_WLAN_DEBUG
priv->sme_i.max_event_count = 0;
 #endif
-- 
2.7.4

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

Re: [PATCH 0/6] staging: ks7010: clean up return statement code

[Tobin C. Harding]

On Tue, Mar 14, 2017 at 09:20:11PM +1100, Tobin C. Harding wrote:
[snip]

Drop this series please. (patch 1 of set already merged).

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

Re: [PATCH 0/5] staging: vc04_services: camera driver maintance

[Michael Zoran]

On Mon, 2017-03-20 at 23:40 +0300, Dan Carpenter wrote:
> On Mon, Mar 20, 2017 at 07:53:18AM -0700, Michael Zoran wrote:
> > On Mon, 2017-03-20 at 16:57 +0300, Dan Carpenter wrote:
> > > On Mon, Mar 20, 2017 at 04:06:00AM -0700, Michael Zoran wrote:
> > > > On Mon, 2017-03-20 at 13:55 +0300, Dan Carpenter wrote:
> > > > > I'm not going to review this because it has kbuild errors.
> > > > > 
> > > > > regards,
> > > > > dan carpenter
> > > > > 
> > > > 
> > > > Hi, can you e-mail out the errors or send them to me.  It
> > > > worked
> > > > when
> > > > I submitted it.
> > > > 
> > > 
> > > The problem is that you added a function only for ARM but the
> > > camera
> > > can build on i386.
> > > 
> > > Anyway, we need to figure out why you aren't getting the kbuild
> > > emails
> > > and fix that.  I forwarded the first email to you.  The other is
> > > basically the same.
> > > 
> > > regards,
> > > dan carpenter
> > > 
> > 
> > OK, thanks for sending it to me.
> > 
> > I don't quite understand how that is possible in all since the
> > whole
> > subtree depends on RASPBERRYPI_FIRMWARE which I had nothing to do
> > with
> > setting up.  Sounds to me like the real issue here is that the
> > RASPBERRYPI_FIRMARE driver wasn't setup correctly in the first
> > place.
> 
> It depends RASPBERRYPI_FIRMARE or COMPILE_TEST.  The kbuild bot has
> COMPILE_TEST enabled so that solves the mystery of why the build
> failed.

OK, since vchiq_arm has no additional dependencies beyond the camera
please explain exactly why the camera is being compiled for i386 but
vchiq_arm isn't.  I think we definately need to get to the bottom of
this.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 4/6] staging: media: atomisp: fix build when PM is disabled

[Jérémy Lefaure]

On Thu, 16 Mar 2017 17:50:10 +
Alan Cox  wrote:

> > +   if (IS_ENABLED(CONFI_PM)) {  
> 
> I think not.
> 
> Please actually test build both ways at the very least.
> 
> Alan
> 

I did test and it did compile both ways. I didn't think about checking
the output of the compilation and I don't have the hardware to test it.
So I think that I will resend only the second part of this patch
(modifying atomisp_v4l2.c) because I think that it could be a good idea
to test the first part.

Thanks,
Jérémy
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 4/4 V2] staging: atomisp: remove redudant condition in if-statement

[Daeseok Youn]

The V4L2_FIELD_ANY is zero, so the (!field) is same meaning
with (field == V4L2_FIELD_ANY) in if-statement.

Signed-off-by: Daeseok Youn 
---
V2: one(2/4) of this series was updated so I tried to send them again.

 drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index f7c0705..943a7ae 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -5084,7 +5084,7 @@ int atomisp_try_fmt(struct video_device *vdev, struct 
v4l2_format *f,
 
depth = get_pixel_depth(pixelformat);
 
-   if (!field || field == V4L2_FIELD_ANY)
+   if (field == V4L2_FIELD_ANY)
field = V4L2_FIELD_NONE;
else if (field != V4L2_FIELD_NONE) {
dev_err(isp->dev, "Wrong output field\n");
-- 
1.9.1

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

[PATCH 3/4 V2] staging: atomisp: remove useless condition in if-statements

[Daeseok Youn]

The css_pipe_id was checked with 'CSS_PIPE_ID_COPY' in previous if-
statement. In this case, if the css_pipe_id equals to 'CSS_PIPE_ID_COPY',
it could not enter the next if-statement. But the "next" if-statement
has the condition to check whether the css_pipe_id equals to
'CSS_PIPE_ID_COPY' or not. It should be removed.

Signed-off-by: Daeseok Youn 
---
V2: one(2/4) of this series was updated so I tried to send them again.

 drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index 811331d..f7c0705 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -910,8 +910,7 @@ static struct atomisp_video_pipe *__atomisp_get_pipe(
} else if (asd->run_mode->val == ATOMISP_RUN_MODE_VIDEO) {
/* For online video or SDV video pipe. */
if (css_pipe_id == CSS_PIPE_ID_VIDEO ||
-   css_pipe_id == CSS_PIPE_ID_COPY ||
-   css_pipe_id == CSS_PIPE_ID_YUVPP) {
+   css_pipe_id == CSS_PIPE_ID_COPY) {
if (buf_type == CSS_BUFFER_TYPE_OUTPUT_FRAME)
return >video_out_video_capture;
return >video_out_preview;
@@ -919,8 +918,7 @@ static struct atomisp_video_pipe *__atomisp_get_pipe(
} else if (asd->run_mode->val == ATOMISP_RUN_MODE_PREVIEW) {
/* For online preview or ZSL preview pipe. */
if (css_pipe_id == CSS_PIPE_ID_PREVIEW ||
-   css_pipe_id == CSS_PIPE_ID_COPY ||
-   css_pipe_id == CSS_PIPE_ID_YUVPP)
+   css_pipe_id == CSS_PIPE_ID_COPY)
return >video_out_preview;
}
/* For capture pipe. */
-- 
1.9.1

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

[PATCH 2/4 V2] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()

[Daeseok Youn]

If v4l2_subdev_call() gets the global frame interval values,
it returned 0 and it could be checked whether numerator is zero or not.

If the numerator is not zero, the fps could be calculated in this function.
If not, it just returns 0.

Signed-off-by: Daeseok Youn 
---
V2: split error handling, the first check is for the result from 
v4l2_subdev_call(),
another check is for fi.interval.numerator > 0. it is more understandable 
and
simpler than before.

 .../media/atomisp/pci/atomisp2/atomisp_cmd.c   | 24 ++
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index 8bdb224..811331d 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -153,21 +153,19 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct 
video_device *dev)
 
 static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device *asd)
 {
-   struct v4l2_subdev_frame_interval frame_interval;
+   struct v4l2_subdev_frame_interval fi;
struct atomisp_device *isp = asd->isp;
-   unsigned short fps;
 
-   if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
-   video, g_frame_interval, _interval)) {
-   fps = 0;
-   } else {
-   if (frame_interval.interval.numerator)
-   fps = frame_interval.interval.denominator /
-   frame_interval.interval.numerator;
-   else
-   fps = 0;
-   }
-   return fps;
+   int ret;
+
+   ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
+  video, g_frame_interval, );
+
+   if (ret)
+   return 0;
+
+   return (fi.interval.numerator > 0) ?
+  (fi.interval.denominator / fi.interval.numerator) : 0;
 }
 
 /*
-- 
1.9.1

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

[PATCH 1/4 V2] staging: atomisp: remove else statement after return

[Daeseok Youn]

It doesn't need to have else statement after return.

Signed-off-by: Daeseok Youn 
---
V2: one(2/4) of this series was updated so I tried to send them again.

 drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index d97a8df..8bdb224 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -2958,11 +2958,11 @@ int atomisp_get_metadata(struct atomisp_sub_device 
*asd, int flag,
dev_err(isp->dev, "copy to user failed: copied %d bytes\n",
ret);
return -EFAULT;
-   } else {
-   list_del_init(_buf->list);
-   list_add_tail(_buf->list, >metadata[md_type]);
}
 
+   list_del_init(_buf->list);
+   list_add_tail(_buf->list, >metadata[md_type]);
+
dev_dbg(isp->dev, "%s: HAL de-queued metadata type %d with exp_id %d\n",
__func__, md_type, md->exp_id);
return 0;
-- 
1.9.1

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

Re: [PATCH 2/4] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()

[DaeSeok Youn]

2017-03-20 22:11 GMT+09:00 walter harms :
>
>
> Am 20.03.2017 13:51, schrieb DaeSeok Youn:
>> 2017-03-20 21:04 GMT+09:00 walter harms :
>>>
>>>
>>> Am 20.03.2017 11:59, schrieb Daeseok Youn:
 If v4l2_subdev_call() gets the global frame interval values,
 it returned 0 and it could be checked whether numerator is zero or not.

 If the numerator is not zero, the fps could be calculated in this function.
 If not, it just returns 0.

 Signed-off-by: Daeseok Youn 
 ---
  .../media/atomisp/pci/atomisp2/atomisp_cmd.c   | 22 
 ++
  1 file changed, 10 insertions(+), 12 deletions(-)

 diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
 b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
 index 8bdb224..6bdd19e 100644
 --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
 +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
 @@ -153,20 +153,18 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct 
 video_device *dev)

  static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device 
 *asd)
  {
 - struct v4l2_subdev_frame_interval frame_interval;
 + struct v4l2_subdev_frame_interval fi;
   struct atomisp_device *isp = asd->isp;
 - unsigned short fps;

 - if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
 - video, g_frame_interval, _interval)) {
 - fps = 0;
 - } else {
 - if (frame_interval.interval.numerator)
 - fps = frame_interval.interval.denominator /
 - frame_interval.interval.numerator;
 - else
 - fps = 0;
 - }
 + unsigned short fps = 0;
 + int ret;
 +
 + ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
 +video, g_frame_interval, );
 +
 + if (!ret && fi.interval.numerator)
 + fps = fi.interval.denominator / fi.interval.numerator;
 +
   return fps;
  }
>>>
>>>
>>>
>>> do you need to check ret at all ? if an error occurs can 
>>> fi.interval.numerator
>>> be something else than 0 ?
>> the return value from the v4l2_subdev_call() function is zero when it
>> is done without any error. and also I checked
>> the ret value whether is 0 or not. if the ret is 0 then the value of
>> numerator should be checked to avoid for dividing by 0.
>>>
>>> if ret is an ERRNO it would be wise to return ret not fps, but this may 
>>> require
>>> changes at other places also.
>> hmm.., yes, you are right. but I think it is ok because the
>> atomisp_get_sensor_fps() function is needed to get fps value.
>> (originally, zero or calculated fps value was returned.)
>
> maybe its better to divide this in:
> if (ret)
>return 0; // error case
>
> return (fi.interval.numerator>0)?fi.interval.denominator / 
> fi.interval.numerator:0;
>
> So there is a chance that someone will a) understand and b) fix the error 
> return.
yes, it looks better than mine. I will update it and resend it.

Thanks walter,
Regards,
Daeseok Youn.

>
> re,
>  wh
>
>>
>>>
>>> re,
>>>  wh
>>>

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

RE: [PATCH] HV: properly delay KVP packets when negotiation is in progress

[Long Li]

> -Original Message-
> From: Long Li
> Sent: Sunday, March 19, 2017 7:49 PM
> To: 'Vitaly Kuznetsov' 
> Cc: KY Srinivasan ; Haiyang Zhang
> ; Stephen Hemminger
> ; de...@linuxdriverproject.org; linux-
> ker...@vger.kernel.org
> Subject: RE: [PATCH] HV: properly delay KVP packets when negotiation is in
> progress
> 
> 
> 
> > -Original Message-
> > From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
> > Sent: Friday, March 17, 2017 9:16 AM
> > To: Long Li 
> > Cc: KY Srinivasan ; Haiyang Zhang
> > ; Stephen Hemminger
> ;
> > de...@linuxdriverproject.org; linux- ker...@vger.kernel.org
> > Subject: Re: [PATCH] HV: properly delay KVP packets when negotiation
> > is in progress
> >
> > Long Li  writes:
> >
> > > The host may send multiple KVP packets before the negotiation with
> > > daemon is finished. We need to keep those packets in ring buffer
> > > until the daemon is negotiated and connected.
> >
> > The patch looks OK but previously we always presumed that this can't
> > happen for util drivers and host will never send a new request before
> > we answer to the previous one. If this is not true we may have more
> > issues which need fixing as all three drivers we have are written in a
> 'transaction'
> > fashion.
> >
> > So my question would be: can the host send multiple (KVP) packets
> > _after_ the negotiation with daemon is finished?
> 
> Thanks Vitaly. I'm checking with Windows guys and will update soon.

It's possible that hosts may send a number of staged KVP requests as soon as 
negotiation is done. The current KVP code can deal with a number of pending KVP 
requests, and respond to them one by one.

To summarize the issue this patch tries to fix:
1. When host sends a negotiation request, and it times out, the host will send 
another negotiation request, and so on.
2. The guest can respond to all negotiation requests from the host. All 
subsequent response (except for the 1st response) are ignored by the host.
3. Before negotiation is done, the host may have staged a number of pending KVP 
requests.
4. As soon as negotiation is done, the host sends all KVP requests to guest.

There is a corner case that if there is only one pending KVP request after the 
2nd (or 3rd etc) negotiation, it may get lost. I'm testing the following code 
to address this condition:

@@ -705,6 +706,7 @@ void hv_kvp_onchannelcallback(void *context)
   VM_PKT_DATA_INBAND, 0);

host_negotiatied = NEGO_FINISHED;
+   hv_poll_channel(kvp_transaction.recv_channel, kvp_poll_wrapper);
}

 }

Please drop this patch. I'll send V2.

> 
> >
> >
> > >
> > > This patch is based on the work of Nick Meier
> > > 
> > >
> > > Signed-off-by: Long Li 
> > > ---
> > >  drivers/hv/hv_kvp.c | 9 +
> > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c index
> > > de26371..b9f928d 100644
> > > --- a/drivers/hv/hv_kvp.c
> > > +++ b/drivers/hv/hv_kvp.c
> > > @@ -628,16 +628,17 @@ void hv_kvp_onchannelcallback(void *context)
> > >NEGO_IN_PROGRESS,
> > >NEGO_FINISHED} host_negotiatied =
> > NEGO_NOT_STARTED;
> > >
> > > - if (host_negotiatied == NEGO_NOT_STARTED &&
> > > - kvp_transaction.state < HVUTIL_READY) {
> > > + if (kvp_transaction.state < HVUTIL_READY) {
> > >   /*
> > >* If userspace daemon is not connected and host is asking
> > >* us to negotiate we need to delay to not lose messages.
> > >* This is important for Failover IP setting.
> > >*/
> > > - host_negotiatied = NEGO_IN_PROGRESS;
> > > - schedule_delayed_work(_host_handshake_work,
> > > + if (host_negotiatied == NEGO_NOT_STARTED) {
> > > + host_negotiatied = NEGO_IN_PROGRESS;
> > > +
> > schedule_delayed_work(_host_handshake_work,
> > > HV_UTIL_NEGO_TIMEOUT * HZ);
> > > + }
> > >   return;
> > >   }
> > >   if (kvp_transaction.state > HVUTIL_READY)
> >
> > --
> >   Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 06:23:24PM +0100, Philipp Zabel wrote:
> --8<--
> >From 2830aebc404bdfc9d7fc1ec94e5282d0b668e8f6 Mon Sep 17 00:00:00 2001
> From: Philipp Zabel 
> Date: Mon, 20 Mar 2017 17:10:21 +0100
> Subject: [PATCH] media: imx: csi: add sink selection rectangles
> 
> Move the crop rectangle to the sink pad and add a sink compose rectangle
> to configure scaling. Also propagate rectangles from sink pad to crop
> rectangle, to compose rectangle, and to the source pads both in ACTIVE
> and TRY variants of set_fmt/selection, and initialize the default crop
> and compose rectangles.

Looks fine for the most part.

> - /*
> -  * Modifying the crop rectangle always changes the format on the source
> -  * pad. If the KEEP_CONFIG flag is set, just return the current crop
> -  * rectangle.
> -  */
> - if (sel->flags & V4L2_SEL_FLAG_KEEP_CONFIG) {
> - sel->r = priv->crop;
> - if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
> - cfg->try_crop = sel->r;
> + infmt = __csi_get_fmt(priv, cfg, CSI_SINK_PAD, sel->which);
> + crop = __csi_get_crop(priv, cfg, sel->which);
> + compose = __csi_get_compose(priv, cfg, sel->which);
> +
> + switch (sel->target) {
> + case V4L2_SEL_TGT_CROP:
> + /*
> +  * Modifying the crop rectangle always changes the format on
> +  * the source pads. If the KEEP_CONFIG flag is set, just return
> +  * the current crop rectangle.
> +  */
> + if (sel->flags & V4L2_SEL_FLAG_KEEP_CONFIG) {
> + sel->r = priv->crop;

My understanding of KEEP_CONFIG is that the only thing we're not
allowed to do is to propagate the change downstream.

Since downstream of the crop is the compose, that means the only
restriction here is that the width and height of the crop window must
be either equal to the compose width/height, or double the compose
width/height.  (Anything else would necessitate the compose window
changing.)

However, the crop window can move position within the crop bounds,
provided it's entirely contained within those crop bounds.

The problem is that this becomes rather more complex it deal with
(as I'm finding out in my imx219 camera driver) and I'm thinking
that some of this complexity should probably be in a helper in
generic v4l2 code.

I don't know whether this applies (I hope it doesn't) but there's a
pile of guidelines in Documentation/media/uapi/v4l/vidioc-g-selection.rst
which describe how a crop/compose rectangle should be adjusted.  As
I say, I hope they don't apply, because if they do, we would _really_
need helpers for this stuff, as I don't think having each driver
implement all these rules would be too successful!

> + if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
> + *crop = sel->r;
> + goto out;
> + }
> +
> + csi_try_crop(priv, >r, cfg, infmt, sensor);
> +
> + *crop = sel->r;
> +
> + /* Reset scaling to 1:1 */
> + compose->width = crop->width;
> + compose->height = crop->height;
> + break;
> + case V4L2_SEL_TGT_COMPOSE:
> + /*
> +  * Modifying the compose rectangle always changes the format on
> +  * the source pads. If the KEEP_CONFIG flag is set, just return
> +  * the current compose rectangle.
> +  */
> + if (sel->flags & V4L2_SEL_FLAG_KEEP_CONFIG) {
> + sel->r = priv->compose;

I think, with my understanding of how the KEEP_CONFIG flag works, this
should be:
sel->r = *compose;

because if we change the compose rectangle width/height, we would need
to propagate this to the source pad, and the KEEP_CONFIG description
says we're not allowed to do that.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/5] staging: vc04_services: camera driver maintance

[Dan Carpenter]

On Mon, Mar 20, 2017 at 07:53:18AM -0700, Michael Zoran wrote:
> On Mon, 2017-03-20 at 16:57 +0300, Dan Carpenter wrote:
> > On Mon, Mar 20, 2017 at 04:06:00AM -0700, Michael Zoran wrote:
> > > On Mon, 2017-03-20 at 13:55 +0300, Dan Carpenter wrote:
> > > > I'm not going to review this because it has kbuild errors.
> > > > 
> > > > regards,
> > > > dan carpenter
> > > > 
> > > 
> > > Hi, can you e-mail out the errors or send them to me.  It worked
> > > when
> > > I submitted it.
> > > 
> > 
> > The problem is that you added a function only for ARM but the camera
> > can build on i386.
> > 
> > Anyway, we need to figure out why you aren't getting the kbuild
> > emails
> > and fix that.  I forwarded the first email to you.  The other is
> > basically the same.
> > 
> > regards,
> > dan carpenter
> > 
> 
> OK, thanks for sending it to me.
> 
> I don't quite understand how that is possible in all since the whole
> subtree depends on RASPBERRYPI_FIRMWARE which I had nothing to do with
> setting up.  Sounds to me like the real issue here is that the
> RASPBERRYPI_FIRMARE driver wasn't setup correctly in the first place.

It depends RASPBERRYPI_FIRMARE or COMPILE_TEST.  The kbuild bot has
COMPILE_TEST enabled so that solves the mystery of why the build failed.

> I also noticed when I was trying to understand how the raspberrypi
> driver works in the first place in that it doesn't always dump the
> firmware revision correctly if the DT node mbox handle gets pointed to
> who knows where.
> 
> I've been complaining about very selective e-mails on so called public
> e-mail lists for a very long time now, and nobody has done anything
> about it.

I have no idea what your talking about but I try to forget emails right
away so that I'll be able to answer honestly that, "I don't recall" in
case I am ever asked to testify in front of the United States senate.

> Sometimes I don't get any e-mail for a week and then I'll get
> a flood of e-mail thats 2 weeks old.  And who knows which e-mails lists
> changes are going to.

This business with multiple lists *is* very annoying.  Patches to do
with kernel code should be sent to driver-devel.

regards,
dan carpenter


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

[PATCH v3] vmbus: fix missed ring events on boot

[Stephen Hemminger]

During initialization, the channel initialization code schedules the
tasklet to scan the VMBUS receive event page (i.e. simulates an
interrupt). The problem was that it invokes the tasklet on a different
CPU from where it normally runs and therefore if an event is present,
it will clear the bit but not find the associated channel.

This can lead to missed events, typically stuck tasks, during bootup
when sub channels are being initialized. Typically seen as stuck
boot with 8 or more CPU's.

This patch is for 4.10 and 4.9; but was introduced by fix to solve
a race on updates. It is not necessary for 4.11 and later since
commit 631e63a9f346 ("vmbus: change to per channel tasklet").
which eliminated the common tasklet which caused the problem.

Cc: sta...@vger.kernel.org
Fixes: 638fea33aee8 ("Drivers: hv: vmbus: fix the race when querying & updating 
the percpu list")
Signed-off-by: Stephen Hemminger 
---
Patch is against linux-4.10.y branch

v3 - avoid possible preempt race
v2 - simplify only change hv_event_tasklet_enable

 drivers/hv/channel_mgmt.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 0af7e39006c8..74afef6271f0 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -361,8 +361,17 @@ void hv_event_tasklet_enable(struct vmbus_channel *channel)
tasklet = hv_context.event_dpc[channel->target_cpu];
tasklet_enable(tasklet);
 
-   /* In case there is any pending event */
-   tasklet_schedule(tasklet);
+   /*
+* In case there is any pending event schedule a rescan
+* but must be on the correct CPU for the channel.
+*/
+   if (channel->target_cpu == get_cpu())
+   tasklet_schedule(tasklet);
+   else
+   smp_call_function_single(channel->target_cpu,
+(smp_call_func_t)tasklet_schedule,
+tasklet, false);
+   put_cpu();
 }
 
 void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
-- 
2.11.0

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

[PATCH v3] staging: ad7606: Replace mlock with driver private lock

[Arushi Singhal]

The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.

In this driver, mlock was being used to protect hardware state
changes.  Replace it with a lock in the devices global data.

Signed-off-by: Arushi Singhal 
---
 changes in v3
 - Add mutex_init.
 - correct the Documentation.

 drivers/staging/iio/adc/ad7606.c | 9 +
 drivers/staging/iio/adc/ad7606.h | 3 +++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 9dbfa64b1e53..18f5f139117e 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -208,7 +208,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_SCALE:
ret = -EINVAL;
-   mutex_lock(_dev->mlock);
+   mutex_lock(>lock);
for (i = 0; i < ARRAY_SIZE(scale_avail); i++)
if (val2 == scale_avail[i][1]) {
gpiod_set_value(st->gpio_range, i);
@@ -217,7 +217,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
ret = 0;
break;
}
-   mutex_unlock(_dev->mlock);
+   mutex_unlock(>lock);
 
return ret;
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
@@ -231,11 +231,11 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
values[1] = (ret >> 1) & 1;
values[2] = (ret >> 2) & 1;
 
-   mutex_lock(_dev->mlock);
+   mutex_lock(>lock);
gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
  values);
st->oversampling = val;
-   mutex_unlock(_dev->mlock);
+   mutex_unlock(>lock);
 
return 0;
default:
@@ -413,6 +413,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem 
*base_address,
st = iio_priv(indio_dev);
 
st->dev = dev;
+   mutex_init(>lock);
st->bops = bops;
st->base_address = base_address;
/* tied to logic low, analog input range is +/- 5V */
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 746f9553d2ba..acaed8d5379c 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -14,6 +14,7 @@
  * @name:  identification string for chip
  * @channels:  channel specification
  * @num_channels:  number of channels
+ * @lock   protect sensor state
  */
 
 struct ad7606_chip_info {
@@ -23,6 +24,7 @@ struct ad7606_chip_info {
 
 /**
  * struct ad7606_state - driver instance specific data
+ * @lock   protect sensor state
  */
 
 struct ad7606_state {
@@ -37,6 +39,7 @@ struct ad7606_state {
booldone;
void __iomem*base_address;
 
+   struct mutexlock; /* protect sensor state */
struct gpio_desc*gpio_convst;
struct gpio_desc*gpio_reset;
struct gpio_desc*gpio_range;
-- 
2.11.0

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

RE: [PATCH v2] vmbus: fix missed ring events on boot

[KY Srinivasan]

> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Saturday, March 18, 2017 9:55 PM
> To: gre...@linuxfoundation.org; KY Srinivasan ;
> Haiyang Zhang 
> Cc: sta...@vger.kernel.org; de...@linuxdriverproject.org; Stephen
> Hemminger 
> Subject: [PATCH v2] vmbus: fix missed ring events on boot
> 
> During initialization, the channel initialization code schedules the
> tasklet to scan the VMBUS receive event page (i.e. simulates an
> interrupt). The problem was that it invokes the tasklet on a different
> CPU from where it normally runs and therefore if an event is present,
> it will clear the bit but not find the associated channel.
> 
> This can lead to missed events, typically stuck tasks, during bootup
> when sub channels are being initialized. Typically seen as stuck
> boot with 8 or more CPU's.
> 
> This patch is not necessary for upstream (4.11 and later) since
> commit 631e63a9f346 ("vmbus: change to per channel tasklet").
> This changed vmbus code to get rid of common tasklet which
> caused the problem.
> 
> Cc: sta...@vger.kernel.org
> Fixes: 638fea33aee8 ("Drivers: hv: vmbus: fix the race when querying &
> updating the percpu list")
> Signed-off-by: Stephen Hemminger 
> ---
> v2 - simplified version (only need to change one function).
>  also don't need to wait for tasklet to be scheduled on other CPU
>  add Fixes tag.
> 
>  drivers/hv/channel_mgmt.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 0af7e39006c8..63c903b00a58 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -361,8 +361,19 @@ void hv_event_tasklet_enable(struct
> vmbus_channel *channel)
>   tasklet = hv_context.event_dpc[channel->target_cpu];
>   tasklet_enable(tasklet);
> 
> - /* In case there is any pending event */
> - tasklet_schedule(tasklet);
> + /*
> +  * In case there is any pending event schedule a rescan
> +  * but must be on the correct CPU for the channel.
> +  */
> + if (channel->target_cpu == get_cpu()) {
> + put_cpu();

We could be preempted here and end up scheduling the taklet on the wrong CPU.

> + tasklet_schedule(tasklet);
> + } else {
> + smp_call_function_single(channel->target_cpu,
> +  (smp_call_func_t)tasklet_schedule,
> +  tasklet, false);
> + put_cpu();
> + }

Why not call put_cpu() at the end of the block to close the preemption window 
we have
earlier.

K. Y
>  }
> 
>  void hv_process_channel_removal(struct vmbus_channel *channel, u32
> relid)
> --
> 2.11.0

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

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Steve Longerbeam]

On 03/20/2017 07:00 AM, Philipp Zabel wrote:

On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote:

On Mon, Mar 20, 2017 at 12:55:26PM +0100, Philipp Zabel wrote:

The above paragraph suggests we skip any rectangles that are not
supported. In our case that would be 3. and 4., since the CSI can't
compose into a larger frame. I hadn't realised that the crop selection
currently happens on the source pad.

I'd recommend viewing the documentation in its post-processed version,
because then you get the examples as pictures, and they say that a
picture is worth 1000 words.  See

   https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/dev-subdev.html

There is almost an exact example of what we're trying to do - it's
figure 4.6.  Here, we have a sink pad with a cropping rectangle on
the input, which is then scaled to a composition rectangle (there's
no bounds rectangle, and it's specified that in such a case the
top,left of the composition rectangle will always be 0,0 - see quote
below).

Where it differs is that the example also supports source cropping
for two source pads.  We don't support that.

The same document says:

   Scaling support is optional. When supported by a subdev, the crop
   rectangle on the subdev's sink pad is scaled to the size configured
   using the
   :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL
   using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the
   subdev supports scaling but not composing, the top and left values are
   not used and must always be set to zero.

Right, this sentence does imply that when scaling is supported, there
must be a sink compose rectangle, even when composing is not.


Ok, this all makes consistent sense to me too. So:

- the CSI hardware cropping rectangle should be specified via the
  sink pad crop selection.

- the CSI hardware /2 downscaler should be specified via the
  sink pad compose selection.

- the final source pad rectangle is the same as the sink pad
  compose rectangle.

So that leaves only step 4 (source pad crop selection) as
unsupported.

Steve



I have previously set up scaling like this:

media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

Does this mean, it should work like this instead?

media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
media-ctl --set-v4l2 
"'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

I suppose setting the source pad format should not be allowed to modify
the sink compose rectangle.

regards
Philipp



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

Re: [PATCH v3] staging: ad7606: Replace mlock with driver private lock

[Lars-Peter Clausen]

On 03/20/2017 07:56 PM, Arushi Singhal wrote:
[...]
> @@ -413,6 +413,7 @@ int ad7606_probe(struct device *dev, int irq, void 
> __iomem *base_address,
>   st = iio_priv(indio_dev);
>  
>   st->dev = dev;
> + mutex_init(>lock);

This is nitpicking, but putting this in the middle of the assignments has a
bit of a weired flow it. Either put it above or below the assignments.

>   st->bops = bops;
>   st->base_address = base_address;
>   /* tied to logic low, analog input range is +/- 5V */
> diff --git a/drivers/staging/iio/adc/ad7606.h 
> b/drivers/staging/iio/adc/ad7606.h
> index 746f9553d2ba..5d59bdd78727 100644
> --- a/drivers/staging/iio/adc/ad7606.h
> +++ b/drivers/staging/iio/adc/ad7606.h
> @@ -14,6 +14,7 @@
>   * @name:identification string for chip
>   * @channels:channel specification
>   * @num_channels:number of channels
> + * @lock protect sensor state

This documentation is for struct ad7607_chip_info, but you are adding the
field to struct ad7606_state.

>   */
>  
>  struct ad7606_chip_info {
> @@ -37,6 +38,7 @@ struct ad7606_state {
>   booldone;
>   void __iomem*base_address;
>  
> + struct mutexlock; /* protect sensor state */
>   struct gpio_desc*gpio_convst;
>   struct gpio_desc*gpio_reset;
>   struct gpio_desc*gpio_range;
> 

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

Re: [PATCH 1/6] bcm2835-gpio-exp: Driver for GPIO expander via mailbox service

[Michael Zoran]

On Mon, 2017-03-20 at 11:54 -0700, Michael Zoran wrote:
> On Mon, 2017-03-20 at 10:28 -0700, Michael Zoran wrote:
> > On Mon, 2017-03-20 at 10:22 -0700, Eric Anholt wrote:
> > > Michael Zoran  writes:
> > > 
> > > > > > Since the API is completely documented, I see no reason we
> > > > > > or
> > > > > > anybody
> > > > > > couldn't essentially rewrite the driver while it's in
> > > > > > staging.  I
> > > > > > just
> > > > > > think it would be best for everyone if the new version was
> > > > > > a
> > > > > > drop
> > > > > > in
> > > > > > replacement for the original version.  Essential an
> > > > > > enhancement
> > > > > > rather
> > > > > > then a competitor.
> > > > > 
> > > > > I think my comments weren't fundamental changes, but you
> > > > > surely
> > > > > mean
> > > > > the devicetree ABI? I like to see this driver ASAP out of
> > > > > staging
> > > > > and
> > > > > i'm not interested to maintain 2 functional identical driver
> > > > > only
> > > > > to
> > > > > keep compability with the Foundation tree. Currently i'm
> > > > > afraid
> > > > > that
> > > > > we build up many drivers in staging, which need a complete
> > > > > rewrite
> > > > > later if they should come out of staging. It would be nice if
> > > > > we
> > > > > could avoid the situation we have with the thermal driver.
> > > > > 
> > > > > Stefan
> > > > 
> > > > The API I'm talking about here is the mailbox API that is used
> > > > to
> > > > talk
> > > > to the firmware.  The numbers and structures to pass are
> > > > documented. 
> > > > Nothing prevents anybody from rewriting this driver and
> > > > submitting
> > > > it
> > > > to the appropriate subsystems.  It's certainly small enough.
> > > > 
> > > > If you really want working thermal or cpu speed drivers today,
> > > > nothing
> > > > stops anybody from submitting the downstream drivers after
> > > > doing
> > > > some
> > > > minor touchups and submitting them to staging.  That would at
> > > > least
> > > > get
> > > > things working while people argue about what the correct DT
> > > > nodes
> > > > should be.
> > > > 
> > > > I would also like to point out that the RPI 3 has been out for
> > > > over
> > > > a
> > > > year and nobody has been able to get working video out of it
> > > > through
> > > > VC4 on a mainline tree.  At least until now.  So I'm not sure
> > > > the
> > > > best
> > > > way to go is for the expander driver to go under the GPIO
> > > > subtree.
> > > 
> > > Excuse me?  Display works fine on my Pi3.  VC4 uses DDC to probe
> > > for
> > > connection when the GPIO line isn't present in the DT.
> > 
> > Is this arm32 or arm64 modes? And is this through the simplefb that
> > was
> > adding for testing is the VC4 driver fully driving the display.  Is
> > VC4
> > still in the .config that you used? Is this a standard version of
> > VC4,
> > or does it include modifications for testing purposes?
> > 
> > If it works so well as is, why do you need or want the
> > expander?  You
> > tried to submit a very similar version a year ago?
> 
> Since Stephan seems to have taken this sudden intense interest in
> vc04_services all of a sudden, perhaps now would be a excellent time
> to
> actually acomplish one of the TODOs and get the DT bindings in for
> this
> driver.  Since it is working an all.

Since Stefan always has such interesting comments on patches and I've
submitting changes to a driver that doesn't even offically load,
perhaps someone can help me get one of the TODO items removed and make
real progress getting this driver out of staging by sending me a real
e-mail address of who I should send an actual working DT for the RPI 3
to that will actually be seriously considered.

I'll leave it at that.

Thanks.

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

[PATCH v2] staging: ad7606: Replace mlock with driver private lock

[Arushi Singhal]

The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.

In this driver, mlock was being used to protect hardware state
changes.  Replace it with a lock in the devices global data.

Signed-off-by: Arushi Singhal 
---
 changes in v2
 - add mutex_init.

 drivers/staging/iio/adc/ad7606.c | 9 +
 drivers/staging/iio/adc/ad7606.h | 2 ++
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 9dbfa64b1e53..b58641369596 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -208,7 +208,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_SCALE:
ret = -EINVAL;
-   mutex_lock(_dev->mlock);
+   mutex_lock(>lock);
for (i = 0; i < ARRAY_SIZE(scale_avail); i++)
if (val2 == scale_avail[i][1]) {
gpiod_set_value(st->gpio_range, i);
@@ -217,7 +217,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
ret = 0;
break;
}
-   mutex_unlock(_dev->mlock);
+   mutex_unlock(>lock);
 
return ret;
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
@@ -231,11 +231,11 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
values[1] = (ret >> 1) & 1;
values[2] = (ret >> 2) & 1;
 
-   mutex_lock(_dev->mlock);
+   mutex_lock(>lock);
gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
  values);
st->oversampling = val;
-   mutex_unlock(_dev->mlock);
+   mutex_unlock(>lock);
 
return 0;
default:
@@ -413,6 +413,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem 
*base_address,
st = iio_priv(indio_dev);
 
st->dev = dev;
+   mutex_init(>state_lock);
st->bops = bops;
st->base_address = base_address;
/* tied to logic low, analog input range is +/- 5V */
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 746f9553d2ba..5d59bdd78727 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -14,6 +14,7 @@
  * @name:  identification string for chip
  * @channels:  channel specification
  * @num_channels:  number of channels
+ * @lock   protect sensor state
  */
 
 struct ad7606_chip_info {
@@ -37,6 +38,7 @@ struct ad7606_state {
booldone;
void __iomem*base_address;
 
+   struct mutexlock; /* protect sensor state */
struct gpio_desc*gpio_convst;
struct gpio_desc*gpio_reset;
struct gpio_desc*gpio_range;
-- 
2.11.0

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

[PATCH v3] staging: ad7606: Replace mlock with driver private lock

[Arushi Singhal]

The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.

In this driver, mlock was being used to protect hardware state
changes.  Replace it with a lock in the devices global data.

Signed-off-by: Arushi Singhal 
---
 changes in v3
 - Add mutex_init.

 drivers/staging/iio/adc/ad7606.c | 9 +
 drivers/staging/iio/adc/ad7606.h | 2 ++
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 9dbfa64b1e53..18f5f139117e 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -208,7 +208,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_SCALE:
ret = -EINVAL;
-   mutex_lock(_dev->mlock);
+   mutex_lock(>lock);
for (i = 0; i < ARRAY_SIZE(scale_avail); i++)
if (val2 == scale_avail[i][1]) {
gpiod_set_value(st->gpio_range, i);
@@ -217,7 +217,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
ret = 0;
break;
}
-   mutex_unlock(_dev->mlock);
+   mutex_unlock(>lock);
 
return ret;
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
@@ -231,11 +231,11 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
values[1] = (ret >> 1) & 1;
values[2] = (ret >> 2) & 1;
 
-   mutex_lock(_dev->mlock);
+   mutex_lock(>lock);
gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
  values);
st->oversampling = val;
-   mutex_unlock(_dev->mlock);
+   mutex_unlock(>lock);
 
return 0;
default:
@@ -413,6 +413,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem 
*base_address,
st = iio_priv(indio_dev);
 
st->dev = dev;
+   mutex_init(>lock);
st->bops = bops;
st->base_address = base_address;
/* tied to logic low, analog input range is +/- 5V */
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 746f9553d2ba..5d59bdd78727 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -14,6 +14,7 @@
  * @name:  identification string for chip
  * @channels:  channel specification
  * @num_channels:  number of channels
+ * @lock   protect sensor state
  */
 
 struct ad7606_chip_info {
@@ -37,6 +38,7 @@ struct ad7606_state {
booldone;
void __iomem*base_address;
 
+   struct mutexlock; /* protect sensor state */
struct gpio_desc*gpio_convst;
struct gpio_desc*gpio_reset;
struct gpio_desc*gpio_range;
-- 
2.11.0

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

Re: [PATCH 1/6] bcm2835-gpio-exp: Driver for GPIO expander via mailbox service

[Michael Zoran]

On Mon, 2017-03-20 at 10:28 -0700, Michael Zoran wrote:
> On Mon, 2017-03-20 at 10:22 -0700, Eric Anholt wrote:
> > Michael Zoran  writes:
> > 
> > > > > Since the API is completely documented, I see no reason we or
> > > > > anybody
> > > > > couldn't essentially rewrite the driver while it's in
> > > > > staging.  I
> > > > > just
> > > > > think it would be best for everyone if the new version was a
> > > > > drop
> > > > > in
> > > > > replacement for the original version.  Essential an
> > > > > enhancement
> > > > > rather
> > > > > then a competitor.
> > > > 
> > > > I think my comments weren't fundamental changes, but you surely
> > > > mean
> > > > the devicetree ABI? I like to see this driver ASAP out of
> > > > staging
> > > > and
> > > > i'm not interested to maintain 2 functional identical driver
> > > > only
> > > > to
> > > > keep compability with the Foundation tree. Currently i'm afraid
> > > > that
> > > > we build up many drivers in staging, which need a complete
> > > > rewrite
> > > > later if they should come out of staging. It would be nice if
> > > > we
> > > > could avoid the situation we have with the thermal driver.
> > > > 
> > > > Stefan
> > > 
> > > The API I'm talking about here is the mailbox API that is used to
> > > talk
> > > to the firmware.  The numbers and structures to pass are
> > > documented. 
> > > Nothing prevents anybody from rewriting this driver and
> > > submitting
> > > it
> > > to the appropriate subsystems.  It's certainly small enough.
> > > 
> > > If you really want working thermal or cpu speed drivers today,
> > > nothing
> > > stops anybody from submitting the downstream drivers after doing
> > > some
> > > minor touchups and submitting them to staging.  That would at
> > > least
> > > get
> > > things working while people argue about what the correct DT nodes
> > > should be.
> > > 
> > > I would also like to point out that the RPI 3 has been out for
> > > over
> > > a
> > > year and nobody has been able to get working video out of it
> > > through
> > > VC4 on a mainline tree.  At least until now.  So I'm not sure the
> > > best
> > > way to go is for the expander driver to go under the GPIO
> > > subtree.
> > 
> > Excuse me?  Display works fine on my Pi3.  VC4 uses DDC to probe
> > for
> > connection when the GPIO line isn't present in the DT.
> 
> Is this arm32 or arm64 modes? And is this through the simplefb that
> was
> adding for testing is the VC4 driver fully driving the display.  Is
> VC4
> still in the .config that you used? Is this a standard version of
> VC4,
> or does it include modifications for testing purposes?
> 
> If it works so well as is, why do you need or want the expander?  You
> tried to submit a very similar version a year ago?

Since Stephan seems to have taken this sudden intense interest in
vc04_services all of a sudden, perhaps now would be a excellent time to
actually acomplish one of the TODOs and get the DT bindings in for this
driver.  Since it is working an all.

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

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 06:40:21PM +0100, Philipp Zabel wrote:
> On Mon, 2017-03-20 at 14:17 +, Russell King - ARM Linux wrote:
> > I have tripped over a bug in media-ctl when specifying both a crop and
> > compose rectangle - the --help output suggests that "," should be used
> > to separate them.  media-ctl rejects that, telling me the character at
> > the "," should be "]".  Replacing the "," with " " allows media-ctl to
> > accept it and set both rectangles, so it sounds like a parser bug - I've
> > not looked into this any further yet.
> 
> I can confirm this. I don't see any place in
> v4l2_subdev_parse_pad_format that handles the "," separator. There's
> just whitespace skipping between the v4l2-properties.

Maybe this is the easiest solution:

 utils/media-ctl/options.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
index 83ca1ca..8b97874 100644
--- a/utils/media-ctl/options.c
+++ b/utils/media-ctl/options.c
@@ -65,7 +65,7 @@ static void usage(const char *argv0)
printf("\tentity  = entity-number | ( '\"' entity-name '\"' ) 
;\n");
printf("\n");
printf("\tv4l2= pad '[' v4l2-properties ']' ;\n");
-   printf("\tv4l2-properties = v4l2-property { ',' v4l2-property } ;\n");
+   printf("\tv4l2-properties = v4l2-property { ' '* v4l2-property } ;\n");
printf("\tv4l2-property   = v4l2-mbusfmt | v4l2-crop | 
v4l2-interval\n");
printf("\t| v4l2-compose | v4l2-interval ;\n");
printf("\tv4l2-mbusfmt= 'fmt:' fcc '/' size ; { 'field:' v4l2-field 
; } { 'colorspace:' v4l2-colorspace ; }\n");

;)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 4.9 52/93] x86/hyperv: Handle unknown NMIs on one CPU when unknown_nmi_panic

[Greg Kroah-Hartman]

4.9-stable review patch.  If anyone has any objections, please let me know.

--

From: Vitaly Kuznetsov 

[ Upstream commit 59107e2f48831daedc46973ce4988605ab066de3 ]

There is a feature in Hyper-V ('Debug-VM --InjectNonMaskableInterrupt')
which injects NMI to the guest. We may want to crash the guest and do kdump
on this NMI by enabling unknown_nmi_panic. To make kdump succeed we need to
allow the kdump kernel to re-establish VMBus connection so it will see
VMBus devices (storage, network,..).

To properly unload VMBus making it possible to start over during kdump we
need to do the following:

 - Send an 'unload' message to the hypervisor. This can be done on any CPU
   so we do this the crashing CPU.

 - Receive the 'unload finished' reply message. WS2012R2 delivers this
   message to the CPU which was used to establish VMBus connection during
   module load and this CPU may differ from the CPU sending 'unload'.

Receiving a VMBus message means the following:

 - There is a per-CPU slot in memory for one message. This slot can in
   theory be accessed by any CPU.

 - We get an interrupt on the CPU when a message was placed into the slot.

 - When we read the message we need to clear the slot and signal the fact
   to the hypervisor. In case there are more messages to this CPU pending
   the hypervisor will deliver the next message. The signaling is done by
   writing to an MSR so this can only be done on the appropriate CPU.

To avoid doing cross-CPU work on crash we have vmbus_wait_for_unload()
function which checks message slots for all CPUs in a loop waiting for the
'unload finished' messages. However, there is an issue which arises when
these conditions are met:

 - We're crashing on a CPU which is different from the one which was used
   to initially contact the hypervisor.

 - The CPU which was used for the initial contact is blocked with interrupts
   disabled and there is a message pending in the message slot.

In this case we won't be able to read the 'unload finished' message on the
crashing CPU. This is reproducible when we receive unknown NMIs on all CPUs
simultaneously: the first CPU entering panic() will proceed to crash and
all other CPUs will stop themselves with interrupts disabled.

The suggested solution is to handle unknown NMIs for Hyper-V guests on the
first CPU which gets them only. This will allow us to rely on VMBus
interrupt handler being able to receive the 'unload finish' message in
case it is delivered to a different CPU.

The issue is not reproducible on WS2016 as Debug-VM delivers NMI to the
boot CPU only, WS2012R2 and earlier Hyper-V versions are affected.

Signed-off-by: Vitaly Kuznetsov 
Acked-by: K. Y. Srinivasan 
Cc: de...@linuxdriverproject.org
Cc: Haiyang Zhang 
Link: http://lkml.kernel.org/r/20161202100720.28121-1-vkuzn...@redhat.com
Signed-off-by: Thomas Gleixner 
Signed-off-by: Ingo Molnar 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/x86/kernel/cpu/mshyperv.c |   24 
 1 file changed, 24 insertions(+)

--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct ms_hyperv_info ms_hyperv;
 EXPORT_SYMBOL_GPL(ms_hyperv);
@@ -158,6 +159,26 @@ static unsigned char hv_get_nmi_reason(v
return 0;
 }
 
+#ifdef CONFIG_X86_LOCAL_APIC
+/*
+ * Prior to WS2016 Debug-VM sends NMIs to all CPUs which makes
+ * it dificult to process CHANNELMSG_UNLOAD in case of crash. Handle
+ * unknown NMI on the first CPU which gets it.
+ */
+static int hv_nmi_unknown(unsigned int val, struct pt_regs *regs)
+{
+   static atomic_t nmi_cpu = ATOMIC_INIT(-1);
+
+   if (!unknown_nmi_panic)
+   return NMI_DONE;
+
+   if (atomic_cmpxchg(_cpu, -1, raw_smp_processor_id()) != -1)
+   return NMI_HANDLED;
+
+   return NMI_DONE;
+}
+#endif
+
 static void __init ms_hyperv_init_platform(void)
 {
/*
@@ -183,6 +204,9 @@ static void __init ms_hyperv_init_platfo
pr_info("HyperV: LAPIC Timer Frequency: %#x\n",
lapic_timer_frequency);
}
+
+   register_nmi_handler(NMI_UNKNOWN, hv_nmi_unknown, NMI_FLAG_FIRST,
+"hv_nmi_unknown");
 #endif
 
if (ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE)


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

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Philipp Zabel]

On Mon, 2017-03-20 at 14:17 +, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 03:00:51PM +0100, Philipp Zabel wrote:
> > On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote:
> > > The same document says:
> > > 
> > >   Scaling support is optional. When supported by a subdev, the crop
> > >   rectangle on the subdev's sink pad is scaled to the size configured
> > >   using the
> > >   :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL
> > >   using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the
> > >   subdev supports scaling but not composing, the top and left values are
> > >   not used and must always be set to zero.
> > 
> > Right, this sentence does imply that when scaling is supported, there
> > must be a sink compose rectangle, even when composing is not.
> > 
> > I have previously set up scaling like this:
> > 
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> > 
> > Does this mean, it should work like this instead?
> > 
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> > media-ctl --set-v4l2 
> > "'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]"
> > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> > 
> > I suppose setting the source pad format should not be allowed to modify
> > the sink compose rectangle.
> 
> That is what I believe having read these documents several times, but
> we need v4l2 people to confirm.
> 
> Note that setting the format on 'ipu1_csi0':0 should already be done by
> the previous media-ctl command, so it should be possible to simplify
> that to:
> 
> media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> media-ctl --set-v4l2 "'ipu1_csi0':0[compose:(0,0)/960x540]"
> media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

Thanks, that works, too.

> I have tripped over a bug in media-ctl when specifying both a crop and
> compose rectangle - the --help output suggests that "," should be used
> to separate them.  media-ctl rejects that, telling me the character at
> the "," should be "]".  Replacing the "," with " " allows media-ctl to
> accept it and set both rectangles, so it sounds like a parser bug - I've
> not looked into this any further yet.

I can confirm this. I don't see any place in
v4l2_subdev_parse_pad_format that handles the "," separator. There's
just whitespace skipping between the v4l2-properties.

regards
Philipp

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

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Mauro Carvalho Chehab]

Em Mon, 20 Mar 2017 16:10:03 +
Russell King - ARM Linux  escreveu:

> On Mon, Mar 20, 2017 at 12:39:38PM -0300, Mauro Carvalho Chehab wrote:
> > Em Mon, 20 Mar 2017 14:24:25 +0100
> > Hans Verkuil  escreveu:  
> > > I don't think this control inheritance patch will magically prevent you
> > > from needed a plugin.  
> > 
> > Yeah, it is not just control inheritance. The driver needs to work
> > without subdev API, e. g. mbus settings should also be done via the
> > video devnode.
> > 
> > Btw, Sakari made a good point on IRC: what happens if some app 
> > try to change the pipeline or subdev settings while another
> > application is using the device? The driver should block such 
> > changes, maybe using the V4L2 priority mechanism.  
> 
> My understanding is that there are already mechanisms in place to
> prevent that, but it's driver dependent - certainly several of the
> imx driver subdevs check whether they have an active stream, and
> refuse (eg) all set_fmt calls with -EBUSY if that is so.
> 
> (That statement raises another question in my mind: if the subdev is
> streaming, should it refuse all set_fmt, even for the TRY stuff?)

Returning -EBUSY only when streaming is too late, as ioctl's
may be changing the pipeline configuration and/or buffer allocation,
while the application is sending other ioctls in order to prepare
for streaming.

V4L2 has a mechanism of blocking other apps to change such
parameters via VIDIOC_S_PRIORITY[1]. If an application sets
priority to V4L2_PRIORITY_RECORD, any other application attempting
to change the device via some other file descriptor will fail.
So, it is a sort of "exclusive write access".

On a quick look at V4L2 core, currently, sending a 
VIDIOC_S_PRIORITY ioctl to a /dev/video device doesn't seem to have
any effect at either MC or V4L2 subdev API for the subdevs connected
to it. We'll likely need to add some code at v4l2_prio_change()
for it to notify the subdevs for them to return -EBUSY if one
would try to change something there, while the device is priorized.

[1] https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/vidioc-g-priority.html

> > In parallel, someone has to fix libv4l for it to be default on
> > applications like gstreamer, e. g. adding support for DMABUF
> > and fixing other issues that are preventing it to be used by
> > default.  
> 
> Hmm, not sure what you mean there - I've used dmabuf with gstreamer's
> v4l2src linked to libv4l2, importing the buffers into etnaviv using
> a custom plugin.  There are distros around (ubuntu) where the v4l2
> plugin is built against libv4l2.

Hmm... I guess some gst developer mentioned that there are/where
some restrictions at libv4l2 with regards to DMABUF. I may be
wrong.

> 
> > My understanding here is that there are developers wanting/needing
> > to have standard V4L2 apps support for (some) i.MX6 devices. Those are
> > the ones that may/will allocate some time for it to happen.  
> 
> Quite - but we need to first know what is acceptable to the v4l2
> community before we waste a lot of effort coding something up that
> may not be suitable.  Like everyone else, there's only a limited
> amount of effort available, so using it wisely is a very good idea.

Sure. That's why we're discussing here :-)

> Up until recently, it seemed that the only solution was to solve the
> userspace side of the media controller API via v4l2 plugins and the
> like.
> 
> Much of my time that I have available to look at the imx6 capture
> stuff at the moment is taken up by triping over UAPI issues with the
> current code (such as the ones about CSI scaling, colorimetry, etc)
> and trying to get concensus on what the right solution to fix those
> issues actually is, and at the moment I don't have spare time to
> start addressing any kind of v4l2 plugin for user controls nor any
> other solution.

I hear you. A solution via libv4l could be more elegant, but it
doesn't seem simple, as nobody did it before, and depends on the
libv4l plugin mechanism, with is currently unused.

Also, I think it is easier to provide a solution using DT and some
driver and/or core support for it, specially since, AFAICT,
currently there's no way request exclusive access to MC and subdevs.

It is probably not possible do to that exclusively in userspace.

> Eg, I spent much of this last weekend sorting out my IMX219 camera
> sensor driver for my new understanding about how scaling is supposed
> to work, the frame enumeration issue (which I've posted patches for)
> and the CSI scaling issue (which I've some half-baked patch for at the
> moment, but probably by the time I've finished sorting that, Philipp
> or Steve will already have a solution.)
> 
> That said, my new understanding of the scaling impacts the four patches
> I posted, and probably makes the frame size enumeration in CSI (due to
> its scaling) rather obsolete.

Yeah, when there's no scaler, it should report just the resolution(s)
supported by 

Re: [PATCH 1/6] bcm2835-gpio-exp: Driver for GPIO expander via mailbox service

[Michael Zoran]

On Mon, 2017-03-20 at 10:22 -0700, Eric Anholt wrote:
> Michael Zoran  writes:
> 
> > > > Since the API is completely documented, I see no reason we or
> > > > anybody
> > > > couldn't essentially rewrite the driver while it's in
> > > > staging.  I
> > > > just
> > > > think it would be best for everyone if the new version was a
> > > > drop
> > > > in
> > > > replacement for the original version.  Essential an enhancement
> > > > rather
> > > > then a competitor.
> > > 
> > > I think my comments weren't fundamental changes, but you surely
> > > mean
> > > the devicetree ABI? I like to see this driver ASAP out of staging
> > > and
> > > i'm not interested to maintain 2 functional identical driver only
> > > to
> > > keep compability with the Foundation tree. Currently i'm afraid
> > > that
> > > we build up many drivers in staging, which need a complete
> > > rewrite
> > > later if they should come out of staging. It would be nice if we
> > > could avoid the situation we have with the thermal driver.
> > > 
> > > Stefan
> > 
> > The API I'm talking about here is the mailbox API that is used to
> > talk
> > to the firmware.  The numbers and structures to pass are
> > documented. 
> > Nothing prevents anybody from rewriting this driver and submitting
> > it
> > to the appropriate subsystems.  It's certainly small enough.
> > 
> > If you really want working thermal or cpu speed drivers today,
> > nothing
> > stops anybody from submitting the downstream drivers after doing
> > some
> > minor touchups and submitting them to staging.  That would at least
> > get
> > things working while people argue about what the correct DT nodes
> > should be.
> > 
> > I would also like to point out that the RPI 3 has been out for over
> > a
> > year and nobody has been able to get working video out of it
> > through
> > VC4 on a mainline tree.  At least until now.  So I'm not sure the
> > best
> > way to go is for the expander driver to go under the GPIO subtree.
> 
> Excuse me?  Display works fine on my Pi3.  VC4 uses DDC to probe for
> connection when the GPIO line isn't present in the DT.

Is this arm32 or arm64 modes? And is this through the simplefb that was
adding for testing is the VC4 driver fully driving the display.  Is VC4
still in the .config that you used? Is this a standard version of VC4,
or does it include modifications for testing purposes?

If it works so well as is, why do you need or want the expander?  You
tried to submit a very similar version a year ago?

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

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Philipp Zabel]

Hi Steve, Russell,

On Mon, 2017-03-20 at 14:17 +, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 03:00:51PM +0100, Philipp Zabel wrote:
> > On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote:
> > > The same document says:
> > > 
> > >   Scaling support is optional. When supported by a subdev, the crop
> > >   rectangle on the subdev's sink pad is scaled to the size configured
> > >   using the
> > >   :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL
> > >   using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the
> > >   subdev supports scaling but not composing, the top and left values are
> > >   not used and must always be set to zero.
> > 
> > Right, this sentence does imply that when scaling is supported, there
> > must be a sink compose rectangle, even when composing is not.
> > 
> > I have previously set up scaling like this:
> > 
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> > 
> > Does this mean, it should work like this instead?
> > 
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> > media-ctl --set-v4l2 
> > "'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]"
> > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> > 
> > I suppose setting the source pad format should not be allowed to modify
> > the sink compose rectangle.
> 
> That is what I believe having read these documents several times, but
> we need v4l2 people to confirm.

What do you think of this:

--8<--
>From 2830aebc404bdfc9d7fc1ec94e5282d0b668e8f6 Mon Sep 17 00:00:00 2001
From: Philipp Zabel 
Date: Mon, 20 Mar 2017 17:10:21 +0100
Subject: [PATCH] media: imx: csi: add sink selection rectangles

Move the crop rectangle to the sink pad and add a sink compose rectangle
to configure scaling. Also propagate rectangles from sink pad to crop
rectangle, to compose rectangle, and to the source pads both in ACTIVE
and TRY variants of set_fmt/selection, and initialize the default crop
and compose rectangles.

Signed-off-by: Philipp Zabel 
---
 drivers/staging/media/imx/imx-media-csi.c | 216 +-
 1 file changed, 152 insertions(+), 64 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 560da3abdd70b..b026a5d602ddf 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -79,6 +79,7 @@ struct csi_priv {
const struct imx_media_pixfmt *cc[CSI_NUM_PADS];
struct v4l2_fract frame_interval;
struct v4l2_rect crop;
+   struct v4l2_rect compose;
const struct csi_skip_desc *skip[CSI_NUM_PADS - 1];
 
/* active vb2 buffers to send to video dev sink */
@@ -574,8 +575,8 @@ static int csi_setup(struct csi_priv *priv)
ipu_csi_set_window(priv->csi, >crop);
 
ipu_csi_set_downsize(priv->csi,
-priv->crop.width == 2 * outfmt->width,
-priv->crop.height == 2 * outfmt->height);
+priv->crop.width == 2 * priv->compose.width,
+priv->crop.height == 2 * priv->compose.height);
 
ipu_csi_init_interface(priv->csi, _mbus_cfg, _fmt);
 
@@ -1001,6 +1002,27 @@ __csi_get_fmt(struct csi_priv *priv, struct 
v4l2_subdev_pad_config *cfg,
return >format_mbus[pad];
 }
 
+static struct v4l2_rect *
+__csi_get_crop(struct csi_priv *priv, struct v4l2_subdev_pad_config *cfg,
+  enum v4l2_subdev_format_whence which)
+{
+   if (which == V4L2_SUBDEV_FORMAT_TRY)
+   return v4l2_subdev_get_try_crop(>sd, cfg, CSI_SINK_PAD);
+   else
+   return >crop;
+}
+
+static struct v4l2_rect *
+__csi_get_compose(struct csi_priv *priv, struct v4l2_subdev_pad_config *cfg,
+ enum v4l2_subdev_format_whence which)
+{
+   if (which == V4L2_SUBDEV_FORMAT_TRY)
+   return v4l2_subdev_get_try_compose(>sd, cfg,
+  CSI_SINK_PAD);
+   else
+   return >compose;
+}
+
 static void csi_try_crop(struct csi_priv *priv,
 struct v4l2_rect *crop,
 struct v4l2_subdev_pad_config *cfg,
@@ -1159,6 +1181,7 @@ static void csi_try_fmt(struct csi_priv *priv,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *sdformat,
struct v4l2_rect *crop,
+   struct v4l2_rect *compose,
const struct imx_media_pixfmt **cc)
 {
const struct imx_media_pixfmt *incc;
@@ -1173,15 +1196,8 @@ static void csi_try_fmt(struct csi_priv *priv,
incc = imx_media_find_mbus_format(infmt->code,
  CS_SEL_ANY, true);
 

Re: [PATCH 1/6] bcm2835-gpio-exp: Driver for GPIO expander via mailbox service

[Eric Anholt]

Michael Zoran  writes:

>> > Since the API is completely documented, I see no reason we or
>> > anybody
>> > couldn't essentially rewrite the driver while it's in staging.  I
>> > just
>> > think it would be best for everyone if the new version was a drop
>> > in
>> > replacement for the original version.  Essential an enhancement
>> > rather
>> > then a competitor.
>> 
>> I think my comments weren't fundamental changes, but you surely mean
>> the devicetree ABI? I like to see this driver ASAP out of staging and
>> i'm not interested to maintain 2 functional identical driver only to
>> keep compability with the Foundation tree. Currently i'm afraid that
>> we build up many drivers in staging, which need a complete rewrite
>> later if they should come out of staging. It would be nice if we
>> could avoid the situation we have with the thermal driver.
>> 
>> Stefan
>
> The API I'm talking about here is the mailbox API that is used to talk
> to the firmware.  The numbers and structures to pass are documented. 
> Nothing prevents anybody from rewriting this driver and submitting it
> to the appropriate subsystems.  It's certainly small enough.
>
> If you really want working thermal or cpu speed drivers today, nothing
> stops anybody from submitting the downstream drivers after doing some
> minor touchups and submitting them to staging.  That would at least get
> things working while people argue about what the correct DT nodes
> should be.
>
> I would also like to point out that the RPI 3 has been out for over a
> year and nobody has been able to get working video out of it through
> VC4 on a mainline tree.  At least until now.  So I'm not sure the best
> way to go is for the expander driver to go under the GPIO subtree.

Excuse me?  Display works fine on my Pi3.  VC4 uses DDC to probe for
connection when the GPIO line isn't present in the DT.


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

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 02:17:05PM +, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 03:00:51PM +0100, Philipp Zabel wrote:
> > On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote:
> > > The same document says:
> > > 
> > >   Scaling support is optional. When supported by a subdev, the crop
> > >   rectangle on the subdev's sink pad is scaled to the size configured
> > >   using the
> > >   :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL
> > >   using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the
> > >   subdev supports scaling but not composing, the top and left values are
> > >   not used and must always be set to zero.
> > 
> > Right, this sentence does imply that when scaling is supported, there
> > must be a sink compose rectangle, even when composing is not.
> > 
> > I have previously set up scaling like this:
> > 
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> > 
> > Does this mean, it should work like this instead?
> > 
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> > media-ctl --set-v4l2 
> > "'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]"
> > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> > 
> > I suppose setting the source pad format should not be allowed to modify
> > the sink compose rectangle.
> 
> That is what I believe having read these documents several times, but
> we need v4l2 people to confirm.
> 
> Note that setting the format on 'ipu1_csi0':0 should already be done by
> the previous media-ctl command, so it should be possible to simplify
> that to:
> 
> media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> media-ctl --set-v4l2 "'ipu1_csi0':0[compose:(0,0)/960x540]"
> media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

There is a slightly puzzling bit in the documentation.  If we consider
the CSI, and the sink compose rectangle size has to always match the
the source output pad format size (since in hardware they are one of
the same), then:

  Inside subdevs, the order of image processing steps will always be from
  the sink pad towards the source pad. This is also reflected in the order
  in which the configuration must be performed by the user: the changes
  made will be propagated to any subsequent stages. If this behaviour is
  not desired, the user must set ``V4L2_SEL_FLAG_KEEP_CONFIG`` flag. This
 
  flag causes no propagation of the changes are allowed in any
  
  circumstances. This may also cause the accessed rectangle to be adjusted
  
  by the driver, depending on the properties of the underlying hardware.
  ^^

presumably, this means if we get a request to change the source compose
rectangle with V4L2_SEL_FLAG_KEEP_CONFIG set, we need to force its size
to be the current output format size.  I don't think we can do anything
else - because the above makes it very clear that any following stages
shall not be updated.  The last sentence appears to give permission to
do that.

This also has implications when changing the sink crop - the sink crop
(eg) must not be smaller than the sink compose, as we don't support
scaling up in CSI.

It seems to me that V4L2_SEL_FLAG_KEEP_CONFIG in practice changes the
way validation of the request works.  So, rather than validating the
request against the upstream rectangle and propagating downstream, it
needs to be validated against both the upstream and downstream
rectangles instead.

It seems there's many subtleties to this...

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RESEND PATCH] staging: ad7606: Replace mlock with driver private lock

[Lars-Peter Clausen]

On 03/20/2017 04:22 PM, Arushi Singhal wrote:
> The IIO subsystem is redefining iio_dev->mlock to be used by
> the IIO core only for protecting device operating mode changes.
> ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
> 
> In this driver, mlock was being used to protect hardware state
> changes.  Replace it with a lock in the devices global data.
> 
> Signed-off-by: Arushi Singhal 

Hi,

Looks good, but mutex_init() is missing.

- Lars

> ---
>  drivers/staging/iio/adc/ad7606.c | 8 
>  drivers/staging/iio/adc/ad7606.h | 2 ++
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7606.c 
> b/drivers/staging/iio/adc/ad7606.c
> index 9dbfa64b1e53..9f529b34e018 100644
> --- a/drivers/staging/iio/adc/ad7606.c
> +++ b/drivers/staging/iio/adc/ad7606.c
> @@ -208,7 +208,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
>   switch (mask) {
>   case IIO_CHAN_INFO_SCALE:
>   ret = -EINVAL;
> - mutex_lock(_dev->mlock);
> + mutex_lock(>lock);
>   for (i = 0; i < ARRAY_SIZE(scale_avail); i++)
>   if (val2 == scale_avail[i][1]) {
>   gpiod_set_value(st->gpio_range, i);
> @@ -217,7 +217,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
>   ret = 0;
>   break;
>   }
> - mutex_unlock(_dev->mlock);
> + mutex_unlock(>lock);
>  
>   return ret;
>   case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> @@ -231,11 +231,11 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
>   values[1] = (ret >> 1) & 1;
>   values[2] = (ret >> 2) & 1;
>  
> - mutex_lock(_dev->mlock);
> + mutex_lock(>lock);
>   gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
> values);
>   st->oversampling = val;
> - mutex_unlock(_dev->mlock);
> + mutex_unlock(>lock);
>  
>   return 0;
>   default:
> diff --git a/drivers/staging/iio/adc/ad7606.h 
> b/drivers/staging/iio/adc/ad7606.h
> index 746f9553d2ba..5d59bdd78727 100644
> --- a/drivers/staging/iio/adc/ad7606.h
> +++ b/drivers/staging/iio/adc/ad7606.h
> @@ -14,6 +14,7 @@
>   * @name:identification string for chip
>   * @channels:channel specification
>   * @num_channels:number of channels
> + * @lock protect sensor state
>   */
>  
>  struct ad7606_chip_info {
> @@ -37,6 +38,7 @@ struct ad7606_state {
>   booldone;
>   void __iomem*base_address;
>  
> + struct mutexlock; /* protect sensor state */
>   struct gpio_desc*gpio_convst;
>   struct gpio_desc*gpio_reset;
>   struct gpio_desc*gpio_range;
> 

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

Re: [PATCH v5 00/39] i.MX Media Driver

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 05:29:07PM +0100, Philipp Zabel wrote:
> According to the documentation [1], you are doing the right thing:
> 
> The struct v4l2_subdev_frame_interval pad references a non-existing
> pad, or the pad doesn’t support frame intervals.
> 
> But v4l2_subdev_call returns -ENOIOCTLCMD if the g_frame_interval op is
> not implemented at all, which is turned into -ENOTTY by video_usercopy.
> 
> [1] 
> https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/vidioc-subdev-g-frame-interval.html#return-value

Thanks for confirming.

> > Maybe something like the following would be a better idea?
> > 
> >  utils/media-ctl/media-ctl.c | 10 +-
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
> > index f61963a..a50a559 100644
> > --- a/utils/media-ctl/media-ctl.c
> > +++ b/utils/media-ctl/media-ctl.c
> > @@ -81,22 +81,22 @@ static void v4l2_subdev_print_format(struct 
> > media_entity *entity,
> > struct v4l2_mbus_framefmt format;
> > struct v4l2_fract interval = { 0, 0 };
> > struct v4l2_rect rect;
> > -   int ret;
> > +   int ret, err_fi;
> >  
> > ret = v4l2_subdev_get_format(entity, , pad, which);
> > if (ret != 0)
> > return;
> >  
> > -   ret = v4l2_subdev_get_frame_interval(entity, , pad);
> > -   if (ret != 0 && ret != -ENOTTY)
> > -   return;
> > +   err_fi = v4l2_subdev_get_frame_interval(entity, , pad);
> 
> Not supporting frame intervals doesn't warrant a visible error message,
> I think -EINVAL should also be ignored above, if the spec is to be
> believed.
> 
> >  
> > printf("\t\t[fmt:%s/%ux%u",
> >v4l2_subdev_pixelcode_to_string(format.code),
> >format.width, format.height);
> >  
> > -   if (interval.numerator || interval.denominator)
> > +   if (err_fi == 0 && (interval.numerator || interval.denominator))
> > printf("@%u/%u", interval.numerator, interval.denominator);
> > +   else if (err_fi != -ENOTTY)
> > +   printf("@", strerror(-err_fi));
> 
> Or here.

I don't mind which - I could change this to:

else if (err_fi != -ENOTTY && err_fi != -EINVAL)

Or an alternative would be to print an error (ignoring ENOTTY and EINVAL)
to stderr at the "v4l2_subdev_get_frame_interval" callsite and continue
on (ensuring that interval is zeroed).

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v5 00/39] i.MX Media Driver

[Philipp Zabel]

On Mon, 2017-03-20 at 15:43 +, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 02:20:16PM +0100, Philipp Zabel wrote:
> > To set and read colorimetry information:
> > https://patchwork.linuxtv.org/patch/39350/
> 
> Thanks, I've applied all four of your patches, but there's a side effect
> from that.  Old media-ctl (modified by me):
> 
> - entity 53: imx219 0-0010 (2 pads, 2 links)
>  type V4L2 subdev subtype Unknown flags 0
>  device node name /dev/v4l-subdev9
> pad0: Source
> [fmt:SRGGB8/816x616 field:none
>  frame_interval:1/25]
> -> "imx6-mipi-csi2":0 [ENABLED]
> pad1: Sink
> [fmt:SRGGB10/3280x2464 field:none
>  crop.bounds:(0,0)/3280x2464
>  crop:(0,0)/3264x2464
>  compose.bounds:(0,0)/3264x2464
>  compose:(0,0)/816x616]
> <- "imx219 pixel 0-0010":0 [ENABLED,IMMUTABLE]
> 
> New media-ctl:
> 
> - entity 53: imx219 0-0010 (2 pads, 2 links)
>  type V4L2 subdev subtype Unknown flags 0
>  device node name /dev/v4l-subdev9
> pad0: Source
> [fmt:SRGGB8_1X8/816x616@1/25 field:none colorspace:srgb 
> xfer:srgb]
> -> "imx6-mipi-csi2":0 [ENABLED]
> pad1: Sink
> <- "imx219 pixel 0-0010":0 [ENABLED,IMMUTABLE]
> 
> It looks like we successfully retrieve the frame interval for pad 0
> and print it, but when we try to retrieve the frame interval for pad 1,
> we get EINVAL (because that's what I'm returning, but I'm wondering if
> that's the correct thing to do...) and that prevents _all_ format
> information being output.

According to the documentation [1], you are doing the right thing:

The struct v4l2_subdev_frame_interval pad references a non-existing
pad, or the pad doesn’t support frame intervals.

But v4l2_subdev_call returns -ENOIOCTLCMD if the g_frame_interval op is
not implemented at all, which is turned into -ENOTTY by video_usercopy.

[1] 
https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/vidioc-subdev-g-frame-interval.html#return-value

> Maybe something like the following would be a better idea?
> 
>  utils/media-ctl/media-ctl.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
> index f61963a..a50a559 100644
> --- a/utils/media-ctl/media-ctl.c
> +++ b/utils/media-ctl/media-ctl.c
> @@ -81,22 +81,22 @@ static void v4l2_subdev_print_format(struct media_entity 
> *entity,
>   struct v4l2_mbus_framefmt format;
>   struct v4l2_fract interval = { 0, 0 };
>   struct v4l2_rect rect;
> - int ret;
> + int ret, err_fi;
>  
>   ret = v4l2_subdev_get_format(entity, , pad, which);
>   if (ret != 0)
>   return;
>  
> - ret = v4l2_subdev_get_frame_interval(entity, , pad);
> - if (ret != 0 && ret != -ENOTTY)
> - return;
> + err_fi = v4l2_subdev_get_frame_interval(entity, , pad);

Not supporting frame intervals doesn't warrant a visible error message,
I think -EINVAL should also be ignored above, if the spec is to be
believed.

>  
>   printf("\t\t[fmt:%s/%ux%u",
>  v4l2_subdev_pixelcode_to_string(format.code),
>  format.width, format.height);
>  
> - if (interval.numerator || interval.denominator)
> + if (err_fi == 0 && (interval.numerator || interval.denominator))
>   printf("@%u/%u", interval.numerator, interval.denominator);
> + else if (err_fi != -ENOTTY)
> + printf("@", strerror(-err_fi));

Or here.

>  
>   if (format.field)
>   printf(" field:%s", v4l2_subdev_field_to_string(format.field));
> 
> 

regards
Philipp

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

Re: [PATCH 3/9] stating/atomisp: fix -Wold-style-definition warning

[Arnd Bergmann]

On Mon, Mar 20, 2017 at 4:05 PM, Stephen Hemminger
 wrote:
> On Mon, 20 Mar 2017 10:32:19 +0100
> Arnd Bergmann  wrote:
>
>> -void ia_css_dequeue_param_buffers(/*unsigned int pipe_num*/)
>> +void ia_css_dequeue_param_buffers(/*unsigned int pipe_num*/ void)
>>  {
> Why keep the comment?

The comment matches one later in the file when this function gets called.

I thought about cleaning up both at the same time, but couldn't figure out
how the comment ended up in there or why it was left behind in the first
place, so I ended up leaving both for another patch on top. If you prefer,
I could resend the patch and do both at once.

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

Re: outreachy

[Pavel Machek]

On Mon 2017-03-20 11:30:08, Greg KH wrote:
> On Mon, Mar 20, 2017 at 11:20:32AM +0100, Pavel Machek wrote:
> > Hi!
> > 
> > > > > Hah!  That's the joy of being a maintainer of a driver in staging.  
> > > > > Even
> > > > > if you filter out outreachy, you are going to get a lot of "basic
> > > > > mistakes" and other type patches cc:ed to you.
> > > > >
> > > > > I strongly suggest, that if you all don't like this type of stuff,
> > > > > either:
> > > > >   - work to get the code out of staging as soon as possible (i.e.
> > > > > send me coding style fixes for everything right now, and then
> > > > > fix up the rest of the stuff.)
> > > > >   - take yourself off the maintainer list for this code.
> > > > >
> > > > > It's your choice, outreachy right now is a lot of patches, but again,
> > > > > it's not going to keep you from getting the "basic" stuff sent to you
> > > > > in ways that is totally wrong.
> > > >
> > > > Could we get these trivial patches off the lkml? Yes, lkml already has
> > > > a lot of traffic, but no, this is not useful :-(.
> > > 
> > > The outreachy instructions say to use the -nol argument to
> > > get_maintainers, which would prevent them from being sent to any mailing
> > > list.  However others thought that all patches should be sent to mailing
> > > lists, and so I haven't enforced anything for people who have omitted
> > > -nol.  However I have tried to remove bcm maintainers from CC lists on
> > > replies and reminded people not to send you patches,
> > 
> > Wonderful :-(.
> > 
> > Can we at least make  those people put the word "outreachy" in the
> > subject so the emails are easier to delete?
> 
> It's easy for you to filter away as-is if you want to to by just looking
> at the cc: list for outreachy right now, don't make a new step for
> people to jump through because you don't want to be bothered.  You only
> have 10 more days of it if you want to just ignore any patch until
> then...

10 more days... I can survive 10 more days. I was afraid we would be
stuck with it "forever".
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


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

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 12:39:38PM -0300, Mauro Carvalho Chehab wrote:
> Em Mon, 20 Mar 2017 14:24:25 +0100
> Hans Verkuil  escreveu:
> > I don't think this control inheritance patch will magically prevent you
> > from needed a plugin.
> 
> Yeah, it is not just control inheritance. The driver needs to work
> without subdev API, e. g. mbus settings should also be done via the
> video devnode.
> 
> Btw, Sakari made a good point on IRC: what happens if some app 
> try to change the pipeline or subdev settings while another
> application is using the device? The driver should block such 
> changes, maybe using the V4L2 priority mechanism.

My understanding is that there are already mechanisms in place to
prevent that, but it's driver dependent - certainly several of the
imx driver subdevs check whether they have an active stream, and
refuse (eg) all set_fmt calls with -EBUSY if that is so.

(That statement raises another question in my mind: if the subdev is
streaming, should it refuse all set_fmt, even for the TRY stuff?)

> In parallel, someone has to fix libv4l for it to be default on
> applications like gstreamer, e. g. adding support for DMABUF
> and fixing other issues that are preventing it to be used by
> default.

Hmm, not sure what you mean there - I've used dmabuf with gstreamer's
v4l2src linked to libv4l2, importing the buffers into etnaviv using
a custom plugin.  There are distros around (ubuntu) where the v4l2
plugin is built against libv4l2.

> My understanding here is that there are developers wanting/needing
> to have standard V4L2 apps support for (some) i.MX6 devices. Those are
> the ones that may/will allocate some time for it to happen.

Quite - but we need to first know what is acceptable to the v4l2
community before we waste a lot of effort coding something up that
may not be suitable.  Like everyone else, there's only a limited
amount of effort available, so using it wisely is a very good idea.

Up until recently, it seemed that the only solution was to solve the
userspace side of the media controller API via v4l2 plugins and the
like.

Much of my time that I have available to look at the imx6 capture
stuff at the moment is taken up by triping over UAPI issues with the
current code (such as the ones about CSI scaling, colorimetry, etc)
and trying to get concensus on what the right solution to fix those
issues actually is, and at the moment I don't have spare time to
start addressing any kind of v4l2 plugin for user controls nor any
other solution.

Eg, I spent much of this last weekend sorting out my IMX219 camera
sensor driver for my new understanding about how scaling is supposed
to work, the frame enumeration issue (which I've posted patches for)
and the CSI scaling issue (which I've some half-baked patch for at the
moment, but probably by the time I've finished sorting that, Philipp
or Steve will already have a solution.)

That said, my new understanding of the scaling impacts the four patches
I posted, and probably makes the frame size enumeration in CSI (due to
its scaling) rather obsolete.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: ad7759: Replace mlock with driver private lock

[Arushi Singhal]

The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.

In this driver, mlock was being used to protect hardware state
changes.  Replace it with a lock in the devices global data.

Signed-off-by: Arushi Singhal 
---
 drivers/staging/iio/meter/ade7759.c | 4 ++--
 drivers/staging/iio/meter/ade7759.h | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7759.c 
b/drivers/staging/iio/meter/ade7759.c
index 944ee3401029..1d1e0b33021f 100644
--- a/drivers/staging/iio/meter/ade7759.c
+++ b/drivers/staging/iio/meter/ade7759.c
@@ -381,7 +381,7 @@ static ssize_t ade7759_write_frequency(struct device *dev,
if (!val)
return -EINVAL;
 
-   mutex_lock(_dev->mlock);
+   mutex_lock(>buf_lock);
 
t = 27900 / val;
if (t > 0)
@@ -402,7 +402,7 @@ static ssize_t ade7759_write_frequency(struct device *dev,
ret = ade7759_spi_write_reg_16(dev, ADE7759_MODE, reg);
 
 out:
-   mutex_unlock(_dev->mlock);
+   mutex_unlock(>buf_lock);
 
return ret ? ret : len;
 }
diff --git a/drivers/staging/iio/meter/ade7759.h 
b/drivers/staging/iio/meter/ade7759.h
index f0716d2fdf8e..4f69bb93cc45 100644
--- a/drivers/staging/iio/meter/ade7759.h
+++ b/drivers/staging/iio/meter/ade7759.h
@@ -42,6 +42,7 @@
  * @buf_lock:  mutex to protect tx and rx
  * @tx:transmit buffer
  * @rx:receive buffer
+ * @lock   protect sensor state
  **/
 struct ade7759_state {
struct spi_device   *us;
-- 
2.11.0

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

Re: [PATCH v5 00/39] i.MX Media Driver

[Hans Verkuil]

On 03/20/2017 03:11 PM, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 02:57:03PM +0100, Hans Verkuil wrote:
>> On 03/20/2017 02:29 PM, Russell King - ARM Linux wrote:
>>> It's what I have - remember, not everyone is happy to constantly replace
>>> their distro packages with random new stuff.
>>
>> This is a compliance test, which is continuously developed in tandem with
>> new kernel versions. If you are working with an upstream kernel, then you
>> should also use the corresponding v4l2-compliance test. What's the point
>> of using an old one?
>>
>> I will not support driver developers that use an old version of the
>> compliance test, that's a waste of my time.
> 
> The reason that people may _not_ wish to constantly update v4l-utils
> is that it changes the libraries installed on their systems.
> 
> So, the solution to that is not to complain about developers not using
> the latest version, but instead to de-couple it from the rest of the
> package, and provide it as a separate, stand-alone package that doesn't
> come with all the extra baggage.
> 
> Now, there's two possible answers to that:
> 
> 1. it depends on the libv4l2 version.  If that's so, then you are
>insisting that people constantly move to the latest libv4l2 because
>of API changes, and those API changes may upset applications they're
>using.  So this isn't really on.
> 
> 2. it doesn't depend on libv4l2 version, in which case there's no reason
>for it to be packaged with v4l-utils.

Run configure with --disable-v4l2-compliance-libv4l.

This avoids linking with libv4l and allows you to build it stand-alone.

Perhaps I should invert this option since in most cases you don't want to
run v4l2-compliance with libv4l (it's off by default unless you pass the
-w option to v4l2-compliance).

> 
> The reality is that v4l2-compliance links with libv4l2, so I'm not sure
> which it is.  What I am sure of is that I don't want to upgrade libv4l2
> on an ad-hoc basis, potentially causing issues with applications.
> 
 To test actual streaming you need to provide the -s option.

 Note: v4l2-compliance has been developed for 'regular' video devices,
 not MC devices. It may or may not work with the -s option.
>>>
>>> Right, and it exists to verify that the establised v4l2 API is correctly
>>> implemented.  If the v4l2 API is being offered to user applications,
>>> then it must be conformant, otherwise it's not offering the v4l2 API.
>>> (That's very much a definition statement in itself.)
>>>
>>> So, are we really going to say MC devices do not offer the v4l2 API to
>>> userspace, but something that might work?  We've already seen today
>>> one user say that they're not going to use mainline because of the
>>> crud surrounding MC.
>>>
>>
>> Actually, my understanding was that he was stuck on the old kernel code.
> 
> Err, sorry, I really don't follow.  Who is "he"?

"one user say that they're not going to use mainline because of the
crud surrounding MC."

> 
> _I_ was the one who reported the EXPBUF problem.  Your comment makes no
> sense.
> 
>> In the case of v4l2-compliance, I never had the time to make it work with
>> MC devices. Same for that matter of certain memory to memory devices.
>>
>> Just like MC devices these too behave differently. They are partially
>> supported in v4l2-compliance, but not fully.
> 
> It seems you saying that the API provided by /dev/video* for a MC device
> breaks the v4l2-compliance tests?

There may be tests in the compliance suite that do not apply for MC devices
and for which I never check. The compliance suite was never written with MC
devices in mind, and it certainly hasn't been tested much with such devices.

It's only very recent that I even got hardware that has MC support...

>From what I can tell from the feedback I got it seems to be OKish, but I
just can't guarantee that the compliance utility is correct for such devices.

In particular I doubt the streaming tests (-s, -f, etc.) will work. The -s
test *might* work if the pipeline is configured correctly before running
v4l2-compliance. I can't imagine that the -f option would work at all since
I would expect pipeline validation errors.

I've been gently pushing Helen Koike to finish her virtual MC driver
(https://patchwork.kernel.org/patch/9312783/) since having a virtual driver
makes writing compliance tests much easier.

> _No one_ has mentioned using v4l2-compliance on the subdevs.
> 
>> Complaining about this really won't help. We know it's a problem and unless
>> someone (me perhaps?) manages to get paid to work on this it's unlikely to
>> change for now.
> 
> Like the above comment, your comment makes no sense.  I'm not complaining,
> I'm trying to find out the details.

Must be me then, it did feel like complaining...

> Yes, MC stuff sucks big time right now, the documentation is poor, there's
> a lack of understanding on all sides of the issues (which can be seen by
> the different opinions that people hold.)  

Re: [PATCH v5 00/39] i.MX Media Driver

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 02:20:16PM +0100, Philipp Zabel wrote:
> To set and read colorimetry information:
> https://patchwork.linuxtv.org/patch/39350/

Thanks, I've applied all four of your patches, but there's a side effect
from that.  Old media-ctl (modified by me):

- entity 53: imx219 0-0010 (2 pads, 2 links)
 type V4L2 subdev subtype Unknown flags 0
 device node name /dev/v4l-subdev9
pad0: Source
[fmt:SRGGB8/816x616 field:none
 frame_interval:1/25]
-> "imx6-mipi-csi2":0 [ENABLED]
pad1: Sink
[fmt:SRGGB10/3280x2464 field:none
 crop.bounds:(0,0)/3280x2464
 crop:(0,0)/3264x2464
 compose.bounds:(0,0)/3264x2464
 compose:(0,0)/816x616]
<- "imx219 pixel 0-0010":0 [ENABLED,IMMUTABLE]

New media-ctl:

- entity 53: imx219 0-0010 (2 pads, 2 links)
 type V4L2 subdev subtype Unknown flags 0
 device node name /dev/v4l-subdev9
pad0: Source
[fmt:SRGGB8_1X8/816x616@1/25 field:none colorspace:srgb 
xfer:srgb]
-> "imx6-mipi-csi2":0 [ENABLED]
pad1: Sink
<- "imx219 pixel 0-0010":0 [ENABLED,IMMUTABLE]

It looks like we successfully retrieve the frame interval for pad 0
and print it, but when we try to retrieve the frame interval for pad 1,
we get EINVAL (because that's what I'm returning, but I'm wondering if
that's the correct thing to do...) and that prevents _all_ format
information being output.

Maybe something like the following would be a better idea?

 utils/media-ctl/media-ctl.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
index f61963a..a50a559 100644
--- a/utils/media-ctl/media-ctl.c
+++ b/utils/media-ctl/media-ctl.c
@@ -81,22 +81,22 @@ static void v4l2_subdev_print_format(struct media_entity 
*entity,
struct v4l2_mbus_framefmt format;
struct v4l2_fract interval = { 0, 0 };
struct v4l2_rect rect;
-   int ret;
+   int ret, err_fi;
 
ret = v4l2_subdev_get_format(entity, , pad, which);
if (ret != 0)
return;
 
-   ret = v4l2_subdev_get_frame_interval(entity, , pad);
-   if (ret != 0 && ret != -ENOTTY)
-   return;
+   err_fi = v4l2_subdev_get_frame_interval(entity, , pad);
 
printf("\t\t[fmt:%s/%ux%u",
   v4l2_subdev_pixelcode_to_string(format.code),
   format.width, format.height);
 
-   if (interval.numerator || interval.denominator)
+   if (err_fi == 0 && (interval.numerator || interval.denominator))
printf("@%u/%u", interval.numerator, interval.denominator);
+   else if (err_fi != -ENOTTY)
+   printf("@", strerror(-err_fi));
 
if (format.field)
printf(" field:%s", v4l2_subdev_field_to_string(format.field));


-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver

[Michael Zoran]

On Mon, 2017-03-20 at 12:33 -0300, Mauro Carvalho Chehab wrote:
> Em Mon, 20 Mar 2017 08:11:41 -0700
> Michael Zoran  escreveu:
> 
> > On Mon, 2017-03-20 at 11:58 -0300, Mauro Carvalho Chehab wrote:
> > > Em Mon, 20 Mar 2017 04:08:21 -0700
> > > Michael Zoran  escreveu:
> > >   
> > > > On Mon, 2017-03-20 at 07:58 -0300, Mauro Carvalho Chehab
> > > > wrote:  
> > > > > Em Sun, 19 Mar 2017 22:11:07 -0300
> > > > > Mauro Carvalho Chehab  escreveu:
> > > > > 
> > > > > > Em Sun, 19 Mar 2017 10:04:28 -0700
> > > > > > Michael Zoran  escreveu:
> > > > > > 
> > > > > > > A working DT that I tried this morning with the current
> > > > > > > firmware
> > > > > > > is
> > > > > > > posted here:
> > > > > > > http://lists.infradead.org/pipermail/linux-rpi-kernel/201
> > > > > > > 7-Ma
> > > > > > > rch/
> > > > > > > 005924
> > > > > > > .html
> > > > > > > 
> > > > > > > It even works with minecraft_pi!  
> > > > > 
> > > > > 
> > > > 
> > > > Hi, can you e-mail out your config.txt?  Do you have audio
> > > > enabled
> > > > in
> > > > config.txt?  
> > > 
> > > yes, I have this:
> > > 
> > > $ cat config.txt |grep -i audio
> > > # uncomment to force a HDMI mode rather than DVI. This can make
> > > audio
> > > work in
> > > # Enable audio (loads snd_bcm2835)
> > > dtparam=audio=on
> > > 
> > > Full config attached.
> > > 
> > > Thanks,
> > > Mauro
> > >   
> > 
> > Are you using Eric Anholt's HDMI Audio driver that's included in
> > VC4? 
> > That could well be incompatible with the firmware driver. Or are
> > you
> > using a half mode of VC4 for audio and VCHIQ for video?
> 
> I'm using vanilla staging Kernel, from Greg's tree:
>   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.
> git/commit/?h=staging-
> next=7bc49cb9b9b8bad32536c4b6d1aff1824c1adc6c
> 
> Plus the DWC2 fixup I wrote and DT changes you pointed
> (see enclosed).
> 
> I can disable the audio overlay here, as I don't have anything 
> connected to audio inputs/outputs.
> 
> Regards,
> Mauro
> 

Why is the vchiq node in the tree twice? For me to even respond anymore
you you going to have to include your entire dtb(whatever you are
using) run through dtc -I dtb -O dts.  You are also going to have to
include your exact .config file you used for building, and exactly what
these DWC2 fixeups are.

You don't even state exactly what platform you are using, Is it even an
RPI of some kind.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Mauro Carvalho Chehab]

Em Mon, 20 Mar 2017 14:24:25 +0100
Hans Verkuil  escreveu:

> On 03/14/2017 11:21 AM, Mauro Carvalho Chehab wrote:
> > Em Tue, 14 Mar 2017 08:55:36 +0100
> > Hans Verkuil  escreveu:
> >   
> >> On 03/14/2017 04:45 AM, Mauro Carvalho Chehab wrote:  
> >>> Hi Sakari,
> >>>

> >> We're all very driver-development-driven, and userspace gets very little
> >> attention in general. So before just throwing in the towel we should take
> >> a good look at the reasons why there has been little or no development: is
> >> it because of fundamental design defects, or because nobody paid attention
> >> to it?  
> > 
> > No. We should look it the other way: basically, there are patches
> > for i.MX6 driver that sends control from videonode to subdevs. 
> > 
> > If we nack apply it, who will write the userspace plugin? When
> > such change will be merged upstream?
> > 
> > If we don't have answers to any of the above questions, we should not
> > nack it.
> > 
> > That's said, that doesn't prevent merging a libv4l plugin if/when
> > someone can find time/interest to develop it.  
> 
> I don't think this control inheritance patch will magically prevent you
> from needed a plugin.

Yeah, it is not just control inheritance. The driver needs to work
without subdev API, e. g. mbus settings should also be done via the
video devnode.

Btw, Sakari made a good point on IRC: what happens if some app 
try to change the pipeline or subdev settings while another
application is using the device? The driver should block such 
changes, maybe using the V4L2 priority mechanism.

> This is literally the first time we have to cater to a cheap devkit. We
> were always aware of this issue, but nobody really needed it.

That's true. Now that we have a real need for that, we need to
provide a solution.

> > I'd say that we should not care anymore on providing a solution for
> > generic applications to run on boards like OMAP3[1]. For hardware that
> > are currently available that have Kernel driver and boards developed
> > to be used as "cheap hobbyist devkit", I'd say we should implement
> > a Kernel solution that would allow them to be used without subdev
> > API, e. g. having all ioctls needed by generic applications to work
> > functional, after some external application sets the pipeline.  
> 
> I liked Russell's idea of having the DT set up an initial video path.
> This would (probably) make it much easier to provide a generic plugin since
> there is already an initial valid path when the driver is loaded, and it
> doesn't require custom code in the driver since this is left to the DT
> which really knows about the HW.

Setting the device via DT indeed makes easier (either for a kernel
or userspace solution), but things like resolution changes should
be possible without needing to change DT.

Also, as MC and subdev changes should be blocked while a V4L2 app
is using the device, using a mechanism like calling VIDIOC_S_PRIORITY
ioctl via the V4l2 interface, Kernel will require changes, anyway.

My suggestion is to touch on one driver to make it work with a
generic application. As we currently have efforts and needs for
the i.MX6 to do it, I'd say that the best would be to make it
work on such driver. Once the work is done, we can see if the
approach taken there would work at libv4l or not.

In parallel, someone has to fix libv4l for it to be default on
applications like gstreamer, e. g. adding support for DMABUF
and fixing other issues that are preventing it to be used by
default.

Nicolas,

Why libv4l is currently disabled at gstreamer's default settings?

> > [1] Yet, I might eventually do that for fun, an OMAP3 board with tvp5150
> > just arrived here last week. It would be nice to have xawtv3 running on it 
> > :-)
> > So, if I have a lot of spare time (with is very unlikely), I might 
> > eventually 
> > do something for it to work.
> >   
> >> I know it took me a very long time before I had a working omap3.  
> > 
> > My first OMAP3 board with working V4L2 source just arrived last week :-)
> >   
> >> So I am not at all surprised that little progress has been made.  
> > 
> > I'm not surprised, but I'm disappointed, as I tried to push toward a
> > solution for this problem since when we had our initial meetings about
> > it.  
> 
> So many things to do, so little time. Sounds corny, but really, that's what
> this is all about. There were always (and frankly, still are) more important
> things that needed to be done.

What's most important for some developer may not be so important for
another developer.

My understanding here is that there are developers wanting/needing
to have standard V4L2 apps support for (some) i.MX6 devices. Those are
the ones that may/will allocate some time for it to happen.

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

Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver

[Mauro Carvalho Chehab]

Em Mon, 20 Mar 2017 08:11:41 -0700
Michael Zoran  escreveu:

> On Mon, 2017-03-20 at 11:58 -0300, Mauro Carvalho Chehab wrote:
> > Em Mon, 20 Mar 2017 04:08:21 -0700
> > Michael Zoran  escreveu:
> >   
> > > On Mon, 2017-03-20 at 07:58 -0300, Mauro Carvalho Chehab wrote:  
> > > > Em Sun, 19 Mar 2017 22:11:07 -0300
> > > > Mauro Carvalho Chehab  escreveu:
> > > >     
> > > > > Em Sun, 19 Mar 2017 10:04:28 -0700
> > > > > Michael Zoran  escreveu:
> > > > >     
> > > > > > A working DT that I tried this morning with the current
> > > > > > firmware
> > > > > > is
> > > > > > posted here:
> > > > > > http://lists.infradead.org/pipermail/linux-rpi-kernel/2017-Ma
> > > > > > rch/
> > > > > > 005924
> > > > > > .html
> > > > > > 
> > > > > > It even works with minecraft_pi!  
> > > > 
> > > >     
> > > 
> > > Hi, can you e-mail out your config.txt?  Do you have audio enabled
> > > in
> > > config.txt?  
> > 
> > yes, I have this:
> > 
> > $ cat config.txt |grep -i audio
> > # uncomment to force a HDMI mode rather than DVI. This can make audio
> > work in
> > # Enable audio (loads snd_bcm2835)
> > dtparam=audio=on
> > 
> > Full config attached.
> > 
> > Thanks,
> > Mauro
> >   
> 
> Are you using Eric Anholt's HDMI Audio driver that's included in VC4? 
> That could well be incompatible with the firmware driver. Or are you
> using a half mode of VC4 for audio and VCHIQ for video?

I'm using vanilla staging Kernel, from Greg's tree:

https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?h=staging-next=7bc49cb9b9b8bad32536c4b6d1aff1824c1adc6c

Plus the DWC2 fixup I wrote and DT changes you pointed
(see enclosed).

I can disable the audio overlay here, as I don't have anything 
connected to audio inputs/outputs.

Regards,
Mauro

---


diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi 
b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index 38e6050035bc..1f42190e8558 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -27,6 +27,14 @@
firmware = <>;
#power-domain-cells = <1>;
};
+
+   vchiq {
+   compatible = "brcm,bcm2835-vchiq";
+   reg = <0x7e00b840 0xf>;
+   interrupts = <0 2>;
+   cache-line-size = <32>;
+   firmware = <>;
+   };
};
 };

diff --git a/arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts 
b/arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts
index c309633a1e87..7e8d42904022 100644
--- a/arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts
+++ b/arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts
@@ -17,6 +17,45 @@
gpios = < 47 0>;
};
};
+
+
+   soc {
+
+// hvs at 7e40 {
+// status = "disabled";
+// };
+
+// v3d: v3d at 7ec0 {
+// status = "disabled";
+// };
+
+   vc4: gpu {
+   status = "disabled";
+   };
+
+   fb: fb {
+   status = "disabled";
+   };
+
+   vchiq: vchiq {
+   compatible = "brcm,bcm2835-vchiq";
+   reg = <0x7e00b840 0xf>;
+   interrupts = <0 2>;
+   cache-line-size = <32>;
+   firmware = <>;
+   };
+
+   audio: audio {
+   compatible = "brcm,bcm2835-audio";
+   brcm,pwm-channels = <8>;
+   };
+
+   };
+
+   __overrides__ {
+   cache_line_size = <>, "cache-line-size:0";
+   };
+
 };
 

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

[RESEND PATCH] staging: ad7606: Replace mlock with driver private lock

[Arushi Singhal]

The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.

In this driver, mlock was being used to protect hardware state
changes.  Replace it with a lock in the devices global data.

Signed-off-by: Arushi Singhal 
---
 drivers/staging/iio/adc/ad7606.c | 8 
 drivers/staging/iio/adc/ad7606.h | 2 ++
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 9dbfa64b1e53..9f529b34e018 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -208,7 +208,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_SCALE:
ret = -EINVAL;
-   mutex_lock(_dev->mlock);
+   mutex_lock(>lock);
for (i = 0; i < ARRAY_SIZE(scale_avail); i++)
if (val2 == scale_avail[i][1]) {
gpiod_set_value(st->gpio_range, i);
@@ -217,7 +217,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
ret = 0;
break;
}
-   mutex_unlock(_dev->mlock);
+   mutex_unlock(>lock);
 
return ret;
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
@@ -231,11 +231,11 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
values[1] = (ret >> 1) & 1;
values[2] = (ret >> 2) & 1;
 
-   mutex_lock(_dev->mlock);
+   mutex_lock(>lock);
gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
  values);
st->oversampling = val;
-   mutex_unlock(_dev->mlock);
+   mutex_unlock(>lock);
 
return 0;
default:
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 746f9553d2ba..5d59bdd78727 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -14,6 +14,7 @@
  * @name:  identification string for chip
  * @channels:  channel specification
  * @num_channels:  number of channels
+ * @lock   protect sensor state
  */
 
 struct ad7606_chip_info {
@@ -37,6 +38,7 @@ struct ad7606_state {
booldone;
void __iomem*base_address;
 
+   struct mutexlock; /* protect sensor state */
struct gpio_desc*gpio_convst;
struct gpio_desc*gpio_reset;
struct gpio_desc*gpio_range;
-- 
2.11.0

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

[PATCH 2/2] staging: ade7754: Clean up #includes

[simran singhal]

Alphabetize and separate kernel and subsystem headers.

Signed-off-by: simran singhal 
---
 drivers/staging/iio/meter/ade7754.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7754.c 
b/drivers/staging/iio/meter/ade7754.c
index 42f7b06..fea93d7 100644
--- a/drivers/staging/iio/meter/ade7754.c
+++ b/drivers/staging/iio/meter/ade7754.c
@@ -6,18 +6,17 @@
  * Licensed under the GPL-2 or later.
  */
 
-#include 
-#include 
 #include 
-#include 
 #include 
+#include 
+#include 
 #include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
-#include 
-#include 
-
 #include 
 #include 
 #include "meter.h"
-- 
2.7.4

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

[PATCH 0/2] staging: iio: ade7754: Header file maintenance

[simran singhal]

Collapse header file into source, and clean up includes in 
implementation.

simran singhal (2):
  staging: ade7754: Move header content to implementation file
  staging: ade7754: Clean up #includes

 drivers/staging/iio/meter/ade7754.c | 98 ++---
 drivers/staging/iio/meter/ade7754.h | 90 --
 2 files changed, 91 insertions(+), 97 deletions(-)
 delete mode 100644 drivers/staging/iio/meter/ade7754.h

-- 
2.7.4

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

[PATCH 1/2] staging: ade7754: Move header content to implementation file

[simran singhal]

The contents of ade7754.h are only used in ade7754.c.
Move the header contents to the implementation file,
and delete the header file.

Signed-off-by: simran singhal 
---
 drivers/staging/iio/meter/ade7754.c | 87 ++-
 drivers/staging/iio/meter/ade7754.h | 90 -
 2 files changed, 86 insertions(+), 91 deletions(-)
 delete mode 100644 drivers/staging/iio/meter/ade7754.h

diff --git a/drivers/staging/iio/meter/ade7754.c 
b/drivers/staging/iio/meter/ade7754.c
index 024463a..42f7b06 100644
--- a/drivers/staging/iio/meter/ade7754.c
+++ b/drivers/staging/iio/meter/ade7754.c
@@ -21,7 +21,92 @@
 #include 
 #include 
 #include "meter.h"
-#include "ade7754.h"
+
+#define ADE7754_AENERGY   0x01
+#define ADE7754_RAENERGY  0x02
+#define ADE7754_LAENERGY  0x03
+#define ADE7754_VAENERGY  0x04
+#define ADE7754_RVAENERGY 0x05
+#define ADE7754_LVAENERGY 0x06
+#define ADE7754_PERIOD0x07
+#define ADE7754_TEMP  0x08
+#define ADE7754_WFORM 0x09
+#define ADE7754_OPMODE0x0A
+#define ADE7754_MMODE 0x0B
+#define ADE7754_WAVMODE   0x0C
+#define ADE7754_WATMODE   0x0D
+#define ADE7754_VAMODE0x0E
+#define ADE7754_IRQEN 0x0F
+#define ADE7754_STATUS0x10
+#define ADE7754_RSTATUS   0x11
+#define ADE7754_ZXTOUT0x12
+#define ADE7754_LINCYC0x13
+#define ADE7754_SAGCYC0x14
+#define ADE7754_SAGLVL0x15
+#define ADE7754_VPEAK 0x16
+#define ADE7754_IPEAK 0x17
+#define ADE7754_GAIN  0x18
+#define ADE7754_AWG   0x19
+#define ADE7754_BWG   0x1A
+#define ADE7754_CWG   0x1B
+#define ADE7754_AVAG  0x1C
+#define ADE7754_BVAG  0x1D
+#define ADE7754_CVAG  0x1E
+#define ADE7754_APHCAL0x1F
+#define ADE7754_BPHCAL0x20
+#define ADE7754_CPHCAL0x21
+#define ADE7754_AAPOS 0x22
+#define ADE7754_BAPOS 0x23
+#define ADE7754_CAPOS 0x24
+#define ADE7754_CFNUM 0x25
+#define ADE7754_CFDEN 0x26
+#define ADE7754_WDIV  0x27
+#define ADE7754_VADIV 0x28
+#define ADE7754_AIRMS 0x29
+#define ADE7754_BIRMS 0x2A
+#define ADE7754_CIRMS 0x2B
+#define ADE7754_AVRMS 0x2C
+#define ADE7754_BVRMS 0x2D
+#define ADE7754_CVRMS 0x2E
+#define ADE7754_AIRMSOS   0x2F
+#define ADE7754_BIRMSOS   0x30
+#define ADE7754_CIRMSOS   0x31
+#define ADE7754_AVRMSOS   0x32
+#define ADE7754_BVRMSOS   0x33
+#define ADE7754_CVRMSOS   0x34
+#define ADE7754_AAPGAIN   0x35
+#define ADE7754_BAPGAIN   0x36
+#define ADE7754_CAPGAIN   0x37
+#define ADE7754_AVGAIN0x38
+#define ADE7754_BVGAIN0x39
+#define ADE7754_CVGAIN0x3A
+#define ADE7754_CHKSUM0x3E
+#define ADE7754_VERSION   0x3F
+
+#define ADE7754_READ_REG(a)a
+#define ADE7754_WRITE_REG(a) ((a) | 0x80)
+
+#define ADE7754_MAX_TX4
+#define ADE7754_MAX_RX4
+#define ADE7754_STARTUP_DELAY 1000
+
+#define ADE7754_SPI_SLOW   (u32)(300 * 1000)
+#define ADE7754_SPI_BURST  (u32)(1000 * 1000)
+#define ADE7754_SPI_FAST   (u32)(2000 * 1000)
+
+/**
+ * struct ade7754_state - device instance specific data
+ * @us:actual spi_device
+ * @buf_lock:  mutex to protect tx and rx
+ * @tx:transmit buffer
+ * @rx:receive buffer
+ **/
+struct ade7754_state {
+   struct spi_device   *us;
+   struct mutexbuf_lock;
+   u8  tx[ADE7754_MAX_TX] cacheline_aligned;
+   u8  rx[ADE7754_MAX_RX];
+};
 
 static int ade7754_spi_write_reg_8(struct device *dev, u8 reg_address, u8 val)
 {
diff --git a/drivers/staging/iio/meter/ade7754.h 
b/drivers/staging/iio/meter/ade7754.h
deleted file mode 100644
index 28f71c2..000
--- a/drivers/staging/iio/meter/ade7754.h
+++ /dev/null
@@ -1,90 +0,0 @@
-#ifndef _ADE7754_H
-#define _ADE7754_H
-
-#define ADE7754_AENERGY   0x01
-#define ADE7754_RAENERGY  0x02
-#define ADE7754_LAENERGY  0x03
-#define ADE7754_VAENERGY  0x04
-#define ADE7754_RVAENERGY 0x05
-#define ADE7754_LVAENERGY 0x06
-#define ADE7754_PERIOD0x07
-#define ADE7754_TEMP  0x08
-#define ADE7754_WFORM 0x09
-#define ADE7754_OPMODE0x0A
-#define ADE7754_MMODE 0x0B
-#define ADE7754_WAVMODE   0x0C
-#define ADE7754_WATMODE   0x0D
-#define ADE7754_VAMODE0x0E
-#define ADE7754_IRQEN 0x0F
-#define ADE7754_STATUS0x10
-#define ADE7754_RSTATUS   0x11
-#define ADE7754_ZXTOUT0x12
-#define ADE7754_LINCYC0x13
-#define ADE7754_SAGCYC0x14
-#define ADE7754_SAGLVL0x15
-#define ADE7754_VPEAK 0x16
-#define ADE7754_IPEAK 0x17
-#define ADE7754_GAIN  0x18
-#define ADE7754_AWG   0x19
-#define ADE7754_BWG   0x1A
-#define ADE7754_CWG   0x1B
-#define ADE7754_AVAG  0x1C
-#define ADE7754_BVAG  0x1D
-#define ADE7754_CVAG  0x1E
-#define ADE7754_APHCAL0x1F
-#define ADE7754_BPHCAL0x20
-#define ADE7754_CPHCAL0x21
-#define ADE7754_AAPOS 0x22
-#define ADE7754_BAPOS 0x23
-#define ADE7754_CAPOS 0x24
-#define 

Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver

[Michael Zoran]

On Mon, 2017-03-20 at 11:58 -0300, Mauro Carvalho Chehab wrote:
> Em Mon, 20 Mar 2017 04:08:21 -0700
> Michael Zoran  escreveu:
> 
> > On Mon, 2017-03-20 at 07:58 -0300, Mauro Carvalho Chehab wrote:
> > > Em Sun, 19 Mar 2017 22:11:07 -0300
> > > Mauro Carvalho Chehab  escreveu:
> > >   
> > > > Em Sun, 19 Mar 2017 10:04:28 -0700
> > > > Michael Zoran  escreveu:
> > > >   
> > > > > A working DT that I tried this morning with the current
> > > > > firmware
> > > > > is
> > > > > posted here:
> > > > > http://lists.infradead.org/pipermail/linux-rpi-kernel/2017-Ma
> > > > > rch/
> > > > > 005924
> > > > > .html
> > > > > 
> > > > > It even works with minecraft_pi!
> > > 
> > >   
> > 
> > Hi, can you e-mail out your config.txt?  Do you have audio enabled
> > in
> > config.txt?
> 
> yes, I have this:
> 
> $ cat config.txt |grep -i audio
> # uncomment to force a HDMI mode rather than DVI. This can make audio
> work in
> # Enable audio (loads snd_bcm2835)
> dtparam=audio=on
> 
> Full config attached.
> 
> Thanks,
> Mauro
> 

Are you using Eric Anholt's HDMI Audio driver that's included in VC4? 
That could well be incompatible with the firmware driver. Or are you
using a half mode of VC4 for audio and VCHIQ for video?


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

Re: [PATCH v5 03/39] [media] dt/bindings: Add bindings for OV5640

[Rob Herring]

On Thu, Mar 09, 2017 at 08:52:43PM -0800, Steve Longerbeam wrote:
> Add device tree binding documentation for the OV5640 camera sensor.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  .../devicetree/bindings/media/i2c/ov5640.txt   | 45 
> ++
>  1 file changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5640.txt

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

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Mauro Carvalho Chehab]

Em Mon, 20 Mar 2017 14:10:30 +0100
Hans Verkuil  escreveu:

> On 03/17/2017 03:37 PM, Russell King - ARM Linux wrote:
> > On Fri, Mar 17, 2017 at 02:51:10PM +0100, Philipp Zabel wrote:  
> >> On Fri, 2017-03-17 at 10:24 -0300, Mauro Carvalho Chehab wrote:
> >> [...]  
> >>> The big question, waiting for an answer on the last 8 years is
> >>> who would do that? Such person would need to have several different
> >>> hardware from different vendors, in order to ensure that it has
> >>> a generic solution.
> >>>
> >>> It is a way more feasible that the Kernel developers that already 
> >>> have a certain hardware on their hands to add support inside the
> >>> driver to forward the controls through the pipeline and to setup
> >>> a "default" pipeline that would cover the common use cases at
> >>> driver's probe.  
> >>
> >> Actually, would setting pipeline via libv4l2 plugin and letting drivers
> >> provide a sane enabled default pipeline configuration be mutually
> >> exclusive? Not sure about the control forwarding, but at least a simple
> >> link setup and format forwarding would also be possible in the kernel
> >> without hindering userspace from doing it themselves later.  
> > 
> > I think this is the exact same problem as controls in ALSA.
> > 
> > When ALSA started off in life, the requirement was that all controls
> > shall default to minimum, and the user is expected to adjust controls
> > after the system is running.
> > 
> > After OSS, this gave quite a marked change in system behaviour, and
> > led to a lot of "why doesn't my sound work anymore" problems, because
> > people then had to figure out which combination of controls had to be
> > set to get sound out of their systems.
> > 
> > Now it seems to be much better, where install Linux on a platform, and
> > you have a working sound system (assuming that the drivers are all there
> > which is generally the case for x86.)
> > 
> > However, it's still possible to adjust all the controls from userspace.
> > All that's changed is the defaults.
> > 
> > Why am I mentioning this - because from what I understand Mauro saying,
> > it's no different from this situation.  Userspace will still have the
> > power to disable all links and setup its own.  The difference is that
> > there will be a default configuration that the kernel sets up at boot
> > time that will be functional, rather than the current default
> > configuration where the system is completely non-functional until
> > manually configured.
> > 
> > However, at the end of the day, I don't care _where_ the usability
> > problems are solved, only that there is some kind of solution.  It's not
> > the _where_ that's the real issue here, but the _how_, and discussion of
> > the _how_ is completely missing.
> > 
> > So, let's try kicking off a discussion about _how_ to do things.
> > 
> > _How_ do we setup a media controller system so that we end up with a
> > usable configuration - let's start with the obvious bit... which links
> > should be enabled.
> > 
> > I think the first pre-requisit is that we stop exposing capture devices
> > that can never be functional for the hardware that's present on the board,
> > so that there isn't this plentora of useless /dev/video* nodes and useless
> > subdevices.
> > 
> > One possible solution to finding a default path may be "find the shortest
> > path between the capture device and the sensor and enable intervening
> > links".
> > 
> > Then we need to try configuring that path with format/resolution
> > information.
> > 
> > However, what if something in the shortest path can't handle the format
> > that the sensor produces?  I think at that point, we'd need to drop that
> > subdev out of the path resolution, re-run the "find the shortest path"
> > algorithm, and try again.
> > 
> > Repeat until success or no path between the capture and sensor exists.
> > 
> > This works fine if you have just one sensor visible from a capture device,
> > but not if there's more than one (which I suspect is the case with the
> > Sabrelite board with its two cameras and video receiver.)  That breaks
> > the "find the shortest path" algorithm.
> > 
> > So, maybe it's a lot better to just let the board people provide via DT
> > a default setup for the connectivity of the modules somehow - certainly
> > one big step forward would be to disable in DT parts of the capture
> > system that can never be used (remembering that boards like the RPi /
> > Hummingboard may end up using DT overlays to describe this for different
> > cameras, so the capture setup may change after initial boot.)  
> 
> The MC was developed before the device tree came along. But now that the DT
> is here, I think this could be a sensible idea to let the DT provide an
> initial path.
> 
> Sakari, Laurent, Mauro: any opinions?

It makes perfect sense to me.

By setting the pipeline via DT on boards with simple configurations,
e. g. just one CSI physical interface, it can create just one

Re: [PATCH 3/9] stating/atomisp: fix -Wold-style-definition warning

[Stephen Hemminger]

On Mon, 20 Mar 2017 10:32:19 +0100
Arnd Bergmann  wrote:

> -void ia_css_dequeue_param_buffers(/*unsigned int pipe_num*/)
> +void ia_css_dequeue_param_buffers(/*unsigned int pipe_num*/ void)
>  {
Why keep the comment?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v5 02/39] [media] dt-bindings: Add bindings for i.MX media driver

[Rob Herring]

+Ramiro

On Thu, Mar 09, 2017 at 08:52:42PM -0800, Steve Longerbeam wrote:
> Add bindings documentation for the i.MX media driver.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  Documentation/devicetree/bindings/media/imx.txt | 74 
> +
>  1 file changed, 74 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/imx.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/imx.txt 
> b/Documentation/devicetree/bindings/media/imx.txt
> new file mode 100644
> index 000..3059c06
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/imx.txt
> @@ -0,0 +1,74 @@
> +Freescale i.MX Media Video Device
> +=
> +
> +Video Media Controller node
> +---
> +
> +This is the media controller node for video capture support. It is a
> +virtual device that lists the camera serial interface nodes that the
> +media device will control.
> +
> +Required properties:
> +- compatible : "fsl,imx-capture-subsystem";
> +- ports  : Should contain a list of phandles pointing to camera
> + sensor interface ports of IPU devices
> +
> +example:
> +
> +capture-subsystem {
> + compatible = "fsl,imx-capture-subsystem";
> + ports = <_csi0>, <_csi1>;
> +};
> +
> +fim child node
> +--
> +
> +This is an optional child node of the ipu_csi port nodes. If present and
> +available, it enables the Frame Interval Monitor. Its properties can be
> +used to modify the method in which the FIM measures frame intervals.
> +Refer to Documentation/media/v4l-drivers/imx.rst for more info on the
> +Frame Interval Monitor.
> +
> +Optional properties:
> +- fsl,input-capture-channel: an input capture channel and channel flags,
> +  specified as . The channel number
> +  must be 0 or 1. The flags can be
> +  IRQ_TYPE_EDGE_RISING, IRQ_TYPE_EDGE_FALLING, or
> +  IRQ_TYPE_EDGE_BOTH, and specify which input
> +  capture signal edge will trigger the input
> +  capture event. If an input capture channel is
> +  specified, the FIM will use this method to
> +  measure frame intervals instead of via the EOF
> +  interrupt. The input capture method is much
> +  preferred over EOF as it is not subject to
> +  interrupt latency errors. However it requires
> +  routing the VSYNC or FIELD output signals of
> +  the camera sensor to one of the i.MX input
> +  capture pads (SD1_DAT0, SD1_DAT1), which also
> +  gives up support for SD1.
> +
> +
> +mipi_csi2 node
> +--
> +
> +This is the device node for the MIPI CSI-2 Receiver, required for MIPI
> +CSI-2 sensors.
> +
> +Required properties:
> +- compatible : "fsl,imx6-mipi-csi2", "snps,dw-mipi-csi2";

Ramiro is also working on a binding for DW MIPI CSI2 block[1]. We need 1 
binding for that.

> +- reg   : physical base address and length of the register set;
> +- clocks : the MIPI CSI-2 receiver requires three clocks: hsi_tx
> +   (the D-PHY clock), video_27m (D-PHY PLL reference
> +   clock), and eim_podf;
> +- clock-names: must contain "dphy", "ref", "pix";
> +- port@*: five port nodes must exist, containing endpoints
> +   connecting to the source and sink devices according to
> +   of_graph bindings. The first port is an input port,
> +   connecting with a MIPI CSI-2 source, and ports 1
> +   through 4 are output ports connecting with parallel
> +   bus sink endpoint nodes and correspond to the four
> +   MIPI CSI-2 virtual channel outputs.
> +
> +Optional properties:
> +- interrupts : must contain two level-triggered interrupts,
> +   in order: 100 and 101;
> -- 
> 2.7.4
> 

[1] https://lkml.org/lkml/2017/3/7/395
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver

[Mauro Carvalho Chehab]

Em Mon, 20 Mar 2017 04:08:21 -0700
Michael Zoran  escreveu:

> On Mon, 2017-03-20 at 07:58 -0300, Mauro Carvalho Chehab wrote:
> > Em Sun, 19 Mar 2017 22:11:07 -0300
> > Mauro Carvalho Chehab  escreveu:
> >   
> > > Em Sun, 19 Mar 2017 10:04:28 -0700
> > > Michael Zoran  escreveu:
> > >   
> > > > A working DT that I tried this morning with the current firmware
> > > > is
> > > > posted here:
> > > > http://lists.infradead.org/pipermail/linux-rpi-kernel/2017-March/
> > > > 005924
> > > > .html
> > > > 
> > > > It even works with minecraft_pi!    
> > 
> > With the new firmware, sometime after booting, I'm getting an oops,
> > caused
> > by bcm2835_audio/vchiq:
> > 
> > [  298.788995] Unable to handle kernel NULL pointer dereference at
> > virtual address 0034
> > [  298.821458] pgd = ed004000
> > [  298.832294] [0034] *pgd=2e5e9835, *pte=,
> > *ppte=
> > [  298.857450] Internal error: Oops: 17 [#1] SMP ARM
> > [  298.876273] Modules linked in: cfg80211 hid_logitech_hidpp
> > hid_logitech_dj snd_bcm2835(C) snd_pcm snd_timer snd soundcore
> > vchiq(C) uio_pdrv_genirq uio fuse
> > [  298.932064] CPU: 3 PID: 847 Comm: pulseaudio Tainted:
> > G C  4.11.0-rc1+ #56
> > [  298.963774] Hardware name: Generic DT based system
> > [  298.982945] task: ef758580 task.stack: ee4c6000
> > [  299.001080] PC is at mutex_lock+0x14/0x3c
> > [  299.017148] LR is at vchiq_add_service_internal+0x138/0x3a0
> > [vchiq]
> > [  299.042246] pc : []lr : []psr:
> > 4013
> > sp : ee4c7ca8  ip :   fp : ef709800
> > [  299.088240] r10:   r9 : ee3bffc0  r8 : 0034
> > [  299.109153] r7 : 0003  r6 :   r5 : ee4c7d00  r4 :
> > ee1d8c00
> > [  299.135291] r3 : ef758580  r2 :   r1 : ffc8  r0 :
> > 0034
> > [  299.161431] Flags: nZcv  IRQs on  FIQs on  Mode SVC_32  ISA
> > ARM  Segment none
> > [  299.190008] Control: 10c5383d  Table: 2d00406a  DAC: 0051
> > [  299.213011] Process pulseaudio (pid: 847, stack limit =
> > 0xee4c6220)
> > [  299.238104] Stack: (0xee4c7ca8 to 0xee4c8000)
> > [  299.255539] 7ca0:   c1403d54 80400040 ff7f0600
> > ff7f0660 bf06b578 ee3bffc0
> > [  299.288301] 7cc0:  ee3afd00  ee4c7d00 
> > bf0640b4  bf066428
> > [  299.321064] 7ce0: ee3afd00 ee3afd00 ee4c7d34 ee3af444 ee3bffc0
> > ee3af444 ee3bffc0 bf0662ec
> > [  299.353826] 7d00: 41554453 bf065db0 ee3afd00 00010002 bf0d7408
> > ee3af440  bf0d7408
> > [  299.386587] 7d20: ee79bd80 bf0d5a04  ef709800 0020
> > 0002 0001 41554453
> > [  299.419349] 7d40:    bf0d559c ee3af440
> > 0001 0001 
> > [  299.452111] 7d60: ee24ac80 ee24ac80 ee1c4a00  ee79bd80
> > ee24ace8 0001 bf0d4dfc
> > [  299.484872] 7d80: 000b  ee4b8c3c  ee4c7dc8
> > ee4b8800 ee4b8c28 ee4c6000
> > [  299.517635] 7da0:  ee4b8c3c ed029e40 bf0c0404 ee4b8800
> > ee1c4a00 ee4b8800 ed029e40
> > [  299.550398] 7dc0:  bf0c0560 ee072340  ef758580
> > c0367b7c ee4b8c40 ee4b8c40
> > [  299.583161] 7de0:  ee4b8800 ed029e40 ee318f80 ed029e40
> > 0006 ee318f80 bf0c0748
> > [  299.615924] 7e00: bf0a3430 ee4f6180  c0428fe0 ee318f80
> > 21b0 0026 ed029e40
> > [  299.648697] 7e20: ee318f80 ed029e48 c0428f1c ee4c7e94 0006
> > c0421cc0 ee4c7ed0 
> > [  299.681464] 7e40: 0802  ee4c7e94 0006 ee318f80
> > c0432c8c ee4c7f40 c0433bc0
> > [  299.714225] 7e60:  ed029e40  0041 
> > ed004000  ee4c6000
> > [  299.746987] 7e80: eec69808 0005  0002 ee318f80
> > ef0d2910 ee924908 bf0ba284
> > [  299.779750] 7ea0: ee31bbc0 bebb53c4 ee4e1d00 0011 ee4c7f74
> > 0001 f000 c0308b04
> > [  299.812512] 7ec0: ee4c6000  bebb5710 c0434578 ef0d2910
> > ee924908 73541c18 0008
> > [  299.845274] 7ee0: ee4a7019   ee899bb0 ee318f80
> > 0101 0002 0084
> > [  299.878037] 7f00:    ee4c7f10 ee318df8
> > ed029840 40045532 bebb53c4
> > [  299.910799] 7f20: ee4c6000 ee4a7000 c1403ef8 bebb550c 0011
> > ee5eca00 0020 ee5eca18
> > [  299.943562] 7f40: ee4a7000  00080802 0002 ff9c
> > f000 0011 ff9c
> > [  299.976324] 7f60: ee4a7000 c0422e70 0002 c04359b0 ed029840
> > 0802 ed02 0006
> > [  300.009086] 7f80: 0100 0001   0004
> > b189d000 0005 c0308b04
> > [  300.041848] 7fa0: ee4c6000 c0308940  0004 bebb550c
> > 00080802 bebb53c4 00084b58
> > [  300.074611] 7fc0:  0004 b189d000 0005 
> > bebb550c 00099448 bebb5710
> > [  300.107373] 7fe0:  bebb53c8 b6c40da4 b6c24334 8010
> > bebb550c 2fffd861 2fffdc61
> > [  300.140190] [] (mutex_lock) from []
> > (vchiq_add_service_internal+0x138/0x3a0 [vchiq])
> > [  300.178237] [] 

Re: [PATCH 0/5] staging: vc04_services: camera driver maintance

[Michael Zoran]

On Mon, 2017-03-20 at 16:57 +0300, Dan Carpenter wrote:
> On Mon, Mar 20, 2017 at 04:06:00AM -0700, Michael Zoran wrote:
> > On Mon, 2017-03-20 at 13:55 +0300, Dan Carpenter wrote:
> > > I'm not going to review this because it has kbuild errors.
> > > 
> > > regards,
> > > dan carpenter
> > > 
> > 
> > Hi, can you e-mail out the errors or send them to me.  It worked
> > when
> > I submitted it.
> > 
> 
> The problem is that you added a function only for ARM but the camera
> can build on i386.
> 
> Anyway, we need to figure out why you aren't getting the kbuild
> emails
> and fix that.  I forwarded the first email to you.  The other is
> basically the same.
> 
> regards,
> dan carpenter
> 

OK, thanks for sending it to me.

I don't quite understand how that is possible in all since the whole
subtree depends on RASPBERRYPI_FIRMWARE which I had nothing to do with
setting up.  Sounds to me like the real issue here is that the
RASPBERRYPI_FIRMARE driver wasn't setup correctly in the first place. 
I also noticed when I was trying to understand how the raspberrypi
driver works in the first place in that it doesn't always dump the
firmware revision correctly if the DT node mbox handle gets pointed to
who knows where.

I've been complaining about very selective e-mails on so called public
e-mail lists for a very long time now, and nobody has done anything
about it. Sometimes I don't get any e-mail for a week and then I'll get
a flood of e-mail thats 2 weeks old.  And who knows which e-mails lists
changes are going to. Some changes are getting in under the radar by
others and other changes have every line criticized.

I have been honestly testing all the changes I have submitted, and most
of them have been small.  But they have caused things to work when they
don't work at all before.  


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

RE:

[WebStar Loan Firm]

Are you in need of a loan for business / personal loan? Apply now by email, 
note, this offer is for serious minded people Only:  web.loanf...@gmail.com

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

Re: [PATCH] staging: vc04_services: fix NULL pointer dereference on pointer 'service'

[Stefan Wahren]

[add Mauro to CC]

Am 20.03.2017 um 15:08 schrieb Colin King:

From: Colin Ian King 

Currently, if pservice is null then service is set to NULL and immediately
afterwards service is dereferenced causing a null pointer dereference. Fix
this by bailing out early of the function with a null return.

Detected by CoverityScan, CID#1419681 ("Explicit null dereferenced")

Signed-off-by: Colin Ian King 


Acked-by: Stefan Wahren 

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

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 03:00:51PM +0100, Philipp Zabel wrote:
> On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote:
> > The same document says:
> > 
> >   Scaling support is optional. When supported by a subdev, the crop
> >   rectangle on the subdev's sink pad is scaled to the size configured
> >   using the
> >   :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL
> >   using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the
> >   subdev supports scaling but not composing, the top and left values are
> >   not used and must always be set to zero.
> 
> Right, this sentence does imply that when scaling is supported, there
> must be a sink compose rectangle, even when composing is not.
> 
> I have previously set up scaling like this:
> 
> media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> 
> Does this mean, it should work like this instead?
> 
> media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> media-ctl --set-v4l2 
> "'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]"
> media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> 
> I suppose setting the source pad format should not be allowed to modify
> the sink compose rectangle.

That is what I believe having read these documents several times, but
we need v4l2 people to confirm.

Note that setting the format on 'ipu1_csi0':0 should already be done by
the previous media-ctl command, so it should be possible to simplify
that to:

media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
media-ctl --set-v4l2 "'ipu1_csi0':0[compose:(0,0)/960x540]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

I have tripped over a bug in media-ctl when specifying both a crop and
compose rectangle - the --help output suggests that "," should be used
to separate them.  media-ctl rejects that, telling me the character at
the "," should be "]".  Replacing the "," with " " allows media-ctl to
accept it and set both rectangles, so it sounds like a parser bug - I've
not looked into this any further yet.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v5 00/39] i.MX Media Driver

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 02:57:03PM +0100, Hans Verkuil wrote:
> On 03/20/2017 02:29 PM, Russell King - ARM Linux wrote:
> > It's what I have - remember, not everyone is happy to constantly replace
> > their distro packages with random new stuff.
> 
> This is a compliance test, which is continuously developed in tandem with
> new kernel versions. If you are working with an upstream kernel, then you
> should also use the corresponding v4l2-compliance test. What's the point
> of using an old one?
> 
> I will not support driver developers that use an old version of the
> compliance test, that's a waste of my time.

The reason that people may _not_ wish to constantly update v4l-utils
is that it changes the libraries installed on their systems.

So, the solution to that is not to complain about developers not using
the latest version, but instead to de-couple it from the rest of the
package, and provide it as a separate, stand-alone package that doesn't
come with all the extra baggage.

Now, there's two possible answers to that:

1. it depends on the libv4l2 version.  If that's so, then you are
   insisting that people constantly move to the latest libv4l2 because
   of API changes, and those API changes may upset applications they're
   using.  So this isn't really on.

2. it doesn't depend on libv4l2 version, in which case there's no reason
   for it to be packaged with v4l-utils.

The reality is that v4l2-compliance links with libv4l2, so I'm not sure
which it is.  What I am sure of is that I don't want to upgrade libv4l2
on an ad-hoc basis, potentially causing issues with applications.

> >> To test actual streaming you need to provide the -s option.
> >>
> >> Note: v4l2-compliance has been developed for 'regular' video devices,
> >> not MC devices. It may or may not work with the -s option.
> > 
> > Right, and it exists to verify that the establised v4l2 API is correctly
> > implemented.  If the v4l2 API is being offered to user applications,
> > then it must be conformant, otherwise it's not offering the v4l2 API.
> > (That's very much a definition statement in itself.)
> > 
> > So, are we really going to say MC devices do not offer the v4l2 API to
> > userspace, but something that might work?  We've already seen today
> > one user say that they're not going to use mainline because of the
> > crud surrounding MC.
> > 
> 
> Actually, my understanding was that he was stuck on the old kernel code.

Err, sorry, I really don't follow.  Who is "he"?

_I_ was the one who reported the EXPBUF problem.  Your comment makes no
sense.

> In the case of v4l2-compliance, I never had the time to make it work with
> MC devices. Same for that matter of certain memory to memory devices.
> 
> Just like MC devices these too behave differently. They are partially
> supported in v4l2-compliance, but not fully.

It seems you saying that the API provided by /dev/video* for a MC device
breaks the v4l2-compliance tests?

_No one_ has mentioned using v4l2-compliance on the subdevs.

> Complaining about this really won't help. We know it's a problem and unless
> someone (me perhaps?) manages to get paid to work on this it's unlikely to
> change for now.

Like the above comment, your comment makes no sense.  I'm not complaining,
I'm trying to find out the details.

Yes, MC stuff sucks big time right now, the documentation is poor, there's
a lack of understanding on all sides of the issues (which can be seen by
the different opinions that people hold.)  The only way to resolve these
differences is via discussion, and if you're going to start thinking that
everyone is complaining, then there's not going to be any forward progress.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: iio: adc: Replace mlock with driver private lock

[Arushi Singhal]

The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.

In this driver, mlock was being used to protect hardware state
changes.  Replace it with a lock in the devices global data.

Signed-off-by: Arushi Singhal 
---
 drivers/staging/iio/adc/ad7606.c | 8 
 drivers/staging/iio/adc/ad7606.h | 2 ++
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 9dbfa64b1e53..9f529b34e018 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -208,7 +208,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_SCALE:
ret = -EINVAL;
-   mutex_lock(_dev->mlock);
+   mutex_lock(>lock);
for (i = 0; i < ARRAY_SIZE(scale_avail); i++)
if (val2 == scale_avail[i][1]) {
gpiod_set_value(st->gpio_range, i);
@@ -217,7 +217,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
ret = 0;
break;
}
-   mutex_unlock(_dev->mlock);
+   mutex_unlock(>lock);
 
return ret;
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
@@ -231,11 +231,11 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
values[1] = (ret >> 1) & 1;
values[2] = (ret >> 2) & 1;
 
-   mutex_lock(_dev->mlock);
+   mutex_lock(>lock);
gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
  values);
st->oversampling = val;
-   mutex_unlock(_dev->mlock);
+   mutex_unlock(>lock);
 
return 0;
default:
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 746f9553d2ba..5d59bdd78727 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -14,6 +14,7 @@
  * @name:  identification string for chip
  * @channels:  channel specification
  * @num_channels:  number of channels
+ * @lock   protect sensor state
  */
 
 struct ad7606_chip_info {
@@ -37,6 +38,7 @@ struct ad7606_state {
booldone;
void __iomem*base_address;
 
+   struct mutexlock; /* protect sensor state */
struct gpio_desc*gpio_convst;
struct gpio_desc*gpio_reset;
struct gpio_desc*gpio_range;
-- 
2.11.0

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

[PATCH] staging: vc04_services: fix NULL pointer dereference on pointer 'service'

[Colin King]

From: Colin Ian King 

Currently, if pservice is null then service is set to NULL and immediately
afterwards service is dereferenced causing a null pointer dereference. Fix
this by bailing out early of the function with a null return.

Detected by CoverityScan, CID#1419681 ("Explicit null dereferenced")

Signed-off-by: Colin Ian King 
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index dc9f85c2a5da..4f9e738abddf 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -2673,7 +2673,7 @@ vchiq_add_service_internal(VCHIQ_STATE_T *state,
 
if (!pservice) {
kfree(service);
-   service = NULL;
+   return NULL;
}
 
service_quota = >service_quotas[service->localport];
-- 
2.11.0

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

[PATCH v5] staging:sm750fb: Code readability is improved.

[Arushi Singhal]

New variables are added to make the code more readable.

Signed-off-by: Arushi Singhal 
---
 changes in v5
 -try to make the code much more readable.
 - defined the variable at the top of a block.
 - change the variable from temp to tmp.

 drivers/staging/sm750fb/ddk750_mode.c | 55 +++
 1 file changed, 30 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_mode.c 
b/drivers/staging/sm750fb/ddk750_mode.c
index eea5aef2956f..37b5d4850fb9 100644
--- a/drivers/staging/sm750fb/ddk750_mode.c
+++ b/drivers/staging/sm750fb/ddk750_mode.c
@@ -81,33 +81,38 @@ static int programModeRegisters(mode_parameter_t 
*pModeParam, struct pll_value *
if (pll->clockType == SECONDARY_PLL) {
/* programe secondary pixel clock */
poke32(CRT_PLL_CTRL, sm750_format_pll_reg(pll));
-   poke32(CRT_HORIZONTAL_TOTAL,
-  (((pModeParam->horizontal_total - 1) <<
-CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) &
-   CRT_HORIZONTAL_TOTAL_TOTAL_MASK) |
-  ((pModeParam->horizontal_display_end - 1) &
-   CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK));
-
-   poke32(CRT_HORIZONTAL_SYNC,
-  ((pModeParam->horizontal_sync_width <<
-CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) &
-   CRT_HORIZONTAL_SYNC_WIDTH_MASK) |
-  ((pModeParam->horizontal_sync_start - 1) &
-   CRT_HORIZONTAL_SYNC_START_MASK));
 
-   poke32(CRT_VERTICAL_TOTAL,
-  (((pModeParam->vertical_total - 1) <<
-CRT_VERTICAL_TOTAL_TOTAL_SHIFT) &
-   CRT_VERTICAL_TOTAL_TOTAL_MASK) |
-  ((pModeParam->vertical_display_end - 1) &
-   CRT_VERTICAL_TOTAL_DISPLAY_END_MASK));
+   tmp = ((pModeParam->horizontal_total - 1) <<
+  CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) &
+CRT_HORIZONTAL_TOTAL_TOTAL_MASK;
+   tmp |= (pModeParam->horizontal_display_end - 1) &
+ CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK;
 
-   poke32(CRT_VERTICAL_SYNC,
-  ((pModeParam->vertical_sync_height <<
-CRT_VERTICAL_SYNC_HEIGHT_SHIFT) &
-   CRT_VERTICAL_SYNC_HEIGHT_MASK) |
-  ((pModeParam->vertical_sync_start - 1) &
-   CRT_VERTICAL_SYNC_START_MASK));
+   poke32(CRT_HORIZONTAL_TOTAL, tmp);
+
+   tmp = (pModeParam->horizontal_sync_width <<
+  CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) &
+CRT_HORIZONTAL_SYNC_WIDTH_MASK;
+   tmp |= (pModeParam->horizontal_sync_start - 1) &
+ CRT_HORIZONTAL_SYNC_START_MASK;
+
+   poke32(CRT_HORIZONTAL_SYNC, tmp);
+
+   tmp = ((pModeParam->vertical_total - 1) <<
+  CRT_VERTICAL_TOTAL_TOTAL_SHIFT) &
+CRT_VERTICAL_TOTAL_TOTAL_MASK;
+   tmp |= (pModeParam->vertical_display_end - 1) &
+ CRT_VERTICAL_TOTAL_DISPLAY_END_MASK;
+
+   poke32(CRT_VERTICAL_TOTAL, tmp);
+
+   tmp = ((pModeParam->vertical_sync_height <<
+  CRT_VERTICAL_SYNC_HEIGHT_SHIFT)) &
+CRT_VERTICAL_SYNC_HEIGHT_MASK;
+   tmp |= (pModeParam->vertical_sync_start - 1) &
+ CRT_VERTICAL_SYNC_START_MASK;
+
+   poke32(CRT_VERTICAL_SYNC, tmp);
 
tmp = DISPLAY_CTRL_TIMING | DISPLAY_CTRL_PLANE;
if (pModeParam->vertical_sync_polarity)
-- 
2.11.0

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

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Philipp Zabel]

On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 12:55:26PM +0100, Philipp Zabel wrote:
> > The above paragraph suggests we skip any rectangles that are not
> > supported. In our case that would be 3. and 4., since the CSI can't
> > compose into a larger frame. I hadn't realised that the crop selection
> > currently happens on the source pad.
> 
> I'd recommend viewing the documentation in its post-processed version,
> because then you get the examples as pictures, and they say that a
> picture is worth 1000 words.  See
> 
>   https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/dev-subdev.html
> 
> There is almost an exact example of what we're trying to do - it's
> figure 4.6.  Here, we have a sink pad with a cropping rectangle on
> the input, which is then scaled to a composition rectangle (there's
> no bounds rectangle, and it's specified that in such a case the
> top,left of the composition rectangle will always be 0,0 - see quote
> below).
> 
> Where it differs is that the example also supports source cropping
> for two source pads.  We don't support that.
>
> The same document says:
> 
>   Scaling support is optional. When supported by a subdev, the crop
>   rectangle on the subdev's sink pad is scaled to the size configured
>   using the
>   :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL
>   using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the
>   subdev supports scaling but not composing, the top and left values are
>   not used and must always be set to zero.

Right, this sentence does imply that when scaling is supported, there
must be a sink compose rectangle, even when composing is not.

I have previously set up scaling like this:

media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

Does this mean, it should work like this instead?

media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
media-ctl --set-v4l2 
"'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"

I suppose setting the source pad format should not be allowed to modify
the sink compose rectangle.

regards
Philipp

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

Re: [PATCH 0/5] staging: vc04_services: camera driver maintance

[Dan Carpenter]

On Mon, Mar 20, 2017 at 04:06:00AM -0700, Michael Zoran wrote:
> On Mon, 2017-03-20 at 13:55 +0300, Dan Carpenter wrote:
> > I'm not going to review this because it has kbuild errors.
> > 
> > regards,
> > dan carpenter
> > 
> 
> Hi, can you e-mail out the errors or send them to me.  It worked when
> I submitted it.
> 

The problem is that you added a function only for ARM but the camera
can build on i386.

Anyway, we need to figure out why you aren't getting the kbuild emails
and fix that.  I forwarded the first email to you.  The other is
basically the same.

regards,
dan carpenter

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

Re: [PATCH v5 00/39] i.MX Media Driver

[Hans Verkuil]

On 03/20/2017 02:29 PM, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 02:01:58PM +0100, Hans Verkuil wrote:
>> On 03/19/2017 06:54 PM, Steve Longerbeam wrote:
>>>
>>>
>>> On 03/19/2017 03:38 AM, Russell King - ARM Linux wrote:
 What did you do with:

 ioctl(3, VIDIOC_REQBUFS, {count=0, type=0 /* V4L2_BUF_TYPE_??? */, 
 memory=0 /* V4L2_MEMORY_??? */}) = -1 EINVAL (Invalid argument)
  test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
 ioctl(3, VIDIOC_EXPBUF, 0xbef405bc) = -1 EINVAL (Invalid argument)
  fail: v4l2-test-buffers.cpp(571): q.has_expbuf(node)
>>
>> This is really a knock-on effect from an earlier issue where the compliance 
>> test
>> didn't detect support for MEMORY_MMAP.
> 
> So why does it succeed when I fix the compliance errors with VIDIOC_G_FMT?
> With that fixed, I now get:
> 
> Format ioctls:
> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
> test VIDIOC_G/S_PARM: OK
> test VIDIOC_G_FBUF: OK (Not Supported)
> test VIDIOC_G_FMT: OK
> test VIDIOC_TRY_FMT: OK
> test VIDIOC_S_FMT: OK
> test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
> test Cropping: OK (Not Supported)
> test Composing: OK (Not Supported)
> test Scaling: OK (Not Supported)
> 
> Buffer ioctls:
> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
> test VIDIOC_EXPBUF: OK
> 
> The reason is, if you look at the code, VIDIOC_G_FMT populates a list
> of possible buffer formats "node->valid_buftypes".  If the VIDIOC_G_FMT
> test fails, then node->valid_buftypes is zero.
> 
> This causes testReqBufs() to only check for the all-zeroed VIDIOC_REQBUFS
> and declare it conformant, without creating any buffers (it can't, it
> doesn't know which formats are supported.)
> 
> This causes node->valid_memorytype to be zero.

It should fail on this and return a more understandable error message.

> 
> We then go on to testExpBuf(), and valid_memorytype zero, claiming (falsely)
> that MMAP is not supported.  The reality is that it _is_ supported, but
> it's just the non-compliant VICIOC_G_FMT call (due to the colorspace
> issue) causes the sequence of tests to fail.

Yeah, you're not the first to complain about this. I plan on fixing this this
week.

> 
>> Always build from the master repo. 1.10 is pretty old.
> 
> It's what I have - remember, not everyone is happy to constantly replace
> their distro packages with random new stuff.

This is a compliance test, which is continuously developed in tandem with
new kernel versions. If you are working with an upstream kernel, then you
should also use the corresponding v4l2-compliance test. What's the point
of using an old one?

I will not support driver developers that use an old version of the
compliance test, that's a waste of my time.

> 
 In any case, it doesn't look like the buffer management is being
 tested at all by v4l2-compliance - we know that gstreamer works, so
 buffers _can_ be allocated, and I've also used dmabufs with gstreamer,
 so I also know that VIDIOC_EXPBUF works there.
>>
>> To test actual streaming you need to provide the -s option.
>>
>> Note: v4l2-compliance has been developed for 'regular' video devices,
>> not MC devices. It may or may not work with the -s option.
> 
> Right, and it exists to verify that the establised v4l2 API is correctly
> implemented.  If the v4l2 API is being offered to user applications,
> then it must be conformant, otherwise it's not offering the v4l2 API.
> (That's very much a definition statement in itself.)
> 
> So, are we really going to say MC devices do not offer the v4l2 API to
> userspace, but something that might work?  We've already seen today
> one user say that they're not going to use mainline because of the
> crud surrounding MC.
> 

Actually, my understanding was that he was stuck on the old kernel code.

In the case of v4l2-compliance, I never had the time to make it work with
MC devices. Same for that matter of certain memory to memory devices.

Just like MC devices these too behave differently. They are partially
supported in v4l2-compliance, but not fully.

Why? NO TIME.

Be glad there *is* a v4l2-compliance test at all! It's really, really useful
already, but it took *years* to develop, little bit by little bit. And yes,
I would really like to update it to fully support codecs and MC devices.
And with a bit of luck I hope to get permission from my boss to work on
this (among others) later in the year.

Complaining about this really won't help. We know it's a problem and unless
someone (me perhaps?) manages to get paid to work on this it's unlikely to
change for now.

Regards,

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

Re: [PATCH v5 00/39] i.MX Media Driver

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 02:01:58PM +0100, Hans Verkuil wrote:
> On 03/19/2017 06:54 PM, Steve Longerbeam wrote:
> > 
> > 
> > On 03/19/2017 03:38 AM, Russell King - ARM Linux wrote:
> >> What did you do with:
> >>
> >> ioctl(3, VIDIOC_REQBUFS, {count=0, type=0 /* V4L2_BUF_TYPE_??? */, 
> >> memory=0 /* V4L2_MEMORY_??? */}) = -1 EINVAL (Invalid argument)
> >>  test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
> >> ioctl(3, VIDIOC_EXPBUF, 0xbef405bc) = -1 EINVAL (Invalid argument)
> >>  fail: v4l2-test-buffers.cpp(571): q.has_expbuf(node)
> 
> This is really a knock-on effect from an earlier issue where the compliance 
> test
> didn't detect support for MEMORY_MMAP.

So why does it succeed when I fix the compliance errors with VIDIOC_G_FMT?
With that fixed, I now get:

Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK
test VIDIOC_TRY_FMT: OK
test VIDIOC_S_FMT: OK
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
test Cropping: OK (Not Supported)
test Composing: OK (Not Supported)
test Scaling: OK (Not Supported)

Buffer ioctls:
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test VIDIOC_EXPBUF: OK

The reason is, if you look at the code, VIDIOC_G_FMT populates a list
of possible buffer formats "node->valid_buftypes".  If the VIDIOC_G_FMT
test fails, then node->valid_buftypes is zero.

This causes testReqBufs() to only check for the all-zeroed VIDIOC_REQBUFS
and declare it conformant, without creating any buffers (it can't, it
doesn't know which formats are supported.)

This causes node->valid_memorytype to be zero.

We then go on to testExpBuf(), and valid_memorytype zero, claiming (falsely)
that MMAP is not supported.  The reality is that it _is_ supported, but
it's just the non-compliant VICIOC_G_FMT call (due to the colorspace
issue) causes the sequence of tests to fail.

> Always build from the master repo. 1.10 is pretty old.

It's what I have - remember, not everyone is happy to constantly replace
their distro packages with random new stuff.

> >> In any case, it doesn't look like the buffer management is being
> >> tested at all by v4l2-compliance - we know that gstreamer works, so
> >> buffers _can_ be allocated, and I've also used dmabufs with gstreamer,
> >> so I also know that VIDIOC_EXPBUF works there.
> 
> To test actual streaming you need to provide the -s option.
> 
> Note: v4l2-compliance has been developed for 'regular' video devices,
> not MC devices. It may or may not work with the -s option.

Right, and it exists to verify that the establised v4l2 API is correctly
implemented.  If the v4l2 API is being offered to user applications,
then it must be conformant, otherwise it's not offering the v4l2 API.
(That's very much a definition statement in itself.)

So, are we really going to say MC devices do not offer the v4l2 API to
userspace, but something that might work?  We've already seen today
one user say that they're not going to use mainline because of the
crud surrounding MC.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Hans Verkuil]

On 03/14/2017 11:21 AM, Mauro Carvalho Chehab wrote:
> Em Tue, 14 Mar 2017 08:55:36 +0100
> Hans Verkuil  escreveu:
> 
>> On 03/14/2017 04:45 AM, Mauro Carvalho Chehab wrote:
>>> Hi Sakari,
>>>
>>> I started preparing a long argument about it, but gave up in favor of a
>>> simpler one.
>>>
>>> Em Mon, 13 Mar 2017 14:46:22 +0200
>>> Sakari Ailus  escreveu:
>>>   
 Drivers are written to support hardware, not particular use case.
>>>
>>> No, it is just the reverse: drivers and hardware are developed to
>>> support use cases.
>>>
>>> Btw, you should remember that the hardware is the full board, not just the
>>> SoC. In practice, the board do limit the use cases: several provide a
>>> single physical CSI connector, allowing just one sensor.
>>>   
> This situation is there since 2009. If I remember well, you tried to write
> such generic plugin in the past, but never finished it, apparently because
> it is too complex. Others tried too over the years. 

 I'd argue I know better what happened with that attempt than you do. I had 
 a
 prototype of a generic pipeline configuration library but due to various
 reasons I haven't been able to continue working on that since around 2012. 
  
>>>
>>> ...
>>>   
> The last trial was done by Jacek, trying to cover just the exynos4 
> driver. 
> Yet, even such limited scope plugin was not good enough, as it was never
> merged upstream. Currently, there's no such plugins upstream.
>
> If we can't even merge a plugin that solves it for just *one* driver,
> I have no hope that we'll be able to do it for the generic case.

 I believe Jacek ceased to work on that plugin in his day job; other than
 that, there are some matters left to be addressed in his latest patchset.  
>>>
>>> The two above basically summaries the issue: the task of doing a generic
>>> plugin on userspace, even for a single driver is complex enough to
>>> not cover within a reasonable timeline.
>>>
>>> From 2009 to 2012, you were working on it, but didn't finish it.
>>>
>>> Apparently, nobody worked on it between 2013-2014 (but I may be wrong, as
>>> I didn't check when the generic plugin interface was added to libv4l).
>>>
>>> In the case of Jacek's work, the first patch I was able to find was
>>> written in Oct, 2014:
>>> https://patchwork.kernel.org/patch/5098111/
>>> (not sure what happened with the version 1).
>>>
>>> The last e-mail about this subject was issued in Dec, 2016.
>>>
>>> In summary, you had this on your task for 3 years for an OMAP3
>>> plugin (where you have a good expertise), and Jacek for 2 years, 
>>> for Exynos 4, where he should also have a good knowledge.
>>>
>>> Yet, with all that efforts, no concrete results were achieved, as none
>>> of the plugins got merged.
>>>
>>> Even if they were merged, if we keep the same mean time to develop a
>>> libv4l plugin, that would mean that a plugin for i.MX6 could take 2-3
>>> years to be developed.
>>>
>>> There's a clear message on it:
>>> - we shouldn't keep pushing for a solution via libv4l.  
>>
>> Or:
>>
>>  - userspace plugin development had a very a low priority and
>>never got the attention it needed.
> 
> The end result is the same: we can't count on it.
> 
>>
>> I know that's *my* reason. I rarely if ever looked at it. I always assumed
>> Sakari and/or Laurent would look at it. If this reason is also valid for
>> Sakari and Laurent, then it is no wonder nothing has happened in all that
>> time.
>>
>> We're all very driver-development-driven, and userspace gets very little
>> attention in general. So before just throwing in the towel we should take
>> a good look at the reasons why there has been little or no development: is
>> it because of fundamental design defects, or because nobody paid attention
>> to it?
> 
> No. We should look it the other way: basically, there are patches
> for i.MX6 driver that sends control from videonode to subdevs. 
> 
> If we nack apply it, who will write the userspace plugin? When
> such change will be merged upstream?
> 
> If we don't have answers to any of the above questions, we should not
> nack it.
> 
> That's said, that doesn't prevent merging a libv4l plugin if/when
> someone can find time/interest to develop it.

I don't think this control inheritance patch will magically prevent you
from needed a plugin.

> 
>> I strongly suspect it is the latter.
>>
>> In addition, I suspect end-users of these complex devices don't really care
>> about a plugin: they want full control and won't typically use generic
>> applications. If they would need support for that, we'd have seen much more
>> interest. The main reason for having a plugin is to simplify testing and
>> if this is going to be used on cheap hobbyist devkits.
> 
> What are the needs for a cheap hobbyist devkit owner? Do we currently
> satisfy those needs? I'd say that having a functional 

Re: [PATCH v5 00/39] i.MX Media Driver

[Philipp Zabel]

On Sun, 2017-03-19 at 12:14 +, Russell King - ARM Linux wrote:
> On Sat, Mar 18, 2017 at 12:58:27PM -0700, Steve Longerbeam wrote:
> > On 03/18/2017 12:22 PM, Russell King - ARM Linux wrote:
> > >0:00:01.955927879 20954  0x15ffe90 INFOv4l2 
> > >gstv4l2object.c:3811:gst_v4l2_object_get_caps: probed caps: 
> > >video/x-bayer, format=(string)rggb, framerate=(fraction)3/1001, 
> > >width=(int)816, height=(int)616, pixel-aspect-ratio=(fraction)1/1; 
> > >video/x-raw, format=(string)I420, framerate=(fraction)3/1001, 
> > >width=(int)816, height=(int)616, interlace-mode=(string)progressive, 
> > >pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)YV12, 
> > >framerate=(fraction)3/1001, width=(int)816, height=(int)616, 
> > >interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1; 
> > >video/x-raw, format=(string)BGR, framerate=(fraction)3/1001, 
> > >width=(int)816, height=(int)616, interlace-mode=(string)progressive, 
> > >pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)RGB, 
> > >framerate=(fraction)3/1001, width=(int)816, height=(int)616, 
> > >interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1
> > >
> > >despite the media pipeline actually being configured for 60fps.
> > >
> > >Forcing it by adjusting the pipeline only results in gstreamer
> > >failing, because it believes that v4l2 is unable to operate at
> > >60fps.
> > >
> > >Also note the complaints from v4l2src about the non-compliance...
> > 
> > Thanks, I've fixed most of v4l2-compliance issues, but this is not
> > done yet. Is that something you can help with?
> 
> I've looked at this, and IMHO it's yet another media control API mess.
> 
> - media-ctl itself allows setting the format on subdev pads via
>   struct v4l2_subdev_format.
> 
> - struct v4l2_subdev_format contains a struct v4l2_mbus_framefmt.
> 
> - struct v4l2_mbus_framefmt contains:
>   * @width:  frame width
>   * @height: frame height
>   * @code:   data format code (from enum v4l2_mbus_pixelcode)
>   * @field:  used interlacing type (from enum v4l2_field)
>   * @colorspace: colorspace of the data (from enum v4l2_colorspace)
>   * @ycbcr_enc:  YCbCr encoding of the data (from enum v4l2_ycbcr_encoding)
>   * @quantization: quantization of the data (from enum v4l2_quantization)
>   * @xfer_func:  transfer function of the data (from enum v4l2_xfer_func)
> 
> - media-ctl sets width, height, code and field, but nothing else.
> 
> We're already agreed that the fields that media-ctl are part of the
> format negotiation between the ultimate source, flowing down to the
> capture device.  However, there's no support in media-ctl to deal
> with these other fields - so media-ctl in itself is only half-
> implemented.

To set and read colorimetry information:
https://patchwork.linuxtv.org/patch/39350/

regards
Philipp

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

Re: [PATCH v5 00/39] i.MX Media Driver

[Philipp Zabel]

On Sat, 2017-03-18 at 12:58 -0700, Steve Longerbeam wrote:
> 
> On 03/18/2017 12:22 PM, Russell King - ARM Linux wrote:
> > Hi Steve,
> >
> > I've just been trying to get gstreamer to capture and h264 encode
> > video from my camera at various frame rates, and what I've discovered
> > does not look good.
> >
> > 1) when setting frame rates, media-ctl _always_ calls
> > VIDIOC_SUBDEV_S_FRAME_INTERVAL with pad=0.

To allow setting pad > 0:
https://patchwork.linuxtv.org/patch/39348/

> > 2) media-ctl never retrieves the frame interval information, so there's
> > no way to read it back with standard tools, and no indication that
> > this is going on...
> 
> I think Philipp Zabel submitted a patch which addresses these
> in media-ctl. Check with him.

To read back and propagate the frame interval:
https://patchwork.linuxtv.org/patch/39349/
https://patchwork.linuxtv.org/patch/39351/

regards
Philipp

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

Re: [PATCH 2/4] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()

[walter harms]

Am 20.03.2017 13:51, schrieb DaeSeok Youn:
> 2017-03-20 21:04 GMT+09:00 walter harms :
>>
>>
>> Am 20.03.2017 11:59, schrieb Daeseok Youn:
>>> If v4l2_subdev_call() gets the global frame interval values,
>>> it returned 0 and it could be checked whether numerator is zero or not.
>>>
>>> If the numerator is not zero, the fps could be calculated in this function.
>>> If not, it just returns 0.
>>>
>>> Signed-off-by: Daeseok Youn 
>>> ---
>>>  .../media/atomisp/pci/atomisp2/atomisp_cmd.c   | 22 
>>> ++
>>>  1 file changed, 10 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
>>> b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>>> index 8bdb224..6bdd19e 100644
>>> --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>>> +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>>> @@ -153,20 +153,18 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct 
>>> video_device *dev)
>>>
>>>  static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device 
>>> *asd)
>>>  {
>>> - struct v4l2_subdev_frame_interval frame_interval;
>>> + struct v4l2_subdev_frame_interval fi;
>>>   struct atomisp_device *isp = asd->isp;
>>> - unsigned short fps;
>>>
>>> - if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
>>> - video, g_frame_interval, _interval)) {
>>> - fps = 0;
>>> - } else {
>>> - if (frame_interval.interval.numerator)
>>> - fps = frame_interval.interval.denominator /
>>> - frame_interval.interval.numerator;
>>> - else
>>> - fps = 0;
>>> - }
>>> + unsigned short fps = 0;
>>> + int ret;
>>> +
>>> + ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
>>> +video, g_frame_interval, );
>>> +
>>> + if (!ret && fi.interval.numerator)
>>> + fps = fi.interval.denominator / fi.interval.numerator;
>>> +
>>>   return fps;
>>>  }
>>
>>
>>
>> do you need to check ret at all ? if an error occurs can 
>> fi.interval.numerator
>> be something else than 0 ?
> the return value from the v4l2_subdev_call() function is zero when it
> is done without any error. and also I checked
> the ret value whether is 0 or not. if the ret is 0 then the value of
> numerator should be checked to avoid for dividing by 0.
>>
>> if ret is an ERRNO it would be wise to return ret not fps, but this may 
>> require
>> changes at other places also.
> hmm.., yes, you are right. but I think it is ok because the
> atomisp_get_sensor_fps() function is needed to get fps value.
> (originally, zero or calculated fps value was returned.)

maybe its better to divide this in:
if (ret)
   return 0; // error case

return (fi.interval.numerator>0)?fi.interval.denominator / 
fi.interval.numerator:0;

So there is a chance that someone will a) understand and b) fix the error 
return.

re,
 wh

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

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Hans Verkuil]

On 03/17/2017 03:37 PM, Russell King - ARM Linux wrote:
> On Fri, Mar 17, 2017 at 02:51:10PM +0100, Philipp Zabel wrote:
>> On Fri, 2017-03-17 at 10:24 -0300, Mauro Carvalho Chehab wrote:
>> [...]
>>> The big question, waiting for an answer on the last 8 years is
>>> who would do that? Such person would need to have several different
>>> hardware from different vendors, in order to ensure that it has
>>> a generic solution.
>>>
>>> It is a way more feasible that the Kernel developers that already 
>>> have a certain hardware on their hands to add support inside the
>>> driver to forward the controls through the pipeline and to setup
>>> a "default" pipeline that would cover the common use cases at
>>> driver's probe.
>>
>> Actually, would setting pipeline via libv4l2 plugin and letting drivers
>> provide a sane enabled default pipeline configuration be mutually
>> exclusive? Not sure about the control forwarding, but at least a simple
>> link setup and format forwarding would also be possible in the kernel
>> without hindering userspace from doing it themselves later.
> 
> I think this is the exact same problem as controls in ALSA.
> 
> When ALSA started off in life, the requirement was that all controls
> shall default to minimum, and the user is expected to adjust controls
> after the system is running.
> 
> After OSS, this gave quite a marked change in system behaviour, and
> led to a lot of "why doesn't my sound work anymore" problems, because
> people then had to figure out which combination of controls had to be
> set to get sound out of their systems.
> 
> Now it seems to be much better, where install Linux on a platform, and
> you have a working sound system (assuming that the drivers are all there
> which is generally the case for x86.)
> 
> However, it's still possible to adjust all the controls from userspace.
> All that's changed is the defaults.
> 
> Why am I mentioning this - because from what I understand Mauro saying,
> it's no different from this situation.  Userspace will still have the
> power to disable all links and setup its own.  The difference is that
> there will be a default configuration that the kernel sets up at boot
> time that will be functional, rather than the current default
> configuration where the system is completely non-functional until
> manually configured.
> 
> However, at the end of the day, I don't care _where_ the usability
> problems are solved, only that there is some kind of solution.  It's not
> the _where_ that's the real issue here, but the _how_, and discussion of
> the _how_ is completely missing.
> 
> So, let's try kicking off a discussion about _how_ to do things.
> 
> _How_ do we setup a media controller system so that we end up with a
> usable configuration - let's start with the obvious bit... which links
> should be enabled.
> 
> I think the first pre-requisit is that we stop exposing capture devices
> that can never be functional for the hardware that's present on the board,
> so that there isn't this plentora of useless /dev/video* nodes and useless
> subdevices.
> 
> One possible solution to finding a default path may be "find the shortest
> path between the capture device and the sensor and enable intervening
> links".
> 
> Then we need to try configuring that path with format/resolution
> information.
> 
> However, what if something in the shortest path can't handle the format
> that the sensor produces?  I think at that point, we'd need to drop that
> subdev out of the path resolution, re-run the "find the shortest path"
> algorithm, and try again.
> 
> Repeat until success or no path between the capture and sensor exists.
> 
> This works fine if you have just one sensor visible from a capture device,
> but not if there's more than one (which I suspect is the case with the
> Sabrelite board with its two cameras and video receiver.)  That breaks
> the "find the shortest path" algorithm.
> 
> So, maybe it's a lot better to just let the board people provide via DT
> a default setup for the connectivity of the modules somehow - certainly
> one big step forward would be to disable in DT parts of the capture
> system that can never be used (remembering that boards like the RPi /
> Hummingboard may end up using DT overlays to describe this for different
> cameras, so the capture setup may change after initial boot.)

The MC was developed before the device tree came along. But now that the DT
is here, I think this could be a sensible idea to let the DT provide an
initial path.

Sakari, Laurent, Mauro: any opinions?

Regards,

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

Re: [PATCH v4] staging: sm750fb: Code readability is improved

[Dan Carpenter]

On Mon, Mar 20, 2017 at 06:25:19PM +0530, Arushi Singhal wrote:
> On Mon, Mar 20, 2017 at 5:58 PM, Dan Carpenter 
> wrote:
> 
> > On Sun, Mar 19, 2017 at 09:19:20PM +0530, Arushi Singhal wrote:
> > > New variables are added to make the code more readable.
> > >
> > > Signed-off-by: Arushi Singhal 
> > > ---
> > >  changes in v4
> > >  -try to make the code much more readable.
> > >  - defined the variable at the top of a block.
> > > ---
> > >  drivers/staging/sm750fb/ddk750_mode.c | 57
> > +++
> > >  1 file changed, 31 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/drivers/staging/sm750fb/ddk750_mode.c
> > b/drivers/staging/sm750fb/ddk750_mode.c
> > > index eea5aef2956f..6517e770e0a7 100644
> > > --- a/drivers/staging/sm750fb/ddk750_mode.c
> > > +++ b/drivers/staging/sm750fb/ddk750_mode.c
> > > @@ -76,38 +76,43 @@ static int programModeRegisters(mode_parameter_t
> > *pModeParam, struct pll_value *
> > >  {
> > >   int ret = 0;
> > >   int cnt = 0;
> > > - unsigned int tmp, reg;
> > > + unsigned int tmp, reg, temp;
> >
> > Let's not have "tmp" and "temp" both.  Generally "tmp" is better because
> > you can't confuse it with temperature.
> >
> 
> Hi Dan
> I have not added the tmp variable.
> So is it good to use any other variable like "a" instead of temp.

Just re-use "tmp".  No need to add "temp".

regards,
dan carpenter

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

Re: [PATCH v5 00/39] i.MX Media Driver

[Hans Verkuil]

On 03/19/2017 06:54 PM, Steve Longerbeam wrote:
> 
> 
> On 03/19/2017 03:38 AM, Russell King - ARM Linux wrote:
>> On Sat, Mar 18, 2017 at 12:58:27PM -0700, Steve Longerbeam wrote:
>>> Right, imx-media-capture.c (the "standard" v4l2 user interface module)
>>> is not implementing VIDIOC_ENUM_FRAMESIZES. It should, but it can only
>>> return the single frame size that the pipeline has configured (the mbus
>>> format of the attached source pad).
>> I now have a set of patches that enumerate the frame sizes and intervals
>> from the source pad of the first subdev (since you're setting the formats
>> etc there from the capture device, it seems sensible to return what it
>> can support.)  This means my patch set doesn't add to non-CSI subdevs.
>>
>>> Can you share your gstreamer pipeline? For now, until
>>> VIDIOC_ENUM_FRAMESIZES is implemented, try a pipeline that
>>> does not attempt to specify a frame rate. I use the attached
>>> script for testing, which works for me.
>> Note that I'm not specifying a frame rate on gstreamer - I'm setting
>> the pipeline up for 60fps, but gstreamer in its wisdom is unable to
>> enumerate the frame sizes, and therefore is unable to enumerate the
>> frame intervals (frame intervals depend on frame sizes), so it
>> falls back to the "tvnorms" which are basically 25/1 and 3/1001.
>>
>> It sees 60fps via G_PARM, and then decides to set 3/1001 via S_PARM.
>> So, we end up with most of the pipeline operating at 60fps, with CSI
>> doing frame skipping to reduce the frame rate to 30fps.
>>
>> gstreamer doesn't complain, doesn't issue any warnings, the only way
>> you can spot this is to enable debugging and look through the copious
>> debug log, or use -v and check the pad capabilities.
>>
>> Testing using gstreamer, and only using "does it produce video" is a
>> good simple test, but it's just that - it's a simple test.  It doesn't
>> tell you that what you're seeing is what you intended to see (such as
>> video at the frame rate you expected) without more work.
>>
>>> Thanks, I've fixed most of v4l2-compliance issues, but this is not
>>> done yet. Is that something you can help with?
>> What did you do with:
>>
>> ioctl(3, VIDIOC_REQBUFS, {count=0, type=0 /* V4L2_BUF_TYPE_??? */, memory=0 
>> /* V4L2_MEMORY_??? */}) = -1 EINVAL (Invalid argument)
>>  test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
>> ioctl(3, VIDIOC_EXPBUF, 0xbef405bc) = -1 EINVAL (Invalid argument)
>>  fail: v4l2-test-buffers.cpp(571): q.has_expbuf(node)

This is really a knock-on effect from an earlier issue where the compliance test
didn't detect support for MEMORY_MMAP.

>>  test VIDIOC_EXPBUF: FAIL
>>
>> To me, this looks like a bug in v4l2-compliance (I'm using 1.10.0).

Always build from the master repo. 1.10 is pretty old.

>> I'm not sure what buffer VIDIOC_EXPBUF is expected to export, since
>> afaics no buffers have been allocated, so of course it's going to fail.

It just tests if EXPBUF is supported.

I think I will modify v4l2-compliance to bail out if it doesn't find support
for MEMORY_MMAP. Even though in theory support for this is optional, in practice
all applications expect that it is supported. That should fix this
hard-to-understand error.

>> Either that, or the v4l2 core vb2 code is non-compliant with v4l2's
>> interface requirements.
>>
>> In any case, it doesn't look like the buffer management is being
>> tested at all by v4l2-compliance - we know that gstreamer works, so
>> buffers _can_ be allocated, and I've also used dmabufs with gstreamer,
>> so I also know that VIDIOC_EXPBUF works there.

To test actual streaming you need to provide the -s option.

Note: v4l2-compliance has been developed for 'regular' video devices,
not MC devices. It may or may not work with the -s option.

As I think I mentioned somewhere else, creating a compliance test for
MC devices would help enormously in verifying drivers. I'm not sure if
it is better to create a new test or integrate it in v4l2-compliance.

I'm leaning towards the latter since there is a lot of overlap.

>>
> 
> I wouldn't be surprised if you hit on a bug in v4l2-compliance. I 
> stopped with v4l2-compliance
> at a different test failure that also didn't make sense to me:
> 
> Streaming ioctls:
>  test read/write: OK (Not Supported)
>  Video Capture:
>  Buffer: 0 Sequence: 0 Field: Any Timestamp: 41.664259s
>  fail: 
> .../v4l-utils-1.6.2/utils/v4l2-compliance/v4l2-test-buffers.cpp(281): 
> !(g_flags() & (V4L2_BUF_FLAG_DONE | V4L2_BUF_FLAG_ERROR))
>  fail: 
> .../v4l-utils-1.6.2/utils/v4l2-compliance/v4l2-test-buffers.cpp(610): 
> buf.check(q, last_seq)
>  fail: 
> .../v4l-utils-1.6.2/utils/v4l2-compliance/v4l2-test-buffers.cpp(883): 
> captureBufs(node, q, m2m_q, frame_count, false)
>  test MMAP: FAIL
>  test USERPTR: OK (Not Supported)
>  test DMABUF: Cannot test, specify --expbuf-device
> 
> Total: 42, Succeeded: 38, Failed: 

[PATCH v7] staging: adis16060_core: Replace mlock with buf_lock and refactor code

[simran singhal]

The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.

In this driver, mlock was being used to protect hardware state
changes. Replace it with buf_lock in the devices global data.

As buf_lock protects both the adis16060_spi_write() and
adis16060_spi_read() functions and both are always called in
pair. First write, then read. Thus, refactor the code to have
one single function adis16060_spi_write_than_read() which is
protected by the existing buf_lock.

Removed nested locks as the function adis16060_read_raw call
a lock on >buf_lock and then calls the function
adis16060_spi_write which again tries to get hold
of the same lock.

The locks in adis16060_read_raw are dropped and now have a
single safe function adis16060_spi_write_than_read
with the locks inside it being called.

Signed-off-by: simran singhal 
---

 v7:
   -Change subject
   -Remove lock from read_raw instead of from 
function adis16060_spi_write_than_read().
 
 drivers/staging/iio/gyro/adis16060_core.c | 38 +--
 1 file changed, 11 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/iio/gyro/adis16060_core.c 
b/drivers/staging/iio/gyro/adis16060_core.c
index c9d46e7..193587c 100644
--- a/drivers/staging/iio/gyro/adis16060_core.c
+++ b/drivers/staging/iio/gyro/adis16060_core.c
@@ -40,25 +40,18 @@ struct adis16060_state {
 
 static struct iio_dev *adis16060_iio_dev;
 
-static int adis16060_spi_write(struct iio_dev *indio_dev, u8 val)
+static int adis16060_spi_write_than_read(struct iio_dev *indio_dev,
+u8 conf, u16 *val)
 {
int ret;
struct adis16060_state *st = iio_priv(indio_dev);
 
mutex_lock(>buf_lock);
-   st->buf[2] = val; /* The last 8 bits clocked in are latched */
+   st->buf[2] = conf; /* The last 8 bits clocked in are latched */
ret = spi_write(st->us_w, st->buf, 3);
-   mutex_unlock(>buf_lock);
-
-   return ret;
-}
-
-static int adis16060_spi_read(struct iio_dev *indio_dev, u16 *val)
-{
-   int ret;
-   struct adis16060_state *st = iio_priv(indio_dev);
 
-   mutex_lock(>buf_lock);
+   if (ret < 0)
+   return ret;
 
ret = spi_read(st->us_r, st->buf, 3);
 
@@ -69,8 +62,8 @@ static int adis16060_spi_read(struct iio_dev *indio_dev, u16 
*val)
 */
if (!ret)
*val = ((st->buf[0] & 0x3) << 12) |
-   (st->buf[1] << 4) |
-   ((st->buf[2] >> 4) & 0xF);
+(st->buf[1] << 4) |
+((st->buf[2] >> 4) & 0xF);
mutex_unlock(>buf_lock);
 
return ret;
@@ -83,20 +76,15 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
 {
u16 tval = 0;
int ret;
+   struct adis16060_state *st = iio_priv(indio_dev);
 
switch (mask) {
case IIO_CHAN_INFO_RAW:
-   /* Take the iio_dev status lock */
-   mutex_lock(_dev->mlock);
-   ret = adis16060_spi_write(indio_dev, chan->address);
+   ret = adis16060_spi_write_than_read(indio_dev,
+   chan->address, );
if (ret < 0)
-   goto out_unlock;
+   return ret;
 
-   ret = adis16060_spi_read(indio_dev, );
-   if (ret < 0)
-   goto out_unlock;
-
-   mutex_unlock(_dev->mlock);
*val = tval;
return IIO_VAL_INT;
case IIO_CHAN_INFO_OFFSET:
@@ -110,10 +98,6 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
}
 
return -EINVAL;
-
-out_unlock:
-   mutex_unlock(_dev->mlock);
-   return ret;
 }
 
 static const struct iio_info adis16060_info = {
-- 
2.7.4

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

Re: [PATCH 2/4] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()

[DaeSeok Youn]

2017-03-20 21:04 GMT+09:00 walter harms :
>
>
> Am 20.03.2017 11:59, schrieb Daeseok Youn:
>> If v4l2_subdev_call() gets the global frame interval values,
>> it returned 0 and it could be checked whether numerator is zero or not.
>>
>> If the numerator is not zero, the fps could be calculated in this function.
>> If not, it just returns 0.
>>
>> Signed-off-by: Daeseok Youn 
>> ---
>>  .../media/atomisp/pci/atomisp2/atomisp_cmd.c   | 22 
>> ++
>>  1 file changed, 10 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
>> b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>> index 8bdb224..6bdd19e 100644
>> --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>> +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>> @@ -153,20 +153,18 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct 
>> video_device *dev)
>>
>>  static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device *asd)
>>  {
>> - struct v4l2_subdev_frame_interval frame_interval;
>> + struct v4l2_subdev_frame_interval fi;
>>   struct atomisp_device *isp = asd->isp;
>> - unsigned short fps;
>>
>> - if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
>> - video, g_frame_interval, _interval)) {
>> - fps = 0;
>> - } else {
>> - if (frame_interval.interval.numerator)
>> - fps = frame_interval.interval.denominator /
>> - frame_interval.interval.numerator;
>> - else
>> - fps = 0;
>> - }
>> + unsigned short fps = 0;
>> + int ret;
>> +
>> + ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
>> +video, g_frame_interval, );
>> +
>> + if (!ret && fi.interval.numerator)
>> + fps = fi.interval.denominator / fi.interval.numerator;
>> +
>>   return fps;
>>  }
>
>
>
> do you need to check ret at all ? if an error occurs can fi.interval.numerator
> be something else than 0 ?
the return value from the v4l2_subdev_call() function is zero when it
is done without any error. and also I checked
the ret value whether is 0 or not. if the ret is 0 then the value of
numerator should be checked to avoid for dividing by 0.
>
> if ret is an ERRNO it would be wise to return ret not fps, but this may 
> require
> changes at other places also.
hmm.., yes, you are right. but I think it is ok because the
atomisp_get_sensor_fps() function is needed to get fps value.
(originally, zero or calculated fps value was returned.)

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

Re: [PATCH v5 00/39] i.MX Media Driver

[Hans Verkuil]

On 03/19/2017 01:14 PM, Russell King - ARM Linux wrote:
> On Sat, Mar 18, 2017 at 12:58:27PM -0700, Steve Longerbeam wrote:
>> On 03/18/2017 12:22 PM, Russell King - ARM Linux wrote:
>>> 0:00:01.955927879 20954  0x15ffe90 INFOv4l2 
>>> gstv4l2object.c:3811:gst_v4l2_object_get_caps: probed caps: 
>>> video/x-bayer, format=(string)rggb, framerate=(fraction)3/1001, 
>>> width=(int)816, height=(int)616, pixel-aspect-ratio=(fraction)1/1; 
>>> video/x-raw, format=(string)I420, framerate=(fraction)3/1001, 
>>> width=(int)816, height=(int)616, interlace-mode=(string)progressive, 
>>> pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)YV12, 
>>> framerate=(fraction)3/1001, width=(int)816, height=(int)616, 
>>> interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1; 
>>> video/x-raw, format=(string)BGR, framerate=(fraction)3/1001, 
>>> width=(int)816, height=(int)616, interlace-mode=(string)progressive, 
>>> pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)RGB, 
>>> framerate=(fraction)3/1001, width=(int)816, height=(int)616, 
>>> interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1
>>>
>>>despite the media pipeline actually being configured for 60fps.
>>>
>>>Forcing it by adjusting the pipeline only results in gstreamer
>>>failing, because it believes that v4l2 is unable to operate at
>>>60fps.
>>>
>>>Also note the complaints from v4l2src about the non-compliance...
>>
>> Thanks, I've fixed most of v4l2-compliance issues, but this is not
>> done yet. Is that something you can help with?
> 
> I've looked at this, and IMHO it's yet another media control API mess.
> 
> - media-ctl itself allows setting the format on subdev pads via
>   struct v4l2_subdev_format.
> 
> - struct v4l2_subdev_format contains a struct v4l2_mbus_framefmt.
> 
> - struct v4l2_mbus_framefmt contains:
>   * @width:  frame width
>   * @height: frame height
>   * @code:   data format code (from enum v4l2_mbus_pixelcode)
>   * @field:  used interlacing type (from enum v4l2_field)
>   * @colorspace: colorspace of the data (from enum v4l2_colorspace)
>   * @ycbcr_enc:  YCbCr encoding of the data (from enum v4l2_ycbcr_encoding)
>   * @quantization: quantization of the data (from enum v4l2_quantization)
>   * @xfer_func:  transfer function of the data (from enum v4l2_xfer_func)
> 
> - media-ctl sets width, height, code and field, but nothing else.
> 
> We're already agreed that the fields that media-ctl are part of the
> format negotiation between the ultimate source, flowing down to the
> capture device.  However, there's no support in media-ctl to deal
> with these other fields - so media-ctl in itself is only half-
> implemented.

Correct. The colorspace et al fields are in practice unimportant for sensors.
For HDMI/DP they are very important, though.

It's the reason why nobody worked on adding support for this to media-ctl,
it's almost exclusively used with sensors. Not saying that it is right that
it hasn't been added to media-ctl, just that it never had a high enough prio.

Regards,

Hans

> 
> From what I can tell, _we_ are doing the right thing in imx-media-capture.
> 
> However, I think part of the problem is the set_fmt implementation.
> When a source pad is configured via set_fmt(), any fields that can
> not be altered (eg, because the subdev doesn't support colorspace
> conversion) need to be preserved from the subdev's sink pad.
> 
> Right now, CSI doesn't do that - it only looks at the width, height,
> code, and field.
> 
> I think we've got other bugs though that haven't been picked up by any
> review - csi_try_fmt() adjusts the format using the _current_
> configuration of the sink pad, even when using V4L2_SUBDEV_FORMAT_TRY.
> This seems wrong according to the docs: the purpose of the try
> mechanism is to be able to setup the _entire_ pipeline using the TRY
> mechanism to work out whether the configuration works, before then
> setting for real.  If we're validating the TRY formats against the
> live configuration, then we're not doing that.
> 
> There's calls for:
> 
> v4l2_subdev_get_try_format
> v4l2_subdev_get_try_crop
> v4l2_subdev_get_try_compose
> 
> to get the try configuration - we hardly make use of all of these.  I
> would suggest that we change the approach to implementing the various
> subdevs such that:
> 
> 1) like __csi_get_fmt(), we have accessors that gets a pointer to the
>correct state for the TRY/live settings.
> 
> 2) everywhere we're asked to get or set parameters that can be TRY/live,
>we use these accessors to retrieve a pointer to the correct state to
>not only read, but also modify.
> 
> 3) when we're evaluating parameters against another pad, we use these
>accessors to obtain the other pad's configuration, rather than poking
>about in the state saved in the subdev's priv-> (which is irrelevant
>for the TRY variant.)
> 
> 4) ensure that all 

Re: [PATCH v4] staging: sm750fb: Code readability is improved

[Dan Carpenter]

On Sun, Mar 19, 2017 at 09:19:20PM +0530, Arushi Singhal wrote:
> New variables are added to make the code more readable.
> 
> Signed-off-by: Arushi Singhal 
> ---
>  changes in v4
>  -try to make the code much more readable.
>  - defined the variable at the top of a block.
> ---
>  drivers/staging/sm750fb/ddk750_mode.c | 57 
> +++
>  1 file changed, 31 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_mode.c 
> b/drivers/staging/sm750fb/ddk750_mode.c
> index eea5aef2956f..6517e770e0a7 100644
> --- a/drivers/staging/sm750fb/ddk750_mode.c
> +++ b/drivers/staging/sm750fb/ddk750_mode.c
> @@ -76,38 +76,43 @@ static int programModeRegisters(mode_parameter_t 
> *pModeParam, struct pll_value *
>  {
>   int ret = 0;
>   int cnt = 0;
> - unsigned int tmp, reg;
> + unsigned int tmp, reg, temp;

Let's not have "tmp" and "temp" both.  Generally "tmp" is better because
you can't confuse it with temperature.

>  
>   if (pll->clockType == SECONDARY_PLL) {
>   /* programe secondary pixel clock */
>   poke32(CRT_PLL_CTRL, sm750_format_pll_reg(pll));
> - poke32(CRT_HORIZONTAL_TOTAL,
> -(((pModeParam->horizontal_total - 1) <<
> -  CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) &
> - CRT_HORIZONTAL_TOTAL_TOTAL_MASK) |
> -((pModeParam->horizontal_display_end - 1) &
> - CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK));
> -
> - poke32(CRT_HORIZONTAL_SYNC,
> -((pModeParam->horizontal_sync_width <<
> -  CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) &
> - CRT_HORIZONTAL_SYNC_WIDTH_MASK) |
> -((pModeParam->horizontal_sync_start - 1) &
> - CRT_HORIZONTAL_SYNC_START_MASK));
>  
> - poke32(CRT_VERTICAL_TOTAL,
> -(((pModeParam->vertical_total - 1) <<
> -  CRT_VERTICAL_TOTAL_TOTAL_SHIFT) &
> - CRT_VERTICAL_TOTAL_TOTAL_MASK) |
> -((pModeParam->vertical_display_end - 1) &
> - CRT_VERTICAL_TOTAL_DISPLAY_END_MASK));
> + temp = (pModeParam->horizontal_total - 1) <<
> + CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT;
> + temp = temp & CRT_HORIZONTAL_TOTAL_TOTAL_MASK;


These two lines are really part of the same thing and they should be
done together.

tmp = ((pModeParam->horizontal_total - 1) <<
   CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) &
  CRT_HORIZONTAL_TOTAL_TOTAL_MASK;

> + temp = temp | ((pModeParam->horizontal_display_end - 1) &
> + CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK);

tmp |= (pModeParam->horizontal_display_end - 1) &
   CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK;

Use |=.  Remove the extra parenthesis.

regards,
dan carpenter

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

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 12:55:26PM +0100, Philipp Zabel wrote:
> The above paragraph suggests we skip any rectangles that are not
> supported. In our case that would be 3. and 4., since the CSI can't
> compose into a larger frame. I hadn't realised that the crop selection
> currently happens on the source pad.

I'd recommend viewing the documentation in its post-processed version,
because then you get the examples as pictures, and they say that a
picture is worth 1000 words.  See

  https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/dev-subdev.html

There is almost an exact example of what we're trying to do - it's
figure 4.6.  Here, we have a sink pad with a cropping rectangle on
the input, which is then scaled to a composition rectangle (there's
no bounds rectangle, and it's specified that in such a case the
top,left of the composition rectangle will always be 0,0 - see quote
below).

Where it differs is that the example also supports source cropping
for two source pads.  We don't support that.

The same document says:

  Scaling support is optional. When supported by a subdev, the crop
  rectangle on the subdev's sink pad is scaled to the size configured
  using the
  :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL
  using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the
  subdev supports scaling but not composing, the top and left values are
  not used and must always be set to zero.

which in itself describes _exactly_ our hardware here as far as the
cropping and scaling the hardware supports.

> Except when composing is not supported. If the sink compose and source
> crop rectangles are not supported, the source pad format takes their
> place in determining the scaling output resolution. At least that's how
> I read the documentation.

This isn't how I read it.  Scaling is the relationship between the size
of the sink crop and sink compose rectangle.  Composition requires that
there be a composition bounds rectangle to define the composition space,
and the top,left of the composition rectangle be adjustable to place
the composition rectangle within that space.

The above quoted paragraph from the documentation backs up my view in
its final sentence - it doesn't say "if the subdev supports scaling
but not composing, there is no composition rectangle" but says that
there _is_ one but its top,left coordinates are always zero.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: radio-bcm2048: Fix checkpatch warnings: WARNING: Prefer 'unsigned int' to bare use of 'unsigned'

[Dan Carpenter]

Also fix your From header.

regards,
dan carpenter

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

Re: [PATCH 2/4] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()

[walter harms]

Am 20.03.2017 11:59, schrieb Daeseok Youn:
> If v4l2_subdev_call() gets the global frame interval values,
> it returned 0 and it could be checked whether numerator is zero or not.
> 
> If the numerator is not zero, the fps could be calculated in this function.
> If not, it just returns 0.
> 
> Signed-off-by: Daeseok Youn 
> ---
>  .../media/atomisp/pci/atomisp2/atomisp_cmd.c   | 22 
> ++
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
> b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> index 8bdb224..6bdd19e 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> @@ -153,20 +153,18 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct 
> video_device *dev)
>  
>  static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device *asd)
>  {
> - struct v4l2_subdev_frame_interval frame_interval;
> + struct v4l2_subdev_frame_interval fi;
>   struct atomisp_device *isp = asd->isp;
> - unsigned short fps;
>  
> - if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
> - video, g_frame_interval, _interval)) {
> - fps = 0;
> - } else {
> - if (frame_interval.interval.numerator)
> - fps = frame_interval.interval.denominator /
> - frame_interval.interval.numerator;
> - else
> - fps = 0;
> - }
> + unsigned short fps = 0;
> + int ret;
> +
> + ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
> +video, g_frame_interval, );
> +
> + if (!ret && fi.interval.numerator)
> + fps = fi.interval.denominator / fi.interval.numerator;
> +
>   return fps;
>  }



do you need to check ret at all ? if an error occurs can fi.interval.numerator
be something else than 0 ?

if ret is an ERRNO it would be wise to return ret not fps, but this may require
changes at other places also.

re,
 wh

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

Re: [PATCH] staging: radio-bcm2048: Fix checkpatch warnings: WARNING: Prefer 'unsigned int' to bare use of 'unsigned'

[Dan Carpenter]

The subject is too long.

On Mon, Mar 20, 2017 at 10:02:33PM +1100, unknown wrote:
> Signed-off-by: Eddie Youseph 

You need to have a changelog.

regards,
dan carpenter

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

Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver

[Stefan Wahren]

Hi Mauro,


Am 20.03.2017 um 11:58 schrieb Mauro Carvalho Chehab:

Em Sun, 19 Mar 2017 22:11:07 -0300
Mauro Carvalho Chehab  escreveu:


Em Sun, 19 Mar 2017 10:04:28 -0700
Michael Zoran  escreveu:


A working DT that I tried this morning with the current firmware is
posted here:
http://lists.infradead.org/pipermail/linux-rpi-kernel/2017-March/005924
.html

It even works with minecraft_pi!

With the new firmware, sometime after booting, I'm getting an oops, caused
by bcm2835_audio/vchiq:

[  298.788995] Unable to handle kernel NULL pointer dereference at virtual 
address 0034
[  298.821458] pgd = ed004000
[  298.832294] [0034] *pgd=2e5e9835, *pte=, *ppte=
[  298.857450] Internal error: Oops: 17 [#1] SMP ARM
[  298.876273] Modules linked in: cfg80211 hid_logitech_hidpp hid_logitech_dj 
snd_bcm2835(C) snd_pcm snd_timer snd soundcore vchiq(C) uio_pdrv_genirq uio fuse
[  298.932064] CPU: 3 PID: 847 Comm: pulseaudio Tainted: G C  
4.11.0-rc1+ #56
[  298.963774] Hardware name: Generic DT based system
[  298.982945] task: ef758580 task.stack: ee4c6000
[  299.001080] PC is at mutex_lock+0x14/0x3c
[  299.017148] LR is at vchiq_add_service_internal+0x138/0x3a0 [vchiq]
[  299.042246] pc : []lr : []psr: 4013
sp : ee4c7ca8  ip :   fp : ef709800
[  299.088240] r10:   r9 : ee3bffc0  r8 : 0034
[  299.109153] r7 : 0003  r6 :   r5 : ee4c7d00  r4 : ee1d8c00
[  299.135291] r3 : ef758580  r2 :   r1 : ffc8  r0 : 0034
[  299.161431] Flags: nZcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[  299.190008] Control: 10c5383d  Table: 2d00406a  DAC: 0051
[  299.213011] Process pulseaudio (pid: 847, stack limit = 0xee4c6220)
[  299.238104] Stack: (0xee4c7ca8 to 0xee4c8000)
[  299.255539] 7ca0:   c1403d54 80400040 ff7f0600 ff7f0660 
bf06b578 ee3bffc0
[  299.288301] 7cc0:  ee3afd00  ee4c7d00  bf0640b4 
 bf066428
[  299.321064] 7ce0: ee3afd00 ee3afd00 ee4c7d34 ee3af444 ee3bffc0 ee3af444 
ee3bffc0 bf0662ec
[  299.353826] 7d00: 41554453 bf065db0 ee3afd00 00010002 bf0d7408 ee3af440 
 bf0d7408
[  299.386587] 7d20: ee79bd80 bf0d5a04  ef709800 0020 0002 
0001 41554453
[  299.419349] 7d40:    bf0d559c ee3af440 0001 
0001 
[  299.452111] 7d60: ee24ac80 ee24ac80 ee1c4a00  ee79bd80 ee24ace8 
0001 bf0d4dfc
[  299.484872] 7d80: 000b  ee4b8c3c  ee4c7dc8 ee4b8800 
ee4b8c28 ee4c6000
[  299.517635] 7da0:  ee4b8c3c ed029e40 bf0c0404 ee4b8800 ee1c4a00 
ee4b8800 ed029e40
[  299.550398] 7dc0:  bf0c0560 ee072340  ef758580 c0367b7c 
ee4b8c40 ee4b8c40
[  299.583161] 7de0:  ee4b8800 ed029e40 ee318f80 ed029e40 0006 
ee318f80 bf0c0748
[  299.615924] 7e00: bf0a3430 ee4f6180  c0428fe0 ee318f80 21b0 
0026 ed029e40
[  299.648697] 7e20: ee318f80 ed029e48 c0428f1c ee4c7e94 0006 c0421cc0 
ee4c7ed0 
[  299.681464] 7e40: 0802  ee4c7e94 0006 ee318f80 c0432c8c 
ee4c7f40 c0433bc0
[  299.714225] 7e60:  ed029e40  0041  ed004000 
 ee4c6000
[  299.746987] 7e80: eec69808 0005  0002 ee318f80 ef0d2910 
ee924908 bf0ba284
[  299.779750] 7ea0: ee31bbc0 bebb53c4 ee4e1d00 0011 ee4c7f74 0001 
f000 c0308b04
[  299.812512] 7ec0: ee4c6000  bebb5710 c0434578 ef0d2910 ee924908 
73541c18 0008
[  299.845274] 7ee0: ee4a7019   ee899bb0 ee318f80 0101 
0002 0084
[  299.878037] 7f00:    ee4c7f10 ee318df8 ed029840 
40045532 bebb53c4
[  299.910799] 7f20: ee4c6000 ee4a7000 c1403ef8 bebb550c 0011 ee5eca00 
0020 ee5eca18
[  299.943562] 7f40: ee4a7000  00080802 0002 ff9c f000 
0011 ff9c
[  299.976324] 7f60: ee4a7000 c0422e70 0002 c04359b0 ed029840 0802 
ed02 0006
[  300.009086] 7f80: 0100 0001   0004 b189d000 
0005 c0308b04
[  300.041848] 7fa0: ee4c6000 c0308940  0004 bebb550c 00080802 
bebb53c4 00084b58
[  300.074611] 7fc0:  0004 b189d000 0005  bebb550c 
00099448 bebb5710
[  300.107373] 7fe0:  bebb53c8 b6c40da4 b6c24334 8010 bebb550c 
2fffd861 2fffdc61
[  300.140190] [] (mutex_lock) from [] 
(vchiq_add_service_internal+0x138/0x3a0 [vchiq])
[  300.178237] [] (vchiq_add_service_internal [vchiq]) from 
[] (vchiq_open_service+0x58/0xf0 [vchiq])
[  300.221152] [] (vchiq_open_service [vchiq]) from [] 
(vchi_service_open+0x74/0xa8 [vchiq])
[  300.260919] [] (vchi_service_open [vchiq]) from [] 
(bcm2835_audio_open+0xe8/0x2d0 [snd_bcm2835])
[  300.303111] [] (bcm2835_audio_open [snd_bcm2835]) from 
[] (snd_bcm2835_playback_open_generic+0xc0/0x1c4 [snd_bcm2835])
[  300.352975] [] (snd_bcm2835_playback_open_generic [snd_bcm2835]) from 
[] (snd_pcm_open_substream+0x60/0x110 [snd_pcm])

Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt

[Philipp Zabel]

On Sun, 2017-03-19 at 12:08 -0700, Steve Longerbeam wrote:
> 
> On 03/19/2017 08:22 AM, Russell King - ARM Linux wrote:
> > On Thu, Mar 09, 2017 at 08:53:18PM -0800, Steve Longerbeam wrote:
> >> From: Philipp Zabel 
> >>
> >> The csi_try_crop call in set_fmt should compare the cropping rectangle
> >> to the currently set input format, not to the previous input format.
> > Are we really sure that the cropping support is implemented correctly?
> >
> > I came across this while looking at what we're doing with the
> > V4L2_SEL_FLAG_KEEP_CONFIG flag.
> >
> > Documentation/media/uapi/v4l/dev-subdev.rst defines the behaviour of
> > the user API, and "Order of configuration and format propagation" says:
> >
> >The coordinates to a step always refer to the actual size of the
> >previous step. The exception to this rule is the source compose
> >rectangle, which refers to the sink compose bounds rectangle --- if it
> >is supported by the hardware.
> >
> >1. Sink pad format. The user configures the sink pad format. This format
> >   defines the parameters of the image the entity receives through the
> >   pad for further processing.
> >
> >2. Sink pad actual crop selection. The sink pad crop defines the crop
> >   performed to the sink pad format.
> >
> >3. Sink pad actual compose selection. The size of the sink pad compose
> >   rectangle defines the scaling ratio compared to the size of the sink
> >   pad crop rectangle. The location of the compose rectangle specifies
> >   the location of the actual sink compose rectangle in the sink compose
> >   bounds rectangle.
> >
> >4. Source pad actual crop selection. Crop on the source pad defines crop
> >   performed to the image in the sink compose bounds rectangle.
> >
> >5. Source pad format. The source pad format defines the output pixel
> >   format of the subdev, as well as the other parameters with the
> >   exception of the image width and height. Width and height are defined
> >   by the size of the source pad actual crop selection.
> >
> >Accessing any of the above rectangles not supported by the subdev will
> >return ``EINVAL``. Any rectangle referring to a previous unsupported
> >rectangle coordinates will instead refer to the previous supported
> >rectangle. For example, if sink crop is not supported, the compose
> >selection will refer to the sink pad format dimensions instead.
> >
> > Note step 3 above: scaling is defined by the ratio of the _sink_ crop
> > rectangle to the _sink_ compose rectangle.

The above paragraph suggests we skip any rectangles that are not
supported. In our case that would be 3. and 4., since the CSI can't
compose into a larger frame. I hadn't realised that the crop selection
currently happens on the source pad.
The hardware actually only supports cropping of the input (the crop
rectangle we write into the window registers are before downscaling). So
the crop rectangle should be moved to the sink pad.

> > So, lets say that the camera produces a 1280x720 image, and the sink
> > pad format is configured with 1280x720.  That's step 1.
> >
> > The sink crop operates within that rectangle, cropping it to an area.
> > Let's say we're only interested in its centre, so we'd chose 640x360
> > with the top-left as 320,180.  This is step 2.
>>
> > Then, if we want to down-scale by a factor of two, we'd set the sink
> > compose selection to 320x180.

Except when composing is not supported. If the sink compose and source
crop rectangles are not supported, the source pad format takes their
place in determining the scaling output resolution. At least that's how
I read the documentation.

> > This seems to be at odds with how the scaling is done in CSI at
> > present: the selection implementations all reject attempts to
> > configure the sink pad, instead only supporting crop rectangles on
> > the source,
> 
> Correct. Currently cropping is only supported at the source pad
> (step 4).
> 
> Initially the CSI didn't support down-scaling, so step 3 is not supported,
> so the sink pad format/crop selection rectangle/crop compose rectangle
> are collapsed into the same sink pad format rectangle.
> 
> Philipp later added support for /2 downscaling, but we didn't put this in
> the correct API, looks like this needs to move into the selection API at
> step 3 (sink pad compose rectangle).

I am not sure about this. Wouldn't moving the input crop to the sink pad
be enough? If we added support for the sink pad compose rectangle, that
wouldn't actually allow to compose the CSI output into a larger frame.
Since the subdevice can't compose, I'd leave the sink compose rectangle
disabled.

> >   and we use the source crop rectangle to define the
> > down-scaling.

We use the source pad format to define the downscaling relative to the
source crop rectangle (which is wrong, it should be relative to the sink
crop 

Re: [PATCH] staging: wilc1000: Fix sparse warning in wilc_wfi_cfgoperations.c

[Dan Carpenter]

On Sun, Mar 19, 2017 at 11:17:08PM -0700, Jacob Zachariah wrote:
> Fix the following warning reported by sparse:
> 
> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c:2006:51: warning: incorrect 
> type in assignment (different base types)
> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c:2006:51:expected 
> unsigned short [unsigned] [assigned] [usertype] ht_capa_info
> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c:2006:51:got restricted 
> __le16 const [usertype] cap_info
> 

This is probably correct but there is another just a few lines down.
Fix that as well.

regards,
dan carpenter

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

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Hans Verkuil]

On 03/17/2017 12:55 PM, Sakari Ailus wrote:
> Hi Russell,
> 
> On 03/17/17 13:42, Russell King - ARM Linux wrote:
>> On Tue, Mar 14, 2017 at 08:55:36AM +0100, Hans Verkuil wrote:
>>> We're all very driver-development-driven, and userspace gets very little
>>> attention in general. So before just throwing in the towel we should take
>>> a good look at the reasons why there has been little or no development: is
>>> it because of fundamental design defects, or because nobody paid attention
>>> to it?
>>>
>>> I strongly suspect it is the latter.
>>>
>>> In addition, I suspect end-users of these complex devices don't really care
>>> about a plugin: they want full control and won't typically use generic
>>> applications. If they would need support for that, we'd have seen much more
>>> interest. The main reason for having a plugin is to simplify testing and
>>> if this is going to be used on cheap hobbyist devkits.
>>
>> I think you're looking at it with a programmers hat on, not a users hat.
>>
>> Are you really telling me that requiring users to 'su' to root, and then
>> use media-ctl to manually configure the capture device is what most
>> users "want" ?
> 
> It depends on who the user is. I don't think anyone is suggesting a
> regular end user is the user of all these APIs: it is either an
> application tailored for that given device, a skilled user with his test
> scripts or as suggested previously, a libv4l plugin knowing the device
> or a generic library geared towards providing best effort service. The
> last one of this list does not exist yet and the second last item
> requires help.
> 
> Typically this class of devices is simply not up to provide the level of
> service you're requesting without additional user space control library
> which is responsible for automatic white balance, exposure and focus.
> 
> Making use of the full potential of the hardware requires using a more
> expressive interface. That's what the kernel interface must provide. If
> we decide to limit ourselves to a small sub-set of that potential on the
> level of the kernel interface, we have made a wrong decision. It's as
> simple as that. This is why the functionality (and which requires taking
> a lot of policy decisions) belongs to the user space. We cannot have
> multiple drivers providing multiple kernel interfaces for the same hardware.

Right. With my Cisco hat on I can tell you that Cisco would want full low-level
control. If the driver would limit us we would not be able to use it.

Same with anyone who wants to put Android CameraHAL on top of a V4L2 driver:
they would need full control. Some simplified interface would be unacceptable.

> 
> That said, I'm not trying to provide an excuse for not having libraries
> available to help the user to configure and control the device more or
> less automatically even in terms of best effort. It's something that
> does require attention, a lot more of it than it has received in recent
> few years.

Right.

Regards,

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

Re: [PATCH 3/4] staging: atomisp: remove useless condition in if-statements

[Dan Carpenter]

On Mon, Mar 20, 2017 at 08:00:06PM +0900, Daeseok Youn wrote:
> The css_pipe_id was checked with 'CSS_PIPE_ID_COPY' in previous if-
> statement. In this case, if the css_pipe_id equals to 'CSS_PIPE_ID_COPY',
> it could not enter the next if-statement. But the "next" if-statement
> has the condition to check whether the css_pipe_id equals to
> 'CSS_PIPE_ID_COPY' or not. It should be removed.
> 
> Signed-off-by: Daeseok Youn 

The patch is correct but the changelog is not.

s/CSS_PIPE_ID_COPY/CSS_PIPE_ID_YUVPP/

regards,
dan carpenter

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