Re: [PATCH] Topcliff: Update PCH_SPI driver to 2.6.35

> > > + This registers SPI devices for a given board. > > + > > +config PCH_SPI_PLATFORM_DEVICE_COUNT > > + int "PCH SPI Bus count" > > + range 1 2 > > + depends on PCH_SPI_PLATFORM_DEVICE > > + help > > + This driver is for PCH SPI of Topcliff which is an IOH for x86 > > + embedded processor. > > + The number of SPI buses/channels supported by the PCH SPI controller. > > + PCH SPI of Topcliff supports only one channel. > > Being required to specific this at kernel config time isn't > multiplatform friendly. Can the driver detect the number of busses at > runtime. > > > + > > +config PCH_SPI > > + tristate "PCH SPI Controller" > > + depends on PCI > > + help > > + This driver is for PCH SPI of Topcliff which is an IOH for x86 > > + embedded processor. > > + This driver can access PCH SPI bus device. > > Put config PCH_SPI above PCH_SPI_PLATFORM_DEVICE and make > PCH_SPI_PLATFORM_DEVICE depend on PCH_SPI. (In fact, since > PCH_SPI_PLATFORM_DEVICE is always yes when PCH_SPI is selected, then > you probably don't need two configuration signals). > > > + > > config SPI_ATMEL > > tristate "Atmel SPI Controller" > > depends on (ARCH_AT91 || AVR32) > > diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile > > index f3d2810..aecc873 100644 > > --- a/drivers/spi/Makefile > > +++ b/drivers/spi/Makefile > > @@ -59,3 +59,7 @@ obj-$(CONFIG_SPI_TLE62X0) += tle62x0.o > > > > # SPI slave drivers (protocol for that link) > > # ... add above this line ... > > +obj-$(CONFIG_PCH_SPI_PLATFORM_DEVICE) += pch_spi_platform_devices.o > > +obj-$(CONFIG_PCH_SPI) += pch_spi.o > > +pch_spi-objs := pch_spi_main.o > > No need to use pch_spi-objs when there is only one .o file. Just name > the .c file what you want the resulting kernel module to be named. > Also, please rename the modules to "spi_pch_platform_device.o" and > "spi_pch.o" (I'm enforcing all new spi drivers to start with the > prefix "spi_"). > > Finally, do pch_spi_platform_devices.o and pch_spi_main.o really need > to be in separate modules? > > > + > > diff --git a/drivers/spi/pch_spi.h b/drivers/spi/pch_spi.h > > new file mode 100644 > > index 0000000..b40377a > > --- /dev/null > > +++ b/drivers/spi/pch_spi.h > > @@ -0,0 +1,177 @@ > > +/* > > + * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD. > > + * > > It is helpful to have a one line description of what this file is for > in at the top of the header comment block. > > [...] > > +#define PCH_SET_BITMSK(var, bitmask) ((var) |= (bitmask)) > > +#define PCH_CLR_BITMSK(var, bitmask) ((var) &= (~(bitmask))) > > Because the macro has side-effects, I'd prefer to see static inlines > used here with an all lowercase name. > > > + > > +/** > > + * struct pch_spi_data - Holds the SPI channel specific details > > + * @io_remap_addr: The remapped PCI base address > > + * @p_master: Pointer to the SPI master structure > > + * @work: Reference to work queue handler > > + * @p_wk_q: Workqueue for carrying out execution of the > > + * requests > > + * @wait: Wait queue for waking up upon receiving an > > + * interrupt. > > + * @b_transfer_complete: Status of SPI Transfer > > + * @bcurrent_msg_processing: Status flag for message processing > > + * @lock: Lock for protecting this structure > > + * @queue: SPI Message queue > > + * @status: Status of the SPI driver > > + * @bpw_len: Length of data to be transferred in bits per > > + * word > > + * @b_transfer_active: Flag showing active transfer > > + * @tx_index: Transmit data count; for bookkeeping during > > + * transfer > > + * @rx_index: Receive data count; for bookkeeping during > > + * transfer > > + * @tx_buff: Buffer for data to be transmitted > > + * @rx_index: Buffer for Received data > > + * @n_curnt_chip: The chip number that this SPI driver currently > > + * operates on > > + * @p_current_chip: Reference to the current chip that this SPI > > + * driver currently operates on > > + * @p_current_msg: The current message that this SPI driver is > > + * handling > > + * @p_cur_trans: The current transfer that this SPI driver is > > + * handling > > + * @p_board_dat: Reference to the SPI device data structure > > + */ > > +struct pch_spi_data { > > + void __iomem *io_remap_addr; > > + struct spi_master *p_master; > > + struct work_struct work; > > + struct workqueue_struct *p_wk_q; > > + wait_queue_head_t wait; > > + u8 b_transfer_complete; > > + u8 bcurrent_msg_processing; > > + spinlock_t lock; > > + struct list_head queue; > > + u8 status; > > + u32 bpw_len; > > + s8 b_transfer_active; > > + u32 tx_index; > > + u32 rx_index; > > + u16 *pkt_tx_buff; > > + u16 *pkt_rx_buff; > > + u8 n_curnt_chip; > > + struct spi_device *p_current_chip; > > + struct spi_message *p_current_msg; > > + struct spi_transfer *p_cur_trans; > > + struct pch_spi_board_data *p_board_dat; > > +}; > > + > > +/** > > + * struct pch_spi_board_data - Holds the SPI device specific details > > + * @pdev: Pointer to the PCI device > > + * @irq_reg_sts: Status of IRQ registration > > + * @pci_req_sts: Status of pci_request_regions > > + * @suspend_sts: Status of suspend > > + * @p_data: Pointer to SPI channel data structure > > + */ > > +struct pch_spi_board_data { > > + struct pci_dev *pdev; > > + u8 irq_reg_sts; > > + u8 pci_req_sts; > > + u8 suspend_sts; > > + struct pch_spi_data *p_data[PCH_MAX_DEV]; > > +}; > > +#endif > > diff --git a/drivers/spi/pch_spi_main.c b/drivers/spi/pch_spi_main.c > > new file mode 100644 > > index 0000000..da87d92 > > --- /dev/null > > +++ b/drivers/spi/pch_spi_main.c > > @@ -0,0 +1,1823 @@ > > +/* > > + * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD. > > + * > > + * 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; version 2 of the License. > > + * > > + * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307, > > USA. > > + */ > > + > > +#include <linux/pci.h> > > +#include <linux/wait.h> > > +#include <linux/spi/spi.h> > > +#include <linux/interrupt.h> > > +#include <linux/sched.h> > > +#include <linux/spi/spidev.h> > > +#include <linux/module.h> > > +#include <linux/device.h> > > +#include <linux/mutex.h> > > +#include "pch_spi.h" > > + > > +static struct pci_device_id pch_spi_pcidev_id[] = { > > + {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_GE_SPI)}, > > + {0,} > > +}; > > + > > +static void (*pch_spi_gcbptr) (struct pch_spi_data *); > > +static DEFINE_MUTEX(pch_spi_mutex); > > Are these statics really necessary? I think the callback can be > removed (see my comment below). The mutex should probably be a member > of the pch_spi_data structure. Getting rid of the static symbols will > make it easier if the driver ever needs to support multiple instances > of the device. > > > +/** > > + * ch_spi_disable_interrupts() - Disables specified interrupts > > + * @master: Pointer to struct spi_master. > > + * @interrupt: Interrups to be enabled. > > + */ > > +static void pch_spi_disable_interrupts(struct spi_master *master, u8 > > interrupt) > > +{ > > + u32 reg_val_spcr; > > + > > + reg_val_spcr = pch_spi_readreg(master, PCH_SPCR); > > + > > + dev_dbg(&master->dev, "%s SPCR content =%x\n", > > + __func__, reg_val_spcr); > > + > > + switch (interrupt & 0x1f) { > > Is it possible for more than one of these interrupt bits to be set at > the same time? If so, then this switch() statement won't always work. > > > + case PCH_RFI: > > + /*clear RFIE bit in SPCR */ > > + dev_dbg(&master->dev, > > + "%s clearing RFI in pch_spi_disable_interrupts\n", __func__); > > + PCH_CLR_BITMSK(reg_val_spcr, SPCR_RFIE_BIT); > > + break; > > + > > + case PCH_TFI: > > + /*clear TFIE bit in SPCR */ > > + dev_dbg(&master->dev, > > + "%s clearing TFI in pch_spi_disable_interrupts\n", __func__); > > + PCH_CLR_BITMSK(reg_val_spcr, SPCR_TFIE_BIT); > > + break; > > + > > + case PCH_FI: > > + /*clear FIE bit in SPCR */ > > + dev_dbg(&master->dev, > > + "%s clearing FI in pch_spi_disable_interrupts\n", __func__); > > + PCH_CLR_BITMSK(reg_val_spcr, SPCR_FIE_BIT); > > + break; > > + > > + case PCH_ORI: > > + /*clear ORIE bit in SPCR */ > > + dev_dbg(&master->dev, > > + "%s clearing ORI in pch_spi_disable_interrupts\n", __func__); > > + PCH_CLR_BITMSK(reg_val_spcr, SPCR_ORIE_BIT); > > + break; > > + > > + case PCH_MDFI: > > + /*clear MODFIE bit in SPCR */ > > + dev_dbg(&master->dev, > > + "%s clearing MDFI in pch_spi_disable_interrupts\n", __func__); > > + PCH_CLR_BITMSK(reg_val_spcr, SPCR_MDFIE_BIT); > > + break; > > + > > + default: > > + dev_info(&master->dev, > > + "%s Unknown interrupt(=0x%02x)\n", __func__, interrupt & 0x1f); > > + } > > + > > + pch_spi_writereg(master, PCH_SPCR, reg_val_spcr); > > + > > + dev_dbg(&master->dev, "%s SPCR after disabling interrupts =%x\n", > > + __func__, reg_val_spcr); > > +} > > + > > +/** > > + * pch_spi_handler() - Interrupt handler > > + * @irq: The interrupt number. > > + * @dev_id: Pointer to struct pch_spi_board_data. > > + */ > > +static irqreturn_t pch_spi_handler(int irq, void *dev_id) > > +{ > > + /*channel & read/write indices */ > > + int dev, read_cnt; > > + > > + /*SPSR content */ > > + u32 reg_spsr_val, reg_spcr_val; > > + > > + /*book keeping variables */ > > + u32 n_read, tx_index, rx_index, bpw_len; > > + > > + /*to hold channel data */ > > + struct pch_spi_data *p_data; > > + > > + /*buffer to store rx/tx data */ > > + u16 *pkt_rx_buffer, *pkt_tx_buff; > > + > > + /*register addresses */ > > + void __iomem *spsr; > > + void __iomem *spdrr; > > + void __iomem *spdwr; > > + > > + /*remapped pci base address */ > > + void __iomem *io_remap_addr; > > + > > + irqreturn_t tRetVal = IRQ_NONE; > > nit: use lowercase for variable names. > > > + > > + struct pch_spi_board_data *p_board_dat = > > + (struct pch_spi_board_data *)dev_id; > > + > > + if (p_board_dat->suspend_sts == true) { > > + dev_dbg(&p_board_dat->pdev->dev, > > + "%s returning due to suspend\n", __func__); > > + } else { > > + for (dev = 0; dev < PCH_MAX_DEV; dev++) { > > + p_data = p_board_dat->p_data[dev]; > > + io_remap_addr = p_data->io_remap_addr; > > + spsr = io_remap_addr + PCH_SPSR; > > + > > + reg_spsr_val = ioread32(spsr); > > + > > + /*Check if the interrupt is for SPI device */ > > + > > + if (reg_spsr_val & (SPSR_FI_BIT | SPSR_RFI_BIT)) { > > + dev_dbg(&p_board_dat->pdev->dev, > > + "SPSR in %s=%x\n", __func__, reg_spsr_val); > > + /*clear interrupt */ > > + iowrite32(reg_spsr_val, spsr); > > + > > + if (p_data->b_transfer_active == true) { > > + rx_index = p_data->rx_index; > > + tx_index = p_data->tx_index; > > + bpw_len = p_data->bpw_len; > > + pkt_rx_buffer = p_data->pkt_rx_buff; > > + pkt_tx_buff = p_data->pkt_tx_buff; > > + > > + spdrr = io_remap_addr + PCH_SPDRR; > > + spdwr = io_remap_addr + PCH_SPDWR; > > + > > + n_read = PCH_READABLE(reg_spsr_val); > > + > > + for (read_cnt = 0; (read_cnt < n_read); > > + read_cnt++) { > > + /*read data */ > > + pkt_rx_buffer[rx_index++] = > > + ioread32(spdrr); > > + /*write data */ > > + > > + if (tx_index < bpw_len) { > > + iowrite32 > > + (pkt_tx_buff > > + [tx_index++], > > + spdwr); > > + } > > + } > > + > > + /*disable RFI if not needed */ > > + if ((bpw_len - rx_index) <= > > + PCH_MAX_FIFO_DEPTH) { > > + dev_dbg(&p_board_dat->pdev->dev, > > + "%s disabling RFI as data " > > + "remaining=%d\n", __func__, > > + (bpw_len - rx_index)); > > + > > + reg_spcr_val = > > + ioread32(io_remap_addr + > > + PCH_SPCR); > > + > > + /*disable RFI */ > > + PCH_CLR_BITMSK(reg_spcr_val, > > + SPCR_RFIE_BIT); > > + > > + /*reset rx threshold */ > > + reg_spcr_val &= > > + MASK_RFIC_SPCR_BITS; > > + reg_spcr_val |= > > + (PCH_RX_THOLD_MAX << > > + SPCR_RFIC_FIELD); > > + > > + iowrite32(PCH_CLR_BITMSK > > + (reg_spcr_val, > > + SPCR_RFIE_BIT), > > + (io_remap_addr + > > + PCH_SPCR)); > > + } > > + > > + /*update counts */ > > + p_data->tx_index = tx_index; > > + > > + p_data->rx_index = rx_index; > > + > > + dev_dbg(&p_board_dat->pdev->dev, > > + "%s rx_index=%d tx_index=%d " > > + "nWritable=%d n_read=%d\n", > > + __func__, rx_index, tx_index, > > + (16 - > > + (PCH_WRITABLE(reg_spsr_val))), > > + n_read); > > + > > + } > > + > > + /*if transfer complete interrupt */ > > + if (reg_spsr_val & SPSR_FI_BIT) { > > + dev_dbg(&p_board_dat->pdev->dev, > > + "%s FI bit in SPSR" > > + " set\n", __func__); > > + > > + /*disable FI & RFI interrupts */ > > + pch_spi_disable_interrupts(p_data-> > > + p_master, PCH_FI | PCH_RFI); > > + > > + /*transfer is completed;inform > > + pch_spi_process_messages */ > > + > > + if (pch_spi_gcbptr != NULL) { > > + dev_dbg(&p_board_dat->pdev->dev, > > + "%s invoking callback\n", > > + __func__); > > + (*pch_spi_gcbptr) (p_data); > > + } > > + } > > + > > + tRetVal = IRQ_HANDLED; > > + } > > + } > > + } > > Wow, the nesting on this function is very deep, and hard to follow. > Candidate for refactoring? > > > + > > + dev_dbg(&p_board_dat->pdev->dev, "%s EXIT return value=%d\n", > > + __func__, tRetVal); > > + > > + return tRetVal; > > +} > > + > > +/** > > + * pch_spi_entcb() - Registers the callback function > > + * @pch_spi_cb: Callback function pointer. > > + */ > > +static void pch_spi_entcb(void (*pch_spi_cb) (struct pch_spi_data *)) > > +{ > > + if (pch_spi_cb != NULL) > > + pch_spi_gcbptr = pch_spi_cb; > > +} > > This trivial function is used exactly once in the probe routine. Just > set the variable in-place. In fact, this doesn't need to be a > callback at all because it is *always* set to pch_spi_callback. > Removing it will make the driver simpler. > > > +/** > > + * pch_spi_setup_transfe() - Configures the PCH SPI hardware for transfer > > + * @spi: Pointer to struct spi_device. > > + */ > > +static void pch_spi_setup_transfer(struct spi_device *spi) > > +{ > > + u32 reg_spcr_val; > > + > > + dev_dbg(&spi->dev, "%s SPBRR content =%x setting baud rate=%d\n", > > + __func__, pch_spi_readreg(spi->master, PCH_SPBRR), spi->max_speed_hz); > > + > > + pch_spi_set_baud_rate(spi->master, spi->max_speed_hz); > > + > > + /*set bits per word */ > > + dev_dbg(&spi->dev, "%s :setting bits_per_word=%d\n", > > + __func__, spi->bits_per_word); > > + pch_spi_set_bits_per_word(spi->master, spi->bits_per_word); > > + > > + dev_dbg(&spi->dev, > > + "%s SPBRR content after setting baud rate & bits per word=%x\n", > > + __func__, pch_spi_readreg(spi->master, PCH_SPBRR)); > > + > > + reg_spcr_val = pch_spi_readreg(spi->master, PCH_SPCR); > > + dev_dbg(&spi->dev, "%s SPCR content = %x\n", __func__, reg_spcr_val); > > + > > + /*set bit justification */ > > + > > + if ((spi->mode & SPI_LSB_FIRST) != 0) { > > + /*LSB first */ > > + PCH_CLR_BITMSK(reg_spcr_val, SPCR_LSBF_BIT); > > + dev_dbg(&spi->dev, "%s :setting LSBF bit to 0\n", __func__); > > + } else { > > + /*MSB first */ > > + PCH_SET_BITMSK(reg_spcr_val, SPCR_LSBF_BIT); > > + dev_dbg(&spi->dev, "%s :setting LSBF bit to 1\n", __func__); > > + } > > There are a lot of these if/else blocks which call > PCH_{SET,CLR}_BITMSK(). The code could be simplified with a helper > function that implements the two paths. Something like > pch_spi_clrsetbits() perhaps. > > > > +static int pch_spi_transfer(struct spi_device *pspi, struct spi_message > > *pmsg) > > +{ > > + > > + struct spi_transfer *p_transfer; > > + struct pch_spi_data *p_data = spi_master_get_devdata(pspi->master); > > + int retval; > > + int ret; > > + > > + ret = mutex_lock_interruptible(&pch_spi_mutex); > > + if (ret) { > > + retval = -ERESTARTSYS; > > + goto err_return; > > + } > > + > > + /*validate spi message and baud rate */ > > + if (unlikely((list_empty(&pmsg->transfers) == 1) || > > + ((pspi->max_speed_hz) == 0))) { > > + if (list_empty(&pmsg->transfers) == 1) > > + dev_err(&pspi->dev, "%s list empty\n", __func__); > > + > > + if ((pspi->max_speed_hz) == 0) { > > + dev_err(&pspi->dev, "%s pch_spi_tranfer maxspeed=%d\n", > > + __func__, (pspi->max_speed_hz)); > > + } > > + dev_err(&pspi->dev, "%s returning EINVAL\n", __func__); > > + > > + retval = -EINVAL; > > + goto err_return_mutex; > > + } > > + > > + dev_dbg(&pspi->dev, > > + "%s Transfer List not empty. Transfer Speed is set.\n", __func__); > > + > > + /*validate Tx/Rx buffers and Transfer length */ > > + list_for_each_entry(p_transfer, &pmsg->transfers, > > + transfer_list) { > > + if ((((p_transfer->tx_buf) == NULL) > > + && ((p_transfer->rx_buf) == NULL)) > > + || (p_transfer->len == 0)) { > > + if (((p_transfer->tx_buf) == NULL) > > + && ((p_transfer->rx_buf) == NULL)) { > > + dev_err(&pspi->dev, > > + "%s Tx and Rx buffer NULL\n", __func__); > > + } > > + > > + if (p_transfer->len == 0) { > > + dev_err(&pspi->dev, "%s Transfer" > > + " length invalid\n", __func__); > > + } > > + > > + dev_err(&pspi->dev, "%s returning EINVAL\n", __func__); > > + > > + retval = -EINVAL; > > + goto err_return_mutex; > > + } > > + > > + dev_dbg(&pspi->dev, > > + "%s Tx/Rx buffer valid. Transfer length valid\n", __func__); > > + > > + /*if baud rate hs been specified validate the same */ > > + if (p_transfer->speed_hz) { > > + if ((p_transfer->speed_hz) > > > + PCH_MAX_BAUDRATE) { > > + retval = -EINVAL; > > + dev_err(&pspi->dev, > > + "%s Invalid Baud rate\n", __func__); > > + goto err_return_mutex; > > + } > > If the requested speed is too fast, then the maximum bus speed should > be used instead. > > > + /*copy Tx Data */ > > + if ((p_data->p_cur_trans->tx_buf) != NULL) { > > + for (j = 0; j < (p_data->bpw_len); j++) { > > + if (PCH_8_BPW == *bpw) { > > + p_data->pkt_tx_buff[j] = > > + (((u8 *) (p_data->p_cur_trans-> > > + tx_buf))[j]); > > + dev_dbg(&p_data->p_master->dev, "%s=%x\n", > > + __func__, (p_data->pkt_tx_buff[j])); > > + } else { > > + p_data->pkt_tx_buff[j] = > > + ((u16 *) (p_data->p_cur_trans-> > > + tx_buf))[j]; > > + dev_dbg(&p_data->p_master->dev, > > + "%s xmt data = %x\n", __func__, > > + (p_data->pkt_tx_buff[j])); > > + } > > + } > > Optimization note; by coding it this way, the (PCH_8_BPW == *bpw) test > is being performed on every iteration through the loop. If you had > separate iterators for 8 and 16 bit transfer, the code will be faster. > > > + } > > + > > + /*if len greater than PCH_MAX_FIFO_DEPTH, > > + write 16,else len bytes */ > > Nit: It would be easier to read if the comment blocks were tidied up. > There should be a space between /* and the first word, and some > comments, like this one, will fit on a single line. > > > + if ((p_data->bpw_len) > PCH_MAX_FIFO_DEPTH) > > + n_writes = PCH_MAX_FIFO_DEPTH; > > + else > > + n_writes = (p_data->bpw_len); > > + > > + dev_dbg(&p_data->p_master->dev, > > + "\n%s:Pulling down SSN low - writing 0x2 to SSNXCR\n", __func__); > > When splitting lines in the middle of an argument list, please indent > up to the same level as the first argument. > > > +static void pch_spi_copy_rx_data(struct pch_spi_data *p_data, int bpw) > > +{ > > + int j; > > + /*copy Rx Data */ > > + if ((p_data->p_cur_trans->rx_buf) != NULL) { > > if (p_data->pcur_trans->rx_buf) > return; > > That reduces the indentation on the rest of the function by one level > and makes it easier to read and understand. > > > + for (j = 0; j < (p_data->bpw_len); j++) { > > + if (PCH_8_BPW == bpw) { > > + ((u8 *) (p_data->p_cur_trans->rx_buf))[j] = > > + (u8) ((p_data->pkt_rx_buff[j]) & 0xFF); > > + > > + dev_dbg(&p_data->p_master->dev, > > + "rcv data in %s =%x\n", __func__, > > + (p_data->pkt_rx_buff[j])); > > + > > + } else { > > + ((u16 *) (p_data->p_cur_trans->rx_buf))[j] = > > + (u16) (p_data->pkt_rx_buff[j]); > > + dev_dbg(&p_data->p_master->dev, > > + "rcv data in %s = %x\n", __func__, > > + (p_data->pkt_rx_buff[j])); > > + } > > + } > > Same comment on optimization. > > > + } > > +} > > + > > + > > +static void pch_spi_process_messages(struct work_struct *pwork) > > +{ > > + struct spi_message *pmsg; > > + int bpw; > > + > > + struct pch_spi_data *p_data = > > + container_of(pwork, struct pch_spi_data, work); > > + dev_dbg(&p_data->p_master->dev, > > + "%s p_data initialized\n", __func__); > > + > > + spin_lock(&p_data->lock); > > + > > + /*check if suspend has been initiated;if yes flush queue */ > > + if ((p_data->p_board_dat->suspend_sts) > > + || (p_data->status == STATUS_EXITING)) { > > + dev_dbg(&p_data->p_master->dev, > > + "%s suspend/remove initiated,flushing queue\n", __func__); > > + list_for_each_entry(pmsg, p_data->queue.next, queue) { > > + pmsg->status = -EIO; > > + > > + if (pmsg->complete != 0) > > + pmsg->complete(pmsg->context); > > The complete callback cannot be called while still holding the spinlock. > > > + > > + /*delete from queue */ > > + list_del_init(&pmsg->queue); > > + } > > + > > + spin_unlock(&p_data->lock); > > Put a return statement here. It will eliminate the else { } > indentation below; again improving readability. > > > + } else { > > + p_data->bcurrent_msg_processing = true; > > + dev_dbg(&p_data->p_master->dev, > > + "%s Set p_data->bcurrent_msg_processing= true\n", > > + __func__); > > + > > + /*Get the message from the queue and delete it from there. */ > > + p_data->p_current_msg = > > + list_entry(p_data->queue.next, struct spi_message, > > + queue); > > + dev_dbg(&p_data->p_master->dev, > > + "%s :Got new message from queue\n", __func__); > > + list_del_init(&p_data->p_current_msg->queue); > > + > > + p_data->p_current_msg->status = 0; > > + > > + dev_dbg(&p_data->p_master->dev, > > + "%s :Invoking pch_spi_select_chip\n", __func__); > > + pch_spi_select_chip(p_data, p_data->p_current_msg->spi); > > + > > + spin_unlock(&p_data->lock); > > + > > + do { > > + /*If we are already processing a message get the next > > + transfer structure from the message otherwise retrieve > > + the 1st transfer request from the message. */ > > + spin_lock(&p_data->lock); > > + > > + if (p_data->p_cur_trans == NULL) { > > + p_data->p_cur_trans = > > + list_entry(p_data->p_current_msg->transfers. > > + next, struct spi_transfer, > > + transfer_list); > > + dev_dbg(&p_data->p_master->dev, > > + "%s :Getting 1st transfer message\n", > > + __func__); > > + } else { > > + p_data->p_cur_trans = > > + list_entry(p_data->p_cur_trans-> > > + transfer_list.next, > > + struct spi_transfer, > > + transfer_list); > > + dev_dbg(&p_data->p_master->dev, > > + "%s :Getting next transfer message\n", > > + __func__); > > + } > > + > > + spin_unlock(&p_data->lock); > > + > > + pch_spi_set_tx(p_data, &bpw, &pmsg); > > + > > + /*Control interrupt*/ > > + pch_spi_set_ir(p_data); > > + > > + /*Disable SPI transfer */ > > + dev_dbg(&p_data->p_master->dev, > > + "%s:invoking pch_spi_set_enable to " > > + "disable spi transfer\n", __func__); > > + pch_spi_set_enable((p_data->p_current_chip), false); > > + > > + /*clear FIFO */ > > + dev_dbg(&p_data->p_master->dev, > > + "%s:invoking pch_spi_clear_fifo to clear fifo\n", > > + __func__); > > + pch_spi_clear_fifo(p_data->p_master); > > + > > + /*copy Rx Data */ > > + pch_spi_copy_rx_data(p_data, bpw); > > + > > + /*free memory */ > > + kfree(p_data->pkt_rx_buff); > > + if (p_data->pkt_rx_buff) > > + p_data->pkt_rx_buff = NULL; > > + > > + kfree(p_data->pkt_tx_buff); > > + if (p_data->pkt_tx_buff) > > + p_data->pkt_tx_buff = NULL; > > + > > + /*increment message count */ > > + p_data->p_current_msg->actual_length += > > + p_data->p_cur_trans->len; > > + > > + dev_dbg(&p_data->p_master->dev, > > + "%s:p_data->p_current_msg->actual_length=%d\n", > > + __func__, p_data->p_current_msg->actual_length); > > + > > + /*check for delay */ > > + if (p_data->p_cur_trans->delay_usecs) { > > + dev_dbg > > + (&p_data->p_master->dev, "%s:" > > + "delay in usec=%d\n", __func__, > > + p_data->p_cur_trans->delay_usecs); > > + udelay(p_data->p_cur_trans->delay_usecs); > > + } > > + > > + spin_lock(&p_data->lock); > > + > > + /*No more transfer in this message. */ > > + if ((p_data->p_cur_trans->transfer_list.next) == > > + &(p_data->p_current_msg->transfers)) { > > + pch_spi_nomore_transfer(p_data, pmsg); > > + } > > + > > + spin_unlock(&p_data->lock); > > + > > + } while ((p_data->p_cur_trans) != NULL); > > + } > > +} > > + > > +static int pch_spi_probe(struct pci_dev *pdev, const struct pci_device_id > > *id) > > +{ > > + > > + struct spi_master *p_master[PCH_MAX_DEV]; > > + > > + struct pch_spi_board_data *p_board_dat; > > + int retval, i, j, k, l, m; > > nit: for loop index variables should be reused. Don't use a new index > for every single for() loop. > > > + int spi_alloc_num, spi_master_num; > > + > > + dev_dbg(&pdev->dev, "%s ENTRY\n", __func__); > > + /* initialize the call back function */ > > + pch_spi_entcb(pch_spi_callback); > > + dev_dbg(&pdev->dev, "%s invoked pch_spi_entcb\n", __func__); > > + > > + /* allocate memory for private data */ > > + p_board_dat = kmalloc(sizeof(struct pch_spi_board_data), GFP_KERNEL); > > kzalloc() > > > + > > + if (p_board_dat == NULL) { > > + dev_err(&pdev->dev, > > + " %s memory allocation for private data failed\n", > > + __func__); > > + retval = -ENOMEM; > > + goto err_kmalloc; > > + } > > + > > + dev_dbg(&pdev->dev, > > + "%s memory allocation for private data success\n", __func__); > > + > > + /* enable PCI device */ > > + retval = pci_enable_device(pdev); > > + if (retval != 0) { > > + dev_err(&pdev->dev, "%s pci_enable_device FAILED\n", __func__); > > + > > + goto err_pci_en_device; > > + } > > + > > + dev_dbg(&pdev->dev, "%s pci_enable_device returned=%d\n", > > + __func__, retval); > > + > > + p_board_dat->pdev = pdev; > > + > > + /* alllocate memory for SPI master */ > > + for (i = 0, spi_alloc_num = 0; i < PCH_MAX_DEV; i++, spi_alloc_num++) { > > + p_master[i] = spi_alloc_master(&pdev->dev, > > + sizeof(struct pch_spi_data)); > > + > > + if (p_master[i] == NULL) { > > + retval = -ENOMEM; > > + dev_err(&pdev->dev, "%s [ch=%d]Fail.\n", __func__, i); > > + goto err_spi_alloc_master; > > + } > > + } > > + > > + dev_dbg(&pdev->dev, > > + "%s spi_alloc_master returned non NULL\n", __func__); > > + > > + /* initialize members of SPI master */ > > + for (j = 0, spi_master_num = 0; j < PCH_MAX_DEV; > > + j++, spi_master_num++) { > > spi_master_num isn't used except for the error path, and it always has > the same value as 'j'. Get it out of the loop initializer! :-) It > can be set in the error path. > > > + p_master[j]->bus_num = 0; > > You probably want -1 here so that the spi layer dynamically assigns an > spi bus number. > > > + p_master[j]->num_chipselect = PCH_MAX_CS; > > + p_master[j]->setup = pch_spi_setup; > > + dev_dbg(&pdev->dev, > > + "%s setup member of SPI master initialized\n", > > + __func__); > > + p_master[j]->transfer = pch_spi_transfer; > > + dev_dbg(&pdev->dev, > > + "%s transfer member of SPI master initialized\n", __func__); > > + p_master[j]->cleanup = pch_spi_cleanup; > > + dev_dbg(&pdev->dev, > > + "%s cleanup member of SPI master initialized\n", __func__); > > + > > + p_board_dat->p_data[j] = > > + spi_master_get_devdata(p_master[j]); > > Unnecessary line break. There are a lot of these in this driver. > > > + > > + p_board_dat->p_data[j]->p_master = p_master[j]; > > + p_board_dat->p_data[j]->n_curnt_chip = 255; > > + p_board_dat->p_data[j]->p_current_chip = NULL; > > + p_board_dat->p_data[j]->b_transfer_complete = false; > > + p_board_dat->p_data[j]->pkt_tx_buff = NULL; > > + p_board_dat->p_data[j]->pkt_rx_buff = NULL; > > + p_board_dat->p_data[j]->tx_index = 0; > > + p_board_dat->p_data[j]->rx_index = 0; > > + p_board_dat->p_data[j]->b_transfer_active = false; > > The above 7 lines can be dropped when the code is changed to use > kzalloc(). kzalloc() clears the allocated memory. > > > + p_board_dat->p_data[j]->p_board_dat = p_board_dat; > > + p_board_dat->suspend_sts = false; > > + > > + /* Register the controller with the SPI core. */ > > + retval = spi_register_master(p_master[j]); > > + if (retval != 0) { > > + dev_err(&pdev->dev, > > + "%s spi_register_master FAILED\n", __func__); > > + goto err_spi_reg_master; > > + } > > + > > + dev_dbg(&pdev->dev, "%s spi_register_master returned=%d\n", > > + __func__, retval); > > + } > > + > > + /* allocate resources for PCH SPI */ > > + retval = pch_spi_get_resources(p_board_dat); > > + if (retval != 0) { > > + dev_err(&pdev->dev, "%s fail(retval=%d)\n", > > + __func__, retval); > > + goto err_spi_get_resources; > > + } > > + > > + dev_dbg(&pdev->dev, "%s pch_spi_get_resources returned=%d\n", > > + __func__, retval); > > + > > + /* save private data in dev */ > > + pci_set_drvdata(pdev, (void *)p_board_dat); > > Remove the (void *) cast. It should not be necessary. > > > + dev_dbg(&pdev->dev, "%s invoked pci_set_drvdata\n", __func__); > > + > > + /* set master mode */ > > + for (k = 0; k < PCH_MAX_DEV; k++) { > > + pch_spi_set_master_mode(p_master[k]); > > + dev_dbg(&pdev->dev, > > + "%s invoked pch_spi_set_master_mode\n", __func__); > > + } > > + > > + return 0; > > + > > +err_spi_get_resources: > > + for (l = 0; l <= spi_master_num; l++) > > + spi_unregister_master(p_master[l]); > > +err_spi_reg_master: > > +err_spi_alloc_master: > > + for (m = 0; m <= spi_alloc_num; m++) > > + spi_master_put(p_master[m]); > > + pci_disable_device(pdev); > > +err_pci_en_device: > > + kfree(p_board_dat); > > +err_kmalloc: > > + return retval; > > +} > > + > > +static void pch_spi_remove(struct pci_dev *pdev) > > +{ > > + int i; > > + > > + struct pch_spi_board_data *p_board_dat = pci_get_drvdata(pdev); > > + > > + dev_dbg(&pdev->dev, "%s ENTRY\n", __func__); > > + > > + if (p_board_dat != NULL) { > > if (!p_board_dat) { > dev_err(...); > return; > } > > > + dev_dbg(&pdev->dev, "%s invoked pci_get_drvdata\n", __func__); > > + > > + /* check for any pending messages */ > > + if ((-EBUSY) == pch_spi_check_request_pending(p_board_dat)) { > > + dev_dbg(&pdev->dev, > > + "%s pch_spi_check_request_pending returned EBUSY\n", > > + __func__); > > + /* no need to take any particular action; > > + proceed with remove even > > + though queue is not empty */ > > + } > > + > > + dev_dbg(&pdev->dev, > > + "%s pch_spi_check_request_pending invoked\n", __func__); > > + > > + /* Free resources allocated for PCH SPI */ > > + pch_spi_free_resources(p_board_dat); > > + dev_dbg(&pdev->dev, > > + "%s invoked pch_spi_free_resources\n", __func__); > > + > > + /* Unregister SPI master */ > > + for (i = 0; i < PCH_MAX_DEV; i++) { > > + spi_unregister_master(p_board_dat->p_data[i]-> > > + p_master); > > + dev_dbg(&pdev->dev, > > + "%s invoked spi_unregister_master\n", __func__); > > + } > > + > > + /* free memory for private data */ > > + kfree(p_board_dat); > > + > > + pci_set_drvdata(pdev, NULL); > > + > > + dev_dbg(&pdev->dev, > > + "%s memory for private data freed\n", __func__); > > + > > + /* disable PCI device */ > > + pci_disable_device(pdev); > > + > > + dev_dbg(&pdev->dev, > > + "%s invoked pci_disable_device\n", __func__); > > + > > + } else { > > + dev_err(&pdev->dev, > > + "%s pci_get_drvdata returned NULL\n", __func__); > > + } > > +} > > + > > +#ifdef CONFIG_PM > > +static int pch_spi_suspend(struct pci_dev *pdev, pm_message_t state) > > +{ > > + int i; > > + u8 count; > > + s32 retval; > > + > > + struct pch_spi_board_data *p_board_dat = pci_get_drvdata(pdev); > > + > > + dev_dbg(&pdev->dev, "%s ENTRY\n", __func__); > > + > > + if (p_board_dat == NULL) { > > + dev_err(&pdev->dev, > > + "%s pci_get_drvdata returned NULL\n", __func__); > > + retval = -EFAULT; > > Ditto here; return -EFAULT; > > > + } else { > > + retval = 0; > > + dev_dbg(&pdev->dev, > > + "%s pci_get_drvdata invoked successfully\n", __func__); > > + p_board_dat->suspend_sts = true; > > + dev_dbg(&pdev->dev, > > + "%s p_board_dat->bSuspending set to true\n", __func__); > > + > > + /* check if the current message is processed: > > + Only after thats done the transfer will be suspended */ > > + for (i = 0; i < PCH_MAX_DEV; i++) { > > + count = 255; > > + > > + while ((--count) > 0) { > > + if (p_board_dat->p_data[i]-> > > + bcurrent_msg_processing == false) { > > + dev_dbg(&pdev->dev, "%s p_board_dat->" > > + "p_data->bCurrent_" > > + "msg_processing = false\n", > > + __func__); > > + break; > > + } else { > > + dev_dbg(&pdev->dev, "%s p_board_dat->" > > + "p_data->bCurrent_msg_" > > + "processing = true\n", > > + __func__); > > + } > > + msleep(PCH_SLEEP_TIME); > > + } > > + } > > + > > + /* Free IRQ */ > > + if (p_board_dat->irq_reg_sts == true) { > > + /* disable all interrupts */ > > + for (i = 0; i < PCH_MAX_DEV; i++) { > > + pch_spi_disable_interrupts(p_board_dat-> > > + p_data[i]-> > > + p_master, > > + PCH_ALL); > > + pch_spi_reset(p_board_dat->p_data[i]-> > > + p_master); > > + dev_dbg(&pdev->dev, > > + "%s pch_spi_disable_interrupts invoked" > > + "successfully\n", __func__); > > + } > > + > > + free_irq(p_board_dat->pdev->irq, (void *)p_board_dat); > > + > > + p_board_dat->irq_reg_sts = false; > > + dev_dbg(&pdev->dev, > > + "%s free_irq invoked successfully." > > + "p_data->irq_reg_sts = false\n", > > + __func__); > > + } > > + > > + /* save config space */ > > + retval = pci_save_state(pdev); > > + > > + if (retval == 0) { > > + dev_dbg(&pdev->dev, "%s pci_save_state returned=%d\n", > > + __func__, retval); > > + /* disable PM notifications */ > > + pci_enable_wake(pdev, PCI_D3hot, 0); > > + dev_dbg(&pdev->dev, > > + "%s pci_enable_wake invoked successfully\n", > > + __func__); > > + /* disable PCI device */ > > + pci_disable_device(pdev); > > + dev_dbg(&pdev->dev, > > + "%s pci_disable_device invoked successfully\n", > > + __func__); > > + /* move device to D3hot state */ > > + pci_set_power_state(pdev, PCI_D3hot); > > + dev_dbg(&pdev->dev, > > + "%s pci_set_power_state invoked successfully\n", > > + __func__); > > + } else { > > + dev_err(&pdev->dev, > > + "%s pci_save_state failed\n", __func__); > > + } > > + } > > + > > + dev_dbg(&pdev->dev, "%s return=%d\n", __func__, retval); > > + > > + return retval; > > +} > > + > > +static int pch_spi_resume(struct pci_dev *pdev) > > +{ > > + int i; > > + s32 retval; > > + > > + struct pch_spi_board_data *p_board_dat = pci_get_drvdata(pdev); > > + dev_dbg(&pdev->dev, "%s ENTRY\n", __func__); > > + > > + if (p_board_dat == NULL) { > > + dev_err(&pdev->dev, > > + "%s pci_get_drvdata returned NULL\n", __func__); > > + retval = -EFAULT; > > + } else { > > + retval = 0; > > + /* move device to DO power state */ > > + pci_set_power_state(pdev, PCI_D0); > > + dev_dbg(&pdev->dev, > > + "%s pci_set_power_state invoked successfully\n", __func__); > > + > > + /* restore state */ > > + pci_restore_state(pdev); > > + dev_dbg(&pdev->dev, > > + "%s pci_restore_state invoked successfully\n", __func__); > > + > > + retval = pci_enable_device(pdev); > > + if (retval < 0) { > > + dev_err(&pdev->dev, > > + "%s pci_enable_device failed\n", __func__); > > + } else { > > + dev_dbg(&pdev->dev, > > + "%s pci_enable_device returned=%d\n", > > + __func__, retval); > > + > > + /* disable PM notifications */ > > + pci_enable_wake(pdev, PCI_D3hot, 0); > > + dev_dbg(&pdev->dev, > > + "%s pci_enable_wake invoked successfully\n", __func__); > > + > > + /* register IRQ handler */ > > + if ((p_board_dat->irq_reg_sts) != true) { > > + /* register IRQ */ > > + retval = request_irq(p_board_dat->pdev->irq, > > + pch_spi_handler, IRQF_SHARED, > > + DRIVER_NAME, > > + p_board_dat); > > + if (retval < 0) { > > + dev_err(&pdev->dev, > > + "%s request_irq failed\n", __func__); > > + } else { > > + dev_dbg(&pdev->dev, "%s request_irq" > > + " returned=%d\n", __func__, retval); > > + p_board_dat->irq_reg_sts = true; > > + > > + /* reset PCH SPI h/w */ > > + for (i = 0; i < PCH_MAX_DEV; i++) { > > + pch_spi_reset(p_board_dat-> > > + p_data[i]-> > > + p_master); > > + dev_dbg(&pdev->dev, > > + "pdev->dev, %s pch_spi_reset " > > + "invoked successfully\n", > > + __func__); > > + pch_spi_set_master_mode > > + (p_board_dat->p_data[i]-> > > + p_master); > > + dev_dbg(&pdev->dev, > > + "%s pch_spi_set_master_mode " > > + "invoked successfully\n", > > + __func__); > > + } > > + > > + /* set suspend status to false */ > > + p_board_dat->suspend_sts = false; > > + > > + dev_dbg(&pdev->dev, "%s set p_board_dat" > > + "->bSuspending = false\n", __func__); > > + } > > + } > > + } > > + } > > Another very deeply indented function. > > > + > > + dev_dbg(&pdev->dev, "%s returning=%d\n", __func__, retval); > > + > > + return retval; > > +} > > +#else > > +#define pch_spi_suspend NULL > > +#define pch_spi_resume NULL > > + > > +#endif > > + > > +static struct pci_driver pch_spi_pcidev = { > > + .name = "pch_spi", > > + .id_table = pch_spi_pcidev_id, > > + .probe = pch_spi_probe, > > + .remove = pch_spi_remove, > > + .suspend = pch_spi_suspend, > > + .resume = pch_spi_resume, > > +}; > > + > > +static int __init pch_spi_init(void) > > +{ > > + return pci_register_driver(&pch_spi_pcidev); > > +} > > + > > + > > +static void __exit pch_spi_exit(void) > > +{ > > + pci_unregister_driver(&pch_spi_pcidev); > > +} > > + > > +MODULE_LICENSE("GPL"); > > +MODULE_DESCRIPTION("PCH SPI PCI Driver"); > > +module_init(pch_spi_init); > > +module_exit(pch_spi_exit); > > + > > diff --git a/drivers/spi/pch_spi_platform_devices.c > > b/drivers/spi/pch_spi_platform_devices.c > > new file mode 100644 > > index 0000000..a0d6488 > > --- /dev/null > > +++ b/drivers/spi/pch_spi_platform_devices.c > > @@ -0,0 +1,56 @@ > > +/* > > + * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD. > > + * > > + * 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; version 2 of the License. > > + * > > + * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307, > > USA. > > + */ > > + > > +#include <linux/module.h> > > +#include <linux/spi/spidev.h> > > +#include <linux/device.h> > > +#include <linux/spi/spi.h> > > + > > +static struct spi_board_info pch_spi_slaves[] = { > > + { > > + .modalias = "spidev", > > + .max_speed_hz = 1000000, > > + .bus_num = 0, > > + .chip_select = 0, > > + .platform_data = NULL, > > + .mode = SPI_MODE_0, > > + }, > > +#if (CONFIG_PCH_SPI_PLATFORM_DEVICE_COUNT - 1) > > + { > > + .modalias = "spidev", > > + .max_speed_hz = 1000000, > > + .bus_num = 1, > > + .chip_select = 1, > > + .platform_data = NULL, > > + .mode = SPI_MODE_0, > > + }, > > +#endif > > +}; > > This file is misnamed; these aren't platform_devices, they are spi_devices. > > > + > > +static __init int Load(void) > > +{ > > + int iRetVal = -1; > > int rc = -1; > > > + > > + if (!spi_register_board_info > > + (pch_spi_slaves, ARRAY_SIZE(pch_spi_slaves))) > > + iRetVal = 0; > > + > > + return iRetVal; > > +} > > + > > +module_init(Load); > > How about when the module is removed? > > This particular module (not the rest of the driver though) is really > rather a hack. It seems like you need a method for creating spidev > devices from userspace at runtime, or having a method for specifying > spi device from the kernel command line. Using a flattened device > tree fragment would work as well. > > g. > > > -- > Grant Likely, B.Sc., P.Eng. > Secret Lab Technologies Ltd. >


linux-kernel:
Ingo Molnar	Re: [PATCH 0/3] v2 Make hierarchical RCU less IPI-happy and add more tracing
Jeremy Fitzhardinge	Re: Linux 2.6.28.10 and Linux 2.6.29.6 XEN Guest Support Broken x86_64 in BUILD
Nick Piggin	Re: [patch] CFS (Completely Fair Scheduler), v2
Gary Hade	Re: [PATCH 0/5][RFC] Physical PCI slot objects
Dave Johnson	Re: expected behavior of PF_PACKET on NETIF_F_HW_VLAN_RX device?
linux-netdev:
Arnd Bergmann	Re: 64-bit net_device_stats
Stephens, Allan	RE: [PATCH]: tipc: Fix oops on send prior to entering networked mode
frank.blaschka	[patch 3/5] [PATCH] qeth: support z/VM VSWITCH Port Isolation
Wu Fengguang	Re: [PATCH] dm9601: handle corrupt mac address
David Miller	Re: [PATCH net-2.6.24] Fix refcounting problem with netif_rx_reschedule()
git:
Junio C Hamano	Re: [PATCH] [RFC] add Message-ID field to log on git-am operation
Junio C Hamano	Re: Handling large files with GIT
Karl	Re: [ANNOUNCE] pg - A patch porcelain for GIT
Josh Triplett	Re: [RFC][PATCH 00/10] Sparse: Git's "make check" target
Pierre Habouzit	Re: [PATCH] git-daemon: more powerful base-path/user-path settings, using formats.
git-commits-head:
Linux Kernel Mailing List	MIPS: RBTX4939: Fix IOC pin-enable register updating
Linux Kernel Mailing List	regulator: update email address for Liam Girdwood
Linux Kernel Mailing List	[SCSI] ipr: add message to error table
Linux Kernel Mailing List	powerpc/32: Wire up the trampoline code for kdump
Linux Kernel Mailing List	USB: omap_udc: sync with OMAP tree
openbsd-misc:
Josh Grosse	Re: error : pkg add phpMyAdmin
Brian Candler	Re: OBSD's perspective on SELinux
Jacob Meuser	Re: /dev/audio: Device busy
David Vasek	Re: Inexpensive, low power, "wall wart" computer
William Boshuck	Re: Richard Stallman...