On 11/05/2012 05:04:13 PM, Timur Tabi wrote:
Varun Sethi wrote:
> +  /* PAACE Offset 0x00 */
> + u32 wbah; /* only valid for Primary PAACE */
> +  u32 addr_bitfields;             /* See P/S PAACE_AF_* */
> +
> +  /* PAACE Offset 0x08 */
> +  /* Interpretation of first 32 bits dependent on DD above */
> +  union {
> +          struct {
> +                  /* Destination ID, see PAACE_DID_* defines */
> +                  u8 did;
> +                  /* Partition ID */
> +                  u8 pid;
> +                  /* Snoop ID */
> +                  u8 snpid;
> +                  /* coherency_required : 1 reserved : 7 */

Please use this format, which is easier to read:

                        /* 1 == coherency required, 7 == reserved */

Every time I look at this comment, I think you are using bitfields.

It is meant as a pseudo-bitfield. "7 == reserved" doesn't make much sense -- that would leave a lot of other values neither defined nor explicitly reserved.

That said, the "See PAACE_DA_*" comment should be sufficient and avoids making people have to care about what bitfield ordering the comment writer was assuming.

-Scott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to