On 28-02-24, 15:27, Harald Mommer wrote:
+static int virtio_spi_probe(struct virtio_device *vdev)The print can be dropped I guess.
+{
+ 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) {
+ dev_err(&vdev->dev, "Kernel memory exhausted in %s()\n",
+ __func__);
+ return -ENOMEM;The comments for obvious statements are normally not required. There
+ }
+
+ priv = spi_controller_get_devdata(ctrl);
+ priv->vdev = vdev;
+ vdev->priv = priv;
+ dev_set_drvdata(&vdev->dev, ctrl);
+
+ 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);
+
+ /* Method to do a single SPI transfer */
are a couple of them here.
+ ctrl->transfer_one = virtio_spi_transfer_one;Maybe "Failed to" instead of "Cannot" ?
+
+ /* Initialize virtqueues */
+ err = virtio_spi_find_vqs(priv);
+ if (err) {
+ dev_err(&vdev->dev, "Cannot setup virtqueues\n");
+ return err;Maybe a blank line here.
+ }
+
+ 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 */
+ for (csi = 0; csi < ctrl->num_chipselect; csi++) {
+ dev_dbg(&vdev->dev, "Setting up CS=%u\n", csi);
+ board_info.chip_select = csi;
+ /* TODO: Discuss setting of board_info.mode */and here to improve readability.
+ 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)) {Not sure if this comment is required or not, since we don't add
+ 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;
+}
+
+static void virtio_spi_remove(struct virtio_device *vdev)
+{
+ struct spi_controller *ctrl = dev_get_drvdata(&vdev->dev);
+
+ /* Order: 1.) unregister controller, 2.) remove virtqueue */
similar ones while registering.
No real code changes. Some comments to be removed, some blank lines to be added, nothing urgent even in case the driver is integrated now locally by someone for some need. No re-test will be needed. Let's see what comes in addition. This is for next week, by than also the maintenance of the virtio mailing lists will have been done.+ spi_unregister_controller(ctrl);Other than that.
+ virtio_spi_del_vq(vdev);
+}
Reviewed-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>