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;
MIME-Version: 1.0
References: <20200423085857.93684-1-acelan.kao@canonical.com> <20200423144529.GM4808@sirena.org.uk>
In-Reply-To: <20200423144529.GM4808@sirena.org.uk>
From:   AceLan Kao <acelan.kao@canonical.com>
Date:   Fri, 24 Apr 2020 20:24:53 +0800
Message-ID: <CAFv23QmNLUqWKHTjK-sFE7ChUCu9j=0p6=Rn22d0E8_HeV-8aw@mail.gmail.com>
Subject: Re: [PATCH] regmap-i2c: add 16 bits register width support
To:     Mark Brown <broonie@kernel.org>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Aaron Sierra <asierra@xes-inc.com>,
        "Linux-Kernel@Vger. Kernel. Org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Mark Brown <broonie@kernel.org> 於 2020年4月23日 週四 下午10:45寫道：
>
> On Thu, Apr 23, 2020 at 04:58:57PM +0800, AceLan Kao wrote:
> > This allows to access data with 16 bits register width
> > via i2c smbus block functions.
>
> > The implementation is inspired by below commit
> > https://patchwork.ozlabs.org/patch/545292/
>
> Do you actually have a system that needs this or is it just being
> implemented for completeness?  The patch you link to mentions that there
> are correctness issues with this implementation.
Yes, I'm working on an Eurotech's new platform which comes with an
EEPROM requires that patch
https://www.eurotech.com/en/news/cpu-162-24-a-new-rugged-com-express-basic-type-6
I'd like to upstream the commit, so that we don't have to maintain the
commit in our kernel,
so I migrate the code to the latest upstream driver.

> Please include human readable descriptions of things like commits and
> issues being discussed in e-mail in your mails, this makes them much
> easier for humans to read especially when they have no internet access.
> I do frequently catch up on my mail on flights or while otherwise
> travelling so this is even more pressing for me than just being about
> making things a bit easier to read.
I'll submit v2 soon and add more description.

> > @@ -255,6 +312,10 @@ static const struct regmap_bus *regmap_get_i2c_bus(struct i2c_client *i2c,
> >                i2c_check_functionality(i2c->adapter,
> >                                        I2C_FUNC_SMBUS_I2C_BLOCK))
> >               return &regmap_i2c_smbus_i2c_block;
> > +     else if (config->val_bits == 8 && config->reg_bits == 16 &&
> > +             i2c_check_functionality(i2c->adapter,
> > +                                     I2C_FUNC_SMBUS_I2C_BLOCK))
> > +             return &regmap_i2c_smbus_i2c_block_reg16;
>
> OTOH we'll only use it if the device wouldn't otherwise work so I'm not
> sure that it's any worse than just hoping the bus is uncontested,
> hopefully system designers have taken this into account when building
> systems without real I2C controllers.
I'm not an expert in this field, please let me know if there is any
better way to archive this.