RE: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
On March 24, 2016 2:16:30 PM PDT, bryan.whiteh...@microchip.com wrote:
>> -Original Message-
>Hi Andrew,
>
>Sorry to bother you with this. But I'm having major difficulty with
>getting my dsa driver entry points called.
>
>Here is what I've done so far.
>
>I copied drivers/net/dsa/mv88x6060.c into
>drivers/net/dsa/mchp9352_dsa.c
>I then modified mchp9352_dsa.c as follows
>I emptied out the function bodies, and replaced them with a printk("Not
>Implemented\n");
>
>I did the same thing with net/dsa/tag_trailer.c which was copied into
>net/dsa/tag_mchp9352.c
>And function bodies were replaced with printk("Not Implemented\n");
>
>I also modified net/dsa/dsa.c, and net/dsa/slave.c, to include the
>hooks into my new tag files.
>
>My intent was to just see one of my "Not Implemented" functions called,
>and then I would focus on implementing it.
>
>But so far I have not seen any of my "Not Implemented" functions
>called.
>
>Here is what I know so far.
>
>It appears the dsa.c is not able to attach my underlying net device.
>And that seems to be due to it is unable to find the mdio_bus.
>So I modified my net device driver so that in probe it calls
>of_mdiobus_register instead of mdiobus_register.
>And of_mdiobus_register seems to be looking for some kind of phy
>definitions in the device tree, which it does not find. And so it does
>not appear to register the bus in such a way that dsa.c can connect to
>it.
If the lan9352 HW has a MDIO bus, which connects to internal switch, or PHYs
from the switch, there must be an mii_bus structure allocated and registered to
make that available.
>
>So the problem appears to be an issue with my device tree, which is
>partially shown below.
>
>&gpmc {
> status = "okay";
> ranges = <0 0 0x1000 0x0800>; // CS0: 128M
> pinctrl-names = "default";
> pinctrl-0 = <&gpmc_pins>;
> lan9352: ethernet@gpmc {
> compatible = "microchip,lan9352";
> interrupt-parent = <&gpio0>;
> interrupts = <7 8>;//7==GPIO bit 7, 8 = Active low level
> triggered.
>
> bank-width = <2>;
>
> phy-mode = "mii";
Having a phy-mode without a phy-handle property or fixed-phy subnode does not
sound correct nor useful.
Considering that you are interfaced to a built-in switch a fixed-link subnode
is needed to tell the Ethernet MAC about the link parameters. See
drivers/net/ethernet/broadcom/bcmsysport.c for an example by grepping for
fixed_phy.
>
> reg = <0 0 0x1>;
>
> reg-io-width = <4>;
> microchip,save-mac-address;
> microchip,irq-push-pull;
>
> };
>};
>
>/ {
> dsa@0 {
> compatible = "microchip,dsa";
Did you also update net/dsa/dsa.c so it tries to match this compatible string?
DSA is right now a platform device (though there is pending working from Andrew
to change that) so we need a way to probe DSA.
> #address-cells = <2>;
> #size-cells = <0>;
> dsa,ethernet = <&lan9352>;
That looks correct.
> dsa,mii-bus = <&lan9352>;
But not that one, this must point to a device tree node which is a MDIO bus
controller. It could be a sub-node to lan9352 provided that there is a way to
expose a MDIO bus controller.
Moving forward, with Andrew's latest patches included, this may become entirely
optional, but it is not just now.
--
Florian
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
> It appears the dsa.c is not able to attach my underlying net
> device. And that seems to be due to it is unable to find the
> mdio_bus.
> So I modified my net device driver so that in probe it calls
> of_mdiobus_register instead of mdiobus_register.
> And of_mdiobus_register seems to be looking for some kind of phy
> definitions in the device tree, which it does not find. And so it
> does not appear to register the bus in such a way that dsa.c can
> connect to it.
Hi Bryan
Are the sources for the ethernet driver available? I don't see them in
net-next.
There are two common ways for this to work, depending on the driver
architecture. Marvell devices have a separate mdio driver. In
kirkwood.dtsi you see:
mdio: mdio-bus@72004 {
compatible = "marvell,orion-mdio";
#address-cells = <1>;
#size-cells = <0>;
reg = <0x72004 0x84>;
interrupts = <46>;
clocks = <&gate_clk 0>;
status = "disabled";
/* add phy nodes in board file */
};
and mvmdio.c calls of_mdiobus_register() passing this device node.
The other way is that the mdio is part of the ethernet
driver. e.g. for the Freescale FEC:
&fec1 {
phy-mode = "rmii";
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_fec1>;
status = "okay";
mdio0: mdio {
#address-cells = <1>;
#size-cells = <0>;
status = "okay";
};
};
In this case, of_mdiobus_register() is passed the mdio0 device node.
> &gpmc {
> status = "okay";
> ranges = <0 0 0x1000 0x0800>; // CS0: 128M
> pinctrl-names = "default";
> pinctrl-0 = <&gpmc_pins>;
> lan9352: ethernet@gpmc {
> compatible = "microchip,lan9352";
> interrupt-parent = <&gpio0>;
> interrupts = <7 8>;//7==GPIO bit 7, 8 = Active low level
> triggered.
>
> bank-width = <2>;
>
> phy-mode = "mii";
>
> reg = <0 0 0x1>;
>
> reg-io-width = <4>;
> microchip,save-mac-address;
> microchip,irq-push-pull;
So i expect to see something like this here:
mdio0: mdio {
#address-cells = <1>;
#size-cells = <0>;
status = "okay";
};
> };
> };
>
> / {
> dsa@0 {
> compatible = "microchip,dsa";
> #address-cells = <2>;
> #size-cells = <0>;
> dsa,ethernet = <&lan9352>;
> dsa,mii-bus = <&lan9352>;
and this would be
dsa,mii-bus = <&mdio0>;
> switch@0 {
> #address-cells = <1>;
> #size-cells = <0>;
> reg = <0 0>;/* MDIO address 0, switch 0 in tree */
> port@0 {
> reg = <0>;
> label = "cpu";
> };
> port@1 {
> reg = <1>;
> label = "lan1";
> };
> port@2 {
> reg = <2>;
> label = "lan2";
> };
> };
> };
> };
Andrew
RE: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Friday, February 19, 2016 3:14 PM
> > I'm not yet sure how it attaches to the underlying Ethernet driver.
>
> The DSA core does that for you. If you look at the device tree
> binding:
>
> dsa@0 {
> compatible = "marvell,dsa";
> #address-cells = <2>;
> #size-cells = <0>;
>
> interrupts = <10>;
> dsa,ethernet = <ðernet0>;
>
> So this says which Ethernet interface to use. net/dsa/dsa.c will create a
> slave
> interface per external port of your switch. This slave is a netdev. Frames
> transmitted by this slave are fed through the tag code to add additional
> headers/trailers, and then passed to this ethernet device. Frames received
> by the ethernet again through the tag code, stripping off any headers/trails
> and demuxing to the correct slave.
Hi Andrew,
Sorry to bother you with this. But I'm having major difficulty with getting my
dsa driver entry points called.
Here is what I've done so far.
I copied drivers/net/dsa/mv88x6060.c into drivers/net/dsa/mchp9352_dsa.c
I then modified mchp9352_dsa.c as follows
I emptied out the function bodies, and replaced them with a printk("Not
Implemented\n");
I did the same thing with net/dsa/tag_trailer.c which was copied into
net/dsa/tag_mchp9352.c
And function bodies were replaced with printk("Not Implemented\n");
I also modified net/dsa/dsa.c, and net/dsa/slave.c, to include the hooks into
my new tag files.
My intent was to just see one of my "Not Implemented" functions called, and
then I would focus on implementing it.
But so far I have not seen any of my "Not Implemented" functions called.
Here is what I know so far.
It appears the dsa.c is not able to attach my underlying net device. And that
seems to be due to it is unable to find the mdio_bus.
So I modified my net device driver so that in probe it calls
of_mdiobus_register instead of mdiobus_register.
And of_mdiobus_register seems to be looking for some kind of phy definitions in
the device tree, which it does not find. And so it does not appear to register
the bus in such a way that dsa.c can connect to it.
So the problem appears to be an issue with my device tree, which is partially
shown below.
&gpmc {
status = "okay";
ranges = <0 0 0x1000 0x0800>; // CS0: 128M
pinctrl-names = "default";
pinctrl-0 = <&gpmc_pins>;
lan9352: ethernet@gpmc {
compatible = "microchip,lan9352";
interrupt-parent = <&gpio0>;
interrupts = <7 8>;//7==GPIO bit 7, 8 = Active low level
triggered.
bank-width = <2>;
phy-mode = "mii";
reg = <0 0 0x1>;
reg-io-width = <4>;
microchip,save-mac-address;
microchip,irq-push-pull;
};
};
/ {
dsa@0 {
compatible = "microchip,dsa";
#address-cells = <2>;
#size-cells = <0>;
dsa,ethernet = <&lan9352>;
dsa,mii-bus = <&lan9352>;
switch@0 {
#address-cells = <1>;
#size-cells = <0>;
reg = <0 0>;/* MDIO address 0, switch 0 in tree */
port@0 {
reg = <0>;
label = "cpu";
};
port@1 {
reg = <1>;
label = "lan1";
};
port@2 {
reg = <2>;
label = "lan2";
};
};
};
};
I modeled this from what I found in
arch/arm/boot/dts/kirkwood-mv88f6281gtw-ge.dts
So my question is. Can you see anything that I'm doing wrong?
Given the issue seems to be about the lan9352 node not having child phy nodes,
then I wonder if I'm not looking at the best example since my example does not
use child phy nodes either. If so, can you point me to a better example?
Anyway I'm really hitting a road block here, so any and all guidance is greatly
appreciated.
Thanks,
Bryan
RE: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
Thanks Andrew, Your tips are much appreciated. Bryan
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
On Fri, Feb 19, 2016 at 07:29:46PM +, bryan.whiteh...@microchip.com wrote:
> > From: Andrew Lunn [mailto:and...@lunn.ch]
> > Sent: Tuesday, February 16, 2016 5:16 PM
> >
> > You are already 1/2 way to a DSA driver, since you have a MAC driver. So i
> > agree with David, do it right and add a simple DSA driver.
> >
> Andrew,
>
> I've done a little research on DSA.
> I've read Documentation/networking/dsa/dsa.txt
> And I've looked over some examples in drivers/net/dsa/
>
> It appears there are about 40 functions to implement.
For a minimal driver, you need a lot less. mv88e6060.c is a standalone
DSA driver and has 5 functions. 250 lines of code. You will also need
a new tag implementation, named something like net/dsa/tag_lan935x.c,
but that is only two functions.
> I see examples from only 2 manufacturers, and they average more than
> 2000 lines of code.
These are more fully featured switches, and the drivers do more than
the minimum. You should be aiming for your first submission to be a
minimal driver. Expose two ports, with a PHY each, send and receive
frames on each port.
Latter patches can add more features, like EEE, MIB counters, hardware
bridging of the ports, 802.1Q, reading and writing the EEPROM
etc. These are all optional features you can add later.
> I'm not yet sure how it attaches to the underlying Ethernet driver.
The DSA core does that for you. If you look at the device tree
binding:
dsa@0 {
compatible = "marvell,dsa";
#address-cells = <2>;
#size-cells = <0>;
interrupts = <10>;
dsa,ethernet = <ðernet0>;
So this says which Ethernet interface to use. net/dsa/dsa.c will
create a slave interface per external port of your switch. This slave
is a netdev. Frames transmitted by this slave are fed through the tag
code to add additional headers/trailers, and then passed to this
ethernet device. Frames received by the ethernet again through the tag
code, stripping off any headers/trails and demuxing to the correct
slave.
> And I have no idea how I would test it at the end.
The whole concept behind this is that the ports of the switch
behaviour like normal network interfaces. So test it the same way you
test an linux ethernet driver. ping, iperf, ethtool, etc. And add some
tests using a linux bridge.
> Given these issues, it will be hard to sell it to my manager.
With time, you can expect to see more switch chips gaining mainline
support. There is interest in implementing a DSA driver for the
Micrel/Kendin KS8995M and KSZ8864RMN chips, for example, which i think
are in the same or similar market segment as the lan935x devices.
> If it can be partly implemented, which parts should be implemented first?
I would suggest you first understand what the tag_ files are about,
and look at implementing for the lan935x. The datasheet talks about a
special VLAN tag with bits 0 and 1 indicating the outgoing port, if
bit 3 is zero. Then do a minimal driver, equivalent to
mv88e6060.c. FYI the datasheet of this device is publicly available.
Andrew
RE: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
> From: Andrew Lunn [mailto:and...@lunn.ch] > Sent: Tuesday, February 16, 2016 5:16 PM > > You are already 1/2 way to a DSA driver, since you have a MAC driver. So i > agree with David, do it right and add a simple DSA driver. > Andrew, I've done a little research on DSA. I've read Documentation/networking/dsa/dsa.txt And I've looked over some examples in drivers/net/dsa/ It appears there are about 40 functions to implement. I see examples from only 2 manufacturers, and they average more than 2000 lines of code. I'm not yet sure how it attaches to the underlying Ethernet driver. And I have no idea how I would test it at the end. Given these issues, it will be hard to sell it to my manager. Since you claim it is simple. Can you explain your reasons? I've never done a DSA driver before. Given that, can you generally outline the steps. If it can be partly implemented, which parts should be implemented first? I appreciate any pointers you can provide. Bryan
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
> Even if your driver does not support tagging Looking at the data sheet, it looks like the hardware does support tagging. You add an extra "VLAN" tag, whos bits have very little to do with VLANs. So a new tag implementation will be needed, but that is not hard. Andrew
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
On 16/02/16 14:15, Andrew Lunn wrote: >> I just spoke with my manager, and we would like to change the target >> device from LAN9352 to LAN9250. The LAN9250 is the same as the >> LAN9352 but without the switch. It has one mac and one phy. > > It is not so easy to get an overview from the website, but it looks > like: > > LAN9250 - one port MAC/PHY > LAN9352 - 10/100 2-Port Managed Ethernet Switch > LAN9353 - 10/100 3-Port Managed Ethernet with Dual RMII or Single > MII/RMII/Turbo MII > LAN9354 - 10/100 3-Port Managed Ethernet Switch with Single RMII > LAN9355 - 10/100 3-Port Managed Ethernet Switch with Dual MII/RMII/Turbo MII > > So i get the feeling this product line is for switches, and the > LAN9250 is an oddball in the series. > > You are already 1/2 way to a DSA driver, since you have a MAC > driver. So i agree with David, do it right and add a simple DSA > driver. I second that, and in fact, implemeting a DSA driver will get you the per-port ethtool control knobs that you are after. Even if your driver does not support tagging (you can set DSA_PROTO_TAG_NONE to not requiring tagging), you still get all the other DSA perks: per-port network devices, ethtool statistics, PHY management, etc. The in-kernel documentation in Documentation/networking/dsa/dsa.txt is reasonably up to date -- Florian
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
> I just spoke with my manager, and we would like to change the target > device from LAN9352 to LAN9250. The LAN9250 is the same as the > LAN9352 but without the switch. It has one mac and one phy. It is not so easy to get an overview from the website, but it looks like: LAN9250 - one port MAC/PHY LAN9352 - 10/100 2-Port Managed Ethernet Switch LAN9353 - 10/100 3-Port Managed Ethernet with Dual RMII or Single MII/RMII/Turbo MII LAN9354 - 10/100 3-Port Managed Ethernet Switch with Single RMII LAN9355 - 10/100 3-Port Managed Ethernet Switch with Dual MII/RMII/Turbo MII So i get the feeling this product line is for switches, and the LAN9250 is an oddball in the series. You are already 1/2 way to a DSA driver, since you have a MAC driver. So i agree with David, do it right and add a simple DSA driver. Andrew
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
On 12.02.2016 20:10, bryan.whiteh...@microchip.com wrote:
> Lino,
>
> Regarding "a matching smp_rmb() in the irq handler"
> There is a smp_wmb() in the irq handler, since in both cases we are forcing a
> write operation on software_irq_signal.
>
> I suppose using atomic operations on software_irq_signal would also work, but
> this driver was based on
> drivers/net/ethernet/smsc/smsc911x.c
> And if possible I'd prefer to keep logical changes to a minimum.
> Plus this is not a "read modify write" scenario so I think the memory barrier
> is sufficient.
> Do you agree?
>
Hi Bryan,
youre right, smsc911x.c does the same thing and probably its ok. As far
as I have understood smp memory barriers (mainly from reading
memory-barriers.txt), they normally should be paired to ensure that a
"reader" thread actually sees what an "updater" thread writes - paired
in a sense that there is a corresponding smp_rmb() for a smp_wmb().
So in this case I expected the need for a smp_rmb() at least in that
loop in open() which waits for the software_irq_signal flag to toggle.
Something like
while (timeout--) {
smp_rmb();
if (pdata->software_irq_signal)
break;
usleep_range(1000, 1);
}
But AFAICS calling usleep_range() already implies memory barriers, so I
agree that there is probably no need for an explicit smp_rmb.
Regards,
Lino
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
From: Date: Tue, 16 Feb 2016 21:37:40 + > Again, I'd like to change our target device to LAN9250, which is just an > Ethernet controller with no switch. > > Will a driver for that device be more easily accepted? No, please just implement your driver correctly instead of trying to side-step the issue.
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
From: Date: Tue, 16 Feb 2016 21:32:20 + > I just spoke with my manager, and we would like to change the target > device from LAN9352 to LAN9250. The LAN9250 is the same as the > LAN9352 but without the switch. It has one mac and one phy. > > Since there is no switch in that product, will that make a pure > Ethernet driver easier to accept? I'm not going to allow you to weasel away from doing things correctly by doing this. You are doing this so you can still put the pure ethernet driver into the tree, and then off-tree you'll just add the device ID for the switch device. Sorry, that won't pass.
RE: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
> From: Andrew Lunn [mailto:and...@lunn.ch] > Sent: Tuesday, February 16, 2016 3:58 PM > > I believe I can make the physical phys accessible through ethertool. > > Would that be reasonable? > > How, you have no netdev to attach them to? > > There was a nice presentation at netdev last week by Mellanox about their > switches. I don't know if the video is available yet, but that shows the > correct > way to do this. > You are right. I misunderstood how ethtool works. Again, I'd like to change our target device to LAN9250, which is just an Ethernet controller with no switch. Will a driver for that device be more easily accepted?
RE: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
> From: David Miller [mailto:da...@davemloft.net] > Sent: Tuesday, February 16, 2016 3:53 PM > >> > >> I do not think, with all the infrastructure we have, that we should > >> accept pure ethernet drivers for such devices any more. > >> > >> About year ago I would have responded differently, but these days all > >> of the necessary support and infrastructure is there, and reasonable > >> easy for driver authors to use. > > > > I believe I can make the physical phys accessible through ethertool. Would > that be reasonable? > > No, we are asking you to implement DSA or switchdev support. I just spoke with my manager, and we would like to change the target device from LAN9352 to LAN9250. The LAN9250 is the same as the LAN9352 but without the switch. It has one mac and one phy. Since there is no switch in that product, will that make a pure Ethernet driver easier to accept?
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
> I believe I can make the physical phys accessible through > ethertool. Would that be reasonable? How, you have no netdev to attach them to? There was a nice presentation at netdev last week by Mellanox about their switches. I don't know if the video is available yet, but that shows the correct way to do this. Andrew
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
From: Date: Tue, 16 Feb 2016 20:48:14 + >> -Original Message- >> From: David Miller [mailto:da...@davemloft.net] >> Sent: Tuesday, February 16, 2016 3:44 PM >> To: and...@lunn.ch >> Cc: Bryan Whitehead - C21958; f.faine...@gmail.com; netdev@vger.kernel.org >> Subject: Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver >> >> From: Andrew Lunn >> Date: Tue, 16 Feb 2016 21:36:25 +0100 >> >> > So this is the discussion we need to have. >> > >> > The interface to the outside work is the two switch ports with real >> > PHYs. What you are implementing is an Ethernet driver for an internal >> > port connected to the switch. This port does not go to the outside >> > world. This driver provides no way to control the ports to the outside >> > world and you have no short term plan to actually implement control of >> > the ports connected to the outside world. >> > >> > Should the Linux community accept this driver in this state? >> > >> > I would prefer to see a simple switchdev or DSA driver which exposes >> > the two external ports. >> >> I do not think, with all the infrastructure we have, that we should accept >> pure >> ethernet drivers for such devices any more. >> >> About year ago I would have responded differently, but these days all of the >> necessary support and infrastructure is there, and reasonable easy for driver >> authors to use. > > I believe I can make the physical phys accessible through ethertool. Would > that be reasonable? No, we are asking you to implement DSA or switchdev support.
RE: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Sent: Tuesday, February 16, 2016 3:44 PM > To: and...@lunn.ch > Cc: Bryan Whitehead - C21958; f.faine...@gmail.com; netdev@vger.kernel.org > Subject: Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver > > From: Andrew Lunn > Date: Tue, 16 Feb 2016 21:36:25 +0100 > > > So this is the discussion we need to have. > > > > The interface to the outside work is the two switch ports with real > > PHYs. What you are implementing is an Ethernet driver for an internal > > port connected to the switch. This port does not go to the outside > > world. This driver provides no way to control the ports to the outside > > world and you have no short term plan to actually implement control of > > the ports connected to the outside world. > > > > Should the Linux community accept this driver in this state? > > > > I would prefer to see a simple switchdev or DSA driver which exposes > > the two external ports. > > I do not think, with all the infrastructure we have, that we should accept > pure > ethernet drivers for such devices any more. > > About year ago I would have responded differently, but these days all of the > necessary support and infrastructure is there, and reasonable easy for driver > authors to use. I believe I can make the physical phys accessible through ethertool. Would that be reasonable? Bryan
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
From: Andrew Lunn Date: Tue, 16 Feb 2016 21:36:25 +0100 > So this is the discussion we need to have. > > The interface to the outside work is the two switch ports with real > PHYs. What you are implementing is an Ethernet driver for an internal > port connected to the switch. This port does not go to the outside > world. This driver provides no way to control the ports to the outside > world and you have no short term plan to actually implement control of > the ports connected to the outside world. > > Should the Linux community accept this driver in this state? > > I would prefer to see a simple switchdev or DSA driver which exposes > the two external ports. I do not think, with all the infrastructure we have, that we should accept pure ethernet drivers for such devices any more. About year ago I would have responded differently, but these days all of the necessary support and infrastructure is there, and reasonable easy for driver authors to use.
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
On Tue, Feb 16, 2016 at 07:41:46PM +, bryan.whiteh...@microchip.com wrote: > Andrew, > > At this point, I am not tasked with implementing switch features, > which would likely take a long time to complete. So this is the discussion we need to have. The interface to the outside work is the two switch ports with real PHYs. What you are implementing is an Ethernet driver for an internal port connected to the switch. This port does not go to the outside world. This driver provides no way to control the ports to the outside world and you have no short term plan to actually implement control of the ports connected to the outside world. Should the Linux community accept this driver in this state? I would prefer to see a simple switchdev or DSA driver which exposes the two external ports. Andrew
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
So you mean mdiobus_scan()
http://lxr.free-electrons.com/source/drivers/net/phy/mdio_bus.c#L349
actually finds three PHY devices. Two are real phys with magnetics and
somewhere to attach a copper cable, and one is virtual.
This is non-obvious. I think it would be good to add some ASCII art to
the binding documentation to explain what this device looks like, and
which address on the MDIO bus maps to what phy.
There is also the issue of which way around is this phy. Typically in
a DSA setup you have:
+---+ +---+
| | RGMII | |
| +---+ +-- 1000baseT MDI ("WAN")
| | | 6-port +-- 1000baseT MDI ("LAN1")
|CPU| | ethernet +-- 1000baseT MDI ("LAN2")
| |MIImgmt| switch +-- 1000baseT MDI ("LAN3")
| +---+ w/5 PHYs +-- 1000baseT MDI ("LAN4")
| | | |
+---+ +---+
Each MDI port has a phy on it. These are your two real PHYs.
Another possible setup, but not very frequently seen is:
+---++---+
| | RGMII RGMII| |
| +---PHY -- PHY --+ +-- 1000baseT MDI
("WAN")
| || 6-port +-- 1000baseT MDI
("LAN1")
|CPU|| ethernet +-- 1000baseT MDI
("LAN2")
| |MIImgmt | switch +-- 1000baseT MDI
("LAN3")
| ++ w/5 PHYs +-- 1000baseT MDI
("LAN4")
| || |
+---++---+
In this setup, you have a short copper section between the two PHYs
copper ports, and the MAC side is connected to a MAC, be it the CPU
MAC or the switch MAC.
The switch you are talking about only has one of these PHYs. Does it
connect to the CPU MAC or the switch MAC? What use is that? Does the
link status tell you anything? Can it be down? Auto negotiation is
meaningless, since there is no link partner. Can you change the speed
to 10/half? What does ethtool show for this phy?
I think we need to understand this PHY before deciding if we need to
include it in the driver model or not. It is so odd, it might be
better to ignore it.
Thanks
Andrew
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
Please stop top-posting. Thank you.
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
From: Date: Tue, 16 Feb 2016 19:34:26 + > Hi Andrew, > > You can find my comments below wrapped in ... Do not do this. Please reply to emails appropriately, which means proper quotation and no top-posting. Look at how other people reply to postings on this list, how it is formatted, and how the replied text is placed. Thank you.
RE: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
Andrew, At this point, I am not tasked with implementing switch features, which would likely take a long time to complete. If it is too difficult to add switch features later, then they may come as an entirely new driver at that time. So this driver should only be thought of as operating a single port ethernet controller. Bryan -Original Message- From: Andrew Lunn [mailto:and...@lunn.ch] Sent: Friday, February 12, 2016 12:18 PM To: Bryan Whitehead - C21958 Cc: f.faine...@gmail.com; da...@davemloft.net; netdev@vger.kernel.org Subject: Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver > At this point my plan is to just get a basic Ethernet controller > driver submitted, and later work on switch features. They are not really separable. The linux way of dealing with switches is to model each port as a net device. So you should have a lan0 device and a lan1 device for this two port switch. You can put IP addresses on the ports, and use them as separate ports. If you want to bridge the two ports, you create a linux bridge and add the ports to the bridge. Using netdev or DSA, you can then push this configuration down to the hardware, so it performs the actual bridging of packets between ports. Andrew
RE: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
Hi Andrew, You can find my comments below wrapped in ... -Original Message- From: Andrew Lunn [mailto:and...@lunn.ch] Sent: Friday, February 12, 2016 12:11 PM To: Bryan Whitehead - C21958 Cc: da...@davemloft.net; netdev@vger.kernel.org Subject: Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver On Fri, Feb 12, 2016 at 04:51:49PM +, bryan.whiteh...@microchip.com wrote: > Hi Andrew, > > Sorry I still did not make this clear. And I'm not sure I understand your > question so I'll try to explain again, but please give me feedback if it's > still not clear. > > Also you can reference Figure 2-1 for an Internal Block Diagram on > page 9 of > http://ww1.microchip.com/downloads/en/DeviceDoc/1923A.pdf > > Conceptually I think it's easier to ignore the switch all together, since the > driver really doesn't touch it. > > Imagine we have two separate components > 1) LAN9218 (which is a 10/100 Ethernet Controller) > 2) An external 3 port switch (which is actually embedded) > > This driver only operates on the Ethernet Controller, whose phy is in reality > just a virtual phy. > That virtual phy is connected directly to the embedded switch fabric, > which has the two physical phys that you are asking about. Since this > driver only operates on the Ethernet controller and its virtual phy. I > makes no sense to talk about phy-modes for the physical phys on the > switch. So the code implements an MDIO bus and registers it with the MDIO framework. Yes It then finds the first phy and connects it to the netdev. Yes When doing this, it passes the phy-mode. Yes My assumption is, the first PHY on the MDIO bus is the PHY connected to port 0 of the switch. Yes, that is the correct assumption So you are setting the phy-mode of this port. Yes This is a real phy, not a virtual phy. >From the software point of view it is a real phy, in that there is a real phy >register set that can be read and written. But from the hardware point of >view, it is virtual because it does not control a physical phy. The following is a clip from the spec. "The Virtual PHY provides a basic MII management interface (MDIO) per EEE 802.3 (clause 22) so that a MAC with an unmodified driver can be supported as if it was attached to a single port PHY. This functionality is designed to allow easy and quick integration of the device into designs with minimal driver modifications. The Virtual PHY provides a full bank of registers which comply with the IEEE 802.3 specification. This enables the Virtual PHY to provide various status and control bits similar to those provided by a real PHY. These include the output of speed selection, duplex, loopback, isolate, collision test, and Auto-Negotiation status." Andrew
RE: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
Thanks Florian,
Lots of good comments here. I'm working on fixes and I'll respond when I get
through it all.
Bryan
-Original Message-
From: Florian Fainelli [mailto:f.faine...@gmail.com]
Sent: Thursday, February 11, 2016 9:18 PM
To: Bryan Whitehead - C21958; da...@davemloft.net
Cc: netdev@vger.kernel.org; Andrew Lunn
Subject: Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
On 11/02/16 10:58, bryan.whiteh...@microchip.com wrote:
> This is the initial submission of an ethernet driver for the Microchip
> LAN9352.
>
> The LAN9352 is a 2-Port 10/100 Managed Ethernet Switch with 16-Bit
> Non-PCI CPU Interface. The CPU interface includes a basic ethernet
> controller interface whose virtual phy is connected internally to a
> 3rd port on the embedded switch.
>
> This driver only operates as a simple ethernet controller on the CPU
> interface. Since this interface is connected directly to the embedded
> switch, the result is that traffic can be sent and received on both
> physical ports.
If this is an integrated switch, have you considered using the DSA subsystem
and having this driver just be the Ethernet driver that interfaces to your CPU
interface, but does not attempt to configure the switch in any way?
It would also help if you clarified the references to the SMSC911x controllers
and explain whether this was used for the Ethernet MAC, or how the things are
pieced together, as it seems like there could be different strategies to
support your hardware here.
>
> Signed-off-by: Bryan Whitehead
> ---
> Documentation/devicetree/bindings/net/mchp9352.txt | 31 +
> MAINTAINERS|9 +
> drivers/net/ethernet/microchip/Kconfig | 32 +-
> drivers/net/ethernet/microchip/Makefile|1 +
> drivers/net/ethernet/microchip/mchp9352.c | 2587
>
> drivers/net/ethernet/microchip/mchp9352.h | 443
> 6 files changed, 3102 insertions(+), 1 deletion(-) create mode
> 100644 Documentation/devicetree/bindings/net/mchp9352.txt
> create mode 100644 drivers/net/ethernet/microchip/mchp9352.c
> create mode 100644 drivers/net/ethernet/microchip/mchp9352.h
>
> diff --git a/Documentation/devicetree/bindings/net/mchp9352.txt
> b/Documentation/devicetree/bindings/net/mchp9352.txt
> new file mode 100644
> index 000..5b22e73
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/mchp9352.txt
> @@ -0,0 +1,31 @@
> +* Microchip LAN9352 Controller
> +
> +Required properties:
> +- compatible : Should be "microchip,lan9352"
> +- reg : Address and length of the io space for Microchip LAN
> +- interrupts : Should contain Microchip LAN interrupt line
> +- interrupt-parent : Should be the phandle for the interrupt
> +controller
This is more like an optional property, in general there is a default parent
configured in your Device Tree.
> + that services interrupts for this device
> +- phy-mode : See ethernet.txt file in the same directory
Same question as Andrew, if this is a 2-port switch, should not we have one
phy-mode per port of the switch?
> +
> +Optional properties:
> +- reg-shift : Specify the quantity to shift the register offsets by
> +- reg-io-width : Specify the size (in bytes) of the IO accesses that
> + should be performed on the device. Valid value for Microchip LAN
> +is
> + 2 or 4. If it's omitted or invalid, the size would be 2.
> +- microchip,irq-active-high : Indicates the IRQ polarity is
> +active-high
Should not that be indicated in the interrupts property as a specific cell?
> +- microchip,irq-push-pull : Indicates the IRQ type is push-pull
Same here maybe?
> +- microchip,save-mac-address : Indicates that mac address needs to be
> +saved
> + before resetting the controller
That's a software construct here, no?
> +
> +Examples:
> +
> +lan9220@f400 {
> + compatible = "microchip,lan9352";
> + reg = <0xf400 0x200>;
> + phy-mode = "mii";
> + interrupt-parent = <&gpio1>;
> + interrupts = <31>;
> + reg-io-width = <4>;
> + microchip,irq-push-pull;
> +};
> diff --git a/MAINTAINERS b/MAINTAINERS index f678c37..c39edef 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7136,6 +7136,15 @@ T: git git://git.monstr.eu/linux-2.6-microblaze.git
> S: Supported
> F: arch/microblaze/
>
> +MICROCHIP LAN9352 ETHERNET DRIVER
> +M: Microchip Linux Driver Support
> +M: Bryan Whitehead
> +L: netdev@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/net/mchp9352.txt
> +F: drivers/net/ethernet/microchip/mchp9352.h
> +F: drivers/net/ethernet/microchip/mc
RE: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
Lino,
Regarding "a matching smp_rmb() in the irq handler"
There is a smp_wmb() in the irq handler, since in both cases we are forcing a
write operation on software_irq_signal.
I suppose using atomic operations on software_irq_signal would also work, but
this driver was based on
drivers/net/ethernet/smsc/smsc911x.c
And if possible I'd prefer to keep logical changes to a minimum.
Plus this is not a "read modify write" scenario so I think the memory barrier
is sufficient.
Do you agree?
Regarding register_netdev.
I'll move register_netdev till after the mac address is set.
Thanks,
Bryan
-Original Message-
From: Lino Sanfilippo [mailto:linosanfili...@gmx.de]
Sent: Thursday, February 11, 2016 7:14 PM
To: Bryan Whitehead - C21958; da...@davemloft.net
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
Hi,
> +static int mchp9352_open(struct net_device *dev) {
> +
> + MCHP_TRACE(pdata, ifup, "Testing irq handler using IRQ %d", dev->irq);
> + pdata->software_irq_signal = 0;
> +
> + /* Testing irq handler */
> + smp_wmb();
Should not there at least be a matching smp_rmb() in the irq handler?
Maybe an atomic_t would be a better choice for a flag like software_irq_signal
here.
> +
> +static int mchp9352_drv_probe(struct platform_device *pdev)
> +
> + netif_carrier_off(dev);
> +
> + retval = register_netdev(dev);
> + if (retval) {
> + MCHP_WARN(pdata, probe, "Error %i registering device", retval);
> + goto out_free_irq;
> + } else {
> + MCHP_TRACE(pdata, probe,
> +"Network interface: \"%s\"", dev->name);
> + }
Note that as soon as the network device is registered "open" may be called so
everything should be set up already. In this case the mac address
(dev->dev_addr) is accessed in open but may not yet contain valid data when
register_netdev is called.
Regards,
Lino
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
> At this point my plan is to just get a basic Ethernet controller > driver submitted, and later work on switch features. They are not really separable. The linux way of dealing with switches is to model each port as a net device. So you should have a lan0 device and a lan1 device for this two port switch. You can put IP addresses on the ports, and use them as separate ports. If you want to bridge the two ports, you create a linux bridge and add the ports to the bridge. Using netdev or DSA, you can then push this configuration down to the hardware, so it performs the actual bridging of packets between ports. Andrew
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
On Fri, Feb 12, 2016 at 04:51:49PM +, bryan.whiteh...@microchip.com wrote: > Hi Andrew, > > Sorry I still did not make this clear. And I'm not sure I understand your > question so I'll try to explain again, but please give me feedback if it's > still not clear. > > Also you can reference Figure 2-1 for an Internal Block Diagram on page 9 of > http://ww1.microchip.com/downloads/en/DeviceDoc/1923A.pdf > > Conceptually I think it's easier to ignore the switch all together, since the > driver really doesn't touch it. > > Imagine we have two separate components > 1) LAN9218 (which is a 10/100 Ethernet Controller) > 2) An external 3 port switch (which is actually embedded) > > This driver only operates on the Ethernet Controller, whose phy is in reality > just a virtual phy. > That virtual phy is connected directly to the embedded switch > fabric, which has the two physical phys that you are asking > about. Since this driver only operates on the Ethernet controller > and its virtual phy. I makes no sense to talk about phy-modes for > the physical phys on the switch. So the code implements an MDIO bus and registers it with the MDIO framework. It then finds the first phy and connects it to the netdev. When doing this, it passes the phy-mode. My assumption is, the first PHY on the MDIO bus is the PHY connected to port 0 of the switch. So you are setting the phy-mode of this port. This is a real phy, not a virtual phy. Andrew
RE: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
Florian, I'm not familiar with the DSA subsystem. Can you refer me to documents about it? Andrew, I'm also not familiar with switchdev, or which is better in this case. Can you refer me to documents about it? At this point my plan is to just get a basic Ethernet controller driver submitted, and later work on switch features. Regards, Bryan -Original Message- From: Andrew Lunn [mailto:and...@lunn.ch] Sent: Friday, February 12, 2016 2:21 AM To: Florian Fainelli Cc: Bryan Whitehead - C21958; da...@davemloft.net; netdev@vger.kernel.org Subject: Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver On Thu, Feb 11, 2016 at 06:18:25PM -0800, Florian Fainelli wrote: > On 11/02/16 10:58, bryan.whiteh...@microchip.com wrote: > > This is the initial submission of an ethernet driver for the > > Microchip LAN9352. > > > > The LAN9352 is a 2-Port 10/100 Managed Ethernet Switch with 16-Bit > > Non-PCI CPU Interface. The CPU interface includes a basic ethernet > > controller interface whose virtual phy is connected internally to a > > 3rd port on the embedded switch. > > > > This driver only operates as a simple ethernet controller on the CPU > > interface. Since this interface is connected directly to the > > embedded switch, the result is that traffic can be sent and received > > on both physical ports. > > If this is an integrated switch, have you considered using the DSA > subsystem and having this driver just be the Ethernet driver that > interfaces to your CPU interface, but does not attempt to configure > the switch in any way? I actually think switchdev might be the better model. Hard to say. The datasheet suggests it could be modelled as a three port switch, so DSA might be appropriate... Andrew
RE: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
Hi Andrew, Sorry I still did not make this clear. And I'm not sure I understand your question so I'll try to explain again, but please give me feedback if it's still not clear. Also you can reference Figure 2-1 for an Internal Block Diagram on page 9 of http://ww1.microchip.com/downloads/en/DeviceDoc/1923A.pdf Conceptually I think it's easier to ignore the switch all together, since the driver really doesn't touch it. Imagine we have two separate components 1) LAN9218 (which is a 10/100 Ethernet Controller) 2) An external 3 port switch (which is actually embedded) This driver only operates on the Ethernet Controller, whose phy is in reality just a virtual phy. That virtual phy is connected directly to the embedded switch fabric, which has the two physical phys that you are asking about. Since this driver only operates on the Ethernet controller and its virtual phy. I makes no sense to talk about phy-modes for the physical phys on the switch. If this is still not clear please let me know. If you think you understand but my explanation could be better, then please advise me on how best to explain this. Regards, Bryan -Original Message- From: Andrew Lunn [mailto:and...@lunn.ch] Sent: Thursday, February 11, 2016 4:55 PM To: Bryan Whitehead - C21958 Cc: da...@davemloft.net; netdev@vger.kernel.org Subject: Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver On Thu, Feb 11, 2016 at 06:58:52PM +, bryan.whiteh...@microchip.com wrote: > This is the initial submission of an ethernet driver for the Microchip > LAN9352. > > The LAN9352 is a 2-Port 10/100 Managed Ethernet Switch with 16-Bit > Non-PCI CPU Interface. The CPU interface includes a basic ethernet > controller interface whose virtual phy is connected internally to a > 3rd port on the embedded switch. > > This driver only operates as a simple ethernet controller on the CPU > interface. Since this interface is connected directly to the embedded > switch, the result is that traffic can be sent and received on both > physical ports. > > Signed-off-by: Bryan Whitehead > --- > Documentation/devicetree/bindings/net/mchp9352.txt | 31 + > MAINTAINERS|9 + > drivers/net/ethernet/microchip/Kconfig | 32 +- > drivers/net/ethernet/microchip/Makefile|1 + > drivers/net/ethernet/microchip/mchp9352.c | 2587 > > drivers/net/ethernet/microchip/mchp9352.h | 443 > 6 files changed, 3102 insertions(+), 1 deletion(-) create mode > 100644 Documentation/devicetree/bindings/net/mchp9352.txt > create mode 100644 drivers/net/ethernet/microchip/mchp9352.c > create mode 100644 drivers/net/ethernet/microchip/mchp9352.h > > diff --git a/Documentation/devicetree/bindings/net/mchp9352.txt > b/Documentation/devicetree/bindings/net/mchp9352.txt > new file mode 100644 > index 000..5b22e73 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/mchp9352.txt > @@ -0,0 +1,31 @@ > +* Microchip LAN9352 Controller > + > +Required properties: > +- compatible : Should be "microchip,lan9352" > +- reg : Address and length of the io space for Microchip LAN > +- interrupts : Should contain Microchip LAN interrupt line > +- interrupt-parent : Should be the phandle for the interrupt > +controller > + that services interrupts for this device > +- phy-mode : See ethernet.txt file in the same directory Hi Bryan You still have not explained which of the two phys this phy-mode applies to? What if i want phy #0 to be rgmii and phy #1 to be rgmii-txid? Andrew
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
On Thu, Feb 11, 2016 at 06:18:25PM -0800, Florian Fainelli wrote: > On 11/02/16 10:58, bryan.whiteh...@microchip.com wrote: > > This is the initial submission of an ethernet driver for > > the Microchip LAN9352. > > > > The LAN9352 is a 2-Port 10/100 Managed Ethernet Switch with > > 16-Bit Non-PCI CPU Interface. The CPU interface includes a basic > > ethernet controller interface whose virtual phy is connected > > internally to a 3rd port on the embedded switch. > > > > This driver only operates as a simple ethernet controller > > on the CPU interface. Since this interface is connected directly > > to the embedded switch, the result is that traffic can be sent and > > received on both physical ports. > > If this is an integrated switch, have you considered using the DSA > subsystem and having this driver just be the Ethernet driver that > interfaces to your CPU interface, but does not attempt to configure the > switch in any way? I actually think switchdev might be the better model. Hard to say. The datasheet suggests it could be modelled as a three port switch, so DSA might be appropriate... Andrew
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
On 11/02/16 10:58, bryan.whiteh...@microchip.com wrote:
> This is the initial submission of an ethernet driver for
> the Microchip LAN9352.
>
> The LAN9352 is a 2-Port 10/100 Managed Ethernet Switch with
> 16-Bit Non-PCI CPU Interface. The CPU interface includes a basic
> ethernet controller interface whose virtual phy is connected
> internally to a 3rd port on the embedded switch.
>
> This driver only operates as a simple ethernet controller
> on the CPU interface. Since this interface is connected directly
> to the embedded switch, the result is that traffic can be sent and
> received on both physical ports.
If this is an integrated switch, have you considered using the DSA
subsystem and having this driver just be the Ethernet driver that
interfaces to your CPU interface, but does not attempt to configure the
switch in any way?
It would also help if you clarified the references to the SMSC911x
controllers and explain whether this was used for the Ethernet MAC, or
how the things are pieced together, as it seems like there could be
different strategies to support your hardware here.
>
> Signed-off-by: Bryan Whitehead
> ---
> Documentation/devicetree/bindings/net/mchp9352.txt | 31 +
> MAINTAINERS|9 +
> drivers/net/ethernet/microchip/Kconfig | 32 +-
> drivers/net/ethernet/microchip/Makefile|1 +
> drivers/net/ethernet/microchip/mchp9352.c | 2587
>
> drivers/net/ethernet/microchip/mchp9352.h | 443
> 6 files changed, 3102 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/net/mchp9352.txt
> create mode 100644 drivers/net/ethernet/microchip/mchp9352.c
> create mode 100644 drivers/net/ethernet/microchip/mchp9352.h
>
> diff --git a/Documentation/devicetree/bindings/net/mchp9352.txt
> b/Documentation/devicetree/bindings/net/mchp9352.txt
> new file mode 100644
> index 000..5b22e73
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/mchp9352.txt
> @@ -0,0 +1,31 @@
> +* Microchip LAN9352 Controller
> +
> +Required properties:
> +- compatible : Should be "microchip,lan9352"
> +- reg : Address and length of the io space for Microchip LAN
> +- interrupts : Should contain Microchip LAN interrupt line
> +- interrupt-parent : Should be the phandle for the interrupt controller
This is more like an optional property, in general there is a default
parent configured in your Device Tree.
> + that services interrupts for this device
> +- phy-mode : See ethernet.txt file in the same directory
Same question as Andrew, if this is a 2-port switch, should not we have
one phy-mode per port of the switch?
> +
> +Optional properties:
> +- reg-shift : Specify the quantity to shift the register offsets by
> +- reg-io-width : Specify the size (in bytes) of the IO accesses that
> + should be performed on the device. Valid value for Microchip LAN is
> + 2 or 4. If it's omitted or invalid, the size would be 2.
> +- microchip,irq-active-high : Indicates the IRQ polarity is active-high
Should not that be indicated in the interrupts property as a specific cell?
> +- microchip,irq-push-pull : Indicates the IRQ type is push-pull
Same here maybe?
> +- microchip,save-mac-address : Indicates that mac address needs to be saved
> + before resetting the controller
That's a software construct here, no?
> +
> +Examples:
> +
> +lan9220@f400 {
> + compatible = "microchip,lan9352";
> + reg = <0xf400 0x200>;
> + phy-mode = "mii";
> + interrupt-parent = <&gpio1>;
> + interrupts = <31>;
> + reg-io-width = <4>;
> + microchip,irq-push-pull;
> +};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f678c37..c39edef 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7136,6 +7136,15 @@ T: git git://git.monstr.eu/linux-2.6-microblaze.git
> S: Supported
> F: arch/microblaze/
>
> +MICROCHIP LAN9352 ETHERNET DRIVER
> +M: Microchip Linux Driver Support
> +M: Bryan Whitehead
> +L: netdev@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/net/mchp9352.txt
> +F: drivers/net/ethernet/microchip/mchp9352.h
> +F: drivers/net/ethernet/microchip/mchp9352.c
> +
> MICROSOFT SURFACE PRO 3 BUTTON DRIVER
> M: Chen Yu
> L: platform-driver-...@vger.kernel.org
> diff --git a/drivers/net/ethernet/microchip/Kconfig
> b/drivers/net/ethernet/microchip/Kconfig
> index 36a09d9..1f346a7 100644
> --- a/drivers/net/ethernet/microchip/Kconfig
> +++ b/drivers/net/ethernet/microchip/Kconfig
> @@ -5,7 +5,6 @@
> config NET_VENDOR_MICROCHIP
> bool "Microchip devices"
> default y
> - depends on SPI
> ---help---
> If you have a network (Ethernet) card belonging to this class, say Y.
>
> @@ -28,6 +27,7 @@ config ENC28J60
>
> config ENC28J60_WRITEVERIFY
> bool "Enable write verify"
> + depends on SPI
> depends on ENC28J60
That could be a preliminary patch, se
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
Hi,
> +static int mchp9352_open(struct net_device *dev)
> +{
> +
> + MCHP_TRACE(pdata, ifup, "Testing irq handler using IRQ %d", dev->irq);
> + pdata->software_irq_signal = 0;
> +
> + /* Testing irq handler */
> + smp_wmb();
Should not there at least be a matching smp_rmb() in the irq handler?
Maybe an atomic_t would be a better choice for a flag like
software_irq_signal here.
> +
> +static int mchp9352_drv_probe(struct platform_device *pdev)
> +
> + netif_carrier_off(dev);
> +
> + retval = register_netdev(dev);
> + if (retval) {
> + MCHP_WARN(pdata, probe, "Error %i registering device", retval);
> + goto out_free_irq;
> + } else {
> + MCHP_TRACE(pdata, probe,
> +"Network interface: \"%s\"", dev->name);
> + }
Note that as soon as the network device is registered "open" may be
called so everything should be set up already. In this case the mac
address (dev->dev_addr) is accessed in open but may not yet contain
valid data when register_netdev is called.
Regards,
Lino
Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
On Thu, Feb 11, 2016 at 06:58:52PM +, bryan.whiteh...@microchip.com wrote: > This is the initial submission of an ethernet driver for > the Microchip LAN9352. > > The LAN9352 is a 2-Port 10/100 Managed Ethernet Switch with > 16-Bit Non-PCI CPU Interface. The CPU interface includes a basic > ethernet controller interface whose virtual phy is connected > internally to a 3rd port on the embedded switch. > > This driver only operates as a simple ethernet controller > on the CPU interface. Since this interface is connected directly > to the embedded switch, the result is that traffic can be sent and > received on both physical ports. > > Signed-off-by: Bryan Whitehead > --- > Documentation/devicetree/bindings/net/mchp9352.txt | 31 + > MAINTAINERS|9 + > drivers/net/ethernet/microchip/Kconfig | 32 +- > drivers/net/ethernet/microchip/Makefile|1 + > drivers/net/ethernet/microchip/mchp9352.c | 2587 > > drivers/net/ethernet/microchip/mchp9352.h | 443 > 6 files changed, 3102 insertions(+), 1 deletion(-) > create mode 100644 Documentation/devicetree/bindings/net/mchp9352.txt > create mode 100644 drivers/net/ethernet/microchip/mchp9352.c > create mode 100644 drivers/net/ethernet/microchip/mchp9352.h > > diff --git a/Documentation/devicetree/bindings/net/mchp9352.txt > b/Documentation/devicetree/bindings/net/mchp9352.txt > new file mode 100644 > index 000..5b22e73 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/mchp9352.txt > @@ -0,0 +1,31 @@ > +* Microchip LAN9352 Controller > + > +Required properties: > +- compatible : Should be "microchip,lan9352" > +- reg : Address and length of the io space for Microchip LAN > +- interrupts : Should contain Microchip LAN interrupt line > +- interrupt-parent : Should be the phandle for the interrupt controller > + that services interrupts for this device > +- phy-mode : See ethernet.txt file in the same directory Hi Bryan You still have not explained which of the two phys this phy-mode applies to? What if i want phy #0 to be rgmii and phy #1 to be rgmii-txid? Andrew
