Messages in this thread | | | Date | Mon, 18 Mar 2024 17:39:45 +0800 | Subject | Re: [PATCH v2 3/3] virtio-spi: Add virtio SPI driver. | From | Haixu Cui <> |
| |
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; > +} > +
| |