On 2/8/23 14:01, Mark Brown wrote:
On Tue, Aug 01, 2023 at 10:18:57PM +0800, Baojun.Xu wrote:
+ while (val_size) {
+ /* to end of page */
+ bytes = SDW_REG_NO_PAGE - (reg & SDW_REGADDR);
regmap has paging support, can't the driver use that?
+static const struct regmap_config tasdevice_regmap = {
+ .reg_bits = 32,
+ .val_bits = 8,
+ .readable_reg = tas2783_readable_register,
+ .volatile_reg = tas2783_volatile_register,
+ .max_register = 0x41008000 + TASDEVICE_REG(0xa1, 0x60, 0x7f),
+ .reg_defaults = tas2783_reg_defaults,
+ .num_reg_defaults = ARRAY_SIZE(tas2783_reg_defaults),
+ .cache_type = REGCACHE_RBTREE,
Please use _MAPLE for new devices, it's more modern than _RBTREE. It
should make little if any practical difference.
+ .use_single_read = true,
+ .use_single_write = true,
+};
+/*
+ * Registers are big-endian on I2C and SPI but little-endian on SoundWire.
+ * Exported firmware controls are big-endian on I2C/SPI but little-endian
+ * on SoundWire.
Are you sure this isn't due to running on different host architecture?
+ * Firmware files are always big-endian and are opaque blobs.
+ * Present a big-endian regmap and hide the endianness swap,
+ * so that the ALSA byte controls always have the same byte order,
+ * and firmware file blobs can be written verbatim.
+ */
+static const struct regmap_bus tas2783_regmap_bus_sdw = {
+ .read = tas2783_sdw_read,
+ .write = tas2783_sdw_write,
+ .gather_write = tas2783_sdw_gather_write,
+ .reg_format_endian_default = REGMAP_ENDIAN_LITTLE,
+ .val_format_endian_default = REGMAP_ENDIAN_BIG,
+};
None of the other SoundWire devices use a custom bus, this all feels
suspicous especially since there's a bunch of bypassing of the bus in
places and calling functions directly. I would expect everything
outside the regmap code should be able to use the regmap, possibly
excluding firmware download, and that regmap should be able to
encapsulate any differences in endianness between the different buses.
At the minute the regmap is reported as having 8 bit registers which
should mean there are no endianness issues.
This looks suspiciously like it has been copied from the cs35l56_sdw.c
driver without understanding why cs35l56 does this. This TAS2783 driver
is only SDW, so what has I2C and SPI have to do with this? It's a huge
coincidence for the TAS2783 to have exactly the same
backward-compatibility quirks as the CS35L56 that necessitated a custom
regmap, i.e. three control interfaces, a register map that is sent big
endian on I2C/SPI but little-endian on SDW, downloadable firmware with
a file format that is big-endian, and DSP registers that are endian
swapped on I2C/SPI but not SDW.
The rt*-sdw.c or max*-sdw.c devices are a better starting point for a
Soundwire codec driver.
