Re: [PATCH v3 4/4] stk1160: Give the chip some time to retrieve data from AC97 codec.

[Ezequiel Garcia]

On 2 December 2016 at 08:09, Mauro Carvalho Chehab
 wrote:
> Em Sun, 27 Nov 2016 12:12:36 +0100
> Marcel Hasler  escreveu:
>
>> The STK1160 needs some time to transfer data from the AC97 registers into 
>> its own. On some
>> systems reading the chip's own registers to soon will return wrong values. 
>> The "proper" way to
>> handle this would be to poll STK1160_AC97CTL_0 after every read or write 
>> command until the
>> command bit has been cleared, but this may not be worth the hassle.
>>
>> Signed-off-by: Marcel Hasler 
>> ---
>>  drivers/media/usb/stk1160/stk1160-ac97.c | 9 +
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/media/usb/stk1160/stk1160-ac97.c 
>> b/drivers/media/usb/stk1160/stk1160-ac97.c
>> index 60327af..b39f51b 100644
>> --- a/drivers/media/usb/stk1160/stk1160-ac97.c
>> +++ b/drivers/media/usb/stk1160/stk1160-ac97.c
>> @@ -23,6 +23,7 @@
>>   *
>>   */
>>
>> +#include 
>>  #include 
>>
>>  #include "stk1160.h"
>> @@ -64,6 +65,14 @@ static u16 stk1160_read_ac97(struct stk1160 *dev, u16 reg)
>>*/
>>   stk1160_write_reg(dev, STK1160_AC97CTL_0, 0x8b);
>>
>> + /*
>> +  * Give the chip some time to transfer the data.
>> +  * The proper way would be to poll STK1160_AC97CTL_0
>> +  * until the command bit has been cleared, but this
>> +  * may not be worth the hassle.
>
> Why not? Relying on a fixed amount time is not nice.
>
> Take a look at em28xx_is_ac97_ready() function, at
> drivers/media/usb/em28xx/em28xx-core.c to see how this could be
> implemented instead.
>

We were reluctant to implement this properly because the read reg
function is only used for debugging purposes, to dump registers.

That said, it's not too much of a hassle to do it properly, and
we might have to if we are going to have our own mixer.

>
>> +  */
>> + usleep_range(20, 40);
>> +
>
>>   /* Retrieve register value */
>>   stk1160_read_reg(dev, STK1160_AC97_CMD, );
>>   stk1160_read_reg(dev, STK1160_AC97_CMD + 1, );
>
>
>
> Thanks,
> Mauro



-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/4] stk1160: Give the chip some time to retrieve data from AC97 codec.

[Mauro Carvalho Chehab]

Em Sun, 27 Nov 2016 12:12:36 +0100
Marcel Hasler  escreveu:

> The STK1160 needs some time to transfer data from the AC97 registers into its 
> own. On some
> systems reading the chip's own registers to soon will return wrong values. 
> The "proper" way to
> handle this would be to poll STK1160_AC97CTL_0 after every read or write 
> command until the
> command bit has been cleared, but this may not be worth the hassle.
> 
> Signed-off-by: Marcel Hasler 
> ---
>  drivers/media/usb/stk1160/stk1160-ac97.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/media/usb/stk1160/stk1160-ac97.c 
> b/drivers/media/usb/stk1160/stk1160-ac97.c
> index 60327af..b39f51b 100644
> --- a/drivers/media/usb/stk1160/stk1160-ac97.c
> +++ b/drivers/media/usb/stk1160/stk1160-ac97.c
> @@ -23,6 +23,7 @@
>   *
>   */
>  
> +#include 
>  #include 
>  
>  #include "stk1160.h"
> @@ -64,6 +65,14 @@ static u16 stk1160_read_ac97(struct stk1160 *dev, u16 reg)
>*/
>   stk1160_write_reg(dev, STK1160_AC97CTL_0, 0x8b);
>  
> + /*
> +  * Give the chip some time to transfer the data.
> +  * The proper way would be to poll STK1160_AC97CTL_0
> +  * until the command bit has been cleared, but this
> +  * may not be worth the hassle.

Why not? Relying on a fixed amount time is not nice.

Take a look at em28xx_is_ac97_ready() function, at
drivers/media/usb/em28xx/em28xx-core.c to see how this could be
implemented instead.


> +  */
> + usleep_range(20, 40);
> +

>   /* Retrieve register value */
>   stk1160_read_reg(dev, STK1160_AC97_CMD, );
>   stk1160_read_reg(dev, STK1160_AC97_CMD + 1, );



Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 4/4] stk1160: Give the chip some time to retrieve data from AC97 codec.

[Marcel Hasler]

The STK1160 needs some time to transfer data from the AC97 registers into its 
own. On some
systems reading the chip's own registers to soon will return wrong values. The 
"proper" way to
handle this would be to poll STK1160_AC97CTL_0 after every read or write 
command until the
command bit has been cleared, but this may not be worth the hassle.

Signed-off-by: Marcel Hasler 
---
 drivers/media/usb/stk1160/stk1160-ac97.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/usb/stk1160/stk1160-ac97.c 
b/drivers/media/usb/stk1160/stk1160-ac97.c
index 60327af..b39f51b 100644
--- a/drivers/media/usb/stk1160/stk1160-ac97.c
+++ b/drivers/media/usb/stk1160/stk1160-ac97.c
@@ -23,6 +23,7 @@
  *
  */
 
+#include 
 #include 
 
 #include "stk1160.h"
@@ -64,6 +65,14 @@ static u16 stk1160_read_ac97(struct stk1160 *dev, u16 reg)
 */
stk1160_write_reg(dev, STK1160_AC97CTL_0, 0x8b);
 
+   /*
+* Give the chip some time to transfer the data.
+* The proper way would be to poll STK1160_AC97CTL_0
+* until the command bit has been cleared, but this
+* may not be worth the hassle.
+*/
+   usleep_range(20, 40);
+
/* Retrieve register value */
stk1160_read_reg(dev, STK1160_AC97_CMD, );
stk1160_read_reg(dev, STK1160_AC97_CMD + 1, );
-- 
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html