<div dir="ltr"><div>Hello Joel,</div><div><br></div><div>Thanks, I will optimize the code following <span style="font-size:14px">Guenter's comments first.</span></div><div><span style="font-size:14px"><br></span></div><div><span style="font-size:14px">Cheers,</span></div><div><span style="font-size:14px"><br></span></div><div><span style="font-size:14px">Xiaohua</span></div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jan 18, 2023 at 11:56 AM Joel Stanley <<a href="mailto:joel@jms.id.au">joel@jms.id.au</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello Xiaohua,<br>
<br>
<br>
On Sat, 10 Dec 2022 at 17:54, Guenter Roeck <<a href="mailto:linux@roeck-us.net" target="_blank">linux@roeck-us.net</a>> wrote:<br>
><br>
> On 12/8/22 18:45, Wang Xiaohua wrote:<br>
> > Add mp2971/mp2973 support in mp2975<br>
<br>
Guenter has some comments for you to address. Are you planning on<br>
working on this further?<br>
<br>
I would like to help you get support for this device in the tree, but<br>
it will require some more work.<br>
<br>
Cheers,<br>
<br>
Joel<br>
<br>
> ><br>
> > Tested with:<br>
> > My unit only include mp2971 and mp2973 devices<br>
> > MP2973:<br>
> > cat /sys/bus/i2c/devices/5-0076/hwmon/hwmon24/*label<br>
> > iin<br>
> > iout1<br>
> > iout2<br>
> > vin<br>
> > vout1<br>
> > vout2<br>
> > pin<br>
> > pout1<br>
> > pout2<br>
> > cat /sys/bus/i2c/devices/5-0076/hwmon/hwmon24/*input<br>
> > 0<br>
> > 82500<br>
> > 14000<br>
> > 12187<br>
> > 1787<br>
> > 1793<br>
> > 0<br>
> > 148000000<br>
> > 25000000<br>
> > 54000<br>
> > MP2971:<br>
> > cat /sys/bus/i2c/devices/5-0062/hwmon/hwmon20/*label<br>
> > iin<br>
> > iout1<br>
> > iout2<br>
> > vin<br>
> > vout1<br>
> > vout2<br>
> > pin<br>
> > pout1<br>
> > pout2<br>
> > cat /sys/bus/i2c/devices/5-0062/hwmon/hwmon20/*input<br>
> > 18500<br>
> > 22000<br>
> > 500<br>
> > 12187<br>
> > 1025<br>
> > 1800<br>
> > 226000000<br>
> > 22000000<br>
> > 1000000<br>
> > 55000<br>
> ><br>
><br>
> Test results are not very useful. Better use something like<br>
> "grep . /sys/bus/i2c/devices/5-0062/hwmon/hwmon20/*".<br>
><br>
> Either case, test results should be provided after "---" and not be part<br>
> of the commit description. Instead, the commit description should explain<br>
> what those chips actually are.<br>
><br>
><br>
> > Signed-off-by: Wang Xiaohua <<a href="mailto:wangxiaohua.1217@bytedance.com" target="_blank">wangxiaohua.1217@bytedance.com</a>><br>
> ><br>
> > v2:<br>
> > - Fix auto build test WARNING<br>
> ><br>
> > v3:<br>
> > - Fix incorrect return code<br>
> > ---<br>
> > drivers/hwmon/pmbus/mp2975.c | 415 +++++++++++++++++++++++++++++++----<br>
> > 1 file changed, 374 insertions(+), 41 deletions(-)<br>
> ><br>
><br>
> Update to Documentation/hwmon/mp2975.rst and<br>
> Documentation/devicetree/bindings/trivial-devices.yaml required.<br>
><br>
> However, there is a more severe problem: The changes are too complex<br>
> for me to review, and the chip datasheets are not published. I can not evaluate<br>
> if the changes are really needed, if there is a less complex solution,<br>
> or if they even make sense. Someone with access to a datasheet will have<br>
> to step up as maintainer for this driver.<br>
><br>
> Additional comments inline.<br>
><br>
> Guenter<br>
><br>
> > diff --git a/drivers/hwmon/pmbus/mp2975.c b/drivers/hwmon/pmbus/mp2975.c<br>
> > index 51986adfbf47..4d1b7ac1800e 100644<br>
> > --- a/drivers/hwmon/pmbus/mp2975.c<br>
> > +++ b/drivers/hwmon/pmbus/mp2975.c<br>
> > @@ -52,10 +52,33 @@<br>
> > #define MP2975_MAX_PHASE_RAIL2 4<br>
> > #define MP2975_PAGE_NUM 2<br>
> ><br>
> > +#define MP2971_RAIL2_FUNC \<br>
> > + (PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_IOUT | \<br>
> > + PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_POUT)<br>
> > +<br>
> > #define MP2975_RAIL2_FUNC (PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | \<br>
> > PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | \<br>
> > PMBUS_HAVE_POUT | PMBUS_PHASE_VIRTUAL)<br>
> ><br>
> > +struct mp2971_device_info {<br>
> > + int max_phase_rail1;<br>
> > + int max_phase_rail2;<br>
> > + int imvp9_en_r1_mask;<br>
> > + int imvp9_en_r2_mask;<br>
> > +};<br>
> > +<br>
> > +struct mp2971_data {<br>
> > + struct pmbus_driver_info info;<br>
> > + int vid_step[MP2975_PAGE_NUM];<br>
> > + int vout_format[MP2975_PAGE_NUM];<br>
> > + int vout_mode[MP2975_PAGE_NUM];<br>
> > + int vout_exponent[MP2975_PAGE_NUM];<br>
> > + int max_phase_rail1;<br>
> > + int max_phase_rail2;<br>
> > + int imvp9_en_r1_mask;<br>
> > + int imvp9_en_r2_mask;<br>
> > +};<br>
> > +<br>
> > struct mp2975_data {<br>
> > struct pmbus_driver_info info;<br>
> > int vout_scale;<br>
> > @@ -68,6 +91,9 @@ struct mp2975_data {<br>
> > int curr_sense_gain[MP2975_PAGE_NUM];<br>
> > };<br>
> ><br>
> > +static const struct i2c_device_id mp2975_id[];<br>
> > +<br>
> > +#define to_mp2971_data(x) container_of(x, struct mp2971_data, info)<br>
> > #define to_mp2975_data(x) container_of(x, struct mp2975_data, info)<br>
> ><br>
> > static int mp2975_read_byte_data(struct i2c_client *client, int page, int reg)<br>
> > @@ -95,6 +121,40 @@ mp2975_read_word_helper(struct i2c_client *client, int page, int phase, u8 reg,<br>
> > return (ret > 0) ? ret & mask : ret;<br>
> > }<br>
> ><br>
> > +static int<br>
> > +mp2971_linear2direct(struct mp2971_data *data, int page, int val)<br>
> > +{<br>
> > + /* simple case */<br>
> > + if (val == 0)<br>
> > + return 0;<br>
> > +<br>
> > + /* LINEAR16 does not support negative voltages */<br>
> > + if (val < 0)<br>
> > + return 0;<br>
> > +<br>
> > + /*<br>
> > + * For a static exponents, we don't have a choice<br>
> > + * but to adjust the value to it.<br>
> > + */<br>
> > +<br>
> > + if (data->vout_exponent[page] < 0)<br>
> > + val <<= -data->vout_exponent[page];<br>
> > + else<br>
> > + val >>= data->vout_exponent[page];<br>
> > + return clamp_val(val, 0, 0xffff);<br>
> > +}<br>
> > +<br>
> > +static int<br>
> > +mp2971_vid2direct(struct mp2971_data *data, int page, int val)<br>
> > +{<br>
> > + int vrf = data->info.vrm_version[page];<br>
> > +<br>
> > + if (vrf == imvp9)<br>
> > + return (val + 29) * data->vid_step[page];<br>
> > +<br>
> > + return (val + 49) * data->vid_step[page];<br>
> > +}<br>
> > +<br>
><br>
> This looks suspicious. VID -> voltage calculations should be well<br>
> defined and be implemented in mp2975_vid2direct(). It is not entirely<br>
> clear why a second conversion function should be needed, and why it would<br>
> use different calculations with different results than those for<br>
> mp2975.<br>
><br>
> Example, for vrf == imvp9, 10mV step size, and vid==1:<br>
><br>
> mp2971: (1 + 29) * 10 = 30 * 10 = 300<br>
> mp2975: 200 + (1 - 1) * 10 = 200 + 0 = 200<br>
><br>
> vid = 0xff = 255:<br>
><br>
> mp2971: (255 + 29) * 10 = 284 * 10 = 2840<br>
> mp2975: 200 + (255 - 1) * 10 = 200 + 254 * 10 = 2740<br>
><br>
> Also questionable is how there could ever be an IMVP9 setting with 5mV<br>
> step size since IMVP9 explicitly specifies a step size of 10mV.<br>
> Also, the maximum voltage for IMVP9 is specified as 2.74V.<br>
><br>
> > static int<br>
> > mp2975_vid2direct(int vrf, int val)<br>
> > {<br>
> > @@ -214,6 +274,74 @@ mp2975_read_phases(struct i2c_client *client, struct mp2975_data *data,<br>
> > return ret;<br>
> > }<br>
> ><br>
> > +static int<br>
> > +mp2971_read_word_data(struct i2c_client *client, int page,<br>
> > + int phase, int reg)<br>
> > +{<br>
> > + const struct pmbus_driver_info *info = pmbus_get_driver_info(client);<br>
> > + struct mp2971_data *data = to_mp2971_data(info);<br>
> > + int ret;<br>
> > +<br>
> > + switch (reg) {<br>
> > + case PMBUS_OT_FAULT_LIMIT:<br>
> > + case PMBUS_VIN_OV_FAULT_LIMIT:<br>
> > + case PMBUS_VOUT_OV_FAULT_LIMIT:<br>
> > + case PMBUS_VOUT_UV_FAULT_LIMIT:<br>
> > + case PMBUS_READ_IOUT:<br>
> > + ret = mp2975_read_word_helper(client, page, phase,<br>
> > + reg, GENMASK(15, 0));<br>
> > + break;<br>
> > + case PMBUS_READ_VOUT:<br>
> > + ret = mp2975_read_word_helper(client, page, phase, reg,<br>
> > + GENMASK(11, 0));<br>
> > + if (ret < 0)<br>
> > + return ret;<br>
> > + /*<br>
> > + * READ_VOUT can be provided in VID or direct format. The<br>
> > + * format type is specified by bit 15 of the register<br>
> > + * MP2971_MFR_DC_LOOP_CTRL. The driver enforces VOUT direct<br>
> > + * format, since device allows to set the different formats for<br>
> > + * the different rails and also all VOUT limits registers are<br>
> > + * provided in a direct format. In case format is VID - convert<br>
> > + * to direct.<br>
> > + */<br>
> > + switch (data->vout_format[page]) {<br>
> > + case linear:<br>
> > + ret = mp2971_linear2direct(data, page, ret);<br>
> > + break;<br>
> > + case vid:<br>
> > + ret = mp2971_vid2direct(data, page, ret);<br>
> > + break;<br>
> > + case direct:<br>
> > + break;<br>
> > + default:<br>
> > + return -ENODATA;<br>
> > + }<br>
> > + break;<br>
> > + case PMBUS_UT_WARN_LIMIT:<br>
> > + case PMBUS_UT_FAULT_LIMIT:<br>
> > + case PMBUS_VIN_UV_WARN_LIMIT:<br>
> > + case PMBUS_VIN_UV_FAULT_LIMIT:<br>
> > + case PMBUS_VOUT_UV_WARN_LIMIT:<br>
> > + case PMBUS_VOUT_OV_WARN_LIMIT:<br>
> > + case PMBUS_VIN_OV_WARN_LIMIT:<br>
> > + case PMBUS_IIN_OC_FAULT_LIMIT:<br>
> > + case PMBUS_IOUT_OC_LV_FAULT_LIMIT:<br>
> > + case PMBUS_IIN_OC_WARN_LIMIT:<br>
> > + case PMBUS_IOUT_OC_WARN_LIMIT:<br>
> > + case PMBUS_IOUT_OC_FAULT_LIMIT:<br>
> > + case PMBUS_IOUT_UC_FAULT_LIMIT:<br>
> > + case PMBUS_POUT_OP_FAULT_LIMIT:<br>
> > + case PMBUS_POUT_OP_WARN_LIMIT:<br>
> > + case PMBUS_PIN_OP_WARN_LIMIT:<br>
> > + return -ENXIO;<br>
> > + default:<br>
> > + return -ENODATA;<br>
> > + }<br>
> > +<br>
> > + return ret;<br>
> > +}<br>
> > +<br>
><br>
> Much of that code seems duplicate from mp2975_read_word_data().<br>
> Without datasheets I can not determine if this really makes sense<br>
> and/or is needed, or if a single function can be used for all chips.<br>
><br>
> > static int mp2975_read_word_data(struct i2c_client *client, int page,<br>
> > int phase, int reg)<br>
> > {<br>
> > @@ -365,6 +493,63 @@ mp2975_set_phase_rail2(struct pmbus_driver_info *info, int num_phases)<br>
> > info->pfunc[MP2975_MAX_PHASE_RAIL1 - i] = PMBUS_HAVE_IOUT;<br>
> > }<br>
> ><br>
> > +static int mp2971_identify_multiphase(struct i2c_client *client,<br>
> > + struct mp2971_data *data,<br>
> > + struct pmbus_driver_info *info)<br>
> > +{<br>
> > + int ret;<br>
> > +<br>
> > + ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 2);<br>
> > + if (ret < 0)<br>
> > + return ret;<br>
> > +<br>
> > + /* Identify multiphase for rail 1 - could be from 1 to 12. */<br>
> > + ret = i2c_smbus_read_word_data(client, MP2975_MFR_VR_MULTI_CONFIG_R1);<br>
> > + if (ret <= 0)<br>
> > + return ret;<br>
> > +<br>
> > + info->phases[0] = ret & GENMASK(3, 0);<br>
> > +<br>
> > + /*<br>
> > + * The device provides a total of 8 PWM pins, and can be configured<br>
> > + * to different phase count applications for rail 1 and rail 2.<br>
> > + * Rail 1 can be set to 8 phases, while rail 2 can only be set to 4<br>
> > + * phases at most. When rail 1’s phase count is configured as 0, rail<br>
> > + * 1 operates with 1-phase DCM. When rail 2 phase count is configured<br>
> > + * as 0, rail 2 is disabled.<br>
> > + */<br>
> > + if (info->phases[0] > data->max_phase_rail1)<br>
> > + return -EINVAL;<br>
> > +<br>
> > + return 0;<br>
> > +}<br>
><br>
> Same here. The code is almost the same as mp2975_identify_multiphase().<br>
> Again, without datasheets I can not determine if this really makes sense<br>
> and/or is needed, or if a single function can be used for all chips.<br>
><br>
> This is a recurring problem. It appears that the patch maximizes the<br>
> changes against the current code instead of even trying to minimize them.<br>
> Without datasheet, it is impossible to compare the chips to check if an<br>
> implementation with fewer / less extensive changes would be warranted.<br>
><br>
> > +<br>
> > +static int<br>
> > +mp2971_identify_vid(struct i2c_client *client, struct mp2971_data *data,<br>
> > + struct pmbus_driver_info *info, u32 reg, int page,<br>
> > + u32 imvp_bit, u32 vr_bit)<br>
> > +{<br>
> > + int ret;<br>
> > +<br>
> > + /* Identify VID mode and step selection. */<br>
> > + ret = i2c_smbus_read_word_data(client, reg);<br>
> > + if (ret < 0)<br>
> > + return ret;<br>
> > +<br>
> > + if (ret & imvp_bit) {<br>
> > + info->vrm_version[page] = imvp9;<br>
> > + data->vid_step[page] = MP2975_PROT_DEV_OV_OFF;<br>
> > + } else if (ret & vr_bit) {<br>
> > + info->vrm_version[page] = vr12;<br>
> > + data->vid_step[page] = MP2975_PROT_DEV_OV_ON;<br>
> > + } else {<br>
> > + info->vrm_version[page] = vr13;<br>
> > + data->vid_step[page] = MP2975_PROT_DEV_OV_OFF;<br>
> > + }<br>
> > +<br>
> > + return 0;<br>
> > +}<br>
> > +<br>
> > static int<br>
> > mp2975_identify_multiphase(struct i2c_client *client, struct mp2975_data *data,<br>
> > struct pmbus_driver_info *info)<br>
> > @@ -428,6 +613,68 @@ mp2975_identify_vid(struct i2c_client *client, struct mp2975_data *data,<br>
> > return 0;<br>
> > }<br>
> ><br>
> > +static int<br>
> > +mp2971_identify_rails_vid(struct i2c_client *client, struct mp2971_data *data,<br>
> > + struct pmbus_driver_info *info)<br>
> > +{<br>
> > + int ret;<br>
> > +<br>
> > + ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 2);<br>
> > + if (ret < 0)<br>
> > + return ret;<br>
> > +<br>
> > + /* Identify VID mode for rail 1. */<br>
> > + ret = mp2971_identify_vid(client, data, info,<br>
> > + MP2975_MFR_VR_MULTI_CONFIG_R1, 0,<br>
> > + data->imvp9_en_r1_mask,<br>
> > + MP2975_VID_STEP_SEL_R1);<br>
> > + if (ret < 0)<br>
> > + return ret;<br>
> > +<br>
> > + /* Identify VID mode for rail 2, if connected. */<br>
> > + if (info->phases[1])<br>
> > + ret = mp2971_identify_vid(client, data, info,<br>
> > + MP2975_MFR_VR_MULTI_CONFIG_R2, 1,<br>
> > + data->imvp9_en_r2_mask,<br>
> > + MP2975_VID_STEP_SEL_R2);<br>
> > + return ret;<br>
> > +}<br>
> > +<br>
> > +static int mp2971_identify_vout_format(struct i2c_client *client,<br>
> > + struct mp2971_data *data,<br>
> > + struct pmbus_driver_info *info)<br>
> > +{<br>
> > + int i, ret, vout_mode;<br>
> > +<br>
> > + for (i = 0; i < info->pages; i++) {<br>
> > + ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, i);<br>
> > + if (ret < 0)<br>
> > + return ret;<br>
> > +<br>
> > + ret = i2c_smbus_read_byte_data(client, PMBUS_VOUT_MODE);<br>
> > + if (ret < 0)<br>
> > + return ret;<br>
> > +<br>
> > + vout_mode = ret;<br>
> > +<br>
> > + switch (vout_mode >> 5) {<br>
> > + case 0:<br>
> > + data->vout_format[i] = linear;<br>
> > + data->vout_exponent[i] = ((s8)(vout_mode << 3)) >> 3;<br>
> > + break;<br>
> > + case 1:<br>
> > + data->vout_format[i] = vid;<br>
> > + break;<br>
> > + case 2:<br>
> > + data->vout_format[i] = direct;<br>
> > + break;<br>
> > + default:<br>
> > + return -ENODEV;<br>
> > + }<br>
> > + }<br>
> > + return 0;<br>
> > +}<br>
> > +<br>
> > static int<br>
> > mp2975_identify_rails_vid(struct i2c_client *client, struct mp2975_data *data,<br>
> > struct pmbus_driver_info *info)<br>
> > @@ -659,6 +906,24 @@ mp2975_vout_per_rail_config_get(struct i2c_client *client,<br>
> > return 0;<br>
> > }<br>
> ><br>
> > +static struct pmbus_driver_info mp2971_info = {<br>
> > + .pages = 1,<br>
> > + .format[PSC_VOLTAGE_IN] = linear,<br>
> > + .format[PSC_VOLTAGE_OUT] = direct,<br>
> > + .format[PSC_TEMPERATURE] = linear,<br>
> > + .format[PSC_CURRENT_IN] = linear,<br>
> > + .format[PSC_CURRENT_OUT] = linear,<br>
> > + .format[PSC_POWER] = linear,<br>
> > + .m[PSC_VOLTAGE_OUT] = 1,<br>
> > + .R[PSC_VOLTAGE_OUT] = 3,<br>
> > + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |<br>
> > + PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |<br>
> > + PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP | PMBUS_HAVE_POUT |<br>
> > + PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT,<br>
> > + .read_byte_data = mp2975_read_byte_data,<br>
> > + .read_word_data = mp2971_read_word_data,<br>
> > +};<br>
> > +<br>
> > static struct pmbus_driver_info mp2975_info = {<br>
> > .pages = 1,<br>
> > .format[PSC_VOLTAGE_IN] = linear,<br>
> > @@ -683,63 +948,131 @@ static struct pmbus_driver_info mp2975_info = {<br>
> > static int mp2975_probe(struct i2c_client *client)<br>
> > {<br>
> > struct pmbus_driver_info *info;<br>
> > - struct mp2975_data *data;<br>
> > int ret;<br>
> > + char *name;<br>
> ><br>
> > - data = devm_kzalloc(&client->dev, sizeof(struct mp2975_data),<br>
> > - GFP_KERNEL);<br>
> > - if (!data)<br>
> > - return -ENOMEM;<br>
> > + name = (char *)i2c_match_id(mp2975_id, client)->name;<br>
> ><br>
> > - memcpy(&data->info, &mp2975_info, sizeof(*info));<br>
> > - info = &data->info;<br>
> > + if (!name)<br>
> > + return -EINVAL;<br>
> ><br>
> > - /* Identify multiphase configuration for rail 2. */<br>
> > - ret = mp2975_identify_multiphase_rail2(client);<br>
> > - if (ret < 0)<br>
> > - return ret;<br>
> > + if (!strcmp(name, "mp2971") || !strcmp(name, "mp2973")) {<br>
> > + struct mp2971_data *data;<br>
> > + struct mp2971_device_info *device_info;<br>
> ><br>
> > - if (ret) {<br>
> > - /* Two rails are connected. */<br>
> > - data->info.pages = MP2975_PAGE_NUM;<br>
> > - data->info.phases[1] = ret;<br>
> > - data->info.func[1] = MP2975_RAIL2_FUNC;<br>
> > - }<br>
> > + data = devm_kzalloc(&client->dev, sizeof(struct mp2971_data),<br>
> > + GFP_KERNEL);<br>
> > + if (!data)<br>
> > + return -ENOMEM;<br>
> ><br>
> > - /* Identify multiphase configuration. */<br>
> > - ret = mp2975_identify_multiphase(client, data, info);<br>
> > - if (ret)<br>
> > - return ret;<br>
> > + device_info =<br>
> > + (struct mp2971_device_info *)i2c_match_id(mp2975_id, client)<br>
> > + ->driver_data;<br>
> ><br>
> > - /* Identify VID setting per rail. */<br>
> > - ret = mp2975_identify_rails_vid(client, data, info);<br>
> > - if (ret < 0)<br>
> > - return ret;<br>
> > + memcpy(&data->info, &mp2971_info, sizeof(*info));<br>
> > + info = &data->info;<br>
> ><br>
> > - /* Obtain current sense gain of power stage. */<br>
> > - ret = mp2975_current_sense_gain_get(client, data);<br>
> > - if (ret)<br>
> > - return ret;<br>
> > + if (device_info) {<br>
> > + data->imvp9_en_r1_mask = device_info->imvp9_en_r1_mask;<br>
> > + data->imvp9_en_r2_mask = device_info->imvp9_en_r2_mask;<br>
> > + data->max_phase_rail1 = device_info->max_phase_rail1;<br>
> > + data->max_phase_rail2 = device_info->max_phase_rail2;<br>
> > + }<br>
> ><br>
> > - /* Obtain voltage reference values. */<br>
> > - ret = mp2975_vref_get(client, data, info);<br>
> > - if (ret)<br>
> > - return ret;<br>
> > + /* Identify multiphase configuration for rail 2. */<br>
> > + ret = mp2975_identify_multiphase_rail2(client);<br>
> > + if (ret < 0)<br>
> > + return ret;<br>
> ><br>
> > - /* Obtain vout over-voltage scales. */<br>
> > - ret = mp2975_vout_ov_scale_get(client, data, info);<br>
> > - if (ret < 0)<br>
> > - return ret;<br>
> > + if (ret) {<br>
> > + /* Two rails are connected. */<br>
> > + data->info.pages = MP2975_PAGE_NUM;<br>
> > + data->info.phases[1] = ret;<br>
> > + data->info.func[1] = MP2971_RAIL2_FUNC;<br>
> > + }<br>
> ><br>
> > - /* Obtain offsets, maximum and format for vout. */<br>
> > - ret = mp2975_vout_per_rail_config_get(client, data, info);<br>
> > - if (ret)<br>
> > - return ret;<br>
> > + /* Identify multiphase configuration. */<br>
> > + ret = mp2971_identify_multiphase(client, data, info);<br>
> > + if (ret)<br>
> > + return ret;<br>
> > +<br>
> > + /* Identify VID setting per rail. */<br>
> > + ret = mp2971_identify_rails_vid(client, data, info);<br>
> > + if (ret < 0)<br>
> > + return ret;<br>
> > +<br>
> > + /* Identify vout format. */<br>
> > + ret = mp2971_identify_vout_format(client, data, info);<br>
> > + if (ret < 0)<br>
> > + return ret;<br>
> > +<br>
> > + } else {<br>
> > + struct mp2975_data *data;<br>
> > +<br>
> > + data = devm_kzalloc(&client->dev, sizeof(struct mp2975_data),<br>
> > + GFP_KERNEL);<br>
> > + if (!data)<br>
> > + return -ENOMEM;<br>
> > +<br>
> > + memcpy(&data->info, &mp2975_info, sizeof(*info));<br>
> > + info = &data->info;<br>
> > +<br>
> > + /* Identify multiphase configuration for rail 2. */<br>
> > + ret = mp2975_identify_multiphase_rail2(client);<br>
> > + if (ret < 0)<br>
> > + return ret;<br>
> > +<br>
> > + if (ret) {<br>
> > + /* Two rails are connected. */<br>
> > + data->info.pages = MP2975_PAGE_NUM;<br>
> > + data->info.phases[1] = ret;<br>
> > + data->info.func[1] = MP2975_RAIL2_FUNC;<br>
> > + }<br>
> > +<br>
> > + /* Identify multiphase configuration. */<br>
> > + ret = mp2975_identify_multiphase(client, data, info);<br>
> > + if (ret)<br>
> > + return ret;<br>
> > +<br>
> > + /* Identify VID setting per rail. */<br>
> > + ret = mp2975_identify_rails_vid(client, data, info);<br>
> > + if (ret < 0)<br>
> > + return ret;<br>
> > +<br>
> > + /* Obtain current sense gain of power stage. */<br>
> > + ret = mp2975_current_sense_gain_get(client, data);<br>
> > + if (ret)<br>
> > + return ret;<br>
> > +<br>
> > + /* Obtain voltage reference values. */<br>
> > + ret = mp2975_vref_get(client, data, info);<br>
> > + if (ret)<br>
> > + return ret;<br>
> > +<br>
> > + /* Obtain vout over-voltage scales. */<br>
> > + ret = mp2975_vout_ov_scale_get(client, data, info);<br>
> > + if (ret < 0)<br>
> > + return ret;<br>
> > +<br>
> > + /* Obtain offsets, maximum and format for vout. */<br>
> > + ret = mp2975_vout_per_rail_config_get(client, data, info);<br>
> > + if (ret)<br>
> > + return ret;<br>
> > + }<br>
> ><br>
> > return pmbus_do_probe(client, info);<br>
> > }<br>
> ><br>
> > +static const struct mp2971_device_info mp2971_device_info = {<br>
> > + .imvp9_en_r1_mask = BIT(14),<br>
> > + .imvp9_en_r2_mask = BIT(13),<br>
> > + .max_phase_rail1 = 8,<br>
> > + .max_phase_rail2 = 4,<br>
> > +};<br>
> > +<br>
> > static const struct i2c_device_id mp2975_id[] = {<br>
> > + {"mp2971", (kernel_ulong_t)&mp2971_device_info },<br>
> > + {"mp2973", (kernel_ulong_t)&mp2971_device_info },<br>
> > {"mp2975", 0},<br>
> > {}<br>
> > };<br>
><br>
</blockquote></div></div>