| Message ID | 20240814-lpm-constraints-firmware-msp-v10-3-bee4314bbdc8@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | firmware: ti_sci: Introduce system suspend support | expand |
On 14/08/24 21:09, Kevin Hilman wrote: > From: Dave Gerlach <d-gerlach@ ti. com> Introduce power management ops > supported by the TISCI Low Power Mode API [1]. 1) > TISCI_MSG_LPM_WAKE_REASON Get which wake up source woke the SoC from Low > Power Mode. The wake up source IDs will be > ZjQcmQRYFpfptBannerStart > This message was sent from outside of Texas Instruments. > Do not click links or open attachments unless you recognize the source > of this email and know the content is safe. > Report Suspicious > <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK!tFdqXRfP9mwbCoXPtNggdrCuJ3bSsNDHabWG8s9g6Hh_v3tSab2OpTcpCeIxxvksAU_Fa5Zl41XmoQoqJiZdQtEck4kHNZot31hxXq-ZtaRCwdotcPeN9ST2tD8$> > ZjQcmQRYFpfptBannerEnd > > From: Dave Gerlach <d-gerlach@ti.com> > > Introduce power management ops supported by the TISCI > Low Power Mode API [1]. > > 1) TISCI_MSG_LPM_WAKE_REASON > Get which wake up source woke the SoC from Low Power Mode. > The wake up source IDs will be common for all K3 platforms. > > 2) TISCI_MSG_LPM_SET_DEVICE_CONSTRAINT > Set LPM constraint on behalf of a device. By setting a constraint, the > device ensures that it will not be powered off or reset in the selected > mode. > > 3) TISCI_MSG_LPM_SET_LATENCY_CONSTRAINT > Set LPM resume latency constraint. By setting a constraint, the host > ensures that the resume time from selected mode will be less than the > constraint value. > > [1] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html > > Signed-off-by: Dave Gerlach <d-gerlach@ti.com> > [g-vlaev@ti.com: LPM_WAKE_REASON and IO_ISOLATION support] > Signed-off-by: Georgi Vlaev <g-vlaev@ti.com> > [a-kaur@ti.com: SET_DEVICE_CONSTRAINT support] > Signed-off-by: Akashdeep Kaur <a-kaur@ti.com> > [vibhore@ti.com: SET_LATENCY_CONSTRAINT support] > Signed-off-by: Vibhore Vardhan <vibhore@ti.com> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> > Signed-off-by: Kevin Hilman <khilman@baylibre.com> > --- [...] My concerns are now addressed. Reviewed-by: Akashdeep Kaur <a-kaur@ti.com>
On 08:39-20240814, Kevin Hilman wrote: > From: Dave Gerlach <d-gerlach@ti.com> > [...] > diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c > index 808149dcc635..107494406fa5 100644 > --- a/drivers/firmware/ti_sci.c > +++ b/drivers/firmware/ti_sci.c > @@ -1822,6 +1822,179 @@ static int ti_sci_cmd_set_io_isolation(const struct ti_sci_handle *handle, > return ret; > } > > +/** > + * ti_sci_msg_cmd_lpm_wake_reason() - Get the wakeup source from LPM > + * @handle: Pointer to TI SCI handle > + * @source: The wakeup source that woke the SoC from LPM > + * @timestamp: Timestamp of the wakeup event > + * @pin: The pin that has triggered wake up > + * @mode: The last entered low power mode > + * > + * Return: 0 if all went well, else returns appropriate error value. > + */ > +static int ti_sci_msg_cmd_lpm_wake_reason(const struct ti_sci_handle *handle, > + u32 *source, u64 *timestamp, u8 *pin, u8 *mode) > +{ > + struct ti_sci_info *info; > + struct ti_sci_xfer *xfer; > + struct ti_sci_msg_resp_lpm_wake_reason *resp; > + struct device *dev; > + int ret = 0; > + > + if (IS_ERR(handle)) > + return PTR_ERR(handle); > + if (!handle) > + return -EINVAL; > + > + info = handle_to_ti_sci_info(handle); > + dev = info->dev; > + > + xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_LPM_WAKE_REASON, > + TI_SCI_FLAG_REQ_ACK_ON_PROCESSED, > + sizeof(struct ti_sci_msg_hdr), > + sizeof(*resp)); > + if (IS_ERR(xfer)) { > + ret = PTR_ERR(xfer); > + dev_err(dev, "Message alloc failed(%d)\n", ret); > + return ret; > + } > + > + ret = ti_sci_do_xfer(info, xfer); > + if (ret) { > + dev_err(dev, "Mbox send fail %d\n", ret); > + goto fail; > + } > + > + resp = (struct ti_sci_msg_resp_lpm_wake_reason *)xfer->xfer_buf; > + > + if (!ti_sci_is_response_ack(resp)) { A dev_err might be worth here. > + ret = -ENODEV; > + goto fail; > + } > + > + if (source) > + *source = resp->wake_source; > + if (timestamp) > + *timestamp = resp->wake_timestamp; > + if (pin) > + *pin = resp->wake_pin; > + if (mode) > + *mode = resp->mode; > + > +fail: > + ti_sci_put_one_xfer(&info->minfo, xfer); > + > + return ret; > +} > + > +/** > + * ti_sci_cmd_set_device_constraint() - Set LPM constraint on behalf of a device > + * @handle: pointer to TI SCI handle > + * @id: Device identifier > + * @state: The desired state of device constraint: set or clear > + * > + * Return: 0 if all went well, else returns appropriate error value. > + */ > +static int ti_sci_cmd_set_device_constraint(const struct ti_sci_handle *handle, > + u32 id, u8 state) > +{ > + struct ti_sci_info *info; > + struct ti_sci_msg_req_lpm_set_device_constraint *req; > + struct ti_sci_msg_hdr *resp; > + struct ti_sci_xfer *xfer; > + struct device *dev; > + int ret = 0; > + > + if (IS_ERR(handle)) > + return PTR_ERR(handle); > + if (!handle) > + return -EINVAL; > + > + info = handle_to_ti_sci_info(handle); > + dev = info->dev; > + > + xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_LPM_SET_DEVICE_CONSTRAINT, > + TI_SCI_FLAG_REQ_ACK_ON_PROCESSED, > + sizeof(*req), sizeof(*resp)); > + if (IS_ERR(xfer)) { > + ret = PTR_ERR(xfer); > + dev_err(dev, "Message alloc failed(%d)\n", ret); > + return ret; > + } > + req = (struct ti_sci_msg_req_lpm_set_device_constraint *)xfer->xfer_buf; > + req->id = id; > + req->state = state; > + > + ret = ti_sci_do_xfer(info, xfer); > + if (ret) { > + dev_err(dev, "Mbox send fail %d\n", ret); > + goto fail; > + } > + > + resp = (struct ti_sci_msg_hdr *)xfer->xfer_buf; > + > + ret = ti_sci_is_response_ack(resp) ? 0 : -ENODEV; A dev_err might be worth here. - same elsewhere. I see a different style of if (ret) vs ? 0: -ENODEV usage - might be good to be consistent through out. > + > +fail: > + ti_sci_put_one_xfer(&info->minfo, xfer); > + > + return ret; > +} > + > +/** > + * ti_sci_cmd_set_latency_constraint() - Set LPM resume latency constraint > + * @handle: pointer to TI SCI handle > + * @latency: maximum acceptable latency (in ms) to wake up from LPM > + * @state: The desired state of latency constraint: set or clear > + * > + * Return: 0 if all went well, else returns appropriate error value. > + */ > +static int ti_sci_cmd_set_latency_constraint(const struct ti_sci_handle *handle, > + u16 latency, u8 state) > +{ > + struct ti_sci_info *info; > + struct ti_sci_msg_req_lpm_set_latency_constraint *req; > + struct ti_sci_msg_hdr *resp; > + struct ti_sci_xfer *xfer; > + struct device *dev; > + int ret = 0; > + > + if (IS_ERR(handle)) > + return PTR_ERR(handle); > + if (!handle) > + return -EINVAL; > + > + info = handle_to_ti_sci_info(handle); > + dev = info->dev; > + > + xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_LPM_SET_LATENCY_CONSTRAINT, > + TI_SCI_FLAG_REQ_ACK_ON_PROCESSED, > + sizeof(*req), sizeof(*resp)); > + if (IS_ERR(xfer)) { > + ret = PTR_ERR(xfer); > + dev_err(dev, "Message alloc failed(%d)\n", ret); > + return ret; > + } > + req = (struct ti_sci_msg_req_lpm_set_latency_constraint *)xfer->xfer_buf; > + req->latency = latency; > + req->state = state; > + > + ret = ti_sci_do_xfer(info, xfer); > + if (ret) { > + dev_err(dev, "Mbox send fail %d\n", ret); > + goto fail; > + } > + > + resp = (struct ti_sci_msg_hdr *)xfer->xfer_buf; > + > + ret = ti_sci_is_response_ack(resp) ? 0 : -ENODEV; Same > + > +fail: > + ti_sci_put_one_xfer(&info->minfo, xfer); > + > + return ret; > +} > + > static int ti_sci_cmd_core_reboot(const struct ti_sci_handle *handle) > { > struct ti_sci_info *info; > @@ -2964,6 +3137,7 @@ static void ti_sci_setup_ops(struct ti_sci_info *info) > struct ti_sci_core_ops *core_ops = &ops->core_ops; > struct ti_sci_dev_ops *dops = &ops->dev_ops; > struct ti_sci_clk_ops *cops = &ops->clk_ops; > + struct ti_sci_pm_ops *pmops = &ops->pm_ops; > struct ti_sci_rm_core_ops *rm_core_ops = &ops->rm_core_ops; > struct ti_sci_rm_irq_ops *iops = &ops->rm_irq_ops; > struct ti_sci_rm_ringacc_ops *rops = &ops->rm_ring_ops; > @@ -3003,6 +3177,13 @@ static void ti_sci_setup_ops(struct ti_sci_info *info) > cops->set_freq = ti_sci_cmd_clk_set_freq; > cops->get_freq = ti_sci_cmd_clk_get_freq; > > + if (info->fw_caps & MSG_FLAG_CAPS_LPM_DM_MANAGED) { > + pr_debug("detected DM managed LPM in fw_caps\n"); > + pmops->lpm_wake_reason = ti_sci_msg_cmd_lpm_wake_reason; > + pmops->set_device_constraint = ti_sci_cmd_set_device_constraint; > + pmops->set_latency_constraint = ti_sci_cmd_set_latency_constraint; > + } > + > rm_core_ops->get_range = ti_sci_cmd_get_resource_range; > rm_core_ops->get_range_from_shost = > ti_sci_cmd_get_resource_range_from_shost; > @@ -3490,12 +3671,20 @@ static int ti_sci_resume_noirq(struct device *dev) > { > struct ti_sci_info *info = dev_get_drvdata(dev); > int ret = 0; > + u32 source; > + u64 time; > + u8 pin; > + u8 mode; > > ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_DISABLE); > if (ret) > return ret; > dev_dbg(dev, "%s: disable isolation: %d\n", __func__, ret); > > + ti_sci_msg_cmd_lpm_wake_reason(&info->handle, &source, &time, &pin, &mode); No handling of error? > + dev_info(dev, "ti_sci: wakeup source:0x%x, pin:0x%x, mode:0x%x\n", > + source, pin, mode); > + > return 0; > } > [...] > > /** > 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.. > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_CAN_IO 0x82 > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_INVALID 0xFF > + > +/* TISCI LPM constraint state values */ > +#define TISCI_MSG_CONSTRAINT_SET 1 > +#define TISCI_MSG_CONSTRAINT_CLR 0 > + [...]
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. Best Markus > > > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_CAN_IO 0x82 > > +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_INVALID 0xFF > > + > > +/* TISCI LPM constraint state values */ > > +#define TISCI_MSG_CONSTRAINT_SET 1 > > +#define TISCI_MSG_CONSTRAINT_CLR 0 > > + > > [...] > > -- > Regards, > Nishanth Menon > Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
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
Hi Nishanth, On Wed, Aug 28, 2024 at 10:24:23PM GMT, Nishanth Menon wrote: > 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.. Thanks for explaining. So should I add the header already with this series although it is unused right now, or should we add it together with the first actual user later on, so there is no unused header in the meantime? Best Markus > > [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
On 10:47-20240829, Markus Schneider-Pargmann wrote: [...] > Thanks for explaining. So should I add the header already with this > series although it is unused right now, or should we add it together > with the first actual user later on, so there is no unused header in the > meantime? Thinking deeper: we have two options: a) dt bindings update with the property without knowing how the driver changes will be accepted or not. b) drop the header changes for the macros. I think (a) at this point is risky given the driver usage model is un-clear - the APIs are abstract enough to be used in any way of choice, but we do not want to be stuck with binding that then has to be backward-forward compatible fixup.. So. (b) is better approach, IMHO..
diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c index 808149dcc635..107494406fa5 100644 --- a/drivers/firmware/ti_sci.c +++ b/drivers/firmware/ti_sci.c @@ -1822,6 +1822,179 @@ static int ti_sci_cmd_set_io_isolation(const struct ti_sci_handle *handle, return ret; } +/** + * ti_sci_msg_cmd_lpm_wake_reason() - Get the wakeup source from LPM + * @handle: Pointer to TI SCI handle + * @source: The wakeup source that woke the SoC from LPM + * @timestamp: Timestamp of the wakeup event + * @pin: The pin that has triggered wake up + * @mode: The last entered low power mode + * + * Return: 0 if all went well, else returns appropriate error value. + */ +static int ti_sci_msg_cmd_lpm_wake_reason(const struct ti_sci_handle *handle, + u32 *source, u64 *timestamp, u8 *pin, u8 *mode) +{ + struct ti_sci_info *info; + struct ti_sci_xfer *xfer; + struct ti_sci_msg_resp_lpm_wake_reason *resp; + struct device *dev; + int ret = 0; + + if (IS_ERR(handle)) + return PTR_ERR(handle); + if (!handle) + return -EINVAL; + + info = handle_to_ti_sci_info(handle); + dev = info->dev; + + xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_LPM_WAKE_REASON, + TI_SCI_FLAG_REQ_ACK_ON_PROCESSED, + sizeof(struct ti_sci_msg_hdr), + sizeof(*resp)); + if (IS_ERR(xfer)) { + ret = PTR_ERR(xfer); + dev_err(dev, "Message alloc failed(%d)\n", ret); + return ret; + } + + ret = ti_sci_do_xfer(info, xfer); + if (ret) { + dev_err(dev, "Mbox send fail %d\n", ret); + goto fail; + } + + resp = (struct ti_sci_msg_resp_lpm_wake_reason *)xfer->xfer_buf; + + if (!ti_sci_is_response_ack(resp)) { + ret = -ENODEV; + goto fail; + } + + if (source) + *source = resp->wake_source; + if (timestamp) + *timestamp = resp->wake_timestamp; + if (pin) + *pin = resp->wake_pin; + if (mode) + *mode = resp->mode; + +fail: + ti_sci_put_one_xfer(&info->minfo, xfer); + + return ret; +} + +/** + * ti_sci_cmd_set_device_constraint() - Set LPM constraint on behalf of a device + * @handle: pointer to TI SCI handle + * @id: Device identifier + * @state: The desired state of device constraint: set or clear + * + * Return: 0 if all went well, else returns appropriate error value. + */ +static int ti_sci_cmd_set_device_constraint(const struct ti_sci_handle *handle, + u32 id, u8 state) +{ + struct ti_sci_info *info; + struct ti_sci_msg_req_lpm_set_device_constraint *req; + struct ti_sci_msg_hdr *resp; + struct ti_sci_xfer *xfer; + struct device *dev; + int ret = 0; + + if (IS_ERR(handle)) + return PTR_ERR(handle); + if (!handle) + return -EINVAL; + + info = handle_to_ti_sci_info(handle); + dev = info->dev; + + xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_LPM_SET_DEVICE_CONSTRAINT, + TI_SCI_FLAG_REQ_ACK_ON_PROCESSED, + sizeof(*req), sizeof(*resp)); + if (IS_ERR(xfer)) { + ret = PTR_ERR(xfer); + dev_err(dev, "Message alloc failed(%d)\n", ret); + return ret; + } + req = (struct ti_sci_msg_req_lpm_set_device_constraint *)xfer->xfer_buf; + req->id = id; + req->state = state; + + ret = ti_sci_do_xfer(info, xfer); + if (ret) { + dev_err(dev, "Mbox send fail %d\n", ret); + goto fail; + } + + resp = (struct ti_sci_msg_hdr *)xfer->xfer_buf; + + ret = ti_sci_is_response_ack(resp) ? 0 : -ENODEV; + +fail: + ti_sci_put_one_xfer(&info->minfo, xfer); + + return ret; +} + +/** + * ti_sci_cmd_set_latency_constraint() - Set LPM resume latency constraint + * @handle: pointer to TI SCI handle + * @latency: maximum acceptable latency (in ms) to wake up from LPM + * @state: The desired state of latency constraint: set or clear + * + * Return: 0 if all went well, else returns appropriate error value. + */ +static int ti_sci_cmd_set_latency_constraint(const struct ti_sci_handle *handle, + u16 latency, u8 state) +{ + struct ti_sci_info *info; + struct ti_sci_msg_req_lpm_set_latency_constraint *req; + struct ti_sci_msg_hdr *resp; + struct ti_sci_xfer *xfer; + struct device *dev; + int ret = 0; + + if (IS_ERR(handle)) + return PTR_ERR(handle); + if (!handle) + return -EINVAL; + + info = handle_to_ti_sci_info(handle); + dev = info->dev; + + xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_LPM_SET_LATENCY_CONSTRAINT, + TI_SCI_FLAG_REQ_ACK_ON_PROCESSED, + sizeof(*req), sizeof(*resp)); + if (IS_ERR(xfer)) { + ret = PTR_ERR(xfer); + dev_err(dev, "Message alloc failed(%d)\n", ret); + return ret; + } + req = (struct ti_sci_msg_req_lpm_set_latency_constraint *)xfer->xfer_buf; + req->latency = latency; + req->state = state; + + ret = ti_sci_do_xfer(info, xfer); + if (ret) { + dev_err(dev, "Mbox send fail %d\n", ret); + goto fail; + } + + resp = (struct ti_sci_msg_hdr *)xfer->xfer_buf; + + ret = ti_sci_is_response_ack(resp) ? 0 : -ENODEV; + +fail: + ti_sci_put_one_xfer(&info->minfo, xfer); + + return ret; +} + static int ti_sci_cmd_core_reboot(const struct ti_sci_handle *handle) { struct ti_sci_info *info; @@ -2964,6 +3137,7 @@ static void ti_sci_setup_ops(struct ti_sci_info *info) struct ti_sci_core_ops *core_ops = &ops->core_ops; struct ti_sci_dev_ops *dops = &ops->dev_ops; struct ti_sci_clk_ops *cops = &ops->clk_ops; + struct ti_sci_pm_ops *pmops = &ops->pm_ops; struct ti_sci_rm_core_ops *rm_core_ops = &ops->rm_core_ops; struct ti_sci_rm_irq_ops *iops = &ops->rm_irq_ops; struct ti_sci_rm_ringacc_ops *rops = &ops->rm_ring_ops; @@ -3003,6 +3177,13 @@ static void ti_sci_setup_ops(struct ti_sci_info *info) cops->set_freq = ti_sci_cmd_clk_set_freq; cops->get_freq = ti_sci_cmd_clk_get_freq; + if (info->fw_caps & MSG_FLAG_CAPS_LPM_DM_MANAGED) { + pr_debug("detected DM managed LPM in fw_caps\n"); + pmops->lpm_wake_reason = ti_sci_msg_cmd_lpm_wake_reason; + pmops->set_device_constraint = ti_sci_cmd_set_device_constraint; + pmops->set_latency_constraint = ti_sci_cmd_set_latency_constraint; + } + rm_core_ops->get_range = ti_sci_cmd_get_resource_range; rm_core_ops->get_range_from_shost = ti_sci_cmd_get_resource_range_from_shost; @@ -3490,12 +3671,20 @@ static int ti_sci_resume_noirq(struct device *dev) { struct ti_sci_info *info = dev_get_drvdata(dev); int ret = 0; + u32 source; + u64 time; + u8 pin; + u8 mode; ret = ti_sci_cmd_set_io_isolation(&info->handle, TISCI_MSG_VALUE_IO_DISABLE); if (ret) return ret; dev_dbg(dev, "%s: disable isolation: %d\n", __func__, ret); + ti_sci_msg_cmd_lpm_wake_reason(&info->handle, &source, &time, &pin, &mode); + dev_info(dev, "ti_sci: wakeup source:0x%x, pin:0x%x, mode:0x%x\n", + source, pin, mode); + return 0; } diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h index 8efe4d0e61fb..053387d7baa0 100644 --- a/drivers/firmware/ti_sci.h +++ b/drivers/firmware/ti_sci.h @@ -38,7 +38,10 @@ /* Low Power Mode Requests */ #define TI_SCI_MSG_PREPARE_SLEEP 0x0300 +#define TI_SCI_MSG_LPM_WAKE_REASON 0x0306 #define TI_SCI_MSG_SET_IO_ISOLATION 0x0307 +#define TI_SCI_MSG_LPM_SET_DEVICE_CONSTRAINT 0x0309 +#define TI_SCI_MSG_LPM_SET_LATENCY_CONSTRAINT 0x030A /* Resource Management Requests */ #define TI_SCI_MSG_GET_RESOURCE_RANGE 0x1500 @@ -610,6 +613,79 @@ struct ti_sci_msg_req_set_io_isolation { u8 state; } __packed; +/** + * struct ti_sci_msg_resp_lpm_wake_reason - Response for TI_SCI_MSG_LPM_WAKE_REASON. + * + * @hdr: Generic header. + * @wake_source: The wake up source that woke soc from LPM. + * @wake_timestamp: Timestamp at which soc woke. + * @wake_pin: The pin that has triggered wake up. + * @mode: The last entered low power mode. + * @rsvd: Reserved for future use. + * + * Response to a generic message with message type TI_SCI_MSG_LPM_WAKE_REASON, + * used to query the wake up source, pin and entered low power mode. + */ +struct ti_sci_msg_resp_lpm_wake_reason { + struct ti_sci_msg_hdr hdr; + u32 wake_source; + u64 wake_timestamp; + u8 wake_pin; + u8 mode; + u32 rsvd[2]; +} __packed; + +/** + * struct ti_sci_msg_req_lpm_set_device_constraint - Request for + * TISCI_MSG_LPM_SET_DEVICE_CONSTRAINT. + * + * @hdr: TISCI header to provide ACK/NAK flags to the host. + * @id: Device ID of device whose constraint has to be modified. + * @state: The desired state of device constraint: set or clear. + * @rsvd: Reserved for future use. + * + * This message is used by host to set constraint on the device. This can be + * sent anytime after boot before prepare sleep message. Any device can set a + * constraint on the low power mode that the SoC can enter. It allows + * configurable information to be easily shared from the application, as this + * is a non-secure message and therefore can be sent by anyone. By setting a + * constraint, the device ensures that it will not be powered off or reset in + * the selected mode. Note: Access Restriction: Exclusivity flag of Device will + * be honored. If some other host already has constraint on this device ID, + * NACK will be returned. + */ +struct ti_sci_msg_req_lpm_set_device_constraint { + struct ti_sci_msg_hdr hdr; + u32 id; + u8 state; + u32 rsvd[2]; +} __packed; + +/** + * struct ti_sci_msg_req_lpm_set_latency_constraint - Request for + * TISCI_MSG_LPM_SET_LATENCY_CONSTRAINT. + * + * @hdr: TISCI header to provide ACK/NAK flags to the host. + * @wkup_latency: The maximum acceptable latency to wake up from low power mode + * in milliseconds. The deeper the state, the higher the latency. + * @state: The desired state of wakeup latency constraint: set or clear. + * @rsvd: Reserved for future use. + * + * This message is used by host to set wakeup latency from low power mode. This can + * be sent anytime after boot before prepare sleep message, and can be sent after + * current low power mode is exited. Any device can set a constraint on the low power + * mode that the SoC can enter. It allows configurable information to be easily shared + * from the application, as this is a non-secure message and therefore can be sent by + * anyone. By setting a wakeup latency constraint, the host ensures that the resume time + * from selected low power mode will be less than the constraint value. + */ +struct ti_sci_msg_req_lpm_set_latency_constraint { + struct ti_sci_msg_hdr hdr; + u16 latency; + u8 state; + u32 rsvd; +} __packed; + #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 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 +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_CAN_IO 0x82 +#define TISCI_MSG_VALUE_LPM_WAKE_SOURCE_INVALID 0xFF + +/* TISCI LPM constraint state values */ +#define TISCI_MSG_CONSTRAINT_SET 1 +#define TISCI_MSG_CONSTRAINT_CLR 0 + +/** + * struct ti_sci_pm_ops - Low Power Mode (LPM) control operations + * @lpm_wake_reason: Get the wake up source that woke the SoC from LPM + * - source: The wake up source that woke soc from LPM. + * - timestamp: Timestamp at which soc woke. + * @set_device_constraint: Set LPM constraint on behalf of a device + * - id: Device Identifier + * - state: The desired state of device constraint: set or clear. + * @set_latency_constraint: Set LPM resume latency constraint + * - latency: maximum acceptable latency to wake up from low power mode + * - state: The desired state of latency constraint: set or clear. + */ +struct ti_sci_pm_ops { + int (*lpm_wake_reason)(const struct ti_sci_handle *handle, + u32 *source, u64 *timestamp, u8 *pin, u8 *mode); + int (*set_device_constraint)(const struct ti_sci_handle *handle, + u32 id, u8 state); + int (*set_latency_constraint)(const struct ti_sci_handle *handle, + u16 latency, u8 state); +}; + /** * struct ti_sci_resource_desc - Description of TI SCI resource instance range. * @start: Start index of the first resource range. @@ -543,6 +584,7 @@ struct ti_sci_ops { struct ti_sci_core_ops core_ops; struct ti_sci_dev_ops dev_ops; struct ti_sci_clk_ops clk_ops; + struct ti_sci_pm_ops pm_ops; struct ti_sci_rm_core_ops rm_core_ops; struct ti_sci_rm_irq_ops rm_irq_ops; struct ti_sci_rm_ringacc_ops rm_ring_ops;