<bgamari>
So I guess the fundamental unit issue just needs to be understood and resolved
gordan has joined #linux-exynos
krzk has joined #linux-exynos
krzk has quit [Quit: Wychodzi]
<javier__>
bgamari: yes, I said that the patch was not a correct fix but just fixing the symptom to allow it to boot
<javier__>
bgamari: I would had expected that the DT parsing code would take care of doing any unit conversion and the CPUFreq core to always work on the same unit
<javier__>
bgamari: unforunately I'll not have time to work on this today, maybe next week
<bgamari>
javier__, so you would expect the cpufreq frequency table to be in Hz?
<bgamari>
Strangely it appears that the result from op_property_read_u64 is already a factor of 1000 smaller than expected
<bgamari>
the relevant dt parsing code is drivers/base/power/opp/core.c:_opp_add_static_v2 I believe
EmilMedve has joined #linux-exynos
<javier__>
bgamari: not sure what the unit should be for the cpufreq frequency, Documentation/cpu-freq/user-guide.txt says that the user visible values should be in KHz
<javier__>
*cpufreq table
<bgamari>
hmm
<bgamari>
it looks like opp typically uses Hz
<bgamari>
javier__, well, I have it working on cluster 0
<bgamari>
javier__, just needed to fix the exynos 5800 dt
<bgamari>
which I had neglected to do
<bgamari>
stupidly assuming that the xu4 would use the 5420 dt
<javier__>
bgamari: yeah, the XU{3,4} has a Exynos5422
<bgamari>
javier__, well, that may be true
<javier__>
bgamari: that's why I shared that patch since I've a XU4 here to test
<bgamari>
but exynos5422-odroidxu3-common.dtsi includes exynos5800.dtsi
<bgamari>
which is where the opp tables are coming from
<bgamari>
javier__, perhaps this is also wrong?
<bgamari>
cluster 1 failed to change frequencies still
<bgamari>
oh, actually...
<bgamari>
it works when I request 1.3GHz
<bgamari>
e.g. cpufreq-set -c4 -f1300M
<bgamari>
1.4GHz also seems to work
<bgamari>
1.5GHz fails, however
<Wizzup>
but the voltage doesn't scale, right?
<Wizzup>
undervoltage may also lead to instability even if it seems stable
<bgamari>
the arm regulator is sitting at 1100mV
<bgamari>
which is what it should be
<javier__>
bgamari: yes, the opp table being duplicated in exynos5420.dtsi and exynos5800.dtsi seems wrong indeed
<javier__>
or unncessary
<bgamari>
really?
<bgamari>
they should share operating points?
<javier__>
it should only be defined in exynos5420.dtsi (unless there are differences that I missed) ?
<bgamari>
the 5800 has more OPs
<bgamari>
17 of them
<bgamari>
whereas the 5420 has 12
<javier__>
bgamari: Ok, sorry I missed that difference since I'm looking at this while working on other stuff
<bgamari>
it looks like the difference is that the 5420 just lacks the OPs corresponding to 200, 300, 400, 500, and 600 MHz
<javier__>
bgamari: then is correct, the Exynos5800 is the same than Exynos5422 but with some hw errata fixed AFAIU
zombah has joined #linux-exynos
<javier__>
and the Exynos5422 big and LITTLE cores are inverted, AFAIU there could be Exynos5422 with the same order than Exynos5420/5800 but I haven't seen it
<bgamari>
javier__, but presumably the XU4 should be using the 5422 OPP table, not the 5800 table, no?
<javier__>
bgamari: but there isn't a 5422 OPP table
<javier__>
there is only a arch/arm/boot/dts/exynos5422-cpus.dtsi that overrides the CPUs nodes
<bgamari>
meh, sorry; s/5422/5400/
<bgamari>
ahh, I see
<bgamari>
alright then
<bgamari>
so this should be fine in that case
<javier__>
bgamari: yes, so in summary:
<javier__>
- exynos5420 should use 5420 OPP table
<javier__>
- exynos5422/5800 should use 5800 OPP table
<javier__>
- exynos5422 should have the opp tables inversted in exynos5422-cpus.dtsi
<javier__>
- exynos5800 OPP tables units should be fixed (I also missed these were wrong before)
<javier__>
bgamari: it would be great if you can squash your changes into Bart's patches adding your S-o-B and attribution of what you changed
<bgamari>
javier__, first I need to fix the ARM15 cpufreq support
<bgamari>
javier__, for some reason my xu4 is failing to set any frequency higher than 1.4GHz on cpus 4-7
<javier__>
bgamari: yes, I meant once you are finished and things are working correctly
<bgamari>
of coruse
<javier__>
bgamari: awesome, thanks a lot for working on this
<bgamari>
any ideas where this 1.4GHz limit is coming from?
<bgamari>
it's quite odd
<javier__>
bgamari: mmm no, AFAICT it should be safe to scale up to 1.8GHz...
<bgamari>
I don't see an OPP table that stops at 1.4GHz
<bgamari>
5800's opp_table1 stops at 1.3GHz
<javier__>
bgamari: care to refresh your branch so I can give a try on my peach pi?
<bgamari>
table0 stops at 1.8GHz
<bgamari>
sure
<javier__>
yes, the A7 should scale up to 1.3GHz
<bgamari>
in fact, it's pushed
<javier__>
bgamari: awesome, thanks!
<bgamari>
Is there any way to dump the opps?
<gordan>
Can we have a /proc or /sys controlled overclocking functionality, please? :D
<bgamari>
I wonder if this is a clock limit
<bgamari>
maybe the clocks need to be swapped as well?
<bgamari>
it would make sense that the A7's clock had a maximum a bit higher than the core's spec frequency
<bgamari>
kind of
<bgamari>
ahh, indeed
<bgamari>
1.4GHz is the highest clockrate specified in exynos5420_kfcclk_d
<bgamari>
javier__, does this help?
<javier__>
bgamari: oh, that's a very good question
<javier__>
yeah, it seems that exynos5422-cpus.dtsi is lacking the clock properties indeed
<bgamari>
indeed
<bgamari>
are they really only necessary on cpus 0 and 4?
<bgamari>
if so, what about operating-points?
<javier__>
bgamari: I don't understand your question about operating points
<bgamari>
pushed a patch
<bgamari>
javier__, is the operating-points-v2 attribute necessary on all cpu nodes?
<bgamari>
well, still seems to boot with these changes
<javier__>
bgamari: I OPP properties are needed by all CPUs according to the DT binding
<javier__>
which also makes sense since the sysfs interface is per CPU
<bgamari>
alright
<bgamari>
things look good
<javier__>
the fact that all the cores that belongs to a cluster have the same OPP is just a property of b.L
<bgamari>
I can clock it up to 1.8GHz
<bgamari>
and the little up to 1.3GHz
<javier__>
bgamari: the clock only on CPU0 and CPU4 makes sense since you do it per cluster
<bgamari>
regulators appear to be set properly
<javier__>
bgamari: awesome, that was it then
<bgamari>
I see
<bgamari>
javier__, indeed
<gordan>
bgamari: Is there a way to extend the table beyond 1.8?
<bgamari>
edit the DT?
<gordan>
Or is the clock generator limited to 1.8?
<javier__>
gordan: is a problem on exynos5420/5422/5800
<javier__>
gordan: for higher frequencies, you need a minimal diff between vdd_arm and vdd_int
<gordan>
I've not looked at Exynos code, but when I wrote the Tegra2 OC patches it was fairly trivial to extend the clock tables beyond the stock 1.0GHz (I added a few entries up to 1400+MHz).
<javier__>
I mentioned that already the other day, the downstream tree has a way to couple those two regulators together so when the voltage is changed in vdd_arm, vdd_int is changed as well
<javier__>
gordan: yes but in this case is not only about adding more OPP in the tables
<bgamari>
javier__, not merge-worthy I guess?
<javier__>
bgamari: it was posted but Mark Brown (regulator maintainer) didn't like the approach
<javier__>
I can't remember now why I nacked the patches but I've on my (very long) TODO list
<javier__>
s/I/he
<gordan>
In Tegra2 code there are several corresponding tables for voltages and several clock tables, and things had to be kept at certain ratios with multipliers and suchlike.
<bgamari>
javier__, do you have an approximate date of this thread?
<javier__>
bgamari: let me search the thread for you
<javier__>
the idea is to couple two regulators when these are used to spread consumption for a single device
<javier__>
this is used in the C.H.I.P board
<javier__>
so maybe that could be extended to cover the user case of the Peach
<javier__>
*use case
<javier__>
I can't type today...
<bgamari>
hmm
<bgamari>
I'm not sure I see how this would apply to the enforcement of regulator "closeness"
<bgamari>
well, anyways, this appears to be a rather tricky area
<bgamari>
perhaps I'll let it be for now
* bgamari
just needed to get his build-bot running at a reasonable speed
<javier__>
bgamari: yeah, I have to admit that I didn't look at maxime patches but I was thinking that both use cases needs to couple two regulators together so maybe the DT binding or implementation could be reused
<javier__>
bgamari: but yeah, is a tricky are and something that can be discussed later
afaerber has quit [Ping timeout: 260 seconds]
<javier__>
bgamari: when you post the CPUFreq patches, could you please add me as cc ?
<bgamari>
yep
<javier__>
bgamari: I tested on my Exynos5800 Peach Pi and things are working correctly as well
<javier__>
bgamari: awesome work :)
<bgamari>
great!
<bgamari>
thanks!
amitk has quit [Quit: leaving]
afaerber has joined #linux-exynos
<Turl>
afaerber: ping
<afaerber>
Turl: pong
<Turl>
afaerber: hey :)
<afaerber>
what's up?
<Turl>
afaerber: I'm trying to find out which EC version is actually shipping/used on spring, any chance you can help me?
<afaerber>
Turl: not immediately, but if you tell me how to find out I can check the next days
<afaerber>
my USB SD card adapter used for USB booting broke :/
<Turl>
ouch :/
<afaerber>
(physically, the mini USB connector)
<Turl>
afaerber: I believe there's an ectool command to print it, and iirc it's exposed on sysfs
<afaerber>
ah, so that would need ChromeOS - don't think I have ectool on openSUSE
<javier__>
afaerber: the sysfs interface should be available in mainline
<javier__>
for ectool you need a special branch that uses the IOCTL in mainline since is different than the one used in the downstream tree
<javier__>
so the chromeos binary won't work
<javier__>
IOW, better to go with the sysfs :)
Zotan has quit [*.net *.split]
gordan has quit [*.net *.split]
ssvb has quit [*.net *.split]
meridion has quit [*.net *.split]
dlan has quit [*.net *.split]
afaerber has quit [*.net *.split]
Vasco_O has quit [*.net *.split]
Swabbles has quit [*.net *.split]
EmilMedve has quit [*.net *.split]
Gottox has quit [*.net *.split]
prahal____ has quit [*.net *.split]
masta has quit [*.net *.split]
bgamari has quit [*.net *.split]
_anomaly_ has quit [*.net *.split]
tyler-baker has quit [*.net *.split]
leming has quit [*.net *.split]
nashpa has quit [*.net *.split]
silv312_ has quit [*.net *.split]
Turl has quit [*.net *.split]
aballier_ has quit [*.net *.split]
mturquette has quit [*.net *.split]
indy has quit [*.net *.split]
arnd has quit [*.net *.split]
sjoerd has quit [*.net *.split]
caveat- has quit [*.net *.split]
Wizzup has quit [*.net *.split]
libv has quit [*.net *.split]
twitch153 has quit [*.net *.split]
daniels has quit [*.net *.split]
javier__ has quit [*.net *.split]
bzyx has quit [*.net *.split]
indy has joined #linux-exynos
afaerber has joined #linux-exynos
Vasco has joined #linux-exynos
Swabbles has joined #linux-exynos
afaerber has quit [*.net *.split]
Vasco has quit [*.net *.split]
Swabbles has quit [*.net *.split]
twitch153 has joined #linux-exynos
libv has joined #linux-exynos
Wizzup has joined #linux-exynos
caveat- has joined #linux-exynos
Vasco has joined #linux-exynos
Swabbles has joined #linux-exynos
sjoerd has joined #linux-exynos
afaerber has joined #linux-exynos
Vasco has quit [*.net *.split]
Swabbles has quit [*.net *.split]
sjoerd has quit [*.net *.split]
caveat- has quit [*.net *.split]
Wizzup has quit [*.net *.split]
libv has quit [*.net *.split]
twitch153 has quit [*.net *.split]
Swabbles has joined #linux-exynos
Vasco has joined #linux-exynos
sjoerd has joined #linux-exynos
caveat- has joined #linux-exynos
Wizzup has joined #linux-exynos
twitch153 has joined #linux-exynos
libv has joined #linux-exynos
afaerber has joined #linux-exynos
afaerber has quit [Changing host]
gordan has left #linux-exynos ["Konversation terminated!"]
<bgamari>
javier__, I've pushed a cleaned up branch
<javier__>
bgamari: my email address is no longer that one
<bgamari>
oh?
<bgamari>
collabora?
<bgamari>
I see
<bgamari>
@osg.samsung
<javier__>
bgamari: yes
<javier__>
bgamari: do you really need both regulators for cluster 0 and 1 in exynos5422-odroidxu4.dts ?
<javier__>
I see that for other boards only the supply for cluster 0 is in CPU0 and for cluster 1 in CPU4
<javier__>
so I would expect that just the power supplies for cluster 1 in CPU0 and cluster 0 in CPU4 are needed
<bgamari>
hmm
<bgamari>
that's a good question actually
<bgamari>
javier__, the interface is a bit awkward
<bgamari>
I don't understand why the cluster number is encoded in the attribute name at all
* javier__
haven't looked at the implementation in detail to answer
<javier__>
bgamari: also, I believe that patch should be squashed with "ARM: dts: Exynos5420/5800: add cluster regulator supply properties"
<javier__>
there is no need to have a separate patch for that
<javier__>
I mean patch "ARM: dts: odroidxu4: add CPU cluster regulators"
<javier__>
bgamari: I see that some patches are missing your S-o-B too
<bgamari>
oh?
<bgamari>
javier__, well, those are the ones I essentially didn't touch
<bgamari>
I believe
<bgamari>
I'll need to double-check
<bgamari>
javier__, which patches should be squashed?
<javier__>
bgamari: I would squash "ARM: dts: odroidxu4: add CPU cluster regulators" into "ARM: dts: Exynos5420/5800: add cluster regulator supply properties"
<javier__>
and mention that is for Exynos5420/5422/5800
<javier__>
so you can add your s-o-b and then add attribution as [ben: added suppplies for exynos5422 boards] before your S-o-B
<javier__>
or something like that
<javier__>
bgamari: even for patches that you didn't touch, you need to add your S-o-B at the end if you were involved in the upstreaming
<bgamari>
Alright
<javier__>
but you should keep the authorship with --author unless you believe that changed too and in that case mention in the changelog that was based on someone else patch
<javier__>
bgamari: sorry if everything I'm saying is evident, I'm just trying to help :)
steev has quit []
<bgamari>
javier__, no worries
<bgamari>
javier__, sure
<bgamari>
javier__, pushed a new branch
<bgamari>
or rather
<bgamari>
pushing
<bgamari>
dpome
<bgamari>
done
zombah has quit [Quit: Leaving]
afaerber has quit [Quit: Ex-Chat]
<javier__>
bgamari: patches looks good to me now
<javier__>
ther is only one S-o-B missing in "ARM: Exynos: use generic cpufreq driver for Exynos5800"
<javier__>
bgamari: again, thanks a lot for working on this
<bgamari>
doh
<bgamari>
javier__, Thanks for the style review
<bgamari>
javier__, my pleasure
<javier__>
bgamari: yw
<bgamari>
javier__, alright, I guess I'll send it out then
<javier__>
bgamari: awesome!
daniels has joined #linux-exynos
ojn has quit [Read error: Connection reset by peer]