[PATCH 5/5] sysreset: Support reset via Renesas RAA215300 PMIC

[Marek Vasut]

On 11/19/23 21:52, Paul Barker wrote:
> On Sun, Nov 19, 2023 at 09:17:40PM +0100, Marek Vasut wrote:
>> On 11/15/23 18:40, Paul Barker wrote:
>>> This patch allows us to reset the RZ/G2L board via the RAA215300 PMIC.
>>>
>>> Note that the RAA215300 documentation names the available reset types
>>> differently to u-boot:
>>>
>>>     - A "warm" reset via the RAA215300 PMIC will fully reset the SoC
>>>       (CPU & GPIOs), so this corresponds to SYSRESET_COLD.
>>>
>>>     - A "cold" reset via the RAA215300 PMIC will cycle all power supply
>>>       rails, so this corresponds to SYSRESET_POWER.
>>>
>>> Signed-off-by: Paul Barker <paul.barker.ct at bp.renesas.com>
>>> ---
>>>    board/renesas/rzg2l/rzg2l.c           |  8 ----
>>>    configs/renesas_rzg2l_smarc_defconfig |  2 +
>>>    drivers/power/pmic/raa215300.c        | 17 ++++++++
>>>    drivers/sysreset/Kconfig              |  6 +++
>>>    drivers/sysreset/Makefile             |  1 +
>>>    drivers/sysreset/sysreset_raa215300.c | 58 +++++++++++++++++++++++++++
>>>    6 files changed, 84 insertions(+), 8 deletions(-)
>>>    create mode 100644 drivers/sysreset/sysreset_raa215300.c
>>>
>>> diff --git a/board/renesas/rzg2l/rzg2l.c b/board/renesas/rzg2l/rzg2l.c
>>> index 73201a8c69e5..0f6d6e7f514f 100644
>>> --- a/board/renesas/rzg2l/rzg2l.c
>>> +++ b/board/renesas/rzg2l/rzg2l.c
>>> @@ -56,11 +56,3 @@ int board_init(void)
>>>    {
>>>    	return 0;
>>>    }
>>> -
>>> -void reset_cpu(void)
>>> -{
>>> -	/*
>>> -	 * TODO: Implement reset support once TrustedFirmware supports
>>> -	 * the appropriate call.
>>> -	 */
>>> -}
>>
>> Board change -- separate patch please.
> 
> Ok, this makes sense. I'll move this and the defconfig change into a
> separate patch.
> 
>>
>>> diff --git a/configs/renesas_rzg2l_smarc_defconfig b/configs/renesas_rzg2l_smarc_defconfig
>>> index b62eae4ee0a4..ba96e746df9e 100644
>>> --- a/configs/renesas_rzg2l_smarc_defconfig
>>> +++ b/configs/renesas_rzg2l_smarc_defconfig
>>> @@ -55,3 +55,5 @@ CONFIG_PMIC_RAA215300=y
>>>    CONFIG_DM_REGULATOR=y
>>>    CONFIG_DM_REGULATOR_FIXED=y
>>>    CONFIG_DM_REGULATOR_GPIO=y
>>> +CONFIG_SYSRESET=y
>>> +CONFIG_SYSRESET_RAA215300=y
>>> diff --git a/drivers/power/pmic/raa215300.c b/drivers/power/pmic/raa215300.c
>>> index 9c0b720994b2..7f68f95f25cf 100644
>>> --- a/drivers/power/pmic/raa215300.c
>>> +++ b/drivers/power/pmic/raa215300.c
>>> @@ -27,9 +27,26 @@ static const struct udevice_id raa215300_ids[] = {
>>>    	{ /* sentinel */ }
>>>    };
>>> +static int raa215300_bind(struct udevice *dev)
>>> +{
>>> +	struct driver *drv;
>>> +
>>> +	if (IS_ENABLED(CONFIG_SYSRESET_RAA215300)) {
>>> +		drv = lists_driver_lookup_name("raa215300_sysreset");
>>> +		if (!drv)
>>> +			return -ENOENT;
>>> +
>>> +		return device_bind(dev, drv, dev->name, NULL, dev_ofnode(dev),
>>> +				   NULL);
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>
>> Driver change should be squashed in 4/5.
> 
> Moving this to the previous patch doesn't make sense to me - the
> sysreset driver needs to exist for this check to be meaningful.
> Conversely, adding the sysreset driver first then the pmic driver
> wouldn't make sense since the sysreset driver depends on the pmic
> driver. This seemed the neatest way to do things to me.

Can't you split this into 4 or so patches , add the bind() callbacks in 
first two, then the if (IS_ENABLED(...)) in next two ?