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. > > + /* 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? > > + 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. > 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.
Attachment:
signature.asc
Description: PGP signature