On Mon, Mar 18, 2019 at 10:40:40PM -0700, Jim Broadus wrote: > On Mon, Mar 18, 2019 at 10:08 AM Charles Keepax > <ckeepax@xxxxxxxxxxxxxxxxxxxxx> wrote: > > On Mon, Mar 11, 2019 at 10:19:19PM -0700, Jim Broadus wrote: > > But looking at this again today it occurred to me are we just > > over complicating things here. What about if just force the remap > > of the IRQ in i2c_device_probe and only update client->irq if we > > get a valid IRQ. Something like the following (note not really > > tested yet). > > > > Hi Charles. Thanks for the response. This patch is definitely more elegant than > what we have now, though behavior may change in some corner cases, for better or > worse. But it still doesn't solve the concerns, as I understood them, that there > are multiple ways that an IRQ can be determined in multiple places. This is > isn't entirely avoidable since there will always be cases where the IRQ is known > ahead of time, but I was looking at common interfaces that could be used in the > probe. > Certainly that was my initial reaction as well, it felt nasty to have some IRQs obtained at device time and some at probe. I am not sure it is really avoidable though (at least not without major refactoring of large parts of the kernel out side I2C). Which leaves us in a situation where really the only thing that can be done is store something from device time and handle that at probe time. Each of your solutions basically handle that store differently, but they are all doing it. Of the options I feel like storing the whole info would have some milage if we expected to want to look back at other parts of it in the future. But for now I would argue that we only need the IRQ and thus the simplest solution is to just store the IRQ as the code did before I broke it :-) > But if this is in line with what Benjamin and Wolfram had in mind, then the > code seems good to me. > Yeah lets see if they have any thoughts. Thanks, Charles