[PATCH net-next v4 0/2] net: phy: add driver for MediaTek SoC built-in GE PHYs

[PATCH net-next v4 0/2] net: phy: add driver for MediaTek SoC built-in GE PHYs

[Daniel Golle]

Some of MediaTek's Filogic SoCs come with built-in gigabit Ethernet
PHYs which require calibration data from the SoC's efuse.
Despite the similar design the driver doesn't share any code with the
existing mediatek-ge.c, so add support for these PHYs by introducing a
new driver for only MediaTek's ARM64 SoCs.

As the PHYs integrated in the MT7988 SoC require reading the polarity
of the LEDs from the SoCs's boottrap also add dt-binding for that.

All LEDs are for now setup with default values, a follow up patch which
allows custom LED setups will be sent after the PHY LED framework is
more in shape.

Changes since v3:
 * fix spelling and reverse xmas tree
 * add dt-binding for mediatek,boottrap

Changes since v2:
 * remove everything related to PHY LEDs for now, LED support will
   be cleaned up and submitted once PHY LED framework is more ready

Changes since v1:
 * split-off SoC-specific driver from mediatek-ge.c as requested
 * address comments made by Heiner Kallweit
 * add pinctrl handling for PHY LED
 * remove calibration details not needed in production hardware

Daniel Golle (2):
  dt-bindings: arm: mediatek: add mediatek,boottrap binding
  net: phy: add driver for MediaTek SoC built-in GE PHYs

 .../arm/mediatek/mediatek,boottrap.yaml       |   37 +
 MAINTAINERS                                   |    9 +
 drivers/net/phy/Kconfig                       |   12 +
 drivers/net/phy/Makefile                      |    1 +
 drivers/net/phy/mediatek-ge-soc.c             | 1264 +++++++++++++++++
 drivers/net/phy/mediatek-ge.c                 |    3 +-
 6 files changed, 1325 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,boottrap.yaml
 create mode 100644 drivers/net/phy/mediatek-ge-soc.c


base-commit: 285b2a46953cecea207c53f7c6a7a76c9bbab303
-- 
2.40.0

[PATCH net-next v4 1/2] dt-bindings: arm: mediatek: add mediatek,boottrap binding

[Daniel Golle]

The boottrap is used to read implementation details from the SoC, such
as the polarity of LED pins. Add bindings for it as we are going to use
it for the LEDs connected to MediaTek built-in 1GE PHYs.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 .../arm/mediatek/mediatek,boottrap.yaml       | 37 +++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,boottrap.yaml

diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,boottrap.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,boottrap.yaml
new file mode 100644
index 000000000000..460e375320a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,boottrap.yaml
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/mediatek/mediatek,boottrap.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek boottrap
+
+maintainers:
+  - Daniel Golle <daniel@makrotopia.org>
+
+description:
+  The boottrap found in some MediaTek SoCs is used to read SoC implementation
+  details such as LED polarities.
+
+properties:
+  $nodename:
+    const: boottrap
+
+  compatible:
+    const: mediatek,boottrap
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    boottrap: boottrap@1001f6f0 {
+      compatible = "mediatek,boottrap";
+      reg = <0 0x1001f6f0 0 0x20>;
+    };
-- 
2.40.0

Re: [PATCH net-next v4 1/2] dt-bindings: arm: mediatek: add mediatek,boottrap binding

[Andrew Lunn]

On Thu, May 11, 2023 at 04:10:20PM +0200, Daniel Golle wrote:
> The boottrap is used to read implementation details from the SoC, such
> as the polarity of LED pins. Add bindings for it as we are going to use
> it for the LEDs connected to MediaTek built-in 1GE PHYs.

What exactly is it? Fuses? Is it memory mapped, or does it need a
driver to access it? How is it shared between its different users?

Thank
	Andrew

Re: [PATCH net-next v4 1/2] dt-bindings: arm: mediatek: add mediatek,boottrap binding

[Krzysztof Kozlowski]

On Thu, 11 May 2023 16:10:20 +0200, Daniel Golle wrote:
> The boottrap is used to read implementation details from the SoC, such
> as the polarity of LED pins. Add bindings for it as we are going to use
> it for the LEDs connected to MediaTek built-in 1GE PHYs.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  .../arm/mediatek/mediatek,boottrap.yaml       | 37 +++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,boottrap.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/mediatek/mediatek,boottrap.example.dtb: boottrap@1001f6f0: $nodename:0: 'boottrap' was expected
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/mediatek/mediatek,boottrap.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/mediatek/mediatek,boottrap.example.dtb: boottrap@1001f6f0: reg: [[0, 268564208], [0, 32]] is too long
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/mediatek/mediatek,boottrap.yaml


See https://patchwork.ozlabs.org/patch/1780124

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH net-next v4 1/2] dt-bindings: arm: mediatek: add mediatek,boottrap binding

[Krzysztof Kozlowski]

On 11/05/2023 17:53, Andrew Lunn wrote:
> On Thu, May 11, 2023 at 04:10:20PM +0200, Daniel Golle wrote:
>> The boottrap is used to read implementation details from the SoC, such
>> as the polarity of LED pins. Add bindings for it as we are going to use
>> it for the LEDs connected to MediaTek built-in 1GE PHYs.
> 
> What exactly is it? Fuses? Is it memory mapped, or does it need a
> driver to access it? How is it shared between its different users?

Yes, looks like some efuse/OTP/nvmem, so it should probably use nvmem
bindings and do not look different than other in such class.

Best regards,
Krzysztof

Re: [PATCH net-next v4 1/2] dt-bindings: arm: mediatek: add mediatek,boottrap binding

[Daniel Golle]

On Fri, May 12, 2023 at 08:54:36AM +0200, Krzysztof Kozlowski wrote:
> On 11/05/2023 17:53, Andrew Lunn wrote:
> > On Thu, May 11, 2023 at 04:10:20PM +0200, Daniel Golle wrote:
> >> The boottrap is used to read implementation details from the SoC, such
> >> as the polarity of LED pins. Add bindings for it as we are going to use
> >> it for the LEDs connected to MediaTek built-in 1GE PHYs.
> > 
> > What exactly is it? Fuses? Is it memory mapped, or does it need a
> > driver to access it? How is it shared between its different users?
> 
> Yes, looks like some efuse/OTP/nvmem, so it should probably use nvmem
> bindings and do not look different than other in such class.

I've asked MediaTek and they have replied with an elaborate definition.
Summary:
The boottrap is a single 32-bit wide register at 0x1001f6f0 which can
be used to read back the bias of bootstrap pins from the SoC as follows:

* bit[8]: Reference CLK source && gphy port0's LED
If bit[8] == 0:
- Reference clock source is XTRL && gphy port0's LED is pulled low on board side
If bit[8] == 1:
- Reference clock source is Oscillator && gphy port0's LED is pulled high on board side

* bit[9]: DDR type && gphy port1's LED
If bit[9] == 0:
- DDR type is DDRx16b x2 && gphy port1's LED is pulled low on board side
If bit[9] == 1:
- DDR type is DDRx16b x1 && gphy port1's LED is pulled high on board side

* bit[10]: gphy port2's LED
If bit[10] == 0:
- phy port2's LED is pulled low on board side
If bit[10] == 1:
- gphy port2's LED is pulled high on board side

* bit[11]: gphy port3's LED
If bit[11] == 0:
- phy port3's LED is pulled low on board side
If bit[11] == 1:
- gphy port3's LED is pulled high on board side

If bit[10] == 0 && bit[11] == 0:
- BROM will boot from SPIM-NOR
If bit[10] == 1 && bit[11] == 0:
- BROM will boot from SPIM-NAND
If bit[10] == 0 && bit[11] == 1:
- BROM will boot from eMMC
If bit[10] == 1 && bit[11] == 1:
- BROM will boot from SNFI-NAND

The boottrap is present in many MediaTek SoCs, however, support for
reading it is only really needed on MT7988 due to the dual-use of some
bootstrap pins as PHY LEDs.

We could say this is some kind of read-only 'syscon' node (and hence
use regmap driver to access it), that would make it easy but it's not
very accurate. Also efuse/OTP/nvmem doesn't seem accurate, though in
terms of software it could work just as well.

I will update DT bindings to contain the gained insights.

Please advise if any existing driver (syscon/regmap or efuse/OTP/nvmem)
should be used or if it's ok to just use plain mmio in the PHY driver.


Best regards


Daniel

Re: [PATCH net-next v4 1/2] dt-bindings: arm: mediatek: add mediatek,boottrap binding

[Krzysztof Kozlowski]

On 18/05/2023 04:44, Daniel Golle wrote:
> On Fri, May 12, 2023 at 08:54:36AM +0200, Krzysztof Kozlowski wrote:
>> On 11/05/2023 17:53, Andrew Lunn wrote:
>>> On Thu, May 11, 2023 at 04:10:20PM +0200, Daniel Golle wrote:
>>>> The boottrap is used to read implementation details from the SoC, such
>>>> as the polarity of LED pins. Add bindings for it as we are going to use
>>>> it for the LEDs connected to MediaTek built-in 1GE PHYs.
>>>
>>> What exactly is it? Fuses? Is it memory mapped, or does it need a
>>> driver to access it? How is it shared between its different users?
>>
>> Yes, looks like some efuse/OTP/nvmem, so it should probably use nvmem
>> bindings and do not look different than other in such class.
> 
> I've asked MediaTek and they have replied with an elaborate definition.
> Summary:
> The boottrap is a single 32-bit wide register at 0x1001f6f0 which can
> be used to read back the bias of bootstrap pins from the SoC as follows:

Is it within some other address space? Register address suggests that.

In such case you should not create a device in the middle of other
device's address space. You punched a hole in uniform address space
which prevents creating that other device for entire space.

> 
> * bit[8]: Reference CLK source && gphy port0's LED
> If bit[8] == 0:
> - Reference clock source is XTRL && gphy port0's LED is pulled low on board side
> If bit[8] == 1:
> - Reference clock source is Oscillator && gphy port0's LED is pulled high on board side
> 
> * bit[9]: DDR type && gphy port1's LED
> If bit[9] == 0:
> - DDR type is DDRx16b x2 && gphy port1's LED is pulled low on board side
> If bit[9] == 1:
> - DDR type is DDRx16b x1 && gphy port1's LED is pulled high on board side
> 
> * bit[10]: gphy port2's LED
> If bit[10] == 0:
> - phy port2's LED is pulled low on board side
> If bit[10] == 1:
> - gphy port2's LED is pulled high on board side
> 
> * bit[11]: gphy port3's LED
> If bit[11] == 0:
> - phy port3's LED is pulled low on board side
> If bit[11] == 1:
> - gphy port3's LED is pulled high on board side
> 
> If bit[10] == 0 && bit[11] == 0:
> - BROM will boot from SPIM-NOR
> If bit[10] == 1 && bit[11] == 0:
> - BROM will boot from SPIM-NAND
> If bit[10] == 0 && bit[11] == 1:
> - BROM will boot from eMMC
> If bit[10] == 1 && bit[11] == 1:
> - BROM will boot from SNFI-NAND
> 
> The boottrap is present in many MediaTek SoCs, however, support for
> reading it is only really needed on MT7988 due to the dual-use of some
> bootstrap pins as PHY LEDs.
> 
> We could say this is some kind of read-only 'syscon' node (and hence
> use regmap driver to access it), that would make it easy but it's not
> very accurate. Also efuse/OTP/nvmem doesn't seem accurate, though in
> terms of software it could work just as well.
> 
> I will update DT bindings to contain the gained insights.

If this is separate address space with one register, then boottrap
sounds ok. If you have multiple read only registers with fused values,
then this is efuse region, so something like nvidia,tegra20-efuse.


Best regards,
Krzysztof

Re: [PATCH net-next v4 1/2] dt-bindings: arm: mediatek: add mediatek,boottrap binding

[Krzysztof Kozlowski]

On 18/05/2023 09:50, Krzysztof Kozlowski wrote:
> On 18/05/2023 04:44, Daniel Golle wrote:
>> On Fri, May 12, 2023 at 08:54:36AM +0200, Krzysztof Kozlowski wrote:
>>> On 11/05/2023 17:53, Andrew Lunn wrote:
>>>> On Thu, May 11, 2023 at 04:10:20PM +0200, Daniel Golle wrote:
>>>>> The boottrap is used to read implementation details from the SoC, such
>>>>> as the polarity of LED pins. Add bindings for it as we are going to use
>>>>> it for the LEDs connected to MediaTek built-in 1GE PHYs.
>>>>
>>>> What exactly is it? Fuses? Is it memory mapped, or does it need a
>>>> driver to access it? How is it shared between its different users?
>>>
>>> Yes, looks like some efuse/OTP/nvmem, so it should probably use nvmem
>>> bindings and do not look different than other in such class.
>>
>> I've asked MediaTek and they have replied with an elaborate definition.
>> Summary:
>> The boottrap is a single 32-bit wide register at 0x1001f6f0 which can
>> be used to read back the bias of bootstrap pins from the SoC as follows:
> 
> Is it within some other address space? Register address suggests that.
> 
> In such case you should not create a device in the middle of other
> device's address space. You punched a hole in uniform address space
> which prevents creating that other device for entire space.
> 
>>
>> * bit[8]: Reference CLK source && gphy port0's LED
>> If bit[8] == 0:
>> - Reference clock source is XTRL && gphy port0's LED is pulled low on board side
>> If bit[8] == 1:
>> - Reference clock source is Oscillator && gphy port0's LED is pulled high on board side
>>
>> * bit[9]: DDR type && gphy port1's LED
>> If bit[9] == 0:
>> - DDR type is DDRx16b x2 && gphy port1's LED is pulled low on board side
>> If bit[9] == 1:
>> - DDR type is DDRx16b x1 && gphy port1's LED is pulled high on board side
>>
>> * bit[10]: gphy port2's LED
>> If bit[10] == 0:
>> - phy port2's LED is pulled low on board side
>> If bit[10] == 1:
>> - gphy port2's LED is pulled high on board side
>>
>> * bit[11]: gphy port3's LED
>> If bit[11] == 0:
>> - phy port3's LED is pulled low on board side
>> If bit[11] == 1:
>> - gphy port3's LED is pulled high on board side
>>
>> If bit[10] == 0 && bit[11] == 0:
>> - BROM will boot from SPIM-NOR
>> If bit[10] == 1 && bit[11] == 0:
>> - BROM will boot from SPIM-NAND
>> If bit[10] == 0 && bit[11] == 1:
>> - BROM will boot from eMMC
>> If bit[10] == 1 && bit[11] == 1:
>> - BROM will boot from SNFI-NAND
>>
>> The boottrap is present in many MediaTek SoCs, however, support for
>> reading it is only really needed on MT7988 due to the dual-use of some
>> bootstrap pins as PHY LEDs.
>>
>> We could say this is some kind of read-only 'syscon' node (and hence
>> use regmap driver to access it), that would make it easy but it's not
>> very accurate. Also efuse/OTP/nvmem doesn't seem accurate, though in
>> terms of software it could work just as well.
>>
>> I will update DT bindings to contain the gained insights.
> 
> If this is separate address space with one register, then boottrap
> sounds ok. If you have multiple read only registers with fused values,
> then this is efuse region, so something like nvidia,tegra20-efuse.

Please align together on some common solution. It looks like you are
solving the same problem:

https://lore.kernel.org/all/?q=%22nvmem%3A+syscon%3A+Add+syscon+backed+nvmem+driver%22

Best regards,
Krzysztof