Messages in this thread | | | From | ChiaEn Wu <> | Date | Tue, 26 Jul 2022 19:45:59 +0800 | Subject | Re: [PATCH v6 11/13] leds: rgb: mt6370: Add MediaTek MT6370 current sink type LED Indicator support |
| |
On Mon, Jul 25, 2022 at 4:41 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: ... > > From: ChiYuan Huang <cy_huang@richtek.com> > > ^^^^ (Note this and read below)
...
> In conjunction with above what SoB of Alice means? > > You really need to take your time and (re-)read > https://www.kernel.org/doc/html/latest/process/submitting-patches.html.
Hi Andy,
Thanks for your reply. We are very sorry for this mistake. We will revise it in the next patch.
> > ... > > > + * Author: Alice Chen <alice_chen@richtek.com> > > + * Author: ChiYuan Huang <cy_huang@richtek.com> > > Would > * Authors: > * Name_of_Author 1 > * Name_of_Author 2 > > work for you?
It looks good, thanks! We will apply this in the next patch.
...
> > +struct mt6370_led { > > + union { > > + struct led_classdev isink; > > + struct led_classdev_mc mc; > > + }; > > Where is the field that makes union work?
Just for saving memory space. Because these led_classdevs do not be used at the same time. Or do you think it would be better to rewrite it as follows? ------------------------------------------------------------------------------------- struct mt6370_led { struct led_classdev isink; struct led_classdev_mc mc; struct mt6370_priv *priv; u32 default_state; u32 index; }; -------------------------------------------------------------------------------------
...
> > +static int mt6370_gen_breath_pattern(struct mt6370_priv *priv, > > + struct led_pattern *pattern, u32 len, > > + u8 *pattern_val, u32 val_len) > > +{ > > + enum mt6370_led_ranges sel_range; > > + struct led_pattern *curr; > > + unsigned int sel; > > + u8 val[P_MAX_PATTERNS / 2] = {}; > > + int i; > > + > > + if (len < P_MAX_PATTERNS && val_len < P_MAX_PATTERNS / 2) > > + return -EINVAL; > > + > > + /* > > + * Pattern list > > + * tr1: byte 0, b'[7: 4] > > + * tr2: byte 0, b'[3: 0] > > + * tf1: byte 1, b'[7: 4] > > + * tf2: byte 1, b'[3: 0] > > + * ton: byte 2, b'[7: 4] > > + * toff: byte 2, b'[3: 0] > > + */ > > + for (i = 0; i < P_MAX_PATTERNS; i++) { > > + curr = pattern + i; > > + > > + sel_range = i == P_LED_TOFF ? R_LED_TOFF : R_LED_TRFON; > > + > > + linear_range_get_selector_within(priv->ranges + sel_range, > > + curr->delta_t, &sel); > > + > > + val[i / 2] |= sel << (4 * ((i + 1) % 2)); > > + } > > + > > + memcpy(pattern_val, val, 3); > > Isn't it something like put_unaligned_be24()/put_unaligned_le24()?
OK, we will try to apply this method in the next patch. Thank you so much for reviewing our patches so many times and providing so many great suggestions!
-- Best Regards, ChiaEn Wu
| |