Received-SPF: pass (google.com: domain of linux-kernel+bounces-17015-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <31a619c6-f3dc-4a31-93c8-1a24a92d88ed@wolfvision.net>
Date: Thu, 4 Jan 2024 18:15:17 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 0/4] usb: typec: tipd: add patch update support for
 tps6598x
Content-Language: en-US
To: Roger Quadros <rogerq@kernel.org>, Jai Luthra <j-luthra@ti.com>,
 Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>,
 linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, vigneshr@ti.com,
 d-gole@ti.com, nm@ti.com
References: <20231207-tps6598x_update-v2-0-f3cfcde6d890@wolfvision.net>
 <vmngazj6si7xxss7txenezkcukqje2glhvvs7ipdcx3vjiqvlk@ohmmhhhlryws>
 <2nqiaxakx6setx4tzgddnbjadbh7miegz5p6wamsbbiyrfuq3x@un2uxajbswkg>
 <e9e8dd9f-b11b-43fc-8d76-6734dbddb540@kernel.org>
 <b0963302-b498-4a81-b635-0b4faf02e83b@wolfvision.net>
 <5ed7ee66-ad7f-43ce-8550-a1a671cce9ad@kernel.org>
From: Javier Carrasco <javier.carrasco@wolfvision.net>
In-Reply-To: <5ed7ee66-ad7f-43ce-8550-a1a671cce9ad@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?a2c1cXNpVmhjbk5JY3AzTnB1WDZ1d00reERRVWE3Zno3M0NoaUZ0d3pITWZS?=
 =?utf-8?B?cU5YOEhxSjRvbm55RGtEWnp1SHpRcjR2SVhjUnAwdnd6eVc4UW9KQ04wWnh6?=
 =?utf-8?B?ZVBNbnZ3MHlnY050aERQMkZOd1VQcnpES3RoQVkxVEg3ZmdMVGV0N2c0TWlC?=
 =?utf-8?B?RG95WEpCQk5YT1dCM2YrYlpqUHhDV05nUllHaS9DZm5Ibm5QcHJVcmFZSG5I?=
 =?utf-8?B?SGdabFhaeUhrTTZ1SFZyVzkvbFZWazV5QlRQeDRHN2xLZFUxSFJ4bmNSQ0Vi?=
 =?utf-8?B?MUR4cU4wYm81Smw5L2h1K1orWlRJdExIMUFoT3R3UFlwRFl3clJoT2tPeGw1?=
 =?utf-8?B?Z0pvZkViNythRkJFUmNFc0NadEUxZTNRdXhZclUyclVNS2I0amNHZ2R4Qk1z?=
 =?utf-8?B?WXlsRnpNQU9TOHg3T2JSY25TUDE4RG5HRzFTRlIyNlNPUjBjTnpQSnJ2SS81?=
 =?utf-8?B?bXdFQ0hka2JRUk1oUS82S1ZESU1tWDdXT3NuRnk4VVBEbjlDMFJnaTNlRS95?=
 =?utf-8?B?clRhQ1lFZ2VJSko5NHVxdnhnSjg4Q3dqT2lOMUFqNlk5NnlYc2FLcmRKcFFZ?=
 =?utf-8?B?dFVQSFFWM0RwNzhTK0w3V0ZMN09kbEQ1cmRYb3BXR0UzYlVNRHRYS1hFd002?=
 =?utf-8?B?KzY2OXlRd2lmQ2dXMVk4VVdla2pBL0lWV0NBV2QyTGNYbWtZS2YvQ04xKzJO?=
 =?utf-8?B?Q2tENGp6ckFNbVRRSDhaOVB6TmFvZkNuclRjYzhkL0E4cUN5cHhIMmdabHFX?=
 =?utf-8?B?amxBeXVpcGhlcXR1ZjBlR1BtbDFQUHVXNjRyZm5YVFZIc1k0bkxPVkszcW1X?=
 =?utf-8?B?elBoUWhzSGZHT0hvTTgwaWtkTytPbjBBcEVaK0JCT3d4SE1NL05sM0R2Kzhl?=
 =?utf-8?B?RkZzU04zcEZ2NjlZd1I2R3JXcEIxNzRQVGxDK3kyeTRrVko2Z1N6TVI1R09X?=
 =?utf-8?B?UHFjRFVDM1YzdkdxcEQ4SlcwOXBHaE1RbXZhTTRhVTdrMVZhejBkOXE5eG10?=
 =?utf-8?B?VDlLTGpoTjZNdTVLUU9LRFVPN001cHFFaVVOeVp3OTJNM3dZejRMUTZ2cXo3?=
 =?utf-8?B?ZFRYeEdIRkp4bFFpN0cvUFB2SklZQk9rNXZ4UVM3NTNJbjRmdzFPNlNoU1Js?=
 =?utf-8?B?aC9rOTV1ZndEU2tjbW9nWVIzb1VONDl0dDJENytadDl6NEovMlVIa0NDV0NX?=
 =?utf-8?B?RStsMnVCRVJoZW1xa0Z2SEN1R282MHR1SHJGdFJyYWRDSk9HeDVpcEdNUHJ2?=
 =?utf-8?B?ODVQMGgwK2tiU2E2eGZVbUdkTkFyV1lva2ZWbnYxSmMyTEx1S0NtTjc5eURq?=
 =?utf-8?B?QmRNSnRjNnU3N2xzSlBrTkZaVzNtTVJrcXdka0NCa2MvZGliQmdUSWc4VU5h?=
 =?utf-8?B?UnByeDROQkZJL3ZBZDhxdjQ4d1VRUXlBck9OZTRzcGhMTktJK2FQTGcxN3Bl?=
 =?utf-8?B?eGZsckxqam9WMm10V2VSZEg0RE9LY1V3cUNOcVJYSXpMQy9relpGdThYeldu?=
 =?utf-8?B?QU43cTFlanA1b2JteGJTQk5SaVFncGJaWFJjTUE0WW4razZZR0hrNDcrRU1B?=
 =?utf-8?B?QWxzOStjODNHNEViRXR4dUxhdlVkZEpnZzR6ak9KMGd5QWZyVFV3RnE5N0VF?=
 =?utf-8?B?dXZERmlGZ00zUzhFallZTGxiaFFtQzJMRXlEc3N3bGxCZDNmZkVyQnNwandS?=
 =?utf-8?B?Yk9EbXlEQmE3b2x5dVI5K0JYRVE1SWlWUUdkS0I1YVFodm9yRkJPUHpyQzE1?=
 =?utf-8?B?QTlHTjFMY0JRb2pZcTE1d0RTbkNuNDA1eklBdTR3eFRqaWxzbDlwL1NYeDIw?=
 =?utf-8?B?eStITURrc2h0K1J1bVRPZGdnNDhDN1ZnbmI5ejV1NDRJeVRGVTZ5Z1F1aUVo?=
 =?utf-8?B?RzY3SVJtMjVyelp2MmZRQzllam1pWTFMK1hNSnMxajEydDBldXcrVWVoMVEw?=
 =?utf-8?B?bFltcUh1YmdCMWdwQk9YVkQ3MnF3elg4ai96UTBnMUlRU09nL2l1VmZFdy9y?=
 =?utf-8?B?aUE4YUxBREkwNUJqNTVYWGZqMitRSnhSV2hwUkoraEJRSjY5QzljcW9IZ0VR?=
 =?utf-8?B?WVhOVFh4TFFHckZCTWpScjY1dFpPMkhLUlRrM3NwUmdObjNFc0pUQjloaU9K?=
 =?utf-8?B?Wk9BdTd5UXVrVzBFTkRBcFJYQ2E4cVBPcW9QTXZKcFJyQU1VWlY1aFBRNTN3?=
 =?utf-8?B?dnMrekZMK0tOaVNtNjhmeUJmUWU0aEtqSmFsUlpQL2w5bHVzbVl0dmU4elhs?=
 =?utf-8?Q?HgYsQ7/HTpK47LWuIIjPJNczQJzTbHuiL/0c8mg9C8=3D?=
X-OriginatorOrg: wolfvision.net
X-MS-Exchange-CrossTenant-Network-Message-Id: b93e6052-4d1f-40b6-18d0-08dc0d48ba6f
X-MS-Exchange-CrossTenant-AuthSource: VE1PR08MB4974.eurprd08.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jan 2024 17:15:19.7883
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: e94ec9da-9183-471e-83b3-51baa8eb804f
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 8GHNeUYzAz7geuXNCJuYvDDwC9KTe6m+NxIJkUh9phlx9W1cJmgh481Y0Gmr2JT9ySjR2JEUPqEv6aIlwSSXmE+y7nmnTFoFqzYngVtXIBE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PR3PR08MB5803

On 04.01.24 18:08, Roger Quadros wrote:
> 
> 
> On 04/01/2024 18:36, Javier Carrasco wrote:
>> 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.
> 
I have to admit that the GAID for the tps6598x should have been added
separately, and not right away with the function pointers. I added extra
functionality that was not expected. On the other hand, the GAID command
is supported by the tps6598x as well (Technical Reference Manual, page
113). Hence why I was surprised that using the same command for the
tps6598x is a problem.