* [PATCH v2 0/4] leds: tps68470: LED driver for TPS68470
@ 2023-03-10 9:56 Kate Hsuan
2023-03-10 9:56 ` [PATCH v2 1/4] platform: x86: int3472: Add MFD cell for tps68470 LED Kate Hsuan
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Kate Hsuan @ 2023-03-10 9:56 UTC (permalink / raw)
To: Pavel Machek, Lee Jones, linux-leds, platform-driver-x86,
Hans de Goede, Daniel Scally, Mark Gross
Cc: Kate Hsuan
The v2 patch of TPS68470 LED driver includes:
1. The code improvements were based on reviewers' comments.
2. strcmp() was removed.
3. V2 also included Hans' LED look-up table for the privacy LED.
Hans de Goede (1):
platform: x86: int3472: Register a LED lookup table entry for the
privacy LED
Kate Hsuan (3):
platform: x86: int3472: Add MFD cell for tps68470 LED
include: mfd: tps68470: Add masks for LEDA and LEDB
leds: tps68470: Add LED control for tps68470
drivers/leds/Kconfig | 12 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-tps68470.c | 182 ++++++++++++++++++
drivers/platform/x86/intel/int3472/tps68470.c | 35 +++-
include/linux/mfd/tps68470.h | 5 +
5 files changed, 231 insertions(+), 4 deletions(-)
create mode 100644 drivers/leds/leds-tps68470.c
--
2.37.3
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/4] platform: x86: int3472: Add MFD cell for tps68470 LED
2023-03-10 9:56 [PATCH v2 0/4] leds: tps68470: LED driver for TPS68470 Kate Hsuan
@ 2023-03-10 9:56 ` Kate Hsuan
2023-03-10 11:13 ` Dan Scally
2023-03-10 9:56 ` [PATCH v2 2/4] include: mfd: tps68470: Add masks for LEDA and LEDB Kate Hsuan
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Kate Hsuan @ 2023-03-10 9:56 UTC (permalink / raw)
To: Pavel Machek, Lee Jones, linux-leds, platform-driver-x86,
Hans de Goede, Daniel Scally, Mark Gross
Cc: Kate Hsuan
Add MFD cell for tps68470-led.
Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
drivers/platform/x86/intel/int3472/tps68470.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/intel/int3472/tps68470.c b/drivers/platform/x86/intel/int3472/tps68470.c
index 5b8d1a9620a5..82ef022f8916 100644
--- a/drivers/platform/x86/intel/int3472/tps68470.c
+++ b/drivers/platform/x86/intel/int3472/tps68470.c
@@ -17,7 +17,7 @@
#define DESIGNED_FOR_CHROMEOS 1
#define DESIGNED_FOR_WINDOWS 2
-#define TPS68470_WIN_MFD_CELL_COUNT 3
+#define TPS68470_WIN_MFD_CELL_COUNT 4
static const struct mfd_cell tps68470_cros[] = {
{ .name = "tps68470-gpio" },
@@ -193,7 +193,8 @@ static int skl_int3472_tps68470_probe(struct i2c_client *client)
cells[1].name = "tps68470-regulator";
cells[1].platform_data = (void *)board_data->tps68470_regulator_pdata;
cells[1].pdata_size = sizeof(struct tps68470_regulator_platform_data);
- cells[2].name = "tps68470-gpio";
+ cells[2].name = "tps68470-led";
+ cells[3].name = "tps68470-gpio";
for (i = 0; i < board_data->n_gpiod_lookups; i++)
gpiod_add_lookup_table(board_data->tps68470_gpio_lookup_tables[i]);
--
2.37.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/4] include: mfd: tps68470: Add masks for LEDA and LEDB
2023-03-10 9:56 [PATCH v2 0/4] leds: tps68470: LED driver for TPS68470 Kate Hsuan
2023-03-10 9:56 ` [PATCH v2 1/4] platform: x86: int3472: Add MFD cell for tps68470 LED Kate Hsuan
@ 2023-03-10 9:56 ` Kate Hsuan
2023-03-10 9:56 ` [PATCH v2 3/4] leds: tps68470: Add LED control for tps68470 Kate Hsuan
2023-03-10 9:56 ` [PATCH v2 4/4] platform: x86: int3472: Register a LED lookup table entry for the privacy LED Kate Hsuan
3 siblings, 0 replies; 11+ messages in thread
From: Kate Hsuan @ 2023-03-10 9:56 UTC (permalink / raw)
To: Pavel Machek, Lee Jones, linux-leds, platform-driver-x86,
Hans de Goede, Daniel Scally, Mark Gross
Cc: Kate Hsuan, Daniel Scally
Add flags for both LEDA(TPS68470_ILEDCTL_ENA), LEDB
(TPS68470_ILEDCTL_ENB), and current control mask for LEDB
(TPS68470_ILEDCTL_CTRLB)
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
include/linux/mfd/tps68470.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/mfd/tps68470.h b/include/linux/mfd/tps68470.h
index 7807fa329db0..2d2abb25b944 100644
--- a/include/linux/mfd/tps68470.h
+++ b/include/linux/mfd/tps68470.h
@@ -34,6 +34,7 @@
#define TPS68470_REG_SGPO 0x22
#define TPS68470_REG_GPDI 0x26
#define TPS68470_REG_GPDO 0x27
+#define TPS68470_REG_ILEDCTL 0x28
#define TPS68470_REG_VCMVAL 0x3C
#define TPS68470_REG_VAUX1VAL 0x3D
#define TPS68470_REG_VAUX2VAL 0x3E
@@ -94,4 +95,8 @@
#define TPS68470_GPIO_MODE_OUT_CMOS 2
#define TPS68470_GPIO_MODE_OUT_ODRAIN 3
+#define TPS68470_ILEDCTL_ENA BIT(2)
+#define TPS68470_ILEDCTL_ENB BIT(6)
+#define TPS68470_ILEDCTL_CTRLB GENMASK(5, 4)
+
#endif /* __LINUX_MFD_TPS68470_H */
--
2.37.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/4] leds: tps68470: Add LED control for tps68470
2023-03-10 9:56 [PATCH v2 0/4] leds: tps68470: LED driver for TPS68470 Kate Hsuan
2023-03-10 9:56 ` [PATCH v2 1/4] platform: x86: int3472: Add MFD cell for tps68470 LED Kate Hsuan
2023-03-10 9:56 ` [PATCH v2 2/4] include: mfd: tps68470: Add masks for LEDA and LEDB Kate Hsuan
@ 2023-03-10 9:56 ` Kate Hsuan
2023-03-10 10:43 ` Dan Scally
2023-03-20 14:23 ` Dan Scally
2023-03-10 9:56 ` [PATCH v2 4/4] platform: x86: int3472: Register a LED lookup table entry for the privacy LED Kate Hsuan
3 siblings, 2 replies; 11+ messages in thread
From: Kate Hsuan @ 2023-03-10 9:56 UTC (permalink / raw)
To: Pavel Machek, Lee Jones, linux-leds, platform-driver-x86,
Hans de Goede, Daniel Scally, Mark Gross
Cc: Kate Hsuan
There are two LED controllers, LEDA indicator LED and LEDB flash LED for
tps68470. LEDA can be enabled by setting TPS68470_ILEDCTL_ENA. Moreover,
tps68470 provides four levels of power status for LEDB. If the
properties called "ti,ledb-current" can be found, the current will be
set according to the property values. These two LEDs can be controlled
through the LED class of sysfs (tps68470-leda and tps68470-ledb).
Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
drivers/leds/Kconfig | 12 +++
drivers/leds/Makefile | 1 +
drivers/leds/leds-tps68470.c | 182 +++++++++++++++++++++++++++++++++++
3 files changed, 195 insertions(+)
create mode 100644 drivers/leds/leds-tps68470.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 9dbce09eabac..fd26036b3c61 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -827,6 +827,18 @@ config LEDS_TPS6105X
It is a single boost converter primarily for white LEDs and
audio amplifiers.
+config LEDS_TPS68470
+ tristate "LED support for TI TPS68470"
+ depends on LEDS_CLASS
+ depends on INTEL_SKL_INT3472
+ help
+ This driver supports TPS68470 PMIC with LED chip.
+ It provide two LED controllers, including an indicator LED
+ and a flash LED.
+
+ To compile this driver as a module, choose M and it will be
+ called leds-tps68470
+
config LEDS_IP30
tristate "LED support for SGI Octane machines"
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index d30395d11fd8..b284bc0daa98 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -84,6 +84,7 @@ obj-$(CONFIG_LEDS_TURRIS_OMNIA) += leds-turris-omnia.o
obj-$(CONFIG_LEDS_WM831X_STATUS) += leds-wm831x-status.o
obj-$(CONFIG_LEDS_WM8350) += leds-wm8350.o
obj-$(CONFIG_LEDS_WRAP) += leds-wrap.o
+obj-$(CONFIG_LEDS_TPS68470) += leds-tps68470.o
# LED SPI Drivers
obj-$(CONFIG_LEDS_CR0014114) += leds-cr0014114.o
diff --git a/drivers/leds/leds-tps68470.c b/drivers/leds/leds-tps68470.c
new file mode 100644
index 000000000000..98bb56153690
--- /dev/null
+++ b/drivers/leds/leds-tps68470.c
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * LED driver for TPS68470 PMIC
+ *
+ * Copyright (C) 2023 Red Hat
+ *
+ * Authors:
+ * Kate Hsuan <hpa@redhat.com>
+ */
+
+#include <linux/gpio/driver.h>
+#include <linux/mfd/tps68470.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/leds.h>
+
+#define lcdev_to_led(led_cdev) \
+ container_of(led_cdev, struct tps68470_led, lcdev)
+
+#define led_to_tps68470(led, index) \
+ container_of(led, struct tps68470_device, leds[index])
+
+enum tps68470_led_ids {
+ TPS68470_ILED_A,
+ TPS68470_ILED_B,
+ TPS68470_NUM_LEDS
+};
+
+static const char *tps68470_led_names[] = {
+ [TPS68470_ILED_A] = "tps68470-iled_a",
+ [TPS68470_ILED_B] = "tps68470-iled_b",
+};
+
+struct tps68470_led {
+ unsigned int led_id;
+ struct led_classdev lcdev;
+};
+
+struct tps68470_device {
+ struct device *dev;
+ struct regmap *regmap;
+ struct tps68470_led leds[TPS68470_NUM_LEDS];
+};
+
+enum ctrlb_current {
+ CTRLB_2MA = 0,
+ CTRLB_4MA = 1,
+ CTRLB_8MA = 2,
+ CTRLB_16MA = 3,
+};
+
+static int tps68470_brightness_set(struct led_classdev *led_cdev, enum led_brightness brightness)
+{
+ struct tps68470_led *led = lcdev_to_led(led_cdev);
+ struct tps68470_device *tps68470 = led_to_tps68470(led, led->led_id);
+ struct regmap *regmap = tps68470->regmap;
+
+ switch (led->led_id) {
+ case TPS68470_ILED_A:
+ return regmap_update_bits(regmap, TPS68470_REG_ILEDCTL, TPS68470_ILEDCTL_ENA,
+ brightness ? TPS68470_ILEDCTL_ENA : 0);
+ case TPS68470_ILED_B:
+ return regmap_update_bits(regmap, TPS68470_REG_ILEDCTL, TPS68470_ILEDCTL_ENB,
+ brightness ? TPS68470_ILEDCTL_ENB : 0);
+ }
+ return -EINVAL;
+}
+
+static enum led_brightness tps68470_brightness_get(struct led_classdev *led_cdev)
+{
+ struct tps68470_led *led = lcdev_to_led(led_cdev);
+ struct tps68470_device *tps68470 = led_to_tps68470(led, led->led_id);
+ struct regmap *regmap = tps68470->regmap;
+ int ret = 0;
+ int value = 0;
+
+ ret = regmap_read(regmap, TPS68470_REG_ILEDCTL, &value);
+ if (ret)
+ goto error;
+
+ switch (led->led_id) {
+ case TPS68470_ILED_A:
+ value = value & TPS68470_ILEDCTL_ENA;
+ break;
+ case TPS68470_ILED_B:
+ value = value & TPS68470_ILEDCTL_ENB;
+ break;
+ }
+
+ return value ? LED_ON : LED_OFF;
+
+error:
+ dev_err(led_cdev->dev, "Failed on reading register\n");
+ return -EINVAL;
+}
+
+static int tps68470_leds_probe(struct platform_device *pdev)
+{
+ int i = 0;
+ int ret = 0;
+ unsigned int curr;
+ struct tps68470_device *tps68470;
+ struct tps68470_led *led;
+ struct led_classdev *lcdev;
+
+ tps68470 = devm_kzalloc(&pdev->dev, sizeof(struct tps68470_device),
+ GFP_KERNEL);
+ tps68470->dev = &pdev->dev;
+ tps68470->regmap = dev_get_drvdata(pdev->dev.parent);
+
+ for (i = 0; i < TPS68470_NUM_LEDS; i++) {
+ led = &tps68470->leds[i];
+ lcdev = &led->lcdev;
+
+ led->led_id = i;
+
+ lcdev->name = devm_kasprintf(tps68470->dev, GFP_KERNEL, "%s::%s",
+ tps68470_led_names[i], LED_FUNCTION_INDICATOR);
+ if (!lcdev->name)
+ return -ENOMEM;
+
+ lcdev->max_brightness = 1;
+ lcdev->brightness = 0;
+ lcdev->brightness_set_blocking = tps68470_brightness_set;
+ lcdev->brightness_get = tps68470_brightness_get;
+ lcdev->dev = &pdev->dev;
+
+ ret = devm_led_classdev_register(tps68470->dev, lcdev);
+ if (ret) {
+ dev_err_probe(tps68470->dev, ret,
+ "error registering led\n");
+ goto err_exit;
+ }
+ }
+
+ /* configure LEDB current if the properties can be got */
+ if (!device_property_read_u32(&pdev->dev, "ti,ledb-current", &curr)) {
+ switch (curr) {
+ case 2:
+ curr = CTRLB_2MA;
+ break;
+ case 4:
+ curr = CTRLB_4MA;
+ break;
+ case 8:
+ curr = CTRLB_8MA;
+ break;
+ case 16:
+ curr = CTRLB_16MA;
+ break;
+ default:
+ dev_err(&pdev->dev, "Invalid LEDB curr value: %d\n", curr);
+ return -EINVAL;
+ }
+ ret = regmap_update_bits(tps68470->regmap, TPS68470_REG_ILEDCTL,
+ TPS68470_ILEDCTL_CTRLB, curr);
+ }
+
+err_exit:
+ if (ret) {
+ for (i = 0; i < TPS68470_NUM_LEDS; i++) {
+ if (tps68470->leds[i].lcdev.name)
+ devm_led_classdev_unregister(&pdev->dev,
+ &tps68470->leds[i].lcdev);
+ }
+ }
+
+ return ret;
+}
+static struct platform_driver tps68470_led_driver = {
+ .driver = {
+ .name = "tps68470-led",
+ },
+ .probe = tps68470_leds_probe,
+};
+
+module_platform_driver(tps68470_led_driver);
+
+MODULE_ALIAS("platform:tps68470-led");
+MODULE_DESCRIPTION("LED driver for TPS68470 PMIC");
+MODULE_LICENSE("GPL v2");
--
2.37.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 4/4] platform: x86: int3472: Register a LED lookup table entry for the privacy LED
2023-03-10 9:56 [PATCH v2 0/4] leds: tps68470: LED driver for TPS68470 Kate Hsuan
` (2 preceding siblings ...)
2023-03-10 9:56 ` [PATCH v2 3/4] leds: tps68470: Add LED control for tps68470 Kate Hsuan
@ 2023-03-10 9:56 ` Kate Hsuan
2023-03-10 11:23 ` Dan Scally
3 siblings, 1 reply; 11+ messages in thread
From: Kate Hsuan @ 2023-03-10 9:56 UTC (permalink / raw)
To: Pavel Machek, Lee Jones, linux-leds, platform-driver-x86,
Hans de Goede, Daniel Scally, Mark Gross
From: Hans de Goede <hdegoede@redhat.com>
All currently known models using the tps68470 PMIC have a privacy LED for
the back sensor (first ACPI consumer dev of the PMIC) connected to the
ileda output of the PMIC.
Add a LED lookup table entry for this, so that the v4l2-core code turns on
the LED when streaming from the sensor.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/intel/int3472/tps68470.c | 30 +++++++++++++++++--
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/intel/int3472/tps68470.c b/drivers/platform/x86/intel/int3472/tps68470.c
index 82ef022f8916..a53de9cd0540 100644
--- a/drivers/platform/x86/intel/int3472/tps68470.c
+++ b/drivers/platform/x86/intel/int3472/tps68470.c
@@ -140,11 +140,18 @@ skl_int3472_fill_clk_pdata(struct device *dev, struct tps68470_clk_platform_data
return n_consumers;
}
+static void skl_int3472_tps68470_unregister_led_lookup(void *led_lookup)
+{
+ led_remove_lookup(led_lookup);
+}
+
static int skl_int3472_tps68470_probe(struct i2c_client *client)
{
- struct acpi_device *adev = ACPI_COMPANION(&client->dev);
+ struct device *dev = &client->dev;
+ struct acpi_device *adev = ACPI_COMPANION(dev);
const struct int3472_tps68470_board_data *board_data;
struct tps68470_clk_platform_data *clk_pdata;
+ struct led_lookup_data *led_lookup;
struct mfd_cell *cells;
struct regmap *regmap;
int n_consumers;
@@ -177,6 +184,25 @@ static int skl_int3472_tps68470_probe(struct i2c_client *client)
if (!board_data)
return dev_err_probe(&client->dev, -ENODEV, "No board-data found for this model\n");
+ /* Add a LED lookup table entry for the privacy LED */
+ led_lookup = devm_kzalloc(&client->dev, sizeof(*led_lookup), GFP_KERNEL);
+ if (!led_lookup)
+ return -ENOMEM;
+
+ ret = skl_int3472_get_sensor_adev_and_name(&client->dev, NULL, &led_lookup->dev_id);
+ if (ret)
+ return ret;
+
+ led_lookup->provider = "tps68470-ileda";
+ led_lookup->con_id = "privacy-led";
+ led_add_lookup(led_lookup);
+
+ ret = devm_add_action_or_reset(&client->dev,
+ skl_int3472_tps68470_unregister_led_lookup,
+ led_lookup);
+ if (ret)
+ return ret;
+
cells = kcalloc(TPS68470_WIN_MFD_CELL_COUNT, sizeof(*cells), GFP_KERNEL);
if (!cells)
return -ENOMEM;
@@ -259,4 +285,4 @@ module_i2c_driver(int3472_tps68470);
MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI TPS68470 Device Driver");
MODULE_AUTHOR("Daniel Scally <djrscally@gmail.com>");
MODULE_LICENSE("GPL v2");
-MODULE_SOFTDEP("pre: clk-tps68470 tps68470-regulator");
+MODULE_SOFTDEP("pre: clk-tps68470 tps68470-regulator leds-tps68470");
--
2.37.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/4] leds: tps68470: Add LED control for tps68470
2023-03-10 9:56 ` [PATCH v2 3/4] leds: tps68470: Add LED control for tps68470 Kate Hsuan
@ 2023-03-10 10:43 ` Dan Scally
2023-03-13 3:21 ` Kate Hsuan
2023-03-20 14:23 ` Dan Scally
1 sibling, 1 reply; 11+ messages in thread
From: Dan Scally @ 2023-03-10 10:43 UTC (permalink / raw)
To: Kate Hsuan, Pavel Machek, Lee Jones, linux-leds,
platform-driver-x86, Hans de Goede, Daniel Scally, Mark Gross
Hi Kate - thanks for the v2
On 10/03/2023 09:56, Kate Hsuan wrote:
> There are two LED controllers, LEDA indicator LED and LEDB flash LED for
> tps68470. LEDA can be enabled by setting TPS68470_ILEDCTL_ENA. Moreover,
> tps68470 provides four levels of power status for LEDB. If the
> properties called "ti,ledb-current" can be found, the current will be
> set according to the property values. These two LEDs can be controlled
> through the LED class of sysfs (tps68470-leda and tps68470-ledb).
>
> Signed-off-by: Kate Hsuan <hpa@redhat.com>
> ---
> drivers/leds/Kconfig | 12 +++
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-tps68470.c | 182 +++++++++++++++++++++++++++++++++++
> 3 files changed, 195 insertions(+)
> create mode 100644 drivers/leds/leds-tps68470.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 9dbce09eabac..fd26036b3c61 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -827,6 +827,18 @@ config LEDS_TPS6105X
> It is a single boost converter primarily for white LEDs and
> audio amplifiers.
>
> +config LEDS_TPS68470
> + tristate "LED support for TI TPS68470"
> + depends on LEDS_CLASS
> + depends on INTEL_SKL_INT3472
> + help
> + This driver supports TPS68470 PMIC with LED chip.
> + It provide two LED controllers, including an indicator LED
> + and a flash LED.
s/provide/provides. Also maybe "It provides two LED controllers, with the ability to drive 2
indicator LEDs and 2 flash LEDs". I actually got the WLED part working now finally so I'll send
patches on top of this series if that's ok?
> +
> + To compile this driver as a module, choose M and it will be
> + called leds-tps68470
> +
> config LEDS_IP30
> tristate "LED support for SGI Octane machines"
> depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index d30395d11fd8..b284bc0daa98 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -84,6 +84,7 @@ obj-$(CONFIG_LEDS_TURRIS_OMNIA) += leds-turris-omnia.o
> obj-$(CONFIG_LEDS_WM831X_STATUS) += leds-wm831x-status.o
> obj-$(CONFIG_LEDS_WM8350) += leds-wm8350.o
> obj-$(CONFIG_LEDS_WRAP) += leds-wrap.o
> +obj-$(CONFIG_LEDS_TPS68470) += leds-tps68470.o
>
> # LED SPI Drivers
> obj-$(CONFIG_LEDS_CR0014114) += leds-cr0014114.o
> diff --git a/drivers/leds/leds-tps68470.c b/drivers/leds/leds-tps68470.c
> new file mode 100644
> index 000000000000..98bb56153690
> --- /dev/null
> +++ b/drivers/leds/leds-tps68470.c
> @@ -0,0 +1,182 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * LED driver for TPS68470 PMIC
> + *
> + * Copyright (C) 2023 Red Hat
> + *
> + * Authors:
> + * Kate Hsuan <hpa@redhat.com>
> + */
> +
> +#include <linux/gpio/driver.h>
Not needed I think?
> +#include <linux/mfd/tps68470.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/leds.h>
Alphabetical order?
> +
> +#define lcdev_to_led(led_cdev) \
> + container_of(led_cdev, struct tps68470_led, lcdev)
> +
> +#define led_to_tps68470(led, index) \
> + container_of(led, struct tps68470_device, leds[index])
> +
> +enum tps68470_led_ids {
> + TPS68470_ILED_A,
> + TPS68470_ILED_B,
> + TPS68470_NUM_LEDS
> +};
> +
> +static const char *tps68470_led_names[] = {
> + [TPS68470_ILED_A] = "tps68470-iled_a",
> + [TPS68470_ILED_B] = "tps68470-iled_b",
> +};
> +
> +struct tps68470_led {
> + unsigned int led_id;
> + struct led_classdev lcdev;
> +};
> +
> +struct tps68470_device {
> + struct device *dev;
> + struct regmap *regmap;
> + struct tps68470_led leds[TPS68470_NUM_LEDS];
> +};
> +
> +enum ctrlb_current {
> + CTRLB_2MA = 0,
> + CTRLB_4MA = 1,
> + CTRLB_8MA = 2,
> + CTRLB_16MA = 3,
> +};
> +
> +static int tps68470_brightness_set(struct led_classdev *led_cdev, enum led_brightness brightness)
> +{
> + struct tps68470_led *led = lcdev_to_led(led_cdev);
> + struct tps68470_device *tps68470 = led_to_tps68470(led, led->led_id);
> + struct regmap *regmap = tps68470->regmap
This would work fine as is...but I would maybe add something like
if (state > LED_ON)
return -EINVAL;
So that brightness values of > 1 aren't just silently accepted...or does the LED core already
prevent that? If so it's fine.
> +
> + switch (led->led_id) {
> + case TPS68470_ILED_A:
> + return regmap_update_bits(regmap, TPS68470_REG_ILEDCTL, TPS68470_ILEDCTL_ENA,
> + brightness ? TPS68470_ILEDCTL_ENA : 0);
> + case TPS68470_ILED_B:
> + return regmap_update_bits(regmap, TPS68470_REG_ILEDCTL, TPS68470_ILEDCTL_ENB,
> + brightness ? TPS68470_ILEDCTL_ENB : 0);
> + }
> + return -EINVAL;
> +}
> +
> +static enum led_brightness tps68470_brightness_get(struct led_classdev *led_cdev)
> +{
> + struct tps68470_led *led = lcdev_to_led(led_cdev);
> + struct tps68470_device *tps68470 = led_to_tps68470(led, led->led_id);
> + struct regmap *regmap = tps68470->regmap;
> + int ret = 0;
> + int value = 0;
> +
> + ret = regmap_read(regmap, TPS68470_REG_ILEDCTL, &value);
> + if (ret)
> + goto error;
Just return dev_err_probe(led_cdev->dev, -EINVAL, "failed on reading register\n") here imo.
> +
> + switch (led->led_id) {
> + case TPS68470_ILED_A:
> + value = value & TPS68470_ILEDCTL_ENA;
> + break;
> + case TPS68470_ILED_B:
> + value = value & TPS68470_ILEDCTL_ENB;
> + break;
> + }
> +
> + return value ? LED_ON : LED_OFF;
> +
> +error:
> + dev_err(led_cdev->dev, "Failed on reading register\n");
> + return -EINVAL;
> +}
> +
> +static int tps68470_leds_probe(struct platform_device *pdev)
> +{
> + int i = 0;
> + int ret = 0;
> + unsigned int curr;
> + struct tps68470_device *tps68470;
> + struct tps68470_led *led;
> + struct led_classdev *lcdev;
> +
> + tps68470 = devm_kzalloc(&pdev->dev, sizeof(struct tps68470_device),
> + GFP_KERNEL);
No -ENOMEM check here?
> + tps68470->dev = &pdev->dev;
> + tps68470->regmap = dev_get_drvdata(pdev->dev.parent);
> +
> + for (i = 0; i < TPS68470_NUM_LEDS; i++) {
> + led = &tps68470->leds[i];
> + lcdev = &led->lcdev;
> +
> + led->led_id = i;
> +
> + lcdev->name = devm_kasprintf(tps68470->dev, GFP_KERNEL, "%s::%s",
> + tps68470_led_names[i], LED_FUNCTION_INDICATOR);
> + if (!lcdev->name)
> + return -ENOMEM;
> +
> + lcdev->max_brightness = 1;
> + lcdev->brightness = 0;
> + lcdev->brightness_set_blocking = tps68470_brightness_set;
> + lcdev->brightness_get = tps68470_brightness_get;
> + lcdev->dev = &pdev->dev;
> +
> + ret = devm_led_classdev_register(tps68470->dev, lcdev);
> + if (ret) {
> + dev_err_probe(tps68470->dev, ret,
> + "error registering led\n");
> + goto err_exit;
> + }
> + }
> +
> + /* configure LEDB current if the properties can be got */
> + if (!device_property_read_u32(&pdev->dev, "ti,ledb-current", &curr)) {
> + switch (curr) {
> + case 2:
> + curr = CTRLB_2MA;
> + break;
> + case 4:
> + curr = CTRLB_4MA;
> + break;
> + case 8:
> + curr = CTRLB_8MA;
> + break;
> + case 16:
> + curr = CTRLB_16MA;
> + break;
> + default:
> + dev_err(&pdev->dev, "Invalid LEDB curr value: %d\n", curr);
> + return -EINVAL;
There's no jump to err_exit here...I think that this whole section should go above the registration
of the LEDS...and probably also into its own function.
> + }
> + ret = regmap_update_bits(tps68470->regmap, TPS68470_REG_ILEDCTL,
> + TPS68470_ILEDCTL_CTRLB, curr);
> + }
> +
> +err_exit:
> + if (ret) {
> + for (i = 0; i < TPS68470_NUM_LEDS; i++) {
> + if (tps68470->leds[i].lcdev.name)
> + devm_led_classdev_unregister(&pdev->dev,
> + &tps68470->leds[i].lcdev);
> + }
> + }
> +
> + return ret;
> +}
> +static struct platform_driver tps68470_led_driver = {
> + .driver = {
> + .name = "tps68470-led",
> + },
> + .probe = tps68470_leds_probe,
> +};
> +
> +module_platform_driver(tps68470_led_driver);
> +
> +MODULE_ALIAS("platform:tps68470-led");
> +MODULE_DESCRIPTION("LED driver for TPS68470 PMIC");
> +MODULE_LICENSE("GPL v2");
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/4] platform: x86: int3472: Add MFD cell for tps68470 LED
2023-03-10 9:56 ` [PATCH v2 1/4] platform: x86: int3472: Add MFD cell for tps68470 LED Kate Hsuan
@ 2023-03-10 11:13 ` Dan Scally
0 siblings, 0 replies; 11+ messages in thread
From: Dan Scally @ 2023-03-10 11:13 UTC (permalink / raw)
To: Kate Hsuan, Pavel Machek, Lee Jones, linux-leds,
platform-driver-x86, Hans de Goede, Daniel Scally, Mark Gross
On 10/03/2023 09:56, Kate Hsuan wrote:
> Add MFD cell for tps68470-led.
>
> Signed-off-by: Kate Hsuan <hpa@redhat.com>
> ---
Looks fine to me: Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
> drivers/platform/x86/intel/int3472/tps68470.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/int3472/tps68470.c b/drivers/platform/x86/intel/int3472/tps68470.c
> index 5b8d1a9620a5..82ef022f8916 100644
> --- a/drivers/platform/x86/intel/int3472/tps68470.c
> +++ b/drivers/platform/x86/intel/int3472/tps68470.c
> @@ -17,7 +17,7 @@
> #define DESIGNED_FOR_CHROMEOS 1
> #define DESIGNED_FOR_WINDOWS 2
>
> -#define TPS68470_WIN_MFD_CELL_COUNT 3
> +#define TPS68470_WIN_MFD_CELL_COUNT 4
>
> static const struct mfd_cell tps68470_cros[] = {
> { .name = "tps68470-gpio" },
> @@ -193,7 +193,8 @@ static int skl_int3472_tps68470_probe(struct i2c_client *client)
> cells[1].name = "tps68470-regulator";
> cells[1].platform_data = (void *)board_data->tps68470_regulator_pdata;
> cells[1].pdata_size = sizeof(struct tps68470_regulator_platform_data);
> - cells[2].name = "tps68470-gpio";
> + cells[2].name = "tps68470-led";
> + cells[3].name = "tps68470-gpio";
>
> for (i = 0; i < board_data->n_gpiod_lookups; i++)
> gpiod_add_lookup_table(board_data->tps68470_gpio_lookup_tables[i]);
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 4/4] platform: x86: int3472: Register a LED lookup table entry for the privacy LED
2023-03-10 9:56 ` [PATCH v2 4/4] platform: x86: int3472: Register a LED lookup table entry for the privacy LED Kate Hsuan
@ 2023-03-10 11:23 ` Dan Scally
0 siblings, 0 replies; 11+ messages in thread
From: Dan Scally @ 2023-03-10 11:23 UTC (permalink / raw)
To: Kate Hsuan, Pavel Machek, Lee Jones, linux-leds,
platform-driver-x86, Hans de Goede, Daniel Scally, Mark Gross
Hi Kate
On 10/03/2023 09:56, Kate Hsuan wrote:
> From: Hans de Goede <hdegoede@redhat.com>
>
> All currently known models using the tps68470 PMIC have a privacy LED for
> the back sensor (first ACPI consumer dev of the PMIC) connected to the
> ileda output of the PMIC.
>
> Add a LED lookup table entry for this, so that the v4l2-core code turns on
> the LED when streaming from the sensor.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/platform/x86/intel/int3472/tps68470.c | 30 +++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/int3472/tps68470.c b/drivers/platform/x86/intel/int3472/tps68470.c
> index 82ef022f8916..a53de9cd0540 100644
> --- a/drivers/platform/x86/intel/int3472/tps68470.c
> +++ b/drivers/platform/x86/intel/int3472/tps68470.c
> @@ -140,11 +140,18 @@ skl_int3472_fill_clk_pdata(struct device *dev, struct tps68470_clk_platform_data
> return n_consumers;
> }
>
> +static void skl_int3472_tps68470_unregister_led_lookup(void *led_lookup)
> +{
> + led_remove_lookup(led_lookup);
> +}
> +
> static int skl_int3472_tps68470_probe(struct i2c_client *client)
> {
> - struct acpi_device *adev = ACPI_COMPANION(&client->dev);
> + struct device *dev = &client->dev;
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> const struct int3472_tps68470_board_data *board_data;
> struct tps68470_clk_platform_data *clk_pdata;
> + struct led_lookup_data *led_lookup;
> struct mfd_cell *cells;
> struct regmap *regmap;
> int n_consumers;
> @@ -177,6 +184,25 @@ static int skl_int3472_tps68470_probe(struct i2c_client *client)
> if (!board_data)
> return dev_err_probe(&client->dev, -ENODEV, "No board-data found for this model\n");
>
> + /* Add a LED lookup table entry for the privacy LED */
> + led_lookup = devm_kzalloc(&client->dev, sizeof(*led_lookup), GFP_KERNEL);
> + if (!led_lookup)
> + return -ENOMEM;
> +
> + ret = skl_int3472_get_sensor_adev_and_name(&client->dev, NULL, &led_lookup->dev_id);
> + if (ret)
> + return ret;
> +
> + led_lookup->provider = "tps68470-ileda";
> + led_lookup->con_id = "privacy-led";
> + led_add_lookup(led_lookup);
> +
I'm not so sure about this one, because whilst the discrete type INT3472's only have a single
dependent, on lots of platforms the TPS68470 type has 2. skl_int3472_get_sensor_adev_and_name() will
just get those attributes of the first dependent; that conceivably might not be the ov5693. If it
returns the ov7251's attributes this would assign the wrong LED to it, as that one needs the IR LED
to function.
I think that this functionality probably needs to be added to the board data returned by
int3472_tps68470_get_board_data(), just as the GPIO lookups are.
> + ret = devm_add_action_or_reset(&client->dev,.
> + skl_int3472_tps68470_unregister_led_lookup,
> + led_lookup);
> + if (ret)
> + return ret;
> +
> cells = kcalloc(TPS68470_WIN_MFD_CELL_COUNT, sizeof(*cells), GFP_KERNEL);
> if (!cells)
> return -ENOMEM;
> @@ -259,4 +285,4 @@ module_i2c_driver(int3472_tps68470);
> MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI TPS68470 Device Driver");
> MODULE_AUTHOR("Daniel Scally <djrscally@gmail.com>");
> MODULE_LICENSE("GPL v2");
> -MODULE_SOFTDEP("pre: clk-tps68470 tps68470-regulator");
> +MODULE_SOFTDEP("pre: clk-tps68470 tps68470-regulator leds-tps68470");
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/4] leds: tps68470: Add LED control for tps68470
2023-03-10 10:43 ` Dan Scally
@ 2023-03-13 3:21 ` Kate Hsuan
0 siblings, 0 replies; 11+ messages in thread
From: Kate Hsuan @ 2023-03-13 3:21 UTC (permalink / raw)
To: Dan Scally
Cc: Pavel Machek, Lee Jones, linux-leds, platform-driver-x86,
Hans de Goede, Daniel Scally, Mark Gross
Hi,
On Fri, Mar 10, 2023 at 6:43 PM Dan Scally <dan.scally@ideasonboard.com> wrote:
>
> Hi Kate - thanks for the v2
>
> On 10/03/2023 09:56, Kate Hsuan wrote:
> > There are two LED controllers, LEDA indicator LED and LEDB flash LED for
> > tps68470. LEDA can be enabled by setting TPS68470_ILEDCTL_ENA. Moreover,
> > tps68470 provides four levels of power status for LEDB. If the
> > properties called "ti,ledb-current" can be found, the current will be
> > set according to the property values. These two LEDs can be controlled
> > through the LED class of sysfs (tps68470-leda and tps68470-ledb).
> >
> > Signed-off-by: Kate Hsuan <hpa@redhat.com>
> > ---
> > drivers/leds/Kconfig | 12 +++
> > drivers/leds/Makefile | 1 +
> > drivers/leds/leds-tps68470.c | 182 +++++++++++++++++++++++++++++++++++
> > 3 files changed, 195 insertions(+)
> > create mode 100644 drivers/leds/leds-tps68470.c
> >
> > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> > index 9dbce09eabac..fd26036b3c61 100644
> > --- a/drivers/leds/Kconfig
> > +++ b/drivers/leds/Kconfig
> > @@ -827,6 +827,18 @@ config LEDS_TPS6105X
> > It is a single boost converter primarily for white LEDs and
> > audio amplifiers.
> >
> > +config LEDS_TPS68470
> > + tristate "LED support for TI TPS68470"
> > + depends on LEDS_CLASS
> > + depends on INTEL_SKL_INT3472
> > + help
> > + This driver supports TPS68470 PMIC with LED chip.
> > + It provide two LED controllers, including an indicator LED
> > + and a flash LED.
>
> s/provide/provides. Also maybe "It provides two LED controllers, with the ability to drive 2
I'll revise the description in the v3 patch.
> indicator LEDs and 2 flash LEDs". I actually got the WLED part working now finally so I'll send
> patches on top of this series if that's ok?
Sounds good! That would be great! Thank you
>
> > +
> > + To compile this driver as a module, choose M and it will be
> > + called leds-tps68470
> > +
> > config LEDS_IP30
> > tristate "LED support for SGI Octane machines"
> > depends on LEDS_CLASS
> > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> > index d30395d11fd8..b284bc0daa98 100644
> > --- a/drivers/leds/Makefile
> > +++ b/drivers/leds/Makefile
> > @@ -84,6 +84,7 @@ obj-$(CONFIG_LEDS_TURRIS_OMNIA) += leds-turris-omnia.o
> > obj-$(CONFIG_LEDS_WM831X_STATUS) += leds-wm831x-status.o
> > obj-$(CONFIG_LEDS_WM8350) += leds-wm8350.o
> > obj-$(CONFIG_LEDS_WRAP) += leds-wrap.o
> > +obj-$(CONFIG_LEDS_TPS68470) += leds-tps68470.o
> >
> > # LED SPI Drivers
> > obj-$(CONFIG_LEDS_CR0014114) += leds-cr0014114.o
> > diff --git a/drivers/leds/leds-tps68470.c b/drivers/leds/leds-tps68470.c
> > new file mode 100644
> > index 000000000000..98bb56153690
> > --- /dev/null
> > +++ b/drivers/leds/leds-tps68470.c
> > @@ -0,0 +1,182 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * LED driver for TPS68470 PMIC
> > + *
> > + * Copyright (C) 2023 Red Hat
> > + *
> > + * Authors:
> > + * Kate Hsuan <hpa@redhat.com>
> > + */
> > +
> > +#include <linux/gpio/driver.h>
>
> Not needed I think?
removed
>
> > +#include <linux/mfd/tps68470.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/leds.h>
>
> Alphabetical order?
Okay.
> > +
> > +#define lcdev_to_led(led_cdev) \
> > + container_of(led_cdev, struct tps68470_led, lcdev)
> > +
> > +#define led_to_tps68470(led, index) \
> > + container_of(led, struct tps68470_device, leds[index])
> > +
> > +enum tps68470_led_ids {
> > + TPS68470_ILED_A,
> > + TPS68470_ILED_B,
> > + TPS68470_NUM_LEDS
> > +};
> > +
> > +static const char *tps68470_led_names[] = {
> > + [TPS68470_ILED_A] = "tps68470-iled_a",
> > + [TPS68470_ILED_B] = "tps68470-iled_b",
> > +};
> > +
> > +struct tps68470_led {
> > + unsigned int led_id;
> > + struct led_classdev lcdev;
> > +};
> > +
> > +struct tps68470_device {
> > + struct device *dev;
> > + struct regmap *regmap;
> > + struct tps68470_led leds[TPS68470_NUM_LEDS];
> > +};
> > +
> > +enum ctrlb_current {
> > + CTRLB_2MA = 0,
> > + CTRLB_4MA = 1,
> > + CTRLB_8MA = 2,
> > + CTRLB_16MA = 3,
> > +};
> > +
> > +static int tps68470_brightness_set(struct led_classdev *led_cdev, enum led_brightness brightness)
> > +{
> > + struct tps68470_led *led = lcdev_to_led(led_cdev);
> > + struct tps68470_device *tps68470 = led_to_tps68470(led, led->led_id);
> > + struct regmap *regmap = tps68470->regmap
> This would work fine as is...but I would maybe add something like
>
> if (state > LED_ON)
> return -EINVAL;
>
> So that brightness values of > 1 aren't just silently accepted...or does the LED core already
> prevent that? If so it's fine.
I think it is unnecessary.
The LED framework already handles this. Since we already set
"max_brightness" for the device, the framework will check the
"brightness" value and make sure the value isn't greater than
"max_brightness".
>
> > +
> > + switch (led->led_id) {
> > + case TPS68470_ILED_A:
> > + return regmap_update_bits(regmap, TPS68470_REG_ILEDCTL, TPS68470_ILEDCTL_ENA,
> > + brightness ? TPS68470_ILEDCTL_ENA : 0);
> > + case TPS68470_ILED_B:
> > + return regmap_update_bits(regmap, TPS68470_REG_ILEDCTL, TPS68470_ILEDCTL_ENB,
> > + brightness ? TPS68470_ILEDCTL_ENB : 0);
> > + }
> > + return -EINVAL;
> > +}
> > +
> > +static enum led_brightness tps68470_brightness_get(struct led_classdev *led_cdev)
> > +{
> > + struct tps68470_led *led = lcdev_to_led(led_cdev);
> > + struct tps68470_device *tps68470 = led_to_tps68470(led, led->led_id);
> > + struct regmap *regmap = tps68470->regmap;
> > + int ret = 0;
> > + int value = 0;
> > +
> > + ret = regmap_read(regmap, TPS68470_REG_ILEDCTL, &value);
> > + if (ret)
> > + goto error;
>
> Just return dev_err_probe(led_cdev->dev, -EINVAL, "failed on reading register\n") here imo.
Okay.
> > +
> > + switch (led->led_id) {
> > + case TPS68470_ILED_A:
> > + value = value & TPS68470_ILEDCTL_ENA;
> > + break;
> > + case TPS68470_ILED_B:
> > + value = value & TPS68470_ILEDCTL_ENB;
> > + break;
> > + }
> > +
> > + return value ? LED_ON : LED_OFF;
> > +
> > +error:
> > + dev_err(led_cdev->dev, "Failed on reading register\n");
> > + return -EINVAL;
> > +}
> > +
> > +static int tps68470_leds_probe(struct platform_device *pdev)
> > +{
> > + int i = 0;
> > + int ret = 0;
> > + unsigned int curr;
> > + struct tps68470_device *tps68470;
> > + struct tps68470_led *led;
> > + struct led_classdev *lcdev;
> > +
> > + tps68470 = devm_kzalloc(&pdev->dev, sizeof(struct tps68470_device),
> > + GFP_KERNEL);
>
> No -ENOMEM check here?
>
I'll add a return value check here in the v3 patch.
> > + tps68470->dev = &pdev->dev;
> > + tps68470->regmap = dev_get_drvdata(pdev->dev.parent);
> > +
> > + for (i = 0; i < TPS68470_NUM_LEDS; i++) {
> > + led = &tps68470->leds[i];
> > + lcdev = &led->lcdev;
> > +
> > + led->led_id = i;
> > +
> > + lcdev->name = devm_kasprintf(tps68470->dev, GFP_KERNEL, "%s::%s",
> > + tps68470_led_names[i], LED_FUNCTION_INDICATOR);
> > + if (!lcdev->name)
> > + return -ENOMEM;
> > +
> > + lcdev->max_brightness = 1;
> > + lcdev->brightness = 0;
> > + lcdev->brightness_set_blocking = tps68470_brightness_set;
> > + lcdev->brightness_get = tps68470_brightness_get;
> > + lcdev->dev = &pdev->dev;
> > +
> > + ret = devm_led_classdev_register(tps68470->dev, lcdev);
> > + if (ret) {
> > + dev_err_probe(tps68470->dev, ret,
> > + "error registering led\n");
> > + goto err_exit;
> > + }
> > + }
> > +
> > + /* configure LEDB current if the properties can be got */
> > + if (!device_property_read_u32(&pdev->dev, "ti,ledb-current", &curr)) {
> > + switch (curr) {
> > + case 2:
> > + curr = CTRLB_2MA;
> > + break;
> > + case 4:
> > + curr = CTRLB_4MA;
> > + break;
> > + case 8:
> > + curr = CTRLB_8MA;
> > + break;
> > + case 16:
> > + curr = CTRLB_16MA;
> > + break;
> > + default:
> > + dev_err(&pdev->dev, "Invalid LEDB curr value: %d\n", curr);
> > + return -EINVAL;
>
> There's no jump to err_exit here...I think that this whole section should go above the registration
> of the LEDS...and probably also into its own function.
Okay.
>
> > + }
> > + ret = regmap_update_bits(tps68470->regmap, TPS68470_REG_ILEDCTL,
> > + TPS68470_ILEDCTL_CTRLB, curr);
> > + }
> > +
> > +err_exit:
> > + if (ret) {
> > + for (i = 0; i < TPS68470_NUM_LEDS; i++) {
> > + if (tps68470->leds[i].lcdev.name)
> > + devm_led_classdev_unregister(&pdev->dev,
> > + &tps68470->leds[i].lcdev);
> > + }
> > + }
> > +
> > + return ret;
> > +}
> > +static struct platform_driver tps68470_led_driver = {
> > + .driver = {
> > + .name = "tps68470-led",
> > + },
> > + .probe = tps68470_leds_probe,
> > +};
> > +
> > +module_platform_driver(tps68470_led_driver);
> > +
> > +MODULE_ALIAS("platform:tps68470-led");
> > +MODULE_DESCRIPTION("LED driver for TPS68470 PMIC");
> > +MODULE_LICENSE("GPL v2");
>
Thank you
--
BR,
Kate
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/4] leds: tps68470: Add LED control for tps68470
2023-03-10 9:56 ` [PATCH v2 3/4] leds: tps68470: Add LED control for tps68470 Kate Hsuan
2023-03-10 10:43 ` Dan Scally
@ 2023-03-20 14:23 ` Dan Scally
2023-03-21 9:36 ` Kate Hsuan
1 sibling, 1 reply; 11+ messages in thread
From: Dan Scally @ 2023-03-20 14:23 UTC (permalink / raw)
To: Kate Hsuan, Pavel Machek, Lee Jones, linux-leds,
platform-driver-x86, Hans de Goede, Daniel Scally, Mark Gross
Hi Kate - sorry just noticed one more thing...
On 10/03/2023 09:56, Kate Hsuan wrote:
> There are two LED controllers, LEDA indicator LED and LEDB flash LED for
> tps68470. LEDA can be enabled by setting TPS68470_ILEDCTL_ENA. Moreover,
> tps68470 provides four levels of power status for LEDB. If the
> properties called "ti,ledb-current" can be found, the current will be
> set according to the property values. These two LEDs can be controlled
> through the LED class of sysfs (tps68470-leda and tps68470-ledb).
>
> Signed-off-by: Kate Hsuan <hpa@redhat.com>
> ---
> drivers/leds/Kconfig | 12 +++
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-tps68470.c | 182 +++++++++++++++++++++++++++++++++++
> 3 files changed, 195 insertions(+)
> create mode 100644 drivers/leds/leds-tps68470.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 9dbce09eabac..fd26036b3c61 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -827,6 +827,18 @@ config LEDS_TPS6105X
> It is a single boost converter primarily for white LEDs and
> audio amplifiers.
>
> +config LEDS_TPS68470
> + tristate "LED support for TI TPS68470"
> + depends on LEDS_CLASS
> + depends on INTEL_SKL_INT3472
> + help
> + This driver supports TPS68470 PMIC with LED chip.
> + It provide two LED controllers, including an indicator LED
> + and a flash LED.
> +
> + To compile this driver as a module, choose M and it will be
> + called leds-tps68470
> +
> config LEDS_IP30
> tristate "LED support for SGI Octane machines"
> depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index d30395d11fd8..b284bc0daa98 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -84,6 +84,7 @@ obj-$(CONFIG_LEDS_TURRIS_OMNIA) += leds-turris-omnia.o
> obj-$(CONFIG_LEDS_WM831X_STATUS) += leds-wm831x-status.o
> obj-$(CONFIG_LEDS_WM8350) += leds-wm8350.o
> obj-$(CONFIG_LEDS_WRAP) += leds-wrap.o
> +obj-$(CONFIG_LEDS_TPS68470) += leds-tps68470.o
Alphabetical order here too please :)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/4] leds: tps68470: Add LED control for tps68470
2023-03-20 14:23 ` Dan Scally
@ 2023-03-21 9:36 ` Kate Hsuan
0 siblings, 0 replies; 11+ messages in thread
From: Kate Hsuan @ 2023-03-21 9:36 UTC (permalink / raw)
To: Dan Scally
Cc: Pavel Machek, Lee Jones, linux-leds, platform-driver-x86,
Hans de Goede, Daniel Scally, Mark Gross
On Mon, Mar 20, 2023 at 10:24 PM Dan Scally <dan.scally@ideasonboard.com> wrote:
>
> Hi Kate - sorry just noticed one more thing...
>
> On 10/03/2023 09:56, Kate Hsuan wrote:
> > There are two LED controllers, LEDA indicator LED and LEDB flash LED for
> > tps68470. LEDA can be enabled by setting TPS68470_ILEDCTL_ENA. Moreover,
> > tps68470 provides four levels of power status for LEDB. If the
> > properties called "ti,ledb-current" can be found, the current will be
> > set according to the property values. These two LEDs can be controlled
> > through the LED class of sysfs (tps68470-leda and tps68470-ledb).
> >
> > Signed-off-by: Kate Hsuan <hpa@redhat.com>
> > ---
> > drivers/leds/Kconfig | 12 +++
> > drivers/leds/Makefile | 1 +
> > drivers/leds/leds-tps68470.c | 182 +++++++++++++++++++++++++++++++++++
> > 3 files changed, 195 insertions(+)
> > create mode 100644 drivers/leds/leds-tps68470.c
> >
> > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> > index 9dbce09eabac..fd26036b3c61 100644
> > --- a/drivers/leds/Kconfig
> > +++ b/drivers/leds/Kconfig
> > @@ -827,6 +827,18 @@ config LEDS_TPS6105X
> > It is a single boost converter primarily for white LEDs and
> > audio amplifiers.
> >
> > +config LEDS_TPS68470
> > + tristate "LED support for TI TPS68470"
> > + depends on LEDS_CLASS
> > + depends on INTEL_SKL_INT3472
> > + help
> > + This driver supports TPS68470 PMIC with LED chip.
> > + It provide two LED controllers, including an indicator LED
> > + and a flash LED.
> > +
> > + To compile this driver as a module, choose M and it will be
> > + called leds-tps68470
> > +
> > config LEDS_IP30
> > tristate "LED support for SGI Octane machines"
> > depends on LEDS_CLASS
> > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> > index d30395d11fd8..b284bc0daa98 100644
> > --- a/drivers/leds/Makefile
> > +++ b/drivers/leds/Makefile
> > @@ -84,6 +84,7 @@ obj-$(CONFIG_LEDS_TURRIS_OMNIA) += leds-turris-omnia.o
> > obj-$(CONFIG_LEDS_WM831X_STATUS) += leds-wm831x-status.o
> > obj-$(CONFIG_LEDS_WM8350) += leds-wm8350.o
> > obj-$(CONFIG_LEDS_WRAP) += leds-wrap.o
> > +obj-$(CONFIG_LEDS_TPS68470) += leds-tps68470.o
>
>
> Alphabetical order here too please :)
>
Thank you. I updated this. :)
--
BR,
Kate
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-03-21 9:37 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-10 9:56 [PATCH v2 0/4] leds: tps68470: LED driver for TPS68470 Kate Hsuan
2023-03-10 9:56 ` [PATCH v2 1/4] platform: x86: int3472: Add MFD cell for tps68470 LED Kate Hsuan
2023-03-10 11:13 ` Dan Scally
2023-03-10 9:56 ` [PATCH v2 2/4] include: mfd: tps68470: Add masks for LEDA and LEDB Kate Hsuan
2023-03-10 9:56 ` [PATCH v2 3/4] leds: tps68470: Add LED control for tps68470 Kate Hsuan
2023-03-10 10:43 ` Dan Scally
2023-03-13 3:21 ` Kate Hsuan
2023-03-20 14:23 ` Dan Scally
2023-03-21 9:36 ` Kate Hsuan
2023-03-10 9:56 ` [PATCH v2 4/4] platform: x86: int3472: Register a LED lookup table entry for the privacy LED Kate Hsuan
2023-03-10 11:23 ` Dan Scally
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).