On Wed, Mar 25, 2020 at 2:47 PM Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote: > > Saravana Kannan <saravanak@xxxxxxxxxx> writes: > > On Tue, Mar 24, 2020 at 11:34 AM Saravana Kannan <saravanak@xxxxxxxxxx> wrote: > > I took a closer look. So two different drivers [1] [2] are saying they > > know how to handle "arm,vexpress-sysreg" and are expecting to run at > > the same time. That seems a bit unusual to me. I wonder if this is a > > violation of the device-driver model because this expectation would > > never be allowed if these device drivers were actual drivers > > registered with driver-core. But that's a discussion for another time. > > > > To fix this issue you are facing, this patch should work: > > https://lore.kernel.org/lkml/20200324195302.203115-1-saravanak@xxxxxxxxxx/T/#u > > Sorry, that's not a fix. That's a crude hack. If device nodes are being handled by drivers without binding a driver to struct devices, then not setting OF_POPULATED is wrong. So the original patch sets it. There are also very valid reasons for allowing OF_POPULATED to be cleared by a driver as discussed here [1]. The approach of the original patch (setting the flag and letting the driver sometimes clear it) is also followed by many other frameworks like irq, clk, i2c, etc. Even ingenic-timer.c already does it for the exact same reason. So, why is the vexpress fix a crude hack? > As this is also causing trouble on tegra30-cardhu-a04 the only sane > solution is to revert it and start over with a proper solution for the > vexpress problem and a root cause analysis for the tegra. If someone can tell me which of the timer drivers are relevant for tegra30-cardhu-a04, I can help fix it. If you want to revert the original patch first before waiting for a tegra fix, that's okay by me. However, for vexpress, what do you propose I do? The driver itself is doing weird stuff with two drivers handling the exact same device. I can't just go edit the DT files because technically I don't know their hardware. Looks to me like they should have a separate and proper device for the timer and not have two drivers handle the same device. If you don't like my vexpress fix, then don't take it but ask the vexpress maintainer to fix their DT and driver maybe? But that might break the kernel compatibility with existing DT binaries on devices (yes, vexpress seems like a virtual platform, so updating DT blobs might not be hard). My vexpress fix doesn't break backwards compatibility. So, can you please accept my vexpress fix or tell us what you think is a "proper solution"? -Saravana
|