diff mbox series

[13/13] MIPS: mobileye: eyeq5: add resets to I2C controllers

Message ID 20240215-mbly-i2c-v1-13-19a336e91dca@bootlin.com
State New
Headers
Series Add Mobileye EyeQ5 support to the Nomadik I2C controller & use hrtimers for timeouts |

Commit Message

Théo Lebrun Feb. 15, 2024, 4:52 p.m. UTC 
  Add resets properties to each I2C controller. This depends on the
reset-eyeq5 platform reset controller driver.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 arch/mips/boot/dts/mobileye/eyeq5.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Krzysztof Kozlowski Feb. 16, 2024, 7:59 a.m. UTC | #1 
On 15/02/2024 17:52, Théo Lebrun wrote:
> Add resets properties to each I2C controller. This depends on the
> reset-eyeq5 platform reset controller driver.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---

This should be squashed with previous patch adding i2c controllers.
Don't add incomplete nodes just to fix them in next patch.

Best regards,
Krzysztof
Théo Lebrun Feb. 16, 2024, 9:05 a.m. UTC | #2 
Hello,

On Fri Feb 16, 2024 at 8:59 AM CET, Krzysztof Kozlowski wrote:
> On 15/02/2024 17:52, Théo Lebrun wrote:
> > Add resets properties to each I2C controller. This depends on the
> > reset-eyeq5 platform reset controller driver.
> > 
> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> > ---
>
> This should be squashed with previous patch adding i2c controllers.
> Don't add incomplete nodes just to fix them in next patch.

The goal was to isolate reset phandles to a single patch. The series
with this patch dropped works because resets in their default state are
deasserted, so this isn't a fix. And it allows testing the series on
hardware with only the base platform series, which I found useful.

Noted, I'll be squashed for next revision.

Regards,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Krzysztof Kozlowski Feb. 16, 2024, 9:17 a.m. UTC | #3 
On 16/02/2024 10:05, Théo Lebrun wrote:
> Hello,
> 
> On Fri Feb 16, 2024 at 8:59 AM CET, Krzysztof Kozlowski wrote:
>> On 15/02/2024 17:52, Théo Lebrun wrote:
>>> Add resets properties to each I2C controller. This depends on the
>>> reset-eyeq5 platform reset controller driver.
>>>
>>> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
>>> ---
>>
>> This should be squashed with previous patch adding i2c controllers.
>> Don't add incomplete nodes just to fix them in next patch.
> 
> The goal was to isolate reset phandles to a single patch. The series

That was what you did, not the goal. If that's the goal, then it is
clearly wrong.

> with this patch dropped works because resets in their default state are
> deasserted, so this isn't a fix. And it allows testing the series on
> hardware with only the base platform series, which I found useful.

Series or half-of-series? Anyway, commits must be logical chunks, so one
chunk is to add I2C controllers, not "part of I2C controllers". DTS is
also independent of drivers (and it will go via different trees!), so
whatever dependency you think of, it does not exist.

Best regards,
Krzysztof
Théo Lebrun Feb. 16, 2024, 10:26 a.m. UTC | #4 
Hello,

On Fri Feb 16, 2024 at 10:17 AM CET, Krzysztof Kozlowski wrote:
> On 16/02/2024 10:05, Théo Lebrun wrote:
> > Hello,
> > 
> > On Fri Feb 16, 2024 at 8:59 AM CET, Krzysztof Kozlowski wrote:
> >> On 15/02/2024 17:52, Théo Lebrun wrote:
> >>> Add resets properties to each I2C controller. This depends on the
> >>> reset-eyeq5 platform reset controller driver.
> >>>
> >>> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> >>> ---
> >>
> >> This should be squashed with previous patch adding i2c controllers.
> >> Don't add incomplete nodes just to fix them in next patch.
> > 
> > The goal was to isolate reset phandles to a single patch. The series
>
> That was what you did, not the goal. If that's the goal, then it is
> clearly wrong.
>
> > with this patch dropped works because resets in their default state are
> > deasserted, so this isn't a fix. And it allows testing the series on
> > hardware with only the base platform series, which I found useful.
>
> Series or half-of-series? Anyway, commits must be logical chunks, so one
> chunk is to add I2C controllers, not "part of I2C controllers". DTS is
> also independent of drivers (and it will go via different trees!), so
> whatever dependency you think of, it does not exist.

My reasoning was focused on my point-of-view as a contributor and tester
of the series. Your explanation makes sense; I had never thought this
through from the maintainer's POV.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
diff mbox series

Patch

diff --git a/arch/mips/boot/dts/mobileye/eyeq5.dtsi b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
index d27e164f0fc1..c0842836fcc8 100644
--- a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
+++ b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
@@ -80,6 +80,7 @@  i2c0: i2c@300000 {
 			#size-cells = <0>;
 			clocks = <&i2c_ser_clk>, <&i2c_clk>;
 			clock-names = "i2cclk", "apb_pclk";
+			resets = <&reset 0 13>;
 			mobileye,olb = <&olb>;
 			mobileye,id = <0>;
 		};
@@ -94,6 +95,7 @@  i2c1: i2c@400000 {
 			#size-cells = <0>;
 			clocks = <&i2c_ser_clk>, <&i2c_clk>;
 			clock-names = "i2cclk", "apb_pclk";
+			resets = <&reset 0 14>;
 			mobileye,olb = <&olb>;
 			mobileye,id = <1>;
 		};
@@ -108,6 +110,7 @@  i2c2: i2c@500000 {
 			#size-cells = <0>;
 			clocks = <&i2c_ser_clk>, <&i2c_clk>;
 			clock-names = "i2cclk", "apb_pclk";
+			resets = <&reset 0 15>;
 			mobileye,olb = <&olb>;
 			mobileye,id = <2>;
 		};
@@ -122,6 +125,7 @@  i2c3: i2c@600000 {
 			#size-cells = <0>;
 			clocks = <&i2c_ser_clk>, <&i2c_clk>;
 			clock-names = "i2cclk", "apb_pclk";
+			resets = <&reset 0 16>;
 			mobileye,olb = <&olb>;
 			mobileye,id = <3>;
 		};
@@ -136,6 +140,7 @@  i2c4: i2c@700000 {
 			#size-cells = <0>;
 			clocks = <&i2c_ser_clk>, <&i2c_clk>;
 			clock-names = "i2cclk", "apb_pclk";
+			resets = <&reset 0 17>;
 			mobileye,olb = <&olb>;
 			mobileye,id = <4>;
 		};