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.

Sign up for GitHub

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

Address Dell issue#46 : Adding MUX reset logic to fix probe failures #2356

Merged
merged 2 commits into from
Jan 8, 2019
Merged

Address Dell issue#46 : Adding MUX reset logic to fix probe failures #2356

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c83dd8
Address Dell issue#46
padmanarayana Dec 6, 2018
360ea10
Address review comments
padmanarayana Dec 18, 2018
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
57 changes: 57 additions & 0 deletions platform/broadcom/sonic-platform-modules-dell/common/dell_i2c_utils.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,60 @@ i2c_poll_bus_exists() {
fi
}

# Perform an i2c mux device create
# Input is of the form:
# "echo [mux driver] <i2c-mux-address> > <i2c-bus/operation>" i2c-channel-first
# where operation = "new_device"
# i2c-channel-first is the first of the 8 channels that this mux should create
i2c_mux_create() {
Copy link
Contributor

Choose a reason for hiding this comment

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

for me, this piece of code is a little bit hard to understand. since the i2c tree is fixed on each platform, it would be better to have some more predefined numbers instead of number manipulation here.

the function could be simpler like i2c_mux_create <root> <sub_root> <first>
so that in the script you only need to call the function e.g. i2c_mux_create 4 0x71 10 or i2c_mux_create 7 0x71 50 or i2c_mux_create 7 0x72 58

the echo command and the details of the retry could be inside the function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed this.

local MAX_MUX_CHANNEL_RETRY=5
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel 5 is too large, could you change to 3.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed this

local MAX_MUX_CHANNELS=8
local count=0
local i
local i2c_bus_op
local i2c_bus
local i2c_bus_num
local i2c_mux
local i2c_mux_channel_first
local i2c_mux_channel_last

# Extract the i2c parameters from the command line
i2c_bus_op=`echo "$1" | cut -d'>' -f 2`
i2c_bus=$(dirname $i2c_bus_op)
i2c_bus_num=`echo $(basename $i2c_bus) | cut -d'-' -f 2`
i2c_mux=`echo "$1" | cut -d' ' -f 3 | sed 's/^0x//'`
i2c_mux_channel_first=$i2c_bus/i2c-$2
i2c_mux_channel_last=$i2c_bus/i2c-$(expr $2 + $MAX_MUX_CHANNELS - 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

the above logic is too complex to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed


if i2c_poll_bus_exists $i2c_bus; then
while [[ "$count" -lt "$MAX_MUX_CHANNEL_RETRY" ]]; do
eval "$1" > /dev/null 2>&1
ret=$?

# Give more time for the mux channels to get created based on retries
i=0
while [[ "$i" -lt "$count" ]]; do
sleep .5
i=$((i+1))
done

# Check if the (first and last) mux channels got created
if [[ $ret -eq "0" && -e $i2c_mux_channel_first && -e $i2c_mux_channel_last ]]; then
break;
else
# If the channel did not get created, remove the mux, reset the mux tree and retry
i2c_config "echo 0x$i2c_mux > /sys/bus/i2c/devices/i2c-$devnum/i2c-$i2c_bus_num/delete_device"
Copy link
Contributor

Choose a reason for hiding this comment

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

could you rename the function i2c_config to i2c_mux_remove? and make the function symmetric with i2c_mux_remove?

reset_muxes
Copy link
Contributor

Choose a reason for hiding this comment

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

before resetting the mux, could you add one line specific log indicating which mux failed to be initialized? what's the root?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added debug with mux details.

fi

count=$((count+1))
done
fi

if [[ "$count" -eq "$MAX_MUX_CHANNEL_RETRY" ]]; then
echo "ERROR: $1,$2 : i2c mux channel not created"
return
fi

return
}
0 ...rm-modules-dell/s6100/scripts/io_rd_wr.py → ...-platform-modules-dell/common/io_rd_wr.py
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion platform/broadcom/sonic-platform-modules-dell/debian/platform-modules-s6100.install
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
s6100/scripts/io_rd_wr.py usr/local/bin
s6100/scripts/iom_power_*.sh usr/local/bin
s6100/scripts/s6100_platform.sh usr/local/bin
common/dell_i2c_utils.sh usr/local/bin
common/io_rd_wr.py usr/local/bin
common/fstrim.timer etc/systemd/system
common/fstrim.service etc/systemd/system
s6100/scripts/platform_sensors.py usr/local/bin
Expand Down
1 change: 1 addition & 0 deletions platform/broadcom/sonic-platform-modules-dell/debian/platform-modules-z9100.install
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
z9100/scripts/check_qsfp.sh usr/local/bin
z9100/scripts/z9100_platform.sh usr/local/bin
common/dell_i2c_utils.sh usr/local/bin
common/io_rd_wr.py usr/local/bin
common/fstrim.timer etc/systemd/system
common/fstrim.service etc/systemd/system
z9100/scripts/platform_sensors.py usr/local/bin
Expand Down
32 changes: 32 additions & 0 deletions platform/broadcom/sonic-platform-modules-dell/s6100/modules/dell_s6100_iom_cpld.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
//iom cpld slave address
#define IOM_CPLD_SLAVE_ADD 0x3e

#define CPLD_SEP_RST0 0x5

//iom cpld ver register
#define IOM_CPLD_SLAVE_VER 0x00

Expand Down Expand Up @@ -384,6 +386,34 @@ static ssize_t set_abs_mask(struct device *dev, struct device_attribute *devattr
return count;
}

static ssize_t get_sep_reset(struct device *dev, struct device_attribute *devattr, char *buf)
{
int ret;
u8 devdata=0;
struct cpld_data *data = dev_get_drvdata(dev);

ret = dell_s6100_iom_cpld_read(data,IOM_CPLD_SLAVE_ADD,CPLD_SEP_RST0);
if(ret < 0)
return sprintf(buf, "read error");
devdata = (u8)ret & 0xff;
return sprintf(buf,"0x%02x\n",devdata);
}

static ssize_t set_sep_reset(struct device *dev, struct device_attribute *devattr, const char *buf, size_t count)
{
unsigned long devdata;
int err;
struct cpld_data *data = dev_get_drvdata(dev);

err = kstrtoul(buf, 16, &devdata);
if (err)
return err;

dell_s6100_iom_cpld_write(data,IOM_CPLD_SLAVE_ADD,CPLD_SEP_RST0,(u8)(devdata & 0xff));

return count;
}

static DEVICE_ATTR(iom_cpld_vers,S_IRUGO,get_cpldver, NULL);
static DEVICE_ATTR(qsfp_modprs, S_IRUGO,get_modprs, NULL);
static DEVICE_ATTR(qsfp_lpmode, S_IRUGO | S_IWUSR,get_lpmode,set_lpmode);
Expand All @@ -396,6 +426,7 @@ static DEVICE_ATTR(qsfp_int, S_IRUGO, get_int, NULL);
static DEVICE_ATTR(qsfp_abs_int, S_IRUGO, get_abs_int, NULL);
static DEVICE_ATTR(qsfp_int_mask, S_IRUGO | S_IWUSR, get_int_mask, set_int_mask);
static DEVICE_ATTR(qsfp_abs_mask, S_IRUGO | S_IWUSR, get_abs_mask, set_abs_mask);
static DEVICE_ATTR(sep_reset, S_IRUGO | S_IWUSR, get_sep_reset, set_sep_reset);

static struct attribute *i2c_cpld_attrs[] = {
&dev_attr_qsfp_lpmode.attr,
Expand All @@ -410,6 +441,7 @@ static struct attribute *i2c_cpld_attrs[] = {
&dev_attr_qsfp_abs_int.attr,
&dev_attr_qsfp_int_mask.attr,
&dev_attr_qsfp_abs_mask.attr,
&dev_attr_sep_reset.attr,
NULL,
};

Expand Down
34 changes: 30 additions & 4 deletions platform/broadcom/sonic-platform-modules-dell/s6100/scripts/s6100_platform.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ init_devnum() {
# Attach/Detach CPU board mux @ 0x70
cpu_board_mux() {
case $1 in
"new_device") i2c_config "echo pca9547 0x70 > /sys/bus/i2c/devices/i2c-${devnum}/$1"
"new_device") i2c_mux_create "echo pca9547 0x70 > /sys/bus/i2c/devices/i2c-${devnum}/$1" 2
;;
"delete_device") i2c_config "echo 0x70 > /sys/bus/i2c/devices/i2c-${devnum}/$1"
;;
Expand All @@ -33,7 +33,7 @@ cpu_board_mux() {
# Attach/Detach Switchboard MUX @ 0x71
switch_board_mux() {
case $1 in
"new_device") i2c_config "echo pca9548 0x71 > /sys/bus/i2c/devices/i2c-4/$1"
"new_device") i2c_mux_create "echo pca9548 0x71 > /sys/bus/i2c/devices/i2c-4/$1" 10
;;
"delete_device") i2c_config "echo 0x71 > /sys/bus/i2c/devices/i2c-4/$1"
;;
Expand Down Expand Up @@ -78,13 +78,17 @@ switch_board_cpld() {
switch_board_qsfp_mux() {
case $1 in
"new_device")
# The mux for the QSFPs spawn {18..25}, {26..33}... {74..81}
# starting at chennel 18 and 16 channels per IOM.
channel_first=18
for ((i=9;i>=6;i--));
do
# 0x71 mux on the IOM 1
mux_index=$(expr $i - 5)
echo "Attaching PCA9548 $mux_index"
i2c_config "echo pca9548 0x71 > /sys/bus/i2c/devices/i2c-$i/$1"
i2c_config "echo pca9548 0x72 > /sys/bus/i2c/devices/i2c-$i/$1"
i2c_mux_create "echo pca9548 0x71 > /sys/bus/i2c/devices/i2c-$i/$1" $channel_first
i2c_mux_create "echo pca9548 0x72 > /sys/bus/i2c/devices/i2c-$i/$1" $(expr $channel_first + 8)
channel_first=$(expr $channel_first + 16)
done
;;
"delete_device")
Expand Down Expand Up @@ -191,6 +195,28 @@ xcvr_presence_interrupts() {
esac
}

