All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] leds: cpcap: new driver
@ 2017-03-05 17:22 Sebastian Reichel
       [not found] ` <20170305172234.24120-1-sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
  2017-03-06 22:11 ` Pavel Machek
  0 siblings, 2 replies; 17+ messages in thread
From: Sebastian Reichel @ 2017-03-05 17:22 UTC (permalink / raw)
  To: Sebastian Reichel, Tony Lindgren, Richard Purdie,
	Jacek Anaszewski, Pavel Machek
  Cc: Rob Herring, Mark Rutland, linux-leds, devicetree, linux-kernel

Motorola CPCAP is a PMIC found in multiple smartphones.
This driver adds support for the chip's LED controllers.
It has explicit support for all controllers used by the
Droid 4. Since no datasheets are available the other
available controllers are not supported until somebody
verified, that the register layout matches.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 .../devicetree/bindings/leds/cpcap-leds.txt        |  29 +++
 drivers/leds/Kconfig                               |   9 +
 drivers/leds/Makefile                              |   1 +
 drivers/leds/leds-cpcap.c                          | 248 +++++++++++++++++++++
 4 files changed, 287 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/cpcap-leds.txt
 create mode 100644 drivers/leds/leds-cpcap.c

diff --git a/Documentation/devicetree/bindings/leds/cpcap-leds.txt b/Documentation/devicetree/bindings/leds/cpcap-leds.txt
new file mode 100644
index 000000000000..d523f8c3c358
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/cpcap-leds.txt
@@ -0,0 +1,29 @@
+Motorola CPCAP PMIC LEDs
+------------------------
+
+This module is part of the CPCAP. For more details about the whole
+chip see Documentation/devicetree/bindings/mfd/motorola-cpcap.txt.
+
+Requires node properties:
+- compatible: should be one of
+   * "motorola,cpcap-led-mdl"		(Main Display Lighting)
+   * "motorola,cpcap-led-kl"		(Keyboard Lighting)
+   * "motorola,cpcap-led-adl"		(Aux Display Lighting)
+   * "motorola,cpcap-led-red"		(Red Triode)
+   * "motorola,cpcap-led-green"		(Green Triode)
+   * "motorola,cpcap-led-blue"		(Blue Triode)
+   * "motorola,cpcap-led-cf"		(Camera Flash)
+   * "motorola,cpcap-led-bt"		(Bluetooth)
+   * "motorola,cpcap-led-cp"		(Camera Privacy LED)
+- label: The label for this LED
+- vdd-supply: A phandle to the regulator powering the LED
+
+Example:
+
+&cpcap {
+	cpcap_led_red: red-led {
+		compatible = "motorola,cpcap-led-red";
+		label = "cpcap:red";
+		vdd-supply = <&sw5>;
+	};
+};
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 275f467956ee..043f02a4fe73 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -76,6 +76,15 @@ config LEDS_BCM6358
 	  This option enables support for LEDs connected to the BCM6358
 	  LED HW controller accessed via MMIO registers.
 
+config LEDS_CPCAP
+	tristate "LED Support for Motorola CPCAP"
+	depends on LEDS_CLASS
+	depends on MFD_CPCAP
+	depends on OF
+	help
+	  This option enables support for LEDs offered by Motorola's
+	  CPCAP PMIC.
+
 config LEDS_LM3530
 	tristate "LCD Backlight driver for LM3530"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 6b8273736478..333e84ce5d3b 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_LEDS_AAT1290)		+= leds-aat1290.o
 obj-$(CONFIG_LEDS_BCM6328)		+= leds-bcm6328.o
 obj-$(CONFIG_LEDS_BCM6358)		+= leds-bcm6358.o
 obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
+obj-$(CONFIG_LEDS_CPCAP)		+= leds-cpcap.o
 obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o
 obj-$(CONFIG_LEDS_LM3530)		+= leds-lm3530.o
 obj-$(CONFIG_LEDS_LM3533)		+= leds-lm3533.o
diff --git a/drivers/leds/leds-cpcap.c b/drivers/leds/leds-cpcap.c
new file mode 100644
index 000000000000..e2fe13aae2f1
--- /dev/null
+++ b/drivers/leds/leds-cpcap.c
@@ -0,0 +1,248 @@
+/*
+ * Copyright (c) 2017 Sebastian Reichel <sre@kernel.org>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#define DEBUG
+
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/mfd/motorola-cpcap.h>
+
+#define CPCAP_LED_NO_CURRENT 0x0001
+#define CPCAP_LED_OFF 0x0000
+
+struct cpcap_led_info {
+	u16 reg;
+	u16 mask;
+	u16 limit;
+	u16 init_mask;
+	u16 init_val;
+};
+
+static const struct cpcap_led_info cpcap_led_red = {
+	.reg	= CPCAP_REG_REDC,
+	.mask	= 0x03FF,
+	.limit	= 31,
+};
+
+static const struct cpcap_led_info cpcap_led_green = {
+	.reg	= CPCAP_REG_GREENC,
+	.mask	= 0x03FF,
+	.limit	= 31,
+};
+
+static const struct cpcap_led_info cpcap_led_blue = {
+	.reg	= CPCAP_REG_BLUEC,
+	.mask	= 0x03FF,
+	.limit	= 31,
+};
+
+/* aux display light */
+static const struct cpcap_led_info cpcap_led_adl = {
+	.reg		= CPCAP_REG_ADLC,
+	.mask		= 0x000F,
+	.limit		= 1,
+	.init_mask	= 0x7FFF,
+	.init_val	= 0x5FF0,
+};
+
+/* camera privacy led */
+static const struct cpcap_led_info cpcap_led_cp = {
+	.reg		= CPCAP_REG_CLEDC,
+	.mask		= 0x0007,
+	.limit		= 1,
+	.init_mask	= 0x03FF,
+	.init_val	= 0x0008,
+};
+
+/* unsupported led */
+static const struct cpcap_led_info cpcap_led_unsup = {
+	.reg		= 0x0000,
+};
+
+struct cpcap_led {
+	struct led_classdev led;
+	const struct cpcap_led_info *info;
+	struct device *dev;
+	struct regmap *regmap;
+	struct regulator *vdd;
+	bool powered;
+
+	u32 current_limit;
+};
+
+static u16 cpcap_led_val(u8 current_limit, u8 duty_cycle)
+{
+	current_limit &= 0x1f; /* 5 bit */
+	duty_cycle &= 0x0f; /* 4 bit */
+
+	return current_limit << 4 | duty_cycle;
+}
+
+static int cpcap_led_set_power(struct cpcap_led *led, bool status)
+{
+	int err;
+
+	if (status == led->powered)
+		return 0;
+
+	if (status)
+		err = regulator_enable(led->vdd);
+	else
+		err = regulator_disable(led->vdd);
+
+	if (err) {
+		dev_err(led->dev, "regulator failure: %d", err);
+		return err;
+	}
+
+	led->powered = status;
+
+	return 0;
+}
+
+static int cpcap_led_set(struct led_classdev *ledc, enum led_brightness value)
+{
+	struct cpcap_led *led = container_of(ledc, struct cpcap_led, led);
+	int brightness;
+	int err;
+
+	if (value > LED_OFF) {
+		err = cpcap_led_set_power(led, true);
+		if (err)
+			return err;
+	}
+
+	if (value == LED_OFF) {
+		/* Avoid HW issue by turning off current before duty cycle */
+		err = regmap_update_bits(led->regmap,
+			led->info->reg, led->info->mask, CPCAP_LED_NO_CURRENT);
+		if (err) {
+			dev_err(led->dev, "regmap failed: %d", err);
+			return err;
+		}
+
+		brightness = CPCAP_LED_OFF;
+	} else {
+		brightness = cpcap_led_val(value, 0x01);
+	}
+
+	dev_dbg(led->dev, "regmap: reg=%04x mask=%04x val=%04x",
+		led->info->reg, led->info->mask, brightness);
+
+	err = regmap_update_bits(led->regmap, led->info->reg, led->info->mask,
+		brightness);
+	if (err) {
+		dev_err(led->dev, "regmap failed: %d", err);
+		return err;
+	}
+
+	if (value == LED_OFF) {
+		err = cpcap_led_set_power(led, false);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id cpcap_led_of_match[] = {
+	{ .compatible = "motorola,cpcap-led-red", .data = &cpcap_led_red },
+	{ .compatible = "motorola,cpcap-led-green", .data = &cpcap_led_green },
+	{ .compatible = "motorola,cpcap-led-blue",  .data = &cpcap_led_blue },
+	{ .compatible = "motorola,cpcap-led-adl", .data = &cpcap_led_adl },
+	{ .compatible = "motorola,cpcap-led-cp", .data = &cpcap_led_cp },
+
+	{ .compatible = "motorola,cpcap-led-cf", .data = &cpcap_led_unsup },
+	{ .compatible = "motorola,cpcap-led-mdl", .data = &cpcap_led_unsup },
+	{ .compatible = "motorola,cpcap-led-kl", .data = &cpcap_led_unsup },
+	{ .compatible = "motorola,cpcap-led-bt", .data = &cpcap_led_unsup },
+	{},
+};
+MODULE_DEVICE_TABLE(of, cpcap_led_of_match);
+
+static int cpcap_led_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *match;
+	struct cpcap_led *led;
+	int err;
+
+	match = of_match_device(of_match_ptr(cpcap_led_of_match), &pdev->dev);
+	if (!match || !match->data)
+		return -EINVAL;
+
+	led = devm_kzalloc(&pdev->dev, sizeof(*led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, led);
+	led->info = match->data;
+	led->dev = &pdev->dev;
+
+	if (led->info->reg == 0x0000) {
+		dev_err(led->dev, "Unsupported LED");
+		return -ENODEV;
+	}
+
+	led->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!led->regmap)
+		return -ENODEV;
+
+	led->vdd = devm_regulator_get(&pdev->dev, "vdd");
+	if (IS_ERR(led->vdd)) {
+		err = PTR_ERR(led->vdd);
+		dev_err(led->dev, "Couldn't get regulator: %d", err);
+		return err;
+	}
+
+	err = device_property_read_string(&pdev->dev, "label", &led->led.name);
+	if (err) {
+		dev_err(led->dev, "Couldn't read led label: %d", err);
+		return err;
+	}
+
+	led->led.max_brightness = led->info->limit;
+
+	led->led.brightness_set_blocking = cpcap_led_set;
+	err = devm_led_classdev_register(&pdev->dev, &led->led);
+	if (err) {
+		dev_err(led->dev, "Couldn't register led: %d", err);
+		return err;
+	}
+
+	if (led->info->init_mask) {
+		err = regmap_update_bits(led->regmap, led->info->reg,
+			led->info->init_mask, led->info->init_val);
+		if (err) {
+			dev_err(led->dev, "regmap failed: %d", err);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static struct platform_driver cpcap_led_driver = {
+	.probe = cpcap_led_probe,
+	.driver = {
+		.name = "cpcap-led",
+		.of_match_table = cpcap_led_of_match,
+	},
+};
+module_platform_driver(cpcap_led_driver);
+
+MODULE_DESCRIPTION("CPCAP LED driver");
+MODULE_AUTHOR("Sebastian Reichel <sre@kernel.org>");
+MODULE_LICENSE("GPL");
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/2] ARM: dts: motorola-cpcap-mapphone: add LEDs
  2017-03-05 17:22 [PATCH 1/2] leds: cpcap: new driver Sebastian Reichel
@ 2017-03-05 17:22     ` Sebastian Reichel
  2017-03-06 22:11 ` Pavel Machek
  1 sibling, 0 replies; 17+ messages in thread
From: Sebastian Reichel @ 2017-03-05 17:22 UTC (permalink / raw)
  To: Sebastian Reichel, Tony Lindgren, Richard Purdie,
	Jacek Anaszewski, Pavel Machek
  Cc: Rob Herring, Mark Rutland, linux-leds-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi | 30 ++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi b/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
index 9970985e685b..e02982d69854 100644
--- a/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
+++ b/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
@@ -38,6 +38,36 @@
 
 			interrupts = <23 IRQ_TYPE_NONE>;
 		};
+
+		led_red: led-red {
+			compatible = "motorola,cpcap-led-red";
+			vdd-supply = <&sw5>;
+			label = "status-led:red";
+		};
+
+		led_green: led-green {
+			compatible = "motorola,cpcap-led-green";
+			vdd-supply = <&sw5>;
+			label = "status-led:green";
+		};
+
+		led_blue: led-blue {
+			compatible = "motorola,cpcap-led-blue";
+			vdd-supply = <&sw5>;
+			label = "status-led:blue";
+		};
+
+		led_adl: led-adl {
+			compatible = "motorola,cpcap-led-adl";
+			vdd-supply = <&sw5>;
+			label = "button-backlight";
+		};
+
+		led_cp: led-cp {
+			compatible = "motorola,cpcap-led-cp";
+			vdd-supply = <&sw5>;
+			label = "shift-key-light";
+		};
 	};
 };
 
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/2] ARM: dts: motorola-cpcap-mapphone: add LEDs
@ 2017-03-05 17:22     ` Sebastian Reichel
  0 siblings, 0 replies; 17+ messages in thread
From: Sebastian Reichel @ 2017-03-05 17:22 UTC (permalink / raw)
  To: Sebastian Reichel, Tony Lindgren, Richard Purdie,
	Jacek Anaszewski, Pavel Machek
  Cc: Rob Herring, Mark Rutland, linux-leds, devicetree, linux-kernel

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi | 30 ++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi b/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
index 9970985e685b..e02982d69854 100644
--- a/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
+++ b/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
@@ -38,6 +38,36 @@
 
 			interrupts = <23 IRQ_TYPE_NONE>;
 		};
+
+		led_red: led-red {
+			compatible = "motorola,cpcap-led-red";
+			vdd-supply = <&sw5>;
+			label = "status-led:red";
+		};
+
+		led_green: led-green {
+			compatible = "motorola,cpcap-led-green";
+			vdd-supply = <&sw5>;
+			label = "status-led:green";
+		};
+
+		led_blue: led-blue {
+			compatible = "motorola,cpcap-led-blue";
+			vdd-supply = <&sw5>;
+			label = "status-led:blue";
+		};
+
+		led_adl: led-adl {
+			compatible = "motorola,cpcap-led-adl";
+			vdd-supply = <&sw5>;
+			label = "button-backlight";
+		};
+
+		led_cp: led-cp {
+			compatible = "motorola,cpcap-led-cp";
+			vdd-supply = <&sw5>;
+			label = "shift-key-light";
+		};
 	};
 };
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] ARM: dts: motorola-cpcap-mapphone: add LEDs
  2017-03-05 17:22     ` Sebastian Reichel
  (?)
@ 2017-03-06 16:11     ` Tony Lindgren
  -1 siblings, 0 replies; 17+ messages in thread
From: Tony Lindgren @ 2017-03-06 16:11 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Richard Purdie, Jacek Anaszewski, Pavel Machek, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

* Sebastian Reichel <sre@kernel.org> [170305 09:24]:
> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> ---
>  arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi | 30 ++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi b/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
> index 9970985e685b..e02982d69854 100644
> --- a/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
> +++ b/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
> @@ -38,6 +38,36 @@
>  
>  			interrupts = <23 IRQ_TYPE_NONE>;
>  		};
> +
> +		led_red: led-red {
> +			compatible = "motorola,cpcap-led-red";
> +			vdd-supply = <&sw5>;
> +			label = "status-led:red";
> +		};
> +
> +		led_green: led-green {
> +			compatible = "motorola,cpcap-led-green";
> +			vdd-supply = <&sw5>;
> +			label = "status-led:green";
> +		};
> +
> +		led_blue: led-blue {
> +			compatible = "motorola,cpcap-led-blue";
> +			vdd-supply = <&sw5>;
> +			label = "status-led:blue";
> +		};
> +
> +		led_adl: led-adl {
> +			compatible = "motorola,cpcap-led-adl";
> +			vdd-supply = <&sw5>;
> +			label = "button-backlight";
> +		};
> +
> +		led_cp: led-cp {
> +			compatible = "motorola,cpcap-led-cp";
> +			vdd-supply = <&sw5>;
> +			label = "shift-key-light";
> +		};
>  	};
>  };

Applying into omap-for-v4.12/dt-droid4 thanks. I added minimal
description to the patch saying "Add LEDs".

Regards,

Tony

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] leds: cpcap: new driver
  2017-03-05 17:22 [PATCH 1/2] leds: cpcap: new driver Sebastian Reichel
@ 2017-03-06 21:41     ` Jacek Anaszewski
  2017-03-06 22:11 ` Pavel Machek
  1 sibling, 0 replies; 17+ messages in thread
From: Jacek Anaszewski @ 2017-03-06 21:41 UTC (permalink / raw)
  To: Sebastian Reichel, Tony Lindgren, Richard Purdie, Pavel Machek
  Cc: Rob Herring, Mark Rutland, linux-leds-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Hi Sebastian,

Thanks for the patch. I have few comments in the code below.

On 03/05/2017 06:22 PM, Sebastian Reichel wrote:
> Motorola CPCAP is a PMIC found in multiple smartphones.
> This driver adds support for the chip's LED controllers.
> It has explicit support for all controllers used by the
> Droid 4. Since no datasheets are available the other
> available controllers are not supported until somebody
> verified, that the register layout matches.
> 
> Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  .../devicetree/bindings/leds/cpcap-leds.txt        |  29 +++
>  drivers/leds/Kconfig                               |   9 +
>  drivers/leds/Makefile                              |   1 +
>  drivers/leds/leds-cpcap.c                          | 248 +++++++++++++++++++++
>  4 files changed, 287 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/cpcap-leds.txt
>  create mode 100644 drivers/leds/leds-cpcap.c
> 
> diff --git a/Documentation/devicetree/bindings/leds/cpcap-leds.txt b/Documentation/devicetree/bindings/leds/cpcap-leds.txt
> new file mode 100644
> index 000000000000..d523f8c3c358
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/cpcap-leds.txt
> @@ -0,0 +1,29 @@
> +Motorola CPCAP PMIC LEDs
> +------------------------
> +
> +This module is part of the CPCAP. For more details about the whole
> +chip see Documentation/devicetree/bindings/mfd/motorola-cpcap.txt.
> +
> +Requires node properties:
> +- compatible: should be one of
> +   * "motorola,cpcap-led-mdl"		(Main Display Lighting)
> +   * "motorola,cpcap-led-kl"		(Keyboard Lighting)
> +   * "motorola,cpcap-led-adl"		(Aux Display Lighting)
> +   * "motorola,cpcap-led-red"		(Red Triode)
> +   * "motorola,cpcap-led-green"		(Green Triode)
> +   * "motorola,cpcap-led-blue"		(Blue Triode)
> +   * "motorola,cpcap-led-cf"		(Camera Flash)
> +   * "motorola,cpcap-led-bt"		(Bluetooth)
> +   * "motorola,cpcap-led-cp"		(Camera Privacy LED)
> +- label: The label for this LED

Please change the property description to the following:

see Documentation/devicetree/bindings/leds/common.txt

> +- vdd-supply: A phandle to the regulator powering the LED
> +
> +Example:
> +
> +&cpcap {
> +	cpcap_led_red: red-led {
> +		compatible = "motorola,cpcap-led-red";
> +		label = "cpcap:red";
> +		vdd-supply = <&sw5>;
> +	};
> +};
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 275f467956ee..043f02a4fe73 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -76,6 +76,15 @@ config LEDS_BCM6358
>  	  This option enables support for LEDs connected to the BCM6358
>  	  LED HW controller accessed via MMIO registers.
>  
> +config LEDS_CPCAP
> +	tristate "LED Support for Motorola CPCAP"
> +	depends on LEDS_CLASS
> +	depends on MFD_CPCAP
> +	depends on OF
> +	help
> +	  This option enables support for LEDs offered by Motorola's
> +	  CPCAP PMIC.
> +
>  config LEDS_LM3530
>  	tristate "LCD Backlight driver for LM3530"
>  	depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 6b8273736478..333e84ce5d3b 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_LEDS_AAT1290)		+= leds-aat1290.o
>  obj-$(CONFIG_LEDS_BCM6328)		+= leds-bcm6328.o
>  obj-$(CONFIG_LEDS_BCM6358)		+= leds-bcm6358.o
>  obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
> +obj-$(CONFIG_LEDS_CPCAP)		+= leds-cpcap.o
>  obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o
>  obj-$(CONFIG_LEDS_LM3530)		+= leds-lm3530.o
>  obj-$(CONFIG_LEDS_LM3533)		+= leds-lm3533.o
> diff --git a/drivers/leds/leds-cpcap.c b/drivers/leds/leds-cpcap.c
> new file mode 100644
> index 000000000000..e2fe13aae2f1
> --- /dev/null
> +++ b/drivers/leds/leds-cpcap.c
> @@ -0,0 +1,248 @@
> +/*
> + * Copyright (c) 2017 Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#define DEBUG

This is stray debug macro I believe.

> +
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/mfd/motorola-cpcap.h>

Please sort these directives alphabetically. I.e. the last entry
should go after linux/leds.h.

> +
> +#define CPCAP_LED_NO_CURRENT 0x0001
> +#define CPCAP_LED_OFF 0x0000
> +
> +struct cpcap_led_info {
> +	u16 reg;
> +	u16 mask;
> +	u16 limit;
> +	u16 init_mask;
> +	u16 init_val;
> +};
> +
> +static const struct cpcap_led_info cpcap_led_red = {
> +	.reg	= CPCAP_REG_REDC,
> +	.mask	= 0x03FF,
> +	.limit	= 31,
> +};
> +
> +static const struct cpcap_led_info cpcap_led_green = {
> +	.reg	= CPCAP_REG_GREENC,
> +	.mask	= 0x03FF,
> +	.limit	= 31,
> +};
> +
> +static const struct cpcap_led_info cpcap_led_blue = {
> +	.reg	= CPCAP_REG_BLUEC,
> +	.mask	= 0x03FF,
> +	.limit	= 31,
> +};
> +
> +/* aux display light */
> +static const struct cpcap_led_info cpcap_led_adl = {
> +	.reg		= CPCAP_REG_ADLC,
> +	.mask		= 0x000F,
> +	.limit		= 1,
> +	.init_mask	= 0x7FFF,
> +	.init_val	= 0x5FF0,
> +};
> +
> +/* camera privacy led */
> +static const struct cpcap_led_info cpcap_led_cp = {
> +	.reg		= CPCAP_REG_CLEDC,
> +	.mask		= 0x0007,
> +	.limit		= 1,
> +	.init_mask	= 0x03FF,
> +	.init_val	= 0x0008,
> +};
> +
> +/* unsupported led */
> +static const struct cpcap_led_info cpcap_led_unsup = {
> +	.reg		= 0x0000,
> +};
> +
> +struct cpcap_led {
> +	struct led_classdev led;
> +	const struct cpcap_led_info *info;
> +	struct device *dev;
> +	struct regmap *regmap;
> +	struct regulator *vdd;
> +	bool powered;
> +
> +	u32 current_limit;
> +};
> +
> +static u16 cpcap_led_val(u8 current_limit, u8 duty_cycle)
> +{
> +	current_limit &= 0x1f; /* 5 bit */
> +	duty_cycle &= 0x0f; /* 4 bit */
> +
> +	return current_limit << 4 | duty_cycle;
> +}
> +
> +static int cpcap_led_set_power(struct cpcap_led *led, bool status)
> +{
> +	int err;
> +
> +	if (status == led->powered)
> +		return 0;
> +
> +	if (status)
> +		err = regulator_enable(led->vdd);
> +	else
> +		err = regulator_disable(led->vdd);
> +
> +	if (err) {
> +		dev_err(led->dev, "regulator failure: %d", err);
> +		return err;
> +	}
> +
> +	led->powered = status;
> +
> +	return 0;
> +}
> +
> +static int cpcap_led_set(struct led_classdev *ledc, enum led_brightness value)
> +{
> +	struct cpcap_led *led = container_of(ledc, struct cpcap_led, led);
> +	int brightness;
> +	int err;

You need mutex protection here so as not to leave the device in an
inconsistent state when concurrent access occurs.
The operation of brightness setting is not atomic here.

> +	if (value > LED_OFF) {
> +		err = cpcap_led_set_power(led, true);
> +		if (err)
> +			return err;
> +	}
> +
> +	if (value == LED_OFF) {
> +		/* Avoid HW issue by turning off current before duty cycle */
> +		err = regmap_update_bits(led->regmap,
> +			led->info->reg, led->info->mask, CPCAP_LED_NO_CURRENT);
> +		if (err) {
> +			dev_err(led->dev, "regmap failed: %d", err);
> +			return err;
> +		}
> +
> +		brightness = CPCAP_LED_OFF;
> +	} else {
> +		brightness = cpcap_led_val(value, 0x01);

Please add a macro for 0x01.

> +	}
> +
> +	dev_dbg(led->dev, "regmap: reg=%04x mask=%04x val=%04x",
> +		led->info->reg, led->info->mask, brightness);

Please remove this debug logging. You can always enable CONFIG_DEBUG_FS.

> +	err = regmap_update_bits(led->regmap, led->info->reg, led->info->mask,
> +		brightness);
> +	if (err) {
> +		dev_err(led->dev, "regmap failed: %d", err);
> +		return err;
> +	}
> +
> +	if (value == LED_OFF) {

Why can't we accomplish this in the second condition?

> +		err = cpcap_led_set_power(led, false);
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id cpcap_led_of_match[] = {
> +	{ .compatible = "motorola,cpcap-led-red", .data = &cpcap_led_red },
> +	{ .compatible = "motorola,cpcap-led-green", .data = &cpcap_led_green },
> +	{ .compatible = "motorola,cpcap-led-blue",  .data = &cpcap_led_blue },
> +	{ .compatible = "motorola,cpcap-led-adl", .data = &cpcap_led_adl },
> +	{ .compatible = "motorola,cpcap-led-cp", .data = &cpcap_led_cp },
> +
> +	{ .compatible = "motorola,cpcap-led-cf", .data = &cpcap_led_unsup },
> +	{ .compatible = "motorola,cpcap-led-mdl", .data = &cpcap_led_unsup },
> +	{ .compatible = "motorola,cpcap-led-kl", .data = &cpcap_led_unsup },
> +	{ .compatible = "motorola,cpcap-led-bt", .data = &cpcap_led_unsup },

Does it make sense to provide entries for unsupported LEDs?

> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, cpcap_led_of_match);
> +
> +static int cpcap_led_probe(struct platform_device *pdev)
> +{
> +	const struct of_device_id *match;
> +	struct cpcap_led *led;
> +	int err;
> +
> +	match = of_match_device(of_match_ptr(cpcap_led_of_match), &pdev->dev);
> +	if (!match || !match->data)
> +		return -EINVAL;
> +
> +	led = devm_kzalloc(&pdev->dev, sizeof(*led), GFP_KERNEL);
> +	if (!led)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, led);
> +	led->info = match->data;
> +	led->dev = &pdev->dev;
> +
> +	if (led->info->reg == 0x0000) {
> +		dev_err(led->dev, "Unsupported LED");
> +		return -ENODEV;
> +	}
> +
> +	led->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!led->regmap)
> +		return -ENODEV;
> +
> +	led->vdd = devm_regulator_get(&pdev->dev, "vdd");
> +	if (IS_ERR(led->vdd)) {
> +		err = PTR_ERR(led->vdd);
> +		dev_err(led->dev, "Couldn't get regulator: %d", err);
> +		return err;
> +	}
> +
> +	err = device_property_read_string(&pdev->dev, "label", &led->led.name);
> +	if (err) {
> +		dev_err(led->dev, "Couldn't read led label: %d", err);
> +		return err;
> +	}
> +
> +	led->led.max_brightness = led->info->limit;
> +
> +	led->led.brightness_set_blocking = cpcap_led_set;
> +	err = devm_led_classdev_register(&pdev->dev, &led->led);
> +	if (err) {
> +		dev_err(led->dev, "Couldn't register led: %d", err);
> +		return err;
> +	}
> +
> +	if (led->info->init_mask) {
> +		err = regmap_update_bits(led->regmap, led->info->reg,
> +			led->info->init_mask, led->info->init_val);
> +		if (err) {
> +			dev_err(led->dev, "regmap failed: %d", err);
> +			return err;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static struct platform_driver cpcap_led_driver = {
> +	.probe = cpcap_led_probe,
> +	.driver = {
> +		.name = "cpcap-led",
> +		.of_match_table = cpcap_led_of_match,
> +	},
> +};
> +module_platform_driver(cpcap_led_driver);
> +
> +MODULE_DESCRIPTION("CPCAP LED driver");
> +MODULE_AUTHOR("Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>");
> +MODULE_LICENSE("GPL");

Your copyright note claims "GPL v2".


-- 
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] leds: cpcap: new driver
@ 2017-03-06 21:41     ` Jacek Anaszewski
  0 siblings, 0 replies; 17+ messages in thread
From: Jacek Anaszewski @ 2017-03-06 21:41 UTC (permalink / raw)
  To: Sebastian Reichel, Tony Lindgren, Richard Purdie, Pavel Machek
  Cc: Rob Herring, Mark Rutland, linux-leds, devicetree, linux-kernel

Hi Sebastian,

Thanks for the patch. I have few comments in the code below.

On 03/05/2017 06:22 PM, Sebastian Reichel wrote:
> Motorola CPCAP is a PMIC found in multiple smartphones.
> This driver adds support for the chip's LED controllers.
> It has explicit support for all controllers used by the
> Droid 4. Since no datasheets are available the other
> available controllers are not supported until somebody
> verified, that the register layout matches.
> 
> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> ---
>  .../devicetree/bindings/leds/cpcap-leds.txt        |  29 +++
>  drivers/leds/Kconfig                               |   9 +
>  drivers/leds/Makefile                              |   1 +
>  drivers/leds/leds-cpcap.c                          | 248 +++++++++++++++++++++
>  4 files changed, 287 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/cpcap-leds.txt
>  create mode 100644 drivers/leds/leds-cpcap.c
> 
> diff --git a/Documentation/devicetree/bindings/leds/cpcap-leds.txt b/Documentation/devicetree/bindings/leds/cpcap-leds.txt
> new file mode 100644
> index 000000000000..d523f8c3c358
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/cpcap-leds.txt
> @@ -0,0 +1,29 @@
> +Motorola CPCAP PMIC LEDs
> +------------------------
> +
> +This module is part of the CPCAP. For more details about the whole
> +chip see Documentation/devicetree/bindings/mfd/motorola-cpcap.txt.
> +
> +Requires node properties:
> +- compatible: should be one of
> +   * "motorola,cpcap-led-mdl"		(Main Display Lighting)
> +   * "motorola,cpcap-led-kl"		(Keyboard Lighting)
> +   * "motorola,cpcap-led-adl"		(Aux Display Lighting)
> +   * "motorola,cpcap-led-red"		(Red Triode)
> +   * "motorola,cpcap-led-green"		(Green Triode)
> +   * "motorola,cpcap-led-blue"		(Blue Triode)
> +   * "motorola,cpcap-led-cf"		(Camera Flash)
> +   * "motorola,cpcap-led-bt"		(Bluetooth)
> +   * "motorola,cpcap-led-cp"		(Camera Privacy LED)
> +- label: The label for this LED

Please change the property description to the following:

see Documentation/devicetree/bindings/leds/common.txt

> +- vdd-supply: A phandle to the regulator powering the LED
> +
> +Example:
> +
> +&cpcap {
> +	cpcap_led_red: red-led {
> +		compatible = "motorola,cpcap-led-red";
> +		label = "cpcap:red";
> +		vdd-supply = <&sw5>;
> +	};
> +};
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 275f467956ee..043f02a4fe73 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -76,6 +76,15 @@ config LEDS_BCM6358
>  	  This option enables support for LEDs connected to the BCM6358
>  	  LED HW controller accessed via MMIO registers.
>  
> +config LEDS_CPCAP
> +	tristate "LED Support for Motorola CPCAP"
> +	depends on LEDS_CLASS
> +	depends on MFD_CPCAP
> +	depends on OF
> +	help
> +	  This option enables support for LEDs offered by Motorola's
> +	  CPCAP PMIC.
> +
>  config LEDS_LM3530
>  	tristate "LCD Backlight driver for LM3530"
>  	depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 6b8273736478..333e84ce5d3b 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_LEDS_AAT1290)		+= leds-aat1290.o
>  obj-$(CONFIG_LEDS_BCM6328)		+= leds-bcm6328.o
>  obj-$(CONFIG_LEDS_BCM6358)		+= leds-bcm6358.o
>  obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
> +obj-$(CONFIG_LEDS_CPCAP)		+= leds-cpcap.o
>  obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o
>  obj-$(CONFIG_LEDS_LM3530)		+= leds-lm3530.o
>  obj-$(CONFIG_LEDS_LM3533)		+= leds-lm3533.o
> diff --git a/drivers/leds/leds-cpcap.c b/drivers/leds/leds-cpcap.c
> new file mode 100644
> index 000000000000..e2fe13aae2f1
> --- /dev/null
> +++ b/drivers/leds/leds-cpcap.c
> @@ -0,0 +1,248 @@
> +/*
> + * Copyright (c) 2017 Sebastian Reichel <sre@kernel.org>
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#define DEBUG

This is stray debug macro I believe.

> +
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/mfd/motorola-cpcap.h>

Please sort these directives alphabetically. I.e. the last entry
should go after linux/leds.h.

> +
> +#define CPCAP_LED_NO_CURRENT 0x0001
> +#define CPCAP_LED_OFF 0x0000
> +
> +struct cpcap_led_info {
> +	u16 reg;
> +	u16 mask;
> +	u16 limit;
> +	u16 init_mask;
> +	u16 init_val;
> +};
> +
> +static const struct cpcap_led_info cpcap_led_red = {
> +	.reg	= CPCAP_REG_REDC,
> +	.mask	= 0x03FF,
> +	.limit	= 31,
> +};
> +
> +static const struct cpcap_led_info cpcap_led_green = {
> +	.reg	= CPCAP_REG_GREENC,
> +	.mask	= 0x03FF,
> +	.limit	= 31,
> +};
> +
> +static const struct cpcap_led_info cpcap_led_blue = {
> +	.reg	= CPCAP_REG_BLUEC,
> +	.mask	= 0x03FF,
> +	.limit	= 31,
> +};
> +
> +/* aux display light */
> +static const struct cpcap_led_info cpcap_led_adl = {
> +	.reg		= CPCAP_REG_ADLC,
> +	.mask		= 0x000F,
> +	.limit		= 1,
> +	.init_mask	= 0x7FFF,
> +	.init_val	= 0x5FF0,
> +};
> +
> +/* camera privacy led */
> +static const struct cpcap_led_info cpcap_led_cp = {
> +	.reg		= CPCAP_REG_CLEDC,
> +	.mask		= 0x0007,
> +	.limit		= 1,
> +	.init_mask	= 0x03FF,
> +	.init_val	= 0x0008,
> +};
> +
> +/* unsupported led */
> +static const struct cpcap_led_info cpcap_led_unsup = {
> +	.reg		= 0x0000,
> +};
> +
> +struct cpcap_led {
> +	struct led_classdev led;
> +	const struct cpcap_led_info *info;
> +	struct device *dev;
> +	struct regmap *regmap;
> +	struct regulator *vdd;
> +	bool powered;
> +
> +	u32 current_limit;
> +};
> +
> +static u16 cpcap_led_val(u8 current_limit, u8 duty_cycle)
> +{
> +	current_limit &= 0x1f; /* 5 bit */
> +	duty_cycle &= 0x0f; /* 4 bit */
> +
> +	return current_limit << 4 | duty_cycle;
> +}
> +
> +static int cpcap_led_set_power(struct cpcap_led *led, bool status)
> +{
> +	int err;
> +
> +	if (status == led->powered)
> +		return 0;
> +
> +	if (status)
> +		err = regulator_enable(led->vdd);
> +	else
> +		err = regulator_disable(led->vdd);
> +
> +	if (err) {
> +		dev_err(led->dev, "regulator failure: %d", err);
> +		return err;
> +	}
> +
> +	led->powered = status;
> +
> +	return 0;
> +}
> +
> +static int cpcap_led_set(struct led_classdev *ledc, enum led_brightness value)
> +{
> +	struct cpcap_led *led = container_of(ledc, struct cpcap_led, led);
> +	int brightness;
> +	int err;

You need mutex protection here so as not to leave the device in an
inconsistent state when concurrent access occurs.
The operation of brightness setting is not atomic here.

> +	if (value > LED_OFF) {
> +		err = cpcap_led_set_power(led, true);
> +		if (err)
> +			return err;
> +	}
> +
> +	if (value == LED_OFF) {
> +		/* Avoid HW issue by turning off current before duty cycle */
> +		err = regmap_update_bits(led->regmap,
> +			led->info->reg, led->info->mask, CPCAP_LED_NO_CURRENT);
> +		if (err) {
> +			dev_err(led->dev, "regmap failed: %d", err);
> +			return err;
> +		}
> +
> +		brightness = CPCAP_LED_OFF;
> +	} else {
> +		brightness = cpcap_led_val(value, 0x01);

Please add a macro for 0x01.

> +	}
> +
> +	dev_dbg(led->dev, "regmap: reg=%04x mask=%04x val=%04x",
> +		led->info->reg, led->info->mask, brightness);

Please remove this debug logging. You can always enable CONFIG_DEBUG_FS.

> +	err = regmap_update_bits(led->regmap, led->info->reg, led->info->mask,
> +		brightness);
> +	if (err) {
> +		dev_err(led->dev, "regmap failed: %d", err);
> +		return err;
> +	}
> +
> +	if (value == LED_OFF) {

Why can't we accomplish this in the second condition?

> +		err = cpcap_led_set_power(led, false);
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id cpcap_led_of_match[] = {
> +	{ .compatible = "motorola,cpcap-led-red", .data = &cpcap_led_red },
> +	{ .compatible = "motorola,cpcap-led-green", .data = &cpcap_led_green },
> +	{ .compatible = "motorola,cpcap-led-blue",  .data = &cpcap_led_blue },
> +	{ .compatible = "motorola,cpcap-led-adl", .data = &cpcap_led_adl },
> +	{ .compatible = "motorola,cpcap-led-cp", .data = &cpcap_led_cp },
> +
> +	{ .compatible = "motorola,cpcap-led-cf", .data = &cpcap_led_unsup },
> +	{ .compatible = "motorola,cpcap-led-mdl", .data = &cpcap_led_unsup },
> +	{ .compatible = "motorola,cpcap-led-kl", .data = &cpcap_led_unsup },
> +	{ .compatible = "motorola,cpcap-led-bt", .data = &cpcap_led_unsup },

Does it make sense to provide entries for unsupported LEDs?

> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, cpcap_led_of_match);
> +
> +static int cpcap_led_probe(struct platform_device *pdev)
> +{
> +	const struct of_device_id *match;
> +	struct cpcap_led *led;
> +	int err;
> +
> +	match = of_match_device(of_match_ptr(cpcap_led_of_match), &pdev->dev);
> +	if (!match || !match->data)
> +		return -EINVAL;
> +
> +	led = devm_kzalloc(&pdev->dev, sizeof(*led), GFP_KERNEL);
> +	if (!led)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, led);
> +	led->info = match->data;
> +	led->dev = &pdev->dev;
> +
> +	if (led->info->reg == 0x0000) {
> +		dev_err(led->dev, "Unsupported LED");
> +		return -ENODEV;
> +	}
> +
> +	led->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!led->regmap)
> +		return -ENODEV;
> +
> +	led->vdd = devm_regulator_get(&pdev->dev, "vdd");
> +	if (IS_ERR(led->vdd)) {
> +		err = PTR_ERR(led->vdd);
> +		dev_err(led->dev, "Couldn't get regulator: %d", err);
> +		return err;
> +	}
> +
> +	err = device_property_read_string(&pdev->dev, "label", &led->led.name);
> +	if (err) {
> +		dev_err(led->dev, "Couldn't read led label: %d", err);
> +		return err;
> +	}
> +
> +	led->led.max_brightness = led->info->limit;
> +
> +	led->led.brightness_set_blocking = cpcap_led_set;
> +	err = devm_led_classdev_register(&pdev->dev, &led->led);
> +	if (err) {
> +		dev_err(led->dev, "Couldn't register led: %d", err);
> +		return err;
> +	}
> +
> +	if (led->info->init_mask) {
> +		err = regmap_update_bits(led->regmap, led->info->reg,
> +			led->info->init_mask, led->info->init_val);
> +		if (err) {
> +			dev_err(led->dev, "regmap failed: %d", err);
> +			return err;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static struct platform_driver cpcap_led_driver = {
> +	.probe = cpcap_led_probe,
> +	.driver = {
> +		.name = "cpcap-led",
> +		.of_match_table = cpcap_led_of_match,
> +	},
> +};
> +module_platform_driver(cpcap_led_driver);
> +
> +MODULE_DESCRIPTION("CPCAP LED driver");
> +MODULE_AUTHOR("Sebastian Reichel <sre@kernel.org>");
> +MODULE_LICENSE("GPL");

Your copyright note claims "GPL v2".


-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] leds: cpcap: new driver
  2017-03-05 17:22 [PATCH 1/2] leds: cpcap: new driver Sebastian Reichel
       [not found] ` <20170305172234.24120-1-sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2017-03-06 22:11 ` Pavel Machek
  2017-03-06 23:38   ` Tony Lindgren
  2017-03-07 17:19     ` Sebastian Reichel
  1 sibling, 2 replies; 17+ messages in thread
From: Pavel Machek @ 2017-03-06 22:11 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Tony Lindgren, Richard Purdie, Jacek Anaszewski, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

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

Hi!

> Motorola CPCAP is a PMIC found in multiple smartphones.
> This driver adds support for the chip's LED controllers.
> It has explicit support for all controllers used by the
> Droid 4. Since no datasheets are available the other
> available controllers are not supported until somebody
> verified, that the register layout matches.

This of course leads me to two questions:

1) Where can I get Droid 4?

2) How well is it supported?

> index 000000000000..d523f8c3c358
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/cpcap-leds.txt
> @@ -0,0 +1,29 @@
> +Motorola CPCAP PMIC LEDs
> +------------------------
> +
> +This module is part of the CPCAP. For more details about the whole
> +chip see Documentation/devicetree/bindings/mfd/motorola-cpcap.txt.
> +
> +Requires node properties:
> +- compatible: should be one of
> +   * "motorola,cpcap-led-mdl"		(Main Display Lighting)
> +   * "motorola,cpcap-led-kl"		(Keyboard Lighting)
> +   * "motorola,cpcap-led-adl"		(Aux Display Lighting)
> +   * "motorola,cpcap-led-red"		(Red Triode)
> +   * "motorola,cpcap-led-green"		(Green Triode)
> +   * "motorola,cpcap-led-blue"		(Blue Triode)
> +   * "motorola,cpcap-led-cf"		(Camera Flash)
> +   * "motorola,cpcap-led-bt"		(Bluetooth)
> +   * "motorola,cpcap-led-cp"		(Camera Privacy LED)

BTW. Does the RGB controller support any kind of "patterns" similar to
what n900 can do?

> +&cpcap {
> +	cpcap_led_red: red-led {
> +		compatible = "motorola,cpcap-led-red";
> +		label = "cpcap:red";
> +		vdd-supply = <&sw5>;
> +	};
> +};

This should be copied to the device tree people.

> index 275f467956ee..043f02a4fe73 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -76,6 +76,15 @@ config LEDS_BCM6358
>  	  This option enables support for LEDs connected to the BCM6358
>  	  LED HW controller accessed via MMIO registers.
>  
> +config LEDS_CPCAP
> +	tristate "LED Support for Motorola CPCAP"
> +	depends on LEDS_CLASS
> +	depends on MFD_CPCAP
> +	depends on OF
> +	help
> +	  This option enables support for LEDs offered by Motorola's
> +	  CPCAP PMIC.
> +

Umm. That help explains exactly what I oculd tell from the name. Can
you spell out "CPCAP" and "PMIC"... and maybe mention that it is used
on the Droid 4 phone?

> +#define DEBUG

Remove for production?

> +	err = device_property_read_string(&pdev->dev, "label", &led->led.name);
> +	if (err) {
> +		dev_err(led->dev, "Couldn't read led label: %d", err);

s/led/LED/.

> +	if (err) {
> +		dev_err(led->dev, "Couldn't register led: %d", err);
> +		return err;

And here.

Acked-by: Pavel Machek <pavel@ucw.cz>
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] leds: cpcap: new driver
  2017-03-06 22:11 ` Pavel Machek
@ 2017-03-06 23:38   ` Tony Lindgren
       [not found]     ` <20170306233859.GQ20572-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
  2017-03-07 17:19     ` Sebastian Reichel
  1 sibling, 1 reply; 17+ messages in thread
From: Tony Lindgren @ 2017-03-06 23:38 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Sebastian Reichel, Richard Purdie, Jacek Anaszewski, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

Hi,

* Pavel Machek <pavel@ucw.cz> [170306 14:13]:
> > Motorola CPCAP is a PMIC found in multiple smartphones.
> > This driver adds support for the chip's LED controllers.
> > It has explicit support for all controllers used by the
> > Droid 4. Since no datasheets are available the other
> > available controllers are not supported until somebody
> > verified, that the register layout matches.
> 
> This of course leads me to two questions:
> 
> 1) Where can I get Droid 4?

There's plenty of them available used for a few tens of $/€
on ebay as it was a popular phone on Verizon network in the
US. It works fine on 3G networks outside the U.S, 4G probably
only works in the US.

> 2) How well is it supported?

Basic things like UART, MMC, eMMC, WLAN, I2C, SPI, HDMI,
clocks, PMIC and regulators work with lots of other dts
configured devices heading into tomorrow's Linux next in my
dts branch omap-for-v4.12/dt-droid4. LCD related patches have
been posted but are not yet in next.

Things that currently don't work are ADC, USB, charging,
audio and modems. The modems are on OHCI/EHCI, audio is on the
PMIC similar to the twl4030. I'm slowly working on a driver for
ADC/USB/charging with few more issues to sort out. I do have
ADC/USB/charging working with my current hacks though.

I've posted some notes at:

http://muru.com/linux/d4/

And Sebastian has information in his blog including a device
status matrix at:

http://elektranox.org/droid4/

Cheers,

Tony

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] leds: cpcap: new driver
  2017-03-06 23:38   ` Tony Lindgren
@ 2017-03-07 11:55         ` Pavel Machek
  0 siblings, 0 replies; 17+ messages in thread
From: Pavel Machek @ 2017-03-07 11:55 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Sebastian Reichel, Richard Purdie, Jacek Anaszewski, Rob Herring,
	Mark Rutland, linux-leds-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

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

Hi!

> * Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org> [170306 14:13]:
> > > Motorola CPCAP is a PMIC found in multiple smartphones.
> > > This driver adds support for the chip's LED controllers.
> > > It has explicit support for all controllers used by the
> > > Droid 4. Since no datasheets are available the other
> > > available controllers are not supported until somebody
> > > verified, that the register layout matches.
> > 
> > This of course leads me to two questions:
> > 
> > 1) Where can I get Droid 4?
> 
> There's plenty of them available used for a few tens of $/€
> on ebay as it was a popular phone on Verizon network in the
> US. It works fine on 3G networks outside the U.S, 4G probably
> only works in the US.

Ok, and more importantly, 2G should work outside U.S., too, good.

> > 2) How well is it supported?
> 
> Basic things like UART, MMC, eMMC, WLAN, I2C, SPI, HDMI,
> clocks, PMIC and regulators work with lots of other dts
> configured devices heading into tomorrow's Linux next in my
> dts branch omap-for-v4.12/dt-droid4. LCD related patches have
> been posted but are not yet in next.
> 
> Things that currently don't work are ADC, USB, charging,
> audio and modems. The modems are on OHCI/EHCI, audio is on the
> PMIC similar to the twl4030. I'm slowly working on a driver for
> ADC/USB/charging with few more issues to sort out. I do have
> ADC/USB/charging working with my current hacks though.

2 cores and more importantly 1GB of RAM. Nice. OTOH, I'm avoiding N950
due to its crazy "security" system and this one is even worse. RAM and
CPUs are nice, but phone calls are what is important for me. And
battery charging, I guess. Serial port on the USB connector _is_ nice,
OTOH. 

Anyway, good luck, and thanks for the pointers. I will get Droid 4 if
it is easy. But I guess I'll stick to N900 for now. I want something
that can do voice calls.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] leds: cpcap: new driver
@ 2017-03-07 11:55         ` Pavel Machek
  0 siblings, 0 replies; 17+ messages in thread
From: Pavel Machek @ 2017-03-07 11:55 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Sebastian Reichel, Richard Purdie, Jacek Anaszewski, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

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

Hi!

> * Pavel Machek <pavel@ucw.cz> [170306 14:13]:
> > > Motorola CPCAP is a PMIC found in multiple smartphones.
> > > This driver adds support for the chip's LED controllers.
> > > It has explicit support for all controllers used by the
> > > Droid 4. Since no datasheets are available the other
> > > available controllers are not supported until somebody
> > > verified, that the register layout matches.
> > 
> > This of course leads me to two questions:
> > 
> > 1) Where can I get Droid 4?
> 
> There's plenty of them available used for a few tens of $/€
> on ebay as it was a popular phone on Verizon network in the
> US. It works fine on 3G networks outside the U.S, 4G probably
> only works in the US.

Ok, and more importantly, 2G should work outside U.S., too, good.

> > 2) How well is it supported?
> 
> Basic things like UART, MMC, eMMC, WLAN, I2C, SPI, HDMI,
> clocks, PMIC and regulators work with lots of other dts
> configured devices heading into tomorrow's Linux next in my
> dts branch omap-for-v4.12/dt-droid4. LCD related patches have
> been posted but are not yet in next.
> 
> Things that currently don't work are ADC, USB, charging,
> audio and modems. The modems are on OHCI/EHCI, audio is on the
> PMIC similar to the twl4030. I'm slowly working on a driver for
> ADC/USB/charging with few more issues to sort out. I do have
> ADC/USB/charging working with my current hacks though.

2 cores and more importantly 1GB of RAM. Nice. OTOH, I'm avoiding N950
due to its crazy "security" system and this one is even worse. RAM and
CPUs are nice, but phone calls are what is important for me. And
battery charging, I guess. Serial port on the USB connector _is_ nice,
OTOH. 

Anyway, good luck, and thanks for the pointers. I will get Droid 4 if
it is easy. But I guess I'll stick to N900 for now. I want something
that can do voice calls.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] leds: cpcap: new driver
  2017-03-07 11:55         ` Pavel Machek
@ 2017-03-07 16:26           ` Tony Lindgren
  -1 siblings, 0 replies; 17+ messages in thread
From: Tony Lindgren @ 2017-03-07 16:26 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Sebastian Reichel, Richard Purdie, Jacek Anaszewski, Rob Herring,
	Mark Rutland, linux-leds-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

* Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org> [170307 03:57]:
> 2 cores and more importantly 1GB of RAM. Nice. OTOH, I'm avoiding N950
> due to its crazy "security" system and this one is even worse.

Yeah this security crap sucks big time in general. It just slows
down Linux development on usable devices making most of them throw
away bricks that nobody bothers to work on.

> Anyway, good luck, and thanks for the pointers. I will get Droid 4 if
> it is easy. But I guess I'll stick to N900 for now. I want something
> that can do voice calls.

Yeah.. For a security free solution, let's hope pyra will be able to
make voice calls too :)

Meanwhile, using droid 4 with a lapdock as a low power laptop is
probably few weeks away, which means I can use it :p

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] leds: cpcap: new driver
@ 2017-03-07 16:26           ` Tony Lindgren
  0 siblings, 0 replies; 17+ messages in thread
From: Tony Lindgren @ 2017-03-07 16:26 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Sebastian Reichel, Richard Purdie, Jacek Anaszewski, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

* Pavel Machek <pavel@ucw.cz> [170307 03:57]:
> 2 cores and more importantly 1GB of RAM. Nice. OTOH, I'm avoiding N950
> due to its crazy "security" system and this one is even worse.

Yeah this security crap sucks big time in general. It just slows
down Linux development on usable devices making most of them throw
away bricks that nobody bothers to work on.

> Anyway, good luck, and thanks for the pointers. I will get Droid 4 if
> it is easy. But I guess I'll stick to N900 for now. I want something
> that can do voice calls.

Yeah.. For a security free solution, let's hope pyra will be able to
make voice calls too :)

Meanwhile, using droid 4 with a lapdock as a low power laptop is
probably few weeks away, which means I can use it :p

Regards,

Tony

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] leds: cpcap: new driver
  2017-03-06 22:11 ` Pavel Machek
@ 2017-03-07 17:19     ` Sebastian Reichel
  2017-03-07 17:19     ` Sebastian Reichel
  1 sibling, 0 replies; 17+ messages in thread
From: Sebastian Reichel @ 2017-03-07 17:19 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Tony Lindgren, Richard Purdie, Jacek Anaszewski, Rob Herring,
	Mark Rutland, linux-leds-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

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

Hi,

On Mon, Mar 06, 2017 at 11:11:47PM +0100, Pavel Machek wrote:
> > Motorola CPCAP is a PMIC found in multiple smartphones.
> > This driver adds support for the chip's LED controllers.
> > It has explicit support for all controllers used by the
> > Droid 4. Since no datasheets are available the other
> > available controllers are not supported until somebody
> > verified, that the register layout matches.
> 
> This of course leads me to two questions:
> 
> 1) Where can I get Droid 4?

I got a used one on Ebay for 42€ incl. shipping & customs. The trick
is clicking the worldwide option, since they are pretty expensive in
EU (they only exist with US LTE modem, so they were not sold here
officially).

> 2) How well is it supported?

UART + WLAN works with mainline master branch. As written by Tony
we have a couple of patches ready for 4.12. Big open tasks are the
cameras and the modems. Cameras are handled via co-processor in the
stock system (that's about all I know about them so far) and modems are
connected via USB + GPIOs (and for the 2G/3G modem an additional UART).
LTE modem support seems simple (USB-CDC based), but does not work
in EU and 2G/3G looks like much work. Modem voice support will be
simpler than on N900, though (data goes directly to the audio codec).
Speaking about audio codec: I'm currently working on that.

> > index 000000000000..d523f8c3c358
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/cpcap-leds.txt
> > @@ -0,0 +1,29 @@
> > +Motorola CPCAP PMIC LEDs
> > +------------------------
> > +
> > +This module is part of the CPCAP. For more details about the whole
> > +chip see Documentation/devicetree/bindings/mfd/motorola-cpcap.txt.
> > +
> > +Requires node properties:
> > +- compatible: should be one of
> > +   * "motorola,cpcap-led-mdl"		(Main Display Lighting)
> > +   * "motorola,cpcap-led-kl"		(Keyboard Lighting)
> > +   * "motorola,cpcap-led-adl"		(Aux Display Lighting)
> > +   * "motorola,cpcap-led-red"		(Red Triode)
> > +   * "motorola,cpcap-led-green"		(Green Triode)
> > +   * "motorola,cpcap-led-blue"		(Blue Triode)
> > +   * "motorola,cpcap-led-cf"		(Camera Flash)
> > +   * "motorola,cpcap-led-bt"		(Bluetooth)
> > +   * "motorola,cpcap-led-cp"		(Camera Privacy LED)
> 
> BTW. Does the RGB controller support any kind of "patterns" similar
> to what n900 can do?

No. Motorola CPCAP has simple blink support for the RGB leds, though.
It can potentially save some CPU cycles, but I did not yet add support
for that. CPCAP also has a few more LED interfaces, that are unused
on Droid 4.

> > +&cpcap {
> > +	cpcap_led_red: red-led {
> > +		compatible = "motorola,cpcap-led-red";
> > +		label = "cpcap:red";
> > +		vdd-supply = <&sw5>;
> > +	};
> > +};
> 
> This should be copied to the device tree people.

They are already in CC.

> > index 275f467956ee..043f02a4fe73 100644
> > --- a/drivers/leds/Kconfig
> > +++ b/drivers/leds/Kconfig
> > @@ -76,6 +76,15 @@ config LEDS_BCM6358
> >  	  This option enables support for LEDs connected to the BCM6358
> >  	  LED HW controller accessed via MMIO registers.
> >  
> > +config LEDS_CPCAP
> > +	tristate "LED Support for Motorola CPCAP"
> > +	depends on LEDS_CLASS
> > +	depends on MFD_CPCAP
> > +	depends on OF
> > +	help
> > +	  This option enables support for LEDs offered by Motorola's
> > +	  CPCAP PMIC.
> > +
> 
> Umm. That help explains exactly what I oculd tell from the name. Can
> you spell out "CPCAP" and "PMIC"... and maybe mention that it is used
> on the Droid 4 phone?

PMIC = power management integrated circuit

https://en.wikipedia.org/wiki/Power_management_integrated_circuit

CPCAP = a chip similar to TWL6040.

CPCAP it used on multiple motorola smartphones, just like TWL6040
is often used as PMIC. Usually we do not add a list of boards using
some feature to config description (except when the option is only
useful for a single one).

> > +#define DEBUG
> 
> Remove for production?

Yes, thanks.

> > +	err = device_property_read_string(&pdev->dev, "label", &led->led.name);
> > +	if (err) {
> > +		dev_err(led->dev, "Couldn't read led label: %d", err);
> 
> s/led/LED/.

ok.

> 
> > +	if (err) {
> > +		dev_err(led->dev, "Couldn't register led: %d", err);
> > +		return err;
> 
> And here.

ok.

> Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>

Thanks.

-- Sebastian

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] leds: cpcap: new driver
@ 2017-03-07 17:19     ` Sebastian Reichel
  0 siblings, 0 replies; 17+ messages in thread
From: Sebastian Reichel @ 2017-03-07 17:19 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Tony Lindgren, Richard Purdie, Jacek Anaszewski, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

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

Hi,

On Mon, Mar 06, 2017 at 11:11:47PM +0100, Pavel Machek wrote:
> > Motorola CPCAP is a PMIC found in multiple smartphones.
> > This driver adds support for the chip's LED controllers.
> > It has explicit support for all controllers used by the
> > Droid 4. Since no datasheets are available the other
> > available controllers are not supported until somebody
> > verified, that the register layout matches.
> 
> This of course leads me to two questions:
> 
> 1) Where can I get Droid 4?

I got a used one on Ebay for 42€ incl. shipping & customs. The trick
is clicking the worldwide option, since they are pretty expensive in
EU (they only exist with US LTE modem, so they were not sold here
officially).

> 2) How well is it supported?

UART + WLAN works with mainline master branch. As written by Tony
we have a couple of patches ready for 4.12. Big open tasks are the
cameras and the modems. Cameras are handled via co-processor in the
stock system (that's about all I know about them so far) and modems are
connected via USB + GPIOs (and for the 2G/3G modem an additional UART).
LTE modem support seems simple (USB-CDC based), but does not work
in EU and 2G/3G looks like much work. Modem voice support will be
simpler than on N900, though (data goes directly to the audio codec).
Speaking about audio codec: I'm currently working on that.

> > index 000000000000..d523f8c3c358
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/cpcap-leds.txt
> > @@ -0,0 +1,29 @@
> > +Motorola CPCAP PMIC LEDs
> > +------------------------
> > +
> > +This module is part of the CPCAP. For more details about the whole
> > +chip see Documentation/devicetree/bindings/mfd/motorola-cpcap.txt.
> > +
> > +Requires node properties:
> > +- compatible: should be one of
> > +   * "motorola,cpcap-led-mdl"		(Main Display Lighting)
> > +   * "motorola,cpcap-led-kl"		(Keyboard Lighting)
> > +   * "motorola,cpcap-led-adl"		(Aux Display Lighting)
> > +   * "motorola,cpcap-led-red"		(Red Triode)
> > +   * "motorola,cpcap-led-green"		(Green Triode)
> > +   * "motorola,cpcap-led-blue"		(Blue Triode)
> > +   * "motorola,cpcap-led-cf"		(Camera Flash)
> > +   * "motorola,cpcap-led-bt"		(Bluetooth)
> > +   * "motorola,cpcap-led-cp"		(Camera Privacy LED)
> 
> BTW. Does the RGB controller support any kind of "patterns" similar
> to what n900 can do?

No. Motorola CPCAP has simple blink support for the RGB leds, though.
It can potentially save some CPU cycles, but I did not yet add support
for that. CPCAP also has a few more LED interfaces, that are unused
on Droid 4.

> > +&cpcap {
> > +	cpcap_led_red: red-led {
> > +		compatible = "motorola,cpcap-led-red";
> > +		label = "cpcap:red";
> > +		vdd-supply = <&sw5>;
> > +	};
> > +};
> 
> This should be copied to the device tree people.

They are already in CC.

> > index 275f467956ee..043f02a4fe73 100644
> > --- a/drivers/leds/Kconfig
> > +++ b/drivers/leds/Kconfig
> > @@ -76,6 +76,15 @@ config LEDS_BCM6358
> >  	  This option enables support for LEDs connected to the BCM6358
> >  	  LED HW controller accessed via MMIO registers.
> >  
> > +config LEDS_CPCAP
> > +	tristate "LED Support for Motorola CPCAP"
> > +	depends on LEDS_CLASS
> > +	depends on MFD_CPCAP
> > +	depends on OF
> > +	help
> > +	  This option enables support for LEDs offered by Motorola's
> > +	  CPCAP PMIC.
> > +
> 
> Umm. That help explains exactly what I oculd tell from the name. Can
> you spell out "CPCAP" and "PMIC"... and maybe mention that it is used
> on the Droid 4 phone?

PMIC = power management integrated circuit

https://en.wikipedia.org/wiki/Power_management_integrated_circuit

CPCAP = a chip similar to TWL6040.

CPCAP it used on multiple motorola smartphones, just like TWL6040
is often used as PMIC. Usually we do not add a list of boards using
some feature to config description (except when the option is only
useful for a single one).

> > +#define DEBUG
> 
> Remove for production?

Yes, thanks.

> > +	err = device_property_read_string(&pdev->dev, "label", &led->led.name);
> > +	if (err) {
> > +		dev_err(led->dev, "Couldn't read led label: %d", err);
> 
> s/led/LED/.

ok.

> 
> > +	if (err) {
> > +		dev_err(led->dev, "Couldn't register led: %d", err);
> > +		return err;
> 
> And here.

ok.

> Acked-by: Pavel Machek <pavel@ucw.cz>

Thanks.

-- Sebastian

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] leds: cpcap: new driver
  2017-03-07 17:19     ` Sebastian Reichel
@ 2017-04-11 20:19       ` Pavel Machek
  -1 siblings, 0 replies; 17+ messages in thread
From: Pavel Machek @ 2017-04-11 20:19 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Tony Lindgren, Richard Purdie, Jacek Anaszewski, Rob Herring,
	Mark Rutland, linux-leds-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

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

Hi!

> On Mon, Mar 06, 2017 at 11:11:47PM +0100, Pavel Machek wrote:
> > > Motorola CPCAP is a PMIC found in multiple smartphones.
> > > This driver adds support for the chip's LED controllers.
> > > It has explicit support for all controllers used by the
> > > Droid 4. Since no datasheets are available the other
> > > available controllers are not supported until somebody
> > > verified, that the register layout matches.
> > 
> > This of course leads me to two questions:
> > 
> > 1) Where can I get Droid 4?
> 
> I got a used one on Ebay for 42€ incl. shipping & customs. The trick
> is clicking the worldwide option, since they are pretty expensive in
> EU (they only exist with US LTE modem, so they were not sold here
> officially).
> 
> > 2) How well is it supported?
> 
> UART + WLAN works with mainline master branch. As written by Tony
> we have a couple of patches ready for 4.12. Big open tasks are the
> cameras and the modems. Cameras are handled via co-processor in the
> stock system (that's about all I know about them so far) and modems are
> connected via USB + GPIOs (and for the 2G/3G modem an additional UART).
> LTE modem support seems simple (USB-CDC based), but does not work
> in EU and 2G/3G looks like much work. Modem voice support will be
> simpler than on N900, though (data goes directly to the audio codec).
> Speaking about audio codec: I'm currently working on that.

Thanks for all the information. It looks like a nice and powerful
machine, but I'm looking for a phone, first. (I'd actually like
something smaller than n900, preferably 3 times smaller... but... as
long as it calls it will have to do :-) ).

Best regards,
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] leds: cpcap: new driver
@ 2017-04-11 20:19       ` Pavel Machek
  0 siblings, 0 replies; 17+ messages in thread
From: Pavel Machek @ 2017-04-11 20:19 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Tony Lindgren, Richard Purdie, Jacek Anaszewski, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

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

Hi!

> On Mon, Mar 06, 2017 at 11:11:47PM +0100, Pavel Machek wrote:
> > > Motorola CPCAP is a PMIC found in multiple smartphones.
> > > This driver adds support for the chip's LED controllers.
> > > It has explicit support for all controllers used by the
> > > Droid 4. Since no datasheets are available the other
> > > available controllers are not supported until somebody
> > > verified, that the register layout matches.
> > 
> > This of course leads me to two questions:
> > 
> > 1) Where can I get Droid 4?
> 
> I got a used one on Ebay for 42€ incl. shipping & customs. The trick
> is clicking the worldwide option, since they are pretty expensive in
> EU (they only exist with US LTE modem, so they were not sold here
> officially).
> 
> > 2) How well is it supported?
> 
> UART + WLAN works with mainline master branch. As written by Tony
> we have a couple of patches ready for 4.12. Big open tasks are the
> cameras and the modems. Cameras are handled via co-processor in the
> stock system (that's about all I know about them so far) and modems are
> connected via USB + GPIOs (and for the 2G/3G modem an additional UART).
> LTE modem support seems simple (USB-CDC based), but does not work
> in EU and 2G/3G looks like much work. Modem voice support will be
> simpler than on N900, though (data goes directly to the audio codec).
> Speaking about audio codec: I'm currently working on that.

Thanks for all the information. It looks like a nice and powerful
machine, but I'm looking for a phone, first. (I'd actually like
something smaller than n900, preferably 3 times smaller... but... as
long as it calls it will have to do :-) ).

Best regards,
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] leds: cpcap: new driver
  2017-03-07 16:26           ` Tony Lindgren
  (?)
@ 2017-04-11 20:20           ` Pavel Machek
  -1 siblings, 0 replies; 17+ messages in thread
From: Pavel Machek @ 2017-04-11 20:20 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Sebastian Reichel, Richard Purdie, Jacek Anaszewski, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

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

On Tue 2017-03-07 08:26:40, Tony Lindgren wrote:
> * Pavel Machek <pavel@ucw.cz> [170307 03:57]:
> > 2 cores and more importantly 1GB of RAM. Nice. OTOH, I'm avoiding N950
> > due to its crazy "security" system and this one is even worse.
> 
> Yeah this security crap sucks big time in general. It just slows
> down Linux development on usable devices making most of them throw
> away bricks that nobody bothers to work on.
> 
> > Anyway, good luck, and thanks for the pointers. I will get Droid 4 if
> > it is easy. But I guess I'll stick to N900 for now. I want something
> > that can do voice calls.
> 
> Yeah.. For a security free solution, let's hope pyra will be able to
> make voice calls too :)
> 
> Meanwhile, using droid 4 with a lapdock as a low power laptop is
> probably few weeks away, which means I can use it :p

Turning phones into laptops, nice ;-). I think I'll keep the thinkpad
for a while, but yes, interesting machine.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2017-04-11 20:20 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-05 17:22 [PATCH 1/2] leds: cpcap: new driver Sebastian Reichel
     [not found] ` <20170305172234.24120-1-sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2017-03-05 17:22   ` [PATCH 2/2] ARM: dts: motorola-cpcap-mapphone: add LEDs Sebastian Reichel
2017-03-05 17:22     ` Sebastian Reichel
2017-03-06 16:11     ` Tony Lindgren
2017-03-06 21:41   ` [PATCH 1/2] leds: cpcap: new driver Jacek Anaszewski
2017-03-06 21:41     ` Jacek Anaszewski
2017-03-06 22:11 ` Pavel Machek
2017-03-06 23:38   ` Tony Lindgren
     [not found]     ` <20170306233859.GQ20572-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
2017-03-07 11:55       ` Pavel Machek
2017-03-07 11:55         ` Pavel Machek
2017-03-07 16:26         ` Tony Lindgren
2017-03-07 16:26           ` Tony Lindgren
2017-04-11 20:20           ` Pavel Machek
2017-03-07 17:19   ` Sebastian Reichel
2017-03-07 17:19     ` Sebastian Reichel
2017-04-11 20:19     ` Pavel Machek
2017-04-11 20:19       ` Pavel Machek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.