[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v1] nrf51: Fix last GPIO CNF address
From: |
Cameron Esfahani |
Subject: |
Re: [PATCH v1] nrf51: Fix last GPIO CNF address |
Date: |
Tue, 07 Apr 2020 03:09:31 -0700 |
I'm not burying anything. This patch is stand alone and all the tests do work.
They work with or without Cedric's nee Andrew's patch. But, if some
derivative of that patch is eventually implemented, something needs to be done
for this NRF51 gpio qtest to work.
There are two possibilities for the following qtest in microbit-test.c:
> actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_END) &
> 0x01;
> g_assert_cmpuint(actual, ==, 0x01);
1 - The code is purposefully reading 32-bits from an unaligned address which
only partially refers to a documented register. And the only reason this code
works is because that 32-bit value is turned into a 8-bit read. And if that's
the case, then someone should update a comment in the test to indicate that
this is being done purposefully.
2 - NRF51_GPIO_REG_CNF_END is incorrectly set to be 0x77F. Looking at the
documentation for this chip, the last defined CNF register starts at 0x77C.
The NRF51 doesn't support unaligned accesses, so I assume that possibility 1
isn't true.
So, is NRF51_GPIO_REG_CNF_END supposed to refer to a valid register, or the end
of the implementation space?
If it's the former, then it should be adjusted to 0x77c and possibly update the
below code in nrf51_gpio.c (line 156):
> case NRF51_GPIO_REG_CNF_START ... NRF51_GPIO_REG_CNF_END:
to become
> case NRF51_GPIO_REG_CNF_START ... NRF51_GPIO_REG_CNF_END + 3:
if unaligned access are expected to work. But, considering the NRF51 doesn't
support unaligned accesses, I don't think this appropriate.
If it's the latter, then the test cases in microbit-test.c should be updated to
something like the following:
> actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_END - 3) &
> 0x01;
> g_assert_cmpuint(actual, ==, 0x01);
Cameron Esfahani
address@hidden
"Americans are very skilled at creating a custom meaning from something that's
mass-produced."
Ann Powers
> On Apr 7, 2020, at 1:44 AM, Philippe Mathieu-Daudé <address@hidden> wrote:
>
>> Considering NRF51 doesn't support unaligned accesses, the simplest fix
>> is to actually set NRF51_GPIO_REG_CNF_END to the start of the last valid
>> CNF register: 0x77c.
>
> NAck. You are burying bugs deeper. This test has to work.
>
> What would be helpful is qtests with unaligned accesses (expected to work)
> such your USB XHCI VERSION example.
Re: [PATCH v1] nrf51: Fix last GPIO CNF address, Philippe Mathieu-Daudé, 2020/04/07
- Re: [PATCH v1] nrf51: Fix last GPIO CNF address,
Cameron Esfahani <=