Re: [PATCH V4 2/2] arm64: dts: qcom: Enable SD card for Shikra EVK

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Redundant and incorrectly ordered `#include <dt-bindings/gpio/gpio.h>` in board .dts files.
--

--- Patch [2]: [PATCH V4 2/2] arm64: dts: qcom: Enable SD card for Shikra EVK ---
commit 7a9a37a5e1e99fa00a175a6837969ca251cbb0d3
Author: Monish Chunara <monish.chunara@oss.qualcomm.com>

arm64: dts: qcom: Enable SD card for Shikra EVK

Enable SD card for Shikra CQS, CQM and IQS EVK variants. Configure the
vmmc/vqmmc regulators and gpio-based card detection for each board
variant.

> diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts b/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts
> index 0a52ab9b7a4c3..8e4656a51371f 100644
> --- a/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts
> +++ b/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts
> @@ -7,6 +7,7 @@
>  
>  #include "shikra-cqm-som.dtsi"
>  #include "shikra-evk.dtsi"
> +#include <dt-bindings/gpio/gpio.h>

[Severity: Low]
Would it be better to move this include directive into shikra-evk.dtsi?

Since shikra-evk.dtsi is the file that actually uses GPIO_ACTIVE_LOW, placing
the include here after shikra-evk.dtsi is processed means it cannot satisfy
the macro dependency directly. The build only succeeds because the preceding
SOM include already happens to include gpio.h.

[ ... ]
> diff --git a/arch/arm64/boot/dts/qcom/shikra-evk.dtsi b/arch/arm64/boot/dts/qcom/shikra-evk.dtsi
> index 6eb4184f76422..177a599cd2c24 100644
> --- a/arch/arm64/boot/dts/qcom/shikra-evk.dtsi
> +++ b/arch/arm64/boot/dts/qcom/shikra-evk.dtsi
> @@ -9,6 +9,17 @@ &qupv3_0 {
>  	status = "okay";
>  };
>  
> +&sdhc_2 {
> +	pinctrl-0 = <&sdc2_default &sdc2_card_det_n>;
> +	pinctrl-1 = <&sdc2_sleep &sdc2_card_det_n>;
> +	pinctrl-names = "default", "sleep";
> +
> +	no-sdio;
> +	no-mmc;
> +
> +	cd-gpios = <&tlmm 89 GPIO_ACTIVE_LOW>;

[Severity: Low]
Should shikra-evk.dtsi include <dt-bindings/gpio/gpio.h> directly to
provide the definition for GPIO_ACTIVE_LOW used here?

> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804122054.3462145-1-monish.chunara@oss.qualcomm.com?part=2