Em Sun, 28 Feb 2016 12:26:23 +0100
Matthieu Rogez <[email protected]> escreveu:
> EMP202 chip inside Terratec Grabby (hw rev 2) seems to require some time before
> accessing reliably its registers. Otherwise it returns some values previously
> put on the I2C bus.
>
> To account for that period, we delay card setup until we have a proof that
> accessing AC97 registers is reliable. We get this proof by polling AC97_RESET
> until the expected value is read.
>
> Signed-off-by: Matthieu Rogez <[email protected]>
> ---
> drivers/media/usb/em28xx/em28xx-cards.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
> index 5e127e4..2e04902 100644
> --- a/drivers/media/usb/em28xx/em28xx-cards.c
> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> @@ -37,6 +37,7 @@
> #include <media/i2c-addr.h>
> #include <media/tveeprom.h>
> #include <media/v4l2-common.h>
> +#include <sound/ac97_codec.h>
>
> #include "em28xx.h"
>
> @@ -2563,6 +2564,24 @@ static inline void em28xx_set_model(struct em28xx *dev)
> dev->def_i2c_bus = dev->board.def_i2c_bus;
> }
>
> +/* Wait until AC97_RESET reports the expected value (or reading error)
> + * before proceeding.
> + * This can help ensuring AC97 register accesses are reliable.
> + */
> +static int em28xx_wait_until_ac97_features_equals(struct em28xx *dev,
> + int expected_feat)
> +{
> + int feat;
> +
> + do {
> + feat = em28xx_read_ac97(dev, AC97_RESET);
> + if (feat < 0)
> + return feat;
> + } while (feat != expected_feat);
Please add some sort of timeout here... We don't want the Kernel to
be in a dead lock here...
> +
> + return 0;
> +}
> +
> /* Since em28xx_pre_card_setup() requires a proper dev->model,
> * this won't work for boards with generic PCI IDs
> */
> @@ -2668,6 +2687,13 @@ static void em28xx_pre_card_setup(struct em28xx *dev)
> em28xx_write_reg(dev, EM2820_R08_GPIO_CTRL, 0xfd);
> msleep(70);
> break;
> +
> + case EM2860_BOARD_TERRATEC_GRABBY:
> + /* HACK?: Ensure AC97 register reading is reliable before
> + * proceeding. In practice, this will wait about 1.6 seconds.
> + */
> + em28xx_wait_until_ac97_features_equals(dev, 0x6a90);
> + break;
> }
>
> em28xx_gpio_set(dev, dev->board.tuner_gpio);
--
Thanks,
Mauro
Thanks Mauro for commenting on my work.
With respect to first version, I've:
* added a timeout mecanism as requested
* added an extra check to avoid cases when the same value is constantly
returned no matter which register is accessed.
---
To test this:
* modprobe em28xx-alsa
* connect Grabby
* mplayer tv:// -tv driver=v4l2:width=720:height=576:device=/dev/video1:input=0:fps=25:norm=SECAM-L:alsa:adevice=plughw.2:audiorate=48000:forceaudio:amode=1:immediatemode=0 -nocache -fps 60
NOTE: for some unknown reason, if em28xx-alsa is not loaded when Grabby is
connected, there will be no sound.
NOTE2: if Grabby is connected without loading em28xx-alsa first, just deconnect
the Grabby and reconnect it.
EMP202 chip inside Terratec Grabby (hw rev 2) seems to require some time
before accessing reliably its registers. Otherwise it returns some values
previously put on the I2C bus.
To account for that period, we delay card setup until we have a proof that
accessing AC97 registers is reliable. We get this proof by polling
AC97_RESET until the expected value is read. We also check that unrelated
registers don't return the same value. This second check handles the case
where the expected value is constantly returned no matter which register
is accessed.
Signed-off-by: Matthieu Rogez <[email protected]>
---
drivers/media/usb/em28xx/em28xx-cards.c | 38 +++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 5e127e4..930e3e3 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -37,6 +37,7 @@
#include <media/i2c-addr.h>
#include <media/tveeprom.h>
#include <media/v4l2-common.h>
+#include <sound/ac97_codec.h>
#include "em28xx.h"
@@ -2563,6 +2564,36 @@ static inline void em28xx_set_model(struct em28xx *dev)
dev->def_i2c_bus = dev->board.def_i2c_bus;
}
+/* Wait until AC97_RESET reports the expected value reliably before proceeding.
+ * We also check that two unrelated registers accesses don't return the same
+ * value to avoid premature return.
+ * This procedure helps ensuring AC97 register accesses are reliable.
+ */
+static int em28xx_wait_until_ac97_features_equals(struct em28xx *dev,
+ int expected_feat)
+{
+ unsigned long timeout = jiffies + msecs_to_jiffies(2000);
+ int feat, powerdown;
+
+ while (time_is_after_jiffies(timeout)) {
+ feat = em28xx_read_ac97(dev, AC97_RESET);
+ if (feat < 0)
+ return feat;
+
+ powerdown = em28xx_read_ac97(dev, AC97_POWERDOWN);
+ if (powerdown < 0)
+ return powerdown;
+
+ if (feat == expected_feat && feat != powerdown)
+ return 0;
+
+ msleep(50);
+ }
+
+ em28xx_warn("AC97 registers access is not reliable !\n");
+ return -ETIMEDOUT;
+}
+
/* Since em28xx_pre_card_setup() requires a proper dev->model,
* this won't work for boards with generic PCI IDs
*/
@@ -2668,6 +2699,13 @@ static void em28xx_pre_card_setup(struct em28xx *dev)
em28xx_write_reg(dev, EM2820_R08_GPIO_CTRL, 0xfd);
msleep(70);
break;
+
+ case EM2860_BOARD_TERRATEC_GRABBY:
+ /* HACK?: Ensure AC97 register reading is reliable before
+ * proceeding. In practice, this will wait about 1.6 seconds.
+ */
+ em28xx_wait_until_ac97_features_equals(dev, 0x6a90);
+ break;
}
em28xx_gpio_set(dev, dev->board.tuner_gpio);
--
2.7.2