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 X-Spam-Level: X-Spam-Status: No, score=-17.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0CD59C2B9F4 for ; Mon, 28 Jun 2021 09:17:33 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2AEA8619AB for ; Mon, 28 Jun 2021 09:17:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2AEA8619AB Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=ti.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 8114C82D3E; Mon, 28 Jun 2021 11:17:29 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=quarantine dis=none) header.from=ti.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=ti.com header.i=@ti.com header.b="xbrymWGv"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id CCEE882D7F; Mon, 28 Jun 2021 11:17:27 +0200 (CEST) Received: from fllv0015.ext.ti.com (fllv0015.ext.ti.com [198.47.19.141]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id EFA2082D22 for ; Mon, 28 Jun 2021 11:17:23 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=quarantine dis=none) header.from=ti.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=p.yadav@ti.com Received: from lelv0265.itg.ti.com ([10.180.67.224]) by fllv0015.ext.ti.com (8.15.2/8.15.2) with ESMTP id 15S9HL9X064519; Mon, 28 Jun 2021 04:17:21 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1624871841; bh=FDxoimwfT/5x+dlavuqXETywROVRKnRx1LjFWpsw++U=; h=Date:From:To:CC:Subject:References:In-Reply-To; b=xbrymWGvj7STTm0QkwuZM/Bmn+CgW3WB9UAo3ZgMHOr3WHye6Z4GPhpiufwrmVyiE Eayqo6lreKh+G50ZO0fcVywtDIYSQZwKFDHBXAKWqKS6c5ZRzaQpEZ+FMG1l8QdUE8 NJpepd2zYTt0MYzaef1Z15wCRWzFPJKpgYEItwzo= Received: from DLEE114.ent.ti.com (dlee114.ent.ti.com [157.170.170.25]) by lelv0265.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 15S9HL2x049743 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Mon, 28 Jun 2021 04:17:21 -0500 Received: from DLEE109.ent.ti.com (157.170.170.41) by DLEE114.ent.ti.com (157.170.170.25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2176.2; Mon, 28 Jun 2021 04:17:21 -0500 Received: from lelv0327.itg.ti.com (10.180.67.183) by DLEE109.ent.ti.com (157.170.170.41) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2176.2 via Frontend Transport; Mon, 28 Jun 2021 04:17:21 -0500 Received: from localhost (ileax41-snat.itg.ti.com [10.172.224.153]) by lelv0327.itg.ti.com (8.15.2/8.15.2) with ESMTP id 15S9HK4p045944; Mon, 28 Jun 2021 04:17:20 -0500 Date: Mon, 28 Jun 2021 14:47:19 +0530 From: Pratyush Yadav To: Jagan Teki CC: Vignesh R , Ryder Lee , Weijie Gao , Chunfeng Yun , GSS_MTK_Uboot_upstream , U-Boot-Denx Subject: Re: [PATCH v10 04/27] spi: spi-mem: add spi_mem_dtr_supports_op() Message-ID: <20210628091717.k24iqf6o4ggkughi@ti.com> References: <20210625191729.31798-1-p.yadav@ti.com> <20210625191729.31798-5-p.yadav@ti.com> <20210627083513.azqvm5wydhuwfygv@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20171215 X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean On 28/06/21 12:09PM, Jagan Teki wrote: > On Sun, Jun 27, 2021 at 2:05 PM Pratyush Yadav wrote: > > > > On 26/06/21 02:44PM, Jagan Teki wrote: > > > On Sat, Jun 26, 2021 at 12:47 AM Pratyush Yadav wrote: > > > > > > > > spi_mem_default_supports_op() rejects DTR ops by default to ensure that > > > > the controller drivers that haven't been updated with DTR support > > > > continue to reject them. It also makes sure that controllers that don't > > > > support DTR mode at all (which is most of them at the moment) also > > > > reject them. > > > > > > > > This means that controller drivers that want to support DTR mode can't > > > > use spi_mem_default_supports_op(). Driver authors have to roll their own > > > > supports_op() function and mimic the buswidth checks. Or even worse, > > > > driver authors might skip it completely or get it wrong. > > > > > > > > Add spi_mem_dtr_supports_op(). It provides a basic sanity check for DTR > > > > ops and performs the buswidth requirement check. Move the logic for > > > > checking buswidth in spi_mem_default_supports_op() to a separate > > > > function so the logic is not repeated twice. > > > > > > > > Signed-off-by: Pratyush Yadav > > > > --- > > > > drivers/spi/spi-mem.c | 32 +++++++++++++++++++++++++++++--- > > > > include/spi-mem.h | 2 ++ > > > > 2 files changed, 31 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c > > > > index 541cd0e5a7..9c1ede1b61 100644 > > > > --- a/drivers/spi/spi-mem.c > > > > +++ b/drivers/spi/spi-mem.c > > > > @@ -145,8 +145,8 @@ static int spi_check_buswidth_req(struct spi_slave *slave, u8 buswidth, bool tx) > > > > return -ENOTSUPP; > > > > } > > > > > > > > -bool spi_mem_default_supports_op(struct spi_slave *slave, > > > > - const struct spi_mem_op *op) > > > > +static bool spi_mem_check_buswidth(struct spi_slave *slave, > > > > + const struct spi_mem_op *op) > > > > { > > > > if (spi_check_buswidth_req(slave, op->cmd.buswidth, true)) > > > > return false; > > > > @@ -164,13 +164,39 @@ bool spi_mem_default_supports_op(struct spi_slave *slave, > > > > op->data.dir == SPI_MEM_DATA_OUT)) > > > > return false; > > > > > > > > + return true; > > > > +} > > > > + > > > > +bool spi_mem_dtr_supports_op(struct spi_slave *slave, > > > > + const struct spi_mem_op *op) > > > > +{ > > > > + if (op->cmd.buswidth == 8 && op->cmd.nbytes % 2) > > > > + return false; > > > > + > > > > + if (op->addr.nbytes && op->addr.buswidth == 8 && op->addr.nbytes % 2) > > > > + return false; > > > > + > > > > + if (op->dummy.nbytes && op->dummy.buswidth == 8 && op->dummy.nbytes % 2) > > > > + return false; > > > > + > > > > + if (op->data.dir != SPI_MEM_NO_DATA && > > > > + op->dummy.buswidth == 8 && op->data.nbytes % 2) > > > > + return false; > > > > + > > > > + return spi_mem_check_buswidth(slave, op); > > > > +} > > > > +EXPORT_SYMBOL_GPL(spi_mem_dtr_supports_op); > > > > > > Does this export_symbol is used in U-Boot? > > > > It is being used in drivers ported from Linux. I see it being used in > > spi-mem.c, mtdcore.c, x530_cert_parser.c, nand/core.c, etc. It is > > defined to an empty macro in include/linux/compat.h. > > Ok, thanks for the details. Somehow we need to refactor the code, I > believe it would require lot of testing but possible. Sorry, I don't quite understand what code you want to refactor. Do you mean the code in this series? Can you be more specific on what you think needs refactoring? Or do you mean a refactor of spi-mem? -- Regards, Pratyush Yadav Texas Instruments Inc.