linux-kernel - Re: [net-next PATCH v5 02/15] net: dsa: qca8k: add LEDs basic support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 20 Mar 2023 18:48:51 +0100
From:   Michal Kubiak <michal.kubiak@...el.com>
To:     Christian Marangi <ansuelsmth@...il.com>
CC:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        "Paolo Abeni" <pabeni@...hat.com>,
        Rob Herring <robh+dt@...nel.org>,
        "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@...aro.org>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Gregory Clement <gregory.clement@...tlin.com>,
        Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
        Andy Gross <agross@...nel.org>,
        "Bjorn Andersson" <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Pavel Machek <pavel@....cz>, Lee Jones <lee@...nel.org>,
        John Crispin <john@...ozen.org>, <netdev@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-arm-msm@...r.kernel.org>, <linux-leds@...r.kernel.org>
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