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:   Sat, 6 Mar 2021 21:19:38 +0100
From:   Jonathan =?utf-8?Q?Neusch=C3=A4fer?= <j.neuschaefer@gmx.net>
To:     Andreas Kemnade <andreas@kemnade.info>
Cc:     Jonathan =?utf-8?Q?Neusch=C3=A4fer?= <j.neuschaefer@gmx.net>,
        lee.jones@linaro.org, linux-kernel@vger.kernel.org,
        Mark Brown <broonie@kernel.org>
Subject: Re: [PATCH -next] mfd: ntxec: Support for EC in Tolino Shine 2 HD
Message-ID: <YEPj2ss13w5p+R4C@latitude>
References: <20210306181314.12673-1-andreas@kemnade.info>
 <YEPUppNWNiAMnczF@latitude>
 <20210306204219.6a7eae56@aktux>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="R03366iBJ+/ZkxIE"
Content-Disposition: inline
In-Reply-To: <20210306204219.6a7eae56@aktux>
Precedence: bulk


--R03366iBJ+/ZkxIE
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sat, Mar 06, 2021 at 08:42:19PM +0100, Andreas Kemnade wrote:
> On Sat, 6 Mar 2021 20:14:46 +0100
> Jonathan Neusch=C3=A4fer <j.neuschaefer@gmx.net> wrote:
[...]
> > > +/* some firmware versions do not ack written data, add a wrapper */
> > > +static const struct regmap_config regmap_config_noack =3D {
> > > +	.name =3D "ntxec_noack",
> > > +	.reg_bits =3D 8,
> > > +	.val_bits =3D 16,
> > > +	.cache_type =3D REGCACHE_NONE,
> > > +	.val_format_endian =3D REGMAP_ENDIAN_BIG,
> > > +	.reg_write =3D regmap_ignore_write,
> > > +	.reg_read =3D regmap_wrap_read =20
> >=20
> > Is the read wrapper necessary? It seems to me from reading regmap.h
> > that leaving .reg_read set to NULL should do the right thing, but I'm
> > not sure.
> >
> well if we want to read from it, there need to be some function for it.
> But if... I do not see anything worth to read besides of version.

Ok, with the understanding that there will be no read functionality
without the .reg_read callback, I'm fine leaving it in, even when the
particular drivers in existence don't use it.

> I think we can leave ouf val_format_endian because a lot of stuff is
> bypassed if no bus is set in regmap_init().
> There is e.g. a goto skip_format_initialization.

Dropping REGMAP_ENDIAN_BIG sounds reasonable, because this regmap
doesn't go to a byte-wise bus.


> > > +	case NTXEC_VERSION_TOLINO_SHINE2:
> > > +		has_rtc =3D false;
> > > +		ec->regmap =3D devm_regmap_init(ec->dev, NULL,
> > > +					      ec->regmap,
> > > +					      &regmap_config_noack); =20
> >=20
> > Ah=E2=80=94 A custom regmap stacked on top of the old regmap=E2=80=A6 I=
 think this
> > deserves a comment.
> >=20
> Yes, devm_regmap_init_i2c() sets a different set of callbacks depending
> on circumstances. Seems to be the easiest way to avoid duplicating too
> much code. I really hope that there are not so much devices requiring
> such a dirty stuff that adding such thing to the generic regmap code
> would be justified.

Ok, please add a short comment here or extend the one above the repmap
config struct, to save interested readers a bit of trouble. I guess "add
a wrapper" goes in right direction, but it didn't make the stacking
obvious to me when I first read it.


Thanks,
Jonathan

--R03366iBJ+/ZkxIE
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEvHAHGBBjQPVy+qvDCDBEmo7zX9sFAmBD49MACgkQCDBEmo7z
X9uAew/9GsIDWh9HC8YVz7JeQtzHRibhMLqtQpFW+WkqrQ1K4UDXvQZO1jIbB0bc
c/v4FOTDA0qQRqhYHPcTZIU4x3Nq7Gfl5RjPE75qNPoi56eIc/iyxoRPJ36LCWSq
8Y2KiRLX9SueEe2K2zpiMz6o1NNDgoDgPshJzXZNWZEqP6zhS07MB/B9h0ncCJ/A
VkCblUqKb5bJ8zlQ1RHxJ7O9d8MV7PZ9WZrPXWE/BgC7GBfX/WMqBHjOBasozMI4
xQqbqjnHnZAAV3wfZSerGgQRIKMJR/wWHxEVUqcepttv2ovtj/cDaAzyiDstCblJ
/V7XVg54jXigNU7KdZnX0taOEB0lHxaE4XzTFypYFbzt6g5Wbq2FWc0q9LHQhVEr
uhF+9JJ9gEKMZrBFOWrlJzShXX5AIBVVM796TkM/IrZjXnQNJyQSSo0mCUZl66Q8
Cvyft490t1tLmqm8xG/n9zqMPuAeUHf53c8nZuuDO1EyBGFwqVf1mhg+P+Vif0n5
yG6OGhecz2hHRB1tUyzAzfhsiNbrn8rafNjXRyzVUCTAiy9nkTqVlFr315DAHBEN
LBgVJwbCvfYeLINKRfQ8JW4KtZxgIkyUnKy2ikf8JuNeZYxb0HYWRFYHaFJVJlPh
p6K/a9pY83tnTme/ZmQRXU/NJvUfysThDnaCsTQznGLsxg1MWAA=
=ihQv
-----END PGP SIGNATURE-----

--R03366iBJ+/ZkxIE--