Re: [PATCH v2] ASoC: tas2783: Add source files for tas2783 soundwire driver
From: Mark Brown
Date: Thu Aug 17 2023 - 11:13:38 EST
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