Thank you very much for your advice On 31/07/2023 08:51, krzysztof.kozlowski@xxxxxxxxxx wrote: > On 31/07/2023 08:41, wangweidong.a@xxxxxxxxxx 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