Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 13 Jul 2023 07:57:17 +0200
From:   Greg KH <gregkh@linuxfoundation.org>
To:     "Gangurde, Abhijit" <abhijit.gangurde@amd.com>
Cc:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Gupta, Nipun" <Nipun.Gupta@amd.com>,
        "Agarwal, Nikhil" <nikhil.agarwal@amd.com>,
        "Gupta, Puneet (DCG-ENG)" <puneet.gupta@amd.com>,
        "git (AMD-Xilinx)" <git@amd.com>,
        "Simek, Michal" <michal.simek@amd.com>,
        "Jansen Van Vuuren, Pieter" <pieter.jansen-van-vuuren@amd.com>
Subject: Re: [PATCH 3/4] cdx: create sysfs resource files
Message-ID: <2023071303-encore-emporium-b847@gregkh>
References: <20230711121027.936487-1-abhijit.gangurde@amd.com>
 <20230711121027.936487-4-abhijit.gangurde@amd.com>
 <2023071148-grievance-gulf-0841@gregkh>
 <DM4PR12MB776565FAF9F3797643C9AFA78F36A@DM4PR12MB7765.namprd12.prod.outlook.com>
 <2023071231-drinking-unskilled-3c00@gregkh>
 <DM4PR12MB7765AEF30AA787C246852C0C8F37A@DM4PR12MB7765.namprd12.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <DM4PR12MB7765AEF30AA787C246852C0C8F37A@DM4PR12MB7765.namprd12.prod.outlook.com>
Precedence: bulk

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@amd.com>
> > > > > Signed-off-by: Puneet Gupta <puneet.gupta@amd.com>
> > > > > Co-developed-by: Nipun Gupta <nipun.gupta@amd.com>
> > > > > Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
> > > > > Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
> > > > > Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-
> > > > vuuren@amd.com>
> > > > > Tested-by: Nikhil Agarwal <nikhil.agarwal@amd.com>
> > > > > ---
> > > > >  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@amd.com
> > > > > +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