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