[RFC v1 0/5] Add Bananapi R3 Mini

[RFC v1 0/5] Add Bananapi R3 Mini

[Frank Wunderlich]

From: Frank Wunderlich <frank-w@public-files.de>

Add mt7986 based BananaPi R3 Mini SBC and fix some related binding errors.

Frank Wunderlich (5):
  dt-bindings: leds: add led trigger netdev
  dt-bindings: clock: mediatek: add address-cells and size-cells to
    ethsys
  dt-bindings: net: mediatek,net: add reset-cells
  dt-bindings: arm64: dts: mediatek: add BananaPi R3 Mini
  arm64: dts: mediatek: Add  mt7986 based Bananapi R3 Mini

 .../devicetree/bindings/arm/mediatek.yaml     |   1 +
 .../bindings/clock/mediatek,ethsys.yaml       |   6 +
 .../devicetree/bindings/leds/common.yaml      |   2 +
 .../devicetree/bindings/net/mediatek,net.yaml |   3 +
 arch/arm64/boot/dts/mediatek/Makefile         |   1 +
 .../mediatek/mt7986a-bananapi-bpi-r3-mini.dts | 486 ++++++++++++++++++
 6 files changed, 499 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts

-- 
2.34.1

[RFC v1 1/5] dt-bindings: leds: add led trigger netdev

[Frank Wunderlich]

From: Frank Wunderlich <frank-w@public-files.de>

Add led trigger implemented with config-symbol LEDS_TRIGGER_NETDEV to
binding.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 Documentation/devicetree/bindings/leds/common.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
index 8a3c2398b10c..bf9a101e4d42 100644
--- a/Documentation/devicetree/bindings/leds/common.yaml
+++ b/Documentation/devicetree/bindings/leds/common.yaml
@@ -113,6 +113,8 @@ properties:
             # LED indicates NAND memory activity (deprecated),
             # in new implementations use "mtd"
           - nand-disk
+            # LED indicates network activity
+          - netdev
             # No trigger assigned to the LED. This is the default mode
             # if trigger is absent
           - none
-- 
2.34.1

Re: [RFC v1 1/5] dt-bindings: leds: add led trigger netdev

[Krzysztof Kozlowski]

On 05/05/2024 18:45, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add led trigger implemented with config-symbol LEDS_TRIGGER_NETDEV to
> binding.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>  Documentation/devicetree/bindings/leds/common.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
> index 8a3c2398b10c..bf9a101e4d42 100644
> --- a/Documentation/devicetree/bindings/leds/common.yaml
> +++ b/Documentation/devicetree/bindings/leds/common.yaml
> @@ -113,6 +113,8 @@ properties:
>              # LED indicates NAND memory activity (deprecated),
>              # in new implementations use "mtd"
>            - nand-disk
> +            # LED indicates network activity
> +          - netdev

"dev" is redundant (there is no flash-dev or usb-host-dev). Two network
interfaces are already provided, so your commit msg must provide
rationale why this is not enough and why this is useful/needed.

Best regards,
Krzysztof

Re: [RFC v1 1/5] dt-bindings: leds: add led trigger netdev

[Daniel Golle]

On Mon, May 06, 2024 at 10:18:09AM +0200, Krzysztof Kozlowski wrote:
> On 05/05/2024 18:45, Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w@public-files.de>
> > 
> > Add led trigger implemented with config-symbol LEDS_TRIGGER_NETDEV to
> > binding.
> > 
> > Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> > ---
> >  Documentation/devicetree/bindings/leds/common.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
> > index 8a3c2398b10c..bf9a101e4d42 100644
> > --- a/Documentation/devicetree/bindings/leds/common.yaml
> > +++ b/Documentation/devicetree/bindings/leds/common.yaml
> > @@ -113,6 +113,8 @@ properties:
> >              # LED indicates NAND memory activity (deprecated),
> >              # in new implementations use "mtd"
> >            - nand-disk
> > +            # LED indicates network activity
> > +          - netdev
> 
> "dev" is redundant (there is no flash-dev or usb-host-dev). Two network
> interfaces are already provided, so your commit msg must provide
> rationale why this is not enough and why this is useful/needed.

Also note that using 'netdev' assigned via DT via linux,default-trigger
currently doesn't work well. This is because the assignment of the trigger
from DT happens when the PHY is being attached initially, and that's
**before** the network device is registered with Linux.
As a result, LED event offloading is never used if done in this way.

I know that bindings and implementation are two different independent
things, but yet I believe that adding bindings for a feature which doesn't
really work would be misleading.

Aw: Re: [RFC v1 1/5] dt-bindings: leds: add led trigger netdev

[Frank Wunderlich]

Hi

> Gesendet: Montag, 06. Mai 2024 um 10:18 Uhr
> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>
> An: "Frank Wunderlich" <linux@fw-web.de>, "Rob Herring" <robh@kernel.org>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>, "Conor Dooley" <conor+dt@kernel.org>, "Michael Turquette" <mturquette@baylibre.com>, "Stephen Boyd" <sboyd@kernel.org>, "Pavel Machek" <pavel@ucw.cz>, "Lee Jones" <lee@kernel.org>, "David S. Miller" <davem@davemloft.net>, "Eric Dumazet" <edumazet@google.com>, "Jakub Kicinski" <kuba@kernel.org>, "Paolo Abeni" <pabeni@redhat.com>, "Matthias Brugger" <matthias.bgg@gmail.com>, "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
> Cc: "Frank Wunderlich" <frank-w@public-files.de>, "Eric Woudstra" <ericwouds@gmail.com>, "Tianling Shen" <cnsztl@immortalwrt.org>, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org, linux-leds@vger.kernel.org, netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org
> Betreff: Re: [RFC v1 1/5] dt-bindings: leds: add led trigger netdev
>
> On 05/05/2024 18:45, Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w@public-files.de>
> >
> > Add led trigger implemented with config-symbol LEDS_TRIGGER_NETDEV to
> > binding.
> >
> > Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> > ---
> >  Documentation/devicetree/bindings/leds/common.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
> > index 8a3c2398b10c..bf9a101e4d42 100644
> > --- a/Documentation/devicetree/bindings/leds/common.yaml
> > +++ b/Documentation/devicetree/bindings/leds/common.yaml
> > @@ -113,6 +113,8 @@ properties:
> >              # LED indicates NAND memory activity (deprecated),
> >              # in new implementations use "mtd"
> >            - nand-disk
> > +            # LED indicates network activity
> > +          - netdev
>
> "dev" is redundant (there is no flash-dev or usb-host-dev). Two network
> interfaces are already provided, so your commit msg must provide
> rationale why this is not enough and why this is useful/needed.

i only see 1 network binding...and this is labeled/described with wlan and phy

        # LED is triggered by WLAN activity
      - pattern: "^phy[0-9]+tx$"

which second do you mean?

btw. usb + disk has 3 trigger and "netdev" is already used in some dts, so i thought adding the binding is a good idea

arch/arm/boot/dts/rockchip/rk3128-xpi-3128.dts:107:			 * linux,default-trigger = "netdev";
arch/arm/boot/dts/nxp/imx/imx53-m53menlo.dts:52:			linux,default-trigger = "netdev";
arch/arm/boot/dts/intel/ixp/intel-ixp42x-dlink-dsm-g600.dts:51:			linux,default-trigger = "netdev";
arch/arm/boot/dts/intel/ixp/intel-ixp42x-iomega-nas100d.dts:39:			linux,default-trigger = "netdev";
arch/arm/boot/dts/ti/omap/am5729-beagleboneai.dts:138:			linux,default-trigger = "netdev";
arch/arm/boot/dts/ti/omap/am335x-netcan-plus-1xx.dts:27:			linux,default-trigger = "netdev";
arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts:107:			linux,default-trigger = "netdev";
arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts:113:			linux,default-trigger = "netdev";

first one has it as comment that not yet in binding and with not that it needs to be set in userspace
like Daniel stated in the dts patch (does not work for phys)...so i drop this patch and the property
in the dts.

> Best regards,
> Krzysztof

regards Frank

Re: Aw: Re: [RFC v1 1/5] dt-bindings: leds: add led trigger netdev

[Krzysztof Kozlowski]

On 06/05/2024 19:10, Frank Wunderlich wrote:
> Hi
> 
>> Gesendet: Montag, 06. Mai 2024 um 10:18 Uhr
>> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>
>> An: "Frank Wunderlich" <linux@fw-web.de>, "Rob Herring" <robh@kernel.org>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>, "Conor Dooley" <conor+dt@kernel.org>, "Michael Turquette" <mturquette@baylibre.com>, "Stephen Boyd" <sboyd@kernel.org>, "Pavel Machek" <pavel@ucw.cz>, "Lee Jones" <lee@kernel.org>, "David S. Miller" <davem@davemloft.net>, "Eric Dumazet" <edumazet@google.com>, "Jakub Kicinski" <kuba@kernel.org>, "Paolo Abeni" <pabeni@redhat.com>, "Matthias Brugger" <matthias.bgg@gmail.com>, "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
>> Cc: "Frank Wunderlich" <frank-w@public-files.de>, "Eric Woudstra" <ericwouds@gmail.com>, "Tianling Shen" <cnsztl@immortalwrt.org>, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org, linux-leds@vger.kernel.org, netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org
>> Betreff: Re: [RFC v1 1/5] dt-bindings: leds: add led trigger netdev
>>
>> On 05/05/2024 18:45, Frank Wunderlich wrote:
>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>
>>> Add led trigger implemented with config-symbol LEDS_TRIGGER_NETDEV to
>>> binding.
>>>
>>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>>> ---
>>>  Documentation/devicetree/bindings/leds/common.yaml | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
>>> index 8a3c2398b10c..bf9a101e4d42 100644
>>> --- a/Documentation/devicetree/bindings/leds/common.yaml
>>> +++ b/Documentation/devicetree/bindings/leds/common.yaml
>>> @@ -113,6 +113,8 @@ properties:
>>>              # LED indicates NAND memory activity (deprecated),
>>>              # in new implementations use "mtd"
>>>            - nand-disk
>>> +            # LED indicates network activity
>>> +          - netdev
>>
>> "dev" is redundant (there is no flash-dev or usb-host-dev). Two network
>> interfaces are already provided, so your commit msg must provide
>> rationale why this is not enough and why this is useful/needed.
> 
> i only see 1 network binding...and this is labeled/described with wlan and phy
> 
>         # LED is triggered by WLAN activity
>       - pattern: "^phy[0-9]+tx$"
> 
> which second do you mean?
> 
> btw. usb + disk has 3 trigger and "netdev" is already used in some dts, so i thought adding the binding is a good idea
> 
> arch/arm/boot/dts/rockchip/rk3128-xpi-3128.dts:107:			 * linux,default-trigger = "netdev";
> arch/arm/boot/dts/nxp/imx/imx53-m53menlo.dts:52:			linux,default-trigger = "netdev";
> arch/arm/boot/dts/intel/ixp/intel-ixp42x-dlink-dsm-g600.dts:51:			linux,default-trigger = "netdev";
> arch/arm/boot/dts/intel/ixp/intel-ixp42x-iomega-nas100d.dts:39:			linux,default-trigger = "netdev";
> arch/arm/boot/dts/ti/omap/am5729-beagleboneai.dts:138:			linux,default-trigger = "netdev";
> arch/arm/boot/dts/ti/omap/am335x-netcan-plus-1xx.dts:27:			linux,default-trigger = "netdev";
> arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts:107:			linux,default-trigger = "netdev";
> arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts:113:			linux,default-trigger = "netdev";
> 

Then please check previous discussions:
https://lore.kernel.org/all/20230217230346.GA2217008-robh@kernel.org/

Best regards,
Krzysztof

[RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys

[Frank Wunderlich]

From: Frank Wunderlich <frank-w@public-files.de>

Add missing properties already used in mt7986a.dtsi.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 .../devicetree/bindings/clock/mediatek,ethsys.yaml          | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/mediatek,ethsys.yaml b/Documentation/devicetree/bindings/clock/mediatek,ethsys.yaml
index f9cddacc2eae..ce953a35f307 100644
--- a/Documentation/devicetree/bindings/clock/mediatek,ethsys.yaml
+++ b/Documentation/devicetree/bindings/clock/mediatek,ethsys.yaml
@@ -32,12 +32,18 @@ properties:
   reg:
     maxItems: 1
 
+  "#address-cells":
+    const: 1
+
   "#clock-cells":
     const: 1
 
   "#reset-cells":
     const: 1
 
+  "#size-cells":
+    const: 1
+
 required:
   - reg
   - "#clock-cells"
-- 
2.34.1

Re: [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys

[Krzysztof Kozlowski]

On 05/05/2024 18:45, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add missing properties already used in mt7986a.dtsi.

Missing for what? Or why? Provide context, IOW, explain why they are
missing.


Best regards,
Krzysztof

Aw: Re: [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys

[Frank Wunderlich]

> Gesendet: Montag, 06. Mai 2024 um 10:18 Uhr
> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>
> On 05/05/2024 18:45, Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w@public-files.de>
> >
> > Add missing properties already used in mt7986a.dtsi.
>
> Missing for what? Or why? Provide context, IOW, explain why they are
> missing.

ethernet-node in mt7986a.dtsi hast reset-cells-property

https://elixir.bootlin.com/linux/v6.9-rc1/source/arch/arm64/boot/dts/mediatek/mt7986a.dtsi#L559

and

address-cells and size-cells are used here:

https://elixir.bootlin.com/linux/v6.9-rc1/source/arch/arm64/boot/dts/mediatek/mt7986a.dtsi#L495

i saw the warnings while checking my r3mini dts...

arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dtb: syscon@15000000: '#address-cells', '#size-cells' do not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/clock/mediatek,ethsys.yaml#
arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dtb: ethernet@15100000: Unevaluated properties are not allowed ('#reset-cells' was unexpected)
	from schema $id: http://devicetree.org/schemas/net/mediatek,net.yaml#

so i thought it is a good idea to fix this now ;)

but basicly not related to my dts as these are inherited, so i can drop binding-changes...

regards Frank

> Best regards,
> Krzysztof

Re: Aw: Re: [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys

[Rob Herring]

On Mon, May 06, 2024 at 06:51:43PM +0200, Frank Wunderlich wrote:
> > Gesendet: Montag, 06. Mai 2024 um 10:18 Uhr
> > Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>
> > On 05/05/2024 18:45, Frank Wunderlich wrote:
> > > From: Frank Wunderlich <frank-w@public-files.de>
> > >
> > > Add missing properties already used in mt7986a.dtsi.
> >
> > Missing for what? Or why? Provide context, IOW, explain why they are
> > missing.
> 
> ethernet-node in mt7986a.dtsi hast reset-cells-property
> 
> https://elixir.bootlin.com/linux/v6.9-rc1/source/arch/arm64/boot/dts/mediatek/mt7986a.dtsi#L559
> 
> and
> 
> address-cells and size-cells are used here:
> 
> https://elixir.bootlin.com/linux/v6.9-rc1/source/arch/arm64/boot/dts/mediatek/mt7986a.dtsi#L495
> 
> i saw the warnings while checking my r3mini dts...
> 
> arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dtb: syscon@15000000: '#address-cells', '#size-cells' do not match any of the regexes: 'pinctrl-[0-9]+'
> 	from schema $id: http://devicetree.org/schemas/clock/mediatek,ethsys.yaml#
> arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dtb: ethernet@15100000: Unevaluated properties are not allowed ('#reset-cells' was unexpected)
> 	from schema $id: http://devicetree.org/schemas/net/mediatek,net.yaml#
> 
> so i thought it is a good idea to fix this now ;)

The dts is already fixed dropping these properties in linux-next.

If you don't have child nodes with reg/ranges, then you never need 
#address-cells or #size-cells.

Rob

Aw: Re: Re: [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys

[Frank Wunderlich]

Hi

> Gesendet: Dienstag, 07. Mai 2024 um 21:35 Uhr
> Von: "Rob Herring" <robh@kernel.org>

> The dts is already fixed dropping these properties in linux-next.
>
> If you don't have child nodes with reg/ranges, then you never need
> #address-cells or #size-cells.

thx for pointing to this,

so i only need part 4+5 when 6.10-rc1 is out (as i drop netdev trigger from dts in v2).

any comments for these?

regards Frank

Re: [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys

[AngeloGioacchino Del Regno]

Il 05/05/24 18:45, Frank Wunderlich ha scritto:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add missing properties already used in mt7986a.dtsi.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>

Fixes tag? :-)

Cheers,
Angelo

Re: [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys

[Frank Wunderlich]

Am 6. Mai 2024 14:48:58 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>Il 05/05/24 18:45, Frank Wunderlich ha scritto:
>> From: Frank Wunderlich <frank-w@public-files.de>
>> 
>> Add missing properties already used in mt7986a.dtsi.
>> 
>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>
>Fixes tag? :-)
>
>Cheers,
>Angelo

Should i use fixes tag of binding commit or where dts (-part) was added?
regards Frank

Re: [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys

[AngeloGioacchino Del Regno]

Il 06/05/24 18:01, Frank Wunderlich ha scritto:
> Am 6. Mai 2024 14:48:58 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>> Il 05/05/24 18:45, Frank Wunderlich ha scritto:
>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>
>>> Add missing properties already used in mt7986a.dtsi.
>>>
>>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>>
>> Fixes tag? :-)
>>
>> Cheers,
>> Angelo
> 
> Should i use fixes tag of binding commit or where dts (-part) was added?
> regards Frank

You're fixing the binding, so, the binding one :-)

Cheers,
Angelo

[RFC v1 3/5] dt-bindings: net: mediatek,net: add reset-cells

[Frank Wunderlich]

From: Frank Wunderlich <frank-w@public-files.de>

Add missing binding for property used in mt7986a.dtsi.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 Documentation/devicetree/bindings/net/mediatek,net.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/mediatek,net.yaml b/Documentation/devicetree/bindings/net/mediatek,net.yaml
index e74502a0afe8..5214927c0fe8 100644
--- a/Documentation/devicetree/bindings/net/mediatek,net.yaml
+++ b/Documentation/devicetree/bindings/net/mediatek,net.yaml
@@ -101,6 +101,9 @@ properties:
   "#address-cells":
     const: 1
 
+  "#reset-cells":
+    const: 1
+
   "#size-cells":
     const: 0
 
-- 
2.34.1

Re: [RFC v1 3/5] dt-bindings: net: mediatek,net: add reset-cells

[Krzysztof Kozlowski]

On 05/05/2024 18:45, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add missing binding for property used in mt7986a.dtsi.
> 

Again, no, it is not missing, if the device is not reset controller.

Provide explanation why this is suitable in the binding. IOW, why DTS is
right, but binding is wrong.

Best regards,
Krzysztof

Re: [RFC v1 3/5] dt-bindings: net: mediatek,net: add reset-cells

[Rob Herring]

On Sun, May 05, 2024 at 06:45:47PM +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add missing binding for property used in mt7986a.dtsi.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>  Documentation/devicetree/bindings/net/mediatek,net.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/mediatek,net.yaml b/Documentation/devicetree/bindings/net/mediatek,net.yaml
> index e74502a0afe8..5214927c0fe8 100644
> --- a/Documentation/devicetree/bindings/net/mediatek,net.yaml
> +++ b/Documentation/devicetree/bindings/net/mediatek,net.yaml
> @@ -101,6 +101,9 @@ properties:
>    "#address-cells":
>      const: 1
>  
> +  "#reset-cells":
> +    const: 1

Also fixed already.

Rob

[RFC v1 4/5] dt-bindings: arm64: dts: mediatek: add BananaPi R3 Mini

[Frank Wunderlich]

From: Frank Wunderlich <frank-w@public-files.de>

Add MT7988A based BananaPi R3 Mini.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 Documentation/devicetree/bindings/arm/mediatek.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml
index 09f9ffd3ff7b..d4640e2e5fad 100644
--- a/Documentation/devicetree/bindings/arm/mediatek.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek.yaml
@@ -91,6 +91,7 @@ properties:
           - enum:
               - acelink,ew-7886cax
               - bananapi,bpi-r3
+              - bananapi,bpi-r3mini
               - mediatek,mt7986a-rfb
           - const: mediatek,mt7986a
       - items:
-- 
2.34.1

[RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini

[Frank Wunderlich]

From: Frank Wunderlich <frank-w@public-files.de>

Add device Tree for Bananapi R3 Mini SBC.

Co-developed-by: Eric Woudstra <ericwouds@gmail.com>
Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
Co-developed-by: Tianling Shen <cnsztl@gmail.com>
Signed-off-by: Tianling Shen <cnsztl@gmail.com>
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 arch/arm64/boot/dts/mediatek/Makefile         |   1 +
 .../mediatek/mt7986a-bananapi-bpi-r3-mini.dts | 486 ++++++++++++++++++
 2 files changed, 487 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts

diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
index 37b4ca3a87c9..1763b001ab06 100644
--- a/arch/arm64/boot/dts/mediatek/Makefile
+++ b/arch/arm64/boot/dts/mediatek/Makefile
@@ -11,6 +11,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-xiaomi-ax3000t.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-acelink-ew-7886cax.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb
+dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-mini.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtbo
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nand.dtbo
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nor.dtbo
diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
new file mode 100644
index 000000000000..c764b4dc4752
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
@@ -0,0 +1,486 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Copyright (C) 2021 MediaTek Inc.
+ * Authors: Frank Wunderlich <frank-w@public-files.de>
+ *          Eric Woudstra <ericwouds@gmail.com>
+ *          Tianling Shen <cnsztl@immortalwrt.org>
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/leds/common.h>
+#include <dt-bindings/pinctrl/mt65xx.h>
+
+#include "mt7986a.dtsi"
+
+/ {
+	model = "Bananapi BPI-R3 Mini";
+	chassis-type = "embedded";
+	compatible = "bananapi,bpi-r3mini", "mediatek,mt7986a";
+
+	aliases {
+		serial0 = &uart0;
+		ethernet0 = &gmac0;
+		ethernet1 = &gmac1;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	dcin: regulator-12vd {
+		compatible = "regulator-fixed";
+		regulator-name = "12vd";
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	fan: pwm-fan {
+		compatible = "pwm-fan";
+		#cooling-cells = <2>;
+		/* cooling level (0, 1, 2) - pwm inverted */
+		cooling-levels = <255 96 0>;
+		pwms = <&pwm 0 10000>;
+		status = "okay";
+	};
+
+	reg_1p8v: regulator-1p8v {
+		compatible = "regulator-fixed";
+		regulator-name = "1.8vd";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-boot-on;
+		regulator-always-on;
+		vin-supply = <&dcin>;
+	};
+
+	reg_3p3v: regulator-3p3v {
+		compatible = "regulator-fixed";
+		regulator-name = "3.3vd";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		regulator-always-on;
+		vin-supply = <&dcin>;
+	};
+
+	usb_vbus: regulator-usb-vbus {
+		compatible = "regulator-fixed";
+		regulator-name = "usb_vbus";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		gpios = <&pio 20 GPIO_ACTIVE_LOW>;
+		regulator-boot-on;
+	};
+
+	en8811_a: regulator-phy1 {
+		compatible = "regulator-fixed";
+		regulator-name = "phy1";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&pio 16 GPIO_ACTIVE_LOW>;
+		regulator-always-on;
+	};
+
+	en8811_b: regulator-phy2 {
+		compatible = "regulator-fixed";
+		regulator-name = "phy2";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&pio 17 GPIO_ACTIVE_LOW>;
+		regulator-always-on;
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		green_led: led-0 {
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_POWER;
+			gpios = <&pio 19 GPIO_ACTIVE_HIGH>;
+			default-state = "on";
+		};
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		reset-key {
+			label = "reset";
+			linux,code = <KEY_RESTART>;
+			gpios = <&pio 7 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+};
+
+&cpu_thermal {
+	cooling-maps {
+		map0 {
+			/* active: set fan to cooling level 2 */
+			cooling-device = <&fan 2 2>;
+			trip = <&cpu_trip_active_high>;
+		};
+
+		map1 {
+			/* active: set fan to cooling level 1 */
+			cooling-device = <&fan 1 1>;
+			trip = <&cpu_trip_active_med>;
+		};
+
+		map2 {
+			/* active: set fan to cooling level 0 */
+			cooling-device = <&fan 0 0>;
+			trip = <&cpu_trip_active_low>;
+		};
+	};
+};
+
+&crypto {
+	status = "okay";
+};
+
+&eth {
+	status = "okay";
+
+	gmac0: mac@0 {
+		compatible = "mediatek,eth-mac";
+		reg = <0>;
+		phy-mode = "2500base-x";
+		phy-handle = <&phy14>;
+	};
+
+	gmac1: mac@1 {
+		compatible = "mediatek,eth-mac";
+		reg = <1>;
+		phy-mode = "2500base-x";
+		phy-handle = <&phy15>;
+	};
+
+	mdio: mdio-bus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+};
+
+&mmc0 {
+	pinctrl-names = "default", "state_uhs";
+	pinctrl-0 = <&mmc0_pins_default>;
+	pinctrl-1 = <&mmc0_pins_uhs>;
+	vmmc-supply = <&reg_3p3v>;
+	vqmmc-supply = <&reg_1p8v>;
+};
+
+
+&i2c0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c_pins>;
+	status = "okay";
+
+	/* MAC Address EEPROM */
+	eeprom@50 {
+		compatible = "atmel,24c02";
+		reg = <0x50>;
+
+		address-width = <8>;
+		pagesize = <8>;
+		size = <256>;
+	};
+};
+
+&mdio {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	phy14: ethernet-phy@14 {
+		reg = <14>;
+		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
+		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
+		reset-assert-us = <10000>;
+		reset-deassert-us = <20000>;
+		phy-mode = "2500base-x";
+		full-duplex;
+		pause;
+		airoha,pnswap-rx;
+
+		leds {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			led@0 { /* en8811_a_gpio5 */
+				reg = <0>;
+				color = <LED_COLOR_ID_YELLOW>;
+				function = LED_FUNCTION_LAN;
+				function-enumerator = <1>;
+				default-state = "keep";
+				linux,default-trigger = "netdev";
+			};
+			led@1 { /* en8811_a_gpio4 */
+				reg = <1>;
+				color = <LED_COLOR_ID_GREEN>;
+				function = LED_FUNCTION_LAN;
+				function-enumerator = <2>;
+				default-state = "keep";
+				linux,default-trigger = "netdev";
+			};
+		};
+	};
+
+	phy15: ethernet-phy@15 {
+		reg = <15>;
+		interrupts-extended = <&pio 46 IRQ_TYPE_EDGE_FALLING>;
+		reset-gpios = <&pio 47 GPIO_ACTIVE_LOW>;
+		reset-assert-us = <10000>;
+		reset-deassert-us = <20000>;
+		phy-mode = "2500base-x";
+		full-duplex;
+		pause;
+		airoha,pnswap-rx;
+
+		leds {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			led@0 { /* en8811_b_gpio5 */
+				reg = <0>;
+				color = <LED_COLOR_ID_YELLOW>;
+				function = LED_FUNCTION_WAN;
+				function-enumerator = <1>;
+				default-state = "keep";
+				linux,default-trigger = "netdev";
+			};
+			led@1 { /* en8811_b_gpio4 */
+				reg = <1>;
+				color = <LED_COLOR_ID_GREEN>;
+				function = LED_FUNCTION_WAN;
+				function-enumerator = <2>;
+				default-state = "keep";
+				linux,default-trigger = "netdev";
+			};
+		};
+	};
+};
+
+&pcie {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie_pins>;
+	status = "okay";
+};
+
+&pcie_phy {
+	status = "okay";
+};
+
+&pio {
+	i2c_pins: i2c-pins {
+		mux {
+			function = "i2c";
+			groups = "i2c";
+		};
+	};
+
+	mmc0_pins_default: mmc0-pins {
+		mux {
+			function = "emmc";
+			groups = "emmc_51";
+		};
+		conf-cmd-dat {
+			pins = "EMMC_DATA_0", "EMMC_DATA_1", "EMMC_DATA_2",
+			       "EMMC_DATA_3", "EMMC_DATA_4", "EMMC_DATA_5",
+			       "EMMC_DATA_6", "EMMC_DATA_7", "EMMC_CMD";
+			input-enable;
+			drive-strength = <4>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_01>; /* pull-up 10K */
+		};
+		conf-clk {
+			pins = "EMMC_CK";
+			drive-strength = <6>;
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>; /* pull-down 50K */
+		};
+		conf-ds {
+			pins = "EMMC_DSL";
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>; /* pull-down 50K */
+		};
+		conf-rst {
+			pins = "EMMC_RSTB";
+			drive-strength = <4>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_01>; /* pull-up 10K */
+		};
+	};
+
+	mmc0_pins_uhs: mmc0-uhs-pins {
+		mux {
+			function = "emmc";
+			groups = "emmc_51";
+		};
+		conf-cmd-dat {
+			pins = "EMMC_DATA_0", "EMMC_DATA_1", "EMMC_DATA_2",
+			       "EMMC_DATA_3", "EMMC_DATA_4", "EMMC_DATA_5",
+			       "EMMC_DATA_6", "EMMC_DATA_7", "EMMC_CMD";
+			input-enable;
+			drive-strength = <4>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_01>; /* pull-up 10K */
+		};
+		conf-clk {
+			pins = "EMMC_CK";
+			drive-strength = <6>;
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>; /* pull-down 50K */
+		};
+		conf-ds {
+			pins = "EMMC_DSL";
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>; /* pull-down 50K */
+		};
+		conf-rst {
+			pins = "EMMC_RSTB";
+			drive-strength = <4>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_01>; /* pull-up 10K */
+		};
+	};
+
+	pcie_pins: pcie-pins {
+		mux {
+			function = "pcie";
+			groups = "pcie_clk", "pcie_wake", "pcie_pereset";
+		};
+	};
+
+	pwm_pins: pwm-pins {
+		mux {
+			function = "pwm";
+			groups = "pwm0";
+		};
+	};
+
+	spi_flash_pins: spi-flash-pins {
+		mux {
+			function = "spi";
+			groups = "spi0", "spi0_wp_hold";
+		};
+	};
+
+	usb_ngff_pins: usb-ngff-pins {
+		ngff-gnss-off-conf {
+			pins = "GPIO_6";
+			drive-strength = <8>;
+			mediatek,pull-up-adv = <1>;
+		};
+		ngff-pe-rst-conf {
+			pins = "GPIO_7";
+			drive-strength = <8>;
+			mediatek,pull-up-adv = <1>;
+		};
+		ngff-wwan-off-conf {
+			pins = "GPIO_8";
+			drive-strength = <8>;
+			mediatek,pull-up-adv = <1>;
+		};
+		ngff-pwr-off-conf {
+			pins = "GPIO_9";
+			drive-strength = <8>;
+			mediatek,pull-up-adv = <1>;
+		};
+		ngff-rst-conf {
+			pins = "GPIO_10";
+			drive-strength = <8>;
+			mediatek,pull-up-adv = <1>;
+		};
+		ngff-coex-conf {
+			pins = "SPI1_CS";
+			drive-strength = <8>;
+			mediatek,pull-up-adv = <1>;
+		};
+	};
+
+	wf_2g_5g_pins: wf-2g-5g-pins {
+		mux {
+			function = "wifi";
+			groups = "wf_2g", "wf_5g";
+		};
+		conf {
+			pins = "WF0_HB1", "WF0_HB2", "WF0_HB3", "WF0_HB4",
+			       "WF0_HB0", "WF0_HB0_B", "WF0_HB5", "WF0_HB6",
+			       "WF0_HB7", "WF0_HB8", "WF0_HB9", "WF0_HB10",
+			       "WF0_TOP_CLK", "WF0_TOP_DATA", "WF1_HB1",
+			       "WF1_HB2", "WF1_HB3", "WF1_HB4", "WF1_HB0",
+			       "WF1_HB5", "WF1_HB6", "WF1_HB7", "WF1_HB8",
+			       "WF1_TOP_CLK", "WF1_TOP_DATA";
+			drive-strength = <4>;
+		};
+	};
+
+	wf_dbdc_pins: wf-dbdc-pins {
+		mux {
+			function = "wifi";
+			groups = "wf_dbdc";
+		};
+		conf {
+			pins = "WF0_HB1", "WF0_HB2", "WF0_HB3", "WF0_HB4",
+			       "WF0_HB0", "WF0_HB0_B", "WF0_HB5", "WF0_HB6",
+			       "WF0_HB7", "WF0_HB8", "WF0_HB9", "WF0_HB10",
+			       "WF0_TOP_CLK", "WF0_TOP_DATA", "WF1_HB1",
+			       "WF1_HB2", "WF1_HB3", "WF1_HB4", "WF1_HB0",
+			       "WF1_HB5", "WF1_HB6", "WF1_HB7", "WF1_HB8",
+			       "WF1_TOP_CLK", "WF1_TOP_DATA";
+			drive-strength = <4>;
+		};
+	};
+
+	wf_led_pins: wf-led-pins {
+		mux {
+			function = "led";
+			groups = "wifi_led";
+		};
+	};
+};
+
+&pwm {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pwm_pins>;
+	status = "okay";
+};
+
+&spi0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi_flash_pins>;
+	status = "okay";
+};
+
+&ssusb {
+	pinctrl-names = "default";
+	pinctrl-0 = <&usb_ngff_pins>;
+	vusb33-supply = <&reg_3p3v>;
+	vbus-supply = <&usb_vbus>;
+	status = "okay";
+};
+
+&trng {
+	status = "okay";
+};
+
+&uart0 {
+	status = "okay";
+};
+
+&usb_phy {
+	status = "okay";
+};
+
+&watchdog {
+	status = "okay";
+};
+
+&wifi {
+	status = "okay";
+	pinctrl-names = "default", "dbdc";
+	pinctrl-0 = <&wf_2g_5g_pins>, <&wf_led_pins>;
+	pinctrl-1 = <&wf_dbdc_pins>, <&wf_led_pins>;
+
+	led {
+		led-active-low;
+	};
+};
+
-- 
2.34.1

Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini

[Krzysztof Kozlowski]

On 05/05/2024 18:45, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add device Tree for Bananapi R3 Mini SBC.
> 
> Co-developed-by: Eric Woudstra <ericwouds@gmail.com>
> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
> Co-developed-by: Tianling Shen <cnsztl@gmail.com>
> Signed-off-by: Tianling Shen <cnsztl@gmail.com>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>  arch/arm64/boot/dts/mediatek/Makefile         |   1 +
>  .../mediatek/mt7986a-bananapi-bpi-r3-mini.dts | 486 ++++++++++++++++++
>  2 files changed, 487 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> 
> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> index 37b4ca3a87c9..1763b001ab06 100644
> --- a/arch/arm64/boot/dts/mediatek/Makefile
> +++ b/arch/arm64/boot/dts/mediatek/Makefile
> @@ -11,6 +11,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-xiaomi-ax3000t.dtb
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-acelink-ew-7886cax.dtb
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb
> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-mini.dtb
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtbo
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nand.dtbo
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nor.dtbo
> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> new file mode 100644
> index 000000000000..c764b4dc4752
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> @@ -0,0 +1,486 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (C) 2021 MediaTek Inc.
> + * Authors: Frank Wunderlich <frank-w@public-files.de>
> + *          Eric Woudstra <ericwouds@gmail.com>
> + *          Tianling Shen <cnsztl@immortalwrt.org>
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pinctrl/mt65xx.h>
> +
> +#include "mt7986a.dtsi"
> +
> +/ {
> +	model = "Bananapi BPI-R3 Mini";
> +	chassis-type = "embedded";
> +	compatible = "bananapi,bpi-r3mini", "mediatek,mt7986a";
> +
> +	aliases {
> +		serial0 = &uart0;
> +		ethernet0 = &gmac0;
> +		ethernet1 = &gmac1;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	dcin: regulator-12vd {

Please use name for all fixed regulators which matches current format
recommendation: 'regulator-[0-9]+v[0-9]+'

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git/commit/?id=b6d4b3500d57370f5b3abf0701c9166b384db976

> +		compatible = "regulator-fixed";
> +		regulator-name = "12vd";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +	};
> +
> +	fan: pwm-fan {
> +		compatible = "pwm-fan";
> +		#cooling-cells = <2>;
> +		/* cooling level (0, 1, 2) - pwm inverted */
> +		cooling-levels = <255 96 0>;
> +		pwms = <&pwm 0 10000>;
> +		status = "okay";

Why? Where is it disabled?

> +	};
> +
> +	reg_1p8v: regulator-1p8v {

Please use name for all fixed regulators which matches current format
recommendation: 'regulator-[0-9]+v[0-9]+'

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git/commit/?id=b6d4b3500d57370f5b3abf0701c9166b384db976

In other places as well.



Best regards,
Krzysztof

Aw: Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini

[Frank Wunderlich]

Hi

Thanks for review



> Gesendet: Montag, 06. Mai 2024 um 10:20 Uhr
> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>
> An: "Frank Wunderlich" <linux@fw-web.de>, "Rob Herring" <robh@kernel.org>, "Krzysztof Kozlowski"
> > +	dcin: regulator-12vd {
>
> Please use name for all fixed regulators which matches current format
> recommendation: 'regulator-[0-9]+v[0-9]+'
>
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git/commit/?id=b6d4b3500d57370f5b3abf0701c9166b384db976
>
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "12vd";
> > +		regulator-min-microvolt = <12000000>;
> > +		regulator-max-microvolt = <12000000>;
> > +		regulator-boot-on;
> > +		regulator-always-on;
> > +	};
> > +
> > +	fan: pwm-fan {
> > +		compatible = "pwm-fan";
> > +		#cooling-cells = <2>;
> > +		/* cooling level (0, 1, 2) - pwm inverted */
> > +		cooling-levels = <255 96 0>;
> > +		pwms = <&pwm 0 10000>;
> > +		status = "okay";
>
> Why? Where is it disabled?

you're right, i'll drop it

> > +	};
> > +
> > +	reg_1p8v: regulator-1p8v {
>
> Please use name for all fixed regulators which matches current format
> recommendation: 'regulator-[0-9]+v[0-9]+'
>
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git/commit/?id=b6d4b3500d57370f5b3abf0701c9166b384db976
>
> In other places as well.

ok i change it like this (label doesn't matter, right?):

dcin: regulator-12v {
reg_1p8v: regulator-1v8 {
reg_3p3v: regulator-3v3 {
usb_vbus: regulator-5v {


> Best regards,
> Krzysztof

regards Frank

Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini

[AngeloGioacchino Del Regno]

Il 05/05/24 18:45, Frank Wunderlich ha scritto:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add device Tree for Bananapi R3 Mini SBC.
> 
> Co-developed-by: Eric Woudstra <ericwouds@gmail.com>
> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
> Co-developed-by: Tianling Shen <cnsztl@gmail.com>
> Signed-off-by: Tianling Shen <cnsztl@gmail.com>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>   arch/arm64/boot/dts/mediatek/Makefile         |   1 +
>   .../mediatek/mt7986a-bananapi-bpi-r3-mini.dts | 486 ++++++++++++++++++
>   2 files changed, 487 insertions(+)
>   create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> 
> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> index 37b4ca3a87c9..1763b001ab06 100644
> --- a/arch/arm64/boot/dts/mediatek/Makefile
> +++ b/arch/arm64/boot/dts/mediatek/Makefile
> @@ -11,6 +11,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-xiaomi-ax3000t.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-acelink-ew-7886cax.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb
> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-mini.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtbo
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nand.dtbo
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nor.dtbo
> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> new file mode 100644
> index 000000000000..c764b4dc4752
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> @@ -0,0 +1,486 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (C) 2021 MediaTek Inc.
> + * Authors: Frank Wunderlich <frank-w@public-files.de>
> + *          Eric Woudstra <ericwouds@gmail.com>
> + *          Tianling Shen <cnsztl@immortalwrt.org>
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pinctrl/mt65xx.h>
> +
> +#include "mt7986a.dtsi"
> +
> +/ {
> +	model = "Bananapi BPI-R3 Mini";
> +	chassis-type = "embedded";
> +	compatible = "bananapi,bpi-r3mini", "mediatek,mt7986a";
> +
> +	aliases {
> +		serial0 = &uart0;
> +		ethernet0 = &gmac0;
> +		ethernet1 = &gmac1;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	dcin: regulator-12vd {
> +		compatible = "regulator-fixed";
> +		regulator-name = "12vd";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +	};
> +
> +	fan: pwm-fan {
> +		compatible = "pwm-fan";
> +		#cooling-cells = <2>;
> +		/* cooling level (0, 1, 2) - pwm inverted */
> +		cooling-levels = <255 96 0>;

Did you try to actually invert the PWM?

Look for PWM_POLARITY_INVERTED ;-)

> +		pwms = <&pwm 0 10000>;
> +		status = "okay";
> +	};
> +
> +	reg_1p8v: regulator-1p8v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "1.8vd";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +		vin-supply = <&dcin>;
> +	};
> +
> +	reg_3p3v: regulator-3p3v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "3.3vd";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +		vin-supply = <&dcin>;
> +	};
> +
> +	usb_vbus: regulator-usb-vbus {
> +		compatible = "regulator-fixed";
> +		regulator-name = "usb_vbus";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		gpios = <&pio 20 GPIO_ACTIVE_LOW>;
> +		regulator-boot-on;
> +	};
> +
> +	en8811_a: regulator-phy1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "phy1";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&pio 16 GPIO_ACTIVE_LOW>;
> +		regulator-always-on;
> +	};
> +
> +	en8811_b: regulator-phy2 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "phy2";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&pio 17 GPIO_ACTIVE_LOW>;
> +		regulator-always-on;
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		green_led: led-0 {
> +			color = <LED_COLOR_ID_GREEN>;
> +			function = LED_FUNCTION_POWER;
> +			gpios = <&pio 19 GPIO_ACTIVE_HIGH>;
> +			default-state = "on";
> +		};
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +
> +		reset-key {
> +			label = "reset";
> +			linux,code = <KEY_RESTART>;
> +			gpios = <&pio 7 GPIO_ACTIVE_LOW>;
> +		};
> +	};
> +
> +};
> +
> +&cpu_thermal {
> +	cooling-maps {
> +		map0 {
> +			/* active: set fan to cooling level 2 */
> +			cooling-device = <&fan 2 2>;
> +			trip = <&cpu_trip_active_high>;
> +		};
> +
> +		map1 {
> +			/* active: set fan to cooling level 1 */
> +			cooling-device = <&fan 1 1>;
> +			trip = <&cpu_trip_active_med>;
> +		};
> +
> +		map2 {
> +			/* active: set fan to cooling level 0 */
> +			cooling-device = <&fan 0 0>;
> +			trip = <&cpu_trip_active_low>;
> +		};
> +	};
> +};
> +
> +&crypto {
> +	status = "okay";
> +};
> +
> +&eth {
> +	status = "okay";
> +
> +	gmac0: mac@0 {
> +		compatible = "mediatek,eth-mac";
> +		reg = <0>;
> +		phy-mode = "2500base-x";
> +		phy-handle = <&phy14>;
> +	};
> +
> +	gmac1: mac@1 {
> +		compatible = "mediatek,eth-mac";
> +		reg = <1>;
> +		phy-mode = "2500base-x";
> +		phy-handle = <&phy15>;
> +	};
> +
> +	mdio: mdio-bus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +	};
> +};
> +
> +&mmc0 {
> +	pinctrl-names = "default", "state_uhs";
> +	pinctrl-0 = <&mmc0_pins_default>;
> +	pinctrl-1 = <&mmc0_pins_uhs>;
> +	vmmc-supply = <&reg_3p3v>;
> +	vqmmc-supply = <&reg_1p8v>;
> +};
> +
> +
> +&i2c0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c_pins>;
> +	status = "okay";
> +
> +	/* MAC Address EEPROM */
> +	eeprom@50 {
> +		compatible = "atmel,24c02";
> +		reg = <0x50>;
> +
> +		address-width = <8>;
> +		pagesize = <8>;
> +		size = <256>;
> +	};
> +};
> +
> +&mdio {

You can just move all this stuff to where you declare the mdio-bus....

> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	phy14: ethernet-phy@14 {

I say that this is `phy0: ethernet-phy@14` - because this is the first PHY on this
board.

> +		reg = <14>;

Uhm.. doesn't this require the ethernet-phy-id03a2.a411 compatible?

> +		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
> +		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
> +		reset-assert-us = <10000>;
> +		reset-deassert-us = <20000>;
> +		phy-mode = "2500base-x";
> +		full-duplex;
> +		pause;
> +		airoha,pnswap-rx;
> +
> +		leds {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			led@0 { /* en8811_a_gpio5 */
> +				reg = <0>;
> +				color = <LED_COLOR_ID_YELLOW>;
> +				function = LED_FUNCTION_LAN;
> +				function-enumerator = <1>;

Why aren't you simply using a label?

> +				default-state = "keep";
> +				linux,default-trigger = "netdev";
> +			};
> +			led@1 { /* en8811_a_gpio4 */
> +				reg = <1>;
> +				color = <LED_COLOR_ID_GREEN>;
> +				function = LED_FUNCTION_LAN;
> +				function-enumerator = <2>;
> +				default-state = "keep";
> +				linux,default-trigger = "netdev";
> +			};
> +		};
> +	};
> +
> +	phy15: ethernet-phy@15 {
> +		reg = <15>;

Same here.

Cheers,
Angelo

Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini

[Frank Wunderlich]

Hi

Thanks for review.

Am 6. Mai 2024 14:48:59 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>Il 05/05/24 18:45, Frank Wunderlich ha scritto:
>> From: Frank Wunderlich <frank-w@public-files.de>
>> 
>> Add device Tree for Bananapi R3 Mini SBC.
>> 
>> Co-developed-by: Eric Woudstra <ericwouds@gmail.com>
>> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
>> Co-developed-by: Tianling Shen <cnsztl@gmail.com>
>> Signed-off-by: Tianling Shen <cnsztl@gmail.com>
>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>> ---
>>   arch/arm64/boot/dts/mediatek/Makefile         |   1 +
>>   .../mediatek/mt7986a-bananapi-bpi-r3-mini.dts | 486 ++++++++++++++++++
>>   2 files changed, 487 insertions(+)
>>   create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
>> 
>> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
>> index 37b4ca3a87c9..1763b001ab06 100644
>> --- a/arch/arm64/boot/dts/mediatek/Makefile
>> +++ b/arch/arm64/boot/dts/mediatek/Makefile
>> @@ -11,6 +11,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-xiaomi-ax3000t.dtb
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-acelink-ew-7886cax.dtb
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb
>> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-mini.dtb
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtbo
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nand.dtbo
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nor.dtbo
>> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
>> new file mode 100644
>> index 000000000000..c764b4dc4752
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
>> @@ -0,0 +1,486 @@
>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>> +/*
>> + * Copyright (C) 2021 MediaTek Inc.
>> + * Authors: Frank Wunderlich <frank-w@public-files.de>
>> + *          Eric Woudstra <ericwouds@gmail.com>
>> + *          Tianling Shen <cnsztl@immortalwrt.org>
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/input/input.h>
>> +#include <dt-bindings/leds/common.h>
>> +#include <dt-bindings/pinctrl/mt65xx.h>
>> +
>> +#include "mt7986a.dtsi"
>> +
>> +/ {
>> +	model = "Bananapi BPI-R3 Mini";
>> +	chassis-type = "embedded";
>> +	compatible = "bananapi,bpi-r3mini", "mediatek,mt7986a";
>> +
>> +	aliases {
>> +		serial0 = &uart0;
>> +		ethernet0 = &gmac0;
>> +		ethernet1 = &gmac1;
>> +	};
>> +
>> +	chosen {
>> +		stdout-path = "serial0:115200n8";
>> +	};
>> +
>> +	dcin: regulator-12vd {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "12vd";
>> +		regulator-min-microvolt = <12000000>;
>> +		regulator-max-microvolt = <12000000>;
>> +		regulator-boot-on;
>> +		regulator-always-on;
>> +	};
>> +
>> +	fan: pwm-fan {
>> +		compatible = "pwm-fan";
>> +		#cooling-cells = <2>;
>> +		/* cooling level (0, 1, 2) - pwm inverted */
>> +		cooling-levels = <255 96 0>;
>
>Did you try to actually invert the PWM?
>
>Look for PWM_POLARITY_INVERTED ;-)

Mtk pwm driver does not support it

https://elixir.bootlin.com/linux/latest/source/drivers/pwm/pwm-mediatek.c#L211

>> +		pwms = <&pwm 0 10000>;
>> +		status = "okay";
>> +	};
>> +
>> +	reg_1p8v: regulator-1p8v {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "1.8vd";
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <1800000>;
>> +		regulator-boot-on;
>> +		regulator-always-on;
>> +		vin-supply = <&dcin>;
>> +	};
>> +
>> +	reg_3p3v: regulator-3p3v {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "3.3vd";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		regulator-boot-on;
>> +		regulator-always-on;
>> +		vin-supply = <&dcin>;
>> +	};
>> +
>> +	usb_vbus: regulator-usb-vbus {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "usb_vbus";
>> +		regulator-min-microvolt = <5000000>;
>> +		regulator-max-microvolt = <5000000>;
>> +		gpios = <&pio 20 GPIO_ACTIVE_LOW>;
>> +		regulator-boot-on;
>> +	};
>> +
>> +	en8811_a: regulator-phy1 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "phy1";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		gpio = <&pio 16 GPIO_ACTIVE_LOW>;
>> +		regulator-always-on;
>> +	};
>> +
>> +	en8811_b: regulator-phy2 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "phy2";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		gpio = <&pio 17 GPIO_ACTIVE_LOW>;
>> +		regulator-always-on;
>> +	};
>> +
>> +	leds {
>> +		compatible = "gpio-leds";
>> +
>> +		green_led: led-0 {
>> +			color = <LED_COLOR_ID_GREEN>;
>> +			function = LED_FUNCTION_POWER;
>> +			gpios = <&pio 19 GPIO_ACTIVE_HIGH>;
>> +			default-state = "on";
>> +		};
>> +	};
>> +
>> +	gpio-keys {
>> +		compatible = "gpio-keys";
>> +
>> +		reset-key {
>> +			label = "reset";
>> +			linux,code = <KEY_RESTART>;
>> +			gpios = <&pio 7 GPIO_ACTIVE_LOW>;
>> +		};
>> +	};
>> +
>> +};
>> +
>> +&cpu_thermal {
>> +	cooling-maps {
>> +		map0 {
>> +			/* active: set fan to cooling level 2 */
>> +			cooling-device = <&fan 2 2>;
>> +			trip = <&cpu_trip_active_high>;
>> +		};
>> +
>> +		map1 {
>> +			/* active: set fan to cooling level 1 */
>> +			cooling-device = <&fan 1 1>;
>> +			trip = <&cpu_trip_active_med>;
>> +		};
>> +
>> +		map2 {
>> +			/* active: set fan to cooling level 0 */
>> +			cooling-device = <&fan 0 0>;
>> +			trip = <&cpu_trip_active_low>;
>> +		};
>> +	};
>> +};
>> +
>> +&crypto {
>> +	status = "okay";
>> +};
>> +
>> +&eth {
>> +	status = "okay";
>> +
>> +	gmac0: mac@0 {
>> +		compatible = "mediatek,eth-mac";
>> +		reg = <0>;
>> +		phy-mode = "2500base-x";
>> +		phy-handle = <&phy14>;
>> +	};
>> +
>> +	gmac1: mac@1 {
>> +		compatible = "mediatek,eth-mac";
>> +		reg = <1>;
>> +		phy-mode = "2500base-x";
>> +		phy-handle = <&phy15>;
>> +	};
>> +
>> +	mdio: mdio-bus {
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +	};
>> +};
>> +
>> +&mmc0 {
>> +	pinctrl-names = "default", "state_uhs";
>> +	pinctrl-0 = <&mmc0_pins_default>;
>> +	pinctrl-1 = <&mmc0_pins_uhs>;
>> +	vmmc-supply = <&reg_3p3v>;
>> +	vqmmc-supply = <&reg_1p8v>;
>> +};
>> +
>> +
>> +&i2c0 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&i2c_pins>;
>> +	status = "okay";
>> +
>> +	/* MAC Address EEPROM */
>> +	eeprom@50 {
>> +		compatible = "atmel,24c02";
>> +		reg = <0x50>;
>> +
>> +		address-width = <8>;
>> +		pagesize = <8>;
>> +		size = <256>;
>> +	};
>> +};
>> +
>> +&mdio {
>
>You can just move all this stuff to where you declare the mdio-bus....

Ok,see these 2 lines are already above,so can be dropped here.

>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +
>> +	phy14: ethernet-phy@14 {
>
>I say that this is `phy0: ethernet-phy@14` - because this is the first PHY on this
>board.

Ok,i change this and phy15

>> +		reg = <14>;
>
>Uhm.. doesn't this require the ethernet-phy-id03a2.a411 compatible?

I can add it,but worked without it.

There was a discussion about that and result was we don't need it in board dts,maybe add compatible in binding example.

https://patchwork.kernel.org/project/netdevbpf/patch/20240206194751.1901802-2-ericwouds@gmail.com/#25703356

>> +		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
>> +		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
>> +		reset-assert-us = <10000>;
>> +		reset-deassert-us = <20000>;
>> +		phy-mode = "2500base-x";
>> +		full-duplex;
>> +		pause;
>> +		airoha,pnswap-rx;
>> +
>> +		leds {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			led@0 { /* en8811_a_gpio5 */
>> +				reg = <0>;
>> +				color = <LED_COLOR_ID_YELLOW>;
>> +				function = LED_FUNCTION_LAN;
>> +				function-enumerator = <1>;
>
>Why aren't you simply using a label?

You mean the comment? I can add it of course like for regulators.

>> +				default-state = "keep";
>> +				linux,default-trigger = "netdev";
>> +			};
>> +			led@1 { /* en8811_a_gpio4 */
>> +				reg = <1>;
>> +				color = <LED_COLOR_ID_GREEN>;
>> +				function = LED_FUNCTION_LAN;
>> +				function-enumerator = <2>;
>> +				default-state = "keep";
>> +				linux,default-trigger = "netdev";
>> +			};
>> +		};
>> +	};
>> +
>> +	phy15: ethernet-phy@15 {
>> +		reg = <15>;
>
>Same here.
>
>Cheers,
>Angelo
>


regards Frank

Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini

[Daniel Golle]

On Mon, May 06, 2024 at 06:00:30PM +0200, Frank Wunderlich wrote:
> Hi
> 
> Thanks for review.
> 
> Am 6. Mai 2024 14:48:59 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
> >Il 05/05/24 18:45, Frank Wunderlich ha scritto:
> >> From: Frank Wunderlich <frank-w@public-files.de>
> >> 
> >> Add device Tree for Bananapi R3 Mini SBC.
> >> 
> >> Co-developed-by: Eric Woudstra <ericwouds@gmail.com>
> >> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
> >> Co-developed-by: Tianling Shen <cnsztl@gmail.com>
> >> Signed-off-by: Tianling Shen <cnsztl@gmail.com>
> >> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> >> ---
> >>   arch/arm64/boot/dts/mediatek/Makefile         |   1 +
> >>   .../mediatek/mt7986a-bananapi-bpi-r3-mini.dts | 486 ++++++++++++++++++
> >>   2 files changed, 487 insertions(+)
> >>   create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> >> 
> >> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> >> index 37b4ca3a87c9..1763b001ab06 100644
> >> --- a/arch/arm64/boot/dts/mediatek/Makefile
> >> +++ b/arch/arm64/boot/dts/mediatek/Makefile
> >> @@ -11,6 +11,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
> >>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-xiaomi-ax3000t.dtb
> >>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-acelink-ew-7886cax.dtb
> >>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb
> >> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-mini.dtb
> >>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtbo
> >>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nand.dtbo
> >>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nor.dtbo
> >> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> >> new file mode 100644
> >> index 000000000000..c764b4dc4752
> >> --- /dev/null
> >> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> >> @@ -0,0 +1,486 @@
> >> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> >> +/*
> >> + * Copyright (C) 2021 MediaTek Inc.
> >> + * Authors: Frank Wunderlich <frank-w@public-files.de>
> >> + *          Eric Woudstra <ericwouds@gmail.com>
> >> + *          Tianling Shen <cnsztl@immortalwrt.org>
> >> + */
> >> +
> >> +/dts-v1/;
> >> +
> >> +#include <dt-bindings/gpio/gpio.h>
> >> +#include <dt-bindings/input/input.h>
> >> +#include <dt-bindings/leds/common.h>
> >> +#include <dt-bindings/pinctrl/mt65xx.h>
> >> +
> >> +#include "mt7986a.dtsi"
> >> +
> >> +/ {
> >> +	model = "Bananapi BPI-R3 Mini";
> >> +	chassis-type = "embedded";
> >> +	compatible = "bananapi,bpi-r3mini", "mediatek,mt7986a";
> >> +
> >> +	aliases {
> >> +		serial0 = &uart0;
> >> +		ethernet0 = &gmac0;
> >> +		ethernet1 = &gmac1;
> >> +	};
> >> +
> >> +	chosen {
> >> +		stdout-path = "serial0:115200n8";
> >> +	};
> >> +
> >> +	dcin: regulator-12vd {
> >> +		compatible = "regulator-fixed";
> >> +		regulator-name = "12vd";
> >> +		regulator-min-microvolt = <12000000>;
> >> +		regulator-max-microvolt = <12000000>;
> >> +		regulator-boot-on;
> >> +		regulator-always-on;
> >> +	};
> >> +
> >> +	fan: pwm-fan {
> >> +		compatible = "pwm-fan";
> >> +		#cooling-cells = <2>;
> >> +		/* cooling level (0, 1, 2) - pwm inverted */
> >> +		cooling-levels = <255 96 0>;
> >
> >Did you try to actually invert the PWM?
> >
> >Look for PWM_POLARITY_INVERTED ;-)
> 
> Mtk pwm driver does not support it
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/pwm/pwm-mediatek.c#L211
> 
> >> +		pwms = <&pwm 0 10000>;
> >> +		status = "okay";
> >> +	};
> >> +
> >> +	reg_1p8v: regulator-1p8v {
> >> +		compatible = "regulator-fixed";
> >> +		regulator-name = "1.8vd";
> >> +		regulator-min-microvolt = <1800000>;
> >> +		regulator-max-microvolt = <1800000>;
> >> +		regulator-boot-on;
> >> +		regulator-always-on;
> >> +		vin-supply = <&dcin>;
> >> +	};
> >> +
> >> +	reg_3p3v: regulator-3p3v {
> >> +		compatible = "regulator-fixed";
> >> +		regulator-name = "3.3vd";
> >> +		regulator-min-microvolt = <3300000>;
> >> +		regulator-max-microvolt = <3300000>;
> >> +		regulator-boot-on;
> >> +		regulator-always-on;
> >> +		vin-supply = <&dcin>;
> >> +	};
> >> +
> >> +	usb_vbus: regulator-usb-vbus {
> >> +		compatible = "regulator-fixed";
> >> +		regulator-name = "usb_vbus";
> >> +		regulator-min-microvolt = <5000000>;
> >> +		regulator-max-microvolt = <5000000>;
> >> +		gpios = <&pio 20 GPIO_ACTIVE_LOW>;
> >> +		regulator-boot-on;
> >> +	};
> >> +
> >> +	en8811_a: regulator-phy1 {
> >> +		compatible = "regulator-fixed";
> >> +		regulator-name = "phy1";
> >> +		regulator-min-microvolt = <3300000>;
> >> +		regulator-max-microvolt = <3300000>;
> >> +		gpio = <&pio 16 GPIO_ACTIVE_LOW>;
> >> +		regulator-always-on;
> >> +	};
> >> +
> >> +	en8811_b: regulator-phy2 {
> >> +		compatible = "regulator-fixed";
> >> +		regulator-name = "phy2";
> >> +		regulator-min-microvolt = <3300000>;
> >> +		regulator-max-microvolt = <3300000>;
> >> +		gpio = <&pio 17 GPIO_ACTIVE_LOW>;
> >> +		regulator-always-on;
> >> +	};
> >> +
> >> +	leds {
> >> +		compatible = "gpio-leds";
> >> +
> >> +		green_led: led-0 {
> >> +			color = <LED_COLOR_ID_GREEN>;
> >> +			function = LED_FUNCTION_POWER;
> >> +			gpios = <&pio 19 GPIO_ACTIVE_HIGH>;
> >> +			default-state = "on";
> >> +		};
> >> +	};
> >> +
> >> +	gpio-keys {
> >> +		compatible = "gpio-keys";
> >> +
> >> +		reset-key {
> >> +			label = "reset";
> >> +			linux,code = <KEY_RESTART>;
> >> +			gpios = <&pio 7 GPIO_ACTIVE_LOW>;
> >> +		};
> >> +	};
> >> +
> >> +};
> >> +
> >> +&cpu_thermal {
> >> +	cooling-maps {
> >> +		map0 {
> >> +			/* active: set fan to cooling level 2 */
> >> +			cooling-device = <&fan 2 2>;
> >> +			trip = <&cpu_trip_active_high>;
> >> +		};
> >> +
> >> +		map1 {
> >> +			/* active: set fan to cooling level 1 */
> >> +			cooling-device = <&fan 1 1>;
> >> +			trip = <&cpu_trip_active_med>;
> >> +		};
> >> +
> >> +		map2 {
> >> +			/* active: set fan to cooling level 0 */
> >> +			cooling-device = <&fan 0 0>;
> >> +			trip = <&cpu_trip_active_low>;
> >> +		};
> >> +	};
> >> +};
> >> +
> >> +&crypto {
> >> +	status = "okay";
> >> +};
> >> +
> >> +&eth {
> >> +	status = "okay";
> >> +
> >> +	gmac0: mac@0 {
> >> +		compatible = "mediatek,eth-mac";
> >> +		reg = <0>;
> >> +		phy-mode = "2500base-x";
> >> +		phy-handle = <&phy14>;
> >> +	};
> >> +
> >> +	gmac1: mac@1 {
> >> +		compatible = "mediatek,eth-mac";
> >> +		reg = <1>;
> >> +		phy-mode = "2500base-x";
> >> +		phy-handle = <&phy15>;
> >> +	};
> >> +
> >> +	mdio: mdio-bus {
> >> +		#address-cells = <1>;
> >> +		#size-cells = <0>;
> >> +	};
> >> +};
> >> +
> >> +&mmc0 {
> >> +	pinctrl-names = "default", "state_uhs";
> >> +	pinctrl-0 = <&mmc0_pins_default>;
> >> +	pinctrl-1 = <&mmc0_pins_uhs>;
> >> +	vmmc-supply = <&reg_3p3v>;
> >> +	vqmmc-supply = <&reg_1p8v>;
> >> +};
> >> +
> >> +
> >> +&i2c0 {
> >> +	pinctrl-names = "default";
> >> +	pinctrl-0 = <&i2c_pins>;
> >> +	status = "okay";
> >> +
> >> +	/* MAC Address EEPROM */
> >> +	eeprom@50 {
> >> +		compatible = "atmel,24c02";
> >> +		reg = <0x50>;
> >> +
> >> +		address-width = <8>;
> >> +		pagesize = <8>;
> >> +		size = <256>;
> >> +	};
> >> +};
> >> +
> >> +&mdio {
> >
> >You can just move all this stuff to where you declare the mdio-bus....
> 
> Ok,see these 2 lines are already above,so can be dropped here.
> 
> >> +	#address-cells = <1>;
> >> +	#size-cells = <0>;
> >> +
> >> +	phy14: ethernet-phy@14 {
> >
> >I say that this is `phy0: ethernet-phy@14` - because this is the first PHY on this
> >board.
> 
> Ok,i change this and phy15
> 
> >> +		reg = <14>;
> >
> >Uhm.. doesn't this require the ethernet-phy-id03a2.a411 compatible?
> 
> I can add it,but worked without it.
> 
> There was a discussion about that and result was we don't need it in board dts,maybe add compatible in binding example.
> 
> https://patchwork.kernel.org/project/netdevbpf/patch/20240206194751.1901802-2-ericwouds@gmail.com/#25703356

I confirm that setting the PHY ID in DT is **not** needed.

The PHY probes fine and it's possible to read the PHY ID even before
firmware has been loaded.

> 
> >> +		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
> >> +		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
> >> +		reset-assert-us = <10000>;
> >> +		reset-deassert-us = <20000>;
> >> +		phy-mode = "2500base-x";
> >> +		full-duplex;
> >> +		pause;
> >> +		airoha,pnswap-rx;
> >> +
> >> +		leds {
> >> +			#address-cells = <1>;
> >> +			#size-cells = <0>;
> >> +
> >> +			led@0 { /* en8811_a_gpio5 */
> >> +				reg = <0>;
> >> +				color = <LED_COLOR_ID_YELLOW>;
> >> +				function = LED_FUNCTION_LAN;
> >> +				function-enumerator = <1>;
> >
> >Why aren't you simply using a label?
> 
> You mean the comment? I can add it of course like for regulators.
> 
> >> +				default-state = "keep";
> >> +				linux,default-trigger = "netdev";

Using linux,default-trigger = "netdev" will NOT work as intended,
see also the reply to your other patch where you are adding netdev
trigger to dt-bindings.
If you do this, it will seemingly work, but if you check
/sys/class/leds/foo/hw_control
it will always be 0, and hardware offloading will hence never be used.
Please just set the LED trigger in userspace for now.

> >> +			};
> >> +			led@1 { /* en8811_a_gpio4 */
> >> +				reg = <1>;
> >> +				color = <LED_COLOR_ID_GREEN>;
> >> +				function = LED_FUNCTION_LAN;
> >> +				function-enumerator = <2>;
> >> +				default-state = "keep";
> >> +				linux,default-trigger = "netdev";
> >> +			};
> >> +		};
> >> +	};
> >> +
> >> +	phy15: ethernet-phy@15 {
> >> +		reg = <15>;
> >
> >Same here.
> >
> >Cheers,
> >Angelo
> >
> 
> 
> regards Frank
> 

Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini

[AngeloGioacchino Del Regno]

Il 06/05/24 18:00, Frank Wunderlich ha scritto:
> Hi
> 
> Thanks for review.
> 
> Am 6. Mai 2024 14:48:59 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>> Il 05/05/24 18:45, Frank Wunderlich ha scritto:
>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>
>>> Add device Tree for Bananapi R3 Mini SBC.
>>>
>>> Co-developed-by: Eric Woudstra <ericwouds@gmail.com>
>>> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
>>> Co-developed-by: Tianling Shen <cnsztl@gmail.com>
>>> Signed-off-by: Tianling Shen <cnsztl@gmail.com>
>>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>>> ---
>>>    arch/arm64/boot/dts/mediatek/Makefile         |   1 +
>>>    .../mediatek/mt7986a-bananapi-bpi-r3-mini.dts | 486 ++++++++++++++++++
>>>    2 files changed, 487 insertions(+)
>>>    create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
>>> index 37b4ca3a87c9..1763b001ab06 100644
>>> --- a/arch/arm64/boot/dts/mediatek/Makefile
>>> +++ b/arch/arm64/boot/dts/mediatek/Makefile
>>> @@ -11,6 +11,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-xiaomi-ax3000t.dtb
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-acelink-ew-7886cax.dtb
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb
>>> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-mini.dtb
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtbo
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nand.dtbo
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nor.dtbo
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
>>> new file mode 100644
>>> index 000000000000..c764b4dc4752
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
>>> @@ -0,0 +1,486 @@
>>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>>> +/*
>>> + * Copyright (C) 2021 MediaTek Inc.
>>> + * Authors: Frank Wunderlich <frank-w@public-files.de>
>>> + *          Eric Woudstra <ericwouds@gmail.com>
>>> + *          Tianling Shen <cnsztl@immortalwrt.org>
>>> + */
>>> +
>>> +/dts-v1/;
>>> +
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +#include <dt-bindings/input/input.h>
>>> +#include <dt-bindings/leds/common.h>
>>> +#include <dt-bindings/pinctrl/mt65xx.h>
>>> +
>>> +#include "mt7986a.dtsi"
>>> +
>>> +/ {
>>> +	model = "Bananapi BPI-R3 Mini";
>>> +	chassis-type = "embedded";
>>> +	compatible = "bananapi,bpi-r3mini", "mediatek,mt7986a";
>>> +
>>> +	aliases {
>>> +		serial0 = &uart0;
>>> +		ethernet0 = &gmac0;
>>> +		ethernet1 = &gmac1;
>>> +	};
>>> +
>>> +	chosen {
>>> +		stdout-path = "serial0:115200n8";
>>> +	};
>>> +
>>> +	dcin: regulator-12vd {
>>> +		compatible = "regulator-fixed";
>>> +		regulator-name = "12vd";
>>> +		regulator-min-microvolt = <12000000>;
>>> +		regulator-max-microvolt = <12000000>;
>>> +		regulator-boot-on;
>>> +		regulator-always-on;
>>> +	};
>>> +
>>> +	fan: pwm-fan {
>>> +		compatible = "pwm-fan";
>>> +		#cooling-cells = <2>;
>>> +		/* cooling level (0, 1, 2) - pwm inverted */
>>> +		cooling-levels = <255 96 0>;
>>
>> Did you try to actually invert the PWM?
>>
>> Look for PWM_POLARITY_INVERTED ;-)
> 
> Mtk pwm driver does not support it
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/pwm/pwm-mediatek.c#L211
> 

You're right, sorry - I confused the general purpose PWM controller with the
rather specific DISP_PWM controller (which does support polarity inversion).

It's good - but I'd appreciate if you can please add a comment stating that
the PWM values are inverted in SW because the controller does *not* support
polarity inversion... so that next time someone looks at this will immediately
understand what's going on and why :-)

>>> +		pwms = <&pwm 0 10000>;
>>> +		status = "okay";
>>> +	};
>>> +
>>> +	reg_1p8v: regulator-1p8v {
>>> +		compatible = "regulator-fixed";
>>> +		regulator-name = "1.8vd";
>>> +		regulator-min-microvolt = <1800000>;
>>> +		regulator-max-microvolt = <1800000>;
>>> +		regulator-boot-on;
>>> +		regulator-always-on;
>>> +		vin-supply = <&dcin>;
>>> +	};
>>> +
>>> +	reg_3p3v: regulator-3p3v {
>>> +		compatible = "regulator-fixed";
>>> +		regulator-name = "3.3vd";
>>> +		regulator-min-microvolt = <3300000>;
>>> +		regulator-max-microvolt = <3300000>;
>>> +		regulator-boot-on;
>>> +		regulator-always-on;
>>> +		vin-supply = <&dcin>;
>>> +	};
>>> +
>>> +	usb_vbus: regulator-usb-vbus {
>>> +		compatible = "regulator-fixed";
>>> +		regulator-name = "usb_vbus";
>>> +		regulator-min-microvolt = <5000000>;
>>> +		regulator-max-microvolt = <5000000>;
>>> +		gpios = <&pio 20 GPIO_ACTIVE_LOW>;
>>> +		regulator-boot-on;
>>> +	};
>>> +
>>> +	en8811_a: regulator-phy1 {
>>> +		compatible = "regulator-fixed";
>>> +		regulator-name = "phy1";
>>> +		regulator-min-microvolt = <3300000>;
>>> +		regulator-max-microvolt = <3300000>;
>>> +		gpio = <&pio 16 GPIO_ACTIVE_LOW>;
>>> +		regulator-always-on;
>>> +	};
>>> +
>>> +	en8811_b: regulator-phy2 {
>>> +		compatible = "regulator-fixed";
>>> +		regulator-name = "phy2";
>>> +		regulator-min-microvolt = <3300000>;
>>> +		regulator-max-microvolt = <3300000>;
>>> +		gpio = <&pio 17 GPIO_ACTIVE_LOW>;
>>> +		regulator-always-on;
>>> +	};
>>> +
>>> +	leds {
>>> +		compatible = "gpio-leds";
>>> +
>>> +		green_led: led-0 {
>>> +			color = <LED_COLOR_ID_GREEN>;
>>> +			function = LED_FUNCTION_POWER;
>>> +			gpios = <&pio 19 GPIO_ACTIVE_HIGH>;
>>> +			default-state = "on";
>>> +		};
>>> +	};
>>> +
>>> +	gpio-keys {
>>> +		compatible = "gpio-keys";
>>> +
>>> +		reset-key {
>>> +			label = "reset";
>>> +			linux,code = <KEY_RESTART>;
>>> +			gpios = <&pio 7 GPIO_ACTIVE_LOW>;
>>> +		};
>>> +	};
>>> +
>>> +};
>>> +
>>> +&cpu_thermal {
>>> +	cooling-maps {
>>> +		map0 {
>>> +			/* active: set fan to cooling level 2 */
>>> +			cooling-device = <&fan 2 2>;
>>> +			trip = <&cpu_trip_active_high>;
>>> +		};
>>> +
>>> +		map1 {
>>> +			/* active: set fan to cooling level 1 */
>>> +			cooling-device = <&fan 1 1>;
>>> +			trip = <&cpu_trip_active_med>;
>>> +		};
>>> +
>>> +		map2 {
>>> +			/* active: set fan to cooling level 0 */
>>> +			cooling-device = <&fan 0 0>;
>>> +			trip = <&cpu_trip_active_low>;
>>> +		};
>>> +	};
>>> +};
>>> +
>>> +&crypto {
>>> +	status = "okay";
>>> +};
>>> +
>>> +&eth {
>>> +	status = "okay";
>>> +
>>> +	gmac0: mac@0 {
>>> +		compatible = "mediatek,eth-mac";
>>> +		reg = <0>;
>>> +		phy-mode = "2500base-x";
>>> +		phy-handle = <&phy14>;
>>> +	};
>>> +
>>> +	gmac1: mac@1 {
>>> +		compatible = "mediatek,eth-mac";
>>> +		reg = <1>;
>>> +		phy-mode = "2500base-x";
>>> +		phy-handle = <&phy15>;
>>> +	};
>>> +
>>> +	mdio: mdio-bus {
>>> +		#address-cells = <1>;
>>> +		#size-cells = <0>;
>>> +	};
>>> +};
>>> +
>>> +&mmc0 {
>>> +	pinctrl-names = "default", "state_uhs";
>>> +	pinctrl-0 = <&mmc0_pins_default>;
>>> +	pinctrl-1 = <&mmc0_pins_uhs>;
>>> +	vmmc-supply = <&reg_3p3v>;
>>> +	vqmmc-supply = <&reg_1p8v>;
>>> +};
>>> +
>>> +
>>> +&i2c0 {
>>> +	pinctrl-names = "default";
>>> +	pinctrl-0 = <&i2c_pins>;
>>> +	status = "okay";
>>> +
>>> +	/* MAC Address EEPROM */
>>> +	eeprom@50 {
>>> +		compatible = "atmel,24c02";
>>> +		reg = <0x50>;
>>> +
>>> +		address-width = <8>;
>>> +		pagesize = <8>;
>>> +		size = <256>;
>>> +	};
>>> +};
>>> +
>>> +&mdio {
>>
>> You can just move all this stuff to where you declare the mdio-bus....
> 
> Ok,see these 2 lines are already above,so can be dropped here.
> 
>>> +	#address-cells = <1>;
>>> +	#size-cells = <0>;
>>> +
>>> +	phy14: ethernet-phy@14 {
>>
>> I say that this is `phy0: ethernet-phy@14` - because this is the first PHY on this
>> board.
> 
> Ok,i change this and phy15
> 
>>> +		reg = <14>;
>>
>> Uhm.. doesn't this require the ethernet-phy-id03a2.a411 compatible?
> 
> I can add it,but worked without it.
> 
> There was a discussion about that and result was we don't need it in board dts,maybe add compatible in binding example.
> 
> https://patchwork.kernel.org/project/netdevbpf/patch/20240206194751.1901802-2-ericwouds@gmail.com/#25703356
> 

Ah, okay. Leave it then.

>>> +		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
>>> +		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
>>> +		reset-assert-us = <10000>;
>>> +		reset-deassert-us = <20000>;
>>> +		phy-mode = "2500base-x";
>>> +		full-duplex;
>>> +		pause;
>>> +		airoha,pnswap-rx;
>>> +
>>> +		leds {
>>> +			#address-cells = <1>;
>>> +			#size-cells = <0>;
>>> +
>>> +			led@0 { /* en8811_a_gpio5 */
>>> +				reg = <0>;
>>> +				color = <LED_COLOR_ID_YELLOW>;
>>> +				function = LED_FUNCTION_LAN;
>>> +				function-enumerator = <1>;
>>
>> Why aren't you simply using a label?
> 
> You mean the comment? I can add it of course like for regulators.
> 

I mean in place of the function-enumerator... that's practically used to
distinguish between instances, it's not too common to see it, and usually
"label" replaces exactly that - just that, instead of a different number,
it gets a different name with no (usually) meaningless numbers :-)

Cheers!

>>> +				default-state = "keep";
>>> +				linux,default-trigger = "netdev";
>>> +			};
>>> +			led@1 { /* en8811_a_gpio4 */
>>> +				reg = <1>;
>>> +				color = <LED_COLOR_ID_GREEN>;
>>> +				function = LED_FUNCTION_LAN;
>>> +				function-enumerator = <2>;
>>> +				default-state = "keep";
>>> +				linux,default-trigger = "netdev";
>>> +			};
>>> +		};
>>> +	};
>>> +
>>> +	phy15: ethernet-phy@15 {
>>> +		reg = <15>;
>>
>> Same here.
>>
>> Cheers,
>> Angelo
>>
> 
> 
> regards Frank

Aw: Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini

[Frank Wunderlich]

Hi

> Gesendet: Dienstag, 07. Mai 2024 um 15:35 Uhr
> Von: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
>
> Il 06/05/24 18:00, Frank Wunderlich ha scritto:

> >>> +	fan: pwm-fan {
> >>> +		compatible = "pwm-fan";
> >>> +		#cooling-cells = <2>;
> >>> +		/* cooling level (0, 1, 2) - pwm inverted */
> >>> +		cooling-levels = <255 96 0>;
> >>
> >> Did you try to actually invert the PWM?
> >>
> >> Look for PWM_POLARITY_INVERTED ;-)
> >
> > Mtk pwm driver does not support it
> >
> > https://elixir.bootlin.com/linux/latest/source/drivers/pwm/pwm-mediatek.c#L211
> >
>
> You're right, sorry - I confused the general purpose PWM controller with the
> rather specific DISP_PWM controller (which does support polarity inversion).
>
> It's good - but I'd appreciate if you can please add a comment stating that
> the PWM values are inverted in SW because the controller does *not* support
> polarity inversion... so that next time someone looks at this will immediately
> understand what's going on and why :-)

so i would change comment like this:

		/* cooling level (0, 1, 2)
		 * signal is inverted on board
		 * mtk pwm driver does not support
		 * PWM_POLARITY_INVERTED */

> >>> +		pwms = <&pwm 0 10000>;
> >>> +		status = "okay";
> >>> +	};
> >>> +
> >>> +	phy14: ethernet-phy@14 {
...
> >>> +		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
> >>> +		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
> >>> +		reset-assert-us = <10000>;
> >>> +		reset-deassert-us = <20000>;
> >>> +		phy-mode = "2500base-x";
> >>> +		full-duplex;
> >>> +		pause;
> >>> +		airoha,pnswap-rx;
> >>> +
> >>> +		leds {
> >>> +			#address-cells = <1>;
> >>> +			#size-cells = <0>;
> >>> +
> >>> +			led@0 { /* en8811_a_gpio5 */
> >>> +				reg = <0>;
> >>> +				color = <LED_COLOR_ID_YELLOW>;
> >>> +				function = LED_FUNCTION_LAN;
> >>> +				function-enumerator = <1>;
> >>
> >> Why aren't you simply using a label?
> >
> > You mean the comment? I can add it of course like for regulators.
> >
>
> I mean in place of the function-enumerator... that's practically used to
> distinguish between instances, it's not too common to see it, and usually
> "label" replaces exactly that - just that, instead of a different number,
> it gets a different name with no (usually) meaningless numbers :-)

as far as i understand using label also makes "function" property useless, after discussing
this with eric i would drop both on all 4 places by labels like these:

label = "yellow-lan";
label = "green-lan";
...

not sure if we should drop color property too...

> >>> +				default-state = "keep";
> >>> +				linux,default-trigger = "netdev";
> >>> +			};
> >>> +			led@1 { /* en8811_a_gpio4 */
> >>> +				reg = <1>;
> >>> +				color = <LED_COLOR_ID_GREEN>;
> >>> +				function = LED_FUNCTION_LAN;
> >>> +				function-enumerator = <2>;
> >>> +				default-state = "keep";
> >>> +				linux,default-trigger = "netdev";
> >>> +			};
> >>> +		};
> >>> +	};
> >>> +
> >>> +	phy15: ethernet-phy@15 {
> >>> +		reg = <15>;
> >>
> >> Same here.
> >>
> >> Cheers,
> >> Angelo

regards Frank

Re: Aw: Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini

[AngeloGioacchino Del Regno]

Il 08/05/24 20:25, Frank Wunderlich ha scritto:
> Hi
> 
>> Gesendet: Dienstag, 07. Mai 2024 um 15:35 Uhr
>> Von: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
>>
>> Il 06/05/24 18:00, Frank Wunderlich ha scritto:
> 
>>>>> +	fan: pwm-fan {
>>>>> +		compatible = "pwm-fan";
>>>>> +		#cooling-cells = <2>;
>>>>> +		/* cooling level (0, 1, 2) - pwm inverted */
>>>>> +		cooling-levels = <255 96 0>;
>>>>
>>>> Did you try to actually invert the PWM?
>>>>
>>>> Look for PWM_POLARITY_INVERTED ;-)
>>>
>>> Mtk pwm driver does not support it
>>>
>>> https://elixir.bootlin.com/linux/latest/source/drivers/pwm/pwm-mediatek.c#L211
>>>
>>
>> You're right, sorry - I confused the general purpose PWM controller with the
>> rather specific DISP_PWM controller (which does support polarity inversion).
>>
>> It's good - but I'd appreciate if you can please add a comment stating that
>> the PWM values are inverted in SW because the controller does *not* support
>> polarity inversion... so that next time someone looks at this will immediately
>> understand what's going on and why :-)
> 
> so i would change comment like this:
> 
> 		/* cooling level (0, 1, 2)
> 		 * signal is inverted on board
> 		 * mtk pwm driver does not support
> 		 * PWM_POLARITY_INVERTED */
> 

There you go:

/*
  * The signal is inverted on this board and the general purpose
  * PWM HW IP in this SoC does not support polarity inversion.
  */
/* Cooling level < 0  1  2> */
cooling-levels = <255 96 0>;



>>>>> +		pwms = <&pwm 0 10000>;
>>>>> +		status = "okay";
>>>>> +	};
>>>>> +
>>>>> +	phy14: ethernet-phy@14 {
> ...
>>>>> +		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
>>>>> +		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
>>>>> +		reset-assert-us = <10000>;
>>>>> +		reset-deassert-us = <20000>;
>>>>> +		phy-mode = "2500base-x";
>>>>> +		full-duplex;
>>>>> +		pause;
>>>>> +		airoha,pnswap-rx;
>>>>> +
>>>>> +		leds {
>>>>> +			#address-cells = <1>;
>>>>> +			#size-cells = <0>;
>>>>> +
>>>>> +			led@0 { /* en8811_a_gpio5 */
>>>>> +				reg = <0>;
>>>>> +				color = <LED_COLOR_ID_YELLOW>;
>>>>> +				function = LED_FUNCTION_LAN;
>>>>> +				function-enumerator = <1>;
>>>>
>>>> Why aren't you simply using a label?
>>>
>>> You mean the comment? I can add it of course like for regulators.
>>>
>>
>> I mean in place of the function-enumerator... that's practically used to
>> distinguish between instances, it's not too common to see it, and usually
>> "label" replaces exactly that - just that, instead of a different number,
>> it gets a different name with no (usually) meaningless numbers :-)
> 
> as far as i understand using label also makes "function" property useless, after discussing
> this with eric i would drop both on all 4 places by labels like these:
> 
> label = "yellow-lan";
> label = "green-lan";
> ...
> 
> not sure if we should drop color property too...
> 

I'm looking at the leds binding (leds/common.yaml) right now.

My suggestion of using 'label' was actually wrong - and your devicetree was
actually right!!! (apart from the default-trigger that may not work)

Infact, the documentation says, in brief:

- function-enumerator is ignored if label is present
- function doesn't say that gets ignored
- color doesn't say that gets ignored
- label says:
   - If not present -> get string from node name
   - function-enumerator ignored
   - This property is deprecated

...but the 'label' binding does not say 'deprecated: true', which is something
that must be fixed!


So, I'm sorry for the confusion, the noise and the useless loss of time around
this - you can keep the LED nodes as they are, and that's a lesson for the future
me reviewing another node like this one.

P.S.: This shouldn't have been a RFC, as the patches are more than RFC quality!!!

Cheers,
Angelo

>>>>> +				default-state = "keep";
>>>>> +				linux,default-trigger = "netdev";
>>>>> +			};
>>>>> +			led@1 { /* en8811_a_gpio4 */
>>>>> +				reg = <1>;
>>>>> +				color = <LED_COLOR_ID_GREEN>;
>>>>> +				function = LED_FUNCTION_LAN;
>>>>> +				function-enumerator = <2>;
>>>>> +				default-state = "keep";
>>>>> +				linux,default-trigger = "netdev";
>>>>> +			};
>>>>> +		};
>>>>> +	};
>>>>> +
>>>>> +	phy15: ethernet-phy@15 {
>>>>> +		reg = <15>;
>>>>
>>>> Same here.
>>>>
>>>> Cheers,
>>>> Angelo
> 
> regards Frank

Re: Aw: Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini

[Frank Wunderlich]

Am 9. Mai 2024 12:10:59 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>Il 08/05/24 20:25, Frank Wunderlich ha scritto:
>> Hi
>> 
>>> Gesendet: Dienstag, 07. Mai 2024 um 15:35 Uhr
>>> Von: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
>>> 
>>> Il 06/05/24 18:00, Frank Wunderlich ha scritto:
>> 
>>>>>> +	fan: pwm-fan {
>>>>>> +		compatible = "pwm-fan";
>>>>>> +		#cooling-cells = <2>;
>>>>>> +		/* cooling level (0, 1, 2) - pwm inverted */
>>>>>> +		cooling-levels = <255 96 0>;
>>>>> 
>>>>> Did you try to actually invert the PWM?
>>>>> 
>>>>> Look for PWM_POLARITY_INVERTED ;-)
>>>> 
>>>> Mtk pwm driver does not support it
>>>> 
>>>> https://elixir.bootlin.com/linux/latest/source/drivers/pwm/pwm-mediatek.c#L211
>>>> 
>>> 
>>> You're right, sorry - I confused the general purpose PWM controller with the
>>> rather specific DISP_PWM controller (which does support polarity inversion).
>>> 
>>> It's good - but I'd appreciate if you can please add a comment stating that
>>> the PWM values are inverted in SW because the controller does *not* support
>>> polarity inversion... so that next time someone looks at this will immediately
>>> understand what's going on and why :-)
>> 
>> so i would change comment like this:
>> 
>> 		/* cooling level (0, 1, 2)
>> 		 * signal is inverted on board
>> 		 * mtk pwm driver does not support
>> 		 * PWM_POLARITY_INVERTED */
>> 
>
>There you go:
>
>/*
> * The signal is inverted on this board and the general purpose
> * PWM HW IP in this SoC does not support polarity inversion.
> */
>/* Cooling level < 0  1  2> */
>cooling-levels = <255 96 0>;

Thanks for clearing structure of the comment,but imho actually it is a driver issue (for all mtk SoC). Not sure it is really a hardware limitation. So i would change this to "... and the PWM driver does not support polarity inversion."

>>>>>> +		pwms = <&pwm 0 10000>;
>>>>>> +		status = "okay";
>>>>>> +	};
>>>>>> +
>>>>>> +	phy14: ethernet-phy@14 {
>> ...
>>>>>> +		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
>>>>>> +		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
>>>>>> +		reset-assert-us = <10000>;
>>>>>> +		reset-deassert-us = <20000>;
>>>>>> +		phy-mode = "2500base-x";
>>>>>> +		full-duplex;
>>>>>> +		pause;
>>>>>> +		airoha,pnswap-rx;
>>>>>> +
>>>>>> +		leds {
>>>>>> +			#address-cells = <1>;
>>>>>> +			#size-cells = <0>;
>>>>>> +
>>>>>> +			led@0 { /* en8811_a_gpio5 */
>>>>>> +				reg = <0>;
>>>>>> +				color = <LED_COLOR_ID_YELLOW>;
>>>>>> +				function = LED_FUNCTION_LAN;
>>>>>> +				function-enumerator = <1>;
>>>>> 
>>>>> Why aren't you simply using a label?
>>>> 
>>>> You mean the comment? I can add it of course like for regulators.
>>>> 
>>> 
>>> I mean in place of the function-enumerator... that's practically used to
>>> distinguish between instances, it's not too common to see it, and usually
>>> "label" replaces exactly that - just that, instead of a different number,
>>> it gets a different name with no (usually) meaningless numbers :-)
>> 
>> as far as i understand using label also makes "function" property useless, after discussing
>> this with eric i would drop both on all 4 places by labels like these:
>> 
>> label = "yellow-lan";
>> label = "green-lan";
>> ...
>> 
>> not sure if we should drop color property too...
>> 
>
>I'm looking at the leds binding (leds/common.yaml) right now.
>
>My suggestion of using 'label' was actually wrong - and your devicetree was
>actually right!!! (apart from the default-trigger that may not work)
>
>Infact, the documentation says, in brief:
>
>- function-enumerator is ignored if label is present
>- function doesn't say that gets ignored
>- color doesn't say that gets ignored
>- label says:
>  - If not present -> get string from node name
>  - function-enumerator ignored
>  - This property is deprecated
>
>...but the 'label' binding does not say 'deprecated: true', which is something
>that must be fixed!

Ok,i can try to add the property to binding (independ of this series). Imho label was cleaner than function and function-enumerator...

>So, I'm sorry for the confusion, the noise and the useless loss of time around
>this - you can keep the LED nodes as they are, and that's a lesson for the future
>me reviewing another node like this one.

Don't worry, we are all humas...i missed looking in linux-next for the other binding-patches.

>P.S.: This shouldn't have been a RFC, as the patches are more than RFC quality!!!

I sent it as RFC because i had not expected to be merged before next is closed.

>Cheers,
>Angelo
>
>>>>>> +				default-state = "keep";
>>>>>> +				linux,default-trigger = "netdev";
>>>>>> +			};
>>>>>> +			led@1 { /* en8811_a_gpio4 */
>>>>>> +				reg = <1>;
>>>>>> +				color = <LED_COLOR_ID_GREEN>;
>>>>>> +				function = LED_FUNCTION_LAN;
>>>>>> +				function-enumerator = <2>;
>>>>>> +				default-state = "keep";
>>>>>> +				linux,default-trigger = "netdev";
>>>>>> +			};
>>>>>> +		};
>>>>>> +	};
>>>>>> +
>>>>>> +	phy15: ethernet-phy@15 {
>>>>>> +		reg = <15>;


regards Frank

Re: Aw: Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini

[AngeloGioacchino Del Regno]

Il 09/05/24 12:30, Frank Wunderlich ha scritto:
> Am 9. Mai 2024 12:10:59 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>> Il 08/05/24 20:25, Frank Wunderlich ha scritto:
>>> Hi
>>>
>>>> Gesendet: Dienstag, 07. Mai 2024 um 15:35 Uhr
>>>> Von: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
>>>>
>>>> Il 06/05/24 18:00, Frank Wunderlich ha scritto:
>>>
>>>>>>> +	fan: pwm-fan {
>>>>>>> +		compatible = "pwm-fan";
>>>>>>> +		#cooling-cells = <2>;
>>>>>>> +		/* cooling level (0, 1, 2) - pwm inverted */
>>>>>>> +		cooling-levels = <255 96 0>;
>>>>>>
>>>>>> Did you try to actually invert the PWM?
>>>>>>
>>>>>> Look for PWM_POLARITY_INVERTED ;-)
>>>>>
>>>>> Mtk pwm driver does not support it
>>>>>
>>>>> https://elixir.bootlin.com/linux/latest/source/drivers/pwm/pwm-mediatek.c#L211
>>>>>
>>>>
>>>> You're right, sorry - I confused the general purpose PWM controller with the
>>>> rather specific DISP_PWM controller (which does support polarity inversion).
>>>>
>>>> It's good - but I'd appreciate if you can please add a comment stating that
>>>> the PWM values are inverted in SW because the controller does *not* support
>>>> polarity inversion... so that next time someone looks at this will immediately
>>>> understand what's going on and why :-)
>>>
>>> so i would change comment like this:
>>>
>>> 		/* cooling level (0, 1, 2)
>>> 		 * signal is inverted on board
>>> 		 * mtk pwm driver does not support
>>> 		 * PWM_POLARITY_INVERTED */
>>>
>>
>> There you go:
>>
>> /*
>> * The signal is inverted on this board and the general purpose
>> * PWM HW IP in this SoC does not support polarity inversion.
>> */
>> /* Cooling level < 0  1  2> */
>> cooling-levels = <255 96 0>;
> 
> Thanks for clearing structure of the comment,but imho actually it is a driver issue (for all mtk SoC). Not sure it is really a hardware limitation. So i would change this to "... and the PWM driver does not support polarity inversion."
> 
>>>>>>> +		pwms = <&pwm 0 10000>;
>>>>>>> +		status = "okay";
>>>>>>> +	};
>>>>>>> +
>>>>>>> +	phy14: ethernet-phy@14 {
>>> ...
>>>>>>> +		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
>>>>>>> +		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
>>>>>>> +		reset-assert-us = <10000>;
>>>>>>> +		reset-deassert-us = <20000>;
>>>>>>> +		phy-mode = "2500base-x";
>>>>>>> +		full-duplex;
>>>>>>> +		pause;
>>>>>>> +		airoha,pnswap-rx;
>>>>>>> +
>>>>>>> +		leds {
>>>>>>> +			#address-cells = <1>;
>>>>>>> +			#size-cells = <0>;
>>>>>>> +
>>>>>>> +			led@0 { /* en8811_a_gpio5 */
>>>>>>> +				reg = <0>;
>>>>>>> +				color = <LED_COLOR_ID_YELLOW>;
>>>>>>> +				function = LED_FUNCTION_LAN;
>>>>>>> +				function-enumerator = <1>;
>>>>>>
>>>>>> Why aren't you simply using a label?
>>>>>
>>>>> You mean the comment? I can add it of course like for regulators.
>>>>>
>>>>
>>>> I mean in place of the function-enumerator... that's practically used to
>>>> distinguish between instances, it's not too common to see it, and usually
>>>> "label" replaces exactly that - just that, instead of a different number,
>>>> it gets a different name with no (usually) meaningless numbers :-)
>>>
>>> as far as i understand using label also makes "function" property useless, after discussing
>>> this with eric i would drop both on all 4 places by labels like these:
>>>
>>> label = "yellow-lan";
>>> label = "green-lan";
>>> ...
>>>
>>> not sure if we should drop color property too...
>>>
>>
>> I'm looking at the leds binding (leds/common.yaml) right now.
>>
>> My suggestion of using 'label' was actually wrong - and your devicetree was
>> actually right!!! (apart from the default-trigger that may not work)
>>
>> Infact, the documentation says, in brief:
>>
>> - function-enumerator is ignored if label is present
>> - function doesn't say that gets ignored
>> - color doesn't say that gets ignored
>> - label says:
>>   - If not present -> get string from node name
>>   - function-enumerator ignored
>>   - This property is deprecated
>>
>> ...but the 'label' binding does not say 'deprecated: true', which is something
>> that must be fixed!
> 
> Ok,i can try to add the property to binding (independ of this series). Imho label was cleaner than function and function-enumerator...
> 

Oh I sort of agree with you, I liked the label more, as it's more consistent with
everything else... but oh well. :-)

>> So, I'm sorry for the confusion, the noise and the useless loss of time around
>> this - you can keep the LED nodes as they are, and that's a lesson for the future
>> me reviewing another node like this one.
> 
> Don't worry, we are all humas...i missed looking in linux-next for the other binding-patches.
> 
>> P.S.: This shouldn't have been a RFC, as the patches are more than RFC quality!!!
> 
> I sent it as RFC because i had not expected to be merged before next is closed.
> 

Ah at least from my side, no worries... when I see RFC I generally expect to see
"dubious/head-scratching stuff", not "sub-optimal timing to send a patch" :-P

Cheers!
Angelo

>>
>>>>>>> +				default-state = "keep";
>>>>>>> +				linux,default-trigger = "netdev";
>>>>>>> +			};
>>>>>>> +			led@1 { /* en8811_a_gpio4 */
>>>>>>> +				reg = <1>;
>>>>>>> +				color = <LED_COLOR_ID_GREEN>;
>>>>>>> +				function = LED_FUNCTION_LAN;
>>>>>>> +				function-enumerator = <2>;
>>>>>>> +				default-state = "keep";
>>>>>>> +				linux,default-trigger = "netdev";
>>>>>>> +			};
>>>>>>> +		};
>>>>>>> +	};
>>>>>>> +
>>>>>>> +	phy15: ethernet-phy@15 {
>>>>>>> +		reg = <15>;
> 
> 
> regards Frank

Re: [RFC v1 0/5] Add Bananapi R3 Mini

[Rob Herring (Arm)]

On Sun, 05 May 2024 18:45:44 +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add mt7986 based BananaPi R3 Mini SBC and fix some related binding errors.
> 
> Frank Wunderlich (5):
>   dt-bindings: leds: add led trigger netdev
>   dt-bindings: clock: mediatek: add address-cells and size-cells to
>     ethsys
>   dt-bindings: net: mediatek,net: add reset-cells
>   dt-bindings: arm64: dts: mediatek: add BananaPi R3 Mini
>   arm64: dts: mediatek: Add  mt7986 based Bananapi R3 Mini
> 
>  .../devicetree/bindings/arm/mediatek.yaml     |   1 +
>  .../bindings/clock/mediatek,ethsys.yaml       |   6 +
>  .../devicetree/bindings/leds/common.yaml      |   2 +
>  .../devicetree/bindings/net/mediatek,net.yaml |   3 +
>  arch/arm64/boot/dts/mediatek/Makefile         |   1 +
>  .../mediatek/mt7986a-bananapi-bpi-r3-mini.dts | 486 ++++++++++++++++++
>  6 files changed, 499 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> 
> --
> 2.34.1
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y mediatek/mt7986a-bananapi-bpi-r3-mini.dtb' for 20240505164549.65644-1-linux@fw-web.de:

arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dtb: crypto@10320000: interrupts: [[0, 116, 4], [0, 117, 4], [0, 118, 4], [0, 119, 4]] is too short
	from schema $id: http://devicetree.org/schemas/crypto/inside-secure,safexcel.yaml#
arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dtb: crypto@10320000: interrupt-names: ['ring0', 'ring1', 'ring2', 'ring3'] is too short
	from schema $id: http://devicetree.org/schemas/crypto/inside-secure,safexcel.yaml#