Am 21.04.22 um 11:13 schrieb Thomas Zimmermann:
Hi
Am 21.04.22 um 10:34 schrieb Christian König:
Am 21.04.22 um 10:30 schrieb Thomas Zimmermann:
(Resending, as some MLs didn't like the size of the origninal mail.)
Hi,
thanks for your submission. Some general comments:
* some functions are prefixed with dla_, others use nvdla_. It
seems arbitrary to me. Please use nvdla_ consistently throughout the
source code.
* For reporting errors, please use drm_err(), drm_warn(), etc. I
suggest to rearrange the error messages to not be located in the
innermost functions.
If you plan to have multiple instances of the driver loaded at the
same time, using drm_dev_err(), drm_dev_warn() etc.. would be even
better.
I thought that these functions exist, but looking for them now I
cannot find them. The macros DRM_DEV_ERR(), etc are deprecated.
That's what I meant with the comment below.
I wasn't 100%, but dev_err() etc.. seems to now be the preferred form
since that allows filtering for log messages of a certain device.
Regards,
Christian.
BTW: I'm still absolutely not keen to enforcing drm_* log functions.
So if you prefer to stick with pr_err() and dev_err() we could
discuss that once more.
Regards,
Christian.
* Could you please split this patch into smaller pieces? It
currently hits size limits of some mailing lists. Maybe add the
register constants separately.
Please find more review comments below. It's not a full review, but
at least something to start with.
Best regards
Thomas
