Re: [virtio-dev] Re: [RFC PATCH v3 3/3] SPI: Add virtio SPI driver. - Correction

Haixu Cui
2024-03-11T09:28:55Z

Hello Harald,
My concern is if possible to create the udev(spidev) by adding the
device-tree node. Although the way of using the udev rule is fine, I
think the way of adding device-tree node also suitable for some scenarios.

Referring to Kumar's examples, I guess the virtio spi device-tree
should be like:

virtio-spi@4b013000 {
compatible = "virtio,mmio";
reg = <0x4b013000 0x1000>;
interrupts = <0 497 4>;

spi {
compatible = "virtio,device45";
#address-cells = <1>;
#size-cells = <0>;

spidev@0 {
compatible = "xxx";
reg = <0>;
spi-max-frequency = <100000>;
};
};
};

Just like virtio-i2c node, virtio-spi@4b013000 has three levels.
And the innermost, spidev node is to match spidev driver, to create
spidev(udev) device. I am working on this recently, but got some
stranger cases. Need more effort and time.

Harald, do you have any idea about this way? I'm looking forward to
it. Thanks a lot.

Haixu Cui

On 3/7/2024 12:18 AM, Harald Mommer wrote:
> Hello Haixu,
>
> not really sure what you mean and where the problem are. But we will
> find out.
>
> What I did in the device tree of some hardware board was
>
> virtio_mmio@4b013000 {
>         compatible = "virtio,mmio";
>         reg = <0x0 0x4b013000 0x0 0x1000>;
>         /* GIC_SPI = 0, IRQ_TYPE_LEVEL_HIGH = 4 */
>         interrupts = <0 497 4>;
>         spi,bus-num = <1234>;
>     };
>
> Simply added a line "spi,bus-num = <1234>;" in the device tree to
> configure the bus number.
>
> (There is no device tree for my very small qemu setup to check
> next/latest, also no full udev, therefore I've to live there with the
> default bus-num which is 0.)
>
> What I guess you mean is that the syntax of the device tree node should
> be changed having GPIO and I2C as template.
>
> And as you need more parameters for the board info, not only this single
> line for the bus number. May this be somewhat for an enhancement in a
> subsequent version?
>
> Why it's not easy to create the device node using the udev rule below in
> a full system with udev (vs. some minimal RAMDISK system) I don't
> understand. It's a single line, rest are comments.
>
> # Bind spi-virtio device spiB.C to user mode SPI device /dev/spidevB.C
> # Requires spi driver_override sysfs entry (Linux version 4.20+ and later)
> #
> # See also
> https://www.mail-archive.com/debian-arm@lists.debian.org/msg22090.html
> # and Documentation/spi/spidev.rst
> #
> #ACTION=="add", SUBSYSTEM=="spi", ENV{MODALIAS}=="spi:spi-virtio",
> PROGRAM+="/bin/sh -c 'echo spidev > %S%p/driver_override && echo %k >
> %S%p/subsystem/drivers/spidev/bind"
> ACTION=="add", SUBSYSTEM=="spi", ENV{MODALIAS}=="spi:spi-virtio",
> PROGRAM+="/bin/sh -c 'echo spidev > %S%p/driver_override && echo %k >
> %S/bus/spi/drivers/spidev/bind"
>
> It may be helpful if you could provide a proposal how exactly the device
> tree entry should look. This would also show which information is needed
> in addition to the bus number.
>
> What I guess is that you in the end it may look like this:
>
> virtio_mmio@4b013000 {
>         compatible = "virtio,mmio";
>         reg = <0x0 0x4b013000 0x0 0x1000>;
>         /* GIC_SPI = 0, IRQ_TYPE_LEVEL_HIGH = 4 */
>         interrupts = <0 497 4>;
>
>         spi {
>             bus-num = <1234>; /* Is it like this? */
>             /* More stuff here especially for the board_info I've
> currently no need for and no idea about that it may be needed by others
> */ /* ??? More info needed */
>         }
>     };
>
> Regards
> Harald Mommer
>
> On 06.03.24 08:48, Haixu Cui wrote:
>> Hello Harald,
>>
>>     In current driver, spi_new_device is used to instantiate the
>> virtio SPI device, spidevX.Y is created manually, this way seems not
>> flexible enough. Besides it's not easy to set the spi_board_info
>> properly.
>>
>>     Viresh Kumar has standardized the device tree node format for
>> virtio-i2c and virtio-gpio:
>>
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
>>
>>     In this way, the driver is unified, board customization only
>> depends on the device-tree node. It's easy to bring up spidev
>> automatically.
>>
>>     Look forward to your opinions. Thanks a lot.
>>
>> Haixu Cui
>>
>>
>> On 3/6/2024 1:54 AM, Harald Mommer wrote:
>>> Hello,
>>>
>>> looked again at my tinny setup and I've to add a small correction.
>>>
>>> It's not the way that I've no udev at all there. What is in place
>>> there is busybox mdev.
>>>
>>> Relevant part of /etc/init.d/rcS:
>>>
>>> #!/bin/sh
>>> mount -t proc none /proc
>>> mount -t sysfs none /sys
>>> depmod
>>> modprobe spi-virtio
>>> mdev -s
>>> mdev -d
>>>
>>> If I kill the "mdev -d" process my small script below does not make
>>> the /dev/spidev0.0 device node appear any more. Of course not, there
>>> must be some user mode process which does the job in the device
>>> directory.
>>>
>>> Regards
>>> Harald Mommer
>>>
>>> On 05.03.24 11:57, Harald Mommer wrote:
>>>> Hello,
>>>>
>>>> I took next/stable as base giving the exact tag/sha of the current
>>>> next/stable so that it's known what was used as base version even
>>>> when next/stable moves. The ordinary next tags are currently not of
>>>> best quality, gets better, therefore next/stable now. We were on
>>>> v6.8-rc7 yesterday with next/stable.
>>>>
>>>> VMM is qemu for the driver you have. But it's a specially modified
>>>> qemu which allows that we use our proprietary virtio SPI device as
>>>> backend.
>>>>
>>>> Proprietary virtio SPI device is started first, this is an own user
>>>> process in our architecture. Subsequently the special internal qemu
>>>> version is started. The virtio SPI driver is compiled as a module
>>>> and inserted manually by a startup script by "modprobe spi-virtio".
>>>> The driver goes live immediately.
>>>>
>>>> In this simple setup I do not have udev rules (no service supporting
>>>> udev => no rules) so no /dev/spidevX.Y automatically after the
>>>> driver went live. What I'm using to test the latest driver before
>>>> sending it to the mailing lists is really a naked kernel + a busybox
>>>> running in a ramdisk. The udev rule I've sent are used on some more
>>>> complete setup on real hardware.
>>>>
>>>> So without udev I have to bring this device up manually:
>>>>
>>>> In /etc/spidev-up.sh there is a script tp bring up /dev/spidev0.0
>>>> manually:
>>>>
>>>> #!/bin/sh
>>>> SPIDEV=spi0.0
>>>> echo spidev > /sys/bus/spi/devices/$SPIDEV/driver_override
>>>> echo $SPIDEV > /sys/bus/spi/drivers/spidev/bind
>>>>
>>>> Afterwards there is /dev/spidev0.0.
>>>>
>>>> In linux/tools/spi there are spidev_test.c and spidev_fdx.c. Those
>>>> (somewhat hacked locally, and I mean "hacked" to be able to test
>>>> somewhat more) are used to play around with /dev/spidev0.0.
>>>>
>>>> I can do this on my Laptop which has no underlying SPI hardware
>>>> which could be used as a backend for the virtio SPI device. The
>>>> proprietary virtio SPI device has a test mode to support this. Using
>>>> this test mode the driver does not communicate with a real backend
>>>> SPI device but does an internal simulation. For example, if I do a
>>>> half duplex read it always gives back the sequence 01 02 03 ...
>>>>
>>>> For full duplex it gives back what has been read but with letter
>>>> case changed, in loopback mode it gives back exactly what was sent.
>>>> With this test mode I could develop a driver and parts of the device
>>>> (device - real backend communication to an actual SPI device) on a
>>>> board which had no user space /dev/spiX.Y available which could have
>>>> served as backend for the virtio SPI device on the host.
>>>>
>>>> Slightly different module version is tested on real hardware with
>>>> the virtio SPI device not in test mode. "Tested on hardware" means
>>>> that device + module work for our special use case (some hardware
>>>> device using 8 bit word size) and the project team for which device
>>>> and driver have been made did until now not complain.
>>>>
>>>> Regards
>>>> Harald Mommer
>>>>
>>>> On 05.03.24 08:46, Haixu Cui wrote:
>>>>> Hello Harald,
>>>>>
>>>>> Thank you for your detailed expatiation. To my knowledge, you took
>>>>> Vanilla as the front-end, and VMM is QEMU. Can you please explain
>>>>> further how do you test the SPI transfer without the Vanilla
>>>>> userspace interface? Thanks again.
>>>>>
>>>>> Haixu Cui
>>>>
>>>>
>>