09:03 < wsa> welcome to today's IO meeting 09:03 < wsa> here are the status updates: 09:03 < wsa> A - what have I done since last time 09:03 < wsa> ------------------------------------ 09:03 < wsa> Geert 09:03 < wsa> : corrected and tested SCIF5 DMA on R-Car E3, and upported MSIOF patches for 09:03 < wsa> reset handling and bits-per-word restrictions 09:03 < wsa> Kaneko-san 09:03 < wsa> : posted patch to update thermal calculation for E3 09:03 < wsa> Marek 09:03 < wsa> : sent patches for AHCI and NVME PCI to respect the 32bit limitation of the 09:03 < wsa> R-Car PCIe controller 09:03 < wsa> Shimoda-san 09:03 < wsa> : fixed a memory leak in the R-Car Gen2 PHY driver, discussed with BSP team 09:03 < wsa> a HS400 problem and found a patch for a similar issue 09:03 < wsa> Ulrich 09:03 < wsa> : 09:03 < wsa> Wolfram 09:03 < wsa> : sent out another RFC implementing atomic I2C transfers, sent out a series 09:03 < wsa> improving concurrency and one improving DMA robustness for i2c-rcar, fixed 09:03 < wsa> SDHI issue regarding Block Count register, helped fixing an I2C core issue 09:03 < wsa> about reusing irqs, reviewed I2C, thermal, MSIOF, PMIC related patches, did 09:03 < wsa> minor periject work 09:03 < wsa> B - what I want to do until next time 09:03 < wsa> ------------------------------------- 09:03 < wsa> Geert 09:03 < wsa> : wants to convert MSIOF to use GPIO descriptors for CS and to make use of 09:03 < wsa> readl_poll_timeout() 09:03 < wsa> Kaneko-san 09:03 < wsa> : wants to post patch to update thermal calculation for H3, M3-W, M3-N 09:03 < wsa> Niklas 09:03 < wsa> : wants to investigate MMC PM imbalance issue on APE6EVM 09:03 < wsa> Shimoda-san 09:03 < wsa> : wants to wants to work on PWM suspend/resume once his atomic API patches 09:03 < wsa> are merged, and to continue to improve phy-rcar-gen3-usb2 driver to enable 09:03 < wsa> each source independency instead of all sources 09:03 < wsa> Simon 09:03 < wsa> : wants to asses RAVB patches in BSP v3.9.2, measure MMC performance across 09:03 < wsa> various Gen3 boards 09:03 < wsa> Wolfram 09:03 < wsa> : wants to keep up with the atomic I2C transfer topic, handle 'arbitration lost' 09:03 < wsa> and 'NAK' better with the IIC core, investigate SDR104 regression with SDIO 09:03 < wsa> C - problems I currently have 09:03 < wsa> ----------------------------- 09:03 < wsa> Geert 09:03 < wsa> : detected a QSPI FLASH regression on Koelsch 09:03 < wsa> Wolfram 09:03 < wsa> : needs attention of someone with expertise to comment on the use of 'in_atomic()' 09:03 < wsa> in his series about atomic I2C transfers 09:04 < wsa> uli__: so, is the D3 phy-mode patch one of those you tested? 09:05 < wsa> ah, now i see the mail 09:05 < wsa> so, it is, thanks 09:05 < uli__> yup 09:07 < wsa> horms: what about the RAVB upporting? do you have time for them or are you super busy and wouldn't mind someone else working on them? 09:08 < wsa> shimoda: a similar question to you. There is a PWM patch to be upported fixing an issue found by lockdep. Do you have time for that or would you actually be happy if this was delegated to someone interested? 09:08 < jmondi> 'morning, sorry, I'm a bit late 09:09 < horms> wsa: I am a bit busy. My main problem is that I don't know how to test them and they seem likely to have negative performance impatct. 09:09 < shimoda> wsa: please wait a little, I'd like to check the BSP patch 09:10 < wsa> shimoda: https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=70b4a22f220fef943c45a1122fa02d6faa42d2fe 09:11 < wsa> horms: one of them references an errata, wouldn't that do? 09:12 < wsa> and maybe we could request more information about the other one then? 09:13 < wsa> neg: did the cable arrive? 09:13 < neg> wsa: Not since last night when I wrote the status update ;-P 09:13 < wsa> ehrm, yes :) 09:14 < neg> wsa: If it's not here tomorrow I will buy one over the counter to get this thing rolling 09:14 < wsa> neg: yes, please 09:14 < shimoda> wsa: OK. I'll try to upport it. of course, reproduce this issue on mainline first :) 09:14 < horms> wsa: the errata patch has a fix which involves not supporting 1GB/s on E3. 09:14 < wsa> shimoda: thanks! 09:14 < horms> And yet that works fine in my environment. 09:15 < horms> But I do take your point. 09:15 < horms> Perhaps the way forwards is to a) test (probably they work) b) post the patches 09:15 < horms> I just feel that these patches apply a sledgehammer to crack an egg 09:15 < horms> But I don't have an alternate solution 09:17 < wsa> horms: right, the patch description say "It communication at 1Gbps may fail" ... "may" 09:17 < horms> Yes, this is my red flag 09:17 < horms> In my limited testing I have never seen it fail 09:17 < wsa> horms: probably this would need communication with the HW guys then? 09:17 < horms> Yes, maybe that is a good idea 09:17 < wsa> if they can think of a better solution? 09:18 < horms> The other one I should test 09:18 < horms> and see if there is a performance impact 09:18 < wsa> Marex: so, was there anything more needed for D3 CAN? 09:18 < horms> if not then I'm not so worried about it 09:18 < wsa> horms: please, do 09:18 < horms> but even in that case I feel that upstream will require some sort of test case 09:19 < horms> How about this 09:19 < horms> 1) I will test the patches 09:19 < horms> 2) I will send some feedback to the periperi ML 09:19 < horms> 3) Shimoda-san or Morimoto-san can pass some questions on to Renesas 09:20 < wsa> sounds good to me 09:20 < wsa> thanks! 09:21 < horms> Likewise, thanks! 09:21 < Marex> wsa: DT enablement ? 09:21 < wsa> Marex: driver or board? 09:21 < morimoto> horms: for 3) anytime is OK for me. But I'm not familiar with it, please add "background, question" etc ;) 09:21 < Marex> wsa: board 09:22 < horms> morimoto: thanks. I will include background information 09:22 < morimoto> horms: thanks 09:22 < Marex> wsa: Draak has both CANs on pins, it just needs to be enabled and tested 09:23 < wsa> I recall Uli did some CAN enablement patches, I was wondering... 09:24 < Marex> wsa: I didn't see any for Draak 09:24 < geertu> wsa: : They were just .dtsi additions based on the datasheet, AFAIK 09:24 < wsa> will check 09:24 < Marex> yep, only the DTSi and documentation 09:24 < Marex> s/documentation/dt bindings/ 09:26 < wsa> right 09:26 < wsa> the board was missed :( 09:26 < wsa> OK, thanks for doing it, Marek 09:27 < Marex> wsa: I need to test it when I have access to Draak tho 09:28 < Marex> wsa: damm -san said he might install a cable between those two CANs on Draak at least, so I can play around with it remotely 09:28 < wsa> OK, nice 09:28 < wsa> any questions from your side? 09:29 < Marex> wsa: PCI 32bit limitation , will we have to patch each and every driver to not rewrite the DMA masks ? 09:29 < Marex> wsa: I think so 09:30 < wsa> I am not deep enough into that topic, it doesn't sound like it would scale well, though 09:31 < Marex> wsa: eventually the drivers shouldn't change the DMA masks at all 09:31 < Marex> unless there's a good reason, in which case they need to respect the bus DMA mask too 09:31 < wsa> that sounds way better to me 09:32 < Marex> wsa: it still means patching a whole lot of drivers 09:32 < geertu> Yah, DMA masks are not a property of the driver, but of the device and bus 09:32 < Marex> I sent out an RFC for AHCI and NVME drivers, which I tested on Gen3 and Gen2 with a PCIe card and those start working with the change 09:33 < Marex> so did xhci controller, but I didn't send a patch yet, I want some feedback on those two RFCs first 09:33 < Marex> on Gen2 , I still have trouble with NVMe, I am not sure why though ... and it seems unrelated to this problem 09:33 < geertu> Marex: So NVMe is working now? 09:33 < Marex> geertu: on Gen3 09:34 < Marex> geertu: on Gen2 I get timeouts , but see above 09:34 < Marex> geertu: I suspect this has to do with the NVMe being mostly tested on 64bit systems 09:35 < geertu> Marex: Probably. It's not a good performance improvement investion on systems limited to 32-bit DMA anyway ;-) 09:36 < wsa> so, shall we move to discussing Magnus' request? 09:36 < wsa> and my own for that matter? :) 09:36 * geertu assumes his NVMe does DMA to all 36-bit of RAM present ;-) 09:36 * morimoto focus to Renesas inside work from now 09:37 < Marex> geertu: probably all 64bit 09:38 < wsa> so, some comments have been made by mail 09:38 < wsa> About the original proposal from damm: 09:38 < geertu> damm: Alive? 09:39 < damm> yep 09:39 < wsa> I agree with pinchartl that review doesn't always result in a rev-by tag 09:39 < geertu> True 09:39 < wsa> if I have review comments not addressed yet, I don't give the tag 09:40 < geertu> I always assume a new version will appear, where I can provide mine, but probably I'm too optimistic... 09:40 < pinchartl> I sometimes end a review e-mail with "and if you address all these, Reviewed-by: ..." but that's only when the patch is mostly ready and I trust that the next round will be correct 09:40 < pinchartl> if deep rework is needed I don't do that 09:41 < wsa> pinchartl: ack 09:41 < pinchartl> sometimes the result of a review is also the realization that a patch isn't needed, in which case no new version appears 09:41 < pinchartl> there's still value there 09:42 < geertu> The list I provide to Magnus is auto-generated, so it only includes actual R-b's given. 09:42 < wsa> would it be an option to include 4) "reviewed patches still pending" or something alike? 09:42 < geertu> Sure! 09:42 < geertu> Reviewed-but-not-approved-by ;-) 09:43 < pinchartl> it's more work to split the two for me, so I'd like to know if that's actually needed 09:43 < pinchartl> same for the tested patches, if the test fails I don't give a Tested-by tag :-) 09:44 < geertu> Everything seems to be success-driven, like social media 09:44 < pinchartl> damm: was your request about splitting the list of patches in the submitted, reviewed and tested catagories, or do you need to account patches that carry a tag separately ? 09:44 < geertu> "bad" outcomes don't end up in the report 09:44 < neg> Test-failed-by: :-) 09:44 < pinchartl> in the latter case, can't we just harvest Linus' tree to extract tags automatically without needing us to account those patches manually ? 09:45 < pinchartl> it won't be linked to a particular reporting period though 09:45 < geertu> Yep, add a 2-3 month delay 09:45 < geertu> Harvesting from patchwork would have less latency 09:45 < pinchartl> but as I explained in the e-mails, a metric based on tags given out is quite useless as most of the value is attached to tags that are not given 09:45 < damm> pinchartl: splitting patches in categories would be good from my side 09:45 < jmondi> that might fail to capture reviewes that do not end up in a tag 09:46 < pinchartl> damm: then I'm fine with that, that's what I do already :-) 09:46 < damm> =) 09:48 < wsa> well, if it is just seperating reviews from tests done (without the need of a tag), I can easily do that 09:48 < wsa> and will happily do so 09:49 < wsa> consensus on that one? damm-san? 09:49 * geertu has already copied-and-modified his list-patches-reviewed-by-me script 09:50 < damm> sure 09:50 < damm> thanks guys 09:50 < wsa> cool 09:51 < wsa> so, about my request on adding desc to reviews / tests... 09:51 < wsa> I am pretty much with pinchartl here again when it comes to seperation between ack and rev. but this is not the main point for me. 09:52 < wsa> rev by only is fine enough for me, but i'd like to know what kind of review 09:52 < jmondi> wsa: I have missed if you're suggesting such a lenghty description in patch reviews or for the bi-weekly report 09:52 < wsa> nope 09:53 < wsa> just some text around the rev-by tag like "i verified against the errata" or "not familiar with PCI internals, but checked the API" or "did formal check" 09:54 < wsa> jmondi: do you get the picture? 09:54 < jmondi> I see, in the patch review email then, not just as a note to report during our meetings, right? 09:54 < pinchartl> wsa: how about not making that a strict requirement to start with, only adding such text when there's value to it 09:54 < pinchartl> (especially when the review is partial only) 09:54 < wsa> pinchartl: I'd like that 09:54 < pinchartl> and the other question was what you'll do with the information 09:55 < pinchartl> as there's no standard it then include it in the commit message 09:55 < pinchartl> s/it then/to then/ 09:55 < wsa> What I'll do with it? As a maintainer, it gives me an idea what I still need to review 09:56 < wsa> Or if a patch has enough review / test that I can send it back to stable or give it another cycle in -next 09:56 < pinchartl> ok, it gives you more information to know how much you can trust the review 09:56 < pinchartl> that's useful 09:57 -!- Marex [~Marex@195.140.253.167] has quit Ping timeout: 259 seconds 09:57 < wsa> It doesn't need to be a whole lot of text; I think it can be covered in one line mostly 09:59 < marex-cloud> Hmmm, something went wrong with my primary irc system 10:00 < wsa> I think it will increase quality because it is clear what is reviewed. There are no assumptions about the review which could be wrong 10:00 < neg> marex-cloud *switchs to the backup troll generator console* ;-) 10:00 < wsa> This is an overall experience, also coming from I2C maintenance 10:01 < wsa> but it seems there are no further comments 10:01 < geertu> I agree it's good to have such information, and thus provide it, if it makes sense. 10:02 < wsa> I totally agree it can be skipped for trivial patches 10:02 -!- Marex [~Marex@195.140.253.167] has joined #periperi 10:02 < wsa> ok, so, let's please do that from now on 10:03 < wsa> and with that we can switch to the Core meeting? 10:03 < geertu> Sounds good to me!