Hi, On Wed, Jul 8, 2020 at 4:40 PM Brian Norris <briannorris@xxxxxxxxxxxx> wrote: > > On Wed, Jul 8, 2020 at 4:14 PM Doug Anderson <dianders@xxxxxxxxxxxx> wrote: > > On Wed, Jul 8, 2020 at 4:03 PM Brian Norris <briannorris@xxxxxxxxxxxx> wrote: > > > If I'm reading correctly, you're removing the only remaining use of > > > 'per_ce_irq'. Should we kill the field entirely? > > > > Ah, you are indeed correct! I hadn't noticed that. Unless I hear > > otherwise, I'll send a v2 tomorrow that removes the field entirely. > > A healthy middle ground might put that in a patch 2, so it's easily > dropped if desired. *shrug* Sure, sounds like a plan. > > > Or perhaps we should > > > leave some kind of WARN_ON() (BUG_ON()?) if this function is called > > > erroneously with per_ce_irq==true? But I suppose this driver is full > > > of landmines if the CE API is used incorrectly. > > > > Yeah, I originally had a WARN_ON() here and then took it out because > > it seemed like extra overhead and, as you said, someone writing the > > code has to know how the API works already I think. ...but I'll add > > it back in if people want. > > I believe WARN_ON() and friends have a built-in unlikely(), so it > shouldn't have much overhead. But I don't really mind either way. Since you don't feel strongly, leaving it off. > > > Do you need to clear this map if the interface goes down or if there's > > > a firmware crash? Right now, I don't think there's a guarantee that > > > we'll run through a NAPI poll in those cases, which is the only place > > > you clear the map, and if the hardware/firmware has been reset, the > > > state map is probably not valid. > > > > Seems like a good idea. Is the right place at the start of > > ath10k_snoc_hif_start()? > > Either there or in .power_down()/.power_up(). I think either would be > equally correct, but I'm not entirely sure if the semantic difference > is meaningful for this. Will choose ath10k_snoc_hif_start() since it's where napi_enable() is and ath10k_snoc_irq_enable() are and those are related. -Doug
