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!