Re: [PATCH 2/3] usb: typec: tipd: Factor out i2c specifics

[Sasha Finkelstein]

On Jul 27, 2026, at 14:56, Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> wrote:
> 
> Hi,
> 
> On Sat, Jul 25, 2026 at 06:20:57PM +0200, Sasha Finkelstein wrote:
>> From: Alyssa Milburn <amilburn@xxxxxxxx>
>> 
>> Make the core driver more bus-agnostic to prepare for SPMI variants of
>> the tipd chip
>> 
>> Signed-off-by: Alyssa Milburn <amilburn@xxxxxxxx>
>> Signed-off-by: Sasha Finkelstein <k@xxxxxxxxxxxxxx>
>> ---
>> drivers/usb/typec/tipd/Makefile           |   2 +-
>> drivers/usb/typec/tipd/{core.c => core.h} | 109 ++++++++++++++++++++++++++++---------------------------------------------------------------------------------
> 
> That has to be a mistake, right? You don't move code into a header
> like that.

Yes, it was a bad idea, already fixed in a v2 that will be sent in the
near future.

>> +static struct tps6598x *tps6598x_from_device(struct device *dev)
>> +{
>> + struct i2c_client *client = i2c_verify_client(dev);
>> + struct tps6598x *tps = i2c_get_clientdata(client);
>> + return tps;
>> +}
> 
> You should not need anything like that.

This should make more sense together with the following patch, as it
can get the tps6598x from either the i2c or spmi backend.

>> --- /dev/null
>> +++ b/drivers/usb/typec/tipd/i2c.c
>> @@ -0,0 +1,86 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Driver for TI TPS6598x USB Power Delivery controller family
>> + *
>> + * Copyright (C) 2017, Intel Corporation
>> + * Author: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
>> + */
> 
> This driver is not coming from me.

I kept the attribution as both core and i2c are your driver, but split
into two. Should I have done something else?

> 
> Thanks,
> 
> -- 
> heikki