[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <63419046-0770-45fd-a602-3ee7e08ff579@foss.st.com>
Date: Mon, 12 Jan 2026 10:13:02 +0100
From: Gatien CHEVALLIER <gatien.chevallier@...s.st.com>
To: Krzysztof Kozlowski <krzk@...nel.org>,
Suzuki K Poulose
<suzuki.poulose@....com>,
Mike Leach <mike.leach@...aro.org>,
James Clark
<james.clark@...aro.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski
<krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Mathieu Poirier
<mathieu.poirier@...aro.org>,
Leo Yan <leo.yan@...ux.dev>,
Clément Le Goffic <legoffic.clement@...il.com>,
"Linus
Walleij" <linusw@...nel.org>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>
CC: <coresight@...ts.linaro.org>, <linux-arm-kernel@...ts.infradead.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-gpio@...r.kernel.org>,
<linux-stm32@...md-mailman.stormreply.com>
Subject: Re: [PATCH 05/11] drivers: bus: add the stm32 debug bus driver
On 1/11/26 12:27, Krzysztof Kozlowski wrote:
> On 09/01/2026 11:55, Gatien Chevallier wrote:
>> Add the stm32 debug bus driver that is responsible of checking the
>> +
>> +static int stm32_dbg_bus_grant_access(struct stm32_firewall_controller *ctrl, u32 dbg_profile)
>> +{
>> + struct tee_ioctl_invoke_arg inv_arg = {0};
>> + struct tee_param param[1] = {0};
>> + u32 session_id;
>> + int ret;
>> +
>> + if (dbg_profile != PERIPHERAL_DBG_PROFILE && dbg_profile != HDP_DBG_PROFILE)
>> + return -EOPNOTSUPP;
>> +
>> + ret = stm32_dbg_pta_open_session(&session_id);
>> + if (ret)
>> + return ret;
>> +
>> + inv_arg.func = PTA_CMD_GRANT_DBG_ACCESS;
>> + inv_arg.session = session_id;
>> + inv_arg.num_params = 1;
>> + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT;
>> + param[0].u.value.a = dbg_profile;
>> +
>> + ret = tee_client_invoke_func(stm32_dbg_bus_priv->ctx, &inv_arg, param);
>> + if (ret < 0 || inv_arg.ret != 0) {
>> + dev_dbg(stm32_dbg_bus_priv->dev,
>> + "When invoking function, err %x, TEE returns: %x\n", ret, inv_arg.ret);
>> + if (!ret)
>> + ret = -EACCES;
>> + }
>> +
>> + stm32_dbg_pta_close_session(session_id);
>> +
>> + return ret;
>> +}
>> +
>> +/* Implement mandatory release_access ops even if it does nothing*/
>> +static void stm32_dbg_bus_release_access(struct stm32_firewall_controller *ctrl, u32 dbg_profile)
>> +{
>> +}
>> +
>> +static int stm32_dbg_bus_plat_probe(struct platform_device *pdev)
>> +{
>> + struct stm32_firewall_controller *dbg_controller;
>> + int ret;
>> +
>> + if (!stm32_dbg_bus_priv)
>> + return dev_err_probe(&pdev->dev, -EPROBE_DEFER,
>> + "OP-TEE debug services not yet available\n");
>> +
>> + dbg_controller = devm_kzalloc(&pdev->dev, sizeof(*dbg_controller), GFP_KERNEL);
>> + if (!dbg_controller)
>> + return dev_err_probe(&pdev->dev, -ENOMEM, "Couldn't allocate debug controller\n");
>> +
>> + dbg_controller->dev = &pdev->dev;
>> + dbg_controller->mmio = NULL;
>> + dbg_controller->name = dev_driver_string(dbg_controller->dev);
>> + dbg_controller->type = STM32_PERIPHERAL_FIREWALL;
>> + dbg_controller->grant_access = stm32_dbg_bus_grant_access;
>> + dbg_controller->release_access = stm32_dbg_bus_release_access;
>> +
>> + stm32_dbg_bus_priv->dbg_clk = devm_clk_get_enabled(&pdev->dev, NULL);
>> + if (IS_ERR(stm32_dbg_bus_priv->dbg_clk))
>> + return PTR_ERR(stm32_dbg_bus_priv->dbg_clk);
>> +
>> + ret = stm32_firewall_controller_register(dbg_controller);
>> + if (ret) {
>> + dev_err(dbg_controller->dev, "Couldn't register as a firewall controller: %d", ret);
>> + return ret;
>> + }
>> +
>> + ret = stm32_firewall_populate_bus(dbg_controller);
>> + if (ret) {
>> + dev_err(dbg_controller->dev, "Couldn't populate debug bus: %d", ret);
>> + stm32_firewall_controller_unregister(dbg_controller);
>> + return ret;
>> + }
>> +
>> + pm_runtime_enable(&pdev->dev);
>> +
>> + ret = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
>> + if (ret) {
>> + dev_err(dbg_controller->dev, "Couldn't populate the node: %d", ret);
>> + stm32_firewall_controller_unregister(dbg_controller);
>> + return ret;
>
> Where do you depopulate on unbind?
>
Nowhere, so I'll add it for V2.
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int __maybe_unused stm32_dbg_bus_runtime_suspend(struct device *dev)
>> +{
>> + clk_disable_unprepare(stm32_dbg_bus_priv->dbg_clk);
>> +
>> + return 0;
>> +}
>> +
>> +static int __maybe_unused stm32_dbg_bus_runtime_resume(struct device *dev)
>> +{
>> + int ret = clk_prepare_enable(stm32_dbg_bus_priv->dbg_clk);
>> +
>> + if (ret) {
>> + dev_err(dev, "Failed to enable clock: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id stm32_dbg_bus_of_match[] = {
>> + { .compatible = "st,stm32mp131-dbg-bus", },
>> + { .compatible = "st,stm32mp151-dbg-bus", },
>
> So devices are fully compatible?
>
As it is a standard bus, yes. The goal of this driver is to be
able to interface with the secure world to check the debug
configuration.
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, stm32_dbg_bus_of_match);
>> +
>> +static const struct dev_pm_ops simple_pm_bus_pm_ops = {
>> + SET_RUNTIME_PM_OPS(stm32_dbg_bus_runtime_suspend, stm32_dbg_bus_runtime_resume, NULL)
>> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
>> +};
>> +
>> +static struct platform_driver stm32_dbg_bus_driver = {
>> + .probe = stm32_dbg_bus_plat_probe,
>> + .driver = {
>> + .name = "stm32-dbg-bus",
>> + .of_match_table = of_match_ptr(stm32_dbg_bus_of_match),
>
> Warning :/. Why do people still keep copying of_match_ptr?
>
I'll remove that in V2.
>> + .pm = pm_ptr(&simple_pm_bus_pm_ops),
>> + },
>> +};
>> +
>> +static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
>> +{
>> + return (ver->impl_id == TEE_IMPL_ID_OPTEE);
>> +}
>> +
>> +static int stm32_dbg_bus_probe(struct device *dev)
>> +{
>> + struct stm32_dbg_bus *priv;
>> +
>> + if (stm32_dbg_bus_priv)
>> + return dev_err_probe(dev, -EBUSY,
>> + "A STM32 debug bus device is already initialized\n");
>> +
>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return dev_err_probe(dev, -ENOMEM, "Cannot allocate priv data\n");
>
> You NEVER print error allocations on kzalloc, even if with dev_err_probe
> it is silenced.
>
Ok, I'll keep that in mind and remove that for V2.
Best regards,
Gatien
>> +
>> + /* Open context with TEE driver */
>> + priv->ctx = tee_client_open_context(NULL, optee_ctx_match, NULL, NULL);
>> + if (IS_ERR_OR_NULL(priv->ctx))
>> + return dev_err_probe(dev, PTR_ERR_OR_ZERO(priv->ctx), "Cannot open TEE context\n");
>> +
>> + stm32_dbg_bus_priv = priv;
>> + stm32_dbg_bus_priv->dev = dev;
>> +
>> + return 0;
>> +}
>> +
>> +static int stm32_dbg_bus_remove(struct device *dev)
>> +{
>> + tee_client_close_context(stm32_dbg_bus_priv->ctx);
>> + stm32_dbg_bus_priv = NULL;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct tee_client_device_id optee_dbg_bus_id_table[] = {
>> + {UUID_INIT(0xdd05bc8b, 0x9f3b, 0x49f0,
>> + 0xb6, 0x49, 0x01, 0xaa, 0x10, 0xc1, 0xc2, 0x10)},
>> + {}
>> +};
>> +
>> +static struct tee_client_driver stm32_optee_dbg_bus_driver = {
>> + .id_table = optee_dbg_bus_id_table,
>> + .driver = {
>> + .name = "optee_dbg_bus",
>> + .bus = &tee_bus_type,
>> + .probe = stm32_dbg_bus_probe,
>> + .remove = stm32_dbg_bus_remove,
>> + },
>> +};
>> +
>> +static int __init optee_dbg_bus_mod_init(void)
>> +{
>> + int ret;
>> +
>> + ret = driver_register(&stm32_optee_dbg_bus_driver.driver);
>> + if (ret)
>> + return ret;
>> +
>> + ret = platform_driver_register(&stm32_dbg_bus_driver);
>> + if (ret)
>> + driver_unregister(&stm32_optee_dbg_bus_driver.driver);
>> +
>> + return ret;
>> +}
>> +
>> +static void __exit optee_dbg_bus_mod_exit(void)
>> +{
>> + platform_driver_unregister(&stm32_dbg_bus_driver);
>> + driver_unregister(&stm32_optee_dbg_bus_driver.driver);
>> +}
>> +
>> +module_init(optee_dbg_bus_mod_init);
>> +module_exit(optee_dbg_bus_mod_exit);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Gatien Chevallier <gatien.chevallier@...s.st.com>");
>> +MODULE_DESCRIPTION("OP-TEE based STM32 debug access bus driver");
>>
>
>
> Best regards,
> Krzysztof
Powered by blists - more mailing lists