Re: [PATCH] arm64: dts: exynos: gs101: define all PERIC USI nodes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Krzysztof & Tudor,

On Fri, 8 Mar 2024 at 16:45, Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> On 07/03/2024 14:59, Tudor Ambarus wrote:
> > Universal Serial Interface (USI) supports three types of serial
> > interface such as UART, SPI and I2C. Each protocol works independently.
> > USI can be configured to work as one of these protocols. Define all the
> > USI nodes from the PERIC blocks (USI0-14), in all their possible
> > configurations. These blocks have the TX/RX FIFO depth of 64 bytes.
> >
> > Signed-off-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxx>

With Krzystof's comments addressed:

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

> > ---
> > Please note that:
> > - google,gs101-spi compatible was queued through the SPI tree:
> >   https://lore.kernel.org/linux-arm-kernel/170742731537.2266792.3851016361229293846.b4-ty@xxxxxxxxxx/
> > - SPI dma properties were marked as not requiered. Queued through the
> >   SPI tree:
> >   https://lore.kernel.org/linux-spi/170967132774.228925.1759895846287455970.b4-ty@xxxxxxxxxx/
> >
> >  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 782 +++++++++++++++++++
> >  1 file changed, 782 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> > index ee65ed9d2cfc..d7ecfbc7e440 100644
> > --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> > +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> > @@ -373,6 +373,398 @@ pinctrl_peric0: pinctrl@10840000 {
> >                       interrupts = <GIC_SPI 625 IRQ_TYPE_LEVEL_HIGH 0>;
> >               };
> >
> > +             usi1: usi@109000c0 {
> > +                     compatible = "google,gs101-usi",
> > +                                  "samsung,exynos850-usi";
>
> This should fit within 81 characters, which is pretty close to 80, so if
> there is going to be any resend/new version, please join the lines.
>
> > +                     reg = <0x109000c0 0x20>;
> > +                     ranges;
> > +                     #address-cells = <1>;
> > +                     #size-cells = <1>;
> > +                     clocks = <&cmu_peric0 CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_0>,
> > +                              <&cmu_peric0 CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_0>;
> > +                     clock-names = "pclk", "ipclk";
> > +                     samsung,sysreg = <&sysreg_peric0 0x1000>;
> > +                     status = "disabled";
> > +
> > +                     hsi2c_1: i2c@10900000 {
> > +                             compatible = "google,gs101-hsi2c",
> > +                                          "samsung,exynosautov9-hsi2c";
> > +                             reg = <0x10900000 0xc0>;
> > +                             interrupts = <GIC_SPI 635 IRQ_TYPE_LEVEL_HIGH 0>;
>
> I wonder why we use four cells in GIC...

Due to the ppi-partitions defined in the gic for the pmu-(0-2) devices.

regards,

Peter.



>
> > +                             #address-cells = <1>;
> > +                             #size-cells = <0>;
> > +                             pinctrl-names = "default";
> > +                             pinctrl-0 = <&hsi2c1_bus>;
>
> Please reverse two lines, first pinctrl-0 then pinctrl-names. I know we
> did not follow this convention till now, but at least new code can be
> correct. Also clocks should be before pinctrl, so we keep some sort of
> alphabetical order.
>
> It is anyway too late in the cycle for me to pick it up, so no need to
> hurry with resend.
>
> Best regards,
> Krzysztof
>




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux