On 1/30/20 8:46 AM, Mark Brown wrote:
On Wed, Jan 29, 2020 at 02:08:24PM -0600, Eddie James wrote:
Overall this looks good, some comments below but they're all fairly
minor.
Thanks for the quick review! I'll fix what you've suggested below.
Thanks,
Eddie
+++ b/drivers/spi/spi-fsi.c
@@ -0,0 +1,547 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) IBM Corporation 2020
+ */
Please make the entire comment a C++ one so things look more
intentional.
+
+static int fsi_spi_data_in(u64 in, u8 *rx, int len)
+{
+ int i;
+ int num_bytes = len > 8 ? 8 : len;
Please write normal conditional statements to improve legibility, the
ternery operator isn't really needed here.
+static int fsi_spi_reset(struct fsi_spi *ctx)
+{
+ int rc;
+
+ dev_info(ctx->dev, "Resetting SPI controller.\n");
This should be lowered to dev_dbg() at most, it's not really adding
anything otherwise.
+static int fsi_spi_remove(struct device *dev)
+{
+ return 0;
+}
Remove empty functions, if they can safely be empty then it should be
possible to omit them.
+static const struct fsi_device_id fsi_spi_ids[] = {
+ { FSI_ENGID_SPI, FSI_VERSION_ANY },
+ { }
+};
This needs a MODULE_DEVICE_TABLE annotation.
