| Message ID | 20608368-44e6-21f8-d970-5ade9990ce59@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Oct 02, 2016 at 02:22:48PM +0200, Heiner Kallweit wrote: > SPI_QE_CPU_MODE doesn't exist for ESPI and is set by of_mpc8xxx_spi_probe > based on DT property "mode". This property is not defined for ESPI, > see Documentation/devicetree/bindings/spi/fsl-spi.txt > Therefore remove the check. Shouldn't we be warning if we encounter a DT that has this set since it probably indicates that something went wrong?
Am 06.10.2016 um 17:45 schrieb Mark Brown: > On Sun, Oct 02, 2016 at 02:22:48PM +0200, Heiner Kallweit wrote: >> SPI_QE_CPU_MODE doesn't exist for ESPI and is set by of_mpc8xxx_spi_probe >> based on DT property "mode". This property is not defined for ESPI, >> see Documentation/devicetree/bindings/spi/fsl-spi.txt >> Therefore remove the check. > > Shouldn't we be warning if we encounter a DT that has this set since it > probably indicates that something went wrong? > The values of mpc8xxx_spi->rx_shift and mpc8xxx_spi->tx_shift were overwritten in fsl_espi_setup_transfer anyway. Therefore the removed code effectively was dead code. As we just remove dead code and don't change the behavior of the driver I see no need to introduce an additional check / warning. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 06, 2016 at 08:36:29PM +0200, Heiner Kallweit wrote: > Am 06.10.2016 um 17:45 schrieb Mark Brown: > > Shouldn't we be warning if we encounter a DT that has this set since it > > probably indicates that something went wrong? > The values of mpc8xxx_spi->rx_shift and mpc8xxx_spi->tx_shift were > overwritten in fsl_espi_setup_transfer anyway. Therefore the removed > code effectively was dead code. > As we just remove dead code and don't change the behavior of the driver > I see no need to introduce an additional check / warning. There's still the potential to either uncover an existing bug that only worked by accident or to catch some future bug that someone introduces. The existing code may have problems but that doesn't mean it's not a good idea to do a good job now.
diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c index 65bb70d..8281aea1 100644 --- a/drivers/spi/spi-fsl-espi.c +++ b/drivers/spi/spi-fsl-espi.c @@ -594,11 +594,6 @@ static int fsl_espi_probe(struct device *dev, struct resource *mem, if (ret) goto err_probe; - if (mpc8xxx_spi->flags & SPI_QE_CPU_MODE) { - mpc8xxx_spi->rx_shift = 16; - mpc8xxx_spi->tx_shift = 24; - } - /* SPI controller initializations */ fsl_espi_write_reg(mpc8xxx_spi, ESPI_SPMODE, 0); fsl_espi_write_reg(mpc8xxx_spi, ESPI_SPIM, 0);
SPI_QE_CPU_MODE doesn't exist for ESPI and is set by of_mpc8xxx_spi_probe based on DT property "mode". This property is not defined for ESPI, see Documentation/devicetree/bindings/spi/fsl-spi.txt Therefore remove the check. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/spi/spi-fsl-espi.c | 5 ----- 1 file changed, 5 deletions(-)