On 7/9/2019 12:32 PM, Jonathan Cameron wrote:
Signed-off-by: Hanna Hawa<hhhawa@xxxxxxxxxx>
A quick drive by review as I was feeling curious.
Just a couple of trivial queries and observation on the fact it
might be useful to add a few devm managed functions to cut down
on edac driver boilerplate.
Thanks,
Jonathan
+#define ARM_CA57_CPUMERRSR_VALID GENMASK(31, 31)
For a single bit it's common to use BIT(31) rather than GENMASK to make
it explicit.
Will fix.
+ edac_dev->mod_name = dev_name(dev);
I'd admit I'm not that familiar with edac, but seems odd that a
module name field would have the dev_name.
Will fix when I got more inputs.
+ edac_device_free_ctl_info(edac_dev);
More a passing observation than a suggestion for this driver, but if there was
ever a place where it looked like a couple of devm_ allocation functions would
be useful, this is it;)
edac_dev = devm_device_alloc_ctrl_info(dev, ...)
...
devm_edac_device_add_device(dev, ...)
I agree.
I can implement the devm_* functions in separate patches as this is not
related to my patches (and not to delay this patches).