From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: 2021-10-13 22:24:22
During recent reviews I've been telling people that new MAC drivers
should adopt a certain DT binding format for RGMII delays in order to
avoid conflicting interpretations. Some suggestions were better received
than others, and it appears we are still far from a consensus.
Part of the problem seems to be that there are still drivers that apply
RGMII delays based on an incorrect interpretation of the device tree,
and these serve as a bad example for others.
I happen to maintain one of those drivers and I am able to test it, so I
figure that one of the ways in which I can make a change is to stop
providing a bad example.
Therefore, this series adds support for the "rx-internal-delay-ps" and
"tx-internal-delay-ps" properties inside sja1105 switch port DT nodes,
and if these are present, they will decide what RGMII delays will the
driver apply.
The in-tree device trees are also updated to follow the new format, as
well as the schema validator.
I assume it's okay to get all changes merged in through the same tree
(net-next). Although the DTS changes could be split, if needed - the
driver works with or without them. There is one more DTS which should be
changed, which is in Shawn's tree but not in net-next:
https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git/tree/arch/arm64/boot/dts/freescale/fsl-lx2160a-bluebox3.dts?h=for-next
For that, I'd have to send a separate patch.
Vladimir Oltean (6):
ARM: dts: imx6qp-prtwd3: update RGMII delays for sja1105 switch
ARM: dts: ls1021a-tsn: update RGMII delays for sja1105 switch
dt-bindings: net: dsa: sja1105: fix example so all ports have a
phy-handle of fixed-link
dt-bindings: net: dsa: inherit the ethernet-controller DT schema
dt-bindings: net: dsa: sja1105: add {rx,tx}-internal-delay-ps
net: dsa: sja1105: parse {rx,tx}-internal-delay-ps properties for
RGMII delays
.../devicetree/bindings/net/dsa/dsa.yaml | 7 ++
.../bindings/net/dsa/nxp,sja1105.yaml | 43 +++++++++
arch/arm/boot/dts/imx6qp-prtwd3.dts | 2 +
arch/arm/boot/dts/ls1021a-tsn.dts | 2 +
drivers/net/dsa/sja1105/sja1105.h | 25 ++++-
drivers/net/dsa/sja1105/sja1105_clocking.c | 35 +++----
drivers/net/dsa/sja1105/sja1105_main.c | 94 ++++++++++++++-----
7 files changed, 161 insertions(+), 47 deletions(-)
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: 2021-10-13 22:24:26
In the new behavior, the sja1105 driver expects there to be explicit
RGMII delays present on the fixed-link ports, otherwise it will complain
that it falls back to legacy behavior, which is to apply RGMII delays
incorrectly derived from the phy-mode string.
In this case, the legacy behavior of the driver is to apply both RX and
TX delays. To preserve that, add explicit 2 nanosecond delays, which are
identical with what the driver used to add (a 90 degree phase shift).
The delays from the phy-mode are ignored by new kernels (it's still
RGMII as long as it's "rgmii*" something), and the explicit
{rx,tx}-internal-delay-ps properties are ignored by old kernels, so the
change works both ways.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
arch/arm/boot/dts/imx6qp-prtwd3.dts | 2 ++
1 file changed, 2 insertions(+)
In the new behavior, the sja1105 driver expects there to be explicit
RGMII delays present on the fixed-link ports, otherwise it will complain
that it falls back to legacy behavior, which is to apply RGMII delays
incorrectly derived from the phy-mode string.
In this case, the legacy behavior of the driver is to apply both RX and
TX delays. To preserve that, add explicit 2 nanosecond delays, which are
identical with what the driver used to add (a 90 degree phase shift).
The delays from the phy-mode are ignored by new kernels (it's still
RGMII as long as it's "rgmii*" something), and the explicit
{rx,tx}-internal-delay-ps properties are ignored by old kernels, so the
change works both ways.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: 2021-10-13 22:24:28
In the new behavior, the sja1105 driver expects there to be explicit
RGMII delays present on the fixed-link ports, otherwise it will complain
that it falls back to legacy behavior, which is to apply RGMII delays
incorrectly derived from the phy-mode string.
In this case, the legacy behavior of the driver is to not apply delays
in any direction (mostly because the SJA1105T can't do that, so this
board uses PCB traces). To preserve that but also silence the driver,
use explicit delays of 0 ns. The delay information from the phy-mode is
ignored by new kernels (it's still RGMII as long as it's "rgmii*"
something), and the explicit {rx,tx}-internal-delay-ps properties are
ignored by old kernels, so the change works both ways.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
arch/arm/boot/dts/ls1021a-tsn.dts | 2 ++
1 file changed, 2 insertions(+)
In the new behavior, the sja1105 driver expects there to be explicit
RGMII delays present on the fixed-link ports, otherwise it will complain
that it falls back to legacy behavior, which is to apply RGMII delays
incorrectly derived from the phy-mode string.
In this case, the legacy behavior of the driver is to not apply delays
in any direction (mostly because the SJA1105T can't do that, so this
board uses PCB traces). To preserve that but also silence the driver,
use explicit delays of 0 ns. The delay information from the phy-mode is
ignored by new kernels (it's still RGMII as long as it's "rgmii*"
something), and the explicit {rx,tx}-internal-delay-ps properties are
ignored by old kernels, so the change works both ways.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: 2021-10-13 22:24:31
All ports require either a phy-handle or a fixed-link, and port 3 in the
example didn't have one. Add it.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml | 1 +
1 file changed, 1 insertion(+)
All ports require either a phy-handle or a fixed-link, and port 3 in the
example didn't have one. Add it.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: 2021-10-13 22:24:38
Since a switch is basically a bunch of Ethernet controllers, just
inherit the common schema for one to get stronger type validation of the
properties of a port.
For example, before this change it was valid to have a phy-mode = "xfi"
even if "xfi" is not part of ethernet-controller.yaml, now it is not.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Documentation/devicetree/bindings/net/dsa/dsa.yaml | 3 +++
1 file changed, 3 insertions(+)
Since a switch is basically a bunch of Ethernet controllers, just
inherit the common schema for one to get stronger type validation of the
properties of a port.
For example, before this change it was valid to have a phy-mode = "xfi"
even if "xfi" is not part of ethernet-controller.yaml, now it is not.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: 2021-10-13 22:24:40
This change does not fix any functional issue or address any real life
use case that wasn't possible before. It is just a small step in the
process of standardizing the way in which Ethernet MAC drivers may apply
RGMII delays (traditionally these have been applied by PHYs, with no
clear definition of what to do in the case of a fixed-link).
The sja1105 driver used to apply MAC-level RGMII delays on the RX data
lines when in fixed-link mode and using a phy-mode of "rgmii-rxid" or
"rgmii-id" and on the TX data lines when using "rgmii-txid" or "rgmii-id".
But the standard definitions don't say anything about behaving
differently when the port is in fixed-link vs when it isn't, and the new
device tree bindings are about having a way of applying the delays in a
way that is independent of the phy-mode and of the fixed-link property.
When the {rx,tx}-internal-delay-ps properties are present, use them,
otherwise fall back to the old behavior and warn.
One other thing to note is that the SJA1105 hardware applies a delay
value in degrees rather than in picoseconds (the delay in ps changes
depending on the frequency of the RGMII clock - 125 MHz at 1G, 25 MHz at
100M, 2.5MHz at 10M). I assume that is fine, we calculate the phase
shift of the internal delay lines assuming that the device tree meant
gigabit, and we let the hardware scale those according to the link speed.
Link: https://patchwork.kernel.org/project/netdevbpf/patch/20210723173108.459770-6-prasanna.vengateshan@microchip.com/
Link: https://patchwork.ozlabs.org/project/netdev/patch/20200616074955.GA9092@laureti-dev/#2461123
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/dsa/sja1105/sja1105.h | 25 +++++-
drivers/net/dsa/sja1105/sja1105_clocking.c | 35 ++++----
drivers/net/dsa/sja1105/sja1105_main.c | 94 ++++++++++++++++------
3 files changed, 107 insertions(+), 47 deletions(-)
@@ -20,6 +20,27 @@#define SJA1105_AGEING_TIME_MS(ms) ((ms) / 10)#define SJA1105_NUM_L2_POLICERS SJA1110_MAX_L2_POLICING_COUNT+/* Calculated assuming 1Gbps, where the clock has 125 MHz (8 ns period)+*Toavoidfloatingpointoperations,we'llmultiplythedegreesby10+*togeta"phase"andget1decimalpointprecision.+*/+#define SJA1105_RGMII_DELAY_PS_TO_PHASE(ps) \+(((ps)*360)/800)+#define SJA1105_RGMII_DELAY_PHASE_TO_PS(phase) \+((800*(phase))/360)+#define SJA1105_RGMII_DELAY_PHASE_TO_HW(phase) \+(((phase)-738)/9)+#define SJA1105_RGMII_DELAY_PS_TO_HW(ps) \+SJA1105_RGMII_DELAY_PHASE_TO_HW(SJA1105_RGMII_DELAY_PS_TO_PHASE(ps))++/* Valid range in degrees is a value between 73.8 and 101.7+*in0.9degreeincrements+*/+#define SJA1105_RGMII_DELAY_MIN_PS \+SJA1105_RGMII_DELAY_PHASE_TO_PS(738)+#define SJA1105_RGMII_DELAY_MAX_PS \+SJA1105_RGMII_DELAY_PHASE_TO_PS(1017)+typedefenum{SPI_READ=0,SPI_WRITE=1,
@@ -498,17 +498,6 @@ sja1110_cfg_pad_mii_id_packing(void *buf, struct sja1105_cfg_pad_mii_id *cmd,sja1105_packing(buf,&cmd->txc_pd,0,0,size,op);}-/* Valid range in degrees is an integer between 73.8 and 101.7 */-staticu64sja1105_rgmii_delay(u64phase)-{-/* UM11040.pdf: The delay in degree phase is 73.8 + delay_tune * 0.9.-*Toavoidfloatingpointoperationswe'llmultiplyby10-*andget1decimalpointprecision.-*/-phase*=10;-return(phase-738)/9;-}-/* The RGMII delay setup procedure is 2-step and gets called upon each*.phylink_mac_config.Botharestrategic.*ThereasonisthattheRXTunableDelayLineoftheSJA1105MAChasissues
@@ -521,13 +510,15 @@ int sja1105pqrs_setup_rgmii_delay(const void *ctx, int port)conststructsja1105_private*priv=ctx;conststructsja1105_regs*regs=priv->info->regs;structsja1105_cfg_pad_mii_idpad_mii_id={0};+intrx_delay=priv->rgmii_rx_delay_ps[port];+inttx_delay=priv->rgmii_tx_delay_ps[port];u8packed_buf[SJA1105_SIZE_CGU_CMD]={0};intrc;-if(priv->rgmii_rx_delay[port])-pad_mii_id.rxc_delay=sja1105_rgmii_delay(90);-if(priv->rgmii_tx_delay[port])-pad_mii_id.txc_delay=sja1105_rgmii_delay(90);+if(rx_delay)+pad_mii_id.rxc_delay=SJA1105_RGMII_DELAY_PS_TO_HW(rx_delay);+if(tx_delay)+pad_mii_id.txc_delay=SJA1105_RGMII_DELAY_PS_TO_HW(tx_delay);/* Stage 1: Turn the RGMII delay lines off. */pad_mii_id.rxc_bypass=1;
@@ -542,11 +533,11 @@ int sja1105pqrs_setup_rgmii_delay(const void *ctx, int port)returnrc;/* Stage 2: Turn the RGMII delay lines on. */-if(priv->rgmii_rx_delay[port]){+if(rx_delay){pad_mii_id.rxc_bypass=0;pad_mii_id.rxc_pd=0;}-if(priv->rgmii_tx_delay[port]){+if(tx_delay){pad_mii_id.txc_bypass=0;pad_mii_id.txc_pd=0;}
@@ -561,20 +552,22 @@ int sja1110_setup_rgmii_delay(const void *ctx, int port)conststructsja1105_private*priv=ctx;conststructsja1105_regs*regs=priv->info->regs;structsja1105_cfg_pad_mii_idpad_mii_id={0};+intrx_delay=priv->rgmii_rx_delay_ps[port];+inttx_delay=priv->rgmii_tx_delay_ps[port];u8packed_buf[SJA1105_SIZE_CGU_CMD]={0};pad_mii_id.rxc_pd=1;pad_mii_id.txc_pd=1;-if(priv->rgmii_rx_delay[port]){-pad_mii_id.rxc_delay=sja1105_rgmii_delay(90);+if(rx_delay){+pad_mii_id.rxc_delay=SJA1105_RGMII_DELAY_PS_TO_HW(rx_delay);/* The "BYPASS" bit in SJA1110 is actually a "don't bypass" */pad_mii_id.rxc_bypass=1;pad_mii_id.rxc_pd=0;}-if(priv->rgmii_tx_delay[port]){-pad_mii_id.txc_delay=sja1105_rgmii_delay(90);+if(tx_delay){+pad_mii_id.txc_delay=SJA1105_RGMII_DELAY_PS_TO_HW(tx_delay);pad_mii_id.txc_bypass=1;pad_mii_id.txc_pd=0;}
@@ -1109,27 +1109,78 @@ static int sja1105_static_config_load(struct sja1105_private *priv)returnsja1105_static_config_upload(priv);}-staticintsja1105_parse_rgmii_delays(structsja1105_private*priv)+/* This is the "new way" for a MAC driver to configure its RGMII delay lines,+*basedontheexplicit"rx-internal-delay-ps"and"tx-internal-delay-ps"+*properties.IthastheadvantageofworkingwithfixedlinksandwithPHYs+*thatapplyRGMIIdelaystoo,andtheMACdriverneedsnotperformany+*specialchecks.+*+*Previouslywewereactinguponthe"phy-mode"propertywhenwewere+*operatinginfixed-link,basicallyactingasaPHY,butwithareversed+*interpretation:PHY_INTERFACE_MODE_RGMII_TXIDmeansthattheMACshould+*behaveasifitisconnectedtoaPHYwhichhasappliedRGMIIdelaysinthe+*TXdirection.Soifanything,RXdelaysshouldhavebeenaddedbytheMAC,+*butwewereaddingTXdelays.+*+*Ifthe"{rx,tx}-internal-delay-ps"propertiesarenotspecified,wefall+*backtothelegacybehaviorandapplydelaysonfixed-linkportsbasedon+*thereverseinterpretationofthephy-mode.Thisisadeviationfromthe+*expecteddefaultbehaviorwhichistosimplyapplynodelays.Toachieve+*thatbehaviorwiththenewbindings,itismandatorytospecify+*"{rx,tx}-internal-delay-ps"withavalueof0.+*/+staticintsja1105_parse_rgmii_delays(structsja1105_private*priv,intport,+structdevice_node*port_dn){-structdsa_switch*ds=priv->ds;-intport;+phy_interface_tphy_mode=priv->phy_mode[port];+structdevice*dev=&priv->spidev->dev;+intrx_delay=-1,tx_delay=-1;-for(port=0;port<ds->num_ports;port++){-if(!priv->fixed_link[port])-continue;+if(!phy_interface_mode_is_rgmii(phy_mode))+return0;-if(priv->phy_mode[port]==PHY_INTERFACE_MODE_RGMII_RXID||-priv->phy_mode[port]==PHY_INTERFACE_MODE_RGMII_ID)-priv->rgmii_rx_delay[port]=true;+of_property_read_u32(port_dn,"rx-internal-delay-ps",&rx_delay);+of_property_read_u32(port_dn,"tx-internal-delay-ps",&tx_delay);-if(priv->phy_mode[port]==PHY_INTERFACE_MODE_RGMII_TXID||-priv->phy_mode[port]==PHY_INTERFACE_MODE_RGMII_ID)-priv->rgmii_tx_delay[port]=true;+if(rx_delay==-1&&tx_delay==-1&&priv->fixed_link[port]){+dev_warn(dev,+"Port %d interpreting RGMII delay settings based on \"phy-mode\" property, "+"please update device tree to specify \"rx-internal-delay-ps\" and "+"\"tx-internal-delay-ps\"",+port);-if((priv->rgmii_rx_delay[port]||priv->rgmii_tx_delay[port])&&-!priv->info->setup_rgmii_delay)-return-EINVAL;+if(phy_mode==PHY_INTERFACE_MODE_RGMII_RXID||+phy_mode==PHY_INTERFACE_MODE_RGMII_ID)+rx_delay=2000;++if(phy_mode==PHY_INTERFACE_MODE_RGMII_TXID||+phy_mode==PHY_INTERFACE_MODE_RGMII_ID)+tx_delay=2000;}++if(rx_delay<0)+rx_delay=0;+if(tx_delay<0)+tx_delay=0;++if((rx_delay||tx_delay)&&!priv->info->setup_rgmii_delay){+dev_err(dev,"Chip cannot apply RGMII delays\n");+return-EINVAL;+}++if((rx_delay&&rx_delay<SJA1105_RGMII_DELAY_MIN_PS)||+(tx_delay&&tx_delay<SJA1105_RGMII_DELAY_MIN_PS)||+(rx_delay>SJA1105_RGMII_DELAY_MAX_PS)||+(tx_delay>SJA1105_RGMII_DELAY_MAX_PS)){+dev_err(dev,+"port %d RGMII delay values out of range, must be between %d and %d ps\n",+port,SJA1105_RGMII_DELAY_MIN_PS,SJA1105_RGMII_DELAY_MAX_PS);+return-ERANGE;+}++priv->rgmii_rx_delay_ps[port]=rx_delay;+priv->rgmii_tx_delay_ps[port]=tx_delay;+return0;}
@@ -1180,6 +1231,10 @@ static int sja1105_parse_ports_node(struct sja1105_private *priv,}priv->phy_mode[index]=phy_mode;++err=sja1105_parse_rgmii_delays(priv,index,child);+if(err)+returnerr;}return0;
@@ -3317,15 +3372,6 @@ static int sja1105_probe(struct spi_device *spi)returnrc;}-/* Error out early if internal delays are required through DT-*andwecan'tapplythem.-*/-rc=sja1105_parse_rgmii_delays(priv);-if(rc<0){-dev_err(ds->dev,"RGMII delay not supported\n");-returnrc;-}-if(IS_ENABLED(CONFIG_NET_SCH_CBS)){priv->cbs=devm_kcalloc(dev,priv->info->num_cbs_shapers,sizeof(structsja1105_cbs_entry),
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Jakub Kicinski <kuba@kernel.org> Date: 2021-10-14 00:24:53
Some take it or leave it comments, checkpatch pointed out some extra
brackets so I had a look at the patch.
On Thu, 14 Oct 2021 01:23:13 +0300 Vladimir Oltean wrote:
If I'm reading this right you're depending on delays being left as -1
in case the property reads fail. Is this commonly accepted practice?
Why not code it up as:
u32 rx_delay;
if (of_property_read_u32(...))
rx_delay = 0;
else if (rx_delay != clamp(rx_delay, ...MIN, ...MAX)
goto err;
or some such?
nit: checkpatch says the brackets around the latter two are unnecessary,
just in case it's not for symmetry / on purpose
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: 2021-10-14 12:00:18
On Wed, Oct 13, 2021 at 05:24:48PM -0700, Jakub Kicinski wrote:
Some take it or leave it comments, checkpatch pointed out some extra
brackets so I had a look at the patch.
On Thu, 14 Oct 2021 01:23:13 +0300 Vladimir Oltean wrote:
If I'm reading this right you're depending on delays being left as -1
in case the property reads fail. Is this commonly accepted practice?
It works.
Why not code it up as:
u32 rx_delay;
if (of_property_read_u32(...))
rx_delay = 0;
else if (rx_delay != clamp(rx_delay, ...MIN, ...MAX)
goto err;
or some such?
"or some such" is not functionally equivalent.
This is what would be functionally equivalent, and following your
suggestion to check the return code of of_property_read_u32 instead of
assigning default values, and to use clamp() instead of open-coding the
bounds checks.
static int sja1105_parse_rgmii_delays(struct sja1105_private *priv, int port,
struct device_node *port_dn)
{
phy_interface_t phy_mode = priv->phy_mode[port];
struct device *dev = &priv->spidev->dev;
int rx_delay, tx_delay;
int err_rx, err_tx;
if (!phy_interface_mode_is_rgmii(phy_mode))
return 0;
err_rx = of_property_read_u32(port_dn, "rx-internal-delay-ps", &rx_delay);
if (err_rx)
rx_delay = 0;
err_tx = of_property_read_u32(port_dn, "tx-internal-delay-ps", &tx_delay);
if (err_tx)
tx_delay = 0;
if (err_rx && err_tx) {
if (priv->fixed_link[port]) {
dev_warn(dev,
"Port %d interpreting RGMII delay settings based on \"phy-mode\" property, "
"please update device tree to specify \"rx-internal-delay-ps\" and "
"\"tx-internal-delay-ps\"",
port);
if (phy_mode == PHY_INTERFACE_MODE_RGMII_RXID ||
phy_mode == PHY_INTERFACE_MODE_RGMII_ID)
rx_delay = 2000;
if (phy_mode == PHY_INTERFACE_MODE_RGMII_TXID ||
phy_mode == PHY_INTERFACE_MODE_RGMII_ID)
tx_delay = 2000;
}
} else {
if ((rx_delay && rx_delay != clamp(rx_delay, SJA1105_RGMII_DELAY_MIN_PS, SJA1105_RGMII_DELAY_MAX_PS)) ||
(tx_delay && tx_delay != clamp(tx_delay, SJA1105_RGMII_DELAY_MIN_PS, SJA1105_RGMII_DELAY_MAX_PS))) {
dev_err(dev,
"port %d RGMII delay values out of range, must be between %d and %d ps\n",
port, SJA1105_RGMII_DELAY_MIN_PS, SJA1105_RGMII_DELAY_MAX_PS);
return -ERANGE;
}
}
priv->rgmii_rx_delay_ps[port] = rx_delay;
priv->rgmii_tx_delay_ps[port] = tx_delay;
return 0;
}
This change does not fix any functional issue or address any real life
use case that wasn't possible before. It is just a small step in the
process of standardizing the way in which Ethernet MAC drivers may apply
RGMII delays (traditionally these have been applied by PHYs, with no
clear definition of what to do in the case of a fixed-link).
The sja1105 driver used to apply MAC-level RGMII delays on the RX data
lines when in fixed-link mode and using a phy-mode of "rgmii-rxid" or
"rgmii-id" and on the TX data lines when using "rgmii-txid" or "rgmii-id".
But the standard definitions don't say anything about behaving
differently when the port is in fixed-link vs when it isn't, and the new
device tree bindings are about having a way of applying the delays in a
way that is independent of the phy-mode and of the fixed-link property.
When the {rx,tx}-internal-delay-ps properties are present, use them,
otherwise fall back to the old behavior and warn.
One other thing to note is that the SJA1105 hardware applies a delay
value in degrees rather than in picoseconds (the delay in ps changes
depending on the frequency of the RGMII clock - 125 MHz at 1G, 25 MHz at
100M, 2.5MHz at 10M). I assume that is fine, we calculate the phase
shift of the internal delay lines assuming that the device tree meant
gigabit, and we let the hardware scale those according to the link speed.
Link: https://patchwork.kernel.org/project/netdevbpf/patch/20210723173108.459770-6-prasanna.vengateshan@microchip.com/
Link: https://patchwork.ozlabs.org/project/netdev/patch/20200616074955.GA9092@laureti-dev/#2461123
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
FWIW, this is definitively a step in the right direction, and this is a
good way to deal with the phy-mode RGMII mess that has plagued us, thanks!
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: 2021-10-13 22:24:51
Add a schema validator to nxp,sja1105.yaml and to dsa.yaml for explicit
MAC-level RGMII delays. These properties must be per port and must be
present only for a phy-mode that represents RGMII.
We tell dsa.yaml that these port properties might be present, we also
define their valid values for SJA1105. We create a common definition for
the RX and TX valid range, since it's quite a mouthful.
We also modify the example to include the explicit RGMII delay properties.
On the fixed-link ports (in the example, port 4), having these explicit
delays is actually mandatory, since with the new behavior, the driver
shouts that it is interpreting what delays to apply based on phy-mode.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
.../devicetree/bindings/net/dsa/dsa.yaml | 4 ++
.../bindings/net/dsa/nxp,sja1105.yaml | 42 +++++++++++++++++++
2 files changed, 46 insertions(+)
From: Rob Herring <robh@kernel.org> Date: 2021-10-14 14:31:16
On Thu, 14 Oct 2021 01:23:12 +0300, Vladimir Oltean wrote:
Add a schema validator to nxp,sja1105.yaml and to dsa.yaml for explicit
MAC-level RGMII delays. These properties must be per port and must be
present only for a phy-mode that represents RGMII.
We tell dsa.yaml that these port properties might be present, we also
define their valid values for SJA1105. We create a common definition for
the RX and TX valid range, since it's quite a mouthful.
We also modify the example to include the explicit RGMII delay properties.
On the fixed-link ports (in the example, port 4), having these explicit
delays is actually mandatory, since with the new behavior, the driver
shouts that it is interpreting what delays to apply based on phy-mode.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
.../devicetree/bindings/net/dsa/dsa.yaml | 4 ++
.../bindings/net/dsa/nxp,sja1105.yaml | 42 +++++++++++++++++++
2 files changed, 46 insertions(+)
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
./Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml:82:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
./Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml:84:17: [warning] wrong indentation: expected 14 but found 16 (indentation)
./Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml:85:21: [warning] wrong indentation: expected 18 but found 20 (indentation)
./Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml:86:25: [warning] wrong indentation: expected 22 but found 24 (indentation)
./Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml:93:17: [warning] wrong indentation: expected 14 but found 16 (indentation)
./Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml:94:21: [warning] wrong indentation: expected 18 but found 20 (indentation)
./Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml:96:21: [warning] wrong indentation: expected 18 but found 20 (indentation)
./Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml:103:5: [warning] wrong indentation: expected 2 but found 4 (indentation)
./Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml:104:9: [warning] wrong indentation: expected 6 but found 8 (indentation)
./Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml:105:13: [warning] wrong indentation: expected 10 but found 12 (indentation)
./Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml:109:13: [warning] wrong indentation: expected 10 but found 12 (indentation)
./Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml:110:13: [warning] wrong indentation: expected 13 but found 12 (indentation)
./Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml:111:13: [warning] wrong indentation: expected 13 but found 12 (indentation)
dtschema/dtc warnings/errors:
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1540707
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Jakub Kicinski <kuba@kernel.org> Date: 2021-10-15 19:55:32
On Thu, 14 Oct 2021 09:31:04 -0500 Rob Herring wrote:
On Thu, 14 Oct 2021 01:23:12 +0300, Vladimir Oltean wrote:
quoted
Add a schema validator to nxp,sja1105.yaml and to dsa.yaml for explicit
MAC-level RGMII delays. These properties must be per port and must be
present only for a phy-mode that represents RGMII.
We tell dsa.yaml that these port properties might be present, we also
define their valid values for SJA1105. We create a common definition for
the RX and TX valid range, since it's quite a mouthful.
We also modify the example to include the explicit RGMII delay properties.
On the fixed-link ports (in the example, port 4), having these explicit
delays is actually mandatory, since with the new behavior, the driver
shouts that it is interpreting what delays to apply based on phy-mode.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
FWIW I dropped the set from pw based on Rob's report, I see a mention
of possible issues with fsl-lx2160a-bluebox3.dts, but it's not clear
to me which DT is disagreeing with the schema.. or is the schema itself
not 100?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: 2021-10-15 22:48:50
On Fri, Oct 15, 2021 at 12:55:27PM -0700, Jakub Kicinski wrote:
On Thu, 14 Oct 2021 09:31:04 -0500 Rob Herring wrote:
quoted
On Thu, 14 Oct 2021 01:23:12 +0300, Vladimir Oltean wrote:
quoted
Add a schema validator to nxp,sja1105.yaml and to dsa.yaml for explicit
MAC-level RGMII delays. These properties must be per port and must be
present only for a phy-mode that represents RGMII.
We tell dsa.yaml that these port properties might be present, we also
define their valid values for SJA1105. We create a common definition for
the RX and TX valid range, since it's quite a mouthful.
We also modify the example to include the explicit RGMII delay properties.
On the fixed-link ports (in the example, port 4), having these explicit
delays is actually mandatory, since with the new behavior, the driver
shouts that it is interpreting what delays to apply based on phy-mode.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
FWIW I dropped the set from pw based on Rob's report, I see a mention
of possible issues with fsl-lx2160a-bluebox3.dts, but it's not clear
to me which DT is disagreeing with the schema.. or is the schema itself
not 100?
I am only saying that I am introducing a new DT binding scheme and
warning all users of the old one. That's also why I am updating the
device trees, to silence the newly introduced warnings. I would like
this series to go through net-next, but fsl-lx2160a-bluebox3.dts isn't
in net-next.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Rob Herring <robh@kernel.org> Date: 2021-10-18 14:03:31
On Fri, Oct 15, 2021 at 5:48 PM Vladimir Oltean [off-list ref] wrote:
On Fri, Oct 15, 2021 at 12:55:27PM -0700, Jakub Kicinski wrote:
quoted
On Thu, 14 Oct 2021 09:31:04 -0500 Rob Herring wrote:
quoted
On Thu, 14 Oct 2021 01:23:12 +0300, Vladimir Oltean wrote:
quoted
Add a schema validator to nxp,sja1105.yaml and to dsa.yaml for explicit
MAC-level RGMII delays. These properties must be per port and must be
present only for a phy-mode that represents RGMII.
We tell dsa.yaml that these port properties might be present, we also
define their valid values for SJA1105. We create a common definition for
the RX and TX valid range, since it's quite a mouthful.
We also modify the example to include the explicit RGMII delay properties.
On the fixed-link ports (in the example, port 4), having these explicit
delays is actually mandatory, since with the new behavior, the driver
shouts that it is interpreting what delays to apply based on phy-mode.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
FWIW I dropped the set from pw based on Rob's report, I see a mention
of possible issues with fsl-lx2160a-bluebox3.dts, but it's not clear
to me which DT is disagreeing with the schema.. or is the schema itself
not 100?
I am only saying that I am introducing a new DT binding scheme and
warning all users of the old one. That's also why I am updating the
device trees, to silence the newly introduced warnings. I would like
this series to go through net-next, but fsl-lx2160a-bluebox3.dts isn't
in net-next.
.dts changes should go in via the sub-arch's tree, not a subsystem tree.
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Add a schema validator to nxp,sja1105.yaml and to dsa.yaml for explicit
MAC-level RGMII delays. These properties must be per port and must be
present only for a phy-mode that represents RGMII.
We tell dsa.yaml that these port properties might be present, we also
define their valid values for SJA1105. We create a common definition for
the RX and TX valid range, since it's quite a mouthful.
We also modify the example to include the explicit RGMII delay properties.
On the fixed-link ports (in the example, port 4), having these explicit
delays is actually mandatory, since with the new behavior, the driver
shouts that it is interpreting what delays to apply based on phy-mode.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Even if you need to make yamllint happy
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel