| Message ID | 20161027203515.GA847@arch-desktop (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 27 October 2016 at 17:35, Marcel Hasler <mahasler@gmail.com> wrote: > Allow setting a custom record gain for the internal AC97 codec (if available). This can be > a value between 0 and 15, 8 is the default and should be suitable for most users. The Windows > driver also sets this to 8 without any possibility for changing it. > > Signed-off-by: Marcel Hasler <mahasler@gmail.com> > --- > drivers/media/usb/stk1160/stk1160-ac97.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/usb/stk1160/stk1160-ac97.c b/drivers/media/usb/stk1160/stk1160-ac97.c > index 6dbc39f..31bdd60d 100644 > --- a/drivers/media/usb/stk1160/stk1160-ac97.c > +++ b/drivers/media/usb/stk1160/stk1160-ac97.c > @@ -25,6 +25,11 @@ > #include "stk1160.h" > #include "stk1160-reg.h" > > +static u8 gain = 8; > + > +module_param(gain, byte, 0444); > +MODULE_PARM_DESC(gain, "Set capture gain level if AC97 codec is available (0-15, default: 8)"); > + > static void stk1160_write_ac97(struct stk1160 *dev, u16 reg, u16 value) > { > /* Set codec register address */ > @@ -122,7 +127,11 @@ void stk1160_ac97_setup(struct stk1160 *dev) > stk1160_write_ac97(dev, 0x16, 0x0808); /* Aux volume */ > stk1160_write_ac97(dev, 0x1a, 0x0404); /* Record select */ > stk1160_write_ac97(dev, 0x02, 0x0000); /* Master volume */ > - stk1160_write_ac97(dev, 0x1c, 0x0808); /* Record gain */ > + > + /* Record gain */ > + gain = (gain > 15) ? 15 : gain; > + stk1160_info("Setting capture gain to %d.", gain); This message doesn't add anything useful, can we drop it? > + stk1160_write_ac97(dev, 0x1c, (gain<<8) | gain); > > #ifdef DEBUG > stk1160_ac97_dump_regs(dev); > -- > 2.10.1 >
2016-11-20 18:36 GMT+01:00 Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>: > On 27 October 2016 at 17:35, Marcel Hasler <mahasler@gmail.com> wrote: >> Allow setting a custom record gain for the internal AC97 codec (if available). This can be >> a value between 0 and 15, 8 is the default and should be suitable for most users. The Windows >> driver also sets this to 8 without any possibility for changing it. >> >> Signed-off-by: Marcel Hasler <mahasler@gmail.com> >> --- >> drivers/media/usb/stk1160/stk1160-ac97.c | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/media/usb/stk1160/stk1160-ac97.c b/drivers/media/usb/stk1160/stk1160-ac97.c >> index 6dbc39f..31bdd60d 100644 >> --- a/drivers/media/usb/stk1160/stk1160-ac97.c >> +++ b/drivers/media/usb/stk1160/stk1160-ac97.c >> @@ -25,6 +25,11 @@ >> #include "stk1160.h" >> #include "stk1160-reg.h" >> >> +static u8 gain = 8; >> + >> +module_param(gain, byte, 0444); >> +MODULE_PARM_DESC(gain, "Set capture gain level if AC97 codec is available (0-15, default: 8)"); >> + >> static void stk1160_write_ac97(struct stk1160 *dev, u16 reg, u16 value) >> { >> /* Set codec register address */ >> @@ -122,7 +127,11 @@ void stk1160_ac97_setup(struct stk1160 *dev) >> stk1160_write_ac97(dev, 0x16, 0x0808); /* Aux volume */ >> stk1160_write_ac97(dev, 0x1a, 0x0404); /* Record select */ >> stk1160_write_ac97(dev, 0x02, 0x0000); /* Master volume */ >> - stk1160_write_ac97(dev, 0x1c, 0x0808); /* Record gain */ >> + >> + /* Record gain */ >> + gain = (gain > 15) ? 15 : gain; >> + stk1160_info("Setting capture gain to %d.", gain); > > This message doesn't add anything useful, can we drop it? > I think it would make sense to have some kind of feedback, at least when the default value has been overridden. How about making this conditional? >> + stk1160_write_ac97(dev, 0x1c, (gain<<8) | gain); >> >> #ifdef DEBUG >> stk1160_ac97_dump_regs(dev); >> -- >> 2.10.1 >> > > > > -- > 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 26 November 2016 at 10:52, Marcel Hasler <mahasler@gmail.com> wrote: > 2016-11-20 18:36 GMT+01:00 Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>: >> On 27 October 2016 at 17:35, Marcel Hasler <mahasler@gmail.com> wrote: >>> Allow setting a custom record gain for the internal AC97 codec (if available). This can be >>> a value between 0 and 15, 8 is the default and should be suitable for most users. The Windows >>> driver also sets this to 8 without any possibility for changing it. >>> >>> Signed-off-by: Marcel Hasler <mahasler@gmail.com> >>> --- >>> drivers/media/usb/stk1160/stk1160-ac97.c | 11 ++++++++++- >>> 1 file changed, 10 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/media/usb/stk1160/stk1160-ac97.c b/drivers/media/usb/stk1160/stk1160-ac97.c >>> index 6dbc39f..31bdd60d 100644 >>> --- a/drivers/media/usb/stk1160/stk1160-ac97.c >>> +++ b/drivers/media/usb/stk1160/stk1160-ac97.c >>> @@ -25,6 +25,11 @@ >>> #include "stk1160.h" >>> #include "stk1160-reg.h" >>> >>> +static u8 gain = 8; >>> + >>> +module_param(gain, byte, 0444); >>> +MODULE_PARM_DESC(gain, "Set capture gain level if AC97 codec is available (0-15, default: 8)"); >>> + >>> static void stk1160_write_ac97(struct stk1160 *dev, u16 reg, u16 value) >>> { >>> /* Set codec register address */ >>> @@ -122,7 +127,11 @@ void stk1160_ac97_setup(struct stk1160 *dev) >>> stk1160_write_ac97(dev, 0x16, 0x0808); /* Aux volume */ >>> stk1160_write_ac97(dev, 0x1a, 0x0404); /* Record select */ >>> stk1160_write_ac97(dev, 0x02, 0x0000); /* Master volume */ >>> - stk1160_write_ac97(dev, 0x1c, 0x0808); /* Record gain */ >>> + >>> + /* Record gain */ >>> + gain = (gain > 15) ? 15 : gain; >>> + stk1160_info("Setting capture gain to %d.", gain); >> >> This message doesn't add anything useful, can we drop it? >> > > I think it would make sense to have some kind of feedback, at least > when the default value has been overridden. How about making this > conditional? > Well, if the user passes a gain, requesting a non-default value, it is expected that the gain be set by the driver. So printing a message for something that is just as expected as "the driver will set the parameter you told him to set". User messages should only be printed when *unexpected* or otherwise relevant events happen.
diff --git a/drivers/media/usb/stk1160/stk1160-ac97.c b/drivers/media/usb/stk1160/stk1160-ac97.c index 6dbc39f..31bdd60d 100644 --- a/drivers/media/usb/stk1160/stk1160-ac97.c +++ b/drivers/media/usb/stk1160/stk1160-ac97.c @@ -25,6 +25,11 @@ #include "stk1160.h" #include "stk1160-reg.h" +static u8 gain = 8; + +module_param(gain, byte, 0444); +MODULE_PARM_DESC(gain, "Set capture gain level if AC97 codec is available (0-15, default: 8)"); + static void stk1160_write_ac97(struct stk1160 *dev, u16 reg, u16 value) { /* Set codec register address */ @@ -122,7 +127,11 @@ void stk1160_ac97_setup(struct stk1160 *dev) stk1160_write_ac97(dev, 0x16, 0x0808); /* Aux volume */ stk1160_write_ac97(dev, 0x1a, 0x0404); /* Record select */ stk1160_write_ac97(dev, 0x02, 0x0000); /* Master volume */ - stk1160_write_ac97(dev, 0x1c, 0x0808); /* Record gain */ + + /* Record gain */ + gain = (gain > 15) ? 15 : gain; + stk1160_info("Setting capture gain to %d.", gain); + stk1160_write_ac97(dev, 0x1c, (gain<<8) | gain); #ifdef DEBUG stk1160_ac97_dump_regs(dev);
Allow setting a custom record gain for the internal AC97 codec (if available). This can be a value between 0 and 15, 8 is the default and should be suitable for most users. The Windows driver also sets this to 8 without any possibility for changing it. Signed-off-by: Marcel Hasler <mahasler@gmail.com> --- drivers/media/usb/stk1160/stk1160-ac97.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)