# Reset the mux tree
reset_muxes() {
Copy link
Contributor

Choose a reason for hiding this comment

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

for resetting the muxes, is it possible to reset only one mux per the failure? or do we need to reset all the muxes? if we reset all the muxes, do we need to re-build the whole i2c tree?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is possible to reset only the specific mux that are part of this channel - however the complexity of that logic is not warranted. Resetting the muxes does not necessitate rebuilding the kernel's i2c tree.

local i

# Reset the IOM muxes (if they have been already instantiated)
for ((i=14;i<=17;i++));
do
if [[ -e /sys/class/i2c-adapter/i2c-$i/$i-003e ]]; then
echo 0xfc > /sys/class/i2c-adapter/i2c-$i/$i-003e/sep_reset
echo 0xff > /sys/class/i2c-adapter/i2c-$i/$i-003e/sep_reset
fi
done

# Reset the switch card PCA9548A
io_rd_wr.py --set --val 0xef --offset 0x110
io_rd_wr.py --set --val 0xff --offset 0x110

# Reset the CPU Card PCA9547
io_rd_wr.py --set --val 0xfd --offset 0x20b
io_rd_wr.py --set --val 0xff --offset 0x20b
}

init_devnum

if [[ "$1" == "init" ]]; then
Expand Down
21 changes: 18 additions & 3 deletions platform/broadcom/sonic-platform-modules-dell/z9100/scripts/z9100_platform.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ init_devnum() {
# Attach/Detach CPU board mux @ 0x70
cpu_board_mux() {
case $1 in
"new_device") i2c_config "echo pca9547 0x70 > /sys/bus/i2c/devices/i2c-${devnum}/$1"
"new_device") i2c_mux_create "echo pca9547 0x70 > /sys/bus/i2c/devices/i2c-${devnum}/$1" 2
;;
"delete_device") i2c_config "echo 0x70 > /sys/bus/i2c/devices/i2c-${devnum}/$1"
;;
Expand All @@ -33,7 +33,7 @@ cpu_board_mux() {
# Attach/Detach switch board MUX to IOM CPLDs @ 0x71
switch_board_mux() {
case $1 in
"new_device") i2c_config "echo pca9548 0x71 > /sys/bus/i2c/devices/i2c-4/$1"
"new_device") i2c_mux_create "echo pca9548 0x71 > /sys/bus/i2c/devices/i2c-4/$1" 10
;;
"delete_device") i2c_config "echo 0x71 > /sys/bus/i2c/devices/i2c-4/$1"
;;
Expand Down Expand Up @@ -78,12 +78,16 @@ switch_board_cpld() {
switch_board_qsfp_mux() {
case $1 in
"new_device")
# The mux for the QSFPs spawn {18..25}, {26..33}, {34..41} and {42..49}
# starting at chennel 18 and 8 channels per mux.
channel_first=18
for ((i=9;i>=6;i--));
do
# 0x71 mux on the IOM 1
mux_index=$(expr $i - 5)
echo "Attaching PCA9548 $mux_index"
i2c_config "echo pca9548 0x71 > /sys/bus/i2c/devices/i2c-$i/$1"
i2c_mux_create "echo pca9548 0x71 > /sys/bus/i2c/devices/i2c-$i/$1" $channel_first
channel_first=$(expr $channel_first + 8)
done
;;
"delete_device")
Expand Down Expand Up @@ -156,6 +160,17 @@ xcvr_presence_interrupts() {
esac
}

# Reset the mux tree
reset_muxes() {
# Reset the IOM muxes and the switch card mux
io_rd_wr.py --set --val 0xe0 --offset 0x110
io_rd_wr.py --set --val 0xff --offset 0x110

# Reset the CPU Card PCA9547
io_rd_wr.py --set --val 0xfd --offset 0x20b
io_rd_wr.py --set --val 0xff --offset 0x20b
}

init_devnum

if [[ "$1" == "init" ]]; then
Expand Down