[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v8 04/10] hw/ssi: imx_spi: Rework imx_spi_reset() to keep CON
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v8 04/10] hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG register value |
Date: |
Thu, 28 Jan 2021 15:49:13 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 |
On 1/28/21 3:41 PM, Peter Maydell wrote:
> On Thu, 28 Jan 2021 at 14:32, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> Oh I totally missed that :S
>>
>> Bin, I'd correct this as:
>>
>> - extract imx_spi_soft_reset(IMXSPIState *s) from imx_spi_reset()
>> - zero-initialize CONREG in imx_spi_reset().
>>
>> static void imx_spi_soft_reset(IMXSPIState *s)
>> {
>> ...
>> }
>>
>> static void imx_spi_reset(DeviceState *dev)
>> {
>> IMXSPIState *s = IMX_SPI(dev);
>>
>> s->regs[ECSPI_CONREG] = 0;
>> imx_spi_soft_reset(s);
>> }
>>
>> What do you think?
>
> That doesn't give you anywhere to put the imx_spi_update_irq()
> call, which must happen only on soft reset and not on DeviceState
> reset. You could do one of:
> * have a 'common reset' function that does most of this,
> plus an imx_spi_reset which clears CONREG and calls
> common reset and an imx_spi_soft_reset which calls
> common reset and imx_spi_update_irq()
> * have imx_spi_soft_reset save the old CONREG in a local
> variable before calling imx_spi_reset and then restore it
> to s->regs
Long term maintenance I'd prefer the 'common reset' approach
(but this is probably subjective to my view on the hardware,
since this is software, the 2nd approach is also valid but
harder to represent thinking of hardware).
Bin, can you send a v9? (using the approach you prefer)
Thanks,
Phil.
- [PATCH v8 03/10] hw/ssi: imx_spi: Remove pointless variable initialization, (continued)
- [PATCH v8 03/10] hw/ssi: imx_spi: Remove pointless variable initialization, Bin Meng, 2021/01/19
- [PATCH v8 04/10] hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG register value, Bin Meng, 2021/01/19
- Re: [PATCH v8 04/10] hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG register value, Peter Maydell, 2021/01/28
- Re: [PATCH v8 04/10] hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG register value, Bin Meng, 2021/01/28
- Re: [PATCH v8 04/10] hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG register value, Peter Maydell, 2021/01/28
- Re: [PATCH v8 04/10] hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG register value, Bin Meng, 2021/01/28
- Re: [PATCH v8 04/10] hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG register value, Philippe Mathieu-Daudé, 2021/01/28
- Re: [PATCH v8 04/10] hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG register value, Peter Maydell, 2021/01/28
- Re: [PATCH v8 04/10] hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG register value, Philippe Mathieu-Daudé, 2021/01/28
- Re: [PATCH v8 04/10] hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG register value, Peter Maydell, 2021/01/28
- Re: [PATCH v8 04/10] hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG register value,
Philippe Mathieu-Daudé <=
- Re: [PATCH v8 04/10] hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG register value, Bin Meng, 2021/01/28
[PATCH v8 05/10] hw/ssi: imx_spi: Rework imx_spi_read() to handle block disabled, Bin Meng, 2021/01/19
[PATCH v8 07/10] hw/ssi: imx_spi: Disable chip selects when controller is disabled, Bin Meng, 2021/01/19
[PATCH v8 06/10] hw/ssi: imx_spi: Rework imx_spi_write() to handle block disabled, Bin Meng, 2021/01/19
[PATCH v8 09/10] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic, Bin Meng, 2021/01/19
[PATCH v8 08/10] hw/ssi: imx_spi: Round up the burst length to be multiple of 8, Bin Meng, 2021/01/19
[PATCH v8 10/10] hw/ssi: imx_spi: Correct tx and rx fifo endianness, Bin Meng, 2021/01/19