Re: [PATCH v7 1/1] leds: LED driver for TI LP3952 6-Channel Color LED

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thank you for the comments Rafael.

On 29/06/16 01:45, Rafael J. Wysocki wrote:
On Tuesday, June 28, 2016 05:05:19 PM Tony Makkiel wrote:


+static int lp3952_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	int status;
+	struct lp3952_ctrl_hdl *leds;
+	struct lp3952_led_array *priv;
+
+	priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	leds = devm_kcalloc(&client->dev, LP3952_LED_ALL, sizeof(*leds),
+			    GFP_KERNEL);
+	if (!leds) {
+		devm_kfree(&client->dev, priv);
+		return -ENOMEM;
+	}
+
+	priv->leds = leds;
+	priv->client = client;
+
+	priv->enable_gpio = devm_gpiod_get(&client->dev, NULL, GPIOD_OUT_HIGH);
+	if (IS_ERR(priv->enable_gpio)) {
+		status = PTR_ERR(priv->enable_gpio);
+		dev_err(&client->dev, "Failed to enable gpio: %d\n", status);
+		return status;
+	}
+
+	priv->regmap = devm_regmap_init_i2c(client, &lp3952_regmap);
+	if (IS_ERR(priv->regmap)) {
+		int err = PTR_ERR(priv->regmap);
+
+		dev_err(&client->dev, "Failed to allocate register map: %d\n",
+			err);
+		return err;
+	}
+
+	i2c_set_clientdata(client, priv);
+
+	status = lp3952_configure(priv);
+	if (status) {
+		dev_err(&client->dev, "Probe failed. Device not found (%d)\n",
+			status);
+		return status;
+	}
+
+	status = lp3952_register_led_classdev(priv);
+	if (status) {
+		dev_err(&client->dev, "Unable to register led_classdev: %d\n",
+			status);
+		return status;
+	}
+
+	return 0;
+}
+
+static int lp3952_remove(struct i2c_client *client)
+{
+	struct lp3952_led_array *priv;
+
+	priv = i2c_get_clientdata(client);
+	lp3952_on_off(priv, LP3952_LED_ALL, 0);
+	gpiod_set_value(priv->enable_gpio, 0);
+
+	return 0;
+}
+
+static const struct i2c_device_id lp3952_id[] = {
+	{LP3952_NAME, 0},
+	{}
+};
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id lp3952_acpi_match[] = {
+	{LP3952_ACPI_NAME, 0},

No, you can't use "PRP0001" in this list.

+	{}
+};
+
+MODULE_DEVICE_TABLE(acpi, lp3952_acpi_match);

And you don't need this for the "PRP0001" thing to work.  The core will
take care of it for you then.

+#endif

So the entire ACPI block can be dropped for now.

And the driver doesn't have to depend on CONFIG_ACPI any more, does it?
We need to pass the reset gpio to the driver.

"devm_gpiod_get(&client->dev, NULL, GPIOD_OUT_HIGH);"

in the "lp3952_probe". I shall remove the PRP0001 from the driver and check whether the gpio can be still read from ACPI. If it can, removing ACPI block shouldn't be a problem.


