[PATCH linux] drivers/fsi: Initial stubs for FSI device driver

[Joel Stanley]

Hey Chris,

Looking better.

It looks like you did not use git send-email to send the patch. When I
try to apply it this happens:

$ git am ~/tmp/\[PATCH_linux\]_drivers_fsi\:_Initial_stubs_for_FSI_device_driver.mbox
Applying: drivers/fsi: Initial stubs for FSI device driver
error: patch failed: drivers/Makefile:172
error: drivers/Makefile: patch does not apply
.git/rebase-apply/patch:93: new blank line at EOF.
+
Patch failed at 0001 drivers/fsi: Initial stubs for FSI device driver

We use git format-patch to create the patch, and git send-email to
send it, as other workflows mess up the patch on the way.

Have a read of Documentation/SubmittingPatches and
Documentation/email-clients.txt for more background.

On Fri, Jul 1, 2016 at 12:15 AM, Christopher Bostic
<christopher.lee.bostic at gmail.com> wrote:
>  Initial stubs for FSI device driver.
>
> Signed-off-by: Christopher Bostic <cbostic at us.ibm.com>
> ---

It's common to put a changelog in here below the --, so reviewers know
what you've modified with respect to earlier versions. This part of
the patch is not part of the code, nor is it part of the commit
message that will end up in the git history, so it is free form and
informal. Check out
https://lists.ozlabs.org/pipermail/linuxppc-dev/2016-June/143899.html
for an example.

Something like:

Changes in V2:
  - Did foo
  - Fixed bar in header
  - Reworked baz following feedback from Daz

>  drivers/Makefile      |  1 +
>  drivers/fsi/Makefile  |  5 +++++
>  drivers/fsi/fsiinit.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/fsi/fsiinit.h | 41 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 104 insertions(+)
>  create mode 100644 drivers/fsi/Makefile
>  create mode 100644 drivers/fsi/fsiinit.c
>  create mode 100644 drivers/fsi/fsiinit.h
>
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 795d0ca..8ea7c58 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -172,3 +172,4 @@ obj-$(CONFIG_STM)           += hwtracing/stm/
>  obj-$(CONFIG_ANDROID)          += android/
>  obj-$(CONFIG_NVMEM)            += nvmem/
>  obj-$(CONFIG_FPGA)             += fpga/
> +obj-$(CONFIG_FSI)                          += fsi/
> diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
> new file mode 100644
> index 0000000..f547c08
> --- /dev/null
> +++ b/drivers/fsi/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# Makefile for the FSI bus specific drivers.
> +#
> +
> +obj-y          += fsiinit.o

You could take the opportunity to add the Kconfig bits for the bus as well.

> diff --git a/drivers/fsi/fsiinit.c b/drivers/fsi/fsiinit.c
> new file mode 100644
> index 0000000..d692ea3
> --- /dev/null
> +++ b/drivers/fsi/fsiinit.c
> @@ -0,0 +1,57 @@
> +/*
> + * FSI Master device driver
> + *
> + * Copyright 2016 IBM Corp.
> + *
> + * Christopher Bostic <cbostic at us.ibm.com>

Looks good.

> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include "fsiinit.h"
> +
> +MODULE_AUTHOR("Christopher Bostic cbostic at us.ibm.com");
> +MODULE_DESCRIPTION("FSI master device driver");
> +
> +#define FSIDD_MAJOR    0               /* FSI device driver major */
> +#define        FSIDD_TOSTR(x)  #x
> +#define        FSIDD_VERNO     4000
> +#define        FSIDD_VER(x)    FSIDD_TOSTR(x)
> +
> +struct fsidd fsidd = {         /* FSI device driver structure definition */
> +       .magic = FSI_DD_MAGIC,
> +       .strno = FSI_DD_STRNO,
> +};
> +
> +

Nit: you've got extra white space here. One line will do.

> +static int fsi_start(void)
> +{
> +       int rc = 0;
> +
> +

Nit: you've got extra white space here. One line will do.

> +       printk("FSI DD v:%d installation ok\n", FSIDD_VERNO);
> +       return rc;
> +}
> +
> +static void fsi_exit(void)
> +{
> +
> +}
> +

Now that you've had a few goes at the process, perhaps add a bit more
functionality in the next set of patches you send.

Cheers,

Joel