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]

Date:   Mon, 24 Feb 2020 12:11:03 +0000
From:   Adam Thomson <Adam.Thomson.Opensource@...semi.com>
To:     Lee Jones <lee.jones@...aro.org>,
        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 24 February 2020 11:27, Lee Jones wrote:

> 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.

To be fair the regmap specific additions for the temporary register access, are
maybe 50 - 60 lines. The rest is to do with handling the result which you'll
need anyway to select the correct register map. I reckon to provide raw read and
write access we're going to probably be similar or more as we'll need to write
the page register then read from the relevant ID registers. Using this an
example for the lines count:

https://elixir.bootlin.com/linux/latest/source/drivers/mfd/tps6507x.c#L37

I can knock something together though just to see what it looks like.