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

[device]: Fix a bug that psuutil cannot access gpio sysfs to get PSU status #1789

Merged
merged 8 commits into from Jun 21, 2018
Merged
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions device/celestica/x86_64-cel_seastone-r0/plugins/psuutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
# Platform-specific PSU status interface for SONiC
#

import os.path
import os
import logging


try:
from sonic_psu.psu_base import PsuBase
Expand All @@ -18,33 +20,41 @@ def __init__(self):
PsuBase.__init__(self)
# DX010 PSU pin mapping
self.psu = [
{'base':216}, # Reserved
{'base': self.get_gpio_base()},
{'abs':27, 'power':22},
{'abs':28, 'power':25}
]

def get_gpio_base(self):
sys_gpio_dir = "/sys/class/gpio"
try:
for r in os.listdir(sys_gpio_dir):
if "gpiochip" in r:
return int(r[8:],10)
except Exception as error:
logging.error("Unable to get gpio base")


def init_psu_gpio(self, pinnum):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this init_psu_gpio should move to your driver, we should not do init psu gpio on-the-fly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jleveque, We will update init_psu_gpio after we updated our driver

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pphuchar Please add this to our platform driver

# export pin, input as default
gpio_base = self.psu[0]['base']
export_file = "/sys/class/gpio/export"
direction_file = '/sys/class/gpio/gpio' + str(gpio_base+pinnum) + '/direction'

try:
with open(export_file, 'w') as fd:
fd.write(str(gpio_base+pinnum))
except Exception as error:
logging.error("Unable to export gpio ", pinnum)
logging.error("Unable to export gpio " + str(gpio_base+pinnum))


# Get a psu status and presence
def read_psu_statuses(self, pinnum):
sys_gpio_dir = "/sys/class/gpio"
retval = 'ERR'
gpio_base = self.psu[0]['base']
retval = 'ERR'

gpio_dir = sys_gpio_dir + '/gpio' + str(gpio_base+pinnum)
gpio_file = gpio_dir + "/value"

# init gpio
if (not os.path.isdir(gpio_dir)):
self.init_psu_gpio(pinnum)
Expand All @@ -53,7 +63,7 @@ def read_psu_statuses(self, pinnum):
with open(gpio_file, 'r') as fd:
retval = fd.read()
except Exception as error:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should not catch general exception here, we should only catch exception we can handle in this case the io or file exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I will change it

logging.error("Unable to open ", gpio_file, "file !")
logging.error("Unable to open " + gpio_file + "file !")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should further raise in the plugin, retval = 'ERR' is not the convention to convene error to the caller.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK , I will add raise statement to report the error


retval = retval.rstrip('\r\n')
return retval
Expand Down