...

+static const struct reg_sequence rt1320_blind_write[] = {

• { 0xc003, 0xe0 },
  

...

...

• { 0xd486, 0xc3 },
  

+};

I would add a comment that the 'blind writes' is an SDCA term to deal with platform-specific initialization, but in this case it seems that all the addresses targeted are in the vendor-specific space

Sure, will add a comment to describe what blind writes means.

...

+static const struct reg_sequence rt1320_patch_code_write[] = {

• { 0x10007000, 0x37 },
  

...

...

• { 0x0000d540, 0x01 },
  

+};

I would add a comment on the targeted register space. It seems to be the SDCA function 1, except for the last one?

Also possibly move the tables to a different file to make the driver code easier to get to (less scrolling required).

The driver will add a comment for the patch_code_write. The setting of the last one notifies the patch code and blind writes are done. That is the handshake mechanism we shall set.

...

+static int rt1320_read_prop(struct sdw_slave *slave) {

• struct sdw_slave_prop *prop = &slave->prop;
  

• int nval;
  

• int i, j;
  

• u32 bit;
  

• unsigned long addr;
  

• struct sdw_dpn_prop *dpn;
  

• sdw_slave_read_prop(slave);
  

add a comment that this is needed to fetch the lane information from platform firmware.

OK, will do.

...

+static const struct sdw_device_id rt1320_id[] = {

• SDW_SLAVE_ENTRY_EXT(0x025d, 0x1320, 0x3, 0x0, 0),\
  

missing class ID 1?

Will add class ID 1.

...

+/* Function_Status */ +#define FUNCTION_NEEDS_INITIALIZATION BIT(5) +#define FUNCTION_HAS_BEEN_RESET BIT(6)

You seem to use only NEEDS_INITIALIZATION, what happens on a RESET?

The RESET should influence the SDCA control. The blind writes doesn't set the SDCA control. Therefore, the driver doesn't check RESET flag.

...

+#define FUNCTION_BUSY BIT(7)

not used, can it be asserted?

Will remove it.

Looks mostly good otherwise, thanks for the patch.