From patchwork Fri Apr 10 06:46:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michal Simek X-Patchwork-Id: 237623 List-Id: U-Boot discussion From: michal.simek at xilinx.com (Michal Simek) Date: Fri, 10 Apr 2020 08:46:58 +0200 Subject: Calling i2c set speed twice for i2c_mux_bus In-Reply-To: References: Message-ID: 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. > 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. > 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. #else Thanks, Michal 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 */ + } i2c->speed_hz = dev_read_u32_default(dev, "clock-frequency", - I2C_SPEED_STANDARD_RATE); + parent_speed); return dm_i2c_set_bus_speed(dev, i2c->speed_hz);