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@xxxxxxxxxxx>
> + * 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.
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
