Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751898AbdCOGhX (ORCPT ); Wed, 15 Mar 2017 02:37:23 -0400 Received: from mail-vk0-f51.google.com ([209.85.213.51]:32948 "EHLO mail-vk0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751242AbdCOGhV (ORCPT ); Wed, 15 Mar 2017 02:37:21 -0400 MIME-Version: 1.0 In-Reply-To: References: <1488793240-25484-1-git-send-email-anup.patel@broadcom.com> <1488793240-25484-2-git-send-email-anup.patel@broadcom.com> From: Anup Patel Date: Wed, 15 Mar 2017 12:07:19 +0530 Message-ID: Subject: Re: [PATCH v5 1/2] mailbox: Add driver for Broadcom FlexRM ring manager To: Jassi Brar Cc: Rob Herring , Mark Rutland , Ray Jui , Scott Branden , Pramod KUMAR , Rob Rice , Devicetree List , Linux Kernel Mailing List , "linux-arm-kernel@lists.infradead.org" , BCM Kernel Feedback Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2943 Lines: 67 On Tue, Mar 14, 2017 at 10:24 PM, Jassi Brar wrote: > On Mon, Mar 6, 2017 at 10:40 AM, Anup Patel wrote: >> Some of the Broadcom iProc SoCs have FlexRM ring manager >> which provides a ring-based programming interface to various >> offload engines (e.g. RAID, Crypto, etc). >> >> This patch adds a common mailbox driver for Broadcom FlexRM >> ring manager which can be shared by various offload engine >> drivers (implemented as mailbox clients). >> >> Reviewed-by: Ray Jui >> Reviewed-by: Scott Branden >> Reviewed-by: Pramod KUMAR >> Signed-off-by: Anup Patel >> --- >> drivers/mailbox/Kconfig | 11 + >> drivers/mailbox/Makefile | 2 + >> drivers/mailbox/mailbox-flexrm/Makefile | 6 + >> drivers/mailbox/mailbox-flexrm/flexrm-desc.c | 764 ++++++++++++++++++++++++ >> drivers/mailbox/mailbox-flexrm/flexrm-desc.h | 47 ++ >> drivers/mailbox/mailbox-flexrm/flexrm-main.c | 829 +++++++++++++++++++++++++++ >> include/linux/mailbox/brcm-message.h | 14 +- >> 7 files changed, 1669 insertions(+), 4 deletions(-) >> create mode 100644 drivers/mailbox/mailbox-flexrm/Makefile >> create mode 100644 drivers/mailbox/mailbox-flexrm/flexrm-desc.c >> create mode 100644 drivers/mailbox/mailbox-flexrm/flexrm-desc.h >> create mode 100644 drivers/mailbox/mailbox-flexrm/flexrm-main.c >> > I would avoid platform specific sub-directory if possible, because > people tend to take it as their fenced island where they can do > whatever they like. Sure, I will remove driver directory and merge everything into one driver source file. > > Can you break this into an upper BRCM specific code (desc management) > and a lower purely mailbox driver (only sets up pointers/addresses to > create rings and track the flexrm's PC)? Is there a reason why you Sure, I will organize merged driver source file as-per your suggestion > must dma_map, the buffers to send, in the spinlock protected > .send_data()? The dma_map() happens only for brcm_message with SPU2 message type. The SPU2 crypto mailbox client driver will just prepare src and dst scatterlist using the scatterlist passed by Linux crypto framework and pass it to mailbox controller driver (i.e. FlexRM driver or PDC driver). The pages in src and dst scatterlist will not be DMA mapped hence we first dma_map() scatterlist and then create descriptors using DMA addresses. I am fine in doing dma_map() outside spinlock protected .send_data() for SPU2 message type but this will impact three drivers: 1. BCM SPU2 Crypto driver (already merged) 2. BCM PDC mailbox driver (already merged) 3. BCM FlexRM mailbox driver (this patch) If required we should have separate patch to move dma_map() out of .send_data() because it impacts three different drivers. Regards, Anup