On Mon, Mar 20, 2023 at 05:33:56PM +0100, Christian Marangi wrote:
>
> Btw ok for the description of the LED mapping? It's a bit complex so
> tried to do my best to describe them.
>
Yes, now it is much easier to understand the logic behind LED mapping.
Thanks for adding that! I think it will save some time for anyone who
will be working with that code in the future.
The only thing I still do not understand is the initial 14 bit shift:
> if (led->port_num == 0 || led->port_num == 4) {
> mask = QCA8K_LED_PATTERN_EN_MASK;
> val <<= QCA8K_LED_PATTERN_EN_SHIFT;
For example, according to the code above, for port 4:
- the value is shifted by 14 bits - to bits (15,14)
- mask is also set to bits (15,14)
- then, both mask and value are shifted again by 16 bits:
> return regmap_update_bits(priv->regmap, reg_info.reg,
> mask << reg_info.shift,
> val << reg_info.shift);
because reg_info.shift == QCA8K_LED_PHY4_CONTROL_RULE_SHIFT == 16 for
port_num == 4.
It means, in fact, for controlling port 4 we use bits (31,30) which
seems to be inconsistent with your comment below.
> * To control port 4:
> * - the 2 bit (17, 16) of:
> * - QCA8K_LED_CTRL0_REG for led1
> * - QCA8K_LED_CTRL1_REG for led2
> * - QCA8K_LED_CTRL2_REG for led3
> *
Are values for ports 0 and 4 correct in your description in
"qca8k_led_brightness_set()"?
Thanks,
Michal
