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


Pavel Machek
 

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

Join cip-dev@lists.cip-project.org to automatically receive all group messages.