| Message ID | 8f933cee57fc4420875e1e2ba14f1937@realtek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: rt1011: export r0 and temperature config | expand |
On Tue, Oct 08, 2019 at 09:33:24AM +0000, Shuming [范書銘] wrote: > In chromebook case, the machine driver will get the > r0 calibration data and temperature from VPD. > Therefore, the codec exports r0 and temperature config API for it. Why will the machine driver do this?
On Tue, Oct 8, 2019 at 6:51 PM Mark Brown <broonie@kernel.org> wrote: > > On Tue, Oct 08, 2019 at 09:33:24AM +0000, Shuming [范書銘] wrote: > > > In chromebook case, the machine driver will get the > > r0 calibration data and temperature from VPD. > > Therefore, the codec exports r0 and temperature config API for it. > > Why will the machine driver do this? Hi Mark, The calibration data (R0 values and temperature) were measured and written to VPD in the factory. When machine driver initializes sound card, it reads the value from VPD and uses the exported API to set calibration value for codec. The purpose is to protect speaker. This needs to be done before sound card is initialized. If, alternatively, this process is done in user space through mixer control after the sound card is initialized, user space may use audio before calibration is done, and this will allow the possibility to damage speakers. Hope this clarifies the intention of this patch. Thanks!
On Tue, Oct 08, 2019 at 06:59:57PM +0800, Cheng-yi Chiang wrote: > The calibration data (R0 values and temperature) were measured and > written to VPD in the factory. When machine driver initializes sound > card, it reads the value from VPD and uses the exported API to set > calibration value for codec. The purpose is to protect speaker. This So the VPD is not part of the CODEC? One question would be why the CODEC driver doesn't directly read this information.
On Tue, Oct 8, 2019 at 7:06 PM Mark Brown <broonie@kernel.org> wrote: > > On Tue, Oct 08, 2019 at 06:59:57PM +0800, Cheng-yi Chiang wrote: > > > The calibration data (R0 values and temperature) were measured and > > written to VPD in the factory. When machine driver initializes sound > > card, it reads the value from VPD and uses the exported API to set > > calibration value for codec. The purpose is to protect speaker. This > > So the VPD is not part of the CODEC? One question would be why the > CODEC driver doesn't directly read this information. +Maxim folks as they are also interested in setting calibration data in driver. Hi Mark, The VPD is not part of the codec. It is a binary blob in system firmware where we can store important information per-device. The calibration data is written to RO section of VPD in the factory during calibration step. The codec driver is not suitable of reading this information directly because the string format written into VPD is customized per board. For example on cml_rt1011_rt5682.c there are four R0 values for four speakers, and one temperature values . So in this case, there are totally 5 values in a VPD dsm_calib key. In VPD, the format is like "dsm_calib"="0x00278F09 0x00251E1B 0x0021AFE6 0x0022720A 0x0000012E" We put all the information into one string to allow arbitrary calibration data needed for smart amp calibration in the future. On other system using smart amp, there might be two speakers, with two temperature values..etc. The format will be changed accordingly. Number of temperature values depends on number of temperature sensor available near the speakers. Since machine driver knows the combination of speakers and the available temperature sensor, we think that machine driver is the better place to put this per-board logic. Thanks!
On Tue, Oct 08, 2019 at 07:22:17PM +0800, Cheng-yi Chiang wrote: > The VPD is not part of the codec. > It is a binary blob in system firmware where we can store important > information per-device. > The calibration data is written to RO section of VPD in the factory > during calibration step. Ugh, this is not idiomatic for a DT system :( > The codec driver is not suitable of reading this information directly > because the string format written into VPD is customized per board. > For example on cml_rt1011_rt5682.c there are four R0 values for four The expected model for a DT system is that this stuff should just come in through DT properties, if for system design/manufacturing reasons it needs to be stored separately then you'd expect it to be merged into the main DT by the bootloader or something else earlier on in boot. > speakers, and one temperature values . So in this case, there are > totally 5 values in a VPD dsm_calib key. In VPD, the format is like > "dsm_calib"="0x00278F09 0x00251E1B 0x0021AFE6 0x0022720A 0x0000012E" > We put all the information into one string to allow arbitrary > calibration data needed for smart amp calibration in the future. > On other system using smart amp, there might be two speakers, with two > temperature values..etc. The format will be changed accordingly. > Number of temperature values depends on number of temperature sensor > available near the speakers. > Since machine driver knows the combination of speakers and the > available temperature sensor, we think that machine driver is the > better place to put this per-board logic. I'm not sure why they all need to be in one property? That's a secondary problem though.
On Tue, Oct 08, 2019 at 09:33:24AM +0000, Shuming [范書銘] wrote: > In chromebook case, the machine driver will get the > r0 calibration data and temperature from VPD. > Therefore, the codec exports r0 and temperature config API for it. This doesn't seem to apply against current code, please check and resend.
> On Tue, Oct 08, 2019 at 09:33:24AM +0000, Shuming [范書銘] wrote: > > In chromebook case, the machine driver will get the > > r0 calibration data and temperature from VPD. > > Therefore, the codec exports r0 and temperature config API for it. > > This doesn't seem to apply against current code, please check and resend. Thanks Mark. I will check with Google whether still using this method. If so, I will resend the patch. > ------Please consider the environment before printing this e-mail.
On Mon, Oct 14, 2019 at 9:46 AM Shuming [范書銘] <shumingf@realtek.com> wrote: > > > On Tue, Oct 08, 2019 at 09:33:24AM +0000, Shuming [范書銘] wrote: > > > In chromebook case, the machine driver will get the > > > r0 calibration data and temperature from VPD. > > > Therefore, the codec exports r0 and temperature config API for it. > > > > This doesn't seem to apply against current code, please check and resend. > > Thanks Mark. I will check with Google whether still using this method. If so, I will resend the patch. Hi Mark and Shuming, Thank you for the suggestion. I agree that it is better to use device property to pass r0 and temperature through device property. I am working on coreboot changes to parse data from VPD, and set device property "rt1011,temperature_calib" and "realtek,r0_calib". I think the patch in rt1011 driver will be much simpler once we have that device property ready. And there will be no machine driver change needed. Thanks! > > > ------Please consider the environment before printing this e-mail.
On Tue, Oct 8, 2019 at 8:38 PM Mark Brown <broonie@kernel.org> wrote: > > On Tue, Oct 08, 2019 at 07:22:17PM +0800, Cheng-yi Chiang wrote: > > > The VPD is not part of the codec. > > It is a binary blob in system firmware where we can store important > > information per-device. > > The calibration data is written to RO section of VPD in the factory > > during calibration step. > > Ugh, this is not idiomatic for a DT system :( > > > The codec driver is not suitable of reading this information directly > > because the string format written into VPD is customized per board. > > For example on cml_rt1011_rt5682.c there are four R0 values for four > > The expected model for a DT system is that this stuff should just come > in through DT properties, if for system design/manufacturing reasons it > needs to be stored separately then you'd expect it to be merged into the > main DT by the bootloader or something else earlier on in boot. > > > speakers, and one temperature values . So in this case, there are > > totally 5 values in a VPD dsm_calib key. In VPD, the format is like > > "dsm_calib"="0x00278F09 0x00251E1B 0x0021AFE6 0x0022720A 0x0000012E" > > We put all the information into one string to allow arbitrary > > calibration data needed for smart amp calibration in the future. > > On other system using smart amp, there might be two speakers, with two > > temperature values..etc. The format will be changed accordingly. > > Number of temperature values depends on number of temperature sensor > > available near the speakers. > > Since machine driver knows the combination of speakers and the > > available temperature sensor, we think that machine driver is the > > better place to put this per-board logic. > > I'm not sure why they all need to be in one property? That's a > secondary problem though. Hi Mark, We did not consider this careful enough when we add this VPD field. The reason we put them all in one field because we thought that machine driver should have the knowledge to parse it. Now that it seems there are various places to parse it: - coreboot board file - coreboot device driver - kernel machine driver - kernel vpd driver - kernel codec driver The parsing becomes a problem, let alone that coreboot does not have some useful string functions to parse it. To ease this effort we are going to separate them into different fields, such that whichever driver wants the data, it can get it easily with key matching. Thanks for the suggestion!
diff --git a/sound/soc/codecs/rt1011.c b/sound/soc/codecs/rt1011.c index a92a0bacd812..a992bb2224b2 100644 --- a/sound/soc/codecs/rt1011.c +++ b/sound/soc/codecs/rt1011.c @@ -1325,6 +1325,47 @@ static int rt1011_r0_load_info(struct snd_kcontrol *kcontrol, .put = rt1011_r0_load_mode_put \ } +int rt1011_r0_config(struct snd_soc_component *component, unsigned int r0) +{ + struct rt1011_priv *rt1011 = + snd_soc_component_get_drvdata(component); + struct device *dev; + unsigned int r0_integer, r0_factor, format; + + if (!component->card->instantiated) + return 0; + + if (!r0) + return -EINVAL; + + dev = regmap_get_device(rt1011->regmap); + rt1011->r0_reg = r0; + + format = 2147483648U; /* 2^24 * 128 */ + r0_integer = format / rt1011->r0_reg / 128; + r0_factor = ((format / rt1011->r0_reg * 100) / 128) + - (r0_integer * 100); + dev_dbg(dev, "New r0 resistance about %d.%02d ohm, reg=0x%X\n", + r0_integer, r0_factor, rt1011->r0_reg); + + if (rt1011->r0_reg) + rt1011_r0_load(rt1011); + + return 0; +} +EXPORT_SYMBOL_GPL(rt1011_r0_config); + +int rt1011_temperature_config(struct snd_soc_component *component, + unsigned int temp) +{ + snd_soc_component_update_bits(component, + RT1011_STP_INITIAL_RESISTANCE_TEMP, 0x3ff, + (temp << 2)); + + return 0; +} +EXPORT_SYMBOL_GPL(rt1011_temperature_config); + static const struct snd_kcontrol_new rt1011_snd_controls[] = { /* I2S Data In Selection */ SOC_ENUM("DIN Source", rt1011_din_source_enum), diff --git a/sound/soc/codecs/rt1011.h b/sound/soc/codecs/rt1011.h index 2d65983f3d0f..b99df334be14 100644 --- a/sound/soc/codecs/rt1011.h +++ b/sound/soc/codecs/rt1011.h @@ -670,4 +670,9 @@ struct rt1011_priv { int recv_spk_mode; }; +int rt1011_r0_config(struct snd_soc_component *component, unsigned int r0); +int rt1011_temperature_config(struct snd_soc_component *component, + unsigned int temp); + + #endif /* end of _RT1011_H_ */
In chromebook case, the machine driver will get the r0 calibration data and temperature from VPD. Therefore, the codec exports r0 and temperature config API for it. Signed-off-by: Shuming Fan <shumingf@realtek.com> --- sound/soc/codecs/rt1011.c | 41 +++++++++++++++++++++++++++++++++++++++ sound/soc/codecs/rt1011.h | 5 +++++ 2 files changed, 46 insertions(+)