Re: [PATCH v2 2/4] edac: Add support for Amazon's Annapurna Labs L1 EDAC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux