acassis commented on code in PR #17102:
URL: https://github.com/apache/nuttx/pull/17102#discussion_r2384191046


##########
arch/arm64/src/bcm2711/bcm2711_mailbox.c:
##########
@@ -0,0 +1,509 @@
+/****************************************************************************
+ * arch/arm64/src/bcm2711/bcm2711_mailbox.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* NOTE: this module is written with the expectation that all requests are
+ * coming from the ARM core, since that is where NuttX runs. As such, mailbox
+ * 0 is used only to read responses from the VC, while mailbox 1 is used
+ * only to write requests from the ARM core to the VC.
+ *
+ * TODO: The VideoCore mailbox API is capable of asynchronous
+ * request/response. It is not guaranteed that requests will be responded to
+ * in the same order they were went. In order to save myself the headache of
+ * working around this, I've implemented each request/response interaction as
+ * a single atomic transaction. All other callers must wait for the lock in
+ * order to get their turn. This is not the most efficient, but it allows me
+ * to move on to other features and the slow-down should be very negligible,
+ * at least for the current use-case of getting property tags.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/cache.h>
+#include <nuttx/config.h>
+
+#include <debug.h>
+#include <nuttx/arch.h>
+#include <nuttx/irq.h>
+
+#include "arm64_arch.h"
+#include "arm64_gic.h"
+#include "arm64_internal.h"
+#include "bcm2711_mailbox.h"
+#include "chip.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Aliases for mailboxes so it is difficult to accidentally violate r/w
+ * constraints.
+ */
+
+#define READ_MBOX BCM_VC_MBOX0
+#define WRITE_MBOX BCM_VC_MBOX1
+
+/* Power status get tag uses bit 1 to indicate if the device ID exists */
+
+#define MBOX_DEVICE_DNE (1 << 1)
+
+/* Constants to keep track of field count for determining buffer sizes */
+
+#define TAG_FIELDS (3) /* See bcm2711_mbox_tag_s fields */
+#define BUF_FIELDS (3) /* Size, request flags, taglist terminator */
+
+/* Helper for special buffer alignment */
+
+#define ALIGNED_MBOX __attribute__((aligned(16)))
+
+/* Helpers */
+
+#define mbox_full(box)                                                       \
+  (getreg32(BCM_VC_MBOX_STATUS((box))) & BCM_VC_MBOX_STATUS_FULL)
+#define mbox_empty(box)                                                      \
+  (getreg32(BCM_VC_MBOX_STATUS((box))) & BCM_VC_MBOX_STATUS_EMPTY)
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/* Represents the format of a mailbox tag */
+
+struct bcm2711_mbox_tag_s
+{
+  uint32_t tagid;   /* Tag identifier */
+  uint32_t vbufsiz; /* Value buffer size in bytes */
+  uint32_t code;    /* Request/response code */
+
+  /* NOTE: Following this structure there should be the value buffer of size
+   * `vbufsiz`
+   */
+};
+
+/* Contains necessary state for operating interrupt-driven API */
+
+struct bcm2711_mbox_s
+{
+  mutex_t lock;  /* Lock for atomic request/response interactions */
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct bcm2711_mbox_s g_mbox =
+{
+  .lock = NXMUTEX_INITIALIZER,
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static uint32_t bcm2711_mbox_rword(uint8_t channel);
+static void bcm2711_mbox_wword(uint8_t channel, uint32_t data);
+static void bcm2711_mbox_makereq(uint32_t tag, FAR void *buf, uint32_t nval,
+                                 uint32_t nbuf);
+static int bcm2711_mbox_sendreq(FAR uint32_t *buf, uint8_t n);
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: bcm2711_mbox_rword
+ *
+ * Description:
+ *   Read a single word from the mailbox that matches `channel`
+ *   WARNING: Data read not from the expected channel is discarded. For now
+ *   this is okay, since the implementation _only_ considers reading the
+ *   property tags.
+ *
+ * Input parameters:
+ *   channel - The mailbox channel data should come from
+ *
+ * Returned Value:
+ *    The value returned by the mailbox, with the channel masked out and the
+ *    data bits shifted to compensate.
+ ****************************************************************************/
+
+static uint32_t bcm2711_mbox_rword(uint8_t channel)
+{
+  uint32_t data;
+
+  for (; ; )
+    {
+      /* Wait for mailbox to have data */
+
+      while (mbox_empty(READ_MBOX))
+        ;
+
+      data = getreg32(BCM_VC_MBOX_RW(READ_MBOX));
+
+      /* Channel matches, return word! */
+
+      if ((data & BCM_VC_MBOX_RW_CHAN_MASK) == channel)
+        {
+          return data & ~BCM_VC_MBOX_RW_CHAN_MASK;
+        }
+
+      /* No channel match, so we continue until it does. */
+    }
+}
+
+/****************************************************************************
+ * Name: bcm2711_mbox_wword
+ *
+ * Description:
+ *   Write a single word to the mailbox channel `channel`
+ *
+ * Input parameters:
+ *   channel - The mailbox channel data will be written to
+ *   data - The data to write to the mailbox
+ ****************************************************************************/
+
+static void bcm2711_mbox_wword(uint8_t channel, uint32_t data)
+{
+  /* Wait for space when the mailbox is full */
+
+  while (mbox_full(WRITE_MBOX))
+    ;
+
+  /* Mask excess channel bits */
+
+  channel &= BCM_VC_MBOX_RW_CHAN_MASK;
+
+  /* Write the data */
+
+  putreg32((data & (~0xf)) | channel, BCM_VC_MBOX_RW(WRITE_MBOX));
+}
+
+/****************************************************************************
+ * Name: bcm2711_mbox_sendreq
+ *
+ * Description:
+ *   Send a request to the VideoCore through the mailbox. This is atomic
+ *   (other requests must wait for this one to complete).
+ *   WARNING: The `buf` parameter must be 16-byte aligned and not NULL.
+ *
+ * Input parameters:
+ *   buf  - A buffer containing enough space for the tag (12 bytes), buffer
+ *          size indicator (4 bytes), buffer request code (4 bytes), end tag
+ *          (4 bytes) and the maximum size of the response length or the tag
+ *          value length (tag dependent).
+ *
+ * Returned Value:
+ *   0 on success, -EIO on failure. Results are inside the buffer.
+ ****************************************************************************/
+
+static int bcm2711_mbox_sendreq(FAR uint32_t *buf, uint8_t n)
+{
+  int res;
+  uint32_t bufptr = (uint32_t)(uintptr_t)(buf);
+
+  /* Write the buffer to the VideoCore */
+
+  up_flush_dcache(bufptr, bufptr + n);
+  nxmutex_lock(&g_mbox.lock);
+  bcm2711_mbox_wword(MBOX_CHAN_TAGS, bufptr);
+
+  /* Get the response from the VideoCore.
+   *
+   * Before we check the buffer contents, we must invalidate the data cache
+   * on this buffer's contents or we'll just read back what we sent without
+   * modification. This is done before the 'write' to provide time for the
+   * cache to flush.
+   */
+
+  res = bcm2711_mbox_rword(MBOX_CHAN_TAGS);
+  nxmutex_unlock(&g_mbox.lock);
+
+  /* The mailbox _MUST_ return the the same buffer address, it is
+   * not possible for it to do otherwise (according to raspberrypi/firmware
+   * docs).
+   */
+
+  if (res != bufptr)
+    {
+      _err("Expected %08x, got %08x", bufptr, res);

Review Comment:
   > I was struggling to figure out what would be a good choice for the error 
message. Even though this is for the "VideoCore", it's not necessarily used for 
video. It does things like get CPU temperature, enable clocks, enable power to 
subsystems, etc. Do you know any other error macros I could use? Should I make 
one for this?
   
   I think other alternative is CONFIG_DEBUG_IPC_ERROR



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to