From: "andriy.shevchenko@linux.intel.com" <andriy.shevchenko@linux.intel.com>
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Aditya Garg <gargaditya08@live.com>,
"maarten.lankhorst@linux.intel.com"
<maarten.lankhorst@linux.intel.com>,
"mripard@kernel.org" <mripard@kernel.org>,
"airlied@gmail.com" <airlied@gmail.com>,
"simona@ffwll.ch" <simona@ffwll.ch>,
Kerem Karabay <kekrby@gmail.com>,
Atharva Tiwari <evepolonium@gmail.com>,
Aun-Ali Zaidi <admin@kodeit.net>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH v5 2/2] drm/tiny: add driver for Apple Touch Bars in x86 Macs
Date: Tue, 25 Feb 2025 15:27:37 +0200 [thread overview]
Message-ID: <Z73FSWR-DK0HxMb2@smile.fi.intel.com> (raw)
In-Reply-To: <9efdb233-2bca-4a5d-a6bc-de81fa96efb3@suse.de>
On Tue, Feb 25, 2025 at 12:59:43PM +0100, Thomas Zimmermann wrote:
> Am 25.02.25 um 12:01 schrieb andriy.shevchenko@linux.intel.com:
> > On Tue, Feb 25, 2025 at 10:48:53AM +0000, Aditya Garg wrote:
> > > > On 25 Feb 2025, at 4:17 PM, andriy.shevchenko@linux.intel.com wrote:
> > > > On Tue, Feb 25, 2025 at 10:36:03AM +0000, Aditya Garg wrote:
> > > > > > > On 25 Feb 2025, at 4:03 PM, andriy.shevchenko@linux.intel.com wrote:
> > > > > > On Tue, Feb 25, 2025 at 10:09:42AM +0000, Aditya Garg wrote:
...
> > > > > > > +static int appletbdrm_probe(struct usb_interface *intf,
> > > > > > > + const struct usb_device_id *id)
> > > > > > > +{
> > > > > > > + struct usb_endpoint_descriptor *bulk_in, *bulk_out;
> > > > > > > + struct device *dev = &intf->dev;
> > > > > > > + struct appletbdrm_device *adev;
> > > > > > > + struct drm_device *drm;
> > > > > > > + int ret;
> > > > > > > +
> > > > > > > + ret = usb_find_common_endpoints(intf->cur_altsetting, &bulk_in, &bulk_out, NULL, NULL);
> > > > > > > + if (ret) {
> > > > > > > + drm_err(drm, "Failed to find bulk endpoints\n");
> > > > > > This is simply wrong (and in this case even lead to crash in some circumstances).
> > > > > > drm_err() may not be used here. That's my point in previous discussions.
> > > > > > Independently on the subsystem the ->probe() for the sake of consistency and
> > > > > > being informative should only rely on struct device *dev,
> > > > > I'm not sure how drm_err works,
> > > > It's a macro.
> > > >
> > > > > but struct drm_device does have a struct device *dev as well.
> > > > Yes, but only when it's initialized.
> > > >
> > > > > Anyways, this is something I'll leave for Thomas to reply.
> > > > The code above is wrong independently on his reply :-)
> > > I'm kinda stuck between contrasting views of 2 kernel maintainers lol,
> > > so I said let Thomas reply.
> > Sure. I also want him to clarify my question about potential drm_err_probe().
>
> These threads get a little lengthy. What is the question?
How drm_err_probe() can be (consistently) implemented as there are and will be
cases when we want to return an error code with the message and having DRM devce
not being available, please?
Also, drm_err() has a downside of not checking for deferred probe and
potentially leads to the noisy log floods.
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2025-02-25 13:27 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-25 10:06 [PATCH v5 0/2] Touch Bar DRM driver for x86 Macs Aditya Garg
2025-02-25 10:07 ` [PATCH v5 1/2] drm/format-helper: Add conversion from XRGB8888 to BGR888 Aditya Garg
2025-02-25 10:09 ` [PATCH v5 2/2] drm/tiny: add driver for Apple Touch Bars in x86 Macs Aditya Garg
2025-02-25 10:33 ` andriy.shevchenko
2025-02-25 10:36 ` Aditya Garg
2025-02-25 10:46 ` andriy.shevchenko
2025-02-25 10:48 ` Aditya Garg
2025-02-25 11:01 ` andriy.shevchenko
2025-02-25 11:59 ` Thomas Zimmermann
2025-02-25 13:27 ` andriy.shevchenko [this message]
2025-02-25 13:53 ` Thomas Zimmermann
2025-02-25 14:17 ` andriy.shevchenko
2025-02-25 11:49 ` Aditya Garg
2025-02-25 11:58 ` Thomas Zimmermann
2025-02-25 13:25 ` andriy.shevchenko
2025-02-25 14:54 ` Aditya Garg
2025-02-25 14:56 ` Aditya Garg
2025-02-25 15:01 ` Aditya Garg
2025-02-25 15:10 ` Aditya Garg
2025-02-25 15:59 ` Aditya Garg
2025-02-25 16:06 ` Thomas Zimmermann
2025-02-25 16:09 ` Aditya Garg
2025-02-25 16:40 ` Aditya Garg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z73FSWR-DK0HxMb2@smile.fi.intel.com \
--to=andriy.shevchenko@linux.intel.com \
--cc=admin@kodeit.net \
--cc=airlied@gmail.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=evepolonium@gmail.com \
--cc=gargaditya08@live.com \
--cc=kekrby@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=simona@ffwll.ch \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox