Hi,

This is how I understand the ecSPI reset works, after
looking at the IMX6DQRM.pdf datasheet.

This is a respin of Ben's v5 series [*].

Since v6:
- Dropped "Reduce 'change_mask' variable scope" patch
- Fixed inverted reset logic
- Added Juan R-b tags
- Removed 'RFC' tag as tests pass

Based-on: <1608688825-81519-1-git-send-email-bmeng.cn@gmail.com>
(queued on riscv-next).

Copy of Ben's v5 cover:

  This series fixes a bunch of bugs in current implementation of the imx
  spi controller, including the following issues:

  - chip select signal was not lower down when spi controller is disabled
  - remove imx_spi_update_irq() in imx_spi_reset()
  - round up the tx burst length to be multiple of 8
  - transfer incorrect data when the burst length is larger than 32 bit
  - spi controller tx and rx fifo endianness is incorrect

[*] https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg02333.html

Diff with v6:

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respective=
ly

001/9:[----] [--] 'hw/ssi: imx_spi: Use a macro for number of chip selects su=
pported'
002/9:[----] [--] 'hw/ssi: imx_spi: Remove pointless variable initialization'
003/9:[----] [-C] 'hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG reg=
ister value'
004/9:[----] [-C] 'hw/ssi: imx_spi: Rework imx_spi_read() to handle block dis=
abled'
005/9:[0003] [FC] 'hw/ssi: imx_spi: Rework imx_spi_write() to handle block di=
sabled'
006/9:[----] [--] 'hw/ssi: imx_spi: Disable chip selects when controller is d=
isabled'
007/9:[----] [--] 'hw/ssi: imx_spi: Round up the burst length to be multiple =
of 8'
008/9:[----] [--] 'hw/ssi: imx_spi: Correct the burst length > 32 bit transfe=
r logic'
009/9:[----] [--] 'hw/ssi: imx_spi: Correct tx and rx fifo endianness'

Bin Meng (4):
  hw/ssi: imx_spi: Use a macro for number of chip selects supported
  hw/ssi: imx_spi: Round up the burst length to be multiple of 8
  hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic
  hw/ssi: imx_spi: Correct tx and rx fifo endianness

Philippe Mathieu-Daud=C3=A9 (4):
  hw/ssi: imx_spi: Remove pointless variable initialization
  hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG register value
  hw/ssi: imx_spi: Rework imx_spi_read() to handle block disabled
  hw/ssi: imx_spi: Rework imx_spi_write() to handle block disabled

Xuzhou Cheng (1):
  hw/ssi: imx_spi: Disable chip selects when controller is disabled

 include/hw/ssi/imx_spi.h |   5 +-
 hw/ssi/imx_spi.c         | 137 +++++++++++++++++++++++----------------
 2 files changed, 86 insertions(+), 56 deletions(-)

--=20
2.26.2

Hi Philippe,

On Fri, Jan 15, 2021 at 11:31 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Hi,
>
> This is how I understand the ecSPI reset works, after
> looking at the IMX6DQRM.pdf datasheet.
>
> This is a respin of Ben's v5 series [*].
>
> Since v6:
> - Dropped "Reduce 'change_mask' variable scope" patch
> - Fixed inverted reset logic
> - Added Juan R-b tags
> - Removed 'RFC' tag as tests pass
>
> Based-on: <1608688825-81519-1-git-send-email-bmeng.cn@gmail.com>
> (queued on riscv-next).
>

This series dropped my imx_spi_soft_reset() change that has the
imx_spi_update_irq() moved from imx_spi_reset(). May I know why?

Regards,
Bin

On 1/16/21 3:03 PM, Bin Meng wrote:
> Hi Philippe,
> 
> On Fri, Jan 15, 2021 at 11:31 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> Hi,
>>
>> This is how I understand the ecSPI reset works, after
>> looking at the IMX6DQRM.pdf datasheet.
>>
>> This is a respin of Ben's v5 series [*].
>>
>> Since v6:
>> - Dropped "Reduce 'change_mask' variable scope" patch
>> - Fixed inverted reset logic
>> - Added Juan R-b tags
>> - Removed 'RFC' tag as tests pass
>>
>> Based-on: <1608688825-81519-1-git-send-email-bmeng.cn@gmail.com>
>> (queued on riscv-next).
>>
> 
> This series dropped my imx_spi_soft_reset() change that has the
> imx_spi_update_irq() moved from imx_spi_reset(). May I know why?

Because we don't need it. My comment without looking at the datasheet
was incorrect: there is only one single reset on the block.

> 
> Regards,
> Bin
> 

On Sat, Jan 16, 2021 at 11:07 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 1/16/21 3:03 PM, Bin Meng wrote:
> > Hi Philippe,
> >
> > On Fri, Jan 15, 2021 at 11:31 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >>
> >> Hi,
> >>
> >> This is how I understand the ecSPI reset works, after
> >> looking at the IMX6DQRM.pdf datasheet.
> >>
> >> This is a respin of Ben's v5 series [*].
> >>
> >> Since v6:
> >> - Dropped "Reduce 'change_mask' variable scope" patch
> >> - Fixed inverted reset logic
> >> - Added Juan R-b tags
> >> - Removed 'RFC' tag as tests pass
> >>
> >> Based-on: <1608688825-81519-1-git-send-email-bmeng.cn@gmail.com>
> >> (queued on riscv-next).
> >>
> >
> > This series dropped my imx_spi_soft_reset() change that has the
> > imx_spi_update_irq() moved from imx_spi_reset(). May I know why?
>
> Because we don't need it. My comment without looking at the datasheet
> was incorrect: there is only one single reset on the block.

Oh, you must have missed Peter's comments.

See his comments here:
http://patchwork.ozlabs.org/project/qemu-devel/patch/20201202144523.24526-2-bmeng.cn@gmail.com/

Regards,
Bin