Portenta H7 PWM: Not sure of the Zephyr capabilities/rules

[KurtE]

Short version:
a) Can you for each of the timers defines multiple pins that one might use, which may have different settings.
b) If not, Are there Portenta Preferred ones? Like on GIGA something like pins D2-12...

Longer version: On MBED, I believe when you your code calls analogWrite(...) (assuming not DAC pin), it will go through MBED to the table in
https://github.com/arduino/mbed-os/blob/extrapatches-6.17.0/targets/TARGET_STM/TARGET_STM32H7/TARGET_STM32H747xI/TARGET_PORTENTA_H7/PeripheralPins.c#L154-L262
This table (actually whole file) is identical to the GIGA variant.

And it finds the port/pin the the table and setups the corresponding timer/channel.

Now suppose there are valid pins on the Portenta that can all map down to the same timer/channel. Can the device tree handle it.
For example:

TIM3_CH1    PA_6
TIM3_CH1    PB_4
TIM3_CH1    PC_6

And that is not including maybe some that might be TIM3_CH1N. Can we define all of these and hope the user only uses one at a time?

If not is there anything in current documentation which suggests which one should be defined? Guessing maybe in this case PC_6?

[KurtE]

@pillo79 @facchinm,

Quick update:
@mjs513 and I have been playing some with trying to get PWM working:

He started off with a subset of possible PWM pins:

&timers1 {
	status = "okay";
	st,prescaler = <0>;

	pwm1: pwm {
		status = "okay";
		pinctrl-0 = <&tim1_ch2_pj11 &tim1_ch1_pa8>;
		pinctrl-names = "default";
	};
};

&timers3 {
	status = "okay";
	st,prescaler = <100>;

	pwm3: pwm {
		status = "okay";
		pinctrl-0 = <&tim3_ch1_pc6 &tim3_ch2_pc7>;
		pinctrl-names = "default";
	};
};

&timers8 {
	status = "okay";
	st,prescaler = <100>;

	pwm8: pwm {
		status = "okay";
		pinctrl-0 = <&tim8_ch2_pj10 &tim8_ch2n_pj7 &tim8_ch3n_ph15 &tim8_bkin2_pa8>;
		pinctrl-names = "default";
	};
};

&timers12 {
	status = "okay";
	st,prescaler = <100>;

	pwm12: pwm {
		status = "okay";
		pinctrl-0 = <&tim12_ch1_ph6>;
		pinctrl-names = "default";
	};
};

&pwm1 {
	/* Use the pwmclock node to start the clock generation */
	pwmclock: pwmclock {
		status = "okay";
		compatible = "pwm-clock";
		clock-frequency = <0>;
		#clock-cells = <1>;
		pwms = <&pwm1 2 PWM_HZ(12000000) PWM_POLARITY_NORMAL>;
	};
};
...
		pwm-pin-gpios =	<&gpioa 8 0>,
                    <&gpioc 6 0>,
                    <&gpioc 7 0>,
                    //<&gpiog 7 0>,
                    <&gpioj 11 0>,
                    //<&gpiok 1 0>,
                    <&gpioh 15 0>,
                    <&gpioj 7 0>,
                    <&gpioj 10 0>,
                    <&gpioh 6 0>;
...
		pwms = <&pwm1 2 PWM_HZ(12000000) PWM_POLARITY_NORMAL>,
			   <&pwm3 1 PWM_HZ(500) PWM_POLARITY_NORMAL>,
			   <&pwm3 2 PWM_HZ(500) PWM_POLARITY_NORMAL>,
         		<&pwm1 1 PWM_HZ(5000) PWM_POLARITY_NORMAL>,
         		<&pwm8 3 PWM_HZ(500) PWM_POLARITY_NORMAL>,
			   <&pwm8 2 PWM_HZ(500) PWM_POLARITY_NORMAL>,
			   <&pwm8 1 PWM_HZ(500) PWM_POLARITY_NORMAL>,
			   <&pwm12 1 PWM_HZ(500) PWM_POLARITY_NORMAL>;
	};

And turn on PWM: CONFIG_PWM=y

First issue we ran into is the same we did with LEDS. The normal pin numbers like D0-xx are again repeated in the second part
of the table, where all possible pins are enumerated again. The issue was the
current code that maps the PWM pin numbers to the PWM index, ends up summing the index in the GPIO table of all of the
matches, so for example pin 2: will not be found:

 const pin_size_t arduino_pwm_pins[] =
   { DT_FOREACH_PROP_ELEM(DT_PATH(zephyr_user), pwm_pin_gpios, PWM_PINS) };
 
 size_t pwm_pin_index(pin_size_t pinNumber) {
 
   for(size_t i=0; i<ARRAY_SIZE(arduino_pwm_pins); i++) {
     if (arduino_pwm_pins[i] == pinNumber) {
       return i;
     }
   }
   return (size_t)-1;
 }

I have a version, where instead I keep the PWM table in the same format as the GPIO pin table,
And then simply compare port and pin values. Have not added checking the attributes, but might be necessary?
Like maybe for complementary pins?

 static const struct gpio_dt_spec arduino_pwm_pins[] = {DT_FOREACH_PROP_ELEM_SEP(
   DT_PATH(zephyr_user), pwm_pin_gpios, GPIO_DT_SPEC_GET_BY_IDX, (, ))};
 
 size_t pwm_pin_index(pin_size_t pinNumber) {
   //printk("pwm_pin_index: %u: ", pinNumber);
   if (pinNumber >= ARRAY_SIZE(arduino_pins)) {
     return (size_t)-1;
   }
   printk("(%p %u):", arduino_pins[pinNumber].port, arduino_pins[pinNumber].pin);
   for(size_t i=0; i<ARRAY_SIZE(arduino_pwm_pins); i++) {
     //printk(" [%p,%u]", arduino_pwm_pins[i].port, arduino_pwm_pins[i].pin);
     if ((arduino_pwm_pins[i].port == arduino_pins[pinNumber].port) && (arduino_pwm_pins[i].pin == arduino_pins[pinNumber].pin)) {
       //printk("\n");
       return i;
     }
   }
       //printk("\n");
   return (size_t)-1;
 }

So it is now matching the pins numbers, like: analogWrite(2, 64); likewise for pin4 ...

But still the writes were not showing up on the IO pin.

So then wondered if it needed the same as the camera clock code in fixups.c and that appears to help.

    Serial.println("*** Getting PWM clock ***");
    const struct device* cam_ext_clk_dev = DEVICE_DT_GET(DT_NODELABEL(pwmclock));

    if (!device_is_ready(cam_ext_clk_dev)) {
      Serial.println("Clock not ready");
    }
    int ret = clock_control_on(cam_ext_clk_dev, (clock_control_subsys_t)0);
    if (ret < 0) {
      Serial.print("clock on: ");
      Serial.println(ret);
    }
    uint32_t rate;
    ret = clock_control_get_rate(cam_ext_clk_dev, (clock_control_subsys_t)0, &rate);
    Serial.print("Rate: ");
    Serial.println(rate);
  }

And with this code being called, it appears like some of the analogWrite code started working
However my calls to analogWrite(2, ...) did not appear to be changing the pulse widths, except maybe the first call:

but then If I switch to pin 4 it does appear to update, where I change

void loop() {
  // put your main code here, to run repeatedly:
  digitalWrite(toggle_pin, !digitalRead(toggle_pin));
  digitalWrite(toggle_pin, !digitalRead(toggle_pin));
  for (int i = 0; i < 256; i += 32) {
    analogWrite(analog_pin, i);
    digitalWrite(toggle_pin, !digitalRead(toggle_pin));
    maybe_pause();
    delay(100);
  }

  digitalWrite(toggle_pin, !digitalRead(toggle_pin));
  digitalWrite(toggle_pin, !digitalRead(toggle_pin));
  for (int i = 255; i >= 0; i -= 32) {
    analogWrite(analog_pin, i);
    digitalWrite(toggle_pin, !digitalRead(toggle_pin));
    maybe_pause();
    delay(100);
  }
  delay(50);
}

Not sure what the reason why starting the camera clock enables the others. ???

[mjs513]

made a few changes this morning to the overlay:

&timers1 {
	status = "okay";
	st,prescaler = <0>;

	pwm1: pwm {
		status = "okay";
		pinctrl-0 = <&tim1_ch1_pk1 &tim1_ch2_pj11>;
		pinctrl-names = "default";
	};
};

&timers3 {
	status = "okay";
	st,prescaler = <100>;

	pwm3: pwm {
		status = "okay";
		pinctrl-0 = <&tim3_ch1_pc6 &tim3_ch2_pc7>;
		pinctrl-names = "default";
	};
};

&timers8 {
	status = "okay";
	st,prescaler = <100>;

	pwm8: pwm {
		status = "okay";
		pinctrl-0 = <&tim8_bkin2_pa8 &tim8_ch3n_ph15 &tim8_ch2_pj10 &tim8_ch2n_pj7>;
		pinctrl-names = "default";
	};
};

&timers12 {
	status = "okay";
	st,prescaler = <100>;

	pwm12: pwm {
		status = "okay";
		pinctrl-0 = <&tim12_ch1_ph6>;
		pinctrl-names = "default";
	};
};

&pwm1 {
	/* Use the pwmclock node to start the clock generation */
	pwmclock: pwmclock {
		status = "okay";
		compatible = "pwm-clock";
		clock-frequency = <0>;
		#clock-cells = <1>;
		pwms = <&pwm1 2 PWM_HZ(12000000) PWM_POLARITY_NORMAL>;
	};
};

and

		pwms = <&pwm8 1 PWM_HZ(500) PWM_POLARITY_NORMAL>,
          <&pwm3 1 PWM_HZ(500) PWM_POLARITY_NORMAL>,
          <&pwm3 2 PWM_HZ(500) PWM_POLARITY_NORMAL>,
          <&pwm1 2 PWM_HZ(12000000) PWM_POLARITY_NORMAL>, 
          <&pwm1 1 PWM_HZ(5000) PWM_POLARITY_NORMAL>,
          <&pwm8 2 PWM_HZ(500) PWM_POLARITY_NORMAL>,
          <&pwm8 4 PWM_HZ(500) PWM_POLARITY_NORMAL>,
          <&pwm8 3 PWM_HZ(500) PWM_POLARITY_NORMAL>,
          <&pwm12 1 PWM_HZ(500) PWM_POLARITY_NORMAL>;

the one thing I noticed is that some of the pwm pins show that HRTIMERS are available but I am not seeing them supported in Zephyr or am I reading something wrong.

[KurtE]

the one thing I noticed is that some of the pwm pins show that HRTIMERS are available but I am not seeing them supported in Zephyr or am I reading something wrong.

I believe that on MBED, that the Arduino pins 3-6 (PG_7, PC_7, PC_6, PA_8), assuming it was implemented, would be handled
by the HRTIM object (Chapter 39 of the RM).

As you mentioned, I don't believe it is implemented yet. I believe that there is a related issue up there:
catie-aq/zephyr_zest-core-stm32g474ve#20
And in the main github:
zephyrproject-rtos/zephyr#38629

Although at least some of this may be there:
zephyrproject-rtos/hal_stm32#87

[mjs513]

As you mentioned, I don't believe it is implemented yet. I believe that there is a related issue up there:
catie-aq/zephyr_zest-core-stm32g474ve#20
And in the main github:
zephyrproject-rtos/zephyr#38629
"
Although at least some of this may be there:
zephyrproject-rtos/hal_stm32#87

looks like some of it added for the f3 and g4 versions but didnt see anything for the H747
https://github.com/martinjaeger/hal_stm32/blob/ef466ff2b83474739a0c74432ce51476469480c7/dts/st/h7/stm32h747xihx-pinctrl.dtsi

[mjs513]

@KurtE - @facchinm - @pillo79

Ok looks like the best that can be done at this point with the following constraints on the board:

1. No HR Timer implemented
2. Timer channel xN not working using either PWM_POLARITY_NORMAL or PWM_POLARITY_INVERTED

this is the overlay I am currently using:

arduino_portenta_h7_m7.zip

In addition for testing I am using @KurtE's pwm test sketch for consistency:

uint8_t analog_pin = 0;

void print_gpio_regs(const char* name, GPIO_TypeDef* port) {
  printk("GPIO%s(%p) %08X %08X %08x\n", name, port, port->MODER, port->AFR[0], port->AFR[1]);
  Serial.print("GPIO");
  Serial.print(name);
  Serial.print(" ");
  Serial.print(port->MODER, HEX);
  Serial.print(" ");
  Serial.print(port->AFR[0], HEX);
  Serial.print(" ");
  Serial.println(port->AFR[1], HEX);
}

#include <cmsis_core.h>
#include <zephyr/init.h>
#include <zephyr/kernel.h>
#include <zephyr/device.h>
#include <zephyr/drivers/clock_control.h>
#include <zephyr/logging/log.h>



#define toggle_pin 12  //31
void setup() {
  // put your setup code here, to run once:
  Serial.begin(115200);
  //while (!Serial) {}
  delay(500);

  Serial.println("Start of PWM test");
  pinMode(toggle_pin, OUTPUT);
  delay(1000);

  Serial.print("Enter something if you want to start cam colock");
  while (Serial.read() != -1) {}
  int ich;
  while ((ich = Serial.read()) == -1) {};
  while (Serial.read() != -1) {}

  if (ich >= ' ') {
    Serial.println("*** Getting PWM clock ***");
    const struct device* cam_ext_clk_dev = DEVICE_DT_GET(DT_NODELABEL(pwmclock));

    if (!device_is_ready(cam_ext_clk_dev)) {
      Serial.println("Clock not ready");
    }
    int ret = clock_control_on(cam_ext_clk_dev, (clock_control_subsys_t)0);
    if (ret < 0) {
      Serial.print("clock on: ");
      Serial.println(ret);
    }
    uint32_t rate;
    ret = clock_control_get_rate(cam_ext_clk_dev, (clock_control_subsys_t)0, &rate);
    Serial.print("Rate: ");
    Serial.println(rate);
  }
}

void maybe_pause() {
  if (Serial.available()) {
    int ch;
    uint8_t new_awPin = 0;
    while (Serial.read() != -1) {}
    Serial.println("Paused");
    while ((ch = Serial.read()) == -1) {}
    while ((ch >= '0') && (ch <= '9')) {
      new_awPin = new_awPin * 10 + ch - '0';
      ch = Serial.read();
    }

    while (Serial.read() != -1) {}

    if (new_awPin != 0) {
      analogWrite(analog_pin, 0);
      analog_pin = new_awPin;
    }
    Serial.print("New Pin: ");Serial.println(analog_pin);
/*    print_gpio_regs("A", GPIOA);
    print_gpio_regs("B", GPIOB);
    print_gpio_regs("C", GPIOC);
    print_gpio_regs("D", GPIOD);
    print_gpio_regs("E", GPIOE);
    print_gpio_regs("F", GPIOF);
    print_gpio_regs("G", GPIOG);
    print_gpio_regs("H", GPIOH);
    print_gpio_regs("I", GPIOI);
    print_gpio_regs("J", GPIOJ);
    print_gpio_regs("K", GPIOK);



    Serial.print("CR1: 0x");
    Serial.println(TIM1->CR1, HEX);
    Serial.print("CR2: 0x");
    Serial.println(TIM1->CR2, HEX);
    Serial.print("SMCR: 0x");
    Serial.println(TIM1->SMCR, HEX);
    Serial.print("DIER: 0x");
    Serial.println(TIM1->DIER, HEX);
    Serial.print("SR: 0x");
    Serial.println(TIM1->SR, HEX);
    Serial.print("EGR: 0x");
    Serial.println(TIM1->EGR, HEX);
    Serial.print("CCMR1: 0x");
    Serial.println(TIM1->CCMR1, HEX);
    Serial.print("CCMR2: 0x");
    Serial.println(TIM1->CCMR2, HEX);
    Serial.print("CCER: 0x");
    Serial.println(TIM1->CCER, HEX);
    Serial.print("CNT: 0x");
    Serial.println(TIM1->CNT, HEX);
    Serial.print("PSC: 0x");
    Serial.println(TIM1->PSC, HEX);
    Serial.print("ARR: 0x");
    Serial.println(TIM1->ARR, HEX);
    Serial.print("RCR: 0x");
    Serial.println(TIM1->RCR, HEX);
    Serial.print("CCR1: 0x");
    Serial.println(TIM1->CCR1, HEX);
    Serial.print("CCR2: 0x");
    Serial.println(TIM1->CCR2, HEX);
    Serial.print("CCR3: 0x");
    Serial.println(TIM1->CCR3, HEX);
    Serial.print("CCR4: 0x");
    Serial.println(TIM1->CCR4, HEX);
    Serial.print("BDTR: 0x");
    Serial.println(TIM1->BDTR, HEX);
    Serial.print("DCR: 0x");
    Serial.println(TIM1->DCR, HEX);
    Serial.print("DMAR: 0x");
    Serial.println(TIM1->DMAR, HEX);
    Serial.print("CCMR3: 0x");
    Serial.println(TIM1->CCMR3, HEX);
    Serial.print("CCR5: 0x");
    Serial.println(TIM1->CCR5, HEX);
    Serial.print("CCR6: 0x");
    Serial.println(TIM1->CCR6, HEX);
    Serial.print("AF1: 0x");
    Serial.println(TIM1->AF1, HEX);
    Serial.print("AF2: 0x");
    Serial.println(TIM1->AF2, HEX);
    Serial.print("TISEL: 0x");
    Serial.println(TIM1->TISEL, HEX);*/
  }
}

void loop() {
  // put your main code here, to run repeatedly:
  digitalWrite(toggle_pin, !digitalRead(toggle_pin));
  digitalWrite(toggle_pin, !digitalRead(toggle_pin));
  for (int i = 0; i < 256; i += 32) {
    analogWrite(analog_pin, i);
    digitalWrite(toggle_pin, !digitalRead(toggle_pin));
    maybe_pause();
    delay(100);
  }

  digitalWrite(toggle_pin, !digitalRead(toggle_pin));
  digitalWrite(toggle_pin, !digitalRead(toggle_pin));
  for (int i = 255; i >= 0; i -= 32) {
    analogWrite(analog_pin, i);
    digitalWrite(toggle_pin, !digitalRead(toggle_pin));
    maybe_pause();
    delay(100);
  }
  delay(50);
}