Hi > -----Original Message----- > From: Mats Karrman [mailto:mats.dev.l...@gmail.com] > Sent: 2018年4月4日 20:07 > To: Jun Li <jun...@nxp.com>; gre...@linuxfoundation.org; robh...@kernel.org; > mark.rutl...@arm.com; heikki.kroge...@linux.intel.com; > hdego...@redhat.com > Cc: li...@roeck-us.net; rmf...@gmail.com; yueyao....@gmail.com; > linux-usb@vger.kernel.org; devicet...@vger.kernel.org; dl-linux-imx > <linux-...@nxp.com> > Subject: Re: [PATCH v2 2/5] usb: typec: fusb302: remove max_snk_* for sink > config > > Hi Li, > > On 2018-03-23 15:58, Li Jun wrote: > > > Since max_snk_* is to be deprecated, so remove max_snk_* by adding a > > variable PDO for sink config. > > > > Signed-off-by: Li Jun <jun...@nxp.com> > > --- > > drivers/usb/typec/fusb302/fusb302.c | 51 > +++++++++++++++++++++++++++---------- > > 1 file changed, 37 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/usb/typec/fusb302/fusb302.c > > b/drivers/usb/typec/fusb302/fusb302.c > > index 7036171..db4d9cd 100644 > > --- a/drivers/usb/typec/fusb302/fusb302.c > > +++ b/drivers/usb/typec/fusb302/fusb302.c > > @@ -120,6 +120,7 @@ struct fusb302_chip { > > enum typec_cc_polarity cc_polarity; > > enum typec_cc_status cc1; > > enum typec_cc_status cc2; > > + u32 snk_pdo[PDO_MAX_OBJECTS]; > > > > #ifdef CONFIG_DEBUG_FS > > struct dentry *dentry; > > @@ -1212,11 +1213,6 @@ static const u32 snk_pdo[] = { > > static const struct tcpc_config fusb302_tcpc_config = { > > .src_pdo = src_pdo, > > .nr_src_pdo = ARRAY_SIZE(src_pdo), > > - .snk_pdo = snk_pdo, > > - .nr_snk_pdo = ARRAY_SIZE(snk_pdo), > > - .max_snk_mv = 5000, > > - .max_snk_ma = 3000, > > - .max_snk_mw = 15000, > > .operating_snk_mw = 2500, > > .type = TYPEC_PORT_DRP, > > .data = TYPEC_PORT_DRD, > > @@ -1756,6 +1752,38 @@ static int init_gpio(struct fusb302_chip *chip) > > return 0; > > } > > > > +static int fusb302_composite_snk_pdo_array(struct fusb302_chip *chip) > > +{ > > + struct device *dev = chip->dev; > > + u32 mv, ma, mw, min_mv; > > + unsigned int i; > > + > > + /* Copy the static snk pdo */ > > + for (i = 0; i < ARRAY_SIZE(snk_pdo); i++) > > + chip->snk_pdo[i] = snk_pdo[i]; > > + > > + if (device_property_read_u32(dev, "fcs,max-sink-microvolt", &mv) || > > + device_property_read_u32(dev, "fcs,max-sink-microamp", &ma) || > > + device_property_read_u32(dev, "fcs,max-sink-microwatt", &mw)) > > + return i; > > + > > + mv = mv / 1000; > > + ma = ma / 1000; > > + mw = mw / 1000; > > + > > + min_mv = 1000 * chip->tcpc_config.operating_snk_mw / ma; > > + if (pdo_type(snk_pdo[i-1] == PDO_TYPE_FIXED)) > > You've got the parentheses wrong.
Good catch. > > Apart from that I don't like/understand why the PDO's should be fixed. This is only for legacy setting(a fixed PDO) and properties(fcs,max-sink-*), to make it can still work after using the new PDO matching policy. > Every product should be able to have its own settings, including the first > PDO (e.g. > it might need to specify a higher current and/or set the "Higher Capability" > bit). > > I think this would be best solved the same way as in your TCPCI driver patch > series [1] with a list freely specified by a property. > > [1] > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.s > pinics.net%2Flists%2Flinux-usb%2Fmsg167398.html&data=02%7C01%7Cjun.li%40 > nxp.com%7C94bfdd71fa87479f54c808d59a24873a%7C686ea1d3bc2b4c6fa92cd99c > 5c301635%7C0%7C0%7C636584404043415663&sdata=RbOoMtJM9XtmvhHYNQ9a > nhPMn%2BJpdqwY3UagRPCgM9k%3D&reserved=0 Agreed, new code(dt) should use[1] to describe the power properties. Fusb302 driver just need pass a fwnode at init, tcpm will read and parse all the properties when register port. Thanks Jun > > > + min_mv = max(min_mv, pdo_fixed_voltage(snk_pdo[i-1])); > > + else > > + min_mv = max(min_mv, pdo_max_voltage(snk_pdo[i-1])); > > + ma = min(ma, 1000 * mw / min_mv); > > + > > + /* Insert the new pdo to the end */ > > + chip->snk_pdo[i] = PDO_VAR(min_mv, mv, ma); > > + > > + return i+1; > > +} > > + > > static int fusb302_probe(struct i2c_client *client, > > const struct i2c_device_id *id) > > { > > @@ -1784,18 +1812,13 @@ static int fusb302_probe(struct i2c_client *client, > > chip->tcpc_dev.config = &chip->tcpc_config; > > mutex_init(&chip->lock); > > > > - if (!device_property_read_u32(dev, "fcs,max-sink-microvolt", &v)) > > - chip->tcpc_config.max_snk_mv = v / 1000; > > - > > - if (!device_property_read_u32(dev, "fcs,max-sink-microamp", &v)) > > - chip->tcpc_config.max_snk_ma = v / 1000; > > - > > - if (!device_property_read_u32(dev, "fcs,max-sink-microwatt", &v)) > > - chip->tcpc_config.max_snk_mw = v / 1000; > > - > > if (!device_property_read_u32(dev, "fcs,operating-sink-microwatt", &v)) > > chip->tcpc_config.operating_snk_mw = v / 1000; > > > > + /* Composite sink PDO */ > > + chip->tcpc_config.nr_snk_pdo = fusb302_composite_snk_pdo_array(chip); > > + chip->tcpc_config.snk_pdo = chip->snk_pdo; > > + > > /* > > * Devicetree platforms should get extcon via phandle (not yet > > * supported). On ACPI platforms, we get the name from a device prop. > >
