Message ID | 20221021095833.62406-7-vivek.2311@samsung.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4242:0:0:0:0:0 with SMTP id s2csp674916wrr; Fri, 21 Oct 2022 05:41:16 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5V38sRh4dYA2Gy4/anF5ksXw51+wR/GrW9lUahP65ujNJrFoaLx8RBDYiRkhKaLvJznOHG X-Received: by 2002:a17:90a:ab90:b0:210:27cb:e337 with SMTP id n16-20020a17090aab9000b0021027cbe337mr14034223pjq.139.1666356075949; Fri, 21 Oct 2022 05:41:15 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666356075; cv=none; d=google.com; s=arc-20160816; b=d1q3LysgpQSJLZVGx5spBApwKnmKA0pF+ovoYgICBBdeFYHfjD1Q5KdAGNy5RZlVvt 6yjS2V3QQ0Ej4RzcO8xc1EalykGqlWhGSi4ck8lfr7SK3LR38DBsWmix7FMINVJO1hvU 9gm6od9kRasgUcC6VRsw/w6bmyBdKPHsfoBUvAuJ5zFTg4VRpHxtxjr7IzNSTz1s0zGT 2r2t4JgJK5fEwxpB3Oe9Wpha/w9zAYbdKb83qnS9FGbDg/YDCwKL2H/w2UHxcPXKMdeT 4mqKSlCy7SEBdlCt2QKKA1Rs90jWokuJe/Suf5b7P7HFdh3Q32dd8aeUlc/etSQYcpD3 QvyA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:references:dlp-filter:cms-type:in-reply-to :message-id:date:subject:cc:to:from:dkim-signature:dkim-filter; bh=NXxJL4i1oJOsidFiGnrxru8pCRE5y8E/9Ytlga4MYSc=; b=ZTXDhT6tkWCLQXTQRoEvBr/Qba9luMG92p/c2mwsH2mLiMqoTJCXQXkQZ/dz7wjI/2 KBvxmi5ItGI38lqmeHrhxLx0B0d93eFS1uin4Uk/mq4AO+xXUR6f/DBqMZT2nbVWQtpk t3yeH7GV9SBR7bbsXXhHETQ2Fmr1RO5NrRcn18xikFIZjoMuOjQkbGvVjtbh+Ny2/xBK 3xkw0SkmEs7j8UwjL5L8ED5qI5kaOJVnWj5DRzlMnX9AW1BV6oBY6cBtSBlQocKZBh6J DLOuP3iazRnRl+v6NcmSQ/Ffjq1Kp8fRKjQgWaXPiVqpWZMJwUDx5fMBvQ09C/HnZ4dY eUmA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@samsung.com header.s=mail20170921 header.b=lQDZUd12; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=samsung.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id l4-20020a170903244400b001867ea919bdsi1369183pls.25.2022.10.21.05.41.02; Fri, 21 Oct 2022 05:41:15 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@samsung.com header.s=mail20170921 header.b=lQDZUd12; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=samsung.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229631AbiJUMjH (ORCPT <rfc822;pwkd43@gmail.com> + 99 others); Fri, 21 Oct 2022 08:39:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39148 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230141AbiJUMir (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 21 Oct 2022 08:38:47 -0400 Received: from mailout3.samsung.com (mailout3.samsung.com [203.254.224.33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 93DEB266409 for <linux-kernel@vger.kernel.org>; Fri, 21 Oct 2022 05:38:34 -0700 (PDT) Received: from epcas5p3.samsung.com (unknown [182.195.41.41]) by mailout3.samsung.com (KnoxPortal) with ESMTP id 20221021123832epoutp0360705ce9b56ac3ca4d243ff5c14d18b7~gFd_gcT8l0743307433epoutp03e for <linux-kernel@vger.kernel.org>; Fri, 21 Oct 2022 12:38:32 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout3.samsung.com 20221021123832epoutp0360705ce9b56ac3ca4d243ff5c14d18b7~gFd_gcT8l0743307433epoutp03e DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1666355912; bh=NXxJL4i1oJOsidFiGnrxru8pCRE5y8E/9Ytlga4MYSc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lQDZUd12eWk6vTDPf8HXw+mzI22+ZgnWW+DEwpZ/ZiK1e/NeSuXYdcJ8J1bu55rxH PFv4Pkl6koS2eepktrbzsescXByOuCfsiOnXrvvkWmTBUfiN/iTQiP76DYZ3TCeb8N 3qeQjGoyRC121tbLixOw30AVgRBPciaTJvIuvxow= Received: from epsnrtp1.localdomain (unknown [182.195.42.162]) by epcas5p4.samsung.com (KnoxPortal) with ESMTP id 20221021123831epcas5p442f741c8f51a075806223d8bbddad6f6~gFd92Bjsw2106521065epcas5p4N; Fri, 21 Oct 2022 12:38:31 +0000 (GMT) Received: from epsmges5p2new.samsung.com (unknown [182.195.38.180]) by epsnrtp1.localdomain (Postfix) with ESMTP id 4Mv3sx73jDz4x9Pr; Fri, 21 Oct 2022 12:38:29 +0000 (GMT) Received: from epcas5p3.samsung.com ( [182.195.41.41]) by epsmges5p2new.samsung.com (Symantec Messaging Gateway) with SMTP id 5B.BB.39477.5C292536; Fri, 21 Oct 2022 21:38:29 +0900 (KST) Received: from epsmtrp1.samsung.com (unknown [182.195.40.13]) by epcas5p2.samsung.com (KnoxPortal) with ESMTPA id 20221021102639epcas5p2241192d3ac873d1262372eeae948b401~gDq0q4jKo3051630516epcas5p29; Fri, 21 Oct 2022 10:26:39 +0000 (GMT) Received: from epsmgms1p2.samsung.com (unknown [182.195.42.42]) by epsmtrp1.samsung.com (KnoxPortal) with ESMTP id 20221021102639epsmtrp1a542600b6a14ba6b394916f35c125727~gDq0qIMLG1811718117epsmtrp12; Fri, 21 Oct 2022 10:26:39 +0000 (GMT) X-AuditID: b6c32a4a-007ff70000019a35-6b-635292c5818f Received: from epsmtip1.samsung.com ( [182.195.34.30]) by epsmgms1p2.samsung.com (Symantec Messaging Gateway) with SMTP id 15.40.18644.ED372536; Fri, 21 Oct 2022 19:26:38 +0900 (KST) Received: from cheetah.sa.corp.samsungelectronics.net (unknown [107.109.115.53]) by epsmtip1.samsung.com (KnoxPortal) with ESMTPA id 20221021102636epsmtip191a748e0c4b4c379ba62a644a9afbf8d~gDqyvVPHR2574025740epsmtip1B; Fri, 21 Oct 2022 10:26:36 +0000 (GMT) From: Vivek Yadav <vivek.2311@samsung.com> To: rcsekar@samsung.com, wg@grandegger.com, mkl@pengutronix.de, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, pankaj.dubey@samsung.com, ravi.patel@samsung.com, alim.akhtar@samsung.com Cc: linux-can@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Vivek Yadav <vivek.2311@samsung.com> Subject: [PATCH 6/7] can: m_can: Add ECC functionality for message RAM Date: Fri, 21 Oct 2022 15:28:32 +0530 Message-Id: <20221021095833.62406-7-vivek.2311@samsung.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20221021095833.62406-1-vivek.2311@samsung.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupjk+LIzCtJLcpLzFFi42LZdlhTU/fopKBkg0uvTSwezNvGZjHnfAuL xdNjj9gtLmzrY7VY9X0qs8XlXXPYLNYvmsJicWyBmMW3028YLRZt/cJu8fDDHnaLWRd2sFr8 WniYxWLpvZ2sDnweW1beZPJYsKnU4+Ol24wem1Z1snn0/zXweL/vKptH35ZVjB6fN8kFcERl 22SkJqakFimk5iXnp2TmpdsqeQfHO8ebmhkY6hpaWpgrKeQl5qbaKrn4BOi6ZeYAXa2kUJaY UwoUCkgsLlbSt7Mpyi8tSVXIyC8usVVKLUjJKTAp0CtOzC0uzUvXy0stsTI0MDAyBSpMyM74 Ofcde8FtrYpTW08xNjCeVepi5OSQEDCROHD2D2MXIxeHkMBuRonW7a/YIJxPjBJ3L+xmhnA+ M0qc+LyeFablVO91FojELkaJR3uusEM4rUwSixcfYQepYhPQknjcuYAFxBYRuMsocW1xZhcj BwezQLXEgSN8IGFhATeJX0cuMYLYLAKqEs+WbWQCsXkFrCVu/FjPCLFMXmL1hgPMIDangI3E q/lbwG6VEJjIIfFj8Xw2iCIXic9LV7BA2MISr45vYYewpSRe9rdB2ckSO/51Qn2QIbFg4h6o BfYSB67MYYG4TVNi/S59iLCsxNRT68DuYRbgk+j9/YQJIs4rsWMejK0i8eLzBFaQVpBVveeE IcIeEue334YGST+jxLJj15gnMMrNQtiwgJFxFaNkakFxbnpqsWmBUV5qOTzSkvNzNzGCE6aW 1w7Ghw8+6B1iZOJgPMQowcGsJMJrUReULMSbklhZlVqUH19UmpNafIjRFBh+E5mlRJPzgSk7 ryTe0MTSwMTMzMzE0tjMUEmcd/EMrWQhgfTEktTs1NSC1CKYPiYOTqkGpl18S//dO/htffUf rz085xukDlw8YlUaK3Pv6kxVxwetLtUx/lp2t6IEMk5OtrvoFzH135oNEV47hF2n7e67da88 66f06hcL/qmvvzxh82sl49Ua/PFZqzSmrHtw6tOZ2sJpBX92zf+05Di7y5653NPWdqif28ek sbZslp2cUfSUSbOWH59cvO4Di8TGjt7X3Yb3d2lt7Ox8f7ho1R7B29lf7q2IP3JqcWiQ+ML0 NZ3P6xNrtP7MuqA4+eHH1X8dpke4x3J+qT8d1cJRwH9tlVh7zJLXj1c8E/30VLiw8cLDXtF1 M5tL3Q4VPV/pe/N5QFyemLL+D9MvrhOZ+x9f/LbsfOB6610ZauzHjmxepR/1R4mlOCPRUIu5 qDgRAHE/ufYhBAAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrGLMWRmVeSWpSXmKPExsWy7bCSnO694qBkgxcPLCwezNvGZjHnfAuL xdNjj9gtLmzrY7VY9X0qs8XlXXPYLNYvmsJicWyBmMW3028YLRZt/cJu8fDDHnaLWRd2sFr8 WniYxWLpvZ2sDnweW1beZPJYsKnU4+Ol24wem1Z1snn0/zXweL/vKptH35ZVjB6fN8kFcERx 2aSk5mSWpRbp2yVwZfyc+4694LZWxamtpxgbGM8qdTFyckgImEic6r3O0sXIxSEksINRYuf6 PawQCSmJKWdeskDYwhIr/z1nhyhqZpK4PmsjWBGbgJbE484FYEUiAi8ZJVrOsoHYzAL1Eu/O 3GQHsYUF3CR+HbnECGKzCKhKPFu2kQnE5hWwlrjxYz0jxAJ5idUbDjCD2JwCNhKv5m8BiwsB 1Sx7dJN9AiPfAkaGVYySqQXFuem5xYYFRnmp5XrFibnFpXnpesn5uZsYwUGtpbWDcc+qD3qH GJk4GA8xSnAwK4nwFrwLSBbiTUmsrEotyo8vKs1JLT7EKM3BoiTOe6HrZLyQQHpiSWp2ampB ahFMlomDU6qBaZXhfJbg31l7TTfZhjzu7doWfGRTA8+tz2VdR69xH58efrEizN3zWqjmZY/J 7AYHDsV/3PNXU6rxwEzhA/mPS1+se5H1Iyb9wfHnov/2WdYHvNraJX/QTk0s+mdGUUb9J9sL fO8cVK9PObDqp+C9/J/9/asefboqv4SpY8ayWbzpHj8zr8Y9S2yXCNGa+s67cfeTSJOaEM7b y/fEik1fx+a08IzdHseXbnZyH2QKpXxf6izV9s9ntEv03j/1hf3VhWf+3N7hEXTuaJxLz1HX Fbd3Ol282ytqzLFTc7r83WiLKPMDNhsVfvVPibjOIsu/d0tywNTLK/dXZK9o+Z+uuSqRx5f5 n+rJHxWKjRO7jyuxFGckGmoxFxUnAgDeavGK2QIAAA== X-CMS-MailID: 20221021102639epcas5p2241192d3ac873d1262372eeae948b401 X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-Sendblock-Type: REQ_APPROVE CMS-TYPE: 105P DLP-Filter: Pass X-CFilter-Loop: Reflected X-CMS-RootMailID: 20221021102639epcas5p2241192d3ac873d1262372eeae948b401 References: <20221021095833.62406-1-vivek.2311@samsung.com> <CGME20221021102639epcas5p2241192d3ac873d1262372eeae948b401@epcas5p2.samsung.com> X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747300988413386551?= X-GMAIL-MSGID: =?utf-8?q?1747300988413386551?= |
Series |
can: mcan: Add MCAN support for FSD SoC
|
|
Commit Message
Vivek Yadav
Oct. 21, 2022, 9:58 a.m. UTC
Whenever MCAN Buffers and FIFOs are stored on message ram, there are
inherent risks of corruption known as single-bit errors.
Enable error correction code (ECC) data intigrity check for Message RAM
to create valid ECC checksums.
ECC uses a respective number of bits, which are added to each word as a
parity and that will raise the error signal on the corruption in the
Interrupt Register(IR).
Message RAM bit error controlled by input signal m_can_aeim_berr[0],
generated by an optional external parity / ECC logic attached to the
Message RAM.
This indicates either Bit Error detected and Corrected(BEC) or No bit
error detected when reading from Message RAM.
Signed-off-by: Vivek Yadav <vivek.2311@samsung.com>
---
drivers/net/can/m_can/m_can.c | 73 +++++++++++++++++++++++++++++++++++
drivers/net/can/m_can/m_can.h | 24 ++++++++++++
2 files changed, 97 insertions(+)
Comments
Hi Vivek, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on robh/for-next] [also build test WARNING on linus/master v6.1-rc1 next-20221021] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Vivek-Yadav/dt-bindings-Document-the-SYSREG-specific-compatibles-found-on-FSD-SoC/20221021-204010 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next patch link: https://lore.kernel.org/r/20221021095833.62406-7-vivek.2311%40samsung.com patch subject: [PATCH 6/7] can: m_can: Add ECC functionality for message RAM config: m68k-allyesconfig compiler: m68k-linux-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/05f01ceda6f70e1942c5530e04637da412b914da git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Vivek-Yadav/dt-bindings-Document-the-SYSREG-specific-compatibles-found-on-FSD-SoC/20221021-204010 git checkout 05f01ceda6f70e1942c5530e04637da412b914da # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/net/can/m_can/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> drivers/net/can/m_can/m_can.c:1538:5: warning: no previous prototype for 'm_can_config_mram_ecc_check' [-Wmissing-prototypes] 1538 | int m_can_config_mram_ecc_check(struct m_can_classdev *cdev, int enable, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ vim +/m_can_config_mram_ecc_check +1538 drivers/net/can/m_can/m_can.c 1537 > 1538 int m_can_config_mram_ecc_check(struct m_can_classdev *cdev, int enable, 1539 struct device_node *np) 1540 { 1541 int val = 0; 1542 int offset = 0, ret = 0; 1543 int delay_count = MRAM_INIT_TIMEOUT; 1544 struct m_can_mraminit *mraminit = &cdev->mraminit_sys; 1545 1546 mraminit->syscon = syscon_regmap_lookup_by_phandle(np, 1547 "mram-ecc-cfg"); 1548 if (IS_ERR(mraminit->syscon)) { 1549 /* can fail with -EPROBE_DEFER */ 1550 ret = PTR_ERR(mraminit->syscon); 1551 return ret; 1552 } 1553 1554 if (of_property_read_u32_index(np, "mram-ecc-cfg", 1, 1555 &mraminit->reg)) { 1556 dev_err(cdev->dev, "couldn't get the mraminit reg. offset!\n"); 1557 return -ENODEV; 1558 } 1559 1560 val = ((MRAM_ECC_ENABLE_MASK | MRAM_CFG_VALID_MASK | 1561 MRAM_INIT_ENABLE_MASK) << offset); 1562 regmap_clear_bits(mraminit->syscon, mraminit->reg, val); 1563 1564 if (enable) { 1565 val = (MRAM_ECC_ENABLE_MASK | MRAM_INIT_ENABLE_MASK) << offset; 1566 regmap_set_bits(mraminit->syscon, mraminit->reg, val); 1567 } 1568 1569 /* after enable or disable valid flag need to be set*/ 1570 val = (MRAM_CFG_VALID_MASK << offset); 1571 regmap_set_bits(mraminit->syscon, mraminit->reg, val); 1572 1573 if (enable) { 1574 do { 1575 regmap_read(mraminit->syscon, mraminit->reg, &val); 1576 1577 if (val & (MRAM_INIT_DONE_MASK << offset)) 1578 return 0; 1579 1580 udelay(1); 1581 } while (delay_count--); 1582 1583 return -ENODEV; 1584 } 1585 1586 return 0; 1587 } 1588
On 21.10.2022 15:28:32, Vivek Yadav wrote: > Whenever MCAN Buffers and FIFOs are stored on message ram, there are RAM > inherent risks of corruption known as single-bit errors. > > Enable error correction code (ECC) data intigrity check for Message RAM > to create valid ECC checksums. > > ECC uses a respective number of bits, which are added to each word as a > parity and that will raise the error signal on the corruption in the > Interrupt Register(IR). > > Message RAM bit error controlled by input signal m_can_aeim_berr[0], > generated by an optional external parity / ECC logic attached to the > Message RAM. > > This indicates either Bit Error detected and Corrected(BEC) or No bit > error detected when reading from Message RAM. There is no ECC error handler added to the code. > Signed-off-by: Vivek Yadav <vivek.2311@samsung.com> > --- > drivers/net/can/m_can/m_can.c | 73 +++++++++++++++++++++++++++++++++++ > drivers/net/can/m_can/m_can.h | 24 ++++++++++++ > 2 files changed, 97 insertions(+) > > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c > index dcb582563d5e..578146707d5b 100644 > --- a/drivers/net/can/m_can/m_can.c > +++ b/drivers/net/can/m_can/m_can.c > @@ -1535,9 +1535,62 @@ static void m_can_stop(struct net_device *dev) > cdev->can.state = CAN_STATE_STOPPED; > } > > +int m_can_config_mram_ecc_check(struct m_can_classdev *cdev, int enable, static ^^^ bool > + struct device_node *np) > +{ > + int val = 0; > + int offset = 0, ret = 0; > + int delay_count = MRAM_INIT_TIMEOUT; > + struct m_can_mraminit *mraminit = &cdev->mraminit_sys; Please sort by reverse Christmas tree. > + > + mraminit->syscon = syscon_regmap_lookup_by_phandle(np, > + "mram-ecc-cfg"); Please check if syscon_regmap_lookup_by_phandle_args() is better suited. You probably want to do the syscon lookup once during m_can_class_register(). > + if (IS_ERR(mraminit->syscon)) { > + /* can fail with -EPROBE_DEFER */ m_can_config_mram_ecc_check() is called from m_can_open() and m_can_close(), returning -EPROBE_DEFER makes no sense there. > + ret = PTR_ERR(mraminit->syscon); > + return ret; > + } > + > + if (of_property_read_u32_index(np, "mram-ecc-cfg", 1, > + &mraminit->reg)) { > + dev_err(cdev->dev, "couldn't get the mraminit reg. offset!\n"); > + return -ENODEV; > + } > + > + val = ((MRAM_ECC_ENABLE_MASK | MRAM_CFG_VALID_MASK | > + MRAM_INIT_ENABLE_MASK) << offset); please make use of FIELD_PREP > + regmap_clear_bits(mraminit->syscon, mraminit->reg, val); > + > + if (enable) { > + val = (MRAM_ECC_ENABLE_MASK | MRAM_INIT_ENABLE_MASK) << offset; same here > + regmap_set_bits(mraminit->syscon, mraminit->reg, val); > + } > + > + /* after enable or disable valid flag need to be set*/ ^^^ missing space > + val = (MRAM_CFG_VALID_MASK << offset); here, too > + regmap_set_bits(mraminit->syscon, mraminit->reg, val); > + > + if (enable) { > + do { > + regmap_read(mraminit->syscon, mraminit->reg, &val); > + > + if (val & (MRAM_INIT_DONE_MASK << offset)) > + return 0; > + > + udelay(1); > + } while (delay_count--); please make use of regmap_read_poll_timeout(). > + > + return -ENODEV; > + } > + > + return 0; > +} > + > static int m_can_close(struct net_device *dev) > { > struct m_can_classdev *cdev = netdev_priv(dev); > + struct device_node *np; > + int err = 0; > > netif_stop_queue(dev); > > @@ -1557,6 +1610,15 @@ static int m_can_close(struct net_device *dev) > if (cdev->is_peripheral) > can_rx_offload_disable(&cdev->offload); > > + np = cdev->dev->of_node; > + > + if (np && of_property_read_bool(np, "mram-ecc-cfg")) { > + err = m_can_config_mram_ecc_check(cdev, ECC_DISABLE, np); > + if (err < 0) > + netdev_err(dev, > + "Message RAM ECC disable config failed\n"); > + } > + > close_candev(dev); > > phy_power_off(cdev->transceiver); > @@ -1754,6 +1816,7 @@ static int m_can_open(struct net_device *dev) > { > struct m_can_classdev *cdev = netdev_priv(dev); > int err; > + struct device_node *np; > > err = phy_power_on(cdev->transceiver); > if (err) > @@ -1798,6 +1861,16 @@ static int m_can_open(struct net_device *dev) > goto exit_irq_fail; > } > > + np = cdev->dev->of_node; > + > + if (np && of_property_read_bool(np, "mram-ecc-cfg")) { > + err = m_can_config_mram_ecc_check(cdev, ECC_ENABLE, np); > + if (err < 0) { > + netdev_err(dev, > + "Message RAM ECC enable config failed\n"); > + } > + } > + > /* start the m_can controller */ > m_can_start(dev); > > diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h > index 4c0267f9f297..3cbfdc64a7db 100644 > --- a/drivers/net/can/m_can/m_can.h > +++ b/drivers/net/can/m_can/m_can.h > @@ -28,6 +28,8 @@ > #include <linux/can/dev.h> > #include <linux/pinctrl/consumer.h> > #include <linux/phy/phy.h> > +#include <linux/mfd/syscon.h> > +#include <linux/regmap.h> please make a separate patch that sorts these includes alphabetically, then add the new includes sorted. > > /* m_can lec values */ > enum m_can_lec_type { > @@ -52,12 +54,33 @@ enum m_can_mram_cfg { > MRAM_CFG_NUM, > }; > > +enum m_can_ecc_cfg { > + ECC_DISABLE = 0, > + ECC_ENABLE, > +}; > + unused > /* address offset and element number for each FIFO/Buffer in the Message RAM */ > struct mram_cfg { > u16 off; > u8 num; > }; > > +/* MRAM_INIT_BITS */ > +#define MRAM_CFG_VALID_SHIFT 5 > +#define MRAM_CFG_VALID_MASK BIT(MRAM_CFG_VALID_SHIFT) > +#define MRAM_ECC_ENABLE_SHIFT 3 > +#define MRAM_ECC_ENABLE_MASK BIT(MRAM_ECC_ENABLE_SHIFT) > +#define MRAM_INIT_ENABLE_SHIFT 1 > +#define MRAM_INIT_ENABLE_MASK BIT(MRAM_INIT_ENABLE_SHIFT) > +#define MRAM_INIT_DONE_SHIFT 0 > +#define MRAM_INIT_DONE_MASK BIT(MRAM_INIT_DONE_SHIFT) > +#define MRAM_INIT_TIMEOUT 50 Please move these bits to the m_can.c file. Add a common prefix M_CAN_ to them. Remove the SHIFT values, directly define the MASK using BIT() (for single set bits) or GEN_MASK() (for multiple set bits). > + > +struct m_can_mraminit { Is this RAM init or ECC related? > + struct regmap *syscon; /* for mraminit ctrl. reg. access */ > + unsigned int reg; /* register index within syscon */ > +}; > + > struct m_can_classdev; > struct m_can_ops { > /* Device specific call backs */ > @@ -92,6 +115,7 @@ struct m_can_classdev { > int pm_clock_support; > int is_peripheral; > > + struct m_can_mraminit mraminit_sys; /* mraminit via syscon regmap */ > struct mram_cfg mcfg[MRAM_CFG_NUM]; > }; > > -- > 2.17.1 > > Marc
> -----Original Message----- > From: Marc Kleine-Budde <mkl@pengutronix.de> > Sent: 25 October 2022 13:46 > To: Vivek Yadav <vivek.2311@samsung.com> > Cc: rcsekar@samsung.com; wg@grandegger.com; davem@davemloft.net; > edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; > pankaj.dubey@samsung.com; ravi.patel@samsung.com; > alim.akhtar@samsung.com; linux-can@vger.kernel.org; > netdev@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 6/7] can: m_can: Add ECC functionality for message RAM > > On 21.10.2022 15:28:32, Vivek Yadav wrote: > > Whenever MCAN Buffers and FIFOs are stored on message ram, there are > RAM > > inherent risks of corruption known as single-bit errors. > > > > Enable error correction code (ECC) data intigrity check for Message > > RAM to create valid ECC checksums. > > > > ECC uses a respective number of bits, which are added to each word as > > a parity and that will raise the error signal on the corruption in the > > Interrupt Register(IR). > > > > Message RAM bit error controlled by input signal m_can_aeim_berr[0], > > generated by an optional external parity / ECC logic attached to the > > Message RAM. > > I will remove this text from commit as this indicates the handling of ECC error. > > This indicates either Bit Error detected and Corrected(BEC) or No bit > > error detected when reading from Message RAM. > > There is no ECC error handler added to the code. > Yes, we are not adding the ECC error handler in the code. As per my understanding this is already handled in <m_can_handle_other_err> function. > > Signed-off-by: Vivek Yadav <vivek.2311@samsung.com> > > --- > > drivers/net/can/m_can/m_can.c | 73 > > +++++++++++++++++++++++++++++++++++ > > drivers/net/can/m_can/m_can.h | 24 ++++++++++++ > > 2 files changed, 97 insertions(+) > > > > diff --git a/drivers/net/can/m_can/m_can.c > > b/drivers/net/can/m_can/m_can.c index dcb582563d5e..578146707d5b > > 100644 > > --- a/drivers/net/can/m_can/m_can.c > > +++ b/drivers/net/can/m_can/m_can.c > > @@ -1535,9 +1535,62 @@ static void m_can_stop(struct net_device *dev) > > cdev->can.state = CAN_STATE_STOPPED; } > > > > +int m_can_config_mram_ecc_check(struct m_can_classdev *cdev, int > > +enable, > static ^^^ bool > > + struct device_node *np) > > +{ > > + int val = 0; > > + int offset = 0, ret = 0; > > + int delay_count = MRAM_INIT_TIMEOUT; > > + struct m_can_mraminit *mraminit = &cdev->mraminit_sys; > > Please sort by reverse Christmas tree. > Okay, I will address this in next patch series. > > + > > + mraminit->syscon = syscon_regmap_lookup_by_phandle(np, > > + "mram-ecc-cfg"); > > Please check if syscon_regmap_lookup_by_phandle_args() is better suited. > Okay, I will check and make it better. > You probably want to do the syscon lookup once during > m_can_class_register(). > Yes, It should be once only, I will address this. > > + if (IS_ERR(mraminit->syscon)) { > > + /* can fail with -EPROBE_DEFER */ > > m_can_config_mram_ecc_check() is called from m_can_open() and > m_can_close(), returning -EPROBE_DEFER makes no sense there. > > > + ret = PTR_ERR(mraminit->syscon); > > + return ret; > > + } > > + > > + if (of_property_read_u32_index(np, "mram-ecc-cfg", 1, > > + &mraminit->reg)) { > > + dev_err(cdev->dev, "couldn't get the mraminit reg. > offset!\n"); > > + return -ENODEV; > > + } > > + > > + val = ((MRAM_ECC_ENABLE_MASK | MRAM_CFG_VALID_MASK | > > + MRAM_INIT_ENABLE_MASK) << offset); > > please make use of FIELD_PREP > Okay, I will do. > > + regmap_clear_bits(mraminit->syscon, mraminit->reg, val); > > + > > + if (enable) { > > + val = (MRAM_ECC_ENABLE_MASK | > MRAM_INIT_ENABLE_MASK) << offset; > > same here > okay > > + regmap_set_bits(mraminit->syscon, mraminit->reg, val); > > + } > > + > > + /* after enable or disable valid flag need to be set*/ > ^^^ > missing space > > + val = (MRAM_CFG_VALID_MASK << offset); > > here, too > okay > > + regmap_set_bits(mraminit->syscon, mraminit->reg, val); > > + > > + if (enable) { > > + do { > > + regmap_read(mraminit->syscon, mraminit->reg, > &val); > > + > > + if (val & (MRAM_INIT_DONE_MASK << offset)) > > + return 0; > > + > > + udelay(1); > > + } while (delay_count--); > > please make use of regmap_read_poll_timeout(). > Okay, I will add this in next patch series. > > + > > + return -ENODEV; > > + } > > + > > + return 0; > > +} > > + > > static int m_can_close(struct net_device *dev) { > > struct m_can_classdev *cdev = netdev_priv(dev); > > + struct device_node *np; > > + int err = 0; > > > > netif_stop_queue(dev); > > > > @@ -1557,6 +1610,15 @@ static int m_can_close(struct net_device *dev) > > if (cdev->is_peripheral) > > can_rx_offload_disable(&cdev->offload); > > > > + np = cdev->dev->of_node; > > + > > + if (np && of_property_read_bool(np, "mram-ecc-cfg")) { > > + err = m_can_config_mram_ecc_check(cdev, ECC_DISABLE, > np); > > + if (err < 0) > > + netdev_err(dev, > > + "Message RAM ECC disable config > failed\n"); > > + } > > + > > close_candev(dev); > > > > phy_power_off(cdev->transceiver); > > @@ -1754,6 +1816,7 @@ static int m_can_open(struct net_device *dev) { > > struct m_can_classdev *cdev = netdev_priv(dev); > > int err; > > + struct device_node *np; > > > > err = phy_power_on(cdev->transceiver); > > if (err) > > @@ -1798,6 +1861,16 @@ static int m_can_open(struct net_device *dev) > > goto exit_irq_fail; > > } > > > > + np = cdev->dev->of_node; > > + > > + if (np && of_property_read_bool(np, "mram-ecc-cfg")) { > > + err = m_can_config_mram_ecc_check(cdev, ECC_ENABLE, > np); > > + if (err < 0) { > > + netdev_err(dev, > > + "Message RAM ECC enable config > failed\n"); > > + } > > + } > > + > > /* start the m_can controller */ > > m_can_start(dev); > > > > diff --git a/drivers/net/can/m_can/m_can.h > > b/drivers/net/can/m_can/m_can.h index 4c0267f9f297..3cbfdc64a7db > > 100644 > > --- a/drivers/net/can/m_can/m_can.h > > +++ b/drivers/net/can/m_can/m_can.h > > @@ -28,6 +28,8 @@ > > #include <linux/can/dev.h> > > #include <linux/pinctrl/consumer.h> > > #include <linux/phy/phy.h> > > +#include <linux/mfd/syscon.h> > > +#include <linux/regmap.h> > > please make a separate patch that sorts these includes alphabetically, then > add the new includes sorted. > Okay, I will post the separate patch for this. > > > > /* m_can lec values */ > > enum m_can_lec_type { > > @@ -52,12 +54,33 @@ enum m_can_mram_cfg { > > MRAM_CFG_NUM, > > }; > > > > +enum m_can_ecc_cfg { > > + ECC_DISABLE = 0, > > + ECC_ENABLE, > > +}; > > + > > unused > Okay, I will remove or make better use of this. > > /* address offset and element number for each FIFO/Buffer in the > > Message RAM */ struct mram_cfg { > > u16 off; > > u8 num; > > }; > > > > +/* MRAM_INIT_BITS */ > > +#define MRAM_CFG_VALID_SHIFT 5 > > +#define MRAM_CFG_VALID_MASK BIT(MRAM_CFG_VALID_SHIFT) > > +#define MRAM_ECC_ENABLE_SHIFT 3 > > +#define MRAM_ECC_ENABLE_MASK BIT(MRAM_ECC_ENABLE_SHIFT) > > +#define MRAM_INIT_ENABLE_SHIFT 1 > > +#define MRAM_INIT_ENABLE_MASK BIT(MRAM_INIT_ENABLE_SHIFT) > > +#define MRAM_INIT_DONE_SHIFT 0 > > +#define MRAM_INIT_DONE_MASK BIT(MRAM_INIT_DONE_SHIFT) > > +#define MRAM_INIT_TIMEOUT 50 > > Please move these bits to the m_can.c file. > Okay, I will move. > Add a common prefix M_CAN_ to them. > Okay, I will address this in next patch series. > Remove the SHIFT values, directly define the MASK using BIT() (for single set > bits) or GEN_MASK() (for multiple set bits). > > > + > > +struct m_can_mraminit { > > Is this RAM init or ECC related? > This is for ECC only, I will give better naming for this for better understanding. > > + struct regmap *syscon; /* for mraminit ctrl. reg. access */ > > + unsigned int reg; /* register index within syscon */ > > +}; > > + > > struct m_can_classdev; > > struct m_can_ops { > > /* Device specific call backs */ > > @@ -92,6 +115,7 @@ struct m_can_classdev { > > int pm_clock_support; > > int is_peripheral; > > > > + struct m_can_mraminit mraminit_sys; /* mraminit via syscon > regmap */ > > struct mram_cfg mcfg[MRAM_CFG_NUM]; > > }; > > > > -- > > 2.17.1 > > > > > > Marc > Thank you for your feedback and reviewing the patches. > -- > Pengutronix e.K. | Marc Kleine-Budde | > Embedded Linux | > https://protect2.fireeye.com/v1/url?k=7f1e79b1-1e956c87-7f1ff2fe- > 74fe485cbff1-92aa04a06e5e6383&q=1&e=543e935e-4838-4692-b1da- > d42741c9ad3f&u=https%3A%2F%2Fwww.pengutronix.de%2F | > Vertretung West/Dortmund | Phone: +49-231-2826-924 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c index dcb582563d5e..578146707d5b 100644 --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -1535,9 +1535,62 @@ static void m_can_stop(struct net_device *dev) cdev->can.state = CAN_STATE_STOPPED; } +int m_can_config_mram_ecc_check(struct m_can_classdev *cdev, int enable, + struct device_node *np) +{ + int val = 0; + int offset = 0, ret = 0; + int delay_count = MRAM_INIT_TIMEOUT; + struct m_can_mraminit *mraminit = &cdev->mraminit_sys; + + mraminit->syscon = syscon_regmap_lookup_by_phandle(np, + "mram-ecc-cfg"); + if (IS_ERR(mraminit->syscon)) { + /* can fail with -EPROBE_DEFER */ + ret = PTR_ERR(mraminit->syscon); + return ret; + } + + if (of_property_read_u32_index(np, "mram-ecc-cfg", 1, + &mraminit->reg)) { + dev_err(cdev->dev, "couldn't get the mraminit reg. offset!\n"); + return -ENODEV; + } + + val = ((MRAM_ECC_ENABLE_MASK | MRAM_CFG_VALID_MASK | + MRAM_INIT_ENABLE_MASK) << offset); + regmap_clear_bits(mraminit->syscon, mraminit->reg, val); + + if (enable) { + val = (MRAM_ECC_ENABLE_MASK | MRAM_INIT_ENABLE_MASK) << offset; + regmap_set_bits(mraminit->syscon, mraminit->reg, val); + } + + /* after enable or disable valid flag need to be set*/ + val = (MRAM_CFG_VALID_MASK << offset); + regmap_set_bits(mraminit->syscon, mraminit->reg, val); + + if (enable) { + do { + regmap_read(mraminit->syscon, mraminit->reg, &val); + + if (val & (MRAM_INIT_DONE_MASK << offset)) + return 0; + + udelay(1); + } while (delay_count--); + + return -ENODEV; + } + + return 0; +} + static int m_can_close(struct net_device *dev) { struct m_can_classdev *cdev = netdev_priv(dev); + struct device_node *np; + int err = 0; netif_stop_queue(dev); @@ -1557,6 +1610,15 @@ static int m_can_close(struct net_device *dev) if (cdev->is_peripheral) can_rx_offload_disable(&cdev->offload); + np = cdev->dev->of_node; + + if (np && of_property_read_bool(np, "mram-ecc-cfg")) { + err = m_can_config_mram_ecc_check(cdev, ECC_DISABLE, np); + if (err < 0) + netdev_err(dev, + "Message RAM ECC disable config failed\n"); + } + close_candev(dev); phy_power_off(cdev->transceiver); @@ -1754,6 +1816,7 @@ static int m_can_open(struct net_device *dev) { struct m_can_classdev *cdev = netdev_priv(dev); int err; + struct device_node *np; err = phy_power_on(cdev->transceiver); if (err) @@ -1798,6 +1861,16 @@ static int m_can_open(struct net_device *dev) goto exit_irq_fail; } + np = cdev->dev->of_node; + + if (np && of_property_read_bool(np, "mram-ecc-cfg")) { + err = m_can_config_mram_ecc_check(cdev, ECC_ENABLE, np); + if (err < 0) { + netdev_err(dev, + "Message RAM ECC enable config failed\n"); + } + } + /* start the m_can controller */ m_can_start(dev); diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h index 4c0267f9f297..3cbfdc64a7db 100644 --- a/drivers/net/can/m_can/m_can.h +++ b/drivers/net/can/m_can/m_can.h @@ -28,6 +28,8 @@ #include <linux/can/dev.h> #include <linux/pinctrl/consumer.h> #include <linux/phy/phy.h> +#include <linux/mfd/syscon.h> +#include <linux/regmap.h> /* m_can lec values */ enum m_can_lec_type { @@ -52,12 +54,33 @@ enum m_can_mram_cfg { MRAM_CFG_NUM, }; +enum m_can_ecc_cfg { + ECC_DISABLE = 0, + ECC_ENABLE, +}; + /* address offset and element number for each FIFO/Buffer in the Message RAM */ struct mram_cfg { u16 off; u8 num; }; +/* MRAM_INIT_BITS */ +#define MRAM_CFG_VALID_SHIFT 5 +#define MRAM_CFG_VALID_MASK BIT(MRAM_CFG_VALID_SHIFT) +#define MRAM_ECC_ENABLE_SHIFT 3 +#define MRAM_ECC_ENABLE_MASK BIT(MRAM_ECC_ENABLE_SHIFT) +#define MRAM_INIT_ENABLE_SHIFT 1 +#define MRAM_INIT_ENABLE_MASK BIT(MRAM_INIT_ENABLE_SHIFT) +#define MRAM_INIT_DONE_SHIFT 0 +#define MRAM_INIT_DONE_MASK BIT(MRAM_INIT_DONE_SHIFT) +#define MRAM_INIT_TIMEOUT 50 + +struct m_can_mraminit { + struct regmap *syscon; /* for mraminit ctrl. reg. access */ + unsigned int reg; /* register index within syscon */ +}; + struct m_can_classdev; struct m_can_ops { /* Device specific call backs */ @@ -92,6 +115,7 @@ struct m_can_classdev { int pm_clock_support; int is_peripheral; + struct m_can_mraminit mraminit_sys; /* mraminit via syscon regmap */ struct mram_cfg mcfg[MRAM_CFG_NUM]; };