Received-SPF: pass (google.com: domain of linux-kernel+bounces-213212-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 13 Jun 2024 14:36:33 +0200
Message-ID: <875xudnhny.wl-tiwai@suse.de>
From: Takashi Iwai <tiwai@suse.de>
To: Baojun Xu <baojun.xu@ti.com>
Cc: <robh+dt@kernel.org>,
	<andriy.shevchenko@linux.intel.com>,
	<lgirdwood@gmail.com>,
	<perex@perex.cz>,
	<pierre-louis.bossart@linux.intel.com>,
	<kevin-lu@ti.com>,
	<shenghao-ding@ti.com>,
	<navada@ti.com>,
	<13916275206@139.com>,
	<v-hampiholi@ti.com>,
	<v-po@ti.com>,
	<niranjan.hy@ti.com>,
	<alsa-devel@alsa-project.org>,
	<linux-kernel@vger.kernel.org>,
	<liam.r.girdwood@intel.com>,
	<yung-chuan.liao@linux.intel.com>,
	<broonie@kernel.org>,
	<soyer@irl.hu>
Subject: Re: [PATCH v7 1/1] ALSA: hda/tas2781: Add tas2781 hda driver based on SPI
In-Reply-To: <20240613082633.388-2-baojun.xu@ti.com>
References: <20240613082633.388-1-baojun.xu@ti.com>
	<20240613082633.388-2-baojun.xu@ti.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0
Precedence: bulk
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII

On Thu, 13 Jun 2024 10:26:33 +0200,
Baojun Xu wrote:
> --- a/sound/pci/hda/Makefile
> +++ b/sound/pci/hda/Makefile
> @@ -70,6 +70,8 @@ obj-$(CONFIG_SND_HDA_SCODEC_CS35L56_SPI) += snd-hda-scodec-cs35l56-spi.o
>  obj-$(CONFIG_SND_HDA_CS_DSP_CONTROLS) += snd-hda-cs-dsp-ctls.o
>  obj-$(CONFIG_SND_HDA_SCODEC_COMPONENT) += snd-hda-scodec-component.o
>  obj-$(CONFIG_SND_HDA_SCODEC_TAS2781_I2C) += snd-hda-scodec-tas2781-i2c.o
> +obj-$(CONFIG_SND_HDA_SCODEC_TAS2781_SPI) += snd-hda-scodec-tas2781-spi.o
> +snd-hda-scodec-tas2781-spi-y :=	tas2781_hda_spi.o tas2781_spi_fwlib.o

Try to put *-y addition to the right place.  There are the section
defining snd-hda-codec-*-y.

> +#define TASDEVICE_REG(book, page, reg)	(((book & GENMASK(7, 0)) << 16) + \
> +					 ((page & GENMASK(7, 0)) << 8) + \
> +					  ((reg & GENMASK(6, 0)) << 1))

Usually bits are combined with '|', not '+'.

> +/* Firmware block Checksum */
> +#define TASDEVICE_Checksum		TASDEVICE_REG(0x0, 0x0, 0x7e)

Use capital letters.


... and at this point, I noticed that the patch doesn't contain the
code for sound/pci/hda/tas2781_spi_fwlib.c at all.  The patch must be
utterly broken.

*PLEASE* try to apply and build the patch to submit beforehand from
the clean state and verify it really works.


thanks,

Takashi