Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 11 Jul 2023 15:59:33 +0200 | From | Greg KH <> | Subject | Re: [PATCH 2/4] cdx: add sysfs for reset_all |
| |
On Tue, Jul 11, 2023 at 05:40:25PM +0530, Abhijit Gangurde wrote: > Add sysfs for reset_all entry which resets all the > devices on the CDX bus.
On all CDX busses, right? Why all? Why not per-bus?
> > 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 | 11 ++++++++++ > drivers/cdx/cdx.c | 29 +++++++++++++++++++++++++ > 2 files changed, 40 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-cdx b/Documentation/ABI/testing/sysfs-bus-cdx > index 0afa85b3c63b..d9e00058471d 100644 > --- a/Documentation/ABI/testing/sysfs-bus-cdx > +++ b/Documentation/ABI/testing/sysfs-bus-cdx > @@ -22,6 +22,17 @@ Description: > > # echo 0 > /sys/bus/cdx/enable > > +What: /sys/bus/cdx/reset_all > +Date: July 2023 > +Contact: puneet.gupta@amd.com > +Description: > + Writing y/1/on to this file resets all the devices > + present on the CDX bus > + > + For example:: > + > + # echo 1 > /sys/bus/cdx/reset_all
What does resetting a device mean will happen?
> + > What: /sys/bus/cdx/devices/.../vendor > Date: March 2023 > Contact: nipun.gupta@amd.com > diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c > index 48c493a43491..4d20047b55bb 100644 > --- a/drivers/cdx/cdx.c > +++ b/drivers/cdx/cdx.c > @@ -106,6 +106,11 @@ int cdx_dev_reset(struct device *dev) > } > EXPORT_SYMBOL_GPL(cdx_dev_reset); > > +static int reset_cdx_device(struct device *dev, void *data) > +{ > + return cdx_dev_reset(dev); > +} > + > /** > * cdx_unregister_device - Unregister a CDX device > * @dev: CDX device > @@ -433,9 +438,33 @@ static ssize_t rescan_store(const struct bus_type *bus, > } > static BUS_ATTR_WO(rescan); > > +static ssize_t reset_all_store(const struct bus_type *bus, > + const char *buf, size_t count) > +{ > + bool val; > + int ret; > + > + if (kstrtobool(buf, &val) < 0) > + return -EINVAL; > + > + if (!val) > + return -EINVAL; > + > + /* Reset all the devices attached to cdx bus */ > + ret = bus_for_each_dev(bus, NULL, NULL, reset_cdx_device);
No locking needed?
> + if (ret) { > + pr_err("error in CDX bus reset\n");
What error? For what device? Put the error message in the reset callback, not here, as you have no idea what device failed.
thanks,
greg k-h
| ![\](/images/icornerr.gif) |