Re: [PATCH 5/6] spi/bcm63xx: move register definitions into the driver

[Florian Fainelli <florian@xxxxxxxxxxx>]

On 10/09/15 07:11, Jonas Gorski wrote:
> Move all register definitions and structs into the driver. This allows
> us dropping the platform_data struct and drop any arch specific
> includes.
> 
> Since we now have full control over the message width, we can drop the
> size check, which was broken anyway, since it never set ret to any error
> code.
> 
> Also since we now have no arch depedendent resources, we can now allow
> compiling it for any arch, hidding behind COMPILE_TEST.
> 
> Signed-off-by: Jonas Gorski <jogo@xxxxxxxxxxx>
> ---

[snip]

> +	switch (resource_size(r)) {
> +	case SPI_6348_RSET_SIZE:
> +		bs->reg_offsets = bcm6348_spi_reg_offsets;
> +		bs->msg_type_shift = SPI_6348_MSG_TYPE_SHIFT;
> +		bs->msg_ctl_width = SPI_6348_MSG_CTL_WIDTH;
> +		bs->fifo_size = SPI_6348_MSG_DATA_SIZE;
> +		break;
> +	case SPI_6358_RSET_SIZE:
> +		bs->reg_offsets = bcm6358_spi_reg_offsets;
> +		bs->msg_type_shift = SPI_6358_MSG_TYPE_SHIFT;
> +		bs->msg_ctl_width = SPI_6358_MSG_CTL_WIDTH;
> +		bs->fifo_size = SPI_6358_MSG_DATA_SIZE;
>  		break;

This is a little fragile, I would rather create more specialized
platform_id names, like "bcm6348-spi" and "bcm6358-spi", very much like
what the FEC driver does, such that:

- you could directly pass this information as part of an additional
structure which specializes the instance of the driver
- the conversion to Device Tree in your other patches would make this
more natural or nearly identical

What do you think?
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html