re: [PATCH 1/3] SLIMbus: Device management on SLIMbus

From: Joe Perches
Date: Sun Jun 14 2015 - 09:20:41 EST


> SLIMbus (Serial Low Power Interchip Media Bus) is a specification
> developed by MIPI (Mobile Industry Processor Interface) alliance.
> SLIMbus is a 2-wire implementation, which is used to communicate with
> peripheral components like audio-codec.

Perhaps these trivial style things could be considered

Use kernel-doc style /**
Argument alignment
Add newlines to logging formats
Convert if/else to if
Rename int slim_compare_eaddr to bool slim_eaddr_equal
---
drivers/slimbus/slimbus.c | 152 ++++++++++++++++++++++------------------------
include/linux/slimbus.h | 72 ++++++++++++----------
2 files changed, 111 insertions(+), 113 deletions(-)

diff --git a/drivers/slimbus/slimbus.c b/drivers/slimbus/slimbus.c
index be4f2c7..278e5a1 100644
--- a/drivers/slimbus/slimbus.c
+++ b/drivers/slimbus/slimbus.c
@@ -25,22 +25,21 @@ static DEFINE_IDR(ctrl_idr);
static struct device_type slim_dev_type;
static struct device_type slim_ctrl_type;

-static int slim_compare_eaddr(struct slim_eaddr *a, struct slim_eaddr *b)
+static bool slim_eaddr_equal(struct slim_eaddr *a, struct slim_eaddr *b)
{
- if (a->manf_id == b->manf_id && a->prod_code == b->prod_code &&
- a->dev_index == b->dev_index &&
- a->instance == b->instance)
- return 0;
- return -EIO;
+ return (a->manf_id == b->manf_id &&
+ a->prod_code == b->prod_code &&
+ a->dev_index == b->dev_index &&
+ a->instance == b->instance);
}

-static const struct slim_device_id *slim_match(const struct slim_device_id *id,
- const struct slim_device *slim_dev)
+static const struct slim_device_id *
+slim_match(const struct slim_device_id *id, const struct slim_device *slim_dev)
{
while (id->manf_id != 0 || id->prod_code != 0) {
if (id->manf_id == slim_dev->e_addr.manf_id &&
- id->prod_code == slim_dev->e_addr.prod_code &&
- id->dev_index == slim_dev->e_addr.dev_index)
+ id->prod_code == slim_dev->e_addr.prod_code &&
+ id->dev_index == slim_dev->e_addr.dev_index)
return id;
id++;
}
@@ -52,10 +51,10 @@ static int slim_device_match(struct device *dev, struct device_driver *driver)
struct slim_device *slim_dev;
struct slim_driver *drv = to_slim_driver(driver);

- if (dev->type == &slim_dev_type)
- slim_dev = to_slim_device(dev);
- else
+ if (dev->type != &slim_dev_type)
return 0;
+
+ slim_dev = to_slim_device(dev);
if (drv->id_table)
return slim_match(drv->id_table, slim_dev) != NULL;
return 0;
@@ -65,14 +64,13 @@ static int slim_device_probe(struct device *dev)
{
struct slim_device *slim_dev;
struct slim_driver *driver;
- struct slim_controller *ctrl;
+ struct slim_controller *ctrl;
int status = 0;

- if (dev->type == &slim_dev_type)
- slim_dev = to_slim_device(dev);
- else
+ if (dev->type != &slim_dev_type)
return -ENXIO;

+ slim_dev = to_slim_device(dev);
driver = to_slim_driver(dev->driver);
if (!driver->id_table)
return -ENODEV;
@@ -95,13 +93,13 @@ static int slim_device_remove(struct device *dev)
struct slim_driver *driver;
int status = 0;

- if (dev->type == &slim_dev_type)
- slim_dev = to_slim_device(dev);
- else
+ if (dev->type != &slim_dev_type)
return -ENXIO;

if (!dev->driver)
return 0;
+
+ slim_dev = to_slim_device(dev);
driver = to_slim_driver(dev->driver);
if (driver->remove)
status = driver->remove(slim_dev);
@@ -117,13 +115,11 @@ static void slim_device_shutdown(struct device *dev)
struct slim_device *slim_dev;
struct slim_driver *driver;

- if (dev->type == &slim_dev_type)
- slim_dev = to_slim_device(dev);
- else
+ if (dev->type != &slim_dev_type)
return;
-
if (!dev->driver)
return;
+ slim_dev = to_slim_device(dev);
driver = to_slim_driver(dev->driver);
if (driver->shutdown)
driver->shutdown(slim_dev);
@@ -135,20 +131,18 @@ static int slim_pm_suspend(struct device *dev)
{
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;

- if (pm)
- return pm_generic_suspend(dev);
- else
+ if (!pm)
return 0;
+ return pm_generic_suspend(dev);
}

static int slim_pm_resume(struct device *dev)
{
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;

- if (pm)
- return pm_generic_resume(dev);
- else
+ if (!pm)
return 0;
+ return pm_generic_resume(dev);
}

#else
@@ -159,12 +153,9 @@ static int slim_pm_resume(struct device *dev)
static const struct dev_pm_ops slimbus_pm = {
.suspend = slim_pm_suspend,
.resume = slim_pm_resume,
- SET_RUNTIME_PM_OPS(
- pm_generic_suspend,
- pm_generic_resume,
- NULL
- )
+ SET_RUNTIME_PM_OPS(pm_generic_suspend, pm_generic_resume, NULL)
};
+
struct bus_type slimbus_type = {
.name = "slimbus",
.match = slim_device_match,
@@ -257,13 +248,13 @@ static struct device_type slim_dev_type = {
static void slim_report(struct work_struct *work)
{
struct slim_driver *sbdrv;
- struct slim_device *sbdev =
- container_of(work, struct slim_device, wd);
+ struct slim_device *sbdev = container_of(work, struct slim_device, wd);
+
if (!sbdev->dev.driver)
return;
/* check if device-up or down needs to be called */
if ((!sbdev->reported && !sbdev->notified) ||
- (sbdev->reported && sbdev->notified))
+ (sbdev->reported && sbdev->notified))
return;

sbdrv = to_slim_driver(sbdev->dev.driver);
@@ -282,7 +273,7 @@ static void slim_report(struct work_struct *work)
}
}

-/*
+/**
* slim_add_device: Add a new device without register board info.
* @ctrl: Controller to which this device is to be added to.
* Called when device doesn't have an explicit client-driver to be probed, or
@@ -298,15 +289,15 @@ int slim_add_device(struct slim_controller *ctrl, struct slim_device *sbdev)
slim_ctrl_get(ctrl);
if (!sbdev->name) {
sbdev->name = kcalloc(SLIMBUS_NAME_SIZE, sizeof(char),
- GFP_KERNEL);
+ GFP_KERNEL);
if (!sbdev->name)
return -ENOMEM;
snprintf(sbdev->name, SLIMBUS_NAME_SIZE, "0x%x:0x%x:0x%x:0x%x",
- sbdev->e_addr.manf_id, sbdev->e_addr.prod_code,
- sbdev->e_addr.dev_index,
- sbdev->e_addr.instance);
+ sbdev->e_addr.manf_id, sbdev->e_addr.prod_code,
+ sbdev->e_addr.dev_index,
+ sbdev->e_addr.instance);
}
- dev_dbg(&ctrl->dev, "adding device:%s", sbdev->name);
+ dev_dbg(&ctrl->dev, "adding device: %s\n", sbdev->name);
dev_set_name(&sbdev->dev, "%s", sbdev->name);
INIT_WORK(&sbdev->wd, slim_report);
mutex_lock(&ctrl->m_ctrl);
@@ -328,7 +319,7 @@ static DEFINE_MUTEX(board_lock);

/* If controller is not present, only add to boards list */
static void slim_match_ctrl_to_boardinfo(struct slim_controller *ctrl,
- struct slim_boardinfo *bi)
+ struct slim_boardinfo *bi)
{
int ret;

@@ -337,7 +328,7 @@ static void slim_match_ctrl_to_boardinfo(struct slim_controller *ctrl,

ret = slim_add_device(ctrl, bi->slim_slave);
if (ret != 0)
- dev_err(ctrl->dev.parent, "can't create new device %s, ret:%d",
+ dev_err(ctrl->dev.parent, "can't create new device %s, ret:%d\n",
bi->slim_slave->name, ret);
}

@@ -371,7 +362,7 @@ int slim_register_board_info(struct slim_boardinfo const *info, unsigned n)
}
EXPORT_SYMBOL(slim_register_board_info);

-/*
+/**
* slim_ctrl_add_boarddevs: Add devices registered by board-info
* @ctrl: Controller to which these devices are to be added to.
* This API is called by controller when it is up and running.
@@ -413,8 +404,8 @@ static int slim_register_controller(struct slim_controller *ctrl)
if (ret)
goto out_list;

- dev_dbg(&ctrl->dev, "Bus [%s] registered:dev:%p\n", ctrl->name,
- &ctrl->dev);
+ dev_dbg(&ctrl->dev, "Bus [%s] registered:dev:%p\n",
+ ctrl->name, &ctrl->dev);

INIT_LIST_HEAD(&ctrl->devs);
ctrl->wq = create_singlethread_workqueue(dev_name(&ctrl->dev));
@@ -445,15 +436,15 @@ void slim_remove_device(struct slim_device *sbdev)
EXPORT_SYMBOL(slim_remove_device);

static void slim_ctrl_remove_device(struct slim_controller *ctrl,
- struct slim_boardinfo *bi)
+ struct slim_boardinfo *bi)
{
if (ctrl->nr == bi->bus_num)
slim_remove_device(bi->slim_slave);
}

-/*
+/**
* slim_del_controller: Controller tear-down.
- * Controller added with the above API is teared down using this API.
+ * @ctrl: Controller to tear-down.
*/
int slim_del_controller(struct slim_controller *ctrl)
{
@@ -488,7 +479,7 @@ int slim_del_controller(struct slim_controller *ctrl)
}
EXPORT_SYMBOL(slim_del_controller);

-/*
+/**
* slim_add_numbered_controller: Controller bring-up.
* @ctrl: Controller to be registered.
* A controller is registered with the framework using this API. ctrl->nr is the
@@ -511,7 +502,7 @@ int slim_add_numbered_controller(struct slim_controller *ctrl)
}
EXPORT_SYMBOL(slim_add_numbered_controller);

-/*
+/**
* slim_report_absent: Controller calls this function when a device
* reports absent, OR when the device cannot be communicated with
* @sbdev: Device that cannot be reached, or sent report absent
@@ -538,13 +529,14 @@ void slim_report_absent(struct slim_device *sbdev)
}
EXPORT_SYMBOL(slim_report_absent);

-/*
+/**
* slim_framer_booted: This function is called by controller after the active
* framer has booted (using Bus Reset sequence, or after it has shutdown and has
- * come back up). Components, devices on the bus may be in undefined state,
- * and this function triggers their drivers to do the needful
- * to bring them back in Reset state so that they can acquire sync, report
- * present and be operational again.
+ * come back up).
+ * @ctrl: Controller
+ * Components, devices on the bus may be in undefined state, and this function
+ * triggers their drivers to do the needful to bring them back in Reset state
+ * so that they can acquire sync, report present and be operational again.
*/
void slim_framer_booted(struct slim_controller *ctrl)
{
@@ -570,7 +562,7 @@ void slim_framer_booted(struct slim_controller *ctrl)
}
EXPORT_SYMBOL(slim_framer_booted);

-/*
+/**
* slim_query_device: Query and get handle to a device.
* @ctrl: Controller on which this device will be added/queried
* @e_addr: Enumeration address of the device to be queried
@@ -578,7 +570,7 @@ EXPORT_SYMBOL(slim_framer_booted);
* device and returns pointer to it if the device has not yet enumerated.
*/
struct slim_device *slim_query_device(struct slim_controller *ctrl,
- struct slim_eaddr *e_addr)
+ struct slim_eaddr *e_addr)
{
struct slim_device *slim = NULL;
struct sbi_boardinfo *bi;
@@ -588,8 +580,8 @@ struct slim_device *slim_query_device(struct slim_controller *ctrl,
list_for_each_entry(bi, &board_list, list) {
if (bi->board_info.bus_num != ctrl->nr)
continue;
- if (slim_compare_eaddr(&bi->board_info.slim_slave->e_addr,
- e_addr) == 0) {
+ if (slim_eaddr_equal(&bi->board_info.slim_slave->e_addr,
+ e_addr)) {
slim = bi->board_info.slim_slave;
break;
}
@@ -600,14 +592,14 @@ struct slim_device *slim_query_device(struct slim_controller *ctrl,

mutex_lock(&ctrl->m_ctrl);
list_for_each_entry(slim, &ctrl->devs, dev_list) {
- if (slim_compare_eaddr(&slim->e_addr, e_addr) == 0) {
+ if (slim_eaddr_equal(&slim->e_addr, e_addr)) {
mutex_unlock(&ctrl->m_ctrl);
return slim;
}
}
mutex_unlock(&ctrl->m_ctrl);

- slim = kcalloc(1, sizeof(struct slim_device), GFP_KERNEL);
+ slim = kzalloc(sizeof(struct slim_device), GFP_KERNEL);
if (IS_ERR(slim))
return NULL;
slim->e_addr = *e_addr;
@@ -620,13 +612,13 @@ struct slim_device *slim_query_device(struct slim_controller *ctrl,
EXPORT_SYMBOL(slim_query_device);

static int ctrl_getaddr_entry(struct slim_controller *ctrl,
- struct slim_eaddr *eaddr, u8 *entry)
+ struct slim_eaddr *eaddr, u8 *entry)
{
int i;

for (i = 0; i < ctrl->num_dev; i++) {
if (ctrl->addrt[i].valid &&
- slim_compare_eaddr(&ctrl->addrt[i].eaddr, eaddr) == 0) {
+ slim_eaddr_equal(&ctrl->addrt[i].eaddr, eaddr)) {
*entry = i;
return 0;
}
@@ -634,21 +626,21 @@ static int ctrl_getaddr_entry(struct slim_controller *ctrl,
return -ENXIO;
}

-/*
+/**
* slim_assign_laddr: Assign logical address to a device enumerated.
* @ctrl: Controller with which device is enumerated.
* @e_addr: Enumeration address of the device.
- * @laddr: Return logical address (if valid flag is false)
- * @valid: true if laddr holds a valid address that controller wants to
- * set for this enumeration address. Otherwise framework sets index into
- * address table as logical address.
+ * @laddr: Return logical address (if valid flag is false)
+ * @valid: true if laddr holds a valid address that controller wants to
+ * set for this enumeration address. Otherwise framework sets index into
+ * address table as logical address.
* Called by controller in response to REPORT_PRESENT. Framework will assign
* a logical address to this enumeration address.
* Function returns -EXFULL to indicate that all logical addresses are already
* taken.
*/
int slim_assign_laddr(struct slim_controller *ctrl, struct slim_eaddr *e_addr,
- u8 *laddr, bool valid)
+ u8 *laddr, bool valid)
{
int ret;
u8 i = 0;
@@ -701,9 +693,9 @@ ret_assigned_laddr:
if (exists || ret)
return ret;

- pr_info("setting slimbus l-addr:%x, ea:%x,%x,%x,%x",
- *laddr, e_addr->manf_id, e_addr->prod_code,
- e_addr->dev_index, e_addr->instance);
+ pr_info("setting slimbus l-addr:%x, ea:%x,%x,%x,%x\n",
+ *laddr, e_addr->manf_id, e_addr->prod_code,
+ e_addr->dev_index, e_addr->instance);
/*
* Add this device to list of devices on this controller if it's
* not already present
@@ -727,17 +719,17 @@ ret_assigned_laddr:
}
EXPORT_SYMBOL(slim_assign_laddr);

-/*
+/**
* slim_get_logical_addr: Return the logical address of a slimbus device.
* @sb: client handle requesting the adddress.
* @e_addr: Enumeration address of the device.
* @laddr: output buffer to store the address
* context: can sleep
- * -EINVAL is returned in case of invalid parameters, and -ENXIO is returned if
- * the device with this enumeration address is not found.
+ * -EINVAL is returned in case of invalid parameters
+ * -ENXIO is returned if the device with this enumeration address is not found
*/
int slim_get_logical_addr(struct slim_device *sb, struct slim_eaddr *e_addr,
- u8 *laddr)
+ u8 *laddr)
{
int ret;
u8 entry;
diff --git a/include/linux/slimbus.h b/include/linux/slimbus.h
index 05b7594..ed93da0 100644
--- a/include/linux/slimbus.h
+++ b/include/linux/slimbus.h
@@ -37,7 +37,7 @@ extern struct bus_type slimbus_type;
struct slim_controller;
struct slim_device;

-/*
+/**
* struct slim_eaddr: Enumeration address for a slimbus device
* @manf_id: Manufacturer Id for the device
* @prod_code: Product code
@@ -51,7 +51,7 @@ struct slim_eaddr {
u8 instance;
};

-/*
+/**
* struct slim_framer - Represents Slimbus framer.
* Every controller may have multiple framers. There is 1 active framer device
* responsible for clocking the bus.
@@ -66,9 +66,10 @@ struct slim_framer {
int rootfreq;
int superfreq;
};
+
#define to_slim_framer(d) container_of(d, struct slim_framer, dev)

-/*
+/**
* struct slim_addrt: slimbus address used internally by the slimbus framework.
* @valid: If the device is present. Valid is set to false when device reports
* absent.
@@ -102,7 +103,7 @@ struct slim_addrt {
#define SLIM_MSG_DEST_ENUMADDR 1
#define SLIM_MSG_DEST_BROADCAST 3

-/*
+/**
* struct slim_controller: Controls every instance of SLIMbus
* (similar to 'master' on SPI)
* 'Manager device' is responsible for device management, bandwidth
@@ -165,13 +166,14 @@ struct slim_controller {
struct workqueue_struct *wq;
struct completion dev_released;
int (*set_laddr)(struct slim_controller *ctrl,
- struct slim_eaddr *ea, u8 laddr);
+ struct slim_eaddr *ea, u8 laddr);
int (*get_laddr)(struct slim_controller *ctrl,
- struct slim_eaddr *ea, u8 *laddr);
+ struct slim_eaddr *ea, u8 *laddr);
};
+
#define to_slim_controller(d) container_of(d, struct slim_controller, dev)

-/*
+/**
* struct slim_driver: Slimbus 'generic device' (slave) device driver
* (similar to 'spi_device' on SPI)
* @probe: Binds this driver to a slimbus device.
@@ -196,7 +198,7 @@ struct slim_driver {
int (*remove)(struct slim_device *sl);
void (*shutdown)(struct slim_device *sl);
int (*suspend)(struct slim_device *sl,
- pm_message_t pmesg);
+ pm_message_t pmesg);
int (*resume)(struct slim_device *sl);
int (*device_up)(struct slim_device *sl);
int (*device_down)(struct slim_device *sl);
@@ -205,9 +207,10 @@ struct slim_driver {
struct device_driver driver;
const struct slim_device_id *id_table;
};
+
#define to_slim_driver(d) container_of(d, struct slim_driver, driver)

-/*
+/**
* Client/device handle (struct slim_device):
* ------------------------------------------
* This is the client/device handle returned when a slimbus
@@ -242,9 +245,10 @@ struct slim_device {
struct list_head dev_list;
struct work_struct wd;
};
+
#define to_slim_device(d) container_of(d, struct slim_device, dev)

-/*
+/**
* struct slim_boardinfo: Declare board info for Slimbus device bringup.
* @bus_num: Controller number (bus) on which this device will sit.
* @slim_slave: Device to be registered with slimbus.
@@ -256,7 +260,7 @@ struct slim_boardinfo {

/* Manager's logical address is set to 0xFF per spec */
#define SLIM_LA_MANAGER 0xFF
-/*
+/**
* slim_get_logical_addr: Return the logical address of a slimbus device.
* @sb: client handle requesting the adddress.
* @e_addr: Enumeration address of the device.
@@ -266,51 +270,53 @@ struct slim_boardinfo {
* the device with this elemental address is not found.
*/

-extern int slim_get_logical_addr(struct slim_device *sb,
- struct slim_eaddr *e_addr, u8 *laddr);
+int slim_get_logical_addr(struct slim_device *sb,
+ struct slim_eaddr *e_addr, u8 *laddr);

-/*
+/**
* slim_driver_register: Client driver registration with slimbus
* @drv:Client driver to be associated with client-device.
* This API will register the client driver with the slimbus
* It is called from the driver's module-init function.
*/
-extern int slim_driver_register(struct slim_driver *drv);
+int slim_driver_register(struct slim_driver *drv);

/*
* slim_driver_unregister: Undo effects of slim_driver_register
* @drv: Client driver to be unregistered
*/
-extern void slim_driver_unregister(struct slim_driver *drv);
+void slim_driver_unregister(struct slim_driver *drv);

-/*
+/**
* slim_add_numbered_controller: Controller bring-up.
* @ctrl: Controller to be registered.
* A controller is registered with the framework using this API. ctrl->nr is the
* desired number with which slimbus framework registers the controller.
* Function will return -EBUSY if the number is in use.
*/
-extern int slim_add_numbered_controller(struct slim_controller *ctrl);
+int slim_add_numbered_controller(struct slim_controller *ctrl);

/*
* slim_del_controller: Controller tear-down.
+ * @ctrl: Controller to be torn-down.
* Controller added with the above API is teared down using this API.
*/
-extern int slim_del_controller(struct slim_controller *ctrl);
+int slim_del_controller(struct slim_controller *ctrl);

-/*
+/**
* slim_add_device: Add a new device without register board info.
* @ctrl: Controller to which this device is to be added to.
+ * @sbdev: slim_device to add
* Called when device doesn't have an explicit client-driver to be probed, or
* the client-driver is a module installed dynamically.
*/
-extern int slim_add_device(struct slim_controller *ctrl,
- struct slim_device *sbdev);
+int slim_add_device(struct slim_controller *ctrl,
+ struct slim_device *sbdev);

/* slim_remove_device: Remove the effect of slim_add_device() */
-extern void slim_remove_device(struct slim_device *sbdev);
+void slim_remove_device(struct slim_device *sbdev);

-/*
+/**
* slim_assign_laddr: Assign logical address to a device enumerated.
* @ctrl: Controller with which device is enumerated.
* @e_addr: Enumeration address of the device.
@@ -323,10 +329,10 @@ extern void slim_remove_device(struct slim_device *sbdev);
* Function returns -EXFULL to indicate that all logical addresses are already
* taken.
*/
-extern int slim_assign_laddr(struct slim_controller *ctrl,
- struct slim_eaddr *e_addr, u8 *laddr, bool valid);
+int slim_assign_laddr(struct slim_controller *ctrl,
+ struct slim_eaddr *e_addr, u8 *laddr, bool valid);

-/*
+/**
* slim_report_absent: Controller calls this function when a device
* reports absent, OR when the device cannot be communicated with
* @sbdev: Device that cannot be reached, or that sent report absent
@@ -343,16 +349,16 @@ void slim_report_absent(struct slim_device *sbdev);
*/
void slim_framer_booted(struct slim_controller *ctrl);

-/*
+/**
* slim_ctrl_add_boarddevs: Add devices registered by board-info
* @ctrl: Controller to which these devices are to be added to.
* This API is called by controller when it is up and running.
* If devices on a controller were registered before controller,
* this will make sure that they get probed when controller is up
*/
-extern void slim_ctrl_add_boarddevs(struct slim_controller *ctrl);
+void slim_ctrl_add_boarddevs(struct slim_controller *ctrl);

-/*
+/**
* slim_register_board_info: Board-initialization routine.
* @info: List of all devices on all controllers present on the board.
* @n: number of entries.
@@ -360,11 +366,11 @@ extern void slim_ctrl_add_boarddevs(struct slim_controller *ctrl);
* Called from board-init function.
*/
#ifdef CONFIG_SLIMBUS
-extern int slim_register_board_info(struct slim_boardinfo const *info,
- unsigned n);
+int slim_register_board_info(struct slim_boardinfo const *info,
+ unsigned n);
#else
static inline int slim_register_board_info(struct slim_boardinfo const *info,
- unsigned n)
+ unsigned n)
{
return 0;
}


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/