linux-mips.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Zhou Yanjie <zhouyanjie@wanyeetech.com>
To: Wolfram Sang <wsa@kernel.org>
Cc: paul@crapouillou.net, stable@vger.kernel.org,
	linux-mips@vger.kernel.org, linux-i2c@vger.kernel.org,
	linux-kernel@vger.kernel.org, dongsheng.qiu@ingenic.com,
	aric.pzqi@ingenic.com, sernia.zhou@foxmail.com
Subject: Re: [PATCH] I2C: JZ4780: Fix bug for Ingenic X1000.
Date: Fri, 19 Mar 2021 14:58:16 +0800	[thread overview]
Message-ID: <644d19d8-9444-4dde-a891-c9dfd523389e@wanyeetech.com> (raw)
In-Reply-To: <20210318170623.GA1961@ninjato>

Hi Wolfram,


Sorry, please forgive my carefulness, I wrongly sent the version that 
did not clean up, resulting in missing the reporter's information and 
some errors in formats.


On 2021/3/19 上午1:06, Wolfram Sang wrote:
> On Fri, Mar 19, 2021 at 12:25:43AM +0800, 周琰杰 (Zhou Yanjie) wrote:
>> Only send "X1000_I2C_DC_STOP" when last byte, or it will cause
>> error when I2C write operation.
> Any write operation? I wonder then why nobody noticed before?


The standard I2C communication should look like this:

Read:

device_addr + w, reg_addr, device_addr + r, data;

Write:

device_addr + w, reg_addr, data;


But without this patch, it looks like this:

Read:

device_addr + w, reg_addr, device_addr + r, data;

Write:

device_addr + w, reg_addr, device_addr + w, data;

This is clearly not correct.


When I added support for X1000 to this driver, the hardware used was 
CU1000-Neo. On this hardware, there was an ADS7830 that communicated 
through I2C, but the operation of ADS7830 only involved read operations, 
so I was at that time failed to realize the problem with the write 
operation.
In addition, because X1000 did not implement relatively complete support 
in the mainline until the second half of 2020, there are still a large 
number of users who are still using the old SDK (kernel 3.10 and 
kernel4.4) provided by Ingenics, which may also be indirectly delayed 
exposure of this problem.


>> -			while ((i2c_sta & JZ4780_I2C_STA_TFNF) &&
>> -					(i2c->wt_len > 0)) {
>> +			while ((i2c_sta & JZ4780_I2C_STA_TFNF) && (i2c->wt_len > 0)) {
> This is a cosmetic change only IIUC. Shouldn't be in a bugfix.
>

My fault, I will remove it in the next version.


Thanks and best regards!



  reply	other threads:[~2021-03-19  6:59 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-18 16:25 Fix bug for Ingenic X1000 周琰杰 (Zhou Yanjie)
2021-03-18 16:25 ` [PATCH] I2C: JZ4780: " 周琰杰 (Zhou Yanjie)
2021-03-18 17:01   ` Greg KH
2021-03-18 17:06   ` Wolfram Sang
2021-03-19  6:58     ` Zhou Yanjie [this message]
2021-03-31  7:18       ` Wolfram Sang
2021-04-02 15:30         ` Zhou Yanjie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=644d19d8-9444-4dde-a891-c9dfd523389e@wanyeetech.com \
    --to=zhouyanjie@wanyeetech.com \
    --cc=aric.pzqi@ingenic.com \
    --cc=dongsheng.qiu@ingenic.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=paul@crapouillou.net \
    --cc=sernia.zhou@foxmail.com \
    --cc=stable@vger.kernel.org \
    --cc=wsa@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).