Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 18 Jun 2020 11:07:04 +0100
From:   Lee Jones <lee.jones@linaro.org>
To:     Arnd Bergmann <arnd@arndb.de>
Cc:     michael@walle.cc, Rob Herring <robh+dt@kernel.org>,
        Mark Brown <broonie@kernel.org>,
        gregkh <gregkh@linuxfoundation.org>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        DTML <devicetree@vger.kernel.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        Bartosz Golaszewski <bgolaszewski@baylibre.com>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 1/1] mfd: Add I2C based System Configuaration (SYSCON)
 access
Message-ID: <20200618100704.GC954398@dell>
References: <20200618080223.951737-1-lee.jones@linaro.org>
 <CAK8P3a3iRmXC2jDj92QHKqyD+x_UJ7rWU_KcGt=MFOD9UW38RA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAK8P3a3iRmXC2jDj92QHKqyD+x_UJ7rWU_KcGt=MFOD9UW38RA@mail.gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, 18 Jun 2020, Arnd Bergmann wrote:

> On Thu, Jun 18, 2020 at 10:03 AM Lee Jones <lee.jones@linaro.org> wrote:
> >
> > The existing SYSCON implementation only supports MMIO (memory mapped)
> > accesses, facilitated by Regmap.  This extends support for registers
> > held behind I2C busses.
> >
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> The implementation looks fine to me, but can you explain how this is going to
> be used, and what the advantage is over open-coding the devm_regmap_init_i2c()
> in each driver that would use this?

Does Regmap let you register/initialise an I2C address more than once?

When I attempt it, I get:

[    0.522988] i2c i2c-0: Failed to register i2c client tmp105 at 0x32 (-16)
[    0.523341] i2c i2c-0: of_i2c: Failure registering /bus@4000000/motherboard/iofpga@7,00000000/i2c@16000/temp@32
[    0.523691] i2c i2c-0: Failed to create I2C device for /bus@4000000/motherboard/iofpga@7,00000000/i2c@16000/temp@32

> Is this about using proper locking through the regmap framework for
> shared i2c clients, or to reduce memory consumption when lots of drivers
> access the same regmap?

All of those things are valid.

My use-case is regarding MFDs sharing an I2C interfaced address space
with their children.

> My impression of the existing syscon code is that the main value-add over
> other ways of doing the same is the syscon_regmap_lookup_by_phandle()
> interface that gives other drivers a much simpler way of getting the
> regmap just based on the DT node. Are you planning to add something
> like that here as well? An ideal driver interface might allow
> syscon_regmap_lookup_by_phandle() to work for both mmio and i2c
> based syscons, or additional ones as well, but implementing this would
> be rather tricky when the i2c core is a loadable module.

I expect the API would be expanded to cover other use-cases.  This is
a bare bones implementation which has been kept as atomic as possible
for ease of review.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog