Re: [PATCH 2/2] arm64: dts: rockchip: add Protonic MECSBC device-tree

[Sascha Hauer]

On Thu, Apr 04, 2024 at 05:10:41PM +0200, Andrew Lunn wrote:
> > +&gmac1 {
> > +	assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
> > +	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru CLK_MAC1_2TOP>;
> > +	phy-handle = <&rgmii_phy1>;
> > +	phy-mode = "rgmii";
> > +	clock_in_out = "output";
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&gmac1m1_miim
> > +		     &gmac1m1_tx_bus2
> > +		     &gmac1m1_rx_bus2
> > +		     &gmac1m1_rgmii_clk
> > +		     &gmac1m1_clkinout
> > +		     &gmac1m1_rgmii_bus>;
> > +	status = "okay";
> > +	tx_delay = <0x30>;
> > +	rx_delay = <0x10>;
> > +};
> 
> There was a discussion about phy-mode = "rgmii"; and these
> tx/rx_delays last month. Please could you go read that discussion and
> them make use of rgmii-id, and change the delays.

Ok, I'll switch to rgmii-id.

> 
> Also, where did you copy this from? If possible, it would be good to
> fix the example everybody copies into new DT blobs.

These are the default values used in over a dozen boards and a also
given in the example in
Documentation/devicetree/bindings/net/rockchip-dwmac.yaml.
These are also the default values the driver uses when tx_delay and
rx_delay are not given in the device tree.

I can prepare a patch to fix the example.

Do you have a pointer why setting the delays in the phy is preferred
over setting them in the network driver? In the end this requires us
to have the correct phy driver whereas setting them in the network
driver would just work for any phy driver?

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |