Re: [PATCH V2 1/6] SLIMbus: Device management on SLIMbus

2015-06-19 Thread Daniel Thompson

On 17/06/15 02:45, Sagar Dharia wrote:

--- /dev/null
+++ b/drivers/slimbus/Kconfig
@@ -0,0 +1,9 @@
+#
+# SLIMBUS driver configuration
+#
+menuconfig SLIMBUS
+   tristate "Slimbus support"
+   help
+ Slimbus is standard interface between baseband and audio codec,
+ and other peripheral components in mobile terminals.


Perhaps this is a fussy comment but this description of slimbus makes 
pretty heavy use of mobile phone jargon. Admittedly it is jargon from 
the slimbus spec which, as standards go, seems rather myopic about its 
potential user base.


However, even the standards own self-description doesn't limit its role 
to baseband <-> peripherals. What happened to the application 
processor/main-SoC?


Also would an "If unsure, choose N." be useful? It could be quite a long 
time before someone who doesn't know they need slimbus actually finds 
that they do.



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


Re: [PATCH V2 1/6] SLIMbus: Device management on SLIMbus

2015-06-19 Thread Daniel Thompson

On 17/06/15 02:45, Sagar Dharia wrote:

--- /dev/null
+++ b/drivers/slimbus/Kconfig
@@ -0,0 +1,9 @@
+#
+# SLIMBUS driver configuration
+#
+menuconfig SLIMBUS
+   tristate Slimbus support
+   help
+ Slimbus is standard interface between baseband and audio codec,
+ and other peripheral components in mobile terminals.


Perhaps this is a fussy comment but this description of slimbus makes 
pretty heavy use of mobile phone jargon. Admittedly it is jargon from 
the slimbus spec which, as standards go, seems rather myopic about its 
potential user base.


However, even the standards own self-description doesn't limit its role 
to baseband - peripherals. What happened to the application 
processor/main-SoC?


Also would an If unsure, choose N. be useful? It could be quite a long 
time before someone who doesn't know they need slimbus actually finds 
that they do.



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


Re: [PATCH V2 1/6] SLIMbus: Device management on SLIMbus

2015-06-18 Thread Srinivas Kandagatla



On 17/06/15 02:45, Sagar Dharia wrote:

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.
SLIMbus uses Time-Division-Multiplexing to accommodate multiple data
channels, and control channel. Control channel has messages to do
device-enumeration, messages to send/receive control-data to/from
slimbus devices, messages for port/channel management, and messages to
do bandwidth allocation.
The framework supports multiple instances of the bus (1 controller per
bus), and multiple slave devices per controller.

This patch does device enumeration, logical address assignment,
informing device when the device reports present/absent etc.
Reporting present may need the driver to do the needful (e.g. turning
on voltage regulators powering the device). So probe is called
if the device is added to board-info list for a controller.
Additionally device is probed when it reports present if that device
doesn't need any such steps mentioned above.

Signed-off-by: Sagar Dharia 


I think I was not as fast as you sent the v2 :-)
However my comments on v1 patcset mostly still applies to v2 as well.

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


Re: [PATCH V2 1/6] SLIMbus: Device management on SLIMbus

2015-06-18 Thread Srinivas Kandagatla



On 17/06/15 02:45, Sagar Dharia wrote:

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.
SLIMbus uses Time-Division-Multiplexing to accommodate multiple data
channels, and control channel. Control channel has messages to do
device-enumeration, messages to send/receive control-data to/from
slimbus devices, messages for port/channel management, and messages to
do bandwidth allocation.
The framework supports multiple instances of the bus (1 controller per
bus), and multiple slave devices per controller.

This patch does device enumeration, logical address assignment,
informing device when the device reports present/absent etc.
Reporting present may need the driver to do the needful (e.g. turning
on voltage regulators powering the device). So probe is called
if the device is added to board-info list for a controller.
Additionally device is probed when it reports present if that device
doesn't need any such steps mentioned above.

