Hi Russell, On Thu, Sep 14, 2023 at 07:07:13PM +0100, Russell King (Oracle) wrote: > On Thu, Sep 14, 2023 at 07:06:11PM +0100, Russell King (Oracle) wrote: > > On Wed, Sep 13, 2023 at 04:59:19PM +0100, Russell King (Oracle) wrote: > > > On Wed, Sep 13, 2023 at 10:42:31AM +0300, Vladimir Oltean wrote: > > > > On Wed, Sep 13, 2023 at 08:52:37AM +0300, Arınç ÜNAL wrote: > > > > > 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. > > > > > > However, phylink pretty much requires phy-mode to be specified to be > > > something sane for shared ports, so I wouldn't be in favour of relaxing > > > the checkinng in dsa_shared_port_validate_of()... not unless you're > > > now going to accept the approach I originally proposed to have DSA > > > drivers tell the core (and thus phylink) what phy-mode and other link > > > parameters should be used when they are missing from DT. > > > > You mean the approach that I picked up using software nodes that got > > thrown out by the software node people? That approach that I picked > > up from you and tried to get merged? > > > > No, that's not going to happen, and it's not a question of whether > > _I_ am going to accept that approach or not. So don't throw that > > back on me, please. > > > > If this is something that we want to solve, we need to stop being so > > devisive (your language above is so) and try to come up with a > > solution that is acceptable to everyone... the swnode approach > > doesn't seem to be it. > > Oh dear. I must be going mad! So first things first: I am not advocating for making phy-mode fully optional in the sense that you say (if absent, then write non-OF code through which DSA infers the phy-mode from drivers). I'm happy with the current form of the code. I was just trying to add some nuance to this bizarre aspect signalled by Arınç - phy-mode is not required for user ports, presumably because when it is absent, user ports will default to GMII. That isn't an intrinsic feature of user ports, but rather of having a phydev, and so, because DSA/CPU ports can also have a phydev, logically it means that phy-mode can also be omitted in that particular case, with the same result. Our missing_phy_mode check from dsa_shared_port_validate_of() is theoretically more restrictive than it needs to be, because it artificially prohibits that behavior, and it results in an inexplicable difference in the phylink dt-bindings for user vs shared ports. So that's where my relaxation proposal came from: we could make missing_phy_mode non-fatal, and that would permit the configurations which can work to work, and the ones which can't work will fail elsewhere. Just like for the user ports. Where I wasn't absolutely clear is that I don't want Arınç to change any of that. Right now, on DSA shared ports, phy-mode is required and on user ports it isn't. The difference is a bit strange (arbitrary considering the example above) and should maybe be settled at some point in the future, but for now, the dt-bindings should document it like that.
