On 04/01/2024 18:36, Javier Carrasco wrote:
> On 04.01.24 17:15, Roger Quadros wrote:
>>
>>
>> On 04/01/2024 17:47, Jai Luthra wrote:
>>> Hi Javier,
>>> The following change seems to fix boot on SK-AM62A without reverting
>>> this whole series:
>>>
>>> ------------------
>>>
>>> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
>>> index a956eb976906a5..8ba2aa05db519b 100644
>>> --- a/drivers/usb/typec/tipd/core.c
>>> +++ b/drivers/usb/typec/tipd/core.c
>>> @@ -1223,11 +1223,16 @@ static int cd321x_reset(struct tps6598x *tps)
>>> return 0;
>>> }
>>>
>>> -static int tps6598x_reset(struct tps6598x *tps)
>>> +static int tps25750_reset(struct tps6598x *tps)
>>> {
>>> return tps6598x_exec_cmd_tmo(tps, "GAID", 0, NULL, 0, NULL, 2000, 0);
>>> }
>>>
>>> +static int tps6598x_reset(struct tps6598x *tps)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> static int
>>> tps25750_register_port(struct tps6598x *tps, struct fwnode_handle *fwnode)
>>> {
>>> @@ -1545,7 +1550,7 @@ static const struct tipd_data tps25750_data = {
>>> .trace_status = trace_tps25750_status,
>>> .apply_patch = tps25750_apply_patch,
>>> .init = tps25750_init,
>>> - .reset = tps6598x_reset,
>>> + .reset = tps25750_reset,
>>> };
>>>
>>> static const struct of_device_id tps6598x_of_match[] = {
>>>
>>> ------------------
>>>
>>> I am not an expert on this, will let you/others decide on what should be
>>> the correct way to reset TPS6598x for patching without breaking this SK.
>>>
>>>
>>
>> This looks like a correct fix to me.
>> Could you please send a proper PATCH with Fixes tag? Thanks!
>>
> Hi Roger,
>
> that fix only removes the reset function and does nothing instead, but
> the reset call is identical for both devices (hence why there was a
> single function for both devices). As I mentioned in my reply to Jai
This is incorrect.
Look at the original code, the GAID command is issued only if it is a
tps25750 device.
Below is your part of the code that replaces it with reset() ops.
> @@ -1340,8 +1360,8 @@ static int tps6598x_probe(struct i2c_client *client)
> tps6598x_write64(tps, TPS_REG_INT_MASK1, 0);
> err_reset_controller:
> /* Reset PD controller to remove any applied patch */
> - if (is_tps25750)
> - tps6598x_exec_cmd_tmo(tps, "GAID", 0, NULL, 0, NULL, 2000, 0);
> + tps->data->reset(tps);
> +
> return ret;
> }
which means the GAID command will be executed for both tps25750 and tps6598x
as you have a single reset function for both.
This is a problem for boards using the tps6598x.
--
cheers,
-roger
