diff mbox series

[2/2] dt-bindings: i2c: exynos5: add exynos-usi-hsi2c compatible

Message ID 20211101113819.50651-2-jaewon02.kim@samsung.com (mailing list archive)
State New
Headers show
Series [1/2] i2c: exynos5: support USI(Universal Serial Interface) | expand

Commit Message

Jaewon Kim Nov. 1, 2021, 11:38 a.m. UTC
This patch adds new "samsung,exynos-usi-hsi2c" compatible.
It is for i2c compatible with HSI2C available on Exynos SoC with USI.

Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
---
 Documentation/devicetree/bindings/i2c/i2c-exynos5.txt | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Krzysztof Kozlowski Nov. 3, 2021, 8:42 a.m. UTC | #1
On 01/11/2021 12:38, Jaewon Kim wrote:
> This patch adds new "samsung,exynos-usi-hsi2c" compatible.
> It is for i2c compatible with HSI2C available on Exynos SoC with USI.
> 
> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-exynos5.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

The bindings go as first patch, please.

> diff --git a/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
> index 2dbc0b62daa6..ce2373c7a357 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
> @@ -14,6 +14,8 @@ Required properties:
>  				on Exynos5260 SoCs.
>  	-> "samsung,exynos7-hsi2c", for i2c compatible with HSI2C available
>  				on Exynos7 SoCs.
> +	-> "samsung,exynos-usi-hsi2c", for i2c compatible with HSI2C available
> +				on Exynos SoCs with USI.

I would prefer to describe the Exynos model, not the feature. USI might
change between different SoCs, so then it will be "usiv2"?

>  
>    - reg: physical base address of the controller and length of memory mapped
>      region.
> @@ -31,6 +33,8 @@ Optional properties:
>         at 100khz.
>      -> If specified, the bus operates in high-speed mode only if the
>         clock-frequency is >= 1Mhz.
> +  - samsung,usi-sysreg : sysreg handle to control USI type.
> +    -> sysreg phandle for "samsung,exynos-usi-hsic" compatible.

s/sysreg/system registers controller/
s/handle/phandle/

Please document to what is this phandle. To which block.

Why it cannot be the existing generic samsung,sysreg?

>  
>  Example:
>  
> @@ -46,6 +50,8 @@ hsi2c@12ca0000 {
>  	#address-cells = <1>;
>  	#size-cells = <0>;
>  
> +	samsung,usi-sysreg = <&usi_sysreg 0x28>;

This does not look correct for this compatible. We should really convert
the bindings to YAML...

> +
>  	s2mps11_pmic@66 {
>  		compatible = "samsung,s2mps11-pmic";
>  		reg = <0x66>;
> 


Best regards,
Krzysztof
Jaewon Kim Nov. 4, 2021, 8:06 a.m. UTC | #2
Hello Krzysztof

> On 01/11/2021 12:38, Jaewon Kim wrote:
> > This patch adds new "samsung,exynos-usi-hsi2c" compatible.
> > It is for i2c compatible with HSI2C available on Exynos SoC with USI.
> >
> > Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
> > ---
> >  Documentation/devicetree/bindings/i2c/i2c-exynos5.txt | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> 
> The bindings go as first patch, please.

Okay, I will change patch order in next.

> 
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
> > b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
> > index 2dbc0b62daa6..ce2373c7a357 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
> > @@ -14,6 +14,8 @@ Required properties:
> >  				on Exynos5260 SoCs.
> >  	-> "samsung,exynos7-hsi2c", for i2c compatible with HSI2C available
> >  				on Exynos7 SoCs.
> > +	-> "samsung,exynos-usi-hsi2c", for i2c compatible with HSI2C available
> > +				on Exynos SoCs with USI.
> 
> I would prefer to describe the Exynos model, not the feature. USI might change between different SoCs,
> so then it will be "usiv2"?
> 

Okay, I will use Exynos model instaed of feature name.
"samsung,exynosautov9-hsi2c"

> >
> >    - reg: physical base address of the controller and length of memory mapped
> >      region.
> > @@ -31,6 +33,8 @@ Optional properties:
> >         at 100khz.
> >      -> If specified, the bus operates in high-speed mode only if the
> >         clock-frequency is >= 1Mhz.
> > +  - samsung,usi-sysreg : sysreg handle to control USI type.
> > +    -> sysreg phandle for "samsung,exynos-usi-hsic" compatible.
> 
> s/sysreg/system registers controller/
> s/handle/phandle/
> 

Okay, I will change it.

> Please document to what is this phandle. To which block.
> 

Okay, I will add below description in next patch.

When using Exynos USI Block, it needs to select which type of Serial IPs(UART, SPI, I2C) to use with
system control register. So, it requires system register control and needs offset value of system register.

> Why it cannot be the existing generic samsung,sysreg?
> 

It is generic samsung,sysreg not for speical system register for USI.
I will change property name to "samsung,sysreg".

> >
> >  Example:
> >
> > @@ -46,6 +50,8 @@ hsi2c@12ca0000 {
> >  	#address-cells = <1>;
> >  	#size-cells = <0>;
> >
> > +	samsung,usi-sysreg = <&usi_sysreg 0x28>;
> 
> This does not look correct for this compatible. We should really convert the bindings to YAML...
> 

I will remove this property example.

> > +
> >  	s2mps11_pmic@66 {
> >  		compatible = "samsung,s2mps11-pmic";
> >  		reg = <0x66>;
> >
> 
> 
> Best regards,
> Krzysztof

Thanks
Jaewon Kim
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
index 2dbc0b62daa6..ce2373c7a357 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
@@ -14,6 +14,8 @@  Required properties:
 				on Exynos5260 SoCs.
 	-> "samsung,exynos7-hsi2c", for i2c compatible with HSI2C available
 				on Exynos7 SoCs.
+	-> "samsung,exynos-usi-hsi2c", for i2c compatible with HSI2C available
+				on Exynos SoCs with USI.
 
   - reg: physical base address of the controller and length of memory mapped
     region.
@@ -31,6 +33,8 @@  Optional properties:
        at 100khz.
     -> If specified, the bus operates in high-speed mode only if the
        clock-frequency is >= 1Mhz.
+  - samsung,usi-sysreg : sysreg handle to control USI type.
+    -> sysreg phandle for "samsung,exynos-usi-hsic" compatible.
 
 Example:
 
@@ -46,6 +50,8 @@  hsi2c@12ca0000 {
 	#address-cells = <1>;
 	#size-cells = <0>;
 
+	samsung,usi-sysreg = <&usi_sysreg 0x28>;
+
 	s2mps11_pmic@66 {
 		compatible = "samsung,s2mps11-pmic";
 		reg = <0x66>;