Re: [cip-dev] [PATCH 4.19.y-cip 2/7] memory: add Renesas RPC-IF driver

From: "Pavel Machek" <pavel@denx.de>
To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>
Cc: Pavel Machek <pavel@denx.de>,
	"cip-dev@lists.cip-project.org" <cip-dev@lists.cip-project.org>,
	Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>,
	Biju Das <biju.das.jz@bp.renesas.com>
Subject: Re: [cip-dev] [PATCH 4.19.y-cip 2/7] memory: add Renesas RPC-IF driver
Date: Tue, 24 Nov 2020 10:27:50 +0100	[thread overview]
Message-ID: <20201124092749.GA26484@amd> (raw)
In-Reply-To: <OSBPR01MB5048F8E3ED247EE8F532969EAAFC0@OSBPR01MB5048.jpnprd01.prod.outlook.com>

[-- Attachment #1.1: Type: text/plain, Size: 1369 bytes --]

Hi!

> > > +void rpcif_enable_rpm(struct rpcif *rpc)
> > > +{
> > > +	pm_runtime_enable(rpc->dev);
> > > +}
> > > +EXPORT_SYMBOL(rpcif_enable_rpm);
> > > +
> > > +void rpcif_disable_rpm(struct rpcif *rpc)
> > > +{
> > > +	pm_runtime_put_sync(rpc->dev);
> > > +}
> > > +EXPORT_SYMBOL(rpcif_disable_rpm);
> > 
> > Should these go to header as static inlines?
> > 
> Would be nice will make them inline and squash it in the current
> patch.

For this series, maybe squashing is not neccessary. It is pretty good
as-is.

> > > +static int wait_msg_xfer_end(struct rpcif *rpc)
> > > +{
> > > +	u32 sts;
> > > +
> > > +	return regmap_read_poll_timeout(rpc->regmap, RPCIF_CMNSR, sts,
> > > +					sts & RPCIF_CMNSR_TEND, 0,
> > > +					USEC_PER_SEC);
> > > +}
> > 
> > This can't be right. sts is used uninitialized here.
> > 
> Third parameter in regmap_read_poll_timeout() is the variable in which value is read and the fourth parameter condition has to be tied with the third parameter (there are similar instance in the kernel).
>

Okay, that's some rather evil code. It should really be
REGMAP_READ_POLL_TIMEOUT, because it is macro with weird semantics.

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 420 bytes --]

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#5837): https://lists.cip-project.org/g/cip-dev/message/5837
Mute This Topic: https://lists.cip-project.org/mt/78451541/4520388
Group Owner: cip-dev+owner@lists.cip-project.org
Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129055/727948398/xyzzy [cip-dev@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-