On 16/03/21 4:43 pm, Guenter Roeck wrote: > On 3/15/21 7:35 PM, Chris Packham wrote: >> The BPA-RS600 is a compact 600W AC to DC removable power supply module. >> >> Signed-off-by: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx> >> --- <snip> >> + >> +static int bpa_rs600_read_word_data(struct i2c_client *client, int page, >> + int phase, int reg)+{ >> + int ret; >> + >> + if (page > 0) >> + return -ENXIO; >> + >> + switch (reg) { >> + case PMBUS_VIN_UV_FAULT_LIMIT: >> + case PMBUS_VIN_OV_FAULT_LIMIT: >> + case PMBUS_VOUT_UV_FAULT_LIMIT: >> + case PMBUS_VOUT_OV_FAULT_LIMIT: >> + ret = -ENXIO; > Is that needed ? Why not -ENODATA ? Basically these commands get responses on the bus but they don't have valid data (nor are they documented in the datasheet). I'll add a comment to that effect. If I'm reading things correctly -ENODATA is a signal to _pmbus_read_word_data use the "normal" read operation. So I need to return something other than that. I found another driver (mp2975.c) doing the same thing for what I assume are similar reasons so I went with -ENXIO. > >> + break; <snip> > + >> +static const struct i2c_device_id bpa_rs600_id[] = { >> + { "bpa_rs600", 0 }, > Hmm, no, this has an underscore. Guess you'll have to use the trick from > iio_hwmon.c or similar to generate a valid name. > > Oh, wait, this is a pmbus driver, and the pmbus core uses client->name. > Maybe we need to add an optional strreplace() to the pmbus core. Looking into this now.