Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 31 Jan 2019 22:54:18 +0100
From:   Thierry Reding <thierry.reding@gmail.com>
To:     Sam Ravnborg <sam@ravnborg.org>
Cc:     Sean Paul <sean@poorly.run>, dri-devel@lists.freedesktop.org,
        Daniel Vetter <daniel@ffwll.ch>,
        David Airlie <airlied@linux.ie>,
        Linus Walleij <linus.walleij@linaro.org>,
        Stefan Mavrodiev <stefan@olimex.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 0/19] drm/panel: drmP.h removal and DRM_DEV*
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"
Content-Disposition: inline
In-Reply-To: <20190131210312.GA8947@ravnborg.org>
User-Agent: Mutt/1.11.2 (2019-01-07)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


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