Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 27 May 2021 11:10:21 +0100
From:   Mark Brown <broonie@kernel.org>
To:     Christian Gmeiner <christian.gmeiner@gmail.com>
Cc:     linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org
Subject: Re: [PATCH] spidev: add platform driver support
Message-ID: <YK9wDd/+c1uAjwk7@sirena.org.uk>
References: <20210527084531.18989-1-christian.gmeiner@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="r3c1S3HTTKrvMj23"
Content-Disposition: inline
In-Reply-To: <20210527084531.18989-1-christian.gmeiner@gmail.com>
Precedence: bulk


--r3c1S3HTTKrvMj23
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, May 27, 2021 at 10:45:15AM +0200, Christian Gmeiner wrote:

> This makes it possible to use spidev in combination with the
> MFD subsystem. The MFD subsystem add platform_driver devices.

This is a really strange thing to want to do so it needs a
changelog which explains what the goal is and why this is a good
way of accomplishing that goal.

> +static int spidev_platform_probe(struct platform_device *pdev)
> +{
> +	struct device *parent = pdev->dev.parent;
> +	struct spi_device *spi;
> +
> +	if (strcmp(parent->bus->name, "spi"))
> +		return -ENODEV;
> +
> +	spi = to_spi_device(parent);
> +
> +	/* This only works if no drvdata is stored */
> +	if (spi_get_drvdata(spi)) {
> +		dev_err(&pdev->dev, "drvdata is not NULL\n");

Why?

> +		return -EOPNOTSUPP;
> +	}
> +
> +	return spidev_probe(spi);

This really does not seem like a good idea, this is exposing the
entire device to userspace in a completely unstructured fashion
while there will be other drivers controlling the same hardware.
That seems like it's asking for trouble, there's absolutely
nothing ensuring that userspace doesn't break things the drivers
are doing.

I really don't think it makes sense to mix kernel drivers with
unmoderated userspace access to the hardware in a single driver.

> +static struct platform_driver spidev_platfoem_driver = {

platfoem?

--r3c1S3HTTKrvMj23
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmCvcAwACgkQJNaLcl1U
h9CA1gf+LtoavMTh0+w1zEhGPCH+/YYD6y9bOFbrKoh5teYYrLwMur2ogt9d50Jw
qlyqeieFrDY9hWjQcDBUnT/Lt6Z2FEZTWpp4fmvhQwLQodHHkkDbpUaTDbgHBX7E
cP4jxSHa3TaecEMV+405QQmKCX6g7EQXVQygipgAcYU2bxnt89GGWxNSuSdU+0SS
aWNNj/Mx21sgUzIJQ0fJ4CL8I8duBXGoj3eNj3V2WVui7dTShyZUUYYrVZ/3nQ1l
UnZZX2G0YC/IARmpCPuNWtkG+S9WFgvkQ1aeC22cBkNK9d8oqFG1oonPusv0aJcc
vVJcrTDQNbbLxo5iChzksEZENG0exw==
=vDdo
-----END PGP SIGNATURE-----

--r3c1S3HTTKrvMj23--