Core-chat-meeting-2016-06-14

10:03 < geertu> (filled) Agenda:
10:03 < geertu> 1. Upcoming Gen3 development for the Core group,
10:03 < geertu> 2. Control of CPG parent clocks,
10:03 < geertu> 3. CPG DTS clean up ("clock-output-names", CPG/MSSR, ...)
10:03 < geertu> 4. Gen3 firmware plan for special memory allocation control
10:03 < geertu> 5. Additional tasks for Q3
10:03 < geertu> 6. Status check for tasks from last meeting - what is remaining?
10:04 < neg> I might drop out for a while during the meeting, got a water leak from the flat above and plumber on the way. So if I stop responding I be back as soon as I can.
10:04 < geertu> Topic 1. Upcoming Gen3 development for the Core group
10:05 < geertu> I will look into DT SMP/APMU support for new SoCs (r8a7793/r8a7794), as Sergei needs this too
10:06 < geertu> Anything else important going on?
10:08 < geertu> OK, nothing
10:08 < geertu> Topic 2. Control of CPG parent clocks
10:09 < horms> sorry, i was slow
10:09 < horms> about r8a7792
10:09 < geertu> yes
10:09 < horms> it seems his patchset is getting very close
10:09 < horms> and SMP even works
10:09 < horms> but we did get burnt with SMP on the r8a7795
10:09 < horms> (my fingers are still in bandages)
10:09 < geertu> using the deprecated smp ops
10:09 < horms> do you have any opinion on the smp aspects of his patchset?
10:10 < geertu> r8a7792 SMP is different. It should be similar to other Gen2. While r8a7795 depends on PSCI
10:10 < geertu> We should not merge his SMP patch/
10:10 < geertu> as we decided to no longer add new per-SoC platform code for that
10:11 < horms> Ah!
10:11 < geertu> cfr. r8a7793 still lacking SMP support
10:11 < horms> ok
10:11 < geertu> Magnus did post patches for using that a while ago, which I have been rebasing and keeping around locally.
10:11 < horms> have you brought this to Sergei's attention?
10:11 < geertu> I'll rework that and post them to the mailing list.
10:11 < horms> ok
10:11 < geertu> After that, r8a7792 support shouldn't need any platform code for SMP.
10:12 < horms> so if Sergei wants his patches to go in quickly he should drop SMP support
10:12 < geertu> The complication here is that doing it without any platform code needs a SYSC device node in DT.
10:12 < geertu> Which we have for r8a7792 (and r8a7793 now).
10:13 < geertu> Hence the major mess is that some SoCs have it, others don't.
10:13 < horms> Don't tell Dirk :^)
10:13 < geertu> I'll have to look into that again to untangle the mess and create a patch set that moves us forward.
10:13 < horms> Is the way forwards to define a reference platform, get that working, then spread the love out to other SoCs?
10:14 < geertu> Yes, that's what Magnus already did.
10:14 < horms> Ok, and you are planning to revisit this?
10:14 < geertu> Yes.
10:14 < horms> Ok, got it.
10:14 < geertu> I had it locally (incl. for r8a7791) running for a long time.
10:14 < horms> I won't merge r8a7792 SMP without it
10:15 < horms> which from my pov should mean r8a7792 UP gets merged first
10:15 < geertu> If only we could drop backwards compatibility with old DT ;-)
10:15 < pinchartl> geertu: I won't oppose to that :-)
10:15 < horms> but Sergei can decide how fast he wants to move
10:15 < horms> geertu: it can be discussed with the BSP team if it is sufficiently important
10:17 < geertu> OK, so Sergei can continue, if he drops SMP for now.
10:17 < horms> agreed
10:18 < geertu> Next topic?
10:18 < geertu> Topic 2. Control of CPG parent clocks
10:19 < geertu>   1. How to set initial value. by DT? by C code?
10:19 < geertu>   2. We want to have max frequency?
10:19 < geertu>   3. We want to save power/frequency if no user?
10:19 < geertu>   4. Schedule?
10:19 < geertu>   5. Use cases from BSP team?
10:19 < geertu>        a. MSIOF hi speed (?) need hi frequency
10:19 < geertu> Currently we just read the current values from the divider config registers
10:19 < geertu> For SDHI we have control of the parent clock (on R-Car Gen2)
10:20 < geertu> That is handled purely in the SDHI driver.
10:21 < geertu> My worries there are (a) that this is similar code that will end up in every single driver, and (b) that these drivers may run on platforms where they shouldn't touch the parent clock (cfr. SDHI changing the main peripheral clock on R-Mobile A1)
10:22 < geertu> Apparently the use case is increases the frequency of the MSIOF parent clock for faster SPI
10:22 < geertu> Morimoto-san: Is that correct?
10:23 < morimoto> Yes, it is
10:23 < morimoto> (at this point)
10:24 < pinchartl> regarding the initial value
10:24 < geertu> A quick solution would be to add similar code to the msiof driver like we did for sdhi
10:24 < pinchartl> we should read the value set by the boot loader(s) from the config registers to initialize the parent
10:25 < pinchartl> then there's the assigned-* DT clock properties that we can use
10:25 < pinchartl> it's not an ideal solution as it's more configuration data than a hardware description
10:25 < geertu> pinchartl: AFAIK we do read the initial value from the registers
10:25 < pinchartl> but it seems to be the upstream way of doing it
10:25 < geertu> pinchartl: Indeed, I don't like assigned-* at all
10:25 < geertu> pinchartl: Plus, for MSIOF you don't want to force this value.
10:25 < pinchartl> of course if there's a single consumer of the clock, then it would make sense for the driver to select the parent instead
10:26 < geertu> pinchartl: There are multiple MSIOFs
10:26 < pinchartl> the difficult case would be multiple consumers without assigned-*
10:26 < geertu> pinchartl: Some SPI slaves may be high-speed, others low-speed
10:26 < pinchartl> in that case the consumers would need to collaborate if you don't want to use assigned-*
10:26 < geertu> pinchartl: So it has to change dynamically
10:28 < geertu> I think MSIOF is also more complicated than SDHI as MSIOF has its own divider inside the module.
10:28 < geertu> So now you have two parameters: parent clock freq and per-device divider
10:29 < geertu> To get the wanted clock frequency, you can change one or both.
10:29 < geertu> Plus some other device may change the parent frequency behind your back, so you have to adapt the per-device divider
10:29 < geertu> if at all possible (and not out-of-range)
10:30 < geertu> And what if the parent frequency is changed while a transfer is in progress?
10:32 < pinchartl> that shouldn't be allowed
10:32 < horms> Are we expecting changes to occur at probe time or any time the driver feels like it?
10:32 < pinchartl> when you sya that some other device can change the frequency behind your back, to you mean another MSIOF device, or a completely unrelated device ?
10:32 < pinchartl> horms: at runtime unfortunately
10:33 < geertu> pinchartl: another MSIOF device (MSIOF parent is shared between MSIOF instances on Gen3)
10:34 < geertu> On Gen3, MSIOF parent is R8A7795_CLK_MSO
10:34 < horms> is it reasonable to wait for transfers to complete and then change the frequency?
10:34 < geertu> On Gen2, it's the shared MP clock
10:34 < geertu> horms: Sure
10:34 < pinchartl> geertu: then you need the drivers to collaborate
10:34 < geertu> but how to know?
10:35 < geertu> and how does the clock subsystem know a transfer is in progress?
10:35 < pinchartl> it doesn't
10:36 < pinchartl> that's for the driver to figure it out
10:36 < geertu> Ugh.
10:36 < geertu> So on Gen3, the msiof driver needs to know it shares a parent clock with other msiof instances?
10:37 < horms> do the instances use the same driver?
10:37 < geertu> On Gen2, it needs to know the parent clock is fixed (already handled ;-)
10:37 < pinchartl> some code might be moved to the ccf core if it's generic enough
10:37 < pinchartl> but there will be driver-specific code
10:37 < geertu> horms: Yes, spi-sh-msiof (assumed they use it for SPI, not for I2S)
10:37 < pinchartl> for instance you could extend the ccf api to add the ability to lock a clock
10:38 < pinchartl> while locked the frequency couldn't be changed by other drivers
10:38 < pinchartl> you'd have to decide what to do at unlock time though, whether frequency changes attempted while the clock is locked should be applied then
10:38 < pinchartl> or possibly use an unlock notifier
10:38 < geertu> I guess on some R-Mobile the MSIOF parent clock is shared with all other devices, and not fixed?
10:38 < pinchartl> but I'm not sure it would be useful in the ccf core
10:39 < pinchartl> as the msiof driver would need specific code to allow instances to collaborate to choose the frequency
10:39 < pinchartl> so it could as well handle the locking there too
10:40 < geertu> In the exterme case, the other instance has to wait until the first one releases the lock?
10:40 -!- shimoda [~shimoda@relprex2.renesas.com] has joined #periperi
10:40 < shimoda> hi
10:40 < geertu> Well, this definitely needs more thought...
10:40 < morimoto> In R-Mobile generation, if my memory was correct, we setuped  parent clock in boot time as fixed rate.
10:40 < geertu> BTW, what do we do with SDHI, w.r.t. collaboration?
10:41 < horms> geertu: that was my first thought; and my second one (1. wait for lock 2. more thought)
10:41 < geertu> IIRC there are multiple SDHI parent clocks, but some are shared between two SDHI devices?
10:41 < morimoto> It doesn't have runtime exchange case
10:41 < geertu> morimoto: it = SDHI?
10:41 < morimoto> it = parent clock
10:42 < pinchartl> geertu: I'm not sure we need more thought, at this point I believe we need a prototype
10:42 < horms> I think it makes sense to handle this in the driver if it can be handled there. Once the implementation is all sorted out it could migrate into the core, if that makes sense.
10:42 < geertu> OK
10:42 < morimoto> We didn't expect that runtime parent clock exchange
10:42 < horms> I agree we need a prototype
10:42 < geertu> Sounds like an additional tasks for Q3 ;-)
10:43 < horms> morimoto: you mean that runtime clock exchange isn't a requirement at this point?
10:43 < geertu> "  3. We want to save power/frequency if no user?
10:43 < geertu> "
10:43 < morimoto> In R-Mobile generation ? Yes, it is not requirement.
10:44 < pinchartl> geertu: agreed regarding additional task. it might be more of a I/O task though
10:44 < morimoto> And it is not requirement on Gen3 too :) BSP team want to change initial rate only
10:44 < geertu> OK, so it's like SDHI, where we limited fiddling with the parent to Gen2
10:44 < geertu> pinchartl: core + I/O
10:44 -!- horms2 [~horms@g1-27-253-251-9.bmobile.ne.jp] has joined #periperi
10:45 < geertu> "want to" != requirement?
10:45 < geertu> (Da evil twin brother has arrived?)
10:45 < morimoto> geertu: ? what is different ?
10:45 < horms> I'm trying to relocate. My twin is on my mobile.
10:46 < geertu> morimoto: OK, IC
10:46 < horms> morimoto: 1. Is it sufficient to do probe-time clock changes? 2. Is there an objection to run-time clock changes (as well) ?
10:46 < geertu> If it's just the initial value, it can be handled by assigned-rates
10:46 -!- horms [~horms@reginn.isobedori.kobe.vergenet.net] has quit [Quit: Leaving]
10:47 < morimoto> horms: 1. I think probe-time clock changes is very enough for BSP.
10:47 < geertu> But assigned-rates that has to take into account the whole system, which is complicated when someone uses DT overlays ;-)
10:47 < morimoto> horms: 2. I have no objection for run-time clock changes, but it is over-kill in my feeling
10:49 < geertu> In an SPI driver, you don't know the wanted frequencies during probe time
10:49 < geertu> Only when the SPI core calls .setup()
10:49 < geertu> When does DT assigned-rates kicks in?
10:50 < geertu> I assume it would not affect the MSIOF driver itself at all?
10:51 < horms2> I thought I saw that sdhi uses assigned-rates at probe time, but I could be mistaken
10:52 < morimoto> BSP team issue is that current MSIOF parent clock (= setuped by firmware ?) is not enough for hi speed. it is enough if CPG setup timing setups parent clocks more hi frequencies.
10:53 < geertu> horms2: It's parsed only in drivers/clk/clk-conf.c
10:53 < morimoto> .setup() can divide on each channels
10:53 < geertu> morimoto: Yes, it runs at 12.5 MHz instead of 66 MHz.
10:53 < pinchartl> geertu: MSIOF might want to change the parent clock frequency for two reasons, either because it's too low for the requested SPI frequency, or to achieve a more accurate SPI frequency
10:53 < geertu> pinchartl: Indeed
10:53 < pinchartl> in both cases a locked parent clock rate due to another transfer in progress wouldn't need to be treated as a fatal issue
10:54 < geertu> pinchartl: Yep
10:54 < pinchartl> the transfer could be performed at a lower than requested speed or at a less accurate speed
10:54 < pinchartl> but it also means that two identical transfer requests for the same device could lead to different speeds
10:54 < pinchartl> depending on the other transfers in progress
10:54 < geertu> pinchartl: Indeed. But the second instance should be able to lock the parent, too. So we need a R/W lock
10:54 < pinchartl> I wonder if that could be an issue for drivers
10:55 < pinchartl> the resulting speed wouldn't be deterministic
10:55 < geertu> pinchartl: Not for drivers, but for devices that have strict frequency requirements.
10:55 < pinchartl> at least from SPI drivers point of view
10:55 < geertu> I can imagine some device require to be driven at a specific frequency? Currently there's only max-frequency, and you usually get that
10:55 < pinchartl> although there's no guarantee in the SPI API that the requested speed will be honoured exactly (or with a certain accuracy)
10:56 < geertu> pinchartl: true
10:56 < pinchartl> what we guarantee today (at least in practice if not in theory) is that the speed will be the same for all transfers requested with the same speed
10:56 < pinchartl> that might change in the future
10:57 < pinchartl> I just wanted to mention the issue, it might not be a problem in practice
10:57 < geertu> Even if we don't support dynamic speed change, MSIOF instances are instantiated sequentially.
10:58 < geertu> So the first one decides?
10:59 < pinchartl> that's an open question
10:59 < geertu> OK, let's continue...
10:59 < geertu> Topic 3. CPG DTS clean up ("clock-output-names", CPG/MSSR, ...)
11:00 < geertu> This is a topic from Magnus
11:00 < geertu>   1. Can we do something to clean up the 32-bit ARM SoCs?
11:00 < geertu>   2. Can we make new SoC support slightly cleaner?
11:00 < geertu>   3. Shall we jump directly to CPG-MSSR for new SoCs and clean up the rest of the R-Car Gen2
11:00 < geertu>      family?
11:00 < geertu> But as he's not here, perhaps we should skip it?
11:01 < geertu> Or postpone till the end, if time permits?
11:01 < horms2> Probably a good idea
11:01 < geertu> Topic 4. Gen3 firmware plan for special memory allocation control
11:01 < morimoto> geertu: 1 question about parent clock. Does it become next quarter's SoW for someone ?
11:01 < geertu> morimoto: Probably
11:01 < morimoto> geertu: OK
11:02 < morimoto> geertu: Can you please add it to peripelist ?
11:02 < geertu> morimoto: It needs coordination with I/O
11:02 < morimoto> Ohh OK I see
11:03 < geertu> Morimoto-san: The mike (microphone) is yours, for topic 4
11:03 < morimoto> OK, As I mentioned in periperi ML, our latest firmware is v2.8.0
11:03 < morimoto> but, because of memory allocation control, it is custom version
11:04 < morimoto> Unfortunately, v2.9.0 has same feature
11:04 < morimoto> v2.10.0 will doesn't have it.
11:04 < morimoto> So,
11:04 < morimoto> I guess we need to indicate this issue on upsteam code somewhere ?
11:04 < geertu> The latest firmware we have (https://osdr.renesas.com/projects/linux-kernel-development/wiki/Periperi-2016-02) is v270
11:05 < morimoto> Oops ?
11:05 < geertu> And I'm quite sure most people aren't even running that version yet ;-)
11:05 < morimoto> Indeed. So, because of this reason, I didn't uploaded v2.8.0
11:06 < morimoto> Our customer already have v2.8.0
11:06 < morimoto> So, if customer uses BSP only, there is no problem
11:06 < morimoto> If customer uses BSP firmware (= v2.8.0 or v2.9.0) and uses upsteam kernel
11:06 < morimoto> it doesn't work
11:07 < morimoto> (BSP kernel already has this solution)
11:07 < morimoto> If customer uses v2.8.0 or v2.9.0 firmware, but custom version, and uses upsteam, then no problem.
11:08 < morimoto> v2.10.0 + upsteam has no problem again
11:09 < morimoto> v2.8.0 and v2.9.0 has special memory allocation as default
11:10 < morimoto> So, we need to indicate it on somewhere ?
11:10 < geertu> Well, this is a temporary issue only, right?
11:11 < horms2> do you mean should we document this?
11:11 < morimoto> geertu: Yes, I think so. BSP team will explain this issue to customer
11:11 < morimoto> horms2: I'm not sure. but we will have confusion ??
11:12 < horms2> probably
11:12 < geertu> We don't really have a place to document it,
11:12 < geertu> except by adding the section to the DTS
11:13 < morimoto> quick explain is enough ? how about comment area on r8a7795-salvator-x.dts ?
11:13 < morimoto> Not sure what should we do
11:14 < horms2> what about the elinux wiki? documenting it somehow in dt seems ok to me too
11:14 < morimoto> super big issue is that our firmware doesn't indicate its version.
11:15 < horms2> ew
11:17 < shimoda> if the function is enabled, the firmware mention somethings :)
11:17 < shimoda> NOTICE:  BL2: Lossy Decomp areas
11:17 < shimoda> NOTICE:       Entry 0: DCMPAREACRAx:0x80000000 DCMPAREACRBx:0x0
11:17 < shimoda> NOTICE:       Entry 1: DCMPAREACRAx:0x40000000 DCMPAREACRBx:0x0
11:17 < shimoda> NOTICE:       Entry 2: DCMPAREACRAx:0x20000000 DCMPAREACRBx:0x0
11:17 < shimoda> this is disabled though
11:18 < shimoda> but, if enabled, DCMPAREACRBx value is not zero and the DCMPAREACRAx value is related to the memory area
11:19 < shimoda> but of course a software cannot understand it :)
11:20 < morimoto> If this feature was enabled, firmware setup its memory area on fixed location. So 2nd solution is that upsteam kernel support it
11:20 < morimoto> Opps firmware indicates its memory area on fixed location
11:21 < horms2> so it's difficult to detect at runtime?
11:22 < geertu> IIRC it's a register that can be read from secure mode only
11:23 < geertu> I would document it on the elinux wiki
11:23 < morimoto> If firmware uses it, it setups memory and indicates its area on fixed memory location.
11:23 < morimoto> I forgot detail, but for example, if firmware uses 0xAAAA0000 for indication
11:24 < geertu> Or will you send the DTS update straight to Linux, to prevent it being out of date?
11:24 < geertu> And what about -stable?
11:24 < morimoto> it will be 0xffffffff if disabled, and it will be 0xXXXXXXXX (= we can't use this area) if enabled
11:25 < shimoda> geertu: I prefer just document on the elinux wiki
11:25 < geertu> OK
11:25 < morimoto> It is easy solution
11:26 < shimoda> but I don't know morimoto-san's thinking about :)
11:26 < morimoto> My favorite is of course easy solution.
11:26 < morimoto> above is 2nd solution, but very PITA for us
11:27 < morimoto> that's it from me
11:28  * pinchartl has to go
11:28 < horms2> let's document it for now
11:28 < geertu> We also ran out of time
11:29 < morimoto> I will give you v2.10.0 of course. But do you want to have v2.8.0 or v2.9.0 custom verion ?
11:30 < morimoto> You can ask me via email :)
11:30 < horms2> good plan
11:32 < geertu> Who will document it on the elinux wiki?
11:33 < shimoda> renesas guys will document it (maybe me? :) )
11:34 < geertu> Topic 5. Additional tasks for Q3
11:34 < geertu>   - M3 support (physical boards too late :-(
11:34 < geertu>   - 64-bit memory handling
11:34 < geertu> I'm afraid we don't have time for this today
11:34 < geertu> Please submit your ideas to the mailing list
11:34 < geertu> Topic 6. Status check for tasks from last meeting - what is remaining?
11:35 < geertu> Any updates?
11:36 < neg> I hope to post a summary of the dmac branches I have looked in to today or tomorrow, following some patches I have extracted form them which I think are worth to keep
11:37 < horms2> geertu: how are r8a7796 clocks going merge wise?
11:37 < geertu> horms2: No response from the clock team to my pull request so far
11:37 < geertu> will ping them
11:37 < horms2> ok
11:37 < horms2> thanks
11:38  * geertu got crippled by gnome-terminal. Only two terminal windows are still working.
11:39 < horms2> boo
11:39 < geertu> Fixing that will kill my irc session... so...
11:39 < geertu> Time to conclude! 
11:40 < horms2> good plan!
11:40 < geertu> Thanks for joining, have a nice continued day!
11:41 < geertu> (woow, the issue has fixed itself)
11:41 < geertu> Bye!
11:41 < shimoda> bye!
11:41 < morimoto> Thank you, bye