All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Christian König" <ckoenig.leichtzumerken@gmail.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>,
	dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH] drm: Differentiate the lack of an interface from invalid parameter
Date: Wed, 12 Sep 2018 18:26:56 +0200	[thread overview]
Message-ID: <846395c9-3fd1-29cb-5c2b-3f863c61f538@gmail.com> (raw)
In-Reply-To: <153674353508.13043.14897507783645518501@skylake-alporthouse-com>

Am 12.09.2018 um 11:12 schrieb Chris Wilson:
> Quoting Daniel Vetter (2018-09-12 10:02:47)
>> On Wed, Sep 12, 2018 at 10:50 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>>> Quoting Daniel Vetter (2018-09-12 09:39:30)
>>>> On Wed, Sep 12, 2018 at 10:27 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>>>>> If the ioctl is not supported on a particular piece of HW/driver
>>>>> combination, report ENODEV so that it can be easily distinguished from
>>>>> both the lack of the ioctl and from a regular invalid parameter.
>>>>>
>>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>> Hm, I thought the canonical errno for "ioctl doesn't apply to this
>>>> device" is ENOTTY?
>>> That's ioctl doesn't exist. Sometimes it's nice to know the kernel knows
>>> about the ioctl but it isn't applicable. Either is fine for my purpose,
>>> which is to know the ioctl exists, I just need to distinguish it from
>>> EINVAL.
>>>
>>>> And ENODEV means that it applies, but atm nothing
>>>> plugged in, or device gone already.
>>>>
>>>> I found a few more modeset ioctl:
>>>> - drm_mode_gamma_set_ioctl, drm_mode_gamma_get_ioctl
>>>> - drm_mode_getconnector
>>>> - drm_mode_getcrtc, drm_mode_setcrtc
>>>> - drm_mode_getencoder
>>>> - drm_mode_create_lease_ioctl, drm_mode_list_lessees_ioctl, ...
>>>>
>>>> Ok now I stop looking through the grep thing, looks like we've been
>>>> using EINVAL consistently. I'm all for switching, it makes sense, but
>>>> I think we should at least try to be consistent across all kms ioctl.
>>> Ah, but we've been consistent on the other side and have been using
>>> ENODEV for the better part of a decade for this meaning (that the ioctl
>>> doesn't apply to this HW) :)
>> Hm indeed, we've been using either ENODEV or EINVAL for "this ioctl
>> doesn't exist/doesn't apply". ENODEV is clearly used in many places
>> (also outside of drm) for "hw absent/gone/disappeared". And we have
>> very few uses of ENOTTY. So I'm kinda leaning towards starting a new
>> trend here, and using ENOTTY for "this ioctl doesn't apply". I don't
>> think we need to differentiate this from the case of "this kernel has
>> no idea about this ioctl at all", since from a userspace pov there's
>> no difference really: Either way it can't use it.
>>
>> But I'm also fine if we're just sticking to ENODEV, just feels like
>> wasting a perfectly useful errno (and there's not many) if we don't
>> give ENOTTY some more use.
> In going through the drm_core_checks, I noticed one brave soul used
> -ENOTSUPP. How about that?

+1 for that as well and I'm pretty sure I have seen that in a couple of 
drivers if an IOCTL isn't supported for a specific device.

Christian.

> -Chris
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2018-09-12 16:26 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-12  8:27 [PATCH] drm: Differentiate the lack of an interface from invalid parameter Chris Wilson
2018-09-12  8:39 ` Daniel Vetter
2018-09-12  8:50   ` Chris Wilson
2018-09-12  9:02     ` Daniel Vetter
2018-09-12  9:12       ` Chris Wilson
2018-09-12 16:26         ` Christian König [this message]
2018-09-12  8:58 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-09-12  9:26 ` [PATCH v2] " Chris Wilson
2018-09-12  9:29 ` [PATCH v3] " Chris Wilson
2018-09-13 13:30   ` Daniel Vetter
2018-09-13 19:20     ` Chris Wilson
2018-09-14 16:58       ` Chris Wilson
2018-09-12  9:58 ` ✓ Fi.CI.BAT: success for drm: Differentiate the lack of an interface from invalid parameter (rev3) Patchwork
2018-09-12 12:23 ` ✓ Fi.CI.IGT: " Patchwork
2018-09-13 19:50 ` ✓ Fi.CI.BAT: success for drm: Differentiate the lack of an interface from invalid parameter (rev4) Patchwork
2018-09-13 21:37 ` ✓ Fi.CI.IGT: " Patchwork

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=846395c9-3fd1-29cb-5c2b-3f863c61f538@gmail.com \
    --to=ckoenig.leichtzumerken@gmail.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=christian.koenig@amd.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.