linux-kernel - Re: [PATCH v3 4/4] input: touchscreen: add SPI support for Goodix Berlin Touchscreen IC

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: <ZJmMJxXxLrC9Xevi@nixie71>
Date:   Mon, 26 Jun 2023 08:01:27 -0500
From:   Jeff LaBundy <jeff@...undy.com>
To:     Neil Armstrong <neil.armstrong@...aro.org>
Cc:     Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Bastien Nocera <hadess@...ess.net>,
        Hans de Goede <hdegoede@...hat.com>,
        Henrik Rydberg <rydberg@...math.org>,
        linux-input@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 4/4] input: touchscreen: add SPI support for Goodix
 Berlin Touchscreen IC

Hi Neil,

On Mon, Jun 26, 2023 at 09:02:16AM +0200, Neil Armstrong wrote:

[...]

> > > +static int goodix_berlin_spi_probe(struct spi_device *spi)
> > > +{
> > > +	struct regmap_config *regmap_config;
> > > +	struct regmap *regmap;
> > > +	size_t max_size;
> > > +	int error = 0;
> > > +
> > > +	regmap_config = devm_kmemdup(&spi->dev, &goodix_berlin_spi_regmap_conf,
> > > +				     sizeof(*regmap_config), GFP_KERNEL);
> > > +	if (!regmap_config)
> > > +		return -ENOMEM;
> > 
> > Is there any reason we cannot simply pass goodix_berlin_spi_regmap_conf to
> > devm_regmap_init() below? Why to duplicate and pass the copy?
> > 
> > For reference, BMP280 in IIO is a similar example of a device with regmap
> > sitting atop a bespoke SPI protocol; it does not seem to take this extra
> > step.
> 
> The goodix_berlin_spi_regmap_conf copy is modified after with the correct
> max raw read/write size, and I'm not a fan of modifying a global structure
> that could be use for multiple probes, I can make a copy in a stack variable
> if it feels simpler.

Ah, that makes sense; in that case, the existing implementation seems fine
to me. No changes necessary.

Correct me if I'm wrong, but the stack variable method wouldn't work since
that memory is gone after goodix_berlin_spi_probe() returns.

Kind regards,
Jeff LaBundy