From: "Gupta, Nipun" <Nipun.Gupta@amd.com> To: Greg KH <gregkh@linuxfoundation.org> Cc: "robh+dt@kernel.org" <robh+dt@kernel.org>, "krzysztof.kozlowski+dt@linaro.org" <krzysztof.kozlowski+dt@linaro.org>, "rafael@kernel.org" <rafael@kernel.org>, "eric.auger@redhat.com" <eric.auger@redhat.com>, "alex.williamson@redhat.com" <alex.williamson@redhat.com>, "cohuck@redhat.com" <cohuck@redhat.com>, "song.bao.hua@hisilicon.com" <song.bao.hua@hisilicon.com>, "mchehab+huawei@kernel.org" <mchehab+huawei@kernel.org>, "maz@kernel.org" <maz@kernel.org>, "f.fainelli@gmail.com" <f.fainelli@gmail.com>, "jeffrey.l.hugo@gmail.com" <jeffrey.l.hugo@gmail.com>, "saravanak@google.com" <saravanak@google.com>, "Michael.Srba@seznam.cz" <Michael.Srba@seznam.cz>, "mani@kernel.org" <mani@kernel.org>, "yishaih@nvidia.com" <yishaih@nvidia.com>, "jgg@ziepe.ca" <jgg@ziepe.ca>, "jgg@nvidia.com" <jgg@nvidia.com>, "robin.murphy@arm.com" <robin.murphy@arm.com>, "will@kernel.org" <will@kernel.org>, "joro@8bytes.org" <joro@8bytes.org>, "masahiroy@kernel.org" <masahiroy@kernel.org>, "ndesaulniers@google.com" <ndesaulniers@google.com>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "linux-kbuild@vger.kernel.org" <linux-kbuild@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, "okaya@kernel.org" <okaya@kernel.org>, "Anand, Harpreet" <harpreet.anand@amd.com>, "Agarwal, Nikhil" <nikhil.agarwal@amd.com>, "Simek, Michal" <michal.simek@amd.com>, "git (AMD-Xilinx)" <git@amd.com> Subject: RE: [PATCH 07/19] bus/cdx: add device attributes Date: Wed, 18 Jan 2023 13:03:00 +0000 [thread overview] Message-ID: <DM6PR12MB30827B678C2D89FC68ABE84DE8C79@DM6PR12MB3082.namprd12.prod.outlook.com> (raw) In-Reply-To: <Y8atGJV6yY4mXD+W@kroah.com> [AMD Official Use Only - General] > -----Original Message----- > From: Greg KH <gregkh@linuxfoundation.org> > Sent: Tuesday, January 17, 2023 7:44 PM > To: Gupta, Nipun <Nipun.Gupta@amd.com> > Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; rafael@kernel.org; > eric.auger@redhat.com; alex.williamson@redhat.com; cohuck@redhat.com; > song.bao.hua@hisilicon.com; mchehab+huawei@kernel.org; maz@kernel.org; > f.fainelli@gmail.com; jeffrey.l.hugo@gmail.com; saravanak@google.com; > Michael.Srba@seznam.cz; mani@kernel.org; yishaih@nvidia.com; > jgg@ziepe.ca; jgg@nvidia.com; robin.murphy@arm.com; will@kernel.org; > joro@8bytes.org; masahiroy@kernel.org; ndesaulniers@google.com; linux-arm- > kernel@lists.infradead.org; linux-kbuild@vger.kernel.org; linux- > kernel@vger.kernel.org; devicetree@vger.kernel.org; okaya@kernel.org; > Anand, Harpreet <harpreet.anand@amd.com>; Agarwal, Nikhil > <nikhil.agarwal@amd.com>; Simek, Michal <michal.simek@amd.com>; git > (AMD-Xilinx) <git@amd.com> > Subject: Re: [PATCH 07/19] bus/cdx: add device attributes > > Caution: This message originated from an External Source. Use proper caution > when opening attachments, clicking links, or responding. > > > On Tue, Jan 17, 2023 at 07:11:39PM +0530, Nipun Gupta wrote: > > Create sysfs entry for CDX devices. > > > > Sysfs entries provided in each of the CDX device detected by > > the CDX controller > > - vendor id > > - device id > > - remove > > - reset of the device. > > - driver override > > > > Signed-off-by: Puneet Gupta <puneet.gupta@amd.com> > > Signed-off-by: Nipun Gupta <nipun.gupta@amd.com> > > Signed-off-by: Tarak Reddy <tarak.reddy@amd.com> > > --- > > Documentation/ABI/testing/sysfs-bus-cdx | 34 +++++ > > drivers/bus/cdx/cdx.c | 144 ++++++++++++++++++++ > > drivers/bus/cdx/controller/cdx_controller.c | 19 +++ > > drivers/bus/cdx/controller/mcdi_functions.c | 14 ++ > > drivers/bus/cdx/controller/mcdi_functions.h | 11 ++ > > include/linux/cdx/cdx_bus.h | 23 ++++ > > 6 files changed, 245 insertions(+) > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-cdx > b/Documentation/ABI/testing/sysfs-bus-cdx > > index 8c2425fdb6d9..1e0fdce18cde 100644 > > --- a/Documentation/ABI/testing/sysfs-bus-cdx > > +++ b/Documentation/ABI/testing/sysfs-bus-cdx > > @@ -10,3 +10,37 @@ Description: > > For example:: > > > > # echo 1 > /sys/bus/cdx/rescan > > + > > +What: /sys/bus/cdx/devices/.../vendor > > +Date: January 2023 > > +Contact: nipun.gupta@amd.com > > +Description: > > + Vendor ID for this CDX device > > What format is this in? How big is it? Examples? This is 16 bits. Agree need to add more info on this and other params. Will update in next version. > > > + > > +What: /sys/bus/cdx/devices/.../device > > +Date: January 2023 > > +Contact: nipun.gupta@amd.com > > +Description: > > + Device ID for this CDX device > > Same here, be specific. > > > + > > +What: /sys/bus/cdx/devices/.../reset > > +Date: January 2023 > > +Contact: nipun.gupta@amd.com > > +Description: > > + Writing a non-zero value to this file would reset the > > + CDX device > > + > > + For example:: > > + > > + # echo 1 > /sys/bus/cdx/.../reset > > Will that remove the device from the driver too? No, it will not remove the device from the driver. We would Introduce reset_done and reset_prepare callbacks which would inform the device driver about reset in next spin. > > Again, more documentation please. Sure. > > > + > > +What: /sys/bus/cdx/devices/.../remove > > +Date: January 2023 > > +Contact: tarak.reddy@amd.com > > +Description: > > + Writing a non-zero value to this file would remove the > > + corrosponding device from the CDX bus > > + > > + For example:: > > + > > + # echo 1 > /sys/bus/cdx/devices/.../remove > > Why would you want to remove a device from the bus like this? This is required to prepare the bus for applying new fabric configuration (pseudo hot plug). So that driver does not access the device while it is being reconfigured. > > > diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c > > index 2737470f08d3..1372d8dcaa4c 100644 > > --- a/drivers/bus/cdx/cdx.c > > +++ b/drivers/bus/cdx/cdx.c > > @@ -72,6 +72,38 @@ static DECLARE_BITMAP(cdx_controller_id_map, > MAX_CDX_CONTROLLERS); > > /* List of CDX controllers registered with the CDX bus */ > > static LIST_HEAD(cdx_controllers); > > > > +/** > > + * reset_cdx_device - Reset a CDX device > > + * @dev: CDX device > > + * @data: This is always passed as NULL, and is not used in this API, > > + * but is required here as the bus_for_each_dev() API expects > > + * the passed function (reset_cdx_device) to have this > > + * as an argument. > > + * > > + * @return: -errno on failure, 0 on success. > > I recommend this function actually be the one without the data pointer, > that way you don't get confused here. Agree will update this. > > > + */ > > +static int reset_cdx_device(struct device *dev, void *data) > > +{ > > + struct cdx_device *cdx_dev = to_cdx_device(dev); > > + struct cdx_controller *cdx = cdx_dev->cdx; > > + struct cdx_device_config dev_config; > > + int ret; > > + > > + dev_config.type = CDX_DEV_RESET_CONF; > > + ret = cdx->ops->dev_configure(cdx, cdx_dev->bus_num, > > + cdx_dev->dev_num, &dev_config); > > + if (ret) > > + dev_err(dev, "cdx device reset failed\n"); > > + > > + return ret; > > +} > > + > > +int cdx_dev_reset(struct device *dev) > > +{ > > + return reset_cdx_device(dev, NULL); > > +} > > +EXPORT_SYMBOL_GPL(cdx_dev_reset); > > Remove the indirection as mentioned above please. > > Otherwise, very minor comments, nice work. > > greg k-h _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: "Gupta, Nipun" <Nipun.Gupta@amd.com> To: Greg KH <gregkh@linuxfoundation.org> Cc: "robh+dt@kernel.org" <robh+dt@kernel.org>, "krzysztof.kozlowski+dt@linaro.org" <krzysztof.kozlowski+dt@linaro.org>, "rafael@kernel.org" <rafael@kernel.org>, "eric.auger@redhat.com" <eric.auger@redhat.com>, "alex.williamson@redhat.com" <alex.williamson@redhat.com>, "cohuck@redhat.com" <cohuck@redhat.com>, "song.bao.hua@hisilicon.com" <song.bao.hua@hisilicon.com>, "mchehab+huawei@kernel.org" <mchehab+huawei@kernel.org>, "maz@kernel.org" <maz@kernel.org>, "f.fainelli@gmail.com" <f.fainelli@gmail.com>, "jeffrey.l.hugo@gmail.com" <jeffrey.l.hugo@gmail.com>, "saravanak@google.com" <saravanak@google.com>, "Michael.Srba@seznam.cz" <Michael.Srba@seznam.cz>, "mani@kernel.org" <mani@kernel.org>, "yishaih@nvidia.com" <yishaih@nvidia.com>, "jgg@ziepe.ca" <jgg@ziepe.ca>, "jgg@nvidia.com" <jgg@nvidia.com>, "robin.murphy@arm.com" <robin.murphy@arm.com>, "will@kernel.org" <will@kernel.org>, "joro@8bytes.org" <joro@8bytes.org>, "masahiroy@kernel.org" <masahiroy@kernel.org>, "ndesaulniers@google.com" <ndesaulniers@google.com>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "linux-kbuild@vger.kernel.org" <linux-kbuild@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, "okaya@kernel.org" <okaya@kernel.org>, "Anand, Harpreet" <harpreet.anand@amd.com>, "Agarwal, Nikhil" <nikhil.agarwal@amd.com>, "Simek, Michal" <michal.simek@amd.com>, "git (AMD-Xilinx)" <git@amd.com> Subject: RE: [PATCH 07/19] bus/cdx: add device attributes Date: Wed, 18 Jan 2023 13:03:00 +0000 [thread overview] Message-ID: <DM6PR12MB30827B678C2D89FC68ABE84DE8C79@DM6PR12MB3082.namprd12.prod.outlook.com> (raw) In-Reply-To: <Y8atGJV6yY4mXD+W@kroah.com> [AMD Official Use Only - General] > -----Original Message----- > From: Greg KH <gregkh@linuxfoundation.org> > Sent: Tuesday, January 17, 2023 7:44 PM > To: Gupta, Nipun <Nipun.Gupta@amd.com> > Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; rafael@kernel.org; > eric.auger@redhat.com; alex.williamson@redhat.com; cohuck@redhat.com; > song.bao.hua@hisilicon.com; mchehab+huawei@kernel.org; maz@kernel.org; > f.fainelli@gmail.com; jeffrey.l.hugo@gmail.com; saravanak@google.com; > Michael.Srba@seznam.cz; mani@kernel.org; yishaih@nvidia.com; > jgg@ziepe.ca; jgg@nvidia.com; robin.murphy@arm.com; will@kernel.org; > joro@8bytes.org; masahiroy@kernel.org; ndesaulniers@google.com; linux-arm- > kernel@lists.infradead.org; linux-kbuild@vger.kernel.org; linux- > kernel@vger.kernel.org; devicetree@vger.kernel.org; okaya@kernel.org; > Anand, Harpreet <harpreet.anand@amd.com>; Agarwal, Nikhil > <nikhil.agarwal@amd.com>; Simek, Michal <michal.simek@amd.com>; git > (AMD-Xilinx) <git@amd.com> > Subject: Re: [PATCH 07/19] bus/cdx: add device attributes > > Caution: This message originated from an External Source. Use proper caution > when opening attachments, clicking links, or responding. > > > On Tue, Jan 17, 2023 at 07:11:39PM +0530, Nipun Gupta wrote: > > Create sysfs entry for CDX devices. > > > > Sysfs entries provided in each of the CDX device detected by > > the CDX controller > > - vendor id > > - device id > > - remove > > - reset of the device. > > - driver override > > > > Signed-off-by: Puneet Gupta <puneet.gupta@amd.com> > > Signed-off-by: Nipun Gupta <nipun.gupta@amd.com> > > Signed-off-by: Tarak Reddy <tarak.reddy@amd.com> > > --- > > Documentation/ABI/testing/sysfs-bus-cdx | 34 +++++ > > drivers/bus/cdx/cdx.c | 144 ++++++++++++++++++++ > > drivers/bus/cdx/controller/cdx_controller.c | 19 +++ > > drivers/bus/cdx/controller/mcdi_functions.c | 14 ++ > > drivers/bus/cdx/controller/mcdi_functions.h | 11 ++ > > include/linux/cdx/cdx_bus.h | 23 ++++ > > 6 files changed, 245 insertions(+) > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-cdx > b/Documentation/ABI/testing/sysfs-bus-cdx > > index 8c2425fdb6d9..1e0fdce18cde 100644 > > --- a/Documentation/ABI/testing/sysfs-bus-cdx > > +++ b/Documentation/ABI/testing/sysfs-bus-cdx > > @@ -10,3 +10,37 @@ Description: > > For example:: > > > > # echo 1 > /sys/bus/cdx/rescan > > + > > +What: /sys/bus/cdx/devices/.../vendor > > +Date: January 2023 > > +Contact: nipun.gupta@amd.com > > +Description: > > + Vendor ID for this CDX device > > What format is this in? How big is it? Examples? This is 16 bits. Agree need to add more info on this and other params. Will update in next version. > > > + > > +What: /sys/bus/cdx/devices/.../device > > +Date: January 2023 > > +Contact: nipun.gupta@amd.com > > +Description: > > + Device ID for this CDX device > > Same here, be specific. > > > + > > +What: /sys/bus/cdx/devices/.../reset > > +Date: January 2023 > > +Contact: nipun.gupta@amd.com > > +Description: > > + Writing a non-zero value to this file would reset the > > + CDX device > > + > > + For example:: > > + > > + # echo 1 > /sys/bus/cdx/.../reset > > Will that remove the device from the driver too? No, it will not remove the device from the driver. We would Introduce reset_done and reset_prepare callbacks which would inform the device driver about reset in next spin. > > Again, more documentation please. Sure. > > > + > > +What: /sys/bus/cdx/devices/.../remove > > +Date: January 2023 > > +Contact: tarak.reddy@amd.com > > +Description: > > + Writing a non-zero value to this file would remove the > > + corrosponding device from the CDX bus > > + > > + For example:: > > + > > + # echo 1 > /sys/bus/cdx/devices/.../remove > > Why would you want to remove a device from the bus like this? This is required to prepare the bus for applying new fabric configuration (pseudo hot plug). So that driver does not access the device while it is being reconfigured. > > > diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c > > index 2737470f08d3..1372d8dcaa4c 100644 > > --- a/drivers/bus/cdx/cdx.c > > +++ b/drivers/bus/cdx/cdx.c > > @@ -72,6 +72,38 @@ static DECLARE_BITMAP(cdx_controller_id_map, > MAX_CDX_CONTROLLERS); > > /* List of CDX controllers registered with the CDX bus */ > > static LIST_HEAD(cdx_controllers); > > > > +/** > > + * reset_cdx_device - Reset a CDX device > > + * @dev: CDX device > > + * @data: This is always passed as NULL, and is not used in this API, > > + * but is required here as the bus_for_each_dev() API expects > > + * the passed function (reset_cdx_device) to have this > > + * as an argument. > > + * > > + * @return: -errno on failure, 0 on success. > > I recommend this function actually be the one without the data pointer, > that way you don't get confused here. Agree will update this. > > > + */ > > +static int reset_cdx_device(struct device *dev, void *data) > > +{ > > + struct cdx_device *cdx_dev = to_cdx_device(dev); > > + struct cdx_controller *cdx = cdx_dev->cdx; > > + struct cdx_device_config dev_config; > > + int ret; > > + > > + dev_config.type = CDX_DEV_RESET_CONF; > > + ret = cdx->ops->dev_configure(cdx, cdx_dev->bus_num, > > + cdx_dev->dev_num, &dev_config); > > + if (ret) > > + dev_err(dev, "cdx device reset failed\n"); > > + > > + return ret; > > +} > > + > > +int cdx_dev_reset(struct device *dev) > > +{ > > + return reset_cdx_device(dev, NULL); > > +} > > +EXPORT_SYMBOL_GPL(cdx_dev_reset); > > Remove the indirection as mentioned above please. > > Otherwise, very minor comments, nice work. > > greg k-h
next prev parent reply other threads:[~2023-01-18 13:04 UTC|newest] Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-01-17 13:41 [PATCH 0/7] add support for CDX bus Nipun Gupta 2023-01-17 13:41 ` Nipun Gupta 2023-01-17 13:41 ` [PATCH 01/19] bus/cdx: add the cdx bus driver Nipun Gupta 2023-01-17 13:41 ` Nipun Gupta 2023-01-17 14:07 ` Greg KH 2023-01-17 14:07 ` Greg KH 2023-01-18 10:56 ` Gupta, Nipun 2023-01-18 10:56 ` Gupta, Nipun 2023-01-17 17:21 ` Randy Dunlap 2023-01-17 17:21 ` Randy Dunlap 2023-01-18 10:59 ` Gupta, Nipun 2023-01-18 10:59 ` Gupta, Nipun 2023-01-17 13:41 ` [PATCH 02/19] iommu/arm-smmu-v3: support ops registration for CDX bus Nipun Gupta 2023-01-17 13:41 ` Nipun Gupta 2023-01-17 13:41 ` [PATCH 03/19] dt-bindings: bus: add CDX bus controller device tree bindings Nipun Gupta 2023-01-17 13:41 ` Nipun Gupta 2023-01-17 17:54 ` Krzysztof Kozlowski 2023-01-17 17:54 ` Krzysztof Kozlowski 2023-01-18 12:39 ` Gupta, Nipun 2023-01-18 12:39 ` Gupta, Nipun 2023-01-18 12:43 ` Krzysztof Kozlowski 2023-01-18 12:43 ` Krzysztof Kozlowski 2023-01-19 7:33 ` Gupta, Nipun 2023-01-19 7:33 ` Gupta, Nipun 2023-01-17 13:41 ` [PATCH 04/19] bus/cdx: add MCDI protocol interface for firmware interaction Nipun Gupta 2023-01-17 13:41 ` Nipun Gupta 2023-01-17 14:08 ` Greg KH 2023-01-17 14:08 ` Greg KH 2023-01-18 10:58 ` Gupta, Nipun 2023-01-18 10:58 ` Gupta, Nipun 2023-01-17 13:41 ` [PATCH 05/19] bus/cdx: add cdx controller Nipun Gupta 2023-01-17 13:41 ` Nipun Gupta 2023-01-17 14:10 ` Greg KH 2023-01-17 14:10 ` Greg KH 2023-01-18 12:36 ` Gupta, Nipun 2023-01-18 12:36 ` Gupta, Nipun 2023-01-17 13:41 ` [PATCH 06/19] bus/cdx: add rpmsg communication channel for CDX Nipun Gupta 2023-01-17 13:41 ` Nipun Gupta 2023-01-17 13:41 ` [PATCH 07/19] bus/cdx: add device attributes Nipun Gupta 2023-01-17 13:41 ` Nipun Gupta 2023-01-17 14:13 ` Greg KH 2023-01-17 14:13 ` Greg KH 2023-01-18 13:03 ` Gupta, Nipun [this message] 2023-01-18 13:03 ` Gupta, Nipun 2023-01-17 17:47 ` [PATCH 0/7] add support for CDX bus Krzysztof Kozlowski 2023-01-17 17:47 ` Krzysztof Kozlowski
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=DM6PR12MB30827B678C2D89FC68ABE84DE8C79@DM6PR12MB3082.namprd12.prod.outlook.com \ --to=nipun.gupta@amd.com \ --cc=Michael.Srba@seznam.cz \ --cc=alex.williamson@redhat.com \ --cc=cohuck@redhat.com \ --cc=devicetree@vger.kernel.org \ --cc=eric.auger@redhat.com \ --cc=f.fainelli@gmail.com \ --cc=git@amd.com \ --cc=gregkh@linuxfoundation.org \ --cc=harpreet.anand@amd.com \ --cc=jeffrey.l.hugo@gmail.com \ --cc=jgg@nvidia.com \ --cc=jgg@ziepe.ca \ --cc=joro@8bytes.org \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kbuild@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mani@kernel.org \ --cc=masahiroy@kernel.org \ --cc=maz@kernel.org \ --cc=mchehab+huawei@kernel.org \ --cc=michal.simek@amd.com \ --cc=ndesaulniers@google.com \ --cc=nikhil.agarwal@amd.com \ --cc=okaya@kernel.org \ --cc=rafael@kernel.org \ --cc=robh+dt@kernel.org \ --cc=robin.murphy@arm.com \ --cc=saravanak@google.com \ --cc=song.bao.hua@hisilicon.com \ --cc=will@kernel.org \ --cc=yishaih@nvidia.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.