| Message ID | 1684878827-40672-3-git-send-email-justin.chen@broadcom.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Brcm ASP 2.0 Ethernet Controller | expand |
On Tue, 23 May 2023 14:53:43 -0700, Justin Chen wrote: > From: Florian Fainelli <florian.fainelli@broadcom.com> > > Add a binding document for the Broadcom ASP 2.0 Ethernet > controller. > > Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com> > Signed-off-by: Justin Chen <justin.chen@broadcom.com> > --- > v3 > - Adjust compatible string example to reference SoC and HW ver > > v3 > - Minor formatting issues > - Change channel prop to brcm,channel for vendor specific format > - Removed redundant v2.0 from compat string > - Fix ranges field > > v2 > - Minor formatting issues > > .../devicetree/bindings/net/brcm,asp-v2.0.yaml | 145 +++++++++++++++++++++ > 1 file changed, 145 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/brcm,asp-v2.0.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/brcm,asp-v2.0.example.dtb: ethernet@9c00000: compatible: ['brcm,bcm72165-asp', 'brcm,asp-v2.0'] is too long From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/brcm,asp-v2.0.yaml doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/1684878827-40672-3-git-send-email-justin.chen@broadcom.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Hey Justin, On Tue, May 23, 2023 at 02:53:43PM -0700, Justin Chen wrote: > + compatible: > + enum: > + - brcm,asp-v2.0 > + - brcm,bcm72165-asp > + - brcm,asp-v2.1 > + - brcm,bcm74165-asp > + compatible = "brcm,bcm72165-asp", "brcm,asp-v2.0"; You can't do this, as Rob's bot has pointed out. Please test the bindings :( You need one of these type of constructs: compatible: oneOf: - items: - const: brcm,bcm72165-asp - const: brcm,asp-v2.0 - items: - const: brcm,bcm74165-asp - const: brcm,asp-v2.1 Although, given either you or Florian said there are likely to be multiple parts, going for an enum, rather than const for the brcm,bcm.. entry will prevent some churn. Up to you. Cheers, Conor.
On Tue, May 23, 2023 at 3:55 PM Conor Dooley <conor@kernel.org> wrote: > > Hey Justin, > > On Tue, May 23, 2023 at 02:53:43PM -0700, Justin Chen wrote: > > > + compatible: > > + enum: > > + - brcm,asp-v2.0 > > + - brcm,bcm72165-asp > > + - brcm,asp-v2.1 > > + - brcm,bcm74165-asp > > > + compatible = "brcm,bcm72165-asp", "brcm,asp-v2.0"; > > You can't do this, as Rob's bot has pointed out. Please test the > bindings :( You need one of these type of constructs: > > compatible: > oneOf: > - items: > - const: brcm,bcm72165-asp > - const: brcm,asp-v2.0 > - items: > - const: brcm,bcm74165-asp > - const: brcm,asp-v2.1 > > Although, given either you or Florian said there are likely to be > multiple parts, going for an enum, rather than const for the brcm,bcm.. > entry will prevent some churn. Up to you. > Urg so close. Thought it was a trivial change, so didn't bother retesting the binding. I think I have it right now... compatible: oneOf: - items: - enum: - brcm,bcm72165-asp - brcm,bcm74165-asp - enum: - brcm,asp-v2.0 - brcm,asp-v2.1 Something like this look good? Will submit a v5 tomorrow. Thanks, Justin > Cheers, > Conor.
Hey Justin, On Tue, May 23, 2023 at 04:27:12PM -0700, Justin Chen wrote: > On Tue, May 23, 2023 at 3:55 PM Conor Dooley <conor@kernel.org> wrote: > > On Tue, May 23, 2023 at 02:53:43PM -0700, Justin Chen wrote: > > > > > + compatible: > > > + enum: > > > + - brcm,asp-v2.0 > > > + - brcm,bcm72165-asp > > > + - brcm,asp-v2.1 > > > + - brcm,bcm74165-asp > > > > > + compatible = "brcm,bcm72165-asp", "brcm,asp-v2.0"; > > > > You can't do this, as Rob's bot has pointed out. Please test the > > bindings :( You need one of these type of constructs: > > > > compatible: > > oneOf: > > - items: > > - const: brcm,bcm72165-asp > > - const: brcm,asp-v2.0 > > - items: > > - const: brcm,bcm74165-asp > > - const: brcm,asp-v2.1 > > > > Although, given either you or Florian said there are likely to be > > multiple parts, going for an enum, rather than const for the brcm,bcm.. > > entry will prevent some churn. Up to you. > > > Urg so close. Thought it was a trivial change, so didn't bother > retesting the binding. I think I have it right now... > > compatible: > oneOf: > - items: > - enum: > - brcm,bcm72165-asp > - brcm,bcm74165-asp > - enum: > - brcm,asp-v2.0 > - brcm,asp-v2.1 > > Something like this look good? I am still caffeine-less, but this implies that both of "brcm,bcm72165-asp", "brcm,asp-v2.0" _and_ "brcm,bcm72165-asp", "brcm,asp-v2.1" are. I suspect that that is not the case, unless "brcm,asp-v2.0" is a valid fallback for "brcm,asp-v2.1"? The oneOf: also becomes redundant since you only have one items:. > Will submit a v5 tomorrow. BTW, when you do, could you use the address listed in MAINTAINERS rather than the one you used for this version? Cheers, Conor.
On Wed, May 24, 2023 at 07:51:07AM +0100, Conor Dooley wrote: > Hey Justin, > On Tue, May 23, 2023 at 04:27:12PM -0700, Justin Chen wrote: > > On Tue, May 23, 2023 at 3:55 PM Conor Dooley <conor@kernel.org> wrote: > > > On Tue, May 23, 2023 at 02:53:43PM -0700, Justin Chen wrote: > > > > > > > + compatible: > > > > + enum: > > > > + - brcm,asp-v2.0 > > > > + - brcm,bcm72165-asp > > > > + - brcm,asp-v2.1 > > > > + - brcm,bcm74165-asp > > > > > > > + compatible = "brcm,bcm72165-asp", "brcm,asp-v2.0"; > > > > > > You can't do this, as Rob's bot has pointed out. Please test the > > > bindings :( You need one of these type of constructs: > > > > > > compatible: > > > oneOf: > > > - items: > > > - const: brcm,bcm72165-asp > > > - const: brcm,asp-v2.0 > > > - items: > > > - const: brcm,bcm74165-asp > > > - const: brcm,asp-v2.1 > > > > > > Although, given either you or Florian said there are likely to be > > > multiple parts, going for an enum, rather than const for the brcm,bcm.. > > > entry will prevent some churn. Up to you. > > > > > Urg so close. Thought it was a trivial change, so didn't bother > > retesting the binding. I think I have it right now... > > > > compatible: > > oneOf: > > - items: > > - enum: > > - brcm,bcm72165-asp > > - brcm,bcm74165-asp > > - enum: > > - brcm,asp-v2.0 > > - brcm,asp-v2.1 > > > > Something like this look good? > > I am still caffeine-less, but this implies that both of > "brcm,bcm72165-asp", "brcm,asp-v2.0" > _and_ > "brcm,bcm72165-asp", "brcm,asp-v2.1" > are. I suspect that that is not the case, unless "brcm,asp-v2.0" is a I a word. s/are/are valid/ > valid fallback for "brcm,asp-v2.1"? > The oneOf: also becomes redundant since you only have one items:. > > > Will submit a v5 tomorrow. > > BTW, when you do, could you use the address listed in MAINTAINERS rather > than the one you used for this version? > > Cheers, > Conor.
On Tue, May 23, 2023 at 11:56 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > On Wed, May 24, 2023 at 07:51:07AM +0100, Conor Dooley wrote: > > Hey Justin, > > On Tue, May 23, 2023 at 04:27:12PM -0700, Justin Chen wrote: > > > On Tue, May 23, 2023 at 3:55 PM Conor Dooley <conor@kernel.org> wrote: > > > > On Tue, May 23, 2023 at 02:53:43PM -0700, Justin Chen wrote: > > > > > > > > > + compatible: > > > > > + enum: > > > > > + - brcm,asp-v2.0 > > > > > + - brcm,bcm72165-asp > > > > > + - brcm,asp-v2.1 > > > > > + - brcm,bcm74165-asp > > > > > > > > > + compatible = "brcm,bcm72165-asp", "brcm,asp-v2.0"; > > > > > > > > You can't do this, as Rob's bot has pointed out. Please test the > > > > bindings :( You need one of these type of constructs: > > > > > > > > compatible: > > > > oneOf: > > > > - items: > > > > - const: brcm,bcm72165-asp > > > > - const: brcm,asp-v2.0 > > > > - items: > > > > - const: brcm,bcm74165-asp > > > > - const: brcm,asp-v2.1 > > > > > > > > Although, given either you or Florian said there are likely to be > > > > multiple parts, going for an enum, rather than const for the brcm,bcm.. > > > > entry will prevent some churn. Up to you. > > > > > > > Urg so close. Thought it was a trivial change, so didn't bother > > > retesting the binding. I think I have it right now... > > > > > > compatible: > > > oneOf: > > > - items: > > > - enum: > > > - brcm,bcm72165-asp > > > - brcm,bcm74165-asp > > > - enum: > > > - brcm,asp-v2.0 > > > - brcm,asp-v2.1 > > > > > > Something like this look good? > > > > I am still caffeine-less, but this implies that both of > > "brcm,bcm72165-asp", "brcm,asp-v2.0" > > _and_ > > "brcm,bcm72165-asp", "brcm,asp-v2.1" > > are. I suspect that that is not the case, unless "brcm,asp-v2.0" is a > > I a word. s/are/are valid/ > Gotcha. I got something like this now. compatible: oneOf: - items: - enum: - brcm,bcm74165-asp - const: brcm,asp-v2.1 - items: - enum: - brcm,bcm72165-asp - const: brcm,asp-v2.0 Apologies, still getting used to this yaml stuff. Starting to make a bit more sense to me now. > > valid fallback for "brcm,asp-v2.1"? > > The oneOf: also becomes redundant since you only have one items:. > > > > > Will submit a v5 tomorrow. > > > > BTW, when you do, could you use the address listed in MAINTAINERS rather > > than the one you used for this version? > > I changed the address listed in MAINTAINERS from the previous versions of this patchset. The current version should match the address that this patch set was sent from. Looks like I forgot to add a changelog for that in v4. Thanks, Justin > > Cheers, > > Conor.
On Wed, May 24, 2023 at 02:47:59PM -0700, Justin Chen wrote: > On Tue, May 23, 2023 at 11:56 PM Conor Dooley <conor.dooley@microchip.com> wrote: > Gotcha. I got something like this now. > > compatible: > oneOf: > - items: > - enum: > - brcm,bcm74165-asp > - const: brcm,asp-v2.1 > - items: > - enum: > - brcm,bcm72165-asp > - const: brcm,asp-v2.0 Yes, this is what I had in mind. > Apologies, still getting used to this yaml stuff. Starting to make a > bit more sense to me now. No worries. > > > valid fallback for "brcm,asp-v2.1"? > > > The oneOf: also becomes redundant since you only have one items:. > > > > > > > Will submit a v5 tomorrow. > > > > > > BTW, when you do, could you use the address listed in MAINTAINERS rather > > > than the one you used for this version? > > > > I changed the address listed in MAINTAINERS from the previous versions > of this patchset. The current version should match the address that > this patch set was sent from. Looks like I forgot to add a changelog > for that in v4. Hmm, I must not have been clear. You sent it to <conor@kernel.org> and I was hoping that you would use <conor+dt@kernel.org> instead so that you end up hitting the right mail filters :) It's not a problem, I was just added to it in -rc1 so get_maintainer.pl probably didn't spit my name out for your original revision. Thanks, Conor.
diff --git a/Documentation/devicetree/bindings/net/brcm,asp-v2.0.yaml b/Documentation/devicetree/bindings/net/brcm,asp-v2.0.yaml new file mode 100644 index 000000000000..07f7373b9d64 --- /dev/null +++ b/Documentation/devicetree/bindings/net/brcm,asp-v2.0.yaml @@ -0,0 +1,145 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/brcm,asp-v2.0.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Broadcom ASP 2.0 Ethernet controller + +maintainers: + - Justin Chen <justin.chen@broadcom.com> + - Florian Fainelli <florian.fainelli@broadcom.com> + +description: Broadcom Ethernet controller first introduced with 72165 + +properties: + '#address-cells': + const: 1 + '#size-cells': + const: 1 + + compatible: + enum: + - brcm,asp-v2.0 + - brcm,bcm72165-asp + - brcm,asp-v2.1 + - brcm,bcm74165-asp + + reg: + maxItems: 1 + + ranges: true + + interrupts: + minItems: 1 + items: + - description: RX/TX interrupt + - description: Port 0 Wake-on-LAN + - description: Port 1 Wake-on-LAN + + clocks: + maxItems: 1 + + ethernet-ports: + type: object + properties: + '#address-cells': + const: 1 + '#size-cells': + const: 0 + + patternProperties: + "^port@[0-9]+$": + type: object + + $ref: ethernet-controller.yaml# + + properties: + reg: + maxItems: 1 + description: Port number + + brcm,channel: + $ref: /schemas/types.yaml#/definitions/uint32 + description: ASP channel number + + required: + - reg + - brcm,channel + + additionalProperties: false + +patternProperties: + "^mdio@[0-9a-f]+$": + type: object + $ref: brcm,unimac-mdio.yaml + + description: + ASP internal UniMAC MDIO bus + +required: + - compatible + - reg + - interrupts + - clocks + - ranges + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + + ethernet@9c00000 { + compatible = "brcm,bcm72165-asp", "brcm,asp-v2.0"; + reg = <0x9c00000 0x1fff14>; + interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>; + ranges = <0x0 0x9c00000 0x1fff14>; + clocks = <&scmi 14>; + #address-cells = <1>; + #size-cells = <1>; + + mdio@c614 { + compatible = "brcm,asp-v2.0-mdio"; + reg = <0xc614 0x8>; + reg-names = "mdio"; + #address-cells = <1>; + #size-cells = <0>; + + phy0: ethernet-phy@1 { + reg = <1>; + }; + }; + + mdio@ce14 { + compatible = "brcm,asp-v2.0-mdio"; + reg = <0xce14 0x8>; + reg-names = "mdio"; + #address-cells = <1>; + #size-cells = <0>; + + phy1: ethernet-phy@1 { + reg = <1>; + }; + }; + + ethernet-ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + brcm,channel = <8>; + phy-mode = "rgmii"; + phy-handle = <&phy0>; + }; + + port@1 { + reg = <1>; + brcm,channel = <9>; + phy-mode = "rgmii"; + phy-handle = <&phy1>; + }; + }; + };