From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3DB2DC433EF for ; Sun, 17 Oct 2021 12:04:07 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 02FAF6124F for ; Sun, 17 Oct 2021 12:04:06 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 02FAF6124F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=rosenzweig.io Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=bViQyN2hNSPUAAuaKy/d9LzhXO3//RqXkFgRnvW3dlU=; b=hWH8F4f9yH2sZf Hp/kfz5F3ssFXbsgQqtQzjz1U7Si+45VttMCUmFY5wb9zYURcoGi7NQLQu0iTV8UMLhF6k94G6PdC W8klDAcQNMD0oI6AD43gCJqDhdUgTUOn9Dm+auOAfsRlO0liHVxmXcF38QiEwbeuMoS2ODW/KdbyQ MEGmo/P+7pUi9Ploj5V5Kd48BEojGCKHg76o80b+JphTOmCC6JTLVnvzFhKw3JhAmomVZFz8l4dld pJGy7N4z4m+WGHXYNkrsfRJQyJfejKR2zZq/QbipPiAcT2lBBTnw7d8yAp+EGasj/a2u3wxBHEa4Y TO/fDH9uxMkGiAhWPoyQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mc4sR-00CLUd-6H; Sun, 17 Oct 2021 12:02:55 +0000 Received: from rosenzweig.io ([138.197.143.207]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mc4sM-00CLTG-LG for linux-arm-kernel@lists.infradead.org; Sun, 17 Oct 2021 12:02:52 +0000 Date: Sun, 17 Oct 2021 08:02:39 -0400 From: Alyssa Rosenzweig To: Sven Peter Cc: Jassi Brar , Rob Herring , Mark Kettenis , Hector Martin , Mohamed Mediouni , Stan Skowronek , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 2/2] mailbox: apple: Add driver for Apple mailboxes Message-ID: References: <20211017114054.67737-1-sven@svenpeter.dev> <20211017114054.67737-3-sven@svenpeter.dev> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20211017114054.67737-3-sven@svenpeter.dev> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211017_050250_779970_F561FDDF X-CRM114-Status: GOOD ( 15.51 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org > Apple SoCs such as the M1 come with various co-processors. Mailboxes > are used to communicate with those. This driver adds support for > two variants of those mailboxes. > > Reviewed-by: Alyssa Rosenzweig > Signed-off-by: Sven Peter In the future, Reviewed-by tags should be dropped after making major changes to a patch. > + writel_relaxed(apple_mbox->hw->irq_bit_recv_not_empty | > + apple_mbox->hw->irq_bit_send_empty, > + apple_mbox->regs + apple_mbox->hw->irq_enable); Nit: weird wrapping, much easier to read as: + writel_relaxed(apple_mbox->hw->irq_bit_recv_not_empty | + apple_mbox->hw->irq_bit_send_empty, + apple_mbox->regs + apple_mbox->hw->irq_enable); > +static const struct apple_mbox_hw apple_mbox_asc_hw = { > + .control_full = APPLE_ASC_MBOX_CONTROL_FULL, > + .control_empty = APPLE_ASC_MBOX_CONTROL_EMPTY, > + > + .a2i_control = APPLE_ASC_MBOX_A2I_CONTROL, > + .a2i_send0 = APPLE_ASC_MBOX_A2I_SEND0, > + .a2i_send1 = APPLE_ASC_MBOX_A2I_SEND1, > + > + .i2a_control = APPLE_ASC_MBOX_I2A_CONTROL, > + .i2a_recv0 = APPLE_ASC_MBOX_I2A_RECV0, > + .i2a_recv1 = APPLE_ASC_MBOX_I2A_RECV1, > + > + .has_irq_controls = false, > +}; Nit: consider dropping the `has_irq_controls = false` assignment. Clearly there are none, or you'd have to fill out the irq_* fields too. > +static const struct of_device_id apple_mbox_of_match[] = { > + { .compatible = "apple,t8103-asc-mailbox", .data = &apple_mbox_asc_hw }, > + { .compatible = "apple,t8103-m3-mailbox", .data = &apple_mbox_m3_hw }, > + {} > +}; No generic compatibles? I assume this driver hasn't changed much in recent iPhones, and hopefully it won't change much in M1X... > +/* SPDX-License-Identifier: GPL-2.0-only OR MIT */ > +/* > + * Apple mailbox message format > + * > + * Copyright (C) 2021 The Asahi Linux Contributors > + */ > + > +#ifndef _LINUX_APPLE_MAILBOX_H_ > +#define _LINUX_APPLE_MAILBOX_H_ > + > +#include > + > +struct apple_mbox_msg { > + u64 msg0; > + u32 msg1; > +}; > + > +#endif Given this file lacks the context of the driver, and the questions raised in v2 review, it might be beneficial to add a quick comment to apple_mbox_msg explaiing that no, really, this is a 96-bit message. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel