RE: [PATCH] ASoC: cs42xx8: Add SPI bus support for CS42448/CS42888 codec

[Chancel Liu (OSS)]

> > > > The existing cs42xx8 driver only supported I2C control interface.
> > > > Add SPI bus support for the Cirrus Logic CS42448/CS42888 Audio
> CODEC.
> > >
> > > > + * The SPI frame is 3 bytes:
> > > > + *   Byte 0: chip address [7:1] = 1001111, bit[0] = R/W (0=write,
> > > 1=read)
> > > > + *           Write: 0x9E,  Read: 0x9F
> > > > + *   Byte 1: MAP - Memory Address Pointer
> > > > + *           bit[7] = INCR (auto-increment for burst), bits[6:0] =
> > > address
> > >
> > > > + * We configure reg_bits=16 so that regmap treats the address
> > > > + field as
> > > 2 bytes
> > > > + * (big-endian). The chip address byte (0x9E/0x9F) is placed in
> > > > + the
> > > high byte
> > > > + * via write_flag_mask / read_flag_mask, and the MAP register
> > > > + address
> > > occupies
> > > > + * the low byte. This produces the correct 3-byte on-wire frame
> > > > + without
> > > any
> > > > + * custom bus implementation:
> > > > + *
> > > > + *   write: [0x9E, MAP_addr, data]
> > > > + *   read:  [0x9F, MAP_addr] -> [data]
> > >
> > > What about INCR?
> > >
> >
> > Indeed. This patch doesn't handle INCR. I'll enable INCR after
> > reworking on write_flag_mask/read_flag_mask.
> >
> 
> I don't think the driver does anything that reads/writes more than a
> single register, so one could probably just set use_single_read/write and
> punt on handling INCR for now.
>

Even after enabling INCR, I found in my testing that the current
configuration still issues single-register writes during
regcache_sync(). So for this case, INCR is not actually exercised by
the current sync path.

Given that, I agree that it's better to keep this series focused on
single read/write accesses for now. If a path that performs true
bulk/raw multi-register accesses is added later, INCR support can be
introduced as a separate follow-up.

Regards, 
Chancel Liu

> > > > +static const struct of_device_id cs42xx8_of_match[] = {  {
> > > > +.compatible = "cirrus,cs42448", .data = &cs42448_data, },  {
> > > > +.compatible = "cirrus,cs42888", .data = &cs42888_data, },  { /*
> > > > +sentinel */ } }; MODULE_DEVICE_TABLE(of, cs42xx8_of_match);
> > >
> > > This should have an update to the bindings as well.
> 
> Thanks sorry missed that.
> 
> Thanks,
> Charles