linux-kernel - Re: [PATCH v11 7/7] drm/lsdc: add drm driver for loongson display controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f0db9bc0-ae9b-e86e-cc14-376dc40b86f4@189.cn>
Date:   Wed, 23 Mar 2022 17:31:57 +0800
From:   Sui Jingfeng <15330273260@....cn>
To:     Rob Herring <robh@...nel.org>
Cc:     Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Roland Scheidegger <sroland@...are.com>,
        Zack Rusin <zackr@...are.com>,
        Christian Gmeiner <christian.gmeiner@...il.com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Andrey Zhizhikin <andrey.zhizhikin@...ca-geosystems.com>,
        Sam Ravnborg <sam@...nborg.org>,
        "David S . Miller" <davem@...emloft.net>,
        Jiaxun Yang <jiaxun.yang@...goat.com>,
        Lucas Stach <l.stach@...gutronix.de>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Ilia Mirkin <imirkin@...m.mit.edu>,
        Qing Zhang <zhangqing@...ngson.cn>,
        suijingfeng <suijingfeng@...ngson.cn>,
        linux-mips@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        kernel test robot <lkp@...el.com>
Subject: Re: [PATCH v11 7/7] drm/lsdc: add drm driver for loongson display
 controller


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.