diff mbox series

[v4,20/22] net/atlantic: LED control DPDK and private APIs

Message ID a3903743df60693a271a7e0d4f7690f2160dd079.1539075891.git.igor.russkikh@aquantia.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series net/atlantic: Aquantia aQtion 10G NIC Family DPDK PMD driver |

Checks

Context Check Description
ci/Intel-compilation fail Compilation issues

Commit Message

Igor Russkikh Oct. 9, 2018, 9:32 a.m. UTC 
  Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
Signed-off-by: Pavel Belous <Pavel.Belous@aquantia.com>
---
 drivers/net/atlantic/Makefile           |  4 +++
 drivers/net/atlantic/atl_ethdev.c       | 53 +++++++++++++++++++++++++++++++++
 drivers/net/atlantic/atl_ethdev.h       |  3 ++
 drivers/net/atlantic/meson.build        |  3 ++
 drivers/net/atlantic/rte_pmd_atlantic.c | 19 ++++++++++++
 drivers/net/atlantic/rte_pmd_atlantic.h | 44 +++++++++++++++++++++++++++
 6 files changed, 126 insertions(+)
 create mode 100644 drivers/net/atlantic/rte_pmd_atlantic.c
 create mode 100644 drivers/net/atlantic/rte_pmd_atlantic.h

Comments

Ferruh Yigit Oct. 10, 2018, 10:32 a.m. UTC | #1 
On 10/9/2018 10:32 AM, Igor Russkikh wrote:
> Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
> Signed-off-by: Pavel Belous <Pavel.Belous@aquantia.com>

<...>

> +/**
> + * This is a custom API for adapter's LED controls.
> + *
> + * @param dev
> + *   Ethernet device to apply control to
> + * @param control
> + *   6 bit value (3 leds each 2bit):
> + *   - bits 0-1: LED0 control
> + *   - bits 2-3: LED1 control
> + *   - bits 4-5: LED2 control
> + *   Each two bit control value is:
> + *   - 0: Firmware manages this LED activity
> + *   - 1: Permanent ON
> + *   - 2: Blinking
> + *   - 3: Permanent OFF
> + *
> + * @return
> + *   - (0) if successful.
> + *   - (-ENOTSUP) if hardware doesn't support.
> + */
> +int rte_pmd_atl_dev_led_control(int port, int control);

What is the intention here, making PMD specific public API?
If so .map file is missing but we discourage using PMD specific APIs,

can't it be possible to extend exiting led related dev_ops in a generic way to
cover your use case?
Igor Russkikh Oct. 10, 2018, 1:35 p.m. UTC | #2 
Hi Ferruh,

>> +int rte_pmd_atl_dev_led_control(int port, int control);
> 
> What is the intention here, making PMD specific public API?
> If so .map file is missing but we discourage using PMD specific APIs,
> 
> can't it be possible to extend exiting led related dev_ops in a generic way to
> cover your use case?

This API was specially requested by our customer.

I'm honestly not sure if such a functionality will be useful as a generic dev_ops.
I'm not aware of any other NIC allowing detailed led control.

Maybe its better to drop this API from the patchset for now and resubmit
later on as a separate feature patchset.

Is this ok with you?

Regards,
  Igor
Ferruh Yigit Oct. 10, 2018, 1:54 p.m. UTC | #3 
On 10/10/2018 2:35 PM, Igor Russkikh wrote:
> Hi Ferruh,
> 
>>> +int rte_pmd_atl_dev_led_control(int port, int control);
>>
>> What is the intention here, making PMD specific public API?
>> If so .map file is missing but we discourage using PMD specific APIs,
>>
>> can't it be possible to extend exiting led related dev_ops in a generic way to
>> cover your use case?
> 
> This API was specially requested by our customer.
> 
> I'm honestly not sure if such a functionality will be useful as a generic dev_ops.
> I'm not aware of any other NIC allowing detailed led control.
> 
> Maybe its better to drop this API from the patchset for now and resubmit
> later on as a separate feature patchset.
> 
> Is this ok with you?

