| Message ID | 1589963516-26703-3-git-send-email-fugang.duan@nxp.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | net: ethernet: fec: move GPR reigster offset and bit into DT | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
> - gpr: phandle of SoC general purpose register mode. Required for wake on LAN > - on some SoCs > + on some SoCs. Register bits of stop mode control, the format is > + <&gpr req_gpr req_bit>. > + gpr is the phandle to general purpose register node. > + req_gpr is the gpr register offset for ENET stop request. > + req_bit is the gpr bit offset for ENET stop request. > More of a DT binding changes policy question, do we care about supporting the old no argument binding too? I don't think it actually matters seeing as the no argument gpr node binding was only added recently anyway. But it was backported to the stable trees and Documentation/bindings/ABI.txt says "Bindings can be augmented, but the driver shouldn't break when given the old binding. ie. add additional properties, but don't change the meaning of an existing property. For drivers, default to the original behaviour when a newly added property is missing." Myself I think this is overkill in this case and am fine with just changing the binding without the driver handling the old case but that's Rob's call to make I think. Martin
From: Fuzzey, Martin <martin.fuzzey@flowbird.group> Sent: Saturday, May 23, 2020 6:16 PM > > - gpr: phandle of SoC general purpose register mode. Required for > > wake on LAN > > - on some SoCs > > + on some SoCs. Register bits of stop mode control, the format is > > + <&gpr req_gpr req_bit>. > > + gpr is the phandle to general purpose register node. > > + req_gpr is the gpr register offset for ENET stop request. > > + req_bit is the gpr bit offset for ENET stop request. > > > > More of a DT binding changes policy question, do we care about supporting > the old no argument binding too? > > I don't think it actually matters seeing as the no argument gpr node binding > was only added recently anyway. > But it was backported to the stable trees and Documentation/bindings/ABI.txt > says > > "Bindings can be augmented, but the driver shouldn't break when given > the old binding. ie. add additional properties, but don't change the > meaning of an existing property. For drivers, default to the original > behaviour when a newly added property is missing." > > Myself I think this is overkill in this case and am fine with just changing the > binding without the driver handling the old case but that's Rob's call to make I > think. The patch set is to add argument binding, and driver also doesn't support wol without argument binding. As you know, current driver only wol feature requests the property. I am not understand why we need to support the old without argument binding. Welcome to your suggestion for the solution. And 'gpr' string is not good description for stop mode, I will change it to the string: ' fsl,stop-mode'.
diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt index 26c492a..c2ea818 100644 --- a/Documentation/devicetree/bindings/net/fsl-fec.txt +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt @@ -23,7 +23,12 @@ Optional properties: the hardware workaround for ERR006687 applied and does not need a software workaround. - gpr: phandle of SoC general purpose register mode. Required for wake on LAN - on some SoCs + on some SoCs. Register bits of stop mode control, the format is + <&gpr req_gpr req_bit>. + gpr is the phandle to general purpose register node. + req_gpr is the gpr register offset for ENET stop request. + req_bit is the gpr bit offset for ENET stop request. + -interrupt-names: names of the interrupts listed in interrupts property in the same order. The defaults if not specified are __Number of interrupts__ __Default__