* [PATCH v0] spi: spi-rockchip spi slave mode @ 2020-05-08 8:37 Chris Ruehl 2020-05-08 8:37 ` [PATCH v0 1/1] spi: spi-rockchip: add support for spi slave_mode Chris Ruehl 0 siblings, 1 reply; 7+ messages in thread From: Chris Ruehl @ 2020-05-08 8:37 UTC (permalink / raw) To: Chris Ruehl, Jack Lo Cc: Mark Brown, Heiko Stuebner, linux-spi, linux-arm-kernel, linux-rockchip, linux-kernel The driver spi-rockchip does not support spi slave mode, but the register map has an entry indicate that the chip support it. An example implementation found here: https://dev.t-firefly.com/thread-101485-1-1.html The patch is my first approach to support slave mode which is needed in one of our projects, the PCBA is not yet available but we think to have it for testing very soon. Yes, the code in the patch isn't tested yet. I found it odd, that the num_chipselect is set fixed to the amount of native chip-select lines rather use the max_native_cs. Changed it. - master->num_chipselect = ROCKCHIP_SPI_MAX_CS_NUM; + of_property_read_u32(np, "num-cs", &num_cs); + master->num_chipselect = num_cs; + master->max_native_cs = ROCKCHIP_SPI_MAX_CS_NUM; That ask to enable cs_gpiods, and support gpio cs + master->use_gpio_descriptors = true; Patch against next-20200505 Thanks for review! Happy hacking Chris Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk> --- ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v0 1/1] spi: spi-rockchip: add support for spi slave_mode 2020-05-08 8:37 [PATCH v0] spi: spi-rockchip spi slave mode Chris Ruehl @ 2020-05-08 8:37 ` Chris Ruehl 2020-05-08 13:13 ` Emil Renner Berthing 0 siblings, 1 reply; 7+ messages in thread From: Chris Ruehl @ 2020-05-08 8:37 UTC (permalink / raw) To: Chris Ruehl, Jack Lo Cc: Mark Brown, Heiko Stuebner, linux-spi, linux-arm-kernel, linux-rockchip, linux-kernel This patch aim to add spi slave mode support to the rockchip driver. Fix the wrong usage of num_cs set fix to ROCKCHIP_SPI_MAX_CS_NUM, instead use max_native_cs flag to set the limit of native chip-select. Enable use_gpio_descriptors to have cs_gpiod for gpio based chip-selects. Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk> --- drivers/spi/spi-rockchip.c | 46 +++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c index 70ef63e0b6b8..9c1ff52c0f85 100644 --- a/drivers/spi/spi-rockchip.c +++ b/drivers/spi/spi-rockchip.c @@ -183,6 +183,9 @@ struct rockchip_spi { u8 rsd; bool cs_asserted[ROCKCHIP_SPI_MAX_CS_NUM]; + + bool slave_mode; + bool slave_abort; }; static inline void spi_enable_chip(struct rockchip_spi *rs, bool enable) @@ -359,7 +362,7 @@ static void rockchip_spi_dma_rxcb(void *data) struct rockchip_spi *rs = spi_master_get_devdata(master); int state = atomic_fetch_andnot(RXDMA, &rs->state); - if (state & TXDMA) + if (state & TXDMA && !rs->slave_abort) return; spi_enable_chip(rs, false); @@ -372,7 +375,7 @@ static void rockchip_spi_dma_txcb(void *data) struct rockchip_spi *rs = spi_master_get_devdata(master); int state = atomic_fetch_andnot(TXDMA, &rs->state); - if (state & RXDMA) + if (state & RXDMA && !rs->slave_abort) return; /* Wait until the FIFO data completely. */ @@ -466,6 +469,10 @@ static void rockchip_spi_config(struct rockchip_spi *rs, u32 cr1; u32 dmacr = 0; + if (rs->slavemode) + cr0 |= CR0_OPM_SLAVE << CR0_OPM_OFFSET; + rs->slave_abort = false; + cr0 |= rs->rsd << CR0_RSD_OFFSET; cr0 |= (spi->mode & 0x3U) << CR0_SCPH_OFFSET; if (spi->mode & SPI_LSB_FIRST) @@ -535,6 +542,16 @@ static size_t rockchip_spi_max_transfer_size(struct spi_device *spi) return ROCKCHIP_SPI_MAX_TRANLEN; } +static int rockchip_spi_slave_abort(struct spi_master *master) +{ + struct rockchip_spi *rs = spi_master_get_devdata(master); + + rs->slave_abort = true; + complete(master); + + return 0; +} + static int rockchip_spi_transfer_one( struct spi_master *master, struct spi_device *spi, @@ -589,14 +606,25 @@ static int rockchip_spi_probe(struct platform_device *pdev) struct spi_master *master; struct resource *mem; u32 rsd_nsecs; + bool slave_mode; + u32 num_cs = 1; + + slave_mode = of_property_read_bool(np, "spi-slave"); + + if (slave_mode) + master = spi_alloc_slave(&pdev->dev, + sizeof(struct rockchip_spi)); + else + master = spi_alloc_master(&pdev->dev, + sizeof(struct rockchip_spi)); - master = spi_alloc_master(&pdev->dev, sizeof(struct rockchip_spi)); if (!master) return -ENOMEM; platform_set_drvdata(pdev, master); rs = spi_master_get_devdata(master); + rs->slave_mode = slave_mode; /* Get basic io resource and map it */ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); @@ -676,7 +704,16 @@ static int rockchip_spi_probe(struct platform_device *pdev) master->auto_runtime_pm = true; master->bus_num = pdev->id; master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP | SPI_LSB_FIRST; - master->num_chipselect = ROCKCHIP_SPI_MAX_CS_NUM; + if (slave_mode) { + master->mode_bits |= SPI_NO_CS; + master->slave_abort = rockchip_spi_slave_abort; + } else { + of_property_read_u32(np, "num-cs", &num_cs); + master->num_chipselect = num_cs; + master->use_gpio_descriptors = true; + master->max_native_cs = ROCKCHIP_SPI_MAX_CS_NUM; + master->flags = SPI_MASTER_GPIO_SS; + } master->dev.of_node = pdev->dev.of_node; master->bits_per_word_mask = SPI_BPW_MASK(16) | SPI_BPW_MASK(8) | SPI_BPW_MASK(4); master->min_speed_hz = rs->freq / BAUDR_SCKDV_MAX; @@ -686,7 +723,6 @@ static int rockchip_spi_probe(struct platform_device *pdev) master->transfer_one = rockchip_spi_transfer_one; master->max_transfer_size = rockchip_spi_max_transfer_size; master->handle_err = rockchip_spi_handle_err; - master->flags = SPI_MASTER_GPIO_SS; master->dma_tx = dma_request_chan(rs->dev, "tx"); if (IS_ERR(master->dma_tx)) { -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v0 1/1] spi: spi-rockchip: add support for spi slave_mode 2020-05-08 8:37 ` [PATCH v0 1/1] spi: spi-rockchip: add support for spi slave_mode Chris Ruehl @ 2020-05-08 13:13 ` Emil Renner Berthing 2020-05-08 13:42 ` Emil Renner Berthing ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Emil Renner Berthing @ 2020-05-08 13:13 UTC (permalink / raw) To: Chris Ruehl Cc: Jack Lo, Heiko Stuebner, Linux Kernel Mailing List, linux-spi, open list:ARM/Rockchip SoC..., Mark Brown, linux-arm-kernel Hi Chris, On Fri, 8 May 2020 at 10:47, Chris Ruehl <chris.ruehl@gtsys.com.hk> wrote: > > This patch aim to add spi slave mode support to the rockchip driver. > Fix the wrong usage of num_cs set fix to ROCKCHIP_SPI_MAX_CS_NUM, > instead use max_native_cs flag to set the limit of native chip-select. > Enable use_gpio_descriptors to have cs_gpiod for gpio based chip-selects. > > Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk> > --- > drivers/spi/spi-rockchip.c | 46 +++++++++++++++++++++++++++++++++----- > 1 file changed, 41 insertions(+), 5 deletions(-) > > diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c > index 70ef63e0b6b8..9c1ff52c0f85 100644 > --- a/drivers/spi/spi-rockchip.c > +++ b/drivers/spi/spi-rockchip.c > @@ -183,6 +183,9 @@ struct rockchip_spi { > u8 rsd; > > bool cs_asserted[ROCKCHIP_SPI_MAX_CS_NUM]; > + > + bool slave_mode; > + bool slave_abort; > }; > > static inline void spi_enable_chip(struct rockchip_spi *rs, bool enable) > @@ -359,7 +362,7 @@ static void rockchip_spi_dma_rxcb(void *data) > struct rockchip_spi *rs = spi_master_get_devdata(master); > int state = atomic_fetch_andnot(RXDMA, &rs->state); > > - if (state & TXDMA) > + if (state & TXDMA && !rs->slave_abort) > return; > > spi_enable_chip(rs, false); > @@ -372,7 +375,7 @@ static void rockchip_spi_dma_txcb(void *data) > struct rockchip_spi *rs = spi_master_get_devdata(master); > int state = atomic_fetch_andnot(TXDMA, &rs->state); > > - if (state & RXDMA) > + if (state & RXDMA && !rs->slave_abort) > return; > > /* Wait until the FIFO data completely. */ > @@ -466,6 +469,10 @@ static void rockchip_spi_config(struct rockchip_spi *rs, > u32 cr1; > u32 dmacr = 0; > > + if (rs->slavemode) > + cr0 |= CR0_OPM_SLAVE << CR0_OPM_OFFSET; > + rs->slave_abort = false; > + > cr0 |= rs->rsd << CR0_RSD_OFFSET; > cr0 |= (spi->mode & 0x3U) << CR0_SCPH_OFFSET; > if (spi->mode & SPI_LSB_FIRST) > @@ -535,6 +542,16 @@ static size_t rockchip_spi_max_transfer_size(struct spi_device *spi) > return ROCKCHIP_SPI_MAX_TRANLEN; > } > > +static int rockchip_spi_slave_abort(struct spi_master *master) > +{ > + struct rockchip_spi *rs = spi_master_get_devdata(master); > + > + rs->slave_abort = true; > + complete(master); > + > + return 0; > +} > + > static int rockchip_spi_transfer_one( > struct spi_master *master, > struct spi_device *spi, > @@ -589,14 +606,25 @@ static int rockchip_spi_probe(struct platform_device *pdev) > struct spi_master *master; > struct resource *mem; > u32 rsd_nsecs; > + bool slave_mode; > + u32 num_cs = 1; > + > + slave_mode = of_property_read_bool(np, "spi-slave"); > + > + if (slave_mode) > + master = spi_alloc_slave(&pdev->dev, > + sizeof(struct rockchip_spi)); > + else > + master = spi_alloc_master(&pdev->dev, > + sizeof(struct rockchip_spi)); > > - master = spi_alloc_master(&pdev->dev, sizeof(struct rockchip_spi)); > if (!master) > return -ENOMEM; > > platform_set_drvdata(pdev, master); > > rs = spi_master_get_devdata(master); > + rs->slave_mode = slave_mode; This entry doesn't seem to be read from any of your code, and even it it was, the same information is available in master->slave, so I don't see why you need it in the rockchip_spi struct. Also spi_master is just #defined to spi_controller in spi.h, so maybe consider changing all 'struct spi_master *master' to 'struct spi_controller *ctrl' now that the driver supports both modes. > > /* Get basic io resource and map it */ > mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > @@ -676,7 +704,16 @@ static int rockchip_spi_probe(struct platform_device *pdev) > master->auto_runtime_pm = true; > master->bus_num = pdev->id; > master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP | SPI_LSB_FIRST; > - master->num_chipselect = ROCKCHIP_SPI_MAX_CS_NUM; > + if (slave_mode) { > + master->mode_bits |= SPI_NO_CS; > + master->slave_abort = rockchip_spi_slave_abort; > + } else { > + of_property_read_u32(np, "num-cs", &num_cs); > + master->num_chipselect = num_cs; If you do something like this you won't need the temporary num_cs variable: if (of_property_read_u32(np, "num-cs", &master->num_chipselect)) master->num_chipselect = 1; Also it seems like you're changing the default from ROCKCHIP_SPI_MAX_CS_NUM to 1 if there is no num-cs property. Did you check that all boards either have the num-cs property defined or only needs num_chipselect = 1? > + master->use_gpio_descriptors = true; > + master->max_native_cs = ROCKCHIP_SPI_MAX_CS_NUM; > + master->flags = SPI_MASTER_GPIO_SS; > + } > master->dev.of_node = pdev->dev.of_node; > master->bits_per_word_mask = SPI_BPW_MASK(16) | SPI_BPW_MASK(8) | SPI_BPW_MASK(4); > master->min_speed_hz = rs->freq / BAUDR_SCKDV_MAX; > @@ -686,7 +723,6 @@ static int rockchip_spi_probe(struct platform_device *pdev) > master->transfer_one = rockchip_spi_transfer_one; > master->max_transfer_size = rockchip_spi_max_transfer_size; > master->handle_err = rockchip_spi_handle_err; > - master->flags = SPI_MASTER_GPIO_SS; > > master->dma_tx = dma_request_chan(rs->dev, "tx"); > if (IS_ERR(master->dma_tx)) { > -- > 2.20.1 > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v0 1/1] spi: spi-rockchip: add support for spi slave_mode 2020-05-08 13:13 ` Emil Renner Berthing @ 2020-05-08 13:42 ` Emil Renner Berthing 2020-05-09 0:10 ` Chris Ruehl 2020-05-09 9:11 ` Chris Ruehl 2 siblings, 0 replies; 7+ messages in thread From: Emil Renner Berthing @ 2020-05-08 13:42 UTC (permalink / raw) To: Chris Ruehl Cc: Jack Lo, Heiko Stuebner, Linux Kernel Mailing List, linux-spi, open list:ARM/Rockchip SoC..., Mark Brown, linux-arm-kernel Hi Chris, On Fri, 8 May 2020 at 15:13, Emil Renner Berthing <emil.renner.berthing@gmail.com> wrote: > If you do something like this you won't need the temporary num_cs variable: > > if (of_property_read_u32(np, "num-cs", &master->num_chipselect)) > master->num_chipselect = 1; Sorry, that should be of_property_read_u16, since num_chipselect is a u16. /Emil ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v0 1/1] spi: spi-rockchip: add support for spi slave_mode 2020-05-08 13:13 ` Emil Renner Berthing 2020-05-08 13:42 ` Emil Renner Berthing @ 2020-05-09 0:10 ` Chris Ruehl 2020-05-10 16:15 ` Emil Renner Berthing 2020-05-09 9:11 ` Chris Ruehl 2 siblings, 1 reply; 7+ messages in thread From: Chris Ruehl @ 2020-05-09 0:10 UTC (permalink / raw) To: Emil Renner Berthing Cc: Jack Lo, Heiko Stuebner, Linux Kernel Mailing List, linux-spi, open list:ARM/Rockchip SoC..., Mark Brown, linux-arm-kernel Hi Emil, thanks for the review and your comments On 8/5/2020 9:13 pm, Emil Renner Berthing wrote: > Hi Chris, > > On Fri, 8 May 2020 at 10:47, Chris Ruehl <chris.ruehl@gtsys.com.hk> wrote: >> This patch aim to add spi slave mode support to the rockchip driver. >> Fix the wrong usage of num_cs set fix to ROCKCHIP_SPI_MAX_CS_NUM, >> instead use max_native_cs flag to set the limit of native chip-select. >> Enable use_gpio_descriptors to have cs_gpiod for gpio based chip-selects. >> >> Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk> >> --- >> drivers/spi/spi-rockchip.c | 46 +++++++++++++++++++++++++++++++++----- >> 1 file changed, 41 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c >> index 70ef63e0b6b8..9c1ff52c0f85 100644 >> --- a/drivers/spi/spi-rockchip.c >> +++ b/drivers/spi/spi-rockchip.c >> @@ -183,6 +183,9 @@ struct rockchip_spi { >> u8 rsd; >> >> bool cs_asserted[ROCKCHIP_SPI_MAX_CS_NUM]; >> + >> + bool slave_mode; >> + bool slave_abort; >> }; >> >> static inline void spi_enable_chip(struct rockchip_spi *rs, bool enable) >> @@ -359,7 +362,7 @@ static void rockchip_spi_dma_rxcb(void *data) >> struct rockchip_spi *rs = spi_master_get_devdata(master); >> int state = atomic_fetch_andnot(RXDMA, &rs->state); >> >> - if (state & TXDMA) >> + if (state & TXDMA && !rs->slave_abort) >> return; >> >> spi_enable_chip(rs, false); >> @@ -372,7 +375,7 @@ static void rockchip_spi_dma_txcb(void *data) >> struct rockchip_spi *rs = spi_master_get_devdata(master); >> int state = atomic_fetch_andnot(TXDMA, &rs->state); >> >> - if (state & RXDMA) >> + if (state & RXDMA && !rs->slave_abort) >> return; >> >> /* Wait until the FIFO data completely. */ >> @@ -466,6 +469,10 @@ static void rockchip_spi_config(struct rockchip_spi *rs, >> u32 cr1; >> u32 dmacr = 0; >> >> + if (rs->slavemode) >> + cr0 |= CR0_OPM_SLAVE << CR0_OPM_OFFSET; >> + rs->slave_abort = false; >> + >> cr0 |= rs->rsd << CR0_RSD_OFFSET; >> cr0 |= (spi->mode & 0x3U) << CR0_SCPH_OFFSET; >> if (spi->mode & SPI_LSB_FIRST) >> @@ -535,6 +542,16 @@ static size_t rockchip_spi_max_transfer_size(struct spi_device *spi) >> return ROCKCHIP_SPI_MAX_TRANLEN; >> } >> >> +static int rockchip_spi_slave_abort(struct spi_master *master) >> +{ >> + struct rockchip_spi *rs = spi_master_get_devdata(master); >> + >> + rs->slave_abort = true; >> + complete(master); >> + >> + return 0; >> +} >> + >> static int rockchip_spi_transfer_one( >> struct spi_master *master, >> struct spi_device *spi, >> @@ -589,14 +606,25 @@ static int rockchip_spi_probe(struct platform_device *pdev) >> struct spi_master *master; >> struct resource *mem; >> u32 rsd_nsecs; >> + bool slave_mode; >> + u32 num_cs = 1; >> + >> + slave_mode = of_property_read_bool(np, "spi-slave"); >> + >> + if (slave_mode) >> + master = spi_alloc_slave(&pdev->dev, >> + sizeof(struct rockchip_spi)); >> + else >> + master = spi_alloc_master(&pdev->dev, >> + sizeof(struct rockchip_spi)); >> >> - master = spi_alloc_master(&pdev->dev, sizeof(struct rockchip_spi)); >> if (!master) >> return -ENOMEM; >> >> platform_set_drvdata(pdev, master); >> >> rs = spi_master_get_devdata(master); >> + rs->slave_mode = slave_mode; > This entry doesn't seem to be read from any of your code, and even it > it was, the same information is available in master->slave, so I don't > see why you need it in the rockchip_spi struct. I haven't see the slave flag in the spi_controller struct, I will store the information there. > > Also spi_master is just #defined to spi_controller in spi.h, so maybe > consider changing all 'struct spi_master *master' to 'struct > spi_controller *ctrl' now that the driver supports both modes. Can do, but I think that is better to have a separate patch for it, make it easier for review. > >> /* Get basic io resource and map it */ >> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> @@ -676,7 +704,16 @@ static int rockchip_spi_probe(struct platform_device *pdev) >> master->auto_runtime_pm = true; >> master->bus_num = pdev->id; >> master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP | SPI_LSB_FIRST; >> - master->num_chipselect = ROCKCHIP_SPI_MAX_CS_NUM; >> + if (slave_mode) { >> + master->mode_bits |= SPI_NO_CS; >> + master->slave_abort = rockchip_spi_slave_abort; >> + } else { >> + of_property_read_u32(np, "num-cs", &num_cs); >> + master->num_chipselect = num_cs; > If you do something like this you won't need the temporary num_cs variable: > > if (of_property_read_u32(np, "num-cs", &master->num_chipselect)) > master->num_chipselect = 1; Like it , can see clearly the fallback to a default if num-cs isn't set in the dts. > > Also it seems like you're changing the default from > ROCKCHIP_SPI_MAX_CS_NUM to 1 if there is no num-cs property. Did you > check that all boards either have the num-cs property defined or only > needs num_chipselect = 1? Only spi0 of the rockchip has a 2nd native chip select, all others a single only therefore I find it less evil to use 1 vs. ROCKCHIP_SPI_MAX_CS_NUM >> + master->use_gpio_descriptors = true; >> + master->max_native_cs = ROCKCHIP_SPI_MAX_CS_NUM; >> + master->flags = SPI_MASTER_GPIO_SS; >> + } >> master->dev.of_node = pdev->dev.of_node; >> master->bits_per_word_mask = SPI_BPW_MASK(16) | SPI_BPW_MASK(8) | SPI_BPW_MASK(4); >> master->min_speed_hz = rs->freq / BAUDR_SCKDV_MAX; >> @@ -686,7 +723,6 @@ static int rockchip_spi_probe(struct platform_device *pdev) >> master->transfer_one = rockchip_spi_transfer_one; >> master->max_transfer_size = rockchip_spi_max_transfer_size; >> master->handle_err = rockchip_spi_handle_err; >> - master->flags = SPI_MASTER_GPIO_SS; >> >> master->dma_tx = dma_request_chan(rs->dev, "tx"); >> if (IS_ERR(master->dma_tx)) { >> -- >> 2.20.1 >> >> >> _______________________________________________ >> Linux-rockchip mailing list >> Linux-rockchip@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-rockchip -- GTSYS Limited RFID Technology 9/F, Unit E, R07, Kwai Shing Industrial Building Phase 2, 42-46 Tai Lin Pai Road, Kwai Chung, N.T., Hong Kong Tel (852) 9079 9521 Disclaimer: https://www.gtsys.com.hk/email/classified.html ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v0 1/1] spi: spi-rockchip: add support for spi slave_mode 2020-05-09 0:10 ` Chris Ruehl @ 2020-05-10 16:15 ` Emil Renner Berthing 0 siblings, 0 replies; 7+ messages in thread From: Emil Renner Berthing @ 2020-05-10 16:15 UTC (permalink / raw) To: Chris Ruehl Cc: Jack Lo, Heiko Stuebner, Linux Kernel Mailing List, linux-spi, open list:ARM/Rockchip SoC..., Mark Brown, linux-arm-kernel Hi Chris, On Sat, 9 May 2020 at 02:10, Chris Ruehl <chris.ruehl@gtsys.com.hk> wrote: > > Hi Emil, > > thanks for the review and your comments > > On 8/5/2020 9:13 pm, Emil Renner Berthing wrote: > > Hi Chris, > > > > On Fri, 8 May 2020 at 10:47, Chris Ruehl <chris.ruehl@gtsys.com.hk> wrote: > >> This patch aim to add spi slave mode support to the rockchip driver. > >> Fix the wrong usage of num_cs set fix to ROCKCHIP_SPI_MAX_CS_NUM, > >> instead use max_native_cs flag to set the limit of native chip-select. > >> Enable use_gpio_descriptors to have cs_gpiod for gpio based chip-selects. > >> > >> Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk> > >> --- > >> drivers/spi/spi-rockchip.c | 46 +++++++++++++++++++++++++++++++++----- > >> 1 file changed, 41 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c > >> index 70ef63e0b6b8..9c1ff52c0f85 100644 > >> --- a/drivers/spi/spi-rockchip.c > >> +++ b/drivers/spi/spi-rockchip.c > >> @@ -183,6 +183,9 @@ struct rockchip_spi { > >> u8 rsd; > >> > >> bool cs_asserted[ROCKCHIP_SPI_MAX_CS_NUM]; > >> + > >> + bool slave_mode; > >> + bool slave_abort; > >> }; > >> > >> static inline void spi_enable_chip(struct rockchip_spi *rs, bool enable) > >> @@ -359,7 +362,7 @@ static void rockchip_spi_dma_rxcb(void *data) > >> struct rockchip_spi *rs = spi_master_get_devdata(master); > >> int state = atomic_fetch_andnot(RXDMA, &rs->state); > >> > >> - if (state & TXDMA) > >> + if (state & TXDMA && !rs->slave_abort) > >> return; > >> > >> spi_enable_chip(rs, false); > >> @@ -372,7 +375,7 @@ static void rockchip_spi_dma_txcb(void *data) > >> struct rockchip_spi *rs = spi_master_get_devdata(master); > >> int state = atomic_fetch_andnot(TXDMA, &rs->state); > >> > >> - if (state & RXDMA) > >> + if (state & RXDMA && !rs->slave_abort) > >> return; > >> > >> /* Wait until the FIFO data completely. */ > >> @@ -466,6 +469,10 @@ static void rockchip_spi_config(struct rockchip_spi *rs, > >> u32 cr1; > >> u32 dmacr = 0; > >> > >> + if (rs->slavemode) > >> + cr0 |= CR0_OPM_SLAVE << CR0_OPM_OFFSET; > >> + rs->slave_abort = false; > >> + > >> cr0 |= rs->rsd << CR0_RSD_OFFSET; > >> cr0 |= (spi->mode & 0x3U) << CR0_SCPH_OFFSET; > >> if (spi->mode & SPI_LSB_FIRST) > >> @@ -535,6 +542,16 @@ static size_t rockchip_spi_max_transfer_size(struct spi_device *spi) > >> return ROCKCHIP_SPI_MAX_TRANLEN; > >> } > >> > >> +static int rockchip_spi_slave_abort(struct spi_master *master) > >> +{ > >> + struct rockchip_spi *rs = spi_master_get_devdata(master); > >> + > >> + rs->slave_abort = true; > >> + complete(master); > >> + > >> + return 0; > >> +} > >> + > >> static int rockchip_spi_transfer_one( > >> struct spi_master *master, > >> struct spi_device *spi, > >> @@ -589,14 +606,25 @@ static int rockchip_spi_probe(struct platform_device *pdev) > >> struct spi_master *master; > >> struct resource *mem; > >> u32 rsd_nsecs; > >> + bool slave_mode; > >> + u32 num_cs = 1; > >> + > >> + slave_mode = of_property_read_bool(np, "spi-slave"); > >> + > >> + if (slave_mode) > >> + master = spi_alloc_slave(&pdev->dev, > >> + sizeof(struct rockchip_spi)); > >> + else > >> + master = spi_alloc_master(&pdev->dev, > >> + sizeof(struct rockchip_spi)); > >> > >> - master = spi_alloc_master(&pdev->dev, sizeof(struct rockchip_spi)); > >> if (!master) > >> return -ENOMEM; > >> > >> platform_set_drvdata(pdev, master); > >> > >> rs = spi_master_get_devdata(master); > >> + rs->slave_mode = slave_mode; > > This entry doesn't seem to be read from any of your code, and even it > > it was, the same information is available in master->slave, so I don't > > see why you need it in the rockchip_spi struct. > I haven't see the slave flag in the spi_controller struct, I will store the > information > there. > > > > Also spi_master is just #defined to spi_controller in spi.h, so maybe > > consider changing all 'struct spi_master *master' to 'struct > > spi_controller *ctrl' now that the driver supports both modes. > Can do, but I think that is better to have a separate patch for it, > make it easier for review. Yes, you should probably do something like patch 1/3: change struct spi_master *master to struct spi_controller *ctrl patch 2/3: add slave slave mode patch 3/3: use num-cs property for ctrl->num_chipselect > > > >> /* Get basic io resource and map it */ > >> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > >> @@ -676,7 +704,16 @@ static int rockchip_spi_probe(struct platform_device *pdev) > >> master->auto_runtime_pm = true; > >> master->bus_num = pdev->id; > >> master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP | SPI_LSB_FIRST; > >> - master->num_chipselect = ROCKCHIP_SPI_MAX_CS_NUM; > >> + if (slave_mode) { > >> + master->mode_bits |= SPI_NO_CS; > >> + master->slave_abort = rockchip_spi_slave_abort; > >> + } else { > >> + of_property_read_u32(np, "num-cs", &num_cs); > >> + master->num_chipselect = num_cs; > > If you do something like this you won't need the temporary num_cs variable: > > > > if (of_property_read_u32(np, "num-cs", &master->num_chipselect)) > > master->num_chipselect = 1; > Like it , can see clearly the fallback to a default if num-cs isn't set in the > dts. > > > > Also it seems like you're changing the default from > > ROCKCHIP_SPI_MAX_CS_NUM to 1 if there is no num-cs property. Did you > > check that all boards either have the num-cs property defined or only > > needs num_chipselect = 1? > Only spi0 of the rockchip has a 2nd native chip select, all others a single only > therefore I find it less evil to use 1 vs. ROCKCHIP_SPI_MAX_CS_NUM > > >> + master->use_gpio_descriptors = true; > >> + master->max_native_cs = ROCKCHIP_SPI_MAX_CS_NUM; > >> + master->flags = SPI_MASTER_GPIO_SS; > >> + } > >> master->dev.of_node = pdev->dev.of_node; > >> master->bits_per_word_mask = SPI_BPW_MASK(16) | SPI_BPW_MASK(8) | SPI_BPW_MASK(4); > >> master->min_speed_hz = rs->freq / BAUDR_SCKDV_MAX; > >> @@ -686,7 +723,6 @@ static int rockchip_spi_probe(struct platform_device *pdev) > >> master->transfer_one = rockchip_spi_transfer_one; > >> master->max_transfer_size = rockchip_spi_max_transfer_size; > >> master->handle_err = rockchip_spi_handle_err; > >> - master->flags = SPI_MASTER_GPIO_SS; > >> > >> master->dma_tx = dma_request_chan(rs->dev, "tx"); > >> if (IS_ERR(master->dma_tx)) { > >> -- > >> 2.20.1 > >> > >> > >> _______________________________________________ > >> Linux-rockchip mailing list > >> Linux-rockchip@lists.infradead.org > >> http://lists.infradead.org/mailman/listinfo/linux-rockchip > > -- > GTSYS Limited RFID Technology > 9/F, Unit E, R07, Kwai Shing Industrial Building Phase 2, > 42-46 Tai Lin Pai Road, Kwai Chung, N.T., Hong Kong > Tel (852) 9079 9521 > > Disclaimer: https://www.gtsys.com.hk/email/classified.html > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v0 1/1] spi: spi-rockchip: add support for spi slave_mode 2020-05-08 13:13 ` Emil Renner Berthing 2020-05-08 13:42 ` Emil Renner Berthing 2020-05-09 0:10 ` Chris Ruehl @ 2020-05-09 9:11 ` Chris Ruehl 2 siblings, 0 replies; 7+ messages in thread From: Chris Ruehl @ 2020-05-09 9:11 UTC (permalink / raw) To: Emil Renner Berthing Cc: Jack Lo, Heiko Stuebner, Linux Kernel Mailing List, linux-spi, open list:ARM/Rockchip SoC..., Mark Brown, linux-arm-kernel Emil, >> + if (rs->slavemode) here is a mistake it is : rs->slave_mode and the use of rs->slave_mode in the rockchip_spi_config() ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-05-10 16:15 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-08 8:37 [PATCH v0] spi: spi-rockchip spi slave mode Chris Ruehl 2020-05-08 8:37 ` [PATCH v0 1/1] spi: spi-rockchip: add support for spi slave_mode Chris Ruehl 2020-05-08 13:13 ` Emil Renner Berthing 2020-05-08 13:42 ` Emil Renner Berthing 2020-05-09 0:10 ` Chris Ruehl 2020-05-10 16:15 ` Emil Renner Berthing 2020-05-09 9:11 ` Chris Ruehl
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).