From: Wolfram Sang <wsa@the-dreams.de>
To: Max Schwarz <max.schwarz@online.de>
Cc: "Grant Likely" <grant.likely@linaro.org>,
"Rob Herring" <robh+dt@kernel.org>,
linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, "Heiko Stübner" <heiko@sntech.de>,
"Maxime Ripard" <maxime.ripard@free-electrons.com>
Subject: Re: [PATCH v4] i2c: add driver for Rockchip RK3xxx SoC I2C adapter
Date: Tue, 10 Jun 2014 10:19:50 +0200 [thread overview]
Message-ID: <20140610081950.GA2620@katana> (raw)
In-Reply-To: <1468105.xYIQWVNzVZ@typ>
[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]
> > > + /* Synchronization & notification */
> > > + spinlock_t lock;
> >
> > Why the lock? The core has per-adapter locks anyhow.
>
> I'm using it to lock the rk3x_i2c struct during interrupts. It's needed there
> because an operation can timeout, which means the interrupt can occur at any
> time and possibly conflict with the cleanup I do after a timeout.
>
> I looked around in i2c-exynos5.c, i2c-pxa.c and others, and they do it the
> same way. Could you explain in more detail why it's not needed?
I saw that you disable IRQs on timeout, so that shouldn't happen? You
don't disable IRQs after a successful transfer, though, AFAICS.
> > It looks to me that you STOP after every message? You should use
> > REPEATED_START inbetween messages and only stop after the last message
> > of a transfer.
>
> I had a fight with the hw today and finally got it to issue a REPEATED_START
> for multiple "boring" messages. Will be included in the next version.
Great.
> Actually, I wasn't aware that (len == 0) is a valid case. The hw supports it
> in both modes, I just tested that. So the check is going away.
Also good!
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
To: Max Schwarz <max.schwarz-BGeptl67XyCzQB+pC5nmwQ@public.gmane.org>
Cc: "Grant Likely"
<grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
"Rob Herring" <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
"Heiko Stübner" <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>,
"Maxime Ripard"
<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Subject: Re: [PATCH v4] i2c: add driver for Rockchip RK3xxx SoC I2C adapter
Date: Tue, 10 Jun 2014 10:19:50 +0200 [thread overview]
Message-ID: <20140610081950.GA2620@katana> (raw)
In-Reply-To: <1468105.xYIQWVNzVZ@typ>
[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]
> > > + /* Synchronization & notification */
> > > + spinlock_t lock;
> >
> > Why the lock? The core has per-adapter locks anyhow.
>
> I'm using it to lock the rk3x_i2c struct during interrupts. It's needed there
> because an operation can timeout, which means the interrupt can occur at any
> time and possibly conflict with the cleanup I do after a timeout.
>
> I looked around in i2c-exynos5.c, i2c-pxa.c and others, and they do it the
> same way. Could you explain in more detail why it's not needed?
I saw that you disable IRQs on timeout, so that shouldn't happen? You
don't disable IRQs after a successful transfer, though, AFAICS.
> > It looks to me that you STOP after every message? You should use
> > REPEATED_START inbetween messages and only stop after the last message
> > of a transfer.
>
> I had a fight with the hw today and finally got it to issue a REPEATED_START
> for multiple "boring" messages. Will be included in the next version.
Great.
> Actually, I wasn't aware that (len == 0) is a valid case. The hw supports it
> in both modes, I just tested that. So the check is going away.
Also good!
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
next prev parent reply other threads:[~2014-06-10 8:19 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-28 23:34 [PATCH v2] i2c: add driver for Rockchip RK3xxx SoC I2C adapter Max Schwarz
2014-04-28 23:34 ` Max Schwarz
2014-04-29 22:34 ` Heiko Stübner
2014-04-29 22:34 ` Heiko Stübner
2014-05-16 22:50 ` Heiko Stübner
2014-05-16 22:50 ` Heiko Stübner
2014-05-19 0:37 ` [PATCH v3] " Max Schwarz
2014-05-19 0:37 ` Max Schwarz
2014-05-19 7:49 ` Maxime Ripard
2014-05-19 7:49 ` Maxime Ripard
2014-05-19 9:32 ` [PATCH v4] " Max Schwarz
2014-05-19 9:32 ` Max Schwarz
2014-05-20 7:42 ` Maxime Ripard
2014-05-20 7:42 ` Maxime Ripard
2014-05-26 8:34 ` Heiko Stübner
2014-06-02 12:08 ` Wolfram Sang
2014-06-02 12:08 ` Wolfram Sang
2014-06-07 15:32 ` Max Schwarz
2014-06-07 15:32 ` Max Schwarz
2014-06-10 8:19 ` Wolfram Sang [this message]
2014-06-10 8:19 ` Wolfram Sang
2014-05-20 7:53 ` [PATCH v3] " Heiko Stübner
2014-05-20 7:53 ` Heiko Stübner
2014-06-07 17:36 ` [PATCH v5] " Max Schwarz
2014-06-10 19:27 ` Wolfram Sang
2014-06-11 20:24 ` Max Schwarz
2014-06-11 20:24 ` Max Schwarz
2014-06-07 17:44 ` Max Schwarz
2014-06-07 17:44 ` Max Schwarz
2014-06-11 20:34 ` [PATCH v6] " Max Schwarz
2014-06-11 20:34 ` Max Schwarz
2014-06-11 22:25 ` Wolfram Sang
2014-06-11 22:25 ` Wolfram Sang
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=20140610081950.GA2620@katana \
--to=wsa@the-dreams.de \
--cc=devicetree@vger.kernel.org \
--cc=grant.likely@linaro.org \
--cc=heiko@sntech.de \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=max.schwarz@online.de \
--cc=maxime.ripard@free-electrons.com \
--cc=robh+dt@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.