linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] media: lgdt330x: fix lock status reporting
@ 2018-12-09  7:11 French, Nicholas A.
  2018-12-09 12:30 ` Laurent Pinchart
  2018-12-10 17:50 ` Adam Stylinski
  0 siblings, 2 replies; 3+ messages in thread
From: French, Nicholas A. @ 2018-12-09  7:11 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Laurent Pinchart, David Airlie, linux-media
  Cc: Adam Stylinski

A typo in code cleanup commit db9c1007bc07 ("media: lgdt330x: do
some cleanups at status logic") broke the FE_HAS_LOCK reporting
for 3303 chips by inadvertently modifying the register mask.

The broken lock status is critial as it prevents video capture
cards from reporting signal strength, scanning for channels,
and capturing video.

Fix regression by reverting mask change.

Signed-off-by: Nick French <naf@ou.edu>
---
 drivers/media/dvb-frontends/lgdt330x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/lgdt330x.c b/drivers/media/dvb-frontends/lgdt330x.c
index 96807e134886..8abb1a510a81 100644
--- a/drivers/media/dvb-frontends/lgdt330x.c
+++ b/drivers/media/dvb-frontends/lgdt330x.c
@@ -783,7 +783,7 @@ static int lgdt3303_read_status(struct dvb_frontend *fe,
 
 		if ((buf[0] & 0x02) == 0x00)
 			*status |= FE_HAS_SYNC;
-		if ((buf[0] & 0xfd) == 0x01)
+		if ((buf[0] & 0x01) == 0x01)
 			*status |= FE_HAS_VITERBI | FE_HAS_LOCK;
 		break;
 	default:
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] media: lgdt330x: fix lock status reporting
  2018-12-09  7:11 [PATCH] media: lgdt330x: fix lock status reporting French, Nicholas A.
@ 2018-12-09 12:30 ` Laurent Pinchart
  2018-12-10 17:50 ` Adam Stylinski
  1 sibling, 0 replies; 3+ messages in thread
From: Laurent Pinchart @ 2018-12-09 12:30 UTC (permalink / raw)
  To: French, Nicholas A.
  Cc: Mauro Carvalho Chehab, David Airlie, linux-media, Adam Stylinski

Hello Nick,

Thank you for the patch.

On Sunday, 9 December 2018 09:11:18 EET French, Nicholas A. wrote:
> A typo in code cleanup commit db9c1007bc07 ("media: lgdt330x: do
> some cleanups at status logic") broke the FE_HAS_LOCK reporting
> for 3303 chips by inadvertently modifying the register mask.
> 
> The broken lock status is critial as it prevents video capture
> cards from reporting signal strength, scanning for channels,
> and capturing video.
> 
> Fix regression by reverting mask change.
> 
> Signed-off-by: Nick French <naf@ou.edu>

This looks good to me. The patch should have a

Fixes: db9c1007bc07 ("media: lgdt330x: do some cleanups at status logic")

line though. With that added,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

The patch that broke this was committed without any review. Once again we get 
a proof that this isn't an acceptable policy. The review process needs to be 
fixed.

> ---
>  drivers/media/dvb-frontends/lgdt330x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/dvb-frontends/lgdt330x.c
> b/drivers/media/dvb-frontends/lgdt330x.c index 96807e134886..8abb1a510a81
> 100644
> --- a/drivers/media/dvb-frontends/lgdt330x.c
> +++ b/drivers/media/dvb-frontends/lgdt330x.c
> @@ -783,7 +783,7 @@ static int lgdt3303_read_status(struct dvb_frontend *fe,
> 
>  		if ((buf[0] & 0x02) == 0x00)
>  			*status |= FE_HAS_SYNC;
> -		if ((buf[0] & 0xfd) == 0x01)
> +		if ((buf[0] & 0x01) == 0x01)
>  			*status |= FE_HAS_VITERBI | FE_HAS_LOCK;
>  		break;
>  	default:

-- 
Regards,

Laurent Pinchart




^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] media: lgdt330x: fix lock status reporting
  2018-12-09  7:11 [PATCH] media: lgdt330x: fix lock status reporting French, Nicholas A.
  2018-12-09 12:30 ` Laurent Pinchart
@ 2018-12-10 17:50 ` Adam Stylinski
  1 sibling, 0 replies; 3+ messages in thread
From: Adam Stylinski @ 2018-12-10 17:50 UTC (permalink / raw)
  To: French, Nicholas A.; +Cc: linux-media

[-- Attachment #1: Type: text/plain, Size: 1264 bytes --]

On Sun, Dec 09, 2018 at 07:11:18AM +0000, French, Nicholas A. wrote:
> A typo in code cleanup commit db9c1007bc07 ("media: lgdt330x: do
> some cleanups at status logic") broke the FE_HAS_LOCK reporting
> for 3303 chips by inadvertently modifying the register mask.
> 
> The broken lock status is critial as it prevents video capture
> cards from reporting signal strength, scanning for channels,
> and capturing video.
> 
> Fix regression by reverting mask change.
> 
> Signed-off-by: Nick French <naf@ou.edu>
> ---
>  drivers/media/dvb-frontends/lgdt330x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/dvb-frontends/lgdt330x.c b/drivers/media/dvb-frontends/lgdt330x.c
> index 96807e134886..8abb1a510a81 100644
> --- a/drivers/media/dvb-frontends/lgdt330x.c
> +++ b/drivers/media/dvb-frontends/lgdt330x.c
> @@ -783,7 +783,7 @@ static int lgdt3303_read_status(struct dvb_frontend *fe,
>  
>  		if ((buf[0] & 0x02) == 0x00)
>  			*status |= FE_HAS_SYNC;
> -		if ((buf[0] & 0xfd) == 0x01)
> +		if ((buf[0] & 0x01) == 0x01)
>  			*status |= FE_HAS_VITERBI | FE_HAS_LOCK;
>  		break;
>  	default:
> -- 
> 2.19.2
> 

Patch worked for me.

Tested-by: Adam Stylinski <kungfujesus06@gmail.com>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-12-10 17:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-09  7:11 [PATCH] media: lgdt330x: fix lock status reporting French, Nicholas A.
2018-12-09 12:30 ` Laurent Pinchart
2018-12-10 17:50 ` Adam Stylinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).