From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Pavel Machek <pavel@denx.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
<linux-kernel@vger.kernel.org>, <stable@vger.kernel.org>,
Mathieu Othacehe <m.othacehe@gmail.com>
Subject: Re: [PATCH 4.19 11/25] iio: vcnl4000: Fix i2c swapped word reading.
Date: Tue, 16 Jun 2020 09:18:16 +0100 [thread overview]
Message-ID: <20200616091816.00004ac9@Huawei.com> (raw)
In-Reply-To: <20200615133018.GA18126@duo.ucw.cz>
On Mon, 15 Jun 2020 15:30:18 +0200
Pavel Machek <pavel@denx.de> wrote:
> Hi!
>
> > From: Mathieu Othacehe <m.othacehe@gmail.com>
> >
> > commit 18dfb5326370991c81a6d1ed6d1aeee055cb8c05 upstream.
> >
> > The bytes returned by the i2c reading need to be swapped
> > unconditionally. Otherwise, on be16 platforms, an incorrect value will be
> > returned.
> >
> > Taking the slow path via next merge window as its been around a while
> > and we have a patch set dependent on this which would be held up.
>
> Is there some explanation how this is correct Somewhere? I assume i2c
> hardware has fixed endianity (not depending on CPU), so unconditional
> swapping will cause problems either on le or on be machines...?
>
Hmm, this isn't introducing a bug, but looking again, it appears
that the original code was fine as well..
smbus/I2C has a fixed ordering on the wire (when people actually obey the spec).
So when an i2c_smbus_read_word_data call is made, the drivers / subsystem
assume that ordering an provide the data in CPU endian order.
Unfortunately a reasonable set of devices provide data in the opposite
order from that required by the i2c spec. Thus it needs to be unconditionally
swapped to put it back into the correct order.
i2c_smbus_read_word_data_swapped does that for us.
Now the reason it worked before was this was previously doing a
block read rather than a word read. i2c_smbus_read_i2c_block_data
which is just reading a bunch of bytes rather than doing a word read.
So this is a false positive as a fix. Sorry about that.
Conversely it shouldn't break anything.
Looking back at the history, what happened was that up to v5 of this patchset
factored out the reads, but whilst doing that converted them to
i2c_smbus_read_word_data with a be16_to_cpu call which was buggy and
picked up in review. Hence confusion occurred and I guess my eyes saw
what they expected to see once the fix was pulled to the front of the series.
Thanks,
Jonathan
> Best regards,
> Pavel
next prev parent reply other threads:[~2020-06-16 8:19 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-09 17:44 [PATCH 4.19 00/25] 4.19.128-rc1 review Greg Kroah-Hartman
2020-06-09 17:44 ` [PATCH 4.19 01/25] devinet: fix memleak in inetdev_init() Greg Kroah-Hartman
2020-06-09 17:44 ` [PATCH 4.19 02/25] l2tp: add sk_family checks to l2tp_validate_socket Greg Kroah-Hartman
2020-06-09 17:44 ` [PATCH 4.19 03/25] l2tp: do not use inet_hash()/inet_unhash() Greg Kroah-Hartman
2020-06-09 17:44 ` [PATCH 4.19 04/25] net: usb: qmi_wwan: add Telit LE910C1-EUX composition Greg Kroah-Hartman
2020-06-09 17:44 ` [PATCH 4.19 05/25] NFC: st21nfca: add missed kfree_skb() in an error path Greg Kroah-Hartman
2020-06-09 17:44 ` [PATCH 4.19 06/25] vsock: fix timeout in vsock_accept() Greg Kroah-Hartman
2020-06-09 17:44 ` [PATCH 4.19 07/25] net: check untrusted gso_size at kernel entry Greg Kroah-Hartman
2020-06-09 17:44 ` [PATCH 4.19 08/25] USB: serial: qcserial: add DW5816e QDL support Greg Kroah-Hartman
2020-06-09 17:44 ` [PATCH 4.19 09/25] USB: serial: usb_wwan: do not resubmit rx urb on fatal errors Greg Kroah-Hartman
2020-06-09 17:45 ` [PATCH 4.19 10/25] USB: serial: option: add Telit LE910C1-EUX compositions Greg Kroah-Hartman
2020-06-09 17:45 ` [PATCH 4.19 11/25] iio: vcnl4000: Fix i2c swapped word reading Greg Kroah-Hartman
2020-06-15 13:30 ` Pavel Machek
2020-06-16 8:18 ` Jonathan Cameron [this message]
2020-06-09 17:45 ` [PATCH 4.19 12/25] usb: musb: start session in resume for host port Greg Kroah-Hartman
2020-06-09 17:45 ` [PATCH 4.19 13/25] usb: musb: Fix runtime PM imbalance on error Greg Kroah-Hartman
2020-06-09 17:45 ` [PATCH 4.19 14/25] vt: keyboard: avoid signed integer overflow in k_ascii Greg Kroah-Hartman
2020-06-09 17:45 ` [PATCH 4.19 15/25] tty: hvc_console, fix crashes on parallel open/close Greg Kroah-Hartman
2020-06-09 17:45 ` [PATCH 4.19 16/25] staging: rtl8712: Fix IEEE80211_ADDBA_PARAM_BUF_SIZE_MASK Greg Kroah-Hartman
2020-06-09 17:45 ` [PATCH 4.19 17/25] CDC-ACM: heed quirk also in error handling Greg Kroah-Hartman
2020-06-09 17:45 ` [PATCH 4.19 18/25] nvmem: qfprom: remove incorrect write support Greg Kroah-Hartman
2020-06-09 17:45 ` [PATCH 4.19 19/25] x86/cpu: Add a steppings field to struct x86_cpu_id Greg Kroah-Hartman
2020-06-09 17:45 ` [PATCH 4.19 20/25] x86/cpu: Add table argument to cpu_matches() Greg Kroah-Hartman
2020-06-09 17:45 ` [PATCH 4.19 21/25] x86/speculation: Add Special Register Buffer Data Sampling (SRBDS) mitigation Greg Kroah-Hartman
2020-06-09 17:45 ` [PATCH 4.19 22/25] x86/speculation: Add SRBDS vulnerability and mitigation documentation Greg Kroah-Hartman
2020-06-09 17:45 ` [PATCH 4.19 23/25] x86/speculation: Add Ivy Bridge to affected list Greg Kroah-Hartman
2020-06-09 17:45 ` [PATCH 4.19 24/25] uprobes: ensure that uprobe->offset and ->ref_ctr_offset are properly aligned Greg Kroah-Hartman
2020-06-09 18:55 ` Naresh Kamboju
2020-06-09 19:03 ` Greg Kroah-Hartman
2020-06-10 14:53 ` Oleg Nesterov
2020-06-10 14:58 ` Greg Kroah-Hartman
2020-06-11 16:51 ` Oleg Nesterov
2020-06-12 5:46 ` Greg Kroah-Hartman
2020-06-09 17:45 ` [PATCH 4.19 25/25] Revert "net/mlx5: Annotate mutex destroy for root ns" Greg Kroah-Hartman
2020-06-11 22:17 ` Saeed Mahameed
2020-06-12 1:07 ` Sasha Levin
2020-06-09 19:01 ` [PATCH 4.19 00/25] 4.19.128-rc1 review Naresh Kamboju
2020-06-09 19:20 ` Shuah Khan
2020-06-09 19:41 ` Greg Kroah-Hartman
2020-06-10 11:29 ` Jon Hunter
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=20200616091816.00004ac9@Huawei.com \
--to=jonathan.cameron@huawei.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=m.othacehe@gmail.com \
--cc=pavel@denx.de \
--cc=stable@vger.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).