Re: [PATCH V3 4/5] ASoC: codecs: aw88261 device related operation functions

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

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.

...

> 
>>> +
>>> +	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.


Best regards,
Krzysztof