Re: [RFC PATCH v6 00/11] hw/ssi: imx_spi: Fix various bugs in the imx_spi model
Hi Ben,
On 1/13/21 6:56 PM, Philippe Mathieu-Daudé wrote:
> On 1/13/21 2:27 PM, Bin Meng wrote:
>> Hi Philippe,
>>
Unfortunately this series breaks SPI flash testing under both U-Boot
and VxWorks 7.
>>>
>>> Thanks for testing :) Can you provide the binary tested and the command
>>> line used? At least one, so I can have a look.
>>
>> Sure, will send you offline.
>
> Arf, stupid mistake in patch 7 :) With this diff I can run your
> test:
>
> -- >8 --
> --- a/hw/ssi/imx_spi.c
> +++ b/hw/ssi/imx_spi.c
> @@ -343,7 +343,7 @@ static void imx_spi_write(void *opaque, hwaddr
> offset, uint64_t value,
> return;
> }
> s->regs[ECSPI_CONREG] = value;
> -if (value & ECSPI_CONREG_EN) {
> +if (!(value & ECSPI_CONREG_EN)) {
> /* Keep disabled */
> return;
> }
> ---
Could you have a try at this? Do you prefer I resubmit
the whole series?
Thanks,
Phil.
Re: [RFC PATCH v6 00/11] hw/ssi: imx_spi: Fix various bugs in the imx_spi model
On 1/13/21 2:27 PM, Bin Meng wrote:
> Hi Philippe,
>
> On Wed, Jan 13, 2021 at 3:53 PM Philippe Mathieu-Daudé
> wrote:
>>
>> Hi Ben,
>>
>> On 1/13/21 4:29 AM, Bin Meng wrote:
>>> On Wed, Jan 13, 2021 at 2:35 AM Philippe Mathieu-Daudé
>>> wrote:
Hi,
As it is sometimes harder for me to express myself in plain
English, I found it easier to write the patches I was thinking
about. I know this doesn't scale.
So 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 [*].
Tagged RFC because I have not tested it :)
>>>
>>> Unfortunately this series breaks SPI flash testing under both U-Boot
>>> and VxWorks 7.
>>
>> Thanks for testing :) Can you provide the binary tested and the command
>> line used? At least one, so I can have a look.
>
> Sure, will send you offline.
Arf, stupid mistake in patch 7 :) With this diff I can run your
test:
-- >8 --
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -343,7 +343,7 @@ static void imx_spi_write(void *opaque, hwaddr
offset, uint64_t value,
return;
}
s->regs[ECSPI_CONREG] = value;
-if (value & ECSPI_CONREG_EN) {
+if (!(value & ECSPI_CONREG_EN)) {
/* Keep disabled */
return;
}
---
Regards,
Phil.
Re: [RFC PATCH v6 00/11] hw/ssi: imx_spi: Fix various bugs in the imx_spi model
Hi Philippe, On Wed, Jan 13, 2021 at 3:53 PM Philippe Mathieu-Daudé wrote: > > Hi Ben, > > On 1/13/21 4:29 AM, Bin Meng wrote: > > On Wed, Jan 13, 2021 at 2:35 AM Philippe Mathieu-Daudé > > wrote: > >> > >> Hi, > >> > >> As it is sometimes harder for me to express myself in plain > >> English, I found it easier to write the patches I was thinking > >> about. I know this doesn't scale. > >> > >> So 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 [*]. > >> Tagged RFC because I have not tested it :) > > > > Unfortunately this series breaks SPI flash testing under both U-Boot > > and VxWorks 7. > > Thanks for testing :) Can you provide the binary tested and the command > line used? At least one, so I can have a look. Sure, will send you offline. > > >> Sometimes changing device reset to better match hardware gives > >> trouble when using '-kernel ...' because there is no bootloader > >> setting the device in the state Linux expects it. > >> > > > > Given most of the new changes in this RFC series are clean-ups, I > > suggest we apply the v5 series unless there is anything seriously > > wrong in v5, IOW, don't fix it unless it's broken. > > > > Thoughts? > > Up to the maintainer :) > > The IMX6DQRM datasheet is available here: > https://community.nxp.com/t5/i-MX-Processors-Knowledge-Base/i-MX-6DQ-Reference-Manual-IMX6DQRM-R2-Part-1/ta-p/1115983 > https://community.nxp.com/t5/i-MX-Processors-Knowledge-Base/i-MX-6DQ-Reference-Manual-IMX6DQRM-R2-Part-2/ta-p/1118510 Regards, Bin
Re: [RFC PATCH v6 00/11] hw/ssi: imx_spi: Fix various bugs in the imx_spi model
Hi Ben, On 1/13/21 4:29 AM, Bin Meng wrote: > On Wed, Jan 13, 2021 at 2:35 AM Philippe Mathieu-Daudé > wrote: >> >> Hi, >> >> As it is sometimes harder for me to express myself in plain >> English, I found it easier to write the patches I was thinking >> about. I know this doesn't scale. >> >> So 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 [*]. >> Tagged RFC because I have not tested it :) > > Unfortunately this series breaks SPI flash testing under both U-Boot > and VxWorks 7. Thanks for testing :) Can you provide the binary tested and the command line used? At least one, so I can have a look. >> Sometimes changing device reset to better match hardware gives >> trouble when using '-kernel ...' because there is no bootloader >> setting the device in the state Linux expects it. >> > > Given most of the new changes in this RFC series are clean-ups, I > suggest we apply the v5 series unless there is anything seriously > wrong in v5, IOW, don't fix it unless it's broken. > > Thoughts? Up to the maintainer :) The IMX6DQRM datasheet is available here: https://community.nxp.com/t5/i-MX-Processors-Knowledge-Base/i-MX-6DQ-Reference-Manual-IMX6DQRM-R2-Part-1/ta-p/1115983 https://community.nxp.com/t5/i-MX-Processors-Knowledge-Base/i-MX-6DQ-Reference-Manual-IMX6DQRM-R2-Part-2/ta-p/1118510 Regards, Phil.
Re: [RFC PATCH v6 00/11] hw/ssi: imx_spi: Fix various bugs in the imx_spi model
Hi Philippe, On Wed, Jan 13, 2021 at 2:35 AM Philippe Mathieu-Daudé wrote: > > Hi, > > As it is sometimes harder for me to express myself in plain > English, I found it easier to write the patches I was thinking > about. I know this doesn't scale. > > So 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 [*]. > Tagged RFC because I have not tested it :) Unfortunately this series breaks SPI flash testing under both U-Boot and VxWorks 7. > Sometimes changing device reset to better match hardware gives > trouble when using '-kernel ...' because there is no bootloader > setting the device in the state Linux expects it. > Given most of the new changes in this RFC series are clean-ups, I suggest we apply the v5 series unless there is anything seriously wrong in v5, IOW, don't fix it unless it's broken. Thoughts? Regards, Bin
[RFC PATCH v6 00/11] hw/ssi: imx_spi: Fix various bugs in the imx_spi model
Hi, As it is sometimes harder for me to express myself in plain English, I found it easier to write the patches I was thinking about. I know this doesn't scale. So 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 [*]. Tagged RFC because I have not tested it :) Sometimes changing device reset to better match hardware gives trouble when using '-kernel ...' because there is no bootloader setting the device in the state Linux expects it. 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 Ben's v5: 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/11:[] [--] 'hw/ssi: imx_spi: Use a macro for number of chip selects s= upported' 002/11:[down] 'hw/ssi: imx_spi: Remove pointless variable initialization' 003/11:[down] 'hw/ssi: imx_spi: Convert some debug printf()s to trace events' 004/11:[down] 'hw/ssi: imx_spi: Reduce 'change_mask' variable scope' 005/11:[down] 'hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG registe= r value' 006/11:[down] 'hw/ssi: imx_spi: Rework imx_spi_read() to handle block disable= d' 007/11:[down] 'hw/ssi: imx_spi: Rework imx_spi_write() to handle block disabl= ed' 008/11:[0004] [FC] 'hw/ssi: imx_spi: Disable chip selects when controller is = disabled' 009/11:[] [--] 'hw/ssi: imx_spi: Round up the burst length to be multiple= of 8' 010/11:[] [--] 'hw/ssi: imx_spi: Correct the burst length > 32 bit transf= er logic' 011/11:[] [--] '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 (6): hw/ssi: imx_spi: Remove pointless variable initialization hw/ssi: imx_spi: Convert some debug printf()s to trace events hw/ssi: imx_spi: Reduce 'change_mask' variable scope 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 | 147 +++ hw/ssi/trace-events | 7 ++ 3 files changed, 97 insertions(+), 62 deletions(-) --=20 2.26.2
