[PATCH] i2c: Floppy controller bus driver (V0.2)

[PATCH] i2c: Floppy controller bus driver (V0.2)

[Herbert Poetzl]

here is the updated version of the floppy controller to I2C
adapter driver, which uses the Motor Control and Disk Changed
lines to interface to an I2C bus.

I hope I fixed the style issues (except for the access to 
global var from "remove" function - to keep it similar to
the i2c-parport-light for now)

of course the patch is compile and run tested on PC style
hardware with my test circuit connecting to an LM75 and
LM85 (temp/fan/pwm)

please consider for (mainline) inclusion!

TIA,
Herbert


Signed-off-by: Herbert Poetzl <herbert-dBHVzrDq9nF4Lj/PQRBjDg@public.gmane.org>

diff -NurpP --minimal linux-2.6.27-rc3/Documentation/i2c/busses/i2c-floppy linux-2.6.27-rc3-fi2c-v0.2/Documentation/i2c/busses/i2c-floppy
--- linux-2.6.27-rc3/Documentation/i2c/busses/i2c-floppy	1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.27-rc3-fi2c-v0.2/Documentation/i2c/busses/i2c-floppy	2008-08-18 04:34:53.000000000 +0200
@@ -0,0 +1,38 @@
+Kernel driver i2c-floppy
+
+Author: Herbert Poetzl <herbert-dBHVzrDq9nF4Lj/PQRBjDg@public.gmane.org>
+
+This driver is for a simple do-it-yourself floppy controller to
+I2C adapters which uses direct I/O access to control both Motor
+Enable lines (A/B) and read back data via the Disk Changed line.
+
+The following circuit is not suited to be used together with
+a Floppy drive (which would need some logic to work properly)
+but as replacement.
+
+ +---+
+ | F |  Motor Enable A [/MOTEA,10] -------------- SCL
+ | L |
+ | O |  Motor Enable B [/MOTEB,16] ------+------- SDA
+ | P |                                   |
+ | P |                                   |
+ | Y |  Disk Change   [/DSKCHG,34] ------+    +-- GND
+ +-+-+                                        |
+   |                                          |
+  GND	                                     GND
+
+the Motor Enable output of Floppy Disk Controllers is an open
+drain output (48mA), usually pulled up to +5V via 1k or higher,
+and the Disk Change is a Schmitt Trigger (0.8V/2.2V) input with
+max 150uA input current (data taken from the WD/FDC 37C6xx
+Floppy Disk Subsystem Controller datasheets, which basically
+combine all the necessary parts of the PC Floppy Circuit)
+
+so it should be fine to connect it to all 5V i2c devices, and
+most 3.3V devices (which are usually capable of handling 5V
+i2c bus voltages)
+
+power for those devices can be drawn from either the Floppy
+power connector (+5V/+12V) or from the SATA 3.3V lines
+(or if you prefer from some external source)
+
diff -NurpP --minimal linux-2.6.27-rc3/drivers/i2c/busses/i2c-floppy.c linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/i2c-floppy.c
--- linux-2.6.27-rc3/drivers/i2c/busses/i2c-floppy.c	1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/i2c-floppy.c	2008-08-18 03:35:34.000000000 +0200
@@ -0,0 +1,261 @@
+/* ------------------------------------------------------------------------ *
+ * i2c-floppy.c I2C bus over floppy controller                              *
+ * ------------------------------------------------------------------------ *
+   Copyright (C) 2008 Herbert Poetzl <herbert-dBHVzrDq9nF4Lj/PQRBjDg@public.gmane.org>
+
+   Somewhat based on i2c-parport-light.c driver
+   Copyright (C) 2003-2007 Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
+
+   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.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ * ------------------------------------------------------------------------ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/ioport.h>
+#include <linux/i2c.h>
+#include <linux/i2c-algo-bit.h>
+#include <linux/io.h>
+
+static struct platform_device *pdev;
+static unsigned char dor;
+
+static u16 base;
+module_param(base, ushort, 0);
+MODULE_PARM_DESC(base, "Base I/O address");
+
+
+#define DEFAULT_BASE	0x3F0		/* for PC style hardware */
+#define DRVNAME		"i2c-floppy"
+
+
+#define FOFF_DOR	0x02
+#define FOFF_DIR	0x07
+
+#define FDOR_MOTEA	0x10
+#define FDOR_MOTEB	0x20
+
+#define FDIR_DCHNG	0x80
+
+#define SCL		FDOR_MOTEA
+#define SDA		FDOR_MOTEB
+#define SDA_IN		FDIR_DCHNG
+
+#define LO_INV		(SDA|SCL)
+#define LI_INV		(SDA_IN)
+
+
+/* ----- Device list ------------------------------------------------------ */
+
+#if 0
+struct i2c_floppy_data {
+	unsigned char dor;
+};
+
+struct i2c_floppy {
+	struct i2c_adapter adapter;
+	struct i2c_algo_bit_data algo_data;
+	struct i2c_floppy_data data;
+	struct i2c_floppy *next;
+};
+#endif
+
+
+/* ----- Low-level floppy access ------------------------------------------ */
+
+static inline void port_dor_out(unsigned char d)
+{
+	outb(d ^ LO_INV, base + FOFF_DOR);
+}
+
+static inline unsigned char port_dir_in(void)
+{
+	return inb(base + FOFF_DIR) ^ LI_INV;
+}
+
+
+/* ----- I2C algorithm call-back functions and structures ----------------- */
+
+static void floppy_setscl(void *data, int state)
+{
+	if (state)
+		dor |= SCL;
+	else
+		dor &= ~SCL;
+
+	port_dor_out(dor);
+}
+
+static void floppy_setsda(void *data, int state)
+{
+	if (state)
+		dor |= SDA;
+	else
+		dor &= ~SDA;
+
+	port_dor_out(dor);
+}
+
+static int floppy_getsda(void *data)
+{
+	return port_dir_in() & SDA_IN;
+}
+
+/* Encapsulate the functions above in the correct structure
+   Note that getscl is set to NULL because SCL cannot be read
+   back with the current driver */
+static struct i2c_algo_bit_data floppy_algo_data = {
+	.setsda		= floppy_setsda,
+	.setscl		= floppy_setscl,
+	.getsda		= floppy_getsda,
+	.udelay		= 50,
+	.timeout	= HZ,
+};
+
+
+/* ----- Driver registration ---------------------------------------------- */
+
+static struct i2c_adapter floppy_adapter = {
+	.owner		= THIS_MODULE,
+	.class		= I2C_CLASS_HWMON,
+	.algo_data	= &floppy_algo_data,
+	.name		= "Floppy controller adapter",
+};
+
+static int __devinit i2c_floppy_probe(struct platform_device *pdev)
+{
+	int err;
+	struct resource *res;
+
+	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+	if (!request_region(res->start, res->end - res->start + 1, DRVNAME))
+		return -EBUSY;
+
+	/* Reset hardware to a sane state (SCL and SDA high) */
+	floppy_setsda(NULL, 1);
+	floppy_setscl(NULL, 1);
+
+	floppy_adapter.dev.parent = &pdev->dev;
+	err = i2c_bit_add_bus(&floppy_adapter);
+	if (err) {
+		dev_err(&pdev->dev, "Unable to register with I2C\n");
+		goto exit_region;
+	}
+	return 0;
+
+exit_region:
+	release_region(res->start, res->end - res->start + 1);
+	return err;
+}
+
+static int __devexit i2c_floppy_remove(struct platform_device *pdev)
+{
+	struct resource *res;
+
+	i2c_del_adapter(&floppy_adapter);
+
+	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+	release_region(res->start, res->end - res->start + 1);
+	return 0;
+}
+
+static struct platform_driver i2c_floppy_driver = {
+	.driver = {
+		.owner	= THIS_MODULE,
+		.name	= DRVNAME,
+	},
+	.probe		= i2c_floppy_probe,
+	.remove		= __devexit_p(i2c_floppy_remove),
+};
+
+static int __init i2c_floppy_device_add(u16 address)
+{
+	struct resource res = {
+		.start	= address,
+		.end	= address + 7,
+		.name	= DRVNAME,
+		.flags	= IORESOURCE_IO,
+	};
+	int err;
+
+	pdev = platform_device_alloc(DRVNAME, -1);
+	if (!pdev) {
+		err = -ENOMEM;
+		printk(KERN_ERR DRVNAME ": Device allocation failed\n");
+		goto exit;
+	}
+
+	err = platform_device_add_resources(pdev, &res, 1);
+	if (err) {
+		printk(KERN_ERR DRVNAME ": Device resource addition failed "
+		       "(%d)\n", err);
+		goto exit_device_put;
+	}
+
+	err = platform_device_add(pdev);
+	if (err) {
+		printk(KERN_ERR DRVNAME ": Device addition failed (%d)\n",
+		       err);
+		goto exit_device_put;
+	}
+
+	return 0;
+
+exit_device_put:
+	platform_device_put(pdev);
+exit:
+	return err;
+}
+
+static int __init i2c_floppy_init(void)
+{
+	int err;
+
+	if (base == 0) {
+		pr_info(DRVNAME ": using default base 0x%x\n", DEFAULT_BASE);
+		base = DEFAULT_BASE;
+	}
+
+	/* Sets global pdev as a side effect */
+	err = i2c_floppy_device_add(base);
+	if (err)
+		goto exit;
+
+	err = platform_driver_register(&i2c_floppy_driver);
+	if (err)
+		goto exit_device;
+
+	return 0;
+
+exit_device:
+	platform_device_unregister(pdev);
+exit:
+	return err;
+}
+
+static void __exit i2c_floppy_exit(void)
+{
+	platform_driver_unregister(&i2c_floppy_driver);
+	platform_device_unregister(pdev);
+}
+
+
+MODULE_AUTHOR("Herbert Poetzl <herbert-dBHVzrDq9nF4Lj/PQRBjDg@public.gmane.org>");
+MODULE_DESCRIPTION("I2C bus over floppy controller");
+MODULE_LICENSE("GPL");
+
+module_init(i2c_floppy_init);
+module_exit(i2c_floppy_exit);
diff -NurpP --minimal linux-2.6.27-rc3/drivers/i2c/busses/Kconfig linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/Kconfig
--- linux-2.6.27-rc3/drivers/i2c/busses/Kconfig	2008-08-15 21:19:24.000000000 +0200
+++ linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/Kconfig	2008-08-18 03:46:31.000000000 +0200
@@ -490,6 +490,22 @@ config I2C_VERSATILE
 
 comment "External I2C/SMBus adapter drivers"
 
+config I2C_FLOPPY
+	tristate "Floppy controller adapter"
+	select I2C_ALGOBIT
+	default n
+	help
+	  This supports a simple do-it-yourself floppy controller to
+	  I2C adapters using the motor control lines for SDA and SCL,
+	  and the drive change input for SDA readback.
+
+	  This support is also available as a module.  If so, the module
+	  will be called i2c-floppy.
+
+	  If you do not have such a device, and do not plan to build one,
+	  it's safe to say N here. Do not say Y here and to the floppy
+	  driver unless you know exactly what you are doing.
+
 config I2C_PARPORT
 	tristate "Parallel port adapter"
 	depends on PARPORT
diff -NurpP --minimal linux-2.6.27-rc3/drivers/i2c/busses/Makefile linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/Makefile
--- linux-2.6.27-rc3/drivers/i2c/busses/Makefile	2008-08-15 21:19:24.000000000 +0200
+++ linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/Makefile	2008-08-18 03:46:42.000000000 +0200
@@ -48,6 +48,7 @@ obj-$(CONFIG_I2C_SIMTEC)	+= i2c-simtec.o
 obj-$(CONFIG_I2C_VERSATILE)	+= i2c-versatile.o
 
 # External I2C/SMBus adapter drivers
+obj-$(CONFIG_I2C_FLOPPY)	+= i2c-floppy.o
 obj-$(CONFIG_I2C_PARPORT)	+= i2c-parport.o
 obj-$(CONFIG_I2C_PARPORT_LIGHT)	+= i2c-parport-light.o
 obj-$(CONFIG_I2C_TAOS_EVM)	+= i2c-taos-evm.o

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] i2c: Floppy controller bus driver (V0.2)

[Jean Delvare]

On Mon, 18 Aug 2008 05:10:46 +0200, Herbert Poetzl wrote:
> 
> here is the updated version of the floppy controller to I2C
> adapter driver, which uses the Motor Control and Disk Changed
> lines to interface to an I2C bus.
> 
> I hope I fixed the style issues (except for the access to 
> global var from "remove" function - to keep it similar to
> the i2c-parport-light for now)
> 
> of course the patch is compile and run tested on PC style
> hardware with my test circuit connecting to an LM75 and
> LM85 (temp/fan/pwm)
> 
> please consider for (mainline) inclusion!
> 
> TIA,
> Herbert
> 
> 
> Signed-off-by: Herbert Poetzl <herbert-dBHVzrDq9nF4Lj/PQRBjDg@public.gmane.org>
> 
> diff -NurpP --minimal linux-2.6.27-rc3/Documentation/i2c/busses/i2c-floppy linux-2.6.27-rc3-fi2c-v0.2/Documentation/i2c/busses/i2c-floppy
> --- linux-2.6.27-rc3/Documentation/i2c/busses/i2c-floppy	1970-01-01 01:00:00.000000000 +0100
> +++ linux-2.6.27-rc3-fi2c-v0.2/Documentation/i2c/busses/i2c-floppy	2008-08-18 04:34:53.000000000 +0200
> @@ -0,0 +1,38 @@
> +Kernel driver i2c-floppy
> +
> +Author: Herbert Poetzl <herbert-dBHVzrDq9nF4Lj/PQRBjDg@public.gmane.org>
> +
> +This driver is for a simple do-it-yourself floppy controller to
> +I2C adapters which uses direct I/O access to control both Motor

Typo: adapter.

> +Enable lines (A/B) and read back data via the Disk Changed line.
> +
> +The following circuit is not suited to be used together with
> +a Floppy drive (which would need some logic to work properly)
> +but as replacement.
> +
> + +---+
> + | F |  Motor Enable A [/MOTEA,10] -------------- SCL
> + | L |
> + | O |  Motor Enable B [/MOTEB,16] ------+------- SDA
> + | P |                                   |
> + | P |                                   |
> + | Y |  Disk Change   [/DSKCHG,34] ------+    +-- GND
> + +-+-+                                        |
> +   |                                          |
> +  GND	                                     GND
> +
> +the Motor Enable output of Floppy Disk Controllers is an open

"floppy disk controllers" (capitals not needed.)

> +drain output (48mA), usually pulled up to +5V via 1k or higher,
> +and the Disk Change is a Schmitt Trigger (0.8V/2.2V) input with
> +max 150uA input current (data taken from the WD/FDC 37C6xx
> +Floppy Disk Subsystem Controller datasheets, which basically
> +combine all the necessary parts of the PC Floppy Circuit)

"PC floppy circuit" (again capitals not needed.)

> +
> +so it should be fine to connect it to all 5V i2c devices, and

"I2C". Technically, you connect the devices to the bus, rather than the
other way around.

> +most 3.3V devices (which are usually capable of handling 5V
> +i2c bus voltages)

"I2C".

> +
> +power for those devices can be drawn from either the Floppy
> +power connector (+5V/+12V) or from the SATA 3.3V lines
> +(or if you prefer from some external source)

These last 3 paragraphs lack leading capitals and ending dots.

> +

Unneeded trailing blank line.

> diff -NurpP --minimal linux-2.6.27-rc3/drivers/i2c/busses/i2c-floppy.c linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/i2c-floppy.c
> --- linux-2.6.27-rc3/drivers/i2c/busses/i2c-floppy.c	1970-01-01 01:00:00.000000000 +0100
> +++ linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/i2c-floppy.c	2008-08-18 03:35:34.000000000 +0200
> @@ -0,0 +1,261 @@
> +/* ------------------------------------------------------------------------ *
> + * i2c-floppy.c I2C bus over floppy controller                              *
> + * ------------------------------------------------------------------------ *
> +   Copyright (C) 2008 Herbert Poetzl <herbert-dBHVzrDq9nF4Lj/PQRBjDg@public.gmane.org>
> +
> +   Somewhat based on i2c-parport-light.c driver
> +   Copyright (C) 2003-2007 Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> +
> +   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.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program; if not, write to the Free Software
> +   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + * ------------------------------------------------------------------------ */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/ioport.h>
> +#include <linux/i2c.h>
> +#include <linux/i2c-algo-bit.h>
> +#include <linux/io.h>
> +
> +static struct platform_device *pdev;
> +static unsigned char dor;
> +
> +static u16 base;
> +module_param(base, ushort, 0);
> +MODULE_PARM_DESC(base, "Base I/O address");
> +
> +
> +#define DEFAULT_BASE	0x3F0		/* for PC style hardware */
> +#define DRVNAME		"i2c-floppy"
> +
> +
> +#define FOFF_DOR	0x02
> +#define FOFF_DIR	0x07
> +
> +#define FDOR_MOTEA	0x10
> +#define FDOR_MOTEB	0x20
> +
> +#define FDIR_DCHNG	0x80
> +
> +#define SCL		FDOR_MOTEA
> +#define SDA		FDOR_MOTEB
> +#define SDA_IN		FDIR_DCHNG
> +
> +#define LO_INV		(SDA|SCL)
> +#define LI_INV		(SDA_IN)
> +
> +
> +/* ----- Device list ------------------------------------------------------ */
> +
> +#if 0
> +struct i2c_floppy_data {
> +	unsigned char dor;
> +};
> +
> +struct i2c_floppy {
> +	struct i2c_adapter adapter;
> +	struct i2c_algo_bit_data algo_data;
> +	struct i2c_floppy_data data;
> +	struct i2c_floppy *next;
> +};
> +#endif
> +
> +

Please just discard the whole section. It can always be added later if
you ever rework the driver (but I doubt it will ever happen...)

> +/* ----- Low-level floppy access ------------------------------------------ */
> +
> +static inline void port_dor_out(unsigned char d)
> +{
> +	outb(d ^ LO_INV, base + FOFF_DOR);
> +}
> +
> +static inline unsigned char port_dir_in(void)
> +{
> +	return inb(base + FOFF_DIR) ^ LI_INV;
> +}
> +
> +
> +/* ----- I2C algorithm call-back functions and structures ----------------- */
> +
> +static void floppy_setscl(void *data, int state)
> +{
> +	if (state)
> +		dor |= SCL;
> +	else
> +		dor &= ~SCL;
> +
> +	port_dor_out(dor);
> +}
> +
> +static void floppy_setsda(void *data, int state)
> +{
> +	if (state)
> +		dor |= SDA;
> +	else
> +		dor &= ~SDA;
> +
> +	port_dor_out(dor);
> +}
> +
> +static int floppy_getsda(void *data)
> +{
> +	return port_dir_in() & SDA_IN;
> +}
> +
> +/* Encapsulate the functions above in the correct structure
> +   Note that getscl is set to NULL because SCL cannot be read
> +   back with the current driver */
> +static struct i2c_algo_bit_data floppy_algo_data = {
> +	.setsda		= floppy_setsda,
> +	.setscl		= floppy_setscl,
> +	.getsda		= floppy_getsda,
> +	.udelay		= 50,
> +	.timeout	= HZ,
> +};
> +
> +
> +/* ----- Driver registration ---------------------------------------------- */
> +
> +static struct i2c_adapter floppy_adapter = {
> +	.owner		= THIS_MODULE,
> +	.class		= I2C_CLASS_HWMON,
> +	.algo_data	= &floppy_algo_data,
> +	.name		= "Floppy controller adapter",
> +};
> +
> +static int __devinit i2c_floppy_probe(struct platform_device *pdev)
> +{
> +	int err;
> +	struct resource *res;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> +	if (!request_region(res->start, res->end - res->start + 1, DRVNAME))
> +		return -EBUSY;
> +
> +	/* Reset hardware to a sane state (SCL and SDA high) */
> +	floppy_setsda(NULL, 1);
> +	floppy_setscl(NULL, 1);
> +
> +	floppy_adapter.dev.parent = &pdev->dev;
> +	err = i2c_bit_add_bus(&floppy_adapter);
> +	if (err) {
> +		dev_err(&pdev->dev, "Unable to register with I2C\n");
> +		goto exit_region;
> +	}
> +	return 0;
> +
> +exit_region:
> +	release_region(res->start, res->end - res->start + 1);
> +	return err;
> +}
> +
> +static int __devexit i2c_floppy_remove(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +
> +	i2c_del_adapter(&floppy_adapter);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> +	release_region(res->start, res->end - res->start + 1);
> +	return 0;
> +}
> +
> +static struct platform_driver i2c_floppy_driver = {
> +	.driver = {
> +		.owner	= THIS_MODULE,
> +		.name	= DRVNAME,
> +	},
> +	.probe		= i2c_floppy_probe,
> +	.remove		= __devexit_p(i2c_floppy_remove),
> +};
> +
> +static int __init i2c_floppy_device_add(u16 address)
> +{
> +	struct resource res = {
> +		.start	= address,
> +		.end	= address + 7,
> +		.name	= DRVNAME,
> +		.flags	= IORESOURCE_IO,
> +	};
> +	int err;
> +
> +	pdev = platform_device_alloc(DRVNAME, -1);
> +	if (!pdev) {
> +		err = -ENOMEM;
> +		printk(KERN_ERR DRVNAME ": Device allocation failed\n");
> +		goto exit;
> +	}
> +
> +	err = platform_device_add_resources(pdev, &res, 1);
> +	if (err) {
> +		printk(KERN_ERR DRVNAME ": Device resource addition failed "
> +		       "(%d)\n", err);
> +		goto exit_device_put;
> +	}
> +
> +	err = platform_device_add(pdev);
> +	if (err) {
> +		printk(KERN_ERR DRVNAME ": Device addition failed (%d)\n",
> +		       err);
> +		goto exit_device_put;
> +	}
> +
> +	return 0;
> +
> +exit_device_put:
> +	platform_device_put(pdev);
> +exit:
> +	return err;
> +}
> +
> +static int __init i2c_floppy_init(void)
> +{
> +	int err;
> +
> +	if (base == 0) {
> +		pr_info(DRVNAME ": using default base 0x%x\n", DEFAULT_BASE);
> +		base = DEFAULT_BASE;
> +	}
> +
> +	/* Sets global pdev as a side effect */
> +	err = i2c_floppy_device_add(base);
> +	if (err)
> +		goto exit;
> +
> +	err = platform_driver_register(&i2c_floppy_driver);
> +	if (err)
> +		goto exit_device;
> +
> +	return 0;
> +
> +exit_device:
> +	platform_device_unregister(pdev);
> +exit:
> +	return err;
> +}
> +
> +static void __exit i2c_floppy_exit(void)
> +{
> +	platform_driver_unregister(&i2c_floppy_driver);
> +	platform_device_unregister(pdev);
> +}
> +
> +
> +MODULE_AUTHOR("Herbert Poetzl <herbert-dBHVzrDq9nF4Lj/PQRBjDg@public.gmane.org>");
> +MODULE_DESCRIPTION("I2C bus over floppy controller");
> +MODULE_LICENSE("GPL");
> +
> +module_init(i2c_floppy_init);
> +module_exit(i2c_floppy_exit);
> diff -NurpP --minimal linux-2.6.27-rc3/drivers/i2c/busses/Kconfig linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/Kconfig
> --- linux-2.6.27-rc3/drivers/i2c/busses/Kconfig	2008-08-15 21:19:24.000000000 +0200
> +++ linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/Kconfig	2008-08-18 03:46:31.000000000 +0200
> @@ -490,6 +490,22 @@ config I2C_VERSATILE
>  
>  comment "External I2C/SMBus adapter drivers"
>  
> +config I2C_FLOPPY
> +	tristate "Floppy controller adapter"
> +	select I2C_ALGOBIT
> +	default n
> +	help
> +	  This supports a simple do-it-yourself floppy controller to
> +	  I2C adapters using the motor control lines for SDA and SCL,
> +	  and the drive change input for SDA readback.
> +
> +	  This support is also available as a module.  If so, the module
> +	  will be called i2c-floppy.
> +
> +	  If you do not have such a device, and do not plan to build one,
> +	  it's safe to say N here. Do not say Y here and to the floppy
> +	  driver unless you know exactly what you are doing.

Good point. In fact it would be safer to depend on !BLK_DEV_FD. There's
no "if you know exactly what you are doing" that holds: if any of these
drivers is built in, the other one _must_ be excluded.

