All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Jani Nikula <jani.nikula@intel.com>
Cc: linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org,
	linux-block@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
	ath11k@lists.infradead.org, ath10k@lists.infradead.org,
	Kalle Valo <kvalo@codeaurora.org>,
	linux-wireless@vger.kernel.org,
	QCA ath9k Development <ath9k-devel@qca.qualcomm.com>
Subject: Re: [PATCH 1/6] relay: allow the use of const callback structs
Date: Thu, 19 Nov 2020 08:11:20 +0000	[thread overview]
Message-ID: <20201119081120.GA6149@infradead.org> (raw)
In-Reply-To: <20201118165320.26829-1-jani.nikula@intel.com>

> +/*
> + * rchan_callback wrappers. Call the callbacks if available, otherwise fall back
> + * to default behaviour.
> + */

This adds an overly long line.  That being said this behavior is pretty
normal for kernel APIs, so I'm not even sure we need it at all.

> +
> +/*
> + * subbuf_start() callback.
> + */

and this one is for sure completley useless.  Same for all the other
similar ones.


But taking one step back:  All instances implement create_buf_file
and remove_buf_file, which makes sense as that is the prime aim
of these methods.  So there is no point in making those optional.
subbuf_start_callback is overriden by two instances, so making that
optional totally makes sense.  buf_mapped and buf_unmapped are
never overriden, so they should be removed entirely.

More importantly there is no case that passes a NULL rchan_callbacks,
which makes complete sense as it wouldn't even create a file.  So
remove that case as well and just replace it with a sanity check in
relay_open().

Please also add a patch to mark all rchan_callbacks instances const
while you're at it.

WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: Jani Nikula <jani.nikula@intel.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	intel-gfx@lists.freedesktop.org, linux-wireless@vger.kernel.org,
	linux-kernel@vger.kernel.org, ath10k@lists.infradead.org,
	QCA ath9k Development <ath9k-devel@qca.qualcomm.com>,
	linux-block@vger.kernel.org, ath11k@lists.infradead.org,
	Kalle Valo <kvalo@codeaurora.org>
Subject: Re: [PATCH 1/6] relay: allow the use of const callback structs
Date: Thu, 19 Nov 2020 08:11:20 +0000	[thread overview]
Message-ID: <20201119081120.GA6149@infradead.org> (raw)
In-Reply-To: <20201118165320.26829-1-jani.nikula@intel.com>

> +/*
> + * rchan_callback wrappers. Call the callbacks if available, otherwise fall back
> + * to default behaviour.
> + */

This adds an overly long line.  That being said this behavior is pretty
normal for kernel APIs, so I'm not even sure we need it at all.

> +
> +/*
> + * subbuf_start() callback.
> + */

and this one is for sure completley useless.  Same for all the other
similar ones.


But taking one step back:  All instances implement create_buf_file
and remove_buf_file, which makes sense as that is the prime aim
of these methods.  So there is no point in making those optional.
subbuf_start_callback is overriden by two instances, so making that
optional totally makes sense.  buf_mapped and buf_unmapped are
never overriden, so they should be removed entirely.

More importantly there is no case that passes a NULL rchan_callbacks,
which makes complete sense as it wouldn't even create a file.  So
remove that case as well and just replace it with a sanity check in
relay_open().

Please also add a patch to mark all rchan_callbacks instances const
while you're at it.

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: Jani Nikula <jani.nikula@intel.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	intel-gfx@lists.freedesktop.org, linux-wireless@vger.kernel.org,
	linux-kernel@vger.kernel.org, ath10k@lists.infradead.org,
	QCA ath9k Development <ath9k-devel@qca.qualcomm.com>,
	linux-block@vger.kernel.org, ath11k@lists.infradead.org,
	Kalle Valo <kvalo@codeaurora.org>
Subject: Re: [PATCH 1/6] relay: allow the use of const callback structs
Date: Thu, 19 Nov 2020 08:11:20 +0000	[thread overview]
Message-ID: <20201119081120.GA6149@infradead.org> (raw)
In-Reply-To: <20201118165320.26829-1-jani.nikula@intel.com>

> +/*
> + * rchan_callback wrappers. Call the callbacks if available, otherwise fall back
> + * to default behaviour.
> + */

This adds an overly long line.  That being said this behavior is pretty
normal for kernel APIs, so I'm not even sure we need it at all.

> +
> +/*
> + * subbuf_start() callback.
> + */

and this one is for sure completley useless.  Same for all the other
similar ones.


But taking one step back:  All instances implement create_buf_file
and remove_buf_file, which makes sense as that is the prime aim
of these methods.  So there is no point in making those optional.
subbuf_start_callback is overriden by two instances, so making that
optional totally makes sense.  buf_mapped and buf_unmapped are
never overriden, so they should be removed entirely.

More importantly there is no case that passes a NULL rchan_callbacks,
which makes complete sense as it wouldn't even create a file.  So
remove that case as well and just replace it with a sanity check in
relay_open().

Please also add a patch to mark all rchan_callbacks instances const
while you're at it.

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: Jani Nikula <jani.nikula@intel.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	intel-gfx@lists.freedesktop.org, linux-wireless@vger.kernel.org,
	linux-kernel@vger.kernel.org, ath10k@lists.infradead.org,
	QCA ath9k Development <ath9k-devel@qca.qualcomm.com>,
	linux-block@vger.kernel.org, ath11k@lists.infradead.org,
	Kalle Valo <kvalo@codeaurora.org>
Subject: Re: [Intel-gfx] [PATCH 1/6] relay: allow the use of const callback structs
Date: Thu, 19 Nov 2020 08:11:20 +0000	[thread overview]
Message-ID: <20201119081120.GA6149@infradead.org> (raw)
In-Reply-To: <20201118165320.26829-1-jani.nikula@intel.com>

> +/*
> + * rchan_callback wrappers. Call the callbacks if available, otherwise fall back
> + * to default behaviour.
> + */

This adds an overly long line.  That being said this behavior is pretty
normal for kernel APIs, so I'm not even sure we need it at all.

> +
> +/*
> + * subbuf_start() callback.
> + */

and this one is for sure completley useless.  Same for all the other
similar ones.


But taking one step back:  All instances implement create_buf_file
and remove_buf_file, which makes sense as that is the prime aim
of these methods.  So there is no point in making those optional.
subbuf_start_callback is overriden by two instances, so making that
optional totally makes sense.  buf_mapped and buf_unmapped are
never overriden, so they should be removed entirely.

More importantly there is no case that passes a NULL rchan_callbacks,
which makes complete sense as it wouldn't even create a file.  So
remove that case as well and just replace it with a sanity check in
relay_open().

Please also add a patch to mark all rchan_callbacks instances const
while you're at it.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2020-11-19  8:11 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-18 16:53 [PATCH 1/6] relay: allow the use of const callback structs Jani Nikula
2020-11-18 16:53 ` [Intel-gfx] " Jani Nikula
2020-11-18 16:53 ` Jani Nikula
2020-11-18 16:53 ` Jani Nikula
2020-11-18 16:53 ` [PATCH 2/6] drm/i915: make relay callbacks const Jani Nikula
2020-11-18 16:53   ` [Intel-gfx] " Jani Nikula
2020-11-18 16:53 ` [PATCH 3/6] ath10k: " Jani Nikula
2020-11-18 16:53   ` [Intel-gfx] " Jani Nikula
2020-11-18 16:53   ` Jani Nikula
2020-11-18 17:13   ` Kalle Valo
2020-11-18 17:13     ` [Intel-gfx] " Kalle Valo
2020-11-18 17:13     ` Kalle Valo
2020-11-18 16:53 ` [PATCH 4/6] ath11k: " Jani Nikula
2020-11-18 16:53   ` [Intel-gfx] " Jani Nikula
2020-11-18 16:53   ` Jani Nikula
2020-11-19  9:31   ` Jani Nikula
2020-11-19  9:31     ` [Intel-gfx] " Jani Nikula
2020-11-19  9:31     ` Jani Nikula
2020-11-19 11:40     ` Kalle Valo
2020-11-19 11:40       ` [Intel-gfx] " Kalle Valo
2020-11-19 11:40       ` Kalle Valo
2020-11-18 16:53 ` [PATCH 5/6] ath9k: " Jani Nikula
2020-11-18 16:53   ` [Intel-gfx] " Jani Nikula
2020-11-18 17:03   ` Kalle Valo
2020-11-18 17:03     ` [Intel-gfx] " Kalle Valo
2020-11-18 17:15     ` Kalle Valo
2020-11-18 17:15       ` [Intel-gfx] " Kalle Valo
2020-11-18 16:53 ` [PATCH 6/6] blktrace: " Jani Nikula
2020-11-18 16:53   ` [Intel-gfx] " Jani Nikula
2020-11-18 18:43 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/6] relay: allow the use of const callback structs Patchwork
2020-11-18 18:45 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-11-18 19:13 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-11-19  4:08 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2020-11-19  8:11 ` Christoph Hellwig [this message]
2020-11-19  8:11   ` [Intel-gfx] [PATCH 1/6] " Christoph Hellwig
2020-11-19  8:11   ` Christoph Hellwig
2020-11-19  8:11   ` Christoph Hellwig
2020-11-19  8:11   ` Christoph Hellwig
2020-11-19  8:11     ` [Intel-gfx] " Christoph Hellwig
2020-11-19  8:11     ` Christoph Hellwig
2020-11-19  8:11     ` Christoph Hellwig
2020-11-23 18:01   ` Jani Nikula
2020-11-23 18:01     ` [Intel-gfx] " Jani Nikula
2020-11-23 18:01     ` Jani Nikula
2020-11-23 18:01     ` Jani Nikula

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=20201119081120.GA6149@infradead.org \
    --to=hch@infradead.org \
    --cc=ath10k@lists.infradead.org \
    --cc=ath11k@lists.infradead.org \
    --cc=ath9k-devel@qca.qualcomm.com \
    --cc=axboe@kernel.dk \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.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.