[PATCH v6 1/3] dt/bindings: Add binding for the Raspberry Pi firmware driver

[PATCH v6 1/3] dt/bindings: Add binding for the Raspberry Pi firmware driver

[Eric Anholt]

This driver will provide support for calls into the firmware that will
be used by other drivers like cpufreq and vc4.

v2: Improve commit message, point to mailbox.txt for how mboxes work.
v3: Use Lee's suggestion for mailbox phandle docs, fix spelling of
    "raspberry".
v4: Change the compatible string to "raspberrypi,bcm2835-firmware"
    (requested by Lee, agreed by Stephen)

Acked-by: Stephen Warren <swarren@wwwdotorg.org>
---
 .../bindings/arm/bcm/raspberrypi,bcm2835-firmware.txt      | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.txt

diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.txt b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.txt
new file mode 100644
index 0000000..5fb1868
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.txt
@@ -0,0 +1,14 @@
+Raspberry Pi VideoCore firmware driver
+
+Required properties:
+
+- compatible:		Should be "rasbperrypi,bcm2835-firmware"
+- mboxes:		Phandle to the firmware device's Mailbox.
+			  (See: ../mailbox/mailbox.txt for more information)
+
+Example:
+
+firmware {
+	compatible = "raspberrypi,bcm2835-firmware";
+	mboxes = <&mailbox>;
+};
-- 
2.1.4

[PATCH v6 2/3] ARM: bcm2835: Add the Raspberry Pi firmware driver

[Eric Anholt]

This gives us a function for making mailbox property channel requests
of the firmware, which is most notable in that it will let us get and
set clock rates.

v2: Drop power-domains stuff for now since we don't have the driver
    core support to make it useful.  Move to drivers/firmware/.
    Capitalize the enums.  De-global the firmware variable.  Use the
    firmware device to allocate our DMA buffer, so that the dma-ranges
    DT property gets respected.  Simplify the property tag transaction
    interface even more, leaving a multi-tag interface still
    available.  For conciseness, rename "raspberrypi" to "rpi" on all
    functions/enums/structs, and the "firmware" variable to "fw".
    Print when the driver is probed successfully, since debugging
    -EPROBE_DEFER handling is such a big part of bcm2835 development.
    Drop -EBUSY mailbox handling since the mailbox core has been fixed
    to return -EPROBE_DEFER in -next.

v3: Use kernel-doc style for big comments (from Noralf), drop stale
    comment, use "dev" when freeing DMA as well.

v4: Move description comment into copyright comment, drop a dead
    initialization of "ret", and print the firmware revision at probe
    time.

v5: Rename the compatible to "raspberrypi,bcm2835-firmware", move
    include to not say "property", add functions to get struct
    rpi_firmware from the device_node and put when done, make property
    functions take the rpi_firmware instead and never return
    -EPROBE_DEFER, put the driver under its own RASPBERRYPI_FIRMWARE
    Kconfig.

v6: Drop the try_module_get/module_put stuff, since all clients will
    be referencing our symbols in order to call those functions,
    anyway.  Fix the kerneldoc comments for the changes in v5.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/firmware/Kconfig                   |   7 +
 drivers/firmware/Makefile                  |   1 +
 drivers/firmware/raspberrypi.c             | 260 +++++++++++++++++++++++++++++
 include/soc/bcm2835/raspberrypi-firmware.h | 115 +++++++++++++
 4 files changed, 383 insertions(+)
 create mode 100644 drivers/firmware/raspberrypi.c
 create mode 100644 include/soc/bcm2835/raspberrypi-firmware.h

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 6517132..564aa5b 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -136,6 +136,13 @@ config QCOM_SCM
 	bool
 	depends on ARM || ARM64
 
+config RASPBERRYPI_FIRMWARE
+	tristate "Raspberry Pi Firmware Driver"
+	depends on BCM2835_MBOX
+	help
+	  This option enables support for communicating with the firmware on the
+	  Raspberry Pi.
+
 source "drivers/firmware/google/Kconfig"
 source "drivers/firmware/efi/Kconfig"
 
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 3fdd391..ee101a2 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_ISCSI_IBFT)	+= iscsi_ibft.o
 obj-$(CONFIG_FIRMWARE_MEMMAP)	+= memmap.o
 obj-$(CONFIG_QCOM_SCM)		+= qcom_scm.o
 CFLAGS_qcom_scm.o :=$(call as-instr,.arch_extension sec,-DREQUIRES_SEC=1)
+obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o
 
 obj-$(CONFIG_GOOGLE_FIRMWARE)	+= google/
 obj-$(CONFIG_EFI)		+= efi/
diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
new file mode 100644
index 0000000..dd506cd
--- /dev/null
+++ b/drivers/firmware/raspberrypi.c
@@ -0,0 +1,260 @@
+/*
+ * Defines interfaces for interacting wtih the Raspberry Pi firmware's
+ * property channel.
+ *
+ * Copyright © 2015 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/dma-mapping.h>
+#include <linux/mailbox_client.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <soc/bcm2835/raspberrypi-firmware.h>
+
+#define MBOX_MSG(chan, data28)		(((data28) & ~0xf) | ((chan) & 0xf))
+#define MBOX_CHAN(msg)			((msg) & 0xf)
+#define MBOX_DATA28(msg)		((msg) & ~0xf)
+#define MBOX_CHAN_PROPERTY		8
+
+struct rpi_firmware {
+	struct mbox_client cl;
+	struct mbox_chan *chan; /* The property channel. */
+	struct completion c;
+	u32 enabled;
+};
+
+static DEFINE_MUTEX(transaction_lock);
+
+static void response_callback(struct mbox_client *cl, void *msg)
+{
+	struct rpi_firmware *fw = container_of(cl, struct rpi_firmware, cl);
+	complete(&fw->c);
+}
+
+/*
+ * Sends a request to the firmware through the BCM2835 mailbox driver,
+ * and synchronously waits for the reply.
+ */
+static int
+rpi_firmware_transaction(struct rpi_firmware *fw, u32 chan, u32 data)
+{
+	u32 message = MBOX_MSG(chan, data);
+	int ret;
+
+	WARN_ON(data & 0xf);
+
+	mutex_lock(&transaction_lock);
+	reinit_completion(&fw->c);
+	ret = mbox_send_message(fw->chan, &message);
+	if (ret >= 0) {
+		wait_for_completion(&fw->c);
+		ret = 0;
+	} else {
+		dev_err(fw->cl.dev, "mbox_send_message returned %d\n", ret);
+	}
+	mutex_unlock(&transaction_lock);
+
+	return ret;
+}
+
+/**
+ * rpi_firmware_property_list - Submit firmware property list
+ * @fw:		Pointer to firmware structure from rpi_firmware_get().
+ * @data:	Buffer holding tags.
+ * @tag_size:	Size of tags buffer.
+ *
+ * Submits a set of concatenated tags to the VPU firmware through the
+ * mailbox property interface.
+ *
+ * The buffer header and the ending tag are added by this function and
+ * don't need to be supplied, just the actual tags for your operation.
+ * See struct rpi_firmware_property_tag_header for the per-tag
+ * structure.
+ */
+int rpi_firmware_property_list(struct rpi_firmware *fw,
+			       void *data, size_t tag_size)
+{
+	size_t size = tag_size + 12;
+	u32 *buf;
+	dma_addr_t bus_addr;
+	int ret;
+
+	/* Packets are processed a dword at a time. */
+	if (size & 3)
+		return -EINVAL;
+
+	buf = dma_alloc_coherent(fw->cl.dev, PAGE_ALIGN(size), &bus_addr,
+				 GFP_ATOMIC);
+	if (!buf)
+		return -ENOMEM;
+
+	/* The firmware will error out without parsing in this case. */
+	WARN_ON(size >= 1024 * 1024);
+
+	buf[0] = size;
+	buf[1] = RPI_FIRMWARE_STATUS_REQUEST;
+	memcpy(&buf[2], data, tag_size);
+	buf[size / 4 - 1] = RPI_FIRMWARE_PROPERTY_END;
+	wmb();
+
+	ret = rpi_firmware_transaction(fw, MBOX_CHAN_PROPERTY, bus_addr);
+
+	rmb();
+	memcpy(data, &buf[2], tag_size);
+	if (ret == 0 && buf[1] != RPI_FIRMWARE_STATUS_SUCCESS) {
+		/*
+		 * The tag name here might not be the one causing the
+		 * error, if there were multiple tags in the request.
+		 * But single-tag is the most common, so go with it.
+		 */
+		dev_err(fw->cl.dev, "Request 0x%08x returned status 0x%08x\n",
+			buf[2], buf[1]);
+		ret = -EINVAL;
+	}
+
+	dma_free_coherent(fw->cl.dev, PAGE_ALIGN(size), buf, bus_addr);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rpi_firmware_property_list);
+
+/**
+ * rpi_firmware_property - Submit single firmware property
+ * @fw:		Pointer to firmware structure from rpi_firmware_get().
+ * @tag:	One of enum_mbox_property_tag.
+ * @tag_data:	Tag data buffer.
+ * @buf_size:	Buffer size.
+ *
+ * Submits a single tag to the VPU firmware through the mailbox
+ * property interface.
+ *
+ * This is a convenience wrapper around
+ * rpi_firmware_property_list() to avoid some of the
+ * boilerplate in property calls.
+ */
+int rpi_firmware_property(struct rpi_firmware *fw,
+			  u32 tag, void *tag_data, size_t buf_size)
+{
+	/* Single tags are very small (generally 8 bytes), so the
+	 * stack should be safe.
+	 */
+	u8 data[buf_size + sizeof(struct rpi_firmware_property_tag_header)];
+	struct rpi_firmware_property_tag_header *header =
+		(struct rpi_firmware_property_tag_header *)data;
+	int ret;
+
+	header->tag = tag;
+	header->buf_size = buf_size;
+	header->req_resp_size = 0;
+	memcpy(data + sizeof(struct rpi_firmware_property_tag_header),
+	       tag_data, buf_size);
+
+	ret = rpi_firmware_property_list(fw, &data, sizeof(data));
+	memcpy(tag_data,
+	       data + sizeof(struct rpi_firmware_property_tag_header),
+	       buf_size);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rpi_firmware_property);
+
+static void
+rpi_firmware_print_firmware_revision(struct rpi_firmware *fw)
+{
+	u32 packet;
+	int ret = rpi_firmware_property(fw,
+					RPI_FIRMWARE_GET_FIRMWARE_REVISION,
+					&packet, sizeof(packet));
+
+	if (ret == 0) {
+		struct tm tm;
+
+		time_to_tm(packet, 0, &tm);
+
+		dev_info(fw->cl.dev,
+			 "Attached to firmware from %04ld-%02d-%02d %02d:%02d\n",
+			 tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
+			 tm.tm_hour, tm.tm_min);
+	}
+}
+
+static int rpi_firmware_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rpi_firmware *fw;
+
+	fw = devm_kzalloc(dev, sizeof(*fw), GFP_KERNEL);
+	if (!fw)
+		return -ENOMEM;
+
+	fw->cl.dev = dev;
+	fw->cl.rx_callback = response_callback;
+	fw->cl.tx_block = true;
+
+	fw->chan = mbox_request_channel(&fw->cl, 0);
+	if (IS_ERR(fw->chan)) {
+		int ret = PTR_ERR(fw->chan);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get mbox channel: %d\n", ret);
+		return ret;
+	}
+
+	init_completion(&fw->c);
+
+	platform_set_drvdata(pdev, fw);
+
+	rpi_firmware_print_firmware_revision(fw);
+
+	return 0;
+}
+
+static int rpi_firmware_remove(struct platform_device *pdev)
+{
+	struct rpi_firmware *fw = platform_get_drvdata(pdev);
+
+	mbox_free_channel(fw->chan);
+
+	return 0;
+}
+
+/**
+ * rpi_firmware_get - Get pointer to rpi_firmware structure.
+ * @firmware_node:    Pointer to the firmware Device Tree node.
+ *
+ * Returns NULL is the firmware device is not ready.
+ */
+struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node)
+{
+	struct platform_device *pdev = of_find_device_by_node(firmware_node);
+
+	if (!pdev)
+		return NULL;
+
+	return platform_get_drvdata(pdev);
+}
+EXPORT_SYMBOL_GPL(rpi_firmware_get);
+
+static const struct of_device_id rpi_firmware_of_match[] = {
+	{ .compatible = "raspberrypi,bcm2835-firmware", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rpi_firmware_of_match);
+
+static struct platform_driver rpi_firmware_driver = {
+	.driver = {
+		.name = "raspberrypi-firmware",
+		.of_match_table = rpi_firmware_of_match,
+	},
+	.probe		= rpi_firmware_probe,
+	.remove		= rpi_firmware_remove,
+};
+module_platform_driver(rpi_firmware_driver);
+
+MODULE_AUTHOR("Eric Anholt <eric@anholt.net>");
+MODULE_DESCRIPTION("Raspberry Pi firmware driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
new file mode 100644
index 0000000..9d9efb7
--- /dev/null
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -0,0 +1,115 @@
+/*
+ *  Copyright © 2015 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/types.h>
+#include <linux/of_device.h>
+
+struct rpi_firmware;
+
+enum rpi_firmware_property_status {
+	RPI_FIRMWARE_STATUS_REQUEST = 0,
+	RPI_FIRMWARE_STATUS_SUCCESS = 0x80000000,
+	RPI_FIRMWARE_STATUS_ERROR =   0x80000001,
+};
+
+/**
+ * struct rpi_firmware_property_tag_header - Firmware property tag header
+ * @tag:		One of enum_mbox_property_tag.
+ * @buf_size:		The number of bytes in the value buffer following this
+ *			struct.
+ * @req_resp_size:	On submit, the length of the request (though it doesn't
+ *			appear to be currently used by the firmware).  On return,
+ *			the length of the response (always 4 byte aligned), with
+ *			the low bit set.
+ */
+struct rpi_firmware_property_tag_header {
+	u32 tag;
+	u32 buf_size;
+	u32 req_resp_size;
+};
+
+enum rpi_firmware_property_tag {
+	RPI_FIRMWARE_PROPERTY_END =                           0,
+	RPI_FIRMWARE_GET_FIRMWARE_REVISION =                  0x00000001,
+
+	RPI_FIRMWARE_SET_CURSOR_INFO =                        0x00008010,
+	RPI_FIRMWARE_SET_CURSOR_STATE =                       0x00008011,
+
+	RPI_FIRMWARE_GET_BOARD_MODEL =                        0x00010001,
+	RPI_FIRMWARE_GET_BOARD_REVISION =                     0x00010002,
+	RPI_FIRMWARE_GET_BOARD_MAC_ADDRESS =                  0x00010003,
+	RPI_FIRMWARE_GET_BOARD_SERIAL =                       0x00010004,
+	RPI_FIRMWARE_GET_ARM_MEMORY =                         0x00010005,
+	RPI_FIRMWARE_GET_VC_MEMORY =                          0x00010006,
+	RPI_FIRMWARE_GET_CLOCKS =                             0x00010007,
+	RPI_FIRMWARE_GET_POWER_STATE =                        0x00020001,
+	RPI_FIRMWARE_GET_TIMING =                             0x00020002,
+	RPI_FIRMWARE_SET_POWER_STATE =                        0x00028001,
+	RPI_FIRMWARE_GET_CLOCK_STATE =                        0x00030001,
+	RPI_FIRMWARE_GET_CLOCK_RATE =                         0x00030002,
+	RPI_FIRMWARE_GET_VOLTAGE =                            0x00030003,
+	RPI_FIRMWARE_GET_MAX_CLOCK_RATE =                     0x00030004,
+	RPI_FIRMWARE_GET_MAX_VOLTAGE =                        0x00030005,
+	RPI_FIRMWARE_GET_TEMPERATURE =                        0x00030006,
+	RPI_FIRMWARE_GET_MIN_CLOCK_RATE =                     0x00030007,
+	RPI_FIRMWARE_GET_MIN_VOLTAGE =                        0x00030008,
+	RPI_FIRMWARE_GET_TURBO =                              0x00030009,
+	RPI_FIRMWARE_GET_MAX_TEMPERATURE =                    0x0003000a,
+	RPI_FIRMWARE_ALLOCATE_MEMORY =                        0x0003000c,
+	RPI_FIRMWARE_LOCK_MEMORY =                            0x0003000d,
+	RPI_FIRMWARE_UNLOCK_MEMORY =                          0x0003000e,
+	RPI_FIRMWARE_RELEASE_MEMORY =                         0x0003000f,
+	RPI_FIRMWARE_EXECUTE_CODE =                           0x00030010,
+	RPI_FIRMWARE_EXECUTE_QPU =                            0x00030011,
+	RPI_FIRMWARE_SET_ENABLE_QPU =                         0x00030012,
+	RPI_FIRMWARE_GET_DISPMANX_RESOURCE_MEM_HANDLE =       0x00030014,
+	RPI_FIRMWARE_GET_EDID_BLOCK =                         0x00030020,
+	RPI_FIRMWARE_SET_CLOCK_STATE =                        0x00038001,
+	RPI_FIRMWARE_SET_CLOCK_RATE =                         0x00038002,
+	RPI_FIRMWARE_SET_VOLTAGE =                            0x00038003,
+	RPI_FIRMWARE_SET_TURBO =                              0x00038009,
+
+	/* Dispmanx TAGS */
+	RPI_FIRMWARE_FRAMEBUFFER_ALLOCATE =                   0x00040001,
+	RPI_FIRMWARE_FRAMEBUFFER_BLANK =                      0x00040002,
+	RPI_FIRMWARE_FRAMEBUFFER_GET_PHYSICAL_WIDTH_HEIGHT =  0x00040003,
+	RPI_FIRMWARE_FRAMEBUFFER_GET_VIRTUAL_WIDTH_HEIGHT =   0x00040004,
+	RPI_FIRMWARE_FRAMEBUFFER_GET_DEPTH =                  0x00040005,
+	RPI_FIRMWARE_FRAMEBUFFER_GET_PIXEL_ORDER =            0x00040006,
+	RPI_FIRMWARE_FRAMEBUFFER_GET_ALPHA_MODE =             0x00040007,
+	RPI_FIRMWARE_FRAMEBUFFER_GET_PITCH =                  0x00040008,
+	RPI_FIRMWARE_FRAMEBUFFER_GET_VIRTUAL_OFFSET =         0x00040009,
+	RPI_FIRMWARE_FRAMEBUFFER_GET_OVERSCAN =               0x0004000a,
+	RPI_FIRMWARE_FRAMEBUFFER_GET_PALETTE =                0x0004000b,
+	RPI_FIRMWARE_FRAMEBUFFER_RELEASE =                    0x00048001,
+	RPI_FIRMWARE_FRAMEBUFFER_TEST_PHYSICAL_WIDTH_HEIGHT = 0x00044003,
+	RPI_FIRMWARE_FRAMEBUFFER_TEST_VIRTUAL_WIDTH_HEIGHT =  0x00044004,
+	RPI_FIRMWARE_FRAMEBUFFER_TEST_DEPTH =                 0x00044005,
+	RPI_FIRMWARE_FRAMEBUFFER_TEST_PIXEL_ORDER =           0x00044006,
+	RPI_FIRMWARE_FRAMEBUFFER_TEST_ALPHA_MODE =            0x00044007,
+	RPI_FIRMWARE_FRAMEBUFFER_TEST_VIRTUAL_OFFSET =        0x00044009,
+	RPI_FIRMWARE_FRAMEBUFFER_TEST_OVERSCAN =              0x0004400a,
+	RPI_FIRMWARE_FRAMEBUFFER_TEST_PALETTE =               0x0004400b,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_PHYSICAL_WIDTH_HEIGHT =  0x00048003,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_VIRTUAL_WIDTH_HEIGHT =   0x00048004,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_DEPTH =                  0x00048005,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_PIXEL_ORDER =            0x00048006,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_ALPHA_MODE =             0x00048007,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_VIRTUAL_OFFSET =         0x00048009,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_OVERSCAN =               0x0004800a,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_PALETTE =                0x0004800b,
+
+	RPI_FIRMWARE_GET_COMMAND_LINE =                       0x00050001,
+	RPI_FIRMWARE_GET_DMA_CHANNELS =                       0x00060001,
+};
+
+int rpi_firmware_property(struct rpi_firmware *fw,
+			  u32 tag, void *data, size_t len);
+int rpi_firmware_property_list(struct rpi_firmware *fw,
+			       void *data, size_t tag_size);
+struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node);
-- 
2.1.4

[PATCH v6 3/3] ARM: bcm2835: Add the firmware driver information to the RPi DT

[Eric Anholt]

v2: Drop pm-domains stuff since I've dropped it from the firmware
    driver for now, until we get drivers/base fixed.

v3: Rename the compatible to "raspberrypi,bcm2835-firmware"

Signed-off-by: Eric Anholt <eric@anholt.net>
Acked-by: Lee Jones <lee@kernel.org> (previous version with pm-domains)
Acked-by: Stephen Warren <swarren@wwwdotorg.org>
---
 arch/arm/boot/dts/bcm2835-rpi.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index 46780bb..ab5474e 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -14,6 +14,13 @@
 			linux,default-trigger = "heartbeat";
 		};
 	};
+
+	soc {
+		firmware: firmware {
+			compatible = "raspberrypi,bcm2835-firmware";
+			mboxes = <&mailbox>;
+		};
+	};
 };
 
 &gpio {
-- 
2.1.4

Re: [PATCH v6 2/3] ARM: bcm2835: Add the Raspberry Pi firmware driver

[Paul Bolle]

On Fri, 2015-05-29 at 12:42 -0700, Eric Anholt wrote:
> This gives us a function for making mailbox property channel requests
> of the firmware, which is most notable in that it will let us get and
> set clock rates.
> 
> v2: Drop power-domains stuff for now since we don't have the driver
>     core support to make it useful.  Move to drivers/firmware/.
>     Capitalize the enums.  De-global the firmware variable.  Use the
>     firmware device to allocate our DMA buffer, so that the dma-ranges
>     DT property gets respected.  Simplify the property tag transaction
>     interface even more, leaving a multi-tag interface still
>     available.  For conciseness, rename "raspberrypi" to "rpi" on all
>     functions/enums/structs, and the "firmware" variable to "fw".
>     Print when the driver is probed successfully, since debugging
>     -EPROBE_DEFER handling is such a big part of bcm2835 development.
>     Drop -EBUSY mailbox handling since the mailbox core has been fixed
>     to return -EPROBE_DEFER in -next.
> 
> v3: Use kernel-doc style for big comments (from Noralf), drop stale
>     comment, use "dev" when freeing DMA as well.
> 
> v4: Move description comment into copyright comment, drop a dead
>     initialization of "ret", and print the firmware revision at probe
>     time.
> 
> v5: Rename the compatible to "raspberrypi,bcm2835-firmware", move
>     include to not say "property", add functions to get struct
>     rpi_firmware from the device_node and put when done, make property
>     functions take the rpi_firmware instead and never return
>     -EPROBE_DEFER, put the driver under its own RASPBERRYPI_FIRMWARE
>     Kconfig.
> 
> v6: Drop the try_module_get/module_put stuff, since all clients will
>     be referencing our symbols in order to call those functions,
>     anyway.  Fix the kerneldoc comments for the changes in v5.

(This style of commit explanation is getting quite common. I must say I
rather dislike it. I think people should just update the entire commit
explanation when needed, and not simply paste any changes at the end of
it, thereby forcing the the reader to determine which older parts are
actually overruled by newer parts. Besides, many, or maybe even most, of
the changes are really not interesting enough to keep in the commit
explanation.)

> --- /dev/null
> +++ b/drivers/firmware/raspberrypi.c

> +EXPORT_SYMBOL_GPL(rpi_firmware_property_list);

> +EXPORT_SYMBOL_GPL(rpi_firmware_property);

A patch that uses these exports hit lkml recently:
https://lkml.org/lkml/2015/5/28/596 .

> +EXPORT_SYMBOL_GPL(rpi_firmware_get);

But I didn't spot a (recent) patch that uses this export. Is it queued
somewhere?

Thanks,


Paul Bolle

Re: [PATCH v6 2/3] ARM: bcm2835: Add the Raspberry Pi firmware driver

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 1600 bytes --]

