Re: [PATCH] spi: Add FSI-attached SPI controller driver

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

 




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.



[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