Re: [PATCH 16/37] PCI: endpoint: Introduce configfs entry for configuring EP functions

[Christoph Hellwig]

On Mon, Jan 16, 2017 at 11:31:23AM +0530, Kishon Vijay Abraham I wrote:
> Actually not all devices have hardcoded headers. E.g the platform I'm using
> doesn't have hardcoded headers and it can be configured based on the function
> the user would like to use. If the devices are hardcoded, then using configfs
> can be skipped altogether. In such cases, APIs like pci_epf_create() can
> directly be used by the drivers instead of going via configfs.

That's exactly what I meant - the IDs need to be set by the driver for
the implemented PCI device, and it's not up to the gadget core to configur
e them, it's up to the implementation of the PCIe device which PCI it
exposes.

Re: [PATCH 16/37] PCI: endpoint: Introduce configfs entry for configuring EP functions

[Kishon Vijay Abraham I]

Hi Christoph,

On Friday 13 January 2017 11:36 PM, Christoph Hellwig wrote:
> Hi Kishon,
> 
> a couple comments on the configfs layout based on my experiments with
> your previous drop to implement a NVMe device using it.

Thanks for trying it out!
> 
> I don't think most of these configfs files should be present here, as
> they are properties of the implemented PCIe devices.  E.g. for my
> NVMe device they will be sort of hardcoded most of the time, as they
> would be for other devices that would always have a fixed vendor/device/
> class ID, cacheline size, etc.

Actually not all devices have hardcoded headers. E.g the platform I'm using
doesn't have hardcoded headers and it can be configured based on the function
the user would like to use. If the devices are hardcoded, then using configfs
can be skipped altogether. In such cases, APIs like pci_epf_create() can
directly be used by the drivers instead of going via configfs.

Thanks
Kishon

Re: [PATCH 16/37] PCI: endpoint: Introduce configfs entry for configuring EP functions

[Christoph Hellwig]

Hi Kishon,

a couple comments on the configfs layout based on my experiments with
your previous drop to implement a NVMe device using it.

I don't think most of these configfs files should be present here, as
they are properties of the implemented PCIe devices.  E.g. for my
NVMe device they will be sort of hardcoded most of the time, as they
would be for other devices that would always have a fixed vendor/device/
class ID, cacheline size, etc.

In the end what we'll to be able to do here is to be able to create
a directory for each function driver, which then can create it's own
attributes inside it.

[PATCH 16/37] PCI: endpoint: Introduce configfs entry for configuring EP functions

[Kishon Vijay Abraham I]

Introduce a new configfs entry to configure the EP function (like
configuring the standard configuration header entries) and to
bind the EP function with EP controller.

Signed-off-by: Kishon Vijay Abraham I 
---
 drivers/pci/endpoint/Kconfig  |4 +-
 drivers/pci/endpoint/Makefile |2 +-
 drivers/pci/endpoint/pci-ep-cfs.c |  427 +
 3 files changed, 431 insertions(+), 2 deletions(-)
 create mode 100644 drivers/pci/endpoint/pci-ep-cfs.c

diff --git a/drivers/pci/endpoint/Kconfig b/drivers/pci/endpoint/Kconfig
index 7eb1c79..930e87a 100644
--- a/drivers/pci/endpoint/Kconfig
+++ b/drivers/pci/endpoint/Kconfig
@@ -14,7 +14,9 @@ config PCI_ENDPOINT
 
   Enabling this option will build the endpoint library, which
   includes endpoint controller library and endpoint function
-  library.
+  library. This will also enable the configfs entry required to
+  configure the endpoint function and used to bind the
+  function with a endpoint controller.
 
   If in doubt, say "N" to disable Endpoint support.
 
diff --git a/drivers/pci/endpoint/Makefile b/drivers/pci/endpoint/Makefile
index eeef1b7..a599c18 100644
--- a/drivers/pci/endpoint/Makefile
+++ b/drivers/pci/endpoint/Makefile
@@ -3,4 +3,4 @@
 #
 
 obj-$(CONFIG_PCI_ENDPOINT) := pci-epc-core.o pci-epf-core.o\
-  pci-epc-mem.o
+  pci-epc-mem.o pci-ep-cfs.o
diff --git a/drivers/pci/endpoint/pci-ep-cfs.c 
b/drivers/pci/endpoint/pci-ep-cfs.c
new file mode 100644
index 000..ed0f8c2
--- /dev/null
+++ b/drivers/pci/endpoint/pci-ep-cfs.c
@@ -0,0 +1,427 @@
+/**
+ * configfs to configure the PCI endpoint
+ *
+ * Copyright (C) 2017 Texas Instruments
+ * Author: Kishon Vijay Abraham I 
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 of
+ * the License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+struct pci_epf_info {
+   struct config_group group;
+   struct list_head list;
+   struct pci_epf *epf;
+};
+
+struct pci_ep_info {
+   struct config_group group;
+   struct config_group pci_epf_group;
+   /* mutex to protect pci_epf list */
+   struct mutex lock;
+   struct list_head pci_epf;
+   const char *epc_name;
+   struct pci_epc *epc;
+};
+
+static inline struct pci_epf_info *to_pci_epf_info(struct config_item *item)
+{
+   return container_of(to_config_group(item), struct pci_epf_info, group);
+}
+
+static inline struct pci_ep_info *to_pci_ep_info(struct config_item *item)
+{
+   return container_of(to_config_group(item), struct pci_ep_info, group);
+}
+
+#define PCI_EPF_HEADER_R(_name)
   \
+static ssize_t pci_epf_##_name##_show(struct config_item *item,char 
*page)\
+{ \
+   struct pci_epf *epf = to_pci_epf_info(item)->epf;  \
+   if (!epf->header) {\
+   WARN_ON_ONCE("epf device not bound to function driver\n"); \
+   return 0;  \
+   }  \
+   return sprintf(page, "0x%04x\n", epf->header->_name);  \
+}
+
+#define PCI_EPF_HEADER_W_u32(_name)   \
+static ssize_t pci_epf_##_name##_store(struct config_item *item,  \
+  const char *page, size_t len)   \
+{ \
+   u32 val;   \
+   int ret;   \
+   struct pci_epf *epf = to_pci_epf_info(item)->epf;  \
+   if (!epf->header) {\
+   WARN_ON_ONCE("epf device not bound to function driver\n"); \
+   return 0;  \
+   }  \
+   ret = kstrtou32(page, 0, );\
+