| Message ID | 20240127001926.495769-7-andre.draszik@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/9] clk: samsung: gs-101: drop extra empty line | expand |
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 >
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 >
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'
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' >
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.
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 --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";
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(+)