Skip to content

drivers: pinctrl: new state based API proposal #37572

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Oct 25, 2021
Merged

drivers: pinctrl: new state based API proposal #37572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
84d040c
drivers: pinctrl: initial skeleton
gmarull Aug 10, 2021
ed1c7e5
drivers: pinctrl: allow to skip states
gmarull Sep 2, 2021
5517b4b
drivers: pinctrl: add support for dynamic pin control
gmarull Sep 13, 2021
85101f6
dts: bindings: pinctrl: add pincfg-node-group
gmarull Oct 6, 2021
4047889
tests: drivers: pinctrl: add tests for API
gmarull Sep 20, 2021
a479319
doc: doxygen: add Doxygen predefined macro
gmarull Sep 16, 2021
b6970e0
doc: reference: add pinctrl API
gmarull Sep 16, 2021
ff13f41
doc: enable figures enumeration
gmarull Sep 22, 2021
89ffb51
doc: guides: add pinctrl guide
gmarull Sep 22, 2021
bc30583
CODEOWNERS: add myself to pinctrl
gmarull Sep 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CODEOWNERS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@
/drivers/modem/Kconfig.hl7800 @LairdCP/zephyr
/drivers/pcie/ @dcpleung @nashif @jhedberg
/drivers/peci/ @albertofloyd @franciscomunoz @scottwcpg
/drivers/pinctrl/ @gmarull
/drivers/pinmux/*b91* @yurvyn
/drivers/pinmux/*hsdk* @iriszzw
/drivers/pinmux/*it8xxx2* @ite
Expand Down
2 changes: 2 additions & 0 deletions doc/conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@

todo_include_todos = False

numfig = True
Copy link
Contributor

@marc-hb marc-hb Feb 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This single numfig = True line made both the incremental build and the build from scratch MULTIPLE times slower. On the system I'm currently using it, commenting it out speeds things up like this:

  • building docs from scratch: 4.5 minutes down from 35 minutes (!)
  • incremental build with zero change: 15 seconds down from 75 seconds

This is with sphinx 5.3, similar speed-up reproduced with sphinx 5.0.2

I don't understand how no one noticed yet. When I make typos in a doc update I don't want to wait minutes and minutes to check whether I successfully fixed it...

I think there's been other serious regressions but this is by far the biggest one.

Copy link
Member Author

@gmarull gmarull Feb 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That's an interesting finding, I guess we could live without it (just not referencing to figures). Could you report a bug to the Sphinx project? It looks like such build time increase is not justified just because we enumerate figures...

Copy link
Contributor

@marc-hb marc-hb Feb 10, 2023

Choose a reason for hiding this comment

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

I already spent an inordinate amount of time bisecting this and I now have a fantastic, one-line workaround "permanently" committed in my workspace so I'm not going to report this, sorry :-( I bet the sphinx project could request a reproduction simpler and smaller than the whole zephyr project (I would!) with fewer dependencies and finding that could take a lot more time. It could be worse: maybe it does not reproduce outside Zephyr? I've also never reported any sphinx issue before.

What I'm more tempted to do now that my doc build times have become "reasonable" again is to try to bisect the other, less dramatic performance regressions of the incremental build. Afraid they could be just down to a bigger and bigger repo though...

Copy link
Member Author

@gmarull gmarull Feb 10, 2023

Choose a reason for hiding this comment

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

I already spent an inordinate amount of time bisecting this and I now have a fantastic, one-line workaround "permanently" committed in my workspace so I'm not going to report this, sorry :-( I bet the sphinx project could request a reproduction simpler and smaller than the whole zephyr project (I would!) with fewer dependencies and finding that could take a lot more time. It could be worse: maybe it does not reproduce outside Zephyr? I've also never reported any sphinx issue before.

This is sad, considering it is just about opening an issue here https://github.com/sphinx-doc/sphinx. Just by reporting you'll increase the chances of a Sphinx developer taking a look at it.

What I'm more tempted to do now that my doc build times have become "reasonable" again is to try to bisect the other, less dramatic performance regressions of the incremental build. Afraid they could be just down to a bigger and bigger repo though...

I think that our docs with current structure are always going to result in slow builds. Using breathe is a problem, or having tons of pages with redundant content (e.g. boards or many samples) doesn't help either. I suspect changing this will face opposition or too many questions, though.

Copy link
Contributor

@marc-hb marc-hb Feb 10, 2023

Choose a reason for hiding this comment

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

This is sad, considering it is just about opening an issue here https://github.com/sphinx-doc/sphinx.

Filing a good bug is usually work, sometimes a lot of work. Also filing is usually just the beginning of a conversation. The first answer is usually: "can you reproduce with the latest [sphinx] version?". Fair enough; we all ask the same.

Also: maybe it's not a bug? Maybe numfig is inherently slow. I found this in sphinx-doc/sphinx#5888 (comment):

... Sphinx assigns section and figure numbers on that action. Therefore, it takes long for large document enabled numbered toctree or numfig.

If you think/hope it's very quick and easy in this particular case to file then why not just do it? :-) It took me a long time to bisect but right now everyone has exactly as much information as I do.

Just by reporting you'll increase the chances of a Sphinx developer taking a look at it.

https://github.com/sphinx-doc/sphinx/issues has currently 1079 open issues so I wouldn't hold my breath. It's only free software.

A new sphinx bug would certainly be much better place to discuss numfig between just ourselves than this merged PR here but sorry again: I don't have (more) time to spend on discussing numfig.

Now I just did "something": I searched for existing numfig bugs and I found none about performance but I mentioned this performance issue in 2 existing bugs that already mentioned numfig cost "in passing". As opposed to filing a brand new bug "in the void", this could draw attention from some people already involved with numfig?

I think that our docs with current structure are always going to result in slow builds.

I understand the time to build from scratch is rarely ever going to get better and I think that's OK. However for "iterative" documentation writing the "zero-change", incremental build time should stay around < 5-6 seconds as it used to be not so long ago. Just like when building anything else.

Copy link
Contributor

@marc-hb marc-hb Feb 12, 2023

Choose a reason for hiding this comment

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

maybe it's not a bug? Maybe numfig is inherently slow.

While using top I also got a vague impression that fewer sphinx-build processes get started while using numfig. Maybe there's some serialization of some sort? Just a wild guess.

I guess we could live without it (just not referencing to figures)

Maybe it could be disabled in make html-fast and left in make html?

Copy link
Contributor

@marc-hb marc-hb Mar 11, 2023

Choose a reason for hiding this comment

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

However for "iterative" documentation writing the "zero-change", incremental build time should stay around < 5-6 seconds as it used to be not so long ago. Just like when building anything else.

#55708 adds an new SPHINXOPTS tag that turns off numfig and breathe and brings the incremental build time back down to asomewhat usable 9 seconds.


rst_epilog = """
.. include:: /substitutions.txt
"""
Expand Down
1 change: 1 addition & 0 deletions doc/guides/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ User and Developer Guides
debug_tools/index
device_mgmt/index
dts/index
pinctrl/index
emulator/index.rst

modules.rst
Expand Down
Binary file added doc/guides/pinctrl/images/hw-cent-control.odg
View file
Open in desktop
Binary file not shown.
905 changes: 905 additions & 0 deletions doc/guides/pinctrl/images/hw-cent-control.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added doc/guides/pinctrl/images/hw-dist-control.odg
View file
Open in desktop
Binary file not shown.
1,198 changes: 1,198 additions & 0 deletions doc/guides/pinctrl/images/hw-dist-control.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading