Re: [PATCH 11/14] arm64: renesas: rzg3s-smarc-som: Invert the logic for SW_SD2_EN macro
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2023-12-06 10:33:35
Also in:
linux-clk, linux-devicetree, linux-gpio, linux-renesas-soc, lkml, netdev
Hi Claudiu, On Mon, Nov 20, 2023 at 8:03 AM Claudiu [off-list ref] wrote:
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
The intention of SW_SD2_EN macro was to reflect the state of SW_CONFIG3
switch available on RZ/G3S Smarc Module. According to documentation SD2
is enabled when switch is in OFF state. For this, changed the logic of
marco to map value 0 to switch's OFF state and value 1 to switch's ON
state. Along with this update the description for each state for better
understanding.
The value of SW_SD2_EN macro was not changed in file because, according to
documentation, the default state for this switch is ON.
Fixes: adb4f0c5699c ("arm64: dts: renesas: Add initial support for RZ/G3S SMARC SoM")
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>Thanks for your patch!
quoted hunk ↗ jump to hunk
--- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi@@ -14,8 +14,8 @@ * 0 - SD0 is connected to eMMC * 1 - SD0 is connected to uSD0 card * @SW_SD2_EN: - * 0 - SCIF1, SSI0, IRQ0, IRQ1 connected to SoC - * 1 - SD2 is connected to SoC + * 0 - (switch OFF) SD2 is connected to SoC + * 1 - (switch ON) SCIF1, SSI0, IRQ0, IRQ1 connected to SoC
I think this is still confusing: SW_SD2_EN refers to an active-low signal (SW_SD2_EN#) in the schematics. Before, SW_SD2_EN used assertion-logic (1 is enabled), and didn't match the physical signal level. After your patch, SW_SD2_EN matches the active-low physical level, but this is not reflected in the name...
quoted hunk ↗ jump to hunk
*/ #define SW_SD0_DEV_SEL 1 #define SW_SD2_EN 1@@ -25,7 +25,7 @@ / { aliases { mmc0 = &sdhi0; -#if SW_SD2_EN +#if !SW_SD2_EN
... so this condition looks really weird.
quoted hunk ↗ jump to hunk
mmc2 = &sdhi2; #endif };@@ -116,7 +116,7 @@ &sdhi0 { }; #endif -#if SW_SD2_EN +#if !SW_SD2_EN &sdhi2 { pinctrl-0 = <&sdhi2_pins>; pinctrl-names = "default";
So I think SW_SD2_EN should be renamed to SW_SD2_EN_N.
Cfr. SW_ET0_EN_N on RZ/G2UL:
arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * DIP-Switch SW1 setting
arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * 1 : High; 0: Low
arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * SW1-2 :
SW_SD0_DEV_SEL (0: uSD; 1: eMMC)
arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * SW1-3 :
SW_ET0_EN_N (0: ETHER0; 1: CAN0, CAN1, SSI1, RSPI1)
arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * Please change
below macros according to SW1 setting on the SoM
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel