Re: [PATCH v3 3/3] backlight: lm3630a: add firmware node support
Pavel On 5/1/19 3:26 AM, Pavel Machek wrote: > Hi! > >>> @@ -396,13 +506,20 @@ static int lm3630a_probe(struct i2c_client *client, >>> GFP_KERNEL); >>> if (pdata == NULL) >>> return -ENOMEM; >>> + >>> /* default values */ >>> - pdata->leda_ctrl = LM3630A_LEDA_ENABLE; >>> - pdata->ledb_ctrl = LM3630A_LEDB_ENABLE; >>> + pdata->leda_ctrl = LM3630A_LEDA_DISABLE; >>> + pdata->ledb_ctrl = LM3630A_LEDB_DISABLE; >> >> This is not needed since default is disabled and kzalloc will set these to 0 > > Let compiler do this kind of optimalizations. Code makes sense as-is. > Yes the code makes sense but it is unnecessary. Dan > Pavel > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dt-bindings: backlight: lm3630a: correct schema validation
On 5/20/19 8:30 AM, Daniel Thompson wrote:
> On Mon, May 20, 2019 at 08:14:03AM -0500, Rob Herring wrote:
>> On Mon, May 20, 2019 at 3:59 AM Brian Masney wrote:
>>>
>>> The '#address-cells' and '#size-cells' properties were not defined in
>>> the lm3630a bindings and would cause the following error when
>>> attempting to validate the examples against the schema:
>>>
>>> Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.example.dt.yaml:
>>> '#address-cells', '#size-cells' do not match any of the regexes:
>>> '^led@[01]$', 'pinctrl-[0-9]+'
>>>
>>> Correct this by adding those two properties.
>>>
>>> While we're here, move the ti,linear-mapping-mode property to the
>>> led@[01] child nodes to correct the following validation error:
>>>
>>> Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.example.dt.yaml:
>>> led@0: 'ti,linear-mapping-mode' does not match any of the regexes:
>>> 'pinctrl-[0-9]+'
>>>
>>> Fixes: 32fcb75c66a0 ("dt-bindings: backlight: Add lm3630a bindings")
>>> Signed-off-by: Brian Masney
>>> Reported-by: Rob Herring
>>> ---
>>> .../leds/backlight/lm3630a-backlight.yaml | 20 +--
>>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> Reviewed-by: Rob Herring
>
> Acked-by: Daniel Thompson
>
Acked-by: Dan Murphy
Re: [PATCH v3 3/3] backlight: lm3630a: add firmware node support
On 4/15/19 2:29 AM, Brian Masney wrote:
> Add fwnode support to the lm3630a driver and allow configuring
> the initial and maximum LED brightness on both LED banks independently.
> The two outputs can be controlled by bank A and B independently or
> bank A can control both outputs.
>
> If the platform data was not configured, then the driver defaults to
> enabling both banks. This patch changes the default value to disable
> both banks before parsing the firmware node so that just a single bank
> can be enabled if desired. There are no in-tree users of this driver.
>
> Driver was tested on a LG Nexus 5 (hammerhead) phone.
>
> Signed-off-by: Brian Masney
> ---
> Changes since v2
> - Separated out control banks and outputs via the reg and led-sources
> properties.
> - Use fwnode instead of of API
> - Disable both banks by default before calling lm3630a_parse_node()
> - Add lots of error handling
> - Check for duplicate source / bank definitions
> - Remove extra ;
> - Make probe() method fail if fwnode parsing fails.
>
> drivers/video/backlight/lm3630a_bl.c | 128 ++-
> 1 file changed, 126 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/backlight/lm3630a_bl.c
> b/drivers/video/backlight/lm3630a_bl.c
> index ef2553f452ca..15922da9c05a 100644
> --- a/drivers/video/backlight/lm3630a_bl.c
> +++ b/drivers/video/backlight/lm3630a_bl.c
> @@ -364,6 +364,116 @@ static const struct regmap_config lm3630a_regmap = {
> .max_register = REG_MAX,
> };
>
> +/**
> + * lm3630a_parse_led_sources - Parse the optional led-sources fwnode
> property.
> + * @node: firmware node
> + * @default_bitmask: bitmask to return if the led-sources property was not
> + * specified
> + *
> + * Parses the optional led-sources firmware node and returns a bitmask that
> + * contains the outputs that are associated with the control bank. If the
> + * property is missing, then the value of of @default_bitmask will be
> returned.
> + */
> +static int lm3630a_parse_led_sources(struct fwnode_handle *node,
> + int default_bitmask)
> +{
> + int ret, num_sources, i;
> + u32 sources[2];
> +
> + num_sources = fwnode_property_read_u32_array(node, "led-sources", NULL,
> + 0);
> + if (num_sources < 0)
> + return default_bitmask;
> + else if (num_sources > ARRAY_SIZE(sources))
> + return -EINVAL;
> +
> + ret = fwnode_property_read_u32_array(node, "led-sources", sources,
> + num_sources);
> + if (ret)
> + return ret;
> +
> + ret = 0;
> + for (i = 0; i < num_sources; i++) {
> + if (sources[i] < 0 || sources[i] > 1)
> + return -EINVAL;
> +
> + ret |= BIT(sources[i]);
> + }
> +
> + return ret;
> +}
> +
> +static int lm3630a_parse_node(struct lm3630a_chip *pchip,
> + struct lm3630a_platform_data *pdata)
> +{
> + int seen = 0, led_sources, ret;
> + struct fwnode_handle *node;
> + u32 bank, val;
> + bool linear;
> +
> + device_for_each_child_node(pchip->dev, node) {
> + ret = fwnode_property_read_u32(node, "reg", &bank);
> + if (ret < 0)
> + return ret;
> +
> + if (bank < 0 || bank > 1)
> + return -EINVAL;
> +
> + if (seen & BIT(bank))
> + return -EINVAL;
Need line break for clarity.
> + seen |= BIT(bank);
> +
> + led_sources = lm3630a_parse_led_sources(node, BIT(bank));
> + if (led_sources < 0)
> + return led_sources;
> +
> + linear = fwnode_property_read_bool(node,
> +"ti,linear-mapping-mode");
> + if (bank == 0) {
> + if (!(led_sources & BIT(0)))
> + return -EINVAL;
> +
> + pdata->leda_ctrl = linear ?
> + LM3630A_LEDA_ENABLE_LINEAR :
> + LM3630A_LEDA_ENABLE;
> +
> + if (led_sources & BIT(1)) {
> + if (seen & BIT(1))
> + return -EINVAL;
Need line break for clarity.
> + seen |= BIT(1);
> +
> + pdata->ledb_ctrl = LM3630A_LEDB_ON_A;
> + }
> + } else {
> + if (led_sources & BIT(0) || !(led_sources & BIT(1)))
> + return -EINVAL;
> +
> + pdata->ledb_ctrl = linear ?
> + LM3630A_LEDB_ENABLE_LINEAR :
> + LM3630A_LEDB_ENABLE;
> + }
> +
> + ret = fwnode_property_read_u32(node, "default-brightness",
> +
Re: [PATCH v3 2/3] dt-bindings: backlight: add lm3630a bindings
Brian
On 4/15/19 2:29 AM, Brian Masney wrote:
> Add new backlight bindings for the TI LM3630A dual-string white LED.
>
> Signed-off-by: Brian Masney
> ---
> Rob: Since the common bindings aren't converted to the new JSON schema
> yet, I'm not sure how to do led-sources here. I would expect that we'd
> have the uint32-array on the common binding once it exists. I had to
> add it here to keep 'make dt_binding_check' happy. I left the
> description off though for that property since that'll come from common
> once its converted.
>
> Changes since v2:
> - Update description of max-brightness
> - Add description for reg
> - Correct typo: s/tranisiton/transition
> - Remove label from bindings since this isn't on backlight_properties
> - add reg to control banks
> - add additionalProperties
>
> .../leds/backlight/lm3630a-backlight.yaml | 124 ++
> 1 file changed, 124 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> new file mode 100644
> index ..cccd43c02732
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> @@ -0,0 +1,124 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/lm3630a-backlight.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI LM3630A High-Efficiency Dual-String White LED
> +
> +maintainers:
> + - Lee Jones
> + - Daniel Thompson
> + - Jingoo Han
> +
> +description: |
> + The LM3630A is a current-mode boost converter which supplies the power and
> + controls the current in up to two strings of 10 LEDs per string.
> + https://www.ti.com/product/LM3630A
> +
> +properties:
> + compatible:
> +const: ti,lm3630a
> +
> + reg:
> +description: The I2C address of the device
> +maxItems: 1
> +
> + ti,linear-mapping-mode:
> +description: |
> + Enable linear mapping mode. If disabled, then it will use exponential
> + mapping mode in which the ramp up/down appears to have a more uniform
> + transition to the human eye.
> +type: boolean
> +
> +required:
> + - compatible
> + - reg
> +
> +patternProperties:
> + "^led@[01]$":
> +type: object
> +description: |
> + Properties for a string of connected LEDs.
> +
> +properties:
> + reg:
> +description: Control Bank
> +maxItems: 1
> +minimum: 0
> +maximum: 1
> +
> + led-sources:
> +allOf:
> + - $ref: /schemas/types.yaml#/definitions/uint32-array
> + - minItems: 1
> +maxItems: 2
> +items:
> + minimum: 0
> + maximum: 1
> +
> + default-brightness:
> +description: Default brightness level on boot.
> +minimum: 0
> +maximum: 255
> +
> + max-brightness:
> +description: Maximum brightness that is allowed during runtime.
> +minimum: 0
> +maximum: 255
> +
> +required:
> + - reg
> +
> +additionalProperties: false
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +i2c {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +lm3630a_bl@38 {
> +compatible = "ti,lm3630a";
> +status = "ok";
> +reg = <0x38>;
> +
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +led@0 {
> +reg = <0>;
> +led-sources = <0 1>;
> +default-brightness = <200>;
> +max-brightness = <255>;
> +};
> +};
> +};
> + - |
I noticed we are missing "label". It is defined as optional and it is hard
coded in the driver
but wondering if there is a need to add it.
Dan
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/3] backlight: lm3630a: add firmware node support
Brian
On 4/16/19 6:53 PM, Brian Masney wrote:
> Add fwnode support to the lm3630a driver and optionally allow
> configuring the label, default brightness level, and maximum brightness
> level. The two outputs can be controlled by bank A and B independently
> or bank A can control both outputs.
>
> If the platform data was not configured, then the driver defaults to
> enabling both banks. This patch changes the default value to disable
> both banks before parsing the firmware node so that just a single bank
> can be enabled if desired. There are no in-tree users of this driver.
>
> Driver was tested on a LG Nexus 5 (hammerhead) phone.
>
> Signed-off-by: Brian Masney
> ---
> Changes since v3
> - Add support for label
> - Changed lm3630a_parse_node() to return -ENODEV if no nodes were found
> - Remove LM3630A_LED{A,B}_DISABLE
> - Add two additional newlines for code readability
> - Remove extra newline
>
> Changes since v2
> - Separated out control banks and outputs via the reg and led-sources
> properties.
> - Use fwnode instead of of API
> - Disable both banks by default before calling lm3630a_parse_node()
> - Add lots of error handling
> - Check for duplicate source / bank definitions
> - Remove extra ;
> - Make probe() method fail if fwnode parsing fails.
>
> drivers/video/backlight/lm3630a_bl.c | 147 ++-
> include/linux/platform_data/lm3630a_bl.h | 4 +
> 2 files changed, 146 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/video/backlight/lm3630a_bl.c
> b/drivers/video/backlight/lm3630a_bl.c
> index ef2553f452ca..6d3c54bfbb10 100644
> --- a/drivers/video/backlight/lm3630a_bl.c
> +++ b/drivers/video/backlight/lm3630a_bl.c
> @@ -329,15 +329,17 @@ static const struct backlight_ops lm3630a_bank_b_ops = {
>
> static int lm3630a_backlight_register(struct lm3630a_chip *pchip)
> {
> - struct backlight_properties props;
> struct lm3630a_platform_data *pdata = pchip->pdata;
> + struct backlight_properties props;
> + const char *label;
>
> props.type = BACKLIGHT_RAW;
> if (pdata->leda_ctrl != LM3630A_LEDA_DISABLE) {
> props.brightness = pdata->leda_init_brt;
> props.max_brightness = pdata->leda_max_brt;
> + label = pdata->leda_label ? pdata->leda_label : "lm3630a_leda";
> pchip->bleda =
> - devm_backlight_device_register(pchip->dev, "lm3630a_leda",
> + devm_backlight_device_register(pchip->dev, label,
> pchip->dev, pchip,
> &lm3630a_bank_a_ops, &props);
> if (IS_ERR(pchip->bleda))
> @@ -348,8 +350,9 @@ static int lm3630a_backlight_register(struct lm3630a_chip
> *pchip)
> (pdata->ledb_ctrl != LM3630A_LEDB_ON_A)) {
> props.brightness = pdata->ledb_init_brt;
> props.max_brightness = pdata->ledb_max_brt;
> + label = pdata->ledb_label ? pdata->ledb_label : "lm3630a_ledb";
> pchip->bledb =
> - devm_backlight_device_register(pchip->dev, "lm3630a_ledb",
> + devm_backlight_device_register(pchip->dev, label,
> pchip->dev, pchip,
> &lm3630a_bank_b_ops, &props);
> if (IS_ERR(pchip->bledb))
> @@ -364,6 +367,129 @@ static const struct regmap_config lm3630a_regmap = {
> .max_register = REG_MAX,
> };
>
> +/**
> + * lm3630a_parse_led_sources - Parse the optional led-sources fwnode
> property.
> + * @node: firmware node
> + * @default_bitmask: bitmask to return if the led-sources property was not
> + * specified
> + *
> + * Parses the optional led-sources firmware node and returns a bitmask that
> + * contains the outputs that are associated with the control bank. If the
> + * property is missing, then the value of of @default_bitmask will be
> returned.
> + */
Not sure if this was intentional but you documented this interface you added
but the others
you did not. If you need to explain what this is doing then the code may not
be clear or the DT
doc is not explicit enough.
> +static int lm3630a_parse_led_sources(struct fwnode_handle *node,
> + int default_bitmask)
> +{
> + int ret, num_sources, i;
> + u32 sources[2];
> +
> + num_sources = fwnode_property_read_u32_array(node, "led-sources", NULL,
> + 0);
> + if (num_sources < 0)
> + return default_bitmask;
> + else if (num_sources > ARRAY_SIZE(sources))
> + return -EINVAL;
> +
> + ret = fwnode_property_read_u32_array(node, "led-sources", sources,
> + num_sources);
> + if (ret)
> + return ret;
> +
> + ret = 0;
This is unneeded since fwnode should return 0 on success if it gets here
Re: [PATCH v5 3/3] backlight: lm3630a: add firmware node support
Brian
On 4/23/19 9:01 AM, Brian Masney wrote:
> On Tue, Apr 23, 2019 at 08:49:20AM -0500, Dan Murphy wrote:
>>> +static int lm3630a_parse_led_sources(struct fwnode_handle *node,
>>> +int default_led_sources)
>>> +{
>>> + u32 sources[LM3630A_NUM_SINKS];
>>> + int ret, num_sources, i;
>>> +
>>> + num_sources = fwnode_property_read_u32_array(node, "led-sources", NULL,
>>> +0);
>>> + if (num_sources < 0)
>>> + return default_led_sources;
>>> + else if (num_sources > ARRAY_SIZE(sources))
>>> + return -EINVAL;
>>> +
>>> + ret = fwnode_property_read_u32_array(node, "led-sources", sources,
>>> +num_sources);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + for (i = 0; i < num_sources; i++) {
>>> + if (sources[i] < LM3630A_SINK_0 || sources[i] > LM3630A_SINK_1)
>>> + return -EINVAL;
>>> +
>>> + ret |= BIT(sources[i]);
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int lm3630a_parse_bank(struct lm3630a_platform_data *pdata,
>>> + struct fwnode_handle *node, int *seen_led_sources)
>>
>> Why is seen_led_sources passed in here?
>> It is initialized on the stack in lm3630a_parse_node but the variable is
>> never referenced in that API.
>
> It's to see all of the led-sources that are configured across all of the
> banks. If it is just in lm3630a_parse_bank(), then it won't catch the
> following invalid configuration:
>
Ok I see what it is for now.
Not sure why it is declared as a pointer though.
Dan
> led@0 {
> reg = <0>;
> led-sources = <0 1>;
> label = "lcd-backlight";
> default-brightness = <200>;
> };
>
> led@1 {
> reg = <1>;
> default-brightness = <150>;
> };
>
> Brian
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 3/3] backlight: lm3630a: add firmware node support
Hello
On 4/18/19 10:11 AM, Brian Masney wrote:
> Add fwnode support to the lm3630a driver and optionally allow
> configuring the label, default brightness level, and maximum brightness
> level. The two outputs can be controlled by bank A and B independently
> or bank A can control both outputs.
>
> If the platform data was not configured, then the driver defaults to
> enabling both banks. This patch changes the default value to disable
> both banks before parsing the firmware node so that just a single bank
> can be enabled if desired. There are no in-tree users of this driver.
>
> Driver was tested on a LG Nexus 5 (hammerhead) phone.
>
> Signed-off-by: Brian Masney
> ---
> Changes since v4
> - Added new function lm3630a_parse_bank()
> - Renamed seen variable to seen_led_sources
> - Use the bitmask returned from lm3630a_parse_led_sources() to compare
> against the seen_led_sources rather than just the control bank. This
> eliminated another if statement that was previously present that
> checked to see if control bank A should control both sinks.
> - #define LM3630A_BANK_0, LM3630A_BANK_1, LM3630A_SINK_0,
> LM3630A_SINK_1, and LM3630A_NUM_SINKS and use where appropriate.
> - Changed all occurances of
> 'if (bank == 0) { BANK_A_WORK } else { BANK_B_WORK }' to
> 'if (bank) { BANK_B_WORK } else { BANK_A_WORK }'
> - Dropped unnecessary 'ret = 0' from lm3630a_parse_led_sources().
> - Changed 'if (ret < 0)' to 'if (ret)' in lm3630a_parse_node().
> - Dropped kerneldoc from lm3630a_parse_led_sources().
>
> Changes since v3
> - Add support for label
> - Changed lm3630a_parse_node() to return -ENODEV if no nodes were found
> - Remove LM3630A_LED{A,B}_DISABLE
> - Add two additional newlines for code readability
> - Remove extra newline
>
> Changes since v2
> - Separated out control banks and outputs via the reg and led-sources
> properties.
> - Use fwnode instead of of API
> - Disable both banks by default before calling lm3630a_parse_node()
> - Add lots of error handling
> - Check for duplicate source / bank definitions
> - Remove extra ;
> - Make probe() method fail if fwnode parsing fails.
>
> drivers/video/backlight/lm3630a_bl.c | 149 ++-
> include/linux/platform_data/lm3630a_bl.h | 4 +
> 2 files changed, 148 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/video/backlight/lm3630a_bl.c
> b/drivers/video/backlight/lm3630a_bl.c
> index ef2553f452ca..75d996490cf0 100644
> --- a/drivers/video/backlight/lm3630a_bl.c
> +++ b/drivers/video/backlight/lm3630a_bl.c
> @@ -35,6 +35,14 @@
> #define REG_MAX 0x50
>
> #define INT_DEBOUNCE_MSEC10
> +
> +#define LM3630A_BANK_0 0
> +#define LM3630A_BANK_1 1
> +
> +#define LM3630A_NUM_SINKS2
> +#define LM3630A_SINK_0 0
> +#define LM3630A_SINK_1 1
> +
> struct lm3630a_chip {
> struct device *dev;
> struct delayed_work work;
> @@ -329,15 +337,17 @@ static const struct backlight_ops lm3630a_bank_b_ops = {
>
> static int lm3630a_backlight_register(struct lm3630a_chip *pchip)
> {
> - struct backlight_properties props;
> struct lm3630a_platform_data *pdata = pchip->pdata;
> + struct backlight_properties props;
> + const char *label;
>
> props.type = BACKLIGHT_RAW;
> if (pdata->leda_ctrl != LM3630A_LEDA_DISABLE) {
> props.brightness = pdata->leda_init_brt;
> props.max_brightness = pdata->leda_max_brt;
> + label = pdata->leda_label ? pdata->leda_label : "lm3630a_leda";
> pchip->bleda =
> - devm_backlight_device_register(pchip->dev, "lm3630a_leda",
> + devm_backlight_device_register(pchip->dev, label,
> pchip->dev, pchip,
> &lm3630a_bank_a_ops, &props);
> if (IS_ERR(pchip->bleda))
> @@ -348,8 +358,9 @@ static int lm3630a_backlight_register(struct lm3630a_chip
> *pchip)
> (pdata->ledb_ctrl != LM3630A_LEDB_ON_A)) {
> props.brightness = pdata->ledb_init_brt;
> props.max_brightness = pdata->ledb_max_brt;
> + label = pdata->ledb_label ? pdata->ledb_label : "lm3630a_ledb";
> pchip->bledb =
> - devm_backlight_device_register(pchip->dev, "lm3630a_ledb",
> + devm_backlight_device_register(pchip->dev, label,
> pchip->dev, pchip,
> &lm3630a_bank_b_ops, &props);
> if (IS_ERR(pchip->bledb))
> @@ -364,6 +375,123 @@ static const struct regmap_config lm3630a_regmap = {
> .max_register = REG_MAX,
> };
>
> +static int lm3630a_parse_led_sources(struct fwnode_handle *node,
> + int default_led_sources)
> +{
> + u32 sources[LM3630A_NUM_SINKS];
> + int ret, num_sources, i;
> +
> +
Re: [PATCH v5 3/3] backlight: lm3630a: add firmware node support
Brian
On 4/23/19 11:00 AM, Brian Masney wrote:
> On Tue, Apr 23, 2019 at 10:31:41AM -0500, Dan Murphy wrote:
>> On 4/23/19 9:01 AM, Brian Masney wrote:
>>> On Tue, Apr 23, 2019 at 08:49:20AM -0500, Dan Murphy wrote:
>>>>> +static int lm3630a_parse_led_sources(struct fwnode_handle *node,
>>>>> + int default_led_sources)
>>>>> +{
>>>>> + u32 sources[LM3630A_NUM_SINKS];
>>>>> + int ret, num_sources, i;
>>>>> +
>>>>> + num_sources = fwnode_property_read_u32_array(node, "led-sources", NULL,
>>>>> + 0);
>>>>> + if (num_sources < 0)
>>>>> + return default_led_sources;
>>>>> + else if (num_sources > ARRAY_SIZE(sources))
>>>>> + return -EINVAL;
>>>>> +
>>>>> + ret = fwnode_property_read_u32_array(node, "led-sources", sources,
>>>>> + num_sources);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + for (i = 0; i < num_sources; i++) {
>>>>> + if (sources[i] < LM3630A_SINK_0 || sources[i] > LM3630A_SINK_1)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + ret |= BIT(sources[i]);
>>>>> + }
>>>>> +
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>> +static int lm3630a_parse_bank(struct lm3630a_platform_data *pdata,
>>>>> + struct fwnode_handle *node, int *seen_led_sources)
>>>>
>>>> Why is seen_led_sources passed in here?
>>>> It is initialized on the stack in lm3630a_parse_node but the variable is
>>>> never referenced in that API.
>>>
>>> It's to see all of the led-sources that are configured across all of the
>>> banks. If it is just in lm3630a_parse_bank(), then it won't catch the
>>> following invalid configuration:
>>>
>>
>> Ok I see what it is for now.
>>
>> Not sure why it is declared as a pointer though.
>
> It's so that lm3630a_parse_bank() can update that value with the
> led-sources that have been seen. Otherwise, the changes wouldn't make
> their way back out to the outer function.
>
OK. Thats another way to do it. I may have just done a return with the value.
Otherwise
Reviewed-by: Dan Murphy
> Brian
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] backlight: arcxcnn: add "arctic" vendor prefix
Hello
On 6/24/19 11:05 PM, Brian Dodge wrote:
The original patch adding this driver and DT bindings improperly
used "arc" as the vendor-prefix. This adds "arctic" which is the
proper prefix and retains "arc" to allow existing users of the
"arc" prefix to update to new kernels. There is at least one
(Samsung Chromebook Plus)
Signed-off-by: Brian Dodge
---
drivers/video/backlight/arcxcnn_bl.c | 35 +--
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/drivers/video/backlight/arcxcnn_bl.c
b/drivers/video/backlight/arcxcnn_bl.c
index 7b1c0a0..14c67f2 100644
--- a/drivers/video/backlight/arcxcnn_bl.c
+++ b/drivers/video/backlight/arcxcnn_bl.c
@@ -1,9 +1,9 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * Backlight driver for ArcticSand ARC_X_C_0N_0N Devices
+ * Backlight driver for pSemi (formerly ArcticSand) ARC_X_C_0N_0N Devices
*
- * Copyright 2016 ArcticSand, Inc.
- * Author : Brian Dodge
+ * Copyright 2016-2019 pSemi, Inc.
+ * Author : Brian Dodge
*/
#include
@@ -191,27 +191,40 @@ static void arcxcnn_parse_dt(struct arcxcnn *lp)
if (ret == 0)
lp->pdata->initial_brightness = prog_val;
- ret = of_property_read_u32(node, "arc,led-config-0", &prog_val);
+ ret = of_property_read_u32(node, "arctic,led-config-0", &prog_val);
+ if (ret)
+ ret = of_property_read_u32(node, "arc,led-config-0", &prog_val);
Can you add new lines between these and all below
if (ret == 0)
lp->pdata->led_config_0 = (u8)prog_val;
- ret = of_property_read_u32(node, "arc,led-config-1", &prog_val);
+ ret = of_property_read_u32(node, "arctic,led-config-1", &prog_val);
+ if (ret)
+ ret = of_property_read_u32(node, "arc,led-config-1", &prog_val);
if (ret == 0)
lp->pdata->led_config_1 = (u8)prog_val;
- ret = of_property_read_u32(node, "arc,dim-freq", &prog_val);
+ ret = of_property_read_u32(node, "arctic,dim-freq", &prog_val);
+ if (ret)
+ ret = of_property_read_u32(node, "arc,dim-freq", &prog_val);
if (ret == 0)
lp->pdata->dim_freq = (u8)prog_val;
- ret = of_property_read_u32(node, "arc,comp-config", &prog_val);
+ ret = of_property_read_u32(node, "arctic,comp-config", &prog_val);
+ if (ret)
+ ret = of_property_read_u32(node, "arc,comp-config", &prog_val);
if (ret == 0)
lp->pdata->comp_config = (u8)prog_val;
- ret = of_property_read_u32(node, "arc,filter-config", &prog_val);
+ ret = of_property_read_u32(node, "arctic,filter-config", &prog_val);
+ if (ret)
+ ret = of_property_read_u32(node,
+ "arc,filter-config", &prog_val);
if (ret == 0)
lp->pdata->filter_config = (u8)prog_val;
- ret = of_property_read_u32(node, "arc,trim-config", &prog_val);
+ ret = of_property_read_u32(node, "arctic,trim-config", &prog_val);
+ if (ret)
+ ret = of_property_read_u32(node, "arc,trim-config", &prog_val);
if (ret == 0)
lp->pdata->trim_config = (u8)prog_val;
@@ -381,6 +394,8 @@ static int arcxcnn_remove(struct i2c_client *cl)
}
static const struct of_device_id arcxcnn_dt_ids[] = {
+ { .compatible = "arctic,arc2c0608" },
+ /* here to remaim compatible with an older binding, do not use */
s/remaim/remain
{ .compatible = "arc,arc2c0608" },
{ }
};
@@ -404,5 +419,5 @@ static struct i2c_driver arcxcnn_driver = {
module_i2c_driver(arcxcnn_driver);
MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Brian Dodge ");
+MODULE_AUTHOR("Brian Dodge ");
MODULE_DESCRIPTION("ARCXCNN Backlight driver");
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] dt-bindings: backlight: fix vendor prefix for ArcticSand arcxcnn driver bindings
Hello
On 6/25/19 3:55 AM, Daniel Thompson wrote:
On Tue, Jun 25, 2019 at 12:05:28AM -0400, Brian Dodge wrote:
The vendor-prefixes.txt file properly refers to ArcticSand
as arctic but the driver bindings improperly abbreviated the
prefix to arc. This was a mistake in the original patch
Signed-off-by: Brian Dodge
---
.../bindings/leds/backlight/arcxcnn_bl.txt | 24 +-
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
index 230abde..9cf4c44 100644
--- a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
+++ b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
@@ -1,8 +1,12 @@
-Binding for ArcticSand arc2c0608 LED driver
+Binding for ArcticSand arc family LED drivers
Required properties:
-- compatible: should be "arc,arc2c0608"
-- reg: slave address
+- compatible: one of
+ "arctic,arc1c0608"
+ "arctic,arc2c0608"
+ "arctic,arc3c0845"
This is more a question for the DT folks than for Brian but...
AFAICT this patch is fixing the binding for the ArcticSand devices to
use the correct value from vendor-prefixes.yaml and has been previously
discussed here:
https://lkml.org/lkml/2018/9/25/726
Currently this patch series just updates the DT bindings but the
implementation also honours the old values (since there is a Chromebook
in the wild that uses the current bindings).
Hence I'm not clear whether the bindings should document the deprecated
options too (e.g. make it easier to find the bindings doc with git grep
and friends).
Daniel.
+
+- reg: slave address
Optional properties:
- default-brightness: brightness value on boot, value from: 0-4095
@@ -11,19 +15,19 @@ Optional properties:
- led-sources:List of enabled channels from 0 to 5.
See Documentation/devicetree/bindings/leds/common.txt
-- arc,led-config-0: setting for register ILED_CONFIG_0
-- arc,led-config-1:setting for register ILED_CONFIG_1
-- arc,dim-freq:PWM mode frequence setting (bits [3:0] used)
-- arc,comp-config: setting for register CONFIG_COMP
-- arc,filter-config: setting for register FILTER_CONFIG
-- arc,trim-config: setting for register IMAXTUNE
IMO I would prefer to keep these and mark them as deprecated since the
driver will still
honor these properties.
Maybe in a Optional Deprecated Properties section in the DT binding.
Dan
+- arctic,led-config-0: setting for register ILED_CONFIG_0
+- arctic,led-config-1: setting for register ILED_CONFIG_1
+- arctic,dim-freq: PWM mode frequence setting (bits [3:0] used)
+- arctic,comp-config: setting for register CONFIG_COMP
+- arctic,filter-config:setting for register FILTER_CONFIG
+- arctic,trim-config: setting for register IMAXTUNE
Note: Optional properties not specified will default to values in IC EPROM
Example:
arc2c0608@30 {
- compatible = "arc,arc2c0608";
+ compatible = "arctic,arc2c0608";
reg = <0x30>;
default-brightness = <500>;
label = "lcd-backlight";
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] dt-bindings: backlight: fix vendor prefix for ArcticSand arcxcnn driver bindings
On 6/26/19 9:56 AM, Pavel Machek wrote: On Wed 2019-06-26 11:56:14, Daniel Thompson wrote: On Tue, Jun 25, 2019 at 07:44:06AM -0400, Brian Dodge wrote: I would like to deprecate the old prefix in the future after communicating with all chip customers, which is why the old prefix is not documented in the new bindings. Deprecation is fine (by me at least) it's just that I'm not sure that removing the documentation for the deprecated bindings is the right way to do it. What is the prior art here? I believe we should keep the docs. I agree with Pavel on keeping the docs. Keep the doc but mark the properties as deprecated since they are not removed from the code Dan Pavel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] backlight: arcxcnn: add "arctic" vendor prefix
Hello
One other thing
On 6/26/19 6:42 AM, Dan Murphy wrote:
Hello
On 6/24/19 11:05 PM, Brian Dodge wrote:
The original patch adding this driver and DT bindings improperly
used "arc" as the vendor-prefix. This adds "arctic" which is the
proper prefix and retains "arc" to allow existing users of the
"arc" prefix to update to new kernels. There is at least one
(Samsung Chromebook Plus)
Signed-off-by: Brian Dodge
---
drivers/video/backlight/arcxcnn_bl.c | 35
+--
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/drivers/video/backlight/arcxcnn_bl.c
b/drivers/video/backlight/arcxcnn_bl.c
index 7b1c0a0..14c67f2 100644
--- a/drivers/video/backlight/arcxcnn_bl.c
+++ b/drivers/video/backlight/arcxcnn_bl.c
@@ -1,9 +1,9 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * Backlight driver for ArcticSand ARC_X_C_0N_0N Devices
+ * Backlight driver for pSemi (formerly ArcticSand) ARC_X_C_0N_0N
Devices
*
- * Copyright 2016 ArcticSand, Inc.
- * Author : Brian Dodge
+ * Copyright 2016-2019 pSemi, Inc.
+ * Author : Brian Dodge
*/
#include
@@ -191,27 +191,40 @@ static void arcxcnn_parse_dt(struct arcxcnn *lp)
if (ret == 0)
lp->pdata->initial_brightness = prog_val;
- ret = of_property_read_u32(node, "arc,led-config-0", &prog_val);
+ ret = of_property_read_u32(node, "arctic,led-config-0", &prog_val);
+ if (ret)
+ ret = of_property_read_u32(node, "arc,led-config-0",
&prog_val);
Can you add new lines between these and all below
Maybe add a dev_info here to indicate that this is being deprecated.
It will make the Chrome book console noisy but at least it won't go silent.
Dan
if (ret == 0)
lp->pdata->led_config_0 = (u8)prog_val;
- ret = of_property_read_u32(node, "arc,led-config-1", &prog_val);
+ ret = of_property_read_u32(node, "arctic,led-config-1", &prog_val);
+ if (ret)
+ ret = of_property_read_u32(node, "arc,led-config-1",
&prog_val);
if (ret == 0)
lp->pdata->led_config_1 = (u8)prog_val;
- ret = of_property_read_u32(node, "arc,dim-freq", &prog_val);
+ ret = of_property_read_u32(node, "arctic,dim-freq", &prog_val);
+ if (ret)
+ ret = of_property_read_u32(node, "arc,dim-freq", &prog_val);
if (ret == 0)
lp->pdata->dim_freq = (u8)prog_val;
- ret = of_property_read_u32(node, "arc,comp-config", &prog_val);
+ ret = of_property_read_u32(node, "arctic,comp-config", &prog_val);
+ if (ret)
+ ret = of_property_read_u32(node, "arc,comp-config", &prog_val);
if (ret == 0)
lp->pdata->comp_config = (u8)prog_val;
- ret = of_property_read_u32(node, "arc,filter-config", &prog_val);
+ ret = of_property_read_u32(node, "arctic,filter-config",
&prog_val);
+ if (ret)
+ ret = of_property_read_u32(node,
+ "arc,filter-config", &prog_val);
if (ret == 0)
lp->pdata->filter_config = (u8)prog_val;
- ret = of_property_read_u32(node, "arc,trim-config", &prog_val);
+ ret = of_property_read_u32(node, "arctic,trim-config", &prog_val);
+ if (ret)
+ ret = of_property_read_u32(node, "arc,trim-config", &prog_val);
if (ret == 0)
lp->pdata->trim_config = (u8)prog_val;
@@ -381,6 +394,8 @@ static int arcxcnn_remove(struct i2c_client *cl)
}
static const struct of_device_id arcxcnn_dt_ids[] = {
+ { .compatible = "arctic,arc2c0608" },
+ /* here to remaim compatible with an older binding, do not use */
s/remaim/remain
{ .compatible = "arc,arc2c0608" },
{ }
};
@@ -404,5 +419,5 @@ static struct i2c_driver arcxcnn_driver = {
module_i2c_driver(arcxcnn_driver);
MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Brian Dodge ");
+MODULE_AUTHOR("Brian Dodge ");
MODULE_DESCRIPTION("ARCXCNN Backlight driver");
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] devicetree: Update led binding
JJ On 7/1/19 10:14 AM, Jean-Jacques Hiblot wrote: Update the led binding to describe the possibility to add a "compatible" option to create a child-device, user of the LED. Signed-off-by: Jean-Jacques Hiblot Cc: devicet...@vger.kernel.org --- Documentation/devicetree/bindings/leds/common.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt index 70876ac11367..2f7882528d97 100644 --- a/Documentation/devicetree/bindings/leds/common.txt +++ b/Documentation/devicetree/bindings/leds/common.txt @@ -11,6 +11,9 @@ have to be tightly coupled with the LED device binding. They are represented by child nodes of the parent LED device binding. Optional properties for child nodes: +- compatible : driver name for a child-device. This child-device is the user + of the LED. It is created when the LED is registered and + destroyed when the LED is unregistered. Can you update the example to show usage? Dan - led-sources : List of device current outputs the LED is connected to. The outputs are identified by the numbers that must be defined in the LED device binding documentation.
Re: [PATCH 1/2] dt-bindings: backlight: fix vendor prefix for ArcticSand arcxcnn driver bindings
Brian
On 6/30/19 7:28 PM, Brian Dodge wrote:
The vendor-prefixes.txt file properly refers to ArcticSand
as arctic but the driver bindings improperly abbreviated the
prefix to arc. This was a mistake in the original patch. This
patch adds "arctic" and retains "arc" (deprecated) bindings
Signed-off-by: Brian Dodge
---
.../bindings/leds/backlight/arcxcnn_bl.txt | 31 +++---
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
index 230abde..4d98394 100644
--- a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
+++ b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
@@ -1,8 +1,13 @@
-Binding for ArcticSand arc2c0608 LED driver
+Binding for ArcticSand arc family LED drivers
Required properties:
-- compatible: should be "arc,arc2c0608"
-- reg: slave address
+- compatible: one of
+ "arctic,arc1c0608"
+ "arctic,arc2c0608"
+ "arctic,arc3c0845"
+ "arc,arc2c0608" (deprecated)
+
+- reg: slave address
Optional properties:
- default-brightness: brightness value on boot, value from: 0-4095
@@ -11,19 +16,25 @@ Optional properties:
- led-sources:List of enabled channels from 0 to 5.
See Documentation/devicetree/bindings/leds/common.txt
-- arc,led-config-0: setting for register ILED_CONFIG_0
-- arc,led-config-1:setting for register ILED_CONFIG_1
-- arc,dim-freq:PWM mode frequence setting (bits [3:0] used)
-- arc,comp-config: setting for register CONFIG_COMP
-- arc,filter-config: setting for register FILTER_CONFIG
-- arc,trim-config: setting for register IMAXTUNE
+- arctic,led-config-0: setting for register ILED_CONFIG_0
+- arctic,led-config-1: setting for register ILED_CONFIG_1
+- arctic,dim-freq: PWM mode frequence setting (bits [3:0] used)
+- arctic,comp-config: setting for register CONFIG_COMP
+- arctic,filter-config:setting for register FILTER_CONFIG
+- arctic,trim-config: setting for register IMAXTUNE
+- arc,led-config-0:setting for register ILED_CONFIG_0 (deprecated)
+- arc,led-config-1:setting for register ILED_CONFIG_1 (deprecated)
+- arc,dim-freq:PWM mode frequence setting (bits [3:0] used)
(deprecated)
+- arc,comp-config: setting for register CONFIG_COMP (deprecated)
+- arc,filter-config: setting for register FILTER_CONFIG (deprecated)
+- arc,trim-config: setting for register IMAXTUNE (deprecated)
Note: Optional properties not specified will default to values in IC EPROM
Example:
arc2c0608@30 {
- compatible = "arc,arc2c0608";
+ compatible = "arctic,arc2c0608";
reg = <0x30>;
default-brightness = <500>;
label = "lcd-backlight";
Reviewed-by: Dan Murphy
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] backlight: arcxcnn: add "arctic" vendor prefix
Brian
On 6/30/19 7:28 PM, Brian Dodge wrote:
The original patch adding this driver and DT bindings improperly
used "arc" as the vendor-prefix. This adds "arctic" which is the
proper prefix and retains "arc" to allow existing users of the
"arc" prefix to update to new kernels. There is at least one
(Samsung Chromebook Plus)
Signed-off-by: Brian Dodge
Acked-by: Daniel Thompson
---
drivers/video/backlight/arcxcnn_bl.c | 41 +++-
1 file changed, 31 insertions(+), 10 deletions(-)
diff --git a/drivers/video/backlight/arcxcnn_bl.c
b/drivers/video/backlight/arcxcnn_bl.c
index 7b1c0a0..a419554 100644
--- a/drivers/video/backlight/arcxcnn_bl.c
+++ b/drivers/video/backlight/arcxcnn_bl.c
@@ -1,9 +1,9 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * Backlight driver for ArcticSand ARC_X_C_0N_0N Devices
+ * Backlight driver for pSemi (formerly ArcticSand) ARC_X_C_0N_0N Devices
*
- * Copyright 2016 ArcticSand, Inc.
- * Author : Brian Dodge
I know you are the original author from ArcticSand but did pSemi
actually own the copyright in 2016?
I don't think this is a big issue just wondering if we should retain the
ArcticSand copyright as well.
Probably a question for your legal department.
Otherwise
Reviewed-by: Dan Murphy
+ * Copyright 2016-2019 pSemi, Inc.
+ * Author : Brian Dodge
*/
#include
@@ -191,27 +191,46 @@ static void arcxcnn_parse_dt(struct arcxcnn *lp)
if (ret == 0)
lp->pdata->initial_brightness = prog_val;
- ret = of_property_read_u32(node, "arc,led-config-0", &prog_val);
+ ret = of_property_read_u32(node, "arctic,led-config-0", &prog_val);
+ if (ret)
+ ret = of_property_read_u32(node, "arc,led-config-0", &prog_val);
+
if (ret == 0)
lp->pdata->led_config_0 = (u8)prog_val;
- ret = of_property_read_u32(node, "arc,led-config-1", &prog_val);
+ ret = of_property_read_u32(node, "arctic,led-config-1", &prog_val);
+ if (ret)
+ ret = of_property_read_u32(node, "arc,led-config-1", &prog_val);
+
if (ret == 0)
lp->pdata->led_config_1 = (u8)prog_val;
- ret = of_property_read_u32(node, "arc,dim-freq", &prog_val);
+ ret = of_property_read_u32(node, "arctic,dim-freq", &prog_val);
+ if (ret)
+ ret = of_property_read_u32(node, "arc,dim-freq", &prog_val);
+
if (ret == 0)
lp->pdata->dim_freq = (u8)prog_val;
- ret = of_property_read_u32(node, "arc,comp-config", &prog_val);
+ ret = of_property_read_u32(node, "arctic,comp-config", &prog_val);
+ if (ret)
+ ret = of_property_read_u32(node, "arc,comp-config", &prog_val);
+
if (ret == 0)
lp->pdata->comp_config = (u8)prog_val;
- ret = of_property_read_u32(node, "arc,filter-config", &prog_val);
+ ret = of_property_read_u32(node, "arctic,filter-config", &prog_val);
+ if (ret)
+ ret = of_property_read_u32(node,
+ "arc,filter-config", &prog_val);
+
if (ret == 0)
lp->pdata->filter_config = (u8)prog_val;
- ret = of_property_read_u32(node, "arc,trim-config", &prog_val);
+ ret = of_property_read_u32(node, "arctic,trim-config", &prog_val);
+ if (ret)
+ ret = of_property_read_u32(node, "arc,trim-config", &prog_val);
+
if (ret == 0)
lp->pdata->trim_config = (u8)prog_val;
@@ -381,6 +400,8 @@ static int arcxcnn_remove(struct i2c_client *cl)
}
static const struct of_device_id arcxcnn_dt_ids[] = {
+ { .compatible = "arctic,arc2c0608" },
+ /* here to remain compatible with an older binding, do not use */
{ .compatible = "arc,arc2c0608" },
{ }
};
@@ -404,5 +425,5 @@ static struct i2c_driver arcxcnn_driver = {
module_i2c_driver(arcxcnn_driver);
MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Brian Dodge ");
+MODULE_AUTHOR("Brian Dodge ");
MODULE_DESCRIPTION("ARCXCNN Backlight driver");
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] dt-bindings: backlight: fix vendor prefix for ArcticSand arcxcnn driver bindings
Brian
On 7/9/19 12:48 PM, Brian Dodge wrote:
FYI: please note that pSemi's legal department has informed me that
they do *not* want to keep the "ArcticSand" copyright notices and the
single pSemi line is appropriate.
Thanks for the follow up. Lawyers can be fickle about this stuff.
On Mon, Jul 8, 2019 at 2:02 PM Dan Murphy <mailto:dmur...@ti.com>> wrote:
Brian
On 6/30/19 7:28 PM, Brian Dodge wrote:
> The vendor-prefixes.txt file properly refers to ArcticSand
> as arctic but the driver bindings improperly abbreviated the
> prefix to arc. This was a mistake in the original patch. This
> patch adds "arctic" and retains "arc" (deprecated) bindings
>
> Signed-off-by: Brian Dodge mailto:bdodg...@gmail.com>>
> ---
> .../bindings/leds/backlight/arcxcnn_bl.txt | 31
+++---
> 1 file changed, 21 insertions(+), 10 deletions(-)
>
> diff --git
a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
> index 230abde..4d98394 100644
> ---
a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
> +++
b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
> @@ -1,8 +1,13 @@
> -Binding for ArcticSand arc2c0608 LED driver
> +Binding for ArcticSand arc family LED drivers
>
> Required properties:
> -- compatible: should be "arc,arc2c0608"
> -- reg: slave address
> +- compatible: one of
> + "arctic,arc1c0608"
> + "arctic,arc2c0608"
> + "arctic,arc3c0845"
> + "arc,arc2c0608" (deprecated)
> +
> +- reg: slave address
>
> Optional properties:
> - default-brightness: brightness value on boot, value
from: 0-4095
> @@ -11,19 +16,25 @@ Optional properties:
> - led-sources: List of enabled channels from 0 to 5.
> See
Documentation/devicetree/bindings/leds/common.txt
>
> -- arc,led-config-0: setting for register ILED_CONFIG_0
> -- arc,led-config-1: setting for register ILED_CONFIG_1
> -- arc,dim-freq: PWM mode frequence setting (bits
[3:0] used)
> -- arc,comp-config: setting for register CONFIG_COMP
> -- arc,filter-config: setting for register FILTER_CONFIG
> -- arc,trim-config: setting for register IMAXTUNE
> +- arctic,led-config-0: setting for register ILED_CONFIG_0
> +- arctic,led-config-1: setting for register ILED_CONFIG_1
> +- arctic,dim-freq: PWM mode frequence setting (bits [3:0] used)
> +- arctic,comp-config: setting for register CONFIG_COMP
> +- arctic,filter-config: setting for register FILTER_CONFIG
> +- arctic,trim-config: setting for register IMAXTUNE
> +- arc,led-config-0: setting for register ILED_CONFIG_0
(deprecated)
> +- arc,led-config-1: setting for register ILED_CONFIG_1
(deprecated)
> +- arc,dim-freq: PWM mode frequence setting (bits
[3:0] used) (deprecated)
> +- arc,comp-config: setting for register CONFIG_COMP (deprecated)
> +- arc,filter-config: setting for register FILTER_CONFIG
(deprecated)
> +- arc,trim-config: setting for register IMAXTUNE (deprecated)
>
> Note: Optional properties not specified will default to values
in IC EPROM
>
> Example:
>
> arc2c0608@30 {
> - compatible = "arc,arc2c0608";
> + compatible = "arctic,arc2c0608";
> reg = <0x30>;
> default-brightness = <500>;
> label = "lcd-backlight";
Reviewed-by: Dan Murphy mailto:dmur...@ti.com>>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/3] dt-bindings: backlight: add lm3630a bindings
Brian
On 4/2/19 8:24 AM, Brian Masney wrote:
> On Tue, Apr 02, 2019 at 07:56:55AM -0500, Dan Murphy wrote:
>> This would connect control bank B to control bank A. Or just use a flag to
>> denote to connect them
>> and not use led-sources. But led-sources is the property of choice.
>>
>> led@0 {
>> reg = <0>;
>> led-sources = < 0 1 >;
>> label = "main-lcd";
>> default-brightness = <200>;
>> max-brightness = <255>;
>> };
>
> OK, I see. I wondered how we could do that in device tree.
>
>>> +properties:
>>> + label:
>>> +description: |
>>> + The label for this LED. If omitted, the label is taken from the
>>> node
>>> + name (excluding the unit address). It has to uniquely identify a
>>> + device, i.e. no other LED class device can be assigned the same
>>> label.
>>> +
>>> + led-sources:
>>> +description: |
>>> + List of device current outputs the LED is connected to.
>>> +allOf:
>>> + - $ref: /schemas/types.yaml#/definitions/uint32-array
>>> + - minItems: 1
>>> +maxItems: 2
>>> +items:
>>> + minimum: 0
>>> + maximum: 1
>>> +
>>
>> label and led-sources are already defined in the common.txt no need to
>> redefine them here.
>
> If I'm going to use the new-style bindings, then I'll need to convert
> common.txt over to use the new format as well so that the automated
> schema validations will work. I'm willing to do that work if there is
> interest from the LED / backlight maintainers. The main issue is that
> there are 62 references to the file common.txt in the directory
> Documentation/devicetree/bindings/leds/. Would the maintainers prefer:
>
> - Once common.txt is converted to common.yaml, make common.txt only
> have a line stating that the common bindings were moved into
> common.yaml. We can remove this file once all of the other bindings
> are converted to the new-style format.
>
> - Update all references to common.txt to common.yaml. (1 patch or 62
> patches?)
>
> - Or, just go with the older-style binding format for now.
>
> Thanks Dan for your other comments. They make sense and I'll incorporate
> those changes into my next version.
>
That is up to the maintainers.
Also one other comment I noticed when reviewing the code that there is no
definition to
which child led properties are optional and which are required?
Dan
> Brian
>
--
--
Dan Murphy
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/3] dt-bindings: backlight: add lm3630a bindings
+label = "main-lcd";
> +led-sources = <0 1>;
> +default-brightness = <200>;
> +max-brightness = <255>;
> +};
> +};
> +};
> + - |
> +i2c {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +lm3630a_bl@38 {
> +compatible = "ti,lm3630a";
> +status = "ok";
> +reg = <0x38>;
> +
> +led-bank-a {
> +led-sources = <0>;
> +default-brightness = <150>;
> +ti,linear-mapping-mode;
> +};
> +
> +led-bank-b {
> +led-sources = <1>;
> +default-brightness = <225>;
> +ti,linear-mapping-mode;
> +};
> +};
> +};
>
--
--
Dan Murphy
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/3] backlight: lm3630a: add device tree supprt
On 4/2/19 11:45 AM, Pavel Machek wrote: > On Tue 2019-04-02 08:45:40, Dan Murphy wrote: >> Hello >> >> On 4/1/19 5:30 AM, Brian Masney wrote: >>> Add device tree support to the lm3630a driver and allow configuring >>> independently on both banks the mapping mode (linear or exponential), >>> initial and maximum LED brightness. >>> >>> Driver was tested on a LG Nexus 5 (hammerhead) phone. >>> >> >> Don't need this in the commit message. > > Actually yes, we want that in commit message. > Noted for future patches. Dan > Pavel > -- -- Dan Murphy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/3] backlight: lm3630a: add device tree supprt
(pchip);
>
Hmm. Wondering if this should be in the if (pdata == NULL) case and if parsing
the
node fails then set defaults.
Because the way it is written if pdata is null then all the defaults are set
then the dt parsing
overwrites the data.
Not sure if thats what you intended.
Dan
> /* chip initialize */
> rval = lm3630a_chip_init(pchip);
> @@ -470,11 +532,18 @@ static const struct i2c_device_id lm3630a_id[] = {
> {}
> };
>
> +static const struct of_device_id lm3630a_match_table[] = {
> + { .compatible = "ti,lm3630a", },
> + { },
> +};
> +
> +
> MODULE_DEVICE_TABLE(i2c, lm3630a_id);
>
> static struct i2c_driver lm3630a_i2c_driver = {
> .driver = {
> .name = LM3630A_NAME,
> +.of_match_table = lm3630a_match_table,
> },
> .probe = lm3630a_probe,
> .remove = lm3630a_remove,
>
--
--
Dan Murphy
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/7] docs: dt: fix several broken references due to renames
Mauro On 2/22/20 3:00 AM, Mauro Carvalho Chehab wrote: Several DT references got broken due to txt->yaml conversion. Those are auto-fixed by running: scripts/documentation-file-ref-check --fix Signed-off-by: Mauro Carvalho Chehab --- Documentation/devicetree/bindings/arm/arm,scmi.txt| 2 +- Documentation/devicetree/bindings/arm/arm,scpi.txt| 2 +- .../devicetree/bindings/arm/bcm/brcm,bcm63138.txt | 2 +- .../devicetree/bindings/arm/hisilicon/hi3519-sysctrl.txt | 2 +- .../devicetree/bindings/arm/msm/qcom,idle-state.txt | 2 +- Documentation/devicetree/bindings/arm/omap/mpu.txt| 2 +- Documentation/devicetree/bindings/arm/psci.yaml | 2 +- .../devicetree/bindings/clock/qcom,gcc-apq8064.yaml | 2 +- .../devicetree/bindings/display/tilcdc/tilcdc.txt | 2 +- Documentation/devicetree/bindings/leds/common.yaml| 2 +- For LEDs Reviewed-by: Dan Murphy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/2] dt-bindings: backlight: lm3630a: add enable_gpios
Andreas
On 9/11/19 12:21 PM, Andreas Kemnade wrote:
add enable-gpios to describe HWEN pin
Signed-off-by: Andreas Kemnade
Acked-by: Daniel Thompson
---
changes in v2: added example
changes in v3: added Acked-by
.../bindings/leds/backlight/lm3630a-backlight.yaml | 5 +
1 file changed, 5 insertions(+)
diff --git
a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
index dc129d9a329e..1fa83feffe16 100644
--- a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
+++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
@@ -29,6 +29,10 @@ properties:
'#size-cells':
const: 0
+ enable-gpios:
+description: GPIO to use to enable/disable the backlight (HWEN pin).
+maxItems: 1
+
required:
- compatible
- reg
@@ -92,6 +96,7 @@ examples:
i2c {
#address-cells = <1>;
#size-cells = <0>;
+enable-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>;
led-controller@38 {
compatible = "ti,lm3630a";
Looks good to me
Reviewed-by: Dan Murphy
Re: [PATCH v3 2/2] backlight: lm3630a: add an enable gpio for the HWEN pin
Andreas
On 9/11/19 12:21 PM, Andreas Kemnade wrote:
For now just enable it in the probe function to allow i2c
access. Disabling also means resetting the register values
to default and according to the datasheet does not give
power savings.
Tested on Kobo Clara HD.
Signed-off-by: Andreas Kemnade
---
changes in v2:
- simplification
- correct gpio direction initialisation
changes in v3:
- removed legacy include
drivers/video/backlight/lm3630a_bl.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/video/backlight/lm3630a_bl.c
b/drivers/video/backlight/lm3630a_bl.c
index 8f84f3684f04..d9e67b9b2571 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
@@ -48,6 +49,7 @@ struct lm3630a_chip {
struct lm3630a_platform_data *pdata;
struct backlight_device *bleda;
struct backlight_device *bledb;
+ struct gpio_desc *enable_gpio;
struct regmap *regmap;
struct pwm_device *pwmd;
};
@@ -535,6 +537,13 @@ static int lm3630a_probe(struct i2c_client *client,
}
pchip->pdata = pdata;
+ pchip->enable_gpio = devm_gpiod_get_optional(&client->dev, "enable",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(pchip->enable_gpio)) {
+ rval = PTR_ERR(pchip->enable_gpio);
+ return rval;
+ }
+
/* chip initialize */
rval = lm3630a_chip_init(pchip);
if (rval < 0) {
Thanks for the explanation
It looks good to me
Reviewed-by: Dan Murphy
Re: [PATCH v3 1/2] dt-bindings: backlight: lm3630a: add enable_gpios
Andreas
On 9/12/19 9:58 AM, Andreas Kemnade wrote:
On Thu, 12 Sep 2019 06:39:50 -0500
Dan Murphy wrote:
Andreas
On 9/11/19 12:21 PM, Andreas Kemnade wrote:
add enable-gpios to describe HWEN pin
Signed-off-by: Andreas Kemnade
Acked-by: Daniel Thompson
---
changes in v2: added example
changes in v3: added Acked-by
.../bindings/leds/backlight/lm3630a-backlight.yaml | 5 +
1 file changed, 5 insertions(+)
diff --git
a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
index dc129d9a329e..1fa83feffe16 100644
--- a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
+++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
@@ -29,6 +29,10 @@ properties:
'#size-cells':
const: 0
+ enable-gpios:
+description: GPIO to use to enable/disable the backlight (HWEN pin).
+maxItems: 1
+
required:
- compatible
- reg
@@ -92,6 +96,7 @@ examples:
i2c {
#address-cells = <1>;
#size-cells = <0>;
+enable-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>;
led-controller@38 {
compatible = "ti,lm3630a";
Looks good to me
well, the enable-gpios is still at the same place as in v2. This was sent
before your comments to v2 have been arrived.
Ah I overlooked that. Yeah that still needs to move I assumed you moved it.
Dan
Regards,
Andreas
Re: [PATCH v4 1/2] dt-bindings: backlight: lm3630a: add enable_gpios
Andreas
On 9/12/19 4:32 PM, Andreas Kemnade wrote:
add enable-gpios to describe HWEN pin
Signed-off-by: Andreas Kemnade
Acked-by: Daniel Thompson
Reviewed-by: Dan Murphy
---
changes in v2: added example
changes in v3: added Acked-by
changes in v4: moved enable-gpios to the right position
in the example
.../bindings/leds/backlight/lm3630a-backlight.yaml | 5 +
1 file changed, 5 insertions(+)
diff --git
a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
index dc129d9a329e..c8470628fe02 100644
--- a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
+++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
@@ -29,6 +29,10 @@ properties:
'#size-cells':
const: 0
+ enable-gpios:
+description: GPIO to use to enable/disable the backlight (HWEN pin).
+maxItems: 1
+
required:
- compatible
- reg
@@ -96,6 +100,7 @@ examples:
led-controller@38 {
compatible = "ti,lm3630a";
reg = <0x38>;
+enable-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>;
#address-cells = <1>;
#size-cells = <0>;
Re: [PATCH V6 1/8] backlight: qcom-wled: Rename pm8941-wled.c to qcom-wled.c
Kiran
On 9/30/19 1:39 AM, Kiran Gunda wrote:
pm8941-wled.c driver is supporting the WLED peripheral
on pm8941. Rename it to qcom-wled.c so that it can support
WLED on multiple PMICs.
Signed-off-by: Kiran Gunda
Reviewed-by: Bjorn Andersson
Acked-by: Rob Herring
Acked-by: Daniel Thompson
Acked-by: Pavel Machek
---
.../bindings/leds/backlight/{pm8941-wled.txt => qcom-wled.txt}| 2 +-
Instead of renaming this file would it be more maintainable to indicate
in the pm8941-wled.txt
to reference the qcom-wled.txt file for complete description?
I will let Rob comment on maintainability.
Dan
drivers/video/backlight/Kconfig | 8
drivers/video/backlight/Makefile | 2 +-
drivers/video/backlight/{pm8941-wled.c => qcom-wled.c}| 0
4 files changed, 6 insertions(+), 6 deletions(-)
rename Documentation/devicetree/bindings/leds/backlight/{pm8941-wled.txt =>
qcom-wled.txt} (95%)
rename drivers/video/backlight/{pm8941-wled.c => qcom-wled.c} (100%)
diff --git a/Documentation/devicetree/bindings/leds/backlight/pm8941-wled.txt
b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
similarity index 95%
rename from Documentation/devicetree/bindings/leds/backlight/pm8941-wled.txt
rename to Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
index e5b294d..fb39e32 100644
--- a/Documentation/devicetree/bindings/leds/backlight/pm8941-wled.txt
+++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
@@ -1,4 +1,4 @@
-Binding for Qualcomm PM8941 WLED driver
+Binding for Qualcomm Technologies, Inc. WLED driver
Required properties:
- compatible: should be "qcom,pm8941-wled"
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 8b081d6..6ff3176 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -284,12 +284,12 @@ config BACKLIGHT_TOSA
If you have an Sharp SL-6000 Zaurus say Y to enable a driver
for its backlight
-config BACKLIGHT_PM8941_WLED
- tristate "Qualcomm PM8941 WLED Driver"
+config BACKLIGHT_QCOM_WLED
+ tristate "Qualcomm PMIC WLED Driver"
select REGMAP
help
- If you have the Qualcomm PM8941, say Y to enable a driver for the
- WLED block.
+ If you have the Qualcomm PMIC, say Y to enable a driver for the
+ WLED block. Currently it supports PM8941 and PMI8998.
config BACKLIGHT_SAHARA
tristate "Tabletkiosk Sahara Touch-iT Backlight Driver"
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 63c507c..6f87770 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -48,8 +48,8 @@ obj-$(CONFIG_BACKLIGHT_OMAP1) += omap1_bl.o
obj-$(CONFIG_BACKLIGHT_OT200) += ot200_bl.o
obj-$(CONFIG_BACKLIGHT_PANDORA) += pandora_bl.o
obj-$(CONFIG_BACKLIGHT_PCF50633) += pcf50633-backlight.o
-obj-$(CONFIG_BACKLIGHT_PM8941_WLED)+= pm8941-wled.o
obj-$(CONFIG_BACKLIGHT_PWM) += pwm_bl.o
+obj-$(CONFIG_BACKLIGHT_QCOM_WLED) += qcom-wled.o
obj-$(CONFIG_BACKLIGHT_SAHARA)+= kb3886_bl.o
obj-$(CONFIG_BACKLIGHT_SKY81452) += sky81452-backlight.o
obj-$(CONFIG_BACKLIGHT_TOSA) += tosa_bl.o
diff --git a/drivers/video/backlight/pm8941-wled.c
b/drivers/video/backlight/qcom-wled.c
similarity index 100%
rename from drivers/video/backlight/pm8941-wled.c
rename to drivers/video/backlight/qcom-wled.c
Re: [PATCH V6 2/8] backlight: qcom-wled: restructure the qcom-wled bindings
Kiran On 9/30/19 1:39 AM, Kiran Gunda wrote: Restructure the qcom-wled bindings for the better readability. Signed-off-by: Kiran Gunda Reviewed-by: Bjorn Andersson Reviewed-by: Rob Herring Acked-by: Daniel Thompson Acked-by: Pavel Machek --- .../bindings/leds/backlight/qcom-wled.txt | 110 - Since you are restructuring would it not be better to convert this to the yaml format? It looks yamlish so the file extension should be .yaml. Dan 1 file changed, 85 insertions(+), 25 deletions(-) diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt index fb39e32..14f28f2 100644 --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt @@ -1,30 +1,90 @@ Binding for Qualcomm Technologies, Inc. WLED driver -Required properties: -- compatible: should be "qcom,pm8941-wled" -- reg: slave address - -Optional properties: -- default-brightness: brightness value on boot, value from: 0-4095 - default: 2048 -- label: The name of the backlight device -- qcom,cs-out: bool; enable current sink output -- qcom,cabc: bool; enable content adaptive backlight control -- qcom,ext-gen: bool; use externally generated modulator signal to dim -- qcom,current-limit: mA; per-string current limit; value from 0 to 25 - default: 20mA -- qcom,current-boost-limit: mA; boost current limit; one of: - 105, 385, 525, 805, 980, 1260, 1400, 1680 - default: 805mA -- qcom,switching-freq: kHz; switching frequency; one of: - 600, 640, 685, 738, 800, 872, 960, 1066, 1200, 1371, - 1600, 1920, 2400, 3200, 4800, 9600, - default: 1600kHz -- qcom,ovp: V; Over-voltage protection limit; one of: - 27, 29, 32, 35 - default: 29V -- qcom,num-strings: #; number of led strings attached; value from 1 to 3 - default: 2 +WLED (White Light Emitting Diode) driver is used for controlling display +backlight that is part of PMIC on Qualcomm Technologies, Inc. reference +platforms. The PMIC is connected to the host processor via SPMI bus. + +- compatible + Usage:required + Value type: + Definition: should be one of: + "qcom,pm8941-wled" + "qcom,pmi8998-wled" + "qcom,pm660l-wled" + +- reg + Usage:required + Value type: + Definition: Base address of the WLED modules. + +- default-brightness + Usage:optional + Value type: + Definition: brightness value on boot, value from: 0-4095 + Default: 2048 + +- label + Usage:required + Value type: + Definition: The name of the backlight device + +- qcom,cs-out + Usage:optional + Value type: + Definition: enable current sink output. + This property is supported only for PM8941. + +- qcom,cabc + Usage:optional + Value type: + Definition: enable content adaptive backlight control. + +- qcom,ext-gen + Usage:optional + Value type: + Definition: use externally generated modulator signal to dim. + This property is supported only for PM8941. + +- qcom,current-limit + Usage:optional + Value type: + Definition: mA; per-string current limit + value: For pm8941: from 0 to 25 with 5 mA step +Default 20 mA. +For pmi8998: from 0 to 30 with 5 mA step +Default 25 mA. + +- qcom,current-boost-limit + Usage:optional + Value type: + Definition: mA; boost current limit. + For pm8941: one of: 105, 385, 525, 805, 980, 1260, 1400, + 1680. Default: 805 mA + For pmi8998: one of: 105, 280, 450, 620, 970, 1150, 1300, + 1500. Default: 970 mA + +- qcom,switching-freq + Usage:optional + Value type: +Definition: kHz; switching frequency; one of: 600, 640, 685, 738, + 800, 872, 960, 1066, 1200, 1371, 1600, 1920, 2400, 3200, + 4800, 9600. + Default: for pm8941: 1600 kHz + for pmi8998: 800 kHz + +- qcom,ovp + Usage:optional + Value type: + Definition: V; Over-voltage protection limit; one of: + 27, 29, 32, 35. default: 29V + This property is supported only for PM8941. + +- qcom,num-strings + Usage:optional + Value type: + Definition: #; number of led strings attached; + value from 1 to 3. default: 2 + This property is supported only for PM8941. Example:
Re: [PATCH V6 3/8] backlight: qcom-wled: Add new properties for PMI8998
Kiran On 9/30/19 1:39 AM, Kiran Gunda wrote: Update the bindings with the new properties used for PMI8998. Signed-off-by: Kiran Gunda Reviewed-by: Bjorn Andersson Reviewed-by: Rob Herring Acked-by: Daniel Thompson --- .../bindings/leds/backlight/qcom-wled.txt | 76 ++ 1 file changed, 62 insertions(+), 14 deletions(-) diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt index 14f28f2..9d840d5 100644 --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt @@ -20,8 +20,7 @@ platforms. The PMIC is connected to the host processor via SPMI bus. - default-brightness Usage:optional Value type: - Definition: brightness value on boot, value from: 0-4095 - Default: 2048 + Definition: brightness value on boot, value from: 0-4095. - label Usage:required @@ -48,20 +47,24 @@ platforms. The PMIC is connected to the host processor via SPMI bus. - qcom,current-limit Usage:optional Value type: - Definition: mA; per-string current limit - value: For pm8941: from 0 to 25 with 5 mA step -Default 20 mA. -For pmi8998: from 0 to 30 with 5 mA step -Default 25 mA. + Definition: mA; per-string current limit; value from 0 to 25 with + 1 mA step. + This property is supported only for pm8941. + +- qcom,current-limit-microamp + Usage:optional + Value type: + Definition: uA; per-string current limit; value from 0 to 3 with + 2500 uA step. - qcom,current-boost-limit Usage:optional Value type: Definition: mA; boost current limit. For pm8941: one of: 105, 385, 525, 805, 980, 1260, 1400, - 1680. Default: 805 mA + 1680. For pmi8998: one of: 105, 280, 450, 620, 970, 1150, 1300, - 1500. Default: 970 mA + 1500. - qcom,switching-freq Usage:optional @@ -69,22 +72,66 @@ platforms. The PMIC is connected to the host processor via SPMI bus. Definition: kHz; switching frequency; one of: 600, 640, 685, 738, 800, 872, 960, 1066, 1200, 1371, 1600, 1920, 2400, 3200, 4800, 9600. - Default: for pm8941: 1600 kHz - for pmi8998: 800 kHz - qcom,ovp Usage:optional Value type: Definition: V; Over-voltage protection limit; one of: - 27, 29, 32, 35. default: 29V + 27, 29, 32, 35. This property is supported only for PM8941. +- qcom,ovp-millivolt + Usage:optional + Value type: + Definition: mV; Over-voltage protection limit; + For pmi8998: one of 18100, 19600, 29600, 31100 + If this property is not specified for PM8941, it + falls back to "qcom,ovp" property. + - qcom,num-strings Usage:optional Value type: Definition: #; number of led strings attached; - value from 1 to 3. default: 2 - This property is supported only for PM8941. + value: For PM8941 from 1 to 3. +For PMI8998 from 1 to 4. We probably don't need this since we define 1 led node per output. And if you need to define multiple strings per node then you use led-sources. Then you will use fwnode_property_count_u32(child, "led-sources"); to get the number of outputs + +- interrupts + Usage:optional + Value type: + Definition: Interrupts associated with WLED. This should be + "short" and "ovp" interrupts. Interrupts can be + specified as per the encoding listed under + Documentation/devicetree/bindings/spmi/ + qcom,spmi-pmic-arb.txt. + +- interrupt-names + Usage:optional + Value type: + Definition: Interrupt names associated with the interrupts. + Must be "short" and "ovp". The short circuit detection + is not supported for PM8941. + +- qcom,enabled-strings + Usage:optional + Value tyoe: + Definition: Array of the WLED strings numbered from 0 to 3. Each + string of leds are operated individually. Specify the + list of strings used by the device. Any combination of + led strings can be u
Re: [PATCH V6 5/8] backlight: qcom-wled: Restructure the driver for WLED3
Kiran On 9/30/19 1:39 AM, Kiran Gunda wrote: Restructure the driver to add the support for new WLED peripherals. Signed-off-by: Kiran Gunda Acked-by: Daniel Thompson --- drivers/video/backlight/qcom-wled.c | 395 ++-- 1 file changed, 245 insertions(+), 150 deletions(-) diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index f191242..740f1b6 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -7,59 +7,71 @@ #include #include #include +#include #include /* From DT binding */ +#define WLED_MAX_STRINGS 4 + #define WLED_DEFAULT_BRIGHTNESS 2048 -#define WLED3_SINK_REG_BRIGHT_MAX 0xFFF -#define WLED3_CTRL_REG_VAL_BASE0x40 +#define WLED_SINK_REG_BRIGHT_MAX 0xFFF Why did you change some of these again? Can you just change it to the final #define in patch 4/8? Dan
Re: [PATCH v2 1/2] dt-bindings: backlight: lm3630a: add enable_gpios
Andreas
On 9/11/19 5:08 AM, Daniel Thompson wrote:
On Tue, Sep 10, 2019 at 11:29:08PM +0200, Andreas Kemnade wrote:
add enable-gpios to describe HWEN pin
Signed-off-by: Andreas Kemnade
Acked-by: Daniel Thompson
---
changes in v2: add example
.../bindings/leds/backlight/lm3630a-backlight.yaml | 5 +
1 file changed, 5 insertions(+)
diff --git
a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
index dc129d9a329e..1fa83feffe16 100644
--- a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
+++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
@@ -29,6 +29,10 @@ properties:
'#size-cells':
const: 0
+ enable-gpios:
+description: GPIO to use to enable/disable the backlight (HWEN pin).
+maxItems: 1
+
required:
- compatible
- reg
@@ -92,6 +96,7 @@ examples:
i2c {
#address-cells = <1>;
#size-cells = <0>;
+enable-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>;
This is in the wrong place. This is implying that the gpio is for the
i2c parent
This needs to go under the led-controller node below
Dan
led-controller@38 {
compatible = "ti,lm3630a";
--
2.20.1
Re: [PATCH v2 2/2] backlight: lm3630a: add an enable gpio for the HWEN pin
On 9/11/19 5:25 AM, Daniel Thompson wrote:
On Tue, Sep 10, 2019 at 11:29:09PM +0200, Andreas Kemnade wrote:
For now just enable it in the probe function to allow i2c
access. Disabling also means resetting the register values
to default and according to the datasheet does not give
power savings
Tested on Kobo Clara HD.
Signed-off-by: Andreas Kemnade
---
changes in v2:
- simplification
- correct gpio direction initialisation
drivers/video/backlight/lm3630a_bl.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/video/backlight/lm3630a_bl.c
b/drivers/video/backlight/lm3630a_bl.c
index 8f84f3684f04..9d0639d4202d 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -12,6 +12,8 @@
#include
#include
#include
+#include
+#include
Nitpicking... but I don't think linux/gpio.h is used anymore.
#include
#include
@@ -48,6 +50,7 @@ struct lm3630a_chip {
struct lm3630a_platform_data *pdata;
struct backlight_device *bleda;
struct backlight_device *bledb;
+ struct gpio_desc *enable_gpio;
struct regmap *regmap;
struct pwm_device *pwmd;
};
@@ -535,6 +538,13 @@ static int lm3630a_probe(struct i2c_client *client,
}
pchip->pdata = pdata;
+ pchip->enable_gpio = devm_gpiod_get_optional(&client->dev, "enable",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(pchip->enable_gpio)) {
+ rval = PTR_ERR(pchip->enable_gpio);
+ return rval;
the enable gpio is optional so if it fails you log the error and move on
Also on driver removal did you want to set the GPIO to low to disable
the device to save power?
Dan
+ }
+
/* chip initialize */
rval = lm3630a_chip_init(pchip);
if (rval < 0) {
--
2.20.1
Re: [PATCH] MAINTAINERS: move Milo Kim to credits
All On 9/22/20 4:36 AM, Mark Brown wrote: On Tue, Sep 22, 2020 at 09:08:37AM +0200, Krzysztof Kozlowski wrote: On Mon, 21 Sep 2020 at 23:06, Pavel Machek wrote: I believe normal way would be to mark the entries "orphaned", not to drop them altogether. Plus, I believe someone from TI is likely to step up. These are entries for specific drivers so they are covered by the subsystem maintainers. You believe someone will step up, I believe if these were important for TI, they would find the person some time ago, so the emails won't bounce... This was similar with BQ chargers where It's fairly common for mobile parts to get dropped relatively quickly as the technology moves fairly quickly in that market, I think a lot of teh parts that Milo was working on were mobile ones. These specific drivers don't see many patches applied to them. These drivers did have a few patches this year to fix random bugs. Since I have worked in these other subsystems if replacing the Maintainer is desired over removal then my name and email can be added like I did with Andrews. Dan Murphy Dan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] MAINTAINERS: add Dan Murphy as TP LP8xxx drivers maintainer
Hello On 9/22/20 10:28 AM, Krzysztof Kozlowski wrote: Milo Kim's email in TI bounces with permanent error (550: Invalid recipient). Last email from him on LKML was in 2017. Move Milo Kim to credits and add Dan Murphy from TI to look after: - TI LP855x backlight driver, - TI LP8727 charger driver, - TI LP8788 MFD (ADC, LEDs, charger and regulator) drivers. Cc: Dan Murphy Signed-off-by: Krzysztof Kozlowski Acked-by: Dan Murphy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] dt-bindings: trivial: add ti,lm3630a binding
Rob/Brian On 11/30/2018 08:13 AM, Rob Herring wrote: > +Dan M > > On Fri, Nov 30, 2018 at 7:59 AM Brian Masney wrote: >> >> On Tue, Nov 27, 2018 at 10:56:42AM +, Daniel Thompson wrote: >>> On Sat, Nov 24, 2018 at 09:17:02AM -0500, Brian Masney wrote: >>>> Add a trivial binding for the Texas Instruments LM3630A Backlight Chip. > > How does this chip relate to ones Dan has been working on? > This is a standard 8-bit white LED driver. It looks like Brian is just adding DT support to load the driver. I would expect that the bindings need to be updated to be able to register one string or another using the led-sources property. There are a couple of examples in the kernel and a couple of them in patch form. This driver and binding need to be updated to the latest spec, as you pointed out with child nodes. And Jacek has some new proposed bindings for the LED class so we may want to adopt those standards here as well. This is what I am waiting on for agreement so I can update my patch set. Dan >>> >>> It's quite unusual for a backlight device to have a trivial binding. >>> >>> The driver supports fairly extensive parametrization via struct >>> lm3530a_platform_data. It is really the case that none of these >>> properties should ever be set via DT? >> >> Hi Daniel, >> >> I initially assumed that we would let user space configure these values >> once the system has booted, but you are right that these should be >> available in device tree. >> >> The driver has two different LED banks that can be configured >> independently. > > That is usually represented with child nodes which makes this anything > but trivial. Plus, given that we have bindings for LEDs/backlights, no > LED/backlight controller is a trivial device. > >> How do you feel about having a single property in >> device tree populate the initial values for both banks? I propose that >> we could use the property default-brightness-level for leda_init_brt >> and ledb_init_brt in struct lm3630a_platform_data. The max-brightness >> property can populate leda_max_brt and ledb_max_brt. >> >> I need to look at other bindings this weekend to see if there are any >> standard properties that I can use for leda_ctrl/ledb_ctrl, pwm_ctrl, >> and pwm_period. >> >> Brian >> -- -- Dan Murphy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
