wiki: add IO chatlog from 20200702

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

1 files changed, 123 insertions, 0 deletions

diff --git a/wiki/Chat_log/20200702-io-chatlog b/wiki/Chat_log/20200702-io-chatlog
new file mode 100644
index 0000000..8f7beaf
--- /dev/null
+++ b/wiki/Chat_log/20200702-io-chatlog
@@ -0,0 +1,123 @@
+09:03 < wsa> welcome everyone to todays meeting
+09:03 < wsa> here are the collected status updates
+09:03 < wsa> Status updates
+09:03 < wsa> ==============
+09:03 < wsa> A - what have I done since last time
+09:03 < wsa> ------------------------------------
+09:03 < wsa> Geert
+09:03 < wsa> : implemented explicit internal delay setting from DT for ravb, DT binding doc conversion to json-schema: ravb
+09:03 < wsa> Shimoda-san
+09:03 < wsa> : converted XHCI binding docs to YAML, fixed undefined temperature if negative for Thermal, and sent various series to fix mmc_suspend() with PSCI
+09:03 < wsa> Ulrich
+09:03 < wsa> : sent new series to handle WDT handover from bootloader which got merged, sent new series implementing atomic transfers for IIC
+09:03 < wsa> Wolfram
+09:03 < wsa> : removed final bits of i2c_new_device API and removed the old API so API conversion is now finished, reviewed slave implementation for SMBusHostNotify, and added the support for i2c-rcar, fixed various corner cases in the slave implemetation of i2c-rcar, wrote a new slave-backend to test rare I2C/SMBus features which usually need specific devices, tried to get an answer to the RPM
+09:03 < wsa>  refcounting issue, reviewed Uli's atomic transfer patch for IIC, minor treewide cleanup patches sent out while working on all of the above
+09:03 < wsa> B - what I want to do until next time
+09:03 < wsa> -------------------------------------
+09:03 < wsa> Geert
+09:03 < wsa> : wants to implement generic bindings for Ethernet MAC explicit internal delay setting
+09:03 < wsa> Niklas
+09:03 < wsa> : wants to eet up with Wolfram and flash Gen3 boards, and do the yearly test with SDIO-WIFI on Koelsch
+09:03 < wsa> Shimoda-san
+09:03 < wsa> : wants to keep working on mmc_suspend() with PSCI, discuss with Cogent about RAVB driver patch, convert usb-xhci, rcar-pci and sdhi DT docs to json-schema
+09:03 < wsa> Ulrich
+09:03 < wsa> : wants to sent new series implementing atomic transfers for IIC and I2C
+09:03 < wsa> Wolfram
+09:03 < wsa> : wants to meet with Niklas and fix Gen3 boards together, have two weeks of holidays, add SMBusAlert and I2C_M_RECV_LEN to both, the slave-testunit and the i2c-rcar driver, resend series 'fix stalled SCC' and 'implement manual calibration' for SDHI, review Shimoda-san's DMA unmapping series for SDHI
+09:03 < wsa> C - problems I currently have
+09:03 < wsa> -----------------------------
+09:03 < wsa> Shimoda-san
+09:03 < wsa> : wonders whether the mmc_suspend() improvement is really needed because we cannot avoid accidentally power off of eMMC around system suspend anyway
+09:03 < wsa> Wolfram
+09:03 < wsa> : asks how to ensure that atomic xfers on IIC have their clock enabled that late, and couldn't use Gen3 boards for a while so he had to switch to work which could be tested on Gen2
+09:04 < wsa> shimoda: I assume with the RAVB patch you mentioned is about the "stop queues on timeout" issue?
+09:05 < wsa> geertu: you mentioned that you'd apply Uli's RWDT clock patches but I don't see them in -next yet
+09:05 < shimoda> wsa: yes
+09:05 < geertu> wsa: They're in clk-renesas. Haven't sent a pull request to mturquette yet
+09:06 < wsa> geertu: ah, they don't go directly into next from your branch?
+09:06  * geertu doublechecks
+09:06 < geertu> wsa: No they don't. Same with sh-pfc.
+09:06 < wsa> shimoda: ok, thanks. will update that information
+09:06 < shimoda> wsa: thanks!
+09:07 < wsa> about the RPM get_sync ref-counting issue:
+09:07 < geertu> shimoda: I assume s/Cogent/Sergei/?
+09:07 < shimoda> geertu: yes
+09:08 < wsa> I confirmed that always increasing the ref-count is intended behaviour. I have not confirmed yet, that it may be a better fix to simply remove the error checking instead of adding a put_noidle everywhere
+09:08 < wsa> but from the last discussion, I thought for R-Car this is true?
+09:09 < geertu> wsa: Yes, Rafael confirmed not checking is fine for us
+09:09 < wsa> ok, nice to know
+09:10 < wsa> so, i will ask people sending in such patches if they checked that removing the check may be the better solution
+09:10 < geertu> https://lore.kernel.org/r/CAJZ5v0i87NGcy9+kxubScdPDyByr8ypQWcGgBFn+V-wDd69BHQ@mail.gmail.com
+09:11 < geertu> "It is the right thing to do in the majority of cases."
+09:11 < geertu> Now, there's still the bunch of "fixers" that will try to add error paths (and pm_runtime_put_*() calls) everywhere
+09:12 < wsa> maybe we should add another function annotation: __must_not_check ;)
+09:13 < wsa> at least, I now have a proper answer for them
+09:13 < geertu> ;-)
+09:14 < wsa> so, I have another topic but wanted to ask you guys first if there are questions regarding the status updates or in general?
+09:14 < neg> wsa: is not  __must_not_check equal to returning void :-)
+09:15 < wsa> shimoda: I will seriously try to review the DMA unbalance patches before my holidays
+09:15 < geertu> wsa: For atomic transfers, it's not just the clock, but also the power domain
+09:15 < wsa> neg: you got it!
+09:16 < wsa> geertu: yes, this was my remaining question
+09:17 < shimoda> wsa: thanks!
+09:17 < geertu> It seems some modules on R-Car Gen2 are "forgiving" when accessing them with the module clock disabled (i2c, rwdt)
+09:17 < wsa> atomic_xfer happen very late, so clocks and power domains may be off already. and we can't use the calls to enable them anymore, right?
+09:17 < geertu> Yes, pm_runtime_get_sync() may sleep
+09:18 < geertu> We might set GENPD_FLAG_IRQ_SAFE for our domains
+09:19 < wsa> but on what condition?
+09:19  * kbingham discovers http://sweng.the-davies.net/Home/rustys-api-design-manifesto :-)
+09:19 < geertu> kbingham: Yeah, pm_runtime_get_sync() is at -1 ;-)
+09:19 < kbingham> geertu, That's the thread I found it from ;-)
+09:20 < pinchartl> did Rafael provide any answer regarding whether pm_runtime_get_sync() should be fixed ?
+09:20 < wsa> I thought it was -4
+09:20 < geertu> there's also dev.power.irq_safe
+09:21 < wsa> sometimes -7 :>
+09:21 < geertu> pinchartl: Not to us.  But according to the commit message for the initial clock fix, he doesn't intend to fix it.
+09:22 < wsa> pinchartl: AFAIU it was expected to be the common case to NOT check the retval
+09:22 < geertu> Correction: it's not in the commit message
+09:22 < pinchartl> I don't see why we should fix it in drivers if he can't be bothered to fix it in runtime PM
+09:22 < wsa> and it was intended that simple get/put-pairs without error checking simply should work
+09:23 < geertu> https://lore.kernel.org/linux-pm/CAJZ5v0j+bsHaQcxK41yph8eRpMZ3DoerqA7uwS2B8De41Jwi7Q@mail.gmail.com/
+09:23 < geertu> "I'd rather update the buggy callers than the ones that are OK.
+09:23 < geertu> "
+09:23 < geertu> But of course the "fixers" take (only) care of the latter
+09:23 < pinchartl> wsa: amazing API design...
+09:23 < wsa> pinchartl: ack
+09:24 < pinchartl> ok, so I'll reject all patches that add error checks, easy
+09:24 < wsa> my impression was that Rafael kind of regrets it today, but looks like "too late" 
+09:24 < geertu> it starts with a function that cannot fail.
+09:24 < geertu> Then someone thinks it should not return void, but return an errorn code.
+09:24 < geertu> Then all callers "must" be fixed
+09:25 < geertu> wsa: indeed
+09:25 < geertu> So the safe way is to always return an error code. At the expense of people checking for an error that cannot happen.
+09:26 < geertu> The real bad thing here is that pm_runtime_get_sync() doesn't undo the refcounting on "failure"
+09:26 < wsa> yep
+09:27 < wsa> that is so counter-intuitive
+09:28 < kbingham> I'm sure it could be fixed with some coccinelle and an intern ;-)
+09:28 < wsa> I doubt coccinelle can find the few cases where an error code is useful
+09:29 < wsa> geertu: about the atomic transfer issue
+09:29 < wsa> does it mean PMIC drivers need to set the irq_safe flag?
+09:30 < geertu> That flag would be for the PM domain
+09:31 < geertu> oh, you mean dev.power.irq_safe
+09:31 < wsa> whatever works, actually. but it should be the PMIC driver requesting the change, or?
+09:31 < wsa> to save the DT description
+09:32 < geertu> dev.power.irq_safe means "it is safe to run the ->runtime_suspend(), ->runtime_resume()
+09:32 < geertu> and ->runtime_idle() callbacks for the given device in atomic context with
+09:32 < geertu> interrupts disabled"
+09:32 < geertu> but the doc for the flag says:
+09:33 < geertu> "indicates that the callbacks will be invoked with the spinlock held and interrupts disabled", which is something different
+09:33 < geertu> "is safe if" vs. "will"
+09:34 < geertu> So if we mark all underlying infrastructure (PM Domain, i2c driver, pmic driver) irq_safe, we still have the might_sleep() call in pm_runtime_get_sync()
+09:35 < geertu> My gut feeling says the only safe way to support this, is for the PMIC driver (which knows it's can be used to restart the system), to tell the i2c core that the i2c driver must stay awake for that.
+09:35 < geertu> s/it's/it/
+09:36 < wsa> can't we the PMIC driver just disable RPM for itself?
+09:36 < wsa> and then this should propagate to all parents?
+09:36 < geertu> That might work actually.
+09:36 < geertu> Alternaively, is there something like an early restart notifier?
+09:37 < geertu> Then the PMIC could wake up the i2c driver in that notifier, which would avoid the need for keeping the i2c driver awake all the time
+09:38 < wsa> uli_: can you check these two options?
+09:39 < wsa> other than that, last call for questions / comments
+09:40 < uli_> i will check it out
+09:40 < wsa> uli_: thanks!
+09:40 < wsa> then I guess we can switch to the core meeting