Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 3 Aug 2023 21:12:04 +0530
From:   Dhruva Gole <d-gole@ti.com>
To:     Andrew Davis <afd@ti.com>
CC:     Nishanth Menon <nm@ti.com>, Tero Kristo <kristo@kernel.org>,
        "Santosh Shilimkar" <ssantosh@kernel.org>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>, <linux-pm@vger.kernel.org>,
        Viresh Kumar <viresh.kumar@linaro.org>,
        Praneeth Bajjuri <praneeth@ti.com>,
        Tony Lindgren <tony@atomide.com>,
        Dave Gerlach <d-gerlach@ti.com>,
        Vibhore Vardhan <vibhore@ti.com>,
        Georgi Vlaev <g-vlaev@ti.com>,
        Roger Quadros <rogerq@kernel.org>
Subject: Re: [PATCH V6 1/4] firmware: ti_sci: Introduce Power Management Ops
Message-ID: <20230803154204.iyohslrpma7bqqpk@dhruva>
References: <20230803064247.503036-1-d-gole@ti.com>
 <20230803064247.503036-2-d-gole@ti.com>
 <930ffb16-f212-0bcb-4713-2acc138c4035@ti.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <930ffb16-f212-0bcb-4713-2acc138c4035@ti.com>
Precedence: bulk

On Aug 03, 2023 at 10:14:03 -0500, Andrew Davis wrote:
[..snip..]
> >   #define TI_SCI_IRQ_SECONDARY_HOST_INVALID	0xff
> >   /**
> > diff --git a/include/linux/soc/ti/ti_sci_protocol.h b/include/linux/soc/ti/ti_sci_protocol.h
> > index bd0d11af76c5..f2d1d74ab8fc 100644
> > --- a/include/linux/soc/ti/ti_sci_protocol.h
> > +++ b/include/linux/soc/ti/ti_sci_protocol.h
> > @@ -195,6 +195,49 @@ struct ti_sci_clk_ops {
> >   			u64 *current_freq);
> >   };
> > +/* 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
> > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_CAN_IO		0x82
> > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_INVALID		0xFF
> 
> I don't see these used in this series, do you need them? lpm_wake_reason()

True, we are not currently using these macros. They _maybe required in
future.
I can remove them if required?

> doesn't seem used either and I'm not sure how you plan to use it, could
> you detail that?

When the system wakes from suspend-to-mem we can check which
subsystem has woken us up with the TISCI LPM_WAKEUP_REASON message.
There's no hardware event generated and we have to ask the firmware
for the actual wake reason.

We may want to add support for a wake up interrupt controller that will
generate an interrupt for other subsystems. This might end up using this
lpm_wake_reason API, hence even though the function maybe unused today
it will be required for above described scenario.

However if you prefer that I remove it and then add it in future when we
finally write a working interrupt controller driver, then do let me
know.

> 
> Andrew
> 
> > +
> > +/* TISCI LPM IO isolation control values */
> > +#define TISCI_MSG_VALUE_IO_ENABLE			1
> > +#define TISCI_MSG_VALUE_IO_DISABLE			0
> > +
[..snip..]

-- 
Best regards,
Dhruva Gole <d-gole@ti.com>