linux-kernel - Re: [PATCH 1/5] ASoC: amd: Registering device endpoints using MFD framework

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: <20190926182300.GD2036@sirena.org.uk>
Date:   Thu, 26 Sep 2019 11:23:00 -0700
From:   Mark Brown <broonie@...nel.org>
To:     Ravulapati Vishnu vardhan rao 
        <Vishnuvardhanrao.Ravulapati@....com>
Cc:     Alexander.Deucher@....com, Liam Girdwood <lgirdwood@...il.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        Vijendar Mukunda <Vijendar.Mukunda@....com>,
        Maruthi Srinivas Bayyavarapu <Maruthi.Bayyavarapu@....com>,
        Sanju R Mehta <sanju.mehta@....com>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Colin Ian King <colin.king@...onical.com>,
        "moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..." 
        <alsa-devel@...a-project.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/5] ASoC: amd: Registering device endpoints using MFD
 framework

On Fri, Sep 27, 2019 at 04:37:35AM +0530, Ravulapati Vishnu vardhan rao wrote:

> -#define ACP3x_PHY_BASE_ADDRESS 0x1240000
> -#define	ACP3x_I2S_MODE	0
> -#define	ACP3x_REG_START	0x1240000
> -#define	ACP3x_REG_END	0x1250200
> -#define I2S_MODE	0x04
> -#define	BT_TX_THRESHOLD 26
> -#define	BT_RX_THRESHOLD 25
> -#define ACP3x_POWER_ON 0x00
> -#define ACP3x_POWER_ON_IN_PROGRESS 0x01
> -#define ACP3x_POWER_OFF 0x02
> -#define ACP3x_POWER_OFF_IN_PROGRESS 0x03
> +#define ACP3x_DEVS		3
> +#define ACP3x_PHY_BASE_ADDRESS	0x1240000
> +#define	ACP3x_I2S_MODE		0
> +#define	ACP3x_REG_START		0x1240000
> +#define	ACP3x_REG_END		0x1250200

A large part of this appears to be unrelated indentation changes,
these should be split out into a separate patch.

> +static struct device *get_mfd_cell_dev(const char *device_name, int r)
> +{
> +	char auto_dev_name[25];
> +	struct device *dev;
> +
> +	snprintf(auto_dev_name, sizeof(auto_dev_name),
> +		 "%s.%d.auto", device_name, r);
> +	dev = bus_find_device_by_name(&platform_bus_type,
> +					NULL, auto_dev_name);
> +	dev_info(dev, "device %s added\n", auto_dev_name);

Remove this log message, it's going to be very noisy.

> +		r = mfd_add_hotplug_devices(adata->parent, adata->cell,	3);
> +		for (i = 0; i < 3 ; i++)
> +			dev = get_mfd_cell_dev(adata->cell[i].name, i);

What is this doing?  We never look at the result of this
get_mfd_cell_dev() and having a function like this suggests that
there's some abstraction issue here.

> +	kfree(adata->cell);
>  	iounmap(adata->acp3x_base);
> +	/*ignore device status and return driver probe error*/
> +	return -ENODEV;
>  release_regions:

This looks broken, as well as discarding error codes (making
things harder to diagnose) it means we stop unwinding things and
leave the rest of the resources lying around.

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