diff mbox series

[6/9] arm64: dts: exynos: gs101: enable i2c bus 12 on gs101-oriole

Message ID 20240127001926.495769-7-andre.draszik@linaro.org
State New
Headers
Series [1/9] clk: samsung: gs-101: drop extra empty line |

Commit Message

André Draszik Jan. 27, 2024, 12:19 a.m. UTC 
  This bus has various USB-related devices attached to it.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 arch/arm64/boot/dts/exynos/google/gs101-oriole.dts | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Sam Protsenko Jan. 27, 2024, 2:58 a.m. UTC | #1 
On Fri, Jan 26, 2024 at 6:19 PM André Draszik <andre.draszik@linaro.org> wrote:
>
> This bus has various USB-related devices attached to it.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---
>  arch/arm64/boot/dts/exynos/google/gs101-oriole.dts | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts
> index cb4d17339b6b..c8f6b955cd4e 100644
> --- a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts
> +++ b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts
> @@ -72,6 +72,10 @@ eeprom: eeprom@50 {
>         };
>  };
>
> +&hsi2c_12 {
> +       status = "okay";

But there are no bus clients declared here? A bit of explanation about
how this bus is being currently used would be nice to have (in commit
message); e.g. maybe it's used in user space somehow, etc. Because
otherwise it doesn't have much sense to enable the bus with no users.

> +};
> +
>  &pinctrl_far_alive {
>         key_voldown: key-voldown-pins {
>                 samsung,pins = "gpa7-3";
> @@ -113,6 +117,11 @@ &usi8 {
>         status = "okay";
>  };
>
> +&usi12 {
> +       samsung,mode = <USI_V2_I2C>;
> +       status = "okay";
> +};
> +
>  &watchdog_cl0 {
>         timeout-sec = <30>;
>         status = "okay";
> --
> 2.43.0.429.g432eaa2c6b-goog
>
Peter Griffin Jan. 29, 2024, 9:28 a.m. UTC | #2 
On Sat, 27 Jan 2024 at 00:19, André Draszik <andre.draszik@linaro.org> wrote:
>
> This bus has various USB-related devices attached to it.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---

As Sam said, you could be a bit more verbose on what those USB devices
are on the bus as they aren't enabled in this series. But apart from
that

Reviewed-by: Peter Griffin <peter.griffin@linaro.org>

>  arch/arm64/boot/dts/exynos/google/gs101-oriole.dts | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts
> index cb4d17339b6b..c8f6b955cd4e 100644
> --- a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts
> +++ b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts
> @@ -72,6 +72,10 @@ eeprom: eeprom@50 {
>         };
>  };
>
> +&hsi2c_12 {
> +       status = "okay";
> +};
> +
>  &pinctrl_far_alive {
>         key_voldown: key-voldown-pins {
>                 samsung,pins = "gpa7-3";
> @@ -113,6 +117,11 @@ &usi8 {
>         status = "okay";
>  };
>
> +&usi12 {
> +       samsung,mode = <USI_V2_I2C>;
> +       status = "okay";
> +};
> +
>  &watchdog_cl0 {
>         timeout-sec = <30>;
>         status = "okay";
> --
> 2.43.0.429.g432eaa2c6b-goog
>
André Draszik Jan. 29, 2024, 10:40 a.m. UTC | #3 
Hi Sam,

On Fri, 2024-01-26 at 20:58 -0600, Sam Protsenko wrote:
> On Fri, Jan 26, 2024 at 6:19 PM André Draszik <andre.draszik@linaro.org> wrote:
> > 
> > This bus has various USB-related devices attached to it.
> > 
> > [...]
> > 
> > +&hsi2c_12 {
> > +       status = "okay";
> 
> But there are no bus clients declared here? A bit of explanation about
> how this bus is being currently used would be nice to have (in commit
> message); e.g. maybe it's used in user space somehow, etc. Because
> otherwise it doesn't have much sense to enable the bus with no users.

As per the commit message, there are devices, but:
* most or all don't have an upstream driver at this stage
* it does make sense to enable the bus, as enabling it allows working on
  the drivers for the devices that are attached to this bus

Cheers,
Andre'
Sam Protsenko Jan. 29, 2024, 4:34 p.m. UTC | #4 
On Mon, Jan 29, 2024 at 4:40 AM André Draszik <andre.draszik@linaro.org> wrote:
>
> Hi Sam,
>
> On Fri, 2024-01-26 at 20:58 -0600, Sam Protsenko wrote:
> > On Fri, Jan 26, 2024 at 6:19 PM André Draszik <andre.draszik@linaro.org> wrote:
> > >
> > > This bus has various USB-related devices attached to it.
> > >
> > > [...]
> > >
> > > +&hsi2c_12 {
> > > +       status = "okay";
> >
> > But there are no bus clients declared here? A bit of explanation about
> > how this bus is being currently used would be nice to have (in commit
> > message); e.g. maybe it's used in user space somehow, etc. Because
> > otherwise it doesn't have much sense to enable the bus with no users.
>
> As per the commit message, there are devices, but:
> * most or all don't have an upstream driver at this stage
> * it does make sense to enable the bus, as enabling it allows working on
>   the drivers for the devices that are attached to this bus
>

Then can you please add the corresponding TODO comment on top of the
code you added in this patch? And perhaps also describe which devices
you have on the bus in commit message.

> Cheers,
> Andre'
>
André Draszik Jan. 29, 2024, 5:35 p.m. UTC | #5 
On Mon, 2024-01-29 at 09:28 +0000, Peter Griffin wrote:
> As Sam said, you could be a bit more verbose on what those USB devices
> are on the bus as they aren't enabled in this series. But apart from

Done.

A.
André Draszik Jan. 29, 2024, 5:35 p.m. UTC | #6 
On Mon, 2024-01-29 at 10:34 -0600, Sam Protsenko wrote:
> Then can you please add the corresponding TODO comment on top of the
> code you added in this patch? And perhaps also describe which devices
> you have on the bus in commit message.

Done.

Cheers,
Andre'
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts
index cb4d17339b6b..c8f6b955cd4e 100644
--- a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts
+++ b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts
@@ -72,6 +72,10 @@  eeprom: eeprom@50 {
 	};
 };
 
+&hsi2c_12 {
+	status = "okay";
+};
+
 &pinctrl_far_alive {
 	key_voldown: key-voldown-pins {
 		samsung,pins = "gpa7-3";
@@ -113,6 +117,11 @@  &usi8 {
 	status = "okay";
 };
 
+&usi12 {
+	samsung,mode = <USI_V2_I2C>;
+	status = "okay";
+};
+
 &watchdog_cl0 {
 	timeout-sec = <30>;
 	status = "okay";