Signed-off-by: Sagar Dharia sdha...@codeaurora.org


I think I was not as fast as you sent the v2 :-)
However my comments on v1 patcset mostly still applies to v2 as well.

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


Re: [PATCH V2 1/6] SLIMbus: Device management on SLIMbus

2015-06-17 Thread Sagar Dharia

On 6/17/2015 7:16 AM, Mark Brown wrote:

On Tue, Jun 16, 2015 at 07:45:59PM -0600, Sagar Dharia wrote:


+   if (status) {
+   slim_dev->driver = NULL;
+   } else if (driver->device_up) {
+   ctrl = slim_dev->ctrl;
+   queue_work(ctrl->wq, _dev->wd);
+   }

Nothing ever cleans this work up if it didn't manage to run or
complete.


+static void slim_report(struct work_struct *work)
+{
+   struct slim_driver *sbdrv;
+   struct slim_device *sbdev = container_of(work, struct slim_device, wd);
+
+   if (!sbdev->dev.driver)
+   return;

So we just forget about the device if we don't have a driver for it?
If device comes up before driver (e.g. dynamic module driver, or 
late-init driver), then to begin with: this doesn't do anything.
But once the driver-binding happens and probe succeeds, we queue 
device_up again.



+   /* check if device-up or down needs to be called */
+   if ((!sbdev->reported && !sbdev->notified) ||
+   (sbdev->reported && sbdev->notified))
+   return;

No locking here?
You are right, first I thought this was only touched in workqueue 
(singlethreaded), but this is also touched from logical-address 
assignment, driver removal etc.

I will add device-level lock for these.
Thanks
Sagar



+/**
+ * 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.
+ */
+void slim_ctrl_add_boarddevs(struct slim_controller *ctrl)

My concerns about the split here still remain.



--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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


Re: [PATCH V2 1/6] SLIMbus: Device management on SLIMbus

2015-06-17 Thread Mark Brown
On Tue, Jun 16, 2015 at 07:45:59PM -0600, Sagar Dharia wrote:

> + if (status) {
> + slim_dev->driver = NULL;
> + } else if (driver->device_up) {
> + ctrl = slim_dev->ctrl;
> + queue_work(ctrl->wq, _dev->wd);
> + }

Nothing ever cleans this work up if it didn't manage to run or
complete.

> +static void slim_report(struct work_struct *work)
> +{
> + struct slim_driver *sbdrv;
> + struct slim_device *sbdev = container_of(work, struct slim_device, wd);
> +
> + if (!sbdev->dev.driver)
> + return;

So we just forget about the device if we don't have a driver for it?

> + /* check if device-up or down needs to be called */
> + if ((!sbdev->reported && !sbdev->notified) ||
> + (sbdev->reported && sbdev->notified))
> + return;

No locking here?

> +/**
> + * 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.
> + */
> +void slim_ctrl_add_boarddevs(struct slim_controller *ctrl)

My concerns about the split here still remain.


signature.asc
Description: Digital signature


Re: [PATCH V2 1/6] SLIMbus: Device management on SLIMbus

2015-06-17 Thread Sagar Dharia

On 6/17/2015 7:16 AM, Mark Brown wrote:

On Tue, Jun 16, 2015 at 07:45:59PM -0600, Sagar Dharia wrote:


+   if (status) {
+   slim_dev-driver = NULL;
+   } else if (driver-device_up) {
+   ctrl = slim_dev-ctrl;
+   queue_work(ctrl-wq, slim_dev-wd);
+   }

Nothing ever cleans this work up if it didn't manage to run or
complete.


+static void slim_report(struct work_struct *work)
+{
+   struct slim_driver *sbdrv;
+   struct slim_device *sbdev = container_of(work, struct slim_device, wd);
+
+   if (!sbdev-dev.driver)
+   return;

So we just forget about the device if we don't have a driver for it?
If device comes up before driver (e.g. dynamic module driver, or 
late-init driver), then to begin with: this doesn't do anything.
But once the driver-binding happens and probe succeeds, we queue 
device_up again.



+   /* check if device-up or down needs to be called */
+   if ((!sbdev-reported  !sbdev-notified) ||
+   (sbdev-reported  sbdev-notified))
+   return;

No locking here?
You are right, first I thought this was only touched in workqueue 
(singlethreaded), but this is also touched from logical-address 
assignment, driver removal etc.

I will add device-level lock for these.
Thanks
Sagar



+/**
+ * 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.
+ */
+void slim_ctrl_add_boarddevs(struct slim_controller *ctrl)

My concerns about the split here still remain.



--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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


Re: [PATCH V2 1/6] SLIMbus: Device management on SLIMbus

2015-06-17 Thread Mark Brown
On Tue, Jun 16, 2015 at 07:45:59PM -0600, Sagar Dharia wrote:

 + if (status) {
 + slim_dev-driver = NULL;
 + } else if (driver-device_up) {
 + ctrl = slim_dev-ctrl;
 + queue_work(ctrl-wq, slim_dev-wd);
 + }

Nothing ever cleans this work up if it didn't manage to run or
complete.

 +static void slim_report(struct work_struct *work)
 +{
 + struct slim_driver *sbdrv;
 + struct slim_device *sbdev = container_of(work, struct slim_device, wd);
 +
 + if (!sbdev-dev.driver)
 + return;

So we just forget about the device if we don't have a driver for it?

 + /* check if device-up or down needs to be called */
 + if ((!sbdev-reported  !sbdev-notified) ||
 + (sbdev-reported  sbdev-notified))
 + return;

No locking here?

 +/**
 + * 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.
 + */
 +void slim_ctrl_add_boarddevs(struct slim_controller *ctrl)

My concerns about the split here still remain.


signature.asc
Description: Digital signature


Re: [PATCH V2 1/6] SLIMbus: Device management on SLIMbus

2015-06-16 Thread Joe Perches
On Tue, 2015-06-16 at 19:45 -0600, Sagar Dharia wrote:
> 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.
[]
> diff --git a/drivers/slimbus/slimbus.c b/drivers/slimbus/slimbus.c
[]
> +static bool slim_eaddr_equal(struct slim_eaddr *a, struct slim_eaddr *b)
> +{
> + 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)
> +{
> + 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)
> + return id;
> + id++;
> + }
> + return NULL;
> +}
> +
> +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 != _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;
> +}

This should probably be a bool function return.

Maybe this:

static bool slim_device_match(struct device *dev, struct device_driver *driver)
{
struct slim_driver *drv = to_slim_driver(driver);

if (dev->type != _dev_type || !drv->id_table)
return false;

return slim_match(drv->id_table, to_slim_device(dev));
}


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


[PATCH V2 1/6] SLIMbus: Device management on SLIMbus

2015-06-16 Thread Sagar Dharia
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.
SLIMbus uses Time-Division-Multiplexing to accommodate multiple data
channels, and control channel. Control channel has messages to do
device-enumeration, messages to send/receive control-data to/from
slimbus devices, messages for port/channel management, and messages to
do bandwidth allocation.
The framework supports multiple instances of the bus (1 controller per
bus), and multiple slave devices per controller.

This patch does device enumeration, logical address assignment,
informing device when the device reports present/absent etc.
Reporting present may need the driver to do the needful (e.g. turning
on voltage regulators powering the device). So probe is called
if the device is added to board-info list for a controller.
Additionally device is probed when it reports present if that device
doesn't need any such steps mentioned above.

Signed-off-by: Sagar Dharia 
---
 drivers/Kconfig |   2 +
 drivers/Makefile|   1 +
 drivers/slimbus/Kconfig |   9 +
 drivers/slimbus/Makefile|   4 +
 drivers/slimbus/slimbus.c   | 714 
 include/linux/mod_devicetable.h |  13 +
 include/linux/slimbus.h | 396 ++
 7 files changed, 1139 insertions(+)
 create mode 100644 drivers/slimbus/Kconfig
 create mode 100644 drivers/slimbus/Makefile
 create mode 100644 drivers/slimbus/slimbus.c
 create mode 100644 include/linux/slimbus.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index c0cc96b..e39c969 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -182,4 +182,6 @@ source "drivers/thunderbolt/Kconfig"
 
 source "drivers/android/Kconfig"
 
+source "drivers/slimbus/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 46d2554..37c1c88 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -74,6 +74,7 @@ obj-$(CONFIG_TARGET_CORE) += target/
 obj-$(CONFIG_MTD)  += mtd/
 obj-$(CONFIG_SPI)  += spi/
 obj-$(CONFIG_SPMI) += spmi/
+obj-$(CONFIG_SLIMBUS)  += slimbus/
 obj-y  += hsi/
 obj-y  += net/
 obj-$(CONFIG_ATM)  += atm/
diff --git a/drivers/slimbus/Kconfig b/drivers/slimbus/Kconfig
new file mode 100644
index 000..fb30497
--- /dev/null
+++ b/drivers/slimbus/Kconfig
@@ -0,0 +1,9 @@
+#
+# SLIMBUS driver configuration
+#
+menuconfig SLIMBUS
+   tristate "Slimbus support"
+   help
+ Slimbus is standard interface between baseband and audio codec,
+ and other peripheral components in mobile terminals.
+
diff --git a/drivers/slimbus/Makefile b/drivers/slimbus/Makefile
new file mode 100644
index 000..05f53bc
--- /dev/null
+++ b/drivers/slimbus/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for kernel slimbus framework.
+#
+obj-$(CONFIG_SLIMBUS)  += slimbus.o
diff --git a/drivers/slimbus/slimbus.c b/drivers/slimbus/slimbus.c
new file mode 100644
index 000..2baf43a
--- /dev/null
+++ b/drivers/slimbus/slimbus.c
@@ -0,0 +1,714 @@
+/* Copyright (c) 2011-2015, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static DEFINE_MUTEX(slim_lock);
+static DEFINE_IDR(ctrl_idr);
+static struct device_type slim_dev_type;
+static struct device_type slim_ctrl_type;
+
+static bool slim_eaddr_equal(struct slim_eaddr *a, struct slim_eaddr *b)
+{
+   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)
+{
+   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)
+   return id;
+   id++;
+   }
+   return NULL;
+}
+
+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);
+
+   

[PATCH V2 1/6] SLIMbus: Device management on SLIMbus

2015-06-16 Thread Sagar Dharia
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.
SLIMbus uses Time-Division-Multiplexing to accommodate multiple data
channels, and control channel. Control channel has messages to do
device-enumeration, messages to send/receive control-data to/from
slimbus devices, messages for port/channel management, and messages to
do bandwidth allocation.
The framework supports multiple instances of the bus (1 controller per
bus), and multiple slave devices per controller.

This patch does device enumeration, logical address assignment,
informing device when the device reports present/absent etc.
Reporting present may need the driver to do the needful (e.g. turning
on voltage regulators powering the device). So probe is called
if the device is added to board-info list for a controller.
Additionally device is probed when it reports present if that device
doesn't need any such steps mentioned above.

Signed-off-by: Sagar Dharia sdha...@codeaurora.org
---
 drivers/Kconfig |   2 +
 drivers/Makefile|   1 +
 drivers/slimbus/Kconfig |   9 +
 drivers/slimbus/Makefile|   4 +
 drivers/slimbus/slimbus.c   | 714 
 include/linux/mod_devicetable.h |  13 +
 include/linux/slimbus.h | 396 ++
 7 files changed, 1139 insertions(+)
 create mode 100644 drivers/slimbus/Kconfig
 create mode 100644 drivers/slimbus/Makefile
 create mode 100644 drivers/slimbus/slimbus.c
 create mode 100644 include/linux/slimbus.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index c0cc96b..e39c969 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -182,4 +182,6 @@ source drivers/thunderbolt/Kconfig
 
 source drivers/android/Kconfig
 
+source drivers/slimbus/Kconfig
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 46d2554..37c1c88 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -74,6 +74,7 @@ obj-$(CONFIG_TARGET_CORE) += target/
 obj-$(CONFIG_MTD)  += mtd/
 obj-$(CONFIG_SPI)  += spi/
 obj-$(CONFIG_SPMI) += spmi/
+obj-$(CONFIG_SLIMBUS)  += slimbus/
 obj-y  += hsi/
 obj-y  += net/
 obj-$(CONFIG_ATM)  += atm/
diff --git a/drivers/slimbus/Kconfig b/drivers/slimbus/Kconfig
new file mode 100644
index 000..fb30497
--- /dev/null
+++ b/drivers/slimbus/Kconfig
@@ -0,0 +1,9 @@
+#
+# SLIMBUS driver configuration
+#
+menuconfig SLIMBUS
+   tristate Slimbus support
+   help
+ Slimbus is standard interface between baseband and audio codec,
+ and other peripheral components in mobile terminals.
+
diff --git a/drivers/slimbus/Makefile b/drivers/slimbus/Makefile
new file mode 100644
index 000..05f53bc
--- /dev/null
+++ b/drivers/slimbus/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for kernel slimbus framework.
+#
+obj-$(CONFIG_SLIMBUS)  += slimbus.o
diff --git a/drivers/slimbus/slimbus.c b/drivers/slimbus/slimbus.c
new file mode 100644
index 000..2baf43a
--- /dev/null
+++ b/drivers/slimbus/slimbus.c
@@ -0,0 +1,714 @@
+/* Copyright (c) 2011-2015, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/kernel.h
+#include linux/errno.h
+#include linux/slab.h
+#include linux/init.h
+#include linux/gcd.h
+#include linux/completion.h
+#include linux/idr.h
+#include linux/pm_runtime.h
+#include linux/slimbus.h
+
+static DEFINE_MUTEX(slim_lock);
+static DEFINE_IDR(ctrl_idr);
+static struct device_type slim_dev_type;
+static struct device_type slim_ctrl_type;
+
+static bool slim_eaddr_equal(struct slim_eaddr *a, struct slim_eaddr *b)
+{
+   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)
+{
+   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)
+   return id;
+   id++;
+   }
+   return NULL;
+}
+
+static int slim_device_match(struct device *dev, struct device_driver 

Re: [PATCH V2 1/6] SLIMbus: Device management on SLIMbus

2015-06-16 Thread Joe Perches
On Tue, 2015-06-16 at 19:45 -0600, Sagar Dharia wrote:
 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.
[]
 diff --git a/drivers/slimbus/slimbus.c b/drivers/slimbus/slimbus.c
[]
 +static bool slim_eaddr_equal(struct slim_eaddr *a, struct slim_eaddr *b)
 +{
 + 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)
 +{
 + 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)
 + return id;
 + id++;
 + }
 + return NULL;
 +}
 +
 +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)
 + return 0;
 +
 + slim_dev = to_slim_device(dev);
 + if (drv-id_table)
 + return slim_match(drv-id_table, slim_dev) != NULL;
 + return 0;
 +}

This should probably be a bool function return.

Maybe this:

static bool slim_device_match(struct device *dev, struct device_driver *driver)
{
struct slim_driver *drv = to_slim_driver(driver);

if (dev-type != slim_dev_type || !drv-id_table)
return false;

return slim_match(drv-id_table, to_slim_device(dev));
}


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