Re: [PATCH 3/4] cdx: create sysfs resource files

[Greg KH]

On Thu, Jul 13, 2023 at 05:36:25AM +0000, Gangurde, Abhijit wrote:
> [AMD Official Use Only - General]
> 
> > > > > Resource files provides the basic MMIO regions info to the
> > > > > user-space. Also, resources<x> devices can be used to mmap the
> > > > > MMIO regions in the user-space.
> > > > >
> > > > > Co-developed-by: Puneet Gupta <puneet.gupta@xxxxxxx>
> > > > > Signed-off-by: Puneet Gupta <puneet.gupta@xxxxxxx>
> > > > > Co-developed-by: Nipun Gupta <nipun.gupta@xxxxxxx>
> > > > > Signed-off-by: Nipun Gupta <nipun.gupta@xxxxxxx>
> > > > > Signed-off-by: Abhijit Gangurde <abhijit.gangurde@xxxxxxx>
> > > > > Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-
> > > > vuuren@xxxxxxx>
> > > > > Tested-by: Nikhil Agarwal <nikhil.agarwal@xxxxxxx>
> > > > > ---
> > > > >  Documentation/ABI/testing/sysfs-bus-cdx |  15 +++
> > > > >  drivers/cdx/cdx.c                       | 139 +++++++++++++++++++++++-
> > > > >  include/linux/cdx/cdx_bus.h             |  10 ++
> > > > >  3 files changed, 163 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-cdx
> > > > b/Documentation/ABI/testing/sysfs-bus-cdx
> > > > > index d9e00058471d..6ca47b6442ce 100644
> > > > > --- a/Documentation/ABI/testing/sysfs-bus-cdx
> > > > > +++ b/Documentation/ABI/testing/sysfs-bus-cdx
> > > > > @@ -76,3 +76,18 @@ Description:
> > > > >             For example::
> > > > >
> > > > >               # echo 1 > /sys/bus/cdx/devices/.../remove
> > > > > +
> > > > > +What:              /sys/bus/cdx/devices/.../resource
> > > > > +Date:              July 2023
> > > > > +Contact:   puneet.gupta@xxxxxxx
> > > > > +Description:
> > > > > +           The resource file contains host addresses of CDX device
> > > > > +           resources. Each line of the resource file describes a region
> > > > > +           with start, end, and flag fields.
> > > >
> > > > If you documented what this file looked like here, it would be obvious
> > > > that this is not an acceptable sysfs file in any sense of the word.
> > > >
> > > > Please do so, and then fix the patch to not do that at all.
> > >
> > > Similar interface exist for pci and we intended to keep it same way. Could you
> > please elaborate on this.
> > >
> > > # cat /sys/bus/pci/devices/0000\:01\:00.0/resource
> > > 0x0000000092100000 0x00000000921fffff 0x000000000014220c
> > > 0x0000000000000000 0x0000000000000000 0x0000000000000000
> >
> > Please don't propagate incorrect decisions in the past.
> >
> > Why do you need all of these "resources" in userspace?  What tool is
> > going to read and parse them and do something with them?
> >
> > This really violates the "one value per file" sysfs rule, you are going
> > to have to have a huge reason why this is not applicable here, AND you
> > are going to have to document it very very well and get everyone to
> > agree with it.
> 
> We don't have any strong reason apart from that this is being used by some
> test and debug applications. Will drop this one now and revisit later by
> complying to the specifications.

If it's only for debug stuff, then use debugfs, that's what it is there
for.

thanks,

greg k-h