| Message ID | 20211115091107.11737-1-zajec5@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] dt-bindings: leds: add Broadcom's BCM63xxx controller | expand |
On 11/15/21 1:11 AM, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > Broadcom used 2 LEDs hardware blocks for their BCM63xx SoCs: > 1. Older one (BCM6318, BCM6328, BCM6362, BCM63268, BCM6838) > 2. Newer one (BCM6848, BCM6858, BCM63138, BCM63148, BCM63381, BCM68360) Just so the existing pattern/regexps continue to work, I would be naming this "bcm63xx" to be consistent with the rest of existing code-base.
On 22.11.2021 22:51, Florian Fainelli wrote: > On 11/15/21 1:11 AM, Rafał Miłecki wrote: >> From: Rafał Miłecki <rafal@milecki.pl> >> >> Broadcom used 2 LEDs hardware blocks for their BCM63xx SoCs: >> 1. Older one (BCM6318, BCM6328, BCM6362, BCM63268, BCM6838) >> 2. Newer one (BCM6848, BCM6858, BCM63138, BCM63148, BCM63381, BCM68360) > > Just so the existing pattern/regexps continue to work, I would be naming > this "bcm63xx" to be consistent with the rest of existing code-base. The problem I saw with "bcm63xx" is that it seems to match all SoCs: those with old block and those with new block. So I guess both groups have the same right to use that "bcm63xx" based binding. To avoid favouring old or new block I decided to avoid "bcm63xx". Given above explanation: do you still prefer using "bcm63xx" based binding for the new block? I'm OK with that, I just want to make sure you're aware of that minor issue. Please let me know :)
On 11/22/21 2:00 PM, Rafał Miłecki wrote: > On 22.11.2021 22:51, Florian Fainelli wrote: >> On 11/15/21 1:11 AM, Rafał Miłecki wrote: >>> From: Rafał Miłecki <rafal@milecki.pl> >>> >>> Broadcom used 2 LEDs hardware blocks for their BCM63xx SoCs: >>> 1. Older one (BCM6318, BCM6328, BCM6362, BCM63268, BCM6838) >>> 2. Newer one (BCM6848, BCM6858, BCM63138, BCM63148, BCM63381, BCM68360) >> >> Just so the existing pattern/regexps continue to work, I would be naming >> this "bcm63xx" to be consistent with the rest of existing code-base. > > The problem I saw with "bcm63xx" is that it seems to match all SoCs: > those with old block and those with new block. So I guess both groups > have the same right to use that "bcm63xx" based binding. > > To avoid favouring old or new block I decided to avoid "bcm63xx". > > Given above explanation: do you still prefer using "bcm63xx" based > binding for the new block? I'm OK with that, I just want to make sure > you're aware of that minor issue. Please let me know :) Maybe we use leds-bcm63138.c then since this is the first chip in the list that featured that block, similar to how leds-bcm6328.c was created? Then my second choice would be leds-bcm63xx.c just so the existing patterns match, really and because it's easy to visually not be able to tell the difference between two x versus three x. Thanks
On 23.11.2021 23:17, Florian Fainelli wrote: > On 11/22/21 2:00 PM, Rafał Miłecki wrote: >> On 22.11.2021 22:51, Florian Fainelli wrote: >>> On 11/15/21 1:11 AM, Rafał Miłecki wrote: >>>> From: Rafał Miłecki <rafal@milecki.pl> >>>> >>>> Broadcom used 2 LEDs hardware blocks for their BCM63xx SoCs: >>>> 1. Older one (BCM6318, BCM6328, BCM6362, BCM63268, BCM6838) >>>> 2. Newer one (BCM6848, BCM6858, BCM63138, BCM63148, BCM63381, BCM68360) >>> >>> Just so the existing pattern/regexps continue to work, I would be naming >>> this "bcm63xx" to be consistent with the rest of existing code-base. >> >> The problem I saw with "bcm63xx" is that it seems to match all SoCs: >> those with old block and those with new block. So I guess both groups >> have the same right to use that "bcm63xx" based binding. >> >> To avoid favouring old or new block I decided to avoid "bcm63xx". >> >> Given above explanation: do you still prefer using "bcm63xx" based >> binding for the new block? I'm OK with that, I just want to make sure >> you're aware of that minor issue. Please let me know :) > > Maybe we use leds-bcm63138.c then since this is the first chip in the > list that featured that block, similar to how leds-bcm6328.c was > created? Then my second choice would be leds-bcm63xx.c just so the > existing patterns match, really and because it's easy to visually not be > able to tell the difference between two x versus three x. Sounds good to me, thanks for your review!
diff --git a/Documentation/devicetree/bindings/leds/leds-bcm63xxx.yaml b/Documentation/devicetree/bindings/leds/leds-bcm63xxx.yaml new file mode 100644 index 000000000000..3910dd607f3f --- /dev/null +++ b/Documentation/devicetree/bindings/leds/leds-bcm63xxx.yaml @@ -0,0 +1,94 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/leds-bcm63xxx.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Broadcom's BCM63xxx LEDs controller + +maintainers: + - Rafał Miłecki <rafal@milecki.pl> + +description: | + This LEDs controller is used on BCM4908, BCM6848, BCM6858, BCM63138, BCM63148, + BCM63381 and BCM68360. + + It supports up to 32 LEDs that can be connected parallelly or serially. It + also includes limited support for hardware blinking. + + Binding serially connected LEDs isn't documented yet. + +properties: + compatible: + items: + - enum: + - brcm,bcm4908-leds + - brcm,bcm6848-leds + - brcm,bcm6858-leds + - brcm,bcm63138-leds + - brcm,bcm63148-leds + - brcm,bcm63381-leds + - brcm,bcm68360-leds + - const: brcm,bcm63xxx-leds + + reg: + maxItems: 1 + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + +patternProperties: + "^led@[a-f0-9]+$": + type: object + + $ref: common.yaml# + + properties: + reg: + maxItems: 1 + description: LED pin number + + active-low: + type: boolean + description: Makes LED active low. + + required: + - reg + + unevaluatedProperties: false + +required: + - reg + - "#address-cells" + - "#size-cells" + +additionalProperties: false + +examples: + - | + #include <dt-bindings/leds/common.h> + + leds@ff800800 { + compatible = "brcm,bcm4908-leds", "brcm,bcm63xxx-leds"; + reg = <0xff800800 0xdc>; + + #address-cells = <1>; + #size-cells = <0>; + + led@0 { + reg = <0x0>; + function = LED_FUNCTION_POWER; + color = <LED_COLOR_ID_GREEN>; + default-state = "on"; + }; + + led@3 { + reg = <0x3>; + function = LED_FUNCTION_STATUS; + color = <LED_COLOR_ID_GREEN>; + active-low; + }; + };