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

Sasha Finkelstein <[email protected]> Mon, 27 Jul 2026 15:05:35 +0200
Newsgroups dev.linux.lists.asahi,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel,org.kernel.vger.linux-usb
Message-ID <[email protected]>
On Jul 27, 2026, at 14:56, Heikki Krogerus =
<[email protected]> wrote:
>=20
> Hi,
>=20
> On Sat, Jul 25, 2026 at 06:20:57PM +0200, Sasha Finkelstein wrote:
>> From: Alyssa Milburn <[email protected]>
>>=20
>> Make the core driver more bus-agnostic to prepare for SPMI variants =
of
>> the tipd chip
>>=20
>> Signed-off-by: Alyssa Milburn <[email protected]>
>> Signed-off-by: Sasha Finkelstein <[email protected]>
>> ---
>> drivers/usb/typec/tipd/Makefile           |   2 +-
>> drivers/usb/typec/tipd/{core.c =3D> core.h} | 109 =
++++++++++++++++++++++++++++----------------------------------------------=
-----------------------------------
>=20
> 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 =3D i2c_verify_client(dev);
>> + struct tps6598x *tps =3D i2c_get_clientdata(client);
>> + return tps;
>> +}
>=20
> 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 <[email protected]>
>> + */
>=20
> 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?

>=20
> Thanks,
>=20
> --=20
> heikki