diff mbox

[v5,4/4] ARM: dts: Add Ethernet chip to SMDK5410

Message ID 4568ad982f0af1846994119961cbb798edff311c.1446542020.git.p.fedin@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Pavel Fedin Nov. 3, 2015, 9:16 a.m. UTC
The chip is smsc9115, connected via SROMc bank 3. Additionally, some GPIO
initialization is required.

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 arch/arm/boot/dts/exynos5410-smdk5410.dts | 40 +++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

Comments

Krzysztof Kozlowski Nov. 4, 2015, 8:28 a.m. UTC | #1
On 03.11.2015 18:16, Pavel Fedin wrote:
> The chip is smsc9115, connected via SROMc bank 3. Additionally, some GPIO
> initialization is required.
> 
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5410-smdk5410.dts | 40 +++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/exynos5410-smdk5410.dts b/arch/arm/boot/dts/exynos5410-smdk5410.dts
> index cebeaab..f41952f 100644
> --- a/arch/arm/boot/dts/exynos5410-smdk5410.dts
> +++ b/arch/arm/boot/dts/exynos5410-smdk5410.dts
> @@ -61,6 +61,27 @@
>  	disable-wp;
>  };
>  
> +&pinctrl_0 {
> +	srom_ctl: srom-ctl {
> +		samsung,pins = "gpy0-3", "gpy0-4", "gpy0-5",
> +			       "gpy1-0", "gpy1-1", "gpy1-2", "gpy1-3";
> +		samsung,pin-function = <2>;
> +		samsung,pin-drv = <0>;
> +	};
> +
> +	srom_ebi: srom-ebi {
> +		samsung,pins = "gpy3-0", "gpy3-1", "gpy3-2", "gpy3-3",
> +			       "gpy3-4", "gpy3-5", "gpy3-6", "gpy3-7",
> +			       "gpy5-0", "gpy5-1", "gpy5-2", "gpy5-3",
> +			       "gpy5-4", "gpy5-5", "gpy5-6", "gpy5-7",
> +			       "gpy6-0", "gpy6-1", "gpy6-2", "gpy6-3",
> +			       "gpy6-4", "gpy6-5", "gpy6-6", "gpy6-7";
> +		samsung,pin-function = <2>;
> +		samsung,pin-pud = <3>;
> +		samsung,pin-drv = <0>;
> +	};
> +};
> +
>  &uart0 {
>  	status = "okay";
>  };
> @@ -72,3 +93,22 @@
>  &uart2 {
>  	status = "okay";
>  };
> +
> +&sromc {

Put sromc in alphabetical order.

> +	pinctrl-names = "default";
> +	pinctrl-0 = <&srom_ctl>, <&srom_ebi>;
> +
> +	ethernet@3 {
> +		compatible = "smsc,lan9115";
> +		reg = <3 0 0x10000>;
> +		phy-mode = "mii";
> +		interrupt-parent = <&gpx0>;
> +		interrupts = <5 8>;

s/8/IRQ_TYPE_LEVEL_LOW/
(is this really level low interrupt?)

> +		reg-io-width = <2>;
> +		smsc,irq-push-pull;
> +		smsc,force-internal-phy;
> +
> +		samsung,srom-page-mode = <1>;
> +		samsung,srom-timing = <9 12 1 9 1 1>;

Some other DTS include regulators: vddvario-supply and vdd33a-supply. It
seems that they are not described in SMSC911x bindings but in
GPMC-eth... but the smsc911x driver is requesting them. Could you
investigate that? I think these regulators should be provided (and
SMSC911x bindings should be updated).

> +	};
> +};

I don't have the board schematics so I couldn't verify the GPIOs.
Overall looks good.

Best regards,
Krzysztof
Pavel Fedin Nov. 5, 2015, 11:14 a.m. UTC | #2
Hello!

> > +	ethernet@3 {
> > +		compatible = "smsc,lan9115";
> > +		reg = <3 0 0x10000>;
> > +		phy-mode = "mii";
> > +		interrupt-parent = <&gpx0>;
> > +		interrupts = <5 8>;
> 
> s/8/IRQ_TYPE_LEVEL_LOW/
> (is this really level low interrupt?)

 Yes, according to: https://github.com/AndreiLux/Perseus-S3/blob/master/arch/arm/mach-exynos/mach-smdk5250.c#L133

> Some other DTS include regulators: vddvario-supply and vdd33a-supply. It
> seems that they are not described in SMSC911x bindings but in
> GPMC-eth... but the smsc911x driver is requesting them. Could you
> investigate that? I think these regulators should be provided (and
> SMSC911x bindings should be updated).

 Sorry, i cannot. The board has lots of jumpers, which choose between fixed voltage and regulators for different components,
according to board's manual, but the manual is very poor IMHO, and i don't understand how to use them. And i use default,
fixed-voltage configuration.
 One of my colleagues tried to get it working, but failed. It actually requires more time, and IIRC you need to bring up i2c before
you can drive regulators.
 So can we leave it as it is for now? At least it works, and this is much better than no ethernet support at all.

> I don't have the board schematics so I couldn't verify the GPIOs.

 Me neither, i wrote GPIO settings according to chip datasheet i have. They are actually chip-specific, but i wrote them in board
file because on different boards you may use different banks, and therefore different pins. Or, if you don't use SROMc at all, you
can configure all pins to do something else.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
Krzysztof Kozlowski Nov. 5, 2015, 11:27 a.m. UTC | #3
W dniu 05.11.2015 o 20:14, Pavel Fedin pisze:
>  Hello!
> 
>>> +	ethernet@3 {
>>> +		compatible = "smsc,lan9115";
>>> +		reg = <3 0 0x10000>;
>>> +		phy-mode = "mii";
>>> +		interrupt-parent = <&gpx0>;
>>> +		interrupts = <5 8>;
>>
>> s/8/IRQ_TYPE_LEVEL_LOW/
>> (is this really level low interrupt?)
> 
>  Yes, according to: https://github.com/AndreiLux/Perseus-S3/blob/master/arch/arm/mach-exynos/mach-smdk5250.c#L133

Although this is different board, but okay.

> 
>> Some other DTS include regulators: vddvario-supply and vdd33a-supply. It
>> seems that they are not described in SMSC911x bindings but in
>> GPMC-eth... but the smsc911x driver is requesting them. Could you
>> investigate that? I think these regulators should be provided (and
>> SMSC911x bindings should be updated).
> 
>  Sorry, i cannot. The board has lots of jumpers, which choose between fixed voltage and regulators for different components,
> according to board's manual, but the manual is very poor IMHO, and i don't understand how to use them. And i use default,
> fixed-voltage configuration.
>  One of my colleagues tried to get it working, but failed. It actually requires more time, and IIRC you need to bring up i2c before
> you can drive regulators.
>  So can we leave it as it is for now? At least it works, and this is much better than no ethernet support at all.

Ok, that is sufficient explanation.

>> I don't have the board schematics so I couldn't verify the GPIOs.
> 
>  Me neither, i wrote GPIO settings according to chip datasheet i have. They are actually chip-specific, but i wrote them in board
> file because on different boards you may use different banks, and therefore different pins. Or, if you don't use SROMc at all, you
> can configure all pins to do something else.
> 

Makes sense.

Best regards,
Krzysztof
Pavel Fedin Nov. 5, 2015, 11:36 a.m. UTC | #4
Hello!

> >>> +	ethernet@3 {
> >>> +		compatible = "smsc,lan9115";
> >>> +		reg = <3 0 0x10000>;
> >>> +		phy-mode = "mii";
> >>> +		interrupt-parent = <&gpx0>;
> >>> +		interrupts = <5 8>;
> >>
> >> s/8/IRQ_TYPE_LEVEL_LOW/
> >> (is this really level low interrupt?)
> >
> >  Yes, according to: https://github.com/AndreiLux/Perseus-S3/blob/master/arch/arm/mach-
> exynos/mach-smdk5250.c#L133
> 
> Although this is different board, but okay.

 I am curious too, so i examined SMSC datasheet. The IRQ is fully programmable on
chip's side too, and our driver indeed defaults to active-low. You can switch it
to active-high by addint smsc,irq-active-high property. But i just copied
settings from those old Android kernels, and it just works.
 So, cleaning up and posting v6.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
diff mbox

Patch

diff --git a/arch/arm/boot/dts/exynos5410-smdk5410.dts b/arch/arm/boot/dts/exynos5410-smdk5410.dts
index cebeaab..f41952f 100644
--- a/arch/arm/boot/dts/exynos5410-smdk5410.dts
+++ b/arch/arm/boot/dts/exynos5410-smdk5410.dts
@@ -61,6 +61,27 @@ 
 	disable-wp;
 };
 
+&pinctrl_0 {
+	srom_ctl: srom-ctl {
+		samsung,pins = "gpy0-3", "gpy0-4", "gpy0-5",
+			       "gpy1-0", "gpy1-1", "gpy1-2", "gpy1-3";
+		samsung,pin-function = <2>;
+		samsung,pin-drv = <0>;
+	};
+
+	srom_ebi: srom-ebi {
+		samsung,pins = "gpy3-0", "gpy3-1", "gpy3-2", "gpy3-3",
+			       "gpy3-4", "gpy3-5", "gpy3-6", "gpy3-7",
+			       "gpy5-0", "gpy5-1", "gpy5-2", "gpy5-3",
+			       "gpy5-4", "gpy5-5", "gpy5-6", "gpy5-7",
+			       "gpy6-0", "gpy6-1", "gpy6-2", "gpy6-3",
+			       "gpy6-4", "gpy6-5", "gpy6-6", "gpy6-7";
+		samsung,pin-function = <2>;
+		samsung,pin-pud = <3>;
+		samsung,pin-drv = <0>;
+	};
+};
+
 &uart0 {
 	status = "okay";
 };
@@ -72,3 +93,22 @@ 
 &uart2 {
 	status = "okay";
 };
+
+&sromc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&srom_ctl>, <&srom_ebi>;
+
+	ethernet@3 {
+		compatible = "smsc,lan9115";
+		reg = <3 0 0x10000>;
+		phy-mode = "mii";
+		interrupt-parent = <&gpx0>;
+		interrupts = <5 8>;
+		reg-io-width = <2>;
+		smsc,irq-push-pull;
+		smsc,force-internal-phy;
+
+		samsung,srom-page-mode = <1>;
+		samsung,srom-timing = <9 12 1 9 1 1>;
+	};
+};