Re: [PATCH 2/2] drm/nvdla: Add driver support for NVDLA

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

 



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 think that's what I mean. Thanks for pointing this out.

Best regards
Thomas


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


--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux