On Fri, 2022-08-26 at 18:37 +0300, Andy Shevchenko wrote: > On Fri, Aug 26, 2022 at 4:04 PM <Tharunkumar.Pasumarthi@xxxxxxxxxxxxx> wrote: > > 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@xxxxxxxxxxxxx wrote: > > > > > On Wed, 2022-08-24 at 21:31 +0300, Andy Shevchenko wrote: > > > > > > On Wed, Aug 24, 2022 at 6:04 PM > > > > > > <Tharunkumar.Pasumarthi@xxxxxxxxxxxxx> > > > > > > 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: > > No hex. > > > #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? > > Of course one with the comment explaining the thing and two decimal numbers. > Now, since we know that both values are > > 1000 and 969 > 500 and 157 > > It's easy to see the difference and meaning. > > So per each mode you need to have those pairs of decimal numbers in ticks. > and one comment explaining all what you have explained here in this mail. Okay. I will follow this approach. Thanks, Tharun Kumar P
