Got it! Thanks!
I'll split this one to three patches:
1) replace magic numbers with named constants (refactoring)
2) add new properties for VC 4
3) Add some unit tests to check the newly added properties via mailbox
Is it OK?
(Sorry for wasting your time by inappropriate patches - this is my first
experience with OSS)
________________________________
От: Peter Maydell <peter.mayd...@linaro.org>
Отправлено: 30 мая 2023 г. 15:12:24
Кому: Sergey Kambalin
Копия: qemu-...@nongnu.org; qemu-devel@nongnu.org; Kambalin, Sergey
Тема: Re: [PATCH] Prepare bcm properties for videocore 4
On Wed, 24 May 2023 at 20:15, Sergey Kambalin <serg.o...@gmail.com> wrote:
>
> Hello!
> Sorry for a quite a big patch, but most of the changes are the same type.
> Most of the patch is about a definition of new constants/structs and replacing
> magic numbers with those constants.
>
>
> Signed-off-by: Sergey Kambalin <sergey.kamba...@auriga.com>
> ---
> @@ -51,48 +98,48 @@ static void
> bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value)
> /* @(value + 8) : Request/response indicator */
> resplen = 0;
> switch (tag) {
> - case 0x00000000: /* End tag */
> + case RPI_FWREQ_PROPERTY_END: /* End tag */
> break;
> - case 0x00000001: /* Get firmware revision */
> + case RPI_FWREQ_GET_FIRMWARE_REVISION: /* Get firmware revision */
> stl_le_phys(&s->dma_as, value + 12, 346337);
> resplen = 4;
> break;
> - case 0x00010001: /* Get board model */
> + case RPI_FWREQ_GET_BOARD_MODEL: /* Get board model */
> qemu_log_mask(LOG_UNIMP,
> "bcm2835_property: 0x%08x get board model NYI\n",
> tag);
> resplen = 4;
> break;
> - case 0x00010002: /* Get board revision */
> + case RPI_FWREQ_GET_BOARD_REVISION: /* Get board revision */
So mostly this is just updating hardcoded constants to
symbolic constants, which is great.
> - case 0x00038002: /* Set clock rate */
> - case 0x00038004: /* Set max clock rate */
> - case 0x00038007: /* Set min clock rate */
> + case RPI_FWREQ_GET_CLOCKS: /* Get clocks */
> + /* TODO: add more clock IDs if needed */
> + stl_le_phys(&s->dma_as, value + 12, 0);
> + stl_le_phys(&s->dma_as, value + 16, RPI_FIRMWARE_ARM_CLK_ID);
> + resplen = 8;
> + break;
But this is adding a new implementation of a new property...
> +
> + case RPI_FWREQ_SET_CLOCK_RATE: /* Set clock rate */
> + case RPI_FWREQ_SET_MAX_CLOCK_RATE: /* Set max clock rate */
> + case RPI_FWREQ_SET_MIN_CLOCK_RATE: /* Set min clock rate */
> qemu_log_mask(LOG_UNIMP,
> "bcm2835_property: 0x%08x set clock rate NYI\n",
> tag);
> @@ -295,6 +357,144 @@ static void
> bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value)
> resplen);
> break;
>
> + case RPI_FWREQ_GET_THROTTLED: /* Get throttled */
> + stl_le_phys(&s->dma_as, value + 12, 0);
> + resplen = 4;
> + break;
...and this and the other bits further down in this part of
the patch are new properties too.
New features should be in separate patches from refactoring cleanup,
please.
By the way: when you split up patches you don't need to send them
just one at a time -- you can send a patchset of multiple related
patches.
thanks
-- PMM
