Re: [RFC] SPI core -- revisited

[Alexey Dobriyan <adobriyan@gmail.com>]

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.