| Message ID | 20201002192210.19967-4-l.stelmach@samsung.com |
|---|---|
| State | New |
| Headers | show |
| Series | AX88796C SPI Ethernet Adapter | expand |
On Fri, 2 Oct 2020 at 21:22, Łukasz Stelmach <l.stelmach@samsung.com> wrote: > > Add node for ax88796c ethernet chip. > > Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> > --- > arch/arm/boot/dts/exynos3250-artik5-eval.dts | 21 ++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/arch/arm/boot/dts/exynos3250-artik5-eval.dts b/arch/arm/boot/dts/exynos3250-artik5-eval.dts > index 20446a846a98..7f115c348a2a 100644 > --- a/arch/arm/boot/dts/exynos3250-artik5-eval.dts > +++ b/arch/arm/boot/dts/exynos3250-artik5-eval.dts > @@ -37,3 +37,24 @@ &mshc_2 { > &serial_2 { > status = "okay"; > }; > + > +&spi_0 { > + status = "okay"; > + cs-gpios = <&gpx3 4 GPIO_ACTIVE_LOW>, <0>; > + > + assigned-clocks = <&cmu CLK_MOUT_MPLL>, <&cmu CLK_DIV_MPLL_PRE>, <&cmu CLK_MOUT_SPI0>, <&cmu CLK_DIV_SPI0>, <&cmu CLK_DIV_SPI0_PRE>, <&cmu CLK_SCLK_SPI0>; No spaces before or after '='. > + assigned-clock-parents = <&cmu CLK_FOUT_MPLL>, <&cmu CLK_MOUT_MPLL>, <&cmu CLK_DIV_MPLL_PRE>, <&cmu CLK_MOUT_SPI0>, <&cmu CLK_DIV_SPI0>, <&cmu CLK_DIV_SPI0_PRE>; This line is still too long. Please wrap it at 80. Checkpatch should complain about it... so it seems you did not run it. Please fix all checkpatch issues. > + > + ax88796c@0 { > + compatible = "asix,ax88796c"; > + local-mac-address = [00 00 00 00 00 00]; /* Filled in by a boot-loader */ > + interrupt-parent = <&gpx2>; > + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; > + spi-max-frequency = <40000000>; > + reg = <0x0>; Put reg after compatible. Best regards, Krzysztof
On Tue, 6 Oct 2020 at 12:06, Lukasz Stelmach <l.stelmach@samsung.com> wrote: > > It was <2020-10-03 sob 12:13>, when Krzysztof Kozlowski wrote: > > On Fri, 2 Oct 2020 at 21:22, Łukasz Stelmach <l.stelmach@samsung.com> wrote: > >> > >> Add node for ax88796c ethernet chip. > >> > >> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> > >> --- > >> arch/arm/boot/dts/exynos3250-artik5-eval.dts | 21 ++++++++++++++++++++ > >> 1 file changed, 21 insertions(+) > >> > >> diff --git a/arch/arm/boot/dts/exynos3250-artik5-eval.dts b/arch/arm/boot/dts/exynos3250-artik5-eval.dts > >> index 20446a846a98..7f115c348a2a 100644 > >> --- a/arch/arm/boot/dts/exynos3250-artik5-eval.dts > >> +++ b/arch/arm/boot/dts/exynos3250-artik5-eval.dts > >> @@ -37,3 +37,24 @@ &mshc_2 { > >> &serial_2 { > >> status = "okay"; > >> }; > >> + > >> +&spi_0 { > >> + status = "okay"; > >> + cs-gpios = <&gpx3 4 GPIO_ACTIVE_LOW>, <0>; > >> + > >> + assigned-clocks = <&cmu CLK_MOUT_MPLL>, <&cmu CLK_DIV_MPLL_PRE>, <&cmu CLK_MOUT_SPI0>, <&cmu CLK_DIV_SPI0>, <&cmu CLK_DIV_SPI0_PRE>, <&cmu CLK_SCLK_SPI0>; > > > > No spaces before or after '='. > > > > You mean " = ", don't you? Ah, of course. > > >> + assigned-clock-parents = <&cmu CLK_FOUT_MPLL>, <&cmu > >> CLK_MOUT_MPLL>, <&cmu CLK_DIV_MPLL_PRE>, <&cmu CLK_MOUT_SPI0>, <&cmu > >> CLK_DIV_SPI0>, <&cmu CLK_DIV_SPI0_PRE>; > > > > This line is still too long. Please wrap it at 80. Checkpatch should > > complain about it... so it seems you did not run it. Please fix all > > checkpatch issues. > > My idea was too keep assigned-clocks and assigned-clock-parrent lines > aligned, so it is clearly visible which parrent applies to which > clock. Is it inappropriate? The line gets too long and in the existing DTSes we wrapped item by item. Solution could be to add comments, e.g.: assigned-clock-parents = <&cmu CLK_FOUT_MPLL>, <&cmu CLK_DIV_MPLL_PRE>, /* for: CLK_DIV_MPLL_PRE */ <&cmu CLK_MOUT_SPI0>, /* for: CLK_MOUT_SPI0 */ but I am not sure if dtc allows such comments. Best regards, Krzysztof
It was <2020-10-06 wto 12:17>, when Krzysztof Kozlowski wrote: > On Tue, 6 Oct 2020 at 12:06, Lukasz Stelmach <l.stelmach@samsung.com> wrote: >> It was <2020-10-03 sob 12:13>, when Krzysztof Kozlowski wrote: >>> On Fri, 2 Oct 2020 at 21:22, Łukasz Stelmach <l.stelmach@samsung.com> wrote: >>>> >>>> Add node for ax88796c ethernet chip. >>>> >>>> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> >>>> --- >>>> arch/arm/boot/dts/exynos3250-artik5-eval.dts | 21 ++++++++++++++++++++ >>>> 1 file changed, 21 insertions(+) >>>> >>>> diff --git a/arch/arm/boot/dts/exynos3250-artik5-eval.dts b/arch/arm/boot/dts/exynos3250-artik5-eval.dts >>>> index 20446a846a98..7f115c348a2a 100644 >>>> --- a/arch/arm/boot/dts/exynos3250-artik5-eval.dts >>>> +++ b/arch/arm/boot/dts/exynos3250-artik5-eval.dts >>>> @@ -37,3 +37,24 @@ &mshc_2 { >>>> &serial_2 { >>>> status = "okay"; >>>> }; >>>> + >>>> +&spi_0 { >>>> + status = "okay"; >>>> + cs-gpios = <&gpx3 4 GPIO_ACTIVE_LOW>, <0>; >>>> + >>>> + assigned-clocks = <&cmu CLK_MOUT_MPLL>, <&cmu CLK_DIV_MPLL_PRE>, <&cmu CLK_MOUT_SPI0>, <&cmu CLK_DIV_SPI0>, <&cmu CLK_DIV_SPI0_PRE>, <&cmu CLK_SCLK_SPI0>; >>> >>> No spaces before or after '='. >>> >> >> You mean " = ", don't you? > > Ah, of course. > >>>> + assigned-clock-parents = <&cmu CLK_FOUT_MPLL>, <&cmu >>>> CLK_MOUT_MPLL>, <&cmu CLK_DIV_MPLL_PRE>, <&cmu CLK_MOUT_SPI0>, <&cmu >>>> CLK_DIV_SPI0>, <&cmu CLK_DIV_SPI0_PRE>; >>> >>> This line is still too long. Please wrap it at 80. Checkpatch should >>> complain about it... so it seems you did not run it. Please fix all >>> checkpatch issues. >> >> My idea was too keep assigned-clocks and assigned-clock-parrent lines >> aligned, so it is clearly visible which parrent applies to which >> clock. Is it inappropriate? > > The line gets too long and in the existing DTSes we wrapped item by > item. Solution could be to add comments, e.g.: > assigned-clock-parents = <&cmu CLK_FOUT_MPLL>, > <&cmu CLK_DIV_MPLL_PRE>, /* for: CLK_DIV_MPLL_PRE */ > <&cmu CLK_MOUT_SPI0>, /* for: CLK_MOUT_SPI0 */ > > but I am not sure if dtc allows such comments. make dbts works fine. Changed. -- Łukasz Stelmach Samsung R&D Institute Poland Samsung Electronics
diff --git a/arch/arm/boot/dts/exynos3250-artik5-eval.dts b/arch/arm/boot/dts/exynos3250-artik5-eval.dts index 20446a846a98..7f115c348a2a 100644 --- a/arch/arm/boot/dts/exynos3250-artik5-eval.dts +++ b/arch/arm/boot/dts/exynos3250-artik5-eval.dts @@ -37,3 +37,24 @@ &mshc_2 { &serial_2 { status = "okay"; }; + +&spi_0 { + status = "okay"; + cs-gpios = <&gpx3 4 GPIO_ACTIVE_LOW>, <0>; + + assigned-clocks = <&cmu CLK_MOUT_MPLL>, <&cmu CLK_DIV_MPLL_PRE>, <&cmu CLK_MOUT_SPI0>, <&cmu CLK_DIV_SPI0>, <&cmu CLK_DIV_SPI0_PRE>, <&cmu CLK_SCLK_SPI0>; + assigned-clock-parents = <&cmu CLK_FOUT_MPLL>, <&cmu CLK_MOUT_MPLL>, <&cmu CLK_DIV_MPLL_PRE>, <&cmu CLK_MOUT_SPI0>, <&cmu CLK_DIV_SPI0>, <&cmu CLK_DIV_SPI0_PRE>; + + ax88796c@0 { + compatible = "asix,ax88796c"; + local-mac-address = [00 00 00 00 00 00]; /* Filled in by a boot-loader */ + interrupt-parent = <&gpx2>; + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; + spi-max-frequency = <40000000>; + reg = <0x0>; + reset-gpios = <&gpe0 2 GPIO_ACTIVE_LOW>; + controller-data { + samsung,spi-feedback-delay = <2>; + }; + }; +};
Add node for ax88796c ethernet chip. Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> --- arch/arm/boot/dts/exynos3250-artik5-eval.dts | 21 ++++++++++++++++++++ 1 file changed, 21 insertions(+)