Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dni_dps460] Fix read and write failure to ‘fan1_target’ attribute #183

Merged

Conversation

ArunSaravananBalachandran
Copy link
Contributor

To fix read and write failure to ‘fan1_target’ attribute of ‘dni_dps460’ driver.

  • Updated the pmbus_data structure in ‘dni_dps460’ driver’s patch to the one present in ‘pmbus_core.c’ in the latest kernel version used.
  • Reverted patch to use ‘kstrtol_from_user’ instead of ‘kstrtol’ for set operation in the driver.

Logs: UT_logs.txt

Resolves sonic-net/sonic-buildimage#6140.

@paulmenzel
Copy link
Contributor

Thank you for the patch.

  1. Why can patch/driver-hwmon-pmbus-dni_dps460-use-kstrtol-from-user.patch be removed?
  2. Could you please add the merge/pull request description also to the git commit message?
  3. Please sign off the commit and patch/driver-hwmon-pmbus-dni_dps460-update-pmbus-core.patch.
  4. Please send the patch to Linux upstream, so it does not need to be maintained in the future in here, but is maintained in upstream Linux.

Item 4 is very important in my opinion.

@lguohan
Copy link
Contributor

lguohan commented Jan 14, 2021

have the same question, @ArunSaravananBalachandran, can you address?

@ArunSaravananBalachandran
Copy link
Contributor Author

Thank you for the patch.

  1. Why can patch/driver-hwmon-pmbus-dni_dps460-use-kstrtol-from-user.patch be removed?
  2. Could you please add the merge/pull request description also to the git commit message?
  3. Please sign off the commit and patch/driver-hwmon-pmbus-dni_dps460-update-pmbus-core.patch.
  4. Please send the patch to Linux upstream, so it does not need to be maintained in the future in here, but is maintained in upstream Linux.

Item 4 is very important in my opinion.

  1. With driver-hwmon-pmbus-dni_dps460-use-kstrtol-from-user.patch introduced to avoid the crash seen in Console shows Kernel crash during boot up with Mater debug image .339 on Dell S6000-ACS/S6000 #152, write on the attribute fails with 'Bad address' error.
root@sonic:/home/admin# echo 15000 > /sys/class/i2c-adapter/i2c-1/1-0059/fan1_target
bash: echo: write error: Bad address
root@sonic:/home/admin#

Updating the pmbus_data structure in ‘dni_dps460’ driver’s patch to the one used in latest kernel version, resolves the crash seen in both read sonic-net/sonic-buildimage#6140 and write #152 on the attribute and removing the driver-hwmon-pmbus-dni_dps460-use-kstrtol-from-user.patch resolves the 'Bad address' error.

  1. Will amend the commit to update the commit message.
  2. Will signoff the commit and the patch through a new commit.
  3. The dni_dps460 driver is defined only in sonic-linux-kernel via patch/driver-hwmon-pmbus-dni_dps460.patch and not available in upstream linux. Currently this driver is used only in Dell S6000 platform for accessing PSU attributes.

… driver.

 - Updated the pmbus_data structure in ‘dni_dps460’ driver’s patch to the one present in ‘pmbus_core.c’ in the latest kernel version used.
 - Reverted patch to use ‘kstrtol_from_user’ instead of ‘kstrtol’ for set operation in the driver.

Signed-off-by: Arun Saravanan Balachandran <[email protected]>
@paulmenzel
Copy link
Contributor

Thank you for your replies. Your explanation in item 1. is very good, and is perfect to have in a commit message, so people do not have to read the merge/pull request discussion.

Regarding 4., I know the driver is not in upstream Linux, but that is the goal, and nothing would prevent that. To avoid future porting work, it’d be great, if you sent the whole driver upstream, seeing that you also have access to the hardware.

@lguohan lguohan merged commit fc74b1c into sonic-net:master Jan 20, 2021
lguohan pushed a commit to lguohan/sonic-linux-kernel that referenced this pull request Mar 10, 2021
… driver. (sonic-net#183)

- Updated the pmbus_data structure in ‘dni_dps460’ driver’s patch to the one present in ‘pmbus_core.c’ in the latest kernel version used.
- Reverted patch to use ‘kstrtol_from_user’ instead of ‘kstrtol’ for set operation in the driver.

Signed-off-by: Arun Saravanan Balachandran <[email protected]>
lguohan pushed a commit that referenced this pull request Mar 10, 2021
… driver. (#183)

- Updated the pmbus_data structure in ‘dni_dps460’ driver’s patch to the one present in ‘pmbus_core.c’ in the latest kernel version used.
- Reverted patch to use ‘kstrtol_from_user’ instead of ‘kstrtol’ for set operation in the driver.

Signed-off-by: Arun Saravanan Balachandran <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[dni_dps460] Read, Write on 'fan1_target' reports failure
3 participants