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

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

[Christopher Bostic]

 Initial stubs for FSI device driver.

Signed-off-by: Christopher Bostic <cbostic@us.ibm.com>
---
 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
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@us.ibm.com>
+ *
+ * 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@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,
+};
+
+
+static int fsi_start(void)
+{
+       int rc = 0;
+
+
+       printk("FSI DD v:%d installation ok\n", FSIDD_VERNO);
+       return rc;
+}
+
+static void fsi_exit(void)
+{
+
+}
+
+static int __init fsi_init(void)
+{
+       int rc = 0;
+
+       /* Set up the host controller */
+       fsi_start();
+       return rc;
+}
+
+module_init(fsi_init);
+module_exit(fsi_exit);
+
diff --git a/drivers/fsi/fsiinit.h b/drivers/fsi/fsiinit.h
new file mode 100644
index 0000000..70c3e13
--- /dev/null
+++ b/drivers/fsi/fsiinit.h
@@ -0,0 +1,41 @@
+/*
+ * FSI Master device driver structure definitions and defines
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * Christopher Bostic <cbostic@us.ibm.com>
+ *
+ * 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.
+ */
+
+#ifndef DRIVERS_FSIINIT_H
+#define DRIVERS_FSIINIT_H
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/device.h>
+#include <linux/workqueue.h>
+#include <linux/hrtimer.h>
+
+#define FSI_DD_MAGIC   0x64644632      /* ddF2 */
+#define FSI_DD_STRNO   0x1             /* Structure version number */
+
+struct fsidd {                         /* FSI Main structure */
+       unsigned long magic;            /* Magic number */
+       unsigned long strno;            /* Structure version number */
+       dev_t major;                    /* Major number of device */
+       struct workqueue_struct *hotp_wq;       /* Hot plug work queue */
+       wait_queue_head_t error_wq;     /* Wait queue for port errors */
+       wait_queue_head_t lbus_wq;      /* Wait queue for lbus events */
+       wait_queue_head_t irq_wq;       /* Wait queue for IRQ loops */
+       wait_queue_head_t link_wq;      /* Wait queue for link changes */
+       unsigned long state;            /* State driver is in */
+       struct device dev;              /* Anchor point in /sys/kernel */
+};
+
+#define        to_fsidd(a)             container_of(a, struct fsidd, kobj)
+
+#endif /* DRIVERS_FSIINIT_H */
-- 
1.8.2.2

Re: [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@gmail.com> wrote:
>  Initial stubs for FSI device driver.
>
> Signed-off-by: Christopher Bostic <cbostic@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@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@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

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

[Joel Stanley]

On Fri, Jul 8, 2016 at 6:40 AM,  <christopher.lee.bostic@gmail.com> wrote:
> From: Christopher Bostic <cbostic@us.ibm.com>
>
> Signed-off-by: Christopher Bostic <cbostic@us.ibm.com>
> ---
>
> Changes to this patch based on reviewer feedback
>
> V2:
> - Removed enums in fsiinit.h and replaced with #defines
> - Added proper file copyright and license headers
> - Added certificate of origin
> - Removed version string
> - Replace kobject with struct device in struct fsidd
> - Suggestions to implement standard bus_type will be implemented in
>   later patches
>
> V3:
> - Removed white space
> - Suggestions to add Kconfig will be added in follow on patches
>
> V4:
> - Removed blank line at end of fsiinit.c

Nice, the patch now applies:

$ git am ~/Downloads/chris.patch
Applying: drivers/fsi: Initial stubs for FSI device driver.
$ echo $?
0

You indicate that this is patch version four, but you haven't put that
in the subject. git format-patch can do this for you with the -v
option:

       -v <n>, --reroll-count=<n>
           Mark the series as the <n>-th iteration of the topic. The output
           filenames have v<n> prepended to them, and the subject prefix
           ("PATCH" by default, but configurable via the --subject-prefix
           option) has ` v<n>` appended to it. E.g.  --reroll-count=4 may
           produce v4-0001-add-makefile.patch file that has "Subject: [PATCH
           v4 1/20] Add makefile" in it

I think you also need to set your username in your git configuration
so we see the emails as coming from you.

$ git config --global user.name "Chris Bostic"

I don't think you need to send another revision for that though.

The next step should be to include this patch as the first in a series
that adds the implementation of the bus.

Cheers,

Joel


> ---
>  drivers/Makefile      |  1 +
>  drivers/fsi/Makefile  |  5 +++++
>  drivers/fsi/fsiinit.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/fsi/fsiinit.h | 41 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 101 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..63019ff 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
> diff --git a/drivers/fsi/fsiinit.c b/drivers/fsi/fsiinit.c
> new file mode 100644
> index 0000000..042e680
> --- /dev/null
> +++ b/drivers/fsi/fsiinit.c
> @@ -0,0 +1,54 @@
> +/*
> + * FSI Master device driver
> + *
> + * Copyright 2016 IBM Corp.
> + *
> + * Christopher Bostic <cbostic@us.ibm.com>
> + *
> + * 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@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,
> +};
> +
> +static int fsi_start(void)
> +{
> +       int rc = 0;
> +
> +       printk("FSI DD v:%d installation ok\n", FSIDD_VERNO);
> +       return rc;
> +}
> +
> +static void fsi_exit(void)
> +{
> +
> +}
> +
> +static int __init fsi_init(void)
> +{
> +       int rc = 0;
> +
> +       /* Set up the host controller */
> +       fsi_start();
> +       return rc;
> +}
> +
> +module_init(fsi_init);
> +module_exit(fsi_exit);
> diff --git a/drivers/fsi/fsiinit.h b/drivers/fsi/fsiinit.h
> new file mode 100644
> index 0000000..70c3e13
> --- /dev/null
> +++ b/drivers/fsi/fsiinit.h
> @@ -0,0 +1,41 @@
> +/*
> + * FSI Master device driver structure definitions and defines
> + *
> + * Copyright 2016 IBM Corp.
> + *
> + * Christopher Bostic <cbostic@us.ibm.com>
> + *
> + * 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.
> + */
> +
> +#ifndef DRIVERS_FSIINIT_H
> +#define DRIVERS_FSIINIT_H
> +
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/device.h>
> +#include <linux/workqueue.h>
> +#include <linux/hrtimer.h>
> +
> +#define FSI_DD_MAGIC   0x64644632      /* ddF2 */
> +#define FSI_DD_STRNO   0x1             /* Structure version number */
> +
> +struct fsidd {                         /* FSI Main structure */
> +       unsigned long magic;            /* Magic number */
> +       unsigned long strno;            /* Structure version number */
> +       dev_t major;                    /* Major number of device */
> +       struct workqueue_struct *hotp_wq;       /* Hot plug work queue */
> +       wait_queue_head_t error_wq;     /* Wait queue for port errors */
> +       wait_queue_head_t lbus_wq;      /* Wait queue for lbus events */
> +       wait_queue_head_t irq_wq;       /* Wait queue for IRQ loops */
> +       wait_queue_head_t link_wq;      /* Wait queue for link changes */
> +       unsigned long state;            /* State driver is in */
> +       struct device dev;              /* Anchor point in /sys/kernel */
> +};
> +
> +#define        to_fsidd(a)             container_of(a, struct fsidd, kobj)
> +
> +#endif /* DRIVERS_FSIINIT_H */
> --
> 1.8.2.2
>

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

[christopher.lee.bostic]

From: Christopher Bostic <cbostic@us.ibm.com>

Signed-off-by: Christopher Bostic <cbostic@us.ibm.com>
---

Changes to this patch based on reviewer feedback

V2:
- Removed enums in fsiinit.h and replaced with #defines
- Added proper file copyright and license headers
- Added certificate of origin
- Removed version string
- Replace kobject with struct device in struct fsidd
- Suggestions to implement standard bus_type will be implemented in
  later patches

V3:
- Removed white space
- Suggestions to add Kconfig will be added in follow on patches

V4:
- Removed blank line at end of fsiinit.c
---
 drivers/Makefile      |  1 +
 drivers/fsi/Makefile  |  5 +++++
 drivers/fsi/fsiinit.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/fsi/fsiinit.h | 41 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 101 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..63019ff 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
diff --git a/drivers/fsi/fsiinit.c b/drivers/fsi/fsiinit.c
new file mode 100644
index 0000000..042e680
--- /dev/null
+++ b/drivers/fsi/fsiinit.c
@@ -0,0 +1,54 @@
+/*
+ * FSI Master device driver
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * Christopher Bostic <cbostic@us.ibm.com>
+ *
+ * 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@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,
+};
+
+static int fsi_start(void)
+{
+	int rc = 0;
+
+	printk("FSI DD v:%d installation ok\n", FSIDD_VERNO);
+	return rc;
+}
+
+static void fsi_exit(void)
+{
+
+}
+
+static int __init fsi_init(void)
+{
+	int rc = 0;
+
+	/* Set up the host controller */
+	fsi_start();
+	return rc;
+}
+
+module_init(fsi_init);
+module_exit(fsi_exit);
diff --git a/drivers/fsi/fsiinit.h b/drivers/fsi/fsiinit.h
new file mode 100644
index 0000000..70c3e13
--- /dev/null
+++ b/drivers/fsi/fsiinit.h
@@ -0,0 +1,41 @@
+/*
+ * FSI Master device driver structure definitions and defines
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * Christopher Bostic <cbostic@us.ibm.com>
+ *
+ * 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.
+ */
+
+#ifndef DRIVERS_FSIINIT_H
+#define DRIVERS_FSIINIT_H
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/device.h>
+#include <linux/workqueue.h>
+#include <linux/hrtimer.h>
+
+#define FSI_DD_MAGIC	0x64644632	/* ddF2 */
+#define FSI_DD_STRNO	0x1		/* Structure version number */
+
+struct fsidd {				/* FSI Main structure */
+	unsigned long magic;		/* Magic number */
+	unsigned long strno;		/* Structure version number */
+	dev_t major;			/* Major number of device */
+	struct workqueue_struct *hotp_wq;	/* Hot plug work queue */
+	wait_queue_head_t error_wq;	/* Wait queue for port errors */
+	wait_queue_head_t lbus_wq;	/* Wait queue for lbus events */
+	wait_queue_head_t irq_wq;	/* Wait queue for IRQ loops */
+	wait_queue_head_t link_wq;	/* Wait queue for link changes */
+	unsigned long state;		/* State driver is in */
+	struct device dev;		/* Anchor point in /sys/kernel */
+};
+
+#define	to_fsidd(a)		container_of(a, struct fsidd, kobj)
+
+#endif /* DRIVERS_FSIINIT_H */
-- 
1.8.2.2