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, 3 Mar 2022 16:10:39 +0200
From:   Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To:     Tali Perry <tali.perry1@gmail.com>
Cc:     Tyrone Ting <warp5tw@gmail.com>, avifishman70@gmail.com,
        Tomer Maimon <tmaimon77@gmail.com>,
        Patrick Venture <venture@google.com>,
        Nancy Yuen <yuenn@google.com>,
        Benjamin Fair <benjaminfair@google.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
        yangyicong@hisilicon.com, semen.protsenko@linaro.org,
        Wolfram Sang <wsa@kernel.org>, jie.deng@intel.com,
        sven@svenpeter.dev, bence98@sch.bme.hu, lukas.bulwahn@gmail.com,
        arnd@arndb.de, olof@lixom.net, Tali Perry <tali.perry@nuvoton.com>,
        Avi Fishman <Avi.Fishman@nuvoton.com>,
        tomer.maimon@nuvoton.com, KWLIU@nuvoton.com, JJLIU0@nuvoton.com,
        kfting@nuvoton.com, OpenBMC Maillist <openbmc@lists.ozlabs.org>,
        Linux I2C <linux-i2c@vger.kernel.org>,
        devicetree <devicetree@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 11/11] i2c: npcm: Support NPCM845
Message-ID: <YiDMX7pUqs/rLJUU@smile.fi.intel.com>
References: <20220303083141.8742-1-warp5tw@gmail.com>
 <20220303083141.8742-12-warp5tw@gmail.com>
 <YiCb7LNY9tmMCZx7@smile.fi.intel.com>
 <CAHb3i=tWhtXK+c5GGbp6m23AHoyy=4woT_+n3a_N-6CqKUYb=g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAHb3i=tWhtXK+c5GGbp6m23AHoyy=4woT_+n3a_N-6CqKUYb=g@mail.gmail.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Precedence: bulk

On Thu, Mar 03, 2022 at 02:35:58PM +0200, Tali Perry wrote:
> > On Thu, Mar 3, 2022 at 12:45 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > On Thu, Mar 03, 2022 at 04:31:41PM +0800, Tyrone Ting wrote:

...

> > > > -     left_in_fifo = FIELD_GET(NPCM_I2CTXF_STS_TX_BYTES,
> > > > -                              ioread8(bus->reg + NPCM_I2CTXF_STS));
> > > > +     left_in_fifo = (bus->data->txf_sts_tx_bytes &
> > > > +                     ioread8(bus->reg + NPCM_I2CTXF_STS));
> > >
> > > Besides too many parentheses, this is an interesting change. So, in different
> > > versions of IP the field is on different bits? Perhaps it means that you need
> > > something like internal ops structure for all these, where you will have been
> > > using the statically defined masks?
> > >
> 
> Those are two very similar modules. The first generation had a 16 bytes HW FIFO
> and the second generation has 32 bytes.
> In V1 of this patchset the masks were defined under
> CONFIG but we were asked to change the approach:
> 
> the entire discussion can be found here:
> 
> https://www.spinics.net/lists/linux-i2c/msg55566.html
> 
> Did we understand the request change right?

Not really. If you have not simply "one (MSB) bit more" for FIFO size, then
I proposed to create a specific operations structure and use callbacks (see
drivers/dma/dw/ case for iDMA 32-bit vs. DesignWare).

But hold on and read set of questions below.

Previously it was a fixed field with the NPCM_I2CTXF_STS_TX_BYTES mask applied,
right? From above I have got that FIFO is growing twice. Is it correct?
Does the LSB stay at the same offset? What is the meaning of the MSB in 32 byte
case? If it's reserved then why not to always use 32 byte approach?

-- 
With Best Regards,
Andy Shevchenko