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

["Pavel Machek" <pavel@denx.de>]

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

Hi!

> +EXPORT_SYMBOL(rpcif_sw_init);

EXPORT_SYMBOL_GPL?

> +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?

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

> +int rpcif_manual_xfer(struct rpcif *rpc)
> +{
> +	default:
> +		regmap_write(rpc->regmap, RPCIF_SMENR, rpc->enable);
> +		regmap_write(rpc->regmap, RPCIF_SMCR,
> +			     rpc->smcr | RPCIF_SMCR_SPIE);
> +		ret = wait_msg_xfer_end(rpc);
> +		if (ret)
> +			goto err_out;
> +	}
> +
> +exit:
> +	pm_runtime_put(rpc->dev);
> +	return ret;
> +
> +err_out:
> +	ret = reset_control_reset(rpc->rstc);
> +	rpcif_hw_init(rpc, rpc->bus_size == 2);
> +	goto exit;
> +}

So we get failure in wait_msg_xfer_end(rpc); but then
reset_control_reset(rpc->rstc); returns success and whole function
returns success. Is that ok?

> +static int rpcif_probe(struct platform_device *pdev)
> +{
> +	struct platform_device *vdev;
> +	struct device_node *flash;
> +	const char *name;
> +
> +	flash = of_get_next_child(pdev->dev.of_node, NULL);
> +	if (!flash) {
> +		dev_warn(&pdev->dev, "no flash node found\n");
> +		return -ENODEV;
> +	}

Does this need corresponding of_node_put()?

> +struct	rpcif_op {

Weird formatting; we normally use spaces (not tabs) for this.

> +	struct {
> +		u8 buswidth;
...
> +	struct {
> +		u8 ncycles;
> +		u8 buswidth;
> +	} dummy;

Are you sure this will be consistent accross architectures? Should the
structure be marked attribute packed or something?

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 (#5827): https://lists.cip-project.org/g/cip-dev/message/5827
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]
-=-=-=-=-=-=-=-=-=-=-=-