| Message ID | 20161127111236.GA1691@arch-desktop (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Em Sun, 27 Nov 2016 12:12:36 +0100 Marcel Hasler <mahasler@gmail.com> 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 <mahasler@gmail.com> > --- > 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 <linux/delay.h> > #include <linux/module.h> > > #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, &vall); > stk1160_read_reg(dev, STK1160_AC97_CMD + 1, &valh); Thanks, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2 December 2016 at 08:09, Mauro Carvalho Chehab <mchehab@s-opensource.com> wrote: > Em Sun, 27 Nov 2016 12:12:36 +0100 > Marcel Hasler <mahasler@gmail.com> 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 <mahasler@gmail.com> >> --- >> 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 <linux/delay.h> >> #include <linux/module.h> >> >> #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, &vall); >> stk1160_read_reg(dev, STK1160_AC97_CMD + 1, &valh); > > > > Thanks, > Mauro
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 <linux/delay.h> #include <linux/module.h> #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, &vall); stk1160_read_reg(dev, STK1160_AC97_CMD + 1, &valh);
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 <mahasler@gmail.com> --- drivers/media/usb/stk1160/stk1160-ac97.c | 9 +++++++++ 1 file changed, 9 insertions(+)