[PATCH] Adding functionalities to Mailbox RPi

[Pavel Pisa]

Hello Mudit and Gedare,

On Friday 24 of June 2016 12:02:36 Mudit Jain wrote:
> Hi Gedare,
>
> When I use uncrustify, it makes a lot of changes to the previous code as
> well. We can have another patch over this for the cosmetic changes.
>
> I myself, did have doubts regarding the empty struct and having two structs
> that are basically identical, however previously it was implemented in the
> similar fashion so I went on with it.
> For example : *bcm2835_mbox_tag_get_arm_memory* &
> *bcm2835_mbox_tag_get_vc_memory.
> *This is the same for a lot of the mailbox code.

I agree that uncrustify should be run before or after changes
and result committed as separate patch.
Suggestion if to run before or after Mudit's patch is a question.
But may it be after would be simpler because actual patch version
is tested and works.

As for empty request struck, I am not sure about it.
It declares that there are no data send to VC for given
function and all data exchanges have defined "req"
and "resp" fields in their buffer format structures
now. The presence of the field (even empty) can make
some access macros more generic. But I am not sure if this
can be required in future. So removal of empty members
can be done.

As for two almost same structures, it is questionale as well.
May it be, we should have three in the fact
bcm2835_mbox_tag_get_fw_rev - .rev
bcm2835_mbox_tag_get_board_model - .model
bcm2835_mbox_tag_get_board_version - .version

May it be actual two are ok

bcm2835_mbox_tag_get_fw_rev - .rev
  (this is of different/strange meaning 1461755178 = 0x57209d2a)
  on my board and the number is confirmed from some other
  source as valid firmware revision - may it be that is checksum?

bcm2835_mbox_tag_get_board_spec - .spec
  (on the other hand these are sane small documented numbers
   0, 20 = 0x14)

May be there should be only one structure

bcm2835_mbox_tag_get_uint32 - .value

which can be used directly or bcm2835_mbox_tag_get_fw_rev,
bcm2835_mbox_tag_get_board_model, bcm2835_mbox_tag_get_board_version
can be derived from that. It would be possible to have only
single function in that case as well.

But generally, I have no preference there and actual Mudit proposed
solution is acceptable by me.

Best wishes,

             Pavel