Re: [RFC] ARM: OMAP2+: hwmod: don't touch hwmod if disabled

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



* Dave Gerlach <d-gerlach@xxxxxx> [170725 08:53]:
> On 07/25/2017 10:38 AM, Tony Lindgren wrote:
> > * Dave Gerlach <d-gerlach@xxxxxx> [170725 08:28]:
> >> On 07/25/2017 10:03 AM, Tony Lindgren wrote:
> >>> * Dave Gerlach <d-gerlach@xxxxxx> [170725 07:18]:
> >>>>
> >>>> This patch will introduce all sorts of chaos with PM low power modes in addition
> >>>> to leaving many IPs in undefined power states. What happens on a system when RTC
> >>>> is present and powered but is marked disabled in the DT? Nothing, which means it
> >>>> is just left powered for no reason at all.
> >>>
> >>> What status = "disabled" means is that Linux does not know about the
> >>> device at all. We are not following the standard right now, and that's why I
> >>> introduced status = "incomplete" which means the device driver can probe and
> >>> bail out after idling the device.
> >>
> >> I don't disagree that we are not following standard, but this is the way it's
> >> been done and it's what makes things work.
> > 
> > I've commented on numerous occasions that there should be no reason to
> > set any SoC internal devices with status = "disabled". And I've just checked
> > that the mainline kernel works just fine for PM with no "disabled" set for any
> > device on omap3. This non-standard PM dependency to "disabled" is something
> > that TI seems to have started with the TI kernel trees, and I don't agree that
> > "it's what makes things work".
> 
> So we want every single IP loaded with a driver even if we aren't using it? What
> if we don't intend to use an IP? UART is a perfect example of this, there are
> six UARTs on am335x. If we are only hoping to use uart0 for the console and none
> of the others, why should we mark all six as "Okay" and have drivers loaded and
> the IPs ready to go?

In general yes, it's best to probe it until we have "incomplete". The
devices are there accessible in the SoC.

Alternatively if PM is important and you want to save some
memory, you can also disable the unused devices in the
bootloader and then you can tag them as "disabled" and have
Linux kernel completely ignore them.

> >>>> With this patch in place the only way to ensure suspend will work is to have
> >>>> every single device marked "okay" regardless of whether or not it's in use or
> >>>> present on the system. The original intent of having omap_hwmod touch hwmods
> >>>> regardless of their dt state is to put the system into a defined state at boot
> >>>> time for PM. HWMODs do not have a fixed default state, every hwmod has a default
> >>>> state which can be on or off at boot depending on the platform, and without this
> >>>> in place, some modules can easily be left on by accident which will entirely
> >>>> prevent suspend on platforms like am335x or am437x if the IP sits in the
> >>>> peripheral power domain.
> >>>
> >>> Default is status = "okay" and there really should be no reason to mark SoC
> >>> devices as "disabled" unless they really are not accessible as on HS devices.
> >>> We do have PM working for many omap3 devices without having to use status =
> >>> "disabled" at all except for some HS devices. Of course this needs to be
> >>> verified though :)
> >>>
> >>> But really, "disabled" is not the problem for PM, the limitations we have
> >>> for PM issues are missing runtime PM implementations in device drivers,
> >>> such as for OHCI/EHCI.
> >>
> >> What about when no driver is present and runtime PM does not apply at all? hwmod
> >> handles that case and shuts the IP off at boot regardless of what state it's in.
> > 
> > Then just the usual, just add a driver :) Please let me know of any where
> > the mainline kernel relies on "disabled".
> > 
> >> Example I can see already, on am335x, uart5 is left active after u-boot runs. We
> >> don't use uart5 by default in the kernel on am335x-evm so it's marked disabled
> >> in the DT, with this patch applied suspends fails with "PM: Could not transition
> >> all powerdomains to target state" message because uart5 (and other IPs for that
> >> matter) block transition of the PER domain and you don't enter suspend. How
> >> would we handle this and other IPs with similar issues?
> > 
> > So why are you marking it with "disabled"? Just strip out all the bogus
> > "disabled" and see what happens.
> 
> We describe all modules in the top level SoC dtsi and mark many as disabled and
> then mark them enabled in the board files as appropriate, so that's just the
> default state of most of the modules we use unless explicitly changed in the
> board file.

That is something that TI started doing, and it's clearly not the policy
for the mainline kernel. And that is a wrong thing to do and also bloats
up the dts files with tons of pointless strings :p

> >> Again, I'm not saying we are doing things right by touching nodes with disabled
> >> set, but that's how it works today, and taking this patch alone will break a lot
> >> of things, there's a lot of other things that will need to be addressed first.
> > 
> > Well then we need to first fix the issues naturally. But I already proved
> > that things work for my PM test case. And I suspect the bogus "disabled"
> > can be stripped out for all the omap variants. So please do let me know of
> > issues with drivers in the mainline kernel that break so we can fix them
> > properly.
> 
> So again, my concern isn't with driver issues, it's if we don't want to use an
> IP and don't want a driver. Or is having a driver loaded for every IP regardless
> of whether or not you plan to use it what we want?

We do already reset/idle/power down devices even if there is
no driver loaded so that would not change.

But for device drivers, yeah I think having drivers probe and idle
on "incomplete" is the best way to go in the long run to avoid
duplicating device driver features at the bus level.

Anyways, we should be able to already just remove the bogus
use of "disabled" except if an SoC internal device is truly not
accessible and the kernel can't do anything about it.

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



[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux