Re: [RFC] SPI core -- revisited

Alexey Dobriyan
2005-06-26T20:43:22Z

On Sunday 26 June 2005 23:36, dpervushin@ru.mvista.com wrote:
> Here is the sample of usage of our SPI core; this is the driver of atmel
> chip connected to the pnx spi bus;

> --- /dev/null
> +++ linux-2.6.10/drivers/spi/spi-pnx010x.c

> +#define DBG(args...) pr_debug(args)

Just use pr_debug()

> +static void *phys_spi_data_reg = 0;

NULL for pointers.

> +void spipnx_select_chip (void)
> +{
> + unsigned reg = gpio_read_reg (PADMUX1_MODE0);
> + gpio_write_reg ((reg & ~GPIO_PIN_SPI_CE), PADMUX1_MODE0); }

Closing brackets on the separate line, _please_.

> +static void spipnx_spi_init (void *algo_data) {

As well as opening ones.

> --- /dev/null
> +++ linux-2.6.10/drivers/spi/spi-pnx010x_atmel.c

> +#define DBG(args...) pr_debug(args)

> +static struct spi_algorithm spipnx_algorithm = {
> + name: ALGORITHM_NAME,
> + xfer: spipnx_xfer,

.name = ALGORITHM_NAME,
.xfer = ...

> +static spi_pnx0105_algo_data_t spi_pnx_algo_data = {
> + cb_data: NULL,
> + chip_cs_callback: NULL,
> +};

> +static struct spi_adapter spipnx_adapter = {
> + name: ADAPTER_NAME,
> + algo: &spipnx_algorithm,
> + algo_data: (void *) &spi_pnx_algo_data,
> + owner: THIS_MODULE,
> + alloc: NULL,
> + free: NULL,
> + copy_from_user: NULL,
> + copy_to_user: NULL,
> +};

> +void *pnx_memory_alloc(size_t size, int base) {
> + spi_pnx_msg_buff_t *buff;
> +
> + buff = (spi_pnx_msg_buff_t *) kmalloc(sizeof(spi_pnx_msg_buff_t),

Useless cast.

> base);
> + buff->size = size;
> + buff->io_buffer = dma_alloc_coherent (NULL, size, &buff->dma_buffer,
>
> +base);
> +
> + DBG("%s:allocated memory(%x) for io_buffer = %x dma_buffer = %x\n",
> + __FUNCTION__, buff, buff->io_buffer,buff->dma_buffer );
> +
> + return (void *) buff;
> +}

No error path. You're an optimist.

> +void pnx_memory_free(const void *data)
> +{
> + spi_pnx_msg_buff_t *buff;
> +
> + buff = (spi_pnx_msg_buff_t *) data;
> + DBG("%s:deleted memory(%x) for io_buffer = %x dma_buffer = %x\n",
> + __FUNCTION__, buff, buff->io_buffer,buff->dma_buffer );

%p was invented for pointers.

> +unsigned long pnx_copy_from_user(void *to, const void *from_user,

const void __user *from, I believe.

> +unsigned long pnx_copy_to_user(void *to_user, const void *from,

const void __user *to.