Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for SEQURE S99, and new Rev. of S60P #1920

Open
wants to merge 43 commits into
base: dev
Choose a base branch
from

Conversation

jonasius
Copy link

@jonasius jonasius commented May 26, 2024

  • Please check if the PR fulfills these requirements
  • The changes have been tested locally
  • There are no breaking changes
  • What kind of change does this PR introduce?
  • Add support for Sequre S99
  • What is the new behavior (if this is a feature change)?
    IronOS booting on Sequre S99

  • Other information:
    Currently work in progress.

Problems / Todo

  • Startup is slow, black screen for a few seconds, maybe due to PD negotiation issues?
  • Negotiation of PD seems to have a problem. Tested with multiple Apple USB-C Chargers.Always uses 5V QC. PD Debug at startup shows State 0 0 21 multiple times, with blinking/resetting screen, after a while showing State 4 1 21.
  • OP_AMP_GAIN_STAGE 536 needs to be adjusted according to the 22k resistor instead of 51k
  • Documentation needs to be updated
  • Maybe source/Code/BSP/Sequre_S60 folder should be renamed to Sequre?
  • Better thermo model for tip temperature measurement?
  • Tip selection option to allow usage of 5.5 Ohm tips?

source/Core/BSP/Sequre_S60/configuration.h Outdated Show resolved Hide resolved
source/Core/Drivers/Font.h Outdated Show resolved Hide resolved
jonasius and others added 2 commits May 26, 2024 23:09
Co-authored-by: discip <53649486+discip@users.noreply.github.com>
Co-authored-by: discip <53649486+discip@users.noreply.github.com>
@Ralim
Copy link
Owner

Ralim commented Jun 1, 2024

Hia,

This is great progress.

The black screen at startup is odd. PD negotiation shouldn't block boot. Is it possible its being restarted rapidly with flaky USB-PD?

(Do you have any tools for debugging USB-PD)?

I haven't really worked on the FS2711 driver much so not sure how best to debug it other than trying to parse the code to match up to what you see.

@jonasius
Copy link
Author

jonasius commented Jun 1, 2024

Hey, yeah. It seems to restart rapidly. It seems to be especially with the Apple Chargers. Another charger I tried let the iron boot straight up.
I hope I can investigate further this weekend.

Sadly I don't have special USB-PD tools. Maybe I need to get something or build something to hook a logic analyzer on?

As I could see with the debug menu, when it comes up it gets all available PD modes, but stays in 5V mode. I think I need to take a look at the measurement of the tip for the power calculation? Maybe you can give me a hint on that?

@jonasius
Copy link
Author

jonasius commented Jun 1, 2024

@Ralim I'm currently digging a bit through the code and debug outputs of the iron. I wondered why it reports PWR QC instead of PWR PD. I think at least I need to extend the getPowerSourceNumber funciton with a ifdef for POW_PD_EXT.

Regarding the PD negotation problem with my Apple 60W USB-C PSUs I tried to increase the PROTOCOL_TIMEOUT to 200 ms, which helped. The Iron boots instantly. PD Debug First Shows State 0 0 21 but quickly changes to State 4 1 21, without a reset/screen flicker. What I noticed when I switched through the PD debug menu and viewed the capabilities was that before increasing the timeout, the PD debug menu showed an entry with no voltage and only amps specified. That entry isn't showing up with the increased timeout.

I hope I can make some progress tomorrow.

@Ralim
Copy link
Owner

Ralim commented Jun 2, 2024

I'm currently digging a bit through the code and debug outputs of the iron. I wondered why it reports PWR QC instead of PWR PD. I think at least I need to extend the getPowerSourceNumber funciton with a ifdef for POW_PD_EXT.

Ah heck, yes that will fix the issue

PROTOCOL_TIMEOUT should have been wired up to the PD timeout we already have thats user adjustable.
If you want I can wire that up (or you can of course). I default to 500ms for normal pd.

@jonasius
Copy link
Author

jonasius commented Jun 2, 2024

PROTOCOL_TIMEOUT should have been wired up to the PD timeout we already have thats user adjustable. If you want I can wire that up (or you can of course). I default to 500ms for normal pd.

@Ralim It's a bit unclear to me. In the Settings.cpp PDNegTimeout default value is 20, max 50. If I got the description right it should be in 100ms steps, so default is 2000ms? :-o

