<font size=2 face="sans-serif">Hi Hari,</font><br><br><font size=2 face="sans-serif">During tonight's call, it was mentioned
that we shouldn't have a separate commit with review comments. If you'd
make the changes on the existing commit and refresh the pull request, this
can be done with "git commit --amend".</font><br><br><font size=2 face="sans-serif">Thanks!</font><br><br><br><br><font size=1 color=#5f5f5f face="sans-serif">From:
</font><font size=1 face="sans-serif">OpenBMC Patches <openbmc-patches@stwcx.xyz></font><br><font size=1 color=#5f5f5f face="sans-serif">To:
</font><font size=1 face="sans-serif">openbmc@lists.ozlabs.org</font><br><font size=1 color=#5f5f5f face="sans-serif">Date:
</font><font size=1 face="sans-serif">05/18/2016 06:21 AM</font><br><font size=1 color=#5f5f5f face="sans-serif">Subject:
</font><font size=1 face="sans-serif">[PATCH skeleton
v3 3/3] Fix code review comments from Chris Austen.</font><br><font size=1 color=#5f5f5f face="sans-serif">Sent by:
</font><font size=1 face="sans-serif">"openbmc"
<openbmc-bounces+anoo=us.ibm.com@lists.ozlabs.org></font><br><hr noshade><br><br><br><tt><font size=2>From: Hariharasubramanian R <hramasub@in.ibm.com><br><br>---<br> bin/Barreleye.py | 2 +-<br> bin/chassis_control.py | 20 +++++++-------<br> includes/gpio-init-gdbus.c | 22 +++++++--------<br> includes/gpio-init-sdbus.c | 53 ++++++++++++++++++------------------<br> includes/gpio.c | 1 -<br> objects/host_xstop_obj.c | 68 ++++++++++++----------------------------------<br> 6 files changed, 64 insertions(+), 102 deletions(-)<br><br>diff --git a/bin/Barreleye.py b/bin/Barreleye.py<br>index 44aa6a3..8459311 100755<br>--- a/bin/Barreleye.py<br>+++ b/bin/Barreleye.py<br>@@ -551,7 +551,7 @@ GPIO_CONFIG['CPLD_TCK']
=
{ 'gpio_pin': 'P0', 'direction': 'out' }<br> GPIO_CONFIG['CPLD_TDO']
= { 'gpio_pin': 'P1',
'direction': 'out' }<br> GPIO_CONFIG['CPLD_TDI']
= { 'gpio_pin': 'P2',
'direction': 'out' }<br> GPIO_CONFIG['CPLD_TMS']
= { 'gpio_pin': 'P3',
'direction': 'out' }<br>-GPIO_CONFIG['CHECKSTOP'] = { 'gpio_pin': 'P5',
'direction': 'in' }<br>+GPIO_CONFIG['CHECKSTOP0']
= { 'gpio_pin': 'P5', 'direction':
'in' }<br> <br> GPIO_CONFIG['SLOT0_RISER_PRESENT'] = { 'gpio_pin': 'N0', 'direction':
'in' }<br> GPIO_CONFIG['SLOT1_RISER_PRESENT'] = { 'gpio_pin': 'N1', 'direction':
'in' }<br>diff --git a/bin/chassis_control.py b/bin/chassis_control.py<br>index 953f9fd..7b39a9f 100755<br>--- a/bin/chassis_control.py<br>+++ b/bin/chassis_control.py<br>@@ -217,17 +217,17 @@ class ChassisControlObject(Openbmc.DbusProperties,Openbmc.DbusObjectManager):<br>
<br> def
host_xstop_signal_handler(self):<br>
print "Checkstop Error, Waiting for 30sec before Hard Rebooting"<br>- time.sleep(30)<br>+
time.sleep(30)<br>
self.Set(DBUS_NAME,"reboot",1)<br>
self.reboot()<br> <br> if __name__ == '__main__':<br>- dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)<br>-<br>- bus = Openbmc.getDBus()<br>- name = dbus.service.BusName(DBUS_NAME, bus)<br>- obj = ChassisControlObject(bus, OBJ_NAME)<br>- mainloop = gobject.MainLoop()<br>- <br>- print "Running ChassisControlService"<br>- mainloop.run()<br>+
dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)<br>+<br>+
bus
= Openbmc.getDBus()<br>+
name
= dbus.service.BusName(DBUS_NAME, bus)<br>+
obj
= ChassisControlObject(bus, OBJ_NAME)<br>+
mainloop
= gobject.MainLoop()<br>+<br>+
print
"Running ChassisControlService"<br>+
mainloop.run()<br>diff --git a/includes/gpio-init-gdbus.c b/includes/gpio-init-gdbus.c<br>index b9d7712..cf47fb1 100644<br>--- a/includes/gpio-init-gdbus.c<br>+++ b/includes/gpio-init-gdbus.c<br>@@ -23,13 +23,13 @@ int gpio_init(void *connection, GPIO* gpio)<br> g_assert_no_error
(error);<br> error
= NULL;<br> proxy
= g_dbus_proxy_new_sync ((GDBusConnection*)connection,<br>-
G_DBUS_PROXY_FLAGS_NONE,<br>-
NULL,
/* GDBusInterfaceInfo */<br>-
"org.openbmc.managers.System",
/* name */<br>-
"/org/openbmc/managers/System",
/* object path */<br>-
"org.openbmc.managers.System",
/* interface */<br>-
NULL, /* GCancellable */<br>-
&error);<br>+
G_DBUS_PROXY_FLAGS_NONE,<br>+
NULL,
/* GDBusInterfaceInfo */<br>+
"org.openbmc.managers.System",
/* name */<br>+
"/org/openbmc/managers/System",/* object path */<br>+
"org.openbmc.managers.System", /* interface */<br>+
NULL,
/* GCancellable */<br>+
&error);<br> if
(error != NULL) {<br>
return GPIO_LOOKUP_ERROR;<br> }<br>@@ -58,9 +58,8 @@ int gpio_init(void *connection, GPIO* gpio)<br>
<br>
sprintf(dev,"%s/gpio%d/value",gpio->dev,gpio->num);<br>
//check if gpio is exported, if not export<br>-
int result = stat(dev, &st);<br>-
if (result)<br>-
{<br>+
int
result = stat(dev, &st);<br>+
if
(result) {<br>
sprintf(dev,"%s/export",gpio->dev);<br>
fd
= open(dev, O_WRONLY);<br>
if
(fd == GPIO_ERROR) {<br>@@ -76,8 +75,7 @@ int gpio_init(void *connection, GPIO* gpio)<br>
}<br>
}<br>
const char* file = "edge";<br>-
if
(strcmp(gpio->direction,"in")==0 || strcmp(gpio->direction,"out")==0)<br>-
{<br>+
if
(strcmp(gpio->direction,"in")==0 || strcmp(gpio->direction,"out")==0)
{<br>
file
= "direction";<br>
}<br>
sprintf(dev,"%s/gpio%d/%s",gpio->dev,gpio->num,file);<br>diff --git a/includes/gpio-init-sdbus.c b/includes/gpio-init-sdbus.c<br>index f756038..d2d0a42 100644<br>--- a/includes/gpio-init-sdbus.c<br>+++ b/includes/gpio-init-sdbus.c<br>@@ -10,38 +10,37 @@<br> #include "gpio.h"<br> #include <systemd/sd-bus.h><br> <br>-<br> // Gets the gpio device path from gpio manager object<br> int gpio_init(void *connection, GPIO* gpio)<br> {<br> int
rc = GPIO_OK;<br>- sd_bus_error err = SD_BUS_ERROR_NULL;<br>- sd_bus_message *res = NULL;<br>+
sd_bus_error err = SD_BUS_ERROR_NULL;<br>+
sd_bus_message *res = NULL;<br> <br>- sd_bus_error_free(&err);<br>- sd_bus_message_unref(res);<br>+
sd_bus_error_free(&err);<br>+
sd_bus_message_unref(res);<br> <br>- rc = sd_bus_call_method ((sd_bus*)connection,<br>-
"org.openbmc.managers.System",
/* name */<br>-
"/org/openbmc/managers/System",
/* object path */<br>-
"org.openbmc.managers.System",
/* interface */<br>-
"gpioInit",<br>-
&err,<br>-
&res,<br>-
"s",<br>-
gpio->name);<br>+
rc = sd_bus_call_method ((sd_bus*)connection,<br>+
"org.openbmc.managers.System", /* name */<br>+
"/org/openbmc/managers/System", /* object path */<br>+
"org.openbmc.managers.System", /* interface */<br>+
"gpioInit",<br>+
&err,<br>+
&res,<br>+
"s",<br>+
gpio->name);<br>
<br>- if(rc < 0)<br>- {<br>- fprintf(stderr, "Failed to init gpio
for %s : %s\n", gpio->name, err.message);<br>- return -1;<br>- }<br>+
if(rc < 0)<br>+
{<br>+
fprintf(stderr,
"Failed to init gpio for %s : %s\n", gpio->name, err.message);<br>+
return
-1;<br>+
}<br> <br>- rc = sd_bus_message_read (res, "sis", &gpio->dev,
&gpio->num, &gpio->direction);<br>- if (rc < 0)<br>- return -1;<br>- else<br>- g_print("GPIO Lookup: %s = %d,%s\n",gpio->name,gpio->num,gpio->direction);<br>+
rc = sd_bus_message_read (res, "sis", &gpio->dev, &gpio->num,
&gpio->direction);<br>+
if (rc < 0)<br>+
return
-1;<br>+
else<br>+
g_print("GPIO
Lookup: %s = %d,%s\n",gpio->name,gpio->num,gpio->direction);<br> <br> //export
and set direction<br> char
dev[254];<br>@@ -49,11 +48,11 @@ int gpio_init(void *connection, GPIO* gpio)<br> int
fd;<br> do
{<br>
struct stat st;<br>-
<br>+<br>
sprintf(dev,"%s/gpio%d/value",gpio->dev,gpio->num);<br>
//check if gpio is exported, if not export<br>-
int result = stat(dev, &st);<br>-
if (result)<br>+
int
result = stat(dev, &st);<br>+
if
(result)<br>
{<br>
sprintf(dev,"%s/export",gpio->dev);<br>
fd
= open(dev, O_WRONLY);<br>diff --git a/includes/gpio.c b/includes/gpio.c<br>index b6fbf13..52cfaeb 100755<br>--- a/includes/gpio.c<br>+++ b/includes/gpio.c<br>@@ -9,7 +9,6 @@<br> #include <sys/mman.h><br> #include "gpio.h"<br> <br>-<br> int gpio_writec(GPIO* gpio, char value)<br> {<br> g_assert
(gpio != NULL);<br>diff --git a/objects/host_xstop_obj.c b/objects/host_xstop_obj.c<br>index 1de4d11..d1f8491 100644<br>--- a/objects/host_xstop_obj.c<br>+++ b/objects/host_xstop_obj.c<br>@@ -10,28 +10,19 @@<br> #include <systemd/sd-bus.h><br> #include "gpio.h"<br> <br>-/* <br>- * -----------------------------------------------<br>- * FIXME: Fetch GPIO NUM from MRW instead?<br>- * -----------------------------------------------<br>- */<br>-GPIO gpio_xstop = (GPIO){ "CHECKSTOP" }; /* This
object will use these GPIOs */<br>-const int gpio_num_xstop = 440;<br>-const char* gpio_dev_path = "/sys/class/gpio";<br>-const char* gpio_dev_exp = "/sys/class/gpio/export";<br>-const char* gpio_dev_xstop = "/sys/class/gpio/gpio440";<br>-const char* gpio_val_xstop = "/sys/class/gpio/gpio440/value";<br>-const char* gpio_dir_xstop = "/sys/class/gpio/gpio440/direction";<br>+static const char* xstop0 = "/org/openbmc/sensors/host/cpu0/Xstop";<br>+static const char* srvc_name = "org.openbmc.Sensors";<br>+static const char* iface_name = "org.openbmc.SensorValue";<br>+<br>+GPIO gpio_xstop0
= (GPIO){ "CHECKSTOP0" }; /* This object will
use these GPIOs */<br>+sd_bus* bus = NULL;<br>+sd_bus_slot* slot = NULL;<br> <br> typedef struct {<br> bool isXstopped;<br> } HostState;<br> <br>-HostState hostState = (HostState) {false};<br>-<br>-static const char* xstop0 = "/org/openbmc/sensors/host/cpu0/Xstop";<br>-static const char* srvc_name = "org.openbmc.Sensors";<br>-static const char* iface_name = "org.openbmc.SensorValue";<br>+HostState hostState0 = (HostState) {false};<br> <br> int bus_property_get_bool(sd_bus *bus, const char *path, const char *interface,
const char *property, sd_bus_message *reply, void *userdata, sd_bus_error
*error)<br> { <br>@@ -47,10 +38,6 @@ static const sd_bus_vtable host_xstop_vtable[] =<br> SD_BUS_VTABLE_END,<br> };<br> <br>-<br>-sd_bus* bus = NULL;<br>-sd_bus_slot* slot = NULL;<br>-<br> static gboolean<br> on_host_xstop (GIOChannel *channel, GIOCondition condition, gpointer user_data)<br> {<br>@@ -62,13 +49,11 @@ on_host_xstop (GIOChannel *channel, GIOCondition condition,
gpointer user_data)<br> g_io_channel_seek_position(channel,
0, G_SEEK_SET, 0);<br> GIOStatus
rc = g_io_channel_read_chars(channel, buf, 1, &bytes_read, &error);<br> <br>-
printf("%s\n",buf);<br>-
<br>-
if (gpio_xstop.irq_inited) {<br>- hostState.isXstopped = true;<br>+
if (gpio_xstop0.irq_inited) {<br>+ hostState0.isXstopped = true;<br> }<br> else {<br>- gpio_xstop.irq_inited = true;<br>+ gpio_xstop0.irq_inited = true;<br> }<br> <br> return
true;<br>@@ -77,11 +62,6 @@ on_host_xstop (GIOChannel *channel, GIOCondition condition,
gpointer user_data)<br> int main(void)<br> {<br> int rc =
0;<br>- int fd =
0;<br>- int result = 0;<br>- struct stat st;<br>-
char data[4];<br>-<br> sd_bus_error err = SD_BUS_ERROR_NULL;<br> sd_bus_message *res = NULL;<br> <br>@@ -104,19 +84,17 @@ int main(void)<br> goto cleanup;<br> }<br> <br>- /*<br>- * Setup GPIO IRQ for Host xstop<br>- */<br>+ /* Setup GPIO IRQ for Host xstop */<br> rc
= GPIO_OK;<br> do
{<br>-
rc
= gpio_init(bus, &gpio_xstop);<br>+
rc
= gpio_init(bus, &gpio_xstop0);<br>
if (rc != GPIO_OK) { goto cleanup; }<br>-
rc
= gpio_open_interrupt(&gpio_xstop, on_host_xstop, NULL/*object*/);<br>+
rc
= gpio_open_interrupt(&gpio_xstop0, on_host_xstop, NULL/*object*/);<br>
if (rc != GPIO_OK) { goto cleanup; }<br> }
while(0);<br> <br> if
(rc != GPIO_OK) {<br>-
printf("ERROR
PowerButton: GPIO setup (rc=%d)\n",rc);<br>+
fprintf(stderr,
"ERROR CHECKSTOP0: GPIO setup =%d\n", rc);<br> goto cleanup;<br> }<br> <br>@@ -127,18 +105,6 @@ int main(void)<br> goto cleanup;<br> }<br> <br>- /* Register for GPIO IRQ */<br>- gpio_xstop.irq_inited = false;<br>-
fd = open(gpio_val_xstop, O_RDONLY | O_NONBLOCK );<br>-
if (fd == -1) {<br>-
rc
= -1;<br>- goto cleanup;<br>-
}<br>-
else {<br>-
GIOChannel*
channel = g_io_channel_unix_new(fd);<br>-
g_io_add_watch(channel,
G_IO_PRI, on_host_xstop, NULL);<br>-
}<br>-<br> /* Register with Sensor Manager */<br> sd_bus_error_free(&err);<br> sd_bus_message_unref(res);<br>@@ -152,8 +118,8 @@ int main(void)<br>
&res,<br>
"ss",<br>
xstop0,<br>-
"HostXstopSensor");
/* FIXME: register method needs the Sensor class name ! */<br>-
<br>+
"HostXstopSensor");<br>+<br> if(rc < 0) {<br> fprintf(stderr, "Failed to init gpio
for %s : %s\n", xstop0, err.message);<br> return -1;<br>-- <br>2.8.2<br><br><br>_______________________________________________<br>openbmc mailing list<br>openbmc@lists.ozlabs.org<br></font></tt><a href=https://lists.ozlabs.org/listinfo/openbmc><tt><font size=2>https://lists.ozlabs.org/listinfo/openbmc</font></tt></a><tt><font size=2><br></font></tt><br><br><BR>