linux-kernel - Re: [RESEND PATCH 1/2] mfd: da9063: Fix revision handling to correctly select reg tables

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200224112714.GT3494@dell>
Date:   Mon, 24 Feb 2020 11:27:14 +0000
From:   Lee Jones <lee.jones@...aro.org>
To:     Adam Thomson <Adam.Thomson.Opensource@...semi.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Support Opensource <Support.Opensource@...semi.com>
Subject: Re: [RESEND PATCH 1/2] mfd: da9063: Fix revision handling to
 correctly select reg tables

On Mon, 24 Feb 2020, Adam Thomson wrote:

> On 24 February 2020 09:57, Lee Jones wrote:
> 
> > On Fri, 24 Jan 2020, Adam Thomson wrote:
> > 
> > > The current implementation performs checking in the i2c_probe()
> > > function of the variant_code but does this immediately after the
> > > containing struct has been initialised as all zero. This means the
> > > check for variant code will always default to using the BB tables
> > > and will never select AD. The variant code is subsequently set
> > > by device_init() and later used by the RTC so really it's a little
> > > fortunate this mismatch works.
> > >
> > > This update creates an initial temporary regmap instantiation to
> > > simply read the chip and variant/revision information (common to
> > > all revisions) so that it can subsequently correctly choose the
> > > proper regmap tables for real initialisation.
> > 
> > IIUC, you have a dependency issue whereby the device type is required
> > before you can select the correct Regmap configuration.  Is that
> > correct?
> 
> Yep, spot on.
> 
> > If so, using Regmap for the initial register reads sounds like
> > over-kill.  What's stopping you simply using raw reads before the
> > Regmap is instantiated?
> 
> Actually nothing and I did consider this at the start. Nice thing with regmap
> is it's all tidily contained and provides the page swapping mechanism to access
> higher page registers like the variant information. Given this is only once at
> probe time it felt like this was a reasonable solution. However if you're not
> keen I can update to use raw access instead.

It would be nice to compare the 2 solutions side by side.  I can't see
the raw reads of a few device-ID registers being anywhere near 170
lines though.

> > > Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@...semi.com>
> > > ---
> > >  drivers/mfd/da9063-core.c            |  31 -------
> > >  drivers/mfd/da9063-i2c.c             | 167 +++++++++++++++++++++++++++++++-
> > ---
> > >  include/linux/mfd/da9063/registers.h |  15 ++--
> > >  3 files changed, 160 insertions(+), 53 deletions(-)
> > 

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