2023-01-19 17:12:00

by Alexandre Mergnat

[permalink] [raw]
Subject: [PATCH 0/2] Add MediaTek MT8365 SPI support

Hi,
This patch series adds SPI support for MT8365-EVK board.
The SPIDEV is enabled, it can be used through the board pin header,
as described directly on the PCB.

This series depends to another one which add support for
MT8365 SoC and EVK board. Link [1].

Test:
- Loopback MOSI and MISO pins
- Issue the following command:
spidev_test -D /dev/spidev0.0 -v
- RX line should be the same as TX line.

Regards,
Alex

[1]: https://lore.kernel.org/linux-mediatek/[email protected]/

To: Rob Herring <[email protected]>
To: Krzysztof Kozlowski <[email protected]>
To: Matthias Brugger <[email protected]>
To: Mark Brown <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Alexandre Mergnat <[email protected]>

---
Alexandre Mergnat (2):
arm64: dts: mediatek: add spidev support for mt8365-evk board
spi: spidev: add new mediatek support

arch/arm64/boot/dts/mediatek/mt8365-evk.dts | 24 ++++++++++++++++++++++++
drivers/spi/spidev.c | 2 ++
2 files changed, 26 insertions(+)
---
base-commit: 8b6cfcce3ce939db11166680a57253c39110f07e
change-id: 20230118-mt8365-spi-support-0d96bc55a4a0

Best regards,
--
Alexandre Mergnat <[email protected]>


2023-01-19 17:24:17

by Alexandre Mergnat

[permalink] [raw]
Subject: [PATCH 2/2] spi: spidev: add new mediatek support

Add the "mediatek,genio" compatible string to support Mediatek
SPI controller on the genio boards.

Signed-off-by: Alexandre Mergnat <[email protected]>
---
drivers/spi/spidev.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index 6313e7d0cdf8..e23b825b8d30 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -702,6 +702,7 @@ static const struct spi_device_id spidev_spi_ids[] = {
{ .name = "m53cpld" },
{ .name = "spi-petra" },
{ .name = "spi-authenta" },
+ { .name = "genio" },
{},
};
MODULE_DEVICE_TABLE(spi, spidev_spi_ids);
@@ -728,6 +729,7 @@ static const struct of_device_id spidev_dt_ids[] = {
{ .compatible = "menlo,m53cpld", .data = &spidev_of_check },
{ .compatible = "cisco,spi-petra", .data = &spidev_of_check },
{ .compatible = "micron,spi-authenta", .data = &spidev_of_check },
+ { .compatible = "mediatek,genio", .data = &spidev_of_check },
{},
};
MODULE_DEVICE_TABLE(of, spidev_dt_ids);

--
b4 0.10.1

2023-01-20 05:54:09

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 2/2] spi: spidev: add new mediatek support

On Thu, Jan 19, 2023 at 05:28:50PM +0100, Alexandre Mergnat wrote:
> Add the "mediatek,genio" compatible string to support Mediatek
> SPI controller on the genio boards.

> { .compatible = "cisco,spi-petra", .data = &spidev_of_check },
> { .compatible = "micron,spi-authenta", .data = &spidev_of_check },
> + { .compatible = "mediatek,genio", .data = &spidev_of_check },

We need a matching update to the binding document.

This does also seem like a terribly generic name - Google
suggests that this is actually a series of numbered products (eg,
Genio 700), perhaps we should be using the specific numbers here?
I guess users would care which they're talking to. It really
parses as being "generic I/O" which would be an end run around
describing the actual product though it's not actually that.


Attachments:
(No filename) (820.00 B)
signature.asc (499.00 B)
Download all attachments

2023-01-20 08:49:44

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH 2/2] spi: spidev: add new mediatek support

From: Alexandre Mergnat <[email protected]>

> Add the "mediatek,genio" compatible string to support Mediatek
> SPI controller on the genio boards.

What is the use case of having the spidev? What if I want to
connect a device with a linux driver to it? It seems like you
just want to expose the SPI bus on the pin header. There was a
similar discussion for a mikrobus connector [1].

-michael

[1] https://lore.kernel.org/linux-devicetree/YmFo+EntwxIsco%[email protected]/

2023-01-23 09:38:16

by Alexandre Mergnat

[permalink] [raw]
Subject: Re: [PATCH 2/2] spi: spidev: add new mediatek support

Le ven. 20 janv. 2023 à 09:20, Michael Walle <[email protected]> a écrit :
>
> From: Alexandre Mergnat <[email protected]>
>
> > Add the "mediatek,genio" compatible string to support Mediatek
> > SPI controller on the genio boards.
>
> What is the use case of having the spidev? What if I want to
> connect a device with a linux driver to it? It seems like you
> just want to expose the SPI bus on the pin header. There was a
> similar discussion for a mikrobus connector [1].
>

Hi Michael,

Yes I want to expose the SPI on the pin header for two reasons:
- It's an Evaluation Kit board, I believe exposing SPI helps new
customers to try/understand it.
- This board will join the KernelCI soon, this setup will help to do
SPI non regression tests for a fixed default configuration.

AFAII from [1] , you can easily modify the current spidev@0 (If you
don't want to keep userspace interface) or simply add (in the DTS or
overlay) another node foo@1 with a different compatible (and so on)
according to the chip you plug on the header pin.

Regards,
Alex

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/spi/spi-controller.yaml

2023-01-23 10:44:40

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH 2/2] spi: spidev: add new mediatek support

Hi,

Am 2023-01-23 10:37, schrieb Alexandre Mergnat:
> Le ven. 20 janv. 2023 à 09:20, Michael Walle <[email protected]> a écrit
> :
>>
>> From: Alexandre Mergnat <[email protected]>
>>
>> > Add the "mediatek,genio" compatible string to support Mediatek
>> > SPI controller on the genio boards.
>>
>> What is the use case of having the spidev? What if I want to
>> connect a device with a linux driver to it? It seems like you
>> just want to expose the SPI bus on the pin header. There was a
>> similar discussion for a mikrobus connector [1].
>>
> Yes I want to expose the SPI on the pin header for two reasons:

Then "mediatek,genio" doesn't really describe the hardware, does it?
If you read that linked thread, NXP was also trying exposing the SPI
bus on a pin header. IMHO this is just misusing the userspace spi-dev.

That being said, exposing something on a pinheader (or on a standardized
connector) seems like a common thing and we should be working towards
a good solution. I still think Robs proposal for the mikrobus connector
makes also sense for your case.

-michael

2023-01-23 12:19:42

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 2/2] spi: spidev: add new mediatek support

On Mon, Jan 23, 2023 at 10:37:58AM +0100, Alexandre Mergnat wrote:

> Yes I want to expose the SPI on the pin header for two reasons:
> - It's an Evaluation Kit board, I believe exposing SPI helps new
> customers to try/understand it.

That's not how this works. Anyone connecting something to the
SPI header will need to update the DT to reflect whatever they
have connected, if that is something that should be controlled
with spidev then they should add the compatible for that thing
to the driver. If that is something that has a regular driver
then the regular driver will be used.


Attachments:
(No filename) (589.00 B)
signature.asc (488.00 B)
Download all attachments

2023-01-23 14:58:17

by Alexandre Mergnat

[permalink] [raw]
Subject: Re: [PATCH 2/2] spi: spidev: add new mediatek support

Le lun. 23 janv. 2023 à 11:44, Michael Walle <[email protected]> a écrit :
>
> Hi,
>
> Am 2023-01-23 10:37, schrieb Alexandre Mergnat:
> > Le ven. 20 janv. 2023 à 09:20, Michael Walle <[email protected]> a écrit
> > :
> >>
> >> From: Alexandre Mergnat <[email protected]>
> >>
> >> > Add the "mediatek,genio" compatible string to support Mediatek
> >> > SPI controller on the genio boards.
> >>
> >> What is the use case of having the spidev? What if I want to
> >> connect a device with a linux driver to it? It seems like you
> >> just want to expose the SPI bus on the pin header. There was a
> >> similar discussion for a mikrobus connector [1].
> >>
> > Yes I want to expose the SPI on the pin header for two reasons:
>
> Then "mediatek,genio" doesn't really describe the hardware, does it?
> If you read that linked thread, NXP was also trying exposing the SPI
> bus on a pin header. IMHO this is just misusing the userspace spi-dev.
>
> That being said, exposing something on a pinheader (or on a standardized
> connector) seems like a common thing and we should be working towards
> a good solution. I still think Robs proposal for the mikrobus connector
> makes also sense for your case.
>

I don't think this is the same case. For the mikrobus case, it's
consistent to have a connector because it fit with other "add-on"
board which can be plug on the "mother board".
Here I've just a simple debug pin header. I've taken a look at
raspberry pi and Beaglebone black DTS, they don't use connector, but
DTS overlay to enable SPI. I think you're right when you say that I'm
misusing the userspace spi-dev.

2023-01-23 15:07:59

by Alexandre Mergnat

[permalink] [raw]
Subject: Re: [PATCH 2/2] spi: spidev: add new mediatek support

Le lun. 23 janv. 2023 à 13:19, Mark Brown <[email protected]> a écrit :
>
> On Mon, Jan 23, 2023 at 10:37:58AM +0100, Alexandre Mergnat wrote:
>
> > Yes I want to expose the SPI on the pin header for two reasons:
> > - It's an Evaluation Kit board, I believe exposing SPI helps new
> > customers to try/understand it.
>
> That's not how this works. Anyone connecting something to the
> SPI header will need to update the DT to reflect whatever they
> have connected, if that is something that should be controlled
> with spidev then they should add the compatible for that thing
> to the driver. If that is something that has a regular driver
> then the regular driver will be used.

Got it. I think this series should be dropped then. If someone needs
the SPI, then he should use overlay (or modify the DTS locally).
I thought I could use spidev to bring SPI into the userspace, to help
future users play with it ("/dev/spidev0.0").