[v2,1/1] adv_mix955x is a scheme that multiplexes PCA9554/PCA9555 into LED and GPIO

[v2,1/1] adv_mix955x is a scheme that multiplexes PCA9554/PCA9555 into LED and GPIO

[yuechao.zhao(赵越超)]

From: Yuechao Zhao <yuechao.zhao@advantech.com.cn>

With this driver, we can multiplex PCA9554/PCA9555 into LED and GPIO based on the ACPI data of BIOS.

Signed-off-by: Yuechao Zhao <yuechao.zhao@advantech.com.cn>
Signed-off-by: Sui Jia <jia.sui@advantech.com.cn>

v2: 
	- alphabetical order title ("ADVANTECH MIX995X DRIVER")
---
 MAINTAINERS                        |   6 +
 drivers/platform/x86/Kconfig       |  14 +
 drivers/platform/x86/Makefile      |   3 +
 drivers/platform/x86/adv_mix955x.c | 703 +++++++++++++++++++++++++++++
 4 files changed, 726 insertions(+)
 create mode 100644 drivers/platform/x86/adv_mix955x.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d92f85ca831d..90033699334c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -573,6 +573,12 @@ S:	Maintained
 F:	Documentation/scsi/advansys.rst
 F:	drivers/scsi/advansys.c
 
+ADVANTECH MIX995X DRIVER
+M:	Yuechao Zhao <yuechao.zhao@advantech.com.cn>
+M:	Sui Jia <jia.sui@advantech.com.cn>
+S:	Maintained
+F:	drivers/platform/x86/adv_mix955x.c
+
 ADXL34X THREE-AXIS DIGITAL ACCELEROMETER DRIVER (ADXL345/ADXL346)
 M:	Michael Hennerich <michael.hennerich@analog.com>
 S:	Supported
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index ad4e630e73e2..ccead75c764b 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1284,6 +1284,20 @@ config INTEL_TELEMETRY
 	  directly via debugfs files. Various tools may use
 	  this interface for SoC state monitoring.
 
+config ADV_MIX955X
+    tristate "Advantech LEDs and GPIOs Driver"
+    depends on X86_64
+    depends on ACPI && GPIOLIB
+    select LEDS_CLASS
+    select LEDS_TRIGGERS
+    help
+      This driver is based on the kernel driver leds-pca955x
+      and gpio-pca953x but rewrite for PCA9554/PCA9555.
+      This driver requires LED data which is defined inside
+      ACPI DSDT or devicetree.
+      In addition, the pin that is not defined as LED will
+      be declared as GPIO.
+
 endif # X86_PLATFORM_DEVICES
 
 config PMC_ATOM
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 60d554073749..5ecad0ae925e 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -138,3 +138,6 @@ obj-$(CONFIG_INTEL_TELEMETRY)		+= intel_telemetry_core.o \
 					   intel_telemetry_pltdrv.o \
 					   intel_telemetry_debugfs.o
 obj-$(CONFIG_PMC_ATOM)			+= pmc_atom.o
+
+# Advantech
+obj-$(CONFIG_ADV_MIX955X)		+= adv_mix955x.o
diff --git a/drivers/platform/x86/adv_mix955x.c b/drivers/platform/x86/adv_mix955x.c
new file mode 100644
index 000000000000..99fc1427d4d9
--- /dev/null
+++ b/drivers/platform/x86/adv_mix955x.c
@@ -0,0 +1,703 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright 2019 Advantech Corp.
+ *
+ * Author:
+ *	Jia Sui <jia.sui@advantech.com.cn>
+ *	Yuechao Zhao <yuechao.zhao@advantech.com.cn>
+ *
+ * A driver for AMIX955<X>(Advantech mixed device based on PCA955X).
+ * This driver provides LED and GPIO functions.
+ *
+ * Supported devices:
+ *
+ *	Device		ACPI_DEVICE_ID	Description	7-bit slave address
+ *	--------	--------------	---------------	-------------------
+ *	AMIX9554	AHC0320		Base on PCA9554	0x20 .. 0x27
+ *	AMIX9555	AHC0321		Base on PCA9555	0x20 .. 0x27
+ */
+
+#include <linux/i2c.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/version.h>
+#include <linux/fwnode.h>
+#include <linux/acpi.h>
+#include <linux/gpio/driver.h>
+#include <linux/gpio/consumer.h>
+#include <asm/unaligned.h>
+
+#define VERSION "0.05"
+
+enum AMIX955X_LED {
+	AMIX955X_LED_ON = 0,
+	AMIX955X_LED_OFF = 1,
+};
+
+#define BANK_SIZE 8
+/*
+ * Only support pca9554 and pca9555,
+ * The max pin numbers is 16
+ * Defind MAX_BANK = 2
+ */
+#define MAX_BANK 2
+
+/* PCA9554 GPIO register */
+#define AMIX955X_INPUT 0x00
+#define AMIX955X_OUTPUT 0x01
+#define AMIX955X_INVERT 0x02
+#define AMIX955X_DIRECTION 0x03
+
+/* Default Value of GPIO register */
+#define AMIX955X_OUTPUT_DEFAULT 0xFF
+#define AMIX955X_DIRECTION_DEFAULT 0xFFFF
+
+enum chip_id {
+	amix9554,
+	amix9555,
+};
+
+struct chip_info_t {
+	u8 bits;
+	u8 slv_addr; /* 7-bit slave address mask */
+	int slv_addr_shift; /* Number of bits to ignore */ };
+
+static struct chip_info_t chip_info_tables[] = {
+	[amix9554] = {
+		.bits = 8,
+		.slv_addr = /* 0100xxx */ 0x20,
+		.slv_addr_shift = 3,
+	},
+	[amix9555] = {
+		.bits = 16,
+		.slv_addr = /* 0100xxx */ 0x20,
+		.slv_addr_shift = 3,
+	},
+};
+
+struct amix955x_reg_config {
+	int direction;
+	int output;
+	int input;
+	int invert;
+};
+
+static const struct amix955x_reg_config amix955x_regs = {
+	.direction = AMIX955X_DIRECTION,
+	.output = AMIX955X_OUTPUT,
+	.input = AMIX955X_INPUT,
+	.invert = AMIX955X_INVERT,
+};
+
+struct driver_data {
+	struct mutex lock; /* protect read/write PCA955X */
+	struct i2c_client *client;
+	struct chip_info_t *chip_info;
+	struct amix955x_led *leds;
+	struct amix955x_gpio *gpio;
+	int (*write_regs)(struct i2c_client *i2c, int reg, u8 *val);
+	int (*read_regs)(struct i2c_client *i2c, int reg, u8 *val); };
+
+struct amix955x_led {
+	struct driver_data *data;
+	struct led_classdev led_cdev;
+	int led_id;
+	char name[32];
+	const char *default_trigger;
+};
+
+struct amix955x_gpio {
+	u8 reg_output[MAX_BANK];
+	u8 reg_direction[MAX_BANK];
+	int gpio_id[MAX_BANK * BANK_SIZE];//the id list of gpio pin
+
+	struct gpio_chip gpio_chip;
+	const struct amix955x_reg_config *regs; };
+
+struct amix955x_platform_data {
+	struct driver_data *data;
+	struct amix955x_led *leds;
+	struct amix955x_gpio *gpio;
+	int num_leds;
+	int gpio_start;
+	int num_gpio;
+};
+
+static const struct acpi_device_id acpi_amix955x_match[] = {
+	{ "AHC0320", amix9554 },
+	{ "AHC0321", amix9555 },
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, acpi_amix955x_match);
+
+static int get_device_index(struct i2c_client *client,
+			    const struct i2c_device_id *id)
+{
+	const struct acpi_device_id *acpi_id;
+
+	if (id)
+		return id->driver_data;
+
+	acpi_id = acpi_match_device(acpi_amix955x_match, &client->dev);
+	if (!acpi_id)
+		return -ENODEV;
+
+	return acpi_id->driver_data;
+}
+
+static inline u8 amix955x_set_bit(u8 val, int led_id, enum AMIX955X_LED 
+state) {
+	led_id %= BANK_SIZE;
+
+	switch (state) {
+	case AMIX955X_LED_ON:
+		val &= ~(1 << led_id); //Clear bit
+		break;
+	case AMIX955X_LED_OFF:
+		val |= 1 << led_id; //Set bit
+		break;
+	}
+
+	return val;
+}
+
+static int amix955x_read_regs_8(struct i2c_client *client, int reg, u8 
+*val) {
+	int ret = i2c_smbus_read_byte_data(client, reg);
+	*val = ret;
+	return ret;
+}
+
+static int amix955x_read_regs_16(struct i2c_client *client, int reg, u8 
+*val) {
+	int ret = i2c_smbus_read_word_data(client, reg << 1);
+
+	val[0] = (u16)ret & 0xFF;
+	val[1] = (u16)ret >> 8;
+
+	return ret;
+}
+
+static int amix955x_read_regs(struct driver_data *data, int reg, u8 
+*val) {
+	int ret;
+
+	ret = data->read_regs(data->client, reg, val);
+	if (ret < 0) {
+		dev_err(&data->client->dev, "failed reading register\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int amix955x_read_single(struct driver_data *data, int reg, u32 *val,
+				int off)
+{
+	int ret;
+	int bank_shift = fls((data->chip_info->bits - 1) / BANK_SIZE);
+	int offset = off / BANK_SIZE;
+
+	ret = i2c_smbus_read_byte_data(data->client,
+				       (reg << bank_shift) + offset);
+
+	*val = ret;
+
+	if (ret < 0)
+		dev_err(&data->client->dev, "failed reading register\n");
+
+	return ret;
+}
+
+static int amix955x_write_regs_8(struct i2c_client *client, int reg, u8 
+*val) {
+	return i2c_smbus_write_byte_data(client, reg, *val); }
+
+static int amix955x_write_regs_16(struct i2c_client *client, int reg, 
+u8 *val) {
+	__le16 word = cpu_to_le16(get_unaligned((u16 *)val));
+
+	return i2c_smbus_write_word_data(client, reg << 1, (__force u16)word); 
+}
+
+static int amix955x_write_regs(struct driver_data *data, int reg, u8 
+*val) {
+	int ret;
+
+	ret = data->write_regs(data->client, reg, val);
+	if (ret < 0) {
+		dev_err(&data->client->dev, "failed writing register\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int amix955x_write_single(struct driver_data *data, int reg, u32 val,
+				 int off)
+{
+	int ret;
+	int bank_shift = fls((data->chip_info->bits - 1) / BANK_SIZE);
+	int offset = off / BANK_SIZE;
+
+	ret = i2c_smbus_write_byte_data(data->client,
+					(reg << bank_shift) + offset, val);
+
+	if (ret < 0)
+		dev_err(&data->client->dev, "failed writing register\n");
+
+	return ret;
+}
+
+static int amix955x_led_set(struct led_classdev *led_cdev,
+			    enum led_brightness value)
+{
+	struct amix955x_led *led;
+	struct driver_data *data;
+	int led_id;
+	int ret = 0;
+	u32 reg_val = 0;
+
+	led = container_of(led_cdev, struct amix955x_led, led_cdev);
+	data = led->data;
+	led_id = led->led_id;
+
+	mutex_lock(&data->lock);
+
+	//get current value
+	ret = amix955x_read_single(data, AMIX955X_OUTPUT, &reg_val, led_id);
+	if (ret < 0)
+		goto set_failed;
+
+	//change bit
+	switch (value) {
+	case LED_OFF:
+		reg_val = amix955x_set_bit(reg_val, led_id, AMIX955X_LED_OFF);
+		break;
+	default:
+		reg_val = amix955x_set_bit(reg_val, led_id, AMIX955X_LED_ON);
+		break;
+	}
+
+	//set new value
+	ret = amix955x_write_single(data, AMIX955X_OUTPUT, reg_val, led_id);
+
+set_failed:
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
+static int amix955x_gpio_direction_input(struct gpio_chip *gc, unsigned 
+int off) {
+	struct driver_data *data = gpiochip_get_data(gc);
+	u8 reg_val;
+	unsigned int index;
+	int ret;
+
+	index = data->gpio->gpio_id[off];
+
+	mutex_lock(&data->lock);
+	reg_val = data->gpio->reg_direction[index / BANK_SIZE]
+					| (1u << (index % BANK_SIZE));
+
+	ret = amix955x_write_single(data, data->gpio->regs->direction,
+				    reg_val, index);
+	if (ret)
+		goto exit;
+
+	data->gpio->reg_direction[index / BANK_SIZE] = reg_val;
+exit:
+	mutex_unlock(&data->lock);
+	return ret;
+}
+
+static int amix955x_gpio_direction_output(struct gpio_chip *gc,
+					  unsigned int off, int val)
+{
+	struct driver_data *data = gpiochip_get_data(gc);
+	u8 reg_val;
+	unsigned int index;
+	int ret;
+
+	index = data->gpio->gpio_id[off];
+
+	mutex_lock(&data->lock);
+	if (val) {
+		reg_val = data->gpio->reg_output[index / BANK_SIZE]
+			| (1u << (index & BANK_SIZE));
+	} else {
+		reg_val = data->gpio->reg_output[index / BANK_SIZE]
+			& ~(1u << (index % BANK_SIZE));
+	}
+
+	ret = amix955x_write_single(data, data->gpio->regs->output,
+				    reg_val, index);
+	if (ret)
+		goto exit;
+
+	data->gpio->reg_output[index / BANK_SIZE] = reg_val;
+
+	reg_val = data->gpio->reg_direction[index / BANK_SIZE]
+					& ~(1u << (index % BANK_SIZE));
+
+	ret = amix955x_write_single(data, data->gpio->regs->direction,
+				    reg_val, index);
+	if (ret)
+		goto exit;
+
+	data->gpio->reg_direction[index / BANK_SIZE] = reg_val;
+exit:
+	mutex_unlock(&data->lock);
+	return ret;
+}
+
+static int amix955x_gpio_get_value(struct gpio_chip *gc, unsigned int 
+off) {
+	struct driver_data *data = gpiochip_get_data(gc);
+	u32 reg_val;
+	int ret;
+	unsigned int index;
+
+	index = data->gpio->gpio_id[off];
+
+	mutex_lock(&data->lock);
+	ret = amix955x_read_single(data, data->gpio->regs->input,
+				   &reg_val, index);
+	mutex_unlock(&data->lock);
+
+	if (ret < 0)
+		return 0;
+
+	return (reg_val & (1u << (index % BANK_SIZE))) ? 1 : 0; }
+
+static void amix955x_gpio_set_value(struct gpio_chip *gc, unsigned int off,
+				    int val)
+{
+	struct driver_data *data = gpiochip_get_data(gc);
+	u8 reg_val;
+	int ret;
+	unsigned int index;
+
+	index = data->gpio->gpio_id[off];
+
+	mutex_lock(&data->lock);
+	if (val)
+		reg_val = data->gpio->reg_output[index / BANK_SIZE]
+						| (1u << (index % BANK_SIZE));
+	else
+		reg_val = data->gpio->reg_output[index / BANK_SIZE]
+						& ~(1u << (index % BANK_SIZE));
+
+	ret = amix955x_write_single(data, data->gpio->regs->output,
+				    reg_val, index);
+	if (ret)
+		goto exit;
+
+	data->gpio->reg_output[index / BANK_SIZE] = reg_val;
+
+exit:
+	mutex_unlock(&data->lock);
+}
+
+static int amix955x_gpio_get_direction(struct gpio_chip *gc, unsigned 
+int off) {
+	struct driver_data *data = gpiochip_get_data(gc);
+	u32 reg_val;
+	int ret;
+	unsigned int index;
+
+	index = data->gpio->gpio_id[off];
+
+	mutex_lock(&data->lock);
+	ret = amix955x_read_single(data, data->gpio->regs->direction,
+				   &reg_val, index);
+	mutex_unlock(&data->lock);
+	if (ret < 0)
+		return ret;
+
+	return !!(reg_val & (1u << (index % BANK_SIZE))); }
+
+static void amix955x_setup_gpio(struct amix955x_platform_data *pdata,
+				struct driver_data *data)
+{
+	struct gpio_chip *gc;
+
+	gc = &pdata->gpio->gpio_chip;
+
+	gc->direction_input = amix955x_gpio_direction_input;
+	gc->direction_output = amix955x_gpio_direction_output;
+	gc->get = amix955x_gpio_get_value;
+	gc->set = amix955x_gpio_set_value;
+	gc->get_direction = amix955x_gpio_get_direction;
+	//gc->set_config
+	//gc->set_multiple
+	gc->can_sleep = true;
+
+	gc->base = pdata->gpio_start;
+	gc->ngpio = pdata->num_gpio;
+
+	gc->label = dev_name(&data->client->dev);
+	gc->parent = &data->client->dev;
+
+	gc->owner = THIS_MODULE;
+}
+
+static struct amix955x_platform_data *
+amix955x_get_pdata(struct i2c_client *client, struct chip_info_t *chip) 
+{
+	struct amix955x_platform_data *pdata;
+	struct fwnode_handle *child;
+	u32 bitmask = 0;
+	int pin_id = 0;
+	int count = 0;
+	int index = 0;
+
+	count = device_get_child_node_count(&client->dev);
+	if (count > chip->bits)
+		return ERR_PTR(-ENODEV);
+
+	pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	pdata->num_leds = 0;
+	//BIOS declares that the LED information is in the ACPI table
+	if (count > 0) {
+		pdata->leds = devm_kcalloc(&client->dev, count,
+					   sizeof(struct amix955x_led),
+					   GFP_KERNEL);
+		if (!pdata->leds)
+			return ERR_PTR(-ENOMEM);
+
+		device_for_each_child_node(&client->dev, child) {
+			const char *name;
+			u32 reg;
+			int res;
+
+			res = fwnode_property_read_u32(child, "reg", &reg);
+			if (res != 0 || reg >= chip->bits)
+				continue;
+
+			pdata->leds[index].led_id = reg;
+			bitmask |= 1 << reg;
+
+			res = fwnode_property_read_string(child, "label",
+							  &name);
+			if (res != 0 && is_of_node(child))
+				name = to_of_node(child)->name;
+
+			snprintf(pdata->leds[index].name,
+				 sizeof(pdata->leds[index].name), "%s", name);
+
+			index++;
+		}
+
+		pdata->num_leds = count;
+	}
+
+	//No pin needs to be initialized to gpio
+	if (count == chip->bits)
+		return pdata;
+
+	pdata->gpio = devm_kzalloc(&client->dev, sizeof(struct amix955x_gpio),
+				   GFP_KERNEL);
+	if (!pdata->gpio)
+		return ERR_PTR(-ENOMEM);
+
+	for (pin_id = 0, index = 0; pin_id < chip->bits; pin_id++) {
+		//this pin is used be led
+		if ((bitmask >> pin_id) & 0x01)
+			continue;
+
+		pdata->gpio->gpio_id[index] = pin_id;
+		index++;
+	}
+
+	pdata->num_gpio = index;
+
+	/*
+	 * Please pass -1 as base to let gpiolib select the chip base
+	 * in all possible cases
+	 */
+	pdata->gpio_start = -1;
+
+	return pdata;
+}
+
+static void
+amix955x_set_direction_as_output(struct amix955x_platform_data *pdata) 
+{
+	u8 i;
+	u16 val = AMIX955X_DIRECTION_DEFAULT;
+	struct driver_data *data = pdata->data;
+	struct i2c_client *client = data->client;
+
+	for (i = 0; i < pdata->num_leds; i++)
+		val &= ~(1 << pdata->leds[i].led_id);
+
+	data->write_regs(client, AMIX955X_DIRECTION, (u8 *)&val); }
+
+static void amix955x_init_output_reg(struct driver_data *data) {
+	u8 val[MAX_BANK] = {AMIX955X_OUTPUT_DEFAULT};
+	struct i2c_client *client = data->client;
+
+	data->write_regs(client, AMIX955X_OUTPUT, val); }
+
+static int device_amix955x_init(struct amix955x_platform_data *pdata) {
+	int ret;
+	u8 val[MAX_BANK] = {0};
+
+	pdata->gpio->regs = &amix955x_regs;
+
+	ret = amix955x_read_regs(pdata->data, pdata->gpio->regs->output,
+				 pdata->gpio->reg_output);
+	if (ret)
+		goto out;
+
+	ret = amix955x_read_regs(pdata->data, pdata->gpio->regs->direction,
+				 pdata->gpio->reg_direction);
+	if (ret)
+		goto out;
+
+	//not set invert
+	ret = amix955x_write_regs(pdata->data, AMIX955X_INVERT, val);
+out:
+	return ret;
+}
+
+static int amix955x_probe(struct i2c_client *client,
+			  const struct i2c_device_id *id)
+{
+	struct driver_data *data;
+	struct chip_info_t *chip;
+	int ret;
+	int i, err;
+	struct amix955x_platform_data *pdata;
+
+	int device_id = 0;
+
+	device_id = get_device_index(client, id);
+	if (device_id < 0 || device_id >= ARRAY_SIZE(chip_info_tables))
+		return -ENODEV;
+
+	chip = &chip_info_tables[device_id];
+
+	pdata = amix955x_get_pdata(client, chip);
+	if (IS_ERR(pdata))
+		return PTR_ERR(pdata);
+
+	/* Make sure the slave address given is possible */
+	if ((client->addr & ~((1 << chip->slv_addr_shift) - 1)) !=
+	    chip->slv_addr) {
+		dev_err(&client->dev, "invalid slave address %02x\n",
+			client->addr);
+		return -ENODEV;
+	}
+
+	dev_info(&client->dev,
+		 "%s: Using %s %d-bit LED driver at slave address 0x%02x\n",
+		 client->dev.driver->name, client->name, chip->bits,
+		 client->addr);
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+		return -EIO;
+
+	//Initial driver data
+	data = devm_kzalloc(&client->dev, sizeof(struct driver_data),
+			    GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	mutex_init(&data->lock);
+
+	data->leds = pdata->leds;
+	data->gpio = pdata->gpio;
+	data->client = client;
+	data->chip_info = chip;
+
+	switch (chip->bits) {
+	case 8:
+		data->read_regs = amix955x_read_regs_8;
+		data->write_regs = amix955x_write_regs_8;
+		break;
+	case 16:
+		data->read_regs = amix955x_read_regs_16;
+		data->write_regs = amix955x_write_regs_16;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	pdata->data = data;
+
+	i2c_set_clientdata(client, data);
+
+	/* Set GPIO direction as output */
+	amix955x_set_direction_as_output(pdata);
+
+	/* Turn off all LEDs */
+	amix955x_init_output_reg(data);
+
+	for (i = 0; i < pdata->num_leds; i++) {
+		struct amix955x_led *led;
+		struct led_classdev *led_cdev;
+
+		led = &data->leds[i];
+		led_cdev = &led->led_cdev;
+
+		led->data = data;
+
+		led_cdev->name = led->name;
+		led_cdev->brightness_set_blocking = amix955x_led_set;
+
+		if (pdata->leds[i].default_trigger) {
+			led_cdev->default_trigger =
+				pdata->leds[i].default_trigger;
+		}
+
+		err = devm_led_classdev_register(&client->dev, led_cdev);
+		if (err)
+			return err;
+
+		amix955x_led_set(led_cdev, LED_OFF);
+	}
+
+	amix955x_setup_gpio(pdata, data);
+
+	ret = device_amix955x_init(pdata);
+	if (ret < 0)
+		return ret;
+
+	ret = devm_gpiochip_add_data(&client->dev, &data->gpio->gpio_chip,
+				     data);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static struct i2c_driver amix955x_driver = {
+	.driver = {
+		.name = "amix955x",
+		.acpi_match_table = ACPI_PTR(acpi_amix955x_match),
+	},
+	.probe = amix955x_probe,
+};
+
+module_i2c_driver(amix955x_driver);
+
+MODULE_AUTHOR("Jia Sui <jia.sui@advantech.com.cn>"); 
+MODULE_DESCRIPTION("AMIX955X LED and GPIO driver"); MODULE_LICENSE("GPL 
+v2"); MODULE_VERSION(VERSION);
--
2.27.0

Re: [v2,1/1] adv_mix955x is a scheme that multiplexes PCA9554/PCA9555 into LED and GPIO

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 911 bytes --]

On Tue 2021-04-27 08:32:05, yuechao.zhao(赵越超) wrote:
> From: Yuechao Zhao <yuechao.zhao@advantech.com.cn>
> 
> With this driver, we can multiplex PCA9554/PCA9555 into LED and GPIO based on the ACPI data of BIOS.
> 
> Signed-off-by: Yuechao Zhao <yuechao.zhao@advantech.com.cn>
> Signed-off-by: Sui Jia <jia.sui@advantech.com.cn>

Please Cc LED drivers to the LED maintainers...

> +
> +static inline u8 amix955x_set_bit(u8 val, int led_id, enum AMIX955X_LED 
> +state) {
> +	led_id %= BANK_SIZE;
> +
> +	switch (state) {
> +	case AMIX955X_LED_ON:
> +		val &= ~(1 << led_id); //Clear bit
> +		break;
> +	case AMIX955X_LED_OFF:
> +		val |= 1 << led_id; //Set bit
> +		break;
> +	}
> +
> +	return val;

Please check CodingStyle, avoid useless comments and check word
wrapping in your email settings.

Best regards,
								Pavel

-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

RE: [v2,1/1] adv_mix955x is a scheme that multiplexes PCA9554/PCA9555 into LED and GPIO

[yuechao.zhao(赵越超)]

Hi Pavel
As for " Please Cc LED drivers to the LED maintainers...", does it mean that I only need to Cc LED drivers to LED maintainers?
Or do I need to send this driver to linux-leds@vger.kernel.org instead of platform-driver-x86@vger.kernel.org.

Best regards
Yuechao Zhao

-----Original Message-----
From: Pavel Machek [mailto:pavel@ucw.cz] 
Sent: Tuesday, April 27, 2021 9:54 PM
To: yuechao.zhao(赵越超) <yuechao.zhao@advantech.com.cn>
Cc: 345351830@qq.com; Rainbow.Zhang(張玉) <Rainbow.Zhang@advantech.com.cn>; yunxia.li(李云霞) <yunxia.li@advantech.com.cn>; Jia.Sui(贾睢) <Jia.Sui@advantech.com.cn>; Hans de Goede <hdegoede@redhat.com>; Mark Gross <mgross@linux.intel.com>; linux-kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org
Subject: Re: [v2,1/1] adv_mix955x is a scheme that multiplexes PCA9554/PCA9555 into LED and GPIO

On Tue 2021-04-27 08:32:05, yuechao.zhao(赵越超) wrote:
> From: Yuechao Zhao <yuechao.zhao@advantech.com.cn>
> 
> With this driver, we can multiplex PCA9554/PCA9555 into LED and GPIO based on the ACPI data of BIOS.
> 
> Signed-off-by: Yuechao Zhao <yuechao.zhao@advantech.com.cn>
> Signed-off-by: Sui Jia <jia.sui@advantech.com.cn>

Please Cc LED drivers to the LED maintainers...

> +
> +static inline u8 amix955x_set_bit(u8 val, int led_id, enum 
> +AMIX955X_LED
> +state) {
> +	led_id %= BANK_SIZE;
> +
> +	switch (state) {
> +	case AMIX955X_LED_ON:
> +		val &= ~(1 << led_id); //Clear bit
> +		break;
> +	case AMIX955X_LED_OFF:
> +		val |= 1 << led_id; //Set bit
> +		break;
> +	}
> +
> +	return val;

Please check CodingStyle, avoid useless comments and check word wrapping in your email settings.

Best regards,
								Pavel

--
http://www.livejournal.com/~pavelmachek

Re: [v2,1/1] adv_mix955x is a scheme that multiplexes PCA9554/PCA9555 into LED and GPIO

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]

On Wed 2021-04-28 01:51:04, yuechao.zhao(赵越超) wrote:
> Hi Pavel
> As for " Please Cc LED drivers to the LED maintainers...", does it mean that I only need to Cc LED drivers to LED maintainers?
> Or do I need to send this driver to linux-leds@vger.kernel.org instead of platform-driver-x86@vger.kernel.org.
>

You should probably send to both lists.
						Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [v2,1/1] adv_mix955x is a scheme that multiplexes PCA9554/PCA9555 into LED and GPIO

[Andy Shevchenko]

On Tue, Apr 27, 2021 at 11:33 AM yuechao.zhao(赵越超)
<yuechao.zhao@advantech.com.cn> wrote:
>
> From: Yuechao Zhao <yuechao.zhao@advantech.com.cn>
>
> With this driver, we can multiplex PCA9554/PCA9555 into LED and GPIO based on the ACPI data of BIOS.

I really did no get the purpose of this rather enormous pile of code.
Why you can't simply use PCA9555 with correctly described leds-gpio.
What the problem this driver solves that can't be solved by existing
code?

NAK till it will gain a very good justification.

-- 
With Best Regards,
Andy Shevchenko