diff mbox series

[net,2/4] dt-bindings: fec: update the gpr property

Message ID 1589963516-26703-3-git-send-email-fugang.duan@nxp.com
State Superseded
Headers show
Series net: ethernet: fec: move GPR reigster offset and bit into DT | expand

Checks

Context Check Description
robh/checkpatch success

Commit Message

Andy Duan May 20, 2020, 8:31 a.m. UTC
From: Fugang Duan <fugang.duan@nxp.com>

Update the gpr property to define gpr register offset and
bit in DT, since different instance have different gpr bit,
and differnet SOC may have different gpr reigster offset.

Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
---
 Documentation/devicetree/bindings/net/fsl-fec.txt | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Fuzzey, Martin May 23, 2020, 10:15 a.m. UTC | #1
>  - 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
Andy Duan May 25, 2020, 3:16 a.m. UTC | #2
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 mbox series

Patch

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__