+1, I think better to have it as separate patch.
diff mbox series

Patch

diff --git a/drivers/net/atlantic/Makefile b/drivers/net/atlantic/Makefile
index 62dcdbffa69c..6e821e013af9 100644
--- a/drivers/net/atlantic/Makefile
+++ b/drivers/net/atlantic/Makefile
@@ -31,5 +31,9 @@  SRCS-$(CONFIG_RTE_LIBRTE_ATLANTIC_PMD) += hw_atl_utils.c
 SRCS-$(CONFIG_RTE_LIBRTE_ATLANTIC_PMD) += hw_atl_llh.c
 SRCS-$(CONFIG_RTE_LIBRTE_ATLANTIC_PMD) += hw_atl_utils_fw2x.c
 SRCS-$(CONFIG_RTE_LIBRTE_ATLANTIC_PMD) += hw_atl_b0.c
+SRCS-$(CONFIG_RTE_LIBRTE_ATLANTIC_PMD) += rte_pmd_atlantic.c
+
+# install this header file
+SYMLINK-$(CONFIG_RTE_LIBRTE_ATLANTIC_PMD)-include := rte_pmd_atlantic.h
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/atlantic/atl_ethdev.c b/drivers/net/atlantic/atl_ethdev.c
index 323769db8087..fb23dc89f88c 100644
--- a/drivers/net/atlantic/atl_ethdev.c
+++ b/drivers/net/atlantic/atl_ethdev.c
@@ -69,6 +69,10 @@  static void atl_vlan_strip_queue_set(struct rte_eth_dev *dev,
 static int atl_vlan_tpid_set(struct rte_eth_dev *dev,
 			     enum rte_vlan_type vlan_type, uint16_t tpid);
 
+/* LEDs */
+static int atl_dev_led_on(struct rte_eth_dev *dev);
+static int atl_dev_led_off(struct rte_eth_dev *dev);
+
 /* EEPROM */
 static int atl_dev_get_eeprom_length(struct rte_eth_dev *dev);
 static int atl_dev_get_eeprom(struct rte_eth_dev *dev,
@@ -276,6 +280,10 @@  static const struct eth_dev_ops atl_eth_dev_ops = {
 	.rx_descriptor_status = atl_dev_rx_descriptor_status,
 	.tx_descriptor_status = atl_dev_tx_descriptor_status,
 
+	/* LEDs */
+	.dev_led_on           = atl_dev_led_on,
+	.dev_led_off          = atl_dev_led_off,
+
 	/* EEPROM */
 	.get_eeprom_length    = atl_dev_get_eeprom_length,
 	.get_eeprom           = atl_dev_get_eeprom,
@@ -1183,6 +1191,51 @@  atl_dev_interrupt_handler(void *param)
 	atl_dev_interrupt_action(dev, dev->intr_handle);
 }
 
+/**
+ * LED ON Enables software controllable LED blinking.
+ * LED status then is independent of link status or traffic
+ */
+static int
+atl_dev_led_on(struct rte_eth_dev *dev)
+{
+	struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	if (hw->aq_fw_ops->led_control == NULL)
+		return -ENOTSUP;
+
+	return hw->aq_fw_ops->led_control(hw,
+				AQ_HW_LED_BLINK |
+				(AQ_HW_LED_BLINK << 2) |
+				(AQ_HW_LED_BLINK << 4));
+}
+
+/**
+ * LED OFF disables software controllable LED blinking
+ * LED is controlled by default logic and depends on link status and
+ * traffic activity
+ */
+static int
+atl_dev_led_off(struct rte_eth_dev *dev)
+{
+	struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	if (hw->aq_fw_ops->led_control == NULL)
+		return -ENOTSUP;
+
+	return hw->aq_fw_ops->led_control(hw, AQ_HW_LED_DEFAULT);
+}
+
+int
+atl_dev_led_control(struct rte_eth_dev *dev, int control)
+{
+	struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	if (hw->aq_fw_ops->led_control == NULL)
+		return -ENOTSUP;
+
+	return hw->aq_fw_ops->led_control(hw, control);
+}
+
 #define SFP_EEPROM_SIZE 0xff
 
 static int
diff --git a/drivers/net/atlantic/atl_ethdev.h b/drivers/net/atlantic/atl_ethdev.h
index f0854e0f1d9f..538359ac3f6f 100644
--- a/drivers/net/atlantic/atl_ethdev.h
+++ b/drivers/net/atlantic/atl_ethdev.h
@@ -105,4 +105,7 @@  uint16_t atl_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 uint16_t atl_prep_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		uint16_t nb_pkts);
 
+int
+atl_dev_led_control(struct rte_eth_dev *dev, int control);
+
 #endif /* _ATLANTIC_ETHDEV_H_ */
diff --git a/drivers/net/atlantic/meson.build b/drivers/net/atlantic/meson.build
index 28fb97cace6e..c6f39d8a7b0a 100644
--- a/drivers/net/atlantic/meson.build
+++ b/drivers/net/atlantic/meson.build
@@ -9,4 +9,7 @@  sources = files(
 	'hw_atl/hw_atl_llh.c',
 	'hw_atl/hw_atl_utils_fw2x.c',
 	'hw_atl/hw_atl_utils.c',
+	'rte_pmd_atlantic.c',
 )
+
+install_headers('rte_pmd_atlantic.h')
diff --git a/drivers/net/atlantic/rte_pmd_atlantic.c b/drivers/net/atlantic/rte_pmd_atlantic.c
new file mode 100644
index 000000000000..4cb09baf2afc
--- /dev/null
+++ b/drivers/net/atlantic/rte_pmd_atlantic.c
@@ -0,0 +1,19 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Aquantia Corporation
+ */
+
+#include <rte_ethdev_driver.h>
+
+#include "rte_pmd_atlantic.h"
+#include "atl_ethdev.h"
+
+int rte_pmd_atl_dev_led_control(int port, int control)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+	dev = &rte_eth_devices[port];
+
+	return atl_dev_led_control(dev, control);
+}
diff --git a/drivers/net/atlantic/rte_pmd_atlantic.h b/drivers/net/atlantic/rte_pmd_atlantic.h
new file mode 100644
index 000000000000..1c80330911a0
--- /dev/null
+++ b/drivers/net/atlantic/rte_pmd_atlantic.h
@@ -0,0 +1,44 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Aquantia Corporation
+ */
+
+/**
+ * @file rte_pmd_atlantic.h
+ * atlantic PMD specific functions.
+ *
+ **/
+
+#ifndef _PMD_ATLANTIC_H_
+#define _PMD_ATLANTIC_H_
+
+#include <rte_ethdev_driver.h>
+
+#define RTE_PMD_AQ_HW_LED_OFF		0x3U
+#define RTE_PMD_AQ_HW_LED_BLINK		0x2U
+#define RTE_PMD_AQ_HW_LED_ON		0x1U
+#define RTE_PMD_AQ_HW_LED_DEFAULT	0x0U
+
+/**
+ * This is a custom API for adapter's LED controls.
+ *
+ * @param dev
+ *   Ethernet device to apply control to
+ * @param control
+ *   6 bit value (3 leds each 2bit):
+ *   - bits 0-1: LED0 control
+ *   - bits 2-3: LED1 control
+ *   - bits 4-5: LED2 control
+ *   Each two bit control value is:
+ *   - 0: Firmware manages this LED activity
+ *   - 1: Permanent ON
+ *   - 2: Blinking
+ *   - 3: Permanent OFF
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if hardware doesn't support.
+ */
+int rte_pmd_atl_dev_led_control(int port, int control);
+
+
+#endif /* _PMD_ATLANTIC_H_ */