Re: [PATCH] staging: media: atomisp: Replace scnprintf with sysfs_emit in bo_show

From: Abdelrahman Fekry
Date: Tue Jun 24 2025 - 08:16:54 EST


On Mon, Jun 23, 2025 at 9:31 PM Hans de Goede <hansg@xxxxxxxxxx> wrote:
>
> Hi,
> As Andy already mentioned you really should try to first better understand
> the code before changing it.
>
> In this case this code is used for 2 custom sysfs attributes called
> "active_bo" and "free_bo".
>
> sysfs attributes are custom userspace API and we really want to remove
> all custom userspace APIs from this driver before moving it out of
> drivers/staging
>
> Instead everything should be done through existing standard kernels
> API, mostly the standard v4l2 API.
>
> Note this is already mentioned in drivers/staging/media/atomisp/TODO
> although these specific sysfs attributes are not named:
>
> """
> TODO
> ====
>
> 1. Items which MUST be fixed before the driver can be moved out of staging:
>
> * Remove/disable private IOCTLs
>
> * Remove/disable custom v4l2-ctrls
>
> * Remove custom sysfs files created by atomisp_drvfs.c
> """
>
> In this case the "active_bo" and "free_bo" sysfs attributes seem
> to be there for debugging purposes only and they can simply be removed.
>
> So instead of replacing scnprintf you should write a new patch
> removing everything starting at:
>
> <--- start removing code here --->
> /*
> * p: private
> * v: vmalloc
>
> ...
>
> static struct attribute_group atomisp_attribute_group[] = {
> {.attrs = sysfs_attrs_ctrl },
> };
> <--- end removing code here --->
>
> and then also remove the sysfs_create_group() and
> sysfs_remove_group() calls.
>
> After writing that patch maybe you can also take a look at tackling
> the "Remove custom sysfs files created by atomisp_drvfs.c" TODO
> list item?
>
> Regards,
>
> Hans
>

Thank you so much Hans for this informative feedback , i now see the
whole picture ,
i will submit a new patch that removes the two custom attributes
active_bo and free_bo
and then will proceed with the TODO list item " Remove custom sysfs
files created by atomisp_drvfs.c"