| Message ID | 20240809094804.391441-2-francesco@dolcini.it |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | net: fec: add PPS channel configuration | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Fri, Aug 09, 2024 at 11:48:02AM +0200, Francesco Dolcini wrote: > From: Francesco Dolcini <francesco.dolcini@toradex.com> > > Add fsl,pps-channel property to specify to which timer instance the PPS > channel is connected to. In the driver patch you say "depending on the soc ... might be routed to different timer instances", why is a soc-specific compatible insufficient to determine which timer instance is in use? I think I know what you mean, but I'm not 100%. That said, the explanation in the driver patch is better than the one here, so a commit message improvement is required. Cheers, Conor. > > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> > --- > v3: added net-next subject prefix > v2: no changes > --- > Documentation/devicetree/bindings/net/fsl,fec.yaml | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/fsl,fec.yaml b/Documentation/devicetree/bindings/net/fsl,fec.yaml > index 5536c06139ca..24e863fdbdab 100644 > --- a/Documentation/devicetree/bindings/net/fsl,fec.yaml > +++ b/Documentation/devicetree/bindings/net/fsl,fec.yaml > @@ -183,6 +183,13 @@ properties: > description: > Register bits of stop mode control, the format is <&gpr req_gpr req_bit>. > > + fsl,pps-channel: > + $ref: /schemas/types.yaml#/definitions/uint32 > + default: 0 > + description: > + Specifies to which timer instance the PPS signal is routed. > + enum: [0, 1, 2, 3] > + > mdio: > $ref: mdio.yaml# > unevaluatedProperties: false > -- > 2.39.2 >
On Fri, Aug 09, 2024 at 03:27:39PM +0100, Conor Dooley wrote: > On Fri, Aug 09, 2024 at 11:48:02AM +0200, Francesco Dolcini wrote: > > From: Francesco Dolcini <francesco.dolcini@toradex.com> > > > > Add fsl,pps-channel property to specify to which timer instance the PPS > > channel is connected to. > > In the driver patch you say "depending on the soc ... might be routed to > different timer instances", why is a soc-specific compatible > insufficient to determine which timer instance is in use? > I think I know what you mean, but I'm not 100%. > > That said, the explanation in the driver patch is better than the one > here, so a commit message improvement is required. This was clarified by NXP during the discussion on this series [1] and the commit messages were not amended to take this new information into account, my fault. I would propose something like this here: ``` Add fsl,pps-channel property to select where to connect the PPS signal. This depends on the internal SoC routing and on the board, for example on the i.MX8 SoC it can be connected to an external pin (using channel 1) or to internal eDMA as DMA request (channel 0). ``` Francesco [1] https://lore.kernel.org/all/ZrPYOWA3FESx197L@lizhi-Precision-Tower-5810/
On Fri, Aug 09, 2024 at 04:49:14PM +0200, Francesco Dolcini wrote: > On Fri, Aug 09, 2024 at 03:27:39PM +0100, Conor Dooley wrote: > > On Fri, Aug 09, 2024 at 11:48:02AM +0200, Francesco Dolcini wrote: > > > From: Francesco Dolcini <francesco.dolcini@toradex.com> > > > > > > Add fsl,pps-channel property to specify to which timer instance the PPS > > > channel is connected to. > > > > In the driver patch you say "depending on the soc ... might be routed to > > different timer instances", why is a soc-specific compatible > > insufficient to determine which timer instance is in use? > > I think I know what you mean, but I'm not 100%. > > > > That said, the explanation in the driver patch is better than the one > > here, so a commit message improvement is required. > > This was clarified by NXP during the discussion on this series [1] and the > commit messages were not amended to take this new information into > account, my fault. > > I would propose something like this here: > > ``` > Add fsl,pps-channel property to select where to connect the PPS > signal. This depends on the internal SoC routing and on the board, for > example on the i.MX8 SoC it can be connected to an external pin (using > channel 1) or to internal eDMA as DMA request (channel 0). > ``` That's definitely better, as it illustrates why the compatible is not sufficient.
diff --git a/Documentation/devicetree/bindings/net/fsl,fec.yaml b/Documentation/devicetree/bindings/net/fsl,fec.yaml index 5536c06139ca..24e863fdbdab 100644 --- a/Documentation/devicetree/bindings/net/fsl,fec.yaml +++ b/Documentation/devicetree/bindings/net/fsl,fec.yaml @@ -183,6 +183,13 @@ properties: description: Register bits of stop mode control, the format is <&gpr req_gpr req_bit>. + fsl,pps-channel: + $ref: /schemas/types.yaml#/definitions/uint32 + default: 0 + description: + Specifies to which timer instance the PPS signal is routed. + enum: [0, 1, 2, 3] + mdio: $ref: mdio.yaml# unevaluatedProperties: false