Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Tue, 3 Oct 2023 09:21:08 +0300
From:   Heikki Krogerus <heikki.krogerus@linux.intel.com>
To:     Abdel Alkuor <alkuor@gmail.com>
Cc:     krzysztof.kozlowski+dt@linaro.org, bryan.odonoghue@linaro.org,
        gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org, ryan.eleceng@gmail.com,
        robh+dt@kernel.org, conor+dt@kernel.org,
        devicetree@vger.kernel.org, Abdel Alkuor <abdelalkuor@geotab.com>
Subject: Re: [PATCH v9 07/14] USB: typec: tps6598x: Apply patch again after
 power resume
Message-ID: <ZRuy1NqA/VfWbBWn@kuha.fi.intel.com>
References: <20231001081134.37101-1-alkuor@gmail.com>
 <20231001081134.37101-8-alkuor@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231001081134.37101-8-alkuor@gmail.com>
Precedence: bulk

On Sun, Oct 01, 2023 at 04:11:27AM -0400, Abdel Alkuor wrote:
> From: Abdel Alkuor <abdelalkuor@geotab.com>
> 
> TPS25750 PD controller might be powered off externally at power suspend,
> after resuming PD controller power back, apply the patch again.
> 
> Signed-off-by: Abdel Alkuor <abdelalkuor@geotab.com>

This one looks also like something that should be part of the patch 4.

My concern is that with these separated features you are creating points
into the kernel git tree where TPS25750 is enabled, but it's not fully
functional, or even broken in scenarious like this (suspend/resume).
You can't do that unless you have some really good reason.

Since all of these add only a bit of code each, I think it would be
better to just merge these into the initial patch that enabled
TPS25750 - so I belive patch 4/14.

> ---
> Changes in v9:
>   - No changes
> Changes in v8:
>   - Use device_is_compatible instead of of_device_is_compatible
> Changes in v7:
>   - Add driver name to commit subject
> Changes in v6:
>   - Check tps25750 using is_compatiable device node
> Changes in v5:
>   - Incorporating tps25750 into tps6598x driver
>  drivers/usb/typec/tipd/core.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 2598433a69cf..32e42798688f 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -1203,6 +1203,17 @@ static int __maybe_unused tps6598x_resume(struct device *dev)
>  {
>  	struct i2c_client *client = to_i2c_client(dev);
>  	struct tps6598x *tps = i2c_get_clientdata(client);
> +	int ret;
> +
> +	ret = tps6598x_check_mode(tps);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (device_is_compatible(tps->dev, "ti,tps25750") && ret == TPS_MODE_PTCH) {
> +		ret = tps25750_apply_patch(tps);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	if (tps->wakeup) {
>  		disable_irq_wake(client->irq);
> -- 
> 2.34.1

-- 
heikki