Date:   Thu, 6 Jan 2022 11:40:34 +0000
From:   Mark Brown <broonie@kernel.org>
To:     Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc:     linux-kernel@vger.kernel.org, Watson Chow <watson.chow@avnet.com>,
        Liam Girdwood <lgirdwood@gmail.com>
Subject: Re: [PATCH 2/2] regulator: Add MAX20086-MAX20089 driver
Message-ID: <YdbVMqKt6iKa/pY+@sirena.org.uk>
References: <20220102211124.18435-1-laurent.pinchart+renesas@ideasonboard.com>
 <20220102211124.18435-3-laurent.pinchart+renesas@ideasonboard.com>
 <YdRWwWmoQGQuUyLz@sirena.org.uk>
 <YdRa0GoSoX8CP694@pendragon.ideasonboard.com>
 <YdYoW2me4aHpJzfm@pendragon.ideasonboard.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="emIq0MHe9V5fJ8DT"
Content-Disposition: inline
In-Reply-To: <YdYoW2me4aHpJzfm@pendragon.ideasonboard.com>
Precedence: bulk


--emIq0MHe9V5fJ8DT
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, Jan 06, 2022 at 01:23:07AM +0200, Laurent Pinchart wrote:
> On Tue, Jan 04, 2022 at 04:33:54PM +0200, Laurent Pinchart wrote:
> > On Tue, Jan 04, 2022 at 02:16:33PM +0000, Mark Brown wrote:

> > > It is worrying that a regulator driver should need the interfaces for
> > > machines...  the driver doesn't look like it actually does though.

> > I'll try to remove it.

> It compiles fine, but I won't be able to check the init data to figure
> out the initial enable GPIO state if I don't include machine.h, as
> that's where regulator_init_data is defined. Am I missing something ?

Right, forgot about that bit - it's more of an issue with the gpiod
conversion.  You could just request always enabled but it's probably
fine to leave as is with looking at but not modifying the constraints.

> > > > +static const struct regmap_config max20086_regmap_config = {
> > > > +	.reg_bits = 8,
> > > > +	.val_bits = 8,
> > > > +	.writeable_reg = max20086_gen_is_writeable_reg,
> > > > +	.max_register = 0x9,
> > > > +	.cache_type = REGCACHE_NONE,
> > > > +};

> > > No readback support?

> > I'll fix that.

> Actually I'm not sure what you mean here. All registers are readable,
> what's wrong with the above regmap_config ?

It's not *wrong* if that's the case, it just looks weird to only provide
writable but not readable.

--emIq0MHe9V5fJ8DT
Content-Type: application/pgp-signature; name="signature.asc"

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

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmHW1TEACgkQJNaLcl1U
h9BFaAf+MO/bJyr38KNE7RLXPr0W7NkXsUPofV1bCQr0s3HxNVCFuMhqd4pBGeq0
Lhs7TnbxraaoGRrI14HvZag8UfAr+E/ffIILi6cc392jm4dnI3lPRVaGQiNMkriO
aeJp9yHJLT8UVaI5hyGnTqQd/g5q7Pw5IewpIcqG/pTIvEk5cFavLRAWCVnZaipN
cmXwPqrsEp+imtmxqkshWkIutwnHoOX+k3rNgiB1cubyiVpt87VKGRjwnXTnARoW
2l8nNVp9E5eyvBf2gDrjyOD+lbl8vz6DFQIf8P1zB6Ke2iL3YhBZmrAvwwBCnXtu
NMzmt5wIwuinBFTlAcjvbfs68imQgA==
=Evrb
-----END PGP SIGNATURE-----

--emIq0MHe9V5fJ8DT--