Hello,
On 26.03.24 10:26, Haixu Cui wrote:
Looking at how i2c-virtio does it, it should be tied to the device
itself
instead of its parent:
Yes, it is
ctrl->dev.of_node = vdev->dev.of_node;
got it when looking into Viresh's code changes.
Right, some work was done in the past to standardize these compatibles:
$ git log -p --stat --reverse 0d8c9e7d4b40..694a1116b405
Here I would like the inner layer "spidev", to match then probe
the spidev driver, the "reg" is the chip select index.
"spi-max-frequency" is not necessary, while It doesn't matter.
You can also customize the inner layer to match your own driver.
In my test, I set the cs max number as 1, and in device tree node
inner layer, reg as 0. So certainly spidev driver probed and spidev0.0
is added successfully.
But then the driver proceed to the following code, chip select
index 0 device is created again, the driver fail with log: "chipselect
0 already in use".
The following lines
board_info.max_speed_hz = priv->max_freq_hz;
board_info.bus_num = (u16)ctrl->bus_num;
if (!(priv->mode_func_supported & VIRTIO_SPI_CS_HIGH))
board_info.mode = SPI_MODE_0;
else
board_info.mode = SPI_MODE_0 | SPI_CS_HIGH;
are moved before the sequence
err = spi_register_controller(ctrl);
if (err) {
dev_err(&vdev->dev, "Cannot register controller\n");
goto err_return;
}
if (vdev->dev.of_node) { // <=== This block is new
dev_dbg(&vdev->dev, "Final setup triggered by DT child node\n");
return 0;
}
for (csi = 0; csi < ctrl->num_chipselect; csi++) {
dev_dbg(&vdev->dev, "Setting up CS=%u\n", csi);
board_info.chip_select = csi;
if (!(priv->mode_func_supported & VIRTIO_SPI_CS_HIGH)) //
<=== This if else is moved up. It is csi invariant anyway.
board_info.mode = SPI_MODE_0;
else
board_info.mode = SPI_MODE_0 | SPI_CS_HIGH;
if (!spi_new_device(ctrl, &board_info)) {
dev_err(&vdev->dev, "Cannot setup device %u\n", csi);
spi_unregister_controller(ctrl);
err = -ENODEV;
goto err_return;
}
}
This means when there is no device tree entry everything behaves as
before (for loop executed) and if there is a device tree entry the chip
select will be setup already by spi_register_controller() and the for
loop need not and will not be executed not annoying anyone with
"chipselect 0 already in use".
I was undecided whether to keep Viresh Kumar's "reviewed by" in the
commit message because I'm unsure whether this is to be considered as a
substantial change. Changes are small but program logic is changed (vs.
change of comment, removal of trace, change of trace wording) so I think
it may be considered as a substantial change and therefore I'll remove
the "reviewed-by" now.