On 14/02/2022 12:30, Sander Vanheule wrote:
On Sun, 2022-02-13 at 19:23 +0300, Arınç ÜNAL wrote:
On 13/02/2022 17:14, Sander Vanheule wrote:
[snip]
Are you sure this is the only way to make this work? Overwriting a
default in this
many
files doesn't make it look like a great default. This is probably
happening because
these
This is actually a minority of devicetrees. 26 opposed to 156 (minus
devicetrees that #include any of the 26 devicetrees) which use all 3 pin
groups on ethernet.
It's also upstreamed so I'd like to stick with it as much as I can.
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?h=staging-testing&id=0a93c0d75809582893e82039143591b9265b520e
files already specify rgmii2 to be used with the GPIO function, and
you're causing
some
sort of race as to what setting gets applied in the end, by adding
rmgii2_pins to the
default state for the ðernet node.
Agreed:
[ 1.177349] rt2880-pinmux pinctrl: pin io22 already requested by
pinctrl; cannot claim for 1e100000.ethernet
[ 1.196966] rt2880-pinmux pinctrl: pin-22 (1e100000.ethernet) status -
22
[ 1.210312] rt2880-pinmux pinctrl: could not request pin 22 (io22)
from group rgmii2 on device rt2880-pinmux
[ 1.230058] mtk_soc_eth 1e100000.ethernet: Error applying setting,
reverse things back
[ 1.245853] mtk_soc_eth: probe of 1e100000.ethernet failed with error -
22
That's why I overwrite the property without it for the 26 devicetrees.
An alternative would be to add the rgmii2 configuration to &state_default
node only on the
devices where it is actually required.
&state_default {
gpio {
groups = /* gpio groups */
function = "gpio";
};
gmac1 {
groups = "rgmii2";
function = "rgmii2";
};
};
That would mean I'd have to do this on ~70 devicetrees (instead of 26 as
originally proposed) which I'm going to mux PHY 0 or 4 to GMAC1 with my
next patch.
That doesn't sound great either.
Secondly, the default mode for the rgmii2 pin group is the SoC function
(which is "rgmii2") and not gpio. To add to this, the majority of the
devicetrees use the SoC function for rgmii2 so we're already doing the
most minimal changes with the current patch.
Not sure what you mean by this. So the "rgmii2" function is normally the
default, but you want to make this explicit by adding it the the ðernet node?
It's not explicitly giving it the rgmii2 function (since it's already
there on mt7621.dtsi) but rather have the rgmii2 pin group called from
the ethernet node so traffic can flow on the rgmii2 bus. I explained
this in detail on my response to Bjørn here:
http://lists.openwrt.org/pipermail/openwrt-devel/2022-February/037939.html
http://lists.openwrt.org/pipermail/openwrt-devel/2022-February/037949.html
&state_default does not refer to the SoC's default HW state, but to the device's
actual configuration. To be really 'clean', I would say the "default" state (as
in 'pinctrl-names = "default"') of the GPIO node should be the one indicating
which groups are to be configured as GPIO, as &state_default is the "default"
state of the pin controller. But such a change is probably out of scope for this
patch series.
Correct.
What would make more sense to me, is if the rgmii settings could be
enabled in the
gmac-s
themselves:
&gmac0 {
pinctrl-names = "default";
pinctrl-0 = <&mdio_pins>, <&rgmii1_pins>;
}
&gmac1 {
/* Has state = "disabled" by default */
pinctrl-names = "default";
pinctrl-0 = <&mdio_pins>, <&rgmii2_pins>;
}
That way, if the pinctrl-s are processed properly, the rgmii2_pins
setting would be
applied automatically when gmac1 is enabled. mdio_pins would be applied
when either
(or
both) gmac-s is (are) active.
This is also what I thought would be best but unfortunately that won't
work. Please read the responses on this thread:
https://lore.kernel.org/netdev/83a35aa3-6cb8-2bc4-2ff4-64278bbcd...@arinc9.com/T/
Right, I think you've pointed this out already a few times.Sorry for the
noise, I should've re-read that thread before replying. :)
Since these gmac-s are always internal to the SoC, pinctrl properties should
make sense
there. Did you investigate why these settings don't get applied?
No, and I don't think I'm capable of doing so (not much of a code reader
I'm not).
I had a brief look at the code (elixir.bootlin.org is very convenient for
browsing the kernel), and it appears that while 'ethernet' is probed by the
kernel as a proper device, the gmac nodes aren't. That means you don't get the
automatic kernel machinery that comes associated with device probing, which
includes pinctrl configuration. The gmac nodes are probed by the ethernet driver
itself, and although that performs pinctrl operations on the main device
('ethernet' node), it doesn't do this for the gmac-s. It probably could do this,
but I have no idea whether that would be acceptable, and is more a matter to
discuss with upstream and the device driver maintainers.
Agreed.
Arınç
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
