On Fri, 5 Aug 2022 at 23:39, Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> wrote: > > On 05/08/2022 19:00, Sudeep Holla wrote: > > On Fri, Aug 05, 2022 at 04:12:42PM +0200, Ulf Hansson wrote: > >> On Thu, 28 Jul 2022 at 10:40, Sudeep Holla <sudeep.holla@xxxxxxx> wrote: > >>> > >>> On Wed, Jul 27, 2022 at 04:24:22PM +0300, Dmitry Baryshkov wrote: > >>>> On Wed, 27 Jul 2022 at 14:14, Sudeep Holla <sudeep.holla@xxxxxxx> wrote: > >>>>> > >>>>> On Wed, Jul 27, 2022 at 12:09:27PM +0300, Dmitry Baryshkov wrote: > >>>>>> - Allow DTS forcing the PSCI power domains even if OSI enablement fails? > >>>>> > >>>>> Meaning DTS flag for this ? If OSI enable fails, why would you want to > >>>>> still proceed. It is non-compliant and must be fixed if the firmware > >>>>> supports OSI and expects OSPM to use the same. > >>>> > >>>> I'm not sure at this moment. PSCI firmware reports that OSI mode is > >>>> supported, but then when psci_pd_try_set_osi_mode() tries to switch > >>>> into OSI mode, it gets NOT_SUPPORTED. > >>> > >>> Yikes, fix the damn broken firmware. That is utter non-sense. I don't > >>> understand why would the firmware authors enable some feature before it > >>> is ready. > >> > >> I certainly agree that the FW is broken and should really have been > >> fixed, but that seems unlikely to happen when moving forward. > >> > >> On the other hand, it's quite common that we try to add workarounds at > >> the Linux side to fix FW issues. Of course, it depends on what kind of > >> hacks it means for us to carry. > >> > >> In this particular case, I am of the opinion that it looks like the > >> "hack" may be worth it. Unless I have underestimated the problem, it > >> seems like a new DT property/flag and a simple if-clause in > >> psci_pd_try_set_osi_mode() should do the trick for us. > >> > > > > I don't like the idea of new property/flag for this for simple reason. > > Once you have that it is impossible to control if downstream new platforms > > are using it and they will expect it to be maintained once they ship the > > product. > > According to my quick research the requested issue was present on > platforms revealed from 2015 (2013?) to 2017. Since sdm845 (December > 2017) the issue is not present. So this is a part of history rather than > current platforms being in need of this quirk. > > > > >> I wouldn't mind maintaining such small parts, when going forward - and > >> of course I think we should also reject any newer platforms from using > >> it. > >> > > > > The only way that we can achieve this IMO is to have quirks based on > > platform compatible which needs to be updated and can be rejected for > > newer platforms. New flags means new feature which is expected to be > > supported for long and hard to control newer platforms not using them. > > I see your point. However from my point of view, the DT property allows > more finer-grained control. It does, but perhaps it's not really needed, as you indicate below? I share Sudeep's concerns about future possible abuse, so if we think we can get the "compat list approach" to work, that sounds better for me too. > > Compare the semantics: > - Proprety: assume that the platform is in OSI mode, do not call > psci_osi_set_mode(). > > - Platform compat list: If the psci_osi_set_mode() has failed, ignore > the error. I would not dare to blindly assume that e.g. all msm8996 > devices are in OSI mode. You have a point! So it looks safer to ignore the error that is returned from psci_osi_set_mode(), rather than skipping to call it (maybe we want to log a warning when it fails though?). [...] Kind regards Uffe
