Support for Fujitsu MB86S7X SoCs

From: Jassi Brar <[email protected]>

Add driver for the ARM Primecell Message-Handling-Unit(MHU) controller.

Signed-off-by Jassi Brar <[email protected]>

Signed-off-by Andy Green <[email protected]>

Signed-off-by Vincent Yang <[email protected]>

Signed-off-by Tetsuya Nuriya <[email protected]>

---
  .../devicetree/bindings/mailbox/arm-mhu.txt        |  35 ++++
  drivers/mailbox/Kconfig                            |   7 +
  drivers/mailbox/Makefile                           |   2 +
  drivers/mailbox/arm_mhu.c                          | 196 +++++++++++++++++++++
  4 files changed, 240 insertions(+)

create mode 100644 Documentation/devicetree/bindings/mailbox/arm-mhu.txt create mode 100644 drivers/mailbox/arm_mhu.c

Documentation/devicetree/bindings/mailbox/arm-mhu.txt new file +0 -0

new file mode 100644
index 0000000..986a205

Documentation/devicetree/bindings/mailbox/arm-mhu.txt new file +13 -0

+Example:
+--------
+
+	mhu: mailbox at 2b1f0000 {
+		#mbox-cells = <1>;
+		compatible = "arm,mhu", "arm,primecell";
+		reg = <0 0x2b1f0000 0x1000>;
+		interrupts = <0 36 4>,
+			     <0 35 4>,
+			     <0 37 4>;
+		clocks = <&clock 0 2 1>;
+		clock-names = "apb_pclk";
+	};

drivers/mailbox/Kconfig +0 -0

index c04fed9..9238440 100644

drivers/mailbox/Kconfig +0 -0

  config PL320_MBOX
  	bool "ARM PL320 Mailbox"
  	depends on ARM_AMBA

drivers/mailbox/Makefile +2 -0

index dd412c2..c83791d 100644
@@ -2,6 +2,8 @@
 
  obj-$(CONFIG_MAILBOX)		+= mailbox.o
 
+obj-$(CONFIG_ARM_MHU)	+= arm_mhu.o
+
  obj-$(CONFIG_PL320_MBOX)	+= pl320-ipc.o
 
  obj-$(CONFIG_OMAP2PLUS_MBOX)	+= omap-mailbox.o

drivers/mailbox/arm_mhu.c new file +29 -0

new file mode 100644
index 0000000..d6fd1cd
@@ -0,0 +1,196 @@
+/*
+ * Copyright (C) 2013-2015 Fujitsu Semiconductor Ltd.
+ * Copyright (C) 2015 Linaro Ltd.
+ * Author: Jassi Brar <[email protected]>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ *
+ * 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/interrupt.h>
+#include <linux/spinlock.h>
+#include <linux/mutex.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/amba/bus.h>
+#include <linux/mailbox_controller.h>
+
+#define INTR_STAT_OFS	0x0
+#define INTR_SET_OFS	0x8
+#define INTR_CLR_OFS	0x10

drivers/mailbox/arm_mhu.c new file +17 -0

+	struct mbox_chan chan[3];
+	struct mbox_controller mbox;
+};
+
+static irqreturn_t mhu_rx_interrupt(int irq, void *p)
+{
+	struct mbox_chan *chan = p;
+	struct mhu_link *mlink = chan->con_priv;
+	u32 val;
+
+	val = readl_relaxed(mlink->rx_reg + INTR_STAT_OFS);
+	if (!val)
+		return IRQ_NONE;
+
+	mbox_chan_received_data(chan, (void *)&val);
+
+	writel_relaxed(val, mlink->rx_reg + INTR_CLR_OFS);

drivers/mailbox/arm_mhu.c new file +10 -0

+	writel_relaxed(*arg, mlink->tx_reg + INTR_SET_OFS);
+
+	return 0;
+}
+
+static int mhu_startup(struct mbox_chan *chan)
+{
+	struct mhu_link *mlink = chan->con_priv;
+	u32 val;
+	int ret;

drivers/mailbox/arm_mhu.c new file +25 -0

+
+	return 0;
+}
+
+static void mhu_shutdown(struct mbox_chan *chan)
+{
+	struct mhu_link *mlink = chan->con_priv;
+
+	free_irq(mlink->irq, chan);
+}
+
+static struct mbox_chan_ops mhu_ops = {
+	.send_data = mhu_send_data,
+	.startup = mhu_startup,
+	.shutdown = mhu_shutdown,
+	.last_tx_done = mhu_last_tx_done,
+};
+
+static int mhu_probe(struct amba_device *adev, const struct amba_id *id)
+{
+	int i, err;
+	struct arm_mhu *mhu;
+	struct device *dev = &adev->dev;
+	int mhu_reg[3] = {0x0, 0x20, 0x200};
+

drivers/mailbox/arm_mhu.c new file +15 -0

+
+	amba_set_drvdata(adev, mhu);
+
+	err = mbox_controller_register(&mhu->mbox);
+	if (err) {
+		dev_err(dev, "Failed to register mailboxes %d\n", err);
+		return err;
+	}
+
+	dev_info(dev, "ARM MHU Mailbox registered\n");
+	return 0;
+}
+
+static int mhu_remove(struct amba_device *adev)
+{

Documentation/devicetree/bindings/mailbox/arm-mhu.txt line 7

Quoted 9 lines

+++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt

Documentation/devicetree/bindings/mailbox/arm-mhu.txt new file +7 -0

@@ -0,0 +1,35 @@
+ARM MHU Mailbox Driver
+======================
+
+The ARM's Message-Handling-Unit (MHU) is a mailbox controller that has
+3 independent channels/links to communicate with remote processor(s).
+ MHU links are hardwired on a platform. A link raises interrupt for any
+received data. However, there is no specified way of knowing if the sent

IIUC the IP as such doesn't have this restriction, it's just the way currently integrated in the SoCs. So we need to provide a way for future expansion.

Documentation/devicetree/bindings/mailbox/arm-mhu.txt line 9

Quoted 2 lines

Documentation/devicetree/bindings/mailbox/arm-mhu.txt new file +2 -0

+data has been read by the remote. This driver assumes the sender polls
+STAT register and the remote clears it after having read the data.

I would rather drop any specifics about what driver does. Bindings should try to confine to the underlying hardware only if possible.

Documentation/devicetree/bindings/mailbox/arm-mhu.txt line 19

Quoted 9 lines

Documentation/devicetree/bindings/mailbox/arm-mhu.txt new file +9 -0

+Mailbox Device Node:
+====================
+
+Required properties:
+--------------------
+- compatible:          Shall be "arm,mhu" & "arm,primecell"
+- reg:                 Contains the mailbox register address range (base
+                       address and length)
+- #mbox-cells          Shall be 1

Need to explain what that one cell must represent.

Quoted 3 lines

(quoted diff) +1 -0

+- interrupts:          Contains the interrupt information corresponding

to
+ each of the 3 links of MHU.

How do we handle if the middle link has no interrupt ?

Also please that the last channel is secure only and must not be used in non-secure execution.

drivers/mailbox/Kconfig menuconfig MAILBOX · line 15

Quoted 6 lines

drivers/mailbox/Kconfig +6 -0

+config ARM_MHU
+       tristate "ARM MHU Mailbox"
+       depends on ARM
+       help
+         Say Y here if you want to build the ARM MHU controller driver
+

May be a brief one/2-liners on what IP does might be useful ?

drivers/mailbox/arm_mhu.c line 39

Quoted 4 lines

drivers/mailbox/arm_mhu.c new file +4 -0

+
+struct arm_mhu {
+       void __iomem *base;
+       struct mhu_link mlink[3];

Replace "3" with some macro throughout the file.

drivers/mailbox/arm_mhu.c line 66

Quoted 7 lines

drivers/mailbox/arm_mhu.c new file +7 -0

+
+static bool mhu_last_tx_done(struct mbox_chan *chan)
+{
+       struct mhu_link *mlink = chan->con_priv;
+       u32 val = readl_relaxed(mlink->tx_reg + INTR_STAT_OFS);
+
+       return (val == 0);

[Nit] How about just

        return readl_relaxed(mlink->tx_reg + INTR_STAT_OFS) == 0;

Quoted 4 lines

drivers/mailbox/arm_mhu.c new file +4 -0

+static int mhu_send_data(struct mbox_chan *chan, void *data)
+{
+       struct mhu_link *mlink = chan->con_priv;
+       u32 *arg = data;

Review

Quoted 6 lines

drivers/mailbox/arm_mhu.c new file +3 -0

+
+       if (!mhu_last_tx_done(chan)) {
+               dev_err(chan->mbox->dev, "Last TX(%d) pending!\n",

mlink->irq);

(quoted diff) +2 -0

+               return -EBUSY;
+       }

Why do you need the above check when the core handles that already ?

Quoted 12 lines

drivers/mailbox/arm_mhu.c new file +10 -0

+static int mhu_startup(struct mbox_chan *chan)
+{
+       struct mhu_link *mlink = chan->con_priv;
+       u32 val;
+       int ret;
+
+       val = readl_relaxed(mlink->tx_reg + INTR_STAT_OFS);
+       writel_relaxed(val, mlink->tx_reg + INTR_CLR_OFS);
+
+       ret = request_irq(mlink->irq, mhu_rx_interrupt, 0, "mhu_link",

chan);
+ if (ret) {

drivers/mailbox/arm_mhu.c line 98

Quoted 4 lines

drivers/mailbox/arm_mhu.c new file +4 -0

+               dev_err(chan->mbox->dev,
+                       "Unable to aquire IRQ %d\n", mlink->irq);
+               return ret;
+       }

This proved costly(in terms of time) on Juno board in my testing, requesting and setting up irq for every small packets you need to send. I would prefer it to be moved to probe.

Quoted 12 lines

drivers/mailbox/arm_mhu.c new file +12 -0

+static int mhu_probe(struct amba_device *adev, const struct amba_id *id)
+{
+       int i, err;
+       struct arm_mhu *mhu;
+       struct device *dev = &adev->dev;
+       int mhu_reg[3] = {0x0, 0x20, 0x200};
+
+       err = amba_request_regions(adev, NULL);
+       if (err)
+               return err;
+
+       /* Allocate memory for device */

drivers/mailbox/arm_mhu.c line 133

Quoted 5 lines

drivers/mailbox/arm_mhu.c new file +5 -0

+       mhu = devm_kzalloc(dev, sizeof(*mhu), GFP_KERNEL);
+       if (!mhu)
+               return -ENOMEM;
+
+       mhu->base = devm_ioremap_resource(dev, &adev->res);

This will either explode or warn as you have already requested the regions. Have you run this code after converting to amba bus device ?

drivers/mailbox/arm_mhu.c line 143

Quoted 5 lines

drivers/mailbox/arm_mhu.c new file +5 -0

+       for (i = 0; i < 3; i++) {
+               mhu->chan[i].con_priv = &mhu->mlink[i];
+               mhu->mlink[i].irq = adev->irq[i];
+               mhu->mlink[i].rx_reg = mhu->base + mhu_reg[i];
+               mhu->mlink[i].tx_reg = mhu->mlink[i].rx_reg + 0x100;

Again a macro for 0x100 ?

drivers/mailbox/arm_mhu.c line 152

Quoted 8 lines

drivers/mailbox/arm_mhu.c new file +8 -0

+
+       mhu->mbox.dev = dev;
+       mhu->mbox.chans = &mhu->chan[0];
+       mhu->mbox.num_chans = 3;
+       mhu->mbox.ops = &mhu_ops;
+       mhu->mbox.txdone_irq = false;
+       mhu->mbox.txdone_poll = true;
+       mhu->mbox.txpoll_period = 10;

10ms seems to high, but if that's a derived value then I am fine. On Juno, typically we get response within a millisecond, so we need not get blocked on Tx even after getting Rx for 10ms. I prefer it to be set to 1 ms.

drivers/mailbox/arm_mhu.c line 177

Quoted 4 lines

drivers/mailbox/arm_mhu.c new file +4 -0

+
+static struct amba_id mhu_ids[] = {
+       {
+               .id     = 0x1bb098,

This is the problem. This IP has PID(0x98 0xB0 0x1B 0x00 0x04) it's 5 bytes[1] . Even I had thought of AMBA initially, it doesn't fit as is, may need changes to the amba core to consider this.

Quoted 15 lines · Sudeep Holla

Documentation/devicetree/bindings/mailbox/arm-mhu.txt line 7

Quoted 9 lines

+++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt

Documentation/devicetree/bindings/mailbox/arm-mhu.txt new file +7 -0

@@ -0,0 +1,35 @@
+ARM MHU Mailbox Driver
+======================
+
+The ARM's Message-Handling-Unit (MHU) is a mailbox controller that has
+3 independent channels/links to communicate with remote processor(s).
+ MHU links are hardwired on a platform. A link raises interrupt for any
+received data. However, there is no specified way of knowing if the sent

IIUC the IP as such doesn't have this restriction, it's just the way currently integrated in the SoCs. So we need to provide a way for future expansion.

Quoted 6 lines

Documentation/devicetree/bindings/mailbox/arm-mhu.txt line 9

Quoted 2 lines

Documentation/devicetree/bindings/mailbox/arm-mhu.txt new file +2 -0

+data has been read by the remote. This driver assumes the sender polls
+STAT register and the remote clears it after having read the data.

I would rather drop any specifics about what driver does. Bindings should try to confine to the underlying hardware only if possible.

The behavior of local controller and remote firmware clubbed together is what mailbox drivers have to deal with.

For example, if the remote f/w doesn't clear the STAT register after
reading it, this driver won't have a way to know if the last tx has been done or not, i.e, mhu_last_tx_done() won't work. And that behavior is not decided by Linux/driver, so is in a way a h/w feature.

Does your remote f/w not clear the STAT?

Quoted 12 lines

Documentation/devicetree/bindings/mailbox/arm-mhu.txt line 19

Quoted 9 lines

Documentation/devicetree/bindings/mailbox/arm-mhu.txt new file +9 -0

+Mailbox Device Node:
+====================
+
+Required properties:
+--------------------
+- compatible:          Shall be "arm,mhu" & "arm,primecell"
+- reg:                 Contains the mailbox register address range (base
+                       address and length)
+- #mbox-cells          Shall be 1

Need to explain what that one cell must represent.

Quoted 6 lines

Quoted 3 lines

(quoted diff) +1 -0

+- interrupts:          Contains the interrupt information corresponding

to
+ each of the 3 links of MHU.

How do we handle if the middle link has no interrupt ?

The MHU chapter in my SoC manual suggests the specification requires an RX-IRQ per channel. Does your platform omit the irq?

Quoted 3 lines

Also please that the last channel is secure only and must not be used in non-secure execution.

I am aware and it is perfectly possible to not touch the secure channel from NS mode.

Quoted 10 lines

drivers/mailbox/arm_mhu.c line 66

Quoted 7 lines

drivers/mailbox/arm_mhu.c new file +7 -0

+
+static bool mhu_last_tx_done(struct mbox_chan *chan)
+{
+       struct mhu_link *mlink = chan->con_priv;
+       u32 val = readl_relaxed(mlink->tx_reg + INTR_STAT_OFS);
+
+       return (val == 0);

[Nit] How about just

Quoted 2 lines

         return readl_relaxed(mlink->tx_reg + INTR_STAT_OFS) == 0;

I think (val == 0) is less cluttered.

Quoted 4 lines

Quoted 4 lines

drivers/mailbox/arm_mhu.c new file +4 -0

+static int mhu_send_data(struct mbox_chan *chan, void *data)
+{
+       struct mhu_link *mlink = chan->con_priv;
+       u32 *arg = data;

Quoted 9 lines

Review

Quoted 6 lines

drivers/mailbox/arm_mhu.c new file +3 -0

+
+       if (!mhu_last_tx_done(chan)) {
+               dev_err(chan->mbox->dev, "Last TX(%d) pending!\n",

mlink->irq);

(quoted diff) +2 -0

+               return -EBUSY;
+       }

Why do you need the above check when the core handles that already ?

Our bootloader also uses mhu so we could have failed/pending bits in STAT when Linux first tries to use it. However since then the driver clears the STAT register upon startup, so this could probably be dropped. Will do.

Quoted 12 lines

Quoted 12 lines

drivers/mailbox/arm_mhu.c new file +10 -0

+static int mhu_startup(struct mbox_chan *chan)
+{
+       struct mhu_link *mlink = chan->con_priv;
+       u32 val;
+       int ret;
+
+       val = readl_relaxed(mlink->tx_reg + INTR_STAT_OFS);
+       writel_relaxed(val, mlink->tx_reg + INTR_CLR_OFS);
+
+       ret = request_irq(mlink->irq, mhu_rx_interrupt, 0, "mhu_link",

chan);
+ if (ret) {

Quoted 9 lines

drivers/mailbox/arm_mhu.c line 98

Quoted 4 lines

drivers/mailbox/arm_mhu.c new file +4 -0

+               dev_err(chan->mbox->dev,
+                       "Unable to aquire IRQ %d\n", mlink->irq);
+               return ret;
+       }

This proved costly(in terms of time) on Juno board in my testing, requesting and setting up irq for every small packets you need to send. I would prefer it to be moved to probe.

If your usage of mailbox is so frequent that request-release of irq proves expensive, maybe you could fix your client to hold onto the channel for the lifetime. I don't see why your scpi_protocol.c shouldn't do that, even if request_irq was in probe.

Quoted 10 lines

drivers/mailbox/arm_mhu.c line 152

Quoted 9 lines

drivers/mailbox/arm_mhu.c new file +8 -0

+
+       mhu->mbox.dev = dev;
+       mhu->mbox.chans = &mhu->chan[0];
+       mhu->mbox.num_chans = 3;
+       mhu->mbox.ops = &mhu_ops;
+       mhu->mbox.txdone_irq = false;
+       mhu->mbox.txdone_poll = true;
+       mhu->mbox.txpoll_period = 10;

Quoted 5 lines

10ms seems to high, but if that's a derived value then I am fine. On Juno, typically we get response within a millisecond, so we need not get blocked on Tx even after getting Rx for 10ms. I prefer it to be set to 1 ms.

Similar on my platform. However 1 isn't much meaning in milliseconds because mod_timer works in jiffiy which is usually atleast 5ms. If we are polling we can't anyway expect latency critical stuff, so 10ms seems like a safe bet.

Quoted 10 lines

drivers/mailbox/arm_mhu.c line 177

Quoted 4 lines

drivers/mailbox/arm_mhu.c new file +4 -0

+
+static struct amba_id mhu_ids[] = {
+       {
+               .id     = 0x1bb098,

This is the problem. This IP has PID(0x98 0xB0 0x1B 0x00 0x04) it's 5 bytes[1] . Even I had thought of AMBA initially, it doesn't fit as is, may need changes to the amba core to consider this.

How is that a problem? AMBA chooses to compare 32bits of PID for the class. The PID4 might change across versions of MHU, which we could figure out in the mhu driver in future.

drivers/clk/clk-mb86s7x.c new file +18 -0

+static struct clk *crg11_get(struct of_phandle_args *clkspec, void *data)
+{
+       struct mb86s70_crg11 *crg11 = data;
+       struct clk_init_data init;
+       u32 cntrlr, domain, port;
+       struct crg_clk *crgclk;
+       struct clk *clk;
+       char clkp[20];
+
+       if (clkspec->args_count != 3)
+               return ERR_PTR(-EINVAL);
+
+       cntrlr = clkspec->args[0];
+       domain = clkspec->args[1];
+       port = clkspec->args[2];
+
+       if (port > 7)
+               snprintf(clkp, 20, "UngatedCLK%d_%X", cntrlr, domain);

drivers/clk/clk-mb86s7x.c new file +41 -0

+
+       crgclk = kzalloc(sizeof(*crgclk), GFP_KERNEL);
+       if (!crgclk) {
+               mutex_unlock(&crg11->lock);
+               return ERR_PTR(-ENOMEM);
+       }
+
+       init.name = clkp;
+       init.num_parents = 0;
+       init.ops = &crg_port_ops;
+       init.flags = CLK_IS_ROOT;
+       crgclk->hw.init = &init;
+       crgclk->cntrlr = cntrlr;
+       crgclk->domain = domain;
+       crgclk->port = port;
+       clk = clk_register(NULL, &crgclk->hw);
+       if (IS_ERR(clk))
+               pr_err("%s:%d Error!\n", __func__, __LINE__);
+       else
+               pr_debug("Registered %s\n", clkp);
+
+       clk_register_clkdev(clk, clkp, NULL);
+       mutex_unlock(&crg11->lock);
+       return clk;
+}
+
+static void __init crg_port_init(struct device_node *node)
+{
+       struct mb86s70_crg11 *crg11;
+
+       crg11 = kzalloc(sizeof(*crg11), GFP_KERNEL);
+       if (!crg11)
+               return;
+
+       mutex_init(&crg11->lock);
+
+       of_clk_add_provider(node, crg11_get, crg11);
+}
+CLK_OF_DECLARE(crg11_gate, "fujitsu,mb86s70-crg11", crg_port_init);
+
+struct cl_clk {

drivers/clk/clk-mb86s7x.c new file +24 -0

+};
+
+static void mhu_cluster_rate(struct clk_hw *hw, unsigned long *rate, int get)
+{
+       struct cl_clk *clc = to_clc_clk(hw);
+       struct mb86s7x_cpu_freq cmd;
+       int code, ret;
+
+       cmd.payload_size = sizeof(cmd);
+       cmd.cluster_class = 0;
+       cmd.cluster_id = clc->cluster;
+       cmd.cpu_id = 0;
+       cmd.freqency = *rate;
+
+       if (get)
+               code = CMD_CPU_CLOCK_RATE_GET_REQ;
+       else
+               code = CMD_CPU_CLOCK_RATE_SET_REQ;
+
+       pr_debug("%s:%d CMD Cl_Class-%u CL_ID-%u CPU_ID-%u Freq-%llu}\n",
+                __func__, __LINE__, cmd.cluster_class,
+                cmd.cluster_id, cmd.cpu_id, cmd.freqency);
+
+       ret = mb86s7x_send_packet(code, &cmd, sizeof(cmd));

Quoting Vincent Yang (2015-02-05 18:10:49)

Quoted 24 lines

drivers/clk/clk-mb86s7x.c new file +24 -0

+static struct clk *crg11_get(struct of_phandle_args *clkspec, void *data)
+{
+       struct mb86s70_crg11 *crg11 = data;
+       struct clk_init_data init;
+       u32 cntrlr, domain, port;
+       struct crg_clk *crgclk;
+       struct clk *clk;
+       char clkp[20];
+
+       if (clkspec->args_count != 3)
+               return ERR_PTR(-EINVAL);
+
+       cntrlr = clkspec->args[0];
+       domain = clkspec->args[1];
+       port = clkspec->args[2];
+
+       if (port > 7)
+               snprintf(clkp, 20, "UngatedCLK%d_%X", cntrlr, domain);
+       else
+               snprintf(clkp, 20, "CLK%d_%X_%d", cntrlr, domain, port);
+
+       mutex_lock(&crg11->lock);
+
+       clk = __clk_lookup(clkp);

drivers/clk/clk-mb86s7x.c line 209

Quoted 4 lines

drivers/clk/clk-mb86s7x.c new file +4 -0

+       if (clk) {
+               mutex_unlock(&crg11->lock);
+               return clk;
+       }

What is the above code doing? It looks like you are checking to see if you are trying to register a clock that is already registered. Why do you need this?

Quoted 45 lines

drivers/clk/clk-mb86s7x.c new file +45 -0

+
+       crgclk = kzalloc(sizeof(*crgclk), GFP_KERNEL);
+       if (!crgclk) {
+               mutex_unlock(&crg11->lock);
+               return ERR_PTR(-ENOMEM);
+       }
+
+       init.name = clkp;
+       init.num_parents = 0;
+       init.ops = &crg_port_ops;
+       init.flags = CLK_IS_ROOT;
+       crgclk->hw.init = &init;
+       crgclk->cntrlr = cntrlr;
+       crgclk->domain = domain;
+       crgclk->port = port;
+       clk = clk_register(NULL, &crgclk->hw);
+       if (IS_ERR(clk))
+               pr_err("%s:%d Error!\n", __func__, __LINE__);
+       else
+               pr_debug("Registered %s\n", clkp);
+
+       clk_register_clkdev(clk, clkp, NULL);
+       mutex_unlock(&crg11->lock);
+       return clk;
+}
+
+static void __init crg_port_init(struct device_node *node)
+{
+       struct mb86s70_crg11 *crg11;
+
+       crg11 = kzalloc(sizeof(*crg11), GFP_KERNEL);
+       if (!crg11)
+               return;
+
+       mutex_init(&crg11->lock);
+
+       of_clk_add_provider(node, crg11_get, crg11);
+}
+CLK_OF_DECLARE(crg11_gate, "fujitsu,mb86s70-crg11", crg_port_init);
+
+struct cl_clk {
+       struct clk_hw hw;
+       int cluster;
+};
+

drivers/clk/clk-mb86s7x.c line 260

Quoted 6 lines

drivers/clk/clk-mb86s7x.c new file +6 -0

+struct mb86s7x_cpu_freq {
+       u32 payload_size;
+       u32 cluster_class;
+       u32 cluster_id;
+       u32 cpu_id;
+       u64 freqency;

s/freqency/frequency/

Quoted 29 lines

drivers/clk/clk-mb86s7x.c new file +29 -0

+};
+
+static void mhu_cluster_rate(struct clk_hw *hw, unsigned long *rate, int get)
+{
+       struct cl_clk *clc = to_clc_clk(hw);
+       struct mb86s7x_cpu_freq cmd;
+       int code, ret;
+
+       cmd.payload_size = sizeof(cmd);
+       cmd.cluster_class = 0;
+       cmd.cluster_id = clc->cluster;
+       cmd.cpu_id = 0;
+       cmd.freqency = *rate;
+
+       if (get)
+               code = CMD_CPU_CLOCK_RATE_GET_REQ;
+       else
+               code = CMD_CPU_CLOCK_RATE_SET_REQ;
+
+       pr_debug("%s:%d CMD Cl_Class-%u CL_ID-%u CPU_ID-%u Freq-%llu}\n",
+                __func__, __LINE__, cmd.cluster_class,
+                cmd.cluster_id, cmd.cpu_id, cmd.freqency);
+
+       ret = mb86s7x_send_packet(code, &cmd, sizeof(cmd));
+       if (ret < 0) {
+               pr_err("%s:%d failed!\n", __func__, __LINE__);
+               return;
+       }
+

drivers/clk/clk-mb86s7x.c line 294

Quoted 5 lines

drivers/clk/clk-mb86s7x.c new file +5 -0

+       pr_debug("%s:%d REP Cl_Class-%u CL_ID-%u CPU_ID-%u Freq-%llu}\n",
+                __func__, __LINE__, cmd.cluster_class,
+                cmd.cluster_id, cmd.cpu_id, cmd.freqency);
+
+       *rate = cmd.freqency;

Also why is this frequency u64 when all of the uses of it are unsigned long?

From: Jassi Brar <[email protected]>

The CRG11 clock controller is managed by remote f/w.
This driver simply maps Linux CLK ops onto mailbox api.

Signed-off-by Jassi Brar <[email protected]>

Signed-off-by Andy Green <[email protected]>

Signed-off-by Vincent Yang <[email protected]>

Signed-off-by Tetsuya Nuriya <[email protected]>

---
 .../bindings/clock/fujitsu,mb86s70-crg11.txt       |  26 ++
 drivers/clk/Makefile                               |   1 +
 drivers/clk/clk-mb86s7x.c                          | 386 +++++++++++++++++++++
 3 files changed, 413 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/fujitsu,mb86s70-crg11.txt
 create mode 100644 drivers/clk/clk-mb86s7x.c

Documentation/devicetree/bindings/clock/fujitsu,mb86s70-crg11.txt new file +14 -0

new file mode 100644
index 0000000..3323962
@@ -0,0 +1,26 @@
+Fujitsu CRG11 clock driver bindings
+-----------------------------------
+
+Required properties :
+- compatible : Shall contain "fujitsu,mb86s70-crg11"
+- #clock-cells : Shall be 3 {cntrlr domain port}
+
+The consumer specifies the desired clock pointing to its phandle.
+
+Example:
+
+       clock: crg11 {
+               compatible = "fujitsu,mb86s70-crg11";
+               #clock-cells = <3>;

Quoting Vincent Yang (2015-02-05 18:10:49)

Quoted 43 lines

From: Jassi Brar <[email protected]>

The CRG11 clock controller is managed by remote f/w.
This driver simply maps Linux CLK ops onto mailbox api.

Signed-off-by Jassi Brar <[email protected]>

Signed-off-by Andy Green <[email protected]>

Signed-off-by Vincent Yang <[email protected]>

Signed-off-by Tetsuya Nuriya <[email protected]>

---
 .../bindings/clock/fujitsu,mb86s70-crg11.txt       |  26 ++
 drivers/clk/Makefile                               |   1 +
 drivers/clk/clk-mb86s7x.c                          | 386 +++++++++++++++++++++
 3 files changed, 413 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/fujitsu,mb86s70-crg11.txt
 create mode 100644 drivers/clk/clk-mb86s7x.c

Documentation/devicetree/bindings/clock/fujitsu,mb86s70-crg11.txt new file +20 -0

new file mode 100644
index 0000000..3323962
@@ -0,0 +1,26 @@
+Fujitsu CRG11 clock driver bindings
+-----------------------------------
+
+Required properties :
+- compatible : Shall contain "fujitsu,mb86s70-crg11"
+- #clock-cells : Shall be 3 {cntrlr domain port}
+
+The consumer specifies the desired clock pointing to its phandle.
+
+Example:
+
+       clock: crg11 {
+               compatible = "fujitsu,mb86s70-crg11";
+               #clock-cells = <3>;
+       };
+
+       mhu: mhu0 at 2b1f0000 {
+               #mbox-cells = <1>;
+               compatible = "arm,mhu";
+               reg = <0 0x2B1F0000 0x1000>;

Documentation/devicetree/bindings/clock/fujitsu,mb86s70-crg11.txt line 24

Quoted 4 lines

Documentation/devicetree/bindings/clock/fujitsu,mb86s70-crg11.txt new file +4 -0

+               interrupts = <0 36 4>, /* LP Non-Sec */
+                            <0 35 4>, /* HP Non-Sec */
+                            <0 37 4>; /* Secure */
+               clocks = <&clock 0 2 1>; /* Cntrlr:0 Domain:2 Port:1 */

Some preprocessor definitions would be better than hardcoding the values for Cntrlr, Domain and Port. The DT include chroot should help you here. Doing so will help you maintain this stuff into the future :-)