Received: by 2002:a05:6a10:af89:0:0:0:0 with SMTP id iu9csp4004038pxb; Tue, 25 Jan 2022 01:08:21 -0800 (PST) X-Google-Smtp-Source: ABdhPJx59BkV4wzv/Tnm2IRA68c7NbAbzkZCcw345llQe3HiPO5hBZPUH20O4yNHGEGCqvO/iXec X-Received: by 2002:a17:902:ecca:b0:14b:4bef:a2c7 with SMTP id a10-20020a170902ecca00b0014b4befa2c7mr8916136plh.25.1643101701622; Tue, 25 Jan 2022 01:08:21 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1643101701; cv=none; d=google.com; s=arc-20160816; b=eASOS8xWf0wKJDoV4MppMDEVBOg4Y6U/Neg1LxzRat5WpW9Rmfjpk+G8T6u9LTQwXl APUzPZcbRJ0ZKyBE659dxUQ5zIvnMCl+LJ6E5BrmBR7KArE0GuAMlvhXR+/fltgM5cph +pdMQv1Y7NJ91DIHT9IUOxg0AlTUkBqpzAvcvQaLvc/q85pBLymgnNITCHmkeERiXsPc MuyQ0RLvJaZ1rORz++YnZVSQvP0InkF2ishrltcI/JtoTkXLrTdduSo3pEX1ifTrCCm5 DMMyrU0OgpdYb/fCGJkqrhhMS2T3lBS+9PGw5ULVX1RwMhjDcaD6QOxCGw4GSJl8t3pP eT9A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=GxSSyNnOqzCbDA/E9stdLnXQVVTk0qCNicklKSphmTE=; b=Cs0VBdXQ9LdemZjXb3V4hdRNgur3ZOrJqaekw/HmIdbzPz2qRazjwTnnJCd8zXcM7c YHZCcJLL14AmYOtRQyF2xeS2HFFsOoiG6DeKsw1rbofL4TMuujzw47yRrf01gFHmj/eP db10eiJcFBTqrBkbccIleuih/3x+CC41F15xKCxdpZkBcNh0rt599uUOq9Nde0CpeO1X VwAJwvPUDsw6n75r67z5Pk4RhMiyd4rQfb+MFtHl62+q0ihHTPrkZV0o/fXHf/x0OGzp lqjRY2BLAGJ0Ix7o0YMIIgedKofd6XqjIgseLQstvlsMrjgBPrsa/zTCUk12JpUigJtm isxA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sifive.com header.s=google header.b=XEpVz6JC; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id j64si8639908pge.828.2022.01.25.01.08.09; Tue, 25 Jan 2022 01:08:21 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@sifive.com header.s=google header.b=XEpVz6JC; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1359681AbiAYGtl (ORCPT + 99 others); Tue, 25 Jan 2022 01:49:41 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41664 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1358811AbiAYGq2 (ORCPT ); Tue, 25 Jan 2022 01:46:28 -0500 Received: from mail-lj1-x233.google.com (mail-lj1-x233.google.com [IPv6:2a00:1450:4864:20::233]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 70828C02B76C for ; Mon, 24 Jan 2022 21:08:31 -0800 (PST) Received: by mail-lj1-x233.google.com with SMTP id t14so13509199ljh.8 for ; Mon, 24 Jan 2022 21:08:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=GxSSyNnOqzCbDA/E9stdLnXQVVTk0qCNicklKSphmTE=; b=XEpVz6JCzRlwvzrVHSlio1hJXN0I7Jo6RUS5WdJB14stes+pFX/TT0467/drOAtV4F Fafx04YchRFLS+NrdBHCAVYSrZeyfGWQJgO4yCFWZ7TNS9nRbfOEXhHc0p8bLu2j1nGx DGm/hPyrHFSepZnu3ufjhHKobDXEsFymq0ySxjPE+KB27IFzhaSyGizFo5pl1Etijd0i 0rWi31Vci8bBF2yteqOmvuMbBsxqQQm7x6M6rLK3Q1uvUO41150UWdvM2ZTZlf2m/5oH iYRqc5B2RKbZYDT2Qmx+L78jmyd89HwnG+Dkzh+HrKzMJRoZ8tp5y6Pa4i29N916vZwE xvAw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=GxSSyNnOqzCbDA/E9stdLnXQVVTk0qCNicklKSphmTE=; b=hJvj07wrGGvVWbkYECpQEYFlDrIt3CyAXtFAwJgZ7tGFbkL2HCP+Tcoq5Martem42D U4TudAGt2P1Ir+6igfmLET9cfFEPTGWztAaamk52Yh15yt8F51xZ2IWG1xpfIvHscM9m MXt32gUjuyoJEzfZzTX/FuSl1LaheTHRef3nVcy/kwl+cW6bGLlpsgtiJGpx2kq8WSdz OWr/0cO7tiX8geF88vSW0ogwSby0LdkOjxdEh1BHzzanMrw0Z0sWrc1V01Oi4XvhhEom dqZv1fbUKPXr/R6Wre/udr49Mhv8f3RIHqL18pnHIbXxKZWmYwtdWqx5XDftDYkr9+nW YGMw== X-Gm-Message-State: AOAM5321Ds2gn/9fxYZJ6u0FrNVSmyw/er5aptc4spp9VeKPRyhkRm76 i5nW2lZUt/Y3P19l68jS4TkP0EQOUjwgQQTrbkje2w== X-Received: by 2002:a2e:3604:: with SMTP id d4mr13188020lja.52.1643087309734; Mon, 24 Jan 2022 21:08:29 -0800 (PST) MIME-Version: 1.0 References: <0d0b0a3ad703f5ef50611e2dd80439675bda666a.1642383007.git.zong.li@sifive.com> In-Reply-To: From: Zong Li Date: Tue, 25 Jan 2022 13:08:18 +0800 Message-ID: Subject: Re: [PATCH v4 3/3] dmaengine: sf-pdma: Get number of channel by device tree To: Geert Uytterhoeven Cc: Palmer Dabbelt , Rob Herring , Paul Walmsley , Albert Ou , Krzysztof Kozlowski , Conor Dooley , Bin Meng , Green Wan , Vinod , dmaengine , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , "linux-kernel@vger.kernel.org List" , linux-riscv Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jan 21, 2022 at 6:29 PM Zong Li wrote: > > On Fri, Jan 21, 2022 at 4:33 PM Geert Uytterhoeven wrote: > > > > Hi Zong, Palmer, > > > > On Fri, Jan 21, 2022 at 3:21 AM Zong Li wrote: > > > On Fri, Jan 21, 2022 at 2:52 AM Palmer Dabbelt wrote: > > > > On Sun, 16 Jan 2022 17:35:28 PST (-0800), zong.li@sifive.com wrote: > > > > > It currently assumes that there are always four channels, it would > > > > > cause the error if there is actually less than four channels. Change > > > > > that by getting number of channel from device tree. > > > > > > > > > > For backwards-compatible, it uses the default value (i.e. 4) when there > > > > > is no 'dma-channels' information in dts. > > > > > > > > Some of the same wording issues here as those I pointed out in the DT > > > > bindings patch. > > > > > > > > > Signed-off-by: Zong Li > > > > > > > --- a/drivers/dma/sf-pdma/sf-pdma.c > > > > > +++ b/drivers/dma/sf-pdma/sf-pdma.c > > > > > @@ -482,9 +482,7 @@ static void sf_pdma_setup_chans(struct sf_pdma *pdma) > > > > > static int sf_pdma_probe(struct platform_device *pdev) > > > > > { > > > > > struct sf_pdma *pdma; > > > > > - struct sf_pdma_chan *chan; > > > > > struct resource *res; > > > > > - int len, chans; > > > > > int ret; > > > > > const enum dma_slave_buswidth widths = > > > > > DMA_SLAVE_BUSWIDTH_1_BYTE | DMA_SLAVE_BUSWIDTH_2_BYTES | > > > > > @@ -492,13 +490,21 @@ static int sf_pdma_probe(struct platform_device *pdev) > > > > > DMA_SLAVE_BUSWIDTH_16_BYTES | DMA_SLAVE_BUSWIDTH_32_BYTES | > > > > > DMA_SLAVE_BUSWIDTH_64_BYTES; > > > > > > > > > > - chans = PDMA_NR_CH; > > > > > - len = sizeof(*pdma) + sizeof(*chan) * chans; > > > > > - pdma = devm_kzalloc(&pdev->dev, len, GFP_KERNEL); > > > > > + pdma = devm_kzalloc(&pdev->dev, sizeof(*pdma), GFP_KERNEL); > > > > > if (!pdma) > > > > > return -ENOMEM; > > > > > > > > > > - pdma->n_chans = chans; > > > > > + ret = of_property_read_u32(pdev->dev.of_node, "dma-channels", > > > > > + &pdma->n_chans); > > > > > + if (ret) { > > > > > + dev_notice(&pdev->dev, "set number of channels to default value: 4\n"); > > > > > + pdma->n_chans = PDMA_MAX_NR_CH; > > > > > + } > > > > > + > > > > > + if (pdma->n_chans > PDMA_MAX_NR_CH) { > > > > > + dev_err(&pdev->dev, "the number of channels exceeds the maximum\n"); > > > > > + return -EINVAL; > > > > > > > > Can we get away with just using only the number of channels the driver > > > > actually supports? ie, just never sending an op to the channels above > > > > MAX_NR_CH? That should leave us with nothing to track. > > > > In theory we can... > > > > > It might be a bit like when pdma->n_chans is bigger than the maximum, > > > set the pdma->chans to PDMA_MAX_NR_CH, then we could ensure that we > > > don't access the channels above the maximum. If I understand > > > correctly, I gave the similar thought in the thread of v2 patch, and > > > there are some discussions on that, but this way seems to lead to > > > hard-to-track problems. > > > > ... but that would mean that when a new variant appears that supports > > more channels, no error is printed, and people might not notice > > immediately that the higher channels are never used. > > > > I guess people might need to follow the dt-bindings, so they couldn't > specify the number of channels to the value which is more than > maximum. But as you mentioned, if people don't notice that and specify > it more than maximum, they wouldn't be aware that the higher channels > are never used. It seems to me that we could keep returning the error > there, or show a warning message and use PDMA_MAX_NR_CH in that > situation, both looks good to me. > Hi all, thank you for the review, I'd like to prepare the next version patch, if current implementation of this part is ok to you, I will keep it in the next version. Please let me know if anything can be improved. Thanks > > Gr{oetje,eeting}s, > > > > Geert > > > > -- > > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > > > In personal conversations with technical people, I call myself a hacker. But > > when I'm talking to journalists I just say "programmer" or something like that. > > -- Linus Torvalds