[PATCH] hwmon: (pmbus/ibm-cffps) Do not swap max_power_out

[PATCH] hwmon: (pmbus/ibm-cffps) Do not swap max_power_out

[Brandon Wyman]

The bytes for max_power_out from the ibm-cffps devices do not need to be
swapped.

Signed-off-by: Brandon Wyman <bjwyman@gmail.com>
---
 drivers/hwmon/pmbus/ibm-cffps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c
index df712ce4b164..29b77f192c9e 100644
--- a/drivers/hwmon/pmbus/ibm-cffps.c
+++ b/drivers/hwmon/pmbus/ibm-cffps.c
@@ -171,7 +171,7 @@ static ssize_t ibm_cffps_debugfs_read(struct file *file, char __user *buf,
 		cmd = CFFPS_SN_CMD;
 		break;
 	case CFFPS_DEBUGFS_MAX_POWER_OUT:
-		rc = i2c_smbus_read_word_swapped(psu->client,
+		rc = i2c_smbus_read_word_data(psu->client,
 						 CFFPS_MAX_POWER_OUT_CMD);
 		if (rc < 0)
 			return rc;
-- 
2.25.1

Re: [PATCH] hwmon: (pmbus/ibm-cffps) Do not swap max_power_out

[Guenter Roeck]

On Fri, Aug 27, 2021 at 11:04:33PM +0000, Brandon Wyman wrote:
> The bytes for max_power_out from the ibm-cffps devices do not need to be
> swapped.
> 
> Signed-off-by: Brandon Wyman <bjwyman@gmail.com>

Eddie, can you confirm this ?

Thanks,
Guenter

> ---
>  drivers/hwmon/pmbus/ibm-cffps.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c
> index df712ce4b164..29b77f192c9e 100644
> --- a/drivers/hwmon/pmbus/ibm-cffps.c
> +++ b/drivers/hwmon/pmbus/ibm-cffps.c
> @@ -171,7 +171,7 @@ static ssize_t ibm_cffps_debugfs_read(struct file *file, char __user *buf,
>  		cmd = CFFPS_SN_CMD;
>  		break;
>  	case CFFPS_DEBUGFS_MAX_POWER_OUT:
> -		rc = i2c_smbus_read_word_swapped(psu->client,
> +		rc = i2c_smbus_read_word_data(psu->client,
>  						 CFFPS_MAX_POWER_OUT_CMD);
>  		if (rc < 0)
>  			return rc;
> -- 
> 2.25.1
> 

Re: [PATCH] hwmon: (pmbus/ibm-cffps) Do not swap max_power_out

[Eddie James]

On Sat, 2021-08-28 at 08:52 -0700, Guenter Roeck wrote:
> On Fri, Aug 27, 2021 at 11:04:33PM +0000, Brandon Wyman wrote:
> > The bytes for max_power_out from the ibm-cffps devices do not need
> > to be
> > swapped.
> > 
> > Signed-off-by: Brandon Wyman <bjwyman@gmail.com>
> 
> Eddie, can you confirm this ?

This can't be true for all the power supplies supported by this driver,
no. I think we need to check the version first. Brandon, I tested this
on witherspoon (which is psu version 1) and get 3148 watts. If it's not
swapped, that would be 19468 watts...

Thanks,
Eddie

> 
> Thanks,
> Guenter
> 
> > ---
> >  drivers/hwmon/pmbus/ibm-cffps.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/hwmon/pmbus/ibm-cffps.c
> > b/drivers/hwmon/pmbus/ibm-cffps.c
> > index df712ce4b164..29b77f192c9e 100644
> > --- a/drivers/hwmon/pmbus/ibm-cffps.c
> > +++ b/drivers/hwmon/pmbus/ibm-cffps.c
> > @@ -171,7 +171,7 @@ static ssize_t ibm_cffps_debugfs_read(struct
> > file *file, char __user *buf,
> >  		cmd = CFFPS_SN_CMD;
> >  		break;
> >  	case CFFPS_DEBUGFS_MAX_POWER_OUT:
> > -		rc = i2c_smbus_read_word_swapped(psu->client,
> > +		rc = i2c_smbus_read_word_data(psu->client,
> >  						 CFFPS_MAX_POWER_OUT_CM
> > D);
> >  		if (rc < 0)
> >  			return rc;
> > -- 
> > 2.25.1
> > 

Re: [PATCH] hwmon: (pmbus/ibm-cffps) Do not swap max_power_out

[Guenter Roeck]

On 8/30/21 6:50 AM, Eddie James wrote:
> On Sat, 2021-08-28 at 08:52 -0700, Guenter Roeck wrote:
>> On Fri, Aug 27, 2021 at 11:04:33PM +0000, Brandon Wyman wrote:
>>> The bytes for max_power_out from the ibm-cffps devices do not need
>>> to be
>>> swapped.
>>>
>>> Signed-off-by: Brandon Wyman <bjwyman@gmail.com>
>>
>> Eddie, can you confirm this ?
> 
> This can't be true for all the power supplies supported by this driver,
> no. I think we need to check the version first. Brandon, I tested this
> on witherspoon (which is psu version 1) and get 3148 watts. If it's not
> swapped, that would be 19468 watts...
> 

Hmm. Eddie, can you also have a look at commit 9fed8fa99334 ("hwmon:
(pmbus/ibm-cffps) Fix write bits for LED control") ?
We need to make sure that it doesn't mess up some versions of this PS.

Thanks,
Guenter

Re: [PATCH] hwmon: (pmbus/ibm-cffps) Do not swap max_power_out

[Eddie James]

On Mon, 2021-08-30 at 08:34 -0700, Guenter Roeck wrote:
> On 8/30/21 6:50 AM, Eddie James wrote:
> > On Sat, 2021-08-28 at 08:52 -0700, Guenter Roeck wrote:
> > > On Fri, Aug 27, 2021 at 11:04:33PM +0000, Brandon Wyman wrote:
> > > > The bytes for max_power_out from the ibm-cffps devices do not
> > > > need
> > > > to be
> > > > swapped.
> > > > 
> > > > Signed-off-by: Brandon Wyman <bjwyman@gmail.com>
> > > 
> > > Eddie, can you confirm this ?
> > 
> > This can't be true for all the power supplies supported by this
> > driver,
> > no. I think we need to check the version first. Brandon, I tested
> > this
> > on witherspoon (which is psu version 1) and get 3148 watts. If it's
> > not
> > swapped, that would be 19468 watts...
> > 
> 
> Hmm. Eddie, can you also have a look at commit 9fed8fa99334 ("hwmon:
> (pmbus/ibm-cffps) Fix write bits for LED control") ?
> We need to make sure that it doesn't mess up some versions of this
> PS.

That one looks correct to me. I believe older PSUs didn't enforce this
so I didn't catch it, but I do see that the older specifications
mention setting bit 6 to "allow write".

Thanks,
Eddie

> 
> Thanks,
> Guenter

Re: [PATCH] hwmon: (pmbus/ibm-cffps) Do not swap max_power_out

[Guenter Roeck]

On 8/30/21 1:11 PM, Eddie James wrote:
> On Mon, 2021-08-30 at 08:34 -0700, Guenter Roeck wrote:
>> On 8/30/21 6:50 AM, Eddie James wrote:
>>> On Sat, 2021-08-28 at 08:52 -0700, Guenter Roeck wrote:
>>>> On Fri, Aug 27, 2021 at 11:04:33PM +0000, Brandon Wyman wrote:
>>>>> The bytes for max_power_out from the ibm-cffps devices do not
>>>>> need
>>>>> to be
>>>>> swapped.
>>>>>
>>>>> Signed-off-by: Brandon Wyman <bjwyman@gmail.com>
>>>>
>>>> Eddie, can you confirm this ?
>>>
>>> This can't be true for all the power supplies supported by this
>>> driver,
>>> no. I think we need to check the version first. Brandon, I tested
>>> this
>>> on witherspoon (which is psu version 1) and get 3148 watts. If it's
>>> not
>>> swapped, that would be 19468 watts...
>>>
>>
>> Hmm. Eddie, can you also have a look at commit 9fed8fa99334 ("hwmon:
>> (pmbus/ibm-cffps) Fix write bits for LED control") ?
>> We need to make sure that it doesn't mess up some versions of this
>> PS.
> 
> That one looks correct to me. I believe older PSUs didn't enforce this
> so I didn't catch it, but I do see that the older specifications
> mention setting bit 6 to "allow write".
> 

Great, thanks a lot for checking.

Guenter

Re: [PATCH] hwmon: (pmbus/ibm-cffps) Do not swap max_power_out

[Brandon Wyman]

On 2021-08-30 08:50, Eddie James wrote:
> On Sat, 2021-08-28 at 08:52 -0700, Guenter Roeck wrote:
>> On Fri, Aug 27, 2021 at 11:04:33PM +0000, Brandon Wyman wrote:
>>> The bytes for max_power_out from the ibm-cffps devices do not need
>>> to be
>>> swapped.
>>>
>>> Signed-off-by: Brandon Wyman <bjwyman@gmail.com>
>> Eddie, can you confirm this ?
> This can't be true for all the power supplies supported by this driver,
> no. I think we need to check the version first. Brandon, I tested this
> on witherspoon (which is psu version 1) and get 3148 watts. If it's not
> swapped, that would be 19468 watts...
>
> Thanks,
> Eddie
I had tested this on a variety of systems with a variety of different 
power supplies, but I did *NOT* test this on the Witherspoon power supplies.

This apparently requires a bit more thought to figure out how to handle 
the other types and also not get Witherspoon wrong.

Thanks for checking Eddie.

>> Thanks,
>> Guenter
>>
>>> ---
>>>   drivers/hwmon/pmbus/ibm-cffps.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwmon/pmbus/ibm-cffps.c
>>> b/drivers/hwmon/pmbus/ibm-cffps.c
>>> index df712ce4b164..29b77f192c9e 100644
>>> --- a/drivers/hwmon/pmbus/ibm-cffps.c
>>> +++ b/drivers/hwmon/pmbus/ibm-cffps.c
>>> @@ -171,7 +171,7 @@ static ssize_t ibm_cffps_debugfs_read(struct
>>> file *file, char __user *buf,
>>>   		cmd = CFFPS_SN_CMD;
>>>   		break;
>>>   	case CFFPS_DEBUGFS_MAX_POWER_OUT:
>>> -		rc = i2c_smbus_read_word_swapped(psu->client,
>>> +		rc = i2c_smbus_read_word_data(psu->client,
>>>   						 CFFPS_MAX_POWER_OUT_CM
>>> D);
>>>   		if (rc < 0)
>>>   			return rc;
>>> -- 
>>> 2.25.1
>>>

Re: [PATCH] hwmon: (pmbus/ibm-cffps) Do not swap max_power_out

[Guenter Roeck]

On 8/30/21 2:07 PM, Brandon Wyman wrote:
> 
> On 2021-08-30 08:50, Eddie James wrote:
>> On Sat, 2021-08-28 at 08:52 -0700, Guenter Roeck wrote:
>>> On Fri, Aug 27, 2021 at 11:04:33PM +0000, Brandon Wyman wrote:
>>>> The bytes for max_power_out from the ibm-cffps devices do not need
>>>> to be
>>>> swapped.
>>>>
>>>> Signed-off-by: Brandon Wyman <bjwyman@gmail.com>
>>> Eddie, can you confirm this ?
>> This can't be true for all the power supplies supported by this driver,
>> no. I think we need to check the version first. Brandon, I tested this
>> on witherspoon (which is psu version 1) and get 3148 watts. If it's not
>> swapped, that would be 19468 watts...
>>
>> Thanks,
>> Eddie
> I had tested this on a variety of systems with a variety of different power supplies, but I did *NOT* test this on the Witherspoon power supplies.
> 
> This apparently requires a bit more thought to figure out how to handle the other types and also not get Witherspoon wrong.
> 

Is the specification for those power supplies available in public ?

Thanks,
Guenter

> Thanks for checking Eddie.
> 
>>> Thanks,
>>> Guenter
>>>
>>>> ---
>>>>   drivers/hwmon/pmbus/ibm-cffps.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/hwmon/pmbus/ibm-cffps.c
>>>> b/drivers/hwmon/pmbus/ibm-cffps.c
>>>> index df712ce4b164..29b77f192c9e 100644
>>>> --- a/drivers/hwmon/pmbus/ibm-cffps.c
>>>> +++ b/drivers/hwmon/pmbus/ibm-cffps.c
>>>> @@ -171,7 +171,7 @@ static ssize_t ibm_cffps_debugfs_read(struct
>>>> file *file, char __user *buf,
>>>>           cmd = CFFPS_SN_CMD;
>>>>           break;
>>>>       case CFFPS_DEBUGFS_MAX_POWER_OUT:
>>>> -        rc = i2c_smbus_read_word_swapped(psu->client,
>>>> +        rc = i2c_smbus_read_word_data(psu->client,
>>>>                            CFFPS_MAX_POWER_OUT_CM
>>>> D);
>>>>           if (rc < 0)
>>>>               return rc;
>>>> -- 
>>>> 2.25.1
>>>>

Re: [PATCH] hwmon: (pmbus/ibm-cffps) Do not swap max_power_out

[Brandon Wyman]

On 2021-08-30 16:27, Guenter Roeck wrote:
> On 8/30/21 2:07 PM, Brandon Wyman wrote:
>>
>> On 2021-08-30 08:50, Eddie James wrote:
>>> On Sat, 2021-08-28 at 08:52 -0700, Guenter Roeck wrote:
>>>> On Fri, Aug 27, 2021 at 11:04:33PM +0000, Brandon Wyman wrote:
>>>>> The bytes for max_power_out from the ibm-cffps devices do not need
>>>>> to be
>>>>> swapped.
>>>>>
>>>>> Signed-off-by: Brandon Wyman <bjwyman@gmail.com>
>>>> Eddie, can you confirm this ?
>>> This can't be true for all the power supplies supported by this driver,
>>> no. I think we need to check the version first. Brandon, I tested this
>>> on witherspoon (which is psu version 1) and get 3148 watts. If it's not
>>> swapped, that would be 19468 watts...
>>>
>>> Thanks,
>>> Eddie
>> I had tested this on a variety of systems with a variety of different 
>> power supplies, but I did *NOT* test this on the Witherspoon power 
>> supplies.
>>
>> This apparently requires a bit more thought to figure out how to 
>> handle the other types and also not get Witherspoon wrong.
>>
>
> Is the specification for those power supplies available in public ?
>
> Thanks,
> Guenter
>
No, unfortunately those power supply specifications are not available to 
the public.

Sorry,

Brandon

>> Thanks for checking Eddie.
>>
>>>> Thanks,
>>>> Guenter
>>>>
>>>>> ---
>>>>>   drivers/hwmon/pmbus/ibm-cffps.c | 2 +-
>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/hwmon/pmbus/ibm-cffps.c
>>>>> b/drivers/hwmon/pmbus/ibm-cffps.c
>>>>> index df712ce4b164..29b77f192c9e 100644
>>>>> --- a/drivers/hwmon/pmbus/ibm-cffps.c
>>>>> +++ b/drivers/hwmon/pmbus/ibm-cffps.c
>>>>> @@ -171,7 +171,7 @@ static ssize_t ibm_cffps_debugfs_read(struct
>>>>> file *file, char __user *buf,
>>>>>           cmd = CFFPS_SN_CMD;
>>>>>           break;
>>>>>       case CFFPS_DEBUGFS_MAX_POWER_OUT:
>>>>> -        rc = i2c_smbus_read_word_swapped(psu->client,
>>>>> +        rc = i2c_smbus_read_word_data(psu->client,
>>>>>                            CFFPS_MAX_POWER_OUT_CM
>>>>> D);
>>>>>           if (rc < 0)
>>>>>               return rc;
>>>>> -- 
>>>>> 2.25.1
>>>>>
>