lkml.org 
[lkml]   [2024]   [Mar]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2 3/3] virtio-spi: Add virtio SPI driver.
From
Hello,

As Mark recommended, I reserve the virtio_spi_probe function only
and list my comments.

On 3/4/2024 11:43 PM, Harald Mommer wrote:
> +static int virtio_spi_probe(struct virtio_device *vdev)
> +{
> + struct device_node *np = vdev->dev.parent->of_node;
> + struct virtio_spi_priv *priv;
> + struct spi_controller *ctrl;
> + int err;
> + u32 bus_num;
> + u16 csi;
> +
> + ctrl = devm_spi_alloc_host(&vdev->dev, sizeof(*priv));
> + if (!ctrl)
> + return -ENOMEM;
> +
> + priv = spi_controller_get_devdata(ctrl);
> + priv->vdev = vdev;
> + vdev->priv = priv;
> + dev_set_drvdata(&vdev->dev, ctrl);

ctrl->dev.of_node is not set, so the child node cannot be parsed. I
would say, it's necessary to support the virtio spi device node in the
format:

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

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

spidev {
compatible = "xxx";
reg = <0>;
spi-max-frequency = <100000>;
};
};
};
> +
> + init_completion(&priv->spi_req.completion);
> +
> + err = of_property_read_u32(np, "spi,bus-num", &bus_num);
> + if (!err && bus_num <= S16_MAX)
> + ctrl->bus_num = (s16)bus_num;
> +
> + virtio_spi_read_config(vdev);
> +
> + ctrl->transfer_one = virtio_spi_transfer_one;
> +
> + err = virtio_spi_find_vqs(priv);
> + if (err) {
> + dev_err(&vdev->dev, "Cannot setup virtqueues\n");
> + return err;
> + }
> +
> + err = spi_register_controller(ctrl);
> + if (err) {
> + dev_err(&vdev->dev, "Cannot register controller\n");
> + goto err_return;
> + }
> +
> + board_info.max_speed_hz = priv->max_freq_hz;
> + /* spi_new_device() currently does not use bus_num but better set it */
> + board_info.bus_num = (u16)ctrl->bus_num;
> +
> + /* Add chip selects to controller */

Creating the spi devices here statically, will introduce
conflicts if the same spi devices also in the device-tree.

> + 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))
> + 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;
> + }
> + }
> +
> + return 0;
> +
> +err_return:
> + vdev->config->del_vqs(vdev);
> + return err;
> +}
> +

\
 
 \ /
  Last update: 2024-05-27 15:52    [W:0.078 / U:0.640 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site