Messages in this thread | | | Date | Wed, 8 Feb 2023 21:08:40 +0100 | Subject | Re: [PATCH v2 03/11] dt-bindings: arm: mediatek: add 'mediatek,pn_swap' property | From | Krzysztof Kozlowski <> |
| |
On 08/02/2023 14:51, Daniel Golle wrote: > On Wed, Feb 08, 2023 at 10:32:53AM +0100, Krzysztof Kozlowski wrote: >> On 07/02/2023 19:00, Daniel Golle wrote: >>> ... >>>> 3. Does not look like property of this node. This is a clock controller >>>> or system controller, not SGMII/phy etc. >>> >>> The register range referred to by this node *does* represent also an >>> SGMII phy. These sgmiisys nodes also carry the 'syscon' compatible, and >>> are referenced in the node of the Ethernet core, and then used by >>> drivers/net/ethernet/mediatek/mtk_sgmii.c using syscon_node_to_regmap. >>> (This is the current situation already, and not related to the patchset >>> now adding only a new property to support hardware which needs that) >> >> Just because a register is located in syscon block, does not mean that >> SGMII configuration is a property of this device. > > It's not just one register, the whole SGMII PCS is located in those > mediatek,sgmiisys syscon nodes.
Then maybe this should be a PCS PHY device instead of adding properties unrelated to clock/system controller? I don't know, currently this binding says it is a provider of clocks...
> >> >>> >>> So: Should I introduce a new binding for the same compatible strings >>> related to the SGMII PHY features? Or is it fine in this case to add >>> this property to the existing binding? >> >> The user of syscon should configure it. I don't think you need new >> binding. You just have to update the user of this syscon. > > Excuse my confusion, but it's still not entirely clear to me. > So in this case I should add the description of the added propterty of > the individual SGMII units (there can be more than one) to > Documentation/devicetree/bindings/net/mediatek,net.yaml > eventhough the properties are in the sgmiisys syscon nodes?
I guess the property should be in the node representing the SGMII. You add it now to the clock (or system) controller, so it does not look right. It's not a property of a clock controller.
Now which node should have this property depends on your devices - which I have no clue about, I read what is in the bindings.
> > If so I will have to figure out how to describe properties of other > nodes in the binding of the node referencing them. Are there any > good examples for that?
phys and pcs'es?
> > Or should the property itself be moved into yet another array of > booleans which should be added in the node describing the ethernet > controller and referencing these sgmiisys syscons using phandles?
Best regards, Krzysztof
| |