Messages in this thread | | | Date | Mon, 20 Mar 2023 18:48:51 +0100 | From | Michal Kubiak <> | Subject | Re: [net-next PATCH v5 02/15] net: dsa: qca8k: add LEDs basic support |
| |
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
| |