linux-kernel - Re: [RFC PATCH] ES938 support for ES18xx driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20130919173649.GA18537@rhlx01.hs-esslingen.de>
Date:	Thu, 19 Sep 2013 19:36:49 +0200
From:	Andreas Mohr <andi@...as.de>
To:	Ondrej Zary <linux@...nbow-software.org>
Cc:	Andreas Mohr <andi@...as.de>, alsa-devel@...a-project.org,
	krzysztof.h1@...pl,
	Kernel development list <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH] ES938 support for ES18xx driver

Hi,

On Mon, Sep 16, 2013 at 10:20:54PM +0200, Ondrej Zary wrote:
> +static int snd_es938_read_reg(struct snd_es938 *chip, u8 reg, u8 *out)
> +{
> +	u8 buf[8];
> +	int i = 0, res;
> +	u8 sysex[] = { MIDI_CMD_COMMON_SYSEX, 0x00, 0x00, ES938_ID,
> +		       ES938_CMD_REG_R, reg, MIDI_CMD_COMMON_SYSEX_END };
> +	unsigned long end_time;
> +
> +	snd_rawmidi_kernel_write(chip->rfile.output, sysex, sizeof(sysex));
> +
> +	memset(buf, 0, sizeof(buf));
> +	end_time = jiffies + msecs_to_jiffies(100);
> +	while (i < sizeof(buf)) {
> +		res = snd_rawmidi_kernel_read(chip->rfile.input, buf + i,
> +					      sizeof(buf) - i);
> +		if (res > 0)
> +			i += res;
> +		if (time_after(jiffies, end_time))
> +			return -1;
> +	}

Forgive me, but I seem to faintly remember that it's preferred for
user code to tend towards non-jiffies-based timing (IIRC due to
NOHZ efforts etc.).
So if that actually is the case, then one might want to change it to
less jiffies-dependent handling (use non-jiffies helpers), but then how?

> +static void snd_es938_write_reg(struct snd_es938 *chip, u8 reg, u8 val)
> +{
> +	u8 sysex[] = { MIDI_CMD_COMMON_SYSEX, 0x00, 0x00, ES938_ID,
> +		       ES938_CMD_REG_W, reg, val, MIDI_CMD_COMMON_SYSEX_END };
> +
> +	snd_rawmidi_kernel_write(chip->rfile.output, sysex, sizeof(sysex));

snd_rawmidi_kernel_write() is prototyped as const unsigned char *buf,
so perhaps...
const unsigned char buf[] = ...;?
(static const probably not so useful
e.g. since static const may be rather prone to cache misses)

Though personally I do favour u8, since "char" seems so unsuitably stringy -
but the prototype currently doesn't offer it typed that way...

(Almost-)Reviewed-By: Andreas Mohr <andim2@...rs.sf.net>

Andreas Mohr
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/