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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 EAE76C433F5 for ; Wed, 23 Mar 2022 09:32:05 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3D19E10E13F; Wed, 23 Mar 2022 09:32:05 +0000 (UTC) Received: from 189.cn (ptr.189.cn [183.61.185.104]) by gabe.freedesktop.org (Postfix) with ESMTP id A871C10E13F for ; Wed, 23 Mar 2022 09:32:03 +0000 (UTC) HMM_SOURCE_IP: 10.64.8.41:35378.1509098416 HMM_ATTACHE_NUM: 0000 HMM_SOURCE_TYPE: SMTP Received: from clientip-114.242.206.180 (unknown [10.64.8.41]) by 189.cn (HERMES) with SMTP id 3405D100225; Wed, 23 Mar 2022 17:31:58 +0800 (CST) Received: from ([114.242.206.180]) by gateway-151646-dep-b7fbf7d79-9vctg with ESMTP id 61937688d8bb45d78e3cf3b4744afb5f for robh@kernel.org; Wed, 23 Mar 2022 17:32:02 CST X-Transaction-ID: 61937688d8bb45d78e3cf3b4744afb5f X-Real-From: 15330273260@189.cn X-Receive-IP: 114.242.206.180 X-MEDUSA-Status: 0 Message-ID: Date: Wed, 23 Mar 2022 17:31:57 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH v11 7/7] drm/lsdc: add drm driver for loongson display controller Content-Language: en-US To: Rob Herring References: <20220321162916.1116541-1-15330273260@189.cn> <20220321162916.1116541-8-15330273260@189.cn> From: Sui Jingfeng <15330273260@189.cn> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Qing Zhang , David Airlie , Jiaxun Yang , linux-kernel@vger.kernel.org, Sam Ravnborg , kernel test robot , Krzysztof Kozlowski , Dan Carpenter , devicetree@vger.kernel.org, suijingfeng , Thomas Zimmermann , Roland Scheidegger , Andrey Zhizhikin , dri-devel@lists.freedesktop.org, Thomas Bogendoerfer , linux-mips@vger.kernel.org, "David S . Miller" Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 2022/3/23 04:49, Rob Herring wrote: >> + >> + if (state) { >> + val = readb(li2c->dir_reg); >> + val |= mask; >> + writeb(val, li2c->dir_reg); >> + } else { >> + val = readb(li2c->dir_reg); >> + val &= ~mask; >> + writeb(val, li2c->dir_reg); >> + >> + val = readb(li2c->dat_reg); >> + if (state) > This condition is never true. We're in the 'else' because !state. > >> + val |= mask; >> + else >> + val &= ~mask; >> + writeb(val, li2c->dat_reg); > Shouldn't you set the data register low first and then change the > direction? Otherwise, you may be driving high for a moment. However, if > high is always done by setting the direction as input, why write the > data register each time? I'm assuming whatever is written to the dat_reg > is maintained regardless of pin state. To be honest, i have rewrite GPIO emulated i2c several times. Either give data first, then give the direction or give the direction first, then the data will be OK in practice. In the theory, the GPIO data should be given before the GPIO direction, I was told doing that way when learning Single-Chip Microcomputer (AT89S52). But the high "MUST" be done by setting the direction as input. It is "MUST" not "CAN" because writing code as the following way works in practice. if (state) { val = readb(li2c->dir_reg); val |= mask; writeb(val, li2c->dir_reg); } else { // ... } If the adjust the above code by first set the detection as output, then set the GPIO data register with high voltage level("1"). as the following demonstrate code, if (state) { /* First set this pin as output */ val = readb(li2c->dir_reg); val |= mask; writeb(val, li2c->dir_reg); /* Then, set the state to high */ val = readb(li2c->dat_reg); val |= mask; writeb(val, li2c->dat_reg); } else { // ... } Then i2c6 will NOT work as exacted, i2c7 will work, so strangely. It may because the GPIO is open drained, not Push-pull output. Output high is achieved by externalpull up resistance on the PCB.