[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <DM4PR12MB7765F5E32490D551BD6C572D8FCAA@DM4PR12MB7765.namprd12.prod.outlook.com>
Date: Thu, 5 Oct 2023 11:30:07 +0000
From: "Gangurde, Abhijit" <abhijit.gangurde@....com>
To: Greg KH <gregkh@...uxfoundation.org>
CC: "masahiroy@...nel.org" <masahiroy@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Gupta, Nipun" <Nipun.Gupta@....com>,
"Agarwal, Nikhil" <nikhil.agarwal@....com>,
"Gupta, Puneet (DCG-ENG)" <puneet.gupta@....com>,
"git (AMD-Xilinx)" <git@....com>,
"Simek, Michal" <michal.simek@....com>,
"Jansen Van Vuuren, Pieter" <pieter.jansen-van-vuuren@....com>
Subject: RE: [PATCH v5 3/5] cdx: add support for bus enable and disable
> > CDX bus needs to be disabled before updating/writing devices
> > in the FPGA. Once the devices are written, the bus shall be
> > rescanned. This change provides sysfs entry to enable/disable the
> > CDX bus.
> >
> > Co-developed-by: Nipun Gupta <nipun.gupta@....com>
> > Signed-off-by: Nipun Gupta <nipun.gupta@....com>
> > Signed-off-by: Abhijit Gangurde <abhijit.gangurde@....com>
> > Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-
> vuuren@....com>
> > Tested-by: Nikhil Agarwal <nikhil.agarwal@....com>
> > ---
> > Changes in v5:
> > - Rebased on Linux 6.6
> >
> > Changes in v4:
> > - Add sparse marking for lock around cdx_find_controller() function
> >
> > Changes in v3:
> > - Use platform bus to locate controller
> >
> > Changes in v2:
> > - Improve documentation
> > - Split sysfs entry enable to enable and disable
> > - sysfs entry enable and disable take bus number as an argument
> >
> > Documentation/ABI/testing/sysfs-bus-cdx | 27 +++++++
> > drivers/cdx/cdx.c | 97 +++++++++++++++++++++++++
> > drivers/cdx/controller/cdx_controller.c | 50 +++++++++++++
> > drivers/cdx/controller/mc_cdx_pcol.h | 54 ++++++++++++++
> > drivers/cdx/controller/mcdi_functions.c | 24 ++++++
> > drivers/cdx/controller/mcdi_functions.h | 18 +++++
> > include/linux/cdx/cdx_bus.h | 12 +++
> > 7 files changed, 282 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-cdx
> b/Documentation/ABI/testing/sysfs-bus-cdx
> > index 7af477f49998..04c8dfe7e201 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-cdx
> > +++ b/Documentation/ABI/testing/sysfs-bus-cdx
> > @@ -11,6 +11,33 @@ Description:
> >
> > # echo 1 > /sys/bus/cdx/rescan
> >
> > +What: /sys/bus/cdx/enable
> > +Date: July 2023
> > +Contact: nipun.gupta@....com
> > +Description:
> > + Writing bus number in hex to this file will attempt to enable
> > + the CDX bus. The bus number for the cdx devices can be
> found
> > + at /sys/bus/cdx/devices/cdx-BB:DD, where BB denotes the
> bus
> > + number for the respective device.
> > +
> > + For example ::
> > +
> > + # echo 00 > /sys/bus/cdx/enable
> > +
> > +What: /sys/bus/cdx/disable
> > +Date: July 2023
> > +Contact: nipun.gupta@....com
> > +Description:
> > + Writing bus number in hex to this file will attempt to disable
> > + the CDX bus. CDX bus should be disabled before updating the
> > + devices in FPGA. The bus number for the cdx devices can be
> > + found at /sys/bus/cdx/devices/cdx-BB:DD, where BB denotes
> the
> > + bus number for the respective device.
> > +
> > + For example ::
> > +
> > + # echo 00 > /sys/bus/cdx/disable
> > +
>
> I can't remember, but why aren't these just attributes on the bus
> controller for that specific bus, and not a bus-wide attribute?
> Wouldn't that make more sense and remove the need for parsing values of
> a bus number and make it more obvious what bus was and was not enabled
> at any point in time (i.e. you can't read to see what bus is
> enabled/disabled right now with this change.)
Currently, bus/controller specific devices are not added on cdx bus. That is why
these attributes are bus wide. That being said, we can have these specific bus
as a cdx device which would have these attributes. Will update the series
accordingly.
Thanks,
Abhijit
Powered by blists - more mailing lists