Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 25 Aug 2022 17:22:46 +0300
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
To:     Tharunkumar.Pasumarthi@microchip.com
Cc:     jsd@semihalf.com, wsa@kernel.org, krzk@kernel.org,
        sven@svenpeter.dev, linux-kernel@vger.kernel.org,
        semen.protsenko@linaro.org, robh@kernel.org, olof@lixom.net,
        UNGLinuxDriver@microchip.com, jarkko.nikula@linux.intel.com,
        arnd@arndb.de, linux-i2c@vger.kernel.org, rafal@milecki.pl
Subject: Re: [PATCH RFC i2c-master] i2c: microchip: pci1xxxx: Add driver for
 I2C host controller in multifunction endpoint of pci1xxxx switch
Message-ID: <YweFttaSE2GOoW83@smile.fi.intel.com>
References: <20220823145603.2606475-1-tharunkumar.pasumarthi@microchip.com>
 <YwTsnCdxM5I6BoJu@smile.fi.intel.com>
 <80debd4bbf819cbfa16681172919436df476558a.camel@microchip.com>
 <CAHp75VevAifCp_Bz5HN1MmGm7UEJWFR017kngzARfBh+Z8aAAA@mail.gmail.com>
 <787644272b3b260980ff8b9efc317ce3194b0acc.camel@microchip.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <787644272b3b260980ff8b9efc317ce3194b0acc.camel@microchip.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Precedence: bulk

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.

> > > > Ditto for the rest similar stuff.
> > > 
> > > There is no direct correlation between the hex value and time. Configuring
> > > registers with these values in driver will set the time in device.

...

> > > > > +#define I2C_DIR_WRITE??????????????? 0
> > > > > +#define I2C_DIR_READ???????? 1
> > 
> > https://elixir.bootlin.com/linux/v6.0-rc2/source/include/uapi/linux/i2c.h#L24
> 
> I2C_M_RD is used in driver. But the purpose of these MACROs is different.
> I2C_DIR_WRITE is used inside both pci1xxxx_i2c_write as well as in
> pci1xxxx_i2c_read (for sending slave address). Thus these MACROs are required

OK. Name collision still stays.

> > > > Namespace collision. Doesn't I?C core provide these?
> > > 
> > > I am unable to find any existing MACROs for WRITE and READ in I2C core.
> > > Kindly let me know the MACROs

-- 
With Best Regards,
Andy Shevchenko