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

Jump to bottom

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 all commits
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
71 changes: 68 additions & 3 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 @@ -18,7 +18,7 @@ i2c_config() {
done

if [[ "$count" -eq "$MAX_BUS_RETRY" ]]; then
echo "ERROR: $@ : i2c bus not created"
echo "dell_i2c_utils : ERROR: $@ : i2c bus not created"
return
fi

Expand All @@ -31,7 +31,7 @@ i2c_config() {
done

if [[ "$count" -eq "$MAX_I2C_OP_RETRY" ]]; then
echo "ERROR: $@ : i2c operation failed"
echo "dell_i2c_utils : ERROR: $@ : i2c operation failed"
return
fi
}
Expand All @@ -53,10 +53,75 @@ i2c_poll_bus_exists() {
done

if [[ "$count" -eq "$MAX_BUS_RETRY" ]]; then
echo "ERROR: $@ : i2c bus not created"
echo "dell_i2c_utils : ERROR: $@ : i2c bus not created"
return 1
else
return 0
fi
}

# Perform an i2c mux device create
# Input is of the form:
# i2c_mux_create mux_driver i2c_addr i2c_bus_num i2c_child_bus_num_start
# where i2c_bus_num is the bus number in which the mux is to be created and
# i2c_child_bus_num_start is the first of the 8 bus 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=3
local MAX_MUX_CHANNELS=8
local count=0
local i
local mux_driver=$1
local i2c_addr=$2
local i2c_bus_num=$3
local i2c_child_bus_num_start=$4

# Construct the i2c bus, the first and last bus channels that will be created under the MUX
i2c_bus=/sys/bus/i2c/devices/i2c-$i2c_bus_num
i2c_mux_channel_first=$i2c_bus/i2c-$i2c_child_bus_num_start
i2c_mux_channel_last=$i2c_bus/i2c-$(expr $i2c_child_bus_num_start + $MAX_MUX_CHANNELS - 1)

if i2c_poll_bus_exists $i2c_bus; then
while [[ "$count" -lt "$MAX_MUX_CHANNEL_RETRY" ]]; do
eval "echo $mux_driver $i2c_addr > /sys/bus/i2c/devices/i2c-$i2c_bus_num/new_device" > /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 1
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
echo "dell_i2c_utils : ERROR: i2c mux channel not created for $mux_driver,$i2c_addr,$i2c_bus_num"
i2c_mux_delete $i2c_addr $i2c_bus_num
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 "dell_i2c_utils : ERROR: $1,$2 : i2c mux channel not created"
return
fi

return
}

# Perform an i2c mux device delete
# Input is of the form:
# i2c_mux_delete i2c_addr i2c_bus_num
i2c_mux_delete() {
local i2c_addr
local i2c_bus_num

i2c_addr=$1
i2c_bus_num=$2
i2c_config "echo $i2c_addr > /sys/bus/i2c/devices/i2c-$i2c_bus_num/delete_device"
}
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
42 changes: 34 additions & 8 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,9 +21,9 @@ 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 pca9547 0x70 $devnum 2
;;
"delete_device") i2c_config "echo 0x70 > /sys/bus/i2c/devices/i2c-${devnum}/$1"
"delete_device") i2c_mux_delete 0x70 $devnum
;;
*) echo "s6100_platform: cpu_board_mux: invalid command !"
;;
Expand All @@ -33,9 +33,9 @@ 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 pca9548 0x71 4 10
;;
"delete_device") i2c_config "echo 0x71 > /sys/bus/i2c/devices/i2c-4/$1"
"delete_device") i2c_mux_delete 0x71 4
;;
*) echo "s6100_platform: switch_board_mux : invalid command !"
;;
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 pca9548 0x71 $i $channel_first
i2c_mux_create pca9548 0x72 $i $(expr $channel_first + 8)
channel_first=$(expr $channel_first + 16)
done
;;
"delete_device")
Expand All @@ -93,8 +97,8 @@ switch_board_qsfp_mux() {
# 0x71 mux on the IOM 1
mux_index=$(expr $i - 5)
echo "Detaching PCA9548 $mux_index"
i2c_config "echo 0x71 > /sys/bus/i2c/devices/i2c-$devnum/i2c-$i/$1"
i2c_config "echo 0x72 > /sys/bus/i2c/devices/i2c-$devnum/i2c-$i/$1"
i2c_mux_delete 0x71 $i
i2c_mux_delete 0x72 $i
done
;;
*) echo "s6100_platform: switch_board_qsfp_mux: invalid command !"
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
27 changes: 21 additions & 6 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,9 +21,9 @@ 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 pca9547 0x70 $devnum 2
;;
"delete_device") i2c_config "echo 0x70 > /sys/bus/i2c/devices/i2c-${devnum}/$1"
"delete_device") i2c_mux_delete 0x70 $devnum
;;
*) echo "z9100_platform: cpu_board_mux: invalid command !"
;;
Expand All @@ -33,9 +33,9 @@ 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 pca9548 0x71 4 10
;;
"delete_device") i2c_config "echo 0x71 > /sys/bus/i2c/devices/i2c-4/$1"
"delete_device") i2c_mux_delete 0x71 4
;;
*) echo "z9100_platform: switch_board_mux : invalid command !"
;;
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 pca9548 0x71 $i $channel_first
channel_first=$(expr $channel_first + 8)
done
;;
"delete_device")
Expand All @@ -92,7 +96,7 @@ switch_board_qsfp_mux() {
# 0x71 mux on the IOM 1
mux_index=$(expr $i - 5)
echo "Detaching PCA9548 $mux_index"
i2c_config "echo 0x71 > /sys/bus/i2c/devices/i2c-$devnum/i2c-$i/$1"
i2c_mux_delete 0x71 $i
done
;;
*) echo "z9100_platform: switch_board_qsfp_mux: invalid command !"
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