[PATCH] spi: spidev: add "generic-spidev" for compatible string

[PATCH] spi: spidev: add "generic-spidev" for compatible string

[egyszeregy]

From: Benjamin Szőke <egyszeregy@freemail.hu>

Spidev is a not an ASIC, IC or Sensor specific driver.
It is better to use a simple and generic compatible
string instead of many dummy vendor/product names
which are all just fake.

Signed-off-by: Benjamin Szőke <egyszeregy@freemail.hu>
---
 drivers/spi/spidev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index 95fb5f1c91c1..bbcc5b4e9c91 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -700,6 +700,7 @@ static const struct class spidev_class = {
 };
 
 static const struct spi_device_id spidev_spi_ids[] = {
+	{ .name = "generic-spidev" },
 	{ .name = "dh2228fv" },
 	{ .name = "ltc2488" },
 	{ .name = "sx1301" },
@@ -728,6 +729,7 @@ static int spidev_of_check(struct device *dev)
 }
 
 static const struct of_device_id spidev_dt_ids[] = {
+	{ .compatible = "generic-spidev", .data = &spidev_of_check },
 	{ .compatible = "cisco,spi-petra", .data = &spidev_of_check },
 	{ .compatible = "dh,dhcom-board", .data = &spidev_of_check },
 	{ .compatible = "lineartechnology,ltc2488", .data = &spidev_of_check },
-- 
2.43.5

Re: [PATCH] spi: spidev: add "generic-spidev" for compatible string

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 650 bytes --]

On Sun, Jul 14, 2024 at 10:23:03PM +0200, egyszeregy@freemail.hu wrote:
> From: Benjamin Szőke <egyszeregy@freemail.hu>
> 
> Spidev is a not an ASIC, IC or Sensor specific driver.
> It is better to use a simple and generic compatible
> string instead of many dummy vendor/product names
> which are all just fake.

> Signed-off-by: Benjamin Szőke <egyszeregy@freemail.hu>
> ---
>  drivers/spi/spidev.c | 2 ++
>  1 file changed, 2 insertions(+)

No, as previously and repeatedly discussed the DT describes the
hardware, not the software that happens to be used to control that
hardware.

You also need to document any new bindings.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] spi: spidev: add "generic-spidev" for compatible string

[Szőke Benjamin]

2024. 07. 15. 16:10 keltezéssel, Mark Brown írta:
> On Sun, Jul 14, 2024 at 10:23:03PM +0200, egyszeregy@freemail.hu wrote:
>> From: Benjamin Szőke <egyszeregy@freemail.hu>
>>
>> Spidev is a not an ASIC, IC or Sensor specific driver.
>> It is better to use a simple and generic compatible
>> string instead of many dummy vendor/product names
>> which are all just fake.
> 
>> Signed-off-by: Benjamin Szőke <egyszeregy@freemail.hu>
>> ---
>>   drivers/spi/spidev.c | 2 ++
>>   1 file changed, 2 insertions(+)
> 
> No, as previously and repeatedly discussed the DT describes the
> hardware, not the software that happens to be used to control that
> hardware.
> 
> You also need to document any new bindings.

If DT describes the hardware, yes this is why need a generic compatible string 
for SPIdev driver. SPIdev driver is a typical driver for boards which have just 
header pin for SPI connection and it is not defined what IC/Sensor will be 
connected on it later.

In normally if a developer start to use an IC/Sensor which has not yet any 
driver in Linux he/she should start to make it in a regular way and not 
hardcoding these fake compatible strings inside spidev.c and use it for longterm.

By the way, please send some reference link about the rules what you say for DT 
and please send the link for SPIdev binding documents, i can not find it, but 
you point on it all the time.

devicetree@vger.kernel.org
Please start a normal discussion about it with devicetree maintainers who can 
decided it real what need in this driver code for compatible strings. I do not 
think it is a good idea to append these list for +100 fake devices in the future 
because you say this is the rules for it.

Re: [PATCH] spi: spidev: add "generic-spidev" for compatible string

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3450 bytes --]

On Tue, Jul 16, 2024 at 09:41:08AM +0200, Szőke Benjamin wrote:
> 2024. 07. 15. 16:10 keltezéssel, Mark Brown írta:
> > On Sun, Jul 14, 2024 at 10:23:03PM +0200, egyszeregy@freemail.hu wrote:
> > > From: Benjamin Szőke <egyszeregy@freemail.hu>
> > > 
> > > Spidev is a not an ASIC, IC or Sensor specific driver.
> > > It is better to use a simple and generic compatible
> > > string instead of many dummy vendor/product names
> > > which are all just fake.
> > 
> > > Signed-off-by: Benjamin Szőke <egyszeregy@freemail.hu>
> > > ---
> > >   drivers/spi/spidev.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > 
> > No, as previously and repeatedly discussed the DT describes the
> > hardware, not the software that happens to be used to control that
> > hardware.
> > 
> > You also need to document any new bindings.
> 
> If DT describes the hardware, yes this is why need a generic compatible
> string for SPIdev driver. SPIdev driver is a typical driver for boards which
> have just header pin for SPI connection and it is not defined what IC/Sensor
> will be connected on it later.

What is preventing you using, for example, overlays to describe what
devices are being connected to your board?

> In normally if a developer start to use an IC/Sensor which has not yet any
> driver in Linux he/she should start to make it in a regular way and not
> hardcoding these fake compatible strings inside spidev.c and use it for
> longterm.

As I understand it the process would be:
- document the actual device you have
- add that compatible to spidev.c
- work on a driver in userspace
- drop the compatible from spidev.c and create a specific driver

The hardware at all points in time remains identical, and so the
description of it does not change depending on what driver the OS
happens to use.

Of course a developer could just develop a specific driver for the
hardware from the beginning, and not ever add its compatible to the
spidev driver.

> By the way, please send some reference link about the rules what you say for
> DT

For instance checkpatch will complain about your change:
WARNING: DT compatible string "generic-spidev" appears un-documented -- check ./Documentation/devicetree/bindings/
#35: FILE: drivers/spi/spidev.c:732:
+	{ .compatible = "generic-spidev", .data = &spidev_of_check },


> and please send the link for SPIdev binding documents, i can not find it,
> but you point on it all the time.

There is no binding for "spidev" because it is not a real device. The
devices currently bound to by the driver should be documented in various
locations.

> devicetree@vger.kernel.org
> Please start a normal discussion about it with devicetree maintainers who
> can decided it real what need in this driver code for compatible strings.

I do not understand what you are saying here. Are you telling Mark to
have a conversation with the devicetree maintainers?

> I
> do not think it is a good idea to append these list for +100 fake devices in
> the future because you say this is the rules for it.

What do you mean "+100 fake devices"? Surely the things being appended
to this list would be real devices that do not have a driver right now?

You keep talking about lots of fake compatibles, but actually
"generic-spidev" is the fake, and the specific compatibles for
different sensors etc are the real ones!

A wee bit confused,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]