Re: [PATCH RFC i2c-master] i2c: microchip: pci1xxxx: Add driver for I2C host controller in multifunction endpoint of pci1xxxx switch

[<Tharunkumar.Pasumarthi@microchip.com>]

On Fri, 2022-08-26 at 04:00 +0000, Tharunkumar Pasumarthi - I67821 wrote:
> On Thu, 2022-08-25 at 17:22 +0300, Andy Shevchenko wrote:
> > 
> > On Thu, Aug 25, 2022 at 01:15:42PM +0000,
> > Tharunkumar.Pasumarthi@microchip.com wrote:
> > > On Wed, 2022-08-24 at 21:31 +0300, Andy Shevchenko wrote:
> > > > On Wed, Aug 24, 2022 at 6:04 PM <Tharunkumar.Pasumarthi@microchip.com>
> > > > wrote:
> > > > > On Tue, 2022-08-23 at 18:05 +0300, Andy Shevchenko wrote:
> > > > > > On Tue, Aug 23, 2022 at 08:26:03PM +0530, Tharun Kumar P wrote:
> > 
> > ...
> > 
> > > > > > > +#define SMB_IDLE_SCALING_100KHZ              0x03E803C9
> > > > > > > +#define SMB_IDLE_SCALING_400KHZ              0x01F4009D
> > > > > > > +#define SMB_IDLE_SCALING_1000KHZ     0x01F4009D
> > > > > > 
> > > > > > Shouldn't these magics be decimals?
> > > > 
> > > > This Q seems unanswered.
> > > 
> > > These magic numbers need not be decimals. Configuring registers with these
> > > values in driver will set the time in device. However, these values do not
> > > convey any meaning when represented in decimals.
> > 
> > Hmm... Maybe you don't see this, but I see the following:
> > 
> > 0x03E803C9 = 65536 (i.e. 2^16) * 1000 + 969
> > 0x01F4009D = 32768 (i.e. 2^15) * 1000 + 157
> > 
> > Pretty much sounds like a bit 15 for standard mode and bit 16 for fast modes
> > shifted by 1000 to have a room for the time in presumably nanoseconds up to
> > 1
> > us.
> > 
> > Please, dig up into the documentation, vendor chat, etc to get more
> > information
> > on these values.
> 

I have went through the documentation.

Following is the bit mapping of idle scaling register:
Reserved [31:28]
Fair Idle Delay [27:16]
Reserved [15:12]
Bus Idle Min [11:0]

'Bus Idle Min' field will indicate the number of ticks of the baud clock
required to program 'bus idle period' delay and can have maximum value of 4095.
'Fair Idle Min' field will indicate the number of ticks of the baud clock
required to program 'fair idle' delay and can have maximum value of 4095.

So, either the entire IDLE_SCALING_REG value can be in hex or I could do
something like below:

#define BUS_IDLE_MIN_TICKS     <VALUE_IN_DECIMAL>
#define FAIR_IDLE_DELAY_TICKS  <VALUE_IN_DECIMAL>

#define IDLE_SCALING_REG ((FAIR_IDLE_DELAY_TICKS << 16) | BUS_IDLE_MIN_TICKS)

Which of these 2 approaches do you feel is better?

> Sure, I will go through the documentation. Thanks.
> Also, I will modify these magic numbers to decimals.

Thanks,
Tharun Kumar P