Re: [PATCH v2 02/20] staging: ccree: replace bit shift with BIT macro

[Gilad Ben-Yossef]

On Sat, Jun 3, 2017 at 11:46 AM, Greg Kroah-Hartman
 wrote:
> On Thu, Jun 01, 2017 at 02:02:52PM +0300, Gilad Ben-Yossef wrote:
>> CC_CTX_SIZE was being defined using a hand rolled bit shift operation.
>> Replace with use of BIT macro.
>>
>> Signed-off-by: Gilad Ben-Yossef 
>> ---
>>  drivers/staging/ccree/cc_crypto_ctx.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> This patch didn't apply at all, are you sure you are working against the
> correct tree?  I've tried to apply some of this series, but most do not
> work at all...
>

Yes, I did. The patch series was created against your staging/staging-next
at commit ca9280d1f82a7a0165a683dc09f182329ebec352

What I believe might have happened is that you took in Gennadii Altukhov's
and Derek Robson's patch set and mine indeed no longer applies on top of theirs.

> Please rebase against my staging-next branch and resend the ones that do
> work.

Sure, I'll rebase on top of current staging/staging-next and resend
the remaining
patches

Thanks,
Gilad



-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v7 16/34] [media] add Omnivision OV5640 sensor driver

[Steve Longerbeam]

On 06/03/2017 02:57 PM, Sakari Ailus wrote:

On Sat, Jun 03, 2017 at 09:51:39PM +0200, Pavel Machek wrote:

Hi!


+   /* Auto/manual exposure */
+   ctrls->auto_exp = v4l2_ctrl_new_std_menu(hdl, ops,
+V4L2_CID_EXPOSURE_AUTO,
+V4L2_EXPOSURE_MANUAL, 0,
+V4L2_EXPOSURE_AUTO);
+   ctrls->exposure = v4l2_ctrl_new_std(hdl, ops,
+   V4L2_CID_EXPOSURE_ABSOLUTE,
+   0, 65535, 1, 0);


Is exposure_absolute supposed to be in microseconds...?


Yes.


According to the docs V4L2_CID_EXPOSURE_ABSOLUTE is in 100 usec units.

  OTOH V4L2_CID_EXPOSURE has no defined unit, so it's a better fit IMO.

Way more drivers appear to be using EXPOSURE than EXPOSURE_ABSOLUTE, too.


Done, switched to V4L2_CID_EXPOSURE. It's true, this control is not
taking 100 usec units, so unit-less is better.


Thanks. If you know the units, it would be of course better to use
right units...


Steve: what's the unit in this case? Is it lines or something else?


Yes, the register interface for exposure takes lines*16.

Maybe converting from seconds to lines is as simple as
framerate * height * seconds. But I'm not sure about that.

Steve



Pavel: we do need to make sure the user space will be able to know the unit,
too. It's rather a case with a number of controls: the unit is known but
there's no API to convey it to the user.

The exposure is a bit special, too: granularity matters a lot on small
values. On most other controls it does not.


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

Re: [PATCH] staging: rtl8723bs: fix a couple of spelling mistakes

[Joe Perches]

On Sat, 2017-06-03 at 23:21 +0100, Colin King wrote:
> Replace cant with cannot, argumetns with arguments and
> add line break to overly long DBG_871X statement.
[]
> diff --git a/drivers/staging/rtl8723bs/hal/hal_com.c 
> b/drivers/staging/rtl8723bs/hal/hal_com.c
[]
> @@ -26,7 +26,7 @@ u8 rtw_hal_data_init(struct adapter *padapter)
>   padapter->hal_data_sz = sizeof(struct hal_com_data);
>   padapter->HalData = vzalloc(padapter->hal_data_sz);
>   if (padapter->HalData == NULL) {
> - DBG_8192C("cant not alloc memory for HAL DATA\n");
> + DBG_8192C("cannot not alloc memory for HAL DATA\n");

"cannot not" does not make sense.
 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 4/4] Staging: ccree: cc_crypto_ctx.h: Added * on subsequent lines of a comment block

[srishti sharma]

Added *'s on subsequent lines of a comment block to fix coding style issues.

Signed-off-by: srishti sharma 
---
 drivers/staging/ccree/cc_crypto_ctx.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/ccree/cc_crypto_ctx.h 
b/drivers/staging/ccree/cc_crypto_ctx.h
index 03164624..a8747d5 100644
--- a/drivers/staging/ccree/cc_crypto_ctx.h
+++ b/drivers/staging/ccree/cc_crypto_ctx.h
@@ -281,9 +281,9 @@ struct drv_ctx_aead {


 /* Get the address of a @member within a given @ctx address
-   @ctx: The context address
-   @type: Type of context structure
-   @member: Associated context field */
+ * @ctx: The context address
+ * @type: Type of context structure
+ * @member: Associated context field */
 #define GET_CTX_FIELD_ADDR(ctx, type, member) (ctx + offsetof(type, member))

 #endif /* _CC_CRYPTO_CTX_H_ */
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/4] Staging: ccree: cc_crypto_ctx.h: Fixed * alignment issues in a comment block

[srishti sharma]

Fixed the alignment of * in a comment block.

Signed-off-by: srishti sharma 
---
 drivers/staging/ccree/cc_crypto_ctx.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/ccree/cc_crypto_ctx.h 
b/drivers/staging/ccree/cc_crypto_ctx.h
index 27a5914..03164624 100644
--- a/drivers/staging/ccree/cc_crypto_ctx.h
+++ b/drivers/staging/ccree/cc_crypto_ctx.h
@@ -242,9 +242,9 @@ struct drv_ctx_cipher {
u32 key_size; /* numeric value in bytes */
u32 data_unit_size; /* required for XTS */
/* block_state is the AES engine block state.
-   *  It is used by the host to pass IV or counter at initialization.
-   *  It is used by SeP for intermediate block chaining state and for
-   *  returning MAC algorithms results. */
+* It is used by the host to pass IV or counter at initialization.
+* It is used by SeP for intermediate block chaining state and for
+* returning MAC algorithms results. */
u8 block_state[CC_AES_BLOCK_SIZE];
u8 key[CC_AES_KEY_SIZE_MAX];
u8 xex_key[CC_AES_KEY_SIZE_MAX];
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/4] Staging: ccree: cc_crypto_ctx.h: Fixed trailing */ issue in a comment block

[srishti sharma]

Fixed trailing */ style issue in a block comment.

Signed-off-by: srishti sharma 
---
 drivers/staging/ccree/cc_crypto_ctx.h | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/ccree/cc_crypto_ctx.h 
b/drivers/staging/ccree/cc_crypto_ctx.h
index 6ee51b8..27a5914 100644
--- a/drivers/staging/ccree/cc_crypto_ctx.h
+++ b/drivers/staging/ccree/cc_crypto_ctx.h
@@ -218,8 +218,10 @@ struct drv_ctx_hash {
CC_DIGEST_SIZE_MAX];
 };

-/*  drv_ctx_hmac should have the same structure as drv_ctx_hash except
- * k0, k0_size fields */
+/*
+ *  drv_ctx_hmac should have the same structure as drv_ctx_hash except
+ * k0, k0_size fields
+ */
 struct drv_ctx_hmac {
enum drv_crypto_alg alg; /* DRV_CRYPTO_ALG_HMAC */
enum drv_hash_mode mode;
@@ -285,4 +287,3 @@ struct drv_ctx_aead {
 #define GET_CTX_FIELD_ADDR(ctx, type, member) (ctx + offsetof(type, member))

 #endif /* _CC_CRYPTO_CTX_H_ */
-
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 0/4] Fixed comment coding style issues.

[srishti sharma]

This patchset contains a series of comment coding style issues fixes.

srishti sharma (4):
  Staging: ccree: cc_crypto_ctx.h: Added * on subsequent lines of a
comment block.
  Staging: ccree: cc_crypto_ctx.h: Fixed trailing */ issue in a comment
block
  Staging: ccree: cc_crypto_ctx.h: Fixed * alignment issues in a comment
block
  Staging: ccree: cc_crypto_ctx.h: Added * on subsequent lines of a
comment block

 drivers/staging/ccree/cc_crypto_ctx.h | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

-- 
2.7.4

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

[PATCH][V2] staging: ccree: fix spelling mistake: "chanined" -> "chained"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in SSI_LOG_ERR message

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

diff --git a/drivers/staging/ccree/ssi_buffer_mgr.c 
b/drivers/staging/ccree/ssi_buffer_mgr.c
index 04515e70d2d3..379eecf85ba1 100644
--- a/drivers/staging/ccree/ssi_buffer_mgr.c
+++ b/drivers/staging/ccree/ssi_buffer_mgr.c
@@ -156,7 +156,7 @@ static unsigned int ssi_buffer_mgr_get_sgl_nents(
unsigned int nents = 0;
while (nbytes != 0) {
if (sg_is_chain(sg_list)) {
-   SSI_LOG_ERR("Unexpected chanined entry "
+   SSI_LOG_ERR("Unexpected chained entry "
   "in sg (entry =0x%X) \n", nents);
BUG();
}
-- 
2.11.0

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

[PATCH 1/4] Staging: ccree: cc_crypto_ctx.h: Added * on subsequent lines of a comment block.

[srishti sharma]

Added * on subsequent lines of a comment block.

Signed-off-by: srishti sharma 
---
 drivers/staging/ccree/cc_crypto_ctx.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ccree/cc_crypto_ctx.h 
b/drivers/staging/ccree/cc_crypto_ctx.h
index ac39d34..6ee51b8 100644
--- a/drivers/staging/ccree/cc_crypto_ctx.h
+++ b/drivers/staging/ccree/cc_crypto_ctx.h
@@ -219,7 +219,7 @@ struct drv_ctx_hash {
 };

 /*  drv_ctx_hmac should have the same structure as drv_ctx_hash except
-   k0, k0_size fields */
+ * k0, k0_size fields */
 struct drv_ctx_hmac {
enum drv_crypto_alg alg; /* DRV_CRYPTO_ALG_HMAC */
enum drv_hash_mode mode;
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] taging: ccree: fix spelling mistake: "chanined" -> "chained"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in SSI_LOG_ERR message

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

diff --git a/drivers/staging/ccree/ssi_buffer_mgr.c 
b/drivers/staging/ccree/ssi_buffer_mgr.c
index 04515e70d2d3..379eecf85ba1 100644
--- a/drivers/staging/ccree/ssi_buffer_mgr.c
+++ b/drivers/staging/ccree/ssi_buffer_mgr.c
@@ -156,7 +156,7 @@ static unsigned int ssi_buffer_mgr_get_sgl_nents(
unsigned int nents = 0;
while (nbytes != 0) {
if (sg_is_chain(sg_list)) {
-   SSI_LOG_ERR("Unexpected chanined entry "
+   SSI_LOG_ERR("Unexpected chained entry "
   "in sg (entry =0x%X) \n", nents);
BUG();
}
-- 
2.11.0

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

[PATCH] staging: rtl8723bs: fix another spelling mistake

[Colin King]

From: Colin Ian King 

I found one more spelling mistake in a DBG_8192C debug message,
replace "avaliable" with "available", add some spacing between
text and a number and split overly long line

Signed-off-by: Colin Ian King 
---
 drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c 
b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
index 163537faefd9..84a89ef74169 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
@@ -1741,7 +1741,8 @@ static u8 hal_EfusePgPacketWrite2ByteHeader(
 
efuse_addr = *pAddr;
if (efuse_addr >= efuse_max_available_len) {
-   DBG_8192C("%s: addr(%d) over avaliable(%d)!!\n", __func__, 
efuse_addr, efuse_max_available_len);
+   DBG_8192C("%s: addr(%d) over available (%d)!!\n", __func__,
+ efuse_addr, efuse_max_available_len);
return false;
}
 
-- 
2.11.0

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

[PATCH] staging: rtl8723bs: fix a couple of spelling mistakes

[Colin King]

From: Colin Ian King 

Replace cant with cannot, argumetns with arguments and
add line break to overly long DBG_871X statement.

Signed-off-by: Colin Ian King 
---
 drivers/staging/rtl8723bs/hal/hal_com.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/hal_com.c 
b/drivers/staging/rtl8723bs/hal/hal_com.c
index 1880d4140bee..c5b715753725 100644
--- a/drivers/staging/rtl8723bs/hal/hal_com.c
+++ b/drivers/staging/rtl8723bs/hal/hal_com.c
@@ -26,7 +26,7 @@ u8 rtw_hal_data_init(struct adapter *padapter)
padapter->hal_data_sz = sizeof(struct hal_com_data);
padapter->HalData = vzalloc(padapter->hal_data_sz);
if (padapter->HalData == NULL) {
-   DBG_8192C("cant not alloc memory for HAL DATA\n");
+   DBG_8192C("cannot not alloc memory for HAL DATA\n");
return _FAIL;
}
}
@@ -1415,7 +1415,8 @@ bool GetHexValueFromString(char *szStr, u32 *pu4bVal, u32 
*pu4bMove)
 
/*  Check input parameter. */
if (szStr == NULL || pu4bVal == NULL || pu4bMove == NULL) {
-   DBG_871X("GetHexValueFromString(): Invalid inpur argumetns! 
szStr: %p, pu4bVal: %p, pu4bMove: %p\n", szStr, pu4bVal, pu4bMove);
+   DBG_871X("GetHexValueFromString(): Invalid input arguments! 
szStr: %p, pu4bVal: %p, pu4bMove: %p\n",
+szStr, pu4bVal, pu4bMove);
return false;
}
 
-- 
2.11.0

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

Re: [PATCH v7 16/34] [media] add Omnivision OV5640 sensor driver

[Pavel Machek]

Hi!

> > > According to the docs V4L2_CID_EXPOSURE_ABSOLUTE is in 100 usec units.
> > > 
> > >  OTOH V4L2_CID_EXPOSURE has no defined unit, so it's a better fit IMO.
> > > >Way more drivers appear to be using EXPOSURE than EXPOSURE_ABSOLUTE, too.
> > > 
> > > Done, switched to V4L2_CID_EXPOSURE. It's true, this control is not
> > > taking 100 usec units, so unit-less is better.
> > 
> > Thanks. If you know the units, it would be of course better to use
> > right units...
> 
> Steve: what's the unit in this case? Is it lines or something else?
> 
> Pavel: we do need to make sure the user space will be able to know the unit,
> too. It's rather a case with a number of controls: the unit is known but
> there's no API to convey it to the user.
> 
> The exposure is a bit special, too: granularity matters a lot on small
> values. On most other controls it does not.

Yeah. Basically problem with exposure is that the control is
logarithmic; by using linear scale we got too much resolution at long
times and too little resolution at short times.

(Plus, 100 usec ... n900 can do times _way_ shorter than that.)

Anyway, even u32 gives us enough range, but I so the linear/log
confusion does not matter. But it would be nicer if values were in 10
usec or usec, not 100 usec... 

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 v7 16/34] [media] add Omnivision OV5640 sensor driver

[Sakari Ailus]

On Sat, Jun 03, 2017 at 09:51:39PM +0200, Pavel Machek wrote:
> Hi!
> 
> > >>>+/* Auto/manual exposure */
> > >>>+ctrls->auto_exp = v4l2_ctrl_new_std_menu(hdl, ops,
> > >>>+ V4L2_CID_EXPOSURE_AUTO,
> > >>>+ V4L2_EXPOSURE_MANUAL, 
> > >>>0,
> > >>>+ V4L2_EXPOSURE_AUTO);
> > >>>+ctrls->exposure = v4l2_ctrl_new_std(hdl, ops,
> > >>>+V4L2_CID_EXPOSURE_ABSOLUTE,
> > >>>+0, 65535, 1, 0);
> > >>
> > >>Is exposure_absolute supposed to be in microseconds...?
> > >
> > >Yes.
> > 
> > According to the docs V4L2_CID_EXPOSURE_ABSOLUTE is in 100 usec units.
> > 
> >  OTOH V4L2_CID_EXPOSURE has no defined unit, so it's a better fit IMO.
> > >Way more drivers appear to be using EXPOSURE than EXPOSURE_ABSOLUTE, too.
> > 
> > Done, switched to V4L2_CID_EXPOSURE. It's true, this control is not
> > taking 100 usec units, so unit-less is better.
> 
> Thanks. If you know the units, it would be of course better to use
> right units...

Steve: what's the unit in this case? Is it lines or something else?

Pavel: we do need to make sure the user space will be able to know the unit,
too. It's rather a case with a number of controls: the unit is known but
there's no API to convey it to the user.

The exposure is a bit special, too: granularity matters a lot on small
values. On most other controls it does not.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v7 16/34] [media] add Omnivision OV5640 sensor driver

[Pavel Machek]

Hi!

> >>>+  /* Auto/manual exposure */
> >>>+  ctrls->auto_exp = v4l2_ctrl_new_std_menu(hdl, ops,
> >>>+   V4L2_CID_EXPOSURE_AUTO,
> >>>+   V4L2_EXPOSURE_MANUAL, 0,
> >>>+   V4L2_EXPOSURE_AUTO);
> >>>+  ctrls->exposure = v4l2_ctrl_new_std(hdl, ops,
> >>>+  V4L2_CID_EXPOSURE_ABSOLUTE,
> >>>+  0, 65535, 1, 0);
> >>
> >>Is exposure_absolute supposed to be in microseconds...?
> >
> >Yes.
> 
> According to the docs V4L2_CID_EXPOSURE_ABSOLUTE is in 100 usec units.
> 
>  OTOH V4L2_CID_EXPOSURE has no defined unit, so it's a better fit IMO.
> >Way more drivers appear to be using EXPOSURE than EXPOSURE_ABSOLUTE, too.
> 
> Done, switched to V4L2_CID_EXPOSURE. It's true, this control is not
> taking 100 usec units, so unit-less is better.

Thanks. If you know the units, it would be of course better to use
right units...

Best regards,
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 v7 16/34] [media] add Omnivision OV5640 sensor driver

[Steve Longerbeam]

On 06/01/2017 01:26 AM, Sakari Ailus wrote:

Hi Pavel,

On Wed, May 31, 2017 at 09:58:21PM +0200, Pavel Machek wrote:

Hi!


+/* min/typical/max system clock (xclk) frequencies */
+#define OV5640_XCLK_MIN  600
+#define OV5640_XCLK_MAX 2400
+
+/*
+ * FIXME: there is no subdev API to set the MIPI CSI-2
+ * virtual channel yet, so this is hardcoded for now.
+ */
+#define OV5640_MIPI_VC 1


Can the FIXME be fixed?


Yes, but it's quite a bit of work. It makes sense to use a static virtual
channel for now. A patchset which is however incomplete can be found here:



For what it's worth, all other devices use virtual channel zero for image
data and so should this one.



Actually no. The CSI2IPU gasket in i.MX6 quad sends virtual channel 0
streams to IPU1-CSI0. But input to IPU1-CSI0 is also muxed with parallel
bus cameras. So if vc0 were chosen instead, platforms that support
parallel cameras to IPU1-CSI0 (SabreLite, SabreSD) would not be able
to use them concurrently with a MIPI CSI-2 source to IPU1-CSI1. So I
prefer to use static channel 1 to support those platforms.

I could convert this to a module parameter however, until a virtual
channel selection subdev API becomes available, at which point that
would have to be stripped.







+/*
+ * image size under 1280 * 960 are SUBSAMPLING


-> Image


+ * image size upper 1280 * 960 are SCALING


above?




done.


+/*
+ * FIXME: all of these register tables are likely filled with
+ * entries that set the register to their power-on default values,
+ * and which are otherwise not touched by this driver. Those entries
+ * should be identified and removed to speed register load time
+ * over i2c.
+ */


load->loading? Can the FIXME be fixed?


That's a lot of work, and risky work at that. If someone could take
this on (strip out power-on default values from the tables), I'd
be grateful, but I don't have the time. For now at least, these
registers sets work fine.





+   /* Auto/manual exposure */
+   ctrls->auto_exp = v4l2_ctrl_new_std_menu(hdl, ops,
+V4L2_CID_EXPOSURE_AUTO,
+V4L2_EXPOSURE_MANUAL, 0,
+V4L2_EXPOSURE_AUTO);
+   ctrls->exposure = v4l2_ctrl_new_std(hdl, ops,
+   V4L2_CID_EXPOSURE_ABSOLUTE,
+   0, 65535, 1, 0);


Is exposure_absolute supposed to be in microseconds...?


Yes.


According to the docs V4L2_CID_EXPOSURE_ABSOLUTE is in 100 usec units.

 OTOH V4L2_CID_EXPOSURE has no defined unit, so it's a better fit IMO.

Way more drivers appear to be using EXPOSURE than EXPOSURE_ABSOLUTE, too.


Done, switched to V4L2_CID_EXPOSURE. It's true, this control is not
taking 100 usec units, so unit-less is better.


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

Re: [PATCH v7 16/34] [media] add Omnivision OV5640 sensor driver

[Steve Longerbeam]

Hi Sakari,


On 05/29/2017 11:56 PM, Sakari Ailus wrote:

Hi Steve,

On Mon, May 29, 2017 at 02:50:34PM -0700, Steve Longerbeam wrote:




+
+static int ov5640_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+   struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
+   struct ov5640_dev *sensor = to_ov5640_dev(sd);
+   int ret = 0;
+
+   mutex_lock(>lock);

Could you use the same lock for the controls as you use for the rest? Just
setting handler->lock after handler init does the trick.


Can you please rephrase, I don't follow. "same lock for the controls as
you use for the rest" - there's only one device lock owned by this driver
and I am already using that same lock.


There's another in the control handler. You could use your own lock for the
control handler as well.


I still don't understand.









+
+static int ov5640_s_stream(struct v4l2_subdev *sd, int enable)
+{
+   struct ov5640_dev *sensor = to_ov5640_dev(sd);
+   int ret = 0;
+
+   mutex_lock(>lock);
+
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   if (sd->entity.stream_count > 1)

The entity stream_count isn't connected to the number of times s_stream(sd,
true) is called. Please remove the check.


It's incremented by media_pipeline_start(), even if the entity is already
a member of the given pipeline.

I added this check because in imx-media, the ov5640 can be streaming
concurrently to multiple video capture devices, and each capture device
calls
media_pipeline_start() at stream on, which increments the entity stream
count.

So if one capture device issues a stream off while others are still
streaming,
ov5640 should remain at stream on. So the entity stream count is being
used as a streaming usage counter. Is there a better way to do this? Should
I use a private stream use counter instead?


Different drivers may use media_pipeline_start() in different ways. Stream
control shouldn't depend on that count. This could cause issues in using the
driver with other ISP / receiver drivers.

I think it should be enough to move the check to the imx driver in this
case.



I will remove this check.



+
+static int ov5640_remove(struct i2c_client *client)
+{
+   struct v4l2_subdev *sd = i2c_get_clientdata(client);
+   struct ov5640_dev *sensor = to_ov5640_dev(sd);
+
+   regulator_bulk_disable(OV5640_NUM_SUPPLIES, sensor->supplies);

Ditto.


I don't understand. regulator_bulk_disable() is still needed, am I missing
something?


You still need to enable it first. I don't see that being done in probe. As
the driver implements the s_power() op, I don't see a need for powering the
device on at probe time (and conversely off at remove time).


Oh you're right, it must have been left over from a previous revision
I guess. Yes, regulator_bulk_enable|disable() is only called in
ov5640_set_power(). I'll remove regulator_bulk_disable() from
probe/remove.

Steve


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

Re: [PATCH] Staging: ccree: ssi_aead.h: Fixed a pointer declaration error.

[srishti sharma]

Hey,

checkpatch.pl generated two errors , because the dereferencing
operator was placed next to the structure name instead of being placed
with the pointer .

for eg:
  struct scatterlist* srcSgl; (this was giving an error)

whereas
 struct scatterlist *srcSgl; (this did not give an error)


Both of them will compile , but the second one is a better
representation of it and does not produce an error on running
checkpatch.pl .

Regards,
Srishti

On Sat, Jun 3, 2017 at 2:07 PM, Greg KH  wrote:
> On Sat, Jun 03, 2017 at 04:15:17AM +0530, srishti sharma wrote:
>> Fixed a pointer declaration error , the dereferencing operator was misplaced.
>
> Odd use of a ,
>
> Also, I don't understand what was "misplaced" here?  There does not seem
> to be a "error" fixed here...
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH V2 00/27] Drivers: ccree - align block comments

[Greg KH]

On Tue, May 30, 2017 at 06:09:37PM +1200, Derek Robson wrote:
> Fixed block comments across whole ccree driver
> 
> Version #1 has some trailing white space issue in a few patches

Some of these applied, some did not :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] rts5208: Fix a sleep-in-atomic bug in rtsx_exclusive_enter_ss

[Greg KH]

On Thu, Jun 01, 2017 at 11:43:35AM +0800, Jia-Ju Bai wrote:
> The driver may sleep under a spin lock, and the function call path is:
> rtsx_exclusive_enter_ss (acquire the lock by spin_lock)
>   rtsx_enter_ss
> rtsx_power_off_card
>   sd_cleanup_work
> sd_stop_seq_mode
>   sd_switch_clock
> sd_ddr_tuning
>   sd_ddr_pre_tuning_tx
> sd_change_phase
>   wait_timeout
> schedule_timeout --> may sleep
> 
> To fix it, "wait_timeout" is replaced with mdelay in sd_change_phase.

Nice work, how are you finding these bugs?  What tools gives you this
kind of analysis?

thanks,

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

Re: [PATCH V2 01/27] Drivers: ccree: ssi_sysfs.h - align block comments

[Greg KH]

On Tue, May 30, 2017 at 06:09:54PM +1200, Derek Robson wrote:
> Fixed block comment alignment, Style fix only
> Found using checkpatch
> 
> Signed-off-by: Derek Robson 
> ---
>  drivers/staging/ccree/ssi_sysfs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Does not apply to my tree :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Restrict argument of tcpci_read16

[gre...@linuxfoundation.org]

On Fri, Jun 02, 2017 at 02:59:32AM +, ? ? wrote:
> From: Pan Li 
> 
> Pan Li (1):
>   Restrict read pointer to 16 bit of tcpci_read16.

Please fix up your subject line to properly describe what part of the
kernel you are modifying.

thanks,

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

Re: [PATCH v4 10/10] staging: fsl-mc: move bus driver out of staging

[Greg KH]

On Wed, May 31, 2017 at 01:58:52PM +0300, laurentiu.tu...@nxp.com wrote:
> From: Stuart Yoder 

Your subject says 'v4' yet the other patches here in the series do not,
they say nothing, or 'v2', and the 0/10 patch says 'v6'!

Please fix up and do this correctly.  The _whole_ patch series is
versioned, otherwise how can I sort them to apply them in the correct
order?

thanks,

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

Re: [PATCH V2] libcfs: Fix a sleep-in-atomic bug in cfs_wi_exit

[Greg KH]

On Wed, May 31, 2017 at 04:00:41PM +0800, Jia-Ju Bai wrote:
> The driver may sleep under a spin lock, and the function call path is:
> cfs_wi_exit (acquire the lock by spin_lock)
>   LASSERT
> lbug_with_loc
>   libcfs_debug_dumplog
> schedule and kthread_run --> may sleep
> 
> To fix it, all "LASSERT" is placed out of the spin_lock and spin_unlock.
> 
> Signed-off-by: Jia-Ju Bai 
> ---
>  drivers/staging/lustre/lnet/libcfs/workitem.c |   13 +++--
>  1 file changed, 7 insertions(+), 6 deletions(-)

What changed from v1?  You have to always tell us that.

Please redo all of these, if they are still valid patches, and send them
as a patch series, so I know what order they should be applied in.

thanks,

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

Re: [PATCH v2 01/11] Staging: android: Fix style issue in ion-ioctl.c

[Greg KH]

On Mon, May 29, 2017 at 11:58:52PM +0200, Mateusz Nowotyński wrote:
> This is patch fixing code alignment style issue in
> android/ion/ion-ioctl.c file
> 
> Signed-off-by: Mateusz Nowotyński 
> ---
>  drivers/staging/android/ion/ion-ioctl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

This series does not apply to my tree at all, please rebase and fix up
and resend.

thanks,

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

Re: [PATCH v2 02/20] staging: ccree: replace bit shift with BIT macro

[Greg Kroah-Hartman]

On Thu, Jun 01, 2017 at 02:02:52PM +0300, Gilad Ben-Yossef wrote:
> CC_CTX_SIZE was being defined using a hand rolled bit shift operation.
> Replace with use of BIT macro.
> 
> Signed-off-by: Gilad Ben-Yossef 
> ---
>  drivers/staging/ccree/cc_crypto_ctx.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

This patch didn't apply at all, are you sure you are working against the
correct tree?  I've tried to apply some of this series, but most do not
work at all...

Please rebase against my staging-next branch and resend the ones that do
work.

thanks,

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

Re: [PATCH 01/10] staging: fsl-mc: enclose macro params in parens

[Greg KH]

On Wed, May 31, 2017 at 01:58:43PM +0300, laurentiu.tu...@nxp.com wrote:
> From: Laurentiu Tudor 
> 
> Several macros didn't had macro params enclosed
> in parens. Fix them to avoid precedence issues.
> 
> Found with checkpatch.pl who was issuing this
> message:
>   "Macro argument 'id' may be better as '(id)'
>to avoid precedence issues"

Why are you line-wrapping at such a small number?  Please use 72 columns
like git asks you to...

thanks,

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

Re: [PATCH] Staging: ccree: cc_crypto_ctx.h: Fixed a comment coding style issue.

[Greg KH]

On Wed, May 31, 2017 at 06:24:55PM +0530, srishti sharma wrote:
> Fixed a comment coding style issue that generated a warning stating that 
> block comments should align the * on each line.
> 

Please properly line-wrap your changelog comments :(

You also sent 2 different patches, that did different things, yet they
had the same subject line.  That's not ok, and also, I have no idea
which one to apply first.

I'm dropping both, please fix and resend them properly as a patch
series.

thanks,

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

Re: [PATCH] drivers: staging: lustre: lustre: llite: file.c - fixed sparse warning about different fmode_t type

[Greg Kroah-Hartman]

On Mon, Apr 24, 2017 at 11:28:58AM +0100, Andrea della Porta wrote:
> Fixed the following sparse warning:
> 
>   CHECK   drivers/staging/lustre/lustre/llite//file.c
>   drivers/staging/lustre/lustre/llite//file.c:441:24: warning: incorrect
>   type in assignment (different base types)
>   drivers/staging/lustre/lustre/llite//file.c:441:24:expected
>   restricted fmode_t [usertype] och_flags
>   drivers/staging/lustre/lustre/llite//file.c:441:24:got unsigned
>   long long [unsigned] [usertype] it_flags
>   drivers/staging/lustre/lustre/llite//file.c:465:65: warning:
>   restricted fmode_t degrades to integer
>   drivers/staging/lustre/lustre/llite//file.c:465:22: warning: incorrect
>   type in assignment (different base types)
>   drivers/staging/lustre/lustre/llite//file.c:465:22:expected
>   restricted fmode_t [usertype] fd_omode
>   drivers/staging/lustre/lustre/llite//file.c:465:22:got unsigned
>   long long
>   drivers/staging/lustre/lustre/llite//file.c:526:38: warning: invalid
>   assignment: |=
>   drivers/staging/lustre/lustre/llite//file.c:526:38:left side has
>   type unsigned long long
>   drivers/staging/lustre/lustre/llite//file.c:526:38:right side has
>   type restricted fmode_t
>   drivers/staging/lustre/lustre/llite//file.c:533:49: warning:
>   restricted fmode_t degrades to integer
>   drivers/staging/lustre/lustre/llite//file.c:553:28: warning:
>   restricted fmode_t degrades to integer
>   drivers/staging/lustre/lustre/llite//file.c:556:35: warning:
>   restricted fmode_t degrades to integer
>   drivers/staging/lustre/lustre/llite//file.c:778:23: warning:
>   restricted fmode_t degrades to integer
>   drivers/staging/lustre/lustre/llite//file.c:1309:62: warning:
>   restricted fmode_t degrades to integer
>   drivers/staging/lustre/lustre/llite//file.c:1357:23: warning:
>   incorrect type in initializer (different base types)
>   drivers/staging/lustre/lustre/llite//file.c:1357:23:expected
>   unsigned long long [unsigned] [usertype] flags
>   drivers/staging/lustre/lustre/llite//file.c:1357:23:got restricted
>   fmode_t [usertype] 

Please don't line-wrap messages like this so that we can see them
easier.

> Signed-off-by: Andrea della Porta 
> ---
>  drivers/staging/lustre/lustre/include/lustre_intent.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/lustre/lustre/include/lustre_intent.h 
> b/drivers/staging/lustre/lustre/include/lustre_intent.h
> index ed2b6c6..c036633 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_intent.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_intent.h
> @@ -38,7 +38,7 @@
>  struct lookup_intent {
>   int it_op;
>   int it_create_mode;
> - __u64   it_flags;
> + fmode_t it_flags;

This adds a bunch of new build warnings when appied, are you sure you
test-built your code?  Always do so, I can't take this patch as-is :(

thanks,

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

Re: [PATCH] Staging: ccree: ssi_aead.h: Fixed a pointer declaration error.

[Greg KH]

On Sat, Jun 03, 2017 at 04:15:17AM +0530, srishti sharma wrote:
> Fixed a pointer declaration error , the dereferencing operator was misplaced.

Odd use of a ,

Also, I don't understand what was "misplaced" here?  There does not seem
to be a "error" fixed here...

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

Re: [PATCH] staging: ks7010: define ether_hdr.h_proto to be big-endian

[Greg KH]

On Sun, May 21, 2017 at 10:15:11AM +0100, d...@acm.org wrote:
> From: Richard Porter 
> 
> Fixes sparse warnings:
> drivers/staging/ks7010/ks_hostif.c:339:21: warning: cast to restricted __be16
> drivers/staging/ks7010/ks_hostif.c:430:21: warning: cast to restricted __be16
> drivers/staging/ks7010/ks_hostif.c:1226:21: warning: cast to restricted __be16
> 
> Signed-off-by: Richard Porter 
> ---
>  drivers/staging/ks7010/eap_packet.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Does not apply to my tree at all :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] Staging: ccree: ssi_aead.h: Fixed a comment coding style issue.

[srishti sharma]

Fixed a comment coding style issue , block comments use * on subsequent lines.

Signed-off-by: srishti sharma 
---
 drivers/staging/ccree/ssi_aead.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ccree/ssi_aead.h b/drivers/staging/ccree/ssi_aead.h
index 9cf5225..4b592b7 100644
--- a/drivers/staging/ccree/ssi_aead.h
+++ b/drivers/staging/ccree/ssi_aead.h
@@ -14,8 +14,9 @@
  * along with this program; if not, see .
  */

-/* \file ssi_aead.h
-   ARM CryptoCell AEAD Crypto API
+/*
+ * \file ssi_aead.h
+ *  ARM CryptoCell AEAD Crypto API
  */

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