[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b074941f-3c6a-4b98-a00e-d22d18280ef5@foss.st.com>
Date: Tue, 25 Feb 2025 17:05:14 +0100
From: Clement LE GOFFIC <clement.legoffic@...s.st.com>
To: Krzysztof Kozlowski <krzk@...nel.org>,
Linus Walleij
<linus.walleij@...aro.org>,
Rob Herring <robh@...nel.org>,
Krzysztof
Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Maxime
Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue
<alexandre.torgue@...s.st.com>,
Bartosz Golaszewski <brgl@...ev.pl>
CC: <linux-kernel@...r.kernel.org>, <linux-gpio@...r.kernel.org>,
<devicetree@...r.kernel.org>,
<linux-stm32@...md-mailman.stormreply.com>,
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 3/9] pinctrl: stm32: Introduce HDP driver
On 2/25/25 13:59, Krzysztof Kozlowski wrote:
> On 25/02/2025 09:48, Clément Le Goffic wrote:
>> This patch introduce the driver for the Hardware Debug Port available on
>
> Please do not use "This commit/patch/change", but imperative mood. See
> longer explanation here:
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
Ok
>
>> STM32MP platforms. The HDP allows the observation of internal SoC
>> signals by using multiplexers. Each HDP port can provide up to 16
>> internal signals (one of them can be software controlled as a GPO)
>
>
>
> ....
>
>> +
>> +static int stm32_hdp_suspend(struct device *dev)
>> +{
>> + struct stm32_hdp *hdp = dev_get_drvdata(dev);
>> +
>> + hdp->gposet_conf = readl_relaxed(hdp->base + HDP_GPOSET);
>> +
>> + pinctrl_pm_select_sleep_state(dev);
>> +
>> + clk_disable_unprepare(hdp->clk);
>> +
>> + return 0;
>> +}
>> +
>> +static int stm32_hdp_resume(struct device *dev)
>> +{
>> + struct stm32_hdp *hdp = dev_get_drvdata(dev);
>> + int err;
>> +
>> + err = clk_prepare_enable(hdp->clk);
>> + if (err)
>> + return dev_err_probe(hdp->dev, err, "Failed to prepare_enable clk");
>
>
> That's wrong, it is not a probe path.
Indeed
>
>> +
>> + writel_relaxed(HDP_CTRL_ENABLE, hdp->base + HDP_CTRL);
>> + writel_relaxed(hdp->gposet_conf, hdp->base + HDP_GPOSET);
>> + writel_relaxed(hdp->mux_conf, hdp->base + HDP_MUX);
>> +
>> + pinctrl_pm_select_default_state(dev);
>> +
>> + return 0;
>> +}
>> +
>> +DEFINE_SIMPLE_DEV_PM_OPS(stm32_hdp_pm_ops, stm32_hdp_suspend, stm32_hdp_resume);
>> +
>> +static struct platform_driver stm32_hdp_driver = {
>> + .probe = stm32_hdp_probe,
>> + .remove = stm32_hdp_remove,
>> + .driver = {
>> + .name = DRIVER_NAME,
>> + .pm = pm_sleep_ptr(&stm32_hdp_pm_ops),
>> + .of_match_table = stm32_hdp_of_match,
>> + }
>> +};
>> +
>> +module_platform_driver(stm32_hdp_driver);
>> +
>> +MODULE_ALIAS("platform:" DRIVER_NAME);
>
>
> You should not need MODULE_ALIAS() in normal cases. If you need it,
> usually it means your device ID table is wrong (e.g. misses either
> entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
> for incomplete ID table.
>
Ok
>
>
> Best regards,
> Krzysztof
Best regards,
Clément
Powered by blists - more mailing lists