I don't see how the call to platform_i2c_transfer() is supposed to do
something useful in case `bus >= SOFTWARE_I2C_MAX_BUS`. I would expect
bus numbers to overlap anyway. Maybe it's the best to never call it
in case of CONFIG(SOFTWARE_I2C)?

I think the problem is just that you never know how many I2C busses a platform may have, and SOFTWARE_I2C_MAX_BUS has to be hardcoded to something. I think I just didn't want to restrict the maximum number of real I2C busses just for this feature when I wrote it. But if it makes things easier now, maybe we can just change SOFTWARE_I2C_MAX_BUS into I2C_MAX_BUS and enforce it everywhere to fix this issue (I believe 10 should be enough for all platforms I've ever seen, but I haven't seen them all).

> > OTOH, if that is true, we shouldn't select SOFTWARE_I2C from a driver
> > because it disables platform_i2c_tansfer() and who knows when it is
> > needed?

CONFIG(SOFTWARE_I2C) only says that the software I2C code is compiled in, it doesn't mean you can't still use hardware I2C controllers. The way it's designed right now it will still always use hardware by default unless you explicitly enable software I2C (by setting the software_i2c[bus] pointer to something non-NULL).

My current hunch would be to redesign software i2c so one can use it
to implement i2c_bus. And deprecate i2c_simple anyway for everything
that might remotely have a second controller chip on board.

I'll have a look at this next year.

i2c_simple is still used for all non-x86 devices, and we'll probably want to keep it that way since i2c_bus is specific to devicetree, which is only used on x86. Making software I2C a possible backend for i2c_bus sounds reasonable, but please don't kill i2c_simple.

View Change

To view, visit change 35726. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7803566b64158405efc04a39f80a0ec98b44e646
Gerrit-Change-Number: 35726
Gerrit-PatchSet: 14
Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter@9elements.com>
Gerrit-Reviewer: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph@9elements.com>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki@gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com>
Gerrit-CC: Matt DeVillier <matt.devillier@gmail.com>
Gerrit-CC: Nico Huber <nico.h@gmx.de>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-CC: Дмитрий Понаморев <dponamorev@gmail.com>
Gerrit-Comment-Date: Fri, 10 Jan 2020 23:25:50 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment