Skip to content

pcv Set/GetClockRate changes in 8.0.0#266

Merged
yellows8 merged 4 commits into
switchbrew:masterfrom
p-sam:pcv-8
Apr 24, 2019
Merged

pcv Set/GetClockRate changes in 8.0.0#266
yellows8 merged 4 commits into
switchbrew:masterfrom
p-sam:pcv-8

Conversation

@p-sam

@p-sam p-sam commented Apr 21, 2019

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread nx/source/services/clkrst.c Outdated
static u64 g_refCnt;

Result clkrstInitialize(void) {
if(!hosversionAtLeast(8,0,0)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if (hosversionBefore(8,0,0))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok

@yellows8 yellows8 self-requested a review April 22, 2019 19:06
Comment thread nx/source/services/clkrst.c Outdated

raw->magic = SFCI_MAGIC;
raw->cmd_id = 0;
raw->module = 0x40000001 + module;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This needs a few more changes. I've RE'd PCV on 8.0.0 and found this: https://switchbrew.org/wiki/PCV_services#Modules
It appears that each module now has an assigned ID which doesn't necessarily map directly into 0x40000001 + module (see I2C1 to I2C6, SPI1 to SPI4 and UARTA, UARTB, UARTC, UARTD).
My suggestion would be a new enum or even a struct describing a module that would contain the regular numerical value for it and the new ID.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Right, while they all map for the ones currently defined in pcv.h, it'd be breaking change in the future if we were to add all of them. @yellows8 Would you like all the modules defined ? It would be an enum that would be common to clkrst and rgltr API

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sure

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should i merge clkrst funcs in pcv.h/.c then, or what's the naming scheme you'd suggest?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just add the enum in pcv.h.

Comment thread nx/source/services/clkrst.c Outdated
raw->magic = SFCI_MAGIC;
raw->cmd_id = 0;
raw->module = 0x40000001 + module;
raw->unk = 3;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It might be worth considering adding this value as a parameter once its purpose is sorted out. Every module does indeed hardcode this to 3, but PCV supports values at least from 0 to 7.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same as previous comment = would avoid breaking changes. Also, this is already something done in capssu service API (passing an unk value as arg, default value documented in header). Waiting on maintainer feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move it to input param.

"would avoid breaking changes" clkrst is a new service in the first place...

@yellows8 yellows8 merged commit bc1786b into switchbrew:master Apr 24, 2019
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.

4 participants