boards: stm: arduino_portenta_h7: support onboard ethernet + pf1550 driver #76542

facchinm · 2024-07-31T10:43:33Z

This PR adds support for the onboard ethernet phy on Arduino Portenta H7.
To properly provide the 1V2 rail for the PHY, I also implemented a driver for the PF1550 PMIC (nice side effect 🙂 ).

The driver is feature complete for our factory OTP programming on the A0 variant. Other variants are untested (the only difference would be BUCK1/2 voltage ranges)

~~On the charger side, I'd need some guidance on how to expose the led configuration via device tree (at the moment it's hardcoded in the init function~~

github-actions · 2024-07-31T10:44:10Z

Hello @facchinm, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

fabiobaltieri

Good stuff, check your compliance error as well (you can also run them locally ./scripts/ci/check_compliance.py -c ..., also the commits need al la description and signed-off-by

drivers/charger/charger_pf1550.c

fabiobaltieri · 2024-07-31T14:21:14Z

drivers/charger/charger_pf1550.c

+	enum {
+		PF1550_CHARGER_OFF_LINEAR_OFF,
+		PF1550_CHARGER_OFF_LINEAR_ON,
+		PF1550_CHARGER_ON_LINEAR_ON,
+	};


just use a define for PF1550_CHARGER_ON_LINEAR_ON outside of here?

drivers/charger/charger_pf1550.c

drivers/mfd/mfd_pf1550.c

fabiobaltieri · 2024-07-31T14:36:22Z

drivers/regulator/Kconfig.pf1550

+config REGULATOR_NXP_PF1550_INIT_PRIORITY
+	int "PF1550 regulator driver init priority"
+	default 87
+	help
+	  Init priority for the NXP PF1550 regulator driver. It must be
+	  greater than REGULATOR_NXP_PF1550_COMMON_INIT_PRIORITY


if you use the same priority value on all devices they should all get sequence correctly thanks to the subpriority value, want to try that? may get rid of few symbols, I would keep the MFD one for all of them

drivers/regulator/regulator_pf1550.c

rriveramcrus · 2024-07-31T15:55:27Z

@facchinm, just a helpful tip, quite a few of the indentation and line wrapping issues can be fixed with a git clang-format :)

dts/bindings/charger/nxp,pf1550-charger.yaml

drivers/charger/charger_pf1550.c

drivers/mfd/mfd_pf1550.c

drivers/regulator/regulator_pf1550.c

gmarull · 2024-08-05T07:52:35Z

dts/bindings/charger/nxp,pf1550-charger.yaml

+  constant-charge-current-max-microamp:
+    required: true
+    type: int
+
+  vbus-current-limit-microamp:
+    type: int
+    required: true
+
+  constant-charge-voltage-max-microvolt:
+    type: int
+    required: true
+
+  system-voltage-min-threshold-microvolt:
+    type: int
+    required: true
+    enum:
+      - 3500000
+      - 3700000
+      - 4300000
+    description: |
+      System voltage minimum threshold.
+
+  thermistor-monitoring-mode:
+    type: string
+    required: true
+    enum:
+      - "disabled"
+      - "thermistor"
+      - "JEITA-1"
+      - "JEITA-2"
+    description: |
+      Thermistor monitoring mode.
+      Refer to ThrmCfg register description and Table 2 for details.
+
+  int-gpios:
+    type: phandle-array
+    required: true
+    description: Interrupt pin


please, prefix vendor specific properties, eg nxp,prop-name.

boards/arduino/portenta_h7/arduino_portenta_h7_stm32h747xx_m7_defconfig

gmarull · 2024-08-05T07:54:48Z

boards/arduino/portenta_h7/Kconfig.defconfig

+config REGULATOR
+	default y if NETWORKING


this looks wrong: device driver relying on regulator to be used should select this option

Can you expand on this? If I understand correctly, the proposal would be to move the explicit configuration to a regulator field for the ethernet phy in the board dts, right?

Networking does not directly depend on regulators, but some device driver. This driver should be the one selecting regulator enablement. The reality though is that regulator usage is nowadays not great, we have no infrastructure for automatically turning them on/off on a device level. So maybe the defconfig should be linked to the device itself, at least future refactors will be easier to spot what's going on.

github-actions · 2024-10-05T00:32:04Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

pillo79 · 2025-02-12T08:13:42Z

@gmarull concerns have been addressed, please revisit your -1. Thanks!

gmarull

a few minor nits, otherwise lgtm

gmarull · 2025-02-12T08:27:09Z

boards/arduino/portenta_h7/arduino_portenta_h7_stm32h747xx_m7_defconfig

+# Enable charger
+CONFIG_CHARGER=y
+
+# Enable USB Stack
+CONFIG_USB_DEVICE_STACK=y


boards need to enable the minimum, e.g. to run hello world, is charger/usb stack needed there? If not, drop

gmarull · 2025-02-12T08:27:18Z

boards/arduino/portenta_h7/arduino_portenta_h7_stm32h747xx_m7.dts

@@ -9,6 +9,7 @@
 #include <st/h7/stm32h747xihx-pinctrl.dtsi>
 #include "arduino_portenta_h7-common.dtsi"
 #include <../boards/common/usb/cdc_acm_serial.dtsi>
+#include <dt-bindings/charger/pf1550.h>


Suggested change

#include <dt-bindings/charger/pf1550.h>

#include <zephyr/dt-bindings/charger/pf1550.h>

gmarull · 2025-02-12T08:28:39Z

dts/bindings/charger/nxp,pf1550-charger.yaml

+  #  The following macros are available:
+  #  - LED_ON_IN_CHARGING_FLASH_IN_FAULT
+  #  - LED_FLASH_IN_CHARGING_ON_IN_FAULT
+  #  - LED_MANUAL_OFF
+  pf1550,led-behaviour:


should likely be a string property, see e.g. how nPM1300 LED behavior is specified. You can later use DT_ENUM_IDX helpers to get a number.

rriveramcrus

Just needs the compliance fix to get my +1 again :)

rriveramcrus · 2025-02-12T17:22:09Z

dts/bindings/charger/nxp,pf1550-charger.yaml

+    - on_in_charging_flash_in_fault
+    - flash_in_charging_on_in_fault
+    - manual_off


These need to be indented another level to satisfy the compliance checks and have a valid dt-binding.

~~Additionally these strings should be in quotes as is done for the pf1550,thermistor-monitoring-mode string enum.~~

Thanks for the heads-up, updated (with quotes for good measure 🙂)

rriveramcrus

Thank you for your contribution!

pillo79 · 2025-02-13T16:44:07Z

@gmarull please revisit your -1, thanks!
(only pushing to get this in 4.1)

gmarull · 2025-02-13T16:56:23Z

dts/bindings/charger/nxp,pf1550-charger.yaml

+      - "on_in_charging_flash_in_fault"
+      - "flash_in_charging_on_in_fault"
+      - "manual_off"


Suggested change

- "on_in_charging_flash_in_fault"

- "flash_in_charging_on_in_fault"

- "manual_off"

- "on-in-charging-flash-in-fault"

- "flash-in-charging-on-in-fault"

- "manual-off"

Add driver for nxp pf1550 PMIC Signed-off-by: Martino Facchin <[email protected]>

Enable PF1550 PMIC for Arduino Portenta H7 Signed-off-by: Martino Facchin <[email protected]>

On older Portenta H7, 1V2 power rail must be enabled to get a functional ethernet phy. Signed-off-by: Martino Facchin <[email protected]>

Selecting the wrong power scheme could potentially destroy the board. Luckily, the bit can only be set once and the default build still uses the Arduino bootloader (which has the correct setting). Signed-off-by: Martino Facchin <[email protected]>

facchinm requested a review from fabiobaltieri July 31, 2024 10:43

facchinm requested review from mmahadevan108, dleach02 and gmarull as code owners July 31, 2024 10:43

facchinm requested a review from erwango July 31, 2024 10:43

zephyrbot added area: Regulators area: Charger area: MFD platform: NXP Drivers NXP Semiconductors, drivers labels Jul 31, 2024

zephyrbot requested review from aasinclair, danieldegrasse, dbaluta, decsny, manuargue and rriveramcrus July 31, 2024 10:44

zephyrbot assigned rriveramcrus Jul 31, 2024

facchinm changed the title ~~[Arduino Portenta H7] Support onboard ethernet + pf1550 driver~~ boards: stm: arduino_portenta_h7: support onboard ethernet + pf1550 driver Jul 31, 2024

fabiobaltieri reviewed Jul 31, 2024

View reviewed changes

rriveramcrus reviewed Jul 31, 2024

View reviewed changes

dts/bindings/charger/nxp,pf1550-charger.yaml Outdated Show resolved Hide resolved

rriveramcrus reviewed Jul 31, 2024

View reviewed changes

drivers/charger/charger_pf1550.c Outdated Show resolved Hide resolved

facchinm force-pushed the portenta_h7_eth_f branch from 63caebe to 35f5779 Compare August 1, 2024 12:03

decsny removed their request for review August 1, 2024 13:55

facchinm force-pushed the portenta_h7_eth_f branch from 35f5779 to af73bba Compare August 1, 2024 14:29

rriveramcrus reviewed Aug 1, 2024

View reviewed changes

drivers/charger/charger_pf1550.c Show resolved Hide resolved

gmarull requested changes Aug 5, 2024

View reviewed changes

github-actions bot added the Stale label Oct 5, 2024

facchinm force-pushed the portenta_h7_eth_f branch from af73bba to ec86154 Compare October 18, 2024 13:03

gmarull reviewed Feb 12, 2025

View reviewed changes

pillo79 dismissed stale reviews from rriveramcrus and fabiobaltieri via e3b0d2e February 12, 2025 09:46

pillo79 force-pushed the portenta_h7_eth_f branch from 4e4ebc0 to e3b0d2e Compare February 12, 2025 09:46

fabiobaltieri previously approved these changes Feb 12, 2025

View reviewed changes

rriveramcrus requested changes Feb 12, 2025

View reviewed changes

pillo79 dismissed fabiobaltieri’s stale review via f7e2227 February 12, 2025 21:02

pillo79 force-pushed the portenta_h7_eth_f branch from e3b0d2e to f7e2227 Compare February 12, 2025 21:02

pillo79 requested review from gmarull, fabiobaltieri and rriveramcrus February 12, 2025 21:04

fabiobaltieri previously approved these changes Feb 13, 2025

View reviewed changes

rriveramcrus previously approved these changes Feb 13, 2025

View reviewed changes

gmarull reviewed Feb 13, 2025

View reviewed changes

facchinm added 4 commits February 13, 2025 17:59

drivers: add nxp pf1550 mfd (charger+regulator)

79421b4

Add driver for nxp pf1550 PMIC Signed-off-by: Martino Facchin <[email protected]>

portenta_h7: enable onboard regulator

68c0b7d

Enable PF1550 PMIC for Arduino Portenta H7 Signed-off-by: Martino Facchin <[email protected]>

portenta_h7: enable ethernet driver

d44d2e6

On older Portenta H7, 1V2 power rail must be enabled to get a functional ethernet phy. Signed-off-by: Martino Facchin <[email protected]>

pillo79 dismissed stale reviews from rriveramcrus and fabiobaltieri via b281743 February 13, 2025 17:00

pillo79 force-pushed the portenta_h7_eth_f branch from f7e2227 to b281743 Compare February 13, 2025 17:00

pillo79 requested review from gmarull, rriveramcrus and fabiobaltieri February 13, 2025 17:00

rriveramcrus approved these changes Feb 13, 2025

View reviewed changes

fabiobaltieri approved these changes Feb 13, 2025

View reviewed changes

gmarull approved these changes Feb 13, 2025

View reviewed changes

kartben merged commit d720fd7 into zephyrproject-rtos:main Feb 14, 2025
25 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

boards: stm: arduino_portenta_h7: support onboard ethernet + pf1550 driver #76542

boards: stm: arduino_portenta_h7: support onboard ethernet + pf1550 driver #76542

facchinm commented Jul 31, 2024 •

edited

Loading

github-actions bot commented Jul 31, 2024

fabiobaltieri left a comment

fabiobaltieri Jul 31, 2024

fabiobaltieri Jul 31, 2024

rriveramcrus commented Jul 31, 2024

gmarull Aug 5, 2024

gmarull Aug 5, 2024

facchinm Oct 18, 2024

gmarull Oct 21, 2024

github-actions bot commented Oct 5, 2024

pillo79 commented Feb 12, 2025

gmarull left a comment

gmarull Feb 12, 2025

gmarull Feb 12, 2025

gmarull Feb 12, 2025

rriveramcrus left a comment

rriveramcrus Feb 12, 2025 •

edited

Loading

pillo79 Feb 12, 2025

rriveramcrus left a comment

pillo79 commented Feb 13, 2025 •

edited

Loading

gmarull Feb 13, 2025

	#include <dt-bindings/charger/pf1550.h>
	#include <zephyr/dt-bindings/charger/pf1550.h>

boards: stm: arduino_portenta_h7: support onboard ethernet + pf1550 driver #76542

boards: stm: arduino_portenta_h7: support onboard ethernet + pf1550 driver #76542

Conversation

facchinm commented Jul 31, 2024 • edited Loading

github-actions bot commented Jul 31, 2024

fabiobaltieri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rriveramcrus commented Jul 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 5, 2024

pillo79 commented Feb 12, 2025

gmarull left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rriveramcrus left a comment

Choose a reason for hiding this comment

rriveramcrus Feb 12, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rriveramcrus left a comment

Choose a reason for hiding this comment

pillo79 commented Feb 13, 2025 • edited Loading

Choose a reason for hiding this comment

facchinm commented Jul 31, 2024 •

edited

Loading

rriveramcrus Feb 12, 2025 •

edited

Loading

pillo79 commented Feb 13, 2025 •

edited

Loading