| Message ID | 20240807061306.3143528-1-carlos.song@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] i3c: master: support to adjust first broadcast address speed | expand |
Hi carlos.song@nxp.com, carlos.song@nxp.com wrote on Wed, 7 Aug 2024 14:13:05 +0800: > From: Carlos Song <carlos.song@nxp.com> > > According to I3C spec 6.2 Timing Specification, the Open Drain High Period > of SCL Clock timing for first broadcast address should be adjusted to 200ns > at least. I3C device working as i2c device will see the broadcast to close > its Spike Filter then change to work at I3C mode. After that I3C open drain > SCL high level should be adjusted back. > > Signed-off-by: Carlos Song <carlos.song@nxp.com> > Reviewed-by: Frank Li <Frank.Li@nxp.com> > --- > - Fix set_speed description from Frank's comment > --- > drivers/i3c/master.c | 12 ++++++++++++ > include/linux/i3c/master.h | 18 ++++++++++++++++++ > 2 files changed, 30 insertions(+) > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > index 7028f03c2c42..6f3eb710a75d 100644 > --- a/drivers/i3c/master.c > +++ b/drivers/i3c/master.c > @@ -1868,6 +1868,12 @@ static int i3c_master_bus_init(struct i3c_master_controller *master) > goto err_bus_cleanup; > } > > + if (master->ops->set_speed) { > + ret = master->ops->set_speed(master, I3C_OPEN_DRAIN_SLOW_SPEED); > + if (ret) > + goto err_bus_cleanup; > + } > + > /* > * Reset all dynamic address that may have been assigned before > * (assigned by the bootloader for example). > @@ -1876,6 +1882,12 @@ static int i3c_master_bus_init(struct i3c_master_controller *master) > if (ret && ret != I3C_ERROR_M2) > goto err_bus_cleanup; > > + if (master->ops->set_speed) { > + master->ops->set_speed(master, I3C_OPEN_DRAIN_NORMAL_SPEED); > + if (ret) > + goto err_bus_cleanup; > + } > + > /* Disable all slave events before starting DAA. */ > ret = i3c_master_disec_locked(master, I3C_BROADCAST_ADDR, > I3C_CCC_EVENT_SIR | I3C_CCC_EVENT_MR | > diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h > index 074f632868d9..b8daf09a9a4a 100644 > --- a/include/linux/i3c/master.h > +++ b/include/linux/i3c/master.h > @@ -277,6 +277,20 @@ enum i3c_bus_mode { > I3C_BUS_MODE_MIXED_SLOW, > }; > > +/** > + * enum i3c_open_drain_speed - I3C open drain speed > + * @I3C_OPEN_DRAIN_SLOW_SPEED: Slow open drain speed for First Broadcast Address. A few minor style comments: open-drain ...for sending the first broadcast address. > + * First Broadcast Address in this speed is visible to all I2C/I3C The first broadcast address at this speed will be visible to all > + * devices on the I3C bus. I3C device working as a I2C device will ... I3C devices working in I2C mode will > + * turn off its 50ns Spike Filter to change to work in I3C mode. their spike filter when switching into I3C mode. > + * @I3C_OPEN_DRAIN_NORMAL_SPEED: Normal open drain speed configured according to open-drain > + * I3C bus mode. I'm sorry, can you rephrase "configured according to I3C bus mode" ? > + */ > +enum i3c_open_drain_speed { > + I3C_OPEN_DRAIN_SLOW_SPEED, > + I3C_OPEN_DRAIN_NORMAL_SPEED, > +}; > + > /** > * enum i3c_addr_slot_status - I3C address slot status > * @I3C_ADDR_SLOT_FREE: address is free > @@ -436,6 +450,9 @@ struct i3c_bus { > * NULL. > * @enable_hotjoin: enable hot join event detect. > * @disable_hotjoin: disable hot join event detect. > + * @set_speed: adjust I3C bus speed, which is generally used for reducing the speed > + * for first broardcast address. > + * > */ > struct i3c_master_controller_ops { > int (*bus_init)(struct i3c_master_controller *master); > @@ -464,6 +481,7 @@ struct i3c_master_controller_ops { > struct i3c_ibi_slot *slot); > int (*enable_hotjoin)(struct i3c_master_controller *master); > int (*disable_hotjoin)(struct i3c_master_controller *master); > + int (*set_speed)(struct i3c_master_controller *master, enum i3c_open_drain_speed speed); > }; > > /** Otherwise lgtm. Thanks, Miquèl
> -----Original Message----- > From: Miquel Raynal <miquel.raynal@bootlin.com> > Sent: Thursday, August 22, 2024 12:07 AM > To: Carlos Song <carlos.song@nxp.com> > Cc: alexandre.belloni@bootlin.com; Frank Li <frank.li@nxp.com>; > conor.culhane@silvaco.com; linux-i3c@lists.infradead.org; > linux-kernel@vger.kernel.org; imx@lists.linux.dev; dl-linux-imx > <linux-imx@nxp.com> > Subject: [EXT] Re: [PATCH v2 1/2] i3c: master: support to adjust first broadcast > address speed > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > Hi carlos.song@nxp.com, > > carlos.song@nxp.com wrote on Wed, 7 Aug 2024 14:13:05 +0800: > > > From: Carlos Song <carlos.song@nxp.com> > > > > According to I3C spec 6.2 Timing Specification, the Open Drain High > > Period of SCL Clock timing for first broadcast address should be > > adjusted to 200ns at least. I3C device working as i2c device will see > > the broadcast to close its Spike Filter then change to work at I3C > > mode. After that I3C open drain SCL high level should be adjusted back. > > > > Signed-off-by: Carlos Song <carlos.song@nxp.com> > > Reviewed-by: Frank Li <Frank.Li@nxp.com> > > --- > > - Fix set_speed description from Frank's comment > > --- > > drivers/i3c/master.c | 12 ++++++++++++ > > include/linux/i3c/master.h | 18 ++++++++++++++++++ > > 2 files changed, 30 insertions(+) > > > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index > > 7028f03c2c42..6f3eb710a75d 100644 > > --- a/drivers/i3c/master.c > > +++ b/drivers/i3c/master.c > > @@ -1868,6 +1868,12 @@ static int i3c_master_bus_init(struct > i3c_master_controller *master) > > goto err_bus_cleanup; > > } > > > > + if (master->ops->set_speed) { > > + ret = master->ops->set_speed(master, > I3C_OPEN_DRAIN_SLOW_SPEED); > > + if (ret) > > + goto err_bus_cleanup; > > + } > > + > > /* > > * Reset all dynamic address that may have been assigned before > > * (assigned by the bootloader for example). > > @@ -1876,6 +1882,12 @@ static int i3c_master_bus_init(struct > i3c_master_controller *master) > > if (ret && ret != I3C_ERROR_M2) > > goto err_bus_cleanup; > > > > + if (master->ops->set_speed) { > > + master->ops->set_speed(master, > I3C_OPEN_DRAIN_NORMAL_SPEED); > > + if (ret) > > + goto err_bus_cleanup; > > + } > > + > > /* Disable all slave events before starting DAA. */ > > ret = i3c_master_disec_locked(master, I3C_BROADCAST_ADDR, > > I3C_CCC_EVENT_SIR | > > I3C_CCC_EVENT_MR | diff --git a/include/linux/i3c/master.h > > b/include/linux/i3c/master.h index 074f632868d9..b8daf09a9a4a 100644 > > --- a/include/linux/i3c/master.h > > +++ b/include/linux/i3c/master.h > > @@ -277,6 +277,20 @@ enum i3c_bus_mode { > > I3C_BUS_MODE_MIXED_SLOW, > > }; > > > > +/** > > + * enum i3c_open_drain_speed - I3C open drain speed > > + * @I3C_OPEN_DRAIN_SLOW_SPEED: Slow open drain speed for First > Broadcast Address. > > A few minor style comments: > > open-drain > > ...for sending the first broadcast address. > > > + * First Broadcast Address in this speed is > visible to all I2C/I3C > > The first broadcast address at this speed will be visible to all > > > + * devices on the I3C bus. I3C device working > as a I2C device will > > ... I3C devices working in I2C mode will > > > + * turn off its 50ns Spike Filter to change to > work in I3C mode. > > their spike filter when > switching into I3C mode. > > > + * @I3C_OPEN_DRAIN_NORMAL_SPEED: Normal open drain speed > configured > > + according to > > open-drain > > > + * I3C bus mode. > > I'm sorry, can you rephrase "configured according to I3C bus mode" ? > Hi! Thank you for your ack! I will change above to: /** * enum i3c_open_drain_speed - I3C open-drain speed * @I3C_OPEN_DRAIN_SLOW_SPEED: Slow open-drain speed for sending the first * broadcast address. The first broadcast address at this speed * will be visible to all devices on the I3C bus. I3C devices * working in I2C mode will turn off their spike filter when * switching into I3C mode. * @I3C_OPEN_DRAIN_NORMAL_SPEED: Normal open-drain speed in I3C bus mode. */ Is this acceptable? > > + */ > > +enum i3c_open_drain_speed { > > + I3C_OPEN_DRAIN_SLOW_SPEED, > > + I3C_OPEN_DRAIN_NORMAL_SPEED, > > +}; > > + > > /** > > * enum i3c_addr_slot_status - I3C address slot status > > * @I3C_ADDR_SLOT_FREE: address is free @@ -436,6 +450,9 @@ struct > > i3c_bus { > > * NULL. > > * @enable_hotjoin: enable hot join event detect. > > * @disable_hotjoin: disable hot join event detect. > > + * @set_speed: adjust I3C bus speed, which is generally used for reducing > the speed > > + * for first broardcast address. > > + * > > */ > > struct i3c_master_controller_ops { > > int (*bus_init)(struct i3c_master_controller *master); @@ -464,6 > > +481,7 @@ struct i3c_master_controller_ops { > > struct i3c_ibi_slot *slot); > > int (*enable_hotjoin)(struct i3c_master_controller *master); > > int (*disable_hotjoin)(struct i3c_master_controller *master); > > + int (*set_speed)(struct i3c_master_controller *master, enum > > + i3c_open_drain_speed speed); > > }; > > > > /** > > Otherwise lgtm. > > Thanks, > Miquèl
Hi Carlos, > > I'm sorry, can you rephrase "configured according to I3C bus mode" ? > > > > Hi! Thank you for your ack! I will change above to: > > /** > * enum i3c_open_drain_speed - I3C open-drain speed > * @I3C_OPEN_DRAIN_SLOW_SPEED: Slow open-drain speed for sending the first > * broadcast address. The first broadcast address at this speed > * will be visible to all devices on the I3C bus. I3C devices > * working in I2C mode will turn off their spike filter when > * switching into I3C mode. > * @I3C_OPEN_DRAIN_NORMAL_SPEED: Normal open-drain speed in I3C bus mode. > */ > > Is this acceptable? Sounds much clearer to me, thanks! Miquèl
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index 7028f03c2c42..6f3eb710a75d 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -1868,6 +1868,12 @@ static int i3c_master_bus_init(struct i3c_master_controller *master) goto err_bus_cleanup; } + if (master->ops->set_speed) { + ret = master->ops->set_speed(master, I3C_OPEN_DRAIN_SLOW_SPEED); + if (ret) + goto err_bus_cleanup; + } + /* * Reset all dynamic address that may have been assigned before * (assigned by the bootloader for example). @@ -1876,6 +1882,12 @@ static int i3c_master_bus_init(struct i3c_master_controller *master) if (ret && ret != I3C_ERROR_M2) goto err_bus_cleanup; + if (master->ops->set_speed) { + master->ops->set_speed(master, I3C_OPEN_DRAIN_NORMAL_SPEED); + if (ret) + goto err_bus_cleanup; + } + /* Disable all slave events before starting DAA. */ ret = i3c_master_disec_locked(master, I3C_BROADCAST_ADDR, I3C_CCC_EVENT_SIR | I3C_CCC_EVENT_MR | diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h index 074f632868d9..b8daf09a9a4a 100644 --- a/include/linux/i3c/master.h +++ b/include/linux/i3c/master.h @@ -277,6 +277,20 @@ enum i3c_bus_mode { I3C_BUS_MODE_MIXED_SLOW, }; +/** + * enum i3c_open_drain_speed - I3C open drain speed + * @I3C_OPEN_DRAIN_SLOW_SPEED: Slow open drain speed for First Broadcast Address. + * First Broadcast Address in this speed is visible to all I2C/I3C + * devices on the I3C bus. I3C device working as a I2C device will + * turn off its 50ns Spike Filter to change to work in I3C mode. + * @I3C_OPEN_DRAIN_NORMAL_SPEED: Normal open drain speed configured according to + * I3C bus mode. + */ +enum i3c_open_drain_speed { + I3C_OPEN_DRAIN_SLOW_SPEED, + I3C_OPEN_DRAIN_NORMAL_SPEED, +}; + /** * enum i3c_addr_slot_status - I3C address slot status * @I3C_ADDR_SLOT_FREE: address is free @@ -436,6 +450,9 @@ struct i3c_bus { * NULL. * @enable_hotjoin: enable hot join event detect. * @disable_hotjoin: disable hot join event detect. + * @set_speed: adjust I3C bus speed, which is generally used for reducing the speed + * for first broardcast address. + * */ struct i3c_master_controller_ops { int (*bus_init)(struct i3c_master_controller *master); @@ -464,6 +481,7 @@ struct i3c_master_controller_ops { struct i3c_ibi_slot *slot); int (*enable_hotjoin)(struct i3c_master_controller *master); int (*disable_hotjoin)(struct i3c_master_controller *master); + int (*set_speed)(struct i3c_master_controller *master, enum i3c_open_drain_speed speed); }; /**