[v2] arm64: dts: mt8183: jacuzzi: Remove CrosEC base detection node

Message ID 20240112142032.909736-1-nfraprado@collabora.com
State New
Headers
Series [v2] arm64: dts: mt8183: jacuzzi: Remove CrosEC base detection node |

Commit Message

Nícolas F. R. A. Prado Jan. 12, 2024, 2:20 p.m. UTC
  The cbas node is used to describe base detection functionality in the
ChromeOS EC, which is used for units that have a detachable keyboard and
thus rely on this functionality to switch between tablet and laptop
mode.

All machines in the jacuzzi family are either clamshells (ie normal
laptops) or convertibles, meaning the keyboard can be flipped but not
detached. The detection for the keyboard getting flipped is handled by
the driver bound to the keyboard-controller node in the EC.

Since there is no base detection in these machines, and the device
backed by this node fails to probe and goes unused, delete the node from
the DT.

Fixes: 4fa8492d1e5b ("arm64: dts: mt8183: add cbas node under cros_ec")
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

---

Changes in v2:
- Moved cbas node removal to jacuzzi dtsi

 arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi | 3 +++
 1 file changed, 3 insertions(+)
  

Comments

Stephen Boyd Jan. 12, 2024, 10:40 p.m. UTC | #1
Quoting Nícolas F. R. A. Prado (2024-01-12 06:20:28)
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
> index 7592e3b86037..8c9f7435cf4c 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
> @@ -484,3 +484,6 @@ volup-btn-odl {
>         };
>  };
>
> +&cros_ec {
> +       /delete-node/ cbas;
> +};

Usually we add nodes instead of delete them. Can you move the cbas node
addition to the other files that include mt8183-kukui.dtsi instead? It
will be a larger patch in number of lines but then we don't run into
problems where the node is moved later and this breaks because 'cbas'
doesn't exist.
  
AngeloGioacchino Del Regno Jan. 15, 2024, 8:47 a.m. UTC | #2
Il 12/01/24 15:20, Nícolas F. R. A. Prado ha scritto:
> The cbas node is used to describe base detection functionality in the
> ChromeOS EC, which is used for units that have a detachable keyboard and
> thus rely on this functionality to switch between tablet and laptop
> mode.
> 
> All machines in the jacuzzi family are either clamshells (ie normal
> laptops) or convertibles, meaning the keyboard can be flipped but not
> detached. The detection for the keyboard getting flipped is handled by
> the driver bound to the keyboard-controller node in the EC.
> 
> Since there is no base detection in these machines, and the device
> backed by this node fails to probe and goes unused, delete the node from
> the DT.
> 
> Fixes: 4fa8492d1e5b ("arm64: dts: mt8183: add cbas node under cros_ec")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 

I really don't like seeing /delete-node/ statements, unless there's an
exceptionally good reason to do so, and I don't think this is the case.

Please remove cbas from the base inclusion and add it on a per-device basis.

Thanks,
Angelo
  

Patch

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
index 7592e3b86037..8c9f7435cf4c 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
@@ -484,3 +484,6 @@  volup-btn-odl {
 	};
 };
 
+&cros_ec {
+	/delete-node/ cbas;
+};