linux-kernel - Re: [PATCH v10 3/4] firmware: ti_sci: Introduce Power Management Ops

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240829032423.ou3hqw2sq5wj2ue6@oversight>
Date: Wed, 28 Aug 2024 22:24:23 -0500
From: Nishanth Menon <nm@...com>
To: Markus Schneider-Pargmann <msp@...libre.com>
CC: Kevin Hilman <khilman@...libre.com>, Tero Kristo <kristo@...nel.org>,
        Santosh Shilimkar <ssantosh@...nel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
        Vibhore Vardhan <vibhore@...com>, Dhruva Gole <d-gole@...com>,
        Akashdeep Kaur
	<a-kaur@...com>
Subject: Re: [PATCH v10 3/4] firmware: ti_sci: Introduce Power Management Ops

On 21:54-20240828, Markus Schneider-Pargmann wrote:
> Hi Nishanth,
> 
> thanks for your review. I will integrate fixes for all your comments
> into the next version.
> 
> On Mon, Aug 26, 2024 at 11:43:46AM GMT, Nishanth Menon wrote:
> > On 08:39-20240814, Kevin Hilman wrote:
> > [...]
> > > diff --git a/include/linux/soc/ti/ti_sci_protocol.h b/include/linux/soc/ti/ti_sci_protocol.h
> > > index 1f1871e23f76..4ba9e520a28f 100644
> > > --- a/include/linux/soc/ti/ti_sci_protocol.h
> > > +++ b/include/linux/soc/ti/ti_sci_protocol.h
> > > @@ -199,6 +199,47 @@ struct ti_sci_clk_ops {
> > >  #define TISCI_MSG_VALUE_IO_ENABLE			1
> > >  #define TISCI_MSG_VALUE_IO_DISABLE			0
> > >  
> > > +/* TISCI LPM wake up sources */
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_I2C0	0x00
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_UART0	0x10
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MCU_GPIO0	0x20
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_ICEMELTER0	0x30
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_TIMER0	0x40
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_TIMER1	0x41
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_WKUP_RTC0	0x50
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_RESET		0x60
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_USB0		0x70
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_USB1		0x71
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MAIN_IO		0x80
> > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MCU_IO		0x81
> > 
> > ^^ I assume these are daisy chain sources. - will these remain OR do we
> > have to consider the wake sources SoC dependent? If so, the
> > sci_protocol.h will be SoC agnostic..
> 
> These are all wakeup sources from LPM for the AM62 family of SoCs, not
> only daisy chain sources. The currently defined wakeup sources are
> relevant for am62x/a/p but will also be reused for others where
> possible. Otherwise these can be extended in the future for other wakeup
> sources.
> 

OK -> I should have been clear, but, I think you also caught on it
when you said am62x/a/p extended for future wakeup sources - sure..
with 32 bits you can indeed reach a large range.. BUT:

MAIN_DOMAIN, MCU_DOMAIN are specific to a SoC part architecture, it is
not a generic K3 SoC family architecture concept - the power domains
will vary depending on device - same with the list of peripherals used
as wakeup source - is WKUP_I2C0 always present in all devices with
wakeup, Do all K3 devices have USB0 and 1 always? We should not bet on
that.

ti_sci_protocol.h is meant as a generic SoC family level header. I see
these as possibly hardware specific description.

Lastly, I do not see the macros used either in the patch series.. I
understand the ti_sci_protocol.h is meant to standardize stuff in
other driver (I tried searching to see if some other series used
it[1], but I could not find a reference)..

So, wondering: Is DT the right place for that - With a DT header ABI
header that is shared between driver and dt? I don't understand the
modelling of how wakeup_reason is getting used from this series, so I
cannot comment better..

[1] https://lore.kernel.org/all/?q=TISCI_MSG_VALUE_LPM_WAKE_SOURCE_MAIN_IO
-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D