Messages in this thread Patch in this message |  | | From | Jassi Brar <> | Date | Sun, 17 Jul 2022 10:17:46 -0500 | Subject | Re: [PATCH 2/2] mailbox: mvl_mhu: Add support for Marvell Message Handling Unit |
| |
On Thu, Jul 14, 2022 at 7:13 AM Wojciech Bartczak <wbartczak@marvell.com> wrote:
... > +config MVL_MHU > + tristate "Marvell MHU Mailbox" if ARM_SCMI_PROTOCOL > + depends on OF && ARM64 You want to make it depend on your ARCH or COMPILE_TEST
.... > diff --git a/drivers/mailbox/mvl_mhu.c b/drivers/mailbox/mvl_mhu.c > @@ -0,0 +1,473 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Marvell Message Handling Unit driver This doesn't seem to be related to ARM-MHU. Please consider calling it something else. ... > +#include <linux/module.h> > +#include <linux/init.h> > +#include <linux/kernel.h> > +#include <linux/interrupt.h> > +#include <linux/io.h> > +#include <linux/interrupt.h> > +#include <linux/device.h> > +#include <linux/platform_device.h> > +#include <linux/pci.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/of_irq.h> > +#include <linux/acpi.h> > +#include <linux/mailbox_controller.h> > +#include <linux/spinlock.h> > + please consider dropping unnecessary includes
... > +/* > + * Doorbell-Register: XCP(0..1)_DEV(0..7)_XCP_MBOX > + * Communication data from devices to XCP. When written, sets > + * XCP(0..1)_DEV(0..7)_XCP_MBOX. > + * PS: it doesn't matter what is written into this register, > + * Attempting to writing 'anything' would cause an interrupt > + * to the target! > + */ > + > +#define DONT_CARE_DATA 0xFF likely only one platform-specific bit needs to be set. you may want to get that from dt/client ?
.... > +#define XCPX_DEVY_XCP_MBOX_OFFSET 0x000E1000 > +#define XCP_TO_DEV_XCP_MBOX(xcp_core, device_id) \ > + (XCPX_DEVY_XCP_MBOX_OFFSET | \ > + ((uint64_t)(xcp_core) << 36) | \ > + ((uint64_t)(device_id) << 4)) > + > +/* AP0-to-SCP doorbell */ > +#define AP0_TO_SCP_MBOX XCP_TO_DEV_XCP_MBOX(SCP_INDEX, DEV_AP0) > + > +/* Register offset: Enable interrupt from SCP to AP */ > +#define XCP0_XCP_DEV0_MBOX_RINT_ENA_W1S 0x000D1C40 > +#define XCP0_XCP_DEV1_MBOX_RINT_ENA_W1S 0x000D1C50 > +#define XCP0_XCP_DEV2_MBOX_RINT_ENA_W1S 0x000D1C60 > +#define XCP0_XCP_DEV3_MBOX_RINT_ENA_W1S 0x000D1C70 maybe use more compact defines? .... > + > +#define MHU_CHANNEL_INDEX(mhu, chan) (chan - &mhu->chan[0]) this is unused .... > + > +/* Secures static data processed in the irq handler */ > +DEFINE_SPINLOCK(mhu_irq_spinlock); Please move this in 'struct mhu' ... > +static irqreturn_t mhu_rx_interrupt_thread(int irq, void *p) > +{ > + struct mhu *mhu = (struct mhu *)p; > + struct int_src_data_s *data = (struct int_src_data_s *)mhu->payload; > + u64 val, scmi_tx_cnt; > + > + /* > + * Local copy of event counters. A mismatch of received > + * count value and the local copy means additional events > + * are being flagged that needs to be attended by AP > + */ > + static u64 event_counter[INDEX_INT_SRC_NONE] = {0}; Please move this in 'struct mhu'
.... > +static bool mhu_last_tx_done(struct mbox_chan *chan) > +{ > + struct mhu *mhu = chan->con_priv; > + u64 status; > + > + status = ioread64(mhu->base + XCPX_XCP_DEVY_MBOX_RINT(0, 2)); > + pr_debug("last_tx_done status: %#llx\n", status); please use dev_XXX instead of pr_debug, here and elsewhere thanks
|  |