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;
Message-ID: <cd19fb5b-7c7e-a33c-9572-4ddbf9dce2d4@foss.st.com>
Date:   Thu, 6 Apr 2023 09:27:57 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH 4/5] remoteproc: stm32: Allow hold boot management by the
 SCMI reset controller
To:     Peng Fan <peng.fan@nxp.com>,
        Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>,
        Bjorn Andersson <andersson@kernel.org>,
        Mathieu Poirier <mathieu.poirier@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
CC:     "linux-remoteproc@vger.kernel.org" <linux-remoteproc@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-stm32@st-md-mailman.stormreply.com" 
        <linux-stm32@st-md-mailman.stormreply.com>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
References: <20230331154651.3107173-1-arnaud.pouliquen@foss.st.com>
 <20230331154651.3107173-5-arnaud.pouliquen@foss.st.com>
 <DU0PR04MB941747DDF6FD2F157A24183288939@DU0PR04MB9417.eurprd04.prod.outlook.com>
 <6bfaef82-458e-256d-b9ba-d6d84c574d4b@foss.st.com>
 <DU0PR04MB94170A9BD5F46D4157196D0888919@DU0PR04MB9417.eurprd04.prod.outlook.com>
Content-Language: en-US
From:   Alexandre TORGUE <alexandre.torgue@foss.st.com>
In-Reply-To: <DU0PR04MB94170A9BD5F46D4157196D0888919@DU0PR04MB9417.eurprd04.prod.outlook.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi

On 4/6/23 07:16, Peng Fan wrote:
>> Subject: Re: [PATCH 4/5] remoteproc: stm32: Allow hold boot management
>> by the SCMI reset controller
>>
>>
>>
>> On 4/4/23 06:55, Peng Fan wrote:
>>>> Subject: [PATCH 4/5] remoteproc: stm32: Allow hold boot management
>> by
>>>> the SCMI reset controller
>>>>
>>>> The hold boot can be managed by the SCMI controller as a reset.
>>>> If the "hold_boot" reset is defined in the device tree, use it.
>>>> Else use the syscon controller directly to access to the register.
>>>>
>>>> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
>>>> ---
>>>>   drivers/remoteproc/stm32_rproc.c | 34
>>>> ++++++++++++++++++++++++++-----
>>>> -
>>>>   1 file changed, 28 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/remoteproc/stm32_rproc.c
>>>> b/drivers/remoteproc/stm32_rproc.c
>>>> index 4be651e734ee..6b0d8f30a5c7 100644
>>>> --- a/drivers/remoteproc/stm32_rproc.c
>>>> +++ b/drivers/remoteproc/stm32_rproc.c
>>>> @@ -78,6 +78,7 @@ struct stm32_mbox {
>>>>
>>>>   struct stm32_rproc {
>>>>   	struct reset_control *rst;
>>>> +	struct reset_control *hold_boot_rst;
>>>>   	struct stm32_syscon hold_boot;
>>>>   	struct stm32_syscon pdds;
>>>>   	struct stm32_syscon m4_state;
>>>> @@ -398,6 +399,14 @@ static int stm32_rproc_set_hold_boot(struct
>>>> rproc *rproc, bool hold)
>>>>   	struct stm32_syscon hold_boot = ddata->hold_boot;
>>>>   	int val, err;
>>>>
>>>> +	if (ddata->hold_boot_rst) {
>>>> +		/* Use the SCMI reset controller */
>>>> +		if (!hold)
>>>> +			return reset_control_deassert(ddata-
>>>>> hold_boot_rst);
>>>> +		else
>>>> +			return reset_control_assert(ddata->hold_boot_rst);
>>>> +	}
>>>> +
>>>>   	val = hold ? HOLD_BOOT : RELEASE_BOOT;
>>>>
>>>>   	err = regmap_update_bits(hold_boot.map, hold_boot.reg, @@ -
>>>> 693,16 +702,29 @@ static int stm32_rproc_parse_dt(struct
>>>> platform_device *pdev,
>>>>   		dev_info(dev, "wdg irq registered\n");
>>>>   	}
>>>>
>>>> -	ddata->rst = devm_reset_control_get_by_index(dev, 0);
>>>> +	ddata->rst = devm_reset_control_get(dev, "mcu_rst");
>>> [Peng Fan]
>>> This may break legacy device tree.
>>
>> That partially true by the fact that I impose the "reset-names" property (but
>> also suppress the "st,syscfg-tz" property)
>>
>> But this should not be the case as the arch/arm/boot/dts/stm32mp151.dtsi
>> is updated in patch 2/5. The DTS files associated to this chip include it.
> 
> DT  maintainers may comment, from my understanding is updating driver
> should not break legacy dts, saying 5.15, 5.10 dts.

An old DT should continue to work with a new kernel.

Alex

> 
> Regards,
> Peng.
> 
>>
>> Thanks,
>> Arnaud
>>
>>
>>>
>>> Regards,
>>> Peng.
>>>
>>>>   	if (IS_ERR(ddata->rst))
>>>>   		return dev_err_probe(dev, PTR_ERR(ddata->rst),
>>>>   				     "failed to get mcu_reset\n");
>>>>
>>>> -	err = stm32_rproc_get_syscon(np, "st,syscfg-holdboot",
>>>> -				     &ddata->hold_boot);
>>>> -	if (err) {
>>>> -		dev_err(dev, "failed to get hold boot\n");
>>>> -		return err;
>>>> +	ddata->hold_boot_rst = devm_reset_control_get(dev, "hold_boot");
>>>> +	if (IS_ERR(ddata->hold_boot_rst)) {
>>>> +		if (PTR_ERR(ddata->hold_boot_rst) == -EPROBE_DEFER)
>>>> +			return PTR_ERR(ddata->hold_boot_rst);
>>>> +		ddata->hold_boot_rst = NULL;
>>>> +	}
>>>> +
>>>> +	if (!ddata->hold_boot_rst) {
>>>> +		/*
>>>> +		 * If the hold boot is not managed by the SCMI reset
>>>> controller,
>>>> +		 * manage it through the syscon controller
>>>> +		 */
>>>> +		err = stm32_rproc_get_syscon(np, "st,syscfg-holdboot",
>>>> +					     &ddata->hold_boot);
>>>> +		if (err) {
>>>> +			dev_err(dev, "failed to get hold boot\n");
>>>> +			return err;
>>>> +		}
>>>>   	}
>>>>
>>>>   	err = stm32_rproc_get_syscon(np, "st,syscfg-pdds", &ddata->pdds);
>>>> --
>>>> 2.25.1
>>>