Re: [PATCH v2 3/3] virtio-spi: Add virtio SPI driver.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.






[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux