On Mon, Feb 28, 2022 at 3:53 PM Arınç ÜNAL <arinc.unal@xxxxxxxxxx> wrote: > > On 28/02/2022 10:01, Sergio Paracuellos wrote: > > 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 > > I'm not sure what a consumer node would be in this context so we can > claim the pin groups with the given function under it. > > Is the ethernet node a consumer node since I claim the rgmii2_pins pin > group under it for example? That is my understanding, yes. Best regards, Sergio Paracuellos > > Arınç
