Hi Arinc, On Sun, Feb 27, 2022 at 4:12 PM Arınç ÜNAL <arinc.unal@xxxxxxxxxx> wrote: > > On 15/02/2022 18:16, Sergio Paracuellos wrote: > > Hi Arinc, > > > > On Tue, Feb 15, 2022 at 3:11 PM Arınç ÜNAL <arinc.unal@xxxxxxxxxx> wrote: > >> > >> On 15/02/2022 12:09, Sergio Paracuellos wrote: > >>> Hi Arinc, > >>> > >>> On Tue, Feb 15, 2022 at 9:50 AM Arınç ÜNAL <arinc.unal@xxxxxxxxxx> wrote: > >>>> > >>>> Hey Sergio, > >>>> > >>>> On 15/02/2022 11:35, Sergio Paracuellos wrote: > >>>>> Hi Arinc, > >>>>> > >>>>> On Tue, Feb 15, 2022 at 9:18 AM Arınç ÜNAL <arinc.unal@xxxxxxxxxx> wrote: > >>>>>> > >>>>>> GB-PC1 uses some of the rgmii2 pins (22 - 33) as GPIO. Therefore, the > >>>>>> rgmii2 bus cannot be used on this device. > >>>>>> Overwrite pinctrl-0 property under the ethernet node without rgmii2_pins on > >>>>>> the GB-PC1 devicetree. > >>>>>> > >>>>>> Signed-off-by: Arınç ÜNAL <arinc.unal@xxxxxxxxxx> > >>>>>> --- > >>>>>> drivers/staging/mt7621-dts/gbpc1.dts | 4 ++++ > >>>>>> 1 file changed, 4 insertions(+) > >>>>> > >>>>> No issues in GB-PC1. So: > >>>>> > >>>>> Tested-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx> > >>>> > >>>> Thanks for testing it so quickly! > >>>> > >>>> I was wondering if you got pinctrl errors on the bootlog before applying > >>>> this patch series. > >>>> > >>>> rgmii2 pin group is given gpio function so calling it from ethernet node > >>>> would cause this on my TP-Link RE650 v1 which also uses the rgmii2_pins > >>>> as GPIO. > >>>> > >>>> [ 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 > >>> > >>> No, I was not getting any kind of error since when I test your last > >>> patch series I was not experimenting any kind of regression. I don't > >>> have any issues now also with your new patch series. Your new changes > >>> make sense since as you have said "rgmii2" pins are requested as GPIO > >>> but it seems are not really being requested? I don't have time to > >>> check the datasheet now but will try to get time to see what is > >>> happening there. > >> > >> I think this must have something to do with pinctrl on newer kernels as > >> the TP-Link RE650 that I tested uses the OpenWrt master branch (Linux 5.10). > > > > I think is this commit which I did according to a review after moving > > the driver from staging into 'drivers/pincrtl/ralink': > > > > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/drivers/pinctrl/ralink?h=staging-next&id=8a55d64c3336fc2ffd488a37d08ceab154c7b56b > > > > You can also check other changes from where the driver was moved: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/log/drivers/pinctrl/ralink?h=staging-next > > I realised current mt7621.dtsi does not apply the functions we specify > for the pin groups on device-specific devicetrees. I believe we need to > add this like on OpenWrt's mt7621.dtsi. > > https://github.com/openwrt/openwrt/blob/master/target/linux/ramips/dts/mt7621.dtsi#L249-L253 > > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/tree/drivers/staging/mt7621-dts/mt7621.dtsi?h=staging-testing#n153 > > Can you apply the patch below to see if you get the error like above? > I also put it in attachments in case of space characters replacing tab > on the mail. > > Arınç > > --- > diff --git a/drivers/staging/mt7621-dts/gbpc1.dts > b/drivers/staging/mt7621-dts/gbpc1.dts > index 1b5175e6ccf3..e38a083811e5 100644 > --- a/drivers/staging/mt7621-dts/gbpc1.dts > +++ b/drivers/staging/mt7621-dts/gbpc1.dts > @@ -114,10 +114,6 @@ default_gpio: gpio { > }; > }; > > -ðernet { > - pinctrl-0 = <&mdio_pins>, <&rgmii1_pins>; > -}; > - > &switch0 { > ports { > port@0 { > diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi > b/drivers/staging/mt7621-dts/mt7621.dtsi > index 4da20da243e6..8e181d6f70ae 100644 > --- a/drivers/staging/mt7621-dts/mt7621.dtsi > +++ b/drivers/staging/mt7621-dts/mt7621.dtsi > @@ -152,6 +152,11 @@ spi0: spi@b00 { > > pinctrl: pinctrl { > compatible = "ralink,rt2880-pinmux"; > + pinctrl-names = "default"; > + pinctrl-0 = <&state_default>; > + > + state_default: pinctrl0 { > + }; This should not be in the pinctrl node. I was told to remove it when bindings were reviewed since these properties must be in consumer nodes [0]. See binding documentation [1]: [0]: https://www.mail-archive.com/driverdev-devel@xxxxxxxxxxxxxxxxxxxxxx/msg102634.html [1]: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/tree/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml?h=staging-linus&id=b6821b0d9b56386d2bf14806f90ec401468c799f Best regards, Sergio Paracuellos > > i2c_pins: i2c0-pins { > pinmux {
