Re: [PATCH] media: lgdt330x: fix lock status reporting

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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