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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 55697C54791 for ; Wed, 13 Mar 2024 14:49:29 +0000 (UTC) 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:Date:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Q8DxWhdK8aMRNS1J5yFL0VffJj4QPuPibiKpafmuUgw=; b=LYr5RVE19f1opm AECzBmsx7JoZkLzkKJqjoUlcCwfMkJ0NVNmbH6jajoQ714MXJNJNTZiOgU0DTE4EN11scjnFVi7RB LGo8QDiQRrUogKZpy3iw5Pz1mVJs5Vgz9cg2QynPsvi1MiwB76HYTWNWN4/azWX6QyeYEF5fxcA9q 7q2D8rbhKjMjoIBx9nGqgW4npl9PEsg85h2IOAtr1aABnX2TfuQpOQZiQufNjCKMM0Gi/Dz6iLvb7 eht1DhY4x+/SDPE5/73qmITAmJJbqoz8tbsj45lf9x9yc0ktaryaH7hi2sap1HPriO8hP+uV2GvrP 7EWp6J/frzfuSAd5TFUw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rkPuq-0000000ATw3-1bx8; Wed, 13 Mar 2024 14:49:12 +0000 Received: from mail-ej1-x631.google.com ([2a00:1450:4864:20::631]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rkPum-0000000ATtH-0hHV for linux-arm-kernel@lists.infradead.org; Wed, 13 Mar 2024 14:49:11 +0000 Received: by mail-ej1-x631.google.com with SMTP id a640c23a62f3a-a45606c8444so823113466b.3 for ; Wed, 13 Mar 2024 07:49:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1710341346; x=1710946146; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:date:from:from:to:cc :subject:date:message-id:reply-to; bh=e3eQum80NNnCkG6phw9dHYDaS2ANjJ+Hfdu++iY8NGk=; b=LTw5EjAYkpaGo0m7D2sPGjoJ3YlAEiveNFfvKiI3qugbTcu8DA5198i+YXhfolQYia VrcZSxVU/Z3qK4CR0Ob0o7xBSZzQcnDrJIoCzy38pC3fkUV5RwgAnpIyYxWTmjC5aaV1 agGB45cR5I3TZ69zO9HOvhu4RGZFc74O+ymi/daBi25hVlyMfB6DQKDE64J5y49TRjun /SPx3k8+s520gvvUFbzHizZ8kQ8HVehCuh1WE3RJHoBATUB1K5HMxtD/s9f054UfSymq 2Fv48rvASd1PLIj/eRxXLAvg2TCv0mKVa2vD4uFqUkfi0Ratt6QHAcqbkltWhzJ0y9c/ P4Dw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710341346; x=1710946146; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:date:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=e3eQum80NNnCkG6phw9dHYDaS2ANjJ+Hfdu++iY8NGk=; b=hyn2VgTh3DeXIERx2paci5PiP31qWU1yeT57mPrPB9jy7Idcwn7F/0LsGAw9egAnPn muU4dzGz4BfiCNGrVTQ6jNwqKRNxjY/CVOO1n97QJOaqh/339Mojk28Umazx5v/U5zt1 6KsXUjY1XGXBbzrHLDEnljVt6a3GlYiNvuun86inoSqHTSN6848UIMDoNASL0vIPrdQt EL6YR3NJQ7W3tS+jb22+Or4dseyx80wHW+MwTPc+c0We1B9BrEZ4Z2BSQ0r8WyHNyeRy Rhw0eBrLadC+wdUbShUpAmTNY9GL6pqxV+OTtzn8Wb/OMvB5FeHNBDiSV15JiVyXkiIi SZ/A== X-Forwarded-Encrypted: i=1; AJvYcCXhjpNvJvi+ZsO/qOK3BRAR0Fp4nSWQ9NiCht88RFA2yUworzliVV8AYwJlTvPc0UwBRZOFq5uQEbOoaNNTuvzAnR20BYZFXItGx9MHBjcNH6ORb4A= X-Gm-Message-State: AOJu0Yy/YrPwt1woEggi7xtKKNx4Wf2AhBn7Y3FElIE3RKaqX0gO9hWA D9E3UN7mTIYh5tEqDet/bBhZG6ITQezHKpAEMuHGMT1f6aWBnMhbIjjDW1T7Zgw= X-Google-Smtp-Source: AGHT+IGoZ5fJmL+zK19ZDe5D2rupsNcSyxqwyd2LWie0z6AU4dn5RVQinylPT0ATlRNZsuyAtUx0KQ== X-Received: by 2002:a17:907:7b06:b0:a46:5b72:6d38 with SMTP id mn6-20020a1709077b0600b00a465b726d38mr1168654ejc.63.1710341345777; Wed, 13 Mar 2024 07:49:05 -0700 (PDT) Received: from localhost (host-82-56-173-172.retail.telecomitalia.it. [82.56.173.172]) by smtp.gmail.com with ESMTPSA id bh21-20020a170906a0d500b00a44efa48c24sm4933249ejb.117.2024.03.13.07.49.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Mar 2024 07:49:05 -0700 (PDT) From: Andrea della Porta X-Google-Original-From: Andrea della Porta Date: Wed, 13 Mar 2024 15:49:04 +0100 To: Dave Stevenson Cc: Andrea della Porta , Vinod Koul , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Florian Fainelli , Ray Jui , Scott Branden , Broadcom internal kernel review list , Saenz Julienne , dmaengine@vger.kernel.org, devicetree@vger.kernel.org, linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Stefan Wahren Subject: Re: [PATCH v2 12/15] dmaengine: bcm2835: introduce multi platform support Message-ID: Mail-Followup-To: Dave Stevenson , Andrea della Porta , Vinod Koul , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Florian Fainelli , Ray Jui , Scott Branden , Broadcom internal kernel review list , Saenz Julienne , dmaengine@vger.kernel.org, devicetree@vger.kernel.org, linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Stefan Wahren References: <5826eba6ab78b9cdba21c12853a85d5f9a6aab76.1710226514.git.andrea.porta@suse.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240313_074908_232274_295CE86F X-CRM114-Status: GOOD ( 33.41 ) 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 On 17:05 Tue 12 Mar , Dave Stevenson wrote: > Hi Andrea > > On Tue, 12 Mar 2024 at 09:13, Andrea della Porta wrote: > > > > This finally moves all platform specific stuff into a separate structure, > > which is initialized on the OF compatible during probing. Since the DMA > > control block is different on the BCM2711 platform, we introduce a common > > control block to reserve the necessary space and adequate methods for > > access. > > > > Signed-off-by: Stefan Wahren > > Signed-off-by: Andrea della Porta > > --- > > drivers/dma/bcm2835-dma.c | 336 +++++++++++++++++++++++++++++--------- > > 1 file changed, 260 insertions(+), 76 deletions(-) > > > > diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c > > index 88ae5d05402e..b015eae29b08 100644 > > --- a/drivers/dma/bcm2835-dma.c > > +++ b/drivers/dma/bcm2835-dma.c > > @@ -48,6 +48,11 @@ struct bcm2835_dmadev { > > struct dma_device ddev; > > void __iomem *base; > > dma_addr_t zero_page; > > + const struct bcm2835_dma_cfg *cfg; > > +}; > > + > > +struct bcm_dma_cb { > > + uint32_t rsvd[8]; > > }; > > > > struct bcm2835_dma_cb { > > @@ -61,7 +66,7 @@ struct bcm2835_dma_cb { > > }; > > > > struct bcm2835_cb_entry { > > - struct bcm2835_dma_cb *cb; > > + struct bcm_dma_cb *cb; > > dma_addr_t paddr; > > }; > > > > @@ -82,6 +87,44 @@ struct bcm2835_chan { > > bool is_lite_channel; > > }; > > > > +struct bcm2835_dma_cfg { > > + dma_addr_t addr_offset; > > + u32 cs_reg; > > + u32 cb_reg; > > + u32 next_reg; > > + u32 ti_reg; > > + > > + u32 wait_mask; > > + u32 reset_mask; > > + u32 int_mask; > > + u32 active_mask; > > + u32 abort_mask; > > + u32 s_dreq_mask; > > + u32 d_dreq_mask; > > + > > + u32 (*cb_get_length)(void *data); > > + dma_addr_t (*cb_get_addr)(void *data, enum dma_transfer_direction); > > + > > + void (*cb_init)(void *data, struct bcm2835_chan *c, > > + enum dma_transfer_direction, u32 src, u32 dst, > > + bool zero_page); > > + void (*cb_set_src)(void *data, enum dma_transfer_direction, u32 src); > > + void (*cb_set_dst)(void *data, enum dma_transfer_direction, u32 dst); > > + void (*cb_set_next)(void *data, u32 next); > > + void (*cb_set_length)(void *data, u32 length); > > + void (*cb_append_extra)(void *data, > > + struct bcm2835_chan *c, > > + enum dma_transfer_direction direction, > > + bool cyclic, bool final, unsigned long flags); > > + > > + dma_addr_t (*to_cb_addr)(dma_addr_t addr); > > + > > + void (*chan_plat_init)(struct bcm2835_chan *c); > > + dma_addr_t (*read_addr)(struct bcm2835_chan *c, > > + enum dma_transfer_direction); > > + u32 (*cs_flags)(struct bcm2835_chan *c); > > +}; > > + > > struct bcm2835_desc { > > struct bcm2835_chan *c; > > struct virt_dma_desc vd; > > @@ -215,6 +258,13 @@ static inline struct bcm2835_dmadev *to_bcm2835_dma_dev(struct dma_device *d) > > return container_of(d, struct bcm2835_dmadev, ddev); > > } > > > > +static inline const struct bcm2835_dma_cfg *to_bcm2835_cfg(struct dma_device *d) > > +{ > > + struct bcm2835_dmadev *od = container_of(d, struct bcm2835_dmadev, ddev); > > + > > + return od->cfg; > > +} > > + > > static inline struct bcm2835_chan *to_bcm2835_dma_chan(struct dma_chan *c) > > { > > return container_of(c, struct bcm2835_chan, vc.chan); > > @@ -292,6 +342,109 @@ static inline bool need_dst_incr(enum dma_transfer_direction direction) > > return false; > > } > > > > +static inline u32 bcm2835_dma_cb_get_length(void *data) > > +{ > > + struct bcm2835_dma_cb *cb = data; > > + > > + return cb->length; > > +} > > + > > +static inline dma_addr_t > > +bcm2835_dma_cb_get_addr(void *data, enum dma_transfer_direction direction) > > +{ > > + struct bcm2835_dma_cb *cb = data; > > + > > + if (direction == DMA_DEV_TO_MEM) > > + return cb->dst; > > + > > + return cb->src; > > +} > > + > > +static inline void > > +bcm2835_dma_cb_init(void *data, struct bcm2835_chan *c, > > + enum dma_transfer_direction direction, u32 src, u32 dst, > > + bool zero_page) > > +{ > > + struct bcm2835_dma_cb *cb = data; > > + > > + cb->info = bcm2835_dma_prepare_cb_info(c, direction, zero_page); > > + cb->src = src; > > + cb->dst = dst; > > + cb->stride = 0; > > + cb->next = 0; > > +} > > + > > +static inline void > > +bcm2835_dma_cb_set_src(void *data, enum dma_transfer_direction direction, > > + u32 src) > > +{ > > + struct bcm2835_dma_cb *cb = data; > > + > > + cb->src = src; > > +} > > + > > +static inline void > > +bcm2835_dma_cb_set_dst(void *data, enum dma_transfer_direction direction, > > + u32 dst) > > +{ > > + struct bcm2835_dma_cb *cb = data; > > + > > + cb->dst = dst; > > +} > > + > > +static inline void bcm2835_dma_cb_set_next(void *data, u32 next) > > +{ > > + struct bcm2835_dma_cb *cb = data; > > + > > + cb->next = next; > > +} > > + > > +static inline void bcm2835_dma_cb_set_length(void *data, u32 length) > > +{ > > + struct bcm2835_dma_cb *cb = data; > > + > > + cb->length = length; > > +} > > + > > +static inline void > > +bcm2835_dma_cb_append_extra(void *data, struct bcm2835_chan *c, > > + enum dma_transfer_direction direction, > > + bool cyclic, bool final, unsigned long flags) > > +{ > > + struct bcm2835_dma_cb *cb = data; > > + > > + cb->info |= bcm2835_dma_prepare_cb_extra(c, direction, cyclic, final, > > + flags); > > +} > > + > > +static inline dma_addr_t bcm2835_dma_to_cb_addr(dma_addr_t addr) > > +{ > > + return addr; > > +} > > + > > +static void bcm2835_dma_chan_plat_init(struct bcm2835_chan *c) > > +{ > > + /* check in DEBUG register if this is a LITE channel */ > > + if (readl(c->chan_base + BCM2835_DMA_DEBUG) & BCM2835_DMA_DEBUG_LITE) > > + c->is_lite_channel = true; > > +} > > + > > +static dma_addr_t bcm2835_dma_read_addr(struct bcm2835_chan *c, > > + enum dma_transfer_direction direction) > > +{ > > + if (direction == DMA_MEM_TO_DEV) > > + return readl(c->chan_base + BCM2835_DMA_SOURCE_AD); > > + else if (direction == DMA_DEV_TO_MEM) > > + return readl(c->chan_base + BCM2835_DMA_DEST_AD); > > + > > + return 0; > > +} > > + > > +static u32 bcm2835_dma_cs_flags(struct bcm2835_chan *c) > > +{ > > + return BCM2835_DMA_CS_FLAGS(c->dreq); > > +} > > + > > static void bcm2835_dma_free_cb_chain(struct bcm2835_desc *desc) > > { > > size_t i; > > @@ -309,16 +462,19 @@ static void bcm2835_dma_desc_free(struct virt_dma_desc *vd) > > } > > > > static bool bcm2835_dma_create_cb_set_length(struct dma_chan *chan, > > - struct bcm2835_dma_cb *control_block, > > + void *data, > > size_t len, > > size_t period_len, > > size_t *total_len) > > { > > + const struct bcm2835_dma_cfg *cfg = to_bcm2835_cfg(chan->device); > > struct bcm2835_chan *c = to_bcm2835_dma_chan(chan); > > size_t max_len = bcm2835_dma_max_frame_length(c); > > > > /* set the length taking lite-channel limitations into account */ > > - control_block->length = min_t(u32, len, max_len); > > + u32 length = min_t(u32, len, max_len); > > + > > + cfg->cb_set_length(data, length); > > > > /* finished if we have no period_length */ > > if (!period_len) > > @@ -333,14 +489,14 @@ static bool bcm2835_dma_create_cb_set_length(struct dma_chan *chan, > > */ > > > > /* have we filled in period_length yet? */ > > - if (*total_len + control_block->length < period_len) { > > + if (*total_len + length < period_len) { > > /* update number of bytes in this period so far */ > > - *total_len += control_block->length; > > + *total_len += length; > > return false; > > } > > > > /* calculate the length that remains to reach period_length */ > > - control_block->length = period_len - *total_len; > > + cfg->cb_set_length(data, period_len - *total_len); > > > > /* reset total_length for next period */ > > *total_len = 0; > > @@ -388,15 +544,14 @@ static struct bcm2835_desc *bcm2835_dma_create_cb_chain( > > size_t buf_len, size_t period_len, > > gfp_t gfp, unsigned long flags) > > { > > + const struct bcm2835_dma_cfg *cfg = to_bcm2835_cfg(chan->device); > > struct bcm2835_dmadev *od = to_bcm2835_dma_dev(chan->device); > > struct bcm2835_chan *c = to_bcm2835_dma_chan(chan); > > size_t len = buf_len, total_len; > > size_t frame; > > struct bcm2835_desc *d; > > struct bcm2835_cb_entry *cb_entry; > > - struct bcm2835_dma_cb *control_block; > > - u32 extrainfo = bcm2835_dma_prepare_cb_extra(c, direction, cyclic, > > - false, flags); > > + struct bcm_dma_cb *control_block; > > bool zero_page = false; > > > > if (!frames) > > @@ -432,12 +587,7 @@ static struct bcm2835_desc *bcm2835_dma_create_cb_chain( > > > > /* fill in the control block */ > > control_block = cb_entry->cb; > > - control_block->info = bcm2835_dma_prepare_cb_info(c, direction, > > - zero_page); > > - control_block->src = src; > > - control_block->dst = dst; > > - control_block->stride = 0; > > - control_block->next = 0; > > + cfg->cb_init(control_block, c, src, dst, direction, zero_page); > > Can I ask how you've been testing these patches? Sure, most of these issues were easily spotted by mem to mem transactions, using dmatest. > > This line was one of the bugs that I found during my work. The > prototype for cb_init is > + void (*cb_init)(void *data, struct bcm2835_chan *c, > + enum dma_transfer_direction, u32 src, u32 dst, > + bool zero_page); > So this call has direction in the wrong place, which leads to quite > comical failures. Exactly, that was one of the funniest. This is indeed resolved in patch number 14. The others being related to address shifting in the 40 bit case, basically I've just replaced << 8 with << 32 on srci and dsti control block fields. Many thanks, Andrea > > Thanks > Dave > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel