[PATCH] Re: [PATCH/RFT] tlclk: convert to the new platform device interface

mgross
2005-12-23T22:44:16Z

[-- Attachment #1: Type: text/plain, Size: 8015 bytes --]

Attached is a patch against 2.6.15-rc6-git4 to update the telecom clock to
match an update to the TPS.

This patch also includes a few clean ups.
Signed-off-by: Mark Gross <mark.gross@intel.com>

--mgross

On Friday 23 December 2005 11:14, Gross, Mark wrote:
> The patch "looks" ok, but I have two issues:
>
> 2.6.15-rc6, hangs on the udev startup on my RHEL4 system so I can't
> smoke test this and more importantly an errata update patch I sent out
> earlier didn't make it in. (I was having email client issues with kmail
> changing tabs to spaces on me that day so it may be my fault that patch
> got dropped.)
>
> How would you like to proceed?
> 1) Should I send out an update patch that includes yours and the errata
> patch, knowing that we can only say that it compiles.
> 2) Should I send a unit tested errata patch to a 2.6.14.latest version?
there is only the 2.6.15-rc versions, duh.

> 3) Both 1 and 2?
> 4) Just ack your patch (I did review it and it compiles and looks fine)
> and re-submit my errata changes on top of your update
>
> Thanks,
>
> --mgross
>
>
> -----Original Message-----
> From: Dmitry Torokhov [mailto:dtor_core@ameritech.net]
> Sent: Thursday, December 15, 2005 10:56 PM
> To: LKML
> Cc: Russell King; Al Viro; torvalds@osdl.org; Gross, Mark
> Subject: [PATCH/RFT] tlclk: convert to the new platform device interface
>
> tlclk: convert to the new platform device interface
>
> Do not use platform_device_register_simple() as it is going away,
> define dcdbas_driver and implement ->probe() and ->remove() functions
> so manual binding and unbinding will work with this driver.
>
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
>
> drivers/char/tlclk.c | 143
> +++++++++++++++++++++++++++++++--------------------
> 1 files changed, 89 insertions(+), 54 deletions(-)
>
> Index: work/drivers/char/tlclk.c
> ===================================================================
> --- work.orig/drivers/char/tlclk.c
> +++ work/drivers/char/tlclk.c
> @@ -202,6 +202,10 @@ static int tlclk_open(struct inode *inod
> {
> int result;
>
> + alarm_events = kzalloc(sizeof(struct tlclk_alarms), GFP_KERNEL);
> + if (!alarm_events)
> + return -ENOMEM;
> +
> /* Make sure there is no interrupt pending while
> * initialising interrupt handler */
> inb(TLCLK_REG6);
> @@ -212,6 +216,8 @@ static int tlclk_open(struct inode *inod
> SA_INTERRUPT, "telco_clock",
> tlclk_interrupt);
> if (result == -EBUSY) {
> printk(KERN_ERR "telco_clock: Interrupt can't be
> reserved!\n");
> + kfree(alarm_events);
> + alarm_events = NULL;
> return -EBUSY;
> }
> inb(TLCLK_REG6); /* Clear interrupt events */
> @@ -222,6 +228,9 @@ static int tlclk_open(struct inode *inod
> static int tlclk_release(struct inode *inode, struct file *filp)
> {
> free_irq(telclk_interrupt, tlclk_interrupt);
> + del_timer_sync(&switchover_timer);
> + kfree(alarm_events);
> + alarm_events = NULL;
>
> return 0;
> }
> @@ -733,89 +742,115 @@ static struct attribute_group tlclk_attr
> .attrs = tlclk_sysfs_entries,
> };
>
> -static struct platform_device *tlclk_device;
> -
> -static int __init tlclk_init(void)
> +static int __devinit tlclk_probe(struct platform_device *dev)
> {
> - int ret;
> + int error;
>
> - ret = register_chrdev(tlclk_major, "telco_clock", &tlclk_fops);
> - if (ret < 0) {
> + init_timer(&switchover_timer);
> +
> + error = register_chrdev(tlclk_major, "telco_clock",
> &tlclk_fops);
> + if (error < 0) {
> printk(KERN_ERR "telco_clock: can't get major! %d\n",
> tlclk_major);
> - return ret;
> + return error;
> }
> - alarm_events = kzalloc( sizeof(struct tlclk_alarms),
> GFP_KERNEL);
> - if (!alarm_events)
> - goto out1;
> +
> + error = misc_register(&tlclk_miscdev);
> + if (error < 0) {
> + printk(KERN_ERR "tlclk: misc_register retruns %d\n",
> error);
> + goto err_unregister_chrdev;
> + }
> +
> + error = sysfs_create_group(&dev->dev.kobj,
> &tlclk_attribute_group);
> + if (error) {
> + printk(KERN_ERR
> + "tlclk: failed to create sysfs device
> attributes\n");
> + goto err_misc_deregister;
> + }
> +
> + return 0;
> +
> + err_misc_deregister:
> + misc_deregister(&tlclk_miscdev);
> + err_unregister_chrdev:
> + unregister_chrdev(tlclk_major, "telco_clock");
> + return error;
> +}
> +
> +static int __devexit tlclk_remove(struct platform_device *dev)
> +{
> + sysfs_remove_group(&dev->dev.kobj, &tlclk_attribute_group);
> + misc_deregister(&tlclk_miscdev);
> + unregister_chrdev(tlclk_major, "telco_clock");
> +
> + return 0;
> +}
> +
> +static struct platform_driver tlclk_driver = {
> + .driver = {
> + .name = "telco_clock",
> + .owner = THIS_MODULE,
> + },
> + .probe = tlclk_probe,
> + .remove = __devexit_p(tlclk_remove),
> +};
> +
> +static struct platform_device *tlclk_device;
> +
> +static int __init tlclk_init(void)
> +{
> + int error;
>
> /* Read telecom clock IRQ number (Set by BIOS) */
> if (!request_region(TLCLK_BASE, 8, "telco_clock")) {
> printk(KERN_ERR "tlclk: request_region failed! 0x%X\n",
> TLCLK_BASE);
> - ret = -EBUSY;
> - goto out2;
> + return -EBUSY;
> }
> - telclk_interrupt = (inb(TLCLK_REG7) & 0x0f);
>
> - if (0x0F == telclk_interrupt ) { /* not MCPBL0010 ? */
> - printk(KERN_ERR "telclk_interrup = 0x%x non-mcpbl0010
> hw\n",
> + telclk_interrupt = (inb(TLCLK_REG7) & 0x0f);
> + if (0x0F == telclk_interrupt) { /* not MCPBL0010 ? */
> + printk(KERN_ERR
> + "tlclk: telclk_interrupt = 0x%x non-mcpbl0010
> hw\n",
> telclk_interrupt);
> - ret = -ENXIO;
> - goto out3;
> + error = -ENODEV;
> + goto err_release_region;
> }
>
> - init_timer(&switchover_timer);
> -
> - ret = misc_register(&tlclk_miscdev);
> - if (ret < 0) {
> - printk(KERN_ERR " misc_register retruns %d\n", ret);
> - ret = -EBUSY;
> - goto out3;
> + error = platform_driver_register(&tlclk_driver);
> + if (error) {
> + printk(KERN_ERR "tlclk: failed to register platform
> driver\n");
> + goto err_free_device;
> }
>
> - tlclk_device = platform_device_register_simple("telco_clock",
> - -1, NULL, 0);
> + tlclk_device = platform_device_alloc("telco_clock", -1);
> if (!tlclk_device) {
> - printk(KERN_ERR " platform_device_register retruns
> 0x%X\n",
> - (unsigned int) tlclk_device);
> - ret = -EBUSY;
> - goto out4;
> + printk(KERN_ERR "tlclk: failed to allocate platform
> device\n");
> + error = -ENOMEM;
> + goto err_unregister_driver;
> }
>
> - ret = sysfs_create_group(&tlclk_device->dev.kobj,
> - &tlclk_attribute_group);
> - if (ret) {
> - printk(KERN_ERR "failed to create sysfs device
> attributes\n");
> - sysfs_remove_group(&tlclk_device->dev.kobj,
> - &tlclk_attribute_group);
> - goto out5;
> + error = platform_device_add(tlclk_device);
> + if (error) {
> + printk(KERN_ERR "tlclk: failed to register platform
> device\n");
> + goto err_free_device;
> }
>
> return 0;
> -out5:
> - platform_device_unregister(tlclk_device);
> -out4:
> - misc_deregister(&tlclk_miscdev);
> -out3:
> +
> + err_free_device:
> + platform_device_put(tlclk_device);
> + err_unregister_driver:
> + platform_driver_unregister(&tlclk_driver);
> + err_release_region:
> release_region(TLCLK_BASE, 8);
> -out2:
> - kfree(alarm_events);
> -out1:
> - unregister_chrdev(tlclk_major, "telco_clock");
> - return ret;
> + return error;
> }
>
> static void __exit tlclk_cleanup(void)
> {
> - sysfs_remove_group(&tlclk_device->dev.kobj,
> &tlclk_attribute_group);
> platform_device_unregister(tlclk_device);
> - misc_deregister(&tlclk_miscdev);
> - unregister_chrdev(tlclk_major, "telco_clock");
> -
> + platform_driver_unregister(&tlclk_driver);
> release_region(TLCLK_BASE, 8);
> - del_timer_sync(&switchover_timer);
> - kfree(alarm_events);
> -
> }
>
> static void switchover_timeout(unsigned long data)
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

[-- Attachment #2: tlclk_2.6.15.rc6.git4.diff --]
[-- Type: text/x-diff, Size: 7011 bytes --]

diff -urN -X dontdiff linux-2.6.15-rc6-git4.org/drivers/char/tlclk.c linux-2.6.15-rc6-git4/drivers/char/tlclk.c
--- linux-2.6.15-rc6-git4.org/drivers/char/tlclk.c 2005-12-23 12:52:42.000000000 -0800
+++ linux-2.6.15-rc6-git4/drivers/char/tlclk.c 2005-12-23 14:25:55.000000000 -0800
@@ -34,7 +34,6 @@
#include <linux/kernel.h> /* printk() */
#include <linux/fs.h> /* everything... */
#include <linux/errno.h> /* error codes */
-#include <linux/delay.h> /* udelay */
#include <linux/slab.h>
#include <linux/ioport.h>
#include <linux/interrupt.h>
@@ -165,7 +164,7 @@
filter_select :
hardware_switching :
hardware_switching_mode :
-interrupt_switch :
+telclock_version :
mode_select :
refalign :
reset :
@@ -173,7 +172,6 @@
select_amcb2_transmit_clock :
select_redundant_clock :
select_ref_frequency :
-test_mode :

All sysfs interfaces are integers in hex format, i.e echo 99 > refalign
has the same effect as echo 0x99 > refalign.
@@ -202,6 +200,11 @@
{
int result;

+ alarm_events = kzalloc(sizeof(struct tlclk_alarms), GFP_KERNEL);
+ if (!alarm_events)
+ return -ENOMEM;
+
+ init_timer(&switchover_timer);
/* Make sure there is no interrupt pending while
* initialising interrupt handler */
inb(TLCLK_REG6);
@@ -212,6 +215,8 @@
SA_INTERRUPT, "telco_clock", tlclk_interrupt);
if (result == -EBUSY) {
printk(KERN_ERR "telco_clock: Interrupt can't be reserved!\n");
+ kfree(alarm_events);
+ alarm_events = NULL;
return -EBUSY;
}
inb(TLCLK_REG6); /* Clear interrupt events */
@@ -222,11 +227,14 @@
static int tlclk_release(struct inode *inode, struct file *filp)
{
free_irq(telclk_interrupt, tlclk_interrupt);
+ del_timer_sync(&switchover_timer);
+ kfree(alarm_events);
+ alarm_events = NULL;

return 0;
}

-ssize_t tlclk_read(struct file *filp, char __user *buf, size_t count,
+static ssize_t tlclk_read(struct file *filp, char __user *buf, size_t count,
loff_t *f_pos)
{
if (count < sizeof(struct tlclk_alarms))
@@ -242,7 +250,7 @@
return sizeof(struct tlclk_alarms);
}

-ssize_t tlclk_write(struct file *filp, const char __user *buf, size_t count,
+static ssize_t tlclk_write(struct file *filp, const char __user *buf, size_t count,
loff_t *f_pos)
{
return 0;
@@ -278,21 +286,21 @@
static DEVICE_ATTR(current_ref, S_IRUGO, show_current_ref, NULL);

-static ssize_t show_interrupt_switch(struct device *d,
+static ssize_t show_telclock_version(struct device *d,
struct device_attribute *attr, char *buf)
{
unsigned long ret_val;
unsigned long flags;

spin_lock_irqsave(&event_lock, flags);
- ret_val = inb(TLCLK_REG6);
+ ret_val = inb(TLCLK_REG5);
spin_unlock_irqrestore(&event_lock, flags);

return sprintf(buf, "0x%lX\n", ret_val);
}

-static DEVICE_ATTR(interrupt_switch, S_IRUGO,
- show_interrupt_switch, NULL);
+static DEVICE_ATTR(telclock_version, S_IRUGO,
+ show_telclock_version, NULL);

static ssize_t show_alarms(struct device *d,
struct device_attribute *attr, char *buf)
@@ -436,26 +444,6 @@
static DEVICE_ATTR(enable_clka0_output, S_IWUGO, NULL,
store_enable_clka0_output);

-static ssize_t store_test_mode(struct device *d,
- struct device_attribute *attr, const char *buf, size_t count)
-{
- unsigned long flags;
- unsigned long tmp;
- unsigned char val;
-
- sscanf(buf, "%lX", &tmp);
- dev_dbg(d, "tmp = 0x%lX\n", tmp);
-
- val = (unsigned char)tmp;
- spin_lock_irqsave(&event_lock, flags);
- SET_PORT_BITS(TLCLK_REG4, 0xfd, 2);
- spin_unlock_irqrestore(&event_lock, flags);
-
- return strnlen(buf, count);
-}
-
-static DEVICE_ATTR(test_mode, S_IWUGO, NULL, store_test_mode);
-
static ssize_t store_select_amcb2_transmit_clock(struct device *d,
struct device_attribute *attr, const char *buf, size_t count)
{
@@ -475,7 +463,7 @@
SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x38);
switch (val) {
case CLK_8_592MHz:
- SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
+ SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
break;
case CLK_11_184MHz:
SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
@@ -484,7 +472,7 @@
SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
break;
case CLK_44_736MHz:
- SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
+ SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
break;
}
} else
@@ -653,9 +641,7 @@
dev_dbg(d, "tmp = 0x%lX\n", tmp);
spin_lock_irqsave(&event_lock, flags);
SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
- udelay(2);
SET_PORT_BITS(TLCLK_REG0, 0xf7, 0x08);
- udelay(2);
SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
spin_unlock_irqrestore(&event_lock, flags);

@@ -706,7 +692,7 @@

static struct attribute *tlclk_sysfs_entries[] = {
&dev_attr_current_ref.attr,
- &dev_attr_interrupt_switch.attr,
+ &dev_attr_telclock_version.attr,
&dev_attr_alarms.attr,
&dev_attr_enable_clk3a_output.attr,
&dev_attr_enable_clk3b_output.attr,
@@ -714,7 +700,6 @@
&dev_attr_enable_clka1_output.attr,
&dev_attr_enable_clkb0_output.attr,
&dev_attr_enable_clka0_output.attr,
- &dev_attr_test_mode.attr,
&dev_attr_select_amcb1_transmit_clock.attr,
&dev_attr_select_amcb2_transmit_clock.attr,
&dev_attr_select_redundant_clock.attr,
@@ -744,16 +729,13 @@
printk(KERN_ERR "telco_clock: can't get major! %d\n", tlclk_major);
return ret;
}
- alarm_events = kzalloc( sizeof(struct tlclk_alarms), GFP_KERNEL);
- if (!alarm_events)
- goto out1;

/* Read telecom clock IRQ number (Set by BIOS) */
if (!request_region(TLCLK_BASE, 8, "telco_clock")) {
printk(KERN_ERR "tlclk: request_region failed! 0x%X\n",
TLCLK_BASE);
ret = -EBUSY;
- goto out2;
+ goto out1;
}
telclk_interrupt = (inb(TLCLK_REG7) & 0x0f);

@@ -761,16 +743,14 @@
printk(KERN_ERR "telclk_interrup = 0x%x non-mcpbl0010 hw\n",
telclk_interrupt);
ret = -ENXIO;
- goto out3;
+ goto out2;
}

- init_timer(&switchover_timer);
-
ret = misc_register(&tlclk_miscdev);
if (ret < 0) {
printk(KERN_ERR " misc_register retruns %d\n", ret);
ret = -EBUSY;
- goto out3;
+ goto out2;
}

tlclk_device = platform_device_register_simple("telco_clock",
@@ -779,7 +759,7 @@
printk(KERN_ERR " platform_device_register retruns 0x%X\n",
(unsigned int) tlclk_device);
ret = -EBUSY;
- goto out4;
+ goto out3;
}

ret = sysfs_create_group(&tlclk_device->dev.kobj,
@@ -788,18 +768,16 @@
printk(KERN_ERR "failed to create sysfs device attributes\n");
sysfs_remove_group(&tlclk_device->dev.kobj,
&tlclk_attribute_group);
- goto out5;
+ goto out4;
}

return 0;
-out5:
- platform_device_unregister(tlclk_device);
out4:
- misc_deregister(&tlclk_miscdev);
+ platform_device_unregister(tlclk_device);
out3:
- release_region(TLCLK_BASE, 8);
+ misc_deregister(&tlclk_miscdev);
out2:
- kfree(alarm_events);
+ release_region(TLCLK_BASE, 8);
out1:
unregister_chrdev(tlclk_major, "telco_clock");
return ret;
@@ -813,9 +791,6 @@
unregister_chrdev(tlclk_major, "telco_clock");

release_region(TLCLK_BASE, 8);
- del_timer_sync(&switchover_timer);
- kfree(alarm_events);
-
}

static void switchover_timeout(unsigned long data)