Re: [EXTERNAL] Re: [PATCH v9] ALSA: hda/tas2781: Add tas2781 hda SPI driver

[Xu, Baojun]

Hi Geert,

Thanks for your review, answer in line.

Best Regards
Jim
________________________________________
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: 12 July 2024 05:05
> To: Xu, Baojun
> Cc: tiwai@xxxxxxx; robh+dt@xxxxxxxxxx; andriy.shevchenko@xxxxxxxxxxxxxxx; lgirdwood@xxxxxxxxx; perex@xxxxxxxx; pierre-louis.bossart@xxxxxxxxxxxxxxx; Lu, Kevin; Ding, Shenghao; Navada Kanyana, Mukund; 13916275206@xxxxxxx; Hampiholi, Vallabha; P O, Vijeth; Holalu Yogendra, Niranjan; alsa-devel@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; liam.r.girdwood@xxxxxxxxx; yung-chuan.liao@xxxxxxxxxxxxxxx; broonie@xxxxxxxxxx; soyer@xxxxxx
> Subject: [EXTERNAL] Re: [PATCH v9] ALSA: hda/tas2781: Add tas2781 hda SPI driver
> 
> Hi Baojun, On Thu, Jul 11, 2024 at 3: 22 PM Baojun Xu <baojun. xu@ ti. com> wrote: > This patch was used to add TAS2781 devices on SPI support in sound/pci/hda. > It use ACPI node descript about parameters of TAS2781 on SPI, it like: 
> ZjQcmQRYFpfptBannerStart
> This message was sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source of this email and know the content is safe. If you wish to report this message to IT Security, please forward the message as an attachment to phishing@xxxxxxxxxxx
> 
> ZjQcmQRYFpfptBannerEnd
> 
> Hi Baojun,
> 
> On Thu, Jul 11, 2024 at 3:22 PM Baojun Xu <baojun.xu@xxxxxx> wrote:
> > This patch was used to add TAS2781 devices on SPI support in sound/pci/hda.
> > It use ACPI node descript about parameters of TAS2781 on SPI, it like:
> >     Scope (_SB.PC00.SPI0)
> >     {
> >         Device (GSPK)
> >         {
> >             Name (_HID, "TXNW2781")  // _HID: Hardware ID
> >             Method (_CRS, 0, NotSerialized)
> >             {
> >                 Name (RBUF, ResourceTemplate ()
> >                 {
> >                     SpiSerialBusV2 (...)
> >                     SpiSerialBusV2 (...)
> >                 }
> >             }
> >         }
> >     }
> >
> > And in platform/x86/serial-multi-instantiate.c, those spi devices will be
> > added into system as a single SPI device, so TAS2781 SPI driver will
> > probe twice for every single SPI device. And driver will also parser
> > mono DSP firmware binary and RCA binary for itself.
> > The code support Realtek as the primary codec.
> >
> > Signed-off-by: Baojun Xu <baojun.xu@xxxxxx>
> > --- /dev/null
> > +++ b/sound/pci/hda/tas2781_hda_spi.c
> 
> Thanks for your patch!
> 
> > +/* fixed m68k compiling issue: mapping table can save code field */
> > +static const struct blktyp_devidx_map ppc3_tas2781_mapping_table[] = {
> 
> > +/* fixed m68k compiling issue: mapping table can save code field */
> > +static unsigned char map_dev_idx(struct tasdevice_fw *tas_fmw,
> > +       struct tasdev_blk *block)
> 
> > +/* Block parser function. */
> > +static int fw_parse_block_data_kernel(struct tasdevice_fw *tas_fmw,
> > +       struct tasdev_blk *block, const struct firmware *fmw, int offset)
> > +{
> 
> > +       /*
> > +        * Fixed m68k compiling issue:
> > +        * 1. mapping table can save code field.
> > +        * 2. storing the dev_idx as a member of block can reduce unnecessary
> > +        *    time and system resource comsumption of dev_idx mapping every
> > +        *    time the block data writing to the dsp.
> > +        */
> 
> Do we really need more copies of this?
> See sound/soc/codecs/tas2781-fmwlib.c.
> 

Yes, it's parser is similar, but for SPI device, will parser single device
firmware binary file for own only, and for I2C, parser support multi devices
firmware binary file, if use same lib functions, need add many code for check.

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 
>