Paul Bolle <pebolle@tiscali.nl> writes:

> On Fri, 2015-05-29 at 12:42 -0700, Eric Anholt wrote:
>> This gives us a function for making mailbox property channel requests
>> of the firmware, which is most notable in that it will let us get and
>> set clock rates.

>> v6: Drop the try_module_get/module_put stuff, since all clients will
>>     be referencing our symbols in order to call those functions,
>>     anyway.  Fix the kerneldoc comments for the changes in v5.
>
> (This style of commit explanation is getting quite common. I must say I
> rather dislike it. I think people should just update the entire commit
> explanation when needed, and not simply paste any changes at the end of
> it, thereby forcing the the reader to determine which older parts are
> actually overruled by newer parts. Besides, many, or maybe even most, of
> the changes are really not interesting enough to keep in the commit
> explanation.)

It was a slip-up that I didn't move it below the '---'this time (I
changed my workflow a bit, and missed the step).  I was assuming that
Lee would just trim them on pulling in the changes.

>> --- /dev/null
>> +++ b/drivers/firmware/raspberrypi.c
>
>> +EXPORT_SYMBOL_GPL(rpi_firmware_property_list);
>
>> +EXPORT_SYMBOL_GPL(rpi_firmware_property);
>
> A patch that uses these exports hit lkml recently:
> https://lkml.org/lkml/2015/5/28/596 .
>
>> +EXPORT_SYMBOL_GPL(rpi_firmware_get);
>
> But I didn't spot a (recent) patch that uses this export. Is it queued
> somewhere?

This is from the v5 changes due to review feedback, and the clients need
to be changed to use it.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH v6 1/3] dt/bindings: Add binding for the Raspberry Pi firmware driver

[Lee Jones]

What's going on here?  This is unlike you.

> This driver will provide support for calls into the firmware that will
> be used by other drivers like cpufreq and vc4.
> 
> v2: Improve commit message, point to mailbox.txt for how mboxes work.
> v3: Use Lee's suggestion for mailbox phandle docs, fix spelling of
>     "raspberry".
> v4: Change the compatible string to "raspberrypi,bcm2835-firmware"
>     (requested by Lee, agreed by Stephen)

Perhaps this should have culminated in a Suggested-by.

The change-log should be below the "---".

> Acked-by: Stephen Warren <swarren@wwwdotorg.org>

Where's your SoB?

> ---
>  .../bindings/arm/bcm/raspberrypi,bcm2835-firmware.txt      | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.txt b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.txt
> new file mode 100644
> index 0000000..5fb1868
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.txt
> @@ -0,0 +1,14 @@
> +Raspberry Pi VideoCore firmware driver
> +
> +Required properties:
> +
> +- compatible:		Should be "rasbperrypi,bcm2835-firmware"

Spelling.

> +- mboxes:		Phandle to the firmware device's Mailbox.
> +			  (See: ../mailbox/mailbox.txt for more information)
> +
> +Example:
> +
> +firmware {
> +	compatible = "raspberrypi,bcm2835-firmware";
> +	mboxes = <&mailbox>;
> +};

Re: [PATCH v6 2/3] ARM: bcm2835: Add the Raspberry Pi firmware driver

[Lee Jones]

On Mon, 01 Jun 2015, Eric Anholt wrote:

> Paul Bolle <pebolle@tiscali.nl> writes:
> 
> > On Fri, 2015-05-29 at 12:42 -0700, Eric Anholt wrote:
> >> This gives us a function for making mailbox property channel requests
> >> of the firmware, which is most notable in that it will let us get and
> >> set clock rates.
> 
> >> v6: Drop the try_module_get/module_put stuff, since all clients will
> >>     be referencing our symbols in order to call those functions,
> >>     anyway.  Fix the kerneldoc comments for the changes in v5.
> >
> > (This style of commit explanation is getting quite common. I must say I
> > rather dislike it. I think people should just update the entire commit
> > explanation when needed, and not simply paste any changes at the end of
> > it, thereby forcing the the reader to determine which older parts are
> > actually overruled by newer parts. Besides, many, or maybe even most, of
> > the changes are really not interesting enough to keep in the commit
> > explanation.)
> 
> It was a slip-up that I didn't move it below the '---'this time (I
> changed my workflow a bit, and missed the step).  I was assuming that
> Lee would just trim them on pulling in the changes.

I'm not the Firmware Maintainer, but I'm sure whoever is will be happy
to remove the changelog.  But Paul is correct, the commit should be
updated.