Message-ID: <aEbuK5FeOgj4qBHu@p620>
Date: Mon, 9 Jun 2025 14:23:07 +0000
From: Kyle Swenson <kyle.swenson@....tech>
To: Oleksij Rempel <o.rempel@...gutronix.de>
CC: "kory.maincent@...tlin.com" <kory.maincent@...tlin.com>,
	"andrew+netdev@...n.ch" <andrew+netdev@...n.ch>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
	<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [RFC PATCH net-next 2/2] net: pse-pd: Add LTC4266 PSE controller
 driver

On Wed, Jun 04, 2025 at 10:39:17AM +0200, Oleksij Rempel wrote:
> Hi Kyle,
> 
> thank you for your work!
> 
> Are there any way to get manual with register description? I would like
> to make a deeper review :)

Yes, but unfortunately I think you'll need to make an account on the Analog Devices
website and go through the "Request Software" process for what they call
the "Software Interface" datasheet.

> On Tue, Jun 03, 2025 at 11:04:39PM +0000, Kyle Swenson wrote:
> > Add a new driver for the Linear Technology LTC4266 I2C Power Sourcing
> > Equipment controller.  This driver integrates with the current PSE
> > controller core, implementing IEEE802.3af and IEEE802.3at PSE standards.
[snip]
> > +#define LTC4266_TLIM_VALUE			0x01
> > +
> > +/* LTC4266_REG_HPEN, enable "High Power" mode (i.e. Type 2, 25.4W PDs) */
> 
> Type 2 Class 4? Probably not, datasheet claims:
> "Supports Proprietary Power Levels Above 25W"
Yes, Type 2 in this context means a Class 4 device.  I think the
comment in the datasheet is referring to the "4-Point PD detection
circuitry" but it's a little unclear.  That said, I've no intent in
supporting these proprietary power levels unless they're the same as
what's described in the IEEE spec.

> > +#define LTC4266_HPEN(_p)			BIT(_p)
> > +
> > +/* LTC4266_REG_MCONF */
> > +#define LTC4266_MCONF_INTERRUPT_ENABLE		BIT(7)
> > +
> > +/* LTC4266_REG_STATPWR */
> > +#define LTC4266_STATPWR_PG(_p)			BIT((_p) + 4)
> > +#define LTC4266_STATPWR_PE(_p)			BIT(_p)
> > +#define LTC4266_PORT_CLASS(_stat)		FIELD_GET(GENMASK(6, 4), (_stat))
> > +
> > +#define LTC4266_REG_ICUT_HP(_p)			(LTC4266_REG_HPMD(_p) + 1)
> > +
> > +/* if R_sense = 0.25 Ohm, this should be set otherwise 0 */
> > +#define LTC4266_ICUT_RSENSE			BIT(7)
> 
> LTC4266_ICUT_RSENSE_025_OHM
Ack.  I'll also fix up the indentation that's (now) obviously
misaligned.

> > +/* if set, halve the range and double the precision */
> > +#define LTC4266_ICUT_RANGE			BIT(6)
> > +
> > +#define LTC4266_ILIM_AF_RSENSE_025		0x80
> > +#define LTC4266_ILIM_AF_RSENSE_050		0x00
> > +#define LTC4266_ILIM_AT_RSENSE_025		0xC0
> > +#define LTC4266_ILIM_AT_RSENSE_050		0x40
> 
> Consider renaming constants AF/AT mentions.
> 
> Replace _AF_ with _TYPE1_ (e.g., LTC4266_ILIM_TYPE1_RSENSE_025)
> Replace _AT_ with _TYPE2_ (e.g., LTC4266_ILIM_TYPE2_RSENSE_025)
Sounds good, will do.

> The terms "Type 1" and "Type 2" are how the official IEEE 802.3 standard refers
> to the PoE capabilities and power levels that were introduced by the 802.3af
> and 802.3at amendments, respectively. Using "Type1" and "Type2" in your code
> will make it clearer and more aligned with the current, consolidated IEEE
> terminology, which is helpful since direct access to the original "af" and "at"
> amendment documents can be challenging for the open-source community.
Ah, makes sense.  I'll change AF/AT related bits to Type 1 and Type 2.

> Do you have access to this amendments?
I don't have them currently, but I'll ask around my organization and see if I can get access.

> > +/* LTC4266_REG_INTSTAT and LTC4266_REG_INTMASK */
> > +#define LTC4266_INT_SUPPLY			BIT(7)
> > +#define LTC4266_INT_TSTART			BIT(6)
> > +#define LTC4266_INT_TCUT			BIT(5)
> > +#define LTC4266_INT_CLASS			BIT(4)
> > +#define LTC4266_INT_DET				BIT(3)
> > +#define LTC4266_INT_DIS				BIT(2)
> > +#define LTC4266_INT_PWRGD			BIT(1)
> > +#define LTC4266_INT_PWRENA			BIT(0)
> > +
> > +#define LTC4266_MAX_PORTS 4
> > +
> > +/* Maximum and minimum power limits for a single port */
> > +#define LTC4266_PW_LIMIT_MAX 25400
> > +#define LTC4266_PW_LIMIT_MIN 1
> > +
> > +enum {
> > +	READ_CURRENT = 0,
> > +	READ_VOLTAGE = 2
> > +};
> > +
> > +enum {
> > +	LTC4266_OPMD_SHUTDOWN = 0,
> > +	LTC4266_OPMD_MANUAL,
> > +	LTC4266_OPMD_SEMI,
> > +	LTC4266_OPMD_AUTO
> 
> Please add explanations to this port modes
Will do, I see now the confusion this has caused.  Sorry for that.

> > +};
> > +
> > +/* Map LTC4266 Classification result to PD class */
> > +static int ltc4266_class_map[] = {
> > +	0, /* Treat as class 3 */
> > +	1,
> > +	2,
> > +	3,
> > +	4,
> > +	-EINVAL,
> > +	3, /* Treat as class 3 */
> > +	-ERANGE
> > +};
> > +
> > +/* Convert a class 0-4 to icut register value */
> > +static int ltc4266_class_to_icut[] = {
> > +	375,
> 
> missing comment, index 0 is class 3.
Ack.

> > +	112,
> > +	206,
> > +	375,
> > +	638
> > +};
> 
> May be we should have a generic function in the framework providing conversion
> from class to min/max Icut and Ilim, otherwise it makes additional work
> validation this values.
Sure, I'm open to something like this, even more so if other PSE
chipsets use a current limit instead of a power limit.

> > +
> > +enum sense_resistor {
> > +	LTC4266_RSENSE_500, /* Rsense 0.5 Ohm */
> > +	LTC4266_RSENSE_250 /* Rsense 0.25 Ohm */
> > +};
> > +
> > +struct ltc4266_port {
> > +	enum sense_resistor rsense;
> > +	struct device_node *node;
> > +	int current_limit;
> > +};
> > +
> > +struct ltc4266 {
> > +	struct i2c_client *client;
> > +	struct mutex lock; /* Protect Read-Modify-Write Sequences */
> > +	struct ltc4266_port *ports[LTC4266_MAX_PORTS];
> > +	struct device *dev;
> > +	struct device_node *np;
> > +	struct pse_controller_dev pcdev;
> > +};
> > +
> > +/* Read-modify-write sequence with value and mask.  Mask is expected to be
> > + * shifted to the correct spot.
> > + */
> > +static int ltc4266_write_reg(struct ltc4266 *ltc4266, u8 reg, u8 value, u8 mask)
> 
> If it is Read-modify-write type of function, it would be better to name
> it ltc4266_rmw_reg(). Or use regmap instead, you will get some extra
> functionality: register dump over sysfs interface, range validation,
> caching if enabled, locking, etc.
Oh, good call- I'll use regmap.

> > +{
> > +	int ret;
> > +	u8 new;
> > +
> > +	mutex_lock(&ltc4266->lock);
> > +	ret = i2c_smbus_read_byte_data(ltc4266->client, reg);
> > +	if (ret < 0) {
> > +		dev_warn(ltc4266->dev, "Failed to read register 0x%02x, err=%d\n", reg, ret);
> > +		mutex_unlock(&ltc4266->lock);
> > +		return ret;
> > +	}
> > +	new = (u8)ret;
> > +	new &= ~mask;
> > +	new |= value & mask;
> > +	ret = i2c_smbus_write_byte_data(ltc4266->client, reg, new);
> > +	mutex_unlock(&ltc4266->lock);
> > +
> > +	return ret;
> > +}
> > +
> > +static int ltc4266_read_iv(struct ltc4266 *ltc4266, int port, u8 iv)
> > +{
> > +	int lsb;
> > +	int msb;
> > +	int result;
> > +	int lsb_reg;
> > +	u64 ivbits = 0;
> > +
> > +	if (iv == READ_CURRENT)
> > +		lsb_reg = LTC4266_IPLSB_REG(port);
> > +	else if (iv == READ_VOLTAGE)
> > +		lsb_reg = LTC4266_VPLSB_REG(port);
> > +	else
> > +		return -EINVAL;
> > +
> > +	result = i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_STATPWR);
> > +	if (result < 0)
> > +		return result;
> > +
> > +	/*  LTC4266 IV readings are only valid if the port is powered. */
> > +	if (!(result & LTC4266_STATPWR_PG(port)))
> > +		return -EINVAL;
> 
> We have two states:
> - admin enabled: admin enabled state
> - delivering power: PSE is actually delivering power
> 
> Please use this words to clarify what is actually happening.
Will do.

> > +	/* LTC4266 expects the MSB register to be read immediately following the LSB
> > +	 * register, so we need to ensure other parts aren't reading other registers in
> > +	 * this chip while we read the current/voltage regulators.
> > +	 */
> > +	mutex_lock(&ltc4266->lock);
> 
> please use guard* variants for locking.
Ack.

> 
> > +
> > +	lsb = i2c_smbus_read_byte_data(ltc4266->client, lsb_reg);
> > +	msb = i2c_smbus_read_byte_data(ltc4266->client, lsb_reg + 1);
> > +
> > +	mutex_unlock(&ltc4266->lock);
> > +
> > +	if (lsb < 0)
> > +		return lsb;
> > +
> > +	if (msb < 0)
> > +		return msb;
> > +
> > +	ivbits = 0;
> > +	ivbits |= ((u8)msb) << 8 | ((u8)lsb);
> > +
> > +	if (iv == READ_CURRENT)
> > +		if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250) /* 122.07 uA/LSB */
> > +			result = DIV_ROUND_CLOSEST_ULL((ivbits * 122070), 1000);
> > +		else /* 61.035 uA/LSB */
> > +			result = DIV_ROUND_CLOSEST_ULL((ivbits * 61035), 1000);
> > +	else /* 5.835 mV/LSB == 5835 uV/LSB */
> > +		result = ivbits * 5835;
> > +
> > +	return result;
> > +}
> 
> > +static int ltc4266_port_set_ilim(struct ltc4266 *ltc4266, int port, int class)
> > +{
> > +	if (class > 4 || class < 0)
> > +		return -EINVAL;
> > +
> > +	/* We want to set 425 mA for class 3 and lower; 850 mA otherwise for IEEE compliance */
> > +	if (class < 4) {
> > +		/* Write 0x80 for 0.25 Ohm sense otherwise 0 */
> > +		if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250)
> > +			return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), LTC4266_ILIM_AF_RSENSE_025);
> > +		return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), LTC4266_ILIM_AF_RSENSE_050);
> > +	}
> > +
> > +	/* Class == 4 */
> > +	if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250)
> > +		return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), LTC4266_ILIM_AT_RSENSE_025);
> > +	/* Class == 4 and the sense resistor is 0.5 */
> > +	return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), LTC4266_ILIM_AT_RSENSE_050);
> 
> May be something like this:
> /*
>  * ltc4266_port_set_ilim - Set the active current limit (ILIM) for a port.
>  * @ltc4266: Pointer to the LTC4266 device structure.
>  * @port: The port number (0-3).
>  * @class: The detected PD class (0-4).
>  *
>  * This function configures the ILIM register (0x48, 0x4D, 0x52, 0x57)
>  * of the LTC4266. The ILIM value determines the threshold at which the
>  * PSE actively limits current to the PD. The chosen values are based on
>  * IEEE Std 802.3-2022 requirements and typical operational values for the
>  * LTC4266 controller.
>  *
>  * IEEE Std 802.3-2022, Table 33-11 specifies ILIM parameter ranges:
>  * - For Type 1 PSE operation (typically PD Classes 0-3):
>  * The minimum ILIM is 0.400A. This driver uses 425mA. This value fits
>  * within typical Type 1 ILIM specifications (e.g., 0.400A min to
>  * around 0.440A-0.500A max for the programmed steady-state limit).
>  *
>  * - For Type 2 PSE operation (typically PD Class 4):
>  * The minimum ILIM is 1.14 * ICable (or ~1.05 * IPort_max from other
>  * interpretations, e.g., ~0.630A to ~0.684A). This driver uses 850mA.
>  * This value meets the minimum requirement and is a supported operational
>  * current limit for high power modes in the LTC4266.
>  *
>  * The overall PSE current output must not exceed the time-dependent PSE
>  * upperbound template, IPSEUT(t), described in IEEE Std 802.3-2022,
>  * Equation (33-6). The programmed ILIM values (425mA/850mA) serve as the
>  * long-term current limit (Ilimmin segment of IPSEUT(t)) and are well
>  * within the higher short-term current allowances of that template (e.g., 1.75A).
>  *
>  * The specific register values written depend on the sense resistor
>  * (0.25 Ohm or 0.50 Ohm) as detailed in the LTC4266 datasheet (Table 5).
>  *
>  * Returns: ...
>  */
> static int ltc4266_port_set_ilim(struct ltc4266 *ltc4266, int port, int class)
> {
> 	u8 ilim_reg_val;
> 
> 	if (class > 4 || class < 0)
> 		return -EINVAL;
> 
> 	if (class < 4) {
> 		/* PD Class is 0, 1, 2, or 3 (Type 1 PSE operation).
> 		 * Set ILIM to 425mA.
> 		 */
> 		if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250) {
> 			/* Using 0.25 Ohm sense resistor. */
> 			ilim_reg_val = LTC4266_ILIM_TYPE1_RSENSE_025;
> 		} else {
> 			/* Using 0.50 Ohm sense resistor. */
> 			ilim_reg_val = LTC4266_ILIM_TYPE1_RSENSE_050;
> 		}
> 	} else {
> 		/* PD Class is 4 (Type 2 PSE operation).
> 		 * Set ILIM to 850mA.
> 		 */
> 		if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250) {
> 			/* Using 0.25 Ohm sense resistor. */
> 			ilim_reg_val = LTC4266_ILIM_TYPE2_RSENSE_025;
> 		} else {
> 			/* Using 0.50 Ohm sense resistor. */
> 			ilim_reg_val = LTC4266_ILIM_TYPE2_RSENSE_050;
> 		}
> 	}
> 
> 	/* Write the determined ILIM value to the appropriate port's ILIM register.
> 	 * The LTC4266_REG_ILIM(port) macro resolves to the correct register
> 	 * address for the given port (e.g., 0x48 for port 0, 0x4D for port 1, etc.).
> 	 */
> 	return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ILIM(port), ilim_reg_val);
> }
Wow, that is much more clear.  Thanks so much, I'll change accordingly.

> > +static int ltc4266_port_set_icut(struct ltc4266 *ltc4266, int port, int icut)
> > +{
> > +	u8 val;
> > +
> > +	if (icut > 850)
> 
> It looks like board specific limit:
> From the LTC4266 datasheet:
> "Non-standard applications that provide more current
> than the 850mA IEEE maximum may require heat sinking and other MOSFET design
> considerations."
Yes, it is board specific but also sounded to me like above 850mA would
violate the spec.

> > +		return -ERANGE;
> > +
> > +	val = (u8)(DIV_ROUND_CLOSEST((icut * 1000), 18750) & 0x3F);
> 
> I assume 18750 micro Amp, per step in the register value and 0x3f is the max
> mask for the bit field. In this case this register supports
> 0x3f * 18750 / 1000 = 1181mA
> 
> Please use defines to make it readable.
Will do.  I'll do this for all the other current/voltage conversions as
well. 

> > +
> > +	if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250)
> > +		val |= LTC4266_ICUT_RSENSE | LTC4266_ICUT_RANGE;
> > +
> > +	return i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_ICUT_HP(port), val);
> > +}
> > +
> > +static int ltc4266_port_mode(struct ltc4266 *ltc4266, int port, u8 opmd)
> > +{
> > +	if (opmd >= LTC4266_OPMD_AUTO)
> > +		return -EINVAL;
> > +
> > +	return ltc4266_write_reg(ltc4266, LTC4266_REG_OPMD, TWO_BIT_WORD_OFFSET(opmd, port),
> > +				TWO_BIT_WORD_MASK(port));
> > +}
> > +
> > +static int ltc4266_port_powered(struct ltc4266 *ltc4266, int port)
> 
> delivering or enabled?
Delivering.  I'll change this per your earlier comment.

> > +{
> > +	int result = i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_STATPWR);
> > +
> > +	if (result < 0)
> > +		return result;
> > +
> > +	return !!((result & LTC4266_STATPWR_PG(port)) && (result & LTC4266_STATPWR_PE(port)));
> > +}
> > +
> > +static int ltc4266_port_init(struct ltc4266 *ltc4266, int port)
> > +{
> > +	int ret;
> > +	u8 tlim_reg;
> > +	u8 tlim_mask;
> > +
> > +	/* Reset the port */
> > +	ret = i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_RSTPB, BIT(port));
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = ltc4266_port_mode(ltc4266, port, LTC4266_OPMD_SEMI);
> 
> Should we have disabled mode before all current limits configured?
Well, OPMD_SEMI means "Semi-Automatic" mode, which will detect and
classify connected PDs but not power them until instructed to do so by
software. 

> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* Enable high power mode on the port (802.3at+) */
> 
> 802.3at+? "Proprietary Power Levels Above 25W"?. Here we will need a discussion
> how to reflect a Proprietary Power levels in the UAPI.
I don't know there is value in this discussion because a) I don't know what the
datasheet is referring to about these "Proprietary Power Levels" and b)
I'm not really interested in adding support for any proprietary power
levels because I can't test them.  The LTC4266 can power Type 1 and Type
2 PDs.  There is a support for high-capacitance devices (seen in really
old Cisco IP Phones), but this support I left (or intended to) disabled
because I can't test that, either.
> 
> > +	ret = ltc4266_write_reg(ltc4266, LTC4266_REG_HPEN,
> > +				LTC4266_HPEN(port), LTC4266_HPEN(port));
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* Enable Ping-Pong Classification */
> 
> This is probably "2-event classification" described in Clause 33 of the
> IEEE Std 802.3-2022.
Yes, this is exactly that.
> 
> > +	ret = ltc4266_write_reg(ltc4266, LTC4266_REG_HPMD(port),
> > +				LTC4266_HPMD_PONGEN, LTC4266_HPMD_PONGEN);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	if (ltc4266->ports[port]->rsense == LTC4266_RSENSE_250)
> > +		ret = ltc4266_write_reg(ltc4266, LTC4266_REG_ICUT_HP(port),
> > +					LTC4266_ICUT_RSENSE, LTC4266_ICUT_RSENSE);
> > +	else
> > +		ret = ltc4266_write_reg(ltc4266, LTC4266_REG_ICUT_HP(port),
> > +					0, LTC4266_ICUT_RSENSE);
> > +
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	if (port <= 1)
> > +		tlim_reg = LTC4266_REG_TLIM12;
> > +	else
> > +		tlim_reg = LTC4266_REG_TLIM34;
> > +
> > +	if (port & BIT(0))
> > +		tlim_mask = GENMASK(7, 4);
> > +	else
> > +		tlim_mask = GENMASK(3, 0);
> > +
> > +	ret = ltc4266_write_reg(ltc4266, tlim_reg, LTC4266_TLIM_VALUE, tlim_mask);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* Enable disconnect detect. */
> > +	ret = ltc4266_write_reg(ltc4266, LTC4266_REG_DISENA, BIT(port), BIT(port));
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* Enable detection (low nibble), classification (high nibble) on the port */
> 
> This seems to correspond to ETHTOOL_C33_PSE_ADMIN_STATE_ENABLED 
Yes, the combination of OPMD_SEMI, PD Detection enabled and PD
classification enabled corresponds to ADMIN_STATE_ENABLED.
> 
> > +	ret = i2c_smbus_write_byte_data(ltc4266->client, LTC4266_REG_DETPB,
> > +					BIT(port + 4) | BIT(port));
> > +
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	dev_dbg(ltc4266->dev, "Port %d has been initialized\n", port);
> > +	return 0;
> > +}
> > +
> > +static int ltc4266_get_opmode(struct ltc4266 *ltc4266, int port)
> > +{
> > +	int ret;
> > +
> > +	ret = i2c_smbus_read_byte_data(ltc4266->client, LTC4266_REG_OPMD);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	switch (port) {
> > +	case 0:
> > +		return FIELD_GET(GENMASK(1, 0), ret);
> > +	case 1:
> > +		return FIELD_GET(GENMASK(3, 2), ret);
> > +	case 2:
> > +		return FIELD_GET(GENMASK(5, 4), ret);
> > +	case 3:
> > +		return FIELD_GET(GENMASK(7, 6), ret);
> > +	}
> > +	return -EINVAL;
> > +}
> > +
> > +static int ltc4266_pi_is_enabled(struct pse_controller_dev *pcdev, int id)
> > +{
> > +	int ret;
> > +	struct ltc4266 *ltc4266 = container_of(pcdev, struct ltc4266, pcdev);
> > +
> > +	ret = ltc4266_get_opmode(ltc4266, id);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	if (ret == LTC4266_OPMD_SEMI)
> > +		return 1; /*  If a port is in OPMODE SEMI, we'll just assume admin has it enabled */
> 
> From HW perspective, every mode except of LTC4266_OPMD_SHUTDOWN can be seen as
> admin state enabled. LTC4266_OPMD_MANUAL - is forced mode controlling
> power delivery manually.
In V2, I'll add a lot more information in comments and such that
describe these modes a little better, but until then:

OPMD_SHUTDOWN means that port is off and will not run any detection or
classification cycles regardless of other configuration.

OPMD_MANUAL means the port will run a single detect cycle after the
corresponding bit is set;  additionally, it will run a single
classification cycle after that bit is set.  The problem with
OPMD_MANUAL is that there is no state enforcement in hardware, meaning
software could instruct the PSE to apply power to a port that doesn't
have a valid PD attached, and will apply power regardless of the result
(if any) from detection/classification.  Frankly, OPMD_MANUAL scares me
and I see supporting it as only adding risk with no value.

OPMD_SEMI means the chip will detect and classify PDs on enabled ports
and will wait until software sets the corresponding bit to power on the
port and (critically) only will power on a port with a valid detection
and classification result.

OPMD_AUTO requires the chip be powered on with the "auto" pin high, and
will automatically detect, classify, and power any valid PD.  I don't
have a board with this configuration so I've not included that support.
It's a little unclear to me what is possible to configure from software
when the chip is in this mode.

> 
> I need to make stop here, i'll try to review the rest later.
Thanks so much for the amazing review thus far, I think I've got more
than enough to adjust for a V2.  

> Regards,
> Oleksij

Thanks again,
Kyle

Powered by blists - more mailing lists