OP_AMP_GAIN_STAGE 536 needs to be adjusted according to the 22k resistor instead of 51k

How did you determined the value for the OP_AMP_GAIN_STAGE? I measured/calculated it roughly to 226 for the different feedback resistor. I don't get exactly how the two OP-Amps are connected.

@Ralim
Copy link
Owner

Ralim commented Jun 4, 2024

so default is 2000ms? :-o

Ah sorry, I think we raised the default later on.

How did you determined the value for the OP_AMP_GAIN_STAGE
This number is calculated as being the "gain" of the op-amp.

In the S60, its two sequential op-amps chained.
Both are configured as non-inverting amplifiers.

Input op-amp has feedback resistors of 9.31K and 1K. (therefore gain is 10.31)
Second op-amp has feedback resistors of 51K and 1K. (therefore gain is 52).

So 10.31 * 50 = 536.12; so the define is set to 536

jonasius added 5 commits June 6, 2024 20:27
* Enable PD Options
* Make PDNegTimeout configureable
* Add default value for PDNegTimeout, also for S60 and S60P
* Add basic DC detection / correct debug readings while powered via DC
* Add basic ThermoModel for C245 Tips
* Modify op-amp gain
bool getIsPoweredByDCIN() { return false; }
bool getIsPoweredByDCIN() {
#if POW_PD_EXT == 2 && defined(POW_DC)
if (!FS2711::has_run_selection()) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼

Copy link
Owner

@Ralim Ralim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems sane to me at a read.

@Ralim
Copy link
Owner

Ralim commented Jun 8, 2024

Let me know when you are happy with this by marking it ready for review 🙇🏼

@jonasius
Copy link
Author

Let me know when you are happy with this by marking it ready for review 🙇🏼

Nice!
I'll try to fine tune the temperature coefficient beforehand. Add documentation / the S90 to the supported irons table and fix the remaining clang style mismatches.

@jonasius
Copy link
Author

jonasius commented Jun 19, 2024

I compared the temperature at a setpoint of 320°C to a original JBC station. In my eyes the coefficient is okay for a first shot.

Then I observed that the name and description for PDNegTimeout and USBPDMode are not shown in the menu. I quite don't get it. The menu Item is here

I got a second S99 which is V1.5 instead of V1.4, it seems that they changed something. Although a FS2711 is used, it does not negotiate PD. I quickly took two picutes:
IMG_4353
IMG_4354

@jonasius
Copy link
Author

@Ralim, I tried your increased pwm speed. Works great on the S99! No more restarts, even with power limit set to 60W on a 60W USB-C charger.

What I got so far regarding v1.5

  • There is a detection of HW version in the original FW
  • I2C to the FS2711Q connection seems to use the same pins
  • Just to make sure I checked the bootloader, it is also the same on the v1.5 iron
  • PD-Debug shows only zeros

@kabel42
Copy link

kabel42 commented Nov 8, 2024

Works on my (v1.1 PCB) S60P with a steamdeck charger and a lenovo 65W charger

@Ralim
Copy link
Owner

Ralim commented Nov 14, 2024

@discip What charger are you using? Ill try this locally

@discip
Copy link
Collaborator

discip commented Nov 14, 2024

@Ralim
This one:
Anker Nano II 65W. 😊

@g5pw
Copy link

g5pw commented Nov 15, 2024

https://github.com/jonasius/IronOS/actions/runs/10709046785/job/29692775793 Works on my new S99 (PCB v1.5) with a 65W charger

Copy link
Collaborator

@discip discip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As already mentioned, in the current state this unfortunately does not work for the S60P.

@jonasius
Copy link
Author

As already mentioned, in the current state this unfortunately does not work for the S60P.

To move on I'm planning to create a few versions for you to test. Maybe the initialization of i2c1 or the probing isn't working properly on your hw rev.

@jonasius
Copy link
Author

@discip
Hexfile_noI2C1_noprobing.zip: PD is working only on my old hw rev S99. This should work on your rev. 1.0 S60P
Hexfile_noI2C1.zip: PD is also only working on my old hw rev S99. This should also work on your S60P. My newer S99 is missing the init on I2C1, the display is off.

If the fw in Hexfile_noI2C1.zip isn't working on your S60P, that would indicate a problem with the I2C-probing on I2C2.

@discip
Copy link
Collaborator

discip commented Nov 21, 2024

@jonasius

If the fw in Hexfile_noI2C1.zip isn't working on your S60P, that would indicate a problem with the I2C-probing on I2C2.

After flashing Hexfile_noI2C1.zip the OLED stayed off.

After flashing Hexfile_noI2C1_noprobing.zip PD Debug reads State 1 allowing only 5.2 V.

@Ralim
Maybe it has something to do with PPS.

@jonasius
Copy link
Author

After flashing Hexfile_noI2C1.zip the OLED stayed off.

After flashing Hexfile_noI2C1_noprobing.zip PD Debug reads State 1 allowing only 5.2 V.

PD Debug State 1 means that the I2C communication with the PD-IC is working. That should be the same behavior as with the current dev branch.

The OLED off with the version with probing means that the probing on I2C2 fails. I'll think about how we could further investigate on that.

@Ralim
Copy link
Owner

Ralim commented Nov 21, 2024

Hmm; PPS evaluation could be involved, it would be my rough guess as its probably the pain part of the protocol. Is it possible to disable PPS for testing?

@discip
Copy link
Collaborator

discip commented Nov 21, 2024

@Ralim
With the mentioned charger (Anker Nano II 65W) I need PPS to use the S60P to its full potential, since the fixed 12V option is missing. 😅

But for testing purposes this could be disabled.

@jonasius
Copy link
Author

@discip I've quickly put together a fw which shows the i2c devices it can find on startup instead of the usb-pd menu. Maybe you could flash it and enter the usb-pd debug menu. It should show numbers on the left and right side of the screen.
s60p_probing_test.zip

@discip
Copy link
Collaborator

discip commented Nov 29, 2024

@jonasius
Thank you.

Here are the numbers:

090 090
091 091
120 120
121 121

@jonasius
Copy link
Author

@discip okay, thats good, 90 is there as expected.
Could you try the following:
s60p_probing_startwith90.zip
and maybe that one:
s60p_probing_delay_startwith90.zip

@discip
Copy link
Collaborator

discip commented Nov 29, 2024

@jonasius
s60p_probing_startwith90.zip

091 090
120 091
121 120
122 121

s60p_probing_delay_startwith90.zip

091 090
120 091
121 120
122 121

@jonasius
Copy link
Author

@discip
Ah, nice. We are getting closer :) So we may need more delay.
Please try this... s60p_probing_startwith89.zip

@discip
Copy link
Collaborator

discip commented Nov 29, 2024

@jonasius
Same as here:

Here are the numbers:

090 090
091 091
120 120
121 121

@jonasius
Copy link
Author

jonasius commented Dec 6, 2024

@discip that version should be able to communicate with the pd-ic. I'm not happy with the solution. Maybe one probe before the real probing should be enough.

@discip
Copy link
Collaborator

discip commented Dec 7, 2024

@jonasius
Thank you for not giving up! 😃👍

Unfortunately now is says:

PD Debug
State 1

allowing only 5.2 V

@jonasius
Copy link
Author

jonasius commented Dec 8, 2024

@discip just for clarification. The latest build of dev branch negotiates with 12V?!

@discip
Copy link
Collaborator

discip commented Dec 9, 2024

@discip just for clarification. The latest build of dev branch negotiates with 12V?!

Sorry, I'm not able to follow here. What exactly is this supposed to tell me? 😊

@jonasius
Copy link
Author

jonasius commented Dec 9, 2024

@discip just for clarification. The latest build of dev branch negotiates with 12V?!

Sorry, I'm not able to follow here. What exactly is this supposed to tell me? 😊

Sorry.
What's the PD-Debug output with the latest build of the dev branch? For example this one:
https://github.com/Ralim/IronOS/actions/runs/12233892729/artifacts/2293020834

@discip
Copy link
Collaborator

discip commented Dec 11, 2024

@jonasius
With the provided build the screen reads:

PD Debug
State 6

Nothing else (There is no option I can go through like e.g. with the Pinecil).

And it negotiates for 8.3 V.

@RodoMa92
Copy link

RodoMa92 commented Dec 22, 2024

Just tested on my new S99 after my Pinecil V1 USB PD decided to die on me, seems to work decently. Haven't tested a lot, though. Thanks a lot for your work! I'll see if I can RE the schematics of it, to be able to properly fix it in case.


#ifdef I2C_SOFT_BUS_1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why insert a check here? The names of the entities are different anyway.
(SOFT_SCL1_HIGH and SOFT_SCL2_HIGH respectively)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.