lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.56k stars 762 forks source link

[Naming] Clock Naming #2007

Closed tjaychen closed 4 years ago

tjaychen commented 4 years ago

hi, just wanted to get some general opinions. Right now we have 3 clocks defined main, fixed and usb.

We need to add another slow clock as well as decide if fixed is a good name.

Some of our partners have started calling the fixed clock io clock, to imply that the clock should be static frequency in nature for io's. Are people okay with that name?

Would anyone object to the creation of "slow" for a KHz level clock?

vogelpi commented 4 years ago

Hi @tjaychen , slow sounds good to me.

Regarding fixed/io: what about fast? This maybe depends a bit on the difference between main and fixed/io. If I remember correctly, both slow and fixed/io are actually fixed, whereas main is about equally fast as fixed/io but not fixed. ;-)

msfschaffner commented 4 years ago

Thanks for bringing this up @tjaychen .

I am fine with creating a slow clock for the KHz level clock (we already started to use that name in some IPs, AFAIK).

re fixed versus io, I would say that fixed may be a bit misleading, as it sort of implies that all other clocks are not fixed. that is only true for the main clock. so switching over to io clock or something else is ok from my side.

eunchan commented 4 years ago

My prev. experience uses that kind of slow clock as aon_clk. or aux_clk. slow is fine also but personally prefer aux_clk. :)

tjaychen commented 4 years ago

Hi @tjaychen , slow sounds good to me.

Regarding fixed/io: what about fast? This maybe depends a bit on the difference between main and fixed/io. If I remember correctly, both slow and fixed/io are actually fixed, whereas main is about equally fast as fixed/io but not fixed. ;-)

so i don't REALLY want to use fast mostly because the main clock is supposed to even faster... the main clock is supposed to be ~100MHz, while fast is really only around 24MHz.

tjaychen commented 4 years ago

o i like aon_clk.... that does very accurately reflect it's function :) I think aux is pretty good too...

let me contemplate this a bit... for the same reason i don't really like fast, i also don't really like slow, (and @msf's point about fixed also makes sense)...

so i think I am leaning towards aon/aux or io. Let me know if you guys would have a big problem with that combination.

msfschaffner commented 4 years ago

+1 for aon and io

sjgitty commented 4 years ago

my preferences:

clk_aon for the ~100kHz always-on clock (although I see some trends to alw for the always-on stuff, I prefer aon) clk_main is fine for jittery proc + crypto clock clk_io or clk_timer or clk_fixed for the other clock, but how will we distinguish the different divided versions?

I kind of don't like the slow and fast connotations. aux for aon wfm, though "auxiliary to what?" will be the question, while aon is more correct, brief, and accurate.

cdgori commented 4 years ago

I also generally like aon for the 100kHz clk and io for the fixed one - main is a reasonable name for the "core" clk (I've seen core used for this but it's so overloaded with meaning that it gets dangerous in a hurry).

slow/fast feel weird to me, would probably avoid.

tjaychen commented 4 years ago

thanks all, i think there is general consensus towards main / io / aon. If there are no other objections, I will close this issue and make the same proposal to our partners. Once they approve, I will begin a clock re-naming exercise.

On Thu, Apr 23, 2020 at 11:26 AM Chris Gori notifications@github.com wrote:

I also generally like aon for the 100kHz clk and io for the fixed one - main is a reasonable name for the "core" clk (I've seen core used for this but it's so overloaded with meaning that it gets dangerous in a hurry).

slow/fast feel weird to me, would probably avoid.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/lowRISC/opentitan/issues/2007#issuecomment-618571276, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH2RSQTTRTGFAW4NRCUNHTROCB4LANCNFSM4MMVVMUQ .

sjgitty commented 4 years ago

If there are no other objections, I will close this issue and make the same proposal to our partners. Once they approve, I will begin a clock re-naming exercise.

@tjaychen which partners? This issue should hit all of the OT team. I think it is good enough to go since you've pinged a bunch of folks. Maybe this: everyone on the thread has until EOD Friday to weigh in if they're not happy with main / io / aon.

sjgitty commented 4 years ago

Pardon on that last comment, I wrote it Thursday PM, but network issues rejected it, just hit resend. We can give until EOD Monday perhaps.

tjaychen commented 4 years ago

sounds good, thanks Scott

On Fri, Apr 24, 2020 at 4:28 PM Scott Johnson notifications@github.com wrote:

Pardon on that last comment, I wrote it Thursday PM, but network issues rejected it, just hit resend. We can give until EOD Monday perhaps.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/lowRISC/opentitan/issues/2007#issuecomment-619279247, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH2RSQDC6FLVQTB6Z6YZHDROIOBDANCNFSM4MMVVMUQ .

tjaychen commented 4 years ago

soo..i'm beginning to think clk_aon is not such a good idea. The aon term is becoming overloaded with the power domain.

For example, imagine we want to create a reset that is in the aon power domain, but also synced to clkaon.. we are going to have a funny looking name if we assume the format is `rst{rootname}{powerdomain}{clk_domain}_n`

Maybe that's okay as long as we are consistent....

sjgitty commented 4 years ago

Hmm, good point. rst_aon_aon_aon_n? :)

maybe back to clk_aux?

On Thu, May 14, 2020 at 12:05 AM tjaychen notifications@github.com wrote:

soo..i'm beginning to think clk_aon is not such a good idea. The aon term is becoming overloaded with the power domain.

For example, imagine we want to create a reset that is in the aon power domain, but also synced to clkaon.. we are going to have a funny looking name if we assume the format is rst{rootname}{powerdomain}{clk_domain}_n

Maybe that's okay as long as we are consistent....

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/lowRISC/opentitan/issues/2007#issuecomment-628433662, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJZQKVNPZOQOUYAV2JQOF3TRROJ5NANCNFSM4MMVVMUQ .

tjaychen commented 4 years ago

@sjgitty sorry for not responding sooner. Yes i think once power domains are introduced, i'm going to move this back to clk_aux.

I'll close this issue down for now, thanks everyone for the input!