* Dave Gerlach <d-gerlach@xxxxxx> [170814 12:53]: > On 08/14/2017 02:08 PM, Tony Lindgren wrote: > > Can you please just check what happens on am335x with USB, CPSW > > and DMA with mainline kernel if you just remove status = "disabled" > > for them? If they are not idled properly without "disabled" that's > > where the bug is. > > They are idled properly without status disabled. They are also idled properly > with status disabled with mainline: > > With status = "okay": CM_PER_USB0_CLKCTRL | 0x44E0001C | 0x00070000 > > With status = "disabled": CM_PER_USB0_CLKCTRL | 0x44E0001C | 0x00070000 OK that's good to hear, I was worried we have some quirks I don't know about where that's not the case :) > But if I apply this patch: > > With status = "okay": CM_PER_USB0_CLKCTRL | 0x44E0001C | 0x00070000 > This is idled, but what if we do not intend to use USB? Do we just want to mark > it OK and make sure no code is present so no driver gets loaded? Yeah that's what we currently do on omap3 for example. EHCI module needs to be currently blacklisted as it does not support runtime PM :( That's where the status = "fail-incomplete" can be used to set devices as incomplete on per board basis in the dts files with something like status = "fail-incomplete". We do have an ack from Rob for the patch adding that a while back, we just need a use case to go with it. Then the related device driver can easily check for that with of_device_is_incomplete() and bail out early. This can also be used for SoC internal devices that are not functional, or when a board has not wired up some SoC internal device. Then status = "disabled" should be reserved for something that kernel can't touch at all when set until it's status changes. It's ePAPR definition says "not presently operational, but it might become operational", so that's way too generic for us to know what we should do with the device. The device can possibly cause an oops when trying to access it, like RNG on some HS omap4 for example depending on the PPA. So cases like that truly justify "disabled". > With status = "disabled": CM_PER_USB0_CLKCTRL | 0x44E0001C | 0x00000002 > This is active which we DO not want, because we did not look at the node at all, > meaning omap_hwmod could not get the IP base address needed to write to the > sysconfig register and did not attempt to idle it. The module is still active > because it could not program the sysconfig register because omap_hwmod doesn't > even know that it exists. Well the simple solution there is to just remove status = "disabled". If you need some driver not to probe, you can usually have the driver probe bail out with missing GPIO pins, regulators, clocks, DMA channels, pins or something until we can use "fail-incomplete". > Again, I am not saying touching status disabled nodes is the right way to do it, > just that currently we do for the reasons described above and if we stop > touching the nodes marked disabled we need a new solution. Well I see the solution being the following steps: 1. Fix up the few problem drivers to be able to bail out if it's essential resources are missing 2. Remove use of status = "disabled" unless the SoC internal device is truly not accessible for the kernel 3. Start using "fail-incomplete" so drivers know to bail out early in a generic way if set Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
