Hi, Any consensus regarding the issue ? Do we go forward with the present implementation or a change is required ?
I have another change for getting and setting the clock rate which is by the driver. I can push the same along with the cosmetic changes done by uncrustify. Thanks Mudit On Fri, Jun 24, 2016 at 3:56 PM, Pavel Pisa <ppisa4li...@pikron.com> wrote: > 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 > >
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
