On 8/17/23 10:12, Mark Brown wrote:
On Thu, Aug 17, 2023 at 09:17:50AM -0500, Pierre-Louis Bossart wrote:
goto out;- }
- /* Read the primary device as the whole */
I can't figure out what this comment means
It's part of...
dev_err(tas_dev->dev, "%s, regmap doesn't exist.\n",__func__);return -EINVAL;- }
- /* Read the primary device */
What is a primary device?
...a thing where they're trying to present multiple devices as a unified device with grouped control, it looks like there's some hardware support for this.
Let me clarify the comment: SDCA peripheral can have multiple functions, each with its own address space and can operate independently. So I am just trying to have clarity on what 'device' means here.
- /* Failed got calibration data from EFI. */
I don't get what the dependency on EFI is. First time I see a codec needing this.
Please describe in details what you are trying to accomplish.
It seems fairly normal to store calibration details in the device firmware?
No objection on the device firmware, but why use an EFI variable?
There is on-going work to standardize with ACPI, and there's also a request_firmware(). Not sure what the direction is to read from an EFI variable. I've been in SDCA circles since the beginning and never heard about this, ever. I am not saying it's bad, just surprised and curious on a 3rd way of getting information needed for initialization.
- if (crc == tmp_val[21]) {
time64_to_tm(tmp_val[20], 0, tm);dev_dbg(tas_priv->dev, "%4ld-%2d-%2d, %2d:%2d:%2d\n",tm->tm_year, tm->tm_mon, tm->tm_mday,tm->tm_hour, tm->tm_min, tm->tm_sec);What is this about? Why would a codec care about time?
I can see someone finding it helpful to confirm when the calibration data that got applied was generated to make sure they're testing/using what they thought they were.
Ah yes, I missed that. I wasn't sure if this was a log on when the calibration finished, if this is a log on when the calibration data was generated that's a different story indeed.
In addition, it's rather surprising that on attachment there is not a single regmap access?
Don't know if there's something different with Soundwire but for I2C/SPI devices it's a reasonable pattern to only actually start initialising the registers when the device actually becomes active. Not checked that this is actually happening.
that's precisely the point, there's an io_init() routine which is when the peripheral is attached on the bus and the earliest time when the registers can be initialized.
But there isn't a single initialization happening, which is different to all existing SoundWire codec drivers. Maybe it's fine, I am just asking the question if this was intentional.