+
+static struct i2c_driver lp3952_i2c_driver = {
+	.driver = {
+		   .name = LP3952_NAME,
+		   .acpi_match_table = ACPI_PTR(lp3952_acpi_match),
+		  },
+	.probe = lp3952_probe,
+	.remove = lp3952_remove,
+	.id_table = lp3952_id,
+};
+
+module_i2c_driver(lp3952_i2c_driver);
+
+MODULE_AUTHOR("Tony Makkiel <tony.makkiel@xxxxxxxxx>");
+MODULE_DESCRIPTION("lp3952 I2C LED controller driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/leds-lp3952.h b/include/linux/leds-lp3952.h
new file mode 100644
index 0000000..9808ab7
--- /dev/null
+++ b/include/linux/leds-lp3952.h
@@ -0,0 +1,128 @@
+/*
+ *	LED driver for TI lp3952 controller
+ *
+ *	Copyright (C) 2016, DAQRI, LLC.
+ *	Author: Tony Makkiel <tony.makkiel@xxxxxxxxx>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#ifndef LEDS_LP3952_H_
+#define LEDS_LP3952_H_
+
+/* ACPI iasl compiler wont take name LP3952,
+ * "ASL_MSG_HID_LENGTH". Hence the name TLP3952
+ */
+#define LP3952_NAME                         "lp3952"
+#define LP3952_ACPI_NAME                    "PRP0001"

This is not necessary.

+#define LP3952_CMD_REG_COUNT                8
+#define LP3952_BRIGHT_MAX                   4
+
+#define LP3952_REG_LED_CTRL                 0x00
+#define LP3952_REG_R1_BLNK_TIME_CTRL        0x01
+#define LP3952_REG_R1_BLNK_CYCLE_CTRL       0x02
+#define LP3952_REG_G1_BLNK_TIME_CTRL        0x03
+#define LP3952_REG_G1_BLNK_CYCLE_CTRL       0x04
+#define LP3952_REG_B1_BLNK_TIME_CTRL        0x05
+#define LP3952_REG_B1_BLNK_CYCLE_CTRL       0x06
+#define LP3952_REG_ENABLES                  0x0B
+#define LP3952_REG_PAT_GEN_CTRL             0x11
+#define LP3952_REG_RGB1_MAX_I_CTRL          0x12
+#define LP3952_REG_RGB2_MAX_I_CTRL          0x13
+#define LP3952_REG_CMD_0                    0x50
+#define LP3952_REG_RESET                    0x60
+#define REG_MAX                             LP3952_REG_RESET
+
+#define LP3952_PATRN_LOOP                   BIT(1)
+#define LP3952_PATRN_GEN_EN                 BIT(2)
+#define LP3952_INT_B00ST_LDR                BIT(2)
+#define LP3952_ACTIVE_MODE                  BIT(6)
+#define LP3952_LED_MASK_ALL                 0x3f
+
+/* Transition Time in ms */
+enum lp3952_tt {
+	TT0,
+	TT55,
+	TT110,
+	TT221,
+	TT422,
+	TT885,
+	TT1770,
+	TT3539
+};
+
+/* Command Execution Time in ms */
+enum lp3952_cet {
+	CET197,
+	CET393,
+	CET590,
+	CET786,
+	CET1180,
+	CET1376,
+	CET1573,
+	CET1769,
+	CET1966,
+	CET2163,
+	CET2359,
+	CET2556,
+	CET2763,
+	CET2949,
+	CET3146
+};
+
+/* Max Current in % */
+enum lp3952_colour_I_log_0 {
+	I0,
+	I7,
+	I14,
+	I21,
+	I32,
+	I46,
+	I71,
+	I100
+};
+
+enum lp3952_leds {
+	LP3952_BLUE_2,
+	LP3952_GREEN_2,
+	LP3952_RED_2,
+	LP3952_BLUE_1,
+	LP3952_GREEN_1,
+	LP3952_RED_1,
+	LP3952_LED_ALL
+};
+
+struct lp3952_ctrl_hdl {
+	struct led_classdev cdev;
+	char name[15];
+	enum lp3952_leds channel;
+	void *priv;
+};
+
+struct ptrn_gen_cmd {
+	union {
+		struct {
+			u16 tt:3;
+			u16 b:3;
+			u16 cet:4;
+			u16 g:3;
+			u16 r:3;
+		};
+		struct {
+			u8 lsb;
+			u8 msb;
+		} bytes;
+	};
+} __packed;
+
+struct lp3952_led_array {
+	struct regmap *regmap;
+	struct i2c_client *client;
+	struct gpio_desc *enable_gpio;
+	struct lp3952_ctrl_hdl *leds;
+};
+
+#endif /* LEDS_LP3952_H_ */

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux