From: Timur Tabi <timur@freescale.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v3] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions
Date: Tue, 16 Dec 2008 17:46:31 -0600 [thread overview]
Message-ID: <49483DD7.5080908@freescale.com> (raw)
In-Reply-To: <20081216204954.CF8DA832E8A1@gemini.denx.de>
Wolfgang Denk wrote:
> Hm... what exactly is broken with the concept of having a "current
> device" or a "current bus"? We use it elasewhere, too (like for
> selection IDE or S-ATA devices and such), and so far I am not aware
> of fundamental issues because of that.
I think it's a kludge because you have to set the current device before you
can access it. It seems ridiculous that you have to do this:
i2c_set_bus_num(x)
i2c_write(...)
when you could do this:
i2c_write(x, ...)
>> Sounds to me like you haven't really looked at the U-Boot code. There are
>> plenty of places where one function does I2C operations, then calls another
>> function that does its own.
>
> Really? Where exactly does such a thing happen?
Well, I can't find a real example, but here's something close. Take a look at
function pib_init() in mpc8568mds.c. This function needs to talk to the 2nd
I2C bus. It has no idea who called it, so it has no idea what the current I2C
bus is. Therefore, it has to save and restore it.
Now consider the code that calls pib_init(). Technically, this code cannot
know that pib_init() does I2C operations. So it doesn't know that pib_init()
could change the current I2C bus. So it wouldn't know to save and restore
what it needs.
This situation could happen anywhere.
> I tend to call this a bug that needs to be fixed, if there is ssuch
> code.
It's not a bug. Function X does I2C operations, and function Y also does I2C
operations, but on a different device. If function X calls function Y, then Y
needs to save and restore the current bus number. You will never get rid of
this requirement as long as you have the concept of a current I2C bus number.
So you're still going to need i2c_get_bus_num() and i2c_set_bus_num(), and
you're still going to have code like this:
bus = i2c_get_bus_num();
i2c_set_bus_num(x);
i2c_write(...)
i2c_set_bus_num(bus);
> This statement is probably correct: I don't share your point of view
> here, either.
In that case, I have no interest in working on this new I2C design. You guys
obviously have everything under control, so good luck.
--
Timur Tabi
Linux Kernel Developer @ Freescale
next prev parent reply other threads:[~2008-12-16 23:46 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-12-03 17:28 [U-Boot] [PATCH v3] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions Timur Tabi
2008-12-06 17:49 ` Jean-Christophe PLAGNIOL-VILLARD
2008-12-15 22:47 ` Wolfgang Denk
2008-12-15 23:37 ` ksi at koi8.net
2008-12-15 23:42 ` Timur Tabi
2008-12-16 0:24 ` ksi at koi8.net
2008-12-16 15:19 ` Timur Tabi
2008-12-16 17:58 ` ksi at koi8.net
2008-12-16 18:51 ` Timur Tabi
2008-12-16 19:40 ` ksi at koi8.net
2008-12-16 20:35 ` Jerry Van Baren
2008-12-16 20:58 ` Wolfgang Denk
2008-12-17 3:55 ` ksi at koi8.net
2008-12-16 20:49 ` Wolfgang Denk
2008-12-16 23:46 ` Timur Tabi [this message]
2008-12-17 1:00 ` ksi at koi8.net
2008-12-17 1:28 ` Wolfgang Denk
2008-12-16 17:47 ` Scott Wood
2008-12-16 18:07 ` ksi at koi8.net
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=49483DD7.5080908@freescale.com \
--to=timur@freescale.com \
--cc=u-boot@lists.denx.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