[PATCH 01/19] bus/cdx: add the cdx bus driver

[Gupta, Nipun]

[AMD Official Use Only - General]



> -----Original Message-----
> From: Randy Dunlap <rdunlap at infradead.org>
> Sent: Tuesday, January 17, 2023 10:52 PM
> To: Gupta, Nipun <Nipun.Gupta at amd.com>; robh+dt at kernel.org;
> krzysztof.kozlowski+dt at linaro.org; gregkh at linuxfoundation.org;
> rafael at kernel.org; eric.auger at redhat.com; alex.williamson at redhat.com;
> cohuck at redhat.com; song.bao.hua at hisilicon.com;
> mchehab+huawei at kernel.org; maz at kernel.org; f.fainelli at gmail.com;
> jeffrey.l.hugo at gmail.com; saravanak at google.com; Michael.Srba at seznam.cz;
> mani at kernel.org; yishaih at nvidia.com; jgg at ziepe.ca; jgg at nvidia.com;
> robin.murphy at arm.com; will at kernel.org; joro at 8bytes.org;
> masahiroy at kernel.org; ndesaulniers at google.com; linux-arm-
> kernel at lists.infradead.org; linux-kbuild at vger.kernel.org; linux-
> kernel at vger.kernel.org; devicetree at vger.kernel.org
> Cc: okaya at kernel.org; Anand, Harpreet <harpreet.anand at amd.com>; Agarwal,
> Nikhil <nikhil.agarwal at amd.com>; Simek, Michal <michal.simek at amd.com>;
> git (AMD-Xilinx) <git at amd.com>
> Subject: Re: [PATCH 01/19] bus/cdx: add the cdx bus driver
> 
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
> 
> 
> Hi--
> 
> On 1/17/23 05:41, Nipun Gupta wrote:
> > Introduce AMD CDX bus, which provides a mechanism for scanning
> > and probing CDX devices. These devices are memory mapped on
> > system bus for Application Processors(APUs).
> >
> > CDX devices can be changed dynamically in the Fabric and CDX
> > bus interacts with CDX controller to rescan the bus and
> > rediscover the devices.
> >
> > Signed-off-by: Nipun Gupta <nipun.gupta at amd.com>
> > Signed-off-by: Tarak Reddy <tarak.reddy at amd.com>
> > ---
> >  Documentation/ABI/testing/sysfs-bus-cdx |  12 +
> >  MAINTAINERS                             |   7 +
> >  drivers/bus/Kconfig                     |   1 +
> >  drivers/bus/Makefile                    |   2 +
> >  drivers/bus/cdx/Kconfig                 |  14 +
> >  drivers/bus/cdx/Makefile                |   8 +
> >  drivers/bus/cdx/cdx.c                   | 433 ++++++++++++++++++++++++
> >  drivers/bus/cdx/cdx.h                   |  62 ++++
> >  include/linux/cdx/cdx_bus.h             | 153 +++++++++
> >  include/linux/mod_devicetable.h         |  15 +
> >  scripts/mod/devicetable-offsets.c       |   4 +
> >  scripts/mod/file2alias.c                |  12 +
> >  12 files changed, 723 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-cdx
> >  create mode 100644 drivers/bus/cdx/Kconfig
> >  create mode 100644 drivers/bus/cdx/Makefile
> >  create mode 100644 drivers/bus/cdx/cdx.c
> >  create mode 100644 drivers/bus/cdx/cdx.h
> >  create mode 100644 include/linux/cdx/cdx_bus.h
> >
> 
> > diff --git a/drivers/bus/cdx/Kconfig b/drivers/bus/cdx/Kconfig
> > new file mode 100644
> > index 000000000000..54e0623ebcff
> > --- /dev/null
> > +++ b/drivers/bus/cdx/Kconfig
> > @@ -0,0 +1,14 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# CDX bus configuration
> > +#
> > +# Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
> > +#
> > +
> > +config CDX_BUS
> > +     bool "CDX Bus driver"
> > +     help
> > +       Driver to enable CDX Bus. CDX bus provides a mechanism for
> > +       scanning and probing of CDX devices. CDX devices are memory
> > +       mapped on system bus for embedded CPUs. CDX bus uses CDX
> > +       controller and firmware to scan the CDX devices.
> 
> Would you mind telling people who use 'make *config' what CDX means,
> either in the bool prompt string or in the help text?

Sure. Will update the help section for this.

> 
> 
> > +/**
> > + * cdx_unregister_device - Unregister 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 (cdx_unregister_device) to have this
> > + *     as an argument.
> > + *
> > + * @return: -errno on failure, 0 on success.
> 
> The syntax (or spelling) for a function's return value in kernel-doc is just:
>  * Return: -errno on failure, 0 on success.
> 
> That should be changed in many places throughout.

Agree. Will update at all places.

Thanks,
Nipun

> 
> Thanks.
> 
> --
> ~Randy