From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Shevchenko Date: Thu, 31 Aug 2023 17:18:26 +0300 Subject: [PATCH v12 2/2] i2c: aspeed: support ast2600 i2c new register mode driver In-Reply-To: References: <20230714074522.23827-1-ryan_chen@aspeedtech.com> <20230714074522.23827-3-ryan_chen@aspeedtech.com> Message-ID: List-Id: To: linux-aspeed@lists.ozlabs.org MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Thu, Aug 31, 2023 at 06:04:30AM +0000, Ryan Chen wrote: > > On Fri, Jul 14, 2023 at 03:45:22PM +0800, Ryan Chen wrote: Stop overquoting! Remove the context you are not answering to. ... > > > + if (--i % 4 != 3) > > > + writel(*(u32 *)wbuf, i2c_bus->buf_base + i - (i % 4)); > > > + writel(AST2600_I2CC_SET_TX_BUF_LEN(xfer_len), > > > + i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL); > > > > Wrong memory accessors. You should use something from asm/byteorder.h > > which includes linux/byteorder/generic.h. > > > > Are you preferring add comment to explain more by following? > /* > * The controller's buffer register supports dword writes only. > * Therefore, write dwords to the buffer register in a 4-byte aligned, > * and write the remaining unaligned data at the end. > */ This does not explain endianess bug (or feature) it has. You are using CPU side byteorder for the aligned data. This is not okay, on top of the code looking ugly and prone to errors. Note, that somebody may refer to your code, once accepted, in educational purposes, but since the code is not good written, it makes a false positive impression that this is the right thing to do in the similar case elsewhere. Please, fix this. > for (i = 0; i < xfer_len; i++) { > wbuf[i % 4] = msg->buf[i2c_bus->master_xfer_cnt + i]; > /* accumulating 4 bytes of data, write as a Dword to the buffer register */ > if (i % 4 == 3) > writel(*(u32 *)wbuf, i2c_bus->buf_base + i - 3); > } > /* less than 4 bytes of remaining data, write the remaining part as a Dword */ > if (--i % 4 != 3) > writel(*(u32 *)wbuf, i2c_bus->buf_base + i - (i % 4)); > writel(AST2600_I2CC_SET_TX_BUF_LEN(xfer_len), > i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL); > > Or more columns (use get_unaligned_le32(wbuf); ) by following. > > for (i = 0; i < xfer_len; i++) { > wbuf[i % 4] = msg->buf[i2c_bus->master_xfer_cnt + i]; > if (i % 4 == 3) { > wbuf_dword = get_unaligned_le32(wbuf); > writel(wbuf_dword, i2c_bus->buf_base + i - 3); > } > } > > if (--i % 4 != 3) { > wbuf_dword = get_unaligned_le32(wbuf); > writel(wbuf_dword, i2c_bus->buf_base + i - (i % 4)); > } -- With Best Regards, Andy Shevchenko 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 23F30C83F10 for ; Thu, 31 Aug 2023 14:26:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232513AbjHaO0P (ORCPT ); Thu, 31 Aug 2023 10:26:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40980 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229834AbjHaO0P (ORCPT ); Thu, 31 Aug 2023 10:26:15 -0400 Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.126]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CB562A3; Thu, 31 Aug 2023 07:26:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1693491971; x=1725027971; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=7paPh2mF9Wy7OEwJY1d2GalGy2y7dQanY+hUyqVQRaU=; b=Y0D3U3kLlFmMGiM54D053w7T1ecUFVORDPL5WeK9AyStCG15zgnG0fNB zmBb8rnE5IiApn6cbxoU879KGzH89//1M9oWwL15ktMlv4btbXLT+pUl9 tqWkwX9pYv/qElFEmipI9F0o1TX7ikeJ6R4gNe8unHexmCJeTJ/ZzAqIO KiAz/2A/oCL59+Su5PNWhVMfrFK8eO5Q1gYEg0w4LK5rnWQh+QKKx/Ny8 yQKL7jwBnOo6yeBcCZlYfD217uw+jAd3Nl/0ZYcM6wp5XUgn58eHRv1WF XtDJy7VYEiwOHAczxY2qWN+RYouqHKz6CKrn19GiZc6bp2hQYH4oS9K0u Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10819"; a="360978690" X-IronPort-AV: E=Sophos;i="6.02,217,1688454000"; d="scan'208";a="360978690" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Aug 2023 07:18:39 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10819"; a="689370098" X-IronPort-AV: E=Sophos;i="6.02,217,1688454000"; d="scan'208";a="689370098" Received: from smile.fi.intel.com ([10.237.72.54]) by orsmga003.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Aug 2023 07:18:31 -0700 Received: from andy by smile.fi.intel.com with local (Exim 4.96) (envelope-from ) id 1qbiV9-005PI8-0Q; Thu, 31 Aug 2023 17:18:27 +0300 Date: Thu, 31 Aug 2023 17:18:26 +0300 From: Andy Shevchenko To: Ryan Chen Cc: "jk@codeconstruct.com.au" , Brendan Higgins , Benjamin Herrenschmidt , Joel Stanley , Rob Herring , Krzysztof Kozlowski , Andrew Jeffery , Philipp Zabel , Wolfram Sang , "linux-i2c@vger.kernel.org" , Florian Fainelli , Jean Delvare , William Zhang , Tyrone Ting , Tharun Kumar P , Conor Dooley , Phil Edworthy , "openbmc@lists.ozlabs.org" , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-aspeed@lists.ozlabs.org" , "=linux-kernel@vger.kernel.org" <=linux-kernel@vger.kernel.org>, Andi Shyti Subject: Re: [PATCH v12 2/2] i2c: aspeed: support ast2600 i2c new register mode driver Message-ID: References: <20230714074522.23827-1-ryan_chen@aspeedtech.com> <20230714074522.23827-3-ryan_chen@aspeedtech.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org On Thu, Aug 31, 2023 at 06:04:30AM +0000, Ryan Chen wrote: > > On Fri, Jul 14, 2023 at 03:45:22PM +0800, Ryan Chen wrote: Stop overquoting! Remove the context you are not answering to. ... > > > + if (--i % 4 != 3) > > > + writel(*(u32 *)wbuf, i2c_bus->buf_base + i - (i % 4)); > > > + writel(AST2600_I2CC_SET_TX_BUF_LEN(xfer_len), > > > + i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL); > > > > Wrong memory accessors. You should use something from asm/byteorder.h > > which includes linux/byteorder/generic.h. > > > > Are you preferring add comment to explain more by following? > /* > * The controller's buffer register supports dword writes only. > * Therefore, write dwords to the buffer register in a 4-byte aligned, > * and write the remaining unaligned data at the end. > */ This does not explain endianess bug (or feature) it has. You are using CPU side byteorder for the aligned data. This is not okay, on top of the code looking ugly and prone to errors. Note, that somebody may refer to your code, once accepted, in educational purposes, but since the code is not good written, it makes a false positive impression that this is the right thing to do in the similar case elsewhere. Please, fix this. > for (i = 0; i < xfer_len; i++) { > wbuf[i % 4] = msg->buf[i2c_bus->master_xfer_cnt + i]; > /* accumulating 4 bytes of data, write as a Dword to the buffer register */ > if (i % 4 == 3) > writel(*(u32 *)wbuf, i2c_bus->buf_base + i - 3); > } > /* less than 4 bytes of remaining data, write the remaining part as a Dword */ > if (--i % 4 != 3) > writel(*(u32 *)wbuf, i2c_bus->buf_base + i - (i % 4)); > writel(AST2600_I2CC_SET_TX_BUF_LEN(xfer_len), > i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL); > > Or more columns (use get_unaligned_le32(wbuf); ) by following. > > for (i = 0; i < xfer_len; i++) { > wbuf[i % 4] = msg->buf[i2c_bus->master_xfer_cnt + i]; > if (i % 4 == 3) { > wbuf_dword = get_unaligned_le32(wbuf); > writel(wbuf_dword, i2c_bus->buf_base + i - 3); > } > } > > if (--i % 4 != 3) { > wbuf_dword = get_unaligned_le32(wbuf); > writel(wbuf_dword, i2c_bus->buf_base + i - (i % 4)); > } -- With Best Regards, Andy Shevchenko 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 lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (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 F3AFFC83F01 for ; Thu, 31 Aug 2023 14:27:22 +0000 (UTC) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.a=rsa-sha256 header.s=Intel header.b=QSL+K/1H; dkim-atps=neutral Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4Rc3Qd2SSnz3bTj for ; Fri, 1 Sep 2023 00:27:21 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.a=rsa-sha256 header.s=Intel header.b=QSL+K/1H; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=linux.intel.com (client-ip=134.134.136.126; helo=mgamail.intel.com; envelope-from=andriy.shevchenko@linux.intel.com; receiver=lists.ozlabs.org) Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.126]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4Rc3PP393hz2ykV; Fri, 1 Sep 2023 00:26:15 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1693491978; x=1725027978; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=7paPh2mF9Wy7OEwJY1d2GalGy2y7dQanY+hUyqVQRaU=; b=QSL+K/1HCqcFFmk7aRz8sDfVdwAb5Fd6F6TLBInXaPkQaomghdPPdDON sk5mBtwsJsXxA4Y1u3Au1k1Vfk0IX8J0JrJKagtSWMFwnzAH0XuY3HZmk R3C/snX0DlmMwyjnQ/LRWn6uMqJp/7X8XLtJxmMf6WZEtxgWxDb93nMx0 g/EHk9fol1ZARVcecYCFy+5Wh+uP8GLqTfv3K5Lv+pYMOWuTvSlwFxnMl YWEgcb6GgJ+IYJ3ETgDzmE49qj2EVuWIj5ZURyPsIcfcVn00zXQHYNP4O NZ4sOS/CjiEh6zBGv+lU4hulxLjXc5trfztMwycsDbP3njxkxkyHzemRG A==; X-IronPort-AV: E=McAfee;i="6600,9927,10819"; a="360978695" X-IronPort-AV: E=Sophos;i="6.02,217,1688454000"; d="scan'208";a="360978695" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Aug 2023 07:18:39 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10819"; a="689370098" X-IronPort-AV: E=Sophos;i="6.02,217,1688454000"; d="scan'208";a="689370098" Received: from smile.fi.intel.com ([10.237.72.54]) by orsmga003.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Aug 2023 07:18:31 -0700 Received: from andy by smile.fi.intel.com with local (Exim 4.96) (envelope-from ) id 1qbiV9-005PI8-0Q; Thu, 31 Aug 2023 17:18:27 +0300 Date: Thu, 31 Aug 2023 17:18:26 +0300 From: Andy Shevchenko To: Ryan Chen Subject: Re: [PATCH v12 2/2] i2c: aspeed: support ast2600 i2c new register mode driver Message-ID: References: <20230714074522.23827-1-ryan_chen@aspeedtech.com> <20230714074522.23827-3-ryan_chen@aspeedtech.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo X-BeenThere: openbmc@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development list for OpenBMC List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "linux-aspeed@lists.ozlabs.org" , Brendan Higgins , Conor Dooley , "linux-i2c@vger.kernel.org" , Krzysztof Kozlowski , "jk@codeconstruct.com.au" , Jean Delvare , Andi Shyti , Phil Edworthy , Florian Fainelli , "=linux-kernel@vger.kernel.org" <=linux-kernel@vger.kernel.org>, "openbmc@lists.ozlabs.org" , Joel Stanley , "devicetree@vger.kernel.org" , William Zhang , Rob Herring , "linux-arm-kernel@lists.infradead.org" , Tharun Kumar P , Andrew Jeffery , Wolfram Sang , Tyrone Ting , Philipp Zabel Errors-To: openbmc-bounces+openbmc=archiver.kernel.org@lists.ozlabs.org Sender: "openbmc" On Thu, Aug 31, 2023 at 06:04:30AM +0000, Ryan Chen wrote: > > On Fri, Jul 14, 2023 at 03:45:22PM +0800, Ryan Chen wrote: Stop overquoting! Remove the context you are not answering to. ... > > > + if (--i % 4 != 3) > > > + writel(*(u32 *)wbuf, i2c_bus->buf_base + i - (i % 4)); > > > + writel(AST2600_I2CC_SET_TX_BUF_LEN(xfer_len), > > > + i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL); > > > > Wrong memory accessors. You should use something from asm/byteorder.h > > which includes linux/byteorder/generic.h. > > > > Are you preferring add comment to explain more by following? > /* > * The controller's buffer register supports dword writes only. > * Therefore, write dwords to the buffer register in a 4-byte aligned, > * and write the remaining unaligned data at the end. > */ This does not explain endianess bug (or feature) it has. You are using CPU side byteorder for the aligned data. This is not okay, on top of the code looking ugly and prone to errors. Note, that somebody may refer to your code, once accepted, in educational purposes, but since the code is not good written, it makes a false positive impression that this is the right thing to do in the similar case elsewhere. Please, fix this. > for (i = 0; i < xfer_len; i++) { > wbuf[i % 4] = msg->buf[i2c_bus->master_xfer_cnt + i]; > /* accumulating 4 bytes of data, write as a Dword to the buffer register */ > if (i % 4 == 3) > writel(*(u32 *)wbuf, i2c_bus->buf_base + i - 3); > } > /* less than 4 bytes of remaining data, write the remaining part as a Dword */ > if (--i % 4 != 3) > writel(*(u32 *)wbuf, i2c_bus->buf_base + i - (i % 4)); > writel(AST2600_I2CC_SET_TX_BUF_LEN(xfer_len), > i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL); > > Or more columns (use get_unaligned_le32(wbuf); ) by following. > > for (i = 0; i < xfer_len; i++) { > wbuf[i % 4] = msg->buf[i2c_bus->master_xfer_cnt + i]; > if (i % 4 == 3) { > wbuf_dword = get_unaligned_le32(wbuf); > writel(wbuf_dword, i2c_bus->buf_base + i - 3); > } > } > > if (--i % 4 != 3) { > wbuf_dword = get_unaligned_le32(wbuf); > writel(wbuf_dword, i2c_bus->buf_base + i - (i % 4)); > } -- With Best Regards, Andy Shevchenko 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 3D0DFC83F10 for ; Thu, 31 Aug 2023 14:26:42 +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:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=JeFEyf5+vJvDSbpOKOvawLeyiiw8cGGBOcz03O/wazQ=; b=2Fy4L3tMitq2Su NKI0O1B/R1m+EYspK66YVlujtoXRaBTT3moDquyzYBuu39+rIfGgWqi9r7SnWBOy0sd9ioY3WliRQ H+2VbeqsNMLrtG0BVZvRkBsPu/eSwsGbQQI9KZVPiCLPTp6vbVSa+c4nof4AOsTKQkNpxMv5LvFdO uH2zuWGFyWTxwDkt897pT8LHfkY4Xi/pktXV1gnIlRPpEZuJJ38rU3OPgDX5m7Y71xaMitva3Tspl +XdDyDbNpAxiXjpO9cvKaCZW9GySDO3sf6UGFCBjpagJe0Mtlli+CjTp64AyJuZT5cO8fIaBmEYGp K1fCxJFTdQmrgQU+crWg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qbicl-00FR9T-2p; Thu, 31 Aug 2023 14:26:19 +0000 Received: from mgamail.intel.com ([134.134.136.126]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qbicj-00FR8t-0r for linux-arm-kernel@lists.infradead.org; Thu, 31 Aug 2023 14:26:18 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1693491977; x=1725027977; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=7paPh2mF9Wy7OEwJY1d2GalGy2y7dQanY+hUyqVQRaU=; b=RqD/kHBlRpRiP5z6TT3i9oZAzvG+I/bp/X6itioBUDuupYgRSBxUTK4w FbCiXGurA1+YulSCcO5lnOeMZVfZv1wwCs+X/wcIctKlzlLG+nF1Analk N2T++xSzrsiFNx3WHj5vObBglSu9qpEZYNMGOLAio9ceqWY/cpCkcFxtv 71LFVw4Nfwlu0YBuXJj235hSF1yPFetXjF9oH8IL0BOBQNPIv68A469mO KA8C0JYeTVCK624YaFKT2TqsRMqGg+fcUPQrw+z9u7H+8bhm8zFMag6CI WNgfQtYfRzfkPK1ladzSIHiHQLfxMmUYkAR5uuM/2UGqD55pwmy5aloZu A==; X-IronPort-AV: E=McAfee;i="6600,9927,10819"; a="360978692" X-IronPort-AV: E=Sophos;i="6.02,217,1688454000"; d="scan'208";a="360978692" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Aug 2023 07:18:39 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10819"; a="689370098" X-IronPort-AV: E=Sophos;i="6.02,217,1688454000"; d="scan'208";a="689370098" Received: from smile.fi.intel.com ([10.237.72.54]) by orsmga003.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Aug 2023 07:18:31 -0700 Received: from andy by smile.fi.intel.com with local (Exim 4.96) (envelope-from ) id 1qbiV9-005PI8-0Q; Thu, 31 Aug 2023 17:18:27 +0300 Date: Thu, 31 Aug 2023 17:18:26 +0300 From: Andy Shevchenko To: Ryan Chen Cc: "jk@codeconstruct.com.au" , Brendan Higgins , Benjamin Herrenschmidt , Joel Stanley , Rob Herring , Krzysztof Kozlowski , Andrew Jeffery , Philipp Zabel , Wolfram Sang , "linux-i2c@vger.kernel.org" , Florian Fainelli , Jean Delvare , William Zhang , Tyrone Ting , Tharun Kumar P , Conor Dooley , Phil Edworthy , "openbmc@lists.ozlabs.org" , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-aspeed@lists.ozlabs.org" , "=linux-kernel@vger.kernel.org" <=linux-kernel@vger.kernel.org>, Andi Shyti Subject: Re: [PATCH v12 2/2] i2c: aspeed: support ast2600 i2c new register mode driver Message-ID: References: <20230714074522.23827-1-ryan_chen@aspeedtech.com> <20230714074522.23827-3-ryan_chen@aspeedtech.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230831_072617_425347_82A08DA8 X-CRM114-Status: GOOD ( 22.78 ) 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 Thu, Aug 31, 2023 at 06:04:30AM +0000, Ryan Chen wrote: > > On Fri, Jul 14, 2023 at 03:45:22PM +0800, Ryan Chen wrote: Stop overquoting! Remove the context you are not answering to. ... > > > + if (--i % 4 != 3) > > > + writel(*(u32 *)wbuf, i2c_bus->buf_base + i - (i % 4)); > > > + writel(AST2600_I2CC_SET_TX_BUF_LEN(xfer_len), > > > + i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL); > > > > Wrong memory accessors. You should use something from asm/byteorder.h > > which includes linux/byteorder/generic.h. > > > > Are you preferring add comment to explain more by following? > /* > * The controller's buffer register supports dword writes only. > * Therefore, write dwords to the buffer register in a 4-byte aligned, > * and write the remaining unaligned data at the end. > */ This does not explain endianess bug (or feature) it has. You are using CPU side byteorder for the aligned data. This is not okay, on top of the code looking ugly and prone to errors. Note, that somebody may refer to your code, once accepted, in educational purposes, but since the code is not good written, it makes a false positive impression that this is the right thing to do in the similar case elsewhere. Please, fix this. > for (i = 0; i < xfer_len; i++) { > wbuf[i % 4] = msg->buf[i2c_bus->master_xfer_cnt + i]; > /* accumulating 4 bytes of data, write as a Dword to the buffer register */ > if (i % 4 == 3) > writel(*(u32 *)wbuf, i2c_bus->buf_base + i - 3); > } > /* less than 4 bytes of remaining data, write the remaining part as a Dword */ > if (--i % 4 != 3) > writel(*(u32 *)wbuf, i2c_bus->buf_base + i - (i % 4)); > writel(AST2600_I2CC_SET_TX_BUF_LEN(xfer_len), > i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL); > > Or more columns (use get_unaligned_le32(wbuf); ) by following. > > for (i = 0; i < xfer_len; i++) { > wbuf[i % 4] = msg->buf[i2c_bus->master_xfer_cnt + i]; > if (i % 4 == 3) { > wbuf_dword = get_unaligned_le32(wbuf); > writel(wbuf_dword, i2c_bus->buf_base + i - 3); > } > } > > if (--i % 4 != 3) { > wbuf_dword = get_unaligned_le32(wbuf); > writel(wbuf_dword, i2c_bus->buf_base + i - (i % 4)); > } -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel