On Mon, 12 May 2025 at 02:25, Tim Lee <timlee660...@gmail.com> wrote:
>
> Hi Peter,
> Sorry about that. When I ran this qtest and I found an error then I
> tried to modify npcm_pspi driver to make data register read/write test
> pass.
>
> [R +0.080118] writew 0xf0201002 0x4
> [S +0.080126] OK
> [R +0.080148] readw 0xf0201002
> [S +0.080153] OK 0x0000000000000004
> [R +0.080168] writew 0xf0201000 0x1234
> [S +0.080171] OK
> [R +0.080191] readw 0xf0201000
> [S +0.080194] OK 0x0000000000001234
> [I +0.080445] CLOSED
> ok 3 /aarch64/npcm8xx_pspi/data
> # End of npcm8xx_pspi tests
> # End of aarch64 tests
>
> Here is the change diff of what I modified in npcm_pspi_write_data()
> Should I submit this patch for npcm_pspi driver? I'm not sure if I
> modified it correctly. Thanks for your time.
>
> static void npcm_pspi_write_data(NPCMPSPIState *s, uint16_t data)
> {
> - uint16_t value = 0;
> + uint16_t data_l, data_h = 0;
>
> if (FIELD_EX16(s->regs[R_PSPI_CTL1], PSPI_CTL1, MOD)) {
> - value = ssi_transfer(s->spi, extract16(data, 8, 8)) << 8;
> + data_h = (extract16(data, 8, 8) << 8);
> + ssi_transfer(s->spi, data_h);
> }
> - value |= ssi_transfer(s->spi, extract16(data, 0, 8));
> - s->regs[R_PSPI_DATA] = value;
> + data_l = extract16(data, 0, 8);
> + ssi_transfer(s->spi, data_l);
> + s->regs[R_PSPI_DATA] = (data_h | data_l);
If you think there's a bug in the PSPI device then the
best thing is to submit what you think is the fix as
its own patch, with a commit message that details what
the incorrect behaviour is (and why you think it's incorrect,
e.g. with reference to a data sheet). Then the NPCM folks can
review it for whether it's correct or not.
You can send a 2-patch series with the bug fix as patch 1,
followed by the new test case as patch 2.
thanks
-- PMM
