linux-kernel - Re: [PATCH 1/2] ARM: dts: imx6dl-yapp4: Fix the QCA switch register address

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: Tue, 13 Feb 2024 13:20:44 +0100
From: Michal Vokáč <michal.vokac@...ft.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Shawn Guo <shawnguo@...nel.org>, Fabio Estevam <festevam@...il.com>,
 Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>,
 Pengutronix Kernel Team <kernel@...gutronix.de>,
 NXP Linux Team <linux-imx@....com>, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 Jonathan McDowell <noodles@...th.li>
Subject: Re: [PATCH 1/2] ARM: dts: imx6dl-yapp4: Fix the QCA switch register
 address

On 12. 02. 24 17:08, Andrew Lunn wrote:
> On Mon, Feb 12, 2024 at 04:23:41PM +0100, Michal Vokáč wrote:
>> The switch address in the node name is in hex while the address in the reg
>> property is decimal which is wrong. Fix that and write the reg address
>> as a hexadecimal number.
> 
> This feels the wrong way around. The reg value is used by the kernel,
> where as the node name is not. If the reg value was wrong, the switch
> would not be found. If this file was tested, why did somebody not
> notice the switch was missing?
> 
> Do you have the hardware? Can you confirm is really does not work
> without this patch? Was 15b43e497ffd never actually tested?
Yes, I have bunch of these boards all around my desk - we manufacture
them. I am pretty sure I tested all the patches I have ever sent to
the mailing list regarding these boards.

The fact is that the switch actually works regardless of the reg value.
It worked prior to the 15b43e497ffd commit with address 0, it worked
later on with the reg value 10 and it works now with reg value 0x10.

While dealing with the broken reset gpio [1] I just noticed that in
the 15b43e497ffd commit I made a typo and the node name address value
and reg address value differ so I wanted to put that in order.

I barely remember that my motivation for creating the 15b43e497ffd
commit was that I saw similar change on the mailing list or in the git
log for the other boards using the QCA8K switch. So I "fixed" that
on our board as well.

Now when I was looking for some references I found that there is an
other board in mainline with similarly wrong setting:

arch/arm/boot/dts/qcom/qcom-ipq8064-rb3011.dts

                 switch1: switch@14 {
                         compatible = "qca,qca8337";

                         dsa,member = <1 0>;

                         pinctrl-0 = <&sw1_reset_pin>;
                         pinctrl-names = "default";

                         reset-gpios = <&qcom_pinmux 17 GPIO_ACTIVE_LOW>;
                         reg = <0x10>;

I admit that my understanding of the MDIO bus and addressing of
the connected external/internal devices is pretty limited. I have no
answer to why it works like that but as you brought up your questions
I would actually like to know as well.

Thank you!
Michal

+Cc: Jonathan McDowell
[1] https://patchwork.kernel.org/project/netdevbpf/patch/1706266175-3408-1-git-send-email-michal.vokac@ysoft.com/