[PATCH] spi: mt7621: Move SPI driver out of staging

[Mark Brown]

On Thu, Mar 14, 2019 at 12:52:12PM +0100, Stefan Roese wrote:

This looks pretty good, a few trivial issues below but nothing major I
think.

> +config SPI_MT7621
> +	tristate "MediaTek MT7621 SPI Controller"
> +	depends on RALINK
> +	help
> +	  This selects a driver for the MediaTek MT7621 SPI Controller.
> +

I'm not seeing any build time dependency on this, please add an ||
COMPILE_TEST to help with build testing.

> @@ -0,0 +1,422 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * spi-mt7621.c -- MediaTek MT7621 SPI controller driver
> + *

Please make the entire comment a C++ one to 

> + * Some parts are based on spi-orion.c:
> + *   Author: Shadi Ammouri <shadi at marvell.com>
> + *   Copyright (C) 2007-2008 Marvell Ltd.

That driver dates from 2008 AFAICT, and had some changes after then?

> +static void mt7621_spi_set_cs(struct spi_device *spi, int enable)
> +{
> +	struct mt7621_spi *rs = spidev_to_mt7621_spi(spi);
> +	int cs = spi->chip_select;
> +	u32 polar = 0;
> +
> +	mt7621_spi_reset(rs);
> +	if (enable)
> +		polar = BIT(cs);
> +	mt7621_spi_write(rs, MT7621_SPI_POLAR, polar);
> +}

Will doing a reset mess up the rate configuration and stuff that's done
in _prepare(), or is _reset() perhaps not an entirely accurate name?

> +	list_for_each_entry(t, &m->transfers, transfer_list)
> +		if (t->speed_hz < speed)
> +			speed = t->speed_hz;

This should be in the core if it's needed, it's going to be the same for
all drivers.

> +	master->setup = mt7621_spi_setup;
> +	master->transfer_one_message = mt7621_spi_transfer_one_message;
> +	master->bits_per_word_mask = SPI_BPW_MASK(8);
> +	master->dev.of_node = pdev->dev.of_node;
> +	master->num_chipselect = 2;

The driver should set SPI_CONTROLLER_HALF_DUPLEX in flags.

> +	clk_disable(rs->clk);

This needs to be clk_disable_unprepare() to ensure the clock is
unprepared as well as disabled.

> +	spi_unregister_master(master);

Just use devm_spi_register_controller()?  The _master() name is legacy
and devm_ would make life easier.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/attachments/20190321/3f007621/attachment.asc>