vrahane commented on a change in pull request #1122: Lp5523 improvements
URL: https://github.com/apache/mynewt-core/pull/1122#discussion_r191274604
##########
File path: hw/drivers/lp5523/src/lp5523.c
##########
@@ -106,22 +109,20 @@ lp5523_get_reg(struct led_itf *itf, enum
lp5523_registers addr,
}
static int
-lp5523_set_2_regs(struct led_itf *itf, enum lp5523_registers addr,
- uint8_t vals[2])
+lp5523_set_n_regs(struct led_itf *itf, enum lp5523_registers addr,
+ uint8_t *vals, uint8_t len)
{
int rc;
- uint8_t regs[3];
-
- regs[0] = addr ;
- regs[1] = vals[0];
- regs[2] = vals[1];
+ uint8_t regs[10] = {addr, 0, 0, 0, 0, 0, 0, 0, 0, 0};
Review comment:
`memset()` does the same thing as this above. Basically multiple store
instructions, one for each byte. 10 is the max payload limit for the driver.
that is the maximum anyone would ever require using this driver. I am defining
that to be `LP5523_MAX_PAYLOAD`.
On dis-assembly what looks better is this piece of code:
````
111 static int
112 lp5523_set_n_regs(struct led_itf *itf, enum lp5523_registers addr,
113 uint8_t *vals, uint8_t len)
114 {
115 int rc;
~ 116 uint8_t regs[LP5523_MAX_PAYLOAD] = {0};
117
118 struct hal_i2c_master_data data_struct = {
119 .address = itf->li_addr,
120 .len = len + 1,
121 .buffer = regs
122 };
123
124 memcpy(regs, vals, len + 1);
125
+ 126 regs[0] = addr;
````
Assembly:
````
2189 uint8_t regs[LP5523_MAX_PAYLOAD] = {0};
2190 c710: 2100 movs r1, #0
2191 c712: 9103 str r1, [sp, #12]
2192 c714: 9104 str r1, [sp, #16]
2193 c716: f8ad 1014 strh.w r1, [sp, #20]
2194
2195 struct hal_i2c_master_data data_struct = {
2196 c71a: 8881 ldrh r1, [r0, #4]
2197 c71c: f88d 1004 strb.w r1, [sp, #4]
2198 c720: 1c59 adds r1, r3, #1
2199 c722: f8ad 1006 strh.w r1, [sp, #6]
2200 c726: a803 add r0, sp, #12
2201 c728: 9002 str r0, [sp, #8]
2202 .address = itf->li_addr,
2203 .len = len + 1,
2204 .buffer = regs
2205 };
2206
2207 memcpy(regs, vals, len + 1);
2208 c72a: 4611 mov r1, r2
2209 c72c: 1c5a adds r2, r3, #1
2210 c72e: f002 fa17 bl eb60 <memcpy>
2211
2212 regs[0] = addr;
2213 c732: f88d 500c strb.w r5, [sp, #12]
````
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
