[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAGn2d8MfaEpfc2a0MbuuGQ8_Z6HjTRLQVLk9AvqxWuSJSo43QA@mail.gmail.com>
Date: Tue, 24 Jun 2025 15:13:51 +0300
From: Abdelrahman Fekry <abdelrahmanfekry375@...il.com>
To: Hans de Goede <hansg@...nel.org>
Cc: andy@...nel.org, mchehab@...nel.org, sakari.ailus@...ux.intel.com,
gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org, linux-staging@...ts.linux.dev,
skhan@...uxfoundation.org, linux-kernel-mentees@...ts.linux.dev
Subject: Re: [PATCH] staging: media: atomisp: Replace scnprintf with
sysfs_emit in bo_show
On Mon, Jun 23, 2025 at 9:31 PM Hans de Goede <hansg@...nel.org> 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"
Powered by blists - more mailing lists