Hello Michal,
Am 11.05.2020 um 15:36 schrieb Michal Simek:
On 07. 05. 20 12:02, Heiko Schocher wrote:
Hello Michal,
Am 07.05.2020 um 10:18 schrieb Michal Simek:
On 06. 05. 20 16:47, Simon Glass wrote:
Hi Michal,
On Tue, 5 May 2020 at 21:43, Simon Glass <s...@chromium.org> wrote:
Hi Michal,
On Tue, 5 May 2020 at 02:26, Michal Simek <michal.si...@xilinx.com>
wrote:
On 15. 04. 20 8:40, Michal Simek wrote:
On 10. 04. 20 10:49, Heiko Schocher wrote:
Hello Michal,
Am 10.04.2020 um 08:46 schrieb Michal Simek:
Hi Heiko,
On 10. 04. 20 7:11, Heiko Schocher wrote:
Hello Michal,
Am 09.04.2020 um 16:03 schrieb Michal Simek:
Hi Heiko and Simon,
I have find out one bug in i2c class. I am using zcu104 revC
board
which
has dts in mainline where i2c1 has i2c mux with some channels.
In DT clock-frequency = <400000>; is specified and it is read in
i2c_post_probe(). But i2c_mux_bus_drv is also UCLASS_I2C which
means
that post probe is called for it too. And because clock-frequency
property is not there default 100k is used.
I think that is bug and should be fixed.
Heiko: Are you aware about this issue?
No :-(
The question is, is this a bug?
I have never seen clock-frequency property in i2c mux bus node.
Also I
have looked at linux dt binding docs and nothing like that is
specified
there. From quick look also pca954x driver is not reading it.
Indeed.
Should a i2c bus behind a mux not be able to set his own speed?
Not sure if that make sense but Linux will likely ignore it. I
am not
saying it doesn't make sense but I haven't seen this feature.
Ok.
But may as a feature (or bugfix?) if "clock-frequency" is not
there,
use the speed of the parent bus...?
I was thinking about this too.
just c&p quick implementation would look like this. Because it is
i2c->i2c_mux->i2c. But maybe there is a better way how to do it.
diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
index 2aa3efe8aaa0..982c467deba3 100644
--- a/drivers/i2c/i2c-uclass.c
+++ b/drivers/i2c/i2c-uclass.c
@@ -640,9 +640,21 @@ static int i2c_post_probe(struct udevice *dev)
{
#if CONFIG_IS_ENABLED(OF_CONTROL) &&
!CONFIG_IS_ENABLED(OF_PLATDATA)
struct dm_i2c_bus *i2c = dev_get_uclass_priv(dev);
+ int parent_speed = I2C_SPEED_STANDARD_RATE;
+
+ if (dev->parent &&
+ device_get_uclass_id(dev->parent) == UCLASS_I2C_MUX &&
+ dev->parent->parent &&
+ device_get_uclass_id(dev->parent->parent) ==
UCLASS_I2C) {
+ struct dm_i2c_bus *i2c_parent;
+
+ i2c_parent =
dev_get_uclass_priv(dev->parent->parent);
+ parent_speed = i2c_parent->speed_hz;
+ /* Not sure if make sense to check that
parent_speed is
not 0 */
I think this check is not needed.
+ }
i2c->speed_hz = dev_read_u32_default(dev,
"clock-frequency",
-
I2C_SPEED_STANDARD_RATE);
+ parent_speed);
Wow, a big if ... may this is clearer (not compile tested)?
udevice *parent = dev_get_parent(dev);
if (parent && device_get_uclass_id(parent) == UCLASS_I2C_MUX) {
udevice *parent2 = dev_get_parent(parent);
if (parent2 && device_get_uclass_id(parent2) == UCLASS_I2C) {
struct dm_i2c_bus *i2cp = dev_get_uclass_priv(parent2);
parent_speed = i2cp->speed_hz;
}
}
but Simon has a deeper DM knowledge, may there is a better approach.
Simon: any comment on this one?
Simon: Can you please comment this?
OK will take a look.
I wonder if i2c-mux-uclass.c should define a new uclass for muxed I2C
buses, something like UCLASS_I2C_MUXED_BUS? Then you can define the
behaviour correctly in i2c-mux-uclass.c.
An I2C controller is not the same as a muxed bus and perhaps we should
be explicitly about the differences. It probably just needs changes to
the mux uclass.
Definitely there need to be some changes in connection to i2c muxes. I
am aware also about bad behavior when you detect devices.
Just look at log below and you will see that devices on base i2c bus are
copied also to subbus (especially listing i2c-mux again looks weird).
Yes, this look like we need here a seperate uclass to handle this
correct...
Are you going to invest to your time to create?
Unfortunately I have no time to look into this soon, and I must search
for a hardware to test...
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de
