On Wed, Sep 30, 2020 at 12:31 PM Andy Shevchenko <andriy.shevche...@linux.intel.com> wrote: > > On Wed, Sep 30, 2020 at 10:13:42AM +0300, Tali Perry wrote: > > Systems that can dinamically add and remove slave devices > > dynamically > > > often need to change the bus speed in runtime. > > > This patch exposes the bus frequency to the user. > > Expose the bus frequency to the user. > > > This feature can also be used for test automation. > > In general I think that DT overlays or so should be user rather than this. > If we allow to change bus speed settings for debugging purposes it might make > sense to do this on framework level for all drivers which support that (via > additional callback or so).
Do you mean adding something like this: struct i2c_algorithm { /* * If an adapter algorithm can't do I2C-level access, set master_xfer * to NULL. If an adapter algorithm can do SMBus access, set * smbus_xfer. If set to NULL, the SMBus protocol is simulated * using common I2C messages. * * master_xfer should return the number of messages successfully * processed, or a negative value on error */ int (*master_xfer)(struct i2c_adapter *adap, struct i2c_msg *msgs, int num); .... int (*set_speed)(struct i2c_adapter *adap, unsigned int speed); .... /* To determine what the adapter supports */ u32 (*functionality)(struct i2c_adapter *adap); ... }; And expose this feature in functionality? > > > Fixes: 56a1485b102e (i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver) > > Signed-off-by: Tali Perry <tali.per...@gmail.com> > > --- > > drivers/i2c/busses/i2c-npcm7xx.c | 36 ++++++++++++++++++++++++++++++++ > > 1 file changed, 36 insertions(+) > > > > diff --git a/drivers/i2c/busses/i2c-npcm7xx.c > > b/drivers/i2c/busses/i2c-npcm7xx.c > > index 2ad166355ec9..44e2340c1893 100644 > > --- a/drivers/i2c/busses/i2c-npcm7xx.c > > +++ b/drivers/i2c/busses/i2c-npcm7xx.c > > @@ -2208,6 +2208,41 @@ static const struct i2c_algorithm npcm_i2c_algo = { > > /* i2c debugfs directory: used to keep health monitor of i2c devices */ > > static struct dentry *npcm_i2c_debugfs_dir; > > > > +static int i2c_speed_get(void *data, u64 *val) > > +{ > > + struct npcm_i2c *bus = data; > > + > > + *val = (u64)bus->bus_freq; > > + return 0; > > +} > > + > > +static int i2c_speed_set(void *data, u64 val) > > +{ > > + struct npcm_i2c *bus = data; > > + int ret; > > + > > + if (val < (u64)I2C_FREQ_MIN_HZ || val > (u64)I2C_FREQ_MAX_HZ) > > + return -EINVAL; > > + > > + if (val == (u64)bus->bus_freq) > > + return 0; > > + > > + i2c_lock_bus(&bus->adap, I2C_LOCK_ROOT_ADAPTER); > > + > > + npcm_i2c_int_enable(bus, false); > > + > > + ret = npcm_i2c_init_module(bus, I2C_MASTER, (u32)val); > > + > > + i2c_unlock_bus(&bus->adap, I2C_LOCK_ROOT_ADAPTER); > > While all these explicit castings? > > > + > > + if (ret) > > + return -EAGAIN; > > + > > + return 0; > > +} > > > + > > No need to have this blank line > > > +DEFINE_DEBUGFS_ATTRIBUTE(i2c_clock_ops, i2c_speed_get, i2c_speed_set, > > "%lld\n"); > > + > > static void npcm_i2c_init_debugfs(struct platform_device *pdev, > > struct npcm_i2c *bus) > > { > > @@ -2223,6 +2258,7 @@ static void npcm_i2c_init_debugfs(struct > > platform_device *pdev, > > debugfs_create_u64("rec_succ_cnt", 0444, d, &bus->rec_succ_cnt); > > debugfs_create_u64("rec_fail_cnt", 0444, d, &bus->rec_fail_cnt); > > debugfs_create_u64("timeout_cnt", 0444, d, &bus->timeout_cnt); > > + debugfs_create_file("i2c_speed", 0644, d, bus, &i2c_clock_ops); > > > > bus->debugfs = d; > > } > > -- > With Best Regards, > Andy Shevchenko > >