On 2024-04-13 04:35, Xu Yilun wrote:
>> /**
>> - * fpga_region_register_full - create and register an FPGA Region device
>> + * __fpga_region_register_full - create and register an FPGA Region device
>> * @parent: device parent
>> * @info: parameters for FPGA Region
>> + * @owner: owner module containing the get_bridges function
>
> This is too specific and easily get unaligned if we add another
> callback. How about "module containing the region ops"?
I had some concerns about using the name "region ops" in the kernel-doc
comment since it was not supported by a struct definition nor referenced
in the documentation. However, since the name is now referred to in the
ops_owner pointer, making the change makes sense to me.
Thanks,
Marco
>
>> *
>> * Return: struct fpga_region or ERR_PTR()
>> */
>> struct fpga_region *
>> -fpga_region_register_full(struct device *parent, const struct fpga_region_info *info)
>> +__fpga_region_register_full(struct device *parent, const struct fpga_region_info *info,
>> + struct module *owner)
>> {
>> struct fpga_region *region;
>> int id, ret = 0;
>> @@ -213,6 +215,7 @@ fpga_region_register_full(struct device *parent, const struct fpga_region_info *
>> region->compat_id = info->compat_id;
>> region->priv = info->priv;
>> region->get_bridges = info->get_bridges;
>> + region->ops_owner = owner;
>>
>> mutex_init(®ion->mutex);
>> INIT_LIST_HEAD(®ion->bridge_list);
>> @@ -241,13 +244,14 @@ fpga_region_register_full(struct device *parent, const struct fpga_region_info *
>>
>> return ERR_PTR(ret);
>> }
>> -EXPORT_SYMBOL_GPL(fpga_region_register_full);
>> +EXPORT_SYMBOL_GPL(__fpga_region_register_full);
>>
>> /**
>> - * fpga_region_register - create and register an FPGA Region device
>> + * __fpga_region_register - create and register an FPGA Region device
>> * @parent: device parent
>> * @mgr: manager that programs this region
>> * @get_bridges: optional function to get bridges to a list
>> + * @owner: owner module containing get_bridges function
>
> ditto
>
>> *
>> * This simple version of the register function should be sufficient for most users.
>> * The fpga_region_register_full() function is available for users that need to
>> @@ -256,17 +260,17 @@ EXPORT_SYMBOL_GPL(fpga_region_register_full);
>> * Return: struct fpga_region or ERR_PTR()
>> */
>> struct fpga_region *
>> -fpga_region_register(struct device *parent, struct fpga_manager *mgr,
>> - int (*get_bridges)(struct fpga_region *))
>> +__fpga_region_register(struct device *parent, struct fpga_manager *mgr,
>> + int (*get_bridges)(struct fpga_region *), struct module *owner)
>> {
>> struct fpga_region_info info = { 0 };
>>
>> info.mgr = mgr;
>> info.get_bridges = get_bridges;
>>
>> - return fpga_region_register_full(parent, &info);
>> + return __fpga_region_register_full(parent, &info, owner);
>> }
>> -EXPORT_SYMBOL_GPL(fpga_region_register);
>> +EXPORT_SYMBOL_GPL(__fpga_region_register);
>>
>> /**
>> * fpga_region_unregister - unregister an FPGA region
>> diff --git a/include/linux/fpga/fpga-region.h b/include/linux/fpga/fpga-region.h
>> index 9d4d32909340..5fbc05fe70a6 100644
>> --- a/include/linux/fpga/fpga-region.h
>> +++ b/include/linux/fpga/fpga-region.h
>> @@ -36,6 +36,7 @@ struct fpga_region_info {
>> * @mgr: FPGA manager
>> * @info: FPGA image info
>> * @compat_id: FPGA region id for compatibility check.
>> + * @ops_owner: module containing the get_bridges function
>
> ditto
>
> Thanks,
> Yilun
>
>> * @priv: private data
>> * @get_bridges: optional function to get bridges to a list
>> */
>> @@ -46,6 +47,7 @@ struct fpga_region {
>> struct fpga_manager *mgr;
>> struct fpga_image_info *info;
>> struct fpga_compat_id *compat_id;
>> + struct module *ops_owner;
>> void *priv;
>> int (*get_bridges)(struct fpga_region *region);
>> };
>
