| Message ID | 20230728082527.3531940-1-Naresh.Solanki@9elements.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | hwmon: (pmbus/mp2975) Fix PGOOD in READ_STATUS_WORD | expand |
On 7/28/23 01:25, Naresh Solanki wrote: > From: Patrick Rudolph <patrick.rudolph@9elements.com> > > MPS returns PGOOD instead of PB_STATUS_POWER_GOOD_N. > Fix that in the read_word_data hook. > Datasheets are not public, so I can not verify, but the code below only handles MP2973 and MP2971. What about MP2975 ? Either case, this appears to affect most Monolithic chips, but not all of them. Please limit the statement to known to be affected chips. > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> > Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> > --- > drivers/hwmon/pmbus/mp2975.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/hwmon/pmbus/mp2975.c b/drivers/hwmon/pmbus/mp2975.c > index 28f33f4618fa..410b6eae6d02 100644 > --- a/drivers/hwmon/pmbus/mp2975.c > +++ b/drivers/hwmon/pmbus/mp2975.c > @@ -297,6 +297,10 @@ static int mp2973_read_word_data(struct i2c_client *client, int page, > int ret; > > switch (reg) { > + case PMBUS_STATUS_WORD: > + ret = pmbus_read_word_data(client, page, phase, reg); if (ret < 0) return ret; Please add a comment explaining what is going on. > + ret ^= PB_STATUS_POWER_GOOD_N; > + break; > case PMBUS_OT_FAULT_LIMIT: > ret = mp2975_read_word_helper(client, page, phase, reg, > GENMASK(7, 0)); > > base-commit: a4bb1ce5b9e3ca403c04e20ebeae77fd6447cb11
Hi Guenter, On Fri, 28 Jul 2023 at 15:30, Guenter Roeck <linux@roeck-us.net> wrote: > > On 7/28/23 01:25, Naresh Solanki wrote: > > From: Patrick Rudolph <patrick.rudolph@9elements.com> > > > > MPS returns PGOOD instead of PB_STATUS_POWER_GOOD_N. > > Fix that in the read_word_data hook. > > > Datasheets are not public, so I can not verify, but the code below > only handles MP2973 and MP2971. What about MP2975 ? I dont have MP2975 to even verify. I'm not sure if its applicable. > > Either case, this appears to affect most Monolithic chips, but > not all of them. Please limit the statement to known to be affected > chips. Yes. With what we observed, this affects mp2973 & mp2971. Will update accordingly. > > > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> > > Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> > > --- > > drivers/hwmon/pmbus/mp2975.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/hwmon/pmbus/mp2975.c b/drivers/hwmon/pmbus/mp2975.c > > index 28f33f4618fa..410b6eae6d02 100644 > > --- a/drivers/hwmon/pmbus/mp2975.c > > +++ b/drivers/hwmon/pmbus/mp2975.c > > @@ -297,6 +297,10 @@ static int mp2973_read_word_data(struct i2c_client *client, int page, > > int ret; > > > > switch (reg) { > > + case PMBUS_STATUS_WORD: > > + ret = pmbus_read_word_data(client, page, phase, reg); > > if (ret < 0) > return ret; > > Please add a comment explaining what is going on. Sure. > > > + ret ^= PB_STATUS_POWER_GOOD_N; > > + break; > > case PMBUS_OT_FAULT_LIMIT: > > ret = mp2975_read_word_helper(client, page, phase, reg, > > GENMASK(7, 0)); > > > > base-commit: a4bb1ce5b9e3ca403c04e20ebeae77fd6447cb11 >
On 7/28/23 04:28, Naresh Solanki wrote: > Hi Guenter, > > > On Fri, 28 Jul 2023 at 15:30, Guenter Roeck <linux@roeck-us.net> wrote: >> >> On 7/28/23 01:25, Naresh Solanki wrote: >>> From: Patrick Rudolph <patrick.rudolph@9elements.com> >>> >>> MPS returns PGOOD instead of PB_STATUS_POWER_GOOD_N. >>> Fix that in the read_word_data hook. >>> >> Datasheets are not public, so I can not verify, but the code below >> only handles MP2973 and MP2971. What about MP2975 ? > I dont have MP2975 to even verify. I'm not sure if its applicable. >> >> Either case, this appears to affect most Monolithic chips, but >> not all of them. Please limit the statement to known to be affected >> chips. > Yes. With what we observed, this affects mp2973 & mp2971. > Will update accordingly. Please also add a comment that MP2975 is likely affected but can not be confirmed due to lack of a datasheet. Thanks, Guenter >> >>> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> >>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> >>> --- >>> drivers/hwmon/pmbus/mp2975.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/drivers/hwmon/pmbus/mp2975.c b/drivers/hwmon/pmbus/mp2975.c >>> index 28f33f4618fa..410b6eae6d02 100644 >>> --- a/drivers/hwmon/pmbus/mp2975.c >>> +++ b/drivers/hwmon/pmbus/mp2975.c >>> @@ -297,6 +297,10 @@ static int mp2973_read_word_data(struct i2c_client *client, int page, >>> int ret; >>> >>> switch (reg) { >>> + case PMBUS_STATUS_WORD: >>> + ret = pmbus_read_word_data(client, page, phase, reg); >> >> if (ret < 0) >> return ret; >> >> Please add a comment explaining what is going on. > Sure. >> >>> + ret ^= PB_STATUS_POWER_GOOD_N; >>> + break; >>> case PMBUS_OT_FAULT_LIMIT: >>> ret = mp2975_read_word_helper(client, page, phase, reg, >>> GENMASK(7, 0)); >>> >>> base-commit: a4bb1ce5b9e3ca403c04e20ebeae77fd6447cb11 >>
On 7/28/23 04:28, Naresh Solanki wrote: > Hi Guenter, > > > On Fri, 28 Jul 2023 at 15:30, Guenter Roeck <linux@roeck-us.net> wrote: >> >> On 7/28/23 01:25, Naresh Solanki wrote: >>> From: Patrick Rudolph <patrick.rudolph@9elements.com> >>> >>> MPS returns PGOOD instead of PB_STATUS_POWER_GOOD_N. >>> Fix that in the read_word_data hook. >>> >> Datasheets are not public, so I can not verify, but the code below >> only handles MP2973 and MP2971. What about MP2975 ? > I dont have MP2975 to even verify. I'm not sure if its applicable. Maybe Vadim can check / confirm. Guenter >> >> Either case, this appears to affect most Monolithic chips, but >> not all of them. Please limit the statement to known to be affected >> chips. > Yes. With what we observed, this affects mp2973 & mp2971. > Will update accordingly. >> >>> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> >>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> >>> --- >>> drivers/hwmon/pmbus/mp2975.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/drivers/hwmon/pmbus/mp2975.c b/drivers/hwmon/pmbus/mp2975.c >>> index 28f33f4618fa..410b6eae6d02 100644 >>> --- a/drivers/hwmon/pmbus/mp2975.c >>> +++ b/drivers/hwmon/pmbus/mp2975.c >>> @@ -297,6 +297,10 @@ static int mp2973_read_word_data(struct i2c_client *client, int page, >>> int ret; >>> >>> switch (reg) { >>> + case PMBUS_STATUS_WORD: >>> + ret = pmbus_read_word_data(client, page, phase, reg); >> >> if (ret < 0) >> return ret; >> >> Please add a comment explaining what is going on. > Sure. >> >>> + ret ^= PB_STATUS_POWER_GOOD_N; >>> + break; >>> case PMBUS_OT_FAULT_LIMIT: >>> ret = mp2975_read_word_helper(client, page, phase, reg, >>> GENMASK(7, 0)); >>> >>> base-commit: a4bb1ce5b9e3ca403c04e20ebeae77fd6447cb11 >>
Hi On Fri, 28 Jul 2023 at 19:56, Guenter Roeck <linux@roeck-us.net> wrote: > > On 7/28/23 04:28, Naresh Solanki wrote: > > Hi Guenter, > > > > > > On Fri, 28 Jul 2023 at 15:30, Guenter Roeck <linux@roeck-us.net> wrote: > >> > >> On 7/28/23 01:25, Naresh Solanki wrote: > >>> From: Patrick Rudolph <patrick.rudolph@9elements.com> > >>> > >>> MPS returns PGOOD instead of PB_STATUS_POWER_GOOD_N. > >>> Fix that in the read_word_data hook. > >>> > >> Datasheets are not public, so I can not verify, but the code below > >> only handles MP2973 and MP2971. What about MP2975 ? > > I dont have MP2975 to even verify. I'm not sure if its applicable. > >> > >> Either case, this appears to affect most Monolithic chips, but > >> not all of them. Please limit the statement to known to be affected > >> chips. > > Yes. With what we observed, this affects mp2973 & mp2971. > > Will update accordingly. > > Please also add a comment that MP2975 is likely affected but can not be > confirmed due to lack of a datasheet. I have the datasheet for MP2975 but haven't verified it on real HW. Regards, Naresh > > Thanks, > Guenter > > >> > >>> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> > >>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> > >>> --- > >>> drivers/hwmon/pmbus/mp2975.c | 4 ++++ > >>> 1 file changed, 4 insertions(+) > >>> > >>> diff --git a/drivers/hwmon/pmbus/mp2975.c b/drivers/hwmon/pmbus/mp2975.c > >>> index 28f33f4618fa..410b6eae6d02 100644 > >>> --- a/drivers/hwmon/pmbus/mp2975.c > >>> +++ b/drivers/hwmon/pmbus/mp2975.c > >>> @@ -297,6 +297,10 @@ static int mp2973_read_word_data(struct i2c_client *client, int page, > >>> int ret; > >>> > >>> switch (reg) { > >>> + case PMBUS_STATUS_WORD: > >>> + ret = pmbus_read_word_data(client, page, phase, reg); > >> > >> if (ret < 0) > >> return ret; > >> > >> Please add a comment explaining what is going on. > > Sure. > >> > >>> + ret ^= PB_STATUS_POWER_GOOD_N; > >>> + break; > >>> case PMBUS_OT_FAULT_LIMIT: > >>> ret = mp2975_read_word_helper(client, page, phase, reg, > >>> GENMASK(7, 0)); > >>> > >>> base-commit: a4bb1ce5b9e3ca403c04e20ebeae77fd6447cb11 > >> >
diff --git a/drivers/hwmon/pmbus/mp2975.c b/drivers/hwmon/pmbus/mp2975.c index 28f33f4618fa..410b6eae6d02 100644 --- a/drivers/hwmon/pmbus/mp2975.c +++ b/drivers/hwmon/pmbus/mp2975.c @@ -297,6 +297,10 @@ static int mp2973_read_word_data(struct i2c_client *client, int page, int ret; switch (reg) { + case PMBUS_STATUS_WORD: + ret = pmbus_read_word_data(client, page, phase, reg); + ret ^= PB_STATUS_POWER_GOOD_N; + break; case PMBUS_OT_FAULT_LIMIT: ret = mp2975_read_word_helper(client, page, phase, reg, GENMASK(7, 0));