Re: [PATCHv2 2/5] mailbox/omap: add support for parsing dt devices

2014-07-16 Thread Markus Mayer
If I may nit-pick here for a minute...

On 11 July 2014 15:04, Suman Anna s-a...@ti.com wrote:
 Logic has been added to the OMAP2+ mailbox code to parse the
 mailbox dt nodes and construct the different sub-mailboxes
 associated with the instance. The DT representation of the
 sub-mailbox devices is different from legacy platform data
 representation to allow flexibility of interrupt configuration
 between Tx and Rx fifos (to also possibly allow simplex devices
 in the future). The DT representation gathers similar information
 that was being passed previously through the platform data, except
 for the number of fifos, interrupts and interrupt type information,
 which are gathered through driver compatible match data.

 The non-DT support has to be maintained for now to not break
 OMAP3 legacy boot, and the legacy-style code will be cleaned
 up once OMAP3 is also converted to DT-boot only.

 Cc: Jassi Brar jassisinghb...@gmail.com
 Cc: Rob Herring robh...@kernel.org
 Signed-off-by: Suman Anna s-a...@ti.com
 ---
  drivers/mailbox/omap-mailbox.c | 156 
 ++---
  1 file changed, 132 insertions(+), 24 deletions(-)

 diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c

[...]

  static int omap_mbox_probe(struct platform_device *pdev)
  {
 struct resource *mem;
 int ret;
 struct omap_mbox **list, *mbox, *mboxblk;
 struct omap_mbox_pdata *pdata = pdev-dev.platform_data;
 -   struct omap_mbox_dev_info *info;
 +   struct omap_mbox_dev_info *info = NULL;
 +   struct omap_mbox_fifo_info *finfo, *finfoblk;
 struct omap_mbox_device *mdev;
 struct omap_mbox_fifo *fifo;
 -   u32 intr_type;
 +   struct device_node *node = pdev-dev.of_node;
 +   struct device_node *child;
 +   const struct of_device_id *match;
 +   u32 intr_type, info_count;
 +   u32 num_users, num_fifos;
 +   u32 tmp[3];
 u32 l;
 int i;

 -   if (!pdata || !pdata-info_cnt || !pdata-info) {
 +   if (!node  (!pdata || !pdata-info_cnt || !pdata-info)) {
 pr_err(%s: platform not supported\n, __func__);
 return -ENODEV;
 }

 +   if (node) {

I noticed here you are using

if (node)
/* DT stuff goes here */
else
/* non-DT stuff goes here */

but below the logic is reversed.

 +   match = of_match_device(omap_mailbox_of_match, pdev-dev);
 +   if (!match)
 +   return -ENODEV;
 +   intr_type = (u32)match-data;
 +
 +   if (of_property_read_u32(node, ti,mbox-num-users,
 +num_users))
 +   return -ENODEV;
 +
 +   if (of_property_read_u32(node, ti,mbox-num-fifos,
 +num_fifos))
 +   return -ENODEV;
 +
 +   info_count = of_get_available_child_count(node);
 +   if (!info_count) {
 +   dev_err(pdev-dev, no available mbox devices 
 found\n);
 +   return -ENODEV;
 +   }
 +   } else { /* non-DT device creation */
 +   info_count = pdata-info_cnt;
 +   info = pdata-info;
 +   intr_type = pdata-intr_type;
 +   num_users = pdata-num_users;
 +   num_fifos = pdata-num_fifos;
 +   }
 +
 +   finfoblk = devm_kzalloc(pdev-dev, info_count * sizeof(*finfoblk),
 +   GFP_KERNEL);
 +   if (!finfoblk)
 +   return -ENOMEM;
 +
 +   finfo = finfoblk;
 +   child = NULL;
 +   for (i = 0; i  info_count; i++, finfo++) {
 +   if (!node) {

Here it's
if (!node)
/* non-DT stuff */
else
/* DT stuff */

I think the if (node) ... version is a bit cleaner. Besides it's
nice if code is consistent. Do you mind changing the if statement here
so it matches the logic used above?

 +   finfo-tx_id = info-tx_id;
 +   finfo-rx_id = info-rx_id;
 +   finfo-tx_usr = info-usr_id;
 +   finfo-tx_irq = info-irq_id;
 +   finfo-rx_usr = info-usr_id;
 +   finfo-rx_irq = info-irq_id;
 +   finfo-name = info-name;
 +   info++;
 +   } else {
 +   child = of_get_next_available_child(node, child);
 +   ret = of_property_read_u32_array(child, ti,mbox-tx,
 +tmp, 
 ARRAY_SIZE(tmp));
 +   if (ret)
 +   return ret;
 +   finfo-tx_id = tmp[0];
 +   finfo-tx_irq = tmp[1];
 +   finfo-tx_usr = tmp[2];
 +
 +   ret = of_property_read_u32_array(child, ti,mbox-rx,
 + 

Re: [PATCHv2 2/5] mailbox/omap: add support for parsing dt devices

2014-07-16 Thread Suman Anna
Hi Markus,

On 07/16/2014 03:50 PM, Markus Mayer wrote:
 If I may nit-pick here for a minute...
 
 On 11 July 2014 15:04, Suman Anna s-a...@ti.com wrote:
 Logic has been added to the OMAP2+ mailbox code to parse the
 mailbox dt nodes and construct the different sub-mailboxes
 associated with the instance. The DT representation of the
 sub-mailbox devices is different from legacy platform data
 representation to allow flexibility of interrupt configuration
 between Tx and Rx fifos (to also possibly allow simplex devices
 in the future). The DT representation gathers similar information
 that was being passed previously through the platform data, except
 for the number of fifos, interrupts and interrupt type information,
 which are gathered through driver compatible match data.

 The non-DT support has to be maintained for now to not break
 OMAP3 legacy boot, and the legacy-style code will be cleaned
 up once OMAP3 is also converted to DT-boot only.

 Cc: Jassi Brar jassisinghb...@gmail.com
 Cc: Rob Herring robh...@kernel.org
 Signed-off-by: Suman Anna s-a...@ti.com
 ---
  drivers/mailbox/omap-mailbox.c | 156 
 ++---
  1 file changed, 132 insertions(+), 24 deletions(-)

 diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
 
 [...]
 
  static int omap_mbox_probe(struct platform_device *pdev)
  {
 struct resource *mem;
 int ret;
 struct omap_mbox **list, *mbox, *mboxblk;
 struct omap_mbox_pdata *pdata = pdev-dev.platform_data;
 -   struct omap_mbox_dev_info *info;
 +   struct omap_mbox_dev_info *info = NULL;
 +   struct omap_mbox_fifo_info *finfo, *finfoblk;
 struct omap_mbox_device *mdev;
 struct omap_mbox_fifo *fifo;
 -   u32 intr_type;
 +   struct device_node *node = pdev-dev.of_node;
 +   struct device_node *child;
 +   const struct of_device_id *match;
 +   u32 intr_type, info_count;
 +   u32 num_users, num_fifos;
 +   u32 tmp[3];
 u32 l;
 int i;

 -   if (!pdata || !pdata-info_cnt || !pdata-info) {
 +   if (!node  (!pdata || !pdata-info_cnt || !pdata-info)) {
 pr_err(%s: platform not supported\n, __func__);
 return -ENODEV;
 }

 +   if (node) {
 
 I noticed here you are using
 
 if (node)
 /* DT stuff goes here */
 else
 /* non-DT stuff goes here */
 
 but below the logic is reversed.
 
 +   match = of_match_device(omap_mailbox_of_match, pdev-dev);
 +   if (!match)
 +   return -ENODEV;
 +   intr_type = (u32)match-data;
 +
 +   if (of_property_read_u32(node, ti,mbox-num-users,
 +num_users))
 +   return -ENODEV;
 +
 +   if (of_property_read_u32(node, ti,mbox-num-fifos,
 +num_fifos))
 +   return -ENODEV;
 +
 +   info_count = of_get_available_child_count(node);
 +   if (!info_count) {
 +   dev_err(pdev-dev, no available mbox devices 
 found\n);
 +   return -ENODEV;
 +   }
 +   } else { /* non-DT device creation */
 +   info_count = pdata-info_cnt;
 +   info = pdata-info;
 +   intr_type = pdata-intr_type;
 +   num_users = pdata-num_users;
 +   num_fifos = pdata-num_fifos;
 +   }
 +
 +   finfoblk = devm_kzalloc(pdev-dev, info_count * sizeof(*finfoblk),
 +   GFP_KERNEL);
 +   if (!finfoblk)
 +   return -ENOMEM;
 +
 +   finfo = finfoblk;
 +   child = NULL;
 +   for (i = 0; i  info_count; i++, finfo++) {
 +   if (!node) {
 
 Here it's
 if (!node)
 /* non-DT stuff */
 else
 /* DT stuff */
 
 I think the if (node) ... version is a bit cleaner. Besides it's
 nice if code is consistent. Do you mind changing the if statement here
 so it matches the logic used above?

No, not at all, I will fix this up in the next version. I have to revise
the framework adaptation patches anyway (remove tidspbridge changes as
that driver is getting deleted and add a missing of_node_put).

Do you prefer that I split up this series between DT conversion and
framework adaptation or good with posting all the patches together? If
latter, I will refresh it once the v9 version of the framework comes out.

regards
Suman

 
 +   finfo-tx_id = info-tx_id;
 +   finfo-rx_id = info-rx_id;
 +   finfo-tx_usr = info-usr_id;
 +   finfo-tx_irq = info-irq_id;
 +   finfo-rx_usr = info-usr_id;
 +   finfo-rx_irq = info-irq_id;
 +   finfo-name = info-name;
 +   info++;
 +   } else {
 +   child = 

Re: [PATCHv2 2/5] mailbox/omap: add support for parsing dt devices

2014-07-12 Thread Pavel Machek
On Fri 2014-07-11 17:04:09, Suman Anna wrote:
 Logic has been added to the OMAP2+ mailbox code to parse the
 mailbox dt nodes and construct the different sub-mailboxes
 associated with the instance. The DT representation of the
 sub-mailbox devices is different from legacy platform data
 representation to allow flexibility of interrupt configuration
 between Tx and Rx fifos (to also possibly allow simplex devices
 in the future). The DT representation gathers similar information
 that was being passed previously through the platform data, except
 for the number of fifos, interrupts and interrupt type information,
 which are gathered through driver compatible match data.
 
 The non-DT support has to be maintained for now to not break
 OMAP3 legacy boot, and the legacy-style code will be cleaned
 up once OMAP3 is also converted to DT-boot only.
 
 Cc: Jassi Brar jassisinghb...@gmail.com
 Cc: Rob Herring robh...@kernel.org
 Signed-off-by: Suman Anna s-a...@ti.com

Acked-by: Pavel Machek pa...@ucw.cz
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCHv2 2/5] mailbox/omap: add support for parsing dt devices

2014-07-11 Thread Suman Anna
Logic has been added to the OMAP2+ mailbox code to parse the
mailbox dt nodes and construct the different sub-mailboxes
associated with the instance. The DT representation of the
sub-mailbox devices is different from legacy platform data
representation to allow flexibility of interrupt configuration
between Tx and Rx fifos (to also possibly allow simplex devices
in the future). The DT representation gathers similar information
that was being passed previously through the platform data, except
for the number of fifos, interrupts and interrupt type information,
which are gathered through driver compatible match data.

The non-DT support has to be maintained for now to not break
OMAP3 legacy boot, and the legacy-style code will be cleaned
up once OMAP3 is also converted to DT-boot only.

Cc: Jassi Brar jassisinghb...@gmail.com
Cc: Rob Herring robh...@kernel.org
Signed-off-by: Suman Anna s-a...@ti.com
---
 drivers/mailbox/omap-mailbox.c | 156 ++---
 1 file changed, 132 insertions(+), 24 deletions(-)

diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
index a27e00e..73eb0fb 100644
--- a/drivers/mailbox/omap-mailbox.c
+++ b/drivers/mailbox/omap-mailbox.c
@@ -31,6 +31,7 @@
 #include linux/err.h
 #include linux/notifier.h
 #include linux/module.h
+#include linux/of_device.h
 #include linux/platform_device.h
 #include linux/pm_runtime.h
 #include linux/platform_data/mailbox-omap.h
@@ -94,6 +95,18 @@ struct omap_mbox_device {
struct list_head elem;
 };
 
+struct omap_mbox_fifo_info {
+   int tx_id;
+   int tx_usr;
+   int tx_irq;
+
+   int rx_id;
+   int rx_usr;
+   int rx_irq;
+
+   const char *name;
+};
+
 struct omap_mbox {
const char  *name;
int irq;
@@ -587,24 +600,118 @@ static int omap_mbox_unregister(struct omap_mbox_device 
*mdev)
return 0;
 }
 
+static const struct of_device_id omap_mailbox_of_match[] = {
+   {
+   .compatible = ti,omap2-mailbox,
+   .data   = (void *)MBOX_INTR_CFG_TYPE1,
+   },
+   {
+   .compatible = ti,omap3-mailbox,
+   .data   = (void *)MBOX_INTR_CFG_TYPE1,
+   },
+   {
+   .compatible = ti,omap4-mailbox,
+   .data   = (void *)MBOX_INTR_CFG_TYPE2,
+   },
+   {
+   /* end */
+   },
+};
+MODULE_DEVICE_TABLE(of, omap_mailbox_of_match);
+
 static int omap_mbox_probe(struct platform_device *pdev)
 {
struct resource *mem;
int ret;
struct omap_mbox **list, *mbox, *mboxblk;
struct omap_mbox_pdata *pdata = pdev-dev.platform_data;
-   struct omap_mbox_dev_info *info;
+   struct omap_mbox_dev_info *info = NULL;
+   struct omap_mbox_fifo_info *finfo, *finfoblk;
struct omap_mbox_device *mdev;
struct omap_mbox_fifo *fifo;
-   u32 intr_type;
+   struct device_node *node = pdev-dev.of_node;
+   struct device_node *child;
+   const struct of_device_id *match;
+   u32 intr_type, info_count;
+   u32 num_users, num_fifos;
+   u32 tmp[3];
u32 l;
int i;
 
-   if (!pdata || !pdata-info_cnt || !pdata-info) {
+   if (!node  (!pdata || !pdata-info_cnt || !pdata-info)) {
pr_err(%s: platform not supported\n, __func__);
return -ENODEV;
}
 
+   if (node) {
+   match = of_match_device(omap_mailbox_of_match, pdev-dev);
+   if (!match)
+   return -ENODEV;
+   intr_type = (u32)match-data;
+
+   if (of_property_read_u32(node, ti,mbox-num-users,
+num_users))
+   return -ENODEV;
+
+   if (of_property_read_u32(node, ti,mbox-num-fifos,
+num_fifos))
+   return -ENODEV;
+
+   info_count = of_get_available_child_count(node);
+   if (!info_count) {
+   dev_err(pdev-dev, no available mbox devices 
found\n);
+   return -ENODEV;
+   }
+   } else { /* non-DT device creation */
+   info_count = pdata-info_cnt;
+   info = pdata-info;
+   intr_type = pdata-intr_type;
+   num_users = pdata-num_users;
+   num_fifos = pdata-num_fifos;
+   }
+
+   finfoblk = devm_kzalloc(pdev-dev, info_count * sizeof(*finfoblk),
+   GFP_KERNEL);
+   if (!finfoblk)
+   return -ENOMEM;
+
+   finfo = finfoblk;
+   child = NULL;
+   for (i = 0; i  info_count; i++, finfo++) {
+   if (!node) {
+   finfo-tx_id = info-tx_id;
+   finfo-rx_id = info-rx_id;
+   finfo-tx_usr = info-usr_id;
+   finfo-tx_irq = info-irq_id;
+

[PATCHv2 2/5] mailbox/omap: add support for parsing dt devices

2013-07-22 Thread Suman Anna
Logic has been added to the OMAP2+ mailbox code to
parse the mailbox dt nodes and construct the different
mailboxes associated with the instance. The design is
based on gathering the same information that was being
passed previously through the platform data, except for
the interrupt type configuration information.

Signed-off-by: Suman Anna s-a...@ti.com
---
 .../devicetree/bindings/mailbox/omap-mailbox.txt   |  43 +++
 drivers/mailbox/mailbox-omap2.c| 130 ++---
 2 files changed, 158 insertions(+), 15 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mailbox/omap-mailbox.txt

diff --git a/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt 
b/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt
new file mode 100644
index 000..8ffb08a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt
@@ -0,0 +1,43 @@
+OMAP2+ Mailbox Driver
+
+Required properties:
+- compatible:  Should be one of the following,
+   ti,omap2-mailbox for
+   OMAP2420, OMAP2430, OMAP3430, OMAP3630 SoCs
+   ti,omap4-mailbox for
+   OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs
+- reg: Contains the mailbox register address range (base 
address
+   and length)
+- interrupts:  Contains the interrupt information for the mailbox
+   device. The format is dependent on which interrupt
+   controller the OMAP device uses
+- ti,hwmods:   Name of the hwmod associated with the mailbox
+- ti,mbox-num-users:   Number of targets (processor devices) that the mailbox 
device
+   can interrupt
+- ti,mbox-num-fifos:   Number of h/w fifos within the mailbox device
+- ti,mbox-names:   Array of the names of the mailboxes
+- ti,mbox-data:Mailbox descriptor data private to each 
mailbox. The 4
+   cells represent the following data,
+ Cell #1 (tx_id) - mailbox fifo id used for
+   transmitting messages
+ Cell #2 (rx_id) - mailbox fifo id on which messages
+   are received
+ Cell #3 (irq_id) - irq identifier index number to use
+   from the interrupts data
+ Cell #4 (usr_id) - mailbox user id for identifying the
+   interrupt into the MPU interrupt
+   controller.
+
+Example:
+
+/* OMAP4 */
+mailbox: mailbox@4a0f4000 {
+   compatible = ti,omap4-mailbox;
+   reg = 0x4a0f4000 0x200;
+   interrupts = GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH;
+   ti,hwmods = mailbox;
+   ti,mbox-num-users = 3;
+   ti,mbox-num-fifos = 8;
+   ti,mbox-names = mbox-ipu, mbox-dsp;
+   ti,mbox-data = 0 1 0 0, 3 2 0 0;
+};
diff --git a/drivers/mailbox/mailbox-omap2.c b/drivers/mailbox/mailbox-omap2.c
index 6c0687c..759f72c 100644
--- a/drivers/mailbox/mailbox-omap2.c
+++ b/drivers/mailbox/mailbox-omap2.c
@@ -14,6 +14,7 @@
 #include linux/slab.h
 #include linux/clk.h
 #include linux/err.h
+#include linux/of_device.h
 #include linux/platform_device.h
 #include linux/io.h
 #include linux/pm_runtime.h
@@ -222,6 +223,21 @@ static struct omap_mbox_ops omap2_mbox_ops = {
.restore_ctx= omap2_mbox_restore_ctx,
 };
 
+static const struct of_device_id omap_mailbox_of_match[] = {
+   {
+   .compatible = ti,omap2-mailbox,
+   .data   = (void *) MBOX_INTR_CFG_TYPE1,
+   },
+   {
+   .compatible = ti,omap4-mailbox,
+   .data   = (void *) MBOX_INTR_CFG_TYPE2,
+   },
+   {
+   /* end */
+   },
+};
+MODULE_DEVICE_TABLE(of, omap_mailbox_of_match);
+
 static int omap2_mbox_probe(struct platform_device *pdev)
 {
struct resource *mem;
@@ -229,47 +245,127 @@ static int omap2_mbox_probe(struct platform_device *pdev)
struct omap_mbox **list, *mbox, *mboxblk;
struct omap_mbox2_priv *priv, *privblk;
struct omap_mbox_pdata *pdata = pdev-dev.platform_data;
-   struct omap_mbox_dev_info *info;
struct omap_mbox_device *mdev;
-   int i;
-
-   if (!pdata || !pdata-info_cnt || !pdata-info) {
+   struct omap_mbox_dev_info *info, *of_info = NULL;
+   struct device_node *node = pdev-dev.of_node;
+   int i, j;
+   u32 info_count = 0, intr_type = 0;
+   u32 num_users = 0, num_fifos = 0;
+   u32 dlen, dsize = 4;
+   u32 *tmp;
+   const __be32 *mbox_data;
+
+   if (!node  (!pdata || !pdata-info_cnt || !pdata-info)) {
pr_err(%s: platform not supported\n, __func__);
return -ENODEV;
}
 
+   if