Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   wangweidong.a@awinic.com
To:     krzysztof.kozlowski@linaro.org
Cc:     13916275206@139.com, alsa-devel@alsa-project.org,
        broonie@kernel.org, ckeepax@opensource.cirrus.com,
        colin.i.king@gmail.com, conor+dt@kernel.org,
        devicetree@vger.kernel.org, doug@schmorgal.com, fido_max@inbox.ru,
        herve.codina@bootlin.com, krzysztof.kozlowski+dt@linaro.org,
        lgirdwood@gmail.com, linux-kernel@vger.kernel.org,
        liweilei@awinic.com, perex@perex.cz, povik+lin@cutebit.org,
        rf@opensource.cirrus.com, robh+dt@kernel.org, ryans.lee@analog.com,
        shumingf@realtek.com, tiwai@suse.com, trix@redhat.com,
        wangweidong.a@awinic.com, yijiangtao@awinic.com,
        zhangjianming@awinic.com
Subject: Re: [PATCH V3 4/5] ASoC: codecs: aw88261 device related operation functions
Date:   Mon, 31 Jul 2023 15:43:00 +0800
Message-ID: <20230731074300.6200-1-wangweidong.a@awinic.com>
In-Reply-To: <be25dead-89f0-8859-d182-84754ad8bfc0@linaro.org>
References: <be25dead-89f0-8859-d182-84754ad8bfc0@linaro.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Thank you very much for your advice

On 31/07/2023 08:51, krzysztof.kozlowski@linaro.org wrote:
> On 31/07/2023 08:41, wangweidong.a@awinic.com wrote:
>> 
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +int aw88261_dev_init(struct aw88261_device *aw_dev, struct aw_container *aw_cfg)
>> 
>>> You already used this function in patch #3, so your order of patches is
>>> confusing.
>> 
>> Do I need to change the order of patch? 
>> Do I neeed to put aw88261_device.c aw88261_device.h in patch #3 and 
>> put aw88261.c aw88261.h in patch #4?
>> Is that how you change the order?

> Your patchset should be logically ordered, so first you add bindings
> (because it must be before their users), then you one piece, then other
> etc. I understand that only the last patch will make everything
> buildable, but still code should be added before its user/caller.

Thank you very much for your suggestion. 
Do I need to keep the Makefile and kconfig files in a separate patch?

...

>> 
>>>> +
>>>> +	switch (chip_id) {
>>>> +	case AW88261_CHIP_ID:
>>>> +		ret = aw_dev_init((*aw_dev));
>>>> +		break;
>>>> +	default:
>>>> +		ret = -EINVAL;
>>>> +		dev_err((*aw_dev)->dev, "unsupported device");
>>>> +		break;
>>>> +	}
>>>> +
>>>> +	return ret;
>>>> +}
>>>> +
>>>> +MODULE_DESCRIPTION("AW88261 device");
>>>> +MODULE_LICENSE("GPL v2");
>> 
>>> Wait, is this a module? Does not look complete. I already saw one
>>> module, so what is this for? For which module?
>> 
>> Can it be changed to MODULE_DESCRIPTION("AW88261 device lib")?

> If this is a module, then it can. If this is not a module, then why
> would you ever like to do it?

>> The function in the aw88261_device.c file, which I used in the aw88261.c file.

> Functions are not modules.

Thank you very much for your suggestion. 
I will delete MODULE_DESCRIPTION and MODULE_LICENSE

Best regards,
Weidong Wang