Hi Suneel and Piuysh, On Thu, Mar 30, 2023 at 06:39:51AM -0700, Piyush Malgujar wrote: > From: Suneel Garapati <sgarapati@xxxxxxxxxxx> > > Handle changes to clock divisor logic for OcteonTX2 SoC family using > subsystem ID and using default reference clock source as 100MHz. > > Signed-off-by: Suneel Garapati <sgarapati@xxxxxxxxxxx> > Signed-off-by: Piyush Malgujar <pmalgujar@xxxxxxxxxxx> > --- > drivers/i2c/busses/i2c-octeon-core.c | 29 ++++++++++++++++++++---- > drivers/i2c/busses/i2c-octeon-core.h | 15 ++++++++++++ > drivers/i2c/busses/i2c-thunderx-pcidrv.c | 6 +++++ > 3 files changed, 46 insertions(+), 4 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c > index 845eda70b8cab52a0453c9f4cb545010fba4305d..dfd58bbec47b1f0554ae0c100c680b6ba9be61ec 100644 > --- a/drivers/i2c/busses/i2c-octeon-core.c > +++ b/drivers/i2c/busses/i2c-octeon-core.c > @@ -17,6 +17,7 @@ > #include <linux/interrupt.h> > #include <linux/kernel.h> > #include <linux/module.h> > +#include <linux/pci.h> > > #include "i2c-octeon-core.h" > > @@ -658,31 +659,51 @@ int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) > void octeon_i2c_set_clock(struct octeon_i2c *i2c) > { > int tclk, thp_base, inc, thp_idx, mdiv_idx, ndiv_idx, foscl, diff; > - int thp = 0x18, mdiv = 2, ndiv = 0, delta_hz = 1000000; > + int mdiv_min = 2; > + /* starting value on search for lowest diff */ > + const int huge_delta = 1000000; would be nice to have this "1000000" value defined, also because it's used in other parts of the code. > + /* > + * Find divisors to produce target frequency, start with large delta > + * to cover wider range of divisors, note thp = TCLK half period. > + */ > + int thp = 0x18, mdiv = 2, ndiv = 0, delta_hz = huge_delta; as you are changing them, they can also be unsigned. Patch looks good to me and, even if it's not breaking anything from the previous platforms, would be nice to have a comment from Robert. With the above comments: Acked-by: Andi Shyti <andi.shyti@xxxxxxxxxx> Thanks, Andi > + > + if (octeon_i2c_is_otx2(to_pci_dev(i2c->dev))) { > + thp = 0x3; > + mdiv_min = 0; > + } > > for (ndiv_idx = 0; ndiv_idx < 8 && delta_hz != 0; ndiv_idx++) { > /* > * An mdiv value of less than 2 seems to not work well > * with ds1337 RTCs, so we constrain it to larger values. > */ > - for (mdiv_idx = 15; mdiv_idx >= 2 && delta_hz != 0; mdiv_idx--) { > + for (mdiv_idx = 15; mdiv_idx >= mdiv_min && delta_hz != 0; mdiv_idx--) { > /* > * For given ndiv and mdiv values check the > * two closest thp values. > */ > tclk = i2c->twsi_freq * (mdiv_idx + 1) * 10; > tclk *= (1 << ndiv_idx); > - thp_base = (i2c->sys_freq / (tclk * 2)) - 1; > + if (octeon_i2c_is_otx2(to_pci_dev(i2c->dev))) > + thp_base = (i2c->sys_freq / tclk) - 2; > + else > + thp_base = (i2c->sys_freq / (tclk * 2)) - 1; > > for (inc = 0; inc <= 1; inc++) { > thp_idx = thp_base + inc; > if (thp_idx < 5 || thp_idx > 0xff) > continue; > > - foscl = i2c->sys_freq / (2 * (thp_idx + 1)); > + if (octeon_i2c_is_otx2(to_pci_dev(i2c->dev))) > + foscl = i2c->sys_freq / (thp_idx + 2); > + else > + foscl = i2c->sys_freq / > + (2 * (thp_idx + 1)); > foscl = foscl / (1 << ndiv_idx); > foscl = foscl / (mdiv_idx + 1) / 10; > diff = abs(foscl - i2c->twsi_freq); > + /* Use it if smaller diff from target */ > if (diff < delta_hz) { > delta_hz = diff; > thp = thp_idx; > diff --git a/drivers/i2c/busses/i2c-octeon-core.h b/drivers/i2c/busses/i2c-octeon-core.h > index 9bb9f64fdda0392364638ecbaafe3fab5612baf6..8a0033c94a8a291fb255b0da03858274035c46f4 100644 > --- a/drivers/i2c/busses/i2c-octeon-core.h > +++ b/drivers/i2c/busses/i2c-octeon-core.h > @@ -7,6 +7,7 @@ > #include <linux/i2c-smbus.h> > #include <linux/io.h> > #include <linux/kernel.h> > +#include <linux/pci.h> > > /* Controller command patterns */ > #define SW_TWSI_V BIT_ULL(63) /* Valid bit */ > @@ -211,6 +212,20 @@ static inline void octeon_i2c_write_int(struct octeon_i2c *i2c, u64 data) > octeon_i2c_writeq_flush(data, i2c->twsi_base + TWSI_INT(i2c)); > } > > +#define PCI_SUBSYS_DEVID_9XXX 0xB > +/** > + * octeon_i2c_is_otx2 - check for chip ID > + * @pdev: PCI dev structure > + * > + * Returns TRUE if OcteonTX2, FALSE otherwise. > + */ > +static inline bool octeon_i2c_is_otx2(struct pci_dev *pdev) > +{ > + u32 chip_id = (pdev->subsystem_device >> 12) & 0xF; > + > + return (chip_id == PCI_SUBSYS_DEVID_9XXX); > +} > + > /* Prototypes */ > irqreturn_t octeon_i2c_isr(int irq, void *dev_id); > int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num); > diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c > index a77cd86fe75ed7401bc041b27c651b9fedf67285..eecd27f9f1730e522dcccafc9f12ea891a3b59ef 100644 > --- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c > +++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c > @@ -205,6 +205,12 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev, > if (ret) > goto error; > > + /* > + * For OcteonTX2 chips, set reference frequency to 100MHz > + * as refclk_src in TWSI_MODE register defaults to 100MHz. > + */ > + if (octeon_i2c_is_otx2(pdev)) > + i2c->sys_freq = 100000000; > octeon_i2c_set_clock(i2c); > > i2c->adap = thunderx_i2c_ops; > -- > 2.17.1 >