From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH v1 0/19] drm/panel: drmP.h removal and DRM_DEV* Date: Thu, 31 Jan 2019 22:54:18 +0100 Message-ID: <20190131215417.GA13156@mithrandir> References: <20190131192619.9763-1-sam@ravnborg.org> <20190131200723.GZ114153@art_vandelay> <20190131210312.GA8947@ravnborg.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="tKW2IUtsqtDRztdT" Return-path: Content-Disposition: inline In-Reply-To: <20190131210312.GA8947@ravnborg.org> Sender: linux-kernel-owner@vger.kernel.org To: Sam Ravnborg Cc: Sean Paul , dri-devel@lists.freedesktop.org, Daniel Vetter , David Airlie , Linus Walleij , Stefan Mavrodiev , linux-kernel@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org --tKW2IUtsqtDRztdT Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jan 31, 2019 at 10:03:12PM +0100, Sam Ravnborg wrote: > Hi Sean. >=20 > > Hey Sam, > > Thanks for the patchset, this will make dmesg grepping easier! One comm= ent, and > > you're going to hate me for it: Why use DRM_DEV* instead of DRM_*? > >=20 > > When I introduced DRM_DEV, it was to cover the case where there are mul= tiple > > instances of the same driver (ie: dual-channel mipi, multiple crtcs, et= c). I > > suppose that _could_ happen in the panel space, but it seems more unlik= ely than > > not. >=20 > The rationale for using DRM_DEV* are solely that if a struct device * is = avalible, > then we can use this to provide more information about the origin of the = logging. >=20 > I have not testet it - but from browsing the code I could not see that > DRM_ERROR and friends picked up the module name. > If DRM_ERROR is the right choice I will redo the patches - no problem. >=20 > But if we loose the module name then the DRM_DEV* variants are preferable= IMO. I personally like the DRM_DEV_* variants better because of the additional information that they provide. That can be useful when grepping logs etc. I'm slightly on the fence about this patch. The unwritten, and admittedly fuzzy, rules that I've been using so far are that dev_*() are used or messages that have to do with the panel device itself, whereas DRM_* variants are used for things that are actually related to DRM. So typically this would mean that roughly everything in ->probe() or ->remove() would be dev_*(), while the rest would be DRM_DEV_*(). The reason for this is that during most of ->probe() there's not really any connection to DRM. In many cases the DRM device doesn't even exist yet. Consider component/master setups where some display component will wait for the panel to be registered before binding the master. So I find it confusing to have the DRM style messages when there's actually no DRM going on yet. That said, I understand that this might not be an immediately obvious rule and it might not be followed rigorously, which both quite possibly contribute to the perception that the messages are all inconsistent. It always made sense to me, but if everyone else thinks that it's totally nuts, I'm sure I can find a way to get over it. Thierry --tKW2IUtsqtDRztdT Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAlxTboUACgkQ3SOs138+ s6Gcug/+JZImCInI8kyPqT4pN4qRbXmCAFeUpKSh19yHN5MnisK14xu14qClW/q2 3cv+JxY6U93cvWkSn+hfLU0K4TRXS/dmz6AB5AC/lj3ADOUxFW75gweGlCVoKMv2 bwYXlOACghDjdW7uhpgHeTxB/TPkE2Oaz9+47SVZBFbeCFTWx6XfQ5tfQG9BperP rXXHyZbOHCEu2yoov1zfIWfxxTA9VsSluKB6AX62EJ1XGMQHC+tPJtPXbryZMc5v AZzaQjv8Q+yitPwrZLmQF7vLJ5C7l/NhOy20BrlHW38gDC8Q2q4quwNaoP8W6mE6 6vp/W3YB7O5YLy5v/fYz+wf9smq3A3o/o3qLAN7g9koMc8OnMCPDLznM4xnkAF69 U0krpggxq/T7aDqroazZI1/WQchbq2+bkSn3xv3CqY3SypEpnoqGhHjR3+cYP3W3 6FnJqpdZ6og4In8P5adVS2KnBXWCZM4iQnFsH+pTSkPv6Wfb6Z3fkeEKX4c5VTXc oEJuNp7BSO1mJQjCHkwiuR2j6o/iEF6gNTuO4OMCwcqjGM61Fyz1BtIat3hpF/3E rBWZSS8tlz98+382Zvbc6w92fbu0Gyqky2JGju3qnUdp3e+c4lADLhQWKIXzw0dS FZd+w9iEO3fBFR/OpcA/KoPE0M1b9FnN54tCCmgdnnVR3W8jBjs= =XIfx -----END PGP SIGNATURE----- --tKW2IUtsqtDRztdT--