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