[dpdk-dev,v5,06/21] eal/soc: introduce very essential SoC infra definitions

Message ID 1477310380-17944-7-git-send-email-shreyansh.jain@nxp.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Shreyansh Jain Oct. 24, 2016, 11:59 a.m. UTC
  From: Jan Viktorin <viktorin@rehivetech.com>

Define initial structures and functions for the SoC infrastructure.
This patch supports only a very minimal functions for now.
More features will be added in the following commits.

Includes rte_device/rte_driver inheritance of
rte_soc_device/rte_soc_driver.

Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 app/test/Makefile                       |   1 +
 app/test/test_soc.c                     |  90 +++++++++++++++++++++
 lib/librte_eal/common/Makefile          |   2 +-
 lib/librte_eal/common/eal_private.h     |   4 +
 lib/librte_eal/common/include/rte_soc.h | 138 ++++++++++++++++++++++++++++++++
 5 files changed, 234 insertions(+), 1 deletion(-)
 create mode 100644 app/test/test_soc.c
 create mode 100644 lib/librte_eal/common/include/rte_soc.h
  

Comments

Jan Viktorin Oct. 24, 2016, 4:21 p.m. UTC | #1
On Mon, 24 Oct 2016 17:29:25 +0530
Shreyansh Jain <shreyansh.jain@nxp.com> wrote:

> From: Jan Viktorin <viktorin@rehivetech.com>
> 
> Define initial structures and functions for the SoC infrastructure.
> This patch supports only a very minimal functions for now.
> More features will be added in the following commits.
> 
> Includes rte_device/rte_driver inheritance of
> rte_soc_device/rte_soc_driver.
> 
> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
>  app/test/Makefile                       |   1 +
>  app/test/test_soc.c                     |  90 +++++++++++++++++++++
>  lib/librte_eal/common/Makefile          |   2 +-
>  lib/librte_eal/common/eal_private.h     |   4 +
>  lib/librte_eal/common/include/rte_soc.h | 138 ++++++++++++++++++++++++++++++++
>  5 files changed, 234 insertions(+), 1 deletion(-)
>  create mode 100644 app/test/test_soc.c
>  create mode 100644 lib/librte_eal/common/include/rte_soc.h
> 
> diff --git a/app/test/Makefile b/app/test/Makefile

[...]

> +++ b/lib/librte_eal/common/include/rte_soc.h
> @@ -0,0 +1,138 @@

[...]

> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <inttypes.h>
> +#include <string.h>
> +
> +#include <rte_dev.h>
> +#include <rte_debug.h>
> +
> +struct rte_soc_id {
> +	const char *compatible; /**< OF compatible specification */
> +	uint64_t priv_data;     /**< SoC Driver specific data */

Do you expect this to be a pointer?

> +};
> +

[...]

> +
> +/**
> + * Initialization function for the driver called during SoC probing.
> + */
> +typedef int (soc_devinit_t)(struct rte_soc_driver *, struct rte_soc_device *);
> +
> +/**
> + * Uninitialization function for the driver called during hotplugging.
> + */
> +typedef int (soc_devuninit_t)(struct rte_soc_device *);
> +
> +/**
> + * A structure describing a SoC driver.
> + */
> +struct rte_soc_driver {
> +	TAILQ_ENTRY(rte_soc_driver) next;  /**< Next in list */
> +	struct rte_driver driver;          /**< Inherit core driver. */
> +	soc_devinit_t *devinit;            /**< Device initialization */
> +	soc_devuninit_t *devuninit;        /**< Device uninitialization */

Shouldn't those functions be named probe/remove?

> +	const struct rte_soc_id *id_table; /**< ID table, NULL terminated */
> +};
> +

[...]

> +#endif
  
Shreyansh Jain Oct. 25, 2016, 5:36 a.m. UTC | #2
Hello Jan,

On Monday 24 October 2016 09:51 PM, Jan Viktorin wrote:
> On Mon, 24 Oct 2016 17:29:25 +0530
> Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>
>> From: Jan Viktorin <viktorin@rehivetech.com>
>>
>> Define initial structures and functions for the SoC infrastructure.
>> This patch supports only a very minimal functions for now.
>> More features will be added in the following commits.
>>
>> Includes rte_device/rte_driver inheritance of
>> rte_soc_device/rte_soc_driver.
>>
>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> ---
>>  app/test/Makefile                       |   1 +
>>  app/test/test_soc.c                     |  90 +++++++++++++++++++++
>>  lib/librte_eal/common/Makefile          |   2 +-
>>  lib/librte_eal/common/eal_private.h     |   4 +
>>  lib/librte_eal/common/include/rte_soc.h | 138 ++++++++++++++++++++++++++++++++
>>  5 files changed, 234 insertions(+), 1 deletion(-)
>>  create mode 100644 app/test/test_soc.c
>>  create mode 100644 lib/librte_eal/common/include/rte_soc.h
>>
>> diff --git a/app/test/Makefile b/app/test/Makefile
>
> [...]
>
>> +++ b/lib/librte_eal/common/include/rte_soc.h
>> @@ -0,0 +1,138 @@
>
> [...]
>
>> +
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <stdint.h>
>> +#include <inttypes.h>
>> +#include <string.h>
>> +
>> +#include <rte_dev.h>
>> +#include <rte_debug.h>
>> +
>> +struct rte_soc_id {
>> +	const char *compatible; /**< OF compatible specification */
>> +	uint64_t priv_data;     /**< SoC Driver specific data */
>
> Do you expect this to be a pointer?

A 64 bit entry, which can be typecasted to pointer by implementations, 
if required. Or, it might as well remain as a 64bit entry as ID.

>
>> +};
>> +
>
> [...]
>
>> +
>> +/**
>> + * Initialization function for the driver called during SoC probing.
>> + */
>> +typedef int (soc_devinit_t)(struct rte_soc_driver *, struct rte_soc_device *);
>> +
>> +/**
>> + * Uninitialization function for the driver called during hotplugging.
>> + */
>> +typedef int (soc_devuninit_t)(struct rte_soc_device *);
>> +
>> +/**
>> + * A structure describing a SoC driver.
>> + */
>> +struct rte_soc_driver {
>> +	TAILQ_ENTRY(rte_soc_driver) next;  /**< Next in list */
>> +	struct rte_driver driver;          /**< Inherit core driver. */
>> +	soc_devinit_t *devinit;            /**< Device initialization */
>> +	soc_devuninit_t *devuninit;        /**< Device uninitialization */
>
> Shouldn't those functions be named probe/remove?

Indeed. I think there was a comment on v4 as well - I thought I had 
fixed it but it seems I have mixed up my patches. I will send v6 
immediately with this fixed. Thanks for pointing out.

>
>> +	const struct rte_soc_id *id_table; /**< ID table, NULL terminated */
>> +};
>> +
>
> [...]
>
>> +#endif
>
>
>

-
Shreyansh
  
Shreyansh Jain Oct. 25, 2016, 12:38 p.m. UTC | #3
On Tuesday 25 October 2016 11:06 AM, Shreyansh Jain wrote:
> Hello Jan,
>
> On Monday 24 October 2016 09:51 PM, Jan Viktorin wrote:
>> On Mon, 24 Oct 2016 17:29:25 +0530
>> Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>>
>>> From: Jan Viktorin <viktorin@rehivetech.com>
>>>
>>> Define initial structures and functions for the SoC infrastructure.
>>> This patch supports only a very minimal functions for now.
>>> More features will be added in the following commits.
>>>
>>> Includes rte_device/rte_driver inheritance of
>>> rte_soc_device/rte_soc_driver.
>>>
>>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
>>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>>> ---
>>>  app/test/Makefile                       |   1 +
>>>  app/test/test_soc.c                     |  90 +++++++++++++++++++++
>>>  lib/librte_eal/common/Makefile          |   2 +-
>>>  lib/librte_eal/common/eal_private.h     |   4 +
>>>  lib/librte_eal/common/include/rte_soc.h | 138
>>> ++++++++++++++++++++++++++++++++
>>>  5 files changed, 234 insertions(+), 1 deletion(-)
>>>  create mode 100644 app/test/test_soc.c
>>>  create mode 100644 lib/librte_eal/common/include/rte_soc.h
>>>
>>> diff --git a/app/test/Makefile b/app/test/Makefile
>>
>> [...]
>>
>>> +++ b/lib/librte_eal/common/include/rte_soc.h
>>> @@ -0,0 +1,138 @@
>>
>> [...]
>>
>>> +
>>> +#include <stdio.h>
>>> +#include <stdlib.h>
>>> +#include <stdint.h>
>>> +#include <inttypes.h>
>>> +#include <string.h>
>>> +
>>> +#include <rte_dev.h>
>>> +#include <rte_debug.h>
>>> +
>>> +struct rte_soc_id {
>>> +    const char *compatible; /**< OF compatible specification */
>>> +    uint64_t priv_data;     /**< SoC Driver specific data */
>>
>> Do you expect this to be a pointer?
>
> A 64 bit entry, which can be typecasted to pointer by implementations,
> if required. Or, it might as well remain as a 64bit entry as ID.
>
>>
>>> +};
>>> +
>>
>> [...]
>>
>>> +
>>> +/**
>>> + * Initialization function for the driver called during SoC probing.
>>> + */
>>> +typedef int (soc_devinit_t)(struct rte_soc_driver *, struct
>>> rte_soc_device *);
>>> +
>>> +/**
>>> + * Uninitialization function for the driver called during hotplugging.
>>> + */
>>> +typedef int (soc_devuninit_t)(struct rte_soc_device *);
>>> +
>>> +/**
>>> + * A structure describing a SoC driver.
>>> + */
>>> +struct rte_soc_driver {
>>> +    TAILQ_ENTRY(rte_soc_driver) next;  /**< Next in list */
>>> +    struct rte_driver driver;          /**< Inherit core driver. */
>>> +    soc_devinit_t *devinit;            /**< Device initialization */
>>> +    soc_devuninit_t *devuninit;        /**< Device uninitialization */
>>
>> Shouldn't those functions be named probe/remove?
>
> Indeed. I think there was a comment on v4 as well - I thought I had
> fixed it but it seems I have mixed up my patches. I will send v6
> immediately with this fixed. Thanks for pointing out.

Ah, I just noticed that I did change it - but in Patch 11. Ideally, it 
should have been done here itself. My bad.

>
>>
>>> +    const struct rte_soc_id *id_table; /**< ID table, NULL
>>> terminated */
>>> +};
>>> +
>>
>> [...]
>>
>>> +#endif
>>
>>
>>
>
> -
> Shreyansh
>

-
Shreyansh
  

Patch

diff --git a/app/test/Makefile b/app/test/Makefile
index 5be023a..30295af 100644
--- a/app/test/Makefile
+++ b/app/test/Makefile
@@ -77,6 +77,7 @@  APP = test
 #
 SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) := commands.c
 SRCS-y += test.c
+SRCS-y += test_soc.c
 SRCS-y += resource.c
 SRCS-y += test_resource.c
 test_resource.res: test_resource.c
diff --git a/app/test/test_soc.c b/app/test/test_soc.c
new file mode 100644
index 0000000..916a863
--- /dev/null
+++ b/app/test/test_soc.c
@@ -0,0 +1,90 @@ 
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2016 RehiveTech. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of RehiveTech nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <stdint.h>
+#include <sys/queue.h>
+
+#include <rte_soc.h>
+#include <rte_devargs.h>
+#include <rte_debug.h>
+
+#include "test.h"
+
+static char *safe_strdup(const char *s)
+{
+	char *c = strdup(s);
+
+	if (c == NULL)
+		rte_panic("failed to strdup '%s'\n", s);
+
+	return c;
+}
+
+static int test_compare_addr(void)
+{
+	struct rte_soc_addr a0;
+	struct rte_soc_addr a1;
+	struct rte_soc_addr a2;
+
+	a0.name = safe_strdup("ethernet0");
+	a0.fdt_path = NULL;
+
+	a1.name = safe_strdup("ethernet0");
+	a1.fdt_path = NULL;
+
+	a2.name = safe_strdup("ethernet1");
+	a2.fdt_path = NULL;
+
+	TEST_ASSERT(!rte_eal_compare_soc_addr(&a0, &a1),
+		    "Failed to compare two soc addresses that equal");
+	TEST_ASSERT(rte_eal_compare_soc_addr(&a0, &a2),
+		    "Failed to compare two soc addresses that differs");
+
+	free(a2.name);
+	free(a1.name);
+	free(a0.name);
+	return 0;
+}
+
+static int
+test_soc(void)
+{
+	if (test_compare_addr())
+		return -1;
+
+	return 0;
+}
+
+REGISTER_TEST_COMMAND(soc_autotest, test_soc);
diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile
index dfd64aa..b414008 100644
--- a/lib/librte_eal/common/Makefile
+++ b/lib/librte_eal/common/Makefile
@@ -33,7 +33,7 @@  include $(RTE_SDK)/mk/rte.vars.mk
 
 INC := rte_branch_prediction.h rte_common.h
 INC += rte_debug.h rte_eal.h rte_errno.h rte_launch.h rte_lcore.h
-INC += rte_log.h rte_memory.h rte_memzone.h rte_pci.h
+INC += rte_log.h rte_memory.h rte_memzone.h rte_soc.h rte_pci.h
 INC += rte_per_lcore.h rte_random.h
 INC += rte_tailq.h rte_interrupts.h rte_alarm.h
 INC += rte_string_fns.h rte_version.h
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index c8c2131..0e8d6f7 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -36,6 +36,7 @@ 
 
 #include <stdio.h>
 #include <rte_pci.h>
+#include <rte_soc.h>
 
 /**
  * Initialize the memzone subsystem (private to eal).
@@ -118,6 +119,9 @@  int rte_eal_log_init(const char *id, int facility);
  */
 int rte_eal_pci_init(void);
 
+struct rte_soc_driver;
+struct rte_soc_device;
+
 struct rte_pci_driver;
 struct rte_pci_device;
 
diff --git a/lib/librte_eal/common/include/rte_soc.h b/lib/librte_eal/common/include/rte_soc.h
new file mode 100644
index 0000000..bc0a43b
--- /dev/null
+++ b/lib/librte_eal/common/include/rte_soc.h
@@ -0,0 +1,138 @@ 
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2016 RehiveTech. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of RehiveTech nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_SOC_H_
+#define _RTE_SOC_H_
+
+/**
+ * @file
+ *
+ * RTE SoC Interface
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <inttypes.h>
+#include <string.h>
+
+#include <rte_dev.h>
+#include <rte_debug.h>
+
+struct rte_soc_id {
+	const char *compatible; /**< OF compatible specification */
+	uint64_t priv_data;     /**< SoC Driver specific data */
+};
+
+struct rte_soc_addr {
+	char *name;     /**< name used in sysfs */
+	char *fdt_path; /**< path to the associated node in FDT */
+};
+
+/**
+ * A structure describing a SoC device.
+ */
+struct rte_soc_device {
+	TAILQ_ENTRY(rte_soc_device) next;   /**< Next probed SoC device */
+	struct rte_device device;           /**< Inherit code device */
+	struct rte_soc_addr addr;           /**< SoC device Location */
+	struct rte_soc_id *id;              /**< SoC device ID list */
+	struct rte_soc_driver *driver;      /**< Associated driver */
+};
+
+struct rte_soc_driver;
+
+/**
+ * Initialization function for the driver called during SoC probing.
+ */
+typedef int (soc_devinit_t)(struct rte_soc_driver *, struct rte_soc_device *);
+
+/**
+ * Uninitialization function for the driver called during hotplugging.
+ */
+typedef int (soc_devuninit_t)(struct rte_soc_device *);
+
+/**
+ * A structure describing a SoC driver.
+ */
+struct rte_soc_driver {
+	TAILQ_ENTRY(rte_soc_driver) next;  /**< Next in list */
+	struct rte_driver driver;          /**< Inherit core driver. */
+	soc_devinit_t *devinit;            /**< Device initialization */
+	soc_devuninit_t *devuninit;        /**< Device uninitialization */
+	const struct rte_soc_id *id_table; /**< ID table, NULL terminated */
+};
+
+/**
+ * Utility function to write a SoC device name, this device name can later be
+ * used to retrieve the corresponding rte_soc_addr using above functions.
+ *
+ * @param addr
+ *	The SoC address
+ * @param output
+ *	The output buffer string
+ * @param size
+ *	The output buffer size
+ * @return
+ *  0 on success, negative on error.
+ */
+static inline void
+rte_eal_soc_device_name(const struct rte_soc_addr *addr,
+			char *output, size_t size)
+{
+	int ret;
+
+	RTE_VERIFY(addr != NULL);
+	RTE_VERIFY(size >= strlen(addr->name));
+	ret = snprintf(output, size, "%s", addr->name);
+	RTE_VERIFY(ret >= 0);
+}
+
+static inline int
+rte_eal_compare_soc_addr(const struct rte_soc_addr *a0,
+			 const struct rte_soc_addr *a1)
+{
+	if (a0 == NULL || a1 == NULL)
+		return -1;
+
+	RTE_VERIFY(a0->name != NULL);
+	RTE_VERIFY(a1->name != NULL);
+
+	return strcmp(a0->name, a1->name);
+}
+
+#endif