linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] media: radio: Tuning bugfix for si470x over i2c
@ 2018-01-18 20:01 Douglas Fischer
  0 siblings, 0 replies; 3+ messages in thread
From: Douglas Fischer @ 2018-01-18 20:01 UTC (permalink / raw)
  To: linux-media

Fixed si470x_set_channel() trying to tune before chip is turned
on, which causes warnings in dmesg and when probing, makes driver
wait for 3s for tuning timeout. This issue did not affect USB
devices because they have a different probing sequence.

Signed-off-by: Douglas Fischer <fischerdouglasc@gmail.com>
---

diff -uprN linux.orig/drivers/media/radio/si470x/radio-si470x-common.c
linux/drivers/media/radio/si470x/radio-si470x-common.c ---
linux.orig/drivers/media/radio/si470x/radio-si470x-common.c
2018-01-15 21:58:10.675620432 -0500 +++
linux/drivers/media/radio/si470x/radio-si470x-common.c
2018-01-16 17:04:59.706409128 -0500 @@ -207,29 +207,37 @@ static int
si470x_set_chan(struct si470x unsigned long time_left; bool timed_out =
false; 
-	/* start tuning */
-	radio->registers[CHANNEL] &= ~CHANNEL_CHAN;
-	radio->registers[CHANNEL] |= CHANNEL_TUNE | chan;
-	retval = si470x_set_register(radio, CHANNEL);
-	if (retval < 0)
-		goto done;
+	retval = si470x_get_register(radio, POWERCFG);
+	if (retval)
+		return retval;
 
-	/* wait till tune operation has completed */
-	reinit_completion(&radio->completion);
-	time_left = wait_for_completion_timeout(&radio->completion,
-
msecs_to_jiffies(tune_timeout));
-	if (time_left == 0)
-		timed_out = true;
+	if ( (radio->registers[POWERCFG] & POWERCFG_ENABLE) && 
+		(radio->registers[POWERCFG] & POWERCFG_DMUTE) ) { 
 
-	if ((radio->registers[STATUSRSSI] & STATUSRSSI_STC) == 0)
-		dev_warn(&radio->videodev.dev, "tune does not
complete\n");
-	if (timed_out)
-		dev_warn(&radio->videodev.dev,
-			"tune timed out after %u ms\n", tune_timeout);
+		/* start tuning */
+		radio->registers[CHANNEL] &= ~CHANNEL_CHAN;
+		radio->registers[CHANNEL] |= CHANNEL_TUNE | chan;
+		retval = si470x_set_register(radio, CHANNEL);
+		if (retval < 0)
+			goto done;
 
-	/* stop tuning */
-	radio->registers[CHANNEL] &= ~CHANNEL_TUNE;
-	retval = si470x_set_register(radio, CHANNEL);
+		/* wait till tune operation has completed */
+		reinit_completion(&radio->completion);
+		time_left =
wait_for_completion_timeout(&radio->completion,
+
msecs_to_jiffies(tune_timeout));
+		if (time_left == 0)
+			timed_out = true;
+	
+		if ((radio->registers[STATUSRSSI] & STATUSRSSI_STC) ==
0)
+			dev_warn(&radio->videodev.dev, "tune does not
complete\n");
+		if (timed_out)
+			dev_warn(&radio->videodev.dev,
+				"tune timed out after %u ms\n",
tune_timeout);
+	
+		/* stop tuning */
+		radio->registers[CHANNEL] &= ~CHANNEL_TUNE;
+		retval = si470x_set_register(radio, CHANNEL);
+	}
 
 done:
 	return retval;

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

* Re: [PATCH] media: radio: Tuning bugfix for si470x over i2c
  2018-01-20 19:19 Douglas Fischer
@ 2018-02-15 14:29 ` Hans Verkuil
  0 siblings, 0 replies; 3+ messages in thread
From: Hans Verkuil @ 2018-02-15 14:29 UTC (permalink / raw)
  To: Douglas Fischer, linux-media

Hi Douglas,

My apologies for the delay, but I have finally time to look at this.

First of all: all your patches are mangles. Your mailer probably is trying
to wrap around long lines and the end result is not usable. Please check this
next time.

Also, when you post newer versions of patches it is good practice to add a
version number: e.g. '[PATCHv3] media: radio: Tuning bugfix for si470x over i2c'.

That helps us keeping track of different versions.

On 20/01/18 20:19, Douglas Fischer wrote:
> Fixed si470x_set_channel() trying to tune before chip is turned
> on, which causes warnings in dmesg and when probing, makes driver
> wait for 3s for tuning timeout. This issue did not affect USB
> devices because they have a different probing sequence.
> 
> Signed-off-by: Douglas Fischer <fischerdouglasc@gmail.com>
> ---
> 
> diff -uprN linux.orig/drivers/media/radio/si470x/radio-si470x-common.c
> linux/drivers/media/radio/si470x/radio-si470x-common.c ---
> linux.orig/drivers/media/radio/si470x/radio-si470x-common.c
> 2018-01-15 21:58:10.675620432 -0500 +++
> linux/drivers/media/radio/si470x/radio-si470x-common.c
> 2018-01-16 17:04:59.706409128 -0500 @@ -207,29 +207,37 @@ static int
> si470x_set_chan(struct si470x unsigned long time_left; bool timed_out =
> false; 
> -	/* start tuning */
> -	radio->registers[CHANNEL] &= ~CHANNEL_CHAN;
> -	radio->registers[CHANNEL] |= CHANNEL_TUNE | chan;
> -	retval = si470x_set_register(radio, CHANNEL);
> -	if (retval < 0)
> -		goto done;
> +	retval = si470x_get_register(radio, POWERCFG);
> +	if (retval)
> +		return retval;
>  
> -	/* wait till tune operation has completed */
> -	reinit_completion(&radio->completion);
> -	time_left = wait_for_completion_timeout(&radio->completion,
> -
> msecs_to_jiffies(tune_timeout));
> -	if (time_left == 0)
> -		timed_out = true;
> +	if ( (radio->registers[POWERCFG] & POWERCFG_ENABLE) && 
> +		(radio->registers[POWERCFG] & POWERCFG_DMUTE) ) { 
>  

Just do:

	if (radio->registers[POWERCFG] & POWERCFG_ENABLE) & (POWERCFG_ENABLE | POWERCFG_DMUTE) !=
	    POWERCFG_ENABLE | POWERCFG_DMUTE)
		return 0;

And the remainder of the code can be indented one tab to the left. It's easier to read
and the diff is also smaller.

Regards,

	Hans

> -	if ((radio->registers[STATUSRSSI] & STATUSRSSI_STC) == 0)
> -		dev_warn(&radio->videodev.dev, "tune does not
> complete\n");
> -	if (timed_out)
> -		dev_warn(&radio->videodev.dev,
> -			"tune timed out after %u ms\n", tune_timeout);
> +		/* start tuning */
> +		radio->registers[CHANNEL] &= ~CHANNEL_CHAN;
> +		radio->registers[CHANNEL] |= CHANNEL_TUNE | chan;
> +		retval = si470x_set_register(radio, CHANNEL);
> +		if (retval < 0)
> +			goto done;
>  
> -	/* stop tuning */
> -	radio->registers[CHANNEL] &= ~CHANNEL_TUNE;
> -	retval = si470x_set_register(radio, CHANNEL);
> +		/* wait till tune operation has completed */
> +		reinit_completion(&radio->completion);
> +		time_left =
> wait_for_completion_timeout(&radio->completion,
> +
> msecs_to_jiffies(tune_timeout));
> +		if (time_left == 0)
> +			timed_out = true;
> +	
> +		if ((radio->registers[STATUSRSSI] & STATUSRSSI_STC) ==
> 0)
> +			dev_warn(&radio->videodev.dev, "tune does not
> complete\n");
> +		if (timed_out)
> +			dev_warn(&radio->videodev.dev,
> +				"tune timed out after %u ms\n",
> tune_timeout);
> +	
> +		/* stop tuning */
> +		radio->registers[CHANNEL] &= ~CHANNEL_TUNE;
> +		retval = si470x_set_register(radio, CHANNEL);
> +	}
>  
>  done:
>  	return retval;
> 

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

* [PATCH] media: radio: Tuning bugfix for si470x over i2c
@ 2018-01-20 19:19 Douglas Fischer
  2018-02-15 14:29 ` Hans Verkuil
  0 siblings, 1 reply; 3+ messages in thread
From: Douglas Fischer @ 2018-01-20 19:19 UTC (permalink / raw)
  To: hverkuil, linux-media

Fixed si470x_set_channel() trying to tune before chip is turned
on, which causes warnings in dmesg and when probing, makes driver
wait for 3s for tuning timeout. This issue did not affect USB
devices because they have a different probing sequence.

Signed-off-by: Douglas Fischer <fischerdouglasc@gmail.com>
---

diff -uprN linux.orig/drivers/media/radio/si470x/radio-si470x-common.c
linux/drivers/media/radio/si470x/radio-si470x-common.c ---
linux.orig/drivers/media/radio/si470x/radio-si470x-common.c
2018-01-15 21:58:10.675620432 -0500 +++
linux/drivers/media/radio/si470x/radio-si470x-common.c
2018-01-16 17:04:59.706409128 -0500 @@ -207,29 +207,37 @@ static int
si470x_set_chan(struct si470x unsigned long time_left; bool timed_out =
false; 
-	/* start tuning */
-	radio->registers[CHANNEL] &= ~CHANNEL_CHAN;
-	radio->registers[CHANNEL] |= CHANNEL_TUNE | chan;
-	retval = si470x_set_register(radio, CHANNEL);
-	if (retval < 0)
-		goto done;
+	retval = si470x_get_register(radio, POWERCFG);
+	if (retval)
+		return retval;
 
-	/* wait till tune operation has completed */
-	reinit_completion(&radio->completion);
-	time_left = wait_for_completion_timeout(&radio->completion,
-
msecs_to_jiffies(tune_timeout));
-	if (time_left == 0)
-		timed_out = true;
+	if ( (radio->registers[POWERCFG] & POWERCFG_ENABLE) && 
+		(radio->registers[POWERCFG] & POWERCFG_DMUTE) ) { 
 
-	if ((radio->registers[STATUSRSSI] & STATUSRSSI_STC) == 0)
-		dev_warn(&radio->videodev.dev, "tune does not
complete\n");
-	if (timed_out)
-		dev_warn(&radio->videodev.dev,
-			"tune timed out after %u ms\n", tune_timeout);
+		/* start tuning */
+		radio->registers[CHANNEL] &= ~CHANNEL_CHAN;
+		radio->registers[CHANNEL] |= CHANNEL_TUNE | chan;
+		retval = si470x_set_register(radio, CHANNEL);
+		if (retval < 0)
+			goto done;
 
-	/* stop tuning */
-	radio->registers[CHANNEL] &= ~CHANNEL_TUNE;
-	retval = si470x_set_register(radio, CHANNEL);
+		/* wait till tune operation has completed */
+		reinit_completion(&radio->completion);
+		time_left =
wait_for_completion_timeout(&radio->completion,
+
msecs_to_jiffies(tune_timeout));
+		if (time_left == 0)
+			timed_out = true;
+	
+		if ((radio->registers[STATUSRSSI] & STATUSRSSI_STC) ==
0)
+			dev_warn(&radio->videodev.dev, "tune does not
complete\n");
+		if (timed_out)
+			dev_warn(&radio->videodev.dev,
+				"tune timed out after %u ms\n",
tune_timeout);
+	
+		/* stop tuning */
+		radio->registers[CHANNEL] &= ~CHANNEL_TUNE;
+		retval = si470x_set_register(radio, CHANNEL);
+	}
 
 done:
 	return retval;

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

end of thread, other threads:[~2018-02-15 14:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-18 20:01 [PATCH] media: radio: Tuning bugfix for si470x over i2c Douglas Fischer
2018-01-20 19:19 Douglas Fischer
2018-02-15 14:29 ` Hans Verkuil

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).