| Message ID | 1459237178-12920-1-git-send-email-petr@barix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Mar 29, 2016 at 09:39:34AM +0200, Petr Kulhavy wrote: > The list of TAS571x registers was incomplete. > Added the missing register definitions up to register 0x25 Please use subject lines matching the style for the subsystem. This makes it easier for people to identify relevant patches.
On Tue, Mar 29, 2016 at 12:39 AM, Petr Kulhavy <petr@barix.com> wrote: > The list of TAS571x registers was incomplete. > Added the missing register definitions up to register 0x25 > > Signed-off-by: Petr Kulhavy <petr@barix.com> For the series: Reviewed-by: Kevin Cernekee <cernekee@chromium.org>
On Tue, Mar 29, 2016 at 09:39:34AM +0200, Petr Kulhavy wrote: > The list of TAS571x registers was incomplete. > Added the missing register definitions up to register 0x25 According to the driver the device has registers up to 0xff? > /* device registers */ > +#define TAS571X_CLK_CTRL_REG 0x00 > +#define TAS571X_DEV_ID_REG 0x01 > +#define TAS571X_ERR_STATUS_REG 0x02 > +#define TAS571X_SYS_CTRL_1_REG 0x03 These look like volatile registers but the device has a register cache and we're not adding a list of volatile registers (or readable registers for that matter).
Hi Mark, On 29.03.2016 23:33, Mark Brown wrote: > On Tue, Mar 29, 2016 at 09:39:34AM +0200, Petr Kulhavy wrote: > >> The list of TAS571x registers was incomplete. >> Added the missing register definitions up to register 0x25 > According to the driver the device has registers up to 0xff? That is indeed true. But from address 0x29 on (0x26 to 0x28 are reserved) the register width varies between 20, 12 and 8 bytes, which I'm afraid the register map is unable to represent. >> /* device registers */ >> +#define TAS571X_CLK_CTRL_REG 0x00 >> +#define TAS571X_DEV_ID_REG 0x01 >> +#define TAS571X_ERR_STATUS_REG 0x02 >> +#define TAS571X_SYS_CTRL_1_REG 0x03 > These look like volatile registers but the device has a register cache > and we're not adding a list of volatile registers (or readable registers > for that matter). That's a good point, thanks! 0x03 is a regular RW register but 0x00 to 0x02 are indeed volatile. Is it better to make them read-only, or volatile? Petr
On Wed, Mar 30, 2016 at 10:14:56AM +0200, Petr Kulhavy wrote: > On 29.03.2016 23:33, Mark Brown wrote: > >On Tue, Mar 29, 2016 at 09:39:34AM +0200, Petr Kulhavy wrote: > >>The list of TAS571x registers was incomplete. > >>Added the missing register definitions up to register 0x25 > >According to the driver the device has registers up to 0xff? > That is indeed true. But from address 0x29 on (0x26 to 0x28 are reserved) > the register width varies between 20, 12 and 8 bytes, which I'm afraid the > register map is unable to represent. Say what's going on in your changelog then. > >>+#define TAS571X_DEV_ID_REG 0x01 > >>+#define TAS571X_ERR_STATUS_REG 0x02 > >These look like volatile registers but the device has a register cache > >and we're not adding a list of volatile registers (or readable registers > >for that matter). > That's a good point, thanks! 0x03 is a regular RW register but 0x00 to 0x02 > are indeed volatile. > Is it better to make them read-only, or volatile? It's not an either/or. If they are read only they should be flagged as that. If they are volatile (if they could change value at runtime) then they need to be flagged as that, I'd expect this applies to ERR_STATUS_REG.
diff --git a/sound/soc/codecs/tas571x.c b/sound/soc/codecs/tas571x.c index 39307ad..346d3da 100644 --- a/sound/soc/codecs/tas571x.c +++ b/sound/soc/codecs/tas571x.c @@ -57,6 +57,10 @@ static int tas571x_register_size(struct tas571x_private *priv, unsigned int reg) case TAS571X_CH1_VOL_REG: case TAS571X_CH2_VOL_REG: return priv->chip->vol_reg_size; + case TAS571X_INPUT_MUX_REG: + case TAS571X_CH4_SRC_SELECT_REG: + case TAS571X_PWM_MUX_REG: + return 4; default: return 1; } diff --git a/sound/soc/codecs/tas571x.h b/sound/soc/codecs/tas571x.h index 0aee471..cf800c3 100644 --- a/sound/soc/codecs/tas571x.h +++ b/sound/soc/codecs/tas571x.h @@ -13,6 +13,10 @@ #define _TAS571X_H /* device registers */ +#define TAS571X_CLK_CTRL_REG 0x00 +#define TAS571X_DEV_ID_REG 0x01 +#define TAS571X_ERR_STATUS_REG 0x02 +#define TAS571X_SYS_CTRL_1_REG 0x03 #define TAS571X_SDI_REG 0x04 #define TAS571X_SDI_FMT_MASK 0x0f @@ -27,7 +31,25 @@ #define TAS571X_MVOL_REG 0x07 #define TAS571X_CH1_VOL_REG 0x08 #define TAS571X_CH2_VOL_REG 0x09 +#define TAS571X_CH3_VOL_REG 0x0a +#define TAS571X_VOL_CFG_REG 0x0e +#define TAS571X_MODULATION_LIMIT_REG 0x10 +#define TAS571X_IC_DELAY_CH1_REG 0x11 +#define TAS571X_IC_DELAY_CH2_REG 0x12 +#define TAS571X_IC_DELAY_CH3_REG 0x13 +#define TAS571X_IC_DELAY_CH4_REG 0x14 +#define TAS571X_PWM_CH_SDN_GROUP_REG 0x19 /* N/A on TAS5717, TAS5719 */ +#define TAS571X_PWM_CH1_SDN_MASK (1<<0) +#define TAS571X_PWM_CH2_SDN_SHIFT (1<<1) +#define TAS571X_PWM_CH3_SDN_SHIFT (1<<2) +#define TAS571X_PWM_CH4_SDN_SHIFT (1<<3) + +#define TAS571X_START_STOP_PERIOD_REG 0x1a #define TAS571X_OSC_TRIM_REG 0x1b +#define TAS571X_BKND_ERR_REG 0x1c +#define TAS571X_INPUT_MUX_REG 0x20 +#define TAS571X_CH4_SRC_SELECT_REG 0x21 +#define TAS571X_PWM_MUX_REG 0x25 #endif /* _TAS571X_H */
The list of TAS571x registers was incomplete. Added the missing register definitions up to register 0x25 Signed-off-by: Petr Kulhavy <petr@barix.com> --- v1: <initial> v2: <no change> sound/soc/codecs/tas571x.c | 4 ++++ sound/soc/codecs/tas571x.h | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+)