Hi, Geert,
On 09.12.2024 12:09, Geert Uytterhoeven wrote:
> Hi Claudiu,
>
> On Fri, Nov 15, 2024 at 2:50 PM Claudiu <claudiu.beznea@xxxxxxxxx> wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>>
>> There are different switches available on both the RZ/G3S SMARC Module and
>> RZ SMARC Carrier II boards. These switches are used to route different SoC
>> signals to different parts available on board.
>>
>> These switches are described in device trees through macros. These macros
>> are set accordingly such that the resulted compiled dtb to describe the
>> on-board switches states.
>>
>> Based on the SW_CONFIG3 switch state (populated on the module board), the
>> SCIF3 SoC interface is routed or not to an U(S)ART pin header available on
>> the carrier board. As the SCIF3 is accessible through the carrier board,
>> the device tree enables it in the carrier DTS. To be able to cope with
>> these type of configurations, add a header file where all the on-board
>> switches can be described and shared accordingly between module and carrier
>> board.
>>
>> Commit prepares the code to enable SCIF3 on the RZ/G3S carrier device
>> tree.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> Thanks for your patch!
>
>> --- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
>> @@ -9,25 +9,7 @@
>> #include <dt-bindings/gpio/gpio.h>
>> #include <dt-bindings/pinctrl/rzg2l-pinctrl.h>
>>
>> -/*
>> - * On-board switches' states:
>> - * @SW_OFF: switch's state is OFF
>> - * @SW_ON: switch's state is ON
>> - */
>> -#define SW_OFF 0
>> -#define SW_ON 1
>> -
>> -/*
>> - * SW_CONFIG[x] switches' states:
>> - * @SW_CONFIG2:
>> - * SW_OFF - SD0 is connected to eMMC
>> - * SW_ON - SD0 is connected to uSD0 card
>> - * @SW_CONFIG3:
>> - * SW_OFF - SD2 is connected to SoC
>> - * SW_ON - SCIF1, SSI0, IRQ0, IRQ1 connected to SoC
>> - */
>> -#define SW_CONFIG2 SW_OFF
>> -#define SW_CONFIG3 SW_ON
>> +#include "rzg3s-smarc-switches.h"
>>
>> / {
>> compatible = "renesas,rzg3s-smarcm", "renesas,r9a08g045s33", "renesas,r9a08g045";
>> diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc-switches.h b/arch/arm64/boot/dts/renesas/rzg3s-smarc-switches.h
>> new file mode 100644
>> index 000000000000..e2d9b953f627
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-switches.h
>> @@ -0,0 +1,32 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>
> I agree with Rob about the license.
>
>> +/*
>> + * On-board switches for the Renesas RZ/G3S SMARC Module and RZ SMARC Carrier II
>> + * boards.
>> + *
>> + * Copyright (C) 2024 Renesas Electronics Corp.
>> + */
>> +
>> +#ifndef __RZG3S_SMARC_SWITCHES__
>> +#define __RZG3S_SMARC_SWITCHES__
>> +
>> +/*
>> + * On-board switches' states:
>> + * @SW_OFF: switch's state is OFF
>> + * @SW_ON: switch's state is ON
>> + */
>> +#define SW_OFF 0
>> +#define SW_ON 1
>> +
>> +/*
>> + * SW_CONFIG[x] switches' states:
>> + * @SW_CONFIG2:
>> + * SW_OFF - SD0 is connected to eMMC
>> + * SW_ON - SD0 is connected to uSD0 card
>> + * @SW_CONFIG3:
>> + * SW_OFF - SD2 is connected to SoC
>> + * SW_ON - SCIF3, SSI3, IRQ0, IRQ1 connected to SoC
>
> Note that the original comment above says "SCIF1, SSI0", and looking
> at the schematics (IC7 and IC8 controlled by SW_SD2_EN#), that is
> actually correct?
You're right, I'm not sure why I've changed it. I'll fix it in the next
version.
Thank you for your review,
Claudiu
>
>> + */
>> +#define SW_CONFIG2 SW_OFF
>> +#define SW_CONFIG3 SW_ON
>> +
>> +#endif /* __RZG3S_SMARC_SWITCHES__ */
>
> Gr{oetje,eeting}s,
>
> Geert
>