> +
>  config I2C_PARPORT
>  	tristate "Parallel port adapter"
>  	depends on PARPORT
> diff -NurpP --minimal linux-2.6.27-rc3/drivers/i2c/busses/Makefile linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/Makefile
> --- linux-2.6.27-rc3/drivers/i2c/busses/Makefile	2008-08-15 21:19:24.000000000 +0200
> +++ linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/Makefile	2008-08-18 03:46:42.000000000 +0200
> @@ -48,6 +48,7 @@ obj-$(CONFIG_I2C_SIMTEC)	+= i2c-simtec.o
>  obj-$(CONFIG_I2C_VERSATILE)	+= i2c-versatile.o
>  
>  # External I2C/SMBus adapter drivers
> +obj-$(CONFIG_I2C_FLOPPY)	+= i2c-floppy.o
>  obj-$(CONFIG_I2C_PARPORT)	+= i2c-parport.o
>  obj-$(CONFIG_I2C_PARPORT_LIGHT)	+= i2c-parport-light.o
>  obj-$(CONFIG_I2C_TAOS_EVM)	+= i2c-taos-evm.o

All the rest looks OK to me. Your driver is almost ready to merge.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] i2c: Floppy controller bus driver (V0.2)

[Herbert Poetzl]

On Mon, Aug 18, 2008 at 10:45:39AM +0200, Jean Delvare wrote:
> On Mon, 18 Aug 2008 05:10:46 +0200, Herbert Poetzl wrote:
> > 
> > here is the updated version of the floppy controller to I2C
> > adapter driver, which uses the Motor Control and Disk Changed
> > lines to interface to an I2C bus.
> > 
> > I hope I fixed the style issues (except for the access to 
> > global var from "remove" function - to keep it similar to
> > the i2c-parport-light for now)
> > 
> > of course the patch is compile and run tested on PC style
> > hardware with my test circuit connecting to an LM75 and
> > LM85 (temp/fan/pwm)
> > 
> > please consider for (mainline) inclusion!
> > 
> > TIA,
> > Herbert
> > 
> > 
> > Signed-off-by: Herbert Poetzl <herbert-dBHVzrDq9nF4Lj/PQRBjDg@public.gmane.org>
> > 
> > diff -NurpP --minimal linux-2.6.27-rc3/Documentation/i2c/busses/i2c-floppy linux-2.6.27-rc3-fi2c-v0.2/Documentation/i2c/busses/i2c-floppy
> > --- linux-2.6.27-rc3/Documentation/i2c/busses/i2c-floppy	1970-01-01 01:00:00.000000000 +0100
> > +++ linux-2.6.27-rc3-fi2c-v0.2/Documentation/i2c/busses/i2c-floppy	2008-08-18 04:34:53.000000000 +0200
> > @@ -0,0 +1,38 @@
> > +Kernel driver i2c-floppy
> > +
> > +Author: Herbert Poetzl <herbert-dBHVzrDq9nF4Lj/PQRBjDg@public.gmane.org>
> > +
> > +This driver is for a simple do-it-yourself floppy controller to
> > +I2C adapters which uses direct I/O access to control both Motor
> 
> Typo: adapter.
> 
> > +Enable lines (A/B) and read back data via the Disk Changed line.
> > +
> > +The following circuit is not suited to be used together with
> > +a Floppy drive (which would need some logic to work properly)
> > +but as replacement.
> > +
> > + +---+
> > + | F |  Motor Enable A [/MOTEA,10] -------------- SCL
> > + | L |
> > + | O |  Motor Enable B [/MOTEB,16] ------+------- SDA
> > + | P |                                   |
> > + | P |                                   |
> > + | Y |  Disk Change   [/DSKCHG,34] ------+    +-- GND
> > + +-+-+                                        |
> > +   |                                          |
> > +  GND	                                     GND
> > +
> > +the Motor Enable output of Floppy Disk Controllers is an open
> 
> "floppy disk controllers" (capitals not needed.)
> 
> > +drain output (48mA), usually pulled up to +5V via 1k or higher,
> > +and the Disk Change is a Schmitt Trigger (0.8V/2.2V) input with
> > +max 150uA input current (data taken from the WD/FDC 37C6xx
> > +Floppy Disk Subsystem Controller datasheets, which basically
> > +combine all the necessary parts of the PC Floppy Circuit)
> 
> "PC floppy circuit" (again capitals not needed.)
> 
> > +
> > +so it should be fine to connect it to all 5V i2c devices, and
> 
> "I2C". Technically, you connect the devices to the bus, rather than the
> other way around.
> 
> > +most 3.3V devices (which are usually capable of handling 5V
> > +i2c bus voltages)
> 
> "I2C".
> 
> > +
> > +power for those devices can be drawn from either the Floppy
> > +power connector (+5V/+12V) or from the SATA 3.3V lines
> > +(or if you prefer from some external source)
> 
> These last 3 paragraphs lack leading capitals and ending dots.
> 
> > +
> 
> Unneeded trailing blank line.
> 
> > diff -NurpP --minimal linux-2.6.27-rc3/drivers/i2c/busses/i2c-floppy.c linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/i2c-floppy.c
> > --- linux-2.6.27-rc3/drivers/i2c/busses/i2c-floppy.c	1970-01-01 01:00:00.000000000 +0100
> > +++ linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/i2c-floppy.c	2008-08-18 03:35:34.000000000 +0200
> > @@ -0,0 +1,261 @@
> > +/* ------------------------------------------------------------------------ *
> > + * i2c-floppy.c I2C bus over floppy controller                              *
> > + * ------------------------------------------------------------------------ *
> > +   Copyright (C) 2008 Herbert Poetzl <herbert-dBHVzrDq9nF4Lj/PQRBjDg@public.gmane.org>
> > +
> > +   Somewhat based on i2c-parport-light.c driver
> > +   Copyright (C) 2003-2007 Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> > +
> > +   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.
> > +
> > +   This program is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +   GNU General Public License for more details.
> > +
> > +   You should have received a copy of the GNU General Public License
> > +   along with this program; if not, write to the Free Software
> > +   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> > + * ------------------------------------------------------------------------ */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/ioport.h>
> > +#include <linux/i2c.h>
> > +#include <linux/i2c-algo-bit.h>
> > +#include <linux/io.h>
> > +
> > +static struct platform_device *pdev;
> > +static unsigned char dor;
> > +
> > +static u16 base;
> > +module_param(base, ushort, 0);
> > +MODULE_PARM_DESC(base, "Base I/O address");
> > +
> > +
> > +#define DEFAULT_BASE	0x3F0		/* for PC style hardware */
> > +#define DRVNAME		"i2c-floppy"
> > +
> > +
> > +#define FOFF_DOR	0x02
> > +#define FOFF_DIR	0x07
> > +
> > +#define FDOR_MOTEA	0x10
> > +#define FDOR_MOTEB	0x20
> > +
> > +#define FDIR_DCHNG	0x80
> > +
> > +#define SCL		FDOR_MOTEA
> > +#define SDA		FDOR_MOTEB
> > +#define SDA_IN		FDIR_DCHNG
> > +
> > +#define LO_INV		(SDA|SCL)
> > +#define LI_INV		(SDA_IN)
> > +
> > +
> > +/* ----- Device list ------------------------------------------------------ */
> > +
> > +#if 0
> > +struct i2c_floppy_data {
> > +	unsigned char dor;
> > +};
> > +
> > +struct i2c_floppy {
> > +	struct i2c_adapter adapter;
> > +	struct i2c_algo_bit_data algo_data;
> > +	struct i2c_floppy_data data;
> > +	struct i2c_floppy *next;
> > +};
> > +#endif
> > +
> > +
> 
> Please just discard the whole section. It can always be added later if
> you ever rework the driver (but I doubt it will ever happen...)

k, consider it done 

> > +/* ----- Low-level floppy access ------------------------------------------ */
> > +
> > +static inline void port_dor_out(unsigned char d)
> > +{
> > +	outb(d ^ LO_INV, base + FOFF_DOR);
> > +}
> > +
> > +static inline unsigned char port_dir_in(void)
> > +{
> > +	return inb(base + FOFF_DIR) ^ LI_INV;
> > +}
> > +
> > +
> > +/* ----- I2C algorithm call-back functions and structures ----------------- */
> > +
> > +static void floppy_setscl(void *data, int state)
> > +{
> > +	if (state)
> > +		dor |= SCL;
> > +	else
> > +		dor &= ~SCL;
> > +
> > +	port_dor_out(dor);
> > +}
> > +
> > +static void floppy_setsda(void *data, int state)
> > +{
> > +	if (state)
> > +		dor |= SDA;
> > +	else
> > +		dor &= ~SDA;
> > +
> > +	port_dor_out(dor);
> > +}
> > +
> > +static int floppy_getsda(void *data)
> > +{
> > +	return port_dir_in() & SDA_IN;
> > +}
> > +
> > +/* Encapsulate the functions above in the correct structure
> > +   Note that getscl is set to NULL because SCL cannot be read
> > +   back with the current driver */
> > +static struct i2c_algo_bit_data floppy_algo_data = {
> > +	.setsda		= floppy_setsda,
> > +	.setscl		= floppy_setscl,
> > +	.getsda		= floppy_getsda,
> > +	.udelay		= 50,
> > +	.timeout	= HZ,
> > +};
> > +
> > +
> > +/* ----- Driver registration ---------------------------------------------- */
> > +
> > +static struct i2c_adapter floppy_adapter = {
> > +	.owner		= THIS_MODULE,
> > +	.class		= I2C_CLASS_HWMON,
> > +	.algo_data	= &floppy_algo_data,
> > +	.name		= "Floppy controller adapter",
> > +};
> > +
> > +static int __devinit i2c_floppy_probe(struct platform_device *pdev)
> > +{
> > +	int err;
> > +	struct resource *res;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> > +	if (!request_region(res->start, res->end - res->start + 1, DRVNAME))
> > +		return -EBUSY;
> > +
> > +	/* Reset hardware to a sane state (SCL and SDA high) */
> > +	floppy_setsda(NULL, 1);
> > +	floppy_setscl(NULL, 1);
> > +
> > +	floppy_adapter.dev.parent = &pdev->dev;
> > +	err = i2c_bit_add_bus(&floppy_adapter);
> > +	if (err) {
> > +		dev_err(&pdev->dev, "Unable to register with I2C\n");
> > +		goto exit_region;
> > +	}
> > +	return 0;
> > +
> > +exit_region:
> > +	release_region(res->start, res->end - res->start + 1);
> > +	return err;
> > +}
> > +
> > +static int __devexit i2c_floppy_remove(struct platform_device *pdev)
> > +{
> > +	struct resource *res;
> > +
> > +	i2c_del_adapter(&floppy_adapter);
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> > +	release_region(res->start, res->end - res->start + 1);
> > +	return 0;
> > +}
> > +
> > +static struct platform_driver i2c_floppy_driver = {
> > +	.driver = {
> > +		.owner	= THIS_MODULE,
> > +		.name	= DRVNAME,
> > +	},
> > +	.probe		= i2c_floppy_probe,
> > +	.remove		= __devexit_p(i2c_floppy_remove),
> > +};
> > +
> > +static int __init i2c_floppy_device_add(u16 address)
> > +{
> > +	struct resource res = {
> > +		.start	= address,
> > +		.end	= address + 7,
> > +		.name	= DRVNAME,
> > +		.flags	= IORESOURCE_IO,
> > +	};
> > +	int err;
> > +
> > +	pdev = platform_device_alloc(DRVNAME, -1);
> > +	if (!pdev) {
> > +		err = -ENOMEM;
> > +		printk(KERN_ERR DRVNAME ": Device allocation failed\n");
> > +		goto exit;
> > +	}
> > +
> > +	err = platform_device_add_resources(pdev, &res, 1);
> > +	if (err) {
> > +		printk(KERN_ERR DRVNAME ": Device resource addition failed "
> > +		       "(%d)\n", err);
> > +		goto exit_device_put;
> > +	}
> > +
> > +	err = platform_device_add(pdev);
> > +	if (err) {
> > +		printk(KERN_ERR DRVNAME ": Device addition failed (%d)\n",
> > +		       err);
> > +		goto exit_device_put;
> > +	}
> > +
> > +	return 0;
> > +
> > +exit_device_put:
> > +	platform_device_put(pdev);
> > +exit:
> > +	return err;
> > +}
> > +
> > +static int __init i2c_floppy_init(void)
> > +{
> > +	int err;
> > +
> > +	if (base == 0) {
> > +		pr_info(DRVNAME ": using default base 0x%x\n", DEFAULT_BASE);
> > +		base = DEFAULT_BASE;
> > +	}
> > +
> > +	/* Sets global pdev as a side effect */
> > +	err = i2c_floppy_device_add(base);
> > +	if (err)
> > +		goto exit;
> > +
> > +	err = platform_driver_register(&i2c_floppy_driver);
> > +	if (err)
> > +		goto exit_device;
> > +
> > +	return 0;
> > +
> > +exit_device:
> > +	platform_device_unregister(pdev);
> > +exit:
> > +	return err;
> > +}
> > +
> > +static void __exit i2c_floppy_exit(void)
> > +{
> > +	platform_driver_unregister(&i2c_floppy_driver);
> > +	platform_device_unregister(pdev);
> > +}
> > +
> > +
> > +MODULE_AUTHOR("Herbert Poetzl <herbert-dBHVzrDq9nF4Lj/PQRBjDg@public.gmane.org>");
> > +MODULE_DESCRIPTION("I2C bus over floppy controller");
> > +MODULE_LICENSE("GPL");
> > +
> > +module_init(i2c_floppy_init);
> > +module_exit(i2c_floppy_exit);
> > diff -NurpP --minimal linux-2.6.27-rc3/drivers/i2c/busses/Kconfig linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/Kconfig
> > --- linux-2.6.27-rc3/drivers/i2c/busses/Kconfig	2008-08-15 21:19:24.000000000 +0200
> > +++ linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/Kconfig	2008-08-18 03:46:31.000000000 +0200
> > @@ -490,6 +490,22 @@ config I2C_VERSATILE
> >  
> >  comment "External I2C/SMBus adapter drivers"
> >  
> > +config I2C_FLOPPY
> > +	tristate "Floppy controller adapter"
> > +	select I2C_ALGOBIT
> > +	default n
> > +	help
> > +	  This supports a simple do-it-yourself floppy controller to
> > +	  I2C adapters using the motor control lines for SDA and SCL,
> > +	  and the drive change input for SDA readback.
> > +
> > +	  This support is also available as a module.  If so, the module
> > +	  will be called i2c-floppy.
> > +
> > +	  If you do not have such a device, and do not plan to build one,
> > +	  it's safe to say N here. Do not say Y here and to the floppy
> > +	  driver unless you know exactly what you are doing.
> 
> Good point. In fact it would be safer to depend on !BLK_DEV_FD. There's
> no "if you know exactly what you are doing" that holds: if any of these
> drivers is built in, the other one _must_ be excluded.

well, actually not true ... I build both drivers
as module, and can then decide wether I want to
attach a floppy or an I2C adapter ... and except
for the fact that floppy access to the I2C adapter
is not the best idea, this works perfectly fine

but as far as I know, there is no (sane) way to
say building both 'M' is fine, but 'Y' is a nono
(I think that is why many drivers write it this
way in the help)

making it mutually exclusive is kind of problematic
as you need to disable one _beforehand_ to see the
other in the config ... magically disabling the 
floppy driver from the i2c-floppy is not an option
either, as the build system doesn't support that

so I personally think the 'default n' + warning
is fine for everyday use, but of course, the final
call is yours

> > +
> >  config I2C_PARPORT
> >  	tristate "Parallel port adapter"
> >  	depends on PARPORT
> > diff -NurpP --minimal linux-2.6.27-rc3/drivers/i2c/busses/Makefile linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/Makefile
> > --- linux-2.6.27-rc3/drivers/i2c/busses/Makefile	2008-08-15 21:19:24.000000000 +0200
> > +++ linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/Makefile	2008-08-18 03:46:42.000000000 +0200
> > @@ -48,6 +48,7 @@ obj-$(CONFIG_I2C_SIMTEC)	+= i2c-simtec.o
> >  obj-$(CONFIG_I2C_VERSATILE)	+= i2c-versatile.o
> >  
> >  # External I2C/SMBus adapter drivers
> > +obj-$(CONFIG_I2C_FLOPPY)	+= i2c-floppy.o
> >  obj-$(CONFIG_I2C_PARPORT)	+= i2c-parport.o
> >  obj-$(CONFIG_I2C_PARPORT_LIGHT)	+= i2c-parport-light.o
> >  obj-$(CONFIG_I2C_TAOS_EVM)	+= i2c-taos-evm.o
> 
> All the rest looks OK to me. Your driver is almost ready to merge.

sounds good to me ... expect an update soon

best,
Herbert

> -- 
> Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] i2c: Floppy controller bus driver (V0.2)

[Jean Delvare]

Hi Herbert,

On Mon, 18 Aug 2008 18:40:53 +0200, Herbert Poetzl wrote:
> On Mon, Aug 18, 2008 at 10:45:39AM +0200, Jean Delvare wrote:
> > On Mon, 18 Aug 2008 05:10:46 +0200, Herbert Poetzl wrote:
> > > diff -NurpP --minimal linux-2.6.27-rc3/drivers/i2c/busses/Kconfig linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/Kconfig
> > > --- linux-2.6.27-rc3/drivers/i2c/busses/Kconfig	2008-08-15 21:19:24.000000000 +0200
> > > +++ linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/Kconfig	2008-08-18 03:46:31.000000000 +0200
> > > @@ -490,6 +490,22 @@ config I2C_VERSATILE
> > >  
> > >  comment "External I2C/SMBus adapter drivers"
> > >  
> > > +config I2C_FLOPPY
> > > +	tristate "Floppy controller adapter"
> > > +	select I2C_ALGOBIT
> > > +	default n
> > > +	help
> > > +	  This supports a simple do-it-yourself floppy controller to
> > > +	  I2C adapters using the motor control lines for SDA and SCL,
> > > +	  and the drive change input for SDA readback.
> > > +
> > > +	  This support is also available as a module.  If so, the module
> > > +	  will be called i2c-floppy.
> > > +
> > > +	  If you do not have such a device, and do not plan to build one,
> > > +	  it's safe to say N here. Do not say Y here and to the floppy
> > > +	  driver unless you know exactly what you are doing.
> > 
> > Good point. In fact it would be safer to depend on !BLK_DEV_FD. There's
> > no "if you know exactly what you are doing" that holds: if any of these
> > drivers is built in, the other one _must_ be excluded.
> 
> well, actually not true ... I build both drivers
> as module, and can then decide wether I want to
> attach a floppy or an I2C adapter ... and except
> for the fact that floppy access to the I2C adapter
> is not the best idea, this works perfectly fine

What I propose wouldn't prevent you from doing that. "depend
on !BLK_DEV_FD" means that both drivers can't be built-in. Both drivers
as modules is OK. Remember that the Kconfig language is based on
ternary logic.

> 
> but as far as I know, there is no (sane) way to
> say building both 'M' is fine, but 'Y' is a nono
> (I think that is why many drivers write it this
> way in the help)

There _is_ a way do to this and that's exactly what I proposed.

> 
> making it mutually exclusive is kind of problematic
> as you need to disable one _beforehand_ to see the
> other in the config ...

This is true, but I don't expect this to be a problem in practice. Most
users have the standard floppy driver built as module, so they will see
i2c-floppy and will be able to build it as a module. Worst case is if
the standard floppy driver is built-in, but I don't expect this to be a
popular choice these days, with all the PCs out there without a floppy
disk drive. If this is really a problem, it's easy to add a message in
Kconfig to tell people what they need to do to get access to
i2c-floppy. But I don't expect your driver to be popular enough to
warrant this.

>                          magically disabling the 
> floppy driver from the i2c-floppy is not an option
> either, as the build system doesn't support that

Agreed. Even if the build system could do that, this would be highly
unfriendly.

> 
> so I personally think the 'default n' + warning
> is fine for everyday use, but of course, the final
> call is yours

I'll add the dependency. If I don't, somebody will quickly complain
that floppy support is broken by allyesconfig.

No need to resend, I'll do that in a incremental patch. I think I'll
add a dependency on EXPERIMENTAL as well - after all your driver has
seen little testing so far.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] i2c: Floppy controller bus driver (V0.2)

[Herbert Poetzl]

On Mon, Aug 18, 2008 at 08:57:08PM +0200, Jean Delvare wrote:
> Hi Herbert,
> 
> On Mon, 18 Aug 2008 18:40:53 +0200, Herbert Poetzl wrote:
> > On Mon, Aug 18, 2008 at 10:45:39AM +0200, Jean Delvare wrote:
> > > On Mon, 18 Aug 2008 05:10:46 +0200, Herbert Poetzl wrote:
> > > > diff -NurpP --minimal linux-2.6.27-rc3/drivers/i2c/busses/Kconfig linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/Kconfig
> > > > --- linux-2.6.27-rc3/drivers/i2c/busses/Kconfig	2008-08-15 21:19:24.000000000 +0200
> > > > +++ linux-2.6.27-rc3-fi2c-v0.2/drivers/i2c/busses/Kconfig	2008-08-18 03:46:31.000000000 +0200
> > > > @@ -490,6 +490,22 @@ config I2C_VERSATILE
> > > >  
> > > >  comment "External I2C/SMBus adapter drivers"
> > > >  
> > > > +config I2C_FLOPPY
> > > > +	tristate "Floppy controller adapter"
> > > > +	select I2C_ALGOBIT
> > > > +	default n
> > > > +	help
> > > > +	  This supports a simple do-it-yourself floppy controller to
> > > > +	  I2C adapters using the motor control lines for SDA and SCL,
> > > > +	  and the drive change input for SDA readback.
> > > > +
> > > > +	  This support is also available as a module.  If so, the module
> > > > +	  will be called i2c-floppy.
> > > > +
> > > > +	  If you do not have such a device, and do not plan to build one,
> > > > +	  it's safe to say N here. Do not say Y here and to the floppy
> > > > +	  driver unless you know exactly what you are doing.
> > > 
> > > Good point. In fact it would be safer to depend on !BLK_DEV_FD. There's
> > > no "if you know exactly what you are doing" that holds: if any of these
> > > drivers is built in, the other one _must_ be excluded.
> > 
> > well, actually not true ... I build both drivers
> > as module, and can then decide wether I want to
> > attach a floppy or an I2C adapter ... and except
> > for the fact that floppy access to the I2C adapter
> > is not the best idea, this works perfectly fine
> 
> What I propose wouldn't prevent you from doing that. "depend
> on !BLK_DEV_FD" means that both drivers can't be built-in. Both drivers
> as modules is OK. Remember that the Kconfig language is based on
> ternary logic.
> 
> > 
> > but as far as I know, there is no (sane) way to
> > say building both 'M' is fine, but 'Y' is a nono
> > (I think that is why many drivers write it this
> > way in the help)
> 
> There _is_ a way do to this and that's exactly what I proposed.
> 
> > 
> > making it mutually exclusive is kind of problematic
> > as you need to disable one _beforehand_ to see the
> > other in the config ...
> 
> This is true, but I don't expect this to be a problem in practice. Most
> users have the standard floppy driver built as module, so they will see
> i2c-floppy and will be able to build it as a module. Worst case is if
> the standard floppy driver is built-in, but I don't expect this to be a
> popular choice these days, with all the PCs out there without a floppy
> disk drive. If this is really a problem, it's easy to add a message in
> Kconfig to tell people what they need to do to get access to
> i2c-floppy. But I don't expect your driver to be popular enough to
> warrant this.
> 
> >                          magically disabling the 
> > floppy driver from the i2c-floppy is not an option
> > either, as the build system doesn't support that
> 
> Agreed. Even if the build system could do that, this would be highly
> unfriendly.
> 
> > 
> > so I personally think the 'default n' + warning
> > is fine for everyday use, but of course, the final
> > call is yours
> 
> I'll add the dependency. If I don't, somebody will quickly complain
> that floppy support is broken by allyesconfig.
> 
> No need to resend, I'll do that in a incremental patch. I think I'll
> add a dependency on EXPERIMENTAL as well - after all your driver has
> seen little testing so far.

okay, no problem with that .. thanks!

best,
Herbert

> -- 
> Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c