On Wed, Sep 13, 2023 at 08:52:37AM +0300, Arınç ÜNAL wrote: > On 12.09.2023 22:34, Vladimir Oltean wrote: > > On Tue, Sep 12, 2023 at 10:23:51PM +0300, Arınç ÜNAL wrote: > > > The phylink bindings for user ports I ended up making by looking up the > > > existing devicetrees are different than the phylink bindings for the shared > > > (CPU and DSA) ports currently enforced on all switches. > > > > > > My phylink bindings for user ports: > > > > > > allOf: > > > - anyOf: > > > - required: [ fixed-link ] > > > - required: [ phy-handle ] > > > - required: [ managed ] > > > > > > - if: > > > required: [ fixed-link ] > > > then: > > > not: > > > required: [ managed ] > > > > Right, it should have been anyOf and not oneOf.. my mistake. It is a bug > > which should be fixed. It's the same phylink that gets used in both cases, > > user ports and shared ports :) > > One more thing, I don't recall phy-mode being required to be defined for > user ports as it will default to GMII. I don't believe this is the same > case for shared ports so phy-mode is required only for them? phy-mode is not strictly required, but I think there is a strong preference to set it. IIRC, when looking at the DSA device trees, there was no case where phy-mode would be absent on CPU/DSA ports if the other link properties were also present, so we required it too. There were no complaints in 1 year since dsa_shared_port_validate_of() is there. The requirement can be relaxed to just a warning and no error in the kernel, and the removal of "required" in the schema, if it helps making it common with user ports. I think that the fallback to PHY_INTERFACE_MODE_GMII applies only if there is a phy_device (phy-handle). But otherwise, I don't remember if the PHY_INTERFACE_MODE_NA passed to phylink_create() will persist at runtime, or cause an error somewhere. > > > The phylink bindings for shared ports enforced on all switches on > > > dsa-port.yaml: > > > > > > allOf: > > > - required: > > > - phy-mode > > > - oneOf: > > > - required: > > > - fixed-link > > > - required: > > > - phy-handle > > > - required: > > > - managed > > > > > > Here's what I understand: > > > > > > - For switches in dsa_switches_apply_workarounds[] > > > - Enforce the latter for shared ports. > > > - Enforce the former for user ports. > > > > > > - For switches not in dsa_switches_apply_workarounds[] > > > - Enforce the former for all ports. > > > > No, no. We enforce the dt-schema regardless of switch presence in > > dsa_switches_apply_workarounds[], to encourage users to fix device trees > > (those who run schema validation). The kernel workaround consists in > > doing something (skipping phylink) for the device trees where the schema > > warns on shared ports. But there should be a single sub-schema for > > validating phylink bindings, whatever port kind it is. > > Hmm, like writing phylink.yaml and then referring to it under the port > pattern node? This could prevent a lot of repetition. > > Arınç Yes, that would sound good.
