* [PATCH v5 0/3] backlight: lm3630a: bug fix and fwnode support @ 2019-04-18 15:11 ` Brian Masney 0 siblings, 0 replies; 23+ messages in thread From: Brian Masney @ 2019-04-18 15:11 UTC (permalink / raw) To: lee.jones, daniel.thompson, jingoohan1, robh+dt Cc: jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, dmurphy, jonathan Here is a patch series that fixes a single bug and adds fwnode support to the lm3630a driver. This work was tested on a LG Nexus 5 (hammerhead) phone. My status page at https://masneyb.github.io/nexus-5-upstream/ describes what is working so far with the upstream kernel. See the individual patches for the changelog. Brian Masney (3): backlight: lm3630a: return 0 on success in update_status functions dt-bindings: backlight: add lm3630a bindings backlight: lm3630a: add firmware node support .../leds/backlight/lm3630a-backlight.yaml | 129 +++++++++++++++ drivers/video/backlight/lm3630a_bl.c | 153 +++++++++++++++++- include/linux/platform_data/lm3630a_bl.h | 4 + 3 files changed, 279 insertions(+), 7 deletions(-) create mode 100644 Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml -- 2.20.1 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v5 0/3] backlight: lm3630a: bug fix and fwnode support @ 2019-04-18 15:11 ` Brian Masney 0 siblings, 0 replies; 23+ messages in thread From: Brian Masney @ 2019-04-18 15:11 UTC (permalink / raw) To: lee.jones, daniel.thompson, jingoohan1, robh+dt Cc: jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, dmurphy, jonathan Here is a patch series that fixes a single bug and adds fwnode support to the lm3630a driver. This work was tested on a LG Nexus 5 (hammerhead) phone. My status page at https://masneyb.github.io/nexus-5-upstream/ describes what is working so far with the upstream kernel. See the individual patches for the changelog. Brian Masney (3): backlight: lm3630a: return 0 on success in update_status functions dt-bindings: backlight: add lm3630a bindings backlight: lm3630a: add firmware node support .../leds/backlight/lm3630a-backlight.yaml | 129 +++++++++++++++ drivers/video/backlight/lm3630a_bl.c | 153 +++++++++++++++++- include/linux/platform_data/lm3630a_bl.h | 4 + 3 files changed, 279 insertions(+), 7 deletions(-) create mode 100644 Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml -- 2.20.1 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v5 1/3] backlight: lm3630a: return 0 on success in update_status functions 2019-04-18 15:11 ` Brian Masney @ 2019-04-18 15:11 ` Brian Masney -1 siblings, 0 replies; 23+ messages in thread From: Brian Masney @ 2019-04-18 15:11 UTC (permalink / raw) To: lee.jones, daniel.thompson, jingoohan1, robh+dt Cc: jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, dmurphy, jonathan lm3630a_bank_a_update_status() and lm3630a_bank_b_update_status() both return the brightness value if the brightness was successfully updated. Writing to these attributes via sysfs would cause a 'Bad address' error to be returned. These functions should return 0 on success, so let's change it to correct that error. Signed-off-by: Brian Masney <masneyb@onstation.org> Fixes: 28e64a68a2ef ("backlight: lm3630: apply chip revision") Acked-by: Pavel Machek <pavel@ucw.cz> --- drivers/video/backlight/lm3630a_bl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c index 2030a6b77a09..ef2553f452ca 100644 --- a/drivers/video/backlight/lm3630a_bl.c +++ b/drivers/video/backlight/lm3630a_bl.c @@ -201,7 +201,7 @@ static int lm3630a_bank_a_update_status(struct backlight_device *bl) LM3630A_LEDA_ENABLE, LM3630A_LEDA_ENABLE); if (ret < 0) goto out_i2c_err; - return bl->props.brightness; + return 0; out_i2c_err: dev_err(pchip->dev, "i2c failed to access\n"); @@ -278,7 +278,7 @@ static int lm3630a_bank_b_update_status(struct backlight_device *bl) LM3630A_LEDB_ENABLE, LM3630A_LEDB_ENABLE); if (ret < 0) goto out_i2c_err; - return bl->props.brightness; + return 0; out_i2c_err: dev_err(pchip->dev, "i2c failed to access REG_CTRL\n"); -- 2.20.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v5 1/3] backlight: lm3630a: return 0 on success in update_status functions @ 2019-04-18 15:11 ` Brian Masney 0 siblings, 0 replies; 23+ messages in thread From: Brian Masney @ 2019-04-18 15:11 UTC (permalink / raw) To: lee.jones, daniel.thompson, jingoohan1, robh+dt Cc: jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, dmurphy, jonathan lm3630a_bank_a_update_status() and lm3630a_bank_b_update_status() both return the brightness value if the brightness was successfully updated. Writing to these attributes via sysfs would cause a 'Bad address' error to be returned. These functions should return 0 on success, so let's change it to correct that error. Signed-off-by: Brian Masney <masneyb@onstation.org> Fixes: 28e64a68a2ef ("backlight: lm3630: apply chip revision") Acked-by: Pavel Machek <pavel@ucw.cz> --- drivers/video/backlight/lm3630a_bl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c index 2030a6b77a09..ef2553f452ca 100644 --- a/drivers/video/backlight/lm3630a_bl.c +++ b/drivers/video/backlight/lm3630a_bl.c @@ -201,7 +201,7 @@ static int lm3630a_bank_a_update_status(struct backlight_device *bl) LM3630A_LEDA_ENABLE, LM3630A_LEDA_ENABLE); if (ret < 0) goto out_i2c_err; - return bl->props.brightness; + return 0; out_i2c_err: dev_err(pchip->dev, "i2c failed to access\n"); @@ -278,7 +278,7 @@ static int lm3630a_bank_b_update_status(struct backlight_device *bl) LM3630A_LEDB_ENABLE, LM3630A_LEDB_ENABLE); if (ret < 0) goto out_i2c_err; - return bl->props.brightness; + return 0; out_i2c_err: dev_err(pchip->dev, "i2c failed to access REG_CTRL\n"); -- 2.20.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v5 2/3] dt-bindings: backlight: add lm3630a bindings 2019-04-18 15:11 ` Brian Masney @ 2019-04-18 15:11 ` Brian Masney -1 siblings, 0 replies; 23+ messages in thread From: Brian Masney @ 2019-04-18 15:11 UTC (permalink / raw) To: lee.jones, daniel.thompson, jingoohan1, robh+dt Cc: jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, dmurphy, jonathan Add new backlight bindings for the TI LM3630A dual-string white LED. Signed-off-by: Brian Masney <masneyb@onstation.org> --- Changes since v4: - Drop $ref from led-sources - Drop description from reg of i2c address - Expand description of reg for the control bank - Drop status from examples Changes since v3: - Add label. I didn't add a description for it since that'll come from the common properties once its converted. Changes since v2: - Update description of max-brightness - Add description for reg - Correct typo: s/tranisiton/transition - add reg to control banks - add additionalProperties .../leds/backlight/lm3630a-backlight.yaml | 129 ++++++++++++++++++ 1 file changed, 129 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 000000000000..e754df569365 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml @@ -0,0 +1,129 @@ +# 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 <lee.jones@linaro.org> + - Daniel Thompson <daniel.thompson@linaro.org> + - Jingoo Han <jingoohan1@gmail.com> + +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: + 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: | + The control bank that is used to program the two current sinks. The + LM3630A has two control banks (A and B) and are represented as 0 or 1 + in this property. The two current sinks can be controlled + independently with both banks, or bank A can be configured to control + both sinks with the led-sources property. + maxItems: 1 + minimum: 0 + maximum: 1 + + label: + maxItems: 1 + + led-sources: + allOf: + - 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"; + reg = <0x38>; + + #address-cells = <1>; + #size-cells = <0>; + + led@0 { + reg = <0>; + led-sources = <0 1>; + label = "lcd-backlight"; + default-brightness = <200>; + max-brightness = <255>; + }; + }; + }; + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + lm3630a_bl@38 { + compatible = "ti,lm3630a"; + reg = <0x38>; + + #address-cells = <1>; + #size-cells = <0>; + + led@0 { + reg = <0>; + default-brightness = <150>; + ti,linear-mapping-mode; + }; + + led@1 { + reg = <1>; + default-brightness = <225>; + ti,linear-mapping-mode; + }; + }; + }; -- 2.20.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v5 2/3] dt-bindings: backlight: add lm3630a bindings @ 2019-04-18 15:11 ` Brian Masney 0 siblings, 0 replies; 23+ messages in thread From: Brian Masney @ 2019-04-18 15:11 UTC (permalink / raw) To: lee.jones, daniel.thompson, jingoohan1, robh+dt Cc: jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, dmurphy, jonathan Add new backlight bindings for the TI LM3630A dual-string white LED. Signed-off-by: Brian Masney <masneyb@onstation.org> --- Changes since v4: - Drop $ref from led-sources - Drop description from reg of i2c address - Expand description of reg for the control bank - Drop status from examples Changes since v3: - Add label. I didn't add a description for it since that'll come from the common properties once its converted. Changes since v2: - Update description of max-brightness - Add description for reg - Correct typo: s/tranisiton/transition - add reg to control banks - add additionalProperties .../leds/backlight/lm3630a-backlight.yaml | 129 ++++++++++++++++++ 1 file changed, 129 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 000000000000..e754df569365 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml @@ -0,0 +1,129 @@ +# 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 <lee.jones@linaro.org> + - Daniel Thompson <daniel.thompson@linaro.org> + - Jingoo Han <jingoohan1@gmail.com> + +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: + 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: | + The control bank that is used to program the two current sinks. The + LM3630A has two control banks (A and B) and are represented as 0 or 1 + in this property. The two current sinks can be controlled + independently with both banks, or bank A can be configured to control + both sinks with the led-sources property. + maxItems: 1 + minimum: 0 + maximum: 1 + + label: + maxItems: 1 + + led-sources: + allOf: + - 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"; + reg = <0x38>; + + #address-cells = <1>; + #size-cells = <0>; + + led@0 { + reg = <0>; + led-sources = <0 1>; + label = "lcd-backlight"; + default-brightness = <200>; + max-brightness = <255>; + }; + }; + }; + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + lm3630a_bl@38 { + compatible = "ti,lm3630a"; + reg = <0x38>; + + #address-cells = <1>; + #size-cells = <0>; + + led@0 { + reg = <0>; + default-brightness = <150>; + ti,linear-mapping-mode; + }; + + led@1 { + reg = <1>; + default-brightness = <225>; + ti,linear-mapping-mode; + }; + }; + }; -- 2.20.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v5 2/3] dt-bindings: backlight: add lm3630a bindings 2019-04-18 15:11 ` Brian Masney @ 2019-04-24 1:52 ` Rob Herring -1 siblings, 0 replies; 23+ messages in thread From: Rob Herring @ 2019-04-24 1:52 UTC (permalink / raw) To: Brian Masney Cc: Lee Jones, Daniel Thompson, Jingoo Han, Jacek Anaszewski, Pavel Machek, Mark Rutland, Bartlomiej Zolnierkiewicz, dri-devel, Linux LED Subsystem, devicetree, linux-kernel, Linux Fbdev development list, Dan Murphy, Jonathan Marek On Thu, Apr 18, 2019 at 10:12 AM Brian Masney <masneyb@onstation.org> wrote: > > Add new backlight bindings for the TI LM3630A dual-string white LED. > > Signed-off-by: Brian Masney <masneyb@onstation.org> > --- > Changes since v4: > - Drop $ref from led-sources > - Drop description from reg of i2c address > - Expand description of reg for the control bank > - Drop status from examples > > Changes since v3: > - Add label. I didn't add a description for it since that'll come from > the common properties once its converted. > > Changes since v2: > - Update description of max-brightness > - Add description for reg > - Correct typo: s/tranisiton/transition > - add reg to control banks > - add additionalProperties > > .../leds/backlight/lm3630a-backlight.yaml | 129 ++++++++++++++++++ > 1 file changed, 129 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 000000000000..e754df569365 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml > @@ -0,0 +1,129 @@ > +# 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 <lee.jones@linaro.org> > + - Daniel Thompson <daniel.thompson@linaro.org> > + - Jingoo Han <jingoohan1@gmail.com> > + > +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: > + 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: | > + The control bank that is used to program the two current sinks. The > + LM3630A has two control banks (A and B) and are represented as 0 or 1 > + in this property. The two current sinks can be controlled > + independently with both banks, or bank A can be configured to control > + both sinks with the led-sources property. > + maxItems: 1 > + minimum: 0 > + maximum: 1 > + > + label: > + maxItems: 1 > + > + led-sources: > + allOf: > + - 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 { Still needs: led-controller@38 > + compatible = "ti,lm3630a"; > + reg = <0x38>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + led@0 { > + reg = <0>; > + led-sources = <0 1>; > + label = "lcd-backlight"; > + default-brightness = <200>; > + max-brightness = <255>; > + }; > + }; > + }; > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + lm3630a_bl@38 { And here. With those fixed: Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 2/3] dt-bindings: backlight: add lm3630a bindings @ 2019-04-24 1:52 ` Rob Herring 0 siblings, 0 replies; 23+ messages in thread From: Rob Herring @ 2019-04-24 1:52 UTC (permalink / raw) To: Brian Masney Cc: Lee Jones, Daniel Thompson, Jingoo Han, Jacek Anaszewski, Pavel Machek, Mark Rutland, Bartlomiej Zolnierkiewicz, dri-devel, Linux LED Subsystem, devicetree, linux-kernel, Linux Fbdev development list, Dan Murphy, Jonathan Marek On Thu, Apr 18, 2019 at 10:12 AM Brian Masney <masneyb@onstation.org> wrote: > > Add new backlight bindings for the TI LM3630A dual-string white LED. > > Signed-off-by: Brian Masney <masneyb@onstation.org> > --- > Changes since v4: > - Drop $ref from led-sources > - Drop description from reg of i2c address > - Expand description of reg for the control bank > - Drop status from examples > > Changes since v3: > - Add label. I didn't add a description for it since that'll come from > the common properties once its converted. > > Changes since v2: > - Update description of max-brightness > - Add description for reg > - Correct typo: s/tranisiton/transition > - add reg to control banks > - add additionalProperties > > .../leds/backlight/lm3630a-backlight.yaml | 129 ++++++++++++++++++ > 1 file changed, 129 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 000000000000..e754df569365 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml > @@ -0,0 +1,129 @@ > +# 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 <lee.jones@linaro.org> > + - Daniel Thompson <daniel.thompson@linaro.org> > + - Jingoo Han <jingoohan1@gmail.com> > + > +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: > + 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: | > + The control bank that is used to program the two current sinks. The > + LM3630A has two control banks (A and B) and are represented as 0 or 1 > + in this property. The two current sinks can be controlled > + independently with both banks, or bank A can be configured to control > + both sinks with the led-sources property. > + maxItems: 1 > + minimum: 0 > + maximum: 1 > + > + label: > + maxItems: 1 > + > + led-sources: > + allOf: > + - 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 { Still needs: led-controller@38 > + compatible = "ti,lm3630a"; > + reg = <0x38>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + led@0 { > + reg = <0>; > + led-sources = <0 1>; > + label = "lcd-backlight"; > + default-brightness = <200>; > + max-brightness = <255>; > + }; > + }; > + }; > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + lm3630a_bl@38 { And here. With those fixed: Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v5 3/3] backlight: lm3630a: add firmware node support 2019-04-18 15:11 ` Brian Masney @ 2019-04-18 15:11 ` Brian Masney -1 siblings, 0 replies; 23+ messages in thread From: Brian Masney @ 2019-04-18 15:11 UTC (permalink / raw) To: lee.jones, daniel.thompson, jingoohan1, robh+dt Cc: jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, dmurphy, jonathan 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 <masneyb@onstation.org> --- 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_MSEC 10 + +#define LM3630A_BANK_0 0 +#define LM3630A_BANK_1 1 + +#define LM3630A_NUM_SINKS 2 +#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; + + 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) +{ + int led_sources, ret; + const char *label; + u32 bank, val; + bool linear; + + ret = fwnode_property_read_u32(node, "reg", &bank); + if (ret) + return ret; + + if (bank < LM3630A_BANK_0 || bank > LM3630A_BANK_1) + return -EINVAL; + + led_sources = lm3630a_parse_led_sources(node, BIT(bank)); + if (led_sources < 0) + return led_sources; + + if (*seen_led_sources & led_sources) + return -EINVAL; + + *seen_led_sources |= led_sources; + + linear = fwnode_property_read_bool(node, + "ti,linear-mapping-mode"); + if (bank) { + if (led_sources & BIT(LM3630A_SINK_0) || + !(led_sources & BIT(LM3630A_SINK_1))) + return -EINVAL; + + pdata->ledb_ctrl = linear ? + LM3630A_LEDB_ENABLE_LINEAR : + LM3630A_LEDB_ENABLE; + } else { + if (!(led_sources & BIT(LM3630A_SINK_0))) + return -EINVAL; + + pdata->leda_ctrl = linear ? + LM3630A_LEDA_ENABLE_LINEAR : + LM3630A_LEDA_ENABLE; + + if (led_sources & BIT(LM3630A_SINK_1)) + pdata->ledb_ctrl = LM3630A_LEDB_ON_A; + } + + ret = fwnode_property_read_string(node, "label", &label); + if (!ret) { + if (bank) + pdata->ledb_label = label; + else + pdata->leda_label = label; + } + + ret = fwnode_property_read_u32(node, "default-brightness", + &val); + if (!ret) { + if (bank) + pdata->ledb_init_brt = val; + else + pdata->leda_init_brt = val; + } + + ret = fwnode_property_read_u32(node, "max-brightness", &val); + if (!ret) { + if (bank) + pdata->ledb_max_brt = val; + else + pdata->leda_max_brt = val; + } + + return 0; +} + +static int lm3630a_parse_node(struct lm3630a_chip *pchip, + struct lm3630a_platform_data *pdata) +{ + int ret = -ENODEV, seen_led_sources = 0; + struct fwnode_handle *node; + + device_for_each_child_node(pchip->dev, node) { + ret = lm3630a_parse_bank(pdata, node, &seen_led_sources); + if (ret) + return ret; + } + + return ret; +} + static int lm3630a_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -396,13 +524,18 @@ 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_max_brt = LM3630A_MAX_BRIGHTNESS; pdata->ledb_max_brt = LM3630A_MAX_BRIGHTNESS; pdata->leda_init_brt = LM3630A_MAX_BRIGHTNESS; pdata->ledb_init_brt = LM3630A_MAX_BRIGHTNESS; + + rval = lm3630a_parse_node(pchip, pdata); + if (rval) { + dev_err(&client->dev, "fail : parse node\n"); + return rval; + } } pchip->pdata = pdata; @@ -470,11 +603,17 @@ 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, diff --git a/include/linux/platform_data/lm3630a_bl.h b/include/linux/platform_data/lm3630a_bl.h index 7538e38e270b..762e68956f31 100644 --- a/include/linux/platform_data/lm3630a_bl.h +++ b/include/linux/platform_data/lm3630a_bl.h @@ -38,9 +38,11 @@ enum lm3630a_ledb_ctrl { #define LM3630A_MAX_BRIGHTNESS 255 /* + *@leda_label : optional led a label. *@leda_init_brt : led a init brightness. 4~255 *@leda_max_brt : led a max brightness. 4~255 *@leda_ctrl : led a disable, enable linear, enable exponential + *@ledb_label : optional led b label. *@ledb_init_brt : led b init brightness. 4~255 *@ledb_max_brt : led b max brightness. 4~255 *@ledb_ctrl : led b disable, enable linear, enable exponential @@ -50,10 +52,12 @@ enum lm3630a_ledb_ctrl { struct lm3630a_platform_data { /* led a config. */ + const char *leda_label; int leda_init_brt; int leda_max_brt; enum lm3630a_leda_ctrl leda_ctrl; /* led b config. */ + const char *ledb_label; int ledb_init_brt; int ledb_max_brt; enum lm3630a_ledb_ctrl ledb_ctrl; -- 2.20.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v5 3/3] backlight: lm3630a: add firmware node support @ 2019-04-18 15:11 ` Brian Masney 0 siblings, 0 replies; 23+ messages in thread From: Brian Masney @ 2019-04-18 15:11 UTC (permalink / raw) To: lee.jones, daniel.thompson, jingoohan1, robh+dt Cc: jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, dmurphy, jonathan 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 <masneyb@onstation.org> --- 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_MSEC 10 + +#define LM3630A_BANK_0 0 +#define LM3630A_BANK_1 1 + +#define LM3630A_NUM_SINKS 2 +#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; + + 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) +{ + int led_sources, ret; + const char *label; + u32 bank, val; + bool linear; + + ret = fwnode_property_read_u32(node, "reg", &bank); + if (ret) + return ret; + + if (bank < LM3630A_BANK_0 || bank > LM3630A_BANK_1) + return -EINVAL; + + led_sources = lm3630a_parse_led_sources(node, BIT(bank)); + if (led_sources < 0) + return led_sources; + + if (*seen_led_sources & led_sources) + return -EINVAL; + + *seen_led_sources |= led_sources; + + linear = fwnode_property_read_bool(node, + "ti,linear-mapping-mode"); + if (bank) { + if (led_sources & BIT(LM3630A_SINK_0) || + !(led_sources & BIT(LM3630A_SINK_1))) + return -EINVAL; + + pdata->ledb_ctrl = linear ? + LM3630A_LEDB_ENABLE_LINEAR : + LM3630A_LEDB_ENABLE; + } else { + if (!(led_sources & BIT(LM3630A_SINK_0))) + return -EINVAL; + + pdata->leda_ctrl = linear ? + LM3630A_LEDA_ENABLE_LINEAR : + LM3630A_LEDA_ENABLE; + + if (led_sources & BIT(LM3630A_SINK_1)) + pdata->ledb_ctrl = LM3630A_LEDB_ON_A; + } + + ret = fwnode_property_read_string(node, "label", &label); + if (!ret) { + if (bank) + pdata->ledb_label = label; + else + pdata->leda_label = label; + } + + ret = fwnode_property_read_u32(node, "default-brightness", + &val); + if (!ret) { + if (bank) + pdata->ledb_init_brt = val; + else + pdata->leda_init_brt = val; + } + + ret = fwnode_property_read_u32(node, "max-brightness", &val); + if (!ret) { + if (bank) + pdata->ledb_max_brt = val; + else + pdata->leda_max_brt = val; + } + + return 0; +} + +static int lm3630a_parse_node(struct lm3630a_chip *pchip, + struct lm3630a_platform_data *pdata) +{ + int ret = -ENODEV, seen_led_sources = 0; + struct fwnode_handle *node; + + device_for_each_child_node(pchip->dev, node) { + ret = lm3630a_parse_bank(pdata, node, &seen_led_sources); + if (ret) + return ret; + } + + return ret; +} + static int lm3630a_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -396,13 +524,18 @@ 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_max_brt = LM3630A_MAX_BRIGHTNESS; pdata->ledb_max_brt = LM3630A_MAX_BRIGHTNESS; pdata->leda_init_brt = LM3630A_MAX_BRIGHTNESS; pdata->ledb_init_brt = LM3630A_MAX_BRIGHTNESS; + + rval = lm3630a_parse_node(pchip, pdata); + if (rval) { + dev_err(&client->dev, "fail : parse node\n"); + return rval; + } } pchip->pdata = pdata; @@ -470,11 +603,17 @@ 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, diff --git a/include/linux/platform_data/lm3630a_bl.h b/include/linux/platform_data/lm3630a_bl.h index 7538e38e270b..762e68956f31 100644 --- a/include/linux/platform_data/lm3630a_bl.h +++ b/include/linux/platform_data/lm3630a_bl.h @@ -38,9 +38,11 @@ enum lm3630a_ledb_ctrl { #define LM3630A_MAX_BRIGHTNESS 255 /* + *@leda_label : optional led a label. *@leda_init_brt : led a init brightness. 4~255 *@leda_max_brt : led a max brightness. 4~255 *@leda_ctrl : led a disable, enable linear, enable exponential + *@ledb_label : optional led b label. *@ledb_init_brt : led b init brightness. 4~255 *@ledb_max_brt : led b max brightness. 4~255 *@ledb_ctrl : led b disable, enable linear, enable exponential @@ -50,10 +52,12 @@ enum lm3630a_ledb_ctrl { struct lm3630a_platform_data { /* led a config. */ + const char *leda_label; int leda_init_brt; int leda_max_brt; enum lm3630a_leda_ctrl leda_ctrl; /* led b config. */ + const char *ledb_label; int ledb_init_brt; int ledb_max_brt; enum lm3630a_ledb_ctrl ledb_ctrl; -- 2.20.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v5 3/3] backlight: lm3630a: add firmware node support 2019-04-18 15:11 ` Brian Masney (?) @ 2019-04-23 13:49 ` Dan Murphy -1 siblings, 0 replies; 23+ messages in thread From: Dan Murphy @ 2019-04-23 13:49 UTC (permalink / raw) To: Brian Masney, lee.jones, daniel.thompson, jingoohan1, robh+dt Cc: jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, jonathan 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 <masneyb@onstation.org> > --- > 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_MSEC 10 > + > +#define LM3630A_BANK_0 0 > +#define LM3630A_BANK_1 1 > + > +#define LM3630A_NUM_SINKS 2 > +#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; > + > + 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. Dan > +{ > + int led_sources, ret; > + const char *label; > + u32 bank, val; > + bool linear; > + > + ret = fwnode_property_read_u32(node, "reg", &bank); > + if (ret) > + return ret; > + > + if (bank < LM3630A_BANK_0 || bank > LM3630A_BANK_1) > + return -EINVAL; > + > + led_sources = lm3630a_parse_led_sources(node, BIT(bank)); > + if (led_sources < 0) > + return led_sources; > + > + if (*seen_led_sources & led_sources) > + return -EINVAL; > + > + *seen_led_sources |= led_sources; > + > + linear = fwnode_property_read_bool(node, > + "ti,linear-mapping-mode"); > + if (bank) { > + if (led_sources & BIT(LM3630A_SINK_0) || > + !(led_sources & BIT(LM3630A_SINK_1))) > + return -EINVAL; > + > + pdata->ledb_ctrl = linear ? > + LM3630A_LEDB_ENABLE_LINEAR : > + LM3630A_LEDB_ENABLE; > + } else { > + if (!(led_sources & BIT(LM3630A_SINK_0))) > + return -EINVAL; > + > + pdata->leda_ctrl = linear ? > + LM3630A_LEDA_ENABLE_LINEAR : > + LM3630A_LEDA_ENABLE; > + > + if (led_sources & BIT(LM3630A_SINK_1)) > + pdata->ledb_ctrl = LM3630A_LEDB_ON_A; > + } > + > + ret = fwnode_property_read_string(node, "label", &label); > + if (!ret) { > + if (bank) > + pdata->ledb_label = label; > + else > + pdata->leda_label = label; > + } > + > + ret = fwnode_property_read_u32(node, "default-brightness", > + &val); > + if (!ret) { > + if (bank) > + pdata->ledb_init_brt = val; > + else > + pdata->leda_init_brt = val; > + } > + > + ret = fwnode_property_read_u32(node, "max-brightness", &val); > + if (!ret) { > + if (bank) > + pdata->ledb_max_brt = val; > + else > + pdata->leda_max_brt = val; > + } > + > + return 0; > +} > + > +static int lm3630a_parse_node(struct lm3630a_chip *pchip, > + struct lm3630a_platform_data *pdata) > +{ > + int ret = -ENODEV, seen_led_sources = 0; > + struct fwnode_handle *node; > + > + device_for_each_child_node(pchip->dev, node) { > + ret = lm3630a_parse_bank(pdata, node, &seen_led_sources); > + if (ret) > + return ret; > + } > + > + return ret; > +} > + > static int lm3630a_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > @@ -396,13 +524,18 @@ 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_max_brt = LM3630A_MAX_BRIGHTNESS; > pdata->ledb_max_brt = LM3630A_MAX_BRIGHTNESS; > pdata->leda_init_brt = LM3630A_MAX_BRIGHTNESS; > pdata->ledb_init_brt = LM3630A_MAX_BRIGHTNESS; > + > + rval = lm3630a_parse_node(pchip, pdata); > + if (rval) { > + dev_err(&client->dev, "fail : parse node\n"); > + return rval; > + } > } > pchip->pdata = pdata; > > @@ -470,11 +603,17 @@ 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, > diff --git a/include/linux/platform_data/lm3630a_bl.h b/include/linux/platform_data/lm3630a_bl.h > index 7538e38e270b..762e68956f31 100644 > --- a/include/linux/platform_data/lm3630a_bl.h > +++ b/include/linux/platform_data/lm3630a_bl.h > @@ -38,9 +38,11 @@ enum lm3630a_ledb_ctrl { > > #define LM3630A_MAX_BRIGHTNESS 255 > /* > + *@leda_label : optional led a label. > *@leda_init_brt : led a init brightness. 4~255 > *@leda_max_brt : led a max brightness. 4~255 > *@leda_ctrl : led a disable, enable linear, enable exponential > + *@ledb_label : optional led b label. > *@ledb_init_brt : led b init brightness. 4~255 > *@ledb_max_brt : led b max brightness. 4~255 > *@ledb_ctrl : led b disable, enable linear, enable exponential > @@ -50,10 +52,12 @@ enum lm3630a_ledb_ctrl { > struct lm3630a_platform_data { > > /* led a config. */ > + const char *leda_label; > int leda_init_brt; > int leda_max_brt; > enum lm3630a_leda_ctrl leda_ctrl; > /* led b config. */ > + const char *ledb_label; > int ledb_init_brt; > int ledb_max_brt; > enum lm3630a_ledb_ctrl ledb_ctrl; > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 3/3] backlight: lm3630a: add firmware node support @ 2019-04-23 13:49 ` Dan Murphy 0 siblings, 0 replies; 23+ messages in thread From: Dan Murphy @ 2019-04-23 13:49 UTC (permalink / raw) To: Brian Masney, lee.jones, daniel.thompson, jingoohan1, robh+dt Cc: jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, jonathan 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 <masneyb@onstation.org> > --- > 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_MSEC 10 > + > +#define LM3630A_BANK_0 0 > +#define LM3630A_BANK_1 1 > + > +#define LM3630A_NUM_SINKS 2 > +#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; > + > + 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. Dan > +{ > + int led_sources, ret; > + const char *label; > + u32 bank, val; > + bool linear; > + > + ret = fwnode_property_read_u32(node, "reg", &bank); > + if (ret) > + return ret; > + > + if (bank < LM3630A_BANK_0 || bank > LM3630A_BANK_1) > + return -EINVAL; > + > + led_sources = lm3630a_parse_led_sources(node, BIT(bank)); > + if (led_sources < 0) > + return led_sources; > + > + if (*seen_led_sources & led_sources) > + return -EINVAL; > + > + *seen_led_sources |= led_sources; > + > + linear = fwnode_property_read_bool(node, > + "ti,linear-mapping-mode"); > + if (bank) { > + if (led_sources & BIT(LM3630A_SINK_0) || > + !(led_sources & BIT(LM3630A_SINK_1))) > + return -EINVAL; > + > + pdata->ledb_ctrl = linear ? > + LM3630A_LEDB_ENABLE_LINEAR : > + LM3630A_LEDB_ENABLE; > + } else { > + if (!(led_sources & BIT(LM3630A_SINK_0))) > + return -EINVAL; > + > + pdata->leda_ctrl = linear ? > + LM3630A_LEDA_ENABLE_LINEAR : > + LM3630A_LEDA_ENABLE; > + > + if (led_sources & BIT(LM3630A_SINK_1)) > + pdata->ledb_ctrl = LM3630A_LEDB_ON_A; > + } > + > + ret = fwnode_property_read_string(node, "label", &label); > + if (!ret) { > + if (bank) > + pdata->ledb_label = label; > + else > + pdata->leda_label = label; > + } > + > + ret = fwnode_property_read_u32(node, "default-brightness", > + &val); > + if (!ret) { > + if (bank) > + pdata->ledb_init_brt = val; > + else > + pdata->leda_init_brt = val; > + } > + > + ret = fwnode_property_read_u32(node, "max-brightness", &val); > + if (!ret) { > + if (bank) > + pdata->ledb_max_brt = val; > + else > + pdata->leda_max_brt = val; > + } > + > + return 0; > +} > + > +static int lm3630a_parse_node(struct lm3630a_chip *pchip, > + struct lm3630a_platform_data *pdata) > +{ > + int ret = -ENODEV, seen_led_sources = 0; > + struct fwnode_handle *node; > + > + device_for_each_child_node(pchip->dev, node) { > + ret = lm3630a_parse_bank(pdata, node, &seen_led_sources); > + if (ret) > + return ret; > + } > + > + return ret; > +} > + > static int lm3630a_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > @@ -396,13 +524,18 @@ 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_max_brt = LM3630A_MAX_BRIGHTNESS; > pdata->ledb_max_brt = LM3630A_MAX_BRIGHTNESS; > pdata->leda_init_brt = LM3630A_MAX_BRIGHTNESS; > pdata->ledb_init_brt = LM3630A_MAX_BRIGHTNESS; > + > + rval = lm3630a_parse_node(pchip, pdata); > + if (rval) { > + dev_err(&client->dev, "fail : parse node\n"); > + return rval; > + } > } > pchip->pdata = pdata; > > @@ -470,11 +603,17 @@ 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, > diff --git a/include/linux/platform_data/lm3630a_bl.h b/include/linux/platform_data/lm3630a_bl.h > index 7538e38e270b..762e68956f31 100644 > --- a/include/linux/platform_data/lm3630a_bl.h > +++ b/include/linux/platform_data/lm3630a_bl.h > @@ -38,9 +38,11 @@ enum lm3630a_ledb_ctrl { > > #define LM3630A_MAX_BRIGHTNESS 255 > /* > + *@leda_label : optional led a label. > *@leda_init_brt : led a init brightness. 4~255 > *@leda_max_brt : led a max brightness. 4~255 > *@leda_ctrl : led a disable, enable linear, enable exponential > + *@ledb_label : optional led b label. > *@ledb_init_brt : led b init brightness. 4~255 > *@ledb_max_brt : led b max brightness. 4~255 > *@ledb_ctrl : led b disable, enable linear, enable exponential > @@ -50,10 +52,12 @@ enum lm3630a_ledb_ctrl { > struct lm3630a_platform_data { > > /* led a config. */ > + const char *leda_label; > int leda_init_brt; > int leda_max_brt; > enum lm3630a_leda_ctrl leda_ctrl; > /* led b config. */ > + const char *ledb_label; > int ledb_init_brt; > int ledb_max_brt; > enum lm3630a_ledb_ctrl ledb_ctrl; > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 3/3] backlight: lm3630a: add firmware node support @ 2019-04-23 13:49 ` Dan Murphy 0 siblings, 0 replies; 23+ messages in thread From: Dan Murphy @ 2019-04-23 13:49 UTC (permalink / raw) To: Brian Masney, lee.jones, daniel.thompson, jingoohan1, robh+dt Cc: jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, jonathan 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 <masneyb@onstation.org> > --- > 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_MSEC 10 > + > +#define LM3630A_BANK_0 0 > +#define LM3630A_BANK_1 1 > + > +#define LM3630A_NUM_SINKS 2 > +#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; > + > + 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. Dan > +{ > + int led_sources, ret; > + const char *label; > + u32 bank, val; > + bool linear; > + > + ret = fwnode_property_read_u32(node, "reg", &bank); > + if (ret) > + return ret; > + > + if (bank < LM3630A_BANK_0 || bank > LM3630A_BANK_1) > + return -EINVAL; > + > + led_sources = lm3630a_parse_led_sources(node, BIT(bank)); > + if (led_sources < 0) > + return led_sources; > + > + if (*seen_led_sources & led_sources) > + return -EINVAL; > + > + *seen_led_sources |= led_sources; > + > + linear = fwnode_property_read_bool(node, > + "ti,linear-mapping-mode"); > + if (bank) { > + if (led_sources & BIT(LM3630A_SINK_0) || > + !(led_sources & BIT(LM3630A_SINK_1))) > + return -EINVAL; > + > + pdata->ledb_ctrl = linear ? > + LM3630A_LEDB_ENABLE_LINEAR : > + LM3630A_LEDB_ENABLE; > + } else { > + if (!(led_sources & BIT(LM3630A_SINK_0))) > + return -EINVAL; > + > + pdata->leda_ctrl = linear ? > + LM3630A_LEDA_ENABLE_LINEAR : > + LM3630A_LEDA_ENABLE; > + > + if (led_sources & BIT(LM3630A_SINK_1)) > + pdata->ledb_ctrl = LM3630A_LEDB_ON_A; > + } > + > + ret = fwnode_property_read_string(node, "label", &label); > + if (!ret) { > + if (bank) > + pdata->ledb_label = label; > + else > + pdata->leda_label = label; > + } > + > + ret = fwnode_property_read_u32(node, "default-brightness", > + &val); > + if (!ret) { > + if (bank) > + pdata->ledb_init_brt = val; > + else > + pdata->leda_init_brt = val; > + } > + > + ret = fwnode_property_read_u32(node, "max-brightness", &val); > + if (!ret) { > + if (bank) > + pdata->ledb_max_brt = val; > + else > + pdata->leda_max_brt = val; > + } > + > + return 0; > +} > + > +static int lm3630a_parse_node(struct lm3630a_chip *pchip, > + struct lm3630a_platform_data *pdata) > +{ > + int ret = -ENODEV, seen_led_sources = 0; > + struct fwnode_handle *node; > + > + device_for_each_child_node(pchip->dev, node) { > + ret = lm3630a_parse_bank(pdata, node, &seen_led_sources); > + if (ret) > + return ret; > + } > + > + return ret; > +} > + > static int lm3630a_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > @@ -396,13 +524,18 @@ 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_max_brt = LM3630A_MAX_BRIGHTNESS; > pdata->ledb_max_brt = LM3630A_MAX_BRIGHTNESS; > pdata->leda_init_brt = LM3630A_MAX_BRIGHTNESS; > pdata->ledb_init_brt = LM3630A_MAX_BRIGHTNESS; > + > + rval = lm3630a_parse_node(pchip, pdata); > + if (rval) { > + dev_err(&client->dev, "fail : parse node\n"); > + return rval; > + } > } > pchip->pdata = pdata; > > @@ -470,11 +603,17 @@ 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, > diff --git a/include/linux/platform_data/lm3630a_bl.h b/include/linux/platform_data/lm3630a_bl.h > index 7538e38e270b..762e68956f31 100644 > --- a/include/linux/platform_data/lm3630a_bl.h > +++ b/include/linux/platform_data/lm3630a_bl.h > @@ -38,9 +38,11 @@ enum lm3630a_ledb_ctrl { > > #define LM3630A_MAX_BRIGHTNESS 255 > /* > + *@leda_label : optional led a label. > *@leda_init_brt : led a init brightness. 4~255 > *@leda_max_brt : led a max brightness. 4~255 > *@leda_ctrl : led a disable, enable linear, enable exponential > + *@ledb_label : optional led b label. > *@ledb_init_brt : led b init brightness. 4~255 > *@ledb_max_brt : led b max brightness. 4~255 > *@ledb_ctrl : led b disable, enable linear, enable exponential > @@ -50,10 +52,12 @@ enum lm3630a_ledb_ctrl { > struct lm3630a_platform_data { > > /* led a config. */ > + const char *leda_label; > int leda_init_brt; > int leda_max_brt; > enum lm3630a_leda_ctrl leda_ctrl; > /* led b config. */ > + const char *ledb_label; > int ledb_init_brt; > int ledb_max_brt; > enum lm3630a_ledb_ctrl ledb_ctrl; > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 3/3] backlight: lm3630a: add firmware node support 2019-04-23 13:49 ` Dan Murphy @ 2019-04-23 14:01 ` Brian Masney -1 siblings, 0 replies; 23+ messages in thread From: Brian Masney @ 2019-04-23 14:01 UTC (permalink / raw) To: Dan Murphy Cc: lee.jones, daniel.thompson, jingoohan1, robh+dt, jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, jonathan 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: led@0 { reg = <0>; led-sources = <0 1>; label = "lcd-backlight"; default-brightness = <200>; }; led@1 { reg = <1>; default-brightness = <150>; }; Brian ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 3/3] backlight: lm3630a: add firmware node support @ 2019-04-23 14:01 ` Brian Masney 0 siblings, 0 replies; 23+ messages in thread From: Brian Masney @ 2019-04-23 14:01 UTC (permalink / raw) To: Dan Murphy Cc: lee.jones, daniel.thompson, jingoohan1, robh+dt, jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, jonathan 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: led@0 { reg = <0>; led-sources = <0 1>; label = "lcd-backlight"; default-brightness = <200>; }; led@1 { reg = <1>; default-brightness = <150>; }; Brian ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 3/3] backlight: lm3630a: add firmware node support 2019-04-23 14:01 ` Brian Masney (?) @ 2019-04-23 15:31 ` Dan Murphy -1 siblings, 0 replies; 23+ messages in thread From: Dan Murphy @ 2019-04-23 15:31 UTC (permalink / raw) To: Brian Masney Cc: lee.jones, daniel.thompson, jingoohan1, robh+dt, jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, jonathan 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 > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 3/3] backlight: lm3630a: add firmware node support @ 2019-04-23 15:31 ` Dan Murphy 0 siblings, 0 replies; 23+ messages in thread From: Dan Murphy @ 2019-04-23 15:31 UTC (permalink / raw) To: Brian Masney Cc: lee.jones, daniel.thompson, jingoohan1, robh+dt, jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, jonathan 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 > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 3/3] backlight: lm3630a: add firmware node support @ 2019-04-23 15:31 ` Dan Murphy 0 siblings, 0 replies; 23+ messages in thread From: Dan Murphy @ 2019-04-23 15:31 UTC (permalink / raw) To: Brian Masney Cc: lee.jones, daniel.thompson, jingoohan1, robh+dt, jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, jonathan 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 > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 3/3] backlight: lm3630a: add firmware node support 2019-04-23 15:31 ` Dan Murphy @ 2019-04-23 16:00 ` Brian Masney -1 siblings, 0 replies; 23+ messages in thread From: Brian Masney @ 2019-04-23 16:00 UTC (permalink / raw) To: Dan Murphy Cc: lee.jones, daniel.thompson, jingoohan1, robh+dt, jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, jonathan 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. Brian ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 3/3] backlight: lm3630a: add firmware node support @ 2019-04-23 16:00 ` Brian Masney 0 siblings, 0 replies; 23+ messages in thread From: Brian Masney @ 2019-04-23 16:00 UTC (permalink / raw) To: Dan Murphy Cc: lee.jones, daniel.thompson, jingoohan1, robh+dt, jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, jonathan 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. Brian ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 3/3] backlight: lm3630a: add firmware node support 2019-04-23 16:00 ` Brian Masney (?) @ 2019-04-23 19:02 ` Dan Murphy -1 siblings, 0 replies; 23+ messages in thread From: Dan Murphy @ 2019-04-23 19:02 UTC (permalink / raw) To: Brian Masney Cc: lee.jones, daniel.thompson, jingoohan1, robh+dt, jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, jonathan 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 <dmurphy@ti.com> > Brian > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 3/3] backlight: lm3630a: add firmware node support @ 2019-04-23 19:02 ` Dan Murphy 0 siblings, 0 replies; 23+ messages in thread From: Dan Murphy @ 2019-04-23 19:02 UTC (permalink / raw) To: Brian Masney Cc: lee.jones, daniel.thompson, jingoohan1, robh+dt, jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, jonathan 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 <dmurphy@ti.com> > Brian > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 3/3] backlight: lm3630a: add firmware node support @ 2019-04-23 19:02 ` Dan Murphy 0 siblings, 0 replies; 23+ messages in thread From: Dan Murphy @ 2019-04-23 19:02 UTC (permalink / raw) To: Brian Masney Cc: lee.jones, daniel.thompson, jingoohan1, robh+dt, jacek.anaszewski, pavel, mark.rutland, b.zolnierkie, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, jonathan 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 <dmurphy@ti.com> > Brian > ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2019-04-24 1:52 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-18 15:11 [PATCH v5 0/3] backlight: lm3630a: bug fix and fwnode support Brian Masney 2019-04-18 15:11 ` Brian Masney 2019-04-18 15:11 ` [PATCH v5 1/3] backlight: lm3630a: return 0 on success in update_status functions Brian Masney 2019-04-18 15:11 ` Brian Masney 2019-04-18 15:11 ` [PATCH v5 2/3] dt-bindings: backlight: add lm3630a bindings Brian Masney 2019-04-18 15:11 ` Brian Masney 2019-04-24 1:52 ` Rob Herring 2019-04-24 1:52 ` Rob Herring 2019-04-18 15:11 ` [PATCH v5 3/3] backlight: lm3630a: add firmware node support Brian Masney 2019-04-18 15:11 ` Brian Masney 2019-04-23 13:49 ` Dan Murphy 2019-04-23 13:49 ` Dan Murphy 2019-04-23 13:49 ` Dan Murphy 2019-04-23 14:01 ` Brian Masney 2019-04-23 14:01 ` Brian Masney 2019-04-23 15:31 ` Dan Murphy 2019-04-23 15:31 ` Dan Murphy 2019-04-23 15:31 ` Dan Murphy 2019-04-23 16:00 ` Brian Masney 2019-04-23 16:00 ` Brian Masney 2019-04-23 19:02 ` Dan Murphy 2019-04-23 19:02 ` Dan Murphy 2019-04-23 19:02 ` Dan Murphy
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.