On Wed, Mar 25, 2015 at 11:19:53AM +0800, Jun Nie wrote:
> +static u8 arm_reg[] = {
> + /* Remap, origin */
> + 0x00, /* 0x00 */
> + 0x04, /* 0x04 */
> + 0x08, /* 0x08 */
> + 0x0c, /* 0x0c */
> + 0x10, /* 0x10 */
> + 0x14, /* 0x14 */
> + 0x18, /* 0x18 */
> + 0x2c, /* 0x1c */
> + 0x20, /* 0x20 */
> + 0x24, /* 0x24 */
> + 0x28, /* 0x28 */
> + 0x2c, /* 0x2c */
> + 0x30, /* 0x30 */
> + 0x34, /* 0x34 */
> + 0x38, /* 0x38 */
> + 0x3c, /* 0x3c */
> + 0x40, /* 0x40 */
> + 0x44, /* 0x44 */
> + 0x48, /* 0x48 */
> +};
NAK, this is horrible. It's really hard to spot what the difference is
between the table above and the table below:
> +static u8 st_reg[] = {
> + /* Remap, origin */
> + 0x00, /* 0x00 */
> + 0x04, /* 0x04 */
> + 0x08, /* 0x08 */
> + 0x0C, /* 0x0c */
> + 0x10, /* 0x10 */
> + 0x14, /* 0x14 */
> + 0x18, /* 0x18 */
> + 0x1c, /* 0x1c */
> + 0x20, /* 0x20 */
> + 0x24, /* 0x24 */
> + 0x28, /* 0x28 */
> + 0x2c, /* 0x2c */
> + 0x30, /* 0x30 */
> + 0x34, /* 0x34 */
> + 0x38, /* 0x38 */
> + 0x3c, /* 0x3c */
> + 0x40, /* 0x40 */
> + 0x44, /* 0x44 */
> + 0x48, /* 0x48 */
> +};
It's only after you spot that the register offset for 0x1c is different
that you can see it.
This is bad. Please put a comment above the tables explaining what the
difference is. Also, please use the [offset] = value, syntax to
initialise these so that they're self-documenting, and are not just a
series of numbers.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
