linux-kernel - Re: [PATCH 1/2] regmap: Add DSI bus support

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190706010604.GG20625@sirena.org.uk>
Date:   Sat, 6 Jul 2019 02:06:04 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Jeffrey Hugo <jeffrey.l.hugo@...il.com>
Cc:     a.hajda@...sung.com, Laurent.pinchart@...asonboard.com,
        airlied@...ux.ie, daniel@...ll.ch, robdclark@...il.com,
        bjorn.andersson@...aro.org, dri-devel@...ts.freedesktop.org,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] regmap: Add DSI bus support

On Wed, Jul 03, 2019 at 02:45:12PM -0700, Jeffrey Hugo wrote:
> Add basic support with a simple implementation that utilizes the generic
> read/write commands to allow device registers to be configured.

This looks good to me but I really don't know anything about DSI,
I'd appreciate some review from other people who do.  I take it
there's some spec thing in DSI that says registers and bytes must
both be 8 bit?

A couple of minor comments, no need to resend just for these:

> +       payload[0] = (char)reg;
> +       payload[1] = (char)val;

Do you need the casts?

> +	ret = mipi_dsi_generic_write(dsi, payload, 2);
> +	return ret < 0 ? ret : 0;

Please just write an if statement, it helps with legibility.

> +struct regmap *__regmap_init_dsi(struct mipi_dsi_device *dsi,
> +				 const struct regmap_config *config,
> +				 struct lock_class_key *lock_key,
> +				 const char *lock_name)
> +{
> +	return __regmap_init(&dsi->dev, &dsi_bus, &dsi->dev, config,
> +			     lock_key, lock_name);
> +}
> +EXPORT_SYMBOL_GPL(__regmap_init_dsi);

Perhaps validate that the config is OK (mainly the register/value
sizes)?  Though I'm not sure it's worth it so perhaps not - up to
you.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)