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

[Sui Jingfeng <15330273260@189.cn>]

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.