On Fri, 8 Jul 2005 19:16:57 -0500 Abhay Salunke wrote: | This is the patch to add dell_rbu driver. This patch requires the | firmware_class.c patch sent earlier which adds request_firmware_nowait_nohotplug | function. | Andrew , | Could you add this patch to the -mm tree. This patch was submitted about a | week ago for review. | | Signed-off-by: Abhay Salunke <[EMAIL PROTECTED]> | | Thanks | Abhay
General comment: The patches have lots of trailing whitespace. Please remove all of that. | diff -uprN linux-2.6.11.11.orig/drivers/firmware/dell_rbu.c linux-2.6.11.11.new/drivers/firmware/dell_rbu.c | --- linux-2.6.11.11.orig/drivers/firmware/dell_rbu.c 1969-12-31 18:00:00.000000000 -0600 | +++ linux-2.6.11.11.new/drivers/firmware/dell_rbu.c 2005-06-30 15:37:27.000000000 -0500 | @@ -0,0 +1,627 @@ | +/* | + * dell_rbu.c | + * Bios Update driver for Dell systems | + * Author: Dell Inc | + * Abhay Salunke <[EMAIL PROTECTED]> | + * | + * Copyright (C) 2005 Dell Inc. | + * | + * Remote BIOS Update (rbu) driver is used for updating DELL BIOS by | + * creating entries in the /sys file systems on Linux 2.6 and higher | + * kernels. The driver supports two mechanism to update the BIOS namely mechanisms BIOS, | + * contiguous and packetized. Both these methods still require having some | + * application to set the CMOS bit indicating the BIOS to update itself | + * after a reboot. | + * | + * Contiguous method: | + * This driver writes the incoming data in a monolithic image by allocating | + * contiguous physical pages large enough to accommodate the incoming BIOS | + * image size. | + * | + * Packetized method: | + * The driver writes the incoming packet image by allocating a new packet | + * on every time the packet data is written. This driver requires an each time that the | + * application to break the BIOS image in to fixed sized packet chunks. into | + * | + * See Documentation/dell_rbu.txt for more info. | + * | + */ | +#include <linux/version.h> | +#include <linux/config.h> | +#include <linux/init.h> | +#include <linux/module.h> | +#include <linux/string.h> | +#include <linux/errno.h> | +#include <linux/blkdev.h> | +#include <linux/device.h> | +#include <linux/spinlock.h> | +#include <linux/moduleparam.h> | +#include <linux/firmware.h> | +#include <linux/dma-mapping.h> | + | +MODULE_AUTHOR("Abhay Salunke <[EMAIL PROTECTED]>"); | +MODULE_DESCRIPTION("Driver for updating BIOS image on DELL systems"); | +MODULE_LICENSE("GPL"); | +MODULE_VERSION("1.0"); | + | +#define BIOS_SCAN_LIMIT 0xffffffff Please remove this, it's not used. | +#define MAX_IMAGE_LENGTH 16 | +static struct _rbu_data { | + void *image_update_buffer; | + unsigned long image_update_buffer_size; | + unsigned long bios_image_size; | + spinlock_t lock; | + unsigned long packet_read_count; | + unsigned long packet_write_count; | + unsigned long num_packets; | + unsigned long packetsize; | +} rbu_data; | + | +static char image_type[MAX_IMAGE_LENGTH] = "mono"; | +module_param_string(image_type, image_type, sizeof (image_type), 0); Why is permission 0? 0 means not visible in sysfs. But you tested this, so.... ? | +MODULE_PARM_DESC(image_type, "BIOS image type. choose- mono or packet"); | + | +struct packet_data { | + struct list_head list; | + size_t length; | + void *data; | + int ordernum; | +}; | + | +static struct packet_data packet_data_head; | + | +struct platform_device *rbu_device; | +int context; | +static dma_addr_t dell_rbu_dmaaddr; | + | +static int | +fill_last_packet(void *data, size_t length) | +{ | + struct list_head *ptemp_list; | + struct packet_data *packet = NULL; | + int packet_count = 0; | + | + pr_debug("fill_last_packet: entry \n"); entry\n (other places also) | + | + if (!rbu_data.num_packets) { | + pr_debug("fill_last_packet: num_packets=0\n"); | + return -ENOMEM; | + } | + | + packet_count = rbu_data.num_packets; | + | + ptemp_list = (&packet_data_head.list)->prev; | + | + packet = list_entry(ptemp_list, struct packet_data, list); | + | + if ((rbu_data.packet_write_count + length) > rbu_data.packetsize) { | + pr_debug("dell_rbu:%s: packet size data " | + "overrun\n", __FUNCTION__); | + return -EINVAL; | + } | + | + pr_debug("fill_last_packet : buffer = %p\n", packet->data); | + | + memcpy((packet->data + rbu_data.packet_write_count), data, length); | + | + if ((rbu_data.packet_write_count + length) == rbu_data.packetsize) { | + /* | + * this was the last data chunk in the packet | + * so reinitialize the packet data counter to zero | + */ | + rbu_data.packet_write_count = 0; | + } else | + rbu_data.packet_write_count += length; | + | + pr_debug("fill_last_packet: exit \n"); exit\n (other places too) | + return 0; | +} | + | +static int | +create_packet(size_t length) | +{ | + struct packet_data *newpacket; | + int ordernum = 0; | + | + pr_debug("create_packet: entry \n"); | + | + if (!rbu_data.packetsize) { | + pr_debug("create_packet: packetsize not specified\n"); | + return -EINVAL; | + } | + | + newpacket = kmalloc(sizeof (struct packet_data), GFP_KERNEL); | + if (!newpacket) { | + printk(KERN_WARNING | + "dell_rbu:%s: failed to allocate new " | + "packet\n", __FUNCTION__); | + return -ENOMEM; | + } | + | + ordernum = get_order(length); | + /* | + * there is no upper limit on memory | + * address for packetized mechanism | + */ | + newpacket->data = (unsigned char *) __get_free_pages(GFP_KERNEL, | + ordernum); | + | + pr_debug("create_packet: newpacket %p\n", newpacket->data); actually newpacket->data | + | + if (!newpacket->data) { | + printk(KERN_WARNING | + "dell_rbu:%s: failed to allocate new " | + "packet\n", __FUNCTION__); | + kfree(newpacket); | + return -ENOMEM; | + } | + | + newpacket->ordernum = ordernum; | + ++rbu_data.num_packets; | + /* | + * initialize the newly created packet headers | + */ | + INIT_LIST_HEAD(&newpacket->list); | + list_add_tail(&newpacket->list, &packet_data_head.list); | + /* | + * packets have fixed size | + */ | + newpacket->length = rbu_data.packetsize; | + | + pr_debug("create_packet: exit \n"); | + | + return 0; | +} | + | +static int | +packetize_data(void *data, size_t length) | +{ | + int rc = 0; | + | + if (!rbu_data.packet_write_count) { | + if ((rc = create_packet(length))) | + return rc; | + } | + if ((rc = fill_last_packet(data, length))) Just do: rc = fill_packet(data, length); return rc; | + return rc; | + | + return rc; | +} | + | +static int | +packet_read_list(char *data, size_t * pread_length) | +{ | + struct list_head *ptemp_list; | + int temp_count = 0; | + int bytes_copied = 0; | + int bytes_read = 0; | + int remaining_bytes = 0; | + char *pdest = data; | + | + /* check if we have any packets */ | + if (0 == rbu_data.num_packets) Please use (variable == constant) form. | + return -ENOMEM; | + | + remaining_bytes = *pread_length; | + bytes_read = rbu_data.packet_read_count; | + | + ptemp_list = (&packet_data_head.list)->next; | + while (!list_empty(ptemp_list)) { | + bytes_copied = do_packet_read(pdest, ptemp_list, | + remaining_bytes, bytes_read, &temp_count); | + remaining_bytes -= bytes_copied; | + bytes_read += bytes_copied; | + pdest += bytes_copied; | + /* | + * check if we reached end of buffer before reaching the | + * last packet | + */ | + if (remaining_bytes == 0) | + break; | + | + ptemp_list = ptemp_list->next; | + } | + /*finally set the bytes read */ /* finally | + *pread_length = bytes_read - rbu_data.packet_read_count; | + rbu_data.packet_read_count = bytes_read; | + return 0; | +} | + | + | +static ssize_t | +read_packet_data(char *buffer, loff_t pos, size_t count) | +{ | + int retval; | + size_t bytes_left; | + size_t data_length; | + char *ptempBuf = buffer; | + unsigned long imagesize; | + | + /* check to see if we have something to return */ | + if (rbu_data.num_packets == 0) { | + pr_debug("read_packet_data: no packets written\n"); | + retval = -ENOMEM; | + goto read_rbu_data_exit; | + } | + | + imagesize = rbu_data.num_packets * rbu_data.packetsize; | + | + if (pos > imagesize) { | + retval = 0; | + printk(KERN_WARNING "dell_rbu:read_packet_data: " | + "data underrun\n"); | + goto read_rbu_data_exit; | + } | + | + bytes_left = imagesize - pos; | + data_length = min(bytes_left, count); | + | + if ((retval = packet_read_list(ptempBuf, &data_length)) < 0) | + goto read_rbu_data_exit; | + | + if ((pos + count) > imagesize) { | + rbu_data.packet_read_count = 0; | + /* this was the last copy */ | + retval = bytes_left; | + } else | + retval = count; | + | + read_rbu_data_exit: Darn, almost missed that label. Please move it to column 1 (or 0). It's hidden as is. | + return retval; | +} | + | +static ssize_t | +read_rbu_mono_data(char *buffer, loff_t pos, size_t count) | +{ | + unsigned char *ptemp = NULL; | + size_t bytes_left = 0; | + size_t data_length = 0; | + ssize_t ret_count = 0; | + | + /* check to see if we have something to return */ | + if ((rbu_data.image_update_buffer == NULL) || | + (rbu_data.bios_image_size == 0)) { | + pr_debug("read_rbu_data_mono: image_update_buffer %p ," | + "bios_image_size %lu\n", | + rbu_data.image_update_buffer, | + rbu_data.bios_image_size); | + ret_count = -ENOMEM; | + goto read_rbu_data_exit; | + } | + | + if (pos > rbu_data.bios_image_size) { | + ret_count = 0; | + goto read_rbu_data_exit; | + } | + | + bytes_left = rbu_data.bios_image_size - pos; | + data_length = min(bytes_left, count); | + | + ptemp = rbu_data.image_update_buffer; | + memcpy(buffer, (ptemp + pos), data_length); | + | + if ((pos + count) > rbu_data.bios_image_size) | + /* this was the last copy */ | + ret_count = bytes_left; | + else | + ret_count = count; | + read_rbu_data_exit: Label in column 1. | + return ret_count; | +} | + | +static ssize_t | +read_rbu_data(struct kobject *kobj, char *buffer, loff_t pos, size_t count) | +{ | + ssize_t ret_count = 0; | + | + spin_lock(&rbu_data.lock); | + | + if (!strcmp(image_type, "mono")) | + ret_count = read_rbu_mono_data(buffer, pos, count); | + else if (!strcmp(image_type, "packet")) | + ret_count = read_packet_data(buffer, pos, count); | + else | + pr_debug("read_rbu_data: invalid image type specified\n"); Why is this pr_debug() and not a printk(KERN_WARNING...)? | + | + spin_unlock(&rbu_data.lock); | + return ret_count; | +} | + | +static ssize_t | +rbu_show_image_type(struct platform_device *rbu_dev, char *buf) | +{ | + unsigned int size = 0; Don't need to init size to 0 and then to the return value of sprintf(). | + size = sprintf(buf, "%s\n", image_type); | + return size; | +} | + | +static ssize_t | +rbu_store_image_type(struct platform_device *rbu_dev, | + const char *buf, size_t count) | +{ | + int rc = count; Blank line between data and code, please. | + spin_lock(&rbu_data.lock); | + | + if (strlen(buf) < MAX_IMAGE_LENGTH) | + sscanf(buf, "%s", image_type); | + else | + printk(KERN_WARNING "dell_rbu: image_type is invalid" | + "max chars = %d\n", MAX_IMAGE_LENGTH); | + | + /* we must free all previous allocations */ | + packet_empty_list(); | + img_update_free(); | + | + spin_unlock(&rbu_data.lock); | + return rc; | +} | + | +struct rbu_attribute { | + struct attribute attr; | + ssize_t(*show) (struct platform_device * rbu_dev, char *buf); | + ssize_t(*store) (struct platform_device * rbu_dev, | + const char *buf, size_t count); | +}; | + | +#define RBU_DEVICE_ATTR(_name,_mode,_show,_store ) \ | +struct rbu_attribute rbu_attr_##_name = { \ | + .attr ={.name= __stringify(_name), .mode= _mode, .owner= THIS_MODULE},\ Use some spaces around '=', as in " = ". And like below. | + .show = _show, \ | + .store = _store, \ | +}; | + | +static RBU_DEVICE_ATTR(image_type, 0644, rbu_show_image_type, | + rbu_store_image_type); | + | +static struct bin_attribute rbu_data_attr = { | + .attr = {.name = "data",.owner = THIS_MODULE,.mode = 0444}, Spaces around '-' and after ','. | + .read = read_rbu_data, | +}; | + | +void | +callbackfn_rbu(const struct firmware *fw, void *context) | +{ | + int rc = 0; | + | + if (!fw || !fw->size) | + return; | + | + spin_lock(&rbu_data.lock); | + if (!strcmp(image_type, "mono")) { | + if (!img_update_realloc(fw->size)) | + memcpy(rbu_data.image_update_buffer, | + fw->data, fw->size); | + } else if (!strcmp(image_type, "packet")) { | + if (!rbu_data.packetsize) | + rbu_data.packetsize = fw->size; | + else if (rbu_data.packetsize != fw->size) { | + packet_empty_list(); | + rbu_data.packetsize = fw->size; | + } | + packetize_data(fw->data, fw->size); | + } else | + pr_debug("invalid image type specified.\n"); Wny pr_debug() instead of printk() ? | + spin_unlock(&rbu_data.lock); | + | + rc = request_firmware_nowait_nohotplug(THIS_MODULE, | + "dell_rbu", &rbu_device->dev, &context, callbackfn_rbu); | + if (rc) | + printk(KERN_ERR | + "dell_rbu:%s request_firmware_nowait failed" | + " %d\n", __FUNCTION__, rc); | +} | + | +static int __init | +dcdrbu_init(void) | +{ | + int rc = 0; Blank line here, please. | + spin_lock_init(&rbu_data.lock); | + | + init_packet_head(); | + | + rbu_device = | + platform_device_register_simple("dell_rbu", -1, NULL, 0); | + if (!rbu_device) { | + printk(KERN_ERR | + "dell_rbu:%s:platform_device_register_simple " | + "failed\n", __FUNCTION__); | + return -EIO; | + } | + | + sysfs_create_file(&rbu_device->dev.kobj, | + &rbu_attr_image_type.attr); | + | + sysfs_create_bin_file(&rbu_device->dev.kobj, &rbu_data_attr); | + | + rc = request_firmware_nowait_nohotplug(THIS_MODULE, | + "dell_rbu", &rbu_device->dev, &context, callbackfn_rbu); | + if (rc) | + printk(KERN_ERR "dell_rbu:%s:request_firmware_nowait" | + " failed %d\n", __FUNCTION__, rc); | + | + return rc; | + | +} | + | +static __exit void | +dcdrbu_exit(void) | +{ | + spin_lock(&rbu_data.lock); | + packet_empty_list(); | + img_update_free(); | + spin_unlock(&rbu_data.lock); | + platform_device_unregister(rbu_device); | +} | + | +module_exit(dcdrbu_exit); | +module_init(dcdrbu_init); | diff -uprN linux-2.6.11.11.orig/drivers/firmware/Kconfig linux-2.6.11.11.new/drivers/firmware/Kconfig | --- linux-2.6.11.11.orig/drivers/firmware/Kconfig 2005-06-14 21:00:10.000000000 -0500 | +++ linux-2.6.11.11.new/drivers/firmware/Kconfig 2005-06-30 15:37:43.000000000 -0500 | @@ -58,4 +58,15 @@ config EFI_PCDP | | See <http://www.dig64.org/specifications/DIG64_HCDPv20_042804.pdf> | | +config DELL_RBU | + tristate "BIOS update support for DELL systems via sysfs" | + default y Why does the rest of the world want to build this driver? | + select FW_LOADER | + help | + Say Y if you want to have the option of updating the BIOS for your | + DELL system. Note you need a supporting application to comunicate | + with the BIOS regardin the new image for the image update to | + take effect. When & where is this mystery app going to be available? | + See <file:Documentation/dell_rbu.txt> for more details on the driver. | + | endmenu --- ~Randy - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/