From: Eddie James <eajames@linux.vnet.ibm.com>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: linux-i2c <linux-i2c@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
devicetree <devicetree@vger.kernel.org>,
Wolfram Sang <wsa@the-dreams.de>,
Rob Herring <robh+dt@kernel.org>, Joel Stanley <joel@jms.id.au>,
Mark Rutland <mark.rutland@arm.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Randy Dunlap <rdunlap@infradead.org>
Subject: Re: [PATCH v9 2/7] i2c: Add FSI-attached I2C master algorithm
Date: Tue, 5 Jun 2018 08:31:16 -0500 [thread overview]
Message-ID: <adbc4f16-326c-1cc8-a293-cfd5e2d8a070@linux.vnet.ibm.com> (raw)
In-Reply-To: <7daf29f643bb0445fceef85b1a7fff71048f2aa6.camel@kernel.crashing.org>
On 06/04/2018 06:38 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2018-06-04 at 22:21 +0300, Andy Shevchenko wrote:
>>> +#define I2C_INT_ENABLE 0x0000ff80
>>> +#define I2C_INT_ERR 0x0000fcc0
>> Now it looks like a flags combinations.
>> For me as for reader would be better to see quickly a decoded line.
>>
>> My proposal is to introduce something like following
>>
>> _INT_ALL GENMASK()
>> _INT_ENABLE (_INT_ALL & ~(_FOO | _BAR))
>> _INT_ERR ... similar way as above ...
>>
>> What do you think?
> I don't think this absolutely needs to change but yes, open coding is
> error prone. However I would think it more readable to use positive
> logic and just list all the bits that are *set* even if it's a bit more
> text:
>
> #define I2C_INT_ERR (I2C_INT_INV_CMD |\
> I2C_INT_PARITY |\
> I2C_INT_BE_OVERRUN |\
> .../...)
>
> #define I2C_INT_ENABLE (I2C_INT_ERR |\
> I2C_INT_DAT_REQ |\
> I2C_INT_CMD_COMP)
>
> Note: Eddie, I notice I2C_INT_BUSY is in "ERR" but not in "ENABLE", any
> reason for that ?
Yes, we don't want to enable an interrupt if I2C gets into the busy
state, as that happens during every transfer. However it would likely
be an error condition if we get that when the transfer is supposed to be
complete. These were from the legacy driver... I just realized that
neither are actually being used in this driver, so I will drop them.
Thanks,
Eddie
>
> Cheers,
> Ben.
>
next prev parent reply other threads:[~2018-06-05 13:33 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-04 19:00 [PATCH v9 0/7] i2c: Add FSI-attached I2C master algorithm Eddie James
2018-06-04 19:00 ` [PATCH v9 1/7] dt-bindings: i2c: Add FSI-attached I2C master dt binding documentation Eddie James
2018-06-04 19:00 ` [PATCH v9 2/7] i2c: Add FSI-attached I2C master algorithm Eddie James
2018-06-04 19:21 ` Andy Shevchenko
2018-06-04 21:11 ` Eddie James
2018-06-04 23:38 ` Benjamin Herrenschmidt
2018-06-05 9:17 ` Andy Shevchenko
2018-06-05 13:31 ` Eddie James [this message]
2018-06-04 19:00 ` [PATCH v9 3/7] i2c: fsi: Add port structures Eddie James
2018-06-04 19:17 ` Andy Shevchenko
2018-06-04 19:40 ` Eddie James
2018-06-04 23:33 ` Benjamin Herrenschmidt
2018-06-04 19:00 ` [PATCH v9 4/7] i2c: fsi: Add abort and hardware reset procedures Eddie James
2018-06-04 19:00 ` [PATCH v9 5/7] i2c: fsi: Add transfer implementation Eddie James
2018-06-04 19:14 ` Andy Shevchenko
2018-06-04 19:37 ` Eddie James
2018-06-04 19:45 ` Peter Rosin
2018-06-04 21:12 ` Eddie James
2018-06-04 19:00 ` [PATCH v9 6/7] i2c: fsi: Add I2C master locking Eddie James
2018-06-04 19:00 ` [PATCH v9 7/7] i2c: fsi: Add bus recovery Eddie James
2018-06-04 19:22 ` [PATCH v9 0/7] i2c: Add FSI-attached I2C master algorithm Andy Shevchenko
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=adbc4f16-326c-1cc8-a293-cfd5e2d8a070@linux.vnet.ibm.com \
--to=eajames@linux.vnet.ibm.com \
--cc=andy.shevchenko@gmail.com \
--cc=benh@kernel.crashing.org \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=joel@jms.id.au \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=rdunlap@infradead.org \
--cc=robh+dt@kernel.org \
--cc=wsa@the-dreams.de \
/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).