mts-io: reorganize accessory card attributes, move them into subdirs under mts-io

	subdirs named ap1, ap2, etc
	create links to subdirs with name of card in port, e.g. gpiob, mfser, eth, etc
	if 2 of same type of card, still make links but use gpiob-2, mfser-2, etc

5 files changed, 408 insertions, 249 deletions

diff --git a/io-module/mtac.c b/io-module/mtac.c
index 1c88b40..9b27e0d 100644
--- a/io-module/mtac.c
+++ b/io-module/mtac.c
@@ -4,7 +4,7 @@ static struct kobj_attribute* create_attribute(const char* _name, umode_t _mode)
 	
 	_attr = kzalloc(sizeof(struct kobj_attribute), GFP_KERNEL);
 	if (! _attr) {
-		log_error("kzalloc of attribute %s failed", _name);
+		log_error("kzalloc of attribute [%s] failed", _name);
 		return NULL;
 	}
 
@@ -21,48 +21,79 @@ static struct kobj_attribute* create_attribute(const char* _name, umode_t _mode)
 	return _attr;
 }
 
+static int port_from_kobject(struct kobject *kobj) {
+	int port;
+	const char *name;
+
+	name = kobj->name;
+	if (! name) {
+		log_error("kobject->name is NULL");
+		return -1;
+	}
+
+	if (sscanf(name, "ap%d", &port) < 1) {
+		log_error("failed to scan port from kobject->name [%s]", name);
+		return -1;
+	}
+
+	if (port < 1 || port > NUM_AP) {
+		log_error("port number %d is invalid", port);
+		return -1;
+	}
+
+	return port;
+}
+
 static ssize_t ap_show_product_info(struct kobject *kobj, struct kobj_attribute *attr, char *buf) {
 	ssize_t value;
 	int port;
+	int port_index;
 
-	if (sscanf(attr->attr.name, "vendor-id:%d", &port) > 0) {
-		value = snprintf(buf, 32, "%s\n", ap_eeprom[port - 1].vendor_id);
-	} else if (sscanf(attr->attr.name, "product-id:%d", &port) > 0) {
-		value = snprintf(buf, 32, "%s\n", ap_eeprom[port - 1].product_id);
-	} else if (sscanf(attr->attr.name, "device-id:%d", &port) > 0) {
-		value = snprintf(buf, 32, "%s\n", ap_eeprom[port - 1].device_id);
-	} else if (sscanf(attr->attr.name, "hw-version:%d", &port) > 0) {
-		value = snprintf(buf, 32, "%s\n", ap_eeprom[port - 1].hw_version);
-	} else if (sscanf(attr->attr.name, "mac-eth:%d", &port) > 0) {
+	port = port_from_kobject(kobj);
+	if (port < 1) {
+		log_error("port_from_kobject returned %d", port);
+		return -1;
+	}
+	port_index = port - 1;
+
+	if (! strcmp(attr->attr.name, "vendor-id")) {
+		value = snprintf(buf, 32, "%s\n", ap_eeprom[port_index].vendor_id);
+	} else if (! strcmp(attr->attr.name, "product-id")) {
+		value = snprintf(buf, 32, "%s\n", ap_eeprom[port_index].product_id);
+	} else if (! strcmp(attr->attr.name, "device-id")) {
+		value = snprintf(buf, 32, "%s\n", ap_eeprom[port_index].device_id);
+	} else if (! strcmp(attr->attr.name, "hw-version")) {
+		value = snprintf(buf, 32, "%s\n", ap_eeprom[port_index].hw_version);
+	} else if (! strcmp(attr->attr.name, "mac-eth")) {
 		value = sprintf(buf, "%02X:%02X:%02X:%02X:%02X:%02X\n",
-			ap_eeprom[port - 1].mac_addr[0],
-			ap_eeprom[port - 1].mac_addr[1],
-			ap_eeprom[port - 1].mac_addr[2],
-			ap_eeprom[port - 1].mac_addr[3],
-			ap_eeprom[port - 1].mac_addr[4],
-			ap_eeprom[port - 1].mac_addr[5]);
+			ap_eeprom[port_index].mac_addr[0],
+			ap_eeprom[port_index].mac_addr[1],
+			ap_eeprom[port_index].mac_addr[2],
+			ap_eeprom[port_index].mac_addr[3],
+			ap_eeprom[port_index].mac_addr[4],
+			ap_eeprom[port_index].mac_addr[5]);
 	} else {
-		log_error("attribute '%s' not found", attr->attr.name);
+		log_error("attribute [%s] not found", attr->attr.name);
 		value = -1;
 	}
 
 	return value;
 }
 
-static bool ap_add_product_info_attributes(int port, int type) {
-	struct kobj_attribute *attr;
+static bool ap_add_product_info_attributes(int port, int type, struct attribute** attrs, int* index) {
 	char buf[32];
+	struct kobj_attribute* kobj_attr;
 
 	switch (type) {
 		case MTAC_ETH_0_0:
-			sprintf(buf, "mac-eth:%d", port);
-			attr = create_attribute(buf, MTS_ATTR_MODE_RO);
-			if (! attr) {
-				log_error("failed to create attribute[%s]", buf);
+			sprintf(buf, "mac-eth");
+			kobj_attr = create_attribute(buf, MTS_ATTR_MODE_RO);
+			if (! kobj_attr) {
+				log_error("failed to create attribute [%s] in port %d", buf, port);
 				return false;
 			}
-			attr->show = ap_show_product_info;
-			device_attrs[device_attrs_size++] = &attr->attr;
+			kobj_attr->show = ap_show_product_info;
+			attrs[*index++] = &kobj_attr->attr;
 			break;
 
 		case MTAC_GPIOB_0_0:
@@ -73,65 +104,51 @@ static bool ap_add_product_info_attributes(int port, int type) {
 			return false;
 	}
 
-	sprintf(buf, "vendor-id:%d", port);
-	attr = create_attribute(buf, MTS_ATTR_MODE_RO);
-	if (! attr) {
-		log_error("failed to create attribute[%s]", buf);
+	sprintf(buf, "vendor-id");
+	kobj_attr = create_attribute(buf, MTS_ATTR_MODE_RO);
+	if (! kobj_attr) {
+		log_error("failed to create attribute [%s] in port %d", buf, port);
 		return false;
 	}
-	attr->show = ap_show_product_info;
-	device_attrs[device_attrs_size++] = &attr->attr;
+	kobj_attr->show = ap_show_product_info;
+	attrs[(*index)++] = &kobj_attr->attr;
 
-	sprintf(buf, "product-id:%d", port);
-	attr = create_attribute(buf, MTS_ATTR_MODE_RO);
-	if (! attr) {
-		log_error("failed to create attribute[%s]", buf);
+	sprintf(buf, "product-id");
+	kobj_attr = create_attribute(buf, MTS_ATTR_MODE_RO);
+	if (! kobj_attr) {
+		log_error("failed to create attribute [%s] in port %d", buf, port);
 		return false;
 	}
-	attr->show = ap_show_product_info;
-	device_attrs[device_attrs_size++] = &attr->attr;
+	kobj_attr->show = ap_show_product_info;
+	attrs[(*index)++] = &kobj_attr->attr;
 
-	sprintf(buf, "device-id:%d", port);
-	attr = create_attribute(buf, MTS_ATTR_MODE_RO);
-	if (! attr) {
-		log_error("failed to create attribute[%s]", buf);
+	sprintf(buf, "device-id");
+	kobj_attr = create_attribute(buf, MTS_ATTR_MODE_RO);
+	if (! kobj_attr) {
+		log_error("failed to create attribute [%s] in port %d", buf, port);
 		return false;
 	}
-	attr->show = ap_show_product_info;
-	device_attrs[device_attrs_size++] = &attr->attr;
+	kobj_attr->show = ap_show_product_info;
+	attrs[(*index)++] = &kobj_attr->attr;
 
-	sprintf(buf, "hw-version:%d", port);
-	attr = create_attribute(buf, MTS_ATTR_MODE_RO);
-	if (! attr) {
-		log_error("failed to create attribute[%s]", buf);
+	sprintf(buf, "hw-version");
+	kobj_attr = create_attribute(buf, MTS_ATTR_MODE_RO);
+	if (! kobj_attr) {
+		log_error("failed to create attribute [%s] in port %d", buf, port);
 		return false;
 	}
-	attr->show = ap_show_product_info;
-	device_attrs[device_attrs_size++] = &attr->attr;
+	kobj_attr->show = ap_show_product_info;
+	attrs[(*index)++] = &kobj_attr->attr;
 
 	return true;
 }
 
-struct gpio_pin *ap_gpio_pin_by_attr_name(const char *name) {
+struct gpio_pin *ap_gpio_pin_by_attr_name(const char *name, int port) {
 	struct gpio_pin *pin;
 	char *pin_attr_name;
-	long port;
-	int port_index;
-	char *colon;
+	int port_index = port - 1;
 
-	colon = strstr(name, ":");
-	if (colon && ++colon) {
-		if (kstrtol(colon, 10, &port)) {
-			log_error("kstrtol failed on [%s]", colon);
-			return NULL;
-		}
-	} else {
-		log_error("could not read port from attr name [%s]", name);
-		return NULL;
-	}
-	port_index = port - 1;
-
-	pin_attr_name = port_info[port_index]->gpio_pin_name_by_attr_name(name);
+	pin_attr_name = port_info[port_index]->gpio_pin_name_by_attr_name(name, port);
 
 	for (pin = gpio_pins; *pin->name; pin++) {
 		if (!strcmp(pin->pin.label, pin_attr_name)) {
@@ -148,8 +165,16 @@ static ssize_t mts_attr_show_ap_gpio_pin(struct kobject *kobj,
 			char *buf)
 {
 	int value;
-	struct gpio_pin *pin = ap_gpio_pin_by_attr_name(attr->attr.name);
+	int port;
+	struct gpio_pin *pin;
+	   
+	port = port_from_kobject(kobj);
+	if (port < 1) {
+		log_error("port_from_kobject returned %d", port);
+		return -EINVAL;
+	}
 
+	pin	= ap_gpio_pin_by_attr_name(attr->attr.name, port);
 	if (!pin) {
 		return -ENODEV;
 	}
@@ -175,8 +200,16 @@ static ssize_t mts_attr_store_ap_gpio_pin(struct kobject *kobj,
 		struct kobj_attribute *attr, const char *buf, size_t count)
 {
 	int value;
-	struct gpio_pin *pin = ap_gpio_pin_by_attr_name(attr->attr.name);
+	int port;
+	struct gpio_pin *pin;
 
+	port = port_from_kobject(kobj);
+	if (port < 1) {
+		log_error("port_from_kobject returned %d", port);
+		return -EINVAL;
+	}
+	   
+	pin	= ap_gpio_pin_by_attr_name(attr->attr.name, port);
 	if (!pin) {
 		return -ENODEV;
 	}
diff --git a/io-module/mtac_gpiob.c b/io-module/mtac_gpiob.c
index f8652c6..3f9631d 100644
--- a/io-module/mtac_gpiob.c
+++ b/io-module/mtac_gpiob.c
@@ -1,7 +1,7 @@
 struct spi_device *gpiob_spi[NUM_AP][3];
 struct spi_driver gpiob_spi_drivers[NUM_AP][3];
 
-static u8 spi_ap_dout_value;
+static u8 spi_ap_dout_value[NUM_AP];
 static DEFINE_MUTEX(spi_ap_dout_mutex);
 static unsigned int ap_dout_max_speed_hz = 1 * 1000 * 1000;
 module_param(ap_dout_max_speed_hz, uint, S_IRUGO);
@@ -41,7 +41,7 @@ static int mts_spi_ap_probe(struct spi_device *spi)
 	gpiob_get_dev_info_from_modalias(spi->modalias, &port, buf);
 	port_index = port - 1;
 	if (port < 1 || port > NUM_AP) {
-		log_error("port [%d] is invalid", port);
+		log_error("port %d is invalid", port);
 		return -ENODEV;
 	}
 
@@ -66,13 +66,13 @@ static int mts_spi_ap_probe(struct spi_device *spi)
 
 	tmp = spi_setup(gpiob_spi[port_index][dev]);
 	if (tmp < 0) {
-		log_error("spi_setup ap %d %s failed", port, buf);
+		log_error("spi_setup ap %d [%s] failed", port, buf);
 		return tmp;
 	}
 
 	if (dev == dout) {
-		spi_ap_dout_value = 0x00;
-		spi_writen(gpiob_spi[port_index][dev], &spi_ap_dout_value, 1);
+		spi_ap_dout_value[port_index] = 0x00;
+		spi_writen(gpiob_spi[port_index][dev], &spi_ap_dout_value[port_index], 1);
 	}
 
 	return 0;
@@ -87,7 +87,7 @@ static int mts_spi_ap_remove(struct spi_device *spi)
 	gpiob_get_dev_info_from_modalias(spi->modalias, &port, buf);
 	port_index = port - 1;
 	if (port < 1 || port > NUM_AP) {
-		log_error("port [%d] is invalid", port);
+		log_error("port %d is invalid", port);
 		return -ENODEV;
 	}
 
@@ -105,26 +105,35 @@ static int mts_spi_ap_remove(struct spi_device *spi)
 	return 0;
 }
 
-static char* gpiob_gpio_pin_name_by_attr_name(const char* name) {
-	if (! strcmp(name, "led1:1")) {
-		return "ap1-gpio3";
-	} else if (! strcmp(name, "led2:1")) {
-		return "ap1-gpio4";
-	} else if (! strcmp(name, "dout-enable:1")) {
-		return "ap1-gpio1";
-	} else if (! strcmp(name, "reset:1")) {
-		return "ap1-reset";
-	} else if (! strcmp(name, "led1:2")) {
-		return "ap2-gpio3";
-	} else if (! strcmp(name, "led2:2")) {
-		return "ap2-gpio4";
-	} else if (! strcmp(name, "dout-enable:2")) {
-		return "ap2-gpio1";
-	} else if (! strcmp(name, "reset:2")) {
-		return "ap2-reset";
-	} else {
-		log_error("attirbute name [%s] is invalid for GPIOB", name);
-		return "";
+static char* gpiob_gpio_pin_name_by_attr_name(const char* name, int port) {
+	switch (port) {
+		case port_1:
+			if (! strcmp(name, "led1")) {
+				return "ap1-gpio3";
+			} else if (! strcmp(name, "led2")) {
+				return "ap1-gpio4";
+			} else if (! strcmp(name, "dout-enable")) {
+				return "ap1-gpio1";
+			} else if (! strcmp(name, "reset")) {
+				return "ap1-reset";
+			} else {
+				log_error("attirbute name [%s] is invalid for GPIOB in port %d", name, port);
+				return "";
+			}
+
+		case port_2:
+			if (! strcmp(name, "led1")) {
+				return "ap2-gpio3";
+			} else if (! strcmp(name, "led2")) {
+				return "ap2-gpio4";
+			} else if (! strcmp(name, "dout-enable")) {
+				return "ap2-gpio1";
+			} else if (! strcmp(name, "reset")) {
+				return "ap2-reset";
+			} else {
+				log_error("attirbute name [%s] is invalid for GPIOB in port %d", name, port);
+				return "";
+			}
 	}
 }
 
@@ -137,16 +146,18 @@ static ssize_t mts_attr_show_ap_din(struct kobject *kobj, struct kobj_attribute
 	u8 bit;
 	u8 byte;
 
-	sscanf(attr->attr.name, "din%d:%d", &channel, &port);
-	port_index = port - 1;
+	sscanf(attr->attr.name, "din%d", &channel);
 	if (channel < 0 || channel > 3) {
-		log_error("channel [%d] is invalid", channel);
+		log_error("channel %d is invalid", channel);
 		return -ENOENT;
 	}
-	if (port < 1 || port > NUM_AP) {
-		log_error("port [%d] is invalid", port);
-		return -ENOENT;
+
+	port = port_from_kobject(kobj);
+	if (port < 0) {
+		log_error("port_from_kobject returned %d", port);
+		return -EINVAL;
 	}
+	port_index = port - 1;
 
 	bit = BIT(channel);
 
@@ -169,33 +180,35 @@ static ssize_t mts_attr_store_ap_dout(struct kobject *kobj, struct kobj_attribut
 	int port_index;
 	u8 bit;
 
-	sscanf(attr->attr.name, "dout%d:%d", &channel, &port);
-	port_index = port - 1;
+	sscanf(attr->attr.name, "dout%d", &channel);
 	if (channel < 0 || channel > 3) {
-		log_error("channel [%d] is invalid", channel);
+		log_error("channel %d is invalid", channel);
 		return -ENOENT;
 	}
-	if (port < 1 || port > NUM_AP) {
-		log_error("port [%d] is invalid", port);
-		return -ENOENT;
+
+	port = port_from_kobject(kobj);
+	if (port < 0) {
+		log_error("port_from_kobject returned %d", port);
+		return -EINVAL;
 	}
+	port_index = port - 1;
 
 	bit = BIT(channel);
 
 	if (sscanf(buf, "%i", &value) != 1) {
-		log_error("accessory card dout attr invalid argument [%d]", value);
+		log_error("accessory card dout attr invalid argument %d", value);
 		return -EINVAL;
 	}
 
 	mutex_lock(&spi_ap_dout_mutex);
 
 	if (value) {
-		spi_ap_dout_value &= ~bit;
+		spi_ap_dout_value[port_index] &= ~bit;
 	} else {
-		spi_ap_dout_value |= bit;
+		spi_ap_dout_value[port_index] |= bit;
 	}
 
-	spi_writen(gpiob_spi[port_index][dout], &spi_ap_dout_value, 1);
+	spi_writen(gpiob_spi[port_index][dout], &spi_ap_dout_value[port_index], 1);
 
 	mutex_unlock(&spi_ap_dout_mutex);
 
@@ -207,23 +220,27 @@ static ssize_t mts_attr_show_ap_dout(struct kobject *kobj, struct kobj_attribute
 	int value;
 	int channel;
 	int port;
+	int port_index;
 	u8 bit;
 
-	sscanf(attr->attr.name, "dout%d:%d", &channel, &port);
+	sscanf(attr->attr.name, "dout%d", &channel);
 	if (channel < 0 || channel > 3) {
-		log_error("channel [%d] is invalid", channel);
+		log_error("channel %d is invalid", channel);
 		return -ENOENT;
 	}
-	if (port < 1 || port > NUM_AP) {
-		log_error("port [%d] is invalid", port);
-		return -ENOENT;
+
+	port = port_from_kobject(kobj);
+	if (port < 0) {
+		log_error("port_from_kobject returned %d", port);
+		return -EINVAL;
 	}
+	port_index = port - 1;
 
 	bit = BIT(channel);
 
 	mutex_lock(&spi_ap_dout_mutex);
 
-	value = spi_ap_dout_value & bit ? 0 : 1;
+	value = spi_ap_dout_value[port_index] & bit ? 0 : 1;
 
 	mutex_unlock(&spi_ap_dout_mutex);
 
@@ -246,16 +263,18 @@ static ssize_t mts_attr_show_ap_adc(struct kobject *kobj, struct kobj_attribute
 	memset(tx, 0, sizeof(tx));
 	memset(rx, 0, sizeof(rx));
 
-	sscanf(attr->attr.name, "adc%d:%d", &channel, &port);
-	port_index = port - 1;
+	sscanf(attr->attr.name, "adc%d", &channel);
 	if (channel < 0 || channel > 2) {
-		log_error("channel [%d] is invalid", channel);
+		log_error("channel %d is invalid", channel);
 		return -ENOENT;
 	}
-	if (port < 1 || port > NUM_AP) {
-		log_error("port [%d] is invalid", port);
-		return -ENOENT;
+
+	port = port_from_kobject(kobj);
+	if (port < 0) {
+		log_error("port_from_kobject returned %d", port);
+		return -EINVAL;
 	}
+	port_index = port - 1;
 
 	/* 1st transfer to set up (5V reference, channel to read from) */
 	tx_data = manual_mode | ((channel << 7) & channel_mask);
@@ -314,107 +333,142 @@ static bool gpiob_spi_driver_setup(struct spi_driver *driver, const char *driver
 // 1 product-id
 // 1 device-id
 // 1 hw-version
-static int ap_gpiob_attrs_size = 19;
+// NULL
+static int ap_gpiob_attrs_size = 20;
 
 static bool gpiob_setup(enum ap port) {
 	int i;
 	int port_index = port - 1;
-	struct kobj_attribute *attr;
+	int index = 0;
+	int count = 0;
+	int ret;
 	char buf[32];
+	struct kobj_attribute* attr;
+	struct attribute **attrs;
 
 	log_info("loading GPIOB accessory card in port %d", port);
 
-	if (device_attrs_size + ap_gpiob_attrs_size >= device_attrs_max_size) {
-		log_error("can't load GPIOB accessory card in port %d - not enough room for attributes", port);
+	sprintf(buf, "ap%d", port);
+	ap_subdirs[port_index] = kobject_create_and_add(buf, &mts_io_platform_device->dev.kobj);
+	if (! ap_subdirs[port_index]) {
+		log_error("kobject_create_and_add for port %d failed", port);
 		return false;
 	}
 
-	// mark the attribute indices we're using so we know what to clean up
-	port_info[port_index]->attrs_start = device_attrs_size;
-	port_info[port_index]->attrs_end = device_attrs_size + ap_gpiob_attrs_size;
+	// create the link to the apX directory this card is in
+	// if we're in the first slot, we get plain "gpiob"
+	// if we're in a different slot, we might need to use "gpiob-2" to differentiate
+	if (port > 1) {
+		for (i = 1; i < port; i++) {
+			if (port_info[i - 1]) {
+				if (strstr(port_info[i - 1]->product_id, PRODUCT_ID_MTAC_GPIOB)) {
+					count++;
+				}
+			}
+		}
+	}
+	if (count > 0) {
+		sprintf(buf, "gpiob-%d", count + 1);
+	} else {
+		sprintf(buf, "gpiob");
+	}
+	ret = sysfs_create_link(ap_subdirs[port_index]->parent, ap_subdirs[port_index], buf);
+	if (ret) {
+		log_error("failed to link [%s] to [%s], %d", buf, ap_subdirs[port_index]->name, ret);
+	}
+
+	attrs = kzalloc(sizeof(struct attribute*) * ap_gpiob_attrs_size, GFP_KERNEL);
+	if (! attrs) {
+		log_error("failed to allocate attribute space for port %d", port);
+		return false;
+	}
 
 	// add digital inputs
 	for (i = 0; i < 4; i++) {
-		sprintf(buf, "din%d:%d", i, port);
+		sprintf(buf, "din%d", i);
 		attr = create_attribute(buf, MTS_ATTR_MODE_RO);
 		if (! attr) {
-			log_error("failed to create attribute[%s]", buf);
+			log_error("failed to create attribute [%s]", buf);
 			return false;
 		}
 		attr->show = mts_attr_show_ap_din;
-		device_attrs[device_attrs_size++] = &attr->attr;
+		attrs[index++] = &attr->attr;
 	}
 
 	// add digital outputs
 	for (i = 0; i < 4; i++) {
-		sprintf(buf, "dout%d:%d", i, port);
+		sprintf(buf, "dout%d", i);
 		attr = create_attribute(buf, MTS_ATTR_MODE_RW);
 		if (! attr) {
-			log_error("failed to create attribute[%s]", buf);
+			log_error("failed to create attribute [%s] for GPIOB in port %d", buf, port);
 			return false;
 		}
 		attr->show = mts_attr_show_ap_dout;
 		attr->store = mts_attr_store_ap_dout;
-		device_attrs[device_attrs_size++] = &attr->attr;
+		attrs[index++] = &attr->attr;
 	}
 
 	// add analog to digital
 	for (i = 0; i < 3; i++) {
-		sprintf(buf, "adc%d:%d", i, port);
+		sprintf(buf, "adc%d", i);
 		attr = create_attribute(buf, MTS_ATTR_MODE_RO);
 		if (! attr) {
-			log_error("failed to create attribute[%s]", buf);
+			log_error("failed to create attribute [%s] for GPIOB in port %d", buf, port);
 			return false;
 		}
 		attr->show = mts_attr_show_ap_adc;
-		device_attrs[device_attrs_size++] = &attr->attr;
+		attrs[index++] = &attr->attr;
 	}
 
 	// add LEDs
 	for (i = 1; i <= 2; i++) {
-		sprintf(buf, "led%d:%d", i, port);
+		sprintf(buf, "led%d", i);
 		attr = create_attribute(buf, MTS_ATTR_MODE_RW);
 		if (! attr) {
-			log_error("failed to create attribute[%s]", buf);
+			log_error("failed to create attribute [%s] for GPIOB in port %d", buf, port);
 			return false;
 		}
 		attr->show = mts_attr_show_ap_gpio_pin;
 		attr->store = mts_attr_store_ap_gpio_pin;
-		device_attrs[device_attrs_size++] = &attr->attr;
-	}
-
-	// add attributes for eeprom contents
-	if (! ap_add_product_info_attributes(port, MTAC_GPIOB_0_0)) {
-		log_error("failed to add product info attributes");
-		return false;
+		attrs[index++] = &attr->attr;
 	}
 
 	// add misc attributes
-	sprintf(buf, "dout-enable:%d", port);
+	sprintf(buf, "dout-enable");
 	attr = create_attribute(buf, MTS_ATTR_MODE_RW);
 	if (! attr) {
-		log_error("failed to create attribute[%s]", buf);
+		log_error("failed to create attribute [%s] for GPIOB in port %d", buf, port);
 		return false;
 	}
 	attr->show = mts_attr_show_ap_gpio_pin;
 	attr->store = mts_attr_store_ap_gpio_pin;
-	device_attrs[device_attrs_size++] = &attr->attr;
+	attrs[index++] = &attr->attr;
 
-	sprintf(buf, "reset:%d", port);
+	sprintf(buf, "reset");
 	attr = create_attribute(buf, MTS_ATTR_MODE_RW);
 	if (! attr) {
-		log_error("failed to create attribute[%s]", buf);
+		log_error("failed to create attribute [%s] for GPIOB in port %d", buf, port);
 		return false;
 	}
 	attr->show = mts_attr_show_ap_gpio_pin;
 	attr->store = mts_attr_store_ap_gpio_pin;
-	device_attrs[device_attrs_size++] = &attr->attr;
+	attrs[index++] = &attr->attr;
+
+	// add attributes for eeprom contents
+	if (! ap_add_product_info_attributes(port, MTAC_GPIOB_0_0, attrs, &index)) {
+		log_error("failed to add product info attributes for GPIOB in port %d", port);
+		return false;
+	}
+
+	attrs[index] = NULL;
+
+	ap_attr_groups[port_index].attrs = attrs;
 
 	// setup and register drivers
 	log_debug("registering accessory card %d dout driver", port);
 	sprintf(buf, "mts-io-ap%d-dout", port);
 	if (! gpiob_spi_driver_setup(&gpiob_spi_drivers[port_index][dout], buf)) {
-		log_error("failed to set up spi driver [%s]", buf);
+		log_error("failed to set up spi driver [%s] for GPIOB in port %d", buf, port);
 		return false;
 	}
 	if (spi_register_driver(&gpiob_spi_drivers[port_index][dout])) {
@@ -426,7 +480,7 @@ static bool gpiob_setup(enum ap port) {
 	log_debug("registering accessory card %d din driver", port);
 	sprintf(buf, "mts-io-ap%d-din", port);
 	if (! gpiob_spi_driver_setup(&gpiob_spi_drivers[port_index][din], buf)) {
-		log_error("failed to set up spi driver [%s]", buf);
+		log_error("failed to set up spi driver [%s] for GPIOB in port %d", buf, port);
 		return false;
 	}
 	if (spi_register_driver(&gpiob_spi_drivers[port_index][din])) {
@@ -438,7 +492,7 @@ static bool gpiob_setup(enum ap port) {
 	log_debug("registering accessory card %d adc driver", port);
 	sprintf(buf, "mts-io-ap%d-adc", port);
 	if (! gpiob_spi_driver_setup(&gpiob_spi_drivers[port_index][adc], buf)) {
-		log_error("failed to set up spi driver [%s]", buf);
+		log_error("failed to set up spi driver [%s] for GPIOB in port %d", buf, port);
 		return false;
 	}
 	if (spi_register_driver(&gpiob_spi_drivers[port_index][adc])) {
@@ -446,25 +500,39 @@ static bool gpiob_setup(enum ap port) {
 		spi_unregister_driver(&gpiob_spi_drivers[port_index][adc]);
 		return false;
 	}
+
+	if (sysfs_create_group(ap_subdirs[port_index], &ap_attr_groups[port_index])) {
+		log_error("sysfs_create_group failed for GPIOB in port %d", port);
+		return false;
+	}
+
 	return true;
 }
 
 static bool gpiob_teardown(enum ap port) {
 	int i;
 	int port_index = port - 1;
+	struct attribute **attrs = ap_attr_groups[port_index].attrs;
 
 	log_info("unloading GPIOB accessory card in port %d", port);
 
 	// clean up allocated memory for attributes
-	for (i = port_info[port_index]->attrs_start; i < port_info[port_index]->attrs_end; i++) {
-		if (device_attrs[i]) {
-			if (device_attrs[i]->name)
-				kfree(device_attrs[i]->name);
+	for (i = 0; i < ap_gpiob_attrs_size; i++) {
+		if (attrs[i]) {
+			if (attrs[i]->name)
+				kfree(attrs[i]->name);
 
-			kfree(device_attrs[i]);
+			kfree(attrs[i]);
 		}
 	}
 
+	kfree(attrs);
+
+	// clean up our "apX/" kobject if it exists
+	if (ap_subdirs[port_index]) {
+		kobject_put(ap_subdirs[port_index]);
+	}
+
 	// clean up allocated memory for SPI drivers
 	if (gpiob_spi_drivers[port_index][dout].driver.name)
 		kfree(gpiob_spi_drivers[port_index][dout].driver.name);
@@ -478,19 +546,13 @@ static bool gpiob_teardown(enum ap port) {
 	spi_unregister_driver(&gpiob_spi_drivers[port_index][din]);
 	spi_unregister_driver(&gpiob_spi_drivers[port_index][adc]);
 
-	// reset attribute index markers
-	port_info[port_index]->attrs_start = 0;
-	port_info[port_index]->attrs_end = 0;
-
 	return true;
 }
 
 bool set_gpiob_info(struct ap_info* info) {
-	info->product_id = MTAC_GPIOB_0_0;
+	snprintf(info->product_id, 32, "%s", PRODUCT_ID_MTAC_GPIOB);
 	info->setup = &gpiob_setup;
 	info->teardown = &gpiob_teardown;
-	info->attrs_start = 0;
-	info->attrs_end = 0;
 	info->gpio_pin_name_by_attr_name = &gpiob_gpio_pin_name_by_attr_name;
 
 	return true;
diff --git a/io-module/mtac_mfser.c b/io-module/mtac_mfser.c
index 7c5862c..27ca4dc 100644
--- a/io-module/mtac_mfser.c
+++ b/io-module/mtac_mfser.c
@@ -1,15 +1,24 @@
-static char* mfser_gpio_pin_name_by_attr_name(const char* name) {
-	if (! strcmp(name, "rs4xx-term-res:1")) {
-		return "ap1-gpio3";
-	} else if (! strcmp(name, "rts-override:1")) {
-		return "ap1-gpio4";
-	} else if (! strcmp(name, "rs4xx-term-res:2")) {
-		return "ap2-gpio3";
-	} else if (! strcmp(name, "rts-override:2")) {
-		return "ap2-gpio4";
-	} else {
-		log_error("attirbute name [%s] is invalid for MFSER", name);
-		return "";
+static char* mfser_gpio_pin_name_by_attr_name(const char* name, int port) {
+	switch (port) {
+		case port_1:
+			if (! strcmp(name, "rs4xx-term-res")) {
+				return "ap1-gpio3";
+			} else if (! strcmp(name, "rts-override")) {
+				return "ap1-gpio4";
+			} else {
+				log_error("attirbute name [%s] is invalid for MFSER in port %d", name, port);
+				return "";
+			}
+
+		case port_2:
+			if (! strcmp(name, "rs4xx-term-res")) {
+				return "ap2-gpio3";
+			} else if (! strcmp(name, "rts-override")) {
+				return "ap2-gpio4";
+			} else {
+				log_error("attirbute name [%s] is invalid for MFSER in port %d", name, port);
+				return "";
+			}
 	}
 }
 
@@ -18,23 +27,33 @@ static ssize_t mts_attr_show_mfser_mode(struct kobject *kobj,
 			char *buf)
 {
 	int ret;
+	int port;
 	int modesel0;
 	int modesel1;
 
 	struct gpio_pin *pin_modesel0;
 	struct gpio_pin *pin_modesel1;
 
-	if (strstr(attr->attr.name, ":1")) {
-		pin_modesel0 = gpio_pin_by_name("AP1_GPIO1");
-		pin_modesel1 = gpio_pin_by_name("AP1_GPIO2");
-	}
-	else if (strstr(attr->attr.name, ":2")) {
-		pin_modesel0 = gpio_pin_by_name("AP2_GPIO1");
-		pin_modesel1 = gpio_pin_by_name("AP2_GPIO2");
+	port = port_from_kobject(kobj);
+	if (port < 0) {
+		log_error("port_from_kobject returned %d", port);
+		return -EINVAL;
 	}
-	else {
-		log_error("unknown serial-mode attr %s", attr->attr.name);
-		return -ENODEV;
+
+	switch (port) {
+		case port_1:
+			pin_modesel0 = gpio_pin_by_name("AP1_GPIO1");
+			pin_modesel1 = gpio_pin_by_name("AP1_GPIO2");
+			break;
+
+		case port_2:
+			pin_modesel0 = gpio_pin_by_name("AP2_GPIO1");
+			pin_modesel1 = gpio_pin_by_name("AP2_GPIO2");
+			break;
+
+		default:
+			log_error("unknown serial-mode attr [%s]", attr->attr.name);
+			return -ENODEV;
 	}
 
 	if (!pin_modesel0 || !pin_modesel1)
@@ -64,22 +83,32 @@ static ssize_t mts_attr_show_mfser_mode(struct kobject *kobj,
 static ssize_t mts_attr_store_mfser_mode(struct kobject *kobj,
 		struct kobj_attribute *attr, const char *buf, size_t count)
 {
+	int port;
 	int modesel0;
 	int modesel1;
 	struct gpio_pin *pin_modesel0;
 	struct gpio_pin *pin_modesel1;
 
-	if (strstr(attr->attr.name, ":1")) {
-		pin_modesel0 = gpio_pin_by_name("AP1_GPIO1");
-		pin_modesel1 = gpio_pin_by_name("AP1_GPIO2");
-	}
-	else if (strstr(attr->attr.name, ":2")) {
-		pin_modesel0 = gpio_pin_by_name("AP2_GPIO1");
-		pin_modesel1 = gpio_pin_by_name("AP2_GPIO2");
+	port = port_from_kobject(kobj);
+	if (port < 0) {
+		log_error("port_from_kobject returned %d", port);
+		return -EINVAL;
 	}
-	else {
-		log_error("unknown serial-mode attr %s", attr->attr.name);
-		return -ENODEV;
+
+	switch (port) {
+		case port_1:
+			pin_modesel0 = gpio_pin_by_name("AP1_GPIO1");
+			pin_modesel1 = gpio_pin_by_name("AP1_GPIO2");
+			break;
+
+		case port_2:
+			pin_modesel0 = gpio_pin_by_name("AP2_GPIO1");
+			pin_modesel1 = gpio_pin_by_name("AP2_GPIO2");
+			break;
+
+		default:
+			log_error("unknown serial-mode attr [%s]", attr->attr.name);
+			return -ENODEV;
 	}
 
 	if (!pin_modesel0 || !pin_modesel1)
@@ -122,57 +151,97 @@ static ssize_t mts_attr_store_mfser_mode(struct kobject *kobj,
 // 1 product-id
 // 1 device-id
 // 1 hw-version
-static int ap_mfser_attrs_size = 7;
+// NULL
+static size_t ap_mfser_attrs_size = 8;
 
 static bool mfser_setup(enum ap port) {
+	int i;
 	int port_index = port - 1;
-	struct kobj_attribute *attr;
+	int index = 0;
+	int count = 0;
+	int ret;
 	char buf[32];
+	struct kobj_attribute* attr;
+	struct attribute **attrs;
 
 	log_info("loading MFSER accessory card in port %d", port);
 
-	if (device_attrs_size + ap_mfser_attrs_size >= device_attrs_max_size) {
-		log_error("can't load MFSER accessory card in port %d - not enough room for attributes", port);
+	sprintf(buf, "ap%d", port);
+	ap_subdirs[port_index] = kobject_create_and_add(buf, &mts_io_platform_device->dev.kobj);
+	if (! ap_subdirs[port_index]) {
+		log_error("kobject_create_and_add for MFSER in port %d failed", port);
 		return false;
 	}
 
-	// mark the attribute indices we're using so we know what to clean up
-	port_info[port_index]->attrs_start = device_attrs_size;
-	port_info[port_index]->attrs_end = device_attrs_size + ap_mfser_attrs_size;
+	// create the link to the apX directory this card is in
+	// if we're in the first slot, we get plain "mfser"
+	// if we're in a different slot, we might need to use "mfser-2" to differentiate
+	if (port > 1) {
+		for (i = 1; i < port; i++) {
+			if (port_info[i - 1]) {
+				if (strstr(port_info[i - 1]->product_id, PRODUCT_ID_MTAC_MFSER)) {
+					count++;
+				}
+			}
+		}
+	}
+	if (count > 0) {
+		sprintf(buf, "mfser-%d", count + 1);
+	} else {
+		sprintf(buf, "mfser");
+	}
+	ret = sysfs_create_link(ap_subdirs[port_index]->parent, ap_subdirs[port_index], buf);
+	if (ret) {
+		log_error("failed to link [%s] to [%s], %d", buf, ap_subdirs[port_index]->name, ret);
+	}
+
+	attrs = kzalloc(sizeof(struct attribute*) * ap_mfser_attrs_size, GFP_KERNEL);
+	if (! attrs) {
+		log_error("failed to allocate attribute space for port %d", port);
+		return false;
+	}
 
-	sprintf(buf, "serial-mode:%d", port);
+	sprintf(buf, "serial-mode");
 	attr = create_attribute(buf, MTS_ATTR_MODE_RW);
 	if (! attr) {
-		log_error("failed to create attribute[%s]", buf);
+		log_error("failed to create attribute [%s] for MFSER in port %d", buf, port);
 		return false;
 	}
 	attr->show = mts_attr_show_mfser_mode;
 	attr->store = mts_attr_store_mfser_mode;
-	device_attrs[device_attrs_size++] = &attr->attr;
+	attrs[index++] = &attr->attr;
 
-	sprintf(buf, "rs4xx-term-res:%d", port);
+	sprintf(buf, "rs4xx-term-res");
 	attr = create_attribute(buf, MTS_ATTR_MODE_RW);
 	if (! attr) {
-		log_error("failed to create attribute[%s]", buf);
+		log_error("failed to create attribute [%s] for MFSER in port %d", buf, port);
 		return false;
 	}
 	attr->show = mts_attr_show_ap_gpio_pin;
 	attr->store = mts_attr_store_ap_gpio_pin;
-	device_attrs[device_attrs_size++] = &attr->attr;
+	attrs[index++] = &attr->attr;
 
-	sprintf(buf, "rts-override:%d", port);
+	sprintf(buf, "rts-override");
 	attr = create_attribute(buf, MTS_ATTR_MODE_RW);
 	if (! attr) {
-		log_error("failed to create attribute[%s]", buf);
+		log_error("failed to create attribute [%s] for MFSER in port %d", buf, port);
 		return false;
 	}
 	attr->show = mts_attr_show_ap_gpio_pin;
 	attr->store = mts_attr_store_ap_gpio_pin;
-	device_attrs[device_attrs_size++] = &attr->attr;
+	attrs[index++] = &attr->attr;
 
 	// add attributes for eeprom contents
-	if (! ap_add_product_info_attributes(port, MTAC_MFSER_0_0)) {
-		log_error("failed to add product info attributes");
+	if (! ap_add_product_info_attributes(port, MTAC_MFSER_0_0, attrs, &index)) {
+		log_error("failed to add product info attributes for MFSER in port %d", port);
+		return false;
+	}
+
+	attrs[index] = NULL;
+
+	ap_attr_groups[port_index].attrs = attrs;
+	if (sysfs_create_group(ap_subdirs[port_index], &ap_attr_groups[port_index])) {
+		log_error("sysfs_create_group failed for MFSER in port %d", port);
 		return false;
 	}
 
@@ -182,31 +251,34 @@ static bool mfser_setup(enum ap port) {
 static bool mfser_teardown(enum ap port) {
 	int i;
 	int port_index = port - 1;
+	struct attribute **attrs = ap_attr_groups[port_index].attrs;
 
 	log_info("unloading MFSER accessory card in port %d", port);
 
 	// clean up allocated memory for attributes
-	for (i = port_info[port_index]->attrs_start; i < port_info[port_index]->attrs_end; i++) {
-		if (device_attrs[i]) {
-			if (device_attrs[i]->name)
-				kfree(device_attrs[i]->name);
+	for (i = 0; i < ap_mfser_attrs_size; i++) {
+		if (attrs[i]) {
+			if (attrs[i]->name)
+				kfree(attrs[i]->name);
 
-			kfree(device_attrs[i]);
+			kfree(attrs[i]);
 		}
 	}
 
-	port_info[port_index]->attrs_start = 0;
-	port_info[port_index]->attrs_end = 0;
+	kfree(attrs);
+
+	// clean up our "apX/" kobject if it exists
+	if (ap_subdirs[port_index]) {
+		kobject_put(ap_subdirs[port_index]);
+	}
 
 	return true;
 }
 
 bool set_mfser_info(struct ap_info* info) {
-	info->product_id = MTAC_MFSER_0_0;
+	snprintf(info->product_id, 32, "%s", PRODUCT_ID_MTAC_MFSER);
 	info->setup = &mfser_setup;
 	info->teardown = &mfser_teardown;
-	info->attrs_start = 0;
-	info->attrs_end = 0;
 	info->gpio_pin_name_by_attr_name = &mfser_gpio_pin_name_by_attr_name;
 
 	return true;
diff --git a/io-module/mts_io.c b/io-module/mts_io.c
index 15a70a3..048f96d 100644
--- a/io-module/mts_io.c
+++ b/io-module/mts_io.c
@@ -78,14 +78,14 @@ static struct mts_id_eeprom_layout id_eeprom;
 /* accessory card EEPROMs */
 extern uint8_t mts_ap_eeprom[NUM_AP][512];
 static struct mts_ap_eeprom_layout ap_eeprom[NUM_AP];
+/* kobject pointers for the apX subdirectories that correspond to the accessory ports */
+static struct kobject *ap_subdirs[NUM_AP];
+/* attribute groups for the accessory ports*/
+static struct attribute_group ap_attr_groups[NUM_AP];
 #endif
 
 static struct ap_info* port_info[NUM_AP];
 
-static struct attribute **device_attrs;
-static size_t device_attrs_size;
-static size_t device_attrs_max_size;
-
 static uint8_t mts_product_id;
 static uint8_t has_spi_sout;
 static uint8_t has_spi_din;
@@ -546,12 +546,6 @@ static int mts_id_eeprom_load(void)
 		has_spi_temp = 0;
 		log_info("detected board %s", HW_VERSION_MTR_0_1);
 	} else if (strncmp(id_eeprom.hw_version, HW_VERSION_MTR2D2_0_0, strlen(HW_VERSION_MTR2D2_0_0)) == 0) {
-		// need to put any accessory card attributes into this list so they show up in sysfs
-		// the port_info->setup callback does this
-		device_attrs = mtr2d2_platform_attributes;
-		device_attrs_size = mtr2d2_platform_attributes_size;
-		device_attrs_max_size = mtr2d2_platform_attributes_max_size;
-
 		attr_group = &mtr2d2_platform_attribute_group;
 		gpio_pins = gpio_pins_mtr2d2_0_0;
 		mts_product_id = MTR2D2_0_0;
@@ -628,13 +622,6 @@ static int __init mts_io_init(void)
 		goto error1;
 	}
 
-	if (NUM_AP) {
-		for (port = 0; port < NUM_AP; port++) {
-			port_info[port] = NULL;
-		}
-		init_accessory_ports();
-	}
-
 	mts_io_platform_device = platform_device_alloc(PLATFORM_NAME, -1);
 	if (!mts_io_platform_device) {
 		ret = -ENOMEM;
@@ -655,6 +642,13 @@ static int __init mts_io_init(void)
 		goto error3;
 	}
 
+	if (NUM_AP) {
+		for (port = 0; port < NUM_AP; port++) {
+			port_info[port] = NULL;
+		}
+		init_accessory_ports();
+	}
+
 	ret = sysfs_create_group(&mts_io_platform_device->dev.kobj, attr_group);
 	if (ret) {
 		goto error4;
diff --git a/io-module/mts_io.h b/io-module/mts_io.h
index cc88392..3645e17 100644
--- a/io-module/mts_io.h
+++ b/io-module/mts_io.h
@@ -107,12 +107,10 @@ enum spi_devices {
 // contains function pointers for setup and teardown and useful info
 // each type of accessory card should have one of these
 struct ap_info {
-	uint8_t product_id;
+	char  product_id[32];
 	bool (*setup)(enum ap port);
 	bool (*teardown)(enum ap port);
-	int attrs_start;
-	int attrs_end;
-	char* (*gpio_pin_name_by_attr_name)(const char* name);
+	char* (*gpio_pin_name_by_attr_name)(const char* name, int port);
 };
 
 #endif /* ~__MTS_IO_H */