| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <36328086-961f-c84d-d8df-acbe96db3261@csgroup.eu>
Date: Wed, 7 Sep 2022 17:02:41 +0000
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Mark Brown <broonie@...nel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
"linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>
Subject: Re: [RFC PATCH v1] spi: fsl_spi: Convert to transfer_one
Le 18/08/2022 à 15:38, Christophe Leroy a écrit :
> Let the core handle all the chipselect bakery and replace
> transfer_one_message() by transfer_one() and prepare_message().
>
> At the time being, there is fsl_spi_cs_control() to handle
> chipselects. That function handles both GPIO and non-GPIO
> chipselects. The GPIO chipselects will now be handled by
> the core directly, so only handle non-GPIO chipselects and
> hook it to ->set_cs
Any comment for/about this conversion ?
Did I do it the right way ? Any recommendation ?
Thanks
Christophe
>
> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
> ---
> Sending as an RFC as I'm not 100% sure of the correctness.
> I successfully tested it on the hardware I have though.
> Not sure about the change from m->is_dma_mapped to !!t->tx_dma || !!t->rx_dma
> ---
> drivers/spi/spi-fsl-spi.c | 157 +++++++++++---------------------------
> 1 file changed, 43 insertions(+), 114 deletions(-)
>
> diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c
> index bdf94cc7be1a..731624f157fc 100644
> --- a/drivers/spi/spi-fsl-spi.c
> +++ b/drivers/spi/spi-fsl-spi.c
> @@ -111,32 +111,6 @@ static void fsl_spi_change_mode(struct spi_device *spi)
> local_irq_restore(flags);
> }
>
> -static void fsl_spi_chipselect(struct spi_device *spi, int value)
> -{
> - struct mpc8xxx_spi *mpc8xxx_spi = spi_master_get_devdata(spi->master);
> - struct fsl_spi_platform_data *pdata;
> - struct spi_mpc8xxx_cs *cs = spi->controller_state;
> -
> - pdata = spi->dev.parent->parent->platform_data;
> -
> - if (value == BITBANG_CS_INACTIVE) {
> - if (pdata->cs_control)
> - pdata->cs_control(spi, false);
> - }
> -
> - if (value == BITBANG_CS_ACTIVE) {
> - mpc8xxx_spi->rx_shift = cs->rx_shift;
> - mpc8xxx_spi->tx_shift = cs->tx_shift;
> - mpc8xxx_spi->get_rx = cs->get_rx;
> - mpc8xxx_spi->get_tx = cs->get_tx;
> -
> - fsl_spi_change_mode(spi);
> -
> - if (pdata->cs_control)
> - pdata->cs_control(spi, true);
> - }
> -}
> -
> static void fsl_spi_qe_cpu_set_shifts(u32 *rx_shift, u32 *tx_shift,
> int bits_per_word, int msb_first)
> {
> @@ -354,15 +328,11 @@ static int fsl_spi_bufs(struct spi_device *spi, struct spi_transfer *t,
> return mpc8xxx_spi->count;
> }
>
> -static int fsl_spi_do_one_msg(struct spi_master *master,
> - struct spi_message *m)
> +static int fsl_spi_prepare_message(struct spi_controller *ctlr,
> + struct spi_message *m)
> {
> - struct mpc8xxx_spi *mpc8xxx_spi = spi_master_get_devdata(master);
> - struct spi_device *spi = m->spi;
> - struct spi_transfer *t, *first;
> - unsigned int cs_change;
> - const int nsecs = 50;
> - int status, last_bpw;
> + struct mpc8xxx_spi *mpc8xxx_spi = spi_controller_get_devdata(ctlr);
> + struct spi_transfer *t;
>
> /*
> * In CPU mode, optimize large byte transfers to use larger
> @@ -378,62 +348,30 @@ static int fsl_spi_do_one_msg(struct spi_master *master,
> t->bits_per_word = 16;
> }
> }
> + return 0;
> +}
>
> - /* Don't allow changes if CS is active */
> - cs_change = 1;
> - list_for_each_entry(t, &m->transfers, transfer_list) {
> - if (cs_change)
> - first = t;
> - cs_change = t->cs_change;
> - if (first->speed_hz != t->speed_hz) {
> - dev_err(&spi->dev,
> - "speed_hz cannot change while CS is active\n");
> - return -EINVAL;
> - }
> - }
> -
> - last_bpw = -1;
> - cs_change = 1;
> - status = -EINVAL;
> - list_for_each_entry(t, &m->transfers, transfer_list) {
> - if (cs_change || last_bpw != t->bits_per_word)
> - status = fsl_spi_setup_transfer(spi, t);
> - if (status < 0)
> - break;
> - last_bpw = t->bits_per_word;
> -
> - if (cs_change) {
> - fsl_spi_chipselect(spi, BITBANG_CS_ACTIVE);
> - ndelay(nsecs);
> - }
> - cs_change = t->cs_change;
> - if (t->len)
> - status = fsl_spi_bufs(spi, t, m->is_dma_mapped);
> - if (status) {
> - status = -EMSGSIZE;
> - break;
> - }
> - m->actual_length += t->len;
> -
> - spi_transfer_delay_exec(t);
> -
> - if (cs_change) {
> - ndelay(nsecs);
> - fsl_spi_chipselect(spi, BITBANG_CS_INACTIVE);
> - ndelay(nsecs);
> - }
> - }
> +static int fsl_spi_transfer_one(struct spi_controller *controller,
> + struct spi_device *spi,
> + struct spi_transfer *t)
> +{
> + int status;
>
> - m->status = status;
> + status = fsl_spi_setup_transfer(spi, t);
> + if (status < 0)
> + return status;
> + if (t->len)
> + status = fsl_spi_bufs(spi, t, !!t->tx_dma || !!t->rx_dma);
> + if (status > 0)
> + return -EMSGSIZE;
>
> - if (status || !cs_change) {
> - ndelay(nsecs);
> - fsl_spi_chipselect(spi, BITBANG_CS_INACTIVE);
> - }
> + return status;
> +}
>
> - fsl_spi_setup_transfer(spi, NULL);
> - spi_finalize_current_message(master);
> - return 0;
> +static int fsl_spi_unprepare_message(struct spi_controller *controller,
> + struct spi_message *msg)
> +{
> + return fsl_spi_setup_transfer(msg->spi, NULL);
> }
>
> static int fsl_spi_setup(struct spi_device *spi)
> @@ -482,9 +420,6 @@ static int fsl_spi_setup(struct spi_device *spi)
> return retval;
> }
>
> - /* Initialize chipselect - might be active for SPI_CS_HIGH mode */
> - fsl_spi_chipselect(spi, BITBANG_CS_INACTIVE);
> -
> return 0;
> }
>
> @@ -557,9 +492,7 @@ static void fsl_spi_grlib_cs_control(struct spi_device *spi, bool on)
> u32 slvsel;
> u16 cs = spi->chip_select;
>
> - if (spi->cs_gpiod) {
> - gpiod_set_value(spi->cs_gpiod, on);
> - } else if (cs < mpc8xxx_spi->native_chipselects) {
> + if (cs < mpc8xxx_spi->native_chipselects) {
> slvsel = mpc8xxx_spi_read_reg(®_base->slvsel);
> slvsel = on ? (slvsel | (1 << cs)) : (slvsel & ~(1 << cs));
> mpc8xxx_spi_write_reg(®_base->slvsel, slvsel);
> @@ -568,7 +501,6 @@ static void fsl_spi_grlib_cs_control(struct spi_device *spi, bool on)
>
> static void fsl_spi_grlib_probe(struct device *dev)
> {
> - struct fsl_spi_platform_data *pdata = dev_get_platdata(dev);
> struct spi_master *master = dev_get_drvdata(dev);
> struct mpc8xxx_spi *mpc8xxx_spi = spi_master_get_devdata(master);
> struct fsl_spi_reg __iomem *reg_base = mpc8xxx_spi->reg_base;
> @@ -588,7 +520,18 @@ static void fsl_spi_grlib_probe(struct device *dev)
> mpc8xxx_spi_write_reg(®_base->slvsel, 0xffffffff);
> }
> master->num_chipselect = mpc8xxx_spi->native_chipselects;
> - pdata->cs_control = fsl_spi_grlib_cs_control;
> + master->set_cs = fsl_spi_grlib_cs_control;
> +}
> +
> +static void fsl_spi_cs_control(struct spi_device *spi, bool on)
> +{
> + struct device *dev = spi->dev.parent->parent;
> + struct fsl_spi_platform_data *pdata = dev_get_platdata(dev);
> + struct mpc8xxx_spi_probe_info *pinfo = to_of_pinfo(pdata);
> +
> + if (WARN_ON_ONCE(!pinfo->immr_spi_cs))
> + return;
> + iowrite32be(on ? 0 : SPI_BOOT_SEL_BIT, pinfo->immr_spi_cs);
> }
>
> static struct spi_master *fsl_spi_probe(struct device *dev,
> @@ -613,8 +556,11 @@ static struct spi_master *fsl_spi_probe(struct device *dev,
>
> master->setup = fsl_spi_setup;
> master->cleanup = fsl_spi_cleanup;
> - master->transfer_one_message = fsl_spi_do_one_msg;
> + master->prepare_message = fsl_spi_prepare_message;
> + master->transfer_one = fsl_spi_transfer_one;
> + master->unprepare_message = fsl_spi_unprepare_message;
> master->use_gpio_descriptors = true;
> + master->set_cs = fsl_spi_cs_control;
>
> mpc8xxx_spi = spi_master_get_devdata(master);
> mpc8xxx_spi->max_bits_per_word = 32;
> @@ -688,21 +634,6 @@ static struct spi_master *fsl_spi_probe(struct device *dev,
> return ERR_PTR(ret);
> }
>
> -static void fsl_spi_cs_control(struct spi_device *spi, bool on)
> -{
> - if (spi->cs_gpiod) {
> - gpiod_set_value(spi->cs_gpiod, on);
> - } else {
> - struct device *dev = spi->dev.parent->parent;
> - struct fsl_spi_platform_data *pdata = dev_get_platdata(dev);
> - struct mpc8xxx_spi_probe_info *pinfo = to_of_pinfo(pdata);
> -
> - if (WARN_ON_ONCE(!pinfo->immr_spi_cs))
> - return;
> - iowrite32be(on ? 0 : SPI_BOOT_SEL_BIT, pinfo->immr_spi_cs);
> - }
> -}
> -
> static int of_fsl_spi_probe(struct platform_device *ofdev)
> {
> struct device *dev = &ofdev->dev;
> @@ -744,12 +675,10 @@ static int of_fsl_spi_probe(struct platform_device *ofdev)
> ret = gpiod_count(dev, "cs");
> if (ret < 0)
> ret = 0;
> - if (ret == 0 && !spisel_boot) {
> + if (ret == 0 && !spisel_boot)
> pdata->max_chipselect = 1;
> - } else {
> + else
> pdata->max_chipselect = ret + spisel_boot;
> - pdata->cs_control = fsl_spi_cs_control;
> - }
> }
>
> ret = of_address_to_resource(np, 0, &mem);
Powered by blists - more mailing lists