[Linux-diag-devel] [PATCH v2 1/3] lpd: Add support for Marvell HDD LEDs on S822LC for HPC

[Mauricio Faria de Oliveira]

This patch introduces support for the identification/fault LEDs of the
Marvell SATA HDD controller present in the IBM Power System S822LC for
HPC (a.k.a. Garrison/Minsky).

indicator.h: add the indicator type 'TYPE_MARVELL'.
indicator_marvell.{h,c}:
implement the 'indicator' interface (get indices,
get/set indicator), and required hardware logic.
indicator_opal.c: call such interface on 'TYPE_MARVELL' devices.
Makefile.c: build and reference the indicator_marvell.o file.

The code for the hardware logic is based on a sample code written by
Douglas Miller <***@us.ibm.com>, acknowledged in indicator_marvell.c.

Signed-off-by: Mauricio Faria de Oliveira <***@linux.vnet.ibm.com>
---

Changelog:
v2:
- renamed filename suffix from 'mv' to 'marvell'.
- renamed MV_* variables to lowercase mv_*.
- mv_unmap_bar5(): try to close() even if munmap() fails.
- mv_set_port_mode(): removed as it's unused code.
- mv_indicator_list(): use strstr() for '/host' to end ata_device.
- get_mv_indices(): do not explicitly 'return' on void function.

lpd/Makefile | 2 +-
lpd/indicator.h | 3 +-
lpd/indicator_marvell.c | 552 ++++++++++++++++++++++++++++++++++++++++++++++++
lpd/indicator_marvell.h | 27 +++
lpd/indicator_opal.c | 8 +
5 files changed, 590 insertions(+), 2 deletions(-)
create mode 100644 lpd/indicator_marvell.c
create mode 100644 lpd/indicator_marvell.h

diff --git a/lpd/Makefile b/lpd/Makefile
index 9390fd3..d6336fc 100644
--- a/lpd/Makefile
+++ b/lpd/Makefile
@@ -9,7 +9,7 @@ CMDS = lp_diag usysident usysattn
COMMON_UTILS_OBJ = $(COMMON_DIR)/utils.o $(COMMON_DIR)/platform.o

LPD_COMMON_OBJS = files.o lp_util.o indicator_ses.o indicator_rtas.o \
- indicator_opal.o indicator.o
+ indicator_opal.o indicator_marvell.o indicator.o
COMMON_OBJS = $(COMMON_UTILS_OBJ) $(LPD_COMMON_OBJS)

LP_DIAG_OBJS = servicelog.o lp_diag.o
diff --git a/lpd/indicator.h b/lpd/indicator.h
index 81ae98c..b8b9208 100644
--- a/lpd/indicator.h
+++ b/lpd/indicator.h
@@ -43,7 +43,8 @@
#define TYPE_SES 2
#define TYPE_OS 3
#define TYPE_OPAL 4
-#define TYPE_EOL 5
+#define TYPE_MARVELL 5
+#define TYPE_EOL 6

/* Indicator state */
#define LED_STATE_SAME -1
diff --git a/lpd/indicator_marvell.c b/lpd/indicator_marvell.c
new file mode 100644
index 0000000..8a4955f
--- /dev/null
+++ b/lpd/indicator_marvell.c
@@ -0,0 +1,552 @@
+/**
+ * @file indicator_marvell.c
+ * @brief Marvell HDD LEDs (indicators) manipulation routines
+ *
+ * Copyright (C) 2016 IBM Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * 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, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * @author Mauricio Faria de Oliveira <***@linux.vnet.ibm.com>
+ * @author Douglas Miller <***@us.ibm.com>
+ */
+
+#include <ctype.h>
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include "indicator.h"
+#include "lp_util.h"
+#include "utils.h"
+
+/*
+ * For debugging messages, export this environment variable.
+ */
+#define mv_dbg(fmt, args...) if (getenv("USYSIDENT_MARVELL_DEBUG")) printf(fmt, ##args)
+
+/*
+ * Path to block devices in sysfs
+ */
+#define SYS_BLOCK_PATH "/sys/block"
+
+/*
+ * PCI IDs of the Marvell 9235 SATA controller in
+ * IBM Power System S822LC for HPC (Garrison/Minsky).
+ */
+#define MV_PCI_VID "0x1b4b"
+#define MV_PCI_DID "0x9235"
+#define MV_PCI_SVID "0x1014"
+#define MV_PCI_SSID "0x0612"
+#define MV_PCI_ID_BUF_LEN 7 /* "0x3456" and '\0' */
+
+/*
+ * Vendor-Specific Registers (VSR) are accessed indirectly
+ * through a pair of registers in PCI BAR5.
+ *
+ * First, set the VSR address in VSR_ADDR;
+ * Then, read/write data from/to VSR_DATA.
+ */
+static const uint32_t mv_pci_bar5_vsr_addr = 0xa8;
+static const uint32_t mv_pci_bar5_vsr_data = 0xac;
+static const uint32_t mv_pci_bar5_length = 2048;
+
+/*
+ * The LEDs are controlled via GPIO pins or SATA link/activity status (default).
+ *
+ * This involves 3 VSRs:
+ * - The LED mode of a SATA port is set in the PORT_ACTIVE_SEL register.
+ * (either GPIO pin state or SATA link/activity status for all/one SATA port)
+ * - The GPIO pin state is set in the DATA_OUT register.
+ * - The GPIO pin state output is enabled in the DATA_OUT_ENABLE register.
+ *
+ * On Garrison/Minsky the identification LED of the SATA port 'N' is wired
+ * to the GPIO pin of the SATA port 'N + 2' (the max. number of disks is 2).
+ * The PORT_ACTIVE_SEL and DATA_OUT_ENABLE registers are set by default so
+ * that SATA ports 3 and 4 (that is, the identification LEDs of SATA ports
+ * 1 and 2) are GPIO-pin based, and only the DATA_OUT register needs to be
+ * modified to (un)light the identification LEDs (a.k.a. fault LEDs).
+ *
+ * Register information:
+ *
+ * GPIO Data Out:
+ * - 1 bit per GPIO pin (31-0)
+ * - default: 0x00000018 (all low, except for pins 3 and 4)
+ * - The LEDs are ON on low and OFF on high.
+ *
+ * GPIO Data Out Enable:
+ * - 1 bit per GPIO bin (31-0)
+ * - default: 0xffffffe7 (all high, except for pins 3 and 4)
+ * - active low (data output enabled on bit value 0)
+ *
+ * Port Active Enable (or Port GPIOx Active Select)
+ * - bits 31-30: reserved (zero)
+ * - bits 29-00: 10x 3-bit mode (SATA ports 9-0)
+ * - default: 0x2db6da8d
+ */
+static const uint32_t mv_gpio_data_out = 0x07071020;
+static const uint32_t mv_gpio_data_out_enable = 0x07071024;
+static const uint32_t mv_gpio_port_active_sel = 0x07071058;
+
+static const uint32_t mv_port_number_offset = 2;
+static const uint32_t mv_port_number_max = 9;
+
+static const uint32_t mv_port_mode_gpio = 0x5;
+static const uint32_t mv_port_mode_max = 0x7;
+
+static char *mv_port_mode[] = {
+ "All SATAx LINK and ACT", // 0x0
+ "SATA0 LINK and ACT", // 0x1
+ "SATA1 LINK and ACT", // 0x2
+ "SATA2 LINK and ACT", // 0x3
+ "SATA3 LINK and ACT", // 0x4
+ "GPIO_DATA_OUT[n]", // 0x5
+ "Invalid (6)", // 0x6
+ "Invalid (7)" // 0x7
+};
+
+/**
+ * mv_mmap_bar5 - open() and mmap() a PCI BAR5 (resource5) file.
+ * the caller should unmap/close (see mv_munmap_bar5()).
+ *
+ * @path path to the 'resource5' file
+ * @fd pointer to a file descriptor
+ *
+ * Return:
+ * - success: pointer to the mmap()'ed PCI BAR5
+ * - failure: NULL
+ */
+static void *
+mv_mmap_bar5(const char* path, int *fd)
+{
+ void *bar5;
+
+ /* open and mmap PCI BAR5 */
+ *fd = open(path, O_RDWR);
+ if (*fd < 0) {
+ log_msg("Unable to open file '%s'", path);
+ return NULL;
+ }
+
+ bar5 = mmap(NULL, mv_pci_bar5_length, PROT_READ | PROT_WRITE,
+ MAP_SHARED, *fd, 0);
+ if (bar5 == MAP_FAILED) {
+ log_msg("Unable to mmap file '%s'", path);
+ close(*fd);
+ return NULL;
+ }
+
+ return bar5;
+}
+
+/**
+ * mv_munmap_bar5 - munmap() and close() a PCI BAR5 (resource5) file.
+ * (release resources from mv_mmap_bar5()).
+ *
+ * @bar5 pointer to the mmap()'ed PCI BAR5
+ * @fd file descriptor of the open()'ed/mmap()'ed PCI BAR5
+ *
+ * Return:
+ * - nothing
+ */
+static void
+mv_munmap_bar5(void *bar5, int fd)
+{
+ /* munmap() and close PCI BAR5 */
+ if (munmap(bar5, mv_pci_bar5_length)) {
+ log_msg("Unable to munmap file");
+ }
+
+ if (close(fd)) {
+ log_msg("Unable to close file");
+ return;
+ }
+}
+
+/**
+ * mv_vsr_read - read a VSR
+ *
+ * @bar5 pointer to mmap()'ed PCI BAR5 (see mv_mmap_bar5())
+ * @vsr_addr VSR address to read from
+ *
+ * Return:
+ * - VSR data read
+ */
+static uint32_t
+mv_vsr_read(void* bar5, uint32_t vsr_addr)
+{
+ uint32_t *addr = (uint32_t *)(bar5 + mv_pci_bar5_vsr_addr);
+ uint32_t *data = (uint32_t *)(bar5 + mv_pci_bar5_vsr_data);
+
+ /* set address and read data */
+ *addr = vsr_addr;
+ return *data;
+}
+
+/**
+ * mv_vsr_write - write a VSR
+ *
+ * @bar5 pointer to mmap()'ed PCI BAR5 (see mv_mmap_bar5())
+ * @vsr_addr VSR address to write to
+ * @vsr_data VSR data to write
+ *
+ * Return:
+ * - nothing
+ */
+static void
+mv_vsr_write(void* bar5, uint32_t vsr_addr, uint32_t vsr_data)
+{
+ uint32_t *addr = (uint32_t *)(bar5 + mv_pci_bar5_vsr_addr);
+ uint32_t *data = (uint32_t *)(bar5 + mv_pci_bar5_vsr_data);
+
+ /* set address and write data */
+ *addr = vsr_addr;
+ *data = vsr_data;
+ return;
+}
+
+/**
+ * mv_get_port_mode - get the LED mode of a SATA port (from register copy)
+ *
+ * @port_number number of the SATA port (0-9)
+ * @port_mode pointer to the LED mode (result) (see mv_port_mode[])
+ * @port_active_sel copy of the PORT_ACTIVE_SEL register
+ *
+ * Return:
+ * - -1 on failure
+ * - 0 on success
+ */
+static int
+mv_get_port_mode(uint32_t port_number, uint32_t *port_mode,
+ uint32_t port_active_sel)
+{
+ uint32_t shift, mask;
+
+ if (port_number > mv_port_number_max) {
+ log_msg("Invalid port number: '%u'", port_number);
+ return -1;
+ }
+
+ /* get the 3-bit mode of SATA port N */
+ shift = 3 * port_number;
+ mask = 0x7 << shift;
+
+ *port_mode = (port_active_sel & mask) >> shift;
+
+ return 0;
+}
+
+/**
+ * check_pci_id() - checks the value of a PCI ID file.
+ *
+ * Return:
+ * - 0 on success (open/read file, and match value)
+ * - -1 on failure
+ * - +1 on different values
+ */
+static int
+check_pci_id(char *path, char *pci_id)
+{
+ FILE *f;
+ char *pci_id_rc, pci_id_buf[MV_PCI_ID_BUF_LEN];
+
+ f = fopen(path, "r");
+ if (!f) {
+ log_msg("Unable to open file '%s'", path);
+ return -1;
+ }
+
+ pci_id_rc = fgets(pci_id_buf, MV_PCI_ID_BUF_LEN, f);
+ if (!pci_id_rc) {
+ log_msg("Unable to read file '%s'", path);
+ fclose(f);
+ return -1;
+ }
+
+ fclose(f);
+ return !!strncmp(pci_id, pci_id_buf, MV_PCI_ID_BUF_LEN);
+}
+
+/**
+ * mv_indicator_list - Build Marvell HDD LED (indicator) list for the given disk vpd
+ *
+ * @list loc_code structure
+ * @vpd dev_vpd structure
+ *
+ * Return:
+ * - 0 on success or skip vpd (not a Marvell SATA disk)
+ * - -1 on failure
+ */
+static int
+mv_indicator_list(struct loc_code **list, struct dev_vpd *vpd)
+{
+ struct loc_code *list_curr, *list_new;
+ char path[PATH_MAX], symlink[PATH_MAX];
+ char *ata_device, *host;
+ ssize_t len;
+
+ /* check for an 'sdX' device name */
+ if (strncmp(vpd->dev, "sd", 2))
+ return 0;
+
+ /* read the '/sys/block/sdX' symlink to '../device/pci.../sdX' */
+ snprintf(path, PATH_MAX, "%s/%s", SYS_BLOCK_PATH, vpd->dev);
+
+ len = readlink(path, symlink, PATH_MAX);
+ if (len < 0) {
+ log_msg("Unable to read the contents of symbolic link '%s'", path);
+ return -1;
+ }
+ symlink[len] = '\0';
+
+ /*
+ * check for an 'ataX' subdir (w/ trailing 'X' digit); for example 'ata1' in
+ * '../devices/pci<...>/0001:08:00.0/ata1/host3/target3:0:0/3:0:0:0/block/sdj'
+ */
+
+ ata_device = strstr(symlink, "/ata");
+ if (!ata_device || !isdigit(ata_device[4]))
+ return 0;
+
+ host = strstr(symlink, "/host");
+ if (!host || !isdigit(host[5]))
+ return 0;
+
+ /* split symlink into relative path for PCI device dir and ataX device */
+ ata_device[0] = '\0'; /* end symlink on leading '/' of '/ataX/' */
+ ata_device++; /* skip the leading '/' of '/ataX/' */
+ host[0] = '\0'; /* end ata_device on trailing '/' of '/ataX/',
+ * which is the leading '/' of '/hostY/' */
+
+ /*
+ * get the absolute path for the PCI device dir from symlink,
+ * which is relative to /sys/block (e.g., '../devices/pci...'),
+ * so skip the leading '../' (3 chars)
+ */
+ len = snprintf(path, PATH_MAX, "%s/%s", "/sys", &symlink[3]);
+ if (len < 0) {
+ log_msg("Unable to format absolute pathname of '%s'", symlink);
+ return -1;
+ }
+
+ /*
+ * check for the PCI IDs of the Marvell 9235 SATA controller.
+ *
+ * concatenate the PCI ID files' basename after the dirname
+ * with strncpy() (string length + 1 for '\0').
+ */
+ strncpy(&path[len], "/vendor", 8);
+ if (check_pci_id(path, MV_PCI_VID))
+ return 0;
+
+ strncpy(&path[len], "/device", 8);
+ if (check_pci_id(path, MV_PCI_DID))
+ return 0;
+
+ strncpy(&path[len], "/subsystem_vendor", 18);
+ if (check_pci_id(path, MV_PCI_SVID))
+ return 0;
+
+ strncpy(&path[len], "/subsystem_device", 18);
+ if (check_pci_id(path, MV_PCI_SSID))
+ return 0;
+
+ path[len] = '\0'; /* restore path as dirname of PCI device dir */
+
+ /* Allocate one loc_code element, and insert/append it to the list */
+ list_new = calloc(1, sizeof(struct loc_code));
+ if (!list_new) {
+ log_msg("Out of memory");
+ return 1;
+ }
+
+ if (*list) {
+ /* position list_curr in the last element of the list */
+ list_curr = *list;
+ while (list_curr->next)
+ list_curr = list_curr->next;
+
+ /* append the new element to the list */
+ list_curr->next = list_new;
+ } else {
+ /* null list; insert the new element at the list head */
+ *list = list_new;
+ }
+
+ /* set the new element's properties */
+ list_new->type = TYPE_MARVELL;
+ strncpy(list_new->code, vpd->location, LOCATION_LENGTH); /* loc. code */
+ strncpy(list_new->devname, vpd->dev, DEV_LENGTH); /* sdX device name */
+ snprintf(list_new->dev, DEV_LENGTH, "%s/resource5", path); /* PCI BAR5 */
+ list_new->index = (uint32_t)atoi(&ata_device[3]); /* use for ATA index */
+
+ mv_dbg("Marvell HDD LED:\n");
+ mv_dbg("- location code: '%s'\n", list_new->code);
+ mv_dbg("- device name (disk): '%s'\n", list_new->devname);
+ mv_dbg("- device name (pci bar5): '%s'\n", list_new->dev);
+ mv_dbg("- ata index: '%u'\n", list_new->index);
+
+ return 0;
+}
+
+/**
+ * get_mv_indices - Get Marvell HDD LEDs (indicators) list
+ *
+ * @indicator led type
+ * @loc_list loc_code structure
+ *
+ * Return:
+ * - nothing
+ */
+void
+get_mv_indices(int indicator, struct loc_code **loc_list)
+{
+ struct dev_vpd *vpd_list, *vpd_curr;
+
+ /* support for identification LEDs only */
+ if (indicator != LED_TYPE_IDENT)
+ return;
+
+ /* get block devices' vpd information */
+ vpd_list = read_device_vpd(SYS_BLOCK_PATH);
+ if (!vpd_list)
+ return;
+
+ /* search for Marvell HDDs, and append to the list */
+ for (vpd_curr = vpd_list; vpd_curr; vpd_curr = vpd_curr->next)
+ if (vpd_curr->location[0] != '\0')
+ if (mv_indicator_list(loc_list, vpd_curr))
+ break;
+
+ free_device_vpd(vpd_list);
+}
+
+/**
+ * get_mv_indicator - get Marvell HDD LED/indicator state
+ *
+ * @loc loc_code structure
+ * @state return indicator state of given loc
+ *
+ * Return:
+ * - 0 on success
+ * - -1 on failure
+ */
+int
+get_mv_indicator(int indicator, struct loc_code *loc, int *state)
+{
+ int fd;
+ void *bar5;
+ uint32_t data_out, data_out_enable, active_sel;
+ uint32_t port_number, port_mode;
+
+ /* support for identification LEDs only */
+ if (indicator != LED_TYPE_IDENT)
+ return -1;
+
+ /* read registers from PCI BAR5 */
+ bar5 = mv_mmap_bar5(loc->dev, &fd);
+ if (!bar5)
+ return -1; /* log_msg() done */
+
+ /* for debugging, read the 3 VSRs; only DATA_OUT is required */
+ data_out = mv_vsr_read(bar5, mv_gpio_data_out);
+ data_out_enable = mv_vsr_read(bar5, mv_gpio_data_out_enable);
+ active_sel = mv_vsr_read(bar5, mv_gpio_port_active_sel);
+
+ /* for debugging, get the LED mode of the SATA port 'N + offset' */
+ port_number = loc->index + mv_port_number_offset;
+
+ if (mv_get_port_mode(port_number, &port_mode, active_sel))
+ return -1; /* log_msg() done */
+
+ mv_dbg("SATA disk '%s'\n", (loc->devname) ? loc->devname : "(null)");
+ mv_dbg("Data Out: '0x%08x'\n", data_out);
+ mv_dbg("Data Out Enable: '0x%08x'\n", data_out_enable);
+ mv_dbg("Port Active Select: '0x%08x'\n", active_sel);
+ mv_dbg("ATA Index: '%u', Port Number: '%u', Port Mode: '%u' ('%s')\n",
+ loc->index, port_number, port_mode, mv_port_mode[port_mode]);
+
+ /* LED state: ON on low (bit 0) and OFF on high (bit 1) */
+ *state = (data_out & (1 << port_number)) ? LED_STATE_OFF : LED_STATE_ON;
+
+ mv_munmap_bar5(bar5, fd);
+ return 0;
+}
+
+/**
+ * set_mv_indicator - set Marvell HDD LED/indicator state
+ *
+ * @loc loc_code structure
+ * @state value to update indicator to
+ *
+ * Return:
+ * - 0 on success
+ * - -1 on failure
+ */
+int
+set_mv_indicator(int indicator, struct loc_code *loc, int new_value)
+{
+ int fd;
+ void *bar5;
+ uint32_t data_out, port_number;
+
+ /* support for identification LEDs only */
+ if (indicator != LED_TYPE_IDENT)
+ return -1;
+
+ /* sanity check */
+ if (new_value != LED_STATE_ON && new_value != LED_STATE_OFF)
+ return -1;
+
+ /* read/write registers from/to PCI BAR5 */
+ bar5 = mv_mmap_bar5(loc->dev, &fd);
+ if (!bar5)
+ return -1; /* log_msg() done */
+
+ /*
+ * only changes to the DATA_OUT register are required with the default
+ * values for the DATA_OUT_ENABLE and PORT_ACTIVE_SEL registers on the
+ * Garrison/Minsky system (see header comments).
+ */
+ data_out = mv_vsr_read(bar5, mv_gpio_data_out);
+
+ /* set the LED state of the SATA port 'N + offset' */
+ port_number = loc->index + mv_port_number_offset;
+
+ /* set/clear the bit in the DATA_OUT register;
+ * LED state: ON on low (bit 0) and OFF on high (bit 1) */
+ if (new_value == LED_STATE_ON)
+ data_out &= ~(1 << port_number);
+ else
+ data_out |= 1 << port_number;
+
+ /* update the register */
+ mv_vsr_write(bar5, mv_gpio_data_out, data_out);
+
+ /* for debugging, read the register again after writing */
+ mv_dbg("Data Out (written to): '0x%08x'\n", data_out);
+ data_out = mv_vsr_read(bar5, mv_gpio_data_out);
+ mv_dbg("Data Out (read again): '0x%08x'\n", data_out);
+
+ mv_munmap_bar5(bar5, fd);
+ return 0;
+}
diff --git a/lpd/indicator_marvell.h b/lpd/indicator_marvell.h
new file mode 100644
index 0000000..1877905
--- /dev/null
+++ b/lpd/indicator_marvell.h
@@ -0,0 +1,27 @@
+/**
+ * Copyright (C) 2016 IBM Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * 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, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+#ifndef INDICATOR_MV_H
+#define INDICATOR_MV_H
+
+/* Marvell HDD LEDs (indicators) support */
+extern void get_mv_indices(int, struct loc_code **);
+extern int get_mv_indicator(int, struct loc_code *, int *);
+extern int set_mv_indicator(int, struct loc_code *, int);
+
+#endif /* INDICATOR_MV_H */
diff --git a/lpd/indicator_opal.c b/lpd/indicator_opal.c
index 707bed0..123a1b2 100644
--- a/lpd/indicator_opal.c
+++ b/lpd/indicator_opal.c
@@ -33,6 +33,7 @@

#include "indicator.h"
#include "indicator_ses.h"
+#include "indicator_marvell.h"

/* LED directory */
#define OPAL_LED_SYSFS_PATH "/sys/class/leds"
@@ -400,6 +401,9 @@ opal_get_indicator_list(int indicator, struct loc_code **list)
/* SES indicators */
get_ses_indices(indicator, list);

+ /* Marvell HDD LEDs (indicators) */
+ get_mv_indices(indicator, list);
+
return rc;
}

@@ -421,6 +425,8 @@ opal_get_indicator_state(int indicator, struct loc_code *loc, int *state)
return opal_get_indicator(loc, state);
case TYPE_SES:
return get_ses_indicator(indicator, loc, state);
+ case TYPE_MARVELL:
+ return get_mv_indicator(indicator, loc, state);
default:
break;
}
@@ -449,6 +455,8 @@ opal_set_indicator_state(int indicator, struct loc_code *loc, int new_value)
return opal_set_indicator(loc, new_value);
case TYPE_SES:
return set_ses_indicator(indicator, loc, new_value);
+ case TYPE_MARVELL:
+ return set_mv_indicator(indicator, loc, new_value);
default:
break;
}

[Mauricio Faria de Oliveira]

Some systems/conditions have no kernel LEDs present (thus nothing in
the /sys/class/leds directory), which would cause usysident to early
exit in the OPAL platform, and not give a chance for other LED types
(e.g., Marvell HDD LEDs) to be used.

This patch adds a check for that in the OPAL platform probe function,
and in case Marvell HDD devices are found, prevent usysident to exit
early, so they can be used.

Signed-off-by: Mauricio Faria de Oliveira <***@linux.vnet.ibm.com>
---

Changelog:
v2:
- opal_indicator_probe() split into 2 specific probe functions:
one for LED class (a.k.a. platform LEDs) and another for Marvell LEDs

lpd/indicator_opal.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 51 insertions(+), 3 deletions(-)

diff --git a/lpd/indicator_opal.c b/lpd/indicator_opal.c
index 123a1b2..c43ab7a 100644
--- a/lpd/indicator_opal.c
+++ b/lpd/indicator_opal.c
@@ -282,13 +282,14 @@ opal_set_indicator(struct loc_code *loc, int new_value)


/*
- * opal_indicator_probe - Probe indicator support on OPAL based platform
+ * opal_indicator_probe_led_class - Probe LED class (/sys/class/leds)
+ * indicator support on OPAL based platform
*
* Returns:
* 0 if indicator is supported, else -1
*/
static int
-opal_indicator_probe(void)
+opal_indicator_probe_led_class(void)
{
int rc = -1;
DIR *led_dir;
@@ -314,12 +315,59 @@ opal_indicator_probe(void)
return 0;
}

- fprintf(stderr, "Service indicators are not supported on this system."
+ fprintf(stderr, "Some service indicators are not supported on this system."
"\nMake sure 'leds_powernv' kernel module is loaded.\n");
close_sysfs_led_dir(led_dir);
return rc;
}

+
+/*
+ * opal_indicator_probe_marvell - Probe Marvell indicator support on
+ * OPAL based platform
+ *
+ * Returns:
+ * 0 if indicator is supported, else -1
+ */
+static int
+opal_indicator_probe_marvell(void)
+{
+ struct loc_code *list = NULL;
+
+ get_mv_indices(LED_TYPE_IDENT, &list);
+ if (list) {
+ free_indicator_list(list);
+ return 0;
+ }
+
+ return -1;
+}
+/*
+ * opal_indicator_probe - Probe indicator support on OPAL based platform
+ *
+ * Returns:
+ * 0 if indicator is supported, else -1
+ */
+static int
+opal_indicator_probe(void)
+{
+ int rc = -1;
+
+ if (!opal_indicator_probe_led_class())
+ rc = 0;
+
+ /*
+ * Marvell HDD LEDs are not presented/controlled via kernel LEDs
+ * (i.e., /sys/class/leds), and some OPAL systems might not have
+ * any kernel LEDs (e.g., modules not loaded) but still have the
+ * Marvell SATA controller with LEDs available, and able to work.
+ */
+ if (!opal_indicator_probe_marvell())
+ rc = 0;
+
+ return rc;
+}
+
/**
* opal_get_indicator_mode - Gets the service indicator operating mode
*

[Mauricio Faria de Oliveira]

On some systems, it's been observed that different disks of the Marvell
SATA HDD controller might be listed with the very same location code.

This could cause an user to specify a device by location code, and have
the wrong device identified; if the service operation is a hotplug/pull
the disk, that error would cause serious problems.

Even though this might be a problem that requires a fix elsewhere, it's
still possible to support such scenario, with the usysident option that
specify the element by device name (e.g., '-d sda') instead of location
code (e.g., '-l -B0-T0-L0').

This patch adds a simple check and workaround for that -- restricted to
the Marvell devices (TYPE_MARVELL); once a device match is found:

- if the device is specified by device name,
then proceed normally.

- if the device is specified by location code,
then check for other devices with this location code.

- if not found,
then proceed normally.

- else,
stop, and ask the user to specify the device name with '-d'.

Signed-off-by: Mauricio Faria de Oliveira <***@linux.vnet.ibm.com>
---
Changelog:
v2:
- no changes.

lpd/usysident.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/lpd/usysident.c b/lpd/usysident.c
index 9eed946..e3dcad0 100644
--- a/lpd/usysident.c
+++ b/lpd/usysident.c
@@ -113,6 +113,7 @@ main(int argc, char **argv)
char *othervalue = NULL;
struct loc_code *current;
struct loc_code *list = NULL;
+ struct loc_code *list_start = NULL;

program_name = argv[0];
if (probe_indicator() != 0)
@@ -345,12 +346,14 @@ main(int argc, char **argv)
strncpy(temp, lvalue, LOCATION_LENGTH);
temp[LOCATION_LENGTH - 1] = '\0';

+ list_start = list;
retry:
- current = get_indicator_for_loc_code(list, lvalue);
+ current = get_indicator_for_loc_code(list_start, lvalue);
if (!current) {
if (trunc) {
if (truncate_loc_code(lvalue)) {
truncated = 1;
+ list_start = list; /* start again */
goto retry;
}
}
@@ -363,6 +366,48 @@ retry:
fprintf(stdout, "Truncated location code : "
"%s\n", lvalue);

+ if (current->type == TYPE_MARVELL) {
+
+ /*
+ * On some systems...
+ * The Marvell SATA HDD controller may have one location code
+ * for all disks. So, in order to uniquely identify a disk in
+ * the list, the -d <dev_name> (dvalue) option must be used.
+ *
+ * eg, if the -l <loc_code> (lvalue) option is used then the
+ * first disk in that location code will match, and it might
+ * be the wrong disk!
+ *
+ * So, check if there is another list entry in this system
+ * with the same location code as the current entry; if so,
+ * only handle Marvell devices if '-d <dev_name>' is used.
+ */
+ if (!dvalue &&
+ get_indicator_for_loc_code(current->next, lvalue)) {
+ fprintf(stdout, "The Marvell HDD LEDs must be "
+ "specified by device name (-d option);"
+ " non-unique location codes found.\n");
+ rc = 1;
+ goto no_retry;
+ }
+
+ /*
+ * At this point, after the check for "if (dvalue && lvalue)",
+ * if dvalue is non-NULL then lvalue is from get_loc_code_dev()
+ * (based on device name: dvalue), and dvalue is from cmdline.
+ *
+ * So, if dvalue is non-NULL and doesn't match the device name
+ * specified in 'current' (found location code), let's try the
+ * next elements in the list (which may match the device name);
+ * for this we need to retry, re-starting on the next element.
+ */
+ if (dvalue && current->devname &&
+ strncmp(dvalue, current->devname, DEV_LENGTH)) {
+ list_start = current->next;
+ goto retry;
+ }
+ }
+
if (svalue) {
rc = get_indicator_state(indicator, current,
&state);
@@ -388,6 +433,7 @@ retry:
}
} /* if-else end */
} /* lvalue end */
+no_retry:

/* Turn on/off all indicators */
if (othervalue) {