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

[Jaewon Kim]

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@xxxxxxxxxxx>
> > ---
> >  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