path: root/wiki/Chat_log/20151020-core-chatlog

11:02 < geertu> Welcome to today's Renesas Core Group Chat
11:03 < geertu> Today we have a special guest: Mike Turquette (thx!)
11:03 < dammsan> \O/
11:03 < mturquette> My pleasure to attend!
11:03 < mturquette> Thank you for the invitation.
11:04 < geertu> Agenda:
11:04 < geertu> 1. Upcoming Gen3 development for the Core group,
11:04 < geertu> 2. New CPG/MSSR Driver,
11:04 < geertu> 3. rcar-dmac: pick up the patches, fix the DMA engine API and save the world,
11:04 < geertu> 4. Status check for tasks from last meeting - what is remaining?
11:04 < dammsan> mturquette: thanks for joining
11:04 < geertu> Given Mike's presence, I think we should start with Topic 2.
11:04 < mturquette> dammsan: :-)
11:05 < dammsan> geertu: your proposal looks all good to me - the only nit i have is the compat string
11:05 < geertu> Mike: I hope the bindings (and driver) I posted are in-line with your remaps?
11:05 < mturquette> geertu: yes. I've reviewed the binding it looks great.
11:05 < geertu> s/remaps/remarks/
11:05 < mturquette> I'm still going through the driver changes, but the binding perfectly fits our discussion from ELC-E
11:06 < geertu> dammsan: What's your nit?
11:06 < dammsan> i feel that the "mssr" bit is something unknown
11:07 < dammsan> but i guess the mssr portion is outside the cpg?
11:07 < dammsan> s/bit/part of the name/g
11:08 < mturquette> Follow on question to dammsan: are the descriptions of the "mssr" bit fields in the same chapter/section as the cpg register descriptions in your internal documentation?
11:08 < mturquette> if so, it might be easier to just name the whole thing "cpg"
11:08 < geertu> Datasheet (for Gen2) says:
11:08 < geertu> 7. Clock Pulse Generator (CPG)
11:08 < geertu> 7A. Module Standby and Software Reset
11:08 < geertu> 7B. Advanced Power Management Unit for AP-System Core (APMU)
11:09 < mturquette> ah
11:09 < geertu> The registers in 7A are inside the same 4 KiB block of the CPG registers
11:09 < geertu> The registers in 7B are in a separate 4 KiB block
11:09 < dammsan> mturquette: i think so
11:11 < dammsan> I guess 7B needs to be managed by PSCI firmware
11:11 < geertu> Same for Gen3 (chapters 8/8A/8B)
11:11 < dammsan> geertu: do you have any strong feelings about the compat string name?
11:12 < geertu> The CPG and MSSR registers are really mixed, so you cannot separate them
11:13 < dammsan> right, so this is a good match for the single DT node approach
11:13 < geertu> dammsan: I just came up with a name that describes the block "renesas,r8a7795-cpg-mssr"
11:14 < geertu> Do you have a better suggestion?
11:14 < dammsan> geertu: i'm ok with that, but i would simply go with "r8a7795-cpg"
11:14 < geertu> (the old bindings for CPG only use e.g. ""renesas,r8a7790-cpg-clocks", and 
11:14 < dammsan> and keep the mssr abstraction in C
11:14 < mturquette> I'm fine with either compat string name. Hopefully future chips will call the IP block something like Clock And Reset (CAR) or Reset and Clock Manager (RCM) or something :-)
11:14 < geertu> "renesas,rcar-gen2-cpg-clocks"
11:15 < dammsan> so our only concern at this point is the compat string? =)
11:15 < mturquette> I did have one other quick question
11:15 < mturquette> Do you plan to introduce reset framework bits into drivers/clk/shmobile/ ?
11:15 < geertu> If the technical side is OK, we can start bikeshedding
11:15 < geertu> mturquette: Yes, that's the plan
11:15 < mturquette> geertu: OK, great.
11:16 < geertu> tegra does it that way, too, IIRC
11:16 < mturquette> geertu: for the binding I'm very happy with it.
11:16 < geertu> I'm running the code here on r8a7795 and r8a7791, so that part works, too.
11:16 < mturquette> geertu: I don't have any review comments for the driver implementation at this time, so I'll post them later today on the list if I have anything, otherwise I can merge the series.
11:16 < dammsan> good!
11:16 < geertu> For the resets, it's still to be seen how good that works, and how it is to be used
11:17 < mturquette> geertu: or, do you plan to package things up in a PR?
11:17 < geertu> (resets have to be used explicitly by each individual driver?)
11:18 < dammsan> if everyone is happy - when can we see it in -next?
11:18 < dammsan> (i mainly care about the binding)
11:18 < geertu> Does anyone have any comment w.r.t. #defining all clocks in include/dt-bindings/clock/r8a7795-cpg-mssr.h upfront, or only having the ones we curently need?
11:19 < dammsan> geertu: as long as error handling is sane anything is fine with me
11:19 < dammsan> (running new DTB on old kernel)
11:20 < geertu> dammsan: new DTB on old kernel won't work
11:20 < geertu> dammsan: old DTB on new kernel should work if you include the old driver
11:20 < dammsan> geertu: but when you add clocks you increase support level in the driver, right?
11:21 < geertu> dammsan: another question is how complex we want the logic in drivers/sh/pm_runtime.c for the legacy clock domain to be
11:21 < mturquette> RE: defining all clocks, I'm of course fine with that, but the header is part of the "stable" binding.
11:21 < geertu> dammsan: yes, the cpg-mssr driver so far only implements the clocks we use
11:21 < mturquette> geertu: so it might be a safer approach to only add clocks as you need them. removing clocks breaks backwards compatibility.
11:21 < geertu> mturquette: of course. So it's append only.
11:21 < mturquette> geertu: but the choice is yours. i'm fine either way.
11:22 < dammsan> geertu: so say you add clocks to upstream
11:22 < dammsan> geertu: and people use the latest DTB with an old kernel
11:22 < dammsan> geertu: then the new clocks should fail to register, but shall the system halt?
11:23 < dammsan> geertu: in my mind it makes sense to keep the system working
11:23 < geertu> mturquette: My main thinking there is to switch over Gen2 (so far there's a single SoC in Gen3 only): do we (try to) keep numbers in sync between different SoCs of the same family? Syncing means we will have gaps in the numbering, but it may make the C code simpler.
11:23 < mturquette> geertu: i don't see why keeping the numbers the same is important
11:24 < mturquette> geertu: you're using preprocessor macros/defines for the numbers, right?
11:24 < mturquette> geertu: the symbols are the only thing that need to be sync'd between Linux kernel and the binding
11:24 < geertu> dammsan: It will print an error message that it can't handle the clock (cfr. the "default" switch case)
11:24 < dammsan> geertu: thats fine if the rest of the system works
11:24 < geertu> mturquette: For the CPG core clock outputs we use #defines.
11:25 < geertu> dammsan: and it will continue
11:25 < dammsan> geertu: wondershon
11:25 < geertu> mturquette: For the module clocks (and future resets) we use hardcoded numbers
11:25 < geertu> as these match the datasheet
11:26 < geertu> The CPG core clock numbers are numbers we come up with ourselves.
11:26 < mturquette> geertu: OK. the only problem there is if you have a NR_CLKS or NR_RESETS value that changes as new entries are added.
11:26 < dammsan> geertu: those hard coded numbers match the documentation 100%, right?
11:26 < geertu> dammsan: yes, they're the old "MSTP numbers"
11:27 < geertu> mturquette: NR_CLOCKS is only in the driver (C code)
11:27 < dammsan> geertu: right, and they are kind of sparsely populated I think
11:27 < mturquette> geertu: sounds good to me.
11:27 < geertu> mturquette: About numbering for different SoCs
11:27 < mturquette> geertu: i prefer a non-sparse list for "fake" numbers, but of course it makes sense to use a sparse list if you are matching the hardware.
11:28 < geertu> E.g. r8a7790 has more CPG core clocks than r8a7791.
11:28 < geertu> But both have QSPI. So R8A7790_CLK_QSPI may be different from R8A7791_CLK_QSPI
11:29 < geertu> Currently I use the CLK_TYPE_GEN2_QSPI clock type (internal to the C driver) to handle that.
11:30 < dammsan> geertu: on gen2 with mssr, will you use a generic compat string, or break out per SoC?
11:30 < mturquette> Hmm, I'll take a look at that.
11:30 < geertu> dammsan: yes, module clocks are sparsely populated. and more may show up or disappear with a revision of the datasheet :-)
11:30 < mturquette> On other implementations I see typically a big list of clocks in the driver which are common. And then various other smaller lists which break out the SoC-specific bits.
11:31 < dammsan> geertu: right. so matching the data sheet directly makes sense
11:31 < geertu> dammsan: per SoC, as the core clock registers are different (mainly about existence vs. non-existence)
11:31 < dammsan> geertu: sounds ggggggggood
11:32 < dammsan> mturquette: this is how to split the common code and soc-specific in C, right?
11:32 < dammsan> or does it affect the DT ABI?
11:32 < mturquette> dammsan: correct. It prevents from having to register two qspi clocks by accident
11:32 < dammsan> makes sense
11:32 < mturquette> dammsan: and thus it does not matter if the DT binding uses the same number for the qspi clocks on different SoCs
11:33 < dammsan> mturquette: so you have one name space per SoC?
11:33 < mturquette> and since you are using per-SoC compat strings, its easy to match on this
11:33 < mturquette> dammsan: right.
11:33 < dammsan> the numbers may or may not be shared
11:33 < geertu> On problem is we don't know in advance which clocks are common and which are SoC-specific (or become SoC-specific once a new derived SoC is released which lacks a hardware block)
11:33 < dammsan> between SoCs
11:33 < mturquette> but, generally you can describe 90% of your clocks in one big array which are shared by all the SoCs in the same family
11:34 < mturquette> and even if a new SoC comes out and changes this, you don't break DT compat, since its just an internal driver change to migrate a clock out of the "common" array into soc-specific arrays
11:34 < mturquette> geertu: ^^^ I think the point above addresses that.
11:34 < dammsan> mturquette: sure, sounds good
11:35 < mturquette> as long as the shuffling happens in the C driver and doesn't break compat, then its fine.
11:35 < mturquette> new kernels with different tables of per-soc clocks should still work with old DTBs
11:35 < geertu> So having the (intermediate) CLK_TYPE_GEN2_* defines is good, as it allows to isolate different SoCs
11:36 < geertu> And the SoC-specific numbers can be contiguous
11:36 < dammsan> mturquette: so you dont care how the DT ABI numbers are allocated?
11:36 < dammsan> as long as they are kept working
11:36 < dammsan> ?
11:37 < geertu> dammsan: Mike said before he prefers a non-sparse list
11:37 < dammsan> geertu: ok thanks, sorry for being slow
11:37 < mturquette> geertu: dammsan: I won't NAK a patch based on the list type. its really your choice.
11:38 < mturquette> geertu: I searched for CLK_TYPE_GEN2 in the v4 series and I don't see it?
11:38 < dammsan> so the current gen2 approach, is it good or bad?
11:38 < dammsan> (i mean the old upstream way with separate CPG and MSTP)
11:39 < dammsan> (the CPG clocks are virtual and non-sparse)
11:39 < geertu> mturquette: I didn't post the Gen2 driver, only the Gen3 one
11:39 < mturquette> geertu: ah, I see. Thanks.
11:39 < geertu> so you want to look for CLK_TYPE_GEN3\
11:40 < geertu> For Gen3 we support a single SoC only
11:40 < geertu> I mainly did the Gen2 version (for r8a7791 only) to validate the approach, as we have more hardware support there
11:41 < geertu> To support SoC-family and SoC-specific tables, I just need to split cpg_mssr_info.core_clks[] into two arrays
11:41 < mturquette> geertu: I see. Thanks.
11:42 < geertu> and call the SoC-family cpg_clk_register() from the SoC-specific .cpg_clk_register() callback
11:42 < mturquette> geertu: using an enum/flag or splitting it per-clock/per-soc tables, either way is fine with me.
11:42 < dammsan> geertu: i think your code looks clean and easy to read
11:42 < mturquette> ack.
11:43 < mturquette> and it looks like the array of clock data follows the hierarchy? that's always nice.
11:43 < mturquette> (at least for cpg)
11:43 < geertu> mturquette: yes, each entry has a parent clock
11:43 < geertu> (div6 clocks with multiple parents are not yet supported)
11:44 < geertu> (but support can be added, of course)
11:44 < dammsan> geertu: it looks like your V4 series is based on the old Gen3 driver?
11:44 < dammsan> clk-rcar-gen3.o
11:45 < dammsan> i mean the code base includes the old driver, but it is not used
11:46 < dammsan> or am i misunderstanding?
11:47 < geertu> dammsan: you mean in renesas-drivers.git?
11:48 < dammsan> i suppose you simply based your tree on the old implementation
11:48 < geertu> yes
11:48 < dammsan> i mean the V4 series
11:48 < dammsan> The [5/5] Makefile hunk gives you away =)
11:49 < geertu> one moment please, there's a delivery here...
11:53 < dammsan> mturquette: so on your side, with the DT binding, when do you think it can end up in -next?
11:53 < dammsan> (we tend to treat it as semi-stable once it hits -next)
11:53 < dammsan> maybe semifreddo is a better word =)
11:55 < mturquette> geertu: are you planning to send a PR?
11:55 < mturquette> dammsan: I have answered your question with a question.
11:59 < dammsan> mturquette: a question to me, or to geertu? =)
11:59 < dammsan> (i can't seem to locate the answer/question in my history)
12:00 < geertu> mturquette: it's just patches 1 and 2 of the series, right?
12:01 < geertu> Is it worth sending a pull request for that?
12:01 < mturquette> geertu: I'm confused. I will finish reviewing all 5 patches later today. I can either merge them directly (if there are no problems) or you can send me a PR with any other outstanding renesas clk patches.
12:02 < mturquette> geertu: but I'm confused by your comment about "just patches 1 and 2"
12:02 < geertu> mturquette: bindings are patches 1 (binding doc) and 2 (clock defines)
12:02 < dammsan> geertu: do you feel the driver bits need more time to stabilize before merging upstream?
12:02 < geertu> mturquette: 3-5 are implementation, which can use some review and need a bit of rework
12:03 < geertu> dammsan: there are FIXMEs and too many debug prints
12:03 < mturquette> geertu: Then lets keep all 5 together. No reason to merge "dead code"
12:03 < dammsan> geertu: ok, no problem
12:03 < geertu> Does that mean we postpone everything to v4.5?
12:04 < geertu> I thought the plan was to get the bindings accepted in v4.4
12:04 < geertu> so we know what we're working against (binding-wise) for v4.5
12:04 < dammsan> that would be very nice yes
12:04 < mturquette> geertu: that's fine, i can merge them if you would like.
12:05 < dammsan> so a separate PR with patch 1 + 2?
12:05 < mturquette> geertu: but once a Linux release is made they considered somewhat stable
12:05 < mturquette> No PR necessary. I was only asking if geertu planned to send a PR already.
12:05 < dammsan> ok sweet
12:06 < geertu> yes, thx
12:06 < dammsan> geertu: so i looked through patch 1 + 2
12:06 < mturquette> (I prefer PRs because I invariably miss something when it is left up to me to pick patches ;-)
12:06 < dammsan> and they look good
12:06 < geertu> Has anyone reviewed include/dt-bindings/clock/r8a7795-cpg-mssr.h?
12:06 < dammsan> geertu: do you need anything from me?
12:06 < geertu> OK, will send a PR later today
12:06 < geertu> for patches 1 and 2
12:07 < geertu> dammsan: Reviewed-by?
12:07 < dammsan> sure, i will go through once more tonight and reply to public space
12:07 < mturquette> geertu: feel free to add my Ack to both patches
12:08 < geertu> The defines are based on Table 8.2a 
12:08 < geertu> ("List of Clocks [R-Car H3]")
12:08 < dammsan> thanks, will check
12:09 < dammsan> will get back to you later tonight JST
12:10 < geertu> mturquette: ok, thx
12:13 < geertu> BTW, the bindings mention r8a7791 too. I guess I better leave that out for now.
12:16 < geertu> Any other clock related topics?
12:16 < mturquette> geertu: None from me.
12:16 < geertu> OK, next topic
12:16 < geertu> 1. Upcoming Gen3 development for the Core group,
12:17 < geertu> shimoda-san/morimoto-san?
12:17 < shimoda> yes, please wait..
12:20 < shimoda> ipmmu, and rcar-dmac is needed by end of Nov.
12:20 < shimoda> dammsan is working for ipmmu now
12:21 < shimoda> who is working for rcar-dmac? but, it is not needed for gen3 binding?
12:21 < geertu> No one is working on rcar-dmac.
12:21 < shimoda> about ipmmu, it is a prototype code
12:21 < dammsan> shimoda: yes, and i have not managed to get the ipmmu working yet
12:22 < geertu> If ipmmu and rcar-dmac are the only items, and ipmmu is being worked on, I guess we should move to Topic 3. rcar-dmac: pick up the patches, fix the DMA engine API and save the world
12:22 < shimoda> geertu: yes, let's move to topic 3
12:23 < geertu> So several patches have been sent by Hamza from Visteon, who is using SCIFA on H2 to talk to Bluetooth
12:24 < geertu> Laurent is the most knowledgeable about the rcar-dmac driber, but he has no time/no mandate to work on it
12:24 < geertu> Several issues can be traced back to core DMA engine API issues
12:25 < geertu> Lars-Peter Clausen has planned to work on DMA engine issues
12:25 < dammsan> geertu: to reproduce these issues, would it make sense to create a loopback SCIF setup for everyone?
12:25 < geertu> None of the above is Gen3-specific though
12:26 < geertu> dammsan: That's one option.
12:26 < dammsan> do we have any better test target?
12:27 < geertu> dammsan: Better would be one SCIF(A) to another SCIF(A)
12:27 < dammsan> (i have much faith in the current state of the SCIF driver, as opposed to other drivers)
12:27 < dammsan> sure
12:27 < geertu> Plain SCIF doesn't work well with "high" (read: medium) speeds
12:27 < dammsan> ideally we would like to test both, right?
12:27 < geertu> Note that Gen3 doesn't have SCIFA, but HSCIF
12:27 < dammsan> ah, right
12:27 < geertu> My breadboard wiring setup doesn't work well for multi-Mbps
12:28 < dammsan> is that on gen2?
12:28 < geertu> If you use loopback, the same spinlock is taken for RX and TX paths
12:28 < geertu> dammsan: yes, koelsch
12:29 < geertu> dammsan: haven't done much with the Salvator-X yet
12:29 < dammsan> loopback = external loopback circuit, or in-uart config?
12:29 < geertu> (except for CPG ;-)
12:29 < dammsan> right, no stress =)
12:29 < geertu> dammsan: I was thinking about external loopback, but SCIF indeed has internal loopback, too
12:29 < geertu> dammsan: no pinctrl nor EXIO connectors needed for internal loopback ;-)
12:30 < dammsan> right, but SCIF only
12:30 < dammsan> or how about HSCIF?
12:30 < geertu> same for HSCIF
12:30 < geertu> (HSCIF is SCIF on steroids)
12:30 < geertu> (SCIFA is SCIF on different steroids)
12:31 < geertu> (SCIFB is SCIFA on more steroids)
12:31 < geertu> What do we do with rcar-dmac?
12:33 < shimoda> How about loopback test of HSCIF?
12:33 < pinchartl> (hello)
12:34 < shimoda> hello!
12:35 < geertu> (Lars-Peter Clausen is already posting DMA engine API patches, cool)
12:36 < geertu> pinchartl: Right on time ;-)
12:36 < pinchartl> give or take an hour and a half, yes. sorry about that
12:37 < geertu> For now I'll add a task to start caring for rcar-dmac
12:37 < pinchartl> regarding rcar-dmac
12:37 < pinchartl> I don't have much time to spend on it
12:37 < pinchartl> but I can find time to review patches
12:37 < pinchartl> including dma engine core patches
12:38 < geertu> good to hear that!
12:38 < pinchartl> I believe a discussion with Lars, Vinod and possibly others would be useful to draft a future for the dma engine API
12:38 < pinchartl> it's growing in an uncontrolled fashion today
12:39 < geertu> Both were at ELCE. Will they be at Kernel Summit?
12:39 < pinchartl> unfortunately not
12:40 < geertu> As we're running (ran) out of time...
12:40 < geertu> 4. Status check for tasks from last meeting - what is remaining?
12:40 < geertu> https://osdr.renesas.com/projects/linux-kernel-development/wiki/Core-todo-list
12:41 < geertu> Not much has happened here
12:41 < geertu> Except for "Propose API to obtain mode pin values in a generic way"
12:41 < geertu> which Magnus wanted to discuss, but horms is not here?
12:42 < dammsan> right
12:42 < dammsan> CMA is not yet enabled I think
12:42 < dammsan> but it can wait a bit longer
12:42 < dammsan> (same as other stuff)
12:42 < dammsan> maybe simon will join next time
12:43 < dammsan> and we can discuss the boot mode API then
12:43 < geertu> ok
12:44 < geertu> Anything else to discuss in the -14 minutes we have left?
12:44 < dammsan> not from my side =)
12:44 < pinchartl> speaking of next time, geertu, could you please CC me when you'll send the next invitation ? I'll then add it to my calendar immediately. I had missed this one
12:45 < geertu> pinchartl: sorry, aren't you on periperi?
12:45 < pinchartl> yes I am
12:45 < pinchartl> but mails on CC have less chance to slip through the cracks
12:46 < pinchartl> I read periperi
12:46 < pinchartl> but I've missed that mail for some reason
12:46 < geertu> oops
12:46 < geertu> Thanks for joining, CU next time!
12:46 < geertu> Bye
12:47 < dammsan> thanks, see you in 2 weeks =)