* [PATCH 1/3] i3c: fix i2c and i3c scl rate by bus mode [not found] <cover.1555350118.git.vitor.soares@synopsys.com> @ 2019-04-15 18:42 ` Vitor Soares 2019-04-15 18:46 ` Vitor Soares 2019-04-16 5:50 ` Boris Brezillon 2019-04-15 18:42 ` [PATCH 2/3] i3c: add mixed limited " Vitor Soares 2019-04-15 18:42 ` [PATCH 3/3] i3c: dw: Add limited bus mode support Vitor Soares 2 siblings, 2 replies; 17+ messages in thread From: Vitor Soares @ 2019-04-15 18:42 UTC (permalink / raw) To: vitor.soares; +Cc: Vitor Soares, Boris Brezillon, stable, linux-kernel Currently in case of mixed slow bus topologie and all i2c devices support FM+ speed, the i3c subsystem limite the SCL to FM speed. Also in case on mixed slow bus mode the max speed for both i2c or i3c transfers is FM or FM+. This patch fix the definition of i2c and i3c scl rate based on bus topologie and LVR[4] if no user input. In case of mixed slow mode the i3c scl rate is overridden. Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure") Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> Cc: Boris Brezillon <bbrezillon@kernel.org> Cc: <stable@vger.kernel.org> Cc: <linux-kernel@vger.kernel.org> --- drivers/i3c/master.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index 909c2ad..1c4a86a 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -564,20 +564,30 @@ static const struct device_type i3c_masterdev_type = { .groups = i3c_masterdev_groups, }; -int i3c_bus_set_mode(struct i3c_bus *i3cbus, enum i3c_bus_mode mode) +int i3c_bus_set_mode(struct i3c_bus *i3cbus, enum i3c_bus_mode mode, + unsigned long i2c_scl_rate) { i3cbus->mode = mode; - if (!i3cbus->scl_rate.i3c) - i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; - - if (!i3cbus->scl_rate.i2c) { - if (i3cbus->mode == I3C_BUS_MODE_MIXED_SLOW) - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_SCL_RATE; - else - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_PLUS_SCL_RATE; + switch (i3cbus->mode) { + case I3C_BUS_MODE_PURE: + if (!i3cbus->scl_rate.i3c) + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; + break; + case I3C_BUS_MODE_MIXED_FAST: + if (!i3cbus->scl_rate.i3c) + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; + if (!i3cbus->scl_rate.i2c) + i3cbus->scl_rate.i2c = i2c_scl_rate; + break; + case I3C_BUS_MODE_MIXED_SLOW: + if (!i3cbus->scl_rate.i2c) + i3cbus->scl_rate.i2c = i2c_scl_rate; + i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; + break; + default: + return -EINVAL; } - /* * I3C/I2C frequency may have been overridden, check that user-provided * values are not exceeding max possible frequency. @@ -1980,9 +1990,6 @@ of_i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master, /* LVR is encoded in reg[2]. */ boardinfo->lvr = reg[2]; - if (boardinfo->lvr & I3C_LVR_I2C_FM_MODE) - master->bus.scl_rate.i2c = I3C_BUS_I2C_FM_SCL_RATE; - list_add_tail(&boardinfo->node, &master->boardinfo.i2c); of_node_get(node); @@ -2432,6 +2439,7 @@ int i3c_master_register(struct i3c_master_controller *master, const struct i3c_master_controller_ops *ops, bool secondary) { + unsigned long i2c_scl_rate = I3C_BUS_I2C_FM_PLUS_SCL_RATE; struct i3c_bus *i3cbus = i3c_master_get_bus(master); enum i3c_bus_mode mode = I3C_BUS_MODE_PURE; struct i2c_dev_boardinfo *i2cbi; @@ -2481,9 +2489,12 @@ int i3c_master_register(struct i3c_master_controller *master, ret = -EINVAL; goto err_put_dev; } + + if (i2cbi->lvr & I3C_LVR_I2C_FM_MODE) + i2c_scl_rate = I3C_BUS_I2C_FM_SCL_RATE; } - ret = i3c_bus_set_mode(i3cbus, mode); + ret = i3c_bus_set_mode(i3cbus, mode, i2c_scl_rate); if (ret) goto err_put_dev; -- 2.7.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 1/3] i3c: fix i2c and i3c scl rate by bus mode 2019-04-15 18:42 ` [PATCH 1/3] i3c: fix i2c and i3c scl rate by bus mode Vitor Soares @ 2019-04-15 18:46 ` Vitor Soares 2019-04-16 5:50 ` Boris Brezillon 1 sibling, 0 replies; 17+ messages in thread From: Vitor Soares @ 2019-04-15 18:46 UTC (permalink / raw) To: linux-i3c; +Cc: joao.pinto, Vitor Soares, Boris Brezillon, stable, linux-kernel Currently in case of mixed slow bus topologie and all i2c devices support FM+ speed, the i3c subsystem limite the SCL to FM speed. Also in case on mixed slow bus mode the max speed for both i2c or i3c transfers is FM or FM+. This patch fix the definition of i2c and i3c scl rate based on bus topologie and LVR[4] if no user input. In case of mixed slow mode the i3c scl rate is overridden. Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure") Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> Cc: Boris Brezillon <bbrezillon@kernel.org> Cc: <stable@vger.kernel.org> Cc: <linux-kernel@vger.kernel.org> --- drivers/i3c/master.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index 909c2ad..1c4a86a 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -564,20 +564,30 @@ static const struct device_type i3c_masterdev_type = { .groups = i3c_masterdev_groups, }; -int i3c_bus_set_mode(struct i3c_bus *i3cbus, enum i3c_bus_mode mode) +int i3c_bus_set_mode(struct i3c_bus *i3cbus, enum i3c_bus_mode mode, + unsigned long i2c_scl_rate) { i3cbus->mode = mode; - if (!i3cbus->scl_rate.i3c) - i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; - - if (!i3cbus->scl_rate.i2c) { - if (i3cbus->mode == I3C_BUS_MODE_MIXED_SLOW) - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_SCL_RATE; - else - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_PLUS_SCL_RATE; + switch (i3cbus->mode) { + case I3C_BUS_MODE_PURE: + if (!i3cbus->scl_rate.i3c) + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; + break; + case I3C_BUS_MODE_MIXED_FAST: + if (!i3cbus->scl_rate.i3c) + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; + if (!i3cbus->scl_rate.i2c) + i3cbus->scl_rate.i2c = i2c_scl_rate; + break; + case I3C_BUS_MODE_MIXED_SLOW: + if (!i3cbus->scl_rate.i2c) + i3cbus->scl_rate.i2c = i2c_scl_rate; + i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; + break; + default: + return -EINVAL; } - /* * I3C/I2C frequency may have been overridden, check that user-provided * values are not exceeding max possible frequency. @@ -1980,9 +1990,6 @@ of_i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master, /* LVR is encoded in reg[2]. */ boardinfo->lvr = reg[2]; - if (boardinfo->lvr & I3C_LVR_I2C_FM_MODE) - master->bus.scl_rate.i2c = I3C_BUS_I2C_FM_SCL_RATE; - list_add_tail(&boardinfo->node, &master->boardinfo.i2c); of_node_get(node); @@ -2432,6 +2439,7 @@ int i3c_master_register(struct i3c_master_controller *master, const struct i3c_master_controller_ops *ops, bool secondary) { + unsigned long i2c_scl_rate = I3C_BUS_I2C_FM_PLUS_SCL_RATE; struct i3c_bus *i3cbus = i3c_master_get_bus(master); enum i3c_bus_mode mode = I3C_BUS_MODE_PURE; struct i2c_dev_boardinfo *i2cbi; @@ -2481,9 +2489,12 @@ int i3c_master_register(struct i3c_master_controller *master, ret = -EINVAL; goto err_put_dev; } + + if (i2cbi->lvr & I3C_LVR_I2C_FM_MODE) + i2c_scl_rate = I3C_BUS_I2C_FM_SCL_RATE; } - ret = i3c_bus_set_mode(i3cbus, mode); + ret = i3c_bus_set_mode(i3cbus, mode, i2c_scl_rate); if (ret) goto err_put_dev; -- 2.7.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] i3c: fix i2c and i3c scl rate by bus mode 2019-04-15 18:42 ` [PATCH 1/3] i3c: fix i2c and i3c scl rate by bus mode Vitor Soares 2019-04-15 18:46 ` Vitor Soares @ 2019-04-16 5:50 ` Boris Brezillon 2019-04-16 14:24 ` Vitor Soares 1 sibling, 1 reply; 17+ messages in thread From: Boris Brezillon @ 2019-04-16 5:50 UTC (permalink / raw) To: Vitor Soares; +Cc: linux-i3c, joao.pinto, Boris Brezillon, stable, linux-kernel On Mon, 15 Apr 2019 20:46:41 +0200 Vitor Soares <vitor.soares@synopsys.com> wrote: > Currently in case of mixed slow bus topologie and all i2c devices > support FM+ speed, the i3c subsystem limite the SCL to FM speed. " Currently the I3C framework limits SCL frequency to FM speed when dealing with a mixed slow bus, even if all I2C devices are FM+ capable. " > Also in case on mixed slow bus mode the max speed for both > i2c or i3c transfers is FM or FM+. Looks like you're basically repeating what you said above. > > This patch fix the definition of i2c and i3c scl rate based on bus ^fixes on the bus > topologie and LVR[4] if no user input. ^topology ^if the rate is not already specified by the user. > In case of mixed slow mode the i3c scl rate is overridden. ^ with the max I2C rate. > > Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure") > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> > Cc: Boris Brezillon <bbrezillon@kernel.org> > Cc: <stable@vger.kernel.org> > Cc: <linux-kernel@vger.kernel.org> > --- > drivers/i3c/master.c | 39 +++++++++++++++++++++++++-------------- > 1 file changed, 25 insertions(+), 14 deletions(-) > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > index 909c2ad..1c4a86a 100644 > --- a/drivers/i3c/master.c > +++ b/drivers/i3c/master.c > @@ -564,20 +564,30 @@ static const struct device_type i3c_masterdev_type = { > .groups = i3c_masterdev_groups, > }; > > -int i3c_bus_set_mode(struct i3c_bus *i3cbus, enum i3c_bus_mode mode) > +int i3c_bus_set_mode(struct i3c_bus *i3cbus, enum i3c_bus_mode mode, > + unsigned long i2c_scl_rate) Can we rename the last arg into max_i2c_scl_rate? > { > i3cbus->mode = mode; > > - if (!i3cbus->scl_rate.i3c) > - i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > - > - if (!i3cbus->scl_rate.i2c) { > - if (i3cbus->mode == I3C_BUS_MODE_MIXED_SLOW) > - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_SCL_RATE; > - else > - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_PLUS_SCL_RATE; > + switch (i3cbus->mode) { > + case I3C_BUS_MODE_PURE: > + if (!i3cbus->scl_rate.i3c) > + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > + break; > + case I3C_BUS_MODE_MIXED_FAST: > + if (!i3cbus->scl_rate.i3c) > + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > + if (!i3cbus->scl_rate.i2c) > + i3cbus->scl_rate.i2c = i2c_scl_rate; > + break; > + case I3C_BUS_MODE_MIXED_SLOW: > + if (!i3cbus->scl_rate.i2c) > + i3cbus->scl_rate.i2c = i2c_scl_rate; > + i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; Maybe we should do if (!i3cbus->scl_rate.i3c || i3cbus->scl_rate.i3c > i3cbus->scl_rate.i2c) i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; Just in case the I3C rate forced by the user is lower than the max I2C rate. The patch looks good otherwise. > + break; > + default: > + return -EINVAL; > } > - > /* > * I3C/I2C frequency may have been overridden, check that user-provided > * values are not exceeding max possible frequency. > @@ -1980,9 +1990,6 @@ of_i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master, > /* LVR is encoded in reg[2]. */ > boardinfo->lvr = reg[2]; > > - if (boardinfo->lvr & I3C_LVR_I2C_FM_MODE) > - master->bus.scl_rate.i2c = I3C_BUS_I2C_FM_SCL_RATE; > - > list_add_tail(&boardinfo->node, &master->boardinfo.i2c); > of_node_get(node); > > @@ -2432,6 +2439,7 @@ int i3c_master_register(struct i3c_master_controller *master, > const struct i3c_master_controller_ops *ops, > bool secondary) > { > + unsigned long i2c_scl_rate = I3C_BUS_I2C_FM_PLUS_SCL_RATE; > struct i3c_bus *i3cbus = i3c_master_get_bus(master); > enum i3c_bus_mode mode = I3C_BUS_MODE_PURE; > struct i2c_dev_boardinfo *i2cbi; > @@ -2481,9 +2489,12 @@ int i3c_master_register(struct i3c_master_controller *master, > ret = -EINVAL; > goto err_put_dev; > } > + > + if (i2cbi->lvr & I3C_LVR_I2C_FM_MODE) > + i2c_scl_rate = I3C_BUS_I2C_FM_SCL_RATE; > } > > - ret = i3c_bus_set_mode(i3cbus, mode); > + ret = i3c_bus_set_mode(i3cbus, mode, i2c_scl_rate); > if (ret) > goto err_put_dev; > ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 1/3] i3c: fix i2c and i3c scl rate by bus mode 2019-04-16 5:50 ` Boris Brezillon @ 2019-04-16 14:24 ` Vitor Soares 2019-04-16 14:52 ` Boris Brezillon 0 siblings, 1 reply; 17+ messages in thread From: Vitor Soares @ 2019-04-16 14:24 UTC (permalink / raw) To: Boris Brezillon, Vitor Soares Cc: linux-i3c, joao.pinto, Boris Brezillon, stable, linux-kernel Hi Boris, From: Boris Brezillon <boris.brezillon@collabora.com> Date: Tue, Apr 16, 2019 at 06:50:41 > On Mon, 15 Apr 2019 20:46:41 +0200 > Vitor Soares <vitor.soares@synopsys.com> wrote: > > > Currently in case of mixed slow bus topologie and all i2c devices > > support FM+ speed, the i3c subsystem limite the SCL to FM speed. > I will it replace with your message below. > " > Currently the I3C framework limits SCL frequency to FM speed when > dealing with a mixed slow bus, even if all I2C devices are FM+ capable. > " > > > Also in case on mixed slow bus mode the max speed for both > > i2c or i3c transfers is FM or FM+. > > Looks like you're basically repeating what you said above. What I meant was that I3C framework isn't limiting the I3C speed in case of mixed slow bus. > > > > > This patch fix the definition of i2c and i3c scl rate based on bus > > ^fixes on the bus > > > topologie and LVR[4] if no user input. > > ^topology ^if the rate is not already specified by the user. > > > In case of mixed slow mode the i3c scl rate is overridden. > > ^ with the max > I2C rate. > > > > > Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure") > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> > > Cc: Boris Brezillon <bbrezillon@kernel.org> > > Cc: <stable@vger.kernel.org> > > Cc: <linux-kernel@vger.kernel.org> > > --- > > drivers/i3c/master.c | 39 +++++++++++++++++++++++++-------------- > > 1 file changed, 25 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > > index 909c2ad..1c4a86a 100644 > > --- a/drivers/i3c/master.c > > +++ b/drivers/i3c/master.c > > @@ -564,20 +564,30 @@ static const struct device_type i3c_masterdev_type = { > > .groups = i3c_masterdev_groups, > > }; > > > > -int i3c_bus_set_mode(struct i3c_bus *i3cbus, enum i3c_bus_mode mode) > > +int i3c_bus_set_mode(struct i3c_bus *i3cbus, enum i3c_bus_mode mode, > > + unsigned long i2c_scl_rate) > > > Can we rename the last arg into max_i2c_scl_rate? The i2c_scl_rate is base on LVR[4] bit and the user can set a higher scl rate, this is reason I didn't named it to max_i2c_scl_rate. But if you think that make more sense I'm ok with that. > > > { > > i3cbus->mode = mode; > > > > - if (!i3cbus->scl_rate.i3c) > > - i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > - > > - if (!i3cbus->scl_rate.i2c) { > > - if (i3cbus->mode == I3C_BUS_MODE_MIXED_SLOW) > > - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_SCL_RATE; > > - else > > - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_PLUS_SCL_RATE; > > + switch (i3cbus->mode) { > > + case I3C_BUS_MODE_PURE: > > + if (!i3cbus->scl_rate.i3c) > > + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > + break; > > + case I3C_BUS_MODE_MIXED_FAST: > > + if (!i3cbus->scl_rate.i3c) > > + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > + if (!i3cbus->scl_rate.i2c) > > + i3cbus->scl_rate.i2c = i2c_scl_rate; > > + break; > > + case I3C_BUS_MODE_MIXED_SLOW: > > + if (!i3cbus->scl_rate.i2c) > > + i3cbus->scl_rate.i2c = i2c_scl_rate; > > + i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; > > Maybe we should do > > if (!i3cbus->scl_rate.i3c || > i3cbus->scl_rate.i3c > i3cbus->scl_rate.i2c) > i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; > > Just in case the I3C rate forced by the user is lower than the max I2C > rate. That was something that I considered but TBH it isn't a real use case. > > The patch looks good otherwise. Thanks. > > > + break; > > + default: > > + return -EINVAL; > > } > > - > > /* > > * I3C/I2C frequency may have been overridden, check that user-provided > > * values are not exceeding max possible frequency. > > @@ -1980,9 +1990,6 @@ of_i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master, > > /* LVR is encoded in reg[2]. */ > > boardinfo->lvr = reg[2]; > > > > - if (boardinfo->lvr & I3C_LVR_I2C_FM_MODE) > > - master->bus.scl_rate.i2c = I3C_BUS_I2C_FM_SCL_RATE; > > - > > list_add_tail(&boardinfo->node, &master->boardinfo.i2c); > > of_node_get(node); > > > > @@ -2432,6 +2439,7 @@ int i3c_master_register(struct i3c_master_controller *master, > > const struct i3c_master_controller_ops *ops, > > bool secondary) > > { > > + unsigned long i2c_scl_rate = I3C_BUS_I2C_FM_PLUS_SCL_RATE; > > struct i3c_bus *i3cbus = i3c_master_get_bus(master); > > enum i3c_bus_mode mode = I3C_BUS_MODE_PURE; > > struct i2c_dev_boardinfo *i2cbi; > > @@ -2481,9 +2489,12 @@ int i3c_master_register(struct i3c_master_controller *master, > > ret = -EINVAL; > > goto err_put_dev; > > } > > + > > + if (i2cbi->lvr & I3C_LVR_I2C_FM_MODE) > > + i2c_scl_rate = I3C_BUS_I2C_FM_SCL_RATE; > > } > > > > - ret = i3c_bus_set_mode(i3cbus, mode); > > + ret = i3c_bus_set_mode(i3cbus, mode, i2c_scl_rate); > > if (ret) > > goto err_put_dev; > > Best regards, Vitor Soares ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] i3c: fix i2c and i3c scl rate by bus mode 2019-04-16 14:24 ` Vitor Soares @ 2019-04-16 14:52 ` Boris Brezillon 2019-04-22 15:54 ` Vitor Soares 0 siblings, 1 reply; 17+ messages in thread From: Boris Brezillon @ 2019-04-16 14:52 UTC (permalink / raw) To: Vitor Soares; +Cc: linux-i3c, joao.pinto, Boris Brezillon, stable, linux-kernel On Tue, 16 Apr 2019 14:24:57 +0000 Vitor Soares <vitor.soares@synopsys.com> wrote: > Hi Boris, > > From: Boris Brezillon <boris.brezillon@collabora.com> > Date: Tue, Apr 16, 2019 at 06:50:41 > > > On Mon, 15 Apr 2019 20:46:41 +0200 > > Vitor Soares <vitor.soares@synopsys.com> wrote: > > > > > Currently in case of mixed slow bus topologie and all i2c devices > > > support FM+ speed, the i3c subsystem limite the SCL to FM speed. > > > > I will it replace with your message below. > > > " > > Currently the I3C framework limits SCL frequency to FM speed when > > dealing with a mixed slow bus, even if all I2C devices are FM+ capable. > > " > > > > > Also in case on mixed slow bus mode the max speed for both > > > i2c or i3c transfers is FM or FM+. > > > > Looks like you're basically repeating what you said above. > > What I meant was that I3C framework isn't limiting the I3C speed in case > of mixed slow bus. Oh, okay, then maybe something like " The core was also not accounting for I3C speed limitations when operating in mixed slow mode and was erroneously using FM+ speed as the max I2C speed when operating in mixed fast mode. " > > > > > > > > > This patch fix the definition of i2c and i3c scl rate based on bus > > > > ^fixes on the bus > > > > > topologie and LVR[4] if no user input. > > > > ^topology ^if the rate is not already specified by the user. > > > > > In case of mixed slow mode the i3c scl rate is overridden. > > > > ^ with the max > > I2C rate. > > > > > > > > Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure") > > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> > > > Cc: Boris Brezillon <bbrezillon@kernel.org> > > > Cc: <stable@vger.kernel.org> > > > Cc: <linux-kernel@vger.kernel.org> > > > --- > > > drivers/i3c/master.c | 39 +++++++++++++++++++++++++-------------- > > > 1 file changed, 25 insertions(+), 14 deletions(-) > > > > > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > > > index 909c2ad..1c4a86a 100644 > > > --- a/drivers/i3c/master.c > > > +++ b/drivers/i3c/master.c > > > @@ -564,20 +564,30 @@ static const struct device_type i3c_masterdev_type = { > > > .groups = i3c_masterdev_groups, > > > }; > > > > > > -int i3c_bus_set_mode(struct i3c_bus *i3cbus, enum i3c_bus_mode mode) > > > +int i3c_bus_set_mode(struct i3c_bus *i3cbus, enum i3c_bus_mode mode, > > > + unsigned long i2c_scl_rate) > > > > > > Can we rename the last arg into max_i2c_scl_rate? > > The i2c_scl_rate is base on LVR[4] bit and the user can set a higher scl > rate, this is reason I didn't named it to max_i2c_scl_rate. > But if you think that make more sense I'm ok with that. In this context it does encode the maximum rate allowed by the spec (based on LVR parsing), so max_i2c_rate sounds like a correct name to me. > > > > > > { > > > i3cbus->mode = mode; > > > > > > - if (!i3cbus->scl_rate.i3c) > > > - i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > - > > > - if (!i3cbus->scl_rate.i2c) { > > > - if (i3cbus->mode == I3C_BUS_MODE_MIXED_SLOW) > > > - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_SCL_RATE; > > > - else > > > - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_PLUS_SCL_RATE; > > > + switch (i3cbus->mode) { > > > + case I3C_BUS_MODE_PURE: > > > + if (!i3cbus->scl_rate.i3c) > > > + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > + break; > > > + case I3C_BUS_MODE_MIXED_FAST: > > > + if (!i3cbus->scl_rate.i3c) > > > + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > + if (!i3cbus->scl_rate.i2c) > > > + i3cbus->scl_rate.i2c = i2c_scl_rate; > > > + break; > > > + case I3C_BUS_MODE_MIXED_SLOW: > > > + if (!i3cbus->scl_rate.i2c) > > > + i3cbus->scl_rate.i2c = i2c_scl_rate; > > > + i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; > > > > Maybe we should do > > > > if (!i3cbus->scl_rate.i3c || > > i3cbus->scl_rate.i3c > i3cbus->scl_rate.i2c) > > i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; > > > > Just in case the I3C rate forced by the user is lower than the max I2C > > rate. > > That was something that I considered but TBH it isn't a real use case. Add a WARN_ON() to at least catch such inconsistencies. And maybe we should add a dev_warn() when the user-defined rates do not match the mode/LVR constraints. It's easy to do a mistake when writing a dts. ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 1/3] i3c: fix i2c and i3c scl rate by bus mode 2019-04-16 14:52 ` Boris Brezillon @ 2019-04-22 15:54 ` Vitor Soares 2019-04-22 16:07 ` Boris Brezillon 0 siblings, 1 reply; 17+ messages in thread From: Vitor Soares @ 2019-04-22 15:54 UTC (permalink / raw) To: Boris Brezillon, Vitor Soares Cc: linux-i3c, joao.pinto@synopsys.com, Boris Brezillon, stable, linux-kernel Hi Boris, From: Boris Brezillon <boris.brezillon@collabora.com> Date: Tue, Apr 16, 2019 at 15:52:50 > On Tue, 16 Apr 2019 14:24:57 +0000 > Vitor Soares <vitor.soares@synopsys.com> wrote: > > > Hi Boris, > > > > From: Boris Brezillon <boris.brezillon@collabora.com> > > Date: Tue, Apr 16, 2019 at 06:50:41 > > > > > On Mon, 15 Apr 2019 20:46:41 +0200 > > > Vitor Soares <vitor.soares@synopsys.com> wrote: > > > > > > > Currently in case of mixed slow bus topologie and all i2c devices > > > > support FM+ speed, the i3c subsystem limite the SCL to FM speed. > > > > > > > I will it replace with your message below. > > > > > " > > > Currently the I3C framework limits SCL frequency to FM speed when > > > dealing with a mixed slow bus, even if all I2C devices are FM+ capable. > > > " > > > > > > > Also in case on mixed slow bus mode the max speed for both > > > > i2c or i3c transfers is FM or FM+. > > > > > > Looks like you're basically repeating what you said above. > > > > What I meant was that I3C framework isn't limiting the I3C speed in case > > of mixed slow bus. > > Oh, okay, then maybe something like > > " > The core was also not accounting for I3C speed limitations when > operating in mixed slow mode and was erroneously using FM+ speed as the > max I2C speed when operating in mixed fast mode. > " Sounds good to me. Thanks for the advise. > > > > > > > > > > > > > > This patch fix the definition of i2c and i3c scl rate based on bus > > > > > > ^fixes on the bus > > > > > > > topologie and LVR[4] if no user input. > > > > > > ^topology ^if the rate is not already specified by the user. > > > > > > > In case of mixed slow mode the i3c scl rate is overridden. > > > > > > ^ with the max > > > I2C rate. > > > > > > > > > > > Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure") > > > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> > > > > Cc: Boris Brezillon <bbrezillon@kernel.org> > > > > Cc: <stable@vger.kernel.org> > > > > Cc: <linux-kernel@vger.kernel.org> > > > > --- > > > > drivers/i3c/master.c | 39 +++++++++++++++++++++++++-------------- > > > > 1 file changed, 25 insertions(+), 14 deletions(-) > > > > > > > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > > > > index 909c2ad..1c4a86a 100644 > > > > --- a/drivers/i3c/master.c > > > > +++ b/drivers/i3c/master.c > > > > @@ -564,20 +564,30 @@ static const struct device_type i3c_masterdev_type = { > > > > .groups = i3c_masterdev_groups, > > > > }; > > > > > > > > -int i3c_bus_set_mode(struct i3c_bus *i3cbus, enum i3c_bus_mode mode) > > > > +int i3c_bus_set_mode(struct i3c_bus *i3cbus, enum i3c_bus_mode mode, > > > > + unsigned long i2c_scl_rate) > > > > > > > > > Can we rename the last arg into max_i2c_scl_rate? > > > > The i2c_scl_rate is base on LVR[4] bit and the user can set a higher scl > > rate, this is reason I didn't named it to max_i2c_scl_rate. > > But if you think that make more sense I'm ok with that. > > In this context it does encode the maximum rate allowed by the spec > (based on LVR parsing), so max_i2c_rate sounds like a correct name to > me. > > > > > > > > > > { > > > > i3cbus->mode = mode; > > > > > > > > - if (!i3cbus->scl_rate.i3c) > > > > - i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > > - > > > > - if (!i3cbus->scl_rate.i2c) { > > > > - if (i3cbus->mode == I3C_BUS_MODE_MIXED_SLOW) > > > > - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_SCL_RATE; > > > > - else > > > > - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_PLUS_SCL_RATE; > > > > + switch (i3cbus->mode) { > > > > + case I3C_BUS_MODE_PURE: > > > > + if (!i3cbus->scl_rate.i3c) > > > > + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > > + break; > > > > + case I3C_BUS_MODE_MIXED_FAST: > > > > + if (!i3cbus->scl_rate.i3c) > > > > + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > > + if (!i3cbus->scl_rate.i2c) > > > > + i3cbus->scl_rate.i2c = i2c_scl_rate; > > > > + break; > > > > + case I3C_BUS_MODE_MIXED_SLOW: > > > > + if (!i3cbus->scl_rate.i2c) > > > > + i3cbus->scl_rate.i2c = i2c_scl_rate; > > > > + i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; > > > > > > Maybe we should do > > > > > > if (!i3cbus->scl_rate.i3c || > > > i3cbus->scl_rate.i3c > i3cbus->scl_rate.i2c) > > > i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; > > > > > > Just in case the I3C rate forced by the user is lower than the max I2C > > > rate. > > > > That was something that I considered but TBH it isn't a real use case. > > Add a WARN_ON() to at least catch such inconsistencies. And maybe we > should add a dev_warn() when the user-defined rates do not match > the mode/LVR constraints. It's easy to do a mistake when writing a dts. I think the WARN_ON() is too evasive on the screen and won't provide the information we want. The dev_warn() should work perfectly here. if (i3cbus->scl_rate.i3c < i3cbus->scl_rate.i2c) dev_warn(&i3cbus->cur_master->dev->dev, "%s: i3c-scl-hz lower then i2c-scl-hz\n", __func__); if (i3cbus->scl_rate.i2c != I3C_BUS_I2C_FM_SCL_RATE || i3cbus->scl_rate.i2c != I3C_BUS_I2C_FM_PLUS_SCL_RATE) dev_warn(&i3cbus->cur_master->dev->dev, "%s: i2c-scl-hz not defined according MIPI I3C spec\n", __func__); Maybe it make more sense to do this check on of_populate_i3c_bus(), what do you think? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] i3c: fix i2c and i3c scl rate by bus mode 2019-04-22 15:54 ` Vitor Soares @ 2019-04-22 16:07 ` Boris Brezillon 2019-04-22 17:54 ` Vitor Soares 0 siblings, 1 reply; 17+ messages in thread From: Boris Brezillon @ 2019-04-22 16:07 UTC (permalink / raw) To: Vitor Soares Cc: linux-i3c, joao.pinto@synopsys.com, Boris Brezillon, stable, linux-kernel On Mon, 22 Apr 2019 15:54:33 +0000 Vitor Soares <Vitor.Soares@synopsys.com> wrote: > > > > > > > > > { > > > > > i3cbus->mode = mode; > > > > > > > > > > - if (!i3cbus->scl_rate.i3c) > > > > > - i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > > > - > > > > > - if (!i3cbus->scl_rate.i2c) { > > > > > - if (i3cbus->mode == I3C_BUS_MODE_MIXED_SLOW) > > > > > - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_SCL_RATE; > > > > > - else > > > > > - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_PLUS_SCL_RATE; > > > > > + switch (i3cbus->mode) { > > > > > + case I3C_BUS_MODE_PURE: > > > > > + if (!i3cbus->scl_rate.i3c) > > > > > + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > > > + break; > > > > > + case I3C_BUS_MODE_MIXED_FAST: > > > > > + if (!i3cbus->scl_rate.i3c) > > > > > + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > > > + if (!i3cbus->scl_rate.i2c) > > > > > + i3cbus->scl_rate.i2c = i2c_scl_rate; > > > > > + break; > > > > > + case I3C_BUS_MODE_MIXED_SLOW: > > > > > + if (!i3cbus->scl_rate.i2c) > > > > > + i3cbus->scl_rate.i2c = i2c_scl_rate; > > > > > + i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; > > > > > > > > Maybe we should do > > > > > > > > if (!i3cbus->scl_rate.i3c || > > > > i3cbus->scl_rate.i3c > i3cbus->scl_rate.i2c) > > > > i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; > > > > > > > > Just in case the I3C rate forced by the user is lower than the max I2C > > > > rate. > > > > > > That was something that I considered but TBH it isn't a real use case. > > > > Add a WARN_ON() to at least catch such inconsistencies. And maybe we > > should add a dev_warn() when the user-defined rates do not match > > the mode/LVR constraints. It's easy to do a mistake when writing a dts. > > I think the WARN_ON() is too evasive on the screen and won't provide the > information we want. > The dev_warn() should work perfectly here. > > if (i3cbus->scl_rate.i3c < i3cbus->scl_rate.i2c) > dev_warn(&i3cbus->cur_master->dev->dev, > "%s: i3c-scl-hz lower then i2c-scl-hz\n", __func__); Using dev_warn() sounds good, though I don't think you need the __func__ here. Also, please print the i2c/i3c rates in the message, and align the second line on the open parens. > if (i3cbus->scl_rate.i2c != I3C_BUS_I2C_FM_SCL_RATE || > i3cbus->scl_rate.i2c != I3C_BUS_I2C_FM_PLUS_SCL_RATE) > dev_warn(&i3cbus->cur_master->dev->dev, > "%s: i2c-scl-hz not defined according MIPI I3C spec\n", > __func__); Is that really a problem? Having an i2c rate that is less than FM speed sounds like a valid case to me. > > Maybe it make more sense to do this check on of_populate_i3c_bus(), what > do you think? > No, we really want to have this check here, because we might support other HW description formats at some point (board-files, ACPI, ...). ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 1/3] i3c: fix i2c and i3c scl rate by bus mode 2019-04-22 16:07 ` Boris Brezillon @ 2019-04-22 17:54 ` Vitor Soares 2019-04-22 18:27 ` Boris Brezillon 0 siblings, 1 reply; 17+ messages in thread From: Vitor Soares @ 2019-04-22 17:54 UTC (permalink / raw) To: Boris Brezillon, Vitor Soares Cc: linux-i3c, joao.pinto@synopsys.com, Boris Brezillon, stable, linux-kernel From: Boris Brezillon <boris.brezillon@collabora.com> Date: Mon, Apr 22, 2019 at 17:07:15 > On Mon, 22 Apr 2019 15:54:33 +0000 > Vitor Soares <Vitor.Soares@synopsys.com> wrote: > > > > > > > > > > > > > { > > > > > > i3cbus->mode = mode; > > > > > > > > > > > > - if (!i3cbus->scl_rate.i3c) > > > > > > - i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > > > > - > > > > > > - if (!i3cbus->scl_rate.i2c) { > > > > > > - if (i3cbus->mode == I3C_BUS_MODE_MIXED_SLOW) > > > > > > - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_SCL_RATE; > > > > > > - else > > > > > > - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_PLUS_SCL_RATE; > > > > > > + switch (i3cbus->mode) { > > > > > > + case I3C_BUS_MODE_PURE: > > > > > > + if (!i3cbus->scl_rate.i3c) > > > > > > + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > > > > + break; > > > > > > + case I3C_BUS_MODE_MIXED_FAST: > > > > > > + if (!i3cbus->scl_rate.i3c) > > > > > > + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > > > > + if (!i3cbus->scl_rate.i2c) > > > > > > + i3cbus->scl_rate.i2c = i2c_scl_rate; > > > > > > + break; > > > > > > + case I3C_BUS_MODE_MIXED_SLOW: > > > > > > + if (!i3cbus->scl_rate.i2c) > > > > > > + i3cbus->scl_rate.i2c = i2c_scl_rate; > > > > > > + i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; > > > > > > > > > > Maybe we should do > > > > > > > > > > if (!i3cbus->scl_rate.i3c || > > > > > i3cbus->scl_rate.i3c > i3cbus->scl_rate.i2c) > > > > > i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; > > > > > > > > > > Just in case the I3C rate forced by the user is lower than the max I2C > > > > > rate. > > > > > > > > That was something that I considered but TBH it isn't a real use case. > > > > > > Add a WARN_ON() to at least catch such inconsistencies. And maybe we > > > should add a dev_warn() when the user-defined rates do not match > > > the mode/LVR constraints. It's easy to do a mistake when writing a dts. > > > > I think the WARN_ON() is too evasive on the screen and won't provide the > > information we want. > > The dev_warn() should work perfectly here. > > > > if (i3cbus->scl_rate.i3c < i3cbus->scl_rate.i2c) > > dev_warn(&i3cbus->cur_master->dev->dev, > > "%s: i3c-scl-hz lower then i2c-scl-hz\n", __func__); > > Using dev_warn() sounds good, though I don't think you need the > __func__ here. Also, please print the i2c/i3c rates in the message, and > align the second line on the open parens. > > > if (i3cbus->scl_rate.i2c != I3C_BUS_I2C_FM_SCL_RATE || > > i3cbus->scl_rate.i2c != I3C_BUS_I2C_FM_PLUS_SCL_RATE) > > dev_warn(&i3cbus->cur_master->dev->dev, > > "%s: i2c-scl-hz not defined according MIPI I3C spec\n", > > __func__); > > Is that really a problem? Having an i2c rate that is less than FM speed > sounds like a valid case to me. I'm addressing the spec constrains. In the practice it can be SM or even HS, it depends on the interface. > > > > > Maybe it make more sense to do this check on of_populate_i3c_bus(), what > > do you think? > > > > No, we really want to have this check here, because we might support > other HW description formats at some point (board-files, ACPI, ...). Yes, you are right. I forgot that point. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] i3c: fix i2c and i3c scl rate by bus mode 2019-04-22 17:54 ` Vitor Soares @ 2019-04-22 18:27 ` Boris Brezillon 2019-04-22 19:59 ` Vitor Soares 0 siblings, 1 reply; 17+ messages in thread From: Boris Brezillon @ 2019-04-22 18:27 UTC (permalink / raw) To: Vitor Soares Cc: linux-i3c, joao.pinto@synopsys.com, Boris Brezillon, stable, linux-kernel On Mon, 22 Apr 2019 17:54:29 +0000 Vitor Soares <Vitor.Soares@synopsys.com> wrote: > > > > > > > > > > > > > { > > > > > > > i3cbus->mode = mode; > > > > > > > > > > > > > > - if (!i3cbus->scl_rate.i3c) > > > > > > > - i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > > > > > - > > > > > > > - if (!i3cbus->scl_rate.i2c) { > > > > > > > - if (i3cbus->mode == I3C_BUS_MODE_MIXED_SLOW) > > > > > > > - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_SCL_RATE; > > > > > > > - else > > > > > > > - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_PLUS_SCL_RATE; > > > > > > > + switch (i3cbus->mode) { > > > > > > > + case I3C_BUS_MODE_PURE: > > > > > > > + if (!i3cbus->scl_rate.i3c) > > > > > > > + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > > > > > + break; > > > > > > > + case I3C_BUS_MODE_MIXED_FAST: > > > > > > > + if (!i3cbus->scl_rate.i3c) > > > > > > > + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > > > > > + if (!i3cbus->scl_rate.i2c) > > > > > > > + i3cbus->scl_rate.i2c = i2c_scl_rate; > > > > > > > + break; > > > > > > > + case I3C_BUS_MODE_MIXED_SLOW: > > > > > > > + if (!i3cbus->scl_rate.i2c) > > > > > > > + i3cbus->scl_rate.i2c = i2c_scl_rate; > > > > > > > + i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; > > > > > > > > > > > > Maybe we should do > > > > > > > > > > > > if (!i3cbus->scl_rate.i3c || > > > > > > i3cbus->scl_rate.i3c > i3cbus->scl_rate.i2c) > > > > > > i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; > > > > > > > > > > > > Just in case the I3C rate forced by the user is lower than the max I2C > > > > > > rate. > > > > > > > > > > That was something that I considered but TBH it isn't a real use case. > > > > > > > > Add a WARN_ON() to at least catch such inconsistencies. And maybe we > > > > should add a dev_warn() when the user-defined rates do not match > > > > the mode/LVR constraints. It's easy to do a mistake when writing a dts. > > > > > > I think the WARN_ON() is too evasive on the screen and won't provide the > > > information we want. > > > The dev_warn() should work perfectly here. > > > > > > if (i3cbus->scl_rate.i3c < i3cbus->scl_rate.i2c) > > > dev_warn(&i3cbus->cur_master->dev->dev, > > > "%s: i3c-scl-hz lower then i2c-scl-hz\n", __func__); > > > > Using dev_warn() sounds good, though I don't think you need the > > __func__ here. Also, please print the i2c/i3c rates in the message, and > > align the second line on the open parens. > > > > > if (i3cbus->scl_rate.i2c != I3C_BUS_I2C_FM_SCL_RATE || > > > i3cbus->scl_rate.i2c != I3C_BUS_I2C_FM_PLUS_SCL_RATE) > > > dev_warn(&i3cbus->cur_master->dev->dev, > > > "%s: i2c-scl-hz not defined according MIPI I3C spec\n", > > > __func__); > > > > Is that really a problem? Having an i2c rate that is less than FM speed > > sounds like a valid case to me. > > I'm addressing the spec constrains. "Table 57 I3C Timing Requirements When Communicating With I2C Legacy Devices" says that freq can be between 0 and 400KHz when operating in slow(FM) mode. Yes, maximum rate when not specified otherwise is 400Khz, but the point of overloading the max I2C/I3C spec is to allow custom rates when the default/spec ones are not achievable, so I'm not sure complaining in that case is legitimate. We should definitely complain when one tries to set a maximum rate that is higher than what devices can do (i3cbus->scl_rate.i2c > max_i2c_scl_rate). Same goes for I3C communications, we shouldn't care when the forced rate is lower than what the bus is capable of, what's important is to complain when it's higher than what's supported. ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 1/3] i3c: fix i2c and i3c scl rate by bus mode 2019-04-22 18:27 ` Boris Brezillon @ 2019-04-22 19:59 ` Vitor Soares 0 siblings, 0 replies; 17+ messages in thread From: Vitor Soares @ 2019-04-22 19:59 UTC (permalink / raw) To: Boris Brezillon, Vitor Soares Cc: linux-i3c, joao.pinto@synopsys.com, Boris Brezillon, stable, linux-kernel From: Boris Brezillon <boris.brezillon@collabora.com> Date: Mon, Apr 22, 2019 at 19:27:32 > On Mon, 22 Apr 2019 17:54:29 +0000 > Vitor Soares <Vitor.Soares@synopsys.com> wrote: > > > > > > > > > > > > > > > > { > > > > > > > > i3cbus->mode = mode; > > > > > > > > > > > > > > > > - if (!i3cbus->scl_rate.i3c) > > > > > > > > - i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > > > > > > - > > > > > > > > - if (!i3cbus->scl_rate.i2c) { > > > > > > > > - if (i3cbus->mode == I3C_BUS_MODE_MIXED_SLOW) > > > > > > > > - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_SCL_RATE; > > > > > > > > - else > > > > > > > > - i3cbus->scl_rate.i2c = I3C_BUS_I2C_FM_PLUS_SCL_RATE; > > > > > > > > + switch (i3cbus->mode) { > > > > > > > > + case I3C_BUS_MODE_PURE: > > > > > > > > + if (!i3cbus->scl_rate.i3c) > > > > > > > > + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > > > > > > + break; > > > > > > > > + case I3C_BUS_MODE_MIXED_FAST: > > > > > > > > + if (!i3cbus->scl_rate.i3c) > > > > > > > > + i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > > > > > > > + if (!i3cbus->scl_rate.i2c) > > > > > > > > + i3cbus->scl_rate.i2c = i2c_scl_rate; > > > > > > > > + break; > > > > > > > > + case I3C_BUS_MODE_MIXED_SLOW: > > > > > > > > + if (!i3cbus->scl_rate.i2c) > > > > > > > > + i3cbus->scl_rate.i2c = i2c_scl_rate; > > > > > > > > + i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; > > > > > > > > > > > > > > Maybe we should do > > > > > > > > > > > > > > if (!i3cbus->scl_rate.i3c || > > > > > > > i3cbus->scl_rate.i3c > i3cbus->scl_rate.i2c) > > > > > > > i3cbus->scl_rate.i3c = i3cbus->scl_rate.i2c; > > > > > > > > > > > > > > Just in case the I3C rate forced by the user is lower than the max I2C > > > > > > > rate. > > > > > > > > > > > > That was something that I considered but TBH it isn't a real use case. > > > > > > > > > > Add a WARN_ON() to at least catch such inconsistencies. And maybe we > > > > > should add a dev_warn() when the user-defined rates do not match > > > > > the mode/LVR constraints. It's easy to do a mistake when writing a dts. > > > > > > > > I think the WARN_ON() is too evasive on the screen and won't provide the > > > > information we want. > > > > The dev_warn() should work perfectly here. > > > > > > > > if (i3cbus->scl_rate.i3c < i3cbus->scl_rate.i2c) > > > > dev_warn(&i3cbus->cur_master->dev->dev, > > > > "%s: i3c-scl-hz lower then i2c-scl-hz\n", __func__); > > > > > > Using dev_warn() sounds good, though I don't think you need the > > > __func__ here. Also, please print the i2c/i3c rates in the message, and > > > align the second line on the open parens. > > > > > > > if (i3cbus->scl_rate.i2c != I3C_BUS_I2C_FM_SCL_RATE || > > > > i3cbus->scl_rate.i2c != I3C_BUS_I2C_FM_PLUS_SCL_RATE) > > > > dev_warn(&i3cbus->cur_master->dev->dev, > > > > "%s: i2c-scl-hz not defined according MIPI I3C spec\n", > > > > __func__); > > > > > > Is that really a problem? Having an i2c rate that is less than FM speed > > > sounds like a valid case to me. > > > > I'm addressing the spec constrains. > > "Table 57 I3C Timing Requirements When Communicating With I2C Legacy > Devices" says that freq can be between 0 and 400KHz when operating in > slow(FM) mode. That is the definition of Fast-mode: "Fast-mode devices can receive and transmit at up to 400kbit/s" as per i2c spec. So this way the slaves can be connected to Standard Mode bus... you already know that. For the FM requirements in I3C bus, please refer to I3C FAQ Q2.2 and table 56 of I3C v1.0 basic spec. > Yes, maximum rate when not specified otherwise is > 400Khz, By default you will have always FM or FM+ due the LVR register be mandatory. > but the point of overloading the max I2C/I3C spec is to allow > custom rates when the default/spec ones are not achievable, so I'm not > sure complaining in that case is legitimate. > Well, if the users aren't able to achieve at least FM is because something is not correct. > We should definitely complain when one tries to set a maximum rate that > is higher than what devices can do (i3cbus->scl_rate.i2c > > max_i2c_scl_rate). > I can put another if() for such cases. > Same goes for I3C communications, we shouldn't care when the forced rate > is lower than what the bus is capable of, what's important is to > complain when it's higher than what's supported. The point is complain when scl_rate.i3c < scl_rate.i2c because it can be error from user. Best regards, Vitor Soares ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/3] i3c: add mixed limited bus mode [not found] <cover.1555350118.git.vitor.soares@synopsys.com> 2019-04-15 18:42 ` [PATCH 1/3] i3c: fix i2c and i3c scl rate by bus mode Vitor Soares @ 2019-04-15 18:42 ` Vitor Soares 2019-04-15 18:46 ` Vitor Soares 2019-04-16 6:00 ` Boris Brezillon 2019-04-15 18:42 ` [PATCH 3/3] i3c: dw: Add limited bus mode support Vitor Soares 2 siblings, 2 replies; 17+ messages in thread From: Vitor Soares @ 2019-04-15 18:42 UTC (permalink / raw) To: vitor.soares; +Cc: Vitor Soares, Boris Brezillon, linux-kernel The i3c bus spec define a bus configuration where the i2c devices doesn't have the 50ns filter yet they allow the SDR max speed. This patch introduce the limited bus mode so the users can use a higher speed on presence of i2c devices index 1. Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> Cc: Boris Brezillon <bbrezillon@kernel.org> Cc: <linux-kernel@vger.kernel.org> --- drivers/i3c/master.c | 5 +++++ include/linux/i3c/master.h | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index 1c4a86a..46d3774 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -463,6 +463,7 @@ static int i3c_bus_init(struct i3c_bus *i3cbus) static const char * const i3c_bus_mode_strings[] = { [I3C_BUS_MODE_PURE] = "pure", [I3C_BUS_MODE_MIXED_FAST] = "mixed-fast", + [I3C_BUS_MODE_MIXED_LIMITED] = "mixed-limited", [I3C_BUS_MODE_MIXED_SLOW] = "mixed-slow", }; @@ -575,6 +576,7 @@ int i3c_bus_set_mode(struct i3c_bus *i3cbus, enum i3c_bus_mode mode, i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; break; case I3C_BUS_MODE_MIXED_FAST: + case I3C_BUS_MODE_MIXED_LIMITED: if (!i3cbus->scl_rate.i3c) i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; if (!i3cbus->scl_rate.i2c) @@ -2481,6 +2483,9 @@ int i3c_master_register(struct i3c_master_controller *master, mode = I3C_BUS_MODE_MIXED_FAST; break; case I3C_LVR_I2C_INDEX(1): + if (mode < I3C_BUS_MODE_MIXED_LIMITED) + mode = I3C_BUS_MODE_MIXED_LIMITED; + break; case I3C_LVR_I2C_INDEX(2): if (mode < I3C_BUS_MODE_MIXED_SLOW) mode = I3C_BUS_MODE_MIXED_SLOW; diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h index 44fb3cf..740235e 100644 --- a/include/linux/i3c/master.h +++ b/include/linux/i3c/master.h @@ -250,12 +250,17 @@ struct i3c_device { * the bus. The only impact in this mode is that the * high SCL pulse has to stay below 50ns to trick I2C * devices when transmitting I3C frames + * @I3C_BUS_MODE_MIXED_LIMITED: I2C devices without 50ns spike filter are + * present on the bus. However they allows + * compliance up to the maximum SDR SCL clock + * frequency. * @I3C_BUS_MODE_MIXED_SLOW: I2C devices without 50ns spike filter are present * on the bus */ enum i3c_bus_mode { I3C_BUS_MODE_PURE, I3C_BUS_MODE_MIXED_FAST, + I3C_BUS_MODE_MIXED_LIMITED, I3C_BUS_MODE_MIXED_SLOW, }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/3] i3c: add mixed limited bus mode 2019-04-15 18:42 ` [PATCH 2/3] i3c: add mixed limited " Vitor Soares @ 2019-04-15 18:46 ` Vitor Soares 2019-04-16 6:00 ` Boris Brezillon 1 sibling, 0 replies; 17+ messages in thread From: Vitor Soares @ 2019-04-15 18:46 UTC (permalink / raw) To: linux-i3c; +Cc: joao.pinto, Vitor Soares, Boris Brezillon, linux-kernel The i3c bus spec define a bus configuration where the i2c devices doesn't have the 50ns filter yet they allow the SDR max speed. This patch introduce the limited bus mode so the users can use a higher speed on presence of i2c devices index 1. Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> Cc: Boris Brezillon <bbrezillon@kernel.org> Cc: <linux-kernel@vger.kernel.org> --- drivers/i3c/master.c | 5 +++++ include/linux/i3c/master.h | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index 1c4a86a..46d3774 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -463,6 +463,7 @@ static int i3c_bus_init(struct i3c_bus *i3cbus) static const char * const i3c_bus_mode_strings[] = { [I3C_BUS_MODE_PURE] = "pure", [I3C_BUS_MODE_MIXED_FAST] = "mixed-fast", + [I3C_BUS_MODE_MIXED_LIMITED] = "mixed-limited", [I3C_BUS_MODE_MIXED_SLOW] = "mixed-slow", }; @@ -575,6 +576,7 @@ int i3c_bus_set_mode(struct i3c_bus *i3cbus, enum i3c_bus_mode mode, i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; break; case I3C_BUS_MODE_MIXED_FAST: + case I3C_BUS_MODE_MIXED_LIMITED: if (!i3cbus->scl_rate.i3c) i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; if (!i3cbus->scl_rate.i2c) @@ -2481,6 +2483,9 @@ int i3c_master_register(struct i3c_master_controller *master, mode = I3C_BUS_MODE_MIXED_FAST; break; case I3C_LVR_I2C_INDEX(1): + if (mode < I3C_BUS_MODE_MIXED_LIMITED) + mode = I3C_BUS_MODE_MIXED_LIMITED; + break; case I3C_LVR_I2C_INDEX(2): if (mode < I3C_BUS_MODE_MIXED_SLOW) mode = I3C_BUS_MODE_MIXED_SLOW; diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h index 44fb3cf..740235e 100644 --- a/include/linux/i3c/master.h +++ b/include/linux/i3c/master.h @@ -250,12 +250,17 @@ struct i3c_device { * the bus. The only impact in this mode is that the * high SCL pulse has to stay below 50ns to trick I2C * devices when transmitting I3C frames + * @I3C_BUS_MODE_MIXED_LIMITED: I2C devices without 50ns spike filter are + * present on the bus. However they allows + * compliance up to the maximum SDR SCL clock + * frequency. * @I3C_BUS_MODE_MIXED_SLOW: I2C devices without 50ns spike filter are present * on the bus */ enum i3c_bus_mode { I3C_BUS_MODE_PURE, I3C_BUS_MODE_MIXED_FAST, + I3C_BUS_MODE_MIXED_LIMITED, I3C_BUS_MODE_MIXED_SLOW, }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] i3c: add mixed limited bus mode 2019-04-15 18:42 ` [PATCH 2/3] i3c: add mixed limited " Vitor Soares 2019-04-15 18:46 ` Vitor Soares @ 2019-04-16 6:00 ` Boris Brezillon 2019-04-16 14:27 ` Vitor Soares 1 sibling, 1 reply; 17+ messages in thread From: Boris Brezillon @ 2019-04-16 6:00 UTC (permalink / raw) To: Vitor Soares; +Cc: linux-i3c, joao.pinto, Boris Brezillon, linux-kernel Hi Vitor, On Mon, 15 Apr 2019 20:46:42 +0200 Vitor Soares <vitor.soares@synopsys.com> wrote: > The i3c bus spec define a bus configuration where the i2c devices ^defines I2C devices... > doesn't have the 50ns filter yet they allow the SDR max speed. ^don't ^ a 50ns filter but support SCL running at SDR max rate (12MHz). > > This patch introduce the limited bus mode so the users can use ^introduces ^ so that users > a higher speed on presence of i2c devices index 1. ^in > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> > Cc: Boris Brezillon <bbrezillon@kernel.org> > Cc: <linux-kernel@vger.kernel.org> > --- > drivers/i3c/master.c | 5 +++++ > include/linux/i3c/master.h | 5 +++++ > 2 files changed, 10 insertions(+) > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > index 1c4a86a..46d3774 100644 > --- a/drivers/i3c/master.c > +++ b/drivers/i3c/master.c > @@ -463,6 +463,7 @@ static int i3c_bus_init(struct i3c_bus *i3cbus) > static const char * const i3c_bus_mode_strings[] = { > [I3C_BUS_MODE_PURE] = "pure", > [I3C_BUS_MODE_MIXED_FAST] = "mixed-fast", > + [I3C_BUS_MODE_MIXED_LIMITED] = "mixed-limited", > [I3C_BUS_MODE_MIXED_SLOW] = "mixed-slow", > }; > > @@ -575,6 +576,7 @@ int i3c_bus_set_mode(struct i3c_bus *i3cbus, enum i3c_bus_mode mode, > i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > break; > case I3C_BUS_MODE_MIXED_FAST: > + case I3C_BUS_MODE_MIXED_LIMITED: > if (!i3cbus->scl_rate.i3c) > i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > if (!i3cbus->scl_rate.i2c) > @@ -2481,6 +2483,9 @@ int i3c_master_register(struct i3c_master_controller *master, > mode = I3C_BUS_MODE_MIXED_FAST; > break; > case I3C_LVR_I2C_INDEX(1): > + if (mode < I3C_BUS_MODE_MIXED_LIMITED) > + mode = I3C_BUS_MODE_MIXED_LIMITED; > + break; > case I3C_LVR_I2C_INDEX(2): > if (mode < I3C_BUS_MODE_MIXED_SLOW) > mode = I3C_BUS_MODE_MIXED_SLOW; > diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h > index 44fb3cf..740235e 100644 > --- a/include/linux/i3c/master.h > +++ b/include/linux/i3c/master.h > @@ -250,12 +250,17 @@ struct i3c_device { > * the bus. The only impact in this mode is that the > * high SCL pulse has to stay below 50ns to trick I2C > * devices when transmitting I3C frames > + * @I3C_BUS_MODE_MIXED_LIMITED: I2C devices without 50ns spike filter are > + * present on the bus. However they allows > + * compliance up to the maximum SDR SCL clock > + * frequency. However they support SCL clock running at maximum SDR rate (12.5MHz). > * @I3C_BUS_MODE_MIXED_SLOW: I2C devices without 50ns spike filter are present > * on the bus > */ > enum i3c_bus_mode { > I3C_BUS_MODE_PURE, > I3C_BUS_MODE_MIXED_FAST, > + I3C_BUS_MODE_MIXED_LIMITED, > I3C_BUS_MODE_MIXED_SLOW, > }; > The code itself looks good. Thanks, Boris ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 2/3] i3c: add mixed limited bus mode 2019-04-16 6:00 ` Boris Brezillon @ 2019-04-16 14:27 ` Vitor Soares 0 siblings, 0 replies; 17+ messages in thread From: Vitor Soares @ 2019-04-16 14:27 UTC (permalink / raw) To: Boris Brezillon, Vitor Soares Cc: linux-i3c, joao.pinto, Boris Brezillon, linux-kernel Hi Boris, From: Boris Brezillon <boris.brezillon@collabora.com> Date: Tue, Apr 16, 2019 at 07:00:49 > Hi Vitor, > > On Mon, 15 Apr 2019 20:46:42 +0200 > Vitor Soares <vitor.soares@synopsys.com> wrote: > > > The i3c bus spec define a bus configuration where the i2c devices > > ^defines I2C devices... > > > doesn't have the 50ns filter yet they allow the SDR max speed. > > ^don't ^ a 50ns filter but support SCL running at SDR max > rate (12MHz). > > > > > This patch introduce the limited bus mode so the users can use > > ^introduces ^ so that users > > > a higher speed on presence of i2c devices index 1. > > ^in > > > > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> > > Cc: Boris Brezillon <bbrezillon@kernel.org> > > Cc: <linux-kernel@vger.kernel.org> > > --- > > drivers/i3c/master.c | 5 +++++ > > include/linux/i3c/master.h | 5 +++++ > > 2 files changed, 10 insertions(+) > > > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > > index 1c4a86a..46d3774 100644 > > --- a/drivers/i3c/master.c > > +++ b/drivers/i3c/master.c > > @@ -463,6 +463,7 @@ static int i3c_bus_init(struct i3c_bus *i3cbus) > > static const char * const i3c_bus_mode_strings[] = { > > [I3C_BUS_MODE_PURE] = "pure", > > [I3C_BUS_MODE_MIXED_FAST] = "mixed-fast", > > + [I3C_BUS_MODE_MIXED_LIMITED] = "mixed-limited", > > [I3C_BUS_MODE_MIXED_SLOW] = "mixed-slow", > > }; > > > > @@ -575,6 +576,7 @@ int i3c_bus_set_mode(struct i3c_bus *i3cbus, enum i3c_bus_mode mode, > > i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > break; > > case I3C_BUS_MODE_MIXED_FAST: > > + case I3C_BUS_MODE_MIXED_LIMITED: > > if (!i3cbus->scl_rate.i3c) > > i3cbus->scl_rate.i3c = I3C_BUS_TYP_I3C_SCL_RATE; > > if (!i3cbus->scl_rate.i2c) > > @@ -2481,6 +2483,9 @@ int i3c_master_register(struct i3c_master_controller *master, > > mode = I3C_BUS_MODE_MIXED_FAST; > > break; > > case I3C_LVR_I2C_INDEX(1): > > + if (mode < I3C_BUS_MODE_MIXED_LIMITED) > > + mode = I3C_BUS_MODE_MIXED_LIMITED; > > + break; > > case I3C_LVR_I2C_INDEX(2): > > if (mode < I3C_BUS_MODE_MIXED_SLOW) > > mode = I3C_BUS_MODE_MIXED_SLOW; > > diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h > > index 44fb3cf..740235e 100644 > > --- a/include/linux/i3c/master.h > > +++ b/include/linux/i3c/master.h > > @@ -250,12 +250,17 @@ struct i3c_device { > > * the bus. The only impact in this mode is that the > > * high SCL pulse has to stay below 50ns to trick I2C > > * devices when transmitting I3C frames > > + * @I3C_BUS_MODE_MIXED_LIMITED: I2C devices without 50ns spike filter are > > + * present on the bus. However they allows > > + * compliance up to the maximum SDR SCL clock > > + * frequency. > > However they support > SCL clock running at maximum SDR rate > (12.5MHz). > > > * @I3C_BUS_MODE_MIXED_SLOW: I2C devices without 50ns spike filter are present > > * on the bus > > */ > > enum i3c_bus_mode { > > I3C_BUS_MODE_PURE, > > I3C_BUS_MODE_MIXED_FAST, > > + I3C_BUS_MODE_MIXED_LIMITED, > > I3C_BUS_MODE_MIXED_SLOW, > > }; > > > > The code itself looks good. > > Thanks, > > Boris Thanks for your feedback I will address the fixes next version. Best regards, Vitor Soares ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] i3c: dw: Add limited bus mode support [not found] <cover.1555350118.git.vitor.soares@synopsys.com> 2019-04-15 18:42 ` [PATCH 1/3] i3c: fix i2c and i3c scl rate by bus mode Vitor Soares 2019-04-15 18:42 ` [PATCH 2/3] i3c: add mixed limited " Vitor Soares @ 2019-04-15 18:42 ` Vitor Soares 2019-04-15 18:46 ` Vitor Soares 2019-04-16 6:09 ` Boris Brezillon 2 siblings, 2 replies; 17+ messages in thread From: Vitor Soares @ 2019-04-15 18:42 UTC (permalink / raw) To: vitor.soares; +Cc: Vitor Soares, Boris Brezillon, linux-kernel This patch add limited bus mode support for DesignWare i3c master Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> Cc: Boris Brezillon <bbrezillon@kernel.org> Cc: <linux-kernel@vger.kernel.org> --- drivers/i3c/master/dw-i3c-master.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c index 0b01607..4005508 100644 --- a/drivers/i3c/master/dw-i3c-master.c +++ b/drivers/i3c/master/dw-i3c-master.c @@ -650,6 +650,7 @@ static int dw_i3c_master_bus_init(struct i3c_master_controller *m) switch (bus->mode) { case I3C_BUS_MODE_MIXED_FAST: + case I3C_BUS_MODE_MIXED_LIMITED: ret = dw_i2c_clk_cfg(master); if (ret) return ret; -- 2.7.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/3] i3c: dw: Add limited bus mode support 2019-04-15 18:42 ` [PATCH 3/3] i3c: dw: Add limited bus mode support Vitor Soares @ 2019-04-15 18:46 ` Vitor Soares 2019-04-16 6:09 ` Boris Brezillon 1 sibling, 0 replies; 17+ messages in thread From: Vitor Soares @ 2019-04-15 18:46 UTC (permalink / raw) To: linux-i3c; +Cc: joao.pinto, Vitor Soares, Boris Brezillon, linux-kernel This patch add limited bus mode support for DesignWare i3c master Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> Cc: Boris Brezillon <bbrezillon@kernel.org> Cc: <linux-kernel@vger.kernel.org> --- drivers/i3c/master/dw-i3c-master.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c index 0b01607..4005508 100644 --- a/drivers/i3c/master/dw-i3c-master.c +++ b/drivers/i3c/master/dw-i3c-master.c @@ -650,6 +650,7 @@ static int dw_i3c_master_bus_init(struct i3c_master_controller *m) switch (bus->mode) { case I3C_BUS_MODE_MIXED_FAST: + case I3C_BUS_MODE_MIXED_LIMITED: ret = dw_i2c_clk_cfg(master); if (ret) return ret; -- 2.7.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] i3c: dw: Add limited bus mode support 2019-04-15 18:42 ` [PATCH 3/3] i3c: dw: Add limited bus mode support Vitor Soares 2019-04-15 18:46 ` Vitor Soares @ 2019-04-16 6:09 ` Boris Brezillon 1 sibling, 0 replies; 17+ messages in thread From: Boris Brezillon @ 2019-04-16 6:09 UTC (permalink / raw) To: Vitor Soares Cc: linux-i3c, joao.pinto, Boris Brezillon, linux-kernel, Przemyslaw Gaj +Przemek On Mon, 15 Apr 2019 20:46:43 +0200 Vitor Soares <vitor.soares@synopsys.com> wrote: > This patch add limited bus mode support for DesignWare i3c master > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> > Cc: Boris Brezillon <bbrezillon@kernel.org> > Cc: <linux-kernel@vger.kernel.org> > --- > drivers/i3c/master/dw-i3c-master.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c > index 0b01607..4005508 100644 > --- a/drivers/i3c/master/dw-i3c-master.c > +++ b/drivers/i3c/master/dw-i3c-master.c > @@ -650,6 +650,7 @@ static int dw_i3c_master_bus_init(struct i3c_master_controller *m) > > switch (bus->mode) { > case I3C_BUS_MODE_MIXED_FAST: > + case I3C_BUS_MODE_MIXED_LIMITED: > ret = dw_i2c_clk_cfg(master); > if (ret) > return ret; Przemek, you might want to do the same thing in the Cadence driver. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2019-04-22 19:59 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <cover.1555350118.git.vitor.soares@synopsys.com>
2019-04-15 18:42 ` [PATCH 1/3] i3c: fix i2c and i3c scl rate by bus mode Vitor Soares
2019-04-15 18:46 ` Vitor Soares
2019-04-16 5:50 ` Boris Brezillon
2019-04-16 14:24 ` Vitor Soares
2019-04-16 14:52 ` Boris Brezillon
2019-04-22 15:54 ` Vitor Soares
2019-04-22 16:07 ` Boris Brezillon
2019-04-22 17:54 ` Vitor Soares
2019-04-22 18:27 ` Boris Brezillon
2019-04-22 19:59 ` Vitor Soares
2019-04-15 18:42 ` [PATCH 2/3] i3c: add mixed limited " Vitor Soares
2019-04-15 18:46 ` Vitor Soares
2019-04-16 6:00 ` Boris Brezillon
2019-04-16 14:27 ` Vitor Soares
2019-04-15 18:42 ` [PATCH 3/3] i3c: dw: Add limited bus mode support Vitor Soares
2019-04-15 18:46 ` Vitor Soares
2019-04-16 6:09 ` Boris Brezillon
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).